Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [Medium] dt: commit message claims MCU GPIO0 but uses Main GPIO0 - [Low] dt: incorrect inline comment for busy_pins_default pad - [Low] dt: raw integer used for GPIO flags instead of macro --
commit d0e18d861dbf41de9b7a268b82e7b844f57b80dc Author: Devarsh Thakkar <[email protected]> arm64: dts: ti: Add AM62L3 EVM overlay for GDEY042T81 e-paper display Add device tree overlay for the Good Display GDEY042T81 4.2" e-paper panel connected to the AM62L3 EVM via SPI. > The overlay configures SPI1 with the SSD1683-based GDEY042T81 panel > at 2MHz, with reset, busy, and DC GPIOs connected to MCU GPIO0. [Severity: Medium] Is the commit message accurate regarding the GPIO domain? The commit message states that the display GPIOs are connected to MCU GPIO0, but the device tree nodes explicitly reference &gpio0, which corresponds to the Main domain GPIO0 controller in the K3 architecture (the MCU domain would be &mcu_gpio0 or &wkup_gpio0). Additionally, the configured pad offsets (0x0080, 0x007c, 0x00b8) belong to the Main domain GPMC0 peripheral, verifying that the physical hardware routing targets the Main domain. Could the commit message be updated to reflect that Main GPIO0 is being used? > diff --git a/arch/arm64/boot/dts/ti/k3-am62l3-evm-epaper-gdey042t81.dtso > b/arch/arm64/boot/dts/ti/k3-am62l3-evm-epaper-gdey042t81.dtso > new file mode 100644 > index 0000000000000..50975a2cdd2a1 > --- /dev/null > +++ b/arch/arm64/boot/dts/ti/k3-am62l3-evm-epaper-gdey042t81.dtso > @@ -0,0 +1,96 @@ [ ... ] > + busy_pins_default: epaper-busy-default-pins { > + pinctrl-single,pins = < > + AM62LX_IOPAD(0x00b8, PIN_INPUT, 7) /* (L21) > GPMC0_AD0.GPIO0_31 */ > + >; > + }; [Severity: Low] Does this inline comment misidentify the pad name? The overlay correctly uses the offset 0x00b8 for the busy-gpios pin, but the comment next to it reads GPMC0_AD0.GPIO0_31. In the AM62L pad configuration, offset 0x00b8 actually corresponds to the GPMC0_CLK pad. The GPMC0_AD0 pad resides at offset 0x0078. Should the comment be updated to prevent confusion during future pinmux debugging? [ ... ] > +/* Drive GPIO0_89 high to select the SPI1 path on the VOUT0 FET switch */ > +&gpio0 { > + fet-sel-hog { > + gpio-hog; > + gpios = <89 0>; [Severity: Low] Should this use the GPIO_ACTIVE_HIGH macro instead of a raw integer? While functionally correct (0 equates to GPIO_ACTIVE_HIGH), standard kernel device tree styling strictly prefers using the named macro for readability and consistency, especially since <dt-bindings/gpio/gpio.h> is included at the top of the file. > + output-high; > + line-name = "GPIO_EXP_CONN_FET_SEL"; > + }; > +}; -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=14
