Alice Guo (OSS) wrote on Tue, Mar 30, 2021 at 02:41:23AM +0000:

Thanks for reporting this issue, I'll check and add a fix to handle defer probe.

I haven't seen any follow up on this, have you had a chance to take a
look?
If this won't make it for 5.12 (in a couple of week probably?) would it
make sense to revert 7d981405d0fd ("soc: imx8m: change to use platform
driver") for now?



While looking at the code earlier I also have an unrelated, late-review
on the patch itself:

+static u32 __init imx8mq_soc_revision(struct device *dev)
[...]
 @@ -191,8 +223,16 @@ static int __init imx8_soc_init(void)
        data = id->data;
        if (data) {
                soc_dev_attr->soc_id = data->name;
-               if (data->soc_revision)
-                       soc_rev = data->soc_revision();
+               if (data->soc_revision) {
+                       if (pdev) {
+                               soc_rev = data->soc_revision(&pdev->dev);
+                               ret = soc_rev;
+                               if (ret < 0)

I appreciate current soc_revision are "small enough" (looking at
include/soc/imx/revision.h we're talking < 256) so this actually works,
but would it make sense to either make soc_rev signed, or to have
soc_revision() return a s64, or have the revision filled in another *u32
argument to make sure the error is an error and not just a large rev?

This is most definitely fine for now but that kind of code patterns can
lead to weird errors down the road.

Thanks,
-- 
Dominique

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel