[v2,1/4] arm64: dts: rk3399-u-boot: Delete vop assigned-clocks/rates

Linux supporting assigned-clocks for VOP on rk3399 by assuming
U-Boot not initializing it on this linux commit:

commit <617f4472bdd3> ("arm64: dts: rockchip: init rk3399 vop clock rates")

There is no specific need to initialize these assigned clock
in U-Boot as video drivers still work with default aclk and  
hclk values. So, these clocks are simply not supported by rk3399
clock driver.

But, during stdio probe of vidconsole, the device probe
will try to check whether the assigned clocks on that video
console node is initialized or not? and return error if not.

So, delete these property via -u-boot dtsi as there is
no specific need in U-Boot.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
Changes for v2:
- none

 arch/arm/dts/rk3399-u-boot.dtsi | 4 ++++
 1 file changed, 4 insertions(+)
  

> From: Jagan Teki <jagan@amarulasolutions.com>
> Cc: sunil@amarulasolutions.com, u-boot@lists.denx.de,
>         linux-rockchip@lists.infradead.org, linux-amarula@amarulasolutions.com,
>         Jagan Teki <jagan@amarulasolutions.com>
> Date: Mon, 30 Mar 2020 23:46:10 +0530
> Content-Type: text/plain; charset=UTF-8
> 
> Linux supporting assigned-clocks for VOP on rk3399 by assuming
> U-Boot not initializing it on this linux commit:
> 
> commit <617f4472bdd3> ("arm64: dts: rockchip: init rk3399 vop clock rates")
> 
> There is no specific need to initialize these assigned clock
> in U-Boot as video drivers still work with default aclk and  
> hclk values. So, these clocks are simply not supported by rk3399
> clock driver.
> 
> But, during stdio probe of vidconsole, the device probe
> will try to check whether the assigned clocks on that video
> console node is initialized or not? and return error if not.
> 
> So, delete these property via -u-boot dtsi as there is
> no specific need in U-Boot.

Deleting these properties isn't very helpful as it means the U-Boot
device tree can no longer be used by the kernel.  Isn't it a better
idea to implement these clocks as stubs in the u-boot clock driver?

> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> ---
> Changes for v2:
> - none
> 
>  arch/arm/dts/rk3399-u-boot.dtsi | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/arm/dts/rk3399-u-boot.dtsi b/arch/arm/dts/rk3399-u-boot.dtsi
> index 8b857ccfc7..b846f9cde7 100644
> --- a/arch/arm/dts/rk3399-u-boot.dtsi
> +++ b/arch/arm/dts/rk3399-u-boot.dtsi
> @@ -99,9 +99,13 @@
>  };
>  
>  &vopb {
> +	/delete-property/ assigned-clocks;
> +	/delete-property/ assigned-clock-rates;
>  	u-boot,dm-pre-reloc;
>  };
>  
>  &vopl {
> +	/delete-property/ assigned-clocks;
> +	/delete-property/ assigned-clock-rates;
>  	u-boot,dm-pre-reloc;
>  };
> -- 
> 2.17.1
> 
>
  

On Tue, Mar 31, 2020 at 1:06 AM Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>
> > From: Jagan Teki <jagan@amarulasolutions.com>
> > Cc: sunil@amarulasolutions.com, u-boot@lists.denx.de,
> >         linux-rockchip@lists.infradead.org, linux-amarula@amarulasolutions.com,
> >         Jagan Teki <jagan@amarulasolutions.com>
> > Date: Mon, 30 Mar 2020 23:46:10 +0530
> > Content-Type: text/plain; charset=UTF-8
> >
> > Linux supporting assigned-clocks for VOP on rk3399 by assuming
> > U-Boot not initializing it on this linux commit:
> >
> > commit <617f4472bdd3> ("arm64: dts: rockchip: init rk3399 vop clock rates")
> >
> > There is no specific need to initialize these assigned clock
> > in U-Boot as video drivers still work with default aclk and
> > hclk values. So, these clocks are simply not supported by rk3399
> > clock driver.
> >
> > But, during stdio probe of vidconsole, the device probe
> > will try to check whether the assigned clocks on that video
> > console node is initialized or not? and return error if not.
> >
> > So, delete these property via -u-boot dtsi as there is
> > no specific need in U-Boot.
>
> Deleting these properties isn't very helpful as it means the U-Boot
> device tree can no longer be used by the kernel.  Isn't it a better
> idea to implement these clocks as stubs in the u-boot clock driver?

I did try this before sorting out these changes, seems like it
requires a bit more tweaking the clock wrt display code. I really
didn't see any use case as of now for just to print u-boot log on
display out, and more over this support has been broken since from
releases. so bypassing these nodes can be a solutions for now.

Jagan.
  

Hi Jagan,

On 2020/3/31 下午1:59, Jagan Teki wrote:
> On Tue, Mar 31, 2020 at 1:06 AM Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>>> From: Jagan Teki <jagan@amarulasolutions.com>
>>> Cc: sunil@amarulasolutions.com, u-boot@lists.denx.de,
>>>          linux-rockchip@lists.infradead.org, linux-amarula@amarulasolutions.com,
>>>          Jagan Teki <jagan@amarulasolutions.com>
>>> Date: Mon, 30 Mar 2020 23:46:10 +0530
>>> Content-Type: text/plain; charset=UTF-8
>>>
>>> Linux supporting assigned-clocks for VOP on rk3399 by assuming
>>> U-Boot not initializing it on this linux commit:
>>>
>>> commit <617f4472bdd3> ("arm64: dts: rockchip: init rk3399 vop clock rates")
>>>
>>> There is no specific need to initialize these assigned clock
>>> in U-Boot as video drivers still work with default aclk and
>>> hclk values. So, these clocks are simply not supported by rk3399
>>> clock driver.
>>>
>>> But, during stdio probe of vidconsole, the device probe
>>> will try to check whether the assigned clocks on that video
>>> console node is initialized or not? and return error if not.
>>>
>>> So, delete these property via -u-boot dtsi as there is
>>> no specific need in U-Boot.
>> Deleting these properties isn't very helpful as it means the U-Boot
>> device tree can no longer be used by the kernel.  Isn't it a better
>> idea to implement these clocks as stubs in the u-boot clock driver?
> I did try this before sorting out these changes, seems like it
> requires a bit more tweaking the clock wrt display code. I really
> didn't see any use case as of now for just to print u-boot log on
> display out, and more over this support has been broken since from
> releases. so bypassing these nodes can be a solutions for now.


I agree with Mark for not touch the dts first. I don't know the detail 
of display driver but:

- The rk3399 driver use to work without touch dts from kernel;

- the clock driver have a rk3399_vop_set_clk() which does not depends on 
dts.


Thanks,

- Kever

>
> Jagan.
>
>
  

Hi Kever,

On Thu, Apr 2, 2020 at 2:48 PM Kever Yang <kever.yang@rock-chips.com> wrote:
>
> Hi Jagan,
>
> On 2020/3/31 下午1:59, Jagan Teki wrote:
> > On Tue, Mar 31, 2020 at 1:06 AM Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
> >>> From: Jagan Teki <jagan@amarulasolutions.com>
> >>> Cc: sunil@amarulasolutions.com, u-boot@lists.denx.de,
> >>>          linux-rockchip@lists.infradead.org, linux-amarula@amarulasolutions.com,
> >>>          Jagan Teki <jagan@amarulasolutions.com>
> >>> Date: Mon, 30 Mar 2020 23:46:10 +0530
> >>> Content-Type: text/plain; charset=UTF-8
> >>>
> >>> Linux supporting assigned-clocks for VOP on rk3399 by assuming
> >>> U-Boot not initializing it on this linux commit:
> >>>
> >>> commit <617f4472bdd3> ("arm64: dts: rockchip: init rk3399 vop clock rates")
> >>>
> >>> There is no specific need to initialize these assigned clock
> >>> in U-Boot as video drivers still work with default aclk and
> >>> hclk values. So, these clocks are simply not supported by rk3399
> >>> clock driver.
> >>>
> >>> But, during stdio probe of vidconsole, the device probe
> >>> will try to check whether the assigned clocks on that video
> >>> console node is initialized or not? and return error if not.
> >>>
> >>> So, delete these property via -u-boot dtsi as there is
> >>> no specific need in U-Boot.
> >> Deleting these properties isn't very helpful as it means the U-Boot
> >> device tree can no longer be used by the kernel.  Isn't it a better
> >> idea to implement these clocks as stubs in the u-boot clock driver?
> > I did try this before sorting out these changes, seems like it
> > requires a bit more tweaking the clock wrt display code. I really
> > didn't see any use case as of now for just to print u-boot log on
> > display out, and more over this support has been broken since from
> > releases. so bypassing these nodes can be a solutions for now.
>
>
> I agree with Mark for not touch the dts first. I don't know the detail
> of display driver but:
>
> - The rk3399 driver use to work without touch dts from kernel;
>
> - the clock driver have a rk3399_vop_set_clk() which does not depends on
> dts.

The existing video drivers are written based on the puma dts and those
are not inline to Linux dts files, i.e. the reason the code is pushed
I think. The rest of rk3399 dtsi files are now inline to Linux as and
display out on these are broken from last 2 releases. so my idea is to
resolve the things one-after-another like
1. Make existing video stuff work with all rk3399 (this series along
with this patch)
2. Drop this patch change and make video drivers working w/o any
explicit changes in dts like this patch does.

Since step 2, would take time, and require close testing of all boards
I would like to pick the existing stuff for the release. Mark my words
to fix the things for the next release.

Jagan.
  

> From: Jagan Teki <jagan@amarulasolutions.com>
> Date: Thu, 2 Apr 2020 15:07:01 +0530
> 
> Hi Kever,
> 
> On Thu, Apr 2, 2020 at 2:48 PM Kever Yang <kever.yang@rock-chips.com> wrote:
> >
> > Hi Jagan,
> >
> > On 2020/3/31 下午1:59, Jagan Teki wrote:
> > > On Tue, Mar 31, 2020 at 1:06 AM Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
> > >>> From: Jagan Teki <jagan@amarulasolutions.com>
> > >>> Cc: sunil@amarulasolutions.com, u-boot@lists.denx.de,
> > >>>          linux-rockchip@lists.infradead.org, linux-amarula@amarulasolutions.com,
> > >>>          Jagan Teki <jagan@amarulasolutions.com>
> > >>> Date: Mon, 30 Mar 2020 23:46:10 +0530
> > >>> Content-Type: text/plain; charset=UTF-8
> > >>>
> > >>> Linux supporting assigned-clocks for VOP on rk3399 by assuming
> > >>> U-Boot not initializing it on this linux commit:
> > >>>
> > >>> commit <617f4472bdd3> ("arm64: dts: rockchip: init rk3399 vop clock rates")
> > >>>
> > >>> There is no specific need to initialize these assigned clock
> > >>> in U-Boot as video drivers still work with default aclk and
> > >>> hclk values. So, these clocks are simply not supported by rk3399
> > >>> clock driver.
> > >>>
> > >>> But, during stdio probe of vidconsole, the device probe
> > >>> will try to check whether the assigned clocks on that video
> > >>> console node is initialized or not? and return error if not.
> > >>>
> > >>> So, delete these property via -u-boot dtsi as there is
> > >>> no specific need in U-Boot.
> > >> Deleting these properties isn't very helpful as it means the U-Boot
> > >> device tree can no longer be used by the kernel.  Isn't it a better
> > >> idea to implement these clocks as stubs in the u-boot clock driver?
> > > I did try this before sorting out these changes, seems like it
> > > requires a bit more tweaking the clock wrt display code. I really
> > > didn't see any use case as of now for just to print u-boot log on
> > > display out, and more over this support has been broken since from
> > > releases. so bypassing these nodes can be a solutions for now.
> >
> >
> > I agree with Mark for not touch the dts first. I don't know the detail
> > of display driver but:
> >
> > - The rk3399 driver use to work without touch dts from kernel;
> >
> > - the clock driver have a rk3399_vop_set_clk() which does not depends on
> > dts.
> 
> The existing video drivers are written based on the puma dts and those
> are not inline to Linux dts files, i.e. the reason the code is pushed
> I think. The rest of rk3399 dtsi files are now inline to Linux as and
> display out on these are broken from last 2 releases. so my idea is to
> resolve the things one-after-another like
> 1. Make existing video stuff work with all rk3399 (this series along
> with this patch)
> 2. Drop this patch change and make video drivers working w/o any
> explicit changes in dts like this patch does.
> 
> Since step 2, would take time, and require close testing of all boards
> I would like to pick the existing stuff for the release. Mark my words
> to fix the things for the next release.

Fair enough.  I don't think fixing the issue is too difficult, but it
is better to do these things in small steps anyway.
  

On Thu, Apr 2, 2020 at 3:48 PM Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>
> > From: Jagan Teki <jagan@amarulasolutions.com>
> > Date: Thu, 2 Apr 2020 15:07:01 +0530
> >
> > Hi Kever,
> >
> > On Thu, Apr 2, 2020 at 2:48 PM Kever Yang <kever.yang@rock-chips.com> wrote:
> > >
> > > Hi Jagan,
> > >
> > > On 2020/3/31 下午1:59, Jagan Teki wrote:
> > > > On Tue, Mar 31, 2020 at 1:06 AM Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
> > > >>> From: Jagan Teki <jagan@amarulasolutions.com>
> > > >>> Cc: sunil@amarulasolutions.com, u-boot@lists.denx.de,
> > > >>>          linux-rockchip@lists.infradead.org, linux-amarula@amarulasolutions.com,
> > > >>>          Jagan Teki <jagan@amarulasolutions.com>
> > > >>> Date: Mon, 30 Mar 2020 23:46:10 +0530
> > > >>> Content-Type: text/plain; charset=UTF-8
> > > >>>
> > > >>> Linux supporting assigned-clocks for VOP on rk3399 by assuming
> > > >>> U-Boot not initializing it on this linux commit:
> > > >>>
> > > >>> commit <617f4472bdd3> ("arm64: dts: rockchip: init rk3399 vop clock rates")
> > > >>>
> > > >>> There is no specific need to initialize these assigned clock
> > > >>> in U-Boot as video drivers still work with default aclk and
> > > >>> hclk values. So, these clocks are simply not supported by rk3399
> > > >>> clock driver.
> > > >>>
> > > >>> But, during stdio probe of vidconsole, the device probe
> > > >>> will try to check whether the assigned clocks on that video
> > > >>> console node is initialized or not? and return error if not.
> > > >>>
> > > >>> So, delete these property via -u-boot dtsi as there is
> > > >>> no specific need in U-Boot.
> > > >> Deleting these properties isn't very helpful as it means the U-Boot
> > > >> device tree can no longer be used by the kernel.  Isn't it a better
> > > >> idea to implement these clocks as stubs in the u-boot clock driver?
> > > > I did try this before sorting out these changes, seems like it
> > > > requires a bit more tweaking the clock wrt display code. I really
> > > > didn't see any use case as of now for just to print u-boot log on
> > > > display out, and more over this support has been broken since from
> > > > releases. so bypassing these nodes can be a solutions for now.
> > >
> > >
> > > I agree with Mark for not touch the dts first. I don't know the detail
> > > of display driver but:
> > >
> > > - The rk3399 driver use to work without touch dts from kernel;
> > >
> > > - the clock driver have a rk3399_vop_set_clk() which does not depends on
> > > dts.
> >
> > The existing video drivers are written based on the puma dts and those
> > are not inline to Linux dts files, i.e. the reason the code is pushed
> > I think. The rest of rk3399 dtsi files are now inline to Linux as and
> > display out on these are broken from last 2 releases. so my idea is to
> > resolve the things one-after-another like
> > 1. Make existing video stuff work with all rk3399 (this series along
> > with this patch)
> > 2. Drop this patch change and make video drivers working w/o any
> > explicit changes in dts like this patch does.
> >
> > Since step 2, would take time, and require close testing of all boards
> > I would like to pick the existing stuff for the release. Mark my words
> > to fix the things for the next release.
>
> Fair enough.  I don't think fixing the issue is too difficult, but it
> is better to do these things in small steps anyway.

I have managed to fix this via clock driver(which seems more
reasonable as per Kever), so this is dropped in v3. thanks for
noticing.

Jagan.