[U-Boot] RFC [PATCH 1/5] drivers/rtc: add Marvell Integrated RTC.

[Prafulla Wadaskar]

> -----Original Message-----
> From: Jason [mailto:twuug at lakedaemon.net]
> Sent: Thursday, July 28, 2011 6:52 AM
> To: Prafulla Wadaskar
> Cc: clint at debian.org; wd at denx.de; u-boot at lists.denx.de; Prabhanjan
> Sarnaik; Ashish Karkare; Siddarth Gore; bdale at gag.com
> Subject: Re: RFC [PATCH 1/5] drivers/rtc: add Marvell Integrated RTC.
> 
> On Wed, Jul 27, 2011 at 11:12:35AM -0700, Prafulla Wadaskar wrote:
> >
> >
> > > -----Original Message-----
> > > From: Jason Cooper [mailto:u-boot at lakedaemon.net]
> > > Sent: Wednesday, July 27, 2011 2:49 AM
> > > To: clint at debian.org; wd at denx.de; Prafulla Wadaskar
> > > Cc: u-boot at lists.denx.de; Prabhanjan Sarnaik; Ashish Karkare;
> Siddarth
> > > Gore; bdale at gag.com; Jason Cooper
> > > Subject: RFC [PATCH 1/5] drivers/rtc: add Marvell Integrated RTC.
> > >
> > >
> > > Signed-off-by: Jason Cooper <u-boot at lakedaemon.net>
> > > ---
> > >  drivers/rtc/Makefile      |    1 +
> > >  drivers/rtc/mvinteg_rtc.c |  151
> >
> > Filename mvrtc.c sounds more relevent
> 
> Will do.
> 
> ...
> > > +#define INTERNAL_REG_BASE_ADDR 0xf1000000
> >
> > This must go in kirkood.h
> 
> Thanks.  That didn't feel right to me, but I didn't know the proper
> place to put it.
> 
> ...
> > > +
> > > +/* register operations macros */
> > > +#define MV_REG_READ(offset) \
> > > +	le32_to_cpu( \
> > > +	*(volatile unsigned int *)(INTERNAL_REG_BASE_ADDR + offset))
> > > +
> > > +#define MV_REG_WRITE(offset, data) \
> > > +	do { \
> > > +		*(volatile unsigned int *)(INTERNAL_REG_BASE_ADDR + offset)
> = \
> > > +		cpu_to_le32(data); \
> > > +	} while (0);
> > > +
> > > +/* RTC registers */
> > > +#define MVINTEG_RTCTIME_REG 0x10300
> > > +#define MVINTEG_RTCDATE_REG 0x10304
> >
> > NAK, please use c-strut for register definations
> 
> So then, in kirkwood.h:
> 
> #define KW_RTC_BASE (KW_REGISTER(0x10300))
> 
> and then use accordingly:
> 
> struct mvrtc_registers {
> 	u32 time;
> 	u32 date;
> };
> 
> struct mvrtc_registers *mvrtc_regs = (struct mvrtc_registers
> *)KW_RTC_BASE;
> 
> Right?  I'm basing this on kirkwood/timer.c

That's perfect :-)

> 
> ...
> > Regards..
> > Prafulla . .
> 
> As always, thanks for the review and comments.

That's my pleasure too.

Regards..
Prafulla . .