Re: [PATCH v4 07/13] ARM: dts: rockchip: add clocks in vop iommu nodes

[Tomasz Figa]

On Fri, Jan 19, 2018 at 1:55 PM, JeffyChen  wrote:
> Hi Tomasz,
>
> Thanks for your reply.
>
>
> On 01/19/2018 11:23 AM, Tomasz Figa wrote:
>>
>> On Thu, Jan 18, 2018 at 8:52 PM, Jeffy Chen 
>> wrote:
>>>
>>> Add clocks in vop iommu nodes, since we are going to control clocks in
>>> rockchip iommu driver.
>>>
>>> Signed-off-by: Jeffy Chen 
>>> ---
>>>
>>> Changes in v4: None
>>> Changes in v3: None
>>> Changes in v2: None
>>>
>>>   arch/arm/boot/dts/rk3036.dtsi | 2 ++
>>>   arch/arm/boot/dts/rk3288.dtsi | 4 
>>>   2 files changed, 6 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/rk3036.dtsi
>>> b/arch/arm/boot/dts/rk3036.dtsi
>>> index 3b704cfed69a..95b0ebc7a40f 100644
>>> --- a/arch/arm/boot/dts/rk3036.dtsi
>>> +++ b/arch/arm/boot/dts/rk3036.dtsi
>>> @@ -197,6 +197,8 @@
>>>  reg = <0x10118300 0x100>;
>>>  interrupts = ;
>>>  interrupt-names = "vop_mmu";
>>> +   clocks = < ACLK_LCDC>, < SCLK_LCDC>, <
>>> HCLK_LCDC>;
>>> +   clock-names = "aclk_vop", "dclk_vop", "hclk_vop";
>>
>>
>> We should remove clock-names from IOMMU nodes. The Rockchip IOMMU
>> bindings don't define clock names and only the clocks property should
>> be given.
>>
> hmmm, i'm trying to switch to clk_bulk APIs, the get and put are name based.
> or maybe i can use clk_get/put along with other clk_bulk APIs

I think it should be possible to just put the clock pointers to the
clk_bulk_data struct manually. Otherwise, I'm not sure what names we
could use for clock-names, since the clocks depend on master.
(Something like "clock0, clock1, clock2, ..., clockN" could work, but
it doesn't add any value IMHO...).

Re: [PATCH v4 07/13] ARM: dts: rockchip: add clocks in vop iommu nodes

[Tomasz Figa]

On Fri, Jan 19, 2018 at 1:55 PM, JeffyChen  wrote:
> Hi Tomasz,
>
> Thanks for your reply.
>
>
> On 01/19/2018 11:23 AM, Tomasz Figa wrote:
>>
>> On Thu, Jan 18, 2018 at 8:52 PM, Jeffy Chen 
>> wrote:
>>>
>>> Add clocks in vop iommu nodes, since we are going to control clocks in
>>> rockchip iommu driver.
>>>
>>> Signed-off-by: Jeffy Chen 
>>> ---
>>>
>>> Changes in v4: None
>>> Changes in v3: None
>>> Changes in v2: None
>>>
>>>   arch/arm/boot/dts/rk3036.dtsi | 2 ++
>>>   arch/arm/boot/dts/rk3288.dtsi | 4 
>>>   2 files changed, 6 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/rk3036.dtsi
>>> b/arch/arm/boot/dts/rk3036.dtsi
>>> index 3b704cfed69a..95b0ebc7a40f 100644
>>> --- a/arch/arm/boot/dts/rk3036.dtsi
>>> +++ b/arch/arm/boot/dts/rk3036.dtsi
>>> @@ -197,6 +197,8 @@
>>>  reg = <0x10118300 0x100>;
>>>  interrupts = ;
>>>  interrupt-names = "vop_mmu";
>>> +   clocks = < ACLK_LCDC>, < SCLK_LCDC>, <
>>> HCLK_LCDC>;
>>> +   clock-names = "aclk_vop", "dclk_vop", "hclk_vop";
>>
>>
>> We should remove clock-names from IOMMU nodes. The Rockchip IOMMU
>> bindings don't define clock names and only the clocks property should
>> be given.
>>
> hmmm, i'm trying to switch to clk_bulk APIs, the get and put are name based.
> or maybe i can use clk_get/put along with other clk_bulk APIs

I think it should be possible to just put the clock pointers to the
clk_bulk_data struct manually. Otherwise, I'm not sure what names we
could use for clock-names, since the clocks depend on master.
(Something like "clock0, clock1, clock2, ..., clockN" could work, but
it doesn't add any value IMHO...).

Re: [PATCH v4 07/13] ARM: dts: rockchip: add clocks in vop iommu nodes

[JeffyChen]

Hi Tomasz,

Thanks for your reply.

On 01/19/2018 11:23 AM, Tomasz Figa wrote:

On Thu, Jan 18, 2018 at 8:52 PM, Jeffy Chen  wrote:

Add clocks in vop iommu nodes, since we are going to control clocks in
rockchip iommu driver.

Signed-off-by: Jeffy Chen 
---

Changes in v4: None
Changes in v3: None
Changes in v2: None

  arch/arm/boot/dts/rk3036.dtsi | 2 ++
  arch/arm/boot/dts/rk3288.dtsi | 4 
  2 files changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/rk3036.dtsi b/arch/arm/boot/dts/rk3036.dtsi
index 3b704cfed69a..95b0ebc7a40f 100644
--- a/arch/arm/boot/dts/rk3036.dtsi
+++ b/arch/arm/boot/dts/rk3036.dtsi
@@ -197,6 +197,8 @@
 reg = <0x10118300 0x100>;
 interrupts = ;
 interrupt-names = "vop_mmu";
+   clocks = < ACLK_LCDC>, < SCLK_LCDC>, < HCLK_LCDC>;
+   clock-names = "aclk_vop", "dclk_vop", "hclk_vop";


We should remove clock-names from IOMMU nodes. The Rockchip IOMMU
bindings don't define clock names and only the clocks property should
be given.

hmmm, i'm trying to switch to clk_bulk APIs, the get and put are name 
based. or maybe i can use clk_get/put along with other clk_bulk APIs

Not even saying that the names currently listed are not good examples,
they name SoC clock controller output, rather than device inputs.

Best regards,
Tomasz

Re: [PATCH v4 07/13] ARM: dts: rockchip: add clocks in vop iommu nodes

[JeffyChen]

Hi Tomasz,

Thanks for your reply.

On 01/19/2018 11:23 AM, Tomasz Figa wrote:

On Thu, Jan 18, 2018 at 8:52 PM, Jeffy Chen  wrote:

Add clocks in vop iommu nodes, since we are going to control clocks in
rockchip iommu driver.

Signed-off-by: Jeffy Chen 
---

Changes in v4: None
Changes in v3: None
Changes in v2: None

  arch/arm/boot/dts/rk3036.dtsi | 2 ++
  arch/arm/boot/dts/rk3288.dtsi | 4 
  2 files changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/rk3036.dtsi b/arch/arm/boot/dts/rk3036.dtsi
index 3b704cfed69a..95b0ebc7a40f 100644
--- a/arch/arm/boot/dts/rk3036.dtsi
+++ b/arch/arm/boot/dts/rk3036.dtsi
@@ -197,6 +197,8 @@
 reg = <0x10118300 0x100>;
 interrupts = ;
 interrupt-names = "vop_mmu";
+   clocks = < ACLK_LCDC>, < SCLK_LCDC>, < HCLK_LCDC>;
+   clock-names = "aclk_vop", "dclk_vop", "hclk_vop";


We should remove clock-names from IOMMU nodes. The Rockchip IOMMU
bindings don't define clock names and only the clocks property should
be given.

hmmm, i'm trying to switch to clk_bulk APIs, the get and put are name 
based. or maybe i can use clk_get/put along with other clk_bulk APIs

Not even saying that the names currently listed are not good examples,
they name SoC clock controller output, rather than device inputs.

Best regards,
Tomasz

Re: [PATCH v4 07/13] ARM: dts: rockchip: add clocks in vop iommu nodes

[Tomasz Figa]

On Thu, Jan 18, 2018 at 8:52 PM, Jeffy Chen  wrote:
> Add clocks in vop iommu nodes, since we are going to control clocks in
> rockchip iommu driver.
>
> Signed-off-by: Jeffy Chen 
> ---
>
> Changes in v4: None
> Changes in v3: None
> Changes in v2: None
>
>  arch/arm/boot/dts/rk3036.dtsi | 2 ++
>  arch/arm/boot/dts/rk3288.dtsi | 4 
>  2 files changed, 6 insertions(+)
>
> diff --git a/arch/arm/boot/dts/rk3036.dtsi b/arch/arm/boot/dts/rk3036.dtsi
> index 3b704cfed69a..95b0ebc7a40f 100644
> --- a/arch/arm/boot/dts/rk3036.dtsi
> +++ b/arch/arm/boot/dts/rk3036.dtsi
> @@ -197,6 +197,8 @@
> reg = <0x10118300 0x100>;
> interrupts = ;
> interrupt-names = "vop_mmu";
> +   clocks = < ACLK_LCDC>, < SCLK_LCDC>, < HCLK_LCDC>;
> +   clock-names = "aclk_vop", "dclk_vop", "hclk_vop";

We should remove clock-names from IOMMU nodes. The Rockchip IOMMU
bindings don't define clock names and only the clocks property should
be given.

Not even saying that the names currently listed are not good examples,
they name SoC clock controller output, rather than device inputs.

Best regards,
Tomasz

Re: [PATCH v4 07/13] ARM: dts: rockchip: add clocks in vop iommu nodes

[Tomasz Figa]

On Thu, Jan 18, 2018 at 8:52 PM, Jeffy Chen  wrote:
> Add clocks in vop iommu nodes, since we are going to control clocks in
> rockchip iommu driver.
>
> Signed-off-by: Jeffy Chen 
> ---
>
> Changes in v4: None
> Changes in v3: None
> Changes in v2: None
>
>  arch/arm/boot/dts/rk3036.dtsi | 2 ++
>  arch/arm/boot/dts/rk3288.dtsi | 4 
>  2 files changed, 6 insertions(+)
>
> diff --git a/arch/arm/boot/dts/rk3036.dtsi b/arch/arm/boot/dts/rk3036.dtsi
> index 3b704cfed69a..95b0ebc7a40f 100644
> --- a/arch/arm/boot/dts/rk3036.dtsi
> +++ b/arch/arm/boot/dts/rk3036.dtsi
> @@ -197,6 +197,8 @@
> reg = <0x10118300 0x100>;
> interrupts = ;
> interrupt-names = "vop_mmu";
> +   clocks = < ACLK_LCDC>, < SCLK_LCDC>, < HCLK_LCDC>;
> +   clock-names = "aclk_vop", "dclk_vop", "hclk_vop";

We should remove clock-names from IOMMU nodes. The Rockchip IOMMU
bindings don't define clock names and only the clocks property should
be given.

Not even saying that the names currently listed are not good examples,
they name SoC clock controller output, rather than device inputs.

Best regards,
Tomasz

[PATCH v4 07/13] ARM: dts: rockchip: add clocks in vop iommu nodes

[Jeffy Chen]

Add clocks in vop iommu nodes, since we are going to control clocks in
rockchip iommu driver.

Signed-off-by: Jeffy Chen 
---

Changes in v4: None
Changes in v3: None
Changes in v2: None

 arch/arm/boot/dts/rk3036.dtsi | 2 ++
 arch/arm/boot/dts/rk3288.dtsi | 4 
 2 files changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/rk3036.dtsi b/arch/arm/boot/dts/rk3036.dtsi
index 3b704cfed69a..95b0ebc7a40f 100644
--- a/arch/arm/boot/dts/rk3036.dtsi
+++ b/arch/arm/boot/dts/rk3036.dtsi
@@ -197,6 +197,8 @@
reg = <0x10118300 0x100>;
interrupts = ;
interrupt-names = "vop_mmu";
+   clocks = < ACLK_LCDC>, < SCLK_LCDC>, < HCLK_LCDC>;
+   clock-names = "aclk_vop", "dclk_vop", "hclk_vop";
#iommu-cells = <0>;
status = "disabled";
};
diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi
index 6102e4e7f35c..5132dd887297 100644
--- a/arch/arm/boot/dts/rk3288.dtsi
+++ b/arch/arm/boot/dts/rk3288.dtsi
@@ -1026,6 +1026,8 @@
reg = <0x0 0xff930300 0x0 0x100>;
interrupts = ;
interrupt-names = "vopb_mmu";
+   clocks = < ACLK_VOP0>, < DCLK_VOP0>, < HCLK_VOP0>;
+   clock-names = "aclk_vop", "dclk_vop", "hclk_vop";
power-domains = < RK3288_PD_VIO>;
#iommu-cells = <0>;
status = "disabled";
@@ -1074,6 +1076,8 @@
reg = <0x0 0xff940300 0x0 0x100>;
interrupts = ;
interrupt-names = "vopl_mmu";
+   clocks = < ACLK_VOP1>, < DCLK_VOP1>, < HCLK_VOP1>;
+   clock-names = "aclk_vop", "dclk_vop", "hclk_vop";
power-domains = < RK3288_PD_VIO>;
#iommu-cells = <0>;
status = "disabled";
-- 
2.11.0

[PATCH v4 07/13] ARM: dts: rockchip: add clocks in vop iommu nodes

[Jeffy Chen]

Add clocks in vop iommu nodes, since we are going to control clocks in
rockchip iommu driver.

Signed-off-by: Jeffy Chen 
---

Changes in v4: None
Changes in v3: None
Changes in v2: None

 arch/arm/boot/dts/rk3036.dtsi | 2 ++
 arch/arm/boot/dts/rk3288.dtsi | 4 
 2 files changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/rk3036.dtsi b/arch/arm/boot/dts/rk3036.dtsi
index 3b704cfed69a..95b0ebc7a40f 100644
--- a/arch/arm/boot/dts/rk3036.dtsi
+++ b/arch/arm/boot/dts/rk3036.dtsi
@@ -197,6 +197,8 @@
reg = <0x10118300 0x100>;
interrupts = ;
interrupt-names = "vop_mmu";
+   clocks = < ACLK_LCDC>, < SCLK_LCDC>, < HCLK_LCDC>;
+   clock-names = "aclk_vop", "dclk_vop", "hclk_vop";
#iommu-cells = <0>;
status = "disabled";
};
diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi
index 6102e4e7f35c..5132dd887297 100644
--- a/arch/arm/boot/dts/rk3288.dtsi
+++ b/arch/arm/boot/dts/rk3288.dtsi
@@ -1026,6 +1026,8 @@
reg = <0x0 0xff930300 0x0 0x100>;
interrupts = ;
interrupt-names = "vopb_mmu";
+   clocks = < ACLK_VOP0>, < DCLK_VOP0>, < HCLK_VOP0>;
+   clock-names = "aclk_vop", "dclk_vop", "hclk_vop";
power-domains = < RK3288_PD_VIO>;
#iommu-cells = <0>;
status = "disabled";
@@ -1074,6 +1076,8 @@
reg = <0x0 0xff940300 0x0 0x100>;
interrupts = ;
interrupt-names = "vopl_mmu";
+   clocks = < ACLK_VOP1>, < DCLK_VOP1>, < HCLK_VOP1>;
+   clock-names = "aclk_vop", "dclk_vop", "hclk_vop";
power-domains = < RK3288_PD_VIO>;
#iommu-cells = <0>;
status = "disabled";
-- 
2.11.0