Re: [PATCH 3/6] ASoC: Intel: Skylake: Add functions for DSP module configuration

[Vinod Koul <vinod.koul@intel.com>]

[-- Attachment #1.1: Type: text/plain, Size: 1337 bytes --]

On Wed, Jul 29, 2015 at 01:33:23PM +0100, Mark Brown wrote:
> On Tue, Jul 21, 2015 at 11:53:57PM +0530, Vinod Koul wrote:
> 
> > +	case SKL_CH_CFG_I2S_DUAL_STEREO_1:
> > +		config = (0xFFFF00FF | (SKL_CHANNEL_LEFT << 8)
> > +			| (SKL_CHANNEL_RIGHT << 12));
> > +		break;
> > +
> > +	default:
> > +		config =  0xFFFFFFFF;
> > +	}
> 
> Missing break.
Should default have one always :)

> 
> > +/*
> > + * Allocates queue for each module.
> > + * if dynamic, the pin_index is allocated 0 to max_pin.
> > + * In static, the pin_index is fixed based on module_id and instance id
> > + */
> > +static int skl_alloc_queue(struct skl_module_pin *mpin,
> > +			struct skl_module_inst_id id, int max)
> > +{
> > +	struct skl_module_pin m_pin = skl_get_queue(mpin, id, max);
> > +
> > +	if (m_pin.pin_index < 0)
> > +		return -EINVAL;
> > +
> > +	if (m_pin.is_dynamic) {
> > +			m_pin.in_use = true;
> > +			m_pin.id.module_id = id.module_id;
> > +			m_pin.id.instance_id = id.instance_id;
> > +	}
> 
> Double indentation here.  It's really unclear to me why a static pin
> doesn't need to get merked as in use?
Oops, will fix

In static pin case, the pins come from topology binary where these are fixed
for a configuration, so don't need to be allocated thus marked and hence
freed.

-- 
~Vinod
-- 

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]