Re: [PATCH 1/1] arm64: dts: Add usbphynop and usbotg pinctrl for S32G platforms

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The `&usbotg` node uses the undocumented `fsl,usbphy` property, which will cause `dtbs_check` validation to fail.
- [Low] Device tree node names use underscores instead of hyphens and do not follow generic naming conventions.
--

commit 009a8765a0c4d65ac0ce298f231d2203b966088a
Author: Khristine Andreea Barbulescu <khristineandreea.barbulescu@oss.nxp.com>
arm64: dts: Add usbphynop and usbotg pinctrl for S32G platforms

This commit adds a usb-nop-xceiv node and pinctrl configurations for the
usbotg interface on S32G2 and S32G3 platforms. It enables the USB controller
to reference the generic PHY and configures pin multiplexing.

> diff --git a/arch/arm64/boot/dts/freescale/s32gxxxa-evb.dtsi b/arch/arm64/boot/dts/freescale/s32gxxxa-evb.dtsi
> --- a/arch/arm64/boot/dts/freescale/s32gxxxa-evb.dtsi
> +++ b/arch/arm64/boot/dts/freescale/s32gxxxa-evb.dtsi

[ ... ]

> +/ {
> +	usbphynop: usbphynop {

This isn't a bug, but does the usbphynop node name follow the devicetree
specification's generic names recommendation? Would a standard class name like
phy or usb-phy be more appropriate?

> +		compatible = "usb-nop-xceiv";
> +		#phy-cells = <0>;
> +	};
> +};
> +

[ ... ]

> +
> +	usbotg_pins: usbotg_pins {
> +		usbotg_grp0 {

Are these underscores in the node names intentional? 

The devicetree specification mandates using hyphens instead of underscores
for node names, and compiling with dtc warnings enabled will produce warnings
here. Should these be named usbotg-pins and usbotg-grp0, along with the
other usbotg_grp nodes?

> +			pinmux = <0x3802>, <0x3812>,
> +				<0x3822>, <0x3832>,

[ ... ]

> +&usbotg {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&usbotg_pins>;
> +	fsl,usbphy = <&usbphynop>;

Will this fsl,usbphy property cause dtbs_check validation to fail?

The S32G platforms use the nxp,s32g2-usb compatible string, which is validated
by Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml. That schema
specifies unevaluatedProperties: false and does not define the legacy
fsl,usbphy property.

Would using the standard phys property (phys = <&usbphynop>;) work instead,
as it is automatically parsed as a fallback by the ci_hdrc_imx driver?

> +	status = "okay";
> +};

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260518061522.2884441-2-khristineandreea.barbulescu@oss.nxp.com?part=1