diff mbox series

[2/3] arm64: dts: allwinner: a64: Add hwspinlock node

Message ID 20200210170143.20007-3-nborisov@suse.com (mailing list archive)
State New, archived
Headers show
Series Add support for hwspinlock on A64 SoC | expand

Commit Message

Nikolay Borisov Feb. 10, 2020, 5:01 p.m. UTC
Add a node describing the hwspinlock on A64.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 9 +++++++++
 1 file changed, 9 insertions(+)

Comments

Maxime Ripard Feb. 11, 2020, 7:55 a.m. UTC | #1
Hi,

On Mon, Feb 10, 2020 at 07:01:42PM +0200, Nikolay Borisov wrote:
> Add a node describing the hwspinlock on A64.
>
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---
>  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> index 27e48234f1c2..01de89fc09cc 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> @@ -1182,5 +1182,14 @@
>  			interrupts = <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
>  			clocks = <&osc24M>;
>  		};
> +
> +		hwspinlock: spinlock@1c18000 {

Nodes are supposed to be ordered by ascending physical address

> +			compatible = "allwinner,sun50i-a64-hwspinlock";
> +			reg = <0x01c18000 0x1000>;
> +			clocks = <&ccu CLK_BUS_SPINLOCK>;
> +			resets = <&ccu RST_BUS_SPINLOCK>;
> +			#hwlock-cells = <1>;
> +			status = "disabled";

Is there a reason to disable it?

Also, my understanding was that hwspinlocks were meant to be used by
client drivers, so surely we must tie them to other device nodes too,
right?

Maxime
Nikolay Borisov Feb. 11, 2020, 8:09 a.m. UTC | #2
On 11.02.20 г. 9:55 ч., Maxime Ripard wrote:
> Hi,
> 
> On Mon, Feb 10, 2020 at 07:01:42PM +0200, Nikolay Borisov wrote:
>> Add a node describing the hwspinlock on A64.
>>
>> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
>> ---
>>  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 9 +++++++++
>>  1 file changed, 9 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
>> index 27e48234f1c2..01de89fc09cc 100644
>> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
>> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
>> @@ -1182,5 +1182,14 @@
>>  			interrupts = <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
>>  			clocks = <&osc24M>;
>>  		};
>> +
>> +		hwspinlock: spinlock@1c18000 {
> 
> Nodes are supposed to be ordered by ascending physical address
> 
>> +			compatible = "allwinner,sun50i-a64-hwspinlock";
>> +			reg = <0x01c18000 0x1000>;
>> +			clocks = <&ccu CLK_BUS_SPINLOCK>;
>> +			resets = <&ccu RST_BUS_SPINLOCK>;
>> +			#hwlock-cells = <1>;
>> +			status = "disabled";
> 
> Is there a reason to disable it?

I wondered about that - on the one hand we can safely leave it always
enabled, on the other hand all devices are are disabled in the dtsi.

> 
> Also, my understanding was that hwspinlocks were meant to be used by
> client drivers, so surely we must tie them to other device nodes too,
> right?

Perhaps but at this point I'm not sure to which device specifically.

> 
> Maxime
>
Maxime Ripard Feb. 11, 2020, 12:36 p.m. UTC | #3
On Tue, Feb 11, 2020 at 10:09:48AM +0200, Nikolay Borisov wrote:
>
>
> On 11.02.20 г. 9:55 ч., Maxime Ripard wrote:
> > Hi,
> >
> > On Mon, Feb 10, 2020 at 07:01:42PM +0200, Nikolay Borisov wrote:
> >> Add a node describing the hwspinlock on A64.
> >>
> >> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> >> ---
> >>  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 9 +++++++++
> >>  1 file changed, 9 insertions(+)
> >>
> >> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> >> index 27e48234f1c2..01de89fc09cc 100644
> >> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> >> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> >> @@ -1182,5 +1182,14 @@
> >>  			interrupts = <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
> >>  			clocks = <&osc24M>;
> >>  		};
> >> +
> >> +		hwspinlock: spinlock@1c18000 {
> >
> > Nodes are supposed to be ordered by ascending physical address
> >
> >> +			compatible = "allwinner,sun50i-a64-hwspinlock";
> >> +			reg = <0x01c18000 0x1000>;
> >> +			clocks = <&ccu CLK_BUS_SPINLOCK>;
> >> +			resets = <&ccu RST_BUS_SPINLOCK>;
> >> +			#hwlock-cells = <1>;
> >> +			status = "disabled";
> >
> > Is there a reason to disable it?
>
> I wondered about that - on the one hand we can safely leave it always
> enabled, on the other hand all devices are are disabled in the dtsi.

Not all of them, only the one that are connected to external pins. The
PMU or GPU aren't for example.

> > Also, my understanding was that hwspinlocks were meant to be used by
> > client drivers, so surely we must tie them to other device nodes too,
> > right?
>
> Perhaps but at this point I'm not sure to which device specifically.

All the one that are shared with the ARISC core and needs some
synchronisation with the firmware running there.

Maxime
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
index 27e48234f1c2..01de89fc09cc 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
@@ -1182,5 +1182,14 @@ 
 			interrupts = <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&osc24M>;
 		};
+
+		hwspinlock: spinlock@1c18000 {
+			compatible = "allwinner,sun50i-a64-hwspinlock";
+			reg = <0x01c18000 0x1000>;
+			clocks = <&ccu CLK_BUS_SPINLOCK>;
+			resets = <&ccu RST_BUS_SPINLOCK>;
+			#hwlock-cells = <1>;
+			status = "disabled";
+		};
 	};
 };