dt-bindings: thermal/armada: describe AP806 and CP110

The AP806 component is integrated in the Armada 8k and 7k lines of processors.

Signed-off-by Baruch Siach <[email protected]>

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

drivers/thermal/armada_thermal.c +4 -0

index ae75328945f7..1f7f81628040 100644
@@ -41,6 +41,10 @@
 #define A375_HW_RESETn			BIT(8)
 #define A380_HW_RESET			BIT(8)
 
+#define AP806_START			BIT(0)
+#define AP806_RESET			BIT(1)
+#define AP806_ENABLE			BIT(2)
+
 struct armada_thermal_data;
 
 /* Marvell EBU Thermal Sensor Dev Structure */

@@ -147,6 +151,18 @@ static void armada380_init_sensor(struct platform_device *pdev,
 	}
 }
 

Quoted 26 lines · Baruch Siach

The AP806 component is integrated in the Armada 8k and 7k lines of processors.

Signed-off-by Baruch Siach <[email protected]>

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

drivers/thermal/armada_thermal.c +4 -0

index ae75328945f7..1f7f81628040 100644
@@ -41,6 +41,10 @@
 #define A375_HW_RESETn			BIT(8)
 #define A380_HW_RESET			BIT(8)
 
+#define AP806_START			BIT(0)
+#define AP806_RESET			BIT(1)
+#define AP806_ENABLE			BIT(2)
+
 struct armada_thermal_data;
 
 /* Marvell EBU Thermal Sensor Dev Structure */

@@ -147,6 +151,18 @@ static void armada380_init_sensor(struct platform_device *pdev,
 	}
 }
 

drivers/thermal/armada_thermal.c static void armada380_init_sensor(struct platform_device *pdev, · line 163

Quoted 10 lines

drivers/thermal/armada_thermal.c +10 -0

+static void armada_ap806_init_sensor(struct platform_device *pdev,
+				     struct armada_thermal_priv *priv)
+{
+	u32 reg = readl_relaxed(priv->control);
+
+	reg &= ~AP806_RESET;
+	reg |= AP806_START;
+	reg |= AP806_ENABLE;
+	writel(reg, priv->control);
+	mdelay(10);

Do you really need to make the CPU busy-wait for 10ms here? This looks like it's called from a schedulable context, so won't msleep() do?

Hi Russell,

Quoted 2 lines · Russell King - ARM Linux

Quoted 1 line · Baruch Siach

The AP806 component is integrated in the Armada 8k and 7k lines of processors.

Quoted 5 lines

drivers/thermal/armada_thermal.c static void armada380_init_sensor(struct platform_device *pdev, · line 163

Quoted 1 line

drivers/thermal/armada_thermal.c +1 -0

+	mdelay(10);

Do you really need to make the CPU busy-wait for 10ms here? This looks like it's called from a schedulable context, so won't msleep() do?

msleep() should be fine as well. I just mindlessly copied this code from the vendor kernel. I'll change that in the next iteration.

Thanks for reviewing,
baruch

Signature · 3 lines

     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -

The CP110 component is integrated in the Armada 8k and 7k lines of processors.

This patch also adds an option of offset to the MSB of the control register. The existing DT binding for Armada 38x refers to a single 32 bit control register. It turns out that this is actually only the MSB of the control area. Changing the binding to fix that would break existing DT files, so the Armada 38x binding is left as is.

The new CP110 binding increases the size of the control area to 64 bits, thus moving the MSB to offset 4.

Signed-off-by Baruch Siach <[email protected]>

---
 drivers/thermal/armada_thermal.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

drivers/thermal/armada_thermal.c +1 -0

index 1f7f81628040..542db578ce36 100644
@@ -72,6 +72,7 @@ struct armada_thermal_data {
 	unsigned int temp_shift;
 	unsigned int temp_mask;
 	unsigned int is_valid_shift;
+	unsigned int control_msb_offset;
 };
 
 static void armadaxp_init_sensor(struct platform_device *pdev,

drivers/thermal/armada_thermal.c +18 -1

 	/* Reset hardware once */
 	if (!(reg & A380_HW_RESET)) {
 		reg |= A380_HW_RESET;
-		writel(reg, priv->control);
+		writel(reg, control_msb);
 		mdelay(10);
 	}
 }

@@ -258,6 +261,19 @@ static const struct armada_thermal_data armada_ap806_data = {
 	.inverted = true,
 };
 
+static const struct armada_thermal_data armada_cp110_data = {
+	.is_valid = armada_is_valid,
+	.init_sensor = armada380_init_sensor,
+	.is_valid_shift = 10,
+	.temp_shift = 0,
+	.temp_mask = 0x3ff,
+	.control_msb_offset = 4,
+	.coef_b = 1172499100UL,
+	.coef_m = 2000096UL,
+	.coef_div = 4201,
+	.inverted = true,
+};
+
 static const struct of_device_id armada_thermal_id_table[] = {
 	{
 		.compatible = "marvell,armadaxp-thermal",

@@ -279,6 +295,10 @@ static const struct of_device_id armada_thermal_id_table[] = {
 		.compatible = "marvell,armada-ap806-thermal",
 		.data       = &armada_ap806_data,
 	},
+	{
+		.compatible = "marvell,armada-cp110-thermal",
+		.data       = &armada_cp110_data,
+	},
 	{
 		/* sentinel */
 	},

Signature · 1 line

2.15.0

Quoted 27 lines · Baruch Siach

The CP110 component is integrated in the Armada 8k and 7k lines of processors.

This patch also adds an option of offset to the MSB of the control register. The existing DT binding for Armada 38x refers to a single 32 bit control register. It turns out that this is actually only the MSB of the control area. Changing the binding to fix that would break existing DT files, so the Armada 38x binding is left as is.

The new CP110 binding increases the size of the control area to 64 bits, thus moving the MSB to offset 4.

Signed-off-by Baruch Siach <[email protected]>

---
 drivers/thermal/armada_thermal.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

drivers/thermal/armada_thermal.c +1 -0

index 1f7f81628040..542db578ce36 100644
@@ -72,6 +72,7 @@ struct armada_thermal_data {
 	unsigned int temp_shift;
 	unsigned int temp_mask;
 	unsigned int is_valid_shift;
+	unsigned int control_msb_offset;
 };
 

drivers/thermal/armada_thermal.c static void armada375_init_sensor(struct platform_device *pdev, · line 147

Quoted 9 lines

drivers/thermal/armada_thermal.c +3 -1

 static void armadaxp_init_sensor(struct platform_device *pdev,

@@ -141,12 +142,14 @@ static void armada375_init_sensor(struct platform_device *pdev,
 static void armada380_init_sensor(struct platform_device *pdev,
 				  struct armada_thermal_priv *priv)
 {
-	unsigned long reg = readl_relaxed(priv->control);
+	void __iomem *control_msb =
+		priv->control + priv->data->control_msb_offset;
+	unsigned long reg = readl_relaxed(control_msb);

Will existing users of this function work with default control_msb_offset? presumably 0, right?

Quoted 39 lines

drivers/thermal/armada_thermal.c +18 -1

 	/* Reset hardware once */
 	if (!(reg & A380_HW_RESET)) {
 		reg |= A380_HW_RESET;
-		writel(reg, priv->control);
+		writel(reg, control_msb);
 		mdelay(10);
 	}
 }

@@ -258,6 +261,19 @@ static const struct armada_thermal_data armada_ap806_data = {
 	.inverted = true,
 };
 
+static const struct armada_thermal_data armada_cp110_data = {
+	.is_valid = armada_is_valid,
+	.init_sensor = armada380_init_sensor,
+	.is_valid_shift = 10,
+	.temp_shift = 0,
+	.temp_mask = 0x3ff,
+	.control_msb_offset = 4,
+	.coef_b = 1172499100UL,
+	.coef_m = 2000096UL,
+	.coef_div = 4201,
+	.inverted = true,
+};
+
 static const struct of_device_id armada_thermal_id_table[] = {
 	{
 		.compatible = "marvell,armadaxp-thermal",

@@ -279,6 +295,10 @@ static const struct of_device_id armada_thermal_id_table[] = {
 		.compatible = "marvell,armada-ap806-thermal",
 		.data       = &armada_ap806_data,
 	},
+	{
+		.compatible = "marvell,armada-cp110-thermal",
+		.data       = &armada_cp110_data,
+	},
 	{
 		/* sentinel */
 	},