Fix i2c bus hang on A0 version of the Armada XP SoCs
Subsystems (5)
cover Fix i2c bus hang on A0 version of the Armada XP SoCs
Hi,
This series is a better approach to fix the i2c bus hang on A0 version of the Armada XP SoCs. It occurred on the early release of the OpenBlocks AX3-4 boards. Indeed 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 first patch add a mean to detect the SoCs version at run-time and the second one use this feature in the driver.
These 2 patches should be applied on 3.13-rc and on stable kernel 3.12 as it fixes a regression introduce by the commit 930ab3d403ae "i2c:
mv64xxx: Add I2C Transaction Generator support".
The first patch could be latter be extend to also be used with dove, kirkwood, orion5x and mv78x00 when there will be merged in mvebu.
Thanks,
Gregory CLEMENT (2):
ARM: mvebu: Add support to get the ID and the revision of a SoC
i2c: mv64xxx: Fix bus hang on A0 version of the Armada XP SoCs
arch/arm/mach-mvebu/Makefile | 2 +- arch/arm/mach-mvebu/mvebu-soc-id.c | 111 +++++++++++++++++++++++++++++++++++++ drivers/i2c/busses/i2c-mv64xxx.c | 11 +++- include/linux/mvebu-soc-id.h | 32 +++++++++++ 4 files changed, 154 insertions(+), 2 deletions(-) create mode 100644 arch/arm/mach-mvebu/mvebu-soc-id.c create mode 100644 include/linux/mvebu-soc-id.h
Signature · 1 line
1.8.1.2
PATCH 1/2 ARM: mvebu: Add support to get the ID and the revision of a SoC
All the mvebu SoCs have information related to their variant and revision that can be read from the PCI control register.
This patch adds support for Armada XP and Armada 370. This reading of the revision and the ID are done before the PCI initialization to avoid any conflicts. Once these data are retrieved, the resources are freed to let the PCI subsystem use it.
--- arch/arm/mach-mvebu/Makefile | 2 +- arch/arm/mach-mvebu/mvebu-soc-id.c | 111 +++++++++++++++++++++++++++++++++++++ include/linux/mvebu-soc-id.h | 32 +++++++++++ 3 files changed, 144 insertions(+), 1 deletion(-) create mode 100644 arch/arm/mach-mvebu/mvebu-soc-id.c create mode 100644 include/linux/mvebu-soc-id.h
arch/arm/mach-mvebu/Makefile +1 -1
@@ -3,7 +3,7 @@ ccflags-$(CONFIG_ARCH_MULTIPLATFORM) := -I$(srctree)/$(src)/include \ AFLAGS_coherency_ll.o := -Wa,-march=armv7-a -obj-y += system-controller.o +obj-y += system-controller.o mvebu-soc-id.o obj-$(CONFIG_MACH_ARMADA_370_XP) += armada-370-xp.o obj-$(CONFIG_ARCH_MVEBU) += coherency.o coherency_ll.o pmsu.o obj-$(CONFIG_SMP) += platsmp.o headsmp.o
arch/arm/mach-mvebu/mvebu-soc-id.c new file +111 -0
111 lines not on this page · raw message
include/linux/mvebu-soc-id.h new file +32 -0
@@ -0,0 +1,32 @@ +/* +* Marvell EBU SoC ID and revision definitions. +* +* Copyright (C) 2014 Marvell Semiconductor +* +* This file is licensed under the terms of the GNU General Public +* License version 2. This program is licensed "as is" without any +* warranty of any kind, whether express or implied. +*/ + +#ifndef __LINUX_MVEBU_SOC_ID_H +#define __LINUX_MVEBU_SOC_ID_H + +/* Armada XP ID */ +#define MV78230_DEV_ID 0x7823 +#define MV78260_DEV_ID 0x7826 +#define MV78460_DEV_ID 0x7846 + +/* Armada XP Revision */ +#define MV78XX0_A0_REV 0x1 +#define MV78XX0_B0_REV 0x2 + +#ifdef CONFIG_ARCH_MVEBU +int mvebu_get_soc_id (u32 *dev,u32 *rev); +#else +int mvebu_get_soc_id (u32 *dev,u32 *rev) +{ +return -1 ; +} +#endif + +#endif /* __LINUX_MVEBU_SOC_ID_H */
Signature · 1 line
1.8.1.2
PATCH 1/2 ARM: mvebu: Add support to get the ID and the revision of a SoC
Quoted 7 lines · Gregory CLEMENT
All the mvebu SoCs have information related to their variant and revision that can be read from the PCI control register.
This patch adds support for Armada XP and Armada 370. This reading of the revision and the ID are done before the PCI initialization to avoid any conflicts. Once these data are retrieved, the resources are freed to let the PCI subsystem use it.
[...]
Quoted 19 lines
include/linux/mvebu-soc-id.h new file +16
-0@@ -0,0 +1,32 @@ +/* +* Marvell EBU SoC ID and revision definitions. +* +* Copyright (C) 2014 Marvell Semiconductor +* +* This file is licensed under the terms of the GNU General Public +* License version 2. This program is licensed "as is" without any +* warranty of any kind, whether express or implied. +*/ + +#ifndef __LINUX_MVEBU_SOC_ID_H +#define __LINUX_MVEBU_SOC_ID_H + +/* Armada XP ID */ +#define MV78230_DEV_ID 0x7823 +#define MV78260_DEV_ID 0x7826
include/linux/mvebu-soc-id.h line 26
Quoted 10 lines
include/linux/mvebu-soc-id.h new file +10
-0+#define MV78460_DEV_ID 0x7846 + +/* Armada XP Revision */ +#define MV78XX0_A0_REV 0x1 +#define MV78XX0_B0_REV 0x2 + +#ifdef CONFIG_ARCH_MVEBU +int mvebu_get_soc_id (u32 *dev,u32 *rev); +#else +int mvebu_get_soc_id (u32 *dev,u32 *rev)
This definition needs to be static inline, assuming anything will actually include this header when !defined(CONFIG_ARCH_MVEBU).
Ben.
Quoted 6 lines
include/linux/mvebu-soc-id.h new file +6
-0+{ +return -1 ; +} +#endif + +#endif /* __LINUX_MVEBU_SOC_ID_H */
Signature · 9 lines
Ben Hutchings Quantity is no substitute for quality, but it's the only one we've got. -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 828 bytes Desc: This is a digitally signed message part URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140112/0c7921e9/attachment.sig>
PATCH 1/2 ARM: mvebu: Add support to get the ID and the revision of a SoC
Hi Ben,
Quoted 33 lines · Ben Hutchings
Quoted 7 lines · Gregory CLEMENT
All the mvebu SoCs have information related to their variant and revision that can be read from the PCI control register.
This patch adds support for Armada XP and Armada 370. This reading of the revision and the ID are done before the PCI initialization to avoid any conflicts. Once these data are retrieved, the resources are freed to let the PCI subsystem use it.
[...]
Quoted 24 lines
include/linux/mvebu-soc-id.h new file +21
-0@@ -0,0 +1,32 @@ +/* +* Marvell EBU SoC ID and revision definitions. +* +* Copyright (C) 2014 Marvell Semiconductor +* +* This file is licensed under the terms of the GNU General Public +* License version 2. This program is licensed "as is" without any +* warranty of any kind, whether express or implied. +*/ + +#ifndef __LINUX_MVEBU_SOC_ID_H +#define __LINUX_MVEBU_SOC_ID_H + +/* Armada XP ID */ +#define MV78230_DEV_ID 0x7823 +#define MV78260_DEV_ID 0x7826 +#define MV78460_DEV_ID 0x7846 + +/* Armada XP Revision */ +#define MV78XX0_A0_REV 0x1 +#define MV78XX0_B0_REV 0x2
Quoted 9 lines
include/linux/mvebu-soc-id.h line 26
Quoted 5 lines
include/linux/mvebu-soc-id.h new file +5
-0+ +#ifdef CONFIG_ARCH_MVEBU +int mvebu_get_soc_id (u32 *dev,u32 *rev); +#else +int mvebu_get_soc_id (u32 *dev,u32 *rev)This definition needs to be static inline, assuming anything will actually include this header when !defined(CONFIG_ARCH_MVEBU).
Your review is appreciate, however there was 4 more version sent since this one. And this issue was already pointed and then fixed in the 3rd version.
Thanks,
Gregory
Quoted 9 lines
Ben.
Quoted 6 lines
include/linux/mvebu-soc-id.h new file +6
-0+{ +return -1 ; +} +#endif + +#endif /* __LINUX_MVEBU_SOC_ID_H */
Signature · 4 lines
Gregory Clement, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com
PATCH 1/2 ARM: mvebu: Add support to get the ID and the revision of a SoC
On Sun, 2014-01-12 at 18:45 +0100, Gregory CLEMENT wrote:
[...]
Quoted 3 lines
Your review is appreciate, however there was 4 more version sent since this one. And this issue was already pointed and then fixed in the 3rd version.
Yes, sorry about that - I'm some way behind on reading the stable list.
Ben.
Signature · 9 lines
Ben Hutchings Quantity is no substitute for quality, but it's the only one we've got. -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 828 bytes Desc: This is a digitally signed message part URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140112/ad14831c/attachment.sig>
PATCH 2/2 i2c: mv64xxx: Fix bus hang on A0 version of the Armada XP SoCs
In reply to Gregory CLEMENTThe 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.
--- drivers/i2c/busses/i2c-mv64xxx.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)
drivers/i2c/busses/i2c-mv64xxx.c +10 -1
@@ -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:
Signature · 1 line
1.8.1.2
PATCH 2/2 i2c: mv64xxx: Fix bus hang on A0 version of the Armada XP SoCs
Wolfram,
Quoted 31 lines · Gregory CLEMENT
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 at vger.kernel.orgSigned-off-by Gregory CLEMENT <[email protected]>--- drivers/i2c/busses/i2c-mv64xxx.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)drivers/i2c/busses/i2c-mv64xxx.c +1
-1@@ -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;
drivers/i2c/busses/i2c-mv64xxx.c mv64xxx_of_config(struct mv64xxx_i2c_data *drv_data, · line 792
Quoted 10 lines
drivers/i2c/busses/i2c-mv64xxx.c +9
-0+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.
PATCH 2/2 i2c: mv64xxx: Fix bus hang on A0 version of the Armada XP SoCs
Quoted 27 lines · Jason Cooper
Wolfram,
Quoted 24 lines · Gregory CLEMENT
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 at vger.kernel.orgSigned-off-by Gregory CLEMENT <[email protected]>--- drivers/i2c/busses/i2c-mv64xxx.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)drivers/i2c/busses/i2c-mv64xxx.c +1
-0@@ -24,6 +24,7 @@#include <linux/clk.h> #include <linux/err.h> #include <linux/delay.h> +#include <linux/mvebu-soc-id.h>
Quoted 16 lines
Quoted 6 lines
drivers/i2c/busses/i2c-mv64xxx.c +0
-0#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" )) {drivers/i2c/busses/i2c-mv64xxx.c mv64xxx_of_config(struct mv64xxx_i2c_data *drv_data, · line 791
Quoted 10 lines
drivers/i2c/busses/i2c-mv64xxx.c +8
-1- 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.
Quoted 5 lines
drivers/i2c/busses/i2c-mv64xxx.c mv64xxx_of_config(struct mv64xxx_i2c_data *drv_data, · line 792
Quoted 1 line
drivers/i2c/busses/i2c-mv64xxx.c +1
-0+ 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:
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140102/f901950c/attachment-0001.sig>
PATCH 2/2 i2c: mv64xxx: Fix bus hang on A0 version of the Armada XP SoCs
Quoted 47 lines · Wolfram Sang
Quoted 27 lines · Jason Cooper
Wolfram,
Quoted 24 lines · Gregory CLEMENT
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 at vger.kernel.orgSigned-off-by Gregory CLEMENT <[email protected]>--- drivers/i2c/busses/i2c-mv64xxx.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)drivers/i2c/busses/i2c-mv64xxx.c +1
-0@@ -24,6 +24,7 @@#include <linux/clk.h> #include <linux/err.h> #include <linux/delay.h> +#include <linux/mvebu-soc-id.h> Quoted 16 lines
Quoted 6 lines
drivers/i2c/busses/i2c-mv64xxx.c +0
-0#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" )) {drivers/i2c/busses/i2c-mv64xxx.c mv64xxx_of_config(struct mv64xxx_i2c_data *drv_data, · line 791
Quoted 10 lines
drivers/i2c/busses/i2c-mv64xxx.c +8
-1- 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:
Quoted 3 lines
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
Quoted 3 lines
And the parantheses around the second comparison are superfluous.
I know but I found it clearer with parenthesis but I can remove them.
Quoted 10 lines
Quoted 5 lines
drivers/i2c/busses/i2c-mv64xxx.c mv64xxx_of_config(struct mv64xxx_i2c_data *drv_data, · line 792
Quoted 1 line
drivers/i2c/busses/i2c-mv64xxx.c +1
-0+ 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 <[email protected]>
I am going to resubmit a series with the change you asked and your acked-by
Signature · 4 lines
Gregory Clement, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com