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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> -----Original Message-----
> From: Linus Walleij [mailto:linus.walleij@xxxxxxxxxx]
> Sent: Tuesday, August 01, 2017 8:04 PM
> To: A.s. Dong
> Cc: linux-gpio@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx;
> Shawn Guo; Stefan Agner; Jacky Bai; Andy Duan; Peter Chen; Dong Aisheng;
> Sascha Hauer; Alexandre Courbot
> Subject: Re: [PATCH RESEND 2/2] gpio: gpio-vf610: add imx7ulp support
> 
> 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.
> 

It's for conveniently adding new flags in the future.
But it's true that it may be premature optimization.

> Add a bool have_paddr;
> 
> simply, and use that in the code.
> 

Will do it.
Thanks for the advice.

Regards
Dong Aisheng

> Apart from that it is fine.
> 
> Yours,
> Linus Walleij
��.n��������+%������w��{.n�����{��
b���ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f




[Index of Archives]     [Linux SPI]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux