linux-kernel - Re: [PATCH v4 5/9] can: m_can: Support pinctrl wakeup state

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 for Android: free password hash cracker in your pocket

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

Message-ID: <CAMZ6Rq+sWB_SV0gt6K8n0cw+PjoN_fifCUJXTxJ7Hp8FEAt7-w@mail.gmail.com>
Date: Thu, 17 Oct 2024 01:09:19 +0900
From: Vincent MAILHOL <mailhol.vincent@...adoo.fr>
To: Markus Schneider-Pargmann <msp@...libre.com>
Cc: Chandrasekar Ramakrishnan <rcsekar@...sung.com>, Marc Kleine-Budde <mkl@...gutronix.de>, 
	"David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, 
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Rob Herring <robh@...nel.org>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, Nishanth Menon <nm@...com>, 
	Vignesh Raghavendra <vigneshr@...com>, Tero Kristo <kristo@...nel.org>, linux-can@...r.kernel.org, 
	netdev@...r.kernel.org, devicetree@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org, 
	Matthias Schiffer <matthias.schiffer@...tq-group.com>, Vishal Mahaveer <vishalm@...com>, 
	Kevin Hilman <khilman@...libre.com>, Dhruva Gole <d-gole@...com>, Simon Horman <horms@...nel.org>
Subject: Re: [PATCH v4 5/9] can: m_can: Support pinctrl wakeup state

On Thu. 17 Oct. 2024 at 00:42, Markus Schneider-Pargmann
<msp@...libre.com> wrote:
> Hi Vincent,
>
> On Wed, Oct 16, 2024 at 11:26:22PM GMT, Vincent MAILHOL wrote:
> > Hi Markus,
> >
> > This is a nice improvement from the v3.
> >
> > On Wed. 16 Oct. 2024 at 04:19, Markus Schneider-Pargmann
> > <msp@...libre.com> wrote:
> > > am62 requires a wakeup flag being set in pinctrl when mcan pins acts as
> > > a wakeup source. Add support to select the wakeup state if WOL is
> > > enabled.
> > >
> > > Signed-off-by: Markus Schneider-Pargmann <msp@...libre.com>

(...)

> > > +       class_dev->pinctrl_state_default =
> > > +               pinctrl_lookup_state(class_dev->pinctrl, "default");
> > > +       if (IS_ERR(class_dev->pinctrl_state_default)) {
> > > +               ret = PTR_ERR(class_dev->pinctrl_state_default);
> >
> > Sorry if this is a silly question, but why aren't you doing the:
> >
> >                   class_dev->pinctrl_state_default = NULL;
> >
> >                   if (ret == -ENODEV)
> >                           return 0;
> >
> > thing the same way you are doing it for the pinctrl and the
> > pinctrl_state_wakeup?
>
> There are no silly questions.
> The idea is that if the wakeup pinctrl state was already found, then the
> default pinctrl state is required and not optional, so no check for
> -ENODEV. Otherwise it doesn't make sense with the current binding or the
> implementation of the driver.

ACK. With this:

Reviewed-by: Vincent Mailhol <mailhol.vincent@...adoo.fr>

This concludes my review of your series. I am skipping the dt-bindings
and the dts parts because I am not knowledgeable in those domains.


Yours sincerely,
Vincent Mailhol