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

[Linus Walleij]

On Mon, Mar 3, 2025 at 5:49 PM Andy Shevchenko
<andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:

> This updates the driver to gpiod API, and removes yet another use of
> of_get_named_gpio().
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>

Reviewed-by: Linus Walleij <linus.walleij@xxxxxxxxxx>

But note:

> @@ -632,10 +630,10 @@ static void ca8210_reset_send(struct spi_device *spi, unsigned int ms)
>         struct ca8210_priv *priv = spi_get_drvdata(spi);
>         long status;
>
> -       gpio_set_value(pdata->gpio_reset, 0);
> +       gpiod_set_value(pdata->reset_gpio, 0);
>         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,
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.

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.

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

Yours,
Linus Walleij