Re: [PATCH v2 1/3] arm64: dts: ti: Add PLL1 refclk to J784S4 SoC SERDES node

[Gokul Praveen <g-praveen@ti.com>]

Hi ,

On 29/06/26 16:01, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] Device Tree Binding Schema validation failure due to missing schema item for the third clock name.
> --
>
> commit c724dfc4b95310a6b37f143a703b52391002ee90
> Author: Gokul Praveen <g-praveen@ti.com>
>
> arm64: dts: ti: Add PLL1 refclk to J784S4 SoC SERDES node
>
> This commit adds the PLL1 refclk to the clocks and clock-names parameters
> of the SERDES2 node. It updates both the devicetree binding schema and the
> k3-j784s4-main.dtsi file to reflect the new clock.
>
>> diff --git a/Documentation/devicetree/bindings/phy/phy-cadence-torrent.yaml b/Documentation/devicetree/bindings/phy/phy-cadence-torrent.yaml
>> --- a/Documentation/devicetree/bindings/phy/phy-cadence-torrent.yaml
>> +++ b/Documentation/devicetree/bindings/phy/phy-cadence-torrent.yaml
> [ ... ]
>> @@ -47,7 +47,7 @@ properties:
>>       minItems: 1
>>       items:
>>         - const: refclk
>> -      - enum: [ pll1_refclk, phy_en_refclk ]
>> +      - enum: [refclk, pll1_refclk, phy_en_refclk ]
> [Severity: Low]
> Does this schema correctly allow a third clock name?
>
> The items list here only defines two elements (the const and the enum). Under
> dt-schema rules, any device tree node specifying a third element will fail
> validation because there is no corresponding schema item defined for it in the
> items array.
>
Yes, because the description of the "clocks" paremeter in the YAML 
mentions the following:

"PHY input reference clocks - refclk (for PLL0) & pll1_refclk (for PLL1).
       pll1_refclk is optional and used for multi-protocol 
configurations requiring
       separate reference clock for each protocol.
       Same refclk is used for both PLL0 and PLL1 if no separate 
pll1_refclk is used.
       Optional parent clock (phy_en_refclk) to enable a reference clock 
output feature
       on some platforms to output either derived or received reference 
clock.
"

which means that a maximum of 3 clocks can be used (refclk for PLL0, 
pll1_refclk for PLL1, phy_en_refclk for PHY_EN_REFCLK)

>> diff --git a/arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi b/arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi
>> --- a/arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi
>> +++ b/arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi
>> @@ -122,8 +122,9 @@ serdes2: serdes@5020000 {
>>   			resets = <&serdes_wiz2 0>;
>>   			reset-names = "torrent_reset";
>>   			clocks = <&serdes_wiz2 TI_WIZ_PLL0_REFCLK>,
>> +				 <&serdes_wiz2 TI_WIZ_PLL1_REFCLK>,
>>   				 <&serdes_wiz2 TI_WIZ_PHY_EN_REFCLK>;
>> -			clock-names = "refclk", "phy_en_refclk";
>> +			clock-names = "refclk","pll1_refclk", "phy_en_refclk";
> [Severity: Low]
> Will this trigger a dtbs_check warning?
>
> Since the device tree node is updated to supply 3 clock-names, but the
> YAML schema items list above is still implicitly limited to 2 definitions,
> this will likely cause a build-time validation failure.
>
> Should a third item be added to the YAML items array to match this 3-clock
> configuration?
The YAML has already been updated in this patch with a 3-clock 
configuration.
>