[v3,3/6] clk: imx: imx8mp: Add audiomix block control

Commit Message

Marek Vasut June 25, 2022, 1:32 a.m. UTC

Unlike the other block control IPs in i.MX8M, the audiomix is mostly a
series of clock gates and muxes. Model it as a large static table of
gates and muxes with one exception, which is the PLL14xx . The PLL14xx
SAI PLL has to be registered separately.

Signed-off-by: Marek Vasut <[e-mail]>
Cc: Abel Vesa <[e-mail]>
Cc: Fabio Estevam <[e-mail]>
Cc: Jacky Bai <[e-mail]>
Cc: Laurent Pinchart <[e-mail]>
Cc: Lucas Stach <[e-mail]>
Cc: Michael Turquette <[e-mail]>
Cc: Shawn Guo <[e-mail]>
Cc: Stephen Boyd <[e-mail]>
Cc: [e-mail]
Cc: [e-mail]
Cc: [e-mail]
---
V2: No change
V3: - Use devm_platform_ioremap_resource
    - Use clk_hw_onecell_data instead of clk_imx8mp_audiomix_priv
    - Include mod_devicetable.h for of_device_id struct
    - Use struct clk_parent_data instead of string parent_name
---
 drivers/clk/imx/Makefile              |   2 +-
 drivers/clk/imx/clk-imx8mp-audiomix.c | 286 ++++++++++++++++++++++++++
 2 files changed, 287 insertions(+), 1 deletion(-)
 create mode 100644 drivers/clk/imx/clk-imx8mp-audiomix.c

Comments

Abel Vesa June 27, 2022, 3:35 p.m. UTC | #1

On 22-06-25 03:32:32, Marek Vasut wrote:
> Unlike the other block control IPs in i.MX8M, the audiomix is mostly a
> series of clock gates and muxes. Model it as a large static table of
> gates and muxes with one exception, which is the PLL14xx . The PLL14xx
> SAI PLL has to be registered separately.
>

Again, there is a chance that the blk-ctrl driver might disable the PD
from under this.

Lucas, are you OK with this implementation, considering that it might
break the future work of audiomix blk-ctrl driver ?

> Signed-off-by: Marek Vasut <[e-mail]>
> Cc: Abel Vesa <[e-mail]>
> Cc: Fabio Estevam <[e-mail]>
> Cc: Jacky Bai <[e-mail]>
> Cc: Laurent Pinchart <[e-mail]>
> Cc: Lucas Stach <[e-mail]>
> Cc: Michael Turquette <[e-mail]>
> Cc: Shawn Guo <[e-mail]>
> Cc: Stephen Boyd <[e-mail]>
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> ---
> V2: No change
> V3: - Use devm_platform_ioremap_resource
>     - Use clk_hw_onecell_data instead of clk_imx8mp_audiomix_priv
>     - Include mod_devicetable.h for of_device_id struct
>     - Use struct clk_parent_data instead of string parent_name
> ---
>  drivers/clk/imx/Makefile              |   2 +-
>  drivers/clk/imx/clk-imx8mp-audiomix.c | 286 ++++++++++++++++++++++++++
>  2 files changed, 287 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/clk/imx/clk-imx8mp-audiomix.c
>
> diff --git a/drivers/clk/imx/Makefile b/drivers/clk/imx/Makefile
> index 88b9b9285d22e..c4290937637eb 100644
> --- a/drivers/clk/imx/Makefile
> +++ b/drivers/clk/imx/Makefile
> @@ -25,7 +25,7 @@ obj-$(CONFIG_MXC_CLK) += mxc-clk.o
>
>  obj-$(CONFIG_CLK_IMX8MM) += clk-imx8mm.o
>  obj-$(CONFIG_CLK_IMX8MN) += clk-imx8mn.o
> -obj-$(CONFIG_CLK_IMX8MP) += clk-imx8mp.o
> +obj-$(CONFIG_CLK_IMX8MP) += clk-imx8mp.o clk-imx8mp-audiomix.o
>  obj-$(CONFIG_CLK_IMX8MQ) += clk-imx8mq.o
>
>  obj-$(CONFIG_CLK_IMX93) += clk-imx93.o
> diff --git a/drivers/clk/imx/clk-imx8mp-audiomix.c b/drivers/clk/imx/clk-imx8mp-audiomix.c
> new file mode 100644
> index 0000000000000..2d5d8255c7fa2
> --- /dev/null
> +++ b/drivers/clk/imx/clk-imx8mp-audiomix.c
> @@ -0,0 +1,286 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Driver for i.MX8M Plus Audio BLK_CTRL
> + *
> + * Copyright (C) 2022 Marek Vasut <[e-mail]>
> + */
> +
> +#include <linux/clk-provider.h>
> +#include <linux/device.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +
> +#include <dt-bindings/clock/imx8mp-clock.h>
> +
> +#include "clk.h"
> +
> +#define CLKEN0			0x000
> +#define CLKEN1			0x004
> +#define SAI_MCLK_SEL(n)		(300 + 4 * (n))	/* n in 0..5 */
> +#define PDM_SEL			0x318
> +#define SAI_PLL_GNRL_CTL	0x400
> +
> +#define SAIn_MCLK1_PARENT(n)						\
> +static const struct clk_parent_data					\
> +clk_imx8mp_audiomix_sai##n##_mclk1_parents[] = {			\
> +	{								\
> +		.fw_name = "sai"__stringify(n),				\
> +		.name = "sai"__stringify(n)				\
> +	}, {								\
> +		.fw_name = "sai"__stringify(n)"_mclk",			\
> +		.name = "sai"__stringify(n)"_mclk"			\
> +	},								\
> +}
> +
> +SAIn_MCLK1_PARENT(1);
> +SAIn_MCLK1_PARENT(2);
> +SAIn_MCLK1_PARENT(3);
> +SAIn_MCLK1_PARENT(5);
> +SAIn_MCLK1_PARENT(6);
> +SAIn_MCLK1_PARENT(7);
> +
> +static const struct clk_parent_data clk_imx8mp_audiomix_sai_mclk2_parents[] = {
> +	{ .fw_name = "sai1", .name = "sai1" },
> +	{ .fw_name = "sai2", .name = "sai2" },
> +	{ .fw_name = "sai3", .name = "sai3" },
> +	{ .name = "dummy" },
> +	{ .fw_name = "sai5", .name = "sai5" },
> +	{ .fw_name = "sai6", .name = "sai6" },
> +	{ .fw_name = "sai7", .name = "sai7" },
> +	{ .fw_name = "sai1_mclk", .name = "sai1_mclk" },
> +	{ .fw_name = "sai2_mclk", .name = "sai2_mclk" },
> +	{ .fw_name = "sai3_mclk", .name = "sai3_mclk" },
> +	{ .name = "dummy" },
> +	{ .fw_name = "sai5_mclk", .name = "sai5_mclk" },
> +	{ .fw_name = "sai6_mclk", .name = "sai6_mclk" },
> +	{ .fw_name = "sai7_mclk", .name = "sai7_mclk" },
> +	{ .fw_name = "spdif_extclk", .name = "spdif_extclk" },
> +	{ .name = "dummy" },
> +};
> +
> +static const struct clk_parent_data clk_imx8mp_audiomix_pdm_parents[] = {
> +	{ .fw_name = "pdm", .name = "pdm" },
> +	{ .name = "sai_pll_out_div2" },
> +	{ .fw_name = "sai1_mclk", .name = "sai1_mclk" },
> +	{ .name = "dummy" },
> +};
> +
> +
> +static const struct clk_parent_data clk_imx8mp_audiomix_pll_parents[] = {
> +	{ .fw_name = "osc_24m", .name = "osc_24m" },
> +	{ .name = "dummy" },
> +	{ .name = "dummy" },
> +	{ .name = "dummy" },
> +};
> +
> +static const struct clk_parent_data clk_imx8mp_audiomix_pll_bypass_sels[] = {
> +	{ .fw_name = "sai_pll", .name = "sai_pll" },
> +	{ .fw_name = "sai_pll_ref_sel", .name = "sai_pll_ref_sel" },
> +};
> +
> +#define CLK_GATE(gname, cname)						\
> +	{								\
> +		gname"_cg",						\
> +		IMX8MP_CLK_AUDIOMIX_##cname,				\
> +		{ .fw_name = "ahb", .name = "ahb" }, NULL, 1,		\
> +		CLKEN0 + 4 * !!(IMX8MP_CLK_AUDIOMIX_##cname / 32),	\
> +		1, IMX8MP_CLK_AUDIOMIX_##cname % 32			\
> +	}
> +
> +#define CLK_SAIn(n)							\
> +	{								\
> +		"sai"__stringify(n)"_mclk1_sel",			\
> +		IMX8MP_CLK_AUDIOMIX_SAI##n##_MCLK1_SEL, {},		\
> +		clk_imx8mp_audiomix_sai##n##_mclk1_parents,		\
> +		ARRAY_SIZE(clk_imx8mp_audiomix_sai##n##_mclk1_parents), \
> +		SAI_MCLK_SEL(n), 1, 0					\
> +	}, {								\
> +		"sai"__stringify(n)"_mclk2_sel",			\
> +		IMX8MP_CLK_AUDIOMIX_SAI##n##_MCLK2_SEL, {},		\
> +		clk_imx8mp_audiomix_sai_mclk2_parents,			\
> +		ARRAY_SIZE(clk_imx8mp_audiomix_sai_mclk2_parents),	\
> +		SAI_MCLK_SEL(n), 4, 1					\
> +	}, {								\
> +		"sai"__stringify(n)"_ipg_cg",				\
> +		IMX8MP_CLK_AUDIOMIX_SAI##n##_IPG,			\
> +		{ .fw_name = "ahb", .name = "ahb" }, NULL, 1,		\
> +		CLKEN0, 1, IMX8MP_CLK_AUDIOMIX_SAI##n##_IPG		\
> +	}, {								\
> +		"sai"__stringify(n)"_mclk1_cg",				\
> +		IMX8MP_CLK_AUDIOMIX_SAI##n##_MCLK1,			\
> +		{							\
> +			.fw_name = "sai"__stringify(n)"_mclk1_sel",	\
> +			.name = "sai"__stringify(n)"_mclk1_sel"		\
> +		}, NULL, 1,						\
> +		CLKEN0, 1, IMX8MP_CLK_AUDIOMIX_SAI##n##_MCLK1		\
> +	}, {								\
> +		"sai"__stringify(n)"_mclk2_cg",				\
> +		IMX8MP_CLK_AUDIOMIX_SAI##n##_MCLK2,			\
> +		{							\
> +			.fw_name = "sai"__stringify(n)"_mclk2_sel",	\
> +			.name = "sai"__stringify(n)"_mclk2_sel"		\
> +		}, NULL, 1,						\
> +		CLKEN0, 1, IMX8MP_CLK_AUDIOMIX_SAI##n##_MCLK2		\
> +	}, {								\
> +		"sai"__stringify(n)"_mclk3_cg",				\
> +		IMX8MP_CLK_AUDIOMIX_SAI##n##_MCLK3,			\
> +		{							\
> +			.fw_name = "sai_pll_out_div2",			\
> +			.name = "sai_pll_out_div2"			\
> +		}, NULL, 1,						\
> +		CLKEN0, 1, IMX8MP_CLK_AUDIOMIX_SAI##n##_MCLK3		\
> +	}
> +
> +#define CLK_PDM								\
> +	{								\
> +		"pdm_sel", IMX8MP_CLK_AUDIOMIX_PDM_SEL, {},		\
> +		clk_imx8mp_audiomix_pdm_parents,			\
> +		ARRAY_SIZE(clk_imx8mp_audiomix_pdm_parents),		\
> +		PDM_SEL, 2, 0						\
> +	}
> +
> +struct clk_imx8mp_audiomix_sel {
> +	const char			*name;
> +	int				clkid;
> +	const struct clk_parent_data	parent;		/* For gate */
> +	const struct clk_parent_data	*parents;	/* For mux */
> +	int				num_parents;
> +	u16				reg;
> +	u8				width;
> +	u8				shift;
> +};
> +
> +static struct clk_imx8mp_audiomix_sel sels[] = {
> +	CLK_GATE("asrc", ASRC_IPG),
> +	CLK_GATE("pdm", PDM_IPG),
> +	CLK_GATE("earc", EARC_IPG),
> +	CLK_GATE("ocrama", OCRAMA_IPG),
> +	CLK_GATE("aud2htx", AUD2HTX_IPG),
> +	CLK_GATE("earc_phy", EARC_PHY),
> +	CLK_GATE("sdma2", SDMA2_ROOT),
> +	CLK_GATE("sdma3", SDMA3_ROOT),
> +	CLK_GATE("spba2", SPBA2_ROOT),
> +	CLK_GATE("dsp", DSP_ROOT),
> +	CLK_GATE("dspdbg", DSPDBG_ROOT),
> +	CLK_GATE("edma", EDMA_ROOT),
> +	CLK_GATE("audpll", AUDPLL_ROOT),
> +	CLK_GATE("mu2", MU2_ROOT),
> +	CLK_GATE("mu3", MU3_ROOT),
> +	CLK_PDM,
> +	CLK_SAIn(1),
> +	CLK_SAIn(2),
> +	CLK_SAIn(3),
> +	CLK_SAIn(5),
> +	CLK_SAIn(6),
> +	CLK_SAIn(7)
> +};
> +
> +static int clk_imx8mp_audiomix_probe(struct platform_device *pdev)
> +{
> +	struct clk_hw_onecell_data *priv;
> +	struct device *dev = &pdev->dev;
> +	void __iomem *base;
> +	struct clk_hw *hw;
> +	int i;
> +
> +	priv = devm_kzalloc(dev,
> +			    struct_size(priv, hws, IMX8MP_CLK_AUDIOMIX_END),
> +			    GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->num = IMX8MP_CLK_AUDIOMIX_END;
> +
> +	base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(base))
> +		return PTR_ERR(base);
> +
> +	for (i = 0; i < ARRAY_SIZE(sels); i++) {
> +		if (sels[i].num_parents == 1) {
> +			hw = devm_clk_hw_register_gate_parent_data(dev,
> +								   sels[i].name,
> +								   &sels[i].parent,
> +								   0,
> +								   base + sels[i].reg,
> +								   sels[i].shift,
> +								   0, NULL);
> +		} else {
> +			hw = devm_clk_hw_register_mux_parent_data(dev,
> +								  sels[i].name,
> +								  sels[i].parents,
> +								  sels[i].num_parents,
> +								  0,
> +								  base + sels[i].reg,
> +								  sels[i].shift,
> +								  sels[i].width,
> +								  0, NULL);
> +		}
> +
> +		if (IS_ERR(hw))
> +			return PTR_ERR(hw);
> +
> +		priv->hws[sels[i].clkid] = hw;
> +	}
> +
> +	/* SAI PLL */
> +	hw = devm_clk_hw_register_mux_parent_data(dev, "sai_pll_ref_sel",
> +						  clk_imx8mp_audiomix_pll_parents,
> +						  ARRAY_SIZE(clk_imx8mp_audiomix_pll_parents),
> +						  CLK_SET_RATE_NO_REPARENT,
> +						  base + SAI_PLL_GNRL_CTL,
> +						  0, 2, 0, NULL);
> +	priv->hws[IMX8MP_CLK_AUDIOMIX_SAI_PLL_REF_SEL] = hw;
> +
> +	hw = imx_dev_clk_hw_pll14xx(dev, "sai_pll", "sai_pll_ref_sel",
> +				    base + 0x400, &imx_1443x_pll);
> +	if (IS_ERR(hw))
> +		return PTR_ERR(hw);
> +	priv->hws[IMX8MP_CLK_AUDIOMIX_SAI_PLL] = hw;
> +
> +	hw = devm_clk_hw_register_mux_parent_data(dev, "sai_pll_bypass",
> +						  clk_imx8mp_audiomix_pll_bypass_sels,
> +						  ARRAY_SIZE(clk_imx8mp_audiomix_pll_bypass_sels),
> +						  CLK_SET_RATE_NO_REPARENT | CLK_SET_RATE_PARENT,
> +						  base + SAI_PLL_GNRL_CTL,
> +						  16, 1, 0, NULL);
> +	if (IS_ERR(hw))
> +		return PTR_ERR(hw);
> +	priv->hws[IMX8MP_CLK_AUDIOMIX_SAI_PLL_BYPASS] = hw;
> +
> +	hw = devm_clk_hw_register_gate(dev, "sai_pll_out", "sai_pll_bypass",
> +				       0, base + SAI_PLL_GNRL_CTL, 13,
> +				       0, NULL);
> +	if (IS_ERR(hw))
> +		return PTR_ERR(hw);
> +	priv->hws[IMX8MP_CLK_AUDIOMIX_SAI_PLL_OUT] = hw;
> +
> +	hw = devm_clk_hw_register_fixed_factor(dev, "sai_pll_out_div2",
> +					       "sai_pll_out", 0, 1, 2);
> +	if (IS_ERR(hw))
> +		return PTR_ERR(hw);
> +
> +	return devm_of_clk_add_hw_provider(&pdev->dev, of_clk_hw_onecell_get,
> +					   priv);
> +}
> +
> +static const struct of_device_id clk_imx8mp_audiomix_of_match[] = {
> +	{ .compatible = "fsl,imx8mp-audio-blk-ctrl" },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, clk_imx8mp_audiomix_of_match);
> +
> +static struct platform_driver clk_imx8mp_audiomix_driver = {
> +	.probe	= clk_imx8mp_audiomix_probe,
> +	.driver = {
> +		.name = "imx8mp-audio-blk-ctrl",
> +		.of_match_table = clk_imx8mp_audiomix_of_match,
> +	},
> +};
> +
> +module_platform_driver(clk_imx8mp_audiomix_driver);
> +
> +MODULE_AUTHOR("Marek Vasut <[e-mail]>");
> +MODULE_DESCRIPTION("Freescale i.MX8MP Audio Block Controller driver");
> +MODULE_LICENSE("GPL");
> --
> 2.35.1
>

Marek Vasut June 27, 2022, 4:23 p.m. UTC | #2

On 6/27/22 17:35, Abel Vesa wrote:
> On 22-06-25 03:32:32, Marek Vasut wrote:
>> Unlike the other block control IPs in i.MX8M, the audiomix is mostly a
>> series of clock gates and muxes. Model it as a large static table of
>> gates and muxes with one exception, which is the PLL14xx . The PLL14xx
>> SAI PLL has to be registered separately.
>>
> 
> Again, there is a chance that the blk-ctrl driver might disable the PD
> from under this.

Can you elaborate a bit more on this ? How/why do you think so ?

Abel Vesa June 28, 2022, 7:44 a.m. UTC | #3

On 22-06-27 18:23:33, Marek Vasut wrote:
> On 6/27/22 17:35, Abel Vesa wrote:
> > On 22-06-25 03:32:32, Marek Vasut wrote:
> > > Unlike the other block control IPs in i.MX8M, the audiomix is mostly a
> > > series of clock gates and muxes. Model it as a large static table of
> > > gates and muxes with one exception, which is the PLL14xx . The PLL14xx
> > > SAI PLL has to be registered separately.
> > >
> >
> > Again, there is a chance that the blk-ctrl driver might disable the PD
> > from under this.
>
> Can you elaborate a bit more on this ? How/why do you think so ?

At some point, the PDs from the Audiomix IP block will be added to the
drivers/soc/imx/imx8mp-blk-ctrl.c. Then, you'll have 2 drivers with the
same address range and the imx8mp-blk-ctrl also has runtime PM enabled.

My worry here is the possibility of imx8mp-blk-ctrl audiomix (when that
will be added) will mess with the PD leaving your clock provider driver
hanging.

Marek Vasut June 28, 2022, 5:06 p.m. UTC | #4

On 6/28/22 09:44, Abel Vesa wrote:
> On 22-06-27 18:23:33, Marek Vasut wrote:
>> On 6/27/22 17:35, Abel Vesa wrote:
>>> On 22-06-25 03:32:32, Marek Vasut wrote:
>>>> Unlike the other block control IPs in i.MX8M, the audiomix is mostly a
>>>> series of clock gates and muxes. Model it as a large static table of
>>>> gates and muxes with one exception, which is the PLL14xx . The PLL14xx
>>>> SAI PLL has to be registered separately.
>>>>
>>>
>>> Again, there is a chance that the blk-ctrl driver might disable the PD
>>> from under this.
>>
>> Can you elaborate a bit more on this ? How/why do you think so ?
> 
> At some point, the PDs from the Audiomix IP block will be added to the
> drivers/soc/imx/imx8mp-blk-ctrl.c. Then, you'll have 2 drivers with the
> same address range and the imx8mp-blk-ctrl also has runtime PM enabled.

Why would the PDs be added into the block control driver?

The audiomix is purely a clock mux driver, not really a block control 
driver providing PDs of its own.

Abel Vesa June 29, 2022, 7:41 a.m. UTC | #5

On 22-06-28 19:06:39, Marek Vasut wrote:
> On 6/28/22 09:44, Abel Vesa wrote:
> > On 22-06-27 18:23:33, Marek Vasut wrote:
> > > On 6/27/22 17:35, Abel Vesa wrote:
> > > > On 22-06-25 03:32:32, Marek Vasut wrote:
> > > > > Unlike the other block control IPs in i.MX8M, the audiomix is mostly a
> > > > > series of clock gates and muxes. Model it as a large static table of
> > > > > gates and muxes with one exception, which is the PLL14xx . The PLL14xx
> > > > > SAI PLL has to be registered separately.
> > > > >
> > > >
> > > > Again, there is a chance that the blk-ctrl driver might disable the PD
> > > > from under this.
> > >
> > > Can you elaborate a bit more on this ? How/why do you think so ?
> >
> > At some point, the PDs from the Audiomix IP block will be added to the
> > drivers/soc/imx/imx8mp-blk-ctrl.c. Then, you'll have 2 drivers with the
> > same address range and the imx8mp-blk-ctrl also has runtime PM enabled.
>
> Why would the PDs be added into the block control driver?
>
> The audiomix is purely a clock mux driver, not really a block control driver
> providing PDs of its own.

OK then, fine by me.

Abel Vesa June 29, 2022, 7:43 a.m. UTC | #6

On 22-06-25 03:32:32, Marek Vasut wrote:
> Unlike the other block control IPs in i.MX8M, the audiomix is mostly a
> series of clock gates and muxes. Model it as a large static table of
> gates and muxes with one exception, which is the PLL14xx . The PLL14xx
> SAI PLL has to be registered separately.
>
> Signed-off-by: Marek Vasut <[e-mail]>
> Cc: Abel Vesa <[e-mail]>
> Cc: Fabio Estevam <[e-mail]>
> Cc: Jacky Bai <[e-mail]>
> Cc: Laurent Pinchart <[e-mail]>
> Cc: Lucas Stach <[e-mail]>
> Cc: Michael Turquette <[e-mail]>
> Cc: Shawn Guo <[e-mail]>
> Cc: Stephen Boyd <[e-mail]>
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]

Reviewed-by: Abel Vesa <[e-mail]>

> ---
> V2: No change
> V3: - Use devm_platform_ioremap_resource
>     - Use clk_hw_onecell_data instead of clk_imx8mp_audiomix_priv
>     - Include mod_devicetable.h for of_device_id struct
>     - Use struct clk_parent_data instead of string parent_name
> ---
>  drivers/clk/imx/Makefile              |   2 +-
>  drivers/clk/imx/clk-imx8mp-audiomix.c | 286 ++++++++++++++++++++++++++
>  2 files changed, 287 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/clk/imx/clk-imx8mp-audiomix.c
>
> diff --git a/drivers/clk/imx/Makefile b/drivers/clk/imx/Makefile
> index 88b9b9285d22e..c4290937637eb 100644
> --- a/drivers/clk/imx/Makefile
> +++ b/drivers/clk/imx/Makefile
> @@ -25,7 +25,7 @@ obj-$(CONFIG_MXC_CLK) += mxc-clk.o
>
>  obj-$(CONFIG_CLK_IMX8MM) += clk-imx8mm.o
>  obj-$(CONFIG_CLK_IMX8MN) += clk-imx8mn.o
> -obj-$(CONFIG_CLK_IMX8MP) += clk-imx8mp.o
> +obj-$(CONFIG_CLK_IMX8MP) += clk-imx8mp.o clk-imx8mp-audiomix.o
>  obj-$(CONFIG_CLK_IMX8MQ) += clk-imx8mq.o
>
>  obj-$(CONFIG_CLK_IMX93) += clk-imx93.o
> diff --git a/drivers/clk/imx/clk-imx8mp-audiomix.c b/drivers/clk/imx/clk-imx8mp-audiomix.c
> new file mode 100644
> index 0000000000000..2d5d8255c7fa2
> --- /dev/null
> +++ b/drivers/clk/imx/clk-imx8mp-audiomix.c
> @@ -0,0 +1,286 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Driver for i.MX8M Plus Audio BLK_CTRL
> + *
> + * Copyright (C) 2022 Marek Vasut <[e-mail]>
> + */
> +
> +#include <linux/clk-provider.h>
> +#include <linux/device.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +
> +#include <dt-bindings/clock/imx8mp-clock.h>
> +
> +#include "clk.h"
> +
> +#define CLKEN0			0x000
> +#define CLKEN1			0x004
> +#define SAI_MCLK_SEL(n)		(300 + 4 * (n))	/* n in 0..5 */
> +#define PDM_SEL			0x318
> +#define SAI_PLL_GNRL_CTL	0x400
> +
> +#define SAIn_MCLK1_PARENT(n)						\
> +static const struct clk_parent_data					\
> +clk_imx8mp_audiomix_sai##n##_mclk1_parents[] = {			\
> +	{								\
> +		.fw_name = "sai"__stringify(n),				\
> +		.name = "sai"__stringify(n)				\
> +	}, {								\
> +		.fw_name = "sai"__stringify(n)"_mclk",			\
> +		.name = "sai"__stringify(n)"_mclk"			\
> +	},								\
> +}
> +
> +SAIn_MCLK1_PARENT(1);
> +SAIn_MCLK1_PARENT(2);
> +SAIn_MCLK1_PARENT(3);
> +SAIn_MCLK1_PARENT(5);
> +SAIn_MCLK1_PARENT(6);
> +SAIn_MCLK1_PARENT(7);
> +
> +static const struct clk_parent_data clk_imx8mp_audiomix_sai_mclk2_parents[] = {
> +	{ .fw_name = "sai1", .name = "sai1" },
> +	{ .fw_name = "sai2", .name = "sai2" },
> +	{ .fw_name = "sai3", .name = "sai3" },
> +	{ .name = "dummy" },
> +	{ .fw_name = "sai5", .name = "sai5" },
> +	{ .fw_name = "sai6", .name = "sai6" },
> +	{ .fw_name = "sai7", .name = "sai7" },
> +	{ .fw_name = "sai1_mclk", .name = "sai1_mclk" },
> +	{ .fw_name = "sai2_mclk", .name = "sai2_mclk" },
> +	{ .fw_name = "sai3_mclk", .name = "sai3_mclk" },
> +	{ .name = "dummy" },
> +	{ .fw_name = "sai5_mclk", .name = "sai5_mclk" },
> +	{ .fw_name = "sai6_mclk", .name = "sai6_mclk" },
> +	{ .fw_name = "sai7_mclk", .name = "sai7_mclk" },
> +	{ .fw_name = "spdif_extclk", .name = "spdif_extclk" },
> +	{ .name = "dummy" },
> +};
> +
> +static const struct clk_parent_data clk_imx8mp_audiomix_pdm_parents[] = {
> +	{ .fw_name = "pdm", .name = "pdm" },
> +	{ .name = "sai_pll_out_div2" },
> +	{ .fw_name = "sai1_mclk", .name = "sai1_mclk" },
> +	{ .name = "dummy" },
> +};
> +
> +
> +static const struct clk_parent_data clk_imx8mp_audiomix_pll_parents[] = {
> +	{ .fw_name = "osc_24m", .name = "osc_24m" },
> +	{ .name = "dummy" },
> +	{ .name = "dummy" },
> +	{ .name = "dummy" },
> +};
> +
> +static const struct clk_parent_data clk_imx8mp_audiomix_pll_bypass_sels[] = {
> +	{ .fw_name = "sai_pll", .name = "sai_pll" },
> +	{ .fw_name = "sai_pll_ref_sel", .name = "sai_pll_ref_sel" },
> +};
> +
> +#define CLK_GATE(gname, cname)						\
> +	{								\
> +		gname"_cg",						\
> +		IMX8MP_CLK_AUDIOMIX_##cname,				\
> +		{ .fw_name = "ahb", .name = "ahb" }, NULL, 1,		\
> +		CLKEN0 + 4 * !!(IMX8MP_CLK_AUDIOMIX_##cname / 32),	\
> +		1, IMX8MP_CLK_AUDIOMIX_##cname % 32			\
> +	}
> +
> +#define CLK_SAIn(n)							\
> +	{								\
> +		"sai"__stringify(n)"_mclk1_sel",			\
> +		IMX8MP_CLK_AUDIOMIX_SAI##n##_MCLK1_SEL, {},		\
> +		clk_imx8mp_audiomix_sai##n##_mclk1_parents,		\
> +		ARRAY_SIZE(clk_imx8mp_audiomix_sai##n##_mclk1_parents), \
> +		SAI_MCLK_SEL(n), 1, 0					\
> +	}, {								\
> +		"sai"__stringify(n)"_mclk2_sel",			\
> +		IMX8MP_CLK_AUDIOMIX_SAI##n##_MCLK2_SEL, {},		\
> +		clk_imx8mp_audiomix_sai_mclk2_parents,			\
> +		ARRAY_SIZE(clk_imx8mp_audiomix_sai_mclk2_parents),	\
> +		SAI_MCLK_SEL(n), 4, 1					\
> +	}, {								\
> +		"sai"__stringify(n)"_ipg_cg",				\
> +		IMX8MP_CLK_AUDIOMIX_SAI##n##_IPG,			\
> +		{ .fw_name = "ahb", .name = "ahb" }, NULL, 1,		\
> +		CLKEN0, 1, IMX8MP_CLK_AUDIOMIX_SAI##n##_IPG		\
> +	}, {								\
> +		"sai"__stringify(n)"_mclk1_cg",				\
> +		IMX8MP_CLK_AUDIOMIX_SAI##n##_MCLK1,			\
> +		{							\
> +			.fw_name = "sai"__stringify(n)"_mclk1_sel",	\
> +			.name = "sai"__stringify(n)"_mclk1_sel"		\
> +		}, NULL, 1,						\
> +		CLKEN0, 1, IMX8MP_CLK_AUDIOMIX_SAI##n##_MCLK1		\
> +	}, {								\
> +		"sai"__stringify(n)"_mclk2_cg",				\
> +		IMX8MP_CLK_AUDIOMIX_SAI##n##_MCLK2,			\
> +		{							\
> +			.fw_name = "sai"__stringify(n)"_mclk2_sel",	\
> +			.name = "sai"__stringify(n)"_mclk2_sel"		\
> +		}, NULL, 1,						\
> +		CLKEN0, 1, IMX8MP_CLK_AUDIOMIX_SAI##n##_MCLK2		\
> +	}, {								\
> +		"sai"__stringify(n)"_mclk3_cg",				\
> +		IMX8MP_CLK_AUDIOMIX_SAI##n##_MCLK3,			\
> +		{							\
> +			.fw_name = "sai_pll_out_div2",			\
> +			.name = "sai_pll_out_div2"			\
> +		}, NULL, 1,						\
> +		CLKEN0, 1, IMX8MP_CLK_AUDIOMIX_SAI##n##_MCLK3		\
> +	}
> +
> +#define CLK_PDM								\
> +	{								\
> +		"pdm_sel", IMX8MP_CLK_AUDIOMIX_PDM_SEL, {},		\
> +		clk_imx8mp_audiomix_pdm_parents,			\
> +		ARRAY_SIZE(clk_imx8mp_audiomix_pdm_parents),		\
> +		PDM_SEL, 2, 0						\
> +	}
> +
> +struct clk_imx8mp_audiomix_sel {
> +	const char			*name;
> +	int				clkid;
> +	const struct clk_parent_data	parent;		/* For gate */
> +	const struct clk_parent_data	*parents;	/* For mux */
> +	int				num_parents;
> +	u16				reg;
> +	u8				width;
> +	u8				shift;
> +};
> +
> +static struct clk_imx8mp_audiomix_sel sels[] = {
> +	CLK_GATE("asrc", ASRC_IPG),
> +	CLK_GATE("pdm", PDM_IPG),
> +	CLK_GATE("earc", EARC_IPG),
> +	CLK_GATE("ocrama", OCRAMA_IPG),
> +	CLK_GATE("aud2htx", AUD2HTX_IPG),
> +	CLK_GATE("earc_phy", EARC_PHY),
> +	CLK_GATE("sdma2", SDMA2_ROOT),
> +	CLK_GATE("sdma3", SDMA3_ROOT),
> +	CLK_GATE("spba2", SPBA2_ROOT),
> +	CLK_GATE("dsp", DSP_ROOT),
> +	CLK_GATE("dspdbg", DSPDBG_ROOT),
> +	CLK_GATE("edma", EDMA_ROOT),
> +	CLK_GATE("audpll", AUDPLL_ROOT),
> +	CLK_GATE("mu2", MU2_ROOT),
> +	CLK_GATE("mu3", MU3_ROOT),
> +	CLK_PDM,
> +	CLK_SAIn(1),
> +	CLK_SAIn(2),
> +	CLK_SAIn(3),
> +	CLK_SAIn(5),
> +	CLK_SAIn(6),
> +	CLK_SAIn(7)
> +};
> +
> +static int clk_imx8mp_audiomix_probe(struct platform_device *pdev)
> +{
> +	struct clk_hw_onecell_data *priv;
> +	struct device *dev = &pdev->dev;
> +	void __iomem *base;
> +	struct clk_hw *hw;
> +	int i;
> +
> +	priv = devm_kzalloc(dev,
> +			    struct_size(priv, hws, IMX8MP_CLK_AUDIOMIX_END),
> +			    GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->num = IMX8MP_CLK_AUDIOMIX_END;
> +
> +	base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(base))
> +		return PTR_ERR(base);
> +
> +	for (i = 0; i < ARRAY_SIZE(sels); i++) {
> +		if (sels[i].num_parents == 1) {
> +			hw = devm_clk_hw_register_gate_parent_data(dev,
> +								   sels[i].name,
> +								   &sels[i].parent,
> +								   0,
> +								   base + sels[i].reg,
> +								   sels[i].shift,
> +								   0, NULL);
> +		} else {
> +			hw = devm_clk_hw_register_mux_parent_data(dev,
> +								  sels[i].name,
> +								  sels[i].parents,
> +								  sels[i].num_parents,
> +								  0,
> +								  base + sels[i].reg,
> +								  sels[i].shift,
> +								  sels[i].width,
> +								  0, NULL);
> +		}
> +
> +		if (IS_ERR(hw))
> +			return PTR_ERR(hw);
> +
> +		priv->hws[sels[i].clkid] = hw;
> +	}
> +
> +	/* SAI PLL */
> +	hw = devm_clk_hw_register_mux_parent_data(dev, "sai_pll_ref_sel",
> +						  clk_imx8mp_audiomix_pll_parents,
> +						  ARRAY_SIZE(clk_imx8mp_audiomix_pll_parents),
> +						  CLK_SET_RATE_NO_REPARENT,
> +						  base + SAI_PLL_GNRL_CTL,
> +						  0, 2, 0, NULL);
> +	priv->hws[IMX8MP_CLK_AUDIOMIX_SAI_PLL_REF_SEL] = hw;
> +
> +	hw = imx_dev_clk_hw_pll14xx(dev, "sai_pll", "sai_pll_ref_sel",
> +				    base + 0x400, &imx_1443x_pll);
> +	if (IS_ERR(hw))
> +		return PTR_ERR(hw);
> +	priv->hws[IMX8MP_CLK_AUDIOMIX_SAI_PLL] = hw;
> +
> +	hw = devm_clk_hw_register_mux_parent_data(dev, "sai_pll_bypass",
> +						  clk_imx8mp_audiomix_pll_bypass_sels,
> +						  ARRAY_SIZE(clk_imx8mp_audiomix_pll_bypass_sels),
> +						  CLK_SET_RATE_NO_REPARENT | CLK_SET_RATE_PARENT,
> +						  base + SAI_PLL_GNRL_CTL,
> +						  16, 1, 0, NULL);
> +	if (IS_ERR(hw))
> +		return PTR_ERR(hw);
> +	priv->hws[IMX8MP_CLK_AUDIOMIX_SAI_PLL_BYPASS] = hw;
> +
> +	hw = devm_clk_hw_register_gate(dev, "sai_pll_out", "sai_pll_bypass",
> +				       0, base + SAI_PLL_GNRL_CTL, 13,
> +				       0, NULL);
> +	if (IS_ERR(hw))
> +		return PTR_ERR(hw);
> +	priv->hws[IMX8MP_CLK_AUDIOMIX_SAI_PLL_OUT] = hw;
> +
> +	hw = devm_clk_hw_register_fixed_factor(dev, "sai_pll_out_div2",
> +					       "sai_pll_out", 0, 1, 2);
> +	if (IS_ERR(hw))
> +		return PTR_ERR(hw);
> +
> +	return devm_of_clk_add_hw_provider(&pdev->dev, of_clk_hw_onecell_get,
> +					   priv);
> +}
> +
> +static const struct of_device_id clk_imx8mp_audiomix_of_match[] = {
> +	{ .compatible = "fsl,imx8mp-audio-blk-ctrl" },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, clk_imx8mp_audiomix_of_match);
> +
> +static struct platform_driver clk_imx8mp_audiomix_driver = {
> +	.probe	= clk_imx8mp_audiomix_probe,
> +	.driver = {
> +		.name = "imx8mp-audio-blk-ctrl",
> +		.of_match_table = clk_imx8mp_audiomix_of_match,
> +	},
> +};
> +
> +module_platform_driver(clk_imx8mp_audiomix_driver);
> +
> +MODULE_AUTHOR("Marek Vasut <[e-mail]>");
> +MODULE_DESCRIPTION("Freescale i.MX8MP Audio Block Controller driver");
> +MODULE_LICENSE("GPL");
> --
> 2.35.1
>

Peng Fan Aug. 4, 2022, 9:13 a.m. UTC | #7

> Subject: Re: [PATCH v3 3/6] clk: imx: imx8mp: Add audiomix block control
> 
> On 6/28/22 09:44, Abel Vesa wrote:
> > On 22-06-27 18:23:33, Marek Vasut wrote:
> >> On 6/27/22 17:35, Abel Vesa wrote:
> >>> On 22-06-25 03:32:32, Marek Vasut wrote:
> >>>> Unlike the other block control IPs in i.MX8M, the audiomix is
> >>>> mostly a series of clock gates and muxes. Model it as a large
> >>>> static table of gates and muxes with one exception, which is the
> >>>> PLL14xx . The PLL14xx SAI PLL has to be registered separately.
> >>>>
> >>>
> >>> Again, there is a chance that the blk-ctrl driver might disable the
> >>> PD from under this.
> >>
> >> Can you elaborate a bit more on this ? How/why do you think so ?
> >
> > At some point, the PDs from the Audiomix IP block will be added to the
> > drivers/soc/imx/imx8mp-blk-ctrl.c. Then, you'll have 2 drivers with
> > the same address range and the imx8mp-blk-ctrl also has runtime PM
> enabled.
> 
> Why would the PDs be added into the block control driver?
> 
> The audiomix is purely a clock mux driver, not really a block control driver
> providing PDs of its own.

I recalled that with with blk-ctrl working as clock provider, there is dead lock
issue, if the blk-ctrl node has a power-domain entry. Not very sure.

Regards,
Peng.

Marek Vasut Aug. 4, 2022, 9:31 a.m. UTC | #8

On 8/4/22 11:13, Peng Fan wrote:
>> Subject: Re: [PATCH v3 3/6] clk: imx: imx8mp: Add audiomix block control
>>
>> On 6/28/22 09:44, Abel Vesa wrote:
>>> On 22-06-27 18:23:33, Marek Vasut wrote:
>>>> On 6/27/22 17:35, Abel Vesa wrote:
>>>>> On 22-06-25 03:32:32, Marek Vasut wrote:
>>>>>> Unlike the other block control IPs in i.MX8M, the audiomix is
>>>>>> mostly a series of clock gates and muxes. Model it as a large
>>>>>> static table of gates and muxes with one exception, which is the
>>>>>> PLL14xx . The PLL14xx SAI PLL has to be registered separately.
>>>>>>
>>>>>
>>>>> Again, there is a chance that the blk-ctrl driver might disable the
>>>>> PD from under this.
>>>>
>>>> Can you elaborate a bit more on this ? How/why do you think so ?
>>>
>>> At some point, the PDs from the Audiomix IP block will be added to the
>>> drivers/soc/imx/imx8mp-blk-ctrl.c. Then, you'll have 2 drivers with
>>> the same address range and the imx8mp-blk-ctrl also has runtime PM
>> enabled.
>>
>> Why would the PDs be added into the block control driver?
>>
>> The audiomix is purely a clock mux driver, not really a block control driver
>> providing PDs of its own.
> 
> I recalled that with with blk-ctrl working as clock provider, there is dead lock
> issue, if the blk-ctrl node has a power-domain entry. Not very sure.

How can I verify that ? Lockdep ?

I run this series for months and haven't seen a lock up or splat.

Abel Vesa Aug. 11, 2022, 2:20 p.m. UTC | #9

On 22-08-04 11:31:33, Marek Vasut wrote:
> On 8/4/22 11:13, Peng Fan wrote:
> > > Subject: Re: [PATCH v3 3/6] clk: imx: imx8mp: Add audiomix block control
> > > 
> > > On 6/28/22 09:44, Abel Vesa wrote:
> > > > On 22-06-27 18:23:33, Marek Vasut wrote:
> > > > > On 6/27/22 17:35, Abel Vesa wrote:
> > > > > > On 22-06-25 03:32:32, Marek Vasut wrote:
> > > > > > > Unlike the other block control IPs in i.MX8M, the audiomix is
> > > > > > > mostly a series of clock gates and muxes. Model it as a large
> > > > > > > static table of gates and muxes with one exception, which is the
> > > > > > > PLL14xx . The PLL14xx SAI PLL has to be registered separately.
> > > > > > > 
> > > > > > 
> > > > > > Again, there is a chance that the blk-ctrl driver might disable the
> > > > > > PD from under this.
> > > > > 
> > > > > Can you elaborate a bit more on this ? How/why do you think so ?
> > > > 
> > > > At some point, the PDs from the Audiomix IP block will be added to the
> > > > drivers/soc/imx/imx8mp-blk-ctrl.c. Then, you'll have 2 drivers with
> > > > the same address range and the imx8mp-blk-ctrl also has runtime PM
> > > enabled.
> > > 
> > > Why would the PDs be added into the block control driver?
> > > 
> > > The audiomix is purely a clock mux driver, not really a block control driver
> > > providing PDs of its own.
> > 
> > I recalled that with with blk-ctrl working as clock provider, there is dead lock
> > issue, if the blk-ctrl node has a power-domain entry. Not very sure.
> 
> How can I verify that ? Lockdep ?
> 
> I run this series for months and haven't seen a lock up or splat.

Audiomix (and every mix actually) has a PD. Once you add the PD, you'll
end up with an ABBA deadlock between genpd lock and clock prepare lock.

Have a read here:

https://lore.kernel.org/lkml/[e-mail]/T/#m0160265b0604ac8a524fedae7845e9f60bae67ef

Marek Vasut Aug. 11, 2022, 2:30 p.m. UTC | #10

On 8/11/22 16:20, Abel Vesa wrote:
> On 22-08-04 11:31:33, Marek Vasut wrote:
>> On 8/4/22 11:13, Peng Fan wrote:
>>>> Subject: Re: [PATCH v3 3/6] clk: imx: imx8mp: Add audiomix block control
>>>>
>>>> On 6/28/22 09:44, Abel Vesa wrote:
>>>>> On 22-06-27 18:23:33, Marek Vasut wrote:
>>>>>> On 6/27/22 17:35, Abel Vesa wrote:
>>>>>>> On 22-06-25 03:32:32, Marek Vasut wrote:
>>>>>>>> Unlike the other block control IPs in i.MX8M, the audiomix is
>>>>>>>> mostly a series of clock gates and muxes. Model it as a large
>>>>>>>> static table of gates and muxes with one exception, which is the
>>>>>>>> PLL14xx . The PLL14xx SAI PLL has to be registered separately.
>>>>>>>>
>>>>>>>
>>>>>>> Again, there is a chance that the blk-ctrl driver might disable the
>>>>>>> PD from under this.
>>>>>>
>>>>>> Can you elaborate a bit more on this ? How/why do you think so ?
>>>>>
>>>>> At some point, the PDs from the Audiomix IP block will be added to the
>>>>> drivers/soc/imx/imx8mp-blk-ctrl.c. Then, you'll have 2 drivers with
>>>>> the same address range and the imx8mp-blk-ctrl also has runtime PM
>>>> enabled.
>>>>
>>>> Why would the PDs be added into the block control driver?
>>>>
>>>> The audiomix is purely a clock mux driver, not really a block control driver
>>>> providing PDs of its own.
>>>
>>> I recalled that with with blk-ctrl working as clock provider, there is dead lock
>>> issue, if the blk-ctrl node has a power-domain entry. Not very sure.
>>
>> How can I verify that ? Lockdep ?
>>
>> I run this series for months and haven't seen a lock up or splat.
> 
> Audiomix (and every mix actually) has a PD. Once you add the PD, you'll
> end up with an ABBA deadlock between genpd lock and clock prepare lock.

Unlike the other mix drivers, this is a pure clock driver, not a power 
domain driver. The PD is already available to this clock driver, see:
[PATCH v3 5/6] arm64: dts: imx8mp: Add SAI, SDMA, AudioMIX

Can you please elaborate on the deadlock problem ?
Because really, I just don't see it.

Were you able to reproduce the deadlock with this driver ?

> Have a read here:
> 
> https://lore.kernel.org/lkml/[e-mail]/T/#m0160265b0604ac8a524fedae7845e9f60bae67ef

Which part of the lengthy thread do you refer to ? I suspect the 
'permalink' might help pointing to specific email in the thread.

Note that the aforementioned thread discusses the other mix drivers 
which are PDs, this driver is not, there is a difference.

Abel Vesa Aug. 11, 2022, 3:03 p.m. UTC | #11

On 22-08-11 16:30:20, Marek Vasut wrote:
> On 8/11/22 16:20, Abel Vesa wrote:
> > On 22-08-04 11:31:33, Marek Vasut wrote:
> > > On 8/4/22 11:13, Peng Fan wrote:
> > > > > Subject: Re: [PATCH v3 3/6] clk: imx: imx8mp: Add audiomix block control
> > > > > 
> > > > > On 6/28/22 09:44, Abel Vesa wrote:
> > > > > > On 22-06-27 18:23:33, Marek Vasut wrote:
> > > > > > > On 6/27/22 17:35, Abel Vesa wrote:
> > > > > > > > On 22-06-25 03:32:32, Marek Vasut wrote:
> > > > > > > > > Unlike the other block control IPs in i.MX8M, the audiomix is
> > > > > > > > > mostly a series of clock gates and muxes. Model it as a large
> > > > > > > > > static table of gates and muxes with one exception, which is the
> > > > > > > > > PLL14xx . The PLL14xx SAI PLL has to be registered separately.
> > > > > > > > > 
> > > > > > > > 
> > > > > > > > Again, there is a chance that the blk-ctrl driver might disable the
> > > > > > > > PD from under this.
> > > > > > > 
> > > > > > > Can you elaborate a bit more on this ? How/why do you think so ?
> > > > > > 
> > > > > > At some point, the PDs from the Audiomix IP block will be added to the
> > > > > > drivers/soc/imx/imx8mp-blk-ctrl.c. Then, you'll have 2 drivers with
> > > > > > the same address range and the imx8mp-blk-ctrl also has runtime PM
> > > > > enabled.
> > > > > 
> > > > > Why would the PDs be added into the block control driver?
> > > > > 
> > > > > The audiomix is purely a clock mux driver, not really a block control driver
> > > > > providing PDs of its own.
> > > > 
> > > > I recalled that with with blk-ctrl working as clock provider, there is dead lock
> > > > issue, if the blk-ctrl node has a power-domain entry. Not very sure.
> > > 
> > > How can I verify that ? Lockdep ?
> > > 
> > > I run this series for months and haven't seen a lock up or splat.
> > 
> > Audiomix (and every mix actually) has a PD. Once you add the PD, you'll
> > end up with an ABBA deadlock between genpd lock and clock prepare lock.
> 
> Unlike the other mix drivers, this is a pure clock driver, not a power
> domain driver. The PD is already available to this clock driver, see:
> [PATCH v3 5/6] arm64: dts: imx8mp: Add SAI, SDMA, AudioMIX

When you will enable the runtime PM for this driver, the deadlock is
going to happen and it will be in some scenario like:
    clk_disable_unused_subtree
      -> clk_prepare (takes prepare lock) (for a clock from your driver)
	-> runtime pm (takes genpd lock)
	  -> clk_prepare (tries to take prepare lock again) (for the clock of the PD)

> 
> Can you please elaborate on the deadlock problem ?
> Because really, I just don't see it.
> 
> Were you able to reproduce the deadlock with this driver ?
> 
> > Have a read here:
> > 
> > https://lore.kernel.org/lkml/[e-mail]/T/#m0160265b0604ac8a524fedae7845e9f60bae67ef
> 
> Which part of the lengthy thread do you refer to ? I suspect the 'permalink'
> might help pointing to specific email in the thread.
> 
> Note that the aforementioned thread discusses the other mix drivers which
> are PDs, this driver is not, there is a difference.

Abel Vesa Aug. 11, 2022, 3:14 p.m. UTC | #12

On 22-08-11 18:03:04, Abel Vesa wrote:
> On 22-08-11 16:30:20, Marek Vasut wrote:
> > On 8/11/22 16:20, Abel Vesa wrote:
> > > On 22-08-04 11:31:33, Marek Vasut wrote:
> > > > On 8/4/22 11:13, Peng Fan wrote:
> > > > > > Subject: Re: [PATCH v3 3/6] clk: imx: imx8mp: Add audiomix block control
> > > > > > 
> > > > > > On 6/28/22 09:44, Abel Vesa wrote:
> > > > > > > On 22-06-27 18:23:33, Marek Vasut wrote:
> > > > > > > > On 6/27/22 17:35, Abel Vesa wrote:
> > > > > > > > > On 22-06-25 03:32:32, Marek Vasut wrote:
> > > > > > > > > > Unlike the other block control IPs in i.MX8M, the audiomix is
> > > > > > > > > > mostly a series of clock gates and muxes. Model it as a large
> > > > > > > > > > static table of gates and muxes with one exception, which is the
> > > > > > > > > > PLL14xx . The PLL14xx SAI PLL has to be registered separately.
> > > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > Again, there is a chance that the blk-ctrl driver might disable the
> > > > > > > > > PD from under this.
> > > > > > > > 
> > > > > > > > Can you elaborate a bit more on this ? How/why do you think so ?
> > > > > > > 
> > > > > > > At some point, the PDs from the Audiomix IP block will be added to the
> > > > > > > drivers/soc/imx/imx8mp-blk-ctrl.c. Then, you'll have 2 drivers with
> > > > > > > the same address range and the imx8mp-blk-ctrl also has runtime PM
> > > > > > enabled.
> > > > > > 
> > > > > > Why would the PDs be added into the block control driver?
> > > > > > 
> > > > > > The audiomix is purely a clock mux driver, not really a block control driver
> > > > > > providing PDs of its own.
> > > > > 
> > > > > I recalled that with with blk-ctrl working as clock provider, there is dead lock
> > > > > issue, if the blk-ctrl node has a power-domain entry. Not very sure.
> > > > 
> > > > How can I verify that ? Lockdep ?
> > > > 
> > > > I run this series for months and haven't seen a lock up or splat.
> > > 
> > > Audiomix (and every mix actually) has a PD. Once you add the PD, you'll
> > > end up with an ABBA deadlock between genpd lock and clock prepare lock.
> > 
> > Unlike the other mix drivers, this is a pure clock driver, not a power
> > domain driver. The PD is already available to this clock driver, see:
> > [PATCH v3 5/6] arm64: dts: imx8mp: Add SAI, SDMA, AudioMIX
> 
> When you will enable the runtime PM for this driver, the deadlock is
> going to happen and it will be in some scenario like:
>     clk_disable_unused_subtree
>       -> clk_prepare (takes prepare lock) (for a clock from your driver)
> 	-> runtime pm (takes genpd lock)
> 	  -> clk_prepare (tries to take prepare lock again) (for the clock of the PD)
> 

Actually, I'm wrong, that is not a deadlock, but this one is:

[   11.667711][  T108] -> #1 (&genpd->mlock){+.+.}-{3:3}:
[   11.675041][  T108]        __lock_acquire+0xae4/0xef8
[   11.680093][  T108]        lock_acquire+0xfc/0x2f8
[   11.684888][  T108]        __mutex_lock+0x90/0x870
[   11.689685][  T108]        mutex_lock_nested+0x44/0x50
[   11.694826][  T108]        genpd_lock_mtx+0x18/0x24
[   11.699706][  T108]        genpd_runtime_resume+0x90/0x214 (hold genpd->mlock)
[   11.705194][  T108]        __rpm_callback+0x80/0x2c0
[   11.710160][  T108]        rpm_resume+0x468/0x650
[   11.714866][  T108]        __pm_runtime_resume+0x60/0x88
[   11.720180][  T108]        clk_pm_runtime_get+0x28/0x9c
[   11.725410][  T108]        clk_disable_unused_subtree+0x8c/0x144
[   11.731420][  T108]        clk_disable_unused_subtree+0x124/0x144
[   11.737518][  T108]        clk_disable_unused+0xa4/0x11c (hold prepare_lock)
[   11.742833][  T108]        do_one_initcall+0x98/0x178
[   11.747888][  T108]        do_initcall_level+0x9c/0xb8
[   11.753028][  T108]        do_initcalls+0x54/0x94
[   11.757736][  T108]        do_basic_setup+0x24/0x30
[   11.762614][  T108]        kernel_init_freeable+0x70/0xa4
[   11.768014][  T108]        kernel_init+0x14/0x18c
[   11.772722][  T108]        ret_from_fork+0x10/0x18

[   11.777512][  T108] -> #0 (prepare_lock){+.+.}-{3:3}:
[   11.784749][  T108]        check_noncircular+0x134/0x13c
[   11.790064][  T108]        validate_chain+0x590/0x2a04
[   11.795204][  T108]        __lock_acquire+0xae4/0xef8
[   11.800258][  T108]        lock_acquire+0xfc/0x2f8
[   11.805050][  T108]        __mutex_lock+0x90/0x870
[   11.809841][  T108]        mutex_lock_nested+0x44/0x50
[   11.814983][  T108]        clk_unprepare+0x5c/0x100 ((hold prepare_lock))
[   11.819864][  T108]        imx8m_pd_power_off+0xac/0x110
[   11.825179][  T108]        genpd_power_off+0x1b4/0x2dc
[   11.830318][  T108]        genpd_power_off_work_fn+0x38/0x58 (hold genpd->mlock)
[   11.835981][  T108]        process_one_work+0x270/0x444
[   11.841208][  T108]        worker_thread+0x280/0x4e4
[   11.846176][  T108]        kthread+0x13c/0x14

All it needs is the runtime PM in your current driver.

Bottom line is, we cannot have clock providers that have PDs that have
in turn their own clocks.

So we need to drop the blk_ctrl as clock providers and go with Lucas's
approach.

> > 
> > Can you please elaborate on the deadlock problem ?
> > Because really, I just don't see it.
> > 
> > Were you able to reproduce the deadlock with this driver ?
> > 
> > > Have a read here:
> > > 
> > > https://lore.kernel.org/lkml/[e-mail]/T/#m0160265b0604ac8a524fedae7845e9f60bae67ef
> > 
> > Which part of the lengthy thread do you refer to ? I suspect the 'permalink'
> > might help pointing to specific email in the thread.
> > 
> > Note that the aforementioned thread discusses the other mix drivers which
> > are PDs, this driver is not, there is a difference.

Marek Vasut Aug. 11, 2022, 4:38 p.m. UTC | #13

On 8/11/22 17:03, Abel Vesa wrote:
> On 22-08-11 16:30:20, Marek Vasut wrote:
>> On 8/11/22 16:20, Abel Vesa wrote:
>>> On 22-08-04 11:31:33, Marek Vasut wrote:
>>>> On 8/4/22 11:13, Peng Fan wrote:
>>>>>> Subject: Re: [PATCH v3 3/6] clk: imx: imx8mp: Add audiomix block control
>>>>>>
>>>>>> On 6/28/22 09:44, Abel Vesa wrote:
>>>>>>> On 22-06-27 18:23:33, Marek Vasut wrote:
>>>>>>>> On 6/27/22 17:35, Abel Vesa wrote:
>>>>>>>>> On 22-06-25 03:32:32, Marek Vasut wrote:
>>>>>>>>>> Unlike the other block control IPs in i.MX8M, the audiomix is
>>>>>>>>>> mostly a series of clock gates and muxes. Model it as a large
>>>>>>>>>> static table of gates and muxes with one exception, which is the
>>>>>>>>>> PLL14xx . The PLL14xx SAI PLL has to be registered separately.
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Again, there is a chance that the blk-ctrl driver might disable the
>>>>>>>>> PD from under this.
>>>>>>>>
>>>>>>>> Can you elaborate a bit more on this ? How/why do you think so ?
>>>>>>>
>>>>>>> At some point, the PDs from the Audiomix IP block will be added to the
>>>>>>> drivers/soc/imx/imx8mp-blk-ctrl.c. Then, you'll have 2 drivers with
>>>>>>> the same address range and the imx8mp-blk-ctrl also has runtime PM
>>>>>> enabled.
>>>>>>
>>>>>> Why would the PDs be added into the block control driver?
>>>>>>
>>>>>> The audiomix is purely a clock mux driver, not really a block control driver
>>>>>> providing PDs of its own.
>>>>>
>>>>> I recalled that with with blk-ctrl working as clock provider, there is dead lock
>>>>> issue, if the blk-ctrl node has a power-domain entry. Not very sure.
>>>>
>>>> How can I verify that ? Lockdep ?
>>>>
>>>> I run this series for months and haven't seen a lock up or splat.
>>>
>>> Audiomix (and every mix actually) has a PD. Once you add the PD, you'll
>>> end up with an ABBA deadlock between genpd lock and clock prepare lock.
>>
>> Unlike the other mix drivers, this is a pure clock driver, not a power
>> domain driver. The PD is already available to this clock driver, see:
>> [PATCH v3 5/6] arm64: dts: imx8mp: Add SAI, SDMA, AudioMIX
> 
> When you will enable the runtime PM for this driver, the deadlock is
> going to happen and it will be in some scenario like:
>      clk_disable_unused_subtree
>        -> clk_prepare (takes prepare lock) (for a clock from your driver)
> 	-> runtime pm (takes genpd lock)
> 	  -> clk_prepare (tries to take prepare lock again) (for the clock of the PD)

Since you seem to have a test case, can you share the test case, 
verbatim, so I can reproduce it locally ?

I seem to be asking for that repeatedly and I am not getting any clear 
answer.

>> Can you please elaborate on the deadlock problem ?
>> Because really, I just don't see it.
>>
>> Were you able to reproduce the deadlock with this driver ?

[...]

Abel Vesa Aug. 11, 2022, 4:51 p.m. UTC | #14

On 22-08-11 18:38:49, Marek Vasut wrote:
> On 8/11/22 17:03, Abel Vesa wrote:
> > On 22-08-11 16:30:20, Marek Vasut wrote:
> > > On 8/11/22 16:20, Abel Vesa wrote:
> > > > On 22-08-04 11:31:33, Marek Vasut wrote:
> > > > > On 8/4/22 11:13, Peng Fan wrote:
> > > > > > > Subject: Re: [PATCH v3 3/6] clk: imx: imx8mp: Add audiomix block control
> > > > > > > 
> > > > > > > On 6/28/22 09:44, Abel Vesa wrote:
> > > > > > > > On 22-06-27 18:23:33, Marek Vasut wrote:
> > > > > > > > > On 6/27/22 17:35, Abel Vesa wrote:
> > > > > > > > > > On 22-06-25 03:32:32, Marek Vasut wrote:
> > > > > > > > > > > Unlike the other block control IPs in i.MX8M, the audiomix is
> > > > > > > > > > > mostly a series of clock gates and muxes. Model it as a large
> > > > > > > > > > > static table of gates and muxes with one exception, which is the
> > > > > > > > > > > PLL14xx . The PLL14xx SAI PLL has to be registered separately.
> > > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > Again, there is a chance that the blk-ctrl driver might disable the
> > > > > > > > > > PD from under this.
> > > > > > > > > 
> > > > > > > > > Can you elaborate a bit more on this ? How/why do you think so ?
> > > > > > > > 
> > > > > > > > At some point, the PDs from the Audiomix IP block will be added to the
> > > > > > > > drivers/soc/imx/imx8mp-blk-ctrl.c. Then, you'll have 2 drivers with
> > > > > > > > the same address range and the imx8mp-blk-ctrl also has runtime PM
> > > > > > > enabled.
> > > > > > > 
> > > > > > > Why would the PDs be added into the block control driver?
> > > > > > > 
> > > > > > > The audiomix is purely a clock mux driver, not really a block control driver
> > > > > > > providing PDs of its own.
> > > > > > 
> > > > > > I recalled that with with blk-ctrl working as clock provider, there is dead lock
> > > > > > issue, if the blk-ctrl node has a power-domain entry. Not very sure.
> > > > > 
> > > > > How can I verify that ? Lockdep ?
> > > > > 
> > > > > I run this series for months and haven't seen a lock up or splat.
> > > > 
> > > > Audiomix (and every mix actually) has a PD. Once you add the PD, you'll
> > > > end up with an ABBA deadlock between genpd lock and clock prepare lock.
> > > 
> > > Unlike the other mix drivers, this is a pure clock driver, not a power
> > > domain driver. The PD is already available to this clock driver, see:
> > > [PATCH v3 5/6] arm64: dts: imx8mp: Add SAI, SDMA, AudioMIX
> > 
> > When you will enable the runtime PM for this driver, the deadlock is
> > going to happen and it will be in some scenario like:
> >      clk_disable_unused_subtree
> >        -> clk_prepare (takes prepare lock) (for a clock from your driver)
> > 	-> runtime pm (takes genpd lock)
> > 	  -> clk_prepare (tries to take prepare lock again) (for the clock of the PD)
> 
> Since you seem to have a test case, can you share the test case, verbatim,
> so I can reproduce it locally ?

I do not have a test case, but we do have a prior experience with this
happening.

> 
> I seem to be asking for that repeatedly and I am not getting any clear
> answer.

I do not have a board to test on anymore.

But anyway, lets apply it and if there is a problem, we can figure it
out later.

> 
> > > Can you please elaborate on the deadlock problem ?
> > > Because really, I just don't see it.
> > > 
> > > Were you able to reproduce the deadlock with this driver ?
> 
> [...]

Marek Vasut Oct. 19, 2022, 2:33 p.m. UTC | #15

On 10/14/22 03:53, Shengjiu Wang wrote:
> Hi Marek

Hi,

[...]

>> +static const struct clk_parent_data clk_imx8mp_audiomix_pll_parents[] = {
>> +       { .fw_name = "osc_24m", .name = "osc_24m" },
>> +       { .name = "dummy" },
>> +       { .name = "dummy" },
>> +       { .name = "dummy" },
>> +};
>> +
>> +static const struct clk_parent_data clk_imx8mp_audiomix_pll_bypass_sels[]
>> = {
>> +       { .fw_name = "sai_pll", .name = "sai_pll" },
>> +       { .fw_name = "sai_pll_ref_sel", .name = "sai_pll_ref_sel" },
>> +};
>> +
>> +#define CLK_GATE(gname, cname)                                         \
>> +       {                                                               \
>> +               gname"_cg",                                             \
>> +               IMX8MP_CLK_AUDIOMIX_##cname,                            \
>> +               { .fw_name = "ahb", .name = "ahb" }, NULL, 1,           \
>>
>> { .fw_name = "audio_root_clk", .name = "audio_root_clk" }, NULL, 1,
>>       \
>>
>> Should be the 'audio_root_clk' better?
>>
>> Then the 'clocks' and 'clock-names' can be removed in dts node?
>>
>> Will you continue to follow up this patch series?

Sure. Did anyone from NXP finally test this patch series, and can 
provide useful review ?

I am somewhat unhappy with the feedback I got thus far, which is based 
on downstream kernel fork, different power domain driver and different 
audiomix driver. I would really appreciate feedback for THIS driver instead.

Marek Vasut Oct. 25, 2022, 9:10 p.m. UTC | #16

On 10/20/22 05:06, Shengjiu Wang wrote:
> On Wed, Oct 19, 2022 at 10:33 PM Marek Vasut <[e-mail]> wrote:
> 
>> On 10/14/22 03:53, Shengjiu Wang wrote:
>>> Hi Marek
>>
>> Hi,
>>
>> [...]
>>
>>>> +static const struct clk_parent_data clk_imx8mp_audiomix_pll_parents[]
>> = {
>>>> +       { .fw_name = "osc_24m", .name = "osc_24m" },
>>>> +       { .name = "dummy" },
>>>> +       { .name = "dummy" },
>>>> +       { .name = "dummy" },
>>>> +};
>>>> +
>>>> +static const struct clk_parent_data
>> clk_imx8mp_audiomix_pll_bypass_sels[]
>>>> = {
>>>> +       { .fw_name = "sai_pll", .name = "sai_pll" },
>>>> +       { .fw_name = "sai_pll_ref_sel", .name = "sai_pll_ref_sel" },
>>>> +};
>>>> +
>>>> +#define CLK_GATE(gname, cname)
>>   \
>>>> +       {
>>   \
>>>> +               gname"_cg",
>>   \
>>>> +               IMX8MP_CLK_AUDIOMIX_##cname,
>> \
>>>> +               { .fw_name = "ahb", .name = "ahb" }, NULL, 1,
>>   \
>>>>
>>>> { .fw_name = "audio_root_clk", .name = "audio_root_clk" }, NULL, 1,
>>>>        \
>>>>
>>>> Should be the 'audio_root_clk' better?
>>>>
>>>> Then the 'clocks' and 'clock-names' can be removed in dts node?
>>>>
>>>> Will you continue to follow up this patch series?
>>
>> Sure. Did anyone from NXP finally test this patch series, and can
>> provide useful review ?
>>
> 
> I have tested it, and I think "ahb" should be "audio_root_clk". others LGTM.

It seems those clock are actually called IMX8MP_CLK_AUDIO_AHB_ROOT in 
the NXP downstream BSP, so those clock do drive AHB, correct ? If so, we 
should keep the "ahb" name here, to differentiate them from already 
existing IMX8MP_CLK_AUDIO_AXI , which seem to drive the AXI part.

Marek Vasut Oct. 26, 2022, 11:03 a.m. UTC | #17

On 10/26/22 12:36, Shengjiu Wang wrote:
> Hi Marek
> 
> On Wed, Oct 26, 2022 at 5:10 AM Marek Vasut <[e-mail]> wrote:
> 
>> On 10/20/22 05:06, Shengjiu Wang wrote:
>>> On Wed, Oct 19, 2022 at 10:33 PM Marek Vasut <[e-mail]> wrote:
>>>
>>>> On 10/14/22 03:53, Shengjiu Wang wrote:
>>>>> Hi Marek
>>>>
>>>> Hi,
>>>>
>>>> [...]
>>>>
>>>>>> +static const struct clk_parent_data clk_imx8mp_audiomix_pll_parents[]
>>>> = {
>>>>>> +       { .fw_name = "osc_24m", .name = "osc_24m" },
>>>>>> +       { .name = "dummy" },
>>>>>> +       { .name = "dummy" },
>>>>>> +       { .name = "dummy" },
>>>>>> +};
>>>>>> +
>>>>>> +static const struct clk_parent_data
>>>> clk_imx8mp_audiomix_pll_bypass_sels[]
>>>>>> = {
>>>>>> +       { .fw_name = "sai_pll", .name = "sai_pll" },
>>>>>> +       { .fw_name = "sai_pll_ref_sel", .name = "sai_pll_ref_sel" },
>>>>>> +};
>>>>>> +
>>>>>> +#define CLK_GATE(gname, cname)
>>>>    \
>>>>>> +       {
>>>>    \
>>>>>> +               gname"_cg",
>>>>    \
>>>>>> +               IMX8MP_CLK_AUDIOMIX_##cname,
>>>> \
>>>>>> +               { .fw_name = "ahb", .name = "ahb" }, NULL, 1,
>>>>    \
>>>>>>
>>>>>> { .fw_name = "audio_root_clk", .name = "audio_root_clk" }, NULL, 1,
>>>>>>         \
>>>>>>
>>>>>> Should be the 'audio_root_clk' better?
>>>>>>
>>>>>> Then the 'clocks' and 'clock-names' can be removed in dts node?
>>>>>>
>>>>>> Will you continue to follow up this patch series?
>>>>
>>>> Sure. Did anyone from NXP finally test this patch series, and can
>>>> provide useful review ?
>>>>
>>>
>>> I have tested it, and I think "ahb" should be "audio_root_clk". others
>> LGTM.
>>
>> It seems those clock are actually called IMX8MP_CLK_AUDIO_AHB_ROOT in
>> the NXP downstream BSP, so those clock do drive AHB, correct ? If so, we
>> should keep the "ahb" name here, to differentiate them from already
>> existing IMX8MP_CLK_AUDIO_AXI , which seem to drive the AXI part.
>>
> 
> Seems IMX8MP_CLK_AUDIO_ROOT needs to be changed to
> IMX8MP_CLK_AUDIO_AHB_ROOT in clk-imx8mp.c
> 
> and
> "ocrama"  parent clock is "audio_axi_root"
> "earc_phy"  parent clock is "sai_pll_out_div2"
> "dsp" parent clock is "audio_axi_root"
> "dspdbg" parent clock is "audio_axi_root"
> "audpll" parent clock is "osc_24m"
> 
> so I think it is better to include downstream change in this patch
> series:
> -       hws[IMX8MP_CLK_AUDIO_ROOT] = imx_clk_hw_gate4("audio_root_clk",
> "ipg_root", ccm_base + 0x4650, 0);
> +
> +       hws[IMX8MP_CLK_AUDIO_AHB_ROOT] =
> imx_clk_hw_gate2_shared2("audio_ahb_root", "audio_ahb", ccm_base + 0x4650,
> 0, &share_count_audio);
> +       hws[IMX8MP_CLK_AUDIO_AXI_ROOT] =
> imx_clk_hw_gate2_shared2("audio_axi_root", "audio_axi", ccm_base + 0x4650,
> 0, &share_count_audio);
> +       hws[IMX8MP_CLK_SAI1_ROOT] = imx_clk_hw_gate2_shared2("sai1_root",
> "sai1", ccm_base + 0x4650, 0, &share_count_audio);
> +       hws[IMX8MP_CLK_SAI2_ROOT] = imx_clk_hw_gate2_shared2("sai2_root",
> "sai2", ccm_base + 0x4650, 0, &share_count_audio);
> +       hws[IMX8MP_CLK_SAI3_ROOT] = imx_clk_hw_gate2_shared2("sai3_root",
> "sai3", ccm_base + 0x4650, 0, &share_count_audio);
> +       hws[IMX8MP_CLK_SAI5_ROOT] = imx_clk_hw_gate2_shared2("sai5_root",
> "sai5", ccm_base + 0x4650, 0, &share_count_audio);
> +       hws[IMX8MP_CLK_SAI6_ROOT] = imx_clk_hw_gate2_shared2("sai6_root",
> "sai6", ccm_base + 0x4650, 0, &share_count_audio);
> +       hws[IMX8MP_CLK_SAI7_ROOT] = imx_clk_hw_gate2_shared2("sai7_root",
> "sai7", ccm_base + 0x4650, 0, &share_count_audio);
> +       hws[IMX8MP_CLK_PDM_ROOT] = imx_clk_hw_gate2_shared2("pdm_root",
> "pdm", ccm_base + 0x4650, 0, &share_count_audio);
> 

Can you prepare such a clock rename patch and submit it ?

Luca Ceresoli Feb. 22, 2023, 4:58 p.m. UTC | #18

Hi Marek,

On Sat, 25 Jun 2022 03:32:32 +0200
Marek Vasut <[e-mail]> wrote:

> Unlike the other block control IPs in i.MX8M, the audiomix is mostly a
> series of clock gates and muxes. Model it as a large static table of
> gates and muxes with one exception, which is the PLL14xx . The PLL14xx
> SAI PLL has to be registered separately.
> 
> Signed-off-by: Marek Vasut <[e-mail]>
> Cc: Abel Vesa <[e-mail]>
> Cc: Fabio Estevam <[e-mail]>
> Cc: Jacky Bai <[e-mail]>
> Cc: Laurent Pinchart <[e-mail]>
> Cc: Lucas Stach <[e-mail]>
> Cc: Michael Turquette <[e-mail]>
> Cc: Shawn Guo <[e-mail]>
> Cc: Stephen Boyd <[e-mail]>
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]

[Tested on MSC SM2-MB-EP1 Carrier Board with SM2S-IMX8PLUS-QC6-14N0600E SoM]
Tested-by: Luca Ceresoli <[e-mail]>

Patch

diff --git a/drivers/clk/imx/Makefile b/drivers/clk/imx/Makefile
index 88b9b9285d22e..c4290937637eb 100644
--- a/drivers/clk/imx/Makefile
+++ b/drivers/clk/imx/Makefile
@@ -25,7 +25,7 @@  obj-$(CONFIG_MXC_CLK) += mxc-clk.o
 
 obj-$(CONFIG_CLK_IMX8MM) += clk-imx8mm.o
 obj-$(CONFIG_CLK_IMX8MN) += clk-imx8mn.o
-obj-$(CONFIG_CLK_IMX8MP) += clk-imx8mp.o
+obj-$(CONFIG_CLK_IMX8MP) += clk-imx8mp.o clk-imx8mp-audiomix.o
 obj-$(CONFIG_CLK_IMX8MQ) += clk-imx8mq.o
 
 obj-$(CONFIG_CLK_IMX93) += clk-imx93.o
diff --git a/drivers/clk/imx/clk-imx8mp-audiomix.c b/drivers/clk/imx/clk-imx8mp-audiomix.c
new file mode 100644
index 0000000000000..2d5d8255c7fa2
--- /dev/null
+++ b/drivers/clk/imx/clk-imx8mp-audiomix.c
@@ -0,0 +1,286 @@ 
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Driver for i.MX8M Plus Audio BLK_CTRL
+ *
+ * Copyright (C) 2022 Marek Vasut <[e-mail]>
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/device.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+
+#include <dt-bindings/clock/imx8mp-clock.h>
+
+#include "clk.h"
+
+#define CLKEN0			0x000
+#define CLKEN1			0x004
+#define SAI_MCLK_SEL(n)		(300 + 4 * (n))	/* n in 0..5 */
+#define PDM_SEL			0x318
+#define SAI_PLL_GNRL_CTL	0x400
+
+#define SAIn_MCLK1_PARENT(n)						\
+static const struct clk_parent_data					\
+clk_imx8mp_audiomix_sai##n##_mclk1_parents[] = {			\
+	{								\
+		.fw_name = "sai"__stringify(n),				\
+		.name = "sai"__stringify(n)				\
+	}, {								\
+		.fw_name = "sai"__stringify(n)"_mclk",			\
+		.name = "sai"__stringify(n)"_mclk"			\
+	},								\
+}
+
+SAIn_MCLK1_PARENT(1);
+SAIn_MCLK1_PARENT(2);
+SAIn_MCLK1_PARENT(3);
+SAIn_MCLK1_PARENT(5);
+SAIn_MCLK1_PARENT(6);
+SAIn_MCLK1_PARENT(7);
+
+static const struct clk_parent_data clk_imx8mp_audiomix_sai_mclk2_parents[] = {
+	{ .fw_name = "sai1", .name = "sai1" },
+	{ .fw_name = "sai2", .name = "sai2" },
+	{ .fw_name = "sai3", .name = "sai3" },
+	{ .name = "dummy" },
+	{ .fw_name = "sai5", .name = "sai5" },
+	{ .fw_name = "sai6", .name = "sai6" },
+	{ .fw_name = "sai7", .name = "sai7" },
+	{ .fw_name = "sai1_mclk", .name = "sai1_mclk" },
+	{ .fw_name = "sai2_mclk", .name = "sai2_mclk" },
+	{ .fw_name = "sai3_mclk", .name = "sai3_mclk" },
+	{ .name = "dummy" },
+	{ .fw_name = "sai5_mclk", .name = "sai5_mclk" },
+	{ .fw_name = "sai6_mclk", .name = "sai6_mclk" },
+	{ .fw_name = "sai7_mclk", .name = "sai7_mclk" },
+	{ .fw_name = "spdif_extclk", .name = "spdif_extclk" },
+	{ .name = "dummy" },
+};
+
+static const struct clk_parent_data clk_imx8mp_audiomix_pdm_parents[] = {
+	{ .fw_name = "pdm", .name = "pdm" },
+	{ .name = "sai_pll_out_div2" },
+	{ .fw_name = "sai1_mclk", .name = "sai1_mclk" },
+	{ .name = "dummy" },
+};
+
+
+static const struct clk_parent_data clk_imx8mp_audiomix_pll_parents[] = {
+	{ .fw_name = "osc_24m", .name = "osc_24m" },
+	{ .name = "dummy" },
+	{ .name = "dummy" },
+	{ .name = "dummy" },
+};
+
+static const struct clk_parent_data clk_imx8mp_audiomix_pll_bypass_sels[] = {
+	{ .fw_name = "sai_pll", .name = "sai_pll" },
+	{ .fw_name = "sai_pll_ref_sel", .name = "sai_pll_ref_sel" },
+};
+
+#define CLK_GATE(gname, cname)						\
+	{								\
+		gname"_cg",						\
+		IMX8MP_CLK_AUDIOMIX_##cname,				\
+		{ .fw_name = "ahb", .name = "ahb" }, NULL, 1,		\
+		CLKEN0 + 4 * !!(IMX8MP_CLK_AUDIOMIX_##cname / 32),	\
+		1, IMX8MP_CLK_AUDIOMIX_##cname % 32			\
+	}
+
+#define CLK_SAIn(n)							\
+	{								\
+		"sai"__stringify(n)"_mclk1_sel",			\
+		IMX8MP_CLK_AUDIOMIX_SAI##n##_MCLK1_SEL, {},		\
+		clk_imx8mp_audiomix_sai##n##_mclk1_parents,		\
+		ARRAY_SIZE(clk_imx8mp_audiomix_sai##n##_mclk1_parents), \
+		SAI_MCLK_SEL(n), 1, 0					\
+	}, {								\
+		"sai"__stringify(n)"_mclk2_sel",			\
+		IMX8MP_CLK_AUDIOMIX_SAI##n##_MCLK2_SEL, {},		\
+		clk_imx8mp_audiomix_sai_mclk2_parents,			\
+		ARRAY_SIZE(clk_imx8mp_audiomix_sai_mclk2_parents),	\
+		SAI_MCLK_SEL(n), 4, 1					\
+	}, {								\
+		"sai"__stringify(n)"_ipg_cg",				\
+		IMX8MP_CLK_AUDIOMIX_SAI##n##_IPG,			\
+		{ .fw_name = "ahb", .name = "ahb" }, NULL, 1,		\
+		CLKEN0, 1, IMX8MP_CLK_AUDIOMIX_SAI##n##_IPG		\
+	}, {								\
+		"sai"__stringify(n)"_mclk1_cg",				\
+		IMX8MP_CLK_AUDIOMIX_SAI##n##_MCLK1,			\
+		{							\
+			.fw_name = "sai"__stringify(n)"_mclk1_sel",	\
+			.name = "sai"__stringify(n)"_mclk1_sel"		\
+		}, NULL, 1,						\
+		CLKEN0, 1, IMX8MP_CLK_AUDIOMIX_SAI##n##_MCLK1		\
+	}, {								\
+		"sai"__stringify(n)"_mclk2_cg",				\
+		IMX8MP_CLK_AUDIOMIX_SAI##n##_MCLK2,			\
+		{							\
+			.fw_name = "sai"__stringify(n)"_mclk2_sel",	\
+			.name = "sai"__stringify(n)"_mclk2_sel"		\
+		}, NULL, 1,						\
+		CLKEN0, 1, IMX8MP_CLK_AUDIOMIX_SAI##n##_MCLK2		\
+	}, {								\
+		"sai"__stringify(n)"_mclk3_cg",				\
+		IMX8MP_CLK_AUDIOMIX_SAI##n##_MCLK3,			\
+		{							\
+			.fw_name = "sai_pll_out_div2",			\
+			.name = "sai_pll_out_div2"			\
+		}, NULL, 1,						\
+		CLKEN0, 1, IMX8MP_CLK_AUDIOMIX_SAI##n##_MCLK3		\
+	}
+
+#define CLK_PDM								\
+	{								\
+		"pdm_sel", IMX8MP_CLK_AUDIOMIX_PDM_SEL, {},		\
+		clk_imx8mp_audiomix_pdm_parents,			\
+		ARRAY_SIZE(clk_imx8mp_audiomix_pdm_parents),		\
+		PDM_SEL, 2, 0						\
+	}
+
+struct clk_imx8mp_audiomix_sel {
+	const char			*name;
+	int				clkid;
+	const struct clk_parent_data	parent;		/* For gate */
+	const struct clk_parent_data	*parents;	/* For mux */
+	int				num_parents;
+	u16				reg;
+	u8				width;
+	u8				shift;
+};
+
+static struct clk_imx8mp_audiomix_sel sels[] = {
+	CLK_GATE("asrc", ASRC_IPG),
+	CLK_GATE("pdm", PDM_IPG),
+	CLK_GATE("earc", EARC_IPG),
+	CLK_GATE("ocrama", OCRAMA_IPG),
+	CLK_GATE("aud2htx", AUD2HTX_IPG),
+	CLK_GATE("earc_phy", EARC_PHY),
+	CLK_GATE("sdma2", SDMA2_ROOT),
+	CLK_GATE("sdma3", SDMA3_ROOT),
+	CLK_GATE("spba2", SPBA2_ROOT),
+	CLK_GATE("dsp", DSP_ROOT),
+	CLK_GATE("dspdbg", DSPDBG_ROOT),
+	CLK_GATE("edma", EDMA_ROOT),
+	CLK_GATE("audpll", AUDPLL_ROOT),
+	CLK_GATE("mu2", MU2_ROOT),
+	CLK_GATE("mu3", MU3_ROOT),
+	CLK_PDM,
+	CLK_SAIn(1),
+	CLK_SAIn(2),
+	CLK_SAIn(3),
+	CLK_SAIn(5),
+	CLK_SAIn(6),
+	CLK_SAIn(7)
+};
+
+static int clk_imx8mp_audiomix_probe(struct platform_device *pdev)
+{
+	struct clk_hw_onecell_data *priv;
+	struct device *dev = &pdev->dev;
+	void __iomem *base;
+	struct clk_hw *hw;
+	int i;
+
+	priv = devm_kzalloc(dev,
+			    struct_size(priv, hws, IMX8MP_CLK_AUDIOMIX_END),
+			    GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->num = IMX8MP_CLK_AUDIOMIX_END;
+
+	base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	for (i = 0; i < ARRAY_SIZE(sels); i++) {
+		if (sels[i].num_parents == 1) {
+			hw = devm_clk_hw_register_gate_parent_data(dev,
+								   sels[i].name,
+								   &sels[i].parent,
+								   0,
+								   base + sels[i].reg,
+								   sels[i].shift,
+								   0, NULL);
+		} else {
+			hw = devm_clk_hw_register_mux_parent_data(dev,
+								  sels[i].name,
+								  sels[i].parents,
+								  sels[i].num_parents,
+								  0,
+								  base + sels[i].reg,
+								  sels[i].shift,
+								  sels[i].width,
+								  0, NULL);
+		}
+
+		if (IS_ERR(hw))
+			return PTR_ERR(hw);
+
+		priv->hws[sels[i].clkid] = hw;
+	}
+
+	/* SAI PLL */
+	hw = devm_clk_hw_register_mux_parent_data(dev, "sai_pll_ref_sel",
+						  clk_imx8mp_audiomix_pll_parents,
+						  ARRAY_SIZE(clk_imx8mp_audiomix_pll_parents),
+						  CLK_SET_RATE_NO_REPARENT,
+						  base + SAI_PLL_GNRL_CTL,
+						  0, 2, 0, NULL);
+	priv->hws[IMX8MP_CLK_AUDIOMIX_SAI_PLL_REF_SEL] = hw;
+
+	hw = imx_dev_clk_hw_pll14xx(dev, "sai_pll", "sai_pll_ref_sel",
+				    base + 0x400, &imx_1443x_pll);
+	if (IS_ERR(hw))
+		return PTR_ERR(hw);
+	priv->hws[IMX8MP_CLK_AUDIOMIX_SAI_PLL] = hw;
+
+	hw = devm_clk_hw_register_mux_parent_data(dev, "sai_pll_bypass",
+						  clk_imx8mp_audiomix_pll_bypass_sels,
+						  ARRAY_SIZE(clk_imx8mp_audiomix_pll_bypass_sels),
+						  CLK_SET_RATE_NO_REPARENT | CLK_SET_RATE_PARENT,
+						  base + SAI_PLL_GNRL_CTL,
+						  16, 1, 0, NULL);
+	if (IS_ERR(hw))
+		return PTR_ERR(hw);
+	priv->hws[IMX8MP_CLK_AUDIOMIX_SAI_PLL_BYPASS] = hw;
+
+	hw = devm_clk_hw_register_gate(dev, "sai_pll_out", "sai_pll_bypass",
+				       0, base + SAI_PLL_GNRL_CTL, 13,
+				       0, NULL);
+	if (IS_ERR(hw))
+		return PTR_ERR(hw);
+	priv->hws[IMX8MP_CLK_AUDIOMIX_SAI_PLL_OUT] = hw;
+
+	hw = devm_clk_hw_register_fixed_factor(dev, "sai_pll_out_div2",
+					       "sai_pll_out", 0, 1, 2);
+	if (IS_ERR(hw))
+		return PTR_ERR(hw);
+
+	return devm_of_clk_add_hw_provider(&pdev->dev, of_clk_hw_onecell_get,
+					   priv);
+}
+
+static const struct of_device_id clk_imx8mp_audiomix_of_match[] = {
+	{ .compatible = "fsl,imx8mp-audio-blk-ctrl" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, clk_imx8mp_audiomix_of_match);
+
+static struct platform_driver clk_imx8mp_audiomix_driver = {
+	.probe	= clk_imx8mp_audiomix_probe,
+	.driver = {
+		.name = "imx8mp-audio-blk-ctrl",
+		.of_match_table = clk_imx8mp_audiomix_of_match,
+	},
+};
+
+module_platform_driver(clk_imx8mp_audiomix_driver);
+
+MODULE_AUTHOR("Marek Vasut <[e-mail]>");
+MODULE_DESCRIPTION("Freescale i.MX8MP Audio Block Controller driver");
+MODULE_LICENSE("GPL");