* [PATCH 0/2] MMP2 Audio clock controller driver
@ 2020-05-11 19:55 Lubomir Rintel
2020-05-11 19:55 ` [PATCH 1/2] dt-bindings: sound: Add Marvell MMP Audio Clock Controller binding Lubomir Rintel
2020-05-11 19:55 ` [PATCH 2/2] clk: mmp2: Add audio clock controller driver Lubomir Rintel
0 siblings, 2 replies; 7+ messages in thread
From: Lubomir Rintel @ 2020-05-11 19:55 UTC (permalink / raw)
To: Michael Turquette
Cc: Stephen Boyd, Liam Girdwood, Mark Brown, Rob Herring, linux-clk,
devicetree, linux-kernel, linux-media
Hi,
please consider applying this patch set. It contains a driver for the
audio clock generator on Marvell MMP2 along with the DT bindings.
Currently the I2S driver (mmp2-sspa) doesn't have support for DT and is
not able to get clocks from this driver. The patch set to address that
will be sent separately.
Note that the DT binding validation is going to complain about the
example in it until the "clk: mmp2: Enable Audio and GPU on MMP2 and
MMP3" [1] patch set is merged. Not a big deal I suppose.
[1] https://lore.kernel.org/lkml/20200511192517.1206442-1-lkundrak@v3.sk/
This patch set has been tested on an OLPC XO-1.75 laptop, along with a
patched mmp-sspa driver and rt5631 codec.
Thank you
Lubo
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] dt-bindings: sound: Add Marvell MMP Audio Clock Controller binding
2020-05-11 19:55 [PATCH 0/2] MMP2 Audio clock controller driver Lubomir Rintel
@ 2020-05-11 19:55 ` Lubomir Rintel
2020-05-12 1:41 ` Rob Herring
2020-05-14 21:06 ` Stephen Boyd
2020-05-11 19:55 ` [PATCH 2/2] clk: mmp2: Add audio clock controller driver Lubomir Rintel
1 sibling, 2 replies; 7+ messages in thread
From: Lubomir Rintel @ 2020-05-11 19:55 UTC (permalink / raw)
To: Michael Turquette
Cc: Stephen Boyd, Liam Girdwood, Mark Brown, Rob Herring, linux-clk,
devicetree, linux-kernel, linux-media, Lubomir Rintel
This describes the bindings for a controller that generates master and bit
clocks for the I2S interface.
Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
---
.../clock/marvell,mmp2-audio-clock.yaml | 73 +++++++++++++++++++
.../dt-bindings/clock/marvell,mmp2-audio.h | 8 ++
2 files changed, 81 insertions(+)
create mode 100644 Documentation/devicetree/bindings/clock/marvell,mmp2-audio-clock.yaml
create mode 100644 include/dt-bindings/clock/marvell,mmp2-audio.h
diff --git a/Documentation/devicetree/bindings/clock/marvell,mmp2-audio-clock.yaml b/Documentation/devicetree/bindings/clock/marvell,mmp2-audio-clock.yaml
new file mode 100644
index 000000000000..b86e9fbfa56d
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/marvell,mmp2-audio-clock.yaml
@@ -0,0 +1,73 @@
+# SPDX-License-Identifier: (GPL-2.0+ OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/marvell,mmp2-audio-clock.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Marvell MMP2 Audio Clock Controller
+
+maintainers:
+ - Lubomir Rintel <lkundrak@v3.sk>
+
+description: |
+ The audio clock controller generates and supplies the clocks to the audio
+ codec.
+
+ Each clock is assigned an identifier and client nodes use this identifier
+ to specify the clock which they consume.
+
+ All these identifiers could be found in <dt-bindings/clock/marvell,mmp2.h>.
+
+properties:
+ compatible:
+ enum:
+ - marvell,mmp2-audio-clock
+
+ reg:
+ maxItems: 1
+
+ clocks:
+ items:
+ - description: Audio subsystem clock
+ - description: The crystal oscillator clock
+ - description: First I2S clock
+ - description: Second I2S clock
+
+ clock-names:
+ items:
+ - const: audio
+ - const: vctcxo
+ - const: i2s0
+ - const: i2s1
+
+ '#clock-cells':
+ const: 1
+
+ power-domains:
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+ - clocks
+ - clock-names
+ - '#clock-cells'
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/marvell,mmp2.h>
+ #include <dt-bindings/power/marvell,mmp2.h>
+
+ clocks@d42a0c30 {
+ compatible = "marvell,mmp2-audio-clock";
+ reg = <0xd42a0c30 0x10>;
+ clock-names = "audio", "vctcxo", "i2s0", "i2s1";
+ clocks = <&soc_clocks MMP2_CLK_AUDIO>,
+ <&soc_clocks MMP2_CLK_VCTCXO>,
+ <&soc_clocks MMP2_CLK_I2S0>,
+ <&soc_clocks MMP2_CLK_I2S1>;
+ power-domains = <&soc_clocks MMP2_POWER_DOMAIN_AUDIO>;
+ #clock-cells = <1>;
+ };
diff --git a/include/dt-bindings/clock/marvell,mmp2-audio.h b/include/dt-bindings/clock/marvell,mmp2-audio.h
new file mode 100644
index 000000000000..127b48ec0f0a
--- /dev/null
+++ b/include/dt-bindings/clock/marvell,mmp2-audio.h
@@ -0,0 +1,8 @@
+/* SPDX-License-Identifier: (GPL-2.0+ OR BSD-2-Clause) */
+#ifndef __DT_BINDINGS_CLOCK_MARVELL_MMP2_AUDIO_H
+#define __DT_BINDINGS_CLOCK_MARVELL_MMP2_AUDIO_H
+
+#define MMP2_CLK_AUDIO_SYSCLK 1
+#define MMP2_CLK_AUDIO_SSPA0 2
+#define MMP2_CLK_AUDIO_SSPA1 3
+#endif
--
2.26.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] clk: mmp2: Add audio clock controller driver
2020-05-11 19:55 [PATCH 0/2] MMP2 Audio clock controller driver Lubomir Rintel
2020-05-11 19:55 ` [PATCH 1/2] dt-bindings: sound: Add Marvell MMP Audio Clock Controller binding Lubomir Rintel
@ 2020-05-11 19:55 ` Lubomir Rintel
2020-05-14 21:15 ` Stephen Boyd
1 sibling, 1 reply; 7+ messages in thread
From: Lubomir Rintel @ 2020-05-11 19:55 UTC (permalink / raw)
To: Michael Turquette
Cc: Stephen Boyd, Liam Girdwood, Mark Brown, Rob Herring, linux-clk,
devicetree, linux-kernel, linux-media, Lubomir Rintel
This is a driver for a block that generates master and bit clocks for
the I2S interface. It's separate from the PMUs that generate clocks for
the peripherals.
Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
---
drivers/clk/Kconfig | 6 +
drivers/clk/mmp/Makefile | 1 +
drivers/clk/mmp/clk-audio.c | 563 ++++++++++++++++++++++++++++++++++++
3 files changed, 570 insertions(+)
create mode 100644 drivers/clk/mmp/clk-audio.c
diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index bcb257baed06..a28cf98ffe68 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -326,6 +326,12 @@ config COMMON_CLK_MMP2
help
Support for Marvell MMP2 and MMP3 SoC clocks
+config COMMON_CLK_MMP2_AUDIO
+ tristate "Clock driver for MMP2 Audio subsystem"
+ depends on COMMON_CLK_MMP2 || COMPILE_TEST
+ help
+ This driver supports clocks for Audio subsystem on MMP2 SoC.
+
config COMMON_CLK_BD718XX
tristate "Clock driver for 32K clk gates on ROHM PMICs"
depends on MFD_ROHM_BD718XX || MFD_ROHM_BD70528 || MFD_ROHM_BD71828
diff --git a/drivers/clk/mmp/Makefile b/drivers/clk/mmp/Makefile
index 14dc8a8a9d08..9a4b79ff6572 100644
--- a/drivers/clk/mmp/Makefile
+++ b/drivers/clk/mmp/Makefile
@@ -9,6 +9,7 @@ obj-$(CONFIG_RESET_CONTROLLER) += reset.o
obj-$(CONFIG_MACH_MMP_DT) += clk-of-pxa168.o clk-of-pxa910.o
obj-$(CONFIG_COMMON_CLK_MMP2) += clk-of-mmp2.o clk-pll.o
+obj-$(CONFIG_COMMON_CLK_MMP2_AUDIO) += clk-audio.o
obj-$(CONFIG_CPU_PXA168) += clk-pxa168.o
obj-$(CONFIG_CPU_PXA910) += clk-pxa910.o
diff --git a/drivers/clk/mmp/clk-audio.c b/drivers/clk/mmp/clk-audio.c
new file mode 100644
index 000000000000..ee89b97dc09a
--- /dev/null
+++ b/drivers/clk/mmp/clk-audio.c
@@ -0,0 +1,563 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * MMP Audio Clock Controller driver
+ *
+ * Copyright (C) 2020 Lubomir Rintel <lkundrak@v3.sk>
+ */
+
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <dt-bindings/clock/marvell,mmp2-audio.h>
+
+enum {
+ SSPA_AUD_CTRL = 0x04,
+ SSPA_AUD_PLL_CTRL0 = 0x08,
+ SSPA_AUD_PLL_CTRL1 = 0x0c,
+};
+
+/* SSPA Audio Control Register */
+#define SSPA_AUD_CTRL_SYSCLK_SHIFT 0
+#define SSPA_AUD_CTRL_SSPA0_MUX_SHIFT 7
+#define SSPA_AUD_CTRL_SSPA0_SHIFT 8
+#define SSPA_AUD_CTRL_SSPA1_SHIFT 16
+#define SSPA_AUD_CTRL_SSPA1_MUX_SHIFT 23
+#define SSPA_AUD_CTRL_DIV_MASK 0x7e
+
+/* SSPA Audio PLL Control 0 Register */
+#define SSPA_AUD_PLL_CTRL0_DIV_OCLK_MODULO_MASK (0x7 << 28)
+#define SSPA_AUD_PLL_CTRL0_DIV_OCLK_MODULO(x) ((x) << 28)
+#define SSPA_AUD_PLL_CTRL0_FRACT_MASK (0xfffff << 8)
+#define SSPA_AUD_PLL_CTRL0_FRACT(x) ((x) << 8)
+#define SSPA_AUD_PLL_CTRL0_ENA_DITHER (1 << 7)
+#define SSPA_AUD_PLL_CTRL0_ICP_2UA (0 << 5)
+#define SSPA_AUD_PLL_CTRL0_ICP_5UA (1 << 5)
+#define SSPA_AUD_PLL_CTRL0_ICP_7UA (2 << 5)
+#define SSPA_AUD_PLL_CTRL0_ICP_10UA (3 << 5)
+#define SSPA_AUD_PLL_CTRL0_DIV_FBCCLK_MASK (0x3 << 3)
+#define SSPA_AUD_PLL_CTRL0_DIV_FBCCLK(x) ((x) << 3)
+#define SSPA_AUD_PLL_CTRL0_DIV_MCLK_MASK (0x1 << 2)
+#define SSPA_AUD_PLL_CTRL0_DIV_MCLK(x) ((x) << 2)
+#define SSPA_AUD_PLL_CTRL0_PD_OVPROT_DIS (1 << 1)
+#define SSPA_AUD_PLL_CTRL0_PU (1 << 0)
+
+/* SSPA Audio PLL Control 1 Register */
+#define SSPA_AUD_PLL_CTRL1_SEL_FAST_CLK (1 << 24)
+#define SSPA_AUD_PLL_CTRL1_CLK_SEL_MASK (1 << 11)
+#define SSPA_AUD_PLL_CTRL1_CLK_SEL_AUDIO_PLL (1 << 11)
+#define SSPA_AUD_PLL_CTRL1_CLK_SEL_VCXO (0 << 11)
+#define SSPA_AUD_PLL_CTRL1_DIV_OCLK_PATTERN_MASK (0x7ff << 0)
+#define SSPA_AUD_PLL_CTRL1_DIV_OCLK_PATTERN(x) ((x) << 0)
+
+struct mmp_audio_mux {
+ struct clk_hw hw;
+ struct mmp2_audio_clk *priv;
+ u8 shift;
+ u8 flags;
+ int index;
+};
+
+#define to_mmp_audio_mux(_hw) container_of(_hw, struct mmp_audio_mux, hw)
+
+struct mmp_audio_div {
+ struct clk_hw hw;
+ struct mmp2_audio_clk *priv;
+ u8 shift;
+ int value;
+};
+
+#define to_mmp_audio_div(_hw) container_of(_hw, struct mmp_audio_div, hw)
+
+struct mmp_audio_pll {
+ struct clk_hw hw;
+ struct mmp2_audio_clk *priv;
+ u32 ctrl0;
+ u32 ctrl1;
+};
+
+#define to_mmp_audio_pll(_hw) container_of(_hw, struct mmp_audio_pll, hw)
+
+struct mmp2_audio_clk {
+ void __iomem *mmio_base;
+
+ struct clk *audio_clk;
+ struct clk *vctcxo_clk;
+ struct clk *i2s0_clk;
+ struct clk *i2s1_clk;
+
+ struct mmp_audio_pll audio_pll;
+ struct mmp_audio_mux sspa_mux;
+ struct mmp_audio_mux sspa1_mux;
+ struct mmp_audio_div sysclk;
+ struct mmp_audio_div sspa0;
+ struct mmp_audio_div sspa1;
+
+ struct clk *clk_table[3];
+ struct clk_onecell_data clk_data;
+
+ spinlock_t lock;
+};
+
+static struct {
+ unsigned long parent_rate;
+ unsigned long freq_vco;
+ unsigned char mclk;
+ unsigned char fbcclk;
+ unsigned short fract;
+} predivs[] = {
+ { 26000000, 135475200, 0, 0, 0x8a18 },
+ { 26000000, 147456000, 0, 1, 0x0da1 },
+ { 38400000, 135475200, 1, 2, 0x8208 },
+ { 38400000, 147456000, 1, 3, 0xaaaa },
+};
+
+static struct {
+ unsigned char divisor;
+ unsigned char modulo;
+ unsigned char pattern;
+} postdivs[] = {
+ { 1, 3, 0, },
+ { 2, 5, 0, },
+ { 4, 0, 0, },
+ { 6, 1, 1, },
+ { 8, 1, 0, },
+ { 9, 1, 2, },
+ { 12, 2, 1, },
+ { 16, 2, 0, },
+ { 18, 2, 2, },
+ { 24, 4, 1, },
+ { 36, 4, 2, },
+ { 48, 6, 1, },
+ { 72, 6, 2, },
+};
+
+static unsigned long mmp_audio_pll_recalc_rate(struct clk_hw *hw,
+ unsigned long parent_rate)
+{
+ struct mmp_audio_pll *pll = to_mmp_audio_pll(hw);
+ unsigned int prediv;
+ unsigned int postdiv;
+
+ for (prediv = 0; prediv < ARRAY_SIZE(predivs); prediv++) {
+ if (predivs[prediv].parent_rate != parent_rate)
+ continue;
+ for (postdiv = 0; postdiv < ARRAY_SIZE(postdivs); postdiv++) {
+ unsigned long freq;
+ u32 val;
+
+ val = SSPA_AUD_PLL_CTRL0_ENA_DITHER;
+ val |= SSPA_AUD_PLL_CTRL0_PU;
+ val |= SSPA_AUD_PLL_CTRL0_DIV_OCLK_MODULO(postdivs[postdiv].modulo);
+ val |= SSPA_AUD_PLL_CTRL0_FRACT(predivs[prediv].fract);
+ val |= SSPA_AUD_PLL_CTRL0_DIV_FBCCLK(predivs[prediv].fbcclk);
+ val |= SSPA_AUD_PLL_CTRL0_DIV_MCLK(predivs[prediv].mclk);
+ if (val != pll->ctrl0)
+ continue;
+
+ val = SSPA_AUD_PLL_CTRL1_CLK_SEL_AUDIO_PLL;
+ val |= SSPA_AUD_PLL_CTRL1_DIV_OCLK_PATTERN(postdivs[postdiv].pattern);
+ if (val != pll->ctrl1)
+ continue;
+
+ freq = predivs[prediv].freq_vco;
+ freq /= postdivs[postdiv].divisor;
+ return freq;
+ }
+ }
+
+ return 0;
+}
+
+static long mmp_audio_pll_round_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long *parent_rate)
+{
+ unsigned int prediv;
+ unsigned int postdiv;
+ long rounded = 0;
+
+ for (prediv = 0; prediv < ARRAY_SIZE(predivs); prediv++) {
+ if (predivs[prediv].parent_rate != *parent_rate)
+ continue;
+ for (postdiv = 0; postdiv < ARRAY_SIZE(postdivs); postdiv++) {
+ long freq = predivs[prediv].freq_vco;
+
+ freq /= postdivs[postdiv].divisor;
+ if (freq == rate)
+ return rate;
+ if (freq < rate)
+ continue;
+ if (rounded && freq > rounded)
+ continue;
+ rounded = freq;
+ }
+ }
+
+ return rounded;
+}
+
+static void mmp_audio_pll_write_rate(struct mmp_audio_pll *pll)
+{
+ struct mmp2_audio_clk *priv = pll->priv;
+
+ __raw_writel(pll->ctrl0, priv->mmio_base + SSPA_AUD_PLL_CTRL0);
+ __raw_writel(pll->ctrl1, priv->mmio_base + SSPA_AUD_PLL_CTRL1);
+}
+
+static int mmp_audio_pll_set_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long parent_rate)
+{
+ struct mmp_audio_pll *pll = to_mmp_audio_pll(hw);
+ struct mmp2_audio_clk *priv = pll->priv;
+ unsigned int prediv;
+ unsigned int postdiv;
+
+ for (prediv = 0; prediv < ARRAY_SIZE(predivs); prediv++) {
+ if (predivs[prediv].parent_rate != parent_rate)
+ continue;
+
+ for (postdiv = 0; postdiv < ARRAY_SIZE(postdivs); postdiv++) {
+ if (rate * postdivs[postdiv].divisor != predivs[prediv].freq_vco)
+ continue;
+
+ pll->ctrl0 = SSPA_AUD_PLL_CTRL0_ENA_DITHER;
+ pll->ctrl0 |= SSPA_AUD_PLL_CTRL0_PU;
+ pll->ctrl0 |= SSPA_AUD_PLL_CTRL0_DIV_OCLK_MODULO(postdivs[postdiv].modulo);
+ pll->ctrl0 |= SSPA_AUD_PLL_CTRL0_FRACT(predivs[prediv].fract);
+ pll->ctrl0 |= SSPA_AUD_PLL_CTRL0_DIV_FBCCLK(predivs[prediv].fbcclk);
+ pll->ctrl0 |= SSPA_AUD_PLL_CTRL0_DIV_MCLK(predivs[prediv].mclk);
+
+ pll->ctrl1 = SSPA_AUD_PLL_CTRL1_CLK_SEL_AUDIO_PLL;
+ pll->ctrl1 |= SSPA_AUD_PLL_CTRL1_DIV_OCLK_PATTERN(postdivs[postdiv].pattern);
+
+ if (__clk_is_enabled(priv->audio_clk))
+ mmp_audio_pll_write_rate(pll);
+
+ return 0;
+ }
+ }
+
+ return -ERANGE;
+}
+
+static int mmp_audio_pll_enable(struct clk_hw *hw)
+{
+ struct mmp_audio_pll *pll = to_mmp_audio_pll(hw);
+ struct mmp2_audio_clk *priv = pll->priv;
+
+ clk_prepare_enable(priv->audio_clk);
+ mmp_audio_pll_write_rate(pll);
+ return 0;
+}
+
+static void mmp_audio_pll_disable(struct clk_hw *hw)
+{
+ struct mmp_audio_pll *pll = to_mmp_audio_pll(hw);
+ struct mmp2_audio_clk *priv = pll->priv;
+
+ clk_disable_unprepare(priv->audio_clk);
+}
+
+const struct clk_ops mmp_audio_pll_ops = {
+ .enable = mmp_audio_pll_enable,
+ .disable = mmp_audio_pll_disable,
+ .recalc_rate = mmp_audio_pll_recalc_rate,
+ .round_rate = mmp_audio_pll_round_rate,
+ .set_rate = mmp_audio_pll_set_rate,
+};
+
+static u8 mmp_audio_mux_get_parent(struct clk_hw *hw)
+{
+ struct mmp_audio_mux *mux = to_mmp_audio_mux(hw);
+ struct mmp2_audio_clk *priv = mux->priv;
+ u32 val;
+
+ if (__clk_is_enabled(priv->audio_clk)) {
+ val = __raw_readl(priv->mmio_base + SSPA_AUD_CTRL);
+ val >>= mux->shift;
+ val &= 1;
+ } else {
+ val = 0;
+ }
+ mux->index = val;
+
+ return mux->index;
+}
+
+static void mmp_audio_mux_write_parent(struct mmp_audio_mux *mux)
+{
+ struct mmp2_audio_clk *priv = mux->priv;
+ unsigned long flags = 0;
+ u32 reg;
+
+ spin_lock_irqsave(&priv->lock, flags);
+ reg = __raw_readl(priv->mmio_base + SSPA_AUD_CTRL);
+ reg &= ~(1 << mux->shift);
+ reg |= mux->index << mux->shift;
+ __raw_writel(reg, priv->mmio_base + SSPA_AUD_CTRL);
+ spin_unlock_irqrestore(&priv->lock, flags);
+}
+
+static int mmp_audio_mux_set_parent(struct clk_hw *hw, u8 index)
+{
+ struct mmp_audio_mux *mux = to_mmp_audio_mux(hw);
+ struct mmp2_audio_clk *priv = mux->priv;
+
+ mux->index = index;
+ if (__clk_is_enabled(priv->audio_clk))
+ mmp_audio_mux_write_parent(mux);
+
+ return 0;
+}
+
+static int mmp_audio_mux_determine_rate(struct clk_hw *hw,
+ struct clk_rate_request *req)
+{
+ return clk_mux_determine_rate_flags(hw, req, 0);
+}
+
+static int mmp_audio_mux_enable(struct clk_hw *hw)
+{
+ struct mmp_audio_mux *mux = to_mmp_audio_mux(hw);
+ struct mmp2_audio_clk *priv = mux->priv;
+
+ clk_prepare_enable(priv->audio_clk);
+ mmp_audio_mux_write_parent(mux);
+ return 0;
+}
+
+static void mmp_audio_mux_disable(struct clk_hw *hw)
+{
+ struct mmp_audio_mux *mux = to_mmp_audio_mux(hw);
+ struct mmp2_audio_clk *priv = mux->priv;
+
+ clk_disable_unprepare(priv->audio_clk);
+}
+
+const struct clk_ops mmp_audio_mux_ops = {
+ .enable = mmp_audio_mux_enable,
+ .disable = mmp_audio_mux_disable,
+
+ .get_parent = mmp_audio_mux_get_parent,
+ .set_parent = mmp_audio_mux_set_parent,
+ .determine_rate = mmp_audio_mux_determine_rate,
+};
+
+static unsigned long mmp_audio_div_recalc_rate(struct clk_hw *hw,
+ unsigned long parent_rate)
+{
+ struct mmp_audio_div *div = to_mmp_audio_div(hw);
+
+ if (div->value == 0)
+ return 0;
+
+ return divider_recalc_rate(hw, parent_rate, div->value, NULL,
+ CLK_DIVIDER_ONE_BASED |
+ CLK_DIVIDER_ROUND_CLOSEST |
+ CLK_DIVIDER_ALLOW_ZERO, 6);
+}
+
+static long mmp_audio_div_round_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long *prate)
+{
+ return divider_round_rate(hw, rate, prate, NULL, 6,
+ CLK_DIVIDER_ONE_BASED |
+ CLK_DIVIDER_ROUND_CLOSEST |
+ CLK_DIVIDER_ALLOW_ZERO);
+}
+
+static void mmp_audio_div_write_rate(struct mmp_audio_div *div, int enable)
+{
+ struct mmp2_audio_clk *priv = div->priv;
+ u32 val;
+
+ clk_prepare_enable(priv->audio_clk);
+ val = __raw_readl(priv->mmio_base + SSPA_AUD_CTRL);
+ val &= ~(SSPA_AUD_CTRL_DIV_MASK << div->shift);
+ val |= (u32)div->value << (div->shift + 1);
+ val |= enable << div->shift;
+ __raw_writel(val, priv->mmio_base + SSPA_AUD_CTRL);
+ clk_disable_unprepare(priv->audio_clk);
+}
+
+static int mmp_audio_div_enable(struct clk_hw *hw)
+{
+ struct mmp_audio_div *div = to_mmp_audio_div(hw);
+ struct mmp2_audio_clk *priv = div->priv;
+
+ clk_prepare_enable(priv->audio_clk);
+ mmp_audio_div_write_rate(div, 1);
+ return 0;
+}
+
+static void mmp_audio_div_disable(struct clk_hw *hw)
+{
+ struct mmp_audio_div *div = to_mmp_audio_div(hw);
+ struct mmp2_audio_clk *priv = div->priv;
+ unsigned long flags = 0;
+ u32 val;
+
+ spin_lock_irqsave(&priv->lock, flags);
+ val = __raw_readl(priv->mmio_base + SSPA_AUD_CTRL);
+ val &= ~(1 << div->shift);
+ __raw_writel(val, priv->mmio_base + SSPA_AUD_CTRL);
+ spin_unlock_irqrestore(&priv->lock, flags);
+ clk_disable_unprepare(priv->audio_clk);
+}
+
+static int mmp_audio_div_set_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long parent_rate)
+{
+ struct mmp_audio_div *div = to_mmp_audio_div(hw);
+ struct mmp2_audio_clk *priv = div->priv;
+ int value;
+
+ if (rate == 0) {
+ value = 0;
+ } else {
+ value = divider_get_val(rate, parent_rate, NULL, 6,
+ CLK_DIVIDER_ONE_BASED |
+ CLK_DIVIDER_ROUND_CLOSEST |
+ CLK_DIVIDER_ALLOW_ZERO);
+ if (value < 0)
+ return value;
+ }
+
+ div->value = value;
+ if (__clk_is_enabled(priv->audio_clk))
+ mmp_audio_div_write_rate(div, 0);
+
+ return 0;
+}
+
+const struct clk_ops mmp_audio_div_ops = {
+ .enable = mmp_audio_div_enable,
+ .disable = mmp_audio_div_disable,
+
+ .recalc_rate = mmp_audio_div_recalc_rate,
+ .round_rate = mmp_audio_div_round_rate,
+ .set_rate = mmp_audio_div_set_rate,
+};
+
+static int mmp2_audio_clk_probe(struct platform_device *pdev)
+{
+ const struct clk_hw *sspa_mux_parents[2];
+ const struct clk_hw *sspa1_mux_parents[2];
+ struct mmp2_audio_clk *priv;
+ int ret;
+
+ priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ spin_lock_init(&priv->lock);
+
+ priv->mmio_base = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(priv->mmio_base))
+ return PTR_ERR(priv->mmio_base);
+
+ priv->audio_clk = devm_clk_get(&pdev->dev, "audio");
+ if (IS_ERR(priv->audio_clk))
+ return PTR_ERR(priv->audio_clk);
+
+ priv->i2s0_clk = devm_clk_get(&pdev->dev, "i2s0");
+ if (IS_ERR(priv->i2s0_clk))
+ return PTR_ERR(priv->i2s0_clk);
+
+ priv->i2s1_clk = devm_clk_get(&pdev->dev, "i2s1");
+ if (IS_ERR(priv->i2s1_clk))
+ return PTR_ERR(priv->i2s1_clk);
+
+ priv->vctcxo_clk = devm_clk_get(&pdev->dev, "vctcxo");
+ if (IS_ERR(priv->vctcxo_clk))
+ return PTR_ERR(priv->vctcxo_clk);
+
+ priv->audio_pll.hw.init =
+ CLK_HW_INIT_HW("audio_pll", __clk_get_hw(priv->vctcxo_clk),
+ &mmp_audio_pll_ops, CLK_SET_RATE_PARENT);
+ priv->audio_pll.priv = priv;
+ ret = devm_clk_hw_register(&pdev->dev, &priv->audio_pll.hw);
+ if (ret)
+ return ret;
+
+ sspa_mux_parents[0] = &priv->audio_pll.hw;
+ sspa_mux_parents[1] = __clk_get_hw(priv->i2s0_clk);
+ priv->sspa_mux.hw.init =
+ CLK_HW_INIT_PARENTS_HW("sspa_mux", sspa_mux_parents,
+ &mmp_audio_mux_ops, CLK_SET_RATE_PARENT);
+ priv->sspa_mux.priv = priv;
+ priv->sspa_mux.shift = SSPA_AUD_CTRL_SSPA0_MUX_SHIFT;
+ ret = devm_clk_hw_register(&pdev->dev, &priv->sspa_mux.hw);
+ if (ret)
+ return ret;
+
+ priv->sysclk.hw.init =
+ CLK_HW_INIT_HW("sysclk", &priv->sspa_mux.hw,
+ &mmp_audio_div_ops, CLK_SET_RATE_PARENT);
+ priv->sysclk.priv = priv;
+ priv->sysclk.shift = SSPA_AUD_CTRL_SYSCLK_SHIFT;
+ ret = devm_clk_hw_register(&pdev->dev, &priv->sysclk.hw);
+ if (ret)
+ return ret;
+
+ priv->sspa0.hw.init =
+ CLK_HW_INIT_HW("sspa0", &priv->sspa_mux.hw,
+ &mmp_audio_div_ops, 0);
+ priv->sspa0.priv = priv;
+ priv->sspa0.shift = SSPA_AUD_CTRL_SSPA0_SHIFT;
+ ret = devm_clk_hw_register(&pdev->dev, &priv->sspa0.hw);
+ if (ret)
+ return ret;
+
+ sspa1_mux_parents[0] = &priv->audio_pll.hw;
+ sspa1_mux_parents[1] = __clk_get_hw(priv->i2s1_clk);
+ priv->sspa1_mux.hw.init =
+ CLK_HW_INIT_PARENTS_HW("sspa1_mux", sspa1_mux_parents,
+ &mmp_audio_mux_ops, CLK_SET_RATE_PARENT);
+ priv->sspa1_mux.priv = priv;
+ priv->sspa1_mux.shift = SSPA_AUD_CTRL_SSPA1_MUX_SHIFT;
+ ret = devm_clk_hw_register(&pdev->dev, &priv->sspa1_mux.hw);
+ if (ret)
+ return ret;
+
+ priv->sspa1.hw.init =
+ CLK_HW_INIT_HW("sspa1", &priv->sspa1_mux.hw,
+ &mmp_audio_div_ops, 0);
+ priv->sspa1.priv = priv;
+ priv->sspa1.shift = SSPA_AUD_CTRL_SSPA1_SHIFT;
+ ret = devm_clk_hw_register(&pdev->dev, &priv->sspa1.hw);
+ if (ret)
+ return ret;
+
+ priv->clk_table[MMP2_CLK_AUDIO_SYSCLK] = priv->sysclk.hw.clk;
+ priv->clk_table[MMP2_CLK_AUDIO_SSPA0] = priv->sspa0.hw.clk;
+ priv->clk_table[MMP2_CLK_AUDIO_SSPA1] = priv->sspa1.hw.clk;
+ priv->clk_data.clks = priv->clk_table;
+ priv->clk_data.clk_num = ARRAY_SIZE(priv->clk_table);
+
+ return of_clk_add_provider(pdev->dev.of_node, of_clk_src_onecell_get,
+ &priv->clk_data);
+}
+
+static const struct of_device_id mmp2_audio_clk_of_match[] = {
+ { .compatible = "marvell,mmp2-audio-clock" },
+ {},
+};
+
+MODULE_DEVICE_TABLE(of, mmp2_audio_clk_of_match);
+
+static struct platform_driver mmp2_audio_clk_driver = {
+ .driver = {
+ .name = "mmp2-audio-clock",
+ .of_match_table = of_match_ptr(mmp2_audio_clk_of_match),
+ },
+ .probe = mmp2_audio_clk_probe,
+};
+
+module_platform_driver(mmp2_audio_clk_driver);
+
+MODULE_AUTHOR("Lubomir Rintel <lkundrak@v3.sk>");
+MODULE_DESCRIPTION("Clock driver for MMP2 Audio subsystem");
+MODULE_LICENSE("GPL");
--
2.26.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] dt-bindings: sound: Add Marvell MMP Audio Clock Controller binding
2020-05-11 19:55 ` [PATCH 1/2] dt-bindings: sound: Add Marvell MMP Audio Clock Controller binding Lubomir Rintel
@ 2020-05-12 1:41 ` Rob Herring
2020-05-14 21:06 ` Stephen Boyd
1 sibling, 0 replies; 7+ messages in thread
From: Rob Herring @ 2020-05-12 1:41 UTC (permalink / raw)
To: Lubomir Rintel
Cc: Michael Turquette, linux-kernel, Stephen Boyd, devicetree,
linux-media, linux-clk, Liam Girdwood, Rob Herring, Mark Brown
On Mon, 11 May 2020 21:55:33 +0200, Lubomir Rintel wrote:
> This describes the bindings for a controller that generates master and bit
> clocks for the I2S interface.
>
> Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
> ---
> .../clock/marvell,mmp2-audio-clock.yaml | 73 +++++++++++++++++++
> .../dt-bindings/clock/marvell,mmp2-audio.h | 8 ++
> 2 files changed, 81 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/clock/marvell,mmp2-audio-clock.yaml
> create mode 100644 include/dt-bindings/clock/marvell,mmp2-audio.h
>
My bot found errors running 'make dt_binding_check' on your patch:
Documentation/devicetree/bindings/clock/marvell,mmp2-audio-clock.example.dts:20:18: fatal error: dt-bindings/power/marvell,mmp2.h: No such file or directory
#include <dt-bindings/power/marvell,mmp2.h>
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
scripts/Makefile.lib:312: recipe for target 'Documentation/devicetree/bindings/clock/marvell,mmp2-audio-clock.example.dt.yaml' failed
make[1]: *** [Documentation/devicetree/bindings/clock/marvell,mmp2-audio-clock.example.dt.yaml] Error 1
make[1]: *** Waiting for unfinished jobs....
Makefile:1300: recipe for target 'dt_binding_check' failed
make: *** [dt_binding_check] Error 2
See https://patchwork.ozlabs.org/patch/1288040
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure dt-schema is up to date:
pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade
Please check and re-submit.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] dt-bindings: sound: Add Marvell MMP Audio Clock Controller binding
2020-05-11 19:55 ` [PATCH 1/2] dt-bindings: sound: Add Marvell MMP Audio Clock Controller binding Lubomir Rintel
2020-05-12 1:41 ` Rob Herring
@ 2020-05-14 21:06 ` Stephen Boyd
2020-05-19 22:21 ` Lubomir Rintel
1 sibling, 1 reply; 7+ messages in thread
From: Stephen Boyd @ 2020-05-14 21:06 UTC (permalink / raw)
To: Lubomir Rintel, Michael Turquette
Cc: Liam Girdwood, Mark Brown, Rob Herring, linux-clk, devicetree,
linux-kernel, linux-media, Lubomir Rintel
Quoting Lubomir Rintel (2020-05-11 12:55:33)
> diff --git a/Documentation/devicetree/bindings/clock/marvell,mmp2-audio-clock.yaml b/Documentation/devicetree/bindings/clock/marvell,mmp2-audio-clock.yaml
> new file mode 100644
> index 000000000000..b86e9fbfa56d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/marvell,mmp2-audio-clock.yaml
> @@ -0,0 +1,73 @@
> +# SPDX-License-Identifier: (GPL-2.0+ OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/marvell,mmp2-audio-clock.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Marvell MMP2 Audio Clock Controller
> +
> +maintainers:
> + - Lubomir Rintel <lkundrak@v3.sk>
> +
> +description: |
> + The audio clock controller generates and supplies the clocks to the audio
> + codec.
> +
> + Each clock is assigned an identifier and client nodes use this identifier
> + to specify the clock which they consume.
> +
> + All these identifiers could be found in <dt-bindings/clock/marvell,mmp2.h>.
Is this right? The patch puts them in mmp2-audio.h
> +
> +properties:
> + compatible:
> + enum:
> + - marvell,mmp2-audio-clock
> +
> + reg:
> + maxItems: 1
> +
> + clocks:
> + items:
> + - description: Audio subsystem clock
> + - description: The crystal oscillator clock
> + - description: First I2S clock
> + - description: Second I2S clock
> +
> + clock-names:
> + items:
> + - const: audio
> + - const: vctcxo
> + - const: i2s0
> + - const: i2s1
> +
> + '#clock-cells':
> + const: 1
> +
> + power-domains:
> + maxItems: 1
> +
> +required:
> + - compatible
> + - reg
> + - clocks
> + - clock-names
> + - '#clock-cells'
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/clock/marvell,mmp2.h>
> + #include <dt-bindings/power/marvell,mmp2.h>
> +
> + clocks@d42a0c30 {
clock-controller@d42a0c30
> + compatible = "marvell,mmp2-audio-clock";
> + reg = <0xd42a0c30 0x10>;
That is a very specific and tiny region. Presumably this is part of a
larger hardware block and thus shouldn't be described in DT this way.
Instead there should be one clock-controller node and a driver that
controls all the clks that it wants to inside that hardware block.
> + clock-names = "audio", "vctcxo", "i2s0", "i2s1";
> + clocks = <&soc_clocks MMP2_CLK_AUDIO>,
> + <&soc_clocks MMP2_CLK_VCTCXO>,
> + <&soc_clocks MMP2_CLK_I2S0>,
> + <&soc_clocks MMP2_CLK_I2S1>;
> + power-domains = <&soc_clocks MMP2_POWER_DOMAIN_AUDIO>;
> + #clock-cells = <1>;
> + };
> diff --git a/include/dt-bindings/clock/marvell,mmp2-audio.h b/include/dt-bindings/clock/marvell,mmp2-audio.h
> new file mode 100644
> index 000000000000..127b48ec0f0a
> --- /dev/null
> +++ b/include/dt-bindings/clock/marvell,mmp2-audio.h
> @@ -0,0 +1,8 @@
> +/* SPDX-License-Identifier: (GPL-2.0+ OR BSD-2-Clause) */
> +#ifndef __DT_BINDINGS_CLOCK_MARVELL_MMP2_AUDIO_H
> +#define __DT_BINDINGS_CLOCK_MARVELL_MMP2_AUDIO_H
> +
> +#define MMP2_CLK_AUDIO_SYSCLK 1
Any reason to start at 1 vs. 0?
> +#define MMP2_CLK_AUDIO_SSPA0 2
> +#define MMP2_CLK_AUDIO_SSPA1 3
> +#endif
> --
> 2.26.2
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] clk: mmp2: Add audio clock controller driver
2020-05-11 19:55 ` [PATCH 2/2] clk: mmp2: Add audio clock controller driver Lubomir Rintel
@ 2020-05-14 21:15 ` Stephen Boyd
0 siblings, 0 replies; 7+ messages in thread
From: Stephen Boyd @ 2020-05-14 21:15 UTC (permalink / raw)
To: Lubomir Rintel, Michael Turquette
Cc: Liam Girdwood, Mark Brown, Rob Herring, linux-clk, devicetree,
linux-kernel, linux-media, Lubomir Rintel
Quoting Lubomir Rintel (2020-05-11 12:55:34)
> diff --git a/drivers/clk/mmp/clk-audio.c b/drivers/clk/mmp/clk-audio.c
> new file mode 100644
> index 000000000000..ee89b97dc09a
> --- /dev/null
> +++ b/drivers/clk/mmp/clk-audio.c
> @@ -0,0 +1,563 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * MMP Audio Clock Controller driver
> + *
> + * Copyright (C) 2020 Lubomir Rintel <lkundrak@v3.sk>
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/clk-provider.h>
Preferably this isn't a clk consumer and a clk provider. If a clk is
needed to read/write registers then look at using pm_clk code to make
this driver runtime PM aware and turn on clks when necessary.
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <dt-bindings/clock/marvell,mmp2-audio.h>
> +
> +enum {
> + SSPA_AUD_CTRL = 0x04,
> + SSPA_AUD_PLL_CTRL0 = 0x08,
> + SSPA_AUD_PLL_CTRL1 = 0x0c,
> +};
Just make defines instead of enum please.
> +
> +/* SSPA Audio Control Register */
> +#define SSPA_AUD_CTRL_SYSCLK_SHIFT 0
> +#define SSPA_AUD_CTRL_SSPA0_MUX_SHIFT 7
> +#define SSPA_AUD_CTRL_SSPA0_SHIFT 8
> +#define SSPA_AUD_CTRL_SSPA1_SHIFT 16
> +#define SSPA_AUD_CTRL_SSPA1_MUX_SHIFT 23
> +#define SSPA_AUD_CTRL_DIV_MASK 0x7e
> +
> +/* SSPA Audio PLL Control 0 Register */
> +#define SSPA_AUD_PLL_CTRL0_DIV_OCLK_MODULO_MASK (0x7 << 28)
> +#define SSPA_AUD_PLL_CTRL0_DIV_OCLK_MODULO(x) ((x) << 28)
> +#define SSPA_AUD_PLL_CTRL0_FRACT_MASK (0xfffff << 8)
> +#define SSPA_AUD_PLL_CTRL0_FRACT(x) ((x) << 8)
> +#define SSPA_AUD_PLL_CTRL0_ENA_DITHER (1 << 7)
> +#define SSPA_AUD_PLL_CTRL0_ICP_2UA (0 << 5)
> +#define SSPA_AUD_PLL_CTRL0_ICP_5UA (1 << 5)
> +#define SSPA_AUD_PLL_CTRL0_ICP_7UA (2 << 5)
> +#define SSPA_AUD_PLL_CTRL0_ICP_10UA (3 << 5)
> +#define SSPA_AUD_PLL_CTRL0_DIV_FBCCLK_MASK (0x3 << 3)
> +#define SSPA_AUD_PLL_CTRL0_DIV_FBCCLK(x) ((x) << 3)
> +#define SSPA_AUD_PLL_CTRL0_DIV_MCLK_MASK (0x1 << 2)
> +#define SSPA_AUD_PLL_CTRL0_DIV_MCLK(x) ((x) << 2)
> +#define SSPA_AUD_PLL_CTRL0_PD_OVPROT_DIS (1 << 1)
> +#define SSPA_AUD_PLL_CTRL0_PU (1 << 0)
> +
> +/* SSPA Audio PLL Control 1 Register */
> +#define SSPA_AUD_PLL_CTRL1_SEL_FAST_CLK (1 << 24)
> +#define SSPA_AUD_PLL_CTRL1_CLK_SEL_MASK (1 << 11)
> +#define SSPA_AUD_PLL_CTRL1_CLK_SEL_AUDIO_PLL (1 << 11)
> +#define SSPA_AUD_PLL_CTRL1_CLK_SEL_VCXO (0 << 11)
> +#define SSPA_AUD_PLL_CTRL1_DIV_OCLK_PATTERN_MASK (0x7ff << 0)
> +#define SSPA_AUD_PLL_CTRL1_DIV_OCLK_PATTERN(x) ((x) << 0)
> +
> +struct mmp_audio_mux {
> + struct clk_hw hw;
> + struct mmp2_audio_clk *priv;
> + u8 shift;
> + u8 flags;
> + int index;
unsigned int index?
> +};
> +
> +#define to_mmp_audio_mux(_hw) container_of(_hw, struct mmp_audio_mux, hw)
> +
> +struct mmp_audio_div {
> + struct clk_hw hw;
> + struct mmp2_audio_clk *priv;
> + u8 shift;
> + int value;
unsigned int value?
> +};
> +
> +#define to_mmp_audio_div(_hw) container_of(_hw, struct mmp_audio_div, hw)
> +
> +struct mmp_audio_pll {
> + struct clk_hw hw;
> + struct mmp2_audio_clk *priv;
> + u32 ctrl0;
> + u32 ctrl1;
> +};
> +
> +#define to_mmp_audio_pll(_hw) container_of(_hw, struct mmp_audio_pll, hw)
> +
> +struct mmp2_audio_clk {
> + void __iomem *mmio_base;
> +
> + struct clk *audio_clk;
> + struct clk *vctcxo_clk;
> + struct clk *i2s0_clk;
> + struct clk *i2s1_clk;
> +
> + struct mmp_audio_pll audio_pll;
> + struct mmp_audio_mux sspa_mux;
> + struct mmp_audio_mux sspa1_mux;
> + struct mmp_audio_div sysclk;
> + struct mmp_audio_div sspa0;
> + struct mmp_audio_div sspa1;
> +
> + struct clk *clk_table[3];
> + struct clk_onecell_data clk_data;
> +
> + spinlock_t lock;
> +};
> +
> +static struct {
Can this be const?
> + unsigned long parent_rate;
> + unsigned long freq_vco;
> + unsigned char mclk;
> + unsigned char fbcclk;
> + unsigned short fract;
> +} predivs[] = {
> + { 26000000, 135475200, 0, 0, 0x8a18 },
> + { 26000000, 147456000, 0, 1, 0x0da1 },
> + { 38400000, 135475200, 1, 2, 0x8208 },
> + { 38400000, 147456000, 1, 3, 0xaaaa },
> +};
> +
> +static struct {
Can this be const?
> + unsigned char divisor;
> + unsigned char modulo;
> + unsigned char pattern;
> +} postdivs[] = {
> + { 1, 3, 0, },
> + { 2, 5, 0, },
> + { 4, 0, 0, },
> + { 6, 1, 1, },
> + { 8, 1, 0, },
> + { 9, 1, 2, },
> + { 12, 2, 1, },
> + { 16, 2, 0, },
> + { 18, 2, 2, },
> + { 24, 4, 1, },
> + { 36, 4, 2, },
> + { 48, 6, 1, },
> + { 72, 6, 2, },
> +};
> +
> +static unsigned long mmp_audio_pll_recalc_rate(struct clk_hw *hw,
> + unsigned long parent_rate)
> +{
> + struct mmp_audio_pll *pll = to_mmp_audio_pll(hw);
> + unsigned int prediv;
> + unsigned int postdiv;
> +
> + for (prediv = 0; prediv < ARRAY_SIZE(predivs); prediv++) {
> + if (predivs[prediv].parent_rate != parent_rate)
> + continue;
> + for (postdiv = 0; postdiv < ARRAY_SIZE(postdivs); postdiv++) {
> + unsigned long freq;
> + u32 val;
> +
> + val = SSPA_AUD_PLL_CTRL0_ENA_DITHER;
> + val |= SSPA_AUD_PLL_CTRL0_PU;
> + val |= SSPA_AUD_PLL_CTRL0_DIV_OCLK_MODULO(postdivs[postdiv].modulo);
> + val |= SSPA_AUD_PLL_CTRL0_FRACT(predivs[prediv].fract);
> + val |= SSPA_AUD_PLL_CTRL0_DIV_FBCCLK(predivs[prediv].fbcclk);
> + val |= SSPA_AUD_PLL_CTRL0_DIV_MCLK(predivs[prediv].mclk);
> + if (val != pll->ctrl0)
> + continue;
> +
> + val = SSPA_AUD_PLL_CTRL1_CLK_SEL_AUDIO_PLL;
> + val |= SSPA_AUD_PLL_CTRL1_DIV_OCLK_PATTERN(postdivs[postdiv].pattern);
> + if (val != pll->ctrl1)
> + continue;
> +
> + freq = predivs[prediv].freq_vco;
> + freq /= postdivs[postdiv].divisor;
> + return freq;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static long mmp_audio_pll_round_rate(struct clk_hw *hw, unsigned long rate,
> + unsigned long *parent_rate)
> +{
> + unsigned int prediv;
> + unsigned int postdiv;
> + long rounded = 0;
> +
> + for (prediv = 0; prediv < ARRAY_SIZE(predivs); prediv++) {
> + if (predivs[prediv].parent_rate != *parent_rate)
> + continue;
> + for (postdiv = 0; postdiv < ARRAY_SIZE(postdivs); postdiv++) {
> + long freq = predivs[prediv].freq_vco;
> +
> + freq /= postdivs[postdiv].divisor;
> + if (freq == rate)
> + return rate;
> + if (freq < rate)
> + continue;
> + if (rounded && freq > rounded)
> + continue;
> + rounded = freq;
> + }
> + }
> +
> + return rounded;
> +}
> +
> +static void mmp_audio_pll_write_rate(struct mmp_audio_pll *pll)
> +{
> + struct mmp2_audio_clk *priv = pll->priv;
> +
> + __raw_writel(pll->ctrl0, priv->mmio_base + SSPA_AUD_PLL_CTRL0);
> + __raw_writel(pll->ctrl1, priv->mmio_base + SSPA_AUD_PLL_CTRL1);
> +}
> +
> +static int mmp_audio_pll_set_rate(struct clk_hw *hw, unsigned long rate,
> + unsigned long parent_rate)
> +{
> + struct mmp_audio_pll *pll = to_mmp_audio_pll(hw);
> + struct mmp2_audio_clk *priv = pll->priv;
> + unsigned int prediv;
> + unsigned int postdiv;
> +
> + for (prediv = 0; prediv < ARRAY_SIZE(predivs); prediv++) {
> + if (predivs[prediv].parent_rate != parent_rate)
> + continue;
> +
> + for (postdiv = 0; postdiv < ARRAY_SIZE(postdivs); postdiv++) {
> + if (rate * postdivs[postdiv].divisor != predivs[prediv].freq_vco)
> + continue;
> +
> + pll->ctrl0 = SSPA_AUD_PLL_CTRL0_ENA_DITHER;
> + pll->ctrl0 |= SSPA_AUD_PLL_CTRL0_PU;
> + pll->ctrl0 |= SSPA_AUD_PLL_CTRL0_DIV_OCLK_MODULO(postdivs[postdiv].modulo);
> + pll->ctrl0 |= SSPA_AUD_PLL_CTRL0_FRACT(predivs[prediv].fract);
> + pll->ctrl0 |= SSPA_AUD_PLL_CTRL0_DIV_FBCCLK(predivs[prediv].fbcclk);
> + pll->ctrl0 |= SSPA_AUD_PLL_CTRL0_DIV_MCLK(predivs[prediv].mclk);
> +
> + pll->ctrl1 = SSPA_AUD_PLL_CTRL1_CLK_SEL_AUDIO_PLL;
> + pll->ctrl1 |= SSPA_AUD_PLL_CTRL1_DIV_OCLK_PATTERN(postdivs[postdiv].pattern);
> +
> + if (__clk_is_enabled(priv->audio_clk))
> + mmp_audio_pll_write_rate(pll);
> +
> + return 0;
> + }
> + }
> +
> + return -ERANGE;
> +}
> +
> +static int mmp_audio_pll_enable(struct clk_hw *hw)
> +{
> + struct mmp_audio_pll *pll = to_mmp_audio_pll(hw);
> + struct mmp2_audio_clk *priv = pll->priv;
> +
> + clk_prepare_enable(priv->audio_clk);
> + mmp_audio_pll_write_rate(pll);
> + return 0;
> +}
> +
> +static void mmp_audio_pll_disable(struct clk_hw *hw)
> +{
> + struct mmp_audio_pll *pll = to_mmp_audio_pll(hw);
> + struct mmp2_audio_clk *priv = pll->priv;
> +
> + clk_disable_unprepare(priv->audio_clk);
> +}
> +
> +const struct clk_ops mmp_audio_pll_ops = {
> + .enable = mmp_audio_pll_enable,
> + .disable = mmp_audio_pll_disable,
> + .recalc_rate = mmp_audio_pll_recalc_rate,
> + .round_rate = mmp_audio_pll_round_rate,
> + .set_rate = mmp_audio_pll_set_rate,
> +};
> +
> +static u8 mmp_audio_mux_get_parent(struct clk_hw *hw)
> +{
> + struct mmp_audio_mux *mux = to_mmp_audio_mux(hw);
> + struct mmp2_audio_clk *priv = mux->priv;
> + u32 val;
> +
> + if (__clk_is_enabled(priv->audio_clk)) {
> + val = __raw_readl(priv->mmio_base + SSPA_AUD_CTRL);
> + val >>= mux->shift;
> + val &= 1;
> + } else {
> + val = 0;
> + }
> + mux->index = val;
> +
> + return mux->index;
> +}
> +
> +static void mmp_audio_mux_write_parent(struct mmp_audio_mux *mux)
> +{
> + struct mmp2_audio_clk *priv = mux->priv;
> + unsigned long flags = 0;
> + u32 reg;
> +
> + spin_lock_irqsave(&priv->lock, flags);
> + reg = __raw_readl(priv->mmio_base + SSPA_AUD_CTRL);
> + reg &= ~(1 << mux->shift);
> + reg |= mux->index << mux->shift;
> + __raw_writel(reg, priv->mmio_base + SSPA_AUD_CTRL);
> + spin_unlock_irqrestore(&priv->lock, flags);
> +}
> +
> +static int mmp_audio_mux_set_parent(struct clk_hw *hw, u8 index)
> +{
> + struct mmp_audio_mux *mux = to_mmp_audio_mux(hw);
> + struct mmp2_audio_clk *priv = mux->priv;
> +
> + mux->index = index;
> + if (__clk_is_enabled(priv->audio_clk))
> + mmp_audio_mux_write_parent(mux);
> +
> + return 0;
> +}
> +
> +static int mmp_audio_mux_determine_rate(struct clk_hw *hw,
> + struct clk_rate_request *req)
> +{
> + return clk_mux_determine_rate_flags(hw, req, 0);
> +}
> +
> +static int mmp_audio_mux_enable(struct clk_hw *hw)
> +{
> + struct mmp_audio_mux *mux = to_mmp_audio_mux(hw);
> + struct mmp2_audio_clk *priv = mux->priv;
> +
> + clk_prepare_enable(priv->audio_clk);
> + mmp_audio_mux_write_parent(mux);
> + return 0;
> +}
> +
> +static void mmp_audio_mux_disable(struct clk_hw *hw)
> +{
> + struct mmp_audio_mux *mux = to_mmp_audio_mux(hw);
> + struct mmp2_audio_clk *priv = mux->priv;
> +
> + clk_disable_unprepare(priv->audio_clk);
> +}
> +
> +const struct clk_ops mmp_audio_mux_ops = {
static?
> + .enable = mmp_audio_mux_enable,
> + .disable = mmp_audio_mux_disable,
> +
> + .get_parent = mmp_audio_mux_get_parent,
> + .set_parent = mmp_audio_mux_set_parent,
> + .determine_rate = mmp_audio_mux_determine_rate,
> +};
> +
[...]
> +
> +static void mmp_audio_div_write_rate(struct mmp_audio_div *div, int enable)
> +{
> + struct mmp2_audio_clk *priv = div->priv;
> + u32 val;
> +
> + clk_prepare_enable(priv->audio_clk);
> + val = __raw_readl(priv->mmio_base + SSPA_AUD_CTRL);
Please don't use __raw IO accessors as they don't do proper endian
swaps and are thus not portable. Use the relaxed accessors if you're
trying to avoid the barrier semantics, but most likely that is overkill
too and can just be readl()/writel().
> + val &= ~(SSPA_AUD_CTRL_DIV_MASK << div->shift);
> + val |= (u32)div->value << (div->shift + 1);
Why cast to u32? Should it be a u32 already?
> + val |= enable << div->shift;
> + __raw_writel(val, priv->mmio_base + SSPA_AUD_CTRL);
> + clk_disable_unprepare(priv->audio_clk);
> +}
> +
> +static int mmp_audio_div_enable(struct clk_hw *hw)
> +{
> + struct mmp_audio_div *div = to_mmp_audio_div(hw);
> + struct mmp2_audio_clk *priv = div->priv;
> +
> + clk_prepare_enable(priv->audio_clk);
What is this clk being enabled and prepared for? Is it needed to access
registers inside the device?
> + mmp_audio_div_write_rate(div, 1);
> + return 0;
> +}
> +
> +static void mmp_audio_div_disable(struct clk_hw *hw)
> +{
> + struct mmp_audio_div *div = to_mmp_audio_div(hw);
> + struct mmp2_audio_clk *priv = div->priv;
> + unsigned long flags = 0;
Please drop assignment to 0.
> + u32 val;
> +
> + spin_lock_irqsave(&priv->lock, flags);
> + val = __raw_readl(priv->mmio_base + SSPA_AUD_CTRL);
> + val &= ~(1 << div->shift);
> + __raw_writel(val, priv->mmio_base + SSPA_AUD_CTRL);
> + spin_unlock_irqrestore(&priv->lock, flags);
> + clk_disable_unprepare(priv->audio_clk);
> +}
> +
> +static int mmp_audio_div_set_rate(struct clk_hw *hw, unsigned long rate,
> + unsigned long parent_rate)
> +{
> + struct mmp_audio_div *div = to_mmp_audio_div(hw);
> + struct mmp2_audio_clk *priv = div->priv;
> + int value;
> +
> + if (rate == 0) {
> + value = 0;
> + } else {
> + value = divider_get_val(rate, parent_rate, NULL, 6,
> + CLK_DIVIDER_ONE_BASED |
> + CLK_DIVIDER_ROUND_CLOSEST |
> + CLK_DIVIDER_ALLOW_ZERO);
> + if (value < 0)
> + return value;
> + }
> +
> + div->value = value;
> + if (__clk_is_enabled(priv->audio_clk))
> + mmp_audio_div_write_rate(div, 0);
> +
> + return 0;
> +}
> +
> +const struct clk_ops mmp_audio_div_ops = {
static?
> + .enable = mmp_audio_div_enable,
> + .disable = mmp_audio_div_disable,
> +
> + .recalc_rate = mmp_audio_div_recalc_rate,
> + .round_rate = mmp_audio_div_round_rate,
> + .set_rate = mmp_audio_div_set_rate,
> +};
> +
> +static int mmp2_audio_clk_probe(struct platform_device *pdev)
> +{
> + const struct clk_hw *sspa_mux_parents[2];
> + const struct clk_hw *sspa1_mux_parents[2];
> + struct mmp2_audio_clk *priv;
> + int ret;
> +
> + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + spin_lock_init(&priv->lock);
> +
> + priv->mmio_base = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(priv->mmio_base))
> + return PTR_ERR(priv->mmio_base);
> +
> + priv->audio_clk = devm_clk_get(&pdev->dev, "audio");
> + if (IS_ERR(priv->audio_clk))
> + return PTR_ERR(priv->audio_clk);
> +
> + priv->i2s0_clk = devm_clk_get(&pdev->dev, "i2s0");
> + if (IS_ERR(priv->i2s0_clk))
> + return PTR_ERR(priv->i2s0_clk);
> +
> + priv->i2s1_clk = devm_clk_get(&pdev->dev, "i2s1");
> + if (IS_ERR(priv->i2s1_clk))
> + return PTR_ERR(priv->i2s1_clk);
> +
> + priv->vctcxo_clk = devm_clk_get(&pdev->dev, "vctcxo");
> + if (IS_ERR(priv->vctcxo_clk))
> + return PTR_ERR(priv->vctcxo_clk);
> +
> + priv->audio_pll.hw.init =
> + CLK_HW_INIT_HW("audio_pll", __clk_get_hw(priv->vctcxo_clk),
Usage of __clk_get_hw() is probably wrong here. Are these clks provided
outside this node in DT? Please specify them as .fw_name parents so clk
core can match up clks to their parents at runtime.
> + &mmp_audio_pll_ops, CLK_SET_RATE_PARENT);
> + priv->audio_pll.priv = priv;
> + ret = devm_clk_hw_register(&pdev->dev, &priv->audio_pll.hw);
> + if (ret)
> + return ret;
> +
> + sspa_mux_parents[0] = &priv->audio_pll.hw;
> + sspa_mux_parents[1] = __clk_get_hw(priv->i2s0_clk);
> + priv->sspa_mux.hw.init =
> + CLK_HW_INIT_PARENTS_HW("sspa_mux", sspa_mux_parents,
> + &mmp_audio_mux_ops, CLK_SET_RATE_PARENT);
> + priv->sspa_mux.priv = priv;
> + priv->sspa_mux.shift = SSPA_AUD_CTRL_SSPA0_MUX_SHIFT;
> + ret = devm_clk_hw_register(&pdev->dev, &priv->sspa_mux.hw);
> + if (ret)
> + return ret;
> +
> + priv->sysclk.hw.init =
> + CLK_HW_INIT_HW("sysclk", &priv->sspa_mux.hw,
> + &mmp_audio_div_ops, CLK_SET_RATE_PARENT);
> + priv->sysclk.priv = priv;
> + priv->sysclk.shift = SSPA_AUD_CTRL_SYSCLK_SHIFT;
> + ret = devm_clk_hw_register(&pdev->dev, &priv->sysclk.hw);
> + if (ret)
> + return ret;
> +
> + priv->sspa0.hw.init =
> + CLK_HW_INIT_HW("sspa0", &priv->sspa_mux.hw,
> + &mmp_audio_div_ops, 0);
> + priv->sspa0.priv = priv;
> + priv->sspa0.shift = SSPA_AUD_CTRL_SSPA0_SHIFT;
> + ret = devm_clk_hw_register(&pdev->dev, &priv->sspa0.hw);
> + if (ret)
> + return ret;
> +
> + sspa1_mux_parents[0] = &priv->audio_pll.hw;
> + sspa1_mux_parents[1] = __clk_get_hw(priv->i2s1_clk);
> + priv->sspa1_mux.hw.init =
> + CLK_HW_INIT_PARENTS_HW("sspa1_mux", sspa1_mux_parents,
> + &mmp_audio_mux_ops, CLK_SET_RATE_PARENT);
> + priv->sspa1_mux.priv = priv;
> + priv->sspa1_mux.shift = SSPA_AUD_CTRL_SSPA1_MUX_SHIFT;
> + ret = devm_clk_hw_register(&pdev->dev, &priv->sspa1_mux.hw);
> + if (ret)
> + return ret;
> +
> + priv->sspa1.hw.init =
> + CLK_HW_INIT_HW("sspa1", &priv->sspa1_mux.hw,
> + &mmp_audio_div_ops, 0);
> + priv->sspa1.priv = priv;
> + priv->sspa1.shift = SSPA_AUD_CTRL_SSPA1_SHIFT;
> + ret = devm_clk_hw_register(&pdev->dev, &priv->sspa1.hw);
> + if (ret)
> + return ret;
> +
> + priv->clk_table[MMP2_CLK_AUDIO_SYSCLK] = priv->sysclk.hw.clk;
> + priv->clk_table[MMP2_CLK_AUDIO_SSPA0] = priv->sspa0.hw.clk;
> + priv->clk_table[MMP2_CLK_AUDIO_SSPA1] = priv->sspa1.hw.clk;
> + priv->clk_data.clks = priv->clk_table;
> + priv->clk_data.clk_num = ARRAY_SIZE(priv->clk_table);
> +
> + return of_clk_add_provider(pdev->dev.of_node, of_clk_src_onecell_get,
Please add a clk_hw provider and not a clk provider. We're trying to
remove that API.
> + &priv->clk_data);
> +}
> +
> +static const struct of_device_id mmp2_audio_clk_of_match[] = {
> + { .compatible = "marvell,mmp2-audio-clock" },
> + {},
Nitpick: Drop comma so nothing can come after this.
> +};
> +
> +MODULE_DEVICE_TABLE(of, mmp2_audio_clk_of_match);
> +
> +static struct platform_driver mmp2_audio_clk_driver = {
> + .driver = {
> + .name = "mmp2-audio-clock",
> + .of_match_table = of_match_ptr(mmp2_audio_clk_of_match),
> + },
> + .probe = mmp2_audio_clk_probe,
> +};
> +
Nitpick: Drop newline here so it sits next to the driver.
> +module_platform_driver(mmp2_audio_clk_driver);
> +
> +MODULE_AUTHOR("Lubomir Rintel <lkundrak@v3.sk>");
> +MODULE_DESCRIPTION("Clock driver for MMP2 Audio subsystem");
> +MODULE_LICENSE("GPL");
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] dt-bindings: sound: Add Marvell MMP Audio Clock Controller binding
2020-05-14 21:06 ` Stephen Boyd
@ 2020-05-19 22:21 ` Lubomir Rintel
0 siblings, 0 replies; 7+ messages in thread
From: Lubomir Rintel @ 2020-05-19 22:21 UTC (permalink / raw)
To: Stephen Boyd
Cc: Michael Turquette, Liam Girdwood, Mark Brown, Rob Herring,
linux-clk, devicetree, linux-kernel, linux-media
On Thu, May 14, 2020 at 02:06:07PM -0700, Stephen Boyd wrote:
> Quoting Lubomir Rintel (2020-05-11 12:55:33)
> > diff --git a/Documentation/devicetree/bindings/clock/marvell,mmp2-audio-clock.yaml b/Documentation/devicetree/bindings/clock/marvell,mmp2-audio-clock.yaml
> > new file mode 100644
> > index 000000000000..b86e9fbfa56d
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/clock/marvell,mmp2-audio-clock.yaml
> > @@ -0,0 +1,73 @@
> > +# SPDX-License-Identifier: (GPL-2.0+ OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/clock/marvell,mmp2-audio-clock.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Marvell MMP2 Audio Clock Controller
> > +
> > +maintainers:
> > + - Lubomir Rintel <lkundrak@v3.sk>
> > +
> > +description: |
> > + The audio clock controller generates and supplies the clocks to the audio
> > + codec.
> > +
> > + Each clock is assigned an identifier and client nodes use this identifier
> > + to specify the clock which they consume.
> > +
> > + All these identifiers could be found in <dt-bindings/clock/marvell,mmp2.h>.
>
> Is this right? The patch puts them in mmp2-audio.h
>
> > +
> > +properties:
> > + compatible:
> > + enum:
> > + - marvell,mmp2-audio-clock
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + clocks:
> > + items:
> > + - description: Audio subsystem clock
> > + - description: The crystal oscillator clock
> > + - description: First I2S clock
> > + - description: Second I2S clock
> > +
> > + clock-names:
> > + items:
> > + - const: audio
> > + - const: vctcxo
> > + - const: i2s0
> > + - const: i2s1
> > +
> > + '#clock-cells':
> > + const: 1
> > +
> > + power-domains:
> > + maxItems: 1
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - clocks
> > + - clock-names
> > + - '#clock-cells'
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > + #include <dt-bindings/clock/marvell,mmp2.h>
> > + #include <dt-bindings/power/marvell,mmp2.h>
> > +
> > + clocks@d42a0c30 {
>
> clock-controller@d42a0c30
>
> > + compatible = "marvell,mmp2-audio-clock";
> > + reg = <0xd42a0c30 0x10>;
>
> That is a very specific and tiny region. Presumably this is part of a
> larger hardware block and thus shouldn't be described in DT this way.
> Instead there should be one clock-controller node and a driver that
> controls all the clks that it wants to inside that hardware block.
This resides in a block that's entirely separate from SoC's main clock
controllers ("power management units"). It is inside the audio block,
separate power island along with two I2S ("SSPA") controllers. The
addresses are weirdly interleaved, with the clock controller being
mapped between the two channels of the first SSPA:
0xd42a0c00 - 0xd42a0c30 SSPA1 RX
0xd42a0c30 - 0xd42a0c40 Clock Control
0xd42a0c80 - 0xd42a0cb0 SSPA1 TX
0xd42a0d00 - 0xd42a0d30 SSPA2 RX
0xd42a0d80 - 0xd42a0cb0 SSPA2 TX
Despite the addresses being interwoven in this way, the Clock Controller
is pretty much independent of the SSPAs and deserves a separate device
node, regardless of how tiny its range is.
> > + clock-names = "audio", "vctcxo", "i2s0", "i2s1";
> > + clocks = <&soc_clocks MMP2_CLK_AUDIO>,
> > + <&soc_clocks MMP2_CLK_VCTCXO>,
> > + <&soc_clocks MMP2_CLK_I2S0>,
> > + <&soc_clocks MMP2_CLK_I2S1>;
> > + power-domains = <&soc_clocks MMP2_POWER_DOMAIN_AUDIO>;
> > + #clock-cells = <1>;
> > + };
> > diff --git a/include/dt-bindings/clock/marvell,mmp2-audio.h b/include/dt-bindings/clock/marvell,mmp2-audio.h
> > new file mode 100644
> > index 000000000000..127b48ec0f0a
> > --- /dev/null
> > +++ b/include/dt-bindings/clock/marvell,mmp2-audio.h
> > @@ -0,0 +1,8 @@
> > +/* SPDX-License-Identifier: (GPL-2.0+ OR BSD-2-Clause) */
> > +#ifndef __DT_BINDINGS_CLOCK_MARVELL_MMP2_AUDIO_H
> > +#define __DT_BINDINGS_CLOCK_MARVELL_MMP2_AUDIO_H
> > +
> > +#define MMP2_CLK_AUDIO_SYSCLK 1
>
> Any reason to start at 1 vs. 0?
>
> > +#define MMP2_CLK_AUDIO_SSPA0 2
> > +#define MMP2_CLK_AUDIO_SSPA1 3
> > +#endif
> > --
> > 2.26.2
> >
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-05-19 22:22 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-11 19:55 [PATCH 0/2] MMP2 Audio clock controller driver Lubomir Rintel
2020-05-11 19:55 ` [PATCH 1/2] dt-bindings: sound: Add Marvell MMP Audio Clock Controller binding Lubomir Rintel
2020-05-12 1:41 ` Rob Herring
2020-05-14 21:06 ` Stephen Boyd
2020-05-19 22:21 ` Lubomir Rintel
2020-05-11 19:55 ` [PATCH 2/2] clk: mmp2: Add audio clock controller driver Lubomir Rintel
2020-05-14 21:15 ` Stephen Boyd
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).