[PATCH v2 0/3] ASoC: wm8782: Allow higher audio rates

[PATCH v2 0/3] ASoC: wm8782: Allow higher audio rates

[John Watts]

The wm8782 supports higher audio rates than just 48kHz. This is
configured by setting the FSAMPEN pin on the codec chip.

This patch series introduces the 'wlf,fsampen' device tree property
to indicate the pin status and control the maximum rate available
when using the codec.

v1 -> v2:
- Switched from max-rate property to wlf,fsampen property
- Clarified property is optional, not required

John Watts (3):
  ASoC: wm8782: Handle maximum audio rate at runtime
  ASoC: wm8782: Use wlf,fsampen device tree property
  ASoC: dt-bindings: wlf,wm8782: Add wlf,fsampen property

 .../devicetree/bindings/sound/wm8782.txt      |  5 ++
 sound/soc/codecs/wm8782.c                     | 66 +++++++++++++++----
 2 files changed, 58 insertions(+), 13 deletions(-)

-- 
2.42.0

[PATCH v2 1/3] ASoC: wm8782: Handle maximum audio rate at runtime

[John Watts]

The wm8782 supports up to 192kHz audio when pins are set correctly.
Instead of hardcoding which rates are supported enable them all
then refer to a max_rate variable at runtime.

Signed-off-by: John Watts <contact@jookia.org>
---
 sound/soc/codecs/wm8782.c | 45 ++++++++++++++++++++++++++++-----------
 1 file changed, 33 insertions(+), 12 deletions(-)

diff --git a/sound/soc/codecs/wm8782.c b/sound/soc/codecs/wm8782.c
index 95ff4339d103..63ab63f3189a 100644
--- a/sound/soc/codecs/wm8782.c
+++ b/sound/soc/codecs/wm8782.c
@@ -23,6 +23,30 @@
 #include <sound/initval.h>
 #include <sound/soc.h>
 
+/* regulator power supply names */
+static const char *supply_names[] = {
+	"Vdda", /* analog supply, 2.7V - 3.6V */
+	"Vdd",  /* digital supply, 2.7V - 5.5V */
+};
+
+struct wm8782_priv {
+	struct regulator_bulk_data supplies[ARRAY_SIZE(supply_names)];
+	int max_rate;
+};
+
+static int wm8782_dai_hw_params(struct snd_pcm_substream *component,
+			    struct snd_pcm_hw_params *params,
+			    struct snd_soc_dai *dai)
+{
+	struct wm8782_priv *priv =
+		snd_soc_component_get_drvdata(dai->component);
+
+	if (params_rate(params) > priv->max_rate)
+		return -EINVAL;
+
+	return 0;
+}
+
 static const struct snd_soc_dapm_widget wm8782_dapm_widgets[] = {
 SND_SOC_DAPM_INPUT("AINL"),
 SND_SOC_DAPM_INPUT("AINR"),
@@ -33,28 +57,22 @@ static const struct snd_soc_dapm_route wm8782_dapm_routes[] = {
 	{ "Capture", NULL, "AINR" },
 };
 
+static const struct snd_soc_dai_ops wm8782_dai_ops = {
+	.hw_params = &wm8782_dai_hw_params,
+};
+
 static struct snd_soc_dai_driver wm8782_dai = {
 	.name = "wm8782",
 	.capture = {
 		.stream_name = "Capture",
 		.channels_min = 2,
 		.channels_max = 2,
-		/* For configurations with FSAMPEN=0 */
-		.rates = SNDRV_PCM_RATE_8000_48000,
+		.rates = SNDRV_PCM_RATE_8000_192000,
 		.formats = SNDRV_PCM_FMTBIT_S16_LE |
 			   SNDRV_PCM_FMTBIT_S20_3LE |
 			   SNDRV_PCM_FMTBIT_S24_LE,
 	},
-};
-
-/* regulator power supply names */
-static const char *supply_names[] = {
-	"Vdda", /* analog supply, 2.7V - 3.6V */
-	"Vdd",  /* digital supply, 2.7V - 5.5V */
-};
-
-struct wm8782_priv {
-	struct regulator_bulk_data supplies[ARRAY_SIZE(supply_names)];
+	.ops = &wm8782_dai_ops,
 };
 
 static int wm8782_soc_probe(struct snd_soc_component *component)
@@ -121,6 +139,9 @@ static int wm8782_probe(struct platform_device *pdev)
 	if (ret < 0)
 		return ret;
 
+	/* For configurations with FSAMPEN=0 */
+	priv->max_rate = 48000;
+
 	return devm_snd_soc_register_component(&pdev->dev,
 			&soc_component_dev_wm8782, &wm8782_dai, 1);
 }
-- 
2.42.0

Re: [PATCH v2 1/3] ASoC: wm8782: Handle maximum audio rate at runtime

[Charles Keepax]

On Thu, Sep 14, 2023 at 03:15:50AM +1000, John Watts wrote:
> The wm8782 supports up to 192kHz audio when pins are set correctly.
> Instead of hardcoding which rates are supported enable them all
> then refer to a max_rate variable at runtime.
> 
> Signed-off-by: John Watts <contact@jookia.org>
> ---
> +static int wm8782_dai_hw_params(struct snd_pcm_substream *component,
> +			    struct snd_pcm_hw_params *params,
> +			    struct snd_soc_dai *dai)
> +{
> +	struct wm8782_priv *priv =
> +		snd_soc_component_get_drvdata(dai->component);
> +
> +	if (params_rate(params) > priv->max_rate)
> +		return -EINVAL;
> +
> +	return 0;
> +}

We should be setting this as a constraint in startup, rather
than returning an error in hw_params. That will let user-space
know the supported rates and allow it to resample if necessary.

Thanks,
Charles

Re: [PATCH v2 1/3] ASoC: wm8782: Handle maximum audio rate at runtime

[John Watts]

On Thu, Sep 14, 2023 at 09:21:07AM +0000, Charles Keepax wrote:
> On Thu, Sep 14, 2023 at 03:15:50AM +1000, John Watts wrote:
> > The wm8782 supports up to 192kHz audio when pins are set correctly.
> > Instead of hardcoding which rates are supported enable them all
> > then refer to a max_rate variable at runtime.
> > 
> > Signed-off-by: John Watts <contact@jookia.org>
> > ---
> > +static int wm8782_dai_hw_params(struct snd_pcm_substream *component,
> > +			    struct snd_pcm_hw_params *params,
> > +			    struct snd_soc_dai *dai)
> > +{
> > +	struct wm8782_priv *priv =
> > +		snd_soc_component_get_drvdata(dai->component);
> > +
> > +	if (params_rate(params) > priv->max_rate)
> > +		return -EINVAL;
> > +
> > +	return 0;
> > +}
> 
> We should be setting this as a constraint in startup, rather
> than returning an error in hw_params. That will let user-space
> know the supported rates and allow it to resample if necessary.

How do you do this? The struct with the rate is statically defined.

> 
> Thanks,
> Charles

John.

Re: [PATCH v2 1/3] ASoC: wm8782: Handle maximum audio rate at runtime

[Charles Keepax]

On Thu, Sep 14, 2023 at 07:27:03PM +1000, John Watts wrote:
> On Thu, Sep 14, 2023 at 09:21:07AM +0000, Charles Keepax wrote:
> > On Thu, Sep 14, 2023 at 03:15:50AM +1000, John Watts wrote:
> > > The wm8782 supports up to 192kHz audio when pins are set correctly.
> > > Instead of hardcoding which rates are supported enable them all
> > > then refer to a max_rate variable at runtime.
> > > 
> > > Signed-off-by: John Watts <contact@jookia.org>
> > > ---
> > > +static int wm8782_dai_hw_params(struct snd_pcm_substream *component,
> > > +			    struct snd_pcm_hw_params *params,
> > > +			    struct snd_soc_dai *dai)
> > > +{
> > > +	struct wm8782_priv *priv =
> > > +		snd_soc_component_get_drvdata(dai->component);
> > > +
> > > +	if (params_rate(params) > priv->max_rate)
> > > +		return -EINVAL;
> > > +
> > > +	return 0;
> > > +}
> > 
> > We should be setting this as a constraint in startup, rather
> > than returning an error in hw_params. That will let user-space
> > know the supported rates and allow it to resample if necessary.
> 
> How do you do this? The struct with the rate is statically defined.
> 

You can programmatically add additional constraints, commonly
this will be done from the startup callback on the DAI. See
something like arizona_startup in sound/soc/codecs/arizona.c for
an example, that enables 44.1/48k rates based on clocks but the
principle should be similar.

Thanks,
Charles

Re: [PATCH v2 1/3] ASoC: wm8782: Handle maximum audio rate at runtime

[Charles Keepax]

On Thu, Sep 14, 2023 at 09:37:31AM +0000, Charles Keepax wrote:
> On Thu, Sep 14, 2023 at 07:27:03PM +1000, John Watts wrote:
> > On Thu, Sep 14, 2023 at 09:21:07AM +0000, Charles Keepax wrote:
> > > On Thu, Sep 14, 2023 at 03:15:50AM +1000, John Watts wrote:
> > > > The wm8782 supports up to 192kHz audio when pins are set correctly.
> > > > Instead of hardcoding which rates are supported enable them all
> > > > then refer to a max_rate variable at runtime.
> > > > 
> > > > Signed-off-by: John Watts <contact@jookia.org>
> > > > ---
> > > > +static int wm8782_dai_hw_params(struct snd_pcm_substream *component,
> > > > +			    struct snd_pcm_hw_params *params,
> > > > +			    struct snd_soc_dai *dai)
> > > > +{
> > > > +	struct wm8782_priv *priv =
> > > > +		snd_soc_component_get_drvdata(dai->component);
> > > > +
> > > > +	if (params_rate(params) > priv->max_rate)
> > > > +		return -EINVAL;
> > > > +
> > > > +	return 0;
> > > > +}
> > > 
> > > We should be setting this as a constraint in startup, rather
> > > than returning an error in hw_params. That will let user-space
> > > know the supported rates and allow it to resample if necessary.
> > 
> > How do you do this? The struct with the rate is statically defined.
> > 
> 
> You can programmatically add additional constraints, commonly
> this will be done from the startup callback on the DAI. See
> something like arizona_startup in sound/soc/codecs/arizona.c for
> an example, that enables 44.1/48k rates based on clocks but the
> principle should be similar.
> 

Although I would also imagine snd_pcm_hw_constraint_minmax is
going to be more appropriate in your case.

Thanks,
Charles

[PATCH v2 2/3] ASoC: wm8782: Use wlf,fsampen device tree property

[John Watts]

The wm8782 supports rates 96kHz and 192kHz as long as the hardware
is configured properly. Allow this to be specified in the device tree.

Signed-off-by: John Watts <contact@jookia.org>
---
 sound/soc/codecs/wm8782.c | 25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/sound/soc/codecs/wm8782.c b/sound/soc/codecs/wm8782.c
index 63ab63f3189a..249b58b093d6 100644
--- a/sound/soc/codecs/wm8782.c
+++ b/sound/soc/codecs/wm8782.c
@@ -122,8 +122,9 @@ static const struct snd_soc_component_driver soc_component_dev_wm8782 = {
 static int wm8782_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
 	struct wm8782_priv *priv;
-	int ret, i;
+	int ret, i, fsampen;
 
 	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
@@ -139,8 +140,26 @@ static int wm8782_probe(struct platform_device *pdev)
 	if (ret < 0)
 		return ret;
 
-	/* For configurations with FSAMPEN=0 */
-	priv->max_rate = 48000;
+	// Assume lowest value by default to avoid inadvertent overclocking
+	fsampen = 0;
+
+	if (np)
+		of_property_read_u32(np, "wlf,fsampen", &fsampen);
+
+	switch (fsampen) {
+	case 0:
+		priv->max_rate = 48000;
+		break;
+	case 1:
+		priv->max_rate = 96000;
+		break;
+	case 2:
+		priv->max_rate = 192000;
+		break;
+	default:
+		dev_err(dev, "Invalid wlf,fsampen value");
+		return -EINVAL;
+	}
 
 	return devm_snd_soc_register_component(&pdev->dev,
 			&soc_component_dev_wm8782, &wm8782_dai, 1);
-- 
2.42.0

[PATCH v2 3/3] ASoC: dt-bindings: wlf,wm8782: Add wlf,fsampen property

[John Watts]

The WM8782 can safely support rates higher than 48kHz by changing the
value of the FSAMPEN pin.

Allow specifying the FSAMPEN pin value in the device tree.

Signed-off-by: John Watts <contact@jookia.org>
---
 Documentation/devicetree/bindings/sound/wm8782.txt | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/wm8782.txt b/Documentation/devicetree/bindings/sound/wm8782.txt
index 256cdec6ec4d..d217a616e103 100644
--- a/Documentation/devicetree/bindings/sound/wm8782.txt
+++ b/Documentation/devicetree/bindings/sound/wm8782.txt
@@ -8,10 +8,15 @@ Required properties:
  - Vdda-supply : phandle to a regulator for the analog power supply (2.7V - 5.5V)
  - Vdd-supply  : phandle to a regulator for the digital power supply (2.7V - 3.6V)
 
+Optional properties:
+
+ - wlf,fsampen : FSAMPEN pin value, 0 for low, 1 for high, 2 for disconnected
+
 Example:
 
 wm8782: stereo-adc {
 	compatible = "wlf,wm8782";
 	Vdda-supply = <&vdda_supply>;
 	Vdd-supply = <&vdd_supply>;
+	wlf,fsampen = <2>; /* 192KHz */
 };
-- 
2.42.0

Re: [PATCH v2 3/3] ASoC: dt-bindings: wlf,wm8782: Add wlf,fsampen property

[Rob Herring]

On Thu, Sep 14, 2023 at 03:15:52AM +1000, John Watts wrote:
> The WM8782 can safely support rates higher than 48kHz by changing the
> value of the FSAMPEN pin.
> 
> Allow specifying the FSAMPEN pin value in the device tree.
> 
> Signed-off-by: John Watts <contact@jookia.org>
> ---
>  Documentation/devicetree/bindings/sound/wm8782.txt | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/sound/wm8782.txt b/Documentation/devicetree/bindings/sound/wm8782.txt
> index 256cdec6ec4d..d217a616e103 100644
> --- a/Documentation/devicetree/bindings/sound/wm8782.txt
> +++ b/Documentation/devicetree/bindings/sound/wm8782.txt
> @@ -8,10 +8,15 @@ Required properties:
>   - Vdda-supply : phandle to a regulator for the analog power supply (2.7V - 5.5V)
>   - Vdd-supply  : phandle to a regulator for the digital power supply (2.7V - 3.6V)
>  
> +Optional properties:
> +
> + - wlf,fsampen : FSAMPEN pin value, 0 for low, 1 for high, 2 for disconnected

What's the default if the property is not present?

> +
>  Example:
>  
>  wm8782: stereo-adc {
>  	compatible = "wlf,wm8782";
>  	Vdda-supply = <&vdda_supply>;
>  	Vdd-supply = <&vdd_supply>;
> +	wlf,fsampen = <2>; /* 192KHz */
>  };
> -- 
> 2.42.0
> 

Re: [PATCH v2 3/3] ASoC: dt-bindings: wlf,wm8782: Add wlf,fsampen property

[John Watts]

On Thu, Sep 14, 2023 at 09:52:34AM -0500, Rob Herring wrote:
> What's the default if the property is not present?

0. Should I specify it here?

John.

Re: [PATCH v2 3/3] ASoC: dt-bindings: wlf,wm8782: Add wlf,fsampen property

[Mark Brown]

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

On Fri, Sep 15, 2023 at 03:39:49PM +1000, John Watts wrote:
> On Thu, Sep 14, 2023 at 09:52:34AM -0500, Rob Herring wrote:
> > What's the default if the property is not present?
> 
> 0. Should I specify it here?

Yes.

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