| Message ID | 20230511231317.158214-2-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 |
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>'
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.
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?
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.
Thank you for your feedback Am Mo., 15. Mai 2023 um 02:34 Uhr schrieb Mark Brown <broonie@kernel.org>: > > 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. 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.
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:
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 ++++++ 1 file changed, 6 insertions(+)