| Message ID | 20210527173153.16470-1-tiwai@suse.de (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] Input: elants_i2c - Fix NULL dereference at probing | expand |
Hi Takashi,
I love your patch! Perhaps something to improve:
[auto build test WARNING on input/next]
[also build test WARNING on hid/for-next linux/master linus/master v5.13-rc3 next-20210527]
[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]
url: https://github.com/0day-ci/linux/commits/Takashi-Iwai/Input-elants_i2c-Fix-NULL-dereference-at-probing/20210528-013428
base: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
config: s390-randconfig-r023-20210526 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 6505c630407c5feec818f0bb1c284f9eeebf2071)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install s390 cross compiling tool for clang build
# apt-get install binutils-s390x-linux-gnu
# https://github.com/0day-ci/linux/commit/8a6af622a33ab5301cea789e5ff6a9afd9b09828
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Takashi-Iwai/Input-elants_i2c-Fix-NULL-dereference-at-probing/20210528-013428
git checkout 8a6af622a33ab5301cea789e5ff6a9afd9b09828
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=s390
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
In file included from drivers/input/touchscreen/elants_i2c.c:26:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/s390/include/asm/io.h:80:
include/asm-generic/io.h:464:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __raw_readb(PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:477:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/big_endian.h:36:59: note: expanded from macro '__le16_to_cpu'
#define __le16_to_cpu(x) __swab16((__force __u16)(__le16)(x))
^
include/uapi/linux/swab.h:102:54: note: expanded from macro '__swab16'
#define __swab16(x) (__u16)__builtin_bswap16((__u16)(x))
^
In file included from drivers/input/touchscreen/elants_i2c.c:26:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/s390/include/asm/io.h:80:
include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/big_endian.h:34:59: note: expanded from macro '__le32_to_cpu'
#define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
^
include/uapi/linux/swab.h:115:54: note: expanded from macro '__swab32'
#define __swab32(x) (__u32)__builtin_bswap32((__u32)(x))
^
In file included from drivers/input/touchscreen/elants_i2c.c:26:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/s390/include/asm/io.h:80:
include/asm-generic/io.h:501:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writeb(value, PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:511:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:521:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:609:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
readsb(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:617:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
readsw(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:625:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
readsl(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:634:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
writesb(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:643:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
writesw(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:652:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
writesl(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
>> drivers/input/touchscreen/elants_i2c.c:1399:16: warning: cast to smaller integer type 'enum elants_chip_id' from 'const void *' [-Wvoid-pointer-to-enum-cast]
ts->chip_id = (enum elants_chip_id)device_get_match_data(&client->dev);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
13 warnings generated.
vim +1399 drivers/input/touchscreen/elants_i2c.c
1371
1372 static int elants_i2c_probe(struct i2c_client *client,
1373 const struct i2c_device_id *id)
1374 {
1375 union i2c_smbus_data dummy;
1376 struct elants_data *ts;
1377 unsigned long irqflags;
1378 int error;
1379
1380 /* Don't bind to i2c-hid compatible devices, these are handled by the i2c-hid drv. */
1381 if (elants_acpi_is_hid_device(&client->dev)) {
1382 dev_warn(&client->dev, "This device appears to be an I2C-HID device, not binding\n");
1383 return -ENODEV;
1384 }
1385
1386 if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
1387 dev_err(&client->dev, "I2C check functionality error\n");
1388 return -ENXIO;
1389 }
1390
1391 ts = devm_kzalloc(&client->dev, sizeof(struct elants_data), GFP_KERNEL);
1392 if (!ts)
1393 return -ENOMEM;
1394
1395 mutex_init(&ts->sysfs_mutex);
1396 init_completion(&ts->cmd_done);
1397
1398 ts->client = client;
> 1399 ts->chip_id = (enum elants_chip_id)device_get_match_data(&client->dev);
1400 i2c_set_clientdata(client, ts);
1401
1402 ts->vcc33 = devm_regulator_get(&client->dev, "vcc33");
1403 if (IS_ERR(ts->vcc33)) {
1404 error = PTR_ERR(ts->vcc33);
1405 if (error != -EPROBE_DEFER)
1406 dev_err(&client->dev,
1407 "Failed to get 'vcc33' regulator: %d\n",
1408 error);
1409 return error;
1410 }
1411
1412 ts->vccio = devm_regulator_get(&client->dev, "vccio");
1413 if (IS_ERR(ts->vccio)) {
1414 error = PTR_ERR(ts->vccio);
1415 if (error != -EPROBE_DEFER)
1416 dev_err(&client->dev,
1417 "Failed to get 'vccio' regulator: %d\n",
1418 error);
1419 return error;
1420 }
1421
1422 ts->reset_gpio = devm_gpiod_get(&client->dev, "reset", GPIOD_OUT_LOW);
1423 if (IS_ERR(ts->reset_gpio)) {
1424 error = PTR_ERR(ts->reset_gpio);
1425
1426 if (error == -EPROBE_DEFER)
1427 return error;
1428
1429 if (error != -ENOENT && error != -ENOSYS) {
1430 dev_err(&client->dev,
1431 "failed to get reset gpio: %d\n",
1432 error);
1433 return error;
1434 }
1435
1436 ts->keep_power_in_suspend = true;
1437 }
1438
1439 error = elants_i2c_power_on(ts);
1440 if (error)
1441 return error;
1442
1443 error = devm_add_action(&client->dev, elants_i2c_power_off, ts);
1444 if (error) {
1445 dev_err(&client->dev,
1446 "failed to install power off action: %d\n", error);
1447 elants_i2c_power_off(ts);
1448 return error;
1449 }
1450
1451 /* Make sure there is something at this address */
1452 if (i2c_smbus_xfer(client->adapter, client->addr, 0,
1453 I2C_SMBUS_READ, 0, I2C_SMBUS_BYTE, &dummy) < 0) {
1454 dev_err(&client->dev, "nothing at this address\n");
1455 return -ENXIO;
1456 }
1457
1458 error = elants_i2c_initialize(ts);
1459 if (error) {
1460 dev_err(&client->dev, "failed to initialize: %d\n", error);
1461 return error;
1462 }
1463
1464 ts->input = devm_input_allocate_device(&client->dev);
1465 if (!ts->input) {
1466 dev_err(&client->dev, "Failed to allocate input device\n");
1467 return -ENOMEM;
1468 }
1469
1470 ts->input->name = "Elan Touchscreen";
1471 ts->input->id.bustype = BUS_I2C;
1472
1473 /* Multitouch input params setup */
1474
1475 input_set_abs_params(ts->input, ABS_MT_POSITION_X, 0, ts->x_max, 0, 0);
1476 input_set_abs_params(ts->input, ABS_MT_POSITION_Y, 0, ts->y_max, 0, 0);
1477 input_set_abs_params(ts->input, ABS_MT_TOUCH_MAJOR, 0, 255, 0, 0);
1478 input_set_abs_params(ts->input, ABS_MT_PRESSURE, 0, 255, 0, 0);
1479 input_set_abs_params(ts->input, ABS_MT_TOOL_TYPE,
1480 0, MT_TOOL_PALM, 0, 0);
1481
1482 touchscreen_parse_properties(ts->input, true, &ts->prop);
1483
1484 if (ts->chip_id == EKTF3624 && ts->phy_x && ts->phy_y) {
1485 /* calculate resolution from size */
1486 ts->x_res = DIV_ROUND_CLOSEST(ts->prop.max_x, ts->phy_x);
1487 ts->y_res = DIV_ROUND_CLOSEST(ts->prop.max_y, ts->phy_y);
1488 }
1489
1490 input_abs_set_res(ts->input, ABS_MT_POSITION_X, ts->x_res);
1491 input_abs_set_res(ts->input, ABS_MT_POSITION_Y, ts->y_res);
1492 input_abs_set_res(ts->input, ABS_MT_TOUCH_MAJOR, ts->major_res);
1493
1494 error = input_mt_init_slots(ts->input, MAX_CONTACT_NUM,
1495 INPUT_MT_DIRECT | INPUT_MT_DROP_UNUSED);
1496 if (error) {
1497 dev_err(&client->dev,
1498 "failed to initialize MT slots: %d\n", error);
1499 return error;
1500 }
1501
1502 error = input_register_device(ts->input);
1503 if (error) {
1504 dev_err(&client->dev,
1505 "unable to register input device: %d\n", error);
1506 return error;
1507 }
1508
1509 /*
1510 * Platform code (ACPI, DTS) should normally set up interrupt
1511 * for us, but in case it did not let's fall back to using falling
1512 * edge to be compatible with older Chromebooks.
1513 */
1514 irqflags = irq_get_trigger_type(client->irq);
1515 if (!irqflags)
1516 irqflags = IRQF_TRIGGER_FALLING;
1517
1518 error = devm_request_threaded_irq(&client->dev, client->irq,
1519 NULL, elants_i2c_irq,
1520 irqflags | IRQF_ONESHOT,
1521 client->name, ts);
1522 if (error) {
1523 dev_err(&client->dev, "Failed to register interrupt\n");
1524 return error;
1525 }
1526
1527 /*
1528 * Systems using device tree should set up wakeup via DTS,
1529 * the rest will configure device as wakeup source by default.
1530 */
1531 if (!client->dev.of_node)
1532 device_init_wakeup(&client->dev, true);
1533
1534 error = devm_device_add_group(&client->dev, &elants_attribute_group);
1535 if (error) {
1536 dev_err(&client->dev, "failed to create sysfs attributes: %d\n",
1537 error);
1538 return error;
1539 }
1540
1541 return 0;
1542 }
1543
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Hi Takashi,
I love your patch! Perhaps something to improve:
[auto build test WARNING on input/next]
[also build test WARNING on hid/for-next linux/master linus/master v5.13-rc3 next-20210527]
[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]
url: https://github.com/0day-ci/linux/commits/Takashi-Iwai/Input-elants_i2c-Fix-NULL-dereference-at-probing/20210528-013428
base: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
config: arm-randconfig-r015-20210526 (attached as .config)
compiler: arm-linux-gnueabi-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/8a6af622a33ab5301cea789e5ff6a9afd9b09828
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Takashi-Iwai/Input-elants_i2c-Fix-NULL-dereference-at-probing/20210528-013428
git checkout 8a6af622a33ab5301cea789e5ff6a9afd9b09828
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/input/touchscreen/elants_i2c.c:1640:43: warning: initialization of 'const void *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
1640 | { .compatible = "elan,ektf3624", .data = EKTF3624 },
| ^~~~~~~~
drivers/input/touchscreen/elants_i2c.c:1640:43: note: (near initialization for 'elants_of_match[1].data')
vim +1640 drivers/input/touchscreen/elants_i2c.c
1636
1637 #ifdef CONFIG_OF
1638 static const struct of_device_id elants_of_match[] = {
1639 { .compatible = "elan,ekth3500", .data = EKTH3500 },
> 1640 { .compatible = "elan,ektf3624", .data = EKTF3624 },
1641 { /* sentinel */ }
1642 };
1643 MODULE_DEVICE_TABLE(of, elants_of_match);
1644 #endif
1645
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Hi Takashi,
I love your patch! Perhaps something to improve:
[auto build test WARNING on input/next]
[also build test WARNING on hid/for-next linux/master linus/master v5.13-rc3 next-20210527]
[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]
url: https://github.com/0day-ci/linux/commits/Takashi-Iwai/Input-elants_i2c-Fix-NULL-dereference-at-probing/20210528-013428
base: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
config: powerpc64-randconfig-r032-20210527 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 6505c630407c5feec818f0bb1c284f9eeebf2071)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install powerpc64 cross compiling tool for clang build
# apt-get install binutils-powerpc64-linux-gnu
# https://github.com/0day-ci/linux/commit/8a6af622a33ab5301cea789e5ff6a9afd9b09828
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Takashi-Iwai/Input-elants_i2c-Fix-NULL-dereference-at-probing/20210528-013428
git checkout 8a6af622a33ab5301cea789e5ff6a9afd9b09828
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=powerpc64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/input/touchscreen/elants_i2c.c:1639:43: warning: expression which evaluates to zero treated as a null pointer constant of type 'const void *' [-Wnon-literal-null-conversion]
{ .compatible = "elan,ekth3500", .data = EKTH3500 },
^~~~~~~~
>> drivers/input/touchscreen/elants_i2c.c:1640:43: warning: incompatible integer to pointer conversion initializing 'const void *' with an expression of type 'int' [-Wint-conversion]
{ .compatible = "elan,ektf3624", .data = EKTF3624 },
^~~~~~~~
2 warnings generated.
vim +1639 drivers/input/touchscreen/elants_i2c.c
1636
1637 #ifdef CONFIG_OF
1638 static const struct of_device_id elants_of_match[] = {
> 1639 { .compatible = "elan,ekth3500", .data = EKTH3500 },
> 1640 { .compatible = "elan,ektf3624", .data = EKTF3624 },
1641 { /* sentinel */ }
1642 };
1643 MODULE_DEVICE_TABLE(of, elants_of_match);
1644 #endif
1645
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Hi Takashi,
I love your patch! Perhaps something to improve:
[auto build test WARNING on input/next]
[also build test WARNING on hid/for-next linux/master linus/master v5.13-rc3 next-20210527]
[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]
url: https://github.com/0day-ci/linux/commits/Takashi-Iwai/Input-elants_i2c-Fix-NULL-dereference-at-probing/20210528-013428
base: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
config: x86_64-randconfig-s021-20210527 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.3-341-g8af24329-dirty
# https://github.com/0day-ci/linux/commit/8a6af622a33ab5301cea789e5ff6a9afd9b09828
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Takashi-Iwai/Input-elants_i2c-Fix-NULL-dereference-at-probing/20210528-013428
git checkout 8a6af622a33ab5301cea789e5ff6a9afd9b09828
# save the attached .config to linux build tree
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' W=1 ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
>> drivers/input/touchscreen/elants_i2c.c:1639:50: sparse: sparse: Using plain integer as NULL pointer
>> drivers/input/touchscreen/elants_i2c.c:1640:50: sparse: sparse: incorrect type in initializer (different base types) @@ expected void const *data @@ got int @@
drivers/input/touchscreen/elants_i2c.c:1640:50: sparse: expected void const *data
drivers/input/touchscreen/elants_i2c.c:1640:50: sparse: got int
vim +1639 drivers/input/touchscreen/elants_i2c.c
1636
1637 #ifdef CONFIG_OF
1638 static const struct of_device_id elants_of_match[] = {
> 1639 { .compatible = "elan,ekth3500", .data = EKTH3500 },
> 1640 { .compatible = "elan,ektf3624", .data = EKTF3624 },
1641 { /* sentinel */ }
1642 };
1643 MODULE_DEVICE_TABLE(of, elants_of_match);
1644 #endif
1645
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
On Thu, 27 May 2021 19:31:53 +0200, Takashi Iwai wrote: > > The recent change in elants_i2c driver to support more chips > introduced a regression leading to Oops at probing. The driver reads > id->driver_data, but the id may be NULL depending on the device type > the driver gets bound. > > Replace the driver data extraction with the device_get_match_data() > helper, and define the driver data in OF table, too. > > Fixes: 9517b95bdc46 ("Input: elants_i2c - add support for eKTF3624") > BugLink: https://bugzilla.suse.com/show_bug.cgi?id=1186454 > Cc: <stable@vger.kernel.org> > Signed-off-by: Takashi Iwai <tiwai@suse.de> It seems that the cast is missing in elants_of_match[] data. I'll post a v3 patch with the correction. Let me know if that's the only needed fix. thanks, Takashi > --- > v1->v2: Use device_get_match_data() > > drivers/input/touchscreen/elants_i2c.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/input/touchscreen/elants_i2c.c b/drivers/input/touchscreen/elants_i2c.c > index 17540bdb1eaf..29b5bb03cff9 100644 > --- a/drivers/input/touchscreen/elants_i2c.c > +++ b/drivers/input/touchscreen/elants_i2c.c > @@ -1396,7 +1396,7 @@ static int elants_i2c_probe(struct i2c_client *client, > init_completion(&ts->cmd_done); > > ts->client = client; > - ts->chip_id = (enum elants_chip_id)id->driver_data; > + ts->chip_id = (enum elants_chip_id)device_get_match_data(&client->dev); > i2c_set_clientdata(client, ts); > > ts->vcc33 = devm_regulator_get(&client->dev, "vcc33"); > @@ -1636,8 +1636,8 @@ MODULE_DEVICE_TABLE(acpi, elants_acpi_id); > > #ifdef CONFIG_OF > static const struct of_device_id elants_of_match[] = { > - { .compatible = "elan,ekth3500" }, > - { .compatible = "elan,ektf3624" }, > + { .compatible = "elan,ekth3500", .data = EKTH3500 }, > + { .compatible = "elan,ektf3624", .data = EKTF3624 }, > { /* sentinel */ } > }; > MODULE_DEVICE_TABLE(of, elants_of_match); > -- > 2.26.2 >
Hi Takashi, On Thu, May 27, 2021 at 07:31:53PM +0200, Takashi Iwai wrote: > The recent change in elants_i2c driver to support more chips > introduced a regression leading to Oops at probing. The driver reads > id->driver_data, but the id may be NULL depending on the device type > the driver gets bound. > > Replace the driver data extraction with the device_get_match_data() > helper, and define the driver data in OF table, too. > > Fixes: 9517b95bdc46 ("Input: elants_i2c - add support for eKTF3624") > BugLink: https://bugzilla.suse.com/show_bug.cgi?id=1186454 > Cc: <stable@vger.kernel.org> > Signed-off-by: Takashi Iwai <tiwai@suse.de> > --- > v1->v2: Use device_get_match_data() > > drivers/input/touchscreen/elants_i2c.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/input/touchscreen/elants_i2c.c b/drivers/input/touchscreen/elants_i2c.c > index 17540bdb1eaf..29b5bb03cff9 100644 > --- a/drivers/input/touchscreen/elants_i2c.c > +++ b/drivers/input/touchscreen/elants_i2c.c > @@ -1396,7 +1396,7 @@ static int elants_i2c_probe(struct i2c_client *client, Might want to switch to probe_new() to avoid same/similar issue down the road, either in the same patch or in a separate one. > init_completion(&ts->cmd_done); > > ts->client = client; > - ts->chip_id = (enum elants_chip_id)id->driver_data; > + ts->chip_id = (enum elants_chip_id)device_get_match_data(&client->dev); I think this might need to go through an intermediate cast to shut up compiler warnings: ts->chip_id = (enum elants_chip_id)(uintptr_t) device_get_match_data(&client->dev); > i2c_set_clientdata(client, ts); > > ts->vcc33 = devm_regulator_get(&client->dev, "vcc33"); > @@ -1636,8 +1636,8 @@ MODULE_DEVICE_TABLE(acpi, elants_acpi_id); > > #ifdef CONFIG_OF > static const struct of_device_id elants_of_match[] = { > - { .compatible = "elan,ekth3500" }, > - { .compatible = "elan,ektf3624" }, > + { .compatible = "elan,ekth3500", .data = EKTH3500 }, > + { .compatible = "elan,ektf3624", .data = EKTF3624 }, As the bot mentioned this needs a cast. > { /* sentinel */ } > }; > MODULE_DEVICE_TABLE(of, elants_of_match); > -- > 2.26.2 > Thanks.
On Thu, 27 May 2021 23:30:59 +0200, Dmitry Torokhov wrote: > > Hi Takashi, > > On Thu, May 27, 2021 at 07:31:53PM +0200, Takashi Iwai wrote: > > The recent change in elants_i2c driver to support more chips > > introduced a regression leading to Oops at probing. The driver reads > > id->driver_data, but the id may be NULL depending on the device type > > the driver gets bound. > > > > Replace the driver data extraction with the device_get_match_data() > > helper, and define the driver data in OF table, too. > > > > Fixes: 9517b95bdc46 ("Input: elants_i2c - add support for eKTF3624") > > BugLink: https://bugzilla.suse.com/show_bug.cgi?id=1186454 > > Cc: <stable@vger.kernel.org> > > Signed-off-by: Takashi Iwai <tiwai@suse.de> > > --- > > v1->v2: Use device_get_match_data() > > > > drivers/input/touchscreen/elants_i2c.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/input/touchscreen/elants_i2c.c b/drivers/input/touchscreen/elants_i2c.c > > index 17540bdb1eaf..29b5bb03cff9 100644 > > --- a/drivers/input/touchscreen/elants_i2c.c > > +++ b/drivers/input/touchscreen/elants_i2c.c > > @@ -1396,7 +1396,7 @@ static int elants_i2c_probe(struct i2c_client *client, > > Might want to switch to probe_new() to avoid same/similar issue down > the road, either in the same patch or in a separate one. I think it's better to split. Will submit v3 with the fixes mentioned below. thanks, Takashi > > > > init_completion(&ts->cmd_done); > > > > ts->client = client; > > - ts->chip_id = (enum elants_chip_id)id->driver_data; > > + ts->chip_id = (enum elants_chip_id)device_get_match_data(&client->dev); > > I think this might need to go through an intermediate cast to shut up > compiler warnings: > > ts->chip_id = (enum elants_chip_id)(uintptr_t) > device_get_match_data(&client->dev); > > > i2c_set_clientdata(client, ts); > > > > ts->vcc33 = devm_regulator_get(&client->dev, "vcc33"); > > @@ -1636,8 +1636,8 @@ MODULE_DEVICE_TABLE(acpi, elants_acpi_id); > > > > #ifdef CONFIG_OF > > static const struct of_device_id elants_of_match[] = { > > - { .compatible = "elan,ekth3500" }, > > - { .compatible = "elan,ektf3624" }, > > + { .compatible = "elan,ekth3500", .data = EKTH3500 }, > > + { .compatible = "elan,ektf3624", .data = EKTF3624 }, > > As the bot mentioned this needs a cast. > > > { /* sentinel */ } > > }; > > MODULE_DEVICE_TABLE(of, elants_of_match); > > -- > > 2.26.2 > > > > Thanks. > > -- > Dmitry >
diff --git a/drivers/input/touchscreen/elants_i2c.c b/drivers/input/touchscreen/elants_i2c.c index 17540bdb1eaf..29b5bb03cff9 100644 --- a/drivers/input/touchscreen/elants_i2c.c +++ b/drivers/input/touchscreen/elants_i2c.c @@ -1396,7 +1396,7 @@ static int elants_i2c_probe(struct i2c_client *client, init_completion(&ts->cmd_done); ts->client = client; - ts->chip_id = (enum elants_chip_id)id->driver_data; + ts->chip_id = (enum elants_chip_id)device_get_match_data(&client->dev); i2c_set_clientdata(client, ts); ts->vcc33 = devm_regulator_get(&client->dev, "vcc33"); @@ -1636,8 +1636,8 @@ MODULE_DEVICE_TABLE(acpi, elants_acpi_id); #ifdef CONFIG_OF static const struct of_device_id elants_of_match[] = { - { .compatible = "elan,ekth3500" }, - { .compatible = "elan,ektf3624" }, + { .compatible = "elan,ekth3500", .data = EKTH3500 }, + { .compatible = "elan,ektf3624", .data = EKTF3624 }, { /* sentinel */ } }; MODULE_DEVICE_TABLE(of, elants_of_match);
The recent change in elants_i2c driver to support more chips introduced a regression leading to Oops at probing. The driver reads id->driver_data, but the id may be NULL depending on the device type the driver gets bound. Replace the driver data extraction with the device_get_match_data() helper, and define the driver data in OF table, too. Fixes: 9517b95bdc46 ("Input: elants_i2c - add support for eKTF3624") BugLink: https://bugzilla.suse.com/show_bug.cgi?id=1186454 Cc: <stable@vger.kernel.org> Signed-off-by: Takashi Iwai <tiwai@suse.de> --- v1->v2: Use device_get_match_data() drivers/input/touchscreen/elants_i2c.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)