[alsa-devel] [RFC PATCH 3/3] ucm: Execute sequence of component devices

[Lin, Mengdong]

> -----Original Message-----
> From: Liam Girdwood [mailto:liam.r.girdwood at linux.intel.com]
> Sent: Tuesday, November 15, 2016 4:50 PM

> > @@ -341,13 +348,22 @@ static int execute_cset(snd_ctl_t *ctl, const char
> *cset, unsigned int type)
> >   * \brief Execute the sequence
> >   * \param uc_mgr Use case manager
> >   * \param seq Sequence
> > + * \param cdev_in input cdev,  parenet's cdev for a component device,
> > + *				or NULL for a machine device.
> >   * \return zero on success, otherwise a negative error code
> > + *
> > + * A machine device's sequence can enable or disable a component device.
> > + * But the enable/disable sequence of a component device does not
> > + define
> > + * cdev, the card device. So when executing a component device's
> > + sequence,
> > + * we need to pass the cdev defined by the sequence of its parent,
> > + the
> > + * machine device. And for machine device, cdev should be set to NULL.
> >   */
> >  static int execute_sequence(snd_use_case_mgr_t *uc_mgr,
> >  			    struct list_head *seq,
> >  			    struct list_head *value_list1,
> >  			    struct list_head *value_list2,
> > -			    struct list_head *value_list3)
> > +			    struct list_head *value_list3,
> > +			    char *cdev_in)
> 
> Could the current cdev be embedded in uc_mgr ?

Yes, it could and there will be less code change. 
Actually I found the machine devices always set the same cdev name in their sequences.

> > @@ -366,7 +382,16 @@ static int execute_sequence(snd_use_case_mgr_t
> *uc_mgr,
> >  		case SEQUENCE_ELEMENT_TYPE_CSET:
> >  		case SEQUENCE_ELEMENT_TYPE_CSET_BIN_FILE:
> >  		case SEQUENCE_ELEMENT_TYPE_CSET_TLV:
> > -			if (cdev == NULL) {
> > +			if (cdev == NULL && cdev_in) {
> > +				/* Sequence of a component device, should
> use
> > +				 * the input cdev defined by sequence of its
> > +				 * parent, the machine device.
> > +				 */
> > +				cdev = strdup(cdev_in);
> > +				if (cdev == NULL)
> > +					goto __fail_nomem;
> > +
> > +			} else if (cdev == NULL) {
> >  				char *playback_ctl = NULL;
> >  				char *capture_ctl = NULL;
> >
> > @@ -405,7 +430,9 @@ static int execute_sequence(snd_use_case_mgr_t
> *uc_mgr,
> >  					free(capture_ctl);
> >  				} else
> >  					cdev = capture_ctl;
> > +
> >  			}
> > +
> 
> Looks like this formatting change was added by mistake ?

Yes, it was added by mistake. I'll remove it.

> > +/* Execute enable or disable sequence of a component device.
> > + *
> > + * For a component device (a codec or embedded DSP), its sequence
> > +doesn't
> > + * specify the sound card device 'cdev', because a component can be
> > +reused
> > + * by different sound cards (machines). So when executing its
> > +sequence, a
> > + * parameter 'cdev_in' is used to pass cdev defined by the sequence
> > +of its
> > + * parent, the machine device.
> > + */
> > +static int execute_component_seq(snd_use_case_mgr_t *uc_mgr,
> > +				 struct component_sequence *cmpt_seq,
> > +				 struct list_head *value_list1,
> > +				 struct list_head *value_list2,
> > +				 struct list_head *value_list3,
> > +				 char *cdev_in)
> > +{
> > +	struct use_case_device *device = cmpt_seq->device;
> > +	struct list_head *seq;
> > +	int err;
> > +
> > +
> > +	if (cmpt_seq->enable)
> > +		seq = &device->enable_list;
> > +	else
> > +		seq = &device->disable_list;
> 
> What happens here if there is only an enable sequence and no disable
> sequence ? i.e. will seq be NULL ?

The device's enable_list or disable_list will be empty, assured by the UCM parser.
And so 'seq' will be an empty list and the following execute_sequence() will exit without doing anything.
> 
> > +
> > +	err = execute_sequence(uc_mgr, seq,
> > +			       &device->value_list,
> > +			       &uc_mgr->active_verb->value_list,
> > +			       &uc_mgr->value_list,
> > +			       cdev_in);
> > +	return err;
> > +}
> > +

Thanks
Mengdong