Re: [PATCH] rtc: rtc-at91sam9.c add DT support

Re: [PATCH] rtc: rtc-at91sam9.c add DT support

[Nicolas Ferre]

On 04/04/2013 05:54 AM, Douglas Gilbert :
> Some members of the at91 SoCs use the Real Time Timer (RTT)
> and the General Purpose Backup Registers (GPBR) to implement
> a real time clock (RTC). The AT91SAM9G20 is one example.
> 
> Attached is a patch to add DT support to rtc-at91sam9.c .
> The patch is against lk 3.9.0-rc5 .
> 
> Below is a snippet of DT code for the 'G20 that was observed
> to work with this patch:
> 
> ahb {
>     apb {
> 
>         rtc {
>             compatible = "atmel,at91sam9-rtc";

The compatible string has to be formed by the name of the first SoC
compatible with this IP. It turns to be the at91sam9260.
The second part of the string should be a name that reflects the nature
of the peripheral. For this binding, I would like to mention the "RTT"
in the compatibility string (because other drivers can use other RTT
with other uses).

What do you think about:
"atmel,at91sam9260-rtt-as-rtc"? or something shorter?

>                          /* RTTC followed by GPBR (backup registers) */
>                          reg = <0xfffffd20 0x10>, <0xfffffd50 0x10>;
>                          interrupts = <1 4 7>;
>                          status = "okay";

Last, but not least, when we add a DT binding, it is a requirement to
add the corresponding documentation in the
Documentation/devicetree/bindings/rtc/ directory.

>         };
>     }
> };
> 
> Signed-off-by: Douglas Gilbert <dgilbert-qazKcTl6WRFWk0Htik3J/w@public.gmane.org>

Thanks, best regards,
-- 
Nicolas Ferre

Re: [PATCH] rtc: rtc-at91sam9.c add DT support

[Douglas Gilbert]

On 13-04-04 04:11 AM, Nicolas Ferre wrote:
> On 04/04/2013 05:54 AM, Douglas Gilbert :
>> Some members of the at91 SoCs use the Real Time Timer (RTT)
>> and the General Purpose Backup Registers (GPBR) to implement
>> a real time clock (RTC). The AT91SAM9G20 is one example.
>>
>> Attached is a patch to add DT support to rtc-at91sam9.c .
>> The patch is against lk 3.9.0-rc5 .
>>
>> Below is a snippet of DT code for the 'G20 that was observed
>> to work with this patch:
>>
>> ahb {
>>      apb {
>>
>>          rtc {
>>              compatible = "atmel,at91sam9-rtc";
>
> The compatible string has to be formed by the name of the first SoC
> compatible with this IP. It turns to be the at91sam9260.
> The second part of the string should be a name that reflects the nature
> of the peripheral. For this binding, I would like to mention the "RTT"
> in the compatibility string (because other drivers can use other RTT
> with other uses).
>
> What do you think about:
> "atmel,at91sam9260-rtt-as-rtc"? or something shorter?

Hi Nicolas,
Johan Hovold suggested:
    atmel,at91sam9260-rtt

I notice (in the G20 doco) that the acronym RTTC is also used
for the rtt registers. What do you want?

>
>>                           /* RTTC followed by GPBR (backup registers) */
>>                           reg = <0xfffffd20 0x10>, <0xfffffd50 0x10>;
>>                           interrupts = <1 4 7>;
>>                           status = "okay";
>
> Last, but not least, when we add a DT binding, it is a requirement to
> add the corresponding documentation in the
> Documentation/devicetree/bindings/rtc/ directory.

I have been underwhelmed by the accuracy and the organisation
of information in that documentation. And the examples are
often misleading given the actual hierarchy of real dtsi/dts
config files.
Give me working examples any day. You could (and should) test
what I gave on a g20ek board.

Also I note there is no "bindings" documentation for
rtc-at91rm9200.c :-) After you, sir ....

Doug Gilbert

Re: [PATCH] rtc: rtc-at91sam9.c add DT support

[Nicolas Ferre]

On 04/04/2013 03:25 PM, Douglas Gilbert :
> On 13-04-04 04:11 AM, Nicolas Ferre wrote:
>> On 04/04/2013 05:54 AM, Douglas Gilbert :
>>> Some members of the at91 SoCs use the Real Time Timer (RTT)
>>> and the General Purpose Backup Registers (GPBR) to implement
>>> a real time clock (RTC). The AT91SAM9G20 is one example.
>>>
>>> Attached is a patch to add DT support to rtc-at91sam9.c .
>>> The patch is against lk 3.9.0-rc5 .
>>>
>>> Below is a snippet of DT code for the 'G20 that was observed
>>> to work with this patch:
>>>
>>> ahb {
>>>      apb {
>>>
>>>          rtc {
>>>              compatible = "atmel,at91sam9-rtc";
>>
>> The compatible string has to be formed by the name of the first SoC
>> compatible with this IP. It turns to be the at91sam9260.
>> The second part of the string should be a name that reflects the nature
>> of the peripheral. For this binding, I would like to mention the "RTT"
>> in the compatibility string (because other drivers can use other RTT
>> with other uses).
>>
>> What do you think about:
>> "atmel,at91sam9260-rtt-as-rtc"? or something shorter?
> 
> Hi Nicolas,
> Johan Hovold suggested:
>    atmel,at91sam9260-rtt

Yes, but I fear this could bring confusion if someone is building a RTT
driver that is not targeted at acting as a RTC...

> I notice (in the G20 doco) that the acronym RTTC is also used

The "C" at the end stands for "Controller" (not very useful convention
in my opinion).

> for the rtt registers. What do you want?

I found xxx-rtt-as-rtc or xxx-rtt-rtc but not completely satisfied with
any of them...

>>>                           /* RTTC followed by GPBR (backup registers) */
>>>                           reg = <0xfffffd20 0x10>, <0xfffffd50 0x10>;
>>>                           interrupts = <1 4 7>;
>>>                           status = "okay";
>>
>> Last, but not least, when we add a DT binding, it is a requirement to
>> add the corresponding documentation in the
>> Documentation/devicetree/bindings/rtc/ directory.
> 
> I have been underwhelmed by the accuracy and the organisation
> of information in that documentation. And the examples are
> often misleading given the actual hierarchy of real dtsi/dts
> config files.

Oh, really? I will have a look one of those days...


> Give me working examples any day. You could (and should) test
> what I gave on a g20ek board.
> 
> Also I note there is no "bindings" documentation for
> rtc-at91rm9200.c :-) After you, sir ....

Already submitted, my dear ;-)
Here:
http://ozlabs.org/~akpm/mmots/broken-out/drivers-rtc-rtc-at91rm9200c-add-dt-support.patch

Bye,
-- 
Nicolas Ferre

Re: [PATCH] rtc: rtc-at91sam9.c add DT support

[Douglas Gilbert]

On 13-04-04 10:08 AM, Nicolas Ferre wrote:
> On 04/04/2013 03:25 PM, Douglas Gilbert :
>> On 13-04-04 04:11 AM, Nicolas Ferre wrote:
>>> On 04/04/2013 05:54 AM, Douglas Gilbert :
>>>> Some members of the at91 SoCs use the Real Time Timer (RTT)
>>>> and the General Purpose Backup Registers (GPBR) to implement
>>>> a real time clock (RTC). The AT91SAM9G20 is one example.
>>>>
>>>> Attached is a patch to add DT support to rtc-at91sam9.c .
>>>> The patch is against lk 3.9.0-rc5 .
>>>>
>>>> Below is a snippet of DT code for the 'G20 that was observed
>>>> to work with this patch:
>>>>
>>>> ahb {
>>>>       apb {
>>>>
>>>>           rtc {
>>>>               compatible = "atmel,at91sam9-rtc";
>>>
>>> The compatible string has to be formed by the name of the first SoC
>>> compatible with this IP. It turns to be the at91sam9260.
>>> The second part of the string should be a name that reflects the nature
>>> of the peripheral. For this binding, I would like to mention the "RTT"
>>> in the compatibility string (because other drivers can use other RTT
>>> with other uses).
>>>
>>> What do you think about:
>>> "atmel,at91sam9260-rtt-as-rtc"? or something shorter?
>>
>> Hi Nicolas,
>> Johan Hovold suggested:
>>     atmel,at91sam9260-rtt
>
> Yes, but I fear this could bring confusion if someone is building a RTT
> driver that is not targeted at acting as a RTC...
>
>> I notice (in the G20 doco) that the acronym RTTC is also used
>
> The "C" at the end stands for "Controller" (not very useful convention
> in my opinion).
>
>> for the rtt registers. What do you want?
>
> I found xxx-rtt-as-rtc or xxx-rtt-rtc but not completely satisfied with
> any of them...
>
>>>>                            /* RTTC followed by GPBR (backup registers) */
>>>>                            reg = <0xfffffd20 0x10>, <0xfffffd50 0x10>;
>>>>                            interrupts = <1 4 7>;
>>>>                            status = "okay";
>>>
>>> Last, but not least, when we add a DT binding, it is a requirement to
>>> add the corresponding documentation in the
>>> Documentation/devicetree/bindings/rtc/ directory.
>>
>> I have been underwhelmed by the accuracy and the organisation
>> of information in that documentation. And the examples are
>> often misleading given the actual hierarchy of real dtsi/dts
>> config files.
>
> Oh, really? I will have a look one of those days...
>
>
>> Give me working examples any day. You could (and should) test
>> what I gave on a g20ek board.
>>
>> Also I note there is no "bindings" documentation for
>> rtc-at91rm9200.c :-) After you, sir ....
>
> Already submitted, my dear ;-)
> Here:
> http://ozlabs.org/~akpm/mmots/broken-out/drivers-rtc-rtc-at91rm9200c-add-dt-support.patch

Good.

On a more serious note, looking at a dts file for working
hardware is useful _but_ it is not clear how a kernel should
be configured to fully utilize that dts file. Without a
defconfig a user may need to resort to several searches like
this:

   find <kernel_src> -name '*.c' -exec grep "atmel,at91sam9x5-rtc" {} \; -print

which is non-obvious to the novice. After a line like
that, the Makefile in that directory may need to be checked
to find out which CONFIG_ option needs to be set to get
the associated driver built in (or a module?). So is there
a policy _not_ to place "compat" strings in Kconfig files
where they might be searchable?

The "bindings" documentation and DT incantations are all
for nought if the associated driver is not configured.
Looking at the dmesg output is futile.

Doug Gilbert

Re: [PATCH] rtc: rtc-at91sam9.c add DT support

[Johan Hovold]

On Thu, Apr 04, 2013 at 04:08:04PM +0200, Nicolas Ferre wrote:
> On 04/04/2013 03:25 PM, Douglas Gilbert :
> > On 13-04-04 04:11 AM, Nicolas Ferre wrote:
> >> On 04/04/2013 05:54 AM, Douglas Gilbert :
> >>> Some members of the at91 SoCs use the Real Time Timer (RTT)
> >>> and the General Purpose Backup Registers (GPBR) to implement
> >>> a real time clock (RTC). The AT91SAM9G20 is one example.
> >>>
> >>> Attached is a patch to add DT support to rtc-at91sam9.c .
> >>> The patch is against lk 3.9.0-rc5 .
> >>>
> >>> Below is a snippet of DT code for the 'G20 that was observed
> >>> to work with this patch:
> >>>
> >>> ahb {
> >>>      apb {
> >>>
> >>>          rtc {
> >>>              compatible = "atmel,at91sam9-rtc";
> >>
> >> The compatible string has to be formed by the name of the first SoC
> >> compatible with this IP. It turns to be the at91sam9260.
> >> The second part of the string should be a name that reflects the nature
> >> of the peripheral. For this binding, I would like to mention the "RTT"
> >> in the compatibility string (because other drivers can use other RTT
> >> with other uses).
> >>
> >> What do you think about:
> >> "atmel,at91sam9260-rtt-as-rtc"? or something shorter?
> > 
> > Hi Nicolas,
> > Johan Hovold suggested:
> >    atmel,at91sam9260-rtt
> 
> Yes, but I fear this could bring confusion if someone is building a RTT
> driver that is not targeted at acting as a RTC...

But isn't the compatible property supposed to describe the peripheral in
a completely platform-agnostic way?

In fact, it seems it should be on the form

	compatible = "atmel,at91sam9g45-rtt", "atmel,at91sam9260-rtt";

where an exact description (the actual SoC-IP) is followed by a more
generic compatible description (e.g. the first SoC that introduced the
IP that is still compatible) (see [1]). This policy seems not have been
followed for at91 though, as only the second (first compatible SoC)
string is generally used. 

Either way, this still leaves open the question of which driver gets
bound to a device if there are more than one driver that matches the
compatible property. I guess this could be solved with a property set
or overridden by the board description. For example:

at91sam9g45.dtsi:

	rtt@fffffd20 {
		compatible = "atmel,at91sam9g45-rtt", "atmel,at91sam9260-rtt";
		reg = <0xfffffd20 0x10>;
		interrupts = <1 4 7>;
		status = "disabled";
	};

at91sam9m10g45ek.dts:

	rtt@fffffd20 {
		atmel,at91-rtt-as-rtc;
		status = "okay";
	};

If the device-specific property atmel,at91-rtt-as-rtc is missing,
the rtc-at91sam9 driver probe should fail so that any other matching
driver is probed. Of course, this will only work if the other matching
rtt-drivers also add device-specific properties so that they can be
disabled as well (should the probe order be reversed), for instance:

	atmel,at91-rtt-as-<xxx>;

> >>>                           /* RTTC followed by GPBR (backup registers) */
> >>>                           reg = <0xfffffd20 0x10>, <0xfffffd50 0x10>;
> >>>                           interrupts = <1 4 7>;
> >>>                           status = "okay";

Which backup register (GPBR) the RTC-time base should be stored in is
currently configurable using CONFIG_RTC_DRV_AT91SAM9_GPBR (and the
address arguably not even a property of the rtt) so the register address
should in any case not be set in the rtt-node of the dtsi.

Instead I think we need the arch code to map the backup-registers and
thus to add a new gpbr-node:

	gpbr@fffffd60 {
		compatible = "atmel,at91sam9g45-gpbr", "atmel,at91sam9260-gpbr";
		reg = <0xfffffd60 0x10>;
	};

We could then use CONFIG_RTC_DRV_AT91SAM9_GPBR for non-DT as before, and
add a new property (or simply replace the boolean) to the rtt-nodes in
the board descriptions:

	rtt@fffffd20 {
		atmel,at91-rtt-as-rtc;
		atmel,at91-rtt-as-rtc-gpbr = <n>;
		status = "okay";
	};

to select which of the four (sam9263 has 20) backup registers to use.

I'm responding to this mail with an RFC-implementing the GPBR and
RTT-as-RTC DT support as outlined above.

Finally, for the record: There are currently no other rtt-drivers, but I
have posted a patch series a while ago to fix hanged boot due to early
spurious RTT (or RTC) interrupts (see [2]). The series uses
"atmel,at91sam9260-rtt" to detect the SoCs that are affected by the
problem, and I guess that is an example of why the compatible string
should merely describe the peripheral (and not its use).

Thanks,
Johan

[1] http://www.devicetree.org/Device_Tree_Usage#Understanding_the_compatible_Property
[2] http://marc.info/?l=linux-arm-kernel&m=136302554825979&w=2

Re: [RFC 1/5] ARM: at91: add general purpose backup register (GPBR) support

[Jean-Christophe PLAGNIOL-VILLARD]

On 17:12 Sun 07 Apr     , Johan Hovold wrote:
> 
> Add General Purpose Backup Register (GPBR) support.
> 
> Most at91 SoCs have at least four 32-bit General Purpose Backup
> Registers (GPBR) powered by backup-power (VDDBU). One such register is
> currently used by rtc-at91sam9 driver to store the RTC time base.
> 
> Make sure the registers are mapped by arch setup code and add generic
> accessors for in-kernel use.
> 
> This is a step in adding device-tree support to the rtc-at91sam9 driver.
> 
This is a regression

we loose the tracking of what request and use the GPBR

Best Regsards,
J.
> Signed-off-by: Johan Hovold <jhovold-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  arch/arm/mach-at91/at91sam9260.c            |  1 +
>  arch/arm/mach-at91/at91sam9260_devices.c    | 15 +++--------
>  arch/arm/mach-at91/at91sam9261.c            |  1 +
>  arch/arm/mach-at91/at91sam9261_devices.c    | 15 +++--------
>  arch/arm/mach-at91/at91sam9263.c            |  1 +
>  arch/arm/mach-at91/at91sam9263_devices.c    | 18 +++----------
>  arch/arm/mach-at91/at91sam9g45.c            |  1 +
>  arch/arm/mach-at91/at91sam9g45_devices.c    | 15 +++--------
>  arch/arm/mach-at91/at91sam9rl.c             |  1 +
>  arch/arm/mach-at91/at91sam9rl_devices.c     | 15 +++--------
>  arch/arm/mach-at91/generic.h                |  3 +++
>  arch/arm/mach-at91/include/mach/at91_gpbr.h | 29 ++++++++++++++++++++
>  arch/arm/mach-at91/setup.c                  | 10 +++++++
>  drivers/rtc/rtc-at91sam9.c                  | 41 +++++++++--------------------
>  14 files changed, 76 insertions(+), 90 deletions(-)
>  create mode 100644 arch/arm/mach-at91/include/mach/at91_gpbr.h
> 
> diff --git a/arch/arm/mach-at91/at91sam9260.c b/arch/arm/mach-at91/at91sam9260.c
> index b67cd53..984b0d5 100644
> --- a/arch/arm/mach-at91/at91sam9260.c
> +++ b/arch/arm/mach-at91/at91sam9260.c
> @@ -334,6 +334,7 @@ static void __init at91sam9260_map_io(void)
>  
>  static void __init at91sam9260_ioremap_registers(void)
>  {
> +	at91_ioremap_gpbr(AT91SAM9260_BASE_GPBR, 0x10);
>  	at91_ioremap_shdwc(AT91SAM9260_BASE_SHDWC);
>  	at91_ioremap_rstc(AT91SAM9260_BASE_RSTC);
>  	at91_ioremap_ramc(0, AT91SAM9260_BASE_SDRAMC, 512);
> diff --git a/arch/arm/mach-at91/at91sam9260_devices.c b/arch/arm/mach-at91/at91sam9260_devices.c
> index eda8d16..10c71a8 100644
> --- a/arch/arm/mach-at91/at91sam9260_devices.c
> +++ b/arch/arm/mach-at91/at91sam9260_devices.c
> @@ -649,8 +649,6 @@ static struct resource rtt_resources[] = {
>  		.end	= AT91SAM9260_BASE_RTT + SZ_16 - 1,
>  		.flags	= IORESOURCE_MEM,
>  	}, {
> -		.flags	= IORESOURCE_MEM,
> -	}, {
>  		.flags  = IORESOURCE_IRQ,
>  	},
>  };
> @@ -666,16 +664,9 @@ static struct platform_device at91sam9260_rtt_device = {
>  static void __init at91_add_device_rtt_rtc(void)
>  {
>  	at91sam9260_rtt_device.name = "rtc-at91sam9";
> -	/*
> -	 * The second resource is needed:
> -	 * GPBR will serve as the storage for RTC time offset
> -	 */
> -	at91sam9260_rtt_device.num_resources = 3;
> -	rtt_resources[1].start = AT91SAM9260_BASE_GPBR +
> -				 4 * CONFIG_RTC_DRV_AT91SAM9_GPBR;
> -	rtt_resources[1].end = rtt_resources[1].start + 3;
> -	rtt_resources[2].start = NR_IRQS_LEGACY + AT91_ID_SYS;
> -	rtt_resources[2].end = NR_IRQS_LEGACY + AT91_ID_SYS;
> +	at91sam9260_rtt_device.num_resources = 2;
> +	rtt_resources[1].start = NR_IRQS_LEGACY + AT91_ID_SYS;
> +	rtt_resources[1].end = NR_IRQS_LEGACY + AT91_ID_SYS;
>  }
>  #else
>  static void __init at91_add_device_rtt_rtc(void)
> diff --git a/arch/arm/mach-at91/at91sam9261.c b/arch/arm/mach-at91/at91sam9261.c
> index 0204f4c..b234c5d 100644
> --- a/arch/arm/mach-at91/at91sam9261.c
> +++ b/arch/arm/mach-at91/at91sam9261.c
> @@ -278,6 +278,7 @@ static void __init at91sam9261_map_io(void)
>  
>  static void __init at91sam9261_ioremap_registers(void)
>  {
> +	at91_ioremap_gpbr(AT91SAM9261_BASE_GPBR, 0x10);
>  	at91_ioremap_shdwc(AT91SAM9261_BASE_SHDWC);
>  	at91_ioremap_rstc(AT91SAM9261_BASE_RSTC);
>  	at91_ioremap_ramc(0, AT91SAM9261_BASE_SDRAMC, 512);
> diff --git a/arch/arm/mach-at91/at91sam9261_devices.c b/arch/arm/mach-at91/at91sam9261_devices.c
> index 629ea5f..e8ff5e7 100644
> --- a/arch/arm/mach-at91/at91sam9261_devices.c
> +++ b/arch/arm/mach-at91/at91sam9261_devices.c
> @@ -618,8 +618,6 @@ static struct resource rtt_resources[] = {
>  		.end	= AT91SAM9261_BASE_RTT + SZ_16 - 1,
>  		.flags	= IORESOURCE_MEM,
>  	}, {
> -		.flags	= IORESOURCE_MEM,
> -	}, {
>  		.flags  = IORESOURCE_IRQ,
>  	}
>  };
> @@ -634,16 +632,9 @@ static struct platform_device at91sam9261_rtt_device = {
>  static void __init at91_add_device_rtt_rtc(void)
>  {
>  	at91sam9261_rtt_device.name = "rtc-at91sam9";
> -	/*
> -	 * The second resource is needed:
> -	 * GPBR will serve as the storage for RTC time offset
> -	 */
> -	at91sam9261_rtt_device.num_resources = 3;
> -	rtt_resources[1].start = AT91SAM9261_BASE_GPBR +
> -				 4 * CONFIG_RTC_DRV_AT91SAM9_GPBR;
> -	rtt_resources[1].end = rtt_resources[1].start + 3;
> -	rtt_resources[2].start = NR_IRQS_LEGACY + AT91_ID_SYS;
> -	rtt_resources[2].end = NR_IRQS_LEGACY + AT91_ID_SYS;
> +	at91sam9261_rtt_device.num_resources = 2;
> +	rtt_resources[1].start = NR_IRQS_LEGACY + AT91_ID_SYS;
> +	rtt_resources[1].end = NR_IRQS_LEGACY + AT91_ID_SYS;
>  }
>  #else
>  static void __init at91_add_device_rtt_rtc(void)
> diff --git a/arch/arm/mach-at91/at91sam9263.c b/arch/arm/mach-at91/at91sam9263.c
> index c0cbb81..42d09f5 100644
> --- a/arch/arm/mach-at91/at91sam9263.c
> +++ b/arch/arm/mach-at91/at91sam9263.c
> @@ -315,6 +315,7 @@ static void __init at91sam9263_map_io(void)
>  
>  static void __init at91sam9263_ioremap_registers(void)
>  {
> +	at91_ioremap_gpbr(AT91SAM9263_BASE_GPBR, 0x50);
>  	at91_ioremap_shdwc(AT91SAM9263_BASE_SHDWC);
>  	at91_ioremap_rstc(AT91SAM9263_BASE_RSTC);
>  	at91_ioremap_ramc(0, AT91SAM9263_BASE_SDRAMC0, 512);
> diff --git a/arch/arm/mach-at91/at91sam9263_devices.c b/arch/arm/mach-at91/at91sam9263_devices.c
> index 858c8aa..374fe56 100644
> --- a/arch/arm/mach-at91/at91sam9263_devices.c
> +++ b/arch/arm/mach-at91/at91sam9263_devices.c
> @@ -1014,8 +1014,6 @@ static struct resource rtt0_resources[] = {
>  		.end	= AT91SAM9263_BASE_RTT0 + SZ_16 - 1,
>  		.flags	= IORESOURCE_MEM,
>  	}, {
> -		.flags	= IORESOURCE_MEM,
> -	}, {
>  		.flags  = IORESOURCE_IRQ,
>  	}
>  };
> @@ -1032,8 +1030,6 @@ static struct resource rtt1_resources[] = {
>  		.end	= AT91SAM9263_BASE_RTT1 + SZ_16 - 1,
>  		.flags	= IORESOURCE_MEM,
>  	}, {
> -		.flags	= IORESOURCE_MEM,
> -	}, {
>  		.flags  = IORESOURCE_IRQ,
>  	}
>  };
> @@ -1052,18 +1048,14 @@ static void __init at91_add_device_rtt_rtc(void)
>  
>  	switch (CONFIG_RTC_DRV_AT91SAM9_RTT) {
>  	case 0:
> -		/*
> -		 * The second resource is needed only for the chosen RTT:
> -		 * GPBR will serve as the storage for RTC time offset
> -		 */
> -		at91sam9263_rtt0_device.num_resources = 3;
> +		at91sam9263_rtt0_device.num_resources = 2;
>  		at91sam9263_rtt1_device.num_resources = 1;
>  		pdev = &at91sam9263_rtt0_device;
>  		r = rtt0_resources;
>  		break;
>  	case 1:
>  		at91sam9263_rtt0_device.num_resources = 1;
> -		at91sam9263_rtt1_device.num_resources = 3;
> +		at91sam9263_rtt1_device.num_resources = 2;
>  		pdev = &at91sam9263_rtt1_device;
>  		r = rtt1_resources;
>  		break;
> @@ -1074,10 +1066,8 @@ static void __init at91_add_device_rtt_rtc(void)
>  	}
>  
>  	pdev->name = "rtc-at91sam9";
> -	r[1].start = AT91SAM9263_BASE_GPBR + 4 * CONFIG_RTC_DRV_AT91SAM9_GPBR;
> -	r[1].end = r[1].start + 3;
> -	r[2].start = NR_IRQS_LEGACY + AT91_ID_SYS;
> -	r[2].end = NR_IRQS_LEGACY + AT91_ID_SYS;
> +	r[1].start = NR_IRQS_LEGACY + AT91_ID_SYS;
> +	r[1].end = NR_IRQS_LEGACY + AT91_ID_SYS;
>  }
>  #else
>  static void __init at91_add_device_rtt_rtc(void)
> diff --git a/arch/arm/mach-at91/at91sam9g45.c b/arch/arm/mach-at91/at91sam9g45.c
> index b4968aa..4b9547f 100644
> --- a/arch/arm/mach-at91/at91sam9g45.c
> +++ b/arch/arm/mach-at91/at91sam9g45.c
> @@ -361,6 +361,7 @@ static void __init at91sam9g45_map_io(void)
>  
>  static void __init at91sam9g45_ioremap_registers(void)
>  {
> +	at91_ioremap_gpbr(AT91SAM9G45_BASE_GPBR, 0x10);
>  	at91_ioremap_shdwc(AT91SAM9G45_BASE_SHDWC);
>  	at91_ioremap_rstc(AT91SAM9G45_BASE_RSTC);
>  	at91_ioremap_ramc(0, AT91SAM9G45_BASE_DDRSDRC1, 512);
> diff --git a/arch/arm/mach-at91/at91sam9g45_devices.c b/arch/arm/mach-at91/at91sam9g45_devices.c
> index fe626d4..65d700b 100644
> --- a/arch/arm/mach-at91/at91sam9g45_devices.c
> +++ b/arch/arm/mach-at91/at91sam9g45_devices.c
> @@ -1290,8 +1290,6 @@ static struct resource rtt_resources[] = {
>  		.end	= AT91SAM9G45_BASE_RTT + SZ_16 - 1,
>  		.flags	= IORESOURCE_MEM,
>  	}, {
> -		.flags	= IORESOURCE_MEM,
> -	}, {
>  		.flags  = IORESOURCE_IRQ,
>  	}
>  };
> @@ -1306,16 +1304,9 @@ static struct platform_device at91sam9g45_rtt_device = {
>  static void __init at91_add_device_rtt_rtc(void)
>  {
>  	at91sam9g45_rtt_device.name = "rtc-at91sam9";
> -	/*
> -	 * The second resource is needed:
> -	 * GPBR will serve as the storage for RTC time offset
> -	 */
> -	at91sam9g45_rtt_device.num_resources = 3;
> -	rtt_resources[1].start = AT91SAM9G45_BASE_GPBR +
> -				 4 * CONFIG_RTC_DRV_AT91SAM9_GPBR;
> -	rtt_resources[1].end = rtt_resources[1].start + 3;
> -	rtt_resources[2].start = NR_IRQS_LEGACY + AT91_ID_SYS;
> -	rtt_resources[2].end = NR_IRQS_LEGACY + AT91_ID_SYS;
> +	at91sam9g45_rtt_device.num_resources = 2;
> +	rtt_resources[1].start = NR_IRQS_LEGACY + AT91_ID_SYS;
> +	rtt_resources[1].end = NR_IRQS_LEGACY + AT91_ID_SYS;
>  }
>  #else
>  static void __init at91_add_device_rtt_rtc(void)
> diff --git a/arch/arm/mach-at91/at91sam9rl.c b/arch/arm/mach-at91/at91sam9rl.c
> index 3de3e04..bf7e555 100644
> --- a/arch/arm/mach-at91/at91sam9rl.c
> +++ b/arch/arm/mach-at91/at91sam9rl.c
> @@ -281,6 +281,7 @@ static void __init at91sam9rl_map_io(void)
>  
>  static void __init at91sam9rl_ioremap_registers(void)
>  {
> +	at91_ioremap_gpbr(AT91SAM9RL_BASE_GPBR, 0x10);
>  	at91_ioremap_shdwc(AT91SAM9RL_BASE_SHDWC);
>  	at91_ioremap_rstc(AT91SAM9RL_BASE_RSTC);
>  	at91_ioremap_ramc(0, AT91SAM9RL_BASE_SDRAMC, 512);
> diff --git a/arch/arm/mach-at91/at91sam9rl_devices.c b/arch/arm/mach-at91/at91sam9rl_devices.c
> index 352468f..533948c 100644
> --- a/arch/arm/mach-at91/at91sam9rl_devices.c
> +++ b/arch/arm/mach-at91/at91sam9rl_devices.c
> @@ -687,8 +687,6 @@ static struct resource rtt_resources[] = {
>  		.end	= AT91SAM9RL_BASE_RTT + SZ_16 - 1,
>  		.flags	= IORESOURCE_MEM,
>  	}, {
> -		.flags	= IORESOURCE_MEM,
> -	}, {
>  		.flags  = IORESOURCE_IRQ,
>  	}
>  };
> @@ -703,16 +701,9 @@ static struct platform_device at91sam9rl_rtt_device = {
>  static void __init at91_add_device_rtt_rtc(void)
>  {
>  	at91sam9rl_rtt_device.name = "rtc-at91sam9";
> -	/*
> -	 * The second resource is needed:
> -	 * GPBR will serve as the storage for RTC time offset
> -	 */
> -	at91sam9rl_rtt_device.num_resources = 3;
> -	rtt_resources[1].start = AT91SAM9RL_BASE_GPBR +
> -				 4 * CONFIG_RTC_DRV_AT91SAM9_GPBR;
> -	rtt_resources[1].end = rtt_resources[1].start + 3;
> -	rtt_resources[2].start = NR_IRQS_LEGACY + AT91_ID_SYS;
> -	rtt_resources[2].end = NR_IRQS_LEGACY + AT91_ID_SYS;
> +	at91sam9rl_rtt_device.num_resources = 2;
> +	rtt_resources[1].start = NR_IRQS_LEGACY + AT91_ID_SYS;
> +	rtt_resources[1].end = NR_IRQS_LEGACY + AT91_ID_SYS;
>  }
>  #else
>  static void __init at91_add_device_rtt_rtc(void)
> diff --git a/arch/arm/mach-at91/generic.h b/arch/arm/mach-at91/generic.h
> index fc593d6..6d444cc 100644
> --- a/arch/arm/mach-at91/generic.h
> +++ b/arch/arm/mach-at91/generic.h
> @@ -59,6 +59,9 @@ extern void at91_irq_resume(void);
>  /* idle */
>  extern void at91sam9_idle(void);
>  
> +/* backup registers */
> +extern void at91_ioremap_gpbr(u32 base_addr, u32 size);
> +
>  /* reset */
>  extern void at91_ioremap_rstc(u32 base_addr);
>  extern void at91sam9_alt_restart(char, const char *);
> diff --git a/arch/arm/mach-at91/include/mach/at91_gpbr.h b/arch/arm/mach-at91/include/mach/at91_gpbr.h
> new file mode 100644
> index 0000000..f4b4895
> --- /dev/null
> +++ b/arch/arm/mach-at91/include/mach/at91_gpbr.h
> @@ -0,0 +1,29 @@
> +/*
> + * arch/arm/mach-at91/include/mach/at91_gpbr.h
> + *
> + * Copyright (C) 2013 Johan Hovold <jhovold-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> + *
> + * General Purpose Backup Registers (GPBR)
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#ifndef AT91_GPBR_H
> +#define AT91_GPBR_H
> +
> +extern void __iomem *at91_gpbr_base;
> +
> +static inline u32 at91_gpbr_read(unsigned nr)
> +{
> +	return __raw_readl(at91_gpbr_base + ((nr) << 2));
> +}
> +
> +static inline void at91_gpbr_write(unsigned nr, u32 value)
> +{
> +	__raw_writel(value, at91_gpbr_base + ((nr) << 2));
> +}
> +
> +#endif	/* AT91_GPBR_H */
> diff --git a/arch/arm/mach-at91/setup.c b/arch/arm/mach-at91/setup.c
> index 9e7c1e1..b01f4aa 100644
> --- a/arch/arm/mach-at91/setup.c
> +++ b/arch/arm/mach-at91/setup.c
> @@ -18,6 +18,7 @@
>  #include <mach/hardware.h>
>  #include <mach/cpu.h>
>  #include <mach/at91_dbgu.h>
> +#include <mach/at91_gpbr.h>
>  #include <mach/at91_pmc.h>
>  
>  #include "at91_shdwc.h"
> @@ -277,6 +278,15 @@ void __init at91_map_io(void)
>  		at91_boot_soc.map_io();
>  }
>  
> +void __iomem *at91_gpbr_base;
> +
> +void __init at91_ioremap_gpbr(u32 base_addr, u32 size)
> +{
> +	at91_gpbr_base = ioremap(base_addr, size);
> +	if (!at91_gpbr_base)
> +		panic("AT91: Failed to ioremap gpbr registers\n");
> +}
> +
>  void __iomem *at91_shdwc_base = NULL;
>  
>  static void at91sam9_poweroff(void)
> diff --git a/drivers/rtc/rtc-at91sam9.c b/drivers/rtc/rtc-at91sam9.c
> index 39cfd2e..983667b 100644
> --- a/drivers/rtc/rtc-at91sam9.c
> +++ b/drivers/rtc/rtc-at91sam9.c
> @@ -21,6 +21,7 @@
>  #include <linux/slab.h>
>  #include <linux/platform_data/atmel.h>
>  
> +#include <mach/at91_gpbr.h>
>  #include <mach/at91_rtt.h>
>  #include <mach/cpu.h>
>  
> @@ -57,7 +58,7 @@ struct sam9_rtc {
>  	void __iomem		*rtt;
>  	struct rtc_device	*rtcdev;
>  	u32			imr;
> -	void __iomem		*gpbr;
> +	unsigned		gpbr;
>  	int 			irq;
>  };
>  
> @@ -66,11 +67,6 @@ struct sam9_rtc {
>  #define rtt_writel(rtc, field, val) \
>  	__raw_writel((val), (rtc)->rtt + AT91_RTT_ ## field)
>  
> -#define gpbr_readl(rtc) \
> -	__raw_readl((rtc)->gpbr)
> -#define gpbr_writel(rtc, val) \
> -	__raw_writel((val), (rtc)->gpbr)
> -
>  /*
>   * Read current time and date in RTC
>   */
> @@ -81,7 +77,7 @@ static int at91_rtc_readtime(struct device *dev, struct rtc_time *tm)
>  	u32 offset;
>  
>  	/* read current time offset */
> -	offset = gpbr_readl(rtc);
> +	offset = at91_gpbr_read(rtc->gpbr);
>  	if (offset == 0)
>  		return -EILSEQ;
>  
> @@ -124,11 +120,11 @@ static int at91_rtc_settime(struct device *dev, struct rtc_time *tm)
>  	rtt_writel(rtc, MR, mr & ~(AT91_RTT_ALMIEN | AT91_RTT_RTTINCIEN));
>  
>  	/* read current time offset */
> -	offset = gpbr_readl(rtc);
> +	offset = at91_gpbr_read(rtc->gpbr);
>  
>  	/* store the new base time in a battery backup register */
>  	secs += 1;
> -	gpbr_writel(rtc, secs);
> +	at91_gpbr_write(rtc->gpbr, secs);
>  
>  	/* adjust the alarm time for the new base */
>  	alarm = rtt_readl(rtc, AR);
> @@ -160,7 +156,7 @@ static int at91_rtc_readalarm(struct device *dev, struct rtc_wkalrm *alrm)
>  	u32 alarm = rtt_readl(rtc, AR);
>  	u32 offset;
>  
> -	offset = gpbr_readl(rtc);
> +	offset = at91_gpbr_read(rtc->gpbr);
>  	if (offset == 0)
>  		return -EILSEQ;
>  
> @@ -192,7 +188,7 @@ static int at91_rtc_setalarm(struct device *dev, struct rtc_wkalrm *alrm)
>  	if (err != 0)
>  		return err;
>  
> -	offset = gpbr_readl(rtc);
> +	offset = at91_gpbr_read(rtc->gpbr);
>  	if (offset == 0) {
>  		/* time is not set */
>  		return -EILSEQ;
> @@ -291,17 +287,14 @@ static const struct rtc_class_ops at91_rtc_ops = {
>   */
>  static int at91_rtc_probe(struct platform_device *pdev)
>  {
> -	struct resource	*r, *r_gpbr;
> +	struct resource	*r;
>  	struct sam9_rtc	*rtc;
>  	int		ret, irq;
>  	u32		mr;
>  
>  	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -	r_gpbr = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> -	if (!r || !r_gpbr) {
> -		dev_err(&pdev->dev, "need 2 ressources\n");
> -		return -ENODEV;
> -	}
> +	if (!r)
> +		return -ENXIO;
>  
>  	irq = platform_get_irq(pdev, 0);
>  	if (irq < 0) {
> @@ -327,19 +320,14 @@ static int at91_rtc_probe(struct platform_device *pdev)
>  		goto fail;
>  	}
>  
> -	rtc->gpbr = ioremap(r_gpbr->start, resource_size(r_gpbr));
> -	if (!rtc->gpbr) {
> -		dev_err(&pdev->dev, "failed to map gpbr registers, aborting.\n");
> -		ret = -ENOMEM;
> -		goto fail_gpbr;
> -	}
> +	rtc->gpbr = CONFIG_RTC_DRV_AT91SAM9_GPBR;
>  
>  	mr = rtt_readl(rtc, MR);
>  
>  	/* unless RTT is counting at 1 Hz, re-initialize it */
>  	if ((mr & AT91_RTT_RTPRES) != AT91_SLOW_CLOCK) {
>  		mr = AT91_RTT_RTTRST | (AT91_SLOW_CLOCK & AT91_RTT_RTPRES);
> -		gpbr_writel(rtc, 0);
> +		at91_gpbr_write(rtc->gpbr, 0);
>  	}
>  
>  	/* disable all interrupts (same as on shutdown path) */
> @@ -368,15 +356,13 @@ static int at91_rtc_probe(struct platform_device *pdev)
>  	 * clock, discrete RTC, etc
>  	 */
>  
> -	if (gpbr_readl(rtc) == 0)
> +	if (at91_gpbr_read(rtc->gpbr) == 0)
>  		dev_warn(&pdev->dev, "%s: SET TIME!\n",
>  				dev_name(&rtc->rtcdev->dev));
>  
>  	return 0;
>  
>  fail_register:
> -	iounmap(rtc->gpbr);
> -fail_gpbr:
>  	iounmap(rtc->rtt);
>  fail:
>  	platform_set_drvdata(pdev, NULL);
> @@ -398,7 +384,6 @@ static int at91_rtc_remove(struct platform_device *pdev)
>  
>  	rtc_device_unregister(rtc->rtcdev);
>  
> -	iounmap(rtc->gpbr);
>  	iounmap(rtc->rtt);
>  	platform_set_drvdata(pdev, NULL);
>  	kfree(rtc);
> -- 
> 1.8.1.5
> 

Re: [RFC 1/5] ARM: at91: add general purpose backup register (GPBR) support

[Johan Hovold]

On Mon, Apr 08, 2013 at 09:33:29AM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 17:12 Sun 07 Apr     , Johan Hovold wrote:
> > 
> > Add General Purpose Backup Register (GPBR) support.
> > 
> > Most at91 SoCs have at least four 32-bit General Purpose Backup
> > Registers (GPBR) powered by backup-power (VDDBU). One such register is
> > currently used by rtc-at91sam9 driver to store the RTC time base.
> > 
> > Make sure the registers are mapped by arch setup code and add generic
> > accessors for in-kernel use.
> > 
> > This is a step in adding device-tree support to the rtc-at91sam9 driver.
> > 
> This is a regression
> 
> we loose the tracking of what request and use the GPBR

Implementing the GPBR accessors using a simple global resource was one
quick way forward to decouple and generalise the GPBR. Of course, this
could be turned into a more elaborate framework or driver where
registers are requested and released by other drivers and possibly also
by user-space.

Thanks,
Johan

Re: [RFC 1/5] ARM: at91: add general purpose backup register (GPBR) support

[Jean-Christophe PLAGNIOL-VILLARD]

On 10:46 Mon 08 Apr     , Johan Hovold wrote:
> On Mon, Apr 08, 2013 at 09:33:29AM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > On 17:12 Sun 07 Apr     , Johan Hovold wrote:
> > > 
> > > Add General Purpose Backup Register (GPBR) support.
> > > 
> > > Most at91 SoCs have at least four 32-bit General Purpose Backup
> > > Registers (GPBR) powered by backup-power (VDDBU). One such register is
> > > currently used by rtc-at91sam9 driver to store the RTC time base.
> > > 
> > > Make sure the registers are mapped by arch setup code and add generic
> > > accessors for in-kernel use.
> > > 
> > > This is a step in adding device-tree support to the rtc-at91sam9 driver.
> > > 
> > This is a regression
> > 
> > we loose the tracking of what request and use the GPBR
> 
> Implementing the GPBR accessors using a simple global resource was one
> quick way forward to decouple and generalise the GPBR. Of course, this
> could be turned into a more elaborate framework or driver where
> registers are requested and released by other drivers and possibly also
> by user-space.
yes must as we used on SAMA5 to pass the boot source from the firstage of the boot
loader to the other software

Re: [RFC 1/5] ARM: at91: add general purpose backup register (GPBR) support

[Johan Hovold]

On Mon, Apr 08, 2013 at 12:04:17PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 10:46 Mon 08 Apr     , Johan Hovold wrote:
> > On Mon, Apr 08, 2013 at 09:33:29AM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > > On 17:12 Sun 07 Apr     , Johan Hovold wrote:
> > > > 
> > > > Add General Purpose Backup Register (GPBR) support.
> > > > 
> > > > Most at91 SoCs have at least four 32-bit General Purpose Backup
> > > > Registers (GPBR) powered by backup-power (VDDBU). One such register is
> > > > currently used by rtc-at91sam9 driver to store the RTC time base.
> > > > 
> > > > Make sure the registers are mapped by arch setup code and add generic
> > > > accessors for in-kernel use.
> > > > 
> > > > This is a step in adding device-tree support to the rtc-at91sam9 driver.
> > > > 
> > > This is a regression
> > > 
> > > we loose the tracking of what request and use the GPBR
> > 
> > Implementing the GPBR accessors using a simple global resource was one
> > quick way forward to decouple and generalise the GPBR. Of course, this
> > could be turned into a more elaborate framework or driver where
> > registers are requested and released by other drivers and possibly also
> > by user-space.
> yes must as we used on SAMA5 to pass the boot source from the firstage
> of the boot loader to the other software

Could you please elaborate on this; how is the GPBR used, how many
registers, and when is it accessed?

Is there already code in at91/at91-3.10-soc or are you referring to
a userspace consumer?

Thanks,
Johan

Re: [RFC 1/5] ARM: at91: add general purpose backup register (GPBR) support

[Jean-Christophe PLAGNIOL-VILLARD]

On 14:39 Thu 11 Apr     , Johan Hovold wrote:
> On Mon, Apr 08, 2013 at 12:04:17PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > On 10:46 Mon 08 Apr     , Johan Hovold wrote:
> > > On Mon, Apr 08, 2013 at 09:33:29AM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > > > On 17:12 Sun 07 Apr     , Johan Hovold wrote:
> > > > > 
> > > > > Add General Purpose Backup Register (GPBR) support.
> > > > > 
> > > > > Most at91 SoCs have at least four 32-bit General Purpose Backup
> > > > > Registers (GPBR) powered by backup-power (VDDBU). One such register is
> > > > > currently used by rtc-at91sam9 driver to store the RTC time base.
> > > > > 
> > > > > Make sure the registers are mapped by arch setup code and add generic
> > > > > accessors for in-kernel use.
> > > > > 
> > > > > This is a step in adding device-tree support to the rtc-at91sam9 driver.
> > > > > 
> > > > This is a regression
> > > > 
> > > > we loose the tracking of what request and use the GPBR
> > > 
> > > Implementing the GPBR accessors using a simple global resource was one
> > > quick way forward to decouple and generalise the GPBR. Of course, this
> > > could be turned into a more elaborate framework or driver where
> > > registers are requested and released by other drivers and possibly also
> > > by user-space.
> > yes must as we used on SAMA5 to pass the boot source from the firstage
> > of the boot loader to the other software
> 
> Could you please elaborate on this; how is the GPBR used, how many
> registers, and when is it accessed?

The second stage bootloader store the content of the boot mode passs via r4
from the Rom-Code in the 2nd gpbr
> 
> Is there already code in at91/at91-3.10-soc or are you referring to
> a userspace consumer?
no user you can see the code in barebox
you can see the code of barebox here
http://git.linupdate.org/git?p=barebox.git;a=shortlog;h=refs/heads/delivery/sama5-release-bootmode

Best Regards,
J.
> 
> Thanks,
> Johan

Re: [RFC 2/5] ARM: at91/dts: add general purpose backup register (GPBR) support

[Jean-Christophe PLAGNIOL-VILLARD]

On 17:12 Sun 07 Apr     , Johan Hovold wrote:
> Add arch device-tree setup code and DT-nodes for the General Purpose
> Backup Registers (GPBR).
> 
> Note that all AT91 SoCs but at91rm9200 have these backup-powered
> registers.
> 
> Signed-off-by: Johan Hovold <jhovold-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  .../devicetree/bindings/arm/atmel-at91.txt          |  4 ++++
>  arch/arm/boot/dts/at91sam9260.dtsi                  |  5 +++++
>  arch/arm/boot/dts/at91sam9263.dtsi                  |  5 +++++
>  arch/arm/boot/dts/at91sam9g45.dtsi                  |  5 +++++
>  arch/arm/boot/dts/at91sam9n12.dtsi                  |  5 +++++
>  arch/arm/boot/dts/at91sam9x5.dtsi                   |  5 +++++
>  arch/arm/mach-at91/setup.c                          | 21 +++++++++++++++++++++
>  7 files changed, 50 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/atmel-at91.txt b/Documentation/devicetree/bindings/arm/atmel-at91.txt
> index 1196290..fe64d0a 100644
> --- a/Documentation/devicetree/bindings/arm/atmel-at91.txt
> +++ b/Documentation/devicetree/bindings/arm/atmel-at91.txt
> @@ -1,6 +1,10 @@
>  Atmel AT91 device tree bindings.
>  ================================
>  
> +General Purpose Backup Registers (GPBR) required properties:
> +- compatible: Should be "atmel,at91sam9260-gpbr"
> +- reg: Should contain register location and length
> +
>  PIT Timer required properties:
>  - compatible: Should be "atmel,at91sam9260-pit"
>  - reg: Should contain registers location and length
> diff --git a/arch/arm/boot/dts/at91sam9260.dtsi b/arch/arm/boot/dts/at91sam9260.dtsi
> index cb7bcc5..48ef04d 100644
> --- a/arch/arm/boot/dts/at91sam9260.dtsi
> +++ b/arch/arm/boot/dts/at91sam9260.dtsi
> @@ -87,6 +87,11 @@
>  				interrupts = <1 4 7>;
>  			};
>  
> +			gpbr@fffffd50 {
> +				compatible = "atmel,at91sam9260-gpbr";
> +				reg = <0xfffffd50 0x10>;
> +			};
> +
>  			tcb0: timer@fffa0000 {
>  				compatible = "atmel,at91rm9200-tcb";
>  				reg = <0xfffa0000 0x100>;
> diff --git a/arch/arm/boot/dts/at91sam9263.dtsi b/arch/arm/boot/dts/at91sam9263.dtsi
> index 271d4de..e9676ba 100644
> --- a/arch/arm/boot/dts/at91sam9263.dtsi
> +++ b/arch/arm/boot/dts/at91sam9263.dtsi
> @@ -75,6 +75,11 @@
>  				interrupts = <1 4 7>;
>  			};
>  
> +			gpbr@fffffd60 {
> +				compatible = "atmel,at91sam9260-gpbr";
> +				reg = <0xfffffd60 0x50>;
> +			};
> +
>  			tcb0: timer@fff7c000 {
>  				compatible = "atmel,at91rm9200-tcb";
>  				reg = <0xfff7c000 0x100>;
> diff --git a/arch/arm/boot/dts/at91sam9g45.dtsi b/arch/arm/boot/dts/at91sam9g45.dtsi
> index 6b1d4ca..d5e37ce 100644
> --- a/arch/arm/boot/dts/at91sam9g45.dtsi
> +++ b/arch/arm/boot/dts/at91sam9g45.dtsi
> @@ -92,6 +92,11 @@
>  				reg = <0xfffffd10 0x10>;
>  			};
>  
> +			gpbr@fffffd60 {
> +				compatible = "atmel,at91sam9260-gpbr";
> +				reg = <0xfffffd60 0x10>;
> +			};
> +
>  			tcb0: timer@fff7c000 {
>  				compatible = "atmel,at91rm9200-tcb";
>  				reg = <0xfff7c000 0x100>;
> diff --git a/arch/arm/boot/dts/at91sam9n12.dtsi b/arch/arm/boot/dts/at91sam9n12.dtsi
> index 4801717..ac122a6 100644
> --- a/arch/arm/boot/dts/at91sam9n12.dtsi
> +++ b/arch/arm/boot/dts/at91sam9n12.dtsi
> @@ -85,6 +85,11 @@
>  				reg = <0xfffffe10 0x10>;
>  			};
>  
> +			gpbr@fffffe60 {
> +				compatible = "atmel,at91sam9260-gpbr";
> +				reg = <0xfffffe60 0x10>;
> +			};
> +
>  			mmc0: mmc@f0008000 {
>  				compatible = "atmel,hsmci";
>  				reg = <0xf0008000 0x600>;
> diff --git a/arch/arm/boot/dts/at91sam9x5.dtsi b/arch/arm/boot/dts/at91sam9x5.dtsi
> index c461e11..13d069e 100644
> --- a/arch/arm/boot/dts/at91sam9x5.dtsi
> +++ b/arch/arm/boot/dts/at91sam9x5.dtsi
> @@ -88,6 +88,11 @@
>  				interrupts = <1 4 7>;
>  			};
>  
> +			gpbr@fffffe60 {
> +				compatible = "atmel,at91sam9260-gpbr";
> +				reg = <0xfffffe60 0x10>;
> +			};
> +
>  			tcb0: timer@f8008000 {
>  				compatible = "atmel,at91sam9x5-tcb";
>  				reg = <0xf8008000 0x100>;
> diff --git a/arch/arm/mach-at91/setup.c b/arch/arm/mach-at91/setup.c
> index b01f4aa..a525390 100644
> --- a/arch/arm/mach-at91/setup.c
> +++ b/arch/arm/mach-at91/setup.c
> @@ -374,6 +374,26 @@ static void at91_dt_ramc(void)
>  	of_node_put(np);
>  }
>  
> +static struct of_device_id gpbr_ids[] = {
> +	{ .compatible = "atmel,at91sam9260-gpbr", },
> +	{ /* sentinel */ }
> +};
> +
> +static void at91_dt_gpbr(void)
> +{
> +	struct device_node *np;
> +
> +	np = of_find_matching_node(NULL, gpbr_ids);
> +	if (!np)
> +		panic("AT91: failed to find compatible gpbr node in dtb\n");
> +
> +	at91_gpbr_base = of_iomap(np, 0);
> +	if (!at91_gpbr_base)
> +		panic("AT91: Failed to map gpbr registers\n");
> +
> +	of_node_put(np);
> +}

binding ok this NO
no global pointer unless no choice

we need a framework

Best Regards,
J.
> +
>  static struct of_device_id shdwc_ids[] = {
>  	{ .compatible = "atmel,at91sam9260-shdwc", },
>  	{ .compatible = "atmel,at91sam9rl-shdwc", },
> @@ -468,6 +488,7 @@ void __init at91_dt_initialize(void)
>  	at91_dt_rstc();
>  	at91_dt_ramc();
>  	at91_dt_shdwc();
> +	at91_dt_gpbr();
>  
>  	/* Init clock subsystem */
>  	at91_dt_clock_init();
> -- 
> 1.8.1.5
> 

Re: [RFC 4/5] RTC: rtc-at91sam9: add device-tree support

[Jean-Christophe PLAGNIOL-VILLARD]

On 17:12 Sun 07 Apr     , Johan Hovold wrote:
> Add device-tree support.
> 
> The AT91 RTT can be used as an RTC if the atmel,at91-rtt-as-rtc-gpbr
> property is present and set to the general-purpose backup register to
> use to store the RTC time base.
> 
> Signed-off-by: Johan Hovold <jhovold-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  .../devicetree/bindings/rtc/rtc-at91sam9.txt       | 19 ++++++++++++
>  drivers/rtc/rtc-at91sam9.c                         | 36 +++++++++++++++++++++-
>  2 files changed, 54 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/devicetree/bindings/rtc/rtc-at91sam9.txt
> 
> diff --git a/Documentation/devicetree/bindings/rtc/rtc-at91sam9.txt b/Documentation/devicetree/bindings/rtc/rtc-at91sam9.txt
> new file mode 100644
> index 0000000..0f54988
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/rtc/rtc-at91sam9.txt
> @@ -0,0 +1,19 @@
> +Atmel AT91 RTT as RTC
> +=====================
> +
> +Required properties:
> +- compatible: Should be "atmel,at91sam9260-rtt"
> +- reg: Should contain register location and length
> +- interrupts: Should contain interrupt for the RTT which is the IRQ line
> +  shared across all System Controller members.
> +- atmel,rtt-as-rtc-gpbr: Should contain the backup-register to use to store
> +  the RTC time base
> +
> +Example:
> +
> +rtt@fffffd20 {
> +		compatible = "atmel,at91sam9g45-rtt", "atmel,at91sam9260-rtt";
> +		reg = <0xfffffd20 0x10>;
> +		interrupts = <1 4 7>;
> +		atmel,at91-rtt-as-rtc-gpbr = <0>;
no you miss the point of the DT

you need to describe the hardware no a particular use of it

the RTT is a general purpose timer backuped that we use in linux as a RTC with a gpbr
to store the time


you need 2 binding on for the RTT one the RTT-rtc
> +	};
> diff --git a/drivers/rtc/rtc-at91sam9.c b/drivers/rtc/rtc-at91sam9.c
> index 983667b..7ccce19 100644
> --- a/drivers/rtc/rtc-at91sam9.c
> +++ b/drivers/rtc/rtc-at91sam9.c
> @@ -13,6 +13,7 @@
>  
>  #include <linux/module.h>
>  #include <linux/kernel.h>
> +#include <linux/of.h>
>  #include <linux/platform_device.h>
>  #include <linux/time.h>
>  #include <linux/rtc.h>
> @@ -282,6 +283,29 @@ static const struct rtc_class_ops at91_rtc_ops = {
>  	.alarm_irq_enable = at91_rtc_alarm_irq_enable,
>  };
>  
> +
> +#ifdef CONFIG_OF
> +static struct of_device_id at91_rtc_of_match[] = {
> +	{ .compatible = "atmel,at91sam9260-rtt", },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, at91_rtc_of_match);
> +#endif
> +
> +static int at91_rtc_parse_dt(struct device_node *node, unsigned *gpbr)
> +{
> +	u32 val;
> +	int res;
> +
> +	res = of_property_read_u32(node, "atmel,at91-rtt-as-rtc-gpbr", &val);
> +	if (!res)
> +		return -ENODEV;
> +
> +	*gpbr = val;
> +
> +	return 0;
> +}
> +
>  /*
>   * Initialize and install RTC driver
>   */
> @@ -291,6 +315,15 @@ static int at91_rtc_probe(struct platform_device *pdev)
>  	struct sam9_rtc	*rtc;
>  	int		ret, irq;
>  	u32		mr;
> +	unsigned	gpbr;
> +
> +	if (pdev->dev.of_node) {
> +		ret = at91_rtc_parse_dt(pdev->dev.of_node, &gpbr);
> +		if (ret)
> +			return ret;
> +	} else {
> +		gpbr = CONFIG_RTC_DRV_AT91SAM9_GPBR;
> +	}
>  
>  	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	if (!r)
> @@ -320,7 +353,7 @@ static int at91_rtc_probe(struct platform_device *pdev)
>  		goto fail;
>  	}
>  
> -	rtc->gpbr = CONFIG_RTC_DRV_AT91SAM9_GPBR;
> +	rtc->gpbr = gpbr;
>  
>  	mr = rtt_readl(rtc, MR);
>  
> @@ -455,6 +488,7 @@ static struct platform_driver at91_rtc_driver = {
>  	.driver		= {
>  		.name	= "rtc-at91sam9",
>  		.owner	= THIS_MODULE,
> +		.of_match_table = of_match_ptr(at91_rtc_of_match),
>  	},
>  };
>  
> -- 
> 1.8.1.5
> 

Re: [RFC 4/5] RTC: rtc-at91sam9: add device-tree support

[Johan Hovold]

On Mon, Apr 08, 2013 at 09:38:07AM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 17:12 Sun 07 Apr     , Johan Hovold wrote:
> > Add device-tree support.
> > 
> > The AT91 RTT can be used as an RTC if the atmel,at91-rtt-as-rtc-gpbr
> > property is present and set to the general-purpose backup register to
> > use to store the RTC time base.
> > 
> > Signed-off-by: Johan Hovold <jhovold@gmail.com>
> > ---
> >  .../devicetree/bindings/rtc/rtc-at91sam9.txt       | 19 ++++++++++++
> >  drivers/rtc/rtc-at91sam9.c                         | 36 +++++++++++++++++++++-
> >  2 files changed, 54 insertions(+), 1 deletion(-)
> >  create mode 100644 Documentation/devicetree/bindings/rtc/rtc-at91sam9.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/rtc/rtc-at91sam9.txt b/Documentation/devicetree/bindings/rtc/rtc-at91sam9.txt
> > new file mode 100644
> > index 0000000..0f54988
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/rtc/rtc-at91sam9.txt
> > @@ -0,0 +1,19 @@
> > +Atmel AT91 RTT as RTC
> > +=====================
> > +
> > +Required properties:
> > +- compatible: Should be "atmel,at91sam9260-rtt"
> > +- reg: Should contain register location and length
> > +- interrupts: Should contain interrupt for the RTT which is the IRQ line
> > +  shared across all System Controller members.
> > +- atmel,rtt-as-rtc-gpbr: Should contain the backup-register to use to store
> > +  the RTC time base
> > +
> > +Example:
> > +
> > +rtt@fffffd20 {
> > +		compatible = "atmel,at91sam9g45-rtt", "atmel,at91sam9260-rtt";
> > +		reg = <0xfffffd20 0x10>;
> > +		interrupts = <1 4 7>;
> > +		atmel,at91-rtt-as-rtc-gpbr = <0>;
> no you miss the point of the DT
> 
> you need to describe the hardware no a particular use of it

That was what I was trying to achieve by adding the two use-neutral
rtt and gpbr-nodes. But then the question is how would you influence
which out of two rtt-drivers to use?

Adding a property as above in the final board descriptions seemed
preferable to adding rtt-as-rtc to the compatible string of the rtt as
that would mean describing use rather than just hardware.

> the RTT is a general purpose timer backuped that we use in linux as a
> RTC with a gpbr to store the time
> 
> you need 2 binding on for the RTT one the RTT-rtc

As in adding some virtual hardware-node which uses the rtt and gpbr as
resources?

Thanks,
Johan

Re: [RFC 4/5] RTC: rtc-at91sam9: add device-tree support

[Nicolas Ferre]

On 04/08/2013 11:00 AM, Johan Hovold :
> On Mon, Apr 08, 2013 at 09:38:07AM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
>> On 17:12 Sun 07 Apr     , Johan Hovold wrote:
>>> Add device-tree support.
>>>
>>> The AT91 RTT can be used as an RTC if the atmel,at91-rtt-as-rtc-gpbr
>>> property is present and set to the general-purpose backup register to
>>> use to store the RTC time base.
>>>
>>> Signed-off-by: Johan Hovold <jhovold-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>> ---
>>>  .../devicetree/bindings/rtc/rtc-at91sam9.txt       | 19 ++++++++++++
>>>  drivers/rtc/rtc-at91sam9.c                         | 36 +++++++++++++++++++++-
>>>  2 files changed, 54 insertions(+), 1 deletion(-)
>>>  create mode 100644 Documentation/devicetree/bindings/rtc/rtc-at91sam9.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/rtc/rtc-at91sam9.txt b/Documentation/devicetree/bindings/rtc/rtc-at91sam9.txt
>>> new file mode 100644
>>> index 0000000..0f54988
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/rtc/rtc-at91sam9.txt
>>> @@ -0,0 +1,19 @@
>>> +Atmel AT91 RTT as RTC
>>> +=====================
>>> +
>>> +Required properties:
>>> +- compatible: Should be "atmel,at91sam9260-rtt"
>>> +- reg: Should contain register location and length
>>> +- interrupts: Should contain interrupt for the RTT which is the IRQ line
>>> +  shared across all System Controller members.
>>> +- atmel,rtt-as-rtc-gpbr: Should contain the backup-register to use to store
>>> +  the RTC time base
>>> +
>>> +Example:
>>> +
>>> +rtt@fffffd20 {
>>> +		compatible = "atmel,at91sam9g45-rtt", "atmel,at91sam9260-rtt";

No, there is no visible difference between the sam9g45 RTT and the
sam9260 one. So the most precise compatibility string is still sam9260.
If one day we feel the need for a advanced feature that exists on a more
recent SoC, we have the possibility to add it at that time...

>>> +		reg = <0xfffffd20 0x10>;
>>> +		interrupts = <1 4 7>;
>>> +		atmel,at91-rtt-as-rtc-gpbr = <0>;
>> no you miss the point of the DT
>>
>> you need to describe the hardware no a particular use of it
> 
> That was what I was trying to achieve by adding the two use-neutral
> rtt and gpbr-nodes. But then the question is how would you influence
> which out of two rtt-drivers to use?
> 
> Adding a property as above in the final board descriptions seemed
> preferable to adding rtt-as-rtc to the compatible string of the rtt as
> that would mean describing use rather than just hardware.

Well, re-reading the Device_Tree_Usage page, I found this sentence:
"
Understanding the compatible Property

Every node in the tree that represents a device is required to have the
compatible property. compatible is the key an operating system uses to
decide which device driver to bind to a device.
"
or ePARP:

"
The compatible property value consists of one or more strings that
define the specific programming model for the device.
"

We have the notion of link between hardware and software in this
*compatible* sting, even if the *node* itself is about hardware description.

>> the RTT is a general purpose timer backuped that we use in linux as a
>> RTC with a gpbr to store the time
>>
>> you need 2 binding on for the RTT one the RTT-rtc
> 
> As in adding some virtual hardware-node which uses the rtt and gpbr as
> resources?

So, why not simply having a compatibility string that collects the uses
of this RTT node:

compatible = "atmel,at91sam9260-rtt-as-rtc", "atmel,at91sam9260-rtt";

And then "decide which device driver to bind to [the RTT] device"...
If the rtt-as-rtc driver is not selected, the device can still be used
as a simple "rtt". The board .dts can overload a compatibility string
according to the use, etc.

Then the way do describe which GPBR to use has still to be discussed.
But for the RTT itself, I would keep it simple like that.

Best regards,
-- 
Nicolas Ferre

Re: [RFC 4/5] RTC: rtc-at91sam9: add device-tree support

[Jean-Christophe PLAGNIOL-VILLARD]

On 11:57 Mon 08 Apr     , Nicolas Ferre wrote:
> On 04/08/2013 11:00 AM, Johan Hovold :
> > On Mon, Apr 08, 2013 at 09:38:07AM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> >> On 17:12 Sun 07 Apr     , Johan Hovold wrote:
> >>> Add device-tree support.
> >>>
> >>> The AT91 RTT can be used as an RTC if the atmel,at91-rtt-as-rtc-gpbr
> >>> property is present and set to the general-purpose backup register to
> >>> use to store the RTC time base.
> >>>
> >>> Signed-off-by: Johan Hovold <jhovold-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> >>> ---
> >>>  .../devicetree/bindings/rtc/rtc-at91sam9.txt       | 19 ++++++++++++
> >>>  drivers/rtc/rtc-at91sam9.c                         | 36 +++++++++++++++++++++-
> >>>  2 files changed, 54 insertions(+), 1 deletion(-)
> >>>  create mode 100644 Documentation/devicetree/bindings/rtc/rtc-at91sam9.txt
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/rtc/rtc-at91sam9.txt b/Documentation/devicetree/bindings/rtc/rtc-at91sam9.txt
> >>> new file mode 100644
> >>> index 0000000..0f54988
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/rtc/rtc-at91sam9.txt
> >>> @@ -0,0 +1,19 @@
> >>> +Atmel AT91 RTT as RTC
> >>> +=====================
> >>> +
> >>> +Required properties:
> >>> +- compatible: Should be "atmel,at91sam9260-rtt"
> >>> +- reg: Should contain register location and length
> >>> +- interrupts: Should contain interrupt for the RTT which is the IRQ line
> >>> +  shared across all System Controller members.
> >>> +- atmel,rtt-as-rtc-gpbr: Should contain the backup-register to use to store
> >>> +  the RTC time base
> >>> +
> >>> +Example:
> >>> +
> >>> +rtt@fffffd20 {
> >>> +		compatible = "atmel,at91sam9g45-rtt", "atmel,at91sam9260-rtt";
> 
> No, there is no visible difference between the sam9g45 RTT and the
> sam9260 one. So the most precise compatibility string is still sam9260.
> If one day we feel the need for a advanced feature that exists on a more
> recent SoC, we have the possibility to add it at that time...
> 
> >>> +		reg = <0xfffffd20 0x10>;
> >>> +		interrupts = <1 4 7>;
> >>> +		atmel,at91-rtt-as-rtc-gpbr = <0>;
> >> no you miss the point of the DT
> >>
> >> you need to describe the hardware no a particular use of it
> > 
> > That was what I was trying to achieve by adding the two use-neutral
> > rtt and gpbr-nodes. But then the question is how would you influence
> > which out of two rtt-drivers to use?
> > 
> > Adding a property as above in the final board descriptions seemed
> > preferable to adding rtt-as-rtc to the compatible string of the rtt as
> > that would mean describing use rather than just hardware.
> 
> Well, re-reading the Device_Tree_Usage page, I found this sentence:
> "
> Understanding the compatible Property
> 
> Every node in the tree that represents a device is required to have the
> compatible property. compatible is the key an operating system uses to
> decide which device driver to bind to a device.
> "
> or ePARP:
> 
> "
> The compatible property value consists of one or more strings that
> define the specific programming model for the device.
> "
> 
> We have the notion of link between hardware and software in this
> *compatible* sting, even if the *node* itself is about hardware description.
> 
> >> the RTT is a general purpose timer backuped that we use in linux as a
> >> RTC with a gpbr to store the time
> >>
> >> you need 2 binding on for the RTT one the RTT-rtc
> > 
> > As in adding some virtual hardware-node which uses the rtt and gpbr as
> > resources?
> 
> So, why not simply having a compatibility string that collects the uses
> of this RTT node:
> 
> compatible = "atmel,at91sam9260-rtt-as-rtc", "atmel,at91sam9260-rtt";
> 
> And then "decide which device driver to bind to [the RTT] device"...
> If the rtt-as-rtc driver is not selected, the device can still be used
> as a simple "rtt". The board .dts can overload a compatibility string
> according to the use, etc.
> 
> Then the way do describe which GPBR to use has still to be discussed.
> But for the RTT itself, I would keep it simple like that.

no as infact the rtc-at91sam9 should not even exist
as this is much more generic

we use a backped register and a timer to emulate a RTC this can be unsed by
any one

and I can use any backuped timer

we need to have frameworks
where the gpbr are tracked and the rtt

	for you describe the resources

	rtt0: rtt@fffffd20 {
		compatible = "atmel,at91sam9260-rtt";
		reg = <0xfffffd20 0x10>;
		interrupts = <1 4 7>;
	};

	rtc-timer {
		compatible = "linux,rtc-timer";
		timer = <&rtt0>;
		backuped-register = <&gpbr 0>;
	};

this need to SoC implemetation generic

Re: [RFC 4/5] RTC: rtc-at91sam9: add device-tree support

[Nicolas Ferre]

On 04/08/2013 12:03 PM, Jean-Christophe PLAGNIOL-VILLARD :
> On 11:57 Mon 08 Apr     , Nicolas Ferre wrote:
>> On 04/08/2013 11:00 AM, Johan Hovold :
>>> On Mon, Apr 08, 2013 at 09:38:07AM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
>>>> On 17:12 Sun 07 Apr     , Johan Hovold wrote:
>>>>> Add device-tree support.
>>>>>
>>>>> The AT91 RTT can be used as an RTC if the atmel,at91-rtt-as-rtc-gpbr
>>>>> property is present and set to the general-purpose backup register to
>>>>> use to store the RTC time base.
>>>>>
>>>>> Signed-off-by: Johan Hovold <jhovold-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>>> ---
>>>>>  .../devicetree/bindings/rtc/rtc-at91sam9.txt       | 19 ++++++++++++
>>>>>  drivers/rtc/rtc-at91sam9.c                         | 36 +++++++++++++++++++++-
>>>>>  2 files changed, 54 insertions(+), 1 deletion(-)
>>>>>  create mode 100644 Documentation/devicetree/bindings/rtc/rtc-at91sam9.txt
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/rtc/rtc-at91sam9.txt b/Documentation/devicetree/bindings/rtc/rtc-at91sam9.txt
>>>>> new file mode 100644
>>>>> index 0000000..0f54988
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/rtc/rtc-at91sam9.txt
>>>>> @@ -0,0 +1,19 @@
>>>>> +Atmel AT91 RTT as RTC
>>>>> +=====================
>>>>> +
>>>>> +Required properties:
>>>>> +- compatible: Should be "atmel,at91sam9260-rtt"
>>>>> +- reg: Should contain register location and length
>>>>> +- interrupts: Should contain interrupt for the RTT which is the IRQ line
>>>>> +  shared across all System Controller members.
>>>>> +- atmel,rtt-as-rtc-gpbr: Should contain the backup-register to use to store
>>>>> +  the RTC time base
>>>>> +
>>>>> +Example:
>>>>> +
>>>>> +rtt@fffffd20 {
>>>>> +		compatible = "atmel,at91sam9g45-rtt", "atmel,at91sam9260-rtt";
>>
>> No, there is no visible difference between the sam9g45 RTT and the
>> sam9260 one. So the most precise compatibility string is still sam9260.
>> If one day we feel the need for a advanced feature that exists on a more
>> recent SoC, we have the possibility to add it at that time...
>>
>>>>> +		reg = <0xfffffd20 0x10>;
>>>>> +		interrupts = <1 4 7>;
>>>>> +		atmel,at91-rtt-as-rtc-gpbr = <0>;
>>>> no you miss the point of the DT
>>>>
>>>> you need to describe the hardware no a particular use of it
>>>
>>> That was what I was trying to achieve by adding the two use-neutral
>>> rtt and gpbr-nodes. But then the question is how would you influence
>>> which out of two rtt-drivers to use?
>>>
>>> Adding a property as above in the final board descriptions seemed
>>> preferable to adding rtt-as-rtc to the compatible string of the rtt as
>>> that would mean describing use rather than just hardware.
>>
>> Well, re-reading the Device_Tree_Usage page, I found this sentence:
>> "
>> Understanding the compatible Property
>>
>> Every node in the tree that represents a device is required to have the
>> compatible property. compatible is the key an operating system uses to
>> decide which device driver to bind to a device.
>> "
>> or ePARP:
>>
>> "
>> The compatible property value consists of one or more strings that
>> define the specific programming model for the device.
>> "
>>
>> We have the notion of link between hardware and software in this
>> *compatible* sting, even if the *node* itself is about hardware description.
>>
>>>> the RTT is a general purpose timer backuped that we use in linux as a
>>>> RTC with a gpbr to store the time
>>>>
>>>> you need 2 binding on for the RTT one the RTT-rtc
>>>
>>> As in adding some virtual hardware-node which uses the rtt and gpbr as
>>> resources?
>>
>> So, why not simply having a compatibility string that collects the uses
>> of this RTT node:
>>
>> compatible = "atmel,at91sam9260-rtt-as-rtc", "atmel,at91sam9260-rtt";
>>
>> And then "decide which device driver to bind to [the RTT] device"...
>> If the rtt-as-rtc driver is not selected, the device can still be used
>> as a simple "rtt". The board .dts can overload a compatibility string
>> according to the use, etc.
>>
>> Then the way do describe which GPBR to use has still to be discussed.
>> But for the RTT itself, I would keep it simple like that.
> 
> no as infact the rtc-at91sam9 should not even exist
> as this is much more generic
> 
> we use a backped register and a timer to emulate a RTC this can be unsed by
> any one
> 
> and I can use any backuped timer
> 
> we need to have frameworks

Is it possible to focus on the current problem instead of calling for
the creation for Yet Another Framework?

> where the gpbr are tracked and the rtt
> 
> 	for you describe the resources
> 
> 	rtt0: rtt@fffffd20 {
> 		compatible = "atmel,at91sam9260-rtt";
> 		reg = <0xfffffd20 0x10>;
> 		interrupts = <1 4 7>;
> 	};
> 
> 	rtc-timer {
> 		compatible = "linux,rtc-timer";
> 		timer = <&rtt0>;
> 		backuped-register = <&gpbr 0>;
> 	};
> 
> this need to SoC implemetation generic

OMG, so well, we will have to re-write the whole rtc-at91sam9 now??!! Oh
yes, I see, we do not have more urgent things to do...

Please stop trying re-inventing things that will do everything but
solving the real issues: why not agree on a sort term solution about
this rtt/rtc thing: I have *never* heard complains about this precise
driver. And we must concentrate now on problem solving.

Bye,
-- 
Nicolas Ferre

Re: [RFC 4/5] RTC: rtc-at91sam9: add device-tree support

[Jean-Christophe PLAGNIOL-VILLARD]

On 12:42 Mon 08 Apr     , Nicolas Ferre wrote:
> On 04/08/2013 12:03 PM, Jean-Christophe PLAGNIOL-VILLARD :
> > On 11:57 Mon 08 Apr     , Nicolas Ferre wrote:
> >> On 04/08/2013 11:00 AM, Johan Hovold :
> >>> On Mon, Apr 08, 2013 at 09:38:07AM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> >>>> On 17:12 Sun 07 Apr     , Johan Hovold wrote:
> >>>>> Add device-tree support.
> >>>>>
> >>>>> The AT91 RTT can be used as an RTC if the atmel,at91-rtt-as-rtc-gpbr
> >>>>> property is present and set to the general-purpose backup register to
> >>>>> use to store the RTC time base.
> >>>>>
> >>>>> Signed-off-by: Johan Hovold <jhovold-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> >>>>> ---
> >>>>>  .../devicetree/bindings/rtc/rtc-at91sam9.txt       | 19 ++++++++++++
> >>>>>  drivers/rtc/rtc-at91sam9.c                         | 36 +++++++++++++++++++++-
> >>>>>  2 files changed, 54 insertions(+), 1 deletion(-)
> >>>>>  create mode 100644 Documentation/devicetree/bindings/rtc/rtc-at91sam9.txt
> >>>>>
> >>>>> diff --git a/Documentation/devicetree/bindings/rtc/rtc-at91sam9.txt b/Documentation/devicetree/bindings/rtc/rtc-at91sam9.txt
> >>>>> new file mode 100644
> >>>>> index 0000000..0f54988
> >>>>> --- /dev/null
> >>>>> +++ b/Documentation/devicetree/bindings/rtc/rtc-at91sam9.txt
> >>>>> @@ -0,0 +1,19 @@
> >>>>> +Atmel AT91 RTT as RTC
> >>>>> +=====================
> >>>>> +
> >>>>> +Required properties:
> >>>>> +- compatible: Should be "atmel,at91sam9260-rtt"
> >>>>> +- reg: Should contain register location and length
> >>>>> +- interrupts: Should contain interrupt for the RTT which is the IRQ line
> >>>>> +  shared across all System Controller members.
> >>>>> +- atmel,rtt-as-rtc-gpbr: Should contain the backup-register to use to store
> >>>>> +  the RTC time base
> >>>>> +
> >>>>> +Example:
> >>>>> +
> >>>>> +rtt@fffffd20 {
> >>>>> +		compatible = "atmel,at91sam9g45-rtt", "atmel,at91sam9260-rtt";
> >>
> >> No, there is no visible difference between the sam9g45 RTT and the
> >> sam9260 one. So the most precise compatibility string is still sam9260.
> >> If one day we feel the need for a advanced feature that exists on a more
> >> recent SoC, we have the possibility to add it at that time...
> >>
> >>>>> +		reg = <0xfffffd20 0x10>;
> >>>>> +		interrupts = <1 4 7>;
> >>>>> +		atmel,at91-rtt-as-rtc-gpbr = <0>;
> >>>> no you miss the point of the DT
> >>>>
> >>>> you need to describe the hardware no a particular use of it
> >>>
> >>> That was what I was trying to achieve by adding the two use-neutral
> >>> rtt and gpbr-nodes. But then the question is how would you influence
> >>> which out of two rtt-drivers to use?
> >>>
> >>> Adding a property as above in the final board descriptions seemed
> >>> preferable to adding rtt-as-rtc to the compatible string of the rtt as
> >>> that would mean describing use rather than just hardware.
> >>
> >> Well, re-reading the Device_Tree_Usage page, I found this sentence:
> >> "
> >> Understanding the compatible Property
> >>
> >> Every node in the tree that represents a device is required to have the
> >> compatible property. compatible is the key an operating system uses to
> >> decide which device driver to bind to a device.
> >> "
> >> or ePARP:
> >>
> >> "
> >> The compatible property value consists of one or more strings that
> >> define the specific programming model for the device.
> >> "
> >>
> >> We have the notion of link between hardware and software in this
> >> *compatible* sting, even if the *node* itself is about hardware description.
> >>
> >>>> the RTT is a general purpose timer backuped that we use in linux as a
> >>>> RTC with a gpbr to store the time
> >>>>
> >>>> you need 2 binding on for the RTT one the RTT-rtc
> >>>
> >>> As in adding some virtual hardware-node which uses the rtt and gpbr as
> >>> resources?
> >>
> >> So, why not simply having a compatibility string that collects the uses
> >> of this RTT node:
> >>
> >> compatible = "atmel,at91sam9260-rtt-as-rtc", "atmel,at91sam9260-rtt";
> >>
> >> And then "decide which device driver to bind to [the RTT] device"...
> >> If the rtt-as-rtc driver is not selected, the device can still be used
> >> as a simple "rtt". The board .dts can overload a compatibility string
> >> according to the use, etc.
> >>
> >> Then the way do describe which GPBR to use has still to be discussed.
> >> But for the RTT itself, I would keep it simple like that.
> > 
> > no as infact the rtc-at91sam9 should not even exist
> > as this is much more generic
> > 
> > we use a backped register and a timer to emulate a RTC this can be unsed by
> > any one
> > 
> > and I can use any backuped timer
> > 
> > we need to have frameworks
> 
> Is it possible to focus on the current problem instead of calling for
> the creation for Yet Another Framework?
> 
> > where the gpbr are tracked and the rtt
> > 
> > 	for you describe the resources
> > 
> > 	rtt0: rtt@fffffd20 {
> > 		compatible = "atmel,at91sam9260-rtt";
> > 		reg = <0xfffffd20 0x10>;
> > 		interrupts = <1 4 7>;
> > 	};
> > 
> > 	rtc-timer {
> > 		compatible = "linux,rtc-timer";
> > 		timer = <&rtt0>;
> > 		backuped-register = <&gpbr 0>;
> > 	};
> > 
> > this need to SoC implemetation generic
> 
> OMG, so well, we will have to re-write the whole rtc-at91sam9 now??!! Oh
> yes, I see, we do not have more urgent things to do...
> 
> Please stop trying re-inventing things that will do everything but
> solving the real issues: why not agree on a sort term solution about
> this rtt/rtc thing: I have *never* heard complains about this precise
> driver. And we must concentrate now on problem solving.

no the current driver is badly design this need to be cleanup

switch to DT does mean do the stuff CLEAN no quirk hack

we did the same for pinctrl/dma/fb. This need to be done correctly

DT is to describe hardware not software implentation

Best Regards,
J.

Re: [RFC 4/5] RTC: rtc-at91sam9: add device-tree support

[Johan Hovold]

On Mon, Apr 08, 2013 at 12:03:01PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 11:57 Mon 08 Apr     , Nicolas Ferre wrote:
> > On 04/08/2013 11:00 AM, Johan Hovold :
> > > On Mon, Apr 08, 2013 at 09:38:07AM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > >> On 17:12 Sun 07 Apr     , Johan Hovold wrote:

[...]

> > >> the RTT is a general purpose timer backuped that we use in linux as a
> > >> RTC with a gpbr to store the time
> > >>
> > >> you need 2 binding on for the RTT one the RTT-rtc
> > > 
> > > As in adding some virtual hardware-node which uses the rtt and gpbr as
> > > resources?
> > 
> > So, why not simply having a compatibility string that collects the uses
> > of this RTT node:
> > 
> > compatible = "atmel,at91sam9260-rtt-as-rtc", "atmel,at91sam9260-rtt";
> > 
> > And then "decide which device driver to bind to [the RTT] device"...
> > If the rtt-as-rtc driver is not selected, the device can still be used
> > as a simple "rtt". The board .dts can overload a compatibility string
> > according to the use, etc.
> > 
> > Then the way do describe which GPBR to use has still to be discussed.
> > But for the RTT itself, I would keep it simple like that.
> 
> no as infact the rtc-at91sam9 should not even exist
> as this is much more generic
> 
> we use a backped register and a timer to emulate a RTC this can be unsed by
> any one
> 
> and I can use any backuped timer
> 
> we need to have frameworks
> where the gpbr are tracked and the rtt
> 
> 	for you describe the resources
> 
> 	rtt0: rtt@fffffd20 {
> 		compatible = "atmel,at91sam9260-rtt";
> 		reg = <0xfffffd20 0x10>;
> 		interrupts = <1 4 7>;
> 	};
> 
> 	rtc-timer {
> 		compatible = "linux,rtc-timer";
> 		timer = <&rtt0>;
> 		backuped-register = <&gpbr 0>;
> 	};
> 
> this need to SoC implemetation generic

This would indeed be a neat solution, albeit one that requires a lot
work to be realised as both a backup-register and backed-up-timer
framework would need to be implemented.

Thanks,
Johan

Re: [RFC 4/5] RTC: rtc-at91sam9: add device-tree support

[Jean-Christophe PLAGNIOL-VILLARD]

On 12:48 Mon 08 Apr     , Johan Hovold wrote:
> On Mon, Apr 08, 2013 at 12:03:01PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > On 11:57 Mon 08 Apr     , Nicolas Ferre wrote:
> > > On 04/08/2013 11:00 AM, Johan Hovold :
> > > > On Mon, Apr 08, 2013 at 09:38:07AM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > > >> On 17:12 Sun 07 Apr     , Johan Hovold wrote:
> 
> [...]
> 
> > > >> the RTT is a general purpose timer backuped that we use in linux as a
> > > >> RTC with a gpbr to store the time
> > > >>
> > > >> you need 2 binding on for the RTT one the RTT-rtc
> > > > 
> > > > As in adding some virtual hardware-node which uses the rtt and gpbr as
> > > > resources?
> > > 
> > > So, why not simply having a compatibility string that collects the uses
> > > of this RTT node:
> > > 
> > > compatible = "atmel,at91sam9260-rtt-as-rtc", "atmel,at91sam9260-rtt";
> > > 
> > > And then "decide which device driver to bind to [the RTT] device"...
> > > If the rtt-as-rtc driver is not selected, the device can still be used
> > > as a simple "rtt". The board .dts can overload a compatibility string
> > > according to the use, etc.
> > > 
> > > Then the way do describe which GPBR to use has still to be discussed.
> > > But for the RTT itself, I would keep it simple like that.
> > 
> > no as infact the rtc-at91sam9 should not even exist
> > as this is much more generic
> > 
> > we use a backped register and a timer to emulate a RTC this can be unsed by
> > any one
> > 
> > and I can use any backuped timer
> > 
> > we need to have frameworks
> > where the gpbr are tracked and the rtt
> > 
> > 	for you describe the resources
> > 
> > 	rtt0: rtt@fffffd20 {
> > 		compatible = "atmel,at91sam9260-rtt";
> > 		reg = <0xfffffd20 0x10>;
> > 		interrupts = <1 4 7>;
> > 	};
> > 
> > 	rtc-timer {
> > 		compatible = "linux,rtc-timer";
> > 		timer = <&rtt0>;
> > 		backuped-register = <&gpbr 0>;
> > 	};
> > 
> > this need to SoC implemetation generic
> 
> This would indeed be a neat solution, albeit one that requires a lot
> work to be realised as both a backup-register and backed-up-timer
> framework would need to be implemented.

not that much

for backuped register or rtt a simple list with spinlock will be enougth
and after just handle the phandle

it's not that complex but if we want to describe the hardware correctly this
is needed

And this will not be AT91 specific but SoC generic

Ensure the GPBR are requested once is mandatory for any bug detection
a global ioremap is really dirty and need to be used when only NO other choice

Best Regards,
J.
> 
> Bye,
> -- 
> Nicolas Ferre
> 
> Thanks,
> Johan

Re: [RFC 4/5] RTC: rtc-at91sam9: add device-tree support

[Johan Hovold]

On Mon, Apr 08, 2013 at 11:57:06AM +0200, Nicolas Ferre wrote:
> On 04/08/2013 11:00 AM, Johan Hovold :
> > On Mon, Apr 08, 2013 at 09:38:07AM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> >> On 17:12 Sun 07 Apr     , Johan Hovold wrote:
> >>> Add device-tree support.
> >>>
> >>> The AT91 RTT can be used as an RTC if the atmel,at91-rtt-as-rtc-gpbr
> >>> property is present and set to the general-purpose backup register to
> >>> use to store the RTC time base.
> >>>
> >>> Signed-off-by: Johan Hovold <jhovold@gmail.com>
> >>> ---
> >>>  .../devicetree/bindings/rtc/rtc-at91sam9.txt       | 19 ++++++++++++
> >>>  drivers/rtc/rtc-at91sam9.c                         | 36 +++++++++++++++++++++-
> >>>  2 files changed, 54 insertions(+), 1 deletion(-)
> >>>  create mode 100644 Documentation/devicetree/bindings/rtc/rtc-at91sam9.txt > >>>
> >>> diff --git
> >>> a/Documentation/devicetree/bindings/rtc/rtc-at91sam9.txt
> >>> b/Documentation/devicetree/bindings/rtc/rtc-at91sam9.txt > >>> new
> >>> file mode 100644
> >>> index 0000000..0f54988
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/rtc/rtc-at91sam9.txt
> >>> @@ -0,0 +1,19 @@
> >>> +Atmel AT91 RTT as RTC
> >>> +=====================
> >>> +
> >>> +Required properties:
> >>> +- compatible: Should be "atmel,at91sam9260-rtt"
> >>> +- reg: Should contain register location and length
> >>> +- interrupts: Should contain interrupt for the RTT which is the IRQ line
> >>> +  shared across all System Controller members.
> >>> +- atmel,rtt-as-rtc-gpbr: Should contain the backup-register to use to store
> >>> +  the RTC time base
> >>> +
> >>> +Example:
> >>> +
> >>> +rtt@fffffd20 {
> >>> +		compatible = "atmel,at91sam9g45-rtt", "atmel,at91sam9260-rtt";
> 
> No, there is no visible difference between the sam9g45 RTT and the
> sam9260 one. So the most precise compatibility string is still sam9260.
> If one day we feel the need for a advanced feature that exists on a more
> recent SoC, we have the possibility to add it at that time...

Yes, this should be just "atmel,at91sam9260-rtt" to follow the current
practise in AT91. However, as I mentioned in an earlier mail one could
interpret

	"The first string in the list specifies the exact device that
	the node represents in the form "<manufacturer>,<model>". The
	following strings represent other devices that the device is
	compatible with.

	For example, the Freescale MPC8349 System on Chip (SoC) has a
	serial device which implements the National Semiconductor
	ns16550 register interface. The compatible property for the
	MPC8349 serial device should therefore be: compatible =
	"fsl,mpc8349-uart", "ns16550". In this case, fsl,mpc8349-uart
	specifies the exact device, and ns16550 states that it is
	register-level compatible with a National Semiconductor 16550
	UART."

	http://www.devicetree.org/Device_Tree_Usage#Understanding_the_compatible_Property

to mean that the compatible property should always be exact SoC-IP
followed by the first (most generic) compatible one.

> >>> +		reg = <0xfffffd20 0x10>;
> >>> +		interrupts = <1 4 7>;
> >>> +		atmel,at91-rtt-as-rtc-gpbr = <0>;
> >> no you miss the point of the DT
> >>
> >> you need to describe the hardware no a particular use of it
> > 
> > That was what I was trying to achieve by adding the two use-neutral
> > rtt and gpbr-nodes. But then the question is how would you influence
> > which out of two rtt-drivers to use?
> > 
> > Adding a property as above in the final board descriptions seemed
> > preferable to adding rtt-as-rtc to the compatible string of the rtt as
> > that would mean describing use rather than just hardware.
> 
> Well, re-reading the Device_Tree_Usage page, I found this sentence:
> "
> Understanding the compatible Property
> 
> Every node in the tree that represents a device is required to have the
> compatible property. compatible is the key an operating system uses to
> decide which device driver to bind to a device.
> "
> or ePARP:
> 
> "
> The compatible property value consists of one or more strings that
> define the specific programming model for the device.
> "
> 
> We have the notion of link between hardware and software in this
> *compatible* sting, even if the *node* itself is about hardware description.
> 
> >> the RTT is a general purpose timer backuped that we use in linux as a
> >> RTC with a gpbr to store the time
> >>
> >> you need 2 binding on for the RTT one the RTT-rtc
> > 
> > As in adding some virtual hardware-node which uses the rtt and gpbr as
> > resources?
> 
> So, why not simply having a compatibility string that collects the uses
> of this RTT node:
> 
> compatible = "atmel,at91sam9260-rtt-as-rtc", "atmel,at91sam9260-rtt";
> 
> And then "decide which device driver to bind to [the RTT] device"...
> If the rtt-as-rtc driver is not selected, the device can still be used
> as a simple "rtt". The board .dts can overload a compatibility string
> according to the use, etc.
> 
> Then the way do describe which GPBR to use has still to be discussed.
> But for the RTT itself, I would keep it simple like that.

Yes, this would also work but it just appears to me that it would
violate the above mentioned description of the compatible property:

	"The first string in the list specifies the exact device that
	the node represents in the form "<manufacturer>,<model>". The
	following strings represent other devices that the device is
	compatible with."

Thanks,
Johan

Re: [RFC 4/5] RTC: rtc-at91sam9: add device-tree support

[Jean-Christophe PLAGNIOL-VILLARD]

On 12:38 Mon 08 Apr     , Johan Hovold wrote:
> On Mon, Apr 08, 2013 at 11:57:06AM +0200, Nicolas Ferre wrote:
> > On 04/08/2013 11:00 AM, Johan Hovold :
> > > On Mon, Apr 08, 2013 at 09:38:07AM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > >> On 17:12 Sun 07 Apr     , Johan Hovold wrote:
> > >>> Add device-tree support.
> > >>>
> > >>> The AT91 RTT can be used as an RTC if the atmel,at91-rtt-as-rtc-gpbr
> > >>> property is present and set to the general-purpose backup register to
> > >>> use to store the RTC time base.
> > >>>
> > >>> Signed-off-by: Johan Hovold <jhovold-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > >>> ---
> > >>>  .../devicetree/bindings/rtc/rtc-at91sam9.txt       | 19 ++++++++++++
> > >>>  drivers/rtc/rtc-at91sam9.c                         | 36 +++++++++++++++++++++-
> > >>>  2 files changed, 54 insertions(+), 1 deletion(-)
> > >>>  create mode 100644 Documentation/devicetree/bindings/rtc/rtc-at91sam9.txt > >>>
> > >>> diff --git
> > >>> a/Documentation/devicetree/bindings/rtc/rtc-at91sam9.txt
> > >>> b/Documentation/devicetree/bindings/rtc/rtc-at91sam9.txt > >>> new
> > >>> file mode 100644
> > >>> index 0000000..0f54988
> > >>> --- /dev/null
> > >>> +++ b/Documentation/devicetree/bindings/rtc/rtc-at91sam9.txt
> > >>> @@ -0,0 +1,19 @@
> > >>> +Atmel AT91 RTT as RTC
> > >>> +=====================
> > >>> +
> > >>> +Required properties:
> > >>> +- compatible: Should be "atmel,at91sam9260-rtt"
> > >>> +- reg: Should contain register location and length
> > >>> +- interrupts: Should contain interrupt for the RTT which is the IRQ line
> > >>> +  shared across all System Controller members.
> > >>> +- atmel,rtt-as-rtc-gpbr: Should contain the backup-register to use to store
> > >>> +  the RTC time base
> > >>> +
> > >>> +Example:
> > >>> +
> > >>> +rtt@fffffd20 {
> > >>> +		compatible = "atmel,at91sam9g45-rtt", "atmel,at91sam9260-rtt";
> > 
> > No, there is no visible difference between the sam9g45 RTT and the
> > sam9260 one. So the most precise compatibility string is still sam9260.
> > If one day we feel the need for a advanced feature that exists on a more
> > recent SoC, we have the possibility to add it at that time...
> 
> Yes, this should be just "atmel,at91sam9260-rtt" to follow the current
> practise in AT91. However, as I mentioned in an earlier mail one could
> interpret
> 
> 	"The first string in the list specifies the exact device that
> 	the node represents in the form "<manufacturer>,<model>". The
> 	following strings represent other devices that the device is
> 	compatible with.
> 
> 	For example, the Freescale MPC8349 System on Chip (SoC) has a
> 	serial device which implements the National Semiconductor
> 	ns16550 register interface. The compatible property for the
> 	MPC8349 serial device should therefore be: compatible =
> 	"fsl,mpc8349-uart", "ns16550". In this case, fsl,mpc8349-uart
> 	specifies the exact device, and ns16550 states that it is
> 	register-level compatible with a National Semiconductor 16550
> 	UART."
> 
> 	http://www.devicetree.org/Device_Tree_Usage#Understanding_the_compatible_Property
> 
> to mean that the compatible property should always be exact SoC-IP
> followed by the first (most generic) compatible one.
here you describe compatible IP not drivers implementation

they are both usuart IP that are compatible at IP level.

Best Regards,
J.

Re: [RFC 4/5] RTC: rtc-at91sam9: add device-tree support

[Johan Hovold]

On Mon, Apr 08, 2013 at 01:11:13PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 12:38 Mon 08 Apr     , Johan Hovold wrote:
> > On Mon, Apr 08, 2013 at 11:57:06AM +0200, Nicolas Ferre wrote:
> > > On 04/08/2013 11:00 AM, Johan Hovold :
> > > > On Mon, Apr 08, 2013 at 09:38:07AM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > > >> On 17:12 Sun 07 Apr     , Johan Hovold wrote:

[...]

> > > >>> +rtt@fffffd20 {
> > > >>> +		compatible = "atmel,at91sam9g45-rtt", "atmel,at91sam9260-rtt";
> > > 
> > > No, there is no visible difference between the sam9g45 RTT and the
> > > sam9260 one. So the most precise compatibility string is still sam9260.
> > > If one day we feel the need for a advanced feature that exists on a more
> > > recent SoC, we have the possibility to add it at that time...
> > 
> > Yes, this should be just "atmel,at91sam9260-rtt" to follow the current
> > practise in AT91. However, as I mentioned in an earlier mail one could
> > interpret
> > 
> > 	"The first string in the list specifies the exact device that
> > 	the node represents in the form "<manufacturer>,<model>". The
> > 	following strings represent other devices that the device is
> > 	compatible with.
> > 
> > 	For example, the Freescale MPC8349 System on Chip (SoC) has a
> > 	serial device which implements the National Semiconductor
> > 	ns16550 register interface. The compatible property for the
> > 	MPC8349 serial device should therefore be: compatible =
> > 	"fsl,mpc8349-uart", "ns16550". In this case, fsl,mpc8349-uart
> > 	specifies the exact device, and ns16550 states that it is
> > 	register-level compatible with a National Semiconductor 16550
> > 	UART."
> > 
> > 	http://www.devicetree.org/Device_Tree_Usage#Understanding_the_compatible_Property
> > 
> > to mean that the compatible property should always be exact SoC-IP
> > followed by the first (most generic) compatible one.
>
> here you describe compatible IP not drivers implementation

I'm referring to the IP and not drivers.

> they are both usuart IP that are compatible at IP level.

Fair enough, I see how this can be considered to be in accordance with
the above quotation.

Johan