| Message ID | 1388678476-12875-3-git-send-email-gregory.clement@free-electrons.com |
|---|---|
| State | Superseded |
| Headers | show |
Wolfram, On Thu, Jan 02, 2014 at 05:01:16PM +0100, Gregory CLEMENT wrote: > The first variants of Armada XP SoCs (A0 stepping) have issues related > to the i2c controller which prevent to use the offload mechanism and > lead to a kernel hang during boot. > > The driver now check the revision of the SoC. If the revision is not > more recent than the A0 or if the driver can't get the SoC revision > then it disables the offload mechanism. > > Cc: stable@vger.kernel.org > Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com> > --- > drivers/i2c/busses/i2c-mv64xxx.c | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c > index 8be7e42aa4de..089a3663ad86 100644 > --- a/drivers/i2c/busses/i2c-mv64xxx.c > +++ b/drivers/i2c/busses/i2c-mv64xxx.c > @@ -24,6 +24,7 @@ > #include <linux/clk.h> > #include <linux/err.h> > #include <linux/delay.h> > +#include <linux/mvebu-soc-id.h> > > #define MV64XXX_I2C_ADDR_ADDR(val) ((val & 0x7f) << 1) > #define MV64XXX_I2C_BAUD_DIV_N(val) (val & 0x7) > @@ -779,8 +780,16 @@ mv64xxx_of_config(struct mv64xxx_i2c_data *drv_data, > * Transaction Generator support and the errata fix. > */ > if (of_device_is_compatible(np, "marvell,mv78230-i2c")) { > - drv_data->offload_enabled = true; > + u32 dev, rev; > + > drv_data->errata_delay = true; > + /* > + * Only revison more recent than A0 support offload > + * mechanism. In case we can't get the SoC revision > + * weplay safe and we don't enable it > + */ > + if (!mvebu_get_soc_id(&rev, &dev) && (dev > MV78XX0_A0_REV)) > + drv_data->offload_enabled = true; Since this depends on arch-specific code in the previous patch, I'd like to keep the two of them together in a topic branch. Would you prefer to take both with my Ack, or vice-versa? I'm fine either way. thx, Jason. -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jan 02, 2014 at 01:28:22PM -0500, Jason Cooper wrote: > Wolfram, > > On Thu, Jan 02, 2014 at 05:01:16PM +0100, Gregory CLEMENT wrote: > > The first variants of Armada XP SoCs (A0 stepping) have issues related > > to the i2c controller which prevent to use the offload mechanism and > > lead to a kernel hang during boot. > > > > The driver now check the revision of the SoC. If the revision is not > > more recent than the A0 or if the driver can't get the SoC revision > > then it disables the offload mechanism. > > > > Cc: stable@vger.kernel.org > > Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com> > > --- > > drivers/i2c/busses/i2c-mv64xxx.c | 11 ++++++++++- > > 1 file changed, 10 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c > > index 8be7e42aa4de..089a3663ad86 100644 > > --- a/drivers/i2c/busses/i2c-mv64xxx.c > > +++ b/drivers/i2c/busses/i2c-mv64xxx.c > > @@ -24,6 +24,7 @@ > > #include <linux/clk.h> > > #include <linux/err.h> > > #include <linux/delay.h> > > +#include <linux/mvebu-soc-id.h> > > > > #define MV64XXX_I2C_ADDR_ADDR(val) ((val & 0x7f) << 1) > > #define MV64XXX_I2C_BAUD_DIV_N(val) (val & 0x7) > > @@ -779,8 +780,16 @@ mv64xxx_of_config(struct mv64xxx_i2c_data *drv_data, > > * Transaction Generator support and the errata fix. > > */ > > if (of_device_is_compatible(np, "marvell,mv78230-i2c")) { > > - drv_data->offload_enabled = true; > > + u32 dev, rev; > > + > > drv_data->errata_delay = true; > > + /* > > + * Only revison more recent than A0 support offload > > + * mechanism. In case we can't get the SoC revision > > + * weplay safe and we don't enable it > > + */ > > + if (!mvebu_get_soc_id(&rev, &dev) && (dev > MV78XX0_A0_REV)) Very minor nits: I'd prefer (mvebu_get_soc_id == 0) here, since !mvebu_get_soc_id can easily be read as "if not get soc id" which leads to the assumption the function failed. And the parantheses around the second comparison are superfluous. > > + drv_data->offload_enabled = true; > > Since this depends on arch-specific code in the previous patch, I'd like > to keep the two of them together in a topic branch. Would you prefer to > take both with my Ack, or vice-versa? I'm fine either way. I'd think you better take it: Acked-by: Wolfram Sang <wsa@the-dreams.de>
On 02/01/2014 19:41, Wolfram Sang wrote: > On Thu, Jan 02, 2014 at 01:28:22PM -0500, Jason Cooper wrote: >> Wolfram, >> >> On Thu, Jan 02, 2014 at 05:01:16PM +0100, Gregory CLEMENT wrote: >>> The first variants of Armada XP SoCs (A0 stepping) have issues related >>> to the i2c controller which prevent to use the offload mechanism and >>> lead to a kernel hang during boot. >>> >>> The driver now check the revision of the SoC. If the revision is not >>> more recent than the A0 or if the driver can't get the SoC revision >>> then it disables the offload mechanism. >>> >>> Cc: stable@vger.kernel.org >>> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com> >>> --- >>> drivers/i2c/busses/i2c-mv64xxx.c | 11 ++++++++++- >>> 1 file changed, 10 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c >>> index 8be7e42aa4de..089a3663ad86 100644 >>> --- a/drivers/i2c/busses/i2c-mv64xxx.c >>> +++ b/drivers/i2c/busses/i2c-mv64xxx.c >>> @@ -24,6 +24,7 @@ >>> #include <linux/clk.h> >>> #include <linux/err.h> >>> #include <linux/delay.h> >>> +#include <linux/mvebu-soc-id.h> >>> >>> #define MV64XXX_I2C_ADDR_ADDR(val) ((val & 0x7f) << 1) >>> #define MV64XXX_I2C_BAUD_DIV_N(val) (val & 0x7) >>> @@ -779,8 +780,16 @@ mv64xxx_of_config(struct mv64xxx_i2c_data *drv_data, >>> * Transaction Generator support and the errata fix. >>> */ >>> if (of_device_is_compatible(np, "marvell,mv78230-i2c")) { >>> - drv_data->offload_enabled = true; >>> + u32 dev, rev; >>> + >>> drv_data->errata_delay = true; >>> + /* >>> + * Only revison more recent than A0 support offload >>> + * mechanism. In case we can't get the SoC revision >>> + * weplay safe and we don't enable it >>> + */ >>> + if (!mvebu_get_soc_id(&rev, &dev) && (dev > MV78XX0_A0_REV)) > > Very minor nits: > > I'd prefer (mvebu_get_soc_id == 0) here, since !mvebu_get_soc_id can > easily be read as "if not get soc id" which leads to the assumption the > function failed. yes fair enough >And the parantheses around the second comparison are > superfluous. > I know but I found it clearer with parenthesis but I can remove them. >>> + drv_data->offload_enabled = true; >> >> Since this depends on arch-specific code in the previous patch, I'd like >> to keep the two of them together in a topic branch. Would you prefer to >> take both with my Ack, or vice-versa? I'm fine either way. > > I'd think you better take it: > > Acked-by: Wolfram Sang <wsa@the-dreams.de> > I am going to resubmit a series with the change you asked and your acked-by
diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c index 8be7e42aa4de..089a3663ad86 100644 --- a/drivers/i2c/busses/i2c-mv64xxx.c +++ b/drivers/i2c/busses/i2c-mv64xxx.c @@ -24,6 +24,7 @@ #include <linux/clk.h> #include <linux/err.h> #include <linux/delay.h> +#include <linux/mvebu-soc-id.h> #define MV64XXX_I2C_ADDR_ADDR(val) ((val & 0x7f) << 1) #define MV64XXX_I2C_BAUD_DIV_N(val) (val & 0x7) @@ -779,8 +780,16 @@ mv64xxx_of_config(struct mv64xxx_i2c_data *drv_data, * Transaction Generator support and the errata fix. */ if (of_device_is_compatible(np, "marvell,mv78230-i2c")) { - drv_data->offload_enabled = true; + u32 dev, rev; + drv_data->errata_delay = true; + /* + * Only revison more recent than A0 support offload + * mechanism. In case we can't get the SoC revision + * weplay safe and we don't enable it + */ + if (!mvebu_get_soc_id(&rev, &dev) && (dev > MV78XX0_A0_REV)) + drv_data->offload_enabled = true; } out:
The first variants of Armada XP SoCs (A0 stepping) have issues related to the i2c controller which prevent to use the offload mechanism and lead to a kernel hang during boot. The driver now check the revision of the SoC. If the revision is not more recent than the A0 or if the driver can't get the SoC revision then it disables the offload mechanism. Cc: stable@vger.kernel.org Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com> --- drivers/i2c/busses/i2c-mv64xxx.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)