Re: [PATCH v2 bus+gpio 3/4] drivers: gpio: Add support for GPIOs over Moxtet bus

[Linus Walleij]

On Fri, Nov 2, 2018 at 11:35 AM Marek BehÃn <marek.behun@xxxxxx> wrote:

> This adds support for interpreting the input and output bits of one
> device on Moxtet bus as GPIOs.
> This is needed for example by the SFP cage module of Turris Mox.
>
> Signed-off-by: Marek BehÃn <marek.behun@xxxxxx>

Overall looks fine, it's OK like this just adding some nitpicking:

> +static const struct moxtet_gpio_desc descs[] = {
> +       [TURRIS_MOX_MODULE_SFP] = {
> +               .in_mask = 0x7,
> +               .out_mask = 0x30,
> +       },
> +};

You can actually use things like:

.in_mask = GENMASK(3,0);

ton indicate exactly which bits you hit, but maybe it is overkill.
(No big deal.)

> +       return (ret & BIT(offset)) ? 1 : 0;

I would usually just clamp like this:

return !!(red & BIT(offset));

> +static int moxtet_gpio_get_direction(struct gpio_chip *gc, unsigned int offset)
> +{
> +       struct moxtet_gpio_chip *chip = gpiochip_get_data(gc);
> +
> +       if (chip->desc->in_mask & BIT(offset))
> +               return 1;
> +       else if (chip->desc->out_mask & BIT(offset))
> +               return 0;
> +       else
> +               return -EINVAL;

Maybe insert a comment here that a line is hard wired
as either input or output.

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

Yours,
Linus Walleij