Re: [PATCH V5 1/2] dt-bindings: pinctrl: qcom-pmic-gpio: Convert qcom pmic gpio bindings to YAML

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2021-07-14 08:16, Bjorn Andersson wrote:
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.

[..]
+
+  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.


Okay.

+        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$'


okay.

+    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".


Okay.

+      - $ref: "pinmux-node.yaml"
+      - $ref: "pincfg-node.yaml"

The generic definition is not sufficient, you want this to be

	- $ref: "#/$defs/gpio-pinctrl-state"


Okay, I will add this ref and remove generic ones as we are already referring them in 'qcom-pmic-gpio-state' definition.

+      - 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?


ok.

+    type: object
+    anyOf:

I have this as "allOf" in the TLMM binding, not entirely sure what the
implications of anyOf here would be though...


Sure, will change it to allOf.

+      - $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]

okay.
+
+      bias-high-impedance: true

+
+      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.


okay.

Thanks,
Satya Priya

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




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux