From: "Pali Rohár" <pali@kernel.org>
To: Don Bollinger <don@thebollingers.org>
Cc: Andrew Lunn <andrew@lunn.ch>, 'Jakub Kicinski' <kuba@kernel.org>,
arndb@arndb.de, gregkh@linuxfoundation.org,
linux-kernel@vger.kernel.org, brandon_chuang@edge-core.com,
wally_wang@accton.com, aken_liu@edge-core.com,
gulv@microsoft.com, jolevequ@microsoft.com,
xinxliu@microsoft.com, 'netdev' <netdev@vger.kernel.org>,
'Moshe Shemesh' <moshe@nvidia.com>
Subject: Re: [PATCH v2] eeprom/optoe: driver to read/write SFP/QSFP/CMIS EEPROMS
Date: Sat, 20 Mar 2021 17:10:21 +0100 [thread overview]
Message-ID: <20210320161021.fngdgxvherg4v3lr@pali> (raw)
In-Reply-To: <YFJHN+raumcJ5/7M@lunn.ch>
Hello Don!
I have read whole discussion and your EEPROM patch proposal. But for me
it looks like some kernel glue code for some old legacy / proprietary
access method which does not have any usage outside of that old code.
Your code does not contain any quirks which are needed to read different
EEPROMs in different SFP modules. As Andrew wrote there are lot of
broken SFPs which needs special handling and this logic is already
implemented in sfp.c and sfp-bus.c kernel drivers. These drivers then
export EEPROM content to userspace via ethtool -m API in unified way and
userspace does not implement any quirks (nor does not have to deal with
quirks).
If you try to read EEPROM "incorrectly" then SFP module with its EEPROM
chip (or emulation of chip) locks and is fully unusable after you unplug
it and plug it again. Kernel really should not export API to userspace
which can cause "damage" to SFP modules. And currently it does *not* do
it.
I have contributed code for some GPON SFP modules, so their EEPROM can
be correctly read and exported to userspace via ethtool -m. So I know
that this is very fragile area and needs to be properly handled.
So I do not see any reason why can be a new optoe method in _current_
form useful. It does not implemented required things for handling
different EEPROM modules.
I would rather suggest you to use ethtool -m IOCTL API and in case it is
not suitable for QSFP (e.g. because of paging system) then extend this
API.
There were already proposals for using netlink socket interface which is
today de-facto standard interface for new code. sysfs API for such thing
really looks like some legacy code and nowadays we have better access
methods.
If you want, I can help you with these steps and make patches to be in
acceptable state; not written in "legacy" style. As I'm working with
GPON SFP modules with different EEPROMs in them, I'm really interested
in any improvements in their EEPROM area.
next prev parent reply other threads:[~2021-03-20 16:11 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20210215193821.3345-1-don@thebollingers.org>
2021-02-26 22:34 ` [PATCH v2] eeprom/optoe: driver to read/write SFP/QSFP/CMIS EEPROMS Andrew Lunn
2021-02-27 2:46 ` Don Bollinger
2021-02-27 16:23 ` Andrew Lunn
2021-03-01 20:00 ` Don Bollinger
2021-03-01 20:45 ` Andrew Lunn
2021-03-05 19:07 ` Don Bollinger
2021-03-05 22:55 ` Jakub Kicinski
2021-03-06 2:30 ` Don Bollinger
2021-03-06 3:31 ` Andrew Lunn
2021-03-12 19:04 ` Don Bollinger
2021-03-12 19:59 ` Andrew Lunn
2021-03-13 21:35 ` Don Bollinger
2021-03-15 17:39 ` Jakub Kicinski
2021-03-15 18:09 ` Don Bollinger
2021-03-17 18:15 ` Andrew Lunn
2021-03-20 16:10 ` Pali Rohár [this message]
2021-03-26 18:43 ` Don Bollinger
2021-03-29 22:13 ` Pali Rohár
2021-03-23 20:32 ` Don Bollinger
2021-03-23 22:29 ` Andrew Lunn
2021-03-26 18:43 ` Don Bollinger
2021-03-26 19:46 ` Andrew Lunn
2021-03-26 20:16 ` Don Bollinger
2021-03-26 20:37 ` Andrew Lunn
2021-03-26 21:09 ` Don Bollinger
2021-03-26 21:54 ` Andrew Lunn
2021-03-26 22:30 ` Don Bollinger
2021-03-27 15:25 ` Andrew Lunn
2021-03-27 21:20 ` Don Bollinger
2021-03-27 12:40 ` Greg KH
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=20210320161021.fngdgxvherg4v3lr@pali \
--to=pali@kernel.org \
--cc=aken_liu@edge-core.com \
--cc=andrew@lunn.ch \
--cc=arndb@arndb.de \
--cc=brandon_chuang@edge-core.com \
--cc=don@thebollingers.org \
--cc=gregkh@linuxfoundation.org \
--cc=gulv@microsoft.com \
--cc=jolevequ@microsoft.com \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=moshe@nvidia.com \
--cc=netdev@vger.kernel.org \
--cc=wally_wang@accton.com \
--cc=xinxliu@microsoft.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