Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Sat, 20 Mar 2021 17:10:21 +0100
From:   Pali =?utf-8?B?Um9ow6Fy?= <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
Message-ID: <20210320161021.fngdgxvherg4v3lr@pali>
References: <003901d711f2$be2f55d0$3a8e0170$@thebollingers.org>
 <20210305145518.57a765bc@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
 <005e01d71230$ad203be0$0760b3a0$@thebollingers.org>
 <YEL3ksdKIW7cVRh5@lunn.ch>
 <018701d71772$7b0ba3f0$7122ebd0$@thebollingers.org>
 <YEvILa9FK8qQs5QK@lunn.ch>
 <01ae01d71850$db4f5a20$91ee0e60$@thebollingers.org>
 <20210315103950.65fedf2c@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
 <001201d719c6$6ac826c0$40587440$@thebollingers.org>
 <YFJHN+raumcJ5/7M@lunn.ch>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <YFJHN+raumcJ5/7M@lunn.ch>
User-Agent: NeoMutt/20180716
Precedence: bulk

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.