diff mbox

[6/7] OMAP3: board-dt: Add generic board file for DT support

Message ID 4E60C174.50604@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Benoit Cousson Sept. 2, 2011, 11:43 a.m. UTC
On 9/2/2011 12:43 PM, Tony Lindgren wrote:
> * Cousson, Benoit<b-cousson@ti.com>  [110902 11:13]:
>> Hi Tony,
>>
>> On 9/2/2011 10:09 AM, Tony Lindgren wrote:
>>> Hi,
>>>
>>> * Benoit Cousson<b-cousson@ti.com>   [110901 19:52]:
>>>> Create an OMAP3 generic board to start the DT migration.
>>>
>>> I don't think this needs to be SoC specific, we can add multiple
>>> DT_MACHINE_START entries into a single file. So it should be
>>> just board-omap-dt.c.
>>
>> I do agree, it should not, I made that comment into the
>> board-omap4-dt.c, but for the moment we still have dedicated OMAP
>> specifics stuff at board level, like the map_io.
> 
> Well map_io can also be set in DT_MACHINE_START. For most part
> it already is generic like omap3_map_io and omap4_map_io.
> So that should not stop anything.

OK, I think I was trying to be a little bit more extreme, meaning only one omap_map_io for every OMAPs.
Then, inside omap_map_io we can use the DT compatible information to retrieve the proper OMAPs revision and thus use the proper init table.

Whereas in your case you are still relying on the various DT_MACHINE entries to detect which OMAP revision is used.

It looks to me that relying on several DT_MACHINE_START to identify a SoC version is a little bit more a hack. 
I will let the device-tree experts answering that question :-)
For my point of view, using DT compatible in the various place where the OMAP revision is important, might allow a finer granularity.

>> I have an other series that make the map_io DT aware to get rid of
>> that, but it still not finalized.
> 
> Hmm maybe take a look again? We've already sorted out quite a bit
> of the init stuff over past few merge windows. If there's still
> something blocking that, let's clear it out ASAP.

Now that I understand your approach, it is fine. Please find below an example of what I was trying to achieve, and why that point was an issue.
The clear advantage of your method is that not further effort will be require for this part. At least for the moment.

>> My goal was have a single DT_MACHINE_START for every OMAPs.
>> But, meanwhile, if you prefer one file with many board descriptors,
>> that's fine.
> 
> Yes that's better. We should only need a separate DT_MACHINE_START
> for major SoC variants.

OK, I'll try that.

Thanks,
Benoit

---
From a8edcd113b6fa38620dae11bcbb55daa84dfb07c Mon Sep 17 00:00:00 2001
From: Benoit Cousson <b-cousson@ti.com>
Date: Mon, 22 Aug 2011 11:10:38 +0200
Subject: [PATCH] OMAP2+: Add a common method to init globals using DT compatible

Thanks to the DT root node compatible information, we can extract
the compatible SoC and use it to select the proper globals.
Create a common omap2_set_globals API that can be used by every
OMAP2+ platforms.

Signed-off-by: Benoit Cousson <b-cousson@ti.com>
Cc: Tony Lindgren <tony@atomide.com>
---
 arch/arm/mach-omap2/common.c             |   73 ++++++++++++++++++++++++++++++
 arch/arm/plat-omap/include/plat/common.h |    1 +
 2 files changed, 74 insertions(+), 0 deletions(-)

Comments

Tony Lindgren Sept. 2, 2011, 11:57 a.m. UTC | #1
* Cousson, Benoit <b-cousson@ti.com> [110902 14:10]:
> On 9/2/2011 12:43 PM, Tony Lindgren wrote:
> > * Cousson, Benoit<b-cousson@ti.com>  [110902 11:13]:
> >> Hi Tony,
> >>
> >> On 9/2/2011 10:09 AM, Tony Lindgren wrote:
> >>> Hi,
> >>>
> >>> * Benoit Cousson<b-cousson@ti.com>   [110901 19:52]:
> >>>> Create an OMAP3 generic board to start the DT migration.
> >>>
> >>> I don't think this needs to be SoC specific, we can add multiple
> >>> DT_MACHINE_START entries into a single file. So it should be
> >>> just board-omap-dt.c.
> >>
> >> I do agree, it should not, I made that comment into the
> >> board-omap4-dt.c, but for the moment we still have dedicated OMAP
> >> specifics stuff at board level, like the map_io.
> > 
> > Well map_io can also be set in DT_MACHINE_START. For most part
> > it already is generic like omap3_map_io and omap4_map_io.
> > So that should not stop anything.
> 
> OK, I think I was trying to be a little bit more extreme, meaning only one omap_map_io for every OMAPs.
> Then, inside omap_map_io we can use the DT compatible information to retrieve the proper OMAPs revision and thus use the proper init table.
>
> Whereas in your case you are still relying on the various DT_MACHINE entries to detect which OMAP revision is used.

Sure that can be changed. But why not get the whole data that's
now in various structs from DT too? It's just something like:

        .class  = OMAP243X_CLASS,
        .tap    = OMAP2_L4_IO_ADDRESS(0x4900a000),
        .sdrc   = OMAP243X_SDRC_BASE,
        .sms    = OMAP243X_SMS_BASE,
        .ctrl   = OMAP243X_CTRL_BASE,
        .prm    = OMAP2430_PRM_BASE,
        .cm     = OMAP2430_CM_BASE,

And all these could be just defined in DT right?
 
> It looks to me that relying on several DT_MACHINE_START to identify a SoC version is a little bit more a hack. 
> I will let the device-tree experts answering that question :-)
> For my point of view, using DT compatible in the various place where the OMAP revision is important, might allow a finer granularity.

Sure I agree.
 
> >> I have an other series that make the map_io DT aware to get rid of
> >> that, but it still not finalized.
> > 
> > Hmm maybe take a look again? We've already sorted out quite a bit
> > of the init stuff over past few merge windows. If there's still
> > something blocking that, let's clear it out ASAP.
> 
> Now that I understand your approach, it is fine. Please find below an example of what I was trying to achieve, and why that point was an issue.
> The clear advantage of your method is that not further effort will be require for this part. At least for the moment.

Well I guess the globals could come from device tree still :)

In general, I'd like to get rid of the cpu_is_omapxxxx stuff
and SoC detection early as that always requires DEBUG_LL to
see any decent debug info when things go wrong.

If we get the SoC major type from DT, then we only need to
do SoC specific ES revision and GP/HS omap detection. And that's
very simple code compared to the current cpu_is_omapxxxx stuff.

Then we can boot with the DT information pretty far to map_io
and initialize low-level serial console, and then set up things
further while having a decent console ;)

Regards,

Tony
Benoit Cousson Sept. 2, 2011, 12:20 p.m. UTC | #2
On 9/2/2011 1:57 PM, Tony Lindgren wrote:
> * Cousson, Benoit<b-cousson@ti.com>  [110902 14:10]:
>> On 9/2/2011 12:43 PM, Tony Lindgren wrote:
>>> * Cousson, Benoit<b-cousson@ti.com>   [110902 11:13]:
>>>> Hi Tony,
>>>>
>>>> On 9/2/2011 10:09 AM, Tony Lindgren wrote:
>>>>> Hi,
>>>>>
>>>>> * Benoit Cousson<b-cousson@ti.com>    [110901 19:52]:
>>>>>> Create an OMAP3 generic board to start the DT migration.
>>>>>
>>>>> I don't think this needs to be SoC specific, we can add multiple
>>>>> DT_MACHINE_START entries into a single file. So it should be
>>>>> just board-omap-dt.c.
>>>>
>>>> I do agree, it should not, I made that comment into the
>>>> board-omap4-dt.c, but for the moment we still have dedicated OMAP
>>>> specifics stuff at board level, like the map_io.
>>>
>>> Well map_io can also be set in DT_MACHINE_START. For most part
>>> it already is generic like omap3_map_io and omap4_map_io.
>>> So that should not stop anything.
>>
>> OK, I think I was trying to be a little bit more extreme, meaning only one omap_map_io for every OMAPs.
>> Then, inside omap_map_io we can use the DT compatible information to retrieve the proper OMAPs revision and thus use the proper init table.
>>
>> Whereas in your case you are still relying on the various DT_MACHINE entries to detect which OMAP revision is used.
>
> Sure that can be changed. But why not get the whole data that's
> now in various structs from DT too? It's just something like:
>
>          .class  = OMAP243X_CLASS,
>          .tap    = OMAP2_L4_IO_ADDRESS(0x4900a000),

BTW, how can we get rid of that OMAP2_L4_IO_ADDRESS?

>          .sdrc   = OMAP243X_SDRC_BASE,
>          .sms    = OMAP243X_SMS_BASE,
>          .ctrl   = OMAP243X_CTRL_BASE,
>          .prm    = OMAP2430_PRM_BASE,
>          .cm     = OMAP2430_CM_BASE,
>
> And all these could be just defined in DT right?

Hehe, it is just a phase-one-proof-of-concept... the phase two was 
supposed to move that to DT. But for the moment, I was trying to have a 
more conservative approach using the existing data like for hwmod.

The other point is that we still have to deal with the FDT at that 
point, the DT is not yest available that why I didn't extract too many 
informations from the FDT.
Moreover, in theory, these informations will not be mixed like today, 
but will be in their own relevant DT nodes, somewhere inside the tree.
Bottomline... it was a little bit more tricky.

>> It looks to me that relying on several DT_MACHINE_START to identify a SoC version is a little bit more a hack.
>> I will let the device-tree experts answering that question :-)
>> For my point of view, using DT compatible in the various place where the OMAP revision is important, might allow a finer granularity.
>
> Sure I agree.
>
>>>> I have an other series that make the map_io DT aware to get rid of
>>>> that, but it still not finalized.
>>>
>>> Hmm maybe take a look again? We've already sorted out quite a bit
>>> of the init stuff over past few merge windows. If there's still
>>> something blocking that, let's clear it out ASAP.
>>
>> Now that I understand your approach, it is fine. Please find below an example of what I was trying to achieve, and why that point was an issue.
>> The clear advantage of your method is that not further effort will be require for this part. At least for the moment.
>
> Well I guess the globals could come from device tree still :)

Fully agree. It just needs some more time :-)

> In general, I'd like to get rid of the cpu_is_omapxxxx stuff
> and SoC detection early as that always requires DEBUG_LL to
> see any decent debug info when things go wrong.
>
> If we get the SoC major type from DT, then we only need to
> do SoC specific ES revision and GP/HS omap detection. And that's
> very simple code compared to the current cpu_is_omapxxxx stuff.

In fact only the tap should be required at map_io time to get the 
DEVICE_ID, I guess that the others entries could be retrieve later?

           .sdrc   = OMAP243X_SDRC_BASE,
           .sms    = OMAP243X_SMS_BASE,
           .ctrl   = OMAP243X_CTRL_BASE,
           .prm    = OMAP2430_PRM_BASE,
           .cm     = OMAP2430_CM_BASE,

If this is the case, we can then add a DEVICE_ID address entry in DT 
root to be retrieved easily with the FDT.

> Then we can boot with the DT information pretty far to map_io
> and initialize low-level serial console, and then set up things
> further while having a decent console ;)

Yeah! The holy grail.

Benoit
Tony Lindgren Sept. 2, 2011, 12:32 p.m. UTC | #3
* Cousson, Benoit <b-cousson@ti.com> [110902 14:47]:
> >         .tap    = OMAP2_L4_IO_ADDRESS(0x4900a000),
> 
> BTW, how can we get rid of that OMAP2_L4_IO_ADDRESS?

That needs to be mapped early for SoC detection to work
for cpu_is_omapxxxx stuff :(
 
> In fact only the tap should be required at map_io time to get the
> DEVICE_ID, I guess that the others entries could be retrieve later?

Yes well whatever needs to be mapped early for the SoC
revision detection. It could be that we can map timers
and serial port only early, then set up the mappings for
the rest later on. That is if we can avoid SoC and revision
detection early.

Tony
diff mbox

Patch

diff --git a/arch/arm/mach-omap2/common.c b/arch/arm/mach-omap2/common.c
index 3f20cbb..ee5a225 100644
--- a/arch/arm/mach-omap2/common.c
+++ b/arch/arm/mach-omap2/common.c
@@ -16,6 +16,7 @@ 
 #include <linux/init.h>
 #include <linux/clk.h>
 #include <linux/io.h>
+#include <linux/of_fdt.h>
 
 #include <plat/common.h>
 #include <plat/board.h>
@@ -140,3 +141,75 @@  void __init omap2_set_globals_443x(void)
 }
 #endif
 
+#ifdef CONFIG_OF
+/*
+ * DT method to get the proper globals configuration
+ */
+static const struct of_device_id omap_soc_match[] = {
+#if defined(CONFIG_ARCH_OMAP4)
+        {
+                .compatible = "ti,omap4",
+                .data = &omap4_globals,
+        },
+#endif
+#if defined(CONFIG_ARCH_OMAP3)
+        {
+                .compatible = "ti,omap3",
+                .data = &omap3_globals,
+        },
+#endif
+#if defined(CONFIG_SOC_OMAP2430)
+        {
+                .compatible = "ti,omap2430",
+                .data = &omap243x_globals,
+        },
+#endif
+#if defined(CONFIG_SOC_OMAP2420)
+        {
+                .compatible = "ti,omap2420",
+                .data = &omap242x_globals,
+        },
+#endif
+	{},
+};
+
+/* Helper to find the proper of_device_id from a FDT */
+static __init const struct of_device_id *_fdt_match_node(
+					const struct of_device_id *matches,
+					int fdt_node)
+{
+	unsigned int score = 0;
+
+	if (!matches)
+		return NULL;
+
+	while (matches->compatible[0]) {
+		score = of_flat_dt_is_compatible(fdt_node, matches->compatible);
+		if (score > 0)
+			return matches;
+		matches++;
+	}
+	return NULL;
+}
+
+void __init omap2_set_globals(void)
+{
+        unsigned long fdt_root;
+	const struct of_device_id *match;
+
+	/* the DT is still flat at map_io time... */
+        fdt_root = of_get_flat_dt_root();
+	match = _fdt_match_node(omap_soc_match, fdt_root);
+	if (!match) {
+		pr_err("%s(): Cannot initialize the platform globals data\n",
+		       __func__);
+	}
+
+	pr_info("%s(): %s\n", __func__, match->compatible);
+
+	omap2_set_globals_tap(match->data);
+	omap2_set_globals_sdrc(match->data);
+	omap2_set_globals_control(match->data);
+	omap2_set_globals_prcm(match->data);
+}
+#endif
diff --git a/arch/arm/plat-omap/include/plat/common.h b/arch/arm/plat-omap/include/plat/common.h
index 4564cc6..fb79269 100644
--- a/arch/arm/plat-omap/include/plat/common.h
+++ b/arch/arm/plat-omap/include/plat/common.h
@@ -67,6 +67,7 @@  void omap2_set_globals_243x(void);
 void omap2_set_globals_3xxx(void);
 void omap2_set_globals_443x(void);
 void omap2_set_globals_ti816x(void);
+void omap2_set_globals(void);
 
 /* These get called from omap2_set_globals_xxxx(), do not call these */
 void omap2_set_globals_tap(struct omap_globals *);