Claude review: ARM: dts: am335x: Add Seeed Studio BeagleBone HDMI cape overlay

[Claude Code Review Bot <claude-review@example.com>]

Patch Review

> +am335x-bonegreen-hdmi-00a0-dtbs := am335x-bonegreen-eco.dtb \
> +	am335x-bone-hdmi-00a0.dtbo

The composite DTB uses `am335x-bonegreen-eco.dtb` as the base. The cover letter says the cape is "compatible with BeagleBone and BeagleBone Black due to pin compatibility," but only one composite DTB is produced for the Green Eco variant. This seems intentional (the primary target board), but it might be worth noting that users of other boards would need to apply the overlay manually at runtime.

> +	it66121: it66121 {
> +		compatible = "ite,it66121";
> +		reg = <0x4d>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&bb_lcd_pins>;

The LCD pin muxing is assigned via `pinctrl-0` on the IT66121 I2C device node rather than on the `&lcdc` node. This is somewhat unusual -- typically you would expect the display controller to own its output pin configuration. However, since this is an overlay, placing it on the bridge device that "triggers" the need for these pins is a pragmatic approach, and the pinctrl framework will configure the pins when the IT66121 device probes. It works, though it could be argued the pins are more logically associated with the LCDC.

> +	sound {
> +		compatible = "simple-audio-card";
> +		simple-audio-card,name = "TI BeagleBone Green HDMI cape";

Minor nit: the product is "Seeed Studio BeagleBone Green HDMI cape" according to the commit message and Link, but the audio card name says "TI BeagleBone Green HDMI cape." This is cosmetic but could be confusing -- it mixes the board vendor (TI/BeagleBone) with the cape vendor (Seeed Studio).

> +&lcdc {
> +	status = "okay";
> +	blue-and-red-wiring = "straight";
> +	port {
> +		lcdc_0: endpoint@0 {
> +			remote-endpoint = <&it66121_in>;
> +		};
> +	};
> +};

The `lcdc` node in `am33xx-l4.dtsi` has `status = "disabled"` by default, so enabling it here is correct. The `blue-and-red-wiring = "straight"` property is a tilcdc-specific property indicating no R/B swap.

> +&mcasp0 {
> +	status = "okay";
> +	#sound-dai-cells = <0>;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&mcasp0_pins>;
> +	op-mode = <0>;
> +	tdm-slots = <2>;
> +	serial-dir = < 0 0 1 0 >;
> +	tx-num-evt = <32>;
> +	rx-num-evt = <32>;
> +};

The McASP configuration looks standard for I2S stereo output (tdm-slots=2, one serializer transmitting at index 2). No issues here.

> +		enable-gpios = <&gpio1 27 0>;

The `0` for the GPIO flags means `GPIO_ACTIVE_HIGH`. This could be written as `GPIO_ACTIVE_HIGH` for readability by including `<dt-bindings/gpio/gpio.h>`, but using the raw value `0` is not incorrect.

Overall this is a clean overlay. No correctness bugs found.

---
Generated by Claude Code Patch Reviewer