| Message ID | 3bed61807fff6268789e7d411412fbc5cd6ffe2a.1607507863.git.hns@goldelico.com |
|---|---|
| State | Accepted |
| Commit | 2fee9583198eb97b5351feda7bd825e0f778385c |
| Headers | show |
| Series | spi: dt-bindings: clarify CS behavior for spi-cs-high and gpio descriptors | expand |
On Wed, Dec 09, 2020 at 12:36:40PM -0500, Sven Van Asbroeck wrote: > On Wed, Dec 9, 2020 at 4:57 AM H. Nikolaus Schaller <hns@goldelico.com> wrote: > > + device node | cs-gpio | CS pin state active | Note > > + ================+===============+=====================+===== > > + spi-cs-high | - | H | > > + - | - | L | > > + spi-cs-high | ACTIVE_HIGH | H | > > + - | ACTIVE_HIGH | L | 1 > > + spi-cs-high | ACTIVE_LOW | H | 2 > > + - | ACTIVE_LOW | L | > > + > Doesn't this table simply say: > - specify 'spi-cs-high' for an active-high chip select > - leave out 'spi-cs-high' for an active-low chip select > - the gpio active high/active low consumer flags are ignored > ? It seems to, yes. > If so, then I would simply document it that way. > Simple is beautiful. Yeah, it'd definitely be easier to read and clearer what people should actually do. As Linus said it'd also be a good idea to explicitly say that this is not great design or particularly intentional since it could be pretty confusing for someone trying to understand why the bindings are the way they are. I'm going to apply this anyway to make sure we get this documentated but some incremental improvements along these lines would be good.
On Fri, Dec 11, 2020 at 8:18 AM Mark Brown <broonie@kernel.org> wrote: > > Yeah, it'd definitely be easier to read and clearer what people should > actually do. I think it would be beneficial if this consisted of two very clearly separated parts: 1. the actual recommended binding - so people writing new devicetrees know what to do 2. the legacy bindings which "also work", which is important to know when working with legacy devicetrees At least, that's what I would want if I put myself in a user's shoes.
On Wed, 9 Dec 2020 10:57:44 +0100, H. Nikolaus Schaller wrote: > Behavior of CS signal in combination of spi-cs-high and gpio descriptors > is not clearly defined and documented. So clarify the documentation Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next Thanks! [1/1] spi: dt-bindings: clarify CS behavior for spi-cs-high and gpio descriptors commit: 2fee9583198eb97b5351feda7bd825e0f778385c All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark
diff --git a/Documentation/devicetree/bindings/spi/spi-controller.yaml b/Documentation/devicetree/bindings/spi/spi-controller.yaml index 1b56d5e40f1fc..5f505810104dd 100644 --- a/Documentation/devicetree/bindings/spi/spi-controller.yaml +++ b/Documentation/devicetree/bindings/spi/spi-controller.yaml @@ -42,6 +42,33 @@ properties: cs2 : &gpio1 1 0 cs3 : &gpio1 2 0 + The second flag of a gpio descriptor can be GPIO_ACTIVE_HIGH (0) + or GPIO_ACTIVE_LOW(1). Legacy device trees often use 0. + + There is a special rule set for combining the second flag of an + cs-gpio with the optional spi-cs-high flag for SPI slaves. + + Each table entry defines how the CS pin is to be physically + driven (not considering potential gpio inversions by pinmux): + + device node | cs-gpio | CS pin state active | Note + ================+===============+=====================+===== + spi-cs-high | - | H | + - | - | L | + spi-cs-high | ACTIVE_HIGH | H | + - | ACTIVE_HIGH | L | 1 + spi-cs-high | ACTIVE_LOW | H | 2 + - | ACTIVE_LOW | L | + + Notes: + 1) Should print a warning about polarity inversion. + Here it would be wise to avoid and define the gpio as + ACTIVE_LOW. + 2) Should print a warning about polarity inversion + because ACTIVE_LOW is overridden by spi-cs-high. + Should be generally avoided and be replaced by + spi-cs-high + ACTIVE_HIGH. + num-cs: $ref: /schemas/types.yaml#/definitions/uint32 description:
Behavior of CS signal in combination of spi-cs-high and gpio descriptors is not clearly defined and documented. So clarify the documentation Cc: linus.walleij@linaro.org Cc: linux-gpio@vger.kernel.org Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com> --- .../bindings/spi/spi-controller.yaml | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+)