On 16.04.2024 00:03, Andrew Lunn wrote:
> On Fri, Apr 12, 2024 at 03:21:24PM +0200, Wojciech Drewek wrote:
>>
>>
>> On 09.04.2024 15:39, Andrew Lunn wrote:
>>>> This is something my current design supports I think. Using
>>>> ETHTOOL_A_MODULE_MAX_POWER_SET user can get what cage supports
>>>> and change it.
>>>
>>>> This could be done using ethtool_module_power_mode_policy I think.
>>>
>>> All these 'I think' don't give me a warm fuzzy feeling this is a well
>>> thought out and designed uAPI.
>>>
>>> I assume you have ethtool patches for your new netlink attributes. So
>>> show us the real usage. Start with an SFP in its default lower power
>>> mode. Show us the commands to display the current status. Allocate it
>>> more power, tell the module it can use more power, and then show us
>>> the status after the change has been made.
>>
>> Ok, but do we really need an API to switch the module between high/low power
>> mode?
>
> Probably not. But you need to document that the API you are adding is
> also expected to talk to the module and tell it to use more/less
> power.
>
>> Regarding the current status and what module supports, there is -m option:
>> $ ethtool -m ens801f0np0
>> Identifier : 0x0d (QSFP+)
>> Extended identifier : 0x00
>> Extended identifier description : 1.5W max. Power
>> consumption
>> Extended identifier description : No CDR in TX, No CDR in
>> RX
>> Extended identifier description : High Power Class (> 3.5
>> W) not enabled
>
> So you can make this part of your commit message. Show this. Invoke
> your new ethtool option, then show this again with the module
> reporting a higher power consumption. The reduce the power using
> ethtool and show the power consumption has reduced.
>
> Also, in the ethtool-netlink.rst file, clearly document what the API
> is doing, so that somebody else can implement it for another device.
>
> Please also document hotplug behaviour. Say I use your new API to
> increase the power to 3.5W. I then eject the module. Does the
> available power automatically get put back into the pool? When i
> reinsert the module, it will be in low power class, and i need to
> issue the ethtool command again to increase its power?
Ok, I'll answer all of those questions in the documentation included
in the 2nd version of the patchset.
>
> Andrew
>
