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