Hi Jon,

On 5/30/2012 5:03 PM, Jon Hunter wrote:

Hi Benoit,

On 05/30/2012 02:54 AM, Cousson, Benoit wrote:

quoted

On 5/30/2012 5:18 AM, Xiao Jiang wrote:

quoted

Jon Hunter wrote:

quoted

On 05/25/2012 05:42 AM, jgq516 at gmail.com wrote:

quoted

From: Xiao Jiang<redacted>

Add device table for omap_wdt to support dt.

Signed-off-by: Xiao Jiang<redacted>
---
drivers/watchdog/omap_wdt.c | 8 ++++++++
1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/drivers/watchdog/omap_wdt.c b/drivers/watchdog/omap_wdt.c
index 8285d65..d98c615 100644
--- a/drivers/watchdog/omap_wdt.c
+++ b/drivers/watchdog/omap_wdt.c

@@ -430,6 +430,13 @@ static int omap_wdt_resume(struct

platform_device *pdev)
#define omap_wdt_resume NULL
#endif

+static const struct of_device_id omap_wdt_of_match[] = {
+ { .compatible = "ti,omap3-wdt", },
+ { .compatible = "ti,omap4-wdt", },

If there is no difference between the OMAP3 and the OMAP4 WDT IP, just
add one entry "ti,omap3-wdt". And then in the OMAP4 DTS you will just
put : compatible = "ti,omap3-wdt"; or compatible =  "ti,omap4-wdt",
"ti,omap3-wdt";

Hmmm ... comparing the omap3 and omap4 wdt registers there are some
differences. omap4 seems to have more registers than omap3. May be we
are not using these right now, but from a register perspective the wdt
in omap2, omap3 and omap4 appear to be slightly different. The revision
ID register on omap3 and omap4 have different values too.

I guess from a driver perspective there is no difference, but it seemed
to me that the IP is not completely the same.

Well, in that case, and assuming that there is no proper HW_REVISION 
information to detect the IP difference, the proper compatible entries 
will indeed have to be used.

quoted

I'm still a little bit confused about the real need for the
"ti,omap4-wdt: entry, but it seems to be the way to do it in PPC.

quoted

quoted

quoted

+ {},
+};
+MODULE_DEVICE_TABLE(of, omap_wdt_of_match);
+
static struct platform_driver omap_wdt_driver = {
.probe = omap_wdt_probe,
.remove = __devexit_p(omap_wdt_remove),

@@ -439,6 +446,7 @@ static struct platform_driver omap_wdt_driver = {

.driver = {
.owner = THIS_MODULE,
.name = "omap_wdt",
+ .of_match_table = omap_wdt_of_match,
},
};

I think we need to add some code to the probe function that calls
of_match_device() and ensures we find a match. For example ...

if (of_have_populated_dt())
if (!of_match_device(omap_wdt_of_match,&pdev->dev))
return -EINVAL;

Will add it in v2, thanks for suggestion.

No, in fact this is not needed. We need that mainly when several
instances can match the same driver and thus we select the proper one
using the of_match_device. Otherwise, just check is the device_node is
there.

In that case, the driver does not even care about any DT node so there
is no need to add extra code for that. Keep it simple.

Ok. So are you saying get rid of the match table altogether? In other
words, drop this patch?

No, the match table is used by the LDM to find the proper driver to be 
bound to a device. So we do need it. But we do not have to use the 
of_match_device if we do not want to get the entry in the device table.

I agree that it does not really do anything today, but I did not know if
in the future you were planning to pass things like, register addresses,
via DT.

Well, yes we will have to, otherwise people will keep complaining that 
our DTS sucks and are not compliant with the DTS standards :-)

Regards,
Benoit