Re: [PATCH 2/3] powerpc/dts: pdm360ng: Convert ADS7845 touchscreen to DT bindings

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The `pendown-gpio` property is configured with active-high polarity instead of active-low, which breaks touchscreen input and causes an interrupt storm.
- [High] Legacy platform data and raw initialization code for the touchscreen were not removed, leaving dead code and conflicting hardware initialization.
--

--- Patch [2]: [PATCH 2/3] powerpc/dts: pdm360ng: Convert ADS7845 touchscreen to DT bindings ---
commit 1e38d4980520e39bd3a703aa35c53c86099afebb
Author: Dmitry Torokhov <dmitry.torokhov@gmail.com>

powerpc/dts: pdm360ng: Convert ADS7845 touchscreen to DT bindings

The PDM360NG board is equipped with an ADS7845 touchscreen controller.
Previously, pdm360ng.dts used "ti,ads7846" as a placeholder and relied
on platform data in pdm360ng.c to override the model to 7845.
Update the ts@0 touchscreen node in pdm360ng.dts to use the correct
"ti,ads7845" compatible string along with pendown-gpio and
interrupt-parent properties referencing gpio_pic.

> diff --git a/arch/powerpc/boot/dts/pdm360ng.dts b/arch/powerpc/boot/dts/pdm360ng.dts
> index 67c3b9db75d76..69d8b6a0241cb 100644
> --- a/arch/powerpc/boot/dts/pdm360ng.dts
> +++ b/arch/powerpc/boot/dts/pdm360ng.dts
> @@ -176,11 +176,12 @@ psc@11900 {
>  
>  			/* ADS7845 touch screen controller */
>  			ts@0 {
> -				compatible = "ti,ads7846";
> +				compatible = "ti,ads7845";

[Severity: High]
Will updating the compatible string here leave conflicting legacy
initialization active in arch/powerpc/platforms/512x/pdm360ng.c?

Since pdm360ng_touchscreen_notifier_call() checks for "ti,ads7846", this
change will cause the notifier to silently fail, abandoning the platform
data injection and leaving it as dead code.

Additionally, pdm360ng_penirq_init() still executes unconditionally on boot,
performing uncoordinated direct memory writes to configure the GPIO
controller interrupt.

Since the touchscreen is now natively bound to the GPIO subsystem via DT,
will this raw memory manipulation conflict with the subsystem's state?

>  				reg = <0x0>;
>  				spi-max-frequency = <3000000>;
> -				/* pen irq is GPIO25 */
> -				interrupts = <78 0x8>;
> +				interrupt-parent = <&gpio_pic>;
> +				interrupts = <25 0x8>;
> +				pendown-gpio = <&gpio_pic 25 0>;

[Severity: High]
Is this pendown-gpio configuration expected to use the GPIO_ACTIVE_HIGH
flag (0)?

The interrupts property correctly defines a level-low trigger (0x8), but the
pendown-gpio property uses 0.

When the pen is down, the driver's gpiod_get_value() in get_pendown_state()
reads the physical 0 and returns a logical 0. This signals to ads7846_irq()
that the pen is up, aborting without executing the SPI sequence needed to
clear the hardware interrupt.

Because the interrupt is level-triggered and never cleared, it fires
continuously.

Should this use GPIO_ACTIVE_LOW (1) instead to prevent an interrupt storm?

>  			};
>  		};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260718-ads7846-pdm360ng-v1-0-f318f85cda57@gmail.com?part=2