Re: [PATCH RESEND 2/2] gpio: gpio-vf610: add imx7ulp support

[Linus Walleij <linus.walleij@xxxxxxxxxx>]

On Tue, Jul 25, 2017 at 3:47 PM, Dong Aisheng <aisheng.dong@xxxxxxx> wrote:

> The Rapid General-Purpose Input and Output with 2 Ports (RGPIO2P)
> on MX7ULP is similar to GPIO on Vibrid. But unlike Vibrid, the
> RGPIO2P has an extra Port Data Direction Register (PDDR) used
> to configure the individual port pins for input or output.
>
> We introduce a FSL_GPIO_HAVE_PDDR with fsl_gpio_soc_data data
> to distinguish this differences. And we support getting the output
> status by checking the GPIO direction in PDDR.
>
> Cc: Linus Walleij <linus.walleij@xxxxxxxxxx>
> Cc: Alexandre Courbot <gnurou@xxxxxxxxx>
> Cc: Shawn Guo <shawnguo@xxxxxxxxxx>
> Cc: Fugang Duan <fugang.duan@xxxxxxx>
> Cc: Peter Chen <peter.chen@xxxxxxx>
> Acked-by: Stefan Agner <stefan@xxxxxxxx>
> Signed-off-by: Dong Aisheng <aisheng.dong@xxxxxxx>

This is mostly OK but I want a change.

> +struct fsl_gpio_soc_data {
> +       u32 flags;
> +};

Why introduce complex things like bitwise flags. That looks
like premature optimization and trying to outsmart the compiler,
don't do that.

Add a bool have_paddr;

simply, and use that in the code.

Apart from that it is fine.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html