Re: [PATCH v3 1/3] Input: ads7846 - Convert to use software nodes

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
To: Linus Walleij <linus.walleij@linaro.org>
Cc: Aaro Koskinen <aaro.koskinen@iki.fi>,
	Janusz Krzysztofik <jmkrzyszt@gmail.com>,
	Tony Lindgren <tony@atomide.com>,
	Russell King <linux@armlinux.org.uk>,
	Daniel Mack <daniel@zonque.org>,
	Haojian Zhuang <haojian.zhuang@gmail.com>,
	Robert Jarzmik <robert.jarzmik@free.fr>,
	Thomas Bogendoerfer <tsbogend@alpha.franken.de>,
	Mark Brown <broonie@kernel.org>,
	Bartosz Golaszewski <bartosz.golaszewski@linaro.org>,
	Andreas Kemnade <andreas@kemnade.info>,
	Helge Deller <deller@gmx.de>,
	Ulf Hansson <ulf.hansson@linaro.org>,
	linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, linux-mips@vger.kernel.org,
	linux-input@vger.kernel.org, linux-spi@vger.kernel.org,
	linux-fbdev@vger.kernel.org, dri-devel@lists.freedesktop.org,
	linux-mmc@vger.kernel.org
Subject: Re: [PATCH v3 1/3] Input: ads7846 - Convert to use software nodes
Date: Fri, 5 May 2023 11:08:50 -0700	[thread overview]
Message-ID: <ZFVGMiuRT+e2eVXw@google.com> (raw)
In-Reply-To: <20230430-nokia770-regression-v3-1-a6d0a89ffa8b@linaro.org>

Hi Linus,
On Fri, May 05, 2023 at 01:16:55PM +0200, Linus Walleij wrote:
> 
> 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.

Wow, so that worked , awesome!

> +static const struct property_entry nokia770_ads7846_props[] = {
> +	PROPERTY_ENTRY_U32("touchscreen-size-x", 4096),
> +	PROPERTY_ENTRY_U32("touchscreen-size-y", 4096),
> +	PROPERTY_ENTRY_U32("touchscreen-max-pressure", 256),
> +	PROPERTY_ENTRY_U32("touchscreen-average-samples", 10),
> +	PROPERTY_ENTRY_U16("ti,x-plate-ohms", 180),
> +	PROPERTY_ENTRY_U16("ti,debounce-tol", 3),
> +	PROPERTY_ENTRY_U16("ti,debounce-rep", 1),
> +	PROPERTY_ENTRY_GPIO("pendown-gpios", &nokia770_gpiochip1_node,
> +			    ADS7846_PENDOWN_GPIO, GPIO_ACTIVE_HIGH),

Looking at the driver this actually needs to be GPIO_ACTIVE_LOW.

>  
> +static struct gpiod_lookup_table spitz_ads7846_gpio_table = {
> +	.dev_id = "spi2.0",
> +	.table = {
> +		GPIO_LOOKUP("gpio-pxa", SPITZ_GPIO_TP_INT,
> +			    "pendown", GPIO_ACTIVE_HIGH),

GPIO_ACTIVE_LOW here too.

> +static struct gpiod_lookup_table db1100_touch_gpio_table = {
> +	.dev_id = "spi0.0",
> +	.table = {
> +		GPIO_LOOKUP("alchemy-gpio2", 21,
> +			    "pendown", GPIO_ACTIVE_HIGH),

And here as well.

> @@ -223,7 +220,7 @@ static int get_pendown_state(struct ads7846 *ts)
>  	if (ts->get_pendown_state)
>  		return ts->get_pendown_state();
>  
> -	return !gpio_get_value(ts->gpio_pendown);
> +	return !gpiod_get_value(ts->gpio_pendown);

This needs to be

	return !gpiod_get_value_raw(ts->gpio_pendown);

I looked at various DTSes we have and they use a mix of active high and
active low annotations, so we have to go with the "raw" variant for now,
and then update to normal one once we update bad DTSes.

Thanks!

-- 
Dmitry