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@xxxxxxxxxxxxxxx >>> Signed-off-by: Gregory CLEMENT <gregory.clement@xxxxxxxxxxxxxxxxxx> >>> --- >>> 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@xxxxxxxxxxxxx> > I am going to resubmit a series with the change you asked and your acked-by -- Gregory Clement, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html