linux-kernel - Re: [PATCH] platform/chrome: cros_ec

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 26 Jan 2022 07:33:38 -0800
From:   Guenter Roeck <groeck@...gle.com>
To:     Prashant Malani <pmalani@...omium.org>
Cc:     Tzung-Bi Shih <tzungbi@...gle.com>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        Alyssa Ross <hi@...ssa.is>, Benson Leung <bleung@...omium.org>,
        Enric Balletbo i Serra <enric.balletbo@...labora.com>,
        Guenter Roeck <groeck@...omium.org>,
        Heikki Krogerus <heikki.krogerus@...ux.intel.com>
Subject: Re: [PATCH] platform/chrome: cros_ec_typec: Check for EC device

On Tue, Jan 25, 2022 at 8:05 PM Prashant Malani <pmalani@...omium.org> wrote:
>
> Hi Tzung-Bi,
>
> Thanks for your review.
>
> On Tue, Jan 25, 2022 at 7:46 PM Tzung-Bi Shih <tzungbi@...gle.com> 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 <hi@...ssa.is>
> > > Signed-off-by: Prashant Malani <pmalani@...omium.org>
> >
> > With a minor comment,
> > Reviewed-by: Tzung-Bi Shih <tzungbi@...gle.com>
> >
> > > @@ -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