| Message ID | 20190904023515.7107-2-andrew.smirnov@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | CAAM bugfixes, small improvements | expand |
On 9/4/2019 5:35 AM, Andrey Smirnov wrote: > In order to access IP block's registers we need to enable appropriate > clocks first, otherwise we are risking hanging the CPU. > > The problem becomes very apparent when trying to use CAAM driver built > as a kernel module. In that case caam_probe() gets called after > clk_disable_unused() which means all of the necessary clocks are > guaranteed to be disabled. > > Coincidentally, this change also fixes iomap leak introduced by early > return (instead of "goto iounmap_ctrl") in commit > 41fc54afae70 ("crypto: caam - simplfy clock initialization") > > Tested on ZII i.MX6Q+ RDU2 > > Fixes: 176435ad2ac7 ("crypto: caam - defer probing until QMan is available") > Fixes: 41fc54afae70 ("crypto: caam - simplfy clock initialization") > 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 Tested-by: Horia Geantă <horia.geanta@nxp.com> Considering this is a boot hang, in case this does not make into v5.4 I would appreciate appending: Cc: <stable@vger.kernel.org> Thanks, Horia
On Fri, Sep 06, 2019 at 11:18:19AM +0000, Horia Geanta wrote: > On 9/4/2019 5:35 AM, Andrey Smirnov wrote: > > In order to access IP block's registers we need to enable appropriate > > clocks first, otherwise we are risking hanging the CPU. > > > > The problem becomes very apparent when trying to use CAAM driver built > > as a kernel module. In that case caam_probe() gets called after > > clk_disable_unused() which means all of the necessary clocks are > > guaranteed to be disabled. > > > > Coincidentally, this change also fixes iomap leak introduced by early > > return (instead of "goto iounmap_ctrl") in commit > > 41fc54afae70 ("crypto: caam - simplfy clock initialization") > > > > Tested on ZII i.MX6Q+ RDU2 > > > > Fixes: 176435ad2ac7 ("crypto: caam - defer probing until QMan is available") > > Fixes: 41fc54afae70 ("crypto: caam - simplfy clock initialization") > > 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 > Tested-by: Horia Geantă <horia.geanta@nxp.com> > > Considering this is a boot hang, in case this does not make into v5.4 > I would appreciate appending: > Cc: <stable@vger.kernel.org> This patch does not apply against cryptodev or crypto. Please rebase. Thanks,
On Mon, Sep 09, 2019 at 05:21:55PM +1000, Herbert Xu wrote: > On Fri, Sep 06, 2019 at 11:18:19AM +0000, Horia Geanta wrote: > > On 9/4/2019 5:35 AM, Andrey Smirnov wrote: > > > In order to access IP block's registers we need to enable appropriate > > > clocks first, otherwise we are risking hanging the CPU. > > > > > > The problem becomes very apparent when trying to use CAAM driver built > > > as a kernel module. In that case caam_probe() gets called after > > > clk_disable_unused() which means all of the necessary clocks are > > > guaranteed to be disabled. > > > > > > Coincidentally, this change also fixes iomap leak introduced by early > > > return (instead of "goto iounmap_ctrl") in commit > > > 41fc54afae70 ("crypto: caam - simplfy clock initialization") > > > > > > Tested on ZII i.MX6Q+ RDU2 > > > > > > Fixes: 176435ad2ac7 ("crypto: caam - defer probing until QMan is available") > > > Fixes: 41fc54afae70 ("crypto: caam - simplfy clock initialization") > > > 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 > > Tested-by: Horia Geantă <horia.geanta@nxp.com> > > > > Considering this is a boot hang, in case this does not make into v5.4 > > I would appreciate appending: > > Cc: <stable@vger.kernel.org> > > This patch does not apply against cryptodev or crypto. Nevermind, I was trying to apply patch 4 on top of patch 1 which is why it didn't work. Cheers,
diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c index 3c059d0e4207..db22777d59b4 100644 --- a/drivers/crypto/caam/ctrl.c +++ b/drivers/crypto/caam/ctrl.c @@ -594,6 +594,21 @@ static int caam_probe(struct platform_device *pdev) dev_set_drvdata(dev, ctrlpriv); nprop = pdev->dev.of_node; + imx_soc_match = soc_device_match(caam_imx_soc_table); + caam_imx = (bool)imx_soc_match; + + if (imx_soc_match) { + if (!imx_soc_match->data) { + dev_err(dev, "No clock data provided for i.MX SoC"); + return -EINVAL; + } + + ret = init_clocks(dev, imx_soc_match->data); + if (ret) + return ret; + } + + /* Get configuration properties from device tree */ /* First, get register page */ ctrl = of_iomap(nprop, 0); @@ -604,9 +619,6 @@ static int caam_probe(struct platform_device *pdev) caam_little_end = !(bool)(rd_reg32(&ctrl->perfmon.status) & (CSTA_PLEND | CSTA_ALT_PLEND)); - imx_soc_match = soc_device_match(caam_imx_soc_table); - caam_imx = (bool)imx_soc_match; - comp_params = rd_reg32(&ctrl->perfmon.comp_parms_ms); if (comp_params & CTPR_MS_PS && rd_reg32(&ctrl->mcr) & MCFGR_LONG_PTR) caam_ptr_sz = sizeof(u64); @@ -640,18 +652,6 @@ static int caam_probe(struct platform_device *pdev) } #endif - if (imx_soc_match) { - if (!imx_soc_match->data) { - dev_err(dev, "No clock data provided for i.MX SoC"); - return -EINVAL; - } - - ret = init_clocks(dev, imx_soc_match->data); - if (ret) - return ret; - } - - /* Allocating the BLOCK_OFFSET based on the supported page size on * the platform */
In order to access IP block's registers we need to enable appropriate clocks first, otherwise we are risking hanging the CPU. The problem becomes very apparent when trying to use CAAM driver built as a kernel module. In that case caam_probe() gets called after clk_disable_unused() which means all of the necessary clocks are guaranteed to be disabled. Coincidentally, this change also fixes iomap leak introduced by early return (instead of "goto iounmap_ctrl") in commit 41fc54afae70 ("crypto: caam - simplfy clock initialization") Tested on ZII i.MX6Q+ RDU2 Fixes: 176435ad2ac7 ("crypto: caam - defer probing until QMan is available") Fixes: 41fc54afae70 ("crypto: caam - simplfy clock initialization") 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/ctrl.c | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-)