[PATCH v4 0/2] pinctrl/arm: dt-bindings: deprecate header with register constants

[PATCH v4 0/2] pinctrl/arm: dt-bindings: deprecate header with register constants

[Krzysztof Kozlowski]

Hi,

Changes since v3
================
1. Add patch 1 - bindings example fixes.
2. All dependencies were merged, so this can go via Samsung pinctrl tree.

Changes since v2
================
1. Split last bindings patch per driver changes and bindings.
2. Add tags.

Changes since v1
================
1. Correct title (in the comment) of each header (Chanho).
2. Patch #7: Adjust warning message.
3. Add tags.

Description
===========
The Samsung pin controller drivers were always expecting DTS to use raw
register values for pin configuration (e.g. pull up/down, drive strength).  DTS
had these values hard-coded all over, so at some point we decided for
convenience to move them to dt-bindings header.  Less code duplication, some
meaning added to raw number, etc.

However these constants do not fit the purpose of bindings.  They do not
provide any abstraction, any hardware and driver independent ID.  With minor
exceptions, the Linux drivers actually do not use the bindings header at
all.  Because of this "dt-bindings ID" approach, these constants were re-used
between chips, e.g. Exynos ones in S5PV210.  These does not make much sense
because the values between Exynos and S5PV210 (or S3C24xx) are not related.  If
it was an abstraction ID, this would be fine. But it's not.

Clean this up by:
1. Moving the constants to DTS-local headers.
2. Deprecating the bindings header.

Tested by comparing DTBs (dtx_diff, fdtdump).

Best regards,
Krzysztof

Krzysztof Kozlowski (2):
  dt-bindings: pinctrl: samsung: stop using bindings header with
    constants
  dt-bindings: pinctrl: samsung: deprecate header with register
    constants

 .../pinctrl/samsung,pinctrl-pins-cfg.yaml     |  1 -
 .../bindings/pinctrl/samsung,pinctrl.yaml     | 63 ++++++++-----------
 include/dt-bindings/pinctrl/samsung.h         |  7 +++
 3 files changed, 34 insertions(+), 37 deletions(-)

-- 
2.34.1

[PATCH v4 1/2] dt-bindings: pinctrl: samsung: stop using bindings header with constants

[Krzysztof Kozlowski]

The bindings header with pin controller register values is being
deprecated and DTS already switched to a DTS-local header.

Do not reference the bindings header in schema and replace the defines
with raw values.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 .../pinctrl/samsung,pinctrl-pins-cfg.yaml     |  1 -
 .../bindings/pinctrl/samsung,pinctrl.yaml     | 63 ++++++++-----------
 2 files changed, 27 insertions(+), 37 deletions(-)

diff --git a/Documentation/devicetree/bindings/pinctrl/samsung,pinctrl-pins-cfg.yaml b/Documentation/devicetree/bindings/pinctrl/samsung,pinctrl-pins-cfg.yaml
index 9869d4dceddb..f796f27bf0e6 100644
--- a/Documentation/devicetree/bindings/pinctrl/samsung,pinctrl-pins-cfg.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/samsung,pinctrl-pins-cfg.yaml
@@ -20,7 +20,6 @@ description: |
   The values used for config properties should be derived from the hardware
   manual and these values are programmed as-is into the pin pull up/down and
   driver strength register of the pin-controller.
-  See also include/dt-bindings/pinctrl/samsung.h with useful constants.
 
   See also Documentation/devicetree/bindings/pinctrl/samsung,pinctrl.yaml for
   additional information and example.
diff --git a/Documentation/devicetree/bindings/pinctrl/samsung,pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/samsung,pinctrl.yaml
index 3a65c66ca71d..dafa51c69c06 100644
--- a/Documentation/devicetree/bindings/pinctrl/samsung,pinctrl.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/samsung,pinctrl.yaml
@@ -15,9 +15,6 @@ description: |
   This is a part of device tree bindings for Samsung S3C/S5P/Exynos SoC pin
   controller.
 
-  Pin group settings (like drive strength, pull up/down) are available as
-  macros in include/dt-bindings/pinctrl/samsung.h.
-
   All the pin controller nodes should be represented in the aliases node using
   the following format 'pinctrl{n}' where n is a unique number for the alias.
 
@@ -138,8 +135,6 @@ additionalProperties: false
 
 examples:
   - |
-    #include <dt-bindings/pinctrl/samsung.h>
-
     pinctrl@7f008000 {
         compatible = "samsung,s3c64xx-pinctrl";
         reg = <0x7f008000 0x1000>;
@@ -166,8 +161,8 @@ examples:
 
         uart0-data-pins {
             samsung,pins = "gpa-0", "gpa-1";
-            samsung,pin-function = <EXYNOS_PIN_FUNC_2>;
-            samsung,pin-pud = <S3C64XX_PIN_PULL_NONE>;
+            samsung,pin-function = <2>;
+            samsung,pin-pud = <0>;
         };
 
         // ...
@@ -175,7 +170,6 @@ examples:
 
   - |
     #include <dt-bindings/interrupt-controller/arm-gic.h>
-    #include <dt-bindings/pinctrl/samsung.h>
 
     pinctrl@11400000 {
         compatible = "samsung,exynos4210-pinctrl";
@@ -197,9 +191,9 @@ examples:
 
         uart0-data-pins {
             samsung,pins = "gpa0-0", "gpa0-1";
-            samsung,pin-function = <EXYNOS_PIN_FUNC_2>;
-            samsung,pin-pud = <EXYNOS_PIN_PULL_NONE>;
-            samsung,pin-drv = <EXYNOS4_PIN_DRV_LV1>;
+            samsung,pin-function = <2>;
+            samsung,pin-pud = <0>;
+            samsung,pin-drv = <0>;
         };
 
         // ...
@@ -207,14 +201,14 @@ examples:
         sleep0: sleep-state {
             gpa0-0-pin {
                 samsung,pins = "gpa0-0";
-                samsung,pin-con-pdn = <EXYNOS_PIN_PDN_INPUT>;
-                samsung,pin-pud-pdn = <EXYNOS_PIN_PULL_NONE>;
+                samsung,pin-con-pdn = <2>;
+                samsung,pin-pud-pdn = <0>;
             };
 
             gpa0-1-pin {
                 samsung,pins = "gpa0-1";
-                samsung,pin-con-pdn = <EXYNOS_PIN_PDN_OUT0>;
-                samsung,pin-pud-pdn = <EXYNOS_PIN_PULL_NONE>;
+                samsung,pin-con-pdn = <0>;
+                samsung,pin-pud-pdn = <0>;
             };
 
             // ...
@@ -223,7 +217,6 @@ examples:
 
   - |
     #include <dt-bindings/interrupt-controller/arm-gic.h>
-    #include <dt-bindings/pinctrl/samsung.h>
 
     pinctrl@11000000 {
         compatible = "samsung,exynos4210-pinctrl";
@@ -272,26 +265,26 @@ examples:
 
         sd0-clk-pins {
             samsung,pins = "gpk0-0";
-            samsung,pin-function = <EXYNOS_PIN_FUNC_2>;
-            samsung,pin-pud = <EXYNOS_PIN_PULL_NONE>;
-            samsung,pin-drv = <EXYNOS4_PIN_DRV_LV4>;
+            samsung,pin-function = <2>;
+            samsung,pin-pud = <0>;
+            samsung,pin-drv = <3>;
         };
 
         sd4-bus-width8-pins {
             part-1-pins {
                 samsung,pins = "gpk0-3", "gpk0-4",
                                "gpk0-5", "gpk0-6";
-                samsung,pin-function = <EXYNOS_PIN_FUNC_3>;
-                samsung,pin-pud = <EXYNOS_PIN_PULL_UP>;
-                samsung,pin-drv = <EXYNOS4_PIN_DRV_LV4>;
+                samsung,pin-function = <3>;
+                samsung,pin-pud = <3>;
+                samsung,pin-drv = <3>;
             };
 
             part-2-pins {
                 samsung,pins = "gpk1-3", "gpk1-4",
                                "gpk1-5", "gpk1-6";
-                samsung,pin-function = <EXYNOS_PIN_FUNC_4>;
-                samsung,pin-pud = <EXYNOS_PIN_PULL_UP>;
-                samsung,pin-drv = <EXYNOS4_PIN_DRV_LV4>;
+                samsung,pin-function = <4>;
+                samsung,pin-pud = <3>;
+                samsung,pin-drv = <3>;
             };
         };
 
@@ -299,16 +292,15 @@ examples:
 
         otg-gp-pins {
             samsung,pins = "gpx3-3";
-            samsung,pin-function = <EXYNOS_PIN_FUNC_OUTPUT>;
-            samsung,pin-pud = <EXYNOS_PIN_PULL_NONE>;
-            samsung,pin-drv = <EXYNOS4_PIN_DRV_LV1>;
+            samsung,pin-function = <1>;
+            samsung,pin-pud = <0>;
+            samsung,pin-drv = <0>;
             samsung,pin-val = <0>;
         };
     };
 
   - |
     #include <dt-bindings/interrupt-controller/arm-gic.h>
-    #include <dt-bindings/pinctrl/samsung.h>
 
     pinctrl@10580000 {
         compatible = "samsung,exynos5433-pinctrl";
@@ -352,9 +344,9 @@ examples:
         initial_alive: initial-state {
             gpa0-0-pin {
                 samsung,pins = "gpa0-0";
-                samsung,pin-function = <EXYNOS_PIN_FUNC_INPUT>;
-                samsung,pin-pud = <EXYNOS_PIN_PULL_DOWN>;
-                samsung,pin-drv = <EXYNOS5433_PIN_DRV_FAST_SR1>;
+                samsung,pin-function = <0>;
+                samsung,pin-pud = <1>;
+                samsung,pin-drv = <0>;
             };
 
             // ...
@@ -363,7 +355,6 @@ examples:
 
   - |
     #include <dt-bindings/interrupt-controller/arm-gic.h>
-    #include <dt-bindings/pinctrl/samsung.h>
 
     pinctrl@114b0000 {
         compatible = "samsung,exynos5433-pinctrl";
@@ -384,9 +375,9 @@ examples:
         i2s0-bus-pins {
             samsung,pins = "gpz0-0", "gpz0-1", "gpz0-2", "gpz0-3",
                            "gpz0-4", "gpz0-5", "gpz0-6";
-            samsung,pin-function = <EXYNOS_PIN_FUNC_2>;
-            samsung,pin-pud = <EXYNOS_PIN_PULL_NONE>;
-            samsung,pin-drv = <EXYNOS5433_PIN_DRV_FAST_SR1>;
+            samsung,pin-function = <2>;
+            samsung,pin-pud = <0>;
+            samsung,pin-drv = <0>;
         };
 
         // ...
-- 
2.34.1

[PATCH v4 2/2] dt-bindings: pinctrl: samsung: deprecate header with register constants

[Krzysztof Kozlowski]

For convenience (less code duplication, some meaning added to raw
number), the pin controller pin configuration register values
were defined in the bindings header.  These are not some IDs or other
abstraction layer but raw numbers used in the registers

These constants do not fit the purpose of bindings.  They do not provide
any abstraction, any hardware and driver independent ID.  With minor
exceptions, the Linux drivers actually do not use the bindings header at
all.

All of the constants were moved already to headers local to DTS
(residing in DTS directory) and to Samsung pinctrl driver (where
applicable), so remove any references to the bindings header and add a
warning tha tit is deprecated.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Reviewed-by: Chanho Park <chanho61.park@samsung.com>
Acked-by: Rob Herring <robh@kernel.org>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
---
 include/dt-bindings/pinctrl/samsung.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/include/dt-bindings/pinctrl/samsung.h b/include/dt-bindings/pinctrl/samsung.h
index 950970634dfe..d1da5ff68d0c 100644
--- a/include/dt-bindings/pinctrl/samsung.h
+++ b/include/dt-bindings/pinctrl/samsung.h
@@ -10,6 +10,13 @@
 #ifndef __DT_BINDINGS_PINCTRL_SAMSUNG_H__
 #define __DT_BINDINGS_PINCTRL_SAMSUNG_H__
 
+/*
+ * These bindings are deprecated, because they do not match the actual
+ * concept of bindings but rather contain pure register values.
+ * Instead include the header in the DTS source directory.
+ */
+#warning "These bindings are deprecated. Instead use the header in the DTS source directory."
+
 #define EXYNOS_PIN_PULL_NONE		0
 #define EXYNOS_PIN_PULL_DOWN		1
 #define EXYNOS_PIN_PULL_UP		3
-- 
2.34.1

Re: [PATCH v4 1/2] dt-bindings: pinctrl: samsung: stop using bindings header with constants

[Rob Herring]

On Tue, Aug 16, 2022 at 04:30:15PM +0300, Krzysztof Kozlowski wrote:
> The bindings header with pin controller register values is being
> deprecated and DTS already switched to a DTS-local header.
> 
> Do not reference the bindings header in schema and replace the defines
> with raw values.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>  .../pinctrl/samsung,pinctrl-pins-cfg.yaml     |  1 -
>  .../bindings/pinctrl/samsung,pinctrl.yaml     | 63 ++++++++-----------
>  2 files changed, 27 insertions(+), 37 deletions(-)

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH v4 0/2] pinctrl/arm: dt-bindings: deprecate header with register constants

[Krzysztof Kozlowski]

On Tue, 16 Aug 2022 16:30:14 +0300, Krzysztof Kozlowski wrote:
> Changes since v3
> ================
> 1. Add patch 1 - bindings example fixes.
> 2. All dependencies were merged, so this can go via Samsung pinctrl tree.
> 
> Changes since v2
> ================
> 1. Split last bindings patch per driver changes and bindings.
> 2. Add tags.
> 
> [...]

Applied, thanks!

[1/2] dt-bindings: pinctrl: samsung: stop using bindings header with constants
      https://git.kernel.org/pinctrl/samsung/c/df805304a820ed10fc3d038dd64b85821c9ee606
[2/2] dt-bindings: pinctrl: samsung: deprecate header with register constants
      https://git.kernel.org/pinctrl/samsung/c/9d9292576810d0b36897718c24dfbc1a2835314b

Best regards,
-- 
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Re: [PATCH v4 2/2] dt-bindings: pinctrl: samsung: deprecate header with register constants

[Linus Walleij]

On Tue, Aug 16, 2022 at 3:30 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:

> For convenience (less code duplication, some meaning added to raw
> number), the pin controller pin configuration register values
> were defined in the bindings header.  These are not some IDs or other
> abstraction layer but raw numbers used in the registers
>
> These constants do not fit the purpose of bindings.  They do not provide
> any abstraction, any hardware and driver independent ID.  With minor
> exceptions, the Linux drivers actually do not use the bindings header at
> all.
>
> All of the constants were moved already to headers local to DTS
> (residing in DTS directory) and to Samsung pinctrl driver (where
> applicable), so remove any references to the bindings header and add a
> warning tha tit is deprecated.
>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Reviewed-by: Chanho Park <chanho61.park@samsung.com>
> Acked-by: Rob Herring <robh@kernel.org>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

I assume this will be queued to one or another of the Samsung trees,
and I will get it by pull request. Just tell me if something else needs to
happen!

Yours,
Linus Walleij

Re: [PATCH v4 1/2] dt-bindings: pinctrl: samsung: stop using bindings header with constants

[Linus Walleij]

On Tue, Aug 16, 2022 at 3:30 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:

> The bindings header with pin controller register values is being
> deprecated and DTS already switched to a DTS-local header.
>
> Do not reference the bindings header in schema and replace the defines
> with raw values.
>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Looks good to me:
Acked-by: Linus Walleij <linus.walleij@linaro.org>

I expect this to come to me with some pull request later.

Yours,
Linus Walleij