Re: [PATCH v4 02/16] crypto: caam - simplify clock initialization

[Andrey Smirnov]

On Thu, Jul 4, 2019 at 8:43 AM Iuliana Prodan <iuliana.prodan@xxxxxxx> wrote:
>
> On 7/3/2019 11:15 AM, Andrey Smirnov wrote:
> > Simplify clock initialization code by converting it to use clk-bulk,
> > devres and soc_device_match() match table. No functional change
> > intended.
> >
> > Signed-off-by: Andrey Smirnov <andrew.smirnov@xxxxxxxxx>
> > Cc: Chris Spencer <christopher.spencer@xxxxxxxxx>
> > Cc: Cory Tusar <cory.tusar@xxxxxxxx>
> > Cc: Chris Healy <cphealy@xxxxxxxxx>
> > Cc: Lucas Stach <l.stach@xxxxxxxxxxxxxx>
> > Cc: Horia GeantÄ <horia.geanta@xxxxxxx>
> > Cc: Aymen Sghaier <aymen.sghaier@xxxxxxx>
> > Cc: Leonard Crestez <leonard.crestez@xxxxxxx>
> > Cc: linux-crypto@xxxxxxxxxxxxxxx
> > Cc: linux-kernel@xxxxxxxxxxxxxxx
> > ---
> >   drivers/crypto/caam/ctrl.c   | 203 +++++++++++++++++------------------
> >   drivers/crypto/caam/intern.h |   7 +-
> >   2 files changed, 98 insertions(+), 112 deletions(-)
> >
> > diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
> > index e674d8770cdb..908d3ecf6d1c 100644
> > --- a/drivers/crypto/caam/ctrl.c
> > +++ b/drivers/crypto/caam/ctrl.c
> > @@ -25,16 +25,6 @@ EXPORT_SYMBOL(caam_dpaa2);
> >   #include "qi.h"
> >   #endif
> >
> > -/*
> > - * i.MX targets tend to have clock control subsystems that can
> > - * enable/disable clocking to our device.
> > - */
> > -static inline struct clk *caam_drv_identify_clk(struct device *dev,
> > -                                             char *clk_name)
> > -{
> > -     return caam_imx ? devm_clk_get(dev, clk_name) : NULL;
> > -}
> > -
> >   /*
> >    * Descriptor to instantiate RNG State Handle 0 in normal mode and
> >    * load the JDKEK, TDKEK and TDSK registers
> > @@ -342,13 +332,6 @@ static int caam_remove(struct platform_device *pdev)
> >       /* Unmap controller region */
> >       iounmap(ctrl);
> >
> > -     /* shut clocks off before finalizing shutdown */
> > -     clk_disable_unprepare(ctrlpriv->caam_ipg);
> > -     if (ctrlpriv->caam_mem)
> > -             clk_disable_unprepare(ctrlpriv->caam_mem);
> > -     clk_disable_unprepare(ctrlpriv->caam_aclk);
> > -     if (ctrlpriv->caam_emi_slow)
> > -             clk_disable_unprepare(ctrlpriv->caam_emi_slow);
> >       return 0;
> >   }
> >
> > @@ -497,20 +480,102 @@ static const struct of_device_id caam_match[] = {
> >   };
> >   MODULE_DEVICE_TABLE(of, caam_match);
> >
> > +struct caam_imx_data {
> > +     const struct clk_bulk_data *clks;
> > +     int num_clks;
> > +};
> > +
> > +static const struct clk_bulk_data caam_imx6_clks[] = {
> > +     { .id = "ipg" },
> > +     { .id = "mem" },
> > +     { .id = "aclk" },
> > +     { .id = "emi_slow" },
> > +};
> > +
> > +static const struct caam_imx_data caam_imx6_data = {
> > +     .clks = caam_imx6_clks,
> > +     .num_clks = ARRAY_SIZE(caam_imx6_clks),
> > +};
> > +
> > +static const struct clk_bulk_data caam_imx7_clks[] = {
> > +     { .id = "ipg" },
> > +     { .id = "aclk" },
> > +};
> > +
> > +static const struct caam_imx_data caam_imx7_data = {
> > +     .clks = caam_imx7_clks,
> > +     .num_clks = ARRAY_SIZE(caam_imx7_clks),
> > +};
> > +
> > +static const struct clk_bulk_data caam_imx6ul_clks[] = {
> > +     { .id = "ipg" },
> > +     { .id = "mem" },
> > +     { .id = "aclk" },
> > +};
> > +
> > +static const struct caam_imx_data caam_imx6ul_data = {
> > +     .clks = caam_imx6ul_clks,
> > +     .num_clks = ARRAY_SIZE(caam_imx6ul_clks),
> > +};
> > +
> > +static const struct soc_device_attribute caam_imx_soc_table[] = {
> > +     { .soc_id = "i.MX6UL", .data = &caam_imx6ul_data },
> > +     { .soc_id = "i.MX6*",  .data = &caam_imx6_data },
> > +     { .soc_id = "i.MX7*",  .data = &caam_imx7_data },
> > +     { .family = "Freescale i.MX" },
> > +};
>
> You need to add a {/* sentinel */} in caam_imx_soc_table, otherwise will
> crash for other than i.MX targets, when trying to identify the SoC.
>

Ugh, definitely, my bad. Thanks for catching this!

Thanks,
Andrey Smirnov