netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Russell King - ARM Linux <linux@armlinux.org.uk>
To: Andrew Lunn <andrew@lunn.ch>,
	Florian Fainelli <f.fainelli@gmail.com>,
	David Miller <davem@davemloft.net>
Cc: netdev@vger.kernel.org
Subject: Re: [RFC net-next] sfp/phylink: move module EEPROM ethtool access into netdev core ethtool
Date: Sun, 17 Dec 2017 17:06:02 +0000	[thread overview]
Message-ID: <20171217170602.GW10595@n2100.armlinux.org.uk> (raw)
In-Reply-To: <E1eQaEt-000329-8b@rmk-PC.armlinux.org.uk>

> Questions:
> 1. Is it worth adding a pointer to struct net_device for these two
>    methods, rather than having multiple duplicate veneers to vector
>    the ethtool module EEPROM ioctls through to the SFP bus layer?
> 
> 2. Should this allow network/phy drivers to override the default -
>    the code is currently structured to allow phy drivers to override
>    network drivers implementations, which seems the wrong way around.

I should also mention that there's another place that having the
sfp bus pointer in the network device comes in handy - the case
where we have a SFP module connected to a PHY rather than the MAC.

In this case, phylink itself is not used to link the SFP to the
netdev, and phylib doesn't provide the necessary hooks into the PHY
driver for the PHY driver to know when the network device comes up
or goes down.  SFP needs to know that to assert/deassert the TX
DISABLE signal to disable the module laser.

Having the net_device structure contain a pointer to the SFP bus
allows phylink or network drivers to directly inform SFP of the
state of the network device, without needing intermediaries to
forward the state.

It's possible that this may not be the best approach - the only setup
I'm aware of at present that has the "mac <-> phy <-> sfp" setup is
the Macchiatobin, but if other phys are involved, it may be better
if instead of having PHY drivers having to add support for SFP, we
instead do it in phylib.  The counter argument to that is that SFP
likely needs more in-depth knowledge of the PHY than a the generic
phylib parts could know about.

The patches as they currently stand are in my "phy" branch, browsable
via:

 http://git.armlinux.org.uk/cgit/linux-arm.git/log/?h=phy

specifically:

 sfp: use netdev sfp_bus for start/stop
 net: phy: Add SFP support to Marvell 10G PHY driver

The last patch is does not (yet) take into account the RX_LOS signal
when determining the link state, which it ought to to avoid false
link assertions as can happen when there's noise pickup by the
detector.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

  reply	other threads:[~2017-12-17 17:06 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-17 14:48 [RFC net-next] sfp/phylink: move module EEPROM ethtool access into netdev core ethtool Russell King
2017-12-17 17:06 ` Russell King - ARM Linux [this message]
2017-12-17 18:29 ` Andrew Lunn
2017-12-17 19:26   ` Russell King - ARM Linux
2018-03-19 19:20 ` Florian Fainelli
2018-03-19 20:39   ` 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=20171217170602.GW10595@n2100.armlinux.org.uk \
    --to=linux@armlinux.org.uk \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=f.fainelli@gmail.com \
    --cc=netdev@vger.kernel.org \
    /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 NNTP newsgroup(s).