[U-Boot] [PATCH-ARM] Add support for Embest SBC2440-II Board 3/7

[Detlev Zundel]

Hi Kevin,

> This patch re-formats the s3c24x0 driver code, excluding the MTD NAND 
> driver which is in patch 4, in preparation for changes to make the 
> NAND driver support both s3c2410 and s3c2440 CPU's, ready for the 
> addition of the Embest SBC2440-II Board.
>
> The changes are as follows:
>
> - re-indent the code using Lindent

If it is at all possible, then please split indentation changes into a
seprate commit.  Not doing this makes such a big patch especially hard
to review as "real" changes can hide behind indentation changes.

Also we do not believe that using Lindent fully automatic is a good
idea.  If you look at the review below, sometimes nice indentation is
lost this way.

> - mak sure register layouts are defined using a C struct, from a 
>   comment by Wolfgang on 03/06/2009
> - replace the upper-case typedef'ed C struct names with lower case 
>   non-typedef'ed ones, from a comment by Scott on 22/06/2009

I don't see that we get rid of the typedefs though - are they still
used elsewhere?

> - make sure registers are accessed using the proper accessor 
>   functions, from a comment by Wolfgang on 03/06/2009
> - run checkpatch.pl and fix any error reports
>
> Signed-off-by: Kevin Morfitt <kevin.morfitt at fearnside-systems.co.uk>

Apart from that changes look good.  I especially looked at the
trab/smdk2400 changes.

> ---
>  board/mpl/vcma9/vcma9.c           |    7 +-
>  board/mpl/vcma9/vcma9.h           |   18 ++--
>  board/samsung/smdk2400/smdk2400.c |    5 +-
>  board/samsung/smdk2410/smdk2410.c |    5 +-
>  board/sbc2410x/sbc2410x.c         |    7 +-
>  board/trab/cmd_trab.c             |    8 +-
>  board/trab/rs485.c                |   12 +-
>  board/trab/trab.c                 |   17 ++-
>  board/trab/trab_fkt.c             |   22 ++--
>  board/trab/tsc2000.c              |   17 ++-
>  board/trab/tsc2000.h              |    4 +-
>  board/trab/vfd.c                  |   12 +-
>  drivers/i2c/s3c24x0_i2c.c         |  273 +++++++++++++++++++------------------
>  drivers/rtc/s3c24x0_rtc.c         |  134 +++++++++---------
>  drivers/serial/serial_s3c24x0.c   |  160 ++++++++++++----------
>  15 files changed, 368 insertions(+), 333 deletions(-)
>

[...]

> -int rtc_get (struct rtc_time *tmp)
> +int rtc_get(struct rtc_time *tmp)
>  {
> -	S3C24X0_RTC * const rtc = S3C24X0_GetBase_RTC();
> +	struct s3c24x0_rtc *rtc = S3C24X0_GetBase_RTC();
>  	uchar sec, min, hour, mday, wday, mon, year;
> -	uchar a_sec,a_min, a_hour, a_date, a_mon, a_year, a_armed;
> +	uchar a_sec, a_min, a_hour, a_date, a_mon, a_year, a_armed;
>  
>  	/* enable access to RTC registers */
>  	SetRTC_Access(RTC_ENABLE);
>  
>  	/* read RTC registers */
>  	do {
> -		sec	= rtc->BCDSEC;
> -		min	= rtc->BCDMIN;
> -		hour	= rtc->BCDHOUR;
> -		mday	= rtc->BCDDATE;
> -		wday	= rtc->BCDDAY;
> -		mon	= rtc->BCDMON;
> -		year	= rtc->BCDYEAR;
> -	} while (sec != rtc->BCDSEC);
> +		sec = readb(&rtc->BCDSEC);
> +		min = readb(&rtc->BCDMIN);
> +		hour = readb(&rtc->BCDHOUR);
> +		mday = readb(&rtc->BCDDATE);
> +		wday = readb(&rtc->BCDDAY);
> +		mon = readb(&rtc->BCDMON);
> +		year = readb(&rtc->BCDYEAR);
> +	} while (sec != readb(&rtc->BCDSEC));
>  
>  	/* read ALARM registers */
> -	a_sec	= rtc->ALMSEC;
> -	a_min	= rtc->ALMMIN;
> -	a_hour	= rtc->ALMHOUR;
> -	a_date	= rtc->ALMDATE;
> -	a_mon	= rtc->ALMMON;
> -	a_year	= rtc->ALMYEAR;
> -	a_armed	= rtc->RTCALM;
> +	a_sec = readb(&rtc->ALMSEC);
> +	a_min = readb(&rtc->ALMMIN);
> +	a_hour = readb(&rtc->ALMHOUR);
> +	a_date = readb(&rtc->ALMDATE);
> +	a_mon = readb(&rtc->ALMMON);
> +	a_year = readb(&rtc->ALMYEAR);
> +	a_armed = readb(&rtc->RTCALM);

Nice indentations which make the code actually easier to read are lost.


[...]

> -	tmp->tm_sec  = bcd2bin(sec  & 0x7F);
> -	tmp->tm_min  = bcd2bin(min  & 0x7F);
> +	tmp->tm_sec = bcd2bin(sec & 0x7F);
> +	tmp->tm_min = bcd2bin(min & 0x7F);
>  	tmp->tm_hour = bcd2bin(hour & 0x3F);
>  	tmp->tm_mday = bcd2bin(mday & 0x3F);
> -	tmp->tm_mon  = bcd2bin(mon & 0x1F);
> +	tmp->tm_mon = bcd2bin(mon & 0x1F);
>  	tmp->tm_year = bcd2bin(year);
>  	tmp->tm_wday = bcd2bin(wday & 0x07);

Ditto.


[...]

> -int rtc_set (struct rtc_time *tmp)
> +int rtc_set(struct rtc_time *tmp)
>  {
> -	S3C24X0_RTC * const rtc = S3C24X0_GetBase_RTC();
> +	struct s3c24x0_rtc *rtc = S3C24X0_GetBase_RTC();
>  	uchar sec, min, hour, mday, wday, mon, year;
>  
>  #ifdef RTC_DEBUG
> -	printf ( "Set DATE: %4d-%02d-%02d (wday=%d)  TIME: %2d:%02d:%02d\n",
> -		tmp->tm_year, tmp->tm_mon, tmp->tm_mday, tmp->tm_wday,
> -		tmp->tm_hour, tmp->tm_min, tmp->tm_sec);
> +	printf("Set DATE: %4d-%02d-%02d (wday=%d)  TIME: %2d:%02d:%02d\n",
> +	       tmp->tm_year, tmp->tm_mon, tmp->tm_mday, tmp->tm_wday,
> +	       tmp->tm_hour, tmp->tm_min, tmp->tm_sec);
>  #endif
> -	year	= bin2bcd(tmp->tm_year % 100);
> -	mon	= bin2bcd(tmp->tm_mon);
> -	wday	= bin2bcd(tmp->tm_wday);
> -	mday	= bin2bcd(tmp->tm_mday);
> -	hour	= bin2bcd(tmp->tm_hour);
> -	min	= bin2bcd(tmp->tm_min);
> -	sec	= bin2bcd(tmp->tm_sec);
> +	year = bin2bcd(tmp->tm_year % 100);
> +	mon = bin2bcd(tmp->tm_mon);
> +	wday = bin2bcd(tmp->tm_wday);
> +	mday = bin2bcd(tmp->tm_mday);
> +	hour = bin2bcd(tmp->tm_hour);
> +	min = bin2bcd(tmp->tm_min);
> +	sec = bin2bcd(tmp->tm_sec);

Ditto.

>  
>  	/* enable access to RTC registers */
>  	SetRTC_Access(RTC_ENABLE);
>  
>  	/* write RTC registers */
> -	rtc->BCDSEC	= sec;
> -	rtc->BCDMIN	= min;
> -	rtc->BCDHOUR	= hour;
> -	rtc->BCDDATE	= mday;
> -	rtc->BCDDAY	= wday;
> -	rtc->BCDMON	= mon;
> -	rtc->BCDYEAR	= year;
> +	writeb(sec, &rtc->BCDSEC);
> +	writeb(min, &rtc->BCDMIN);
> +	writeb(hour, &rtc->BCDHOUR);
> +	writeb(mday, &rtc->BCDDATE);
> +	writeb(wday, &rtc->BCDDAY);
> +	writeb(mon, &rtc->BCDMON);
> +	writeb(year, &rtc->BCDYEAR);

And again.


[...]

> @@ -289,12 +312,11 @@ serial_puts (const char *s)
>  #if defined(CONFIG_SERIAL_MULTI)
>  DECLARE_S3C_SERIAL_FUNCTIONS(0);
>  struct serial_device s3c24xx_serial0_device =
> -	INIT_S3C_SERIAL_STRUCTURE(0, "s3ser0", "S3UART1");
> +INIT_S3C_SERIAL_STRUCTURE(0, "s3ser0", "S3UART1");
>  DECLARE_S3C_SERIAL_FUNCTIONS(1);
>  struct serial_device s3c24xx_serial1_device =
> -	INIT_S3C_SERIAL_STRUCTURE(1, "s3ser1", "S3UART2");
> +INIT_S3C_SERIAL_STRUCTURE(1, "s3ser1", "S3UART2");
>  DECLARE_S3C_SERIAL_FUNCTIONS(2);
>  struct serial_device s3c24xx_serial2_device =
> -	INIT_S3C_SERIAL_STRUCTURE(2, "s3ser2", "S3UART3");
> -
> +INIT_S3C_SERIAL_STRUCTURE(2, "s3ser2", "S3UART3");
>  #endif /* CONFIG_SERIAL_MULTI */

Why is there no indentation here?  The previous version looked better.

Cheers
  Detlev

-- 
When critics disagree the artist is in accord with himself.
                                    --- Oscar Wilde
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de