2024-04-29 06:31:37

by Pankaj Gupta

[permalink] [raw]
Subject: [PATCH v4 0/2] caam: init-clk based on caam-page0-access

v4:
- Correct the null pointer checking

v3:
- Splitting the patch into two.
- Disposed-off comments received on v2.

v2:
- Considering the OPTEE enablement check too, for setting the
variable 'reg_access'.

Pankaj Gupta (2):
caam: init-clk based on caam-page0-access
drivers: crypto: caam: i.MX8ULP donot have CAAM page0 access

drivers/crypto/caam/ctrl.c | 19 ++++++++++++++++++-
1 file changed, 18 insertions(+), 1 deletion(-)

--
2.34.1



2024-04-29 06:31:52

by Pankaj Gupta

[permalink] [raw]
Subject: [PATCH v4 0/2] caam: init-clk based on caam-page0-access

v4:
- Correct the null pointer checking

v3:
- Splitting the patch into two.
- Disposed-off comments received on v2.

v2:
- Considering the OPTEE enablement check too, for setting the
variable 'reg_access'.

Pankaj Gupta (2):
caam: init-clk based on caam-page0-access
drivers: crypto: caam: i.MX8ULP donot have CAAM page0 access

drivers/crypto/caam/ctrl.c | 19 ++++++++++++++++++-
1 file changed, 18 insertions(+), 1 deletion(-)

--
2.34.1


2024-04-29 06:32:07

by Pankaj Gupta

[permalink] [raw]
Subject: [PATCH v4 1/2] caam: init-clk based on caam-page0-access

CAAM clock initializat is done based on the basis of soc specific
info stored in struct caam_imx_data:
- caam-page0-access flag
- num_clks

CAAM driver needs to be aware of access rights to CAAM control page
i.e., page0, to do things differently.

Signed-off-by: Pankaj Gupta <[email protected]>
Reviewed-by: Gaurav Jain <[email protected]>
---
drivers/crypto/caam/ctrl.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
index bdf367f3f679..02363c467cf3 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,6 +558,7 @@ 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),
};
@@ -860,6 +865,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)
@@ -894,12 +900,20 @@ static int caam_probe(struct platform_device *pdev)
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;
+
ret = init_clocks(dev, imx_soc_match->data);
if (ret)
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


2024-04-29 06:32:39

by Pankaj Gupta

[permalink] [raw]
Subject: [PATCH v4 1/2] caam: init-clk based on caam-page0-access

CAAM clock initializat is done based on the basis of soc specific
info stored in struct caam_imx_data:
- caam-page0-access flag
- num_clks

CAAM driver needs to be aware of access rights to CAAM control page
i.e., page0, to do things differently.

Signed-off-by: Pankaj Gupta <[email protected]>
Reviewed-by: Gaurav Jain <[email protected]>
---
drivers/crypto/caam/ctrl.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
index bdf367f3f679..02363c467cf3 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,6 +558,7 @@ 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),
};
@@ -860,6 +865,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)
@@ -894,12 +900,20 @@ static int caam_probe(struct platform_device *pdev)
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;
+
ret = init_clocks(dev, imx_soc_match->data);
if (ret)
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


2024-04-29 06:32:56

by Pankaj Gupta

[permalink] [raw]
Subject: [PATCH v4 2/2] drivers: crypto: caam: i.MX8ULP donot have CAAM page0 access

iMX8ULP have a secure-enclave hardware IP called EdgeLock Enclave(ELE),
that control access to caam controller's register page, i.e., page0.

At all, if the ELE release access to CAAM controller's register page,
it will release to secure-world only.

Clocks are turned on automatically for iMX8ULP. There exists the caam
clock gating bit, but it is not advised to gate the clock at linux, as
optee-os or any other entity might be using it.

Signed-off-by: Pankaj Gupta <[email protected]>
Reviewed-by: Gaurav Jain <[email protected]>
Reviewed-by: Horia Geanta <[email protected]>
---
drivers/crypto/caam/ctrl.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
index 02363c467cf3..bd418dea586d 100644
--- a/drivers/crypto/caam/ctrl.c
+++ b/drivers/crypto/caam/ctrl.c
@@ -563,11 +563,14 @@ static const struct caam_imx_data caam_vf610_data = {
.num_clks = ARRAY_SIZE(caam_vf610_clks),
};

+static const struct caam_imx_data caam_imx8ulp_data;
+
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 */ }
--
2.34.1


2024-04-29 07:20:17

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v4 0/2] caam: init-clk based on caam-page0-access

On 29/04/2024 08:28, Pankaj Gupta wrote:
> v4:
> - Correct the null pointer checking
>
> v3:
> - Splitting the patch into two.
> - Disposed-off comments received on v2.
>
> v2:
> - Considering the OPTEE enablement check too, for setting the
> variable 'reg_access'.
>
> Pankaj Gupta (2):
> caam: init-clk based on caam-page0-access
> drivers: crypto: caam: i.MX8ULP donot have CAAM page0 access
>

Two patches here, 5 patches in patchset, some with entirely different
patch prefixes.

Bring some order to this.

Best regards,
Krzysztof