Hi Fabio, that’s interesting.
Looks like the fsl,imx-audio-sgtl5000 sound card driver that is used on the imx51-babbage
does not try to adjust MCLK in response to the playback sample rate.
The simple-audio-card driver does that (it calls snd_soc_dai_set_sysclk whenever playback
starts).
I think we can do without the new dts property when we fix sgtl5000_set_dai_sysclk to
actually call clk_set_rate on the MCLK instead of blindly setting any requested value.
On a fixed clock, clk_set_rate is a no-op if the requested rate matches the fixed rate
and errors out otherwise.
I think this would be the more correct fix, but it carries the risk of breaking existing
boards that rely on the current behaviour. What do you think?
Best regards,
Jakob
> On 16. Oct 2017, at 01:09, Fabio Estevam <fabio.estevam@xxxxxxx> wrote:
>
> [Sorry for the top-posting. I did not see your message in the mailing list]
>
> Hi Jakob,
>
> arch/arm/boot/dts/imx51-babbage.dts has SYS_MCLK generated by an external fixed oscillator and it works fine.
>
> Do you really need this new property?
> From: Jakob Unterwurzacher <jakob.unterwurzacher@xxxxxxxxxxxxxxxxxxxxx>
> Sent: Friday, October 6, 2017 5:34:44 AM
> To: linux-kernel@xxxxxxxxxxxxxxx
> Cc: Jakob Unterwurzacher; Klaus Goger; Liam Girdwood; Mark Brown; Rob Herring; Mark Rutland; Jaroslav Kysela; Takashi Iwai; Richard Leitner; Fabio Estevam; Bhumika Goyal; alsa-devel@xxxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx
> Subject: [PATCH] ASoC: sgtl5000: add support for a fixed sysclk frequency
>
> The system clock, also called SYS_MCLK in the SGTL5000 datasheet,
> is usually generated by the I2S master with a frequency proportional
> to the sample rate.
>
> However, this is not always the case. Compute modules following
> the Qseven specification, for example, do not even have a pin for
> I2S SYS_MCLK on their board-to-board connector. Only I2S_WS, I2S_RST#
> I2S_CLK, I2S_SDI and I2S_SDO are specified.
>
> Instead, SYS_MCLK is usually generated by an external fixed oscillator.
>
> Add support for this scenario, enabled by the new "sysclk-fixed"
> devicetree property.
>
> Tested on a "Puma" RK3399-Q7 compute module with a "Haikou" baseboard
> that has a fixed 24.576 MHz oscillator connected to the codec's SYS_MCLK.
>
> Signed-off-by: Jakob Unterwurzacher <jakob.unterwurzacher@xxxxxxxxxxxxxxxxxxxxx>
> Cc: Klaus Goger <klaus.goger@xxxxxxxxxxxxxxxxxxxxx>
> ---
> Documentation/devicetree/bindings/sound/sgtl5000.txt | 5 +++++
> sound/soc/codecs/sgtl5000.c | 12 ++++++++++++
> 2 files changed, 17 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/sound/sgtl5000.txt b/Documentation/devicetree/bindings/sound/sgtl5000.txt
> index 7a73a9d62015..0d0dd58b2d4a 100644
> --- a/Documentation/devicetree/bindings/sound/sgtl5000.txt
> +++ b/Documentation/devicetree/bindings/sound/sgtl5000.txt
> @@ -35,6 +35,11 @@ VDDIO 1.8V 2.5V 3.3V
> 2 = 3.33 mA 5.74 mA 8.03 mA
> 3 = 4.99 mA 8.61 mA 12.05 mA
>
> +- sysclk-fixed : If this property exists, the sysclk is considered to be
> + generated by a fixed-frequency oscillator. The frequency value is read
> + from the clock defined in the "clocks" property. set_sysclk requests
> + are ignored.
> +
> Example:
>
> codec: sgtl5000@0a {
> diff --git a/sound/soc/codecs/sgtl5000.c b/sound/soc/codecs/sgtl5000.c
> index f2bb4feba3b6..be3aea89371c 100644
> --- a/sound/soc/codecs/sgtl5000.c
> +++ b/sound/soc/codecs/sgtl5000.c
> @@ -133,6 +133,7 @@ struct sgtl5000_priv {
> u8 micbias_resistor;
> u8 micbias_voltage;
> u8 lrclk_strength;
> + u8 sysclk_fixed; /* sysclk rate cannot be changed */
> };
>
> /*
> @@ -613,6 +614,10 @@ static int sgtl5000_set_dai_sysclk(struct snd_soc_dai *codec_dai,
>
> switch (clk_id) {
> case SGTL5000_SYSCLK:
> + if (sgtl5000->sysclk_fixed) {
> + dev_dbg(codec->dev, "sysclk-fixed: ignoring set_sysclk request\n");
> + return 0;
> + }
> sgtl5000->sysclk = freq;
> break;
> default:
> @@ -1444,6 +1449,13 @@ static int sgtl5000_i2c_probe(struct i2c_client *client,
> } else {
> sgtl5000->micbias_voltage = 0;
> }
> + if (of_property_read_bool(np, "sysclk-fixed")) {
> + sgtl5000->sysclk = clk_get_rate(sgtl5000->mclk);
> + sgtl5000->sysclk_fixed = 1;
> + dev_dbg(&client->dev,
> + "sysclk-fixed: setting sysclk to fixed value %d\n",
> + sgtl5000->sysclk);
> + }
> }
>
> sgtl5000->lrclk_strength = I2S_LRCLK_STRENGTH_LOW;
> --
> 2.11.0
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
