| Message ID | 20221202094934.9420-1-edmund.berenson@emlix.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/3] spi: dw: select SS0 when gpio cs is used | expand |
On Fri, Dec 02, 2022 at 10:49:34AM +0100, Edmund Berenson wrote: > Using chips with different cpol, causes first communication > to fail on cpol change. > To avoid this issue write cr0 register before cs is set. I need more details on the problem otherwise this patch seems more like a hack than a fix. The dw_spi_update_config() method already updates the mode before start to execute each SPI-transfer. Basically what you suggest here is the double CR0 register update before the first transfer submission. This doesn't seem like a proper solution. Moreover updating the device setting in the set_cs callback makes it less coherent. So there must be a firm justification why the update is supposed to be done in the dw_spi_set_cs method. > > Suggested-by: Lukasz Zemla <Lukasz.Zemla@woodward.com> > Signed-off-by: Edmund Berenson <edmund.berenson@emlix.com> > --- > drivers/spi/spi-dw-core.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c > index 57c9e384d6d4..c3da4fe3e510 100644 > --- a/drivers/spi/spi-dw-core.c > +++ b/drivers/spi/spi-dw-core.c > @@ -93,6 +93,7 @@ static inline void dw_spi_debugfs_remove(struct dw_spi *dws) > void dw_spi_set_cs(struct spi_device *spi, bool enable) > { > struct dw_spi *dws = spi_controller_get_devdata(spi->controller); > + struct dw_spi_chip_data *chip = spi_get_ctldata(spi); > bool cs_high = !!(spi->mode & SPI_CS_HIGH); > u8 enable_cs = 0; > > @@ -106,8 +107,13 @@ void dw_spi_set_cs(struct spi_device *spi, bool enable) > * Enable register no matter whether the SPI core is configured to > * support active-high or active-low CS level. > */ > - if (cs_high == enable) > + if (cs_high == enable) { > + dw_spi_enable_chip(dws, 0); > + dw_writel(dws, DW_SPI_CTRLR0, chip->cr0); > + dw_spi_enable_chip(dws, 1); > + > dw_writel(dws, DW_SPI_SER, BIT(enable_cs)); > + } > else Documentation/process/coding-style.rst: 3) Placing Braces and Spaces -Sergey > dw_writel(dws, DW_SPI_SER, 0); > } > -- > 2.37.4 >
diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c index 57c9e384d6d4..c3da4fe3e510 100644 --- a/drivers/spi/spi-dw-core.c +++ b/drivers/spi/spi-dw-core.c @@ -93,6 +93,7 @@ static inline void dw_spi_debugfs_remove(struct dw_spi *dws) void dw_spi_set_cs(struct spi_device *spi, bool enable) { struct dw_spi *dws = spi_controller_get_devdata(spi->controller); + struct dw_spi_chip_data *chip = spi_get_ctldata(spi); bool cs_high = !!(spi->mode & SPI_CS_HIGH); u8 enable_cs = 0; @@ -106,8 +107,13 @@ void dw_spi_set_cs(struct spi_device *spi, bool enable) * Enable register no matter whether the SPI core is configured to * support active-high or active-low CS level. */ - if (cs_high == enable) + if (cs_high == enable) { + dw_spi_enable_chip(dws, 0); + dw_writel(dws, DW_SPI_CTRLR0, chip->cr0); + dw_spi_enable_chip(dws, 1); + dw_writel(dws, DW_SPI_SER, BIT(enable_cs)); + } else dw_writel(dws, DW_SPI_SER, 0); }
Using chips with different cpol, causes first communication to fail on cpol change. To avoid this issue write cr0 register before cs is set. Suggested-by: Lukasz Zemla <Lukasz.Zemla@woodward.com> Signed-off-by: Edmund Berenson <edmund.berenson@emlix.com> --- drivers/spi/spi-dw-core.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)