* Re: [PATCH v2] i2c: aspeed: Update the stop sw state when the bus recovery occurs
2024-06-08 4:36 [PATCH v2] i2c: aspeed: Update the stop sw state when the bus recovery occurs Tommy Huang
@ 2024-06-08 16:19 ` kernel test robot
2024-06-08 17:03 ` kernel test robot
2024-06-27 15:25 ` Andi Shyti
2 siblings, 0 replies; 5+ messages in thread
From: kernel test robot @ 2024-06-08 16:19 UTC (permalink / raw)
To: Tommy Huang, brendan.higgins, benh, joel, andi.shyti, andrew, wsa
Cc: oe-kbuild-all, linux-i2c, openbmc, linux-arm-kernel, linux-aspeed,
linux-kernel, stable, BMC-SW
Hi Tommy,
kernel test robot noticed the following build errors:
[auto build test ERROR on andi-shyti/i2c/i2c-host]
[also build test ERROR on linus/master v6.10-rc2 next-20240607]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Tommy-Huang/i2c-aspeed-Update-the-stop-sw-state-when-the-bus-recovery-occurs/20240608-124429
base: git://git.kernel.org/pub/scm/linux/kernel/git/andi.shyti/linux.git i2c/i2c-host
patch link: https://lore.kernel.org/r/20240608043653.4086647-1-tommy_huang%40aspeedtech.com
patch subject: [PATCH v2] i2c: aspeed: Update the stop sw state when the bus recovery occurs
config: arm-aspeed_g5_defconfig (https://download.01.org/0day-ci/archive/20240609/202406090041.5IMjYB8x-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240609/202406090041.5IMjYB8x-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202406090041.5IMjYB8x-lkp@intel.com/
All error/warnings (new ones prefixed by >>):
>> drivers/i2c/busses/i2c-aspeed.c:28:39: warning: 'struct aspeed_i2c_bus' declared inside parameter list will not be visible outside of this definition or declaration
28 | static void aspeed_i2c_do_stop(struct aspeed_i2c_bus *bus);
| ^~~~~~~~~~~~~~
drivers/i2c/busses/i2c-aspeed.c: In function 'aspeed_i2c_recover_bus':
>> drivers/i2c/busses/i2c-aspeed.c:192:36: error: passing argument 1 of 'aspeed_i2c_do_stop' from incompatible pointer type [-Werror=incompatible-pointer-types]
192 | aspeed_i2c_do_stop(bus);
| ^~~
| |
| struct aspeed_i2c_bus *
drivers/i2c/busses/i2c-aspeed.c:28:55: note: expected 'struct aspeed_i2c_bus *' but argument is of type 'struct aspeed_i2c_bus *'
28 | static void aspeed_i2c_do_stop(struct aspeed_i2c_bus *bus);
| ~~~~~~~~~~~~~~~~~~~~~~~^~~
drivers/i2c/busses/i2c-aspeed.c: At top level:
>> drivers/i2c/busses/i2c-aspeed.c:396:13: error: conflicting types for 'aspeed_i2c_do_stop'; have 'void(struct aspeed_i2c_bus *)'
396 | static void aspeed_i2c_do_stop(struct aspeed_i2c_bus *bus)
| ^~~~~~~~~~~~~~~~~~
drivers/i2c/busses/i2c-aspeed.c:28:13: note: previous declaration of 'aspeed_i2c_do_stop' with type 'void(struct aspeed_i2c_bus *)'
28 | static void aspeed_i2c_do_stop(struct aspeed_i2c_bus *bus);
| ^~~~~~~~~~~~~~~~~~
>> drivers/i2c/busses/i2c-aspeed.c:28:13: warning: 'aspeed_i2c_do_stop' used but never defined
cc1: some warnings being treated as errors
vim +/aspeed_i2c_do_stop +192 drivers/i2c/busses/i2c-aspeed.c
27
> 28 static void aspeed_i2c_do_stop(struct aspeed_i2c_bus *bus);
29
30 /* I2C Register */
31 #define ASPEED_I2C_FUN_CTRL_REG 0x00
32 #define ASPEED_I2C_AC_TIMING_REG1 0x04
33 #define ASPEED_I2C_AC_TIMING_REG2 0x08
34 #define ASPEED_I2C_INTR_CTRL_REG 0x0c
35 #define ASPEED_I2C_INTR_STS_REG 0x10
36 #define ASPEED_I2C_CMD_REG 0x14
37 #define ASPEED_I2C_DEV_ADDR_REG 0x18
38 #define ASPEED_I2C_BYTE_BUF_REG 0x20
39
40 /* Global Register Definition */
41 /* 0x00 : I2C Interrupt Status Register */
42 /* 0x08 : I2C Interrupt Target Assignment */
43
44 /* Device Register Definition */
45 /* 0x00 : I2CD Function Control Register */
46 #define ASPEED_I2CD_MULTI_MASTER_DIS BIT(15)
47 #define ASPEED_I2CD_SDA_DRIVE_1T_EN BIT(8)
48 #define ASPEED_I2CD_M_SDA_DRIVE_1T_EN BIT(7)
49 #define ASPEED_I2CD_M_HIGH_SPEED_EN BIT(6)
50 #define ASPEED_I2CD_SLAVE_EN BIT(1)
51 #define ASPEED_I2CD_MASTER_EN BIT(0)
52
53 /* 0x04 : I2CD Clock and AC Timing Control Register #1 */
54 #define ASPEED_I2CD_TIME_TBUF_MASK GENMASK(31, 28)
55 #define ASPEED_I2CD_TIME_THDSTA_MASK GENMASK(27, 24)
56 #define ASPEED_I2CD_TIME_TACST_MASK GENMASK(23, 20)
57 #define ASPEED_I2CD_TIME_SCL_HIGH_SHIFT 16
58 #define ASPEED_I2CD_TIME_SCL_HIGH_MASK GENMASK(19, 16)
59 #define ASPEED_I2CD_TIME_SCL_LOW_SHIFT 12
60 #define ASPEED_I2CD_TIME_SCL_LOW_MASK GENMASK(15, 12)
61 #define ASPEED_I2CD_TIME_BASE_DIVISOR_MASK GENMASK(3, 0)
62 #define ASPEED_I2CD_TIME_SCL_REG_MAX GENMASK(3, 0)
63 /* 0x08 : I2CD Clock and AC Timing Control Register #2 */
64 #define ASPEED_NO_TIMEOUT_CTRL 0
65
66 /* 0x0c : I2CD Interrupt Control Register &
67 * 0x10 : I2CD Interrupt Status Register
68 *
69 * These share bit definitions, so use the same values for the enable &
70 * status bits.
71 */
72 #define ASPEED_I2CD_INTR_RECV_MASK 0xf000ffff
73 #define ASPEED_I2CD_INTR_SDA_DL_TIMEOUT BIT(14)
74 #define ASPEED_I2CD_INTR_BUS_RECOVER_DONE BIT(13)
75 #define ASPEED_I2CD_INTR_SLAVE_MATCH BIT(7)
76 #define ASPEED_I2CD_INTR_SCL_TIMEOUT BIT(6)
77 #define ASPEED_I2CD_INTR_ABNORMAL BIT(5)
78 #define ASPEED_I2CD_INTR_NORMAL_STOP BIT(4)
79 #define ASPEED_I2CD_INTR_ARBIT_LOSS BIT(3)
80 #define ASPEED_I2CD_INTR_RX_DONE BIT(2)
81 #define ASPEED_I2CD_INTR_TX_NAK BIT(1)
82 #define ASPEED_I2CD_INTR_TX_ACK BIT(0)
83 #define ASPEED_I2CD_INTR_MASTER_ERRORS \
84 (ASPEED_I2CD_INTR_SDA_DL_TIMEOUT | \
85 ASPEED_I2CD_INTR_SCL_TIMEOUT | \
86 ASPEED_I2CD_INTR_ABNORMAL | \
87 ASPEED_I2CD_INTR_ARBIT_LOSS)
88 #define ASPEED_I2CD_INTR_ALL \
89 (ASPEED_I2CD_INTR_SDA_DL_TIMEOUT | \
90 ASPEED_I2CD_INTR_BUS_RECOVER_DONE | \
91 ASPEED_I2CD_INTR_SCL_TIMEOUT | \
92 ASPEED_I2CD_INTR_ABNORMAL | \
93 ASPEED_I2CD_INTR_NORMAL_STOP | \
94 ASPEED_I2CD_INTR_ARBIT_LOSS | \
95 ASPEED_I2CD_INTR_RX_DONE | \
96 ASPEED_I2CD_INTR_TX_NAK | \
97 ASPEED_I2CD_INTR_TX_ACK)
98
99 /* 0x14 : I2CD Command/Status Register */
100 #define ASPEED_I2CD_SCL_LINE_STS BIT(18)
101 #define ASPEED_I2CD_SDA_LINE_STS BIT(17)
102 #define ASPEED_I2CD_BUS_BUSY_STS BIT(16)
103 #define ASPEED_I2CD_BUS_RECOVER_CMD BIT(11)
104
105 /* Command Bit */
106 #define ASPEED_I2CD_M_STOP_CMD BIT(5)
107 #define ASPEED_I2CD_M_S_RX_CMD_LAST BIT(4)
108 #define ASPEED_I2CD_M_RX_CMD BIT(3)
109 #define ASPEED_I2CD_S_TX_CMD BIT(2)
110 #define ASPEED_I2CD_M_TX_CMD BIT(1)
111 #define ASPEED_I2CD_M_START_CMD BIT(0)
112 #define ASPEED_I2CD_MASTER_CMDS_MASK \
113 (ASPEED_I2CD_M_STOP_CMD | \
114 ASPEED_I2CD_M_S_RX_CMD_LAST | \
115 ASPEED_I2CD_M_RX_CMD | \
116 ASPEED_I2CD_M_TX_CMD | \
117 ASPEED_I2CD_M_START_CMD)
118
119 /* 0x18 : I2CD Slave Device Address Register */
120 #define ASPEED_I2CD_DEV_ADDR_MASK GENMASK(6, 0)
121
122 enum aspeed_i2c_master_state {
123 ASPEED_I2C_MASTER_INACTIVE,
124 ASPEED_I2C_MASTER_PENDING,
125 ASPEED_I2C_MASTER_START,
126 ASPEED_I2C_MASTER_TX_FIRST,
127 ASPEED_I2C_MASTER_TX,
128 ASPEED_I2C_MASTER_RX_FIRST,
129 ASPEED_I2C_MASTER_RX,
130 ASPEED_I2C_MASTER_STOP,
131 };
132
133 enum aspeed_i2c_slave_state {
134 ASPEED_I2C_SLAVE_INACTIVE,
135 ASPEED_I2C_SLAVE_START,
136 ASPEED_I2C_SLAVE_READ_REQUESTED,
137 ASPEED_I2C_SLAVE_READ_PROCESSED,
138 ASPEED_I2C_SLAVE_WRITE_REQUESTED,
139 ASPEED_I2C_SLAVE_WRITE_RECEIVED,
140 ASPEED_I2C_SLAVE_STOP,
141 };
142
143 struct aspeed_i2c_bus {
144 struct i2c_adapter adap;
145 struct device *dev;
146 void __iomem *base;
147 struct reset_control *rst;
148 /* Synchronizes I/O mem access to base. */
149 spinlock_t lock;
150 struct completion cmd_complete;
151 u32 (*get_clk_reg_val)(struct device *dev,
152 u32 divisor);
153 unsigned long parent_clk_frequency;
154 u32 bus_frequency;
155 /* Transaction state. */
156 enum aspeed_i2c_master_state master_state;
157 struct i2c_msg *msgs;
158 size_t buf_index;
159 size_t msgs_index;
160 size_t msgs_count;
161 bool send_stop;
162 int cmd_err;
163 /* Protected only by i2c_lock_bus */
164 int master_xfer_result;
165 /* Multi-master */
166 bool multi_master;
167 #if IS_ENABLED(CONFIG_I2C_SLAVE)
168 struct i2c_client *slave;
169 enum aspeed_i2c_slave_state slave_state;
170 #endif /* CONFIG_I2C_SLAVE */
171 };
172
173 static int aspeed_i2c_reset(struct aspeed_i2c_bus *bus);
174
175 static int aspeed_i2c_recover_bus(struct aspeed_i2c_bus *bus)
176 {
177 unsigned long time_left, flags;
178 int ret = 0;
179 u32 command;
180
181 spin_lock_irqsave(&bus->lock, flags);
182 command = readl(bus->base + ASPEED_I2C_CMD_REG);
183
184 if (command & ASPEED_I2CD_SDA_LINE_STS) {
185 /* Bus is idle: no recovery needed. */
186 if (command & ASPEED_I2CD_SCL_LINE_STS)
187 goto out;
188 dev_dbg(bus->dev, "SCL hung (state %x), attempting recovery\n",
189 command);
190
191 reinit_completion(&bus->cmd_complete);
> 192 aspeed_i2c_do_stop(bus);
193 spin_unlock_irqrestore(&bus->lock, flags);
194
195 time_left = wait_for_completion_timeout(
196 &bus->cmd_complete, bus->adap.timeout);
197
198 spin_lock_irqsave(&bus->lock, flags);
199 if (time_left == 0)
200 goto reset_out;
201 else if (bus->cmd_err)
202 goto reset_out;
203 /* Recovery failed. */
204 else if (!(readl(bus->base + ASPEED_I2C_CMD_REG) &
205 ASPEED_I2CD_SCL_LINE_STS))
206 goto reset_out;
207 /* Bus error. */
208 } else {
209 dev_dbg(bus->dev, "SDA hung (state %x), attempting recovery\n",
210 command);
211
212 reinit_completion(&bus->cmd_complete);
213 /* Writes 1 to 8 SCL clock cycles until SDA is released. */
214 writel(ASPEED_I2CD_BUS_RECOVER_CMD,
215 bus->base + ASPEED_I2C_CMD_REG);
216 spin_unlock_irqrestore(&bus->lock, flags);
217
218 time_left = wait_for_completion_timeout(
219 &bus->cmd_complete, bus->adap.timeout);
220
221 spin_lock_irqsave(&bus->lock, flags);
222 if (time_left == 0)
223 goto reset_out;
224 else if (bus->cmd_err)
225 goto reset_out;
226 /* Recovery failed. */
227 else if (!(readl(bus->base + ASPEED_I2C_CMD_REG) &
228 ASPEED_I2CD_SDA_LINE_STS))
229 goto reset_out;
230 }
231
232 out:
233 spin_unlock_irqrestore(&bus->lock, flags);
234
235 return ret;
236
237 reset_out:
238 spin_unlock_irqrestore(&bus->lock, flags);
239
240 return aspeed_i2c_reset(bus);
241 }
242
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2] i2c: aspeed: Update the stop sw state when the bus recovery occurs
2024-06-08 4:36 [PATCH v2] i2c: aspeed: Update the stop sw state when the bus recovery occurs Tommy Huang
2024-06-08 16:19 ` kernel test robot
@ 2024-06-08 17:03 ` kernel test robot
2024-06-27 15:25 ` Andi Shyti
2 siblings, 0 replies; 5+ messages in thread
From: kernel test robot @ 2024-06-08 17:03 UTC (permalink / raw)
To: Tommy Huang, brendan.higgins, benh, joel, andi.shyti, andrew, wsa
Cc: llvm, oe-kbuild-all, linux-i2c, openbmc, linux-arm-kernel,
linux-aspeed, linux-kernel, stable, BMC-SW
Hi Tommy,
kernel test robot noticed the following build errors:
[auto build test ERROR on andi-shyti/i2c/i2c-host]
[also build test ERROR on linus/master v6.10-rc2 next-20240607]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Tommy-Huang/i2c-aspeed-Update-the-stop-sw-state-when-the-bus-recovery-occurs/20240608-124429
base: git://git.kernel.org/pub/scm/linux/kernel/git/andi.shyti/linux.git i2c/i2c-host
patch link: https://lore.kernel.org/r/20240608043653.4086647-1-tommy_huang%40aspeedtech.com
patch subject: [PATCH v2] i2c: aspeed: Update the stop sw state when the bus recovery occurs
config: hexagon-allmodconfig (https://download.01.org/0day-ci/archive/20240609/202406090027.3FWHQLNi-lkp@intel.com/config)
compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project d7d2d4f53fc79b4b58e8d8d08151b577c3699d4a)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240609/202406090027.3FWHQLNi-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202406090027.3FWHQLNi-lkp@intel.com/
All error/warnings (new ones prefixed by >>):
In file included from drivers/i2c/busses/i2c-aspeed.c:14:
In file included from include/linux/i2c.h:19:
In file included from include/linux/regulator/consumer.h:35:
In file included from include/linux/suspend.h:5:
In file included from include/linux/swap.h:9:
In file included from include/linux/memcontrol.h:13:
In file included from include/linux/cgroup.h:26:
In file included from include/linux/kernel_stat.h:9:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:14:
In file included from arch/hexagon/include/asm/io.h:328:
include/asm-generic/io.h:548:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
548 | val = __raw_readb(PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:561:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
561 | val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
37 | #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
| ^
In file included from drivers/i2c/busses/i2c-aspeed.c:14:
In file included from include/linux/i2c.h:19:
In file included from include/linux/regulator/consumer.h:35:
In file included from include/linux/suspend.h:5:
In file included from include/linux/swap.h:9:
In file included from include/linux/memcontrol.h:13:
In file included from include/linux/cgroup.h:26:
In file included from include/linux/kernel_stat.h:9:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:14:
In file included from arch/hexagon/include/asm/io.h:328:
include/asm-generic/io.h:574:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
574 | val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
35 | #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
| ^
In file included from drivers/i2c/busses/i2c-aspeed.c:14:
In file included from include/linux/i2c.h:19:
In file included from include/linux/regulator/consumer.h:35:
In file included from include/linux/suspend.h:5:
In file included from include/linux/swap.h:9:
In file included from include/linux/memcontrol.h:13:
In file included from include/linux/cgroup.h:26:
In file included from include/linux/kernel_stat.h:9:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:14:
In file included from arch/hexagon/include/asm/io.h:328:
include/asm-generic/io.h:585:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
585 | __raw_writeb(value, PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:595:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
595 | __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:605:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
605 | __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
In file included from drivers/i2c/busses/i2c-aspeed.c:14:
In file included from include/linux/i2c.h:19:
In file included from include/linux/regulator/consumer.h:35:
In file included from include/linux/suspend.h:5:
In file included from include/linux/swap.h:9:
In file included from include/linux/memcontrol.h:21:
In file included from include/linux/mm.h:2253:
include/linux/vmstat.h:514:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
514 | return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
| ~~~~~~~~~~~ ^ ~~~
>> drivers/i2c/busses/i2c-aspeed.c:28:39: warning: declaration of 'struct aspeed_i2c_bus' will not be visible outside of this function [-Wvisibility]
28 | static void aspeed_i2c_do_stop(struct aspeed_i2c_bus *bus);
| ^
>> drivers/i2c/busses/i2c-aspeed.c:192:22: error: incompatible pointer types passing 'struct aspeed_i2c_bus *' to parameter of type 'struct aspeed_i2c_bus *' [-Werror,-Wincompatible-pointer-types]
192 | aspeed_i2c_do_stop(bus);
| ^~~
drivers/i2c/busses/i2c-aspeed.c:28:55: note: passing argument to parameter 'bus' here
28 | static void aspeed_i2c_do_stop(struct aspeed_i2c_bus *bus);
| ^
>> drivers/i2c/busses/i2c-aspeed.c:396:13: error: conflicting types for 'aspeed_i2c_do_stop'
396 | static void aspeed_i2c_do_stop(struct aspeed_i2c_bus *bus)
| ^
drivers/i2c/busses/i2c-aspeed.c:28:13: note: previous declaration is here
28 | static void aspeed_i2c_do_stop(struct aspeed_i2c_bus *bus);
| ^
drivers/i2c/busses/i2c-aspeed.c:409:22: error: incompatible pointer types passing 'struct aspeed_i2c_bus *' to parameter of type 'struct aspeed_i2c_bus *' [-Werror,-Wincompatible-pointer-types]
409 | aspeed_i2c_do_stop(bus);
| ^~~
drivers/i2c/busses/i2c-aspeed.c:28:55: note: passing argument to parameter 'bus' here
28 | static void aspeed_i2c_do_stop(struct aspeed_i2c_bus *bus);
| ^
drivers/i2c/busses/i2c-aspeed.c:469:23: error: incompatible pointer types passing 'struct aspeed_i2c_bus *' to parameter of type 'struct aspeed_i2c_bus *' [-Werror,-Wincompatible-pointer-types]
469 | aspeed_i2c_do_stop(bus);
| ^~~
drivers/i2c/busses/i2c-aspeed.c:28:55: note: passing argument to parameter 'bus' here
28 | static void aspeed_i2c_do_stop(struct aspeed_i2c_bus *bus);
| ^
drivers/i2c/busses/i2c-aspeed.c:507:23: error: incompatible pointer types passing 'struct aspeed_i2c_bus *' to parameter of type 'struct aspeed_i2c_bus *' [-Werror,-Wincompatible-pointer-types]
507 | aspeed_i2c_do_stop(bus);
| ^~~
drivers/i2c/busses/i2c-aspeed.c:28:55: note: passing argument to parameter 'bus' here
28 | static void aspeed_i2c_do_stop(struct aspeed_i2c_bus *bus);
| ^
drivers/i2c/busses/i2c-aspeed.c:512:23: error: incompatible pointer types passing 'struct aspeed_i2c_bus *' to parameter of type 'struct aspeed_i2c_bus *' [-Werror,-Wincompatible-pointer-types]
512 | aspeed_i2c_do_stop(bus);
| ^~~
drivers/i2c/busses/i2c-aspeed.c:28:55: note: passing argument to parameter 'bus' here
28 | static void aspeed_i2c_do_stop(struct aspeed_i2c_bus *bus);
| ^
drivers/i2c/busses/i2c-aspeed.c:562:24: error: incompatible pointer types passing 'struct aspeed_i2c_bus *' to parameter of type 'struct aspeed_i2c_bus *' [-Werror,-Wincompatible-pointer-types]
562 | aspeed_i2c_do_stop(bus);
| ^~~
drivers/i2c/busses/i2c-aspeed.c:28:55: note: passing argument to parameter 'bus' here
28 | static void aspeed_i2c_do_stop(struct aspeed_i2c_bus *bus);
| ^
drivers/i2c/busses/i2c-aspeed.c:608:21: error: incompatible pointer types passing 'struct aspeed_i2c_bus *' to parameter of type 'struct aspeed_i2c_bus *' [-Werror,-Wincompatible-pointer-types]
608 | aspeed_i2c_do_stop(bus);
| ^~~
drivers/i2c/busses/i2c-aspeed.c:28:55: note: passing argument to parameter 'bus' here
28 | static void aspeed_i2c_do_stop(struct aspeed_i2c_bus *bus);
| ^
8 warnings and 8 errors generated.
vim +192 drivers/i2c/busses/i2c-aspeed.c
27
> 28 static void aspeed_i2c_do_stop(struct aspeed_i2c_bus *bus);
29
30 /* I2C Register */
31 #define ASPEED_I2C_FUN_CTRL_REG 0x00
32 #define ASPEED_I2C_AC_TIMING_REG1 0x04
33 #define ASPEED_I2C_AC_TIMING_REG2 0x08
34 #define ASPEED_I2C_INTR_CTRL_REG 0x0c
35 #define ASPEED_I2C_INTR_STS_REG 0x10
36 #define ASPEED_I2C_CMD_REG 0x14
37 #define ASPEED_I2C_DEV_ADDR_REG 0x18
38 #define ASPEED_I2C_BYTE_BUF_REG 0x20
39
40 /* Global Register Definition */
41 /* 0x00 : I2C Interrupt Status Register */
42 /* 0x08 : I2C Interrupt Target Assignment */
43
44 /* Device Register Definition */
45 /* 0x00 : I2CD Function Control Register */
46 #define ASPEED_I2CD_MULTI_MASTER_DIS BIT(15)
47 #define ASPEED_I2CD_SDA_DRIVE_1T_EN BIT(8)
48 #define ASPEED_I2CD_M_SDA_DRIVE_1T_EN BIT(7)
49 #define ASPEED_I2CD_M_HIGH_SPEED_EN BIT(6)
50 #define ASPEED_I2CD_SLAVE_EN BIT(1)
51 #define ASPEED_I2CD_MASTER_EN BIT(0)
52
53 /* 0x04 : I2CD Clock and AC Timing Control Register #1 */
54 #define ASPEED_I2CD_TIME_TBUF_MASK GENMASK(31, 28)
55 #define ASPEED_I2CD_TIME_THDSTA_MASK GENMASK(27, 24)
56 #define ASPEED_I2CD_TIME_TACST_MASK GENMASK(23, 20)
57 #define ASPEED_I2CD_TIME_SCL_HIGH_SHIFT 16
58 #define ASPEED_I2CD_TIME_SCL_HIGH_MASK GENMASK(19, 16)
59 #define ASPEED_I2CD_TIME_SCL_LOW_SHIFT 12
60 #define ASPEED_I2CD_TIME_SCL_LOW_MASK GENMASK(15, 12)
61 #define ASPEED_I2CD_TIME_BASE_DIVISOR_MASK GENMASK(3, 0)
62 #define ASPEED_I2CD_TIME_SCL_REG_MAX GENMASK(3, 0)
63 /* 0x08 : I2CD Clock and AC Timing Control Register #2 */
64 #define ASPEED_NO_TIMEOUT_CTRL 0
65
66 /* 0x0c : I2CD Interrupt Control Register &
67 * 0x10 : I2CD Interrupt Status Register
68 *
69 * These share bit definitions, so use the same values for the enable &
70 * status bits.
71 */
72 #define ASPEED_I2CD_INTR_RECV_MASK 0xf000ffff
73 #define ASPEED_I2CD_INTR_SDA_DL_TIMEOUT BIT(14)
74 #define ASPEED_I2CD_INTR_BUS_RECOVER_DONE BIT(13)
75 #define ASPEED_I2CD_INTR_SLAVE_MATCH BIT(7)
76 #define ASPEED_I2CD_INTR_SCL_TIMEOUT BIT(6)
77 #define ASPEED_I2CD_INTR_ABNORMAL BIT(5)
78 #define ASPEED_I2CD_INTR_NORMAL_STOP BIT(4)
79 #define ASPEED_I2CD_INTR_ARBIT_LOSS BIT(3)
80 #define ASPEED_I2CD_INTR_RX_DONE BIT(2)
81 #define ASPEED_I2CD_INTR_TX_NAK BIT(1)
82 #define ASPEED_I2CD_INTR_TX_ACK BIT(0)
83 #define ASPEED_I2CD_INTR_MASTER_ERRORS \
84 (ASPEED_I2CD_INTR_SDA_DL_TIMEOUT | \
85 ASPEED_I2CD_INTR_SCL_TIMEOUT | \
86 ASPEED_I2CD_INTR_ABNORMAL | \
87 ASPEED_I2CD_INTR_ARBIT_LOSS)
88 #define ASPEED_I2CD_INTR_ALL \
89 (ASPEED_I2CD_INTR_SDA_DL_TIMEOUT | \
90 ASPEED_I2CD_INTR_BUS_RECOVER_DONE | \
91 ASPEED_I2CD_INTR_SCL_TIMEOUT | \
92 ASPEED_I2CD_INTR_ABNORMAL | \
93 ASPEED_I2CD_INTR_NORMAL_STOP | \
94 ASPEED_I2CD_INTR_ARBIT_LOSS | \
95 ASPEED_I2CD_INTR_RX_DONE | \
96 ASPEED_I2CD_INTR_TX_NAK | \
97 ASPEED_I2CD_INTR_TX_ACK)
98
99 /* 0x14 : I2CD Command/Status Register */
100 #define ASPEED_I2CD_SCL_LINE_STS BIT(18)
101 #define ASPEED_I2CD_SDA_LINE_STS BIT(17)
102 #define ASPEED_I2CD_BUS_BUSY_STS BIT(16)
103 #define ASPEED_I2CD_BUS_RECOVER_CMD BIT(11)
104
105 /* Command Bit */
106 #define ASPEED_I2CD_M_STOP_CMD BIT(5)
107 #define ASPEED_I2CD_M_S_RX_CMD_LAST BIT(4)
108 #define ASPEED_I2CD_M_RX_CMD BIT(3)
109 #define ASPEED_I2CD_S_TX_CMD BIT(2)
110 #define ASPEED_I2CD_M_TX_CMD BIT(1)
111 #define ASPEED_I2CD_M_START_CMD BIT(0)
112 #define ASPEED_I2CD_MASTER_CMDS_MASK \
113 (ASPEED_I2CD_M_STOP_CMD | \
114 ASPEED_I2CD_M_S_RX_CMD_LAST | \
115 ASPEED_I2CD_M_RX_CMD | \
116 ASPEED_I2CD_M_TX_CMD | \
117 ASPEED_I2CD_M_START_CMD)
118
119 /* 0x18 : I2CD Slave Device Address Register */
120 #define ASPEED_I2CD_DEV_ADDR_MASK GENMASK(6, 0)
121
122 enum aspeed_i2c_master_state {
123 ASPEED_I2C_MASTER_INACTIVE,
124 ASPEED_I2C_MASTER_PENDING,
125 ASPEED_I2C_MASTER_START,
126 ASPEED_I2C_MASTER_TX_FIRST,
127 ASPEED_I2C_MASTER_TX,
128 ASPEED_I2C_MASTER_RX_FIRST,
129 ASPEED_I2C_MASTER_RX,
130 ASPEED_I2C_MASTER_STOP,
131 };
132
133 enum aspeed_i2c_slave_state {
134 ASPEED_I2C_SLAVE_INACTIVE,
135 ASPEED_I2C_SLAVE_START,
136 ASPEED_I2C_SLAVE_READ_REQUESTED,
137 ASPEED_I2C_SLAVE_READ_PROCESSED,
138 ASPEED_I2C_SLAVE_WRITE_REQUESTED,
139 ASPEED_I2C_SLAVE_WRITE_RECEIVED,
140 ASPEED_I2C_SLAVE_STOP,
141 };
142
143 struct aspeed_i2c_bus {
144 struct i2c_adapter adap;
145 struct device *dev;
146 void __iomem *base;
147 struct reset_control *rst;
148 /* Synchronizes I/O mem access to base. */
149 spinlock_t lock;
150 struct completion cmd_complete;
151 u32 (*get_clk_reg_val)(struct device *dev,
152 u32 divisor);
153 unsigned long parent_clk_frequency;
154 u32 bus_frequency;
155 /* Transaction state. */
156 enum aspeed_i2c_master_state master_state;
157 struct i2c_msg *msgs;
158 size_t buf_index;
159 size_t msgs_index;
160 size_t msgs_count;
161 bool send_stop;
162 int cmd_err;
163 /* Protected only by i2c_lock_bus */
164 int master_xfer_result;
165 /* Multi-master */
166 bool multi_master;
167 #if IS_ENABLED(CONFIG_I2C_SLAVE)
168 struct i2c_client *slave;
169 enum aspeed_i2c_slave_state slave_state;
170 #endif /* CONFIG_I2C_SLAVE */
171 };
172
173 static int aspeed_i2c_reset(struct aspeed_i2c_bus *bus);
174
175 static int aspeed_i2c_recover_bus(struct aspeed_i2c_bus *bus)
176 {
177 unsigned long time_left, flags;
178 int ret = 0;
179 u32 command;
180
181 spin_lock_irqsave(&bus->lock, flags);
182 command = readl(bus->base + ASPEED_I2C_CMD_REG);
183
184 if (command & ASPEED_I2CD_SDA_LINE_STS) {
185 /* Bus is idle: no recovery needed. */
186 if (command & ASPEED_I2CD_SCL_LINE_STS)
187 goto out;
188 dev_dbg(bus->dev, "SCL hung (state %x), attempting recovery\n",
189 command);
190
191 reinit_completion(&bus->cmd_complete);
> 192 aspeed_i2c_do_stop(bus);
193 spin_unlock_irqrestore(&bus->lock, flags);
194
195 time_left = wait_for_completion_timeout(
196 &bus->cmd_complete, bus->adap.timeout);
197
198 spin_lock_irqsave(&bus->lock, flags);
199 if (time_left == 0)
200 goto reset_out;
201 else if (bus->cmd_err)
202 goto reset_out;
203 /* Recovery failed. */
204 else if (!(readl(bus->base + ASPEED_I2C_CMD_REG) &
205 ASPEED_I2CD_SCL_LINE_STS))
206 goto reset_out;
207 /* Bus error. */
208 } else {
209 dev_dbg(bus->dev, "SDA hung (state %x), attempting recovery\n",
210 command);
211
212 reinit_completion(&bus->cmd_complete);
213 /* Writes 1 to 8 SCL clock cycles until SDA is released. */
214 writel(ASPEED_I2CD_BUS_RECOVER_CMD,
215 bus->base + ASPEED_I2C_CMD_REG);
216 spin_unlock_irqrestore(&bus->lock, flags);
217
218 time_left = wait_for_completion_timeout(
219 &bus->cmd_complete, bus->adap.timeout);
220
221 spin_lock_irqsave(&bus->lock, flags);
222 if (time_left == 0)
223 goto reset_out;
224 else if (bus->cmd_err)
225 goto reset_out;
226 /* Recovery failed. */
227 else if (!(readl(bus->base + ASPEED_I2C_CMD_REG) &
228 ASPEED_I2CD_SDA_LINE_STS))
229 goto reset_out;
230 }
231
232 out:
233 spin_unlock_irqrestore(&bus->lock, flags);
234
235 return ret;
236
237 reset_out:
238 spin_unlock_irqrestore(&bus->lock, flags);
239
240 return aspeed_i2c_reset(bus);
241 }
242
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2] i2c: aspeed: Update the stop sw state when the bus recovery occurs
2024-06-08 4:36 [PATCH v2] i2c: aspeed: Update the stop sw state when the bus recovery occurs Tommy Huang
2024-06-08 16:19 ` kernel test robot
2024-06-08 17:03 ` kernel test robot
@ 2024-06-27 15:25 ` Andi Shyti
2024-06-29 2:01 ` Tommy Huang
2 siblings, 1 reply; 5+ messages in thread
From: Andi Shyti @ 2024-06-27 15:25 UTC (permalink / raw)
To: Tommy Huang
Cc: brendan.higgins, benh, joel, andrew, wsa, linux-i2c, openbmc,
linux-arm-kernel, linux-aspeed, linux-kernel, stable, BMC-SW
Hi Tommy,
any update on this patch?
Andi
On Sat, Jun 08, 2024 at 12:36:53PM GMT, Tommy Huang wrote:
> When the i2c bus recovery occurs, driver will send i2c stop command
> in the scl low condition. In this case the sw state will still keep
> original situation. Under multi-master usage, i2c bus recovery will
> be called when i2c transfer timeout occurs. Update the stop command
> calling with aspeed_i2c_do_stop function to update master_state.
>
> Fixes: f327c686d3ba ("i2c: aspeed: added driver for Aspeed I2C")
>
> Cc: <stable@vger.kernel.org> # v4.13+
> Signed-off-by: Tommy Huang <tommy_huang@aspeedtech.com>
> ---
> drivers/i2c/busses/i2c-aspeed.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
> index ce8c4846b7fa..be64e419adf0 100644
> --- a/drivers/i2c/busses/i2c-aspeed.c
> +++ b/drivers/i2c/busses/i2c-aspeed.c
> @@ -25,6 +25,8 @@
> #include <linux/reset.h>
> #include <linux/slab.h>
>
> +static void aspeed_i2c_do_stop(struct aspeed_i2c_bus *bus);
> +
> /* I2C Register */
> #define ASPEED_I2C_FUN_CTRL_REG 0x00
> #define ASPEED_I2C_AC_TIMING_REG1 0x04
> @@ -187,7 +189,7 @@ static int aspeed_i2c_recover_bus(struct aspeed_i2c_bus *bus)
> command);
>
> reinit_completion(&bus->cmd_complete);
> - writel(ASPEED_I2CD_M_STOP_CMD, bus->base + ASPEED_I2C_CMD_REG);
> + aspeed_i2c_do_stop(bus);
> spin_unlock_irqrestore(&bus->lock, flags);
>
> time_left = wait_for_completion_timeout(
> --
> 2.25.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 5+ messages in thread* RE: [PATCH v2] i2c: aspeed: Update the stop sw state when the bus recovery occurs
2024-06-27 15:25 ` Andi Shyti
@ 2024-06-29 2:01 ` Tommy Huang
0 siblings, 0 replies; 5+ messages in thread
From: Tommy Huang @ 2024-06-29 2:01 UTC (permalink / raw)
To: Andi Shyti
Cc: brendan.higgins@linux.dev, benh@kernel.crashing.org,
joel@jms.id.au, andrew@codeconstruct.com.au, wsa@kernel.org,
linux-i2c@vger.kernel.org, openbmc@lists.ozlabs.org,
linux-arm-kernel@lists.infradead.org,
linux-aspeed@lists.ozlabs.org, linux-kernel@vger.kernel.org,
stable@vger.kernel.org, BMC-SW
Hi Andi,
I have sent back the mail on 6/11.
But I don't receive your feedback, I post it again in this mail.
" There is a problem to move aspeed_i2c_do_stop() on top.
This function is like with aspeed_i2c_reset function needs the aspeed_i2c_bus structure definition."
BR,
By Tommy
> -----Original Message-----
> From: Andi Shyti <andi.shyti@kernel.org>
> Sent: Thursday, June 27, 2024 11:26 PM
> To: Tommy Huang <tommy_huang@aspeedtech.com>
> Cc: brendan.higgins@linux.dev; benh@kernel.crashing.org; joel@jms.id.au;
> andrew@codeconstruct.com.au; wsa@kernel.org; linux-i2c@vger.kernel.org;
> openbmc@lists.ozlabs.org; linux-arm-kernel@lists.infradead.org;
> linux-aspeed@lists.ozlabs.org; linux-kernel@vger.kernel.org;
> stable@vger.kernel.org; BMC-SW <BMC-SW@aspeedtech.com>
> Subject: Re: [PATCH v2] i2c: aspeed: Update the stop sw state when the bus
> recovery occurs
>
> Hi Tommy,
>
> any update on this patch?
>
> Andi
>
> On Sat, Jun 08, 2024 at 12:36:53PM GMT, Tommy Huang wrote:
> > When the i2c bus recovery occurs, driver will send i2c stop command in
> > the scl low condition. In this case the sw state will still keep
> > original situation. Under multi-master usage, i2c bus recovery will be
> > called when i2c transfer timeout occurs. Update the stop command
> > calling with aspeed_i2c_do_stop function to update master_state.
> >
> > Fixes: f327c686d3ba ("i2c: aspeed: added driver for Aspeed I2C")
> >
> > Cc: <stable@vger.kernel.org> # v4.13+
> > Signed-off-by: Tommy Huang <tommy_huang@aspeedtech.com>
> > ---
> > drivers/i2c/busses/i2c-aspeed.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/i2c/busses/i2c-aspeed.c
> > b/drivers/i2c/busses/i2c-aspeed.c index ce8c4846b7fa..be64e419adf0
> > 100644
> > --- a/drivers/i2c/busses/i2c-aspeed.c
> > +++ b/drivers/i2c/busses/i2c-aspeed.c
> > @@ -25,6 +25,8 @@
> > #include <linux/reset.h>
> > #include <linux/slab.h>
> >
> > +static void aspeed_i2c_do_stop(struct aspeed_i2c_bus *bus);
> > +
> > /* I2C Register */
> > #define ASPEED_I2C_FUN_CTRL_REG 0x00
> > #define ASPEED_I2C_AC_TIMING_REG1 0x04
> > @@ -187,7 +189,7 @@ static int aspeed_i2c_recover_bus(struct
> aspeed_i2c_bus *bus)
> > command);
> >
> > reinit_completion(&bus->cmd_complete);
> > - writel(ASPEED_I2CD_M_STOP_CMD, bus->base +
> ASPEED_I2C_CMD_REG);
> > + aspeed_i2c_do_stop(bus);
> > spin_unlock_irqrestore(&bus->lock, flags);
> >
> > time_left = wait_for_completion_timeout(
> > --
> > 2.25.1
> >
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 5+ messages in thread