Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

Input: raydium_ts_i2c - do not split tx transactions

Raydium device does not like splitting of tx transactions into multiple
messages - one for the register address and one for the actual data. This
results in incorrect behavior on the device side.

This change updates raydium_i2c_read and raydium_i2c_write to create
i2c_msg arrays separately and passes those arrays into raydium_i2c_xfer
which decides based on the address whether the bank switch command should
be sent. The bank switch header is still added by raydium_i2c_read and
raydium_i2c_write to ensure that all these operations are performed as part
of a single I2C transfer. It guarantees that no other transactions are
initiated to any other device on the same bus after the bank switch command
is sent.

Signed-off-by: Furquan Shaikh <furquan@google.com>
Link: https://lore.kernel.org/r/20201205005941.1427643-1-furquan@google.com
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

authored by

Furquan Shaikh and committed by
Dmitry Torokhov
3b384bd6 8c3b55a2

+88 -38
+88 -38
drivers/input/touchscreen/raydium_i2c_ts.c
··· 137 137 bool wake_irq_enabled; 138 138 }; 139 139 140 - static int raydium_i2c_xfer(struct i2c_client *client, 141 - u32 addr, void *data, size_t len, bool is_read) 140 + /* 141 + * Header to be sent for RM_CMD_BANK_SWITCH command. This is used by 142 + * raydium_i2c_{read|send} below. 143 + */ 144 + struct __packed raydium_bank_switch_header { 145 + u8 cmd; 146 + __be32 be_addr; 147 + }; 148 + 149 + static int raydium_i2c_xfer(struct i2c_client *client, u32 addr, 150 + struct i2c_msg *xfer, size_t xfer_count) 142 151 { 143 - struct raydium_bank_switch_header { 144 - u8 cmd; 145 - __be32 be_addr; 146 - } __packed header = { 147 - .cmd = RM_CMD_BANK_SWITCH, 148 - .be_addr = cpu_to_be32(addr), 149 - }; 150 - 151 - u8 reg_addr = addr & 0xff; 152 - 153 - struct i2c_msg xfer[] = { 154 - { 155 - .addr = client->addr, 156 - .len = sizeof(header), 157 - .buf = (u8 *)&header, 158 - }, 159 - { 160 - .addr = client->addr, 161 - .len = 1, 162 - .buf = &reg_addr, 163 - }, 164 - { 165 - .addr = client->addr, 166 - .len = len, 167 - .buf = data, 168 - .flags = is_read ? I2C_M_RD : 0, 169 - } 170 - }; 171 - 152 + int ret; 172 153 /* 173 154 * If address is greater than 255, then RM_CMD_BANK_SWITCH needs to be 174 155 * sent first. Else, skip the header i.e. xfer[0]. 175 156 */ 176 157 int xfer_start_idx = (addr > 0xff) ? 0 : 1; 177 - size_t xfer_count = ARRAY_SIZE(xfer) - xfer_start_idx; 178 - int ret; 158 + xfer_count -= xfer_start_idx; 179 159 180 160 ret = i2c_transfer(client->adapter, &xfer[xfer_start_idx], xfer_count); 181 161 if (likely(ret == xfer_count)) ··· 169 189 { 170 190 int tries = 0; 171 191 int error; 192 + u8 *tx_buf; 193 + u8 reg_addr = addr & 0xff; 194 + 195 + tx_buf = kmalloc(len + 1, GFP_KERNEL); 196 + if (!tx_buf) 197 + return -ENOMEM; 198 + 199 + tx_buf[0] = reg_addr; 200 + memcpy(tx_buf + 1, data, len); 172 201 173 202 do { 174 - error = raydium_i2c_xfer(client, addr, (void *)data, len, 175 - false); 203 + struct raydium_bank_switch_header header = { 204 + .cmd = RM_CMD_BANK_SWITCH, 205 + .be_addr = cpu_to_be32(addr), 206 + }; 207 + 208 + /* 209 + * Perform as a single i2c_transfer transaction to ensure that 210 + * no other I2C transactions are initiated on the bus to any 211 + * other device in between. Initiating transacations to other 212 + * devices after RM_CMD_BANK_SWITCH is sent is known to cause 213 + * issues. This is also why regmap infrastructure cannot be used 214 + * for this driver. Regmap handles page(bank) switch and reads 215 + * as separate i2c_transfer() operations. This can result in 216 + * problems if the Raydium device is on a shared I2C bus. 217 + */ 218 + struct i2c_msg xfer[] = { 219 + { 220 + .addr = client->addr, 221 + .len = sizeof(header), 222 + .buf = (u8 *)&header, 223 + }, 224 + { 225 + .addr = client->addr, 226 + .len = len + 1, 227 + .buf = tx_buf, 228 + }, 229 + }; 230 + 231 + error = raydium_i2c_xfer(client, addr, xfer, ARRAY_SIZE(xfer)); 176 232 if (likely(!error)) 177 233 return 0; 178 234 ··· 222 206 static int raydium_i2c_read(struct i2c_client *client, 223 207 u32 addr, void *data, size_t len) 224 208 { 225 - size_t xfer_len; 226 209 int error; 227 210 228 211 while (len) { 229 - xfer_len = min_t(size_t, len, RM_MAX_READ_SIZE); 230 - error = raydium_i2c_xfer(client, addr, data, xfer_len, true); 212 + u8 reg_addr = addr & 0xff; 213 + struct raydium_bank_switch_header header = { 214 + .cmd = RM_CMD_BANK_SWITCH, 215 + .be_addr = cpu_to_be32(addr), 216 + }; 217 + size_t xfer_len = min_t(size_t, len, RM_MAX_READ_SIZE); 218 + 219 + /* 220 + * Perform as a single i2c_transfer transaction to ensure that 221 + * no other I2C transactions are initiated on the bus to any 222 + * other device in between. Initiating transacations to other 223 + * devices after RM_CMD_BANK_SWITCH is sent is known to cause 224 + * issues. This is also why regmap infrastructure cannot be used 225 + * for this driver. Regmap handles page(bank) switch and writes 226 + * as separate i2c_transfer() operations. This can result in 227 + * problems if the Raydium device is on a shared I2C bus. 228 + */ 229 + struct i2c_msg xfer[] = { 230 + { 231 + .addr = client->addr, 232 + .len = sizeof(header), 233 + .buf = (u8 *)&header, 234 + }, 235 + { 236 + .addr = client->addr, 237 + .len = 1, 238 + .buf = &reg_addr, 239 + }, 240 + { 241 + .addr = client->addr, 242 + .len = xfer_len, 243 + .buf = data, 244 + .flags = I2C_M_RD, 245 + } 246 + }; 247 + 248 + error = raydium_i2c_xfer(client, addr, xfer, ARRAY_SIZE(xfer)); 231 249 if (unlikely(error)) 232 250 return error; 233 251