[PATCH 0/6] thermal: Add Armada 375 SoC support

[PATCH 0/6] thermal: Add Armada 375 SoC support

[Ezequiel Garcia]

This patchset adds the support for the thermal sensor in the recently
introduced Armada 375 SoC.

The first three patches are preparation work. They add a generic
infrastructure that allows to support similar thermal sensors in
a non-intrusive way.

Patch four uses the infrastructure to support the Armada 375 SoC
thermal sensor.

Since there are some issues in the Z1 SoC thermal sensor, patch five
adds a quirk to workaround such issues. The Z1 stepping is detected
and the compatible string is updated, so the driver can perform
a special sensor initialization. Also, the quirk moves the offset of
the thermal control register, and allows to specifiy the correct
(A0 stepping) offset in the devicetree.

This quirk is applied only for the A375-DB board, being the only
board with the problematic Z1 SoC.

Finally, the last patch enables the thermal sensor in the devicetree.
for the Armada 375 A0 stepping SoC.

The series applies on v3.15-rc1, and has been tested on A375-DB board and
A370-RD board.

Feedback and comments are welcome!

Ezequiel Garcia (6):
  thermal: armada: Rename armada_thermal_ops struct
  thermal: armada: Add infrastructure to support generic formulas
  thermal: armada: Add generic infrastructure to handle the sensor
  thermal: armada: Support Armada 375 SoC
  ARM: mvebu: Add thermal quirk for the Armada 375 DB board
  ARM: mvebu: Enable the thermal sensor in Armada 375 SoC

 .../devicetree/bindings/thermal/armada-thermal.txt |   7 ++
 arch/arm/boot/dts/armada-375.dtsi                  |   6 +
 arch/arm/mach-mvebu/board-v7.c                     |  57 +++++++++
 arch/arm/mach-mvebu/mvebu-soc-id.h                 |   3 +
 drivers/thermal/armada_thermal.c                   | 140 ++++++++++++++++++---
 5 files changed, 197 insertions(+), 16 deletions(-)

-- 
1.9.1

[PATCH 1/6] thermal: armada: Rename armada_thermal_ops struct

[Ezequiel Garcia]

As preparation work to add a generic infrastructure to support
different SoC variants, the armada_thermal_ops will be used
to host the SoC-specific fields, such as formula values and
register offsets.

For this reason, the name armada_thermal_ops is no longer suitable,
and this commit replaces it with armada_thermal_data.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 drivers/thermal/armada_thermal.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
index 5e53212..c5db071 100644
--- a/drivers/thermal/armada_thermal.c
+++ b/drivers/thermal/armada_thermal.c
@@ -38,16 +38,16 @@
 #define PMU_TDC0_OTF_CAL_MASK		(0x1 << 30)
 #define PMU_TDC0_START_CAL_MASK		(0x1 << 25)
 
-struct armada_thermal_ops;
+struct armada_thermal_data;
 
 /* Marvell EBU Thermal Sensor Dev Structure */
 struct armada_thermal_priv {
 	void __iomem *sensor;
 	void __iomem *control;
-	struct armada_thermal_ops *ops;
+	struct armada_thermal_data *data;
 };
 
-struct armada_thermal_ops {
+struct armada_thermal_data {
 	/* Initialize the sensor */
 	void (*init_sensor)(struct armada_thermal_priv *);
 
@@ -113,7 +113,7 @@ static int armada_get_temp(struct thermal_zone_device *thermal,
 	unsigned long reg;
 
 	/* Valid check */
-	if (priv->ops->is_valid && !priv->ops->is_valid(priv)) {
+	if (priv->data->is_valid && !priv->data->is_valid(priv)) {
 		dev_err(&thermal->device,
 			"Temperature sensor reading not valid\n");
 		return -EIO;
@@ -129,11 +129,11 @@ static struct thermal_zone_device_ops ops = {
 	.get_temp = armada_get_temp,
 };
 
-static const struct armada_thermal_ops armadaxp_ops = {
+static const struct armada_thermal_data armadaxp_data = {
 	.init_sensor = armadaxp_init_sensor,
 };
 
-static const struct armada_thermal_ops armada370_ops = {
+static const struct armada_thermal_data armada370_data = {
 	.is_valid = armada_is_valid,
 	.init_sensor = armada370_init_sensor,
 };
@@ -141,11 +141,11 @@ static const struct armada_thermal_ops armada370_ops = {
 static const struct of_device_id armada_thermal_id_table[] = {
 	{
 		.compatible = "marvell,armadaxp-thermal",
-		.data       = &armadaxp_ops,
+		.data       = &armadaxp_data,
 	},
 	{
 		.compatible = "marvell,armada370-thermal",
-		.data       = &armada370_ops,
+		.data       = &armada370_data,
 	},
 	{
 		/* sentinel */
@@ -178,8 +178,8 @@ static int armada_thermal_probe(struct platform_device *pdev)
 	if (IS_ERR(priv->control))
 		return PTR_ERR(priv->control);
 
-	priv->ops = (struct armada_thermal_ops *)match->data;
-	priv->ops->init_sensor(priv);
+	priv->data = (struct armada_thermal_data *)match->data;
+	priv->data->init_sensor(priv);
 
 	thermal = thermal_zone_device_register("armada_thermal", 0, 0,
 					       priv, &ops, NULL, 0, 0);
-- 
1.9.1

[PATCH 2/6] thermal: armada: Add infrastructure to support generic formulas

[Ezequiel Garcia]

In order to support other similar SoC, with different sensor
coeficients, this commit adds the coeficients to the per-variant
structure, instead of having the formula hardcoded.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 drivers/thermal/armada_thermal.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
index c5db071..4de6e56 100644
--- a/drivers/thermal/armada_thermal.c
+++ b/drivers/thermal/armada_thermal.c
@@ -53,6 +53,11 @@ struct armada_thermal_data {
 
 	/* Test for a valid sensor value (optional) */
 	bool (*is_valid)(struct armada_thermal_priv *);
+
+	/* Formula coeficients: temp = (b + m * reg) / div */
+	unsigned long coef_b;
+	unsigned long coef_m;
+	unsigned long coef_div;
 };
 
 static void armadaxp_init_sensor(struct armada_thermal_priv *priv)
@@ -111,6 +116,7 @@ static int armada_get_temp(struct thermal_zone_device *thermal,
 {
 	struct armada_thermal_priv *priv = thermal->devdata;
 	unsigned long reg;
+	unsigned long m, b, div;
 
 	/* Valid check */
 	if (priv->data->is_valid && !priv->data->is_valid(priv)) {
@@ -121,7 +127,13 @@ static int armada_get_temp(struct thermal_zone_device *thermal,
 
 	reg = readl_relaxed(priv->sensor);
 	reg = (reg >> THERMAL_TEMP_OFFSET) & THERMAL_TEMP_MASK;
-	*temp = (3153000000UL - (10000000UL*reg)) / 13825;
+
+	/* Get formula coeficients */
+	b = priv->data->coef_b;
+	m = priv->data->coef_m;
+	div = priv->data->coef_div;
+
+	*temp = (b - (m * reg)) / div;
 	return 0;
 }
 
@@ -131,11 +143,17 @@ static struct thermal_zone_device_ops ops = {
 
 static const struct armada_thermal_data armadaxp_data = {
 	.init_sensor = armadaxp_init_sensor,
+	.coef_b = 3153000000UL,
+	.coef_m = 10000000UL,
+	.coef_div = 13825,
 };
 
 static const struct armada_thermal_data armada370_data = {
 	.is_valid = armada_is_valid,
 	.init_sensor = armada370_init_sensor,
+	.coef_b = 3153000000UL,
+	.coef_m = 10000000UL,
+	.coef_div = 13825,
 };
 
 static const struct of_device_id armada_thermal_id_table[] = {
-- 
1.9.1

[PATCH 3/6] thermal: armada: Add generic infrastructure to handle the sensor

[Ezequiel Garcia]

In order to support similar SoC where the sensor value and valid
bit can have different offset and/or mask, we add such fields to the
per-variant structure, instead of having the values hardcoded.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 drivers/thermal/armada_thermal.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
index 4de6e56..3e4d8ef 100644
--- a/drivers/thermal/armada_thermal.c
+++ b/drivers/thermal/armada_thermal.c
@@ -24,10 +24,7 @@
 #include <linux/of_device.h>
 #include <linux/thermal.h>
 
-#define THERMAL_VALID_OFFSET		9
 #define THERMAL_VALID_MASK		0x1
-#define THERMAL_TEMP_OFFSET		10
-#define THERMAL_TEMP_MASK		0x1ff
 
 /* Thermal Manager Control and Status Register */
 #define PMU_TDC0_SW_RST_MASK		(0x1 << 1)
@@ -58,6 +55,11 @@ struct armada_thermal_data {
 	unsigned long coef_b;
 	unsigned long coef_m;
 	unsigned long coef_div;
+
+	/* Offset and mask to access the sensor temperature */
+	unsigned int temp_offset;
+	unsigned int temp_mask;
+	unsigned int is_valid_offset;
 };
 
 static void armadaxp_init_sensor(struct armada_thermal_priv *priv)
@@ -108,7 +110,7 @@ static bool armada_is_valid(struct armada_thermal_priv *priv)
 {
 	unsigned long reg = readl_relaxed(priv->sensor);
 
-	return (reg >> THERMAL_VALID_OFFSET) & THERMAL_VALID_MASK;
+	return (reg >> priv->data->is_valid_offset) & THERMAL_VALID_MASK;
 }
 
 static int armada_get_temp(struct thermal_zone_device *thermal,
@@ -126,7 +128,7 @@ static int armada_get_temp(struct thermal_zone_device *thermal,
 	}
 
 	reg = readl_relaxed(priv->sensor);
-	reg = (reg >> THERMAL_TEMP_OFFSET) & THERMAL_TEMP_MASK;
+	reg = (reg >> priv->data->temp_offset) & priv->data->temp_mask;
 
 	/* Get formula coeficients */
 	b = priv->data->coef_b;
@@ -143,6 +145,8 @@ static struct thermal_zone_device_ops ops = {
 
 static const struct armada_thermal_data armadaxp_data = {
 	.init_sensor = armadaxp_init_sensor,
+	.temp_offset = 10,
+	.temp_mask = 0x1ff,
 	.coef_b = 3153000000UL,
 	.coef_m = 10000000UL,
 	.coef_div = 13825,
@@ -151,6 +155,9 @@ static const struct armada_thermal_data armadaxp_data = {
 static const struct armada_thermal_data armada370_data = {
 	.is_valid = armada_is_valid,
 	.init_sensor = armada370_init_sensor,
+	.is_valid_offset = 9,
+	.temp_offset = 10,
+	.temp_mask = 0x1ff,
 	.coef_b = 3153000000UL,
 	.coef_m = 10000000UL,
 	.coef_div = 13825,
-- 
1.9.1

[PATCH 4/6] thermal: armada: Support Armada 375 SoC

[Ezequiel Garcia]

Now that a generic infrastructure is in place, it's possible to support
the new Armada 375 SoC thermal sensor. This sensor is similar to the one
available in the already supported SoCs, with its specific temperature formula
and specific sensor initialization.

In addition, we also add support for the Z1 SoC stepping, which needs
an initialization-quirk to work properly.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 .../devicetree/bindings/thermal/armada-thermal.txt |  7 ++
 drivers/thermal/armada_thermal.c                   | 83 ++++++++++++++++++++++
 2 files changed, 90 insertions(+)

diff --git a/Documentation/devicetree/bindings/thermal/armada-thermal.txt b/Documentation/devicetree/bindings/thermal/armada-thermal.txt
index fff93d5..745d241 100644
--- a/Documentation/devicetree/bindings/thermal/armada-thermal.txt
+++ b/Documentation/devicetree/bindings/thermal/armada-thermal.txt
@@ -4,8 +4,15 @@ Required properties:
 
 - compatible:	Should be set to one of the following:
 		marvell,armada370-thermal
+		marvell,armada375-thermal
+		marvell,armada375-z1-thermal
 		marvell,armadaxp-thermal
 
+		Note: As the name suggests, "marvell,armada375-z1-thermal"
+		applies for the SoC Z1 stepping only. The operating system
+		may auto-detect the SoC stepping and update the compatible
+		at runtime.
+
 - reg:		Device's register space.
 		Two entries are expected, see the examples below.
 		The first one is required for the sensor register;
diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
index 3e4d8ef..a37942b 100644
--- a/drivers/thermal/armada_thermal.c
+++ b/drivers/thermal/armada_thermal.c
@@ -35,6 +35,15 @@
 #define PMU_TDC0_OTF_CAL_MASK		(0x1 << 30)
 #define PMU_TDC0_START_CAL_MASK		(0x1 << 25)
 
+#define A375_Z1_CAL_RESET_LSB		0x8011e214
+#define A375_Z1_CAL_RESET_MSB		0x30a88019
+#define A375_Z1_WORKAROUND_BIT		BIT(9)
+
+#define TSEN40_UNIT_CONTROL_OFFSET	27
+#define TSEN40_UNIT_CONTROL_MASK	0x7
+#define TSEN40_READOUT_INVERT		BIT(15)
+#define TSEN40_HW_RESETn		BIT(8)
+
 struct armada_thermal_data;
 
 /* Marvell EBU Thermal Sensor Dev Structure */
@@ -106,6 +115,50 @@ static void armada370_init_sensor(struct armada_thermal_priv *priv)
 	mdelay(10);
 }
 
+static void armada375_init_sensor(struct armada_thermal_priv *priv)
+{
+	unsigned long reg;
+
+	reg = readl(priv->control + 4);
+	reg &= ~(TSEN40_UNIT_CONTROL_MASK << TSEN40_UNIT_CONTROL_OFFSET);
+	reg &= ~TSEN40_READOUT_INVERT;
+	reg &= ~TSEN40_HW_RESETn;
+
+	writel(reg, priv->control + 4);
+	mdelay(20);
+
+	reg |= TSEN40_HW_RESETn;
+	writel(reg, priv->control + 4);
+	mdelay(50);
+}
+
+static void armada375_z1_init_sensor(struct armada_thermal_priv *priv)
+{
+	unsigned long reg;
+
+	/*
+	 * On A375 Z1 SoC silicon revision the default (reset) values
+	 * must be written.
+	 */
+	writel(A375_Z1_CAL_RESET_LSB, priv->control);
+	writel(A375_Z1_CAL_RESET_MSB, priv->control + 0x4);
+
+	reg = readl(priv->control + 4);
+	reg &= ~(TSEN40_UNIT_CONTROL_MASK << TSEN40_UNIT_CONTROL_OFFSET);
+	reg &= ~TSEN40_READOUT_INVERT;
+	reg &= ~TSEN40_HW_RESETn;
+
+	/* This is only needed on A375 Z1 SoC silicon revision */
+	reg |= A375_Z1_WORKAROUND_BIT;
+
+	writel(reg, priv->control + 4);
+	mdelay(20);
+
+	reg |= TSEN40_HW_RESETn;
+	writel(reg, priv->control + 4);
+	mdelay(50);
+}
+
 static bool armada_is_valid(struct armada_thermal_priv *priv)
 {
 	unsigned long reg = readl_relaxed(priv->sensor);
@@ -163,6 +216,28 @@ static const struct armada_thermal_data armada370_data = {
 	.coef_div = 13825,
 };
 
+static const struct armada_thermal_data armada375_data = {
+	.is_valid = armada_is_valid,
+	.init_sensor = armada375_init_sensor,
+	.is_valid_offset = 10,
+	.temp_offset = 0,
+	.temp_mask = 0x1ff,
+	.coef_b = 3171900000UL,
+	.coef_m = 10000000UL,
+	.coef_div = 13616,
+};
+
+static const struct armada_thermal_data armada375_z1_data = {
+	.is_valid = armada_is_valid,
+	.init_sensor = armada375_z1_init_sensor,
+	.is_valid_offset = 10,
+	.temp_offset = 0,
+	.temp_mask = 0x1ff,
+	.coef_b = 3171900000UL,
+	.coef_m = 10000000UL,
+	.coef_div = 13616,
+};
+
 static const struct of_device_id armada_thermal_id_table[] = {
 	{
 		.compatible = "marvell,armadaxp-thermal",
@@ -173,6 +248,14 @@ static const struct of_device_id armada_thermal_id_table[] = {
 		.data       = &armada370_data,
 	},
 	{
+		.compatible = "marvell,armada375-thermal",
+		.data       = &armada375_data,
+	},
+	{
+		.compatible = "marvell,armada375-z1-thermal",
+		.data       = &armada375_z1_data,
+	},
+	{
 		/* sentinel */
 	},
 };
-- 
1.9.1

Re: [PATCH 4/6] thermal: armada: Support Armada 375 SoC

[Jason Cooper]

Ezequiel,

On Wed, Apr 16, 2014 at 11:15:18AM -0300, Ezequiel Garcia wrote:
> Now that a generic infrastructure is in place, it's possible to support
> the new Armada 375 SoC thermal sensor. This sensor is similar to the one
> available in the already supported SoCs, with its specific temperature formula
> and specific sensor initialization.
> 
> In addition, we also add support for the Z1 SoC stepping, which needs
> an initialization-quirk to work properly.
> 
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> ---
>  .../devicetree/bindings/thermal/armada-thermal.txt |  7 ++
>  drivers/thermal/armada_thermal.c                   | 83 ++++++++++++++++++++++
>  2 files changed, 90 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/thermal/armada-thermal.txt b/Documentation/devicetree/bindings/thermal/armada-thermal.txt
> index fff93d5..745d241 100644
> --- a/Documentation/devicetree/bindings/thermal/armada-thermal.txt
> +++ b/Documentation/devicetree/bindings/thermal/armada-thermal.txt
> @@ -4,8 +4,15 @@ Required properties:
>  
>  - compatible:	Should be set to one of the following:
>  		marvell,armada370-thermal
> +		marvell,armada375-thermal
> +		marvell,armada375-z1-thermal
>  		marvell,armadaxp-thermal
>  
> +		Note: As the name suggests, "marvell,armada375-z1-thermal"
> +		applies for the SoC Z1 stepping only. The operating system
> +		may auto-detect the SoC stepping and update the compatible
> +		at runtime.
> +
>  - reg:		Device's register space.
>  		Two entries are expected, see the examples below.
>  		The first one is required for the sensor register;
> diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
> index 3e4d8ef..a37942b 100644
> --- a/drivers/thermal/armada_thermal.c
> +++ b/drivers/thermal/armada_thermal.c
> @@ -35,6 +35,15 @@
>  #define PMU_TDC0_OTF_CAL_MASK		(0x1 << 30)
>  #define PMU_TDC0_START_CAL_MASK		(0x1 << 25)
>  
> +#define A375_Z1_CAL_RESET_LSB		0x8011e214
> +#define A375_Z1_CAL_RESET_MSB		0x30a88019
> +#define A375_Z1_WORKAROUND_BIT		BIT(9)
> +
> +#define TSEN40_UNIT_CONTROL_OFFSET	27
> +#define TSEN40_UNIT_CONTROL_MASK	0x7
> +#define TSEN40_READOUT_INVERT		BIT(15)
> +#define TSEN40_HW_RESETn		BIT(8)
> +
>  struct armada_thermal_data;
>  
>  /* Marvell EBU Thermal Sensor Dev Structure */
> @@ -106,6 +115,50 @@ static void armada370_init_sensor(struct armada_thermal_priv *priv)
>  	mdelay(10);
>  }
>  
> +static void armada375_init_sensor(struct armada_thermal_priv *priv)
> +{
> +	unsigned long reg;
> +
> +	reg = readl(priv->control + 4);
> +	reg &= ~(TSEN40_UNIT_CONTROL_MASK << TSEN40_UNIT_CONTROL_OFFSET);
> +	reg &= ~TSEN40_READOUT_INVERT;
> +	reg &= ~TSEN40_HW_RESETn;
> +
> +	writel(reg, priv->control + 4);
> +	mdelay(20);
> +
> +	reg |= TSEN40_HW_RESETn;
> +	writel(reg, priv->control + 4);
> +	mdelay(50);
> +}
> +
> +static void armada375_z1_init_sensor(struct armada_thermal_priv *priv)
> +{
> +	unsigned long reg;
> +


> +	/*
> +	 * On A375 Z1 SoC silicon revision the default (reset) values
> +	 * must be written.
> +	 */
> +	writel(A375_Z1_CAL_RESET_LSB, priv->control);
> +	writel(A375_Z1_CAL_RESET_MSB, priv->control + 0x4);

this...

> +
> +	reg = readl(priv->control + 4);
> +	reg &= ~(TSEN40_UNIT_CONTROL_MASK << TSEN40_UNIT_CONTROL_OFFSET);
> +	reg &= ~TSEN40_READOUT_INVERT;
> +	reg &= ~TSEN40_HW_RESETn;
> +


> +	/* This is only needed on A375 Z1 SoC silicon revision */
> +	reg |= A375_Z1_WORKAROUND_BIT;

and this seem to be the only differences between the two init functions.

It also appears to be the only reason for having two data structs below.
Is it worth checking for the compatible string in the init function so
you only have one init and one data struct?

> +
> +	writel(reg, priv->control + 4);
> +	mdelay(20);
> +
> +	reg |= TSEN40_HW_RESETn;
> +	writel(reg, priv->control + 4);
> +	mdelay(50);
> +}
> +
>  static bool armada_is_valid(struct armada_thermal_priv *priv)
>  {
>  	unsigned long reg = readl_relaxed(priv->sensor);
> @@ -163,6 +216,28 @@ static const struct armada_thermal_data armada370_data = {
>  	.coef_div = 13825,
>  };
>  



> +static const struct armada_thermal_data armada375_data = {
> +	.is_valid = armada_is_valid,
> +	.init_sensor = armada375_init_sensor,
> +	.is_valid_offset = 10,
> +	.temp_offset = 0,
> +	.temp_mask = 0x1ff,
> +	.coef_b = 3171900000UL,
> +	.coef_m = 10000000UL,
> +	.coef_div = 13616,
> +};
> +
> +static const struct armada_thermal_data armada375_z1_data = {
> +	.is_valid = armada_is_valid,
> +	.init_sensor = armada375_z1_init_sensor,
> +	.is_valid_offset = 10,
> +	.temp_offset = 0,
> +	.temp_mask = 0x1ff,
> +	.coef_b = 3171900000UL,
> +	.coef_m = 10000000UL,
> +	.coef_div = 13616,
> +};

thx,

Jason.

> +
>  static const struct of_device_id armada_thermal_id_table[] = {
>  	{
>  		.compatible = "marvell,armadaxp-thermal",
> @@ -173,6 +248,14 @@ static const struct of_device_id armada_thermal_id_table[] = {
>  		.data       = &armada370_data,
>  	},
>  	{
> +		.compatible = "marvell,armada375-thermal",
> +		.data       = &armada375_data,
> +	},
> +	{
> +		.compatible = "marvell,armada375-z1-thermal",
> +		.data       = &armada375_z1_data,
> +	},
> +	{
>  		/* sentinel */
>  	},
>  };
> -- 
> 1.9.1
> 

Re: [PATCH 4/6] thermal: armada: Support Armada 375 SoC

[Ezequiel Garcia]

Jason,

Thanks for taking a look.

On Apr 16, Jason Cooper wrote:
> On Wed, Apr 16, 2014 at 11:15:18AM -0300, Ezequiel Garcia wrote:
> > +	/* This is only needed on A375 Z1 SoC silicon revision */
> > +	reg |= A375_Z1_WORKAROUND_BIT;
> 
> and this seem to be the only differences between the two init functions.
> 
> It also appears to be the only reason for having two data structs below.
> Is it worth checking for the compatible string in the init function so
> you only have one init and one data struct?
> 

Yes, thought about it at one point but I guess it seemed to me cleaner
this way.

I'll squash it if you think keeping two structs is stupid bloat.
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

Re: [PATCH 4/6] thermal: armada: Support Armada 375 SoC

[Jason Cooper]

On Wed, Apr 16, 2014 at 12:49:27PM -0300, Ezequiel Garcia wrote:
> On Apr 16, Jason Cooper wrote:
> > On Wed, Apr 16, 2014 at 11:15:18AM -0300, Ezequiel Garcia wrote:
> > > +	/* This is only needed on A375 Z1 SoC silicon revision */
> > > +	reg |= A375_Z1_WORKAROUND_BIT;
> > 
> > and this seem to be the only differences between the two init functions.
> > 
> > It also appears to be the only reason for having two data structs below.
> > Is it worth checking for the compatible string in the init function so
> > you only have one init and one data struct?
> > 
> 
> Yes, thought about it at one point but I guess it seemed to me cleaner
> this way.
> 
> I'll squash it if you think keeping two structs is stupid bloat.

Well, it's really up to the thermal maintainer.  Either way will work.
I personally think it's easier to grok using the compatible string.
Unfortunately, you're going to have the either check it twice, or set a
variable.

thx,

Jason.

Re: [PATCH 4/6] thermal: armada: Support Armada 375 SoC

[Jason Cooper]

On Wed, Apr 16, 2014 at 11:15:18AM -0300, Ezequiel Garcia wrote:
> In addition, we also add support for the Z1 SoC stepping, which needs
> an initialization-quirk to work properly.
...
> diff --git a/Documentation/devicetree/bindings/thermal/armada-thermal.txt b/Documentation/devicetree/bindings/thermal/armada-thermal.txt
> index fff93d5..745d241 100644
> --- a/Documentation/devicetree/bindings/thermal/armada-thermal.txt
> +++ b/Documentation/devicetree/bindings/thermal/armada-thermal.txt
> @@ -4,8 +4,15 @@ Required properties:
>  
>  - compatible:	Should be set to one of the following:
>  		marvell,armada370-thermal
> +		marvell,armada375-thermal
> +		marvell,armada375-z1-thermal
>  		marvell,armadaxp-thermal
>  
> +		Note: As the name suggests, "marvell,armada375-z1-thermal"
> +		applies for the SoC Z1 stepping only. The operating system
> +		may auto-detect the SoC stepping and update the compatible
> +		at runtime.

Please also include a statement in here regarding the register quirk for
the z1 stepping.

thx,

Jason.

Re: [PATCH 4/6] thermal: armada: Support Armada 375 SoC

[Ezequiel Garcia]

On Apr 16, Jason Cooper wrote:
> On Wed, Apr 16, 2014 at 11:15:18AM -0300, Ezequiel Garcia wrote:
> > In addition, we also add support for the Z1 SoC stepping, which needs
> > an initialization-quirk to work properly.
> ...
> > diff --git a/Documentation/devicetree/bindings/thermal/armada-thermal.txt b/Documentation/devicetree/bindings/thermal/armada-thermal.txt
> > index fff93d5..745d241 100644
> > --- a/Documentation/devicetree/bindings/thermal/armada-thermal.txt
> > +++ b/Documentation/devicetree/bindings/thermal/armada-thermal.txt
> > @@ -4,8 +4,15 @@ Required properties:
> >  
> >  - compatible:	Should be set to one of the following:
> >  		marvell,armada370-thermal
> > +		marvell,armada375-thermal
> > +		marvell,armada375-z1-thermal
> >  		marvell,armadaxp-thermal
> >  
> > +		Note: As the name suggests, "marvell,armada375-z1-thermal"
> > +		applies for the SoC Z1 stepping only. The operating system
> > +		may auto-detect the SoC stepping and update the compatible
> > +		at runtime.
> 
> Please also include a statement in here regarding the register quirk for
> the z1 stepping.
> 

Sure, no problem.
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

[PATCH 5/6] ARM: mvebu: Add thermal quirk for the Armada 375 DB board

[Ezequiel Garcia]

The initial release of the Armada 375 DB board has an Armada 375
Z1 stepping silicon. This commit introduces a quirk that allows
to workaround a series of issues with the thermal sensor in this
stepping, but updating the devicetree:

  * Updates the compatible string for the thermal, so the driver
    can perform a specific initialization of the sensor.

  * Moves the offset of the thermal control register. This quirk
    allows to specifiy the correct (A0 stepping) offset in the
    devicetree.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 arch/arm/mach-mvebu/board-v7.c     | 57 ++++++++++++++++++++++++++++++++++++++
 arch/arm/mach-mvebu/mvebu-soc-id.h |  3 ++
 2 files changed, 60 insertions(+)

diff --git a/arch/arm/mach-mvebu/board-v7.c b/arch/arm/mach-mvebu/board-v7.c
index 333fca8..93f50f2 100644
--- a/arch/arm/mach-mvebu/board-v7.c
+++ b/arch/arm/mach-mvebu/board-v7.c
@@ -96,10 +96,66 @@ static void __init i2c_quirk(void)
 	return;
 }
 
+#define A375_Z1_THERMAL_FIXUP_OFFSET 0xc
+
+static void __init thermal_quirk(void)
+{
+	struct device_node *np;
+	u32 dev, rev;
+
+	if (mvebu_get_soc_id(&dev, &rev) && rev > ARMADA_375_Z1_REV)
+		return;
+
+	for_each_compatible_node(np, NULL, "marvell,armada375-thermal") {
+		struct property *prop;
+		__be32 newval, *newprop, *oldprop;
+		int len;
+
+		/*
+		 * The register offset is at a wrong location. This quirk
+		 * creates a new reg property as a clone of the previous
+		 * one and corrects the offset.
+		 */
+		oldprop = (__be32 *)of_get_property(np, "reg", &len);
+		if (!oldprop)
+			continue;
+
+		/* Create a duplicate of the 'reg' property */
+		prop = kzalloc(sizeof(*prop), GFP_KERNEL);
+		prop->length = len;
+		prop->name = kstrdup("reg", GFP_KERNEL);
+		prop->value = kzalloc(len, GFP_KERNEL);
+		memcpy(prop->value, oldprop, len);
+
+		/* Fixup the register offset of the second entry */
+		oldprop += 2;
+		newprop = (__be32 *)prop->value + 2;
+		newval = cpu_to_be32(be32_to_cpu(*oldprop) -
+				     A375_Z1_THERMAL_FIXUP_OFFSET);
+		*newprop = newval;
+		of_update_property(np, prop);
+
+		/*
+		 * The thermal controller needs some quirk too, so let's change
+		 * the compatible string to reflect this.
+		 */
+		prop = kzalloc(sizeof(*prop), GFP_KERNEL);
+		prop->name = kstrdup("compatible", GFP_KERNEL);
+		prop->length = sizeof("marvell,armada375-z1-thermal");
+		prop->value = kstrdup("marvell,armada375-z1-thermal",
+						GFP_KERNEL);
+		of_update_property(np, prop);
+	}
+	return;
+}
+
 static void __init mvebu_dt_init(void)
 {
 	if (of_machine_is_compatible("plathome,openblocks-ax3-4"))
 		i2c_quirk();
+	if (of_machine_is_compatible("marvell,a375-db"))
+		thermal_quirk();
+
 	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
 }
 
@@ -123,6 +179,7 @@ static const char * const armada_375_dt_compat[] = {
 
 DT_MACHINE_START(ARMADA_375_DT, "Marvell Armada 375 (Device Tree)")
 	.init_time	= mvebu_timer_and_clk_init,
+	.init_machine	= mvebu_dt_init,
 	.restart	= mvebu_restart,
 	.dt_compat	= armada_375_dt_compat,
 MACHINE_END
diff --git a/arch/arm/mach-mvebu/mvebu-soc-id.h b/arch/arm/mach-mvebu/mvebu-soc-id.h
index 3165425..294a443 100644
--- a/arch/arm/mach-mvebu/mvebu-soc-id.h
+++ b/arch/arm/mach-mvebu/mvebu-soc-id.h
@@ -20,6 +20,9 @@
 #define MV78XX0_A0_REV	    0x1
 #define MV78XX0_B0_REV	    0x2
 
+/* Armada 375 */
+#define ARMADA_375_Z1_REV   0x0
+
 #ifdef CONFIG_ARCH_MVEBU
 int mvebu_get_soc_id(u32 *dev, u32 *rev);
 #else
-- 
1.9.1

Re: [PATCH 5/6] ARM: mvebu: Add thermal quirk for the Armada 375 DB board

[Sebastian Hesselbarth]

On 04/16/2014 04:15 PM, Ezequiel Garcia wrote:
> The initial release of the Armada 375 DB board has an Armada 375
> Z1 stepping silicon. This commit introduces a quirk that allows
> to workaround a series of issues with the thermal sensor in this
> stepping, but updating the devicetree:
>
>    * Updates the compatible string for the thermal, so the driver
>      can perform a specific initialization of the sensor.
>
>    * Moves the offset of the thermal control register. This quirk
>      allows to specifiy the correct (A0 stepping) offset in the
>      devicetree.
>
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> ---
>   arch/arm/mach-mvebu/board-v7.c     | 57 ++++++++++++++++++++++++++++++++++++++
>   arch/arm/mach-mvebu/mvebu-soc-id.h |  3 ++
>   2 files changed, 60 insertions(+)
>
> diff --git a/arch/arm/mach-mvebu/board-v7.c b/arch/arm/mach-mvebu/board-v7.c
> index 333fca8..93f50f2 100644
> --- a/arch/arm/mach-mvebu/board-v7.c
> +++ b/arch/arm/mach-mvebu/board-v7.c
> @@ -96,10 +96,66 @@ static void __init i2c_quirk(void)
>   	return;
>   }
>
> +#define A375_Z1_THERMAL_FIXUP_OFFSET 0xc
> +
> +static void __init thermal_quirk(void)

Are we sure, we want to fixup quirks like this the way below?

Alternatively, we can also keep some armada-375-z1.dtsi and one
for the board including it.

Sebastian

> +{
> +	struct device_node *np;
> +	u32 dev, rev;
> +
> +	if (mvebu_get_soc_id(&dev, &rev) && rev > ARMADA_375_Z1_REV)
> +		return;
> +
> +	for_each_compatible_node(np, NULL, "marvell,armada375-thermal") {
> +		struct property *prop;
> +		__be32 newval, *newprop, *oldprop;
> +		int len;
> +
> +		/*
> +		 * The register offset is at a wrong location. This quirk
> +		 * creates a new reg property as a clone of the previous
> +		 * one and corrects the offset.
> +		 */
> +		oldprop = (__be32 *)of_get_property(np, "reg", &len);
> +		if (!oldprop)
> +			continue;
> +
> +		/* Create a duplicate of the 'reg' property */
> +		prop = kzalloc(sizeof(*prop), GFP_KERNEL);
> +		prop->length = len;
> +		prop->name = kstrdup("reg", GFP_KERNEL);
> +		prop->value = kzalloc(len, GFP_KERNEL);
> +		memcpy(prop->value, oldprop, len);
> +
> +		/* Fixup the register offset of the second entry */
> +		oldprop += 2;
> +		newprop = (__be32 *)prop->value + 2;
> +		newval = cpu_to_be32(be32_to_cpu(*oldprop) -
> +				     A375_Z1_THERMAL_FIXUP_OFFSET);
> +		*newprop = newval;
> +		of_update_property(np, prop);
> +
> +		/*
> +		 * The thermal controller needs some quirk too, so let's change
> +		 * the compatible string to reflect this.
> +		 */
> +		prop = kzalloc(sizeof(*prop), GFP_KERNEL);
> +		prop->name = kstrdup("compatible", GFP_KERNEL);
> +		prop->length = sizeof("marvell,armada375-z1-thermal");
> +		prop->value = kstrdup("marvell,armada375-z1-thermal",
> +						GFP_KERNEL);
> +		of_update_property(np, prop);
> +	}
> +	return;
> +}
> +
>   static void __init mvebu_dt_init(void)
>   {
>   	if (of_machine_is_compatible("plathome,openblocks-ax3-4"))
>   		i2c_quirk();
> +	if (of_machine_is_compatible("marvell,a375-db"))
> +		thermal_quirk();
> +
>   	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
>   }
>
> @@ -123,6 +179,7 @@ static const char * const armada_375_dt_compat[] = {
>
>   DT_MACHINE_START(ARMADA_375_DT, "Marvell Armada 375 (Device Tree)")
>   	.init_time	= mvebu_timer_and_clk_init,
> +	.init_machine	= mvebu_dt_init,
>   	.restart	= mvebu_restart,
>   	.dt_compat	= armada_375_dt_compat,
>   MACHINE_END
> diff --git a/arch/arm/mach-mvebu/mvebu-soc-id.h b/arch/arm/mach-mvebu/mvebu-soc-id.h
> index 3165425..294a443 100644
> --- a/arch/arm/mach-mvebu/mvebu-soc-id.h
> +++ b/arch/arm/mach-mvebu/mvebu-soc-id.h
> @@ -20,6 +20,9 @@
>   #define MV78XX0_A0_REV	    0x1
>   #define MV78XX0_B0_REV	    0x2
>
> +/* Armada 375 */
> +#define ARMADA_375_Z1_REV   0x0
> +
>   #ifdef CONFIG_ARCH_MVEBU
>   int mvebu_get_soc_id(u32 *dev, u32 *rev);
>   #else
>

Re: [PATCH 5/6] ARM: mvebu: Add thermal quirk for the Armada 375 DB board

[Thomas Petazzoni]

Dear Sebastian Hesselbarth,

On Wed, 16 Apr 2014 17:59:05 +0200, Sebastian Hesselbarth wrote:

> Are we sure, we want to fixup quirks like this the way below?

We already have an exactly identical quirk for the A0 I2C issue, in the
same file, right above the quirk Ezequiel is adding here. So using the
same strategy for both cases would be nice.

> Alternatively, we can also keep some armada-375-z1.dtsi and one
> for the board including it.

For minor differences such as SoC stepping, I personally prefer to not
have separate Device Trees. We already have many of them, for each
variant of the various SOCs. If we add the different steppings, it's
going to be even more complicated. Also, there will be a new iteration
of the Armada 375 DB with an A0 chip, which does not have the Z1 bug.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH 5/6] ARM: mvebu: Add thermal quirk for the Armada 375 DB board

[Andrew Lunn]

> For minor differences such as SoC stepping, I personally prefer to not
> have separate Device Trees. We already have many of them, for each
> variant of the various SOCs. If we add the different steppings, it's
> going to be even more complicated. Also, there will be a new iteration
> of the Armada 375 DB with an A0 chip, which does not have the Z1 bug.

How many Z1 are there out and about? Would it be simpler to just
disable thermal on Z1? If only development boards have Z1, this could
be reasonable.

	Andrew

Re: [PATCH 5/6] ARM: mvebu: Add thermal quirk for the Armada 375 DB board

[Thomas Petazzoni]

Dear Andrew Lunn,

On Wed, 16 Apr 2014 18:08:24 +0200, Andrew Lunn wrote:
> > For minor differences such as SoC stepping, I personally prefer to not
> > have separate Device Trees. We already have many of them, for each
> > variant of the various SOCs. If we add the different steppings, it's
> > going to be even more complicated. Also, there will be a new iteration
> > of the Armada 375 DB with an A0 chip, which does not have the Z1 bug.
> 
> How many Z1 are there out and about? Would it be simpler to just
> disable thermal on Z1? If only development boards have Z1, this could
> be reasonable.

Not many, but we also need workarounds for other Z1 issues, such as the
I/O coherency workaround (see the 375 coherency patches) and the SMP
issue (see the 375 SMP patches).

For now, Gregory, Ezequiel and myself only have access to Z1 boards, so
we would like to support this stepping at least until all of us have
access to A0 boards. If we don't do this, then we need to keep an ugly
pile of out-of-tree patches just to get our boards running, which is
clearly not the best way of ensuring that mainline has all the
necessary fixes.

So, I would like to see the Z1 stepping supported for now, and have the
freedom to remove its support later once we are all ready to switch to
A0.

For 375 and 38x, we have the chance of having started the mainlining
process very early compared to the life cycle of the SoC, and part of
the consequences of this chance is that we work on early steppings. It
would seem weird for the kernel community to ask silicon vendors to
mainline their code earlier, and at the same time refuse workarounds
needed to bring up early SoC variants.

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH 5/6] ARM: mvebu: Add thermal quirk for the Armada 375 DB board

[Andrew Lunn]

> So, I would like to see the Z1 stepping supported for now, and have the
> freedom to remove its support later once we are all ready to switch to
> A0.

Hi Thomas

Sounds like a good plan. Try to keep all the mess to support Z1 in one
place so that it can be easily taken out once people have A0.

       Andrew

Re: [PATCH 5/6] ARM: mvebu: Add thermal quirk for the Armada 375 DB board

[Jason Cooper]

On Wed, Apr 16, 2014 at 06:34:47PM +0200, Andrew Lunn wrote:
> > So, I would like to see the Z1 stepping supported for now, and have the
> > freedom to remove its support later once we are all ready to switch to
> > A0.
> 
> Hi Thomas
> 
> Sounds like a good plan. Try to keep all the mess to support Z1 in one
> place so that it can be easily taken out once people have A0.

Agreed.  Thomas, do you think you'll be able to get a definitive answer
from Marvell re the number of Z1 boards in the wild?  Once they've moved
to the A0, of course.

Unless we can get a hard answer on that, I doubt we'll ever withdraw
support for the Z1.  Not that that's a bad thing, just trying to be
realistic.  If we add code, expect to support it.

And keeping it all in one place is kind of an impossibility.  Just for
thermal Z1, there are changes to the binding docs, the thermal driver,
and the soc code.

I'd say it's more important to keep it clean, with dts files targeting
the A0+ SoCs, and the Z1 being the exception case(s).

thx,

Jason.

Re: [PATCH 5/6] ARM: mvebu: Add thermal quirk for the Armada 375 DB board

[Thomas Petazzoni]

Dear Jason Cooper,

On Wed, 16 Apr 2014 12:55:48 -0400, Jason Cooper wrote:

> > Sounds like a good plan. Try to keep all the mess to support Z1 in one
> > place so that it can be easily taken out once people have A0.
> 
> Agreed.  Thomas, do you think you'll be able to get a definitive answer
> from Marvell re the number of Z1 boards in the wild?  Once they've moved
> to the A0, of course.

My understanding is that Marvell has never been interested in having
mainline support for the Z1 stepping. It just happens to be a necessary
step to make progress with the general goal of supporting 375 in
mainline, but I don't expect Marvell to be interested in supporting the
Z1 boards in the wild.

> Unless we can get a hard answer on that, I doubt we'll ever withdraw
> support for the Z1.  Not that that's a bad thing, just trying to be
> realistic.  If we add code, expect to support it.

Yes, indeed.

> And keeping it all in one place is kind of an impossibility.  Just for
> thermal Z1, there are changes to the binding docs, the thermal driver,
> and the soc code.
> 
> I'd say it's more important to keep it clean, with dts files targeting
> the A0+ SoCs, and the Z1 being the exception case(s).

That's what we've tried to do so far: the thermal driver works for the
A0+ by default, and only as an exception supports Z1.

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[PATCH 6/6] ARM: mvebu: Enable the thermal sensor in Armada 375 SoC

[Ezequiel Garcia]

This commit enables the thermal sensor found in Armada 375 SoCs.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 arch/arm/boot/dts/armada-375.dtsi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/armada-375.dtsi b/arch/arm/boot/dts/armada-375.dtsi
index 3877693f..7467f9d 100644
--- a/arch/arm/boot/dts/armada-375.dtsi
+++ b/arch/arm/boot/dts/armada-375.dtsi
@@ -391,6 +391,12 @@
 				status = "disabled";
 			};
 
+			thermal@e8078 {
+				compatible = "marvell,armada375-thermal";
+				reg = <0xe8078 0x4>, <0xe807c 0x8>;
+				status = "okay";
+			};
+
 			coreclk: mvebu-sar@e8204 {
 				compatible = "marvell,armada-375-core-clock";
 				reg = <0xe8204 0x04>;
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html