linux-kernel - RE: [PATCH] spi: Add option to keep the MOSI line low, when it is idle.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Mon, 15 May 2023 06:37:22 +0000
From:   "Mahapatra, Amit Kumar" <amit.kumar-mahapatra@....com>
To:     Boerge Struempfel <boerge.struempfel@...il.com>
CC:     "bstruempfel@...ratronik.de" <bstruempfel@...ratronik.de>,
        Mark Brown <broonie@...nel.org>,
        Shawn Guo <shawnguo@...nel.org>,
        Sascha Hauer <s.hauer@...gutronix.de>,
        Pengutronix Kernel Team <kernel@...gutronix.de>,
        Fabio Estevam <festevam@...il.com>,
        NXP Linux Team <linux-imx@....com>,
        "linux-spi@...r.kernel.org" <linux-spi@...r.kernel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH] spi: Add option to keep the MOSI line low, when it is
 idle.

Hello,

> -----Original Message-----
> From: Boerge Struempfel <boerge.struempfel@...il.com>
> Sent: Thursday, May 11, 2023 7:27 PM
> Cc: boerge.struempfel@...il.com; bstruempfel@...ratronik.de; Mark
> Brown <broonie@...nel.org>; Shawn Guo <shawnguo@...nel.org>; Sascha
> Hauer <s.hauer@...gutronix.de>; Pengutronix Kernel Team
> <kernel@...gutronix.de>; Fabio Estevam <festevam@...il.com>; NXP
> Linux Team <linux-imx@....com>; linux-spi@...r.kernel.org; linux-arm-
> kernel@...ts.infradead.org; linux-kernel@...r.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@...ratronik.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