[alsa-devel] [PATCH v3 1/5] ALSA: hda - move bus_parse_capabilities to core

[Takashi Iwai]

On Wed, 03 Aug 2016 18:06:10 +0200,
Vinod Koul wrote:
> 
> HDA capability introduced recently are moved to hdac core so that it can
> be used by legacy driver as well. Also move the capability pointers up to
> hdac_bus object.
> 
> Signed-off-by: Vinod Koul <vinod.koul at intel.com>

The patch looks almost good, but just minor nitpicking:


> +int snd_hdac_bus_parse_capabilities(struct hdac_bus *bus)
> +{
> +	unsigned int cur_cap;
> +	unsigned int offset;
> +	unsigned int counter = 0;
> +
> +	offset = snd_hdac_chip_readl(bus, LLCH);
> +
> +	/* Lets walk the linked capabilities list */
> +	do {
> +		cur_cap = _snd_hdac_chip_read(l, bus, offset);
> +
> +		dev_dbg(bus->dev, "Capability version: 0x%x\n",
> +				((cur_cap & AZX_CAP_HDR_VER_MASK) >> AZX_CAP_HDR_VER_OFF));

Too much indentation.  Also the parentheses are superfluous (unlike
below).

> +
> +		dev_dbg(bus->dev, "HDA capability ID: 0x%x\n",
> +				(cur_cap & AZX_CAP_HDR_ID_MASK) >> AZX_CAP_HDR_ID_OFF);

Too much indentation.

> +		switch ((cur_cap & AZX_CAP_HDR_ID_MASK) >> AZX_CAP_HDR_ID_OFF) {
> +		case AZX_ML_CAP_ID:
> +			dev_dbg(bus->dev, "Found ML capability\n");
> +			bus->mlcap = bus->remap_addr + offset;
> +			break;
> +
> +		case AZX_GTS_CAP_ID:
> +			dev_dbg(bus->dev, "Found GTS capability offset=%x\n", offset);
> +			bus->gtscap = bus->remap_addr + offset;
> +			break;
> +
> +		case AZX_PP_CAP_ID:
> +			/* PP capability found, the Audio DSP is present */
> +			dev_dbg(bus->dev, "Found PP capability offset=%x\n", offset);
> +			bus->ppcap = bus->remap_addr + offset;
> +			break;
> +
> +		case AZX_SPB_CAP_ID:
> +			/* SPIB capability found, handler function */
> +			dev_dbg(bus->dev, "Found SPB capability\n");
> +			bus->spbcap = bus->remap_addr + offset;
> +			break;
> +
> +		case AZX_DRSM_CAP_ID:
> +			/* DMA resume  capability found, handler function */
> +			dev_dbg(bus->dev, "Found DRSM capability\n");
> +			bus->drsmcap = bus->remap_addr + offset;
> +			break;
> +
> +		default:
> +			dev_dbg(bus->dev, "Unknown capability %d\n", cur_cap);
> +			break;
> +		}
> +
> +		counter++;
> +
> +		if (counter > HDAC_MAX_CAPS) {
> +			dev_err(bus->dev, "We exceeded HDAC capablities!!!\n");
> +			break;
> +		}
> +
> +		/* read the offset of next capabiity */

A typo.


thanks,

Takashi