> -----邮件原件-----
> 发件人: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> 发送时间: 2023年6月22日 23:07
> 收件人: 13916275206@xxxxxxx
> 抄送: alsa-devel@xxxxxxxxxxxxxxxx
> 主题: [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
