| Message ID | 1397657720-10893-6-git-send-email-ezequiel.garcia@free-electrons.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 >
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
> 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
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
> 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
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.
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
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
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(+)