On Wed, Jul 20, 2016 at 07:31:17AM +0200, Takashi Iwai wrote: > > + unsigned int offset; unsigned int counter = 0; > > Need a line break. arghh, will fix.. > > + offset = azx_readl(chip, LLCH); > > + > > + /* Lets walk the linked capabilities list */ > > + do { > > + cur_cap = _snd_hdac_chip_read(l, azx_bus(chip), offset); > > + > > + switch ((cur_cap & CAP_HDR_ID_MASK) >> CAP_HDR_ID_OFF) { > > + case GTS_CAP_ID: > > + dev_dbg(chip->card->dev, "Found GTS capability"); > > + chip->gts_present = 1; > > + break; > > + > > + default: > > + break; > > + } > > + > > + counter++; > > + > > + if (counter > AZX_MAX_CAPS) { > > + dev_err(chip->card->dev, "We exceeded azx capabilities!!!\n"); > > + break; > > + } > > + > > + /* read the offset of next capabiity */ > > + offset = cur_cap & CAP_HDR_NXT_PTR_MASK; > > + > > + } while (offset); > > Wouldn't it be safer to use a normal while () {} loop? > The first LLCH read might be zero, in theory. Then in that case first read will give error. But yes I see the benifits. Btw this is same as snd_hdac_ext_bus_parse_capabilities() Should we move this to hdac and use for both. Ofcourse many new capablities do not make sense to legacy driver, so we would have to ignore them. > > + if (!(chip->gts_present && boot_cpu_has(X86_FEATURE_ART))) > > + chip->gts_present = false; > > Need #ifdef CONFIG_X86 guard here, too. > Also, the inclusion of <asm/cpufeature.h> isn't needed? (Again > X86-only.) This is intel.c :) I did compile it for ARM before sending. -- ~Vinod _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx http://mailman.alsa-project.org/mailman/listinfo/alsa-devel