diff mbox series

[PATCHv4,8/9] ASoC: fsl-asoc-card: add DT property "cpu-system-clock-direction-out"

Message ID 20240515135411.343333-9-elinor.montmasson@savoirfairelinux.com
State New
Headers show
Series [PATCHv4,1/9] ASoC: fsl-asoc-card: add support for dai links with multiple codecs | expand

Commit Message

Elinor Montmasson May 15, 2024, 1:54 p.m. UTC
Add new optional DT property "cpu-system-clock-direction-out" to set
sysclk direction as "out" for the CPU DAI when using the generic codec.
It is set for both Tx and Rx.
If not set, the direction is "in".
The way the direction value is used is up to the CPU DAI driver
implementation.

Signed-off-by: Elinor Montmasson <elinor.montmasson@savoirfairelinux.com>
---
 sound/soc/fsl/fsl-asoc-card.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Mark Brown May 16, 2024, 12:18 p.m. UTC | #1
On Wed, May 15, 2024 at 03:54:10PM +0200, Elinor Montmasson wrote:
> Add new optional DT property "cpu-system-clock-direction-out" to set
> sysclk direction as "out" for the CPU DAI when using the generic codec.
> It is set for both Tx and Rx.
> If not set, the direction is "in".
> The way the direction value is used is up to the CPU DAI driver
> implementation.

This feels like we should be using the clock bindings to specify the
clock input of whatever is using the output from the SoC, though that's
a lot more work.
Elinor Montmasson May 17, 2024, 9:05 a.m. UTC | #2
From: "Mark Brown" <broonie@kernel.org>
Sent: Thursday, 16 May, 2024 14:18:00
> On Wed, May 15, 2024 at 03:54:10PM +0200, Elinor Montmasson wrote:
>> Add new optional DT property "cpu-system-clock-direction-out" to set
>> sysclk direction as "out" for the CPU DAI when using the generic codec.
>> It is set for both Tx and Rx.
>> If not set, the direction is "in".
>> The way the direction value is used is up to the CPU DAI driver
>> implementation.
> 
> This feels like we should be using the clock bindings to specify the
> clock input of whatever is using the output from the SoC, though that's
> a lot more work.

Similarly to patch 7/9, I exposed this parameter because the driver has it, and
because there might be CPU DAIs needing this parameter.
Otherwise the cpu sysclk direction will always be IN as a default.
This parameter could be needed for cases with CPU DAIs, such as an SAI,
which can provide or consume Tx/Rx clocks.
For these devices I know the sysclk direction should correspond to
what was set in the dai format.

This new compatible is intended to be used when there is no codec
device/driver. There is technically no codec device/driver for which
the clock input can be set.

Is it a bad idea to allow setting the cpu sysclk direction only ?
Should the compatible be limited to use-cases where the cpu sysclk
direction cannot be set by the machine driver ?

Best regards,
Elinor Montmasson
Mark Brown May 17, 2024, 11:06 a.m. UTC | #3
On Fri, May 17, 2024 at 05:05:38AM -0400, Elinor Montmasson wrote:

> This new compatible is intended to be used when there is no codec
> device/driver. There is technically no codec device/driver for which
> the clock input can be set.

This is obviously not true, there clearly is a driver.

> Is it a bad idea to allow setting the cpu sysclk direction only ?
> Should the compatible be limited to use-cases where the cpu sysclk
> direction cannot be set by the machine driver ?

When I said "this should use the clock bindings" I meant that we should
use the clock bindings for configuration here.
Elinor Montmasson May 31, 2024, 12:47 p.m. UTC | #4
From: "Mark Brown" <broonie@kernel.org>
Sent: Friday, 17 May, 2024 13:06:03
> On Fri, May 17, 2024 at 05:05:38AM -0400, Elinor Montmasson wrote:
> 
>> This new compatible is intended to be used when there is no codec
>> device/driver. There is technically no codec device/driver for which
>> the clock input can be set.
> 
> This is obviously not true, there clearly is a driver.
> 
>> Is it a bad idea to allow setting the cpu sysclk direction only ?
>> Should the compatible be limited to use-cases where the cpu sysclk
>> direction cannot be set by the machine driver ?
> 
> When I said "this should use the clock bindings" I meant that we should
> use the clock bindings for configuration here.

As far I as know, it's not possible to set the direction with
the clock bindings, but maybe there is and I missed something ?
Mark Brown May 31, 2024, 1:09 p.m. UTC | #5
On Fri, May 31, 2024 at 08:47:22AM -0400, Elinor Montmasson wrote:

> > When I said "this should use the clock bindings" I meant that we should
> > use the clock bindings for configuration here.

> As far I as know, it's not possible to set the direction with
> the clock bindings, but maybe there is and I missed something ?

If a given clock has an input configured then it can't function as an
output and vice versa.
diff mbox series

Patch

diff --git a/sound/soc/fsl/fsl-asoc-card.c b/sound/soc/fsl/fsl-asoc-card.c
index c7fc9c16f761..f3fc2b29c92f 100644
--- a/sound/soc/fsl/fsl-asoc-card.c
+++ b/sound/soc/fsl/fsl-asoc-card.c
@@ -760,6 +760,10 @@  static int fsl_asoc_card_probe(struct platform_device *pdev)
 			priv->cpu_priv.sysclk_freq[RX] = priv->cpu_priv.sysclk_freq[TX];
 			clk_put(cpu_sysclk);
 		}
+		priv->cpu_priv.sysclk_dir[TX] =
+			of_property_read_bool(np, "cpu-system-clock-direction-out") ?
+			SND_SOC_CLOCK_OUT : SND_SOC_CLOCK_IN;
+		priv->cpu_priv.sysclk_dir[RX] = priv->cpu_priv.sysclk_dir[TX];
 	} else {
 		dev_err(&pdev->dev, "unknown Device Tree compatible\n");
 		ret = -EINVAL;