* [bug report] ASoC: tas2781: Add tas2781 driver
@ 2023-06-22 15:07 Dan Carpenter
2023-06-23 5:30 ` 回复: " 13916275206
0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2023-06-22 15:07 UTC (permalink / raw)
To: 13916275206; +Cc: alsa-devel
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);
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
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
^ permalink raw reply [flat|nested] 2+ messages in thread
* 回复: [bug report] ASoC: tas2781: Add tas2781 driver
2023-06-22 15:07 [bug report] ASoC: tas2781: Add tas2781 driver Dan Carpenter
@ 2023-06-23 5:30 ` 13916275206
0 siblings, 0 replies; 2+ messages in thread
From: 13916275206 @ 2023-06-23 5:30 UTC (permalink / raw)
To: 'Dan Carpenter'; +Cc: alsa-devel
> -----邮件原件-----
> 发件人: 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
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2023-06-26 11:17 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-22 15:07 [bug report] ASoC: tas2781: Add tas2781 driver Dan Carpenter
2023-06-23 5:30 ` 回复: " 13916275206
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.