| Message ID | 20230511135632.78344-1-bstruempfel@ultratronik.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | spi: Add option to keep the MOSI line low, when it is idle. | expand |
Hi Boerge, On Thu, May 11, 2023 at 10:58 AM Boerge Struempfel <boerge.struempfel@gmail.com> wrote: > > By default, the imx spi controller uses a high mosi line, whenever it is > idle. This may not be desired in all use cases. For example neopixel > leds can get confused and flicker due to misinterpreting the idle state. > Therefore, we introduce a new spi-mode bit, with which the idle behaviour > can be overwritten on a per device basis. ... > + if (of_property_read_bool(nc, "spi-mosi-idle-low")) > + spi->mode |= SPI_MOSI_IDLE_LOW; Yes, this is useful. As this is a new property, please send a patch that adds it to: Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml Thanks
Hello, > -----Original Message----- > From: Boerge Struempfel <boerge.struempfel@gmail.com> > Sent: Thursday, May 11, 2023 7:27 PM > Cc: boerge.struempfel@gmail.com; bstruempfel@ultratronik.de; Mark > Brown <broonie@kernel.org>; Shawn Guo <shawnguo@kernel.org>; Sascha > Hauer <s.hauer@pengutronix.de>; Pengutronix Kernel Team > <kernel@pengutronix.de>; Fabio Estevam <festevam@gmail.com>; NXP > Linux Team <linux-imx@nxp.com>; linux-spi@vger.kernel.org; linux-arm- > kernel@lists.infradead.org; linux-kernel@vger.kernel.org > Subject: [PATCH] spi: Add option to keep the MOSI line low, when it is idle. > > CAUTION: This message has originated from an External Source. Please use > proper judgment and caution when opening attachments, clicking links, or > responding to this email. > > > By default, the imx spi controller uses a high mosi line, whenever it is idle. > This may not be desired in all use cases. For example neopixel leds can get > confused and flicker due to misinterpreting the idle state. > Therefore, we introduce a new spi-mode bit, with which the idle behaviour > can be overwritten on a per device basis. > > Signed-off-by: Boerge Struempfel <bstruempfel@ultratronik.de> > --- > drivers/spi/spi-imx.c | 9 ++++++++- > drivers/spi/spi.c | 2 ++ > include/uapi/linux/spi/spi.h | 3 ++- > 3 files changed, 12 insertions(+), 2 deletions(-) > > diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c index > 34e5f81ec431e..6acab2b4ffaa5 100644 > --- a/drivers/spi/spi-imx.c > +++ b/drivers/spi/spi-imx.c > @@ -281,6 +281,7 @@ static bool spi_imx_can_dma(struct spi_controller > *controller, struct spi_device #define MX51_ECSPI_CONFIG_SCLKPOL(cs) (1 > << ((cs & 3) + 4)) #define MX51_ECSPI_CONFIG_SBBCTRL(cs) (1 << ((cs & 3) + > 8)) > #define MX51_ECSPI_CONFIG_SSBPOL(cs) (1 << ((cs & 3) + 12)) > +#define MX51_ECSPI_CONFIG_DATACTL(cs) (1 << ((cs & 3) + 16)) > #define MX51_ECSPI_CONFIG_SCLKCTL(cs) (1 << ((cs & 3) + 20)) > > #define MX51_ECSPI_INT 0x10 > @@ -573,6 +574,11 @@ static int mx51_ecspi_prepare_message(struct > spi_imx_data *spi_imx, > cfg &= ~MX51_ECSPI_CONFIG_SCLKCTL(spi_get_chipselect(spi, 0)); > } > > + if (spi->mode & SPI_MOSI_IDLE_LOW) > + cfg |= MX51_ECSPI_CONFIG_DATACTL(spi->chip_select); Kindly replace all occurrence of spi->chip_select with spi_get_chipselect(spi, 0) https://github.com/torvalds/linux/commit/9e264f3f85a56cc109cc2d6010a48aa89d5c1ff1 > + else > + cfg &= ~MX51_ECSPI_CONFIG_DATACTL(spi->chip_select); > + > if (spi->mode & SPI_CS_HIGH) > cfg |= MX51_ECSPI_CONFIG_SSBPOL(spi_get_chipselect(spi, 0)); > else > @@ -1743,7 +1749,8 @@ static int spi_imx_probe(struct platform_device > *pdev) > spi_imx->controller->prepare_message = spi_imx_prepare_message; > spi_imx->controller->unprepare_message = > spi_imx_unprepare_message; > spi_imx->controller->slave_abort = spi_imx_slave_abort; > - spi_imx->controller->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH | > SPI_NO_CS; > + spi_imx->controller->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH > | SPI_NO_CS | > + SPI_MOSI_IDLE_LOW; > > if (is_imx35_cspi(spi_imx) || is_imx51_ecspi(spi_imx) || > is_imx53_ecspi(spi_imx)) > diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index > 9291b2a0e8871..3ad538b317a84 100644 > --- a/drivers/spi/spi.c > +++ b/drivers/spi/spi.c > @@ -2260,6 +2260,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_property_read_bool(nc, "spi-mosi-idle-low")) > + spi->mode |= SPI_MOSI_IDLE_LOW; > > /* Device DUAL/QUAD mode */ > if (!of_property_read_u32(nc, "spi-tx-bus-width", &value)) { diff --git > a/include/uapi/linux/spi/spi.h b/include/uapi/linux/spi/spi.h index > 9d5f580597039..ca56e477d1619 100644 > --- a/include/uapi/linux/spi/spi.h > +++ b/include/uapi/linux/spi/spi.h > @@ -28,6 +28,7 @@ > #define SPI_RX_OCTAL _BITUL(14) /* receive with 8 wires */ > #define SPI_3WIRE_HIZ _BITUL(15) /* high impedance > turnaround */ > #define SPI_RX_CPHA_FLIP _BITUL(16) /* flip CPHA on Rx only xfer > */ > +#define SPI_MOSI_IDLE_LOW _BITUL(17) /* leave mosi line low when > idle */ > > /* > * All the bits defined above should be covered by SPI_MODE_USER_MASK. > @@ -37,6 +38,6 @@ > * These bits must not overlap. A static assert check should make sure of > that. > * If adding extra bits, make sure to increase the bit index below as well. > */ > -#define SPI_MODE_USER_MASK (_BITUL(17) - 1) > +#define SPI_MODE_USER_MASK (_BITUL(18) - 1) > > #endif /* _UAPI_SPI_H */ > -- > 2.25.1
Am Mo., 15. Mai 2023 um 08:37 Uhr schrieb Mahapatra, Amit Kumar <amit.kumar-mahapatra@amd.com>: > > Hello, > > > -----Original Message----- > > From: Boerge Struempfel <boerge.struempfel@gmail.com> > > Sent: Thursday, May 11, 2023 7:27 PM > > Cc: boerge.struempfel@gmail.com; bstruempfel@ultratronik.de; Mark > > Brown <broonie@kernel.org>; Shawn Guo <shawnguo@kernel.org>; Sascha > > Hauer <s.hauer@pengutronix.de>; Pengutronix Kernel Team > > <kernel@pengutronix.de>; Fabio Estevam <festevam@gmail.com>; NXP > > Linux Team <linux-imx@nxp.com>; linux-spi@vger.kernel.org; linux-arm- > > kernel@lists.infradead.org; linux-kernel@vger.kernel.org > > Subject: [PATCH] spi: Add option to keep the MOSI line low, when it is idle. > > > > CAUTION: This message has originated from an External Source. Please use > > proper judgment and caution when opening attachments, clicking links, or > > responding to this email. > > > > > > By default, the imx spi controller uses a high mosi line, whenever it is idle. > > This may not be desired in all use cases. For example neopixel leds can get > > confused and flicker due to misinterpreting the idle state. > > Therefore, we introduce a new spi-mode bit, with which the idle behaviour > > can be overwritten on a per device basis. > > > > Signed-off-by: Boerge Struempfel <bstruempfel@ultratronik.de> > > --- > > drivers/spi/spi-imx.c | 9 ++++++++- > > drivers/spi/spi.c | 2 ++ > > include/uapi/linux/spi/spi.h | 3 ++- > > 3 files changed, 12 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c index > > 34e5f81ec431e..6acab2b4ffaa5 100644 > > --- a/drivers/spi/spi-imx.c > > +++ b/drivers/spi/spi-imx.c > > @@ -281,6 +281,7 @@ static bool spi_imx_can_dma(struct spi_controller > > *controller, struct spi_device #define MX51_ECSPI_CONFIG_SCLKPOL(cs) (1 > > << ((cs & 3) + 4)) #define MX51_ECSPI_CONFIG_SBBCTRL(cs) (1 << ((cs & 3) + > > 8)) > > #define MX51_ECSPI_CONFIG_SSBPOL(cs) (1 << ((cs & 3) + 12)) > > +#define MX51_ECSPI_CONFIG_DATACTL(cs) (1 << ((cs & 3) + 16)) > > #define MX51_ECSPI_CONFIG_SCLKCTL(cs) (1 << ((cs & 3) + 20)) > > > > #define MX51_ECSPI_INT 0x10 > > @@ -573,6 +574,11 @@ static int mx51_ecspi_prepare_message(struct > > spi_imx_data *spi_imx, > > cfg &= ~MX51_ECSPI_CONFIG_SCLKCTL(spi_get_chipselect(spi, 0)); > > } > > > > + if (spi->mode & SPI_MOSI_IDLE_LOW) > > + cfg |= MX51_ECSPI_CONFIG_DATACTL(spi->chip_select); > > Kindly replace all occurrence of spi->chip_select with spi_get_chipselect(spi, 0) > https://github.com/torvalds/linux/commit/9e264f3f85a56cc109cc2d6010a48aa89d5c1ff1 Thank you very much for noticing this. I have changed it for the next version of the patch. > > > + else > > + cfg &= ~MX51_ECSPI_CONFIG_DATACTL(spi->chip_select); > > > + > > if (spi->mode & SPI_CS_HIGH) > > cfg |= MX51_ECSPI_CONFIG_SSBPOL(spi_get_chipselect(spi, 0)); > > else > > @@ -1743,7 +1749,8 @@ static int spi_imx_probe(struct platform_device > > *pdev) > > spi_imx->controller->prepare_message = spi_imx_prepare_message; > > spi_imx->controller->unprepare_message = > > spi_imx_unprepare_message; > > spi_imx->controller->slave_abort = spi_imx_slave_abort; > > - spi_imx->controller->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH | > > SPI_NO_CS; > > + spi_imx->controller->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH > > | SPI_NO_CS | > > + SPI_MOSI_IDLE_LOW; > > > > if (is_imx35_cspi(spi_imx) || is_imx51_ecspi(spi_imx) || > > is_imx53_ecspi(spi_imx)) > > diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index > > 9291b2a0e8871..3ad538b317a84 100644 > > --- a/drivers/spi/spi.c > > +++ b/drivers/spi/spi.c > > @@ -2260,6 +2260,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_property_read_bool(nc, "spi-mosi-idle-low")) > > + spi->mode |= SPI_MOSI_IDLE_LOW; > > > > /* Device DUAL/QUAD mode */ > > if (!of_property_read_u32(nc, "spi-tx-bus-width", &value)) { diff --git > > a/include/uapi/linux/spi/spi.h b/include/uapi/linux/spi/spi.h index > > 9d5f580597039..ca56e477d1619 100644 > > --- a/include/uapi/linux/spi/spi.h > > +++ b/include/uapi/linux/spi/spi.h > > @@ -28,6 +28,7 @@ > > #define SPI_RX_OCTAL _BITUL(14) /* receive with 8 wires */ > > #define SPI_3WIRE_HIZ _BITUL(15) /* high impedance > > turnaround */ > > #define SPI_RX_CPHA_FLIP _BITUL(16) /* flip CPHA on Rx only xfer > > */ > > +#define SPI_MOSI_IDLE_LOW _BITUL(17) /* leave mosi line low when > > idle */ > > > > /* > > * All the bits defined above should be covered by SPI_MODE_USER_MASK. > > @@ -37,6 +38,6 @@ > > * These bits must not overlap. A static assert check should make sure of > > that. > > * If adding extra bits, make sure to increase the bit index below as well. > > */ > > -#define SPI_MODE_USER_MASK (_BITUL(17) - 1) > > +#define SPI_MODE_USER_MASK (_BITUL(18) - 1) > > > > #endif /* _UAPI_SPI_H */ > > -- > > 2.25.1 >
diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c index 34e5f81ec431e..6acab2b4ffaa5 100644 --- a/drivers/spi/spi-imx.c +++ b/drivers/spi/spi-imx.c @@ -281,6 +281,7 @@ static bool spi_imx_can_dma(struct spi_controller *controller, struct spi_device #define MX51_ECSPI_CONFIG_SCLKPOL(cs) (1 << ((cs & 3) + 4)) #define MX51_ECSPI_CONFIG_SBBCTRL(cs) (1 << ((cs & 3) + 8)) #define MX51_ECSPI_CONFIG_SSBPOL(cs) (1 << ((cs & 3) + 12)) +#define MX51_ECSPI_CONFIG_DATACTL(cs) (1 << ((cs & 3) + 16)) #define MX51_ECSPI_CONFIG_SCLKCTL(cs) (1 << ((cs & 3) + 20)) #define MX51_ECSPI_INT 0x10 @@ -573,6 +574,11 @@ static int mx51_ecspi_prepare_message(struct spi_imx_data *spi_imx, cfg &= ~MX51_ECSPI_CONFIG_SCLKCTL(spi_get_chipselect(spi, 0)); } + if (spi->mode & SPI_MOSI_IDLE_LOW) + cfg |= MX51_ECSPI_CONFIG_DATACTL(spi->chip_select); + else + cfg &= ~MX51_ECSPI_CONFIG_DATACTL(spi->chip_select); + if (spi->mode & SPI_CS_HIGH) cfg |= MX51_ECSPI_CONFIG_SSBPOL(spi_get_chipselect(spi, 0)); else @@ -1743,7 +1749,8 @@ static int spi_imx_probe(struct platform_device *pdev) spi_imx->controller->prepare_message = spi_imx_prepare_message; spi_imx->controller->unprepare_message = spi_imx_unprepare_message; spi_imx->controller->slave_abort = spi_imx_slave_abort; - spi_imx->controller->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH | SPI_NO_CS; + spi_imx->controller->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH | SPI_NO_CS | + SPI_MOSI_IDLE_LOW; if (is_imx35_cspi(spi_imx) || is_imx51_ecspi(spi_imx) || is_imx53_ecspi(spi_imx)) diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index 9291b2a0e8871..3ad538b317a84 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -2260,6 +2260,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_property_read_bool(nc, "spi-mosi-idle-low")) + spi->mode |= SPI_MOSI_IDLE_LOW; /* Device DUAL/QUAD mode */ if (!of_property_read_u32(nc, "spi-tx-bus-width", &value)) { diff --git a/include/uapi/linux/spi/spi.h b/include/uapi/linux/spi/spi.h index 9d5f580597039..ca56e477d1619 100644 --- a/include/uapi/linux/spi/spi.h +++ b/include/uapi/linux/spi/spi.h @@ -28,6 +28,7 @@ #define SPI_RX_OCTAL _BITUL(14) /* receive with 8 wires */ #define SPI_3WIRE_HIZ _BITUL(15) /* high impedance turnaround */ #define SPI_RX_CPHA_FLIP _BITUL(16) /* flip CPHA on Rx only xfer */ +#define SPI_MOSI_IDLE_LOW _BITUL(17) /* leave mosi line low when idle */ /* * All the bits defined above should be covered by SPI_MODE_USER_MASK. @@ -37,6 +38,6 @@ * These bits must not overlap. A static assert check should make sure of that. * If adding extra bits, make sure to increase the bit index below as well. */ -#define SPI_MODE_USER_MASK (_BITUL(17) - 1) +#define SPI_MODE_USER_MASK (_BITUL(18) - 1) #endif /* _UAPI_SPI_H */
By default, the imx spi controller uses a high mosi line, whenever it is idle. This may not be desired in all use cases. For example neopixel leds can get confused and flicker due to misinterpreting the idle state. Therefore, we introduce a new spi-mode bit, with which the idle behaviour can be overwritten on a per device basis. Signed-off-by: Boerge Struempfel <bstruempfel@ultratronik.de> --- drivers/spi/spi-imx.c | 9 ++++++++- drivers/spi/spi.c | 2 ++ include/uapi/linux/spi/spi.h | 3 ++- 3 files changed, 12 insertions(+), 2 deletions(-)