On Wed, Dec 30, 2020 at 04:47:52PM +0100, Pali Rohár wrote:
> Workaround for GPON SFP modules based on VSOL V2801F brand was added in
> commit 0d035bed2a4a ("net: sfp: VSOL V2801F / CarlitoxxPro CPGOS03-0490
> v2.0 workaround"). But it works only for ids explicitly added to the list.
> As there are more rebraded VSOL V2801F modules and OEM vendors are putting
> into vendor name random strings we cannot build workaround based on ids.
>
> Moreover issue which that commit tried to workaround is generic not only to
> VSOL based modules, but rather to all GPON modules based on Realtek RTL8672
> and RTL9601C chips.
>
> They can be found for example in following GPON modules:
> * V-SOL V2801F
> * C-Data FD511GX-RM0
> * OPTON GP801R
> * BAUDCOM BD-1234-SFM
> * CPGOS03-0490 v2.0
> * Ubiquiti U-Fiber Instant
> * EXOT EGS1
>
> Those Realtek chips have broken EEPROM emulator which for N-byte read
> operation returns just one byte of EEPROM data followed by N-1 zeros.
>
> So introduce a new function sfp_id_needs_byte_io() which detects SFP
> modules with these Realtek chips which have broken EEPROM emulator based on
> N-1 zeros and switch to 1 byte EEPROM reading operation which workaround
> this issue.
>
> This patch fixes reading EEPROM content from SFP modules based on Realtek
> RTL8672 and RTL9601C chips.
>
> Fixes: 0d035bed2a4a ("net: sfp: VSOL V2801F / CarlitoxxPro CPGOS03-0490 v2.0
> workaround")
> Co-developed-by: Russell King <rmk+ker...@armlinux.org.uk>
> Signed-off-by: Russell King <rmk+ker...@armlinux.org.uk>
> Signed-off-by: Pali Rohár <p...@kernel.org>
> ---
> drivers/net/phy/sfp.c | 78 ++++++++++++++++++++++++-------------------
> 1 file changed, 44 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
> index 91d74c1a920a..490e78a72dd6 100644
> --- a/drivers/net/phy/sfp.c
> +++ b/drivers/net/phy/sfp.c
> @@ -336,19 +336,11 @@ static int sfp_i2c_read(struct sfp *sfp, bool a2, u8
> dev_addr, void *buf,
> size_t len)
> {
> struct i2c_msg msgs[2];
> - size_t block_size;
> + u8 bus_addr = a2 ? 0x51 : 0x50;
> + size_t block_size = sfp->i2c_block_size;
> size_t this_len;
> - u8 bus_addr;
> int ret;
>
> - if (a2) {
> - block_size = 16;
> - bus_addr = 0x51;
> - } else {
> - block_size = sfp->i2c_block_size;
> - bus_addr = 0x50;
> - }
> -
NAK. You are undoing something that is definitely needed. The
diagnostics must be read with sequential reads to be able to properly
read the 16-bit values.
The rest of the patch is fine; it's a shame the entire thing has
been spoilt by this extra addition that was not in the patch we had
been discussing off-list.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
