From: Wojciech Drewek <wojciech.drewek@intel.com>
To: Andrew Lunn <andrew@lunn.ch>
Cc: netdev@vger.kernel.org, edumazet@google.com,
marcin.szycik@linux.intel.com, anthony.l.nguyen@intel.com,
idosch@nvidia.com, kuba@kernel.org, pabeni@redhat.com,
przemyslaw.kitszel@intel.com, intel-wired-lan@lists.osuosl.org
Subject: Re: [Intel-wired-lan] [PATCH net-next 0/3] ethtool: Max power support
Date: Thu, 18 Apr 2024 13:48:25 +0200 [thread overview]
Message-ID: <88fc6454-01dc-4907-9b9b-702c9464c9ec@intel.com> (raw)
In-Reply-To: <6514e6a9-3b4d-48ba-b895-a12c5beff820@lunn.ch>
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
>
prev parent reply other threads:[~2024-04-18 11:48 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-29 9:23 [Intel-wired-lan] [PATCH net-next 0/3] ethtool: Max power support Wojciech Drewek
2024-03-29 9:23 ` [Intel-wired-lan] [PATCH net-next 1/3] ethtool: Make module API more generic Wojciech Drewek
2024-03-29 9:23 ` [Intel-wired-lan] [PATCH net-next 2/3] ethtool: Introduce max power support Wojciech Drewek
2024-03-29 22:29 ` Jakub Kicinski
2024-03-29 22:29 ` Jakub Kicinski
2024-04-02 11:25 ` Wojciech Drewek
2024-04-02 14:34 ` Jakub Kicinski
2024-04-03 10:19 ` Wojciech Drewek
2024-04-04 0:18 ` Jakub Kicinski
2024-04-04 12:19 ` Wojciech Drewek
2024-03-30 22:14 ` Andrew Lunn
2024-03-30 22:14 ` Andrew Lunn
2024-04-03 9:50 ` Wojciech Drewek
2024-03-29 9:23 ` [Intel-wired-lan] [PATCH net-next 3/3] ice: Implement ethtool max power configuration Wojciech Drewek
2024-03-29 22:16 ` [Intel-wired-lan] [PATCH net-next 0/3] ethtool: Max power support Jakub Kicinski
2024-04-02 9:58 ` Wojciech Drewek
2024-03-30 21:57 ` Andrew Lunn
2024-03-30 21:57 ` Andrew Lunn
2024-04-02 11:38 ` Wojciech Drewek
2024-04-02 14:25 ` Jakub Kicinski
2024-04-02 14:53 ` Andrew Lunn
2024-04-02 14:46 ` Andrew Lunn
2024-04-02 14:57 ` Jakub Kicinski
2024-04-03 13:18 ` Wojciech Drewek
2024-04-03 13:40 ` Andrew Lunn
2024-04-04 12:21 ` Wojciech Drewek
2024-04-03 13:49 ` Andrew Lunn
2024-04-04 12:45 ` Wojciech Drewek
2024-04-04 13:53 ` Andrew Lunn
2024-04-09 12:20 ` Wojciech Drewek
2024-04-09 13:39 ` Andrew Lunn
2024-04-12 13:21 ` Wojciech Drewek
2024-04-15 22:03 ` Andrew Lunn
2024-04-18 11:48 ` Wojciech Drewek [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=88fc6454-01dc-4907-9b9b-702c9464c9ec@intel.com \
--to=wojciech.drewek@intel.com \
--cc=andrew@lunn.ch \
--cc=anthony.l.nguyen@intel.com \
--cc=edumazet@google.com \
--cc=idosch@nvidia.com \
--cc=intel-wired-lan@lists.osuosl.org \
--cc=kuba@kernel.org \
--cc=marcin.szycik@linux.intel.com \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=przemyslaw.kitszel@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).