[bug report] ASoC: SDCA: Create DAPM widgets and routes from DisCo

[bug report] ASoC: SDCA: Create DAPM widgets and routes from DisCo

[Dan Carpenter]

Hello Charles Keepax,

Commit 2c8b3a8e6aa8 ("ASoC: SDCA: Create DAPM widgets and routes from
DisCo") from May 16, 2025 (linux-next), leads to the following Smatch
static checker warning:

sound/soc/sdca/sdca_asoc.c:405 entity_pde_event() error: uninitialized symbol 'from'.
sound/soc/sdca/sdca_asoc.c:405 entity_pde_event() error: uninitialized symbol 'to'.
sound/soc/sdca/sdca_asoc.c:417 entity_pde_event() error: uninitialized symbol 'poll_us'.

sound/soc/sdca/sdca_asoc.c
    377 static int entity_pde_event(struct snd_soc_dapm_widget *widget,
    378                             struct snd_kcontrol *kctl, int event)
    379 {
    380         struct snd_soc_component *component = widget->dapm->component;
    381         struct sdca_entity *entity = widget->priv;
    382         static const int polls = 100;
    383         unsigned int reg, val;
    384         int from, to, i;
    385         int poll_us;
    386         int ret;
    387 
    388         if (!component)
    389                 return -EIO;
    390 
    391         switch (event) {
    392         case SND_SOC_DAPM_POST_PMD:
    393                 from = widget->on_val;
    394                 to = widget->off_val;
    395                 break;
    396         case SND_SOC_DAPM_POST_PMU:
    397                 from = widget->off_val;
    398                 to = widget->on_val;
    399                 break;

Smatch does a fairly naive analysis of the callers and thinks that
event could be 1-2,4,8,16,32,64,128.

    400         }
    401 
    402         for (i = 0; i < entity->pde.num_max_delay; i++) {
    403                 struct sdca_pde_delay *delay = &entity->pde.max_delay[i];
    404 
--> 405                 if (delay->from_ps == from && delay->to_ps == to) {
                                              ^^^^                    ^^^
Potentially uninitialized.

    406                         poll_us = delay->us / polls;
    407                         break;
    408                 }
    409         }
    410 
    411         reg = SDW_SDCA_CTL(SDW_SDCA_CTL_FUNC(widget->reg),
    412                            SDW_SDCA_CTL_ENT(widget->reg),
    413                            SDCA_CTL_PDE_ACTUAL_PS, 0);
    414 
    415         for (i = 0; i < polls; i++) {
    416                 if (i)
    417                         fsleep(poll_us);
    418 
    419                 ret = regmap_read(component->regmap, reg, &val);
    420                 if (ret)
    421                         return ret;
    422                 else if (val == to)
    423                         return 0;
    424         }
    425 
    426         dev_err(component->dev, "%s: power transition failed: %x\n",
    427                 entity->label, val);
    428         return -ETIMEDOUT;
    429 }

regards,
dan carpenter

Re: [bug report] ASoC: SDCA: Create DAPM widgets and routes from DisCo

[Charles Keepax]

On Tue, May 27, 2025 at 01:04:26PM +0300, Dan Carpenter wrote:
> sound/soc/sdca/sdca_asoc.c
>     377 static int entity_pde_event(struct snd_soc_dapm_widget *widget,
>     378                             struct snd_kcontrol *kctl, int event)
>     379 {
>     380         struct snd_soc_component *component = widget->dapm->component;
>     381         struct sdca_entity *entity = widget->priv;
>     382         static const int polls = 100;
>     383         unsigned int reg, val;
>     384         int from, to, i;
>     385         int poll_us;
>     386         int ret;
>     387 
>     388         if (!component)
>     389                 return -EIO;
>     390 
>     391         switch (event) {
>     392         case SND_SOC_DAPM_POST_PMD:
>     393                 from = widget->on_val;
>     394                 to = widget->off_val;
>     395                 break;
>     396         case SND_SOC_DAPM_POST_PMU:
>     397                 from = widget->off_val;
>     398                 to = widget->on_val;
>     399                 break;
> 
> Smatch does a fairly naive analysis of the callers and thinks that
> event could be 1-2,4,8,16,32,64,128.

This should actually be safe as the widget that uses this
callback only registers for these two events, that said it is
probably nicer to have a default case anyway incase things change
later. I will fire out a patch shortly.

Thanks,
Charles