Re: [PATCH v4 1/2] dt-bindings: phy: Document MT8196 MediaTek PCI-Express Gen4 S-PHY

[AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>]

On 9/15/26 12:18, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] dt-bindings: strict array prevents 1-lane S-PHY nvmem configuration
> --
> 
> commit c6841c20d82c1db99761e33c189c44960b8b5284
> Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> 
> dt-bindings: phy: Document MT8196 MediaTek PCI-Express Gen4 S-PHY
> 
> This commit adds device tree bindings for the MediaTek PCI-Express Gen4 S-PHY
> found in newer SoCs like the MT8196. The PHY supports link rates up to 16.0
> GT/s per lane, up to two lanes, and both Root Complex and End Point modes.
> 
>> diff --git a/Documentation/devicetree/bindings/phy/mediatek,mt8196-pcie-sphy.yaml b/Documentation/devicetree/bindings/phy/mediatek,mt8196-pcie-sphy.yaml
>> new file mode 100644
>> index 0000000000000..3fd1bfbc3822f
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/phy/mediatek,mt8196-pcie-sphy.yaml
> [ ... ]
>> +  nvmem-cell-names:
>> +    items:
>> +      - const: int-r
> [ ... ]
>> +      - const: tx-lanes-pmos
>> +      - const: tx-lanes-nmos
>> +      - const: tx-ln0-rswn
>> +      - const: tx-ln1-rswn
>> +      - const: cal-version
> 
> [Severity: Medium]
> Does this rigid array structure prevent valid 1-lane S-PHY configurations from
> passing schema validation?
> 
> Defining a device tree node for a 1-lane PHY typically involves omitting the
> unused "tx-ln1-rswn" eFuse cell. However, because the items array enforces
> strict positional constraints, omitting that entry shifts the index of the
> global "cal-version" cell. This will cause dt_binding_check to fail for
> legitimate 1-lane configurations.
> 
> Could the schema be adjusted to allow the second lane's eFuse cell to be
> optional without breaking the expected position of "cal-version"? Otherwise,
> it seems DT authors are forced to either drop calibration entirely or include
> irrelevant dummy eFuse references.

In the single lane case, one lane is usually used for USB3, but the calibrations
are specific to the interface, so the second lane calibration is always present
even if only one is used.

Besides, this is done on purpose to enforce having all calibration handles in the
SoC DTSI file, because the second lane being used for this or that is something
board specific - so this avoids the (too usual) mistake of enabling two-lane PCIe
on a board while only one lane has calibration (which means none get calibrated).

So yes, it could be done, but it wasn't done on purpose.

In any case, should the need to declare only one lane calibration, it's still
something that can be done later with an if block.

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy