diff mbox

[2/4] thermal: rockchip: ensure the otp state before resetting the controller

Message ID 1445332264-6054-3-git-send-email-wxt@rock-chips.com (mailing list archive)
State New, archived
Headers show

Commit Message

Caesar Wang Oct. 20, 2015, 9:11 a.m. UTC
We need the OTP pin is gpio state before resetting the TSADC controller,
since the tshut polarity will generate a high signal.

Says:
The TSHUT temperature is setting more than 80 degree, the default tshut
polarity is high.

If T > 80C, the OTP output the High Signal.
If T < 80C, the OTP output the Low Signal.

On the moment, the TSADC controller is reset, the tshut polarity will be
low in a short period of time.
So:

If T < 80C, the OTP output the High Signal.
If T > 80C, the OTP output the Low Signal.

In some cases, the OTP pin is connected to the PMIC, maybe the PMIC can
accept the reset response time to avoid this issue.
In other words, the system will be always reboot if we make the OTP pin
is connected the others IC to control the power.

Signed-off-by: Caesar Wang <wxt@rock-chips.com>
---

 drivers/thermal/rockchip_thermal.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

Comments

Doug Anderson Oct. 20, 2015, 3:52 p.m. UTC | #1
Caesar,

On Tue, Oct 20, 2015 at 2:11 AM, Caesar Wang <wxt@rock-chips.com> wrote:
> We need the OTP pin is gpio state before resetting the TSADC controller,
> since the tshut polarity will generate a high signal.
>
> Says:
> The TSHUT temperature is setting more than 80 degree, the default tshut
> polarity is high.
>
> If T > 80C, the OTP output the High Signal.
> If T < 80C, the OTP output the Low Signal.
>
> On the moment, the TSADC controller is reset, the tshut polarity will be
> low in a short period of time.
> So:
>
> If T < 80C, the OTP output the High Signal.
> If T > 80C, the OTP output the Low Signal.
>
> In some cases, the OTP pin is connected to the PMIC, maybe the PMIC can
> accept the reset response time to avoid this issue.
> In other words, the system will be always reboot if we make the OTP pin
> is connected the others IC to control the power.
>
> Signed-off-by: Caesar Wang <wxt@rock-chips.com>
> ---
>
>  drivers/thermal/rockchip_thermal.c | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)

I think you could do this with no code changes to the thermal driver
if we simply convince Linus W. to apply a change that I posted up just
about a year ago.  See:

https://patchwork.kernel.org/patch/5055741/

In v1 of that patch at <https://patchwork.kernel.org/patch/5049041/>
Linus said he liked it "A lot" and was willing to merge it with Greg
KH's Ack and with a small comment fix.  I obtained the Ack and fixed
the comment, but then the patch didn't end up being needed for me and
so I never bumped it and it got lost...

Maybe you could re-test that patch?  It looks like it has a merge
conflict with current linuxnext but it looks trivial to resolve.  You
could re-post my patch or I could repost it and you could add your
Tested-by.

You'd still want to have a bindings change to describe "init", but at
least you shouldn't need any code changes.


-Doug
Caesar Wang Oct. 21, 2015, 1:41 a.m. UTC | #2
Doug,

? 2015?10?20? 23:52, Doug Anderson ??:
> Caesar,
>
> On Tue, Oct 20, 2015 at 2:11 AM, Caesar Wang <wxt@rock-chips.com> wrote:
>> We need the OTP pin is gpio state before resetting the TSADC controller,
>> since the tshut polarity will generate a high signal.
>>
>> Says:
>> The TSHUT temperature is setting more than 80 degree, the default tshut
>> polarity is high.
>>
>> If T > 80C, the OTP output the High Signal.
>> If T < 80C, the OTP output the Low Signal.
>>
>> On the moment, the TSADC controller is reset, the tshut polarity will be
>> low in a short period of time.
>> So:
>>
>> If T < 80C, the OTP output the High Signal.
>> If T > 80C, the OTP output the Low Signal.
>>
>> In some cases, the OTP pin is connected to the PMIC, maybe the PMIC can
>> accept the reset response time to avoid this issue.
>> In other words, the system will be always reboot if we make the OTP pin
>> is connected the others IC to control the power.
>>
>> Signed-off-by: Caesar Wang <wxt@rock-chips.com>
>> ---
>>
>>   drivers/thermal/rockchip_thermal.c | 32 ++++++++++++++++++++++++++++++++
>>   1 file changed, 32 insertions(+)
> I think you could do this with no code changes to the thermal driver
> if we simply convince Linus W. to apply a change that I posted up just
> about a year ago.  See:
>
> https://patchwork.kernel.org/patch/5055741/
>
> In v1 of that patch at <https://patchwork.kernel.org/patch/5049041/>
> Linus said he liked it "A lot" and was willing to merge it with Greg
> KH's Ack and with a small comment fix.  I obtained the Ack and fixed
> the comment, but then the patch didn't end up being needed for me and
> so I never bumped it and it got lost...
>
> Maybe you could re-test that patch?  It looks like it has a merge
> conflict with current linuxnext but it looks trivial to resolve.  You
> could re-post my patch or I could repost it and you could add your
> Tested-by.
>
> You'd still want to have a bindings change to describe "init", but at
> least you shouldn't need any code changes.

Okay, https://patchwork.kernel.org/patch/5055741/ that's working for me.
Fell free add my  test tag if you resend the patch. (Tested-by: Caesar 
Wang <wxt@rock-chips.com>)


1634ed8 FROMLIST: drivers/pinctrl: Add the concept of an "init" state
15158f8 FROMLIST: ARM: dts: rockchip: Add the OTP gpio pinctrl
e7d3b88 FROMLIST: thermal: rockchip: change the TSHUT default state
184b154 FROMLIST: thermal: rockchip: ensure the otp state before 
resetting the controller
9edbe15 FROMLIST: dt-bindings: Sync the dts to this document

Meanwhile, I change the dts as follows.

-               pinctrl-names = "default", "otp_out";
+               pinctrl-names = "init", "default";
                 pinctrl-0 = <&otp_gpio>;
                 pinctrl-1 = <&otp_out>;

>
> -Doug
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
Linus Walleij Oct. 27, 2015, 10:23 a.m. UTC | #3
On Wed, Oct 21, 2015 at 3:41 AM, Caesar Wang <caesar.upstream@gmail.com> wrote:
>> On Tue, Oct 20, 2015 at 2:11 AM, Caesar Wang <wxt@rock-chips.com> wrote:

>> You'd still want to have a bindings change to describe "init", but at
>> least you shouldn't need any code changes.
>
> Okay, https://patchwork.kernel.org/patch/5055741/ that's working for me.
> Fell free add my  test tag if you resend the patch. (Tested-by: Caesar Wang
> <wxt@rock-chips.com>)

Doug, can you resend this rebased version with Caesar's Test tag?
I kinda like this more than any alternatives.

Yours,
Linus Walleij
diff mbox

Patch

diff --git a/drivers/thermal/rockchip_thermal.c b/drivers/thermal/rockchip_thermal.c
index c89ffb2..c53e318 100644
--- a/drivers/thermal/rockchip_thermal.c
+++ b/drivers/thermal/rockchip_thermal.c
@@ -22,6 +22,7 @@ 
 #include <linux/platform_device.h>
 #include <linux/reset.h>
 #include <linux/thermal.h>
+#include <linux/pinctrl/consumer.h>
 
 /**
  * If the temperature over a period of time High,
@@ -79,6 +80,9 @@  struct rockchip_thermal_sensor {
 
 struct rockchip_thermal_data {
 	const struct rockchip_tsadc_chip *chip;
+	struct pinctrl *pinctrl;
+	struct pinctrl_state *pins_default;
+	struct pinctrl_state *pins_otp;
 	struct platform_device *pdev;
 	struct reset_control *reset;
 
@@ -548,6 +552,28 @@  static int rockchip_thermal_probe(struct platform_device *pdev)
 		goto err_disable_clk;
 	}
 
+	/*
+	 * We need the OTP pin is gpio state before reset the TSADC controller
+	 * since the tshut polarity will generate a high signal.
+	 */
+
+	thermal->pinctrl = devm_pinctrl_get(&pdev->dev);
+	if (IS_ERR(thermal->pinctrl)) {
+		error = PTR_ERR(thermal->pinctrl);
+		dev_err(&pdev->dev, "failed to get thermal pinctrl: %d\n",
+			error);
+		goto err_disable_pclk;
+	}
+
+	thermal->pins_default = pinctrl_lookup_state(thermal->pinctrl,
+						     "default");
+	if (IS_ERR(thermal->pins_default))
+		dev_warn(&pdev->dev, "could not get the pinctrl default state\n");
+
+	thermal->pins_otp = pinctrl_lookup_state(thermal->pinctrl, "otp_out");
+	if (IS_ERR(thermal->pins_otp))
+		dev_warn(&pdev->dev, "could not get otp state\n");
+
 	rockchip_thermal_reset_controller(thermal->reset);
 
 	error = rockchip_configure_from_dt(&pdev->dev, np, thermal);
@@ -592,6 +618,8 @@  static int rockchip_thermal_probe(struct platform_device *pdev)
 	for (i = 0; i < ARRAY_SIZE(thermal->sensors); i++)
 		rockchip_thermal_toggle_sensor(&thermal->sensors[i], true);
 
+	pinctrl_select_state(thermal->pinctrl, thermal->pins_otp);
+
 	platform_set_drvdata(pdev, thermal);
 
 	return 0;
@@ -660,6 +688,8 @@  static int __maybe_unused rockchip_thermal_resume(struct device *dev)
 	if (error)
 		return error;
 
+	pinctrl_select_state(thermal->pinctrl, thermal->pins_default);
+
 	rockchip_thermal_reset_controller(thermal->reset);
 
 	thermal->chip->initialize(thermal->regs, thermal->tshut_polarity);
@@ -678,6 +708,8 @@  static int __maybe_unused rockchip_thermal_resume(struct device *dev)
 	for (i = 0; i < ARRAY_SIZE(thermal->sensors); i++)
 		rockchip_thermal_toggle_sensor(&thermal->sensors[i], true);
 
+	pinctrl_select_state(thermal->pinctrl, thermal->pins_otp);
+
 	return 0;
 }