* [BUG] SPI broken for SPI based panel drivers @ 2020-11-30 19:03 H. Nikolaus Schaller 2020-11-30 20:13 ` Sven Van Asbroeck 0 siblings, 1 reply; 42+ messages in thread From: H. Nikolaus Schaller @ 2020-11-30 19:03 UTC (permalink / raw) To: Sven Van Asbroeck Cc: Linus Walleij, Mark Brown, kernel list, Laurent Pinchart, Discussions about the Letux Kernel Hi, starting from v5.10-rc5 the SPI based panel of the GTA04 device is broken. It uses compatible = "spi-gpio"; [1] i.e. gpio descriptors very indirectly. Bisect shows that it is commit 766c6b63aa04 ("spi: fix client driver breakages when using GPIO descriptors") The commit description tells about a problematic pattern and indeed the driver is using it - like ca. 15 other spi based panel drivers in drivers/gpu/drm/panel/ I understood that it wants to fix the spi system to handle that correctly again. But reverting your patch brings back the display. So it appears as if it does not fix a breakage, rather breaks a previously working setup. What should we do? BR and thanks, Nikolaus [1]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/omap3-gta04.dtsi?h=v5.10-rc6#n107 ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [BUG] SPI broken for SPI based panel drivers 2020-11-30 19:03 [BUG] SPI broken for SPI based panel drivers H. Nikolaus Schaller @ 2020-11-30 20:13 ` Sven Van Asbroeck 2020-11-30 20:22 ` Sven Van Asbroeck ` (2 more replies) 0 siblings, 3 replies; 42+ messages in thread From: Sven Van Asbroeck @ 2020-11-30 20:13 UTC (permalink / raw) To: H. Nikolaus Schaller Cc: Linus Walleij, Mark Brown, kernel list, Laurent Pinchart, Discussions about the Letux Kernel Hi Nikolaus, thank you for reaching out ! On Mon, Nov 30, 2020 at 2:06 PM H. Nikolaus Schaller <hns@goldelico.com> wrote: > > But reverting your patch brings back the display. So it appears as if it does not > fix a breakage, rather breaks a previously working setup. The patch in question fixes an important breakage: before the patch, literally hundreds of SPI drivers no longer worked - only if the SPI bus master driver was using gpio descriptors. We knew that there was a chance that our fix would break something else. But hopefully "it fixes more than it breaks" > > What should we do? > Can you try the following patch ? diff --git a/drivers/spi/spi-gpio.c b/drivers/spi/spi-gpio.c index 7ceb0ba27b75..c173d7de73b3 100644 --- a/drivers/spi/spi-gpio.c +++ b/drivers/spi/spi-gpio.c @@ -208,8 +208,8 @@ static void spi_gpio_chipselect(struct spi_device *spi, int is_active) if (spi_gpio->cs_gpios) { struct gpio_desc *cs = spi_gpio->cs_gpios[spi->chip_select]; - /* SPI chip selects are normally active-low */ - gpiod_set_value_cansleep(cs, (spi->mode & SPI_CS_HIGH) ? is_active : !is_active); + /* SPI chip select polarity handled by gpiolib*/ + gpiod_set_value_cansleep(cs, is_active); } } ^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: [BUG] SPI broken for SPI based panel drivers 2020-11-30 20:13 ` Sven Van Asbroeck @ 2020-11-30 20:22 ` Sven Van Asbroeck 2020-12-01 8:59 ` H. Nikolaus Schaller 2020-12-05 20:57 ` Pavel Machek 2 siblings, 0 replies; 42+ messages in thread From: Sven Van Asbroeck @ 2020-11-30 20:22 UTC (permalink / raw) To: H. Nikolaus Schaller Cc: Linus Walleij, Mark Brown, kernel list, Laurent Pinchart, Discussions about the Letux Kernel And probably also: @@ -226,8 +226,7 @@ static int spi_gpio_setup(struct spi_device *spi) if (spi_gpio->cs_gpios) { cs = spi_gpio->cs_gpios[spi->chip_select]; if (!spi->controller_state && cs) - status = gpiod_direction_output(cs, - !(spi->mode & SPI_CS_HIGH)); + status = gpiod_direction_output(cs, false); } if (!status) ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [BUG] SPI broken for SPI based panel drivers 2020-11-30 20:13 ` Sven Van Asbroeck 2020-11-30 20:22 ` Sven Van Asbroeck @ 2020-12-01 8:59 ` H. Nikolaus Schaller 2020-12-01 12:16 ` Mark Brown 2020-12-01 12:41 ` Sven Van Asbroeck 2020-12-05 20:57 ` Pavel Machek 2 siblings, 2 replies; 42+ messages in thread From: H. Nikolaus Schaller @ 2020-12-01 8:59 UTC (permalink / raw) To: Sven Van Asbroeck Cc: Linus Walleij, Mark Brown, kernel list, Laurent Pinchart, Discussions about the Letux Kernel Hi Sven, > Am 30.11.2020 um 21:13 schrieb Sven Van Asbroeck <thesven73@gmail.com>: > > Hi Nikolaus, thank you for reaching out ! > > On Mon, Nov 30, 2020 at 2:06 PM H. Nikolaus Schaller <hns@goldelico.com> wrote: >> >> But reverting your patch brings back the display. So it appears as if it does not >> fix a breakage, rather breaks a previously working setup. > > The patch in question fixes an important breakage: before the patch, literally > hundreds of SPI drivers no longer worked - only if the SPI bus master > driver was using gpio descriptors. > > We knew that there was a chance that our fix would break something else. > But hopefully "it fixes more than it breaks" Then it should not have been applied to mainline but fully worked out and tested. > >> >> What should we do? >> > > Can you try the following patch ? Unfortunately it doesn't seem to fix it. And combined with the second proposed fix also not. BR and thanks, Nikolaus my combined change: diff --git a/drivers/spi/spi-gpio.c b/drivers/spi/spi-gpio.c index 7ceb0ba27b755c..ec2da62716a279 100644 --- a/drivers/spi/spi-gpio.c +++ b/drivers/spi/spi-gpio.c @@ -208,8 +208,8 @@ static void spi_gpio_chipselect(struct spi_device *spi, int is_active) if (spi_gpio->cs_gpios) { struct gpio_desc *cs = spi_gpio->cs_gpios[spi->chip_select]; - /* SPI chip selects are normally active-low */ - gpiod_set_value_cansleep(cs, (spi->mode & SPI_CS_HIGH) ? is_active : !is_active); + /* SPI chip select polarity handled by gpiolib*/ + gpiod_set_value_cansleep(cs, is_active); } } @@ -226,8 +226,7 @@ static int spi_gpio_setup(struct spi_device *spi) if (spi_gpio->cs_gpios) { cs = spi_gpio->cs_gpios[spi->chip_select]; if (!spi->controller_state && cs) - status = gpiod_direction_output(cs, - !(spi->mode & SPI_CS_HIGH)); + status = gpiod_direction_output(cs, false); } if (!status) ^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: [BUG] SPI broken for SPI based panel drivers 2020-12-01 8:59 ` H. Nikolaus Schaller @ 2020-12-01 12:16 ` Mark Brown 2020-12-01 14:05 ` H. Nikolaus Schaller 2020-12-01 12:41 ` Sven Van Asbroeck 1 sibling, 1 reply; 42+ messages in thread From: Mark Brown @ 2020-12-01 12:16 UTC (permalink / raw) To: H. Nikolaus Schaller Cc: Sven Van Asbroeck, Linus Walleij, kernel list, Laurent Pinchart, Discussions about the Letux Kernel [-- Attachment #1: Type: text/plain, Size: 829 bytes --] On Tue, Dec 01, 2020 at 09:59:43AM +0100, H. Nikolaus Schaller wrote: > > Am 30.11.2020 um 21:13 schrieb Sven Van Asbroeck <thesven73@gmail.com>: > > We knew that there was a chance that our fix would break something else. > > But hopefully "it fixes more than it breaks" > Then it should not have been applied to mainline but fully worked out > and tested. If you want to see better testing of things in mainline please contribute to the various kernel testing efforts that are out there, there's a huge range of systems out there using the kernel and it's simply not realistic to expect that they'll all be covered, the testing effort for the kernel is very much a community effort. If there are things that are particularly important to you the best way to ensure that they are tested is to provide that testing yourself. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [BUG] SPI broken for SPI based panel drivers 2020-12-01 12:16 ` Mark Brown @ 2020-12-01 14:05 ` H. Nikolaus Schaller 2020-12-01 14:20 ` Linus Walleij 0 siblings, 1 reply; 42+ messages in thread From: H. Nikolaus Schaller @ 2020-12-01 14:05 UTC (permalink / raw) To: Mark Brown Cc: Sven Van Asbroeck, Linus Walleij, kernel list, Laurent Pinchart, Discussions about the Letux Kernel > Am 01.12.2020 um 13:16 schrieb Mark Brown <broonie@kernel.org>: > > On Tue, Dec 01, 2020 at 09:59:43AM +0100, H. Nikolaus Schaller wrote: >>> Am 30.11.2020 um 21:13 schrieb Sven Van Asbroeck <thesven73@gmail.com>: > >>> We knew that there was a chance that our fix would break something else. >>> But hopefully "it fixes more than it breaks" > >> Then it should not have been applied to mainline but fully worked out >> and tested. > > If you want to see better testing of things in mainline please > contribute to the various kernel testing efforts that are out there, > there's a huge range of systems out there using the kernel and it's > simply not realistic to expect that they'll all be covered, the testing > effort for the kernel is very much a community effort. If there are > things that are particularly important to you the best way to ensure > that they are tested is to provide that testing yourself. Well, having a working display of a portable device that has mainline Linux support for many years is particularly important... BTW, I am already part of this testing efforts out there. Because I test almost all -rc when they arrive. So I just did what you propose, And if I can locate the commit of a regression I write bug reports like this one. The only thing I could have done differently is to always test based on linux-next but that is a less structured testing environment and has its own pitfalls. But that would have revealed the issue only earlier but not with less effort or with a quicker fix. I am not sure if ít is realistic to dream of some way of informing/contacting the potentially affected driver authors/maintainers to run a quick test before it is merged upstream. To be clear: I do not expect that there are no bugs at all (for that I know Linux and software far too long). But I do not expect regression of the type hopefully "it fixes more than it breaks". Well, for me it (apparently) breaks more than it fixes. So the easiest fix for me would be to revert the patch. But I am sure that then you are not happy with it... So let's set out for a better solution. BR and thanks, Nikolaus ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [BUG] SPI broken for SPI based panel drivers 2020-12-01 14:05 ` H. Nikolaus Schaller @ 2020-12-01 14:20 ` Linus Walleij 2020-12-01 14:34 ` Sven Van Asbroeck ` (2 more replies) 0 siblings, 3 replies; 42+ messages in thread From: Linus Walleij @ 2020-12-01 14:20 UTC (permalink / raw) To: H. Nikolaus Schaller Cc: Mark Brown, Sven Van Asbroeck, kernel list, Laurent Pinchart, Discussions about the Letux Kernel Hi Nikolaus, Sven, the fault that the thing broke in the first place is all mine. Credit where credit is due! The reason why I shoot in the dark to convert all SPI drivers to use GPIO descriptors instead of the global GPIO numberspace is detailed in drivers/gpio/TODO so I will not repeat it here. I don't know if much can be done about it other than having better programmers than me at the task. Or less tired when they write the patch. etc. What other operating systems do to get around the same type of refactoring problem is to aggressively deprecate and delete code that does not follow the latest ideas of the driver subsystem developer. This is not an option on Linux because we don't like to leave working hardware and users behind so I am painstakingly fixing it all over the place, with a little help from my friends. Sometimes it blows up in my face, sometimes in other people faces too, sorry about that. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [BUG] SPI broken for SPI based panel drivers 2020-12-01 14:20 ` Linus Walleij @ 2020-12-01 14:34 ` Sven Van Asbroeck 2020-12-01 14:35 ` H. Nikolaus Schaller 2020-12-01 16:20 ` Mark Brown 2 siblings, 0 replies; 42+ messages in thread From: Sven Van Asbroeck @ 2020-12-01 14:34 UTC (permalink / raw) To: Linus Walleij Cc: H. Nikolaus Schaller, Mark Brown, kernel list, Laurent Pinchart, Discussions about the Letux Kernel Hi Linus, On Tue, Dec 1, 2020 at 9:20 AM Linus Walleij <linus.walleij@linaro.org> wrote: > > I don't know if much can be done about it other than > having better programmers than me at the task. Or > less tired when they write the patch. etc. I don't think that we have many programmers that are better than you :) IMHO the fundamental dilemma is between features, security and agility on the one hand, and stability on the other. Too much emphasis on stability, and one ends up with a system which is hard to change, i.e. improve or keep secure. A system like that runs the real risk of being rapidly overtaken by a more nimble alternative. More testing is good and will make breakages rarer, but I guess we need to be realistic here and realize that even with a huge community effort, we might never get 100% or even 80% test coverage. But to be honest these sound like questions for the Greg KHs and Torvaldses of this world, not for me. ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [BUG] SPI broken for SPI based panel drivers 2020-12-01 14:20 ` Linus Walleij 2020-12-01 14:34 ` Sven Van Asbroeck @ 2020-12-01 14:35 ` H. Nikolaus Schaller 2020-12-01 15:52 ` Sven Van Asbroeck 2020-12-01 16:10 ` Sven Van Asbroeck 2020-12-01 16:20 ` Mark Brown 2 siblings, 2 replies; 42+ messages in thread From: H. Nikolaus Schaller @ 2020-12-01 14:35 UTC (permalink / raw) To: Linus Walleij Cc: Mark Brown, Sven Van Asbroeck, kernel list, Laurent Pinchart, Discussions about the Letux Kernel Hi Linus, > Am 01.12.2020 um 15:20 schrieb Linus Walleij <linus.walleij@linaro.org>: > > Hi Nikolaus, Sven, > > the fault that the thing broke in the first place is all mine. > Credit where credit is due! > > The reason why I shoot in the dark to convert all SPI > drivers to use GPIO descriptors instead of the global > GPIO numberspace is detailed in drivers/gpio/TODO > so I will not repeat it here. > > I don't know if much can be done about it other than > having better programmers than me at the task. Or > less tired when they write the patch. etc. > > What other operating systems do to get around the same > type of refactoring problem is to aggressively > deprecate and delete code that does not follow the > latest ideas of the driver subsystem developer. This > is not an option on Linux because we don't like to leave > working hardware and users behind so I am painstakingly > fixing it all over the place, with a little help from my > friends. Sometimes it blows up in my face, sometimes > in other people faces too, sorry about that. Everyone has such a patch written at the wrong time of the day... See for example: f069b5a0b27a Let's work on a fix for the fix now. BR and thanks, Nikolaus ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [BUG] SPI broken for SPI based panel drivers 2020-12-01 14:35 ` H. Nikolaus Schaller @ 2020-12-01 15:52 ` Sven Van Asbroeck 2020-12-01 16:46 ` H. Nikolaus Schaller 2020-12-01 16:10 ` Sven Van Asbroeck 1 sibling, 1 reply; 42+ messages in thread From: Sven Van Asbroeck @ 2020-12-01 15:52 UTC (permalink / raw) To: H. Nikolaus Schaller Cc: Linus Walleij, Mark Brown, kernel list, Laurent Pinchart, Discussions about the Letux Kernel Nikolaus, On Tue, Dec 1, 2020 at 9:38 AM H. Nikolaus Schaller <hns@goldelico.com> wrote: > > Let's work on a fix for the fix now. I tested spi-gpio on my system, by converting a built-in or hardware spi, to a spi-gpio. Interestingly, the patch has the opposite effect on my system: before the patch, spi-gpio did not work, but after it's applied, it does work. Can you tell me the idle status of your chip-select gpio in debugfs? # mount -t debugfs none /sys/kernel/debug # cat /sys/kernel/debug/gpio Look for something like this: gpiochip0: GPIOs 0-31, parent: platform/209c000.gpio, 209c000.gpio: gpio-17 ( |spi5 CS0 ) out hi ACTIVE LOW Also, apply the following patch, and tell me a) does this dev_err() get called on your system, and b) what is the value when your chip is de-selected diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index 7e8804b02be9..b2f4cf5c9ffb 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -813,11 +813,12 @@ static void spi_set_cs(struct spi_device *spi, bool enable) if (spi->cs_gpiod || gpio_is_valid(spi->cs_gpio)) { if (!(spi->mode & SPI_NO_CS)) { - if (spi->cs_gpiod) + if (spi->cs_gpiod) { + dev_err(&spi->dev, "gpiod %s", enable1 ? "enable" : "disable"); /* polarity handled by gpiolib */ gpiod_set_value_cansleep(spi->cs_gpiod, enable1); - else + } else /* * invert the enable line, as active low is * default for SPI. ^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: [BUG] SPI broken for SPI based panel drivers 2020-12-01 15:52 ` Sven Van Asbroeck @ 2020-12-01 16:46 ` H. Nikolaus Schaller 0 siblings, 0 replies; 42+ messages in thread From: H. Nikolaus Schaller @ 2020-12-01 16:46 UTC (permalink / raw) To: Sven Van Asbroeck Cc: Linus Walleij, Mark Brown, kernel list, Laurent Pinchart, Discussions about the Letux Kernel > Am 01.12.2020 um 16:52 schrieb Sven Van Asbroeck <thesven73@gmail.com>: > > Nikolaus, > > On Tue, Dec 1, 2020 at 9:38 AM H. Nikolaus Schaller <hns@goldelico.com> wrote: >> >> Let's work on a fix for the fix now. > > I tested spi-gpio on my system, by converting a built-in or hardware spi, > to a spi-gpio. Interestingly, the patch has the opposite effect on my system: > before the patch, spi-gpio did not work, but after it's applied, it does work. > > Can you tell me the idle status of your chip-select gpio in debugfs? > # mount -t debugfs none /sys/kernel/debug > # cat /sys/kernel/debug/gpio > Look for something like this: > gpiochip0: GPIOs 0-31, parent: platform/209c000.gpio, 209c000.gpio: > gpio-17 ( |spi5 CS0 ) out hi ACTIVE LOW root@letux:~# mount -t debugfs none /sys/kernel/debug root@letux:~# cat /sys/kernel/debug/gpio|fgrep spi gpio-179 ( |spi4 CS0 ) out lo root@letux:~# That is after the panel driver did send the commands. > > Also, apply the following patch, and tell me > a) does this dev_err() get called on your system, and yes. Many times. > b) what is the value when your chip is de-selected root@letux:~# dmesg|fgrep td028 [ 14.530456] panel-tpo-td028ttec1 spi4.0: spi->mode = 00000003 [ 14.599212] panel-tpo-td028ttec1 spi4.0: gpiod disable [ 14.817871] panel-tpo-td028ttec1 spi4.0: spi->mode = 00000003 [ 14.817871] panel-tpo-td028ttec1 spi4.0: gpiod enable So it is disabled first and then enabled. Which appears to be the opposite of what it should be. BTW: I have added another dev_err to print the spi->mode and like you describe it is (overwritten? by SPI_MODE_3. I can check what value it has in the driver before it is set to SPI_MODE_3. Maybe, there the spi-cs-high gets lost? BR and thanks, Nikolaus > diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c > index 7e8804b02be9..b2f4cf5c9ffb 100644 > --- a/drivers/spi/spi.c > +++ b/drivers/spi/spi.c > @@ -813,11 +813,12 @@ static void spi_set_cs(struct spi_device *spi, > bool enable) > > if (spi->cs_gpiod || gpio_is_valid(spi->cs_gpio)) { > if (!(spi->mode & SPI_NO_CS)) { > - if (spi->cs_gpiod) > + if (spi->cs_gpiod) { > + dev_err(&spi->dev, "gpiod %s", enable1 > ? "enable" : "disable"); > /* polarity handled by gpiolib */ > gpiod_set_value_cansleep(spi->cs_gpiod, > enable1); > - else > + } else > /* > * invert the enable line, as active low is > * default for SPI. ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [BUG] SPI broken for SPI based panel drivers 2020-12-01 14:35 ` H. Nikolaus Schaller 2020-12-01 15:52 ` Sven Van Asbroeck @ 2020-12-01 16:10 ` Sven Van Asbroeck 2020-12-01 16:39 ` H. Nikolaus Schaller 2020-12-01 16:44 ` [Letux-kernel] " Andreas Kemnade 1 sibling, 2 replies; 42+ messages in thread From: Sven Van Asbroeck @ 2020-12-01 16:10 UTC (permalink / raw) To: H. Nikolaus Schaller Cc: Linus Walleij, Mark Brown, kernel list, Laurent Pinchart, Discussions about the Letux Kernel Nikolaus, On Tue, Dec 1, 2020 at 9:38 AM H. Nikolaus Schaller <hns@goldelico.com> wrote: > > Let's work on a fix for the fix now. > Are you quite sure the chip-select of the tpo,td028ttec1 panel is active-high? A quick google produced a datasheet which seems to indicate that XCS is active-low? See page 17 here: http://www.lcd-source.com/datasheet/TPO/TD028TTEC1.pdf It is of course possible that you are driving that line behind some inverting circuitry. Hardware designers seem to do that all the time, if they need to go from one voltage domain to the other, etc. ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [BUG] SPI broken for SPI based panel drivers 2020-12-01 16:10 ` Sven Van Asbroeck @ 2020-12-01 16:39 ` H. Nikolaus Schaller 2020-12-01 16:53 ` Sven Van Asbroeck 2020-12-01 16:44 ` [Letux-kernel] " Andreas Kemnade 1 sibling, 1 reply; 42+ messages in thread From: H. Nikolaus Schaller @ 2020-12-01 16:39 UTC (permalink / raw) To: Sven Van Asbroeck Cc: Linus Walleij, Mark Brown, kernel list, Laurent Pinchart, Discussions about the Letux Kernel > Am 01.12.2020 um 17:10 schrieb Sven Van Asbroeck <thesven73@gmail.com>: > > Nikolaus, > > On Tue, Dec 1, 2020 at 9:38 AM H. Nikolaus Schaller <hns@goldelico.com> wrote: >> >> Let's work on a fix for the fix now. >> > > Are you quite sure the chip-select of the tpo,td028ttec1 panel > is active-high? A quick google produced a datasheet which > seems to indicate that XCS is active-low? > > See page 17 here: > http://www.lcd-source.com/datasheet/TPO/TD028TTEC1.pdf > > It is of course possible that you are driving that line behind > some inverting circuitry. Hardware designers seem to do that > all the time, if they need to go from one voltage domain to > the other, etc. You are right. It is active low. But that is the start of a long story... It was introduced in DT by c2e138bc8ed80ae with cs-gpios = <&gpio1 19 0>; because the third parameter did not have any meaning back then, AFAIR. It was ignored and drivers did initialize gpios with inversion if needed. Missing spi-cs-high; did define active low back then for spi-gpio. In 3a637e008e542b8ebdc2ceed616b229af0462b14 the "0" was replaced by the constant cs-gpios = <&gpio1 19 GPIO_ACTIVE_HIGH>; keeping the DTB unchanged and compatible. This did work because there was still no spi-cs-high; Then came 6953c57ab172 which mixed cs-gpios and spi-cs-high; and made use of the now wrong GPIO_ACTIVE_HIGH; A special logic converted GPIO_ACTIVE_HIGH to GPIO_ACTIVE_LOW in case of spi-cs-high; being present. A long discussion revealed that the only solution to stay compatible with old and new DT/kernels was to add spi-cs-high; and keep the wrong <&gpio1 19 GPIO_ACTIVE_HIGH>; f1f028ff89cb0d3 did "fix" this by adding spi-cs-high to the DT. Please note that apparently we were already confused about what the right value should be (ACTIVE_HIGH or ACTIVE_LOW) in 2019 but the result was working until your new patch appeared. Yes, it could be that the problem introduced by 6953c57ab172 is just coming up again with your new change. On the other hand I remember from the old discussion that changing a DT is "forbidden" if it is not upwards compatible with earlier kernels. The driver must take what it gets from (legacy) DT. BR, Nikolaus ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [BUG] SPI broken for SPI based panel drivers 2020-12-01 16:39 ` H. Nikolaus Schaller @ 2020-12-01 16:53 ` Sven Van Asbroeck 2020-12-01 17:10 ` H. Nikolaus Schaller 0 siblings, 1 reply; 42+ messages in thread From: Sven Van Asbroeck @ 2020-12-01 16:53 UTC (permalink / raw) To: H. Nikolaus Schaller Cc: Linus Walleij, Mark Brown, kernel list, Laurent Pinchart, Discussions about the Letux Kernel, Andreas Kemnade Hi Nikolaus, On Tue, Dec 1, 2020 at 11:43 AM H. Nikolaus Schaller <hns@goldelico.com> wrote: > > You are right. It is active low. > In that case, we have a very simple solution, just remove the spi-cs-high, and things will work. In case of SPI CS gpios, the current kernel ignores all GPIO_ACTIVE_HIGH/LOW flags, and uses the presence/absence of spi-cs-high instead, to determine active high / active low. ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [BUG] SPI broken for SPI based panel drivers 2020-12-01 16:53 ` Sven Van Asbroeck @ 2020-12-01 17:10 ` H. Nikolaus Schaller 2020-12-01 18:43 ` Sven Van Asbroeck 2020-12-01 22:51 ` Linus Walleij 0 siblings, 2 replies; 42+ messages in thread From: H. Nikolaus Schaller @ 2020-12-01 17:10 UTC (permalink / raw) To: Sven Van Asbroeck Cc: Linus Walleij, Mark Brown, kernel list, Laurent Pinchart, Discussions about the Letux Kernel, Andreas Kemnade Hi Sven, > Am 01.12.2020 um 17:53 schrieb Sven Van Asbroeck <thesven73@gmail.com>: > > Hi Nikolaus, > > On Tue, Dec 1, 2020 at 11:43 AM H. Nikolaus Schaller <hns@goldelico.com> wrote: >> >> You are right. It is active low. >> > > In that case, we have a very simple solution, just remove the spi-cs-high, > and things will work. We originally had it that way and because there was a change in gpiolib we had to introduce it. See: f1f028ff89cb0d3 where we were forced to introduce it although I had preferred to not change DT. I am not sure if DT maintainers accept that we revert a DT change just to handle some change in a driver. Usually they insist on fixing a driver and live with the DT. DT is carved in stone or could be ROM... So you could try to submit a revert of f1f028ff89cb0d3 with a description why it is needed. And please make sure that it is also applied where your patch is backported to stable. So it should have some Fixes: 766c6b63aa04 ("spi: fix client driver breakages when using GPIO descriptors") > > In case of SPI CS gpios, the current kernel ignores all > GPIO_ACTIVE_HIGH/LOW > flags, and uses the presence/absence of spi-cs-high instead, to > determine active high / active low. So you mean you are just restoring the behaviour before 6953c57ab172 was introduced? The alternative is that you restore the cs-gpios + spi-cs-high semantics as defined by 6953c57ab172 and everything is fine without touching (our) DTS (again). BR and thanks, Nikolaus ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [BUG] SPI broken for SPI based panel drivers 2020-12-01 17:10 ` H. Nikolaus Schaller @ 2020-12-01 18:43 ` Sven Van Asbroeck 2020-12-02 12:19 ` Mark Brown 2020-12-04 10:08 ` H. Nikolaus Schaller 2020-12-01 22:51 ` Linus Walleij 1 sibling, 2 replies; 42+ messages in thread From: Sven Van Asbroeck @ 2020-12-01 18:43 UTC (permalink / raw) To: H. Nikolaus Schaller Cc: Linus Walleij, Mark Brown, kernel list, Laurent Pinchart, Discussions about the Letux Kernel, Andreas Kemnade Hi Nikolaus, On Tue, Dec 1, 2020 at 12:13 PM H. Nikolaus Schaller <hns@goldelico.com> wrote: > > Am 01.12.2020 um 17:53 schrieb Sven Van Asbroeck <thesven73@gmail.com>: > > On Tue, Dec 1, 2020 at 11:43 AM H. Nikolaus Schaller <hns@goldelico.com> wrote: > >> > >> You are right. It is active low. > >> > > > > In that case, we have a very simple solution, just remove the spi-cs-high, > > and things will work. > > We originally had it that way and because there was a change in gpiolib we had > to introduce it. The current rules re. spi chip-selects in devicetrees are here: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpio/gpiolib-of.c?h=v5.10-rc6#n191 This is the way I see things: - according to the current rules, your devicetree describes a spi panel with an active-high chip select - the actual chip select of your panel is active-low - a spi/gpiod bug inverted the chip-select in many instances - because of this bug, your devicetree happened to work before 766c6b63aa04 - 766c6b63aa04 ("spi: fix client driver breakages when using GPIO descriptors") fixes this chip-select polarity bug - you now need to remove your devicetree work-around for this bug by reverting f1f028ff89cb0d3 > > I am not sure if DT maintainers accept that we revert a DT change just to > handle some change in a driver. Usually they insist on fixing a driver and > live with the DT. DT is carved in stone or could be ROM... This is above my paygrade, but I've always assumed that the devicetree ABI is an in-kernel ABI, i.e. not a userspace ABI. Meaning that it is flexible and there is no obligation to keep it 100% backwards compatible. Of course Rob Herring may want to keep it as backwards-compatible as possible, but that's an altogether different thing from having a userspace-type ABI. > > So you could try to submit a revert of f1f028ff89cb0d3 with a description > why it is needed. And please make sure that it is also applied where your > patch is backported to stable. So it should have some > > Fixes: 766c6b63aa04 ("spi: fix client driver breakages when using GPIO descriptors") I have no insight in your devicetrees, and no hardware to test it out either. As a user of these trees, you are best placed to make the change and test it out. I invite you to submit a patch (revert of f1f028ff89cb0d3) to the mailing list. > > > So you mean you are just restoring the behaviour before > > 6953c57ab172 > > was introduced? > That was based on my incorrect interpretation of the devicetree spi cs rules, my apologies. I have linked to the correct rules in the link above. ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [BUG] SPI broken for SPI based panel drivers 2020-12-01 18:43 ` Sven Van Asbroeck @ 2020-12-02 12:19 ` Mark Brown 2020-12-04 10:08 ` H. Nikolaus Schaller 1 sibling, 0 replies; 42+ messages in thread From: Mark Brown @ 2020-12-02 12:19 UTC (permalink / raw) To: Sven Van Asbroeck Cc: H. Nikolaus Schaller, Linus Walleij, kernel list, Laurent Pinchart, Discussions about the Letux Kernel, Andreas Kemnade [-- Attachment #1: Type: text/plain, Size: 1247 bytes --] On Tue, Dec 01, 2020 at 01:43:36PM -0500, Sven Van Asbroeck wrote: > On Tue, Dec 1, 2020 at 12:13 PM H. Nikolaus Schaller <hns@goldelico.com> wrote: > > I am not sure if DT maintainers accept that we revert a DT change just to > > handle some change in a driver. Usually they insist on fixing a driver and > > live with the DT. DT is carved in stone or could be ROM... > This is above my paygrade, but I've always assumed that the devicetree ABI > is an in-kernel ABI, i.e. not a userspace ABI. Meaning that it is flexible and > there is no obligation to keep it 100% backwards compatible. Of course Rob > Herring may want to keep it as backwards-compatible as possible, but that's > an altogether different thing from having a userspace-type ABI. It's supposed to be an ABI, though sometimes that gets broken - in a case like this if there's only a very limited set of users relying on something that's going to make things harder to maintain and which don't in practice distribute the kernel separately to the DT then it can make sense to just break the DT since realistically nobody's going to actually notice. On the other hand if people are distributing the kernel separately to the DT then compatibility definitely has to be maintained. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [BUG] SPI broken for SPI based panel drivers 2020-12-01 18:43 ` Sven Van Asbroeck 2020-12-02 12:19 ` Mark Brown @ 2020-12-04 10:08 ` H. Nikolaus Schaller 2020-12-04 13:46 ` Sven Van Asbroeck 1 sibling, 1 reply; 42+ messages in thread From: H. Nikolaus Schaller @ 2020-12-04 10:08 UTC (permalink / raw) To: Sven Van Asbroeck, Linus Walleij, Lukas Wunner Cc: Mark Brown, kernel list, Laurent Pinchart, Discussions about the Letux Kernel, Andreas Kemnade Hi Sven and Linux, > Am 01.12.2020 um 19:43 schrieb Sven Van Asbroeck <thesven73@gmail.com>: > > Hi Nikolaus, I was about to submit two patches. One that reverts our "spi-cs-high" and one that actively sets the GPIO_ACTIVE_LOW because that seems to be the right thing. It would work with v5.10, but is it really the right thing? Before submitting I would like to clarify the rules. Especially since it is the same discussion as before: https://lkml.org/lkml/2019/3/23/131 https://lkml.org/lkml/2019/3/24/2 > On Tue, Dec 1, 2020 at 12:13 PM H. Nikolaus Schaller <hns@goldelico.com> wrote: >>> Am 01.12.2020 um 17:53 schrieb Sven Van Asbroeck <thesven73@gmail.com>: >>> On Tue, Dec 1, 2020 at 11:43 AM H. Nikolaus Schaller <hns@goldelico.com> wrote: >>>> >>>> You are right. It is active low. >>>> >>> >>> In that case, we have a very simple solution, just remove the spi-cs-high, >>> and things will work. >> >> We originally had it that way and because there was a change in gpiolib we had >> to introduce it. > > The current rules re. spi chip-selects in devicetrees are here: > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpio/gpiolib-of.c?h=v5.10-rc6#n191 Yes, there is /* * SPI children have active low chip selects * by default. This can be specified negatively * by just omitting "spi-cs-high" in the * device node, or actively by tagging on * GPIO_ACTIVE_LOW as flag in the device * tree. If the line is simultaneously * tagged as active low in the device tree * and has the "spi-cs-high" set, we get a * conflict and the "spi-cs-high" flag will * take precedence. */ It is not complete and a formulated a little in reverse. A table would would be more precise. And it is not the complete rule: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpio/gpiolib-of.c?h=v5.10-rc6#n175 /* * Legacy handling of SPI active high chip select. If we have a * property named "cs-gpios" we need to inspect the child node * to determine if the flags should have inverted semantics. */ This seems to indicate that the gpio flags can invert spi-cs-high. There are only two options: invert if they are GPIO_ACTIVE_LOW or GPIO_ACTIVE_HIGH. This makes IMHO much sense. But it seems that it was not implemented that way. > > This is the way I see things: > - according to the current rules, your devicetree describes a spi panel with > an active-high chip select I have now tested all 4 combinations of GPIO_ACTIVE_LOW/HIGH and missing/present "spi-cs-high" with v5.4.79. This leads to the following definition: a) general SPI controller or if no cs-gpios available in spi-gpio device node | CS pin state active ================+================== spi-cs-high | H - | L This was the single (legacy) rule until v4.18 effectively ignoring any gpio flags. b) with v4.19 and 6953c57ab172 ("gpio: of: Handle SPI chipselect legacy bindings") was introduced it was apparently extended to (by code inspection of the patch): device node | cs-gpio | CS pin state active ================+===============+==================== spi-cs-high | - | H - | - | L spi-cs-high | ACTIVE_HIGH | L (polarity inversion, no warning) - | ACTIVE_HIGH | L + "enforce active low on chipselect handle" spi-cs-high | ACTIVE_LOW | H + "GPIO handle specifies active low - ignored" - | ACTIVE_LOW | H (no warning) But I did test all 4 combinations with v5.4.79 and assuming CS pin state L if the panel works gave me: device node | cs-gpio | CS pin state active ================+===============+==================== spi-cs-high | ACTIVE_HIGH | L (panel works) - | ACTIVE_HIGH | H (panel fails) + "enforce active low on chipselect handle" spi-cs-high | ACTIVE_LOW | L (panel works) + "GPIO handle specifies active low - ignored" - | ACTIVE_LOW | H (panel fails without comment) This means there was some additional tweak to the rules effectively ignoring the cs-gpio. Like before - but with opposite polarity. Note that some legacy gpio flags are 0 (ACTIVE_HIGH) so it seems to make sense for the ACTIVE_HIGH case like ours. Therefore, only GPIO_ACTIVE_HIGH + "spi-cs-high" works without side-effects which is the reason why we had modified our device tree and defined the wrong "spi-cs-high" as a solution. Anyways it is debatable if this is a bug at all. It is just a definition. Which is not well documented anywhere. c) now with v5.10 and 766c6b63aa04 ("spi: fix client driver breakages when using GPIO descriptors") the spi driver does no longer rely on gpiolib to do the right thing but adds its own rule effectively inverting things again. I hope I got it right because the logic is now spread over several files: device node | cs-gpio | CS pin state active ================+===============+==================== spi-cs-high | - | H - | - | L spi-cs-high | ACTIVE_HIGH | H (breaks our panel with current DT) - | ACTIVE_HIGH | L (should print a warning about polarity inversion - because here it would be wise to switch to ACTIVE_LOW) spi-cs-high | ACTIVE_LOW | H (could print a warning about polarity inversion - because ACTIVE_LOW is overridden by spi-cs-high) - | ACTIVE_LOW | L The implementation with your fix now seems to completely ignore the gpio definition since in the absence of spi-cs-high the panel works with either ACTIVE_ definition. Basically, we are back at a) i.e. the same as it was up to v4.18. So in total there should be no automatic polarity inversion of the flags by gpiolib at all. Only the spi driver should know that it has to invert the parameter for ACTIVE_LOW to gpiod_set_value_cansleep() to really control the CS pin state (H/L). > - the actual chip select of your panel is active-low Yes. > - a spi/gpiod bug inverted the chip-select in many instances Ok, there was another patch in between: 138c9c32f090 ("spi: spidev: Fix CS polarity if GPIO descriptors are used") Maybe this has introduced the bug - and we just didn't notice. This is why I have added Lukas Wunner <lukas@wunner.de> to the discussion. > - because of this bug, your devicetree happened to work before 766c6b63aa04 IMHO it is debatable if it is a bug or what is the bug. In any case it is a gap in clear and binding documentation. > - 766c6b63aa04 ("spi: fix client driver breakages when using GPIO descriptors") > fixes this chip-select polarity bug > - you now need to remove your devicetree work-around for this bug by reverting > f1f028ff89cb0d3 Before doing this I would like to see a table (like mine above) how it should be. Definitively and finally. Agreed and approved by maintainers. This table should then go somewhere to the .yaml definitions of gpiod or spi or whereever it belongs to. Then you can check if your code does what the table mandates and I can check if our device tree does the right thing. Otherwise we risk that I change the DT now and in 2 years someone else finds that the definition is still not sane and tweaks SPI or gpiolib again and I have to modify our DT once more (which costs a lot of time to find the reasons of breakage and takes away a lot of work-time from other nice things I would like to add to Linux). What I especially wonder is why you fix that at all in drivers/spi/spi.c if polarity inversion is handled in gpiolib. BR and thanks, Nikolaus ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [BUG] SPI broken for SPI based panel drivers 2020-12-04 10:08 ` H. Nikolaus Schaller @ 2020-12-04 13:46 ` Sven Van Asbroeck 2020-12-04 16:49 ` H. Nikolaus Schaller 0 siblings, 1 reply; 42+ messages in thread From: Sven Van Asbroeck @ 2020-12-04 13:46 UTC (permalink / raw) To: H. Nikolaus Schaller Cc: Linus Walleij, Lukas Wunner, Mark Brown, kernel list, Laurent Pinchart, Discussions about the Letux Kernel, Andreas Kemnade On Fri, Dec 4, 2020 at 5:11 AM H. Nikolaus Schaller <hns@goldelico.com> wrote: > > Anyways it is debatable if this is a bug at all. It is just a definition. I respectfully disagree. Prior to the fix, your panel's active-low chip select needed to be described in the devicetree with 'spi-cs-high'. That sounds very much like a bug to me. > Which is not well documented anywhere. I agree that documentation can be improved here. Would you like to submit a patch that improves: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/spi/spi-controller.yaml?h=v5.10-rc6#n28 ? This way, we also get Rob Herring involved, which may lead to more elegant documentation. He is more likely to respond to a patch than to a question. > > What I especially wonder is why you fix that at all in drivers/spi/spi.c > if polarity inversion is handled in gpiolib. The reason for that is described in the commit message of 766c6b63aa04 ("spi: fix client driver breakages when using GPIO descriptors") ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [BUG] SPI broken for SPI based panel drivers 2020-12-04 13:46 ` Sven Van Asbroeck @ 2020-12-04 16:49 ` H. Nikolaus Schaller 2020-12-04 19:19 ` Sven Van Asbroeck 2020-12-05 0:25 ` Linus Walleij 0 siblings, 2 replies; 42+ messages in thread From: H. Nikolaus Schaller @ 2020-12-04 16:49 UTC (permalink / raw) To: Sven Van Asbroeck, Linus Walleij Cc: Lukas Wunner, Mark Brown, kernel list, Laurent Pinchart, Discussions about the Letux Kernel, Andreas Kemnade Hi Sven, > Am 04.12.2020 um 14:46 schrieb Sven Van Asbroeck <thesven73@gmail.com>: > > On Fri, Dec 4, 2020 at 5:11 AM H. Nikolaus Schaller <hns@goldelico.com> wrote: >> >> Anyways it is debatable if this is a bug at all. It is just a definition. > > I respectfully disagree. Prior to the fix, your panel's active-low chip select > needed to be described in the devicetree with 'spi-cs-high'. That sounds > very much like a bug to me. It could have been described by ACTIVE_LOW without spi-cs-high but that did emit a nasty and not helpful warning on each boot. Well, there are of course better and worse definitions and I agree that spi-cs-high to define an active-low chip select sounds strange. Still it is just a definition. But what I don't know is if I can omit spi-cs-high and have to keep ACTIVE_HIGH (my revert patch) or also change to ACTIVE_LOW (my additional patch). This is arbitrary and someone has to decide what it should be. Then, the gpiolib / spi driver code should be adapted if necessary and a unit-test or real-hardware test with all possible combinations would prove it for all other devices as well. So testing against a specification does not need /my/ hardware. > >> Which is not well documented anywhere. > > I agree that documentation can be improved here. > Would you like to submit a patch that improves: > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/spi/spi-controller.yaml?h=v5.10-rc6#n28 > ? Yes, that is the right location. Basically it involves adding a table like in my previous mail to the comment so that it also covers all cases where the second parameter is not 0. I'd prefer if you or maybe Linus could submit such a patch and I am happy to review it. The reason is that I am neither involved in gpiolib nor spi subsystem development so I neither know what your intended setup is. I may define a wrong table. I just need a stable definition by the subsystem maintainers so that I can finish the device tree I am responsible for. What I can do is to provide just a skeleton for the table that you or Linus can fix/fill in and make a patch out of it. Is attached and the ??? is something you should discuss and define. > This way, we also get Rob Herring involved, which may lead to more elegant > documentation. He is more likely to respond to a patch than to a question. > >> >> What I especially wonder is why you fix that at all in drivers/spi/spi.c >> if polarity inversion is handled in gpiolib. > > The reason for that is described in the commit message of > 766c6b63aa04 ("spi: fix client driver breakages when using GPIO descriptors") IMHO it could have been fixed in the gpiolib alone. In my assumption, gpiolib would know (or be informed) that the gpio is used by spi and could invert gpio_set_value() if needed. Then any SPI code would simply use gpio_set_value(true) if spi-cs-high is defined and gpio_set_value(false) if not to enable the chip. The alternative would be that it is only done centrally in one place in the spi subsystem. But I may be wrong, because the real architecture of the spi and gpiolib code is quite new to me. My focus is on very different things (e.g. camera driver, drm panel drivers) which is already complex enough. BR and thanks, Nikolaus diff --git a/Documentation/devicetree/bindings/spi/spi-controller.yaml b/Documentation/devicetree/bindings/spi/spi-controller.yaml index 1b56d5e40f1f..4f8755dabecc 100644 --- a/Documentation/devicetree/bindings/spi/spi-controller.yaml +++ b/Documentation/devicetree/bindings/spi/spi-controller.yaml @@ -42,6 +42,30 @@ properties: cs2 : &gpio1 1 0 cs3 : &gpio1 2 0 + The second flag can be GPIO_ACTIVE_HIGH/0 or GPIO_ACTIVE_LOW/1. + + There is special rule set for combining the second flag of an + cs-gpio with the optional spi-cs-high flag for SPI slaves. + + Each table entry defines how the CS pin is physically driven + (not considering gpio inversions by pinmux): + + device node | cs-gpio | CS pin state active | Note + ================+===============+=====================+===== + spi-cs-high | - | H | + - | - | L | + spi-cs-high | ACTIVE_HIGH | H | + - | ACTIVE_HIGH | L (or H ???) | 1 + spi-cs-high | ACTIVE_LOW | H (or L ???) | 2 + - | ACTIVE_LOW | L | + + Notes: + 1) should print a warning about polarity inversion + because here it would be wise to define the gpio as ACTIVE_LOW + 2) could print a warning about polarity inversion + because ACTIVE_LOW is overridden by spi-cs-high + 3) Effectively this rule defines that the ACTIVE level of the + gpio has to be ignored + num-cs: $ref: /schemas/types.yaml#/definitions/uint32 description: ^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: [BUG] SPI broken for SPI based panel drivers 2020-12-04 16:49 ` H. Nikolaus Schaller @ 2020-12-04 19:19 ` Sven Van Asbroeck 2020-12-04 21:31 ` H. Nikolaus Schaller 2020-12-05 0:25 ` Linus Walleij 1 sibling, 1 reply; 42+ messages in thread From: Sven Van Asbroeck @ 2020-12-04 19:19 UTC (permalink / raw) To: H. Nikolaus Schaller Cc: Linus Walleij, Lukas Wunner, Mark Brown, kernel list, Laurent Pinchart, Discussions about the Letux Kernel, Andreas Kemnade On Fri, Dec 4, 2020 at 11:52 AM H. Nikolaus Schaller <hns@goldelico.com> wrote: > > >> Anyways it is debatable if this is a bug at all. It is just a definition. > > > > I respectfully disagree. Prior to the fix, your panel's active-low chip select > > needed to be described in the devicetree with 'spi-cs-high'. That sounds > > very much like a bug to me. > > It could have been described by ACTIVE_LOW without spi-cs-high but that did > emit a nasty and not helpful warning on each boot. That will not work, try it out. You will see that without the bugfix, your chip select is consistently inverted, no matter how you formulate it in the devicetree. > > I'd prefer if you or maybe Linus could submit such a patch and I am happy to review it. I cannot help you with that, I'm sorry. ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [BUG] SPI broken for SPI based panel drivers 2020-12-04 19:19 ` Sven Van Asbroeck @ 2020-12-04 21:31 ` H. Nikolaus Schaller 0 siblings, 0 replies; 42+ messages in thread From: H. Nikolaus Schaller @ 2020-12-04 21:31 UTC (permalink / raw) To: Sven Van Asbroeck Cc: Linus Walleij, Lukas Wunner, Mark Brown, kernel list, Laurent Pinchart, Discussions about the Letux Kernel, Andreas Kemnade > Am 04.12.2020 um 20:19 schrieb Sven Van Asbroeck <thesven73@gmail.com>: > > On Fri, Dec 4, 2020 at 11:52 AM H. Nikolaus Schaller <hns@goldelico.com> wrote: >> >>>> Anyways it is debatable if this is a bug at all. It is just a definition. >>> >>> I respectfully disagree. Prior to the fix, your panel's active-low chip select >>> needed to be described in the devicetree with 'spi-cs-high'. That sounds >>> very much like a bug to me. >> >> It could have been described by ACTIVE_LOW without spi-cs-high but that did >> emit a nasty and not helpful warning on each boot. > > That will not work, try it out. You will see that without the bugfix, your chip > select is consistently inverted, no matter how you formulate it in the > devicetree. I have. But please show me which line in my analyses table of my mail 12 hours ago is wrong. Then I can repeat the test and we can discuss the reasons. > >> >> I'd prefer if you or maybe Linus could submit such a patch and I am happy to review it. > > I cannot help you with that, I'm sorry. Come on... BR and thanks, Nikolaus ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [BUG] SPI broken for SPI based panel drivers 2020-12-04 16:49 ` H. Nikolaus Schaller 2020-12-04 19:19 ` Sven Van Asbroeck @ 2020-12-05 0:25 ` Linus Walleij 2020-12-05 7:04 ` H. Nikolaus Schaller 1 sibling, 1 reply; 42+ messages in thread From: Linus Walleij @ 2020-12-05 0:25 UTC (permalink / raw) To: H. Nikolaus Schaller Cc: Sven Van Asbroeck, Lukas Wunner, Mark Brown, kernel list, Laurent Pinchart, Discussions about the Letux Kernel, Andreas Kemnade On Fri, Dec 4, 2020 at 5:52 PM H. Nikolaus Schaller <hns@goldelico.com> wrote: > It could have been described by ACTIVE_LOW without spi-cs-high but that did > emit a nasty and not helpful warning on each boot. > > Well, there are of course better and worse definitions and I agree that > spi-cs-high to define an active-low chip select sounds strange. Still it > is just a definition. This has an historical background which is why we have the problem in the first place. We ended up with two different ways of doing polarity inversion in some subsystems because polarity flags *and* local polarity flags such as this existed. So the semantics became ambiguous. commit f618ebfcbf9616a0fa9a78f5ecb69762f0fa3c59 "of/spi: Support specifying chip select as active high via device tree" created spi-cs-high October 2008 commit b908b53d580c3e9aba81ebe3339c5b7b4fa8031d "of/gpio: Implement of_get_gpio_flags()" Created of_get_gpio_flags() and OF_GPIO_ACTIVE_LOW as an optional but strongly encouraged cell. December 2008 What we are seeing is the consequence of a formal language with ambiguous semantics. It's no-ones fault other than the human error of allowing both to exist simultaneously in the first place. Both ways of doing things existed before I took over as GPIO maintainer and before Mark took over as SPI maintainer. What we try to do is contain the situation. My attempt was to hide it inside the GPIO descriptor, which we still do, if and only if GPIO descriptors are used. > But what I don't know is if I can omit spi-cs-high and have to keep > ACTIVE_HIGH (my revert patch) or also change to ACTIVE_LOW (my additional > patch). This is arbitrary and someone has to decide what it should be. (...) > I'd prefer if you or maybe Linus could submit such a patch and I am happy to review it. It seems really ill-advised to have me do that since I have not managed very well to deal with this. Clearly better developers are needed. But I can review a patch and see if it makes me smarter :) > > The reason for that is described in the commit message of > > 766c6b63aa04 ("spi: fix client driver breakages when using GPIO descriptors") > > IMHO it could have been fixed in the gpiolib alone. In my assumption, gpiolib > would know (or be informed) that the gpio is used by spi and could invert gpio_set_value() > if needed. Then any SPI code would simply use gpio_set_value(true) if spi-cs-high > is defined and gpio_set_value(false) if not to enable the chip. That was the intention behind my code in of_gpio_flags_quirks(). And I suppose how it finally works after the recent patches as well, since we now pass enable1 (the non-inverted assertion parameter) to gpiod_set_value_cansleep() if and only if you are using GPIO descriptors. The reason it didn't work and a lot of ill-advised patches (mostly mine) has a lot to do with native chip selects which both listened to the same flag "spi-cs-high" and expected specific semantics from the spi core. For native chip selects there is no ambiguity: they can only be made active high using "spi-cs-high". > The alternative would be that it is only done centrally in one place in the > spi subsystem. FWIW I think GPIO CS is fine to handle in the gpiolib but then spi-cs-high also applies to native chip selects and that makes it necessary for the SPI core to also parse for spi-cs-high sometimes, for something that is not a GPIO, setting SPI_CS_HIGH and calling spi->controller->set_cs() on the controller with 0 for active low and 1 for active high. I think we are there now? Yours, Linus Walleij ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [BUG] SPI broken for SPI based panel drivers 2020-12-05 0:25 ` Linus Walleij @ 2020-12-05 7:04 ` H. Nikolaus Schaller 2020-12-09 8:04 ` Andreas Kemnade 2020-12-09 8:38 ` Linus Walleij 0 siblings, 2 replies; 42+ messages in thread From: H. Nikolaus Schaller @ 2020-12-05 7:04 UTC (permalink / raw) To: Linus Walleij Cc: Sven Van Asbroeck, Lukas Wunner, Mark Brown, kernel list, Laurent Pinchart, Discussions about the Letux Kernel, Andreas Kemnade Hi Linus, > Am 05.12.2020 um 01:25 schrieb Linus Walleij <linus.walleij@linaro.org>: > > On Fri, Dec 4, 2020 at 5:52 PM H. Nikolaus Schaller <hns@goldelico.com> wrote: > >> But what I don't know is if I can omit spi-cs-high and have to keep >> ACTIVE_HIGH (my revert patch) or also change to ACTIVE_LOW (my additional >> patch). This is arbitrary and someone has to decide what it should be. > (...) >> I'd prefer if you or maybe Linus could submit such a patch and I am happy to review it. > > It seems really ill-advised to have me do that since I have not > managed very well to deal with this. Clearly better developers > are needed. But I can review a patch and see if it makes me > smarter :) I find it interesting that so far nobody wants to take responsibility for a decision and to write down the behaviour really should be. Coding is the second step then. Anyways you did not cite the really important part of my mail. So let me copy it back. Here it is again: > What I can do is to provide just a skeleton for the table that you or Linus > can fix/fill in and make a patch out of it. Is attached and the ??? is > something you should discuss and define. Please take the attached diff, comment it here and define the question marks according to your intention and then make a patch for the YAML bindings out of it. (I can't do because I don't know your intentions and what to write into the commit message). As soon as we have settled this, we can check if code is correct and really define if my device tree fits and which change it needs exactly. BR and thanks, Nikolaus [slightly edited] diff --git a/Documentation/devicetree/bindings/spi/spi-controller.yaml b/Documentation/devicetree/bindings/spi/spi-controller.yaml index 1b56d5e40f1f..4f8755dabecc 100644 --- a/Documentation/devicetree/bindings/spi/spi-controller.yaml +++ b/Documentation/devicetree/bindings/spi/spi-controller.yaml @@ -42,6 +42,30 @@ properties: cs2 : &gpio1 1 0 cs3 : &gpio1 2 0 + The second flag of a gpio descriptor can be GPIO_ACTIVE_HIGH/0 + or GPIO_ACTIVE_LOW/1. + + There is a special rule set for combining the second flag of an + cs-gpio with the optional spi-cs-high flag for SPI slaves. + + Each table entry defines how the CS pin is physically driven + (not considering potential gpio inversions by pinmux): + + device node | cs-gpio | CS pin state active | Note + ================+===============+=====================+===== + spi-cs-high | - | H | + - | - | L | + spi-cs-high | ACTIVE_HIGH | H | + - | ACTIVE_HIGH | L (or H ???) | 1 + spi-cs-high | ACTIVE_LOW | H (or L ???) | 2 + - | ACTIVE_LOW | L | + + Notes: + 1) should print a warning about polarity inversion + because here it would be wise to define the gpio as ACTIVE_LOW + 2) could print a warning about polarity inversion + because ACTIVE_LOW is overridden by spi-cs-high + 3) Effectively this rule defines that the ACTIVE level of the + gpio has to be ignored + num-cs: $ref: /schemas/types.yaml#/definitions/uint32 description: ^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: [BUG] SPI broken for SPI based panel drivers 2020-12-05 7:04 ` H. Nikolaus Schaller @ 2020-12-09 8:04 ` Andreas Kemnade 2020-12-09 8:40 ` H. Nikolaus Schaller 2020-12-09 8:38 ` Linus Walleij 1 sibling, 1 reply; 42+ messages in thread From: Andreas Kemnade @ 2020-12-09 8:04 UTC (permalink / raw) To: H. Nikolaus Schaller Cc: Linus Walleij, Sven Van Asbroeck, Lukas Wunner, Mark Brown, kernel list, Laurent Pinchart, Discussions about the Letux Kernel Hi, On Sat, 5 Dec 2020 08:04:25 +0100 "H. Nikolaus Schaller" <hns@goldelico.com> wrote: > Hi Linus, > > > Am 05.12.2020 um 01:25 schrieb Linus Walleij <linus.walleij@linaro.org>: > > > > On Fri, Dec 4, 2020 at 5:52 PM H. Nikolaus Schaller <hns@goldelico.com> wrote: > > > >> But what I don't know is if I can omit spi-cs-high and have to keep > >> ACTIVE_HIGH (my revert patch) or also change to ACTIVE_LOW (my additional > >> patch). This is arbitrary and someone has to decide what it should be. > > (...) > >> I'd prefer if you or maybe Linus could submit such a patch and I am happy to review it. > > > > It seems really ill-advised to have me do that since I have not > > managed very well to deal with this. Clearly better developers > > are needed. But I can review a patch and see if it makes me > > smarter :) Hmm, if those developers are not available, then probably finding those bugs has to be time-optimized, like establishing better automatic display testing. > > I find it interesting that so far nobody wants to take responsibility > for a decision and to write down the behaviour really should be. Coding > is the second step then. > well, the interesting people are not involved yet (DTML) because no patch is sent. > Anyways you did not cite the really important part of my mail. So let me > copy it back. Here it is again: > > > What I can do is to provide just a skeleton for the table that you or Linus > > can fix/fill in and make a patch out of it. Is attached and the ??? is > > something you should discuss and define. > > Please take the attached diff, comment it here and define the question marks > according to your intention and then make a patch for the YAML bindings out > of it. (I can't do because I don't know your intentions and what to write into > the commit message). > Well, I the easiest step forward is just to document clearer how things behave now, so the commit message is just something like "Behavior of CS signal is not clearly documented, clarify the documentation". And then send the patch to the proper mailing lists including devicetree folks. Regards, Andreas > As soon as we have settled this, we can check if code is correct and really > define if my device tree fits and which change it needs exactly. > > BR and thanks, > Nikolaus > > [slightly edited] > > diff --git a/Documentation/devicetree/bindings/spi/spi-controller.yaml b/Documentation/devicetree/bindings/spi/spi-controller.yaml > index 1b56d5e40f1f..4f8755dabecc 100644 > --- a/Documentation/devicetree/bindings/spi/spi-controller.yaml > +++ b/Documentation/devicetree/bindings/spi/spi-controller.yaml > @@ -42,6 +42,30 @@ properties: > cs2 : &gpio1 1 0 > cs3 : &gpio1 2 0 > > + The second flag of a gpio descriptor can be GPIO_ACTIVE_HIGH/0 > + or GPIO_ACTIVE_LOW/1. > + > + There is a special rule set for combining the second flag of an > + cs-gpio with the optional spi-cs-high flag for SPI slaves. > + > + Each table entry defines how the CS pin is physically driven > + (not considering potential gpio inversions by pinmux): > + > + device node | cs-gpio | CS pin state active | Note > + ================+===============+=====================+===== > + spi-cs-high | - | H | > + - | - | L | > + spi-cs-high | ACTIVE_HIGH | H | > + - | ACTIVE_HIGH | L (or H ???) | 1 > + spi-cs-high | ACTIVE_LOW | H (or L ???) | 2 > + - | ACTIVE_LOW | L | > + > + Notes: > + 1) should print a warning about polarity inversion > + because here it would be wise to define the gpio as ACTIVE_LOW > + 2) could print a warning about polarity inversion > + because ACTIVE_LOW is overridden by spi-cs-high > + 3) Effectively this rule defines that the ACTIVE level of the > + gpio has to be ignored > + > num-cs: > $ref: /schemas/types.yaml#/definitions/uint32 > description: > > > ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [BUG] SPI broken for SPI based panel drivers 2020-12-09 8:04 ` Andreas Kemnade @ 2020-12-09 8:40 ` H. Nikolaus Schaller 0 siblings, 0 replies; 42+ messages in thread From: H. Nikolaus Schaller @ 2020-12-09 8:40 UTC (permalink / raw) To: Andreas Kemnade Cc: Linus Walleij, Sven Van Asbroeck, Lukas Wunner, Mark Brown, kernel list, Laurent Pinchart, Discussions about the Letux Kernel Hi Andreas, > Am 09.12.2020 um 09:04 schrieb Andreas Kemnade <andreas@kemnade.info>: > > Hi, > > On Sat, 5 Dec 2020 08:04:25 +0100 > "H. Nikolaus Schaller" <hns@goldelico.com> wrote: > >> Hi Linus, >> >>> Am 05.12.2020 um 01:25 schrieb Linus Walleij <linus.walleij@linaro.org>: >>> >>> On Fri, Dec 4, 2020 at 5:52 PM H. Nikolaus Schaller <hns@goldelico.com> wrote: >>> >>>> But what I don't know is if I can omit spi-cs-high and have to keep >>>> ACTIVE_HIGH (my revert patch) or also change to ACTIVE_LOW (my additional >>>> patch). This is arbitrary and someone has to decide what it should be. >>> (...) >>>> I'd prefer if you or maybe Linus could submit such a patch and I am happy to review it. >>> >>> It seems really ill-advised to have me do that since I have not >>> managed very well to deal with this. Clearly better developers >>> are needed. But I can review a patch and see if it makes me >>> smarter :) > > Hmm, if those developers are not available, then probably finding > those bugs has to be time-optimized, like establishing better automatic > display testing. Well, I don't think we need automatic display testing. We just need to test if any SPI CS behaves correctly according to some specification. Then all displays and other chips will work - unless they have a bug in their DT. Basically it would need a software unit-test going through all 6 variants of having spi-cs-high and gpiod and parameters and looking if the chip (maybe some SPI EEPROM with known SPI polarity) responds or not. This can be done with hardware SPI controllers and spi-gpio. And it can be re-run each time something significant in gpiolib or spi-gpio is changed. > >> >> I find it interesting that so far nobody wants to take responsibility >> for a decision and to write down the behaviour really should be. Coding >> is the second step then. >> > well, the interesting people are not involved yet (DTML) because no > patch is sent. Well, I think we (gpiolib maintainers, spi maintainers and users of it) are the right ones to define the truth table. This is not primarily a DT issue, it is a matter of what we want to have and then it is cast it into DT (documentation). So I am not sure if delegation to someone else solves it. > >> Anyways you did not cite the really important part of my mail. So let me >> copy it back. Here it is again: >> >>> What I can do is to provide just a skeleton for the table that you or Linus >>> can fix/fill in and make a patch out of it. Is attached and the ??? is >>> something you should discuss and define. >> >> Please take the attached diff, comment it here and define the question marks >> according to your intention and then make a patch for the YAML bindings out >> of it. (I can't do because I don't know your intentions and what to write into >> the commit message). >> > Well, I the easiest step forward is just to document clearer how things > behave now, so the commit message is just something like > > "Behavior of CS signal is not clearly documented, clarify the > documentation". And then send the patch to the proper mailing lists > including devicetree folks. Ok, that looks like a good solution to get out of the deadlock. BR and thanks, Nikolaus > > Regards, > Andreas > >> As soon as we have settled this, we can check if code is correct and really >> define if my device tree fits and which change it needs exactly. >> >> BR and thanks, >> Nikolaus >> >> [slightly edited] >> >> diff --git a/Documentation/devicetree/bindings/spi/spi-controller.yaml b/Documentation/devicetree/bindings/spi/spi-controller.yaml >> index 1b56d5e40f1f..4f8755dabecc 100644 >> --- a/Documentation/devicetree/bindings/spi/spi-controller.yaml >> +++ b/Documentation/devicetree/bindings/spi/spi-controller.yaml >> @@ -42,6 +42,30 @@ properties: >> cs2 : &gpio1 1 0 >> cs3 : &gpio1 2 0 >> >> + The second flag of a gpio descriptor can be GPIO_ACTIVE_HIGH/0 >> + or GPIO_ACTIVE_LOW/1. >> + >> + There is a special rule set for combining the second flag of an >> + cs-gpio with the optional spi-cs-high flag for SPI slaves. >> + >> + Each table entry defines how the CS pin is physically driven >> + (not considering potential gpio inversions by pinmux): >> + >> + device node | cs-gpio | CS pin state active | Note >> + ================+===============+=====================+===== >> + spi-cs-high | - | H | >> + - | - | L | >> + spi-cs-high | ACTIVE_HIGH | H | >> + - | ACTIVE_HIGH | L (or H ???) | 1 >> + spi-cs-high | ACTIVE_LOW | H (or L ???) | 2 >> + - | ACTIVE_LOW | L | >> + >> + Notes: >> + 1) should print a warning about polarity inversion >> + because here it would be wise to define the gpio as ACTIVE_LOW >> + 2) could print a warning about polarity inversion >> + because ACTIVE_LOW is overridden by spi-cs-high >> + 3) Effectively this rule defines that the ACTIVE level of the >> + gpio has to be ignored >> + >> num-cs: >> $ref: /schemas/types.yaml#/definitions/uint32 >> description: >> >> >> > ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [BUG] SPI broken for SPI based panel drivers 2020-12-05 7:04 ` H. Nikolaus Schaller 2020-12-09 8:04 ` Andreas Kemnade @ 2020-12-09 8:38 ` Linus Walleij 2020-12-09 8:45 ` H. Nikolaus Schaller 1 sibling, 1 reply; 42+ messages in thread From: Linus Walleij @ 2020-12-09 8:38 UTC (permalink / raw) To: H. Nikolaus Schaller Cc: Sven Van Asbroeck, Lukas Wunner, Mark Brown, kernel list, Laurent Pinchart, Discussions about the Letux Kernel, Andreas Kemnade On Sat, Dec 5, 2020 at 8:07 AM H. Nikolaus Schaller <hns@goldelico.com> wrote: > I find it interesting that so far nobody wants to take responsibility > for a decision (...) What causes some consternation in this discussion is the appeal to higher authority. The kernel community in general does not like authority/responsibility by way of formal hierarchy. Have you read this document? Especially point 1) Decisions: https://www.kernel.org/doc/html/latest/process/management-style.html (We can have a meta-discussion about this but it is not really your point I believe.) > > What I can do is to provide just a skeleton for the table that you or Linus > > can fix/fill in and make a patch out of it. Is attached and the ??? is > > something you should discuss and define. > > Please take the attached diff, comment it here and define the question marks > according to your intention and then make a patch for the YAML bindings out > of it. (I can't do because I don't know your intentions and what to write into > the commit message). I'll comment what I know, then you can send a proper patch to Mark. But you really need more people than me to look at this. > + device node | cs-gpio | CS pin state active | Note > + ================+===============+=====================+===== > + spi-cs-high | - | H | > + - | - | L | > + spi-cs-high | ACTIVE_HIGH | H | > + - | ACTIVE_HIGH | L (or H ???) | 1 When using GPIO descriptors it will be enforced to ACTIVE_LOW (L) with an explicit warning in dmesg, see drivers/gpio/gpiolib-of.c When using legacy GPIOs, will be enforced ACTIVE_LOW by the SPI core. > + spi-cs-high | ACTIVE_LOW | H (or L ???) | 2 When using GPIO descriptors it will be enforced to ACTIVE_HIGH (H) with an explicit warning in dmesg, see drivers/gpio/gpiolib-of.c > + 3) Effectively this rule defines that the ACTIVE level of the > + gpio has to be ignored Nr 3 isn't tagged in the table. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [BUG] SPI broken for SPI based panel drivers 2020-12-09 8:38 ` Linus Walleij @ 2020-12-09 8:45 ` H. Nikolaus Schaller 0 siblings, 0 replies; 42+ messages in thread From: H. Nikolaus Schaller @ 2020-12-09 8:45 UTC (permalink / raw) To: Linus Walleij Cc: Sven Van Asbroeck, Lukas Wunner, Mark Brown, kernel list, Laurent Pinchart, Discussions about the Letux Kernel, Andreas Kemnade Hi Linus, > Am 09.12.2020 um 09:38 schrieb Linus Walleij <linus.walleij@linaro.org>: > > On Sat, Dec 5, 2020 at 8:07 AM H. Nikolaus Schaller <hns@goldelico.com> wrote: > >> I find it interesting that so far nobody wants to take responsibility >> for a decision > (...) > > >>> What I can do is to provide just a skeleton for the table that you or Linus >>> can fix/fill in and make a patch out of it. Is attached and the ??? is >>> something you should discuss and define. >> >> Please take the attached diff, comment it here and define the question marks >> according to your intention and then make a patch for the YAML bindings out >> of it. (I can't do because I don't know your intentions and what to write into >> the commit message). > > I'll comment what I know, then you can send a proper patch to > Mark. But you really need more people than me to look at this. > >> + device node | cs-gpio | CS pin state active | Note >> + ================+===============+=====================+===== >> + spi-cs-high | - | H | >> + - | - | L | >> + spi-cs-high | ACTIVE_HIGH | H | >> + - | ACTIVE_HIGH | L (or H ???) | 1 > > When using GPIO descriptors it will be enforced to ACTIVE_LOW (L) with an > explicit warning in dmesg, see drivers/gpio/gpiolib-of.c Ok, so in this line the L is ok. > > When using legacy GPIOs, will be enforced ACTIVE_LOW by the SPI > core. > >> + spi-cs-high | ACTIVE_LOW | H (or L ???) | 2 > > When using GPIO descriptors it will be enforced to ACTIVE_HIGH (H) with an > explicit warning in dmesg, see drivers/gpio/gpiolib-of.c Ok, so my assumption about H is right and not the part in parenthesis with ???. > >> + 3) Effectively this rule defines that the ACTIVE level of the >> + gpio has to be ignored > > Nr 3 isn't tagged in the table. Well, it was thought as a third but general note. Maybe should have been a *) or omitted since the table stands for itself. > > Yours, > Linus Walleij So let me prepare a patch with fixes for this. BR and thanks, Nikolaus ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [BUG] SPI broken for SPI based panel drivers 2020-12-01 17:10 ` H. Nikolaus Schaller 2020-12-01 18:43 ` Sven Van Asbroeck @ 2020-12-01 22:51 ` Linus Walleij 1 sibling, 0 replies; 42+ messages in thread From: Linus Walleij @ 2020-12-01 22:51 UTC (permalink / raw) To: H. Nikolaus Schaller Cc: Sven Van Asbroeck, Mark Brown, kernel list, Laurent Pinchart, Discussions about the Letux Kernel, Andreas Kemnade On Tue, Dec 1, 2020 at 6:13 PM H. Nikolaus Schaller <hns@goldelico.com> wrote: > I am not sure if DT maintainers accept that we revert a DT change just to > handle some change in a driver. Usually they insist on fixing a driver and > live with the DT. DT is carved in stone or could be ROM... I usually use this rough consensus: is the DTB flashed into millions of devices and supplied to the kernel using some bootloader, and is the kernel upgraded on the device without also upgrading the DTB? And I mean in practice, not in theory. So whether the DTB ABI can be changed or not is a practical deployment question, not a religious sacrament. It came from systems such as Sun machines where the DTB was, indeed, in a PROM, and indeed intended for SunOS so Linux had no control over it. We had to just treat it as static ABI. If the actual situation is different, sucn as kernel and DTB are always updated together, or those are a few custom systems in a factory floor (not millions of mobile phones or laptops) then it is fine to change it occasionally even if it is seen as "bad". Yours, Linus Walleij ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [Letux-kernel] [BUG] SPI broken for SPI based panel drivers 2020-12-01 16:10 ` Sven Van Asbroeck 2020-12-01 16:39 ` H. Nikolaus Schaller @ 2020-12-01 16:44 ` Andreas Kemnade 2020-12-01 16:51 ` H. Nikolaus Schaller ` (2 more replies) 1 sibling, 3 replies; 42+ messages in thread From: Andreas Kemnade @ 2020-12-01 16:44 UTC (permalink / raw) To: Sven Van Asbroeck Cc: Discussions about the Letux Kernel, H. Nikolaus Schaller, Linus Walleij, Mark Brown, kernel list, Laurent Pinchart On Tue, 1 Dec 2020 11:10:49 -0500 Sven Van Asbroeck <thesven73@gmail.com> wrote: > Nikolaus, > > On Tue, Dec 1, 2020 at 9:38 AM H. Nikolaus Schaller <hns@goldelico.com> wrote: > > > > Let's work on a fix for the fix now. > > > > Are you quite sure the chip-select of the tpo,td028ttec1 panel > is active-high? A quick google produced a datasheet which > seems to indicate that XCS is active-low? > Schematics is here: https://projects.goldelico.com/p/gta04-main/downloads/48/ The display connector P204-LCD indicates some inversion at the XCS and XRES pins. This patch fixes things for a boot where the display was not touched by the bootloader diff --git a/arch/arm/boot/dts/omap3-gta04.dtsi b/arch/arm/boot/dts/omap3-gta04.dtsi index c8745bc800f7..003202d12990 100644 --- a/arch/arm/boot/dts/omap3-gta04.dtsi +++ b/arch/arm/boot/dts/omap3-gta04.dtsi @@ -124,7 +124,6 @@ spi-max-frequency = <100000>; spi-cpol; spi-cpha; - spi-cs-high; backlight= <&backlight>; label = "lcd"; So if that one is really active-low, why did it ever work?! Regards, Andreas ^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: [Letux-kernel] [BUG] SPI broken for SPI based panel drivers 2020-12-01 16:44 ` [Letux-kernel] " Andreas Kemnade @ 2020-12-01 16:51 ` H. Nikolaus Schaller 2020-12-01 16:52 ` H. Nikolaus Schaller 2020-12-01 16:55 ` Sven Van Asbroeck 2 siblings, 0 replies; 42+ messages in thread From: H. Nikolaus Schaller @ 2020-12-01 16:51 UTC (permalink / raw) To: Andreas Kemnade Cc: Sven Van Asbroeck, Discussions about the Letux Kernel, Linus Walleij, Mark Brown, kernel list, Laurent Pinchart > Am 01.12.2020 um 17:44 schrieb Andreas Kemnade <andreas@kemnade.info>: > > On Tue, 1 Dec 2020 11:10:49 -0500 > Sven Van Asbroeck <thesven73@gmail.com> wrote: > >> Nikolaus, >> >> On Tue, Dec 1, 2020 at 9:38 AM H. Nikolaus Schaller <hns@goldelico.com> wrote: >>> >>> Let's work on a fix for the fix now. >>> >> >> Are you quite sure the chip-select of the tpo,td028ttec1 panel >> is active-high? A quick google produced a datasheet which >> seems to indicate that XCS is active-low? >> > Schematics is here: > https://projects.goldelico.com/p/gta04-main/downloads/48/ > > The display connector P204-LCD indicates some inversion at the XCS and > XRES pins. > > This patch fixes things for a boot where the display was not > touched by the bootloader > diff --git a/arch/arm/boot/dts/omap3-gta04.dtsi b/arch/arm/boot/dts/omap3-gta04.dtsi > index c8745bc800f7..003202d12990 100644 > --- a/arch/arm/boot/dts/omap3-gta04.dtsi > +++ b/arch/arm/boot/dts/omap3-gta04.dtsi > @@ -124,7 +124,6 @@ > spi-max-frequency = <100000>; > spi-cpol; > spi-cpha; > - spi-cs-high; > > backlight= <&backlight>; > label = "lcd"; > > So if that one is really active-low, why did it ever work?! Apparently, because there was patch f1f028ff89cb0d3 to fix 6953c57ab172 ... BR, Nikolaus ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [Letux-kernel] [BUG] SPI broken for SPI based panel drivers 2020-12-01 16:44 ` [Letux-kernel] " Andreas Kemnade 2020-12-01 16:51 ` H. Nikolaus Schaller @ 2020-12-01 16:52 ` H. Nikolaus Schaller 2020-12-01 16:55 ` Sven Van Asbroeck 2 siblings, 0 replies; 42+ messages in thread From: H. Nikolaus Schaller @ 2020-12-01 16:52 UTC (permalink / raw) To: Andreas Kemnade Cc: Sven Van Asbroeck, Discussions about the Letux Kernel, Linus Walleij, Mark Brown, kernel list, Laurent Pinchart > Am 01.12.2020 um 17:44 schrieb Andreas Kemnade <andreas@kemnade.info>: > > On Tue, 1 Dec 2020 11:10:49 -0500 > Sven Van Asbroeck <thesven73@gmail.com> wrote: > >> Nikolaus, >> >> On Tue, Dec 1, 2020 at 9:38 AM H. Nikolaus Schaller <hns@goldelico.com> wrote: >>> >>> Let's work on a fix for the fix now. >>> >> >> Are you quite sure the chip-select of the tpo,td028ttec1 panel >> is active-high? A quick google produced a datasheet which >> seems to indicate that XCS is active-low? >> > Schematics is here: > https://projects.goldelico.com/p/gta04-main/downloads/48/ > > The display connector P204-LCD indicates some inversion at the XCS and > XRES pins. > > This patch fixes things for a boot where the display was not > touched by the bootloader > diff --git a/arch/arm/boot/dts/omap3-gta04.dtsi b/arch/arm/boot/dts/omap3-gta04.dtsi > index c8745bc800f7..003202d12990 100644 > --- a/arch/arm/boot/dts/omap3-gta04.dtsi > +++ b/arch/arm/boot/dts/omap3-gta04.dtsi > @@ -124,7 +124,6 @@ > spi-max-frequency = <100000>; > spi-cpol; > spi-cpha; > - spi-cs-high; BTW: that is what I had planned to try next... > > backlight= <&backlight>; > label = "lcd"; > > So if that one is really active-low, why did it ever work?! > > Regards, > Andreas ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [Letux-kernel] [BUG] SPI broken for SPI based panel drivers 2020-12-01 16:44 ` [Letux-kernel] " Andreas Kemnade 2020-12-01 16:51 ` H. Nikolaus Schaller 2020-12-01 16:52 ` H. Nikolaus Schaller @ 2020-12-01 16:55 ` Sven Van Asbroeck 2 siblings, 0 replies; 42+ messages in thread From: Sven Van Asbroeck @ 2020-12-01 16:55 UTC (permalink / raw) To: Andreas Kemnade Cc: Discussions about the Letux Kernel, H. Nikolaus Schaller, Linus Walleij, Mark Brown, kernel list, Laurent Pinchart Hi Andreas, On Tue, Dec 1, 2020 at 11:44 AM Andreas Kemnade <andreas@kemnade.info> wrote: > > So if that one is really active-low, why did it ever work?! Because the spi gpio chipselect was broken (inverted), and 766c6b63aa04 ("spi: fix client driver breakages when using GPIO descriptors") fixes it. ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [BUG] SPI broken for SPI based panel drivers 2020-12-01 14:20 ` Linus Walleij 2020-12-01 14:34 ` Sven Van Asbroeck 2020-12-01 14:35 ` H. Nikolaus Schaller @ 2020-12-01 16:20 ` Mark Brown 2020-12-01 16:41 ` H. Nikolaus Schaller 2 siblings, 1 reply; 42+ messages in thread From: Mark Brown @ 2020-12-01 16:20 UTC (permalink / raw) To: Linus Walleij Cc: H. Nikolaus Schaller, Sven Van Asbroeck, kernel list, Laurent Pinchart, Discussions about the Letux Kernel [-- Attachment #1: Type: text/plain, Size: 878 bytes --] On Tue, Dec 01, 2020 at 03:20:12PM +0100, Linus Walleij wrote: > The reason why I shoot in the dark to convert all SPI > drivers to use GPIO descriptors instead of the global > GPIO numberspace is detailed in drivers/gpio/TODO > so I will not repeat it here. > I don't know if much can be done about it other than > having better programmers than me at the task. Or > less tired when they write the patch. etc. I think the problem here is more to do with where we started than where we're going or how we got there - things have been glued together or happened to work in ways that mean I'm not sure we reasonably understand the situation we started from or all the requirements it has. As you say I'm not sure anything beyond throwing the API away and starting afresh would really help here, but that's not really how we tend to do things for a bunch of very good reasons. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [BUG] SPI broken for SPI based panel drivers 2020-12-01 16:20 ` Mark Brown @ 2020-12-01 16:41 ` H. Nikolaus Schaller 2020-12-01 17:11 ` Mark Brown 0 siblings, 1 reply; 42+ messages in thread From: H. Nikolaus Schaller @ 2020-12-01 16:41 UTC (permalink / raw) To: Mark Brown Cc: Linus Walleij, Sven Van Asbroeck, kernel list, Laurent Pinchart, Discussions about the Letux Kernel > Am 01.12.2020 um 17:20 schrieb Mark Brown <broonie@kernel.org>: > > On Tue, Dec 01, 2020 at 03:20:12PM +0100, Linus Walleij wrote: > >> The reason why I shoot in the dark to convert all SPI >> drivers to use GPIO descriptors instead of the global >> GPIO numberspace is detailed in drivers/gpio/TODO >> so I will not repeat it here. > >> I don't know if much can be done about it other than >> having better programmers than me at the task. Or >> less tired when they write the patch. etc. > > I think the problem here is more to do with where we started than where > we're going or how we got there - things have been glued together or > happened to work in ways that mean I'm not sure we reasonably understand > the situation we started from or all the requirements it has. As you > say I'm not sure anything beyond throwing the API away and starting > afresh would really help here, but that's not really how we tend to do > things for a bunch of very good reasons. I think the key problem is GPIO_ACTIVE_HIGH 0 and GPIO_ACTIVE_LOW 1 in device tree blobs. But that is so fundamental that we have to live with it. So I guess that even a new API from scratch wouldn't improve that. ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [BUG] SPI broken for SPI based panel drivers 2020-12-01 16:41 ` H. Nikolaus Schaller @ 2020-12-01 17:11 ` Mark Brown 0 siblings, 0 replies; 42+ messages in thread From: Mark Brown @ 2020-12-01 17:11 UTC (permalink / raw) To: H. Nikolaus Schaller Cc: Linus Walleij, Sven Van Asbroeck, kernel list, Laurent Pinchart, Discussions about the Letux Kernel [-- Attachment #1: Type: text/plain, Size: 1094 bytes --] On Tue, Dec 01, 2020 at 05:41:54PM +0100, H. Nikolaus Schaller wrote: > > Am 01.12.2020 um 17:20 schrieb Mark Brown <broonie@kernel.org>: > > I think the problem here is more to do with where we started than where > > we're going or how we got there - things have been glued together or > > happened to work in ways that mean I'm not sure we reasonably understand > > the situation we started from or all the requirements it has. As you > > say I'm not sure anything beyond throwing the API away and starting > > afresh would really help here, but that's not really how we tend to do > > things for a bunch of very good reasons. > I think the key problem is GPIO_ACTIVE_HIGH 0 and GPIO_ACTIVE_LOW 1 > in device tree blobs. But that is so fundamental that we have to live with it. > So I guess that even a new API from scratch wouldn't improve that. Yeah, that's definitely part of it - more generally there's multiple places trying to determine if the signal is inverted with different interactions/expectations. Having it in an ABI definitely contributes a lot to causing trouble though. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [BUG] SPI broken for SPI based panel drivers 2020-12-01 8:59 ` H. Nikolaus Schaller 2020-12-01 12:16 ` Mark Brown @ 2020-12-01 12:41 ` Sven Van Asbroeck 2020-12-01 13:32 ` H. Nikolaus Schaller 1 sibling, 1 reply; 42+ messages in thread From: Sven Van Asbroeck @ 2020-12-01 12:41 UTC (permalink / raw) To: H. Nikolaus Schaller Cc: Linus Walleij, Mark Brown, kernel list, Laurent Pinchart, Discussions about the Letux Kernel On Tue, Dec 1, 2020 at 4:04 AM H. Nikolaus Schaller <hns@goldelico.com> wrote: > > Then it should not have been applied to mainline but fully worked out and tested. > That would be a reasonable expectation of a product. But Linux isn't a product, it's a hugely complex, shared system, which may form the basis of your product. The core maintainers aren't superhuman, nor do they have access to the 1000s of configurations and devices where Linux runs or will run. They do their very best, but if every change had to be 100% tested in every possible configuration, then few things could ever change, and Linux would slow down to a snail's pace. When your product is based on Linux and you pull a newer version off kernel.org, it's not unreasonable to expect the occasional breakage. In my case, when I moved from 5.7 to 5.9, some of the things that broke were my network chip, and most SPI drivers. That was a bad day, most pulls are trouble-free. I believe LTSes are more stable than 'stable releases' which are in turn more stable than RCs. The choice involves a trade-off between features, security and stability. When you do run into a breakage, complaining on the mailing list is good, but posting a fix is better :) This is my layman's understanding of the situation, I'm just a user and not a maintainer. > > > >> > >> What should we do? Hopefully I have some time this week to look into your breakage, I may get overtaken by someone much more knowledgeable than me on spi-gpio. ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [BUG] SPI broken for SPI based panel drivers 2020-12-01 12:41 ` Sven Van Asbroeck @ 2020-12-01 13:32 ` H. Nikolaus Schaller 2020-12-01 14:08 ` Sven Van Asbroeck 0 siblings, 1 reply; 42+ messages in thread From: H. Nikolaus Schaller @ 2020-12-01 13:32 UTC (permalink / raw) To: Sven Van Asbroeck Cc: Linus Walleij, Mark Brown, kernel list, Laurent Pinchart, Discussions about the Letux Kernel Hi Sven, > Am 01.12.2020 um 13:41 schrieb Sven Van Asbroeck <thesven73@gmail.com>: > > On Tue, Dec 1, 2020 at 4:04 AM H. Nikolaus Schaller <hns@goldelico.com> wrote: >> >> Then it should not have been applied to mainline but fully worked out and tested. Well I only complain because you wrote that you knew that it may break something else. So it is known to induces a regression. But I did not know until I got a report from our own testing effort (we run a vendor kernel and do tests all days over the week). So I had to bisect what was going on. There could have been 100 reasons. Took me several hours because I had to look at the display to see if it is on or off after boot. There was no simple shell command to find out and let the bisect run over night. Maybe printing a "please check your spi setup" in spi_setup() with a comment hinting at your patch would have saved me a lot of time. > > That would be a reasonable expectation of a product. But Linux > isn't a product, it's a hugely complex, shared system, which may > form the basis of your product. The core maintainers aren't > superhuman, nor do they have access to the 1000s of configurations > and devices where Linux runs or will run. They do their very best, > but if every change had to be 100% tested in every possible > configuration, then few things could ever change, and Linux > would slow down to a snail's pace. > When your product is based on Linux and you pull a newer version > off kernel.org, it's not unreasonable to expect the occasional > breakage. In my case, when I moved from 5.7 to 5.9, some of the > things that broke were my network chip, and most SPI drivers. That > was a bad day, most pulls are trouble-free. If it were occasional it would be good.... It is not the first time that I have to do bisect tests to find what did go wrong upstream. Even in LTS kernels and some of my fixes were backported later. > I believe LTSes are more stable than 'stable releases' which are in > turn more stable than RCs. The choice involves a trade-off between > features, security and stability. > > When you do run into a breakage, complaining on the mailing list > is good, but posting a fix is better :) I usually do - if I have the time to go the extra mile to fix something what somebody else did break. But I need to have a lot of spare time if I have no idea what made the regression for a device driver where I know that it was not touched and have to study the details. And for me it is much less effort (hours vs. seconds) to do a revert... I could submit that as a quick fix :) But then you are not happy. > This is my layman's understanding of the situation, I'm just a user > and not a maintainer. Me too... Well, I am sort of maintainer of a vendor kernel that tries to follow linus/master and fix things before we release an LTS. That is why I need a solution and can not go back to some LTS or wait for the next one... Anyways, there is still time until v5.10.0 to fix it better than by a revert. > >>> >>>> >>>> What should we do? > > Hopefully I have some time this week to look into your breakage, > I may get overtaken by someone much more knowledgeable than > me on spi-gpio. Hm. Then we are already two of us who have not much knowledge about spi-gpio... Hope that you have an idea soon. I am happy to test any suggestions/patches/alternatives better than a simple revert. BR and thanks, Nikolaus ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [BUG] SPI broken for SPI based panel drivers 2020-12-01 13:32 ` H. Nikolaus Schaller @ 2020-12-01 14:08 ` Sven Van Asbroeck 2020-12-01 15:33 ` Mark Brown 0 siblings, 1 reply; 42+ messages in thread From: Sven Van Asbroeck @ 2020-12-01 14:08 UTC (permalink / raw) To: H. Nikolaus Schaller Cc: Linus Walleij, Mark Brown, kernel list, Laurent Pinchart, Discussions about the Letux Kernel On Tue, Dec 1, 2020 at 8:36 AM H. Nikolaus Schaller <hns@goldelico.com> wrote: > > Well I only complain because you wrote that you knew that it may > break something else. So it is known to induces a regression. We knew that it would fix an important, common problem, but we also knew that there is always a possibility of breaking something else when making a change to the core. > > Maybe printing a "please check your spi setup" in spi_setup() with > a comment hinting at your patch would have saved me a lot of time. > You could ask the maintainer for such a policy, but I fear that soon the code would emit too many "please check" messages. > > Well, I am sort of maintainer of a vendor kernel that tries to > follow linus/master and fix things before we release an LTS. Makes sense, I understand your situation better now. > > Anyways, there is still time until v5.10.0 to fix it better than by > a revert. When we find a fix, it'll have a Fixes: tag, which means it'll automatically be applied to every supported kernel, including v5.10 even if already released. > > Hope that you have an idea soon. I am happy to test any suggestions/patches/alternatives > better than a simple revert. > Thank you, that's great. I may come back with a few suggestions for you to test this week. ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [BUG] SPI broken for SPI based panel drivers 2020-12-01 14:08 ` Sven Van Asbroeck @ 2020-12-01 15:33 ` Mark Brown 0 siblings, 0 replies; 42+ messages in thread From: Mark Brown @ 2020-12-01 15:33 UTC (permalink / raw) To: Sven Van Asbroeck Cc: H. Nikolaus Schaller, Linus Walleij, kernel list, Laurent Pinchart, Discussions about the Letux Kernel [-- Attachment #1: Type: text/plain, Size: 986 bytes --] On Tue, Dec 01, 2020 at 09:08:34AM -0500, Sven Van Asbroeck wrote: > On Tue, Dec 1, 2020 at 8:36 AM H. Nikolaus Schaller <hns@goldelico.com> wrote: > > Well I only complain because you wrote that you knew that it may > > break something else. So it is known to induces a regression. > We knew that it would fix an important, common problem, but > we also knew that there is always a possibility of breaking > something else when making a change to the core. It's worth pointing out that this is an exceptionally fragile area of the code thanks to some regrettable historical decisions with regard to how GPIOs are managed so this assessment is based on repeated past experiences with changes that look sensible, fix real problems for real systems and yet cause problems to crop up elsewhere due to unforseen interactions elsewhere. Eventually we'll shake out all these issues and end up with something that's more understandable and hence managable but clearly we aren't there yet. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [BUG] SPI broken for SPI based panel drivers 2020-11-30 20:13 ` Sven Van Asbroeck 2020-11-30 20:22 ` Sven Van Asbroeck 2020-12-01 8:59 ` H. Nikolaus Schaller @ 2020-12-05 20:57 ` Pavel Machek 2020-12-07 13:43 ` Mark Brown 2 siblings, 1 reply; 42+ messages in thread From: Pavel Machek @ 2020-12-05 20:57 UTC (permalink / raw) To: Sven Van Asbroeck Cc: H. Nikolaus Schaller, Linus Walleij, Mark Brown, kernel list, Laurent Pinchart, Discussions about the Letux Kernel [-- Attachment #1: Type: text/plain, Size: 836 bytes --] On Mon 2020-11-30 15:13:02, Sven Van Asbroeck wrote: > Hi Nikolaus, thank you for reaching out ! > > On Mon, Nov 30, 2020 at 2:06 PM H. Nikolaus Schaller <hns@goldelico.com> wrote: > > > > But reverting your patch brings back the display. So it appears as if it does not > > fix a breakage, rather breaks a previously working setup. > > The patch in question fixes an important breakage: before the patch, literally > hundreds of SPI drivers no longer worked - only if the SPI bus master > driver was using gpio descriptors. > > We knew that there was a chance that our fix would break something else. > But hopefully "it fixes more than it breaks" If the patch causes regression it will ultimately need to be reverted... no matter how much it fixes. Pavel -- http://www.livejournal.com/~pavelmachek [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [BUG] SPI broken for SPI based panel drivers 2020-12-05 20:57 ` Pavel Machek @ 2020-12-07 13:43 ` Mark Brown 0 siblings, 0 replies; 42+ messages in thread From: Mark Brown @ 2020-12-07 13:43 UTC (permalink / raw) To: Pavel Machek Cc: Sven Van Asbroeck, H. Nikolaus Schaller, Linus Walleij, kernel list, Laurent Pinchart, Discussions about the Letux Kernel [-- Attachment #1: Type: text/plain, Size: 212 bytes --] On Sat, Dec 05, 2020 at 09:57:44PM +0100, Pavel Machek wrote: > If the patch causes regression it will ultimately need to be > reverted... no matter how much it fixes. It's regression fixes all the way down... [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 42+ messages in thread
end of thread, other threads:[~2020-12-09 8:48 UTC | newest] Thread overview: 42+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-11-30 19:03 [BUG] SPI broken for SPI based panel drivers H. Nikolaus Schaller 2020-11-30 20:13 ` Sven Van Asbroeck 2020-11-30 20:22 ` Sven Van Asbroeck 2020-12-01 8:59 ` H. Nikolaus Schaller 2020-12-01 12:16 ` Mark Brown 2020-12-01 14:05 ` H. Nikolaus Schaller 2020-12-01 14:20 ` Linus Walleij 2020-12-01 14:34 ` Sven Van Asbroeck 2020-12-01 14:35 ` H. Nikolaus Schaller 2020-12-01 15:52 ` Sven Van Asbroeck 2020-12-01 16:46 ` H. Nikolaus Schaller 2020-12-01 16:10 ` Sven Van Asbroeck 2020-12-01 16:39 ` H. Nikolaus Schaller 2020-12-01 16:53 ` Sven Van Asbroeck 2020-12-01 17:10 ` H. Nikolaus Schaller 2020-12-01 18:43 ` Sven Van Asbroeck 2020-12-02 12:19 ` Mark Brown 2020-12-04 10:08 ` H. Nikolaus Schaller 2020-12-04 13:46 ` Sven Van Asbroeck 2020-12-04 16:49 ` H. Nikolaus Schaller 2020-12-04 19:19 ` Sven Van Asbroeck 2020-12-04 21:31 ` H. Nikolaus Schaller 2020-12-05 0:25 ` Linus Walleij 2020-12-05 7:04 ` H. Nikolaus Schaller 2020-12-09 8:04 ` Andreas Kemnade 2020-12-09 8:40 ` H. Nikolaus Schaller 2020-12-09 8:38 ` Linus Walleij 2020-12-09 8:45 ` H. Nikolaus Schaller 2020-12-01 22:51 ` Linus Walleij 2020-12-01 16:44 ` [Letux-kernel] " Andreas Kemnade 2020-12-01 16:51 ` H. Nikolaus Schaller 2020-12-01 16:52 ` H. Nikolaus Schaller 2020-12-01 16:55 ` Sven Van Asbroeck 2020-12-01 16:20 ` Mark Brown 2020-12-01 16:41 ` H. Nikolaus Schaller 2020-12-01 17:11 ` Mark Brown 2020-12-01 12:41 ` Sven Van Asbroeck 2020-12-01 13:32 ` H. Nikolaus Schaller 2020-12-01 14:08 ` Sven Van Asbroeck 2020-12-01 15:33 ` Mark Brown 2020-12-05 20:57 ` Pavel Machek 2020-12-07 13:43 ` Mark Brown
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).