On Fri, Nov 25, 2022 at 05:44:11PM +0800, Jiaxin Yu wrote:

> +     /*
> +      * PCM trigger callback.
> +      * Mandatory
> +      */
> +     int (*trigger)(struct device *dev, int cmd);
> +

Making this mandatory would break all existing users, though...

> +     switch (event) {
> +     case SND_SOC_DAPM_PRE_PMU:
> +             if (hcp->hcd.ops->trigger)
> +                     hcp->hcd.ops->trigger(component->dev->parent, 
> SNDRV_PCM_TRIGGER_START);

...it's not actually mandatory so it's just the comment that's wrong.
I'm a little unclear why this is being implemented as a DAPM operation
rather than having the driver forward the PCM trigger op if it's needed?
Or alternatively if a DAPM callback is needed why not provide one
directly rather than hooking into the trigger function - that's going to
be called out of sequence with the rest of DAPM and be potentially
confusing given the very different environments that trigger and DAPM
operations run in.  A quick glance at the it6505 driver suggests it'd be
happier with a DAPM callback.

Attachment: signature.asc
Description: PGP signature

Reply via email to