* [PATCH v2 0/3] ASoC: rockchip: Use guard() for spin locks @ 2026-06-04 3:35 ` phucduc.bui 0 siblings, 0 replies; 16+ messages in thread From: phucduc.bui @ 2026-06-04 3:35 UTC (permalink / raw) To: Heiko Stuebner, Mark Brown, Liam Girdwood Cc: Nicolas Frattaroli, Jaroslav Kysela, Takashi Iwai, linux-sound, linux-rockchip, linux-arm-kernel, linux-kernel, bui duc phuc From: bui duc phuc <phucduc.bui@gmail.com> Hi all, This series converts spinlock handling in the Rockchip sound drivers to use guard() helpers. The changes are code cleanup only and should have no functional impact. Changes in v2: - Remove the unnecessary err_pm_put label in rockchip_sai_hw_params(). Compile tested only. Best regards, Phuc bui duc phuc (3): ASoC: rockchip: rockchip_i2s: Use guard() for spin locks ASoC: rockchip: i2s-tdm: Use guard() for spin locks ASoC: rockchip: rockchip_sai: Use guard() for spin locks sound/soc/rockchip/rockchip_i2s.c | 160 ++++++++-------- sound/soc/rockchip/rockchip_i2s_tdm.c | 8 +- sound/soc/rockchip/rockchip_sai.c | 262 +++++++++++++------------- 3 files changed, 211 insertions(+), 219 deletions(-) -- 2.43.0 _______________________________________________ Linux-rockchip mailing list Linux-rockchip@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-rockchip ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 0/3] ASoC: rockchip: Use guard() for spin locks @ 2026-06-04 3:35 ` phucduc.bui 0 siblings, 0 replies; 16+ messages in thread From: phucduc.bui @ 2026-06-04 3:35 UTC (permalink / raw) To: Heiko Stuebner, Mark Brown, Liam Girdwood Cc: Nicolas Frattaroli, Jaroslav Kysela, Takashi Iwai, linux-sound, linux-rockchip, linux-arm-kernel, linux-kernel, bui duc phuc From: bui duc phuc <phucduc.bui@gmail.com> Hi all, This series converts spinlock handling in the Rockchip sound drivers to use guard() helpers. The changes are code cleanup only and should have no functional impact. Changes in v2: - Remove the unnecessary err_pm_put label in rockchip_sai_hw_params(). Compile tested only. Best regards, Phuc bui duc phuc (3): ASoC: rockchip: rockchip_i2s: Use guard() for spin locks ASoC: rockchip: i2s-tdm: Use guard() for spin locks ASoC: rockchip: rockchip_sai: Use guard() for spin locks sound/soc/rockchip/rockchip_i2s.c | 160 ++++++++-------- sound/soc/rockchip/rockchip_i2s_tdm.c | 8 +- sound/soc/rockchip/rockchip_sai.c | 262 +++++++++++++------------- 3 files changed, 211 insertions(+), 219 deletions(-) -- 2.43.0 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 1/3] ASoC: rockchip: rockchip_i2s: Use guard() for spin locks 2026-06-04 3:35 ` phucduc.bui @ 2026-06-04 3:35 ` phucduc.bui -1 siblings, 0 replies; 16+ messages in thread From: phucduc.bui @ 2026-06-04 3:35 UTC (permalink / raw) To: Heiko Stuebner, Mark Brown, Liam Girdwood Cc: Nicolas Frattaroli, Jaroslav Kysela, Takashi Iwai, linux-sound, linux-rockchip, linux-arm-kernel, linux-kernel, bui duc phuc From: bui duc phuc <phucduc.bui@gmail.com> Clean up the code using guard() for spin locks. Merely code refactoring, and no behavior change. Signed-off-by: bui duc phuc <phucduc.bui@gmail.com> --- NOTE: This patch is compile-tested only. sound/soc/rockchip/rockchip_i2s.c | 160 +++++++++++++++--------------- 1 file changed, 80 insertions(+), 80 deletions(-) diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c index 0a0a95b4f520..5a5087f4ae03 100644 --- a/sound/soc/rockchip/rockchip_i2s.c +++ b/sound/soc/rockchip/rockchip_i2s.c @@ -127,52 +127,52 @@ static int rockchip_snd_txctrl(struct rk_i2s_dev *i2s, int on) unsigned int val = 0; int ret = 0; - spin_lock(&i2s->lock); - if (on) { - ret = regmap_update_bits(i2s->regmap, I2S_DMACR, - I2S_DMACR_TDE_ENABLE, - I2S_DMACR_TDE_ENABLE); - if (ret < 0) - goto end; - ret = regmap_update_bits(i2s->regmap, I2S_XFER, - I2S_XFER_TXS_START | I2S_XFER_RXS_START, - I2S_XFER_TXS_START | I2S_XFER_RXS_START); - if (ret < 0) - goto end; - i2s->tx_start = true; - } else { - i2s->tx_start = false; - - ret = regmap_update_bits(i2s->regmap, I2S_DMACR, - I2S_DMACR_TDE_ENABLE, - I2S_DMACR_TDE_DISABLE); - if (ret < 0) - goto end; - - if (!i2s->rx_start) { + scoped_guard(spinlock, &i2s->lock) { + if (on) { + ret = regmap_update_bits(i2s->regmap, I2S_DMACR, + I2S_DMACR_TDE_ENABLE, + I2S_DMACR_TDE_ENABLE); + if (ret < 0) + break; ret = regmap_update_bits(i2s->regmap, I2S_XFER, I2S_XFER_TXS_START | I2S_XFER_RXS_START, - I2S_XFER_TXS_STOP | I2S_XFER_RXS_STOP); + I2S_XFER_TXS_START | I2S_XFER_RXS_START); if (ret < 0) - goto end; - udelay(150); - ret = regmap_update_bits(i2s->regmap, I2S_CLR, - I2S_CLR_TXC | I2S_CLR_RXC, - I2S_CLR_TXC | I2S_CLR_RXC); - if (ret < 0) - goto end; - ret = regmap_read_poll_timeout_atomic(i2s->regmap, - I2S_CLR, - val, - val == 0, - 20, - 200); + break; + i2s->tx_start = true; + } else { + i2s->tx_start = false; + + ret = regmap_update_bits(i2s->regmap, I2S_DMACR, + I2S_DMACR_TDE_ENABLE, + I2S_DMACR_TDE_DISABLE); if (ret < 0) - dev_warn(i2s->dev, "fail to clear: %d\n", ret); + break; + + if (!i2s->rx_start) { + ret = regmap_update_bits(i2s->regmap, I2S_XFER, + I2S_XFER_TXS_START | I2S_XFER_RXS_START, + I2S_XFER_TXS_STOP | I2S_XFER_RXS_STOP); + if (ret < 0) + break; + udelay(150); + ret = regmap_update_bits(i2s->regmap, I2S_CLR, + I2S_CLR_TXC | I2S_CLR_RXC, + I2S_CLR_TXC | I2S_CLR_RXC); + if (ret < 0) + break; + ret = regmap_read_poll_timeout_atomic(i2s->regmap, + I2S_CLR, + val, + val == 0, + 20, + 200); + if (ret < 0) + dev_warn(i2s->dev, "fail to clear: %d\n", ret); + } } } -end: - spin_unlock(&i2s->lock); + if (ret < 0) dev_err(i2s->dev, "lrclk update failed\n"); @@ -184,53 +184,53 @@ static int rockchip_snd_rxctrl(struct rk_i2s_dev *i2s, int on) unsigned int val = 0; int ret = 0; - spin_lock(&i2s->lock); - if (on) { - ret = regmap_update_bits(i2s->regmap, I2S_DMACR, - I2S_DMACR_RDE_ENABLE, - I2S_DMACR_RDE_ENABLE); - if (ret < 0) - goto end; - - ret = regmap_update_bits(i2s->regmap, I2S_XFER, - I2S_XFER_TXS_START | I2S_XFER_RXS_START, - I2S_XFER_TXS_START | I2S_XFER_RXS_START); - if (ret < 0) - goto end; - i2s->rx_start = true; - } else { - i2s->rx_start = false; - - ret = regmap_update_bits(i2s->regmap, I2S_DMACR, - I2S_DMACR_RDE_ENABLE, - I2S_DMACR_RDE_DISABLE); - if (ret < 0) - goto end; + scoped_guard(spinlock, &i2s->lock) { + if (on) { + ret = regmap_update_bits(i2s->regmap, I2S_DMACR, + I2S_DMACR_RDE_ENABLE, + I2S_DMACR_RDE_ENABLE); + if (ret < 0) + break; - if (!i2s->tx_start) { ret = regmap_update_bits(i2s->regmap, I2S_XFER, I2S_XFER_TXS_START | I2S_XFER_RXS_START, - I2S_XFER_TXS_STOP | I2S_XFER_RXS_STOP); + I2S_XFER_TXS_START | I2S_XFER_RXS_START); if (ret < 0) - goto end; - udelay(150); - ret = regmap_update_bits(i2s->regmap, I2S_CLR, - I2S_CLR_TXC | I2S_CLR_RXC, - I2S_CLR_TXC | I2S_CLR_RXC); - if (ret < 0) - goto end; - ret = regmap_read_poll_timeout_atomic(i2s->regmap, - I2S_CLR, - val, - val == 0, - 20, - 200); + break; + i2s->rx_start = true; + } else { + i2s->rx_start = false; + + ret = regmap_update_bits(i2s->regmap, I2S_DMACR, + I2S_DMACR_RDE_ENABLE, + I2S_DMACR_RDE_DISABLE); if (ret < 0) - dev_warn(i2s->dev, "fail to clear: %d\n", ret); + break; + + if (!i2s->tx_start) { + ret = regmap_update_bits(i2s->regmap, I2S_XFER, + I2S_XFER_TXS_START | I2S_XFER_RXS_START, + I2S_XFER_TXS_STOP | I2S_XFER_RXS_STOP); + if (ret < 0) + break; + udelay(150); + ret = regmap_update_bits(i2s->regmap, I2S_CLR, + I2S_CLR_TXC | I2S_CLR_RXC, + I2S_CLR_TXC | I2S_CLR_RXC); + if (ret < 0) + break; + ret = regmap_read_poll_timeout_atomic(i2s->regmap, + I2S_CLR, + val, + val == 0, + 20, + 200); + if (ret < 0) + dev_warn(i2s->dev, "fail to clear: %d\n", ret); + } } } -end: - spin_unlock(&i2s->lock); + if (ret < 0) dev_err(i2s->dev, "lrclk update failed\n"); -- 2.43.0 _______________________________________________ Linux-rockchip mailing list Linux-rockchip@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-rockchip ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 1/3] ASoC: rockchip: rockchip_i2s: Use guard() for spin locks @ 2026-06-04 3:35 ` phucduc.bui 0 siblings, 0 replies; 16+ messages in thread From: phucduc.bui @ 2026-06-04 3:35 UTC (permalink / raw) To: Heiko Stuebner, Mark Brown, Liam Girdwood Cc: Nicolas Frattaroli, Jaroslav Kysela, Takashi Iwai, linux-sound, linux-rockchip, linux-arm-kernel, linux-kernel, bui duc phuc From: bui duc phuc <phucduc.bui@gmail.com> Clean up the code using guard() for spin locks. Merely code refactoring, and no behavior change. Signed-off-by: bui duc phuc <phucduc.bui@gmail.com> --- NOTE: This patch is compile-tested only. sound/soc/rockchip/rockchip_i2s.c | 160 +++++++++++++++--------------- 1 file changed, 80 insertions(+), 80 deletions(-) diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c index 0a0a95b4f520..5a5087f4ae03 100644 --- a/sound/soc/rockchip/rockchip_i2s.c +++ b/sound/soc/rockchip/rockchip_i2s.c @@ -127,52 +127,52 @@ static int rockchip_snd_txctrl(struct rk_i2s_dev *i2s, int on) unsigned int val = 0; int ret = 0; - spin_lock(&i2s->lock); - if (on) { - ret = regmap_update_bits(i2s->regmap, I2S_DMACR, - I2S_DMACR_TDE_ENABLE, - I2S_DMACR_TDE_ENABLE); - if (ret < 0) - goto end; - ret = regmap_update_bits(i2s->regmap, I2S_XFER, - I2S_XFER_TXS_START | I2S_XFER_RXS_START, - I2S_XFER_TXS_START | I2S_XFER_RXS_START); - if (ret < 0) - goto end; - i2s->tx_start = true; - } else { - i2s->tx_start = false; - - ret = regmap_update_bits(i2s->regmap, I2S_DMACR, - I2S_DMACR_TDE_ENABLE, - I2S_DMACR_TDE_DISABLE); - if (ret < 0) - goto end; - - if (!i2s->rx_start) { + scoped_guard(spinlock, &i2s->lock) { + if (on) { + ret = regmap_update_bits(i2s->regmap, I2S_DMACR, + I2S_DMACR_TDE_ENABLE, + I2S_DMACR_TDE_ENABLE); + if (ret < 0) + break; ret = regmap_update_bits(i2s->regmap, I2S_XFER, I2S_XFER_TXS_START | I2S_XFER_RXS_START, - I2S_XFER_TXS_STOP | I2S_XFER_RXS_STOP); + I2S_XFER_TXS_START | I2S_XFER_RXS_START); if (ret < 0) - goto end; - udelay(150); - ret = regmap_update_bits(i2s->regmap, I2S_CLR, - I2S_CLR_TXC | I2S_CLR_RXC, - I2S_CLR_TXC | I2S_CLR_RXC); - if (ret < 0) - goto end; - ret = regmap_read_poll_timeout_atomic(i2s->regmap, - I2S_CLR, - val, - val == 0, - 20, - 200); + break; + i2s->tx_start = true; + } else { + i2s->tx_start = false; + + ret = regmap_update_bits(i2s->regmap, I2S_DMACR, + I2S_DMACR_TDE_ENABLE, + I2S_DMACR_TDE_DISABLE); if (ret < 0) - dev_warn(i2s->dev, "fail to clear: %d\n", ret); + break; + + if (!i2s->rx_start) { + ret = regmap_update_bits(i2s->regmap, I2S_XFER, + I2S_XFER_TXS_START | I2S_XFER_RXS_START, + I2S_XFER_TXS_STOP | I2S_XFER_RXS_STOP); + if (ret < 0) + break; + udelay(150); + ret = regmap_update_bits(i2s->regmap, I2S_CLR, + I2S_CLR_TXC | I2S_CLR_RXC, + I2S_CLR_TXC | I2S_CLR_RXC); + if (ret < 0) + break; + ret = regmap_read_poll_timeout_atomic(i2s->regmap, + I2S_CLR, + val, + val == 0, + 20, + 200); + if (ret < 0) + dev_warn(i2s->dev, "fail to clear: %d\n", ret); + } } } -end: - spin_unlock(&i2s->lock); + if (ret < 0) dev_err(i2s->dev, "lrclk update failed\n"); @@ -184,53 +184,53 @@ static int rockchip_snd_rxctrl(struct rk_i2s_dev *i2s, int on) unsigned int val = 0; int ret = 0; - spin_lock(&i2s->lock); - if (on) { - ret = regmap_update_bits(i2s->regmap, I2S_DMACR, - I2S_DMACR_RDE_ENABLE, - I2S_DMACR_RDE_ENABLE); - if (ret < 0) - goto end; - - ret = regmap_update_bits(i2s->regmap, I2S_XFER, - I2S_XFER_TXS_START | I2S_XFER_RXS_START, - I2S_XFER_TXS_START | I2S_XFER_RXS_START); - if (ret < 0) - goto end; - i2s->rx_start = true; - } else { - i2s->rx_start = false; - - ret = regmap_update_bits(i2s->regmap, I2S_DMACR, - I2S_DMACR_RDE_ENABLE, - I2S_DMACR_RDE_DISABLE); - if (ret < 0) - goto end; + scoped_guard(spinlock, &i2s->lock) { + if (on) { + ret = regmap_update_bits(i2s->regmap, I2S_DMACR, + I2S_DMACR_RDE_ENABLE, + I2S_DMACR_RDE_ENABLE); + if (ret < 0) + break; - if (!i2s->tx_start) { ret = regmap_update_bits(i2s->regmap, I2S_XFER, I2S_XFER_TXS_START | I2S_XFER_RXS_START, - I2S_XFER_TXS_STOP | I2S_XFER_RXS_STOP); + I2S_XFER_TXS_START | I2S_XFER_RXS_START); if (ret < 0) - goto end; - udelay(150); - ret = regmap_update_bits(i2s->regmap, I2S_CLR, - I2S_CLR_TXC | I2S_CLR_RXC, - I2S_CLR_TXC | I2S_CLR_RXC); - if (ret < 0) - goto end; - ret = regmap_read_poll_timeout_atomic(i2s->regmap, - I2S_CLR, - val, - val == 0, - 20, - 200); + break; + i2s->rx_start = true; + } else { + i2s->rx_start = false; + + ret = regmap_update_bits(i2s->regmap, I2S_DMACR, + I2S_DMACR_RDE_ENABLE, + I2S_DMACR_RDE_DISABLE); if (ret < 0) - dev_warn(i2s->dev, "fail to clear: %d\n", ret); + break; + + if (!i2s->tx_start) { + ret = regmap_update_bits(i2s->regmap, I2S_XFER, + I2S_XFER_TXS_START | I2S_XFER_RXS_START, + I2S_XFER_TXS_STOP | I2S_XFER_RXS_STOP); + if (ret < 0) + break; + udelay(150); + ret = regmap_update_bits(i2s->regmap, I2S_CLR, + I2S_CLR_TXC | I2S_CLR_RXC, + I2S_CLR_TXC | I2S_CLR_RXC); + if (ret < 0) + break; + ret = regmap_read_poll_timeout_atomic(i2s->regmap, + I2S_CLR, + val, + val == 0, + 20, + 200); + if (ret < 0) + dev_warn(i2s->dev, "fail to clear: %d\n", ret); + } } } -end: - spin_unlock(&i2s->lock); + if (ret < 0) dev_err(i2s->dev, "lrclk update failed\n"); -- 2.43.0 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 2/3] ASoC: rockchip: i2s-tdm: Use guard() for spin locks 2026-06-04 3:35 ` phucduc.bui @ 2026-06-04 3:35 ` phucduc.bui -1 siblings, 0 replies; 16+ messages in thread From: phucduc.bui @ 2026-06-04 3:35 UTC (permalink / raw) To: Heiko Stuebner, Mark Brown, Liam Girdwood Cc: Nicolas Frattaroli, Jaroslav Kysela, Takashi Iwai, linux-sound, linux-rockchip, linux-arm-kernel, linux-kernel, bui duc phuc From: bui duc phuc <phucduc.bui@gmail.com> Clean up the code using guard() for spin locks. Merely code refactoring, and no behavior change. Signed-off-by: bui duc phuc <phucduc.bui@gmail.com> --- NOTE: This patch is compile-tested only. sound/soc/rockchip/rockchip_i2s_tdm.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/sound/soc/rockchip/rockchip_i2s_tdm.c b/sound/soc/rockchip/rockchip_i2s_tdm.c index fc52149ed6ae..3f3db28b8940 100644 --- a/sound/soc/rockchip/rockchip_i2s_tdm.c +++ b/sound/soc/rockchip/rockchip_i2s_tdm.c @@ -285,9 +285,8 @@ static void rockchip_snd_txrxctrl(struct snd_pcm_substream *substream, struct snd_soc_dai *dai, int on) { struct rk_i2s_tdm_dev *i2s_tdm = to_info(dai); - unsigned long flags; - spin_lock_irqsave(&i2s_tdm->lock, flags); + guard(spinlock_irqsave)(&i2s_tdm->lock); if (on) { if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) rockchip_enable_tde(i2s_tdm->regmap); @@ -313,7 +312,6 @@ static void rockchip_snd_txrxctrl(struct snd_pcm_substream *substream, I2S_CLR_TXC | I2S_CLR_RXC); } } - spin_unlock_irqrestore(&i2s_tdm->lock, flags); } static void rockchip_snd_txctrl(struct rk_i2s_tdm_dev *i2s_tdm, int on) @@ -587,12 +585,11 @@ static int rockchip_i2s_trcm_mode(struct snd_pcm_substream *substream, unsigned int fmt) { struct rk_i2s_tdm_dev *i2s_tdm = to_info(dai); - unsigned long flags; if (!i2s_tdm->clk_trcm) return 0; - spin_lock_irqsave(&i2s_tdm->lock, flags); + guard(spinlock_irqsave)(&i2s_tdm->lock); if (i2s_tdm->refcount) rockchip_i2s_tdm_xfer_pause(substream, i2s_tdm); @@ -614,7 +611,6 @@ static int rockchip_i2s_trcm_mode(struct snd_pcm_substream *substream, if (i2s_tdm->refcount) rockchip_i2s_tdm_xfer_resume(substream, i2s_tdm); - spin_unlock_irqrestore(&i2s_tdm->lock, flags); return 0; } -- 2.43.0 _______________________________________________ Linux-rockchip mailing list Linux-rockchip@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-rockchip ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 2/3] ASoC: rockchip: i2s-tdm: Use guard() for spin locks @ 2026-06-04 3:35 ` phucduc.bui 0 siblings, 0 replies; 16+ messages in thread From: phucduc.bui @ 2026-06-04 3:35 UTC (permalink / raw) To: Heiko Stuebner, Mark Brown, Liam Girdwood Cc: Nicolas Frattaroli, Jaroslav Kysela, Takashi Iwai, linux-sound, linux-rockchip, linux-arm-kernel, linux-kernel, bui duc phuc From: bui duc phuc <phucduc.bui@gmail.com> Clean up the code using guard() for spin locks. Merely code refactoring, and no behavior change. Signed-off-by: bui duc phuc <phucduc.bui@gmail.com> --- NOTE: This patch is compile-tested only. sound/soc/rockchip/rockchip_i2s_tdm.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/sound/soc/rockchip/rockchip_i2s_tdm.c b/sound/soc/rockchip/rockchip_i2s_tdm.c index fc52149ed6ae..3f3db28b8940 100644 --- a/sound/soc/rockchip/rockchip_i2s_tdm.c +++ b/sound/soc/rockchip/rockchip_i2s_tdm.c @@ -285,9 +285,8 @@ static void rockchip_snd_txrxctrl(struct snd_pcm_substream *substream, struct snd_soc_dai *dai, int on) { struct rk_i2s_tdm_dev *i2s_tdm = to_info(dai); - unsigned long flags; - spin_lock_irqsave(&i2s_tdm->lock, flags); + guard(spinlock_irqsave)(&i2s_tdm->lock); if (on) { if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) rockchip_enable_tde(i2s_tdm->regmap); @@ -313,7 +312,6 @@ static void rockchip_snd_txrxctrl(struct snd_pcm_substream *substream, I2S_CLR_TXC | I2S_CLR_RXC); } } - spin_unlock_irqrestore(&i2s_tdm->lock, flags); } static void rockchip_snd_txctrl(struct rk_i2s_tdm_dev *i2s_tdm, int on) @@ -587,12 +585,11 @@ static int rockchip_i2s_trcm_mode(struct snd_pcm_substream *substream, unsigned int fmt) { struct rk_i2s_tdm_dev *i2s_tdm = to_info(dai); - unsigned long flags; if (!i2s_tdm->clk_trcm) return 0; - spin_lock_irqsave(&i2s_tdm->lock, flags); + guard(spinlock_irqsave)(&i2s_tdm->lock); if (i2s_tdm->refcount) rockchip_i2s_tdm_xfer_pause(substream, i2s_tdm); @@ -614,7 +611,6 @@ static int rockchip_i2s_trcm_mode(struct snd_pcm_substream *substream, if (i2s_tdm->refcount) rockchip_i2s_tdm_xfer_resume(substream, i2s_tdm); - spin_unlock_irqrestore(&i2s_tdm->lock, flags); return 0; } -- 2.43.0 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 3/3] ASoC: rockchip: rockchip_sai: Use guard() for spin locks 2026-06-04 3:35 ` phucduc.bui @ 2026-06-04 3:35 ` phucduc.bui -1 siblings, 0 replies; 16+ messages in thread From: phucduc.bui @ 2026-06-04 3:35 UTC (permalink / raw) To: Heiko Stuebner, Mark Brown, Liam Girdwood Cc: Nicolas Frattaroli, Jaroslav Kysela, Takashi Iwai, linux-sound, linux-rockchip, linux-arm-kernel, linux-kernel, bui duc phuc From: bui duc phuc <phucduc.bui@gmail.com> Clean up the code using guard() for spin locks. Merely code refactoring, and no behavior change. Signed-off-by: bui duc phuc <phucduc.bui@gmail.com> --- NOTE: This patch is compile-tested only. Changes in v2: - Remove the unnecessary err_pm_put label in rockchip_sai_hw_params(). sound/soc/rockchip/rockchip_sai.c | 262 +++++++++++++++--------------- 1 file changed, 129 insertions(+), 133 deletions(-) diff --git a/sound/soc/rockchip/rockchip_sai.c b/sound/soc/rockchip/rockchip_sai.c index ed393e5034a4..a195e96fed0a 100644 --- a/sound/soc/rockchip/rockchip_sai.c +++ b/sound/soc/rockchip/rockchip_sai.c @@ -18,7 +18,6 @@ #include <sound/pcm_params.h> #include <sound/dmaengine_pcm.h> #include <sound/tlv.h> - #include "rockchip_sai.h" #define DRV_NAME "rockchip-sai" @@ -216,14 +215,12 @@ static void rockchip_sai_xfer_clk_stop_and_wait(struct rk_sai_dev *sai, unsigned static int rockchip_sai_runtime_suspend(struct device *dev) { struct rk_sai_dev *sai = dev_get_drvdata(dev); - unsigned long flags; rockchip_sai_fsync_lost_detect(sai, 0); rockchip_sai_fsync_err_detect(sai, 0); - spin_lock_irqsave(&sai->xfer_lock, flags); - rockchip_sai_xfer_clk_stop_and_wait(sai, NULL); - spin_unlock_irqrestore(&sai->xfer_lock, flags); + scoped_guard(spinlock_irqsave, &sai->xfer_lock) + rockchip_sai_xfer_clk_stop_and_wait(sai, NULL); regcache_cache_only(sai->regmap, true); /* @@ -483,7 +480,6 @@ static int rockchip_sai_set_fmt(struct snd_soc_dai *dai, unsigned int fmt) struct rk_sai_dev *sai = snd_soc_dai_get_drvdata(dai); unsigned int mask = 0, val = 0; unsigned int clk_gates; - unsigned long flags; int ret = 0; pm_runtime_get_sync(dai->dev); @@ -499,56 +495,56 @@ static int rockchip_sai_set_fmt(struct snd_soc_dai *dai, unsigned int fmt) sai->is_master_mode = false; break; default: - ret = -EINVAL; - goto err_pm_put; - } - - spin_lock_irqsave(&sai->xfer_lock, flags); - rockchip_sai_xfer_clk_stop_and_wait(sai, &clk_gates); - if (sai->initialized) { - if (sai->has_capture && sai->has_playback) - rockchip_sai_xfer_stop(sai, -1); - else if (sai->has_capture) - rockchip_sai_xfer_stop(sai, SNDRV_PCM_STREAM_CAPTURE); - else - rockchip_sai_xfer_stop(sai, SNDRV_PCM_STREAM_PLAYBACK); - } else { - rockchip_sai_clear(sai, 0); - sai->initialized = true; + pm_runtime_put(dai->dev); + return -EINVAL; } - regmap_update_bits(sai->regmap, SAI_CKR, mask, val); + scoped_guard(spinlock_irqsave, &sai->xfer_lock) { + rockchip_sai_xfer_clk_stop_and_wait(sai, &clk_gates); + if (sai->initialized) { + if (sai->has_capture && sai->has_playback) + rockchip_sai_xfer_stop(sai, -1); + else if (sai->has_capture) + rockchip_sai_xfer_stop(sai, SNDRV_PCM_STREAM_CAPTURE); + else + rockchip_sai_xfer_stop(sai, SNDRV_PCM_STREAM_PLAYBACK); + } else { + rockchip_sai_clear(sai, 0); + sai->initialized = true; + } - mask = SAI_CKR_CKP_MASK | SAI_CKR_FSP_MASK; - switch (fmt & SND_SOC_DAIFMT_INV_MASK) { - case SND_SOC_DAIFMT_NB_NF: - val = SAI_CKR_CKP_NORMAL | SAI_CKR_FSP_NORMAL; - break; - case SND_SOC_DAIFMT_NB_IF: - val = SAI_CKR_CKP_NORMAL | SAI_CKR_FSP_INVERTED; - break; - case SND_SOC_DAIFMT_IB_NF: - val = SAI_CKR_CKP_INVERTED | SAI_CKR_FSP_NORMAL; - break; - case SND_SOC_DAIFMT_IB_IF: - val = SAI_CKR_CKP_INVERTED | SAI_CKR_FSP_INVERTED; - break; - default: - ret = -EINVAL; - goto err_xfer_unlock; - } + regmap_update_bits(sai->regmap, SAI_CKR, mask, val); + + mask = SAI_CKR_CKP_MASK | SAI_CKR_FSP_MASK; + switch (fmt & SND_SOC_DAIFMT_INV_MASK) { + case SND_SOC_DAIFMT_NB_NF: + val = SAI_CKR_CKP_NORMAL | SAI_CKR_FSP_NORMAL; + break; + case SND_SOC_DAIFMT_NB_IF: + val = SAI_CKR_CKP_NORMAL | SAI_CKR_FSP_INVERTED; + break; + case SND_SOC_DAIFMT_IB_NF: + val = SAI_CKR_CKP_INVERTED | SAI_CKR_FSP_NORMAL; + break; + case SND_SOC_DAIFMT_IB_IF: + val = SAI_CKR_CKP_INVERTED | SAI_CKR_FSP_INVERTED; + break; + default: + ret = -EINVAL; + break; + } - regmap_update_bits(sai->regmap, SAI_CKR, mask, val); + if (ret == 0) { + regmap_update_bits(sai->regmap, SAI_CKR, mask, val); + rockchip_sai_fmt_create(sai, fmt); + } - rockchip_sai_fmt_create(sai, fmt); + if (clk_gates) + regmap_update_bits(sai->regmap, SAI_XFER, + SAI_XFER_CLK_MASK | SAI_XFER_FSS_MASK, + clk_gates); + } -err_xfer_unlock: - if (clk_gates) - regmap_update_bits(sai->regmap, SAI_XFER, - SAI_XFER_CLK_MASK | SAI_XFER_FSS_MASK, - clk_gates); - spin_unlock_irqrestore(&sai->xfer_lock, flags); -err_pm_put: pm_runtime_put(dai->dev); return ret; @@ -564,7 +560,6 @@ static int rockchip_sai_hw_params(struct snd_pcm_substream *substream, unsigned int ch_per_lane, slot_width; unsigned int val, fscr, reg; unsigned int lanes, req_lanes; - unsigned long flags; int ret = 0; if (!rockchip_sai_stream_valid(substream, dai)) @@ -591,8 +586,8 @@ static int rockchip_sai_hw_params(struct snd_pcm_substream *substream, dev_err(sai->dev, "not enough lanes (%d) for requested number of %s channels (%d)\n", lanes, reg == SAI_TXCR ? "playback" : "capture", params_channels(params)); - ret = -EINVAL; - goto err_pm_put; + pm_runtime_put(sai->dev); + return -EINVAL; } else { lanes = req_lanes; } @@ -618,84 +613,88 @@ static int rockchip_sai_hw_params(struct snd_pcm_substream *substream, val = SAI_XCR_VDW(32); break; default: - ret = -EINVAL; - goto err_pm_put; + pm_runtime_put(sai->dev); + return -EINVAL; } val |= SAI_XCR_CSR(lanes); - spin_lock_irqsave(&sai->xfer_lock, flags); - - regmap_update_bits(sai->regmap, reg, SAI_XCR_VDW_MASK | SAI_XCR_CSR_MASK, val); + scoped_guard(spinlock_irqsave, &sai->xfer_lock) { - if (!sai->is_tdm) - regmap_update_bits(sai->regmap, reg, SAI_XCR_SBW_MASK, - SAI_XCR_SBW(params_physical_width(params))); + regmap_update_bits(sai->regmap, reg, SAI_XCR_VDW_MASK | SAI_XCR_CSR_MASK, val); - regmap_read(sai->regmap, reg, &val); + if (!sai->is_tdm) + regmap_update_bits(sai->regmap, reg, SAI_XCR_SBW_MASK, + SAI_XCR_SBW(params_physical_width(params))); - slot_width = SAI_XCR_SBW_V(val); - ch_per_lane = params_channels(params) / lanes; + regmap_read(sai->regmap, reg, &val); - regmap_update_bits(sai->regmap, reg, SAI_XCR_SNB_MASK, - SAI_XCR_SNB(ch_per_lane)); + slot_width = SAI_XCR_SBW_V(val); + ch_per_lane = params_channels(params) / lanes; - fscr = SAI_FSCR_FW(sai->fw_ratio * slot_width * ch_per_lane); + regmap_update_bits(sai->regmap, reg, SAI_XCR_SNB_MASK, + SAI_XCR_SNB(ch_per_lane)); - switch (sai->fpw) { - case FPW_ONE_BCLK_WIDTH: - fscr |= SAI_FSCR_FPW(1); - break; - case FPW_ONE_SLOT_WIDTH: - fscr |= SAI_FSCR_FPW(slot_width); - break; - case FPW_HALF_FRAME_WIDTH: - fscr |= SAI_FSCR_FPW(sai->fw_ratio * slot_width * ch_per_lane / 2); - break; - default: - dev_err(sai->dev, "Invalid Frame Pulse Width %d\n", sai->fpw); - ret = -EINVAL; - goto err_xfer_unlock; - } + fscr = SAI_FSCR_FW(sai->fw_ratio * slot_width * ch_per_lane); - regmap_update_bits(sai->regmap, SAI_FSCR, - SAI_FSCR_FW_MASK | SAI_FSCR_FPW_MASK, fscr); - - if (sai->is_master_mode) { - bclk_rate = sai->fw_ratio * slot_width * ch_per_lane * params_rate(params); - ret = clk_set_rate(sai->mclk, sai->mclk_rate); - if (ret) { - dev_err(sai->dev, "Failed to set mclk to %u: %pe\n", - sai->mclk_rate, ERR_PTR(ret)); - goto err_xfer_unlock; - } - - mclk_rate = clk_get_rate(sai->mclk); - if (mclk_rate < bclk_rate) { - dev_err(sai->dev, "Mismatch mclk: %u, at least %u\n", - mclk_rate, bclk_rate); + switch (sai->fpw) { + case FPW_ONE_BCLK_WIDTH: + fscr |= SAI_FSCR_FPW(1); + break; + case FPW_ONE_SLOT_WIDTH: + fscr |= SAI_FSCR_FPW(slot_width); + break; + case FPW_HALF_FRAME_WIDTH: + fscr |= SAI_FSCR_FPW(sai->fw_ratio * slot_width * ch_per_lane / 2); + break; + default: + dev_err(sai->dev, "Invalid Frame Pulse Width %d\n", sai->fpw); ret = -EINVAL; - goto err_xfer_unlock; + break; } - div_bclk = DIV_ROUND_CLOSEST(mclk_rate, bclk_rate); - mclk_req_rate = bclk_rate * div_bclk; - - if (mclk_rate < mclk_req_rate - CLK_SHIFT_RATE_HZ_MAX || - mclk_rate > mclk_req_rate + CLK_SHIFT_RATE_HZ_MAX) { - dev_err(sai->dev, "Mismatch mclk: %u, expected %u (+/- %dHz)\n", - mclk_rate, mclk_req_rate, CLK_SHIFT_RATE_HZ_MAX); - ret = -EINVAL; - goto err_xfer_unlock; + if (ret == 0) { + regmap_update_bits(sai->regmap, SAI_FSCR, + SAI_FSCR_FW_MASK | SAI_FSCR_FPW_MASK, fscr); + + if (sai->is_master_mode) { + bclk_rate = sai->fw_ratio * slot_width * + ch_per_lane * params_rate(params); + ret = clk_set_rate(sai->mclk, sai->mclk_rate); + if (ret) + dev_err(sai->dev, "Failed to set mclk to %u: %pe\n", + sai->mclk_rate, ERR_PTR(ret)); + else { + mclk_rate = clk_get_rate(sai->mclk); + if (mclk_rate < bclk_rate) { + dev_err(sai->dev, "Mismatch mclk: %u, at least %u\n", + mclk_rate, bclk_rate); + ret = -EINVAL; + } else { + + div_bclk = DIV_ROUND_CLOSEST(mclk_rate, bclk_rate); + mclk_req_rate = bclk_rate * div_bclk; + + if (mclk_rate < + mclk_req_rate - CLK_SHIFT_RATE_HZ_MAX || + mclk_rate > + mclk_req_rate + CLK_SHIFT_RATE_HZ_MAX) { + dev_err(sai->dev, + "Mismatch mclk: %u, expected %u (+/- %dHz)\n", + mclk_rate, mclk_req_rate, + CLK_SHIFT_RATE_HZ_MAX); + ret = -EINVAL; + } else + regmap_update_bits(sai->regmap, + SAI_CKR, + SAI_CKR_MDIV_MASK, + SAI_CKR_MDIV(div_bclk)); + } + } + } } - - regmap_update_bits(sai->regmap, SAI_CKR, SAI_CKR_MDIV_MASK, - SAI_CKR_MDIV(div_bclk)); } -err_xfer_unlock: - spin_unlock_irqrestore(&sai->xfer_lock, flags); -err_pm_put: pm_runtime_put(sai->dev); return ret; @@ -705,7 +704,6 @@ static int rockchip_sai_prepare(struct snd_pcm_substream *substream, struct snd_soc_dai *dai) { struct rk_sai_dev *sai = snd_soc_dai_get_drvdata(dai); - unsigned long flags; if (!rockchip_sai_stream_valid(substream, dai)) return 0; @@ -726,13 +724,12 @@ static int rockchip_sai_prepare(struct snd_pcm_substream *substream, * udelay falls short. */ udelay(20); - spin_lock_irqsave(&sai->xfer_lock, flags); - regmap_update_bits(sai->regmap, SAI_XFER, - SAI_XFER_CLK_MASK | - SAI_XFER_FSS_MASK, - SAI_XFER_CLK_EN | - SAI_XFER_FSS_EN); - spin_unlock_irqrestore(&sai->xfer_lock, flags); + scoped_guard(spinlock_irqsave, &sai->xfer_lock) + regmap_update_bits(sai->regmap, SAI_XFER, + SAI_XFER_CLK_MASK | + SAI_XFER_FSS_MASK, + SAI_XFER_CLK_EN | + SAI_XFER_FSS_EN); } rockchip_sai_fsync_lost_detect(sai, 1); @@ -915,7 +912,6 @@ static int rockchip_sai_set_tdm_slot(struct snd_soc_dai *dai, int slots, int slot_width) { struct rk_sai_dev *sai = snd_soc_dai_get_drvdata(dai); - unsigned long flags; unsigned int clk_gates; int sw = slot_width; @@ -931,16 +927,16 @@ static int rockchip_sai_set_tdm_slot(struct snd_soc_dai *dai, return -EINVAL; pm_runtime_get_sync(dai->dev); - spin_lock_irqsave(&sai->xfer_lock, flags); - rockchip_sai_xfer_clk_stop_and_wait(sai, &clk_gates); - regmap_update_bits(sai->regmap, SAI_TXCR, SAI_XCR_SBW_MASK, - SAI_XCR_SBW(sw)); - regmap_update_bits(sai->regmap, SAI_RXCR, SAI_XCR_SBW_MASK, - SAI_XCR_SBW(sw)); - regmap_update_bits(sai->regmap, SAI_XFER, - SAI_XFER_CLK_MASK | SAI_XFER_FSS_MASK, - clk_gates); - spin_unlock_irqrestore(&sai->xfer_lock, flags); + scoped_guard(spinlock_irqsave, &sai->xfer_lock) { + rockchip_sai_xfer_clk_stop_and_wait(sai, &clk_gates); + regmap_update_bits(sai->regmap, SAI_TXCR, SAI_XCR_SBW_MASK, + SAI_XCR_SBW(sw)); + regmap_update_bits(sai->regmap, SAI_RXCR, SAI_XCR_SBW_MASK, + SAI_XCR_SBW(sw)); + regmap_update_bits(sai->regmap, SAI_XFER, + SAI_XFER_CLK_MASK | SAI_XFER_FSS_MASK, + clk_gates); + } pm_runtime_put(dai->dev); return 0; -- 2.43.0 _______________________________________________ Linux-rockchip mailing list Linux-rockchip@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-rockchip ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 3/3] ASoC: rockchip: rockchip_sai: Use guard() for spin locks @ 2026-06-04 3:35 ` phucduc.bui 0 siblings, 0 replies; 16+ messages in thread From: phucduc.bui @ 2026-06-04 3:35 UTC (permalink / raw) To: Heiko Stuebner, Mark Brown, Liam Girdwood Cc: Nicolas Frattaroli, Jaroslav Kysela, Takashi Iwai, linux-sound, linux-rockchip, linux-arm-kernel, linux-kernel, bui duc phuc From: bui duc phuc <phucduc.bui@gmail.com> Clean up the code using guard() for spin locks. Merely code refactoring, and no behavior change. Signed-off-by: bui duc phuc <phucduc.bui@gmail.com> --- NOTE: This patch is compile-tested only. Changes in v2: - Remove the unnecessary err_pm_put label in rockchip_sai_hw_params(). sound/soc/rockchip/rockchip_sai.c | 262 +++++++++++++++--------------- 1 file changed, 129 insertions(+), 133 deletions(-) diff --git a/sound/soc/rockchip/rockchip_sai.c b/sound/soc/rockchip/rockchip_sai.c index ed393e5034a4..a195e96fed0a 100644 --- a/sound/soc/rockchip/rockchip_sai.c +++ b/sound/soc/rockchip/rockchip_sai.c @@ -18,7 +18,6 @@ #include <sound/pcm_params.h> #include <sound/dmaengine_pcm.h> #include <sound/tlv.h> - #include "rockchip_sai.h" #define DRV_NAME "rockchip-sai" @@ -216,14 +215,12 @@ static void rockchip_sai_xfer_clk_stop_and_wait(struct rk_sai_dev *sai, unsigned static int rockchip_sai_runtime_suspend(struct device *dev) { struct rk_sai_dev *sai = dev_get_drvdata(dev); - unsigned long flags; rockchip_sai_fsync_lost_detect(sai, 0); rockchip_sai_fsync_err_detect(sai, 0); - spin_lock_irqsave(&sai->xfer_lock, flags); - rockchip_sai_xfer_clk_stop_and_wait(sai, NULL); - spin_unlock_irqrestore(&sai->xfer_lock, flags); + scoped_guard(spinlock_irqsave, &sai->xfer_lock) + rockchip_sai_xfer_clk_stop_and_wait(sai, NULL); regcache_cache_only(sai->regmap, true); /* @@ -483,7 +480,6 @@ static int rockchip_sai_set_fmt(struct snd_soc_dai *dai, unsigned int fmt) struct rk_sai_dev *sai = snd_soc_dai_get_drvdata(dai); unsigned int mask = 0, val = 0; unsigned int clk_gates; - unsigned long flags; int ret = 0; pm_runtime_get_sync(dai->dev); @@ -499,56 +495,56 @@ static int rockchip_sai_set_fmt(struct snd_soc_dai *dai, unsigned int fmt) sai->is_master_mode = false; break; default: - ret = -EINVAL; - goto err_pm_put; - } - - spin_lock_irqsave(&sai->xfer_lock, flags); - rockchip_sai_xfer_clk_stop_and_wait(sai, &clk_gates); - if (sai->initialized) { - if (sai->has_capture && sai->has_playback) - rockchip_sai_xfer_stop(sai, -1); - else if (sai->has_capture) - rockchip_sai_xfer_stop(sai, SNDRV_PCM_STREAM_CAPTURE); - else - rockchip_sai_xfer_stop(sai, SNDRV_PCM_STREAM_PLAYBACK); - } else { - rockchip_sai_clear(sai, 0); - sai->initialized = true; + pm_runtime_put(dai->dev); + return -EINVAL; } - regmap_update_bits(sai->regmap, SAI_CKR, mask, val); + scoped_guard(spinlock_irqsave, &sai->xfer_lock) { + rockchip_sai_xfer_clk_stop_and_wait(sai, &clk_gates); + if (sai->initialized) { + if (sai->has_capture && sai->has_playback) + rockchip_sai_xfer_stop(sai, -1); + else if (sai->has_capture) + rockchip_sai_xfer_stop(sai, SNDRV_PCM_STREAM_CAPTURE); + else + rockchip_sai_xfer_stop(sai, SNDRV_PCM_STREAM_PLAYBACK); + } else { + rockchip_sai_clear(sai, 0); + sai->initialized = true; + } - mask = SAI_CKR_CKP_MASK | SAI_CKR_FSP_MASK; - switch (fmt & SND_SOC_DAIFMT_INV_MASK) { - case SND_SOC_DAIFMT_NB_NF: - val = SAI_CKR_CKP_NORMAL | SAI_CKR_FSP_NORMAL; - break; - case SND_SOC_DAIFMT_NB_IF: - val = SAI_CKR_CKP_NORMAL | SAI_CKR_FSP_INVERTED; - break; - case SND_SOC_DAIFMT_IB_NF: - val = SAI_CKR_CKP_INVERTED | SAI_CKR_FSP_NORMAL; - break; - case SND_SOC_DAIFMT_IB_IF: - val = SAI_CKR_CKP_INVERTED | SAI_CKR_FSP_INVERTED; - break; - default: - ret = -EINVAL; - goto err_xfer_unlock; - } + regmap_update_bits(sai->regmap, SAI_CKR, mask, val); + + mask = SAI_CKR_CKP_MASK | SAI_CKR_FSP_MASK; + switch (fmt & SND_SOC_DAIFMT_INV_MASK) { + case SND_SOC_DAIFMT_NB_NF: + val = SAI_CKR_CKP_NORMAL | SAI_CKR_FSP_NORMAL; + break; + case SND_SOC_DAIFMT_NB_IF: + val = SAI_CKR_CKP_NORMAL | SAI_CKR_FSP_INVERTED; + break; + case SND_SOC_DAIFMT_IB_NF: + val = SAI_CKR_CKP_INVERTED | SAI_CKR_FSP_NORMAL; + break; + case SND_SOC_DAIFMT_IB_IF: + val = SAI_CKR_CKP_INVERTED | SAI_CKR_FSP_INVERTED; + break; + default: + ret = -EINVAL; + break; + } - regmap_update_bits(sai->regmap, SAI_CKR, mask, val); + if (ret == 0) { + regmap_update_bits(sai->regmap, SAI_CKR, mask, val); + rockchip_sai_fmt_create(sai, fmt); + } - rockchip_sai_fmt_create(sai, fmt); + if (clk_gates) + regmap_update_bits(sai->regmap, SAI_XFER, + SAI_XFER_CLK_MASK | SAI_XFER_FSS_MASK, + clk_gates); + } -err_xfer_unlock: - if (clk_gates) - regmap_update_bits(sai->regmap, SAI_XFER, - SAI_XFER_CLK_MASK | SAI_XFER_FSS_MASK, - clk_gates); - spin_unlock_irqrestore(&sai->xfer_lock, flags); -err_pm_put: pm_runtime_put(dai->dev); return ret; @@ -564,7 +560,6 @@ static int rockchip_sai_hw_params(struct snd_pcm_substream *substream, unsigned int ch_per_lane, slot_width; unsigned int val, fscr, reg; unsigned int lanes, req_lanes; - unsigned long flags; int ret = 0; if (!rockchip_sai_stream_valid(substream, dai)) @@ -591,8 +586,8 @@ static int rockchip_sai_hw_params(struct snd_pcm_substream *substream, dev_err(sai->dev, "not enough lanes (%d) for requested number of %s channels (%d)\n", lanes, reg == SAI_TXCR ? "playback" : "capture", params_channels(params)); - ret = -EINVAL; - goto err_pm_put; + pm_runtime_put(sai->dev); + return -EINVAL; } else { lanes = req_lanes; } @@ -618,84 +613,88 @@ static int rockchip_sai_hw_params(struct snd_pcm_substream *substream, val = SAI_XCR_VDW(32); break; default: - ret = -EINVAL; - goto err_pm_put; + pm_runtime_put(sai->dev); + return -EINVAL; } val |= SAI_XCR_CSR(lanes); - spin_lock_irqsave(&sai->xfer_lock, flags); - - regmap_update_bits(sai->regmap, reg, SAI_XCR_VDW_MASK | SAI_XCR_CSR_MASK, val); + scoped_guard(spinlock_irqsave, &sai->xfer_lock) { - if (!sai->is_tdm) - regmap_update_bits(sai->regmap, reg, SAI_XCR_SBW_MASK, - SAI_XCR_SBW(params_physical_width(params))); + regmap_update_bits(sai->regmap, reg, SAI_XCR_VDW_MASK | SAI_XCR_CSR_MASK, val); - regmap_read(sai->regmap, reg, &val); + if (!sai->is_tdm) + regmap_update_bits(sai->regmap, reg, SAI_XCR_SBW_MASK, + SAI_XCR_SBW(params_physical_width(params))); - slot_width = SAI_XCR_SBW_V(val); - ch_per_lane = params_channels(params) / lanes; + regmap_read(sai->regmap, reg, &val); - regmap_update_bits(sai->regmap, reg, SAI_XCR_SNB_MASK, - SAI_XCR_SNB(ch_per_lane)); + slot_width = SAI_XCR_SBW_V(val); + ch_per_lane = params_channels(params) / lanes; - fscr = SAI_FSCR_FW(sai->fw_ratio * slot_width * ch_per_lane); + regmap_update_bits(sai->regmap, reg, SAI_XCR_SNB_MASK, + SAI_XCR_SNB(ch_per_lane)); - switch (sai->fpw) { - case FPW_ONE_BCLK_WIDTH: - fscr |= SAI_FSCR_FPW(1); - break; - case FPW_ONE_SLOT_WIDTH: - fscr |= SAI_FSCR_FPW(slot_width); - break; - case FPW_HALF_FRAME_WIDTH: - fscr |= SAI_FSCR_FPW(sai->fw_ratio * slot_width * ch_per_lane / 2); - break; - default: - dev_err(sai->dev, "Invalid Frame Pulse Width %d\n", sai->fpw); - ret = -EINVAL; - goto err_xfer_unlock; - } + fscr = SAI_FSCR_FW(sai->fw_ratio * slot_width * ch_per_lane); - regmap_update_bits(sai->regmap, SAI_FSCR, - SAI_FSCR_FW_MASK | SAI_FSCR_FPW_MASK, fscr); - - if (sai->is_master_mode) { - bclk_rate = sai->fw_ratio * slot_width * ch_per_lane * params_rate(params); - ret = clk_set_rate(sai->mclk, sai->mclk_rate); - if (ret) { - dev_err(sai->dev, "Failed to set mclk to %u: %pe\n", - sai->mclk_rate, ERR_PTR(ret)); - goto err_xfer_unlock; - } - - mclk_rate = clk_get_rate(sai->mclk); - if (mclk_rate < bclk_rate) { - dev_err(sai->dev, "Mismatch mclk: %u, at least %u\n", - mclk_rate, bclk_rate); + switch (sai->fpw) { + case FPW_ONE_BCLK_WIDTH: + fscr |= SAI_FSCR_FPW(1); + break; + case FPW_ONE_SLOT_WIDTH: + fscr |= SAI_FSCR_FPW(slot_width); + break; + case FPW_HALF_FRAME_WIDTH: + fscr |= SAI_FSCR_FPW(sai->fw_ratio * slot_width * ch_per_lane / 2); + break; + default: + dev_err(sai->dev, "Invalid Frame Pulse Width %d\n", sai->fpw); ret = -EINVAL; - goto err_xfer_unlock; + break; } - div_bclk = DIV_ROUND_CLOSEST(mclk_rate, bclk_rate); - mclk_req_rate = bclk_rate * div_bclk; - - if (mclk_rate < mclk_req_rate - CLK_SHIFT_RATE_HZ_MAX || - mclk_rate > mclk_req_rate + CLK_SHIFT_RATE_HZ_MAX) { - dev_err(sai->dev, "Mismatch mclk: %u, expected %u (+/- %dHz)\n", - mclk_rate, mclk_req_rate, CLK_SHIFT_RATE_HZ_MAX); - ret = -EINVAL; - goto err_xfer_unlock; + if (ret == 0) { + regmap_update_bits(sai->regmap, SAI_FSCR, + SAI_FSCR_FW_MASK | SAI_FSCR_FPW_MASK, fscr); + + if (sai->is_master_mode) { + bclk_rate = sai->fw_ratio * slot_width * + ch_per_lane * params_rate(params); + ret = clk_set_rate(sai->mclk, sai->mclk_rate); + if (ret) + dev_err(sai->dev, "Failed to set mclk to %u: %pe\n", + sai->mclk_rate, ERR_PTR(ret)); + else { + mclk_rate = clk_get_rate(sai->mclk); + if (mclk_rate < bclk_rate) { + dev_err(sai->dev, "Mismatch mclk: %u, at least %u\n", + mclk_rate, bclk_rate); + ret = -EINVAL; + } else { + + div_bclk = DIV_ROUND_CLOSEST(mclk_rate, bclk_rate); + mclk_req_rate = bclk_rate * div_bclk; + + if (mclk_rate < + mclk_req_rate - CLK_SHIFT_RATE_HZ_MAX || + mclk_rate > + mclk_req_rate + CLK_SHIFT_RATE_HZ_MAX) { + dev_err(sai->dev, + "Mismatch mclk: %u, expected %u (+/- %dHz)\n", + mclk_rate, mclk_req_rate, + CLK_SHIFT_RATE_HZ_MAX); + ret = -EINVAL; + } else + regmap_update_bits(sai->regmap, + SAI_CKR, + SAI_CKR_MDIV_MASK, + SAI_CKR_MDIV(div_bclk)); + } + } + } } - - regmap_update_bits(sai->regmap, SAI_CKR, SAI_CKR_MDIV_MASK, - SAI_CKR_MDIV(div_bclk)); } -err_xfer_unlock: - spin_unlock_irqrestore(&sai->xfer_lock, flags); -err_pm_put: pm_runtime_put(sai->dev); return ret; @@ -705,7 +704,6 @@ static int rockchip_sai_prepare(struct snd_pcm_substream *substream, struct snd_soc_dai *dai) { struct rk_sai_dev *sai = snd_soc_dai_get_drvdata(dai); - unsigned long flags; if (!rockchip_sai_stream_valid(substream, dai)) return 0; @@ -726,13 +724,12 @@ static int rockchip_sai_prepare(struct snd_pcm_substream *substream, * udelay falls short. */ udelay(20); - spin_lock_irqsave(&sai->xfer_lock, flags); - regmap_update_bits(sai->regmap, SAI_XFER, - SAI_XFER_CLK_MASK | - SAI_XFER_FSS_MASK, - SAI_XFER_CLK_EN | - SAI_XFER_FSS_EN); - spin_unlock_irqrestore(&sai->xfer_lock, flags); + scoped_guard(spinlock_irqsave, &sai->xfer_lock) + regmap_update_bits(sai->regmap, SAI_XFER, + SAI_XFER_CLK_MASK | + SAI_XFER_FSS_MASK, + SAI_XFER_CLK_EN | + SAI_XFER_FSS_EN); } rockchip_sai_fsync_lost_detect(sai, 1); @@ -915,7 +912,6 @@ static int rockchip_sai_set_tdm_slot(struct snd_soc_dai *dai, int slots, int slot_width) { struct rk_sai_dev *sai = snd_soc_dai_get_drvdata(dai); - unsigned long flags; unsigned int clk_gates; int sw = slot_width; @@ -931,16 +927,16 @@ static int rockchip_sai_set_tdm_slot(struct snd_soc_dai *dai, return -EINVAL; pm_runtime_get_sync(dai->dev); - spin_lock_irqsave(&sai->xfer_lock, flags); - rockchip_sai_xfer_clk_stop_and_wait(sai, &clk_gates); - regmap_update_bits(sai->regmap, SAI_TXCR, SAI_XCR_SBW_MASK, - SAI_XCR_SBW(sw)); - regmap_update_bits(sai->regmap, SAI_RXCR, SAI_XCR_SBW_MASK, - SAI_XCR_SBW(sw)); - regmap_update_bits(sai->regmap, SAI_XFER, - SAI_XFER_CLK_MASK | SAI_XFER_FSS_MASK, - clk_gates); - spin_unlock_irqrestore(&sai->xfer_lock, flags); + scoped_guard(spinlock_irqsave, &sai->xfer_lock) { + rockchip_sai_xfer_clk_stop_and_wait(sai, &clk_gates); + regmap_update_bits(sai->regmap, SAI_TXCR, SAI_XCR_SBW_MASK, + SAI_XCR_SBW(sw)); + regmap_update_bits(sai->regmap, SAI_RXCR, SAI_XCR_SBW_MASK, + SAI_XCR_SBW(sw)); + regmap_update_bits(sai->regmap, SAI_XFER, + SAI_XFER_CLK_MASK | SAI_XFER_FSS_MASK, + clk_gates); + } pm_runtime_put(dai->dev); return 0; -- 2.43.0 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v2 0/3] ASoC: rockchip: Use guard() for spin locks 2026-06-04 3:35 ` phucduc.bui @ 2026-06-11 19:53 ` Mark Brown -1 siblings, 0 replies; 16+ messages in thread From: Mark Brown @ 2026-06-11 19:53 UTC (permalink / raw) To: Heiko Stuebner, Liam Girdwood, phucduc.bui Cc: Nicolas Frattaroli, Jaroslav Kysela, Takashi Iwai, linux-sound, linux-rockchip, linux-arm-kernel, linux-kernel On Thu, 04 Jun 2026 10:35:50 +0700, phucduc.bui@gmail.com wrote: > ASoC: rockchip: Use guard() for spin locks > > From: bui duc phuc <phucduc.bui@gmail.com> > > Hi all, > > This series converts spinlock handling in the Rockchip sound drivers > to use guard() helpers. > The changes are code cleanup only and should have no functional impact. > > [...] Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-7.2 Thanks! [1/3] ASoC: rockchip: rockchip_i2s: Use guard() for spin locks https://git.kernel.org/broonie/sound/c/4bda5af16920 [2/3] ASoC: rockchip: i2s-tdm: Use guard() for spin locks https://git.kernel.org/broonie/sound/c/ec22437fc41a [3/3] ASoC: rockchip: rockchip_sai: Use guard() for spin locks https://git.kernel.org/broonie/sound/c/f7fe9f707360 All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 0/3] ASoC: rockchip: Use guard() for spin locks @ 2026-06-11 19:53 ` Mark Brown 0 siblings, 0 replies; 16+ messages in thread From: Mark Brown @ 2026-06-11 19:53 UTC (permalink / raw) To: Heiko Stuebner, Liam Girdwood, phucduc.bui Cc: Nicolas Frattaroli, Jaroslav Kysela, Takashi Iwai, linux-sound, linux-rockchip, linux-arm-kernel, linux-kernel On Thu, 04 Jun 2026 10:35:50 +0700, phucduc.bui@gmail.com wrote: > ASoC: rockchip: Use guard() for spin locks > > From: bui duc phuc <phucduc.bui@gmail.com> > > Hi all, > > This series converts spinlock handling in the Rockchip sound drivers > to use guard() helpers. > The changes are code cleanup only and should have no functional impact. > > [...] Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-7.2 Thanks! [1/3] ASoC: rockchip: rockchip_i2s: Use guard() for spin locks https://git.kernel.org/broonie/sound/c/4bda5af16920 [2/3] ASoC: rockchip: i2s-tdm: Use guard() for spin locks https://git.kernel.org/broonie/sound/c/ec22437fc41a [3/3] ASoC: rockchip: rockchip_sai: Use guard() for spin locks https://git.kernel.org/broonie/sound/c/f7fe9f707360 All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark _______________________________________________ Linux-rockchip mailing list Linux-rockchip@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-rockchip ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 0/3] ASoC: rockchip: Use guard() for spin locks 2026-06-11 19:53 ` Mark Brown @ 2026-06-17 11:24 ` Nicolas Frattaroli -1 siblings, 0 replies; 16+ messages in thread From: Nicolas Frattaroli @ 2026-06-17 11:24 UTC (permalink / raw) To: Heiko Stuebner, Liam Girdwood, phucduc.bui, Mark Brown Cc: Nicolas Frattaroli, Jaroslav Kysela, Takashi Iwai, linux-sound, linux-rockchip, linux-arm-kernel, linux-kernel On Thursday, 11 June 2026 21:53:26 Central European Summer Time Mark Brown wrote: > On Thu, 04 Jun 2026 10:35:50 +0700, phucduc.bui@gmail.com wrote: > > ASoC: rockchip: Use guard() for spin locks > > > > From: bui duc phuc <phucduc.bui@gmail.com> > > > > Hi all, > > > > This series converts spinlock handling in the Rockchip sound drivers > > to use guard() helpers. > > The changes are code cleanup only and should have no functional impact. > > > > [...] > > Applied to > > https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-7.2 > > Thanks! FWIW, the SAI patch wasn't sent to the proper maintainer e-mail for it which is why I didn't notice this series at all until now, and the whole thing is pointless churn that wasn't even tested. From a cursory glance, whatever LLM was pointed at this and decided to make the output my problem also didn't do a good job, the scoped indentation of rockchip_sai_runtime_suspend seems pointless because it could've been replaced by a pm guard followed by a spinlock guard, without an additional level of indentation, and `if (ret == 0) {` is not kernel style. It's not worth the revert but it sucks that I have to now set up a new lei search for all the drivers I am supposed to receive mail for because vibecoders can't be bothered to run get_maintainers as they pad their CV with nonsense like this. > > [1/3] ASoC: rockchip: rockchip_i2s: Use guard() for spin locks > https://git.kernel.org/broonie/sound/c/4bda5af16920 > [2/3] ASoC: rockchip: i2s-tdm: Use guard() for spin locks > https://git.kernel.org/broonie/sound/c/ec22437fc41a > [3/3] ASoC: rockchip: rockchip_sai: Use guard() for spin locks > https://git.kernel.org/broonie/sound/c/f7fe9f707360 > > All being well this means that it will be integrated into the linux-next > tree (usually sometime in the next 24 hours) and sent to Linus during > the next merge window (or sooner if it is a bug fix), however if > problems are discovered then the patch may be dropped or reverted. > > You may get further e-mails resulting from automated or manual testing > and review of the tree, please engage with people reporting problems and > send followup patches addressing any issues that are reported if needed. > > If any updates are required or you are submitting further changes they > should be sent as incremental updates against current git, existing > patches will not be replaced. > > Please add any relevant lists and maintainers to the CCs when replying > to this mail. > > Thanks, > Mark > > > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 0/3] ASoC: rockchip: Use guard() for spin locks @ 2026-06-17 11:24 ` Nicolas Frattaroli 0 siblings, 0 replies; 16+ messages in thread From: Nicolas Frattaroli @ 2026-06-17 11:24 UTC (permalink / raw) To: Heiko Stuebner, Liam Girdwood, phucduc.bui, Mark Brown Cc: Nicolas Frattaroli, Jaroslav Kysela, Takashi Iwai, linux-sound, linux-rockchip, linux-arm-kernel, linux-kernel On Thursday, 11 June 2026 21:53:26 Central European Summer Time Mark Brown wrote: > On Thu, 04 Jun 2026 10:35:50 +0700, phucduc.bui@gmail.com wrote: > > ASoC: rockchip: Use guard() for spin locks > > > > From: bui duc phuc <phucduc.bui@gmail.com> > > > > Hi all, > > > > This series converts spinlock handling in the Rockchip sound drivers > > to use guard() helpers. > > The changes are code cleanup only and should have no functional impact. > > > > [...] > > Applied to > > https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-7.2 > > Thanks! FWIW, the SAI patch wasn't sent to the proper maintainer e-mail for it which is why I didn't notice this series at all until now, and the whole thing is pointless churn that wasn't even tested. From a cursory glance, whatever LLM was pointed at this and decided to make the output my problem also didn't do a good job, the scoped indentation of rockchip_sai_runtime_suspend seems pointless because it could've been replaced by a pm guard followed by a spinlock guard, without an additional level of indentation, and `if (ret == 0) {` is not kernel style. It's not worth the revert but it sucks that I have to now set up a new lei search for all the drivers I am supposed to receive mail for because vibecoders can't be bothered to run get_maintainers as they pad their CV with nonsense like this. > > [1/3] ASoC: rockchip: rockchip_i2s: Use guard() for spin locks > https://git.kernel.org/broonie/sound/c/4bda5af16920 > [2/3] ASoC: rockchip: i2s-tdm: Use guard() for spin locks > https://git.kernel.org/broonie/sound/c/ec22437fc41a > [3/3] ASoC: rockchip: rockchip_sai: Use guard() for spin locks > https://git.kernel.org/broonie/sound/c/f7fe9f707360 > > All being well this means that it will be integrated into the linux-next > tree (usually sometime in the next 24 hours) and sent to Linus during > the next merge window (or sooner if it is a bug fix), however if > problems are discovered then the patch may be dropped or reverted. > > You may get further e-mails resulting from automated or manual testing > and review of the tree, please engage with people reporting problems and > send followup patches addressing any issues that are reported if needed. > > If any updates are required or you are submitting further changes they > should be sent as incremental updates against current git, existing > patches will not be replaced. > > Please add any relevant lists and maintainers to the CCs when replying > to this mail. > > Thanks, > Mark > > > _______________________________________________ Linux-rockchip mailing list Linux-rockchip@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-rockchip ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 0/3] ASoC: rockchip: Use guard() for spin locks 2026-06-17 11:24 ` Nicolas Frattaroli @ 2026-06-18 3:16 ` Bui Duc Phuc -1 siblings, 0 replies; 16+ messages in thread From: Bui Duc Phuc @ 2026-06-18 3:16 UTC (permalink / raw) To: Nicolas Frattaroli Cc: Heiko Stuebner, Liam Girdwood, Mark Brown, Nicolas Frattaroli, Jaroslav Kysela, Takashi Iwai, linux-sound, linux-rockchip, linux-arm-kernel, linux-kernel Hi Nicolas, Thanks for the feedback. > FWIW, the SAI patch wasn't sent to the proper maintainer e-mail > for it which is why I didn't notice this series at all until now, > and the whole thing is pointless churn that wasn't even tested. > > From a cursory glance, whatever LLM was pointed at this and decided > to make the output my problem also didn't do a good job, the scoped > indentation of rockchip_sai_runtime_suspend seems pointless because > it could've been replaced by a pm guard followed by a spinlock guard, > without an additional level of indentation, and `if (ret == 0) {` is > not kernel style. > > It's not worth the revert but it sucks that I have to now set up a > new lei search for all the drivers I am supposed to receive mail for > because vibecoders can't be bothered to run get_maintainers as they > pad their CV with nonsense like this. > > > > > [1/3] ASoC: rockchip: rockchip_i2s: Use guard() for spin locks > > https://git.kernel.org/broonie/sound/c/4bda5af16920 > > [2/3] ASoC: rockchip: i2s-tdm: Use guard() for spin locks > > https://git.kernel.org/broonie/sound/c/ec22437fc41a > > [3/3] ASoC: rockchip: rockchip_sai: Use guard() for spin locks > > https://git.kernel.org/broonie/sound/c/f7fe9f707360 > > Regarding the claim that the Rockchip SAI patch was not sent to the proper maintainer, I believe there may be a misunderstanding. Before sending the series, I ran get_maintainer.pl on the patch set and included all recipients reported by the script, including: Nicolas Frattaroli <frattaroli.nicolas@gmail.com> along with the relevant mailing lists. ------------------------------------------ phuc@phuc-desktop:~/work/linux$ ls ../patch/sound/rockchip/clean/ 0000-cover-letter.patch v2-0000-cover-letter.patch 0001-ASoC-rockchip-rockchip_i2s-Use-guard-for-spin-locks.patch v2-0001-ASoC-rockchip-rockchip_i2s-Use-guard-for-spin-loc.patch 0002-ASoC-rockchip-i2s-tdm-Use-guard-for-spin-locks.patch v2-0002-ASoC-rockchip-i2s-tdm-Use-guard-for-spin-locks.patch 0003-ASoC-rockchip-rockchip_sai-Use-guard-for-spin-locks.patch v2-0003-ASoC-rockchip-rockchip_sai-Use-guard-for-spin-loc.patch phuc@phuc-desktop:~/work/linux$ phuc@phuc-desktop:~/work/linux$ phuc@phuc-desktop:~/work/linux$ phuc@phuc-desktop:~/work/linux$ ./scripts/get_maintainer.pl ../patch/sound/rockchip/clean/000*.patch ./scripts/get_maintainer.pl: file '../patch/sound/rockchip/clean/0000-cover-letter.patch' doesn't appear to be a patch. Add -f to options? Liam Girdwood <lgirdwood@gmail.com> (maintainer:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...) Mark Brown <broonie@kernel.org> (maintainer:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...) Jaroslav Kysela <perex@perex.cz> (maintainer:SOUND) Takashi Iwai <tiwai@suse.com> (maintainer:SOUND) Heiko Stuebner <heiko@sntech.de> (maintainer:ARM/Rockchip SoC support) Nicolas Frattaroli <frattaroli.nicolas@gmail.com> (maintainer:ROCKCHIP I2S TDM DRIVER) linux-sound@vger.kernel.org (open list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...) linux-arm-kernel@lists.infradead.org (moderated list:ARM/Rockchip SoC support) linux-rockchip@lists.infradead.org (open list:ARM/Rockchip SoC support) linux-kernel@vger.kernel.org (open list) SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEMENT (ASoC) status: Supported phuc@phuc-desktop:~/work/linux$ phuc@phuc-desktop:~/work/linux$ --------------------------------------------- If there are additional maintainers or mailing lists that should receive these patches but are not currently reported by get_maintainer.pl, please let me know so I can include them in future submissions. I would also appreciate avoiding assumptions that the series was generated by an LLM. The patches were prepared manually and submitted as part of my ongoing effort to convert lock/unlock patterns to guard()/scoped_guard() helpers where appropriate. The series was build-tested before submission, so I do not believe it is accurate to describe it as "wasn't even tested". Whether a particular conversion is worthwhile is certainly open to technical discussion, but I do not think it is reasonable to conclude that a patch was AI-generated simply based on disagreements about the implementation details. I understand maintainers have different preferences regarding how aggressively such conversions should be applied, and I am happy to adjust the approach based on review feedback. Best regards, Phuc ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 0/3] ASoC: rockchip: Use guard() for spin locks @ 2026-06-18 3:16 ` Bui Duc Phuc 0 siblings, 0 replies; 16+ messages in thread From: Bui Duc Phuc @ 2026-06-18 3:16 UTC (permalink / raw) To: Nicolas Frattaroli Cc: Heiko Stuebner, Liam Girdwood, Mark Brown, Nicolas Frattaroli, Jaroslav Kysela, Takashi Iwai, linux-sound, linux-rockchip, linux-arm-kernel, linux-kernel Hi Nicolas, Thanks for the feedback. > FWIW, the SAI patch wasn't sent to the proper maintainer e-mail > for it which is why I didn't notice this series at all until now, > and the whole thing is pointless churn that wasn't even tested. > > From a cursory glance, whatever LLM was pointed at this and decided > to make the output my problem also didn't do a good job, the scoped > indentation of rockchip_sai_runtime_suspend seems pointless because > it could've been replaced by a pm guard followed by a spinlock guard, > without an additional level of indentation, and `if (ret == 0) {` is > not kernel style. > > It's not worth the revert but it sucks that I have to now set up a > new lei search for all the drivers I am supposed to receive mail for > because vibecoders can't be bothered to run get_maintainers as they > pad their CV with nonsense like this. > > > > > [1/3] ASoC: rockchip: rockchip_i2s: Use guard() for spin locks > > https://git.kernel.org/broonie/sound/c/4bda5af16920 > > [2/3] ASoC: rockchip: i2s-tdm: Use guard() for spin locks > > https://git.kernel.org/broonie/sound/c/ec22437fc41a > > [3/3] ASoC: rockchip: rockchip_sai: Use guard() for spin locks > > https://git.kernel.org/broonie/sound/c/f7fe9f707360 > > Regarding the claim that the Rockchip SAI patch was not sent to the proper maintainer, I believe there may be a misunderstanding. Before sending the series, I ran get_maintainer.pl on the patch set and included all recipients reported by the script, including: Nicolas Frattaroli <frattaroli.nicolas@gmail.com> along with the relevant mailing lists. ------------------------------------------ phuc@phuc-desktop:~/work/linux$ ls ../patch/sound/rockchip/clean/ 0000-cover-letter.patch v2-0000-cover-letter.patch 0001-ASoC-rockchip-rockchip_i2s-Use-guard-for-spin-locks.patch v2-0001-ASoC-rockchip-rockchip_i2s-Use-guard-for-spin-loc.patch 0002-ASoC-rockchip-i2s-tdm-Use-guard-for-spin-locks.patch v2-0002-ASoC-rockchip-i2s-tdm-Use-guard-for-spin-locks.patch 0003-ASoC-rockchip-rockchip_sai-Use-guard-for-spin-locks.patch v2-0003-ASoC-rockchip-rockchip_sai-Use-guard-for-spin-loc.patch phuc@phuc-desktop:~/work/linux$ phuc@phuc-desktop:~/work/linux$ phuc@phuc-desktop:~/work/linux$ phuc@phuc-desktop:~/work/linux$ ./scripts/get_maintainer.pl ../patch/sound/rockchip/clean/000*.patch ./scripts/get_maintainer.pl: file '../patch/sound/rockchip/clean/0000-cover-letter.patch' doesn't appear to be a patch. Add -f to options? Liam Girdwood <lgirdwood@gmail.com> (maintainer:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...) Mark Brown <broonie@kernel.org> (maintainer:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...) Jaroslav Kysela <perex@perex.cz> (maintainer:SOUND) Takashi Iwai <tiwai@suse.com> (maintainer:SOUND) Heiko Stuebner <heiko@sntech.de> (maintainer:ARM/Rockchip SoC support) Nicolas Frattaroli <frattaroli.nicolas@gmail.com> (maintainer:ROCKCHIP I2S TDM DRIVER) linux-sound@vger.kernel.org (open list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...) linux-arm-kernel@lists.infradead.org (moderated list:ARM/Rockchip SoC support) linux-rockchip@lists.infradead.org (open list:ARM/Rockchip SoC support) linux-kernel@vger.kernel.org (open list) SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEMENT (ASoC) status: Supported phuc@phuc-desktop:~/work/linux$ phuc@phuc-desktop:~/work/linux$ --------------------------------------------- If there are additional maintainers or mailing lists that should receive these patches but are not currently reported by get_maintainer.pl, please let me know so I can include them in future submissions. I would also appreciate avoiding assumptions that the series was generated by an LLM. The patches were prepared manually and submitted as part of my ongoing effort to convert lock/unlock patterns to guard()/scoped_guard() helpers where appropriate. The series was build-tested before submission, so I do not believe it is accurate to describe it as "wasn't even tested". Whether a particular conversion is worthwhile is certainly open to technical discussion, but I do not think it is reasonable to conclude that a patch was AI-generated simply based on disagreements about the implementation details. I understand maintainers have different preferences regarding how aggressively such conversions should be applied, and I am happy to adjust the approach based on review feedback. Best regards, Phuc _______________________________________________ Linux-rockchip mailing list Linux-rockchip@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-rockchip ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 0/3] ASoC: rockchip: Use guard() for spin locks 2026-06-18 3:16 ` Bui Duc Phuc @ 2026-06-18 12:16 ` Nicolas Frattaroli -1 siblings, 0 replies; 16+ messages in thread From: Nicolas Frattaroli @ 2026-06-18 12:16 UTC (permalink / raw) To: Bui Duc Phuc Cc: Heiko Stuebner, Liam Girdwood, Mark Brown, Nicolas Frattaroli, Jaroslav Kysela, Takashi Iwai, linux-sound, linux-rockchip, linux-arm-kernel, linux-kernel On Thursday, 18 June 2026 05:16:24 Central European Summer Time Bui Duc Phuc wrote: > Hi Nicolas, > > Thanks for the feedback. > > > FWIW, the SAI patch wasn't sent to the proper maintainer e-mail > > for it which is why I didn't notice this series at all until now, > > and the whole thing is pointless churn that wasn't even tested. > > > > From a cursory glance, whatever LLM was pointed at this and decided > > to make the output my problem also didn't do a good job, the scoped > > indentation of rockchip_sai_runtime_suspend seems pointless because > > it could've been replaced by a pm guard followed by a spinlock guard, > > without an additional level of indentation, and `if (ret == 0) {` is > > not kernel style. > > > > It's not worth the revert but it sucks that I have to now set up a > > new lei search for all the drivers I am supposed to receive mail for > > because vibecoders can't be bothered to run get_maintainers as they > > pad their CV with nonsense like this. > > > > > > > > [1/3] ASoC: rockchip: rockchip_i2s: Use guard() for spin locks > > > https://git.kernel.org/broonie/sound/c/4bda5af16920 > > > [2/3] ASoC: rockchip: i2s-tdm: Use guard() for spin locks > > > https://git.kernel.org/broonie/sound/c/ec22437fc41a > > > [3/3] ASoC: rockchip: rockchip_sai: Use guard() for spin locks > > > https://git.kernel.org/broonie/sound/c/f7fe9f707360 > > > > > Regarding the claim that the Rockchip SAI patch was not sent to the proper > maintainer, I believe there may be a misunderstanding. > Before sending the series, I ran get_maintainer.pl on the patch set and > included all recipients reported by the script, including: > > Nicolas Frattaroli <frattaroli.nicolas@gmail.com> > > along with the relevant mailing lists. > > ------------------------------------------ > phuc@phuc-desktop:~/work/linux$ ls ../patch/sound/rockchip/clean/ > 0000-cover-letter.patch > v2-0000-cover-letter.patch > 0001-ASoC-rockchip-rockchip_i2s-Use-guard-for-spin-locks.patch > v2-0001-ASoC-rockchip-rockchip_i2s-Use-guard-for-spin-loc.patch > 0002-ASoC-rockchip-i2s-tdm-Use-guard-for-spin-locks.patch > v2-0002-ASoC-rockchip-i2s-tdm-Use-guard-for-spin-locks.patch > 0003-ASoC-rockchip-rockchip_sai-Use-guard-for-spin-locks.patch > v2-0003-ASoC-rockchip-rockchip_sai-Use-guard-for-spin-loc.patch > phuc@phuc-desktop:~/work/linux$ > phuc@phuc-desktop:~/work/linux$ > phuc@phuc-desktop:~/work/linux$ > phuc@phuc-desktop:~/work/linux$ ./scripts/get_maintainer.pl > ../patch/sound/rockchip/clean/000*.patch > ./scripts/get_maintainer.pl: file > '../patch/sound/rockchip/clean/0000-cover-letter.patch' doesn't appear > to be a patch. Add -f to options? > Liam Girdwood <lgirdwood@gmail.com> (maintainer:SOUND - SOC LAYER / > DYNAMIC AUDIO POWER MANAGEM...) > Mark Brown <broonie@kernel.org> (maintainer:SOUND - SOC LAYER / > DYNAMIC AUDIO POWER MANAGEM...) > Jaroslav Kysela <perex@perex.cz> (maintainer:SOUND) > Takashi Iwai <tiwai@suse.com> (maintainer:SOUND) > Heiko Stuebner <heiko@sntech.de> (maintainer:ARM/Rockchip SoC support) > Nicolas Frattaroli <frattaroli.nicolas@gmail.com> (maintainer:ROCKCHIP > I2S TDM DRIVER) > linux-sound@vger.kernel.org (open list:SOUND - SOC LAYER / DYNAMIC > AUDIO POWER MANAGEM...) > linux-arm-kernel@lists.infradead.org (moderated list:ARM/Rockchip SoC support) > linux-rockchip@lists.infradead.org (open list:ARM/Rockchip SoC support) > linux-kernel@vger.kernel.org (open list) > SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEMENT (ASoC) status: Supported > phuc@phuc-desktop:~/work/linux$ > phuc@phuc-desktop:~/work/linux$ > --------------------------------------------- $ ./scripts/get_maintainer.pl sound/soc/rockchip/rockchip_sai.c Nicolas Frattaroli <nicolas.frattaroli@collabora.com> (maintainer:ROCKCHIP SAI DRIVER) [...] b4 will also pick this up. Running get_maintainer.pl on a patch with rockchip_sai.* in the diff will also pick it up, see MAINTAINERS. It likely wasn't picked up for you because by passing every patch into it at once, it decided to "deduplicate" my gmail from my work e-mail based on name, which seems like really silly default behaviour for get_maintainers to have. `--no-remove-duplicates` works around this. I should be setting up a forward or checking my gmail at work more often. > > If there are additional maintainers or mailing lists that should receive these > patches but are not currently reported by get_maintainer.pl, please let me know > so I can include them in future submissions. > I would also appreciate avoiding assumptions that the series was generated > by an LLM. The patches were prepared manually and submitted as part of > my ongoing effort to convert lock/unlock patterns to guard()/scoped_guard() > helpers where appropriate. > The series was build-tested before submission, so I do not believe it > is accurate > to describe it as "wasn't even tested". > Whether a particular conversion is worthwhile is certainly open to > technical discussion, > but I do not think it is reasonable to conclude that a patch was AI-generated > simply based on disagreements about the implementation details. > I understand maintainers have different preferences regarding how aggressively > such conversions should be applied, and I am happy to adjust the approach > based on review feedback. If the patch worsens code legibility through excessive indentation and does not make any functional changes, then I do not see the point of it. Build-testing will tell you whether it compiles, but a lot of things in C compile but aren't correct. If you want to try again, do so in a way where you don't have to indent large parts of a function another level, it's possible and the diff will be something I can actually read. Remember that you can place a guard for the spinlock in the middle of a function, and it'll only be picked up then, which should reduce the need for the scoped_guards in this. To avoid having to manually do pm unwind (and therefore require that the spinlock is dropped somehow before function exit), use the runtime pm guard macros as well. That should simplify this immensely. Last but not least, please run things through checkpatch. Even at the default options, it'll warn about the indentation level. With --strict, it'll also tell you about the braces. Personally I just use whatever preset `b4 prep --check` uses, which usually is quite reasonable. > > Best regards, > Phuc > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 0/3] ASoC: rockchip: Use guard() for spin locks @ 2026-06-18 12:16 ` Nicolas Frattaroli 0 siblings, 0 replies; 16+ messages in thread From: Nicolas Frattaroli @ 2026-06-18 12:16 UTC (permalink / raw) To: Bui Duc Phuc Cc: Heiko Stuebner, Liam Girdwood, Mark Brown, Nicolas Frattaroli, Jaroslav Kysela, Takashi Iwai, linux-sound, linux-rockchip, linux-arm-kernel, linux-kernel On Thursday, 18 June 2026 05:16:24 Central European Summer Time Bui Duc Phuc wrote: > Hi Nicolas, > > Thanks for the feedback. > > > FWIW, the SAI patch wasn't sent to the proper maintainer e-mail > > for it which is why I didn't notice this series at all until now, > > and the whole thing is pointless churn that wasn't even tested. > > > > From a cursory glance, whatever LLM was pointed at this and decided > > to make the output my problem also didn't do a good job, the scoped > > indentation of rockchip_sai_runtime_suspend seems pointless because > > it could've been replaced by a pm guard followed by a spinlock guard, > > without an additional level of indentation, and `if (ret == 0) {` is > > not kernel style. > > > > It's not worth the revert but it sucks that I have to now set up a > > new lei search for all the drivers I am supposed to receive mail for > > because vibecoders can't be bothered to run get_maintainers as they > > pad their CV with nonsense like this. > > > > > > > > [1/3] ASoC: rockchip: rockchip_i2s: Use guard() for spin locks > > > https://git.kernel.org/broonie/sound/c/4bda5af16920 > > > [2/3] ASoC: rockchip: i2s-tdm: Use guard() for spin locks > > > https://git.kernel.org/broonie/sound/c/ec22437fc41a > > > [3/3] ASoC: rockchip: rockchip_sai: Use guard() for spin locks > > > https://git.kernel.org/broonie/sound/c/f7fe9f707360 > > > > > Regarding the claim that the Rockchip SAI patch was not sent to the proper > maintainer, I believe there may be a misunderstanding. > Before sending the series, I ran get_maintainer.pl on the patch set and > included all recipients reported by the script, including: > > Nicolas Frattaroli <frattaroli.nicolas@gmail.com> > > along with the relevant mailing lists. > > ------------------------------------------ > phuc@phuc-desktop:~/work/linux$ ls ../patch/sound/rockchip/clean/ > 0000-cover-letter.patch > v2-0000-cover-letter.patch > 0001-ASoC-rockchip-rockchip_i2s-Use-guard-for-spin-locks.patch > v2-0001-ASoC-rockchip-rockchip_i2s-Use-guard-for-spin-loc.patch > 0002-ASoC-rockchip-i2s-tdm-Use-guard-for-spin-locks.patch > v2-0002-ASoC-rockchip-i2s-tdm-Use-guard-for-spin-locks.patch > 0003-ASoC-rockchip-rockchip_sai-Use-guard-for-spin-locks.patch > v2-0003-ASoC-rockchip-rockchip_sai-Use-guard-for-spin-loc.patch > phuc@phuc-desktop:~/work/linux$ > phuc@phuc-desktop:~/work/linux$ > phuc@phuc-desktop:~/work/linux$ > phuc@phuc-desktop:~/work/linux$ ./scripts/get_maintainer.pl > ../patch/sound/rockchip/clean/000*.patch > ./scripts/get_maintainer.pl: file > '../patch/sound/rockchip/clean/0000-cover-letter.patch' doesn't appear > to be a patch. Add -f to options? > Liam Girdwood <lgirdwood@gmail.com> (maintainer:SOUND - SOC LAYER / > DYNAMIC AUDIO POWER MANAGEM...) > Mark Brown <broonie@kernel.org> (maintainer:SOUND - SOC LAYER / > DYNAMIC AUDIO POWER MANAGEM...) > Jaroslav Kysela <perex@perex.cz> (maintainer:SOUND) > Takashi Iwai <tiwai@suse.com> (maintainer:SOUND) > Heiko Stuebner <heiko@sntech.de> (maintainer:ARM/Rockchip SoC support) > Nicolas Frattaroli <frattaroli.nicolas@gmail.com> (maintainer:ROCKCHIP > I2S TDM DRIVER) > linux-sound@vger.kernel.org (open list:SOUND - SOC LAYER / DYNAMIC > AUDIO POWER MANAGEM...) > linux-arm-kernel@lists.infradead.org (moderated list:ARM/Rockchip SoC support) > linux-rockchip@lists.infradead.org (open list:ARM/Rockchip SoC support) > linux-kernel@vger.kernel.org (open list) > SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEMENT (ASoC) status: Supported > phuc@phuc-desktop:~/work/linux$ > phuc@phuc-desktop:~/work/linux$ > --------------------------------------------- $ ./scripts/get_maintainer.pl sound/soc/rockchip/rockchip_sai.c Nicolas Frattaroli <nicolas.frattaroli@collabora.com> (maintainer:ROCKCHIP SAI DRIVER) [...] b4 will also pick this up. Running get_maintainer.pl on a patch with rockchip_sai.* in the diff will also pick it up, see MAINTAINERS. It likely wasn't picked up for you because by passing every patch into it at once, it decided to "deduplicate" my gmail from my work e-mail based on name, which seems like really silly default behaviour for get_maintainers to have. `--no-remove-duplicates` works around this. I should be setting up a forward or checking my gmail at work more often. > > If there are additional maintainers or mailing lists that should receive these > patches but are not currently reported by get_maintainer.pl, please let me know > so I can include them in future submissions. > I would also appreciate avoiding assumptions that the series was generated > by an LLM. The patches were prepared manually and submitted as part of > my ongoing effort to convert lock/unlock patterns to guard()/scoped_guard() > helpers where appropriate. > The series was build-tested before submission, so I do not believe it > is accurate > to describe it as "wasn't even tested". > Whether a particular conversion is worthwhile is certainly open to > technical discussion, > but I do not think it is reasonable to conclude that a patch was AI-generated > simply based on disagreements about the implementation details. > I understand maintainers have different preferences regarding how aggressively > such conversions should be applied, and I am happy to adjust the approach > based on review feedback. If the patch worsens code legibility through excessive indentation and does not make any functional changes, then I do not see the point of it. Build-testing will tell you whether it compiles, but a lot of things in C compile but aren't correct. If you want to try again, do so in a way where you don't have to indent large parts of a function another level, it's possible and the diff will be something I can actually read. Remember that you can place a guard for the spinlock in the middle of a function, and it'll only be picked up then, which should reduce the need for the scoped_guards in this. To avoid having to manually do pm unwind (and therefore require that the spinlock is dropped somehow before function exit), use the runtime pm guard macros as well. That should simplify this immensely. Last but not least, please run things through checkpatch. Even at the default options, it'll warn about the indentation level. With --strict, it'll also tell you about the braces. Personally I just use whatever preset `b4 prep --check` uses, which usually is quite reasonable. > > Best regards, > Phuc > _______________________________________________ Linux-rockchip mailing list Linux-rockchip@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-rockchip ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2026-06-18 12:17 UTC | newest] Thread overview: 16+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-06-04 3:35 [PATCH v2 0/3] ASoC: rockchip: Use guard() for spin locks phucduc.bui 2026-06-04 3:35 ` phucduc.bui 2026-06-04 3:35 ` [PATCH v2 1/3] ASoC: rockchip: rockchip_i2s: " phucduc.bui 2026-06-04 3:35 ` phucduc.bui 2026-06-04 3:35 ` [PATCH v2 2/3] ASoC: rockchip: i2s-tdm: " phucduc.bui 2026-06-04 3:35 ` phucduc.bui 2026-06-04 3:35 ` [PATCH v2 3/3] ASoC: rockchip: rockchip_sai: " phucduc.bui 2026-06-04 3:35 ` phucduc.bui 2026-06-11 19:53 ` [PATCH v2 0/3] ASoC: rockchip: " Mark Brown 2026-06-11 19:53 ` Mark Brown 2026-06-17 11:24 ` Nicolas Frattaroli 2026-06-17 11:24 ` Nicolas Frattaroli 2026-06-18 3:16 ` Bui Duc Phuc 2026-06-18 3:16 ` Bui Duc Phuc 2026-06-18 12:16 ` Nicolas Frattaroli 2026-06-18 12:16 ` Nicolas Frattaroli
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.