[PATCH 0/3] add more clock definitions for hi3798cv200-poplar board

[PATCH 0/3] add more clock definitions for hi3798cv200-poplar board

[Jiancheng Xue]

Add more clock definitions for hi3798cv200-poplar board.

Younian Wang (1):
  clk: hisilicon: correct ir clock rate for hi3798cv200 SoC

tianshuliang (2):
  clk: hisilicon: add hisi phase clock support
  clk: hisilicon: add emmc sample and drive clock for hi3798cv200 SoC

 drivers/clk/hisilicon/Makefile          |   2 +-
 drivers/clk/hisilicon/clk-hisi-phase.c  | 117 ++++++++++++++++++++++++++++++++
 drivers/clk/hisilicon/clk.c             |  45 ++++++++++++
 drivers/clk/hisilicon/clk.h             |  22 ++++++
 drivers/clk/hisilicon/crg-hi3798cv200.c |  27 +++++++-
 5 files changed, 210 insertions(+), 3 deletions(-)
 create mode 100644 drivers/clk/hisilicon/clk-hisi-phase.c

-- 
2.7.4

[PATCH 1/3] clk: hisilicon: add hisi phase clock support

[Jiancheng Xue]

From: tianshuliang <tianshuliang@hisilicon.com>

Add a phase clock type for HiSilicon SoCs,which supports
clk_set_phase operation.

Signed-off-by: tianshuliang <tianshuliang@hisilicon.com>
Signed-off-by: Jiancheng Xue <xuejiancheng@hisilicon.com>
---
 drivers/clk/hisilicon/Makefile         |   2 +-
 drivers/clk/hisilicon/clk-hisi-phase.c | 117 +++++++++++++++++++++++++++++++++
 drivers/clk/hisilicon/clk.c            |  45 +++++++++++++
 drivers/clk/hisilicon/clk.h            |  22 +++++++
 4 files changed, 185 insertions(+), 1 deletion(-)
 create mode 100644 drivers/clk/hisilicon/clk-hisi-phase.c

diff --git a/drivers/clk/hisilicon/Makefile b/drivers/clk/hisilicon/Makefile
index 1e4c3dd..7189f07 100644
--- a/drivers/clk/hisilicon/Makefile
+++ b/drivers/clk/hisilicon/Makefile
@@ -2,7 +2,7 @@
 # Hisilicon Clock specific Makefile
 #
 
-obj-y	+= clk.o clkgate-separated.o clkdivider-hi6220.o
+obj-y	+= clk.o clkgate-separated.o clkdivider-hi6220.o clk-hisi-phase.o
 
 obj-$(CONFIG_ARCH_HI3xxx)	+= clk-hi3620.o
 obj-$(CONFIG_ARCH_HIP04)	+= clk-hip04.o
diff --git a/drivers/clk/hisilicon/clk-hisi-phase.c b/drivers/clk/hisilicon/clk-hisi-phase.c
new file mode 100644
index 0000000..436f0a1
--- /dev/null
+++ b/drivers/clk/hisilicon/clk-hisi-phase.c
@@ -0,0 +1,117 @@
+/*
+ * Copyright (c) 2017 HiSilicon Technologies Co., Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * Simple HiSilicon phase clock implementation.
+ */
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include "clk.h"
+
+struct clk_hisi_phase {
+	struct clk_hw	hw;
+	void __iomem	*reg;
+	u32		*phase_values;
+	u32		*phase_regs;
+	u8		phase_num;
+	u32		mask;
+	u8		shift;
+	u8		flags;
+	spinlock_t	*lock;
+};
+#define to_clk_hisi_phase(_hw) container_of(_hw, struct clk_hisi_phase, hw)
+
+static u32 hisi_clk_get_phase_reg(struct clk_hisi_phase *phase, int degrees)
+{
+	int i;
+
+	for (i = 0; i < phase->phase_num; i++)
+		if (phase->phase_values[i] == degrees)
+			return phase->phase_regs[i];
+
+	return -EINVAL;
+}
+
+static int hisi_clk_set_phase(struct clk_hw *hw, int degrees)
+{
+	struct clk_hisi_phase *phase = to_clk_hisi_phase(hw);
+	u32 val, phase_reg;
+	unsigned long flags = 0;
+
+	phase_reg = hisi_clk_get_phase_reg(phase, degrees);
+	if (phase_reg < 0)
+		return phase_reg;
+
+	if (phase->lock)
+		spin_lock_irqsave(phase->lock, flags);
+	else
+		__acquire(phase->lock);
+
+	val = clk_readl(phase->reg);
+	val &= ~(phase->mask << phase->shift);
+	val |= phase_reg << phase->shift;
+	clk_writel(val, phase->reg);
+
+	if (phase->lock)
+		spin_unlock_irqrestore(phase->lock, flags);
+	else
+		__release(phase->lock);
+
+	return 0;
+}
+
+const struct clk_ops clk_phase_ops = {
+	.set_phase = hisi_clk_set_phase,
+};
+
+void clk_unregister_hisi_phase(struct clk *clk)
+{
+	struct clk_hisi_phase *phase;
+	struct clk_hw *hw;
+
+	hw = __clk_get_hw(clk);
+	if (!hw)
+		return;
+
+	phase = to_clk_hisi_phase(hw);
+	clk_unregister(clk);
+}
+EXPORT_SYMBOL_GPL(clk_unregister_hisi_phase);
+
+struct clk *clk_register_hisi_phase(struct device *dev,
+		const struct hisi_phase_clock *clks,
+		void __iomem *base, spinlock_t *lock)
+{
+	struct clk_hisi_phase *phase;
+	struct clk *clk;
+	struct clk_init_data init;
+
+	phase = devm_kzalloc(dev, sizeof(struct clk_hisi_phase), GFP_KERNEL);
+	if (!phase)
+		return ERR_PTR(-ENOMEM);
+
+	init.name = clks->name;
+	init.ops = &clk_phase_ops;
+	init.flags = clks->flags | CLK_IS_BASIC;
+	init.parent_names = clks->parent_names ? &clks->parent_names : NULL;
+	init.num_parents = clks->parent_names ? 1 : 0;
+
+	phase->reg = base + clks->offset;
+	phase->shift = clks->shift;
+	phase->mask = BIT(clks->width) - 1;
+	phase->lock = lock;
+	phase->phase_values = clks->phase_values;
+	phase->phase_regs = clks->phase_regs;
+	phase->phase_num = clks->phase_num;
+	phase->hw.init = &init;
+
+	clk = clk_register(NULL, &phase->hw);
+	return clk;
+}
+EXPORT_SYMBOL_GPL(clk_register_hisi_phase);
diff --git a/drivers/clk/hisilicon/clk.c b/drivers/clk/hisilicon/clk.c
index b73c1df..e3adfad 100644
--- a/drivers/clk/hisilicon/clk.c
+++ b/drivers/clk/hisilicon/clk.c
@@ -197,6 +197,51 @@ int hisi_clk_register_mux(const struct hisi_mux_clock *clks,
 }
 EXPORT_SYMBOL_GPL(hisi_clk_register_mux);
 
+int hisi_clk_register_phase(struct device *dev,
+			const struct hisi_phase_clock *clks,
+			int nums, struct hisi_clock_data *data)
+{
+	int i;
+	struct clk *clk;
+	void __iomem *base = data->base;
+
+	for (i = 0; i < nums; i++) {
+		clk =	clk_register_hisi_phase(dev,
+				&clks[i], base, &hisi_clk_lock);
+
+		if (IS_ERR(clk)) {
+			pr_err("%s: failed to register clock %s\n",
+			       __func__, clks[i].name);
+			goto err;
+		}
+
+		data->clk_data.clks[clks[i].id] = clk;
+	}
+	return 0;
+
+err:
+	while (i--)
+		clk_unregister_hisi_phase(data->clk_data.clks[clks[i].id]);
+
+	return PTR_ERR(clk);
+}
+EXPORT_SYMBOL_GPL(hisi_clk_register_phase);
+
+void hisi_clk_unregister_phase(const struct hisi_phase_clock *clks,
+				int nums, struct hisi_clock_data *data)
+{
+	struct clk **clocks = data->clk_data.clks;
+	int i, id;
+
+	for (i = 0; i < nums; i++) {
+		id = clks[i].id;
+
+		if (clocks[id])
+			clk_unregister_hisi_phase(clocks[id]);
+	}
+}
+EXPORT_SYMBOL_GPL(hisi_clk_unregister_phase);
+
 int hisi_clk_register_divider(const struct hisi_divider_clock *clks,
 				      int nums, struct hisi_clock_data *data)
 {
diff --git a/drivers/clk/hisilicon/clk.h b/drivers/clk/hisilicon/clk.h
index 4e1d1af..bc18730 100644
--- a/drivers/clk/hisilicon/clk.h
+++ b/drivers/clk/hisilicon/clk.h
@@ -68,6 +68,19 @@ struct hisi_mux_clock {
 	const char		*alias;
 };
 
+struct hisi_phase_clock {
+	unsigned int    id;
+	const char      *name;
+	const char      *parent_names;
+	unsigned long   flags;
+	unsigned long   offset;
+	u8		shift;
+	u8		width;
+	u32		*phase_values;
+	u32		*phase_regs;
+	u8		phase_num;
+};
+
 struct hisi_divider_clock {
 	unsigned int		id;
 	const char		*name;
@@ -120,6 +133,15 @@ int hisi_clk_register_fixed_factor(const struct hisi_fixed_factor_clock *,
 				int, struct hisi_clock_data *);
 int hisi_clk_register_mux(const struct hisi_mux_clock *, int,
 				struct hisi_clock_data *);
+struct clk *clk_register_hisi_phase(struct device *dev,
+				const struct hisi_phase_clock *clks,
+				void __iomem *base, spinlock_t *lock);
+void clk_unregister_hisi_phase(struct clk *clk);
+int hisi_clk_register_phase(struct device *dev,
+				const struct hisi_phase_clock *clks,
+				int nums, struct hisi_clock_data *data);
+void hisi_clk_unregister_phase(const struct hisi_phase_clock *clks,
+				int nums, struct hisi_clock_data *data);
 int hisi_clk_register_divider(const struct hisi_divider_clock *,
 				int, struct hisi_clock_data *);
 int hisi_clk_register_gate(const struct hisi_gate_clock *,
-- 
2.7.4

[PATCH 2/3] clk: hisilicon: add emmc sample and drive clock for hi3798cv200 SoC

[Jiancheng Xue]

From: tianshuliang <tianshuliang@hisilicon.com>

Add emmc sample and emmc drive clock for Hi3798cv200 SoC

Signed-off-by: tianshuliang <tianshuliang@hisilicon.com>
Signed-off-by: Jiancheng Xue <xuejiancheng@hisilicon.com>
---
 drivers/clk/hisilicon/crg-hi3798cv200.c | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/hisilicon/crg-hi3798cv200.c b/drivers/clk/hisilicon/crg-hi3798cv200.c
index ed8bb5f..25d750c 100644
--- a/drivers/clk/hisilicon/crg-hi3798cv200.c
+++ b/drivers/clk/hisilicon/crg-hi3798cv200.c
@@ -83,6 +83,18 @@ static struct hisi_mux_clock hi3798cv200_mux_clks[] = {
 		CLK_SET_RATE_PARENT, 0x188, 10, 2, 0, comphy1_mux_table, },
 };
 
+static u32 mmc_phase_reg[] = {0, 1, 2, 3, 4, 5, 6, 7};
+static u32 mmc_phase_val[] = {0, 45, 90, 135, 180, 225, 270, 315};
+
+static struct hisi_phase_clock hi3798cv200_phase_clks[] = {
+	{ HISTB_MMC_SAMPLE_CLK, "mmc_sample", "clk_mmc_ciu",
+	CLK_SET_RATE_PARENT, 0xa0, 12, 3, mmc_phase_val,
+	mmc_phase_reg, ARRAY_SIZE(mmc_phase_reg)},
+	{ HISTB_MMC_DRV_CLK, "mmc_drive", "clk_mmc_ciu",
+	CLK_SET_RATE_PARENT, 0xa0, 16, 3, mmc_phase_val,
+	mmc_phase_reg, ARRAY_SIZE(mmc_phase_reg)},
+};
+
 static const struct hisi_gate_clock hi3798cv200_gate_clks[] = {
 	/* UART */
 	{ HISTB_UART2_CLK, "clk_uart2", "75m",
@@ -179,11 +191,18 @@ static struct hisi_clock_data *hi3798cv200_clk_register(
 	if (ret)
 		goto unregister_fixed_rate;
 
+	ret = hisi_clk_register_phase(&pdev->dev,
+				hi3798cv200_phase_clks,
+				ARRAY_SIZE(hi3798cv200_phase_clks),
+				clk_data);
+	if (ret)
+		goto unregister_mux;
+
 	ret = hisi_clk_register_gate(hi3798cv200_gate_clks,
 				ARRAY_SIZE(hi3798cv200_gate_clks),
 				clk_data);
 	if (ret)
-		goto unregister_mux;
+		goto unregister_phase;
 
 	ret = of_clk_add_provider(pdev->dev.of_node,
 			of_clk_src_onecell_get, &clk_data->clk_data);
@@ -201,6 +220,10 @@ static struct hisi_clock_data *hi3798cv200_clk_register(
 	hisi_clk_unregister_mux(hi3798cv200_mux_clks,
 				ARRAY_SIZE(hi3798cv200_mux_clks),
 				clk_data);
+unregister_phase:
+	hisi_clk_unregister_phase(hi3798cv200_phase_clks,
+				ARRAY_SIZE(hi3798cv200_phase_clks),
+				clk_data);
 unregister_gate:
 	hisi_clk_unregister_gate(hi3798cv200_gate_clks,
 				ARRAY_SIZE(hi3798cv200_gate_clks),
-- 
2.7.4

[PATCH 3/3] clk: hisilicon: correct ir clock rate for hi3798cv200 SoC

[Jiancheng Xue]

From: Younian Wang <wangyounian@hisilicon.com>

Correct ir clock rate for hi3798cv200 SoC.

Signed-off-by: Younian Wang <wangyounian@hisilicon.com>
---
 drivers/clk/hisilicon/crg-hi3798cv200.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/hisilicon/crg-hi3798cv200.c b/drivers/clk/hisilicon/crg-hi3798cv200.c
index 25d750c..61bd941 100644
--- a/drivers/clk/hisilicon/crg-hi3798cv200.c
+++ b/drivers/clk/hisilicon/crg-hi3798cv200.c
@@ -258,7 +258,7 @@ static const struct hisi_crg_funcs hi3798cv200_crg_funcs = {
 #define HI3798CV200_SYSCTRL_NR_CLKS 16
 
 static const struct hisi_gate_clock hi3798cv200_sysctrl_gate_clks[] = {
-	{ HISTB_IR_CLK, "clk_ir", "100m",
+	{ HISTB_IR_CLK, "clk_ir", "24m",
 		CLK_SET_RATE_PARENT, 0x48, 4, 0, },
 	{ HISTB_TIMER01_CLK, "clk_timer01", "24m",
 		CLK_SET_RATE_PARENT, 0x48, 6, 0, },
-- 
2.7.4

Re: [PATCH 0/3] add more clock definitions for hi3798cv200-poplar board

[Shawn Guo]

Hi Stephen,

On Wed, Oct 18, 2017 at 07:00:26AM -0400, Jiancheng Xue wrote:
> Add more clock definitions for hi3798cv200-poplar board.
> 
> Younian Wang (1):
>   clk: hisilicon: correct ir clock rate for hi3798cv200 SoC
> 
> tianshuliang (2):
>   clk: hisilicon: add hisi phase clock support
>   clk: hisilicon: add emmc sample and drive clock for hi3798cv200 SoC

Since you haven't applied this series, I will update it with a few more
patches added.  I assume it's late for 4.15 merge window anyway, so I
intend to maintain a branch for all those Hi3798CV200 clock patches.
After the patches are properly reviewed, I can rebase the branch to
4.15-rc and then ask you to pull for next merge window.  Sounds good to
you?

Shawn

Re: [PATCH 1/3] clk: hisilicon: add hisi phase clock support

[Shawn Guo]

On Wed, Oct 18, 2017 at 07:00:27AM -0400, Jiancheng Xue wrote:
> From: tianshuliang <tianshuliang@hisilicon.com>
> 
> Add a phase clock type for HiSilicon SoCs,which supports
> clk_set_phase operation.

As the pair of phase operation, I don't see why clk_get_phase operation
is missing.

> 
> Signed-off-by: tianshuliang <tianshuliang@hisilicon.com>
> Signed-off-by: Jiancheng Xue <xuejiancheng@hisilicon.com>
> ---
>  drivers/clk/hisilicon/Makefile         |   2 +-
>  drivers/clk/hisilicon/clk-hisi-phase.c | 117 +++++++++++++++++++++++++++++++++
>  drivers/clk/hisilicon/clk.c            |  45 +++++++++++++
>  drivers/clk/hisilicon/clk.h            |  22 +++++++
>  4 files changed, 185 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/clk/hisilicon/clk-hisi-phase.c
> 
> diff --git a/drivers/clk/hisilicon/Makefile b/drivers/clk/hisilicon/Makefile
> index 1e4c3dd..7189f07 100644
> --- a/drivers/clk/hisilicon/Makefile
> +++ b/drivers/clk/hisilicon/Makefile
> @@ -2,7 +2,7 @@
>  # Hisilicon Clock specific Makefile
>  #
>  
> -obj-y	+= clk.o clkgate-separated.o clkdivider-hi6220.o
> +obj-y	+= clk.o clkgate-separated.o clkdivider-hi6220.o clk-hisi-phase.o
>  
>  obj-$(CONFIG_ARCH_HI3xxx)	+= clk-hi3620.o
>  obj-$(CONFIG_ARCH_HIP04)	+= clk-hip04.o
> diff --git a/drivers/clk/hisilicon/clk-hisi-phase.c b/drivers/clk/hisilicon/clk-hisi-phase.c
> new file mode 100644
> index 0000000..436f0a1
> --- /dev/null
> +++ b/drivers/clk/hisilicon/clk-hisi-phase.c
> @@ -0,0 +1,117 @@
> +/*
> + * Copyright (c) 2017 HiSilicon Technologies Co., Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * Simple HiSilicon phase clock implementation.
> + */
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include "clk.h"
> +
> +struct clk_hisi_phase {
> +	struct clk_hw	hw;
> +	void __iomem	*reg;
> +	u32		*phase_values;
> +	u32		*phase_regs;
> +	u8		phase_num;

I do not think this value-reg table is necessary, as the register value
maps to phase degree in a way that is easy for programming, i.e. degree
increases 45 with register value increases one.

> +	u32		mask;
> +	u8		shift;
> +	u8		flags;
> +	spinlock_t	*lock;
> +};

Have a newline here.

> +#define to_clk_hisi_phase(_hw) container_of(_hw, struct clk_hisi_phase, hw)
> +
> +static u32 hisi_clk_get_phase_reg(struct clk_hisi_phase *phase, int degrees)
> +{
> +	int i;
> +
> +	for (i = 0; i < phase->phase_num; i++)
> +		if (phase->phase_values[i] == degrees)
> +			return phase->phase_regs[i];
> +
> +	return -EINVAL;
> +}
> +
> +static int hisi_clk_set_phase(struct clk_hw *hw, int degrees)
> +{
> +	struct clk_hisi_phase *phase = to_clk_hisi_phase(hw);
> +	u32 val, phase_reg;
> +	unsigned long flags = 0;
> +
> +	phase_reg = hisi_clk_get_phase_reg(phase, degrees);
> +	if (phase_reg < 0)
> +		return phase_reg;
> +
> +	if (phase->lock)
> +		spin_lock_irqsave(phase->lock, flags);
> +	else
> +		__acquire(phase->lock);
> +
> +	val = clk_readl(phase->reg);
> +	val &= ~(phase->mask << phase->shift);
> +	val |= phase_reg << phase->shift;
> +	clk_writel(val, phase->reg);
> +
> +	if (phase->lock)
> +		spin_unlock_irqrestore(phase->lock, flags);
> +	else
> +		__release(phase->lock);
> +
> +	return 0;
> +}
> +
> +const struct clk_ops clk_phase_ops = {
> +	.set_phase = hisi_clk_set_phase,
> +};
> +
> +void clk_unregister_hisi_phase(struct clk *clk)
> +{
> +	struct clk_hisi_phase *phase;
> +	struct clk_hw *hw;
> +
> +	hw = __clk_get_hw(clk);
> +	if (!hw)
> +		return;
> +
> +	phase = to_clk_hisi_phase(hw);
> +	clk_unregister(clk);
> +}
> +EXPORT_SYMBOL_GPL(clk_unregister_hisi_phase);

If there no reason that this unregister function need to be before
register function, I would suggest you put it after register function,
so that you are consistent with other register/unregister function pair
like hisi_clk_register[unregister]_phase() etc.

Shawn

> +
> +struct clk *clk_register_hisi_phase(struct device *dev,
> +		const struct hisi_phase_clock *clks,
> +		void __iomem *base, spinlock_t *lock)
> +{
> +	struct clk_hisi_phase *phase;
> +	struct clk *clk;
> +	struct clk_init_data init;
> +
> +	phase = devm_kzalloc(dev, sizeof(struct clk_hisi_phase), GFP_KERNEL);
> +	if (!phase)
> +		return ERR_PTR(-ENOMEM);
> +
> +	init.name = clks->name;
> +	init.ops = &clk_phase_ops;
> +	init.flags = clks->flags | CLK_IS_BASIC;
> +	init.parent_names = clks->parent_names ? &clks->parent_names : NULL;
> +	init.num_parents = clks->parent_names ? 1 : 0;
> +
> +	phase->reg = base + clks->offset;
> +	phase->shift = clks->shift;
> +	phase->mask = BIT(clks->width) - 1;
> +	phase->lock = lock;
> +	phase->phase_values = clks->phase_values;
> +	phase->phase_regs = clks->phase_regs;
> +	phase->phase_num = clks->phase_num;
> +	phase->hw.init = &init;
> +
> +	clk = clk_register(NULL, &phase->hw);
> +	return clk;
> +}
> +EXPORT_SYMBOL_GPL(clk_register_hisi_phase);
> diff --git a/drivers/clk/hisilicon/clk.c b/drivers/clk/hisilicon/clk.c
> index b73c1df..e3adfad 100644
> --- a/drivers/clk/hisilicon/clk.c
> +++ b/drivers/clk/hisilicon/clk.c
> @@ -197,6 +197,51 @@ int hisi_clk_register_mux(const struct hisi_mux_clock *clks,
>  }
>  EXPORT_SYMBOL_GPL(hisi_clk_register_mux);
>  
> +int hisi_clk_register_phase(struct device *dev,
> +			const struct hisi_phase_clock *clks,
> +			int nums, struct hisi_clock_data *data)
> +{
> +	int i;
> +	struct clk *clk;
> +	void __iomem *base = data->base;
> +
> +	for (i = 0; i < nums; i++) {
> +		clk =	clk_register_hisi_phase(dev,
> +				&clks[i], base, &hisi_clk_lock);
> +
> +		if (IS_ERR(clk)) {
> +			pr_err("%s: failed to register clock %s\n",
> +			       __func__, clks[i].name);
> +			goto err;
> +		}
> +
> +		data->clk_data.clks[clks[i].id] = clk;
> +	}
> +	return 0;
> +
> +err:
> +	while (i--)
> +		clk_unregister_hisi_phase(data->clk_data.clks[clks[i].id]);
> +
> +	return PTR_ERR(clk);
> +}
> +EXPORT_SYMBOL_GPL(hisi_clk_register_phase);
> +
> +void hisi_clk_unregister_phase(const struct hisi_phase_clock *clks,
> +				int nums, struct hisi_clock_data *data)
> +{
> +	struct clk **clocks = data->clk_data.clks;
> +	int i, id;
> +
> +	for (i = 0; i < nums; i++) {
> +		id = clks[i].id;
> +
> +		if (clocks[id])
> +			clk_unregister_hisi_phase(clocks[id]);
> +	}
> +}
> +EXPORT_SYMBOL_GPL(hisi_clk_unregister_phase);
> +
>  int hisi_clk_register_divider(const struct hisi_divider_clock *clks,
>  				      int nums, struct hisi_clock_data *data)
>  {
> diff --git a/drivers/clk/hisilicon/clk.h b/drivers/clk/hisilicon/clk.h
> index 4e1d1af..bc18730 100644
> --- a/drivers/clk/hisilicon/clk.h
> +++ b/drivers/clk/hisilicon/clk.h
> @@ -68,6 +68,19 @@ struct hisi_mux_clock {
>  	const char		*alias;
>  };
>  
> +struct hisi_phase_clock {
> +	unsigned int    id;
> +	const char      *name;
> +	const char      *parent_names;
> +	unsigned long   flags;
> +	unsigned long   offset;
> +	u8		shift;
> +	u8		width;
> +	u32		*phase_values;
> +	u32		*phase_regs;
> +	u8		phase_num;
> +};
> +
>  struct hisi_divider_clock {
>  	unsigned int		id;
>  	const char		*name;
> @@ -120,6 +133,15 @@ int hisi_clk_register_fixed_factor(const struct hisi_fixed_factor_clock *,
>  				int, struct hisi_clock_data *);
>  int hisi_clk_register_mux(const struct hisi_mux_clock *, int,
>  				struct hisi_clock_data *);
> +struct clk *clk_register_hisi_phase(struct device *dev,
> +				const struct hisi_phase_clock *clks,
> +				void __iomem *base, spinlock_t *lock);
> +void clk_unregister_hisi_phase(struct clk *clk);
> +int hisi_clk_register_phase(struct device *dev,
> +				const struct hisi_phase_clock *clks,
> +				int nums, struct hisi_clock_data *data);
> +void hisi_clk_unregister_phase(const struct hisi_phase_clock *clks,
> +				int nums, struct hisi_clock_data *data);
>  int hisi_clk_register_divider(const struct hisi_divider_clock *,
>  				int, struct hisi_clock_data *);
>  int hisi_clk_register_gate(const struct hisi_gate_clock *,
> -- 
> 2.7.4
> 

Re: [PATCH 2/3] clk: hisilicon: add emmc sample and drive clock for hi3798cv200 SoC

[Shawn Guo]

On Wed, Oct 18, 2017 at 07:00:28AM -0400, Jiancheng Xue wrote:
> From: tianshuliang <tianshuliang@hisilicon.com>
> 
> Add emmc sample and emmc drive clock for Hi3798cv200 SoC
> 
> Signed-off-by: tianshuliang <tianshuliang@hisilicon.com>
> Signed-off-by: Jiancheng Xue <xuejiancheng@hisilicon.com>
> ---
>  drivers/clk/hisilicon/crg-hi3798cv200.c | 25 ++++++++++++++++++++++++-
>  1 file changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/hisilicon/crg-hi3798cv200.c b/drivers/clk/hisilicon/crg-hi3798cv200.c
> index ed8bb5f..25d750c 100644
> --- a/drivers/clk/hisilicon/crg-hi3798cv200.c
> +++ b/drivers/clk/hisilicon/crg-hi3798cv200.c
> @@ -83,6 +83,18 @@ static struct hisi_mux_clock hi3798cv200_mux_clks[] = {
>  		CLK_SET_RATE_PARENT, 0x188, 10, 2, 0, comphy1_mux_table, },
>  };
>  
> +static u32 mmc_phase_reg[] = {0, 1, 2, 3, 4, 5, 6, 7};
> +static u32 mmc_phase_val[] = {0, 45, 90, 135, 180, 225, 270, 315};
> +
> +static struct hisi_phase_clock hi3798cv200_phase_clks[] = {
> +	{ HISTB_MMC_SAMPLE_CLK, "mmc_sample", "clk_mmc_ciu",
> +	CLK_SET_RATE_PARENT, 0xa0, 12, 3, mmc_phase_val,
> +	mmc_phase_reg, ARRAY_SIZE(mmc_phase_reg)},
> +	{ HISTB_MMC_DRV_CLK, "mmc_drive", "clk_mmc_ciu",
> +	CLK_SET_RATE_PARENT, 0xa0, 16, 3, mmc_phase_val,
> +	mmc_phase_reg, ARRAY_SIZE(mmc_phase_reg)},

Can we align the indentation with other clock types, like the following?

	{ HISTB_MMC_SAMPLE_CLK, "mmc_sample", "clk_mmc_ciu",
		CLK_SET_RATE_PARENT, 0xa0, 12, 3, mmc_phase_val,
		mmc_phase_reg, ARRAY_SIZE(mmc_phase_reg) },

Also we have a space after '{' and please do the same before '}'.

Shawn

> +};
> +
>  static const struct hisi_gate_clock hi3798cv200_gate_clks[] = {
>  	/* UART */
>  	{ HISTB_UART2_CLK, "clk_uart2", "75m",
> @@ -179,11 +191,18 @@ static struct hisi_clock_data *hi3798cv200_clk_register(
>  	if (ret)
>  		goto unregister_fixed_rate;
>  
> +	ret = hisi_clk_register_phase(&pdev->dev,
> +				hi3798cv200_phase_clks,
> +				ARRAY_SIZE(hi3798cv200_phase_clks),
> +				clk_data);
> +	if (ret)
> +		goto unregister_mux;
> +
>  	ret = hisi_clk_register_gate(hi3798cv200_gate_clks,
>  				ARRAY_SIZE(hi3798cv200_gate_clks),
>  				clk_data);
>  	if (ret)
> -		goto unregister_mux;
> +		goto unregister_phase;
>  
>  	ret = of_clk_add_provider(pdev->dev.of_node,
>  			of_clk_src_onecell_get, &clk_data->clk_data);
> @@ -201,6 +220,10 @@ static struct hisi_clock_data *hi3798cv200_clk_register(
>  	hisi_clk_unregister_mux(hi3798cv200_mux_clks,
>  				ARRAY_SIZE(hi3798cv200_mux_clks),
>  				clk_data);
> +unregister_phase:
> +	hisi_clk_unregister_phase(hi3798cv200_phase_clks,
> +				ARRAY_SIZE(hi3798cv200_phase_clks),
> +				clk_data);
>  unregister_gate:
>  	hisi_clk_unregister_gate(hi3798cv200_gate_clks,
>  				ARRAY_SIZE(hi3798cv200_gate_clks),
> -- 
> 2.7.4
> 

Re: [project-aspen-dev] Re: [PATCH 1/3] clk: hisilicon: add hisi phase clock support

[Jiancheng Xue]

Hi Shawn,

On 2017/11/16 10:31, Shawn Guo wrote:
> On Wed, Oct 18, 2017 at 07:00:27AM -0400, Jiancheng Xue wrote:
>> From: tianshuliang <tianshuliang@hisilicon.com>
>>
>> Add a phase clock type for HiSilicon SoCs,which supports
>> clk_set_phase operation.
> 
> As the pair of phase operation, I don't see why clk_get_phase operation
> is missing.
> 
>>
>> Signed-off-by: tianshuliang <tianshuliang@hisilicon.com>
>> Signed-off-by: Jiancheng Xue <xuejiancheng@hisilicon.com>
>> ---
>>  drivers/clk/hisilicon/Makefile         |   2 +-
>>  drivers/clk/hisilicon/clk-hisi-phase.c | 117 +++++++++++++++++++++++++++++++++
>>  drivers/clk/hisilicon/clk.c            |  45 +++++++++++++
>>  drivers/clk/hisilicon/clk.h            |  22 +++++++
>>  4 files changed, 185 insertions(+), 1 deletion(-)
>>  create mode 100644 drivers/clk/hisilicon/clk-hisi-phase.c
>>
>> diff --git a/drivers/clk/hisilicon/Makefile b/drivers/clk/hisilicon/Makefile
>> index 1e4c3dd..7189f07 100644
>> --- a/drivers/clk/hisilicon/Makefile
>> +++ b/drivers/clk/hisilicon/Makefile
>> @@ -2,7 +2,7 @@
>>  # Hisilicon Clock specific Makefile
>>  #
>>  
>> -obj-y	+= clk.o clkgate-separated.o clkdivider-hi6220.o
>> +obj-y	+= clk.o clkgate-separated.o clkdivider-hi6220.o clk-hisi-phase.o
>>  
>>  obj-$(CONFIG_ARCH_HI3xxx)	+= clk-hi3620.o
>>  obj-$(CONFIG_ARCH_HIP04)	+= clk-hip04.o
>> diff --git a/drivers/clk/hisilicon/clk-hisi-phase.c b/drivers/clk/hisilicon/clk-hisi-phase.c
>> new file mode 100644
>> index 0000000..436f0a1
>> --- /dev/null
>> +++ b/drivers/clk/hisilicon/clk-hisi-phase.c
>> @@ -0,0 +1,117 @@
>> +/*
>> + * Copyright (c) 2017 HiSilicon Technologies Co., Ltd.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + *
>> + * Simple HiSilicon phase clock implementation.
>> + */
>> +#include <linux/err.h>
>> +#include <linux/io.h>
>> +#include <linux/module.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/slab.h>
>> +#include "clk.h"
>> +
>> +struct clk_hisi_phase {
>> +	struct clk_hw	hw;
>> +	void __iomem	*reg;
>> +	u32		*phase_values;
>> +	u32		*phase_regs;
>> +	u8		phase_num;
> 
> I do not think this value-reg table is necessary, as the register value
> maps to phase degree in a way that is easy for programming, i.e. degree
> increases 45 with register value increases one.
> 
We expected that this interface could be more generic. That means it can
be also used in other maps instances.

Regards,
Jiancheng

>> +	u32		mask;
>> +	u8		shift;
>> +	u8		flags;
>> +	spinlock_t	*lock;
>> +};
> 
> Have a newline here.
> 
>> +#define to_clk_hisi_phase(_hw) container_of(_hw, struct clk_hisi_phase, hw)
>> +
>> +static u32 hisi_clk_get_phase_reg(struct clk_hisi_phase *phase, int degrees)
>> +{
>> +	int i;
>> +
>> +	for (i = 0; i < phase->phase_num; i++)
>> +		if (phase->phase_values[i] == degrees)
>> +			return phase->phase_regs[i];
>> +
>> +	return -EINVAL;
>> +}
>> +
>> +static int hisi_clk_set_phase(struct clk_hw *hw, int degrees)
>> +{
>> +	struct clk_hisi_phase *phase = to_clk_hisi_phase(hw);
>> +	u32 val, phase_reg;
>> +	unsigned long flags = 0;
>> +
>> +	phase_reg = hisi_clk_get_phase_reg(phase, degrees);
>> +	if (phase_reg < 0)
>> +		return phase_reg;
>> +
>> +	if (phase->lock)
>> +		spin_lock_irqsave(phase->lock, flags);
>> +	else
>> +		__acquire(phase->lock);
>> +
>> +	val = clk_readl(phase->reg);
>> +	val &= ~(phase->mask << phase->shift);
>> +	val |= phase_reg << phase->shift;
>> +	clk_writel(val, phase->reg);
>> +
>> +	if (phase->lock)
>> +		spin_unlock_irqrestore(phase->lock, flags);
>> +	else
>> +		__release(phase->lock);
>> +
>> +	return 0;
>> +}
>> +
>> +const struct clk_ops clk_phase_ops = {
>> +	.set_phase = hisi_clk_set_phase,
>> +};
>> +
>> +void clk_unregister_hisi_phase(struct clk *clk)
>> +{
>> +	struct clk_hisi_phase *phase;
>> +	struct clk_hw *hw;
>> +
>> +	hw = __clk_get_hw(clk);
>> +	if (!hw)
>> +		return;
>> +
>> +	phase = to_clk_hisi_phase(hw);
>> +	clk_unregister(clk);
>> +}
>> +EXPORT_SYMBOL_GPL(clk_unregister_hisi_phase);
> 
> If there no reason that this unregister function need to be before
> register function, I would suggest you put it after register function,
> so that you are consistent with other register/unregister function pair
> like hisi_clk_register[unregister]_phase() etc.
> 
> Shawn
> 
>> +
>> +struct clk *clk_register_hisi_phase(struct device *dev,
>> +		const struct hisi_phase_clock *clks,
>> +		void __iomem *base, spinlock_t *lock)
>> +{
>> +	struct clk_hisi_phase *phase;
>> +	struct clk *clk;
>> +	struct clk_init_data init;
>> +
>> +	phase = devm_kzalloc(dev, sizeof(struct clk_hisi_phase), GFP_KERNEL);
>> +	if (!phase)
>> +		return ERR_PTR(-ENOMEM);
>> +
>> +	init.name = clks->name;
>> +	init.ops = &clk_phase_ops;
>> +	init.flags = clks->flags | CLK_IS_BASIC;
>> +	init.parent_names = clks->parent_names ? &clks->parent_names : NULL;
>> +	init.num_parents = clks->parent_names ? 1 : 0;
>> +
>> +	phase->reg = base + clks->offset;
>> +	phase->shift = clks->shift;
>> +	phase->mask = BIT(clks->width) - 1;
>> +	phase->lock = lock;
>> +	phase->phase_values = clks->phase_values;
>> +	phase->phase_regs = clks->phase_regs;
>> +	phase->phase_num = clks->phase_num;
>> +	phase->hw.init = &init;
>> +
>> +	clk = clk_register(NULL, &phase->hw);
>> +	return clk;
>> +}
>> +EXPORT_SYMBOL_GPL(clk_register_hisi_phase);
>> diff --git a/drivers/clk/hisilicon/clk.c b/drivers/clk/hisilicon/clk.c
>> index b73c1df..e3adfad 100644
>> --- a/drivers/clk/hisilicon/clk.c
>> +++ b/drivers/clk/hisilicon/clk.c
>> @@ -197,6 +197,51 @@ int hisi_clk_register_mux(const struct hisi_mux_clock *clks,
>>  }
>>  EXPORT_SYMBOL_GPL(hisi_clk_register_mux);
>>  
>> +int hisi_clk_register_phase(struct device *dev,
>> +			const struct hisi_phase_clock *clks,
>> +			int nums, struct hisi_clock_data *data)
>> +{
>> +	int i;
>> +	struct clk *clk;
>> +	void __iomem *base = data->base;
>> +
>> +	for (i = 0; i < nums; i++) {
>> +		clk =	clk_register_hisi_phase(dev,
>> +				&clks[i], base, &hisi_clk_lock);
>> +
>> +		if (IS_ERR(clk)) {
>> +			pr_err("%s: failed to register clock %s\n",
>> +			       __func__, clks[i].name);
>> +			goto err;
>> +		}
>> +
>> +		data->clk_data.clks[clks[i].id] = clk;
>> +	}
>> +	return 0;
>> +
>> +err:
>> +	while (i--)
>> +		clk_unregister_hisi_phase(data->clk_data.clks[clks[i].id]);
>> +
>> +	return PTR_ERR(clk);
>> +}
>> +EXPORT_SYMBOL_GPL(hisi_clk_register_phase);
>> +
>> +void hisi_clk_unregister_phase(const struct hisi_phase_clock *clks,
>> +				int nums, struct hisi_clock_data *data)
>> +{
>> +	struct clk **clocks = data->clk_data.clks;
>> +	int i, id;
>> +
>> +	for (i = 0; i < nums; i++) {
>> +		id = clks[i].id;
>> +
>> +		if (clocks[id])
>> +			clk_unregister_hisi_phase(clocks[id]);
>> +	}
>> +}
>> +EXPORT_SYMBOL_GPL(hisi_clk_unregister_phase);
>> +
>>  int hisi_clk_register_divider(const struct hisi_divider_clock *clks,
>>  				      int nums, struct hisi_clock_data *data)
>>  {
>> diff --git a/drivers/clk/hisilicon/clk.h b/drivers/clk/hisilicon/clk.h
>> index 4e1d1af..bc18730 100644
>> --- a/drivers/clk/hisilicon/clk.h
>> +++ b/drivers/clk/hisilicon/clk.h
>> @@ -68,6 +68,19 @@ struct hisi_mux_clock {
>>  	const char		*alias;
>>  };
>>  
>> +struct hisi_phase_clock {
>> +	unsigned int    id;
>> +	const char      *name;
>> +	const char      *parent_names;
>> +	unsigned long   flags;
>> +	unsigned long   offset;
>> +	u8		shift;
>> +	u8		width;
>> +	u32		*phase_values;
>> +	u32		*phase_regs;
>> +	u8		phase_num;
>> +};
>> +
>>  struct hisi_divider_clock {
>>  	unsigned int		id;
>>  	const char		*name;
>> @@ -120,6 +133,15 @@ int hisi_clk_register_fixed_factor(const struct hisi_fixed_factor_clock *,
>>  				int, struct hisi_clock_data *);
>>  int hisi_clk_register_mux(const struct hisi_mux_clock *, int,
>>  				struct hisi_clock_data *);
>> +struct clk *clk_register_hisi_phase(struct device *dev,
>> +				const struct hisi_phase_clock *clks,
>> +				void __iomem *base, spinlock_t *lock);
>> +void clk_unregister_hisi_phase(struct clk *clk);
>> +int hisi_clk_register_phase(struct device *dev,
>> +				const struct hisi_phase_clock *clks,
>> +				int nums, struct hisi_clock_data *data);
>> +void hisi_clk_unregister_phase(const struct hisi_phase_clock *clks,
>> +				int nums, struct hisi_clock_data *data);
>>  int hisi_clk_register_divider(const struct hisi_divider_clock *,
>>  				int, struct hisi_clock_data *);
>>  int hisi_clk_register_gate(const struct hisi_gate_clock *,
>> -- 
>> 2.7.4
>>
> 
> .
> 

Re: [project-aspen-dev] Re: [PATCH 1/3] clk: hisilicon: add hisi phase clock support

[Shawn Guo]

On Thu, Nov 16, 2017 at 11:40:18AM +0800, Jiancheng Xue wrote:
> >> +struct clk_hisi_phase {
> >> +	struct clk_hw	hw;
> >> +	void __iomem	*reg;
> >> +	u32		*phase_values;
> >> +	u32		*phase_regs;
> >> +	u8		phase_num;
> > 
> > I do not think this value-reg table is necessary, as the register value
> > maps to phase degree in a way that is easy for programming, i.e. degree
> > increases 45 with register value increases one.
> > 
> We expected that this interface could be more generic. That means it can
> be also used in other maps instances.

Okay, if you think there will be some case using different programming
model, I'm fine with it.

Shawn

Re: [PATCH 0/3] add more clock definitions for hi3798cv200-poplar board

[Stephen Boyd]

On 11/16, Shawn Guo wrote:
> Hi Stephen,
> 
> On Wed, Oct 18, 2017 at 07:00:26AM -0400, Jiancheng Xue wrote:
> > Add more clock definitions for hi3798cv200-poplar board.
> > 
> > Younian Wang (1):
> >   clk: hisilicon: correct ir clock rate for hi3798cv200 SoC
> > 
> > tianshuliang (2):
> >   clk: hisilicon: add hisi phase clock support
> >   clk: hisilicon: add emmc sample and drive clock for hi3798cv200 SoC
> 
> Since you haven't applied this series, I will update it with a few more
> patches added.  I assume it's late for 4.15 merge window anyway, so I
> intend to maintain a branch for all those Hi3798CV200 clock patches.
> After the patches are properly reviewed, I can rebase the branch to
> 4.15-rc and then ask you to pull for next merge window.  Sounds good to
> you?

Yes that's fine. I am sending the PR today/tomorrow.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project