[PATCH 0/2] AM654: Add pinmux support

[PATCH 0/2] AM654: Add pinmux support

[Vignesh R]

Add pinmux definitions and pinctrl regions for AM654 SoCs

Tero Kristo (2):
  dt-bindings: pinctrl: k3-am6: Introduce pinmux definitions
  arm64: dts: ti: k3-am65: Add pinctrl regions

 MAINTAINERS                                |  1 +
 arch/arm64/boot/dts/ti/k3-am65-main.dtsi   | 16 +++++++
 arch/arm64/boot/dts/ti/k3-am65-wakeup.dtsi |  8 ++++
 arch/arm64/boot/dts/ti/k3-am65.dtsi        |  1 +
 include/dt-bindings/pinctrl/k3-am6.h       | 49 ++++++++++++++++++++++
 5 files changed, 75 insertions(+)
 create mode 100644 include/dt-bindings/pinctrl/k3-am6.h

-- 
2.19.1

[PATCH 0/2] AM654: Add pinmux support

[Vignesh R]

Add pinmux definitions and pinctrl regions for AM654 SoCs

Tero Kristo (2):
  dt-bindings: pinctrl: k3-am6: Introduce pinmux definitions
  arm64: dts: ti: k3-am65: Add pinctrl regions

 MAINTAINERS                                |  1 +
 arch/arm64/boot/dts/ti/k3-am65-main.dtsi   | 16 +++++++
 arch/arm64/boot/dts/ti/k3-am65-wakeup.dtsi |  8 ++++
 arch/arm64/boot/dts/ti/k3-am65.dtsi        |  1 +
 include/dt-bindings/pinctrl/k3-am6.h       | 49 ++++++++++++++++++++++
 5 files changed, 75 insertions(+)
 create mode 100644 include/dt-bindings/pinctrl/k3-am6.h

-- 
2.19.1

[PATCH 1/2] dt-bindings: pinctrl: k3-am6: Introduce pinmux definitions

[Vignesh R]

From: Tero Kristo <t-kristo@ti.com>

The dt-bindings header for TI K3-AM6 SoCs define a set of macros for
defining pinmux configs in human readable form, instead of raw-coded
hex values.

Signed-off-by: Tero Kristo <t-kristo@ti.com>
Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
Signed-off-by: Vignesh R <vigneshr@ti.com>
---
 MAINTAINERS                          |  1 +
 include/dt-bindings/pinctrl/k3-am6.h | 49 ++++++++++++++++++++++++++++
 2 files changed, 50 insertions(+)
 create mode 100644 include/dt-bindings/pinctrl/k3-am6.h

diff --git a/MAINTAINERS b/MAINTAINERS
index fb58c64dda49..7fd59955fd21 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2204,6 +2204,7 @@ S:	Supported
 F:	Documentation/devicetree/bindings/arm/ti/k3.txt
 F:	arch/arm64/boot/dts/ti/Makefile
 F:	arch/arm64/boot/dts/ti/k3-*
+F:	include/dt-bindings/pinctrl/k3-am6.h
 
 ARM/TEXAS INSTRUMENT KEYSTONE ARCHITECTURE
 M:	Santosh Shilimkar <ssantosh@kernel.org>
diff --git a/include/dt-bindings/pinctrl/k3-am6.h b/include/dt-bindings/pinctrl/k3-am6.h
new file mode 100644
index 000000000000..42e22ee57600
--- /dev/null
+++ b/include/dt-bindings/pinctrl/k3-am6.h
@@ -0,0 +1,49 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * This header provides constants for TI K3-AM6 pinctrl bindings.
+ *
+ * Copyright (C) 2018 Texas Instruments
+ */
+#ifndef _DT_BINDINGS_PINCTRL_TI_K3_AM6_H
+#define _DT_BINDINGS_PINCTRL_TI_K3_AM6_H
+
+/* K3 mux mode options for each pin. See TRM for options */
+#define MUX_MODE0	0
+#define MUX_MODE1	1
+#define MUX_MODE2	2
+#define MUX_MODE3	3
+#define MUX_MODE4	4
+#define MUX_MODE5	5
+#define MUX_MODE6	6
+#define MUX_MODE7	7
+#define MUX_MODE15	15
+
+#define PULL_DISABLE		(1 << 16)
+#define PULL_UP			(1 << 17)
+#define INPUT_EN		(1 << 18)
+#define SLEWCTRL_200MHZ		0
+#define SLEWCTRL_150MHZ		(1 << 19)
+#define SLEWCTRL_100MHZ		(2 << 19)
+#define SLEWCTRL_50MHZ		(3 << 19)
+#define TX_DIS			(1 << 21)
+#define ISO_OVR			(1 << 22)
+#define ISO_BYPASS		(1 << 23)
+#define DS_EN			(1 << 24)
+#define DS_INPUT		(1 << 25)
+#define DS_FORCE_OUT_HIGH	(1 << 26)
+#define DS_PULL_UP_DOWN_EN	0
+#define DS_PULL_UP_DOWN_DIS	(1 << 27)
+#define DS_PULL_UP_SEL		(1 << 28)
+#define WAKEUP_ENABLE		(1 << 29)
+
+#define PIN_OUTPUT		(PULL_DISABLE)
+#define PIN_OUTPUT_PULLUP	(PULL_UP)
+#define PIN_OUTPUT_PULLDOWN	0
+#define PIN_INPUT		(INPUT_EN | PULL_DISABLE)
+#define PIN_INPUT_PULLUP	(INPUT_EN | PULL_UP)
+#define PIN_INPUT_PULLDOWN	(INPUT_EN)
+
+#define AM65X_IOPAD(pa, val)		(((pa) & 0x1fff)) (val)
+#define AM65X_WKUP_IOPAD(pa, val)	(((pa) & 0x1fff)) (val)
+
+#endif
-- 
2.19.1

[PATCH 1/2] dt-bindings: pinctrl: k3-am6: Introduce pinmux definitions

[Vignesh R]

From: Tero Kristo <t-kristo@ti.com>

The dt-bindings header for TI K3-AM6 SoCs define a set of macros for
defining pinmux configs in human readable form, instead of raw-coded
hex values.

Signed-off-by: Tero Kristo <t-kristo@ti.com>
Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
Signed-off-by: Vignesh R <vigneshr@ti.com>
---
 MAINTAINERS                          |  1 +
 include/dt-bindings/pinctrl/k3-am6.h | 49 ++++++++++++++++++++++++++++
 2 files changed, 50 insertions(+)
 create mode 100644 include/dt-bindings/pinctrl/k3-am6.h

diff --git a/MAINTAINERS b/MAINTAINERS
index fb58c64dda49..7fd59955fd21 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2204,6 +2204,7 @@ S:	Supported
 F:	Documentation/devicetree/bindings/arm/ti/k3.txt
 F:	arch/arm64/boot/dts/ti/Makefile
 F:	arch/arm64/boot/dts/ti/k3-*
+F:	include/dt-bindings/pinctrl/k3-am6.h
 
 ARM/TEXAS INSTRUMENT KEYSTONE ARCHITECTURE
 M:	Santosh Shilimkar <ssantosh@kernel.org>
diff --git a/include/dt-bindings/pinctrl/k3-am6.h b/include/dt-bindings/pinctrl/k3-am6.h
new file mode 100644
index 000000000000..42e22ee57600
--- /dev/null
+++ b/include/dt-bindings/pinctrl/k3-am6.h
@@ -0,0 +1,49 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * This header provides constants for TI K3-AM6 pinctrl bindings.
+ *
+ * Copyright (C) 2018 Texas Instruments
+ */
+#ifndef _DT_BINDINGS_PINCTRL_TI_K3_AM6_H
+#define _DT_BINDINGS_PINCTRL_TI_K3_AM6_H
+
+/* K3 mux mode options for each pin. See TRM for options */
+#define MUX_MODE0	0
+#define MUX_MODE1	1
+#define MUX_MODE2	2
+#define MUX_MODE3	3
+#define MUX_MODE4	4
+#define MUX_MODE5	5
+#define MUX_MODE6	6
+#define MUX_MODE7	7
+#define MUX_MODE15	15
+
+#define PULL_DISABLE		(1 << 16)
+#define PULL_UP			(1 << 17)
+#define INPUT_EN		(1 << 18)
+#define SLEWCTRL_200MHZ		0
+#define SLEWCTRL_150MHZ		(1 << 19)
+#define SLEWCTRL_100MHZ		(2 << 19)
+#define SLEWCTRL_50MHZ		(3 << 19)
+#define TX_DIS			(1 << 21)
+#define ISO_OVR			(1 << 22)
+#define ISO_BYPASS		(1 << 23)
+#define DS_EN			(1 << 24)
+#define DS_INPUT		(1 << 25)
+#define DS_FORCE_OUT_HIGH	(1 << 26)
+#define DS_PULL_UP_DOWN_EN	0
+#define DS_PULL_UP_DOWN_DIS	(1 << 27)
+#define DS_PULL_UP_SEL		(1 << 28)
+#define WAKEUP_ENABLE		(1 << 29)
+
+#define PIN_OUTPUT		(PULL_DISABLE)
+#define PIN_OUTPUT_PULLUP	(PULL_UP)
+#define PIN_OUTPUT_PULLDOWN	0
+#define PIN_INPUT		(INPUT_EN | PULL_DISABLE)
+#define PIN_INPUT_PULLUP	(INPUT_EN | PULL_UP)
+#define PIN_INPUT_PULLDOWN	(INPUT_EN)
+
+#define AM65X_IOPAD(pa, val)		(((pa) & 0x1fff)) (val)
+#define AM65X_WKUP_IOPAD(pa, val)	(((pa) & 0x1fff)) (val)
+
+#endif
-- 
2.19.1

[PATCH 2/2] arm64: dts: ti: k3-am65: Add pinctrl regions

[Vignesh R]

From: Tero Kristo <t-kristo@ti.com>

Add pinctrl regions for the main and wkup mmr.

The range for main pinctrl region contains a gap
at offset 0x2e4, and because of this, the pinctrl
range is split into two sections.

Signed-off-by: Tero Kristo <t-kristo@ti.com>
Signed-off-by: Vignesh R <vigneshr@ti.com>
---
 arch/arm64/boot/dts/ti/k3-am65-main.dtsi   | 16 ++++++++++++++++
 arch/arm64/boot/dts/ti/k3-am65-wakeup.dtsi |  8 ++++++++
 arch/arm64/boot/dts/ti/k3-am65.dtsi        |  1 +
 3 files changed, 25 insertions(+)

diff --git a/arch/arm64/boot/dts/ti/k3-am65-main.dtsi b/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
index adcd6341e40c..f7c2a60d5c80 100644
--- a/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
@@ -69,4 +69,20 @@
 		clock-frequency = <48000000>;
 		current-speed = <115200>;
 	};
+
+	main_pmx0: pinmux@11c000 {
+		compatible = "pinctrl-single";
+		reg = <0x0 0x11c000 0x0 0x2e4>;
+		#pinctrl-cells = <1>;
+		pinctrl-single,register-width = <32>;
+		pinctrl-single,function-mask = <0xffffffff>;
+	};
+
+	main_pmx1: pinmux@11c2e8 {
+		compatible = "pinctrl-single";
+		reg = <0x0 0x11c2e8 0x0 0x24>;
+		#pinctrl-cells = <1>;
+		pinctrl-single,register-width = <32>;
+		pinctrl-single,function-mask = <0xffffffff>;
+	};
 };
diff --git a/arch/arm64/boot/dts/ti/k3-am65-wakeup.dtsi b/arch/arm64/boot/dts/ti/k3-am65-wakeup.dtsi
index 8d7b47f9dfbf..19b46f40789b 100644
--- a/arch/arm64/boot/dts/ti/k3-am65-wakeup.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am65-wakeup.dtsi
@@ -34,6 +34,14 @@
 		};
 	};
 
+	wkup_pmx0: pinmux@4301c000 {
+		compatible = "pinctrl-single";
+		reg = <0x4301c000 0x118>;
+		#pinctrl-cells = <1>;
+		pinctrl-single,register-width = <32>;
+		pinctrl-single,function-mask = <0xffffffff>;
+	};
+
 	wkup_uart0: serial@42300000 {
 		compatible = "ti,am654-uart";
 		reg = <0x42300000 0x100>;
diff --git a/arch/arm64/boot/dts/ti/k3-am65.dtsi b/arch/arm64/boot/dts/ti/k3-am65.dtsi
index 3d4bf369d030..873dca1d0813 100644
--- a/arch/arm64/boot/dts/ti/k3-am65.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am65.dtsi
@@ -8,6 +8,7 @@
 #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-am65.h>
 
 / {
 	model = "Texas Instruments K3 AM654 SoC";
-- 
2.19.1

[PATCH 2/2] arm64: dts: ti: k3-am65: Add pinctrl regions

[Vignesh R]

From: Tero Kristo <t-kristo@ti.com>

Add pinctrl regions for the main and wkup mmr.

The range for main pinctrl region contains a gap
at offset 0x2e4, and because of this, the pinctrl
range is split into two sections.

Signed-off-by: Tero Kristo <t-kristo@ti.com>
Signed-off-by: Vignesh R <vigneshr@ti.com>
---
 arch/arm64/boot/dts/ti/k3-am65-main.dtsi   | 16 ++++++++++++++++
 arch/arm64/boot/dts/ti/k3-am65-wakeup.dtsi |  8 ++++++++
 arch/arm64/boot/dts/ti/k3-am65.dtsi        |  1 +
 3 files changed, 25 insertions(+)

diff --git a/arch/arm64/boot/dts/ti/k3-am65-main.dtsi b/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
index adcd6341e40c..f7c2a60d5c80 100644
--- a/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
@@ -69,4 +69,20 @@
 		clock-frequency = <48000000>;
 		current-speed = <115200>;
 	};
+
+	main_pmx0: pinmux at 11c000 {
+		compatible = "pinctrl-single";
+		reg = <0x0 0x11c000 0x0 0x2e4>;
+		#pinctrl-cells = <1>;
+		pinctrl-single,register-width = <32>;
+		pinctrl-single,function-mask = <0xffffffff>;
+	};
+
+	main_pmx1: pinmux at 11c2e8 {
+		compatible = "pinctrl-single";
+		reg = <0x0 0x11c2e8 0x0 0x24>;
+		#pinctrl-cells = <1>;
+		pinctrl-single,register-width = <32>;
+		pinctrl-single,function-mask = <0xffffffff>;
+	};
 };
diff --git a/arch/arm64/boot/dts/ti/k3-am65-wakeup.dtsi b/arch/arm64/boot/dts/ti/k3-am65-wakeup.dtsi
index 8d7b47f9dfbf..19b46f40789b 100644
--- a/arch/arm64/boot/dts/ti/k3-am65-wakeup.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am65-wakeup.dtsi
@@ -34,6 +34,14 @@
 		};
 	};
 
+	wkup_pmx0: pinmux at 4301c000 {
+		compatible = "pinctrl-single";
+		reg = <0x4301c000 0x118>;
+		#pinctrl-cells = <1>;
+		pinctrl-single,register-width = <32>;
+		pinctrl-single,function-mask = <0xffffffff>;
+	};
+
 	wkup_uart0: serial at 42300000 {
 		compatible = "ti,am654-uart";
 		reg = <0x42300000 0x100>;
diff --git a/arch/arm64/boot/dts/ti/k3-am65.dtsi b/arch/arm64/boot/dts/ti/k3-am65.dtsi
index 3d4bf369d030..873dca1d0813 100644
--- a/arch/arm64/boot/dts/ti/k3-am65.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am65.dtsi
@@ -8,6 +8,7 @@
 #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-am65.h>
 
 / {
 	model = "Texas Instruments K3 AM654 SoC";
-- 
2.19.1

Re: [PATCH 1/2] dt-bindings: pinctrl: k3-am6: Introduce pinmux definitions

[Nishanth Menon]

On 16:56-20181108, Vignesh R wrote:
> From: Tero Kristo <t-kristo@ti.com>
> 
> The dt-bindings header for TI K3-AM6 SoCs define a set of macros for
> defining pinmux configs in human readable form, instead of raw-coded
> hex values.
> 
> Signed-off-by: Tero Kristo <t-kristo@ti.com>
> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
> Signed-off-by: Vignesh R <vigneshr@ti.com>
> ---
>  MAINTAINERS                          |  1 +
>  include/dt-bindings/pinctrl/k3-am6.h | 49 ++++++++++++++++++++++++++++
>  2 files changed, 50 insertions(+)
>  create mode 100644 include/dt-bindings/pinctrl/k3-am6.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index fb58c64dda49..7fd59955fd21 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2204,6 +2204,7 @@ S:	Supported
>  F:	Documentation/devicetree/bindings/arm/ti/k3.txt
>  F:	arch/arm64/boot/dts/ti/Makefile
>  F:	arch/arm64/boot/dts/ti/k3-*
> +F:	include/dt-bindings/pinctrl/k3-am6.h
>  
>  ARM/TEXAS INSTRUMENT KEYSTONE ARCHITECTURE
>  M:	Santosh Shilimkar <ssantosh@kernel.org>
> diff --git a/include/dt-bindings/pinctrl/k3-am6.h b/include/dt-bindings/pinctrl/k3-am6.h
> new file mode 100644
> index 000000000000..42e22ee57600
> --- /dev/null
> +++ b/include/dt-bindings/pinctrl/k3-am6.h

Are we thinking of creating headers for every single SoC?

Is it possible for us to reduce the number of headers we'd need?

> @@ -0,0 +1,49 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * This header provides constants for TI K3-AM6 pinctrl bindings.
> + *
> + * Copyright (C) 2018 Texas Instruments

Please fix this:
Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/

> + */
> +#ifndef _DT_BINDINGS_PINCTRL_TI_K3_AM6_H
> +#define _DT_BINDINGS_PINCTRL_TI_K3_AM6_H
> +
> +/* K3 mux mode options for each pin. See TRM for options */
> +#define MUX_MODE0	0
> +#define MUX_MODE1	1
> +#define MUX_MODE2	2
> +#define MUX_MODE3	3
> +#define MUX_MODE4	4
> +#define MUX_MODE5	5
> +#define MUX_MODE6	6
> +#define MUX_MODE7	7
> +#define MUX_MODE15	15
> +
bit 15?

> +#define PULL_DISABLE		(1 << 16)
PULLUDEN -> this bit enables the Pull
> +#define PULL_UP			(1 << 17)
This is a pull direction selection bit.

so, should these be:
#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 << 18)
#define INPUT_EN (1 << RXACTIVE_SHIFT)

These names dont seem to match up to the http://www.ti.com/lit/pdf/spruid7

This might be readable when people look up trm and try to generate the
pinmux setup.

> +#define SLEWCTRL_200MHZ		0
> +#define SLEWCTRL_150MHZ		(1 << 19)
> +#define SLEWCTRL_100MHZ		(2 << 19)
> +#define SLEWCTRL_50MHZ		(3 << 19)

> +#define TX_DIS			(1 << 21)
What does this mean? Transmit disable? or output disable?

> +#define ISO_OVR			(1 << 22)
> +#define ISO_BYPASS		(1 << 23)
> +#define DS_EN			(1 << 24)
> +#define DS_INPUT		(1 << 25)
Seems to be DSOUT_DIS not the same as input -> in deepsleep,
there is no input - this bit simply means that you are driving an output
or not during deep sleep?

> +#define DS_FORCE_OUT_HIGH	(1 << 26)
you need a FORCE_OUT_LOW as well to be readable.

> +#define DS_PULL_UP_DOWN_EN	0
> +#define DS_PULL_UP_DOWN_DIS	(1 << 27)
> +#define DS_PULL_UP_SEL		(1 << 28)
> +#define WAKEUP_ENABLE		(1 << 29)
I do have a similar set of confusion here as well.
> +
> +#define PIN_OUTPUT		(PULL_DISABLE)
> +#define PIN_OUTPUT_PULLUP	(PULL_UP)
> +#define PIN_OUTPUT_PULLDOWN	0
> +#define PIN_INPUT		(INPUT_EN | PULL_DISABLE)
> +#define PIN_INPUT_PULLUP	(INPUT_EN | PULL_UP)
> +#define PIN_INPUT_PULLDOWN	(INPUT_EN)


Not exactly sure of the approach taken here -> it seems to be a mix of
implicit macro -> there needs to be some level of consistency and
guidance to dts folks as to which macros are to be used and what not.

will be nice if the number of macros are minimal and is clearly
documented as to what macros are expected to be used in dts.

> +
> +#define AM65X_IOPAD(pa, val)		(((pa) & 0x1fff)) (val)
> +#define AM65X_WKUP_IOPAD(pa, val)	(((pa) & 0x1fff)) (val)
> +
> +#endif
> -- 
> 2.19.1
> 

-- 
Regards,
Nishanth Menon

[PATCH 1/2] dt-bindings: pinctrl: k3-am6: Introduce pinmux definitions

[Nishanth Menon]

On 16:56-20181108, Vignesh R wrote:
> From: Tero Kristo <t-kristo@ti.com>
> 
> The dt-bindings header for TI K3-AM6 SoCs define a set of macros for
> defining pinmux configs in human readable form, instead of raw-coded
> hex values.
> 
> Signed-off-by: Tero Kristo <t-kristo@ti.com>
> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
> Signed-off-by: Vignesh R <vigneshr@ti.com>
> ---
>  MAINTAINERS                          |  1 +
>  include/dt-bindings/pinctrl/k3-am6.h | 49 ++++++++++++++++++++++++++++
>  2 files changed, 50 insertions(+)
>  create mode 100644 include/dt-bindings/pinctrl/k3-am6.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index fb58c64dda49..7fd59955fd21 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2204,6 +2204,7 @@ S:	Supported
>  F:	Documentation/devicetree/bindings/arm/ti/k3.txt
>  F:	arch/arm64/boot/dts/ti/Makefile
>  F:	arch/arm64/boot/dts/ti/k3-*
> +F:	include/dt-bindings/pinctrl/k3-am6.h
>  
>  ARM/TEXAS INSTRUMENT KEYSTONE ARCHITECTURE
>  M:	Santosh Shilimkar <ssantosh@kernel.org>
> diff --git a/include/dt-bindings/pinctrl/k3-am6.h b/include/dt-bindings/pinctrl/k3-am6.h
> new file mode 100644
> index 000000000000..42e22ee57600
> --- /dev/null
> +++ b/include/dt-bindings/pinctrl/k3-am6.h

Are we thinking of creating headers for every single SoC?

Is it possible for us to reduce the number of headers we'd need?

> @@ -0,0 +1,49 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * This header provides constants for TI K3-AM6 pinctrl bindings.
> + *
> + * Copyright (C) 2018 Texas Instruments

Please fix this:
Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/

> + */
> +#ifndef _DT_BINDINGS_PINCTRL_TI_K3_AM6_H
> +#define _DT_BINDINGS_PINCTRL_TI_K3_AM6_H
> +
> +/* K3 mux mode options for each pin. See TRM for options */
> +#define MUX_MODE0	0
> +#define MUX_MODE1	1
> +#define MUX_MODE2	2
> +#define MUX_MODE3	3
> +#define MUX_MODE4	4
> +#define MUX_MODE5	5
> +#define MUX_MODE6	6
> +#define MUX_MODE7	7
> +#define MUX_MODE15	15
> +
bit 15?

> +#define PULL_DISABLE		(1 << 16)
PULLUDEN -> this bit enables the Pull
> +#define PULL_UP			(1 << 17)
This is a pull direction selection bit.

so, should these be:
#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 << 18)
#define INPUT_EN (1 << RXACTIVE_SHIFT)

These names dont seem to match up to the http://www.ti.com/lit/pdf/spruid7

This might be readable when people look up trm and try to generate the
pinmux setup.

> +#define SLEWCTRL_200MHZ		0
> +#define SLEWCTRL_150MHZ		(1 << 19)
> +#define SLEWCTRL_100MHZ		(2 << 19)
> +#define SLEWCTRL_50MHZ		(3 << 19)

> +#define TX_DIS			(1 << 21)
What does this mean? Transmit disable? or output disable?

> +#define ISO_OVR			(1 << 22)
> +#define ISO_BYPASS		(1 << 23)
> +#define DS_EN			(1 << 24)
> +#define DS_INPUT		(1 << 25)
Seems to be DSOUT_DIS not the same as input -> in deepsleep,
there is no input - this bit simply means that you are driving an output
or not during deep sleep?

> +#define DS_FORCE_OUT_HIGH	(1 << 26)
you need a FORCE_OUT_LOW as well to be readable.

> +#define DS_PULL_UP_DOWN_EN	0
> +#define DS_PULL_UP_DOWN_DIS	(1 << 27)
> +#define DS_PULL_UP_SEL		(1 << 28)
> +#define WAKEUP_ENABLE		(1 << 29)
I do have a similar set of confusion here as well.
> +
> +#define PIN_OUTPUT		(PULL_DISABLE)
> +#define PIN_OUTPUT_PULLUP	(PULL_UP)
> +#define PIN_OUTPUT_PULLDOWN	0
> +#define PIN_INPUT		(INPUT_EN | PULL_DISABLE)
> +#define PIN_INPUT_PULLUP	(INPUT_EN | PULL_UP)
> +#define PIN_INPUT_PULLDOWN	(INPUT_EN)


Not exactly sure of the approach taken here -> it seems to be a mix of
implicit macro -> there needs to be some level of consistency and
guidance to dts folks as to which macros are to be used and what not.

will be nice if the number of macros are minimal and is clearly
documented as to what macros are expected to be used in dts.

> +
> +#define AM65X_IOPAD(pa, val)		(((pa) & 0x1fff)) (val)
> +#define AM65X_WKUP_IOPAD(pa, val)	(((pa) & 0x1fff)) (val)
> +
> +#endif
> -- 
> 2.19.1
> 

-- 
Regards,
Nishanth Menon

Re: [PATCH 2/2] arm64: dts: ti: k3-am65: Add pinctrl regions

[Nishanth Menon]

On 16:56-20181108, Vignesh R wrote:
> diff --git a/arch/arm64/boot/dts/ti/k3-am65.dtsi b/arch/arm64/boot/dts/ti/k3-am65.dtsi
> index 3d4bf369d030..873dca1d0813 100644
> --- a/arch/arm64/boot/dts/ti/k3-am65.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-am65.dtsi
> @@ -8,6 +8,7 @@
>  #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-am65.h>
>  
>  / {
>  	model = "Texas Instruments K3 AM654 SoC";
> -- 
> 2.19.1
> 

Where is this used? Could we mux the uart console that we are using?
will be good to see an example of usage.

-- 
Regards,
Nishanth Menon

[PATCH 2/2] arm64: dts: ti: k3-am65: Add pinctrl regions

[Nishanth Menon]

On 16:56-20181108, Vignesh R wrote:
> diff --git a/arch/arm64/boot/dts/ti/k3-am65.dtsi b/arch/arm64/boot/dts/ti/k3-am65.dtsi
> index 3d4bf369d030..873dca1d0813 100644
> --- a/arch/arm64/boot/dts/ti/k3-am65.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-am65.dtsi
> @@ -8,6 +8,7 @@
>  #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-am65.h>
>  
>  / {
>  	model = "Texas Instruments K3 AM654 SoC";
> -- 
> 2.19.1
> 

Where is this used? Could we mux the uart console that we are using?
will be good to see an example of usage.

-- 
Regards,
Nishanth Menon

Re: [PATCH 2/2] arm64: dts: ti: k3-am65: Add pinctrl regions

[Tony Lindgren]

* Nishanth Menon <nm@ti.com> [181108 13:48]:
> On 16:56-20181108, Vignesh R wrote:
> > diff --git a/arch/arm64/boot/dts/ti/k3-am65.dtsi b/arch/arm64/boot/dts/ti/k3-am65.dtsi
> > index 3d4bf369d030..873dca1d0813 100644
> > --- a/arch/arm64/boot/dts/ti/k3-am65.dtsi
> > +++ b/arch/arm64/boot/dts/ti/k3-am65.dtsi
> > @@ -8,6 +8,7 @@
> >  #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-am65.h>
> >  
> >  / {
> >  	model = "Texas Instruments K3 AM654 SoC";
> > -- 
> > 2.19.1
> > 
> 
> Where is this used? Could we mux the uart console that we are using?
> will be good to see an example of usage.

Yes please.

And please add the SoC specific macro for the pins in a format
that makes the pinconf and pinmux options separate parameters
for the macro where the usage becomes something like:

AM65X_IOPAD(0x1234, PIN_INPUT_PULLUP, 0)

The above  0 is just the mux mode number in the example.
So no more need to use MUX_MODE0 define to redefine a number
when set up that way :)

This will allow us to eventually take advantage of the
#pinctrl-cells = <2>  and not have to change all the dts files
and just change the macro.

I also added Linus Walleij to Cc as this might interest him
also.

Regards,

Tony

[PATCH 2/2] arm64: dts: ti: k3-am65: Add pinctrl regions

[Tony Lindgren]

* Nishanth Menon <nm@ti.com> [181108 13:48]:
> On 16:56-20181108, Vignesh R wrote:
> > diff --git a/arch/arm64/boot/dts/ti/k3-am65.dtsi b/arch/arm64/boot/dts/ti/k3-am65.dtsi
> > index 3d4bf369d030..873dca1d0813 100644
> > --- a/arch/arm64/boot/dts/ti/k3-am65.dtsi
> > +++ b/arch/arm64/boot/dts/ti/k3-am65.dtsi
> > @@ -8,6 +8,7 @@
> >  #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-am65.h>
> >  
> >  / {
> >  	model = "Texas Instruments K3 AM654 SoC";
> > -- 
> > 2.19.1
> > 
> 
> Where is this used? Could we mux the uart console that we are using?
> will be good to see an example of usage.

Yes please.

And please add the SoC specific macro for the pins in a format
that makes the pinconf and pinmux options separate parameters
for the macro where the usage becomes something like:

AM65X_IOPAD(0x1234, PIN_INPUT_PULLUP, 0)

The above  0 is just the mux mode number in the example.
So no more need to use MUX_MODE0 define to redefine a number
when set up that way :)

This will allow us to eventually take advantage of the
#pinctrl-cells = <2>  and not have to change all the dts files
and just change the macro.

I also added Linus Walleij to Cc as this might interest him
also.

Regards,

Tony

Re: [PATCH 1/2] dt-bindings: pinctrl: k3-am6: Introduce pinmux definitions

[Vignesh R]

On 08/11/18 7:15 PM, Nishanth Menon wrote:
> On 16:56-20181108, Vignesh R wrote:
>> From: Tero Kristo <t-kristo@ti.com>
>>
>> The dt-bindings header for TI K3-AM6 SoCs define a set of macros for
>> defining pinmux configs in human readable form, instead of raw-coded
>> hex values.
>>
>> Signed-off-by: Tero Kristo <t-kristo@ti.com>
>> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
>> Signed-off-by: Vignesh R <vigneshr@ti.com>
>> ---
>>  MAINTAINERS                          |  1 +
>>  include/dt-bindings/pinctrl/k3-am6.h | 49 ++++++++++++++++++++++++++++
>>  2 files changed, 50 insertions(+)
>>  create mode 100644 include/dt-bindings/pinctrl/k3-am6.h
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index fb58c64dda49..7fd59955fd21 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -2204,6 +2204,7 @@ S:	Supported
>>  F:	Documentation/devicetree/bindings/arm/ti/k3.txt
>>  F:	arch/arm64/boot/dts/ti/Makefile
>>  F:	arch/arm64/boot/dts/ti/k3-*
>> +F:	include/dt-bindings/pinctrl/k3-am6.h
>>  
>>  ARM/TEXAS INSTRUMENT KEYSTONE ARCHITECTURE
>>  M:	Santosh Shilimkar <ssantosh@kernel.org>
>> diff --git a/include/dt-bindings/pinctrl/k3-am6.h b/include/dt-bindings/pinctrl/k3-am6.h
>> new file mode 100644
>> index 000000000000..42e22ee57600
>> --- /dev/null
>> +++ b/include/dt-bindings/pinctrl/k3-am6.h
> 
> Are we thinking of creating headers for every single SoC?

We would need one file per SoC family (i.e atleast one for K3 family as
a whole) as pinctrl register layout is significantly different than
other TI SoCs. I can rename the file to include/dt-bindings/pinctrl/k3.h
to indicate its intended to be common for all K3 SoCs, if you prefer.

> 
> Is it possible for us to reduce the number of headers we'd need?
> 
>> @@ -0,0 +1,49 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * This header provides constants for TI K3-AM6 pinctrl bindings.
>> + *
>> + * Copyright (C) 2018 Texas Instruments
> 
> Please fix this:
> Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/
> 

Done.

>> + */
>> +#ifndef _DT_BINDINGS_PINCTRL_TI_K3_AM6_H
>> +#define _DT_BINDINGS_PINCTRL_TI_K3_AM6_H
>> +
>> +/* K3 mux mode options for each pin. See TRM for options */
>> +#define MUX_MODE0	0
>> +#define MUX_MODE1	1
>> +#define MUX_MODE2	2
>> +#define MUX_MODE3	3
>> +#define MUX_MODE4	4
>> +#define MUX_MODE5	5
>> +#define MUX_MODE6	6
>> +#define MUX_MODE7	7
>> +#define MUX_MODE15	15
>> +
> bit 15?
> 

Nope its 0xF. Anyway, dropping these macros as suggested by Tony on
Patch 2/2

>> +#define PULL_DISABLE		(1 << 16)
> PULLUDEN -> this bit enables the Pull
>> +#define PULL_UP			(1 << 17)
> This is a pull direction selection bit.
> 
> so, should these be:
> #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 << 18)
> #define INPUT_EN (1 << RXACTIVE_SHIFT)
> 
> These names dont seem to match up to the http://www.ti.com/lit/pdf/spruid7
> 
> This might be readable when people look up trm and try to generate the
> pinmux setup.
> 

Ok, will update as you suggested above.

>> +#define SLEWCTRL_200MHZ		0
>> +#define SLEWCTRL_150MHZ		(1 << 19)
>> +#define SLEWCTRL_100MHZ		(2 << 19)
>> +#define SLEWCTRL_50MHZ		(3 << 19)
> 
>> +#define TX_DIS			(1 << 21)
> What does this mean? Transmit disable? or output disable?
> 
>> +#define ISO_OVR			(1 << 22)
>> +#define ISO_BYPASS		(1 << 23)
>> +#define DS_EN			(1 << 24)
>> +#define DS_INPUT		(1 << 25)
> Seems to be DSOUT_DIS not the same as input -> in deepsleep,
> there is no input - this bit simply means that you are driving an output
> or not during deep sleep?
> 
>> +#define DS_FORCE_OUT_HIGH	(1 << 26)
> you need a FORCE_OUT_LOW as well to be readable.
> 
>> +#define DS_PULL_UP_DOWN_EN	0
>> +#define DS_PULL_UP_DOWN_DIS	(1 << 27)
>> +#define DS_PULL_UP_SEL		(1 << 28)
>> +#define WAKEUP_ENABLE		(1 << 29)
> I do have a similar set of confusion here as well.

Dropping all the above from v2 as we wont be needing them ATM.

>> +
>> +#define PIN_OUTPUT		(PULL_DISABLE)
>> +#define PIN_OUTPUT_PULLUP	(PULL_UP)
>> +#define PIN_OUTPUT_PULLDOWN	0
>> +#define PIN_INPUT		(INPUT_EN | PULL_DISABLE)
>> +#define PIN_INPUT_PULLUP	(INPUT_EN | PULL_UP)
>> +#define PIN_INPUT_PULLDOWN	(INPUT_EN)
> 
> 
> Not exactly sure of the approach taken here -> it seems to be a mix of
> implicit macro -> there needs to be some level of consistency and
> guidance to dts folks as to which macros are to be used and what not.
> 

Will fix that.

> will be nice if the number of macros are minimal and is clearly
> documented as to what macros are expected to be used in dts.
> 

I will come up with minimal set needed for now and add a comment as to
what macros are intended to be used in DT.

>> +
>> +#define AM65X_IOPAD(pa, val)		(((pa) & 0x1fff)) (val)
>> +#define AM65X_WKUP_IOPAD(pa, val)	(((pa) & 0x1fff)) (val)
>> +
>> +#endif
>> -- 
>> 2.19.1
>>
> 

-- 
Regards
Vignesh

[PATCH 1/2] dt-bindings: pinctrl: k3-am6: Introduce pinmux definitions

[Vignesh R]

On 08/11/18 7:15 PM, Nishanth Menon wrote:
> On 16:56-20181108, Vignesh R wrote:
>> From: Tero Kristo <t-kristo@ti.com>
>>
>> The dt-bindings header for TI K3-AM6 SoCs define a set of macros for
>> defining pinmux configs in human readable form, instead of raw-coded
>> hex values.
>>
>> Signed-off-by: Tero Kristo <t-kristo@ti.com>
>> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
>> Signed-off-by: Vignesh R <vigneshr@ti.com>
>> ---
>>  MAINTAINERS                          |  1 +
>>  include/dt-bindings/pinctrl/k3-am6.h | 49 ++++++++++++++++++++++++++++
>>  2 files changed, 50 insertions(+)
>>  create mode 100644 include/dt-bindings/pinctrl/k3-am6.h
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index fb58c64dda49..7fd59955fd21 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -2204,6 +2204,7 @@ S:	Supported
>>  F:	Documentation/devicetree/bindings/arm/ti/k3.txt
>>  F:	arch/arm64/boot/dts/ti/Makefile
>>  F:	arch/arm64/boot/dts/ti/k3-*
>> +F:	include/dt-bindings/pinctrl/k3-am6.h
>>  
>>  ARM/TEXAS INSTRUMENT KEYSTONE ARCHITECTURE
>>  M:	Santosh Shilimkar <ssantosh@kernel.org>
>> diff --git a/include/dt-bindings/pinctrl/k3-am6.h b/include/dt-bindings/pinctrl/k3-am6.h
>> new file mode 100644
>> index 000000000000..42e22ee57600
>> --- /dev/null
>> +++ b/include/dt-bindings/pinctrl/k3-am6.h
> 
> Are we thinking of creating headers for every single SoC?

We would need one file per SoC family (i.e atleast one for K3 family as
a whole) as pinctrl register layout is significantly different than
other TI SoCs. I can rename the file to include/dt-bindings/pinctrl/k3.h
to indicate its intended to be common for all K3 SoCs, if you prefer.

> 
> Is it possible for us to reduce the number of headers we'd need?
> 
>> @@ -0,0 +1,49 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * This header provides constants for TI K3-AM6 pinctrl bindings.
>> + *
>> + * Copyright (C) 2018 Texas Instruments
> 
> Please fix this:
> Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/
> 

Done.

>> + */
>> +#ifndef _DT_BINDINGS_PINCTRL_TI_K3_AM6_H
>> +#define _DT_BINDINGS_PINCTRL_TI_K3_AM6_H
>> +
>> +/* K3 mux mode options for each pin. See TRM for options */
>> +#define MUX_MODE0	0
>> +#define MUX_MODE1	1
>> +#define MUX_MODE2	2
>> +#define MUX_MODE3	3
>> +#define MUX_MODE4	4
>> +#define MUX_MODE5	5
>> +#define MUX_MODE6	6
>> +#define MUX_MODE7	7
>> +#define MUX_MODE15	15
>> +
> bit 15?
> 

Nope its 0xF. Anyway, dropping these macros as suggested by Tony on
Patch 2/2

>> +#define PULL_DISABLE		(1 << 16)
> PULLUDEN -> this bit enables the Pull
>> +#define PULL_UP			(1 << 17)
> This is a pull direction selection bit.
> 
> so, should these be:
> #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 << 18)
> #define INPUT_EN (1 << RXACTIVE_SHIFT)
> 
> These names dont seem to match up to the http://www.ti.com/lit/pdf/spruid7
> 
> This might be readable when people look up trm and try to generate the
> pinmux setup.
> 

Ok, will update as you suggested above.

>> +#define SLEWCTRL_200MHZ		0
>> +#define SLEWCTRL_150MHZ		(1 << 19)
>> +#define SLEWCTRL_100MHZ		(2 << 19)
>> +#define SLEWCTRL_50MHZ		(3 << 19)
> 
>> +#define TX_DIS			(1 << 21)
> What does this mean? Transmit disable? or output disable?
> 
>> +#define ISO_OVR			(1 << 22)
>> +#define ISO_BYPASS		(1 << 23)
>> +#define DS_EN			(1 << 24)
>> +#define DS_INPUT		(1 << 25)
> Seems to be DSOUT_DIS not the same as input -> in deepsleep,
> there is no input - this bit simply means that you are driving an output
> or not during deep sleep?
> 
>> +#define DS_FORCE_OUT_HIGH	(1 << 26)
> you need a FORCE_OUT_LOW as well to be readable.
> 
>> +#define DS_PULL_UP_DOWN_EN	0
>> +#define DS_PULL_UP_DOWN_DIS	(1 << 27)
>> +#define DS_PULL_UP_SEL		(1 << 28)
>> +#define WAKEUP_ENABLE		(1 << 29)
> I do have a similar set of confusion here as well.

Dropping all the above from v2 as we wont be needing them ATM.

>> +
>> +#define PIN_OUTPUT		(PULL_DISABLE)
>> +#define PIN_OUTPUT_PULLUP	(PULL_UP)
>> +#define PIN_OUTPUT_PULLDOWN	0
>> +#define PIN_INPUT		(INPUT_EN | PULL_DISABLE)
>> +#define PIN_INPUT_PULLUP	(INPUT_EN | PULL_UP)
>> +#define PIN_INPUT_PULLDOWN	(INPUT_EN)
> 
> 
> Not exactly sure of the approach taken here -> it seems to be a mix of
> implicit macro -> there needs to be some level of consistency and
> guidance to dts folks as to which macros are to be used and what not.
> 

Will fix that.

> will be nice if the number of macros are minimal and is clearly
> documented as to what macros are expected to be used in dts.
> 

I will come up with minimal set needed for now and add a comment as to
what macros are intended to be used in DT.

>> +
>> +#define AM65X_IOPAD(pa, val)		(((pa) & 0x1fff)) (val)
>> +#define AM65X_WKUP_IOPAD(pa, val)	(((pa) & 0x1fff)) (val)
>> +
>> +#endif
>> -- 
>> 2.19.1
>>
> 

-- 
Regards
Vignesh

Re: [PATCH 1/2] dt-bindings: pinctrl: k3-am6: Introduce pinmux definitions

[Nishanth Menon]

On 14:20-20181109, Vignesh R wrote:
> >> +++ b/include/dt-bindings/pinctrl/k3-am6.h
> > 
> > Are we thinking of creating headers for every single SoC?
> 
> We would need one file per SoC family (i.e atleast one for K3 family as
> a whole) as pinctrl register layout is significantly different than

Yes, I am aware of the same.

> other TI SoCs. I can rename the file to include/dt-bindings/pinctrl/k3.h
> to indicate its intended to be common for all K3 SoCs, if you prefer.

Yes, that would be the preference. There are two types of IO muxing IPs
(the equivalent of padconf from omap days) that I
am aware of. However these are all within the same 32bit definitions and
I know there has been significant effort spend by designers based on
software team feedbacks to maintain consistency as much as possible
within technological constraints.

So macros could be used to differentiate within the same header. The
deltas do exist, however they can be differentiated easily.

You should have access to the alternative solution, and even if you
cannot publish it, you should plan the header to scale easily when the
time is appropriate as a trivial additional patch.

For the rest.. Will wait for v2.

-- 
Regards,
Nishanth Menon

[PATCH 1/2] dt-bindings: pinctrl: k3-am6: Introduce pinmux definitions

[Nishanth Menon]

On 14:20-20181109, Vignesh R wrote:
> >> +++ b/include/dt-bindings/pinctrl/k3-am6.h
> > 
> > Are we thinking of creating headers for every single SoC?
> 
> We would need one file per SoC family (i.e atleast one for K3 family as
> a whole) as pinctrl register layout is significantly different than

Yes, I am aware of the same.

> other TI SoCs. I can rename the file to include/dt-bindings/pinctrl/k3.h
> to indicate its intended to be common for all K3 SoCs, if you prefer.

Yes, that would be the preference. There are two types of IO muxing IPs
(the equivalent of padconf from omap days) that I
am aware of. However these are all within the same 32bit definitions and
I know there has been significant effort spend by designers based on
software team feedbacks to maintain consistency as much as possible
within technological constraints.

So macros could be used to differentiate within the same header. The
deltas do exist, however they can be differentiated easily.

You should have access to the alternative solution, and even if you
cannot publish it, you should plan the header to scale easily when the
time is appropriate as a trivial additional patch.

For the rest.. Will wait for v2.

-- 
Regards,
Nishanth Menon