The Type C ACPI device on older Chromebooks is not generated correctly
(since their EC firmware doesn't support the new commands required). In
such cases, the crafted ACPI device doesn't have an EC parent, and it is
therefore not useful (it shouldn't be generated in the first place since
the EC firmware doesn't support any of the Type C commands).
To handle devices which use these older firmware revisions, check for
the parent EC device handle, and fail the probe if it's not found.
Fixes: fdc6b21e2444 ("platform/chrome: Add Type C connector class driver")
Reported-by: Alyssa Ross <[email protected]>
Signed-off-by: Prashant Malani <[email protected]>
---
Alyssa, could you kindly test this with your existing setup? Thanks!
drivers/platform/chrome/cros_ec_typec.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
index 5de0bfb0bc4d..7188f1d72f68 100644
--- a/drivers/platform/chrome/cros_ec_typec.c
+++ b/drivers/platform/chrome/cros_ec_typec.c
@@ -1076,6 +1076,12 @@ static int cros_typec_probe(struct platform_device *pdev)
typec->dev = dev;
typec->ec = dev_get_drvdata(pdev->dev.parent);
+
+ if (!typec->ec) {
+ dev_err(dev, "couldn't find parent EC device\n");
+ return -ENODEV;
+ }
+
platform_set_drvdata(pdev, typec);
ret = cros_typec_get_cmd_version(typec);
--
2.35.0.rc0.227.g00780c9af4-goog
On Wed, Jan 26, 2022 at 01:22:03AM +0000, Prashant Malani wrote:
> Fixes: fdc6b21e2444 ("platform/chrome: Add Type C connector class driver")
> Reported-by: Alyssa Ross <[email protected]>
> Signed-off-by: Prashant Malani <[email protected]>
With a minor comment,
Reviewed-by: Tzung-Bi Shih <[email protected]>
> @@ -1076,6 +1076,12 @@ static int cros_typec_probe(struct platform_device *pdev)
>
> typec->dev = dev;
> typec->ec = dev_get_drvdata(pdev->dev.parent);
> +
I would prefer to remove the blank line to make it look like an integrated block.
> + if (!typec->ec) {
> + dev_err(dev, "couldn't find parent EC device\n");
> + return -ENODEV;
> + }
> +
Hi Tzung-Bi,
Thanks for your review.
On Tue, Jan 25, 2022 at 7:46 PM Tzung-Bi Shih <[email protected]> wrote:
>
> On Wed, Jan 26, 2022 at 01:22:03AM +0000, Prashant Malani wrote:
> > Fixes: fdc6b21e2444 ("platform/chrome: Add Type C connector class driver")
> > Reported-by: Alyssa Ross <[email protected]>
> > Signed-off-by: Prashant Malani <[email protected]>
>
> With a minor comment,
> Reviewed-by: Tzung-Bi Shih <[email protected]>
>
> > @@ -1076,6 +1076,12 @@ static int cros_typec_probe(struct platform_device *pdev)
> >
> > typec->dev = dev;
> > typec->ec = dev_get_drvdata(pdev->dev.parent);
> > +
>
> I would prefer to remove the blank line to make it look like an integrated block.
I actually prefer it as it is. typec->dev is not really part of this
"integrated block", and I don't want to add another space there.
In any case, since this is a very minor style nit, I will address it
in case there is another version required due to other comments.
>
> > + if (!typec->ec) {
> > + dev_err(dev, "couldn't find parent EC device\n");
> > + return -ENODEV;
> > + }
> > +
Best,
-Prashant
On Tue, Jan 25, 2022 at 8:05 PM Prashant Malani <[email protected]> wrote:
>
> Hi Tzung-Bi,
>
> Thanks for your review.
>
> On Tue, Jan 25, 2022 at 7:46 PM Tzung-Bi Shih <[email protected]> wrote:
> >
> > On Wed, Jan 26, 2022 at 01:22:03AM +0000, Prashant Malani wrote:
> > > Fixes: fdc6b21e2444 ("platform/chrome: Add Type C connector class driver")
> > > Reported-by: Alyssa Ross <[email protected]>
> > > Signed-off-by: Prashant Malani <[email protected]>
> >
> > With a minor comment,
> > Reviewed-by: Tzung-Bi Shih <[email protected]>
> >
> > > @@ -1076,6 +1076,12 @@ static int cros_typec_probe(struct platform_device *pdev)
> > >
> > > typec->dev = dev;
> > > typec->ec = dev_get_drvdata(pdev->dev.parent);
> > > +
> >
> > I would prefer to remove the blank line to make it look like an integrated block.
>
> I actually prefer it as it is. typec->dev is not really part of this
> "integrated block", and I don't want to add another space there.
But on the other side the check is part of the "integrated block".
Maybe add an empty line between the two assignments if you want a
separation.
> In any case, since this is a very minor style nit, I will address it
> in case there is another version required due to other comments.
>
> >
> > > + if (!typec->ec) {
> > > + dev_err(dev, "couldn't find parent EC device\n");
> > > + return -ENODEV;
> > > + }
> > > +
>
> Best,
>
> -Prashant
Hey Guenter,
On Wed, Jan 26, 2022 at 7:33 AM Guenter Roeck <[email protected]> wrote:
>
> On Tue, Jan 25, 2022 at 8:05 PM Prashant Malani <[email protected]> wrote:
> >
> > Hi Tzung-Bi,
> >
> > Thanks for your review.
> >
> > On Tue, Jan 25, 2022 at 7:46 PM Tzung-Bi Shih <[email protected]> wrote:
> > >
> > > On Wed, Jan 26, 2022 at 01:22:03AM +0000, Prashant Malani wrote:
> > > > Fixes: fdc6b21e2444 ("platform/chrome: Add Type C connector class driver")
> > > > Reported-by: Alyssa Ross <[email protected]>
> > > > Signed-off-by: Prashant Malani <[email protected]>
> > >
> > > With a minor comment,
> > > Reviewed-by: Tzung-Bi Shih <[email protected]>
> > >
> > > > @@ -1076,6 +1076,12 @@ static int cros_typec_probe(struct platform_device *pdev)
> > > >
> > > > typec->dev = dev;
> > > > typec->ec = dev_get_drvdata(pdev->dev.parent);
> > > > +
> > >
> > > I would prefer to remove the blank line to make it look like an integrated block.
> >
> > I actually prefer it as it is. typec->dev is not really part of this
> > "integrated block", and I don't want to add another space there.
>
> But on the other side the check is part of the "integrated block".
> Maybe add an empty line between the two assignments if you want a
> separation.
OK. I'll add the space before it.
Thanks,