Skip to content

Commit f726eaa

Browse files
Jan Bottorffwsakernel
authored andcommitted
i2c: designware: Fix corrupted memory seen in the ISR
When running on a many core ARM64 server, errors were happening in the ISR that looked like corrupted memory. These corruptions would fix themselves if small delays were inserted in the ISR. Errors reported by the driver included "i2c_designware APMC0D0F:00: i2c_dw_xfer_msg: invalid target address" and "i2c_designware APMC0D0F:00:controller timed out" during in-band IPMI SSIF stress tests. The problem was determined to be memory writes in the driver were not becoming visible to all cores when execution rapidly shifted between cores, like when a register write immediately triggers an ISR. Processors with weak memory ordering, like ARM64, make no guarantees about the order normal memory writes become globally visible, unless barrier instructions are used to control ordering. To solve this, regmap accessor functions configured by this driver were changed to use non-relaxed forms of the low-level register access functions, which include a barrier on platforms that require it. This assures memory writes before a controller register access are visible to all cores. The community concluded defaulting to correct operation outweighed defaulting to the small performance gains from using relaxed access functions. Being a low speed device added weight to this choice of default register access behavior. Signed-off-by: Jan Bottorff <janb@os.amperecomputing.com> Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com> Tested-by: Serge Semin <fancer.lancer@gmail.com> Reviewed-by: Serge Semin <fancer.lancer@gmail.com> Signed-off-by: Wolfram Sang <wsa@kernel.org>
1 parent 7b211c7 commit f726eaa

File tree

1 file changed

+8
-8
lines changed

1 file changed

+8
-8
lines changed

drivers/i2c/busses/i2c-designware-common.c

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ static int dw_reg_read(void *context, unsigned int reg, unsigned int *val)
6363
{
6464
struct dw_i2c_dev *dev = context;
6565

66-
*val = readl_relaxed(dev->base + reg);
66+
*val = readl(dev->base + reg);
6767

6868
return 0;
6969
}
@@ -72,7 +72,7 @@ static int dw_reg_write(void *context, unsigned int reg, unsigned int val)
7272
{
7373
struct dw_i2c_dev *dev = context;
7474

75-
writel_relaxed(val, dev->base + reg);
75+
writel(val, dev->base + reg);
7676

7777
return 0;
7878
}
@@ -81,7 +81,7 @@ static int dw_reg_read_swab(void *context, unsigned int reg, unsigned int *val)
8181
{
8282
struct dw_i2c_dev *dev = context;
8383

84-
*val = swab32(readl_relaxed(dev->base + reg));
84+
*val = swab32(readl(dev->base + reg));
8585

8686
return 0;
8787
}
@@ -90,7 +90,7 @@ static int dw_reg_write_swab(void *context, unsigned int reg, unsigned int val)
9090
{
9191
struct dw_i2c_dev *dev = context;
9292

93-
writel_relaxed(swab32(val), dev->base + reg);
93+
writel(swab32(val), dev->base + reg);
9494

9595
return 0;
9696
}
@@ -99,8 +99,8 @@ static int dw_reg_read_word(void *context, unsigned int reg, unsigned int *val)
9999
{
100100
struct dw_i2c_dev *dev = context;
101101

102-
*val = readw_relaxed(dev->base + reg) |
103-
(readw_relaxed(dev->base + reg + 2) << 16);
102+
*val = readw(dev->base + reg) |
103+
(readw(dev->base + reg + 2) << 16);
104104

105105
return 0;
106106
}
@@ -109,8 +109,8 @@ static int dw_reg_write_word(void *context, unsigned int reg, unsigned int val)
109109
{
110110
struct dw_i2c_dev *dev = context;
111111

112-
writew_relaxed(val, dev->base + reg);
113-
writew_relaxed(val >> 16, dev->base + reg + 2);
112+
writew(val, dev->base + reg);
113+
writew(val >> 16, dev->base + reg + 2);
114114

115115
return 0;
116116
}

0 commit comments

Comments
 (0)