On Thu, Sep 30, 2021 at 9:59 AM zhiyong.tao [off-list ref] wrote:

On Wed, 2021-09-29 at 16:47 -0500, Rob Herring wrote:

quoted

On Fri, Sep 24, 2021 at 04:06:29PM +0800, Zhiyong Tao wrote:

quoted

For supporting SI units in "bias-pull-down" & "bias-pull-up",
change pull up/down description
and add "mediatek,rsel_resistance_in_si_unit" description.

Signed-off-by: Zhiyong Tao <redacted>
---
 .../bindings/pinctrl/pinctrl-mt8195.yaml      | 86
++++++++++++++++++-
 1 file changed, 84 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-

mt8195.yaml b/Documentation/devicetree/bindings/pinctrl/pinctrl-
mt8195.yaml
index 2f12ec59eee5..5f642bef72af 100644

--- a/Documentation/devicetree/bindings/pinctrl/pinctrl-mt8195.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-mt8195.yaml

@@ -49,6 +49,12 @@ properties:
     description: The interrupt outputs to sysirq.
     maxItems: 1

+  mediatek,rsel_resistance_in_si_unit:

s/_/-/

Hi Rob,

what do you mean?

He means: replace the hyphens ("-") with underscores ("_").
(s/X/Y/ is a regular expression.)

quoted

quoted

+    type: boolean
+    description: |
+      Identifying i2c pins pull up/down type which is RSEL. It can
support
+      RSEL define or si unit value(ohm) to set different
resistance.

Aren't the RSEL and ohms disjoint values? 0-207 for RSEL and >1000
for
ohms. Why is this property even needed.

No, they aren't.
As we talked in v11. "mediatek,rsel_resistance_in_si_unit" is only a
flag.



Hi ChenYu,

In the next version, we provide a solution which we discussed internal
to avoid value clashes.

The solution:
1. We will keep the define "MTK_PULL_SET_RSEL_000 200". It won't
change.

2. We will add a property in pio dtsi node, for example,
the property name is "rsel_resistance_in_si_unit".
We will add a flag "rsel_si_unit" in pinctrl device.
in probe function, we will identify the property name
"rsel_resistance_in_si_unit" to set the flag "rsel_si_unit" value.
So it can void value clashes.

3.We will provide the define "MTK_PULL_SET_RSEL_000 200" and si unit
two solution. users can support which solution by add property
"rsel_resistance_in_si_unit" in dts node or not.

Right. I thought that is what is implemented in this version already?

Also I just realized that this binding is limited in scope to just the
MT8195, for which we already know that the RSEL values do not overlap
with MTK_PULL_SET_RSEL_*. I assume that is why Rob thinks the flag
is unnecessary.

quoted

quoted

+
 #PIN CONFIGURATION NODES
 patternProperties:
   '-pins$':

@@ -85,9 +91,85 @@ patternProperties:
           2/4/6/8/10/12/14/16mA in mt8195.
         enum: [0, 1, 2, 3, 4, 5, 6, 7]

-      bias-pull-down: true
+      bias-pull-down:
+        description: |
+          For pull down type is normal, it don't need add RSEL &

R1R0 define
+          and resistance value.
+          For pull down type is PUPD/R0/R1 type, it can add R1R0
define to
+          set different resistance. It can support
"MTK_PUPD_SET_R1R0_00" &
+          "MTK_PUPD_SET_R1R0_01" & "MTK_PUPD_SET_R1R0_10" &
"MTK_PUPD_SET_R1R0_11"
+          define in mt8195.
+          For pull down type is RSEL, it can add RSEL define &
resistance value(ohm)
+          to set different resistance by identifying property
"mediatek,rsel_resistance_in_si_unit".
+          It can support "MTK_PULL_SET_RSEL_000" &
"MTK_PULL_SET_RSEL_001"
+          & "MTK_PULL_SET_RSEL_010" & "MTK_PULL_SET_RSEL_011" &
"MTK_PULL_SET_RSEL_100"
+          & "MTK_PULL_SET_RSEL_101" & "MTK_PULL_SET_RSEL_110" &
"MTK_PULL_SET_RSEL_111"
+          define in mt8195. It can also support resistance
value(ohm) "75000" & "5000" in mt8195.
+          oneOf:

Because of the indentation, this is all just part of 'description'.

Can you help to give some suggestion to fix it?

Unindent it by two spaces, so that it is at the same level with
"description:".

quoted

quoted

+            - enum: [100, 101, 102, 103]
+            - description: mt8195 pull down PUPD/R0/R1 type define
value.

This entry is always true.

why is it always true? we only get define value.
"100~104" are means that "#define MTK_PUPD_SET_R1R0_10 102" in
include/dt-bindings/pinctrl/mt65xx.h.

"description" is not a conditional match, so it always evaluates to true.
Based on my limited DT schema and YAML knowledge, I think the underlying
issue is that you have the structure incorrectly defined.

"-" denotes a list item. So in your example, you have "enum" and "description"
as separate associative arrays, each as a list item part of the "oneOf" list.

What you want is actually:

    oneOf:
      - enum: [100, 101, 102, 103]
        description: mt8195 pull down PUPD/R0/R1 type define value.
      - enum: [200, 201, 202, 203, 204, 205, 206, 207]
        description: mt8195 pull down RSEL type define value.

So that "enum" and "description" are part of the same associative array.
Note the lack of a "-" and the extra indentation in front of "description".


Regards
ChenYu

quoted

quoted

+            - enum: [200, 201, 202, 203, 204, 205, 206, 207]

Are these supposed to be hex?

yes, it is patch 1/5 define "#define MTK_PULL_SET_RSEL_000  200".

quoted

quoted

+            - description: mt8195 pull down RSEL type define
value.

And so is this one. That makes 'oneOf' always false.

why is it always false? we only get the si unit value.

quoted

quoted

+            - enum: [75000, 5000]
+            - description: mt8195 pull down RSEL type si unit
value(ohm).
+
+          An example of using RSEL define:
+          pincontroller {
+            i2c0_pin {
+              pinmux = <PINMUX_GPIO8__FUNC_SDA0>;
+              bias-pull-down = <MTK_PULL_SET_RSEL_001>;
+            };
+          };
+          An example of using si unit resistance value(ohm):
+          &pio {
+            mediatek,rsel_resistance_in_si_unit;
+          }
+          pincontroller {
+            i2c0_pin {
+              pinmux = <PINMUX_GPIO8__FUNC_SDA0>;
+              bias-pull-down = <75000>;
+            };
+          };

-      bias-pull-up: true
+      bias-pull-up:
+        description: |
+          For pull up type is normal, it don't need add RSEL &
R1R0 define
+          and resistance value.
+          For pull up type is PUPD/R0/R1 type, it can add R1R0
define to
+          set different resistance. It can support
"MTK_PUPD_SET_R1R0_00" &
+          "MTK_PUPD_SET_R1R0_01" & "MTK_PUPD_SET_R1R0_10" &
"MTK_PUPD_SET_R1R0_11"
+          define in mt8195.
+          For pull up type is RSEL, it can add RSEL define &
resistance value(ohm)
+          to set different resistance by identifying property
"mediatek,rsel_resistance_in_si_unit".
+          It can support "MTK_PULL_SET_RSEL_000" &
"MTK_PULL_SET_RSEL_001"
+          & "MTK_PULL_SET_RSEL_010" & "MTK_PULL_SET_RSEL_011" &
"MTK_PULL_SET_RSEL_100"
+          & "MTK_PULL_SET_RSEL_101" & "MTK_PULL_SET_RSEL_110" &
"MTK_PULL_SET_RSEL_111"
+          define in mt8195. It can also support resistance
value(ohm)
+          "1000" & "1500" & "2000" & "3000" & "4000" & "5000" &
"10000" & "75000" in mt8195.
+          oneOf:
+            - enum: [100, 101, 102, 103]
+            - description: mt8195 pull up PUPD/R0/R1 type define
value.
+            - enum: [200, 201, 202, 203, 204, 205, 206, 207]
+            - description: mt8195 pull up RSEL type define value.
+            - enum: [1000, 1500, 2000, 3000, 4000, 5000, 10000,
75000]
+            - description: mt8195 pull up RSEL type si unit
value(ohm).

Same issues here.

quoted

+          An example of using RSEL define:
+          pincontroller {
+            i2c0_pin {
+              pinmux = <PINMUX_GPIO8__FUNC_SDA0>;
+              bias-pull-up = <MTK_PULL_SET_RSEL_001>;
+            };
+          };
+          An example of using si unit resistance value(ohm):
+          &pio {
+            mediatek,rsel_resistance_in_si_unit;
+          }
+          pincontroller {
+            i2c0_pin {
+              pinmux = <PINMUX_GPIO8__FUNC_SDA0>;
+              bias-pull-up = <1000>;
+            };
+          };

       bias-disable: true

--
2.25.1

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek