Re: [PATCH v4 6/6] clk: fsl-sai: Add MCLK generation support

Brian Masney
2026-04-07T19:57:36Z

Hi Marek,

On Mon, Apr 06, 2026 at 11:49:46PM +0200, Marek Vasut wrote:
> The driver currently supports generating BCLK. There are systems which
> require generation of MCLK instead. Register new MCLK clock and handle
> clock-cells = <1> to differentiate between BCLK and MCLK. In case of a
> legacy system with clock-cells = <0>, the driver behaves as before, i.e.
> always returns BCLK.
>
> Note that it is not possible re-use the current SAI audio driver to
> generate MCLK and correctly enable and disable the MCLK.
>
> If SAI (audio driver) is used to control the MCLK enablement, then MCLK
> clock is not always enabled, and it is not necessarily enabled when the
> codec may need the clock to be enabled. There is also no way for the
> codec node to specify phandle to clock provider in DT, because the SAI
> (audio driver) is not clock provider.
>
> If SAI (clock driver) is used to control the MCLK enablement, then MCLK
> clock is enabled when the codec needs the clock enabled, because the
> codec is the clock consumer and the SAI (clock driver) is the clock
> provider, and the codec driver can request the clock to be enabled when
> needed. There is also the usual phandle to clock provider in DT, because
> the SAI (clock driver) is clock provider.
>
> Acked-by: Michael Walle <mwalle@kernel.org>
> Signed-off-by: Marek Vasut <marex@nabladev.com>
> ---
> Cc: Brian Masney <bmasney@redhat.com>
> Cc: Conor Dooley <conor+dt@kernel.org>
> Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
> Cc: Michael Turquette <mturquette@baylibre.com>
> Cc: Michael Walle <michael@walle.cc>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Stephen Boyd <sboyd@kernel.org>
> Cc: devicetree@vger.kernel.org
> Cc: linux-clk@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> ---
> V2: No change
> V3: - Rebase on current next, update mail address
> - Update commit message according to clarify the difference between
> SAI audio and SAI clock driver
> - Pick ancient AB from Michael, although this may be outdated
> https://patchwork.kernel.org/project/alsa-devel/patch/20241226162234.40141-4-marex@denx.de/
> V4: Use the fsl_sai_clk_register() helper.
> ---
> drivers/clk/clk-fsl-sai.c | 31 ++++++++++++++++++++++++++++++-
> 1 file changed, 30 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/clk/clk-fsl-sai.c b/drivers/clk/clk-fsl-sai.c
> index 7ec9a4f22735c..386ee5f77a986 100644
> --- a/drivers/clk/clk-fsl-sai.c
> +++ b/drivers/clk/clk-fsl-sai.c
> @@ -6,6 +6,7 @@
> */
>
> #include <linux/clk-provider.h>
> +#include <linux/clk.h>

This include needs to be moved earlier into the series to keep
bisectability. It looks like on patch 2. I'll also comment there.

> #include <linux/err.h>
> #include <linux/module.h>
> #include <linux/of.h>
> @@ -15,22 +16,37 @@
>
> #define I2S_CSR 0x00
> #define I2S_CR2 0x08
> +#define I2S_MCR 0x100
> #define CSR_BCE_BIT 28
> +#define CSR_TE_BIT 31
> #define CR2_BCD BIT(24)
> #define CR2_DIV_SHIFT 0
> #define CR2_DIV_WIDTH 8
> +#define MCR_MOE BIT(30)
>
> struct fsl_sai_clk {
> struct clk_divider bclk_div;
> + struct clk_divider mclk_div;
> struct clk_gate bclk_gate;
> + struct clk_gate mclk_gate;
> struct clk_hw *bclk_hw;
> + struct clk_hw *mclk_hw;
> spinlock_t lock;
> };
>
> struct fsl_sai_data {
> unsigned int offset; /* Register offset */
> + bool have_mclk; /* Have MCLK control */
> };
>
> +static struct clk_hw *
> +fsl_sai_of_clk_get(struct of_phandle_args *clkspec, void *data)
> +{
> + struct fsl_sai_clk *sai_clk = data;
> +
> + return clkspec->args[0] ? sai_clk->mclk_hw : sai_clk->bclk_hw;

For the case when clock-cells is 0, should this check:

clkspec->args_count == 0 || return clkspec->args[0]

Brian