| Message ID | 20230710053418.2406915-1-pankaj.gupta@nxp.com |
|---|---|
| State | New |
| Headers | show |
| Series | caam: init-clk based on caam-page0-access | expand |
On 7/10/2023 8:34 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 > Any specific reason for deviating from downstream implementation (based on DT)? https://github.com/nxp-imx/linux-imx/blob/lf-6.1.1-1.0.1/drivers/crypto/caam/ctrl.c#L911 Thanks, Horia
> -----Original Message----- > From: Horia Geanta <horia.geanta@nxp.com> > Sent: Monday, July 17, 2023 10:39 PM > To: Pankaj Gupta <pankaj.gupta@nxp.com>; Gaurav Jain > <gaurav.jain@nxp.com>; Varun Sethi <V.Sethi@nxp.com>; Herbert Xu > <herbert@gondor.apana.org.au>; David S . Miller <davem@davemloft.net>; > Iuliana Prodan <iuliana.prodan@nxp.com> > Cc: linux-crypto@vger.kernel.org; linux-kernel@vger.kernel.org; dl-linux-imx > <linux-imx@nxp.com> > Subject: Re: [PATCH] caam: init-clk based on caam-page0-access > > On 7/10/2023 8:34 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 > > > Any specific reason for deviating from downstream implementation (based on > DT)? Implementing based on DT will lead to multiple DT entries in the same code section: - one entry for imx8ulp(fsl,imx8ulp-ele) - then for imx8dxl(fsl,imx-scu) - then for imx93 (fsl,imx93-ele) - Similar entries for future SoC as well. Hence, followed this approach. > https://github.com/nxp-imx/linux-imx/blob/lf-6.1.1- > 1.0.1/drivers/crypto/caam/ctrl.c#L911 > > Thanks, > Horia
Hi Horia/Gaurav, Are there any further comments? Please share. Regards Pankaj > -----Original Message----- > From: Pankaj Gupta > Sent: Thursday, July 27, 2023 1:14 PM > To: Horia Geanta <horia.geanta@nxp.com>; Gaurav Jain > <gaurav.jain@nxp.com>; Varun Sethi <V.Sethi@nxp.com>; Herbert Xu > <herbert@gondor.apana.org.au>; David S . Miller <davem@davemloft.net>; > Iuliana Prodan <iuliana.prodan@nxp.com> > Cc: linux-crypto@vger.kernel.org; linux-kernel@vger.kernel.org; dl-linux-imx > <linux-imx@nxp.com> > Subject: RE: [PATCH] caam: init-clk based on caam-page0-access > > > > > -----Original Message----- > > From: Horia Geanta <horia.geanta@nxp.com> > > Sent: Monday, July 17, 2023 10:39 PM > > To: Pankaj Gupta <pankaj.gupta@nxp.com>; Gaurav Jain > > <gaurav.jain@nxp.com>; Varun Sethi <V.Sethi@nxp.com>; Herbert Xu > > <herbert@gondor.apana.org.au>; David S . Miller <davem@davemloft.net>; > > Iuliana Prodan <iuliana.prodan@nxp.com> > > Cc: linux-crypto@vger.kernel.org; linux-kernel@vger.kernel.org; > > dl-linux-imx <linux-imx@nxp.com> > > Subject: Re: [PATCH] caam: init-clk based on caam-page0-access > > > > On 7/10/2023 8:34 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 > > > > > Any specific reason for deviating from downstream implementation > > (based on DT)? > > Implementing based on DT will lead to multiple DT entries in the same code > section: > - one entry for imx8ulp(fsl,imx8ulp-ele) > - then for imx8dxl(fsl,imx-scu) > - then for imx93 (fsl,imx93-ele) > - Similar entries for future SoC as well. > > Hence, followed this approach. > > > https://github.com/nxp-imx/linux-imx/blob/lf-6.1.1- > > 1.0.1/drivers/crypto/caam/ctrl.c#L911 > > > > Thanks, > > Horia
Hello, On Mon, Jul 10, 2023 at 11:04:18AM +0530, 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 > > Signed-off-by: Pankaj Gupta <pankaj.gupta@nxp.com> > --- > drivers/crypto/caam/ctrl.c | 26 ++++++++++++++++++++++++-- > 1 file changed, 24 insertions(+), 2 deletions(-) > > diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c > index ff9ddbbca377..74d0b7541d54 100644 > --- a/drivers/crypto/caam/ctrl.c > +++ b/drivers/crypto/caam/ctrl.c [...] > 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 }, This change seems unrelated to the change described in the commit message. Should it be a separate patch? Francesco
Hi Pankaj Please check the below comments. Regards Gaurav Jain > -----Original Message----- > From: Pankaj Gupta <pankaj.gupta@nxp.com> > Sent: Monday, July 10, 2023 11:03 AM > To: Horia Geanta <horia.geanta@nxp.com>; Gaurav Jain > <gaurav.jain@nxp.com> > Cc: Pankaj Gupta <pankaj.gupta@nxp.com> > Subject: [PATCH] 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> > --- > drivers/crypto/caam/ctrl.c | 26 ++++++++++++++++++++++++-- > 1 file changed, 24 insertions(+), 2 deletions(-) > > diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c > index > ff9ddbbca377..74d0b7541d54 100644 > --- a/drivers/crypto/caam/ctrl.c > +++ b/drivers/crypto/caam/ctrl.c > @@ -511,6 +511,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; > }; > @@ -523,6 +524,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), }; @@ -533,6 +535,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), }; @@ -544,6 +547,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), }; @@ -553,15 +557,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 */ } > @@ -756,6 +768,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) > @@ -772,6 +785,15 @@ static int caam_probe(struct platform_device *pdev) > caam_imx = (bool)imx_soc_match; > > if (imx_soc_match) { > + if (imx_soc_match->data) { > + imx_soc_data = imx_soc_match->data; > + reg_access = imx_soc_data->page0_access; > + /* > + * CAAM clocks cannot be controlled from kernel. > + */ > + if (!imx_soc_data->num_clks) > + goto iomap_ctrl; OPTEE enablement check is ignored because of this goto statement. Regards Gaurav Jain > + } > /* > * Until Layerscape and i.MX OP-TEE get in sync, > * only i.MX OP-TEE use cases disallow access to @@ -781,7 > +803,7 @@ static int caam_probe(struct platform_device *pdev) > ctrlpriv->optee_en = !!np; > of_node_put(np); > > - reg_access = !ctrlpriv->optee_en; > + reg_access = reg_access && !ctrlpriv->optee_en; > > if (!imx_soc_match->data) { > dev_err(dev, "No clock data provided for i.MX SoC"); @@ -793,7 > +815,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
> -----Original Message----- > From: Gaurav Jain <gaurav.jain@nxp.com> > Sent: Wednesday, April 10, 2024 12:29 PM > To: Pankaj Gupta <pankaj.gupta@nxp.com>; Horia Geanta > <horia.geanta@nxp.com>; Varun Sethi <V.Sethi@nxp.com>; 'Herbert Xu' > <herbert@gondor.apana.org.au>; 'David S . Miller' <davem@davemloft.net>; > Iuliana Prodan <iuliana.prodan@nxp.com> > Cc: 'linux-crypto@vger.kernel.org' <linux-crypto@vger.kernel.org>; 'linux- > kernel@vger.kernel.org' <linux-kernel@vger.kernel.org>; dl-linux-imx <linux- > imx@nxp.com> > Subject: RE: [PATCH] caam: init-clk based on caam-page0-access > > Hi Pankaj > > Please check the below comments. > > Regards > Gaurav Jain > > > -----Original Message----- > > From: Pankaj Gupta <pankaj.gupta@nxp.com> > > Sent: Monday, July 10, 2023 11:03 AM > > To: Horia Geanta <horia.geanta@nxp.com>; Gaurav Jain > > <gaurav.jain@nxp.com> > > Cc: Pankaj Gupta <pankaj.gupta@nxp.com> > > Subject: [PATCH] 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> > > --- > > drivers/crypto/caam/ctrl.c | 26 ++++++++++++++++++++++++-- > > 1 file changed, 24 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c > > index > > ff9ddbbca377..74d0b7541d54 100644 > > --- a/drivers/crypto/caam/ctrl.c > > +++ b/drivers/crypto/caam/ctrl.c > > @@ -511,6 +511,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; > > }; > > @@ -523,6 +524,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), }; @@ -533,6 +535,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), }; @@ -544,6 +547,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), }; @@ -553,15 +557,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 */ } > > @@ -756,6 +768,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) > > @@ -772,6 +785,15 @@ static int caam_probe(struct platform_device > *pdev) > > caam_imx = (bool)imx_soc_match; > > > > if (imx_soc_match) { > > + if (imx_soc_match->data) { > > + imx_soc_data = imx_soc_match->data; > > + reg_access = imx_soc_data->page0_access; > > + /* > > + * CAAM clocks cannot be controlled from kernel. > > + */ > > + if (!imx_soc_data->num_clks) > > + goto iomap_ctrl; > > OPTEE enablement check is ignored because of this goto statement. > Regards > Gaurav Jain [Accepted] Will be corrected in V2. > > > + } > > /* > > * Until Layerscape and i.MX OP-TEE get in sync, > > * only i.MX OP-TEE use cases disallow access to @@ -781,7 > > +803,7 @@ static int caam_probe(struct platform_device *pdev) > > ctrlpriv->optee_en = !!np; > > of_node_put(np); > > > > - reg_access = !ctrlpriv->optee_en; > > + reg_access = reg_access && !ctrlpriv->optee_en; > > > > if (!imx_soc_match->data) { > > dev_err(dev, "No clock data provided for i.MX SoC"); > @@ -793,7 > > +815,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
diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c index ff9ddbbca377..74d0b7541d54 100644 --- a/drivers/crypto/caam/ctrl.c +++ b/drivers/crypto/caam/ctrl.c @@ -511,6 +511,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; }; @@ -523,6 +524,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), }; @@ -533,6 +535,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), }; @@ -544,6 +547,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), }; @@ -553,15 +557,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 */ } @@ -756,6 +768,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) @@ -772,6 +785,15 @@ static int caam_probe(struct platform_device *pdev) caam_imx = (bool)imx_soc_match; if (imx_soc_match) { + if (imx_soc_match->data) { + imx_soc_data = imx_soc_match->data; + reg_access = imx_soc_data->page0_access; + /* + * CAAM clocks cannot be controlled from kernel. + */ + if (!imx_soc_data->num_clks) + goto iomap_ctrl; + } /* * Until Layerscape and i.MX OP-TEE get in sync, * only i.MX OP-TEE use cases disallow access to @@ -781,7 +803,7 @@ static int caam_probe(struct platform_device *pdev) ctrlpriv->optee_en = !!np; of_node_put(np); - reg_access = !ctrlpriv->optee_en; + reg_access = reg_access && !ctrlpriv->optee_en; if (!imx_soc_match->data) { dev_err(dev, "No clock data provided for i.MX SoC"); @@ -793,7 +815,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> --- drivers/crypto/caam/ctrl.c | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-)