diff mbox

[RFC] ARM: OMAP3630: Add generic machine descriptor

Message ID 1379693332-10585-1-git-send-email-nm@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Nishanth Menon Sept. 20, 2013, 4:08 p.m. UTC
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 <nm@ti.com>
---

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(+)

Comments

Olof Johansson Sept. 20, 2013, 4:19 p.m. UTC | #1
Hi,

On Fri, Sep 20, 2013 at 9:08 AM, Nishanth Menon <nm@ti.com> 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
Felipe Balbi Sept. 20, 2013, 4:23 p.m. UTC | #2
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 <nm@ti.com> 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
Olof Johansson Sept. 20, 2013, 5:16 p.m. UTC | #3
On Fri, Sep 20, 2013 at 9:23 AM, Felipe Balbi <balbi@ti.com> 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 <nm@ti.com> 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
Felipe Balbi Sept. 20, 2013, 5:42 p.m. UTC | #4
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 <nm@ti.com> 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
Nishanth Menon Sept. 20, 2013, 7:10 p.m. UTC | #5
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 <nm@ti.com> 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..
Nishanth Menon Oct. 7, 2013, 4:49 p.m. UTC | #6
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
diff mbox

Patch

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