On Fri, Sep 13, 2024 at 10:55:38AM -0400, Parker Newman wrote: > From: Parker Newman <pnewman@xxxxxxxxxxxxxxx> > > This patch adds a quirk similar to eeprom_93xx46 to add an extra clock > cycle before reading data from the EEPROM. > > The 93Cx6 family of EEPROMs output a "dummy 0 bit" between the writing > of the op-code/address from the host to the EEPROM and the reading of > the actual data from the EEPROM. > > More info can be found on page 6 of the AT93C46 datasheet. Similar notes > are found in other 93xx6 datasheets. > Link: https://ww1.microchip.com/downloads/en/DeviceDoc/Atmel-5193-SEEPROM-AT93C46D-Datasheet.pdf Make it a tag (i.e. locate just above your SoB tag) > In summary the read operation for a 93Cx6 EEPROM is: > Write to EEPROM : 110[A5-A0] (9 bits) > Read from EEPROM: 0[D15-D0] (17 bits) The mixed TABs/space here (one extra space after :) > Where: > 110 is the start bit and READ OpCode > [A5-A0] is the address to read from > 0 is a "dummy bit" preceding the actual data > [D15-D0] is the actual data. Also leading spaces, please remove them and use TAB, or use spaces only. > Looking at the READ timing diagrams in the 93Cx6 datasheets the dummy > bit should be clocked out on the last address bit clock cycle meaning it > should be discarded naturally. > > However, depending on the hardware configuration sometimes this dummy > bit is not discarded. This is the case with Exar PCI UARTs which require > an extra clock cycle between sending the address and reading the data. ... > +static inline bool has_quirk_extra_read_cycle(struct eeprom_93cx6 *eeprom) > +{ > + return eeprom->quirks & PCI_EEPROM_QUIRK_EXTRA_READ_CYCLE; > +} So, this makes sense to be in a header since everything else related to that also in the header already. ... > + if (has_quirk_extra_read_cycle(eeprom)) { > + eeprom_93cx6_pulse_high(eeprom); No additional delay is needed? > + eeprom_93cx6_pulse_low(eeprom); > + } > + if (has_quirk_extra_read_cycle(eeprom)) { > + eeprom_93cx6_pulse_high(eeprom); Ditto. > + eeprom_93cx6_pulse_low(eeprom); > + } ... > +/* Some EEPROMs require an extra clock cycle before reading */ > +#define PCI_EEPROM_QUIRK_EXTRA_READ_CYCLE BIT(0) I would move it directly into the structure definitions, just after quirk field (the same way it's done in the other driver)... ... > int width; > + unsigned int quirks; ...somewhere here. > char drive_data; > char reg_data_in; -- With Best Regards, Andy Shevchenko
