On Fri, Dec 22, 2017 at 2:17 AM, Andrew Cooks <andrew.cooks@xxxxxxxxxxxx> wrote: > On 21/12/17 23:02, Christian Lamparter wrote: >> Just a FYI: due to these difficulties with getting a gpio driver >> upstream, Alan Mizrahi upstreamed an in-kernel led-apu.c driver [0] >> that sort of bypasses the whole pinctrl vs gpio issue. > > Thanks, I saw that and was somewhat surprised to see it accepted. I looked at the driver and it seems actually good and doing the right thing. If I understand it right: - It does a bunch of magic dmi_match() on DMI_BOARD_NAME to figure out what board this is. - It then proceeds to register LEDs using some bits in the MMIO area that are used for LEDs. So these bits/lines are actually not GPIO, since GPIO means "general purpose input/output" - they are specific purpose and the specific purpose can be detected. So I don't have any objections from an architectural point of view. Some may expose the register as GPIO and add LEDs on top but it (using drivers/leds/leds-gpio.c) but that may be pointless extra abstraction. It has a deeper problem though. It is writing these GPIO registers without any thought of someone else possibly accessing them and without any shared locks. This problem is usually solved by either using GPIO inbetween if the GPIO driver takes care of protecting the register, or using regmap, preferably with drivers/mfd/syscon.c, so that regmap will protect the MMIO register. So there may be technical reasons to use GPIO or syscon abstractions that are not architectural reasons if you see what I mean. If only these few GPIO lines are actually used and only used for these LEDs, (the rest of the bits in the register unused) then this driver is as good as any. 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