[U-Boot] [PATCH] bug fix and cleanup for s3c44b0 serial driver
Specifically, don't dereference the URXH0 register twice; calculate the BRD based on the formula in the databook instead of using a messy switch statement; and migrate the BRD calculation to the hardware hardware header because it's board specific.
Signed-off-by: Brian Cavagnolo brian@cozybit.com --- arch/arm/include/asm/arch-s3c44b0/hardware.h | 3 +- drivers/serial/serial_s3c44b0.c | 65 +------------------------- 2 files changed, 4 insertions(+), 64 deletions(-)
diff --git a/arch/arm/include/asm/arch-s3c44b0/hardware.h b/arch/arm/include/asm/arch-s3c44b0/hardware.h index 146e265..38ff32c 100644 --- a/arch/arm/include/asm/arch-s3c44b0/hardware.h +++ b/arch/arm/include/asm/arch-s3c44b0/hardware.h @@ -11,7 +11,8 @@ #define REGL(addr) (*(volatile unsigned int *)(REGBASE+addr)) #define REGW(addr) (*(volatile unsigned short *)(REGBASE+addr)) #define REGB(addr) (*(volatile unsigned char *)(REGBASE+addr)) - +#define BRD(bps) (DIV_ROUND(CONFIG_S3C44B0_CLOCK_SPEED * 1000000, \ + (bps)*16) - 1)
/*****************************/ /* CPU Wrapper Registers */ diff --git a/drivers/serial/serial_s3c44b0.c b/drivers/serial/serial_s3c44b0.c index 95d0266..e6c535c 100644 --- a/drivers/serial/serial_s3c44b0.c +++ b/drivers/serial/serial_s3c44b0.c @@ -48,7 +48,7 @@ static int serial_flush_input(void)
/* keep on reading as long as the receiver is not empty */ while(UTRSTAT0&0x01) { - tmp = REGB(URXH0); + tmp = URXH0; }
return 0; @@ -70,68 +70,7 @@ static int serial_flush_output(void)
void serial_setbrg (void) { - u32 divisor = 0; - - /* get correct divisor */ - switch(gd->baudrate) { - - case 1200: -#if CONFIG_S3C44B0_CLOCK_SPEED==66 - divisor = 3124; -#elif CONFIG_S3C44B0_CLOCK_SPEED==75 - divisor = 3905; -#else -# error CONFIG_S3C44B0_CLOCK_SPEED undefined -#endif - break; - - case 9600: -#if CONFIG_S3C44B0_CLOCK_SPEED==66 - divisor = 390; -#elif CONFIG_S3C44B0_CLOCK_SPEED==75 - divisor = 487; -#else -# error CONFIG_S3C44B0_CLOCK_SPEED undefined -#endif - break; - - case 19200: -#if CONFIG_S3C44B0_CLOCK_SPEED==66 - divisor = 194; -#elif CONFIG_S3C44B0_CLOCK_SPEED==75 - divisor = 243; -#else -# error CONFIG_S3C44B0_CLOCK_SPEED undefined -#endif - break; - - case 38400: -#if CONFIG_S3C44B0_CLOCK_SPEED==66 - divisor = 97; -#elif CONFIG_S3C44B0_CLOCK_SPEED==75 - divisor = 121; -#else -# error CONFIG_S3C44B0_CLOCK_SPEED undefined -#endif /* break; */ - - case 57600: -#if CONFIG_S3C44B0_CLOCK_SPEED==66 - divisor = 64; -#elif CONFIG_S3C44B0_CLOCK_SPEED==75 - divisor = 80; -#else -# error CONFIG_S3C44B0_CLOCK_SPEED undefined -#endif /* break; */ - - case 115200: -#if CONFIG_S3C44B0_CLOCK_SPEED==66 - divisor = 32; -#elif CONFIG_S3C44B0_CLOCK_SPEED==75 - divisor = 40; -#else -# error CONFIG_S3C44B0_CLOCK_SPEED undefined -#endif /* break; */ - } + u32 divisor = BRD(gd->baudrate);
serial_flush_output(); serial_flush_input();
Dear Minkyu Kang,
In message 1277830727-1794-1-git-send-email-brian@cozybit.com Brian Cavagnolo wrote:
Specifically, don't dereference the URXH0 register twice; calculate the BRD based on the formula in the databook instead of using a messy switch statement; and migrate the BRD calculation to the hardware hardware header because it's board specific.
Signed-off-by: Brian Cavagnolo brian@cozybit.com
arch/arm/include/asm/arch-s3c44b0/hardware.h | 3 +- drivers/serial/serial_s3c44b0.c | 65 +------------------------- 2 files changed, 4 insertions(+), 64 deletions(-)
Is this patch on your queue? See http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/80548
Best regards,
Wolfgang Denk
Dear Brian Cavagnolo,
On 30 June 2010 01:58, Brian Cavagnolo brian@cozybit.com wrote:
Specifically, don't dereference the URXH0 register twice; calculate the BRD based on the formula in the databook instead of using a messy switch statement; and migrate the BRD calculation to the hardware hardware header because it's
hardware hardware..? is it typo?
board specific.
Signed-off-by: Brian Cavagnolo brian@cozybit.com
arch/arm/include/asm/arch-s3c44b0/hardware.h | 3 +- drivers/serial/serial_s3c44b0.c | 65 +------------------------- 2 files changed, 4 insertions(+), 64 deletions(-)
diff --git a/arch/arm/include/asm/arch-s3c44b0/hardware.h b/arch/arm/include/asm/arch-s3c44b0/hardware.h index 146e265..38ff32c 100644 --- a/arch/arm/include/asm/arch-s3c44b0/hardware.h +++ b/arch/arm/include/asm/arch-s3c44b0/hardware.h @@ -11,7 +11,8 @@ #define REGL(addr) (*(volatile unsigned int *)(REGBASE+addr)) #define REGW(addr) (*(volatile unsigned short *)(REGBASE+addr)) #define REGB(addr) (*(volatile unsigned char *)(REGBASE+addr))
+#define BRD(bps) (DIV_ROUND(CONFIG_S3C44B0_CLOCK_SPEED * 1000000, \
- (bps)*16) - 1)
no need brace and please add the space around the operator. bps * 16) - 1)
/*****************************/ /* CPU Wrapper Registers */ diff --git a/drivers/serial/serial_s3c44b0.c b/drivers/serial/serial_s3c44b0.c index 95d0266..e6c535c 100644 --- a/drivers/serial/serial_s3c44b0.c +++ b/drivers/serial/serial_s3c44b0.c @@ -48,7 +48,7 @@ static int serial_flush_input(void)
/* keep on reading as long as the receiver is not empty */ while(UTRSTAT0&0x01) {
Space required before the open parenthesis '('. Could you please run checkpatch and make clean-up patch for this file?
- tmp = REGB(URXH0);
- tmp = URXH0;
}
return 0; @@ -70,68 +70,7 @@ static int serial_flush_output(void)
void serial_setbrg (void) {
- u32 divisor = 0;
- /* get correct divisor */
- switch(gd->baudrate) {
- case 1200:
-#if CONFIG_S3C44B0_CLOCK_SPEED==66
- divisor = 3124;
-#elif CONFIG_S3C44B0_CLOCK_SPEED==75
- divisor = 3905;
-#else -# error CONFIG_S3C44B0_CLOCK_SPEED undefined -#endif
- break;
- case 9600:
-#if CONFIG_S3C44B0_CLOCK_SPEED==66
- divisor = 390;
-#elif CONFIG_S3C44B0_CLOCK_SPEED==75
- divisor = 487;
-#else -# error CONFIG_S3C44B0_CLOCK_SPEED undefined -#endif
- break;
- case 19200:
-#if CONFIG_S3C44B0_CLOCK_SPEED==66
- divisor = 194;
-#elif CONFIG_S3C44B0_CLOCK_SPEED==75
- divisor = 243;
-#else -# error CONFIG_S3C44B0_CLOCK_SPEED undefined -#endif
- break;
- case 38400:
-#if CONFIG_S3C44B0_CLOCK_SPEED==66
- divisor = 97;
-#elif CONFIG_S3C44B0_CLOCK_SPEED==75
- divisor = 121;
-#else -# error CONFIG_S3C44B0_CLOCK_SPEED undefined -#endif /* break; */
- case 57600:
-#if CONFIG_S3C44B0_CLOCK_SPEED==66
- divisor = 64;
-#elif CONFIG_S3C44B0_CLOCK_SPEED==75
- divisor = 80;
-#else -# error CONFIG_S3C44B0_CLOCK_SPEED undefined -#endif /* break; */
- case 115200:
-#if CONFIG_S3C44B0_CLOCK_SPEED==66
- divisor = 32;
-#elif CONFIG_S3C44B0_CLOCK_SPEED==75
- divisor = 40;
-#else -# error CONFIG_S3C44B0_CLOCK_SPEED undefined -#endif /* break; */
- }
- u32 divisor = BRD(gd->baudrate);
serial_flush_output(); serial_flush_input(); -- 1.6.0.4
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Thanks Minkyu Kang.
Dear Minkyu Kang,
my five cents:
#define REGL(addr) (*(volatile unsigned int *)(REGBASE+addr)) #define REGW(addr) (*(volatile unsigned short *)(REGBASE+addr)) #define REGB(addr) (*(volatile unsigned char *)(REGBASE+addr))
isn't that way of accessing hardware VERY depreciated?
+#define BRD(bps) (DIV_ROUND(CONFIG_S3C44B0_CLOCK_SPEED * 1000000, \
(bps)*16) - 1)no need brace and please add the space around the operator. bps * 16) - 1)
its always wise to () a macro parameter and not make assumptions on the parameters. What about BRD(9600+9600) ?
/*****************************/ /* CPU Wrapper Registers */
I think that version of multi-line comment is not allowed...
Best Regards, Reinhard
Dear Reinhard Meyer,
On 10 August 2010 15:10, Reinhard Meyer u-boot@emk-elektronik.de wrote:
Dear Minkyu Kang,
my five cents:
#define REGL(addr) (*(volatile unsigned int *)(REGBASE+addr)) #define REGW(addr) (*(volatile unsigned short *)(REGBASE+addr)) #define REGB(addr) (*(volatile unsigned char *)(REGBASE+addr))
isn't that way of accessing hardware VERY depreciated?
Agreed, we need update for s3c44b0. But, this patch is not for it. I think, It would be another work.
+#define BRD(bps) (DIV_ROUND(CONFIG_S3C44B0_CLOCK_SPEED * 1000000, \
- (bps)*16) - 1)
no need brace and please add the space around the operator. bps * 16) - 1)
its always wise to () a macro parameter and not make assumptions on the parameters. What about BRD(9600+9600) ?
Yes, you're right! Thanks (:
/*****************************/ /* CPU Wrapper Registers */
I think that version of multi-line comment is not allowed...
Yes, so I requested clean-up patch for this file.
Best Regards, Reinhard
Thanks Minkyu Kang.
participants (4)
-
Brian Cavagnolo -
Minkyu Kang -
Reinhard Meyer -
Wolfgang Denk