[PATCH v2 1/2] pinctrl: add pinctrl_provide_dummies interface for platforms to use

[PATCH v2 1/2] pinctrl: add pinctrl_provide_dummies interface for platforms to use

[Dong Aisheng]

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

Add a interface pinctrl_provide_dummies for platform to indicate
whether it needs use pinctrl dummy state and dummy gpio.

Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Stephen Warren <swarren@nvidia.com>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Signed-off-by: Dong Aisheng <dong.aisheng@linaro.org>
---
ChangeLog v1->v2:
* Based on sascha's suggestion, drop using kconfig since it will hide
  pinctrl errors on all other boards.
  See: https://lkml.org/lkml/2012/4/18/282
  It seemed both Linus and Stephen agreed with this way, so i'm ok
  with it too.
* add dummy gpio support.
  pinctrl gpio in the same situation as state.
* patch name changed.
  Original is pinctrl: handle dummy state in core.
* split removing old dt dummy interface into a separate patch
---
 drivers/pinctrl/core.c          |   35 ++++++++++++++++++++++++++++++++---
 include/linux/pinctrl/machine.h |    5 ++++-
 2 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
index 5cd5a5a..c6d0fdb 100644
--- a/drivers/pinctrl/core.c
+++ b/drivers/pinctrl/core.c
@@ -43,6 +43,9 @@ struct pinctrl_maps {
 	unsigned num_maps;
 };
 
+static bool pinctrl_dummy_state;
+static bool pinctrl_dummy_gpio;
+
 /* Mutex taken by all entry points */
 DEFINE_MUTEX(pinctrl_mutex);
 
@@ -61,6 +64,17 @@ static LIST_HEAD(pinctrl_maps);
 			_i_ < _maps_node_->num_maps; \
 			i++, _map_ = &_maps_node_->maps[_i_])
 
+/**
+ * pinctrl_provide_dummies() - indicate if provide dummies for state and gpio
+ * @state - provide dummy state
+ * @gpio - provide dummy gpio
+ */
+void pinctrl_provide_dummies(bool state, bool gpio)
+{
+	pinctrl_dummy_state = state;
+	pinctrl_dummy_gpio = gpio;
+}
+
 const char *pinctrl_dev_get_name(struct pinctrl_dev *pctldev)
 {
 	/* We're not allowed to register devices without name */
@@ -382,7 +396,12 @@ int pinctrl_request_gpio(unsigned gpio)
 	ret = pinctrl_get_device_gpio_range(gpio, &pctldev, &range);
 	if (ret) {
 		mutex_unlock(&pinctrl_mutex);
-		return -EINVAL;
+		if (pinctrl_dummy_gpio) {
+			pr_debug("pinctrl: using dummy gpio(%u)\n", gpio);
+			return 0;
+		} else {
+			return -EINVAL;
+		}
 	}
 
 	/* Convert to the pin controllers number space */
@@ -719,8 +738,18 @@ static struct pinctrl_state *pinctrl_lookup_state_locked(struct pinctrl *p,
 	struct pinctrl_state *state;
 
 	state = find_state(p, name);
-	if (!state)
-		return ERR_PTR(-ENODEV);
+	if (!state) {
+		if (pinctrl_dummy_state) {
+			/* create dummy state */
+			dev_dbg(p->dev, "using pinctrl dummy state (%s)\n",
+				name);
+			state = create_state(p, name);
+			if (IS_ERR(state))
+				return state;
+		} else {
+			return ERR_PTR(-ENODEV);
+		}
+	}
 
 	return state;
 }
diff --git a/include/linux/pinctrl/machine.h b/include/linux/pinctrl/machine.h
index e4d1de7..849a844 100644
--- a/include/linux/pinctrl/machine.h
+++ b/include/linux/pinctrl/machine.h
@@ -154,7 +154,7 @@ struct pinctrl_map {
 
 extern int pinctrl_register_mappings(struct pinctrl_map const *map,
 				unsigned num_maps);
-
+extern void pinctrl_provide_dummies(bool state, bool gpio);
 #else
 
 static inline int pinctrl_register_mappings(struct pinctrl_map const *map,
@@ -163,5 +163,8 @@ static inline int pinctrl_register_mappings(struct pinctrl_map const *map,
 	return 0;
 }
 
+static inline void pinctrl_provide_dummies(bool state, bool gpio)
+{
+}
 #endif /* !CONFIG_PINMUX */
 #endif
-- 
1.7.0.4

[PATCH v2 1/2] pinctrl: add pinctrl_provide_dummies interface for platforms to use

[Dong Aisheng]

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

Add a interface pinctrl_provide_dummies for platform to indicate
whether it needs use pinctrl dummy state and dummy gpio.

Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Stephen Warren <swarren@nvidia.com>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Signed-off-by: Dong Aisheng <dong.aisheng@linaro.org>
---
ChangeLog v1->v2:
* Based on sascha's suggestion, drop using kconfig since it will hide
  pinctrl errors on all other boards.
  See: https://lkml.org/lkml/2012/4/18/282
  It seemed both Linus and Stephen agreed with this way, so i'm ok
  with it too.
* add dummy gpio support.
  pinctrl gpio in the same situation as state.
* patch name changed.
  Original is pinctrl: handle dummy state in core.
* split removing old dt dummy interface into a separate patch
---
 drivers/pinctrl/core.c          |   35 ++++++++++++++++++++++++++++++++---
 include/linux/pinctrl/machine.h |    5 ++++-
 2 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
index 5cd5a5a..c6d0fdb 100644
--- a/drivers/pinctrl/core.c
+++ b/drivers/pinctrl/core.c
@@ -43,6 +43,9 @@ struct pinctrl_maps {
 	unsigned num_maps;
 };
 
+static bool pinctrl_dummy_state;
+static bool pinctrl_dummy_gpio;
+
 /* Mutex taken by all entry points */
 DEFINE_MUTEX(pinctrl_mutex);
 
@@ -61,6 +64,17 @@ static LIST_HEAD(pinctrl_maps);
 			_i_ < _maps_node_->num_maps; \
 			i++, _map_ = &_maps_node_->maps[_i_])
 
+/**
+ * pinctrl_provide_dummies() - indicate if provide dummies for state and gpio
+ * @state - provide dummy state
+ * @gpio - provide dummy gpio
+ */
+void pinctrl_provide_dummies(bool state, bool gpio)
+{
+	pinctrl_dummy_state = state;
+	pinctrl_dummy_gpio = gpio;
+}
+
 const char *pinctrl_dev_get_name(struct pinctrl_dev *pctldev)
 {
 	/* We're not allowed to register devices without name */
@@ -382,7 +396,12 @@ int pinctrl_request_gpio(unsigned gpio)
 	ret = pinctrl_get_device_gpio_range(gpio, &pctldev, &range);
 	if (ret) {
 		mutex_unlock(&pinctrl_mutex);
-		return -EINVAL;
+		if (pinctrl_dummy_gpio) {
+			pr_debug("pinctrl: using dummy gpio(%u)\n", gpio);
+			return 0;
+		} else {
+			return -EINVAL;
+		}
 	}
 
 	/* Convert to the pin controllers number space */
@@ -719,8 +738,18 @@ static struct pinctrl_state *pinctrl_lookup_state_locked(struct pinctrl *p,
 	struct pinctrl_state *state;
 
 	state = find_state(p, name);
-	if (!state)
-		return ERR_PTR(-ENODEV);
+	if (!state) {
+		if (pinctrl_dummy_state) {
+			/* create dummy state */
+			dev_dbg(p->dev, "using pinctrl dummy state (%s)\n",
+				name);
+			state = create_state(p, name);
+			if (IS_ERR(state))
+				return state;
+		} else {
+			return ERR_PTR(-ENODEV);
+		}
+	}
 
 	return state;
 }
diff --git a/include/linux/pinctrl/machine.h b/include/linux/pinctrl/machine.h
index e4d1de7..849a844 100644
--- a/include/linux/pinctrl/machine.h
+++ b/include/linux/pinctrl/machine.h
@@ -154,7 +154,7 @@ struct pinctrl_map {
 
 extern int pinctrl_register_mappings(struct pinctrl_map const *map,
 				unsigned num_maps);
-
+extern void pinctrl_provide_dummies(bool state, bool gpio);
 #else
 
 static inline int pinctrl_register_mappings(struct pinctrl_map const *map,
@@ -163,5 +163,8 @@ static inline int pinctrl_register_mappings(struct pinctrl_map const *map,
 	return 0;
 }
 
+static inline void pinctrl_provide_dummies(bool state, bool gpio)
+{
+}
 #endif /* !CONFIG_PINMUX */
 #endif
-- 
1.7.0.4

[PATCH v2 2/2] pinctrl: remove the old pinctrl dt dummy state interfaces

[Dong Aisheng]

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

We already have pinctrl_provide_dummies, so remove the old
one to avoid diversity.

Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Stephen Warren <swarren@nvidia.com>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Signed-off-by: Dong Aisheng <dong.aisheng@linaro.org>
---
ChangeLog v1->v2:
* No changes except reformed to a separate patch.

Hi Stephen,
I removed the old dt pinctrl dummy sate interface.
The purpose is to get a unified way to handle pinctrl dummy state.
One disadvantage is that we may not meet the requirement
that for platform which only want to use dummy state for some specific
devices while not affect others. For this case, it may reply on users
to refer to the pinctrl debug message to see which devices are using
dummy state while which are not.
However, if keep it we may have two type of user interface to handle
dummy state which i'm not sure is a good thing. And as regulator also
does not provide per device dummies, so i removed it first.

What's your suggestion on it?
I reform this clean up into a separate patch, if you do not like it,
we can drop it later.
---
 Documentation/pinctrl.txt       |   15 ++++-----------
 drivers/pinctrl/core.c          |   14 +++-----------
 drivers/pinctrl/core.h          |    3 +--
 drivers/pinctrl/devicetree.c    |   26 --------------------------
 include/linux/pinctrl/machine.h |   11 +----------
 5 files changed, 9 insertions(+), 60 deletions(-)

diff --git a/Documentation/pinctrl.txt b/Documentation/pinctrl.txt
index e40f4b4..261c41a 100644
--- a/Documentation/pinctrl.txt
+++ b/Documentation/pinctrl.txt
@@ -845,17 +845,10 @@ static struct pinctrl_map __initdata mapping[] = {
 	PIN_MAP_MUX_CONFIGS_PIN("foo-i2c.0", PINCTRL_STATE_DEFAULT, "pinctrl-foo", "i2c0sda", i2c_pin_configs),
 };
 
-Finally, some devices expect the mapping table to contain certain specific
-named states. When running on hardware that doesn't need any pin controller
-configuration, the mapping table must still contain those named states, in
-order to explicitly indicate that the states were provided and intended to
-be empty. Table entry macro PIN_MAP_DUMMY_STATE serves the purpose of defining
-a named state without causing any pin controller to be programmed:
-
-static struct pinctrl_map __initdata mapping[] = {
-	PIN_MAP_DUMMY_STATE("foo-i2c.0", PINCTRL_STATE_DEFAULT),
-};
-
+Finally, for platforms without pinctrl driver support but run with the shared
+drivers using pinctrl, user can call pinctrl_provide_dummies in board file
+to tell the core if these platforms want the core to provide dummies rather
+than return an error.
 
 Complex mappings
 ================
diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
index c6d0fdb..4c739d4 100644
--- a/drivers/pinctrl/core.c
+++ b/drivers/pinctrl/core.c
@@ -543,9 +543,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,
@@ -927,16 +924,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)
@@ -1139,7 +1133,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",
@@ -1166,9 +1159,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 849a844..0f72cb7 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 v2 2/2] pinctrl: remove the old pinctrl dt dummy state interfaces

[Dong Aisheng]

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

We already have pinctrl_provide_dummies, so remove the old
one to avoid diversity.

Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Stephen Warren <swarren@nvidia.com>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Signed-off-by: Dong Aisheng <dong.aisheng@linaro.org>
---
ChangeLog v1->v2:
* No changes except reformed to a separate patch.

Hi Stephen,
I removed the old dt pinctrl dummy sate interface.
The purpose is to get a unified way to handle pinctrl dummy state.
One disadvantage is that we may not meet the requirement
that for platform which only want to use dummy state for some specific
devices while not affect others. For this case, it may reply on users
to refer to the pinctrl debug message to see which devices are using
dummy state while which are not.
However, if keep it we may have two type of user interface to handle
dummy state which i'm not sure is a good thing. And as regulator also
does not provide per device dummies, so i removed it first.

What's your suggestion on it?
I reform this clean up into a separate patch, if you do not like it,
we can drop it later.
---
 Documentation/pinctrl.txt       |   15 ++++-----------
 drivers/pinctrl/core.c          |   14 +++-----------
 drivers/pinctrl/core.h          |    3 +--
 drivers/pinctrl/devicetree.c    |   26 --------------------------
 include/linux/pinctrl/machine.h |   11 +----------
 5 files changed, 9 insertions(+), 60 deletions(-)

diff --git a/Documentation/pinctrl.txt b/Documentation/pinctrl.txt
index e40f4b4..261c41a 100644
--- a/Documentation/pinctrl.txt
+++ b/Documentation/pinctrl.txt
@@ -845,17 +845,10 @@ static struct pinctrl_map __initdata mapping[] = {
 	PIN_MAP_MUX_CONFIGS_PIN("foo-i2c.0", PINCTRL_STATE_DEFAULT, "pinctrl-foo", "i2c0sda", i2c_pin_configs),
 };
 
-Finally, some devices expect the mapping table to contain certain specific
-named states. When running on hardware that doesn't need any pin controller
-configuration, the mapping table must still contain those named states, in
-order to explicitly indicate that the states were provided and intended to
-be empty. Table entry macro PIN_MAP_DUMMY_STATE serves the purpose of defining
-a named state without causing any pin controller to be programmed:
-
-static struct pinctrl_map __initdata mapping[] = {
-	PIN_MAP_DUMMY_STATE("foo-i2c.0", PINCTRL_STATE_DEFAULT),
-};
-
+Finally, for platforms without pinctrl driver support but run with the shared
+drivers using pinctrl, user can call pinctrl_provide_dummies in board file
+to tell the core if these platforms want the core to provide dummies rather
+than return an error.
 
 Complex mappings
 ================
diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
index c6d0fdb..4c739d4 100644
--- a/drivers/pinctrl/core.c
+++ b/drivers/pinctrl/core.c
@@ -543,9 +543,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,
@@ -927,16 +924,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)
@@ -1139,7 +1133,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",
@@ -1166,9 +1159,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 849a844..0f72cb7 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 v2 1/2] pinctrl: add pinctrl_provide_dummies interface for platforms to use

[Stephen Warren]

On 04/24/2012 03:33 AM, Dong Aisheng wrote:
> From: Dong Aisheng <dong.aisheng@linaro.org>
> 
> Add a interface pinctrl_provide_dummies for platform to indicate
> whether it needs use pinctrl dummy state and dummy gpio.

> @@ -382,7 +396,12 @@ int pinctrl_request_gpio(unsigned gpio)
>  	ret = pinctrl_get_device_gpio_range(gpio, &pctldev, &range);
>  	if (ret) {
>  		mutex_unlock(&pinctrl_mutex);
> -		return -EINVAL;
> +		if (pinctrl_dummy_gpio) {
> +			pr_debug("pinctrl: using dummy gpio(%u)\n", gpio);
> +			return 0;
> +		} else {
> +			return -EINVAL;
> +		}
>  	}

The only thing that should be calling pinctrl_request_gpio() is a GPIO
driver. It should only be calling it for the GPIOs it manages. I'd
expect that if a platform's pinctrl driver was not yet written to
support the GPIO functionality, then the GPIO driver would not be
calling this function.

As such, I'm not sure that this part of the change is necessary.

If it is, then surely all the other pinctrl GPIO APIs need a similar change?

Finally, how does this interact with deferred probe: How does the code
know whether the pinctrl driver and/or GPIO range is simply not yet
registered, or whether it never will be? That'd be the difference
between returning -EPROBE_DEFER or 0 in the if block above.


I'm fine with the other parts of this patch.

[PATCH v2 1/2] pinctrl: add pinctrl_provide_dummies interface for platforms to use

[Stephen Warren]

On 04/24/2012 03:33 AM, Dong Aisheng wrote:
> From: Dong Aisheng <dong.aisheng@linaro.org>
> 
> Add a interface pinctrl_provide_dummies for platform to indicate
> whether it needs use pinctrl dummy state and dummy gpio.

> @@ -382,7 +396,12 @@ int pinctrl_request_gpio(unsigned gpio)
>  	ret = pinctrl_get_device_gpio_range(gpio, &pctldev, &range);
>  	if (ret) {
>  		mutex_unlock(&pinctrl_mutex);
> -		return -EINVAL;
> +		if (pinctrl_dummy_gpio) {
> +			pr_debug("pinctrl: using dummy gpio(%u)\n", gpio);
> +			return 0;
> +		} else {
> +			return -EINVAL;
> +		}
>  	}

The only thing that should be calling pinctrl_request_gpio() is a GPIO
driver. It should only be calling it for the GPIOs it manages. I'd
expect that if a platform's pinctrl driver was not yet written to
support the GPIO functionality, then the GPIO driver would not be
calling this function.

As such, I'm not sure that this part of the change is necessary.

If it is, then surely all the other pinctrl GPIO APIs need a similar change?

Finally, how does this interact with deferred probe: How does the code
know whether the pinctrl driver and/or GPIO range is simply not yet
registered, or whether it never will be? That'd be the difference
between returning -EPROBE_DEFER or 0 in the if block above.


I'm fine with the other parts of this patch.

Re: [PATCH v2 2/2] pinctrl: remove the old pinctrl dt dummy state interfaces

[Stephen Warren]

On 04/24/2012 03:33 AM, Dong Aisheng wrote:
> From: Dong Aisheng <dong.aisheng@linaro.org>
> 
> We already have pinctrl_provide_dummies, so remove the old
> one to avoid diversity.

Nak, if I may be so bold.

> Hi Stephen,
> I removed the old dt pinctrl dummy sate interface.
> The purpose is to get a unified way to handle pinctrl dummy state.

Well, there are two completely different scenarios being covered here,
and I don't think it makes sense to unify them:

1) Platform under development without complete pinctrl support yet
(covered by patch 1 in this series).

2) Platform with complete pinctrl support, but using some common HW
modules whose drivers need to use pinctrl on some platforms, but not on
all, so that dummy states are required. This patch removes the ability
to correctly represent this situation.

> One disadvantage is that we may not meet the requirement
> that for platform which only want to use dummy state for some specific
> devices while not affect others. For this case, it may reply on users
> to refer to the pinctrl debug message to see which devices are using
> dummy state while which are not.
> However, if keep it we may have two type of user interface to handle
> dummy state which i'm not sure is a good thing. And as regulator also
> does not provide per device dummies, so i removed it first.

Well, first I'd say that if regulator didn't have this feature, it'd
probably just be a missing feature in regulator, and not a good
justification for removing the feature from pinctrl.

But that said, regulator does in fact have this feature - it's called
the fixed regulator.

> What's your suggestion on it?
> I reform this clean up into a separate patch, if you do not like it,
> we can drop it later.

I'd love to drop it, please.

[PATCH v2 2/2] pinctrl: remove the old pinctrl dt dummy state interfaces

[Stephen Warren]

On 04/24/2012 03:33 AM, Dong Aisheng wrote:
> From: Dong Aisheng <dong.aisheng@linaro.org>
> 
> We already have pinctrl_provide_dummies, so remove the old
> one to avoid diversity.

Nak, if I may be so bold.

> Hi Stephen,
> I removed the old dt pinctrl dummy sate interface.
> The purpose is to get a unified way to handle pinctrl dummy state.

Well, there are two completely different scenarios being covered here,
and I don't think it makes sense to unify them:

1) Platform under development without complete pinctrl support yet
(covered by patch 1 in this series).

2) Platform with complete pinctrl support, but using some common HW
modules whose drivers need to use pinctrl on some platforms, but not on
all, so that dummy states are required. This patch removes the ability
to correctly represent this situation.

> One disadvantage is that we may not meet the requirement
> that for platform which only want to use dummy state for some specific
> devices while not affect others. For this case, it may reply on users
> to refer to the pinctrl debug message to see which devices are using
> dummy state while which are not.
> However, if keep it we may have two type of user interface to handle
> dummy state which i'm not sure is a good thing. And as regulator also
> does not provide per device dummies, so i removed it first.

Well, first I'd say that if regulator didn't have this feature, it'd
probably just be a missing feature in regulator, and not a good
justification for removing the feature from pinctrl.

But that said, regulator does in fact have this feature - it's called
the fixed regulator.

> What's your suggestion on it?
> I reform this clean up into a separate patch, if you do not like it,
> we can drop it later.

I'd love to drop it, please.

Re: [PATCH v2 1/2] pinctrl: add pinctrl_provide_dummies interface for platforms to use

[Dong Aisheng]

On Wed, Apr 25, 2012 at 03:00:23AM +0800, Stephen Warren wrote:
> On 04/24/2012 03:33 AM, Dong Aisheng wrote:
> > From: Dong Aisheng <dong.aisheng@linaro.org>
> > 
> > Add a interface pinctrl_provide_dummies for platform to indicate
> > whether it needs use pinctrl dummy state and dummy gpio.
> 
> > @@ -382,7 +396,12 @@ int pinctrl_request_gpio(unsigned gpio)
> >  	ret = pinctrl_get_device_gpio_range(gpio, &pctldev, &range);
> >  	if (ret) {
> >  		mutex_unlock(&pinctrl_mutex);
> > -		return -EINVAL;
> > +		if (pinctrl_dummy_gpio) {
> > +			pr_debug("pinctrl: using dummy gpio(%u)\n", gpio);
> > +			return 0;
> > +		} else {
> > +			return -EINVAL;
> > +		}
> >  	}
> 
> The only thing that should be calling pinctrl_request_gpio() is a GPIO
> driver. It should only be calling it for the GPIOs it manages. I'd
> expect that if a platform's pinctrl driver was not yet written to
> support the GPIO functionality, then the GPIO driver would not be
> calling this function.
> 
Hmm, pinctrl gpio is in the same situation as pinctrl state that gpio
driver may be shared between several platforms, with pinctrl support
or not.

> As such, I'm not sure that this part of the change is necessary.
> 
> If it is, then surely all the other pinctrl GPIO APIs need a similar change?
> 
Yes, i missed it, will fix.
Thanks for reminder.

> Finally, how does this interact with deferred probe: How does the code
> know whether the pinctrl driver and/or GPIO range is simply not yet
> registered, or whether it never will be? That'd be the difference
> between returning -EPROBE_DEFER or 0 in the if block above.
> 
Yes, it is an issue.
I don't have any good idea to distinguish them.
It seems regulator has the same issue.
What regulator does is if dummy regulator is used, use dummy gpio instead
regardless of defer probe.
I guess we may use the same way as regulator for pinctrl.
User can decide if using dummy gpio or defer probe since based on their
driver support.

The rule may be if your pinctrl driver supports gpio, then you should not use
dummy gpio. Then the dummy gpio support will not affect the defer probe.

> 
> I'm fine with the other parts of this patch.

Regards
Dong Aisheng

[PATCH v2 1/2] pinctrl: add pinctrl_provide_dummies interface for platforms to use

[Dong Aisheng]

On Wed, Apr 25, 2012 at 03:00:23AM +0800, Stephen Warren wrote:
> On 04/24/2012 03:33 AM, Dong Aisheng wrote:
> > From: Dong Aisheng <dong.aisheng@linaro.org>
> > 
> > Add a interface pinctrl_provide_dummies for platform to indicate
> > whether it needs use pinctrl dummy state and dummy gpio.
> 
> > @@ -382,7 +396,12 @@ int pinctrl_request_gpio(unsigned gpio)
> >  	ret = pinctrl_get_device_gpio_range(gpio, &pctldev, &range);
> >  	if (ret) {
> >  		mutex_unlock(&pinctrl_mutex);
> > -		return -EINVAL;
> > +		if (pinctrl_dummy_gpio) {
> > +			pr_debug("pinctrl: using dummy gpio(%u)\n", gpio);
> > +			return 0;
> > +		} else {
> > +			return -EINVAL;
> > +		}
> >  	}
> 
> The only thing that should be calling pinctrl_request_gpio() is a GPIO
> driver. It should only be calling it for the GPIOs it manages. I'd
> expect that if a platform's pinctrl driver was not yet written to
> support the GPIO functionality, then the GPIO driver would not be
> calling this function.
> 
Hmm, pinctrl gpio is in the same situation as pinctrl state that gpio
driver may be shared between several platforms, with pinctrl support
or not.

> As such, I'm not sure that this part of the change is necessary.
> 
> If it is, then surely all the other pinctrl GPIO APIs need a similar change?
> 
Yes, i missed it, will fix.
Thanks for reminder.

> Finally, how does this interact with deferred probe: How does the code
> know whether the pinctrl driver and/or GPIO range is simply not yet
> registered, or whether it never will be? That'd be the difference
> between returning -EPROBE_DEFER or 0 in the if block above.
> 
Yes, it is an issue.
I don't have any good idea to distinguish them.
It seems regulator has the same issue.
What regulator does is if dummy regulator is used, use dummy gpio instead
regardless of defer probe.
I guess we may use the same way as regulator for pinctrl.
User can decide if using dummy gpio or defer probe since based on their
driver support.

The rule may be if your pinctrl driver supports gpio, then you should not use
dummy gpio. Then the dummy gpio support will not affect the defer probe.

> 
> I'm fine with the other parts of this patch.

Regards
Dong Aisheng

Re: [PATCH v2 1/2] pinctrl: add pinctrl_provide_dummies interface for platforms to use

[Linus Walleij]

On Wed, Apr 25, 2012 at 11:49 AM, Dong Aisheng
<aisheng.dong@freescale.com> wrote:
> On Wed, Apr 25, 2012 at 03:00:23AM +0800, Stephen Warren wrote:

>> The only thing that should be calling pinctrl_request_gpio() is a GPIO
>> driver. It should only be calling it for the GPIOs it manages. I'd
>> expect that if a platform's pinctrl driver was not yet written to
>> support the GPIO functionality, then the GPIO driver would not be
>> calling this function.
>>
> Hmm, pinctrl gpio is in the same situation as pinctrl state that gpio
> driver may be shared between several platforms, with pinctrl support
> or not.

I think it's mostly safe to assume that either:

- pinctrl calls from GPIO drivers gets stubbed out totally due to
  CONFIG_PINCTRL not being selected

or:

- You need to pass a token through platform data to the
  GPIO driver telling it whether it needs to request pins for
  it's GPIOs or not. Just a bool should work fine?

Linus Walleij

[PATCH v2 1/2] pinctrl: add pinctrl_provide_dummies interface for platforms to use

[Linus Walleij]

On Wed, Apr 25, 2012 at 11:49 AM, Dong Aisheng
<aisheng.dong@freescale.com> wrote:
> On Wed, Apr 25, 2012 at 03:00:23AM +0800, Stephen Warren wrote:

>> The only thing that should be calling pinctrl_request_gpio() is a GPIO
>> driver. It should only be calling it for the GPIOs it manages. I'd
>> expect that if a platform's pinctrl driver was not yet written to
>> support the GPIO functionality, then the GPIO driver would not be
>> calling this function.
>>
> Hmm, pinctrl gpio is in the same situation as pinctrl state that gpio
> driver may be shared between several platforms, with pinctrl support
> or not.

I think it's mostly safe to assume that either:

- pinctrl calls from GPIO drivers gets stubbed out totally due to
  CONFIG_PINCTRL not being selected

or:

- You need to pass a token through platform data to the
  GPIO driver telling it whether it needs to request pins for
  it's GPIOs or not. Just a bool should work fine?

Linus Walleij

Re: [PATCH v2 1/2] pinctrl: add pinctrl_provide_dummies interface for platforms to use

[Dong Aisheng]

On Wed, Apr 25, 2012 at 07:19:43PM +0800, Linus Walleij wrote:
> On Wed, Apr 25, 2012 at 11:49 AM, Dong Aisheng
> <aisheng.dong@freescale.com> wrote:
> > On Wed, Apr 25, 2012 at 03:00:23AM +0800, Stephen Warren wrote:
> 
> >> The only thing that should be calling pinctrl_request_gpio() is a GPIO
> >> driver. It should only be calling it for the GPIOs it manages. I'd
> >> expect that if a platform's pinctrl driver was not yet written to
> >> support the GPIO functionality, then the GPIO driver would not be
> >> calling this function.
> >>
> > Hmm, pinctrl gpio is in the same situation as pinctrl state that gpio
> > driver may be shared between several platforms, with pinctrl support
> > or not.
> 
> I think it's mostly safe to assume that either:
> 
I just saw your reply after i sent out the revised patch...

> - pinctrl calls from GPIO drivers gets stubbed out totally due to
>   CONFIG_PINCTRL not being selected
> 
Yes, we already have that in include/linux/pinctrl/consumer.h

> or:
> 
> - You need to pass a token through platform data to the
>   GPIO driver telling it whether it needs to request pins for
>   it's GPIOs or not. Just a bool should work fine?
> 
Yes, this is an alternative way.
I'm using a similar way, but pass the data to pinctrl core
rather than gpio driver. Then it is be handled together with
dummy state in platform code.

Do you think if the current way i used is ok?
Or i need to change to your proposed way?

Regards
Dong Aisheng

[PATCH v2 1/2] pinctrl: add pinctrl_provide_dummies interface for platforms to use

[Dong Aisheng]

On Wed, Apr 25, 2012 at 07:19:43PM +0800, Linus Walleij wrote:
> On Wed, Apr 25, 2012 at 11:49 AM, Dong Aisheng
> <aisheng.dong@freescale.com> wrote:
> > On Wed, Apr 25, 2012 at 03:00:23AM +0800, Stephen Warren wrote:
> 
> >> The only thing that should be calling pinctrl_request_gpio() is a GPIO
> >> driver. It should only be calling it for the GPIOs it manages. I'd
> >> expect that if a platform's pinctrl driver was not yet written to
> >> support the GPIO functionality, then the GPIO driver would not be
> >> calling this function.
> >>
> > Hmm, pinctrl gpio is in the same situation as pinctrl state that gpio
> > driver may be shared between several platforms, with pinctrl support
> > or not.
> 
> I think it's mostly safe to assume that either:
> 
I just saw your reply after i sent out the revised patch...

> - pinctrl calls from GPIO drivers gets stubbed out totally due to
>   CONFIG_PINCTRL not being selected
> 
Yes, we already have that in include/linux/pinctrl/consumer.h

> or:
> 
> - You need to pass a token through platform data to the
>   GPIO driver telling it whether it needs to request pins for
>   it's GPIOs or not. Just a bool should work fine?
> 
Yes, this is an alternative way.
I'm using a similar way, but pass the data to pinctrl core
rather than gpio driver. Then it is be handled together with
dummy state in platform code.

Do you think if the current way i used is ok?
Or i need to change to your proposed way?

Regards
Dong Aisheng

Re: [PATCH v2 2/2] pinctrl: remove the old pinctrl dt dummy state interfaces

[Dong Aisheng]

On Wed, Apr 25, 2012 at 03:04:27AM +0800, Stephen Warren wrote:
> On 04/24/2012 03:33 AM, Dong Aisheng wrote:
> > From: Dong Aisheng <dong.aisheng@linaro.org>
> > 
> > We already have pinctrl_provide_dummies, so remove the old
> > one to avoid diversity.
> 
> Nak, if I may be so bold.
> 
No, it's fine to me.
Since i'm not sure this patch is really needed, so i sent it out to
request for your comment.

> > Hi Stephen,
> > I removed the old dt pinctrl dummy sate interface.
> > The purpose is to get a unified way to handle pinctrl dummy state.
> 
> Well, there are two completely different scenarios being covered here,
> and I don't think it makes sense to unify them:
> 
> 1) Platform under development without complete pinctrl support yet
> (covered by patch 1 in this series).
> 
> 2) Platform with complete pinctrl support, but using some common HW
> modules whose drivers need to use pinctrl on some platforms, but not on
> all, so that dummy states are required. This patch removes the ability
> to correctly represent this situation.
> 
Looks reasonable to me.

> > One disadvantage is that we may not meet the requirement
> > that for platform which only want to use dummy state for some specific
> > devices while not affect others. For this case, it may reply on users
> > to refer to the pinctrl debug message to see which devices are using
> > dummy state while which are not.
> > However, if keep it we may have two type of user interface to handle
> > dummy state which i'm not sure is a good thing. And as regulator also
> > does not provide per device dummies, so i removed it first.
> 
> Well, first I'd say that if regulator didn't have this feature, it'd
> probably just be a missing feature in regulator, and not a good
> justification for removing the feature from pinctrl.
> 
> But that said, regulator does in fact have this feature - it's called
> the fixed regulator.
> 
Yes, i see.

> > What's your suggestion on it?
> > I reform this clean up into a separate patch, if you do not like it,
> > we can drop it later.
> 
> I'd love to drop it, please.
Okay, we can drop it.

Regards
Dong Aisheng

[PATCH v2 2/2] pinctrl: remove the old pinctrl dt dummy state interfaces

[Dong Aisheng]

On Wed, Apr 25, 2012 at 03:04:27AM +0800, Stephen Warren wrote:
> On 04/24/2012 03:33 AM, Dong Aisheng wrote:
> > From: Dong Aisheng <dong.aisheng@linaro.org>
> > 
> > We already have pinctrl_provide_dummies, so remove the old
> > one to avoid diversity.
> 
> Nak, if I may be so bold.
> 
No, it's fine to me.
Since i'm not sure this patch is really needed, so i sent it out to
request for your comment.

> > Hi Stephen,
> > I removed the old dt pinctrl dummy sate interface.
> > The purpose is to get a unified way to handle pinctrl dummy state.
> 
> Well, there are two completely different scenarios being covered here,
> and I don't think it makes sense to unify them:
> 
> 1) Platform under development without complete pinctrl support yet
> (covered by patch 1 in this series).
> 
> 2) Platform with complete pinctrl support, but using some common HW
> modules whose drivers need to use pinctrl on some platforms, but not on
> all, so that dummy states are required. This patch removes the ability
> to correctly represent this situation.
> 
Looks reasonable to me.

> > One disadvantage is that we may not meet the requirement
> > that for platform which only want to use dummy state for some specific
> > devices while not affect others. For this case, it may reply on users
> > to refer to the pinctrl debug message to see which devices are using
> > dummy state while which are not.
> > However, if keep it we may have two type of user interface to handle
> > dummy state which i'm not sure is a good thing. And as regulator also
> > does not provide per device dummies, so i removed it first.
> 
> Well, first I'd say that if regulator didn't have this feature, it'd
> probably just be a missing feature in regulator, and not a good
> justification for removing the feature from pinctrl.
> 
> But that said, regulator does in fact have this feature - it's called
> the fixed regulator.
> 
Yes, i see.

> > What's your suggestion on it?
> > I reform this clean up into a separate patch, if you do not like it,
> > we can drop it later.
> 
> I'd love to drop it, please.
Okay, we can drop it.

Regards
Dong Aisheng

Re: [PATCH v2 1/2] pinctrl: add pinctrl_provide_dummies interface for platforms to use

[Stephen Warren]

On 04/25/2012 05:49 AM, Dong Aisheng wrote:
> On Wed, Apr 25, 2012 at 07:19:43PM +0800, Linus Walleij wrote:
>> On Wed, Apr 25, 2012 at 11:49 AM, Dong Aisheng
>> <aisheng.dong@freescale.com> wrote:
>>> On Wed, Apr 25, 2012 at 03:00:23AM +0800, Stephen Warren wrote:
>>
>>>> The only thing that should be calling pinctrl_request_gpio() is a GPIO
>>>> driver. It should only be calling it for the GPIOs it manages. I'd
>>>> expect that if a platform's pinctrl driver was not yet written to
>>>> support the GPIO functionality, then the GPIO driver would not be
>>>> calling this function.
>>>>
>>> Hmm, pinctrl gpio is in the same situation as pinctrl state that gpio
>>> driver may be shared between several platforms, with pinctrl support
>>> or not.
>>
>> I think it's mostly safe to assume that either:
>>
> I just saw your reply after i sent out the revised patch...
> 
>> - pinctrl calls from GPIO drivers gets stubbed out totally due to
>>   CONFIG_PINCTRL not being selected
>>
> Yes, we already have that in include/linux/pinctrl/consumer.h
> 
>> or:
>>
>> - You need to pass a token through platform data to the
>>   GPIO driver telling it whether it needs to request pins for
>>   it's GPIOs or not. Just a bool should work fine?
>>
> Yes, this is an alternative way.
> I'm using a similar way, but pass the data to pinctrl core
> rather than gpio driver. Then it is be handled together with
> dummy state in platform code.
> 
> Do you think if the current way i used is ok?
> Or i need to change to your proposed way?

I think Linus was suggesting a flag in platform data for each GPIO
driver rather than a global flag for the entire pinctrl subsystem.

That way, if one of the pinctrl drivers did fully support all the GPIO
functionality and the other didn't, you'd be able to have just one of
the GPIO drivers not call into pinctrl (or ignore certain errors) yet
the other GPIO driver could still fully interact with pinctrl as desired.

[PATCH v2 1/2] pinctrl: add pinctrl_provide_dummies interface for platforms to use

[Stephen Warren]

On 04/25/2012 05:49 AM, Dong Aisheng wrote:
> On Wed, Apr 25, 2012 at 07:19:43PM +0800, Linus Walleij wrote:
>> On Wed, Apr 25, 2012 at 11:49 AM, Dong Aisheng
>> <aisheng.dong@freescale.com> wrote:
>>> On Wed, Apr 25, 2012 at 03:00:23AM +0800, Stephen Warren wrote:
>>
>>>> The only thing that should be calling pinctrl_request_gpio() is a GPIO
>>>> driver. It should only be calling it for the GPIOs it manages. I'd
>>>> expect that if a platform's pinctrl driver was not yet written to
>>>> support the GPIO functionality, then the GPIO driver would not be
>>>> calling this function.
>>>>
>>> Hmm, pinctrl gpio is in the same situation as pinctrl state that gpio
>>> driver may be shared between several platforms, with pinctrl support
>>> or not.
>>
>> I think it's mostly safe to assume that either:
>>
> I just saw your reply after i sent out the revised patch...
> 
>> - pinctrl calls from GPIO drivers gets stubbed out totally due to
>>   CONFIG_PINCTRL not being selected
>>
> Yes, we already have that in include/linux/pinctrl/consumer.h
> 
>> or:
>>
>> - You need to pass a token through platform data to the
>>   GPIO driver telling it whether it needs to request pins for
>>   it's GPIOs or not. Just a bool should work fine?
>>
> Yes, this is an alternative way.
> I'm using a similar way, but pass the data to pinctrl core
> rather than gpio driver. Then it is be handled together with
> dummy state in platform code.
> 
> Do you think if the current way i used is ok?
> Or i need to change to your proposed way?

I think Linus was suggesting a flag in platform data for each GPIO
driver rather than a global flag for the entire pinctrl subsystem.

That way, if one of the pinctrl drivers did fully support all the GPIO
functionality and the other didn't, you'd be able to have just one of
the GPIO drivers not call into pinctrl (or ignore certain errors) yet
the other GPIO driver could still fully interact with pinctrl as desired.

Re: [PATCH v2 1/2] pinctrl: add pinctrl_provide_dummies interface for platforms to use

[Dong Aisheng]

On Wed, Apr 25, 2012 at 11:22:11PM +0800, Stephen Warren wrote:
> On 04/25/2012 05:49 AM, Dong Aisheng wrote:
> > On Wed, Apr 25, 2012 at 07:19:43PM +0800, Linus Walleij wrote:
> >> On Wed, Apr 25, 2012 at 11:49 AM, Dong Aisheng
> >> <aisheng.dong@freescale.com> wrote:
> >>> On Wed, Apr 25, 2012 at 03:00:23AM +0800, Stephen Warren wrote:
> >>
> >>>> The only thing that should be calling pinctrl_request_gpio() is a GPIO
> >>>> driver. It should only be calling it for the GPIOs it manages. I'd
> >>>> expect that if a platform's pinctrl driver was not yet written to
> >>>> support the GPIO functionality, then the GPIO driver would not be
> >>>> calling this function.
> >>>>
> >>> Hmm, pinctrl gpio is in the same situation as pinctrl state that gpio
> >>> driver may be shared between several platforms, with pinctrl support
> >>> or not.
> >>
> >> I think it's mostly safe to assume that either:
> >>
> > I just saw your reply after i sent out the revised patch...
> > 
> >> - pinctrl calls from GPIO drivers gets stubbed out totally due to
> >>   CONFIG_PINCTRL not being selected
> >>
> > Yes, we already have that in include/linux/pinctrl/consumer.h
> > 
> >> or:
> >>
> >> - You need to pass a token through platform data to the
> >>   GPIO driver telling it whether it needs to request pins for
> >>   it's GPIOs or not. Just a bool should work fine?
> >>
> > Yes, this is an alternative way.
> > I'm using a similar way, but pass the data to pinctrl core
> > rather than gpio driver. Then it is be handled together with
> > dummy state in platform code.
> > 
> > Do you think if the current way i used is ok?
> > Or i need to change to your proposed way?
> 
> I think Linus was suggesting a flag in platform data for each GPIO
> driver rather than a global flag for the entire pinctrl subsystem.
> 
> That way, if one of the pinctrl drivers did fully support all the GPIO
> functionality and the other didn't, you'd be able to have just one of
> the GPIO drivers not call into pinctrl (or ignore certain errors) yet
> the other GPIO driver could still fully interact with pinctrl as desired.
> 
Yes, it's true.
I will drop the dummy gpio support in pinctrl subsystem and let gpio
driver to decide whether it wants to use pinctrl gpio mux function.

Regards
Dong Aisheng

[PATCH v2 1/2] pinctrl: add pinctrl_provide_dummies interface for platforms to use

[Dong Aisheng]

On Wed, Apr 25, 2012 at 11:22:11PM +0800, Stephen Warren wrote:
> On 04/25/2012 05:49 AM, Dong Aisheng wrote:
> > On Wed, Apr 25, 2012 at 07:19:43PM +0800, Linus Walleij wrote:
> >> On Wed, Apr 25, 2012 at 11:49 AM, Dong Aisheng
> >> <aisheng.dong@freescale.com> wrote:
> >>> On Wed, Apr 25, 2012 at 03:00:23AM +0800, Stephen Warren wrote:
> >>
> >>>> The only thing that should be calling pinctrl_request_gpio() is a GPIO
> >>>> driver. It should only be calling it for the GPIOs it manages. I'd
> >>>> expect that if a platform's pinctrl driver was not yet written to
> >>>> support the GPIO functionality, then the GPIO driver would not be
> >>>> calling this function.
> >>>>
> >>> Hmm, pinctrl gpio is in the same situation as pinctrl state that gpio
> >>> driver may be shared between several platforms, with pinctrl support
> >>> or not.
> >>
> >> I think it's mostly safe to assume that either:
> >>
> > I just saw your reply after i sent out the revised patch...
> > 
> >> - pinctrl calls from GPIO drivers gets stubbed out totally due to
> >>   CONFIG_PINCTRL not being selected
> >>
> > Yes, we already have that in include/linux/pinctrl/consumer.h
> > 
> >> or:
> >>
> >> - You need to pass a token through platform data to the
> >>   GPIO driver telling it whether it needs to request pins for
> >>   it's GPIOs or not. Just a bool should work fine?
> >>
> > Yes, this is an alternative way.
> > I'm using a similar way, but pass the data to pinctrl core
> > rather than gpio driver. Then it is be handled together with
> > dummy state in platform code.
> > 
> > Do you think if the current way i used is ok?
> > Or i need to change to your proposed way?
> 
> I think Linus was suggesting a flag in platform data for each GPIO
> driver rather than a global flag for the entire pinctrl subsystem.
> 
> That way, if one of the pinctrl drivers did fully support all the GPIO
> functionality and the other didn't, you'd be able to have just one of
> the GPIO drivers not call into pinctrl (or ignore certain errors) yet
> the other GPIO driver could still fully interact with pinctrl as desired.
> 
Yes, it's true.
I will drop the dummy gpio support in pinctrl subsystem and let gpio
driver to decide whether it wants to use pinctrl gpio mux function.

Regards
Dong Aisheng