linux-clk.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 1/2] dt-bindings: clock: document the fsl-sai driver
@ 2019-12-09 23:33 Michael Walle
  2019-12-09 23:33 ` [PATCH v2 2/2] clk: fsl-sai: new driver Michael Walle
  2019-12-11 14:28 ` [PATCH v2 1/2] dt-bindings: clock: document the fsl-sai driver Rob Herring
  0 siblings, 2 replies; 7+ messages in thread
From: Michael Walle @ 2019-12-09 23:33 UTC (permalink / raw)
  To: linux-clk, devicetree, linux-kernel
  Cc: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland,
	Michael Walle

Signed-off-by: Michael Walle <michael@walle.cc>
---
changes since v1:
 - dual license gpl-2.0-only and bsd-2-clause
 - add "additionalProperties: false"
 - wrap example in soc {} node with correct #address-cells and #size-cells

 .../bindings/clock/fsl,sai-clock.yaml         | 55 +++++++++++++++++++
 1 file changed, 55 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/fsl,sai-clock.yaml

diff --git a/Documentation/devicetree/bindings/clock/fsl,sai-clock.yaml b/Documentation/devicetree/bindings/clock/fsl,sai-clock.yaml
new file mode 100644
index 000000000000..8fb2060ac47f
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/fsl,sai-clock.yaml
@@ -0,0 +1,55 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/bindings/clock/fsl,sai-clock.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Freescale SAI bitclock-as-a-clock binding
+
+maintainers:
+  - Michael Walle <michael@walle.cc>
+
+description: |
+  It is possible to use the BCLK pin of a SAI module as a generic clock
+  output. Some SoC are very constrained in their pin multiplexer
+  configuration. Eg. pins can only be changed groups. For example, on the
+  LS1028A SoC you can only enable SAIs in pairs. If you use only one SAI,
+  the second pins are wasted. Using this binding it is possible to use the
+  clock of the second SAI as a MCLK clock for an audio codec, for example.
+
+  This is a composite of a gated clock and a divider clock.
+
+properties:
+  compatible:
+    const: fsl,vf610-sai-clock
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  '#clock-cells':
+    const: 0
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - '#clock-cells'
+
+additionalProperties: false
+
+examples:
+  - |
+    soc {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        mclk: clock-mclk@f130080 {
+            compatible = "fsl,vf610-sai-clock";
+            reg = <0x0 0xf130080 0x0 0x80>;
+            #clock-cells = <0>;
+            clocks = <&parentclk>;
+        };
+    };
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v2 2/2] clk: fsl-sai: new driver
  2019-12-09 23:33 [PATCH v2 1/2] dt-bindings: clock: document the fsl-sai driver Michael Walle
@ 2019-12-09 23:33 ` Michael Walle
  2019-12-24  8:05   ` Stephen Boyd
  2019-12-11 14:28 ` [PATCH v2 1/2] dt-bindings: clock: document the fsl-sai driver Rob Herring
  1 sibling, 1 reply; 7+ messages in thread
From: Michael Walle @ 2019-12-09 23:33 UTC (permalink / raw)
  To: linux-clk, devicetree, linux-kernel
  Cc: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland,
	Michael Walle

With this driver it is possible to use the BCLK pin of the SAI module as
a generic clock output. This is esp. useful if you want to drive a clock
to an audio codec. Because the output only allows integer divider values
the audio codec needs an integrated PLL.

Signed-off-by: Michael Walle <michael@walle.cc>
---
changes since v1:
 - none

 drivers/clk/Kconfig       | 12 ++++++
 drivers/clk/Makefile      |  1 +
 drivers/clk/clk-fsl-sai.c | 84 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 97 insertions(+)
 create mode 100644 drivers/clk/clk-fsl-sai.c

diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index 45653a0e6ecd..dd1a5abc4ce8 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -174,6 +174,18 @@ config COMMON_CLK_CS2000_CP
 	help
 	  If you say yes here you get support for the CS2000 clock multiplier.
 
+config COMMON_CLK_FSL_SAI
+	bool "Clock driver for BCLK of Freescale SAI cores"
+	depends on ARCH_LAYERSCAPE || COMPILE_TEST
+	help
+	  This driver supports the Freescale SAI (Synchronous Audio Interface)
+	  to be used as a generic clock output. Some SoCs have restrictions
+	  regarding the possible pin multiplexer settings. Eg. on some SoCs
+	  two SAI interfaces can only be enabled together. If just one is
+	  needed, the BCLK pin of the second one can be used as general
+	  purpose clock output. Ideally, it can be used to drive an audio
+	  codec (sometimes known as MCLK).
+
 config COMMON_CLK_GEMINI
 	bool "Clock driver for Cortina Systems Gemini SoC"
 	depends on ARCH_GEMINI || COMPILE_TEST
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index 0696a0c1ab58..ec23fd956228 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -29,6 +29,7 @@ obj-$(CONFIG_ARCH_CLPS711X)		+= clk-clps711x.o
 obj-$(CONFIG_COMMON_CLK_CS2000_CP)	+= clk-cs2000-cp.o
 obj-$(CONFIG_ARCH_EFM32)		+= clk-efm32gg.o
 obj-$(CONFIG_COMMON_CLK_FIXED_MMIO)	+= clk-fixed-mmio.o
+obj-$(CONFIG_COMMON_CLK_FSL_SAI)	+= clk-fsl-sai.o
 obj-$(CONFIG_COMMON_CLK_GEMINI)		+= clk-gemini.o
 obj-$(CONFIG_COMMON_CLK_ASPEED)		+= clk-aspeed.o
 obj-$(CONFIG_MACH_ASPEED_G6)		+= clk-ast2600.o
diff --git a/drivers/clk/clk-fsl-sai.c b/drivers/clk/clk-fsl-sai.c
new file mode 100644
index 000000000000..b92054d15ab1
--- /dev/null
+++ b/drivers/clk/clk-fsl-sai.c
@@ -0,0 +1,84 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Freescale SAI BCLK as a generic clock driver
+ *
+ * Copyright 2019 Kontron Europe GmbH
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/err.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/slab.h>
+
+#define I2S_CSR		0x00
+#define I2S_CR2		0x08
+#define CSR_BCE_BIT	28
+#define CR2_BCD		BIT(24)
+#define CR2_DIV_SHIFT	0
+#define CR2_DIV_WIDTH	8
+
+struct fsl_sai_clk {
+	struct clk_divider div;
+	struct clk_gate gate;
+	spinlock_t lock;
+};
+
+static void __init fsl_sai_clk_setup(struct device_node *node)
+{
+	const char *clk_name = node->name;
+	struct fsl_sai_clk *sai_clk;
+	unsigned int num_parents;
+	const char *parent_name;
+	void __iomem *base;
+	struct clk_hw *hw;
+
+	num_parents = of_clk_get_parent_count(node);
+	if (!num_parents) {
+		pr_err("%s: no parent found", clk_name);
+		return;
+	}
+
+	parent_name = of_clk_get_parent_name(node, 0);
+
+	sai_clk = kzalloc(sizeof(*sai_clk), GFP_KERNEL);
+	if (!sai_clk)
+		return;
+
+	base = of_iomap(node, 0);
+	if (base == NULL) {
+		pr_err("%s: failed to map register space", clk_name);
+		goto err;
+	}
+
+	spin_lock_init(&sai_clk->lock);
+
+	sai_clk->gate.reg = base + I2S_CSR;
+	sai_clk->gate.bit_idx = CSR_BCE_BIT;
+	sai_clk->gate.lock = &sai_clk->lock;
+
+	sai_clk->div.reg = base + I2S_CR2;
+	sai_clk->div.shift = CR2_DIV_SHIFT;
+	sai_clk->div.width = CR2_DIV_WIDTH;
+	sai_clk->div.lock = &sai_clk->lock;
+
+	/* set clock direction, we are the BCLK master */
+	writel(CR2_BCD, base + I2S_CR2);
+
+	hw = clk_hw_register_composite(NULL, clk_name, &parent_name, 1,
+				       NULL, NULL,
+				       &sai_clk->div.hw, &clk_divider_ops,
+				       &sai_clk->gate.hw, &clk_gate_ops,
+				       CLK_SET_RATE_GATE);
+	if (IS_ERR(hw))
+		goto err;
+
+	of_clk_add_hw_provider(node, of_clk_hw_simple_get, hw);
+
+	return;
+
+err:
+	kfree(sai_clk);
+}
+
+CLK_OF_DECLARE(fsl_sai_clk, "fsl,vf610-sai-clock", fsl_sai_clk_setup);
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 1/2] dt-bindings: clock: document the fsl-sai driver
  2019-12-09 23:33 [PATCH v2 1/2] dt-bindings: clock: document the fsl-sai driver Michael Walle
  2019-12-09 23:33 ` [PATCH v2 2/2] clk: fsl-sai: new driver Michael Walle
@ 2019-12-11 14:28 ` Rob Herring
  1 sibling, 0 replies; 7+ messages in thread
From: Rob Herring @ 2019-12-11 14:28 UTC (permalink / raw)
  To: Michael Walle
  Cc: linux-clk, devicetree, linux-kernel, Michael Turquette,
	Stephen Boyd, Mark Rutland

On Mon, Dec 9, 2019 at 5:33 PM Michael Walle <michael@walle.cc> wrote:
>
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---
> changes since v1:
>  - dual license gpl-2.0-only and bsd-2-clause
>  - add "additionalProperties: false"
>  - wrap example in soc {} node with correct #address-cells and #size-cells
>
>  .../bindings/clock/fsl,sai-clock.yaml         | 55 +++++++++++++++++++
>  1 file changed, 55 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/fsl,sai-clock.yaml

Reviewed-by: Rob Herring <robh@kernel.org>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 2/2] clk: fsl-sai: new driver
  2019-12-09 23:33 ` [PATCH v2 2/2] clk: fsl-sai: new driver Michael Walle
@ 2019-12-24  8:05   ` Stephen Boyd
  2020-01-01 15:15     ` Michael Walle
  0 siblings, 1 reply; 7+ messages in thread
From: Stephen Boyd @ 2019-12-24  8:05 UTC (permalink / raw)
  To: Michael Walle, devicetree, linux-clk, linux-kernel
  Cc: Michael Turquette, Rob Herring, Mark Rutland, Michael Walle

Quoting Michael Walle (2019-12-09 15:33:05)
> diff --git a/drivers/clk/clk-fsl-sai.c b/drivers/clk/clk-fsl-sai.c
> new file mode 100644
> index 000000000000..b92054d15ab1
> --- /dev/null
> +++ b/drivers/clk/clk-fsl-sai.c
> @@ -0,0 +1,84 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Freescale SAI BCLK as a generic clock driver
> + *
> + * Copyright 2019 Kontron Europe GmbH
> + */
> +
> +#include <linux/clk-provider.h>
> +#include <linux/err.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/slab.h>
> +
> +#define I2S_CSR                0x00
> +#define I2S_CR2                0x08
> +#define CSR_BCE_BIT    28
> +#define CR2_BCD                BIT(24)
> +#define CR2_DIV_SHIFT  0
> +#define CR2_DIV_WIDTH  8
> +
> +struct fsl_sai_clk {
> +       struct clk_divider div;
> +       struct clk_gate gate;
> +       spinlock_t lock;
> +};
> +
> +static void __init fsl_sai_clk_setup(struct device_node *node)
> +{
> +       const char *clk_name = node->name;
> +       struct fsl_sai_clk *sai_clk;
> +       unsigned int num_parents;
> +       const char *parent_name;
> +       void __iomem *base;
> +       struct clk_hw *hw;
> +
> +       num_parents = of_clk_get_parent_count(node);
> +       if (!num_parents) {
> +               pr_err("%s: no parent found", clk_name);
> +               return;
> +       }
> +
> +       parent_name = of_clk_get_parent_name(node, 0);

Could this use the new way of specifying clk parents so that we don't
have to query DT for parent names and just let the core framework do it
whenever it needs to?

> +
> +       sai_clk = kzalloc(sizeof(*sai_clk), GFP_KERNEL);
> +       if (!sai_clk)
> +               return;
> +
> +       base = of_iomap(node, 0);
> +       if (base == NULL) {
> +               pr_err("%s: failed to map register space", clk_name);
> +               goto err;
> +       }
> +
> +       spin_lock_init(&sai_clk->lock);
> +
> +       sai_clk->gate.reg = base + I2S_CSR;
> +       sai_clk->gate.bit_idx = CSR_BCE_BIT;
> +       sai_clk->gate.lock = &sai_clk->lock;
> +
> +       sai_clk->div.reg = base + I2S_CR2;
> +       sai_clk->div.shift = CR2_DIV_SHIFT;
> +       sai_clk->div.width = CR2_DIV_WIDTH;
> +       sai_clk->div.lock = &sai_clk->lock;
> +
> +       /* set clock direction, we are the BCLK master */

Should this configuration come from DT somehow?

> +       writel(CR2_BCD, base + I2S_CR2);
> +
> +       hw = clk_hw_register_composite(NULL, clk_name, &parent_name, 1,
> +                                      NULL, NULL,
> +                                      &sai_clk->div.hw, &clk_divider_ops,
> +                                      &sai_clk->gate.hw, &clk_gate_ops,
> +                                      CLK_SET_RATE_GATE);
> +       if (IS_ERR(hw))
> +               goto err;
> +
> +       of_clk_add_hw_provider(node, of_clk_hw_simple_get, hw);
> +
> +       return;
> +
> +err:
> +       kfree(sai_clk);
> +}
> +
> +CLK_OF_DECLARE(fsl_sai_clk, "fsl,vf610-sai-clock", fsl_sai_clk_setup);

Is there a reason this can't be a platform device driver?


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 2/2] clk: fsl-sai: new driver
  2019-12-24  8:05   ` Stephen Boyd
@ 2020-01-01 15:15     ` Michael Walle
  2020-01-02  8:09       ` Stephen Boyd
  0 siblings, 1 reply; 7+ messages in thread
From: Michael Walle @ 2020-01-01 15:15 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: devicetree, linux-clk, linux-kernel, Michael Turquette,
	Rob Herring, Mark Rutland


Hi Stephen,

thanks for the review.

Am 2019-12-24 09:05, schrieb Stephen Boyd:
> Quoting Michael Walle (2019-12-09 15:33:05)
>> diff --git a/drivers/clk/clk-fsl-sai.c b/drivers/clk/clk-fsl-sai.c
>> new file mode 100644
>> index 000000000000..b92054d15ab1
>> --- /dev/null
>> +++ b/drivers/clk/clk-fsl-sai.c
>> @@ -0,0 +1,84 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Freescale SAI BCLK as a generic clock driver
>> + *
>> + * Copyright 2019 Kontron Europe GmbH
>> + */
>> +
>> +#include <linux/clk-provider.h>
>> +#include <linux/err.h>
>> +#include <linux/of.h>
>> +#include <linux/of_address.h>
>> +#include <linux/slab.h>
>> +
>> +#define I2S_CSR                0x00
>> +#define I2S_CR2                0x08
>> +#define CSR_BCE_BIT    28
>> +#define CR2_BCD                BIT(24)
>> +#define CR2_DIV_SHIFT  0
>> +#define CR2_DIV_WIDTH  8
>> +
>> +struct fsl_sai_clk {
>> +       struct clk_divider div;
>> +       struct clk_gate gate;
>> +       spinlock_t lock;
>> +};
>> +
>> +static void __init fsl_sai_clk_setup(struct device_node *node)
>> +{
>> +       const char *clk_name = node->name;
>> +       struct fsl_sai_clk *sai_clk;
>> +       unsigned int num_parents;
>> +       const char *parent_name;
>> +       void __iomem *base;
>> +       struct clk_hw *hw;
>> +
>> +       num_parents = of_clk_get_parent_count(node);
>> +       if (!num_parents) {
>> +               pr_err("%s: no parent found", clk_name);
>> +               return;
>> +       }
>> +
>> +       parent_name = of_clk_get_parent_name(node, 0);
> 
> Could this use the new way of specifying clk parents so that we don't
> have to query DT for parent names and just let the core framework do it
> whenever it needs to?

you mean specifying parent_data with .index = 0? Seems like 
clk_composite
does not support this. The parent can only be specified by supplying the
clock names.

I could add that in a separate patch. What do you think about the
following new functions, where a driver can use parent_data instead
of parent_names.

+struct clk *clk_register_composite_pdata(struct device *dev, const char 
*name,
+               const struct clk_parent_data *parent_data,
+               struct clk_hw *mux_hw, const struct clk_ops *mux_ops,
+               struct clk_hw *rate_hw, const struct clk_ops *rate_ops,
+               struct clk_hw *gate_hw, const struct clk_ops *gate_ops,
+               unsigned long flags);

+struct clk_hw *clk_hw_register_composite_pdata(struct device *dev,
+               const char *name, const struct clk_parent_data 
*parent_data,
+               struct clk_hw *mux_hw, const struct clk_ops *mux_ops,
+               struct clk_hw *rate_hw, const struct clk_ops *rate_ops,
+               struct clk_hw *gate_hw, const struct clk_ops *gate_ops,
+               unsigned long flags);


>> +
>> +       sai_clk = kzalloc(sizeof(*sai_clk), GFP_KERNEL);
>> +       if (!sai_clk)
>> +               return;
>> +
>> +       base = of_iomap(node, 0);
>> +       if (base == NULL) {
>> +               pr_err("%s: failed to map register space", clk_name);
>> +               goto err;
>> +       }
>> +
>> +       spin_lock_init(&sai_clk->lock);
>> +
>> +       sai_clk->gate.reg = base + I2S_CSR;
>> +       sai_clk->gate.bit_idx = CSR_BCE_BIT;
>> +       sai_clk->gate.lock = &sai_clk->lock;
>> +
>> +       sai_clk->div.reg = base + I2S_CR2;
>> +       sai_clk->div.shift = CR2_DIV_SHIFT;
>> +       sai_clk->div.width = CR2_DIV_WIDTH;
>> +       sai_clk->div.lock = &sai_clk->lock;
>> +
>> +       /* set clock direction, we are the BCLK master */
> 
> Should this configuration come from DT somehow?

No, we are always master, because as a slave, there would be no clock
output ;)

>> +       writel(CR2_BCD, base + I2S_CR2);
>> +
>> +       hw = clk_hw_register_composite(NULL, clk_name, &parent_name, 
>> 1,
>> +                                      NULL, NULL,
>> +                                      &sai_clk->div.hw, 
>> &clk_divider_ops,
>> +                                      &sai_clk->gate.hw, 
>> &clk_gate_ops,
>> +                                      CLK_SET_RATE_GATE);
>> +       if (IS_ERR(hw))
>> +               goto err;
>> +
>> +       of_clk_add_hw_provider(node, of_clk_hw_simple_get, hw);
>> +
>> +       return;
>> +
>> +err:
>> +       kfree(sai_clk);
>> +}
>> +
>> +CLK_OF_DECLARE(fsl_sai_clk, "fsl,vf610-sai-clock", 
>> fsl_sai_clk_setup);
> 
> Is there a reason this can't be a platform device driver?

I don't think so, the user will be a sound codec for now. I'll convert 
it
to a platform device, in that case I could also use the devm_ variants.

-michael

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 2/2] clk: fsl-sai: new driver
  2020-01-01 15:15     ` Michael Walle
@ 2020-01-02  8:09       ` Stephen Boyd
  2020-01-03  9:00         ` Michael Walle
  0 siblings, 1 reply; 7+ messages in thread
From: Stephen Boyd @ 2020-01-02  8:09 UTC (permalink / raw)
  To: Michael Walle
  Cc: devicetree, linux-clk, linux-kernel, Michael Turquette,
	Rob Herring, Mark Rutland

Quoting Michael Walle (2020-01-01 07:15:32)
> 
> Hi Stephen,
> 
> thanks for the review.
> 
> Am 2019-12-24 09:05, schrieb Stephen Boyd:
> > Quoting Michael Walle (2019-12-09 15:33:05)
> >> diff --git a/drivers/clk/clk-fsl-sai.c b/drivers/clk/clk-fsl-sai.c
> >> new file mode 100644
> >> index 000000000000..b92054d15ab1
> >> --- /dev/null
> >> +++ b/drivers/clk/clk-fsl-sai.c
> >> @@ -0,0 +1,84 @@
> >> +// SPDX-License-Identifier: GPL-2.0
> >> +/*
> >> + * Freescale SAI BCLK as a generic clock driver
> >> + *
> >> + * Copyright 2019 Kontron Europe GmbH
> >> + */
> >> +
> >> +#include <linux/clk-provider.h>
> >> +#include <linux/err.h>
> >> +#include <linux/of.h>
> >> +#include <linux/of_address.h>
> >> +#include <linux/slab.h>
> >> +
> >> +#define I2S_CSR                0x00
> >> +#define I2S_CR2                0x08
> >> +#define CSR_BCE_BIT    28
> >> +#define CR2_BCD                BIT(24)
> >> +#define CR2_DIV_SHIFT  0
> >> +#define CR2_DIV_WIDTH  8
> >> +
> >> +struct fsl_sai_clk {
> >> +       struct clk_divider div;
> >> +       struct clk_gate gate;
> >> +       spinlock_t lock;
> >> +};
> >> +
> >> +static void __init fsl_sai_clk_setup(struct device_node *node)
> >> +{
> >> +       const char *clk_name = node->name;
> >> +       struct fsl_sai_clk *sai_clk;
> >> +       unsigned int num_parents;
> >> +       const char *parent_name;
> >> +       void __iomem *base;
> >> +       struct clk_hw *hw;
> >> +
> >> +       num_parents = of_clk_get_parent_count(node);
> >> +       if (!num_parents) {
> >> +               pr_err("%s: no parent found", clk_name);
> >> +               return;
> >> +       }
> >> +
> >> +       parent_name = of_clk_get_parent_name(node, 0);
> > 
> > Could this use the new way of specifying clk parents so that we don't
> > have to query DT for parent names and just let the core framework do it
> > whenever it needs to?
> 
> you mean specifying parent_data with .index = 0? Seems like 
> clk_composite
> does not support this. The parent can only be specified by supplying the
> clock names.
> 
> I could add that in a separate patch. What do you think about the
> following new functions, where a driver can use parent_data instead
> of parent_names.

I started doing this in
https://lkml.kernel.org/r/20190830150923.259497-1-sboyd@kernel.org but I
never got around to the composite clks. Sounds fine to add this new API
for your use case.

> 
> +struct clk *clk_register_composite_pdata(struct device *dev, const char 
> *name,
> +               const struct clk_parent_data *parent_data,
> +               struct clk_hw *mux_hw, const struct clk_ops *mux_ops,
> +               struct clk_hw *rate_hw, const struct clk_ops *rate_ops,
> +               struct clk_hw *gate_hw, const struct clk_ops *gate_ops,
> +               unsigned long flags);
> 
> +struct clk_hw *clk_hw_register_composite_pdata(struct device *dev,
> +               const char *name, const struct clk_parent_data 
> *parent_data,
> +               struct clk_hw *mux_hw, const struct clk_ops *mux_ops,
> +               struct clk_hw *rate_hw, const struct clk_ops *rate_ops,
> +               struct clk_hw *gate_hw, const struct clk_ops *gate_ops,
> +               unsigned long flags);
> 
> 
> >> +
> >> +       sai_clk = kzalloc(sizeof(*sai_clk), GFP_KERNEL);
> >> +       if (!sai_clk)
> >> +               return;
> >> +
> >> +       base = of_iomap(node, 0);
> >> +       if (base == NULL) {
> >> +               pr_err("%s: failed to map register space", clk_name);
> >> +               goto err;
> >> +       }
> >> +
> >> +       spin_lock_init(&sai_clk->lock);
> >> +
> >> +       sai_clk->gate.reg = base + I2S_CSR;
> >> +       sai_clk->gate.bit_idx = CSR_BCE_BIT;
> >> +       sai_clk->gate.lock = &sai_clk->lock;
> >> +
> >> +       sai_clk->div.reg = base + I2S_CR2;
> >> +       sai_clk->div.shift = CR2_DIV_SHIFT;
> >> +       sai_clk->div.width = CR2_DIV_WIDTH;
> >> +       sai_clk->div.lock = &sai_clk->lock;
> >> +
> >> +       /* set clock direction, we are the BCLK master */
> > 
> > Should this configuration come from DT somehow?
> 
> No, we are always master, because as a slave, there would be no clock
> output ;)

Got it.

> 
> >> +       writel(CR2_BCD, base + I2S_CR2);
> >> +
> >> +       hw = clk_hw_register_composite(NULL, clk_name, &parent_name, 
> >> 1,
> >> +                                      NULL, NULL,
> >> +                                      &sai_clk->div.hw, 
> >> &clk_divider_ops,
> >> +                                      &sai_clk->gate.hw, 
> >> &clk_gate_ops,
> >> +                                      CLK_SET_RATE_GATE);
> >> +       if (IS_ERR(hw))
> >> +               goto err;
> >> +
> >> +       of_clk_add_hw_provider(node, of_clk_hw_simple_get, hw);
> >> +
> >> +       return;
> >> +
> >> +err:
> >> +       kfree(sai_clk);
> >> +}
> >> +
> >> +CLK_OF_DECLARE(fsl_sai_clk, "fsl,vf610-sai-clock", 
> >> fsl_sai_clk_setup);
> > 
> > Is there a reason this can't be a platform device driver?
> 
> I don't think so, the user will be a sound codec for now. I'll convert 
> it
> to a platform device, in that case I could also use the devm_ variants.
> 

Awesome. Thanks!


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 2/2] clk: fsl-sai: new driver
  2020-01-02  8:09       ` Stephen Boyd
@ 2020-01-03  9:00         ` Michael Walle
  0 siblings, 0 replies; 7+ messages in thread
From: Michael Walle @ 2020-01-03  9:00 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: devicetree, linux-clk, linux-kernel, Michael Turquette,
	Rob Herring, Mark Rutland

Hi Stephen,

>> >> +       parent_name = of_clk_get_parent_name(node, 0);
>> >
>> > Could this use the new way of specifying clk parents so that we don't
>> > have to query DT for parent names and just let the core framework do it
>> > whenever it needs to?
>> 
>> you mean specifying parent_data with .index = 0? Seems like
>> clk_composite
>> does not support this. The parent can only be specified by supplying 
>> the
>> clock names.
>> 
>> I could add that in a separate patch. What do you think about the
>> following new functions, where a driver can use parent_data instead
>> of parent_names.
> 
> I started doing this in
> https://lkml.kernel.org/r/20190830150923.259497-1-sboyd@kernel.org but 
> I
> never got around to the composite clks. Sounds fine to add this new API
> for your use case.

Yeah took me a while to figure out what you've meant by the "new way" ;)
Anyway, I've posted a v3 of this series with the new composite clock 
API.

> 
>> 
>> +struct clk *clk_register_composite_pdata(struct device *dev, const 
>> char
>> *name,
>> +               const struct clk_parent_data *parent_data,
>> +               struct clk_hw *mux_hw, const struct clk_ops *mux_ops,
>> +               struct clk_hw *rate_hw, const struct clk_ops 
>> *rate_ops,
>> +               struct clk_hw *gate_hw, const struct clk_ops 
>> *gate_ops,
>> +               unsigned long flags);

num_parents was missing here. added that in the v3.

-michael

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2020-01-03  9:00 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-09 23:33 [PATCH v2 1/2] dt-bindings: clock: document the fsl-sai driver Michael Walle
2019-12-09 23:33 ` [PATCH v2 2/2] clk: fsl-sai: new driver Michael Walle
2019-12-24  8:05   ` Stephen Boyd
2020-01-01 15:15     ` Michael Walle
2020-01-02  8:09       ` Stephen Boyd
2020-01-03  9:00         ` Michael Walle
2019-12-11 14:28 ` [PATCH v2 1/2] dt-bindings: clock: document the fsl-sai driver Rob Herring

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).