| Message ID | 20190904023515.7107-3-andrew.smirnov@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | CAAM bugfixes, small improvements | expand |
Hi Andrey, On Tue, Sep 3, 2019 at 11:37 PM Andrey Smirnov <andrew.smirnov@gmail.com> wrote: > > Use devres to unmap memory and drop explicit de-initialization > code. > > NOTE: There's no corresponding unmapping code in caam_jr_remove which > seems like a resource leak. > > Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com> > Cc: Chris Healy <cphealy@gmail.com> > Cc: Lucas Stach <l.stach@pengutronix.de> > Cc: Horia Geantă <horia.geanta@nxp.com> > Cc: Herbert Xu <herbert@gondor.apana.org.au> > Cc: Iuliana Prodan <iuliana.prodan@nxp.com> > Cc: linux-crypto@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > --- > drivers/crypto/caam/jr.c | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > > diff --git a/drivers/crypto/caam/jr.c b/drivers/crypto/caam/jr.c > index 417ad52615c6..7947d61a25cf 100644 > --- a/drivers/crypto/caam/jr.c > +++ b/drivers/crypto/caam/jr.c > @@ -498,6 +498,7 @@ static int caam_jr_probe(struct platform_device *pdev) > struct caam_job_ring __iomem *ctrl; > struct caam_drv_private_jr *jrpriv; > static int total_jobrs; > + struct resource *r; > int error; > > jrdev = &pdev->dev; > @@ -513,9 +514,15 @@ static int caam_jr_probe(struct platform_device *pdev) > nprop = pdev->dev.of_node; > /* Get configuration properties from device tree */ > /* First, get register page */ > - ctrl = of_iomap(nprop, 0); > + r = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + if (!r) { > + dev_err(jrdev, "platform_get_resource() failed\n"); > + return -ENOMEM; > + } > + > + ctrl = devm_ioremap(jrdev, r->start, resource_size(r)); It seems that using devm_platform_ioremap_resource() could make the code even smaller. Regards, Fabio Estevam
On Tue, Sep 3, 2019 at 7:43 PM Fabio Estevam <festevam@gmail.com> wrote: > > Hi Andrey, > > On Tue, Sep 3, 2019 at 11:37 PM Andrey Smirnov <andrew.smirnov@gmail.com> wrote: > > > > Use devres to unmap memory and drop explicit de-initialization > > code. > > > > NOTE: There's no corresponding unmapping code in caam_jr_remove which > > seems like a resource leak. > > > > Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com> > > Cc: Chris Healy <cphealy@gmail.com> > > Cc: Lucas Stach <l.stach@pengutronix.de> > > Cc: Horia Geantă <horia.geanta@nxp.com> > > Cc: Herbert Xu <herbert@gondor.apana.org.au> > > Cc: Iuliana Prodan <iuliana.prodan@nxp.com> > > Cc: linux-crypto@vger.kernel.org > > Cc: linux-kernel@vger.kernel.org > > --- > > drivers/crypto/caam/jr.c | 13 +++++++++---- > > 1 file changed, 9 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/crypto/caam/jr.c b/drivers/crypto/caam/jr.c > > index 417ad52615c6..7947d61a25cf 100644 > > --- a/drivers/crypto/caam/jr.c > > +++ b/drivers/crypto/caam/jr.c > > @@ -498,6 +498,7 @@ static int caam_jr_probe(struct platform_device *pdev) > > struct caam_job_ring __iomem *ctrl; > > struct caam_drv_private_jr *jrpriv; > > static int total_jobrs; > > + struct resource *r; > > int error; > > > > jrdev = &pdev->dev; > > @@ -513,9 +514,15 @@ static int caam_jr_probe(struct platform_device *pdev) > > nprop = pdev->dev.of_node; > > /* Get configuration properties from device tree */ > > /* First, get register page */ > > - ctrl = of_iomap(nprop, 0); > > + r = platform_get_resource(pdev, IORESOURCE_MEM, 0); > > + if (!r) { > > + dev_err(jrdev, "platform_get_resource() failed\n"); > > + return -ENOMEM; > > + } > > + > > + ctrl = devm_ioremap(jrdev, r->start, resource_size(r)); > > It seems that using devm_platform_ioremap_resource() could make the > code even smaller. Unfortunately that function would do devm_ioremap_resource() under the hood and claim the ownership of the corresponding memory region. That's going to create a conflict with devm_of_iomap() used in "crypto: caam - use devres to unmap memory". Thanks, Andrey Smirnov
On 9/4/2019 5:35 AM, Andrey Smirnov wrote: > Use devres to unmap memory and drop explicit de-initialization > code. > > NOTE: There's no corresponding unmapping code in caam_jr_remove which > seems like a resource leak. > > Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com> > Cc: Chris Healy <cphealy@gmail.com> > Cc: Lucas Stach <l.stach@pengutronix.de> > Cc: Horia Geantă <horia.geanta@nxp.com> > Cc: Herbert Xu <herbert@gondor.apana.org.au> > Cc: Iuliana Prodan <iuliana.prodan@nxp.com> > Cc: linux-crypto@vger.kernel.org > Cc: linux-kernel@vger.kernel.org Reviewed-by: Horia Geantă <horia.geanta@nxp.com> Thanks, Horia
diff --git a/drivers/crypto/caam/jr.c b/drivers/crypto/caam/jr.c index 417ad52615c6..7947d61a25cf 100644 --- a/drivers/crypto/caam/jr.c +++ b/drivers/crypto/caam/jr.c @@ -498,6 +498,7 @@ static int caam_jr_probe(struct platform_device *pdev) struct caam_job_ring __iomem *ctrl; struct caam_drv_private_jr *jrpriv; static int total_jobrs; + struct resource *r; int error; jrdev = &pdev->dev; @@ -513,9 +514,15 @@ static int caam_jr_probe(struct platform_device *pdev) nprop = pdev->dev.of_node; /* Get configuration properties from device tree */ /* First, get register page */ - ctrl = of_iomap(nprop, 0); + r = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (!r) { + dev_err(jrdev, "platform_get_resource() failed\n"); + return -ENOMEM; + } + + ctrl = devm_ioremap(jrdev, r->start, resource_size(r)); if (!ctrl) { - dev_err(jrdev, "of_iomap() failed\n"); + dev_err(jrdev, "devm_ioremap() failed\n"); return -ENOMEM; } @@ -525,7 +532,6 @@ static int caam_jr_probe(struct platform_device *pdev) if (error) { dev_err(jrdev, "dma_set_mask_and_coherent failed (%d)\n", error); - iounmap(ctrl); return error; } @@ -536,7 +542,6 @@ static int caam_jr_probe(struct platform_device *pdev) error = caam_jr_init(jrdev); /* now turn on hardware */ if (error) { irq_dispose_mapping(jrpriv->irq); - iounmap(ctrl); return error; }
Use devres to unmap memory and drop explicit de-initialization code. NOTE: There's no corresponding unmapping code in caam_jr_remove which seems like a resource leak. Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com> Cc: Chris Healy <cphealy@gmail.com> Cc: Lucas Stach <l.stach@pengutronix.de> Cc: Horia Geantă <horia.geanta@nxp.com> Cc: Herbert Xu <herbert@gondor.apana.org.au> Cc: Iuliana Prodan <iuliana.prodan@nxp.com> Cc: linux-crypto@vger.kernel.org Cc: linux-kernel@vger.kernel.org --- drivers/crypto/caam/jr.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)