From: Alice Guo <[email protected]>
drivers/soc/imx/soc-imx8m.c is probed later than the caam driver so that
return -EPROBE_DEFER is needed after calling soc_device_match() in
drivers/crypto/caam/ctrl.c. For i.MX8M, soc_device_match returning NULL
can be considered that the SoC device has not been probed yet, so it
returns -EPROBE_DEFER directly.
Fixes: 7d981405d0fd ("soc: imx8m: change to use platform driver")
Signed-off-by: Alice Guo <[email protected]>
---
drivers/crypto/caam/ctrl.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
index ca0361b2dbb0..9bba3b93cf35 100644
--- a/drivers/crypto/caam/ctrl.c
+++ b/drivers/crypto/caam/ctrl.c
@@ -79,6 +79,14 @@ static void build_deinstantiation_desc(u32 *desc, int handle)
append_jump(desc, JUMP_CLASS_CLASS1 | JUMP_TYPE_HALT);
}
+static const struct of_device_id imx8m_machine_match[] = {
+ { .compatible = "fsl,imx8mm", },
+ { .compatible = "fsl,imx8mn", },
+ { .compatible = "fsl,imx8mp", },
+ { .compatible = "fsl,imx8mq", },
+ { }
+};
+
/*
* run_descriptor_deco0 - runs a descriptor on DECO0, under direct control of
* the software (no JR/QI used).
@@ -635,6 +643,8 @@ static int caam_probe(struct platform_device *pdev)
nprop = pdev->dev.of_node;
imx_soc_match = soc_device_match(caam_imx_soc_table);
+ if (!imx_soc_match && of_match_node(imx8m_machine_match, of_root))
+ return -EPROBE_DEFER;
caam_imx = (bool)imx_soc_match;
if (imx_soc_match) {
--
2.17.1
From: Alice Guo <[email protected]>
Patch "fix the built-in caam driver cannot match soc_id" provides
imx8m_machine_match to match i.MX8M{Q,M,N,P}, so change to use to
of_match_node which can simplify the code.
Signed-off-by: Alice Guo <[email protected]>
---
drivers/crypto/caam/ctrl.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
index 9bba3b93cf35..7e6a525e0c11 100644
--- a/drivers/crypto/caam/ctrl.c
+++ b/drivers/crypto/caam/ctrl.c
@@ -113,10 +113,7 @@ static inline int run_descriptor_deco0(struct device *ctrldev, u32 *desc,
* Apparently on i.MX8M{Q,M,N,P} it doesn't matter if virt_en == 1
* and the following steps should be performed regardless
*/
- of_machine_is_compatible("fsl,imx8mq") ||
- of_machine_is_compatible("fsl,imx8mm") ||
- of_machine_is_compatible("fsl,imx8mn") ||
- of_machine_is_compatible("fsl,imx8mp")) {
+ of_match_node(imx8m_machine_match, of_root)) {
clrsetbits_32(&ctrl->deco_rsr, 0, DECORSR_JR0);
while (!(rd_reg32(&ctrl->deco_rsr) & DECORSR_VALID) &&
--
2.17.1
> -----Original Message-----
> From: Fabio Estevam <[email protected]>
> Sent: 2021年4月29日 22:05
> Subject: Re: [PATCH v1 2/2] caam: imx8m: change to use of_match_node in
> run_descriptor_deco0
>
> Hi Alice
>
> On Thu, Apr 29, 2021 at 11:02 AM Alice Guo (OSS) <[email protected]>
> wrote:
> >
> > From: Alice Guo <[email protected]>
> >
> > Patch "fix the built-in caam driver cannot match soc_id" provides
> > imx8m_machine_match to match i.MX8M{Q,M,N,P}, so change to use to
> > of_match_node which can simplify the code.
>
> Shouldn't these patches be squashed?
These patches should not be squashed because "fix the built-in caam driver cannot match soc_id"
is mainly used to provide defer probe when soc device has not been probed yet, and is only for
i.MX8M. "change to use of_match_node in run_descriptor_deco0" is just to simplify code.
Best Regards,
Alice Guo
Alice Guo (OSS) wrote on Thu, Apr 29, 2021 at 10:02:49PM +0800:
> From: Alice Guo <[email protected]>
>
> drivers/soc/imx/soc-imx8m.c is probed later than the caam driver so that
> return -EPROBE_DEFER is needed after calling soc_device_match() in
> drivers/crypto/caam/ctrl.c. For i.MX8M, soc_device_match returning NULL
> can be considered that the SoC device has not been probed yet, so it
> returns -EPROBE_DEFER directly.
So basically you're saying if the soc is imx8m then soc_device_match()
has to find a match -- if for some reason there is rightfully no match
the caam driver will forever loop on EPROBE_DEFER (not sure how that is
handled by the driver stack?); but in this particular case we don't
actually need soc_device_match() to work: it's just there to pick the
appropriate clock data from caam_imx_soc_table[], and we already know we
should use &caam_imx7_data if imx8m_machine_match got a hit.
If we're going this way (making the caam driver only handle soc init
being late as that was noticeable), then I'd tend to agree with arnd's
comment[1] and not rely on soc_device_match at all in this case -- just
keeping it as a fallback if direct of_match_node didn't work for
compabitility with other devices.
[1] https://lore.kernel.org/r/CAK8P3a1GjeHyMCworQYVtp5U0uu2B9VBHmf9y0hGn-o8aKSJZw@mail.gmail.com/
Note I haven't had time to play with device_link_add or other ways to
make the soc init successfully early, but it's probably better to not
wait for me on this so I'm quite happy with this for now.
> Fixes: 7d981405d0fd ("soc: imx8m: change to use platform driver")
> Signed-off-by: Alice Guo <[email protected]>
And philosophical questions aside, this works for me:
Tested-by: Dominique Martinet <[email protected]>
--
Dominique
On 29.04.21 16:02, Alice Guo (OSS) wrote:
> From: Alice Guo <[email protected]>
>
> drivers/soc/imx/soc-imx8m.c is probed later than the caam driver so that
> return -EPROBE_DEFER is needed after calling soc_device_match() in
> drivers/crypto/caam/ctrl.c. For i.MX8M, soc_device_match returning NULL
> can be considered that the SoC device has not been probed yet, so it
> returns -EPROBE_DEFER directly.
>
> Fixes: 7d981405d0fd ("soc: imx8m: change to use platform driver")
> Signed-off-by: Alice Guo <[email protected]>
I didn't look at the details of this at all, but I can report that this
series fixes the bug below for me when booting v5.12.1 with built-in caam.
Tested-by: Frieder Schrempf <[email protected]>
[ 1.425534] caam 30900000.crypto: Entropy delay = 3200
[ 1.430781] caam 30900000.crypto: Entropy delay = 3600
[ 1.503610] caam 30900000.crypto: Instantiated RNG4 SH0
[ 1.571266] caam 30900000.crypto: Instantiated RNG4 SH1
[ 1.576511] caam 30900000.crypto: device ID = 0x0a16040100000000 (Era 9)
[ 1.583229] caam 30900000.crypto: job rings = 3, qi = 0
[ 1.594172] caam algorithms registered in /proc/crypto
[ 1.600308] caam 30900000.crypto: caam pkc algorithms registered in
/proc/crypto
[ 1.607777] caam 30900000.crypto: registering rng-caam
[ 1.612966] caam_jr 30901000.jr: job ring error: irqstate: 00000103
[ 1.619316] ------------[ cut here ]------------
[ 1.623936] kernel BUG at drivers/crypto/caam/jr.c:187!
[ 1.629171] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
[ 1.634665] Modules linked in:
[ 1.637730] CPU: 0 PID: 0 Comm: swapper/0 Not tainted
5.12.1-ktn+g807a88195d76 #1
[ 1.645223] Hardware name: Kontron i.MX8MM N801X S LVDS (DT)
[ 1.650888] pstate: 60000085 (nZCv daIf -PAN -UAO -TCO BTYPE=--)
[ 1.656904] pc : caam_jr_interrupt+0x180/0x188
[ 1.661363] lr : caam_jr_interrupt+0x180/0x188
[ 1.665815] sp : ffff800010003e90
[ 1.669134] x29: ffff800010003e90 x28: ffff800011362f40
[ 1.674459] x27: ffff800011362f40 x26: ffff800010faf3b0
[ 1.679783] x25: ffff800011460f42 x24: ffff0000023be600
[ 1.685107] x23: 000000000000003e x22: ffff800010003f24
[ 1.690431] x21: 0000000000000000 x20: ffff0000023be600
[ 1.695754] x19: ffff000002397a80 x18: 0000000000000010
[ 1.701077] x17: 0000000000000000 x16: 000000005e86dba6
[ 1.706400] x15: ffff800011363390 x14: 00000000000000e2
[ 1.711727] x13: ffff800011363390 x12: 00000000ffffffea
[ 1.717051] x11: ffff8000113e2048 x10: ffff8000113ca008
[ 1.722376] x9 : ffff8000113ca060 x8 : 0000000000017fe8
[ 1.727701] x7 : c0000000ffffefff x6 : 0000000000000001
[ 1.733023] x5 : 0000000000000000 x4 : 0000000000000000
[ 1.738345] x3 : 00000000ffffffff x2 : f0868b59cca25e00
[ 1.743667] x1 : f0868b59cca25e00 x0 : 0000000000000000
[ 1.748991] Call trace:
[ 1.751443] caam_jr_interrupt+0x180/0x188
[ 1.755547] __handle_irq_event_percpu+0x54/0x170
[ 1.760262] handle_irq_event_percpu+0x34/0x90
[ 1.764712] handle_irq_event+0x48/0xe0
[ 1.768555] handle_fasteoi_irq+0xb8/0x170
[ 1.772661] generic_handle_irq+0x30/0x48
[ 1.776680] __handle_domain_irq+0x64/0xc0
[ 1.780786] gic_handle_irq+0x58/0x128
[ 1.784545] el1_irq+0xb4/0x180
[ 1.787693] arch_cpu_idle+0x18/0x28
[ 1.791276] default_idle_call+0x20/0x68
[ 1.795209] do_idle+0x218/0x268
[ 1.798444] cpu_startup_entry+0x28/0x68
[ 1.802373] rest_init+0xd8/0xe8
[ 1.805606] arch_call_rest_init+0x10/0x1c
[ 1.809716] start_kernel+0x500/0x538
[ 1.813388] 0x0
[ 1.815239] Code: aa0103e0 90002ec1 91388021 94077cf5 (d4210000)
[ 1.821353] ---[ end trace c369bd5cc770522f ]---
[ 1.825979] Kernel panic - not syncing: Oops - BUG: Fatal exception
in interrupt
> ---
> drivers/crypto/caam/ctrl.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
> index ca0361b2dbb0..9bba3b93cf35 100644
> --- a/drivers/crypto/caam/ctrl.c
> +++ b/drivers/crypto/caam/ctrl.c
> @@ -79,6 +79,14 @@ static void build_deinstantiation_desc(u32 *desc, int handle)
> append_jump(desc, JUMP_CLASS_CLASS1 | JUMP_TYPE_HALT);
> }
>
> +static const struct of_device_id imx8m_machine_match[] = {
> + { .compatible = "fsl,imx8mm", },
> + { .compatible = "fsl,imx8mn", },
> + { .compatible = "fsl,imx8mp", },
> + { .compatible = "fsl,imx8mq", },
> + { }
> +};
> +
> /*
> * run_descriptor_deco0 - runs a descriptor on DECO0, under direct control of
> * the software (no JR/QI used).
> @@ -635,6 +643,8 @@ static int caam_probe(struct platform_device *pdev)
> nprop = pdev->dev.of_node;
>
> imx_soc_match = soc_device_match(caam_imx_soc_table);
> + if (!imx_soc_match && of_match_node(imx8m_machine_match, of_root))
> + return -EPROBE_DEFER;
> caam_imx = (bool)imx_soc_match;
>
> if (imx_soc_match) {
> --
> 2.17.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> https://eur04.safelinks.protection.outlook.com/?url=http%3A%2F%2Flists.infradead.org%2Fmailman%2Flistinfo%2Flinux-arm-kernel&data=04%7C01%7Cfrieder.schrempf%40kontron.de%7Ceab62a59cfaf45dd5acd08d90b17ad0c%7C8c9d3c973fd941c8a2b1646f3942daf1%7C0%7C0%7C637553018576335889%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=kHLtIGIsyueosiAj0h9EurdZYzR3rk3fIuY74IhXySw%3D&reserved=0
>