b.liu | e958203 | 2025-04-17 19:18:16 +0800 | [diff] [blame^] | 1 | From 426c6cbc409cbda9ab1a9dbf15d3c2ef947eb8c1 Mon Sep 17 00:00:00 2001 |
| 2 | From: =?UTF-8?q?Pali=20Roh=C3=A1r?= <pali@kernel.org> |
| 3 | Date: Mon, 25 Jan 2021 16:02:27 +0100 |
| 4 | Subject: [PATCH] net: sfp: add workaround for Realtek RTL8672 and RTL9601C |
| 5 | chips |
| 6 | MIME-Version: 1.0 |
| 7 | Content-Type: text/plain; charset=UTF-8 |
| 8 | Content-Transfer-Encoding: 8bit |
| 9 | |
| 10 | The workaround for VSOL V2801F brand based GPON SFP modules added in commit |
| 11 | 0d035bed2a4a ("net: sfp: VSOL V2801F / CarlitoxxPro CPGOS03-0490 v2.0 |
| 12 | workaround") works only for IDs added explicitly to the list. Since there |
| 13 | are rebranded modules where OEM vendors put different strings into the |
| 14 | vendor name field, we cannot base workaround on IDs only. |
| 15 | |
| 16 | Moreover the issue which the above mentioned commit tried to work around is |
| 17 | generic not only to VSOL based modules, but rather to all GPON modules |
| 18 | based on Realtek RTL8672 and RTL9601C chips. |
| 19 | |
| 20 | These include at least the following GPON modules: |
| 21 | * V-SOL V2801F |
| 22 | * C-Data FD511GX-RM0 |
| 23 | * OPTON GP801R |
| 24 | * BAUDCOM BD-1234-SFM |
| 25 | * CPGOS03-0490 v2.0 |
| 26 | * Ubiquiti U-Fiber Instant |
| 27 | * EXOT EGS1 |
| 28 | |
| 29 | These Realtek chips have broken EEPROM emulator which for N-byte read |
| 30 | operation returns just the first byte of EEPROM data, followed by N-1 |
| 31 | zeros. |
| 32 | |
| 33 | Introduce a new function, sfp_id_needs_byte_io(), which detects SFP modules |
| 34 | with broken EEPROM emulator based on N-1 zeros and switch to 1 byte EEPROM |
| 35 | reading operation. |
| 36 | |
| 37 | Function sfp_i2c_read() now always uses single byte reading when it is |
| 38 | required and when function sfp_hwmon_probe() detects single byte access, |
| 39 | it disables registration of hwmon device, because in this case we cannot |
| 40 | reliably and atomically read 2 bytes as is required by the standard for |
| 41 | retrieving values from diagnostic area. |
| 42 | |
| 43 | (These Realtek chips are broken in a way that violates SFP standards for |
| 44 | diagnostic interface. Kernel in this case simply cannot do anything less |
| 45 | of skipping registration of the hwmon interface.) |
| 46 | |
| 47 | This patch fixes reading of EEPROM content from SFP modules based on |
| 48 | Realtek RTL8672 and RTL9601C chips. Diagnostic interface of EEPROM stays |
| 49 | broken and cannot be fixed. |
| 50 | |
| 51 | Fixes: 0d035bed2a4a ("net: sfp: VSOL V2801F / CarlitoxxPro CPGOS03-0490 v2.0 workaround") |
| 52 | Co-developed-by: Russell King <rmk+kernel@armlinux.org.uk> |
| 53 | Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk> |
| 54 | Signed-off-by: Pali Rohรกr <pali@kernel.org> |
| 55 | Signed-off-by: Jakub Kicinski <kuba@kernel.org> |
| 56 | --- |
| 57 | drivers/net/phy/sfp.c | 100 ++++++++++++++++++++++++++++-------------- |
| 58 | 1 file changed, 67 insertions(+), 33 deletions(-) |
| 59 | |
| 60 | --- a/drivers/net/phy/sfp.c |
| 61 | +++ b/drivers/net/phy/sfp.c |
| 62 | @@ -306,19 +306,11 @@ static int sfp_i2c_read(struct sfp *sfp, |
| 63 | size_t len) |
| 64 | { |
| 65 | struct i2c_msg msgs[2]; |
| 66 | - size_t block_size; |
| 67 | + u8 bus_addr = a2 ? 0x51 : 0x50; |
| 68 | + size_t block_size = sfp->i2c_block_size; |
| 69 | size_t this_len; |
| 70 | - u8 bus_addr; |
| 71 | int ret; |
| 72 | |
| 73 | - if (a2) { |
| 74 | - block_size = 16; |
| 75 | - bus_addr = 0x51; |
| 76 | - } else { |
| 77 | - block_size = sfp->i2c_block_size; |
| 78 | - bus_addr = 0x50; |
| 79 | - } |
| 80 | - |
| 81 | msgs[0].addr = bus_addr; |
| 82 | msgs[0].flags = 0; |
| 83 | msgs[0].len = 1; |
| 84 | @@ -1245,6 +1237,20 @@ static void sfp_hwmon_probe(struct work_ |
| 85 | struct sfp *sfp = container_of(work, struct sfp, hwmon_probe.work); |
| 86 | int err, i; |
| 87 | |
| 88 | + /* hwmon interface needs to access 16bit registers in atomic way to |
| 89 | + * guarantee coherency of the diagnostic monitoring data. If it is not |
| 90 | + * possible to guarantee coherency because EEPROM is broken in such way |
| 91 | + * that does not support atomic 16bit read operation then we have to |
| 92 | + * skip registration of hwmon device. |
| 93 | + */ |
| 94 | + if (sfp->i2c_block_size < 2) { |
| 95 | + dev_info(sfp->dev, |
| 96 | + "skipping hwmon device registration due to broken EEPROM\n"); |
| 97 | + dev_info(sfp->dev, |
| 98 | + "diagnostic EEPROM area cannot be read atomically to guarantee data coherency\n"); |
| 99 | + return; |
| 100 | + } |
| 101 | + |
| 102 | err = sfp_read(sfp, true, 0, &sfp->diag, sizeof(sfp->diag)); |
| 103 | if (err < 0) { |
| 104 | if (sfp->hwmon_tries--) { |
| 105 | @@ -1587,26 +1593,30 @@ static int sfp_sm_mod_hpower(struct sfp |
| 106 | return 0; |
| 107 | } |
| 108 | |
| 109 | -/* Some modules (Nokia 3FE46541AA) lock up if byte 0x51 is read as a |
| 110 | - * single read. Switch back to reading 16 byte blocks unless we have |
| 111 | - * a CarlitoxxPro module (rebranded VSOL V2801F). Even more annoyingly, |
| 112 | - * some VSOL V2801F have the vendor name changed to OEM. |
| 113 | +/* GPON modules based on Realtek RTL8672 and RTL9601C chips (e.g. V-SOL |
| 114 | + * V2801F, CarlitoxxPro CPGOS03-0490, Ubiquiti U-Fiber Instant, ...) do |
| 115 | + * not support multibyte reads from the EEPROM. Each multi-byte read |
| 116 | + * operation returns just one byte of EEPROM followed by zeros. There is |
| 117 | + * no way to identify which modules are using Realtek RTL8672 and RTL9601C |
| 118 | + * chips. Moreover every OEM of V-SOL V2801F module puts its own vendor |
| 119 | + * name and vendor id into EEPROM, so there is even no way to detect if |
| 120 | + * module is V-SOL V2801F. Therefore check for those zeros in the read |
| 121 | + * data and then based on check switch to reading EEPROM to one byte |
| 122 | + * at a time. |
| 123 | */ |
| 124 | -static int sfp_quirk_i2c_block_size(const struct sfp_eeprom_base *base) |
| 125 | +static bool sfp_id_needs_byte_io(struct sfp *sfp, void *buf, size_t len) |
| 126 | { |
| 127 | - if (!memcmp(base->vendor_name, "VSOL ", 16)) |
| 128 | - return 1; |
| 129 | - if (!memcmp(base->vendor_name, "OEM ", 16) && |
| 130 | - !memcmp(base->vendor_pn, "V2801F ", 16)) |
| 131 | - return 1; |
| 132 | + size_t i, block_size = sfp->i2c_block_size; |
| 133 | |
| 134 | - /* Some modules can't cope with long reads */ |
| 135 | - return 16; |
| 136 | -} |
| 137 | + /* Already using byte IO */ |
| 138 | + if (block_size == 1) |
| 139 | + return false; |
| 140 | |
| 141 | -static void sfp_quirks_base(struct sfp *sfp, const struct sfp_eeprom_base *base) |
| 142 | -{ |
| 143 | - sfp->i2c_block_size = sfp_quirk_i2c_block_size(base); |
| 144 | + for (i = 1; i < len; i += block_size) { |
| 145 | + if (memchr_inv(buf + i, '\0', min(block_size - 1, len - i))) |
| 146 | + return false; |
| 147 | + } |
| 148 | + return true; |
| 149 | } |
| 150 | |
| 151 | static int sfp_sm_mod_probe(struct sfp *sfp, bool report) |
| 152 | @@ -1617,11 +1627,11 @@ static int sfp_sm_mod_probe(struct sfp * |
| 153 | u8 check; |
| 154 | int ret; |
| 155 | |
| 156 | - /* Some modules (CarlitoxxPro CPGOS03-0490) do not support multibyte |
| 157 | - * reads from the EEPROM, so start by reading the base identifying |
| 158 | - * information one byte at a time. |
| 159 | + /* Some SFP modules and also some Linux I2C drivers do not like reads |
| 160 | + * longer than 16 bytes, so read the EEPROM in chunks of 16 bytes at |
| 161 | + * a time. |
| 162 | */ |
| 163 | - sfp->i2c_block_size = 1; |
| 164 | + sfp->i2c_block_size = 16; |
| 165 | |
| 166 | ret = sfp_read(sfp, false, 0, &id.base, sizeof(id.base)); |
| 167 | if (ret < 0) { |
| 168 | @@ -1635,6 +1645,33 @@ static int sfp_sm_mod_probe(struct sfp * |
| 169 | return -EAGAIN; |
| 170 | } |
| 171 | |
| 172 | + /* Some SFP modules (e.g. Nokia 3FE46541AA) lock up if read from |
| 173 | + * address 0x51 is just one byte at a time. Also SFF-8472 requires |
| 174 | + * that EEPROM supports atomic 16bit read operation for diagnostic |
| 175 | + * fields, so do not switch to one byte reading at a time unless it |
| 176 | + * is really required and we have no other option. |
| 177 | + */ |
| 178 | + if (sfp_id_needs_byte_io(sfp, &id.base, sizeof(id.base))) { |
| 179 | + dev_info(sfp->dev, |
| 180 | + "Detected broken RTL8672/RTL9601C emulated EEPROM\n"); |
| 181 | + dev_info(sfp->dev, |
| 182 | + "Switching to reading EEPROM to one byte at a time\n"); |
| 183 | + sfp->i2c_block_size = 1; |
| 184 | + |
| 185 | + ret = sfp_read(sfp, false, 0, &id.base, sizeof(id.base)); |
| 186 | + if (ret < 0) { |
| 187 | + if (report) |
| 188 | + dev_err(sfp->dev, "failed to read EEPROM: %d\n", |
| 189 | + ret); |
| 190 | + return -EAGAIN; |
| 191 | + } |
| 192 | + |
| 193 | + if (ret != sizeof(id.base)) { |
| 194 | + dev_err(sfp->dev, "EEPROM short read: %d\n", ret); |
| 195 | + return -EAGAIN; |
| 196 | + } |
| 197 | + } |
| 198 | + |
| 199 | /* Cotsworks do not seem to update the checksums when they |
| 200 | * do the final programming with the final module part number, |
| 201 | * serial number and date code. |
| 202 | @@ -1658,9 +1695,6 @@ static int sfp_sm_mod_probe(struct sfp * |
| 203 | } |
| 204 | } |
| 205 | |
| 206 | - /* Apply any early module-specific quirks */ |
| 207 | - sfp_quirks_base(sfp, &id.base); |
| 208 | - |
| 209 | ret = sfp_read(sfp, false, SFP_CC_BASE + 1, &id.ext, sizeof(id.ext)); |
| 210 | if (ret < 0) { |
| 211 | if (report) |