Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
MIME-Version: 1.0
References: <20220404041101.6276-1-akihiko.odaki@gmail.com>
 <Yk4DGZfpYbK8dofL@chromium.org> <CABXOdTcY3w56hc7kWsDLxKU-c6fCLYt_jigK13tKjjm9OHi2+w@mail.gmail.com>
 <033c1ec4-4bee-a689-140c-9694dfee435b@gmail.com>
In-Reply-To: <033c1ec4-4bee-a689-140c-9694dfee435b@gmail.com>
From:   Guenter Roeck <groeck@google.com>
Date:   Thu, 7 Apr 2022 09:28:41 -0700
Message-ID: <CABXOdTet5ynSXf94qMimobJF4LLzHc89cVbwJ5NuAz8G6jmVdQ@mail.gmail.com>
Subject: Re: [PATCH] platform/chrome: cros_ec_typec: Check for EC driver
To:     Akihiko Odaki <akihiko.odaki@gmail.com>
Cc:     Prashant Malani <pmalani@chromium.org>,
        linux-kernel <linux-kernel@vger.kernel.org>,
        chrome-platform@lists.linux.dev,
        Benson Leung <bleung@chromium.org>,
        Guenter Roeck <groeck@chromium.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

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
>