Add generic machine which can handle inits for omap3630.
Currently beagle-xm would matchup with ti,omap3 which invokes
omap3430_init_early instead of omap3630_init_early
This defeats the purpose of having SoC specific init_early handlers,
example: clock nodes are not the same between 3430 and 3630.
Signed-off-by: Nishanth Menon <[email protected]>
---
An alternative approach may be to (for all SoCs):
1. define every SoC entry - ti,omap3430 ti,omap3630...
2. have a generic omap3_init which uses "if (of_machine_is_compatible("ti,omap3630"))"
to invoke the appropriate omap3xxx_init_early.
arch/arm/mach-omap2/board-generic.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/arch/arm/mach-omap2/board-generic.c b/arch/arm/mach-omap2/board-generic.c
index 39c7838..cd85b36 100644
--- a/arch/arm/mach-omap2/board-generic.c
+++ b/arch/arm/mach-omap2/board-generic.c
@@ -147,6 +147,24 @@ DT_MACHINE_START(OMAP3_GP_DT, "Generic OMAP3-GP (Flattened Device Tree)")
.dt_compat = omap3_gp_boards_compat,
.restart = omap3xxx_restart,
MACHINE_END
+
+static const char *omap3630_gp_boards_compat[] __initdata = {
+ "ti,omap3-beagle-xm",
+ NULL,
+};
+
+DT_MACHINE_START(OMAP3630_GP_DT, "Generic OMAP3630-GP (Flattened Device Tree)")
+ .reserve = omap_reserve,
+ .map_io = omap3_map_io,
+ .init_early = omap3630_init_early,
+ .init_irq = omap_intc_of_init,
+ .handle_irq = omap3_intc_handle_irq,
+ .init_machine = omap_generic_init,
+ .init_late = omap3_init_late,
+ .init_time = omap3_secure_sync32k_timer_init,
+ .dt_compat = omap3630_gp_boards_compat,
+ .restart = omap3xxx_restart,
+MACHINE_END
#endif
#ifdef CONFIG_SOC_AM33XX
--
1.7.9.5
Hi,
On Fri, Sep 20, 2013 at 9:08 AM, Nishanth Menon <[email protected]> wrote:
> An alternative approach may be to (for all SoCs):
> 1. define every SoC entry - ti,omap3430 ti,omap3630...
> 2. have a generic omap3_init which uses "if (of_machine_is_compatible("ti,omap3630"))"
> to invoke the appropriate omap3xxx_init_early.
Yes, this would be better, but you can do add a DT_MACHINE as in this
patch but have ti,omap3630 as the dt_compat table. Then there's no
need to add runtime checks.
-Olof
Hi,
On Fri, Sep 20, 2013 at 09:19:02AM -0700, Olof Johansson wrote:
> On Fri, Sep 20, 2013 at 9:08 AM, Nishanth Menon <[email protected]> wrote:
> > An alternative approach may be to (for all SoCs):
> > 1. define every SoC entry - ti,omap3430 ti,omap3630...
> > 2. have a generic omap3_init which uses "if (of_machine_is_compatible("ti,omap3630"))"
> > to invoke the appropriate omap3xxx_init_early.
>
> Yes, this would be better, but you can do add a DT_MACHINE as in this
> patch but have ti,omap3630 as the dt_compat table. Then there's no
> need to add runtime checks.
I was going to reply that adding of_machine_is_compatible("ti,omap3630")
would help in some situations, but guess it's already tainted ;-)
cheers
--
balbi
On Fri, Sep 20, 2013 at 9:23 AM, Felipe Balbi <[email protected]> wrote:
> Hi,
>
> On Fri, Sep 20, 2013 at 09:19:02AM -0700, Olof Johansson wrote:
>> On Fri, Sep 20, 2013 at 9:08 AM, Nishanth Menon <[email protected]> wrote:
>> > An alternative approach may be to (for all SoCs):
>> > 1. define every SoC entry - ti,omap3430 ti,omap3630...
>> > 2. have a generic omap3_init which uses "if (of_machine_is_compatible("ti,omap3630"))"
>> > to invoke the appropriate omap3xxx_init_early.
>>
>> Yes, this would be better, but you can do add a DT_MACHINE as in this
>> patch but have ti,omap3630 as the dt_compat table. Then there's no
>> need to add runtime checks.
>
> I was going to reply that adding of_machine_is_compatible("ti,omap3630")
> would help in some situations, but guess it's already tainted ;-)
Oh, if it's just a few checks, then by all means go down that route. I
didn't look at the code to see how much it would be.
But if a new DT_MACHINE is added, then it should definitely be based
on ti,omap3630 instead of listing all the boards.
-Olof
Hi,
On Fri, Sep 20, 2013 at 10:16:48AM -0700, Olof Johansson wrote:
> > On Fri, Sep 20, 2013 at 09:19:02AM -0700, Olof Johansson wrote:
> >> On Fri, Sep 20, 2013 at 9:08 AM, Nishanth Menon <[email protected]> wrote:
> >> > An alternative approach may be to (for all SoCs):
> >> > 1. define every SoC entry - ti,omap3430 ti,omap3630...
> >> > 2. have a generic omap3_init which uses "if (of_machine_is_compatible("ti,omap3630"))"
> >> > to invoke the appropriate omap3xxx_init_early.
> >>
> >> Yes, this would be better, but you can do add a DT_MACHINE as in this
> >> patch but have ti,omap3630 as the dt_compat table. Then there's no
> >> need to add runtime checks.
> >
> > I was going to reply that adding of_machine_is_compatible("ti,omap3630")
> > would help in some situations, but guess it's already tainted ;-)
>
> Oh, if it's just a few checks, then by all means go down that route. I
> didn't look at the code to see how much it would be.
>
> But if a new DT_MACHINE is added, then it should definitely be based
> on ti,omap3630 instead of listing all the boards.
the idea was to CPU compatible property to conditionally enable known
erratas workarounds. In some cases, Revision register can't be trusted,
so instead of creating per-errata DT properties (since that'd be
describing the SW, in a way), I thought of using
of_machine_is_compatible() checks, but that assumes CPU compatible is
"correct".
cheers
--
balbi
On 09/20/2013 12:42 PM, Felipe Balbi wrote:
> Hi,
>
> On Fri, Sep 20, 2013 at 10:16:48AM -0700, Olof Johansson wrote:
>>> On Fri, Sep 20, 2013 at 09:19:02AM -0700, Olof Johansson wrote:
>>>> On Fri, Sep 20, 2013 at 9:08 AM, Nishanth Menon <[email protected]> wrote:
>>>>> An alternative approach may be to (for all SoCs):
>>>>> 1. define every SoC entry - ti,omap3430 ti,omap3630...
>>>>> 2. have a generic omap3_init which uses "if (of_machine_is_compatible("ti,omap3630"))"
>>>>> to invoke the appropriate omap3xxx_init_early.
>>>>
>>>> Yes, this would be better, but you can do add a DT_MACHINE as in this
>>>> patch but have ti,omap3630 as the dt_compat table. Then there's no
>>>> need to add runtime checks.
>>>
>>> I was going to reply that adding of_machine_is_compatible("ti,omap3630")
>>> would help in some situations, but guess it's already tainted ;-)
>>
>> Oh, if it's just a few checks, then by all means go down that route. I
>> didn't look at the code to see how much it would be.
>>
>> But if a new DT_MACHINE is added, then it should definitely be based
>> on ti,omap3630 instead of listing all the boards.
This was more in the direction I was hoping to go.
>
> the idea was to CPU compatible property to conditionally enable known
> erratas workarounds. In some cases, Revision register can't be trusted,
> so instead of creating per-errata DT properties (since that'd be
> describing the SW, in a way), I thought of using
> of_machine_is_compatible() checks, but that assumes CPU compatible is
> "correct".
I think the quirk handling is part of what Tony is attempting, and the
definition of these might help, I think..
--
Regards,
Nishanth Menon
beagle-xm currently would matchup with ti,omap3 which invokes
omap3430_init_early instead of omap3630_init_early. So add
compatiblity for 3630 to allow match
Signed-off-by: Nishanth Menon <[email protected]>
---
arch/arm/boot/dts/omap3-beagle-xm.dts | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/omap3-beagle-xm.dts b/arch/arm/boot/dts/omap3-beagle-xm.dts
index 0f7cfc5..2079e22 100644
--- a/arch/arm/boot/dts/omap3-beagle-xm.dts
+++ b/arch/arm/boot/dts/omap3-beagle-xm.dts
@@ -11,7 +11,7 @@
/ {
model = "TI OMAP3 BeagleBoard xM";
- compatible = "ti,omap3-beagle-xm", "ti,omap3-beagle", "ti,omap3";
+ compatible = "ti,omap3-beagle-xm", "ti,omap363x", "ti,omap3-beagle", "ti,omap3";
cpus {
cpu@0 {
--
1.7.9.5
SoC family definitions at the moment are reactive to board needs
as a result, beagle-xm would matchup with ti,omap3 which invokes
omap3430_init_early instead of omap3630_init_early. Obviously, this is
the wrong behavior.
Eventually, we will have descriptors match only with SoC types and
should not contain anything specific to board handling.
Signed-off-by: Nishanth Menon <[email protected]>
---
.../devicetree/bindings/arm/omap/omap.txt | 45 ++++++++++++++++++++
arch/arm/mach-omap2/board-generic.c | 23 ++++++++++
2 files changed, 68 insertions(+)
diff --git a/Documentation/devicetree/bindings/arm/omap/omap.txt b/Documentation/devicetree/bindings/arm/omap/omap.txt
index 91b7049..28b20a2 100644
--- a/Documentation/devicetree/bindings/arm/omap/omap.txt
+++ b/Documentation/devicetree/bindings/arm/omap/omap.txt
@@ -30,6 +30,51 @@ spinlock@1 {
ti,hwmods = "spinlock";
};
+SoC Type(optional):
+- ti,gp - General Purpose devices
+- ti,hs - High Security devices
+
+SoC Families:
+
+- OMAP2 generic - defaults to OMAP2420
+ compatible = "ti,omap2"
+- OMAP3 generic - defaults to OMAP3430
+ compatible = "ti,omap3"
+- OMAP4 generic - defaults to OMAP4430
+ compatible = "ti,omap4"
+- OMAP5 generic - defaults to OMAP5430
+ compatible = "ti,omap5"
+- DRA7 generic - defaults to DRA742
+ compatible = "ti,dra7"
+- AM43x generic - defaults to AM437x
+ compatible = "ti,am43"
+
+SoCs:
+
+- OMAP2420
+ compatible = "ti,omap2420", "ti,omap2"
+- OMAP2430
+ compatible = "ti,omap2430", "ti,omap2"
+
+- OMAP3430
+ compatible = "ti,omap343x", "ti,omap3"
+- OMAP3630
+ compatible = "ti,omap363x", "ti,omap3"
+- AM33xx
+ compatible = "ti,am33xx", "ti,omap3"
+
+- OMAP4430
+ compatible = "ti,omap443x", "ti,omap4"
+- OMAP4460
+ compatible = "ti,omap446x", "ti,omap4"
+
+- OMAP5430
+ compatible = "ti,omap5430", "ti,omap5"
+- OMAP5432
+ compatible = "ti,omap5432", "ti,omap5"
+
+- DRA742
+ compatible = "ti,dra7xx", "ti,dra7"
Boards:
diff --git a/arch/arm/mach-omap2/board-generic.c b/arch/arm/mach-omap2/board-generic.c
index 39c7838..c78b070 100644
--- a/arch/arm/mach-omap2/board-generic.c
+++ b/arch/arm/mach-omap2/board-generic.c
@@ -113,6 +113,7 @@ MACHINE_END
#ifdef CONFIG_ARCH_OMAP3
static const char *omap3_boards_compat[] __initdata = {
"ti,omap3",
+ "ti,omap343x",
NULL,
};
@@ -129,6 +130,24 @@ DT_MACHINE_START(OMAP3_DT, "Generic OMAP3 (Flattened Device Tree)")
.restart = omap3xxx_restart,
MACHINE_END
+static const char *omap36xx_boards_compat[] __initdata = {
+ "ti,omap363x",
+ NULL,
+};
+
+DT_MACHINE_START(OMAP36xx_DT, "Generic OMAP363x (Flattened Device Tree)")
+ .reserve = omap_reserve,
+ .map_io = omap3_map_io,
+ .init_early = omap3630_init_early,
+ .init_irq = omap_intc_of_init,
+ .handle_irq = omap3_intc_handle_irq,
+ .init_machine = omap_generic_init,
+ .init_late = omap3_init_late,
+ .init_time = omap3_sync32k_timer_init,
+ .dt_compat = omap36xx_boards_compat,
+ .restart = omap3xxx_restart,
+MACHINE_END
+
static const char *omap3_gp_boards_compat[] __initdata = {
"ti,omap3-beagle",
"timll,omap3-devkit8000",
@@ -171,6 +190,8 @@ MACHINE_END
#ifdef CONFIG_ARCH_OMAP4
static const char *omap4_boards_compat[] __initdata = {
"ti,omap4",
+ "ti,omap4430",
+ "ti,omap4460",
NULL,
};
@@ -191,6 +212,8 @@ MACHINE_END
#ifdef CONFIG_SOC_OMAP5
static const char *omap5_boards_compat[] __initdata = {
"ti,omap5",
+ "ti,omap5430",
+ "ti,omap5432",
NULL,
};
--
1.7.9.5
This is a respin of [1] based on of Olof's comments to introduce a
generic SoC binding. This standardizes the binding definitions for
SoCs based on existing "implied" bindings and based on existing usage
in arch/arm/mach-omap2/soc.h. Eventually we should be able to get rid
of soc_is_xyz() functions and allow machine descriptors to seamlessly
handle the deltas.
The series is based on [2] and was triggered primarily due to the bug
seen with [3] - clock dts conversion.
Nishanth Menon (2):
Documentation: dt: OMAP: standardize SoC naming definition
ARM: dts: omap3-beagle: use 3630 definitions
.../devicetree/bindings/arm/omap/omap.txt | 45 ++++++++++++++++++++
arch/arm/boot/dts/omap3-beagle-xm.dts | 2 +-
arch/arm/mach-omap2/board-generic.c | 23 ++++++++++
3 files changed, 69 insertions(+), 1 deletion(-)
[1] https://patchwork.kernel.org/patch/2919661/
[2] https://git.kernel.org/cgit/linux/kernel/git/bcousson/linux-omap-dt.git/log/?h=for_3.13/dts
[3] http://marc.info/?t=138009899400001&r=1&w=2
--
1.7.9.5
Hi,
Few comments below.
* Nishanth Menon <[email protected]> [131007 09:57]:
> SoC family definitions at the moment are reactive to board needs
> as a result, beagle-xm would matchup with ti,omap3 which invokes
> omap3430_init_early instead of omap3630_init_early. Obviously, this is
> the wrong behavior.
It seems that we should try to queue this as a fix to avoid
debugging it multiple times as it's actually quite easy to hit this
one.
So maybe use a subject along lines of:
"ARM: OMAP3: Fix hardware detection for omap3630 when booted with device tree"
And also include these warnings you can get:
omap_hwmod: uart4: cannot clk_get main_clk uart4_fck
omap_hwmod: uart4: cannot _init_clocks
WARNING: CPU: 0 PID: 1 at arch/arm/mach-omap2/omap_hwmod.c:2434 _init+0x6c/0x80()
omap_hwmod: uart4: couldn't init clocks
...
WARNING: CPU: 0 PID: 1 at arch/arm/mach-omap2/omap_hwmod.c:2126 _enable+0x254/0x280()
omap_hwmod: timer12: enabled state can only be entered from initialized, idle, or disabled state
...
WARNING: CPU: 0 PID: 46 at arch/arm/mach-omap2/omap_hwmod.c:2224 _idle+0xd4/0xf8()
omap_hwmod: timer12: idle state can only be entered from enabled state
WARNING: CPU: 0 PID: 1 at arch/arm/mach-omap2/omap_hwmod.c:2126 _enable+0x254/0x280()
omap_hwmod: uart4: enabled state can only be entered from initialized, idle, or disabled state
And also describe that the outcome is that the system fails to boot.
> --- a/Documentation/devicetree/bindings/arm/omap/omap.txt
> +++ b/Documentation/devicetree/bindings/arm/omap/omap.txt
> @@ -30,6 +30,51 @@ spinlock@1 {
> ti,hwmods = "spinlock";
> };
>
> +SoC Type(optional):
> +- ti,gp - General Purpose devices
> +- ti,hs - High Security devices
> +
> +SoC Families:
> +
> +- OMAP2 generic - defaults to OMAP2420
> + compatible = "ti,omap2"
> +- OMAP3 generic - defaults to OMAP3430
> + compatible = "ti,omap3"
> +- OMAP4 generic - defaults to OMAP4430
> + compatible = "ti,omap4"
> +- OMAP5 generic - defaults to OMAP5430
> + compatible = "ti,omap5"
> +- DRA7 generic - defaults to DRA742
> + compatible = "ti,dra7"
> +- AM43x generic - defaults to AM437x
> + compatible = "ti,am43"
> +
> +SoCs:
> +
> +- OMAP2420
> + compatible = "ti,omap2420", "ti,omap2"
> +- OMAP2430
> + compatible = "ti,omap2430", "ti,omap2"
> +
> +- OMAP3430
> + compatible = "ti,omap343x", "ti,omap3"
> +- OMAP3630
> + compatible = "ti,omap363x", "ti,omap3"
> +- AM33xx
> + compatible = "ti,am33xx", "ti,omap3"
> +
> +- OMAP4430
> + compatible = "ti,omap443x", "ti,omap4"
> +- OMAP4460
> + compatible = "ti,omap446x", "ti,omap4"
> +
> +- OMAP5430
> + compatible = "ti,omap5430", "ti,omap5"
> +- OMAP5432
> + compatible = "ti,omap5432", "ti,omap5"
> +
> +- DRA742
> + compatible = "ti,dra7xx", "ti,dra7"
>
> Boards:
And I would leave the documentation parts out of the fix as
we're pretty late into the -rc cycle.
> +++ b/arch/arm/mach-omap2/board-generic.c
> @@ -113,6 +113,7 @@ MACHINE_END
> #ifdef CONFIG_ARCH_OMAP3
> static const char *omap3_boards_compat[] __initdata = {
> "ti,omap3",
> + "ti,omap343x",
> NULL,
> };
>
> @@ -129,6 +130,24 @@ DT_MACHINE_START(OMAP3_DT, "Generic OMAP3 (Flattened Device Tree)")
> .restart = omap3xxx_restart,
> MACHINE_END
>
> +static const char *omap36xx_boards_compat[] __initdata = {
> + "ti,omap363x",
> + NULL,
> +};
Why not make it just "ti,omap36xx"? This also applies to 3703
where the 3 is missing as it does not have the DSP.
> +DT_MACHINE_START(OMAP36xx_DT, "Generic OMAP363x (Flattened Device Tree)")
Upper case OMAP36XX_DT here please like the others have.
> + .reserve = omap_reserve,
> + .map_io = omap3_map_io,
> + .init_early = omap3630_init_early,
> + .init_irq = omap_intc_of_init,
> + .handle_irq = omap3_intc_handle_irq,
> + .init_machine = omap_generic_init,
> + .init_late = omap3_init_late,
> + .init_time = omap3_sync32k_timer_init,
Looks like this version has the correct init_time function too :)
> + .dt_compat = omap36xx_boards_compat,
> + .restart = omap3xxx_restart,
> +MACHINE_END
This looks correct to me.
> static const char *omap3_gp_boards_compat[] __initdata = {
> "ti,omap3-beagle",
> "timll,omap3-devkit8000",
> @@ -171,6 +190,8 @@ MACHINE_END
> #ifdef CONFIG_ARCH_OMAP4
> static const char *omap4_boards_compat[] __initdata = {
> "ti,omap4",
> + "ti,omap4430",
> + "ti,omap4460",
> NULL,
> };
>
> @@ -191,6 +212,8 @@ MACHINE_END
> #ifdef CONFIG_SOC_OMAP5
> static const char *omap5_boards_compat[] __initdata = {
> "ti,omap5",
> + "ti,omap5430",
> + "ti,omap5432",
> NULL,
> };
I would leave these parts for later, maybe with the documentation
changes?
Regards,
Tony
* Nishanth Menon <[email protected]> [131007 09:57]:
> beagle-xm currently would matchup with ti,omap3 which invokes
> omap3430_init_early instead of omap3630_init_early. So add
> compatiblity for 3630 to allow match
>
> Signed-off-by: Nishanth Menon <[email protected]>
> ---
> arch/arm/boot/dts/omap3-beagle-xm.dts | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/omap3-beagle-xm.dts b/arch/arm/boot/dts/omap3-beagle-xm.dts
> index 0f7cfc5..2079e22 100644
> --- a/arch/arm/boot/dts/omap3-beagle-xm.dts
> +++ b/arch/arm/boot/dts/omap3-beagle-xm.dts
> @@ -11,7 +11,7 @@
>
> / {
> model = "TI OMAP3 BeagleBoard xM";
> - compatible = "ti,omap3-beagle-xm", "ti,omap3-beagle", "ti,omap3";
> + compatible = "ti,omap3-beagle-xm", "ti,omap363x", "ti,omap3-beagle", "ti,omap3";
>
> cpus {
> cpu@0 {
This compatible string looks hacky.. How about just make it
"ti,omap3-beagle-xm", "ti,omap363x", "ti,omap3";
How about just leave out "ti,omap3-beagle" here?
And I would fold this change into the fix part of your first
patch in this series.
Regards,
Tony
SoC family definitions at the moment are reactive to board needs
as a result, beagle-xm would matchup with ti,omap3 which invokes
omap3430_init_early instead of omap3630_init_early. Obviously, this is
the wrong behavior.
With clock node dts conversion, we get the following warnings as a
result and 3630 based platforms fails to boot (uart4 clocks are only
present in OMAP3630 and not present in OMAP3430):
...
omap_hwmod: uart4: cannot clk_get main_clk uart4_fck
omap_hwmod: uart4: cannot _init_clocks
WARNING: CPU: 0 PID: 1 at arch/arm/mach-omap2/omap_hwmod.c:2434
_init+0x6c/0x80()
omap_hwmod: uart4: couldn't init clocks
...
WARNING: CPU: 0 PID: 1 at arch/arm/mach-omap2/omap_hwmod.c:2126
_enable+0x254/0x280()
omap_hwmod: timer12: enabled state can only be entered from
initialized, idle, or disabled state
...
WARNING: CPU: 0 PID: 46 at arch/arm/mach-omap2/omap_hwmod.c:2224
_idle+0xd4/0xf8()
omap_hwmod: timer12: idle state can only be entered from enabled state
WARNING: CPU: 0 PID: 1 at arch/arm/mach-omap2/omap_hwmod.c:2126
_enable+0x254/0x280()
omap_hwmod: uart4: enabled state can only be entered from
initialized, idle, or disabled state
So, add specific compatiblity for 3630 to allow match for Beagle-XM
platform.
Signed-off-by: Nishanth Menon <[email protected]>
---
Changes in V2 (since v1):
- based on v3.12-rc4 tag
- Update based on Tony's review comments.
V1: http://marc.info/?l=linux-omap&m=138117342909375&w=2
RFC: https://patchwork.kernel.org/patch/2919661/
arch/arm/boot/dts/omap3-beagle-xm.dts | 2 +-
arch/arm/mach-omap2/board-generic.c | 19 +++++++++++++++++++
2 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/omap3-beagle-xm.dts b/arch/arm/boot/dts/omap3-beagle-xm.dts
index 0c514dc..02dd4a9 100644
--- a/arch/arm/boot/dts/omap3-beagle-xm.dts
+++ b/arch/arm/boot/dts/omap3-beagle-xm.dts
@@ -11,7 +11,7 @@
/ {
model = "TI OMAP3 BeagleBoard xM";
- compatible = "ti,omap3-beagle-xm", "ti,omap3-beagle", "ti,omap3";
+ compatible = "ti,omap3-beagle-xm", "ti,omap363x", "ti,omap3";
cpus {
cpu@0 {
diff --git a/arch/arm/mach-omap2/board-generic.c b/arch/arm/mach-omap2/board-generic.c
index 39c7838..4fe5b9c 100644
--- a/arch/arm/mach-omap2/board-generic.c
+++ b/arch/arm/mach-omap2/board-generic.c
@@ -113,6 +113,7 @@ MACHINE_END
#ifdef CONFIG_ARCH_OMAP3
static const char *omap3_boards_compat[] __initdata = {
"ti,omap3",
+ "ti,omap343x",
NULL,
};
@@ -129,6 +130,24 @@ DT_MACHINE_START(OMAP3_DT, "Generic OMAP3 (Flattened Device Tree)")
.restart = omap3xxx_restart,
MACHINE_END
+static const char *omap36xx_boards_compat[] __initdata = {
+ "ti,omap36xx",
+ NULL,
+};
+
+DT_MACHINE_START(OMAP36XX_DT, "Generic OMAP363x (Flattened Device Tree)")
+ .reserve = omap_reserve,
+ .map_io = omap3_map_io,
+ .init_early = omap3630_init_early,
+ .init_irq = omap_intc_of_init,
+ .handle_irq = omap3_intc_handle_irq,
+ .init_machine = omap_generic_init,
+ .init_late = omap3_init_late,
+ .init_time = omap3_sync32k_timer_init,
+ .dt_compat = omap36xx_boards_compat,
+ .restart = omap3xxx_restart,
+MACHINE_END
+
static const char *omap3_gp_boards_compat[] __initdata = {
"ti,omap3-beagle",
"timll,omap3-devkit8000",
--
1.7.9.5
On 10/07/2013 03:30 PM, Nishanth Menon wrote:
[...]
> diff --git a/arch/arm/boot/dts/omap3-beagle-xm.dts b/arch/arm/boot/dts/omap3-beagle-xm.dts
> index 0c514dc..02dd4a9 100644
> --- a/arch/arm/boot/dts/omap3-beagle-xm.dts
> +++ b/arch/arm/boot/dts/omap3-beagle-xm.dts
> @@ -11,7 +11,7 @@
>
> / {
> model = "TI OMAP3 BeagleBoard xM";
> - compatible = "ti,omap3-beagle-xm", "ti,omap3-beagle", "ti,omap3";
> + compatible = "ti,omap3-beagle-xm", "ti,omap363x", "ti,omap3";
^^^ yikes - should have been ti,omap36xx -> apologies, my fix was not
committed in local branch :( - will update and resend - grr..
--
Regards,
Nishanth Menon
SoC family definitions at the moment are reactive to board needs
as a result, beagle-xm would matchup with ti,omap3 which invokes
omap3430_init_early instead of omap3630_init_early. Obviously, this is
the wrong behavior.
With clock node dts conversion, we get the following warnings as a
result and 3630 based platforms fails to boot (uart4 clocks are only
present in OMAP3630 and not present in OMAP3430):
...
omap_hwmod: uart4: cannot clk_get main_clk uart4_fck
omap_hwmod: uart4: cannot _init_clocks
WARNING: CPU: 0 PID: 1 at arch/arm/mach-omap2/omap_hwmod.c:2434
_init+0x6c/0x80()
omap_hwmod: uart4: couldn't init clocks
...
WARNING: CPU: 0 PID: 1 at arch/arm/mach-omap2/omap_hwmod.c:2126
_enable+0x254/0x280()
omap_hwmod: timer12: enabled state can only be entered from
initialized, idle, or disabled state
...
WARNING: CPU: 0 PID: 46 at arch/arm/mach-omap2/omap_hwmod.c:2224
_idle+0xd4/0xf8()
omap_hwmod: timer12: idle state can only be entered from enabled state
WARNING: CPU: 0 PID: 1 at arch/arm/mach-omap2/omap_hwmod.c:2126
_enable+0x254/0x280()
omap_hwmod: uart4: enabled state can only be entered from
initialized, idle, or disabled state
So, add specific compatiblity for 3630 to allow match for Beagle-XM
platform.
Signed-off-by: Nishanth Menon <[email protected]>
---
Changes in V3 (since v2):
- Fix typo in dts :(
V2: https://patchwork.kernel.org/patch/2999111/
- based on v3.12-rc4 tag
- Update based on Tony's review comments.
V1: http://marc.info/?l=linux-omap&m=138117342909375&w=2
RFC: https://patchwork.kernel.org/patch/2919661/
arch/arm/boot/dts/omap3-beagle-xm.dts | 2 +-
arch/arm/mach-omap2/board-generic.c | 19 +++++++++++++++++++
2 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/omap3-beagle-xm.dts b/arch/arm/boot/dts/omap3-beagle-xm.dts
index 0c514dc..2816bf6 100644
--- a/arch/arm/boot/dts/omap3-beagle-xm.dts
+++ b/arch/arm/boot/dts/omap3-beagle-xm.dts
@@ -11,7 +11,7 @@
/ {
model = "TI OMAP3 BeagleBoard xM";
- compatible = "ti,omap3-beagle-xm", "ti,omap3-beagle", "ti,omap3";
+ compatible = "ti,omap3-beagle-xm", "ti,omap36xx", "ti,omap3";
cpus {
cpu@0 {
diff --git a/arch/arm/mach-omap2/board-generic.c b/arch/arm/mach-omap2/board-generic.c
index 39c7838..4fe5b9c 100644
--- a/arch/arm/mach-omap2/board-generic.c
+++ b/arch/arm/mach-omap2/board-generic.c
@@ -113,6 +113,7 @@ MACHINE_END
#ifdef CONFIG_ARCH_OMAP3
static const char *omap3_boards_compat[] __initdata = {
"ti,omap3",
+ "ti,omap343x",
NULL,
};
@@ -129,6 +130,24 @@ DT_MACHINE_START(OMAP3_DT, "Generic OMAP3 (Flattened Device Tree)")
.restart = omap3xxx_restart,
MACHINE_END
+static const char *omap36xx_boards_compat[] __initdata = {
+ "ti,omap36xx",
+ NULL,
+};
+
+DT_MACHINE_START(OMAP36XX_DT, "Generic OMAP36xx (Flattened Device Tree)")
+ .reserve = omap_reserve,
+ .map_io = omap3_map_io,
+ .init_early = omap3630_init_early,
+ .init_irq = omap_intc_of_init,
+ .handle_irq = omap3_intc_handle_irq,
+ .init_machine = omap_generic_init,
+ .init_late = omap3_init_late,
+ .init_time = omap3_sync32k_timer_init,
+ .dt_compat = omap36xx_boards_compat,
+ .restart = omap3xxx_restart,
+MACHINE_END
+
static const char *omap3_gp_boards_compat[] __initdata = {
"ti,omap3-beagle",
"timll,omap3-devkit8000",
--
1.7.9.5
On Mon, Oct 07, 2013 at 03:43:49PM -0500, Nishanth Menon wrote:
> diff --git a/arch/arm/mach-omap2/board-generic.c b/arch/arm/mach-omap2/board-generic.c
> index 39c7838..4fe5b9c 100644
> --- a/arch/arm/mach-omap2/board-generic.c
> +++ b/arch/arm/mach-omap2/board-generic.c
> @@ -113,6 +113,7 @@ MACHINE_END
> #ifdef CONFIG_ARCH_OMAP3
> static const char *omap3_boards_compat[] __initdata = {
> "ti,omap3",
> + "ti,omap343x",
^^^^
You used omap36xx everywhere, so I guess this should be
"ti,omap34xx"?
-- Sebastian
On 10/07/2013 07:05 PM, Sebastian Reichel wrote:
> On Mon, Oct 07, 2013 at 03:43:49PM -0500, Nishanth Menon wrote:
>> diff --git a/arch/arm/mach-omap2/board-generic.c b/arch/arm/mach-omap2/board-generic.c
>> index 39c7838..4fe5b9c 100644
>> --- a/arch/arm/mach-omap2/board-generic.c
>> +++ b/arch/arm/mach-omap2/board-generic.c
>> @@ -113,6 +113,7 @@ MACHINE_END
>> #ifdef CONFIG_ARCH_OMAP3
>> static const char *omap3_boards_compat[] __initdata = {
>> "ti,omap3",
>> + "ti,omap343x",
> ^^^^
> You used omap36xx everywhere, so I guess this should be
> "ti,omap34xx"?
3630 has at least two variants:
http://www.ti.com/product/omap3621
http://www.ti.com/product/omap3630
It got further spun off in 37xx family:
http://www.ti.com/product/dm3730 (DM3730 and DM3725)
3430 is no longer in production - but just had a single version (at
least what I can remember).
3530 variant of 3430 on the other hand has a few:
http://processors.wiki.ti.com/index.php/OMAP3_Overview
I am ok to change to ti,omap34xx if folks think that is the right
thing to do. Personally, I might prefer to handle 35xx family slightly
differently considering deltas.
--
Regards,
Nishanth Menon
Hi,
On Mon, Oct 07, 2013 at 12:20:09PM -0700, Tony Lindgren wrote:
> * Nishanth Menon <[email protected]> [131007 09:57]:
> > beagle-xm currently would matchup with ti,omap3 which invokes
> > omap3430_init_early instead of omap3630_init_early. So add
> > compatiblity for 3630 to allow match
> >
> > Signed-off-by: Nishanth Menon <[email protected]>
> > ---
> > arch/arm/boot/dts/omap3-beagle-xm.dts | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/arm/boot/dts/omap3-beagle-xm.dts b/arch/arm/boot/dts/omap3-beagle-xm.dts
> > index 0f7cfc5..2079e22 100644
> > --- a/arch/arm/boot/dts/omap3-beagle-xm.dts
> > +++ b/arch/arm/boot/dts/omap3-beagle-xm.dts
> > @@ -11,7 +11,7 @@
> >
> > / {
> > model = "TI OMAP3 BeagleBoard xM";
> > - compatible = "ti,omap3-beagle-xm", "ti,omap3-beagle", "ti,omap3";
> > + compatible = "ti,omap3-beagle-xm", "ti,omap363x", "ti,omap3-beagle", "ti,omap3";
> >
> > cpus {
> > cpu@0 {
>
> This compatible string looks hacky.. How about just make it
>
> "ti,omap3-beagle-xm", "ti,omap363x", "ti,omap3";
>
> How about just leave out "ti,omap3-beagle" here?
ti,omap3-beagle was already part of the original file, we can definitely
remove but I'd expect us to maintain support for that string
indefinitely should anybody continue to use older dtbs.
--
balbi
* Nishanth Menon <[email protected]> [131008 05:08]:
> On 10/07/2013 07:05 PM, Sebastian Reichel wrote:
> > On Mon, Oct 07, 2013 at 03:43:49PM -0500, Nishanth Menon wrote:
> >> diff --git a/arch/arm/mach-omap2/board-generic.c b/arch/arm/mach-omap2/board-generic.c
> >> index 39c7838..4fe5b9c 100644
> >> --- a/arch/arm/mach-omap2/board-generic.c
> >> +++ b/arch/arm/mach-omap2/board-generic.c
> >> @@ -113,6 +113,7 @@ MACHINE_END
> >> #ifdef CONFIG_ARCH_OMAP3
> >> static const char *omap3_boards_compat[] __initdata = {
> >> "ti,omap3",
> >> + "ti,omap343x",
> > ^^^^
> > You used omap36xx everywhere, so I guess this should be
> > "ti,omap34xx"?
>
> 3630 has at least two variants:
> http://www.ti.com/product/omap3621
> http://www.ti.com/product/omap3630
>
> It got further spun off in 37xx family:
> http://www.ti.com/product/dm3730 (DM3730 and DM3725)
>
> 3430 is no longer in production - but just had a single version (at
> least what I can remember).
>
> 3530 variant of 3430 on the other hand has a few:
> http://processors.wiki.ti.com/index.php/OMAP3_Overview
>
>
> I am ok to change to ti,omap34xx if folks think that is the right
> thing to do. Personally, I might prefer to handle 35xx family slightly
> differently considering deltas.
I've dropped that part as that's not needed for the fix AFAIK.
Here's what I've applied and pushed out to omap-for-v3.12/fixes.
Regards,
Tony
From: Nishanth Menon <[email protected]>
Date: Mon, 7 Oct 2013 15:43:49 -0500
Subject: [PATCH] ARM: OMAP3: Fix hardware detection for omap3630 when booted with device tree
SoC family definitions at the moment are reactive to board needs
as a result, beagle-xm would matchup with ti,omap3 which invokes
omap3430_init_early instead of omap3630_init_early. Obviously, this is
the wrong behavior.
With clock node dts conversion, we get the following warnings before
system hangs as a result and 3630 based platforms fails to boot
(uart4 clocks are only present in OMAP3630 and not present in
OMAP3430):
...
omap_hwmod: uart4: cannot clk_get main_clk uart4_fck
omap_hwmod: uart4: cannot _init_clocks
WARNING: CPU: 0 PID: 1 at arch/arm/mach-omap2/omap_hwmod.c:2434
_init+0x6c/0x80()
omap_hwmod: uart4: couldn't init clocks
...
WARNING: CPU: 0 PID: 1 at arch/arm/mach-omap2/omap_hwmod.c:2126
_enable+0x254/0x280()
omap_hwmod: timer12: enabled state can only be entered from
initialized, idle, or disabled state
...
WARNING: CPU: 0 PID: 46 at arch/arm/mach-omap2/omap_hwmod.c:2224
_idle+0xd4/0xf8()
omap_hwmod: timer12: idle state can only be entered from enabled state
WARNING: CPU: 0 PID: 1 at arch/arm/mach-omap2/omap_hwmod.c:2126
_enable+0x254/0x280()
omap_hwmod: uart4: enabled state can only be entered from
initialized, idle, or disabled state
So, add specific compatiblity for 3630 to allow match for Beagle-XM
platform.
Signed-off-by: Nishanth Menon <[email protected]>
[[email protected]: left out ti,omap343x, updated comments]
Signed-off-by: Tony Lindgren <[email protected]>
diff --git a/arch/arm/boot/dts/omap3-beagle-xm.dts b/arch/arm/boot/dts/omap3-beagle-xm.dts
index 0c514dc..2816bf6 100644
--- a/arch/arm/boot/dts/omap3-beagle-xm.dts
+++ b/arch/arm/boot/dts/omap3-beagle-xm.dts
@@ -11,7 +11,7 @@
/ {
model = "TI OMAP3 BeagleBoard xM";
- compatible = "ti,omap3-beagle-xm", "ti,omap3-beagle", "ti,omap3";
+ compatible = "ti,omap3-beagle-xm", "ti,omap36xx", "ti,omap3";
cpus {
cpu@0 {
diff --git a/arch/arm/mach-omap2/board-generic.c b/arch/arm/mach-omap2/board-generic.c
index 39c7838..87162e1 100644
--- a/arch/arm/mach-omap2/board-generic.c
+++ b/arch/arm/mach-omap2/board-generic.c
@@ -129,6 +129,24 @@ DT_MACHINE_START(OMAP3_DT, "Generic OMAP3 (Flattened Device Tree)")
.restart = omap3xxx_restart,
MACHINE_END
+static const char *omap36xx_boards_compat[] __initdata = {
+ "ti,omap36xx",
+ NULL,
+};
+
+DT_MACHINE_START(OMAP36XX_DT, "Generic OMAP36xx (Flattened Device Tree)")
+ .reserve = omap_reserve,
+ .map_io = omap3_map_io,
+ .init_early = omap3630_init_early,
+ .init_irq = omap_intc_of_init,
+ .handle_irq = omap3_intc_handle_irq,
+ .init_machine = omap_generic_init,
+ .init_late = omap3_init_late,
+ .init_time = omap3_sync32k_timer_init,
+ .dt_compat = omap36xx_boards_compat,
+ .restart = omap3xxx_restart,
+MACHINE_END
+
static const char *omap3_gp_boards_compat[] __initdata = {
"ti,omap3-beagle",
"timll,omap3-devkit8000",
On 10/08/2013 12:58 PM, Tony Lindgren wrote:
> * Nishanth Menon <[email protected]> [131008 05:08]:
>> On 10/07/2013 07:05 PM, Sebastian Reichel wrote:
>>> On Mon, Oct 07, 2013 at 03:43:49PM -0500, Nishanth Menon wrote:
>>>> diff --git a/arch/arm/mach-omap2/board-generic.c b/arch/arm/mach-omap2/board-generic.c
>>>> index 39c7838..4fe5b9c 100644
>>>> --- a/arch/arm/mach-omap2/board-generic.c
>>>> +++ b/arch/arm/mach-omap2/board-generic.c
>>>> @@ -113,6 +113,7 @@ MACHINE_END
>>>> #ifdef CONFIG_ARCH_OMAP3
>>>> static const char *omap3_boards_compat[] __initdata = {
>>>> "ti,omap3",
>>>> + "ti,omap343x",
>>> ^^^^
>>> You used omap36xx everywhere, so I guess this should be
>>> "ti,omap34xx"?
>>
>> 3630 has at least two variants:
>> http://www.ti.com/product/omap3621
>> http://www.ti.com/product/omap3630
>>
>> It got further spun off in 37xx family:
>> http://www.ti.com/product/dm3730 (DM3730 and DM3725)
>>
>> 3430 is no longer in production - but just had a single version (at
>> least what I can remember).
>>
>> 3530 variant of 3430 on the other hand has a few:
>> http://processors.wiki.ti.com/index.php/OMAP3_Overview
>>
>>
>> I am ok to change to ti,omap34xx if folks think that is the right
>> thing to do. Personally, I might prefer to handle 35xx family slightly
>> differently considering deltas.
>
> I've dropped that part as that's not needed for the fix AFAIK.
> Here's what I've applied and pushed out to omap-for-v3.12/fixes.
>
> Regards,
>
> Tony
>
> From: Nishanth Menon <[email protected]>
> Date: Mon, 7 Oct 2013 15:43:49 -0500
> Subject: [PATCH] ARM: OMAP3: Fix hardware detection for omap3630 when booted with device tree
>
> SoC family definitions at the moment are reactive to board needs
> as a result, beagle-xm would matchup with ti,omap3 which invokes
> omap3430_init_early instead of omap3630_init_early. Obviously, this is
> the wrong behavior.
>
> With clock node dts conversion, we get the following warnings before
> system hangs as a result and 3630 based platforms fails to boot
> (uart4 clocks are only present in OMAP3630 and not present in
> OMAP3430):
>
> ...
> omap_hwmod: uart4: cannot clk_get main_clk uart4_fck
> omap_hwmod: uart4: cannot _init_clocks
>
> WARNING: CPU: 0 PID: 1 at arch/arm/mach-omap2/omap_hwmod.c:2434
> _init+0x6c/0x80()
> omap_hwmod: uart4: couldn't init clocks
> ...
>
> WARNING: CPU: 0 PID: 1 at arch/arm/mach-omap2/omap_hwmod.c:2126
> _enable+0x254/0x280()
> omap_hwmod: timer12: enabled state can only be entered from
> initialized, idle, or disabled state
> ...
>
> WARNING: CPU: 0 PID: 46 at arch/arm/mach-omap2/omap_hwmod.c:2224
> _idle+0xd4/0xf8()
> omap_hwmod: timer12: idle state can only be entered from enabled state
>
> WARNING: CPU: 0 PID: 1 at arch/arm/mach-omap2/omap_hwmod.c:2126
> _enable+0x254/0x280()
> omap_hwmod: uart4: enabled state can only be entered from
> initialized, idle, or disabled state
>
> So, add specific compatiblity for 3630 to allow match for Beagle-XM
> platform.
>
> Signed-off-by: Nishanth Menon <[email protected]>
> [[email protected]: left out ti,omap343x, updated comments]
> Signed-off-by: Tony Lindgren <[email protected]>
>
> diff --git a/arch/arm/boot/dts/omap3-beagle-xm.dts b/arch/arm/boot/dts/omap3-beagle-xm.dts
> index 0c514dc..2816bf6 100644
> --- a/arch/arm/boot/dts/omap3-beagle-xm.dts
> +++ b/arch/arm/boot/dts/omap3-beagle-xm.dts
> @@ -11,7 +11,7 @@
>
> / {
> model = "TI OMAP3 BeagleBoard xM";
> - compatible = "ti,omap3-beagle-xm", "ti,omap3-beagle", "ti,omap3";
> + compatible = "ti,omap3-beagle-xm", "ti,omap36xx", "ti,omap3";
>
> cpus {
> cpu@0 {
> diff --git a/arch/arm/mach-omap2/board-generic.c b/arch/arm/mach-omap2/board-generic.c
> index 39c7838..87162e1 100644
> --- a/arch/arm/mach-omap2/board-generic.c
> +++ b/arch/arm/mach-omap2/board-generic.c
> @@ -129,6 +129,24 @@ DT_MACHINE_START(OMAP3_DT, "Generic OMAP3 (Flattened Device Tree)")
> .restart = omap3xxx_restart,
> MACHINE_END
>
> +static const char *omap36xx_boards_compat[] __initdata = {
> + "ti,omap36xx",
> + NULL,
> +};
> +
> +DT_MACHINE_START(OMAP36XX_DT, "Generic OMAP36xx (Flattened Device Tree)")
> + .reserve = omap_reserve,
> + .map_io = omap3_map_io,
> + .init_early = omap3630_init_early,
> + .init_irq = omap_intc_of_init,
> + .handle_irq = omap3_intc_handle_irq,
> + .init_machine = omap_generic_init,
> + .init_late = omap3_init_late,
> + .init_time = omap3_sync32k_timer_init,
> + .dt_compat = omap36xx_boards_compat,
> + .restart = omap3xxx_restart,
> +MACHINE_END
> +
> static const char *omap3_gp_boards_compat[] __initdata = {
> "ti,omap3-beagle",
> "timll,omap3-devkit8000",
>
LGTM. Thanks for doing it.
--
Regards,
Nishanth Menon
On 10/08/2013 12:47 PM, Felipe Balbi wrote:
> Hi,
>
> On Mon, Oct 07, 2013 at 12:20:09PM -0700, Tony Lindgren wrote:
>> * Nishanth Menon <[email protected]> [131007 09:57]:
>>> beagle-xm currently would matchup with ti,omap3 which invokes
>>> omap3430_init_early instead of omap3630_init_early. So add
>>> compatiblity for 3630 to allow match
>>>
>>> Signed-off-by: Nishanth Menon <[email protected]>
>>> ---
>>> arch/arm/boot/dts/omap3-beagle-xm.dts | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm/boot/dts/omap3-beagle-xm.dts b/arch/arm/boot/dts/omap3-beagle-xm.dts
>>> index 0f7cfc5..2079e22 100644
>>> --- a/arch/arm/boot/dts/omap3-beagle-xm.dts
>>> +++ b/arch/arm/boot/dts/omap3-beagle-xm.dts
>>> @@ -11,7 +11,7 @@
>>>
>>> / {
>>> model = "TI OMAP3 BeagleBoard xM";
>>> - compatible = "ti,omap3-beagle-xm", "ti,omap3-beagle", "ti,omap3";
>>> + compatible = "ti,omap3-beagle-xm", "ti,omap363x", "ti,omap3-beagle", "ti,omap3";
>>>
>>> cpus {
>>> cpu@0 {
>>
>> This compatible string looks hacky.. How about just make it
>>
>> "ti,omap3-beagle-xm", "ti,omap363x", "ti,omap3";
>>
>> How about just leave out "ti,omap3-beagle" here?
>
> ti,omap3-beagle was already part of the original file, we can definitely
> remove but I'd expect us to maintain support for that string
> indefinitely should anybody continue to use older dtbs.
>
it already is supported (we still have a beagleboard) - the current
patch rev does nothing to break existing code or bindings.
--
Regards,
Nishanth Menon