[PATCH 1/1] pinctrl: handle dummy state in core

[PATCH 1/1] pinctrl: handle dummy state in core

[Dong Aisheng]

From: Dong Aisheng <dong.aisheng@linaro.org>

Remove dummy state user interface and handle it totally in core.
This can make it more easy to use by platforms which has neither pinctrl
driver support nor dt support.

Signed-off-by: Dong Aisheng <dong.aisheng@linaro.org>
---
 drivers/pinctrl/Kconfig         |    5 +++++
 drivers/pinctrl/core.c          |   25 +++++++++++++------------
 drivers/pinctrl/core.h          |    3 +--
 drivers/pinctrl/devicetree.c    |   26 --------------------------
 include/linux/pinctrl/machine.h |   11 +----------
 5 files changed, 20 insertions(+), 50 deletions(-)

diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index 01557e7..9316c97 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -26,6 +26,11 @@ config DEBUG_PINCTRL
 	help
 	  Say Y here to add some extra checks and diagnostics to PINCTRL calls.
 
+config PINCTRL_DUMMY
+	bool "pinctrl dummy state support"
+	help
+	  Say Y here to allow the driver to run well without pinctrl support
+
 config PINCTRL_IMX
 	bool "Freescale IMX core pinctrl driver"
 	depends on ARCH_MXC
diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
index 74a7843..d0313a9 100644
--- a/drivers/pinctrl/core.c
+++ b/drivers/pinctrl/core.c
@@ -523,9 +523,6 @@ static int add_setting(struct pinctrl *p, struct pinctrl_map const *map)
 	if (IS_ERR(state))
 		return PTR_ERR(state);
 
-	if (map->type == PIN_MAP_TYPE_DUMMY_STATE)
-		return 0;
-
 	setting = kzalloc(sizeof(*setting), GFP_KERNEL);
 	if (setting == NULL) {
 		dev_err(p->dev,
@@ -718,8 +715,17 @@ static struct pinctrl_state *pinctrl_lookup_state_locked(struct pinctrl *p,
 	struct pinctrl_state *state;
 
 	state = find_state(p, name);
-	if (!state)
+	if (!state) {
+#ifdef CONFIG_PINCTRL_DUMMY
+		/* create dummy state */
+		dev_dbg(p->dev, "using dummy pinctrl state (%s)\n", name);
+		state = create_state(p, name);
+		if (IS_ERR(state))
+			return state;
+#else
 		return ERR_PTR(-ENODEV);
+#endif
+	}
 
 	return state;
 }
@@ -842,16 +848,13 @@ int pinctrl_register_map(struct pinctrl_map const *maps, unsigned num_maps,
 			return -EINVAL;
 		}
 
-		if (maps[i].type != PIN_MAP_TYPE_DUMMY_STATE &&
-				!maps[i].ctrl_dev_name) {
+		if (!maps[i].ctrl_dev_name) {
 			pr_err("failed to register map %s (%d): no pin control device given\n",
 			       maps[i].name, i);
 			return -EINVAL;
 		}
 
 		switch (maps[i].type) {
-		case PIN_MAP_TYPE_DUMMY_STATE:
-			break;
 		case PIN_MAP_TYPE_MUX_GROUP:
 			ret = pinmux_validate_map(&maps[i], i);
 			if (ret < 0)
@@ -1054,7 +1057,6 @@ static inline const char *map_type(enum pinctrl_map_type type)
 {
 	static const char * const names[] = {
 		"INVALID",
-		"DUMMY_STATE",
 		"MUX_GROUP",
 		"CONFIGS_PIN",
 		"CONFIGS_GROUP",
@@ -1081,9 +1083,8 @@ static int pinctrl_maps_show(struct seq_file *s, void *what)
 			   map->dev_name, map->name, map_type(map->type),
 			   map->type);
 
-		if (map->type != PIN_MAP_TYPE_DUMMY_STATE)
-			seq_printf(s, "controlling device %s\n",
-				   map->ctrl_dev_name);
+		seq_printf(s, "controlling device %s\n",
+			   map->ctrl_dev_name);
 
 		switch (map->type) {
 		case PIN_MAP_TYPE_MUX_GROUP:
diff --git a/drivers/pinctrl/core.h b/drivers/pinctrl/core.h
index 1f40ff6..4c23f19 100644
--- a/drivers/pinctrl/core.h
+++ b/drivers/pinctrl/core.h
@@ -103,8 +103,7 @@ struct pinctrl_setting_configs {
  * struct pinctrl_setting - an individual mux or config setting
  * @node: list node for struct pinctrl_settings's @settings field
  * @type: the type of setting
- * @pctldev: pin control device handling to be programmed. Not used for
- *   PIN_MAP_TYPE_DUMMY_STATE.
+ * @pctldev: pin control device handling to be programmed.
  * @data: Data specific to the setting type
  */
 struct pinctrl_setting {
diff --git a/drivers/pinctrl/devicetree.c b/drivers/pinctrl/devicetree.c
index fcb1de4..3ac360f 100644
--- a/drivers/pinctrl/devicetree.c
+++ b/drivers/pinctrl/devicetree.c
@@ -43,9 +43,6 @@ static void dt_free_map(struct pinctrl_dev *pctldev,
 	if (pctldev) {
 		struct pinctrl_ops *ops = pctldev->desc->pctlops;
 		ops->dt_free_map(pctldev, map, num_maps);
-	} else {
-		/* There is no pctldev for PIN_MAP_TYPE_DUMMY_STATE */
-		kfree(map);
 	}
 }
 
@@ -151,22 +148,6 @@ static int dt_to_map_one_config(struct pinctrl *p, const char *statename,
 	return dt_remember_or_free_map(p, statename, pctldev, map, num_maps);
 }
 
-static int dt_remember_dummy_state(struct pinctrl *p, const char *statename)
-{
-	struct pinctrl_map *map;
-
-	map = kzalloc(sizeof(*map), GFP_KERNEL);
-	if (!map) {
-		dev_err(p->dev, "failed to alloc struct pinctrl_map\n");
-		return -ENOMEM;
-	}
-
-	/* There is no pctldev for PIN_MAP_TYPE_DUMMY_STATE */
-	map->type = PIN_MAP_TYPE_DUMMY_STATE;
-
-	return dt_remember_or_free_map(p, statename, NULL, map, 1);
-}
-
 int pinctrl_dt_to_map(struct pinctrl *p)
 {
 	struct device_node *np = p->dev->of_node;
@@ -232,13 +213,6 @@ int pinctrl_dt_to_map(struct pinctrl *p)
 			if (ret < 0)
 				goto err;
 		}
-
-		/* No entries in DT? Generate a dummy state table entry */
-		if (!size) {
-			ret = dt_remember_dummy_state(p, statename);
-			if (ret < 0)
-				goto err;
-		}
 	}
 
 	return 0;
diff --git a/include/linux/pinctrl/machine.h b/include/linux/pinctrl/machine.h
index e4d1de7..7383946 100644
--- a/include/linux/pinctrl/machine.h
+++ b/include/linux/pinctrl/machine.h
@@ -18,7 +18,6 @@
 
 enum pinctrl_map_type {
 	PIN_MAP_TYPE_INVALID,
-	PIN_MAP_TYPE_DUMMY_STATE,
 	PIN_MAP_TYPE_MUX_GROUP,
 	PIN_MAP_TYPE_CONFIGS_PIN,
 	PIN_MAP_TYPE_CONFIGS_GROUP,
@@ -61,8 +60,7 @@ struct pinctrl_map_configs {
  *	This is the parameter passed to pinmux_lookup_state()
  * @type: the type of mapping table entry
  * @ctrl_dev_name: the name of the device controlling this specific mapping,
- *	the name must be the same as in your struct device*. This field is not
- *	used for PIN_MAP_TYPE_DUMMY_STATE
+ *	the name must be the same as in your struct device*.
  * @data: Data specific to the mapping type
  */
 struct pinctrl_map {
@@ -78,13 +76,6 @@ struct pinctrl_map {
 
 /* Convenience macros to create mapping table entries */
 
-#define PIN_MAP_DUMMY_STATE(dev, state) \
-	{								\
-		.dev_name = dev,					\
-		.name = state,						\
-		.type = PIN_MAP_TYPE_DUMMY_STATE,			\
-	}
-
 #define PIN_MAP_MUX_GROUP(dev, state, pinctrl, grp, func)		\
 	{								\
 		.dev_name = dev,					\
-- 
1.7.0.4

[PATCH 1/1] pinctrl: handle dummy state in core

[Dong Aisheng]

From: Dong Aisheng <dong.aisheng@linaro.org>

Remove dummy state user interface and handle it totally in core.
This can make it more easy to use by platforms which has neither pinctrl
driver support nor dt support.

Signed-off-by: Dong Aisheng <dong.aisheng@linaro.org>
---
 drivers/pinctrl/Kconfig         |    5 +++++
 drivers/pinctrl/core.c          |   25 +++++++++++++------------
 drivers/pinctrl/core.h          |    3 +--
 drivers/pinctrl/devicetree.c    |   26 --------------------------
 include/linux/pinctrl/machine.h |   11 +----------
 5 files changed, 20 insertions(+), 50 deletions(-)

diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index 01557e7..9316c97 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -26,6 +26,11 @@ config DEBUG_PINCTRL
 	help
 	  Say Y here to add some extra checks and diagnostics to PINCTRL calls.
 
+config PINCTRL_DUMMY
+	bool "pinctrl dummy state support"
+	help
+	  Say Y here to allow the driver to run well without pinctrl support
+
 config PINCTRL_IMX
 	bool "Freescale IMX core pinctrl driver"
 	depends on ARCH_MXC
diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
index 74a7843..d0313a9 100644
--- a/drivers/pinctrl/core.c
+++ b/drivers/pinctrl/core.c
@@ -523,9 +523,6 @@ static int add_setting(struct pinctrl *p, struct pinctrl_map const *map)
 	if (IS_ERR(state))
 		return PTR_ERR(state);
 
-	if (map->type == PIN_MAP_TYPE_DUMMY_STATE)
-		return 0;
-
 	setting = kzalloc(sizeof(*setting), GFP_KERNEL);
 	if (setting == NULL) {
 		dev_err(p->dev,
@@ -718,8 +715,17 @@ static struct pinctrl_state *pinctrl_lookup_state_locked(struct pinctrl *p,
 	struct pinctrl_state *state;
 
 	state = find_state(p, name);
-	if (!state)
+	if (!state) {
+#ifdef CONFIG_PINCTRL_DUMMY
+		/* create dummy state */
+		dev_dbg(p->dev, "using dummy pinctrl state (%s)\n", name);
+		state = create_state(p, name);
+		if (IS_ERR(state))
+			return state;
+#else
 		return ERR_PTR(-ENODEV);
+#endif
+	}
 
 	return state;
 }
@@ -842,16 +848,13 @@ int pinctrl_register_map(struct pinctrl_map const *maps, unsigned num_maps,
 			return -EINVAL;
 		}
 
-		if (maps[i].type != PIN_MAP_TYPE_DUMMY_STATE &&
-				!maps[i].ctrl_dev_name) {
+		if (!maps[i].ctrl_dev_name) {
 			pr_err("failed to register map %s (%d): no pin control device given\n",
 			       maps[i].name, i);
 			return -EINVAL;
 		}
 
 		switch (maps[i].type) {
-		case PIN_MAP_TYPE_DUMMY_STATE:
-			break;
 		case PIN_MAP_TYPE_MUX_GROUP:
 			ret = pinmux_validate_map(&maps[i], i);
 			if (ret < 0)
@@ -1054,7 +1057,6 @@ static inline const char *map_type(enum pinctrl_map_type type)
 {
 	static const char * const names[] = {
 		"INVALID",
-		"DUMMY_STATE",
 		"MUX_GROUP",
 		"CONFIGS_PIN",
 		"CONFIGS_GROUP",
@@ -1081,9 +1083,8 @@ static int pinctrl_maps_show(struct seq_file *s, void *what)
 			   map->dev_name, map->name, map_type(map->type),
 			   map->type);
 
-		if (map->type != PIN_MAP_TYPE_DUMMY_STATE)
-			seq_printf(s, "controlling device %s\n",
-				   map->ctrl_dev_name);
+		seq_printf(s, "controlling device %s\n",
+			   map->ctrl_dev_name);
 
 		switch (map->type) {
 		case PIN_MAP_TYPE_MUX_GROUP:
diff --git a/drivers/pinctrl/core.h b/drivers/pinctrl/core.h
index 1f40ff6..4c23f19 100644
--- a/drivers/pinctrl/core.h
+++ b/drivers/pinctrl/core.h
@@ -103,8 +103,7 @@ struct pinctrl_setting_configs {
  * struct pinctrl_setting - an individual mux or config setting
  * @node: list node for struct pinctrl_settings's @settings field
  * @type: the type of setting
- * @pctldev: pin control device handling to be programmed. Not used for
- *   PIN_MAP_TYPE_DUMMY_STATE.
+ * @pctldev: pin control device handling to be programmed.
  * @data: Data specific to the setting type
  */
 struct pinctrl_setting {
diff --git a/drivers/pinctrl/devicetree.c b/drivers/pinctrl/devicetree.c
index fcb1de4..3ac360f 100644
--- a/drivers/pinctrl/devicetree.c
+++ b/drivers/pinctrl/devicetree.c
@@ -43,9 +43,6 @@ static void dt_free_map(struct pinctrl_dev *pctldev,
 	if (pctldev) {
 		struct pinctrl_ops *ops = pctldev->desc->pctlops;
 		ops->dt_free_map(pctldev, map, num_maps);
-	} else {
-		/* There is no pctldev for PIN_MAP_TYPE_DUMMY_STATE */
-		kfree(map);
 	}
 }
 
@@ -151,22 +148,6 @@ static int dt_to_map_one_config(struct pinctrl *p, const char *statename,
 	return dt_remember_or_free_map(p, statename, pctldev, map, num_maps);
 }
 
-static int dt_remember_dummy_state(struct pinctrl *p, const char *statename)
-{
-	struct pinctrl_map *map;
-
-	map = kzalloc(sizeof(*map), GFP_KERNEL);
-	if (!map) {
-		dev_err(p->dev, "failed to alloc struct pinctrl_map\n");
-		return -ENOMEM;
-	}
-
-	/* There is no pctldev for PIN_MAP_TYPE_DUMMY_STATE */
-	map->type = PIN_MAP_TYPE_DUMMY_STATE;
-
-	return dt_remember_or_free_map(p, statename, NULL, map, 1);
-}
-
 int pinctrl_dt_to_map(struct pinctrl *p)
 {
 	struct device_node *np = p->dev->of_node;
@@ -232,13 +213,6 @@ int pinctrl_dt_to_map(struct pinctrl *p)
 			if (ret < 0)
 				goto err;
 		}
-
-		/* No entries in DT? Generate a dummy state table entry */
-		if (!size) {
-			ret = dt_remember_dummy_state(p, statename);
-			if (ret < 0)
-				goto err;
-		}
 	}
 
 	return 0;
diff --git a/include/linux/pinctrl/machine.h b/include/linux/pinctrl/machine.h
index e4d1de7..7383946 100644
--- a/include/linux/pinctrl/machine.h
+++ b/include/linux/pinctrl/machine.h
@@ -18,7 +18,6 @@
 
 enum pinctrl_map_type {
 	PIN_MAP_TYPE_INVALID,
-	PIN_MAP_TYPE_DUMMY_STATE,
 	PIN_MAP_TYPE_MUX_GROUP,
 	PIN_MAP_TYPE_CONFIGS_PIN,
 	PIN_MAP_TYPE_CONFIGS_GROUP,
@@ -61,8 +60,7 @@ struct pinctrl_map_configs {
  *	This is the parameter passed to pinmux_lookup_state()
  * @type: the type of mapping table entry
  * @ctrl_dev_name: the name of the device controlling this specific mapping,
- *	the name must be the same as in your struct device*. This field is not
- *	used for PIN_MAP_TYPE_DUMMY_STATE
+ *	the name must be the same as in your struct device*.
  * @data: Data specific to the mapping type
  */
 struct pinctrl_map {
@@ -78,13 +76,6 @@ struct pinctrl_map {
 
 /* Convenience macros to create mapping table entries */
 
-#define PIN_MAP_DUMMY_STATE(dev, state) \
-	{								\
-		.dev_name = dev,					\
-		.name = state,						\
-		.type = PIN_MAP_TYPE_DUMMY_STATE,			\
-	}
-
 #define PIN_MAP_MUX_GROUP(dev, state, pinctrl, grp, func)		\
 	{								\
 		.dev_name = dev,					\
-- 
1.7.0.4

Re: [PATCH 1/1] pinctrl: handle dummy state in core

[Stephen Warren]

On 04/16/2012 08:24 AM, Dong Aisheng wrote:
> From: Dong Aisheng <dong.aisheng@linaro.org>
> 
> Remove dummy state user interface and handle it totally in core.
> This can make it more easy to use by platforms which has neither pinctrl
> driver support nor dt support.
> 
> Signed-off-by: Dong Aisheng <dong.aisheng@linaro.org>

I think this is the wrong direction.

I specifically want people to think about which drivers require what
pinctrl states to be defined, and what their content should be, and
hence explicitly define dummy states when it's appropriate. This patch
prevents that.

[PATCH 1/1] pinctrl: handle dummy state in core

[Stephen Warren]

On 04/16/2012 08:24 AM, Dong Aisheng wrote:
> From: Dong Aisheng <dong.aisheng@linaro.org>
> 
> Remove dummy state user interface and handle it totally in core.
> This can make it more easy to use by platforms which has neither pinctrl
> driver support nor dt support.
> 
> Signed-off-by: Dong Aisheng <dong.aisheng@linaro.org>

I think this is the wrong direction.

I specifically want people to think about which drivers require what
pinctrl states to be defined, and what their content should be, and
hence explicitly define dummy states when it's appropriate. This patch
prevents that.

Re: [PATCH 1/1] pinctrl: handle dummy state in core

[Dong Aisheng]

On Tue, Apr 17, 2012 at 12:03:34AM +0800, Stephen Warren wrote:
> On 04/16/2012 08:24 AM, Dong Aisheng wrote:
> > From: Dong Aisheng <dong.aisheng@linaro.org>
> > 
> > Remove dummy state user interface and handle it totally in core.
> > This can make it more easy to use by platforms which has neither pinctrl
> > driver support nor dt support.
> > 
> > Signed-off-by: Dong Aisheng <dong.aisheng@linaro.org>
> 
> I think this is the wrong direction.
> 
> I specifically want people to think about which drivers require what
> pinctrl states to be defined, and what their content should be, and
> hence explicitly define dummy states when it's appropriate. This patch
> prevents that.
> 
It's correct if all platforms have switched to pinctrl subsystem.

I think the main issue is for one platform neither supports dt nor using pinctrl
subsystem, do you think it still makes too much sense to force that platform
to define a _PINCTRL_ dummy state in their machine code?

But the driver is commonly shared between these different platforms(using pinctrl
or not).

That's why i proposed to handle dummy state in core in this patch rather than require
user to do it.

You must can assume there're so many platforms still have not switched to use pinctrl
subsystem. If we require user to do it, we need change many code.

Regards
Dong Aisheng

[PATCH 1/1] pinctrl: handle dummy state in core

[Dong Aisheng]

On Tue, Apr 17, 2012 at 12:03:34AM +0800, Stephen Warren wrote:
> On 04/16/2012 08:24 AM, Dong Aisheng wrote:
> > From: Dong Aisheng <dong.aisheng@linaro.org>
> > 
> > Remove dummy state user interface and handle it totally in core.
> > This can make it more easy to use by platforms which has neither pinctrl
> > driver support nor dt support.
> > 
> > Signed-off-by: Dong Aisheng <dong.aisheng@linaro.org>
> 
> I think this is the wrong direction.
> 
> I specifically want people to think about which drivers require what
> pinctrl states to be defined, and what their content should be, and
> hence explicitly define dummy states when it's appropriate. This patch
> prevents that.
> 
It's correct if all platforms have switched to pinctrl subsystem.

I think the main issue is for one platform neither supports dt nor using pinctrl
subsystem, do you think it still makes too much sense to force that platform
to define a _PINCTRL_ dummy state in their machine code?

But the driver is commonly shared between these different platforms(using pinctrl
or not).

That's why i proposed to handle dummy state in core in this patch rather than require
user to do it.

You must can assume there're so many platforms still have not switched to use pinctrl
subsystem. If we require user to do it, we need change many code.

Regards
Dong Aisheng

Re: [PATCH 1/1] pinctrl: handle dummy state in core

[Stephen Warren]

On 04/16/2012 09:42 PM, Dong Aisheng wrote:
> On Tue, Apr 17, 2012 at 12:03:34AM +0800, Stephen Warren wrote:
>> On 04/16/2012 08:24 AM, Dong Aisheng wrote:
>>> From: Dong Aisheng <dong.aisheng@linaro.org>
>>>
>>> Remove dummy state user interface and handle it totally in core.
>>> This can make it more easy to use by platforms which has neither pinctrl
>>> driver support nor dt support.
>>>
>>> Signed-off-by: Dong Aisheng <dong.aisheng@linaro.org>
>>
>> I think this is the wrong direction.
>>
>> I specifically want people to think about which drivers require what
>> pinctrl states to be defined, and what their content should be, and
>> hence explicitly define dummy states when it's appropriate. This patch
>> prevents that.
>>
> It's correct if all platforms have switched to pinctrl subsystem.
> 
> I think the main issue is for one platform neither supports dt nor using pinctrl
> subsystem, do you think it still makes too much sense to force that platform
> to define a _PINCTRL_ dummy state in their machine code?
> 
> But the driver is commonly shared between these different platforms(using pinctrl
> or not).

For a platform that doesn't have any pinctrl driver, wouldn't you just
disable CONFIG_PINCTRL. When that's disabled, all the pinctrl APIs are
already stubbed out not to return errors, IIRC.

If you're converting a platform to pinctrl for the first time, I don't
think it's unreasonable to have to define the dummy states for any
drivers that already use pinctrl. After all, there will on average
probably not be any/many such drivers, since presumably you'd write the
pinctrl driver first, then enable pinctrl for the platform, then port
all the relevant drivers to using pinctrl. The only pre-existing drivers
that actually use pinctrl would be shared IP blocks, which aren't too
common across different SoCs. (They are common within a series of SoCs
from the same vendor, but I still think it's reasonable to just write
few dummy states during the early conversion process either way).

If you don't agree with my thinking, can we at least make your patch
conditional, probably using a Kconfig variable, so that fully ported
platforms can require the mapping table (or DT) to fully specify all
pinctrl state (including dummy states if required), but people doing
development can flip a Kconfig switch to make pinctrl_get/lookup_state
return a dummy state instead of an error"?

> That's why i proposed to handle dummy state in core in this patch rather than require
> user to do it.
> 
> You must can assume there're so many platforms still have not switched to use pinctrl
> subsystem. If we require user to do it, we need change many code.
> 
> Regards
> Dong Aisheng

[PATCH 1/1] pinctrl: handle dummy state in core

[Stephen Warren]

On 04/16/2012 09:42 PM, Dong Aisheng wrote:
> On Tue, Apr 17, 2012 at 12:03:34AM +0800, Stephen Warren wrote:
>> On 04/16/2012 08:24 AM, Dong Aisheng wrote:
>>> From: Dong Aisheng <dong.aisheng@linaro.org>
>>>
>>> Remove dummy state user interface and handle it totally in core.
>>> This can make it more easy to use by platforms which has neither pinctrl
>>> driver support nor dt support.
>>>
>>> Signed-off-by: Dong Aisheng <dong.aisheng@linaro.org>
>>
>> I think this is the wrong direction.
>>
>> I specifically want people to think about which drivers require what
>> pinctrl states to be defined, and what their content should be, and
>> hence explicitly define dummy states when it's appropriate. This patch
>> prevents that.
>>
> It's correct if all platforms have switched to pinctrl subsystem.
> 
> I think the main issue is for one platform neither supports dt nor using pinctrl
> subsystem, do you think it still makes too much sense to force that platform
> to define a _PINCTRL_ dummy state in their machine code?
> 
> But the driver is commonly shared between these different platforms(using pinctrl
> or not).

For a platform that doesn't have any pinctrl driver, wouldn't you just
disable CONFIG_PINCTRL. When that's disabled, all the pinctrl APIs are
already stubbed out not to return errors, IIRC.

If you're converting a platform to pinctrl for the first time, I don't
think it's unreasonable to have to define the dummy states for any
drivers that already use pinctrl. After all, there will on average
probably not be any/many such drivers, since presumably you'd write the
pinctrl driver first, then enable pinctrl for the platform, then port
all the relevant drivers to using pinctrl. The only pre-existing drivers
that actually use pinctrl would be shared IP blocks, which aren't too
common across different SoCs. (They are common within a series of SoCs
from the same vendor, but I still think it's reasonable to just write
few dummy states during the early conversion process either way).

If you don't agree with my thinking, can we at least make your patch
conditional, probably using a Kconfig variable, so that fully ported
platforms can require the mapping table (or DT) to fully specify all
pinctrl state (including dummy states if required), but people doing
development can flip a Kconfig switch to make pinctrl_get/lookup_state
return a dummy state instead of an error"?

> That's why i proposed to handle dummy state in core in this patch rather than require
> user to do it.
> 
> You must can assume there're so many platforms still have not switched to use pinctrl
> subsystem. If we require user to do it, we need change many code.
> 
> Regards
> Dong Aisheng

Re: [PATCH 1/1] pinctrl: handle dummy state in core

[Linus Walleij]

On Tue, Apr 17, 2012 at 5:42 AM, Dong Aisheng
<aisheng.dong@freescale.com> wrote:

> I think the main issue is for one platform neither supports dt nor using pinctrl
> subsystem, do you think it still makes too much sense to force that platform
> to define a _PINCTRL_ dummy state in their machine code?
>
> But the driver is commonly shared between these different platforms(using pinctrl
> or not).

So this is for the situation where some machines have pinctrl and some haven't.

And I think that's how some archs have used the regulator dummies earlier,
so I clearly see the point. So if converting a subset of machines in an
architecture you will need this for a transitional period.

Is this helpful to get i.MX support in place?

On one hand I'd say we merge it for that reason alone, instead of requiring
all i.MX machines to be converted at once.

On the other hand, I fear that this will be used to avoid *ever* migrating
the old machines to pinctrl. And it's a generic disease that people working
in the arch/arm/* tree want to ditch old hardware without actually deleting
the code.

I'd like Sascha's view on this to make a good decision.

Yours,
Linus Walleij

[PATCH 1/1] pinctrl: handle dummy state in core

[Linus Walleij]

On Tue, Apr 17, 2012 at 5:42 AM, Dong Aisheng
<aisheng.dong@freescale.com> wrote:

> I think the main issue is for one platform neither supports dt nor using pinctrl
> subsystem, do you think it still makes too much sense to force that platform
> to define a _PINCTRL_ dummy state in their machine code?
>
> But the driver is commonly shared between these different platforms(using pinctrl
> or not).

So this is for the situation where some machines have pinctrl and some haven't.

And I think that's how some archs have used the regulator dummies earlier,
so I clearly see the point. So if converting a subset of machines in an
architecture you will need this for a transitional period.

Is this helpful to get i.MX support in place?

On one hand I'd say we merge it for that reason alone, instead of requiring
all i.MX machines to be converted at once.

On the other hand, I fear that this will be used to avoid *ever* migrating
the old machines to pinctrl. And it's a generic disease that people working
in the arch/arm/* tree want to ditch old hardware without actually deleting
the code.

I'd like Sascha's view on this to make a good decision.

Yours,
Linus Walleij

Re: [PATCH 1/1] pinctrl: handle dummy state in core

[Sascha Hauer]

On Wed, Apr 18, 2012 at 01:31:17PM +0200, Linus Walleij wrote:
> On Tue, Apr 17, 2012 at 5:42 AM, Dong Aisheng
> <aisheng.dong@freescale.com> wrote:
> 
> > I think the main issue is for one platform neither supports dt nor using pinctrl
> > subsystem, do you think it still makes too much sense to force that platform
> > to define a _PINCTRL_ dummy state in their machine code?
> >
> > But the driver is commonly shared between these different platforms(using pinctrl
> > or not).
> 
> So this is for the situation where some machines have pinctrl and some haven't.
> 
> And I think that's how some archs have used the regulator dummies earlier,
> so I clearly see the point. So if converting a subset of machines in an
> architecture you will need this for a transitional period.
> 
> Is this helpful to get i.MX support in place?
> 
> On one hand I'd say we merge it for that reason alone, instead of requiring
> all i.MX machines to be converted at once.
> 
> On the other hand, I fear that this will be used to avoid *ever* migrating
> the old machines to pinctrl. And it's a generic disease that people working
> in the arch/arm/* tree want to ditch old hardware without actually deleting
> the code.

'existing' doesn't necessarily mean 'old'

> 
> I'd like Sascha's view on this to make a good decision.

Currently Dong Aisheng is working on i.MX6, we have i.MX31, i.MX35,
i.MX51 and i.MX53 enabled in the same defconfig. Waiting for all
machines turn over at once to pinctrl is illusionary. We have to make
drivers work with and without pinctrl. We have a bunch of bad choices:

- create a dummy pinctrl for all boards which do not have real pinmux
  support
- ignore pinctrl_request errors in drivers.
- generate and return a dummy pinctrl in the core if no real pinctrl is
  found.

(replace pinctrl with regulators or clocks, it's the same situation, and
it's not only i.MX specific)

I don't have a good solution for this, only the idea that it should be
possible to attach these kinds of resources to a device and then a
driver would only call a make_me_work(dev).

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 |

[PATCH 1/1] pinctrl: handle dummy state in core

[Sascha Hauer]

On Wed, Apr 18, 2012 at 01:31:17PM +0200, Linus Walleij wrote:
> On Tue, Apr 17, 2012 at 5:42 AM, Dong Aisheng
> <aisheng.dong@freescale.com> wrote:
> 
> > I think the main issue is for one platform neither supports dt nor using pinctrl
> > subsystem, do you think it still makes too much sense to force that platform
> > to define a _PINCTRL_ dummy state in their machine code?
> >
> > But the driver is commonly shared between these different platforms(using pinctrl
> > or not).
> 
> So this is for the situation where some machines have pinctrl and some haven't.
> 
> And I think that's how some archs have used the regulator dummies earlier,
> so I clearly see the point. So if converting a subset of machines in an
> architecture you will need this for a transitional period.
> 
> Is this helpful to get i.MX support in place?
> 
> On one hand I'd say we merge it for that reason alone, instead of requiring
> all i.MX machines to be converted at once.
> 
> On the other hand, I fear that this will be used to avoid *ever* migrating
> the old machines to pinctrl. And it's a generic disease that people working
> in the arch/arm/* tree want to ditch old hardware without actually deleting
> the code.

'existing' doesn't necessarily mean 'old'

> 
> I'd like Sascha's view on this to make a good decision.

Currently Dong Aisheng is working on i.MX6, we have i.MX31, i.MX35,
i.MX51 and i.MX53 enabled in the same defconfig. Waiting for all
machines turn over at once to pinctrl is illusionary. We have to make
drivers work with and without pinctrl. We have a bunch of bad choices:

- create a dummy pinctrl for all boards which do not have real pinmux
  support
- ignore pinctrl_request errors in drivers.
- generate and return a dummy pinctrl in the core if no real pinctrl is
  found.

(replace pinctrl with regulators or clocks, it's the same situation, and
it's not only i.MX specific)

I don't have a good solution for this, only the idea that it should be
possible to attach these kinds of resources to a device and then a
driver would only call a make_me_work(dev).

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 |

Re: [PATCH 1/1] pinctrl: handle dummy state in core

[Linus Walleij]

On Wed, Apr 18, 2012 at 2:28 PM, Sascha Hauer <s.hauer@pengutronix.de> wrote:

> Currently Dong Aisheng is working on i.MX6, we have i.MX31, i.MX35,
> i.MX51 and i.MX53 enabled in the same defconfig. Waiting for all
> machines turn over at once to pinctrl is illusionary. We have to make
> drivers work with and without pinctrl. We have a bunch of bad choices:
>
> - create a dummy pinctrl for all boards which do not have real pinmux
>  support

Viable compromise.

> - ignore pinctrl_request errors in drivers.

Ugh. Not good.

> - generate and return a dummy pinctrl in the core if no real pinctrl is
>  found.
>
> (replace pinctrl with regulators or clocks, it's the same situation, and
> it's not only i.MX specific)

Since we have dummy regulators, we should not break the design
pattern creating more confusion.

Stephen can you live with dummy pinctrl handles emitted by the
core, if explictly enabled by a Kconfig option?

Yours,
Linus Walleij

[PATCH 1/1] pinctrl: handle dummy state in core

[Linus Walleij]

On Wed, Apr 18, 2012 at 2:28 PM, Sascha Hauer <s.hauer@pengutronix.de> wrote:

> Currently Dong Aisheng is working on i.MX6, we have i.MX31, i.MX35,
> i.MX51 and i.MX53 enabled in the same defconfig. Waiting for all
> machines turn over at once to pinctrl is illusionary. We have to make
> drivers work with and without pinctrl. We have a bunch of bad choices:
>
> - create a dummy pinctrl for all boards which do not have real pinmux
> ?support

Viable compromise.

> - ignore pinctrl_request errors in drivers.

Ugh. Not good.

> - generate and return a dummy pinctrl in the core if no real pinctrl is
> ?found.
>
> (replace pinctrl with regulators or clocks, it's the same situation, and
> it's not only i.MX specific)

Since we have dummy regulators, we should not break the design
pattern creating more confusion.

Stephen can you live with dummy pinctrl handles emitted by the
core, if explictly enabled by a Kconfig option?

Yours,
Linus Walleij

Re: [PATCH 1/1] pinctrl: handle dummy state in core

[Sascha Hauer]

On Wed, Apr 18, 2012 at 03:52:23PM +0200, Linus Walleij wrote:
> On Wed, Apr 18, 2012 at 2:28 PM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> 
> > Currently Dong Aisheng is working on i.MX6, we have i.MX31, i.MX35,
> > i.MX51 and i.MX53 enabled in the same defconfig. Waiting for all
> > machines turn over at once to pinctrl is illusionary. We have to make
> > drivers work with and without pinctrl. We have a bunch of bad choices:
> >
> > - create a dummy pinctrl for all boards which do not have real pinmux
> >  support
> 
> Viable compromise.
> 
> > - ignore pinctrl_request errors in drivers.
> 
> Ugh. Not good.
> 
> > - generate and return a dummy pinctrl in the core if no real pinctrl is
> >  found.
> >
> > (replace pinctrl with regulators or clocks, it's the same situation, and
> > it's not only i.MX specific)
> 
> Since we have dummy regulators, we should not break the design
> pattern creating more confusion.
> 
> Stephen can you live with dummy pinctrl handles emitted by the
> core, if explictly enabled by a Kconfig option?

I'd rather not make it a Kconfig option but a runtime option. With a
Kconfig option we can only chose in the defconfigs whether it won't
work on some boards or it will hide pinctrl errors on all other boards.

Create a pinctrl_provide_dummies() functions which boards can call when
they do not have pinctrl support.

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 |

[PATCH 1/1] pinctrl: handle dummy state in core

[Sascha Hauer]

On Wed, Apr 18, 2012 at 03:52:23PM +0200, Linus Walleij wrote:
> On Wed, Apr 18, 2012 at 2:28 PM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> 
> > Currently Dong Aisheng is working on i.MX6, we have i.MX31, i.MX35,
> > i.MX51 and i.MX53 enabled in the same defconfig. Waiting for all
> > machines turn over at once to pinctrl is illusionary. We have to make
> > drivers work with and without pinctrl. We have a bunch of bad choices:
> >
> > - create a dummy pinctrl for all boards which do not have real pinmux
> > ?support
> 
> Viable compromise.
> 
> > - ignore pinctrl_request errors in drivers.
> 
> Ugh. Not good.
> 
> > - generate and return a dummy pinctrl in the core if no real pinctrl is
> > ?found.
> >
> > (replace pinctrl with regulators or clocks, it's the same situation, and
> > it's not only i.MX specific)
> 
> Since we have dummy regulators, we should not break the design
> pattern creating more confusion.
> 
> Stephen can you live with dummy pinctrl handles emitted by the
> core, if explictly enabled by a Kconfig option?

I'd rather not make it a Kconfig option but a runtime option. With a
Kconfig option we can only chose in the defconfigs whether it won't
work on some boards or it will hide pinctrl errors on all other boards.

Create a pinctrl_provide_dummies() functions which boards can call when
they do not have pinctrl support.

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 |

Re: [PATCH 1/1] pinctrl: handle dummy state in core

[Dong Aisheng]

On Wed, Apr 18, 2012 at 10:00:46PM +0800, Sascha Hauer wrote:
> On Wed, Apr 18, 2012 at 03:52:23PM +0200, Linus Walleij wrote:
> > On Wed, Apr 18, 2012 at 2:28 PM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > 
> > > Currently Dong Aisheng is working on i.MX6, we have i.MX31, i.MX35,
> > > i.MX51 and i.MX53 enabled in the same defconfig. Waiting for all
> > > machines turn over at once to pinctrl is illusionary. We have to make
> > > drivers work with and without pinctrl. We have a bunch of bad choices:
> > >
> > > - create a dummy pinctrl for all boards which do not have real pinmux
> > >  support
> > 
> > Viable compromise.
> > 
> > > - ignore pinctrl_request errors in drivers.
> > 
> > Ugh. Not good.
> > 
> > > - generate and return a dummy pinctrl in the core if no real pinctrl is
> > >  found.
> > >
> > > (replace pinctrl with regulators or clocks, it's the same situation, and
> > > it's not only i.MX specific)
> > 
> > Since we have dummy regulators, we should not break the design
> > pattern creating more confusion.
> > 
> > Stephen can you live with dummy pinctrl handles emitted by the
> > core, if explictly enabled by a Kconfig option?
> 
> I'd rather not make it a Kconfig option but a runtime option. With a
> Kconfig option we can only chose in the defconfigs whether it won't
> work on some boards or it will hide pinctrl errors on all other boards.
> 
> Create a pinctrl_provide_dummies() functions which boards can call when
> they do not have pinctrl support.
> 
I think this is a feasible method and do better than Kconfig but it also
needs more changes.

>From a long term view all SoC may migrate to pinctrl subsystem, we first need
to change all board files that without pinctrl support to call this function
although just for enable pinctrl for one of them and finally we need remove
it again. (For imx, maybe more than 10 files and double if with dt enabled).
I'm not sure it's worth.

Turn on pinctrl debug information will tell us which devices are using
dummy state.

Regards
Dong Aisheng

[PATCH 1/1] pinctrl: handle dummy state in core

[Dong Aisheng]

On Wed, Apr 18, 2012 at 10:00:46PM +0800, Sascha Hauer wrote:
> On Wed, Apr 18, 2012 at 03:52:23PM +0200, Linus Walleij wrote:
> > On Wed, Apr 18, 2012 at 2:28 PM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > 
> > > Currently Dong Aisheng is working on i.MX6, we have i.MX31, i.MX35,
> > > i.MX51 and i.MX53 enabled in the same defconfig. Waiting for all
> > > machines turn over at once to pinctrl is illusionary. We have to make
> > > drivers work with and without pinctrl. We have a bunch of bad choices:
> > >
> > > - create a dummy pinctrl for all boards which do not have real pinmux
> > > ?support
> > 
> > Viable compromise.
> > 
> > > - ignore pinctrl_request errors in drivers.
> > 
> > Ugh. Not good.
> > 
> > > - generate and return a dummy pinctrl in the core if no real pinctrl is
> > > ?found.
> > >
> > > (replace pinctrl with regulators or clocks, it's the same situation, and
> > > it's not only i.MX specific)
> > 
> > Since we have dummy regulators, we should not break the design
> > pattern creating more confusion.
> > 
> > Stephen can you live with dummy pinctrl handles emitted by the
> > core, if explictly enabled by a Kconfig option?
> 
> I'd rather not make it a Kconfig option but a runtime option. With a
> Kconfig option we can only chose in the defconfigs whether it won't
> work on some boards or it will hide pinctrl errors on all other boards.
> 
> Create a pinctrl_provide_dummies() functions which boards can call when
> they do not have pinctrl support.
> 
I think this is a feasible method and do better than Kconfig but it also
needs more changes.

>From a long term view all SoC may migrate to pinctrl subsystem, we first need
to change all board files that without pinctrl support to call this function
although just for enable pinctrl for one of them and finally we need remove
it again. (For imx, maybe more than 10 files and double if with dt enabled).
I'm not sure it's worth.

Turn on pinctrl debug information will tell us which devices are using
dummy state.

Regards
Dong Aisheng

Re: [PATCH 1/1] pinctrl: handle dummy state in core

[Stephen Warren]

On 04/18/2012 07:52 AM, Linus Walleij wrote:
> On Wed, Apr 18, 2012 at 2:28 PM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> 
>> Currently Dong Aisheng is working on i.MX6, we have i.MX31, i.MX35,
>> i.MX51 and i.MX53 enabled in the same defconfig. Waiting for all
>> machines turn over at once to pinctrl is illusionary. We have to make
>> drivers work with and without pinctrl. We have a bunch of bad choices:
>>
>> - create a dummy pinctrl for all boards which do not have real pinmux
>>  support
> 
> Viable compromise.
> 
>> - ignore pinctrl_request errors in drivers.
> 
> Ugh. Not good.
> 
>> - generate and return a dummy pinctrl in the core if no real pinctrl is
>>  found.
>>
>> (replace pinctrl with regulators or clocks, it's the same situation, and
>> it's not only i.MX specific)
> 
> Since we have dummy regulators, we should not break the design
> pattern creating more confusion.
> 
> Stephen can you live with dummy pinctrl handles emitted by the
> core, if explicitly enabled by a Kconfig option?

Yes, I'm happy with the pinctrl core having the ability to "magically"
work when the dummy mapping table entries (or DT entries) are missing,
so long as there is a default-off Kconfig or runtime option that enables
that feature, which I'd expect to be use only for boards/platforms/...
that are "under conversion".

[PATCH 1/1] pinctrl: handle dummy state in core

[Stephen Warren]

On 04/18/2012 07:52 AM, Linus Walleij wrote:
> On Wed, Apr 18, 2012 at 2:28 PM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> 
>> Currently Dong Aisheng is working on i.MX6, we have i.MX31, i.MX35,
>> i.MX51 and i.MX53 enabled in the same defconfig. Waiting for all
>> machines turn over at once to pinctrl is illusionary. We have to make
>> drivers work with and without pinctrl. We have a bunch of bad choices:
>>
>> - create a dummy pinctrl for all boards which do not have real pinmux
>>  support
> 
> Viable compromise.
> 
>> - ignore pinctrl_request errors in drivers.
> 
> Ugh. Not good.
> 
>> - generate and return a dummy pinctrl in the core if no real pinctrl is
>>  found.
>>
>> (replace pinctrl with regulators or clocks, it's the same situation, and
>> it's not only i.MX specific)
> 
> Since we have dummy regulators, we should not break the design
> pattern creating more confusion.
> 
> Stephen can you live with dummy pinctrl handles emitted by the
> core, if explicitly enabled by a Kconfig option?

Yes, I'm happy with the pinctrl core having the ability to "magically"
work when the dummy mapping table entries (or DT entries) are missing,
so long as there is a default-off Kconfig or runtime option that enables
that feature, which I'd expect to be use only for boards/platforms/...
that are "under conversion".

Re: [PATCH 1/1] pinctrl: handle dummy state in core

[Linus Walleij]

On Wed, Apr 18, 2012 at 4:38 PM, Dong Aisheng
<aisheng.dong@freescale.com> wrote:
> On Wed, Apr 18, 2012 at 10:00:46PM +0800, Sascha Hauer wrote:

>> Create a pinctrl_provide_dummies() functions which boards can call when
>> they do not have pinctrl support.
>>
> I think this is a feasible method and do better than Kconfig but it also
> needs more changes.

If you are both happy with this (and I suspect Stephen can also accept
it), I'd say just do it.

Yours,
Linus Walleij

[PATCH 1/1] pinctrl: handle dummy state in core

[Linus Walleij]

On Wed, Apr 18, 2012 at 4:38 PM, Dong Aisheng
<aisheng.dong@freescale.com> wrote:
> On Wed, Apr 18, 2012 at 10:00:46PM +0800, Sascha Hauer wrote:

>> Create a pinctrl_provide_dummies() functions which boards can call when
>> they do not have pinctrl support.
>>
> I think this is a feasible method and do better than Kconfig but it also
> needs more changes.

If you are both happy with this (and I suspect Stephen can also accept
it), I'd say just do it.

Yours,
Linus Walleij