[rtc-linux] [PATCH v3 0/4] SA1100 RTC clean-up for ARM64

[rtc-linux] [PATCH v3 0/4] SA1100 RTC clean-up for ARM64

[Rob Herring]

This series enables building the SA1100 RTC driver for ARM64 in
preparation for enabling CONFIG_ARCH_MMP on ARM64. This is needed for
supporting the PXA1928 SOC.

The previous versions can be found here[1][2]. Changes in this version are
some whitespace fixes and using BIT() macro. I've added Robert's ack as
well.

Russell and Eric/Haojian, Can I get an ack for the SA1100 and MMP bits so
Alexandre can apply.

Rob

[1] http://comments.gmane.org/gmane.linux.ports.arm.kernel/391410
[2] http://comments.gmane.org/gmane.linux.ports.arm.kernel/409740

Rob Herring (4):
  ARM: pxa: add memory resource to RTC device
  rtc: sa1100: convert to run-time register mapping
  ARM: sa1100: remove unused RTC register definitions
  ARM: mmp: remove unused RTC register definitions

 arch/arm/mach-mmp/include/mach/regs-rtc.h   | 23 --------
 arch/arm/mach-pxa/devices.c                 | 18 +-----
 arch/arm/mach-sa1100/include/mach/SA-1100.h | 34 -----------
 drivers/rtc/rtc-sa1100.c                    | 87 ++++++++++++++++++++---------
 4 files changed, 62 insertions(+), 100 deletions(-)
 delete mode 100644 arch/arm/mach-mmp/include/mach/regs-rtc.h

--
2.1.0

-- 
-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[PATCH v3 0/4] SA1100 RTC clean-up for ARM64

[Rob Herring]

This series enables building the SA1100 RTC driver for ARM64 in
preparation for enabling CONFIG_ARCH_MMP on ARM64. This is needed for
supporting the PXA1928 SOC.

The previous versions can be found here[1][2]. Changes in this version are
some whitespace fixes and using BIT() macro. I've added Robert's ack as
well.

Russell and Eric/Haojian, Can I get an ack for the SA1100 and MMP bits so
Alexandre can apply.

Rob

[1] http://comments.gmane.org/gmane.linux.ports.arm.kernel/391410
[2] http://comments.gmane.org/gmane.linux.ports.arm.kernel/409740

Rob Herring (4):
  ARM: pxa: add memory resource to RTC device
  rtc: sa1100: convert to run-time register mapping
  ARM: sa1100: remove unused RTC register definitions
  ARM: mmp: remove unused RTC register definitions

 arch/arm/mach-mmp/include/mach/regs-rtc.h   | 23 --------
 arch/arm/mach-pxa/devices.c                 | 18 +-----
 arch/arm/mach-sa1100/include/mach/SA-1100.h | 34 -----------
 drivers/rtc/rtc-sa1100.c                    | 87 ++++++++++++++++++++---------
 4 files changed, 62 insertions(+), 100 deletions(-)
 delete mode 100644 arch/arm/mach-mmp/include/mach/regs-rtc.h

--
2.1.0

[rtc-linux] [PATCH v3 1/4] ARM: pxa: add memory resource to RTC device

[Rob Herring]

Add the memory resource for the sa1100-rtc device. Since the memory
resource is already present in the pxa_rtc_resources, that makes
sa1100_rtc_resources and pxa_rtc_resources equivalent, so use
pxa_rtc_resources for both devices and remove the duplicate
sa1100_rtc_resources.

Signed-off-by: Rob Herring <robh@kernel.org>
Cc: Daniel Mack <daniel@zonque.org>
Cc: Haojian Zhuang <haojian.zhuang@gmail.com>
Acked-by: Robert Jarzmik <robert.jarzmik@free.fr>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: linux-arm-kernel@lists.infradead.org
---
 arch/arm/mach-pxa/devices.c | 18 ++----------------
 1 file changed, 2 insertions(+), 16 deletions(-)

diff --git a/arch/arm/mach-pxa/devices.c b/arch/arm/mach-pxa/devices.c
index 3543466..b598db6 100644
--- a/arch/arm/mach-pxa/devices.c
+++ b/arch/arm/mach-pxa/devices.c
@@ -439,25 +439,11 @@ struct platform_device pxa_device_rtc = {
 	.resource       = pxa_rtc_resources,
 };
 
-static struct resource sa1100_rtc_resources[] = {
-	{
-		.start  = IRQ_RTC1Hz,
-		.end    = IRQ_RTC1Hz,
-		.name	= "rtc 1Hz",
-		.flags  = IORESOURCE_IRQ,
-	}, {
-		.start  = IRQ_RTCAlrm,
-		.end    = IRQ_RTCAlrm,
-		.name	= "rtc alarm",
-		.flags  = IORESOURCE_IRQ,
-	},
-};
-
 struct platform_device sa1100_device_rtc = {
 	.name		= "sa1100-rtc",
 	.id		= -1,
-	.num_resources	= ARRAY_SIZE(sa1100_rtc_resources),
-	.resource	= sa1100_rtc_resources,
+	.num_resources  = ARRAY_SIZE(pxa_rtc_resources),
+	.resource       = pxa_rtc_resources,
 };
 
 static struct resource pxa_ac97_resources[] = {
-- 
2.1.0

-- 
-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[PATCH v3 1/4] ARM: pxa: add memory resource to RTC device

[Rob Herring]

Add the memory resource for the sa1100-rtc device. Since the memory
resource is already present in the pxa_rtc_resources, that makes
sa1100_rtc_resources and pxa_rtc_resources equivalent, so use
pxa_rtc_resources for both devices and remove the duplicate
sa1100_rtc_resources.

Signed-off-by: Rob Herring <robh@kernel.org>
Cc: Daniel Mack <daniel@zonque.org>
Cc: Haojian Zhuang <haojian.zhuang@gmail.com>
Acked-by: Robert Jarzmik <robert.jarzmik@free.fr>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: linux-arm-kernel at lists.infradead.org
---
 arch/arm/mach-pxa/devices.c | 18 ++----------------
 1 file changed, 2 insertions(+), 16 deletions(-)

diff --git a/arch/arm/mach-pxa/devices.c b/arch/arm/mach-pxa/devices.c
index 3543466..b598db6 100644
--- a/arch/arm/mach-pxa/devices.c
+++ b/arch/arm/mach-pxa/devices.c
@@ -439,25 +439,11 @@ struct platform_device pxa_device_rtc = {
 	.resource       = pxa_rtc_resources,
 };
 
-static struct resource sa1100_rtc_resources[] = {
-	{
-		.start  = IRQ_RTC1Hz,
-		.end    = IRQ_RTC1Hz,
-		.name	= "rtc 1Hz",
-		.flags  = IORESOURCE_IRQ,
-	}, {
-		.start  = IRQ_RTCAlrm,
-		.end    = IRQ_RTCAlrm,
-		.name	= "rtc alarm",
-		.flags  = IORESOURCE_IRQ,
-	},
-};
-
 struct platform_device sa1100_device_rtc = {
 	.name		= "sa1100-rtc",
 	.id		= -1,
-	.num_resources	= ARRAY_SIZE(sa1100_rtc_resources),
-	.resource	= sa1100_rtc_resources,
+	.num_resources  = ARRAY_SIZE(pxa_rtc_resources),
+	.resource       = pxa_rtc_resources,
 };
 
 static struct resource pxa_ac97_resources[] = {
-- 
2.1.0

[rtc-linux] Re: [PATCH v3 1/4] ARM: pxa: add memory resource to RTC device

[Russell King - ARM Linux]

On Mon, May 11, 2015 at 05:41:26PM -0500, Rob Herring wrote:
> Add the memory resource for the sa1100-rtc device. Since the memory
> resource is already present in the pxa_rtc_resources, that makes
> sa1100_rtc_resources and pxa_rtc_resources equivalent, so use
> pxa_rtc_resources for both devices and remove the duplicate
> sa1100_rtc_resources.

This really isn't a good idea - what do you think happens when
the same struct resource gets passed into insert_resource()
twice?

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

-- 
-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[PATCH v3 1/4] ARM: pxa: add memory resource to RTC device

[Russell King - ARM Linux]

On Mon, May 11, 2015 at 05:41:26PM -0500, Rob Herring wrote:
> Add the memory resource for the sa1100-rtc device. Since the memory
> resource is already present in the pxa_rtc_resources, that makes
> sa1100_rtc_resources and pxa_rtc_resources equivalent, so use
> pxa_rtc_resources for both devices and remove the duplicate
> sa1100_rtc_resources.

This really isn't a good idea - what do you think happens when
the same struct resource gets passed into insert_resource()
twice?

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

[rtc-linux] Re: [PATCH v3 1/4] ARM: pxa: add memory resource to RTC device

[Rob Herring]

On Mon, May 11, 2015 at 5:49 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Mon, May 11, 2015 at 05:41:26PM -0500, Rob Herring wrote:
>> Add the memory resource for the sa1100-rtc device. Since the memory
>> resource is already present in the pxa_rtc_resources, that makes
>> sa1100_rtc_resources and pxa_rtc_resources equivalent, so use
>> pxa_rtc_resources for both devices and remove the duplicate
>> sa1100_rtc_resources.
>
> This really isn't a good idea - what do you think happens when
> the same struct resource gets passed into insert_resource()
> twice?

Bad things. If you recall, we discussed this on v1[1]. See the commit
msg of patch 2 for the summary. There is not currently a problem
because the rtc-pxa driver does not request the resource.

Rob

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2015-February/322355.html

>
> --
> FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
> according to speedtest.net.

-- 
-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[PATCH v3 1/4] ARM: pxa: add memory resource to RTC device

[Rob Herring]

On Mon, May 11, 2015 at 5:49 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Mon, May 11, 2015 at 05:41:26PM -0500, Rob Herring wrote:
>> Add the memory resource for the sa1100-rtc device. Since the memory
>> resource is already present in the pxa_rtc_resources, that makes
>> sa1100_rtc_resources and pxa_rtc_resources equivalent, so use
>> pxa_rtc_resources for both devices and remove the duplicate
>> sa1100_rtc_resources.
>
> This really isn't a good idea - what do you think happens when
> the same struct resource gets passed into insert_resource()
> twice?

Bad things. If you recall, we discussed this on v1[1]. See the commit
msg of patch 2 for the summary. There is not currently a problem
because the rtc-pxa driver does not request the resource.

Rob

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2015-February/322355.html

>
> --
> FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
> according to speedtest.net.

[rtc-linux] Re: [PATCH v3 1/4] ARM: pxa: add memory resource to RTC device

[Robert Jarzmik]

Rob Herring <robh@kernel.org> writes:

>> This really isn't a good idea - what do you think happens when
>> the same struct resource gets passed into insert_resource()
>> twice?
>
> Bad things. If you recall, we discussed this on v1[1]. See the commit
> msg of patch 2 for the summary. There is not currently a problem
> because the rtc-pxa driver does not request the resource.

I think you're talking about resource claiming, while Russell is talking about
resource declaration.

The point Russell is raising is if you do a platform_add_device() twice with the
same resource (which is done in pxa, once for sa1100-rtc, once for pxa-rtc), you
call insert_resource() twice with the same resource structure.

I had not thought of that before (that's in my reply noted "nasty consequences"
in [1]), and I'll do my homework this week, and try this only patch, but if I
follow Russell's thinking, then sa1100-rtc and pxa-rtc cannot be declared
together anymore.

Cheers.

--
Robert

-- 
-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[PATCH v3 1/4] ARM: pxa: add memory resource to RTC device

[Robert Jarzmik]

Rob Herring <robh@kernel.org> writes:

>> This really isn't a good idea - what do you think happens when
>> the same struct resource gets passed into insert_resource()
>> twice?
>
> Bad things. If you recall, we discussed this on v1[1]. See the commit
> msg of patch 2 for the summary. There is not currently a problem
> because the rtc-pxa driver does not request the resource.

I think you're talking about resource claiming, while Russell is talking about
resource declaration.

The point Russell is raising is if you do a platform_add_device() twice with the
same resource (which is done in pxa, once for sa1100-rtc, once for pxa-rtc), you
call insert_resource() twice with the same resource structure.

I had not thought of that before (that's in my reply noted "nasty consequences"
in [1]), and I'll do my homework this week, and try this only patch, but if I
follow Russell's thinking, then sa1100-rtc and pxa-rtc cannot be declared
together anymore.

Cheers.

--
Robert

Re: [PATCH v3 1/4] ARM: pxa: add memory resource to RTC device

[Robert Jarzmik]

Robert Jarzmik <robert.jarzmik@free.fr> writes:

> Rob Herring <robh@kernel.org> writes:
>
>>> This really isn't a good idea - what do you think happens when
>>> the same struct resource gets passed into insert_resource()
>>> twice?
>>
>> Bad things. If you recall, we discussed this on v1[1]. See the commit
>> msg of patch 2 for the summary. There is not currently a problem
>> because the rtc-pxa driver does not request the resource.
>
> I think you're talking about resource claiming, while Russell is talking about
> resource declaration.
>
> The point Russell is raising is if you do a platform_add_device() twice with the
> same resource (which is done in pxa, once for sa1100-rtc, once for pxa-rtc), you
> call insert_resource() twice with the same resource structure.
>
> I had not thought of that before (that's in my reply noted "nasty consequences"
> in [1]), and I'll do my homework this week, and try this only patch, but if I
> follow Russell's thinking, then sa1100-rtc and pxa-rtc cannot be declared
> together anymore.

I made the try, and Russell was right, this breaks the pxa architecture. The
relevant extract of kernel message is in [1].

The consequences with your patches :
 - pxa27x_init()
   - platform_add_devices()
     - the pxa_device_rtc fails (as resource is duplicated)
     - all the devices are unregistered (rollback)
And pxa27x fails.

Now I'm pondering about the right approach :
 - either remove sa1100_device_rtc from pxas
 - or remove pxa_device_rtc
 - or both
 - or something else

Let me think a bit about it.

-- 
Robert

[1]
2015/05/12  22:12:57  ------------[ cut here ]------------
2015/05/12  22:12:57  WARNING: CPU: 0 PID: 1 at kernel/resource.c:755 __insert_resource+0x74/0x134()
2015/05/12  22:12:57  Modules linked in:
2015/05/12  22:12:57  CPU: 0 PID: 1 Comm: swapper Tainted: G        W       4.0.0-00004-g7fa5ed0 #528
2015/05/12  22:12:57  Hardware name: Intel HCDDBBVA0 Development Platform (aka Mainstone)
2015/05/12  22:12:57  [<c000e170>] (unwind_backtrace) from [<c000c064>] (show_stack+0x10/0x14)
2015/05/12  22:12:57  [<c000c064>] (show_stack) from [<c00176b0>] (warn_slowpath_common+0x7c/0xb4)
2015/05/12  22:12:57  [<c00176b0>] (warn_slowpath_common) from [<c0017784>] (warn_slowpath_null+0x1c/0x24)
2015/05/12  22:12:57  [<c0017784>] (warn_slowpath_null) from [<c001ada4>] (__insert_resource+0x74/0x134)
2015/05/12  22:12:57  [<c001ada4>] (__insert_resource) from [<c001bfd4>] (insert_resource_conflict+0x20/0x60)
2015/05/12  22:12:57  [<c001bfd4>] (insert_resource_conflict) from [<c001c01c>] (insert_resource+0x8/0x18)
2015/05/12  22:12:57  [<c001c01c>] (insert_resource) from [<c0207a7c>] (platform_device_add+0x88/0x254)
2015/05/12  22:12:57  [<c0207a7c>] (platform_device_add) from [<c02082c4>] (platform_add_devices+0x34/0x70)
2015/05/12  22:12:57  [<c02082c4>] (platform_add_devices) from [<c00088e0>] (do_one_initcall+0xa8/0x204)
2015/05/12  22:12:58  [<c00088e0>] (do_one_initcall) from [<c050dd6c>] (kernel_init_freeable+0xf4/0x1b4)
2015/05/12  22:12:58  [<c050dd6c>] (kernel_init_freeable) from [<c03b5a9c>] (kernel_init+0x8/0xec)
2015/05/12  22:12:58  [<c03b5a9c>] (kernel_init) from [<c0009698>] (ret_from_fork+0x14/0x3c)
2015/05/12  22:12:58  ---[ end trace cb88537fdc8fa201 ]---
2015/05/12  22:12:58  platform pxa-rtc: failed to claim resource 0

[PATCH v3 1/4] ARM: pxa: add memory resource to RTC device

[Robert Jarzmik]

Robert Jarzmik <robert.jarzmik@free.fr> writes:

> Rob Herring <robh@kernel.org> writes:
>
>>> This really isn't a good idea - what do you think happens when
>>> the same struct resource gets passed into insert_resource()
>>> twice?
>>
>> Bad things. If you recall, we discussed this on v1[1]. See the commit
>> msg of patch 2 for the summary. There is not currently a problem
>> because the rtc-pxa driver does not request the resource.
>
> I think you're talking about resource claiming, while Russell is talking about
> resource declaration.
>
> The point Russell is raising is if you do a platform_add_device() twice with the
> same resource (which is done in pxa, once for sa1100-rtc, once for pxa-rtc), you
> call insert_resource() twice with the same resource structure.
>
> I had not thought of that before (that's in my reply noted "nasty consequences"
> in [1]), and I'll do my homework this week, and try this only patch, but if I
> follow Russell's thinking, then sa1100-rtc and pxa-rtc cannot be declared
> together anymore.

I made the try, and Russell was right, this breaks the pxa architecture. The
relevant extract of kernel message is in [1].

The consequences with your patches :
 - pxa27x_init()
   - platform_add_devices()
     - the pxa_device_rtc fails (as resource is duplicated)
     - all the devices are unregistered (rollback)
And pxa27x fails.

Now I'm pondering about the right approach :
 - either remove sa1100_device_rtc from pxas
 - or remove pxa_device_rtc
 - or both
 - or something else

Let me think a bit about it.

-- 
Robert

[1]
2015/05/12  22:12:57  ------------[ cut here ]------------
2015/05/12  22:12:57  WARNING: CPU: 0 PID: 1 at kernel/resource.c:755 __insert_resource+0x74/0x134()
2015/05/12  22:12:57  Modules linked in:
2015/05/12  22:12:57  CPU: 0 PID: 1 Comm: swapper Tainted: G        W       4.0.0-00004-g7fa5ed0 #528
2015/05/12  22:12:57  Hardware name: Intel HCDDBBVA0 Development Platform (aka Mainstone)
2015/05/12  22:12:57  [<c000e170>] (unwind_backtrace) from [<c000c064>] (show_stack+0x10/0x14)
2015/05/12  22:12:57  [<c000c064>] (show_stack) from [<c00176b0>] (warn_slowpath_common+0x7c/0xb4)
2015/05/12  22:12:57  [<c00176b0>] (warn_slowpath_common) from [<c0017784>] (warn_slowpath_null+0x1c/0x24)
2015/05/12  22:12:57  [<c0017784>] (warn_slowpath_null) from [<c001ada4>] (__insert_resource+0x74/0x134)
2015/05/12  22:12:57  [<c001ada4>] (__insert_resource) from [<c001bfd4>] (insert_resource_conflict+0x20/0x60)
2015/05/12  22:12:57  [<c001bfd4>] (insert_resource_conflict) from [<c001c01c>] (insert_resource+0x8/0x18)
2015/05/12  22:12:57  [<c001c01c>] (insert_resource) from [<c0207a7c>] (platform_device_add+0x88/0x254)
2015/05/12  22:12:57  [<c0207a7c>] (platform_device_add) from [<c02082c4>] (platform_add_devices+0x34/0x70)
2015/05/12  22:12:57  [<c02082c4>] (platform_add_devices) from [<c00088e0>] (do_one_initcall+0xa8/0x204)
2015/05/12  22:12:58  [<c00088e0>] (do_one_initcall) from [<c050dd6c>] (kernel_init_freeable+0xf4/0x1b4)
2015/05/12  22:12:58  [<c050dd6c>] (kernel_init_freeable) from [<c03b5a9c>] (kernel_init+0x8/0xec)
2015/05/12  22:12:58  [<c03b5a9c>] (kernel_init) from [<c0009698>] (ret_from_fork+0x14/0x3c)
2015/05/12  22:12:58  ---[ end trace cb88537fdc8fa201 ]---
2015/05/12  22:12:58  platform pxa-rtc: failed to claim resource 0

[rtc-linux] Re: [PATCH v3 1/4] ARM: pxa: add memory resource to RTC device

[Arnd Bergmann]

On Tuesday 12 May 2015 22:24:49 Robert Jarzmik wrote:
> 
> I made the try, and Russell was right, this breaks the pxa architecture. The
> relevant extract of kernel message is in [1].
> 
> The consequences with your patches :
>  - pxa27x_init()
>    - platform_add_devices()
>      - the pxa_device_rtc fails (as resource is duplicated)
>      - all the devices are unregistered (rollback)
> And pxa27x fails.
> 
> Now I'm pondering about the right approach :
>  - either remove sa1100_device_rtc from pxas
>  - or remove pxa_device_rtc
>  - or both
>  - or something else
> 
> Let me think a bit about it.
> 

To solve the problem with the duplicate registration of one resource,
I'd suggest using platform_device_register_simple() for the registration,
which will copy the resource. You can then mark the resource as __initconst
and remove the device to save a little memory at runtime.

	Arnd

-- 
-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[PATCH v3 1/4] ARM: pxa: add memory resource to RTC device

[Arnd Bergmann]

On Tuesday 12 May 2015 22:24:49 Robert Jarzmik wrote:
> 
> I made the try, and Russell was right, this breaks the pxa architecture. The
> relevant extract of kernel message is in [1].
> 
> The consequences with your patches :
>  - pxa27x_init()
>    - platform_add_devices()
>      - the pxa_device_rtc fails (as resource is duplicated)
>      - all the devices are unregistered (rollback)
> And pxa27x fails.
> 
> Now I'm pondering about the right approach :
>  - either remove sa1100_device_rtc from pxas
>  - or remove pxa_device_rtc
>  - or both
>  - or something else
> 
> Let me think a bit about it.
> 

To solve the problem with the duplicate registration of one resource,
I'd suggest using platform_device_register_simple() for the registration,
which will copy the resource. You can then mark the resource as __initconst
and remove the device to save a little memory at runtime.

	Arnd

[rtc-linux] Re: [PATCH v3 1/4] ARM: pxa: add memory resource to RTC device

[Russell King - ARM Linux]

On Tue, May 12, 2015 at 10:30:12PM +0200, Arnd Bergmann wrote:
> On Tuesday 12 May 2015 22:24:49 Robert Jarzmik wrote:
> > 
> > I made the try, and Russell was right, this breaks the pxa architecture. The
> > relevant extract of kernel message is in [1].
> > 
> > The consequences with your patches :
> >  - pxa27x_init()
> >    - platform_add_devices()
> >      - the pxa_device_rtc fails (as resource is duplicated)
> >      - all the devices are unregistered (rollback)
> > And pxa27x fails.
> > 
> > Now I'm pondering about the right approach :
> >  - either remove sa1100_device_rtc from pxas
> >  - or remove pxa_device_rtc
> >  - or both
> >  - or something else
> > 
> > Let me think a bit about it.
> > 
> 
> To solve the problem with the duplicate registration of one resource,
> I'd suggest using platform_device_register_simple() for the registration,
> which will copy the resource. You can then mark the resource as __initconst
> and remove the device to save a little memory at runtime.

No, a better solution is to solve the problem which requires the
duplication in the first place, which is a broken driver structure.

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

-- 
-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[PATCH v3 1/4] ARM: pxa: add memory resource to RTC device

[Russell King - ARM Linux]

On Tue, May 12, 2015 at 10:30:12PM +0200, Arnd Bergmann wrote:
> On Tuesday 12 May 2015 22:24:49 Robert Jarzmik wrote:
> > 
> > I made the try, and Russell was right, this breaks the pxa architecture. The
> > relevant extract of kernel message is in [1].
> > 
> > The consequences with your patches :
> >  - pxa27x_init()
> >    - platform_add_devices()
> >      - the pxa_device_rtc fails (as resource is duplicated)
> >      - all the devices are unregistered (rollback)
> > And pxa27x fails.
> > 
> > Now I'm pondering about the right approach :
> >  - either remove sa1100_device_rtc from pxas
> >  - or remove pxa_device_rtc
> >  - or both
> >  - or something else
> > 
> > Let me think a bit about it.
> > 
> 
> To solve the problem with the duplicate registration of one resource,
> I'd suggest using platform_device_register_simple() for the registration,
> which will copy the resource. You can then mark the resource as __initconst
> and remove the device to save a little memory at runtime.

No, a better solution is to solve the problem which requires the
duplication in the first place, which is a broken driver structure.

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

[rtc-linux] Re: [PATCH v3 1/4] ARM: pxa: add memory resource to RTC device

[Arnd Bergmann]

On Tuesday 12 May 2015 21:36:13 Russell King - ARM Linux wrote:
> On Tue, May 12, 2015 at 10:30:12PM +0200, Arnd Bergmann wrote:
> > On Tuesday 12 May 2015 22:24:49 Robert Jarzmik wrote:
> > > 
> > > I made the try, and Russell was right, this breaks the pxa architecture. The
> > > relevant extract of kernel message is in [1].
> > > 
> > > The consequences with your patches :
> > >  - pxa27x_init()
> > >    - platform_add_devices()
> > >      - the pxa_device_rtc fails (as resource is duplicated)
> > >      - all the devices are unregistered (rollback)
> > > And pxa27x fails.
> > > 
> > > Now I'm pondering about the right approach :
> > >  - either remove sa1100_device_rtc from pxas
> > >  - or remove pxa_device_rtc
> > >  - or both
> > >  - or something else
> > > 
> > > Let me think a bit about it.
> > > 
> > 
> > To solve the problem with the duplicate registration of one resource,
> > I'd suggest using platform_device_register_simple() for the registration,
> > which will copy the resource. You can then mark the resource as __initconst
> > and remove the device to save a little memory at runtime.
> 
> No, a better solution is to solve the problem which requires the
> duplication in the first place, which is a broken driver structure.

Yes, makes sense. Or possibly do both ;-)

	Arnd

-- 
-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[PATCH v3 1/4] ARM: pxa: add memory resource to RTC device

[Arnd Bergmann]

On Tuesday 12 May 2015 21:36:13 Russell King - ARM Linux wrote:
> On Tue, May 12, 2015 at 10:30:12PM +0200, Arnd Bergmann wrote:
> > On Tuesday 12 May 2015 22:24:49 Robert Jarzmik wrote:
> > > 
> > > I made the try, and Russell was right, this breaks the pxa architecture. The
> > > relevant extract of kernel message is in [1].
> > > 
> > > The consequences with your patches :
> > >  - pxa27x_init()
> > >    - platform_add_devices()
> > >      - the pxa_device_rtc fails (as resource is duplicated)
> > >      - all the devices are unregistered (rollback)
> > > And pxa27x fails.
> > > 
> > > Now I'm pondering about the right approach :
> > >  - either remove sa1100_device_rtc from pxas
> > >  - or remove pxa_device_rtc
> > >  - or both
> > >  - or something else
> > > 
> > > Let me think a bit about it.
> > > 
> > 
> > To solve the problem with the duplicate registration of one resource,
> > I'd suggest using platform_device_register_simple() for the registration,
> > which will copy the resource. You can then mark the resource as __initconst
> > and remove the device to save a little memory at runtime.
> 
> No, a better solution is to solve the problem which requires the
> duplication in the first place, which is a broken driver structure.

Yes, makes sense. Or possibly do both ;-)

	Arnd

[rtc-linux] Re: [PATCH v3 1/4] ARM: pxa: add memory resource to RTC device

[Rob Herring]

On Tue, May 12, 2015 at 1:20 AM, Robert Jarzmik <robert.jarzmik@free.fr> wrote:
> Rob Herring <robh@kernel.org> writes:
>
>>> This really isn't a good idea - what do you think happens when
>>> the same struct resource gets passed into insert_resource()
>>> twice?
>>
>> Bad things. If you recall, we discussed this on v1[1]. See the commit
>> msg of patch 2 for the summary. There is not currently a problem
>> because the rtc-pxa driver does not request the resource.
>
> I think you're talking about resource claiming, while Russell is talking about
> resource declaration.
>
> The point Russell is raising is if you do a platform_add_device() twice with the
> same resource (which is done in pxa, once for sa1100-rtc, once for pxa-rtc), you
> call insert_resource() twice with the same resource structure.
>
> I had not thought of that before (that's in my reply noted "nasty consequences"
> in [1]), and I'll do my homework this week, and try this only patch, but if I
> follow Russell's thinking, then sa1100-rtc and pxa-rtc cannot be declared
> together anymore.

Oh yes, that is a problem. So looks like we have to make PXA a single driver. 
Please take a look at the patch below. It's on top of this series currently, so 
I've got to go back and re-order things. It is build tested only.

Technically, the locking is all wrong currently, and the spinlock should be 
shared, too. However, since this driver is always on a UP system everything 
is fine.

Rob

8<---------------------------------------------------------

Author: Rob Herring <robh@kernel.org>
Date:   Tue May 12 14:31:13 2015 -0500

    rtc rework
    
    Signed-off-by: Rob Herring <robh@kernel.org>

diff --git a/drivers/rtc/rtc-pxa.c b/drivers/rtc/rtc-pxa.c
index 4561f37..70412c9c 100644
--- a/drivers/rtc/rtc-pxa.c
+++ b/drivers/rtc/rtc-pxa.c
@@ -32,6 +32,8 @@
 
 #include <mach/hardware.h>
 
+#include "rtc-sa1100.h"
+
 #define RTC_DEF_DIVIDER		(32768 - 1)
 #define RTC_DEF_TRIM		0
 #define MAXFREQ_PERIODIC	1000
@@ -86,6 +88,7 @@
 	__raw_writel((value), (pxa_rtc)->base + (reg))
 
 struct pxa_rtc {
+	struct sa1100_rtc sa1100_rtc;
 	struct resource	*ress;
 	void __iomem		*base;
 	int			irq_1Hz;
@@ -321,7 +324,6 @@ static int __init pxa_rtc_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct pxa_rtc *pxa_rtc;
 	int ret;
-	u32 rttr;
 
 	pxa_rtc = devm_kzalloc(dev, sizeof(*pxa_rtc), GFP_KERNEL);
 	if (!pxa_rtc)
@@ -354,15 +356,16 @@ static int __init pxa_rtc_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
-	/*
-	 * If the clock divider is uninitialized then reset it to the
-	 * default value to get the 1Hz clock.
-	 */
-	if (rtc_readl(pxa_rtc, RTTR) == 0) {
-		rttr = RTC_DEF_DIVIDER + (RTC_DEF_TRIM << 16);
-		rtc_writel(pxa_rtc, RTTR, rttr);
-		dev_warn(dev, "warning: initializing default clock"
-			 " divider/trim value\n");
+	pxa_rtc->sa1100_rtc.rcnr = pxa_rtc->base + 0x0;
+	pxa_rtc->sa1100_rtc.rtsr = pxa_rtc->base + 0x8;
+	pxa_rtc->sa1100_rtc.rtar = pxa_rtc->base + 0x4;
+	pxa_rtc->sa1100_rtc.rttr = pxa_rtc->base + 0xc;
+	pxa_rtc->sa1100_rtc.irq_1hz = pxa_rtc->irq_1Hz;
+	pxa_rtc->sa1100_rtc.irq_alarm = pxa_rtc->irq_Alrm;
+	ret = sa1100_rtc_init(pdev, &pxa_rtc->sa1100_rtc);
+	if (!ret) {
+		dev_err(dev, "Unable to init SA1100 RTC sub-device\n");
+		return ret;
 	}
 
 	rtsr_clear_bits(pxa_rtc, RTSR_PIALE | RTSR_RDALE1 | RTSR_HZE);
diff --git a/drivers/rtc/rtc-sa1100.c b/drivers/rtc/rtc-sa1100.c
index 52d2a8a..6e3f63b 100644
--- a/drivers/rtc/rtc-sa1100.c
+++ b/drivers/rtc/rtc-sa1100.c
@@ -35,6 +35,8 @@
 #include <linux/bitops.h>
 #include <linux/io.h>
 
+#include "rtc-sa1100.h"
+
 #define RTSR_HZE		BIT(3)	/* HZ interrupt enable */
 #define RTSR_ALE		BIT(2)	/* RTC alarm interrupt enable */
 #define RTSR_HZ			BIT(1)	/* HZ rising-edge detected */
@@ -44,17 +46,6 @@
 #define RTC_DEF_TRIM		0
 #define RTC_FREQ		1024
 
-struct sa1100_rtc {
-	spinlock_t		lock;
-	void __iomem		*rcnr;
-	void __iomem		*rtar;
-	void __iomem		*rtsr;
-	void __iomem		*rttr;
-	int			irq_1hz;
-	int			irq_alarm;
-	struct rtc_device	*rtc;
-	struct clk		*clk;
-};
 
 static irqreturn_t sa1100_rtc_interrupt(int irq, void *dev_id)
 {
@@ -235,50 +226,18 @@ static const struct rtc_class_ops sa1100_rtc_ops = {
 	.alarm_irq_enable = sa1100_rtc_alarm_irq_enable,
 };
 
-static int sa1100_rtc_probe(struct platform_device *pdev)
+int sa1100_rtc_init(struct platform_device *pdev, struct sa1100_rtc *info)
 {
 	struct rtc_device *rtc;
-	struct sa1100_rtc *info;
-	struct resource *iores;
-	void __iomem *base;
-	int irq_1hz, irq_alarm, ret = 0;
+	int ret;
 
-	irq_1hz = platform_get_irq_byname(pdev, "rtc 1Hz");
-	irq_alarm = platform_get_irq_byname(pdev, "rtc alarm");
-	if (irq_1hz < 0 || irq_alarm < 0)
-		return -ENODEV;
+	spin_lock_init(&info->lock);
 
-	info = devm_kzalloc(&pdev->dev, sizeof(struct sa1100_rtc), GFP_KERNEL);
-	if (!info)
-		return -ENOMEM;
 	info->clk = devm_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(info->clk)) {
 		dev_err(&pdev->dev, "failed to find rtc clock source\n");
 		return PTR_ERR(info->clk);
 	}
-	info->irq_1hz = irq_1hz;
-	info->irq_alarm = irq_alarm;
-
-	iores = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	base = devm_ioremap_resource(&pdev->dev, iores);
-	if (IS_ERR(base))
-		return PTR_ERR(base);
-
-	if (IS_ENABLED(CONFIG_ARCH_SA1100) ||
-	    of_device_is_compatible(pdev->dev.of_node, "mrvl,sa1100-rtc")) {
-		info->rcnr = base + 0x04;
-		info->rtsr = base + 0x10;
-		info->rtar = base + 0x00;
-		info->rttr = base + 0x08;
-	} else {
-		info->rcnr = base + 0x0;
-		info->rtsr = base + 0x8;
-		info->rtar = base + 0x4;
-		info->rttr = base + 0xc;
-	}
-
-	spin_lock_init(&info->lock);
-	platform_set_drvdata(pdev, info);
 
 	ret = clk_prepare_enable(info->clk);
 	if (ret)
@@ -298,14 +257,11 @@ static int sa1100_rtc_probe(struct platform_device *pdev)
 		writel_relaxed(0, info->rcnr);
 	}
 
-	device_init_wakeup(&pdev->dev, 1);
-
 	rtc = devm_rtc_device_register(&pdev->dev, pdev->name, &sa1100_rtc_ops,
 					THIS_MODULE);
-
 	if (IS_ERR(rtc)) {
-		ret = PTR_ERR(rtc);
-		goto err_dev;
+		clk_disable_unprepare(info->clk);
+		return PTR_ERR(rtc);
 	}
 	info->rtc = rtc;
 
@@ -334,9 +290,49 @@ static int sa1100_rtc_probe(struct platform_device *pdev)
 	writel_relaxed(RTSR_AL | RTSR_HZ, info->rtsr);
 
 	return 0;
-err_dev:
-	clk_disable_unprepare(info->clk);
-	return ret;
+}
+EXPORT_SYMBOL_GPL(sa1100_rtc_init);
+
+static int sa1100_rtc_probe(struct platform_device *pdev)
+{
+	struct sa1100_rtc *info;
+	struct resource *iores;
+	void __iomem *base;
+	int irq_1hz, irq_alarm;
+
+	irq_1hz = platform_get_irq_byname(pdev, "rtc 1Hz");
+	irq_alarm = platform_get_irq_byname(pdev, "rtc alarm");
+	if (irq_1hz < 0 || irq_alarm < 0)
+		return -ENODEV;
+
+	info = devm_kzalloc(&pdev->dev, sizeof(struct sa1100_rtc), GFP_KERNEL);
+	if (!info)
+		return -ENOMEM;
+	info->irq_1hz = irq_1hz;
+	info->irq_alarm = irq_alarm;
+
+	iores = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	base = devm_ioremap_resource(&pdev->dev, iores);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	if (IS_ENABLED(CONFIG_ARCH_SA1100) ||
+	    of_device_is_compatible(pdev->dev.of_node, "mrvl,sa1100-rtc")) {
+		info->rcnr = base + 0x04;
+		info->rtsr = base + 0x10;
+		info->rtar = base + 0x00;
+		info->rttr = base + 0x08;
+	} else {
+		info->rcnr = base + 0x0;
+		info->rtsr = base + 0x8;
+		info->rtar = base + 0x4;
+		info->rttr = base + 0xc;
+	}
+
+	platform_set_drvdata(pdev, info);
+	device_init_wakeup(&pdev->dev, 1);
+
+	return sa1100_rtc_init(pdev, info);
 }
 
 static int sa1100_rtc_remove(struct platform_device *pdev)
diff --git a/drivers/rtc/rtc-sa1100.h b/drivers/rtc/rtc-sa1100.h
new file mode 100644
index 0000000..2c79c0c
--- /dev/null
+++ b/drivers/rtc/rtc-sa1100.h
@@ -0,0 +1,23 @@
+#ifndef __RTC_SA1100_H__
+#define __RTC_SA1100_H__
+
+#include <linux/kernel.h>
+
+struct clk;
+struct platform_device;
+
+struct sa1100_rtc {
+	spinlock_t		lock;
+	void __iomem		*rcnr;
+	void __iomem		*rtar;
+	void __iomem		*rtsr;
+	void __iomem		*rttr;
+	int			irq_1hz;
+	int			irq_alarm;
+	struct rtc_device	*rtc;
+	struct clk		*clk;
+};
+
+int sa1100_rtc_init(struct platform_device *pdev, struct sa1100_rtc *info);
+
+#endif

-- 
-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[PATCH v3 1/4] ARM: pxa: add memory resource to RTC device

[Rob Herring]

On Tue, May 12, 2015 at 1:20 AM, Robert Jarzmik <robert.jarzmik@free.fr> wrote:
> Rob Herring <robh@kernel.org> writes:
>
>>> This really isn't a good idea - what do you think happens when
>>> the same struct resource gets passed into insert_resource()
>>> twice?
>>
>> Bad things. If you recall, we discussed this on v1[1]. See the commit
>> msg of patch 2 for the summary. There is not currently a problem
>> because the rtc-pxa driver does not request the resource.
>
> I think you're talking about resource claiming, while Russell is talking about
> resource declaration.
>
> The point Russell is raising is if you do a platform_add_device() twice with the
> same resource (which is done in pxa, once for sa1100-rtc, once for pxa-rtc), you
> call insert_resource() twice with the same resource structure.
>
> I had not thought of that before (that's in my reply noted "nasty consequences"
> in [1]), and I'll do my homework this week, and try this only patch, but if I
> follow Russell's thinking, then sa1100-rtc and pxa-rtc cannot be declared
> together anymore.

Oh yes, that is a problem. So looks like we have to make PXA a single driver. 
Please take a look at the patch below. It's on top of this series currently, so 
I've got to go back and re-order things. It is build tested only.

Technically, the locking is all wrong currently, and the spinlock should be 
shared, too. However, since this driver is always on a UP system everything 
is fine.

Rob

8<---------------------------------------------------------

Author: Rob Herring <robh@kernel.org>
Date:   Tue May 12 14:31:13 2015 -0500

    rtc rework
    
    Signed-off-by: Rob Herring <robh@kernel.org>

diff --git a/drivers/rtc/rtc-pxa.c b/drivers/rtc/rtc-pxa.c
index 4561f37..70412c9c 100644
--- a/drivers/rtc/rtc-pxa.c
+++ b/drivers/rtc/rtc-pxa.c
@@ -32,6 +32,8 @@
 
 #include <mach/hardware.h>
 
+#include "rtc-sa1100.h"
+
 #define RTC_DEF_DIVIDER		(32768 - 1)
 #define RTC_DEF_TRIM		0
 #define MAXFREQ_PERIODIC	1000
@@ -86,6 +88,7 @@
 	__raw_writel((value), (pxa_rtc)->base + (reg))
 
 struct pxa_rtc {
+	struct sa1100_rtc sa1100_rtc;
 	struct resource	*ress;
 	void __iomem		*base;
 	int			irq_1Hz;
@@ -321,7 +324,6 @@ static int __init pxa_rtc_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct pxa_rtc *pxa_rtc;
 	int ret;
-	u32 rttr;
 
 	pxa_rtc = devm_kzalloc(dev, sizeof(*pxa_rtc), GFP_KERNEL);
 	if (!pxa_rtc)
@@ -354,15 +356,16 @@ static int __init pxa_rtc_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
-	/*
-	 * If the clock divider is uninitialized then reset it to the
-	 * default value to get the 1Hz clock.
-	 */
-	if (rtc_readl(pxa_rtc, RTTR) == 0) {
-		rttr = RTC_DEF_DIVIDER + (RTC_DEF_TRIM << 16);
-		rtc_writel(pxa_rtc, RTTR, rttr);
-		dev_warn(dev, "warning: initializing default clock"
-			 " divider/trim value\n");
+	pxa_rtc->sa1100_rtc.rcnr = pxa_rtc->base + 0x0;
+	pxa_rtc->sa1100_rtc.rtsr = pxa_rtc->base + 0x8;
+	pxa_rtc->sa1100_rtc.rtar = pxa_rtc->base + 0x4;
+	pxa_rtc->sa1100_rtc.rttr = pxa_rtc->base + 0xc;
+	pxa_rtc->sa1100_rtc.irq_1hz = pxa_rtc->irq_1Hz;
+	pxa_rtc->sa1100_rtc.irq_alarm = pxa_rtc->irq_Alrm;
+	ret = sa1100_rtc_init(pdev, &pxa_rtc->sa1100_rtc);
+	if (!ret) {
+		dev_err(dev, "Unable to init SA1100 RTC sub-device\n");
+		return ret;
 	}
 
 	rtsr_clear_bits(pxa_rtc, RTSR_PIALE | RTSR_RDALE1 | RTSR_HZE);
diff --git a/drivers/rtc/rtc-sa1100.c b/drivers/rtc/rtc-sa1100.c
index 52d2a8a..6e3f63b 100644
--- a/drivers/rtc/rtc-sa1100.c
+++ b/drivers/rtc/rtc-sa1100.c
@@ -35,6 +35,8 @@
 #include <linux/bitops.h>
 #include <linux/io.h>
 
+#include "rtc-sa1100.h"
+
 #define RTSR_HZE		BIT(3)	/* HZ interrupt enable */
 #define RTSR_ALE		BIT(2)	/* RTC alarm interrupt enable */
 #define RTSR_HZ			BIT(1)	/* HZ rising-edge detected */
@@ -44,17 +46,6 @@
 #define RTC_DEF_TRIM		0
 #define RTC_FREQ		1024
 
-struct sa1100_rtc {
-	spinlock_t		lock;
-	void __iomem		*rcnr;
-	void __iomem		*rtar;
-	void __iomem		*rtsr;
-	void __iomem		*rttr;
-	int			irq_1hz;
-	int			irq_alarm;
-	struct rtc_device	*rtc;
-	struct clk		*clk;
-};
 
 static irqreturn_t sa1100_rtc_interrupt(int irq, void *dev_id)
 {
@@ -235,50 +226,18 @@ static const struct rtc_class_ops sa1100_rtc_ops = {
 	.alarm_irq_enable = sa1100_rtc_alarm_irq_enable,
 };
 
-static int sa1100_rtc_probe(struct platform_device *pdev)
+int sa1100_rtc_init(struct platform_device *pdev, struct sa1100_rtc *info)
 {
 	struct rtc_device *rtc;
-	struct sa1100_rtc *info;
-	struct resource *iores;
-	void __iomem *base;
-	int irq_1hz, irq_alarm, ret = 0;
+	int ret;
 
-	irq_1hz = platform_get_irq_byname(pdev, "rtc 1Hz");
-	irq_alarm = platform_get_irq_byname(pdev, "rtc alarm");
-	if (irq_1hz < 0 || irq_alarm < 0)
-		return -ENODEV;
+	spin_lock_init(&info->lock);
 
-	info = devm_kzalloc(&pdev->dev, sizeof(struct sa1100_rtc), GFP_KERNEL);
-	if (!info)
-		return -ENOMEM;
 	info->clk = devm_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(info->clk)) {
 		dev_err(&pdev->dev, "failed to find rtc clock source\n");
 		return PTR_ERR(info->clk);
 	}
-	info->irq_1hz = irq_1hz;
-	info->irq_alarm = irq_alarm;
-
-	iores = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	base = devm_ioremap_resource(&pdev->dev, iores);
-	if (IS_ERR(base))
-		return PTR_ERR(base);
-
-	if (IS_ENABLED(CONFIG_ARCH_SA1100) ||
-	    of_device_is_compatible(pdev->dev.of_node, "mrvl,sa1100-rtc")) {
-		info->rcnr = base + 0x04;
-		info->rtsr = base + 0x10;
-		info->rtar = base + 0x00;
-		info->rttr = base + 0x08;
-	} else {
-		info->rcnr = base + 0x0;
-		info->rtsr = base + 0x8;
-		info->rtar = base + 0x4;
-		info->rttr = base + 0xc;
-	}
-
-	spin_lock_init(&info->lock);
-	platform_set_drvdata(pdev, info);
 
 	ret = clk_prepare_enable(info->clk);
 	if (ret)
@@ -298,14 +257,11 @@ static int sa1100_rtc_probe(struct platform_device *pdev)
 		writel_relaxed(0, info->rcnr);
 	}
 
-	device_init_wakeup(&pdev->dev, 1);
-
 	rtc = devm_rtc_device_register(&pdev->dev, pdev->name, &sa1100_rtc_ops,
 					THIS_MODULE);
-
 	if (IS_ERR(rtc)) {
-		ret = PTR_ERR(rtc);
-		goto err_dev;
+		clk_disable_unprepare(info->clk);
+		return PTR_ERR(rtc);
 	}
 	info->rtc = rtc;
 
@@ -334,9 +290,49 @@ static int sa1100_rtc_probe(struct platform_device *pdev)
 	writel_relaxed(RTSR_AL | RTSR_HZ, info->rtsr);
 
 	return 0;
-err_dev:
-	clk_disable_unprepare(info->clk);
-	return ret;
+}
+EXPORT_SYMBOL_GPL(sa1100_rtc_init);
+
+static int sa1100_rtc_probe(struct platform_device *pdev)
+{
+	struct sa1100_rtc *info;
+	struct resource *iores;
+	void __iomem *base;
+	int irq_1hz, irq_alarm;
+
+	irq_1hz = platform_get_irq_byname(pdev, "rtc 1Hz");
+	irq_alarm = platform_get_irq_byname(pdev, "rtc alarm");
+	if (irq_1hz < 0 || irq_alarm < 0)
+		return -ENODEV;
+
+	info = devm_kzalloc(&pdev->dev, sizeof(struct sa1100_rtc), GFP_KERNEL);
+	if (!info)
+		return -ENOMEM;
+	info->irq_1hz = irq_1hz;
+	info->irq_alarm = irq_alarm;
+
+	iores = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	base = devm_ioremap_resource(&pdev->dev, iores);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	if (IS_ENABLED(CONFIG_ARCH_SA1100) ||
+	    of_device_is_compatible(pdev->dev.of_node, "mrvl,sa1100-rtc")) {
+		info->rcnr = base + 0x04;
+		info->rtsr = base + 0x10;
+		info->rtar = base + 0x00;
+		info->rttr = base + 0x08;
+	} else {
+		info->rcnr = base + 0x0;
+		info->rtsr = base + 0x8;
+		info->rtar = base + 0x4;
+		info->rttr = base + 0xc;
+	}
+
+	platform_set_drvdata(pdev, info);
+	device_init_wakeup(&pdev->dev, 1);
+
+	return sa1100_rtc_init(pdev, info);
 }
 
 static int sa1100_rtc_remove(struct platform_device *pdev)
diff --git a/drivers/rtc/rtc-sa1100.h b/drivers/rtc/rtc-sa1100.h
new file mode 100644
index 0000000..2c79c0c
--- /dev/null
+++ b/drivers/rtc/rtc-sa1100.h
@@ -0,0 +1,23 @@
+#ifndef __RTC_SA1100_H__
+#define __RTC_SA1100_H__
+
+#include <linux/kernel.h>
+
+struct clk;
+struct platform_device;
+
+struct sa1100_rtc {
+	spinlock_t		lock;
+	void __iomem		*rcnr;
+	void __iomem		*rtar;
+	void __iomem		*rtsr;
+	void __iomem		*rttr;
+	int			irq_1hz;
+	int			irq_alarm;
+	struct rtc_device	*rtc;
+	struct clk		*clk;
+};
+
+int sa1100_rtc_init(struct platform_device *pdev, struct sa1100_rtc *info);
+
+#endif

[rtc-linux] Re: [PATCH v3 1/4] ARM: pxa: add memory resource to RTC device

[Russell King - ARM Linux]

On Mon, May 11, 2015 at 07:35:24PM -0500, Rob Herring wrote:
> On Mon, May 11, 2015 at 5:49 PM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> > On Mon, May 11, 2015 at 05:41:26PM -0500, Rob Herring wrote:
> >> Add the memory resource for the sa1100-rtc device. Since the memory
> >> resource is already present in the pxa_rtc_resources, that makes
> >> sa1100_rtc_resources and pxa_rtc_resources equivalent, so use
> >> pxa_rtc_resources for both devices and remove the duplicate
> >> sa1100_rtc_resources.
> >
> > This really isn't a good idea - what do you think happens when
> > the same struct resource gets passed into insert_resource()
> > twice?
> 
> Bad things. If you recall, we discussed this on v1[1]. See the commit
> msg of patch 2 for the summary. There is not currently a problem
> because the rtc-pxa driver does not request the resource.

There /is/ a problem, because it's not only the driver which does this,
it's also the platform device code when the device is registered.
See platform_device_add().

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

-- 
-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[PATCH v3 1/4] ARM: pxa: add memory resource to RTC device

[Russell King - ARM Linux]

On Mon, May 11, 2015 at 07:35:24PM -0500, Rob Herring wrote:
> On Mon, May 11, 2015 at 5:49 PM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> > On Mon, May 11, 2015 at 05:41:26PM -0500, Rob Herring wrote:
> >> Add the memory resource for the sa1100-rtc device. Since the memory
> >> resource is already present in the pxa_rtc_resources, that makes
> >> sa1100_rtc_resources and pxa_rtc_resources equivalent, so use
> >> pxa_rtc_resources for both devices and remove the duplicate
> >> sa1100_rtc_resources.
> >
> > This really isn't a good idea - what do you think happens when
> > the same struct resource gets passed into insert_resource()
> > twice?
> 
> Bad things. If you recall, we discussed this on v1[1]. See the commit
> msg of patch 2 for the summary. There is not currently a problem
> because the rtc-pxa driver does not request the resource.

There /is/ a problem, because it's not only the driver which does this,
it's also the platform device code when the device is registered.
See platform_device_add().

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

[rtc-linux] [PATCH v3 2/4] rtc: sa1100: convert to run-time register mapping

[Rob Herring]

SA1100 and PXA differ only in register offsets which are currently
hardcoded in a machine specific header. Some arm64 platforms (PXA1928)
have this RTC block as well (and not the PXA270 variant).

Convert the driver to use ioremap and set the register offsets dynamically.
Since we are touching all the register accesses, convert them all to
readl_relaxed/writel_relaxed.

Currently, the rtc-sa1100 and rtc-pxa drivers co-exist as rtc-pxa has a
superset of functionality. This commit makes the drivers one step closer
to being mutually exclusive by using devm_ioremap_resource and claiming
the resource. The sharing of overlapping resources does not work if both
drivers claim the resource. That is not done currently, but will be done
as the drivers are converted to DT and follow proper driver rules.
Likely, the common portion of the 2 drivers will be made into library
functions for the SA1100 and PXA drivers to shared.

Signed-off-by: Rob Herring <robh@kernel.org>
Acked-by: Robert Jarzmik <robert.jarzmik@free.fr>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Cc: rtc-linux@googlegroups.com
---
 drivers/rtc/rtc-sa1100.c | 87 +++++++++++++++++++++++++++++++++---------------
 1 file changed, 60 insertions(+), 27 deletions(-)

diff --git a/drivers/rtc/rtc-sa1100.c b/drivers/rtc/rtc-sa1100.c
index b6e1ca0..52d2a8a 100644
--- a/drivers/rtc/rtc-sa1100.c
+++ b/drivers/rtc/rtc-sa1100.c
@@ -35,12 +35,10 @@
 #include <linux/bitops.h>
 #include <linux/io.h>
 
-#include <mach/hardware.h>
-#include <mach/irqs.h>
-
-#if defined(CONFIG_ARCH_PXA) || defined(CONFIG_ARCH_MMP)
-#include <mach/regs-rtc.h>
-#endif
+#define RTSR_HZE		BIT(3)	/* HZ interrupt enable */
+#define RTSR_ALE		BIT(2)	/* RTC alarm interrupt enable */
+#define RTSR_HZ			BIT(1)	/* HZ rising-edge detected */
+#define RTSR_AL			BIT(0)	/* RTC alarm detected */
 
 #define RTC_DEF_DIVIDER		(32768 - 1)
 #define RTC_DEF_TRIM		0
@@ -48,6 +46,10 @@
 
 struct sa1100_rtc {
 	spinlock_t		lock;
+	void __iomem		*rcnr;
+	void __iomem		*rtar;
+	void __iomem		*rtsr;
+	void __iomem		*rttr;
 	int			irq_1hz;
 	int			irq_alarm;
 	struct rtc_device	*rtc;
@@ -63,16 +65,16 @@ static irqreturn_t sa1100_rtc_interrupt(int irq, void *dev_id)
 
 	spin_lock(&info->lock);
 
-	rtsr = RTSR;
+	rtsr = readl_relaxed(info->rtsr);
 	/* clear interrupt sources */
-	RTSR = 0;
+	writel_relaxed(0, info->rtsr);
 	/* Fix for a nasty initialization problem the in SA11xx RTSR register.
 	 * See also the comments in sa1100_rtc_probe(). */
 	if (rtsr & (RTSR_ALE | RTSR_HZE)) {
 		/* This is the original code, before there was the if test
 		 * above. This code does not clear interrupts that were not
 		 * enabled. */
-		RTSR = (RTSR_AL | RTSR_HZ) & (rtsr >> 2);
+		writel_relaxed((RTSR_AL | RTSR_HZ) & (rtsr >> 2), info->rtsr);
 	} else {
 		/* For some reason, it is possible to enter this routine
 		 * without interruptions enabled, it has been tested with
@@ -81,13 +83,13 @@ static irqreturn_t sa1100_rtc_interrupt(int irq, void *dev_id)
 		 * This situation leads to an infinite "loop" of interrupt
 		 * routine calling and as a result the processor seems to
 		 * lock on its first call to open(). */
-		RTSR = RTSR_AL | RTSR_HZ;
+		writel_relaxed(RTSR_AL | RTSR_HZ, info->rtsr);
 	}
 
 	/* clear alarm interrupt if it has occurred */
 	if (rtsr & RTSR_AL)
 		rtsr &= ~RTSR_ALE;
-	RTSR = rtsr & (RTSR_ALE | RTSR_HZE);
+	writel_relaxed(rtsr & (RTSR_ALE | RTSR_HZE), info->rtsr);
 
 	/* update irq data & counter */
 	if (rtsr & RTSR_AL)
@@ -135,7 +137,7 @@ static void sa1100_rtc_release(struct device *dev)
 	struct sa1100_rtc *info = dev_get_drvdata(dev);
 
 	spin_lock_irq(&info->lock);
-	RTSR = 0;
+	writel_relaxed(0, info->rtsr);
 	spin_unlock_irq(&info->lock);
 
 	free_irq(info->irq_alarm, dev);
@@ -144,39 +146,46 @@ static void sa1100_rtc_release(struct device *dev)
 
 static int sa1100_rtc_alarm_irq_enable(struct device *dev, unsigned int enabled)
 {
+	u32 rtsr;
 	struct sa1100_rtc *info = dev_get_drvdata(dev);
 
 	spin_lock_irq(&info->lock);
+	rtsr = readl_relaxed(info->rtsr);
 	if (enabled)
-		RTSR |= RTSR_ALE;
+		rtsr |= RTSR_ALE;
 	else
-		RTSR &= ~RTSR_ALE;
+		rtsr &= ~RTSR_ALE;
+	writel_relaxed(rtsr, info->rtsr);
 	spin_unlock_irq(&info->lock);
 	return 0;
 }
 
 static int sa1100_rtc_read_time(struct device *dev, struct rtc_time *tm)
 {
-	rtc_time_to_tm(RCNR, tm);
+	struct sa1100_rtc *info = dev_get_drvdata(dev);
+
+	rtc_time_to_tm(readl_relaxed(info->rcnr), tm);
 	return 0;
 }
 
 static int sa1100_rtc_set_time(struct device *dev, struct rtc_time *tm)
 {
+	struct sa1100_rtc *info = dev_get_drvdata(dev);
 	unsigned long time;
 	int ret;
 
 	ret = rtc_tm_to_time(tm, &time);
 	if (ret == 0)
-		RCNR = time;
+		writel_relaxed(time, info->rcnr);
 	return ret;
 }
 
 static int sa1100_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 {
 	u32	rtsr;
+	struct sa1100_rtc *info = dev_get_drvdata(dev);
 
-	rtsr = RTSR;
+	rtsr = readl_relaxed(info->rtsr);
 	alrm->enabled = (rtsr & RTSR_ALE) ? 1 : 0;
 	alrm->pending = (rtsr & RTSR_AL) ? 1 : 0;
 	return 0;
@@ -192,12 +201,13 @@ static int sa1100_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 	ret = rtc_tm_to_time(&alrm->time, &time);
 	if (ret != 0)
 		goto out;
-	RTSR = RTSR & (RTSR_HZE|RTSR_ALE|RTSR_AL);
-	RTAR = time;
+	writel_relaxed(readl_relaxed(info->rtsr) &
+		(RTSR_HZE | RTSR_ALE | RTSR_AL), info->rtsr);
+	writel_relaxed(time, info->rtar);
 	if (alrm->enabled)
-		RTSR |= RTSR_ALE;
+		writel_relaxed(readl_relaxed(info->rtsr) | RTSR_ALE, info->rtsr);
 	else
-		RTSR &= ~RTSR_ALE;
+		writel_relaxed(readl_relaxed(info->rtsr) & ~RTSR_ALE, info->rtsr);
 out:
 	spin_unlock_irq(&info->lock);
 
@@ -206,8 +216,10 @@ out:
 
 static int sa1100_rtc_proc(struct device *dev, struct seq_file *seq)
 {
-	seq_printf(seq, "trim/divider\t\t: 0x%08x\n", (u32) RTTR);
-	seq_printf(seq, "RTSR\t\t\t: 0x%08x\n", (u32)RTSR);
+	struct sa1100_rtc *info = dev_get_drvdata(dev);
+
+	seq_printf(seq, "trim/divider\t\t: 0x%08x\n", readl_relaxed(info->rttr));
+	seq_printf(seq, "RTSR\t\t\t: 0x%08x\n", readl_relaxed(info->rtsr));
 
 	return 0;
 }
@@ -227,6 +239,8 @@ static int sa1100_rtc_probe(struct platform_device *pdev)
 {
 	struct rtc_device *rtc;
 	struct sa1100_rtc *info;
+	struct resource *iores;
+	void __iomem *base;
 	int irq_1hz, irq_alarm, ret = 0;
 
 	irq_1hz = platform_get_irq_byname(pdev, "rtc 1Hz");
@@ -244,6 +258,25 @@ static int sa1100_rtc_probe(struct platform_device *pdev)
 	}
 	info->irq_1hz = irq_1hz;
 	info->irq_alarm = irq_alarm;
+
+	iores = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	base = devm_ioremap_resource(&pdev->dev, iores);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	if (IS_ENABLED(CONFIG_ARCH_SA1100) ||
+	    of_device_is_compatible(pdev->dev.of_node, "mrvl,sa1100-rtc")) {
+		info->rcnr = base + 0x04;
+		info->rtsr = base + 0x10;
+		info->rtar = base + 0x00;
+		info->rttr = base + 0x08;
+	} else {
+		info->rcnr = base + 0x0;
+		info->rtsr = base + 0x8;
+		info->rtar = base + 0x4;
+		info->rttr = base + 0xc;
+	}
+
 	spin_lock_init(&info->lock);
 	platform_set_drvdata(pdev, info);
 
@@ -257,12 +290,12 @@ static int sa1100_rtc_probe(struct platform_device *pdev)
 	 * If the clock divider is uninitialized then reset it to the
 	 * default value to get the 1Hz clock.
 	 */
-	if (RTTR == 0) {
-		RTTR = RTC_DEF_DIVIDER + (RTC_DEF_TRIM << 16);
+	if (readl_relaxed(info->rttr) == 0) {
+		writel_relaxed(RTC_DEF_DIVIDER + (RTC_DEF_TRIM << 16), info->rttr);
 		dev_warn(&pdev->dev, "warning: "
 			"initializing default clock divider/trim value\n");
 		/* The current RTC value probably doesn't make sense either */
-		RCNR = 0;
+		writel_relaxed(0, info->rcnr);
 	}
 
 	device_init_wakeup(&pdev->dev, 1);
@@ -298,7 +331,7 @@ static int sa1100_rtc_probe(struct platform_device *pdev)
 	 *
 	 * Notice that clearing bit 1 and 0 is accomplished by writting ONES to
 	 * the corresponding bits in RTSR. */
-	RTSR = RTSR_AL | RTSR_HZ;
+	writel_relaxed(RTSR_AL | RTSR_HZ, info->rtsr);
 
 	return 0;
 err_dev:
-- 
2.1.0

-- 
-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[PATCH v3 2/4] rtc: sa1100: convert to run-time register mapping

[Rob Herring]

SA1100 and PXA differ only in register offsets which are currently
hardcoded in a machine specific header. Some arm64 platforms (PXA1928)
have this RTC block as well (and not the PXA270 variant).

Convert the driver to use ioremap and set the register offsets dynamically.
Since we are touching all the register accesses, convert them all to
readl_relaxed/writel_relaxed.

Currently, the rtc-sa1100 and rtc-pxa drivers co-exist as rtc-pxa has a
superset of functionality. This commit makes the drivers one step closer
to being mutually exclusive by using devm_ioremap_resource and claiming
the resource. The sharing of overlapping resources does not work if both
drivers claim the resource. That is not done currently, but will be done
as the drivers are converted to DT and follow proper driver rules.
Likely, the common portion of the 2 drivers will be made into library
functions for the SA1100 and PXA drivers to shared.

Signed-off-by: Rob Herring <robh@kernel.org>
Acked-by: Robert Jarzmik <robert.jarzmik@free.fr>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Cc: rtc-linux at googlegroups.com
---
 drivers/rtc/rtc-sa1100.c | 87 +++++++++++++++++++++++++++++++++---------------
 1 file changed, 60 insertions(+), 27 deletions(-)

diff --git a/drivers/rtc/rtc-sa1100.c b/drivers/rtc/rtc-sa1100.c
index b6e1ca0..52d2a8a 100644
--- a/drivers/rtc/rtc-sa1100.c
+++ b/drivers/rtc/rtc-sa1100.c
@@ -35,12 +35,10 @@
 #include <linux/bitops.h>
 #include <linux/io.h>
 
-#include <mach/hardware.h>
-#include <mach/irqs.h>
-
-#if defined(CONFIG_ARCH_PXA) || defined(CONFIG_ARCH_MMP)
-#include <mach/regs-rtc.h>
-#endif
+#define RTSR_HZE		BIT(3)	/* HZ interrupt enable */
+#define RTSR_ALE		BIT(2)	/* RTC alarm interrupt enable */
+#define RTSR_HZ			BIT(1)	/* HZ rising-edge detected */
+#define RTSR_AL			BIT(0)	/* RTC alarm detected */
 
 #define RTC_DEF_DIVIDER		(32768 - 1)
 #define RTC_DEF_TRIM		0
@@ -48,6 +46,10 @@
 
 struct sa1100_rtc {
 	spinlock_t		lock;
+	void __iomem		*rcnr;
+	void __iomem		*rtar;
+	void __iomem		*rtsr;
+	void __iomem		*rttr;
 	int			irq_1hz;
 	int			irq_alarm;
 	struct rtc_device	*rtc;
@@ -63,16 +65,16 @@ static irqreturn_t sa1100_rtc_interrupt(int irq, void *dev_id)
 
 	spin_lock(&info->lock);
 
-	rtsr = RTSR;
+	rtsr = readl_relaxed(info->rtsr);
 	/* clear interrupt sources */
-	RTSR = 0;
+	writel_relaxed(0, info->rtsr);
 	/* Fix for a nasty initialization problem the in SA11xx RTSR register.
 	 * See also the comments in sa1100_rtc_probe(). */
 	if (rtsr & (RTSR_ALE | RTSR_HZE)) {
 		/* This is the original code, before there was the if test
 		 * above. This code does not clear interrupts that were not
 		 * enabled. */
-		RTSR = (RTSR_AL | RTSR_HZ) & (rtsr >> 2);
+		writel_relaxed((RTSR_AL | RTSR_HZ) & (rtsr >> 2), info->rtsr);
 	} else {
 		/* For some reason, it is possible to enter this routine
 		 * without interruptions enabled, it has been tested with
@@ -81,13 +83,13 @@ static irqreturn_t sa1100_rtc_interrupt(int irq, void *dev_id)
 		 * This situation leads to an infinite "loop" of interrupt
 		 * routine calling and as a result the processor seems to
 		 * lock on its first call to open(). */
-		RTSR = RTSR_AL | RTSR_HZ;
+		writel_relaxed(RTSR_AL | RTSR_HZ, info->rtsr);
 	}
 
 	/* clear alarm interrupt if it has occurred */
 	if (rtsr & RTSR_AL)
 		rtsr &= ~RTSR_ALE;
-	RTSR = rtsr & (RTSR_ALE | RTSR_HZE);
+	writel_relaxed(rtsr & (RTSR_ALE | RTSR_HZE), info->rtsr);
 
 	/* update irq data & counter */
 	if (rtsr & RTSR_AL)
@@ -135,7 +137,7 @@ static void sa1100_rtc_release(struct device *dev)
 	struct sa1100_rtc *info = dev_get_drvdata(dev);
 
 	spin_lock_irq(&info->lock);
-	RTSR = 0;
+	writel_relaxed(0, info->rtsr);
 	spin_unlock_irq(&info->lock);
 
 	free_irq(info->irq_alarm, dev);
@@ -144,39 +146,46 @@ static void sa1100_rtc_release(struct device *dev)
 
 static int sa1100_rtc_alarm_irq_enable(struct device *dev, unsigned int enabled)
 {
+	u32 rtsr;
 	struct sa1100_rtc *info = dev_get_drvdata(dev);
 
 	spin_lock_irq(&info->lock);
+	rtsr = readl_relaxed(info->rtsr);
 	if (enabled)
-		RTSR |= RTSR_ALE;
+		rtsr |= RTSR_ALE;
 	else
-		RTSR &= ~RTSR_ALE;
+		rtsr &= ~RTSR_ALE;
+	writel_relaxed(rtsr, info->rtsr);
 	spin_unlock_irq(&info->lock);
 	return 0;
 }
 
 static int sa1100_rtc_read_time(struct device *dev, struct rtc_time *tm)
 {
-	rtc_time_to_tm(RCNR, tm);
+	struct sa1100_rtc *info = dev_get_drvdata(dev);
+
+	rtc_time_to_tm(readl_relaxed(info->rcnr), tm);
 	return 0;
 }
 
 static int sa1100_rtc_set_time(struct device *dev, struct rtc_time *tm)
 {
+	struct sa1100_rtc *info = dev_get_drvdata(dev);
 	unsigned long time;
 	int ret;
 
 	ret = rtc_tm_to_time(tm, &time);
 	if (ret == 0)
-		RCNR = time;
+		writel_relaxed(time, info->rcnr);
 	return ret;
 }
 
 static int sa1100_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 {
 	u32	rtsr;
+	struct sa1100_rtc *info = dev_get_drvdata(dev);
 
-	rtsr = RTSR;
+	rtsr = readl_relaxed(info->rtsr);
 	alrm->enabled = (rtsr & RTSR_ALE) ? 1 : 0;
 	alrm->pending = (rtsr & RTSR_AL) ? 1 : 0;
 	return 0;
@@ -192,12 +201,13 @@ static int sa1100_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 	ret = rtc_tm_to_time(&alrm->time, &time);
 	if (ret != 0)
 		goto out;
-	RTSR = RTSR & (RTSR_HZE|RTSR_ALE|RTSR_AL);
-	RTAR = time;
+	writel_relaxed(readl_relaxed(info->rtsr) &
+		(RTSR_HZE | RTSR_ALE | RTSR_AL), info->rtsr);
+	writel_relaxed(time, info->rtar);
 	if (alrm->enabled)
-		RTSR |= RTSR_ALE;
+		writel_relaxed(readl_relaxed(info->rtsr) | RTSR_ALE, info->rtsr);
 	else
-		RTSR &= ~RTSR_ALE;
+		writel_relaxed(readl_relaxed(info->rtsr) & ~RTSR_ALE, info->rtsr);
 out:
 	spin_unlock_irq(&info->lock);
 
@@ -206,8 +216,10 @@ out:
 
 static int sa1100_rtc_proc(struct device *dev, struct seq_file *seq)
 {
-	seq_printf(seq, "trim/divider\t\t: 0x%08x\n", (u32) RTTR);
-	seq_printf(seq, "RTSR\t\t\t: 0x%08x\n", (u32)RTSR);
+	struct sa1100_rtc *info = dev_get_drvdata(dev);
+
+	seq_printf(seq, "trim/divider\t\t: 0x%08x\n", readl_relaxed(info->rttr));
+	seq_printf(seq, "RTSR\t\t\t: 0x%08x\n", readl_relaxed(info->rtsr));
 
 	return 0;
 }
@@ -227,6 +239,8 @@ static int sa1100_rtc_probe(struct platform_device *pdev)
 {
 	struct rtc_device *rtc;
 	struct sa1100_rtc *info;
+	struct resource *iores;
+	void __iomem *base;
 	int irq_1hz, irq_alarm, ret = 0;
 
 	irq_1hz = platform_get_irq_byname(pdev, "rtc 1Hz");
@@ -244,6 +258,25 @@ static int sa1100_rtc_probe(struct platform_device *pdev)
 	}
 	info->irq_1hz = irq_1hz;
 	info->irq_alarm = irq_alarm;
+
+	iores = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	base = devm_ioremap_resource(&pdev->dev, iores);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	if (IS_ENABLED(CONFIG_ARCH_SA1100) ||
+	    of_device_is_compatible(pdev->dev.of_node, "mrvl,sa1100-rtc")) {
+		info->rcnr = base + 0x04;
+		info->rtsr = base + 0x10;
+		info->rtar = base + 0x00;
+		info->rttr = base + 0x08;
+	} else {
+		info->rcnr = base + 0x0;
+		info->rtsr = base + 0x8;
+		info->rtar = base + 0x4;
+		info->rttr = base + 0xc;
+	}
+
 	spin_lock_init(&info->lock);
 	platform_set_drvdata(pdev, info);
 
@@ -257,12 +290,12 @@ static int sa1100_rtc_probe(struct platform_device *pdev)
 	 * If the clock divider is uninitialized then reset it to the
 	 * default value to get the 1Hz clock.
 	 */
-	if (RTTR == 0) {
-		RTTR = RTC_DEF_DIVIDER + (RTC_DEF_TRIM << 16);
+	if (readl_relaxed(info->rttr) == 0) {
+		writel_relaxed(RTC_DEF_DIVIDER + (RTC_DEF_TRIM << 16), info->rttr);
 		dev_warn(&pdev->dev, "warning: "
 			"initializing default clock divider/trim value\n");
 		/* The current RTC value probably doesn't make sense either */
-		RCNR = 0;
+		writel_relaxed(0, info->rcnr);
 	}
 
 	device_init_wakeup(&pdev->dev, 1);
@@ -298,7 +331,7 @@ static int sa1100_rtc_probe(struct platform_device *pdev)
 	 *
 	 * Notice that clearing bit 1 and 0 is accomplished by writting ONES to
 	 * the corresponding bits in RTSR. */
-	RTSR = RTSR_AL | RTSR_HZ;
+	writel_relaxed(RTSR_AL | RTSR_HZ, info->rtsr);
 
 	return 0;
 err_dev:
-- 
2.1.0

[rtc-linux] [PATCH v3 3/4] ARM: sa1100: remove unused RTC register definitions

[Rob Herring]

Now that register definitions have been moved to the driver, we can remove
them from machine specific code.

Signed-off-by: Rob Herring <robh@kernel.org>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: linux-arm-kernel@lists.infradead.org
---
 arch/arm/mach-sa1100/include/mach/SA-1100.h | 34 -----------------------------
 1 file changed, 34 deletions(-)

diff --git a/arch/arm/mach-sa1100/include/mach/SA-1100.h b/arch/arm/mach-sa1100/include/mach/SA-1100.h
index 0ac6cc0..7972617 100644
--- a/arch/arm/mach-sa1100/include/mach/SA-1100.h
+++ b/arch/arm/mach-sa1100/include/mach/SA-1100.h
@@ -858,40 +858,6 @@
 
 
 /*
- * Real-Time Clock (RTC) control registers
- *
- * Registers
- *    RTAR      	Real-Time Clock (RTC) Alarm Register (read/write).
- *    RCNR      	Real-Time Clock (RTC) CouNt Register (read/write).
- *    RTTR      	Real-Time Clock (RTC) Trim Register (read/write).
- *    RTSR      	Real-Time Clock (RTC) Status Register (read/write).
- *
- * Clocks
- *    frtx, Trtx	Frequency, period of the real-time clock crystal
- *              	(32.768 kHz nominal).
- *    frtc, Trtc	Frequency, period of the real-time clock counter
- *              	(1 Hz nominal).
- */
-
-#define RTAR		__REG(0x90010000)  /* RTC Alarm Reg. */
-#define RCNR		__REG(0x90010004)  /* RTC CouNt Reg. */
-#define RTTR		__REG(0x90010008)  /* RTC Trim Reg. */
-#define RTSR		__REG(0x90010010)  /* RTC Status Reg. */
-
-#define RTTR_C  	Fld (16, 0)	/* clock divider Count - 1         */
-#define RTTR_D  	Fld (10, 16)	/* trim Delete count               */
-                	        	/* frtc = (1023*(C + 1) - D)*frtx/ */
-                	        	/*        (1023*(C + 1)^2)         */
-                	        	/* Trtc = (1023*(C + 1)^2)*Trtx/   */
-                	        	/*        (1023*(C + 1) - D)       */
-
-#define RTSR_AL 	0x00000001	/* ALarm detected                  */
-#define RTSR_HZ 	0x00000002	/* 1 Hz clock detected             */
-#define RTSR_ALE	0x00000004	/* ALarm interrupt Enable          */
-#define RTSR_HZE	0x00000008	/* 1 Hz clock interrupt Enable     */
-
-
-/*
  * Power Manager (PM) control registers
  *
  * Registers
-- 
2.1.0

-- 
-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[PATCH v3 3/4] ARM: sa1100: remove unused RTC register definitions

[Rob Herring]

Now that register definitions have been moved to the driver, we can remove
them from machine specific code.

Signed-off-by: Rob Herring <robh@kernel.org>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: linux-arm-kernel at lists.infradead.org
---
 arch/arm/mach-sa1100/include/mach/SA-1100.h | 34 -----------------------------
 1 file changed, 34 deletions(-)

diff --git a/arch/arm/mach-sa1100/include/mach/SA-1100.h b/arch/arm/mach-sa1100/include/mach/SA-1100.h
index 0ac6cc0..7972617 100644
--- a/arch/arm/mach-sa1100/include/mach/SA-1100.h
+++ b/arch/arm/mach-sa1100/include/mach/SA-1100.h
@@ -858,40 +858,6 @@
 
 
 /*
- * Real-Time Clock (RTC) control registers
- *
- * Registers
- *    RTAR      	Real-Time Clock (RTC) Alarm Register (read/write).
- *    RCNR      	Real-Time Clock (RTC) CouNt Register (read/write).
- *    RTTR      	Real-Time Clock (RTC) Trim Register (read/write).
- *    RTSR      	Real-Time Clock (RTC) Status Register (read/write).
- *
- * Clocks
- *    frtx, Trtx	Frequency, period of the real-time clock crystal
- *              	(32.768 kHz nominal).
- *    frtc, Trtc	Frequency, period of the real-time clock counter
- *              	(1 Hz nominal).
- */
-
-#define RTAR		__REG(0x90010000)  /* RTC Alarm Reg. */
-#define RCNR		__REG(0x90010004)  /* RTC CouNt Reg. */
-#define RTTR		__REG(0x90010008)  /* RTC Trim Reg. */
-#define RTSR		__REG(0x90010010)  /* RTC Status Reg. */
-
-#define RTTR_C  	Fld (16, 0)	/* clock divider Count - 1         */
-#define RTTR_D  	Fld (10, 16)	/* trim Delete count               */
-                	        	/* frtc = (1023*(C + 1) - D)*frtx/ */
-                	        	/*        (1023*(C + 1)^2)         */
-                	        	/* Trtc = (1023*(C + 1)^2)*Trtx/   */
-                	        	/*        (1023*(C + 1) - D)       */
-
-#define RTSR_AL 	0x00000001	/* ALarm detected                  */
-#define RTSR_HZ 	0x00000002	/* 1 Hz clock detected             */
-#define RTSR_ALE	0x00000004	/* ALarm interrupt Enable          */
-#define RTSR_HZE	0x00000008	/* 1 Hz clock interrupt Enable     */
-
-
-/*
  * Power Manager (PM) control registers
  *
  * Registers
-- 
2.1.0

[rtc-linux] [PATCH v3 4/4] ARM: mmp: remove unused RTC register definitions

[Rob Herring]

Now that register definitions have been moved to the driver, regs-rtc.h is
no longer used and can be removed.

Signed-off-by: Rob Herring <robh@kernel.org>
Cc: Eric Miao <eric.y.miao@gmail.com>
Cc: Haojian Zhuang <haojian.zhuang@gmail.com>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: linux-arm-kernel@lists.infradead.org
---
 arch/arm/mach-mmp/include/mach/regs-rtc.h | 23 -----------------------
 1 file changed, 23 deletions(-)
 delete mode 100644 arch/arm/mach-mmp/include/mach/regs-rtc.h

diff --git a/arch/arm/mach-mmp/include/mach/regs-rtc.h b/arch/arm/mach-mmp/include/mach/regs-rtc.h
deleted file mode 100644
index 5bff886..0000000
--- a/arch/arm/mach-mmp/include/mach/regs-rtc.h
+++ /dev/null
@@ -1,23 +0,0 @@
-#ifndef __ASM_MACH_REGS_RTC_H
-#define __ASM_MACH_REGS_RTC_H
-
-#include <mach/addr-map.h>
-
-#define RTC_VIRT_BASE	(APB_VIRT_BASE + 0x10000)
-#define RTC_REG(x)	(*((volatile u32 __iomem *)(RTC_VIRT_BASE + (x))))
-
-/*
- * Real Time Clock
- */
-
-#define RCNR		RTC_REG(0x00)	/* RTC Count Register */
-#define RTAR		RTC_REG(0x04)	/* RTC Alarm Register */
-#define RTSR		RTC_REG(0x08)	/* RTC Status Register */
-#define RTTR		RTC_REG(0x0C)	/* RTC Timer Trim Register */
-
-#define RTSR_HZE	(1 << 3)	/* HZ interrupt enable */
-#define RTSR_ALE	(1 << 2)	/* RTC alarm interrupt enable */
-#define RTSR_HZ		(1 << 1)	/* HZ rising-edge detected */
-#define RTSR_AL		(1 << 0)	/* RTC alarm detected */
-
-#endif /* __ASM_MACH_REGS_RTC_H */
-- 
2.1.0

-- 
-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[PATCH v3 4/4] ARM: mmp: remove unused RTC register definitions

[Rob Herring]

Now that register definitions have been moved to the driver, regs-rtc.h is
no longer used and can be removed.

Signed-off-by: Rob Herring <robh@kernel.org>
Cc: Eric Miao <eric.y.miao@gmail.com>
Cc: Haojian Zhuang <haojian.zhuang@gmail.com>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: linux-arm-kernel at lists.infradead.org
---
 arch/arm/mach-mmp/include/mach/regs-rtc.h | 23 -----------------------
 1 file changed, 23 deletions(-)
 delete mode 100644 arch/arm/mach-mmp/include/mach/regs-rtc.h

diff --git a/arch/arm/mach-mmp/include/mach/regs-rtc.h b/arch/arm/mach-mmp/include/mach/regs-rtc.h
deleted file mode 100644
index 5bff886..0000000
--- a/arch/arm/mach-mmp/include/mach/regs-rtc.h
+++ /dev/null
@@ -1,23 +0,0 @@
-#ifndef __ASM_MACH_REGS_RTC_H
-#define __ASM_MACH_REGS_RTC_H
-
-#include <mach/addr-map.h>
-
-#define RTC_VIRT_BASE	(APB_VIRT_BASE + 0x10000)
-#define RTC_REG(x)	(*((volatile u32 __iomem *)(RTC_VIRT_BASE + (x))))
-
-/*
- * Real Time Clock
- */
-
-#define RCNR		RTC_REG(0x00)	/* RTC Count Register */
-#define RTAR		RTC_REG(0x04)	/* RTC Alarm Register */
-#define RTSR		RTC_REG(0x08)	/* RTC Status Register */
-#define RTTR		RTC_REG(0x0C)	/* RTC Timer Trim Register */
-
-#define RTSR_HZE	(1 << 3)	/* HZ interrupt enable */
-#define RTSR_ALE	(1 << 2)	/* RTC alarm interrupt enable */
-#define RTSR_HZ		(1 << 1)	/* HZ rising-edge detected */
-#define RTSR_AL		(1 << 0)	/* RTC alarm detected */
-
-#endif /* __ASM_MACH_REGS_RTC_H */
-- 
2.1.0