Re: [PATCH v2 phy 12/16] dt-bindings: phy: lynx-28g: add compatible strings per SerDes and instantiation

Josua Mayer
2025-09-24T15:56:49Z

Am 24.09.25 um 17:45 schrieb Vladimir Oltean:
> Hi Rob,
>
> On Wed, Sep 24, 2025 at 08:54:29AM -0500, Rob Herring wrote:
>>> +description: |
>> Don't need '|' if no formatting to preserve.
> Thanks, will drop.
>
>>> + "#address-cells":
>>> + const: 1
>>> + description: "Address cells for child lane nodes"
>> You don't need generic descriptions of common properties.
> Ok, I'll also drop the description from #size-cells but keep it in
> #phy-cells (less obvious).
>
>>> +
>>> + "#size-cells":
>>> + const: 0
>>> + description: "Size cells for child lane nodes"
>>> +
>>> "#phy-cells":
>>> + description: "Number of cells in PHY specifier (legacy binding only)"
>>> const: 1
>>>
>>> @@ -32,9 +124,51 @@ examples:
>>> soc {
>>> #address-cells = <2>;
>>> #size-cells = <2>;
>>> - serdes_1: phy@1ea0000 {
>>> - compatible = "fsl,lynx-28g";
>>> +
>>> + serdes_1: serdes@1ea0000 {
>>> + compatible = "fsl,lx2160a-serdes1";
>>> reg = <0x0 0x1ea0000 0x0 0x1e30>;
>>> - #phy-cells = <1>;
>>> + #address-cells = <1>;
>>> + #size-cells = <0>;
>>> +
>>> + phy@0 {
>>> + reg = <0>;
>>> + #phy-cells = <0>;
>>> + };
>> There's really no difference between having child nodes 0-7 and 8 phy
>> providers vs. putting 0-7 into a phy cell arg and 1 phy provider.
>>
>> The only difference I see is it is more straight-forward to determine
>> what lanes are present in the phy driver if the driver needs to know
>> that. But you can also just read all 'phys' properties in the DT with a
>> &serdes_1 phandle and determine that. Is that efficient? No, but you
>> have to do that exactly once and probably has no measurable impact.
>>
>> With that, then can't you simply just add a more specific compatible:
>>
>> compatible = "fsl,lx2160a-serdes1", "fsl,lynx-28g";
>>
>> Then you maintain some compatibility.
>>
>> Rob
> With the patches that have been presented to you thus far -- yes, this
> is the correct conclusion, there is not much of a difference. But this
> is not all.
>
> If I want in the future to apply the properties from
> Documentation/devicetree/bindings/phy/transmit-amplitude.yaml to just
> one of the lanes, how would I do that with just 1 phy provider?
I believe it is possible for a driver to create multiple phy objects
during probe, and for the xlate function to return the correct one.

Then, whether you follow a phandle to the parent with 1 argument,
or a  phandle to the phy child with 0 arguments provides same results.

The driver already creates a phy object for each lane with:

phy = devm_phy_create(&pdev->dev, NULL, &lynx_28g_ops);

Once the second argument is changed to a valid lane node,
it's properties will be accessible.

I prototyped this a while ago:
https://github.com/SolidRun/lx2160a_build/blob/develop-ls-5.15.71-2.2.0/patches/linux/0030-phy-lynx-28g-add-support-for-device-tree-per-lane-ph.patch

> It's not
> so clear. Compared to 8 phy providers, each with its OF node => much
> easier to structure and to understand.
>
> This is essentially what the discussion with Josua from v1 boils down to.