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 <[email protected]>
---
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
Reviewed-by: Gaurav Jain <[email protected]>
> -----Original Message-----
> From: Pankaj Gupta <[email protected]>
> Sent: Wednesday, April 10, 2024 1:28 PM
> To: Gaurav Jain <[email protected]>; Horia Geanta <[email protected]>;
> Varun Sethi <[email protected]>; [email protected];
> [email protected]; Iuliana Prodan <[email protected]>; linux-
> [email protected]; [email protected]; dl-linux-imx <linux-
> [email protected]>
> Cc: Pankaj Gupta <[email protected]>
> 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 <[email protected]>
> ---
> 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 <[email protected]>
> Sent: Friday, April 12, 2024 6:20 PM
> To: Pankaj Gupta <[email protected]>; Gaurav Jain
> <[email protected]>; Varun Sethi <[email protected]>;
> [email protected]; [email protected]; Iuliana Prodan
> <[email protected]>; [email protected]; linux-
> [email protected]; dl-linux-imx <[email protected]>; Francesco Dolcini
> <[email protected]>
> 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