mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Andrew Lunn <andrew@lunn.ch>
To: Marek Pazdan <mpazdan@arista.com>
Cc: aleksander.lobakin@intel.com, almasrymina@google.com,
	andrew+netdev@lunn.ch, anthony.l.nguyen@intel.com,
	daniel.zahka@gmail.com, davem@davemloft.net,
	ecree.xilinx@gmail.com, edumazet@google.com, gal@nvidia.com,
	horms@kernel.org, intel-wired-lan@lists.osuosl.org,
	jianbol@nvidia.com, kory.maincent@bootlin.com, kuba@kernel.org,
	linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
	pabeni@redhat.com, przemyslaw.kitszel@intel.com,
	willemb@google.com
Subject: Re: [Intel-wired-lan] [PATCH 1/2] ethtool: transceiver reset and presence pin control
Date: Tue, 8 Apr 2025 18:10:31 +0200	[thread overview]
Message-ID: <6f127b5b-77c6-4bd4-8124-8eea6a12ca61@lunn.ch> (raw)
In-Reply-To: <20250408153311.30539-1-mpazdan@arista.com>

On Tue, Apr 08, 2025 at 03:32:30PM +0000, Marek Pazdan wrote:
> On Mon, 7 Apr 2025 22:39:17 +0200 Andrew Lunn wrote:
> > How do you tell the kernel to stop managing the SFP? If you hit the
> > module with a reset from user space, the kernel is going to get
> > confused. And how are you talking to the module? Are you going to
> > hijack the i2c device via i2-dev? Again, you need to stop the kernel
> > from using the device.
> 
> This is something to implement in driver code. For ice driver this reset will
> be executed through AQ command (Admin Queue) which is communication channel
> between driver and firmware. What I probably need to do is to add additional PHY
> state (like USER_MODULE_RESET) and check it when driver wants to execute AQ command.
> 
> > Before you go any further, i think you need to zoom out and tell us
> > the big picture....
> 
> In my use case I need to have ability to reset transceiver module. There are 
> several reasons for that. Most common is to reinit module if case of error state.
> (this according to CMIS spec). Another use case is that in our switch's cli there
> is a command for transceiver reinitialisation which involves transceiver reset.

Now zoom out, ignore your hardware, look at the Linux abstraction for
an SFP, across all NICs and switches.

There are ethtool calls to retrieve the modules eeprom contents. There
is an ethtool call to program the modules firmware. There is an
ethtool call to get/set the power mode. Modules can also export their
sensors via HWMON, the temperature, receive power, etc.

How does your wish to reset the module fit into the general Linux
model of an SFP? Should it be allowed in the middle of a firmware
upgrade? Should the power mode be lost due to a reset? Can you still
read from the EEPROM etc while it is in reset? What should HWMON
report? Should it be a foot gun?

It does however seem to me, what you want should somehow be an ethtool
operation. And it probably needs to be plumbed through
net/ethtool/module.c, and you need to look at how it interacts with
all the other code in that file. And you maybe need to force the
netdev to do a down/up so that it gets the new state of the module, or
you only allow it on an netdev which is admin down?

Your patch needs to explain the big picture, how it fits into the
Linux abstraction of an SFP, and how other vendors should implement
the same operation, if they wish to implement it.

    Andrew

  reply	other threads:[~2025-04-08 16:10 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-04-07 12:35 Marek Pazdan
2025-04-07 12:35 ` [PATCH 2/2] ice: add qsfp " Marek Pazdan
2025-04-07 13:46   ` Kory Maincent
2025-04-07 20:30   ` Andrew Lunn
2025-04-08 14:22     ` [Intel-wired-lan] " Marek Pazdan
2025-04-08 15:23       ` Andrew Lunn
2025-04-07 13:32 ` [PATCH 1/2] ethtool: " Kory Maincent
2025-04-08 15:54   ` [Intel-wired-lan] " Marek Pazdan
2025-04-07 20:39 ` Andrew Lunn
2025-04-08 15:32   ` [Intel-wired-lan] " Marek Pazdan
2025-04-08 16:10     ` Andrew Lunn [this message]
2025-05-13 22:40       ` [PATCH net-next v2 1/2] ethtool: qsfp transceiver reset, interrupt " Marek Pazdan
2025-05-13 22:40         ` [Intel-wired-lan] [PATCH net-next v2 2/2] ice: add " Marek Pazdan
2025-05-15  1:23           ` Andrew Lunn
2025-05-14  0:26         ` [PATCH net-next v2 1/2] ethtool: " Jakub Kicinski
2025-05-15  1:10         ` Andrew Lunn

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=6f127b5b-77c6-4bd4-8124-8eea6a12ca61@lunn.ch \
    --to=andrew@lunn.ch \
    --cc=aleksander.lobakin@intel.com \
    --cc=almasrymina@google.com \
    --cc=andrew+netdev@lunn.ch \
    --cc=anthony.l.nguyen@intel.com \
    --cc=daniel.zahka@gmail.com \
    --cc=davem@davemloft.net \
    --cc=ecree.xilinx@gmail.com \
    --cc=edumazet@google.com \
    --cc=gal@nvidia.com \
    --cc=horms@kernel.org \
    --cc=intel-wired-lan@lists.osuosl.org \
    --cc=jianbol@nvidia.com \
    --cc=kory.maincent@bootlin.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mpazdan@arista.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=przemyslaw.kitszel@intel.com \
    --cc=willemb@google.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

all inboxes | Powered by JetHome®