| Message ID | 20201217112708.3473-3-kostap@marvell.com |
|---|---|
| State | New |
| Headers | show |
| Series | spi: new feature and fix for Marvell Orion driver | expand |
Hi Mark, czw., 17 gru 2020 o 12:27 <kostap@marvell.com> napisał(a): > > From: Marcin Wojtas <mw@semihalf.com> > > Some SPI devices, such as SLIC (Subscriber Line Interface Card) > require toggling the CS every transferred byte. Enable such > possibility by creating a new DT property and enabling SPI > device mode update. Add according support in the spi-orion driver. > > Signed-off-by: Marcin Wojtas <mw@semihalf.com> > Signed-off-by: Konstantin Porotchkin <kostap@marvell.com> > --- > drivers/spi/spi-orion.c | 20 +++++++++++++++++++- > drivers/spi/spi.c | 6 ++++-- > include/linux/spi/spi.h | 1 + > 3 files changed, 24 insertions(+), 3 deletions(-) > > diff --git a/drivers/spi/spi-orion.c b/drivers/spi/spi-orion.c > index 3bfda4225d45..7db9034b0879 100644 > --- a/drivers/spi/spi-orion.c > +++ b/drivers/spi/spi-orion.c > @@ -369,8 +369,15 @@ orion_spi_write_read_8bit(struct spi_device *spi, > { > void __iomem *tx_reg, *rx_reg, *int_reg; > struct orion_spi *orion_spi; > + bool cs_single_byte; > + > + cs_single_byte = spi->mode & SPI_1BYTE_CS; > > orion_spi = spi_master_get_devdata(spi->master); > + > + if (cs_single_byte) > + orion_spi_set_cs(spi, 0); > + > tx_reg = spi_reg(orion_spi, ORION_SPI_DATA_OUT_REG); > rx_reg = spi_reg(orion_spi, ORION_SPI_DATA_IN_REG); > int_reg = spi_reg(orion_spi, ORION_SPI_INT_CAUSE_REG); > @@ -384,6 +391,11 @@ orion_spi_write_read_8bit(struct spi_device *spi, > writel(0, tx_reg); > > if (orion_spi_wait_till_ready(orion_spi) < 0) { > + if (cs_single_byte) { > + orion_spi_set_cs(spi, 1); > + /* Satisfy some SLIC devices requirements */ > + udelay(4); > + } > dev_err(&spi->dev, "TXS timed out\n"); > return -1; > } > @@ -391,6 +403,12 @@ orion_spi_write_read_8bit(struct spi_device *spi, > if (rx_buf && *rx_buf) > *(*rx_buf)++ = readl(rx_reg); > > + if (cs_single_byte) { > + orion_spi_set_cs(spi, 1); > + /* Satisfy some SLIC devices requirements */ > + udelay(4); > + } > + > return 1; > } > > @@ -626,7 +644,7 @@ static int orion_spi_probe(struct platform_device *pdev) > } > > /* we support all 4 SPI modes and LSB first option */ > - master->mode_bits = SPI_CPHA | SPI_CPOL | SPI_LSB_FIRST; > + master->mode_bits = SPI_CPHA | SPI_CPOL | SPI_LSB_FIRST | SPI_1BYTE_CS; > master->set_cs = orion_spi_set_cs; > master->transfer_one = orion_spi_transfer_one; > master->num_chipselect = ORION_NUM_CHIPSELECTS; > diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c > index 51d7c004fbab..998579807a04 100644 > --- a/drivers/spi/spi.c > +++ b/drivers/spi/spi.c > @@ -1937,6 +1937,8 @@ static int of_spi_parse_dt(struct spi_controller *ctlr, struct spi_device *spi, > spi->mode |= SPI_LSB_FIRST; > if (of_property_read_bool(nc, "spi-cs-high")) > spi->mode |= SPI_CS_HIGH; > + if (of_find_property(nc, "spi-1byte-cs", NULL)) > + spi->mode |= SPI_1BYTE_CS; Regarding your comment from patch 3/3 that "spi-1byte-cs" should be replaced by handling based on the compatible string - do you mean dropping above parsing and updating SPI bus mode field with SPI_1BYTE_CS flag in the relevant SPI device driver? Best regards, Marcin > > /* Device DUAL/QUAD mode */ > if (!of_property_read_u32(nc, "spi-tx-bus-width", &value)) { > @@ -3419,15 +3421,15 @@ int spi_setup(struct spi_device *spi) > spi_set_thread_rt(spi->controller); > } > > - dev_dbg(&spi->dev, "setup mode %d, %s%s%s%s%u bits/w, %u Hz max --> %d\n", > + dev_dbg(&spi->dev, "setup mode %d, %s%s%s%s%s%u bits/w, %u Hz max --> %d\n", > (int) (spi->mode & (SPI_CPOL | SPI_CPHA)), > (spi->mode & SPI_CS_HIGH) ? "cs_high, " : "", > (spi->mode & SPI_LSB_FIRST) ? "lsb, " : "", > (spi->mode & SPI_3WIRE) ? "3wire, " : "", > (spi->mode & SPI_LOOP) ? "loopback, " : "", > + (spi->mode & SPI_1BYTE_CS) ? "single_cs_byte, " : "", > spi->bits_per_word, spi->max_speed_hz, > status); > - > return status; > } > EXPORT_SYMBOL_GPL(spi_setup); > diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h > index aa09fdc8042d..7f65ff6fc25d 100644 > --- a/include/linux/spi/spi.h > +++ b/include/linux/spi/spi.h > @@ -186,6 +186,7 @@ struct spi_device { > #define SPI_TX_OCTAL 0x2000 /* transmit with 8 wires */ > #define SPI_RX_OCTAL 0x4000 /* receive with 8 wires */ > #define SPI_3WIRE_HIZ 0x8000 /* high impedance turnaround */ > +#define SPI_1BYTE_CS 0x10000 /* toggle cs after each byte */ > int irq; > void *controller_state; > void *controller_data; > -- > 2.17.1 >
On Thu, Dec 17, 2020 at 01:27:07PM +0200, kostap@marvell.com wrote: > Some SPI devices, such as SLIC (Subscriber Line Interface Card) > require toggling the CS every transferred byte. Enable such > possibility by creating a new DT property and enabling SPI > device mode update. Add according support in the spi-orion driver. I'm pretty sure we already support this - if the client driver sets the word length to 8 bits then SPI_CS_WORD ought to do what your change describes as far as I can see. What's missing there? > --- > drivers/spi/spi-orion.c | 20 +++++++++++++++++++- > drivers/spi/spi.c | 6 ++++-- > include/linux/spi/spi.h | 1 + > 3 files changed, 24 insertions(+), 3 deletions(-) This is introducing something into the core and adding a user of it, it should be two separate patches.
czw., 17 gru 2020 o 15:15 Mark Brown <broonie@kernel.org> napisał(a): > > On Thu, Dec 17, 2020 at 01:27:07PM +0200, kostap@marvell.com wrote: > > > Some SPI devices, such as SLIC (Subscriber Line Interface Card) > > require toggling the CS every transferred byte. Enable such > > possibility by creating a new DT property and enabling SPI > > device mode update. Add according support in the spi-orion driver. > > I'm pretty sure we already support this - if the client driver sets the > word length to 8 bits then SPI_CS_WORD ought to do what your change > describes as far as I can see. What's missing there? Sure, that needs to be confirmed on HW, but it looks like the required functionality can be handled with SPI_CS_WORD. As for the justification, the new flag was developed on a kernel revision without it and applied easily, hence the possible oversight. > > > --- > > drivers/spi/spi-orion.c | 20 +++++++++++++++++++- > > drivers/spi/spi.c | 6 ++++-- > > include/linux/spi/spi.h | 1 + > > 3 files changed, 24 insertions(+), 3 deletions(-) > > This is introducing something into the core and adding a user of it, it > should be two separate patches. Agree. Thanks, Marcin
diff --git a/drivers/spi/spi-orion.c b/drivers/spi/spi-orion.c index 3bfda4225d45..7db9034b0879 100644 --- a/drivers/spi/spi-orion.c +++ b/drivers/spi/spi-orion.c @@ -369,8 +369,15 @@ orion_spi_write_read_8bit(struct spi_device *spi, { void __iomem *tx_reg, *rx_reg, *int_reg; struct orion_spi *orion_spi; + bool cs_single_byte; + + cs_single_byte = spi->mode & SPI_1BYTE_CS; orion_spi = spi_master_get_devdata(spi->master); + + if (cs_single_byte) + orion_spi_set_cs(spi, 0); + tx_reg = spi_reg(orion_spi, ORION_SPI_DATA_OUT_REG); rx_reg = spi_reg(orion_spi, ORION_SPI_DATA_IN_REG); int_reg = spi_reg(orion_spi, ORION_SPI_INT_CAUSE_REG); @@ -384,6 +391,11 @@ orion_spi_write_read_8bit(struct spi_device *spi, writel(0, tx_reg); if (orion_spi_wait_till_ready(orion_spi) < 0) { + if (cs_single_byte) { + orion_spi_set_cs(spi, 1); + /* Satisfy some SLIC devices requirements */ + udelay(4); + } dev_err(&spi->dev, "TXS timed out\n"); return -1; } @@ -391,6 +403,12 @@ orion_spi_write_read_8bit(struct spi_device *spi, if (rx_buf && *rx_buf) *(*rx_buf)++ = readl(rx_reg); + if (cs_single_byte) { + orion_spi_set_cs(spi, 1); + /* Satisfy some SLIC devices requirements */ + udelay(4); + } + return 1; } @@ -626,7 +644,7 @@ static int orion_spi_probe(struct platform_device *pdev) } /* we support all 4 SPI modes and LSB first option */ - master->mode_bits = SPI_CPHA | SPI_CPOL | SPI_LSB_FIRST; + master->mode_bits = SPI_CPHA | SPI_CPOL | SPI_LSB_FIRST | SPI_1BYTE_CS; master->set_cs = orion_spi_set_cs; master->transfer_one = orion_spi_transfer_one; master->num_chipselect = ORION_NUM_CHIPSELECTS; diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index 51d7c004fbab..998579807a04 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -1937,6 +1937,8 @@ static int of_spi_parse_dt(struct spi_controller *ctlr, struct spi_device *spi, spi->mode |= SPI_LSB_FIRST; if (of_property_read_bool(nc, "spi-cs-high")) spi->mode |= SPI_CS_HIGH; + if (of_find_property(nc, "spi-1byte-cs", NULL)) + spi->mode |= SPI_1BYTE_CS; /* Device DUAL/QUAD mode */ if (!of_property_read_u32(nc, "spi-tx-bus-width", &value)) { @@ -3419,15 +3421,15 @@ int spi_setup(struct spi_device *spi) spi_set_thread_rt(spi->controller); } - dev_dbg(&spi->dev, "setup mode %d, %s%s%s%s%u bits/w, %u Hz max --> %d\n", + dev_dbg(&spi->dev, "setup mode %d, %s%s%s%s%s%u bits/w, %u Hz max --> %d\n", (int) (spi->mode & (SPI_CPOL | SPI_CPHA)), (spi->mode & SPI_CS_HIGH) ? "cs_high, " : "", (spi->mode & SPI_LSB_FIRST) ? "lsb, " : "", (spi->mode & SPI_3WIRE) ? "3wire, " : "", (spi->mode & SPI_LOOP) ? "loopback, " : "", + (spi->mode & SPI_1BYTE_CS) ? "single_cs_byte, " : "", spi->bits_per_word, spi->max_speed_hz, status); - return status; } EXPORT_SYMBOL_GPL(spi_setup); diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h index aa09fdc8042d..7f65ff6fc25d 100644 --- a/include/linux/spi/spi.h +++ b/include/linux/spi/spi.h @@ -186,6 +186,7 @@ struct spi_device { #define SPI_TX_OCTAL 0x2000 /* transmit with 8 wires */ #define SPI_RX_OCTAL 0x4000 /* receive with 8 wires */ #define SPI_3WIRE_HIZ 0x8000 /* high impedance turnaround */ +#define SPI_1BYTE_CS 0x10000 /* toggle cs after each byte */ int irq; void *controller_state; void *controller_data;