netdev - RE: [PATCH net-next] rswitch: Add PM ops

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]

Message-ID:
 <TYBPR01MB53411F936A70688D34CD459CD8D2A@TYBPR01MB5341.jpnprd01.prod.outlook.com>
Date: Fri, 13 Oct 2023 10:02:11 +0000
From: Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com>
To: Geert Uytterhoeven <geert@...ux-m68k.org>
CC: "s.shtylyov@....ru" <s.shtylyov@....ru>, "davem@...emloft.net"
	<davem@...emloft.net>, "edumazet@...gle.com" <edumazet@...gle.com>,
	"kuba@...nel.org" <kuba@...nel.org>, "pabeni@...hat.com" <pabeni@...hat.com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"linux-renesas-soc@...r.kernel.org" <linux-renesas-soc@...r.kernel.org>
Subject: RE: [PATCH net-next] rswitch: Add PM ops

Hi Geert-san,

> From: Geert Uytterhoeven, Sent: Friday, October 13, 2023 6:19 PM
> 
> Hi Shimoda-san,
> 
> On Fri, Oct 13, 2023 at 10:55 AM Yoshihiro Shimoda
> <yoshihiro.shimoda.uh@...esas.com> wrote:
> > > From: Geert Uytterhoeven, Sent: Friday, October 13, 2023 4:29 PM
> > > On Fri, Oct 13, 2023 at 4:10 AM Yoshihiro Shimoda
> > > <yoshihiro.shimoda.uh@...esas.com> wrote:
> > > > > From: Geert Uytterhoeven, Sent: Thursday, October 12, 2023 9:35 PM
> > > > > On Thu, Oct 12, 2023 at 2:16 PM Yoshihiro Shimoda
> > > > > <yoshihiro.shimoda.uh@...esas.com> wrote:
> > > > > > Add PM ops for Suspend to Idle. When the system suspended,
> > > > > > the Ethernet Serdes's clock will be stopped. So, this driver needs
> > > > > > to re-initialize the Ethernet Serdes by phy_init() in
> > > > > > renesas_eth_sw_resume(). Otherwise, timeout happened in phy_power_on().
> > > > > >
> > > > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com>
> 
> > > > > > --- a/drivers/net/ethernet/renesas/rswitch.c
> > > > > > +++ b/drivers/net/ethernet/renesas/rswitch.c
> > > > > > @@ -1991,11 +1993,52 @@ static void renesas_eth_sw_remove(struct platform_device *pdev)
> > > > > >         platform_set_drvdata(pdev, NULL);
> > > > > >  }
> > > > > >
> > > > > > +static int __maybe_unused renesas_eth_sw_suspend(struct device *dev)
> > > > > > +{
> > > > > > +       struct rswitch_private *priv = dev_get_drvdata(dev);
> > > > > > +       struct net_device *ndev;
> > > > > > +       int i;
> > > > >
> > > > > unsigned int (also below)
> > > >
> > > > I don't know why unsigned int is needed. Other functions use
> > >
> > > There's this old mantra "prefer unsigned over signed in C",
> > > and a valid port array index here is always unsigned.
> >
> > I understood it.
> >
> > > > rswitch_for_each_enabled_port{_continue_reverse}() with int.
> > > > Especially, rswitch_for_each_enabled_port_continue_reverse()
> > > > has the following code, unsigned int will not work correctly:
> > >
> > > Oh, there is also a reverse variant, which indeed needs a signed
> > > iterator, currently...
> > >
> > > > ---
> > > > #define rswitch_for_each_enabled_port_continue_reverse(priv, i) \
> > > >         for (i--; i >= 0; i--)                                  \
> > >
> > > I think this can be made to work with an unsigned iterator using
> > >
> > >     for (; i-- > 0; )
> >
> > I think that this loop cannot access index 0 :)
> 
> rswitch_for_each_enabled_port_continue_reverse() is meant to be
> used in the error path, to tear down all ports that were initialized
> succesfully? So i is always the index of the first port that failed to
> initialize, i.e. i > 0 on entry.
> 
> See "git grep -Ww rswitch_for_each_enabled_port_continue_reverse".

Thank you for your comment. And I'm sorry, I completely misunderstood
about "for (; i-- > 0; )" behavior... I should have tested this before
I send such a comment...

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