Re: [PATCH 2/3] arm: dts: omap: Add support for BeagleBone Black RevD

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:
- [High] Hardware/Driver mismatch: The BeagleBone Black physically routes 16-bit video data, but the DT and bridge driver are configured for a 24-bit bus, leading to severely corrupted video output.
- [Medium] The IT66122 bridge interrupt is incorrectly configured as edge-falling instead of level-low, which can lead to missed interrupts.
- [Medium] The board compatible string is missing standard fallback identifiers, breaking userspace board detection.
- [Low] The new board compatible string 'beagle,am335x-bone-black-revd' is not documented in the Devicetree bindings.
- [Low] Stale gpio-line-names entry masks the HDMI reset pin (GPIO1_9) as 'NC' (Not Connected).
- [Low] The lcdc endpoint node uses a unit address without a matching reg property, causing dtc warnings.
- [Low] A stale comment incorrectly references the removed TDA19988 HDMI framer node.
--

commit d7b1924005badde2a155d4ba4a4cb12a5f9be451
Author: Robert Nelson <robertcnelson@gmail.com>

arm: dts: omap: Add support for BeagleBone Black RevD

The BeagleBoard.org BeagleBone Black RevD and Industrial (00D0/EID0) are
redesigns of the BeagleBone Black Rev C3. They feature an IT66122 HDMI PHY to
replace the EOL NXP TDA19988. Additionally, these models include a larger eMMC
capacity to address the limited availability of 4GB modules.

> diff --git a/arch/arm/boot/dts/ti/omap/am335x-boneblack-ite-hdmi.dtsi b/arch/arm/boot/dts/ti/omap/am335x-boneblack-ite-hdmi.dtsi
> new file mode 100644
> index 0000000000000..77dd7d9aa57d9
> --- /dev/null
> +++ b/arch/arm/boot/dts/ti/omap/am335x-boneblack-ite-hdmi.dtsi
> @@ -0,0 +1,184 @@

[ ... ]

> +&lcdc {
> +	status = "okay";
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&nxp_hdmi_bonelt_pins>;
> +
> +	/* If you want to get 24 bit RGB and 16 BGR mode instead of
> +	 * current 16 bit RGB and 24 BGR modes, set the propety
> +	 * below to "crossed" and uncomment the video-ports -property
> +	 * in tda19988 node.
> +	 */

[Severity: Low]
Is this comment still accurate? The comment instructs to uncomment a property
in the tda19988 node, but the Rev D board replaced this chip with the IT66122.

> +	blue-and-red-wiring = "straight";
> +
> +	port {
> +		lcdc_0: endpoint@0 {

[Severity: Low]
Does this node need a matching reg property? The node uses a unit address
(endpoint@0) but lacks a corresponding reg = <0>; property, which can
trigger devicetree compiler unit_address_vs_reg warnings.

> +			remote-endpoint = <&it66122_in>;
> +		};
> +	};
> +};
> +
> +&i2c0 {
> +	it66122: bridge-hdmi@4c {
> +		compatible = "ite,it66122";
> +		reg = <0x4c>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&hdmi_gpio_pins_default>;
> +		vcn33-supply = <&vmmcsd_fixed>;
> +		vcn18-supply = <&it66122_vcn18_fixed>;
> +		vrf12-supply = <&it66122_vcn12_fixed>;
> +		reset-gpios = <&gpio1 9 GPIO_ACTIVE_LOW>;
> +		interrupt-parent = <&gpio1>;
> +		interrupts = <25 IRQ_TYPE_EDGE_FALLING>;

[Severity: Medium]
Should this interrupt trigger type be level-low instead of edge-falling?

I2C bridge chips like the IT66122 use active-low, open-drain interrupts.
If configured as edge-falling, a second interrupt condition occurring while
the line is already low won't generate a new falling edge. Because the kernel
uses a threaded interrupt handler for this device, this can race with the
clearing of the first event and lead to missed interrupts.

> +		#sound-dai-cells = <0>;
> +
> +		ports {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			port@0 {
> +				reg = <0>;
> +
> +				it66122_in: endpoint {
> +					bus-width = <24>;

[Severity: High]
Does this bus-width configuration mismatch the physical hardware?

The BeagleBone Black routes only 16 LCD data pins (LCD_DATA0 to LCD_DATA15)
for the display, as pins 16-23 are reserved for eMMC. This can be seen in
the pinmux setup in nxp_hdmi_bonelt_pins.

However, configuring the IT66122 endpoint for a 24-bit bus causes the
bridge driver to request a 24-bit input format. The bridge will then read
floating or eMMC data on its upper 8 bits, which leads to severely corrupted
video output on HDMI.

> +					remote-endpoint = <&lcdc_0>;
> +				};
> +			};

[ ... ]

> diff --git a/arch/arm/boot/dts/ti/omap/am335x-boneblack-revd.dts b/arch/arm/boot/dts/ti/omap/am335x-boneblack-revd.dts
> new file mode 100644
> index 0000000000000..fe9536d5f55ea
> --- /dev/null
> +++ b/arch/arm/boot/dts/ti/omap/am335x-boneblack-revd.dts
> @@ -0,0 +1,166 @@

[ ... ]

> +/ {
> +	model = "TI AM335x BeagleBone Black Rev D";
> +	compatible = "beagle,am335x-bone-black-revd", "ti,am33xx";

[Severity: Medium]
Are the standard BeagleBone fallback identifiers intentionally omitted?

This compatible string omits standard fallbacks like "ti,am335x-bone-black"
and "ti,am335x-bone" used by all other variants. Without them, existing
userspace tools, cape managers, and distributions will fail to detect the
board and apply BeagleBone-specific quirks.

[Severity: Low]
Also, has the new board compatible string "beagle,am335x-bone-black-revd"
been documented in the Devicetree bindings? Omitting it from the bindings
will produce validation warnings during devicetree checks.

> +};
> +
> +&gpio0 {

[ ... ]

> +&gpio1 {
> +	gpio-line-names =
> +		"P8_25 [mmc1_dat0]",
> +		"[mmc1_dat1]",
> +		"P8_5 [mmc1_dat2]",
> +		"P8_6 [mmc1_dat3]",
> +		"P8_23 [mmc1_dat4]",
> +		"P8_22 [mmc1_dat5]",
> +		"P8_3 [mmc1_dat6]",
> +		"P8_4 [mmc1_dat7]",
> +		"NC",
> +		"NC",

[Severity: Low]
Is this "NC" label accurate for GPIO1_9?

The am335x-boneblack-ite-hdmi.dtsi file routes the IT66122 bridge's
active-low reset to GPIO1_9. Retaining the old "NC" (Not Connected)
label masks the HDMI reset pin from userspace tools like libgpiod.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260901-v7-3-rc1-beaglebone-black-revd-v1-0-f521950b777a@gmail.com?part=2