[RFC PATCH 1/2] ARM: CSR: add rtc i/o bridge interface for SiRFprimaII

[Barry Song]

2011/8/5 Jamie Iles <jamie at jamieiles.com>:
> On Fri, Aug 05, 2011 at 07:28:05PM +0800, Barry Song wrote:
>> Hi Jamie,
>> Thanks.
>>
>> 2011/8/5 Jamie Iles <jamie at jamieiles.com>:
>> > Hi Barry, Zhiwu,
>> >
>> > A couple of minor comments inline.
>> >
>> > Jamie
>> >
>> > On Thu, Aug 04, 2011 at 07:54:47PM -0700, Barry Song wrote:
>> >> From: Zhiwu Song <zhiwu.song at csr.com>
>> >>
>> >> The module is a bridge between the RTC clock domain and the CPU interface
>> >> clock domain. ARM access the register of SYSRTC, GPSRTC and PWRC through
>> >> this module.
>> >>
>> >> Signed-off-by: Zhiwu Song <zhiwu.song at csr.com>
>> >> Signed-off-by: Barry Song <baohua.song at csr.com>
>> >> ---
>> >>  arch/arm/mach-prima2/Makefile        |    1 +
>> >>  arch/arm/mach-prima2/rtciobrg.c      |  118 ++++++++++++++++++++++++++++++++++
>> >>  include/linux/rtc/sirfsoc_rtciobrg.h |   18 +++++
>> >>  3 files changed, 137 insertions(+), 0 deletions(-)
>> >>  create mode 100644 arch/arm/mach-prima2/rtciobrg.c
>> >>  create mode 100644 include/linux/rtc/sirfsoc_rtciobrg.h
>> >>
>> >> diff --git a/arch/arm/mach-prima2/Makefile b/arch/arm/mach-prima2/Makefile
>> >> index 7af7fc0..f49d70b 100644
>> >> --- a/arch/arm/mach-prima2/Makefile
>> >> +++ b/arch/arm/mach-prima2/Makefile
>> >> @@ -3,5 +3,6 @@ obj-y += irq.o
>> >>  obj-y += clock.o
>> >>  obj-y += rstc.o
>> >>  obj-y += prima2.o
>> >> +obj-y += rtciobrg.o
>> >>  obj-$(CONFIG_DEBUG_LL) += lluart.o
>> >>  obj-$(CONFIG_CACHE_L2X0) += l2x0.o
>> >> diff --git a/arch/arm/mach-prima2/rtciobrg.c b/arch/arm/mach-prima2/rtciobrg.c
>> >> new file mode 100644
>> >> index 0000000..0dc29f8
>> >> --- /dev/null
>> >> +++ b/arch/arm/mach-prima2/rtciobrg.c
>> >> @@ -0,0 +1,118 @@
>> >> +/*
>> >> + * RTC I/O Bridge interfaces for CSR SiRFprimaII
>> >> + * ARM access the registers of SYSRTC, GPSRTC and PWRC through this module
>> >> + *
>> >> + * Copyright (c) 2011 Cambridge Silicon Radio Limited, a CSR plc group company.
>> >> + *
>> >> + * Licensed under GPLv2 or later.
>> >> + */
>> >> +
>> >> +#include <linux/kernel.h>
>> >> +#include <linux/module.h>
>> >> +#include <linux/io.h>
>> >> +#include <linux/of.h>
>> >> +#include <linux/of_address.h>
>> >> +
>> >> +#define SIRFSOC_CPUIOBRG_CTRL           0x00
>> >> +#define SIRFSOC_CPUIOBRG_WRBE           0x04
>> >> +#define SIRFSOC_CPUIOBRG_ADDR           0x08
>> >> +#define SIRFSOC_CPUIOBRG_DATA           0x0c
>> >> +
>> >> +void __iomem *sirfsoc_rtciobrg_base;
>> >
>> > static void __iomem?
>>
>> this var will be accessed by sleep.S in patch 2/2.
>
> OK, but then shouldn't these be in a header file somewhere?  I think
> sparse will warn about this otherwise saying it could be declared as
> static.

yes.these are APIs and sparse will warn if we don't place them in head
file. the problem is that sleep.S is the only user of those functions.
the asm does't need and can't refer the c function declare.

we have other 3 functions in head file since they will be referred by
rtc drivers.

rtciobrg connect PM, rtc and gps rtc. only sleep.S of PM refer these
low level functions not in head file. it is really a special case.


>
> [...]
>> >> +u32 __sirfsoc_rtc_iobrg_readl(u32 addr)
>> >> +{
>> >
>> > Maybe for clarity have offset rather than address for these accessors to
>> > indicate the nature of their use?
>>
>> it should be an address not an offset. the address is local address
>> againest rtciobrg. it is not an address in memory bus againest CPU
>> core.
>> if it is an offset, than its memory address is:
>> MEMORY_BASE + addr.
>>
>> but here its address is addr in local bus.
>
> OK, I guess it's a terminology thing, but I don't particularly mind
> either way.
>
> Jamie
>