[PATCH] dt-bindings: pinctrl: mt8192: Add mediatek,pull-down-adv property

Nícolas F. R. A. Prado posted 1 patch 2 years, 4 months ago
.../devicetree/bindings/pinctrl/pinctrl-mt8192.yaml   | 11 +++++++++++
1 file changed, 11 insertions(+)
[PATCH] dt-bindings: pinctrl: mt8192: Add mediatek,pull-down-adv property
Posted by Nícolas F. R. A. Prado 2 years, 4 months ago
Add the mediatek,pull-down-adv property to the pinctrl-mt8192 dt-binding
to allow configuring pull-down resistors on the pins of MT8192. It is
the same as in mt8183-pinctrl.

Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>

---

 .../devicetree/bindings/pinctrl/pinctrl-mt8192.yaml   | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-mt8192.yaml b/Documentation/devicetree/bindings/pinctrl/pinctrl-mt8192.yaml
index c90a132fbc79..e462f49eae6f 100644
--- a/Documentation/devicetree/bindings/pinctrl/pinctrl-mt8192.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-mt8192.yaml
@@ -117,6 +117,17 @@ patternProperties:
             $ref: /schemas/types.yaml#/definitions/uint32
             enum: [0, 1, 2, 3]
 
+          mediatek,pull-down-adv:
+            description: |
+              Pull down settings for 2 pull resistors, R0 and R1. User can
+              configure those special pins. Valid arguments are described as below:
+              0: (R1, R0) = (0, 0) which means R1 disabled and R0 disabled.
+              1: (R1, R0) = (0, 1) which means R1 disabled and R0 enabled.
+              2: (R1, R0) = (1, 0) which means R1 enabled and R0 disabled.
+              3: (R1, R0) = (1, 1) which means R1 enabled and R0 enabled.
+            $ref: /schemas/types.yaml#/definitions/uint32
+            enum: [0, 1, 2, 3]
+
           bias-pull-down: true
 
           bias-pull-up: true
-- 
2.36.0

Re: [PATCH] dt-bindings: pinctrl: mt8192: Add mediatek,pull-down-adv property
Posted by Krzysztof Kozlowski 2 years, 4 months ago
On 29/04/2022 22:06, Nícolas F. R. A. Prado wrote:
> Add the mediatek,pull-down-adv property to the pinctrl-mt8192 dt-binding
> to allow configuring pull-down resistors on the pins of MT8192. It is
> the same as in mt8183-pinctrl.
> 
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> 
> ---
> 
>  .../devicetree/bindings/pinctrl/pinctrl-mt8192.yaml   | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-mt8192.yaml b/Documentation/devicetree/bindings/pinctrl/pinctrl-mt8192.yaml
> index c90a132fbc79..e462f49eae6f 100644
> --- a/Documentation/devicetree/bindings/pinctrl/pinctrl-mt8192.yaml
> +++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-mt8192.yaml
> @@ -117,6 +117,17 @@ patternProperties:
>              $ref: /schemas/types.yaml#/definitions/uint32
>              enum: [0, 1, 2, 3]
>  
> +          mediatek,pull-down-adv:
> +            description: |
> +              Pull down settings for 2 pull resistors, R0 and R1. User can
> +              configure those special pins. Valid arguments are described as below:

Trailing ':' should be escaped, so '::'


> +              0: (R1, R0) = (0, 0) which means R1 disabled and R0 disabled.
> +              1: (R1, R0) = (0, 1) which means R1 disabled and R0 enabled.
> +              2: (R1, R0) = (1, 0) which means R1 enabled and R0 disabled.
> +              3: (R1, R0) = (1, 1) which means R1 enabled and R0 enabled.
> +            $ref: /schemas/types.yaml#/definitions/uint32
> +            enum: [0, 1, 2, 3]

It's okay, but for all these and other values (you have few such in the
binding), you should maybe add header and defines. It's much more
readable for humans...

Is the property valid without bias-pull-down?

Best regards,
Krzysztof
Re: [PATCH] dt-bindings: pinctrl: mt8192: Add mediatek,pull-down-adv property
Posted by Nícolas F. R. A. Prado 2 years, 4 months ago
On Fri, Apr 29, 2022 at 10:31:52PM +0200, Krzysztof Kozlowski wrote:
> On 29/04/2022 22:06, Nícolas F. R. A. Prado wrote:
> > Add the mediatek,pull-down-adv property to the pinctrl-mt8192 dt-binding
> > to allow configuring pull-down resistors on the pins of MT8192. It is
> > the same as in mt8183-pinctrl.
> > 
> > Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> > 
> > ---
> > 
> >  .../devicetree/bindings/pinctrl/pinctrl-mt8192.yaml   | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-mt8192.yaml b/Documentation/devicetree/bindings/pinctrl/pinctrl-mt8192.yaml
> > index c90a132fbc79..e462f49eae6f 100644
> > --- a/Documentation/devicetree/bindings/pinctrl/pinctrl-mt8192.yaml
> > +++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-mt8192.yaml
> > @@ -117,6 +117,17 @@ patternProperties:
> >              $ref: /schemas/types.yaml#/definitions/uint32
> >              enum: [0, 1, 2, 3]
> >  
> > +          mediatek,pull-down-adv:
> > +            description: |
> > +              Pull down settings for 2 pull resistors, R0 and R1. User can
> > +              configure those special pins. Valid arguments are described as below:
> 
> Trailing ':' should be escaped, so '::'

OK.

> 
> 
> > +              0: (R1, R0) = (0, 0) which means R1 disabled and R0 disabled.
> > +              1: (R1, R0) = (0, 1) which means R1 disabled and R0 enabled.
> > +              2: (R1, R0) = (1, 0) which means R1 enabled and R0 disabled.
> > +              3: (R1, R0) = (1, 1) which means R1 enabled and R0 enabled.
> > +            $ref: /schemas/types.yaml#/definitions/uint32
> > +            enum: [0, 1, 2, 3]
> 
> It's okay, but for all these and other values (you have few such in the
> binding), you should maybe add header and defines. It's much more
> readable for humans...

OK. I'll look into adding defines for these.

> 
> Is the property valid without bias-pull-down?

Actually it doesn't really make sense to set this property together with
bias-pull-down, as they set conflicting register settings for the pin bias...
I'll add a condition to avoid both being present.

Thanks,
Nícolas

> 
> Best regards,
> Krzysztof