> (Was: [PATCH v4 4/4] soc: imx8m: change to use platform driver) > > 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? We are trying to find a proper solution for this. The proper method might be make soc_device_match return probe defer, and take early soc attr into consideration, but I am not sure this would win maintainer's vote. > 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? Please no. We are targeting android GKI, make driver as modules. And reverting to original method will also break kexec. I am on IRC #linux-imx, we could take more if you would like to. Thanks, Peng. > > > > 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