[v5,2/6] pinctrl: rockchip: save and restore gpio6_c6 pinmux in suspend/resume
diff mbox

Message ID 1414417650-19402-3-git-send-email-zyw@rock-chips.com
State New, archived
Headers show

Commit Message

Chris Zhong Oct. 27, 2014, 1:47 p.m. UTC
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.

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(-)

Comments

Heiko Stübner Oct. 28, 2014, 2:31 p.m. UTC | #1
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);
>  }
Douglas Anderson Oct. 29, 2014, 4:26 p.m. UTC | #2
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
Heiko Stübner Oct. 29, 2014, 8:02 p.m. UTC | #3
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
Chris Zhong Oct. 30, 2014, 8:47 a.m. UTC | #4
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
>
>
>

Patch
diff mbox

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);
 }