[PATCH 1/2] dt-bindings: rockchip-thermal: Support the RK3568 SoC compatible

[PATCH 1/2] dt-bindings: rockchip-thermal: Support the RK3568 SoC compatible

[Ezequiel Garcia]

Add a new compatible for the thermal sensor device on RK3568 SoCs.

Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
 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 7f94669e9ebe..346e466c2006 100644
--- a/Documentation/devicetree/bindings/thermal/rockchip-thermal.txt
+++ b/Documentation/devicetree/bindings/thermal/rockchip-thermal.txt
@@ -9,6 +9,7 @@ Required properties:
    "rockchip,rk3328-tsadc": found on RK3328 SoCs
    "rockchip,rk3368-tsadc": found on RK3368 SoCs
    "rockchip,rk3399-tsadc": found on RK3399 SoCs
+   "rockchip,rk3568-tsadc": found on RK3568 SoCs
 - reg : physical base address of the controller and length of memory mapped
 	region.
 - interrupts : The interrupt number to the cpu. The interrupt specifier format
-- 
2.30.0

[PATCH 2/2] thermal/rockchip: Support RK3568 SoCs in the thermal driver

[Ezequiel Garcia]

From: Finley Xiao <finley.xiao@rock-chips.com>

The RK3568 SoCs have two Temperature Sensors, channel 0 is for CPU,
channel 1 is for GPU.

Signed-off-by: Finley Xiao <finley.xiao@rock-chips.com>
Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
 drivers/thermal/rockchip_thermal.c | 107 +++++++++++++++++++++++++++++
 1 file changed, 107 insertions(+)

diff --git a/drivers/thermal/rockchip_thermal.c b/drivers/thermal/rockchip_thermal.c
index aa9e0e31ef98..8e26f2685003 100644
--- a/drivers/thermal/rockchip_thermal.c
+++ b/drivers/thermal/rockchip_thermal.c
@@ -211,7 +211,11 @@ struct rockchip_thermal_data {
 #define TSADCV3_AUTO_PERIOD_TIME		1875 /* 2.5ms */
 #define TSADCV3_AUTO_PERIOD_HT_TIME		1875 /* 2.5ms */
 
+#define TSADCV5_AUTO_PERIOD_TIME		1622 /* 2.5ms */
+#define TSADCV5_AUTO_PERIOD_HT_TIME		1622 /* 2.5ms */
+
 #define TSADCV2_USER_INTER_PD_SOC		0x340 /* 13 clocks */
+#define TSADCV5_USER_INTER_PD_SOC		0xfc0 /* 97us, at least 90us */
 
 #define GRF_SARADC_TESTBIT			0x0e644
 #define GRF_TSADC_TESTBIT_L			0x0e648
@@ -219,6 +223,12 @@ struct rockchip_thermal_data {
 
 #define PX30_GRF_SOC_CON2			0x0408
 
+#define RK3568_GRF_TSADC_CON			0x0600
+#define RK3568_GRF_TSADC_ANA_REG0		(0x10001 << 0)
+#define RK3568_GRF_TSADC_ANA_REG1		(0x10001 << 1)
+#define RK3568_GRF_TSADC_ANA_REG2		(0x10001 << 2)
+#define RK3568_GRF_TSADC_TSEN			(0x10001 << 8)
+
 #define GRF_SARADC_TESTBIT_ON			(0x10001 << 2)
 #define GRF_TSADC_TESTBIT_H_ON			(0x10001 << 2)
 #define GRF_TSADC_VCM_EN_L			(0x10001 << 7)
@@ -474,6 +484,45 @@ static const struct tsadc_table rk3399_code_table[] = {
 	{TSADCV3_DATA_MASK, 125000},
 };
 
+static const struct tsadc_table rk3568_code_table[] = {
+	{0, -40000},
+	{1584, -40000},
+	{1620, -35000},
+	{1652, -30000},
+	{1688, -25000},
+	{1720, -20000},
+	{1756, -15000},
+	{1788, -10000},
+	{1824, -5000},
+	{1856, 0},
+	{1892, 5000},
+	{1924, 10000},
+	{1956, 15000},
+	{1992, 20000},
+	{2024, 25000},
+	{2060, 30000},
+	{2092, 35000},
+	{2128, 40000},
+	{2160, 45000},
+	{2196, 50000},
+	{2228, 55000},
+	{2264, 60000},
+	{2300, 65000},
+	{2332, 70000},
+	{2368, 75000},
+	{2400, 80000},
+	{2436, 85000},
+	{2468, 90000},
+	{2500, 95000},
+	{2536, 100000},
+	{2572, 105000},
+	{2604, 110000},
+	{2636, 115000},
+	{2672, 120000},
+	{2704, 125000},
+	{TSADCV2_DATA_MASK, 125000},
+};
+
 static u32 rk_tsadcv2_temp_to_code(const struct chip_tsadc_table *table,
 				   int temp)
 {
@@ -701,6 +750,35 @@ static void rk_tsadcv4_initialize(struct regmap *grf, void __iomem *regs,
 	regmap_write(grf, PX30_GRF_SOC_CON2, GRF_CON_TSADC_CH_INV);
 }
 
+static void rk_tsadcv7_initialize(struct regmap *grf, void __iomem *regs,
+				  enum tshut_polarity tshut_polarity)
+{
+	writel_relaxed(TSADCV5_USER_INTER_PD_SOC, regs + TSADCV2_USER_CON);
+	writel_relaxed(TSADCV5_AUTO_PERIOD_TIME, regs + TSADCV2_AUTO_PERIOD);
+	writel_relaxed(TSADCV2_HIGHT_INT_DEBOUNCE_COUNT,
+		       regs + TSADCV2_HIGHT_INT_DEBOUNCE);
+	writel_relaxed(TSADCV5_AUTO_PERIOD_HT_TIME,
+		       regs + TSADCV2_AUTO_PERIOD_HT);
+	writel_relaxed(TSADCV2_HIGHT_TSHUT_DEBOUNCE_COUNT,
+		       regs + TSADCV2_HIGHT_TSHUT_DEBOUNCE);
+
+	if (tshut_polarity == TSHUT_HIGH_ACTIVE)
+		writel_relaxed(0U | TSADCV2_AUTO_TSHUT_POLARITY_HIGH,
+			       regs + TSADCV2_AUTO_CON);
+	else
+		writel_relaxed(0U & ~TSADCV2_AUTO_TSHUT_POLARITY_HIGH,
+			       regs + TSADCV2_AUTO_CON);
+
+	if (!IS_ERR(grf)) {
+		regmap_write(grf, RK3568_GRF_TSADC_CON, RK3568_GRF_TSADC_TSEN);
+		udelay(15);
+		regmap_write(grf, RK3568_GRF_TSADC_CON, RK3568_GRF_TSADC_ANA_REG0);
+		regmap_write(grf, RK3568_GRF_TSADC_CON, RK3568_GRF_TSADC_ANA_REG1);
+		regmap_write(grf, RK3568_GRF_TSADC_CON, RK3568_GRF_TSADC_ANA_REG2);
+		usleep_range(100, 200);
+	}
+}
+
 static void rk_tsadcv2_irq_ack(void __iomem *regs)
 {
 	u32 val;
@@ -1027,6 +1105,31 @@ static const struct rockchip_tsadc_chip rk3399_tsadc_data = {
 	},
 };
 
+static const struct rockchip_tsadc_chip rk3568_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, /* two channels for tsadc */
+
+	.tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
+	.tshut_polarity = TSHUT_LOW_ACTIVE, /* default TSHUT LOW ACTIVE */
+	.tshut_temp = 95000,
+
+	.initialize = rk_tsadcv7_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 = rk3568_code_table,
+		.length = ARRAY_SIZE(rk3568_code_table),
+		.data_mask = TSADCV2_DATA_MASK,
+		.mode = ADC_INCREMENT,
+	},
+};
+
 static const struct of_device_id of_rockchip_thermal_match[] = {
 	{	.compatible = "rockchip,px30-tsadc",
 		.data = (void *)&px30_tsadc_data,
@@ -1059,6 +1162,10 @@ static const struct of_device_id of_rockchip_thermal_match[] = {
 		.compatible = "rockchip,rk3399-tsadc",
 		.data = (void *)&rk3399_tsadc_data,
 	},
+	{
+		.compatible = "rockchip,rk3568-tsadc",
+		.data = (void *)&rk3568_tsadc_data,
+	},
 	{ /* end */ },
 };
 MODULE_DEVICE_TABLE(of, of_rockchip_thermal_match);
-- 
2.30.0

Re: [PATCH 1/2] dt-bindings: rockchip-thermal: Support the RK3568 SoC compatible

[Johan Jonker]

On 4/21/21 10:04 PM, Ezequiel Garcia wrote:
> Add a new compatible for the thermal sensor device on RK3568 SoCs.
> 
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> ---
>  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 7f94669e9ebe..346e466c2006 100644
> --- a/Documentation/devicetree/bindings/thermal/rockchip-thermal.txt
> +++ b/Documentation/devicetree/bindings/thermal/rockchip-thermal.txt
> @@ -9,6 +9,7 @@ Required properties:
>     "rockchip,rk3328-tsadc": found on RK3328 SoCs
>     "rockchip,rk3368-tsadc": found on RK3368 SoCs
>     "rockchip,rk3399-tsadc": found on RK3399 SoCs

> +   "rockchip,rk3568-tsadc": found on RK3568 SoCs

This is still a text document.
rob+dt has now scripts that check for undocumented compatibility
strings, so first convert rockchip-thermal.txt to YAML and then add this
in a separated patch.

Johan

>  - reg : physical base address of the controller and length of memory mapped
>  	region.
>  - interrupts : The interrupt number to the cpu. The interrupt specifier format
> 

Re: [PATCH 1/2] dt-bindings: rockchip-thermal: Support the RK3568 SoC compatible

[Ezequiel Garcia]

On Wed, 2021-04-21 at 22:46 +0200, Johan Jonker wrote:
> On 4/21/21 10:04 PM, Ezequiel Garcia wrote:
> > Add a new compatible for the thermal sensor device on RK3568 SoCs.
> > 
> > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> > ---
> >  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 7f94669e9ebe..346e466c2006 100644
> > --- a/Documentation/devicetree/bindings/thermal/rockchip-thermal.txt
> > +++ b/Documentation/devicetree/bindings/thermal/rockchip-thermal.txt
> > @@ -9,6 +9,7 @@ Required properties:
> >     "rockchip,rk3328-tsadc": found on RK3328 SoCs
> >     "rockchip,rk3368-tsadc": found on RK3368 SoCs
> >     "rockchip,rk3399-tsadc": found on RK3399 SoCs
> 
> > +   "rockchip,rk3568-tsadc": found on RK3568 SoCs
> 
> This is still a text document.
> rob+dt has now scripts that check for undocumented compatibility
> strings, so first convert rockchip-thermal.txt to YAML and then add this
> in a separated patch.
> 

Is it a showstopper to convert devicetree bindings to YAML for driver submission?

Thanks,
Ezequiel

Re: [PATCH 1/2] dt-bindings: rockchip-thermal: Support the RK3568 SoC compatible

[Johan Jonker]

On 4/21/21 11:06 PM, Ezequiel Garcia wrote:
> On Wed, 2021-04-21 at 22:46 +0200, Johan Jonker wrote:
>> On 4/21/21 10:04 PM, Ezequiel Garcia wrote:
>>> Add a new compatible for the thermal sensor device on RK3568 SoCs.
>>>
>>> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
>>> ---
>>>  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 7f94669e9ebe..346e466c2006 100644
>>> --- a/Documentation/devicetree/bindings/thermal/rockchip-thermal.txt
>>> +++ b/Documentation/devicetree/bindings/thermal/rockchip-thermal.txt
>>> @@ -9,6 +9,7 @@ Required properties:
>>>     "rockchip,rk3328-tsadc": found on RK3328 SoCs
>>>     "rockchip,rk3368-tsadc": found on RK3368 SoCs
>>>     "rockchip,rk3399-tsadc": found on RK3399 SoCs
>>
>>> +   "rockchip,rk3568-tsadc": found on RK3568 SoCs
>>
>> This is still a text document.
>> rob+dt has now scripts that check for undocumented compatibility
>> strings, so first convert rockchip-thermal.txt to YAML and then add this
>> in a separated patch.
>>
> 
> Is it a showstopper to convert devicetree bindings to YAML for driver submission?

You now that hardware best, so try to fix the documents as well.
The new norm is YAML, so aim for that.
Try to submit a complete package of YAML, driver (and dts nodes) for review.
New nodes should not generate more notifications then we already have.

Johan
> 
> Thanks,
> Ezequiel
> 

Re: [PATCH 1/2] dt-bindings: rockchip-thermal: Support the RK3568 SoC compatible

[Ezequiel Garcia]

On Wed, 2021-04-21 at 23:25 +0200, Johan Jonker wrote:
> On 4/21/21 11:06 PM, Ezequiel Garcia wrote:
> > On Wed, 2021-04-21 at 22:46 +0200, Johan Jonker wrote:
> > > On 4/21/21 10:04 PM, Ezequiel Garcia wrote:
> > > > Add a new compatible for the thermal sensor device on RK3568 SoCs.
> > > > 
> > > > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> > > > ---
> > > >  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 7f94669e9ebe..346e466c2006 100644
> > > > --- a/Documentation/devicetree/bindings/thermal/rockchip-thermal.txt
> > > > +++ b/Documentation/devicetree/bindings/thermal/rockchip-thermal.txt
> > > > @@ -9,6 +9,7 @@ Required properties:
> > > >     "rockchip,rk3328-tsadc": found on RK3328 SoCs
> > > >     "rockchip,rk3368-tsadc": found on RK3368 SoCs
> > > >     "rockchip,rk3399-tsadc": found on RK3399 SoCs
> > > 
> > > > +   "rockchip,rk3568-tsadc": found on RK3568 SoCs
> > > 
> > > This is still a text document.
> > > rob+dt has now scripts that check for undocumented compatibility
> > > strings, so first convert rockchip-thermal.txt to YAML and then add this
> > > in a separated patch.
> > > 
> > 
> > Is it a showstopper to convert devicetree bindings to YAML for driver submission?
> 
> You now that hardware best, so try to fix the documents as well.

Well, not really. I'm just forward porting the driver from downstream kernels,
so we can support this new SoC. Not really a hardware _expert_ for all the
devices I plan to be pushing.

> The new norm is YAML, so aim for that.

I am aware of that. In fact, at Collabora we encourage all the kernel
developers to convert to YAML, if/when possible.

> Try to submit a complete package of YAML, driver (and dts nodes) for review.

The devicetree for RK3566 and RK3568 is under discussion, in fact it was submitted today.
Rockhip is leading that, and doing a great job already :)

Meanwhile, I'd like to merge the small drivers (thermal, pmic, dwmac, io-domains and so on),
so they are ready when the devicetree lands.

Most if not all of these devices just need a new compatible string. It would really delay
things if I aim to convert all those bindings docs to YAML first, so let's please avoid that...
... unless it's a new hard-rule that DT maintainers have agreed on.

Having said that, if you want to help the RK3568 bringup adventure,
converting the bindings to YAML, that would be really appreciated! Of course, I know
you've converted plenty of them already, and I already appreciate that :)

Thanks,
Ezequiel

Re: [PATCH 1/2] dt-bindings: rockchip-thermal: Support the RK3568 SoC compatible

[Johan Jonker]

On 4/21/21 11:56 PM, Ezequiel Garcia wrote:
> On Wed, 2021-04-21 at 23:25 +0200, Johan Jonker wrote:
>> On 4/21/21 11:06 PM, Ezequiel Garcia wrote:
>>> On Wed, 2021-04-21 at 22:46 +0200, Johan Jonker wrote:
>>>> On 4/21/21 10:04 PM, Ezequiel Garcia wrote:
>>>>> Add a new compatible for the thermal sensor device on RK3568 SoCs.
>>>>>
>>>>> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
>>>>> ---
>>>>>  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 7f94669e9ebe..346e466c2006 100644
>>>>> --- a/Documentation/devicetree/bindings/thermal/rockchip-thermal.txt
>>>>> +++ b/Documentation/devicetree/bindings/thermal/rockchip-thermal.txt
>>>>> @@ -9,6 +9,7 @@ Required properties:
>>>>>     "rockchip,rk3328-tsadc": found on RK3328 SoCs
>>>>>     "rockchip,rk3368-tsadc": found on RK3368 SoCs
>>>>>     "rockchip,rk3399-tsadc": found on RK3399 SoCs
>>>>
>>>>> +   "rockchip,rk3568-tsadc": found on RK3568 SoCs
>>>>
>>>> This is still a text document.
>>>> rob+dt has now scripts that check for undocumented compatibility
>>>> strings, so first convert rockchip-thermal.txt to YAML and then add this
>>>> in a separated patch.
>>>>
>>>
>>> Is it a showstopper to convert devicetree bindings to YAML for driver submission?
>>
>> You now that hardware best, so try to fix the documents as well.
> 
> Well, not really. I'm just forward porting the driver from downstream kernels,
> so we can support this new SoC. Not really a hardware _expert_ for all the
> devices I plan to be pushing.
> 
>> The new norm is YAML, so aim for that.
> 
> I am aware of that. In fact, at Collabora we encourage all the kernel
> developers to convert to YAML, if/when possible.
> 
>> Try to submit a complete package of YAML, driver (and dts nodes) for review.
> 
> The devicetree for RK3566 and RK3568 is under discussion, in fact it was submitted today.
> Rockhip is leading that, and doing a great job already :)
> 
> Meanwhile, I'd like to merge the small drivers (thermal, pmic, dwmac, io-domains and so on),
> so they are ready when the devicetree lands.
> 

> Most if not all of these devices just need a new compatible string. It would really delay
> things if I aim to convert all those bindings docs to YAML first, so let's please avoid that...
> ... unless it's a new hard-rule that DT maintainers have agreed on.

Every driver group has it's own delay time, so better do it right in one
run.
Mostly people tend to 'forget' documentation and then someone else has
to clean up the mess. So I propose that the person that submits a new
driver also fixes the documentation. The norm is now YAML, so this serie
has more work then other, so be it. Others can help you with it if you ask.

Johan

> 
> Having said that, if you want to help the RK3568 bringup adventure,
> converting the bindings to YAML, that would be really appreciated! Of course, I know
> you've converted plenty of them already, and I already appreciate that :)
> 
> Thanks,
> Ezequiel
> 

Re: [PATCH 1/2] dt-bindings: rockchip-thermal: Support the RK3568 SoC compatible

[Heiko Stübner]

Hi,

Am Donnerstag, 22. April 2021, 00:12:45 CEST schrieb Johan Jonker:
> On 4/21/21 11:56 PM, Ezequiel Garcia wrote:
> > On Wed, 2021-04-21 at 23:25 +0200, Johan Jonker wrote:
> >> On 4/21/21 11:06 PM, Ezequiel Garcia wrote:
> >>> On Wed, 2021-04-21 at 22:46 +0200, Johan Jonker wrote:
> >>>> On 4/21/21 10:04 PM, Ezequiel Garcia wrote:
> >>>>> Add a new compatible for the thermal sensor device on RK3568 SoCs.
> >>>>>
> >>>>> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> >>>>> ---
> >>>>>  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 7f94669e9ebe..346e466c2006 100644
> >>>>> --- a/Documentation/devicetree/bindings/thermal/rockchip-thermal.txt
> >>>>> +++ b/Documentation/devicetree/bindings/thermal/rockchip-thermal.txt
> >>>>> @@ -9,6 +9,7 @@ Required properties:
> >>>>>     "rockchip,rk3328-tsadc": found on RK3328 SoCs
> >>>>>     "rockchip,rk3368-tsadc": found on RK3368 SoCs
> >>>>>     "rockchip,rk3399-tsadc": found on RK3399 SoCs
> >>>>
> >>>>> +   "rockchip,rk3568-tsadc": found on RK3568 SoCs
> >>>>
> >>>> This is still a text document.
> >>>> rob+dt has now scripts that check for undocumented compatibility
> >>>> strings, so first convert rockchip-thermal.txt to YAML and then add this
> >>>> in a separated patch.
> >>>>
> >>>
> >>> Is it a showstopper to convert devicetree bindings to YAML for driver submission?
> >>
> >> You now that hardware best, so try to fix the documents as well.
> > 
> > Well, not really. I'm just forward porting the driver from downstream kernels,
> > so we can support this new SoC. Not really a hardware _expert_ for all the
> > devices I plan to be pushing.
> > 
> >> The new norm is YAML, so aim for that.
> > 
> > I am aware of that. In fact, at Collabora we encourage all the kernel
> > developers to convert to YAML, if/when possible.
> > 
> >> Try to submit a complete package of YAML, driver (and dts nodes) for review.
> > 
> > The devicetree for RK3566 and RK3568 is under discussion, in fact it was submitted today.
> > Rockhip is leading that, and doing a great job already :)
> > 
> > Meanwhile, I'd like to merge the small drivers (thermal, pmic, dwmac, io-domains and so on),
> > so they are ready when the devicetree lands.
> > 
> 
> > Most if not all of these devices just need a new compatible string. It would really delay
> > things if I aim to convert all those bindings docs to YAML first, so let's please avoid that...
> > ... unless it's a new hard-rule that DT maintainers have agreed on.
> 
> Every driver group has it's own delay time, so better do it right in one
> run.
> Mostly people tend to 'forget' documentation and then someone else has
> to clean up the mess. So I propose that the person that submits a new
> driver also fixes the documentation. The norm is now YAML, so this serie
> has more work then other, so be it. Others can help you with it if you ask.

personally I feel this approach being a bit too strict.

While it is definitly cool to convert everything to a yaml base in a
hopefully short time, being overly strict can also stiffle participation.

This is especially true if a series only adds a single compatible to an
already existing binding. So depending on the time constraints of the
contributor they might very well refrain from submitting another version.

In then end though, it's Rob's decision on how strict this conversion
is to be taken.


Heiko

Re: [PATCH 1/2] dt-bindings: rockchip-thermal: Support the RK3568 SoC compatible

[Daniel Lezcano]

On 22/04/2021 20:20, Heiko Stübner wrote:
> Hi,
> 
> Am Donnerstag, 22. April 2021, 00:12:45 CEST schrieb Johan Jonker:
>> On 4/21/21 11:56 PM, Ezequiel Garcia wrote:
>>> On Wed, 2021-04-21 at 23:25 +0200, Johan Jonker wrote:
>>>> On 4/21/21 11:06 PM, Ezequiel Garcia wrote:
>>>>> On Wed, 2021-04-21 at 22:46 +0200, Johan Jonker wrote:
>>>>>> On 4/21/21 10:04 PM, Ezequiel Garcia wrote:
>>>>>>> Add a new compatible for the thermal sensor device on RK3568 SoCs.
>>>>>>>
>>>>>>> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
>>>>>>> ---
>>>>>>>  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 7f94669e9ebe..346e466c2006 100644
>>>>>>> --- a/Documentation/devicetree/bindings/thermal/rockchip-thermal.txt
>>>>>>> +++ b/Documentation/devicetree/bindings/thermal/rockchip-thermal.txt
>>>>>>> @@ -9,6 +9,7 @@ Required properties:
>>>>>>>     "rockchip,rk3328-tsadc": found on RK3328 SoCs
>>>>>>>     "rockchip,rk3368-tsadc": found on RK3368 SoCs
>>>>>>>     "rockchip,rk3399-tsadc": found on RK3399 SoCs
>>>>>>
>>>>>>> +   "rockchip,rk3568-tsadc": found on RK3568 SoCs
>>>>>>
>>>>>> This is still a text document.
>>>>>> rob+dt has now scripts that check for undocumented compatibility
>>>>>> strings, so first convert rockchip-thermal.txt to YAML and then add this
>>>>>> in a separated patch.
>>>>>>
>>>>>
>>>>> Is it a showstopper to convert devicetree bindings to YAML for driver submission?
>>>>
>>>> You now that hardware best, so try to fix the documents as well.
>>>
>>> Well, not really. I'm just forward porting the driver from downstream kernels,
>>> so we can support this new SoC. Not really a hardware _expert_ for all the
>>> devices I plan to be pushing.
>>>
>>>> The new norm is YAML, so aim for that.
>>>
>>> I am aware of that. In fact, at Collabora we encourage all the kernel
>>> developers to convert to YAML, if/when possible.
>>>
>>>> Try to submit a complete package of YAML, driver (and dts nodes) for review.
>>>
>>> The devicetree for RK3566 and RK3568 is under discussion, in fact it was submitted today.
>>> Rockhip is leading that, and doing a great job already :)
>>>
>>> Meanwhile, I'd like to merge the small drivers (thermal, pmic, dwmac, io-domains and so on),
>>> so they are ready when the devicetree lands.
>>>
>>
>>> Most if not all of these devices just need a new compatible string. It would really delay
>>> things if I aim to convert all those bindings docs to YAML first, so let's please avoid that...
>>> ... unless it's a new hard-rule that DT maintainers have agreed on.
>>
>> Every driver group has it's own delay time, so better do it right in one
>> run.
>> Mostly people tend to 'forget' documentation and then someone else has
>> to clean up the mess. So I propose that the person that submits a new
>> driver also fixes the documentation. The norm is now YAML, so this serie
>> has more work then other, so be it. Others can help you with it if you ask.
> 
> personally I feel this approach being a bit too strict.
> 
> While it is definitly cool to convert everything to a yaml base in a
> hopefully short time, being overly strict can also stiffle participation.

+1


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

Re: [PATCH 2/2] thermal/rockchip: Support RK3568 SoCs in the thermal driver

[Daniel Lezcano]

On 21/04/2021 22:04, Ezequiel Garcia wrote:
> From: Finley Xiao <finley.xiao@rock-chips.com>
> 
> The RK3568 SoCs have two Temperature Sensors, channel 0 is for CPU,
> channel 1 is for GPU.
> 
> Signed-off-by: Finley Xiao <finley.xiao@rock-chips.com>
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> ---
>  drivers/thermal/rockchip_thermal.c | 107 +++++++++++++++++++++++++++++
>  1 file changed, 107 insertions(+)
> 
> diff --git a/drivers/thermal/rockchip_thermal.c b/drivers/thermal/rockchip_thermal.c
> index aa9e0e31ef98..8e26f2685003 100644
> --- a/drivers/thermal/rockchip_thermal.c
> +++ b/drivers/thermal/rockchip_thermal.c
> @@ -211,7 +211,11 @@ struct rockchip_thermal_data {
>  #define TSADCV3_AUTO_PERIOD_TIME		1875 /* 2.5ms */
>  #define TSADCV3_AUTO_PERIOD_HT_TIME		1875 /* 2.5ms */
>  
> +#define TSADCV5_AUTO_PERIOD_TIME		1622 /* 2.5ms */
> +#define TSADCV5_AUTO_PERIOD_HT_TIME		1622 /* 2.5ms */
> +
>  #define TSADCV2_USER_INTER_PD_SOC		0x340 /* 13 clocks */
> +#define TSADCV5_USER_INTER_PD_SOC		0xfc0 /* 97us, at least 90us */
>  
>  #define GRF_SARADC_TESTBIT			0x0e644
>  #define GRF_TSADC_TESTBIT_L			0x0e648
> @@ -219,6 +223,12 @@ struct rockchip_thermal_data {
>  
>  #define PX30_GRF_SOC_CON2			0x0408
>  
> +#define RK3568_GRF_TSADC_CON			0x0600
> +#define RK3568_GRF_TSADC_ANA_REG0		(0x10001 << 0)
> +#define RK3568_GRF_TSADC_ANA_REG1		(0x10001 << 1)
> +#define RK3568_GRF_TSADC_ANA_REG2		(0x10001 << 2)
> +#define RK3568_GRF_TSADC_TSEN			(0x10001 << 8)
> +
>  #define GRF_SARADC_TESTBIT_ON			(0x10001 << 2)
>  #define GRF_TSADC_TESTBIT_H_ON			(0x10001 << 2)
>  #define GRF_TSADC_VCM_EN_L			(0x10001 << 7)
> @@ -474,6 +484,45 @@ static const struct tsadc_table rk3399_code_table[] = {
>  	{TSADCV3_DATA_MASK, 125000},
>  };
>  
> +static const struct tsadc_table rk3568_code_table[] = {
> +	{0, -40000},
> +	{1584, -40000},
> +	{1620, -35000},
> +	{1652, -30000},
> +	{1688, -25000},
> +	{1720, -20000},
> +	{1756, -15000},
> +	{1788, -10000},
> +	{1824, -5000},
> +	{1856, 0},
> +	{1892, 5000},
> +	{1924, 10000},
> +	{1956, 15000},
> +	{1992, 20000},
> +	{2024, 25000},
> +	{2060, 30000},
> +	{2092, 35000},
> +	{2128, 40000},
> +	{2160, 45000},
> +	{2196, 50000},
> +	{2228, 55000},
> +	{2264, 60000},
> +	{2300, 65000},
> +	{2332, 70000},
> +	{2368, 75000},
> +	{2400, 80000},
> +	{2436, 85000},
> +	{2468, 90000},
> +	{2500, 95000},
> +	{2536, 100000},
> +	{2572, 105000},
> +	{2604, 110000},
> +	{2636, 115000},
> +	{2672, 120000},
> +	{2704, 125000},
> +	{TSADCV2_DATA_MASK, 125000},
> +};
> +
>  static u32 rk_tsadcv2_temp_to_code(const struct chip_tsadc_table *table,
>  				   int temp)
>  {
> @@ -701,6 +750,35 @@ static void rk_tsadcv4_initialize(struct regmap *grf, void __iomem *regs,
>  	regmap_write(grf, PX30_GRF_SOC_CON2, GRF_CON_TSADC_CH_INV);
>  }
>  
> +static void rk_tsadcv7_initialize(struct regmap *grf, void __iomem *regs,
> +				  enum tshut_polarity tshut_polarity)
> +{
> +	writel_relaxed(TSADCV5_USER_INTER_PD_SOC, regs + TSADCV2_USER_CON);
> +	writel_relaxed(TSADCV5_AUTO_PERIOD_TIME, regs + TSADCV2_AUTO_PERIOD);
> +	writel_relaxed(TSADCV2_HIGHT_INT_DEBOUNCE_COUNT,
> +		       regs + TSADCV2_HIGHT_INT_DEBOUNCE);
> +	writel_relaxed(TSADCV5_AUTO_PERIOD_HT_TIME,
> +		       regs + TSADCV2_AUTO_PERIOD_HT);
> +	writel_relaxed(TSADCV2_HIGHT_TSHUT_DEBOUNCE_COUNT,
> +		       regs + TSADCV2_HIGHT_TSHUT_DEBOUNCE);
> +
> +	if (tshut_polarity == TSHUT_HIGH_ACTIVE)
> +		writel_relaxed(0U | TSADCV2_AUTO_TSHUT_POLARITY_HIGH,
> +			       regs + TSADCV2_AUTO_CON);
> +	else
> +		writel_relaxed(0U & ~TSADCV2_AUTO_TSHUT_POLARITY_HIGH,
> +			       regs + TSADCV2_AUTO_CON);
> +
> +	if (!IS_ERR(grf)) {

That is strange to do this check with a parameter. Is the sensor
functional if the regmap failed ?

> +		regmap_write(grf, RK3568_GRF_TSADC_CON, RK3568_GRF_TSADC_TSEN);
> +		udelay(15);
> +		regmap_write(grf, RK3568_GRF_TSADC_CON, RK3568_GRF_TSADC_ANA_REG0);
> +		regmap_write(grf, RK3568_GRF_TSADC_CON, RK3568_GRF_TSADC_ANA_REG1);
> +		regmap_write(grf, RK3568_GRF_TSADC_CON, RK3568_GRF_TSADC_ANA_REG2);
> +		usleep_range(100, 200);

Is it possible to put a comment on why these delays are necessary (if
you have the info) ?

> +	}
> +}
> +
>  static void rk_tsadcv2_irq_ack(void __iomem *regs)
>  {
>  	u32 val;
> @@ -1027,6 +1105,31 @@ static const struct rockchip_tsadc_chip rk3399_tsadc_data = {
>  	},
>  };
>  
> +static const struct rockchip_tsadc_chip rk3568_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, /* two channels for tsadc */
> +
> +	.tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
> +	.tshut_polarity = TSHUT_LOW_ACTIVE, /* default TSHUT LOW ACTIVE */
> +	.tshut_temp = 95000,
> +
> +	.initialize = rk_tsadcv7_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 = rk3568_code_table,
> +		.length = ARRAY_SIZE(rk3568_code_table),
> +		.data_mask = TSADCV2_DATA_MASK,
> +		.mode = ADC_INCREMENT,
> +	},
> +};
> +
>  static const struct of_device_id of_rockchip_thermal_match[] = {
>  	{	.compatible = "rockchip,px30-tsadc",
>  		.data = (void *)&px30_tsadc_data,
> @@ -1059,6 +1162,10 @@ static const struct of_device_id of_rockchip_thermal_match[] = {
>  		.compatible = "rockchip,rk3399-tsadc",
>  		.data = (void *)&rk3399_tsadc_data,
>  	},
> +	{
> +		.compatible = "rockchip,rk3568-tsadc",
> +		.data = (void *)&rk3568_tsadc_data,
> +	},
>  	{ /* end */ },
>  };
>  MODULE_DEVICE_TABLE(of, of_rockchip_thermal_match);
> 


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

Re: [PATCH 2/2] thermal/rockchip: Support RK3568 SoCs in the thermal driver

[Ezequiel Garcia]

Hi Daniel,

Thanks for the review.

On Sat, 2021-04-24 at 16:14 +0200, Daniel Lezcano wrote:
> On 21/04/2021 22:04, Ezequiel Garcia wrote:
> > From: Finley Xiao <finley.xiao@rock-chips.com>
> > 
> > The RK3568 SoCs have two Temperature Sensors, channel 0 is for CPU,
> > channel 1 is for GPU.
> > 
> > Signed-off-by: Finley Xiao <finley.xiao@rock-chips.com>
> > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> > ---
> >  drivers/thermal/rockchip_thermal.c | 107 +++++++++++++++++++++++++++++
> >  1 file changed, 107 insertions(+)
> > 
> > diff --git a/drivers/thermal/rockchip_thermal.c b/drivers/thermal/rockchip_thermal.c
> > index aa9e0e31ef98..8e26f2685003 100644
> > --- a/drivers/thermal/rockchip_thermal.c
> > +++ b/drivers/thermal/rockchip_thermal.c
> > @@ -211,7 +211,11 @@ struct rockchip_thermal_data {
> >  #define TSADCV3_AUTO_PERIOD_TIME               1875 /* 2.5ms */
> >  #define TSADCV3_AUTO_PERIOD_HT_TIME            1875 /* 2.5ms */
> >  
> > +#define TSADCV5_AUTO_PERIOD_TIME               1622 /* 2.5ms */
> > +#define TSADCV5_AUTO_PERIOD_HT_TIME            1622 /* 2.5ms */
> > +
> >  #define TSADCV2_USER_INTER_PD_SOC              0x340 /* 13 clocks */
> > +#define TSADCV5_USER_INTER_PD_SOC              0xfc0 /* 97us, at least 90us */
> >  
> >  #define GRF_SARADC_TESTBIT                     0x0e644
> >  #define GRF_TSADC_TESTBIT_L                    0x0e648
> > @@ -219,6 +223,12 @@ struct rockchip_thermal_data {
> >  
> >  #define PX30_GRF_SOC_CON2                      0x0408
> >  
> > +#define RK3568_GRF_TSADC_CON                   0x0600
> > +#define RK3568_GRF_TSADC_ANA_REG0              (0x10001 << 0)
> > +#define RK3568_GRF_TSADC_ANA_REG1              (0x10001 << 1)
> > +#define RK3568_GRF_TSADC_ANA_REG2              (0x10001 << 2)
> > +#define RK3568_GRF_TSADC_TSEN                  (0x10001 << 8)
> > +
> >  #define GRF_SARADC_TESTBIT_ON                  (0x10001 << 2)
> >  #define GRF_TSADC_TESTBIT_H_ON                 (0x10001 << 2)
> >  #define GRF_TSADC_VCM_EN_L                     (0x10001 << 7)
> > @@ -474,6 +484,45 @@ static const struct tsadc_table rk3399_code_table[] = {
> >         {TSADCV3_DATA_MASK, 125000},
> >  };
> >  
> > +static const struct tsadc_table rk3568_code_table[] = {
> > +       {0, -40000},
> > +       {1584, -40000},
> > +       {1620, -35000},
> > +       {1652, -30000},
> > +       {1688, -25000},
> > +       {1720, -20000},
> > +       {1756, -15000},
> > +       {1788, -10000},
> > +       {1824, -5000},
> > +       {1856, 0},
> > +       {1892, 5000},
> > +       {1924, 10000},
> > +       {1956, 15000},
> > +       {1992, 20000},
> > +       {2024, 25000},
> > +       {2060, 30000},
> > +       {2092, 35000},
> > +       {2128, 40000},
> > +       {2160, 45000},
> > +       {2196, 50000},
> > +       {2228, 55000},
> > +       {2264, 60000},
> > +       {2300, 65000},
> > +       {2332, 70000},
> > +       {2368, 75000},
> > +       {2400, 80000},
> > +       {2436, 85000},
> > +       {2468, 90000},
> > +       {2500, 95000},
> > +       {2536, 100000},
> > +       {2572, 105000},
> > +       {2604, 110000},
> > +       {2636, 115000},
> > +       {2672, 120000},
> > +       {2704, 125000},
> > +       {TSADCV2_DATA_MASK, 125000},
> > +};
> > +
> >  static u32 rk_tsadcv2_temp_to_code(const struct chip_tsadc_table *table,
> >                                    int temp)
> >  {
> > @@ -701,6 +750,35 @@ static void rk_tsadcv4_initialize(struct regmap *grf, void __iomem *regs,
> >         regmap_write(grf, PX30_GRF_SOC_CON2, GRF_CON_TSADC_CH_INV);
> >  }
> >  
> > +static void rk_tsadcv7_initialize(struct regmap *grf, void __iomem *regs,
> > +                                 enum tshut_polarity tshut_polarity)
> > +{
> > +       writel_relaxed(TSADCV5_USER_INTER_PD_SOC, regs + TSADCV2_USER_CON);
> > +       writel_relaxed(TSADCV5_AUTO_PERIOD_TIME, regs + TSADCV2_AUTO_PERIOD);
> > +       writel_relaxed(TSADCV2_HIGHT_INT_DEBOUNCE_COUNT,
> > +                      regs + TSADCV2_HIGHT_INT_DEBOUNCE);
> > +       writel_relaxed(TSADCV5_AUTO_PERIOD_HT_TIME,
> > +                      regs + TSADCV2_AUTO_PERIOD_HT);
> > +       writel_relaxed(TSADCV2_HIGHT_TSHUT_DEBOUNCE_COUNT,
> > +                      regs + TSADCV2_HIGHT_TSHUT_DEBOUNCE);
> > +
> > +       if (tshut_polarity == TSHUT_HIGH_ACTIVE)
> > +               writel_relaxed(0U | TSADCV2_AUTO_TSHUT_POLARITY_HIGH,
> > +                              regs + TSADCV2_AUTO_CON);
> > +       else
> > +               writel_relaxed(0U & ~TSADCV2_AUTO_TSHUT_POLARITY_HIGH,
> > +                              regs + TSADCV2_AUTO_CON);
> > +
> > +       if (!IS_ERR(grf)) {
> 
> That is strange to do this check with a parameter. Is the sensor
> functional if the regmap failed ?
> 

Indeed, it seems it's optional. The same check is in rk_tsadcv3_initialize,
and also in rockchip_configure_from_dt():

        /* The tsadc wont to handle the error in here since some SoCs didn't     
         * need this property.                                                   
         */                                                                      
        thermal->grf = syscon_regmap_lookup_by_phandle(np, "rockchip,grf");      
        if (IS_ERR(thermal->grf))                                                
                dev_warn(dev, "Missing rockchip,grf property\n"); 

But I completely agree it looks strange without a comment,
might be better to add one.

> > +               regmap_write(grf, RK3568_GRF_TSADC_CON, RK3568_GRF_TSADC_TSEN);
> > +               udelay(15);
> > +               regmap_write(grf, RK3568_GRF_TSADC_CON, RK3568_GRF_TSADC_ANA_REG0);
> > +               regmap_write(grf, RK3568_GRF_TSADC_CON, RK3568_GRF_TSADC_ANA_REG1);
> > +               regmap_write(grf, RK3568_GRF_TSADC_CON, RK3568_GRF_TSADC_ANA_REG2);
> > +               usleep_range(100, 200);
> 
> Is it possible to put a comment on why these delays are necessary (if
> you have the info) ?
> 

Sure, I'll add a comment there. It is described in RK3568 Part1 TRM,
section 18.5.2.

Thanks,
Ezequiel

Re: [PATCH 1/2] dt-bindings: rockchip-thermal: Support the RK3568 SoC compatible

[Rob Herring]

On Wed, Apr 21, 2021 at 10:46:37PM +0200, Johan Jonker wrote:
> On 4/21/21 10:04 PM, Ezequiel Garcia wrote:
> > Add a new compatible for the thermal sensor device on RK3568 SoCs.
> > 
> > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> > ---
> >  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 7f94669e9ebe..346e466c2006 100644
> > --- a/Documentation/devicetree/bindings/thermal/rockchip-thermal.txt
> > +++ b/Documentation/devicetree/bindings/thermal/rockchip-thermal.txt
> > @@ -9,6 +9,7 @@ Required properties:
> >     "rockchip,rk3328-tsadc": found on RK3328 SoCs
> >     "rockchip,rk3368-tsadc": found on RK3368 SoCs
> >     "rockchip,rk3399-tsadc": found on RK3399 SoCs
> 
> > +   "rockchip,rk3568-tsadc": found on RK3568 SoCs
> 
> This is still a text document.
> rob+dt has now scripts that check for undocumented compatibility
> strings, so first convert rockchip-thermal.txt to YAML and then add this
> in a separated patch.

Thanks for pushing on this, but for now at least, simple compatible 
additions are fine. If changes would complicate the conversion to 
schema, then we should convert it first. For example, if the new 
compatible was accompanied with differing clocks, irqs, resets, etc., 
then we should convert (or consider a separate schema).

In parallel though, we do want to get platforms to 0 undocumented (by 
schema) compatibles. And then 0 warnings after that...

Rob

Re: [PATCH 1/2] dt-bindings: rockchip-thermal: Support the RK3568 SoC compatible

[Rob Herring]

On Wed, 21 Apr 2021 17:04:44 -0300, Ezequiel Garcia wrote:
> Add a new compatible for the thermal sensor device on RK3568 SoCs.
> 
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> ---
>  Documentation/devicetree/bindings/thermal/rockchip-thermal.txt | 1 +
>  1 file changed, 1 insertion(+)
> 

Acked-by: Rob Herring <robh@kernel.org>