On Tue, Jun 25, 2019 at 9:33 PM Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx> wrote: > On Tue, Jun 25, 2019 at 12:53:46PM +0200, Linus Walleij wrote: > > The siox GPIO driver passes a IRQ_TYPE_EDGE_RISING as > > default IRQ trigger type which seems wrong, as consumers > > should explicitly set this up, so set IRQ_TYPE_NONE instead. > > > > Also gpiochip_remove() was called on the errorpath if > > gpiochip_add() failed: this is wrong, if the chip failed > > to add it is not there so it should not be removed. > > So we have a bugfix (gpiochip_remove() in error path), a change of > default behaviour (IRQ_TYPE_EDGE_RISING -> IRQ_TYPE_NONE) and a cleanup > for an API change (I'm guessing here) in a single patch. :-| Yes I tend to do that to save time because I am a bit overwhelmed by all the stuff that falls upwards to the GPIO maintainer. More often than not there is zero feedback from the maintainer(s) of the drivers, and the kernel looks better after than before. But since you provide some feedback I'll just go and split the patch. > @Thorsten: I'm not entirely sure if there is code relying on the default > IRQ_TYPE_EDGE_RISING. Do you know off-hand? I saw that the driver has #include <linux/of.h> (hm seems unused) so if this is used on devicetree systems with normal twocell irqchips then there shouldn't be a need for any default type. The default type is only used with board files. The siox seems not even possible to use with board files (no platform data path). Yours, Linus Walleij