Re: [PATCH net-next v2 3/3] ieee802154: ca8210: Switch to using gpiod API

[Andy Shevchenko <andy.shevchenko@xxxxxxxxx>]

Mon, Mar 03, 2025 at 09:00:39PM +0100, Linus Walleij kirjoitti:
> On Mon, Mar 3, 2025 at 5:49 PM Andy Shevchenko
> <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:

...

> >         reinit_completion(&priv->ca8210_is_awake);
> >         msleep(ms);
> > -       gpio_set_value(pdata->gpio_reset, 1);
> > +       gpiod_set_value(pdata->reset_gpio, 1);
> 
> This drives the GPIO low to assert reset, meaning it is something
> that should have GPIO_ACTIVE_LOW set in the device tree,

Yeah, the pin 27 is marked as NRESET and description is pointing out to it as
active low.

> and it might even have, so let's check what we can check:
> 
> git grep cascoda,ca8210
> Documentation/devicetree/bindings/net/ieee802154/ca8210.txt:    -
> compatible:           Should be "cascoda,ca8210"
> Documentation/devicetree/bindings/net/ieee802154/ca8210.txt:
>  compatible = "cascoda,ca8210";
> drivers/net/ieee802154/ca8210.c:        {.compatible = "cascoda,ca8210", },
> 
> well ain't that typical, all users are out of tree. The example
> in the bindings file is wrong, setting ACTIVE_HIGH. Sigh, let's
> assume all those DTS files somewhere are wrong and they
> didn't set GPIO_ACTIVE_LOW in them...

> Maybe add a comment in the code that this is wrong and the
> driver and DTS files should be fixed.

Or maybe fix in the driver and schema and add a quirk to gpiolib-of.c?

> Alternatively patch Documentation/devicetree/bindings/net/ieee802154/ca8210.txt
> to set GPIO_ACTIVE_LOW and fix the code to invert it both
> here and when getting the GPIO, but it could cause problems
> for outoftree users.

Would it? We have such quirks to fix a polarity for other drivers/devices.

> Either way, this is good progress:
> Reviewed-by: Linus Walleij <linus.walleij@xxxxxxxxxx>

Thank you!

-- 
With Best Regards,
Andy Shevchenko