linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/1] add support for the NAND clocks on Meson8b
@ 2017-04-01 13:10 Martin Blumenstingl
  2017-04-01 13:10 ` [PATCH 1/1] clk: meson: meson8b: add support for the NAND clocks Martin Blumenstingl
  0 siblings, 1 reply; 4+ messages in thread
From: Martin Blumenstingl @ 2017-04-01 13:10 UTC (permalink / raw)
  To: linux-arm-kernel

This adds support for the NAND clocks found in the Meson8b SoC.
The clocks consist of a simple gate, a divider and a mux. The mux
parents are not documented in the public S805 datasheet [0], so I
had to use a bit of math and take the vendor kernel as reference [1]
to find the actual parent clocks.
Some mux parents cannot be divided down without remainder to the
target clocks (as expected by the vendor NAND driver) these clocks
have the ROUND_CLOSEST flag set.

This is based on the "clk-meson" branch (e65ae3fb97b4 "dt-bindings:
clock: gxbb-clkc: Add GXL compatible variant")


[0] http://dn.odroid.com/S805/Datasheet/S805_Datasheet%20V0.8%2020150126.pdf
[1] https://github.com/khadas/linux/blob/9587681285cb/drivers/amlogic/amlnf/dev/amlnf_ctrl.c#L314

Martin Blumenstingl (1):
  clk: meson: meson8b: add support for the NAND clocks

 drivers/clk/meson/meson8b.c | 53 +++++++++++++++++++++++++++++++++++++++++++++
 drivers/clk/meson/meson8b.h |  6 ++++-
 2 files changed, 58 insertions(+), 1 deletion(-)

-- 
2.12.1

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 1/1] clk: meson: meson8b: add support for the NAND clocks
  2017-04-01 13:10 [PATCH 0/1] add support for the NAND clocks on Meson8b Martin Blumenstingl
@ 2017-04-01 13:10 ` Martin Blumenstingl
  2017-04-02 15:38   ` Jerome Brunet
  0 siblings, 1 reply; 4+ messages in thread
From: Martin Blumenstingl @ 2017-04-01 13:10 UTC (permalink / raw)
  To: linux-arm-kernel

This adds the NAND clocks (from the HHI_NAND_CLK_CNTL register) to the
Meson8b clock driver. There are three NAND clocks: a gate which enables
or disables the NAND clock, a mux and a divider (which divides the mux
output).
Unfortunately the public S805 datasheet does not document the mux
parents. However, the vendor kernel has a few hints for us which allows
us to make an educated guess about the clock parents. To do this we need
to have a look at set_nand_core_clk() from the vendor's NAND driver (see
[0]):
- XTAL = (4<<9) | (1<<8) | 0
- 160MHz = (0<<9) | (1<<8) | 3)
- 182MHz = (3<<9) | (1<<8) | 1)
- 212MHz = (1<<9) | (1<<8) | 3)
- 255MHz = (2<<9) | (1<<8) | 1)

While there is a comment for the XTAL parent (which indicates that it
should only be used for debugging) we have to do a bit of math for the
other parents: target_freq * divider = rate of parent clock
Bit 8 above is the enable bit, so we can ignore it here. Bits 11:9 are
the mux index and bits 6:0 are the 0-based divider (so we need to add
1). This gives us:
- mux 0 (160MHz * 4) = fclk_div4 (actual rate = 637.5MHz, off by 2.5MHz)
- mux 1 (212MHz * 4) = fclk_div3 (actual rate = 850MHz, off by 2MHz)
- mux 2 (255MHz * 2) = fclk_div5 (matches exactly 510MHz)
- mux 3 (182MHz * 2) = fclk_div7 (actual rate = 346.3MHz, off by 0.3MHz)

[0] https://github.com/khadas/linux/blob/9587681285cb/drivers/amlogic/amlnf/dev/amlnf_ctrl.c#L314

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 drivers/clk/meson/meson8b.c | 53 +++++++++++++++++++++++++++++++++++++++++++++
 drivers/clk/meson/meson8b.h |  6 ++++-
 2 files changed, 58 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/meson/meson8b.c b/drivers/clk/meson/meson8b.c
index e9985503165c..b7f3cf8ed386 100644
--- a/drivers/clk/meson/meson8b.c
+++ b/drivers/clk/meson/meson8b.c
@@ -403,6 +403,53 @@ struct clk_gate meson8b_clk81 = {
 	},
 };
 
+static u32 mux_table_nand[]	= { 0, 1, 2, 3, 4 };
+
+struct clk_mux meson8b_nand_clk_sel = {
+	.reg = (void *)HHI_NAND_CLK_CNTL,
+	.mask = 0x7,
+	.shift = 9,
+	.table = mux_table_nand,
+	.flags = CLK_MUX_ROUND_CLOSEST,
+	.lock = &clk_lock,
+	.hw.init = &(struct clk_init_data){
+		.name = "nand_clk_sel",
+		.ops = &clk_mux_ops,
+		/* FIXME all other parents are unknown: */
+		.parent_names = (const char *[]){ "fclk_div4", "fclk_div3",
+			"fclk_div5", "fclk_div7", "xtal" },
+		.num_parents = 5,
+		.flags = CLK_SET_RATE_NO_REPARENT,
+	},
+};
+
+struct clk_divider meson8b_nand_clk_div = {
+	.reg = (void *)HHI_NAND_CLK_CNTL,
+	.shift = 0,
+	.width = 6,
+	.flags = CLK_DIVIDER_ROUND_CLOSEST,
+	.lock = &clk_lock,
+	.hw.init = &(struct clk_init_data){
+		.name = "nand_clk_div",
+		.ops = &clk_divider_ops,
+		.parent_names = (const char *[]){ "nand_clk_sel" },
+		.num_parents = 1,
+	},
+};
+
+struct clk_gate meson8b_nand_clk_gate = {
+	.reg = (void *)HHI_NAND_CLK_CNTL,
+	.bit_idx = 8,
+	.lock = &clk_lock,
+	.hw.init = &(struct clk_init_data){
+		.name = "nand_clk_gate",
+		.ops = &clk_gate_ops,
+		.parent_names = (const char *[]){ "nand_clk_div" },
+		.num_parents = 1,
+		.flags = CLK_SET_RATE_PARENT,
+	},
+};
+
 /* Everything Else (EE) domain gates */
 
 static MESON_GATE(meson8b_ddr, HHI_GCLK_MPEG0, 0);
@@ -584,6 +631,9 @@ static struct clk_hw_onecell_data meson8b_hw_onecell_data = {
 		[CLKID_MPLL0]		    = &meson8b_mpll0.hw,
 		[CLKID_MPLL1]		    = &meson8b_mpll1.hw,
 		[CLKID_MPLL2]		    = &meson8b_mpll2.hw,
+		[CLKID_NAND_SEL]	    = &meson8b_nand_clk_sel.hw,
+		[CLKID_NAND_DIV]	    = &meson8b_nand_clk_div.hw,
+		[CLKID_NAND_CLK]	    = &meson8b_nand_clk_gate.hw,
 	},
 	.num = CLK_NR_CLKS,
 };
@@ -679,14 +729,17 @@ static struct clk_gate *const meson8b_clk_gates[] = {
 	&meson8b_ao_ahb_sram,
 	&meson8b_ao_ahb_bus,
 	&meson8b_ao_iface,
+	&meson8b_nand_clk_gate,
 };
 
 static struct clk_mux *const meson8b_clk_muxes[] = {
 	&meson8b_mpeg_clk_sel,
+	&meson8b_nand_clk_sel,
 };
 
 static struct clk_divider *const meson8b_clk_dividers[] = {
 	&meson8b_mpeg_clk_div,
+	&meson8b_nand_clk_div,
 };
 
 static int meson8b_clkc_probe(struct platform_device *pdev)
diff --git a/drivers/clk/meson/meson8b.h b/drivers/clk/meson/meson8b.h
index 3881defc8644..cbb8001b5fe8 100644
--- a/drivers/clk/meson/meson8b.h
+++ b/drivers/clk/meson/meson8b.h
@@ -37,6 +37,7 @@
 #define HHI_GCLK_AO			0x154 /* 0x55 offset in data sheet */
 #define HHI_SYS_CPU_CLK_CNTL1		0x15c /* 0x57 offset in data sheet */
 #define HHI_MPEG_CLK_CNTL		0x174 /* 0x5d offset in data sheet */
+#define HHI_NAND_CLK_CNTL		0x25c /* 0x97 offset in data sheet */
 #define HHI_MPLL_CNTL			0x280 /* 0xa0 offset in data sheet */
 #define HHI_SYS_PLL_CNTL		0x300 /* 0xc0 offset in data sheet */
 #define HHI_VID_PLL_CNTL		0x320 /* 0xc8 offset in data sheet */
@@ -160,8 +161,11 @@
 #define CLKID_MPLL0		93
 #define CLKID_MPLL1		94
 #define CLKID_MPLL2		95
+#define CLKID_NAND_SEL		96
+#define CLKID_NAND_DIV		97
+#define CLKID_NAND_CLK		98
 
-#define CLK_NR_CLKS		96
+#define CLK_NR_CLKS		99
 
 /* include the CLKIDs that have been made part of the stable DT binding */
 #include <dt-bindings/clock/meson8b-clkc.h>
-- 
2.12.1

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 1/1] clk: meson: meson8b: add support for the NAND clocks
  2017-04-01 13:10 ` [PATCH 1/1] clk: meson: meson8b: add support for the NAND clocks Martin Blumenstingl
@ 2017-04-02 15:38   ` Jerome Brunet
  2017-04-02 18:54     ` Martin Blumenstingl
  0 siblings, 1 reply; 4+ messages in thread
From: Jerome Brunet @ 2017-04-02 15:38 UTC (permalink / raw)
  To: linux-arm-kernel

On Sat, 2017-04-01 at 15:10 +0200, Martin Blumenstingl wrote:
> This adds the NAND clocks (from the HHI_NAND_CLK_CNTL register) to the
> Meson8b clock driver. There are three NAND clocks: a gate which enables
> or disables the NAND clock, a mux and a divider (which divides the mux
> output).
> Unfortunately the public S805 datasheet does not document the mux
> parents. However, the vendor kernel has a few hints for us which allows
> us to make an educated guess about the clock parents. To do this we need
> to have a look at set_nand_core_clk() from the vendor's NAND driver (see
> [0]):
> - XTAL = (4<<9) | (1<<8) | 0
> - 160MHz = (0<<9) | (1<<8) | 3)
> - 182MHz = (3<<9) | (1<<8) | 1)
> - 212MHz = (1<<9) | (1<<8) | 3)
> - 255MHz = (2<<9) | (1<<8) | 1)
> 
> should only be used for debugging) we have to do a bit of math for the
> other parents: target_freq * divider = rate of parent clock
> Bit 8 above is the enable bit, so we can ignore it here. Bits 11:9 are
> the mux index and bits 6:0 are the 0-based divider (so we need to add
> 1). This gives us:
> - mux 0 (160MHz * 4) = fclk_div4 (actual rate = 637.5MHz, off by 2.5MHz)
> - mux 1 (212MHz * 4) = fclk_div3 (actual rate = 850MHz, off by 2MHz)
> - mux 2 (255MHz * 2) = fclk_div5 (matches exactly 510MHz)
> - mux 3 (182MHz * 2) = fclk_div7 (actual rate = 346.3MHz, off by 0.3MHz)
> 

Good job detective ;)
What is strange is that amlogic seems to have change the index of these parents
on the gxbb. I have a documentation (which I'm not at liberty to share
unfortunately) which gives the following index on the S905:
0: xtal
1: fclk_div2
2: fclk_div3
3: flck_div5
4: fclk_div7
5: mpll2
6: mpll3
7: gp0

Did they change the ids, or is the documentation wrong ? I don't know, but we'll
to pay attention to that if you port your NAND work the gx family.

> [0]
> https://github.com/khadas/linux/blob/9587681285cb/drivers/amlogic/amlnf/dev/am
> lnf_ctrl.c#L314
> 
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
> ?drivers/clk/meson/meson8b.c | 53
> +++++++++++++++++++++++++++++++++++++++++++++
> ?drivers/clk/meson/meson8b.h |??6 ++++-
> ?2 files changed, 58 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/meson/meson8b.c b/drivers/clk/meson/meson8b.c
> index e9985503165c..b7f3cf8ed386 100644
> --- a/drivers/clk/meson/meson8b.c
> +++ b/drivers/clk/meson/meson8b.c
> @@ -403,6 +403,53 @@ struct clk_gate meson8b_clk81 = {
> ?	},
> ?};
> ?
> +static u32 mux_table_nand[]	= { 0, 1, 2, 3, 4 };
You can drop this table, you don't need it.

> +
> +struct clk_mux meson8b_nand_clk_sel = {
> +	.reg = (void *)HHI_NAND_CLK_CNTL,
> +	.mask = 0x7,
> +	.shift = 9,
> +	.table = mux_table_nand,
> +	.flags = CLK_MUX_ROUND_CLOSEST,

If you go for CLK_SET_RATE_NO_REPARENT, I think this flag is not really useful.

> +	.lock = &clk_lock,
> +	.hw.init = &(struct clk_init_data){
> +		.name = "nand_clk_sel",
> +		.ops = &clk_mux_ops,
> +		/* FIXME all other parents are unknown: */
> +		.parent_names = (const char *[]){ "fclk_div4", "fclk_div3",
> +			"fclk_div5", "fclk_div7", "xtal" },
> +		.num_parents = 5,
> +		.flags = CLK_SET_RATE_NO_REPARENT,

PSB

> +	},
> +};
> +
> +struct clk_divider meson8b_nand_clk_div = {
> +	.reg = (void *)HHI_NAND_CLK_CNTL,
> +	.shift = 0,
> +	.width = 6,
> +	.flags = CLK_DIVIDER_ROUND_CLOSEST,
> +	.lock = &clk_lock,
> +	.hw.init = &(struct clk_init_data){
> +		.name = "nand_clk_div",
> +		.ops = &clk_divider_ops,
> +		.parent_names = (const char *[]){ "nand_clk_sel" },
> +		.num_parents = 1,

I think you are trying to tightly control all those elements in your nand
driver, aren't you ? To mimic the vendor driver settings, maybe ?

While it would surely work, another way is possible. You could give the flag
CLK_SET_RATE_PARENT (and drop the CLK_SET_RATE_NO_REPARENT) to all those
elements and let the CCF do the work for you.

There is a good chance it will come with same settings anyway (if that's the
best math can provide)

The good thing about it is that it hides those quirks from your driver. So if
the gxbb has slighly different clock setup for the nand, you won't have to deal
with it in your driver, just call clk_set_rate on the gate and be done with it.

Could you give it try ?

> +	},
> +};
> +
> +struct clk_gate meson8b_nand_clk_gate = {
> +	.reg = (void *)HHI_NAND_CLK_CNTL,
> +	.bit_idx = 8,
> +	.lock = &clk_lock,
> +	.hw.init = &(struct clk_init_data){
> +		.name = "nand_clk_gate",
> +		.ops = &clk_gate_ops,
> +		.parent_names = (const char *[]){ "nand_clk_div" },
> +		.num_parents = 1,
> +		.flags = CLK_SET_RATE_PARENT,
> +	},
> +};
> +
> ?/* Everything Else (EE) domain gates */
> ?
> ?static MESON_GATE(meson8b_ddr, HHI_GCLK_MPEG0, 0);
> @@ -584,6 +631,9 @@ static struct clk_hw_onecell_data meson8b_hw_onecell_data
> = {
> ?		[CLKID_MPLL0]		????= &meson8b_mpll0.hw,
> ?		[CLKID_MPLL1]		????= &meson8b_mpll1.hw,
> ?		[CLKID_MPLL2]		????= &meson8b_mpll2.hw,
> +		[CLKID_NAND_SEL]	????= &meson8b_nand_clk_sel.hw,
> +		[CLKID_NAND_DIV]	????= &meson8b_nand_clk_div.hw,
> +		[CLKID_NAND_CLK]	????= &meson8b_nand_clk_gate.hw,
> ?	},
> ?	.num = CLK_NR_CLKS,
> ?};
> @@ -679,14 +729,17 @@ static struct clk_gate *const meson8b_clk_gates[] = {
> ?	&meson8b_ao_ahb_sram,
> ?	&meson8b_ao_ahb_bus,
> ?	&meson8b_ao_iface,
> +	&meson8b_nand_clk_gate,
> ?};
> ?
> ?static struct clk_mux *const meson8b_clk_muxes[] = {
> ?	&meson8b_mpeg_clk_sel,
> +	&meson8b_nand_clk_sel,
> ?};
> ?
> ?static struct clk_divider *const meson8b_clk_dividers[] = {
> ?	&meson8b_mpeg_clk_div,
> +	&meson8b_nand_clk_div,
> ?};
> ?
> ?static int meson8b_clkc_probe(struct platform_device *pdev)
> diff --git a/drivers/clk/meson/meson8b.h b/drivers/clk/meson/meson8b.h
> index 3881defc8644..cbb8001b5fe8 100644
> --- a/drivers/clk/meson/meson8b.h
> +++ b/drivers/clk/meson/meson8b.h
> @@ -37,6 +37,7 @@
> ?#define HHI_GCLK_AO			0x154 /* 0x55 offset in data sheet
> */
> ?#define HHI_SYS_CPU_CLK_CNTL1		0x15c /* 0x57 offset in data
> sheet */
> ?#define HHI_MPEG_CLK_CNTL		0x174 /* 0x5d offset in data sheet
> */
> +#define HHI_NAND_CLK_CNTL		0x25c /* 0x97 offset in data sheet
> */
> ?#define HHI_MPLL_CNTL			0x280 /* 0xa0 offset in data
> sheet */
> ?#define HHI_SYS_PLL_CNTL		0x300 /* 0xc0 offset in data sheet */
> ?#define HHI_VID_PLL_CNTL		0x320 /* 0xc8 offset in data sheet */
> @@ -160,8 +161,11 @@
> ?#define CLKID_MPLL0		93
> ?#define CLKID_MPLL1		94
> ?#define CLKID_MPLL2		95
> +#define CLKID_NAND_SEL		96
> +#define CLKID_NAND_DIV		97
> +#define CLKID_NAND_CLK		98
> ?
> -#define CLK_NR_CLKS		96
> +#define CLK_NR_CLKS		99
> ?
> ?/* include the CLKIDs that have been made part of the stable DT binding */
> ?#include <dt-bindings/clock/meson8b-clkc.h>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 1/1] clk: meson: meson8b: add support for the NAND clocks
  2017-04-02 15:38   ` Jerome Brunet
@ 2017-04-02 18:54     ` Martin Blumenstingl
  0 siblings, 0 replies; 4+ messages in thread
From: Martin Blumenstingl @ 2017-04-02 18:54 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Jerome,

thanks for taking time to review this!

On Sun, Apr 2, 2017 at 5:38 PM, Jerome Brunet <jbrunet@baylibre.com> wrote:
> On Sat, 2017-04-01 at 15:10 +0200, Martin Blumenstingl wrote:
>> This adds the NAND clocks (from the HHI_NAND_CLK_CNTL register) to the
>> Meson8b clock driver. There are three NAND clocks: a gate which enables
>> or disables the NAND clock, a mux and a divider (which divides the mux
>> output).
>> Unfortunately the public S805 datasheet does not document the mux
>> parents. However, the vendor kernel has a few hints for us which allows
>> us to make an educated guess about the clock parents. To do this we need
>> to have a look at set_nand_core_clk() from the vendor's NAND driver (see
>> [0]):
>> - XTAL = (4<<9) | (1<<8) | 0
>> - 160MHz = (0<<9) | (1<<8) | 3)
>> - 182MHz = (3<<9) | (1<<8) | 1)
>> - 212MHz = (1<<9) | (1<<8) | 3)
>> - 255MHz = (2<<9) | (1<<8) | 1)
>>
>> should only be used for debugging) we have to do a bit of math for the
>> other parents: target_freq * divider = rate of parent clock
>> Bit 8 above is the enable bit, so we can ignore it here. Bits 11:9 are
>> the mux index and bits 6:0 are the 0-based divider (so we need to add
>> 1). This gives us:
>> - mux 0 (160MHz * 4) = fclk_div4 (actual rate = 637.5MHz, off by 2.5MHz)
>> - mux 1 (212MHz * 4) = fclk_div3 (actual rate = 850MHz, off by 2MHz)
>> - mux 2 (255MHz * 2) = fclk_div5 (matches exactly 510MHz)
>> - mux 3 (182MHz * 2) = fclk_div7 (actual rate = 346.3MHz, off by 0.3MHz)
>>
>
> Good job detective ;)
> What is strange is that amlogic seems to have change the index of these parents
> on the gxbb. I have a documentation (which I'm not at liberty to share
> unfortunately) which gives the following index on the S905:
> 0: xtal
> 1: fclk_div2
> 2: fclk_div3
> 3: flck_div5
> 4: fclk_div7
> 5: mpll2
> 6: mpll3
> 7: gp0
>
> Did they change the ids, or is the documentation wrong ? I don't know, but we'll
> to pay attention to that if you port your NAND work the gx family.
I guess they changed the IDs. on Meson8b the only NAND clock rates
that are actually used are 212MHz and 255MHz (hardcoded in their
driver).
however, on GXBB there's a tiny difference: there 200MHz and 250MHz are used.

my assumption was: if they change even the clock frequencies then it's
likely that the mux parents are different as well


>> [0]
>> https://github.com/khadas/linux/blob/9587681285cb/drivers/amlogic/amlnf/dev/am
>> lnf_ctrl.c#L314
>>
>> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
>> ---
>>  drivers/clk/meson/meson8b.c | 53
>> +++++++++++++++++++++++++++++++++++++++++++++
>>  drivers/clk/meson/meson8b.h |  6 ++++-
>>  2 files changed, 58 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/clk/meson/meson8b.c b/drivers/clk/meson/meson8b.c
>> index e9985503165c..b7f3cf8ed386 100644
>> --- a/drivers/clk/meson/meson8b.c
>> +++ b/drivers/clk/meson/meson8b.c
>> @@ -403,6 +403,53 @@ struct clk_gate meson8b_clk81 = {
>>       },
>>  };
>>
>> +static u32 mux_table_nand[]  = { 0, 1, 2, 3, 4 };
> You can drop this table, you don't need it.
>
>> +
>> +struct clk_mux meson8b_nand_clk_sel = {
>> +     .reg = (void *)HHI_NAND_CLK_CNTL,
>> +     .mask = 0x7,
>> +     .shift = 9,
>> +     .table = mux_table_nand,
>> +     .flags = CLK_MUX_ROUND_CLOSEST,
>
> If you go for CLK_SET_RATE_NO_REPARENT, I think this flag is not really useful.
yep, I'd rather leave it and remove CLK_SET_RATE_NO_REPARENT below

>> +     .lock = &clk_lock,
>> +     .hw.init = &(struct clk_init_data){
>> +             .name = "nand_clk_sel",
>> +             .ops = &clk_mux_ops,
>> +             /* FIXME all other parents are unknown: */
>> +             .parent_names = (const char *[]){ "fclk_div4", "fclk_div3",
>> +                     "fclk_div5", "fclk_div7", "xtal" },
>> +             .num_parents = 5,
>> +             .flags = CLK_SET_RATE_NO_REPARENT,
>
> PSB
>
>> +     },
>> +};
>> +
>> +struct clk_divider meson8b_nand_clk_div = {
>> +     .reg = (void *)HHI_NAND_CLK_CNTL,
>> +     .shift = 0,
>> +     .width = 6,
>> +     .flags = CLK_DIVIDER_ROUND_CLOSEST,
>> +     .lock = &clk_lock,
>> +     .hw.init = &(struct clk_init_data){
>> +             .name = "nand_clk_div",
>> +             .ops = &clk_divider_ops,
>> +             .parent_names = (const char *[]){ "nand_clk_sel" },
>> +             .num_parents = 1,
>
> I think you are trying to tightly control all those elements in your nand
> driver, aren't you ? To mimic the vendor driver settings, maybe ?
>
> While it would surely work, another way is possible. You could give the flag
> CLK_SET_RATE_PARENT (and drop the CLK_SET_RATE_NO_REPARENT) to all those
> elements and let the CCF do the work for you.
>
> There is a good chance it will come with same settings anyway (if that's the
> best math can provide)
>
> The good thing about it is that it hides those quirks from your driver. So if
> the gxbb has slighly different clock setup for the nand, you won't have to deal
> with it in your driver, just call clk_set_rate on the gate and be done with it.
>
> Could you give it try ?
it seems that I missed this:
at first I thought that I have to set the mux parent manually.
but after playing around with my NAND driver code I figured out that
(like you suggested) I could probably get away by only setting the
rate on the gate and let the framework do the rest.

I'll send an update as soon as I have time

>> +     },
>> +};
>> +
>> +struct clk_gate meson8b_nand_clk_gate = {
>> +     .reg = (void *)HHI_NAND_CLK_CNTL,
>> +     .bit_idx = 8,
>> +     .lock = &clk_lock,
>> +     .hw.init = &(struct clk_init_data){
>> +             .name = "nand_clk_gate",
>> +             .ops = &clk_gate_ops,
>> +             .parent_names = (const char *[]){ "nand_clk_div" },
>> +             .num_parents = 1,
>> +             .flags = CLK_SET_RATE_PARENT,
>> +     },
>> +};
>> +
>>  /* Everything Else (EE) domain gates */
>>
>>  static MESON_GATE(meson8b_ddr, HHI_GCLK_MPEG0, 0);
>> @@ -584,6 +631,9 @@ static struct clk_hw_onecell_data meson8b_hw_onecell_data
>> = {
>>               [CLKID_MPLL0]               = &meson8b_mpll0.hw,
>>               [CLKID_MPLL1]               = &meson8b_mpll1.hw,
>>               [CLKID_MPLL2]               = &meson8b_mpll2.hw,
>> +             [CLKID_NAND_SEL]            = &meson8b_nand_clk_sel.hw,
>> +             [CLKID_NAND_DIV]            = &meson8b_nand_clk_div.hw,
>> +             [CLKID_NAND_CLK]            = &meson8b_nand_clk_gate.hw,
>>       },
>>       .num = CLK_NR_CLKS,
>>  };
>> @@ -679,14 +729,17 @@ static struct clk_gate *const meson8b_clk_gates[] = {
>>       &meson8b_ao_ahb_sram,
>>       &meson8b_ao_ahb_bus,
>>       &meson8b_ao_iface,
>> +     &meson8b_nand_clk_gate,
>>  };
>>
>>  static struct clk_mux *const meson8b_clk_muxes[] = {
>>       &meson8b_mpeg_clk_sel,
>> +     &meson8b_nand_clk_sel,
>>  };
>>
>>  static struct clk_divider *const meson8b_clk_dividers[] = {
>>       &meson8b_mpeg_clk_div,
>> +     &meson8b_nand_clk_div,
>>  };
>>
>>  static int meson8b_clkc_probe(struct platform_device *pdev)
>> diff --git a/drivers/clk/meson/meson8b.h b/drivers/clk/meson/meson8b.h
>> index 3881defc8644..cbb8001b5fe8 100644
>> --- a/drivers/clk/meson/meson8b.h
>> +++ b/drivers/clk/meson/meson8b.h
>> @@ -37,6 +37,7 @@
>>  #define HHI_GCLK_AO                  0x154 /* 0x55 offset in data sheet
>> */
>>  #define HHI_SYS_CPU_CLK_CNTL1                0x15c /* 0x57 offset in data
>> sheet */
>>  #define HHI_MPEG_CLK_CNTL            0x174 /* 0x5d offset in data sheet
>> */
>> +#define HHI_NAND_CLK_CNTL            0x25c /* 0x97 offset in data sheet
>> */
>>  #define HHI_MPLL_CNTL                        0x280 /* 0xa0 offset in data
>> sheet */
>>  #define HHI_SYS_PLL_CNTL             0x300 /* 0xc0 offset in data sheet */
>>  #define HHI_VID_PLL_CNTL             0x320 /* 0xc8 offset in data sheet */
>> @@ -160,8 +161,11 @@
>>  #define CLKID_MPLL0          93
>>  #define CLKID_MPLL1          94
>>  #define CLKID_MPLL2          95
>> +#define CLKID_NAND_SEL               96
>> +#define CLKID_NAND_DIV               97
>> +#define CLKID_NAND_CLK               98
>>
>> -#define CLK_NR_CLKS          96
>> +#define CLK_NR_CLKS          99
>>
>>  /* include the CLKIDs that have been made part of the stable DT binding */
>>  #include <dt-bindings/clock/meson8b-clkc.h>

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2017-04-02 18:54 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-01 13:10 [PATCH 0/1] add support for the NAND clocks on Meson8b Martin Blumenstingl
2017-04-01 13:10 ` [PATCH 1/1] clk: meson: meson8b: add support for the NAND clocks Martin Blumenstingl
2017-04-02 15:38   ` Jerome Brunet
2017-04-02 18:54     ` Martin Blumenstingl

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).