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 <[email protected]>
---
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);
+ 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
Hi Boerge,
On Thu, May 11, 2023 at 10:58 AM Boerge Struempfel
<[email protected]> 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
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(-)
--
2.25.1
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 <[email protected]>
---
.../devicetree/bindings/spi/spi-peripheral-props.yaml | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
index 782a014b63a76..46d5acc1fc232 100644
--- a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
+++ b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
@@ -34,6 +34,12 @@ properties:
description:
The device requires the chip select active high.
+ spi-mosi-idle-low:
+ $ref: /schemas/types.yaml#/definitions/flag
+ description:
+ The device requires the mosi line to be low when idle and the
+ chip select is asserted.
+
spi-lsb-first:
$ref: /schemas/types.yaml#/definitions/flag
description:
--
2.25.1
On Thu, May 11, 2023 at 8:14 PM Boerge Struempfel
<[email protected]> 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 <[email protected]>
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 <[email protected]>' != 'Signed-off-by: Boerge
Struempfel <[email protected]>'
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.
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 <[email protected]>
> ---
> .../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.
Am Fr., 12. Mai 2023 um 05:30 Uhr schrieb Mark Brown <[email protected]>:
>
> 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 <[email protected]>
> > ---
> > .../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?
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 <[email protected]>:
> > 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.
Hello,
> -----Original Message-----
> From: Boerge Struempfel <[email protected]>
> Sent: Thursday, May 11, 2023 7:27 PM
> Cc: [email protected]; [email protected]; Mark
> Brown <[email protected]>; Shawn Guo <[email protected]>; Sascha
> Hauer <[email protected]>; Pengutronix Kernel Team
> <[email protected]>; Fabio Estevam <[email protected]>; NXP
> Linux Team <[email protected]>; [email protected]; linux-arm-
> [email protected]; [email protected]
> 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 <[email protected]>
> ---
> 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
<[email protected]>:
>
> Hello,
>
> > -----Original Message-----
> > From: Boerge Struempfel <[email protected]>
> > Sent: Thursday, May 11, 2023 7:27 PM
> > Cc: [email protected]; [email protected]; Mark
> > Brown <[email protected]>; Shawn Guo <[email protected]>; Sascha
> > Hauer <[email protected]>; Pengutronix Kernel Team
> > <[email protected]>; Fabio Estevam <[email protected]>; NXP
> > Linux Team <[email protected]>; [email protected]; linux-arm-
> > [email protected]; [email protected]
> > 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 <[email protected]>
> > ---
> > 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
>
Thank you for your feedback
Am Mo., 15. Mai 2023 um 02:34 Uhr schrieb Mark Brown <[email protected]>:
>
> 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 <[email protected]>:
>
> > > 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.
I agree with you, that we should not need DT here. I will remove the
dt-binding in the next patch version.
However I am not so sure about setting it unconditionally, since this
is dependent on the spi-controller. Not all spi-controller show this
behavior, that they use a high mosi line in idle mode and have the
ability to change this. As far as I know, another common behavior
is that the mosi just keeps the last state which it transmitted. In this
case, devices like Neopixel would still work without this mode bit.
On Wed, May 17, 2023 at 10:26:07AM +0200, B?rge Str?mpfel wrote:
> However I am not so sure about setting it unconditionally, since this
> is dependent on the spi-controller. Not all spi-controller show this
> behavior, that they use a high mosi line in idle mode and have the
> ability to change this. As far as I know, another common behavior
> is that the mosi just keeps the last state which it transmitted. In this
> case, devices like Neopixel would still work without this mode bit.
The behaviour the device needs is that the device have a low MOSI, how
that is achieved is immaterial.