Re: [PATCH 3/6] ARM: dts: broadcom: Fix SP804 node

2020-08-28 Thread André Przywara
On 26/08/2020 21:55, Florian Fainelli wrote:
> On 8/26/20 11:59 AM, Florian Fainelli wrote:
>> On 8/26/20 11:53 AM, André Przywara wrote:
>>> On 26/08/2020 19:42, Florian Fainelli wrote:

Hi Florian,

>>>
>>> Hi,
>>>
 On 8/26/20 11:38 AM, Andre Przywara wrote:
> The DT binding for SP804 requires to have an "arm,primecell" compatible
> string.
> Add this string so that the Linux primecell bus driver picks the device
> up and activates the clock.
>
> Signed-off-by: Andre Przywara 

 The commit subject should be:

 ARM: dts: NSP: Fix SP804 compatible node

 and we should probably have a Fixes tag that is:

 Fixes: a0efb0d28b77 ("ARM: dts: NSP: Add SP804 Support to DT")

 Could you please re-submit with those things corrected? Thanks
>>>
>>> Sure, will include that in a v2.
>>>
>>> Out of curiosity, do you have the hardware and can check the impact that
>>> has?
>>
>> I have the hardware and could run some tests if you would like.
>>
>>> Not sure we actually create the device without the primecell compatible?
>>> Or is the sp804 an exception here, compared to the other AMBA devices
>>> (SP805, PL011)?
>>
>> No idea, I have never used those timers personally, and I doubt that
>> anybody besides me within broadcom and hobbyists actually care about NSP
>> these days.
> 
> Seems to be working fine for me with your patch applied, it probes:
> 
> # dmesg | grep sp804
> [0.035363] clocksource: arm,sp804: mask: 0x max_cycles:
> 0x, max_idle_ns: 15290083572 ns
> 
> and it is usable:
> 
> # cat clocksource0/available_clocksource
> arm_global_timer arm,sp804
> 
> and appears to work:
> 
> # echo "arm,sp804" > clocksource0/current_clocksource
> [  105.108547] clocksource: Switched to clocksource arm,sp804
> 
> # date; sleep 5; date
> Thu Jan  1 00:01:51 UTC 1970
> Thu Jan  1 00:01:56 UTC 1970
> 
> Feel free to add Tested-by: Florian Fainelli  in
> your v2, thanks André!

Wow, thanks a lot for this test!

Sending out a v2 in a minute.

Cheers,
Andre


Re: [PATCH 3/6] ARM: dts: broadcom: Fix SP804 node

2020-08-26 Thread Rob Herring
On Wed, Aug 26, 2020 at 12:54 PM André Przywara  wrote:
>
> On 26/08/2020 19:42, Florian Fainelli wrote:
>
> Hi,
>
> > On 8/26/20 11:38 AM, Andre Przywara wrote:
> >> The DT binding for SP804 requires to have an "arm,primecell" compatible
> >> string.
> >> Add this string so that the Linux primecell bus driver picks the device
> >> up and activates the clock.
> >>
> >> Signed-off-by: Andre Przywara 
> >
> > The commit subject should be:
> >
> > ARM: dts: NSP: Fix SP804 compatible node
> >
> > and we should probably have a Fixes tag that is:
> >
> > Fixes: a0efb0d28b77 ("ARM: dts: NSP: Add SP804 Support to DT")
> >
> > Could you please re-submit with those things corrected? Thanks
>
> Sure, will include that in a v2.
>
> Out of curiosity, do you have the hardware and can check the impact that
> has?
> Not sure we actually create the device without the primecell compatible?

My first thought was no, but since the timer isn't using the driver
model (i.e. amba bus), it doesn't need it.

So I think without it, we'd create a platform device instead, but then
there's some logic to prevent that IIRC.

Rob


Re: [PATCH 3/6] ARM: dts: broadcom: Fix SP804 node

2020-08-26 Thread Florian Fainelli
On 8/26/20 11:59 AM, Florian Fainelli wrote:
> On 8/26/20 11:53 AM, André Przywara wrote:
>> On 26/08/2020 19:42, Florian Fainelli wrote:
>>
>> Hi,
>>
>>> On 8/26/20 11:38 AM, Andre Przywara wrote:
 The DT binding for SP804 requires to have an "arm,primecell" compatible
 string.
 Add this string so that the Linux primecell bus driver picks the device
 up and activates the clock.

 Signed-off-by: Andre Przywara 
>>>
>>> The commit subject should be:
>>>
>>> ARM: dts: NSP: Fix SP804 compatible node
>>>
>>> and we should probably have a Fixes tag that is:
>>>
>>> Fixes: a0efb0d28b77 ("ARM: dts: NSP: Add SP804 Support to DT")
>>>
>>> Could you please re-submit with those things corrected? Thanks
>>
>> Sure, will include that in a v2.
>>
>> Out of curiosity, do you have the hardware and can check the impact that
>> has?
> 
> I have the hardware and could run some tests if you would like.
> 
>> Not sure we actually create the device without the primecell compatible?
>> Or is the sp804 an exception here, compared to the other AMBA devices
>> (SP805, PL011)?
> 
> No idea, I have never used those timers personally, and I doubt that
> anybody besides me within broadcom and hobbyists actually care about NSP
> these days.

Seems to be working fine for me with your patch applied, it probes:

# dmesg | grep sp804
[0.035363] clocksource: arm,sp804: mask: 0x max_cycles:
0x, max_idle_ns: 15290083572 ns

and it is usable:

# cat clocksource0/available_clocksource
arm_global_timer arm,sp804

and appears to work:

# echo "arm,sp804" > clocksource0/current_clocksource
[  105.108547] clocksource: Switched to clocksource arm,sp804

# date; sleep 5; date
Thu Jan  1 00:01:51 UTC 1970
Thu Jan  1 00:01:56 UTC 1970

Feel free to add Tested-by: Florian Fainelli  in
your v2, thanks André!
-- 
Florian


Re: [PATCH 3/6] ARM: dts: broadcom: Fix SP804 node

2020-08-26 Thread Florian Fainelli
On 8/26/20 11:53 AM, André Przywara wrote:
> On 26/08/2020 19:42, Florian Fainelli wrote:
> 
> Hi,
> 
>> On 8/26/20 11:38 AM, Andre Przywara wrote:
>>> The DT binding for SP804 requires to have an "arm,primecell" compatible
>>> string.
>>> Add this string so that the Linux primecell bus driver picks the device
>>> up and activates the clock.
>>>
>>> Signed-off-by: Andre Przywara 
>>
>> The commit subject should be:
>>
>> ARM: dts: NSP: Fix SP804 compatible node
>>
>> and we should probably have a Fixes tag that is:
>>
>> Fixes: a0efb0d28b77 ("ARM: dts: NSP: Add SP804 Support to DT")
>>
>> Could you please re-submit with those things corrected? Thanks
> 
> Sure, will include that in a v2.
> 
> Out of curiosity, do you have the hardware and can check the impact that
> has?

I have the hardware and could run some tests if you would like.

> Not sure we actually create the device without the primecell compatible?
> Or is the sp804 an exception here, compared to the other AMBA devices
> (SP805, PL011)?

No idea, I have never used those timers personally, and I doubt that
anybody besides me within broadcom and hobbyists actually care about NSP
these days.
-- 
Florian


Re: [PATCH 3/6] ARM: dts: broadcom: Fix SP804 node

2020-08-26 Thread André Przywara
On 26/08/2020 19:42, Florian Fainelli wrote:

Hi,

> On 8/26/20 11:38 AM, Andre Przywara wrote:
>> The DT binding for SP804 requires to have an "arm,primecell" compatible
>> string.
>> Add this string so that the Linux primecell bus driver picks the device
>> up and activates the clock.
>>
>> Signed-off-by: Andre Przywara 
> 
> The commit subject should be:
> 
> ARM: dts: NSP: Fix SP804 compatible node
> 
> and we should probably have a Fixes tag that is:
> 
> Fixes: a0efb0d28b77 ("ARM: dts: NSP: Add SP804 Support to DT")
> 
> Could you please re-submit with those things corrected? Thanks

Sure, will include that in a v2.

Out of curiosity, do you have the hardware and can check the impact that
has?
Not sure we actually create the device without the primecell compatible?
Or is the sp804 an exception here, compared to the other AMBA devices
(SP805, PL011)?

Cheers,
Andre

>> ---
>>  arch/arm/boot/dts/bcm-nsp.dtsi | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/boot/dts/bcm-nsp.dtsi b/arch/arm/boot/dts/bcm-nsp.dtsi
>> index 0346ea621f0f..1333ef8be0a2 100644
>> --- a/arch/arm/boot/dts/bcm-nsp.dtsi
>> +++ b/arch/arm/boot/dts/bcm-nsp.dtsi
>> @@ -368,7 +368,7 @@
>>  };
>>  
>>  ccbtimer0: timer@34000 {
>> -compatible = "arm,sp804";
>> +compatible = "arm,sp804", "arm,primecell";
>>  reg = <0x34000 0x1000>;
>>  interrupts = ,
>>   ;
>>
> 
> 



Re: [PATCH 3/6] ARM: dts: broadcom: Fix SP804 node

2020-08-26 Thread Florian Fainelli
On 8/26/20 11:38 AM, Andre Przywara wrote:
> The DT binding for SP804 requires to have an "arm,primecell" compatible
> string.
> Add this string so that the Linux primecell bus driver picks the device
> up and activates the clock.
> 
> Signed-off-by: Andre Przywara 

The commit subject should be:

ARM: dts: NSP: Fix SP804 compatible node

and we should probably have a Fixes tag that is:

Fixes: a0efb0d28b77 ("ARM: dts: NSP: Add SP804 Support to DT")

Could you please re-submit with those things corrected? Thanks

> ---
>  arch/arm/boot/dts/bcm-nsp.dtsi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/boot/dts/bcm-nsp.dtsi b/arch/arm/boot/dts/bcm-nsp.dtsi
> index 0346ea621f0f..1333ef8be0a2 100644
> --- a/arch/arm/boot/dts/bcm-nsp.dtsi
> +++ b/arch/arm/boot/dts/bcm-nsp.dtsi
> @@ -368,7 +368,7 @@
>   };
>  
>   ccbtimer0: timer@34000 {
> - compatible = "arm,sp804";
> + compatible = "arm,sp804", "arm,primecell";
>   reg = <0x34000 0x1000>;
>   interrupts = ,
>;
> 


-- 
Florian


[PATCH 3/6] ARM: dts: broadcom: Fix SP804 node

2020-08-26 Thread Andre Przywara
The DT binding for SP804 requires to have an "arm,primecell" compatible
string.
Add this string so that the Linux primecell bus driver picks the device
up and activates the clock.

Signed-off-by: Andre Przywara 
---
 arch/arm/boot/dts/bcm-nsp.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/bcm-nsp.dtsi b/arch/arm/boot/dts/bcm-nsp.dtsi
index 0346ea621f0f..1333ef8be0a2 100644
--- a/arch/arm/boot/dts/bcm-nsp.dtsi
+++ b/arch/arm/boot/dts/bcm-nsp.dtsi
@@ -368,7 +368,7 @@
};
 
ccbtimer0: timer@34000 {
-   compatible = "arm,sp804";
+   compatible = "arm,sp804", "arm,primecell";
reg = <0x34000 0x1000>;
interrupts = ,
 ;
-- 
2.17.1