| Message ID | 1365775405-115297-4-git-send-email-arnd@arndb.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 12 April 2013 19:33, Arnd Bergmann <arnd@arndb.de> wrote: > The exynos combiner irqchip needs to find the parent interrupts > and needs to know their number, so add the missing properties > for exynos4 as they were already present for exynos5. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > arch/arm/boot/dts/exynos4210.dtsi | 1 + > arch/arm/boot/dts/exynos4212.dtsi | 9 +++++++++ > arch/arm/boot/dts/exynos4412.dtsi | 9 +++++++++ > 3 files changed, 19 insertions(+) > > diff --git a/arch/arm/boot/dts/exynos4210.dtsi b/arch/arm/boot/dts/exynos4210.dtsi > index 15143bd..c102869 100644 > --- a/arch/arm/boot/dts/exynos4210.dtsi > +++ b/arch/arm/boot/dts/exynos4210.dtsi > @@ -41,6 +41,7 @@ > }; > > combiner:interrupt-controller@10440000 { > + combiner-nr = <16>; The default value of the combiner-nr is case it is not specified is 16. The default value is documented in the bindings documentation. > interrupts = <0 0 0>, <0 1 0>, <0 2 0>, <0 3 0>, > <0 4 0>, <0 5 0>, <0 6 0>, <0 7 0>, > <0 8 0>, <0 9 0>, <0 10 0>, <0 11 0>, > diff --git a/arch/arm/boot/dts/exynos4212.dtsi b/arch/arm/boot/dts/exynos4212.dtsi > index 36d4299..f83c3c1 100644 > --- a/arch/arm/boot/dts/exynos4212.dtsi > +++ b/arch/arm/boot/dts/exynos4212.dtsi > @@ -26,6 +26,15 @@ > cpu-offset = <0x8000>; > }; > > + interrupt-controller@10440000 { > + combiner-nr = <18>; > + interrupts = <0 0 0>, <0 1 0>, <0 2 0>, <0 3 0>, > + <0 4 0>, <0 5 0>, <0 6 0>, <0 7 0>, > + <0 8 0>, <0 9 0>, <0 10 0>, <0 11 0>, > + <0 12 0>, <0 13 0>, <0 14 0>, <0 15 0>, > + <0 107 0>, <0 108 0>; > + }; > + > mct@10050000 { > compatible = "samsung,exynos4412-mct"; > reg = <0x10050000 0x800>; > diff --git a/arch/arm/boot/dts/exynos4412.dtsi b/arch/arm/boot/dts/exynos4412.dtsi > index d75c047..4cb657e 100644 > --- a/arch/arm/boot/dts/exynos4412.dtsi > +++ b/arch/arm/boot/dts/exynos4412.dtsi > @@ -26,6 +26,15 @@ > cpu-offset = <0x4000>; > }; > > + interrupt-controller@10440000 { > + combiner-nr = <20>; > + interrupts = <0 0 0>, <0 1 0>, <0 2 0>, <0 3 0>, > + <0 4 0>, <0 5 0>, <0 6 0>, <0 7 0>, > + <0 8 0>, <0 9 0>, <0 10 0>, <0 11 0>, > + <0 12 0>, <0 13 0>, <0 14 0>, <0 15 0>, > + <0 107 0>, <0 108 0>>, <0 48 0>, <0 42 0>; Extra '>' for <0 108 0> can be removed. Reviewed-by: Thomas Abraham <thomas.abraham@linaro.org? > + }; > + > mct@10050000 { > compatible = "samsung,exynos4412-mct"; > reg = <0x10050000 0x800>; > -- > 1.8.1.2 > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 12 April 2013 19:33, Arnd Bergmann <arnd@arndb.de> wrote: > The exynos combiner irqchip needs to find the parent interrupts > and needs to know their number, so add the missing properties > for exynos4 as they were already present for exynos5. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > arch/arm/boot/dts/exynos4210.dtsi | 1 + > arch/arm/boot/dts/exynos4212.dtsi | 9 +++++++++ > arch/arm/boot/dts/exynos4412.dtsi | 9 +++++++++ > 3 files changed, 19 insertions(+) > > diff --git a/arch/arm/boot/dts/exynos4210.dtsi b/arch/arm/boot/dts/exynos4210.dtsi > index 15143bd..c102869 100644 > --- a/arch/arm/boot/dts/exynos4210.dtsi > +++ b/arch/arm/boot/dts/exynos4210.dtsi > @@ -41,6 +41,7 @@ > }; > > combiner:interrupt-controller@10440000 { > + combiner-nr = <16>; I think this should be "samsung,combiner-nr" Without this i see the following message in boot log: "combiner_of_init: number of combiners not specified" > interrupts = <0 0 0>, <0 1 0>, <0 2 0>, <0 3 0>, > <0 4 0>, <0 5 0>, <0 6 0>, <0 7 0>, > <0 8 0>, <0 9 0>, <0 10 0>, <0 11 0>, > diff --git a/arch/arm/boot/dts/exynos4212.dtsi b/arch/arm/boot/dts/exynos4212.dtsi > index 36d4299..f83c3c1 100644 > --- a/arch/arm/boot/dts/exynos4212.dtsi > +++ b/arch/arm/boot/dts/exynos4212.dtsi > @@ -26,6 +26,15 @@ > cpu-offset = <0x8000>; > }; > > + interrupt-controller@10440000 { Don't we need a node name here (combiner:interrupt-controller@10440000)? > + combiner-nr = <18>; ^^^^^^^^^^^ nit: tabs instead of space would look better. > + interrupts = <0 0 0>, <0 1 0>, <0 2 0>, <0 3 0>, > + <0 4 0>, <0 5 0>, <0 6 0>, <0 7 0>, > + <0 8 0>, <0 9 0>, <0 10 0>, <0 11 0>, > + <0 12 0>, <0 13 0>, <0 14 0>, <0 15 0>, > + <0 107 0>, <0 108 0>; > + }; > + > mct@10050000 { > compatible = "samsung,exynos4412-mct"; > reg = <0x10050000 0x800>; > diff --git a/arch/arm/boot/dts/exynos4412.dtsi b/arch/arm/boot/dts/exynos4412.dtsi > index d75c047..4cb657e 100644 > --- a/arch/arm/boot/dts/exynos4412.dtsi > +++ b/arch/arm/boot/dts/exynos4412.dtsi > @@ -26,6 +26,15 @@ > cpu-offset = <0x4000>; > }; > > + interrupt-controller@10440000 { > + combiner-nr = <20>; Same as above (for exynos4212.dtsi). > + interrupts = <0 0 0>, <0 1 0>, <0 2 0>, <0 3 0>, > + <0 4 0>, <0 5 0>, <0 6 0>, <0 7 0>, > + <0 8 0>, <0 9 0>, <0 10 0>, <0 11 0>, > + <0 12 0>, <0 13 0>, <0 14 0>, <0 15 0>, > + <0 107 0>, <0 108 0>>, <0 48 0>, <0 42 0>; ^^^ Syntax error. We have a combiner node defined in exynos4x12.dtsi. With the bindings now defined separately in 4212 and 4412 dtsi files, probably the one in 4x12 could be dropped?
On Monday 15 April 2013, Thomas Abraham wrote: > On 12 April 2013 19:33, Arnd Bergmann <arnd@arndb.de> wrote: > > The exynos combiner irqchip needs to find the parent interrupts > > and needs to know their number, so add the missing properties > > for exynos4 as they were already present for exynos5. > > > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > > --- > > arch/arm/boot/dts/exynos4210.dtsi | 1 + > > arch/arm/boot/dts/exynos4212.dtsi | 9 +++++++++ > > arch/arm/boot/dts/exynos4412.dtsi | 9 +++++++++ > > 3 files changed, 19 insertions(+) > > > > diff --git a/arch/arm/boot/dts/exynos4210.dtsi b/arch/arm/boot/dts/exynos4210.dtsi > > index 15143bd..c102869 100644 > > --- a/arch/arm/boot/dts/exynos4210.dtsi > > +++ b/arch/arm/boot/dts/exynos4210.dtsi > > @@ -41,6 +41,7 @@ > > }; > > > > combiner:interrupt-controller@10440000 { > > + combiner-nr = <16>; > > The default value of the combiner-nr is case it is not specified is > 16. The default value is documented in the bindings documentation. Unfortunately, the driver now warns about a missing attribute, which seems right. I also made the default "20" instead of "16" to cover the other exynos4 chips. I'd say it's best to keep this one explicitly listed even if it's optional. > > + interrupt-controller@10440000 { > > + combiner-nr = <20>; > > + interrupts = <0 0 0>, <0 1 0>, <0 2 0>, <0 3 0>, > > + <0 4 0>, <0 5 0>, <0 6 0>, <0 7 0>, > > + <0 8 0>, <0 9 0>, <0 10 0>, <0 11 0>, > > + <0 12 0>, <0 13 0>, <0 14 0>, <0 15 0>, > > + <0 107 0>, <0 108 0>>, <0 48 0>, <0 42 0>; > > Extra '>' for <0 108 0> can be removed. > > Reviewed-by: Thomas Abraham <thomas.abraham@linaro.org? Thanks! -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Monday 15 April 2013, Sachin Kamat wrote: > On 12 April 2013 19:33, Arnd Bergmann <arnd@arndb.de> wrote: > > > > combiner:interrupt-controller@10440000 { > > + combiner-nr = <16>; > > I think this should be "samsung,combiner-nr" > Without this i see the following message in boot log: > "combiner_of_init: number of combiners not specified" Right, thanks! > > diff --git a/arch/arm/boot/dts/exynos4212.dtsi b/arch/arm/boot/dts/exynos4212.dtsi > > index 36d4299..f83c3c1 100644 > > --- a/arch/arm/boot/dts/exynos4212.dtsi > > +++ b/arch/arm/boot/dts/exynos4212.dtsi > > @@ -26,6 +26,15 @@ > > cpu-offset = <0x8000>; > > }; > > > > + interrupt-controller@10440000 { > > Don't we need a node name here (combiner:interrupt-controller@10440000)? Why? The "combiner:" part is just a label. Actually I think the preferred syntax is &combiner { samsung,combiner-nr = <18>; }; which is much shorter to write, but I did not want to change the style used in the rest of the file. > > + combiner-nr = <18>; > ^^^^^^^^^^^ > nit: tabs instead of space would look better. I fixed that up locally, but I think I sent out the wrong version. > > diff --git a/arch/arm/boot/dts/exynos4412.dtsi b/arch/arm/boot/dts/exynos4412.dtsi > > index d75c047..4cb657e 100644 > > --- a/arch/arm/boot/dts/exynos4412.dtsi > > +++ b/arch/arm/boot/dts/exynos4412.dtsi > > @@ -26,6 +26,15 @@ > > cpu-offset = <0x4000>; > > }; > > > > + interrupt-controller@10440000 { > > + combiner-nr = <20>; > Same as above (for exynos4212.dtsi). > > > + interrupts = <0 0 0>, <0 1 0>, <0 2 0>, <0 3 0>, > > + <0 4 0>, <0 5 0>, <0 6 0>, <0 7 0>, > > + <0 8 0>, <0 9 0>, <0 10 0>, <0 11 0>, > > + <0 12 0>, <0 13 0>, <0 14 0>, <0 15 0>, > > + <0 107 0>, <0 108 0>>, <0 48 0>, <0 42 0>; > ^^^ > Syntax error. > > We have a combiner node defined in exynos4x12.dtsi. With the bindings > now defined separately in 4212 and 4412 dtsi files, probably the one > in 4x12 could be dropped? I did not see that one, but it seems to have the wrong numbers in the last four interrupt specifiers. I think it would be better to just fix that one and keep using it. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
>> > diff --git a/arch/arm/boot/dts/exynos4212.dtsi b/arch/arm/boot/dts/exynos4212.dtsi >> > index 36d4299..f83c3c1 100644 >> > --- a/arch/arm/boot/dts/exynos4212.dtsi >> > +++ b/arch/arm/boot/dts/exynos4212.dtsi >> > @@ -26,6 +26,15 @@ >> > cpu-offset = <0x8000>; >> > }; >> > >> > + interrupt-controller@10440000 { >> >> Don't we need a node name here (combiner:interrupt-controller@10440000)? > > Why? Since it is being referenced as "&combiner" by other bindings, i thought that was required. The "combiner:" part is just a label. Actually I think the preferred > syntax is > > &combiner { > samsung,combiner-nr = <18>; > }; > > which is much shorter to write, but I did not want to change the style used > in the rest of the file. > >> > + combiner-nr = <18>; >> ^^^^^^^^^^^ >> nit: tabs instead of space would look better. > > I fixed that up locally, but I think I sent out the wrong version. OK. > >> > diff --git a/arch/arm/boot/dts/exynos4412.dtsi b/arch/arm/boot/dts/exynos4412.dtsi >> > index d75c047..4cb657e 100644 >> > --- a/arch/arm/boot/dts/exynos4412.dtsi >> > +++ b/arch/arm/boot/dts/exynos4412.dtsi >> > @@ -26,6 +26,15 @@ >> > cpu-offset = <0x4000>; >> > }; >> > >> > + interrupt-controller@10440000 { >> > + combiner-nr = <20>; >> Same as above (for exynos4212.dtsi). >> >> > + interrupts = <0 0 0>, <0 1 0>, <0 2 0>, <0 3 0>, >> > + <0 4 0>, <0 5 0>, <0 6 0>, <0 7 0>, >> > + <0 8 0>, <0 9 0>, <0 10 0>, <0 11 0>, >> > + <0 12 0>, <0 13 0>, <0 14 0>, <0 15 0>, >> > + <0 107 0>, <0 108 0>>, <0 48 0>, <0 42 0>; >> ^^^ >> Syntax error. >> >> We have a combiner node defined in exynos4x12.dtsi. With the bindings >> now defined separately in 4212 and 4412 dtsi files, probably the one >> in 4x12 could be dropped? > > I did not see that one, but it seems to have the wrong numbers in the last > four interrupt specifiers. I think it would be better to just fix that one > and keep using it. OK. Do you want me to fix it and send as a patch or would you like to do it?
On Monday 15 April 2013, Sachin Kamat wrote: > >> > diff --git a/arch/arm/boot/dts/exynos4212.dtsi b/arch/arm/boot/dts/exynos4212.dtsi > >> > index 36d4299..f83c3c1 100644 > >> > --- a/arch/arm/boot/dts/exynos4212.dtsi > >> > +++ b/arch/arm/boot/dts/exynos4212.dtsi > >> > @@ -26,6 +26,15 @@ > >> > cpu-offset = <0x8000>; > >> > }; > >> > > >> > + interrupt-controller@10440000 { > >> > >> Don't we need a node name here (combiner:interrupt-controller@10440000)? > > > > Why? > > Since it is being referenced as "&combiner" by other bindings, i > thought that was required. The original definition actually comes from exynos4.dtsi. It's enough to have the label in one place. > >> We have a combiner node defined in exynos4x12.dtsi. With the bindings > >> now defined separately in 4212 and 4412 dtsi files, probably the one > >> in 4x12 could be dropped? > > > > I did not see that one, but it seems to have the wrong numbers in the last > > four interrupt specifiers. I think it would be better to just fix that one > > and keep using it. > > OK. Do you want me to fix it and send as a patch or would you like to do it? If you have a patch that works, you could just send it out, oetherwise I'd do it. ARnd -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 15 April 2013 17:50, Arnd Bergmann <arnd@arndb.de> wrote: > On Monday 15 April 2013, Sachin Kamat wrote: >> >> > diff --git a/arch/arm/boot/dts/exynos4212.dtsi b/arch/arm/boot/dts/exynos4212.dtsi >> >> > index 36d4299..f83c3c1 100644 >> >> > --- a/arch/arm/boot/dts/exynos4212.dtsi >> >> > +++ b/arch/arm/boot/dts/exynos4212.dtsi >> >> > @@ -26,6 +26,15 @@ >> >> > cpu-offset = <0x8000>; >> >> > }; >> >> > >> >> > + interrupt-controller@10440000 { >> >> >> >> Don't we need a node name here (combiner:interrupt-controller@10440000)? >> > >> > Why? >> >> Since it is being referenced as "&combiner" by other bindings, i >> thought that was required. > > The original definition actually comes from exynos4.dtsi. It's enough > to have the label in one place. Yes, right. I overlooked that. > >> >> We have a combiner node defined in exynos4x12.dtsi. With the bindings >> >> now defined separately in 4212 and 4412 dtsi files, probably the one >> >> in 4x12 could be dropped? >> > >> > I did not see that one, but it seems to have the wrong numbers in the last >> > four interrupt specifiers. I think it would be better to just fix that one >> > and keep using it. >> >> OK. Do you want me to fix it and send as a patch or would you like to do it? > > If you have a patch that works, you could just send it out, oetherwise I'd > do it. Not ATM, so please go ahead.
diff --git a/arch/arm/boot/dts/exynos4210.dtsi b/arch/arm/boot/dts/exynos4210.dtsi index 15143bd..c102869 100644 --- a/arch/arm/boot/dts/exynos4210.dtsi +++ b/arch/arm/boot/dts/exynos4210.dtsi @@ -41,6 +41,7 @@ }; combiner:interrupt-controller@10440000 { + combiner-nr = <16>; interrupts = <0 0 0>, <0 1 0>, <0 2 0>, <0 3 0>, <0 4 0>, <0 5 0>, <0 6 0>, <0 7 0>, <0 8 0>, <0 9 0>, <0 10 0>, <0 11 0>, diff --git a/arch/arm/boot/dts/exynos4212.dtsi b/arch/arm/boot/dts/exynos4212.dtsi index 36d4299..f83c3c1 100644 --- a/arch/arm/boot/dts/exynos4212.dtsi +++ b/arch/arm/boot/dts/exynos4212.dtsi @@ -26,6 +26,15 @@ cpu-offset = <0x8000>; }; + interrupt-controller@10440000 { + combiner-nr = <18>; + interrupts = <0 0 0>, <0 1 0>, <0 2 0>, <0 3 0>, + <0 4 0>, <0 5 0>, <0 6 0>, <0 7 0>, + <0 8 0>, <0 9 0>, <0 10 0>, <0 11 0>, + <0 12 0>, <0 13 0>, <0 14 0>, <0 15 0>, + <0 107 0>, <0 108 0>; + }; + mct@10050000 { compatible = "samsung,exynos4412-mct"; reg = <0x10050000 0x800>; diff --git a/arch/arm/boot/dts/exynos4412.dtsi b/arch/arm/boot/dts/exynos4412.dtsi index d75c047..4cb657e 100644 --- a/arch/arm/boot/dts/exynos4412.dtsi +++ b/arch/arm/boot/dts/exynos4412.dtsi @@ -26,6 +26,15 @@ cpu-offset = <0x4000>; }; + interrupt-controller@10440000 { + combiner-nr = <20>; + interrupts = <0 0 0>, <0 1 0>, <0 2 0>, <0 3 0>, + <0 4 0>, <0 5 0>, <0 6 0>, <0 7 0>, + <0 8 0>, <0 9 0>, <0 10 0>, <0 11 0>, + <0 12 0>, <0 13 0>, <0 14 0>, <0 15 0>, + <0 107 0>, <0 108 0>>, <0 48 0>, <0 42 0>; + }; + mct@10050000 { compatible = "samsung,exynos4412-mct"; reg = <0x10050000 0x800>;
The exynos combiner irqchip needs to find the parent interrupts and needs to know their number, so add the missing properties for exynos4 as they were already present for exynos5. Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- arch/arm/boot/dts/exynos4210.dtsi | 1 + arch/arm/boot/dts/exynos4212.dtsi | 9 +++++++++ arch/arm/boot/dts/exynos4412.dtsi | 9 +++++++++ 3 files changed, 19 insertions(+)