[PATCH 0/2] wm8904: adapt driver for use with audio-graph-card

[PATCH 0/2] wm8904: adapt driver for use with audio-graph-card

[Michał Mirosław]

This series allows to use WM8904 codec as audio-graph-card component.
It is a rewrite of an earlier approach from
https://patchwork.kernel.org/patch/10738097/

Series based on tiwai/sound/for-next tree. You can also pull from:
   https://rere.qmqm.pl/git/linux
branch:
   wm8904

Example DTS:

/ {
        sound { 
                compatible = "audio-graph-card"; 

                widgets = 
                        "Speaker", "Loudspeaker", 
                        "Headphone", "Headphone Jack", 
                        "Microphone", "Internal Mic", 

                routing = 
                        "Headphone Jack", "HPOUTL", 
                        "Headphone Jack", "HPOUTR", 
                        "Amplifier", "LINEOUTL",
                        "Amplifier", "LINEOUTR",
                        "Loudspeaker", "Amplifier",
                        "IN1L", "MICBIAS",
                        "IN1L", "Internal Mic";

                dais = <&ssc0_port>;
        };
};

&i2c1 {
        status = "okay";

        wm8904: acodec@1a {
                compatible = "wlf,wm8904";
                reg = <0x1a>;
                clocks = <&rtc_32k>;
                clock-names = "mclk";
                interrupts-extended = <&pioA PIN_PB3 IRQ_TYPE_LEVEL_HIGH>;

                #sound-dai-cells = <0>;

                codec_port: port {         
                        codec_ep: endpoint {
                                remote-endpoint = <&ssc0_ep>;
                                dai-format = "left_j";
                                bitclock-master;
                                frame-master;
                                mclk-fs = <256>;
                        };
                };
        };
};

&ssc0 {
        pinctrl-names = "default";
        pinctrl-0 = <&pinctrl_ssc0_default>;
        status = "okay";

        atmel,shared-fs-pin;
        #sound-dai-cells = <0>;

        ssc0_port: port {
                ssc0_ep: endpoint {
                        remote-endpoint = <&codec_ep>;
                };
        };
};


Michał Mirosław (2):
  ASoC: wm8904: rework FLL handling
  ASoC: wm8904: automatically choose clock source

 .../devicetree/bindings/sound/wm8904.txt      |   4 +
 sound/soc/atmel/atmel_wm8904.c                |  11 +-
 sound/soc/codecs/Kconfig                      |   4 +
 sound/soc/codecs/Makefile                     |   2 +
 sound/soc/codecs/wm8904.c                     | 494 ++++++------------
 sound/soc/codecs/wm8904.h                     |   5 -
 sound/soc/codecs/wm_fll.c                     | 339 ++++++++++++
 sound/soc/codecs/wm_fll.h                     |  26 +
 8 files changed, 531 insertions(+), 354 deletions(-)
 create mode 100644 sound/soc/codecs/wm_fll.c
 create mode 100644 sound/soc/codecs/wm_fll.h

-- 
2.20.1

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[PATCH 2/2] ASoC: wm8904: automatically choose clock source

[Michał Mirosław]

Choose clock source automatically if not provided. This will be the case
with eg. audio-graph-card.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 sound/soc/codecs/wm8904.c | 42 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 40 insertions(+), 2 deletions(-)

diff --git a/sound/soc/codecs/wm8904.c b/sound/soc/codecs/wm8904.c
index 033a38e408b5..2d8cb61a2d97 100644
--- a/sound/soc/codecs/wm8904.c
+++ b/sound/soc/codecs/wm8904.c
@@ -367,15 +367,34 @@ static int wm8904_enable_sysclk(struct wm8904_priv *priv)
 	return err;
 }
 
+static int wm8904_bump_fll_sysclk(unsigned int *rate);
+
 static int wm8904_set_sysclk(struct snd_soc_dai *dai, int clk_id,
 			     unsigned int rate, int dir)
 {
 	struct snd_soc_component *component = dai->component;
 	struct wm8904_priv *wm8904 = snd_soc_component_get_drvdata(component);
 	unsigned int clock0, clock2;
-	int err;
+	int err, do_div = false;
 
 	switch (clk_id) {
+	case 0:
+		if (rate == clk_round_rate(wm8904->mclk, rate)) {
+			clk_id = WM8904_CLK_MCLK;
+		} else if (rate * 2 == clk_round_rate(wm8904->mclk, rate * 2)) {
+			rate *= 2;
+			clk_id = WM8904_CLK_MCLK;
+			do_div = true;
+		} else {
+			clk_id = WM8904_CLK_FLL;
+			err = wm8904_bump_fll_sysclk(&rate);
+			if (err) {
+				dev_dbg(component->dev, "Can't match %u over FLL 1406250 Hz minimum\n", rate);
+				return err;
+			}
+		}
+		break;
+
 	case WM8904_CLK_MCLK:
 	case WM8904_CLK_FLL:
 		break;
@@ -419,7 +438,9 @@ static int wm8904_set_sysclk(struct snd_soc_dai *dai, int clk_id,
 	}
 
 	/* SYSCLK shouldn't be over 13.5MHz */
-	if (rate > 13500000) {
+	if (rate > 13500000)
+		do_div = true;
+	if (do_div) {
 		clock0 = WM8904_MCLK_DIV;
 		wm8904->sysclk_rate = rate / 2;
 	} else {
@@ -1334,6 +1355,23 @@ static struct {
 	{ 480, 20 },
 };
 
+static int wm8904_bump_fll_sysclk(unsigned int *rate)
+{
+	int i;
+
+	/* bump SYSCLK rate if below minimal FLL output */
+
+	for (i = 0; i < ARRAY_SIZE(bclk_divs); i++) {
+		if (*rate * bclk_divs[i].div >= 1406250 * 10)
+			break;
+	}
+
+	if (i == ARRAY_SIZE(bclk_divs))
+		return -ERANGE;
+
+	*rate = (*rate * bclk_divs[i].div) / 10;
+	return 0;
+}
 
 static int wm8904_hw_params(struct snd_pcm_substream *substream,
 			    struct snd_pcm_hw_params *params,
-- 
2.20.1

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[PATCH 1/2] ASoC: wm8904: rework FLL handling

[Michał Mirosław]

Extract and rework FLL handling. This makes it possible to reuse
the code for other Wolfson codecs and makes codec adapt SYSCLK to
exactly match frequency required for used sampling rate.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 .../devicetree/bindings/sound/wm8904.txt      |   4 +
 sound/soc/atmel/atmel_wm8904.c                |  11 +-
 sound/soc/codecs/Kconfig                      |   4 +
 sound/soc/codecs/Makefile                     |   2 +
 sound/soc/codecs/wm8904.c                     | 454 +++++-------------
 sound/soc/codecs/wm8904.h                     |   5 -
 sound/soc/codecs/wm_fll.c                     | 339 +++++++++++++
 sound/soc/codecs/wm_fll.h                     |  26 +
 8 files changed, 492 insertions(+), 353 deletions(-)

diff --git a/Documentation/devicetree/bindings/sound/wm8904.txt b/Documentation/devicetree/bindings/sound/wm8904.txt
index 66bf261423b9..6b51d87f5987 100644
--- a/Documentation/devicetree/bindings/sound/wm8904.txt
+++ b/Documentation/devicetree/bindings/sound/wm8904.txt
@@ -9,6 +9,10 @@ Required properties:
   - clocks: reference to
     <Documentation/devicetree/bindings/clock/clock-bindings.txt>
 
+Optional properties:
+  - wlf,fll-input: FLL input signal: "mclk", "bclk", "lrclk" or "osc"
+	("mclk" by default)
+
 Pins on the device (for linking into audio routes):
 
   * IN1L
diff --git a/sound/soc/atmel/atmel_wm8904.c b/sound/soc/atmel/atmel_wm8904.c
index 776b27d3686e..b77ea2495efe 100644
--- a/sound/soc/atmel/atmel_wm8904.c
+++ b/sound/soc/atmel/atmel_wm8904.c
@@ -30,20 +30,11 @@ static int atmel_asoc_wm8904_hw_params(struct snd_pcm_substream *substream,
 	struct snd_soc_dai *codec_dai = rtd->codec_dai;
 	int ret;
 
-	ret = snd_soc_dai_set_pll(codec_dai, WM8904_FLL_MCLK, WM8904_FLL_MCLK,
-		32768, params_rate(params) * 256);
-	if (ret < 0) {
-		pr_err("%s - failed to set wm8904 codec PLL.", __func__);
-		return ret;
-	}
-
 	/*
 	 * As here wm8904 use FLL output as its system clock
-	 * so calling set_sysclk won't care freq parameter
-	 * then we pass 0
 	 */
 	ret = snd_soc_dai_set_sysclk(codec_dai, WM8904_CLK_FLL,
-			0, SND_SOC_CLOCK_IN);
+			params_rate(params) * 256, SND_SOC_CLOCK_IN);
 	if (ret < 0) {
 		pr_err("%s -failed to set wm8904 SYSCLK\n", __func__);
 		return ret;
diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
index 9f89a5346299..15a39faf33ab 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -281,6 +281,9 @@ config SND_SOC_ARIZONA
 	default m if SND_SOC_WM8997=m
 	default m if SND_SOC_WM8998=m
 
+config SND_SOC_WM_FLL
+	tristate
+
 config SND_SOC_WM_HUBS
 	tristate
 	default y if SND_SOC_WM8993=y || SND_SOC_WM8994=y
@@ -1325,6 +1328,7 @@ config SND_SOC_WM8903
 config SND_SOC_WM8904
 	tristate "Wolfson Microelectronics WM8904 CODEC"
 	depends on I2C
+	select SND_SOC_WM_FLL
 
 config SND_SOC_WM8940
         tristate
diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
index 5b4bb8cf4325..22704fbb7497 100644
--- a/sound/soc/codecs/Makefile
+++ b/sound/soc/codecs/Makefile
@@ -269,6 +269,7 @@ snd-soc-wm9090-objs := wm9090.o
 snd-soc-wm9705-objs := wm9705.o
 snd-soc-wm9712-objs := wm9712.o
 snd-soc-wm9713-objs := wm9713.o
+snd-soc-wm-fll-objs := wm_fll.o
 snd-soc-wm-hubs-objs := wm_hubs.o
 snd-soc-zx-aud96p22-objs := zx_aud96p22.o
 # Amp
@@ -549,6 +550,7 @@ obj-$(CONFIG_SND_SOC_WM9705)	+= snd-soc-wm9705.o
 obj-$(CONFIG_SND_SOC_WM9712)	+= snd-soc-wm9712.o
 obj-$(CONFIG_SND_SOC_WM9713)	+= snd-soc-wm9713.o
 obj-$(CONFIG_SND_SOC_WM_ADSP)	+= snd-soc-wm-adsp.o
+obj-$(CONFIG_SND_SOC_WM_FLL)	+= snd-soc-wm-fll.o
 obj-$(CONFIG_SND_SOC_WM_HUBS)	+= snd-soc-wm-hubs.o
 obj-$(CONFIG_SND_SOC_ZX_AUD96P22) += snd-soc-zx-aud96p22.o
 
diff --git a/sound/soc/codecs/wm8904.c b/sound/soc/codecs/wm8904.c
index 5ebdd1d9afde..033a38e408b5 100644
--- a/sound/soc/codecs/wm8904.c
+++ b/sound/soc/codecs/wm8904.c
@@ -24,6 +24,7 @@
 #include <sound/tlv.h>
 #include <sound/wm8904.h>
 
+#include "wm_fll.h"
 #include "wm8904.h"
 
 enum wm8904_type {
@@ -66,12 +67,8 @@ struct wm8904_priv {
 	int retune_mobile_cfg;
 	struct soc_enum retune_mobile_enum;
 
-	/* FLL setup */
-	int fll_src;
-	int fll_fref;
-	int fll_fout;
-
 	/* Clocking configuration */
+	struct wm_fll_data fll;
 	unsigned int mclk_rate;
 	int sysclk_src;
 	unsigned int sysclk_rate;
@@ -311,35 +308,109 @@ static bool wm8904_readable_register(struct device *dev, unsigned int reg)
 	}
 }
 
-static int wm8904_configure_clocking(struct snd_soc_component *component)
+static void wm8904_unprepare_sysclk(struct wm8904_priv *priv)
 {
+	switch (priv->sysclk_src) {
+	case WM8904_CLK_MCLK:
+		clk_disable_unprepare(priv->mclk);
+		break;
+
+	case WM8904_CLK_FLL:
+		wm_fll_disable(&priv->fll);
+		break;
+	}
+}
+
+static int wm8904_prepare_sysclk(struct wm8904_priv *priv)
+{
+	int err;
+
+	switch (priv->sysclk_src) {
+	case WM8904_CLK_MCLK:
+		err = clk_set_rate(priv->mclk, priv->mclk_rate);
+		if (!err)
+			err = clk_prepare_enable(priv->mclk);
+		break;
+
+	case WM8904_CLK_FLL:
+		err = wm_fll_enable(&priv->fll);
+		break;
+
+	default:
+		err = -EINVAL;
+		break;
+	}
+
+	return err;
+}
+
+static void wm8904_disable_sysclk(struct wm8904_priv *priv)
+{
+	regmap_update_bits(priv->regmap, WM8904_CLOCK_RATES_2,
+			   WM8904_CLK_SYS_ENA, 0);
+	wm8904_unprepare_sysclk(priv);
+}
+
+static int wm8904_enable_sysclk(struct wm8904_priv *priv)
+{
+	int err;
+
+	err = wm8904_prepare_sysclk(priv);
+	if (err < 0)
+		return err;
+
+	err = regmap_update_bits(priv->regmap, WM8904_CLOCK_RATES_2,
+				 WM8904_CLK_SYS_ENA_MASK, WM8904_CLK_SYS_ENA);
+	if (err < 0)
+		wm8904_unprepare_sysclk(priv);
+
+	return err;
+}
+
+static int wm8904_set_sysclk(struct snd_soc_dai *dai, int clk_id,
+			     unsigned int rate, int dir)
+{
+	struct snd_soc_component *component = dai->component;
 	struct wm8904_priv *wm8904 = snd_soc_component_get_drvdata(component);
-	unsigned int clock0, clock2, rate;
+	unsigned int clock0, clock2;
+	int err;
+
+	switch (clk_id) {
+	case WM8904_CLK_MCLK:
+	case WM8904_CLK_FLL:
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	if (clk_id == wm8904->sysclk_src && rate == wm8904->mclk_rate)
+		return 0;
+
+	dev_dbg(dai->dev, "Clock source is %d at %uHz\n", clk_id, rate);
 
 	/* Gate the clock while we're updating to avoid misclocking */
 	clock2 = snd_soc_component_read32(component, WM8904_CLOCK_RATES_2);
-	snd_soc_component_update_bits(component, WM8904_CLOCK_RATES_2,
-			    WM8904_SYSCLK_SRC, 0);
+	wm8904_disable_sysclk(wm8904);
+
+	wm8904->sysclk_src = clk_id;
+	wm8904->mclk_rate = rate;
 
-	/* This should be done on init() for bypass paths */
 	switch (wm8904->sysclk_src) {
 	case WM8904_CLK_MCLK:
-		dev_dbg(component->dev, "Using %dHz MCLK\n", wm8904->mclk_rate);
+		dev_dbg(component->dev, "Using %dHz MCLK\n", rate);
 
 		clock2 &= ~WM8904_SYSCLK_SRC;
-		rate = wm8904->mclk_rate;
-
-		/* Ensure the FLL is stopped */
-		snd_soc_component_update_bits(component, WM8904_FLL_CONTROL_1,
-				    WM8904_FLL_OSC_ENA | WM8904_FLL_ENA, 0);
 		break;
 
 	case WM8904_CLK_FLL:
-		dev_dbg(component->dev, "Using %dHz FLL clock\n",
-			wm8904->fll_fout);
+		err = wm_fll_set_rate(&wm8904->fll, rate);
+		if (err < 0)
+			return err;
+
+		dev_dbg(component->dev, "Using %dHz FLL clock\n", rate);
 
 		clock2 |= WM8904_SYSCLK_SRC;
-		rate = wm8904->fll_fout;
 		break;
 
 	default:
@@ -356,11 +427,18 @@ static int wm8904_configure_clocking(struct snd_soc_component *component)
 		wm8904->sysclk_rate = rate;
 	}
 
-	snd_soc_component_update_bits(component, WM8904_CLOCK_RATES_0, WM8904_MCLK_DIV,
-			    clock0);
-
+	snd_soc_component_update_bits(component, WM8904_CLOCK_RATES_0,
+				      WM8904_MCLK_DIV, clock0);
 	snd_soc_component_update_bits(component, WM8904_CLOCK_RATES_2,
-			    WM8904_CLK_SYS_ENA | WM8904_SYSCLK_SRC, clock2);
+				      WM8904_SYSCLK_SRC, clock2);
+
+	if (clock2 & WM8904_CLK_SYS_ENA) {
+		err = wm8904_enable_sysclk(wm8904);
+		if (err < 0) {
+			dev_err(component->dev, "Failed to reenable CLK_SYS: %d\n", err);
+			return err;
+		}
+	}
 
 	dev_dbg(component->dev, "CLK_SYS is %dHz\n", wm8904->sysclk_rate);
 
@@ -670,33 +748,21 @@ static int sysclk_event(struct snd_soc_dapm_widget *w,
 {
 	struct snd_soc_component *component = snd_soc_dapm_to_component(w->dapm);
 	struct wm8904_priv *wm8904 = snd_soc_component_get_drvdata(component);
+	int ret = 0;
 
 	switch (event) {
 	case SND_SOC_DAPM_PRE_PMU:
-		/* If we're using the FLL then we only start it when
-		 * required; we assume that the configuration has been
-		 * done previously and all we need to do is kick it
-		 * off.
-		 */
-		switch (wm8904->sysclk_src) {
-		case WM8904_CLK_FLL:
-			snd_soc_component_update_bits(component, WM8904_FLL_CONTROL_1,
-					    WM8904_FLL_OSC_ENA,
-					    WM8904_FLL_OSC_ENA);
-
-			snd_soc_component_update_bits(component, WM8904_FLL_CONTROL_1,
-					    WM8904_FLL_ENA,
-					    WM8904_FLL_ENA);
-			break;
-
-		default:
-			break;
-		}
+		ret = wm8904_prepare_sysclk(wm8904);
+		if (ret)
+			dev_err(component->dev,
+				"Failed to prepare SYSCLK: %d\n", ret);
+		else
+			dev_dbg(component->dev, "SYSCLK on\n");
 		break;
 
 	case SND_SOC_DAPM_POST_PMD:
-		snd_soc_component_update_bits(component, WM8904_FLL_CONTROL_1,
-				    WM8904_FLL_OSC_ENA | WM8904_FLL_ENA, 0);
+		wm8904_unprepare_sysclk(wm8904);
+		dev_dbg(component->dev, "SYSCLK off\n");
 		break;
 	}
 
@@ -1275,7 +1341,7 @@ static int wm8904_hw_params(struct snd_pcm_substream *substream,
 {
 	struct snd_soc_component *component = dai->component;
 	struct wm8904_priv *wm8904 = snd_soc_component_get_drvdata(component);
-	int ret, i, best, best_val, cur_val;
+	int i, best, best_val, cur_val;
 	unsigned int aif1 = 0;
 	unsigned int aif2 = 0;
 	unsigned int aif3 = 0;
@@ -1310,13 +1376,8 @@ static int wm8904_hw_params(struct snd_pcm_substream *substream,
 		return -EINVAL;
 	}
 
-
 	dev_dbg(component->dev, "Target BCLK is %dHz\n", wm8904->bclk);
 
-	ret = wm8904_configure_clocking(component);
-	if (ret != 0)
-		return ret;
-
 	/* Select nearest CLK_SYS_RATE */
 	best = 0;
 	best_val = abs((wm8904->sysclk_rate / clk_sys_rates[0].ratio)
@@ -1368,8 +1429,8 @@ static int wm8904_hw_params(struct snd_pcm_substream *substream,
 		}
 	}
 	wm8904->bclk = (wm8904->sysclk_rate * 10) / bclk_divs[best].div;
-	dev_dbg(component->dev, "Selected BCLK_DIV of %d for %dHz BCLK\n",
-		bclk_divs[best].div, wm8904->bclk);
+	dev_dbg(component->dev, "Selected BCLK_DIV of %d.%d for %dHz BCLK\n",
+		bclk_divs[best].div / 10, bclk_divs[best].div % 10,  wm8904->bclk);
 	aif2 |= bclk_divs[best].bclk_div;
 
 	/* LRCLK is a simple fraction of BCLK */
@@ -1396,34 +1457,6 @@ static int wm8904_hw_params(struct snd_pcm_substream *substream,
 	return 0;
 }
 
-
-static int wm8904_set_sysclk(struct snd_soc_dai *dai, int clk_id,
-			     unsigned int freq, int dir)
-{
-	struct snd_soc_component *component = dai->component;
-	struct wm8904_priv *priv = snd_soc_component_get_drvdata(component);
-
-	switch (clk_id) {
-	case WM8904_CLK_MCLK:
-		priv->sysclk_src = clk_id;
-		priv->mclk_rate = freq;
-		break;
-
-	case WM8904_CLK_FLL:
-		priv->sysclk_src = clk_id;
-		break;
-
-	default:
-		return -EINVAL;
-	}
-
-	dev_dbg(dai->dev, "Clock source is %d at %uHz\n", clk_id, freq);
-
-	wm8904_configure_clocking(component);
-
-	return 0;
-}
-
 static int wm8904_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
 {
 	struct snd_soc_component *component = dai->component;
@@ -1563,253 +1596,6 @@ static int wm8904_set_tdm_slot(struct snd_soc_dai *dai, unsigned int tx_mask,
 	return 0;
 }
 
-struct _fll_div {
-	u16 fll_fratio;
-	u16 fll_outdiv;
-	u16 fll_clk_ref_div;
-	u16 n;
-	u16 k;
-};
-
-/* The size in bits of the FLL divide multiplied by 10
- * to allow rounding later */
-#define FIXED_FLL_SIZE ((1 << 16) * 10)
-
-static struct {
-	unsigned int min;
-	unsigned int max;
-	u16 fll_fratio;
-	int ratio;
-} fll_fratios[] = {
-	{       0,    64000, 4, 16 },
-	{   64000,   128000, 3,  8 },
-	{  128000,   256000, 2,  4 },
-	{  256000,  1000000, 1,  2 },
-	{ 1000000, 13500000, 0,  1 },
-};
-
-static int fll_factors(struct _fll_div *fll_div, unsigned int Fref,
-		       unsigned int Fout)
-{
-	u64 Kpart;
-	unsigned int K, Ndiv, Nmod, target;
-	unsigned int div;
-	int i;
-
-	/* Fref must be <=13.5MHz */
-	div = 1;
-	fll_div->fll_clk_ref_div = 0;
-	while ((Fref / div) > 13500000) {
-		div *= 2;
-		fll_div->fll_clk_ref_div++;
-
-		if (div > 8) {
-			pr_err("Can't scale %dMHz input down to <=13.5MHz\n",
-			       Fref);
-			return -EINVAL;
-		}
-	}
-
-	pr_debug("Fref=%u Fout=%u\n", Fref, Fout);
-
-	/* Apply the division for our remaining calculations */
-	Fref /= div;
-
-	/* Fvco should be 90-100MHz; don't check the upper bound */
-	div = 4;
-	while (Fout * div < 90000000) {
-		div++;
-		if (div > 64) {
-			pr_err("Unable to find FLL_OUTDIV for Fout=%uHz\n",
-			       Fout);
-			return -EINVAL;
-		}
-	}
-	target = Fout * div;
-	fll_div->fll_outdiv = div - 1;
-
-	pr_debug("Fvco=%dHz\n", target);
-
-	/* Find an appropriate FLL_FRATIO and factor it out of the target */
-	for (i = 0; i < ARRAY_SIZE(fll_fratios); i++) {
-		if (fll_fratios[i].min <= Fref && Fref <= fll_fratios[i].max) {
-			fll_div->fll_fratio = fll_fratios[i].fll_fratio;
-			target /= fll_fratios[i].ratio;
-			break;
-		}
-	}
-	if (i == ARRAY_SIZE(fll_fratios)) {
-		pr_err("Unable to find FLL_FRATIO for Fref=%uHz\n", Fref);
-		return -EINVAL;
-	}
-
-	/* Now, calculate N.K */
-	Ndiv = target / Fref;
-
-	fll_div->n = Ndiv;
-	Nmod = target % Fref;
-	pr_debug("Nmod=%d\n", Nmod);
-
-	/* Calculate fractional part - scale up so we can round. */
-	Kpart = FIXED_FLL_SIZE * (long long)Nmod;
-
-	do_div(Kpart, Fref);
-
-	K = Kpart & 0xFFFFFFFF;
-
-	if ((K % 10) >= 5)
-		K += 5;
-
-	/* Move down to proper range now rounding is done */
-	fll_div->k = K / 10;
-
-	pr_debug("N=%x K=%x FLL_FRATIO=%x FLL_OUTDIV=%x FLL_CLK_REF_DIV=%x\n",
-		 fll_div->n, fll_div->k,
-		 fll_div->fll_fratio, fll_div->fll_outdiv,
-		 fll_div->fll_clk_ref_div);
-
-	return 0;
-}
-
-static int wm8904_set_fll(struct snd_soc_dai *dai, int fll_id, int source,
-			  unsigned int Fref, unsigned int Fout)
-{
-	struct snd_soc_component *component = dai->component;
-	struct wm8904_priv *wm8904 = snd_soc_component_get_drvdata(component);
-	struct _fll_div fll_div;
-	int ret, val;
-	int clock2, fll1;
-
-	/* Any change? */
-	if (source == wm8904->fll_src && Fref == wm8904->fll_fref &&
-	    Fout == wm8904->fll_fout)
-		return 0;
-
-	clock2 = snd_soc_component_read32(component, WM8904_CLOCK_RATES_2);
-
-	if (Fout == 0) {
-		dev_dbg(component->dev, "FLL disabled\n");
-
-		wm8904->fll_fref = 0;
-		wm8904->fll_fout = 0;
-
-		/* Gate SYSCLK to avoid glitches */
-		snd_soc_component_update_bits(component, WM8904_CLOCK_RATES_2,
-				    WM8904_CLK_SYS_ENA, 0);
-
-		snd_soc_component_update_bits(component, WM8904_FLL_CONTROL_1,
-				    WM8904_FLL_OSC_ENA | WM8904_FLL_ENA, 0);
-
-		goto out;
-	}
-
-	/* Validate the FLL ID */
-	switch (source) {
-	case WM8904_FLL_MCLK:
-	case WM8904_FLL_LRCLK:
-	case WM8904_FLL_BCLK:
-		ret = fll_factors(&fll_div, Fref, Fout);
-		if (ret != 0)
-			return ret;
-		break;
-
-	case WM8904_FLL_FREE_RUNNING:
-		dev_dbg(component->dev, "Using free running FLL\n");
-		/* Force 12MHz and output/4 for now */
-		Fout = 12000000;
-		Fref = 12000000;
-
-		memset(&fll_div, 0, sizeof(fll_div));
-		fll_div.fll_outdiv = 3;
-		break;
-
-	default:
-		dev_err(component->dev, "Unknown FLL ID %d\n", fll_id);
-		return -EINVAL;
-	}
-
-	/* Save current state then disable the FLL and SYSCLK to avoid
-	 * misclocking */
-	fll1 = snd_soc_component_read32(component, WM8904_FLL_CONTROL_1);
-	snd_soc_component_update_bits(component, WM8904_CLOCK_RATES_2,
-			    WM8904_CLK_SYS_ENA, 0);
-	snd_soc_component_update_bits(component, WM8904_FLL_CONTROL_1,
-			    WM8904_FLL_OSC_ENA | WM8904_FLL_ENA, 0);
-
-	/* Unlock forced oscilator control to switch it on/off */
-	snd_soc_component_update_bits(component, WM8904_CONTROL_INTERFACE_TEST_1,
-			    WM8904_USER_KEY, WM8904_USER_KEY);
-
-	if (fll_id == WM8904_FLL_FREE_RUNNING) {
-		val = WM8904_FLL_FRC_NCO;
-	} else {
-		val = 0;
-	}
-
-	snd_soc_component_update_bits(component, WM8904_FLL_NCO_TEST_1, WM8904_FLL_FRC_NCO,
-			    val);
-	snd_soc_component_update_bits(component, WM8904_CONTROL_INTERFACE_TEST_1,
-			    WM8904_USER_KEY, 0);
-
-	switch (fll_id) {
-	case WM8904_FLL_MCLK:
-		snd_soc_component_update_bits(component, WM8904_FLL_CONTROL_5,
-				    WM8904_FLL_CLK_REF_SRC_MASK, 0);
-		break;
-
-	case WM8904_FLL_LRCLK:
-		snd_soc_component_update_bits(component, WM8904_FLL_CONTROL_5,
-				    WM8904_FLL_CLK_REF_SRC_MASK, 1);
-		break;
-
-	case WM8904_FLL_BCLK:
-		snd_soc_component_update_bits(component, WM8904_FLL_CONTROL_5,
-				    WM8904_FLL_CLK_REF_SRC_MASK, 2);
-		break;
-	}
-
-	if (fll_div.k)
-		val = WM8904_FLL_FRACN_ENA;
-	else
-		val = 0;
-	snd_soc_component_update_bits(component, WM8904_FLL_CONTROL_1,
-			    WM8904_FLL_FRACN_ENA, val);
-
-	snd_soc_component_update_bits(component, WM8904_FLL_CONTROL_2,
-			    WM8904_FLL_OUTDIV_MASK | WM8904_FLL_FRATIO_MASK,
-			    (fll_div.fll_outdiv << WM8904_FLL_OUTDIV_SHIFT) |
-			    (fll_div.fll_fratio << WM8904_FLL_FRATIO_SHIFT));
-
-	snd_soc_component_write(component, WM8904_FLL_CONTROL_3, fll_div.k);
-
-	snd_soc_component_update_bits(component, WM8904_FLL_CONTROL_4, WM8904_FLL_N_MASK,
-			    fll_div.n << WM8904_FLL_N_SHIFT);
-
-	snd_soc_component_update_bits(component, WM8904_FLL_CONTROL_5,
-			    WM8904_FLL_CLK_REF_DIV_MASK,
-			    fll_div.fll_clk_ref_div 
-			    << WM8904_FLL_CLK_REF_DIV_SHIFT);
-
-	dev_dbg(component->dev, "FLL configured for %dHz->%dHz\n", Fref, Fout);
-
-	wm8904->fll_fref = Fref;
-	wm8904->fll_fout = Fout;
-	wm8904->fll_src = source;
-
-	/* Enable the FLL if it was previously active */
-	snd_soc_component_update_bits(component, WM8904_FLL_CONTROL_1,
-			    WM8904_FLL_OSC_ENA, fll1);
-	snd_soc_component_update_bits(component, WM8904_FLL_CONTROL_1,
-			    WM8904_FLL_ENA, fll1);
-
-out:
-	/* Reenable SYSCLK if it was previously active */
-	snd_soc_component_update_bits(component, WM8904_CLOCK_RATES_2,
-			    WM8904_CLK_SYS_ENA, clock2);
-
-	return 0;
-}
-
 static int wm8904_digital_mute(struct snd_soc_dai *codec_dai, int mute)
 {
 	struct snd_soc_component *component = codec_dai->component;
@@ -1857,15 +1643,6 @@ static int wm8904_set_bias_level(struct snd_soc_component *component,
 				return ret;
 			}
 
-			ret = clk_prepare_enable(wm8904->mclk);
-			if (ret) {
-				dev_err(component->dev,
-					"Failed to enable MCLK: %d\n", ret);
-				regulator_bulk_disable(ARRAY_SIZE(wm8904->supplies),
-						       wm8904->supplies);
-				return ret;
-			}
-
 			regcache_cache_only(wm8904->regmap, false);
 			regcache_sync(wm8904->regmap);
 
@@ -1908,7 +1685,6 @@ static int wm8904_set_bias_level(struct snd_soc_component *component,
 
 		regulator_bulk_disable(ARRAY_SIZE(wm8904->supplies),
 				       wm8904->supplies);
-		clk_disable_unprepare(wm8904->mclk);
 		break;
 	}
 	return 0;
@@ -1923,7 +1699,6 @@ static const struct snd_soc_dai_ops wm8904_dai_ops = {
 	.set_sysclk = wm8904_set_sysclk,
 	.set_fmt = wm8904_set_fmt,
 	.set_tdm_slot = wm8904_set_tdm_slot,
-	.set_pll = wm8904_set_fll,
 	.hw_params = wm8904_hw_params,
 	.digital_mute = wm8904_digital_mute,
 };
@@ -2151,6 +1926,8 @@ static int wm8904_i2c_probe(struct i2c_client *i2c,
 		return ret;
 	}
 
+	wm_fll_init(&wm8904->fll, &i2c->dev);
+
 	if (i2c->dev.of_node) {
 		const struct of_device_id *match;
 
@@ -2262,6 +2039,7 @@ static int wm8904_i2c_probe(struct i2c_client *i2c,
 			    WM8904_POBCTRL, 0);
 
 	/* Can leave the device powered off until we need it */
+	wm8904_disable_sysclk(wm8904);
 	regcache_cache_only(wm8904->regmap, true);
 	regulator_bulk_disable(ARRAY_SIZE(wm8904->supplies), wm8904->supplies);
 
diff --git a/sound/soc/codecs/wm8904.h b/sound/soc/codecs/wm8904.h
index c1bca52f9927..60af09e0bb15 100644
--- a/sound/soc/codecs/wm8904.h
+++ b/sound/soc/codecs/wm8904.h
@@ -13,11 +13,6 @@
 #define WM8904_CLK_MCLK 1
 #define WM8904_CLK_FLL  2
 
-#define WM8904_FLL_MCLK          1
-#define WM8904_FLL_BCLK          2
-#define WM8904_FLL_LRCLK         3
-#define WM8904_FLL_FREE_RUNNING  4
-
 /*
  * Register values.
  */
diff --git a/sound/soc/codecs/wm_fll.c b/sound/soc/codecs/wm_fll.c
new file mode 100644
index 000000000000..c76714e5fee5
--- /dev/null
+++ b/sound/soc/codecs/wm_fll.c
@@ -0,0 +1,339 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * wm_fll.c  --  WM89xx FLL support
+ *
+ * Copyright 2019 Michał Mirosław
+ *
+ * WM8904 can generate its clock directly from MCLK, from
+ * internal FLL synchronizing to one of hw frame clocks
+ * or from FLL's VCO in free-running mode
+ */
+
+#include <linux/bitfield.h>
+#include <linux/clk-provider.h>
+#include <linux/delay.h>
+#include <linux/kernel.h>
+#include "wm_fll.h"
+#include "wm8904.h"
+
+enum wm_fll_ref_source
+{
+	FLL_REF_MCLK,
+	FLL_REF_BCLK,
+	FLL_REF_FSCLK,
+	FLL_REF_OSC,
+};
+
+static const char *const wm_fll_parents[] = {
+	"mclk", "bclk", "lrclk", "osc"
+};
+
+static bool wm_fll_in_free_running_mode(struct wm_fll_data *hw)
+{
+	unsigned int val;
+
+	if (regmap_read(hw->regmap, WM8904_FLL_NCO_TEST_0, &val) < 0)
+		return false;
+
+	return FIELD_GET(WM8904_FLL_FRC_NCO_MASK, val);
+}
+
+static int wm_fll_get_parent(struct wm_fll_data *hw)
+{
+	unsigned int val;
+	int err;
+
+	/* free-running mode? */
+	if (wm_fll_in_free_running_mode(hw))
+		return FLL_REF_OSC;
+
+	err = regmap_read(hw->regmap, WM8904_FLL_CONTROL_5, &val);
+	if (err < 0)
+		return err;
+
+	return FIELD_GET(WM8904_FLL_CLK_REF_SRC_MASK, val);
+}
+
+static int wm_fll_set_parent(struct wm_fll_data *hw, enum wm_fll_ref_source index)
+{
+	unsigned val;
+	bool osc_en = index == FLL_REF_OSC;
+	int err;
+
+	if (osc_en) {
+		/* set osc freq (approx 96MHz) */
+		val = FIELD_PREP(WM8904_FLL_FRC_NCO_VAL_MASK, 0x19);
+		err = regmap_update_bits(hw->regmap, WM8904_FLL_NCO_TEST_1,
+					 WM8904_FLL_FRC_NCO_VAL_MASK, val);
+		if (err < 0)
+			return err;
+	}
+
+	/* set free-running mode */
+	val = FIELD_PREP(WM8904_FLL_FRC_NCO_MASK, osc_en);
+	err = regmap_update_bits(hw->regmap, WM8904_FLL_NCO_TEST_0,
+				 WM8904_FLL_FRC_NCO_MASK, val);
+	if (osc_en || err < 0)
+		return err;
+
+	/* set FLL reference input */
+	val = FIELD_PREP(WM8904_FLL_CLK_REF_SRC_MASK, index);
+	err = regmap_update_bits(hw->regmap, WM8904_FLL_CONTROL_5,
+				 WM8904_FLL_CLK_REF_SRC_MASK, val);
+
+	return err;
+}
+
+int wm_fll_enable(struct wm_fll_data *hw)
+{
+	unsigned int val;
+	int clk_src;
+	int err, retry;
+
+	clk_src = wm_fll_get_parent(hw);
+	if (clk_src == FLL_REF_MCLK) {
+		err = clk_prepare_enable(hw->mclk);
+		if (err < 0)
+			return err;
+	}
+
+	err = regmap_update_bits(hw->regmap, WM8904_FLL_CONTROL_1,
+				 WM8904_FLL_OSC_ENA_MASK, WM8904_FLL_OSC_ENA);
+	if (err < 0)
+		goto err_out;
+
+	err = regmap_write(hw->regmap, WM8904_INTERRUPT_STATUS,
+			   WM8904_FLL_LOCK_EINT);
+	if (err < 0)
+		goto err_out;
+
+	err = regmap_update_bits(hw->regmap, WM8904_FLL_CONTROL_1,
+				 WM8904_FLL_ENA_MASK, WM8904_FLL_ENA);
+	if (err)
+		goto err_out;
+
+	if (clk_src == FLL_REF_OSC) {
+		usleep_range(150, 250);
+		return 0;
+	}
+
+	for (retry = 3; retry; --retry) {
+		msleep(1);
+		err = regmap_read(hw->regmap, WM8904_INTERRUPT_STATUS, &val);
+		if (err < 0)
+			goto err_out;
+
+		if (val & WM8904_FLL_LOCK_EINT)
+			break;
+	}
+
+	/* it seems that FLL_LOCK might never be asserted */
+	/* eg. WM8904's FLL doesn't, but works anyway */
+	return 0;
+
+err_out:
+	wm_fll_disable(hw);
+
+	if (clk_src == FLL_REF_MCLK)
+		clk_disable_unprepare(hw->mclk);
+
+	return err;
+}
+EXPORT_SYMBOL_GPL(wm_fll_enable);
+
+int wm_fll_disable(struct wm_fll_data *hw)
+{
+	return regmap_update_bits(hw->regmap, WM8904_FLL_CONTROL_1,
+				  WM8904_FLL_ENA_MASK|WM8904_FLL_OSC_ENA_MASK,
+				  0);
+}
+EXPORT_SYMBOL_GPL(wm_fll_disable);
+
+static int wm_fll_is_enabled(struct wm_fll_data *hw)
+{
+	unsigned int val;
+	int err;
+
+	err = regmap_read(hw->regmap, WM8904_FLL_CONTROL_1, &val);
+	if (err < 0)
+		return err;
+
+	return FIELD_GET(WM8904_FLL_ENA_MASK, val);
+}
+
+static unsigned int wm_fll_apply_refdiv(unsigned long *parent_rate)
+{
+	unsigned int refdiv;
+
+	/* FLL input divider; should ensure Fin <= 13.5MHz */
+
+	refdiv = DIV_ROUND_UP(*parent_rate, 13500000);
+	refdiv = order_base_2(refdiv);
+	if (refdiv > 3)
+		refdiv = 3;
+
+	*parent_rate >>= refdiv;
+
+	return refdiv;
+}
+
+static unsigned int wm_fll_apply_fratio(unsigned long *parent_rate)
+{
+	unsigned int fratio;
+
+	/* FLL comparator divider; efectively Fin multiplier */
+	/* as tabularized in WM8904 datasheet */
+
+	if (*parent_rate >= 256000)
+		fratio = *parent_rate < 1024000;
+	else if (*parent_rate >= 64000)
+		fratio = 2 + (*parent_rate < 128000);
+	else
+		fratio = 4;
+
+	*parent_rate <<= fratio;
+
+	return fratio;
+}
+
+static unsigned int wm_fll_apply_outdiv_rev(unsigned long *rate)
+{
+	unsigned int div;
+
+	/* Fvco -> Fout divider; target: 90 <= Fvco <= 100 MHz */
+
+	div = DIV_ROUND_UP(90000000, *rate);
+	if (div > 64) {
+		*rate = 90000000;
+		return 64;
+	}
+
+	if (div < 4)
+		div = 4;
+
+	*rate *= div;
+	return div;
+}
+
+int wm_fll_set_rate(struct wm_fll_data *hw, unsigned long rate)
+{
+	unsigned long long freq;
+	unsigned long mclk_rate;
+	unsigned int val, mask, refdiv, outdiv, fratio;
+	int err, enabled;
+
+	err = wm_fll_get_parent(hw);
+	if (err < 0)
+		return err;
+
+	if (err != FLL_REF_OSC) {
+		unsigned long parent_rate = mclk_rate = clk_get_rate(hw->mclk);
+
+		refdiv = wm_fll_apply_refdiv(&parent_rate);
+		fratio = wm_fll_apply_fratio(&parent_rate);
+		outdiv = wm_fll_apply_outdiv_rev(&rate);
+
+		freq = (unsigned long long)rate << 16;
+		freq += parent_rate / 2;
+		do_div(freq, parent_rate);
+
+		rate = (freq * parent_rate) >> 16;
+	} else {
+		unsigned long vco_rate = 96000000;
+
+		mclk_rate = 0;
+		fratio = refdiv = 0;
+		rate = DIV_ROUND_CLOSEST(vco_rate, rate);
+		outdiv = clamp_t(unsigned long, rate, 4, 64);
+		freq = 0x177 << 16;
+
+		rate = vco_rate;
+	}
+
+	/* configure */
+
+	enabled = err = wm_fll_is_enabled(hw);
+	if (err > 0)
+		err = wm_fll_disable(hw);
+	if (err < 0)
+		return err;
+
+	dev_dbg(regmap_get_device(hw->regmap),
+		"configuring FLL for %luHz -> %luHz -> %luHz%s\n",
+		mclk_rate, rate, rate / outdiv,
+		enabled ? " (while enabled)" : "");
+	dev_dbg(regmap_get_device(hw->regmap),
+		"FLL settings: N=%llu K=%llu FRATIO=%u OUTDIV=%u REF_DIV=%u\n",
+		freq >> 16, freq & 0xFFFF, fratio, outdiv, refdiv);
+
+	val = FIELD_PREP(WM8904_FLL_CLK_REF_DIV_MASK, refdiv);
+	err = regmap_update_bits(hw->regmap, WM8904_FLL_CONTROL_5,
+				 WM8904_FLL_CLK_REF_DIV_MASK, val);
+	if (err < 0)
+		return err;
+
+	val = FIELD_PREP(WM8904_FLL_OUTDIV_MASK, outdiv - 1) |
+	      FIELD_PREP(WM8904_FLL_FRATIO_MASK, fratio);
+	mask = WM8904_FLL_OUTDIV_MASK | WM8904_FLL_FRATIO_MASK;
+	err = regmap_update_bits(hw->regmap, WM8904_FLL_CONTROL_2, mask, val);
+	if (err < 0)
+		return err;
+
+	err = regmap_write(hw->regmap, WM8904_FLL_CONTROL_3, (uint16_t)freq);
+	if (err < 0)
+		return err;
+
+	val = FIELD_PREP(WM8904_FLL_FRACN_ENA_MASK, !!(uint16_t)freq);
+	err = regmap_update_bits(hw->regmap, WM8904_FLL_CONTROL_1,
+				 WM8904_FLL_FRACN_ENA_MASK, val);
+	if (err < 0)
+		return err;
+
+	val = FIELD_PREP(WM8904_FLL_N_MASK, freq >> 16);
+	err = regmap_update_bits(hw->regmap, WM8904_FLL_CONTROL_4,
+				 WM8904_FLL_N_MASK, val);
+	if (err < 0)
+		return err;
+
+	if (enabled)
+		err = wm_fll_enable(hw);
+
+	return err;
+}
+EXPORT_SYMBOL_GPL(wm_fll_set_rate);
+
+int wm_fll_init(struct wm_fll_data *hw, struct device *dev)
+{
+	const char *src;
+	int err, index;
+
+	err = of_property_read_string(dev->of_node, "wlf,fll-input", &src);
+	if (err == -EINVAL)
+		index = err = 0;
+	else if (err >= 0)
+		index = err = match_string(wm_fll_parents,
+					   ARRAY_SIZE(wm_fll_parents), src);
+	if (err < 0)
+		return err;
+
+	hw->mclk = NULL;
+	if (!index) {
+		hw->mclk = devm_clk_get(dev, "mclk");
+		if (IS_ERR(hw->mclk)) {
+			err = PTR_ERR(hw->mclk);
+			dev_err(dev, "Failed to get MCLK for FLL: %d\n", err);
+			return err;
+		}
+	}
+
+	hw->regmap = dev_get_regmap(dev, NULL);
+	if (!hw->regmap) {
+		dev_err(dev, "driver BUG: regmap not configured\n");
+		return -EINVAL;
+	}
+
+	err = wm_fll_set_parent(hw, index);
+
+	return err;
+}
+EXPORT_SYMBOL_GPL(wm_fll_init);
diff --git a/sound/soc/codecs/wm_fll.h b/sound/soc/codecs/wm_fll.h
new file mode 100644
index 000000000000..2518a0910179
--- /dev/null
+++ b/sound/soc/codecs/wm_fll.h
@@ -0,0 +1,26 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * wm_fll.h  --  FLL support for Wolfson codecs
+ *
+ * Copyright 2019 Michał Mirosław
+ */
+
+#ifndef _WM_FLL_H
+#define _WM_FLL_H
+
+#include <linux/clk.h>
+#include <linux/device.h>
+#include <linux/regmap.h>
+
+struct wm_fll_data
+{
+	struct regmap			*regmap;
+	struct clk			*mclk;
+};
+
+int wm_fll_init(struct wm_fll_data *hw, struct device *dev);
+int wm_fll_enable(struct wm_fll_data *hw);
+int wm_fll_disable(struct wm_fll_data *hw);
+int wm_fll_set_rate(struct wm_fll_data *hw, unsigned long rate);
+
+#endif /* _WM_FLL_H */
-- 
2.20.1

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 1/2] ASoC: wm8904: rework FLL handling

[Charles Keepax]

On Mon, Jul 22, 2019 at 07:57:21PM +0200, Michał Mirosław wrote:
> Extract and rework FLL handling. This makes it possible to reuse
> the code for other Wolfson codecs and makes codec adapt SYSCLK to
> exactly match frequency required for used sampling rate.
> 

Do you have thoughts on which CODECs you would be including in
this? These older parts often have small differences between the
configuration that might make this challenging so if you have
plans here would be good to have a look from this end.

> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> ---
>  .../devicetree/bindings/sound/wm8904.txt      |   4 +
>  sound/soc/atmel/atmel_wm8904.c                |  11 +-
>  sound/soc/codecs/Kconfig                      |   4 +
>  sound/soc/codecs/Makefile                     |   2 +
>  sound/soc/codecs/wm8904.c                     | 454 +++++-------------
>  sound/soc/codecs/wm8904.h                     |   5 -
>  sound/soc/codecs/wm_fll.c                     | 339 +++++++++++++
>  sound/soc/codecs/wm_fll.h                     |  26 +
>  8 files changed, 492 insertions(+), 353 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/sound/wm8904.txt b/Documentation/devicetree/bindings/sound/wm8904.txt
> index 66bf261423b9..6b51d87f5987 100644
> --- a/Documentation/devicetree/bindings/sound/wm8904.txt
> +++ b/Documentation/devicetree/bindings/sound/wm8904.txt
> @@ -9,6 +9,10 @@ Required properties:
>    - clocks: reference to
>      <Documentation/devicetree/bindings/clock/clock-bindings.txt>
>  
> +Optional properties:
> +  - wlf,fll-input: FLL input signal: "mclk", "bclk", "lrclk" or "osc"
> +	("mclk" by default)
> +

Would be keen to see some other peoples thoughts on this one, but
this feels a little off to me. Selecting the FLL input is
generally something that would be done on a use-case by use-case
basis rather than being a property of the hardware that would
belong in device tree.

Thanks,
Charles
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 1/2] ASoC: wm8904: rework FLL handling

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 879 bytes --]

On Tue, Jul 23, 2019 at 09:27:16AM +0100, Charles Keepax wrote:
> On Mon, Jul 22, 2019 at 07:57:21PM +0200, Michał Mirosław wrote:
> > Extract and rework FLL handling. This makes it possible to reuse
> > the code for other Wolfson codecs and makes codec adapt SYSCLK to
> > exactly match frequency required for used sampling rate.

> Do you have thoughts on which CODECs you would be including in
> this? These older parts often have small differences between the
> configuration that might make this challenging so if you have
> plans here would be good to have a look from this end.

Right, it's not like it's the same IP being dropped into multiple chips
in an identical fashion.  There's a lot of high level similarities in
the register interfaces but also many small per device tweaks, and it's
not clear what benefit we get from refactoring at this point.

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

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 1/2] ASoC: wm8904: rework FLL handling

[Michał Mirosław]

On Tue, Jul 23, 2019 at 11:52:48AM +0100, Mark Brown wrote:
> On Tue, Jul 23, 2019 at 09:27:16AM +0100, Charles Keepax wrote:
> > On Mon, Jul 22, 2019 at 07:57:21PM +0200, Michał Mirosław wrote:
> > > Extract and rework FLL handling. This makes it possible to reuse
> > > the code for other Wolfson codecs and makes codec adapt SYSCLK to
> > > exactly match frequency required for used sampling rate.
> 
> > Do you have thoughts on which CODECs you would be including in
> > this? These older parts often have small differences between the
> > configuration that might make this challenging so if you have
> > plans here would be good to have a look from this end.
> 
> Right, it's not like it's the same IP being dropped into multiple chips
> in an identical fashion.  There's a lot of high level similarities in
> the register interfaces but also many small per device tweaks, and it's
> not clear what benefit we get from refactoring at this point.

This would be mainly code separation, so it's easier to understand and
has a potential for direct reuse. I can see that clock selection needs
to be changed, but the idea is to have it configurable via device-tree.

I picked at random WM9081. It's FLL implementation looks very similar -
major diffferences being in FLL_OUTDIV selection (direct divider vs 2^N)
and register block offset.

Another random pick - WM8900. The general FLL idea seems the same, but
this one has a bit more complicated register layout, so I wouldn't
consider it at first.

WM8994 - it has two FLL's but otherwise has identical register layout
for them as WM8904. The only difference is in clock source selection.

Best Regards,
Michał Mirosław

Re: [PATCH 1/2] ASoC: wm8904: rework FLL handling

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1488 bytes --]

On Tue, Jul 23, 2019 at 01:54:15PM +0200, Michał Mirosław wrote:
> On Tue, Jul 23, 2019 at 11:52:48AM +0100, Mark Brown wrote:

> > Right, it's not like it's the same IP being dropped into multiple chips
> > in an identical fashion.  There's a lot of high level similarities in
> > the register interfaces but also many small per device tweaks, and it's
> > not clear what benefit we get from refactoring at this point.

> This would be mainly code separation, so it's easier to understand and
> has a potential for direct reuse. I can see that clock selection needs
> to be changed, but the idea is to have it configurable via device-tree.

Not all the world is DT...

> I picked at random WM9081. It's FLL implementation looks very similar -
> major diffferences being in FLL_OUTDIV selection (direct divider vs 2^N)
> and register block offset.

> Another random pick - WM8900. The general FLL idea seems the same, but
> this one has a bit more complicated register layout, so I wouldn't
> consider it at first.

Yeah, there's a lot that on the surface looks similar but there's a lot
of variations in the detail - different numbers getting plugged in,
register layouts getting tweaked, different sets of sources and so on.
I get that we can potentially combine the implementations which in
theory is code reuse but what is the end goal for that code reuse given
that all the refactoring is going to warrant some testing over a bunch
of different parts.

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

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 1/2] ASoC: wm8904: rework FLL handling

[Charles Keepax]

On Tue, Jul 23, 2019 at 01:54:15PM +0200, Michał Mirosław wrote:
> On Tue, Jul 23, 2019 at 11:52:48AM +0100, Mark Brown wrote:
> > On Tue, Jul 23, 2019 at 09:27:16AM +0100, Charles Keepax wrote:
> > > On Mon, Jul 22, 2019 at 07:57:21PM +0200, Michał Mirosław wrote:
> > > > Extract and rework FLL handling. This makes it possible to reuse
> > > > the code for other Wolfson codecs and makes codec adapt SYSCLK to
> > > > exactly match frequency required for used sampling rate.
> > 
> > > Do you have thoughts on which CODECs you would be including in
> > > this? These older parts often have small differences between the
> > > configuration that might make this challenging so if you have
> > > plans here would be good to have a look from this end.
> > 
> > Right, it's not like it's the same IP being dropped into multiple chips
> > in an identical fashion.  There's a lot of high level similarities in
> > the register interfaces but also many small per device tweaks, and it's
> > not clear what benefit we get from refactoring at this point.
> 
> This would be mainly code separation, so it's easier to understand and
> has a potential for direct reuse. I can see that clock selection needs
> to be changed, but the idea is to have it configurable via device-tree.
> 
> I picked at random WM9081. It's FLL implementation looks very similar -
> major diffferences being in FLL_OUTDIV selection (direct divider vs 2^N)
> and register block offset.
> 
> Another random pick - WM8900. The general FLL idea seems the same, but
> this one has a bit more complicated register layout, so I wouldn't
> consider it at first.
> 
> WM8994 - it has two FLL's but otherwise has identical register layout
> for them as WM8904. The only difference is in clock source selection.
> 

The register layouts do look similar but the code controlling the
FLLs in these cases is quite different. I am somewhat nervous
there are subtle factors at play here which are going to cause
problems and its very hard to seperate divergence and actually
required sequencing here.

At the very least if you are really sure you want to proceed in
this direction I think we should look at splitting the patch into
two parts one that factors out the functionality and a separate
patch that adds any new functionality. It makes things much
easier to review that way.

Thanks,
Charles
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel