On Fri, Apr 12, 2024 at 10:20:24AM +0200, Linus Walleij wrote: > On Tue, Apr 9, 2024 at 1:17 AM Andy Shevchenko > <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote: > > > The GPIO_* flag definitions are *almost* duplicated in two files > > (with unmatches OPEN_SOURCE / OPEN_DRAIN). Moreover, some code relies > > on one set of definitions while the rest is on the other. Clean up > > this mess by providing only one source of the definitions to all. > > > > Fixes: b424808115cb ("brcm80211: brcmsmac: Move LEDs to GPIO descriptors") > > Fixes: 5923ea6c2ce6 ("gpio: pass lookup and descriptor flags to request_own") > > Fixes: fed7026adc7c ("gpiolib: Make use of enum gpio_lookup_flags consistent") > > Fixes: 4c0facddb7d8 ("gpio: core: Decouple open drain/source flag with active low/high") > > Fixes: 69d301fdd196 ("gpio: add DT bindings for existing consumer flags") > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> > > The way the line lookup flags ("lflags") were conceived was through > support for non-DT systems using descriptor tables, and that is how > enum gpio_lookup_flags came to be. > > When OF support was added it was bolted on on the side, in essence > assuming that the DT/OF ABI was completely separate (and they/we > sure like to think about it that way...) and thus needed translation from > OF flags to kernel-internal enum gpio_lookup_flags. > > The way *I* thought about this when writing it was certainly that the > DT bindings was a separate thing (<dt-bindings/*.h> didn't even exist > at the time I think) and that translation from OF to kernel-internal > lflags would happen in *one* place. > > The main reasoning still holds: the OF define is an ABI, so it can > *never* be changed, but the enum gpio_lookup_flags is subject to > Documentation/process/stable-api-nonsense.rst and that means > that if we want to swap around the order of the definitions we can. > > But admittedly this is a bit over-belief in process and separation of > concerns and practical matters may be something else... Got it. But we have a name clash and the mess added to the users. I can redo this to separate these entities. Note, that there is code in the kernel that *does* use #include <dt-bindings/*.h> for Linux internals. $ git grep -lw '^#include <dt-bindings/.*\.h>' -- drivers/ | xargs dirname | cut -f 1,2 -d '/' | sort -u drivers/bus drivers/clk drivers/clocksource drivers/cpufreq drivers/dma drivers/firmware drivers/gpio drivers/gpu drivers/hwtracing drivers/i2c drivers/iio drivers/input drivers/interconnect drivers/iommu drivers/irqchip drivers/leds drivers/mailbox drivers/media drivers/memory drivers/mfd drivers/net drivers/phy drivers/pinctrl drivers/platform drivers/pmdomain drivers/power drivers/pwm drivers/regulator drivers/remoteproc drivers/reset drivers/rtc drivers/soc drivers/spmi drivers/thermal drivers/tty drivers/video drivers/watchdog P.S> One of the patch this tries to fix is yours IIRC :-) -- With Best Regards, Andy Shevchenko