Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <033c1ec4-4bee-a689-140c-9694dfee435b@gmail.com>
Date:   Thu, 7 Apr 2022 10:16:34 +0900
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux aarch64; rv:91.0) Gecko/20100101
 Thunderbird/91.7.0
Subject: Re: [PATCH] platform/chrome: cros_ec_typec: Check for EC driver
Content-Language: en-US
To:     Guenter Roeck <groeck@google.com>,
        Prashant Malani <pmalani@chromium.org>
Cc:     linux-kernel <linux-kernel@vger.kernel.org>,
        chrome-platform@lists.linux.dev,
        Benson Leung <bleung@chromium.org>,
        Guenter Roeck <groeck@chromium.org>
References: <20220404041101.6276-1-akihiko.odaki@gmail.com>
 <Yk4DGZfpYbK8dofL@chromium.org>
 <CABXOdTcY3w56hc7kWsDLxKU-c6fCLYt_jigK13tKjjm9OHi2+w@mail.gmail.com>
From:   Akihiko Odaki <akihiko.odaki@gmail.com>
In-Reply-To: <CABXOdTcY3w56hc7kWsDLxKU-c6fCLYt_jigK13tKjjm9OHi2+w@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

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