* [PATCH V3 0/2] dt-bindings: pinctrl: pins, groups & functions @ 2021-12-10 11:42 Rafał Miłecki 2021-12-10 11:42 ` [PATCH V3 1/2] dt-bindings: pinctrl: support specifying " Rafał Miłecki 2021-12-10 11:42 ` [PATCH V3 2/2] dt-bindings: pinctrl: brcm,ns-pinmux: describe " Rafał Miłecki 0 siblings, 2 replies; 10+ messages in thread From: Rafał Miłecki @ 2021-12-10 11:42 UTC (permalink / raw) To: Linus Walleij, Rob Herring Cc: Tony Lindgren, Andy Shevchenko, linux-gpio, devicetree, linux-arm-kernel, Florian Fainelli, bcm-kernel-feedback-list, Rafał Miłecki From: Rafał Miłecki <rafal@milecki.pl> This is my minimalized attempt of setting up pins, groups & functions bindings. I've been warned that Linux pinctrl subsystem may require refactoring before it's ready to handle such bindings properly and that appears to be correct. I'll need to invest more time into reorganizing Linux structs. Right now it's not ready for tree-like design and it uses more-or-less magic numbers to handle pins <-> groups relation. Meanwhile I'd like to get dt-bindings reviewed & possibly merged. To be honest - my initial reason for that work was developing U-Boot drivers. Please kindly review those bindings and optionally treat my WIP work on Linux implementation as proof of concept. Reference: RFC: https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20211110231436.8866-1-zajec5@gmail.com/ V1: https://patchwork.ozlabs.org/project/linux-gpio/list/?series=272685&submitter=&state=*&q=&archive=&delegate= V2: https://patchwork.ozlabs.org/project/linux-gpio/list/?series=273711&submitter=&state=*&q=&archive=&delegate= Rafał Miłecki (2): dt-bindings: pinctrl: support specifying pins, groups & functions dt-bindings: pinctrl: brcm,ns-pinmux: describe pins, groups & functions .../bindings/pinctrl/brcm,ns-pinmux.yaml | 38 ++++++++++++++++++- .../devicetree/bindings/pinctrl/pinctrl.yaml | 34 +++++++++++++++++ 2 files changed, 71 insertions(+), 1 deletion(-) -- 2.31.1 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH V3 1/2] dt-bindings: pinctrl: support specifying pins, groups & functions 2021-12-10 11:42 [PATCH V3 0/2] dt-bindings: pinctrl: pins, groups & functions Rafał Miłecki @ 2021-12-10 11:42 ` Rafał Miłecki 2021-12-10 23:26 ` Linus Walleij 2021-12-10 11:42 ` [PATCH V3 2/2] dt-bindings: pinctrl: brcm,ns-pinmux: describe " Rafał Miłecki 1 sibling, 1 reply; 10+ messages in thread From: Rafał Miłecki @ 2021-12-10 11:42 UTC (permalink / raw) To: Linus Walleij, Rob Herring Cc: Tony Lindgren, Andy Shevchenko, linux-gpio, devicetree, linux-arm-kernel, Florian Fainelli, bcm-kernel-feedback-list, Rafał Miłecki From: Rafał Miłecki <rafal@milecki.pl> This binding change is meant to introduce a generic way of describing pinctrl blocks details. Every pinmux block is expected to have: 1. Named pins 2. Named groups containing one or more pins 3. Named functions referencing one or more groups It doesn't describe how hw should be programmed. That remains binding and driver specific. This commit describes a common syntax for "pins", "groups" & "functions" nodes. Every node allows specifying its entries. That design is meant to be extendable and minimalistic enough to be generic (matching any hardware). Relations between pins, groups and functions are expected to be the same for every hardware. Using subnode objects allows extending this binding to cover hardware specific needs (e.g. custom values). Example to consider: pins { foo: foo { vendor,magic = <0xc0fe>; }; } groups { bar { pins = <&foo>; vendor,secret = <0xbeaf>; }; }; Signed-off-by: Rafał Miłecki <rafal@milecki.pl> --- .../devicetree/bindings/pinctrl/pinctrl.yaml | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/pinctrl.yaml index d471563119a9..e36662cb1f3b 100644 --- a/Documentation/devicetree/bindings/pinctrl/pinctrl.yaml +++ b/Documentation/devicetree/bindings/pinctrl/pinctrl.yaml @@ -42,4 +42,38 @@ properties: This property can be set either globally for the pin controller or in child nodes for individual pin group control. + pins: + type: object + + patternProperties: + "^.*$": + type: object + description: Pin named by node name + + groups: + type: object + + patternProperties: + "^.*$": + type: object + description: Group named by node name + + properties: + pins: + $ref: /schemas/types.yaml#/definitions/phandle-array + description: Array of pins belonging to this group + + functions: + type: object + + patternProperties: + "^.*$": + type: object + description: Function named by node name + + properties: + groups: + $ref: /schemas/types.yaml#/definitions/phandle-array + description: Array of groups used by this function + additionalProperties: true -- 2.31.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH V3 1/2] dt-bindings: pinctrl: support specifying pins, groups & functions 2021-12-10 11:42 ` [PATCH V3 1/2] dt-bindings: pinctrl: support specifying " Rafał Miłecki @ 2021-12-10 23:26 ` Linus Walleij 2021-12-11 11:16 ` Rafał Miłecki 0 siblings, 1 reply; 10+ messages in thread From: Linus Walleij @ 2021-12-10 23:26 UTC (permalink / raw) To: Rafał Miłecki Cc: Rob Herring, Tony Lindgren, Andy Shevchenko, linux-gpio, devicetree, linux-arm-kernel, Florian Fainelli, bcm-kernel-feedback-list, Rafał Miłecki On Fri, Dec 10, 2021 at 12:42 PM Rafał Miłecki <zajec5@gmail.com> wrote: > This binding change is meant to introduce a generic way of describing > pinctrl blocks details. Every pinmux block is expected to have: > 1. Named pins > 2. Named groups containing one or more pins > 3. Named functions referencing one or more groups > > It doesn't describe how hw should be programmed. That remains binding > and driver specific. So what this does is to take a large chunk of data that we known to be associated with the compatible string (names of pins, groups and functions, etc) and put it into the device tree instead of the alternative, which is what most drivers do, and that is to compile in the data into the operating system and just look it up by using a compatible string. The DT maintainers have already indicated that this is not desirable and I don't see it getting merged before it has a Reviewed-by tag from one of the DT binding maintainers. I think we need to know what the USP (unique selling point) is? Would it be something like not having to duplicate work across some boot loaders and operating systems? (Well they all need to parse this type of description but that can be put into a library.) Or something else? Yours, Linus Walleij ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH V3 1/2] dt-bindings: pinctrl: support specifying pins, groups & functions 2021-12-10 23:26 ` Linus Walleij @ 2021-12-11 11:16 ` Rafał Miłecki 2021-12-14 19:59 ` Rob Herring 0 siblings, 1 reply; 10+ messages in thread From: Rafał Miłecki @ 2021-12-11 11:16 UTC (permalink / raw) To: Linus Walleij, Rafał Miłecki, Rob Herring Cc: Tony Lindgren, Andy Shevchenko, linux-gpio, devicetree, linux-arm-kernel, Florian Fainelli, bcm-kernel-feedback-list Rob: please kindly comment on this idea of storing pins/groups/functions in DT. For a sample Linux implementation you can check (incomplete): [PATCH V2 4/6] pinctrl: support reading pins, groups & functions from DT https://patchwork.ozlabs.org/project/linux-gpio/patch/20211124230439.17531-5-zajec5@gmail.com/ For a real life DT usage you can check: [PATCH V2 6/6] ARM: dts: BCM5301X: add pinctrl pins, groups & functions https://patchwork.ozlabs.org/project/linux-gpio/patch/20211124230439.17531-7-zajec5@gmail.com/ Also see below inline comments. On 11.12.2021 00:26, Linus Walleij wrote: > On Fri, Dec 10, 2021 at 12:42 PM Rafał Miłecki <zajec5@gmail.com> wrote: > >> This binding change is meant to introduce a generic way of describing >> pinctrl blocks details. Every pinmux block is expected to have: >> 1. Named pins >> 2. Named groups containing one or more pins >> 3. Named functions referencing one or more groups >> >> It doesn't describe how hw should be programmed. That remains binding >> and driver specific. > > So what this does is to take a large chunk of data that we known to be > associated with the compatible string (names of pins, groups and functions, > etc) and put it into the device tree instead of the alternative, which is > what most drivers do, and that is to compile in the data into the > operating system and just look it up by using a compatible > string. Correct. It changes the place of storing platform specific data. > The DT maintainers have already indicated that this is not desirable > and I don't see it getting merged before it has a Reviewed-by > tag from one of the DT binding maintainers. Tony pointed out that it was back in 2011. It's worth reconsidering. https://patchwork.ozlabs.org/comment/2786915/ Rob said it depends on whether "data be static (complete) and correct" https://patchwork.ozlabs.org/comment/2786688/ I find it absolutely required to get Rob's Reviewed-by before we merge it. I hope we can get Rob's opinion on this. > I think we need to know what the USP (unique selling point) is? > > Would it be something like not having to duplicate work across some > boot loaders and operating systems? (Well they all need to parse this > type of description but that can be put into a library.) > > Or something else? There are two reasons for me to work on this binding: 1. I think it's cleaner to keep pinctrl details in DT DT seems more natural (than C code) for: a) Translating info from datasheets b) Storing pin/group/function custom values c) Defining relations (phandles) d) Handling chip differences (adding new pins in newer revisions) Last time I learnt that pins don't always/usually have numbers (in datasheets) but are rather named. Still in the "pinctrl_pin_desc" we have "unsigned number" just to enumerate them and reference in groups. Adding or removing pins/groups/functions in DT is as simple as adding/deleting nodes. That also means less logic in C code. 2. It avoids data duplication It allows me to keep pins/groups/functions data in one place (DT) and use it in both: Linux and U-Boot. No duplication & easier maintenance. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH V3 1/2] dt-bindings: pinctrl: support specifying pins, groups & functions 2021-12-11 11:16 ` Rafał Miłecki @ 2021-12-14 19:59 ` Rob Herring 2021-12-14 20:10 ` Rafał Miłecki 0 siblings, 1 reply; 10+ messages in thread From: Rob Herring @ 2021-12-14 19:59 UTC (permalink / raw) To: Rafał Miłecki Cc: Linus Walleij, Rafał Miłecki, Tony Lindgren, Andy Shevchenko, linux-gpio, devicetree, linux-arm-kernel, Florian Fainelli, bcm-kernel-feedback-list On Sat, Dec 11, 2021 at 12:16:25PM +0100, Rafał Miłecki wrote: > Rob: please kindly comment on this idea of storing pins/groups/functions > in DT. I was never a fan of stuffing pin mux/ctrl into DT for what's mostly a one time stuffing of register values. And given how many things run before getting to the kernel, doing proper pin configuration in the kernel is much too late (or redundant because it was actually already done). > > For a sample Linux implementation you can check (incomplete): > [PATCH V2 4/6] pinctrl: support reading pins, groups & functions from DT > https://patchwork.ozlabs.org/project/linux-gpio/patch/20211124230439.17531-5-zajec5@gmail.com/ > > For a real life DT usage you can check: > [PATCH V2 6/6] ARM: dts: BCM5301X: add pinctrl pins, groups & functions > https://patchwork.ozlabs.org/project/linux-gpio/patch/20211124230439.17531-7-zajec5@gmail.com/ What about h/w with no concept of 'groups'? > Also see below inline comments. > > > On 11.12.2021 00:26, Linus Walleij wrote: > > On Fri, Dec 10, 2021 at 12:42 PM Rafał Miłecki <zajec5@gmail.com> wrote: > > > > > This binding change is meant to introduce a generic way of describing > > > pinctrl blocks details. Every pinmux block is expected to have: > > > 1. Named pins > > > 2. Named groups containing one or more pins > > > 3. Named functions referencing one or more groups > > > > > > It doesn't describe how hw should be programmed. That remains binding > > > and driver specific. > > > > So what this does is to take a large chunk of data that we known to be > > associated with the compatible string (names of pins, groups and functions, > > etc) and put it into the device tree instead of the alternative, which is > > what most drivers do, and that is to compile in the data into the > > operating system and just look it up by using a compatible > > string. > > Correct. It changes the place of storing platform specific data. > > > > The DT maintainers have already indicated that this is not desirable > > and I don't see it getting merged before it has a Reviewed-by > > tag from one of the DT binding maintainers. > > Tony pointed out that it was back in 2011. It's worth reconsidering. > https://patchwork.ozlabs.org/comment/2786915/ > > Rob said it depends on whether "data be static (complete) and correct" > https://patchwork.ozlabs.org/comment/2786688/ I haven't seen an answer for that question... That and working for multiple platforms (from different vendors) are the main things that matter to me. > I find it absolutely required to get Rob's Reviewed-by before we merge > it. I hope we can get Rob's opinion on this. > > > > I think we need to know what the USP (unique selling point) is? > > > > Would it be something like not having to duplicate work across some > > boot loaders and operating systems? (Well they all need to parse this > > type of description but that can be put into a library.) > > > > Or something else? > > There are two reasons for me to work on this binding: > > > 1. I think it's cleaner to keep pinctrl details in DT > > DT seems more natural (than C code) for: > a) Translating info from datasheets > b) Storing pin/group/function custom values > c) Defining relations (phandles) > d) Handling chip differences (adding new pins in newer revisions) > > Last time I learnt that pins don't always/usually have numbers (in > datasheets) but are rather named. Still in the "pinctrl_pin_desc" we > have "unsigned number" just to enumerate them and reference in groups. > > Adding or removing pins/groups/functions in DT is as simple as > adding/deleting nodes. That also means less logic in C code. > > > 2. It avoids data duplication > > It allows me to keep pins/groups/functions data in one place (DT) and > use it in both: Linux and U-Boot. No duplication & easier maintenance. > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH V3 1/2] dt-bindings: pinctrl: support specifying pins, groups & functions 2021-12-14 19:59 ` Rob Herring @ 2021-12-14 20:10 ` Rafał Miłecki 2021-12-14 21:50 ` Rafał Miłecki 0 siblings, 1 reply; 10+ messages in thread From: Rafał Miłecki @ 2021-12-14 20:10 UTC (permalink / raw) To: Rob Herring, Rafał Miłecki Cc: Linus Walleij, Tony Lindgren, Andy Shevchenko, linux-gpio, devicetree, linux-arm-kernel, Florian Fainelli, bcm-kernel-feedback-list On 14.12.2021 20:59, Rob Herring wrote: > On Sat, Dec 11, 2021 at 12:16:25PM +0100, Rafał Miłecki wrote: >> Rob: please kindly comment on this idea of storing pins/groups/functions >> in DT. > > I was never a fan of stuffing pin mux/ctrl into DT for what's mostly a > one time stuffing of register values. And given how many things run > before getting to the kernel, doing proper pin configuration in the > kernel is much too late (or redundant because it was actually already > done). OK, thanks for sharing that. Given a pretty limited optimism on this approach I'll simply drop it and do things the old good way. I thought it's a better desing but I probably was wrong. It was still worth a try :) Thanks to everyone involved in this discussion. >> For a sample Linux implementation you can check (incomplete): >> [PATCH V2 4/6] pinctrl: support reading pins, groups & functions from DT >> https://patchwork.ozlabs.org/project/linux-gpio/patch/20211124230439.17531-5-zajec5@gmail.com/ >> >> For a real life DT usage you can check: >> [PATCH V2 6/6] ARM: dts: BCM5301X: add pinctrl pins, groups & functions >> https://patchwork.ozlabs.org/project/linux-gpio/patch/20211124230439.17531-7-zajec5@gmail.com/ > > What about h/w with no concept of 'groups'? It could probably be handled with sth like functions { bar { pins = <&foo>; } } but my binding didn't cover that indeed. >> Also see below inline comments. >> >> >> On 11.12.2021 00:26, Linus Walleij wrote: >>> On Fri, Dec 10, 2021 at 12:42 PM Rafał Miłecki <zajec5@gmail.com> wrote: >>> >>>> This binding change is meant to introduce a generic way of describing >>>> pinctrl blocks details. Every pinmux block is expected to have: >>>> 1. Named pins >>>> 2. Named groups containing one or more pins >>>> 3. Named functions referencing one or more groups >>>> >>>> It doesn't describe how hw should be programmed. That remains binding >>>> and driver specific. >>> >>> So what this does is to take a large chunk of data that we known to be >>> associated with the compatible string (names of pins, groups and functions, >>> etc) and put it into the device tree instead of the alternative, which is >>> what most drivers do, and that is to compile in the data into the >>> operating system and just look it up by using a compatible >>> string. >> >> Correct. It changes the place of storing platform specific data. >> >> >>> The DT maintainers have already indicated that this is not desirable >>> and I don't see it getting merged before it has a Reviewed-by >>> tag from one of the DT binding maintainers. >> >> Tony pointed out that it was back in 2011. It's worth reconsidering. >> https://patchwork.ozlabs.org/comment/2786915/ >> >> Rob said it depends on whether "data be static (complete) and correct" >> https://patchwork.ozlabs.org/comment/2786688/ > > I haven't seen an answer for that question... > > That and working for multiple platforms (from different vendors) are the > main things that matter to me. I thought my design description & BCM5301X DTS patch may be a proof of that but apparently it wasn't enough ;) ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH V3 1/2] dt-bindings: pinctrl: support specifying pins, groups & functions 2021-12-14 20:10 ` Rafał Miłecki @ 2021-12-14 21:50 ` Rafał Miłecki 0 siblings, 0 replies; 10+ messages in thread From: Rafał Miłecki @ 2021-12-14 21:50 UTC (permalink / raw) To: Rob Herring, Rafał Miłecki Cc: Linus Walleij, Tony Lindgren, Andy Shevchenko, linux-gpio, devicetree, linux-arm-kernel, Florian Fainelli, bcm-kernel-feedback-list On 14.12.2021 21:10, Rafał Miłecki wrote: > On 14.12.2021 20:59, Rob Herring wrote: >> On Sat, Dec 11, 2021 at 12:16:25PM +0100, Rafał Miłecki wrote: >>> Rob: please kindly comment on this idea of storing pins/groups/functions >>> in DT. >> >> I was never a fan of stuffing pin mux/ctrl into DT for what's mostly a >> one time stuffing of register values. And given how many things run >> before getting to the kernel, doing proper pin configuration in the >> kernel is much too late (or redundant because it was actually already >> done). > > OK, thanks for sharing that. Given a pretty limited optimism on this > approach I'll simply drop it and do things the old good way. I feel I need to post one more comment though. *** What I find a really clean DT code for defining some BCM4908 groups: groups { led_0_grp { pins = <&pin0 3>; }; led_1_grp { pins = <&pin1 3>; }; nand_grp { pins = <&pin32 0>, <&pin33 0>, <&pin34 0>, <&pin43 0>, <&pin44 0>, <&pin45 0>, <&pin56 1>; }; }; *** Gets a bit cumbersome (for me) when using ANSI C structs. I remain unconvinced about ANSI C being a good place for storing such data. Maybe I'm just getting too old & grumpy ;) struct bcm4908_pinctrl_pin_setup { unsigned number; unsigned function; }; static const struct bcm4908_pinctrl_pin_setup led_0_pins[] = { { 0, 3 }, }; static const struct bcm4908_pinctrl_pin_setup led_1_pins[] = { { 0, 3 }, }; static const struct bcm4908_pinctrl_pin_setup nand_pins[] = { { 32, 0 }, { 33, 0 }, { 34, 0 }, { 43, 0 }, { 44, 0 }, { 45, 0 }, { 56, 1 }, }; struct bcm4908_pinctrl_grp { const char *name; const struct bcm4908_pinctrl_pin_setup *pins; const unsigned int num_pins; }; static const struct bcm4908_pinctrl_grp bcm4908_pinctrl_grps[] = { { "led_0_grp", led_0_pins, ARRAY_SIZE(led_0_pins) }, { "led_1_grp", led_1_pins, ARRAY_SIZE(led_1_pins) }, { "nand_grp", nand_pins, ARRAY_SIZE(nand_pins) }, }; ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH V3 2/2] dt-bindings: pinctrl: brcm,ns-pinmux: describe pins, groups & functions 2021-12-10 11:42 [PATCH V3 0/2] dt-bindings: pinctrl: pins, groups & functions Rafał Miłecki 2021-12-10 11:42 ` [PATCH V3 1/2] dt-bindings: pinctrl: support specifying " Rafał Miłecki @ 2021-12-10 11:42 ` Rafał Miłecki 2021-12-10 14:02 ` Rob Herring 2021-12-11 7:07 ` Tony Lindgren 1 sibling, 2 replies; 10+ messages in thread From: Rafał Miłecki @ 2021-12-10 11:42 UTC (permalink / raw) To: Linus Walleij, Rob Herring Cc: Tony Lindgren, Andy Shevchenko, linux-gpio, devicetree, linux-arm-kernel, Florian Fainelli, bcm-kernel-feedback-list, Rafał Miłecki From: Rafał Miłecki <rafal@milecki.pl> Use and extend generic pinctrl binding to include info about pins, groups & functions. Northstar platform pins have numbers assigned to them (they are needed for mux programming) so add a custom "number" property for that. Extend example to provide a complete binding of a single / random function. Signed-off-by: Rafał Miłecki <rafal@milecki.pl> --- .../bindings/pinctrl/brcm,ns-pinmux.yaml | 38 ++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/pinctrl/brcm,ns-pinmux.yaml b/Documentation/devicetree/bindings/pinctrl/brcm,ns-pinmux.yaml index 8d1e5b1cdd5f..82d3e52a2229 100644 --- a/Documentation/devicetree/bindings/pinctrl/brcm,ns-pinmux.yaml +++ b/Documentation/devicetree/bindings/pinctrl/brcm,ns-pinmux.yaml @@ -30,6 +30,20 @@ properties: reg-names: const: cru_gpio_control + pins: + type: object + + patternProperties: + "^.*$": + type: object + + properties: + number: + description: Pin number + $ref: /schemas/types.yaml#/definitions/uint32 + + unevaluatedProperties: false + patternProperties: '-pins$': type: object @@ -74,7 +88,7 @@ required: - reg - reg-names -additionalProperties: false +unevaluatedProperties: false examples: - | @@ -83,6 +97,28 @@ examples: reg = <0x1800c1c0 0x24>; reg-names = "cru_gpio_control"; + pins { + i2c_scl: i2c_scl { + number = <4>; + }; + + i2c_sda: i2c_sda { + number = <5>; + }; + }; + + groups { + i2c_grp: i2c_grp { + pins = <&i2c_scl &i2c_sda>; + }; + }; + + functions { + i2c { + groups = <&i2c_grp>; + }; + }; + spi-pins { function = "spi"; groups = "spi_grp"; -- 2.31.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH V3 2/2] dt-bindings: pinctrl: brcm,ns-pinmux: describe pins, groups & functions 2021-12-10 11:42 ` [PATCH V3 2/2] dt-bindings: pinctrl: brcm,ns-pinmux: describe " Rafał Miłecki @ 2021-12-10 14:02 ` Rob Herring 2021-12-11 7:07 ` Tony Lindgren 1 sibling, 0 replies; 10+ messages in thread From: Rob Herring @ 2021-12-10 14:02 UTC (permalink / raw) To: Rafał Miłecki Cc: linux-gpio, Rafał Miłecki, Rob Herring, Linus Walleij, Florian Fainelli, linux-arm-kernel, devicetree, Tony Lindgren, bcm-kernel-feedback-list, Andy Shevchenko [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1: Type: text/plain, Size: 4803 bytes --] On Fri, 10 Dec 2021 12:42:22 +0100, Rafał Miłecki wrote: > From: Rafał Miłecki <rafal@milecki.pl> > > Use and extend generic pinctrl binding to include info about pins, > groups & functions. > > Northstar platform pins have numbers assigned to them (they are needed > for mux programming) so add a custom "number" property for that. > > Extend example to provide a complete binding of a single / random > function. > > Signed-off-by: Rafał Miłecki <rafal@milecki.pl> > --- > .../bindings/pinctrl/brcm,ns-pinmux.yaml | 38 ++++++++++++++++++- > 1 file changed, 37 insertions(+), 1 deletion(-) > 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: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/pinctrl/brcm,ns-pinmux.yaml: 'additionalProperties' is a required property hint: A schema without a "$ref" to another schema must define all properties and use "additionalProperties" from schema $id: http://devicetree.org/meta-schemas/base.yaml# schemas/pinctrl/brcm,ns-pinmux.yaml: ignoring, error in schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/pinctrl/brcm,ns-pinmux.yaml: ignoring, error in schema: warning: no schema found in file: ./Documentation/devicetree/bindings/pinctrl/brcm,ns-pinmux.yaml Documentation/devicetree/bindings/pinctrl/brcm,ns-pinmux.example.dt.yaml:0:0: /example-0/pin-controller@1800c1c0: failed to match any schema with compatible: ['brcm,bcm4708-pinmux'] make[1]: *** Deleting file 'Documentation/devicetree/bindings/mfd/brcm,cru.example.dt.yaml' schemas/pinctrl/brcm,ns-pinmux.yaml: ignoring, error in schema: Traceback (most recent call last): File "/usr/local/bin/dt-validate", line 170, in <module> sg.check_trees(filename, testtree) File "/usr/local/bin/dt-validate", line 119, in check_trees self.check_subtree(dt, subtree, False, "/", "/", filename) File "/usr/local/bin/dt-validate", line 110, in check_subtree self.check_subtree(tree, value, disabled, name, fullname + name, filename) File "/usr/local/bin/dt-validate", line 110, in check_subtree self.check_subtree(tree, value, disabled, name, fullname + name, filename) File "/usr/local/bin/dt-validate", line 105, in check_subtree self.check_node(tree, subtree, disabled, nodename, fullname, filename) File "/usr/local/bin/dt-validate", line 49, in check_node errors = sorted(dtschema.DTValidator(schema).iter_errors(node), key=lambda e: e.linecol) File "/usr/local/lib/python3.8/dist-packages/dtschema/lib.py", line 766, in iter_errors for error in super().iter_errors(instance, _schema): File "/usr/local/lib/python3.8/dist-packages/jsonschema/validators.py", line 224, in iter_errors for error in errors: File "/usr/local/lib/python3.8/dist-packages/jsonschema/_validators.py", line 25, in patternProperties yield from validator.descend( File "/usr/local/lib/python3.8/dist-packages/jsonschema/validators.py", line 240, in descend for error in self.evolve(schema=schema).iter_errors(instance): File "/usr/local/lib/python3.8/dist-packages/dtschema/lib.py", line 766, in iter_errors for error in super().iter_errors(instance, _schema): File "/usr/local/lib/python3.8/dist-packages/jsonschema/validators.py", line 224, in iter_errors for error in errors: File "/usr/local/lib/python3.8/dist-packages/jsonschema/_validators.py", line 298, in ref yield from validator.descend(instance, resolved) File "/usr/local/lib/python3.8/dist-packages/jsonschema/validators.py", line 240, in descend for error in self.evolve(schema=schema).iter_errors(instance): File "/usr/local/lib/python3.8/dist-packages/dtschema/lib.py", line 766, in iter_errors for error in super().iter_errors(instance, _schema): File "/usr/local/lib/python3.8/dist-packages/jsonschema/validators.py", line 214, in iter_errors scope = id_of(_schema) File "/usr/local/lib/python3.8/dist-packages/jsonschema/validators.py", line 90, in _id_of return schema.get("$id", "") AttributeError: 'NoneType' object has no attribute 'get' make[1]: *** [scripts/Makefile.lib:373: Documentation/devicetree/bindings/mfd/brcm,cru.example.dt.yaml] Error 1 make[1]: *** Waiting for unfinished jobs.... make: *** [Makefile:1413: dt_binding_check] Error 2 doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/patch/1566458 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. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH V3 2/2] dt-bindings: pinctrl: brcm,ns-pinmux: describe pins, groups & functions 2021-12-10 11:42 ` [PATCH V3 2/2] dt-bindings: pinctrl: brcm,ns-pinmux: describe " Rafał Miłecki 2021-12-10 14:02 ` Rob Herring @ 2021-12-11 7:07 ` Tony Lindgren 1 sibling, 0 replies; 10+ messages in thread From: Tony Lindgren @ 2021-12-11 7:07 UTC (permalink / raw) To: Rafał Miłecki Cc: Linus Walleij, Rob Herring, Andy Shevchenko, linux-gpio, devicetree, linux-arm-kernel, Florian Fainelli, bcm-kernel-feedback-list, Rafał Miłecki * Rafał Miłecki <zajec5@gmail.com> [211210 11:43]: > @@ -83,6 +97,28 @@ examples: > reg = <0x1800c1c0 0x24>; > reg-names = "cru_gpio_control"; > > + pins { > + i2c_scl: i2c_scl { > + number = <4>; > + }; > + > + i2c_sda: i2c_sda { > + number = <5>; > + }; > + }; Please don't add custom properties for something that can be done with standard register based addressing using a hardware offset based reg property. Your driver can easily translate it. Also, please don't use custom node names, instead do: pins { i2c_scl: pin@0x1234 { /* Any generic standard properties or numbers here please :)d */ }; ... }; I think I've already commented on the register addressing twice before.. Regards, Tony ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-12-14 21:50 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-12-10 11:42 [PATCH V3 0/2] dt-bindings: pinctrl: pins, groups & functions Rafał Miłecki 2021-12-10 11:42 ` [PATCH V3 1/2] dt-bindings: pinctrl: support specifying " Rafał Miłecki 2021-12-10 23:26 ` Linus Walleij 2021-12-11 11:16 ` Rafał Miłecki 2021-12-14 19:59 ` Rob Herring 2021-12-14 20:10 ` Rafał Miłecki 2021-12-14 21:50 ` Rafał Miłecki 2021-12-10 11:42 ` [PATCH V3 2/2] dt-bindings: pinctrl: brcm,ns-pinmux: describe " Rafał Miłecki 2021-12-10 14:02 ` Rob Herring 2021-12-11 7:07 ` Tony Lindgren
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).