Re: [PATCH v1] ASoC: simple-card-utils: set CPU sysclk before codec for mclk-fs

[Stefano Radaelli]

Hi Kuninori
> 
> Actually, this is one of my concern (= which should be worked first
> (CPU or Codec)). I don't remember why current code calls Codec first,
> but maybe original code was.
> I guess, some other Card needs to call as-is (= Codec first), and
> you want to call opposite (= CPU first). It depends on clock
> provider/consumer settings, instead of mclk-fs, but based on each Card.
> 
> Not same but similar issue we had before about how we want to call
> .trigger.
> 
> 	356caf663deee8dc46ff3168ec0b24bcbeb00b28
> 	("ASoC: add new trigger ordering method")
> 
> It was fixed order, but is now controlled by flag.
> Maybe we want to have similar method ?
> 
> // unreliable name
> #define XX_CLK_START_CODEC_FIRST	0
> #define XX_CLK_START_CPU_FIRST		1
> 
> 	if (xx_clk_start_first == XX_CLK_START_CODEC_FIRST) {
> 		// same as current code
> 		start_codec_clk()
> 		start_cpu_clk()
> 	} else {
> 		// opposite
> 		start_cpu_clk()
> 		start_codec_clk()
> 	}
> 

Thank you for your suggestion. I agree that the default ordering should
remain unchanged to maintain backward compatibility, and we should
implement a configurable feature to change the order when needed.
Following your recommendation, I could implement something similar to the
trigger ordering approach:

#define SND_SOC_SYSCLK_ORDER_CODEC_FIRST    0
#define SND_SOC_SYSCLK_ORDER_CPU_FIRST      1

static int asoc_simple_init_dai(struct snd_soc_dai *dai,
                                struct asoc_simple_dai *simple_dai)
{
    int sysclk_order = SND_SOC_SYSCLK_ORDER_CODEC_FIRST;  // <- Default

    if (simple_dai->sysclk) {
        if (sysclk_order == SND_SOC_SYSCLK_ORDER_CODEC_FIRST) {
            // Codec first
            ret = snd_soc_dai_set_sysclk(codec_dai, 0, sysclk, ...);
            ret = snd_soc_dai_set_sysclk(cpu_dai, 0, sysclk, ...);
        } else {
            // CPU first
            ret = snd_soc_dai_set_sysclk(cpu_dai, 0, sysclk, ...);
            ret = snd_soc_dai_set_sysclk(codec_dai, 0, sysclk, ...);
        }
    }
}

This approach would work well for our case and is acceptable as-is for now.
The ordering would be controlled via the define in the driver code.
As a future enhancement, would you be open to adding a device tree property
to make this configurable per-board?

I'll send you v2 with this modification.

Thanks a lot for your guidance,
Best Regards,
Stefano