Maxime Chevallier <maxime.chevallier@bootlin.com> writes:

>> diff --git a/Documentation/netlink/specs/ethtool.yaml b/Documentation/netlink/specs/ethtool.yaml
>> index 4707063af3b4..05ebad6ae4e0 100644
>> --- a/Documentation/netlink/specs/ethtool.yaml
>> +++ b/Documentation/netlink/specs/ethtool.yaml
>> @@ -211,6 +211,39 @@ definitions:
>>          name: discard
>>          value: 31
>> 
>> +  -
>> +    name: loopback-component
>> +    type: enum
>> +    doc: |
>> +      Loopback component. Identifies where in the network path the
>> +      loopback is applied.
>> +    entries:
>> +      -
>> +        name: mac
>> +        doc: MAC loopback
>> +      -
>> +        name: pcs
>> +        doc: PCS loopback
>> +      -
>> +        name: phy
>> +        doc: PHY loopback
>> +      -
>> +        name: module
>> +        doc: Pluggable module (e.g. CMIS (Q)SFP) loopback
>
> Should we also add "serdes" ?

Good question! I've asked Naveen in another reply...

>> @@ -1903,6 +1936,60 @@ attribute-sets:
>>          name: link
>>          type: nest
>>          nested-attributes: mse-snapshot
>> +  -
>> +    name: loopback-entry
>> +    doc: Per-component loopback configuration entry.
>> +    attr-cnt-name: __ethtool-a-loopback-entry-cnt
>> +    attributes:
>> +      -
>> +        name: unspec
>> +        type: unused
>> +        value: 0
>> +      -
>> +        name: component
>> +        type: u32
>> +        enum: loopback-component
>> +        doc: Loopback component
>> +      -
>> +        name: id
>> +        type: u32
>> +        doc: |
>> +          Optional component instance identifier. Required for PHY,
>> +          optional for MODULE, omitted for MAC and PCS.
>
> it doesn't have to be required for PHY. The current idea is that if you
> don't pass any PHY index when issueing a PHY-targetting command, then it
> means you're targetting net_device->phydev, that is the PHY device
> attached to the netdev (if any).
>
> I think we can keep that behaviour, as systems with multiple PHYs are
> not very common.

Got it! I'll update the wording! Thanks for the clarification!

>> +      -
>> +        name: name
>> +        type: string
>> +        doc: |
>> +          Subsystem-specific name for the loopback point within the
>> +          component.
>
> We'll need to be careful about keeping this subsystem-specific and not
> driver-specific :)
>
>> +      -
>> +        name: supported
>> +        type: u32
>> +        enum: loopback-direction
>> +        enum-as-flags: true
>> +        doc: Bitmask of supported loopback directions
>> +      -
>> +        name: direction
>> +        type: u32
>> +        enum: loopback-direction
>> +        doc: Current loopback direction, 0 means disabled
>
> no need for an u32 for 3 different values I think :)

ACK! ...or maybe uint (which will use as least 32, but shows that we
don't care)?


Björn