On Thu, Apr 22, 2021 at 10:53:44AM +0900, Kuninori Morimoto wrote: > +/* Describes the possible PCM format */ > +#define SND_SOC_POSSIBLE_DAIFMT_FORMAT_SHIFT 0 > +#define SND_SOC_POSSIBLE_DAIFMT_FORMAT_MASK (0xFFFF << SND_SOC_POSSIBLE_DAIFMT_FORMAT_SHIFT) > +#define SND_SOC_POSSIBLE_DAIFMT_I2S (1 << SND_SOC_DAI_FORMAT_I2S) > +#define SND_SOC_POSSIBLE_DAIFMT_RIGHT_J (1 << SND_SOC_DAI_FORMAT_RIGHT_J) > +#define SND_SOC_POSSIBLE_DAIFMT_LEFT_J (1 << SND_SOC_DAI_FORMAT_LEFT_J) > +#define SND_SOC_POSSIBLE_DAIFMT_DSP_A (1 << SND_SOC_DAI_FORMAT_DSP_A) > +#define SND_SOC_POSSIBLE_DAIFMT_DSP_B (1 << SND_SOC_DAI_FORMAT_DSP_B) > +#define SND_SOC_POSSIBLE_DAIFMT_AC97 (1 << SND_SOC_DAI_FORMAT_AC97) > +#define SND_SOC_POSSIBLE_DAIFMT_PDM (1 << SND_SOC_DAI_FORMAT_PDM) I'm not 100% sure I get why we have the separate _POSSIBLE_ macros here? One thing we'll have to take account of is that there's some DAIs that have some restrictions on what options they can combine - for example only doing I2S with one format of clock but allowing clock inversion in DSP mode. It might be safer (if tedious for driver authors without some help...) to just have arrays of fully specified daifmt values. > /* Digital Audio interface formatting */ > +u64 snd_soc_dai_get_fmt(struct snd_soc_dai *dai); Like I said on the cover letter I think we need to be able to specify at least two levels of preference here. How about int snd_soc_dai_get_fmt(struct snd_soc_dai *dai, u64 *preferred, u64 *supported); or something? Just thinking off the top of my head, that's a bit ugly and doesn't scale to multiple levels so I don't know if I'm *super* happy with that interface. But we might be better off using just arrays of daifmt values like I say, if we do that perhaps just one array but sorting it might do? > + /* use original dai_fmt if sound card specify */ > + if (!(dai_link->dai_fmt & SND_SOC_DAIFMT_FORMAT_MASK)) > + mask |= SND_SOC_DAIFMT_FORMAT_MASK; > + if (!(dai_link->dai_fmt & SND_SOC_DAIFMT_CLOCK_MASK)) > + mask |= SND_SOC_DAIFMT_CLOCK_MASK; > + if (!(dai_link->dai_fmt & SND_SOC_DAIFMT_INV_MASK)) > + mask |= SND_SOC_DAIFMT_INV_MASK; > + if (!(dai_link->dai_fmt & SND_SOC_DAIFMT_MASTER_MASK)) > + mask |= SND_SOC_DAIFMT_MASTER_MASK; > + > + dai_link->dai_fmt = dai_link->dai_fmt | (dai_fmt & mask); If the sound card specifies something why not just use it verbatim instead of trying to merge?
Attachment:
signature.asc
Description: PGP signature
