linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/3] thermal: rockchip: fix up thermal driver
@ 2019-04-25 10:12 Elaine Zhang
  2019-04-25 10:12 ` [PATCH v2 1/3] thermal: rockchip: fix up the tsadc pinctrl setting error Elaine Zhang
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Elaine Zhang @ 2019-04-25 10:12 UTC (permalink / raw)
  To: heiko
  Cc: rui.zhang, edubezval, daniel.lezcano, robh+dt, mark.rutland,
	linux-pm, devicetree, linux-arm-kernel, linux-rockchip,
	linux-kernel, xxx, xf, huangtao, Elaine Zhang

1. add pinctrl control.
2. support PX30 soc

change in V2:
PATCH V2 1/3: keep tshut_mode TSHUT_MODE_GPIO;
	      In case of pinctrl get or lookup error, just bail out;
	      No need to use the thermal_pinctrl_select_otp/gpio wrappers,
	      just replace them with:
PATCH V2 2/3: No change in V2.
PATCH V2 2/3: keep tshut_mode TSHUT_MODE_GPIO;
              Remove the grf in 'rk_tsadcv4_initialize' function.

pinctrl_select_state(thermal->pinctrl, thermal->pinctrl_state);
Elaine Zhang (3):
  thermal: rockchip: fix up the tsadc pinctrl setting error
  dt-bindings: rockchip-thermal: Support the PX30 SoC compatible
  thermal: rockchip: Support the PX30 SoC in thermal driver

 .../bindings/thermal/rockchip-thermal.txt          |  1 +
 drivers/thermal/rockchip_thermal.c                 | 74 +++++++++++++++++++++-
 2 files changed, 72 insertions(+), 3 deletions(-)

-- 
1.9.1




^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 1/3] thermal: rockchip: fix up the tsadc pinctrl setting error
  2019-04-25 10:12 [PATCH v2 0/3] thermal: rockchip: fix up thermal driver Elaine Zhang
@ 2019-04-25 10:12 ` Elaine Zhang
  2019-04-28 12:45   ` Daniel Lezcano
  2019-04-25 10:12 ` [PATCH v2 2/3] dt-bindings: rockchip-thermal: Support the PX30 SoC compatible Elaine Zhang
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 10+ messages in thread
From: Elaine Zhang @ 2019-04-25 10:12 UTC (permalink / raw)
  To: heiko
  Cc: rui.zhang, edubezval, daniel.lezcano, robh+dt, mark.rutland,
	linux-pm, devicetree, linux-arm-kernel, linux-rockchip,
	linux-kernel, xxx, xf, huangtao, Elaine Zhang

Explicitly use the pinctrl to set/unset the right mode
instead of relying on the pinctrl init mode.
And it requires setting the tshut polarity before select pinctrl.

When the temperature sensor mode is set to 0, it will automatically
reset the board via the Clock-Reset-Unit (CRU) if the over temperature
threshold is reached. However, when the pinctrl initializes, it does a
transition to "otp_out" which may lead the SoC restart all the time.

"otp_out" IO may be connected to the RESET circuit on the hardware.
If the IO is in the wrong state, it will trigger RESET.
(similar to the effect of pressing the RESET button)
which will cause the soc to restart all the time.

Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>
---
 drivers/thermal/rockchip_thermal.c | 37 ++++++++++++++++++++++++++++++++++---
 1 file changed, 34 insertions(+), 3 deletions(-)

diff --git a/drivers/thermal/rockchip_thermal.c b/drivers/thermal/rockchip_thermal.c
index 9c7643d62ed7..03ff494f2864 100644
--- a/drivers/thermal/rockchip_thermal.c
+++ b/drivers/thermal/rockchip_thermal.c
@@ -172,6 +172,9 @@ struct rockchip_thermal_data {
 	int tshut_temp;
 	enum tshut_mode tshut_mode;
 	enum tshut_polarity tshut_polarity;
+	struct pinctrl *pinctrl;
+	struct pinctrl_state *gpio_state;
+	struct pinctrl_state *otp_state;
 };
 
 /**
@@ -1242,6 +1245,8 @@ static int rockchip_thermal_probe(struct platform_device *pdev)
 		return error;
 	}
 
+	thermal->chip->control(thermal->regs, false);
+
 	error = clk_prepare_enable(thermal->clk);
 	if (error) {
 		dev_err(&pdev->dev, "failed to enable converter clock: %d\n",
@@ -1267,6 +1272,31 @@ static int rockchip_thermal_probe(struct platform_device *pdev)
 	thermal->chip->initialize(thermal->grf, thermal->regs,
 				  thermal->tshut_polarity);
 
+	if (thermal->tshut_mode == TSHUT_MODE_GPIO) {
+		thermal->pinctrl = devm_pinctrl_get(&pdev->dev);
+		if (IS_ERR(thermal->pinctrl)) {
+			dev_err(&pdev->dev, "failed to find thermal pinctrl\n");
+			panic("panic_on_find thermal pinctrl...\n");
+			return PTR_ERR(thermal->pinctrl);
+		}
+
+		thermal->gpio_state = pinctrl_lookup_state(thermal->pinctrl,
+							   "gpio");
+		if (IS_ERR_OR_NULL(thermal->gpio_state)) {
+			dev_err(&pdev->dev, "failed to find thermal gpio state\n");
+			return -EINVAL;
+		}
+
+		thermal->otp_state = pinctrl_lookup_state(thermal->pinctrl,
+							  "otpout");
+		if (IS_ERR_OR_NULL(thermal->otp_state)) {
+			dev_err(&pdev->dev, "failed to find thermal otpout state\n");
+			return -EINVAL;
+		}
+
+		pinctrl_select_state(thermal->pinctrl, thermal->otp_state);
+	}
+
 	for (i = 0; i < thermal->chip->chn_num; i++) {
 		error = rockchip_thermal_register_sensor(pdev, thermal,
 						&thermal->sensors[i],
@@ -1337,8 +1367,8 @@ static int __maybe_unused rockchip_thermal_suspend(struct device *dev)
 
 	clk_disable(thermal->pclk);
 	clk_disable(thermal->clk);
-
-	pinctrl_pm_select_sleep_state(dev);
+	if (thermal->tshut_mode == TSHUT_MODE_GPIO)
+		pinctrl_select_state(thermal->pinctrl, thermal->gpio_state);
 
 	return 0;
 }
@@ -1383,7 +1413,8 @@ static int __maybe_unused rockchip_thermal_resume(struct device *dev)
 	for (i = 0; i < thermal->chip->chn_num; i++)
 		rockchip_thermal_toggle_sensor(&thermal->sensors[i], true);
 
-	pinctrl_pm_select_default_state(dev);
+	if (thermal->tshut_mode == TSHUT_MODE_GPIO)
+		pinctrl_select_state(thermal->pinctrl, thermal->otp_state);
 
 	return 0;
 }
-- 
1.9.1




^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 2/3] dt-bindings: rockchip-thermal: Support the PX30 SoC compatible
  2019-04-25 10:12 [PATCH v2 0/3] thermal: rockchip: fix up thermal driver Elaine Zhang
  2019-04-25 10:12 ` [PATCH v2 1/3] thermal: rockchip: fix up the tsadc pinctrl setting error Elaine Zhang
@ 2019-04-25 10:12 ` Elaine Zhang
  2019-04-25 10:12 ` [PATCH v2 3/3] thermal: rockchip: Support the PX30 SoC in thermal driver Elaine Zhang
  2019-04-25 10:12 ` [PATCH v3 0/4] clk: rockchip: support clock controller for px30 SoC Elaine Zhang
  3 siblings, 0 replies; 10+ messages in thread
From: Elaine Zhang @ 2019-04-25 10:12 UTC (permalink / raw)
  To: heiko
  Cc: rui.zhang, edubezval, daniel.lezcano, robh+dt, mark.rutland,
	linux-pm, devicetree, linux-arm-kernel, linux-rockchip,
	linux-kernel, xxx, xf, huangtao, Elaine Zhang

Add a new compatible for thermal founding on PX30 SoCs.

Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>
Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/thermal/rockchip-thermal.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/thermal/rockchip-thermal.txt b/Documentation/devicetree/bindings/thermal/rockchip-thermal.txt
index 43d744e5305e..c6aac9bcacf1 100644
--- a/Documentation/devicetree/bindings/thermal/rockchip-thermal.txt
+++ b/Documentation/devicetree/bindings/thermal/rockchip-thermal.txt
@@ -2,6 +2,7 @@
 
 Required properties:
 - compatible : should be "rockchip,<name>-tsadc"
+   "rockchip,px30-tsadc":   found on PX30 SoCs
    "rockchip,rv1108-tsadc": found on RV1108 SoCs
    "rockchip,rk3228-tsadc": found on RK3228 SoCs
    "rockchip,rk3288-tsadc": found on RK3288 SoCs
-- 
1.9.1




^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 3/3] thermal: rockchip: Support the PX30 SoC in thermal driver
  2019-04-25 10:12 [PATCH v2 0/3] thermal: rockchip: fix up thermal driver Elaine Zhang
  2019-04-25 10:12 ` [PATCH v2 1/3] thermal: rockchip: fix up the tsadc pinctrl setting error Elaine Zhang
  2019-04-25 10:12 ` [PATCH v2 2/3] dt-bindings: rockchip-thermal: Support the PX30 SoC compatible Elaine Zhang
@ 2019-04-25 10:12 ` Elaine Zhang
  2019-04-25 10:12 ` [PATCH v3 0/4] clk: rockchip: support clock controller for px30 SoC Elaine Zhang
  3 siblings, 0 replies; 10+ messages in thread
From: Elaine Zhang @ 2019-04-25 10:12 UTC (permalink / raw)
  To: heiko
  Cc: rui.zhang, edubezval, daniel.lezcano, robh+dt, mark.rutland,
	linux-pm, devicetree, linux-arm-kernel, linux-rockchip,
	linux-kernel, xxx, xf, huangtao, Elaine Zhang

PX30 SOC has two Temperature Sensors for CPU and GPU.

Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>
---
 drivers/thermal/rockchip_thermal.c | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/drivers/thermal/rockchip_thermal.c b/drivers/thermal/rockchip_thermal.c
index 03ff494f2864..246d9513a465 100644
--- a/drivers/thermal/rockchip_thermal.c
+++ b/drivers/thermal/rockchip_thermal.c
@@ -225,11 +225,15 @@ struct rockchip_thermal_data {
 #define GRF_TSADC_TESTBIT_L			0x0e648
 #define GRF_TSADC_TESTBIT_H			0x0e64c
 
+#define PX30_GRF_SOC_CON2			0x0408
+
 #define GRF_SARADC_TESTBIT_ON			(0x10001 << 2)
 #define GRF_TSADC_TESTBIT_H_ON			(0x10001 << 2)
 #define GRF_TSADC_VCM_EN_L			(0x10001 << 7)
 #define GRF_TSADC_VCM_EN_H			(0x10001 << 7)
 
+#define GRF_CON_TSADC_CH_INV			(0x10001 << 1)
+
 /**
  * struct tsadc_table - code to temperature conversion table
  * @code: the value of adc channel
@@ -692,6 +696,13 @@ static void rk_tsadcv3_initialize(struct regmap *grf, void __iomem *regs,
 			       regs + TSADCV2_AUTO_CON);
 }
 
+static void rk_tsadcv4_initialize(struct regmap *grf, void __iomem *regs,
+				  enum tshut_polarity tshut_polarity)
+{
+	rk_tsadcv2_initialize(grf, regs, tshut_polarity);
+	regmap_write(grf, PX30_GRF_SOC_CON2, GRF_CON_TSADC_CH_INV);
+}
+
 static void rk_tsadcv2_irq_ack(void __iomem *regs)
 {
 	u32 val;
@@ -821,6 +832,30 @@ static void rk_tsadcv2_tshut_mode(int chn, void __iomem *regs,
 	writel_relaxed(val, regs + TSADCV2_INT_EN);
 }
 
+static const struct rockchip_tsadc_chip px30_tsadc_data = {
+	.chn_id[SENSOR_CPU] = 0, /* cpu sensor is channel 0 */
+	.chn_id[SENSOR_GPU] = 1, /* gpu sensor is channel 1 */
+	.chn_num = 2, /* 2 channels for tsadc */
+
+	.tshut_mode = TSHUT_MODE_CRU, /* default TSHUT via CRU */
+	.tshut_temp = 95000,
+
+	.initialize = rk_tsadcv4_initialize,
+	.irq_ack = rk_tsadcv3_irq_ack,
+	.control = rk_tsadcv3_control,
+	.get_temp = rk_tsadcv2_get_temp,
+	.set_alarm_temp = rk_tsadcv2_alarm_temp,
+	.set_tshut_temp = rk_tsadcv2_tshut_temp,
+	.set_tshut_mode = rk_tsadcv2_tshut_mode,
+
+	.table = {
+		.id = rk3328_code_table,
+		.length = ARRAY_SIZE(rk3328_code_table),
+		.data_mask = TSADCV2_DATA_MASK,
+		.mode = ADC_INCREMENT,
+	},
+};
+
 static const struct rockchip_tsadc_chip rv1108_tsadc_data = {
 	.chn_id[SENSOR_CPU] = 0, /* cpu sensor is channel 0 */
 	.chn_num = 1, /* one channel for tsadc */
@@ -993,6 +1028,9 @@ static void rk_tsadcv2_tshut_mode(int chn, void __iomem *regs,
 };
 
 static const struct of_device_id of_rockchip_thermal_match[] = {
+	{	.compatible = "rockchip,px30-tsadc",
+		.data = (void *)&px30_tsadc_data,
+	},
 	{
 		.compatible = "rockchip,rv1108-tsadc",
 		.data = (void *)&rv1108_tsadc_data,
-- 
1.9.1




^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v3 0/4] clk: rockchip: support clock controller for px30 SoC
  2019-04-25 10:12 [PATCH v2 0/3] thermal: rockchip: fix up thermal driver Elaine Zhang
                   ` (2 preceding siblings ...)
  2019-04-25 10:12 ` [PATCH v2 3/3] thermal: rockchip: Support the PX30 SoC in thermal driver Elaine Zhang
@ 2019-04-25 10:12 ` Elaine Zhang
  3 siblings, 0 replies; 10+ messages in thread
From: Elaine Zhang @ 2019-04-25 10:12 UTC (permalink / raw)
  To: heiko
  Cc: rui.zhang, edubezval, daniel.lezcano, robh+dt, mark.rutland,
	linux-pm, devicetree, linux-arm-kernel, linux-rockchip,
	linux-kernel, xxx, xf, huangtao, Elaine Zhang

Change in V3:
[PATCH v3 1/4]: Correct description
[PATCH v3 2/4]: Use an SPDX tag instead.
[PATCH v3 3/4]: Use an SPDX tag instead,
		parent_rate might overflow and fix it.
		fix up the checkpatch warning.
		add more CMPOSITE_xxx_HALFdiv.

Change in V2:
[PATCH v2 2/4]: modify the Author name
[PATCH v2 3/4]: provide a bit more explanation for commit message

Elaine Zhang (4):
  dt-bindings: add bindings for px30 clock controller
  clk: rockchip: add dt-binding header for px30
  clk: rockchip: add support for half divider
  clk: rockchip: add clock controller for px30

 .../bindings/clock/rockchip,px30-cru.txt           |   66 ++
 drivers/clk/rockchip/Makefile                      |    2 +
 drivers/clk/rockchip/clk-half-divider.c            |  231 +++++
 drivers/clk/rockchip/clk-px30.c                    | 1080 ++++++++++++++++++++
 drivers/clk/rockchip/clk.c                         |   10 +
 drivers/clk/rockchip/clk.h                         |  126 ++-
 include/dt-bindings/clock/px30-cru.h               |  389 +++++++
 7 files changed, 1903 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/clock/rockchip,px30-cru.txt
 create mode 100644 drivers/clk/rockchip/clk-half-divider.c
 create mode 100644 drivers/clk/rockchip/clk-px30.c
 create mode 100644 include/dt-bindings/clock/px30-cru.h

-- 
1.9.1




^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/3] thermal: rockchip: fix up the tsadc pinctrl setting error
  2019-04-25 10:12 ` [PATCH v2 1/3] thermal: rockchip: fix up the tsadc pinctrl setting error Elaine Zhang
@ 2019-04-28 12:45   ` Daniel Lezcano
  2019-04-29  9:51     ` elaine.zhang
  0 siblings, 1 reply; 10+ messages in thread
From: Daniel Lezcano @ 2019-04-28 12:45 UTC (permalink / raw)
  To: Elaine Zhang, heiko
  Cc: rui.zhang, edubezval, robh+dt, mark.rutland, linux-pm,
	devicetree, linux-arm-kernel, linux-rockchip, linux-kernel, xxx,
	xf, huangtao

On 25/04/2019 12:12, Elaine Zhang wrote:
> Explicitly use the pinctrl to set/unset the right mode
> instead of relying on the pinctrl init mode.
> And it requires setting the tshut polarity before select pinctrl.
> 
> When the temperature sensor mode is set to 0, it will automatically
> reset the board via the Clock-Reset-Unit (CRU) if the over temperature
> threshold is reached. However, when the pinctrl initializes, it does a
> transition to "otp_out" which may lead the SoC restart all the time.
> 
> "otp_out" IO may be connected to the RESET circuit on the hardware.
> If the IO is in the wrong state, it will trigger RESET.
> (similar to the effect of pressing the RESET button)
> which will cause the soc to restart all the time.
> 
> Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>
> ---
>  drivers/thermal/rockchip_thermal.c | 37 ++++++++++++++++++++++++++++++++++---
>  1 file changed, 34 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/thermal/rockchip_thermal.c b/drivers/thermal/rockchip_thermal.c
> index 9c7643d62ed7..03ff494f2864 100644
> --- a/drivers/thermal/rockchip_thermal.c
> +++ b/drivers/thermal/rockchip_thermal.c
> @@ -172,6 +172,9 @@ struct rockchip_thermal_data {
>  	int tshut_temp;
>  	enum tshut_mode tshut_mode;
>  	enum tshut_polarity tshut_polarity;
> +	struct pinctrl *pinctrl;
> +	struct pinctrl_state *gpio_state;
> +	struct pinctrl_state *otp_state;
>  };
>  
>  /**
> @@ -1242,6 +1245,8 @@ static int rockchip_thermal_probe(struct platform_device *pdev)
>  		return error;
>  	}
>  
> +	thermal->chip->control(thermal->regs, false);
> +
>  	error = clk_prepare_enable(thermal->clk);
>  	if (error) {
>  		dev_err(&pdev->dev, "failed to enable converter clock: %d\n",
> @@ -1267,6 +1272,31 @@ static int rockchip_thermal_probe(struct platform_device *pdev)
>  	thermal->chip->initialize(thermal->grf, thermal->regs,
>  				  thermal->tshut_polarity);
>  
> +	if (thermal->tshut_mode == TSHUT_MODE_GPIO) {
> +		thermal->pinctrl = devm_pinctrl_get(&pdev->dev);
> +		if (IS_ERR(thermal->pinctrl)) {
> +			dev_err(&pdev->dev, "failed to find thermal pinctrl\n");
> +			panic("panic_on_find thermal pinctrl...\n");

I realize my comment was confusing. I was not saying to add a panic()
call here but that the code was accessing a NULL pointer. Please remove
the panic.

> +			return PTR_ERR(thermal->pinctrl);
> +		}
> +
> +		thermal->gpio_state = pinctrl_lookup_state(thermal->pinctrl,
> +							   "gpio");
> +		if (IS_ERR_OR_NULL(thermal->gpio_state)) {
> +			dev_err(&pdev->dev, "failed to find thermal gpio state\n");
> +			return -EINVAL;
> +		}
> +
> +		thermal->otp_state = pinctrl_lookup_state(thermal->pinctrl,
> +							  "otpout");
> +		if (IS_ERR_OR_NULL(thermal->otp_state)) {
> +			dev_err(&pdev->dev, "failed to find thermal otpout state\n");
> +			return -EINVAL;
> +		}
> +
> +		pinctrl_select_state(thermal->pinctrl, thermal->otp_state);

I don't understand this portion of code. The test above says tshut_mode
is TSHUT_MODE_GPIO. Why acting on thermal->otp_state then ?


In my previous comment, I was suggesting the following:

Two more fields instead of three:

	struct rockchip_thermal_data {
  		int tshut_temp;
	  	enum tshut_mode tshut_mode;
  		enum tshut_polarity tshut_polarity;
	 	struct pinctrl *pinctrl;
		struct pinctrl_state *pinctrl_state;
	};

	[ ... ]

	thermal->pinctrl = devm_pinctrl_get(&pdev->dev);
	if (IS_ERR(thermal->pinctrl)) {
		dev_err("...");
		return PTR_ERR(thermal->pinctrl);
	}

	thermal->pinctrl_state = pinctrl_lookup_state(thermal->pinctrl,
		thermal->tshut_mode == TSHUT_MODE_GPIO ? "gpio" :
							"otpout";

	if (IS_ERR_OR_NULL(thermal->pinctrl_state) {
		dev_err("...");
		return PTR_ERR(thermal->pinctrl_state);
	}

 	pinctrl_select_state(thermal->pinctrl, thermal->pinctrl_state);


	[ ... ]

Is it wrong ?


> +	}
> +
>  	for (i = 0; i < thermal->chip->chn_num; i++) {
>  		error = rockchip_thermal_register_sensor(pdev, thermal,
>  						&thermal->sensors[i],
> @@ -1337,8 +1367,8 @@ static int __maybe_unused rockchip_thermal_suspend(struct device *dev)
>  
>  	clk_disable(thermal->pclk);
>  	clk_disable(thermal->clk);
> -
> -	pinctrl_pm_select_sleep_state(dev);
> +	if (thermal->tshut_mode == TSHUT_MODE_GPIO)
> +		pinctrl_select_state(thermal->pinctrl, thermal->gpio_state);

And then:
	 pinctrl_select_state(thermal->pinctrl, thermal->pinctrl_state);

>  
>  	return 0;
>  }
> @@ -1383,7 +1413,8 @@ static int __maybe_unused rockchip_thermal_resume(struct device *dev)
>  	for (i = 0; i < thermal->chip->chn_num; i++)
>  		rockchip_thermal_toggle_sensor(&thermal->sensors[i], true);
>  
> -	pinctrl_pm_select_default_state(dev);
> +	if (thermal->tshut_mode == TSHUT_MODE_GPIO)
> +		pinctrl_select_state(thermal->pinctrl, thermal->otp_state);

And then
	pinctrl_select_state(thermal->pinctrl, thermal->pinctrl_state);

>  	return 0;
>  }
> 


-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/3] thermal: rockchip: fix up the tsadc pinctrl setting error
  2019-04-28 12:45   ` Daniel Lezcano
@ 2019-04-29  9:51     ` elaine.zhang
  2019-04-30  9:44       ` Daniel Lezcano
  0 siblings, 1 reply; 10+ messages in thread
From: elaine.zhang @ 2019-04-29  9:51 UTC (permalink / raw)
  To: Daniel Lezcano, heiko
  Cc: rui.zhang, edubezval, robh+dt, mark.rutland, linux-pm,
	devicetree, linux-arm-kernel, linux-rockchip, linux-kernel, xxx,
	xf, huangtao

hi,

在 2019/4/28 下午8:45, Daniel Lezcano 写道:
> On 25/04/2019 12:12, Elaine Zhang wrote:
>> Explicitly use the pinctrl to set/unset the right mode
>> instead of relying on the pinctrl init mode.
>> And it requires setting the tshut polarity before select pinctrl.
>>
>> When the temperature sensor mode is set to 0, it will automatically
>> reset the board via the Clock-Reset-Unit (CRU) if the over temperature
>> threshold is reached. However, when the pinctrl initializes, it does a
>> transition to "otp_out" which may lead the SoC restart all the time.
>>
>> "otp_out" IO may be connected to the RESET circuit on the hardware.
>> If the IO is in the wrong state, it will trigger RESET.
>> (similar to the effect of pressing the RESET button)
>> which will cause the soc to restart all the time.
>>
>> Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>
>> ---
>>   drivers/thermal/rockchip_thermal.c | 37 ++++++++++++++++++++++++++++++++++---
>>   1 file changed, 34 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/thermal/rockchip_thermal.c b/drivers/thermal/rockchip_thermal.c
>> index 9c7643d62ed7..03ff494f2864 100644
>> --- a/drivers/thermal/rockchip_thermal.c
>> +++ b/drivers/thermal/rockchip_thermal.c
>> @@ -172,6 +172,9 @@ struct rockchip_thermal_data {
>>   	int tshut_temp;
>>   	enum tshut_mode tshut_mode;
>>   	enum tshut_polarity tshut_polarity;
>> +	struct pinctrl *pinctrl;
>> +	struct pinctrl_state *gpio_state;
>> +	struct pinctrl_state *otp_state;
>>   };
>>   
>>   /**
>> @@ -1242,6 +1245,8 @@ static int rockchip_thermal_probe(struct platform_device *pdev)
>>   		return error;
>>   	}
>>   
>> +	thermal->chip->control(thermal->regs, false);
>> +
>>   	error = clk_prepare_enable(thermal->clk);
>>   	if (error) {
>>   		dev_err(&pdev->dev, "failed to enable converter clock: %d\n",
>> @@ -1267,6 +1272,31 @@ static int rockchip_thermal_probe(struct platform_device *pdev)
>>   	thermal->chip->initialize(thermal->grf, thermal->regs,
>>   				  thermal->tshut_polarity);
>>   
>> +	if (thermal->tshut_mode == TSHUT_MODE_GPIO) {
>> +		thermal->pinctrl = devm_pinctrl_get(&pdev->dev);
>> +		if (IS_ERR(thermal->pinctrl)) {
>> +			dev_err(&pdev->dev, "failed to find thermal pinctrl\n");
>> +			panic("panic_on_find thermal pinctrl...\n");
> I realize my comment was confusing. I was not saying to add a panic()
> call here but that the code was accessing a NULL pointer. Please remove
> the panic.
OK, I'll fixed it.
>
>> +			return PTR_ERR(thermal->pinctrl);
>> +		}
>> +
>> +		thermal->gpio_state = pinctrl_lookup_state(thermal->pinctrl,
>> +							   "gpio");
>> +		if (IS_ERR_OR_NULL(thermal->gpio_state)) {
>> +			dev_err(&pdev->dev, "failed to find thermal gpio state\n");
>> +			return -EINVAL;
>> +		}
>> +
>> +		thermal->otp_state = pinctrl_lookup_state(thermal->pinctrl,
>> +							  "otpout");
>> +		if (IS_ERR_OR_NULL(thermal->otp_state)) {
>> +			dev_err(&pdev->dev, "failed to find thermal otpout state\n");
>> +			return -EINVAL;
>> +		}
>> +
>> +		pinctrl_select_state(thermal->pinctrl, thermal->otp_state);
> I don't understand this portion of code. The test above says tshut_mode
> is TSHUT_MODE_GPIO. Why acting on thermal->otp_state then ?
>
>
> In my previous comment, I was suggesting the following:
>
> Two more fields instead of three:
>
> 	struct rockchip_thermal_data {
>    		int tshut_temp;
> 	  	enum tshut_mode tshut_mode;
>    		enum tshut_polarity tshut_polarity;
> 	 	struct pinctrl *pinctrl;
> 		struct pinctrl_state *pinctrl_state;
> 	};
>
> 	[ ... ]
>
> 	thermal->pinctrl = devm_pinctrl_get(&pdev->dev);
> 	if (IS_ERR(thermal->pinctrl)) {
> 		dev_err("...");
> 		return PTR_ERR(thermal->pinctrl);
> 	}
>
> 	thermal->pinctrl_state = pinctrl_lookup_state(thermal->pinctrl,
> 		thermal->tshut_mode == TSHUT_MODE_GPIO ? "gpio" :
> 							"otpout";
>
> 	if (IS_ERR_OR_NULL(thermal->pinctrl_state) {
> 		dev_err("...");
> 		return PTR_ERR(thermal->pinctrl_state);
> 	}
>
>   	pinctrl_select_state(thermal->pinctrl, thermal->pinctrl_state);
>
>
> 	[ ... ]
>
> Is it wrong ?
>
>
>> +	}
>> +
>>   	for (i = 0; i < thermal->chip->chn_num; i++) {
>>   		error = rockchip_thermal_register_sensor(pdev, thermal,
>>   						&thermal->sensors[i],
>> @@ -1337,8 +1367,8 @@ static int __maybe_unused rockchip_thermal_suspend(struct device *dev)
>>   
>>   	clk_disable(thermal->pclk);
>>   	clk_disable(thermal->clk);
>> -
>> -	pinctrl_pm_select_sleep_state(dev);
>> +	if (thermal->tshut_mode == TSHUT_MODE_GPIO)
>> +		pinctrl_select_state(thermal->pinctrl, thermal->gpio_state);
> And then:
> 	 pinctrl_select_state(thermal->pinctrl, thermal->pinctrl_state);

pinctrl select to gpio mode when tsadc suspend and shutdown.

When suspend, tsadc is disabled, the otp_pin should revert to the 
default gpio state.

>
>>   
>>   	return 0;
>>   }
>> @@ -1383,7 +1413,8 @@ static int __maybe_unused rockchip_thermal_resume(struct device *dev)
>>   	for (i = 0; i < thermal->chip->chn_num; i++)
>>   		rockchip_thermal_toggle_sensor(&thermal->sensors[i], true);
>>   
>> -	pinctrl_pm_select_default_state(dev);
>> +	if (thermal->tshut_mode == TSHUT_MODE_GPIO)
>> +		pinctrl_select_state(thermal->pinctrl, thermal->otp_state);
> And then
> 	pinctrl_select_state(thermal->pinctrl, thermal->pinctrl_state);

pinctrl select to otp mode when tsadc resume.

>
>>   	return 0;
>>   }
>>
>



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/3] thermal: rockchip: fix up the tsadc pinctrl setting error
  2019-04-29  9:51     ` elaine.zhang
@ 2019-04-30  9:44       ` Daniel Lezcano
  0 siblings, 0 replies; 10+ messages in thread
From: Daniel Lezcano @ 2019-04-30  9:44 UTC (permalink / raw)
  To: elaine.zhang, heiko
  Cc: rui.zhang, edubezval, robh+dt, mark.rutland, linux-pm,
	devicetree, linux-arm-kernel, linux-rockchip, linux-kernel, xxx,
	xf, huangtao

On 29/04/2019 11:51, elaine.zhang wrote:

[ ... ]

> pinctrl select to gpio mode when tsadc suspend and shutdown.
> 
> When suspend, tsadc is disabled, the otp_pin should revert to the
> default gpio state.
> 
>>
>>>         return 0;
>>>   }
>>> @@ -1383,7 +1413,8 @@ static int __maybe_unused
>>> rockchip_thermal_resume(struct device *dev)
>>>       for (i = 0; i < thermal->chip->chn_num; i++)
>>>           rockchip_thermal_toggle_sensor(&thermal->sensors[i], true);
>>>   -    pinctrl_pm_select_default_state(dev);
>>> +    if (thermal->tshut_mode == TSHUT_MODE_GPIO)
>>> +        pinctrl_select_state(thermal->pinctrl, thermal->otp_state);
>> And then
>>     pinctrl_select_state(thermal->pinctrl, thermal->pinctrl_state);
> 
> pinctrl select to otp mode when tsadc resume.

Ok, thanks for clarifying.

  -- Daniel



-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 0/4] clk: rockchip: support clock controller for px30 SoC
  2018-06-15  2:16 Elaine Zhang
@ 2018-07-04 10:08 ` Heiko Stuebner
  0 siblings, 0 replies; 10+ messages in thread
From: Heiko Stuebner @ 2018-07-04 10:08 UTC (permalink / raw)
  To: Elaine Zhang
  Cc: sboyd, robh+dt, mark.rutland, mturquette, linux-clk, devicetree,
	linux-arm-kernel, linux-rockchip, linux-kernel, cl, xxx, xf,
	huangtao

Hi Elaine,

Am Freitag, 15. Juni 2018, 04:16:47 CEST schrieb Elaine Zhang:
> Change in V3:
> [PATCH v3 1/4]: Correct description
> [PATCH v3 2/4]: Use an SPDX tag instead.
> [PATCH v3 3/4]: Use an SPDX tag instead,
> 		parent_rate might overflow and fix it.
> 		fix up the checkpatch warning.
> 		add more CMPOSITE_xxx_HALFdiv.
> 
> Change in V2:
> [PATCH v2 2/4]: modify the Author name
> [PATCH v2 3/4]: provide a bit more explanation for commit message
> 
> Elaine Zhang (4):
>   dt-bindings: add bindings for px30 clock controller
>   clk: rockchip: add dt-binding header for px30
>   clk: rockchip: add support for half divider
>   clk: rockchip: add clock controller for px30

I've applied the set for 4.19 but dropped the panic-notifier
dump function from the px30 clock driver.

Heiko



^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v3 0/4] clk: rockchip: support clock controller for px30 SoC
@ 2018-06-15  2:16 Elaine Zhang
  2018-07-04 10:08 ` Heiko Stuebner
  0 siblings, 1 reply; 10+ messages in thread
From: Elaine Zhang @ 2018-06-15  2:16 UTC (permalink / raw)
  To: heiko
  Cc: sboyd, robh+dt, mark.rutland, mturquette, linux-clk, devicetree,
	linux-arm-kernel, linux-rockchip, linux-kernel, cl, xxx, xf,
	huangtao, Elaine Zhang

Change in V3:
[PATCH v3 1/4]: Correct description
[PATCH v3 2/4]: Use an SPDX tag instead.
[PATCH v3 3/4]: Use an SPDX tag instead,
		parent_rate might overflow and fix it.
		fix up the checkpatch warning.
		add more CMPOSITE_xxx_HALFdiv.

Change in V2:
[PATCH v2 2/4]: modify the Author name
[PATCH v2 3/4]: provide a bit more explanation for commit message

Elaine Zhang (4):
  dt-bindings: add bindings for px30 clock controller
  clk: rockchip: add dt-binding header for px30
  clk: rockchip: add support for half divider
  clk: rockchip: add clock controller for px30

 .../bindings/clock/rockchip,px30-cru.txt           |   66 ++
 drivers/clk/rockchip/Makefile                      |    2 +
 drivers/clk/rockchip/clk-half-divider.c            |  231 +++++
 drivers/clk/rockchip/clk-px30.c                    | 1080 ++++++++++++++++++++
 drivers/clk/rockchip/clk.c                         |   10 +
 drivers/clk/rockchip/clk.h                         |  126 ++-
 include/dt-bindings/clock/px30-cru.h               |  389 +++++++
 7 files changed, 1903 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/clock/rockchip,px30-cru.txt
 create mode 100644 drivers/clk/rockchip/clk-half-divider.c
 create mode 100644 drivers/clk/rockchip/clk-px30.c
 create mode 100644 include/dt-bindings/clock/px30-cru.h

-- 
1.9.1



^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2019-04-30  9:44 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-25 10:12 [PATCH v2 0/3] thermal: rockchip: fix up thermal driver Elaine Zhang
2019-04-25 10:12 ` [PATCH v2 1/3] thermal: rockchip: fix up the tsadc pinctrl setting error Elaine Zhang
2019-04-28 12:45   ` Daniel Lezcano
2019-04-29  9:51     ` elaine.zhang
2019-04-30  9:44       ` Daniel Lezcano
2019-04-25 10:12 ` [PATCH v2 2/3] dt-bindings: rockchip-thermal: Support the PX30 SoC compatible Elaine Zhang
2019-04-25 10:12 ` [PATCH v2 3/3] thermal: rockchip: Support the PX30 SoC in thermal driver Elaine Zhang
2019-04-25 10:12 ` [PATCH v3 0/4] clk: rockchip: support clock controller for px30 SoC Elaine Zhang
  -- strict thread matches above, loose matches on Subject: below --
2018-06-15  2:16 Elaine Zhang
2018-07-04 10:08 ` Heiko Stuebner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).