Messages in this thread | | | Date | Thu, 7 Apr 2022 10:16:34 +0900 | Subject | Re: [PATCH] platform/chrome: cros_ec_typec: Check for EC driver | From | Akihiko Odaki <> |
| |
On 2022/04/07 6:32, Guenter Roeck wrote: > On Wed, Apr 6, 2022 at 2:16 PM Prashant Malani <pmalani@chromium.org> wrote: >> >> Hi Akihiko, >> >> Thanks for the patch. >> >> On Apr 04 13:11, Akihiko Odaki wrote: >>> The EC driver may not be initialized when cros_typec_probe is called, >>> particulary when CONFIG_CROS_EC_CHARDEV=m. >>> >>> Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com> >>> --- >>> drivers/platform/chrome/cros_ec_typec.c | 3 +++ >>> 1 file changed, 3 insertions(+) >>> >>> diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c >>> index 4bd2752c0823..7cb2e35c4ded 100644 >>> --- a/drivers/platform/chrome/cros_ec_typec.c >>> +++ b/drivers/platform/chrome/cros_ec_typec.c >>> @@ -1084,6 +1084,9 @@ static int cros_typec_probe(struct platform_device *pdev) >>> } >>> >>> ec_dev = dev_get_drvdata(&typec->ec->ec->dev); >>> + if (!ec_dev) >>> + return -EPROBE_DEFER; >>> + >> >> Just a quick check: are you still seeing this issue with 5.18-rc1, which >> contains a null check for the parent EC device [1] ?
Yes, I'm seeing this problem with the check.
>> > > I may be missing something, but from the context I suspect this may > make the problem worse. My understanding was that the problem was seen > specifically if CONFIG_CROS_EC_CHARDEV=m. In that situation, it > appears that the parent EC device does _not yet_ exist. If the driver > returns -ENODEV in that situation, it will never be instantiated. The > big question for me is why the type C device is instantiated in the > first place if the parent EC device does not [yet] exist. I have not > been able to identify the code path where this happens. > > There is a similar problem with other EC child devices which are also > sometimes instantiated even though the parent EC device does not exist > (ie dev_get_drvdata(pdev->dev.parent) returns NULL). That can happen > if the parent EC device instantiation fails because of EC > communication errors (see https://b.corp.google.com/issues/228118385 > for examples [sorry, internal only, I can't make it public]). I think > we need to track down why that happens and prevent child devices from > being instantiated in the first place instead of trying to work around > the problem in the child drivers.
Well, I think you have two misunderstanding.
1. The parent exists and dev_get_drvdata(pdev->dev.parent) returns non-NULL value. However, dev_get_drvdata(&typec->ec->ec->dev) returns NULL. (Yes, that is confusing.) I'm wondering dev_get_drvdata(pdev->dev.parent) returned NULL in the following crash log but it would be a problem distinct from what is handled with my patch: https://lore.kernel.org/lkml/CABXOdTe9u_DW=NZM1-J120Gu1gibDy8SsgHP3bJwwLsE_iuLAQ@mail.gmail.com/
2. My patch returns -EPROBE_DEFER instead of -ENODEV and I confirmed it will eventually be instantiated.
Regards, Akihiko Odaki
> > Guenter > >> Thanks, >> >> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/platform/chrome?id=ffebd90532728086007038986900426544e3df4e
| |