Re: [PATCH] spi: Add option to keep the MOSI line low, when it is idle.

[Börge Strümpfel <boerge.struempfel@xxxxxxxxx>]

Am Mo., 15. Mai 2023 um 08:37 Uhr schrieb Mahapatra, Amit Kumar
<amit.kumar-mahapatra@xxxxxxx>:
>
> Hello,
>
> > -----Original Message-----
> > From: Boerge Struempfel <boerge.struempfel@xxxxxxxxx>
> > Sent: Thursday, May 11, 2023 7:27 PM
> > Cc: boerge.struempfel@xxxxxxxxx; bstruempfel@xxxxxxxxxxxxxx; Mark
> > Brown <broonie@xxxxxxxxxx>; Shawn Guo <shawnguo@xxxxxxxxxx>; Sascha
> > Hauer <s.hauer@xxxxxxxxxxxxxx>; Pengutronix Kernel Team
> > <kernel@xxxxxxxxxxxxxx>; Fabio Estevam <festevam@xxxxxxxxx>; NXP
> > Linux Team <linux-imx@xxxxxxx>; linux-spi@xxxxxxxxxxxxxxx; linux-arm-
> > kernel@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> > 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@xxxxxxxxxxxxxx>
> > ---
> >  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
>