From: Tinghan Shen <tinghan.shen@mediatek.com>
To: Rob Herring <robh@kernel.org>
Cc: <linus.walleij@linaro.org>, <matthias.bgg@gmail.com>,
<broonie@kernel.org>, <bgolaszewski@baylibre.com>,
<sean.wang@mediatek.com>, <bayi.cheng@mediatek.com>,
<gch981213@gmail.com>, <devicetree@vger.kernel.org>,
<linux-kernel@vger.kernel.org>, <linux-gpio@vger.kernel.org>,
<linux-arm-kernel@lists.infradead.org>,
<linux-mediatek@lists.infradead.org>, <linux-spi@vger.kernel.org>,
<Project_Global_Chrome_Upstream_Group@mediatek.com>
Subject: Re: [PATCH v6 3/4] dt-bindings: pinctrl: mt8195: add 'pins' wrapping node
Date: Thu, 16 Dec 2021 10:43:49 +0800 [thread overview]
Message-ID: <a0f7493a991137809645d5a85ce26846ffbe4769.camel@mediatek.com> (raw)
In-Reply-To: <YbjIhaJgxtx9aSU6@robh.at.kernel.org>
Hi Rob,
On Tue, 2021-12-14 at 10:38 -0600, Rob Herring wrote:
> On Sun, Dec 12, 2021 at 04:40:13AM +0800, Tinghan Shen wrote:
> > The mt8195 pinctrl driver requires a 'pins' subnode to wrap pin
> > properties.
>
> Why?
It's because the dtbs_check reports following errors,
/proj/mtk15399/cros/src/third_party/kernel/v5.10/arch/arm64/boot/dts/me
diatek/mt8195-evb.dt.yaml: pinctrl@10005000: i2c0-pins: 'pinmux' is a
required property
From schema:
/proj/mtk15399/cros/src/third_party/kernel/v5.10/Documentation/devicetr
ee/bindings/pinctrl/pinctrl-mt8195.yaml
/proj/mtk15399/cros/src/third_party/kernel/v5.10/arch/arm64/boot/dts/me
diatek/mt8195-evb.dt.yaml: pinctrl@10005000: i2c0-pins: 'pins_bus' does
not match any of the regexes: 'pinctrl-[0-9]+'
From schema:
/proj/mtk15399/cros/src/third_party/kernel/v5.10/Documentation/devicetr
ee/bindings/pinctrl/pinctrl-mt8195.yaml
So, I've to add 'pins' to fix this failure.
>
> > Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
> > ---
> > .../bindings/pinctrl/pinctrl-mt8195.yaml | 307 +++++++++-----
> > ----
> > 1 file changed, 160 insertions(+), 147 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-
> > mt8195.yaml b/Documentation/devicetree/bindings/pinctrl/pinctrl-
> > mt8195.yaml
> > index 5e2bb2bf3a55..2035bf164740 100644
> > --- a/Documentation/devicetree/bindings/pinctrl/pinctrl-mt8195.yaml
> > +++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-mt8195.yaml
> > @@ -59,133 +59,143 @@ properties:
> > patternProperties:
> > '-pins$':
> > type: object
> > - description: |
> > - A pinctrl node should contain at least one subnodes
> > representing the
> > - pinctrl groups available on the machine. Each subnode will
> > list the
> > - pins it needs, and how they should be configured, with
> > regard to muxer
> > - configuration, pullups, drive strength, input enable/disable
> > and
> > - input schmitt.
> > - An example of using macro:
> > - pincontroller {
> > - /* GPIO0 set as multifunction GPIO0 */
> > - gpio_pin {
> > - pinmux = <PINMUX_GPIO0__FUNC_GPIO0>;
> > - };
> > - /* GPIO8 set as multifunction SDA0 */
> > - i2c0_pin {
> > - pinmux = <PINMUX_GPIO8__FUNC_SDA0>;
> > - };
> > - };
> > - $ref: "pinmux-node.yaml"
> > -
> > - properties:
> > - pinmux:
> > - description: |
> > - Integer array, represents gpio pin number and mux
> > setting.
> > - Supported pin number and mux varies for different SoCs,
> > and are defined
> > - as macros in dt-bindings/pinctrl/<soc>-pinfunc.h
> > directly.
> > -
> > - drive-strength:
> > - enum: [2, 4, 6, 8, 10, 12, 14, 16]
> > -
> > - bias-pull-down:
> > - description: |
> > - For pull down type is normal, it don't need add RSEL &
> > R1R0 define
> > - and resistance value.
> > - For pull down type is PUPD/R0/R1 type, it can add R1R0
> > define to
> > - set different resistance. It can support
> > "MTK_PUPD_SET_R1R0_00" &
> > - "MTK_PUPD_SET_R1R0_01" & "MTK_PUPD_SET_R1R0_10" &
> > "MTK_PUPD_SET_R1R0_11"
> > - define in mt8195.
> > - For pull down type is RSEL, it can add RSEL define &
> > resistance value(ohm)
> > - to set different resistance by identifying property
> > "mediatek,rsel_resistance_in_si_unit".
> > - It can support "MTK_PULL_SET_RSEL_000" &
> > "MTK_PULL_SET_RSEL_001"
> > - & "MTK_PULL_SET_RSEL_010" & "MTK_PULL_SET_RSEL_011" &
> > "MTK_PULL_SET_RSEL_100"
> > - & "MTK_PULL_SET_RSEL_101" & "MTK_PULL_SET_RSEL_110" &
> > "MTK_PULL_SET_RSEL_111"
> > - define in mt8195. It can also support resistance
> > value(ohm) "75000" & "5000" in mt8195.
> > - oneOf:
> > - - enum: [100, 101, 102, 103]
> > - - description: mt8195 pull down PUPD/R0/R1 type define
> > value.
> > - - enum: [200, 201, 202, 203, 204, 205, 206, 207]
> > - - description: mt8195 pull down RSEL type define
> > value.
> > - - enum: [75000, 5000]
> > - - description: mt8195 pull down RSEL type si unit
> > value(ohm).
> > -
> > - An example of using RSEL define:
> > - pincontroller {
> > - i2c0_pin {
> > - pinmux = <PINMUX_GPIO8__FUNC_SDA0>;
> > - bias-pull-down = <MTK_PULL_SET_RSEL_001>;
> > - };
> > - };
> > - An example of using si unit resistance value(ohm):
> > - &pio {
> > - mediatek,rsel_resistance_in_si_unit;
> > - }
> > - pincontroller {
> > - i2c0_pin {
> > - pinmux = <PINMUX_GPIO8__FUNC_SDA0>;
> > - bias-pull-down = <75000>;
> > - };
> > - };
> > -
> > - bias-pull-up:
> > + additionalProperties: false
> > + patternProperties:
> > + 'pins':
>
> Can we constrain this a bit more.
After discussed with colleagues working on pinctrl,
'^pins' is fine for them. is this acceptable?
>
> > + type: object
> > + additionalProperties: false
> > description: |
> > - For pull up type is normal, it don't need add RSEL &
> > R1R0 define
> > - and resistance value.
> > - For pull up type is PUPD/R0/R1 type, it can add R1R0
> > define to
> > - set different resistance. It can support
> > "MTK_PUPD_SET_R1R0_00" &
> > - "MTK_PUPD_SET_R1R0_01" & "MTK_PUPD_SET_R1R0_10" &
> > "MTK_PUPD_SET_R1R0_11"
> > - define in mt8195.
> > - For pull up type is RSEL, it can add RSEL define &
> > resistance value(ohm)
> > - to set different resistance by identifying property
> > "mediatek,rsel_resistance_in_si_unit".
> > - It can support "MTK_PULL_SET_RSEL_000" &
> > "MTK_PULL_SET_RSEL_001"
> > - & "MTK_PULL_SET_RSEL_010" & "MTK_PULL_SET_RSEL_011" &
> > "MTK_PULL_SET_RSEL_100"
> > - & "MTK_PULL_SET_RSEL_101" & "MTK_PULL_SET_RSEL_110" &
> > "MTK_PULL_SET_RSEL_111"
> > - define in mt8195. It can also support resistance
> > value(ohm)
> > - "1000" & "1500" & "2000" & "3000" & "4000" & "5000" &
> > "10000" & "75000" in mt8195.
> > - oneOf:
> > - - enum: [100, 101, 102, 103]
> > - - description: mt8195 pull up PUPD/R0/R1 type define
> > value.
> > - - enum: [200, 201, 202, 203, 204, 205, 206, 207]
> > - - description: mt8195 pull up RSEL type define value.
> > - - enum: [1000, 1500, 2000, 3000, 4000, 5000, 10000,
> > 75000]
> > - - description: mt8195 pull up RSEL type si unit
> > value(ohm).
> > - An example of using RSEL define:
> > + A pinctrl node should contain at least one subnodes
> > representing the
> > + pinctrl groups available on the machine. Each subnode
> > will list the
> > + pins it needs, and how they should be configured, with
> > regard to muxer
> > + configuration, pullups, drive strength, input
> > enable/disable and
> > + input schmitt.
> > + An example of using macro:
> > pincontroller {
> > - i2c0_pin {
> > - pinmux = <PINMUX_GPIO8__FUNC_SDA0>;
> > - bias-pull-up = <MTK_PULL_SET_RSEL_001>;
> > + /* GPIO0 set as multifunction GPIO0 */
> > + gpio_pin {
> > + pinmux = <PINMUX_GPIO0__FUNC_GPIO0>;
> > };
> > - };
> > - An example of using si unit resistance value(ohm):
> > - &pio {
> > - mediatek,rsel_resistance_in_si_unit;
> > - }
> > - pincontroller {
> > + /* GPIO8 set as multifunction SDA0 */
> > i2c0_pin {
> > pinmux = <PINMUX_GPIO8__FUNC_SDA0>;
> > - bias-pull-up = <1000>;
> > };
> > };
> > -
> > - bias-disable: true
> > -
> > - output-high: true
> > -
> > - output-low: true
> > -
> > - input-enable: true
> > -
> > - input-disable: true
> > -
> > - input-schmitt-enable: true
> > -
> > - input-schmitt-disable: true
> > -
> > - required:
> > - - pinmux
> > -
> > - additionalProperties: false
> > + $ref: "pinmux-node.yaml"
> > +
> > + properties:
> > + pinmux:
> > + description: |
> > + Integer array, represents gpio pin number and mux
> > setting.
> > + Supported pin number and mux varies for different
> > SoCs, and are
> > + defined as macros in dt-bindings/pinctrl/<soc>-
> > pinfunc.h
> > + directly.
> > +
> > + drive-strength:
> > + enum: [2, 4, 6, 8, 10, 12, 14, 16]
> > +
> > + bias-pull-down:
> > + description: |
> > + For pull down type is normal, it don't need add RSEL
> > & R1R0 define
> > + and resistance value.
> > + For pull down type is PUPD/R0/R1 type, it can add
> > R1R0 define to
> > + set different resistance. It can support
> > "MTK_PUPD_SET_R1R0_00" &
> > + "MTK_PUPD_SET_R1R0_01" & "MTK_PUPD_SET_R1R0_10" &
> > + "MTK_PUPD_SET_R1R0_11" define in mt8195.
> > + For pull down type is RSEL, it can add RSEL define &
> > resistance
> > + value(ohm) to set different resistance by
> > identifying property
> > + "mediatek,rsel_resistance_in_si_unit".
> > + It can support "MTK_PULL_SET_RSEL_000" &
> > "MTK_PULL_SET_RSEL_001"
> > + & "MTK_PULL_SET_RSEL_010" & "MTK_PULL_SET_RSEL_011"
> > + & "MTK_PULL_SET_RSEL_100" & "MTK_PULL_SET_RSEL_101"
> > + & "MTK_PULL_SET_RSEL_110" & "MTK_PULL_SET_RSEL_111"
> > + define in mt8195. It can also support resistance
> > value(ohm)
> > + "75000" & "5000" in mt8195.
> > + oneOf:
> > + - enum: [100, 101, 102, 103]
> > + - description: mt8195 pull down PUPD/R0/R1 type
> > define value.
> > + - enum: [200, 201, 202, 203, 204, 205, 206, 207]
> > + - description: mt8195 pull down RSEL type define
> > value.
> > + - enum: [75000, 5000]
> > + - description: mt8195 pull down RSEL type si unit
> > value(ohm).
> > +
> > + An example of using RSEL define:
> > + pincontroller {
> > + i2c0_pin {
> > + pinmux = <PINMUX_GPIO8__FUNC_SDA0>;
> > + bias-pull-down = <MTK_PULL_SET_RSEL_001>;
>
> Isn't this now out of date?
>
> > + };
> > + };
> > + An example of using si unit resistance value(ohm):
> > + &pio {
> > + mediatek,rsel_resistance_in_si_unit;
> > + }
> > + pincontroller {
> > + i2c0_pin {
> > + pinmux = <PINMUX_GPIO8__FUNC_SDA0>;
> > + bias-pull-down = <75000>;
> > + };
> > + };
> > +
> > + bias-pull-up:
> > + description: |
> > + For pull up type is normal, it don't need add RSEL &
> > R1R0 define
> > + and resistance value.
> > + For pull up type is PUPD/R0/R1 type, it can add R1R0
> > define to
> > + set different resistance. It can support
> > "MTK_PUPD_SET_R1R0_00" &
> > + "MTK_PUPD_SET_R1R0_01" & "MTK_PUPD_SET_R1R0_10" &
> > + "MTK_PUPD_SET_R1R0_11" define in mt8195.
> > + For pull up type is RSEL, it can add RSEL define &
> > resistance
> > + value(ohm) to set different resistance by
> > identifying property
> > + "mediatek,rsel_resistance_in_si_unit".
> > + It can support "MTK_PULL_SET_RSEL_000" &
> > "MTK_PULL_SET_RSEL_001"
> > + & "MTK_PULL_SET_RSEL_010" & "MTK_PULL_SET_RSEL_011"
> > + & "MTK_PULL_SET_RSEL_100" & "MTK_PULL_SET_RSEL_101"
> > + & "MTK_PULL_SET_RSEL_110" & "MTK_PULL_SET_RSEL_111"
> > + define in mt8195. It can also support resistance
> > value(ohm)
> > + "1000" & "1500" & "2000" & "3000" & "4000" & "5000"
> > & "10000" &
> > + "75000" in mt8195.
> > + oneOf:
> > + - enum: [100, 101, 102, 103]
> > + - description: mt8195 pull up PUPD/R0/R1 type
> > define value.
> > + - enum: [200, 201, 202, 203, 204, 205, 206, 207]
> > + - description: mt8195 pull up RSEL type define
> > value.
> > + - enum: [1000, 1500, 2000, 3000, 4000, 5000,
> > 10000, 75000]
> > + - description: mt8195 pull up RSEL type si unit
> > value(ohm).
> > + An example of using RSEL define:
> > + pincontroller {
> > + i2c0_pin {
> > + pinmux = <PINMUX_GPIO8__FUNC_SDA0>;
> > + bias-pull-up = <MTK_PULL_SET_RSEL_001>;
> > + };
> > + };
> > + An example of using si unit resistance value(ohm):
> > + &pio {
> > + mediatek,rsel_resistance_in_si_unit;
> > + }
> > + pincontroller {
> > + i2c0_pin {
> > + pinmux = <PINMUX_GPIO8__FUNC_SDA0>;
> > + bias-pull-up = <1000>;
> > + };
> > + };
> > +
> > + bias-disable: true
> > +
> > + output-high: true
> > +
> > + output-low: true
> > +
> > + input-enable: true
> > +
> > + input-disable: true
> > +
> > + input-schmitt-enable: true
> > +
> > + input-schmitt-disable: true
> > +
> > + required:
> > + - pinmux
> >
> > required:
> > - compatible
> > @@ -201,30 +211,33 @@ additionalProperties: false
> >
> > examples:
> > - |
> > - #include <dt-bindings/pinctrl/mt8195-pinfunc.h>
> > - #include <dt-bindings/interrupt-controller/arm-gic.h>
> > - pio: pinctrl@10005000 {
> > - compatible = "mediatek,mt8195-pinctrl";
> > - reg = <0x10005000 0x1000>,
> > - <0x11d10000 0x1000>,
> > - <0x11d30000 0x1000>,
> > - <0x11d40000 0x1000>,
> > - <0x11e20000 0x1000>,
> > - <0x11eb0000 0x1000>,
> > - <0x11f40000 0x1000>,
> > - <0x1000b000 0x1000>;
> > - reg-names = "iocfg0", "iocfg_bm", "iocfg_bl",
> > - "iocfg_br", "iocfg_lm", "iocfg_rb",
> > - "iocfg_tl", "eint";
> > - gpio-controller;
> > - #gpio-cells = <2>;
> > - gpio-ranges = <&pio 0 0 144>;
> > - interrupt-controller;
> > - interrupts = <GIC_SPI 225 IRQ_TYPE_LEVEL_HIGH
> > 0>;
> > - #interrupt-cells = <2>;
> > -
> > - pio-pins {
> > - pinmux = <PINMUX_GPIO0__FUNC_GPIO0>;
> > - output-low;
> > - };
> > - };
> > + #include <dt-bindings/pinctrl/mt8195-pinfunc.h>
> > + #include <dt-bindings/interrupt-controller/arm-gic.h>
> > + #
> > + pio: pinctrl@10005000 {
> > + compatible = "mediatek,mt8195-pinctrl";
> > + reg = <0x10005000 0x1000>,
> > + <0x11d10000 0x1000>,
> > + <0x11d30000 0x1000>,
> > + <0x11d40000 0x1000>,
> > + <0x11e20000 0x1000>,
> > + <0x11eb0000 0x1000>,
> > + <0x11f40000 0x1000>,
> > + <0x1000b000 0x1000>;
> > + reg-names = "iocfg0", "iocfg_bm", "iocfg_bl",
> > + "iocfg_br", "iocfg_lm", "iocfg_rb",
> > + "iocfg_tl", "eint";
> > + gpio-controller;
> > + #gpio-cells = <2>;
> > + gpio-ranges = <&pio 0 0 144>;
> > + interrupt-controller;
> > + interrupts = <GIC_SPI 225 IRQ_TYPE_LEVEL_HIGH 0>;
> > + #interrupt-cells = <2>;
> > +
> > + pio-pins {
> > + pins_bus {
>
> Don't use '_' in node names.
>
> > + pinmux = <PINMUX_GPIO0__FUNC_GPIO0>;
> > + output-low;
> > + };
> > + };
> > + };
> > --
> > 2.18.0
> >
> >
I'll update the examples and node names at next version.
Thank you!
Best regards,
TingHan
next prev parent reply other threads:[~2021-12-16 2:43 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-12-11 20:40 [PATCH v6 0/4] Add basic SoC support for mediatek mt8195 Tinghan Shen
2021-12-11 20:40 ` [PATCH v6 1/4] dt-bindings: arm: mediatek: add mt8195 pericfg compatible Tinghan Shen
2021-12-11 20:40 ` [PATCH v6 2/4] dt-bindings: spi: add new clock name 'axi' for spi nor Tinghan Shen
2021-12-13 16:04 ` Mark Brown
2021-12-14 16:26 ` Rob Herring
2021-12-15 2:03 ` Tinghan Shen
2021-12-11 20:40 ` [PATCH v6 3/4] dt-bindings: pinctrl: mt8195: add 'pins' wrapping node Tinghan Shen
2021-12-14 16:38 ` Rob Herring
2021-12-16 2:43 ` Tinghan Shen [this message]
2021-12-11 20:40 ` [PATCH v6 4/4] arm64: dts: Add mediatek SoC mt8195 and evaluation board Tinghan Shen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=a0f7493a991137809645d5a85ce26846ffbe4769.camel@mediatek.com \
--to=tinghan.shen@mediatek.com \
--cc=Project_Global_Chrome_Upstream_Group@mediatek.com \
--cc=bayi.cheng@mediatek.com \
--cc=bgolaszewski@baylibre.com \
--cc=broonie@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=gch981213@gmail.com \
--cc=linus.walleij@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-gpio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mediatek@lists.infradead.org \
--cc=linux-spi@vger.kernel.org \
--cc=matthias.bgg@gmail.com \
--cc=robh@kernel.org \
--cc=sean.wang@mediatek.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).