On Thu. 17 Oct. 2024 at 00:42, Markus Schneider-Pargmann
<msp@xxxxxxxxxxxx> 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@xxxxxxxxxxxx> 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@xxxxxxxxxxxx>
(...)
> > > + 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@xxxxxxxxxx>
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
