[PATCH 1/3] dt-bindings: thermal/armada: describe AP806 and CP110

[PATCH 1/3] dt-bindings: thermal/armada: describe AP806 and CP110

[Baruch Siach]

Add compatible strings for AP806 and CP110 that are part of the Armada 8k/7k
line of SoCs.

Add a note on the difference in the size of the control area in different
bindings. This is an existing difference between the Armada 375 binding and
the rest. The new AP806 and CP110 bindings are similar to the existing Armada
375 in this regard.

Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
 Documentation/devicetree/bindings/thermal/armada-thermal.txt | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/thermal/armada-thermal.txt b/Documentation/devicetree/bindings/thermal/armada-thermal.txt
index 24aacf8948c5..5ec944d4138b 100644
--- a/Documentation/devicetree/bindings/thermal/armada-thermal.txt
+++ b/Documentation/devicetree/bindings/thermal/armada-thermal.txt
@@ -7,12 +7,20 @@ Required properties:
 		marvell,armada375-thermal
 		marvell,armada380-thermal
 		marvell,armadaxp-thermal
+		marvell,armada-ap806-thermal
+		marvell,armada-cp110-thermal
 
 - reg:		Device's register space.
 		Two entries are expected, see the examples below.
 		The first one is required for the sensor register;
-		the second one is required for the control register
+		the second one is required for the control area
 		to be used for sensor initialization (a.k.a. calibration).
+		The size of the control area must be 4 for
+		marvell,armada370-thermal, marvell,armada380-thermal, and
+		marvell,armadaxp-thermal. The size must be 8 for
+		marvell,armada375-thermal, marvell,armada-ap806-thermal, and
+		marvell,armada-cp110-thermal.
+
 
 Example:
 
-- 
2.15.0

[PATCH 1/3] dt-bindings: thermal/armada: describe AP806 and CP110

[Baruch Siach]

Add compatible strings for AP806 and CP110 that are part of the Armada 8k/7k
line of SoCs.

Add a note on the difference in the size of the control area in different
bindings. This is an existing difference between the Armada 375 binding and
the rest. The new AP806 and CP110 bindings are similar to the existing Armada
375 in this regard.

Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
 Documentation/devicetree/bindings/thermal/armada-thermal.txt | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/thermal/armada-thermal.txt b/Documentation/devicetree/bindings/thermal/armada-thermal.txt
index 24aacf8948c5..5ec944d4138b 100644
--- a/Documentation/devicetree/bindings/thermal/armada-thermal.txt
+++ b/Documentation/devicetree/bindings/thermal/armada-thermal.txt
@@ -7,12 +7,20 @@ Required properties:
 		marvell,armada375-thermal
 		marvell,armada380-thermal
 		marvell,armadaxp-thermal
+		marvell,armada-ap806-thermal
+		marvell,armada-cp110-thermal
 
 - reg:		Device's register space.
 		Two entries are expected, see the examples below.
 		The first one is required for the sensor register;
-		the second one is required for the control register
+		the second one is required for the control area
 		to be used for sensor initialization (a.k.a. calibration).
+		The size of the control area must be 4 for
+		marvell,armada370-thermal, marvell,armada380-thermal, and
+		marvell,armadaxp-thermal. The size must be 8 for
+		marvell,armada375-thermal, marvell,armada-ap806-thermal, and
+		marvell,armada-cp110-thermal.
+
 
 Example:
 
-- 
2.15.0

[PATCH 2/3] thermal: armada: add support for AP806

[Baruch Siach]

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

Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
 drivers/thermal/armada_thermal.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
index ae75328945f7..1f7f81628040 100644
--- a/drivers/thermal/armada_thermal.c
+++ b/drivers/thermal/armada_thermal.c
@@ -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,
 	}
 }
 
+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);
+}
+
 static bool armada_is_valid(struct armada_thermal_priv *priv)
 {
 	unsigned long reg = readl_relaxed(priv->sensor);
@@ -230,6 +246,18 @@ static const struct armada_thermal_data armada380_data = {
 	.inverted = true,
 };
 
+static const struct armada_thermal_data armada_ap806_data = {
+	.is_valid = armada_is_valid,
+	.init_sensor = armada_ap806_init_sensor,
+	.is_valid_shift = 16,
+	.temp_shift = 0,
+	.temp_mask = 0x3ff,
+	.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",
@@ -247,6 +275,10 @@ static const struct of_device_id armada_thermal_id_table[] = {
 		.compatible = "marvell,armada380-thermal",
 		.data       = &armada380_data,
 	},
+	{
+		.compatible = "marvell,armada-ap806-thermal",
+		.data       = &armada_ap806_data,
+	},
 	{
 		/* sentinel */
 	},
-- 
2.15.0

[PATCH 2/3] thermal: armada: add support for AP806

[Baruch Siach]

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

Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
 drivers/thermal/armada_thermal.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
index ae75328945f7..1f7f81628040 100644
--- a/drivers/thermal/armada_thermal.c
+++ b/drivers/thermal/armada_thermal.c
@@ -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,
 	}
 }
 
+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);
+}
+
 static bool armada_is_valid(struct armada_thermal_priv *priv)
 {
 	unsigned long reg = readl_relaxed(priv->sensor);
@@ -230,6 +246,18 @@ static const struct armada_thermal_data armada380_data = {
 	.inverted = true,
 };
 
+static const struct armada_thermal_data armada_ap806_data = {
+	.is_valid = armada_is_valid,
+	.init_sensor = armada_ap806_init_sensor,
+	.is_valid_shift = 16,
+	.temp_shift = 0,
+	.temp_mask = 0x3ff,
+	.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",
@@ -247,6 +275,10 @@ static const struct of_device_id armada_thermal_id_table[] = {
 		.compatible = "marvell,armada380-thermal",
 		.data       = &armada380_data,
 	},
+	{
+		.compatible = "marvell,armada-ap806-thermal",
+		.data       = &armada_ap806_data,
+	},
 	{
 		/* sentinel */
 	},
-- 
2.15.0

[PATCH 3/3] thermal: armada: add support for CP110

[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 <baruch@tkos.co.il>
---
 drivers/thermal/armada_thermal.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
index 1f7f81628040..542db578ce36 100644
--- a/drivers/thermal/armada_thermal.c
+++ b/drivers/thermal/armada_thermal.c
@@ -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,
@@ -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);
 
 	/* 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 */
 	},
-- 
2.15.0

[PATCH 3/3] thermal: armada: add support for CP110

[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 <baruch@tkos.co.il>
---
 drivers/thermal/armada_thermal.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
index 1f7f81628040..542db578ce36 100644
--- a/drivers/thermal/armada_thermal.c
+++ b/drivers/thermal/armada_thermal.c
@@ -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,
@@ -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);
 
 	/* 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 */
 	},
-- 
2.15.0

Re: [PATCH 3/3] thermal: armada: add support for CP110

[Eduardo Valentin]

On Wed, Nov 22, 2017 at 04:42:05PM +0200, Baruch Siach wrote:
> 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 <baruch@tkos.co.il>
> ---
>  drivers/thermal/armada_thermal.c | 24 ++++++++++++++++++++++--
>  1 file changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
> index 1f7f81628040..542db578ce36 100644
> --- a/drivers/thermal/armada_thermal.c
> +++ b/drivers/thermal/armada_thermal.c
> @@ -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,
> @@ -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?

>  
>  	/* 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 */
>  	},
> -- 
> 2.15.0
> 

[PATCH 3/3] thermal: armada: add support for CP110

[Eduardo Valentin]

On Wed, Nov 22, 2017 at 04:42:05PM +0200, Baruch Siach wrote:
> 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 <baruch@tkos.co.il>
> ---
>  drivers/thermal/armada_thermal.c | 24 ++++++++++++++++++++++--
>  1 file changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
> index 1f7f81628040..542db578ce36 100644
> --- a/drivers/thermal/armada_thermal.c
> +++ b/drivers/thermal/armada_thermal.c
> @@ -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,
> @@ -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?

>  
>  	/* 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 */
>  	},
> -- 
> 2.15.0
> 

Re: [PATCH 3/3] thermal: armada: add support for CP110

[Baruch Siach]

Hi Eduardo,

On Wed, Nov 22, 2017 at 08:23:36AM -0800, Eduardo Valentin wrote:
> On Wed, Nov 22, 2017 at 04:42:05PM +0200, Baruch Siach wrote:
> > 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 <baruch@tkos.co.il>
> > ---
> >  drivers/thermal/armada_thermal.c | 24 ++++++++++++++++++++++--
> >  1 file changed, 22 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
> > index 1f7f81628040..542db578ce36 100644
> > --- a/drivers/thermal/armada_thermal.c
> > +++ b/drivers/thermal/armada_thermal.c
> > @@ -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,
> > @@ -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?

Of course. With control_msb_offset=0, this hunk is a nop. All 
armada_thermal_data instances are static, so any field that is not explicitly 
set should be initialized to zero. The code already relies on this behaviour 
for the is_valid field.

baruch

> >  	/* 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 */
> >  	},

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

[PATCH 3/3] thermal: armada: add support for CP110

[Baruch Siach]

Hi Eduardo,

On Wed, Nov 22, 2017 at 08:23:36AM -0800, Eduardo Valentin wrote:
> On Wed, Nov 22, 2017 at 04:42:05PM +0200, Baruch Siach wrote:
> > 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 <baruch@tkos.co.il>
> > ---
> >  drivers/thermal/armada_thermal.c | 24 ++++++++++++++++++++++--
> >  1 file changed, 22 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
> > index 1f7f81628040..542db578ce36 100644
> > --- a/drivers/thermal/armada_thermal.c
> > +++ b/drivers/thermal/armada_thermal.c
> > @@ -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,
> > @@ -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?

Of course. With control_msb_offset=0, this hunk is a nop. All 
armada_thermal_data instances are static, so any field that is not explicitly 
set should be initialized to zero. The code already relies on this behaviour 
for the is_valid field.

baruch

> >  	/* 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 */
> >  	},

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

Re: [PATCH 2/3] thermal: armada: add support for AP806

[Russell King - ARM Linux]

On Wed, Nov 22, 2017 at 04:42:04PM +0200, Baruch Siach wrote:
> The AP806 component is integrated in the Armada 8k and 7k lines of processors.
> 
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> ---
>  drivers/thermal/armada_thermal.c | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
> index ae75328945f7..1f7f81628040 100644
> --- a/drivers/thermal/armada_thermal.c
> +++ b/drivers/thermal/armada_thermal.c
> @@ -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,
>  	}
>  }
>  
> +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?

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

[PATCH 2/3] thermal: armada: add support for AP806

[Russell King - ARM Linux]

On Wed, Nov 22, 2017 at 04:42:04PM +0200, Baruch Siach wrote:
> The AP806 component is integrated in the Armada 8k and 7k lines of processors.
> 
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> ---
>  drivers/thermal/armada_thermal.c | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
> index ae75328945f7..1f7f81628040 100644
> --- a/drivers/thermal/armada_thermal.c
> +++ b/drivers/thermal/armada_thermal.c
> @@ -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,
>  	}
>  }
>  
> +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?

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

Re: [PATCH 2/3] thermal: armada: add support for AP806

[Baruch Siach]

Hi Russell,

On Thu, Nov 23, 2017 at 03:24:17PM +0000, Russell King - ARM Linux wrote:
> On Wed, Nov 22, 2017 at 04:42:04PM +0200, Baruch Siach wrote:
> > The AP806 component is integrated in the Armada 8k and 7k lines of processors.
> > 
> > Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> > ---
> >  drivers/thermal/armada_thermal.c | 32 ++++++++++++++++++++++++++++++++
> >  1 file changed, 32 insertions(+)
> > 
> > diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
> > index ae75328945f7..1f7f81628040 100644
> > --- a/drivers/thermal/armada_thermal.c
> > +++ b/drivers/thermal/armada_thermal.c
> > @@ -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,
> >  	}
> >  }
> >  
> > +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?

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

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

[PATCH 2/3] thermal: armada: add support for AP806

[Baruch Siach]

Hi Russell,

On Thu, Nov 23, 2017 at 03:24:17PM +0000, Russell King - ARM Linux wrote:
> On Wed, Nov 22, 2017 at 04:42:04PM +0200, Baruch Siach wrote:
> > The AP806 component is integrated in the Armada 8k and 7k lines of processors.
> > 
> > Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> > ---
> >  drivers/thermal/armada_thermal.c | 32 ++++++++++++++++++++++++++++++++
> >  1 file changed, 32 insertions(+)
> > 
> > diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
> > index ae75328945f7..1f7f81628040 100644
> > --- a/drivers/thermal/armada_thermal.c
> > +++ b/drivers/thermal/armada_thermal.c
> > @@ -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,
> >  	}
> >  }
> >  
> > +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?

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

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

Re: [PATCH 2/3] thermal: armada: add support for AP806

[Eduardo Valentin]

On Thu, Nov 23, 2017 at 06:35:10PM +0200, Baruch Siach wrote:
> Hi Russell,
> 
> On Thu, Nov 23, 2017 at 03:24:17PM +0000, Russell King - ARM Linux wrote:
> > On Wed, Nov 22, 2017 at 04:42:04PM +0200, Baruch Siach wrote:
> > > The AP806 component is integrated in the Armada 8k and 7k lines of processors.

<cut>

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

Yes, that is schedulable context.

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

Yes please, send next version without CPU spinning delays.


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

[PATCH 2/3] thermal: armada: add support for AP806

[Eduardo Valentin]

On Thu, Nov 23, 2017 at 06:35:10PM +0200, Baruch Siach wrote:
> Hi Russell,
> 
> On Thu, Nov 23, 2017 at 03:24:17PM +0000, Russell King - ARM Linux wrote:
> > On Wed, Nov 22, 2017 at 04:42:04PM +0200, Baruch Siach wrote:
> > > The AP806 component is integrated in the Armada 8k and 7k lines of processors.

<cut>

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

Yes, that is schedulable context.

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

Yes please, send next version without CPU spinning delays.


> Thanks for reviewing,
> baruch
> 
> -- 
>      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 -