Re: [PATCH 2/2] serial: mctrl_gpio: Support all GPIO suffixes (gpios vs gpio)

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

 



Hi Stefan,

On Mon, Aug 12, 2019 at 1:54 PM Stefan Roese <sr@xxxxxxx> wrote:
> On 12.08.19 13:17, Geert Uytterhoeven wrote:
> > On Thu, Aug 8, 2019 at 3:25 PM Stefan Roese <sr@xxxxxxx> wrote:
> >> This patch fixes a backward compatibility issue, when boards use the
> >> old style GPIO suffix "-gpio" instead of the new "-gpios". This
> >> potential problem has been introduced by commit d99482673f95 ("serial:
> >> mctrl_gpio: Check if GPIO property exisits before requesting it").
> >>
> >> This patch now fixes this issue by iterating over all supported GPIO
> >> suffixes by using the newly introduced for_each_gpio_suffix() helper.
> >>
> >> Also, the string buffer is now allocated on the stack to avoid the
> >> problem of allocation in a loop and its potential failure.
> >>
> >> Signed-off-by: Stefan Roese <sr@xxxxxxx>
> >
> > Do we really need to spread this *-gpio" legacy support all over the kernel?
> >
> > Seeing the only in-kernel users of legacy "rts-gpio" are
> >
> >      arch/arm/boot/dts/am335x-nano.dts:      rts-gpio = <&gpio0 13
> > GPIO_ACTIVE_HIGH>;
> >      arch/arm/boot/dts/am335x-nano.dts:      rts-gpio = <&gpio2 15
> > GPIO_ACTIVE_HIGH>;
> >      arch/arm/boot/dts/am335x-pdu001.dts:    rts-gpio = <&gpio1 9
> > GPIO_ACTIVE_HIGH>;
> >
> > and this is handled by omap-serial.c, predating mctrl_gpio, I'd like to
> > reconsider.
> >
> > Documentation/devicetree/bindings/serial/serial.txt always described
> > the "*-gpios"
> > variants, so there should be no users of the legacy "*-gpio" variants.
>
> Hmmm, you were the one to comment about supporting (not breaking) the
> "-gpio" variant:
>
> https://lkml.org/lkml/2019/6/24/248
>
> That was my main motivation to work on this patch.

I know. That's why I wrote "reconsider".  Before, I assumed there were actual
users of the legacy properties, which turned out to be wrong.

IMHO no driver should be aware there can be multiple possible suffixes.

Sorry for the trouble.

> If we all agree, that only the documented "-gpios" variant needs to
> be supported, then we can drop this patch.

BTW, I'm still wondering if d99482673f950817 ("serial: mctrl_gpio: Check
if GPIO property exisits before requesting it") is the right fix.
This is a place where we rely explicitly on named GPIOs being present, so
IMHO the ACPI code should not return a random GPIO instead.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds



[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux PPP]     [Linux FS]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Linmodem]     [Device Mapper]     [Linux Kernel for ARM]

  Powered by Linux