Re: [PATCH v2 0/2] gpio: mmio: Support ngpios property

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

 



On Fri, Oct 18, 2024 at 01:49:40PM +0200, Linus Walleij wrote:
> On Fri, Oct 18, 2024 at 1:30 PM Andy Shevchenko
> <andriy.shevchenko@xxxxxxxxx> wrote:
> 
> > First one is why? What the *practical* issue you have? Can you elaborate
> > on that?
> 
> Sure, there are these hardwares that probe directly from the
> gpio-mmio driver:
> Documentation/devicetree/bindings/gpio/gpio-mmio.yaml
> properties:
>   compatible:
>     enum:
>       - brcm,bcm6345-gpio
>       - ni,169445-nand-gpio
>       - wd,mbl-gpio # Western Digital MyBook Live memory-mapped GPIO controller
> 
> The practical issue is (similar to what was responded to Rob
> in patch 2/2) that non-existing GPIOs will get exposed to userspace.
> 
> For patch 1/2 (adding the DT binding) it would be that without
> ngpios we do not model the hardware properly.
> 
> The objection "it makes no harm to register GPIO lines
> for all bits in the register" can likewise be raised to the
> other 28 (if I count correctly) GPIO drivers that use this
> property (git grep ngpios drivers/gpio) and I think the train left the
> station long ago to object to the property in general, people
> don't want to expose non-existing GPIOs to the GPIO
> framework.

Sorry that I likely wasn't clear enough. My question was if you really
experienced any bugs in practice. The above is the theory part and
I completely agree with.

> > Second one, is there any other way to avoid duplication of the code so
> > we have one place of the property parsing?
> >
> > For the background I have to mention this commit:
> > 55b2395e4e92 ("gpio: mmio: handle "ngpios" properly in bgpio_init()")
> 
> Oh well spotted! I completely missed the fact that we already
> added ngpios parsing elsewhere in the driver.
> 
> Bartosz, can you please drop patch 2/2?

> Patch 1/2 is needed however: it is just documenting the behaviour
> that is already implemented.

I'm not agianst this, the first patch is the correct advertisement.
My questioning was related solely to the second patch in the series.

-- 
With Best Regards,
Andy Shevchenko






[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