| Message ID | 1562298766-25066-1-git-send-email-bmeng.cn@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | riscv: dts: fu540-c000: Add "status" property to cpu node | expand |
> -----Original Message----- > From: linux-riscv <linux-riscv-bounces@lists.infradead.org> On Behalf Of Bin > Meng > Sent: Friday, July 5, 2019 9:23 AM > To: linux-riscv <linux-riscv@lists.infradead.org>; devicetree > <devicetree@vger.kernel.org>; Rob Herring <robh+dt@kernel.org>; Mark > Rutland <mark.rutland@arm.com>; Albert Ou <aou@eecs.berkeley.edu>; > Paul Walmsley <paul.walmsley@sifive.com>; Palmer Dabbelt > <palmer@sifive.com>; Yash Shah <yash.shah@sifive.com> > Subject: [PATCH] riscv: dts: fu540-c000: Add "status" property to cpu node > > Per device tree spec, the "status" property property shall be present for > nodes representing CPUs in a SMP configuration. This property is currently > missing in cpu 1/2/3/4 node in the fu540-c000.dtsi. We don't need explicit "status = okay" for SOC internal devices (such as PLIC, INTC, etc) which are always enabled by default. Absence of "status" DT prop is treated as enabled by default. Regards, Anup > > Signed-off-by: Bin Meng <bmeng.cn@gmail.com> > --- > > arch/riscv/boot/dts/sifive/fu540-c000.dtsi | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/arch/riscv/boot/dts/sifive/fu540-c000.dtsi > b/arch/riscv/boot/dts/sifive/fu540-c000.dtsi > index 4098349..0fff2a4 100644 > --- a/arch/riscv/boot/dts/sifive/fu540-c000.dtsi > +++ b/arch/riscv/boot/dts/sifive/fu540-c000.dtsi > @@ -53,6 +53,7 @@ > mmu-type = "riscv,sv39"; > reg = <1>; > riscv,isa = "rv64imafdc"; > + status = "okay"; > tlb-split; > cpu1_intc: interrupt-controller { > #interrupt-cells = <1>; > @@ -77,6 +78,7 @@ > mmu-type = "riscv,sv39"; > reg = <2>; > riscv,isa = "rv64imafdc"; > + status = "okay"; > tlb-split; > cpu2_intc: interrupt-controller { > #interrupt-cells = <1>; > @@ -101,6 +103,7 @@ > mmu-type = "riscv,sv39"; > reg = <3>; > riscv,isa = "rv64imafdc"; > + status = "okay"; > tlb-split; > cpu3_intc: interrupt-controller { > #interrupt-cells = <1>; > @@ -125,6 +128,7 @@ > mmu-type = "riscv,sv39"; > reg = <4>; > riscv,isa = "rv64imafdc"; > + status = "okay"; > tlb-split; > cpu4_intc: interrupt-controller { > #interrupt-cells = <1>; > -- > 2.7.4 > > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv
On Fri, Jul 5, 2019 at 11:59 AM Anup Patel <Anup.Patel@wdc.com> wrote: > > > > > -----Original Message----- > > From: linux-riscv <linux-riscv-bounces@lists.infradead.org> On Behalf Of Bin > > Meng > > Sent: Friday, July 5, 2019 9:23 AM > > To: linux-riscv <linux-riscv@lists.infradead.org>; devicetree > > <devicetree@vger.kernel.org>; Rob Herring <robh+dt@kernel.org>; Mark > > Rutland <mark.rutland@arm.com>; Albert Ou <aou@eecs.berkeley.edu>; > > Paul Walmsley <paul.walmsley@sifive.com>; Palmer Dabbelt > > <palmer@sifive.com>; Yash Shah <yash.shah@sifive.com> > > Subject: [PATCH] riscv: dts: fu540-c000: Add "status" property to cpu node > > > > Per device tree spec, the "status" property property shall be present for > > nodes representing CPUs in a SMP configuration. This property is currently > > missing in cpu 1/2/3/4 node in the fu540-c000.dtsi. > > We don't need explicit "status = okay" for SOC internal devices > (such as PLIC, INTC, etc) which are always enabled by default. > Yes, that's fine because those device bindings do not require them. > Absence of "status" DT prop is treated as enabled by default. > But per current device tree spec, "status" in cpu node is mandatory. (spec uses "shall"). Missing it is a spec violation. Regards, Bin
On Fri, Jul 5, 2019 at 1:11 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > On Fri, Jul 5, 2019 at 11:59 AM Anup Patel <Anup.Patel@wdc.com> wrote: > > > > > > > > > -----Original Message----- > > > From: linux-riscv <linux-riscv-bounces@lists.infradead.org> On Behalf Of Bin > > > Meng > > > Sent: Friday, July 5, 2019 9:23 AM > > > To: linux-riscv <linux-riscv@lists.infradead.org>; devicetree > > > <devicetree@vger.kernel.org>; Rob Herring <robh+dt@kernel.org>; Mark > > > Rutland <mark.rutland@arm.com>; Albert Ou <aou@eecs.berkeley.edu>; > > > Paul Walmsley <paul.walmsley@sifive.com>; Palmer Dabbelt > > > <palmer@sifive.com>; Yash Shah <yash.shah@sifive.com> > > > Subject: [PATCH] riscv: dts: fu540-c000: Add "status" property to cpu node > > > > > > Per device tree spec, the "status" property property shall be present for > > > nodes representing CPUs in a SMP configuration. This property is currently > > > missing in cpu 1/2/3/4 node in the fu540-c000.dtsi. > > > > We don't need explicit "status = okay" for SOC internal devices > > (such as PLIC, INTC, etc) which are always enabled by default. > > > > Yes, that's fine because those device bindings do not require them. > > > Absence of "status" DT prop is treated as enabled by default. > > > > But per current device tree spec, "status" in cpu node is mandatory. > (spec uses "shall"). Missing it is a spec violation. Ping? Regards, Bin
> -----Original Message----- > From: Bin Meng <bmeng.cn@gmail.com> > Sent: Monday, July 22, 2019 1:32 PM > To: Anup Patel <Anup.Patel@wdc.com> > Cc: linux-riscv <linux-riscv@lists.infradead.org>; devicetree > <devicetree@vger.kernel.org>; Rob Herring <robh+dt@kernel.org>; Mark > Rutland <mark.rutland@arm.com>; Albert Ou <aou@eecs.berkeley.edu>; > Paul Walmsley <paul.walmsley@sifive.com>; Palmer Dabbelt > <palmer@sifive.com>; Yash Shah <yash.shah@sifive.com> > Subject: Re: [PATCH] riscv: dts: fu540-c000: Add "status" property to cpu > node > > On Fri, Jul 5, 2019 at 1:11 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > On Fri, Jul 5, 2019 at 11:59 AM Anup Patel <Anup.Patel@wdc.com> wrote: > > > > > > > > > > > > > -----Original Message----- > > > > From: linux-riscv <linux-riscv-bounces@lists.infradead.org> On > > > > Behalf Of Bin Meng > > > > Sent: Friday, July 5, 2019 9:23 AM > > > > To: linux-riscv <linux-riscv@lists.infradead.org>; devicetree > > > > <devicetree@vger.kernel.org>; Rob Herring <robh+dt@kernel.org>; > > > > Mark Rutland <mark.rutland@arm.com>; Albert Ou > > > > <aou@eecs.berkeley.edu>; Paul Walmsley > <paul.walmsley@sifive.com>; > > > > Palmer Dabbelt <palmer@sifive.com>; Yash Shah > > > > <yash.shah@sifive.com> > > > > Subject: [PATCH] riscv: dts: fu540-c000: Add "status" property to > > > > cpu node > > > > > > > > Per device tree spec, the "status" property property shall be > > > > present for nodes representing CPUs in a SMP configuration. This > > > > property is currently missing in cpu 1/2/3/4 node in the fu540-c000.dtsi. > > > > > > We don't need explicit "status = okay" for SOC internal devices > > > (such as PLIC, INTC, etc) which are always enabled by default. > > > > > > > Yes, that's fine because those device bindings do not require them. > > > > > Absence of "status" DT prop is treated as enabled by default. > > > > > > > But per current device tree spec, "status" in cpu node is mandatory. > > (spec uses "shall"). Missing it is a spec violation. > > Ping? I am fine with explicit status = "okay". I am hoping DT maintainers will share there views on this. Regards, Anup
On Fri, Jul 05, 2019 at 01:11:01PM +0800, Bin Meng wrote: > On Fri, Jul 5, 2019 at 11:59 AM Anup Patel <Anup.Patel@wdc.com> wrote: > > > > > > > > > -----Original Message----- > > > From: linux-riscv <linux-riscv-bounces@lists.infradead.org> On Behalf Of Bin > > > Meng > > > Sent: Friday, July 5, 2019 9:23 AM > > > To: linux-riscv <linux-riscv@lists.infradead.org>; devicetree > > > <devicetree@vger.kernel.org>; Rob Herring <robh+dt@kernel.org>; Mark > > > Rutland <mark.rutland@arm.com>; Albert Ou <aou@eecs.berkeley.edu>; > > > Paul Walmsley <paul.walmsley@sifive.com>; Palmer Dabbelt > > > <palmer@sifive.com>; Yash Shah <yash.shah@sifive.com> > > > Subject: [PATCH] riscv: dts: fu540-c000: Add "status" property to cpu node > > > > > > Per device tree spec, the "status" property property shall be present for > > > nodes representing CPUs in a SMP configuration. This property is currently > > > missing in cpu 1/2/3/4 node in the fu540-c000.dtsi. > > > > We don't need explicit "status = okay" for SOC internal devices > > (such as PLIC, INTC, etc) which are always enabled by default. > > > > Yes, that's fine because those device bindings do not require them. > > > Absence of "status" DT prop is treated as enabled by default. > > > > But per current device tree spec, "status" in cpu node is mandatory. > (spec uses "shall"). Missing it is a spec violation. I think this is a spec bug (or at least misleading wording in the spec). IEEE 1275 says (for status as a generic property): The absence of this property menas that the operational status is unknown or okay. ... and I think it's fine to treat that the same as an explicit "okay" here, as we do generically in Linux. Thanks, Mark.
Hi Mark, On Mon, Jul 22, 2019 at 4:18 PM Mark Rutland <mark.rutland@arm.com> wrote: > > On Fri, Jul 05, 2019 at 01:11:01PM +0800, Bin Meng wrote: > > On Fri, Jul 5, 2019 at 11:59 AM Anup Patel <Anup.Patel@wdc.com> wrote: > > > > > > > > > > > > > -----Original Message----- > > > > From: linux-riscv <linux-riscv-bounces@lists.infradead.org> On Behalf Of Bin > > > > Meng > > > > Sent: Friday, July 5, 2019 9:23 AM > > > > To: linux-riscv <linux-riscv@lists.infradead.org>; devicetree > > > > <devicetree@vger.kernel.org>; Rob Herring <robh+dt@kernel.org>; Mark > > > > Rutland <mark.rutland@arm.com>; Albert Ou <aou@eecs.berkeley.edu>; > > > > Paul Walmsley <paul.walmsley@sifive.com>; Palmer Dabbelt > > > > <palmer@sifive.com>; Yash Shah <yash.shah@sifive.com> > > > > Subject: [PATCH] riscv: dts: fu540-c000: Add "status" property to cpu node > > > > > > > > Per device tree spec, the "status" property property shall be present for > > > > nodes representing CPUs in a SMP configuration. This property is currently > > > > missing in cpu 1/2/3/4 node in the fu540-c000.dtsi. > > > > > > We don't need explicit "status = okay" for SOC internal devices > > > (such as PLIC, INTC, etc) which are always enabled by default. > > > > > > > Yes, that's fine because those device bindings do not require them. > > > > > Absence of "status" DT prop is treated as enabled by default. > > > > > > > But per current device tree spec, "status" in cpu node is mandatory. > > (spec uses "shall"). Missing it is a spec violation. > > I think this is a spec bug (or at least misleading wording in the spec). > > IEEE 1275 says (for status as a generic property): > > The absence of this property menas that the operational status is unknown or > okay. Yes, I checked IEEE 1275 doc, and it indeed says like you mentioned. However, it says "unknown" _or_ "okay", yet provides a definite value. > > ... and I think it's fine to treat that the same as an explicit "okay" here, as > we do generically in Linux. So what Linux does is a defacto interpretation? If everyone agrees this is a device tree spec bug, I will submit the patch to devicetree spec then. Regards, Bin
On Mon, Jul 22, 2019 at 9:35 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > Hi Mark, > > On Mon, Jul 22, 2019 at 4:18 PM Mark Rutland <mark.rutland@arm.com> wrote: > > > > On Fri, Jul 05, 2019 at 01:11:01PM +0800, Bin Meng wrote: > > > On Fri, Jul 5, 2019 at 11:59 AM Anup Patel <Anup.Patel@wdc.com> wrote: > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > From: linux-riscv <linux-riscv-bounces@lists.infradead.org> On Behalf Of Bin > > > > > Meng > > > > > Sent: Friday, July 5, 2019 9:23 AM > > > > > To: linux-riscv <linux-riscv@lists.infradead.org>; devicetree > > > > > <devicetree@vger.kernel.org>; Rob Herring <robh+dt@kernel.org>; Mark > > > > > Rutland <mark.rutland@arm.com>; Albert Ou <aou@eecs.berkeley.edu>; > > > > > Paul Walmsley <paul.walmsley@sifive.com>; Palmer Dabbelt > > > > > <palmer@sifive.com>; Yash Shah <yash.shah@sifive.com> > > > > > Subject: [PATCH] riscv: dts: fu540-c000: Add "status" property to cpu node > > > > > > > > > > Per device tree spec, the "status" property property shall be present for > > > > > nodes representing CPUs in a SMP configuration. This property is currently > > > > > missing in cpu 1/2/3/4 node in the fu540-c000.dtsi. > > > > > > > > We don't need explicit "status = okay" for SOC internal devices > > > > (such as PLIC, INTC, etc) which are always enabled by default. > > > > > > > > > > Yes, that's fine because those device bindings do not require them. > > > > > > > Absence of "status" DT prop is treated as enabled by default. > > > > > > > > > > But per current device tree spec, "status" in cpu node is mandatory. > > > (spec uses "shall"). Missing it is a spec violation. > > > > I think this is a spec bug (or at least misleading wording in the spec). > > > > IEEE 1275 says (for status as a generic property): > > > > The absence of this property menas that the operational status is unknown or > > okay. > > Yes, I checked IEEE 1275 doc, and it indeed says like you mentioned. > > However, it says "unknown" _or_ "okay", yet provides a definite value. > > > > > ... and I think it's fine to treat that the same as an explicit "okay" here, as > > we do generically in Linux. > > So what Linux does is a defacto interpretation? > > If everyone agrees this is a device tree spec bug, I will submit the > patch to devicetree spec then. Any comments from the device tree folks? thanks! Regards, Bin
diff --git a/arch/riscv/boot/dts/sifive/fu540-c000.dtsi b/arch/riscv/boot/dts/sifive/fu540-c000.dtsi index 4098349..0fff2a4 100644 --- a/arch/riscv/boot/dts/sifive/fu540-c000.dtsi +++ b/arch/riscv/boot/dts/sifive/fu540-c000.dtsi @@ -53,6 +53,7 @@ mmu-type = "riscv,sv39"; reg = <1>; riscv,isa = "rv64imafdc"; + status = "okay"; tlb-split; cpu1_intc: interrupt-controller { #interrupt-cells = <1>; @@ -77,6 +78,7 @@ mmu-type = "riscv,sv39"; reg = <2>; riscv,isa = "rv64imafdc"; + status = "okay"; tlb-split; cpu2_intc: interrupt-controller { #interrupt-cells = <1>; @@ -101,6 +103,7 @@ mmu-type = "riscv,sv39"; reg = <3>; riscv,isa = "rv64imafdc"; + status = "okay"; tlb-split; cpu3_intc: interrupt-controller { #interrupt-cells = <1>; @@ -125,6 +128,7 @@ mmu-type = "riscv,sv39"; reg = <4>; riscv,isa = "rv64imafdc"; + status = "okay"; tlb-split; cpu4_intc: interrupt-controller { #interrupt-cells = <1>;
Per device tree spec, the "status" property property shall be present for nodes representing CPUs in a SMP configuration. This property is currently missing in cpu 1/2/3/4 node in the fu540-c000.dtsi. Signed-off-by: Bin Meng <bmeng.cn@gmail.com> --- arch/riscv/boot/dts/sifive/fu540-c000.dtsi | 4 ++++ 1 file changed, 4 insertions(+)