2023-09-15 14:32:03

by Krzysztof Kozlowski

Subject: Re: [PATCH v2 1/2] dt-bindings: pinctrl: qcom: Add SM4450 pinctrl

On 15/09/2023 03:58, Tengfei Fan wrote:
> Add device tree binding Documentation details for Qualcomm SM4450
> TLMM device.
>
> Signed-off-by: Tengfei Fan <[email protected]>

...

> +
> +patternProperties:
> + "-state$":
> + oneOf:
> + - $ref: "#/$defs/qcom-sm4450-tlmm-state"
> + - patternProperties:
> + "-pins$":
> + $ref: "#/$defs/qcom-sm4450-tlmm-state"
> + additionalProperties: false
> +
> +$defs:
> + qcom-sm4450-tlmm-state:
> + type: object
> + description:
> + Pinctrl node's client devices use subnodes for desired pin configuration.
> + Client device subnodes use below standard properties.
> + $ref: qcom,tlmm-common.yaml#/$defs/qcom-tlmm-state
> + unevaluatedProperties: false
> +
> + properties:
> + pins:
> + description:
> + List of gpio pins affected by the properties specified in this
> + subnode.
> + items:
> + oneOf:
> + - pattern: "^gpio([0-9]|[1-9][0-9]|1[0-9][0-9])$"

This is still wrong. How many GPIOs do you have? Please open existing
bindings for recent device (e.g. sm8550) and look how it is done there.

...


> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> + tlmm: pinctrl@f100000 {
> + compatible = "qcom,sm4450-tlmm";
> + reg = <0x0f100000 0x300000>;
> + gpio-controller;
> + #gpio-cells = <2>;
> + gpio-ranges = <&tlmm 0 0 137>;
> + interrupt-controller;
> + #interrupt-cells = <2>;
> + interrupts = <GIC_SPI 208 IRQ_TYPE_LEVEL_HIGH>;

Missing example pieces. Again, please base your work on other recent files.

> + };
> +...

Best regards,
Krzysztof

2023-09-15 19:11:12

by Tengfei Fan

Subject: Re: [PATCH v2 1/2] dt-bindings: pinctrl: qcom: Add SM4450 pinctrl

在 9/15/2023 3:04 PM, Krzysztof Kozlowski 写道:
> On 15/09/2023 03:58, Tengfei Fan wrote:
>> Add device tree binding Documentation details for Qualcomm SM4450
>> TLMM device.
>>
>> Signed-off-by: Tengfei Fan <[email protected]>
>
> ...
>
>> +
>> +patternProperties:
>> + "-state$":
>> + oneOf:
>> + - $ref: "#/$defs/qcom-sm4450-tlmm-state"
>> + - patternProperties:
>> + "-pins$":
>> + $ref: "#/$defs/qcom-sm4450-tlmm-state"
>> + additionalProperties: false
>> +
>> +$defs:
>> + qcom-sm4450-tlmm-state:
>> + type: object
>> + description:
>> + Pinctrl node's client devices use subnodes for desired pin configuration.
>> + Client device subnodes use below standard properties.
>> + $ref: qcom,tlmm-common.yaml#/$defs/qcom-tlmm-state
>> + unevaluatedProperties: false
>> +
>> + properties:
>> + pins:
>> + description:
>> + List of gpio pins affected by the properties specified in this
>> + subnode.
>> + items:
>> + oneOf:
>> + - pattern: "^gpio([0-9]|[1-9][0-9]|1[0-9][0-9])$"
>
> This is still wrong. How many GPIOs do you have? Please open existing
> bindings for recent device (e.g. sm8550) and look how it is done there.
yes, will update to "^gpio([0-9]|[1-9][0-9]|1[0-2][0-5])$".
>
> ...
>
>
>> +
>> +examples:
>> + - |
>> + #include <dt-bindings/interrupt-controller/arm-gic.h>
>> + tlmm: pinctrl@f100000 {
>> + compatible = "qcom,sm4450-tlmm";
>> + reg = <0x0f100000 0x300000>;
>> + gpio-controller;
>> + #gpio-cells = <2>;
>> + gpio-ranges = <&tlmm 0 0 137>;
>> + interrupt-controller;
>> + #interrupt-cells = <2>;
>> + interrupts = <GIC_SPI 208 IRQ_TYPE_LEVEL_HIGH>;
>
> Missing example pieces. Again, please base your work on other recent files.
yes, will reference other platform, and update
>
>> + };
>> +...
>
> Best regards,
> Krzysztof
>

--
Thx and BRs,
Tengfei Fan