On 06/10/2022 18:20, Neil Armstrong wrote: > Hi, > > On 06/10/2022 13:09, Krzysztof Kozlowski wrote: >> On 06/10/2022 11:57, Neil Armstrong wrote: >>> Convert the MDM9515 pinctrl bindings to dt-schema. >>> >>> Signed-off-by: Neil Armstrong <neil.armstrong@xxxxxxxxxx> >>> --- >>> .../bindings/pinctrl/qcom,mdm9615-pinctrl.txt | 161 --------------------- >>> .../bindings/pinctrl/qcom,mdm9615-pinctrl.yaml | 101 +++++++++++++ >>> 2 files changed, 101 insertions(+), 161 deletions(-) >>> >> >> Thank you for your patch. There is something to discuss/improve. >> >>> - }; >>> - }; >>> diff --git a/Documentation/devicetree/bindings/pinctrl/qcom,mdm9615-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/qcom,mdm9615-pinctrl.yaml >>> new file mode 100644 >>> index 000000000000..6a5966fc0098 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/pinctrl/qcom,mdm9615-pinctrl.yaml >>> @@ -0,0 +1,101 @@ >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>> +%YAML 1.2 >>> +--- >>> +$id: http://devicetree.org/schemas/pinctrl/qcom,mdm9615-pinctrl.yaml# >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>> + >>> +title: Qualcomm Technologies, Inc. MDM9615 TLMM block >>> + >>> +maintainers: >>> + - Bjorn Andersson <andersson@xxxxxxxxxx> >>> + >>> +description: | >> >> No need for | > > Ack > >> >>> + This binding describes the Top Level Mode Multiplexer block found in the >>> + MDM9615 platform. >> >> Instead: "Top Level Mode Multiplexer pin controller node in Qualcomm >> MDM9615 SoC." >> >> I see this pattern is coming from other file, so I will fix all of them. > > Ack, wil use this. > >> >>> + >>> +allOf: >>> + - $ref: "pinctrl.yaml#" >> >> Drop it, it's included from tlmm-common > > Ack > >> >>> + - $ref: /schemas/pinctrl/qcom,tlmm-common.yaml# >>> + >>> +properties: >>> + compatible: >>> + const: qcom,mdm9615-pinctrl >>> + >>> + reg: >>> + maxItems: 1 >>> + >>> + interrupts: true >>> + interrupt-controller: true >>> + '#interrupt-cells': true >>> + gpio-controller: true >>> + '#gpio-cells': true >>> + gpio-ranges: true >>> + >>> +required: >>> + - compatible >>> + - reg >>> + >>> +additionalProperties: false >>> + >>> +patternProperties: >>> + '-state$': >> >> Use " as quotes > > Ack > >> >>> + oneOf: >>> + - $ref: "#/$defs/qcom-mdm9615-pinctrl-state" >>> + - patternProperties: >>> + "-pins$": >>> + $ref: "#/$defs/qcom-mdm9615-pinctrl-state" > > Interesting, if I add this subnode (that should be valid): > gsbi3-state { > pins = "gpio8", "gpio9", "gpio10", "gpio11"; > function = "gsbi3"; > drive-strength = <8>; > bias-disable; > }; > > then I get the following warning from dt_bindings_check: First, your block for using defs/qcom-mdm9615-pinctrl-state is not exactly like it should be, e.g. it misses additionalProperties: false. Maybe it misses few things more. Please use one of my latest patches as example, e.g. https://lore.kernel.org/linux-devicetree/20221006144518.256956-1-krzysztof.kozlowski@xxxxxxxxxx/T/#t > > Documentation/devicetree/bindings/pinctrl/qcom,mdm9615-pinctrl.example.dtb: pinctrl@1000000: gsbi3-state: More than one condition true in oneOf schema: > {'oneOf': [{'$ref': '#/$defs/qcom-mdm9615-pinctrl-state'}, > {'patternProperties': {'-pins$': {'$ref': '#/$defs/qcom-mdm9615-pinctrl-state'}, > 'pinctrl-[0-9]+': True}, > 'properties': {'$nodename': True, > 'phandle': True, > 'pinctrl-names': True, > 'secure-status': True, > 'status': True}}]} > From schema: /Documentation/devicetree/bindings/pinctrl/qcom,mdm9615-pinctrl.yaml > > And I don't understand why, the nodename should not match "-pins$" nor "pinctrl-[0-9]+'... Hm, dunno... You have a trailing coma in enum with functions - maybe remove it? I looked briefly between your patch and mine for sm8150 and could not find differences except that additionalProperties and coma. Best regards, Krzysztof
