[PATCH] platform/chrome: cros_ec_typec: Check for EC driver

[PATCH] platform/chrome: cros_ec_typec: Check for EC driver

[Akihiko Odaki]

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;
+
 	typec->typec_cmd_supported = cros_ec_check_features(ec_dev, EC_FEATURE_TYPEC_CMD);
 	typec->needs_mux_ack = cros_ec_check_features(ec_dev, EC_FEATURE_TYPEC_MUX_REQUIRE_AP_ACK);
 
-- 
2.35.1

Re: [PATCH] platform/chrome: cros_ec_typec: Check for EC driver

[Guenter Roeck]

On Sun, Apr 3, 2022 at 9:11 PM Akihiko Odaki <akihiko.odaki@gmail.com> wrote:
>
> The EC driver may not be initialized when cros_typec_probe is called,
> particulary when CONFIG_CROS_EC_CHARDEV=m.
>

Does this cause a crash ? If so, do you have a crash log ?

Reason for asking is that I saw the following crash, and it would be
good to know if the problem is the same.

<1>[    6.651137] #PF: error_code(0x0002) - not-present page
<6>[    6.651139] PGD 0 P4D 0
<4>[    6.651143] Oops: 0002 [#1] PREEMPT SMP NOPTI
<4>[    6.651146] CPU: 0 PID: 1656 Comm: udevd Tainted: G     U
    5.4.163-17384-g99ca1c60d20c #1
<4>[    6.651147] Hardware name: HP Lantis/Lantis, BIOS
Google_Lantis.13606.204.0 05/26/2021
<4>[    6.651152] RIP: 0010:mutex_lock+0x2b/0x3c
...
<4>[    6.651174] Call Trace:
<4>[    6.651180]  cros_ec_cmd_xfer+0x22/0xc1
<4>[    6.651183]  cros_ec_cmd_xfer_status+0x19/0x59
<4>[    6.651185]  cros_ec_command+0x82/0xc3
<4>[    6.651189]  cros_typec_probe+0x8a/0x5a2 [cros_ec_typec]

> Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com>

Not sure if this is the best solution, but I don't know a better one.

Reviewed-by: Guenter Roeck <groeck@chromium.org>

> ---
>  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;
> +
>         typec->typec_cmd_supported = cros_ec_check_features(ec_dev, EC_FEATURE_TYPEC_CMD);
>         typec->needs_mux_ack = cros_ec_check_features(ec_dev, EC_FEATURE_TYPEC_MUX_REQUIRE_AP_ACK);
>
> --
> 2.35.1
>

Re: [PATCH] platform/chrome: cros_ec_typec: Check for EC driver

[Akihiko Odaki]

On 2022/04/05 10:57, Guenter Roeck wrote:
> On Sun, Apr 3, 2022 at 9:11 PM Akihiko Odaki <akihiko.odaki@gmail.com> wrote:
>>
>> The EC driver may not be initialized when cros_typec_probe is called,
>> particulary when CONFIG_CROS_EC_CHARDEV=m.
>>
> 
> Does this cause a crash ? If so, do you have a crash log ?

It indeed caused a crash but I don't have a log because I couldn't get a 
serial console. Adding dev_error calls revealed ec_dev in 
cros_typec_probe can be NULL if CONFIG_CROS_EC_CHARDEV=m.

> 
> Reason for asking is that I saw the following crash, and it would be
> good to know if the problem is the same.

This is just a guess, but I don't think it is unlikely.

The call trace indicates it dereferenced a NULL pointer in 
cros_ec_command, which is directly called by cros_typec_probe.

At the source level, cros_ec_command is directly called just once in 
cros_typec_probe, which dereferences typec->ec. typec->ec is however 
already used by cros_typec_get_cmd_version so it would never trigger a 
NULL dereference. Therefore, the crash should have happened in an 
inlined function.

cros_ec_command is called by cros_typec_get_cmd_version and 
cros_ec_check_features. cros_ec_check_features, which dereferenced NULL 
on my laptop, is called twice and unlikely to be inlined. 
cros_typec_get_cmd_version is called only once and is more likely to be 
inlined and thus the cause of the Oops in your crash. (By the way, the 
possibility of inlining would also make comparing call traces 
meaningless due to compiler/kernel version variances.)

This is just a guess anyway so you may disassemble your kernel build to 
know if it is same or not, which I think should be straightforward enough.

Regards,
Akihiko Odaki

> 
> <1>[    6.651137] #PF: error_code(0x0002) - not-present page
> <6>[    6.651139] PGD 0 P4D 0
> <4>[    6.651143] Oops: 0002 [#1] PREEMPT SMP NOPTI
> <4>[    6.651146] CPU: 0 PID: 1656 Comm: udevd Tainted: G     U
>      5.4.163-17384-g99ca1c60d20c #1
> <4>[    6.651147] Hardware name: HP Lantis/Lantis, BIOS
> Google_Lantis.13606.204.0 05/26/2021
> <4>[    6.651152] RIP: 0010:mutex_lock+0x2b/0x3c
> ...
> <4>[    6.651174] Call Trace:
> <4>[    6.651180]  cros_ec_cmd_xfer+0x22/0xc1
> <4>[    6.651183]  cros_ec_cmd_xfer_status+0x19/0x59
> <4>[    6.651185]  cros_ec_command+0x82/0xc3
> <4>[    6.651189]  cros_typec_probe+0x8a/0x5a2 [cros_ec_typec]
> 
>> Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com>
> 
> Not sure if this is the best solution, but I don't know a better one.
> 
> Reviewed-by: Guenter Roeck <groeck@chromium.org>
> 
>> ---
>>   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;
>> +
>>          typec->typec_cmd_supported = cros_ec_check_features(ec_dev, EC_FEATURE_TYPEC_CMD);
>>          typec->needs_mux_ack = cros_ec_check_features(ec_dev, EC_FEATURE_TYPEC_MUX_REQUIRE_AP_ACK);
>>
>> --
>> 2.35.1
>>

Re: [PATCH] platform/chrome: cros_ec_typec: Check for EC driver

[Prashant Malani]

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] ?

Thanks,

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/platform/chrome?id=ffebd90532728086007038986900426544e3df4e

Re: [PATCH] platform/chrome: cros_ec_typec: Check for EC driver

[Guenter Roeck]

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] ?
>

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.

Guenter

> Thanks,
>
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/platform/chrome?id=ffebd90532728086007038986900426544e3df4e

Re: [PATCH] platform/chrome: cros_ec_typec: Check for EC driver

[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

Re: [PATCH] platform/chrome: cros_ec_typec: Check for EC driver

[Guenter Roeck]

On Wed, Apr 6, 2022 at 6:16 PM Akihiko Odaki <akihiko.odaki@gmail.com> wrote:
[ ... ]
> >>>        ec_dev = dev_get_drvdata(&typec->ec->ec->dev);

I completely missed the part that this is not on the parent.

> >>> +     if (!ec_dev)
> >>> +             return -EPROBE_DEFER;
[ ... ]
>
> 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

I am actually surprised that typec->ec->ec is not NULL. Underlying
problem (or, one underlying problem) is that it is set in
cros_ec_register():

        /* Register a platform device for the main EC instance */
        ec_dev->ec = platform_device_register_data(ec_dev->dev, "cros-ec-dev",
                                        PLATFORM_DEVID_AUTO, &ec_p,
                                        sizeof(struct cros_ec_platform));

"cros-ec-dev" is the mfd device which instantiates the character
device. On devicetree (arm64) systems, the typec device is registered
as child of google,cros-ec-spi and thus should be instantiated only
after the spi device has been instantiated. The same should happen on
ACPI systems, but I don't know if that is really correct.

I don't know what exactly is happening, but apparently typec
registration happens in parallel with cros-ec-dev registration, which
is delayed because the character device is not loaded. As mentioned, I
don't understand why typec->ec->ec is not NULL. Can you check what it
points to ?

Thanks,
Guenter

> 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
>

Re: [PATCH] platform/chrome: cros_ec_typec: Check for EC driver

[Akihiko Odaki]

On 2022/04/08 1:28, Guenter Roeck wrote:
> On Wed, Apr 6, 2022 at 6:16 PM Akihiko Odaki <akihiko.odaki@gmail.com> wrote:
> [ ... ]
>>>>>         ec_dev = dev_get_drvdata(&typec->ec->ec->dev);
> 
> I completely missed the part that this is not on the parent.
> 
>>>>> +     if (!ec_dev)
>>>>> +             return -EPROBE_DEFER;
> [ ... ]
>>
>> 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
> 
> I am actually surprised that typec->ec->ec is not NULL. Underlying
> problem (or, one underlying problem) is that it is set in
> cros_ec_register():
> 
>          /* Register a platform device for the main EC instance */
>          ec_dev->ec = platform_device_register_data(ec_dev->dev, "cros-ec-dev",
>                                          PLATFORM_DEVID_AUTO, &ec_p,
>                                          sizeof(struct cros_ec_platform));
> 
> "cros-ec-dev" is the mfd device which instantiates the character
> device. On devicetree (arm64) systems, the typec device is registered
> as child of google,cros-ec-spi and thus should be instantiated only
> after the spi device has been instantiated. The same should happen on
> ACPI systems, but I don't know if that is really correct.
> 
> I don't know what exactly is happening, but apparently typec
> registration happens in parallel with cros-ec-dev registration, which
> is delayed because the character device is not loaded. As mentioned, I
> don't understand why typec->ec->ec is not NULL. Can you check what it
> points to ?

If I read the code correctly, the registration itself happens 
synchronously and platform_device_register_data() always returns a 
non-NULL value unless it returns -ENOMEM. The driver, however, can be 
asynchronously bound and dev_get_drvdata(&typec->ec->ec->dev) can return 
NULL as the consequence. It would have a call trace like the following 
when scheduling asynchronous driver binding:
platform_device_register_data()
platform_device_register_resndata()
platform_device_register_full()
-  This always creates and returns platform_device.
platform_device_add()
- This adds the created platform_device.
device_add()
bus_probe_device()
device_initial_probe()
__device_attach()
- This schedules asynchronous probing.

typec->ec->ec should be pointing to the correct platform_device as the 
patched driver works without Oops on my computer. It is not NULL at least.

Regards,
Akihiko Odaki

> 
> Thanks,
> Guenter
> 
>> 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
>>

Re: [PATCH] platform/chrome: cros_ec_typec: Check for EC driver

[Benson Leung]

[-- Attachment #1: Type: text/plain, Size: 1600 bytes --]

Hi Akihiko,

On Fri, Apr 08, 2022 at 02:03:52AM +0900, Akihiko Odaki wrote:
> If I read the code correctly, the registration itself happens synchronously
> and platform_device_register_data() always returns a non-NULL value unless
> it returns -ENOMEM. The driver, however, can be asynchronously bound and
> dev_get_drvdata(&typec->ec->ec->dev) can return NULL as the consequence. It
> would have a call trace like the following when scheduling asynchronous
> driver binding:
> platform_device_register_data()
> platform_device_register_resndata()
> platform_device_register_full()
> -  This always creates and returns platform_device.
> platform_device_add()
> - This adds the created platform_device.
> device_add()
> bus_probe_device()
> device_initial_probe()
> __device_attach()
> - This schedules asynchronous probing.
> 
> typec->ec->ec should be pointing to the correct platform_device as the
> patched driver works without Oops on my computer. It is not NULL at least.

Can you provide more information about your test computer in this case?

Is it a Chromebook running stock firmware (if so, please let us know which
model, and which firmware version it is running).
In the past, we've also gotten some reports from people running MrChromebox
custom firmware on older Chromebooks which have exposed other bugs in
this driver.

Let us know if that's the case here, and where we can get that firmware.

Thanks,
Benson

-- 
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
bleung@google.com
Chromium OS Project
bleung@chromium.org

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH] platform/chrome: cros_ec_typec: Check for EC driver

[Akihiko Odaki]

On 2022/04/08 2:09, Benson Leung wrote:
> Hi Akihiko,
> 
> On Fri, Apr 08, 2022 at 02:03:52AM +0900, Akihiko Odaki wrote:
>> If I read the code correctly, the registration itself happens synchronously
>> and platform_device_register_data() always returns a non-NULL value unless
>> it returns -ENOMEM. The driver, however, can be asynchronously bound and
>> dev_get_drvdata(&typec->ec->ec->dev) can return NULL as the consequence. It
>> would have a call trace like the following when scheduling asynchronous
>> driver binding:
>> platform_device_register_data()
>> platform_device_register_resndata()
>> platform_device_register_full()
>> -  This always creates and returns platform_device.
>> platform_device_add()
>> - This adds the created platform_device.
>> device_add()
>> bus_probe_device()
>> device_initial_probe()
>> __device_attach()
>> - This schedules asynchronous probing.
>>
>> typec->ec->ec should be pointing to the correct platform_device as the
>> patched driver works without Oops on my computer. It is not NULL at least.
> 
> Can you provide more information about your test computer in this case?
> 
> Is it a Chromebook running stock firmware (if so, please let us know which
> model, and which firmware version it is running).
> In the past, we've also gotten some reports from people running MrChromebox
> custom firmware on older Chromebooks which have exposed other bugs in
> this driver.
> 
> Let us know if that's the case here, and where we can get that firmware.

My computer is Lenovo ThinkPad C13 Yoga Chromebook. It is running the 
stock firmware. The firmware was updated by running the following 
command on Google Chrome OS Version 99.0.4844.86 (Official Build) (64-Bit):
chromeos-firmwareupdate --mode=recovery

Regards,
Akihiko Odaki

> 
> Thanks,
> Benson
> 

Re: [PATCH] platform/chrome: cros_ec_typec: Check for EC driver

[Guenter Roeck]

On Thu, Apr 7, 2022 at 10:09 AM Benson Leung <bleung@google.com> wrote:
>
> Hi Akihiko,
>
> On Fri, Apr 08, 2022 at 02:03:52AM +0900, Akihiko Odaki wrote:
> > If I read the code correctly, the registration itself happens synchronously
> > and platform_device_register_data() always returns a non-NULL value unless
> > it returns -ENOMEM. The driver, however, can be asynchronously bound and
> > dev_get_drvdata(&typec->ec->ec->dev) can return NULL as the consequence. It
> > would have a call trace like the following when scheduling asynchronous
> > driver binding:
> > platform_device_register_data()
> > platform_device_register_resndata()
> > platform_device_register_full()
> > -  This always creates and returns platform_device.
> > platform_device_add()
> > - This adds the created platform_device.
> > device_add()
> > bus_probe_device()
> > device_initial_probe()
> > __device_attach()
> > - This schedules asynchronous probing.
> >
> > typec->ec->ec should be pointing to the correct platform_device as the
> > patched driver works without Oops on my computer. It is not NULL at least.
>
> Can you provide more information about your test computer in this case?
>
> Is it a Chromebook running stock firmware (if so, please let us know which
> model, and which firmware version it is running).
> In the past, we've also gotten some reports from people running MrChromebox
> custom firmware on older Chromebooks which have exposed other bugs in
> this driver.
>

I think we should be able to reproduce the problem by configuring
CONFIG_CROS_EC_CHARDEV=m on any Chromebook supporting Type C.

Guenter

> Let us know if that's the case here, and where we can get that firmware.
>
> Thanks,
> Benson
>
> --
> Benson Leung
> Staff Software Engineer
> Chrome OS Kernel
> Google Inc.
> bleung@google.com
> Chromium OS Project
> bleung@chromium.org

Re: [PATCH] platform/chrome: cros_ec_typec: Check for EC driver

[patchwork-bot+chrome-platform]

Hello:

This patch was applied to chrome-platform/linux.git (for-kernelci)
by Tzung-Bi Shih <tzungbi@kernel.org>:

On Mon,  4 Apr 2022 13:11:01 +0900 you 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(+)

Here is the summary with links:
  - platform/chrome: cros_ec_typec: Check for EC driver
    https://git.kernel.org/chrome-platform/c/cce465a867bc

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH] platform/chrome: cros_ec_typec: Check for EC driver

[patchwork-bot+chrome-platform]

Hello:

This patch was applied to chrome-platform/linux.git (for-kernelci)
by Prashant Malani <pmalani@chromium.org>:

On Mon,  4 Apr 2022 13:11:01 +0900 you 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(+)

Here is the summary with links:
  - platform/chrome: cros_ec_typec: Check for EC driver
    https://git.kernel.org/chrome-platform/c/7464ff8bf2d7

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH] platform/chrome: cros_ec_typec: Check for EC driver

[patchwork-bot+chrome-platform]

Hello:

This patch was applied to chrome-platform/linux.git (for-next)
by Prashant Malani <pmalani@chromium.org>:

On Mon,  4 Apr 2022 13:11:01 +0900 you 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(+)

Here is the summary with links:
  - platform/chrome: cros_ec_typec: Check for EC driver
    https://git.kernel.org/chrome-platform/c/7464ff8bf2d7

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html