[PATCH 0/2] Farewell Armada 375 Z1 support

[PATCH 0/2] Farewell Armada 375 Z1 support

[Ezequiel Garcia]

Hi all,

As we recently discussed [1], the Armada 375 Z1 stepping is no longer supported.

Once the 25 MHz reference clock patches are merged, watchdog and clocksource
will stop working properly [2]. Also, the ethernet driver for Armada 375 SoC
(mvpp2) never supported the Z1 stepping.

These two patches remove the thermal quirk needed for Z1, which currently is
the only piece that explicitly mentions Z1.

[1] http://www.spinics.net/lists/arm-kernel/msg374443.html
[2] http://www.spinics.net/lists/arm-kernel/msg371524.html

Ezequiel Garcia (2):
  thermal: armada: Remove support for A375-Z1 SoC
  ARM: mvebu: Remove thermal quirk for Z1 SoC

 .../devicetree/bindings/thermal/armada-thermal.txt |  8 ---
 arch/arm/mach-mvebu/board-v7.c                     | 66 +---------------------
 drivers/thermal/armada_thermal.c                   | 20 -------
 3 files changed, 1 insertion(+), 93 deletions(-)

-- 
2.1.0

[PATCH 1/2] thermal: armada: Remove support for A375-Z1 SoC

[Ezequiel Garcia]

The Armada 375 Z1 SoC revision is no longer supported. This commit
removes the quirk needed for the thermal sensor.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 .../devicetree/bindings/thermal/armada-thermal.txt   |  8 --------
 drivers/thermal/armada_thermal.c                     | 20 --------------------
 2 files changed, 28 deletions(-)

diff --git a/Documentation/devicetree/bindings/thermal/armada-thermal.txt b/Documentation/devicetree/bindings/thermal/armada-thermal.txt
index 4cf0249..4698e0e 100644
--- a/Documentation/devicetree/bindings/thermal/armada-thermal.txt
+++ b/Documentation/devicetree/bindings/thermal/armada-thermal.txt
@@ -5,17 +5,9 @@ Required properties:
 - compatible:	Should be set to one of the following:
 		marvell,armada370-thermal
 		marvell,armada375-thermal
-		marvell,armada375-z1-thermal
 		marvell,armada380-thermal
 		marvell,armadaxp-thermal
 
-		Note: As the name suggests, "marvell,armada375-z1-thermal"
-		applies for the SoC Z1 stepping only. On such stepping
-		some quirks need to be done and the register offset differs
-		from the one in the A0 stepping.
-		The operating system may auto-detect the SoC stepping and
-		update the compatible and register offsets 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 9d1420a..9c8e783 100644
--- a/drivers/thermal/armada_thermal.c
+++ b/drivers/thermal/armada_thermal.c
@@ -35,10 +35,6 @@
 #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 A375_UNIT_CONTROL_SHIFT		27
 #define A375_UNIT_CONTROL_MASK		0x7
 #define A375_READOUT_INVERT		BIT(15)
@@ -124,24 +120,12 @@ static void armada375_init_sensor(struct platform_device *pdev,
 				  struct armada_thermal_priv *priv)
 {
 	unsigned long reg;
-	bool quirk_needed =
-		!!of_device_is_compatible(pdev->dev.of_node,
-					  "marvell,armada375-z1-thermal");
-
-	if (quirk_needed) {
-		/* Ensure these registers have the default (reset) values */
-		writel(A375_Z1_CAL_RESET_LSB, priv->control);
-		writel(A375_Z1_CAL_RESET_MSB, priv->control + 0x4);
-	}
 
 	reg = readl(priv->control + 4);
 	reg &= ~(A375_UNIT_CONTROL_MASK << A375_UNIT_CONTROL_SHIFT);
 	reg &= ~A375_READOUT_INVERT;
 	reg &= ~A375_HW_RESETn;
 
-	if (quirk_needed)
-		reg |= A375_Z1_WORKAROUND_BIT;
-
 	writel(reg, priv->control + 4);
 	mdelay(20);
 
@@ -260,10 +244,6 @@ static const struct of_device_id armada_thermal_id_table[] = {
 		.data       = &armada375_data,
 	},
 	{
-		.compatible = "marvell,armada375-z1-thermal",
-		.data       = &armada375_data,
-	},
-	{
 		.compatible = "marvell,armada380-thermal",
 		.data       = &armada380_data,
 	},
-- 
2.1.0

Re: [PATCH 1/2] thermal: armada: Remove support for A375-Z1 SoC

[Jason Cooper]

Ezequiel,

On Tue, Nov 04, 2014 at 01:00:38PM -0300, Ezequiel Garcia wrote:
> The Armada 375 Z1 SoC revision is no longer supported. This commit
> removes the quirk needed for the thermal sensor.
> 
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> ---
>  .../devicetree/bindings/thermal/armada-thermal.txt   |  8 --------
>  drivers/thermal/armada_thermal.c                     | 20 --------------------
>  2 files changed, 28 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/thermal/armada-thermal.txt b/Documentation/devicetree/bindings/thermal/armada-thermal.txt
> index 4cf0249..4698e0e 100644
> --- a/Documentation/devicetree/bindings/thermal/armada-thermal.txt
> +++ b/Documentation/devicetree/bindings/thermal/armada-thermal.txt
> @@ -5,17 +5,9 @@ Required properties:
>  - compatible:	Should be set to one of the following:
>  		marvell,armada370-thermal
>  		marvell,armada375-thermal
> -		marvell,armada375-z1-thermal
>  		marvell,armada380-thermal
>  		marvell,armadaxp-thermal
>  
> -		Note: As the name suggests, "marvell,armada375-z1-thermal"
> -		applies for the SoC Z1 stepping only. On such stepping
> -		some quirks need to be done and the register offset differs
> -		from the one in the A0 stepping.
> -		The operating system may auto-detect the SoC stepping and
> -		update the compatible and register offsets at runtime.
> -
>  - reg:		Device's register space.
>  		Two entries are expected, see the examples below.
>  		The first one is required for the sensor register;

I've no problem with removing support for the z1 stepping from the
kernel.  However, I don't think we should erase it from binding docs.
I'm not sure what the DT maintainers think is the appropriate action
here, but I'm thinking we could add a 'Deprecated' section at the end of
the doc and move these hunks there.  With a little rewording of course.

I'm primarily concerned about users with older dtbs looking to upgrade,
"Hey, wtf is marvell,armada375-z1-thermal?  I looked in the binding docs
and there's nothing there, do I replace it with marvell,armada375-thermal?"

thx,

Jason.

Re: [PATCH 1/2] thermal: armada: Remove support for A375-Z1 SoC

[Ezequiel Garcia]

[-- Attachment #1: Type: text/plain, Size: 2757 bytes --]

On 11/07/2014 12:26 AM, Jason Cooper wrote:
> Ezequiel,
> 
> On Tue, Nov 04, 2014 at 01:00:38PM -0300, Ezequiel Garcia wrote:
>> The Armada 375 Z1 SoC revision is no longer supported. This commit
>> removes the quirk needed for the thermal sensor.
>>
>> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
>> ---
>>  .../devicetree/bindings/thermal/armada-thermal.txt   |  8 --------
>>  drivers/thermal/armada_thermal.c                     | 20 --------------------
>>  2 files changed, 28 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/thermal/armada-thermal.txt b/Documentation/devicetree/bindings/thermal/armada-thermal.txt
>> index 4cf0249..4698e0e 100644
>> --- a/Documentation/devicetree/bindings/thermal/armada-thermal.txt
>> +++ b/Documentation/devicetree/bindings/thermal/armada-thermal.txt
>> @@ -5,17 +5,9 @@ Required properties:
>>  - compatible:	Should be set to one of the following:
>>  		marvell,armada370-thermal
>>  		marvell,armada375-thermal
>> -		marvell,armada375-z1-thermal
>>  		marvell,armada380-thermal
>>  		marvell,armadaxp-thermal
>>  
>> -		Note: As the name suggests, "marvell,armada375-z1-thermal"
>> -		applies for the SoC Z1 stepping only. On such stepping
>> -		some quirks need to be done and the register offset differs
>> -		from the one in the A0 stepping.
>> -		The operating system may auto-detect the SoC stepping and
>> -		update the compatible and register offsets at runtime.
>> -
>>  - reg:		Device's register space.
>>  		Two entries are expected, see the examples below.
>>  		The first one is required for the sensor register;
> 
> I've no problem with removing support for the z1 stepping from the
> kernel.  However, I don't think we should erase it from binding docs.
> I'm not sure what the DT maintainers think is the appropriate action
> here, but I'm thinking we could add a 'Deprecated' section at the end of
> the doc and move these hunks there.  With a little rewording of course.
> 
> I'm primarily concerned about users with older dtbs looking to upgrade,
> "Hey, wtf is marvell,armada375-z1-thermal?  I looked in the binding docs
> and there's nothing there, do I replace it with marvell,armada375-thermal?"
> 

We can do that if you think it's really useful. However, I think we've
designed this so *nobody* would actually have to put the z1 compatible
string. The mvebu quirk (tries) to auto-detect it from the revision
register and hot fix the compatible string.

Moreover, I'm not at all sure *anyone* would have a Z1 board except
early developers like us. Am I being too naive here?

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 1/2] thermal: armada: Remove support for A375-Z1 SoC

[Jason Cooper]

Ezequiel,

+ Mark, Grant (sorry, thought you were Cc'd on the original patch)

On Fri, Nov 07, 2014 at 09:41:19AM -0300, Ezequiel Garcia wrote:
> On 11/07/2014 12:26 AM, Jason Cooper wrote:
> > Ezequiel,
> > 
> > On Tue, Nov 04, 2014 at 01:00:38PM -0300, Ezequiel Garcia wrote:
> >> The Armada 375 Z1 SoC revision is no longer supported. This commit
> >> removes the quirk needed for the thermal sensor.
> >>
> >> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> >> ---
> >>  .../devicetree/bindings/thermal/armada-thermal.txt   |  8 --------
> >>  drivers/thermal/armada_thermal.c                     | 20 --------------------
> >>  2 files changed, 28 deletions(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/thermal/armada-thermal.txt b/Documentation/devicetree/bindings/thermal/armada-thermal.txt
> >> index 4cf0249..4698e0e 100644
> >> --- a/Documentation/devicetree/bindings/thermal/armada-thermal.txt
> >> +++ b/Documentation/devicetree/bindings/thermal/armada-thermal.txt
> >> @@ -5,17 +5,9 @@ Required properties:
> >>  - compatible:	Should be set to one of the following:
> >>  		marvell,armada370-thermal
> >>  		marvell,armada375-thermal
> >> -		marvell,armada375-z1-thermal
> >>  		marvell,armada380-thermal
> >>  		marvell,armadaxp-thermal
> >>  
> >> -		Note: As the name suggests, "marvell,armada375-z1-thermal"
> >> -		applies for the SoC Z1 stepping only. On such stepping
> >> -		some quirks need to be done and the register offset differs
> >> -		from the one in the A0 stepping.
> >> -		The operating system may auto-detect the SoC stepping and
> >> -		update the compatible and register offsets at runtime.
> >> -
> >>  - reg:		Device's register space.
> >>  		Two entries are expected, see the examples below.
> >>  		The first one is required for the sensor register;
> > 
> > I've no problem with removing support for the z1 stepping from the
> > kernel.  However, I don't think we should erase it from binding docs.
> > I'm not sure what the DT maintainers think is the appropriate action
> > here, but I'm thinking we could add a 'Deprecated' section at the end of
> > the doc and move these hunks there.  With a little rewording of course.
> > 
> > I'm primarily concerned about users with older dtbs looking to upgrade,
> > "Hey, wtf is marvell,armada375-z1-thermal?  I looked in the binding docs
> > and there's nothing there, do I replace it with marvell,armada375-thermal?"
> > 
> 
> We can do that if you think it's really useful. However, I think we've
> designed this so *nobody* would actually have to put the z1 compatible
> string. The mvebu quirk (tries) to auto-detect it from the revision
> register and hot fix the compatible string.
> 
> Moreover, I'm not at all sure *anyone* would have a Z1 board except
> early developers like us. Am I being too naive here?

I'm primarily concerned about dtbs in the wild which have that
compatible string.  But as you mention, and a quick grep proves, this
compatible string was never in a dts{i} file.  At least not in the
kernel tree.

It's not a rush, so I'd like to hear from Mark/Grant what they prefer
for this sort of situation.  It'll be in the git history either way, but
not everyone goes there when the current docs don't mention something.
:)

thx,

Jason.

Re: [PATCH 1/2] thermal: armada: Remove support for A375-Z1 SoC

[Thomas Petazzoni]

Dear Ezequiel Garcia,

On Fri, 07 Nov 2014 09:41:19 -0300, Ezequiel Garcia wrote:

> We can do that if you think it's really useful. However, I think we've
> designed this so *nobody* would actually have to put the z1 compatible
> string. The mvebu quirk (tries) to auto-detect it from the revision
> register and hot fix the compatible string.

Correct.

> Moreover, I'm not at all sure *anyone* would have a Z1 board except
> early developers like us. Am I being too naive here?

I confirm: those early revisions have only been provided to a very
small selection of persons, mainly within Marvell, and only a few
external partners which are all well aware of the early status of such
SoC revisions.

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

Re: [PATCH 1/2] thermal: armada: Remove support for A375-Z1 SoC

[Jason Cooper]

On Tue, Nov 04, 2014 at 01:00:38PM -0300, Ezequiel Garcia wrote:
> The Armada 375 Z1 SoC revision is no longer supported. This commit
> removes the quirk needed for the thermal sensor.
> 
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> ---
>  .../devicetree/bindings/thermal/armada-thermal.txt   |  8 --------
>  drivers/thermal/armada_thermal.c                     | 20 --------------------
>  2 files changed, 28 deletions(-)

As Thomas and Ezequiel rightfully mentioned, the compatible string below
has never been in a dts file.  Our arch code detected the Z1, and
changed the compatible on the fly.  As a result, there shouldn't be any
dtbs in the wild with this compatible string.  If there are, it will be
held by the few developers (Marvell, partners) that know what they are
doing.

Acked-by: Jason Cooper <jason@lakedaemon.net>

thx,

Jason.

> diff --git a/Documentation/devicetree/bindings/thermal/armada-thermal.txt b/Documentation/devicetree/bindings/thermal/armada-thermal.txt
> index 4cf0249..4698e0e 100644
> --- a/Documentation/devicetree/bindings/thermal/armada-thermal.txt
> +++ b/Documentation/devicetree/bindings/thermal/armada-thermal.txt
> @@ -5,17 +5,9 @@ Required properties:
>  - compatible:	Should be set to one of the following:
>  		marvell,armada370-thermal
>  		marvell,armada375-thermal
> -		marvell,armada375-z1-thermal
>  		marvell,armada380-thermal
>  		marvell,armadaxp-thermal
>  
> -		Note: As the name suggests, "marvell,armada375-z1-thermal"
> -		applies for the SoC Z1 stepping only. On such stepping
> -		some quirks need to be done and the register offset differs
> -		from the one in the A0 stepping.
> -		The operating system may auto-detect the SoC stepping and
> -		update the compatible and register offsets 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 9d1420a..9c8e783 100644
> --- a/drivers/thermal/armada_thermal.c
> +++ b/drivers/thermal/armada_thermal.c
> @@ -35,10 +35,6 @@
>  #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 A375_UNIT_CONTROL_SHIFT		27
>  #define A375_UNIT_CONTROL_MASK		0x7
>  #define A375_READOUT_INVERT		BIT(15)
> @@ -124,24 +120,12 @@ static void armada375_init_sensor(struct platform_device *pdev,
>  				  struct armada_thermal_priv *priv)
>  {
>  	unsigned long reg;
> -	bool quirk_needed =
> -		!!of_device_is_compatible(pdev->dev.of_node,
> -					  "marvell,armada375-z1-thermal");
> -
> -	if (quirk_needed) {
> -		/* Ensure these registers have the default (reset) values */
> -		writel(A375_Z1_CAL_RESET_LSB, priv->control);
> -		writel(A375_Z1_CAL_RESET_MSB, priv->control + 0x4);
> -	}
>  
>  	reg = readl(priv->control + 4);
>  	reg &= ~(A375_UNIT_CONTROL_MASK << A375_UNIT_CONTROL_SHIFT);
>  	reg &= ~A375_READOUT_INVERT;
>  	reg &= ~A375_HW_RESETn;
>  
> -	if (quirk_needed)
> -		reg |= A375_Z1_WORKAROUND_BIT;
> -
>  	writel(reg, priv->control + 4);
>  	mdelay(20);
>  
> @@ -260,10 +244,6 @@ static const struct of_device_id armada_thermal_id_table[] = {
>  		.data       = &armada375_data,
>  	},
>  	{
> -		.compatible = "marvell,armada375-z1-thermal",
> -		.data       = &armada375_data,
> -	},
> -	{
>  		.compatible = "marvell,armada380-thermal",
>  		.data       = &armada380_data,
>  	},
> -- 
> 2.1.0
> 

Re: [PATCH 1/2] thermal: armada: Remove support for A375-Z1 SoC

[Eduardo Valentin]

[-- Attachment #1: Type: text/plain, Size: 4516 bytes --]

Ezequiel and Jason,

On Sat, Nov 08, 2014 at 10:16:46PM -0500, Jason Cooper wrote:
> On Tue, Nov 04, 2014 at 01:00:38PM -0300, Ezequiel Garcia wrote:
> > The Armada 375 Z1 SoC revision is no longer supported. This commit
> > removes the quirk needed for the thermal sensor.
> > 
> > Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> > ---
> >  .../devicetree/bindings/thermal/armada-thermal.txt   |  8 --------
> >  drivers/thermal/armada_thermal.c                     | 20 --------------------
> >  2 files changed, 28 deletions(-)
> 
> As Thomas and Ezequiel rightfully mentioned, the compatible string below
> has never been in a dts file.  Our arch code detected the Z1, and
> changed the compatible on the fly.  As a result, there shouldn't be any
> dtbs in the wild with this compatible string.  If there are, it will be
> held by the few developers (Marvell, partners) that know what they are
> doing.

First of, I would like to mention that best thing to avoid such
situation is to be careful when documenting dt entries that represent hw
that no one has access to (except internal people).

> 
> Acked-by: Jason Cooper <jason@lakedaemon.net>
> 

OK Jason. I saw you picked patch 2. Does it mean I can queue this one? 

BR, 

Eduardo Valentin

> thx,
> 
> Jason.
> 
> > diff --git a/Documentation/devicetree/bindings/thermal/armada-thermal.txt b/Documentation/devicetree/bindings/thermal/armada-thermal.txt
> > index 4cf0249..4698e0e 100644
> > --- a/Documentation/devicetree/bindings/thermal/armada-thermal.txt
> > +++ b/Documentation/devicetree/bindings/thermal/armada-thermal.txt
> > @@ -5,17 +5,9 @@ Required properties:
> >  - compatible:	Should be set to one of the following:
> >  		marvell,armada370-thermal
> >  		marvell,armada375-thermal
> > -		marvell,armada375-z1-thermal
> >  		marvell,armada380-thermal
> >  		marvell,armadaxp-thermal
> >  
> > -		Note: As the name suggests, "marvell,armada375-z1-thermal"
> > -		applies for the SoC Z1 stepping only. On such stepping
> > -		some quirks need to be done and the register offset differs
> > -		from the one in the A0 stepping.
> > -		The operating system may auto-detect the SoC stepping and
> > -		update the compatible and register offsets 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 9d1420a..9c8e783 100644
> > --- a/drivers/thermal/armada_thermal.c
> > +++ b/drivers/thermal/armada_thermal.c
> > @@ -35,10 +35,6 @@
> >  #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 A375_UNIT_CONTROL_SHIFT		27
> >  #define A375_UNIT_CONTROL_MASK		0x7
> >  #define A375_READOUT_INVERT		BIT(15)
> > @@ -124,24 +120,12 @@ static void armada375_init_sensor(struct platform_device *pdev,
> >  				  struct armada_thermal_priv *priv)
> >  {
> >  	unsigned long reg;
> > -	bool quirk_needed =
> > -		!!of_device_is_compatible(pdev->dev.of_node,
> > -					  "marvell,armada375-z1-thermal");
> > -
> > -	if (quirk_needed) {
> > -		/* Ensure these registers have the default (reset) values */
> > -		writel(A375_Z1_CAL_RESET_LSB, priv->control);
> > -		writel(A375_Z1_CAL_RESET_MSB, priv->control + 0x4);
> > -	}
> >  
> >  	reg = readl(priv->control + 4);
> >  	reg &= ~(A375_UNIT_CONTROL_MASK << A375_UNIT_CONTROL_SHIFT);
> >  	reg &= ~A375_READOUT_INVERT;
> >  	reg &= ~A375_HW_RESETn;
> >  
> > -	if (quirk_needed)
> > -		reg |= A375_Z1_WORKAROUND_BIT;
> > -
> >  	writel(reg, priv->control + 4);
> >  	mdelay(20);
> >  
> > @@ -260,10 +244,6 @@ static const struct of_device_id armada_thermal_id_table[] = {
> >  		.data       = &armada375_data,
> >  	},
> >  	{
> > -		.compatible = "marvell,armada375-z1-thermal",
> > -		.data       = &armada375_data,
> > -	},
> > -	{
> >  		.compatible = "marvell,armada380-thermal",
> >  		.data       = &armada380_data,
> >  	},
> > -- 
> > 2.1.0
> > 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH 1/2] thermal: armada: Remove support for A375-Z1 SoC

[Jason Cooper]

On Thu, Nov 20, 2014 at 03:38:06PM -0400, Eduardo Valentin wrote:
> Ezequiel and Jason,
> 
> On Sat, Nov 08, 2014 at 10:16:46PM -0500, Jason Cooper wrote:
> > On Tue, Nov 04, 2014 at 01:00:38PM -0300, Ezequiel Garcia wrote:
> > > The Armada 375 Z1 SoC revision is no longer supported. This commit
> > > removes the quirk needed for the thermal sensor.
> > > 
> > > Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> > > ---
> > >  .../devicetree/bindings/thermal/armada-thermal.txt   |  8 --------
> > >  drivers/thermal/armada_thermal.c                     | 20 --------------------
> > >  2 files changed, 28 deletions(-)
> > 
> > As Thomas and Ezequiel rightfully mentioned, the compatible string below
> > has never been in a dts file.  Our arch code detected the Z1, and
> > changed the compatible on the fly.  As a result, there shouldn't be any
> > dtbs in the wild with this compatible string.  If there are, it will be
> > held by the few developers (Marvell, partners) that know what they are
> > doing.
> 
> First of, I would like to mention that best thing to avoid such
> situation is to be careful when documenting dt entries that represent hw
> that no one has access to (except internal people).

Agreed, mainline support for an SoC so early in it's lifetime was new
for all of us.  Lesson learned.

> > 
> > Acked-by: Jason Cooper <jason@lakedaemon.net>
> > 
> 
> OK Jason. I saw you picked patch 2. Does it mean I can queue this one? 

Yep.  Take it away.

thx,

Jason.

Re: [PATCH 1/2] thermal: armada: Remove support for A375-Z1 SoC

[Thomas Petazzoni]

Dear Jason Cooper,

On Fri, 21 Nov 2014 15:18:58 -0500, Jason Cooper wrote:

> > First of, I would like to mention that best thing to avoid such
> > situation is to be careful when documenting dt entries that represent hw
> > that no one has access to (except internal people).
> 
> Agreed, mainline support for an SoC so early in it's lifetime was new
> for all of us.  Lesson learned.

So the suggestion would be to not document the DT bindings at all,
until we reach a "stable" hardware that is distributed externally?

Note that the Z1 stepping is not completely internal: a small
selection of early customers have access to it. But it normally never
ends up in final products, it's aimed at allowing those early customers
to start their development soon.

I don't mind adjusting how DT bindings are documented for such early
SoCs stepping. But I really believe it's important to have a way to
handle this situation nicely: we've been asking for years SoC vendors
to start upstreaming their code early. Now that they start to do it, we
shouldn't complain and instead adapt to this situation :-)

Best regards,

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

Re: [PATCH 1/2] thermal: armada: Remove support for A375-Z1 SoC

[Jason Cooper]

On Fri, Nov 21, 2014 at 10:51:22PM +0100, Thomas Petazzoni wrote:
> Dear Jason Cooper,
> 
> On Fri, 21 Nov 2014 15:18:58 -0500, Jason Cooper wrote:
> 
> > > First of, I would like to mention that best thing to avoid such
> > > situation is to be careful when documenting dt entries that represent hw
> > > that no one has access to (except internal people).
> > 
> > Agreed, mainline support for an SoC so early in it's lifetime was new
> > for all of us.  Lesson learned.
> 
> So the suggestion would be to not document the DT bindings at all,
> until we reach a "stable" hardware that is distributed externally?

No, I think it's better to document.  However, we should also document
the fact that the binding is for an early version of the SoC and subject
to incompatible change once the production version of the SoC is
released.  ie, an un-stable binding.

thx,

Jason.

> I don't mind adjusting how DT bindings are documented for such early
> SoCs stepping. But I really believe it's important to have a way to
> handle this situation nicely: we've been asking for years SoC vendors
> to start upstreaming their code early. Now that they start to do it, we
> shouldn't complain and instead adapt to this situation :-)

Does the above proposal meet your expectations?

thx,

Jason.

Re: [PATCH 1/2] thermal: armada: Remove support for A375-Z1 SoC

[Thomas Petazzoni]

Hello,

On Fri, 21 Nov 2014 17:05:03 -0500, Jason Cooper wrote:

> > So the suggestion would be to not document the DT bindings at all,
> > until we reach a "stable" hardware that is distributed externally?
> 
> No, I think it's better to document.  However, we should also document
> the fact that the binding is for an early version of the SoC and subject
> to incompatible change once the production version of the SoC is
> released.  ie, an un-stable binding.

Sounds good.

> > I don't mind adjusting how DT bindings are documented for such early
> > SoCs stepping. But I really believe it's important to have a way to
> > handle this situation nicely: we've been asking for years SoC vendors
> > to start upstreaming their code early. Now that they start to do it, we
> > shouldn't complain and instead adapt to this situation :-)
> 
> Does the above proposal meet your expectations?

Absolutely!

Thanks,

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

[PATCH 2/2] ARM: mvebu: Remove thermal quirk for A375 Z1 revision

[Ezequiel Garcia]

The Armada 375 Z1 SoC revision is no longer supported. This commit
removes the quirk required to "fix" the reg property and the compatible
string of the thermal devicetree node.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 arch/arm/mach-mvebu/board-v7.c | 66 +-----------------------------------------
 1 file changed, 1 insertion(+), 65 deletions(-)

diff --git a/arch/arm/mach-mvebu/board-v7.c b/arch/arm/mach-mvebu/board-v7.c
index 6478626..461e726 100644
--- a/arch/arm/mach-mvebu/board-v7.c
+++ b/arch/arm/mach-mvebu/board-v7.c
@@ -124,76 +124,12 @@ 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;
-	int res;
-
-	/*
-	 * The early SoC Z1 revision needs a quirk to be applied in order
-	 * for the thermal controller to work properly. This quirk breaks
-	 * the thermal support if applied on a SoC that doesn't need it,
-	 * so we enforce the SoC revision to be known.
-	 */
-	res = mvebu_get_soc_id(&dev, &rev);
-	if (res < 0 || (res == 0 && 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 and allow the driver
-		 * the take the necessary action.
-		 */
-		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")) {
+	if (of_machine_is_compatible("marvell,a375-db"))
 		external_abort_quirk();
-		thermal_quirk();
-	}
 
 	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
 }
-- 
2.1.0

Re: [PATCH 2/2] ARM: mvebu: Remove thermal quirk for A375 Z1 revision

[Jason Cooper]

On Tue, Nov 04, 2014 at 01:00:39PM -0300, Ezequiel Garcia wrote:
> The Armada 375 Z1 SoC revision is no longer supported. This commit
> removes the quirk required to "fix" the reg property and the compatible
> string of the thermal devicetree node.
> 
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> ---
>  arch/arm/mach-mvebu/board-v7.c | 66 +-----------------------------------------
>  1 file changed, 1 insertion(+), 65 deletions(-)

Applied to mvebu/soc

thx,

Jason.