Re: [PATCH 03/10] ARM: dts: omap: dm816x: use dedicated SoC pinctrl

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] pinctrl: dm816x API provides broken PIN_INPUT/PIN_OUTPUT macros and omits DT fixes
--

commit 8bc3ab0228d1fbf7757bd0740351d9face4c0e7c
Author: Ettore Chimenti <ettore.chimenti@linaro.org>

ARM: dts: omap: dm816x: use dedicated SoC pinctrl

In the TI Sitara DM816x SoCs, the pinctrl configuration macros are not
identical to OMAP2/3. They are actually inverted between Pullup/down
enabled and disabled (third bit in pinctrl registers). Using dedicated
DM816x pinctrl header to differentiate between the SoC families.

> diff --git a/arch/arm/boot/dts/ti/omap/dm816x.dtsi b/arch/arm/boot/dts/ti/omap/dm816x.dtsi
> index ee0090f7aa64e..de4d48babf778 100644
> --- a/arch/arm/boot/dts/ti/omap/dm816x.dtsi
> +++ b/arch/arm/boot/dts/ti/omap/dm816x.dtsi
> @@ -3,7 +3,7 @@
>  #include <dt-bindings/bus/ti-sysc.h>
>  #include <dt-bindings/clock/dm816.h>
>  #include <dt-bindings/gpio/gpio.h>
> -#include <dt-bindings/pinctrl/omap.h>
> +#include <dt-bindings/pinctrl/dm816x.h>

[Severity: Medium]
Since dm816x.h inherits macros from omap.h, will using PIN_INPUT or
PIN_OUTPUT unintentionally enable an active pull-down on DM816x?

include/dt-bindings/pinctrl/omap.h defines:
#define PIN_INPUT        INPUT_EN
#define PIN_OUTPUT       0

Because neither includes bit 3 (so it remains 0), and DM816x has inverted
logic where bit 3=0 enables the pull resistor, these macros now actively turn
on pull-downs. Should dm816x.h #undef and redefine PIN_INPUT and PIN_OUTPUT
to include PULL_DIS?

Similarly, does this inclusion leak inapplicable off-mode macros like
PIN_OFF_OUTPUT_HIGH into the DM816x namespace, where they might write garbage
to inapplicable padconf bits if used?

Additionally, while this commit changes the header to allow fixing the pull
logic, does this leave pins in arch/arm/boot/dts/ti/omap/dm8168-evm.dts with
unintended pull-downs?

For example, SPI_SCLK is defined without a pull flag:

arch/arm/boot/dts/ti/omap/dm8168-evm.dts:mcspi1_pins() {
    ...
    DM816X_IOPAD(0x0a94, MUX_MODE0)    /* SPI_SCLK */
    ...
}

Under the old OMAP rules, omitting PULL_ENA disabled the pull. But with the
new DM816x rules, omitting PULL_DIS leaves bit 3=0, which enables the pull.

While follow-up patches in the series fix the SD card pins, do other pins
like SPI_SCLK and SPI_SCS0 need to be updated to explicitly include PULL_DIS?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016-patch-ektor5-dm8168-dts-v1-0-b4ce30217028@linaro.org?part=3