Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1565075 > unrolled thread

Re: [PATCH v4 2/9] ARM: dts: rockchip: update compatible property for rk3228 timer

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v4 2/9] ARM: dts: rockchip: update compatible property  for rk3228 timer Daniel Lezcano <daniel.lezcano@linaro.org> - 2017-01-23 16:50 +0100
    Re: [PATCH v4 2/9] ARM: dts: rockchip: update compatible property for rk3228 timer Heiko Stübner <heiko@sntech.de> - 2017-01-23 17:20 +0100
    Re: [PATCH v4 2/9] ARM: dts: rockchip: update compatible property for rk3228 timer Alexander Kochetkov <al.kochet@gmail.com> - 2017-01-23 17:20 +0100

#1565075 — Re: [PATCH v4 2/9] ARM: dts: rockchip: update compatible property for rk3228 timer

On Tue, Nov 29, 2016 at 07:14:45PM +0300, Alexander Kochetkov wrote:
> Property set to '"rockchip,rk3228-timer", "rockchip,rk3288-timer"'
> to match devicetree bindings.
> 
> Signed-off-by: Alexander Kochetkov <al.kochet@gmail.com>
> ---
>  arch/arm/boot/dts/rk3228-evb.dts |    4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/rk3228-evb.dts b/arch/arm/boot/dts/rk3228-evb.dts
> index 904668e..38eab87 100644
> --- a/arch/arm/boot/dts/rk3228-evb.dts
> +++ b/arch/arm/boot/dts/rk3228-evb.dts
> @@ -70,3 +70,7 @@
>  &uart2 {
>  	status = "okay";
>  };
> +
> +&timer {
> +	compatible = "rockchip,rk3228-timer", "rockchip,rk3288-timer";
> +};

I'm not sure this is correct, to which timer &timer will refer ?

        timer {
                compatible = "arm,armv7-timer";
		...
	}

        timer: timer@110c0000 {
                compatible = "rockchip,rk3288-timer";
		...
	}

Why not change the compatible string in the timer definition in rk322x.dtsi ?

Same comment for the other patches doing these changes.

  -- Daniel

-- 

 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

[toc] | [next] | [standalone]

#1565101 — Re: [PATCH v4 2/9] ARM: dts: rockchip: update compatible property for rk3228 timer

Am Montag, 23. Januar 2017, 16:40:46 CET schrieb Daniel Lezcano:
> On Tue, Nov 29, 2016 at 07:14:45PM +0300, Alexander Kochetkov wrote:
> > Property set to '"rockchip,rk3228-timer", "rockchip,rk3288-timer"'
> > to match devicetree bindings.
> > 
> > Signed-off-by: Alexander Kochetkov <al.kochet@gmail.com>
> > ---
> > 
> >  arch/arm/boot/dts/rk3228-evb.dts |    4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/arch/arm/boot/dts/rk3228-evb.dts
> > b/arch/arm/boot/dts/rk3228-evb.dts index 904668e..38eab87 100644
> > --- a/arch/arm/boot/dts/rk3228-evb.dts
> > +++ b/arch/arm/boot/dts/rk3228-evb.dts
> > @@ -70,3 +70,7 @@
> > 
> >  &uart2 {
> >  
> >  	status = "okay";
> >  
> >  };
> > 
> > +
> > +&timer {
> > +	compatible = "rockchip,rk3228-timer", "rockchip,rk3288-timer";
> > +};
> 
> I'm not sure this is correct, to which timer &timer will refer ?
> 
>         timer {
>                 compatible = "arm,armv7-timer";
> 		...
> 	}
> 
>         timer: timer@110c0000 {
>                 compatible = "rockchip,rk3288-timer";
> 		...
> 	}
> 
> Why not change the compatible string in the timer definition in rk322x.dtsi
> ?
> 
> Same comment for the other patches doing these changes.

I think Alexander didn't know which timer name to use.
Both the rk3228 and rk3229 are so similar that so far no-one has seen any 
difference and thus they really share the whole rk322x.dtsi file.

On the other hand compatibles are supposed to be explicit, so a rk322x-timer 
compatible is not allowed.


But as the socs are soooo similar, I'd suggest just using the rk3228-timer 
compatible in the rk322x.dtsi and not changing individual board files.
Similar to what Daniel suggested.

Heiko

[toc] | [prev] | [next] | [standalone]

#1565104 — Re: [PATCH v4 2/9] ARM: dts: rockchip: update compatible property for rk3228 timer

Hello Daniel!

:
> +
> +&timer {
> +	compatible = "rockchip,rk3228-timer", "rockchip,rk3288-timer";
> +};

> 23 янв. 2017 г., в 18:40, Daniel Lezcano <daniel.lezcano@linaro.org> написал(а):
> 
> I'm not sure this is correct, to which timer &timer will refer ?
> 
>        timer {
>                compatible = "arm,armv7-timer";
> 		...
> 	}
> 
>        timer: timer@110c0000 {
>                compatible = "rockchip,rk3288-timer";
> 		...
> 	}

The block '&timer { …}' change compatible string for 'timer: timer@110c0000’ timer'. It refers to it using ’timer:’ label.
'arm,armv7-timer’ has ’timer’ node name without label and cannot be addressed in underline dts files.

> Why not change the compatible string in the timer definition in rk322x.dtsi ?

Initially my patch series was for rk3188 only. I declared rk3188 timer using rk3228 compatible string[1].
Heiko reviewed the series and suggest[2] to use another ‘compatible=‘ binding for rk3188. And
suggest[3] to update rockchip,rk-timer.txt bindings, like mmc/rockchip-dw-mshc.txt does.
So I updated rockchip,rk-timer.txt. rk322x family has has two parts rk3228 and rk3229. So I replaced
rk322x to rk3228 and rk3229 and updated dts file to match bindings.

[1] http://lists.infradead.org/pipermail/linux-rockchip/2016-November/013152.html
[2] http://lists.infradead.org/pipermail/linux-rockchip/2016-November/013174.html
[3] http://lists.infradead.org/pipermail/linux-rockchip/2016-November/013174.html

> Same comment for the other patches doing these changes.
> 
>  -- Daniel

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web