On 31.05.2018 18:26, Stephen Boyd wrote:
Quoting Codrin Ciubotariu (2018-05-25 05:34:23)
This driver is a simple muxing driver that controls the
I2S's clock input by using syscon/regmap to change the parrent.
s/parrent/parent/
I will fix it.
The available inputs can be Peripheral clock and Generated clock.
Why capitalize peripheral and generated?
In DS, at I2S block these clocks appear defined with capital letters...
I will fix it.
Signed-off-by: Codrin Ciubotariu <codrin.ciubota...@microchip.com>
---
diff --git a/arch/arm/mach-at91/Kconfig b/arch/arm/mach-at91/Kconfig
index 1254bf9..903f23c 100644
--- a/arch/arm/mach-at91/Kconfig
+++ b/arch/arm/mach-at91/Kconfig
@@ -27,6 +27,7 @@ config SOC_SAMA5D2
select HAVE_AT91_H32MX
select HAVE_AT91_GENERATED_CLK
select HAVE_AT91_AUDIO_PLL
+ select HAVE_AT91_I2S_MUX_CLK
select PINCTRL_AT91PIO4
help
Select this if ou are using one of Microchip's SAMA5D2 family SoC.
@@ -129,6 +130,9 @@ config HAVE_AT91_GENERATED_CLK
config HAVE_AT91_AUDIO_PLL
bool
+config HAVE_AT91_I2S_MUX_CLK
+ bool
+
config SOC_SAM_V4_V5
bool
I guess this is OK.
diff --git a/drivers/clk/at91/Makefile b/drivers/clk/at91/Makefile
index 082596f..facc169 100644
--- a/drivers/clk/at91/Makefile
+++ b/drivers/clk/at91/Makefile
@@ -13,3 +13,4 @@ obj-$(CONFIG_HAVE_AT91_USB_CLK) += clk-usb.o
obj-$(CONFIG_HAVE_AT91_SMD) += clk-smd.o
obj-$(CONFIG_HAVE_AT91_H32MX) += clk-h32mx.o
obj-$(CONFIG_HAVE_AT91_GENERATED_CLK) += clk-generated.o
+obj-$(CONFIG_HAVE_AT91_I2S_MUX_CLK) += clk-i2s-mux.o
diff --git a/drivers/clk/at91/clk-i2s-mux.c b/drivers/clk/at91/clk-i2s-mux.c
new file mode 100644
index 0000000..2d56ded
--- /dev/null
+++ b/drivers/clk/at91/clk-i2s-mux.c
@@ -0,0 +1,117 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2018 Microchip Technology Inc,
+ * Codrin Ciubotariu <codrin.ciubota...@microchip.com>
+ *
+ *
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/of.h>
+#include <linux/mfd/syscon.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+
+#include <soc/at91/atmel-sfr.h>
+
+#define I2S_BUS_NR 2
+
+struct clk_i2s_mux {
+ struct clk_hw hw;
+ struct regmap *regmap;
+ u32 bus_id;
Can be a u8?
I think so, I will cast out_value parameter of of_property_read_u32() to u8.
+};
+
+#define to_clk_i2s_mux(hw) container_of(hw, struct clk_i2s_mux, hw)
+
+static u8 clk_i2s_mux_get_parent(struct clk_hw *hw)
+{
+ struct clk_i2s_mux *mux = to_clk_i2s_mux(hw);
+ u32 val;
+
+ regmap_read(mux->regmap, AT91_SFR_I2SCLKSEL, &val);
+
+ return (val & BIT(mux->bus_id)) >> mux->bus_id;
+}
+
+static int clk_i2s_mux_set_parent(struct clk_hw *hw, u8 index)
+{
+ struct clk_i2s_mux *mux = to_clk_i2s_mux(hw);
+
+ return regmap_update_bits(mux->regmap, AT91_SFR_I2SCLKSEL,
+ BIT(mux->bus_id), index << mux->bus_id);
+}
+
+const struct clk_ops clk_i2s_mux_ops = {
static?
Yes, I will fix it.
+ .get_parent = clk_i2s_mux_get_parent,
+ .set_parent = clk_i2s_mux_set_parent,
+ .determine_rate = __clk_mux_determine_rate,
+};
+
+static struct clk_hw * __init
+at91_clk_i2s_mux_register(struct regmap *regmap, const char *name,
+ const char * const *parent_names,
+ unsigned int num_parents, u32 bus_id)
+{
+ struct clk_init_data init = {};
+ struct clk_i2s_mux *i2s_ck;
+ int ret;
+
+ i2s_ck = kzalloc(sizeof(*i2s_ck), GFP_KERNEL);
+ if (!i2s_ck)
+ return ERR_PTR(-ENOMEM);
+
+ init.name = name;
+ init.ops = &clk_i2s_mux_ops;
+ init.parent_names = parent_names;
+ init.num_parents = num_parents;
+ init.flags = CLK_IGNORE_UNUSED;
Really? Why?
I am thinking that there is no need to gate this clock, since there is
no way to gate this clock in HW.
+
+ i2s_ck->hw.init = &init;
+ i2s_ck->bus_id = bus_id;
+ i2s_ck->regmap = regmap;
+
+ ret = clk_hw_register(NULL, &i2s_ck->hw);
+ if (ret) {
+ kfree(i2s_ck);
+ return ERR_PTR(ret);
+ }
+
+ return &i2s_ck->hw;
+}
Thank you for your review. I will wait a few more days for more comments
on this series and send a V5 afterwards.
Best regards,
Codrin
