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.morf...@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: d...@denx.de
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
