mbox series

[v2,0/4] spi: Add option to keep the MOSI line low, when it is idle

Message ID 20230511231317.158214-1-bstruempfel@ultratronik.de
Headers show
Series spi: Add option to keep the MOSI line low, when it is idle | expand

Message

Boerge Struempfel May 11, 2023, 11:13 p.m. UTC
Some spi controller like the imx spi controller switch the mosi line to
high, whenever they are 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.

Changes from V1:
  - Added patch, introducing the new devicetree binding flag
  - Split the generic spi part of the patch from the imx-spi specific
    part
  - Replaced SPI_CPOL and SPI_CPHA by the combined SPI_MODE_X_MASK bit
    in the imx-spi.c modebits.
  - Added the SPI_MOSI_IDLE_LOW bit to spidev

Boerge Struempfel (4):
  spi: dt-bindings: Introduce spi-mosi-idle-low flag
  spi: add SPI_MOSI_IDLE_LOW mode bit
  spi: spi-imx: add support for SPI_MOSI_IDLE_LOW mode bit
  spi: spidev: add SPI_MOSI_IDLE_LOW mode bit

 .../devicetree/bindings/spi/spi-peripheral-props.yaml    | 6 ++++++
 drivers/spi/spi-imx.c                                    | 9 ++++++++-
 drivers/spi/spi.c                                        | 2 ++
 drivers/spi/spidev.c                                     | 2 +-
 include/uapi/linux/spi/spi.h                             | 3 ++-
 5 files changed, 19 insertions(+), 3 deletions(-)

Comments

Fabio Estevam May 12, 2023, 12:51 a.m. UTC | #1
On Thu, May 11, 2023 at 8:14 PM Boerge Struempfel
<boerge.struempfel@gmail.com> wrote:
>
> Some spi controller switch the mosi line to high, whenever they are
> 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>

Please run checkpatch on all the patches.

The following issue reported by checkpatch needs to be fixed:

WARNING: From:/Signed-off-by: email address mismatch: 'From: Boerge
Struempfel <boerge.struempfel@gmail.com>' != 'Signed-off-by: Boerge
Struempfel <bstruempfel@ultratronik.de>'
Mark Brown May 12, 2023, 1:17 a.m. UTC | #2
On Fri, May 12, 2023 at 01:13:13AM +0200, Boerge Struempfel wrote:
> Some spi controller like the imx spi controller switch the mosi line to
> high, whenever they are 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.

Please don't send new patches in reply to old patches or serieses, this
makes it harder for both people and tools to understand what is going
on - it can bury things in mailboxes and make it difficult to keep track
of what current patches are, both for the new patches and the old ones.
Mark Brown May 12, 2023, 3:30 a.m. UTC | #3
On Fri, May 12, 2023 at 01:13:14AM +0200, Boerge Struempfel wrote:
> Some spi controller switch the mosi line to high, whenever they are
> 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>
> ---
>  .../devicetree/bindings/spi/spi-peripheral-props.yaml       | 6 ++++++

If this is always required for a given device (which I'd expect to be
the case) why configure it through DT?  I know we've got some legacy
stuff like that but not all legacy DT choices were good and no need to
continue the pattern.
Boerge Struempfel May 12, 2023, 6:54 a.m. UTC | #4
Am Fr., 12. Mai 2023 um 05:30 Uhr schrieb Mark Brown <broonie@kernel.org>:
>
> On Fri, May 12, 2023 at 01:13:14AM +0200, Boerge Struempfel wrote:
> > Some spi controller switch the mosi line to high, whenever they are
> > 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>
> > ---
> >  .../devicetree/bindings/spi/spi-peripheral-props.yaml       | 6 ++++++
>
> If this is always required for a given device (which I'd expect to be
> the case) why configure it through DT?  I know we've got some legacy
> stuff like that but not all legacy DT choices were good and no need to
> continue the pattern.

Yes this will always be the case for specific spi-device, spi-controller
combinations. Just to make sure, that I understand your suggestion
correctly: You propose to check from the specific spi-device-driver, if
the spi-controller supports this particular mode-bit, and then set it if
it does and thereby loose the need for the DT entry completely?
Mark Brown May 15, 2023, 12:34 a.m. UTC | #5
On Fri, May 12, 2023 at 08:54:19AM +0200, Börge Strümpfel wrote:
> Am Fr., 12. Mai 2023 um 05:30 Uhr schrieb Mark Brown <broonie@kernel.org>:

> > If this is always required for a given device (which I'd expect to be
> > the case) why configure it through DT?  I know we've got some legacy
> > stuff like that but not all legacy DT choices were good and no need to
> > continue the pattern.

> Yes this will always be the case for specific spi-device, spi-controller
> combinations. Just to make sure, that I understand your suggestion
> correctly: You propose to check from the specific spi-device-driver, if
> the spi-controller supports this particular mode-bit, and then set it if
> it does and thereby loose the need for the DT entry completely?

Yes, we shouldn't need DT here.  Though the device should just be
setting this unconditionally if it's always required.