[<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
Powered by blists - more mailing lists