Re: [PATCH v4 2/7] clk: at91: add I2S clock mux driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.ciubotariu@xxxxxxxxxxxxx>
---
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.ciubotariu@xxxxxxxxxxxxx>
+ *
+ *
+ */
+
+#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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux