[PATCH V2 0/3] pinctrl/arm: dt-bindings: k3: Deprecate header with register constants

[PATCH V2 0/3] pinctrl/arm: dt-bindings: k3: Deprecate header with register constants

[Nishanth Menon]

Hi,

This is an updated series to move the pinctrl bindings over to arch as
the definitions are hardware definitions without driver usage.

This series was triggered by the discussion in [1]

v1: https://lore.kernel.org/linux-arm-kernel/20230311131325.9750-1-nm@ti.com/

Nishanth Menon (3):
  dt-bindings: net: ti: k3-am654-cpsw-nuss: Drop pinmux header
  arm64: dts: ti: Use local header for pinctrl register values
  dt-bindings: pinctrl: k3: Deprecate header with register constants

 .../devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml     | 1 -
 arch/arm64/boot/dts/ti/k3-am62.dtsi                        | 3 ++-
 arch/arm64/boot/dts/ti/k3-am62a.dtsi                       | 3 ++-
 arch/arm64/boot/dts/ti/k3-am64.dtsi                        | 3 ++-
 arch/arm64/boot/dts/ti/k3-am65.dtsi                        | 3 ++-
 arch/arm64/boot/dts/ti/k3-j7200.dtsi                       | 3 ++-
 arch/arm64/boot/dts/ti/k3-j721e.dtsi                       | 3 ++-
 arch/arm64/boot/dts/ti/k3-j721s2.dtsi                      | 3 ++-
 arch/arm64/boot/dts/ti/k3-j784s4.dtsi                      | 3 ++-
 .../pinctrl/k3.h => arch/arm64/boot/dts/ti/k3-pinctrl.h    | 6 +++---
 include/dt-bindings/pinctrl/k3.h                           | 7 +++++++
 11 files changed, 26 insertions(+), 12 deletions(-)
 copy include/dt-bindings/pinctrl/k3.h => arch/arm64/boot/dts/ti/k3-pinctrl.h (93%)

[1] https://lore.kernel.org/all/c4d53e9c-dac0-8ccc-dc86-faada324beba@linaro.org/
-- 
2.40.0

[PATCH V2 1/3] dt-bindings: net: ti: k3-am654-cpsw-nuss: Drop pinmux header

[Nishanth Menon]

Drop the pinmux header reference as it is not used. Examples should
just show the node definition.

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Nishanth Menon <nm@ti.com>
---
Changes since V1:
 - Minor update to the commit message to indicate the header reference
   is not used.

V1: https://lore.kernel.org/linux-arm-kernel/20230311131325.9750-2-nm@ti.com/

 Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml | 1 -
 1 file changed, 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml b/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
index 628d63e1eb1f..f456093840ed 100644
--- a/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
+++ b/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
@@ -232,7 +232,6 @@ additionalProperties: false
 
 examples:
   - |
-    #include <dt-bindings/pinctrl/k3.h>
     #include <dt-bindings/soc/ti,sci_pm_domain.h>
     #include <dt-bindings/net/ti-dp83867.h>
     #include <dt-bindings/interrupt-controller/irq.h>
-- 
2.40.0

[PATCH V2 2/3] arm64: dts: ti: Use local header for pinctrl register values

[Nishanth Menon]

The DTS uses hardware register values directly in pin controller pin
configuration and not an abstraction of any form.

These definitions were previously put in the bindings header to avoid
code duplication and to provide some context meaning (name), but they
do not fit the purpose of bindings.

Store the constants in a header next to DTS and use them instead of
bindings.

Suggested-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Suggested-by: Linus Walleij <linus.walleij@linaro.org>
Link: https://lore.kernel.org/all/c4d53e9c-dac0-8ccc-dc86-faada324beba@linaro.org/
Signed-off-by: Nishanth Menon <nm@ti.com>
---

Changes since V1:
 - Updated $subject and $commit-message
 - Dropped the deletion of bindings header (instead followon patch
   provides a deprecation warning)
 - NOTE: checkpatch insists the following error exists:
   ERROR: Macros with complex values should be enclosed in parentheses
   However, as explained in patches to include/dt-bindings/pinctrl/k3.h,
   we do not need parentheses enclosing the values for this macro as we
   do intend it to generate two separate values as has been done for
   other similar platforms.

V1: https://lore.kernel.org/linux-arm-kernel/20230311131325.9750-3-nm@ti.com/

 arch/arm64/boot/dts/ti/k3-am62.dtsi   |  3 +-
 arch/arm64/boot/dts/ti/k3-am62a.dtsi  |  3 +-
 arch/arm64/boot/dts/ti/k3-am64.dtsi   |  3 +-
 arch/arm64/boot/dts/ti/k3-am65.dtsi   |  3 +-
 arch/arm64/boot/dts/ti/k3-j7200.dtsi  |  3 +-
 arch/arm64/boot/dts/ti/k3-j721e.dtsi  |  3 +-
 arch/arm64/boot/dts/ti/k3-j721s2.dtsi |  3 +-
 arch/arm64/boot/dts/ti/k3-j784s4.dtsi |  3 +-
 arch/arm64/boot/dts/ti/k3-pinctrl.h   | 53 +++++++++++++++++++++++++++
 9 files changed, 69 insertions(+), 8 deletions(-)
 create mode 100644 arch/arm64/boot/dts/ti/k3-pinctrl.h

diff --git a/arch/arm64/boot/dts/ti/k3-am62.dtsi b/arch/arm64/boot/dts/ti/k3-am62.dtsi
index 37fcbe7a3c33..a401f5225243 100644
--- a/arch/arm64/boot/dts/ti/k3-am62.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am62.dtsi
@@ -8,9 +8,10 @@
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
-#include <dt-bindings/pinctrl/k3.h>
 #include <dt-bindings/soc/ti,sci_pm_domain.h>
 
+#include "k3-pinctrl.h"
+
 / {
 	model = "Texas Instruments K3 AM625 SoC";
 	compatible = "ti,am625";
diff --git a/arch/arm64/boot/dts/ti/k3-am62a.dtsi b/arch/arm64/boot/dts/ti/k3-am62a.dtsi
index 6eb87c3f9f3c..fe60c9ce21e3 100644
--- a/arch/arm64/boot/dts/ti/k3-am62a.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am62a.dtsi
@@ -8,9 +8,10 @@
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
-#include <dt-bindings/pinctrl/k3.h>
 #include <dt-bindings/soc/ti,sci_pm_domain.h>
 
+#include "k3-pinctrl.h"
+
 / {
 	model = "Texas Instruments K3 AM62A SoC";
 	compatible = "ti,am62a7";
diff --git a/arch/arm64/boot/dts/ti/k3-am64.dtsi b/arch/arm64/boot/dts/ti/k3-am64.dtsi
index c858725133af..60fe95b48312 100644
--- a/arch/arm64/boot/dts/ti/k3-am64.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am64.dtsi
@@ -8,9 +8,10 @@
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
-#include <dt-bindings/pinctrl/k3.h>
 #include <dt-bindings/soc/ti,sci_pm_domain.h>
 
+#include "k3-pinctrl.h"
+
 / {
 	model = "Texas Instruments K3 AM642 SoC";
 	compatible = "ti,am642";
diff --git a/arch/arm64/boot/dts/ti/k3-am65.dtsi b/arch/arm64/boot/dts/ti/k3-am65.dtsi
index c538a0bf3cdd..3093ef6b9b23 100644
--- a/arch/arm64/boot/dts/ti/k3-am65.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am65.dtsi
@@ -8,9 +8,10 @@
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
-#include <dt-bindings/pinctrl/k3.h>
 #include <dt-bindings/soc/ti,sci_pm_domain.h>
 
+#include "k3-pinctrl.h"
+
 / {
 	model = "Texas Instruments K3 AM654 SoC";
 	compatible = "ti,am654";
diff --git a/arch/arm64/boot/dts/ti/k3-j7200.dtsi b/arch/arm64/boot/dts/ti/k3-j7200.dtsi
index d74f86b0f622..bbe380c72a7e 100644
--- a/arch/arm64/boot/dts/ti/k3-j7200.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j7200.dtsi
@@ -7,9 +7,10 @@
 
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
-#include <dt-bindings/pinctrl/k3.h>
 #include <dt-bindings/soc/ti,sci_pm_domain.h>
 
+#include "k3-pinctrl.h"
+
 / {
 	model = "Texas Instruments K3 J7200 SoC";
 	compatible = "ti,j7200";
diff --git a/arch/arm64/boot/dts/ti/k3-j721e.dtsi b/arch/arm64/boot/dts/ti/k3-j721e.dtsi
index 6975cae644d9..4c7d5f9d61a8 100644
--- a/arch/arm64/boot/dts/ti/k3-j721e.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j721e.dtsi
@@ -7,9 +7,10 @@
 
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
-#include <dt-bindings/pinctrl/k3.h>
 #include <dt-bindings/soc/ti,sci_pm_domain.h>
 
+#include "k3-pinctrl.h"
+
 / {
 	model = "Texas Instruments K3 J721E SoC";
 	compatible = "ti,j721e";
diff --git a/arch/arm64/boot/dts/ti/k3-j721s2.dtsi b/arch/arm64/boot/dts/ti/k3-j721s2.dtsi
index 78295ee0fee5..376924726f1f 100644
--- a/arch/arm64/boot/dts/ti/k3-j721s2.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j721s2.dtsi
@@ -10,9 +10,10 @@
 
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
-#include <dt-bindings/pinctrl/k3.h>
 #include <dt-bindings/soc/ti,sci_pm_domain.h>
 
+#include "k3-pinctrl.h"
+
 / {
 
 	model = "Texas Instruments K3 J721S2 SoC";
diff --git a/arch/arm64/boot/dts/ti/k3-j784s4.dtsi b/arch/arm64/boot/dts/ti/k3-j784s4.dtsi
index 3eb0d0568959..2e03d84da7d2 100644
--- a/arch/arm64/boot/dts/ti/k3-j784s4.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j784s4.dtsi
@@ -10,9 +10,10 @@
 
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
-#include <dt-bindings/pinctrl/k3.h>
 #include <dt-bindings/soc/ti,sci_pm_domain.h>
 
+#include "k3-pinctrl.h"
+
 / {
 	model = "Texas Instruments K3 J784S4 SoC";
 	compatible = "ti,j784s4";
diff --git a/arch/arm64/boot/dts/ti/k3-pinctrl.h b/arch/arm64/boot/dts/ti/k3-pinctrl.h
new file mode 100644
index 000000000000..c97548a3f42d
--- /dev/null
+++ b/arch/arm64/boot/dts/ti/k3-pinctrl.h
@@ -0,0 +1,53 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * This header provides constants for pinctrl bindings for TI's K3 SoC
+ * family.
+ *
+ * Copyright (C) 2018-2023 Texas Instruments Incorporated - https://www.ti.com/
+ */
+#ifndef DTS_ARM64_TI_K3_PINCTRL_H
+#define DTS_ARM64_TI_K3_PINCTRL_H
+
+#define PULLUDEN_SHIFT		(16)
+#define PULLTYPESEL_SHIFT	(17)
+#define RXACTIVE_SHIFT		(18)
+
+#define PULL_DISABLE		(1 << PULLUDEN_SHIFT)
+#define PULL_ENABLE		(0 << PULLUDEN_SHIFT)
+
+#define PULL_UP			(1 << PULLTYPESEL_SHIFT | PULL_ENABLE)
+#define PULL_DOWN		(0 << PULLTYPESEL_SHIFT | PULL_ENABLE)
+
+#define INPUT_EN		(1 << RXACTIVE_SHIFT)
+#define INPUT_DISABLE		(0 << RXACTIVE_SHIFT)
+
+/* Only these macros are expected be used directly in device tree files */
+#define PIN_OUTPUT		(INPUT_DISABLE | PULL_DISABLE)
+#define PIN_OUTPUT_PULLUP	(INPUT_DISABLE | PULL_UP)
+#define PIN_OUTPUT_PULLDOWN	(INPUT_DISABLE | PULL_DOWN)
+#define PIN_INPUT		(INPUT_EN | PULL_DISABLE)
+#define PIN_INPUT_PULLUP	(INPUT_EN | PULL_UP)
+#define PIN_INPUT_PULLDOWN	(INPUT_EN | PULL_DOWN)
+
+#define AM62AX_IOPAD(pa, val, muxmode)		(((pa) & 0x1fff)) ((val) | (muxmode))
+#define AM62AX_MCU_IOPAD(pa, val, muxmode)	(((pa) & 0x1fff)) ((val) | (muxmode))
+
+#define AM62X_IOPAD(pa, val, muxmode)		(((pa) & 0x1fff)) ((val) | (muxmode))
+#define AM62X_MCU_IOPAD(pa, val, muxmode)	(((pa) & 0x1fff)) ((val) | (muxmode))
+
+#define AM64X_IOPAD(pa, val, muxmode)		(((pa) & 0x1fff)) ((val) | (muxmode))
+#define AM64X_MCU_IOPAD(pa, val, muxmode)	(((pa) & 0x1fff)) ((val) | (muxmode))
+
+#define AM65X_IOPAD(pa, val, muxmode)		(((pa) & 0x1fff)) ((val) | (muxmode))
+#define AM65X_WKUP_IOPAD(pa, val, muxmode)	(((pa) & 0x1fff)) ((val) | (muxmode))
+
+#define J721E_IOPAD(pa, val, muxmode)		(((pa) & 0x1fff)) ((val) | (muxmode))
+#define J721E_WKUP_IOPAD(pa, val, muxmode)	(((pa) & 0x1fff)) ((val) | (muxmode))
+
+#define J721S2_IOPAD(pa, val, muxmode)		(((pa) & 0x1fff)) ((val) | (muxmode))
+#define J721S2_WKUP_IOPAD(pa, val, muxmode)	(((pa) & 0x1fff)) ((val) | (muxmode))
+
+#define J784S4_IOPAD(pa, val, muxmode)		(((pa) & 0x1fff)) ((val) | (muxmode))
+#define J784S4_WKUP_IOPAD(pa, val, muxmode)	(((pa) & 0x1fff)) ((val) | (muxmode))
+
+#endif
-- 
2.40.0

Re: [PATCH V2 2/3] arm64: dts: ti: Use local header for pinctrl register values

[Krzysztof Kozlowski]

On 15/03/2023 16:52, Nishanth Menon wrote:
> The DTS uses hardware register values directly in pin controller pin
> configuration and not an abstraction of any form.
> 
> These definitions were previously put in the bindings header to avoid
> code duplication and to provide some context meaning (name), but they
> do not fit the purpose of bindings.
> 
> Store the constants in a header next to DTS and use them instead of
> bindings.
> 
> Suggested-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Suggested-by: Linus Walleij <linus.walleij@linaro.org>
> Link: https://lore.kernel.org/all/c4d53e9c-dac0-8ccc-dc86-faada324beba@linaro.org/
> Signed-off-by: Nishanth Menon <nm@ti.com>
> ---

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

[PATCH V2 3/3] dt-bindings: pinctrl: k3: Deprecate header with register constants

[Nishanth Menon]

For convenience (less code duplication), 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. In
fact, the Linux pinctrl-single driver 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), so remove any references to the bindings
header and add a warning that it is deprecated.

Suggested-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Link: https://lore.kernel.org/linux-arm-kernel/71c7feff-4189-f12f-7353-bce41a61119d@linaro.org/
Signed-off-by: Nishanth Menon <nm@ti.com>
---
New patch in V2 series and we expect to remove this header after a kernel
rev.

 include/dt-bindings/pinctrl/k3.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/include/dt-bindings/pinctrl/k3.h b/include/dt-bindings/pinctrl/k3.h
index 6bb9df1a264d..b5aca149664e 100644
--- a/include/dt-bindings/pinctrl/k3.h
+++ b/include/dt-bindings/pinctrl/k3.h
@@ -8,6 +8,13 @@
 #ifndef _DT_BINDINGS_PINCTRL_TI_K3_H
 #define _DT_BINDINGS_PINCTRL_TI_K3_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 PULLUDEN_SHIFT		(16)
 #define PULLTYPESEL_SHIFT	(17)
 #define RXACTIVE_SHIFT		(18)
-- 
2.40.0

Re: [PATCH V2 3/3] dt-bindings: pinctrl: k3: Deprecate header with register constants

[Krzysztof Kozlowski]

On 15/03/2023 16:52, Nishanth Menon wrote:
> For convenience (less code duplication), 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. In
> fact, the Linux pinctrl-single driver 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), so remove any references to the bindings
> header and add a warning that it is deprecated.
> 
> Suggested-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Link: https://lore.kernel.org/linux-arm-kernel/71c7feff-4189-f12f-7353-bce41a61119d@linaro.org/
> Signed-off-by: Nishanth Menon <nm@ti.com>
> ---
> New patch in V2 series and we expect to remove this header after a kernel
> rev.
> 
>  include/dt-bindings/pinctrl/k3.h | 7 +++++++
>  1 file changed, 7 insertions(+)

This should go to the same branch as DTS, so not pinctrl tree, to avoid
warnings.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH V2 0/3] pinctrl/arm: dt-bindings: k3: Deprecate header with register constants

[Linus Walleij]

On Wed, Mar 15, 2023 at 4:52 PM Nishanth Menon <nm@ti.com> wrote:

> This is an updated series to move the pinctrl bindings over to arch as
> the definitions are hardware definitions without driver usage.
>
> This series was triggered by the discussion in [1]

Acked-by: Linus Walleij <linus.walleij@linaro.org>

As mentioned by the DT maintainer I think this best all go
into the same tree as the DTS files, so let's merge it all
through the TI SoC tree.

Yours,
Linus Walleij

Re: [PATCH V2 0/3] pinctrl/arm: dt-bindings: k3: Deprecate header with register constants

[Nishanth Menon]

Hi Nishanth Menon,

On Wed, 15 Mar 2023 10:52:25 -0500, Nishanth Menon wrote:
> This is an updated series to move the pinctrl bindings over to arch as
> the definitions are hardware definitions without driver usage.
> 
> This series was triggered by the discussion in [1]
> 
> v1: https://lore.kernel.org/linux-arm-kernel/20230311131325.9750-1-nm@ti.com/
> 
> [...]

I have applied the following to branch ti-k3-dts-next on [1].
Thank you!

[1/3] dt-bindings: net: ti: k3-am654-cpsw-nuss: Drop pinmux header
      commit: c680fa2a09a9550284546d2deeb31640ec3b56c8
[2/3] arm64: dts: ti: Use local header for pinctrl register values
      commit: fe49f2d776f7994dc60dd04712a437fd0bdc67a0
[3/3] dt-bindings: pinctrl: k3: Deprecate header with register constants
      commit: f2de003e1426ccbefa281a066040da7699f6d461

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent up the chain during
the next merge window (or sooner if it is a relevant bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

[1] git://git.kernel.org/pub/scm/linux/kernel/git/ti/linux.git
-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D