Hi Benoit,
On Fri, Aug 16, 2013 at 03:15:57PM +0100, Benoit Cousson wrote:
> Hi Gururaja,
>
> On 16/08/2013 13:36, Hebbar, Gururaja wrote:
> > The syntax of compatible property in DT is to mention the Most specific
> > match to most generic match.
> >
> > Since AM335x is the platform with latest IP revision, add it 1st in
> > the device id table.
>
> I don't understand why? The order should not matter at all.
>
> I've tried to follow the thread you had with Mark on the v2, but AFAIK,
> you've never answered to his latest question.
>
> Moreover, checking the differences between the Davinci and the am3352
> RTC IP, I would not claim that both are compatible.
>
> Sure you can use the am3352 with the Davinci driver, but you will lose
> the wakeup functionality without even being notify about that.
Could you describe the wakeup functionality, and how it differs between
the am3352-rtc and the da830-rtc?
As I understand it, the am3352 functionality is a superset of the da830
functionality. You can use the old driver, and get some functionality,
or use the new driver and get it all.
That means that am3352-rtc is compatible with da830. As long as the
kernel first checks for am3352-rtc, there will be *no* loss of
functionality. All this does is enable older kernels to use the hardware
in some fashion, and given the older kernel didn't have support for the
am3352-rtc features, this is a *gain* in functionality, not a loss.
>
> For my point of view, compatible mean that the HW will still be fully
> functional with both versions of the driver, which is not the case here.
What? A driver for any entry in the compatible list should be able to
drive the hardware to *some* level of functionality. We list from
most-specific to most-general to allow a graceful degradation from fully
supported to bare minimum functionality.
>
> am3352 DTS must use the ti,am3352-rtc to have the expected behavior.
> Using the ti,da830-rtc version will not make the board working as
> expected. So we cannot claim the compatibility.
Please can you explain what you mean by "expected behaviour"?
>
> > This way, we can add new matching compatible as 1st and maintain old
> > compatible string for backwards compatibility.
> >
> > ex:
> > compatible = "ti,am3352-rtc", "ti,da830-rtc";
> >
> > Signed-off-by: Hebbar, Gururaja <gururaja.hebbar@xxxxxx>
> > CC: mark.rutland@xxxxxxx
> > ---
> > Changes in v3:
> > - new patch
> >
> > :100644 100644 dc62cc3... 2f0968c... M drivers/rtc/rtc-omap.c
> > drivers/rtc/rtc-omap.c | 6 +++---
> > 1 files changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/rtc/rtc-omap.c b/drivers/rtc/rtc-omap.c
> > index dc62cc3..2f0968c 100644
> > --- a/drivers/rtc/rtc-omap.c
> > +++ b/drivers/rtc/rtc-omap.c
> > @@ -330,12 +330,12 @@ static struct platform_device_id omap_rtc_devtype[] = {
> > MODULE_DEVICE_TABLE(platform, omap_rtc_devtype);
> >
> > static const struct of_device_id omap_rtc_of_match[] = {
> > - { .compatible = "ti,da830-rtc",
> > - .data = &omap_rtc_devtype[OMAP_RTC_DATA_DA830_IDX],
> > - },
> > { .compatible = "ti,am3352-rtc",
> > .data = &omap_rtc_devtype[OMAP_RTC_DATA_AM335X_IDX],
> > },
> > + { .compatible = "ti,da830-rtc",
> > + .data = &omap_rtc_devtype[OMAP_RTC_DATA_DA830_IDX],
> > + },
> > {},
> > };
> > MODULE_DEVICE_TABLE(of, omap_rtc_of_match);
>
> Bottom-line, if you get rid of the old compatible entry, you will not
> have to play some trick with the entries order.
I don't think it's playing tricks. It's ensuring compatibility, and it's
a simple change in ordering.
Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
