Re: [PATCH 1/3] hix5hd2-clock: add complex clk

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

 




Dear David

On 05/19/2014 09:11 PM, David Laight wrote:
From: Zhangfei Gao
...
diff --git a/drivers/clk/hisilicon/clk-hix5hd2.c b/drivers/clk/hisilicon/clk-hix5hd2.c
index e5fcfb4..1b1347f 100644
--- a/drivers/clk/hisilicon/clk-hix5hd2.c
+++ b/drivers/clk/hisilicon/clk-hix5hd2.c
@@ -9,6 +9,8 @@

  #include <linux/of_address.h>
  #include <dt-bindings/clock/hix5hd2-clock.h>
+#include <linux/slab.h>
+#include <linux/delay.h>
  #include "clk.h"

  static struct hisi_fixed_rate_clock hix5hd2_fixed_rate_clks[] __initdata = {
@@ -79,8 +81,186 @@ static struct hisi_gate_clock hix5hd2_gate_clks[] __initdata = {
  		CLK_SET_RATE_PARENT, 0xa0, 1, 0, },
  	{ HIX5HD2_MMC_CIU_RST, "rst_mmc_ciu", "clk_mmc_ciu",
  		CLK_SET_RATE_PARENT, 0xa0, 4, CLK_GATE_SET_TO_DISABLE, },
+	/*gsf*/
+	{ HIX5HD2_FWD_BUS_CLK, "clk_fwd_bus", NULL, 0, 0xcc, 0, 0, },
+	{ HIX5HD2_FWD_SYS_CLK, "clk_fwd_sys", "clk_fwd_bus", 0, 0xcc, 5, 0, },
+	{ HIX5HD2_MAC0_PHY_CLK, "clk_fephy", "clk_fwd_sys",
+		 CLK_SET_RATE_PARENT, 0x120, 0, 0, },
  };

+enum {TYPE_COMPLEX, TYPE_ETHER};

Shouldn't this be a named enum to make it more obvious
where the values should be used?
Yes, it's better.

+
+struct hix5hd2_complex_clock {
+	unsigned int	id;

Reorder the fields to avoid the implicit pad here.
Curious, what's the impact if there is a pad.
It may inconsistent with other table.


+	const char	*name;
+	const char	*parent_name;
+	u32		ctrl_reg;
+	u32		ctrl_clk_mask;
+	u32		ctrl_rst_mask;
+	u32		phy_reg;
+	u32		phy_clk_mask;
+	u32		phy_rst_mask;
+	u32		type;
+};
+
+struct hix5hd2_clk_complex {
+	struct clk_hw	hw;
+	u32		id;
+	void __iomem	*ctrl_reg;
+	u32		ctrl_clk_mask;
+	u32		ctrl_rst_mask;
+	void __iomem	*phy_reg;
+	u32		phy_clk_mask;
+	u32		phy_rst_mask;
+};
+
+static struct hix5hd2_complex_clock hix5hd2_complex_clks[] __initdata = {
+	{HIX5HD2_MAC0_CLK, "clk_mac0", "clk_fephy",
+		0xcc, 0xa, 0x500, 0x120, 0, 0x10, TYPE_ETHER},
+	{HIX5HD2_MAC1_CLK, "clk_mac1", "clk_fwd_sys",
+		0xcc, 0x14, 0xa00, 0x168, 0x2, 0, TYPE_ETHER},
+	{HIX5HD2_SATA_CLK, "clk_sata", NULL,
+		0xa8, 0x1f, 0x300, 0xac, 0x1, 0x0, TYPE_COMPLEX},
+	{HIX5HD2_USB_CLK, "clk_usb", NULL,
+		0xb8, 0xff, 0x3f00, 0xbc, 0x7, 0x3f00, TYPE_COMPLEX},
+};
+
+#define to_complex_clk(_hw) container_of(_hw, struct hix5hd2_clk_complex, hw)
+
+static int clk_ether_enable(struct clk_hw *hw)
+{
+	struct hix5hd2_clk_complex *clk = to_complex_clk(hw);
+	u32 val;
+
+	val = readl(clk->ctrl_reg);
+	val |= clk->ctrl_clk_mask | clk->ctrl_rst_mask;
+	writel(val, clk->ctrl_reg);
+	udelay(50);
+	val &= ~(clk->ctrl_rst_mask);
+	writel(val, clk->ctrl_reg);

I'd need to be convinced that the udelay() has the desired effect.
I suspect you are trying to assert reset for a minimum period.
Yes, it is just want to make sure the reset takeing effect.
In fact, it works without udelay here, it is not required as timing as the following mdelay.

However the first write can be 'posted' by all sorts of hardware
for all sorts of reasons - so the writes can actually be back to back.
Sorry, not understand.
Do you mean the first write will not take effect immediately, so the two write happens continuously.
What's the recommended behavior?

Will use readl_relaxed & writel_relaxed instead.


	David

+
+	val = readl(clk->phy_reg);
+	val |= clk->phy_clk_mask;
+	val &= ~(clk->phy_rst_mask);
+	writel(val, clk->phy_reg);
+	mdelay(10);
+
+	val &= ~(clk->phy_clk_mask);
+	val |= clk->phy_rst_mask;
+	writel(val, clk->phy_reg);
+	mdelay(10);
+
+	val |= clk->phy_clk_mask;
+	val &= ~(clk->phy_rst_mask);
+	writel(val, clk->phy_reg);
+	mdelay(30);
+	return 0;
+}
+

Thanks
--
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