| Message ID | 1366644455-16550-13-git-send-email-lorenzo.pieralisi@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 23/04/13 03:27, Lorenzo Pieralisi wrote: > This patch updates the in-kernel dts files according to the latest cpus > and cpu bindings updates for ARM. > > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > --- > arch/arm/boot/dts/wm8505.dtsi | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/arm/boot/dts/wm8505.dtsi b/arch/arm/boot/dts/wm8505.dtsi > index e74a1c0..a470808 100644 > --- a/arch/arm/boot/dts/wm8505.dtsi > +++ b/arch/arm/boot/dts/wm8505.dtsi > @@ -13,7 +13,7 @@ > > cpus { > cpu@0 { > - compatible = "arm,arm926ejs"; > + compatible = "arm,arm926"; > }; > }; > As the only author for that file, and the arch-vt8500 maintainer, a CC would have been nice :) This binding is still incomplete.device_type, compatible and reg are all required properties. Also, you seem to have only fixed the wm8505.dtsi, while vt8500.dtsi, wm8650.dtsi and wm8850.dtsi are all left incorrect. I am already carrying a patch to fix all this properly so could you drop the arch-vt8500 related patch and I will send in a more complete version for 3.11 Regards Tony Prisk
On 23/04/13 14:13, Tony Prisk wrote: > On 23/04/13 03:27, Lorenzo Pieralisi wrote: >> This patch updates the in-kernel dts files according to the latest cpus >> and cpu bindings updates for ARM. >> >> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> >> --- >> arch/arm/boot/dts/wm8505.dtsi | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/arch/arm/boot/dts/wm8505.dtsi >> b/arch/arm/boot/dts/wm8505.dtsi >> index e74a1c0..a470808 100644 >> --- a/arch/arm/boot/dts/wm8505.dtsi >> +++ b/arch/arm/boot/dts/wm8505.dtsi >> @@ -13,7 +13,7 @@ >> cpus { >> cpu@0 { >> - compatible = "arm,arm926ejs"; >> + compatible = "arm,arm926"; >> }; >> }; > As the only author for that file, and the arch-vt8500 maintainer, a CC > would have been nice :) > > This binding is still incomplete.device_type, compatible and reg are > all required properties. > Also, you seem to have only fixed the wm8505.dtsi, while vt8500.dtsi, > wm8650.dtsi and wm8850.dtsi are all left incorrect. > > I am already carrying a patch to fix all this properly so could you > drop the arch-vt8500 related patch and I will send in a more complete > version for 3.11 > > Regards > Tony Prisk I didn't notice the rules had been changed in the last patch - shouldn't you be changing the rules (read: binding) before changing the devicetree files? Request still stands - please drop this patch and I will submit a more complete one with the other SoCs updated as well. Tony P
On 23/04/13 03:27, Lorenzo Pieralisi wrote: > This patch updates the in-kernel dts files according to the latest cpus > and cpu bindings updates for ARM. > > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > --- > arch/arm/boot/dts/wm8505.dtsi | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/arm/boot/dts/wm8505.dtsi b/arch/arm/boot/dts/wm8505.dtsi > index e74a1c0..a470808 100644 > --- a/arch/arm/boot/dts/wm8505.dtsi > +++ b/arch/arm/boot/dts/wm8505.dtsi > @@ -13,7 +13,7 @@ > > cpus { > cpu@0 { > - compatible = "arm,arm926ejs"; > + compatible = "arm,arm926"; > }; > }; > The more I look at this, the more wrong it is :/ From the new binding documentation, + A cpus node must define the following properties: + + - #address-cells + Usage: required + Value type: <u32> + Definition: must be set to 1 for 32-bit systems and 2 for + 64-bit systems + - #size-cells + Usage: required + Value type: <u32> + Definition: must be set to 0 ... +- cpu node + + Description: Describes a CPU in an ARM based system + + PROPERTIES + + - device_type + Usage: required + Value type: <string> + Definition: must be "cpu" Three required properties that aren't present in the patch. cpus { #size-cells = <0>; #address-cells = <1>; cpu { device_type = "cpu" compatible = "arm,arm926"; }; }; Regards Tony P
On Tue, Apr 23, 2013 at 03:13:18AM +0100, Tony Prisk wrote: > On 23/04/13 03:27, Lorenzo Pieralisi wrote: > > This patch updates the in-kernel dts files according to the latest cpus > > and cpu bindings updates for ARM. > > > > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > > --- > > arch/arm/boot/dts/wm8505.dtsi | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/arch/arm/boot/dts/wm8505.dtsi b/arch/arm/boot/dts/wm8505.dtsi > > index e74a1c0..a470808 100644 > > --- a/arch/arm/boot/dts/wm8505.dtsi > > +++ b/arch/arm/boot/dts/wm8505.dtsi > > @@ -13,7 +13,7 @@ > > > > cpus { > > cpu@0 { > > - compatible = "arm,arm926ejs"; > > + compatible = "arm,arm926"; > > }; > > }; > > > As the only author for that file, and the arch-vt8500 maintainer, a CC > would have been nice :) Apologies. > This binding is still incomplete.device_type, compatible and reg are all > required properties. device_type should already be there and I should not be in charge of adding it; reg is not required for that processor. > Also, you seem to have only fixed the wm8505.dtsi, while vt8500.dtsi, > wm8650.dtsi and wm8850.dtsi are all left incorrect. There is no cpus node in there. I can't add cpus node in all dts files in the kernel where it is missing (why have those dts files been accepted in the first place, with no cpus node ? why ? cpus node is mandatory since device tree was introduced, I really do not understand why those dts landed in the kernel as they are), I would kindly ask maintainers to do that, I just can not know what cpus are there for every given SoC. > I am already carrying a patch to fix all this properly so could you drop > the arch-vt8500 related patch and I will send in a more complete version > for 3.11 Good, thanks. Lorenzo
On Tue, Apr 23, 2013 at 03:20:46AM +0100, Tony Prisk wrote: > On 23/04/13 14:13, Tony Prisk wrote: > > On 23/04/13 03:27, Lorenzo Pieralisi wrote: > >> This patch updates the in-kernel dts files according to the latest cpus > >> and cpu bindings updates for ARM. > >> > >> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > >> --- > >> arch/arm/boot/dts/wm8505.dtsi | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/arch/arm/boot/dts/wm8505.dtsi > >> b/arch/arm/boot/dts/wm8505.dtsi > >> index e74a1c0..a470808 100644 > >> --- a/arch/arm/boot/dts/wm8505.dtsi > >> +++ b/arch/arm/boot/dts/wm8505.dtsi > >> @@ -13,7 +13,7 @@ > >> cpus { > >> cpu@0 { > >> - compatible = "arm,arm926ejs"; > >> + compatible = "arm,arm926"; > >> }; > >> }; > > As the only author for that file, and the arch-vt8500 maintainer, a CC > > would have been nice :) > > > > This binding is still incomplete.device_type, compatible and reg are > > all required properties. > > Also, you seem to have only fixed the wm8505.dtsi, while vt8500.dtsi, > > wm8650.dtsi and wm8850.dtsi are all left incorrect. > > > > I am already carrying a patch to fix all this properly so could you > > drop the arch-vt8500 related patch and I will send in a more complete > > version for 3.11 > > > > Regards > > Tony Prisk > > I didn't notice the rules had been changed in the last patch - shouldn't > you be changing the rules (read: binding) before changing the devicetree > files? Fair enough, point taken. > Request still stands - please drop this patch and I will submit a more > complete one with the other SoCs updated as well. Great, thanks, Lorenzo
On Tue, Apr 23, 2013 at 03:43:48AM +0100, Tony Prisk wrote: > On 23/04/13 03:27, Lorenzo Pieralisi wrote: > > This patch updates the in-kernel dts files according to the latest cpus > > and cpu bindings updates for ARM. > > > > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > > --- > > arch/arm/boot/dts/wm8505.dtsi | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/arch/arm/boot/dts/wm8505.dtsi b/arch/arm/boot/dts/wm8505.dtsi > > index e74a1c0..a470808 100644 > > --- a/arch/arm/boot/dts/wm8505.dtsi > > +++ b/arch/arm/boot/dts/wm8505.dtsi > > @@ -13,7 +13,7 @@ > > > > cpus { > > cpu@0 { > > - compatible = "arm,arm926ejs"; > > + compatible = "arm,arm926"; > > }; > > }; > > > The more I look at this, the more wrong it is :/ > > From the new binding documentation, > > + A cpus node must define the following properties: > + > + - #address-cells > + Usage: required > + Value type: <u32> > + Definition: must be set to 1 for 32-bit systems and 2 for > + 64-bit systems > + - #size-cells > + Usage: required > + Value type: <u32> > + Definition: must be set to 0 > > ... > > +- cpu node > + > + Description: Describes a CPU in an ARM based system > + > + PROPERTIES > + > + - device_type > + Usage: required > + Value type: <string> > + Definition: must be "cpu" > This property is required since DT was invented, again, I should not be in charge of adding it. > Three required properties that aren't present in the patch. > > cpus { > #size-cells = <0>; > #address-cells = <1>; > > cpu { > device_type = "cpu" > compatible = "arm,arm926"; > }; > }; I am of two minds about this. That processor does not have an MPIDR equivalent so in theory #address-cells and #size-cells could be omitted because the reg property is meaningless. I do not know to be honest the best way to handle this. Thoughts ? Thanks for the review, Lorenzo
diff --git a/arch/arm/boot/dts/wm8505.dtsi b/arch/arm/boot/dts/wm8505.dtsi index e74a1c0..a470808 100644 --- a/arch/arm/boot/dts/wm8505.dtsi +++ b/arch/arm/boot/dts/wm8505.dtsi @@ -13,7 +13,7 @@ cpus { cpu@0 { - compatible = "arm,arm926ejs"; + compatible = "arm,arm926"; }; };
This patch updates the in-kernel dts files according to the latest cpus and cpu bindings updates for ARM. Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> --- arch/arm/boot/dts/wm8505.dtsi | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)