* regression v4.16 on Nokia N900:/dev/input/event6 aka AV Jack support disappeared @ 2018-02-24 21:46 Pavel Machek 2018-02-26 9:47 ` Thorsten Leemhuis 2018-02-26 15:43 ` regression v4.16 on Nokia N900:/dev/input/event6 aka AV Jack support disappeared Andrew F. Davis 0 siblings, 2 replies; 20+ messages in thread From: Pavel Machek @ 2018-02-24 21:46 UTC (permalink / raw) To: pali.rohar, sre, kernel list, linux-arm-kernel, linux-omap, tony, khilman, aaro.koskinen, ivo.g.dimitrov.75, patrikbachan, serge, abcloriens, clayton, martijn, sakari.ailus, Filip Matijević, afd Cc: broonie, peter.ujfalusi, alsa-devel [-- Attachment #1: Type: text/plain, Size: 1124 bytes --] Hi! In v4.16, AV jack support disappeared. This one is suspect: commit 7be4b5dc7ffa9499ac6ef33a5ffa9ff43f9b7057 Author: Andrew F. Davis <afd@ti.com> Date: Wed Nov 29 11:13:59 2017 -0600 ARM: dts: omap3-n900: Fix the audio CODEC's reset pin The correct DT property for specifying a GPIO used for reset is "reset-gpios", fix this here. Fixes: 14e3e295b2b9 ("ARM: dts: omap3-n900: Add TLV320AIC3X support") Signed-off-by: Andrew F. Davis <afd@ti.com> Signed-off-by: Tony Lindgren <tony@atomide.com> How was it tested? It reverts polarity of reset pin, but sound/soc/codecs/tlv320aic3x.c treats those as aliases: ret = of_get_named_gpio(np, "reset-gpios", 0); if (ret >= 0) { aic3x->gpio_reset = ret; } else { ret = of_get_named_gpio(np, "gpio-reset", 0); if (ret > 0) { dev_warn(&i2c->dev, "Using deprecated property \"gpio-r\eset\", please update your DT"); aic3x->gpio_reset = ret; Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: regression v4.16 on Nokia N900:/dev/input/event6 aka AV Jack support disappeared 2018-02-24 21:46 regression v4.16 on Nokia N900:/dev/input/event6 aka AV Jack support disappeared Pavel Machek @ 2018-02-26 9:47 ` Thorsten Leemhuis 2018-02-26 13:13 ` regression v4.16 on Nokia N900: sound does not work Pavel Machek 2018-02-26 15:43 ` regression v4.16 on Nokia N900:/dev/input/event6 aka AV Jack support disappeared Andrew F. Davis 1 sibling, 1 reply; 20+ messages in thread From: Thorsten Leemhuis @ 2018-02-26 9:47 UTC (permalink / raw) To: Pavel Machek, pali.rohar, sre, kernel list, linux-arm-kernel, linux-omap, tony, khilman, aaro.koskinen, ivo.g.dimitrov.75, patrikbachan, serge, abcloriens, clayton, martijn, sakari.ailus, Filip Matijević, afd Cc: broonie, peter.ujfalusi, alsa-devel JFYI: This issues is tracked in the regression reports for Linux 4.16 (http://bit.ly/lnxregrep416 ) with this id: Linux-Regression-ID: lr#4b650f Please include this line in the comment section of patches that are supposed to fix the issue. Please also mention the string once in other mailinglist threads or different bug tracking entries if you or someone else start to discuss the issue there. By including that string you make it a whole lot easier to track where an issue gets discussed and how far patches to fix it have made it. For more details on this please see here: http://bit.ly/lnxregtrackid Thx for your help. Ciao, Thorsten On 24.02.2018 22:46, Pavel Machek wrote: > Hi! > > In v4.16, AV jack support disappeared. > > This one is suspect: > > commit 7be4b5dc7ffa9499ac6ef33a5ffa9ff43f9b7057 > Author: Andrew F. Davis <afd@ti.com> > Date: Wed Nov 29 11:13:59 2017 -0600 > > ARM: dts: omap3-n900: Fix the audio CODEC's reset pin > > The correct DT property for specifying a GPIO used for reset > is "reset-gpios", fix this here. > > Fixes: 14e3e295b2b9 ("ARM: dts: omap3-n900: Add TLV320AIC3X > support") > > Signed-off-by: Andrew F. Davis <afd@ti.com> > Signed-off-by: Tony Lindgren <tony@atomide.com> > > How was it tested? It reverts polarity of reset pin, but > sound/soc/codecs/tlv320aic3x.c treats those as aliases: > > ret = of_get_named_gpio(np, "reset-gpios", 0); > if (ret >= 0) { > aic3x->gpio_reset = ret; > } else { > ret = of_get_named_gpio(np, "gpio-reset", 0); > if (ret > 0) { > dev_warn(&i2c->dev, "Using deprecated property \"gpio-r\eset\", please update your DT"); > aic3x->gpio_reset = ret; > > > Pavel > ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: regression v4.16 on Nokia N900: sound does not work 2018-02-26 9:47 ` Thorsten Leemhuis @ 2018-02-26 13:13 ` Pavel Machek 2018-02-26 14:02 ` [alsa-devel] " Daniel Baluta 0 siblings, 1 reply; 20+ messages in thread From: Pavel Machek @ 2018-02-26 13:13 UTC (permalink / raw) To: Thorsten Leemhuis Cc: pali.rohar, sre, kernel list, linux-arm-kernel, linux-omap, tony, khilman, aaro.koskinen, ivo.g.dimitrov.75, patrikbachan, serge, abcloriens, clayton, martijn, sakari.ailus, Filip Matijević, afd, broonie, peter.ujfalusi, alsa-devel [-- Attachment #1: Type: text/plain, Size: 577 bytes --] Hi! > JFYI: This issues is tracked in the regression reports for Linux 4.16 > (http://bit.ly/lnxregrep416 ) with this id: > > Linux-Regression-ID: lr#4b650f Ok, so it seems that issue is bigger: whole sound subsystem does not work. /proc/asound/cards is empty. 7e6127c1240ed569cdda2a67c8f03836f9f28c05 seems to be bad already. I tried to revert sound/soc changes, and sound is broken, too. Nasty :-(. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [alsa-devel] regression v4.16 on Nokia N900: sound does not work 2018-02-26 13:13 ` regression v4.16 on Nokia N900: sound does not work Pavel Machek @ 2018-02-26 14:02 ` Daniel Baluta 2018-02-26 23:13 ` Pavel Machek 0 siblings, 1 reply; 20+ messages in thread From: Daniel Baluta @ 2018-02-26 14:02 UTC (permalink / raw) To: Pavel Machek Cc: Thorsten Leemhuis, peter.ujfalusi, Linux-ALSA, ivo.g.dimitrov.75, khilman, tony, aaro.koskinen, kernel list, sre, martijn, Filip Matijević, Mark Brown, abcloriens, sakari.ailus, pali.rohar, clayton, linux-omap, Andrew F . Davis, patrikbachan, linux-arm-kernel, serge On Mon, Feb 26, 2018 at 3:13 PM, Pavel Machek <pavel@ucw.cz> wrote: > Hi! > >> JFYI: This issues is tracked in the regression reports for Linux 4.16 >> (http://bit.ly/lnxregrep416 ) with this id: >> >> Linux-Regression-ID: lr#4b650f > > Ok, so it seems that issue is bigger: whole sound subsystem does not > work. /proc/asound/cards is empty. > > 7e6127c1240ed569cdda2a67c8f03836f9f28c05 seems to be bad already. > > I tried to revert sound/soc changes, and sound is broken, too. Nasty dmesg log? ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [alsa-devel] regression v4.16 on Nokia N900: sound does not work 2018-02-26 14:02 ` [alsa-devel] " Daniel Baluta @ 2018-02-26 23:13 ` Pavel Machek 2018-02-26 23:30 ` Pavel Machek 2018-02-27 8:43 ` Linus Walleij 0 siblings, 2 replies; 20+ messages in thread From: Pavel Machek @ 2018-02-26 23:13 UTC (permalink / raw) To: Daniel Baluta, linus.walleij Cc: Thorsten Leemhuis, peter.ujfalusi, Linux-ALSA, ivo.g.dimitrov.75, khilman, tony, aaro.koskinen, kernel list, sre, martijn, Filip Matijević, Mark Brown, abcloriens, sakari.ailus, pali.rohar, clayton, linux-omap, Andrew F . Davis, patrikbachan, linux-arm-kernel, serge [-- Attachment #1: Type: text/plain, Size: 2362 bytes --] On Mon 2018-02-26 16:02:22, Daniel Baluta wrote: > On Mon, Feb 26, 2018 at 3:13 PM, Pavel Machek <pavel@ucw.cz> wrote: > > Hi! > > > >> JFYI: This issues is tracked in the regression reports for Linux 4.16 > >> (http://bit.ly/lnxregrep416 ) with this id: > >> > >> Linux-Regression-ID: lr#4b650f > > > > Ok, so it seems that issue is bigger: whole sound subsystem does not > > work. /proc/asound/cards is empty. > > > > 7e6127c1240ed569cdda2a67c8f03836f9f28c05 seems to be bad already. > > > > I tried to revert sound/soc changes, and sound is broken, too. Nasty > > > dmesg log? Partial dmesg is at: https://github.com/pavelmachek/missy/blob/master/db/phone/nokia/n900/pavel/2018.1291171648263/dmesg.out I should be able to get full one... I did git bisect, and the winner seems to be: pavel@duo:/data/l/linux-n900$ git bisect bad c85823390215e52d68d3826df92a447ed31e5c80 is the first bad commit commit c85823390215e52d68d3826df92a447ed31e5c80 Author: Linus Walleij <linus.walleij@linaro.org> Date: Wed Dec 27 16:37:44 2017 +0100 gpio: of: Support SPI nonstandard GPIO properties Before it was clearly established that all GPIO properties in the device tree shall be named "foo-gpios" (with the deprecated variant "foo-gpio" for single lines) we unfortunately merged a few bindings which named the lines "gpio-foo" instead. This is most prominent in the GPIO SPI driver in Linux which names the lines "gpio-sck", "gpio-mosi" and "gpio-miso". As we want to switch the GPIO SPI driver to using descriptors, we need devm_gpiod_get() to return something reasonable when looking up these in the device tree. Put in a special #ifdef:ed kludge to do this special lookup only for the SPI case and gets compiled out if we're not enabling SPI. If we have more oddly defined legacy GPIOs like this, they can be handled in a similar manner. Reviewed-by: Rob Herring <robh@kernel.org> Signed-off-by: Linus Walleij <linus.walleij@linaro.org> Unfortunately, it does not seem to revert cleanly on my v4.16 branch. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [alsa-devel] regression v4.16 on Nokia N900: sound does not work 2018-02-26 23:13 ` Pavel Machek @ 2018-02-26 23:30 ` Pavel Machek 2018-02-27 8:43 ` Linus Walleij 1 sibling, 0 replies; 20+ messages in thread From: Pavel Machek @ 2018-02-26 23:30 UTC (permalink / raw) To: Daniel Baluta, linus.walleij Cc: Thorsten Leemhuis, peter.ujfalusi, Linux-ALSA, ivo.g.dimitrov.75, khilman, tony, aaro.koskinen, kernel list, sre, martijn, Filip Matijević, Mark Brown, abcloriens, sakari.ailus, pali.rohar, clayton, linux-omap, Andrew F . Davis, patrikbachan, linux-arm-kernel, serge [-- Attachment #1: Type: text/plain, Size: 3472 bytes --] Hi! > > >> JFYI: This issues is tracked in the regression reports for Linux 4.16 > > >> (http://bit.ly/lnxregrep416 ) with this id: > > >> > > >> Linux-Regression-ID: lr#4b650f > > > > > > Ok, so it seems that issue is bigger: whole sound subsystem does not > > > work. /proc/asound/cards is empty. > > > > > > 7e6127c1240ed569cdda2a67c8f03836f9f28c05 seems to be bad already. > > > > > > I tried to revert sound/soc changes, and sound is broken, too. Nasty > > > > > > dmesg log? > > Partial dmesg is at: > https://github.com/pavelmachek/missy/blob/master/db/phone/nokia/n900/pavel/2018.1291171648263/dmesg.out > > I should be able to get full one... > > I did git bisect, and the winner seems to be: > > pavel@duo:/data/l/linux-n900$ git bisect bad > c85823390215e52d68d3826df92a447ed31e5c80 is the first bad commit > commit c85823390215e52d68d3826df92a447ed31e5c80 > Author: Linus Walleij <linus.walleij@linaro.org> > Date: Wed Dec 27 16:37:44 2017 +0100 I reverted it on top of v4.16-rc2, and sound now works. Ideas? (Aha, and I see I made small mistake reverting... but...) Pavel diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c index 564bb7a..50cc590 100644 --- a/drivers/gpio/gpiolib-of.c +++ b/drivers/gpio/gpiolib-of.c @@ -157,36 +157,6 @@ int of_get_named_gpio_flags(struct device_node *np, const char *list_name, EXPORT_SYMBOL(of_get_named_gpio_flags); /* - * The SPI GPIO bindings happened before we managed to establish that GPIO - * properties should be named "foo-gpios" so we have this special kludge for - * them. - */ -static struct gpio_desc *of_find_spi_gpio(struct device *dev, const char *con_id, - enum of_gpio_flags *of_flags) -{ - char prop_name[32]; /* 32 is max size of property name */ - struct device_node *np = dev->of_node; - struct gpio_desc *desc; - - /* - * Hopefully the compiler stubs the rest of the function if this - * is false. - */ - if (!IS_ENABLED(CONFIG_SPI_MASTER)) - return ERR_PTR(-ENOENT); - - /* Allow this specifically for "spi-gpio" devices */ - if (!of_device_is_compatible(np, "spi-gpio") || !con_id) - return ERR_PTR(-ENOENT); - - /* Will be "gpio-sck", "gpio-mosi" or "gpio-miso" */ - snprintf(prop_name, sizeof(prop_name), "%s-%s", "gpio", con_id); - - desc = of_get_named_gpiod_flags(np, prop_name, 0, of_flags); - return desc; -} - -/* * Some regulator bindings happened before we managed to establish that GPIO * properties should be named "foo-gpios" so we have this special kludge for * them. @@ -230,7 +200,6 @@ struct gpio_desc *of_find_gpio(struct device *dev, const char *con_id, struct gpio_desc *desc; unsigned int i; - /* Try GPIO property "foo-gpios" and "foo-gpio" */ for (i = 0; i < ARRAY_SIZE(gpio_suffixes); i++) { if (con_id) snprintf(prop_name, sizeof(prop_name), "%s-%s", con_id, @@ -245,14 +214,6 @@ struct gpio_desc *of_find_gpio(struct device *dev, const char *con_id, break; } - /* Special handling for SPI GPIOs if used */ - if (IS_ERR(desc)) - desc = of_find_spi_gpio(dev, con_id, &of_flags); - - /* Special handling for regulator GPIOs if used */ - if (IS_ERR(desc)) - desc = of_find_regulator_gpio(dev, con_id, &of_flags); - if (IS_ERR(desc)) return desc; -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [alsa-devel] regression v4.16 on Nokia N900: sound does not work 2018-02-26 23:13 ` Pavel Machek 2018-02-26 23:30 ` Pavel Machek @ 2018-02-27 8:43 ` Linus Walleij 2018-03-02 9:10 ` Pavel Machek 1 sibling, 1 reply; 20+ messages in thread From: Linus Walleij @ 2018-02-27 8:43 UTC (permalink / raw) To: Pavel Machek Cc: Daniel Baluta, Thorsten Leemhuis, Peter Ujfalusi, Linux-ALSA, Ivajlo Dimitrov, Kevin Hilman, ext Tony Lindgren, Aaro Koskinen, kernel list, Sebastian Reichel, martijn, Filip Matijević, Mark Brown, abcloriens, Sakari Ailus, Pali Rohár, clayton, Linux-OMAP, Andrew F . Davis, patrikbachan, linux-arm-kernel, serge On Tue, Feb 27, 2018 at 12:13 AM, Pavel Machek <pavel@ucw.cz> wrote: > I did git bisect, and the winner seems to be: > > pavel@duo:/data/l/linux-n900$ git bisect bad > c85823390215e52d68d3826df92a447ed31e5c80 is the first bad commit > commit c85823390215e52d68d3826df92a447ed31e5c80 > Author: Linus Walleij <linus.walleij@linaro.org> > Date: Wed Dec 27 16:37:44 2017 +0100 > > gpio: of: Support SPI nonstandard GPIO properties I have fixes queued for this, tried to send a pull request yesterday but it turns out the fixes need fixing... OK I'm onto it anyways. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [alsa-devel] regression v4.16 on Nokia N900: sound does not work 2018-02-27 8:43 ` Linus Walleij @ 2018-03-02 9:10 ` Pavel Machek 2018-03-02 9:33 ` Linus Walleij 0 siblings, 1 reply; 20+ messages in thread From: Pavel Machek @ 2018-03-02 9:10 UTC (permalink / raw) To: Linus Walleij Cc: Daniel Baluta, Thorsten Leemhuis, Peter Ujfalusi, Linux-ALSA, Ivajlo Dimitrov, Kevin Hilman, ext Tony Lindgren, Aaro Koskinen, kernel list, Sebastian Reichel, martijn, Filip Matijević, Mark Brown, abcloriens, Sakari Ailus, Pali Rohár, clayton, Linux-OMAP, Andrew F . Davis, patrikbachan, linux-arm-kernel, serge [-- Attachment #1: Type: text/plain, Size: 1087 bytes --] Hi! Linux-Regression-ID: lr#4b650f On Tue 2018-02-27 09:43:32, Linus Walleij wrote: > On Tue, Feb 27, 2018 at 12:13 AM, Pavel Machek <pavel@ucw.cz> wrote: > > > I did git bisect, and the winner seems to be: > > > > pavel@duo:/data/l/linux-n900$ git bisect bad > > c85823390215e52d68d3826df92a447ed31e5c80 is the first bad commit > > commit c85823390215e52d68d3826df92a447ed31e5c80 > > Author: Linus Walleij <linus.walleij@linaro.org> > > Date: Wed Dec 27 16:37:44 2017 +0100 > > > > gpio: of: Support SPI nonstandard GPIO properties > > I have fixes queued for this, tried to send a pull request yesterday > but it turns out the fixes need fixing... OK I'm onto it anyways. Do you have any updates? Is there way to fix dts so that this does not trigger on N900? If this is taking longer to fix, should c85823390215 be reverted in the meantime? It does not seem particulary important/urgent... Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [alsa-devel] regression v4.16 on Nokia N900: sound does not work 2018-03-02 9:10 ` Pavel Machek @ 2018-03-02 9:33 ` Linus Walleij 2018-03-02 10:31 ` Pavel Machek 2018-03-02 11:10 ` Pavel Machek 0 siblings, 2 replies; 20+ messages in thread From: Linus Walleij @ 2018-03-02 9:33 UTC (permalink / raw) To: Pavel Machek Cc: Daniel Baluta, Thorsten Leemhuis, Peter Ujfalusi, Linux-ALSA, Ivajlo Dimitrov, Kevin Hilman, ext Tony Lindgren, Aaro Koskinen, kernel list, Sebastian Reichel, Martijn Braam, Filip Matijević, Mark Brown, Mickuláš Qwertz, Sakari Ailus, Pali Rohár, clayton, Linux-OMAP, Andrew F . Davis, Patrik Bachan, linux-arm-kernel, serge On Fri, Mar 2, 2018 at 10:10 AM, Pavel Machek <pavel@ucw.cz> wrote: > Linux-Regression-ID: lr#4b650f > > On Tue 2018-02-27 09:43:32, Linus Walleij wrote: >> On Tue, Feb 27, 2018 at 12:13 AM, Pavel Machek <pavel@ucw.cz> wrote: >> >> > I did git bisect, and the winner seems to be: >> > >> > pavel@duo:/data/l/linux-n900$ git bisect bad >> > c85823390215e52d68d3826df92a447ed31e5c80 is the first bad commit >> > commit c85823390215e52d68d3826df92a447ed31e5c80 >> > Author: Linus Walleij <linus.walleij@linaro.org> >> > Date: Wed Dec 27 16:37:44 2017 +0100 >> > >> > gpio: of: Support SPI nonstandard GPIO properties >> >> I have fixes queued for this, tried to send a pull request yesterday >> but it turns out the fixes need fixing... OK I'm onto it anyways. > > Do you have any updates? Is there way to fix dts so that this does not > trigger on N900? > > If this is taking longer to fix, should c85823390215 be reverted in > the meantime? It does not seem particulary important/urgent... No patience between the v4.16 release candidates eh ;) commit 6662ae6af82df10259a70c7569b4c12ea7f3ba93 ("gpiolib: Keep returning EPROBE_DEFER when we should") and commit ce27fb2c56db6ccfe8099343bb4afdab15e77e7b ("gpio: Handle deferred probing in of_find_gpio() properly") that are both in Torvalds' tree since yesterday should be fixing this, I think? Did you try just using the upstream HEAD? Yours, Linus Walleij ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [alsa-devel] regression v4.16 on Nokia N900: sound does not work 2018-03-02 9:33 ` Linus Walleij @ 2018-03-02 10:31 ` Pavel Machek 2018-03-02 12:07 ` Linus Walleij 2018-03-02 11:10 ` Pavel Machek 1 sibling, 1 reply; 20+ messages in thread From: Pavel Machek @ 2018-03-02 10:31 UTC (permalink / raw) To: Linus Walleij Cc: Daniel Baluta, Thorsten Leemhuis, Peter Ujfalusi, Linux-ALSA, Ivajlo Dimitrov, Kevin Hilman, ext Tony Lindgren, Aaro Koskinen, kernel list, Sebastian Reichel, Martijn Braam, Filip Matijević, Mark Brown, Mickuláš Qwertz, Sakari Ailus, Pali Rohár, clayton, Linux-OMAP, Andrew F . Davis, Patrik Bachan, linux-arm-kernel, serge [-- Attachment #1: Type: text/plain, Size: 2268 bytes --] On Fri 2018-03-02 10:33:24, Linus Walleij wrote: > On Fri, Mar 2, 2018 at 10:10 AM, Pavel Machek <pavel@ucw.cz> wrote: > > Linux-Regression-ID: lr#4b650f > > > > On Tue 2018-02-27 09:43:32, Linus Walleij wrote: > >> On Tue, Feb 27, 2018 at 12:13 AM, Pavel Machek <pavel@ucw.cz> wrote: > >> > >> > I did git bisect, and the winner seems to be: > >> > > >> > pavel@duo:/data/l/linux-n900$ git bisect bad > >> > c85823390215e52d68d3826df92a447ed31e5c80 is the first bad commit > >> > commit c85823390215e52d68d3826df92a447ed31e5c80 > >> > Author: Linus Walleij <linus.walleij@linaro.org> > >> > Date: Wed Dec 27 16:37:44 2017 +0100 > >> > > >> > gpio: of: Support SPI nonstandard GPIO properties > >> > >> I have fixes queued for this, tried to send a pull request yesterday > >> but it turns out the fixes need fixing... OK I'm onto it anyways. > > > > Do you have any updates? Is there way to fix dts so that this does not > > trigger on N900? > > > > If this is taking longer to fix, should c85823390215 be reverted in > > the meantime? It does not seem particulary important/urgent... > > No patience between the v4.16 release candidates eh ;) > > commit 6662ae6af82df10259a70c7569b4c12ea7f3ba93 > ("gpiolib: Keep returning EPROBE_DEFER when we should") > > and > > commit ce27fb2c56db6ccfe8099343bb4afdab15e77e7b > ("gpio: Handle deferred probing in of_find_gpio() properly") > > that are both in Torvalds' tree since yesterday should be fixing > this, I think? Did you try just using the upstream HEAD? After I spent hours bisecting, I was kind of assuming you'd cc me on merge request or something like that... so that I could test it before going mainline. Which of those should fix it? I tested today's mainline, and... sound fails, different way: pavel@n900:~$ cat /proc/asound/cards 0 [RX51 ]: RX-51 - RX-51 RX-51 pavel@n900:~$ cd g/tui/ofone/ pavel@n900:~/g/tui/ofone$ cd pavel@n900:~$ festival --tts ahoj ^D uname -a (Festival is hung). Let me try the revert again... Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [alsa-devel] regression v4.16 on Nokia N900: sound does not work 2018-03-02 10:31 ` Pavel Machek @ 2018-03-02 12:07 ` Linus Walleij 2018-03-02 12:14 ` Pavel Machek 0 siblings, 1 reply; 20+ messages in thread From: Linus Walleij @ 2018-03-02 12:07 UTC (permalink / raw) To: Pavel Machek Cc: Daniel Baluta, Thorsten Leemhuis, Peter Ujfalusi, Linux-ALSA, Ivajlo Dimitrov, Kevin Hilman, ext Tony Lindgren, Aaro Koskinen, kernel list, Sebastian Reichel, Martijn Braam, Filip Matijević, Mark Brown, Mickuláš Qwertz, Sakari Ailus, Pali Rohár, clayton, Linux-OMAP, Andrew F . Davis, Patrik Bachan, linux-arm-kernel, serge On Fri, Mar 2, 2018 at 11:31 AM, Pavel Machek <pavel@ucw.cz> wrote: > On Fri 2018-03-02 10:33:24, Linus Walleij wrote: >> On Fri, Mar 2, 2018 at 10:10 AM, Pavel Machek <pavel@ucw.cz> wrote: >> > Linux-Regression-ID: lr#4b650f >> > >> > On Tue 2018-02-27 09:43:32, Linus Walleij wrote: >> >> On Tue, Feb 27, 2018 at 12:13 AM, Pavel Machek <pavel@ucw.cz> wrote: >> >> >> >> > I did git bisect, and the winner seems to be: >> >> > >> >> > pavel@duo:/data/l/linux-n900$ git bisect bad >> >> > c85823390215e52d68d3826df92a447ed31e5c80 is the first bad commit >> >> > commit c85823390215e52d68d3826df92a447ed31e5c80 >> >> > Author: Linus Walleij <linus.walleij@linaro.org> >> >> > Date: Wed Dec 27 16:37:44 2017 +0100 >> >> > >> >> > gpio: of: Support SPI nonstandard GPIO properties >> >> >> >> I have fixes queued for this, tried to send a pull request yesterday >> >> but it turns out the fixes need fixing... OK I'm onto it anyways. >> > >> > Do you have any updates? Is there way to fix dts so that this does not >> > trigger on N900? >> > >> > If this is taking longer to fix, should c85823390215 be reverted in >> > the meantime? It does not seem particulary important/urgent... >> >> No patience between the v4.16 release candidates eh ;) >> >> commit 6662ae6af82df10259a70c7569b4c12ea7f3ba93 >> ("gpiolib: Keep returning EPROBE_DEFER when we should") >> >> and >> >> commit ce27fb2c56db6ccfe8099343bb4afdab15e77e7b >> ("gpio: Handle deferred probing in of_find_gpio() properly") >> >> that are both in Torvalds' tree since yesterday should be fixing >> this, I think? Did you try just using the upstream HEAD? > > After I spent hours bisecting, I was kind of assuming you'd cc me on > merge request or something like that... so that I could test it before > going mainline. Sorry, I'm not very good with planning and coordination. I just try my best to keep this working. I guess ideally we should use Bugzilla to track regressions like this, but it also comes with a bit of overhead so I don't know if it helps more than trying to keep things in the head. > Which of those should fix it? The first one I guess, the second is mostly about supporting -EPROBE_DEFER for different use cases. > I tested today's mainline, and... sound fails, different way: > > pavel@n900:~$ cat /proc/asound/cards > 0 [RX51 ]: RX-51 - RX-51 > RX-51 > pavel@n900:~$ cd g/tui/ofone/ > pavel@n900:~/g/tui/ofone$ cd > pavel@n900:~$ festival --tts > ahoj > ^D > uname -a > > (Festival is hung). Sorry but I need some background here, I have no idea what festival is, other than it seems to be some kind of userspace test program? >> Ok, so this code looks pretty crazy to me: I tried removing the >> "of_find_spi_gpio" part, and audio started working. > > Hmm. Looks like audio is working w/o any changes, too. Not sure why > festival hung on me before. Does it mean that mainline is working find for you or do we need to look deeper into the problem in the OF lookup? Yours, Linus Walleij ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [alsa-devel] regression v4.16 on Nokia N900: sound does not work 2018-03-02 12:07 ` Linus Walleij @ 2018-03-02 12:14 ` Pavel Machek 2018-03-02 12:33 ` Linus Walleij 0 siblings, 1 reply; 20+ messages in thread From: Pavel Machek @ 2018-03-02 12:14 UTC (permalink / raw) To: Linus Walleij Cc: Daniel Baluta, Thorsten Leemhuis, Peter Ujfalusi, Linux-ALSA, Ivajlo Dimitrov, Kevin Hilman, ext Tony Lindgren, Aaro Koskinen, kernel list, Sebastian Reichel, Martijn Braam, Filip Matijević, Mark Brown, Mickuláš Qwertz, Sakari Ailus, Pali Rohár, clayton, Linux-OMAP, Andrew F . Davis, Patrik Bachan, linux-arm-kernel, serge [-- Attachment #1: Type: text/plain, Size: 693 bytes --] Hi! > >> Ok, so this code looks pretty crazy to me: I tried removing the > >> "of_find_spi_gpio" part, and audio started working. > > > > Hmm. Looks like audio is working w/o any changes, too. Not sure why > > festival hung on me before. > > Does it mean that mainline is working find for you or > do we need to look deeper into the problem in the OF > lookup? It now works for me. (But take a look at the patch I sent -- I believe you are still overwriting return values in a bad way, and code is quite confusing). Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [alsa-devel] regression v4.16 on Nokia N900: sound does not work 2018-03-02 12:14 ` Pavel Machek @ 2018-03-02 12:33 ` Linus Walleij 0 siblings, 0 replies; 20+ messages in thread From: Linus Walleij @ 2018-03-02 12:33 UTC (permalink / raw) To: Pavel Machek Cc: Daniel Baluta, Thorsten Leemhuis, Peter Ujfalusi, Linux-ALSA, Ivajlo Dimitrov, Kevin Hilman, ext Tony Lindgren, Aaro Koskinen, kernel list, Sebastian Reichel, Martijn Braam, Filip Matijević, Mark Brown, Mickuláš Qwertz, Sakari Ailus, Pali Rohár, clayton, Linux-OMAP, Andrew F . Davis, Patrik Bachan, linux-arm-kernel, serge On Fri, Mar 2, 2018 at 1:14 PM, Pavel Machek <pavel@ucw.cz> wrote: >> >> Ok, so this code looks pretty crazy to me: I tried removing the >> >> "of_find_spi_gpio" part, and audio started working. >> > >> > Hmm. Looks like audio is working w/o any changes, too. Not sure why >> > festival hung on me before. >> >> Does it mean that mainline is working find for you or >> do we need to look deeper into the problem in the OF >> lookup? > > It now works for me. > > (But take a look at the patch I sent -- I believe you are still > overwriting return values in a bad way, and code is quite confusing). I think that code is a result of piled fixes, so noone really designed it to look like that. I'll try to look at it, I am a bit afraid of the code since I've broken so many things with it. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [alsa-devel] regression v4.16 on Nokia N900: sound does not work 2018-03-02 9:33 ` Linus Walleij 2018-03-02 10:31 ` Pavel Machek @ 2018-03-02 11:10 ` Pavel Machek 2018-03-02 11:21 ` Pavel Machek 2018-03-02 14:22 ` Andrew F. Davis 1 sibling, 2 replies; 20+ messages in thread From: Pavel Machek @ 2018-03-02 11:10 UTC (permalink / raw) To: Linus Walleij Cc: Daniel Baluta, Thorsten Leemhuis, Peter Ujfalusi, Linux-ALSA, Ivajlo Dimitrov, Kevin Hilman, ext Tony Lindgren, Aaro Koskinen, kernel list, Sebastian Reichel, Martijn Braam, Filip Matijević, Mark Brown, Mickuláš Qwertz, Sakari Ailus, Pali Rohár, clayton, Linux-OMAP, Andrew F . Davis, Patrik Bachan, linux-arm-kernel, serge [-- Attachment #1: Type: text/plain, Size: 3436 bytes --] Hi! > > If this is taking longer to fix, should c85823390215 be reverted in > > the meantime? It does not seem particulary important/urgent... > > No patience between the v4.16 release candidates eh ;) > > commit 6662ae6af82df10259a70c7569b4c12ea7f3ba93 > ("gpiolib: Keep returning EPROBE_DEFER when we should") > > and > > commit ce27fb2c56db6ccfe8099343bb4afdab15e77e7b > ("gpio: Handle deferred probing in of_find_gpio() properly") > > that are both in Torvalds' tree since yesterday should be fixing > this, I think? Did you try just using the upstream HEAD? Ok, so this code looks pretty crazy to me: I tried removing the "of_find_spi_gpio" part, and audio started working. What is going on with the ()s around == s? You made me look up C operator precedence. Hmm, and it is also wrong, right? It turns any error code into ENOENT, as it tries to do the "special handling". * * This means we don't need to look any further for * alternate name conventions, and we should really * preserve the return code for our user to be able to * retry probing later. */ if (IS_ERR(desc) && PTR_ERR(desc) == -EPROBE_DEFER) return desc; if (!IS_ERR(desc) || (PTR_ERR(desc) != -ENOENT)) break; } /* Special handling for SPI GPIOs if used */ if (IS_ERR(desc)) desc = of_find_spi_gpio(dev, con_id, &of_flags); /* Special handling for regulator GPIOs if used */ if (IS_ERR(desc) && PTR_ERR(desc) != -EPROBE_DEFER) desc = of_find_regulator_gpio(dev, con_id, &of_flags); Something like this? diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c index 84e5a9d..f0fab26 100644 --- a/drivers/gpio/gpiolib-of.c +++ b/drivers/gpio/gpiolib-of.c @@ -241,29 +241,17 @@ struct gpio_desc *of_find_gpio(struct device *dev, const char *con_id, desc = of_get_named_gpiod_flags(dev->of_node, prop_name, idx, &of_flags); - /* - * -EPROBE_DEFER in our case means that we found a - * valid GPIO property, but no controller has been - * registered so far. - * - * This means we don't need to look any further for - * alternate name conventions, and we should really - * preserve the return code for our user to be able to - * retry probing later. - */ - if (IS_ERR(desc) && PTR_ERR(desc) == -EPROBE_DEFER) - return desc; - if (!IS_ERR(desc) || (PTR_ERR(desc) != -ENOENT)) + if (!IS_ERR(desc) || PTR_ERR(desc) != -ENOENT) break; } /* Special handling for SPI GPIOs if used */ - if (IS_ERR(desc)) + if (IS_ERR(desc) && PTR_ERR(desc) == -ENOENT) desc = of_find_spi_gpio(dev, con_id, &of_flags); /* Special handling for regulator GPIOs if used */ - if (IS_ERR(desc) && PTR_ERR(desc) != -EPROBE_DEFER) + if (IS_ERR(desc) && PTR_ERR(desc) == -ENOENT) desc = of_find_regulator_gpio(dev, con_id, &of_flags); if (IS_ERR(desc)) -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [alsa-devel] regression v4.16 on Nokia N900: sound does not work 2018-03-02 11:10 ` Pavel Machek @ 2018-03-02 11:21 ` Pavel Machek 2018-03-02 14:22 ` Andrew F. Davis 1 sibling, 0 replies; 20+ messages in thread From: Pavel Machek @ 2018-03-02 11:21 UTC (permalink / raw) To: Linus Walleij Cc: Daniel Baluta, Thorsten Leemhuis, Peter Ujfalusi, Linux-ALSA, Ivajlo Dimitrov, Kevin Hilman, ext Tony Lindgren, Aaro Koskinen, kernel list, Sebastian Reichel, Martijn Braam, Filip Matijević, Mark Brown, Mickuláš Qwertz, Sakari Ailus, Pali Rohár, clayton, Linux-OMAP, Andrew F . Davis, Patrik Bachan, linux-arm-kernel, serge [-- Attachment #1: Type: text/plain, Size: 1050 bytes --] On Fri 2018-03-02 12:10:40, Pavel Machek wrote: > Hi! > > > > If this is taking longer to fix, should c85823390215 be reverted in > > > the meantime? It does not seem particulary important/urgent... > > > > No patience between the v4.16 release candidates eh ;) > > > > commit 6662ae6af82df10259a70c7569b4c12ea7f3ba93 > > ("gpiolib: Keep returning EPROBE_DEFER when we should") > > > > and > > > > commit ce27fb2c56db6ccfe8099343bb4afdab15e77e7b > > ("gpio: Handle deferred probing in of_find_gpio() properly") > > > > that are both in Torvalds' tree since yesterday should be fixing > > this, I think? Did you try just using the upstream HEAD? > > Ok, so this code looks pretty crazy to me: I tried removing the > "of_find_spi_gpio" part, and audio started working. Hmm. Looks like audio is working w/o any changes, too. Not sure why festival hung on me before. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [alsa-devel] regression v4.16 on Nokia N900: sound does not work 2018-03-02 11:10 ` Pavel Machek 2018-03-02 11:21 ` Pavel Machek @ 2018-03-02 14:22 ` Andrew F. Davis 2018-03-02 16:53 ` Pavel Machek 2018-03-02 17:08 ` Russell King - ARM Linux 1 sibling, 2 replies; 20+ messages in thread From: Andrew F. Davis @ 2018-03-02 14:22 UTC (permalink / raw) To: Pavel Machek, Linus Walleij Cc: Daniel Baluta, Thorsten Leemhuis, Peter Ujfalusi, Linux-ALSA, Ivajlo Dimitrov, Kevin Hilman, ext Tony Lindgren, Aaro Koskinen, kernel list, Sebastian Reichel, Martijn Braam, Filip Matijević, Mark Brown, Mickuláš Qwertz, Sakari Ailus, Pali Rohár, clayton, Linux-OMAP, Patrik Bachan, linux-arm-kernel, serge On 03/02/2018 05:10 AM, Pavel Machek wrote: > Hi! > >>> If this is taking longer to fix, should c85823390215 be reverted in >>> the meantime? It does not seem particulary important/urgent... >> >> No patience between the v4.16 release candidates eh ;) >> >> commit 6662ae6af82df10259a70c7569b4c12ea7f3ba93 >> ("gpiolib: Keep returning EPROBE_DEFER when we should") >> >> and >> >> commit ce27fb2c56db6ccfe8099343bb4afdab15e77e7b >> ("gpio: Handle deferred probing in of_find_gpio() properly") >> >> that are both in Torvalds' tree since yesterday should be fixing >> this, I think? Did you try just using the upstream HEAD? > > Ok, so this code looks pretty crazy to me: I tried removing the > "of_find_spi_gpio" part, and audio started working. > > What is going on with the ()s around == s? You made me look up C > operator precedence. > > Hmm, and it is also wrong, right? It turns any error code into ENOENT, > as it tries to do the "special handling". > > * > * This means we don't need to look any further for > * alternate name conventions, and we should really > * preserve the return code for our user to be able to > * retry probing later. > */ > if (IS_ERR(desc) && PTR_ERR(desc) == -EPROBE_DEFER) > return desc; > > if (!IS_ERR(desc) || (PTR_ERR(desc) != -ENOENT)) > break; > } > > /* Special handling for SPI GPIOs if used */ > if (IS_ERR(desc)) > desc = of_find_spi_gpio(dev, con_id, &of_flags); > > /* Special handling for regulator GPIOs if used */ > if (IS_ERR(desc) && PTR_ERR(desc) != -EPROBE_DEFER) > desc = of_find_regulator_gpio(dev, con_id, &of_flags); > > Something like this? > > diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c > index 84e5a9d..f0fab26 100644 > --- a/drivers/gpio/gpiolib-of.c > +++ b/drivers/gpio/gpiolib-of.c > @@ -241,29 +241,17 @@ struct gpio_desc *of_find_gpio(struct device *dev, const char *con_id, > > desc = of_get_named_gpiod_flags(dev->of_node, prop_name, idx, > &of_flags); > - /* > - * -EPROBE_DEFER in our case means that we found a > - * valid GPIO property, but no controller has been > - * registered so far. > - * > - * This means we don't need to look any further for > - * alternate name conventions, and we should really > - * preserve the return code for our user to be able to > - * retry probing later. > - */ > - if (IS_ERR(desc) && PTR_ERR(desc) == -EPROBE_DEFER) > - return desc; > > - if (!IS_ERR(desc) || (PTR_ERR(desc) != -ENOENT)) > + if (!IS_ERR(desc) || PTR_ERR(desc) != -ENOENT) I rather like the () so one doesn't always have to look up C operator precedence to verify.. > break; > } > > /* Special handling for SPI GPIOs if used */ > - if (IS_ERR(desc)) > + if (IS_ERR(desc) && PTR_ERR(desc) == -ENOENT) > desc = of_find_spi_gpio(dev, con_id, &of_flags); > > /* Special handling for regulator GPIOs if used */ > - if (IS_ERR(desc) && PTR_ERR(desc) != -EPROBE_DEFER) > + if (IS_ERR(desc) && PTR_ERR(desc) == -ENOENT) > desc = of_find_regulator_gpio(dev, con_id, &of_flags); > > if (IS_ERR(desc)) > ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [alsa-devel] regression v4.16 on Nokia N900: sound does not work 2018-03-02 14:22 ` Andrew F. Davis @ 2018-03-02 16:53 ` Pavel Machek 2018-03-02 17:08 ` Russell King - ARM Linux 1 sibling, 0 replies; 20+ messages in thread From: Pavel Machek @ 2018-03-02 16:53 UTC (permalink / raw) To: Andrew F. Davis Cc: Linus Walleij, Daniel Baluta, Thorsten Leemhuis, Peter Ujfalusi, Linux-ALSA, Ivajlo Dimitrov, Kevin Hilman, ext Tony Lindgren, Aaro Koskinen, kernel list, Sebastian Reichel, Martijn Braam, Filip Matijević, Mark Brown, Mickuláš Qwertz, Sakari Ailus, Pali Rohár, clayton, Linux-OMAP, Patrik Bachan, linux-arm-kernel, serge [-- Attachment #1: Type: text/plain, Size: 4070 bytes --] On Fri 2018-03-02 08:22:52, Andrew F. Davis wrote: > On 03/02/2018 05:10 AM, Pavel Machek wrote: > > Hi! > > > >>> If this is taking longer to fix, should c85823390215 be reverted in > >>> the meantime? It does not seem particulary important/urgent... > >> > >> No patience between the v4.16 release candidates eh ;) > >> > >> commit 6662ae6af82df10259a70c7569b4c12ea7f3ba93 > >> ("gpiolib: Keep returning EPROBE_DEFER when we should") > >> > >> and > >> > >> commit ce27fb2c56db6ccfe8099343bb4afdab15e77e7b > >> ("gpio: Handle deferred probing in of_find_gpio() properly") > >> > >> that are both in Torvalds' tree since yesterday should be fixing > >> this, I think? Did you try just using the upstream HEAD? > > > > Ok, so this code looks pretty crazy to me: I tried removing the > > "of_find_spi_gpio" part, and audio started working. > > > > What is going on with the ()s around == s? You made me look up C > > operator precedence. > > > > Hmm, and it is also wrong, right? It turns any error code into ENOENT, > > as it tries to do the "special handling". > > > > * > > * This means we don't need to look any further for > > * alternate name conventions, and we should really > > * preserve the return code for our user to be able to > > * retry probing later. > > */ > > if (IS_ERR(desc) && PTR_ERR(desc) == -EPROBE_DEFER) > > return desc; > > > > if (!IS_ERR(desc) || (PTR_ERR(desc) != -ENOENT)) > > break; > > } > > > > /* Special handling for SPI GPIOs if used */ > > if (IS_ERR(desc)) > > desc = of_find_spi_gpio(dev, con_id, &of_flags); > > > > /* Special handling for regulator GPIOs if used */ > > if (IS_ERR(desc) && PTR_ERR(desc) != -EPROBE_DEFER) > > desc = of_find_regulator_gpio(dev, con_id, &of_flags); > > > > Something like this? > > > > diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c > > index 84e5a9d..f0fab26 100644 > > --- a/drivers/gpio/gpiolib-of.c > > +++ b/drivers/gpio/gpiolib-of.c > > @@ -241,29 +241,17 @@ struct gpio_desc *of_find_gpio(struct device *dev, const char *con_id, > > > > desc = of_get_named_gpiod_flags(dev->of_node, prop_name, idx, > > &of_flags); > > - /* > > - * -EPROBE_DEFER in our case means that we found a > > - * valid GPIO property, but no controller has been > > - * registered so far. > > - * > > - * This means we don't need to look any further for > > - * alternate name conventions, and we should really > > - * preserve the return code for our user to be able to > > - * retry probing later. > > - */ > > - if (IS_ERR(desc) && PTR_ERR(desc) == -EPROBE_DEFER) > > - return desc; > > > > - if (!IS_ERR(desc) || (PTR_ERR(desc) != -ENOENT)) > > + if (!IS_ERR(desc) || PTR_ERR(desc) != -ENOENT) > > > I rather like the () so one doesn't always have to look up C operator > precedence to verify.. Well, Ok, but then the ()s should be at the other ifs nearby, too. See? > > > break; > > } > > > > /* Special handling for SPI GPIOs if used */ > > - if (IS_ERR(desc)) > > + if (IS_ERR(desc) && PTR_ERR(desc) == -ENOENT) > > desc = of_find_spi_gpio(dev, con_id, &of_flags); > > > > /* Special handling for regulator GPIOs if used */ > > - if (IS_ERR(desc) && PTR_ERR(desc) != -EPROBE_DEFER) > > + if (IS_ERR(desc) && PTR_ERR(desc) == -ENOENT) > > desc = of_find_regulator_gpio(dev, con_id, &of_flags); > > > > if (IS_ERR(desc)) > > Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [alsa-devel] regression v4.16 on Nokia N900: sound does not work 2018-03-02 14:22 ` Andrew F. Davis 2018-03-02 16:53 ` Pavel Machek @ 2018-03-02 17:08 ` Russell King - ARM Linux 2018-03-02 17:18 ` Andrew F. Davis 1 sibling, 1 reply; 20+ messages in thread From: Russell King - ARM Linux @ 2018-03-02 17:08 UTC (permalink / raw) To: Andrew F. Davis Cc: Pavel Machek, Linus Walleij, Martijn Braam, Daniel Baluta, Ivajlo Dimitrov, Kevin Hilman, ext Tony Lindgren, Mark Brown, Aaro Koskinen, Pali Rohár, Linux-ALSA, kernel list, Peter Ujfalusi, Filip Matijević, Thorsten Leemhuis, Mickuláš Qwertz, Sakari Ailus, Sebastian Reichel, clayton, Linux-OMAP, Patrik Bachan, linux-arm-kernel, serge On Fri, Mar 02, 2018 at 08:22:52AM -0600, Andrew F. Davis wrote: > > diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c > > index 84e5a9d..f0fab26 100644 > > --- a/drivers/gpio/gpiolib-of.c > > +++ b/drivers/gpio/gpiolib-of.c > > @@ -241,29 +241,17 @@ struct gpio_desc *of_find_gpio(struct device *dev, const char *con_id, > > > > desc = of_get_named_gpiod_flags(dev->of_node, prop_name, idx, > > &of_flags); > > - /* > > - * -EPROBE_DEFER in our case means that we found a > > - * valid GPIO property, but no controller has been > > - * registered so far. > > - * > > - * This means we don't need to look any further for > > - * alternate name conventions, and we should really > > - * preserve the return code for our user to be able to > > - * retry probing later. > > - */ > > - if (IS_ERR(desc) && PTR_ERR(desc) == -EPROBE_DEFER) > > - return desc; > > > > - if (!IS_ERR(desc) || (PTR_ERR(desc) != -ENOENT)) > > + if (!IS_ERR(desc) || PTR_ERR(desc) != -ENOENT) > > > I rather like the () so one doesn't always have to look up C operator > precedence to verify.. Too many make it impossible to see which close paren ties up with which open paren. I've spent way too long in the past reformatting code, where people think that () are a good thing, to work out what the comparison is actually doing before then rewriting the damn thing with less () and in a much clearer way. I'm now convinced that unnecessary () are a very bad thing as they severely harm readability as test complexity increases. > > > > break; > > } > > > > /* Special handling for SPI GPIOs if used */ > > - if (IS_ERR(desc)) > > + if (IS_ERR(desc) && PTR_ERR(desc) == -ENOENT) These can be simplified to: if (desc == ERR_PTR(-ENOENT)) since error pointers are unique - ERR_PTR(-ENOENT) is what was returned as an error-pointer, and if IS_ERR(ERR_PTR(-ENOMENT)) etc were false, we'd have big problems as it'd mean we couldn't detect error pointers! As an added bonus, they don't involve any operator precedence questions either. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up According to speedtest.net: 8.21Mbps down 510kbps up ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [alsa-devel] regression v4.16 on Nokia N900: sound does not work 2018-03-02 17:08 ` Russell King - ARM Linux @ 2018-03-02 17:18 ` Andrew F. Davis 0 siblings, 0 replies; 20+ messages in thread From: Andrew F. Davis @ 2018-03-02 17:18 UTC (permalink / raw) To: Russell King - ARM Linux Cc: Pavel Machek, Linus Walleij, Martijn Braam, Daniel Baluta, Ivajlo Dimitrov, Kevin Hilman, ext Tony Lindgren, Mark Brown, Aaro Koskinen, Pali Rohár, Linux-ALSA, kernel list, Peter Ujfalusi, Filip Matijević, Thorsten Leemhuis, Mickuláš Qwertz, Sakari Ailus, Sebastian Reichel, clayton, Linux-OMAP, Patrik Bachan, linux-arm-kernel, serge On 03/02/2018 11:08 AM, Russell King - ARM Linux wrote: > On Fri, Mar 02, 2018 at 08:22:52AM -0600, Andrew F. Davis wrote: >>> diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c >>> index 84e5a9d..f0fab26 100644 >>> --- a/drivers/gpio/gpiolib-of.c >>> +++ b/drivers/gpio/gpiolib-of.c >>> @@ -241,29 +241,17 @@ struct gpio_desc *of_find_gpio(struct device *dev, const char *con_id, >>> >>> desc = of_get_named_gpiod_flags(dev->of_node, prop_name, idx, >>> &of_flags); >>> - /* >>> - * -EPROBE_DEFER in our case means that we found a >>> - * valid GPIO property, but no controller has been >>> - * registered so far. >>> - * >>> - * This means we don't need to look any further for >>> - * alternate name conventions, and we should really >>> - * preserve the return code for our user to be able to >>> - * retry probing later. >>> - */ >>> - if (IS_ERR(desc) && PTR_ERR(desc) == -EPROBE_DEFER) >>> - return desc; >>> >>> - if (!IS_ERR(desc) || (PTR_ERR(desc) != -ENOENT)) >>> + if (!IS_ERR(desc) || PTR_ERR(desc) != -ENOENT) >> >> >> I rather like the () so one doesn't always have to look up C operator >> precedence to verify.. > > Too many make it impossible to see which close paren ties up with > which open paren. I've spent way too long in the past reformatting > code, where people think that () are a good thing, to work out what > the comparison is actually doing before then rewriting the damn > thing with less () and in a much clearer way. I'm now convinced > that unnecessary () are a very bad thing as they severely harm > readability as test complexity increases. > Fair enough, I personally prefer always having a new line per condition when doing chained conditionals: if (something && this == that && !not_this) >> >> >>> break; >>> } >>> >>> /* Special handling for SPI GPIOs if used */ >>> - if (IS_ERR(desc)) >>> + if (IS_ERR(desc) && PTR_ERR(desc) == -ENOENT) > > These can be simplified to: > > if (desc == ERR_PTR(-ENOENT)) > > since error pointers are unique - ERR_PTR(-ENOENT) is what was > returned as an error-pointer, and if IS_ERR(ERR_PTR(-ENOMENT)) etc > were false, we'd have big problems as it'd mean we couldn't detect > error pointers! > > As an added bonus, they don't involve any operator precedence > questions either. > Looks like your suggestion clears up this one anyway. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: regression v4.16 on Nokia N900:/dev/input/event6 aka AV Jack support disappeared 2018-02-24 21:46 regression v4.16 on Nokia N900:/dev/input/event6 aka AV Jack support disappeared Pavel Machek 2018-02-26 9:47 ` Thorsten Leemhuis @ 2018-02-26 15:43 ` Andrew F. Davis 1 sibling, 0 replies; 20+ messages in thread From: Andrew F. Davis @ 2018-02-26 15:43 UTC (permalink / raw) To: Pavel Machek, pali.rohar, sre, kernel list, linux-arm-kernel, linux-omap, tony, khilman, aaro.koskinen, ivo.g.dimitrov.75, patrikbachan, serge, abcloriens, clayton, martijn, sakari.ailus, Filip Matijević Cc: broonie, peter.ujfalusi, alsa-devel On 02/24/2018 03:46 PM, Pavel Machek wrote: > Hi! > > In v4.16, AV jack support disappeared. > > This one is suspect: > > commit 7be4b5dc7ffa9499ac6ef33a5ffa9ff43f9b7057 > Author: Andrew F. Davis <afd@ti.com> > Date: Wed Nov 29 11:13:59 2017 -0600 > > ARM: dts: omap3-n900: Fix the audio CODEC's reset pin > > The correct DT property for specifying a GPIO used for reset > is "reset-gpios", fix this here. > > Fixes: 14e3e295b2b9 ("ARM: dts: omap3-n900: Add TLV320AIC3X > support") > > Signed-off-by: Andrew F. Davis <afd@ti.com> > Signed-off-by: Tony Lindgren <tony@atomide.com> > > How was it tested? It reverts polarity of reset pin, but > sound/soc/codecs/tlv320aic3x.c treats those as aliases: > The polarity was wrong before, the AIC3x devices are active low reset and there is no logic inverters between the SoC and the reset line. GPIO_ACTIVE_LOW is therefor the correct polarity. This does not change the driver behavior as currently it uses the old gpio_set_value() which does not respect the polarity as set in DT. When this driver is converted to use gpiod_ style calls having the polarity correct will be important, and right now the old style gpio calls will cause this driver to fail to work if a board comes along that does have some logic inversion on the reset line as GPIO_ACTIVE is ignored. As Pavel points out I think your trouble is elsewhere. Andrew > ret = of_get_named_gpio(np, "reset-gpios", 0); > if (ret >= 0) { > aic3x->gpio_reset = ret; > } else { > ret = of_get_named_gpio(np, "gpio-reset", 0); > if (ret > 0) { > dev_warn(&i2c->dev, "Using deprecated property \"gpio-r\eset\", please update your DT"); > aic3x->gpio_reset = ret; > > > Pavel > ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2018-03-02 17:19 UTC | newest] Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-02-24 21:46 regression v4.16 on Nokia N900:/dev/input/event6 aka AV Jack support disappeared Pavel Machek 2018-02-26 9:47 ` Thorsten Leemhuis 2018-02-26 13:13 ` regression v4.16 on Nokia N900: sound does not work Pavel Machek 2018-02-26 14:02 ` [alsa-devel] " Daniel Baluta 2018-02-26 23:13 ` Pavel Machek 2018-02-26 23:30 ` Pavel Machek 2018-02-27 8:43 ` Linus Walleij 2018-03-02 9:10 ` Pavel Machek 2018-03-02 9:33 ` Linus Walleij 2018-03-02 10:31 ` Pavel Machek 2018-03-02 12:07 ` Linus Walleij 2018-03-02 12:14 ` Pavel Machek 2018-03-02 12:33 ` Linus Walleij 2018-03-02 11:10 ` Pavel Machek 2018-03-02 11:21 ` Pavel Machek 2018-03-02 14:22 ` Andrew F. Davis 2018-03-02 16:53 ` Pavel Machek 2018-03-02 17:08 ` Russell King - ARM Linux 2018-03-02 17:18 ` Andrew F. Davis 2018-02-26 15:43 ` regression v4.16 on Nokia N900:/dev/input/event6 aka AV Jack support disappeared Andrew F. Davis
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).