linux-kernel - RE: [PATCH v2] phy: renesas: rcar-gen3-usb2: fix imbalance powered flag

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] [day] [month] [year] [list]

Date:   Mon, 10 Jun 2019 06:02:59 +0000
From:   Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com>
To:     Geert Uytterhoeven <geert@...ux-m68k.org>
CC:     Kishon Vijay Abraham I <kishon@...com>,
        Geert Uytterhoeven <geert+renesas@...der.be>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Linux-Renesas <linux-renesas-soc@...r.kernel.org>
Subject: RE: [PATCH v2] phy: renesas: rcar-gen3-usb2: fix imbalance powered
 flag

Hi Geert-san,

> From: Geert Uytterhoeven, Sent: Friday, June 7, 2019 10:23 PM
> 
> Hi Shimoda-san,
> 
> Thanks for the update!
> 
> On Fri, Jun 7, 2019 at 12:07 PM Yoshihiro Shimoda
> <yoshihiro.shimoda.uh@...esas.com> wrote:
> > The powered flag should be set for any other phys anyway. Also
> > the flag should be locked by the channel. Otherwise, after we have
> > revised the device tree for the usb phy, the following warning
> > happened during a second system suspend. And if the driver doesn't
> > lock the flag, enabling the regulator is possible to be imbalance
> 
> I think it reads better as:
> 
> ..., an imbalance is possible when enabling the regulator ...

Thank you for your suggestion! I'll revise it on v3.

> > during system resume. So, this patch fixes the issues.
> 
> > --- a/drivers/phy/renesas/phy-rcar-gen3-usb2.c
> > +++ b/drivers/phy/renesas/phy-rcar-gen3-usb2.c
> 
> > @@ -106,6 +107,7 @@ struct rcar_gen3_chan {
> >         struct rcar_gen3_phy rphys[NUM_OF_PHYS];
> >         struct regulator *vbus;
> >         struct work_struct work;
> > +       struct mutex lock;
> 
> It is always a good idea to document what is protected by the mutex:
> 
>         struct mutex lock;    /* protects rphys[...].powered */

I got it.

> Checkpatch does have a check for this, but unfortunately it is enabled for
> drivers/net/, net/, and drivers/staging/ only:
> 
>     CHECK: struct mutex definition without comment

Oh, I didn't know that. Thank you for your information.

> Reviewed-by: Geert Uytterhoeven <geert+renesas@...der.be>
> 
> and the imbalance is gone:
> Tested-by: Geert Uytterhoeven <geert+renesas@...der.be>

Thank you very much for your Reviewed-by and Tested-by tags!

Best regards,
Yoshihiro Shimoda

> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds