-----邮件原件----- 发件人: Dan Carpenter dan.carpenter@linaro.org 发送时间: 2023年6月22日 23:07 收件人: 13916275206@139.com 抄送: alsa-devel@alsa-project.org 主题: [bug report] ASoC: tas2781: Add tas2781 driver

Hello Shenghao Ding,

The patch ef3bcde75d06: "ASoC: tas2781: Add tas2781 driver" from Jun 18, 2023, leads to the following Smatch static checker warning:

sound/soc/codecs/tas2781-i2c.c:651 tasdevice_parse_dt() warn: assigning signed to unsigned: 'tas_priv->ndev = ndev' 's32min-s32max'

sound/soc/codecs/tas2781-i2c.c 602 static void tasdevice_parse_dt(struct tasdevice_priv *tas_priv) 603 { 604 struct i2c_client *client = (struct i2c_client *)tas_priv->client; 605 unsigned int dev_addrs[TASDEVICE_MAX_CHANNELS]; 606 int rc, i, ndev = 0; 607 608 if (tas_priv->isacpi) { 609 ndev = device_property_read_u32_array(&client->dev, 610 "ti,audio-slots", NULL, 0);

When we pass NULL to device_property_read_u32_array() then it returns then number of items.

611                 if (ndev <= 0) {
612                         ndev = 1;
613                         dev_addrs[0] = client->addr;
614                 } else {
615                         ndev = (ndev <

ARRAY_SIZE(dev_addrs)) 616 ? ndev : ARRAY_SIZE(dev_addrs); 617 ndev = device_property_read_u32_array(&client->dev, ^^^^^^^ Smatch is concerned that this value can be negative. But actually this

sets it

to zero, doesn't it? Is that intentional? It feels like we should leave

ndev as

the number of items. Or if we want ndev to be zero do "ndev = 0;" on the next line.

618                                 "ti,audio-slots", dev_addrs,

ndev);

Thanks for your report, correct and add as following: if (ndev <= 0) { ndev = 1; dev_addrs[0] = client->addr; } One more thing, how to smatch the code in kernel. Where can I find the guideline?

619                 }
620
621                 tas_priv->irq_info.irq_gpio =
622

acpi_dev_gpio_irq_get(ACPI_COMPANION(&client->dev), 0); 623 } else { 624 struct device_node *np = tas_priv->dev->of_node; 625 #ifdef CONFIG_OF 626 const __be32 *reg, *reg_end; 627 int len, sw, aw; 628 629 aw = of_n_addr_cells(np); 630 sw = of_n_size_cells(np); 631 if (sw == 0) { 632 reg = (const __be32 *)of_get_property(np, 633 "reg", &len); 634 reg_end = reg + len/sizeof(*reg); 635 ndev = 0; 636 do { 637 dev_addrs[ndev] = of_read_number(reg, aw); 638 reg += aw; 639 ndev++; 640 } while (reg < reg_end); 641 } else { 642 ndev = 1; 643 dev_addrs[0] = client->addr; 644 } 645 #else 646 ndev = 1; 647 dev_addrs[0] = client->addr; 648 #endif 649 tas_priv->irq_info.irq_gpio = of_irq_get(np, 0); 650 } --> 651 tas_priv->ndev = ndev; ^^^^^^^^^^^^^^^^^^^^^ Warning

Do I need the casting? ndev is sure to less than or equal to 8 and more than 1, is won't be overflow in unsigned char.

652         for (i = 0; i < ndev; i++)
653                 tas_priv->tasdevice[i].dev_addr = dev_addrs[i];
654
655         tas_priv->reset = devm_gpiod_get_optional(&client->dev,
656                         "reset-gpios", GPIOD_OUT_HIGH);

regards, dan carpenter