| Message ID | 1554362413-3305-3-git-send-email-mojha@codeaurora.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | irqchip: Use devm_platform_ioremap_resource() | expand |
On 04/04/2019 08:20, Mukesh Ojha wrote: > devm_platform_ioremap_resource() internally have platform_get_resource() > and devm_ioremap_resource() in it. So instead of calling them separately > use devm_platform_ioremap_resource() directly. > > Signed-off-by: Mukesh Ojha <mojha@codeaurora.org> > --- > drivers/irqchip/irq-imgpdc.c | 10 +--------- > 1 file changed, 1 insertion(+), 9 deletions(-) > > diff --git a/drivers/irqchip/irq-imgpdc.c b/drivers/irqchip/irq-imgpdc.c > index d00489a..8904a5f 100644 > --- a/drivers/irqchip/irq-imgpdc.c > +++ b/drivers/irqchip/irq-imgpdc.c > @@ -307,13 +307,6 @@ static int pdc_intc_probe(struct platform_device *pdev) > if (!node) > return -ENOENT; > > - /* Get registers */ > - res_regs = platform_get_resource(pdev, IORESOURCE_MEM, 0); > - if (res_regs == NULL) { > - dev_err(&pdev->dev, "cannot find registers resource\n"); > - return -ENOENT; > - } > - > /* Allocate driver data */ > priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL); > if (!priv) { > @@ -324,8 +317,7 @@ static int pdc_intc_probe(struct platform_device *pdev) > platform_set_drvdata(pdev, priv); > > /* Ioremap the registers */ > - priv->pdc_base = devm_ioremap(&pdev->dev, res_regs->start, > - resource_size(res_regs)); > + priv->pdc_base = devm_platform_ioremap_resource(pdev, 0); > if (!priv->pdc_base) > return -EIO; > > What happens to the res_regs variable then? Also, and more importantly, devm_platform_ioremap_resource doesn't return NULL on error, but an ERR_PTR. Yes, the bug was already there, but since you're changing it, you might as well fix the thing. Thanks, M.
On 29/04/2019 09:13, Marc Zyngier wrote: > On 04/04/2019 08:20, Mukesh Ojha wrote: >> devm_platform_ioremap_resource() internally have platform_get_resource() >> and devm_ioremap_resource() in it. So instead of calling them separately >> use devm_platform_ioremap_resource() directly. >> >> Signed-off-by: Mukesh Ojha <mojha@codeaurora.org> >> --- >> drivers/irqchip/irq-imgpdc.c | 10 +--------- >> 1 file changed, 1 insertion(+), 9 deletions(-) >> >> diff --git a/drivers/irqchip/irq-imgpdc.c b/drivers/irqchip/irq-imgpdc.c >> index d00489a..8904a5f 100644 >> --- a/drivers/irqchip/irq-imgpdc.c >> +++ b/drivers/irqchip/irq-imgpdc.c >> @@ -307,13 +307,6 @@ static int pdc_intc_probe(struct platform_device *pdev) >> if (!node) >> return -ENOENT; >> >> - /* Get registers */ >> - res_regs = platform_get_resource(pdev, IORESOURCE_MEM, 0); >> - if (res_regs == NULL) { >> - dev_err(&pdev->dev, "cannot find registers resource\n"); >> - return -ENOENT; >> - } >> - >> /* Allocate driver data */ >> priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL); >> if (!priv) { >> @@ -324,8 +317,7 @@ static int pdc_intc_probe(struct platform_device *pdev) >> platform_set_drvdata(pdev, priv); >> >> /* Ioremap the registers */ >> - priv->pdc_base = devm_ioremap(&pdev->dev, res_regs->start, >> - resource_size(res_regs)); >> + priv->pdc_base = devm_platform_ioremap_resource(pdev, 0); >> if (!priv->pdc_base) >> return -EIO; >> >> > > What happens to the res_regs variable then? > > Also, and more importantly, devm_platform_ioremap_resource doesn't > return NULL on error, but an ERR_PTR. Yes, the bug was already there, > but since you're changing it, you might as well fix the thing. Actually, the current code is right, and you're actively breaking it. Boo. M.
diff --git a/drivers/irqchip/irq-imgpdc.c b/drivers/irqchip/irq-imgpdc.c index d00489a..8904a5f 100644 --- a/drivers/irqchip/irq-imgpdc.c +++ b/drivers/irqchip/irq-imgpdc.c @@ -307,13 +307,6 @@ static int pdc_intc_probe(struct platform_device *pdev) if (!node) return -ENOENT; - /* Get registers */ - res_regs = platform_get_resource(pdev, IORESOURCE_MEM, 0); - if (res_regs == NULL) { - dev_err(&pdev->dev, "cannot find registers resource\n"); - return -ENOENT; - } - /* Allocate driver data */ priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL); if (!priv) { @@ -324,8 +317,7 @@ static int pdc_intc_probe(struct platform_device *pdev) platform_set_drvdata(pdev, priv); /* Ioremap the registers */ - priv->pdc_base = devm_ioremap(&pdev->dev, res_regs->start, - resource_size(res_regs)); + priv->pdc_base = devm_platform_ioremap_resource(pdev, 0); if (!priv->pdc_base) return -EIO;
devm_platform_ioremap_resource() internally have platform_get_resource() and devm_ioremap_resource() in it. So instead of calling them separately use devm_platform_ioremap_resource() directly. Signed-off-by: Mukesh Ojha <mojha@codeaurora.org> --- drivers/irqchip/irq-imgpdc.c | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-)