On Thu, 30 Mar 2023 18:09:36 +0200,
Pierre-Louis Bossart wrote:
>
>
>
> On 3/30/23 10:51, Takashi Iwai wrote:
> > On Mon, 27 Mar 2023 13:29:19 +0200,
> > Peter Ujfalusi wrote:
> >>
> >> int hda_bus_ml_get_capabilities(struct hdac_bus *bus)
> >> {
> >> - if (bus->mlcap)
> >> - return snd_hdac_ext_bus_get_ml_capabilities(bus);
> >> + u32 link_count;
> >> + int ret;
> >> + int i;
> >> +
> >> + if (!bus->mlcap)
> >> + return 0;
> >> +
> >> + link_count = readl(bus->mlcap + AZX_REG_ML_MLCD) + 1;
> >> +
> >> + dev_dbg(bus->dev, "HDAudio Multi-Link count: %d\n", link_count);
> >> +
> >> + for (i = 0; i < link_count; i++) {
> >> + ret = hda_ml_alloc_h2link(bus, i);
> >> + if (ret < 0) {
> >> + hda_bus_ml_free(bus);
> >> + return ret;
> >> + }
> >> + }
> >> return 0;
> >
> > This makes that each call of hda_bus_ml_get_capabilities() adds the
> > h2link entries blindly. If the driver calls it multiple times
> > mistakenly (the function name sounds as if it's just a helper to query
> > the capability bits), it'll lead to doubly entries. Maybe adding some
> > check would be safer, IMO.
>
> Interesting comment, I didn't think of that one.
>
> This function is currently called in the probe and indirectly via
> hda_init_caps(). I think the driver framework guarantees the probe is
> only called once, doesn't it?
>
> we can also rename this function to make it clearer if there are any
> suggestions, but the name is aligned with previous implementations of
> the snd_hdac_ext stuff.
Yeah, naming it as an init function would avoid the misuse.
Takashi
