Re: [PATCH 1/2] arm64: dts: imx8mp-evk: enable CBTU02043 for USB Type-C orientation switch

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The GPIO hog for the CBTU02043 XSD pin incorrectly drives the physical pin HIGH instead of LOW, placing the switch in shutdown mode.
- [High] The `pinctrl-names` and `pinctrl-0` properties inside the `gpio-hog` child node will be silently ignored by the `pinctrl` core.
--

--- Patch [1]: [PATCH 1/2] arm64: dts: imx8mp-evk: enable CBTU02043 for USB Type-C orientation switch ---
commit b6ef05965c4d69ccad672c2aa85487746c77918d
Author: Peng Fan <[email protected]>

arm64: dts: imx8mp-evk: enable CBTU02043 for USB Type-C orientation switch

This commit adds a gpio-hog to keep the CBTU02043 crossbar switch always
enabled by driving its XSD pin low. It also adds a pinctrl group for the
XSD pin to ensure it is properly multiplexed.

> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
> index 7bdd641292fb6..dc3baeac5e3d3 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
> +++ b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
> @@ -935,6 +935,17 @@ usb3_data_hs: endpoint {
>  	};
>  };
>  
> +&gpio2 {
> +	cbtu02043-xsd-hog {
> +		gpio-hog;
> +		gpios = <20 GPIO_ACTIVE_LOW>;
> +		output-low;

[Severity: High]
Will this actually drive the physical pin HIGH instead of LOW?

In the gpiolib framework, "output-low" requests a logical 0. Because the
pin is specified as GPIO_ACTIVE_LOW, gpiod_direction_output_nonotify()
applies an inversion to the logical 0, resulting in the pad being driven
to a physical 1 (HIGH level).

Since the commit message notes that XSD must be driven LOW for normal
operation, should this use "output-high" with GPIO_ACTIVE_LOW, or
"output-low" with GPIO_ACTIVE_HIGH?

> +		line-name = "CBTU02043_XSD";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_cbtu02043_xsd>;

[Severity: High]
Will these pinctrl properties be silently ignored by the pinctrl core?

The pinctrl bindings are evaluated by the driver core via
pinctrl_bind_pins() only for nodes that instantiate an associated
struct device. A gpio-hog node is parsed manually by gpiolib as a firmware
subnode and does not instantiate a device.

Does this mean the MX8MP_IOMUXC_SD2_WP__GPIO2_IO20 pin will not be
multiplexed to its GPIO function, preventing the hog from controlling
the actual physical line? Should the pinctrl properties be placed in a
probed device node, like the parent &gpio2 or &iomuxc?

> +	};
> +};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1