All of lore.kernel.org
 help / color / mirror / Atom feed
From: Rob Herring <robh@kernel.org>
To: Richard Fitzgerald <rf@opensource.cirrus.com>
Cc: devicetree@vger.kernel.org, alsa-devel@alsa-project.org,
	patches@opensource.cirrus.com, linux-kernel@vger.kernel.org,
	broonie@kernel.org, linux-arm-kernel@lists.infradead.org,
	nsaenzjulienne@suse.de, linux-rpi-kernel@lists.infradead.org
Subject: Re: [PATCH v2 2/7] ASoC: audio-graph-card: Add plls and sysclks DT bindings
Date: Mon, 26 Oct 2020 08:27:04 -0500	[thread overview]
Message-ID: <20201026132704.GA19204@bogus> (raw)
In-Reply-To: <20201016173541.21180-3-rf@opensource.cirrus.com>

On Fri, Oct 16, 2020 at 06:35:36PM +0100, Richard Fitzgerald wrote:
> This adds the two new properties 'plls' and 'sysclks' to the dt bindings.
> These add the ability to set values that will be
> passed to snd_soc_component_set_sysclk() and snd_soc_component_set_pll().

I worry this looks like Linux implementation details leaking into the 
binding.

> 
> Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
> ---
>  .../bindings/sound/audio-graph-card.txt       | 44 +++++++++++++++++++
>  1 file changed, 44 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/sound/audio-graph-card.txt b/Documentation/devicetree/bindings/sound/audio-graph-card.txt
> index d5f6919a2d69..59bbd5b55b59 100644
> --- a/Documentation/devicetree/bindings/sound/audio-graph-card.txt
> +++ b/Documentation/devicetree/bindings/sound/audio-graph-card.txt
> @@ -32,6 +32,19 @@ Required properties:
>  Optional properties:
>  - pa-gpios: GPIO used to control external amplifier.
>  
> +- plls: A list of component pll settings that will be applied with
> +      snd_soc_component_set_pll. Each entry is a phandle to the node of the
> +      codec or cpu component, followed by the four arguments id, source,
> +      frequency_in, frequency_out. Multiple entries can have the same phandle
> +      so that several plls can be set in the same component.

Where do the values of id and source come from?

> +
> +- sysclks: A list of component sysclk settings that will be applied with
> +      snd_soc_component_set_sysclk. Each entry is a phandle to the node of
> +      the codec or cpu component, followed by the four arguments id, source,
> +      frequency, direction. Direction is 0 if the clock is an input, 1 if it
> +      is an output. Multiple entries can have the same phandle so that several
> +      clocks can be set in the same component.

Are these really common properties? They seem kind of Cirrus specific 
and perhaps should be located in the codec node(s).

> +
>  -----------------------
>  Example: Single DAI case
>  -----------------------
> @@ -335,3 +348,34 @@ Example: Multi DAI with DPCM
>  			};
>  		};
>  	};
> +
> +-----------------------
> +Example: Set component sysclks and PLLs
> +-----------------------
> +
> +	sound {
> +		compatible = "audio-graph-card";
> +
> +		sysclks = <
> +			&cs47l15 1 4 98304000 0
> +			&cs47l15 8 4 147456000 0
> +		>;
> +		plls = <
> +			&cs47l15 1 0 24576000 98304000
> +		>;
> +
> +		dais = <&cpu_i2s_port>;
> +	};
> +
> +	cs47l15: codec@0 {
> +		...
> +		ports {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			cs47l15_aif1_port: port@0 {
> +				reg = <0>;
> +				cs47l15_aif1: endpoint {
> +					remote-endpoint = <&cpu_i2s_endpoint>;
> +				};
> +			};
> +	};
> -- 
> 2.20.1
> 

WARNING: multiple messages have this Message-ID (diff)
From: Rob Herring <robh@kernel.org>
To: Richard Fitzgerald <rf@opensource.cirrus.com>
Cc: devicetree@vger.kernel.org, alsa-devel@alsa-project.org,
	patches@opensource.cirrus.com, linux-kernel@vger.kernel.org,
	broonie@kernel.org, linux-arm-kernel@lists.infradead.org,
	nsaenzjulienne@suse.de, linux-rpi-kernel@lists.infradead.org
Subject: Re: [PATCH v2 2/7] ASoC: audio-graph-card: Add plls and sysclks DT bindings
Date: Mon, 26 Oct 2020 08:27:04 -0500	[thread overview]
Message-ID: <20201026132704.GA19204@bogus> (raw)
In-Reply-To: <20201016173541.21180-3-rf@opensource.cirrus.com>

On Fri, Oct 16, 2020 at 06:35:36PM +0100, Richard Fitzgerald wrote:
> This adds the two new properties 'plls' and 'sysclks' to the dt bindings.
> These add the ability to set values that will be
> passed to snd_soc_component_set_sysclk() and snd_soc_component_set_pll().

I worry this looks like Linux implementation details leaking into the 
binding.

> 
> Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
> ---
>  .../bindings/sound/audio-graph-card.txt       | 44 +++++++++++++++++++
>  1 file changed, 44 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/sound/audio-graph-card.txt b/Documentation/devicetree/bindings/sound/audio-graph-card.txt
> index d5f6919a2d69..59bbd5b55b59 100644
> --- a/Documentation/devicetree/bindings/sound/audio-graph-card.txt
> +++ b/Documentation/devicetree/bindings/sound/audio-graph-card.txt
> @@ -32,6 +32,19 @@ Required properties:
>  Optional properties:
>  - pa-gpios: GPIO used to control external amplifier.
>  
> +- plls: A list of component pll settings that will be applied with
> +      snd_soc_component_set_pll. Each entry is a phandle to the node of the
> +      codec or cpu component, followed by the four arguments id, source,
> +      frequency_in, frequency_out. Multiple entries can have the same phandle
> +      so that several plls can be set in the same component.

Where do the values of id and source come from?

> +
> +- sysclks: A list of component sysclk settings that will be applied with
> +      snd_soc_component_set_sysclk. Each entry is a phandle to the node of
> +      the codec or cpu component, followed by the four arguments id, source,
> +      frequency, direction. Direction is 0 if the clock is an input, 1 if it
> +      is an output. Multiple entries can have the same phandle so that several
> +      clocks can be set in the same component.

Are these really common properties? They seem kind of Cirrus specific 
and perhaps should be located in the codec node(s).

> +
>  -----------------------
>  Example: Single DAI case
>  -----------------------
> @@ -335,3 +348,34 @@ Example: Multi DAI with DPCM
>  			};
>  		};
>  	};
> +
> +-----------------------
> +Example: Set component sysclks and PLLs
> +-----------------------
> +
> +	sound {
> +		compatible = "audio-graph-card";
> +
> +		sysclks = <
> +			&cs47l15 1 4 98304000 0
> +			&cs47l15 8 4 147456000 0
> +		>;
> +		plls = <
> +			&cs47l15 1 0 24576000 98304000
> +		>;
> +
> +		dais = <&cpu_i2s_port>;
> +	};
> +
> +	cs47l15: codec@0 {
> +		...
> +		ports {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			cs47l15_aif1_port: port@0 {
> +				reg = <0>;
> +				cs47l15_aif1: endpoint {
> +					remote-endpoint = <&cpu_i2s_endpoint>;
> +				};
> +			};
> +	};
> -- 
> 2.20.1
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

WARNING: multiple messages have this Message-ID (diff)
From: Rob Herring <robh@kernel.org>
To: Richard Fitzgerald <rf@opensource.cirrus.com>
Cc: broonie@kernel.org, nsaenzjulienne@suse.de,
	patches@opensource.cirrus.com, alsa-devel@alsa-project.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-rpi-kernel@lists.infradead.org
Subject: Re: [PATCH v2 2/7] ASoC: audio-graph-card: Add plls and sysclks DT bindings
Date: Mon, 26 Oct 2020 08:27:04 -0500	[thread overview]
Message-ID: <20201026132704.GA19204@bogus> (raw)
In-Reply-To: <20201016173541.21180-3-rf@opensource.cirrus.com>

On Fri, Oct 16, 2020 at 06:35:36PM +0100, Richard Fitzgerald wrote:
> This adds the two new properties 'plls' and 'sysclks' to the dt bindings.
> These add the ability to set values that will be
> passed to snd_soc_component_set_sysclk() and snd_soc_component_set_pll().

I worry this looks like Linux implementation details leaking into the 
binding.

> 
> Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
> ---
>  .../bindings/sound/audio-graph-card.txt       | 44 +++++++++++++++++++
>  1 file changed, 44 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/sound/audio-graph-card.txt b/Documentation/devicetree/bindings/sound/audio-graph-card.txt
> index d5f6919a2d69..59bbd5b55b59 100644
> --- a/Documentation/devicetree/bindings/sound/audio-graph-card.txt
> +++ b/Documentation/devicetree/bindings/sound/audio-graph-card.txt
> @@ -32,6 +32,19 @@ Required properties:
>  Optional properties:
>  - pa-gpios: GPIO used to control external amplifier.
>  
> +- plls: A list of component pll settings that will be applied with
> +      snd_soc_component_set_pll. Each entry is a phandle to the node of the
> +      codec or cpu component, followed by the four arguments id, source,
> +      frequency_in, frequency_out. Multiple entries can have the same phandle
> +      so that several plls can be set in the same component.

Where do the values of id and source come from?

> +
> +- sysclks: A list of component sysclk settings that will be applied with
> +      snd_soc_component_set_sysclk. Each entry is a phandle to the node of
> +      the codec or cpu component, followed by the four arguments id, source,
> +      frequency, direction. Direction is 0 if the clock is an input, 1 if it
> +      is an output. Multiple entries can have the same phandle so that several
> +      clocks can be set in the same component.

Are these really common properties? They seem kind of Cirrus specific 
and perhaps should be located in the codec node(s).

> +
>  -----------------------
>  Example: Single DAI case
>  -----------------------
> @@ -335,3 +348,34 @@ Example: Multi DAI with DPCM
>  			};
>  		};
>  	};
> +
> +-----------------------
> +Example: Set component sysclks and PLLs
> +-----------------------
> +
> +	sound {
> +		compatible = "audio-graph-card";
> +
> +		sysclks = <
> +			&cs47l15 1 4 98304000 0
> +			&cs47l15 8 4 147456000 0
> +		>;
> +		plls = <
> +			&cs47l15 1 0 24576000 98304000
> +		>;
> +
> +		dais = <&cpu_i2s_port>;
> +	};
> +
> +	cs47l15: codec@0 {
> +		...
> +		ports {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			cs47l15_aif1_port: port@0 {
> +				reg = <0>;
> +				cs47l15_aif1: endpoint {
> +					remote-endpoint = <&cpu_i2s_endpoint>;
> +				};
> +			};
> +	};
> -- 
> 2.20.1
> 

  reply	other threads:[~2020-10-26 13:28 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-16 17:35 [PATCH v2 0/7] Add support for Rpi4b + Cirrus Lochnagar2 and CS47L15 Richard Fitzgerald
2020-10-16 17:35 ` Richard Fitzgerald
2020-10-16 17:35 ` Richard Fitzgerald
2020-10-16 17:35 ` [PATCH v2 1/7] of: base: Add of_count_phandle_with_fixed_args() Richard Fitzgerald
2020-10-16 17:35   ` Richard Fitzgerald
2020-10-16 17:35   ` Richard Fitzgerald
2020-10-26 13:19   ` Rob Herring
2020-10-26 13:19     ` Rob Herring
2020-10-26 13:19     ` Rob Herring
2020-10-16 17:35 ` [PATCH v2 2/7] ASoC: audio-graph-card: Add plls and sysclks DT bindings Richard Fitzgerald
2020-10-16 17:35   ` Richard Fitzgerald
2020-10-16 17:35   ` Richard Fitzgerald
2020-10-26 13:27   ` Rob Herring [this message]
2020-10-26 13:27     ` Rob Herring
2020-10-26 13:27     ` Rob Herring
2020-10-26 13:38     ` Mark Brown
2020-10-26 13:38       ` Mark Brown
2020-10-26 13:38       ` Mark Brown
2020-11-02 11:48     ` Richard Fitzgerald
2020-11-02 11:48       ` Richard Fitzgerald
2020-11-02 11:48       ` Richard Fitzgerald
2020-10-16 17:35 ` [PATCH v2 3/7] ASoC: audio-graph-card: Support setting component plls and sysclks Richard Fitzgerald
2020-10-16 17:35   ` Richard Fitzgerald
2020-10-16 17:35   ` Richard Fitzgerald
2020-10-16 17:35 ` [PATCH v2 4/7] ASoC: arizona: Allow codecs to be selected from kernel config Richard Fitzgerald
2020-10-16 17:35   ` Richard Fitzgerald
2020-10-16 17:35   ` Richard Fitzgerald
2020-10-16 17:35 ` [PATCH v2 5/7] ASoC: madera: " Richard Fitzgerald
2020-10-16 17:35   ` Richard Fitzgerald
2020-10-16 17:35   ` Richard Fitzgerald
2020-10-16 17:35 ` [PATCH v2 6/7] ARM: dts: Add dts for RPi4b + Cirrus Logic Lochnagar2 + CS47L15 Richard Fitzgerald
2020-10-16 17:35   ` Richard Fitzgerald
2020-10-16 17:35   ` Richard Fitzgerald
2020-10-16 17:35 ` [PATCH v2 7/7] MAINTAINERS: Add dts for Cirrus Logic Lochnagar on RPi4 Richard Fitzgerald
2020-10-16 17:35   ` Richard Fitzgerald
2020-10-16 17:35   ` Richard Fitzgerald

Reply instructions:

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

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

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

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

  git send-email \
    --in-reply-to=20201026132704.GA19204@bogus \
    --to=robh@kernel.org \
    --cc=alsa-devel@alsa-project.org \
    --cc=broonie@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rpi-kernel@lists.infradead.org \
    --cc=nsaenzjulienne@suse.de \
    --cc=patches@opensource.cirrus.com \
    --cc=rf@opensource.cirrus.com \
    /path/to/YOUR_REPLY

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

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