[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAJq09z4wgPo=1_OtA6Y-0O4gLJ2nxy1jdf0BDoZwVmL=TkOdUg@mail.gmail.com>
Date: Wed, 14 Feb 2024 19:17:55 -0300
From: Luiz Angelo Daros de Luca <luizluca@...il.com>
To: Linus Walleij <linus.walleij@...aro.org>
Cc: Alvin Šipraga <alsi@...g-olufsen.dk>,
Andrew Lunn <andrew@...n.ch>, Florian Fainelli <f.fainelli@...il.com>,
Vladimir Oltean <olteanv@...il.com>, "David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>, Conor Dooley <conor+dt@...nel.org>,
Philipp Zabel <p.zabel@...gutronix.de>, netdev@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next v3 3/3] net: dsa: realtek: support reset controller
> Hi Luiz,
Hi Linus,
> thanks for your patch!
I'm glad to help ;-)
> On Wed, Feb 14, 2024 at 1:54 AM Luiz Angelo Daros de Luca
> <luizluca@...il.com> wrote:
>
> > The 'reset-gpios' will not work when the switch reset is controlled by a
> > reset controller.
> >
> > Although the reset is optional and the driver performs a soft reset
> > during setup, if the initial reset state was asserted, the driver will
> > not detect it.
> >
> > The reset controller will take precedence over the reset GPIO.
> >
> > Signed-off-by: Luiz Angelo Daros de Luca <luizluca@...il.com>
> (...)
> > +void rtl83xx_reset_assert(struct realtek_priv *priv)
> > +{
> > + int ret;
> > +
> > + if (priv->reset_ctl) {
> > + ret = reset_control_assert(priv->reset_ctl);
>
> Actually, reset_control_assert() is NULL-tolerand (as well as the
> stubs) so you can just issue this unconditionally. If priv->reset_ctl
> is NULL it will just return 0.
The idea was to avoid gpiod_set_value if the reset_control_assert was
configured and worked. However, I don't see a big issue in calling
them both as you don't expect both to be configured.
I'll remove the "ifs not null" and let both be called.
>
> > + if (!ret)
> > + return;
> > +
> > + dev_warn(priv->dev,
> > + "Failed to assert the switch reset control: %pe\n",
> > + ERR_PTR(ret));
> > + }
> > +
> > + if (priv->reset)
> > + gpiod_set_value(priv->reset, true);
>
> Same here! Also NULL-tolerant. You do not need to check priv->reset.
> Just issue it.
>
> > +void rtl83xx_reset_deassert(struct realtek_priv *priv)
>
> Same comments for deassert.
>
> With this fixed (the rest looks just fine):
> Reviewed-by: Linus Walleij <linus.walleij@...aro.org>
>
> Yours,
> Linus Walleij
Powered by blists - more mailing lists