| Message ID | 20240410075815.4030570-1-pankaj.gupta@nxp.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | [v2] caam: init-clk based on caam-page0-access | expand |
Reviewed-by: Gaurav Jain <gaurav.jain@nxp.com> > -----Original Message----- > From: Pankaj Gupta <pankaj.gupta@nxp.com> > Sent: Wednesday, April 10, 2024 1:28 PM > To: Gaurav Jain <gaurav.jain@nxp.com>; Horia Geanta <horia.geanta@nxp.com>; > Varun Sethi <V.Sethi@nxp.com>; herbert@gondor.apana.org.au; > davem@davemloft.net; Iuliana Prodan <iuliana.prodan@nxp.com>; linux- > crypto@vger.kernel.org; linux-kernel@vger.kernel.org; dl-linux-imx <linux- > imx@nxp.com> > Cc: Pankaj Gupta <pankaj.gupta@nxp.com> > Subject: [PATCH v2] caam: init-clk based on caam-page0-access > > CAAM clock initialization to be done based on, soc specific info stored in struct > caam_imx_data: > - caam-page0-access flag > - num_clks > > Signed-off-by: Pankaj Gupta <pankaj.gupta@nxp.com> > --- > v2: > - Considering the OPTEE enablement check too, for setting the > variable 'reg_access'. > > drivers/crypto/caam/ctrl.c | 25 +++++++++++++++++++++++-- > 1 file changed, 23 insertions(+), 2 deletions(-) > > diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c index > bdf367f3f679..355ff92f4549 100644 > --- a/drivers/crypto/caam/ctrl.c > +++ b/drivers/crypto/caam/ctrl.c > @@ -512,6 +512,7 @@ static const struct of_device_id caam_match[] = > { MODULE_DEVICE_TABLE(of, caam_match); > > struct caam_imx_data { > + bool page0_access; > const struct clk_bulk_data *clks; > int num_clks; > }; > @@ -524,6 +525,7 @@ static const struct clk_bulk_data caam_imx6_clks[] = { }; > > static const struct caam_imx_data caam_imx6_data = { > + .page0_access = true, > .clks = caam_imx6_clks, > .num_clks = ARRAY_SIZE(caam_imx6_clks), }; @@ -534,6 +536,7 @@ > static const struct clk_bulk_data caam_imx7_clks[] = { }; > > static const struct caam_imx_data caam_imx7_data = { > + .page0_access = true, > .clks = caam_imx7_clks, > .num_clks = ARRAY_SIZE(caam_imx7_clks), }; @@ -545,6 +548,7 @@ > static const struct clk_bulk_data caam_imx6ul_clks[] = { }; > > static const struct caam_imx_data caam_imx6ul_data = { > + .page0_access = true, > .clks = caam_imx6ul_clks, > .num_clks = ARRAY_SIZE(caam_imx6ul_clks), }; @@ -554,15 +558,23 > @@ static const struct clk_bulk_data caam_vf610_clks[] = { }; > > static const struct caam_imx_data caam_vf610_data = { > + .page0_access = true, > .clks = caam_vf610_clks, > .num_clks = ARRAY_SIZE(caam_vf610_clks), }; > > +static const struct caam_imx_data caam_imx8ulp_data = { > + .page0_access = false, > + .clks = NULL, > + .num_clks = 0, > +}; > + > 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 }, > { .soc_id = "i.MX8M*", .data = &caam_imx7_data }, > + { .soc_id = "i.MX8ULP", .data = &caam_imx8ulp_data }, > { .soc_id = "VF*", .data = &caam_vf610_data }, > { .family = "Freescale i.MX" }, > { /* sentinel */ } > @@ -860,6 +872,7 @@ static int caam_probe(struct platform_device *pdev) > int pg_size; > int BLOCK_OFFSET = 0; > bool reg_access = true; > + const struct caam_imx_data *imx_soc_data; > > ctrlpriv = devm_kzalloc(&pdev->dev, sizeof(*ctrlpriv), GFP_KERNEL); > if (!ctrlpriv) > @@ -889,7 +902,15 @@ static int caam_probe(struct platform_device *pdev) > > reg_access = !ctrlpriv->optee_en; > > - if (!imx_soc_match->data) { > + if (imx_soc_match->data) { > + imx_soc_data = imx_soc_match->data; > + reg_access = reg_access && imx_soc_data- > >page0_access; > + /* > + * CAAM clocks cannot be controlled from kernel. > + */ > + if (!imx_soc_data->num_clks) > + goto iomap_ctrl; > + } else { > dev_err(dev, "No clock data provided for i.MX SoC"); > return -EINVAL; > } > @@ -899,7 +920,7 @@ static int caam_probe(struct platform_device *pdev) > return ret; > } > > - > +iomap_ctrl: > /* Get configuration properties from device tree */ > /* First, get register page */ > ctrl = devm_of_iomap(dev, nprop, 0, NULL); > -- > 2.34.1
On 4/10/2024 11:00 AM, Pankaj Gupta wrote: > CAAM clock initialization to be done based on, soc specific > info stored in struct caam_imx_data: > - caam-page0-access flag > - num_clks > [...] > +static const struct caam_imx_data caam_imx8ulp_data = { > + .page0_access = false, > + .clks = NULL, > + .num_clks = 0, > +}; Not needed, an empty static struct would do. > 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 }, > { .soc_id = "i.MX8M*", .data = &caam_imx7_data }, > + { .soc_id = "i.MX8ULP", .data = &caam_imx8ulp_data }, > { .soc_id = "VF*", .data = &caam_vf610_data }, > { .family = "Freescale i.MX" }, > { /* sentinel */ } As Francesco pointed out in v1, this change is not related. Ideally, adding support for i.MX8ULP SoC should be done in a separate patch, explaining a bit the particularities - similar to the commit message here: https://github.com/nxp-imx/linux-imx/commit/d420f224ee02c164f6bdb3c8bbb5ec5827eaba37 > @@ -889,7 +902,15 @@ static int caam_probe(struct platform_device *pdev) > > reg_access = !ctrlpriv->optee_en; > > - if (!imx_soc_match->data) { > + if (imx_soc_match->data) { > + imx_soc_data = imx_soc_match->data; > + reg_access = reg_access && imx_soc_data->page0_access; > + /* > + * CAAM clocks cannot be controlled from kernel. > + */ > + if (!imx_soc_data->num_clks) > + goto iomap_ctrl; > + } else { > dev_err(dev, "No clock data provided for i.MX SoC"); > return -EINVAL; > } if/else could be avoided, making code simpler: if (!imx_soc_match->data) { dev_err(dev, "No clock data provided for i.MX SoC"); return -EINVAL; } imx_soc_data = imx_soc_match->data; reg_access = reg_access && imx_soc_data->page0_access; /* CAAM clocks cannot be controlled from kernel. */ if (!imx_soc_data->num_clks) goto iomap_ctrl; Regards, Horia
> -----Original Message----- > From: Horia Geanta <horia.geanta@nxp.com> > Sent: Friday, April 12, 2024 6:20 PM > To: Pankaj Gupta <pankaj.gupta@nxp.com>; Gaurav Jain > <gaurav.jain@nxp.com>; Varun Sethi <V.Sethi@nxp.com>; > herbert@gondor.apana.org.au; davem@davemloft.net; Iuliana Prodan > <iuliana.prodan@nxp.com>; linux-crypto@vger.kernel.org; linux- > kernel@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; Francesco Dolcini > <francesco@dolcini.it> > Subject: Re: [PATCH v2] caam: init-clk based on caam-page0-access > > On 4/10/2024 11:00 AM, Pankaj Gupta wrote: > > CAAM clock initialization to be done based on, soc specific info > > stored in struct caam_imx_data: > > - caam-page0-access flag > > - num_clks > > > [...] > > +static const struct caam_imx_data caam_imx8ulp_data = { > > + .page0_access = false, > > + .clks = NULL, > > + .num_clks = 0, > > +}; > Not needed, an empty static struct would do. Accepted. Will add this as part of separate patch, requested below. > > > 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 }, > > { .soc_id = "i.MX8M*", .data = &caam_imx7_data }, > > + { .soc_id = "i.MX8ULP", .data = &caam_imx8ulp_data }, > > { .soc_id = "VF*", .data = &caam_vf610_data }, > > { .family = "Freescale i.MX" }, > > { /* sentinel */ } > As Francesco pointed out in v1, this change is not related. > > Ideally, adding support for i.MX8ULP SoC should be done in a separate patch, > explaining a bit the particularities - similar to the commit message here: > https://github.com/nxp-imx/linux- > imx/commit/d420f224ee02c164f6bdb3c8bbb5ec5827eaba37 Will create a separate patch for this change. > > > @@ -889,7 +902,15 @@ static int caam_probe(struct platform_device > > *pdev) > > > > reg_access = !ctrlpriv->optee_en; > > > > - if (!imx_soc_match->data) { > > + if (imx_soc_match->data) { > > + imx_soc_data = imx_soc_match->data; > > + reg_access = reg_access && imx_soc_data- > >page0_access; > > + /* > > + * CAAM clocks cannot be controlled from kernel. > > + */ > > + if (!imx_soc_data->num_clks) > > + goto iomap_ctrl; > > + } else { > > dev_err(dev, "No clock data provided for i.MX SoC"); > > return -EINVAL; > > } > if/else could be avoided, making code simpler: > > if (!imx_soc_match->data) { > dev_err(dev, "No clock data provided for i.MX SoC"); > return -EINVAL; > } > > imx_soc_data = imx_soc_match->data; > reg_access = reg_access && imx_soc_data->page0_access; > /* CAAM clocks cannot be controlled from kernel. */ > if (!imx_soc_data->num_clks) > goto iomap_ctrl; > [Accepted] > Regards, > Horia
diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c index bdf367f3f679..355ff92f4549 100644 --- a/drivers/crypto/caam/ctrl.c +++ b/drivers/crypto/caam/ctrl.c @@ -512,6 +512,7 @@ static const struct of_device_id caam_match[] = { MODULE_DEVICE_TABLE(of, caam_match); struct caam_imx_data { + bool page0_access; const struct clk_bulk_data *clks; int num_clks; }; @@ -524,6 +525,7 @@ static const struct clk_bulk_data caam_imx6_clks[] = { }; static const struct caam_imx_data caam_imx6_data = { + .page0_access = true, .clks = caam_imx6_clks, .num_clks = ARRAY_SIZE(caam_imx6_clks), }; @@ -534,6 +536,7 @@ static const struct clk_bulk_data caam_imx7_clks[] = { }; static const struct caam_imx_data caam_imx7_data = { + .page0_access = true, .clks = caam_imx7_clks, .num_clks = ARRAY_SIZE(caam_imx7_clks), }; @@ -545,6 +548,7 @@ static const struct clk_bulk_data caam_imx6ul_clks[] = { }; static const struct caam_imx_data caam_imx6ul_data = { + .page0_access = true, .clks = caam_imx6ul_clks, .num_clks = ARRAY_SIZE(caam_imx6ul_clks), }; @@ -554,15 +558,23 @@ static const struct clk_bulk_data caam_vf610_clks[] = { }; static const struct caam_imx_data caam_vf610_data = { + .page0_access = true, .clks = caam_vf610_clks, .num_clks = ARRAY_SIZE(caam_vf610_clks), }; +static const struct caam_imx_data caam_imx8ulp_data = { + .page0_access = false, + .clks = NULL, + .num_clks = 0, +}; + 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 }, { .soc_id = "i.MX8M*", .data = &caam_imx7_data }, + { .soc_id = "i.MX8ULP", .data = &caam_imx8ulp_data }, { .soc_id = "VF*", .data = &caam_vf610_data }, { .family = "Freescale i.MX" }, { /* sentinel */ } @@ -860,6 +872,7 @@ static int caam_probe(struct platform_device *pdev) int pg_size; int BLOCK_OFFSET = 0; bool reg_access = true; + const struct caam_imx_data *imx_soc_data; ctrlpriv = devm_kzalloc(&pdev->dev, sizeof(*ctrlpriv), GFP_KERNEL); if (!ctrlpriv) @@ -889,7 +902,15 @@ static int caam_probe(struct platform_device *pdev) reg_access = !ctrlpriv->optee_en; - if (!imx_soc_match->data) { + if (imx_soc_match->data) { + imx_soc_data = imx_soc_match->data; + reg_access = reg_access && imx_soc_data->page0_access; + /* + * CAAM clocks cannot be controlled from kernel. + */ + if (!imx_soc_data->num_clks) + goto iomap_ctrl; + } else { dev_err(dev, "No clock data provided for i.MX SoC"); return -EINVAL; } @@ -899,7 +920,7 @@ static int caam_probe(struct platform_device *pdev) return ret; } - +iomap_ctrl: /* Get configuration properties from device tree */ /* First, get register page */ ctrl = devm_of_iomap(dev, nprop, 0, NULL);
CAAM clock initialization to be done based on, soc specific info stored in struct caam_imx_data: - caam-page0-access flag - num_clks Signed-off-by: Pankaj Gupta <pankaj.gupta@nxp.com> --- v2: - Considering the OPTEE enablement check too, for setting the variable 'reg_access'. drivers/crypto/caam/ctrl.c | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-)