From: Bjorn Andersson <bjorn.andersson@linaro.org>
To: satya priya <skakit@codeaurora.org>
Cc: Linus Walleij <linus.walleij@linaro.org>,
Rob Herring <robh+dt@kernel.org>, Andy Gross <agross@kernel.org>,
kgunda@codeaurora.org, linux-gpio@vger.kernel.org,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
linux-arm-msm@vger.kernel.org
Subject: Re: [PATCH V5 1/2] dt-bindings: pinctrl: qcom-pmic-gpio: Convert qcom pmic gpio bindings to YAML
Date: Tue, 13 Jul 2021 21:46:42 -0500 [thread overview]
Message-ID: <YO5QErHuGaPhU41k@yoga> (raw)
In-Reply-To: <1625032241-3458-2-git-send-email-skakit@codeaurora.org>
On Wed 30 Jun 00:50 CDT 2021, satya priya wrote:
> Convert Qualcomm PMIC GPIO bindings from .txt to .yaml format.
>
Thanks for updating this Satya, this is looking quite good now. Just got
one issue with the definition of the state child node.
> Signed-off-by: satya priya <skakit@codeaurora.org>
> Reviewed-by: Rob Herring <robh@kernel.org>
[..]
> diff --git a/Documentation/devicetree/bindings/pinctrl/qcom,pmic-gpio.yaml b/Documentation/devicetree/bindings/pinctrl/qcom,pmic-gpio.yaml
> new file mode 100644
> index 0000000..22c58de
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/qcom,pmic-gpio.yaml
> @@ -0,0 +1,259 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pinctrl/qcom,pmic-gpio.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm PMIC GPIO block
> +
> +maintainers:
> + - Bjorn Andersson <bjorn.andersson@linaro.org>
> +
> +description:
> + This binding describes the GPIO block(s) found in the 8xxx series of
> + PMIC's from Qualcomm.
> +
> +properties:
> + compatible:
> + items:
> + - enum:
> + - qcom,pm660-gpio
> + - qcom,pm660l-gpio
> + - qcom,pm6150-gpio
> + - qcom,pm6150l-gpio
> + - qcom,pm7325-gpio
> + - qcom,pm8005-gpio
> + - qcom,pm8008-gpio
> + - qcom,pm8018-gpio
> + - qcom,pm8038-gpio
> + - qcom,pm8058-gpio
> + - qcom,pm8150-gpio
> + - qcom,pm8150b-gpio
> + - qcom,pm8350-gpio
> + - qcom,pm8350b-gpio
> + - qcom,pm8350c-gpio
> + - qcom,pm8916-gpio
> + - qcom,pm8917-gpio
> + - qcom,pm8921-gpio
> + - qcom,pm8941-gpio
> + - qcom,pm8950-gpio
> + - qcom,pm8994-gpio
> + - qcom,pm8998-gpio
> + - qcom,pma8084-gpio
> + - qcom,pmi8950-gpio
> + - qcom,pmi8994-gpio
> + - qcom,pmi8998-gpio
> + - qcom,pmk8350-gpio
> + - qcom,pmr735a-gpio
> + - qcom,pmr735b-gpio
> + - qcom,pms405-gpio
> + - qcom,pmx55-gpio
> +
> + - enum:
> + - qcom,spmi-gpio
> + - qcom,ssbi-gpio
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + minItems: 1
> + maxItems: 44
> + description: |
> + Must contain an array of encoded interrupt specifiers for
> + each available GPIO
> +
> + '#interrupt-cells':
> + const: 2
> +
> + interrupt-controller: true
> +
> + gpio-controller: true
> +
> + gpio-ranges:
> + maxItems: 1
> +
> + '#gpio-cells':
> + const: 2
> + description: |
Not need for the '|', as the formatting isn't significant.
> + The first cell will be used to define gpio number and the
> + second denotes the flags for this gpio
> +
> +additionalProperties: false
> +
> +required:
> + - compatible
> + - reg
> + - gpio-controller
> + - '#gpio-cells'
> + - gpio-ranges
> +
> +patternProperties:
> + '.*':
I would prefer this match to follow tlmm and go '-state$'
> + anyOf:
Either we want the immediate child node to match gpio-pinctrl-state
or we want one of more children matching gpio-pinctrl-state, but not
both. So "oneOf".
> + - $ref: "pinmux-node.yaml"
> + - $ref: "pincfg-node.yaml"
The generic definition is not sufficient, you want this to be
- $ref: "#/$defs/gpio-pinctrl-state"
> + - patternProperties:
> + ".*":
The subnodes of the state can be named whatever, so this (the .*) is
good.
> + $ref: "#/$defs/gpio-pinctrl-state"
> +
> +$defs:
> + gpio-pinctrl-state:
This is too generic, how about qcom-pmic-gpio-state?
> + type: object
> + anyOf:
I have this as "allOf" in the TLMM binding, not entirely sure what the
implications of anyOf here would be though...
> + - $ref: "pinmux-node.yaml"
> + - $ref: "pincfg-node.yaml"
> + properties:
> + pins:
> + description: |
> + List of gpio pins affected by the properties specified in
> + this subnode. Valid pins are
> + - gpio1-gpio10 for pm6150
> + - gpio1-gpio12 for pm6150l
> + - gpio1-gpio10 for pm7325
> + - gpio1-gpio4 for pm8005
> + - gpio1-gpio2 for pm8008
> + - gpio1-gpio6 for pm8018
> + - gpio1-gpio12 for pm8038
> + - gpio1-gpio40 for pm8058
> + - gpio1-gpio10 for pm8150 (holes on gpio2, gpio5,
> + gpio7 and gpio8)
> + - gpio1-gpio12 for pm8150b (holes on gpio3, gpio4
> + and gpio7)
> + - gpio1-gpio12 for pm8150l (hole on gpio7)
> + - gpio1-gpio4 for pm8916
> + - gpio1-gpio10 for pm8350
> + - gpio1-gpio8 for pm8350b
> + - gpio1-gpio9 for pm8350c
> + - gpio1-gpio38 for pm8917
> + - gpio1-gpio44 for pm8921
> + - gpio1-gpio36 for pm8941
> + - gpio1-gpio8 for pm8950 (hole on gpio3)
> + - gpio1-gpio22 for pm8994
> + - gpio1-gpio26 for pm8998
> + - gpio1-gpio22 for pma8084
> + - gpio1-gpio2 for pmi8950
> + - gpio1-gpio10 for pmi8994
> + - gpio1-gpio4 for pmk8350
> + - gpio1-gpio4 for pmr735a
> + - gpio1-gpio4 for pmr735b
> + - gpio1-gpio12 for pms405 (holes on gpio1, gpio9
> + and gpio10)
> + - gpio1-gpio11 for pmx55 (holes on gpio3, gpio7, gpio10
> + and gpio11)
> +
> + items:
> + pattern: "^gpio([0-9]+)$"
> +
> + function:
> + items:
> + - enum:
> + - normal
> + - paired
> + - func1
> + - func2
> + - dtest1
> + - dtest2
> + - dtest3
> + - dtest4
> + - func3 # supported by LV/MV GPIO subtypes
> + - func4 # supported by LV/MV GPIO subtypes
> +
> + bias-disable: true
> + bias-pull-down: true
> + bias-pull-up: true
> +
> + qcom,pull-up-strength:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description: |
> + Specifies the strength to use for pull up, if selected.
> + Valid values are defined in
> + <dt-bindings/pinctrl/qcom,pmic-gpio.h>
> + If this property is omitted 30uA strength will be used
> + if pull up is selected
enum: [0, 1, 2, 3]
> +
> + bias-high-impedance: true
> + input-enable: true
> + output-high: true
> + output-low: true
> + power-source: true
> +
> + qcom,drive-strength:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description: |
> + Selects the drive strength for the specified pins
> + Valid drive strength values are defined in
> + <dt-bindings/pinctrl/qcom,pmic-gpio.h>
> + enum: [0, 1, 2, 3]
> +
> + drive-push-pull: true
> + drive-open-drain: true
> + drive-open-source: true
> +
> + qcom,analog-pass:
> + $ref: /schemas/types.yaml#/definitions/flag
> + description: |
> + The specified pins are configured in
> + analog-pass-through mode.
> +
> + qcom,atest:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description: |
> + Selects ATEST rail to route to GPIO when it's
> + configured in analog-pass-through mode.
> + enum: [1, 2, 3, 4]
> +
> + qcom,dtest-buffer:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description: |
> + Selects DTEST rail to route to GPIO when it's
> + configured as digital input.
> + enum: [1, 2, 3, 4]
> +
> + required:
> + - pins
> + - function
> +
> + additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/pinctrl/qcom,pmic-gpio.h>
> +
> + pm8921_gpio: gpio@150 {
> + compatible = "qcom,pm8921-gpio", "qcom,ssbi-gpio";
> + reg = <0x150 0x160>;
> + interrupts = <192 1>, <193 1>, <194 1>,
> + <195 1>, <196 1>, <197 1>,
> + <198 1>, <199 1>, <200 1>,
> + <201 1>, <202 1>, <203 1>,
> + <204 1>, <205 1>, <206 1>,
> + <207 1>, <208 1>, <209 1>,
> + <210 1>, <211 1>, <212 1>,
> + <213 1>, <214 1>, <215 1>,
> + <216 1>, <217 1>, <218 1>,
> + <219 1>, <220 1>, <221 1>,
> + <222 1>, <223 1>, <224 1>,
> + <225 1>, <226 1>, <227 1>,
> + <228 1>, <229 1>, <230 1>,
> + <231 1>, <232 1>, <233 1>,
> + <234 1>, <235 1>;
> +
> + gpio-controller;
> + gpio-ranges = <&pm8921_gpio 0 0 44>;
> + #gpio-cells = <2>;
> +
> + pm8921_gpio_keys: gpio-keys {
Per my above request of changing the pattern, this would have to be
"gpio-keys-state" instead.
Regards,
bjorn
> + volume-keys {
> + pins = "gpio20", "gpio21";
> + function = "normal";
> +
> + input-enable;
> + bias-pull-up;
> + drive-push-pull;
> + qcom,drive-strength = <PMIC_GPIO_STRENGTH_NO>;
> + power-source = <PM8921_GPIO_S4>;
> + };
> + };
> + };
> +...
> --
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
> of Code Aurora Forum, hosted by The Linux Foundation
>
next prev parent reply other threads:[~2021-07-14 2:46 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-30 5:50 [PATCH V5 0/2] Convert qcom pmic gpio bindings to YAML satya priya
2021-06-30 5:50 ` [PATCH V5 1/2] dt-bindings: pinctrl: qcom-pmic-gpio: " satya priya
2021-07-14 2:46 ` Bjorn Andersson [this message]
2021-07-19 10:15 ` skakit
2021-06-30 5:50 ` [PATCH V5 2/2] dt-bindings: pinctrl: qcom-pmic-gpio: Remove the interrupts property satya priya
2021-07-14 2:15 ` Rob Herring
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=YO5QErHuGaPhU41k@yoga \
--to=bjorn.andersson@linaro.org \
--cc=agross@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=kgunda@codeaurora.org \
--cc=linus.walleij@linaro.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-gpio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=robh+dt@kernel.org \
--cc=skakit@codeaurora.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.