* [PATCH v3 1/5] misc: eeprom: eeprom_93cx6: Add quirk for extra read clock cycle
2024-10-02 14:14 [PATCH v3 0/5] serial: 8250_exar: Replace custom EEPROM code with eeprom_93cx6 Parker Newman
@ 2024-10-02 14:14 ` Parker Newman
2024-10-02 14:27 ` Andy Shevchenko
2024-10-02 14:14 ` [PATCH v3 2/5] misc: eeprom: eeprom_93cx6: Switch to BIT() macro Parker Newman
` (4 subsequent siblings)
5 siblings, 1 reply; 10+ messages in thread
From: Parker Newman @ 2024-10-02 14:14 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, Andy Shevchenko, Arnd Bergmann,
linux-kernel, linux-serial
Cc: Parker Newman
From: Parker Newman <pnewman@connecttech.com>
Add 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 (linked below).
Similar notes are found in other 93xx6 datasheets.
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)
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.
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.
Datasheet: https://ww1.microchip.com/downloads/en/DeviceDoc/Atmel-5193-SEEPROM-AT93C46D-Datasheet.pdf
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Parker Newman <pnewman@connecttech.com>
---
Changes in v2:
- Moved quirk define into struct eeprom_93cx6.
- Moved has_quirk_extra_read_cycle() from eeprom_93cx6.c into eeprom_93cx6.h.
- Fixed commit message formatting.
Changes in v3:
- Commit message wording.
- Rebased to v6.12-rc1.
drivers/misc/eeprom/eeprom_93cx6.c | 10 ++++++++++
include/linux/eeprom_93cx6.h | 12 ++++++++++++
2 files changed, 22 insertions(+)
diff --git a/drivers/misc/eeprom/eeprom_93cx6.c b/drivers/misc/eeprom/eeprom_93cx6.c
index 9627294fe3e9..4c9827fe9217 100644
--- a/drivers/misc/eeprom/eeprom_93cx6.c
+++ b/drivers/misc/eeprom/eeprom_93cx6.c
@@ -186,6 +186,11 @@ void eeprom_93cx6_read(struct eeprom_93cx6 *eeprom, const u8 word,
eeprom_93cx6_write_bits(eeprom, command,
PCI_EEPROM_WIDTH_OPCODE + eeprom->width);
+ if (has_quirk_extra_read_cycle(eeprom)) {
+ eeprom_93cx6_pulse_high(eeprom);
+ eeprom_93cx6_pulse_low(eeprom);
+ }
+
/*
* Read the requested 16 bits.
*/
@@ -252,6 +257,11 @@ void eeprom_93cx6_readb(struct eeprom_93cx6 *eeprom, const u8 byte,
eeprom_93cx6_write_bits(eeprom, command,
PCI_EEPROM_WIDTH_OPCODE + eeprom->width + 1);
+ if (has_quirk_extra_read_cycle(eeprom)) {
+ eeprom_93cx6_pulse_high(eeprom);
+ eeprom_93cx6_pulse_low(eeprom);
+ }
+
/*
* Read the requested 8 bits.
*/
diff --git a/include/linux/eeprom_93cx6.h b/include/linux/eeprom_93cx6.h
index c860c72a921d..4d141345f4d4 100644
--- a/include/linux/eeprom_93cx6.h
+++ b/include/linux/eeprom_93cx6.h
@@ -11,6 +11,8 @@
Supported chipsets: 93c46, 93c56 and 93c66.
*/
+#include <linux/bits.h>
+
/*
* EEPROM operation defines.
*/
@@ -34,6 +36,7 @@
* @register_write(struct eeprom_93cx6 *eeprom): handler to
* write to the eeprom register by using all reg_* fields.
* @width: eeprom width, should be one of the PCI_EEPROM_WIDTH_* defines
+ * @quirks: eeprom or controller quirks
* @drive_data: Set if we're driving the data line.
* @reg_data_in: register field to indicate data input
* @reg_data_out: register field to indicate data output
@@ -50,6 +53,9 @@ struct eeprom_93cx6 {
void (*register_write)(struct eeprom_93cx6 *eeprom);
int width;
+ unsigned int quirks;
+/* Some EEPROMs require an extra clock cycle before reading */
+#define PCI_EEPROM_QUIRK_EXTRA_READ_CYCLE BIT(0)
char drive_data;
char reg_data_in;
@@ -71,3 +77,9 @@ extern void eeprom_93cx6_wren(struct eeprom_93cx6 *eeprom, bool enable);
extern void eeprom_93cx6_write(struct eeprom_93cx6 *eeprom,
u8 addr, u16 data);
+
+static inline bool has_quirk_extra_read_cycle(struct eeprom_93cx6 *eeprom)
+{
+ return eeprom->quirks & PCI_EEPROM_QUIRK_EXTRA_READ_CYCLE;
+}
+
--
2.46.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v3 1/5] misc: eeprom: eeprom_93cx6: Add quirk for extra read clock cycle
2024-10-02 14:14 ` [PATCH v3 1/5] misc: eeprom: eeprom_93cx6: Add quirk for extra read clock cycle Parker Newman
@ 2024-10-02 14:27 ` Andy Shevchenko
0 siblings, 0 replies; 10+ messages in thread
From: Andy Shevchenko @ 2024-10-02 14:27 UTC (permalink / raw)
To: Parker Newman
Cc: Greg Kroah-Hartman, Jiri Slaby, Arnd Bergmann, linux-kernel,
linux-serial, Parker Newman
On Wed, Oct 02, 2024 at 10:14:07AM -0400, Parker Newman wrote:
> From: Parker Newman <pnewman@connecttech.com>
>
> Add 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 (linked below).
> Similar notes are found in other 93xx6 datasheets.
>
> 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)
>
> 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.
>
> 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;
> +}
> +
Seems to me the redundant new line at the end of file.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3 2/5] misc: eeprom: eeprom_93cx6: Switch to BIT() macro
2024-10-02 14:14 [PATCH v3 0/5] serial: 8250_exar: Replace custom EEPROM code with eeprom_93cx6 Parker Newman
2024-10-02 14:14 ` [PATCH v3 1/5] misc: eeprom: eeprom_93cx6: Add quirk for extra read clock cycle Parker Newman
@ 2024-10-02 14:14 ` Parker Newman
2024-10-02 14:14 ` [PATCH v3 3/5] serial: 8250_exar: Replace custom EEPROM read with eeprom_93cx6 Parker Newman
` (3 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Parker Newman @ 2024-10-02 14:14 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, Andy Shevchenko, Arnd Bergmann,
linux-kernel, linux-serial
Cc: Parker Newman
From: Parker Newman <pnewman@connecttech.com>
Use the BIT() macro rather than (1 << (i - 1)).
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Parker Newman <pnewman@connecttech.com>
---
drivers/misc/eeprom/eeprom_93cx6.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/misc/eeprom/eeprom_93cx6.c b/drivers/misc/eeprom/eeprom_93cx6.c
index 4c9827fe9217..e6f0e0fc1ca2 100644
--- a/drivers/misc/eeprom/eeprom_93cx6.c
+++ b/drivers/misc/eeprom/eeprom_93cx6.c
@@ -8,6 +8,7 @@
* Supported chipsets: 93c46 & 93c66.
*/
+#include <linux/bits.h>
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/delay.h>
@@ -102,7 +103,7 @@ static void eeprom_93cx6_write_bits(struct eeprom_93cx6 *eeprom,
/*
* Check if this bit needs to be set.
*/
- eeprom->reg_data_in = !!(data & (1 << (i - 1)));
+ eeprom->reg_data_in = !!(data & BIT(i - 1));
/*
* Write the bit to the eeprom register.
@@ -152,7 +153,7 @@ static void eeprom_93cx6_read_bits(struct eeprom_93cx6 *eeprom,
* Read if the bit has been set.
*/
if (eeprom->reg_data_out)
- buf |= (1 << (i - 1));
+ buf |= BIT(i - 1);
eeprom_93cx6_pulse_low(eeprom);
}
--
2.46.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v3 3/5] serial: 8250_exar: Replace custom EEPROM read with eeprom_93cx6
2024-10-02 14:14 [PATCH v3 0/5] serial: 8250_exar: Replace custom EEPROM code with eeprom_93cx6 Parker Newman
2024-10-02 14:14 ` [PATCH v3 1/5] misc: eeprom: eeprom_93cx6: Add quirk for extra read clock cycle Parker Newman
2024-10-02 14:14 ` [PATCH v3 2/5] misc: eeprom: eeprom_93cx6: Switch to BIT() macro Parker Newman
@ 2024-10-02 14:14 ` Parker Newman
2024-10-02 14:14 ` [PATCH v3 4/5] serial: 8250_exar: Remove old exar_ee_read() and other unneeded code Parker Newman
` (2 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Parker Newman @ 2024-10-02 14:14 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, Andy Shevchenko, Arnd Bergmann,
linux-kernel, linux-serial
Cc: Parker Newman
From: Parker Newman <pnewman@connecttech.com>
Replace the custom 93cx6 EEPROM read functions with the eeprom_93cx6
driver. This removes duplicate code and improves code readability.
Replace exar_ee_read() calls with eeprom_93cx6_read() or
eeprom_93cx6_multiread().
Add "select EEPROM_93CX6" to config SERIAL_8250_EXAR to ensure
eeprom_93cx6 driver is also compiled when 8250_exar driver is selected.
Note: Old exar_ee_read() and associated functions are removed in next
patch in this series.
Link to mailing list discussion with Andy Shevchenko for reference.
Link: https://lore.kernel.org/linux-serial/Ztr5u2wEt8VF1IdI@black.fi.intel.com/
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Parker Newman <pnewman@connecttech.com>
---
Changes in v2:
- Refactored cti_read_osc_freq() based on feedback.
- Moved Kconfig change into this patch.
- Sorted headers.
- Fixed comment.
Changes in v3:
- Commit message wording.
- Rebased to v6.12-rc1.
drivers/tty/serial/8250/8250_exar.c | 55 +++++++++++++++++++++++------
drivers/tty/serial/8250/Kconfig | 1 +
2 files changed, 45 insertions(+), 11 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_exar.c b/drivers/tty/serial/8250/8250_exar.c
index b7a75db15249..c40e86920110 100644
--- a/drivers/tty/serial/8250/8250_exar.c
+++ b/drivers/tty/serial/8250/8250_exar.c
@@ -11,6 +11,7 @@
#include <linux/delay.h>
#include <linux/device.h>
#include <linux/dmi.h>
+#include <linux/eeprom_93cx6.h>
#include <linux/interrupt.h>
#include <linux/io.h>
#include <linux/math.h>
@@ -252,6 +253,7 @@ struct exar8250 {
unsigned int nr;
unsigned int osc_freq;
struct exar8250_board *board;
+ struct eeprom_93cx6 eeprom;
void __iomem *virt;
int line[];
};
@@ -355,6 +357,39 @@ static u16 exar_ee_read(struct exar8250 *priv, u8 ee_addr)
return data;
}
+static void exar_eeprom_93cx6_reg_read(struct eeprom_93cx6 *eeprom)
+{
+ struct exar8250 *priv = eeprom->data;
+ u8 regb = exar_read_reg(priv, UART_EXAR_REGB);
+
+ /* EECK and EECS always read 0 from REGB so only set EEDO */
+ eeprom->reg_data_out = regb & UART_EXAR_REGB_EEDO;
+}
+
+static void exar_eeprom_93cx6_reg_write(struct eeprom_93cx6 *eeprom)
+{
+ struct exar8250 *priv = eeprom->data;
+ u8 regb = 0;
+
+ if (eeprom->reg_data_in)
+ regb |= UART_EXAR_REGB_EEDI;
+ if (eeprom->reg_data_clock)
+ regb |= UART_EXAR_REGB_EECK;
+ if (eeprom->reg_chip_select)
+ regb |= UART_EXAR_REGB_EECS;
+
+ exar_write_reg(priv, UART_EXAR_REGB, regb);
+}
+
+static void exar_eeprom_init(struct exar8250 *priv)
+{
+ priv->eeprom.data = priv;
+ priv->eeprom.register_read = exar_eeprom_93cx6_reg_read;
+ priv->eeprom.register_write = exar_eeprom_93cx6_reg_write;
+ priv->eeprom.width = PCI_EEPROM_WIDTH_93C46;
+ priv->eeprom.quirks |= PCI_EEPROM_QUIRK_EXTRA_READ_CYCLE;
+}
+
/**
* exar_mpio_config_output() - Configure an Exar MPIO as an output
* @priv: Device's private structure
@@ -696,20 +731,16 @@ static int cti_plx_int_enable(struct exar8250 *priv)
*/
static int cti_read_osc_freq(struct exar8250 *priv, u8 eeprom_offset)
{
- u16 lower_word;
- u16 upper_word;
+ __le16 ee_words[2];
+ u32 osc_freq;
- lower_word = exar_ee_read(priv, eeprom_offset);
- // Check if EEPROM word was blank
- if (lower_word == 0xFFFF)
- return -EIO;
+ eeprom_93cx6_multiread(&priv->eeprom, eeprom_offset, ee_words, ARRAY_SIZE(ee_words));
- upper_word = exar_ee_read(priv, (eeprom_offset + 1));
- if (upper_word == 0xFFFF)
+ osc_freq = le16_to_cpu(ee_words[0]) | (le16_to_cpu(ee_words[1]) << 16);
+ if (osc_freq == GENMASK(31, 0))
return -EIO;
- return FIELD_PREP(CTI_EE_MASK_OSC_FREQ_LOWER, lower_word) |
- FIELD_PREP(CTI_EE_MASK_OSC_FREQ_UPPER, upper_word);
+ return osc_freq;
}
/**
@@ -833,7 +864,7 @@ static enum cti_port_type cti_get_port_type_xr17v35x(struct exar8250 *priv,
u8 offset;
offset = CTI_EE_OFF_XR17V35X_PORT_FLAGS + port_num;
- port_flags = exar_ee_read(priv, offset);
+ eeprom_93cx6_read(&priv->eeprom, offset, &port_flags);
port_type = FIELD_GET(CTI_EE_MASK_PORT_FLAGS_TYPE, port_flags);
if (CTI_PORT_TYPE_VALID(port_type))
@@ -1551,6 +1582,8 @@ exar_pci_probe(struct pci_dev *pcidev, const struct pci_device_id *ent)
if (rc)
return rc;
+ exar_eeprom_init(priv);
+
for (i = 0; i < nr_ports && i < maxnr; i++) {
rc = board->setup(priv, pcidev, &uart, i);
if (rc) {
diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
index 47ff50763c04..94910ced8238 100644
--- a/drivers/tty/serial/8250/Kconfig
+++ b/drivers/tty/serial/8250/Kconfig
@@ -150,6 +150,7 @@ config SERIAL_8250_EXAR
tristate "8250/16550 Exar/Commtech PCI/PCIe device support"
depends on SERIAL_8250 && PCI
select SERIAL_8250_PCILIB
+ select EEPROM_93CX6
default SERIAL_8250
help
This builds support for XR17C1xx, XR17V3xx and some Commtech
--
2.46.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v3 4/5] serial: 8250_exar: Remove old exar_ee_read() and other unneeded code
2024-10-02 14:14 [PATCH v3 0/5] serial: 8250_exar: Replace custom EEPROM code with eeprom_93cx6 Parker Newman
` (2 preceding siblings ...)
2024-10-02 14:14 ` [PATCH v3 3/5] serial: 8250_exar: Replace custom EEPROM read with eeprom_93cx6 Parker Newman
@ 2024-10-02 14:14 ` Parker Newman
2024-10-02 14:29 ` Andy Shevchenko
2024-10-02 14:14 ` [PATCH v3 5/5] serial: 8250_exar: Group CTI EEPROM offsets by device Parker Newman
2024-10-02 14:31 ` [PATCH v3 0/5] serial: 8250_exar: Replace custom EEPROM code with eeprom_93cx6 Andy Shevchenko
5 siblings, 1 reply; 10+ messages in thread
From: Parker Newman @ 2024-10-02 14:14 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, Andy Shevchenko, Arnd Bergmann,
linux-kernel, linux-serial
Cc: Parker Newman
From: Parker Newman <pnewman@connecttech.com>
Remove the old exar_ee_read() and associated helper functions.
Remove defines that are no longer needed after the switch to using the
eeprom_93cx6 driver.
Add CTI_EE_MASK_OSC_FREQ define.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Parker Newman <pnewman@connecttech.com>
---
Changes in v3:
- Commit message wording.
- Rebased to v6.12-rc1.
- Remove CTI_EE_MASK_OSC_FREQ_LOWER and CTI_EE_MASK_OSC_FREQ_UPPER.
- Add CTI_EE_MASK_OSC_FREQ define.
drivers/tty/serial/8250/8250_exar.c | 95 +----------------------------
1 file changed, 2 insertions(+), 93 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_exar.c b/drivers/tty/serial/8250/8250_exar.c
index c40e86920110..470e8128c79a 100644
--- a/drivers/tty/serial/8250/8250_exar.c
+++ b/drivers/tty/serial/8250/8250_exar.c
@@ -136,8 +136,6 @@
#define UART_EXAR_REGB_EECS BIT(5)
#define UART_EXAR_REGB_EEDI BIT(6)
#define UART_EXAR_REGB_EEDO BIT(7)
-#define UART_EXAR_REGB_EE_ADDR_SIZE 6
-#define UART_EXAR_REGB_EE_DATA_SIZE 16
#define UART_EXAR_XR17C15X_PORT_OFFSET 0x200
#define UART_EXAR_XR17V25X_PORT_OFFSET 0x200
@@ -190,8 +188,7 @@
#define CTI_EE_OFF_XR17V35X_PORT_FLAGS 0x14 /* 1 word */
#define CTI_EE_MASK_PORT_FLAGS_TYPE GENMASK(7, 0)
-#define CTI_EE_MASK_OSC_FREQ_LOWER GENMASK(15, 0)
-#define CTI_EE_MASK_OSC_FREQ_UPPER GENMASK(31, 16)
+#define CTI_EE_MASK_OSC_FREQ GENMASK(31, 0)
#define CTI_FPGA_RS485_IO_REG 0x2008
#define CTI_FPGA_CFG_INT_EN_REG 0x48
@@ -269,94 +266,6 @@ static inline u8 exar_read_reg(struct exar8250 *priv, unsigned int reg)
return readb(priv->virt + reg);
}
-static inline void exar_ee_select(struct exar8250 *priv)
-{
- // Set chip select pin high to enable EEPROM reads/writes
- exar_write_reg(priv, UART_EXAR_REGB, UART_EXAR_REGB_EECS);
- // Min ~500ns delay needed between CS assert and EEPROM access
- udelay(1);
-}
-
-static inline void exar_ee_deselect(struct exar8250 *priv)
-{
- exar_write_reg(priv, UART_EXAR_REGB, 0x00);
-}
-
-static inline void exar_ee_write_bit(struct exar8250 *priv, u8 bit)
-{
- u8 value = UART_EXAR_REGB_EECS;
-
- if (bit)
- value |= UART_EXAR_REGB_EEDI;
-
- // Clock out the bit on the EEPROM interface
- exar_write_reg(priv, UART_EXAR_REGB, value);
- // 2us delay = ~500khz clock speed
- udelay(2);
-
- value |= UART_EXAR_REGB_EECK;
-
- exar_write_reg(priv, UART_EXAR_REGB, value);
- udelay(2);
-}
-
-static inline u8 exar_ee_read_bit(struct exar8250 *priv)
-{
- u8 regb;
- u8 value = UART_EXAR_REGB_EECS;
-
- // Clock in the bit on the EEPROM interface
- exar_write_reg(priv, UART_EXAR_REGB, value);
- // 2us delay = ~500khz clock speed
- udelay(2);
-
- value |= UART_EXAR_REGB_EECK;
-
- exar_write_reg(priv, UART_EXAR_REGB, value);
- udelay(2);
-
- regb = exar_read_reg(priv, UART_EXAR_REGB);
-
- return (regb & UART_EXAR_REGB_EEDO ? 1 : 0);
-}
-
-/**
- * exar_ee_read() - Read a word from the EEPROM
- * @priv: Device's private structure
- * @ee_addr: Offset of EEPROM to read word from
- *
- * Read a single 16bit word from an Exar UART's EEPROM.
- * The type of the EEPROM is AT93C46D.
- *
- * Return: EEPROM word
- */
-static u16 exar_ee_read(struct exar8250 *priv, u8 ee_addr)
-{
- int i;
- u16 data = 0;
-
- exar_ee_select(priv);
-
- // Send read command (opcode 110)
- exar_ee_write_bit(priv, 1);
- exar_ee_write_bit(priv, 1);
- exar_ee_write_bit(priv, 0);
-
- // Send address to read from
- for (i = UART_EXAR_REGB_EE_ADDR_SIZE - 1; i >= 0; i--)
- exar_ee_write_bit(priv, ee_addr & BIT(i));
-
- // Read data 1 bit at a time starting with a dummy bit
- for (i = UART_EXAR_REGB_EE_DATA_SIZE; i >= 0; i--) {
- if (exar_ee_read_bit(priv))
- data |= BIT(i);
- }
-
- exar_ee_deselect(priv);
-
- return data;
-}
-
static void exar_eeprom_93cx6_reg_read(struct eeprom_93cx6 *eeprom)
{
struct exar8250 *priv = eeprom->data;
@@ -737,7 +646,7 @@ static int cti_read_osc_freq(struct exar8250 *priv, u8 eeprom_offset)
eeprom_93cx6_multiread(&priv->eeprom, eeprom_offset, ee_words, ARRAY_SIZE(ee_words));
osc_freq = le16_to_cpu(ee_words[0]) | (le16_to_cpu(ee_words[1]) << 16);
- if (osc_freq == GENMASK(31, 0))
+ if (osc_freq == CTI_EE_MASK_OSC_FREQ)
return -EIO;
return osc_freq;
--
2.46.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v3 4/5] serial: 8250_exar: Remove old exar_ee_read() and other unneeded code
2024-10-02 14:14 ` [PATCH v3 4/5] serial: 8250_exar: Remove old exar_ee_read() and other unneeded code Parker Newman
@ 2024-10-02 14:29 ` Andy Shevchenko
0 siblings, 0 replies; 10+ messages in thread
From: Andy Shevchenko @ 2024-10-02 14:29 UTC (permalink / raw)
To: Parker Newman
Cc: Greg Kroah-Hartman, Jiri Slaby, Arnd Bergmann, linux-kernel,
linux-serial, Parker Newman
On Wed, Oct 02, 2024 at 10:14:10AM -0400, Parker Newman wrote:
> From: Parker Newman <pnewman@connecttech.com>
>
> Remove the old exar_ee_read() and associated helper functions.
> Remove defines that are no longer needed after the switch to using the
> eeprom_93cx6 driver.
> Add CTI_EE_MASK_OSC_FREQ define.
Seems like you have a rebase issues (you folded changes to the wrong patch).
...
> drivers/tty/serial/8250/8250_exar.c | 95 +----------------------------
> 1 file changed, 2 insertions(+), 93 deletions(-)
Shouldn't be + lines here.
...
> +#define CTI_EE_MASK_OSC_FREQ GENMASK(31, 0)
...
> osc_freq = le16_to_cpu(ee_words[0]) | (le16_to_cpu(ee_words[1]) << 16);
> - if (osc_freq == GENMASK(31, 0))
> + if (osc_freq == CTI_EE_MASK_OSC_FREQ)
> return -EIO;
You just introduced the - variant in the previous patch.
So, squash these two changes in the patch that adds this new conditional check.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3 5/5] serial: 8250_exar: Group CTI EEPROM offsets by device
2024-10-02 14:14 [PATCH v3 0/5] serial: 8250_exar: Replace custom EEPROM code with eeprom_93cx6 Parker Newman
` (3 preceding siblings ...)
2024-10-02 14:14 ` [PATCH v3 4/5] serial: 8250_exar: Remove old exar_ee_read() and other unneeded code Parker Newman
@ 2024-10-02 14:14 ` Parker Newman
2024-10-02 14:31 ` [PATCH v3 0/5] serial: 8250_exar: Replace custom EEPROM code with eeprom_93cx6 Andy Shevchenko
5 siblings, 0 replies; 10+ messages in thread
From: Parker Newman @ 2024-10-02 14:14 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, Andy Shevchenko, Arnd Bergmann,
linux-kernel, linux-serial
Cc: Parker Newman
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
It's not obvious from the first glance that the list of the CTI EEPROM
offsets covers three different models, let's group them accordingly for
better readability.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Parker Newman <pnewman@connecttech.com>
---
drivers/tty/serial/8250/8250_exar.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_exar.c b/drivers/tty/serial/8250/8250_exar.c
index 470e8128c79a..e37c10b60a7e 100644
--- a/drivers/tty/serial/8250/8250_exar.c
+++ b/drivers/tty/serial/8250/8250_exar.c
@@ -178,11 +178,13 @@
/* CTI EEPROM offsets */
#define CTI_EE_OFF_XR17C15X_OSC_FREQ 0x04 /* 2 words */
-#define CTI_EE_OFF_XR17V25X_OSC_FREQ 0x08 /* 2 words */
#define CTI_EE_OFF_XR17C15X_PART_NUM 0x0A /* 4 words */
-#define CTI_EE_OFF_XR17V25X_PART_NUM 0x0E /* 4 words */
#define CTI_EE_OFF_XR17C15X_SERIAL_NUM 0x0E /* 1 word */
+
+#define CTI_EE_OFF_XR17V25X_OSC_FREQ 0x08 /* 2 words */
+#define CTI_EE_OFF_XR17V25X_PART_NUM 0x0E /* 4 words */
#define CTI_EE_OFF_XR17V25X_SERIAL_NUM 0x12 /* 1 word */
+
#define CTI_EE_OFF_XR17V35X_SERIAL_NUM 0x11 /* 2 word */
#define CTI_EE_OFF_XR17V35X_BRD_FLAGS 0x13 /* 1 word */
#define CTI_EE_OFF_XR17V35X_PORT_FLAGS 0x14 /* 1 word */
--
2.46.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v3 0/5] serial: 8250_exar: Replace custom EEPROM code with eeprom_93cx6
2024-10-02 14:14 [PATCH v3 0/5] serial: 8250_exar: Replace custom EEPROM code with eeprom_93cx6 Parker Newman
` (4 preceding siblings ...)
2024-10-02 14:14 ` [PATCH v3 5/5] serial: 8250_exar: Group CTI EEPROM offsets by device Parker Newman
@ 2024-10-02 14:31 ` Andy Shevchenko
2024-10-02 14:37 ` Parker Newman
5 siblings, 1 reply; 10+ messages in thread
From: Andy Shevchenko @ 2024-10-02 14:31 UTC (permalink / raw)
To: Parker Newman
Cc: Greg Kroah-Hartman, Jiri Slaby, Arnd Bergmann, linux-kernel,
linux-serial, Parker Newman
On Wed, Oct 02, 2024 at 10:14:06AM -0400, Parker Newman wrote:
> From: Parker Newman <pnewman@connecttech.com>
>
> This series of patches replaces the custom 93cx6 EEPROM read functions in
> the 8250_exar driver with the eeprom_93cx6 driver. This removes duplicate code
> and improves code readability.
>
> In order to use the eeprom_93cx6 driver a quirk needed to be added to add an
> extra clock cycle before reading from the EEPROM. This is similar to the
> quirk in the eeprom_93xx46 driver.
Everything is fine except one minor (in patch 1) and one more serious
(see patches 3 & 4) issue. I just commented on them. Fix and send a final
v4, my tags are already there and you may keep them.
Thank you!
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v3 0/5] serial: 8250_exar: Replace custom EEPROM code with eeprom_93cx6
2024-10-02 14:31 ` [PATCH v3 0/5] serial: 8250_exar: Replace custom EEPROM code with eeprom_93cx6 Andy Shevchenko
@ 2024-10-02 14:37 ` Parker Newman
0 siblings, 0 replies; 10+ messages in thread
From: Parker Newman @ 2024-10-02 14:37 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Greg Kroah-Hartman, Jiri Slaby, Arnd Bergmann, linux-kernel,
linux-serial, Parker Newman
On Wed, 2 Oct 2024 17:31:22 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> On Wed, Oct 02, 2024 at 10:14:06AM -0400, Parker Newman wrote:
> > From: Parker Newman <pnewman@connecttech.com>
> >
> > This series of patches replaces the custom 93cx6 EEPROM read functions in
> > the 8250_exar driver with the eeprom_93cx6 driver. This removes duplicate code
> > and improves code readability.
> >
> > In order to use the eeprom_93cx6 driver a quirk needed to be added to add an
> > extra clock cycle before reading from the EEPROM. This is similar to the
> > quirk in the eeprom_93xx46 driver.
>
> Everything is fine except one minor (in patch 1) and one more serious
> (see patches 3 & 4) issue. I just commented on them. Fix and send a final
> v4, my tags are already there and you may keep them.
>
> Thank you!
>
Thank you, I will fix and resend.
^ permalink raw reply [flat|nested] 10+ messages in thread