[PATCH 0/3] ASoC: renesas: fsi: Fix system hang by adding SPU clock

[PATCH 0/3] ASoC: renesas: fsi: Fix system hang by adding SPU clock

[phucduc.bui]

From: bui duc phuc <phucduc.bui@gmail.com>

Hi all,

The FSI on r8a7740 requires the SPU clock to be enabled
before accessing its internal registers. Without this clock,
register access may lead to a system hang, even when the FSI
functional clock itself is properly enabled.

This series adds support for the SPU clock and updates the
bindings to allow multiple clocks. The driver retrieves the
SPU clock and enables it during DAI startup, then disables
it on shutdown to match the audio stream lifecycle.

The binding is also extended to support additional clocks,
as FSIB may require more clock inputs, while FSIA typically
uses fewer.

This has been tested on r8a7740 (Armadillo800eva) and fixes
system hangs observed during audio playback.

Patch overview:
 [1/3] dt-bindings: sound: renesas,fsi: Add support for multiple clocks
 [2/3] arm: dts: renesas: r8a7740-armadillo800eva: Add spu clock to FSI
 [3/3] ASoC: renesas: fsi: Fix hang by enabling SPU clock

Thanks,
Phuc

bui duc phuc (3):
  dt-bindings: sound: renesas,fsi: Add support for multiple clocks
  arm: dts: renesas: r8a7740-armadillo800eva: Add spu clock to FSI
  ASoC: renesas: fsi: Fix hang by enabling SPU clock

 .../devicetree/bindings/sound/renesas,fsi.yaml     | 12 ++++++++++--
 arch/arm/boot/dts/renesas/r8a7740.dtsi             |  3 ++-
 sound/soc/renesas/fsi.c                            | 14 ++++++++++++++
 3 files changed, 26 insertions(+), 3 deletions(-)

-- 
2.43.0

[PATCH 1/3] dt-bindings: sound: renesas,fsi: Add support for multiple clocks

[phucduc.bui]

From: bui duc phuc <phucduc.bui@gmail.com>

The FSI on r8a7740 requires the SPU clock to be enabled
before accessing its registers.
Without this clock, register access may lead to a system
hang.
Add support for the "spu" clock so it can be managed by
the driver.
The binding is also extended to allow additional clocks,
as FSIB may require more clock inputs, while FSIA
typically uses fewer.

Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
 .../devicetree/bindings/sound/renesas,fsi.yaml       | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/sound/renesas,fsi.yaml b/Documentation/devicetree/bindings/sound/renesas,fsi.yaml
index df91991699a7..225cd8d369bb 100644
--- a/Documentation/devicetree/bindings/sound/renesas,fsi.yaml
+++ b/Documentation/devicetree/bindings/sound/renesas,fsi.yaml
@@ -38,7 +38,11 @@ properties:
     maxItems: 1
 
   clocks:
-    maxItems: 1
+    minItems: 1
+    maxItems: 8
+
+  clock-names:
+    description: List of necessary clock names.
 
   power-domains:
     maxItems: 1
@@ -77,7 +81,11 @@ examples:
             compatible = "renesas,fsi2-r8a7740", "renesas,sh_fsi2";
             reg = <0xfe1f0000 0x400>;
             interrupts = <GIC_SPI 9 0x4>;
-            clocks = <&mstp3_clks R8A7740_CLK_FSI>;
+            clocks = <&mstp3_clks R8A7740_CLK_FSI>, <&spu_clk>,
+                     <&fsia_clk>, <&fsib_clk>, <&fsidiva_clk>,
+                     <&fsidivb_clk>,<&fsiack_clk>,<&fsibck_clk>;
+            clock-names = "fsi", "spu", "icka", "ickb",
+                          "diva", "divb", "xcka", "xckb";
             power-domains = <&pd_a4mp>;
 
             #sound-dai-cells = <1>;
-- 
2.43.0

Re: [PATCH 1/3] dt-bindings: sound: renesas,fsi: Add support for multiple clocks

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 704 bytes --]

On Fri, Apr 03, 2026 at 06:26:53PM +0700, phucduc.bui@gmail.com wrote:

>    clocks:
> -    maxItems: 1
> +    minItems: 1
> +    maxItems: 8
> +
> +  clock-names:
> +    description: List of necessary clock names.

This should list the valid names.  Ideally there'd be some specification
of which clocks are required where, but that might be more trouble than
it's worth.

Please submit patches using subject lines reflecting the style for the
subsystem, this makes it easier for people to identify relevant patches.
Look at what existing commits in the area you're changing are doing and
make sure your subject lines visually resemble what they're doing.
There's no need to resubmit to fix this alone.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 1/3] dt-bindings: sound: renesas,fsi: Add support for multiple clocks

[Krzysztof Kozlowski]

On Fri, Apr 03, 2026 at 06:26:53PM +0700, phucduc.bui@gmail.com wrote:
> From: bui duc phuc <phucduc.bui@gmail.com>
> 
> The FSI on r8a7740 requires the SPU clock to be enabled
> before accessing its registers.
> Without this clock, register access may lead to a system
> hang.
> Add support for the "spu" clock so it can be managed by
> the driver.
> The binding is also extended to allow additional clocks,
> as FSIB may require more clock inputs, while FSIA
> typically uses fewer.

Please wrap commit message according to Linux coding style / submission
process (neither too early nor over the limit):
https://elixir.bootlin.com/linux/v6.4-rc1/source/Documentation/process/submitting-patches.rst#L597

And not after every sentece, BTW.

> Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
> ---
>  .../devicetree/bindings/sound/renesas,fsi.yaml       | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/sound/renesas,fsi.yaml b/Documentation/devicetree/bindings/sound/renesas,fsi.yaml
> index df91991699a7..225cd8d369bb 100644
> --- a/Documentation/devicetree/bindings/sound/renesas,fsi.yaml
> +++ b/Documentation/devicetree/bindings/sound/renesas,fsi.yaml
> @@ -38,7 +38,11 @@ properties:
>      maxItems: 1
>  
>    clocks:
> -    maxItems: 1
> +    minItems: 1
> +    maxItems: 8

Needs valid descriptions.

> +
> +  clock-names:
> +    description: List of necessary clock names.

Instead constrain it. See also writing-bindings, writing-schema or
example-schema documents.

Best regards,
Krzysztof

Re: [PATCH 1/3] dt-bindings: sound: renesas,fsi: Add support for multiple clocks

[Bui Duc Phuc]

Hi Mark, Krzysztof,

Thank you for your reviews. I will fix these in v2:

- Subject Line: Change to ASoC: dt-bindings: renesas,fsi: add support
for multiple clocks to match subsystem style.
- Commit Message: Reformat to 72-75 characters per line and remove
manual line breaks after every sentence.
- YAML Bindings: Properly constrain clocks and clock-names by
following the writing-schema and existing
Renesas sound examples.

I will submit the v2 series shortly.

Best regards,
Phuc

On Sun, Apr 5, 2026 at 2:32 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On Fri, Apr 03, 2026 at 06:26:53PM +0700, phucduc.bui@gmail.com wrote:
> > From: bui duc phuc <phucduc.bui@gmail.com>
> >
> > The FSI on r8a7740 requires the SPU clock to be enabled
> > before accessing its registers.
> > Without this clock, register access may lead to a system
> > hang.
> > Add support for the "spu" clock so it can be managed by
> > the driver.
> > The binding is also extended to allow additional clocks,
> > as FSIB may require more clock inputs, while FSIA
> > typically uses fewer.
>
> Please wrap commit message according to Linux coding style / submission
> process (neither too early nor over the limit):
> https://elixir.bootlin.com/linux/v6.4-rc1/source/Documentation/process/submitting-patches.rst#L597
>
> And not after every sentece, BTW.
>
> > Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
> > ---
> >  .../devicetree/bindings/sound/renesas,fsi.yaml       | 12 ++++++++++--
> >  1 file changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/sound/renesas,fsi.yaml b/Documentation/devicetree/bindings/sound/renesas,fsi.yaml
> > index df91991699a7..225cd8d369bb 100644
> > --- a/Documentation/devicetree/bindings/sound/renesas,fsi.yaml
> > +++ b/Documentation/devicetree/bindings/sound/renesas,fsi.yaml
> > @@ -38,7 +38,11 @@ properties:
> >      maxItems: 1
> >
> >    clocks:
> > -    maxItems: 1
> > +    minItems: 1
> > +    maxItems: 8
>
> Needs valid descriptions.
>
> > +
> > +  clock-names:
> > +    description: List of necessary clock names.
>
> Instead constrain it. See also writing-bindings, writing-schema or
> example-schema documents.
>
> Best regards,
> Krzysztof
>

[PATCH 2/3] arm: dts: renesas: r8a7740-armadillo800eva: Add spu clock to FSI

[phucduc.bui]

From: bui duc phuc <phucduc.bui@gmail.com>

The FSI on r8a7740 requires the SPU clock to be enabled
before accessing its registers.
Without this clock, register access may lead to a system
hang.
Describe the "spu" clock in the FSI node.

Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
 arch/arm/boot/dts/renesas/r8a7740.dtsi | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/renesas/r8a7740.dtsi b/arch/arm/boot/dts/renesas/r8a7740.dtsi
index d13ab86c3ab4..9cae87a1979c 100644
--- a/arch/arm/boot/dts/renesas/r8a7740.dtsi
+++ b/arch/arm/boot/dts/renesas/r8a7740.dtsi
@@ -393,7 +393,8 @@ sh_fsi2: sound@fe1f0000 {
 		compatible = "renesas,fsi2-r8a7740", "renesas,sh_fsi2";
 		reg = <0xfe1f0000 0x400>;
 		interrupts = <GIC_SPI 9 0x4>;
-		clocks = <&mstp3_clks R8A7740_CLK_FSI>;
+		clocks = <&mstp3_clks R8A7740_CLK_FSI>, <&spu_clk>;
+		clock-names = "fsi", "spu";
 		power-domains = <&pd_a4mp>;
 		status = "disabled";
 	};
-- 
2.43.0

[PATCH 3/3] ASoC: renesas: fsi: Fix hang by enabling SPU clock

[phucduc.bui]

From: bui duc phuc <phucduc.bui@gmail.com>

The FSI on r8a7740 requires the SPU clock to be enabled
before accessing its registers.
Without this clock, register access may lead to a system
hang.
Retrieve the "spu" clock in probe and enable it during
DAI startup. Disable the clock on shutdown to match the
audio stream lifecycle.
This ensures safe register access and prevents system
hangs during audio playback.
This is required even if the FSI functional clock is
enabled, as internal units depend on the SPU clock.

Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
 sound/soc/renesas/fsi.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/sound/soc/renesas/fsi.c b/sound/soc/renesas/fsi.c
index 1491c2f2cc96..44bd1c1e6294 100644
--- a/sound/soc/renesas/fsi.c
+++ b/sound/soc/renesas/fsi.c
@@ -292,6 +292,7 @@ struct fsi_master {
 	void __iomem *base;
 	struct fsi_priv fsia;
 	struct fsi_priv fsib;
+	struct clk *clk_spu;
 	const struct fsi_core *core;
 	spinlock_t lock;
 };
@@ -1554,6 +1555,11 @@ static int fsi_dai_startup(struct snd_pcm_substream *substream,
 			   struct snd_soc_dai *dai)
 {
 	struct fsi_priv *fsi = fsi_get_priv(substream);
+	int ret;
+
+	ret = clk_prepare_enable(fsi->master->clk_spu);
+	if (ret)
+		return ret;
 
 	fsi_clk_invalid(fsi);
 
@@ -1566,6 +1572,7 @@ static void fsi_dai_shutdown(struct snd_pcm_substream *substream,
 	struct fsi_priv *fsi = fsi_get_priv(substream);
 
 	fsi_clk_invalid(fsi);
+	clk_disable_unprepare(fsi->master->clk_spu);
 }
 
 static int fsi_dai_trigger(struct snd_pcm_substream *substream, int cmd,
@@ -1963,6 +1970,13 @@ static int fsi_probe(struct platform_device *pdev)
 	master->core		= core;
 	spin_lock_init(&master->lock);
 
+	/* SPU clock is required for FSI register access */
+	master->clk_spu = devm_clk_get(&pdev->dev, "spu");
+	if (IS_ERR(master->clk_spu)) {
+		dev_err(&pdev->dev, "Failed to get spu clock\n");
+		return PTR_ERR(master->clk_spu);
+	}
+
 	/* FSI A setting */
 	fsi		= &master->fsia;
 	fsi->base	= master->base;
-- 
2.43.0

Re: [PATCH 3/3] ASoC: renesas: fsi: Fix hang by enabling SPU clock

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1235 bytes --]

On Fri, Apr 03, 2026 at 06:26:55PM +0700, phucduc.bui@gmail.com wrote:

> @@ -1554,6 +1555,11 @@ static int fsi_dai_startup(struct snd_pcm_substream *substream,
>  			   struct snd_soc_dai *dai)
>  {
>  	struct fsi_priv *fsi = fsi_get_priv(substream);
> +	int ret;
> +
> +	ret = clk_prepare_enable(fsi->master->clk_spu);
> +	if (ret)
> +		return ret;
>  

Should we also be managing the clock during system suspend, or if the
power consumption doesn't really matter should we just keep it enabled
all the time and not worry about starting and stopping it?

> +	/* SPU clock is required for FSI register access */
> +	master->clk_spu = devm_clk_get(&pdev->dev, "spu");
> +	if (IS_ERR(master->clk_spu)) {
> +		dev_err(&pdev->dev, "Failed to get spu clock\n");
> +		return PTR_ERR(master->clk_spu);
> +	}
> +

This is going to unconditionally require a clock called "spu" on all
devices using this driver, not just the one SoC you mentioned as
requiring it.  Presumably this worked at least somewhere (possibly the
clock is always on, or they're just lucky that something else enables
it) and this will cause regressions for those platforms?

This should either (ideally) be conditional, or use _optional.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 3/3] ASoC: renesas: fsi: Fix hang by enabling SPU clock

[Kuninori Morimoto]

Hi

Thank you for the patch

> From: bui duc phuc <phucduc.bui@gmail.com>
> 
> The FSI on r8a7740 requires the SPU clock to be enabled
> before accessing its registers.
> Without this clock, register access may lead to a system
> hang.
> Retrieve the "spu" clock in probe and enable it during
> DAI startup. Disable the clock on shutdown to match the
> audio stream lifecycle.
> This ensures safe register access and prevents system
> hangs during audio playback.
> This is required even if the FSI functional clock is
> enabled, as internal units depend on the SPU clock.
> 
> Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
> ---
(snip)
> @@ -1554,6 +1555,11 @@ static int fsi_dai_startup(struct snd_pcm_substream *substream,
>                            struct snd_soc_dai *dai)
>  {
>         struct fsi_priv *fsi = fsi_get_priv(substream);
> +       int ret;
> +
> +       ret = clk_prepare_enable(fsi->master->clk_spu);
> +       if (ret)
> +               return ret;
> 
>         fsi_clk_invalid(fsi);

If it is needed for register access, you need to call it on
fsi_hw_startup/shutdown() which cares suspend/resume too.

And I guess it need to count user, because we have FSI-A / FSI-B ?

> @@ -1963,6 +1970,13 @@ static int fsi_probe(struct platform_device *pdev)
>         master->core            = core;
>         spin_lock_init(&master->lock);
> 
> +       /* SPU clock is required for FSI register access */
> +       master->clk_spu = devm_clk_get(&pdev->dev, "spu");
> +       if (IS_ERR(master->clk_spu)) {
> +               dev_err(&pdev->dev, "Failed to get spu clock\n");
> +               return PTR_ERR(master->clk_spu);
> +       }

As Mark mentioned, it should be optional. Otherwise it breaks compatibility.
And we already have fsi_clk_init() for clock initialize.
spu should be handled in it.

Now, it is called if clock master (A.

(A)	if (fsi_is_clk_master(fsi)) {
		if (fsi->clk_cpg)
			fsi_clk_init(dev, fsi, 0, 1, 1,
				     fsi_clk_set_rate_cpg);
		else
			fsi_clk_init(dev, fsi, 1, 1, 0,
				     fsi_clk_set_rate_external);
	}

I think it (A) can be checked inside fsi_clk_init().
fsi_clk_init() is now called when .set_fmt, but it can be called
at _probe() timing ?

Thank you for your help !!

Best regards
---
Kuninori Morimoto

Re: [PATCH 3/3] ASoC: renesas: fsi: Fix hang by enabling SPU clock

[Bui Duc Phuc]

Hi Morimoto-san, Mark,

Thank you for your review.

> If it is needed for register access,

Yes, enabling this clock is essential as it functions as a bus bridge clock.
Currently, the SPU clock is still enabled by the bootloader. In legacy
kernels (v4.2 and earlier) using the Armadillo board-file/defconfig, this
clock remained active after boot, allowing the FSI to function correctly.
However, after migrating to a full Device Tree (DTS) implementation,
the kernel's unused clock cleanup mechanism disables the SPU clock
because it isn't explicitly claimed. This leads to a system hang every
time aplay is executed, as the FSI registers become inaccessible
without this clock.

> you need to call it on
> fsi_hw_startup/shutdown() which cares suspend/resume too.

I previously attempted to manage the clock within fsi_hw_startup/
shutdown, but the system would hang when stopping aplay
(e.g., via Ctrl+C). This happens because certain cleanup operations,
such as fsi_irq_disable(), are performed after fsi_hw_shutdown()
finishes. These operations require register access, which triggers a
system hang if the SPU clock has already been disabled. Therefore,
I moved the clock management to fsi_dai_startup/shutdown to ensure
the clock remains active throughout the entire lifecycle of the stream.

Furthermore, my testing shows that using dai_startup/shutdown
eliminates the need for explicit Suspend/Resume handling for this clock.
Since the ALSA framework typically invokes the hw_ callbacks during
power management transitions rather than the dai_ ones, the SPU clock
state remains stable, preventing any illegal register access during
these transitions.

> As Mark mentioned, it should be optional.
> Otherwise it breaks compatibility.

You are right. I will implement it this way in v2.

> And we already have fsi_clk_init() for clock initialize.
> spu should be handled in it.

> Now, it is called if clock master (A.

> (A)     if (fsi_is_clk_master(fsi)) {
>                 if (fsi->clk_cpg)
>                         fsi_clk_init(dev, fsi, 0, 1, 1,
>                                      fsi_clk_set_rate_cpg);
>                 else
>                         fsi_clk_init(dev, fsi, 1, 1, 0,
>                                      fsi_clk_set_rate_external);
>         }

You are right. Currently, our FSIA is configured as a slave,
so it never executes the clk_init() function.

> I think it (A) can be checked inside fsi_clk_init().
> fsi_clk_init() is now called when .set_fmt, but it can be called
> at _probe() timing ?

Yes. I can handle the implementation/coding side of this.

> Should we also be managing the clock during system suspend, or if the
> power consumption doesn't really matter should we just keep it enabled
> all the time and not worry about starting and stopping it?

Regarding the SPU clock management, I haven't measured the exact
power consumption of this block yet. However, to keep the code simple
and ensure maximum stability for register access (avoiding system
hangs during cleanup), I am open to enabling it once in fsi_probe()
if you find the dynamic management in dai_startup/shutdown
unnecessary.

> This is going to unconditionally require a clock called "spu" on all
> devices using this driver, not just the one SoC you mentioned as
> requiring it.  Presumably this worked at least somewhere (possibly the
> clock is always on, or they're just lucky that something else enables
> it) and this will cause regressions for those platforms?
> This should either (ideally) be conditional, or use _optional.

Thank you for your suggestion. I will switch to using
devm_clk_get_optional() in the v2

Best regards,
Phuc

On Mon, Apr 6, 2026 at 6:52 AM Kuninori Morimoto
<kuninori.morimoto.gx@renesas.com> wrote:
>
>
> Hi
>
> Thank you for the patch
>
> > From: bui duc phuc <phucduc.bui@gmail.com>
> >
> > The FSI on r8a7740 requires the SPU clock to be enabled
> > before accessing its registers.
> > Without this clock, register access may lead to a system
> > hang.
> > Retrieve the "spu" clock in probe and enable it during
> > DAI startup. Disable the clock on shutdown to match the
> > audio stream lifecycle.
> > This ensures safe register access and prevents system
> > hangs during audio playback.
> > This is required even if the FSI functional clock is
> > enabled, as internal units depend on the SPU clock.
> >
> > Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
> > ---
> (snip)
> > @@ -1554,6 +1555,11 @@ static int fsi_dai_startup(struct snd_pcm_substream *substream,
> >                            struct snd_soc_dai *dai)
> >  {
> >         struct fsi_priv *fsi = fsi_get_priv(substream);
> > +       int ret;
> > +
> > +       ret = clk_prepare_enable(fsi->master->clk_spu);
> > +       if (ret)
> > +               return ret;
> >
> >         fsi_clk_invalid(fsi);
>
> If it is needed for register access, you need to call it on
> fsi_hw_startup/shutdown() which cares suspend/resume too.
>
> And I guess it need to count user, because we have FSI-A / FSI-B ?
>
> > @@ -1963,6 +1970,13 @@ static int fsi_probe(struct platform_device *pdev)
> >         master->core            = core;
> >         spin_lock_init(&master->lock);
> >
> > +       /* SPU clock is required for FSI register access */
> > +       master->clk_spu = devm_clk_get(&pdev->dev, "spu");
> > +       if (IS_ERR(master->clk_spu)) {
> > +               dev_err(&pdev->dev, "Failed to get spu clock\n");
> > +               return PTR_ERR(master->clk_spu);
> > +       }
>
> As Mark mentioned, it should be optional. Otherwise it breaks compatibility.
> And we already have fsi_clk_init() for clock initialize.
> spu should be handled in it.
>
> Now, it is called if clock master (A.
>
> (A)     if (fsi_is_clk_master(fsi)) {
>                 if (fsi->clk_cpg)
>                         fsi_clk_init(dev, fsi, 0, 1, 1,
>                                      fsi_clk_set_rate_cpg);
>                 else
>                         fsi_clk_init(dev, fsi, 1, 1, 0,
>                                      fsi_clk_set_rate_external);
>         }
>
> I think it (A) can be checked inside fsi_clk_init().
> fsi_clk_init() is now called when .set_fmt, but it can be called
> at _probe() timing ?
>
> Thank you for your help !!
>
> Best regards
> ---
> Kuninori Morimoto