| Message ID | 20230430-nokia770-regression-v4-0-9b6dc5536b17@linaro.org |
|---|---|
| Headers | show |
| Series | Fix up Nokia 770 regression | expand |
Hi, This does not compile as nokia770_ads7846_props is declared twice, and nokia770_cbus_props and nokia770_mpuio_gpiochip_swnode are missing. On Mon, May 08, 2023 at 11:20:06PM +0200, Linus Walleij wrote: > +static const struct software_node_ref_args nokia770_cbus_gpio_refs[] = { > + SOFTWARE_NODE_REFERENCE(&nokia770_mpuio_gpiochip_swnode, 9, 0), > + SOFTWARE_NODE_REFERENCE(&nokia770_mpuio_gpiochip_swnode, 10, 0), > + SOFTWARE_NODE_REFERENCE(&nokia770_mpuio_gpiochip_swnode, 11, 0), > +}; These should be nokia770_mpuio_gpiochip_node. > +static const struct property_entry nokia770_ads7846_props[] = { > + PROPERTY_ENTRY_REF_ARRAY("gpios", nokia770_cbus_gpio_refs), > + { } > }; This should be nokia770_cbus_props. A.
Hi,
On Mon, May 08, 2023 at 11:20:06PM +0200, Linus Walleij wrote:
> The CBUS also has the ADS7846 touchscreen attached.
Not sure what this comment means. CBUS is for Retu/Tahvo, and touchscreen
is SPI.
When tested w/gpio-descriptors-omap branch, the touchscreen probe fails:
[ 2.378540] SPI driver ads7846 has no spi_device_id for ti,tsc2046
[ 2.391906] SPI driver ads7846 has no spi_device_id for ti,ads7843
[ 2.405029] SPI driver ads7846 has no spi_device_id for ti,ads7845
[ 2.418151] SPI driver ads7846 has no spi_device_id for ti,ads7873
[ 2.432556] ads7846 spi2.0: Unknown device model
[ 2.443817] ads7846: probe of spi2.0 failed with error -22
I don't know if that's caused by any the patches in the branch or some
other regression. With v6.2 it probes OK.
A.
On Wed, May 17, 2023 at 9:59 PM Aaro Koskinen <aaro.koskinen@iki.fi> wrote: > This does not compile as nokia770_ads7846_props is declared twice, > and nokia770_cbus_props and nokia770_mpuio_gpiochip_swnode are missing. Hmmmm I think we should probably update omap1_defconfig to enable all the OMAP1 drivers so we have good compile coverage. It's the ifdefs that fool me into believeing the code actually compiles ... > On Mon, May 08, 2023 at 11:20:06PM +0200, Linus Walleij wrote: > > +static const struct software_node_ref_args nokia770_cbus_gpio_refs[] = { > > + SOFTWARE_NODE_REFERENCE(&nokia770_mpuio_gpiochip_swnode, 9, 0), > > + SOFTWARE_NODE_REFERENCE(&nokia770_mpuio_gpiochip_swnode, 10, 0), > > + SOFTWARE_NODE_REFERENCE(&nokia770_mpuio_gpiochip_swnode, 11, 0), > > +}; > > These should be nokia770_mpuio_gpiochip_node. Fixed it. > > +static const struct property_entry nokia770_ads7846_props[] = { > > + PROPERTY_ENTRY_REF_ARRAY("gpios", nokia770_cbus_gpio_refs), > > + { } > > }; > > This should be nokia770_cbus_props. Fixed it. Also enabled CONFIG_I2C_CBUS_GPIO and recompiled. Yours, Linus Walleij Yours, Linus Walleij
On Wed, May 17, 2023 at 10:39 PM Aaro Koskinen <aaro.koskinen@iki.fi> wrote: > When tested w/gpio-descriptors-omap branch, the touchscreen probe fails: > > [ 2.378540] SPI driver ads7846 has no spi_device_id for ti,tsc2046 > [ 2.391906] SPI driver ads7846 has no spi_device_id for ti,ads7843 > [ 2.405029] SPI driver ads7846 has no spi_device_id for ti,ads7845 > [ 2.418151] SPI driver ads7846 has no spi_device_id for ti,ads7873 This is just regular noise from SPI device drivers that are missing spi_device_id. > [ 2.432556] ads7846 spi2.0: Unknown device model > [ 2.443817] ads7846: probe of spi2.0 failed with error -22 > > I don't know if that's caused by any the patches in the branch or some > other regression. With v6.2 it probes OK. The device is missing compatible. I fixed it. Will push the branch after looking at the rest of the comments. Yours, Linus Walleij
Le 08/05/2023 à 23:20, Linus Walleij a écrit : > The Nokia 770 is using GPIOs from the global numberspace on the > CBUS node to pass down to the LCD controller. This regresses when we > let the OMAP GPIO driver use dynamic GPIO base. > > The Nokia 770 now has dynamic allocation of IRQ numbers, so this > needs to be fixed for it to work. > > As this is the only user of LCD MIPID we can easily augment the > driver to use a GPIO descriptor instead and resolve the issue. > > The platform data .shutdown() callback wasn't even used in the > code, but we encode a shutdown asserting RESET in the remove() > callback for completeness sake. > > The CBUS also has the ADS7846 touchscreen attached. > > Populate the devices on the Nokia 770 CBUS I2C using software > nodes instead of platform data quirks. This includes the LCD > and the ADS7846 touchscreen so the conversion just brings the LCD > along with it as software nodes is an all-or-nothing design > pattern. > > The ADS7846 has some limited support for using GPIO descriptors, > let's convert it over completely to using device properties and then > fix all remaining boardfile users to provide all platform data using > software nodes. > > Dump the of includes and of_match_ptr() in the ADS7846 driver as part > of the job. > > Since we have to move ADS7846 over to obtaining the GPIOs it is > using exclusively from descriptors, we provide descriptor tables > for the two remaining in-kernel boardfiles using ADS7846: > > - PXA Spitz > - MIPS Alchemy DB1000 development board > > It was too hard for me to include software node conversion of > these two remaining users at this time: the spitz is using a > hscync callback in the platform data that would require further > GPIO descriptor conversion of the Spitz, and moving the hsync > callback down into the driver: it will just become too big of > a job, but it can be done separately. > > The MIPS Alchemy DB1000 is simply something I cannot test, so take > the easier approach of just providing some GPIO descriptors in > this case as I don't want the patch to grow too intrusive. > > As we see that several device trees have incorrect polarity flags > and just expect to bypass the gpiolib polarity handling, fix up > all device trees too, in a separate patch. > > Suggested-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> > Fixes: 92bf78b33b0b ("gpio: omap: use dynamic allocation of base") > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > --- [...] > diff --git a/drivers/video/fbdev/omap/lcd_mipid.c b/drivers/video/fbdev/omap/lcd_mipid.c > index 03cff39d392d..e4a7f0b824ff 100644 > --- a/drivers/video/fbdev/omap/lcd_mipid.c > +++ b/drivers/video/fbdev/omap/lcd_mipid.c > @@ -7,6 +7,7 @@ > */ > #include <linux/device.h> > #include <linux/delay.h> > +#include <linux/gpio/consumer.h> > #include <linux/slab.h> > #include <linux/workqueue.h> > #include <linux/spi/spi.h> > @@ -41,6 +42,7 @@ struct mipid_device { > when we can issue the > next sleep in/out command */ > unsigned long hw_guard_wait; /* max guard time in jiffies */ > + struct gpio_desc *reset; > > struct omapfb_device *fbdev; > struct spi_device *spi; > @@ -556,6 +558,12 @@ static int mipid_spi_probe(struct spi_device *spi) > return -ENOMEM; > } > > + /* This will de-assert RESET if active */ > + md->reset = gpiod_get(&spi->dev, "reset", GPIOD_OUT_LOW); > + if (IS_ERR(md->reset)) > + return dev_err_probe(&spi->dev, PTR_ERR(md->reset), > + "no reset GPIO line\n"); > + > spi->mode = SPI_MODE_0; > md->spi = spi; > dev_set_drvdata(&spi->dev, md); > @@ -574,6 +582,8 @@ static void mipid_spi_remove(struct spi_device *spi) > { > struct mipid_device *md = dev_get_drvdata(&spi->dev); > > + /* Asserts RESET */ > + gpiod_set_value(md->reset, 1); Hi, should this also be done in the probe if mipid_detect() fails? If yes, please also look at [1], that I've just sent, which introduces an error handling path in the probe. CJ [1]: https://lore.kernel.org/all/8b82e34724755b69f34f15dddb288cd373080390.1620505229.git.christophe.jaillet@wanadoo.fr/ > mipid_disable(&md->panel); > kfree(md); > } [...]
On Sun, Jun 4, 2023 at 5:44 PM Christophe JAILLET <christophe.jaillet@wanadoo.fr> wrote: > > + /* Asserts RESET */ > > + gpiod_set_value(md->reset, 1); > > Hi, > > should this also be done in the probe if mipid_detect() fails? It's a nice bonus but surely not urgent or necessary. > If yes, please also look at [1], that I've just sent, which introduces > an error handling path in the probe. Looks good to me. Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Yours, Linus Walleij
A recent change to use dynamic GPIO base allocation in the OMAP GPIO driver caused a regression in some OMAP1 boards. This series fixes up the Nokia 770 board from 2005: https://en.wikipedia.org/wiki/Nokia_770_Internet_Tablet I don't know how urgent the fix is, you decide. For me, it is fair if fringe systems get fixed in due time, as they are hardly anyones main development laptop. Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- Changes in v4: - Fix up the polarity issues identified by Dmitry. - Fix all erronous device trees as well. - Link to v3: https://lore.kernel.org/r/20230430-nokia770-regression-v3-0-a6d0a89ffa8b@linaro.org Changes in v3: - Fix a compile error in the ADS7846 driver by dropping some leftover OF ifdeffery. - Link to v2: https://lore.kernel.org/r/20230430-nokia770-regression-v2-0-984ed3ca5444@linaro.org Changes in v2: - Thoroughly rewrote the approach taken for the ADS7846 touchscreen following Dmitry's ambition to go a step further and take a swnode approach to this conversion: I'm fine with that, the patch just get a bit bigger. - Picked up Ulf's ACK on the MMC patch. - Link to v1: https://lore.kernel.org/r/20230430-nokia770-regression-v1-0-97704e36b094@linaro.org --- Linus Walleij (4): Input: ads7846 - Convert to use software nodes ARM/mmc: Convert old mmci-omap to GPIO descriptors ARM: omap1: Fix up the Nokia 770 board device IRQs ARM: dts: Fix erroneous ADS touchscreen polarities arch/arm/boot/dts/am57xx-cl-som-am57x.dts | 2 +- arch/arm/boot/dts/at91sam9261ek.dts | 2 +- arch/arm/boot/dts/imx7d-pico-hobbit.dts | 2 +- arch/arm/boot/dts/imx7d-sdb.dts | 2 +- arch/arm/boot/dts/omap3-cm-t3x.dtsi | 2 +- arch/arm/boot/dts/omap3-devkit8000-lcd-common.dtsi | 2 +- arch/arm/boot/dts/omap3-lilly-a83x.dtsi | 2 +- arch/arm/boot/dts/omap3-overo-common-lcd35.dtsi | 2 +- arch/arm/boot/dts/omap3-overo-common-lcd43.dtsi | 2 +- arch/arm/boot/dts/omap3-pandora-common.dtsi | 2 +- arch/arm/boot/dts/omap5-cm-t54.dts | 2 +- arch/arm/mach-omap1/board-nokia770.c | 198 +++++++++++++-------- arch/arm/mach-omap1/board-sx1-mmc.c | 1 - arch/arm/mach-omap2/board-n8x0.c | 85 +++------ arch/arm/mach-pxa/spitz.c | 11 +- arch/mips/alchemy/devboards/db1000.c | 11 +- drivers/input/touchscreen/ads7846.c | 113 +++++------- drivers/mmc/host/omap.c | 46 ++++- drivers/video/fbdev/omap/lcd_mipid.c | 10 ++ include/linux/platform_data/lcd-mipid.h | 2 - include/linux/platform_data/mmc-omap.h | 2 - include/linux/spi/ads7846.h | 2 - 22 files changed, 273 insertions(+), 230 deletions(-) --- base-commit: ac9a78681b921877518763ba0e89202254349d1b change-id: 20230430-nokia770-regression-2b5a07497ec9 Best regards,