All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alexandre Belloni <alexandre.belloni@bootlin.com>
To: Detlev Casanova <detlev.casanova@collabora.com>
Cc: linux-kernel@vger.kernel.org,
	Nicolas Frattaroli <frattaroli.nicolas@gmail.com>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Mark Brown <broonie@kernel.org>, Jaroslav Kysela <perex@perex.cz>,
	Takashi Iwai <tiwai@suse.com>, Heiko Stuebner <heiko@sntech.de>,
	linux-rockchip@lists.infradead.org, linux-sound@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, kernel@collabora.com,
	Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
	Luca Ceresoli <luca.ceresoli@bootlin.com>
Subject: Re: [PATCH] ASoC: rockchip: i2s-tdm: Use param rate if not provided by set_sysclk
Date: Thu, 19 Feb 2026 10:37:18 +0100	[thread overview]
Message-ID: <20260219093718b7d29076@mail.local> (raw)
In-Reply-To: <20260218201834.924358-1-detlev.casanova@collabora.com>

Hello,

On 18/02/2026 15:18:34-0500, Detlev Casanova wrote:
> Drivers will not always call set_sysclk() for all clocks, especially when
> default mclk-fs can be used.
> When that is the case, use the clock rate set in the params multiplied by the
> default mclk-fs.
> 
> Fixes: 5323186e2e8d ("ASoC: rockchip: i2s_tdm: Re-add the set_sysclk callback")
> Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
> ---
>  sound/soc/rockchip/rockchip_i2s_tdm.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/sound/soc/rockchip/rockchip_i2s_tdm.c b/sound/soc/rockchip/rockchip_i2s_tdm.c
> index 770b9bfbb384..fc52149ed6ae 100644
> --- a/sound/soc/rockchip/rockchip_i2s_tdm.c
> +++ b/sound/soc/rockchip/rockchip_i2s_tdm.c
> @@ -22,6 +22,7 @@
>  
>  #define DRV_NAME "rockchip-i2s-tdm"
>  
> +#define DEFAULT_MCLK_FS				256
>  #define CH_GRP_MAX				4  /* The max channel 8 / 2 */
>  #define MULTIPLEX_CH_MAX			10
>  
> @@ -665,6 +666,15 @@ static int rockchip_i2s_tdm_hw_params(struct snd_pcm_substream *substream,
>  			mclk_rate = i2s_tdm->mclk_rx_freq;
>  		}
>  
> +		/*
> +		 * When the dai/component driver doesn't need to set mclk-fs for a specific
> +		 * clock, it can skip the call to set_sysclk() for that clock.
> +		 * In that case, simply use the clock rate from the params and multiply it by
> +		 * the default mclk-fs value.
> +		 */
> +		if (!mclk_rate)
> +			mclk_rate = DEFAULT_MCLK_FS * params_rate(params);
> +

Maybe this could be set as the default value of i2s_tdm->mclk_rx_freq
and i2s_tdm->mclk_tx_freq in rockchip_i2s_tdm_probe instead of relying
on it being 0 and testing later on?

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


WARNING: multiple messages have this Message-ID (diff)
From: Alexandre Belloni <alexandre.belloni@bootlin.com>
To: Detlev Casanova <detlev.casanova@collabora.com>
Cc: linux-kernel@vger.kernel.org,
	Nicolas Frattaroli <frattaroli.nicolas@gmail.com>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Mark Brown <broonie@kernel.org>, Jaroslav Kysela <perex@perex.cz>,
	Takashi Iwai <tiwai@suse.com>, Heiko Stuebner <heiko@sntech.de>,
	linux-rockchip@lists.infradead.org, linux-sound@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, kernel@collabora.com,
	Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
	Luca Ceresoli <luca.ceresoli@bootlin.com>
Subject: Re: [PATCH] ASoC: rockchip: i2s-tdm: Use param rate if not provided by set_sysclk
Date: Thu, 19 Feb 2026 10:37:18 +0100	[thread overview]
Message-ID: <20260219093718b7d29076@mail.local> (raw)
In-Reply-To: <20260218201834.924358-1-detlev.casanova@collabora.com>

Hello,

On 18/02/2026 15:18:34-0500, Detlev Casanova wrote:
> Drivers will not always call set_sysclk() for all clocks, especially when
> default mclk-fs can be used.
> When that is the case, use the clock rate set in the params multiplied by the
> default mclk-fs.
> 
> Fixes: 5323186e2e8d ("ASoC: rockchip: i2s_tdm: Re-add the set_sysclk callback")
> Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
> ---
>  sound/soc/rockchip/rockchip_i2s_tdm.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/sound/soc/rockchip/rockchip_i2s_tdm.c b/sound/soc/rockchip/rockchip_i2s_tdm.c
> index 770b9bfbb384..fc52149ed6ae 100644
> --- a/sound/soc/rockchip/rockchip_i2s_tdm.c
> +++ b/sound/soc/rockchip/rockchip_i2s_tdm.c
> @@ -22,6 +22,7 @@
>  
>  #define DRV_NAME "rockchip-i2s-tdm"
>  
> +#define DEFAULT_MCLK_FS				256
>  #define CH_GRP_MAX				4  /* The max channel 8 / 2 */
>  #define MULTIPLEX_CH_MAX			10
>  
> @@ -665,6 +666,15 @@ static int rockchip_i2s_tdm_hw_params(struct snd_pcm_substream *substream,
>  			mclk_rate = i2s_tdm->mclk_rx_freq;
>  		}
>  
> +		/*
> +		 * When the dai/component driver doesn't need to set mclk-fs for a specific
> +		 * clock, it can skip the call to set_sysclk() for that clock.
> +		 * In that case, simply use the clock rate from the params and multiply it by
> +		 * the default mclk-fs value.
> +		 */
> +		if (!mclk_rate)
> +			mclk_rate = DEFAULT_MCLK_FS * params_rate(params);
> +

Maybe this could be set as the default value of i2s_tdm->mclk_rx_freq
and i2s_tdm->mclk_tx_freq in rockchip_i2s_tdm_probe instead of relying
on it being 0 and testing later on?

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

  parent reply	other threads:[~2026-02-19  9:37 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-18 20:18 [PATCH] ASoC: rockchip: i2s-tdm: Use param rate if not provided by set_sysclk Detlev Casanova
2026-02-18 20:18 ` Detlev Casanova
2026-02-18 21:02 ` Detlev Casanova
2026-02-18 21:02   ` Detlev Casanova
2026-02-19 16:51   ` Luca Ceresoli
2026-02-19 16:51     ` Luca Ceresoli
2026-02-19  9:37 ` Alexandre Belloni [this message]
2026-02-19  9:37   ` Alexandre Belloni
2026-02-19 12:34   ` Mark Brown
2026-02-19 12:34     ` Mark Brown
2026-02-19 13:30     ` Alexandre Belloni
2026-02-19 13:30       ` Alexandre Belloni
2026-02-19 15:36 ` Mark Brown
2026-02-19 15:36   ` Mark Brown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260219093718b7d29076@mail.local \
    --to=alexandre.belloni@bootlin.com \
    --cc=broonie@kernel.org \
    --cc=detlev.casanova@collabora.com \
    --cc=frattaroli.nicolas@gmail.com \
    --cc=heiko@sntech.de \
    --cc=kernel@collabora.com \
    --cc=lgirdwood@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=linux-sound@vger.kernel.org \
    --cc=luca.ceresoli@bootlin.com \
    --cc=perex@perex.cz \
    --cc=thomas.petazzoni@bootlin.com \
    --cc=tiwai@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.