RE: [PATCH v2] eeprom/optoe: driver to read/write SFP/QSFP/CMIS EEPROMS

quoted

I have offered, in every response, to collaborate with the simple
integration to use optoe as the default upstream driver to access the
module EEPROMs.  optoe would be superior to the existing default
routines in sfp.c

Actually, i'm not sure they would be. Since the KAPI issues are pretty

much a

NACK on their own, i didn't bother raising other issues. Both Russell King

and

I has issues with quirks and hotplug.

Our experience is that a number of SFPs are broken, they don't follow the
standard. Some you cannot perform more than 16 bytes reads without them
locking up. Others will perform a 16 byte read, but only give you one

useful

byte of data. So you have to read enough of the EEPROM a byte at a time to
get the vendor and product strings in order to determine what quirks need
to be applied. optoe has nothing like this. Either you don't care and only
support well behaved SFPs, or you have the quirk handling in user space,

in

the various vendor code blobs, repeated again and again. To make optoe
generically usable, you are going to have to push the quirk handling into
optoe. The brokenness should be hidden from userspace.

Interesting.  I would throw away such devices.  That's why switch vendors
publish supported parts lists.

Can you point me to the code that is handling those quirks?  Since I haven't
seen those problems, I don't know what they are and how to address them.

Note there are a VAST number of data items in those EEPROMs, including
proprietary capabilities.  Many of the items are configuration dependent,
and mean different things depending on the value of other data items.  Most
of these items are not of any interest to kernel networking.  I try to
minimize the size of the kernel footprint and move those decoding and
management functions to user space.

And then you repeat all the quirk handling sfp.c has. That does not scale,

we

don't want the same quirks in two different places. However, because SFPs
are hot pluggable, you need to re-evaluate the quirks whenever there is a
hot-plug event. optoe has no idea if there has been a hotplug event, since

it

does not have access to the GPIOs. Your user space vendor code might
know, it has access to the GPIOs. So maybe you could add an IOCTL call or
something, to let optoe know the module has changed and it needs to
update its quirks. Or for every user space read, you actually re-read the
vendor IDs and refresh the quirks before performing the read the user
actually wants. That all seems ugly and is missing from the current patch.

Actually I do need to know whether the device supports paging, that's the
only device state I need.  Since I don't detect hotplug events, I read the
'paging supported' bit on every read that changes the page register.  

There is a GPIO line to detect 'presence', which presumably could be
accessed via device tree configuration with the GPIO driver.  I haven't
figured out how to connect those pieces so I just read the page register on
every access.  Adding that would be a useful feature.

I fully agree with Jakub NACK.

  Andrew

Don