| Message ID | 1414417650-19402-3-git-send-email-zyw@rock-chips.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Am Montag, 27. Oktober 2014, 21:47:26 schrieb Chris Zhong: > Save and restore the gpio6_c6 pinmux setting, since Maskrom of RK3288 > would modify it to sdmmc0_det, so it need to be restored to the correct > setting after resume from Maskrom. Does the maskrom only touch the iomux or also stuff like pull-bias or drive strength for this pin? Also could you please look up if the maskrom on rk3066a and rk3188 does something similar. While not strictly necessary for this patch, I'd like the driver to not diverge in functionality for to long. Otherwise it looks fine for the time being. Tested-by: Heiko Stuebner <heiko@sntech.de> Reviewed-by: Heiko Stuebner <heiko@sntech.de> > > Signed-off-by: Chris Zhong <zyw@rock-chips.com> > > Reviewed-by: Doug Anderson <dianders@chromium.org> > Tested-by: Doug Anderson <dianders@chromium.org> > --- > > Changes in v5: > - add pinctrl_force_default() in the error case > > Changes in v4: None > Changes in v3: None > Changes in v2: None > > drivers/pinctrl/pinctrl-rockchip.c | 30 +++++++++++++++++++++++++++++- > 1 file changed, 29 insertions(+), 1 deletion(-) > > diff --git a/drivers/pinctrl/pinctrl-rockchip.c > b/drivers/pinctrl/pinctrl-rockchip.c index 172ad1d..49ca40c 100644 > --- a/drivers/pinctrl/pinctrl-rockchip.c > +++ b/drivers/pinctrl/pinctrl-rockchip.c > @@ -1770,16 +1770,44 @@ static struct rockchip_pin_ctrl > *rockchip_pinctrl_get_soc_data( return ctrl; > } > > +#define RK3288_GRF_GPIO6C_IOMUX 0x64 > +#define GPIO6C6_SEL_WRITE_ENABLE BIT(28) > + > +static u32 rk3288_grf_gpio6c_iomux; > + > static int __maybe_unused rockchip_pinctrl_suspend(struct device *dev) > { > struct rockchip_pinctrl *info = dev_get_drvdata(dev); > + int ret = pinctrl_force_sleep(info->pctl_dev); > + > + if (ret) > + return ret; > + > + /* > + * RK3288 GPIO6_C6 mux would be modified by Maskrom when resume, so save > + * the setting here, and restore it at resume. > + */ > + if (info->ctrl->type == RK3288) { > + ret = regmap_read(info->regmap_base, RK3288_GRF_GPIO6C_IOMUX, > + &rk3288_grf_gpio6c_iomux); > + if (ret) { > + pinctrl_force_default(info->pctl_dev); > + return ret; > + } > + } > > - return pinctrl_force_sleep(info->pctl_dev); > + return 0; > } > > static int __maybe_unused rockchip_pinctrl_resume(struct device *dev) > { > struct rockchip_pinctrl *info = dev_get_drvdata(dev); > + int ret = regmap_write(info->regmap_base, RK3288_GRF_GPIO6C_IOMUX, > + rk3288_grf_gpio6c_iomux | > + GPIO6C6_SEL_WRITE_ENABLE); > + > + if (ret) > + return ret; > > return pinctrl_force_default(info->pctl_dev); > }
Chris, On Tue, Oct 28, 2014 at 7:31 AM, Heiko Stübner <heiko@sntech.de> wrote: > Am Montag, 27. Oktober 2014, 21:47:26 schrieb Chris Zhong: >> Save and restore the gpio6_c6 pinmux setting, since Maskrom of RK3288 >> would modify it to sdmmc0_det, so it need to be restored to the correct >> setting after resume from Maskrom. > > Does the maskrom only touch the iomux or also stuff like pull-bias or drive > strength for this pin? > > Also could you please look up if the maskrom on rk3066a and rk3188 does > something similar. While not strictly necessary for this patch, I'd like the > driver to not diverge in functionality for to long. > > > Otherwise it looks fine for the time being. > > Tested-by: Heiko Stuebner <heiko@sntech.de> > Reviewed-by: Heiko Stuebner <heiko@sntech.de> I notice that you posted patch v6 but you didn't respond to Heiko's questions here and made no changes in v6. Can you please answer Heiko's questions? The questions about whether the pull-bias and drive also need to be reset would be good to get answered. -Doug
Am Mittwoch, 29. Oktober 2014, 09:26:39 schrieb Doug Anderson: > Chris, > > On Tue, Oct 28, 2014 at 7:31 AM, Heiko Stübner <heiko@sntech.de> wrote: > > Am Montag, 27. Oktober 2014, 21:47:26 schrieb Chris Zhong: > >> Save and restore the gpio6_c6 pinmux setting, since Maskrom of RK3288 > >> would modify it to sdmmc0_det, so it need to be restored to the correct > >> setting after resume from Maskrom. > > > > Does the maskrom only touch the iomux or also stuff like pull-bias or > > drive > > strength for this pin? > > > > Also could you please look up if the maskrom on rk3066a and rk3188 does > > something similar. While not strictly necessary for this patch, I'd like > > the driver to not diverge in functionality for to long. > > > > > > Otherwise it looks fine for the time being. > > > > Tested-by: Heiko Stuebner <heiko@sntech.de> > > Reviewed-by: Heiko Stuebner <heiko@sntech.de> > > I notice that you posted patch v6 but you didn't respond to Heiko's > questions here and made no changes in v6. > > Can you please answer Heiko's questions? The questions about whether > the pull-bias and drive also need to be reset would be good to get > answered. and also how the other socs 3066a and 3188 behave in this regard would be really nice to know. Because otherwise this might be forgotten when we implement suspend on those and result in us searching for reason why it might fail then. Thanks Heiko
On 10/30/2014 04:02 AM, Heiko Stübner wrote: > Am Mittwoch, 29. Oktober 2014, 09:26:39 schrieb Doug Anderson: >> Chris, >> >> On Tue, Oct 28, 2014 at 7:31 AM, Heiko Stübner <heiko@sntech.de> wrote: >>> Am Montag, 27. Oktober 2014, 21:47:26 schrieb Chris Zhong: >>>> Save and restore the gpio6_c6 pinmux setting, since Maskrom of RK3288 >>>> would modify it to sdmmc0_det, so it need to be restored to the correct >>>> setting after resume from Maskrom. >>> Does the maskrom only touch the iomux or also stuff like pull-bias or >>> drive >>> strength for this pin? >>> >>> Also could you please look up if the maskrom on rk3066a and rk3188 does >>> something similar. While not strictly necessary for this patch, I'd like >>> the driver to not diverge in functionality for to long. >>> >>> >>> Otherwise it looks fine for the time being. >>> >>> Tested-by: Heiko Stuebner <heiko@sntech.de> >>> Reviewed-by: Heiko Stuebner <heiko@sntech.de> >> I notice that you posted patch v6 but you didn't respond to Heiko's >> questions here and made no changes in v6. >> >> Can you please answer Heiko's questions? The questions about whether >> the pull-bias and drive also need to be reset would be good to get >> answered. > and also how the other socs 3066a and 3188 behave in this regard would be > really nice to know. Because otherwise this might be forgotten when we > implement suspend on those and result in us searching for reason why it might > fail then. Hi, Heiko sorry for I did not respond promptly. RK3288 maskrom only change the iomux function setting, does not change pull-bias and drive strength. And RK3066A and RK3188 don't support turn ARM off during suspend, so they have not this problem, since they wouldn't go to maskrom during resume. > > Thanks > Heiko > > _______________________________________________ > Linux-rockchip mailing list > Linux-rockchip@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-rockchip > > >
diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c index 172ad1d..49ca40c 100644 --- a/drivers/pinctrl/pinctrl-rockchip.c +++ b/drivers/pinctrl/pinctrl-rockchip.c @@ -1770,16 +1770,44 @@ static struct rockchip_pin_ctrl *rockchip_pinctrl_get_soc_data( return ctrl; } +#define RK3288_GRF_GPIO6C_IOMUX 0x64 +#define GPIO6C6_SEL_WRITE_ENABLE BIT(28) + +static u32 rk3288_grf_gpio6c_iomux; + static int __maybe_unused rockchip_pinctrl_suspend(struct device *dev) { struct rockchip_pinctrl *info = dev_get_drvdata(dev); + int ret = pinctrl_force_sleep(info->pctl_dev); + + if (ret) + return ret; + + /* + * RK3288 GPIO6_C6 mux would be modified by Maskrom when resume, so save + * the setting here, and restore it at resume. + */ + if (info->ctrl->type == RK3288) { + ret = regmap_read(info->regmap_base, RK3288_GRF_GPIO6C_IOMUX, + &rk3288_grf_gpio6c_iomux); + if (ret) { + pinctrl_force_default(info->pctl_dev); + return ret; + } + } - return pinctrl_force_sleep(info->pctl_dev); + return 0; } static int __maybe_unused rockchip_pinctrl_resume(struct device *dev) { struct rockchip_pinctrl *info = dev_get_drvdata(dev); + int ret = regmap_write(info->regmap_base, RK3288_GRF_GPIO6C_IOMUX, + rk3288_grf_gpio6c_iomux | + GPIO6C6_SEL_WRITE_ENABLE); + + if (ret) + return ret; return pinctrl_force_default(info->pctl_dev); }