| Message ID | 1369592616-23972-2-git-send-email-mugunthanvnm@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
* Mugunthan V N <mugunthanvnm@ti.com> [130526 11:28]: > From: Hebbar Gururaja <gururaja.hebbar@ti.com> > > Amend cpsw controller to optionally take a pin control handle and set > the state of the pins to: > > - "default" on boot, resume > - "sleep" on suspend() > > This should make it possible to optimize energy usage for the pins > for the suspend/resume cycle. > > 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. > > Signed-off-by: Hebbar Gururaja <gururaja.hebbar@ti.com> > Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com> > --- > drivers/net/ethernet/ti/cpsw.c | 48 ++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 48 insertions(+) > > diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c > index 21a5b29..c9ed730 100644 > --- a/drivers/net/ethernet/ti/cpsw.c > +++ b/drivers/net/ethernet/ti/cpsw.c > @@ -35,6 +35,7 @@ > #include <linux/if_vlan.h> > > #include <linux/platform_data/cpsw.h> > +#include <linux/pinctrl/consumer.h> > > #include "cpsw_ale.h" > #include "cpts.h" > @@ -351,6 +352,11 @@ struct cpsw_priv { > bool irq_enabled; > struct cpts *cpts; > u32 emac_port; > + > + /* Two optional pin states - default & sleep */ > + struct pinctrl *pinctrl; > + struct pinctrl_state *pins_def; > + struct pinctrl_state *pins_sleep; > }; Which pins do you need to dynamically remux? If it's not all the pins, you should have three sets: default, active and idle. This way the static pins in the default group don't need to be constantly toggled. Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 5/28/2013 3:06 AM, Tony Lindgren wrote: > * Mugunthan V N <mugunthanvnm@ti.com> [130526 11:28]: >> From: Hebbar Gururaja <gururaja.hebbar@ti.com> >> >> Amend cpsw controller to optionally take a pin control handle and set >> the state of the pins to: >> >> - "default" on boot, resume >> - "sleep" on suspend() >> >> This should make it possible to optimize energy usage for the pins >> for the suspend/resume cycle. >> >> 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. >> >> Signed-off-by: Hebbar Gururaja <gururaja.hebbar@ti.com> >> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com> >> --- >> drivers/net/ethernet/ti/cpsw.c | 48 ++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 48 insertions(+) >> >> diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c >> index 21a5b29..c9ed730 100644 >> --- a/drivers/net/ethernet/ti/cpsw.c >> +++ b/drivers/net/ethernet/ti/cpsw.c >> @@ -35,6 +35,7 @@ >> #include <linux/if_vlan.h> >> >> #include <linux/platform_data/cpsw.h> >> +#include <linux/pinctrl/consumer.h> >> >> #include "cpsw_ale.h" >> #include "cpts.h" >> @@ -351,6 +352,11 @@ struct cpsw_priv { >> bool irq_enabled; >> struct cpts *cpts; >> u32 emac_port; >> + >> + /* Two optional pin states - default & sleep */ >> + struct pinctrl *pinctrl; >> + struct pinctrl_state *pins_def; >> + struct pinctrl_state *pins_sleep; >> }; > Which pins do you need to dynamically remux? If it's not all > the pins, you should have three sets: default, active and idle. > This way the static pins in the default group don't need to be > constantly toggled. > > Regards, > > Tony Tony I am using this for all the pins, in probe all the cpsw pins will be configured and i have used the same in suspend/resume callback for power saving. Regards Mugunthan V N -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 5/28/2013 7:35 PM, Mugunthan V N wrote: > On 5/28/2013 3:06 AM, Tony Lindgren wrote: >> * Mugunthan V N <mugunthanvnm@ti.com> [130526 11:28]: >>> From: Hebbar Gururaja <gururaja.hebbar@ti.com> >>> >>> Amend cpsw controller to optionally take a pin control handle and set >>> the state of the pins to: >>> >>> - "default" on boot, resume >>> - "sleep" on suspend() >>> >>> This should make it possible to optimize energy usage for the pins >>> for the suspend/resume cycle. >>> >>> 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. >>> >>> Signed-off-by: Hebbar Gururaja <gururaja.hebbar@ti.com> >>> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com> >>> --- >>> drivers/net/ethernet/ti/cpsw.c | 48 >>> ++++++++++++++++++++++++++++++++++++++++ >>> 1 file changed, 48 insertions(+) >>> >>> diff --git a/drivers/net/ethernet/ti/cpsw.c >>> b/drivers/net/ethernet/ti/cpsw.c >>> index 21a5b29..c9ed730 100644 >>> --- a/drivers/net/ethernet/ti/cpsw.c >>> +++ b/drivers/net/ethernet/ti/cpsw.c >>> @@ -35,6 +35,7 @@ >>> #include <linux/if_vlan.h> >>> #include <linux/platform_data/cpsw.h> >>> +#include <linux/pinctrl/consumer.h> >>> #include "cpsw_ale.h" >>> #include "cpts.h" >>> @@ -351,6 +352,11 @@ struct cpsw_priv { >>> bool irq_enabled; >>> struct cpts *cpts; >>> u32 emac_port; >>> + >>> + /* Two optional pin states - default & sleep */ >>> + struct pinctrl *pinctrl; >>> + struct pinctrl_state *pins_def; >>> + struct pinctrl_state *pins_sleep; >>> }; >> Which pins do you need to dynamically remux? If it's not all >> the pins, you should have three sets: default, active and idle. >> This way the static pins in the default group don't need to be >> constantly toggled. >> >> Regards, >> >> Tony > Tony > > I am using this for all the pins, in probe all the cpsw pins will be > configured > and i have used the same in suspend/resume callback for power saving. > Tony Do you have any comments on this, or is it ok with two pinctrl_state nodes? Regards Mugunthan V N -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
* Mugunthan V N <mugunthanvnm@ti.com> [130530 01:14]: > On 5/28/2013 7:35 PM, Mugunthan V N wrote: > >On 5/28/2013 3:06 AM, Tony Lindgren wrote: > >>* Mugunthan V N <mugunthanvnm@ti.com> [130526 11:28]: > >>>From: Hebbar Gururaja <gururaja.hebbar@ti.com> > >>> > >>>Amend cpsw controller to optionally take a pin control handle and set > >>>the state of the pins to: > >>> > >>>- "default" on boot, resume > >>>- "sleep" on suspend() > >>> > >>>This should make it possible to optimize energy usage for the pins > >>>for the suspend/resume cycle. > >>> > >>>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. > >>> > >>>Signed-off-by: Hebbar Gururaja <gururaja.hebbar@ti.com> > >>>Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com> > >>>--- > >>> drivers/net/ethernet/ti/cpsw.c | 48 > >>>++++++++++++++++++++++++++++++++++++++++ > >>> 1 file changed, 48 insertions(+) > >>> > >>>diff --git a/drivers/net/ethernet/ti/cpsw.c > >>>b/drivers/net/ethernet/ti/cpsw.c > >>>index 21a5b29..c9ed730 100644 > >>>--- a/drivers/net/ethernet/ti/cpsw.c > >>>+++ b/drivers/net/ethernet/ti/cpsw.c > >>>@@ -35,6 +35,7 @@ > >>> #include <linux/if_vlan.h> > >>> #include <linux/platform_data/cpsw.h> > >>>+#include <linux/pinctrl/consumer.h> > >>> #include "cpsw_ale.h" > >>> #include "cpts.h" > >>>@@ -351,6 +352,11 @@ struct cpsw_priv { > >>> bool irq_enabled; > >>> struct cpts *cpts; > >>> u32 emac_port; > >>>+ > >>>+ /* Two optional pin states - default & sleep */ > >>>+ struct pinctrl *pinctrl; > >>>+ struct pinctrl_state *pins_def; > >>>+ struct pinctrl_state *pins_sleep; > >>> }; > >>Which pins do you need to dynamically remux? If it's not all > >>the pins, you should have three sets: default, active and idle. > >>This way the static pins in the default group don't need to be > >>constantly toggled. > >> > >>Regards, > >> > >>Tony > >Tony > > > >I am using this for all the pins, in probe all the cpsw pins will > >be configured > >and i have used the same in suspend/resume callback for power saving. > > > Tony > > Do you have any comments on this, or is it ok with two pinctrl_state nodes? If you always need to remux all the pins, then yes that's fine with me. Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 6/5/2013 9:19 PM, Tony Lindgren wrote: > * Mugunthan V N <mugunthanvnm@ti.com> [130530 01:14]: >> On 5/28/2013 7:35 PM, Mugunthan V N wrote: >>> On 5/28/2013 3:06 AM, Tony Lindgren wrote: >>>> * Mugunthan V N <mugunthanvnm@ti.com> [130526 11:28]: >>>>> From: Hebbar Gururaja <gururaja.hebbar@ti.com> >>>>> >>>>> Amend cpsw controller to optionally take a pin control handle and set >>>>> the state of the pins to: >>>>> >>>>> - "default" on boot, resume >>>>> - "sleep" on suspend() >>>>> >>>>> This should make it possible to optimize energy usage for the pins >>>>> for the suspend/resume cycle. >>>>> >>>>> 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. >>>>> >>>>> Signed-off-by: Hebbar Gururaja <gururaja.hebbar@ti.com> >>>>> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com> >>>>> --- >>>>> drivers/net/ethernet/ti/cpsw.c | 48 >>>>> ++++++++++++++++++++++++++++++++++++++++ >>>>> 1 file changed, 48 insertions(+) >>>>> >>>>> diff --git a/drivers/net/ethernet/ti/cpsw.c >>>>> b/drivers/net/ethernet/ti/cpsw.c >>>>> index 21a5b29..c9ed730 100644 >>>>> --- a/drivers/net/ethernet/ti/cpsw.c >>>>> +++ b/drivers/net/ethernet/ti/cpsw.c >>>>> @@ -35,6 +35,7 @@ >>>>> #include <linux/if_vlan.h> >>>>> #include <linux/platform_data/cpsw.h> >>>>> +#include <linux/pinctrl/consumer.h> >>>>> #include "cpsw_ale.h" >>>>> #include "cpts.h" >>>>> @@ -351,6 +352,11 @@ struct cpsw_priv { >>>>> bool irq_enabled; >>>>> struct cpts *cpts; >>>>> u32 emac_port; >>>>> + >>>>> + /* Two optional pin states - default & sleep */ >>>>> + struct pinctrl *pinctrl; >>>>> + struct pinctrl_state *pins_def; >>>>> + struct pinctrl_state *pins_sleep; >>>>> }; >>>> Which pins do you need to dynamically remux? If it's not all >>>> the pins, you should have three sets: default, active and idle. >>>> This way the static pins in the default group don't need to be >>>> constantly toggled. >>>> >>>> Regards, >>>> >>>> Tony >>> Tony >>> >>> I am using this for all the pins, in probe all the cpsw pins will >>> be configured >>> and i have used the same in suspend/resume callback for power saving. >>> >> Tony >> >> Do you have any comments on this, or is it ok with two pinctrl_state nodes? > If you always need to remux all the pins, then yes that's fine with me. > > Regards, > > Tony David As Tony accepted the implementation, I will resend the patch series as one of the patch (pasted below) in this series is already merged in net-next. f6655d6 Mugunthan V N ARM: dts: AM33XX: Add CPSW phy_id device tree data to am335x-evmsk Mon Jun 3 20:10:09 2013 +0000 Regards Mugunthan V N -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c index 21a5b29..c9ed730 100644 --- a/drivers/net/ethernet/ti/cpsw.c +++ b/drivers/net/ethernet/ti/cpsw.c @@ -35,6 +35,7 @@ #include <linux/if_vlan.h> #include <linux/platform_data/cpsw.h> +#include <linux/pinctrl/consumer.h> #include "cpsw_ale.h" #include "cpts.h" @@ -351,6 +352,11 @@ struct cpsw_priv { bool irq_enabled; struct cpts *cpts; u32 emac_port; + + /* Two optional pin states - default & sleep */ + struct pinctrl *pinctrl; + struct pinctrl_state *pins_def; + struct pinctrl_state *pins_sleep; }; #define napi_to_priv(napi) container_of(napi, struct cpsw_priv, napi) @@ -1689,6 +1695,35 @@ static int cpsw_probe(struct platform_device *pdev) */ pm_runtime_enable(&pdev->dev); + priv->pinctrl = devm_pinctrl_get(&pdev->dev); + if (!IS_ERR(priv->pinctrl)) { + priv->pins_def = pinctrl_lookup_state(priv->pinctrl, + PINCTRL_STATE_DEFAULT); + /* enable pins to be muxed in and configured */ + if (IS_ERR(priv->pins_def)) + dev_warn(&pdev->dev, "could not get default pinstate\n"); + else + if (pinctrl_select_state(priv->pinctrl, + priv->pins_def)) + dev_err(&pdev->dev, + "could not set default pins\n"); + + priv->pins_sleep = pinctrl_lookup_state(priv->pinctrl, + PINCTRL_STATE_SLEEP); + if (IS_ERR(priv->pins_sleep)) + dev_warn(&pdev->dev, "could not get sleep pinstate\n"); + } else { + /* Since we continue even when pinctrl node is not found, + * Invalidate pins as not available. This is to make sure that + * IS_ERR(pins_xxx) results in failure when used. + */ + priv->pins_def = ERR_PTR(-ENODATA); + priv->pins_sleep = ERR_PTR(-ENODATA); + + dev_warn(&pdev->dev, + "pins are not configured from the driver\n"); + } + if (cpsw_probe_dt(&priv->data, pdev)) { pr_err("cpsw: platform data missing\n"); ret = -ENODEV; @@ -1973,11 +2008,17 @@ static int cpsw_suspend(struct device *dev) { struct platform_device *pdev = to_platform_device(dev); struct net_device *ndev = platform_get_drvdata(pdev); + struct cpsw_priv *priv = netdev_priv(ndev); if (netif_running(ndev)) cpsw_ndo_stop(ndev); pm_runtime_put_sync(&pdev->dev); + /* Optionally let pins go into sleep states */ + if (!IS_ERR(priv->pins_sleep)) + if (pinctrl_select_state(priv->pinctrl, priv->pins_sleep)) + dev_err(dev, "could not set pins to sleep state\n"); + return 0; } @@ -1985,8 +2026,15 @@ static int cpsw_resume(struct device *dev) { struct platform_device *pdev = to_platform_device(dev); struct net_device *ndev = platform_get_drvdata(pdev); + struct cpsw_priv *priv = netdev_priv(ndev); pm_runtime_get_sync(&pdev->dev); + + /* Optionaly enable pins to be muxed in and configured */ + if (!IS_ERR(priv->pins_def)) + if (pinctrl_select_state(priv->pinctrl, priv->pins_def)) + dev_err(dev, "could not set default pins\n"); + if (netif_running(ndev)) cpsw_ndo_open(ndev); return 0;