Re: [PATCH v8 1/2] arm64: dts: imx8mp: Add SAI, SDMA, AudioMIX

[Adam Ford <aford173@gmail.com>]

On Mon, May 8, 2023 at 6:42 AM Marek Vasut <marex@denx.de> wrote:
>
> Add all SAI nodes, SDMA2 and SDMA3 nodes, and AudioMIX node. This is
> needed to get audio operational on i.MX8MP .
>
> Acked-by: Peng Fan <peng.fan@nxp.com>
> Reviewed-by: Fabio Estevam <festevam@gmail.com>
> Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> Reviewed-by: Marco Felsch <m.felsch@pengutronix.de>
> Tested-by: Adam Ford <aford173@gmail.com> #imx8mp-beacon-kit
> Tested-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> Tested-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> Tested-by: Richard Leitner <richard.leitner@skidata.com>
> Signed-off-by: Marek Vasut <marex@denx.de>
> ---
> Cc: Abel Vesa <abelvesa@kernel.org>
> Cc: Alexander Stein <alexander.stein@ew.tq-group.com>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Jacky Bai <ping.bai@nxp.com>
> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Luca Ceresoli <luca.ceresoli@bootlin.com>
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Cc: Marco Felsch <m.felsch@pengutronix.de>
> Cc: Michael Turquette <mturquette@baylibre.com>
> Cc: NXP Linux Team <linux-imx@nxp.com>
> Cc: Peng Fan <peng.fan@nxp.com>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> Cc: Richard Cochran <richardcochran@gmail.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Stephen Boyd <sboyd@kernel.org>
> Cc: devicetree@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-clk@vger.kernel.org
> ---
> V2: - Add AUDIO_AXI clock to audio gpc
>     - Use IMX8MP_CLK_AUDIOMIX_SDMA2_ROOT for SDMA2 IPG clock
> V3: Rename audio_ahb to plain ahb
> V4: - Add RB/TB from Luca
>     - Rebase on next 20230223
> V5: - Add TB from Adam and Alexander
>     - Replace blk-ctrl@ with clock-controller@
>     - Specify sound-dai-cells in sai nodes
> V6: - Add RB from Fabio
>     - Drop power-domain-names from audiomix block/clock controller
>     - Move reg below compatible property
>     - Move sound-dai-cells below reg property
>     - Sort DT properties: compatible, regs, #cells, properties, status
> V7: - Move #clock-cells below reg property
>     - Add AB from Peng
>     - Add RB from Marco, sort the tags
> V8: Add TB from Richard
> ---
>  arch/arm64/boot/dts/freescale/imx8mp.dtsi | 158 ++++++++++++++++++++++
>  1 file changed, 158 insertions(+)
>
<snip>
Marek,

I have a question about the clocking for eASRC and PDM.

> +
> +                       audio_blk_ctrl: clock-controller@30e20000 {
> +                               compatible = "fsl,imx8mp-audio-blk-ctrl";
> +                               reg = <0x30e20000 0x10000>;
> +                               #clock-cells = <1>;
> +                               clocks = <&clk IMX8MP_CLK_AUDIO_ROOT>,
> +                                        <&clk IMX8MP_CLK_SAI1>,
> +                                        <&clk IMX8MP_CLK_SAI2>,
> +                                        <&clk IMX8MP_CLK_SAI3>,
> +                                        <&clk IMX8MP_CLK_SAI5>,
> +                                        <&clk IMX8MP_CLK_SAI6>,
> +                                        <&clk IMX8MP_CLK_SAI7>;
> +                               clock-names = "ahb",
> +                                             "sai1", "sai2", "sai3",
> +                                             "sai5", "sai6", "sai7";
> +                               power-domains = <&pgc_audio>;
> +                       };
> +               };
> +

I am trying to plumb in the micfil driver with a PDM microphone on a
Plus.  I have SAI3 and SAI5 audio working, but if I try to use the
micfil, the PDM clock doesn't get turned on, and the micfil doesn't
appear to see anything coming in.  I was curious why the
audio_blk_ctrl has clock entries for IMX8MP_CLK_SAIx, but there isn't
one for the PDM nor the ASRC clocks.  I added the MICFIL noted to the
8mp in a previous patch [1], and I am trying to customize the MICFIL
node as follows:

&micfil {
#sound-dai-cells = <0>;
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_pdm>;
assigned-clocks = <&clk IMX8MP_CLK_PDM>;
assigned-clock-parents = <&clk IMX8MP_AUDIO_PLL1_OUT>;
assigned-clock-rates = <196608000>;
status = "okay";
};

I also noticed in the down-stream kernel, the pdm_ipg_clk and
pdm_root_clk are shared gates with separate parents.

The PDM tree of the down-stream kernel looks like this:
 audio_pll1_ref_sel                0        0        0    24000000
     0     0  50000         Y
       audio_pll1                     0        0        0   393216000
        0     0  50000         Y
          audio_pll1_bypass           0        0        0   393216000
        0     0  50000         Y
             audio_pll1_out           0        0        0   393216000
        0     0  50000         N
                pdm                   0        0        0   196608000
        0     0  50000         N
                   pdm_root           0        0        0   196608000
        0     0  50000         N
                      pdm_sel         0        0        0   196608000
        0     0  50000         Y
                         pdm_root_clk       0        0        0
196608000          0     0  50000         N

The PDM tree of the mainline looks like this:

   audio_pll1_ref_sel                0        0        0    24000000
       0     0  50000         Y
       audio_pll1                     0        0        0   393216000
        0     0  50000         Y
          audio_pll1_bypass           0        0        0   393216000
        0     0  50000         Y
             audio_pll1_out           0        0        0   393216000
        0     0  50000         N
                pdm                   0        0        0   196608000
        0     0  50000         N
                   pdm_root           0        0        0   196608000
        0     0  50000         N
                      pdm_sel         0        0        0   196608000
        0     0  50000         Y

It seems like the "pdm_root_clk" generated by the shared audo-blk
down-sream driver is missing from the mainline.  I looked up the clock
I referenced when I attempted to enable the miffil, but
'IMX8MP_CLK_AUDIOMIX_PDM_ROOT doesn't appear to be configured in
either clk-imx8mp.c or clk-imx8mp-audiomix.c.  Maybe it's obscured by
the macros, but it seems like the pdm_sel should somehow have an
additional variable for the shared clock and an additional clock like
pdm_root_clk assigned with it.

I have similar configurations for Mini and Nano, and both of them are
able to record audio, so I think there might be a clock issue
somewhere related to the audiomix driver, and not a misconfiguration
of the sound-card or the micfil itself.

thanks for any suggestions,

adam


[1] - https://patchwork.kernel.org/project/linux-arm-kernel/patch/20230827023155.467807-3-aford173@gmail.com/