Re: [PATCH 2/4] pinctrl: Allow pinctrl to have multiple active states
* Linus Walleij linus.wall...@linaro.org [130722 16:01]:
On Thu, Jul 18, 2013 at 5:15 PM, Tony Lindgren t...@atomide.com wrote:
It's quite common that we need to dynamically change some pins for a
device for runtime PM, or toggle a pin between rx and tx. Changing all
What does change mean above?
Please reword to remux if that is what is meant.
Yes remux or reconfigure the pinconf depending on the hardware.
And possibly based on the board specific configuration if both
options are available.
the pins for a device is not efficient way of doing it.
So let's allow setting up multiple active states for pinctrl.
Do you mean multiple default states?
I have a hard time understanding so please help me out :-/
One default state, which should only contain the static pins that
don't need to be remuxed or reconfigured during runtime PM.
It should probably say support for multiple currently activated
states, does that make it clearer?
Currently
we only need PINCTRL_STATIC and PINCTRL_DYNAMIC, where PINCTRL_STATIC
covers the current default pins, and PINCTRL_DYNAMIC holds the dynamic
pins that need to be toggled.
Toggled when?
Remuxed or reconfigured constantly for runtime PM.
When changing state to another one? Any other state? Or back to
this state? Sorry not following ... this needs to be more verbose.
How about toggle dynamic pins between active and idle state
for runtime PM?
--- a/drivers/pinctrl/core.h
+++ b/drivers/pinctrl/core.h
@@ -53,12 +53,18 @@ struct pinctrl_dev {
#endif
};
+enum pinctrl_states {
+ PINCTRL_STATIC,
+ PINCTRL_DYNAMIC,
+ PINCTRL_NR_STATES,
+};
This needs some very precise kerneldoc so it is chrystal clear what these
states are all about. pinctrl_states is not a good name for this enum,
please find some more precise name, because we have functions
with names like pinctrl_select_state() which is not related to this.
Is this substates or?
This lists the sets of pins that can be activated the same
time. So after this, we support two sets of pins that a
pinctrl consumer driver can have activated the same time:
static pins and dynamic pins. There can be any number of
named modes for dynamic pins, but typically only active and
idle.
Then if for some reason we need more than two sets activated
the same time, we can expand the array as needed. So that's
mostly to try to keep things future proof and not limit things
to two sets as noted by Stephen earlier.
+
/**
* struct pinctrl - per-device pin control state holder
* @node: global list node
* @dev: the device using this pin control handle
* @states: a list of states for this device
- * @state: the current state
+ * @state: the current state(s)
How can more than one state be the current state? Sorry I don't get this.
This needs to be more precise I think.
Hopefully the explanation above helps with that, if not,
let me know.
Regards,
Tony
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] pinctrl: Allow pinctrl to have multiple active states
On Thu, Jul 18, 2013 at 5:15 PM, Tony Lindgren t...@atomide.com wrote:
It's quite common that we need to dynamically change some pins for a
device for runtime PM, or toggle a pin between rx and tx. Changing all
What does change mean above?
Please reword to remux if that is what is meant.
the pins for a device is not efficient way of doing it.
So let's allow setting up multiple active states for pinctrl.
Do you mean multiple default states?
I have a hard time understanding so please help me out :-/
Currently
we only need PINCTRL_STATIC and PINCTRL_DYNAMIC, where PINCTRL_STATIC
covers the current default pins, and PINCTRL_DYNAMIC holds the dynamic
pins that need to be toggled.
Toggled when?
When changing state to another one? Any other state? Or back to
this state? Sorry not following ... this needs to be more verbose.
Cc: Felipe Balbi ba...@ti.com
Cc: Grygorii Strashko grygorii.stras...@ti.com
Cc: Stephen Warren swar...@wwwdotorg.org
Signed-off-by: Tony Lindgren t...@atomide.com
---
drivers/pinctrl/core.c | 18 ++
drivers/pinctrl/core.h | 10 --
2 files changed, 18 insertions(+), 10 deletions(-)
diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
index a97b717..c9b709b 100644
--- a/drivers/pinctrl/core.c
+++ b/drivers/pinctrl/core.c
@@ -885,7 +885,8 @@ static void pinctrl_free(struct pinctrl *p, bool inlist)
mutex_lock(pinctrl_list_mutex);
list_for_each_entry_safe(state, n1, p-states, node) {
list_for_each_entry_safe(setting, n2, state-settings, node)
{
- pinctrl_free_setting(state == p-state, setting);
+ pinctrl_free_setting(state ==
p-state[PINCTRL_STATIC],
+setting);
list_del(setting-node);
kfree(setting);
}
@@ -955,13 +956,13 @@ EXPORT_SYMBOL_GPL(pinctrl_lookup_state);
int pinctrl_select_state(struct pinctrl *p, struct pinctrl_state *state)
{
struct pinctrl_setting *setting, *setting2;
- struct pinctrl_state *old_state = p-state;
+ struct pinctrl_state *old_state = p-state[PINCTRL_STATIC];
int ret;
- if (p-state == state)
+ if (old_state == state)
return 0;
- if (p-state) {
+ if (old_state) {
/*
* The set of groups with a mux configuration in the old state
* may not be identical to the set of groups with a mux
setting
@@ -971,7 +972,7 @@ int pinctrl_select_state(struct pinctrl *p, struct
pinctrl_state *state)
* but not in the new state, this code puts that group into a
* safe/disabled state.
*/
- list_for_each_entry(setting, p-state-settings, node) {
+ list_for_each_entry(setting, old_state-settings, node) {
bool found = false;
if (setting-type != PIN_MAP_TYPE_MUX_GROUP)
continue;
@@ -989,7 +990,7 @@ int pinctrl_select_state(struct pinctrl *p, struct
pinctrl_state *state)
}
}
- p-state = NULL;
+ p-state[PINCTRL_STATIC] = NULL;
/* Apply all the settings for the new state */
list_for_each_entry(setting, state-settings, node) {
@@ -1011,7 +1012,7 @@ int pinctrl_select_state(struct pinctrl *p, struct
pinctrl_state *state)
}
}
- p-state = state;
+ p-state[PINCTRL_STATIC] = state;
return 0;
@@ -1492,7 +1493,8 @@ static int pinctrl_show(struct seq_file *s, void *what)
list_for_each_entry(p, pinctrl_list, node) {
seq_printf(s, device: %s current state: %s\n,
dev_name(p-dev),
- p-state ? p-state-name : none);
+ p-state[PINCTRL_STATIC] ?
+ p-state[PINCTRL_STATIC]-name : none);
list_for_each_entry(state, p-states, node) {
seq_printf(s, state: %s\n, state-name);
diff --git a/drivers/pinctrl/core.h b/drivers/pinctrl/core.h
index 75476b3..ac5a269 100644
--- a/drivers/pinctrl/core.h
+++ b/drivers/pinctrl/core.h
@@ -53,12 +53,18 @@ struct pinctrl_dev {
#endif
};
+enum pinctrl_states {
+ PINCTRL_STATIC,
+ PINCTRL_DYNAMIC,
+ PINCTRL_NR_STATES,
+};
This needs some very precise kerneldoc so it is chrystal clear what these
states are all about. pinctrl_states is not a good name for this enum,
please find some more precise name, because we have functions
with names like pinctrl_select_state() which is not related to this.
Is this substates or?
+
/**
* struct pinctrl - per-device pin control state holder
* @node: global list node
* @dev: the device using this pin control handle
* @states: a list of states for this
[PATCH 2/4] pinctrl: Allow pinctrl to have multiple active states
It's quite common that we need to dynamically change some pins for a
device for runtime PM, or toggle a pin between rx and tx. Changing all
the pins for a device is not efficient way of doing it.
So let's allow setting up multiple active states for pinctrl. Currently
we only need PINCTRL_STATIC and PINCTRL_DYNAMIC, where PINCTRL_STATIC
covers the current default pins, and PINCTRL_DYNAMIC holds the dynamic
pins that need to be toggled.
Cc: Felipe Balbi ba...@ti.com
Cc: Grygorii Strashko grygorii.stras...@ti.com
Cc: Stephen Warren swar...@wwwdotorg.org
Signed-off-by: Tony Lindgren t...@atomide.com
---
drivers/pinctrl/core.c | 18 ++
drivers/pinctrl/core.h | 10 --
2 files changed, 18 insertions(+), 10 deletions(-)
diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
index a97b717..c9b709b 100644
--- a/drivers/pinctrl/core.c
+++ b/drivers/pinctrl/core.c
@@ -885,7 +885,8 @@ static void pinctrl_free(struct pinctrl *p, bool inlist)
mutex_lock(pinctrl_list_mutex);
list_for_each_entry_safe(state, n1, p-states, node) {
list_for_each_entry_safe(setting, n2, state-settings, node) {
- pinctrl_free_setting(state == p-state, setting);
+ pinctrl_free_setting(state == p-state[PINCTRL_STATIC],
+setting);
list_del(setting-node);
kfree(setting);
}
@@ -955,13 +956,13 @@ EXPORT_SYMBOL_GPL(pinctrl_lookup_state);
int pinctrl_select_state(struct pinctrl *p, struct pinctrl_state *state)
{
struct pinctrl_setting *setting, *setting2;
- struct pinctrl_state *old_state = p-state;
+ struct pinctrl_state *old_state = p-state[PINCTRL_STATIC];
int ret;
- if (p-state == state)
+ if (old_state == state)
return 0;
- if (p-state) {
+ if (old_state) {
/*
* The set of groups with a mux configuration in the old state
* may not be identical to the set of groups with a mux setting
@@ -971,7 +972,7 @@ int pinctrl_select_state(struct pinctrl *p, struct
pinctrl_state *state)
* but not in the new state, this code puts that group into a
* safe/disabled state.
*/
- list_for_each_entry(setting, p-state-settings, node) {
+ list_for_each_entry(setting, old_state-settings, node) {
bool found = false;
if (setting-type != PIN_MAP_TYPE_MUX_GROUP)
continue;
@@ -989,7 +990,7 @@ int pinctrl_select_state(struct pinctrl *p, struct
pinctrl_state *state)
}
}
- p-state = NULL;
+ p-state[PINCTRL_STATIC] = NULL;
/* Apply all the settings for the new state */
list_for_each_entry(setting, state-settings, node) {
@@ -1011,7 +1012,7 @@ int pinctrl_select_state(struct pinctrl *p, struct
pinctrl_state *state)
}
}
- p-state = state;
+ p-state[PINCTRL_STATIC] = state;
return 0;
@@ -1492,7 +1493,8 @@ static int pinctrl_show(struct seq_file *s, void *what)
list_for_each_entry(p, pinctrl_list, node) {
seq_printf(s, device: %s current state: %s\n,
dev_name(p-dev),
- p-state ? p-state-name : none);
+ p-state[PINCTRL_STATIC] ?
+ p-state[PINCTRL_STATIC]-name : none);
list_for_each_entry(state, p-states, node) {
seq_printf(s, state: %s\n, state-name);
diff --git a/drivers/pinctrl/core.h b/drivers/pinctrl/core.h
index 75476b3..ac5a269 100644
--- a/drivers/pinctrl/core.h
+++ b/drivers/pinctrl/core.h
@@ -53,12 +53,18 @@ struct pinctrl_dev {
#endif
};
+enum pinctrl_states {
+ PINCTRL_STATIC,
+ PINCTRL_DYNAMIC,
+ PINCTRL_NR_STATES,
+};
+
/**
* struct pinctrl - per-device pin control state holder
* @node: global list node
* @dev: the device using this pin control handle
* @states: a list of states for this device
- * @state: the current state
+ * @state: the current state(s)
* @dt_maps: the mapping table chunks dynamically parsed from device tree for
* this device, if any
* @users: reference count
@@ -67,7 +73,7 @@ struct pinctrl {
struct list_head node;
struct device *dev;
struct list_head states;
- struct pinctrl_state *state;
+ struct pinctrl_state *state[PINCTRL_NR_STATES];
struct list_head dt_maps;
struct kref users;
};
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] pinctrl: Allow pinctrl to have multiple active states
On 07/16/2013 03:05 AM, Tony Lindgren wrote:
It's quite common that we need to dynamically change some pins for a
device for runtime PM, or toggle a pin between rx and tx. Changing all
the pins for a device is not efficient way of doing it.
So let's allow setting up multiple active states for pinctrl. Currently
we only need PINCTRL_STATIC and PINCTRL_DYNAMIC, where PINCTRL_STATIC
covers the current default pins, and PINCTRL_DYNAMIC holds the dynamic
pins that need to be toggled.
diff --git a/drivers/pinctrl/core.h b/drivers/pinctrl/core.h
+enum pinctr_states {
s/pinctr/pinctrl/
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/4] pinctrl: Allow pinctrl to have multiple active states
It's quite common that we need to dynamically change some pins for a
device for runtime PM, or toggle a pin between rx and tx. Changing all
the pins for a device is not efficient way of doing it.
So let's allow setting up multiple active states for pinctrl. Currently
we only need PINCTRL_STATIC and PINCTRL_DYNAMIC, where PINCTRL_STATIC
covers the current default pins, and PINCTRL_DYNAMIC holds the dynamic
pins that need to be toggled.
Cc: Stephen Warren swar...@wwwdotorg.org
Signed-off-by: Tony Lindgren t...@atomide.com
---
drivers/pinctrl/core.c | 18 ++
drivers/pinctrl/core.h | 10 --
2 files changed, 18 insertions(+), 10 deletions(-)
diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
index bda2c61..8da11d5 100644
--- a/drivers/pinctrl/core.c
+++ b/drivers/pinctrl/core.c
@@ -885,7 +885,8 @@ static void pinctrl_free(struct pinctrl *p, bool inlist)
mutex_lock(pinctrl_list_mutex);
list_for_each_entry_safe(state, n1, p-states, node) {
list_for_each_entry_safe(setting, n2, state-settings, node) {
- pinctrl_free_setting(state == p-state, setting);
+ pinctrl_free_setting(state == p-state[PINCTRL_STATIC],
+setting);
list_del(setting-node);
kfree(setting);
}
@@ -955,13 +956,13 @@ EXPORT_SYMBOL_GPL(pinctrl_lookup_state);
int pinctrl_select_state(struct pinctrl *p, struct pinctrl_state *state)
{
struct pinctrl_setting *setting, *setting2;
- struct pinctrl_state *old_state = p-state;
+ struct pinctrl_state *old_state = p-state[PINCTRL_STATIC];
int ret;
- if (p-state == state)
+ if (old_state == state)
return 0;
- if (p-state) {
+ if (old_state) {
/*
* The set of groups with a mux configuration in the old state
* may not be identical to the set of groups with a mux setting
@@ -971,7 +972,7 @@ int pinctrl_select_state(struct pinctrl *p, struct
pinctrl_state *state)
* but not in the new state, this code puts that group into a
* safe/disabled state.
*/
- list_for_each_entry(setting, p-state-settings, node) {
+ list_for_each_entry(setting, old_state-settings, node) {
bool found = false;
if (setting-type != PIN_MAP_TYPE_MUX_GROUP)
continue;
@@ -989,7 +990,7 @@ int pinctrl_select_state(struct pinctrl *p, struct
pinctrl_state *state)
}
}
- p-state = NULL;
+ p-state[PINCTRL_STATIC] = NULL;
/* Apply all the settings for the new state */
list_for_each_entry(setting, state-settings, node) {
@@ -1011,7 +1012,7 @@ int pinctrl_select_state(struct pinctrl *p, struct
pinctrl_state *state)
}
}
- p-state = state;
+ p-state[PINCTRL_STATIC] = state;
return 0;
@@ -1484,7 +1485,8 @@ static int pinctrl_show(struct seq_file *s, void *what)
list_for_each_entry(p, pinctrl_list, node) {
seq_printf(s, device: %s current state: %s\n,
dev_name(p-dev),
- p-state ? p-state-name : none);
+ p-state[PINCTRL_STATIC] ?
+ p-state[PINCTRL_STATIC]-name : none);
list_for_each_entry(state, p-states, node) {
seq_printf(s, state: %s\n, state-name);
diff --git a/drivers/pinctrl/core.h b/drivers/pinctrl/core.h
index 75476b3..b99e4b3 100644
--- a/drivers/pinctrl/core.h
+++ b/drivers/pinctrl/core.h
@@ -53,12 +53,18 @@ struct pinctrl_dev {
#endif
};
+enum pinctr_states {
+ PINCTRL_STATIC,
+ PINCTRL_DYNAMIC,
+ PINCTRL_NR_STATES,
+};
+
/**
* struct pinctrl - per-device pin control state holder
* @node: global list node
* @dev: the device using this pin control handle
* @states: a list of states for this device
- * @state: the current state
+ * @state: the current state(s)
* @dt_maps: the mapping table chunks dynamically parsed from device tree for
* this device, if any
* @users: reference count
@@ -67,7 +73,7 @@ struct pinctrl {
struct list_head node;
struct device *dev;
struct list_head states;
- struct pinctrl_state *state;
+ struct pinctrl_state *state[PINCTRL_NR_STATES];
struct list_head dt_maps;
struct kref users;
};
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
