[PATCH v2 1/3] dt-bindings: add binding for i.MX8MQ IOMUXC

[PATCH v2 1/3] dt-bindings: add binding for i.MX8MQ IOMUXC

[Lucas Stach]

This adds the binding for the i.MX8MQ pin controller, in the same
fashion as earlier i.MX SoCs.

Signed-off-by: Lucas Stach <l.stach-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
v2: switch to generic pinconf properties
---
 .../bindings/pinctrl/fsl,imx8mq-pinctrl.txt        | 39 ++++++++++++++++++++++
 1 file changed, 39 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pinctrl/fsl,imx8mq-pinctrl.txt

diff --git a/Documentation/devicetree/bindings/pinctrl/fsl,imx8mq-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/fsl,imx8mq-pinctrl.txt
new file mode 100644
index 000000000000..5c5d2d835d05
--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/fsl,imx8mq-pinctrl.txt
@@ -0,0 +1,39 @@
+* Freescale IMX8MQ IOMUX Controller
+
+Please refer to fsl,imx-pinctrl.txt and pinctrl-bindings.txt in this directory
+for common binding part and usage.
+
+=== Pin Controller Node ===
+
+Required properties:
+- compatible: "fsl,imx8mq-iomuxc"
+- reg: Must contain the base physical address and size of the iomuxc registers.
+
+=== Pin Configuration Node ===
+Required properties:
+- pinmux: Array of integersy, representing a group of pins mux setting.
+	The format is pinmux = <PIN_FUNC_ID>, PIN_FUNC_ID is a pin working on
+	a specific function.
+
+	Refer to imx8mq-pinfunc.h in device tree source folder for all available
+	imx8mq PIN_FUNC_ID.
+
+Optional Properties:
+- drive-strength		Integer: controls Drive Strength
+					0: driver disabled
+					1: 255 Ohm
+					2: 105 Ohm
+					3:  75 Ohm
+					4:  85 Ohm
+					5:  65 Ohm
+					6:  45 Ohm
+					7:  40 Ohm
+- slew-rate:			Integer: controls Slew Rate
+					0: Slow
+					1: Medium
+					2: Fast
+					3: Maximum
+- drive-open-drain		Bool: enable Open-drian
+- bias-pull-up			Bool: enable Pull-up
+- input-schmitt-enable		Bool: enable schmitt-trigger
+- input-enable			Bool: enable input, overriding module settings
-- 
2.15.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 2/3] pinctrl: imx: allow to configure SION with generic pinconf

[Lucas Stach]

The SION bit force enables the input buffer, overriding the configuration
driven by the muxed module. It is not located in the pad config register,
but in the mux register and thus needs special handling.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
v2: new patch in v2
---
 drivers/pinctrl/freescale/pinctrl-imx.c | 19 +++++++------------
 drivers/pinctrl/freescale/pinctrl-imx.h |  4 ++++
 2 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/drivers/pinctrl/freescale/pinctrl-imx.c b/drivers/pinctrl/freescale/pinctrl-imx.c
index 24aaddd760a0..c976ffe7d47b 100644
--- a/drivers/pinctrl/freescale/pinctrl-imx.c
+++ b/drivers/pinctrl/freescale/pinctrl-imx.c
@@ -31,10 +31,6 @@
 #include "../pinmux.h"
 #include "pinctrl-imx.h"
 
-/* The bits in CONFIG cell defined in binding doc*/
-#define IMX_NO_PAD_CTL	0x80000000	/* no pin config need */
-#define IMX_PAD_SION 0x40000000		/* set SION */
-
 static inline const struct group_desc *imx_pinctrl_find_group_by_name(
 				struct pinctrl_dev *pctldev,
 				const char *name)
@@ -509,18 +505,17 @@ static int imx_pinctrl_parse_groups(struct device_node *np,
 		pin->mux_mode = be32_to_cpu(*list++);
 		pin->input_val = be32_to_cpu(*list++);
 
-		if (info->generic_pinconf) {
+		if (info->generic_pinconf)
 			/* generic pin config decoded */
 			pin->config = config;
-		} else {
+		else
 			/* legacy pin config read from devicetree */
-			config = be32_to_cpu(*list++);
+			pin->config = be32_to_cpu(*list++);
 
-			/* SION bit is in mux register */
-			if (config & IMX_PAD_SION)
-				pin->mux_mode |= IOMUXC_CONFIG_SION;
-			pin->config = config & ~IMX_PAD_SION;
-		}
+		/* SION bit is in mux register */
+		if (pin->config & IMX_PAD_SION)
+			pin->mux_mode |= IOMUXC_CONFIG_SION;
+		pin->config = pin->config & ~IMX_PAD_SION;
 
 		dev_dbg(ipctl->dev, "%s: 0x%x 0x%08lx", info->pins[pin_id].name,
 				pin->mux_mode, pin->config);
diff --git a/drivers/pinctrl/freescale/pinctrl-imx.h b/drivers/pinctrl/freescale/pinctrl-imx.h
index 038e8c0e5b96..8e74ddc9cd8a 100644
--- a/drivers/pinctrl/freescale/pinctrl-imx.h
+++ b/drivers/pinctrl/freescale/pinctrl-imx.h
@@ -18,6 +18,10 @@
 #include <linux/pinctrl/pinconf-generic.h>
 #include <linux/pinctrl/pinmux.h>
 
+/* The bits in CONFIG cell defined in binding doc*/
+#define IMX_NO_PAD_CTL	0x80000000	/* no pin config need */
+#define IMX_PAD_SION 0x40000000		/* set SION */
+
 struct platform_device;
 
 extern struct pinmux_ops imx_pmx_ops;
-- 
2.15.1

[PATCH v2 3/3] pinctrl: imx: add driver for i.MX8MQ

[Lucas Stach]

The i.MX8MQ pincontrol works in a similar way to the earlier i.MX SoCs.
This driver builds on top of the generic pinconf handling introduced
with the i.MX7ULP pinctrl driver.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
v2:
- declare config structures as const
- switch to generic pinconf
---
 drivers/pinctrl/freescale/Kconfig          |   7 +
 drivers/pinctrl/freescale/Makefile         |   1 +
 drivers/pinctrl/freescale/pinctrl-imx8mq.c | 368 +++++++++++++++++++++++++++++
 3 files changed, 376 insertions(+)
 create mode 100644 drivers/pinctrl/freescale/pinctrl-imx8mq.c

diff --git a/drivers/pinctrl/freescale/Kconfig b/drivers/pinctrl/freescale/Kconfig
index 4dbc576ae27c..b0b62964e8ac 100644
--- a/drivers/pinctrl/freescale/Kconfig
+++ b/drivers/pinctrl/freescale/Kconfig
@@ -110,6 +110,13 @@ config PINCTRL_IMX7ULP
 	help
 	  Say Y here to enable the imx7ulp pinctrl driver
 
+config PINCTRL_IMX8MQ
+	bool "IMX8MQ pinctrl driver"
+	depends on SOC_IMX8MQ
+	select PINCTRL_IMX
+	help
+	  Say Y here to enable the imx8mq pinctrl driver
+
 config PINCTRL_VF610
 	bool "Freescale Vybrid VF610 pinctrl driver"
 	depends on SOC_VF610
diff --git a/drivers/pinctrl/freescale/Makefile b/drivers/pinctrl/freescale/Makefile
index 19bb9a55a567..2b2b89bb8454 100644
--- a/drivers/pinctrl/freescale/Makefile
+++ b/drivers/pinctrl/freescale/Makefile
@@ -16,6 +16,7 @@ obj-$(CONFIG_PINCTRL_IMX6SX)	+= pinctrl-imx6sx.o
 obj-$(CONFIG_PINCTRL_IMX6UL)	+= pinctrl-imx6ul.o
 obj-$(CONFIG_PINCTRL_IMX7D)	+= pinctrl-imx7d.o
 obj-$(CONFIG_PINCTRL_IMX7ULP)	+= pinctrl-imx7ulp.o
+obj-$(CONFIG_PINCTRL_IMX8MQ)	+= pinctrl-imx8mq.o
 obj-$(CONFIG_PINCTRL_VF610)	+= pinctrl-vf610.o
 obj-$(CONFIG_PINCTRL_MXS)	+= pinctrl-mxs.o
 obj-$(CONFIG_PINCTRL_IMX23)	+= pinctrl-imx23.o
diff --git a/drivers/pinctrl/freescale/pinctrl-imx8mq.c b/drivers/pinctrl/freescale/pinctrl-imx8mq.c
new file mode 100644
index 000000000000..e6af506dfb60
--- /dev/null
+++ b/drivers/pinctrl/freescale/pinctrl-imx8mq.c
@@ -0,0 +1,368 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2016 Freescale Semiconductor, Inc.
+ * Copyright 2017 NXP
+ * Copyright (C) 2018 Pengutronix, Lucas Stach <kernel@pengutronix.de>
+ */
+
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/pinctrl/pinctrl.h>
+
+#include "pinctrl-imx.h"
+
+enum imx8mq_pads {
+	MX8MQ_PAD_RESERVE0 = 0,
+	MX8MQ_PAD_RESERVE1 = 1,
+	MX8MQ_PAD_RESERVE2 = 2,
+	MX8MQ_PAD_RESERVE3 = 3,
+	MX8MQ_PAD_RESERVE4 = 4,
+	MX8MQ_IOMUXC_PMIC_STBY_REQ_CCMSRCGPCMIX = 5,
+	MX8MQ_IOMUXC_PMIC_ON_REQ_SNVSMIX = 6,
+	MX8MQ_IOMUXC_ONOFF_SNVSMIX = 7,
+	MX8MQ_IOMUXC_POR_B_SNVSMIX = 8,
+	MX8MQ_IOMUXC_RTC_RESET_B_SNVSMIX = 9,
+	MX8MQ_IOMUXC_GPIO1_IO00 = 10,
+	MX8MQ_IOMUXC_GPIO1_IO01 = 11,
+	MX8MQ_IOMUXC_GPIO1_IO02 = 12,
+	MX8MQ_IOMUXC_GPIO1_IO03 = 13,
+	MX8MQ_IOMUXC_GPIO1_IO04 = 14,
+	MX8MQ_IOMUXC_GPIO1_IO05 = 15,
+	MX8MQ_IOMUXC_GPIO1_IO06 = 16,
+	MX8MQ_IOMUXC_GPIO1_IO07 = 17,
+	MX8MQ_IOMUXC_GPIO1_IO08 = 18,
+	MX8MQ_IOMUXC_GPIO1_IO09 = 19,
+	MX8MQ_IOMUXC_GPIO1_IO10 = 20,
+	MX8MQ_IOMUXC_GPIO1_IO11 = 21,
+	MX8MQ_IOMUXC_GPIO1_IO12 = 22,
+	MX8MQ_IOMUXC_GPIO1_IO13 = 23,
+	MX8MQ_IOMUXC_GPIO1_IO14 = 24,
+	MX8MQ_IOMUXC_GPIO1_IO15 = 25,
+	MX8MQ_IOMUXC_ENET_MDC = 26,
+	MX8MQ_IOMUXC_ENET_MDIO = 27,
+	MX8MQ_IOMUXC_ENET_TD3 = 28,
+	MX8MQ_IOMUXC_ENET_TD2 = 29,
+	MX8MQ_IOMUXC_ENET_TD1 = 30,
+	MX8MQ_IOMUXC_ENET_TD0 = 31,
+	MX8MQ_IOMUXC_ENET_TX_CTL = 32,
+	MX8MQ_IOMUXC_ENET_TXC = 33,
+	MX8MQ_IOMUXC_ENET_RX_CTL = 34,
+	MX8MQ_IOMUXC_ENET_RXC = 35,
+	MX8MQ_IOMUXC_ENET_RD0 = 36,
+	MX8MQ_IOMUXC_ENET_RD1 = 37,
+	MX8MQ_IOMUXC_ENET_RD2 = 38,
+	MX8MQ_IOMUXC_ENET_RD3 = 39,
+	MX8MQ_IOMUXC_SD1_CLK = 40,
+	MX8MQ_IOMUXC_SD1_CMD = 41,
+	MX8MQ_IOMUXC_SD1_DATA0 = 42,
+	MX8MQ_IOMUXC_SD1_DATA1 = 43,
+	MX8MQ_IOMUXC_SD1_DATA2 = 44,
+	MX8MQ_IOMUXC_SD1_DATA3 = 45,
+	MX8MQ_IOMUXC_SD1_DATA4 = 46,
+	MX8MQ_IOMUXC_SD1_DATA5 = 47,
+	MX8MQ_IOMUXC_SD1_DATA6 = 48,
+	MX8MQ_IOMUXC_SD1_DATA7 = 49,
+	MX8MQ_IOMUXC_SD1_RESET_B = 50,
+	MX8MQ_IOMUXC_SD1_STROBE = 51,
+	MX8MQ_IOMUXC_SD2_CD_B = 52,
+	MX8MQ_IOMUXC_SD2_CLK = 53,
+	MX8MQ_IOMUXC_SD2_CMD = 54,
+	MX8MQ_IOMUXC_SD2_DATA0 = 55,
+	MX8MQ_IOMUXC_SD2_DATA1 = 56,
+	MX8MQ_IOMUXC_SD2_DATA2 = 57,
+	MX8MQ_IOMUXC_SD2_DATA3 = 58,
+	MX8MQ_IOMUXC_SD2_RESET_B = 59,
+	MX8MQ_IOMUXC_SD2_WP = 60,
+	MX8MQ_IOMUXC_NAND_ALE = 61,
+	MX8MQ_IOMUXC_NAND_CE0_B = 62,
+	MX8MQ_IOMUXC_NAND_CE1_B = 63,
+	MX8MQ_IOMUXC_NAND_CE2_B = 64,
+	MX8MQ_IOMUXC_NAND_CE3_B = 65,
+	MX8MQ_IOMUXC_NAND_CLE = 66,
+	MX8MQ_IOMUXC_NAND_DATA00 = 67,
+	MX8MQ_IOMUXC_NAND_DATA01 = 68,
+	MX8MQ_IOMUXC_NAND_DATA02 = 69,
+	MX8MQ_IOMUXC_NAND_DATA03 = 70,
+	MX8MQ_IOMUXC_NAND_DATA04 = 71,
+	MX8MQ_IOMUXC_NAND_DATA05 = 72,
+	MX8MQ_IOMUXC_NAND_DATA06 = 73,
+	MX8MQ_IOMUXC_NAND_DATA07 = 74,
+	MX8MQ_IOMUXC_NAND_DQS = 75,
+	MX8MQ_IOMUXC_NAND_RE_B = 76,
+	MX8MQ_IOMUXC_NAND_READY_B = 77,
+	MX8MQ_IOMUXC_NAND_WE_B = 78,
+	MX8MQ_IOMUXC_NAND_WP_B = 79,
+	MX8MQ_IOMUXC_SAI5_RXFS = 80,
+	MX8MQ_IOMUXC_SAI5_RXC = 81,
+	MX8MQ_IOMUXC_SAI5_RXD0 = 82,
+	MX8MQ_IOMUXC_SAI5_RXD1 = 83,
+	MX8MQ_IOMUXC_SAI5_RXD2 = 84,
+	MX8MQ_IOMUXC_SAI5_RXD3 = 85,
+	MX8MQ_IOMUXC_SAI5_MCLK = 86,
+	MX8MQ_IOMUXC_SAI1_RXFS = 87,
+	MX8MQ_IOMUXC_SAI1_RXC = 88,
+	MX8MQ_IOMUXC_SAI1_RXD0 = 89,
+	MX8MQ_IOMUXC_SAI1_RXD1 = 90,
+	MX8MQ_IOMUXC_SAI1_RXD2 = 91,
+	MX8MQ_IOMUXC_SAI1_RXD3 = 92,
+	MX8MQ_IOMUXC_SAI1_RXD4 = 93,
+	MX8MQ_IOMUXC_SAI1_RXD5 = 94,
+	MX8MQ_IOMUXC_SAI1_RXD6 = 95,
+	MX8MQ_IOMUXC_SAI1_RXD7 = 96,
+	MX8MQ_IOMUXC_SAI1_TXFS = 97,
+	MX8MQ_IOMUXC_SAI1_TXC = 98,
+	MX8MQ_IOMUXC_SAI1_TXD0 = 99,
+	MX8MQ_IOMUXC_SAI1_TXD1 = 100,
+	MX8MQ_IOMUXC_SAI1_TXD2 = 101,
+	MX8MQ_IOMUXC_SAI1_TXD3 = 102,
+	MX8MQ_IOMUXC_SAI1_TXD4 = 103,
+	MX8MQ_IOMUXC_SAI1_TXD5 = 104,
+	MX8MQ_IOMUXC_SAI1_TXD6 = 105,
+	MX8MQ_IOMUXC_SAI1_TXD7 = 106,
+	MX8MQ_IOMUXC_SAI1_MCLK = 107,
+	MX8MQ_IOMUXC_SAI2_RXFS = 108,
+	MX8MQ_IOMUXC_SAI2_RXC = 109,
+	MX8MQ_IOMUXC_SAI2_RXD0 = 110,
+	MX8MQ_IOMUXC_SAI2_TXFS = 111,
+	MX8MQ_IOMUXC_SAI2_TXC = 112,
+	MX8MQ_IOMUXC_SAI2_TXD0 = 113,
+	MX8MQ_IOMUXC_SAI2_MCLK = 114,
+	MX8MQ_IOMUXC_SAI3_RXFS = 115,
+	MX8MQ_IOMUXC_SAI3_RXC = 116,
+	MX8MQ_IOMUXC_SAI3_RXD = 117,
+	MX8MQ_IOMUXC_SAI3_TXFS = 118,
+	MX8MQ_IOMUXC_SAI3_TXC = 119,
+	MX8MQ_IOMUXC_SAI3_TXD = 120,
+	MX8MQ_IOMUXC_SAI3_MCLK = 121,
+	MX8MQ_IOMUXC_SPDIF_TX = 122,
+	MX8MQ_IOMUXC_SPDIF_RX = 123,
+	MX8MQ_IOMUXC_SPDIF_EXT_CLK = 124,
+	MX8MQ_IOMUXC_ECSPI1_SCLK = 125,
+	MX8MQ_IOMUXC_ECSPI1_MOSI = 126,
+	MX8MQ_IOMUXC_ECSPI1_MISO = 127,
+	MX8MQ_IOMUXC_ECSPI1_SS0 = 128,
+	MX8MQ_IOMUXC_ECSPI2_SCLK = 129,
+	MX8MQ_IOMUXC_ECSPI2_MOSI = 130,
+	MX8MQ_IOMUXC_ECSPI2_MISO = 131,
+	MX8MQ_IOMUXC_ECSPI2_SS0 = 132,
+	MX8MQ_IOMUXC_I2C1_SCL = 133,
+	MX8MQ_IOMUXC_I2C1_SDA = 134,
+	MX8MQ_IOMUXC_I2C2_SCL = 135,
+	MX8MQ_IOMUXC_I2C2_SDA = 136,
+	MX8MQ_IOMUXC_I2C3_SCL = 137,
+	MX8MQ_IOMUXC_I2C3_SDA = 138,
+	MX8MQ_IOMUXC_I2C4_SCL = 139,
+	MX8MQ_IOMUXC_I2C4_SDA = 140,
+	MX8MQ_IOMUXC_UART1_RXD = 141,
+	MX8MQ_IOMUXC_UART1_TXD = 142,
+	MX8MQ_IOMUXC_UART2_RXD = 143,
+	MX8MQ_IOMUXC_UART2_TXD = 144,
+	MX8MQ_IOMUXC_UART3_RXD = 145,
+	MX8MQ_IOMUXC_UART3_TXD = 146,
+	MX8MQ_IOMUXC_UART4_RXD = 147,
+	MX8MQ_IOMUXC_UART4_TXD = 148,
+};
+
+/* Pad names for the pinmux subsystem */
+static const struct pinctrl_pin_desc imx8mq_pinctrl_pads[] = {
+	IMX_PINCTRL_PIN(MX8MQ_PAD_RESERVE0),
+	IMX_PINCTRL_PIN(MX8MQ_PAD_RESERVE1),
+	IMX_PINCTRL_PIN(MX8MQ_PAD_RESERVE2),
+	IMX_PINCTRL_PIN(MX8MQ_PAD_RESERVE3),
+	IMX_PINCTRL_PIN(MX8MQ_PAD_RESERVE4),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_PMIC_STBY_REQ_CCMSRCGPCMIX),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_PMIC_ON_REQ_SNVSMIX),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_ONOFF_SNVSMIX),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_POR_B_SNVSMIX),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_RTC_RESET_B_SNVSMIX),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_GPIO1_IO00),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_GPIO1_IO01),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_GPIO1_IO02),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_GPIO1_IO03),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_GPIO1_IO04),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_GPIO1_IO05),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_GPIO1_IO06),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_GPIO1_IO07),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_GPIO1_IO08),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_GPIO1_IO09),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_GPIO1_IO10),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_GPIO1_IO11),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_GPIO1_IO12),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_GPIO1_IO13),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_GPIO1_IO14),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_GPIO1_IO15),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_ENET_MDC),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_ENET_MDIO),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_ENET_TD3),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_ENET_TD2),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_ENET_TD1),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_ENET_TD0),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_ENET_TX_CTL),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_ENET_TXC),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_ENET_RX_CTL),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_ENET_RXC),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_ENET_RD0),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_ENET_RD1),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_ENET_RD2),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_ENET_RD3),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SD1_CLK),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SD1_CMD),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SD1_DATA0),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SD1_DATA1),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SD1_DATA2),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SD1_DATA3),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SD1_DATA4),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SD1_DATA5),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SD1_DATA6),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SD1_DATA7),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SD1_RESET_B),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SD1_STROBE),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SD2_CD_B),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SD2_CLK),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SD2_CMD),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SD2_DATA0),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SD2_DATA1),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SD2_DATA2),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SD2_DATA3),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SD2_RESET_B),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SD2_WP),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_NAND_ALE),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_NAND_CE0_B),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_NAND_CE1_B),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_NAND_CE2_B),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_NAND_CE3_B),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_NAND_CLE),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_NAND_DATA00),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_NAND_DATA01),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_NAND_DATA02),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_NAND_DATA03),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_NAND_DATA04),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_NAND_DATA05),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_NAND_DATA06),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_NAND_DATA07),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_NAND_DQS),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_NAND_RE_B),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_NAND_READY_B),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_NAND_WE_B),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_NAND_WP_B),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SAI5_RXFS),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SAI5_RXC),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SAI5_RXD0),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SAI5_RXD1),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SAI5_RXD2),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SAI5_RXD3),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SAI5_MCLK),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SAI1_RXFS),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SAI1_RXC),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SAI1_RXD0),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SAI1_RXD1),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SAI1_RXD2),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SAI1_RXD3),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SAI1_RXD4),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SAI1_RXD5),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SAI1_RXD6),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SAI1_RXD7),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SAI1_TXFS),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SAI1_TXC),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SAI1_TXD0),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SAI1_TXD1),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SAI1_TXD2),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SAI1_TXD3),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SAI1_TXD4),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SAI1_TXD5),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SAI1_TXD6),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SAI1_TXD7),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SAI1_MCLK),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SAI2_RXFS),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SAI2_RXC),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SAI2_RXD0),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SAI2_TXFS),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SAI2_TXC),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SAI2_TXD0),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SAI2_MCLK),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SAI3_RXFS),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SAI3_RXC),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SAI3_RXD),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SAI3_TXFS),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SAI3_TXC),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SAI3_TXD),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SAI3_MCLK),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SPDIF_TX),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SPDIF_RX),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_SPDIF_EXT_CLK),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_ECSPI1_SCLK),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_ECSPI1_MOSI),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_ECSPI1_MISO),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_ECSPI1_SS0),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_ECSPI2_SCLK),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_ECSPI2_MOSI),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_ECSPI2_MISO),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_ECSPI2_SS0),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_I2C1_SCL),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_I2C1_SDA),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_I2C2_SCL),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_I2C2_SDA),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_I2C3_SCL),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_I2C3_SDA),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_I2C4_SCL),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_I2C4_SDA),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_UART1_RXD),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_UART1_TXD),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_UART2_RXD),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_UART2_TXD),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_UART3_RXD),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_UART3_TXD),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_UART4_RXD),
+	IMX_PINCTRL_PIN(MX8MQ_IOMUXC_UART4_TXD),
+};
+
+static const struct imx_cfg_params_decode imx8mq_cfg_decodes[] = {
+	IMX_CFG_PARAMS_DECODE(PIN_CONFIG_DRIVE_STRENGTH,		0x07, 0),
+	IMX_CFG_PARAMS_DECODE(PIN_CONFIG_SLEW_RATE,			0x18, 3),
+	IMX_CFG_PARAMS_DECODE_INVERT(PIN_CONFIG_DRIVE_OPEN_DRAIN,	0x20, 5),
+	IMX_CFG_PARAMS_DECODE(PIN_CONFIG_BIAS_PULL_UP,			0x40, 6),
+	IMX_CFG_PARAMS_DECODE(PIN_CONFIG_INPUT_SCHMITT_ENABLE,		0x80, 7),
+	IMX_CFG_PARAMS_DECODE(PIN_CONFIG_INPUT_ENABLE,         IMX_PAD_SION, 30),
+};
+
+static const struct imx_pinctrl_soc_info imx8mq_pinctrl_info = {
+	.pins = imx8mq_pinctrl_pads,
+	.npins = ARRAY_SIZE(imx8mq_pinctrl_pads),
+	.gpr_compatible = "fsl,imx8mq-iomuxc-gpr",
+	.generic_pinconf = true,
+	.decodes = imx8mq_cfg_decodes,
+	.num_decodes = ARRAY_SIZE(imx8mq_cfg_decodes),
+};
+
+static const struct of_device_id imx8mq_pinctrl_of_match[] = {
+	{ .compatible = "fsl,imx8mq-iomuxc", .data = &imx8mq_pinctrl_info, },
+	{ /* sentinel */ }
+};
+
+static int imx8mq_pinctrl_probe(struct platform_device *pdev)
+{
+	const struct imx_pinctrl_soc_info *pinctrl_info;
+
+	pinctrl_info = of_device_get_match_data(&pdev->dev);
+	if (!pinctrl_info)
+		return -ENODEV;
+
+	return imx_pinctrl_probe(pdev, pinctrl_info);
+}
+
+static struct platform_driver imx8mq_pinctrl_driver = {
+	.driver = {
+		.name = "imx8mq-pinctrl",
+		.of_match_table = of_match_ptr(imx8mq_pinctrl_of_match),
+	},
+	.probe = imx8mq_pinctrl_probe,
+};
+
+static int __init imx8mq_pinctrl_init(void)
+{
+	return platform_driver_register(&imx8mq_pinctrl_driver);
+}
+arch_initcall(imx8mq_pinctrl_init);
-- 
2.15.1

Re: [PATCH v2 1/3] dt-bindings: add binding for i.MX8MQ IOMUXC

[Rob Herring]

On Thu, Feb 01, 2018 at 06:49:21PM +0100, Lucas Stach wrote:
> This adds the binding for the i.MX8MQ pin controller, in the same
> fashion as earlier i.MX SoCs.
> 
> Signed-off-by: Lucas Stach <l.stach-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> ---
> v2: switch to generic pinconf properties
> ---
>  .../bindings/pinctrl/fsl,imx8mq-pinctrl.txt        | 39 ++++++++++++++++++++++
>  1 file changed, 39 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pinctrl/fsl,imx8mq-pinctrl.txt
> 
> diff --git a/Documentation/devicetree/bindings/pinctrl/fsl,imx8mq-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/fsl,imx8mq-pinctrl.txt
> new file mode 100644
> index 000000000000..5c5d2d835d05
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/fsl,imx8mq-pinctrl.txt
> @@ -0,0 +1,39 @@
> +* Freescale IMX8MQ IOMUX Controller
> +
> +Please refer to fsl,imx-pinctrl.txt and pinctrl-bindings.txt in this directory
> +for common binding part and usage.
> +
> +=== Pin Controller Node ===
> +
> +Required properties:
> +- compatible: "fsl,imx8mq-iomuxc"
> +- reg: Must contain the base physical address and size of the iomuxc registers.
> +
> +=== Pin Configuration Node ===
> +Required properties:
> +- pinmux: Array of integersy, representing a group of pins mux setting.
> +	The format is pinmux = <PIN_FUNC_ID>, PIN_FUNC_ID is a pin working on
> +	a specific function.
> +
> +	Refer to imx8mq-pinfunc.h in device tree source folder for all available
> +	imx8mq PIN_FUNC_ID.
> +
> +Optional Properties:
> +- drive-strength		Integer: controls Drive Strength

I thought drive-strength is supposed to be be in mA. We should not have 
differing units in common properties. 

There was an Atmel binding the other day that this came up. 

> +					0: driver disabled
> +					1: 255 Ohm
> +					2: 105 Ohm
> +					3:  75 Ohm
> +					4:  85 Ohm
> +					5:  65 Ohm
> +					6:  45 Ohm
> +					7:  40 Ohm
> +- slew-rate:			Integer: controls Slew Rate
> +					0: Slow
> +					1: Medium
> +					2: Fast
> +					3: Maximum
> +- drive-open-drain		Bool: enable Open-drian
> +- bias-pull-up			Bool: enable Pull-up
> +- input-schmitt-enable		Bool: enable schmitt-trigger
> +- input-enable			Bool: enable input, overriding module settings
> -- 
> 2.15.1
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/3] dt-bindings: add binding for i.MX8MQ IOMUXC

[Lucas Stach]

Am Montag, den 05.02.2018, 00:09 -0600 schrieb Rob Herring:
> On Thu, Feb 01, 2018 at 06:49:21PM +0100, Lucas Stach wrote:
> > This adds the binding for the i.MX8MQ pin controller, in the same
> > fashion as earlier i.MX SoCs.
> > 
> > > > Signed-off-by: Lucas Stach <l.stach-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > ---
> > v2: switch to generic pinconf properties
> > ---
> >  .../bindings/pinctrl/fsl,imx8mq-pinctrl.txt        | 39 ++++++++++++++++++++++
> >  1 file changed, 39 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/pinctrl/fsl,imx8mq-pinctrl.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/pinctrl/fsl,imx8mq-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/fsl,imx8mq-pinctrl.txt
> > new file mode 100644
> > index 000000000000..5c5d2d835d05
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pinctrl/fsl,imx8mq-pinctrl.txt
> > @@ -0,0 +1,39 @@
> > +* Freescale IMX8MQ IOMUX Controller
> > +
> > +Please refer to fsl,imx-pinctrl.txt and pinctrl-bindings.txt in this directory
> > +for common binding part and usage.
> > +
> > +=== Pin Controller Node ===
> > +
> > +Required properties:
> > +- compatible: "fsl,imx8mq-iomuxc"
> > +- reg: Must contain the base physical address and size of the iomuxc registers.
> > +
> > +=== Pin Configuration Node ===
> > +Required properties:
> > +- pinmux: Array of integersy, representing a group of pins mux setting.
> > > > +	The format is pinmux = <PIN_FUNC_ID>, PIN_FUNC_ID is a pin working on
> > > > +	a specific function.
> > +
> > > > +	Refer to imx8mq-pinfunc.h in device tree source folder for all available
> > > > +	imx8mq PIN_FUNC_ID.
> > +
> > +Optional Properties:
> > +- drive-strength		Integer: controls Drive Strength
> 
> I thought drive-strength is supposed to be be in mA. We should not have 
> differing units in common properties. 
> 
> There was an Atmel binding the other day that this came up.

I know it's in the common binding, but it's going to be very hard to
support. The drive-strength in mA really depends on external loading of
the pin and the pin voltage, at least for the most widespread
controllers that only switch between resistors, instead of driving the
pin by a current regulator.
So we would need much more information, which doesn't really seem
practical.

I would much prefer if we just remove unit from the common binding and
let the drivers define it in a way that fits their hardware
description.

It's the same thing with the slew-rate really, but the common binding
didn't make the mistake to define a single unit there, with many
pincontrollers being really vague about how fast the slew really is,
but some may actually define it in uS or whatever else makes sense.

Regards,
Lucas
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/3] dt-bindings: add binding for i.MX8MQ IOMUXC

[Linus Walleij]

On Mon, Feb 5, 2018 at 11:09 AM, Lucas Stach <l.stach-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> wrote:

>> > +Optional Properties:
>> > +- drive-strength           Integer: controls Drive Strength
>>
>> I thought drive-strength is supposed to be be in mA. We should not have
>> differing units in common properties.
>>
>> There was an Atmel binding the other day that this came up.
>
> I know it's in the common binding, but it's going to be very hard to
> support. The drive-strength in mA really depends on external loading of
> the pin and the pin voltage, at least for the most widespread
> controllers that only switch between resistors, instead of driving the
> pin by a current regulator.

How does this circuit really look? Can you sketch something so
we understand what is going on electronically here?

The way I imagine most drive strength out there works is by simply
connection more MOS-totempoles after each other and each of them
has a production-technology-related max output current like 4mA, so
by putting two in a series yoy double that to 8 mA.

Is that what you mean with "current regulator"?

If you mean rather "output impedance", that is something else and
something we need a new (generic) property for. I never saw that
before, really.

> I would much prefer if we just remove unit from the common binding and
> let the drivers define it in a way that fits their hardware
> description.

Yeah but is this really the same thing even ...

> It's the same thing with the slew-rate really, but the common binding
> didn't make the mistake to define a single unit there, with many
> pincontrollers being really vague about how fast the slew really is,
> but some may actually define it in uS or whatever else makes sense.

I agree with that. Also for skewrate and output time delays
(skew delay).

It's very unclear how cell libraries really implement this.
But with drive strength I thought we knew it ...

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/3] dt-bindings: add binding for i.MX8MQ IOMUXC

[Lucas Stach]

Hi Linus,

Am Dienstag, den 06.02.2018, 00:13 +0100 schrieb Linus Walleij:
> > On Mon, Feb 5, 2018 at 11:09 AM, Lucas Stach <l.stach@pengutronix.de> wrote:
> 
> > > > +Optional Properties:
> > > > +- drive-strength           Integer: controls Drive Strength
> > > 
> > > I thought drive-strength is supposed to be be in mA. We should not have
> > > differing units in common properties.
> > > 
> > > There was an Atmel binding the other day that this came up.
> > 
> > I know it's in the common binding, but it's going to be very hard to
> > support. The drive-strength in mA really depends on external loading of
> > the pin and the pin voltage, at least for the most widespread
> > controllers that only switch between resistors, instead of driving the
> > pin by a current regulator.
> 
> How does this circuit really look? Can you sketch something so
> we understand what is going on electronically here?

There is no description of how the circuit really looks like and I
would very much prefer this not being a prerequisite to get some
software support in place.

The best description we have is from NXP AN5078:
"The drive strength enable (DSE) can be explained as series resistance
between an ideal driver's output and its load. To achieve maximal
transferred power, the impedance of the driver has to match the load
impedance."

Which means the control bits don't really vary the output impedance,
but the description models it like that. If we _assume_ the simple MOS
totempole setup then adding more totempoles will bring down the output
impedance.

> The way I imagine most drive strength out there works is by simply
> connection more MOS-totempoles after each other and each of them
> has a production-technology-related max output current like 4mA, so
> by putting two in a series yoy double that to 8 mA.

So with this description in mind the drive-strength property doesn't
describe an actual real world drive-strength, but some maximum current
output under ideal load matching?

That's pretty confusing actually, as in a real-world setup increasing
the drive-strength property value might negatively affect the load
matching, leading to _decreasing_ the actual drive-strength.

> Is that what you mean with "current regulator"?
> 
> If you mean rather "output impedance", that is something else and
> something we need a new (generic) property for. I never saw that
> before, really.

So, just because the description of the drive-strength value uses some
(idealized) output impedance, we are now going to introduce yet another
property for the same thing? This is confusing to people reading the
reference manual, which never speaks about output-impedance, but always
about drive-strength.

Hardware people (the ones responsible to come up with the padctrl
settings) are much more likely to read the reference manual than any
idealized dt-binding. I want to make it as easy as possible for them to
match up the hardware and reference manual with the DT description.
So, can we please just keep the drive-strength property and deal with
it being described in different units in different hardware manuals?

Regards,
Lucas

Re: [PATCH v2 1/3] dt-bindings: add binding for i.MX8MQ IOMUXC

[Linus Walleij]

On Tue, Feb 6, 2018 at 11:53 AM, Lucas Stach <l.stach@pengutronix.de> wrote:

Hi Lucas,

your reply seems a bit annoyed.

Are you annoyed with me?

In that case please take it down a bit, I understand it can
be stressful as developer, but I need to maintain the stuff
we're pushing in for years to come and it's my job to
ask the tricksy questions.

> Am Dienstag, den 06.02.2018, 00:13 +0100 schrieb Linus Walleij:
>> > On Mon, Feb 5, 2018 at 11:09 AM, Lucas Stach <l.stach@pengutronix.de> wrote:
>>
>> > > > +Optional Properties:
>> > > > +- drive-strength           Integer: controls Drive Strength
>> > >
>> > > I thought drive-strength is supposed to be be in mA. We should not have
>> > > differing units in common properties.
>> > >
>> > > There was an Atmel binding the other day that this came up.
>> >
>> > I know it's in the common binding, but it's going to be very hard to
>> > support. The drive-strength in mA really depends on external loading of
>> > the pin and the pin voltage, at least for the most widespread
>> > controllers that only switch between resistors, instead of driving the
>> > pin by a current regulator.
>>
>> How does this circuit really look? Can you sketch something so
>> we understand what is going on electronically here?
>
> There is no description of how the circuit really looks like and I
> would very much prefer this not being a prerequisite to get some
> software support in place.

That depends. If you're a hobbyist with limited access to
documentation I agree. In that case let's go for something
that floats your project, I'm fine with that. I have a weak spot
for people with a good project and limited resources, so I
am here to help. I'd say then let's go for some "nxp,foo" property.

If you are working for NXP directly or under contract, what you say
is that the left hand does not know what the right hand is doing and that
is never a good sign. Such as the hardware and software departments
do not talk to each other, do not have each other mail addresses
and practice what is called "throw over the wall-engineering".
Or throw it all the way over to Pengutronix, maybe, a separate
legal entity they don't even prioritize to inform. What do I know.

In this latter case it is more or less my responsibility as subsystem
maintainer to push back at NXP as a legal body, it has nothing to
do with the people involved. Especially not you personally. In this
case I am adressing your organization, not you.

So which one is it?

> The best description we have is from NXP AN5078:
> "The drive strength enable (DSE) can be explained as series resistance
> between an ideal driver's output and its load. To achieve maximal
> transferred power, the impedance of the driver has to match the load
> impedance."
>
> Which means the control bits don't really vary the output impedance,
> but the description models it like that. If we _assume_ the simple MOS
> totempole setup then adding more totempoles will bring down the output
> impedance.

+- drive-strength               Integer: controls Drive Strength
+                                       0: driver disabled
+                                       1: 255 Ohm
+                                       2: 105 Ohm
+                                       3:  75 Ohm
+                                       4:  85 Ohm
+                                       5:  65 Ohm
+                                       6:  45 Ohm
+                                       7:  40 Ohm

As for equal resistances R = Rx/N assuming it is
really 255 Ohms internal resistance per totempole:

1: 255
2: 127,5
3: 85
4: 63,75
5: 51
6: 42,5
7: 36,4

I don't know. Something is not right. Maybe this is how
CMOS work under load?

>> The way I imagine most drive strength out there works is by simply
>> connection more MOS-totempoles after each other and each of them
>> has a production-technology-related max output current like 4mA, so
>> by putting two in a series yoy double that to 8 mA.
>
> So with this description in mind the drive-strength property doesn't
> describe an actual real world drive-strength, but some maximum current
> output under ideal load matching?

I guess that is what other datasheets have been listing.
I have personally seen a few of those.

I don't know if it is as simple as a totem-pole, I guess
IIRC under ideal circumstances the internal resistance of an
operational amplifier is zero. E.g. on a popular TTL part uA 741
it is 5.5 milliohms. What is Rout for a normal CMOS totem-pole?
Some googling suggests ~400 Ohm so 255 Ohm may not be
so far off.

Does anyone else know?

This is pretty interesting stuff :D

> That's pretty confusing actually, as in a real-world setup increasing
> the drive-strength property value might negatively affect the load
> matching, leading to _decreasing_ the actual drive-strength.

This sounds like a perfect argument to create a new
property "output-impedance" and use that in my ears.
It makes more sense to a designer, because what they
really want to do, electrically, is to match impedances.

I am thinking some designer writing this device tree.
What they want to control is the output impedance, not
drive strength. So that name makes more sense.

>> Is that what you mean with "current regulator"?
>>
>> If you mean rather "output impedance", that is something else and
>> something we need a new (generic) property for. I never saw that
>> before, really.
>
> So, just because the description of the drive-strength value uses some
> (idealized) output impedance, we are now going to introduce yet another
> property for the same thing?

Yes it seems like a good idea, if (by your own line of reasoning)
it is the physical principle underlying these resistance settings.
Then we are likely to see it again.

> This is confusing to people reading the
> reference manual, which never speaks about output-impedance, but always
> about drive-strength.

Since we are already (obviously) confused, we have to choose
the lesser evil here.

Apparently what (board- system-) designers want to do is match
output resistance to load, I would argue that it is more confusing
to call that "drive strength", i.e. what is misleading is not what I am
discussing here, but the manual you are referring to. Just a bad
piece of technical writing.

> Hardware people (the ones responsible to come up with the padctrl
> settings) are much more likely to read the reference manual than any
> idealized dt-binding.

Can we talk to them about this?

I don't like to think about them as some foreign people in a far
away dimension we can never understand or communicate
with, how do they really do what they do?

> I want to make it as easy as possible for them to
> match up the hardware and reference manual with the DT description.

I was hoping we could dig in a bit and figure out what we
are going to do and why and how these people think.

I want to avoid poking magic values we don't really understand
into registers and using the manual as alchemic magic or
spellbook, essentially. (As so often happens...)

> So, can we please just keep the drive-strength property and deal with
> it being described in different units in different hardware manuals?

If you want something to match the specific hardware
manual from NXP and you don't want it to be translated
into the units of the DT binding, then I think it is better
to use something prefixed "nxp,*".

That clearly indicates that this is some NXP oddity that
we don't really understand.

I was just hoping not to introduce too many of these.

Yours,
Linus Walleij

Re: [PATCH v2 1/3] dt-bindings: add binding for i.MX8MQ IOMUXC

[Lucas Stach]

Hi Linus,

Am Dienstag, den 06.02.2018, 15:32 +0100 schrieb Linus Walleij:
> On Tue, Feb 6, 2018 at 11:53 AM, Lucas Stach <l.stach-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> wrote:
> 
> Hi Lucas,
> 
> your reply seems a bit annoyed.
> 
> Are you annoyed with me?
> 
> In that case please take it down a bit, I understand it can
> be stressful as developer, but I need to maintain the stuff
> we're pushing in for years to come and it's my job to
> ask the tricksy questions.

To be fully honest I'm a bit annoyed with the pinctrl framework making
things (IMHO) unnecessarily complex, for what is basically a pretty
easy task. But I'll try to keep things focused on the technical issue
at hand, so we can all make some progress.

> > Am Dienstag, den 06.02.2018, 00:13 +0100 schrieb Linus Walleij:
> > > > On Mon, Feb 5, 2018 at 11:09 AM, Lucas Stach <l.stach@pengutron
> > > > ix.de> wrote:
> > > > > > +Optional Properties:
> > > > > > +- drive-strength           Integer: controls Drive
> > > > > > Strength
> > > > > 
> > > > > I thought drive-strength is supposed to be be in mA. We
> > > > > should not have
> > > > > differing units in common properties.
> > > > > 
> > > > > There was an Atmel binding the other day that this came up.
> > > > 
> > > > I know it's in the common binding, but it's going to be very
> > > > hard to
> > > > support. The drive-strength in mA really depends on external
> > > > loading of
> > > > the pin and the pin voltage, at least for the most widespread
> > > > controllers that only switch between resistors, instead of
> > > > driving the
> > > > pin by a current regulator.
> > > 
> > > How does this circuit really look? Can you sketch something so
> > > we understand what is going on electronically here?
> > 
> > There is no description of how the circuit really looks like and I
> > would very much prefer this not being a prerequisite to get some
> > software support in place.
> 
> That depends. If you're a hobbyist with limited access to
> documentation I agree. In that case let's go for something
> that floats your project, I'm fine with that. I have a weak spot
> for people with a good project and limited resources, so I
> am here to help. I'd say then let's go for some "nxp,foo" property.
> 
> If you are working for NXP directly or under contract, what you say
> is that the left hand does not know what the right hand is doing and
> that
> is never a good sign. Such as the hardware and software departments
> do not talk to each other, do not have each other mail addresses
> and practice what is called "throw over the wall-engineering".
> Or throw it all the way over to Pengutronix, maybe, a separate
> legal entity they don't even prioritize to inform. What do I know.
> 
> In this latter case it is more or less my responsibility as subsystem
> maintainer to push back at NXP as a legal body, it has nothing to
> do with the people involved. Especially not you personally. In this
> case I am adressing your organization, not you.
> 
> So which one is it?

Neither. Pengutronix is mostly contracted by customers of NXP to enable
stuff in mainline. So I'm not working on this in my spare time, but I
still have to deal with the fact that I can only get the information
from the reference manual, NXP downstream BSPs and the occasional
helpful NXP employee.

Also even if we were inside of NXP, pad cells are usually something
that chip makers buy or get from the Fab design library. So probably
even they don't know what's inside exactly.

> > The best description we have is from NXP AN5078:
> > "The drive strength enable (DSE) can be explained as series
> > resistance
> > between an ideal driver's output and its load. To achieve maximal
> > transferred power, the impedance of the driver has to match the
> > load
> > impedance."
> > 
> > Which means the control bits don't really vary the output
> > impedance,
> > but the description models it like that. If we _assume_ the simple
> > MOS
> > totempole setup then adding more totempoles will bring down the
> > output
> > impedance.
> 
> +- drive-strength               Integer: controls Drive Strength
> +                                       0: driver disabled
> +                                       1: 255 Ohm
> +                                       2: 105 Ohm
> +                                       3:  75 Ohm
> +                                       4:  85 Ohm
> +                                       5:  65 Ohm
> +                                       6:  45 Ohm
> +                                       7:  40 Ohm
> 
> As for equal resistances R = Rx/N assuming it is
> really 255 Ohms internal resistance per totempole:
> 
> 1: 255
> 2: 127,5
> 3: 85
> 4: 63,75
> 5: 51
> 6: 42,5
> 7: 36,4
> 
> I don't know. Something is not right. Maybe this is how
> CMOS work under load?

Semiconductor physics tends to be slightly more complex than this. :)

> > > The way I imagine most drive strength out there works is by
> > > simply
> > > connection more MOS-totempoles after each other and each of them
> > > has a production-technology-related max output current like 4mA,
> > > so
> > > by putting two in a series yoy double that to 8 mA.
> > 
> > So with this description in mind the drive-strength property
> > doesn't
> > describe an actual real world drive-strength, but some maximum
> > current
> > output under ideal load matching?
> 
> I guess that is what other datasheets have been listing.
> I have personally seen a few of those.
> 
> I don't know if it is as simple as a totem-pole, I guess
> IIRC under ideal circumstances the internal resistance of an
> operational amplifier is zero. E.g. on a popular TTL part uA 741
> it is 5.5 milliohms. What is Rout for a normal CMOS totem-pole?
> Some googling suggests ~400 Ohm so 255 Ohm may not be
> so far off.
> 
> Does anyone else know?
> 
> This is pretty interesting stuff :D

Interesting indeed, but pretty far away from the dt-binding we set out
to discuss. And even if we might come up with a simplistic model of
what's going on inside the pad cell, how do we know if it matches
reality? Does this really help us define a useful binding?

> > That's pretty confusing actually, as in a real-world setup
> > increasing
> > the drive-strength property value might negatively affect the load
> > matching, leading to _decreasing_ the actual drive-strength.
> 
> This sounds like a perfect argument to create a new
> property "output-impedance" and use that in my ears.
> It makes more sense to a designer, because what they
> really want to do, electrically, is to match impedances.
> 
> I am thinking some designer writing this device tree.
> What they want to control is the output impedance, not
> drive strength. So that name makes more sense.
> 
> > > Is that what you mean with "current regulator"?
> > > 
> > > If you mean rather "output impedance", that is something else and
> > > something we need a new (generic) property for. I never saw that
> > > before, really.
> > 
> > So, just because the description of the drive-strength value uses
> > some
> > (idealized) output impedance, we are now going to introduce yet
> > another
> > property for the same thing?
> 
> Yes it seems like a good idea, if (by your own line of reasoning)
> it is the physical principle underlying these resistance settings.
> Then we are likely to see it again.
> 
> > This is confusing to people reading the
> > reference manual, which never speaks about output-impedance, but
> > always
> > about drive-strength.
> 
> Since we are already (obviously) confused, we have to choose
> the lesser evil here.
> 
> Apparently what (board- system-) designers want to do is match
> output resistance to load, I would argue that it is more confusing
> to call that "drive strength", i.e. what is misleading is not what I
> am
> discussing here, but the manual you are referring to. Just a bad
> piece of technical writing.

What usually happens is that hardware (I'm talking about board/system
here) designers start by reading the reference manual and hardware
design guide and work with that. They come up with all the necessary
configuration in the terms of the manual.

After that they or someone else has to translate this into DT. Things
get confusing when the reference manual and the DT binding disagree
about the used terms.

> > Hardware people (the ones responsible to come up with the padctrl
> > settings) are much more likely to read the reference manual than
> > any
> > idealized dt-binding.
> 
> Can we talk to them about this?
> 
> I don't like to think about them as some foreign people in a far
> away dimension we can never understand or communicate
> with, how do they really do what they do?

I wasn't referring to the chip design people, but board designers. I
talk to them quite often, but it's a lot easier if the Linux people and
board designers agree on the terms used. :)

> > I want to make it as easy as possible for them to
> > match up the hardware and reference manual with the DT description.
> 
> I was hoping we could dig in a bit and figure out what we
> are going to do and why and how these people think.
> 
> I want to avoid poking magic values we don't really understand
> into registers and using the manual as alchemic magic or
> spellbook, essentially. (As so often happens...)
> 
> > So, can we please just keep the drive-strength property and deal
> > with
> > it being described in different units in different hardware
> > manuals?
> 
> If you want something to match the specific hardware
> manual from NXP and you don't want it to be translated
> into the units of the DT binding, then I think it is better
> to use something prefixed "nxp,*".

> That clearly indicates that this is some NXP oddity that
> we don't really understand.

I'm not keen on using the common padctrl stuff, which already bloats
the DT description compared to i.MX6, and then again need to introduce
custom properties. That's just worst of both worlds, verbose DT to use
the common stuff, mixed with special properties, only valid on this
single controller.

If you insist I guess I'm fine with compromising on an "output-
impedance" common property, but then this just makes things harder for
everyone involved, as the impedance even seems to vary slightly with
the used pad voltage. So to not get into a combinatorial explosion, we
would need to describe this property as somethings like "output
impedance at 3.3v)", at least on this specific hardware.

Or we could agree that drive-strength property could be described in
some unit that makes sense on each controller. mA for hardware
described with some fabled ideal load matching, Ohms for hardware that
models it this way with maximum drive strength at the point of best
load matching.

In the end this is about mapping 3 hardware bits to a DT description...

Regards,
Lucas

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/3] dt-bindings: add binding for i.MX8MQ IOMUXC

[Linus Walleij]

On Tue, Feb 6, 2018 at 4:47 PM, Lucas Stach <l.stach-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> wrote:

> To be fully honest I'm a bit annoyed with the pinctrl framework making
> things (IMHO) unnecessarily complex, for what is basically a pretty
> easy task.

My ambition is to make things readable, understandable and
maintainable. In generic terms, incorporating a bunch of knowledge
of the electronics that really happen into the stuff we encode in
the kernel.

I guess it varies a bit on what goal one has.

If the goal is "ship product with upstream kernel really fast now"
then things like pinctrl-single.c where we just hammer magic values
into registers, make sense. OMAP developers had no idea whatsoever
what their ASIC people or cell library authors were doing so they
just threw in the towel. HiSilicon also use this. Intels ambition was
to use ACPI BIOS to handle all pin control and route around the kernel
altogether, but that is not working out so well for them I think.

All of them are approaches to avoid putting the hairy details into the
kernel, just poke some magic values into some magic registers
and be happy.

So i.MX could have been like that, but then I guess you need to take
legacy into account and discuss with the other i.MX driver authors
about how they really wanted and want to do things.

Their current silence wrt this mailchain is actually becoming a
problem, and the problem is that discussing with you falls upwards
to me as subsystem maintainer. Which sucks. I prefer that people
who know this hardware discuss amongst themselves how they
want things to work.

Surely Sascha must have an opinion? It means much to me
what he wants to do. I take it you guys are colleagues?

> Pengutronix is mostly contracted by customers of NXP to enable
> stuff in mainline. So I'm not working on this in my spare time, but I
> still have to deal with the fact that I can only get the information
> from the reference manual, NXP downstream BSPs and the occasional
> helpful NXP employee.

Hm I see, this seems like a bit of crappy position to be in when
you just want to ask someone in hardware how things really work.

But we have Freescale i.MX maintainers on the inside of the company
now NXP (soon Qualcomm? soon Broadcom?).

Hell this mail is even going to linux-imx-3arQi8VN3Tc@public.gmane.org, what do they do
with it? Put it in a mailbox and never read it?

Surely someone on the inside must be able to provide some details.

> Also even if we were inside of NXP, pad cells are usually something
> that chip makers buy or get from the Fab design library. So probably
> even they don't know what's inside exactly.

Yeah that is what I call "throw over the wall engineering".
It's not good, if NXP works with information brick walls like that it is
not any good for them, because it is not any good for their customers.

Not something you or I can fix though...

> What usually happens is that hardware (I'm talking about board/system
> here) designers start by reading the reference manual and hardware
> design guide and work with that. They come up with all the necessary
> configuration in the terms of the manual.

Sometimes half-guessing and a bit of trial-and-error right?

> After that they or someone else has to translate this into DT. Things
> get confusing when the reference manual and the DT binding disagree
> about the used terms.

I see.

>> If you want something to match the specific hardware
>> manual from NXP and you don't want it to be translated
>> into the units of the DT binding, then I think it is better
>> to use something prefixed "nxp,*".
>
>> That clearly indicates that this is some NXP oddity that
>> we don't really understand.
>
> I'm not keen on using the common padctrl stuff, which already bloats
> the DT description compared to i.MX6,

This you need to discuss with the generic i.MX driver maintainers.
They are the maintainers of drivers/pinctrl/freescale/* after all.

They never put an entry in MAINTAINERS though, but I
always regarded these as the pinctrl maintainers for i.MX:

ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
M:      Shawn Guo <shawnguo-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
M:      Sascha Hauer <kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
R:      Fabio Estevam <fabio.estevam-3arQi8VN3Tc@public.gmane.org>

On top of this, Dong Aisheng, Gary Bisson and
Vladimir Zapolskiy has made major contributions.

It's a little ecosystem on its own, not really to be discussed
by just you and me. I wonder where the rest of the voices are,
I hope not silenced by organizational stress after the NXP
merger...

> and then again need to introduce
> custom properties. That's just worst of both worlds, verbose DT to use
> the common stuff, mixed with special properties, only valid on this
> single controller.

No matter how much you dislike it, it is what e.g. Qualcomm is
doing. (Who might soon be one and the same as NXP I hear.)

> If you insist I guess I'm fine with compromising on an "output-
> impedance" common property, but then this just makes things harder for
> everyone involved, as the impedance even seems to vary slightly with
> the used pad voltage. So to not get into a combinatorial explosion, we
> would need to describe this property as somethings like "output
> impedance at 3.3v)", at least on this specific hardware.

Hm, should it me "nxp,drive-strength" then, as it is just some
magic value? I guess "nxp,magic-drive-strength" is not going to
cut it :D

Also maybe we should use the old "freescale,*" notation for legacy
reasons... I don't know. This Vendor prefix seems less stable
than the chipsets.

> Or we could agree that drive-strength property could be described in
> some unit that makes sense on each controller. mA for hardware
> described with some fabled ideal load matching, Ohms for hardware that
> models it this way with maximum drive strength at the point of best
> load matching.

I am not like stubbornly against. I just want some discussion here,
it would be nice to know the opinion of the i.MX maintainers.

> In the end this is about mapping 3 hardware bits to a DT description...

Pleas don't think about it like "can't we just do this real simple
thing now, just merge this and stop being an ass".

Things just poking three bits can look very simple, like in so
many BSPs:

volatile unsigned long *foo;

foo = (unsigned long *) 0xfec0be00;
*foo &= ~0x700;
*foo |= 0x200; /* do the magic */

But this is not helpful for developers or maintainers. That is IMHO
why we have the frameworks and all the DT standardization in
the first place, exactly so we understand what we are doing.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v2 1/3] dt-bindings: add binding for i.MX8MQ IOMUXC

[A.s. Dong]

Hi Linus,

> -----Original Message-----
> From: Linus Walleij [mailto:linus.walleij@linaro.org]
> Sent: 2018年2月7日 17:09
> To: Lucas Stach <l.stach@pengutronix.de>; A.s. Dong
> <aisheng.dong@nxp.com>; dl-linux-imx <linux-imx@nxp.com>; Gary Bisson
> <gary.bisson@boundarydevices.com>; Vladimir Zapolskiy
> <vladimir_zapolskiy@mentor.com>; Sascha Hauer <kernel@pengutronix.de>
> Cc: Rob Herring <robh@kernel.org>; Mark Rutland <mark.rutland@arm.com>;
> linux-gpio@vger.kernel.org; open list:OPEN FIRMWARE AND FLATTENED
> DEVICE TREE BINDINGS <devicetree@vger.kernel.org>;
> patchwork-lst@pengutronix.de
> Subject: Re: [PATCH v2 1/3] dt-bindings: add binding for i.MX8MQ IOMUXC
> 
> On Tue, Feb 6, 2018 at 4:47 PM, Lucas Stach <l.stach@pengutronix.de> wrote:
> 
> > To be fully honest I'm a bit annoyed with the pinctrl framework making
> > things (IMHO) unnecessarily complex, for what is basically a pretty
> > easy task.
> 
> My ambition is to make things readable, understandable and maintainable. In
> generic terms, incorporating a bunch of knowledge of the electronics that really
> happen into the stuff we encode in the kernel.
> 
> I guess it varies a bit on what goal one has.
> 
> If the goal is "ship product with upstream kernel really fast now"
> then things like pinctrl-single.c where we just hammer magic values into
> registers, make sense. OMAP developers had no idea whatsoever what their
> ASIC people or cell library authors were doing so they just threw in the towel.
> HiSilicon also use this. Intels ambition was to use ACPI BIOS to handle all pin
> control and route around the kernel altogether, but that is not working out so
> well for them I think.
> 
> All of them are approaches to avoid putting the hairy details into the kernel, just
> poke some magic values into some magic registers and be happy.
> 
> So i.MX could have been like that, but then I guess you need to take legacy into
> account and discuss with the other i.MX driver authors about how they really
> wanted and want to do things.
> 
> Their current silence wrt this mailchain is actually becoming a problem, and the
> problem is that discussing with you falls upwards to me as subsystem maintainer.
> Which sucks. I prefer that people who know this hardware discuss amongst
> themselves how they want things to work.
> 
> Surely Sascha must have an opinion? It means much to me what he wants to do.
> I take it you guys are colleagues?
> 
> > Pengutronix is mostly contracted by customers of NXP to enable stuff
> > in mainline. So I'm not working on this in my spare time, but I still
> > have to deal with the fact that I can only get the information from
> > the reference manual, NXP downstream BSPs and the occasional helpful
> > NXP employee.
> 
> Hm I see, this seems like a bit of crappy position to be in when you just want to
> ask someone in hardware how things really work.
> 
> But we have Freescale i.MX maintainers on the inside of the company now NXP
> (soon Qualcomm? soon Broadcom?).
> 
> Hell this mail is even going to linux-imx@nxp.com, what do they do with it? Put
> it in a mailbox and never read it?
> 
> Surely someone on the inside must be able to provide some details.
> 

I feel terribly sorry that did not come up in the first time these two days as I was
busy with some other urgent things requested by my boss.
It should be my responsibility as I promised to take that ownership before.

I quickly went through the email and I fully understand your concern.
As I'm not a specialist on the HW internals of IOMUX design, i already forwarded the
Issue to our IP owner. 

Due to it's already off work time at my side, I will discuss with him tomorrow
Morning and get back to you later.

And i will carefully review the reference manual and doc on my hands.
Hopefully I can give you some information later you want.

Regards
Dong Aisheng

> > Also even if we were inside of NXP, pad cells are usually something
> > that chip makers buy or get from the Fab design library. So probably
> > even they don't know what's inside exactly.
> 
> Yeah that is what I call "throw over the wall engineering".
> It's not good, if NXP works with information brick walls like that it is not any good
> for them, because it is not any good for their customers.
> 
> Not something you or I can fix though...
> 
> > What usually happens is that hardware (I'm talking about board/system
> > here) designers start by reading the reference manual and hardware
> > design guide and work with that. They come up with all the necessary
> > configuration in the terms of the manual.
> 
> Sometimes half-guessing and a bit of trial-and-error right?
> 
> > After that they or someone else has to translate this into DT. Things
> > get confusing when the reference manual and the DT binding disagree
> > about the used terms.
> 
> I see.
> 
> >> If you want something to match the specific hardware manual from NXP
> >> and you don't want it to be translated into the units of the DT
> >> binding, then I think it is better to use something prefixed "nxp,*".
> >
> >> That clearly indicates that this is some NXP oddity that we don't
> >> really understand.
> >
> > I'm not keen on using the common padctrl stuff, which already bloats
> > the DT description compared to i.MX6,
> 
> This you need to discuss with the generic i.MX driver maintainers.
> They are the maintainers of drivers/pinctrl/freescale/* after all.
> 
> They never put an entry in MAINTAINERS though, but I always regarded these as
> the pinctrl maintainers for i.MX:
> 
> ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
> M:      Shawn Guo <shawnguo@kernel.org>
> M:      Sascha Hauer <kernel@pengutronix.de>
> R:      Fabio Estevam <fabio.estevam@nxp.com>
> 
> On top of this, Dong Aisheng, Gary Bisson and Vladimir Zapolskiy has made major
> contributions.
> 
> It's a little ecosystem on its own, not really to be discussed by just you and me. I
> wonder where the rest of the voices are, I hope not silenced by organizational
> stress after the NXP merger...
> 
> > and then again need to introduce
> > custom properties. That's just worst of both worlds, verbose DT to use
> > the common stuff, mixed with special properties, only valid on this
> > single controller.
> 
> No matter how much you dislike it, it is what e.g. Qualcomm is doing. (Who
> might soon be one and the same as NXP I hear.)
> 
> > If you insist I guess I'm fine with compromising on an "output-
> > impedance" common property, but then this just makes things harder for
> > everyone involved, as the impedance even seems to vary slightly with
> > the used pad voltage. So to not get into a combinatorial explosion, we
> > would need to describe this property as somethings like "output
> > impedance at 3.3v)", at least on this specific hardware.
> 
> Hm, should it me "nxp,drive-strength" then, as it is just some magic value? I
> guess "nxp,magic-drive-strength" is not going to cut it :D
> 
> Also maybe we should use the old "freescale,*" notation for legacy reasons... I
> don't know. This Vendor prefix seems less stable than the chipsets.
> 
> > Or we could agree that drive-strength property could be described in
> > some unit that makes sense on each controller. mA for hardware
> > described with some fabled ideal load matching, Ohms for hardware that
> > models it this way with maximum drive strength at the point of best
> > load matching.
> 
> I am not like stubbornly against. I just want some discussion here, it would be
> nice to know the opinion of the i.MX maintainers.
> 
> > In the end this is about mapping 3 hardware bits to a DT description...
> 
> Pleas don't think about it like "can't we just do this real simple thing now, just
> merge this and stop being an ass".
> 
> Things just poking three bits can look very simple, like in so many BSPs:
> 
> volatile unsigned long *foo;
> 
> foo = (unsigned long *) 0xfec0be00;
> *foo &= ~0x700;
> *foo |= 0x200; /* do the magic */
> 
> But this is not helpful for developers or maintainers. That is IMHO why we have
> the frameworks and all the DT standardization in the first place, exactly so we
> understand what we are doing.
> 
> Yours,
> Linus Walleij

RE: [PATCH v2 1/3] dt-bindings: add binding for i.MX8MQ IOMUXC

[A.s. Dong]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 10697 bytes --]

> -----Original Message-----
> From: A.s. Dong
> Sent: 2018年2月7日 19:03
> To: 'Linus Walleij' <linus.walleij@linaro.org>; Lucas Stach
> <l.stach@pengutronix.de>; dl-linux-imx <linux-imx@nxp.com>; Gary Bisson
> <gary.bisson@boundarydevices.com>; Vladimir Zapolskiy
> <vladimir_zapolskiy@mentor.com>; Sascha Hauer <kernel@pengutronix.de>
> Cc: Rob Herring <robh@kernel.org>; Mark Rutland <mark.rutland@arm.com>;
> linux-gpio@vger.kernel.org; open list:OPEN FIRMWARE AND FLATTENED
> DEVICE TREE BINDINGS <devicetree@vger.kernel.org>;
> patchwork-lst@pengutronix.de
> Subject: RE: [PATCH v2 1/3] dt-bindings: add binding for i.MX8MQ IOMUXC
> 
> Hi Linus,
> 
> > -----Original Message-----
> > From: Linus Walleij [mailto:linus.walleij@linaro.org]
> > Sent: 2018年2月7日 17:09
> > To: Lucas Stach <l.stach@pengutronix.de>; A.s. Dong
> > <aisheng.dong@nxp.com>; dl-linux-imx <linux-imx@nxp.com>; Gary Bisson
> > <gary.bisson@boundarydevices.com>; Vladimir Zapolskiy
> > <vladimir_zapolskiy@mentor.com>; Sascha Hauer <kernel@pengutronix.de>
> > Cc: Rob Herring <robh@kernel.org>; Mark Rutland
> > <mark.rutland@arm.com>; linux-gpio@vger.kernel.org; open list:OPEN
> > FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
> > <devicetree@vger.kernel.org>; patchwork-lst@pengutronix.de
> > Subject: Re: [PATCH v2 1/3] dt-bindings: add binding for i.MX8MQ
> > IOMUXC
> >
> > On Tue, Feb 6, 2018 at 4:47 PM, Lucas Stach <l.stach@pengutronix.de> wrote:
> >
> > > To be fully honest I'm a bit annoyed with the pinctrl framework
> > > making things (IMHO) unnecessarily complex, for what is basically a
> > > pretty easy task.
> >
> > My ambition is to make things readable, understandable and
> > maintainable. In generic terms, incorporating a bunch of knowledge of
> > the electronics that really happen into the stuff we encode in the kernel.
> >
> > I guess it varies a bit on what goal one has.
> >
> > If the goal is "ship product with upstream kernel really fast now"
> > then things like pinctrl-single.c where we just hammer magic values
> > into registers, make sense. OMAP developers had no idea whatsoever
> > what their ASIC people or cell library authors were doing so they just threw in
> the towel.
> > HiSilicon also use this. Intels ambition was to use ACPI BIOS to
> > handle all pin control and route around the kernel altogether, but
> > that is not working out so well for them I think.
> >
> > All of them are approaches to avoid putting the hairy details into the
> > kernel, just poke some magic values into some magic registers and be happy.
> >
> > So i.MX could have been like that, but then I guess you need to take
> > legacy into account and discuss with the other i.MX driver authors
> > about how they really wanted and want to do things.
> >
> > Their current silence wrt this mailchain is actually becoming a
> > problem, and the problem is that discussing with you falls upwards to me as
> subsystem maintainer.
> > Which sucks. I prefer that people who know this hardware discuss
> > amongst themselves how they want things to work.
> >
> > Surely Sascha must have an opinion? It means much to me what he wants to
> do.
> > I take it you guys are colleagues?
> >
> > > Pengutronix is mostly contracted by customers of NXP to enable stuff
> > > in mainline. So I'm not working on this in my spare time, but I
> > > still have to deal with the fact that I can only get the information
> > > from the reference manual, NXP downstream BSPs and the occasional
> > > helpful NXP employee.
> >
> > Hm I see, this seems like a bit of crappy position to be in when you
> > just want to ask someone in hardware how things really work.
> >
> > But we have Freescale i.MX maintainers on the inside of the company
> > now NXP (soon Qualcomm? soon Broadcom?).
> >
> > Hell this mail is even going to linux-imx@nxp.com, what do they do
> > with it? Put it in a mailbox and never read it?
> >
> > Surely someone on the inside must be able to provide some details.
> >
> 
> I feel terribly sorry that did not come up in the first time these two days as I was
> busy with some other urgent things requested by my boss.
> It should be my responsibility as I promised to take that ownership before.
> 
> I quickly went through the email and I fully understand your concern.
> As I'm not a specialist on the HW internals of IOMUX design, i already forwarded
> the Issue to our IP owner.
> 
> Due to it's already off work time at my side, I will discuss with him tomorrow
> Morning and get back to you later.
> 
> And i will carefully review the reference manual and doc on my hands.
> Hopefully I can give you some information later you want.
> 

Here is some information from our IP designer.
It probably explains why i.MX uses Ohm to represents driver strength.

"Electrical spec for different interfaces typically define output driver in Ohm.
So we follow the same way to comply with spec requirement. Also, for the
board designer it is important to know the driver impedance to match the
driver and PCB trace resistance. Defining the driver in Ohm allows to
calculate IR drop, as well as output slew rate, which are important to know."

An example from eMMC 5.1 spec:

Table 206 - I/O driver strength types
0x0		50Ω		x1
0x1		33Ω		x1.5
0x2		66Ω		x0.75
0x3		100Ω	x0.5
0x4		40Ω		x1.2

And for one question you asked in another email.
- Is the intended usage for impedance matching?
For low frequency interface, for example, I2C/GPIO, it’s not necessary. 
For high frequency interface, for example, DDR/HS200/HS400/Rawnand
differential 200M, in theory, we should consider impedance matching for
better signal quality. Anyway, we can check that by board level simulation
with spice model.

And IC guys thought Ohm probably is more suitable for high speed
IO pads.

Hope it helps.

Regards
Dong Aisheng

> Regards
> Dong Aisheng
> 
> > > Also even if we were inside of NXP, pad cells are usually something
> > > that chip makers buy or get from the Fab design library. So probably
> > > even they don't know what's inside exactly.
> >
> > Yeah that is what I call "throw over the wall engineering".
> > It's not good, if NXP works with information brick walls like that it
> > is not any good for them, because it is not any good for their customers.
> >
> > Not something you or I can fix though...
> >
> > > What usually happens is that hardware (I'm talking about
> > > board/system
> > > here) designers start by reading the reference manual and hardware
> > > design guide and work with that. They come up with all the necessary
> > > configuration in the terms of the manual.
> >
> > Sometimes half-guessing and a bit of trial-and-error right?
> >
> > > After that they or someone else has to translate this into DT.
> > > Things get confusing when the reference manual and the DT binding
> > > disagree about the used terms.
> >
> > I see.
> >
> > >> If you want something to match the specific hardware manual from
> > >> NXP and you don't want it to be translated into the units of the DT
> > >> binding, then I think it is better to use something prefixed "nxp,*".
> > >
> > >> That clearly indicates that this is some NXP oddity that we don't
> > >> really understand.
> > >
> > > I'm not keen on using the common padctrl stuff, which already bloats
> > > the DT description compared to i.MX6,
> >
> > This you need to discuss with the generic i.MX driver maintainers.
> > They are the maintainers of drivers/pinctrl/freescale/* after all.
> >
> > They never put an entry in MAINTAINERS though, but I always regarded
> > these as the pinctrl maintainers for i.MX:
> >
> > ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
> > M:      Shawn Guo <shawnguo@kernel.org>
> > M:      Sascha Hauer <kernel@pengutronix.de>
> > R:      Fabio Estevam <fabio.estevam@nxp.com>
> >
> > On top of this, Dong Aisheng, Gary Bisson and Vladimir Zapolskiy has
> > made major contributions.
> >
> > It's a little ecosystem on its own, not really to be discussed by just
> > you and me. I wonder where the rest of the voices are, I hope not
> > silenced by organizational stress after the NXP merger...
> >
> > > and then again need to introduce
> > > custom properties. That's just worst of both worlds, verbose DT to
> > > use the common stuff, mixed with special properties, only valid on
> > > this single controller.
> >
> > No matter how much you dislike it, it is what e.g. Qualcomm is doing.
> > (Who might soon be one and the same as NXP I hear.)
> >
> > > If you insist I guess I'm fine with compromising on an "output-
> > > impedance" common property, but then this just makes things harder
> > > for everyone involved, as the impedance even seems to vary slightly
> > > with the used pad voltage. So to not get into a combinatorial
> > > explosion, we would need to describe this property as somethings
> > > like "output impedance at 3.3v)", at least on this specific hardware.
> >
> > Hm, should it me "nxp,drive-strength" then, as it is just some magic
> > value? I guess "nxp,magic-drive-strength" is not going to cut it :D
> >
> > Also maybe we should use the old "freescale,*" notation for legacy
> > reasons... I don't know. This Vendor prefix seems less stable than the chipsets.
> >
> > > Or we could agree that drive-strength property could be described in
> > > some unit that makes sense on each controller. mA for hardware
> > > described with some fabled ideal load matching, Ohms for hardware
> > > that models it this way with maximum drive strength at the point of
> > > best load matching.
> >
> > I am not like stubbornly against. I just want some discussion here, it
> > would be nice to know the opinion of the i.MX maintainers.
> >
> > > In the end this is about mapping 3 hardware bits to a DT description...
> >
> > Pleas don't think about it like "can't we just do this real simple
> > thing now, just merge this and stop being an ass".
> >
> > Things just poking three bits can look very simple, like in so many BSPs:
> >
> > volatile unsigned long *foo;
> >
> > foo = (unsigned long *) 0xfec0be00;
> > *foo &= ~0x700;
> > *foo |= 0x200; /* do the magic */
> >
> > But this is not helpful for developers or maintainers. That is IMHO
> > why we have the frameworks and all the DT standardization in the first
> > place, exactly so we understand what we are doing.
> >
> > Yours,
> > Linus Walleij
N‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·zøœzÚÞz)í…æèw*\x1fjg¬±¨\x1e¶‰šŽŠÝ¢j.ïÛ°\½½MŽúgjÌæa×\x02››–' ™©Þ¢¸\f¢·¦j:+v‰¨ŠwèjØm¶Ÿÿ¾\a«‘êçzZ+ƒùšŽŠÝ¢j"ú!¶i

Re: [PATCH v2 1/3] dt-bindings: add binding for i.MX8MQ IOMUXC

[Sascha Hauer]

On Wed, Feb 07, 2018 at 10:09:20AM +0100, Linus Walleij wrote:
> On Tue, Feb 6, 2018 at 4:47 PM, Lucas Stach <l.stach-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> wrote:
> 
> > To be fully honest I'm a bit annoyed with the pinctrl framework making
> > things (IMHO) unnecessarily complex, for what is basically a pretty
> > easy task.
> 
> My ambition is to make things readable, understandable and
> maintainable. In generic terms, incorporating a bunch of knowledge
> of the electronics that really happen into the stuff we encode in
> the kernel.
> 
> I guess it varies a bit on what goal one has.
> 
> If the goal is "ship product with upstream kernel really fast now"
> then things like pinctrl-single.c where we just hammer magic values
> into registers, make sense. OMAP developers had no idea whatsoever
> what their ASIC people or cell library authors were doing so they
> just threw in the towel. HiSilicon also use this. Intels ambition was
> to use ACPI BIOS to handle all pin control and route around the kernel
> altogether, but that is not working out so well for them I think.
> 
> All of them are approaches to avoid putting the hairy details into the
> kernel, just poke some magic values into some magic registers
> and be happy.
> 
> So i.MX could have been like that, but then I guess you need to take
> legacy into account and discuss with the other i.MX driver authors
> about how they really wanted and want to do things.
> 
> Their current silence wrt this mailchain is actually becoming a
> problem, and the problem is that discussing with you falls upwards
> to me as subsystem maintainer. Which sucks. I prefer that people
> who know this hardware discuss amongst themselves how they
> want things to work.
> 
> Surely Sascha must have an opinion? It means much to me
> what he wants to do. I take it you guys are colleagues?

My opinion is that all that is generic about padctrl is a device driver
saying "Put my pins into a suitable mode". That is what padctrl is good
for and we are there for years now. I have always been happy with the
plain register values in the device tree. Before device tree we had
exactly these values in the board files and I never heard anyone
complaining about it. There were defines for the bits in the register
which you could use when you were unhappy with plain register values.

It's really trivial to look in the reference manual to make up the
needed register values. It's also trivial to take a register value
and look into the reference manual what this value does. Every
translation layer, call it generic properties, just makes things more
complicated. Often enough our input is register value tables
from either our customers our from spreadsheets from FSL/NXP. Every
translation layer in the way just means we have to translate the already
existing register values into something hoping that this correctly
translates back into the register values.

It's not that some board designer comes up with "I need a drive strength
of 150mA" and wants to put that value into the device tree. Instead they
start with the reference manual, see which values they can (must) adjust
and then adjust the values until they are happy. No one wants to ask
questions like "How do I have to manipulate that device tree to change
that particular bit?"

As said, I am happy with plain register values in the device tree and
I consider everything else overengineered.
FSL/NXP Reference Manuals are freely available and of high quality so
everybody can understand the register values. There's nothing magic to
them. That might change slightly when the Manuals are not available, but
even then I think that not the device tree ABI is the right place to
add that missing documentation.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/3] dt-bindings: add binding for i.MX8MQ IOMUXC

[Fabio Estevam]

[Adding Shawn on Cc]

On Wed, Feb 7, 2018 at 9:11 AM, Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> wrote:

> My opinion is that all that is generic about padctrl is a device driver
> saying "Put my pins into a suitable mode". That is what padctrl is good
> for and we are there for years now. I have always been happy with the
> plain register values in the device tree. Before device tree we had
> exactly these values in the board files and I never heard anyone
> complaining about it. There were defines for the bits in the register
> which you could use when you were unhappy with plain register values.
>
> It's really trivial to look in the reference manual to make up the
> needed register values. It's also trivial to take a register value
> and look into the reference manual what this value does. Every
> translation layer, call it generic properties, just makes things more
> complicated. Often enough our input is register value tables
> from either our customers our from spreadsheets from FSL/NXP. Every
> translation layer in the way just means we have to translate the already
> existing register values into something hoping that this correctly
> translates back into the register values.
>
> It's not that some board designer comes up with "I need a drive strength
> of 150mA" and wants to put that value into the device tree. Instead they
> start with the reference manual, see which values they can (must) adjust
> and then adjust the values until they are happy. No one wants to ask
> questions like "How do I have to manipulate that device tree to change
> that particular bit?"
>
> As said, I am happy with plain register values in the device tree and
> I consider everything else overengineered.
> FSL/NXP Reference Manuals are freely available and of high quality so
> everybody can understand the register values. There's nothing magic to
> them. That might change slightly when the Manuals are not available, but
> even then I think that not the device tree ABI is the right place to
> add that missing documentation.

I agree 100% with Sascha.

I am also happy with the current plain register values in device tree.

Having two methods (plain register values on most of i.MX SoCs and the
new generic one for imx7ulp, imx8mq) is confusing for the end users.

I would prefer to use the plain register values in device tree for all
i.MX SoCs instead.

Thanks
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/3] dt-bindings: add binding for i.MX8MQ IOMUXC

[Shawn Guo]

On Wed, Feb 07, 2018 at 09:41:22AM -0200, Fabio Estevam wrote:
> [Adding Shawn on Cc]

Thanks Fabio.

> On Wed, Feb 7, 2018 at 9:11 AM, Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> wrote:
> 
> > My opinion is that all that is generic about padctrl is a device driver
> > saying "Put my pins into a suitable mode". That is what padctrl is good
> > for and we are there for years now. I have always been happy with the
> > plain register values in the device tree. Before device tree we had
> > exactly these values in the board files and I never heard anyone
> > complaining about it. There were defines for the bits in the register
> > which you could use when you were unhappy with plain register values.
> >
> > It's really trivial to look in the reference manual to make up the
> > needed register values. It's also trivial to take a register value
> > and look into the reference manual what this value does. Every
> > translation layer, call it generic properties, just makes things more
> > complicated. Often enough our input is register value tables
> > from either our customers our from spreadsheets from FSL/NXP. Every
> > translation layer in the way just means we have to translate the already
> > existing register values into something hoping that this correctly
> > translates back into the register values.
> >
> > It's not that some board designer comes up with "I need a drive strength
> > of 150mA" and wants to put that value into the device tree. Instead they
> > start with the reference manual, see which values they can (must) adjust
> > and then adjust the values until they are happy. No one wants to ask
> > questions like "How do I have to manipulate that device tree to change
> > that particular bit?"
> >
> > As said, I am happy with plain register values in the device tree and
> > I consider everything else overengineered.
> > FSL/NXP Reference Manuals are freely available and of high quality so
> > everybody can understand the register values. There's nothing magic to
> > them. That might change slightly when the Manuals are not available, but
> > even then I think that not the device tree ABI is the right place to
> > add that missing documentation.
> 
> I agree 100% with Sascha.

I would vote for not going generic pinconf either, as the controversy
here starts from something, that indicates the generic stuff doesn't
work for i.MX.

Shawn
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/3] dt-bindings: add binding for i.MX8MQ IOMUXC

[Lucas Stach]

Am Donnerstag, den 08.02.2018, 16:56 +0800 schrieb Shawn Guo:
> On Wed, Feb 07, 2018 at 09:41:22AM -0200, Fabio Estevam wrote:
> > [Adding Shawn on Cc]
> 
> Thanks Fabio.
> 
> > On Wed, Feb 7, 2018 at 9:11 AM, Sascha Hauer <s.hauer@pengutronix.d
> > e> wrote:
> > 
> > > My opinion is that all that is generic about padctrl is a device
> > > driver
> > > saying "Put my pins into a suitable mode". That is what padctrl
> > > is good
> > > for and we are there for years now. I have always been happy with
> > > the
> > > plain register values in the device tree. Before device tree we
> > > had
> > > exactly these values in the board files and I never heard anyone
> > > complaining about it. There were defines for the bits in the
> > > register
> > > which you could use when you were unhappy with plain register
> > > values.
> > > 
> > > It's really trivial to look in the reference manual to make up
> > > the
> > > needed register values. It's also trivial to take a register
> > > value
> > > and look into the reference manual what this value does. Every
> > > translation layer, call it generic properties, just makes things
> > > more
> > > complicated. Often enough our input is register value tables
> > > from either our customers our from spreadsheets from FSL/NXP.
> > > Every
> > > translation layer in the way just means we have to translate the
> > > already
> > > existing register values into something hoping that this
> > > correctly
> > > translates back into the register values.
> > > 
> > > It's not that some board designer comes up with "I need a drive
> > > strength
> > > of 150mA" and wants to put that value into the device tree.
> > > Instead they
> > > start with the reference manual, see which values they can (must)
> > > adjust
> > > and then adjust the values until they are happy. No one wants to
> > > ask
> > > questions like "How do I have to manipulate that device tree to
> > > change
> > > that particular bit?"
> > > 
> > > As said, I am happy with plain register values in the device tree
> > > and
> > > I consider everything else overengineered.
> > > FSL/NXP Reference Manuals are freely available and of high
> > > quality so
> > > everybody can understand the register values. There's nothing
> > > magic to
> > > them. That might change slightly when the Manuals are not
> > > available, but
> > > even then I think that not the device tree ABI is the right place
> > > to
> > > add that missing documentation.
> > 
> > I agree 100% with Sascha.
> 
> I would vote for not going generic pinconf either, as the controversy
> here starts from something, that indicates the generic stuff doesn't
> work for i.MX.

So it seems with that we are at a point where the majority vote of
users/maintainers are in favor of keeping the binding that has served
us well on MX5/6. Which is right where we started with v1 of the MX8
patches.

How do we proceed? I would like to send out a respin of those series
next week. Can we all agree to roll back the pinctrl binding to the
MX5/6 one? Or are there still major reservations against it? I would
like to avoid introducing any unnecessary churn.

Regards,
Lucas
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/3] dt-bindings: add binding for i.MX8MQ IOMUXC

[Fabio Estevam]

Hi Lucas,

On Thu, Feb 8, 2018 at 1:28 PM, Lucas Stach <l.stach-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> wrote:

> So it seems with that we are at a point where the majority vote of
> users/maintainers are in favor of keeping the binding that has served
> us well on MX5/6. Which is right where we started with v1 of the MX8
> patches.
>
> How do we proceed? I would like to send out a respin of those series
> next week. Can we all agree to roll back the pinctrl binding to the
> MX5/6 one? Or are there still major reservations against it? I would

Yes, looks like a good plan IMHO.

Thanks
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/3] dt-bindings: add binding for i.MX8MQ IOMUXC

[Linus Walleij]

On Thu, Feb 8, 2018 at 4:28 PM, Lucas Stach <l.stach@pengutronix.de> wrote:

> So it seems with that we are at a point where the majority vote of
> users/maintainers are in favor of keeping the binding that has served
> us well on MX5/6. Which is right where we started with v1 of the MX8
> patches.
>
> How do we proceed? I would like to send out a respin of those series
> next week. Can we all agree to roll back the pinctrl binding to the
> MX5/6 one? Or are there still major reservations against it? I would
> like to avoid introducing any unnecessary churn.

What we need to see is for i.MX maintainers to agree, e.g.
figure out how to do all stuff in drivers/pinctrl/freescale/*
should be engineered going forward.

(Maybe that filepath and symbols needs to be renamed drivers/pinctrl/imx
by the way, this platform seems to change company more often
than I change my car tyres.)

I'll try to follow up on the resulting patch series.

Yours,
Linus Walleij

RE: [PATCH v2 1/3] dt-bindings: add binding for i.MX8MQ IOMUXC

[A.s. Dong]

Hi Sascha,

> -----Original Message-----
> From: Sascha Hauer [mailto:s.hauer@pengutronix.de]
> Sent: 2018年2月7日 19:12
> To: Linus Walleij <linus.walleij@linaro.org>
> Cc: Lucas Stach <l.stach@pengutronix.de>; A.s. Dong
> <aisheng.dong@nxp.com>; dl-linux-imx <linux-imx@nxp.com>; Gary Bisson
> <gary.bisson@boundarydevices.com>; Vladimir Zapolskiy
> <vladimir_zapolskiy@mentor.com>; Sascha Hauer <kernel@pengutronix.de>;
> Mark Rutland <mark.rutland@arm.com>; Rob Herring <robh@kernel.org>;
> open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
> <devicetree@vger.kernel.org>; patchwork-lst@pengutronix.de;
> linux-gpio@vger.kernel.org
> Subject: Re: [PATCH v2 1/3] dt-bindings: add binding for i.MX8MQ IOMUXC
> 
> On Wed, Feb 07, 2018 at 10:09:20AM +0100, Linus Walleij wrote:
> > On Tue, Feb 6, 2018 at 4:47 PM, Lucas Stach <l.stach@pengutronix.de> wrote:
> >
> > > To be fully honest I'm a bit annoyed with the pinctrl framework
> > > making things (IMHO) unnecessarily complex, for what is basically a
> > > pretty easy task.
> >
> > My ambition is to make things readable, understandable and
> > maintainable. In generic terms, incorporating a bunch of knowledge of
> > the electronics that really happen into the stuff we encode in the
> > kernel.
> >
> > I guess it varies a bit on what goal one has.
> >
> > If the goal is "ship product with upstream kernel really fast now"
> > then things like pinctrl-single.c where we just hammer magic values
> > into registers, make sense. OMAP developers had no idea whatsoever
> > what their ASIC people or cell library authors were doing so they just
> > threw in the towel. HiSilicon also use this. Intels ambition was to
> > use ACPI BIOS to handle all pin control and route around the kernel
> > altogether, but that is not working out so well for them I think.
> >
> > All of them are approaches to avoid putting the hairy details into the
> > kernel, just poke some magic values into some magic registers and be
> > happy.
> >
> > So i.MX could have been like that, but then I guess you need to take
> > legacy into account and discuss with the other i.MX driver authors
> > about how they really wanted and want to do things.
> >
> > Their current silence wrt this mailchain is actually becoming a
> > problem, and the problem is that discussing with you falls upwards to
> > me as subsystem maintainer. Which sucks. I prefer that people who know
> > this hardware discuss amongst themselves how they want things to work.
> >
> > Surely Sascha must have an opinion? It means much to me what he wants
> > to do. I take it you guys are colleagues?
> 
> My opinion is that all that is generic about padctrl is a device driver saying "Put
> my pins into a suitable mode". That is what padctrl is good for and we are there
> for years now. I have always been happy with the plain register values in the
> device tree. Before device tree we had exactly these values in the board files
> and I never heard anyone complaining about it. There were defines for the bits
> in the register which you could use when you were unhappy with plain register
> values.
> 
> It's really trivial to look in the reference manual to make up the needed register
> values. It's also trivial to take a register value and look into the reference
> manual what this value does. Every translation layer, call it generic properties,
> just makes things more complicated. Often enough our input is register value
> tables from either our customers our from spreadsheets from FSL/NXP. Every
> translation layer in the way just means we have to translate the already existing
> register values into something hoping that this correctly translates back into the
> register values.
> 
> It's not that some board designer comes up with "I need a drive strength of
> 150mA" and wants to put that value into the device tree. Instead they start
> with the reference manual, see which values they can (must) adjust and then
> adjust the values until they are happy. No one wants to ask questions like "How
> do I have to manipulate that device tree to change that particular bit?"
> 
> As said, I am happy with plain register values in the device tree and I consider
> everything else overengineered.
> FSL/NXP Reference Manuals are freely available and of high quality so everybody
> can understand the register values. There's nothing magic to them. That might
> change slightly when the Manuals are not available, but even then I think that
> not the device tree ABI is the right place to add that missing documentation.
> 

Probably as I first implemented generic pinconfig support for i.MX, personally
I'm tend to it a bit, mainly because below reasons we find in real situations:

First and most important, plain register value is a bit error prone.
(Actually we meet many times of issues during internal development).
User has to compose it manually by refer to reference manual
which might be more easy to mistake than using standard binding property.

And not everyone would like to compose the register value manually
when writing their device tree, they may simply do copy & paste of an exist one
and quickly test, if it works, everything is done. They don't know which setting is actually
used as they can see nothing from the plain register value. Untill the device becomes
unwork in some special circumstances, then they have to waste a lot time to find out
the pad setting issue. This happens especially more often when switch to
a new SoC while the pad config register layout changed a bit.
We certainly would like to avoid it!

Secondly, plain register value is also unreadable and unmaintainable,
also not quite friendly in reviewing other one's patches.

Last, it might be trivial, people don't want to refer to reference manual
again and again once being asked which driver strength or something else
Is used.

Regards
Dong Aisheng

> Sascha
> 
> --
> Pengutronix e.K.                           |
> |
> Industrial Linux Solutions                 |
> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.p
> engutronix.de%2F&data=02%7C01%7Caisheng.dong%40nxp.com%7C2644d871
> 2e234a6c2e7a08d56e1b9b29%7Cbd8a2a2207224ec7b35f1c4f0497e341%7C0%
> 7C0%7C636535987211269744&sdata=vImuWxtT7Ya0uxEHrnROUYZ%2BiCWLs8
> LmxWB5oNPcgjk%3D&reserved=0  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:
> +49-5121-206917-5555 |

Re: [PATCH v2 1/3] dt-bindings: add binding for i.MX8MQ IOMUXC

[Sascha Hauer]

On Wed, Feb 07, 2018 at 01:21:44PM +0000, A.s. Dong wrote:
> Hi Sascha,
> 
> > -----Original Message-----
> > From: Sascha Hauer [mailto:s.hauer@pengutronix.de]
> > Sent: 2018年2月7日 19:12
> > To: Linus Walleij <linus.walleij@linaro.org>
> > Cc: Lucas Stach <l.stach@pengutronix.de>; A.s. Dong
> > <aisheng.dong@nxp.com>; dl-linux-imx <linux-imx@nxp.com>; Gary Bisson
> > <gary.bisson@boundarydevices.com>; Vladimir Zapolskiy
> > <vladimir_zapolskiy@mentor.com>; Sascha Hauer <kernel@pengutronix.de>;
> > Mark Rutland <mark.rutland@arm.com>; Rob Herring <robh@kernel.org>;
> > open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
> > <devicetree@vger.kernel.org>; patchwork-lst@pengutronix.de;
> > linux-gpio@vger.kernel.org
> > Subject: Re: [PATCH v2 1/3] dt-bindings: add binding for i.MX8MQ IOMUXC
> > 
> > On Wed, Feb 07, 2018 at 10:09:20AM +0100, Linus Walleij wrote:
> > > On Tue, Feb 6, 2018 at 4:47 PM, Lucas Stach <l.stach@pengutronix.de> wrote:
> > >
> > > > To be fully honest I'm a bit annoyed with the pinctrl framework
> > > > making things (IMHO) unnecessarily complex, for what is basically a
> > > > pretty easy task.
> > >
> > > My ambition is to make things readable, understandable and
> > > maintainable. In generic terms, incorporating a bunch of knowledge of
> > > the electronics that really happen into the stuff we encode in the
> > > kernel.
> > >
> > > I guess it varies a bit on what goal one has.
> > >
> > > If the goal is "ship product with upstream kernel really fast now"
> > > then things like pinctrl-single.c where we just hammer magic values
> > > into registers, make sense. OMAP developers had no idea whatsoever
> > > what their ASIC people or cell library authors were doing so they just
> > > threw in the towel. HiSilicon also use this. Intels ambition was to
> > > use ACPI BIOS to handle all pin control and route around the kernel
> > > altogether, but that is not working out so well for them I think.
> > >
> > > All of them are approaches to avoid putting the hairy details into the
> > > kernel, just poke some magic values into some magic registers and be
> > > happy.
> > >
> > > So i.MX could have been like that, but then I guess you need to take
> > > legacy into account and discuss with the other i.MX driver authors
> > > about how they really wanted and want to do things.
> > >
> > > Their current silence wrt this mailchain is actually becoming a
> > > problem, and the problem is that discussing with you falls upwards to
> > > me as subsystem maintainer. Which sucks. I prefer that people who know
> > > this hardware discuss amongst themselves how they want things to work.
> > >
> > > Surely Sascha must have an opinion? It means much to me what he wants
> > > to do. I take it you guys are colleagues?
> > 
> > My opinion is that all that is generic about padctrl is a device driver saying "Put
> > my pins into a suitable mode". That is what padctrl is good for and we are there
> > for years now. I have always been happy with the plain register values in the
> > device tree. Before device tree we had exactly these values in the board files
> > and I never heard anyone complaining about it. There were defines for the bits
> > in the register which you could use when you were unhappy with plain register
> > values.
> > 
> > It's really trivial to look in the reference manual to make up the needed register
> > values. It's also trivial to take a register value and look into the reference
> > manual what this value does. Every translation layer, call it generic properties,
> > just makes things more complicated. Often enough our input is register value
> > tables from either our customers our from spreadsheets from FSL/NXP. Every
> > translation layer in the way just means we have to translate the already existing
> > register values into something hoping that this correctly translates back into the
> > register values.
> > 
> > It's not that some board designer comes up with "I need a drive strength of
> > 150mA" and wants to put that value into the device tree. Instead they start
> > with the reference manual, see which values they can (must) adjust and then
> > adjust the values until they are happy. No one wants to ask questions like "How
> > do I have to manipulate that device tree to change that particular bit?"
> > 
> > As said, I am happy with plain register values in the device tree and I consider
> > everything else overengineered.
> > FSL/NXP Reference Manuals are freely available and of high quality so everybody
> > can understand the register values. There's nothing magic to them. That might
> > change slightly when the Manuals are not available, but even then I think that
> > not the device tree ABI is the right place to add that missing documentation.
> > 
> 
> Probably as I first implemented generic pinconfig support for i.MX, personally
> I'm tend to it a bit, mainly because below reasons we find in real situations:
> 
> First and most important, plain register value is a bit error prone.
> (Actually we meet many times of issues during internal development).
> User has to compose it manually by refer to reference manual
> which might be more easy to mistake than using standard binding property.
> 
> And not everyone would like to compose the register value manually
> when writing their device tree, they may simply do copy & paste of an exist one
> and quickly test, if it works, everything is done. They don't know which setting is actually
> used as they can see nothing from the plain register value. Untill the device becomes
> unwork in some special circumstances, then they have to waste a lot time to find out
> the pad setting issue. This happens especially more often when switch to
> a new SoC while the pad config register layout changed a bit.
> We certainly would like to avoid it!
> 
> Secondly, plain register value is also unreadable and unmaintainable,
> also not quite friendly in reviewing other one's patches.
> 
> Last, it might be trivial, people don't want to refer to reference manual
> again and again once being asked which driver strength or something else
> Is used.

These issues could be solved by bitmask defines aswell, no need to bloat
the dtbs with strings.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |