Re: [PATCH 2/2] clk: meson: a1: add support for Amlogic A1 clock driver

2019-10-17 Thread Jian Hu

Hi, jerome

On 2019/10/14 22:55, Jerome Brunet wrote:


On Mon 14 Oct 2019 at 15:42, Jian Hu  wrote:


if peripheral clocks probe first, it will fail to get
fixed_pll clocks. A lot of peripheral clocks parent are fclk_div2/3/5/7.
and we can not get fclk_div2/3/5/7 clocks.


What does "fail" mean ?
I intended to get clock using devm_clk_get API in each driver, In this

scene,it will get failed because of the clock not being reigstered. In
fact, we could not use devm_clk_get.


Unless I missed somthing, I don't see why you would need to call
devm_clk_get(). This is now handled directly by the framework.


Just my wrong idea,I had not noticed the CCF would do it.


I can think of two solutions:
1) Do not describe xtal_fixpll, xtal_hifipll.
 that is to say, do not decribe the SYS_OSCIN_CTRL register.

2) Put peripheral and pll clock driver in one driver.


Those are work arounds. Actually fixing the problem is usually
preferable.

   So if rephrase your problem:

   * We have 2 clock controllers (A and B)
   * Clock are passed between the controllers using DT
   * We have a PLL in controller B which is used by clocks in
 controller A.
   * the PLL parent clock is in controller A.


Yeah, it is the scene.

=> So if I understand correctly you are saying that it will "fail"
because there is a circular dependency between controller A and B, right
?

Do you have evidence that your problem comes from this circular
dependency ?


I have realized the peripheral driver and PLL drivers,

PLL driver probes first, Peripheral clock driver is the second.


It should work regarless of the order.



In addition,for A1 SoC, it will not work using meson_clk_pll_ops,

it needs strictly sequence,so maybe another ops is required.hifi pll will
be sent with sys pll and CPU clock driver.


The PCie PLL has a good reason to have a single frequency, only one is needed

That's the case the case of the HIFI PLL which, as explained in previous
mails, needs to provide more that the single frenquency you have described.

If the pll driver needs to extended with new ops that's fine. Please
explain this "strict sequence" you are refering too.
What is part of the initial settings, what needs to be done each time ?

The inital settings is the PLL internal analog modules Power-on 
sequence, and it is provided by the VLSI colleague.


For A1 PLL, the pll lock monitor block will initialise again in each 
time. It is the internal principle.It should keep the strict sequence 
when set one target frequency.



AFAIK, CCF will orphan the clock and continue if the parent is not
available. Later, when the parent comes up, the orphan will be
reparented.

IOW, the problem you are reporting should already be covered by CCF.



And  which sulution is better above two?


Neither, I'm afraid



Or maybe other good ideas for it?


My bet would be that an important clocks (maybe more than 1) is being
gated during the init process.

Maybe you should try the command line parameter "clk_ignore_unused"
until you get things running with your 2 controllers.



On 2019/9/29 17:38, Jian Hu wrote:



On 2019/9/27 21:32, Jerome Brunet wrote:


On Fri 27 Sep 2019 at 11:52, Jian Hu  wrote:


Hi, Jerome

Thank you for review.

On 2019/9/25 23:09, Jerome Brunet wrote:

On Wed 25 Sep 2019 at 19:44, Jian Hu  wrote:


The Amlogic A1 clock includes three parts:
peripheral clocks, pll clocks, CPU clocks.
sys pll and CPU clocks will be sent in next patch.

Unlike the previous series, there is no EE/AO domain
in A1 CLK controllers.

Signed-off-by: Jian Hu 
Signed-off-by: Jianxin Pan 
---
 arch/arm64/Kconfig.platforms |1 +
 drivers/clk/meson/Kconfig|   10 +
 drivers/clk/meson/Makefile   |1 +
 drivers/clk/meson/a1.c   | 2617
++
 drivers/clk/meson/a1.h   |  172 +++
 5 files changed, 2801 insertions(+)
 create mode 100644 drivers/clk/meson/a1.c
 create mode 100644 drivers/clk/meson/a1.h


[...]

diff --git a/drivers/clk/meson/a1.c b/drivers/clk/meson/a1.c
new file mode 100644
index 000..26edae0f
--- /dev/null
+++ b/drivers/clk/meson/a1.c
@@ -0,0 +1,2617 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright (c) 2019 Amlogic, Inc. All rights reserved.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "clk-mpll.h"
+#include "clk-pll.h"
+#include "clk-regmap.h"
+#include "vid-pll-div.h"
+#include "clk-dualdiv.h"
+#include "meson-eeclk.h"
+#include "a1.h"
+
+/* PLLs clock in gates, its parent is xtal */
+static struct clk_regmap a1_xtal_clktree = {
+.data = &(struct clk_regmap_gate_data){
+.offset = SYS_OSCIN_CTRL,
+.bit_idx = 0,
+},
+.hw.init = &(struct clk_init_data) {
+.name = "xtal_clktree",
+.ops = _regmap_gate_ops,
+.parent_data = &(const struct clk_parent_data) {
+.fw_name = "xtal",
+},
+.num_parents = 1,
+.flags = CLK_IS_CRITICAL,


Is CCF even expected to 

Re: [PATCH 2/2] clk: meson: a1: add support for Amlogic A1 clock driver

2019-10-14 Thread Jerome Brunet


On Mon 14 Oct 2019 at 15:42, Jian Hu  wrote:

>>> if peripheral clocks probe first, it will fail to get
>>> fixed_pll clocks. A lot of peripheral clocks parent are fclk_div2/3/5/7.
>>> and we can not get fclk_div2/3/5/7 clocks.
>>
>> What does "fail" mean ?
>> I intended to get clock using devm_clk_get API in each driver, In this 
> scene,it will get failed because of the clock not being reigstered. In
> fact, we could not use devm_clk_get.

Unless I missed somthing, I don't see why you would need to call
devm_clk_get(). This is now handled directly by the framework.

>>>
>>> I can think of two solutions:
>>> 1) Do not describe xtal_fixpll, xtal_hifipll.
>>> that is to say, do not decribe the SYS_OSCIN_CTRL register.
>>>
>>> 2) Put peripheral and pll clock driver in one driver.
>>
>> Those are work arounds. Actually fixing the problem is usually
>> preferable.
>>
>>   So if rephrase your problem:
>>
>>   * We have 2 clock controllers (A and B)
>>   * Clock are passed between the controllers using DT
>>   * We have a PLL in controller B which is used by clocks in
>> controller A.
>>   * the PLL parent clock is in controller A.
>>
> Yeah, it is the scene.
>> => So if I understand correctly you are saying that it will "fail"
>> because there is a circular dependency between controller A and B, right
>> ?
>>
>> Do you have evidence that your problem comes from this circular
>> dependency ?
>>
> I have realized the peripheral driver and PLL drivers,
>
> PLL driver probes first, Peripheral clock driver is the second.

It should work regarless of the order.

>
> In addition,for A1 SoC, it will not work using meson_clk_pll_ops,
>
> it needs strictly sequence,so maybe another ops is required.hifi pll will
> be sent with sys pll and CPU clock driver.

The PCie PLL has a good reason to have a single frequency, only one is needed

That's the case the case of the HIFI PLL which, as explained in previous
mails, needs to provide more that the single frenquency you have described.

If the pll driver needs to extended with new ops that's fine. Please
explain this "strict sequence" you are refering too.
What is part of the initial settings, what needs to be done each time ?

>> AFAIK, CCF will orphan the clock and continue if the parent is not
>> available. Later, when the parent comes up, the orphan will be
>> reparented.
>>
>> IOW, the problem you are reporting should already be covered by CCF.
>>
>>>
>>> And  which sulution is better above two?
>>
>> Neither, I'm afraid
>>
>>>
>>> Or maybe other good ideas for it?
>>
>> My bet would be that an important clocks (maybe more than 1) is being
>> gated during the init process.
>>
>> Maybe you should try the command line parameter "clk_ignore_unused"
>> until you get things running with your 2 controllers.
>>
>>>
>>> On 2019/9/29 17:38, Jian Hu wrote:


 On 2019/9/27 21:32, Jerome Brunet wrote:
>
> On Fri 27 Sep 2019 at 11:52, Jian Hu  wrote:
>
>> Hi, Jerome
>>
>> Thank you for review.
>>
>> On 2019/9/25 23:09, Jerome Brunet wrote:
>>> On Wed 25 Sep 2019 at 19:44, Jian Hu  wrote:
>>>
 The Amlogic A1 clock includes three parts:
 peripheral clocks, pll clocks, CPU clocks.
 sys pll and CPU clocks will be sent in next patch.

 Unlike the previous series, there is no EE/AO domain
 in A1 CLK controllers.

 Signed-off-by: Jian Hu 
 Signed-off-by: Jianxin Pan 
 ---
 arch/arm64/Kconfig.platforms |1 +
 drivers/clk/meson/Kconfig|   10 +
 drivers/clk/meson/Makefile   |1 +
 drivers/clk/meson/a1.c   | 2617
 ++
 drivers/clk/meson/a1.h   |  172 +++
 5 files changed, 2801 insertions(+)
 create mode 100644 drivers/clk/meson/a1.c
 create mode 100644 drivers/clk/meson/a1.h

>>> [...]
 diff --git a/drivers/clk/meson/a1.c b/drivers/clk/meson/a1.c
 new file mode 100644
 index 000..26edae0f
 --- /dev/null
 +++ b/drivers/clk/meson/a1.c
 @@ -0,0 +1,2617 @@
 +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
 +/*
 + * Copyright (c) 2019 Amlogic, Inc. All rights reserved.
 + */
 +
 +#include 
 +#include 
 +#include 
 +#include 
 +#include 
 +#include "clk-mpll.h"
 +#include "clk-pll.h"
 +#include "clk-regmap.h"
 +#include "vid-pll-div.h"
 +#include "clk-dualdiv.h"
 +#include "meson-eeclk.h"
 +#include "a1.h"
 +
 +/* PLLs clock in gates, its parent is xtal */
 +static struct clk_regmap a1_xtal_clktree = {
 +.data = &(struct clk_regmap_gate_data){
 +.offset = SYS_OSCIN_CTRL,
 +.bit_idx = 0,
 +},
 +.hw.init = &(struct 

Re: [PATCH 2/2] clk: meson: a1: add support for Amlogic A1 clock driver

2019-10-14 Thread Jian Hu

Hi, Jerome

sorry, it's my problem.

I did not discribe clearly the issue.

On 2019/10/11 15:39, Jerome Brunet wrote:


On Tue 08 Oct 2019 at 10:03, Jian Hu  wrote:


Hi, Jerome

PLL clocks and peripheral clocks rely on each other.

for fixed_pll, we can describe its parent like this:

xtal-->xtal_fixpll-->fixed_dco-->fixed_pll

xtal fixpll is belong to peripheral region.
fixed_pll/fclk_div2/fclk_div3 is belong to PLL region.

if PLL clocks probe first, it will fail to get xtal_fixpll.
we can not get fixed_dco's parent clock.

if peripheral clocks probe first, it will fail to get
fixed_pll clocks. A lot of peripheral clocks parent are fclk_div2/3/5/7.
and we can not get fclk_div2/3/5/7 clocks.


What does "fail" mean ?
I intended to get clock using devm_clk_get API in each driver, In this 
scene,it will get failed because of the clock not being reigstered. In 
fact, we could not use devm_clk_get.


I can think of two solutions:
1) Do not describe xtal_fixpll, xtal_hifipll.
that is to say, do not decribe the SYS_OSCIN_CTRL register.

2) Put peripheral and pll clock driver in one driver.


Those are work arounds. Actually fixing the problem is usually
preferable.

  So if rephrase your problem:

  * We have 2 clock controllers (A and B)
  * Clock are passed between the controllers using DT
  * We have a PLL in controller B which is used by clocks in
controller A.
  * the PLL parent clock is in controller A.


Yeah, it is the scene.

=> So if I understand correctly you are saying that it will "fail"
because there is a circular dependency between controller A and B, right
?

Do you have evidence that your problem comes from this circular
dependency ?


I have realized the peripheral driver and PLL drivers,

PLL driver probes first, Peripheral clock driver is the second.

In addition,for A1 SoC, it will not work using meson_clk_pll_ops,

it needs strictly sequence,so maybe another ops is required.hifi pll 
will be sent with sys pll and CPU clock driver.

AFAIK, CCF will orphan the clock and continue if the parent is not
available. Later, when the parent comes up, the orphan will be
reparented.

IOW, the problem you are reporting should already be covered by CCF.



And  which sulution is better above two?


Neither, I'm afraid



Or maybe other good ideas for it?


My bet would be that an important clocks (maybe more than 1) is being
gated during the init process.

Maybe you should try the command line parameter "clk_ignore_unused"
until you get things running with your 2 controllers.



On 2019/9/29 17:38, Jian Hu wrote:



On 2019/9/27 21:32, Jerome Brunet wrote:


On Fri 27 Sep 2019 at 11:52, Jian Hu  wrote:


Hi, Jerome

Thank you for review.

On 2019/9/25 23:09, Jerome Brunet wrote:

On Wed 25 Sep 2019 at 19:44, Jian Hu  wrote:


The Amlogic A1 clock includes three parts:
peripheral clocks, pll clocks, CPU clocks.
sys pll and CPU clocks will be sent in next patch.

Unlike the previous series, there is no EE/AO domain
in A1 CLK controllers.

Signed-off-by: Jian Hu 
Signed-off-by: Jianxin Pan 
---
arch/arm64/Kconfig.platforms |1 +
drivers/clk/meson/Kconfig|   10 +
drivers/clk/meson/Makefile   |1 +
drivers/clk/meson/a1.c   | 2617
++
drivers/clk/meson/a1.h   |  172 +++
5 files changed, 2801 insertions(+)
create mode 100644 drivers/clk/meson/a1.c
create mode 100644 drivers/clk/meson/a1.h


[...]

diff --git a/drivers/clk/meson/a1.c b/drivers/clk/meson/a1.c
new file mode 100644
index 000..26edae0f
--- /dev/null
+++ b/drivers/clk/meson/a1.c
@@ -0,0 +1,2617 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright (c) 2019 Amlogic, Inc. All rights reserved.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "clk-mpll.h"
+#include "clk-pll.h"
+#include "clk-regmap.h"
+#include "vid-pll-div.h"
+#include "clk-dualdiv.h"
+#include "meson-eeclk.h"
+#include "a1.h"
+
+/* PLLs clock in gates, its parent is xtal */
+static struct clk_regmap a1_xtal_clktree = {
+.data = &(struct clk_regmap_gate_data){
+.offset = SYS_OSCIN_CTRL,
+.bit_idx = 0,
+},
+.hw.init = &(struct clk_init_data) {
+.name = "xtal_clktree",
+.ops = _regmap_gate_ops,
+.parent_data = &(const struct clk_parent_data) {
+.fw_name = "xtal",
+},
+.num_parents = 1,
+.flags = CLK_IS_CRITICAL,


Is CCF even expected to touch this ever ? what about RO ops ?
Please review your other clocks with this in mind


the clock should not be changed at runtime.clk_regmap_gate_ro_ops
is a good idea. Set RO ops and remove the CLK_IS_CRITICAL flag.

+},
+};
+
+static struct clk_regmap a1_xtal_fixpll = {
+.data = &(struct clk_regmap_gate_data){
+.offset = SYS_OSCIN_CTRL,
+.bit_idx = 1,
+},
+.hw.init = &(struct clk_init_data) {
+.name = "xtal_fixpll",
+.ops = _regmap_gate_ops,
+.parent_data = &(const 

Re: [PATCH 2/2] clk: meson: a1: add support for Amlogic A1 clock driver

2019-10-11 Thread Jerome Brunet


On Tue 08 Oct 2019 at 10:03, Jian Hu  wrote:

> Hi, Jerome
>
> PLL clocks and peripheral clocks rely on each other.
>
> for fixed_pll, we can describe its parent like this:
>
> xtal-->xtal_fixpll-->fixed_dco-->fixed_pll
>
> xtal fixpll is belong to peripheral region.
> fixed_pll/fclk_div2/fclk_div3 is belong to PLL region.
>
> if PLL clocks probe first, it will fail to get xtal_fixpll.
> we can not get fixed_dco's parent clock.
>
> if peripheral clocks probe first, it will fail to get
> fixed_pll clocks. A lot of peripheral clocks parent are fclk_div2/3/5/7.
> and we can not get fclk_div2/3/5/7 clocks.

What does "fail" mean ?

>
> I can think of two solutions:
> 1) Do not describe xtal_fixpll, xtal_hifipll.
>that is to say, do not decribe the SYS_OSCIN_CTRL register.
>
> 2) Put peripheral and pll clock driver in one driver.

Those are work arounds. Actually fixing the problem is usually
preferable.

 So if rephrase your problem:

 * We have 2 clock controllers (A and B)
 * Clock are passed between the controllers using DT
 * We have a PLL in controller B which is used by clocks in
   controller A.
 * the PLL parent clock is in controller A.

=> So if I understand correctly you are saying that it will "fail"
because there is a circular dependency between controller A and B, right
?

Do you have evidence that your problem comes from this circular
dependency ?

AFAIK, CCF will orphan the clock and continue if the parent is not
available. Later, when the parent comes up, the orphan will be
reparented.

IOW, the problem you are reporting should already be covered by CCF.

>
> And  which sulution is better above two?

Neither, I'm afraid

>
> Or maybe other good ideas for it?

My bet would be that an important clocks (maybe more than 1) is being
gated during the init process.

Maybe you should try the command line parameter "clk_ignore_unused"
until you get things running with your 2 controllers.

>
> On 2019/9/29 17:38, Jian Hu wrote:
>>
>>
>> On 2019/9/27 21:32, Jerome Brunet wrote:
>>>
>>> On Fri 27 Sep 2019 at 11:52, Jian Hu  wrote:
>>>
 Hi, Jerome

 Thank you for review.

 On 2019/9/25 23:09, Jerome Brunet wrote:
> On Wed 25 Sep 2019 at 19:44, Jian Hu  wrote:
>
>> The Amlogic A1 clock includes three parts:
>> peripheral clocks, pll clocks, CPU clocks.
>> sys pll and CPU clocks will be sent in next patch.
>>
>> Unlike the previous series, there is no EE/AO domain
>> in A1 CLK controllers.
>>
>> Signed-off-by: Jian Hu 
>> Signed-off-by: Jianxin Pan 
>> ---
>>arch/arm64/Kconfig.platforms |1 +
>>drivers/clk/meson/Kconfig|   10 +
>>drivers/clk/meson/Makefile   |1 +
>>drivers/clk/meson/a1.c   | 2617
>> ++
>>drivers/clk/meson/a1.h   |  172 +++
>>5 files changed, 2801 insertions(+)
>>create mode 100644 drivers/clk/meson/a1.c
>>create mode 100644 drivers/clk/meson/a1.h
>>
> [...]
>> diff --git a/drivers/clk/meson/a1.c b/drivers/clk/meson/a1.c
>> new file mode 100644
>> index 000..26edae0f
>> --- /dev/null
>> +++ b/drivers/clk/meson/a1.c
>> @@ -0,0 +1,2617 @@
>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>> +/*
>> + * Copyright (c) 2019 Amlogic, Inc. All rights reserved.
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include "clk-mpll.h"
>> +#include "clk-pll.h"
>> +#include "clk-regmap.h"
>> +#include "vid-pll-div.h"
>> +#include "clk-dualdiv.h"
>> +#include "meson-eeclk.h"
>> +#include "a1.h"
>> +
>> +/* PLLs clock in gates, its parent is xtal */
>> +static struct clk_regmap a1_xtal_clktree = {
>> +.data = &(struct clk_regmap_gate_data){
>> +.offset = SYS_OSCIN_CTRL,
>> +.bit_idx = 0,
>> +},
>> +.hw.init = &(struct clk_init_data) {
>> +.name = "xtal_clktree",
>> +.ops = _regmap_gate_ops,
>> +.parent_data = &(const struct clk_parent_data) {
>> +.fw_name = "xtal",
>> +},
>> +.num_parents = 1,
>> +.flags = CLK_IS_CRITICAL,
>
> Is CCF even expected to touch this ever ? what about RO ops ?
> Please review your other clocks with this in mind
>
 the clock should not be changed at runtime.clk_regmap_gate_ro_ops
 is a good idea. Set RO ops and remove the CLK_IS_CRITICAL flag.
>> +},
>> +};
>> +
>> +static struct clk_regmap a1_xtal_fixpll = {
>> +.data = &(struct clk_regmap_gate_data){
>> +.offset = SYS_OSCIN_CTRL,
>> +.bit_idx = 1,
>> +},
>> +.hw.init = &(struct clk_init_data) {
>> +.name = "xtal_fixpll",
>> +.ops = _regmap_gate_ops,
>> +.parent_data = &(const struct clk_parent_data) {
>> +

Re: [PATCH 2/2] clk: meson: a1: add support for Amlogic A1 clock driver

2019-10-08 Thread Jian Hu

Hi, Jerome

PLL clocks and peripheral clocks rely on each other.

for fixed_pll, we can describe its parent like this:

xtal-->xtal_fixpll-->fixed_dco-->fixed_pll

xtal fixpll is belong to peripheral region.
fixed_pll/fclk_div2/fclk_div3 is belong to PLL region.

if PLL clocks probe first, it will fail to get xtal_fixpll.
we can not get fixed_dco's parent clock.

if peripheral clocks probe first, it will fail to get
fixed_pll clocks. A lot of peripheral clocks parent are fclk_div2/3/5/7.
and we can not get fclk_div2/3/5/7 clocks.

I can think of two solutions:
1) Do not describe xtal_fixpll, xtal_hifipll.
   that is to say, do not decribe the SYS_OSCIN_CTRL register.

2) Put peripheral and pll clock driver in one driver.

And  which sulution is better above two?

Or maybe other good ideas for it?

On 2019/9/29 17:38, Jian Hu wrote:



On 2019/9/27 21:32, Jerome Brunet wrote:


On Fri 27 Sep 2019 at 11:52, Jian Hu  wrote:


Hi, Jerome

Thank you for review.

On 2019/9/25 23:09, Jerome Brunet wrote:

On Wed 25 Sep 2019 at 19:44, Jian Hu  wrote:


The Amlogic A1 clock includes three parts:
peripheral clocks, pll clocks, CPU clocks.
sys pll and CPU clocks will be sent in next patch.

Unlike the previous series, there is no EE/AO domain
in A1 CLK controllers.

Signed-off-by: Jian Hu 
Signed-off-by: Jianxin Pan 
---
   arch/arm64/Kconfig.platforms |    1 +
   drivers/clk/meson/Kconfig    |   10 +
   drivers/clk/meson/Makefile   |    1 +
   drivers/clk/meson/a1.c   | 2617 
++

   drivers/clk/meson/a1.h   |  172 +++
   5 files changed, 2801 insertions(+)
   create mode 100644 drivers/clk/meson/a1.c
   create mode 100644 drivers/clk/meson/a1.h


[...]

diff --git a/drivers/clk/meson/a1.c b/drivers/clk/meson/a1.c
new file mode 100644
index 000..26edae0f
--- /dev/null
+++ b/drivers/clk/meson/a1.c
@@ -0,0 +1,2617 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright (c) 2019 Amlogic, Inc. All rights reserved.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "clk-mpll.h"
+#include "clk-pll.h"
+#include "clk-regmap.h"
+#include "vid-pll-div.h"
+#include "clk-dualdiv.h"
+#include "meson-eeclk.h"
+#include "a1.h"
+
+/* PLLs clock in gates, its parent is xtal */
+static struct clk_regmap a1_xtal_clktree = {
+    .data = &(struct clk_regmap_gate_data){
+    .offset = SYS_OSCIN_CTRL,
+    .bit_idx = 0,
+    },
+    .hw.init = &(struct clk_init_data) {
+    .name = "xtal_clktree",
+    .ops = _regmap_gate_ops,
+    .parent_data = &(const struct clk_parent_data) {
+    .fw_name = "xtal",
+    },
+    .num_parents = 1,
+    .flags = CLK_IS_CRITICAL,


Is CCF even expected to touch this ever ? what about RO ops ?
Please review your other clocks with this in mind


the clock should not be changed at runtime.clk_regmap_gate_ro_ops
is a good idea. Set RO ops and remove the CLK_IS_CRITICAL flag.

+    },
+};
+
+static struct clk_regmap a1_xtal_fixpll = {
+    .data = &(struct clk_regmap_gate_data){
+    .offset = SYS_OSCIN_CTRL,
+    .bit_idx = 1,
+    },
+    .hw.init = &(struct clk_init_data) {
+    .name = "xtal_fixpll",
+    .ops = _regmap_gate_ops,
+    .parent_data = &(const struct clk_parent_data) {
+    .fw_name = "xtal",
+    },
+    .num_parents = 1,
+    .flags = CLK_IS_CRITICAL,
+    },
+};
+
+static struct clk_regmap a1_xtal_usb_phy = {
+    .data = &(struct clk_regmap_gate_data){
+    .offset = SYS_OSCIN_CTRL,
+    .bit_idx = 2,
+    },
+    .hw.init = &(struct clk_init_data) {
+    .name = "xtal_usb_phy",
+    .ops = _regmap_gate_ops,
+    .parent_data = &(const struct clk_parent_data) {
+    .fw_name = "xtal",
+    },
+    .num_parents = 1,
+    .flags = CLK_IS_CRITICAL,


How is an USB clock critical to the system ?
Please review your other clocks with comment in mind ...

the usb clock does not affect the system,
remove the CLK_IS_CRITICAL flag



+    },
+};
+
+static struct clk_regmap a1_xtal_usb_ctrl = {
+    .data = &(struct clk_regmap_gate_data){
+    .offset = SYS_OSCIN_CTRL,
+    .bit_idx = 3,
+    },
+    .hw.init = &(struct clk_init_data) {
+    .name = "xtal_usb_ctrl",
+    .ops = _regmap_gate_ops,
+    .parent_data = &(const struct clk_parent_data) {
+    .fw_name = "xtal",
+    },
+    .num_parents = 1,
+    .flags = CLK_IS_CRITICAL,
+    },
+};

the usb clock does not affect the system,
remove the CLK_IS_CRITICAL flag

+
+static struct clk_regmap a1_xtal_hifipll = {
+    .data = &(struct clk_regmap_gate_data){
+    .offset = SYS_OSCIN_CTRL,
+    .bit_idx = 4,
+    },
+    .hw.init = &(struct clk_init_data) {
+    .name = "xtal_hifipll",
+    .ops = _regmap_gate_ops,
+    .parent_data = &(const struct clk_parent_data) {
+    .fw_name = "xtal",
+    },
+    .num_parents = 1,
+    .flags = CLK_IS_CRITICAL,
+    },
+};


Re: [PATCH 2/2] clk: meson: a1: add support for Amlogic A1 clock driver

2019-09-29 Thread Jian Hu




On 2019/9/27 21:32, Jerome Brunet wrote:


On Fri 27 Sep 2019 at 11:52, Jian Hu  wrote:


Hi, Jerome

Thank you for review.

On 2019/9/25 23:09, Jerome Brunet wrote:

On Wed 25 Sep 2019 at 19:44, Jian Hu  wrote:


The Amlogic A1 clock includes three parts:
peripheral clocks, pll clocks, CPU clocks.
sys pll and CPU clocks will be sent in next patch.

Unlike the previous series, there is no EE/AO domain
in A1 CLK controllers.

Signed-off-by: Jian Hu 
Signed-off-by: Jianxin Pan 
---
   arch/arm64/Kconfig.platforms |1 +
   drivers/clk/meson/Kconfig|   10 +
   drivers/clk/meson/Makefile   |1 +
   drivers/clk/meson/a1.c   | 2617 
++
   drivers/clk/meson/a1.h   |  172 +++
   5 files changed, 2801 insertions(+)
   create mode 100644 drivers/clk/meson/a1.c
   create mode 100644 drivers/clk/meson/a1.h

diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
index 16d7614..a48f67d 100644
--- a/arch/arm64/Kconfig.platforms
+++ b/arch/arm64/Kconfig.platforms
@@ -138,6 +138,7 @@ config ARCH_MESON
select COMMON_CLK_AXG
select COMMON_CLK_G12A
select MESON_IRQ_GPIO
+   select COMMON_CLK_A1


This need to be separate patch addressed to Kevin once your driver is merged


ok, I will remove it in PATCH V2. And send it again after the driver is
merged.

help
  This enables support for the arm64 based Amlogic SoCs
  such as the s905, S905X/D, S912, A113X/D or S905X/D2
diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
index dabeb43..e6cb4c3 100644
--- a/drivers/clk/meson/Kconfig
+++ b/drivers/clk/meson/Kconfig
@@ -107,3 +107,13 @@ config COMMON_CLK_G12A
help
  Support for the clock controller on Amlogic S905D2, S905X2 and S905Y2
  devices, aka g12a. Say Y if you want peripherals to work.
+
+config COMMON_CLK_A1
+   bool
+   depends on ARCH_MESON
+   select COMMON_CLK_MESON_REGMAP
+   select COMMON_CLK_MESON_DUALDIV
+   select COMMON_CLK_MESON_PLL
+   help
+ Support for the clock controller on Amlogic A113L device,
+ aka a1. Say Y if you want peripherals to work.
diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
index 3939f21..6be3a8f 100644
--- a/drivers/clk/meson/Makefile
+++ b/drivers/clk/meson/Makefile
@@ -19,3 +19,4 @@ obj-$(CONFIG_COMMON_CLK_AXG_AUDIO) += axg-audio.o
   obj-$(CONFIG_COMMON_CLK_GXBB) += gxbb.o gxbb-aoclk.o
   obj-$(CONFIG_COMMON_CLK_G12A) += g12a.o g12a-aoclk.o
   obj-$(CONFIG_COMMON_CLK_MESON8B) += meson8b.o
+obj-$(CONFIG_COMMON_CLK_A1) += a1.o
diff --git a/drivers/clk/meson/a1.c b/drivers/clk/meson/a1.c
new file mode 100644
index 000..26edae0f
--- /dev/null
+++ b/drivers/clk/meson/a1.c
@@ -0,0 +1,2617 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright (c) 2019 Amlogic, Inc. All rights reserved.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "clk-mpll.h"
+#include "clk-pll.h"
+#include "clk-regmap.h"
+#include "vid-pll-div.h"
+#include "clk-dualdiv.h"
+#include "meson-eeclk.h"
+#include "a1.h"
+
+/* PLLs clock in gates, its parent is xtal */
+static struct clk_regmap a1_xtal_clktree = {
+   .data = &(struct clk_regmap_gate_data){
+   .offset = SYS_OSCIN_CTRL,
+   .bit_idx = 0,
+   },
+   .hw.init = &(struct clk_init_data) {
+   .name = "xtal_clktree",
+   .ops = _regmap_gate_ops,
+   .parent_data = &(const struct clk_parent_data) {
+   .fw_name = "xtal",
+   },
+   .num_parents = 1,
+   .flags = CLK_IS_CRITICAL,


Is CCF even expected to touch this ever ? what about RO ops ?
Please review your other clocks with this in mind


the clock should not be changed at runtime.clk_regmap_gate_ro_ops
is a good idea. Set RO ops and remove the CLK_IS_CRITICAL flag.

+   },
+};
+
+static struct clk_regmap a1_xtal_fixpll = {
+   .data = &(struct clk_regmap_gate_data){
+   .offset = SYS_OSCIN_CTRL,
+   .bit_idx = 1,
+   },
+   .hw.init = &(struct clk_init_data) {
+   .name = "xtal_fixpll",
+   .ops = _regmap_gate_ops,
+   .parent_data = &(const struct clk_parent_data) {
+   .fw_name = "xtal",
+   },
+   .num_parents = 1,
+   .flags = CLK_IS_CRITICAL,
+   },
+};
+
+static struct clk_regmap a1_xtal_usb_phy = {
+   .data = &(struct clk_regmap_gate_data){
+   .offset = SYS_OSCIN_CTRL,
+   .bit_idx = 2,
+   },
+   .hw.init = &(struct clk_init_data) {
+   .name = "xtal_usb_phy",
+   .ops = _regmap_gate_ops,
+   .parent_data = &(const struct clk_parent_data) {
+   .fw_name = "xtal",
+   },
+   .num_parents = 1,
+   .flags = CLK_IS_CRITICAL,


How is an USB clock critical 

Re: [PATCH 2/2] clk: meson: a1: add support for Amlogic A1 clock driver

2019-09-29 Thread Jian Hu



On 2019/9/27 20:56, Jerome Brunet wrote:


On Fri 27 Sep 2019 at 05:11, Jian Hu  wrote:


Hi, Stephen

Thank you for review

On 2019/9/25 21:12, Stephen Boyd wrote:

Quoting Jian Hu (2019-09-25 04:44:48)

The Amlogic A1 clock includes three parts:
peripheral clocks, pll clocks, CPU clocks.
sys pll and CPU clocks will be sent in next patch.

Unlike the previous series, there is no EE/AO domain
in A1 CLK controllers.

Signed-off-by: Jian Hu 
Signed-off-by: Jianxin Pan 


This second name didn't send the patch. Please follow the signoff
procedures documented in Documentation/process/submitting-patches.rst


diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
index 16d7614..a48f67d 100644
--- a/arch/arm64/Kconfig.platforms
+++ b/arch/arm64/Kconfig.platforms
@@ -138,6 +138,7 @@ config ARCH_MESON
  select COMMON_CLK_AXG
  select COMMON_CLK_G12A
  select MESON_IRQ_GPIO
+   select COMMON_CLK_A1


Sort?

ok, I will put it behind COMMON_CLK_AXG



  help
This enables support for the arm64 based Amlogic SoCs
such as the s905, S905X/D, S912, A113X/D or S905X/D2
diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
index dabeb43..e6cb4c3 100644
--- a/drivers/clk/meson/Kconfig
+++ b/drivers/clk/meson/Kconfig
@@ -107,3 +107,13 @@ config COMMON_CLK_G12A
  help
Support for the clock controller on Amlogic S905D2, S905X2 and 
S905Y2
devices, aka g12a. Say Y if you want peripherals to work.
+
+config COMMON_CLK_A1


Probably should be placed somewhere alphabetically in this file?

ok, I will put it behind COMMON_CLK_AXG_AUDIO



+   bool
+   depends on ARCH_MESON
+   select COMMON_CLK_MESON_REGMAP
+   select COMMON_CLK_MESON_DUALDIV
+   select COMMON_CLK_MESON_PLL
+   help
+ Support for the clock controller on Amlogic A113L device,
+ aka a1. Say Y if you want peripherals to work.
diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
index 3939f21..6be3a8f 100644
--- a/drivers/clk/meson/Makefile
+++ b/drivers/clk/meson/Makefile
@@ -19,3 +19,4 @@ obj-$(CONFIG_COMMON_CLK_AXG_AUDIO) += axg-audio.o
   obj-$(CONFIG_COMMON_CLK_GXBB) += gxbb.o gxbb-aoclk.o
   obj-$(CONFIG_COMMON_CLK_G12A) += g12a.o g12a-aoclk.o
   obj-$(CONFIG_COMMON_CLK_MESON8B) += meson8b.o
+obj-$(CONFIG_COMMON_CLK_A1) += a1.o


I would guess this should be sorted on Kconfig name in this file?

ok, I will put it behind COMMON_CLK_AXG_AUDIO



diff --git a/drivers/clk/meson/a1.c b/drivers/clk/meson/a1.c
new file mode 100644
index 000..26edae0f
--- /dev/null
+++ b/drivers/clk/meson/a1.c
@@ -0,0 +1,2617 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright (c) 2019 Amlogic, Inc. All rights reserved.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "clk-mpll.h"
+#include "clk-pll.h"
+#include "clk-regmap.h"
+#include "vid-pll-div.h"
+#include "clk-dualdiv.h"
+#include "meson-eeclk.h"
+#include "a1.h"
+

[...]

+
+/*
+ * The Meson A1 HIFI PLL is 614.4M, it requires
+ * a strict register sequence to enable the PLL.
+ * set meson_clk_pcie_pll_ops as its ops


Please remove this last line as it's obvious from the code what ops are
used.


ok, I will remove it.

+ */
+static struct clk_regmap a1_hifi_pll = {
+   .data = &(struct meson_clk_pll_data){
+   .en = {
+   .reg_off = ANACTRL_HIFIPLL_CTRL0,
+   .shift   = 28,
+   .width   = 1,
+   },
+   .m = {
+   .reg_off = ANACTRL_HIFIPLL_CTRL0,
+   .shift   = 0,
+   .width   = 8,
+   },
+   .n = {
+   .reg_off = ANACTRL_HIFIPLL_CTRL0,
+   .shift   = 10,
+   .width   = 5,
+   },
+   .frac = {
+   .reg_off = ANACTRL_HIFIPLL_CTRL1,
+   .shift   = 0,
+   .width   = 19,
+   },
+   .l = {
+   .reg_off = ANACTRL_HIFIPLL_STS,
+   .shift   = 31,
+   .width   = 1,
+   },
+   .table = a1_hifi_pll_params_table,
+   .init_regs = a1_hifi_init_regs,
+   .init_count = ARRAY_SIZE(a1_hifi_init_regs),
+   },
+   .hw.init = &(struct clk_init_data){
+   .name = "hifi_pll",
+   .ops = _clk_pcie_pll_ops,
+   .parent_hws = (const struct clk_hw *[]) {
+   _xtal_hifipll.hw
+   },
+   .num_parents = 1,
+   },
+};
+

[..]

+
+static struct clk_regmap a1_fclk_div2 = {
+   .data = &(struct clk_regmap_gate_data){
+   .offset = ANACTRL_FIXPLL_CTRL0,
+   .bit_idx = 21,
+   },
+   .hw.init = &(struct clk_init_data){
+   .name = "fclk_div2",
+   

Re: [PATCH 2/2] clk: meson: a1: add support for Amlogic A1 clock driver

2019-09-27 Thread Jerome Brunet


On Fri 27 Sep 2019 at 11:52, Jian Hu  wrote:

> Hi, Jerome
>
> Thank you for review.
>
> On 2019/9/25 23:09, Jerome Brunet wrote:
>> On Wed 25 Sep 2019 at 19:44, Jian Hu  wrote:
>>
>>> The Amlogic A1 clock includes three parts:
>>> peripheral clocks, pll clocks, CPU clocks.
>>> sys pll and CPU clocks will be sent in next patch.
>>>
>>> Unlike the previous series, there is no EE/AO domain
>>> in A1 CLK controllers.
>>>
>>> Signed-off-by: Jian Hu 
>>> Signed-off-by: Jianxin Pan 
>>> ---
>>>   arch/arm64/Kconfig.platforms |1 +
>>>   drivers/clk/meson/Kconfig|   10 +
>>>   drivers/clk/meson/Makefile   |1 +
>>>   drivers/clk/meson/a1.c   | 2617 
>>> ++
>>>   drivers/clk/meson/a1.h   |  172 +++
>>>   5 files changed, 2801 insertions(+)
>>>   create mode 100644 drivers/clk/meson/a1.c
>>>   create mode 100644 drivers/clk/meson/a1.h
>>>
>>> diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
>>> index 16d7614..a48f67d 100644
>>> --- a/arch/arm64/Kconfig.platforms
>>> +++ b/arch/arm64/Kconfig.platforms
>>> @@ -138,6 +138,7 @@ config ARCH_MESON
>>> select COMMON_CLK_AXG
>>> select COMMON_CLK_G12A
>>> select MESON_IRQ_GPIO
>>> +   select COMMON_CLK_A1
>>
>> This need to be separate patch addressed to Kevin once your driver is merged
>>
> ok, I will remove it in PATCH V2. And send it again after the driver is
> merged.
>>> help
>>>   This enables support for the arm64 based Amlogic SoCs
>>>   such as the s905, S905X/D, S912, A113X/D or S905X/D2
>>> diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
>>> index dabeb43..e6cb4c3 100644
>>> --- a/drivers/clk/meson/Kconfig
>>> +++ b/drivers/clk/meson/Kconfig
>>> @@ -107,3 +107,13 @@ config COMMON_CLK_G12A
>>> help
>>>   Support for the clock controller on Amlogic S905D2, S905X2 and S905Y2
>>>   devices, aka g12a. Say Y if you want peripherals to work.
>>> +
>>> +config COMMON_CLK_A1
>>> +   bool
>>> +   depends on ARCH_MESON
>>> +   select COMMON_CLK_MESON_REGMAP
>>> +   select COMMON_CLK_MESON_DUALDIV
>>> +   select COMMON_CLK_MESON_PLL
>>> +   help
>>> + Support for the clock controller on Amlogic A113L device,
>>> + aka a1. Say Y if you want peripherals to work.
>>> diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
>>> index 3939f21..6be3a8f 100644
>>> --- a/drivers/clk/meson/Makefile
>>> +++ b/drivers/clk/meson/Makefile
>>> @@ -19,3 +19,4 @@ obj-$(CONFIG_COMMON_CLK_AXG_AUDIO) += axg-audio.o
>>>   obj-$(CONFIG_COMMON_CLK_GXBB) += gxbb.o gxbb-aoclk.o
>>>   obj-$(CONFIG_COMMON_CLK_G12A) += g12a.o g12a-aoclk.o
>>>   obj-$(CONFIG_COMMON_CLK_MESON8B) += meson8b.o
>>> +obj-$(CONFIG_COMMON_CLK_A1) += a1.o
>>> diff --git a/drivers/clk/meson/a1.c b/drivers/clk/meson/a1.c
>>> new file mode 100644
>>> index 000..26edae0f
>>> --- /dev/null
>>> +++ b/drivers/clk/meson/a1.c
>>> @@ -0,0 +1,2617 @@
>>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>>> +/*
>>> + * Copyright (c) 2019 Amlogic, Inc. All rights reserved.
>>> + */
>>> +
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include "clk-mpll.h"
>>> +#include "clk-pll.h"
>>> +#include "clk-regmap.h"
>>> +#include "vid-pll-div.h"
>>> +#include "clk-dualdiv.h"
>>> +#include "meson-eeclk.h"
>>> +#include "a1.h"
>>> +
>>> +/* PLLs clock in gates, its parent is xtal */
>>> +static struct clk_regmap a1_xtal_clktree = {
>>> +   .data = &(struct clk_regmap_gate_data){
>>> +   .offset = SYS_OSCIN_CTRL,
>>> +   .bit_idx = 0,
>>> +   },
>>> +   .hw.init = &(struct clk_init_data) {
>>> +   .name = "xtal_clktree",
>>> +   .ops = _regmap_gate_ops,
>>> +   .parent_data = &(const struct clk_parent_data) {
>>> +   .fw_name = "xtal",
>>> +   },
>>> +   .num_parents = 1,
>>> +   .flags = CLK_IS_CRITICAL,
>>
>> Is CCF even expected to touch this ever ? what about RO ops ?
>> Please review your other clocks with this in mind
>>
> the clock should not be changed at runtime.clk_regmap_gate_ro_ops
> is a good idea. Set RO ops and remove the CLK_IS_CRITICAL flag.
>>> +   },
>>> +};
>>> +
>>> +static struct clk_regmap a1_xtal_fixpll = {
>>> +   .data = &(struct clk_regmap_gate_data){
>>> +   .offset = SYS_OSCIN_CTRL,
>>> +   .bit_idx = 1,
>>> +   },
>>> +   .hw.init = &(struct clk_init_data) {
>>> +   .name = "xtal_fixpll",
>>> +   .ops = _regmap_gate_ops,
>>> +   .parent_data = &(const struct clk_parent_data) {
>>> +   .fw_name = "xtal",
>>> +   },
>>> +   .num_parents = 1,
>>> +   .flags = CLK_IS_CRITICAL,
>>> +   },
>>> +};
>>> +
>>> +static struct clk_regmap a1_xtal_usb_phy = {
>>> +   .data = &(struct clk_regmap_gate_data){
>>> +   .offset = SYS_OSCIN_CTRL,
>>> +   .bit_idx = 2,
>>> +   },
>>> +   .hw.init = &(struct clk_init_data) {
>>> +   .name = "xtal_usb_phy",
>>> +

Re: [PATCH 2/2] clk: meson: a1: add support for Amlogic A1 clock driver

2019-09-27 Thread Jerome Brunet


On Fri 27 Sep 2019 at 05:11, Jian Hu  wrote:

> Hi, Stephen
>
> Thank you for review
>
> On 2019/9/25 21:12, Stephen Boyd wrote:
>> Quoting Jian Hu (2019-09-25 04:44:48)
>>> The Amlogic A1 clock includes three parts:
>>> peripheral clocks, pll clocks, CPU clocks.
>>> sys pll and CPU clocks will be sent in next patch.
>>>
>>> Unlike the previous series, there is no EE/AO domain
>>> in A1 CLK controllers.
>>>
>>> Signed-off-by: Jian Hu 
>>> Signed-off-by: Jianxin Pan 
>>
>> This second name didn't send the patch. Please follow the signoff
>> procedures documented in Documentation/process/submitting-patches.rst
>>
>>> diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
>>> index 16d7614..a48f67d 100644
>>> --- a/arch/arm64/Kconfig.platforms
>>> +++ b/arch/arm64/Kconfig.platforms
>>> @@ -138,6 +138,7 @@ config ARCH_MESON
>>>  select COMMON_CLK_AXG
>>>  select COMMON_CLK_G12A
>>>  select MESON_IRQ_GPIO
>>> +   select COMMON_CLK_A1
>>
>> Sort?
> ok, I will put it behind COMMON_CLK_AXG
>>
>>>  help
>>>This enables support for the arm64 based Amlogic SoCs
>>>such as the s905, S905X/D, S912, A113X/D or S905X/D2
>>> diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
>>> index dabeb43..e6cb4c3 100644
>>> --- a/drivers/clk/meson/Kconfig
>>> +++ b/drivers/clk/meson/Kconfig
>>> @@ -107,3 +107,13 @@ config COMMON_CLK_G12A
>>>  help
>>>Support for the clock controller on Amlogic S905D2, S905X2 and 
>>> S905Y2
>>>devices, aka g12a. Say Y if you want peripherals to work.
>>> +
>>> +config COMMON_CLK_A1
>>
>> Probably should be placed somewhere alphabetically in this file?
> ok, I will put it behind COMMON_CLK_AXG_AUDIO
>>
>>> +   bool
>>> +   depends on ARCH_MESON
>>> +   select COMMON_CLK_MESON_REGMAP
>>> +   select COMMON_CLK_MESON_DUALDIV
>>> +   select COMMON_CLK_MESON_PLL
>>> +   help
>>> + Support for the clock controller on Amlogic A113L device,
>>> + aka a1. Say Y if you want peripherals to work.
>>> diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
>>> index 3939f21..6be3a8f 100644
>>> --- a/drivers/clk/meson/Makefile
>>> +++ b/drivers/clk/meson/Makefile
>>> @@ -19,3 +19,4 @@ obj-$(CONFIG_COMMON_CLK_AXG_AUDIO) += axg-audio.o
>>>   obj-$(CONFIG_COMMON_CLK_GXBB) += gxbb.o gxbb-aoclk.o
>>>   obj-$(CONFIG_COMMON_CLK_G12A) += g12a.o g12a-aoclk.o
>>>   obj-$(CONFIG_COMMON_CLK_MESON8B) += meson8b.o
>>> +obj-$(CONFIG_COMMON_CLK_A1) += a1.o
>>
>> I would guess this should be sorted on Kconfig name in this file?
> ok, I will put it behind COMMON_CLK_AXG_AUDIO
>>
>>> diff --git a/drivers/clk/meson/a1.c b/drivers/clk/meson/a1.c
>>> new file mode 100644
>>> index 000..26edae0f
>>> --- /dev/null
>>> +++ b/drivers/clk/meson/a1.c
>>> @@ -0,0 +1,2617 @@
>>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>>> +/*
>>> + * Copyright (c) 2019 Amlogic, Inc. All rights reserved.
>>> + */
>>> +
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include "clk-mpll.h"
>>> +#include "clk-pll.h"
>>> +#include "clk-regmap.h"
>>> +#include "vid-pll-div.h"
>>> +#include "clk-dualdiv.h"
>>> +#include "meson-eeclk.h"
>>> +#include "a1.h"
>>> +
>> [...]
>>> +
>>> +/*
>>> + * The Meson A1 HIFI PLL is 614.4M, it requires
>>> + * a strict register sequence to enable the PLL.
>>> + * set meson_clk_pcie_pll_ops as its ops
>>
>> Please remove this last line as it's obvious from the code what ops are
>> used.
>>
> ok, I will remove it.
>>> + */
>>> +static struct clk_regmap a1_hifi_pll = {
>>> +   .data = &(struct meson_clk_pll_data){
>>> +   .en = {
>>> +   .reg_off = ANACTRL_HIFIPLL_CTRL0,
>>> +   .shift   = 28,
>>> +   .width   = 1,
>>> +   },
>>> +   .m = {
>>> +   .reg_off = ANACTRL_HIFIPLL_CTRL0,
>>> +   .shift   = 0,
>>> +   .width   = 8,
>>> +   },
>>> +   .n = {
>>> +   .reg_off = ANACTRL_HIFIPLL_CTRL0,
>>> +   .shift   = 10,
>>> +   .width   = 5,
>>> +   },
>>> +   .frac = {
>>> +   .reg_off = ANACTRL_HIFIPLL_CTRL1,
>>> +   .shift   = 0,
>>> +   .width   = 19,
>>> +   },
>>> +   .l = {
>>> +   .reg_off = ANACTRL_HIFIPLL_STS,
>>> +   .shift   = 31,
>>> +   .width   = 1,
>>> +   },
>>> +   .table = a1_hifi_pll_params_table,
>>> +   .init_regs = a1_hifi_init_regs,
>>> +   .init_count = ARRAY_SIZE(a1_hifi_init_regs),
>>> +   },
>>> +   .hw.init = &(struct clk_init_data){
>>> +   .name = "hifi_pll",
>>> +   .ops = _clk_pcie_pll_ops,
>>> +

Re: [PATCH 2/2] clk: meson: a1: add support for Amlogic A1 clock driver

2019-09-27 Thread Jian Hu

Hi, Jerome

Thank you for review.

On 2019/9/25 23:09, Jerome Brunet wrote:

On Wed 25 Sep 2019 at 19:44, Jian Hu  wrote:


The Amlogic A1 clock includes three parts:
peripheral clocks, pll clocks, CPU clocks.
sys pll and CPU clocks will be sent in next patch.

Unlike the previous series, there is no EE/AO domain
in A1 CLK controllers.

Signed-off-by: Jian Hu 
Signed-off-by: Jianxin Pan 
---
  arch/arm64/Kconfig.platforms |1 +
  drivers/clk/meson/Kconfig|   10 +
  drivers/clk/meson/Makefile   |1 +
  drivers/clk/meson/a1.c   | 2617 ++
  drivers/clk/meson/a1.h   |  172 +++
  5 files changed, 2801 insertions(+)
  create mode 100644 drivers/clk/meson/a1.c
  create mode 100644 drivers/clk/meson/a1.h

diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
index 16d7614..a48f67d 100644
--- a/arch/arm64/Kconfig.platforms
+++ b/arch/arm64/Kconfig.platforms
@@ -138,6 +138,7 @@ config ARCH_MESON
select COMMON_CLK_AXG
select COMMON_CLK_G12A
select MESON_IRQ_GPIO
+   select COMMON_CLK_A1


This need to be separate patch addressed to Kevin once your driver is merged

ok, I will remove it in PATCH V2. And send it again after the driver is 
merged.

help
  This enables support for the arm64 based Amlogic SoCs
  such as the s905, S905X/D, S912, A113X/D or S905X/D2
diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
index dabeb43..e6cb4c3 100644
--- a/drivers/clk/meson/Kconfig
+++ b/drivers/clk/meson/Kconfig
@@ -107,3 +107,13 @@ config COMMON_CLK_G12A
help
  Support for the clock controller on Amlogic S905D2, S905X2 and S905Y2
  devices, aka g12a. Say Y if you want peripherals to work.
+
+config COMMON_CLK_A1
+   bool
+   depends on ARCH_MESON
+   select COMMON_CLK_MESON_REGMAP
+   select COMMON_CLK_MESON_DUALDIV
+   select COMMON_CLK_MESON_PLL
+   help
+ Support for the clock controller on Amlogic A113L device,
+ aka a1. Say Y if you want peripherals to work.
diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
index 3939f21..6be3a8f 100644
--- a/drivers/clk/meson/Makefile
+++ b/drivers/clk/meson/Makefile
@@ -19,3 +19,4 @@ obj-$(CONFIG_COMMON_CLK_AXG_AUDIO) += axg-audio.o
  obj-$(CONFIG_COMMON_CLK_GXBB) += gxbb.o gxbb-aoclk.o
  obj-$(CONFIG_COMMON_CLK_G12A) += g12a.o g12a-aoclk.o
  obj-$(CONFIG_COMMON_CLK_MESON8B) += meson8b.o
+obj-$(CONFIG_COMMON_CLK_A1) += a1.o
diff --git a/drivers/clk/meson/a1.c b/drivers/clk/meson/a1.c
new file mode 100644
index 000..26edae0f
--- /dev/null
+++ b/drivers/clk/meson/a1.c
@@ -0,0 +1,2617 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright (c) 2019 Amlogic, Inc. All rights reserved.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "clk-mpll.h"
+#include "clk-pll.h"
+#include "clk-regmap.h"
+#include "vid-pll-div.h"
+#include "clk-dualdiv.h"
+#include "meson-eeclk.h"
+#include "a1.h"
+
+/* PLLs clock in gates, its parent is xtal */
+static struct clk_regmap a1_xtal_clktree = {
+   .data = &(struct clk_regmap_gate_data){
+   .offset = SYS_OSCIN_CTRL,
+   .bit_idx = 0,
+   },
+   .hw.init = &(struct clk_init_data) {
+   .name = "xtal_clktree",
+   .ops = _regmap_gate_ops,
+   .parent_data = &(const struct clk_parent_data) {
+   .fw_name = "xtal",
+   },
+   .num_parents = 1,
+   .flags = CLK_IS_CRITICAL,


Is CCF even expected to touch this ever ? what about RO ops ?
Please review your other clocks with this in mind


the clock should not be changed at runtime.clk_regmap_gate_ro_ops
is a good idea. Set RO ops and remove the CLK_IS_CRITICAL flag.

+   },
+};
+
+static struct clk_regmap a1_xtal_fixpll = {
+   .data = &(struct clk_regmap_gate_data){
+   .offset = SYS_OSCIN_CTRL,
+   .bit_idx = 1,
+   },
+   .hw.init = &(struct clk_init_data) {
+   .name = "xtal_fixpll",
+   .ops = _regmap_gate_ops,
+   .parent_data = &(const struct clk_parent_data) {
+   .fw_name = "xtal",
+   },
+   .num_parents = 1,
+   .flags = CLK_IS_CRITICAL,
+   },
+};
+
+static struct clk_regmap a1_xtal_usb_phy = {
+   .data = &(struct clk_regmap_gate_data){
+   .offset = SYS_OSCIN_CTRL,
+   .bit_idx = 2,
+   },
+   .hw.init = &(struct clk_init_data) {
+   .name = "xtal_usb_phy",
+   .ops = _regmap_gate_ops,
+   .parent_data = &(const struct clk_parent_data) {
+   .fw_name = "xtal",
+   },
+   .num_parents = 1,
+   .flags = CLK_IS_CRITICAL,


How is an USB clock critical to the system ?
Please review your other clocks with comment in mind ...

the usb clock does not affect 

Re: [PATCH 2/2] clk: meson: a1: add support for Amlogic A1 clock driver

2019-09-26 Thread Jian Hu

Hi, Stephen

Thank you for review

On 2019/9/25 21:12, Stephen Boyd wrote:

Quoting Jian Hu (2019-09-25 04:44:48)

The Amlogic A1 clock includes three parts:
peripheral clocks, pll clocks, CPU clocks.
sys pll and CPU clocks will be sent in next patch.

Unlike the previous series, there is no EE/AO domain
in A1 CLK controllers.

Signed-off-by: Jian Hu 
Signed-off-by: Jianxin Pan 


This second name didn't send the patch. Please follow the signoff
procedures documented in Documentation/process/submitting-patches.rst


diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
index 16d7614..a48f67d 100644
--- a/arch/arm64/Kconfig.platforms
+++ b/arch/arm64/Kconfig.platforms
@@ -138,6 +138,7 @@ config ARCH_MESON
 select COMMON_CLK_AXG
 select COMMON_CLK_G12A
 select MESON_IRQ_GPIO
+   select COMMON_CLK_A1


Sort?

ok, I will put it behind COMMON_CLK_AXG



 help
   This enables support for the arm64 based Amlogic SoCs
   such as the s905, S905X/D, S912, A113X/D or S905X/D2
diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
index dabeb43..e6cb4c3 100644
--- a/drivers/clk/meson/Kconfig
+++ b/drivers/clk/meson/Kconfig
@@ -107,3 +107,13 @@ config COMMON_CLK_G12A
 help
   Support for the clock controller on Amlogic S905D2, S905X2 and S905Y2
   devices, aka g12a. Say Y if you want peripherals to work.
+
+config COMMON_CLK_A1


Probably should be placed somewhere alphabetically in this file?

ok, I will put it behind COMMON_CLK_AXG_AUDIO



+   bool
+   depends on ARCH_MESON
+   select COMMON_CLK_MESON_REGMAP
+   select COMMON_CLK_MESON_DUALDIV
+   select COMMON_CLK_MESON_PLL
+   help
+ Support for the clock controller on Amlogic A113L device,
+ aka a1. Say Y if you want peripherals to work.
diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
index 3939f21..6be3a8f 100644
--- a/drivers/clk/meson/Makefile
+++ b/drivers/clk/meson/Makefile
@@ -19,3 +19,4 @@ obj-$(CONFIG_COMMON_CLK_AXG_AUDIO) += axg-audio.o
  obj-$(CONFIG_COMMON_CLK_GXBB) += gxbb.o gxbb-aoclk.o
  obj-$(CONFIG_COMMON_CLK_G12A) += g12a.o g12a-aoclk.o
  obj-$(CONFIG_COMMON_CLK_MESON8B) += meson8b.o
+obj-$(CONFIG_COMMON_CLK_A1) += a1.o


I would guess this should be sorted on Kconfig name in this file?

ok, I will put it behind COMMON_CLK_AXG_AUDIO



diff --git a/drivers/clk/meson/a1.c b/drivers/clk/meson/a1.c
new file mode 100644
index 000..26edae0f
--- /dev/null
+++ b/drivers/clk/meson/a1.c
@@ -0,0 +1,2617 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright (c) 2019 Amlogic, Inc. All rights reserved.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "clk-mpll.h"
+#include "clk-pll.h"
+#include "clk-regmap.h"
+#include "vid-pll-div.h"
+#include "clk-dualdiv.h"
+#include "meson-eeclk.h"
+#include "a1.h"
+

[...]

+
+/*
+ * The Meson A1 HIFI PLL is 614.4M, it requires
+ * a strict register sequence to enable the PLL.
+ * set meson_clk_pcie_pll_ops as its ops


Please remove this last line as it's obvious from the code what ops are
used.


ok, I will remove it.

+ */
+static struct clk_regmap a1_hifi_pll = {
+   .data = &(struct meson_clk_pll_data){
+   .en = {
+   .reg_off = ANACTRL_HIFIPLL_CTRL0,
+   .shift   = 28,
+   .width   = 1,
+   },
+   .m = {
+   .reg_off = ANACTRL_HIFIPLL_CTRL0,
+   .shift   = 0,
+   .width   = 8,
+   },
+   .n = {
+   .reg_off = ANACTRL_HIFIPLL_CTRL0,
+   .shift   = 10,
+   .width   = 5,
+   },
+   .frac = {
+   .reg_off = ANACTRL_HIFIPLL_CTRL1,
+   .shift   = 0,
+   .width   = 19,
+   },
+   .l = {
+   .reg_off = ANACTRL_HIFIPLL_STS,
+   .shift   = 31,
+   .width   = 1,
+   },
+   .table = a1_hifi_pll_params_table,
+   .init_regs = a1_hifi_init_regs,
+   .init_count = ARRAY_SIZE(a1_hifi_init_regs),
+   },
+   .hw.init = &(struct clk_init_data){
+   .name = "hifi_pll",
+   .ops = _clk_pcie_pll_ops,
+   .parent_hws = (const struct clk_hw *[]) {
+   _xtal_hifipll.hw
+   },
+   .num_parents = 1,
+   },
+};
+

[..]

+
+static struct clk_regmap a1_fclk_div2 = {
+   .data = &(struct clk_regmap_gate_data){
+   .offset = ANACTRL_FIXPLL_CTRL0,
+   .bit_idx = 21,
+   },
+   .hw.init = &(struct clk_init_data){
+   .name = "fclk_div2",
+   .ops = _regmap_gate_ops,
+   .parent_hws = (const struct clk_hw *[]) {
+ 

Re: [PATCH 2/2] clk: meson: a1: add support for Amlogic A1 clock driver

2019-09-25 Thread Jerome Brunet
On Wed 25 Sep 2019 at 19:44, Jian Hu  wrote:

> The Amlogic A1 clock includes three parts:
> peripheral clocks, pll clocks, CPU clocks.
> sys pll and CPU clocks will be sent in next patch.
>
> Unlike the previous series, there is no EE/AO domain
> in A1 CLK controllers.
>
> Signed-off-by: Jian Hu 
> Signed-off-by: Jianxin Pan 
> ---
>  arch/arm64/Kconfig.platforms |1 +
>  drivers/clk/meson/Kconfig|   10 +
>  drivers/clk/meson/Makefile   |1 +
>  drivers/clk/meson/a1.c   | 2617 
> ++
>  drivers/clk/meson/a1.h   |  172 +++
>  5 files changed, 2801 insertions(+)
>  create mode 100644 drivers/clk/meson/a1.c
>  create mode 100644 drivers/clk/meson/a1.h
>
> diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
> index 16d7614..a48f67d 100644
> --- a/arch/arm64/Kconfig.platforms
> +++ b/arch/arm64/Kconfig.platforms
> @@ -138,6 +138,7 @@ config ARCH_MESON
>   select COMMON_CLK_AXG
>   select COMMON_CLK_G12A
>   select MESON_IRQ_GPIO
> + select COMMON_CLK_A1

This need to be separate patch addressed to Kevin once your driver is merged

>   help
> This enables support for the arm64 based Amlogic SoCs
> such as the s905, S905X/D, S912, A113X/D or S905X/D2
> diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
> index dabeb43..e6cb4c3 100644
> --- a/drivers/clk/meson/Kconfig
> +++ b/drivers/clk/meson/Kconfig
> @@ -107,3 +107,13 @@ config COMMON_CLK_G12A
>   help
> Support for the clock controller on Amlogic S905D2, S905X2 and S905Y2
> devices, aka g12a. Say Y if you want peripherals to work.
> +
> +config COMMON_CLK_A1
> + bool
> + depends on ARCH_MESON
> + select COMMON_CLK_MESON_REGMAP
> + select COMMON_CLK_MESON_DUALDIV
> + select COMMON_CLK_MESON_PLL
> + help
> +   Support for the clock controller on Amlogic A113L device,
> +   aka a1. Say Y if you want peripherals to work.
> diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
> index 3939f21..6be3a8f 100644
> --- a/drivers/clk/meson/Makefile
> +++ b/drivers/clk/meson/Makefile
> @@ -19,3 +19,4 @@ obj-$(CONFIG_COMMON_CLK_AXG_AUDIO) += axg-audio.o
>  obj-$(CONFIG_COMMON_CLK_GXBB) += gxbb.o gxbb-aoclk.o
>  obj-$(CONFIG_COMMON_CLK_G12A) += g12a.o g12a-aoclk.o
>  obj-$(CONFIG_COMMON_CLK_MESON8B) += meson8b.o
> +obj-$(CONFIG_COMMON_CLK_A1) += a1.o
> diff --git a/drivers/clk/meson/a1.c b/drivers/clk/meson/a1.c
> new file mode 100644
> index 000..26edae0f
> --- /dev/null
> +++ b/drivers/clk/meson/a1.c
> @@ -0,0 +1,2617 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (c) 2019 Amlogic, Inc. All rights reserved.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include "clk-mpll.h"
> +#include "clk-pll.h"
> +#include "clk-regmap.h"
> +#include "vid-pll-div.h"
> +#include "clk-dualdiv.h"
> +#include "meson-eeclk.h"
> +#include "a1.h"
> +
> +/* PLLs clock in gates, its parent is xtal */
> +static struct clk_regmap a1_xtal_clktree = {
> + .data = &(struct clk_regmap_gate_data){
> + .offset = SYS_OSCIN_CTRL,
> + .bit_idx = 0,
> + },
> + .hw.init = &(struct clk_init_data) {
> + .name = "xtal_clktree",
> + .ops = _regmap_gate_ops,
> + .parent_data = &(const struct clk_parent_data) {
> + .fw_name = "xtal",
> + },
> + .num_parents = 1,
> + .flags = CLK_IS_CRITICAL,

Is CCF even expected to touch this ever ? what about RO ops ?
Please review your other clocks with this in mind

> + },
> +};
> +
> +static struct clk_regmap a1_xtal_fixpll = {
> + .data = &(struct clk_regmap_gate_data){
> + .offset = SYS_OSCIN_CTRL,
> + .bit_idx = 1,
> + },
> + .hw.init = &(struct clk_init_data) {
> + .name = "xtal_fixpll",
> + .ops = _regmap_gate_ops,
> + .parent_data = &(const struct clk_parent_data) {
> + .fw_name = "xtal",
> + },
> + .num_parents = 1,
> + .flags = CLK_IS_CRITICAL,
> + },
> +};
> +
> +static struct clk_regmap a1_xtal_usb_phy = {
> + .data = &(struct clk_regmap_gate_data){
> + .offset = SYS_OSCIN_CTRL,
> + .bit_idx = 2,
> + },
> + .hw.init = &(struct clk_init_data) {
> + .name = "xtal_usb_phy",
> + .ops = _regmap_gate_ops,
> + .parent_data = &(const struct clk_parent_data) {
> + .fw_name = "xtal",
> + },
> + .num_parents = 1,
> + .flags = CLK_IS_CRITICAL,

How is an USB clock critical to the system ?
Please review your other clocks with comment in mind ...

> + },
> +};
> +
> +static struct clk_regmap a1_xtal_usb_ctrl = {
> + .data = &(struct clk_regmap_gate_data){
> + .offset = SYS_OSCIN_CTRL,
> + .bit_idx = 3,
> + 

Re: [PATCH 2/2] clk: meson: a1: add support for Amlogic A1 clock driver

2019-09-25 Thread Stephen Boyd
Quoting Jian Hu (2019-09-25 04:44:48)
> The Amlogic A1 clock includes three parts:
> peripheral clocks, pll clocks, CPU clocks.
> sys pll and CPU clocks will be sent in next patch.
> 
> Unlike the previous series, there is no EE/AO domain
> in A1 CLK controllers.
> 
> Signed-off-by: Jian Hu 
> Signed-off-by: Jianxin Pan 

This second name didn't send the patch. Please follow the signoff
procedures documented in Documentation/process/submitting-patches.rst

> diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
> index 16d7614..a48f67d 100644
> --- a/arch/arm64/Kconfig.platforms
> +++ b/arch/arm64/Kconfig.platforms
> @@ -138,6 +138,7 @@ config ARCH_MESON
> select COMMON_CLK_AXG
> select COMMON_CLK_G12A
> select MESON_IRQ_GPIO
> +   select COMMON_CLK_A1

Sort?

> help
>   This enables support for the arm64 based Amlogic SoCs
>   such as the s905, S905X/D, S912, A113X/D or S905X/D2
> diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
> index dabeb43..e6cb4c3 100644
> --- a/drivers/clk/meson/Kconfig
> +++ b/drivers/clk/meson/Kconfig
> @@ -107,3 +107,13 @@ config COMMON_CLK_G12A
> help
>   Support for the clock controller on Amlogic S905D2, S905X2 and 
> S905Y2
>   devices, aka g12a. Say Y if you want peripherals to work.
> +
> +config COMMON_CLK_A1

Probably should be placed somewhere alphabetically in this file?

> +   bool
> +   depends on ARCH_MESON
> +   select COMMON_CLK_MESON_REGMAP
> +   select COMMON_CLK_MESON_DUALDIV
> +   select COMMON_CLK_MESON_PLL
> +   help
> + Support for the clock controller on Amlogic A113L device,
> + aka a1. Say Y if you want peripherals to work.
> diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
> index 3939f21..6be3a8f 100644
> --- a/drivers/clk/meson/Makefile
> +++ b/drivers/clk/meson/Makefile
> @@ -19,3 +19,4 @@ obj-$(CONFIG_COMMON_CLK_AXG_AUDIO) += axg-audio.o
>  obj-$(CONFIG_COMMON_CLK_GXBB) += gxbb.o gxbb-aoclk.o
>  obj-$(CONFIG_COMMON_CLK_G12A) += g12a.o g12a-aoclk.o
>  obj-$(CONFIG_COMMON_CLK_MESON8B) += meson8b.o
> +obj-$(CONFIG_COMMON_CLK_A1) += a1.o

I would guess this should be sorted on Kconfig name in this file?

> diff --git a/drivers/clk/meson/a1.c b/drivers/clk/meson/a1.c
> new file mode 100644
> index 000..26edae0f
> --- /dev/null
> +++ b/drivers/clk/meson/a1.c
> @@ -0,0 +1,2617 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (c) 2019 Amlogic, Inc. All rights reserved.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include "clk-mpll.h"
> +#include "clk-pll.h"
> +#include "clk-regmap.h"
> +#include "vid-pll-div.h"
> +#include "clk-dualdiv.h"
> +#include "meson-eeclk.h"
> +#include "a1.h"
> +
[...]
> +
> +/*
> + * The Meson A1 HIFI PLL is 614.4M, it requires
> + * a strict register sequence to enable the PLL.
> + * set meson_clk_pcie_pll_ops as its ops

Please remove this last line as it's obvious from the code what ops are
used.

> + */
> +static struct clk_regmap a1_hifi_pll = {
> +   .data = &(struct meson_clk_pll_data){
> +   .en = {
> +   .reg_off = ANACTRL_HIFIPLL_CTRL0,
> +   .shift   = 28,
> +   .width   = 1,
> +   },
> +   .m = {
> +   .reg_off = ANACTRL_HIFIPLL_CTRL0,
> +   .shift   = 0,
> +   .width   = 8,
> +   },
> +   .n = {
> +   .reg_off = ANACTRL_HIFIPLL_CTRL0,
> +   .shift   = 10,
> +   .width   = 5,
> +   },
> +   .frac = {
> +   .reg_off = ANACTRL_HIFIPLL_CTRL1,
> +   .shift   = 0,
> +   .width   = 19,
> +   },
> +   .l = {
> +   .reg_off = ANACTRL_HIFIPLL_STS,
> +   .shift   = 31,
> +   .width   = 1,
> +   },
> +   .table = a1_hifi_pll_params_table,
> +   .init_regs = a1_hifi_init_regs,
> +   .init_count = ARRAY_SIZE(a1_hifi_init_regs),
> +   },
> +   .hw.init = &(struct clk_init_data){
> +   .name = "hifi_pll",
> +   .ops = _clk_pcie_pll_ops,
> +   .parent_hws = (const struct clk_hw *[]) {
> +   _xtal_hifipll.hw
> +   },
> +   .num_parents = 1,
> +   },
> +};
> +
[..]
> +
> +static struct clk_regmap a1_fclk_div2 = {
> +   .data = &(struct clk_regmap_gate_data){
> +   .offset = ANACTRL_FIXPLL_CTRL0,
> +   .bit_idx = 21,
> +   },
> +   .hw.init = &(struct clk_init_data){
> +   .name = "fclk_div2",
> +   .ops = _regmap_gate_ops,
> +   .parent_hws = (const struct clk_hw *[]) {
> +   

[PATCH 2/2] clk: meson: a1: add support for Amlogic A1 clock driver

2019-09-25 Thread Jian Hu
The Amlogic A1 clock includes three parts:
peripheral clocks, pll clocks, CPU clocks.
sys pll and CPU clocks will be sent in next patch.

Unlike the previous series, there is no EE/AO domain
in A1 CLK controllers.

Signed-off-by: Jian Hu 
Signed-off-by: Jianxin Pan 
---
 arch/arm64/Kconfig.platforms |1 +
 drivers/clk/meson/Kconfig|   10 +
 drivers/clk/meson/Makefile   |1 +
 drivers/clk/meson/a1.c   | 2617 ++
 drivers/clk/meson/a1.h   |  172 +++
 5 files changed, 2801 insertions(+)
 create mode 100644 drivers/clk/meson/a1.c
 create mode 100644 drivers/clk/meson/a1.h

diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
index 16d7614..a48f67d 100644
--- a/arch/arm64/Kconfig.platforms
+++ b/arch/arm64/Kconfig.platforms
@@ -138,6 +138,7 @@ config ARCH_MESON
select COMMON_CLK_AXG
select COMMON_CLK_G12A
select MESON_IRQ_GPIO
+   select COMMON_CLK_A1
help
  This enables support for the arm64 based Amlogic SoCs
  such as the s905, S905X/D, S912, A113X/D or S905X/D2
diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
index dabeb43..e6cb4c3 100644
--- a/drivers/clk/meson/Kconfig
+++ b/drivers/clk/meson/Kconfig
@@ -107,3 +107,13 @@ config COMMON_CLK_G12A
help
  Support for the clock controller on Amlogic S905D2, S905X2 and S905Y2
  devices, aka g12a. Say Y if you want peripherals to work.
+
+config COMMON_CLK_A1
+   bool
+   depends on ARCH_MESON
+   select COMMON_CLK_MESON_REGMAP
+   select COMMON_CLK_MESON_DUALDIV
+   select COMMON_CLK_MESON_PLL
+   help
+ Support for the clock controller on Amlogic A113L device,
+ aka a1. Say Y if you want peripherals to work.
diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
index 3939f21..6be3a8f 100644
--- a/drivers/clk/meson/Makefile
+++ b/drivers/clk/meson/Makefile
@@ -19,3 +19,4 @@ obj-$(CONFIG_COMMON_CLK_AXG_AUDIO) += axg-audio.o
 obj-$(CONFIG_COMMON_CLK_GXBB) += gxbb.o gxbb-aoclk.o
 obj-$(CONFIG_COMMON_CLK_G12A) += g12a.o g12a-aoclk.o
 obj-$(CONFIG_COMMON_CLK_MESON8B) += meson8b.o
+obj-$(CONFIG_COMMON_CLK_A1) += a1.o
diff --git a/drivers/clk/meson/a1.c b/drivers/clk/meson/a1.c
new file mode 100644
index 000..26edae0f
--- /dev/null
+++ b/drivers/clk/meson/a1.c
@@ -0,0 +1,2617 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright (c) 2019 Amlogic, Inc. All rights reserved.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "clk-mpll.h"
+#include "clk-pll.h"
+#include "clk-regmap.h"
+#include "vid-pll-div.h"
+#include "clk-dualdiv.h"
+#include "meson-eeclk.h"
+#include "a1.h"
+
+/* PLLs clock in gates, its parent is xtal */
+static struct clk_regmap a1_xtal_clktree = {
+   .data = &(struct clk_regmap_gate_data){
+   .offset = SYS_OSCIN_CTRL,
+   .bit_idx = 0,
+   },
+   .hw.init = &(struct clk_init_data) {
+   .name = "xtal_clktree",
+   .ops = _regmap_gate_ops,
+   .parent_data = &(const struct clk_parent_data) {
+   .fw_name = "xtal",
+   },
+   .num_parents = 1,
+   .flags = CLK_IS_CRITICAL,
+   },
+};
+
+static struct clk_regmap a1_xtal_fixpll = {
+   .data = &(struct clk_regmap_gate_data){
+   .offset = SYS_OSCIN_CTRL,
+   .bit_idx = 1,
+   },
+   .hw.init = &(struct clk_init_data) {
+   .name = "xtal_fixpll",
+   .ops = _regmap_gate_ops,
+   .parent_data = &(const struct clk_parent_data) {
+   .fw_name = "xtal",
+   },
+   .num_parents = 1,
+   .flags = CLK_IS_CRITICAL,
+   },
+};
+
+static struct clk_regmap a1_xtal_usb_phy = {
+   .data = &(struct clk_regmap_gate_data){
+   .offset = SYS_OSCIN_CTRL,
+   .bit_idx = 2,
+   },
+   .hw.init = &(struct clk_init_data) {
+   .name = "xtal_usb_phy",
+   .ops = _regmap_gate_ops,
+   .parent_data = &(const struct clk_parent_data) {
+   .fw_name = "xtal",
+   },
+   .num_parents = 1,
+   .flags = CLK_IS_CRITICAL,
+   },
+};
+
+static struct clk_regmap a1_xtal_usb_ctrl = {
+   .data = &(struct clk_regmap_gate_data){
+   .offset = SYS_OSCIN_CTRL,
+   .bit_idx = 3,
+   },
+   .hw.init = &(struct clk_init_data) {
+   .name = "xtal_usb_ctrl",
+   .ops = _regmap_gate_ops,
+   .parent_data = &(const struct clk_parent_data) {
+   .fw_name = "xtal",
+   },
+   .num_parents = 1,
+   .flags = CLK_IS_CRITICAL,
+   },
+};
+
+static struct clk_regmap a1_xtal_hifipll = {
+   .data = &(struct clk_regmap_gate_data){
+   .offset = SYS_OSCIN_CTRL,
+