| Message ID | 20210530161333.3996-2-maukka@ext.kapsi.fi |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | [v4,1/2] dt-bindings: gpio-mux-input: add documentation | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/dt-meta-schema | fail | build log |
On Sun, May 30, 2021 at 6:16 PM Mauri Sandberg <maukka@ext.kapsi.fi> wrote: > Add documentation for a general GPIO multiplexer. > > Signed-off-by: Mauri Sandberg <maukka@ext.kapsi.fi> > Tested-by: Drew Fustini <drew@beagleboard.org> > Reviewed-by: Drew Fustini <drew@beagleboard.org> Overall very interesting! > + pin-gpios: > + description: | > + The GPIO pin used as the output from the multiplexer This is not a good name. "pin" what pin? Choose a descriptive name, like: parent-gpios multiplexed-gpios cascaded-gpios fanout-gpios (My order of preference.) Otherwise these bindings look good. Yours, Linus Walleij
On Sun, May 30, 2021 at 6:16 PM Mauri Sandberg <maukka@ext.kapsi.fi> wrote: > Add documentation for a general GPIO multiplexer. > > Signed-off-by: Mauri Sandberg <maukka@ext.kapsi.fi> > Tested-by: Drew Fustini <drew@beagleboard.org> > Reviewed-by: Drew Fustini <drew@beagleboard.org> After some thinking I realized these bindings should not be restricted to just input. There exist electronic constructions such as open drain that would make it possible to mux also outputs. > .../bindings/gpio/gpio-mux-input.yaml | 75 +++++++++++++++++++ Rename it just gpio-mux.yaml > +$id: http://devicetree.org/schemas/gpio/gpio-mux-input.yaml# Also here > +title: Generic GPIO input multiplexer Generic GPIO multiplexer > +description: | > + A generic GPIO based input multiplexer Not just input > + This driver uses a mux-controller to drive the multiplexer and has a single > + output pin for reading the inputs to the mux. Make this clearer and do not mention "driver". Here is a suggestion: This hardware construction multiplexes (cascades) several GPIO lines from one-to-many using a software controlled multiplexer. The most common use case is probably reading several inputs by switching the multiplexer over several input lines, which in practice works well since input lines has high impedance. Constructions with multiplexed outputs are also possible using open drain electronics. > + For GPIO consumer documentation see gpio.txt. No need to mention this I think, not your problem :D > + pin-gpios: I still want this renamed like in my previous mail. Hope all is clear! Yours, Linus Walleij
On Sun, 30 May 2021 19:13:32 +0300, Mauri Sandberg wrote: > Add documentation for a general GPIO multiplexer. > > Signed-off-by: Mauri Sandberg <maukka@ext.kapsi.fi> > Tested-by: Drew Fustini <drew@beagleboard.org> > Reviewed-by: Drew Fustini <drew@beagleboard.org> > --- > v3 -> v4: > - Changed author email > - Included Tested-by and Reviewed-by from Drew > v2 -> v3: added a complete example on dual 4-way multiplexer > v1 -> v2: added a little bit more text in the binding documenation > --- > .../bindings/gpio/gpio-mux-input.yaml | 75 +++++++++++++++++++ > 1 file changed, 75 insertions(+) > create mode 100644 Documentation/devicetree/bindings/gpio/gpio-mux-input.yaml > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: dtschema/dtc warnings/errors: Documentation/devicetree/bindings/gpio/gpio-mux-input.example.dt.yaml:0:0: /example-0/mux-controller: failed to match any schema with compatible: ['gpio-mux'] /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/gpio/gpio-mux-input.example.dt.yaml: key-mux1: 'mux-controls' does not match any of the regexes: 'pinctrl-[0-9]+' From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/gpio/gpio-mux-input.yaml /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/gpio/gpio-mux-input.example.dt.yaml: key-mux2: 'mux-controls' does not match any of the regexes: 'pinctrl-[0-9]+' From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/gpio/gpio-mux-input.yaml See https://patchwork.ozlabs.org/patch/1485492 This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit.
On 1.6.2021 13.44, Linus Walleij wrote: > On Sun, May 30, 2021 at 6:16 PM Mauri Sandberg <maukka@ext.kapsi.fi> wrote: > >> Add documentation for a general GPIO multiplexer. >> >> Signed-off-by: Mauri Sandberg <maukka@ext.kapsi.fi> >> Tested-by: Drew Fustini <drew@beagleboard.org> >> Reviewed-by: Drew Fustini <drew@beagleboard.org> > After some thinking I realized these bindings should not > be restricted to just input. There exist electronic constructions > such as open drain that would make it possible to mux also > outputs. > >> .../bindings/gpio/gpio-mux-input.yaml | 75 +++++++++++++++++++ > Rename it just gpio-mux.yaml > >> +$id: http://devicetree.org/schemas/gpio/gpio-mux-input.yaml# > Also here > >> +title: Generic GPIO input multiplexer > Generic GPIO multiplexer > >> +description: | >> + A generic GPIO based input multiplexer > Not just input > >> + This driver uses a mux-controller to drive the multiplexer and has a single >> + output pin for reading the inputs to the mux. > Make this clearer and do not mention "driver". > Here is a suggestion: > > This hardware construction multiplexes (cascades) several GPIO > lines from one-to-many using a software controlled multiplexer. > The most common use case is probably reading several inputs > by switching the multiplexer over several input lines, which in > practice works well since input lines has high impedance. > > Constructions with multiplexed outputs are also possible using > open drain electronics. > >> + For GPIO consumer documentation see gpio.txt. > No need to mention this I think, not your problem :D > >> + pin-gpios: > I still want this renamed like in my previous mail. > > Hope all is clear! > > Yours, > Linus Walleij Hi and thanks the comments. Generally I agree with everything you noted above and elsewhere and will make changes accordingly. But there is a small detail that needs to be sorted out. The name 'gpio-mux' has already been taken by 'mux-gpio' driver [2] [3]. Should we look for another name for this driver and it's bindings or refactor the mux-gpio's bindings first? I would be inclined to do the latter as the config symbol for mux-gpio is the same way around, MUX_GPIO. The bindings for mux-gpio need to be converted to .yaml anyhow and maybe the issues with the schema that Rob pointed out elsewhere would go away too. Otherwise I cannot really say what's wrong as the errors look unrelated to me. -- Mauri [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/mux/gpio-mux.txt [3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/mux/gpio.c
Hi Mauri, On Wed, Jun 2, 2021 at 11:31 AM Mauri Sandberg <maukka@ext.kapsi.fi> wrote: > But there is a small detail that needs to be sorted out. > The name 'gpio-mux' > has already been taken by 'mux-gpio' driver [2] [3]. What about "gpio-multiplexer"? It is not good that the thing using GPIOs to do multiplexing has take a name that seem to infer that GPIOs are being multiplexed. Now we can't do much about that we just have to live with it. How typical of formal languages to screw with the semantics of natural languages and create confusion... > Should we look for another name for this driver and it's bindings or > refactor the mux-gpio's bindings > first? Bindings are etched in stone and cannot be changed. Unless we change them anyways. But generally we can't. > The bindings for mux-gpio need to be converted to .yaml anyhow Yeah just do it if you have the time, all conversions are appreciated. (Separate patch and work item though, don't know if you need to mix that with this work?) > and maybe > the issues with the schema > that Rob pointed out elsewhere would go away too. Otherwise I cannot > really say what's wrong as the > errors look unrelated to me. I don't know about these, tell Rob if you have issues and I might be able to pitch in, I write a fair amount of schema too. Yours, Linus Walleij
Hi Linus, On 2.6.2021 13.35, Linus Walleij wrote: > On Wed, Jun 2, 2021 at 11:31 AM Mauri Sandberg <maukka@ext.kapsi.fi> wrote: > >> But there is a small detail that needs to be sorted out. >> The name 'gpio-mux' >> has already been taken by 'mux-gpio' driver [2] [3]. > > What about "gpio-multiplexer"? > > It is not good that the thing using GPIOs to do multiplexing > has take a name that seem to infer that GPIOs are being > multiplexed. Now we can't do much about that we just have > to live with it. How typical of formal languages to screw > with the semantics of natural languages and create confusion... > I am afraid having 'gpio-mux' and 'gpio-multiplexer' would create too many what-were-they-thinking moments for any unfortunate reader so I would rather choose something else. Can we just call it 'gpio-cascade' without referral to the underlying mux? Maybe at somepoint in future something else could be used in its place too. -- Mauri
Hi Rob, On 1.6.2021 16.32, Rob Herring wrote: > On Sun, 30 May 2021 19:13:32 +0300, Mauri Sandberg wrote: >> Add documentation for a general GPIO multiplexer. >> >> Signed-off-by: Mauri Sandberg <maukka@ext.kapsi.fi> >> Tested-by: Drew Fustini <drew@beagleboard.org> >> Reviewed-by: Drew Fustini <drew@beagleboard.org> >> --- >> v3 -> v4: >> - Changed author email >> - Included Tested-by and Reviewed-by from Drew >> v2 -> v3: added a complete example on dual 4-way multiplexer >> v1 -> v2: added a little bit more text in the binding documenation >> --- >> .../bindings/gpio/gpio-mux-input.yaml | 75 +++++++++++++++++++ >> 1 file changed, 75 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/gpio/gpio-mux-input.yaml >> > > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' > on your patch (DT_CHECKER_FLAGS is new in v5.13): > > yamllint warnings/errors: > > dtschema/dtc warnings/errors: > Documentation/devicetree/bindings/gpio/gpio-mux-input.example.dt.yaml:0:0: /example-0/mux-controller: failed to match any schema with compatible: ['gpio-mux'] > /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/gpio/gpio-mux-input.example.dt.yaml: key-mux1: 'mux-controls' does not match any of the regexes: 'pinctrl-[0-9]+' > From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/gpio/gpio-mux-input.yaml > /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/gpio/gpio-mux-input.example.dt.yaml: key-mux2: 'mux-controls' does not match any of the regexes: 'pinctrl-[0-9]+' > From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/gpio/gpio-mux-input.yaml > These look like they could be caused by gpio-mux bindings [2], which this depends on, not being formulated in yaml. Should it be addressed before carrying on? Thanks, Mauri [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/mux/gpio-mux.txt
On Wed, Jun 2, 2021 at 1:21 PM Mauri Sandberg <maukka@ext.kapsi.fi> wrote: > Can we just call it 'gpio-cascade' > without referral to the underlying mux? Maybe at somepoint in future > something else could be used in its place too. That has a nice ring to it, go with that! Yours, Linus Walleij
diff --git a/Documentation/devicetree/bindings/gpio/gpio-mux-input.yaml b/Documentation/devicetree/bindings/gpio/gpio-mux-input.yaml new file mode 100644 index 000000000000..1ca4c3c8d64b --- /dev/null +++ b/Documentation/devicetree/bindings/gpio/gpio-mux-input.yaml @@ -0,0 +1,75 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/gpio/gpio-mux-input.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Generic GPIO input multiplexer + +maintainers: + - Mauri Sandberg <maukka@ext.kapsi.fi> + +description: | + A generic GPIO based input multiplexer + + This driver uses a mux-controller to drive the multiplexer and has a single + output pin for reading the inputs to the mux. + + For GPIO consumer documentation see gpio.txt. + +properties: + compatible: + enum: + - gpio-mux-input + + gpio-controller: true + + '#gpio-cells': + const: 2 + + pin-gpios: + description: | + The GPIO pin used as the output from the multiplexer + +required: + - compatible + - gpio-controller + - "#gpio-cells" + - pin-gpios + +additionalProperties: false + +examples: + - | + #include <dt-bindings/gpio/gpio.h> + mux: mux-controller { + compatible = "gpio-mux"; + #mux-control-cells = <0>; + + mux-gpios = <&gpio 9 GPIO_ACTIVE_HIGH>, + <&gpio 11 GPIO_ACTIVE_HIGH>; + }; + + gpio2: key-mux1 { + compatible = "gpio-mux-input"; + mux-controls = <&mux>; + + gpio-controller; + #gpio-cells = <2>; + + // GPIOs used by this node, mux pin + pin-gpios = <&gpio 12 GPIO_ACTIVE_HIGH>; + }; + + gpio3: key-mux2 { + compatible = "gpio-mux-input"; + mux-controls = <&mux>; + + gpio-controller; + #gpio-cells = <2>; + + // GPIOs used by this node, mux pin + pin-gpios = <&gpio 14 GPIO_ACTIVE_HIGH>; + }; + +...