Signed-off-by: Michael Walle <[email protected]>
---
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 <[email protected]>
+
+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
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 <[email protected]>
---
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
On Mon, Dec 9, 2019 at 5:33 PM Michael Walle <[email protected]> wrote:
>
> Signed-off-by: Michael Walle <[email protected]>
> ---
> 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 <[email protected]>
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?
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
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/[email protected] 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!
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/[email protected] 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