RE: [PATCH 11/11] i2c: omap: enhance pinctrl support
On Fri, May 31, 2013 at 20:25:38, Strashko, Grygorii wrote:
On 05/31/2013 01:13 PM, Hebbar Gururaja wrote:
Amend the I2C omap pin controller to optionally take a pin control
handle and set the state of the pins to:
- default on boot, resume and before performing an i2c transfer
- idle after initial default, after resume default, and after each
i2c xfer
- sleep on suspend()
By optionally putting the pins into sleep state in the suspend callback
we can accomplish two things.
- One is to minimize current leakage from pins and thus save power,
- second, we can prevent the IP from driving pins output in an
uncontrolled manner, which may happen if the power domain drops the
domain regulator.
Note:
A .suspend .resume callback is added which simply puts the pins to sleep
state upon suspend are moved to default idle state upon resume.
If any of the above pin states are missing in dt, a warning message
about the missing state is displayed.
If certain pin-states are not available, to remove this warning message
pass respective state name with null phandler.
(Changes based on i2c-nomadik.c)
Signed-off-by: Hebbar Gururaja gururaja.heb...@ti.com
Cc: Tony Lindgren t...@atomide.com
Cc: Wolfram Sang w...@the-dreams.de
Cc: linux-omap@vger.kernel.org
Cc: linux-...@vger.kernel.org
---
:100644 100644 e02f9e3... 588ba28... M drivers/i2c/busses/i2c-omap.c
drivers/i2c/busses/i2c-omap.c | 112
++---
1 file changed, 105 insertions(+), 7 deletions(-)
diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index e02f9e3..588ba28 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -214,7 +214,11 @@ struct omap_i2c_dev {
u16 westate;
u16 errata;
- struct pinctrl *pins;
+ /* Three pin states - default, idle sleep */
+ struct pinctrl *pinctrl;
+ struct pinctrl_state*pins_default;
+ struct pinctrl_state*pins_idle;
+ struct pinctrl_state*pins_sleep;
};
static const u8 reg_map_ip_v1[] = {
@@ -641,6 +645,11 @@ omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg
msgs[], int num)
if (IS_ERR_VALUE(r))
goto out;
The current HWMOD framework configures PINs to enable state before
enabling the device and
switch PINs to idle state after disabling the device. Why here its done
in different order?
AFAIK, in case of DT boot, oh-mux will be NULL. So hwmod will not change
Any pins.
+ /* Optionaly enable pins to be muxed in and configured */
+ if (!IS_ERR(dev-pins_default))
+ if (pinctrl_select_state(dev-pinctrl, dev-pins_default))
+ dev_err(dev-dev, could not set default pins\n);
+
r = omap_i2c_wait_for_bb(dev);
if (r 0)
goto out;
@@ -664,7 +673,13 @@ omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg
msgs[], int num)
out:
pm_runtime_mark_last_busy(dev-dev);
+
pm_runtime_put_autosuspend(dev-dev);
+ /* Optionally let pins go into idle state */
+ if (!IS_ERR(dev-pins_idle))
+ if (pinctrl_select_state(dev-pinctrl, dev-pins_idle))
+ dev_err(dev-dev, could not set pins to idle state\n);
+
return r;
}
@@ -1123,14 +1138,47 @@ omap_i2c_probe(struct platform_device *pdev)
dev-set_mpu_wkup_lat = pdata-set_mpu_wkup_lat;
}
- dev-pins = devm_pinctrl_get_select_default(pdev-dev);
- if (IS_ERR(dev-pins)) {
- if (PTR_ERR(dev-pins) == -EPROBE_DEFER)
+ dev-pinctrl = devm_pinctrl_get(pdev-dev);
May be struct device -pins-p can be used instead of dev-pinctrl?
+ if (!IS_ERR(dev-pinctrl)) {
+ dev-pins_default = pinctrl_lookup_state(dev-pinctrl,
+PINCTRL_STATE_DEFAULT);
+ if (IS_ERR(dev-pins_default))
+ dev_dbg(pdev-dev, could not get default pinstate\n);
+ else
+ if (pinctrl_select_state(dev-pinctrl,
+dev-pins_default))
+ dev_err(pdev-dev,
+ could not set default pinstate\n);
Don't need to set Default pin state
Default pins state is applied by DD framework automatically before
device probing
and stored in struct device -pins-default_state
Correct.
+
+ dev-pins_idle = pinctrl_lookup_state(dev-pinctrl,
+ PINCTRL_STATE_IDLE);
+ if (IS_ERR(dev-pins_idle))
+ dev_dbg(pdev-dev, could not get idle pinstate\n);
+ else
+ /* If possible, let's idle until the first transfer */
+ if
RE: [PATCH 11/11] i2c: omap: enhance pinctrl support
On Fri, May 31, 2013 at 23:04:59, Kevin Hilman wrote: Hebbar Gururaja gururaja.heb...@ti.com writes: Amend the I2C omap pin controller to optionally take a pin control handle and set the state of the pins to: - default on boot, resume and before performing an i2c transfer - idle after initial default, after resume default, and after each i2c xfer - sleep on suspend() By optionally putting the pins into sleep state in the suspend callback we can accomplish two things. - One is to minimize current leakage from pins and thus save power, - second, we can prevent the IP from driving pins output in an uncontrolled manner, which may happen if the power domain drops the domain regulator. Note: A .suspend .resume callback is added which simply puts the pins to sleep state upon suspend are moved to default idle state upon resume. If any of the above pin states are missing in dt, a warning message about the missing state is displayed. If certain pin-states are not available, to remove this warning message pass respective state name with null phandler. (Changes based on i2c-nomadik.c) Signed-off-by: Hebbar Gururaja gururaja.heb...@ti.com Cc: Tony Lindgren t...@atomide.com Cc: Wolfram Sang w...@the-dreams.de Cc: linux-omap@vger.kernel.org Cc: linux-...@vger.kernel.org [...] @@ -664,7 +673,13 @@ omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num) out: pm_runtime_mark_last_busy(dev-dev); + pm_runtime_put_autosuspend(dev-dev); + /* Optionally let pins go into idle state */ + if (!IS_ERR(dev-pins_idle)) + if (pinctrl_select_state(dev-pinctrl, dev-pins_idle)) + dev_err(dev-dev, could not set pins to idle state\n); This is wrong. You're changing the muxing before the device actually goes idle. Anything you want to happen when the device actually idles for real has to be in runtime PM callbacks. Looking below, I see it's already in the callbacks, so why is it here also? Just to be double sure. Seems it is unwanted. [...] @@ -1300,6 +1348,10 @@ static int omap_i2c_runtime_suspend(struct device *dev) omap_i2c_read_reg(_dev, OMAP_I2C_STAT_REG); } + if (!IS_ERR(_dev-pins_idle)) + if (pinctrl_select_state(_dev-pinctrl, _dev-pins_idle)) + dev_err(dev, could not set pins to idle state\n); + return 0; } Kevin Regards, Gururaja -- 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 11/11] i2c: omap: enhance pinctrl support
On Fri, May 31, 2013 at 8:07 PM, Kevin Hilman khil...@linaro.org wrote: But that brings up a bigger question about whether or not we should be doing the rest of this (idle/sleep) pin management in the drivers or in the driver core as well. I would much prefer it be handled by the driver core. Yes. See my suggestion in 2/11. In fact, since these are all PM related events, it should probably be handled by the PM core and seems pretty straight forward to do so. I can see a clean interface with three simple functions toward pinctrl/consumer.h for switching between the default, idle and sleep states, but you may see even further... Yours, Linus Walleij -- 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 11/11] i2c: omap: enhance pinctrl support
On Fri, May 31, 2013 at 23:37:02, Kevin Hilman wrote:
+Linus Walleij (pinctrl maintainer)
Hebbar Gururaja gururaja.heb...@ti.com writes:
Amend the I2C omap pin controller to optionally take a pin control
handle and set the state of the pins to:
- default on boot, resume and before performing an i2c transfer
- idle after initial default, after resume default, and after each
i2c xfer
- sleep on suspend()
By optionally putting the pins into sleep state in the suspend callback
we can accomplish two things.
- One is to minimize current leakage from pins and thus save power,
- second, we can prevent the IP from driving pins output in an
uncontrolled manner, which may happen if the power domain drops the
domain regulator.
Note:
A .suspend .resume callback is added which simply puts the pins to sleep
state upon suspend are moved to default idle state upon resume.
If any of the above pin states are missing in dt, a warning message
about the missing state is displayed.
If certain pin-states are not available, to remove this warning message
pass respective state name with null phandler.
(Changes based on i2c-nomadik.c)
Signed-off-by: Hebbar Gururaja gururaja.heb...@ti.com
Cc: Tony Lindgren t...@atomide.com
Cc: Wolfram Sang w...@the-dreams.de
Cc: linux-omap@vger.kernel.org
Cc: linux-...@vger.kernel.org
[...]
@@ -1123,14 +1138,47 @@ omap_i2c_probe(struct platform_device *pdev)
dev-set_mpu_wkup_lat = pdata-set_mpu_wkup_lat;
}
- dev-pins = devm_pinctrl_get_select_default(pdev-dev);
- if (IS_ERR(dev-pins)) {
- if (PTR_ERR(dev-pins) == -EPROBE_DEFER)
+ dev-pinctrl = devm_pinctrl_get(pdev-dev);
+ if (!IS_ERR(dev-pinctrl)) {
+ dev-pins_default = pinctrl_lookup_state(dev-pinctrl,
+PINCTRL_STATE_DEFAULT);
This part is already done by probe in driver core, why does it need to
be done again. dev-pins-default_state should already have this.
(c.f. pinctrl_bind_pins() in drivers/base/pinctrl.c)
But that brings up a bigger question about whether or not we should be
doing the rest of this (idle/sleep) pin management in the drivers or in
the driver core as well. I would much prefer it be handled by the
driver core.
In fact, since these are all PM related events, it should probably be
handled by the PM core and seems pretty straight forward to do so.
Let me pull out some info about these and come back
Kevin
Regards,
Gururaja
--
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 11/11] i2c: omap: enhance pinctrl support
Hi Kevin, Gururaja On 05/31/2013 08:34 PM, Kevin Hilman wrote: Hebbar Gururaja gururaja.heb...@ti.com writes: Amend the I2C omap pin controller to optionally take a pin control handle and set the state of the pins to: - default on boot, resume and before performing an i2c transfer - idle after initial default, after resume default, and after each i2c xfer - sleep on suspend() By optionally putting the pins into sleep state in the suspend callback we can accomplish two things. - One is to minimize current leakage from pins and thus save power, - second, we can prevent the IP from driving pins output in an uncontrolled manner, which may happen if the power domain drops the domain regulator. Note: A .suspend .resume callback is added which simply puts the pins to sleep state upon suspend are moved to default idle state upon resume. If any of the above pin states are missing in dt, a warning message about the missing state is displayed. If certain pin-states are not available, to remove this warning message pass respective state name with null phandler. (Changes based on i2c-nomadik.c) Signed-off-by: Hebbar Gururaja gururaja.heb...@ti.com Cc: Tony Lindgren t...@atomide.com Cc: Wolfram Sang w...@the-dreams.de Cc: linux-omap@vger.kernel.org Cc: linux-...@vger.kernel.org [...] @@ -664,7 +673,13 @@ omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num) out: pm_runtime_mark_last_busy(dev-dev); + pm_runtime_put_autosuspend(dev-dev); + /* Optionally let pins go into idle state */ + if (!IS_ERR(dev-pins_idle)) + if (pinctrl_select_state(dev-pinctrl, dev-pins_idle)) + dev_err(dev-dev, could not set pins to idle state\n); This is wrong. You're changing the muxing before the device actually goes idle. Anything you want to happen when the device actually idles for real has to be in runtime PM callbacks. Looking below, I see it's already in the callbacks, so why is it here also? I have two questions regarding this patch the whole series: 1) PM runtime suspend/resume Current sequence: - resume |- omap_hwmod_enable() |- _enable() |- omap_hwmod_mux(oh-mux, _HWMOD_STATE_ENABLED) |- enable module (syscclocks) |- omap_i2c_runtime_resume() - suspend |- omap_i2c_runtime_suspend() |- omap_hwmod_idle() |- _idle() |- disbale module (syscclocks) |- omap_hwmod_mux(oh-mux, _HWMOD_STATE_IDLE); The new order will change this sequence - *Is it safe?*: - resume |- omap_hwmod_enable() |- _enable() |- enable module (syscclocks) Is it valid to enable module before PINs? |- omap_i2c_runtime_resume() |- PINs state set to DEFAULT - suspend |- omap_i2c_runtime_suspend() |- PINs state set to IDLE |- omap_hwmod_idle() |- _idle() |- disbale module (syscclocks) Is it valid to disable module after PINs? 2) System suspend (OFF-mode) with assumption that noirq stage will be used for PINs cfg Current implementation: handled in the same way as PM runtime The new implementation: -- total mess is here((: - suspend_noirq - I2C device can be still active because of PM auto-suspend |-_od_suspend_noirq |- omap_i2c_suspend_noirq |- PINs state set to SLEEP |- pm_generic_runtime_suspend |- omap_i2c_runtime_suspend() |- PINs state set to IDLE --- *oops* PINs state is IDLE and not SLEEP |- omap_device_idle() |- omap_hwmod_idle() |- _idle() |- disbale module (syscclocks) - resume_noirq - I2C was active before suspend |-_od_resume_noirq |- omap_hwmod_enable() |- _enable() |- enable module (syscclocks) |- pm_generic_runtime_resume |- omap_i2c_runtime_resume() |- PINs state set to DEFAULT --- |- omap_i2c_resume_noirq |- PINs state set to DEFAULT |- PINs state set to IDLE--- *big oops* we have active module and its PINs state is IDLE All mentioned above, applicable for most of all patches in series. And It seems, that this functionality can't be implemented in such way ( - OMAP device FW and Driver core might handle PINs states changing and drivers (DT) should provide set of available states only. Am I wrong? Regards, -grygorii [...] @@ -1300,6 +1348,10 @@ static int omap_i2c_runtime_suspend(struct device *dev) omap_i2c_read_reg(_dev, OMAP_I2C_STAT_REG); } + if (!IS_ERR(_dev-pins_idle)) + if (pinctrl_select_state(_dev-pinctrl, _dev-pins_idle)) + dev_err(dev, could not set pins to idle state\n); + return 0; } Kevin -- 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 -- To unsubscribe from this list: send the line
[PATCH 11/11] i2c: omap: enhance pinctrl support
Amend the I2C omap pin controller to optionally take a pin control
handle and set the state of the pins to:
- default on boot, resume and before performing an i2c transfer
- idle after initial default, after resume default, and after each
i2c xfer
- sleep on suspend()
By optionally putting the pins into sleep state in the suspend callback
we can accomplish two things.
- One is to minimize current leakage from pins and thus save power,
- second, we can prevent the IP from driving pins output in an
uncontrolled manner, which may happen if the power domain drops the
domain regulator.
Note:
A .suspend .resume callback is added which simply puts the pins to sleep
state upon suspend are moved to default idle state upon resume.
If any of the above pin states are missing in dt, a warning message
about the missing state is displayed.
If certain pin-states are not available, to remove this warning message
pass respective state name with null phandler.
(Changes based on i2c-nomadik.c)
Signed-off-by: Hebbar Gururaja gururaja.heb...@ti.com
Cc: Tony Lindgren t...@atomide.com
Cc: Wolfram Sang w...@the-dreams.de
Cc: linux-omap@vger.kernel.org
Cc: linux-...@vger.kernel.org
---
:100644 100644 e02f9e3... 588ba28... M drivers/i2c/busses/i2c-omap.c
drivers/i2c/busses/i2c-omap.c | 112 ++---
1 file changed, 105 insertions(+), 7 deletions(-)
diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index e02f9e3..588ba28 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -214,7 +214,11 @@ struct omap_i2c_dev {
u16 westate;
u16 errata;
- struct pinctrl *pins;
+ /* Three pin states - default, idle sleep */
+ struct pinctrl *pinctrl;
+ struct pinctrl_state*pins_default;
+ struct pinctrl_state*pins_idle;
+ struct pinctrl_state*pins_sleep;
};
static const u8 reg_map_ip_v1[] = {
@@ -641,6 +645,11 @@ omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg
msgs[], int num)
if (IS_ERR_VALUE(r))
goto out;
+ /* Optionaly enable pins to be muxed in and configured */
+ if (!IS_ERR(dev-pins_default))
+ if (pinctrl_select_state(dev-pinctrl, dev-pins_default))
+ dev_err(dev-dev, could not set default pins\n);
+
r = omap_i2c_wait_for_bb(dev);
if (r 0)
goto out;
@@ -664,7 +673,13 @@ omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg
msgs[], int num)
out:
pm_runtime_mark_last_busy(dev-dev);
+
pm_runtime_put_autosuspend(dev-dev);
+ /* Optionally let pins go into idle state */
+ if (!IS_ERR(dev-pins_idle))
+ if (pinctrl_select_state(dev-pinctrl, dev-pins_idle))
+ dev_err(dev-dev, could not set pins to idle state\n);
+
return r;
}
@@ -1123,14 +1138,47 @@ omap_i2c_probe(struct platform_device *pdev)
dev-set_mpu_wkup_lat = pdata-set_mpu_wkup_lat;
}
- dev-pins = devm_pinctrl_get_select_default(pdev-dev);
- if (IS_ERR(dev-pins)) {
- if (PTR_ERR(dev-pins) == -EPROBE_DEFER)
+ dev-pinctrl = devm_pinctrl_get(pdev-dev);
+ if (!IS_ERR(dev-pinctrl)) {
+ dev-pins_default = pinctrl_lookup_state(dev-pinctrl,
+PINCTRL_STATE_DEFAULT);
+ if (IS_ERR(dev-pins_default))
+ dev_dbg(pdev-dev, could not get default pinstate\n);
+ else
+ if (pinctrl_select_state(dev-pinctrl,
+dev-pins_default))
+ dev_err(pdev-dev,
+ could not set default pinstate\n);
+
+ dev-pins_idle = pinctrl_lookup_state(dev-pinctrl,
+ PINCTRL_STATE_IDLE);
+ if (IS_ERR(dev-pins_idle))
+ dev_dbg(pdev-dev, could not get idle pinstate\n);
+ else
+ /* If possible, let's idle until the first transfer */
+ if (pinctrl_select_state(dev-pinctrl, dev-pins_idle))
+ dev_err(pdev-dev,
+ could not set idle pinstate\n);
+
+ dev-pins_sleep = pinctrl_lookup_state(dev-pinctrl,
+ PINCTRL_STATE_SLEEP);
+ if (IS_ERR(dev-pins_sleep))
+ dev_dbg(pdev-dev, could not get sleep pinstate\n);
+ } else {
+ if (PTR_ERR(dev-pinctrl) == -EPROBE_DEFER)
return -EPROBE_DEFER;
- dev_warn(pdev-dev, did not get pins for i2c error: %li\n,
-PTR_ERR(dev-pins));
-
Re: [PATCH 11/11] i2c: omap: enhance pinctrl support
On 05/31/2013 01:13 PM, Hebbar Gururaja wrote:
Amend the I2C omap pin controller to optionally take a pin control
handle and set the state of the pins to:
- default on boot, resume and before performing an i2c transfer
- idle after initial default, after resume default, and after each
i2c xfer
- sleep on suspend()
By optionally putting the pins into sleep state in the suspend callback
we can accomplish two things.
- One is to minimize current leakage from pins and thus save power,
- second, we can prevent the IP from driving pins output in an
uncontrolled manner, which may happen if the power domain drops the
domain regulator.
Note:
A .suspend .resume callback is added which simply puts the pins to sleep
state upon suspend are moved to default idle state upon resume.
If any of the above pin states are missing in dt, a warning message
about the missing state is displayed.
If certain pin-states are not available, to remove this warning message
pass respective state name with null phandler.
(Changes based on i2c-nomadik.c)
Signed-off-by: Hebbar Gururaja gururaja.heb...@ti.com
Cc: Tony Lindgren t...@atomide.com
Cc: Wolfram Sang w...@the-dreams.de
Cc: linux-omap@vger.kernel.org
Cc: linux-...@vger.kernel.org
---
:100644 100644 e02f9e3... 588ba28... M drivers/i2c/busses/i2c-omap.c
drivers/i2c/busses/i2c-omap.c | 112 ++---
1 file changed, 105 insertions(+), 7 deletions(-)
diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index e02f9e3..588ba28 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -214,7 +214,11 @@ struct omap_i2c_dev {
u16 westate;
u16 errata;
- struct pinctrl *pins;
+ /* Three pin states - default, idle sleep */
+ struct pinctrl *pinctrl;
+ struct pinctrl_state*pins_default;
+ struct pinctrl_state*pins_idle;
+ struct pinctrl_state*pins_sleep;
};
static const u8 reg_map_ip_v1[] = {
@@ -641,6 +645,11 @@ omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg
msgs[], int num)
if (IS_ERR_VALUE(r))
goto out;
The current HWMOD framework configures PINs to enable state before
enabling the device and
switch PINs to idle state after disabling the device. Why here its done
in different order?
+ /* Optionaly enable pins to be muxed in and configured */
+ if (!IS_ERR(dev-pins_default))
+ if (pinctrl_select_state(dev-pinctrl, dev-pins_default))
+ dev_err(dev-dev, could not set default pins\n);
+
r = omap_i2c_wait_for_bb(dev);
if (r 0)
goto out;
@@ -664,7 +673,13 @@ omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg
msgs[], int num)
out:
pm_runtime_mark_last_busy(dev-dev);
+
pm_runtime_put_autosuspend(dev-dev);
+ /* Optionally let pins go into idle state */
+ if (!IS_ERR(dev-pins_idle))
+ if (pinctrl_select_state(dev-pinctrl, dev-pins_idle))
+ dev_err(dev-dev, could not set pins to idle state\n);
+
return r;
}
@@ -1123,14 +1138,47 @@ omap_i2c_probe(struct platform_device *pdev)
dev-set_mpu_wkup_lat = pdata-set_mpu_wkup_lat;
}
- dev-pins = devm_pinctrl_get_select_default(pdev-dev);
- if (IS_ERR(dev-pins)) {
- if (PTR_ERR(dev-pins) == -EPROBE_DEFER)
+ dev-pinctrl = devm_pinctrl_get(pdev-dev);
May be struct device -pins-p can be used instead of dev-pinctrl?
+ if (!IS_ERR(dev-pinctrl)) {
+ dev-pins_default = pinctrl_lookup_state(dev-pinctrl,
+PINCTRL_STATE_DEFAULT);
+ if (IS_ERR(dev-pins_default))
+ dev_dbg(pdev-dev, could not get default pinstate\n);
+ else
+ if (pinctrl_select_state(dev-pinctrl,
+dev-pins_default))
+ dev_err(pdev-dev,
+ could not set default pinstate\n);
Don't need to set Default pin state
Default pins state is applied by DD framework automatically before
device probing
and stored in struct device -pins-default_state
+
+ dev-pins_idle = pinctrl_lookup_state(dev-pinctrl,
+ PINCTRL_STATE_IDLE);
+ if (IS_ERR(dev-pins_idle))
+ dev_dbg(pdev-dev, could not get idle pinstate\n);
+ else
+ /* If possible, let's idle until the first transfer */
+ if (pinctrl_select_state(dev-pinctrl, dev-pins_idle))
+ dev_err(pdev-dev,
+ could not set idle pinstate\n);
+
+ dev-pins_sleep =
Re: [PATCH 11/11] i2c: omap: enhance pinctrl support
Hebbar Gururaja gururaja.heb...@ti.com writes: Amend the I2C omap pin controller to optionally take a pin control handle and set the state of the pins to: - default on boot, resume and before performing an i2c transfer - idle after initial default, after resume default, and after each i2c xfer - sleep on suspend() By optionally putting the pins into sleep state in the suspend callback we can accomplish two things. - One is to minimize current leakage from pins and thus save power, - second, we can prevent the IP from driving pins output in an uncontrolled manner, which may happen if the power domain drops the domain regulator. Note: A .suspend .resume callback is added which simply puts the pins to sleep state upon suspend are moved to default idle state upon resume. If any of the above pin states are missing in dt, a warning message about the missing state is displayed. If certain pin-states are not available, to remove this warning message pass respective state name with null phandler. (Changes based on i2c-nomadik.c) Signed-off-by: Hebbar Gururaja gururaja.heb...@ti.com Cc: Tony Lindgren t...@atomide.com Cc: Wolfram Sang w...@the-dreams.de Cc: linux-omap@vger.kernel.org Cc: linux-...@vger.kernel.org [...] @@ -664,7 +673,13 @@ omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num) out: pm_runtime_mark_last_busy(dev-dev); + pm_runtime_put_autosuspend(dev-dev); + /* Optionally let pins go into idle state */ + if (!IS_ERR(dev-pins_idle)) + if (pinctrl_select_state(dev-pinctrl, dev-pins_idle)) + dev_err(dev-dev, could not set pins to idle state\n); This is wrong. You're changing the muxing before the device actually goes idle. Anything you want to happen when the device actually idles for real has to be in runtime PM callbacks. Looking below, I see it's already in the callbacks, so why is it here also? [...] @@ -1300,6 +1348,10 @@ static int omap_i2c_runtime_suspend(struct device *dev) omap_i2c_read_reg(_dev, OMAP_I2C_STAT_REG); } + if (!IS_ERR(_dev-pins_idle)) + if (pinctrl_select_state(_dev-pinctrl, _dev-pins_idle)) + dev_err(dev, could not set pins to idle state\n); + return 0; } Kevin -- 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
