Hi Biji, On Wed, Jul 26, 2023 at 10:08 AM Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote: > Currently audio mclk uses a fixed clk of 11.2896MHz (multiple of 44.1kHz). > Replace this fixed clk with the programmable versa3 clk that can provide > the clocking to support both 44.1kHz (with a clock of 11.2896MHz) and > 48kHz (with a clock of 12.2880MHz), based on audio sampling rate for > playback and record. > > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx> Thanks for your patch! > --- a/arch/arm64/boot/dts/renesas/rz-smarc-common.dtsi > +++ b/arch/arm64/boot/dts/renesas/rz-smarc-common.dtsi > @@ -32,12 +32,6 @@ chosen { > stdout-path = "serial0:115200n8"; > }; > > - audio_mclock: audio_mclock { > - compatible = "fixed-clock"; > - #clock-cells = <0>; > - clock-frequency = <11289600>; > - }; > - > snd_rzg2l: sound { > compatible = "simple-audio-card"; > simple-audio-card,format = "i2s"; > @@ -55,7 +49,7 @@ cpu_dai: simple-audio-card,cpu { > }; > > codec_dai: simple-audio-card,codec { > - clocks = <&audio_mclock>; > + clocks = <&versa3 3>; The bindings do not mention the mapping from clock indices to actual outputs. According to Table 3. ("Output Source") in the 5P35023 datasheet, I would expect the mapping to be 0=REF, 1=SE1, 2=SE2, 3=SE3, 4=DIFF1, 5=DIFF2. But as AUDIO_MCK is sourced from SE2, that can't be correct? Oops, the mapping in the driver uses the order in the Block Diagram, which is the inverse... > sound-dai = <&wm8978>; > }; > }; > @@ -76,6 +70,12 @@ vccq_sdhi1: regulator-vccq-sdhi1 { > gpios-states = <1>; > states = <3300000 1>, <1800000 0>; > }; > + > + x1_x2: xtal { X2 is a different 32768 kHz crystal. "xtal" is a too-generic node name, and may cause conflicts that are hard to debug . "x1: x1-clock {", cfr. "x2" in arch/arm64/boot/dts/renesas/rzg2lc-smarc-som.dtsi? > + compatible = "fixed-clock"; > + #clock-cells = <0>; > + clock-frequency = <24000000>; > + }; > }; > > --- a/arch/arm64/boot/dts/renesas/rzg2ul-smarc.dtsi > +++ b/arch/arm64/boot/dts/renesas/rzg2ul-smarc.dtsi > @@ -20,6 +20,27 @@ &cpu_dai { > sound-dai = <&ssi1>; > }; > > +&i2c0 { Any specific reason you're not adding clock-frequency = <400000>; like is already present on the other SoMs? > + versa3: versa3@68 { > + compatible = "renesas,5p35023"; The rest LGTM, I didn't verify renesas,settings though. Note that to avoid introducing regressions, this patch has to be postponed until Versa3 clock support has reached upstream (v6.6-rc1?). Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds