Skip to contentSkip to conversation

bug fix and cleanup for s3c44b0 serial driver

PATCH New Brian Cavagnolo 4 participants 5 messages Jun 29, 2010 – Aug 10, 2010 1 message not in the archive Arm (32-bit)

lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): s3c44b0-serial-driver

Brian Cavagnolo

author
#1 lore raw

PATCH 1/1 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 <[email protected]>
---
 arch/arm/include/asm/arch-s3c44b0/hardware.h |    3 +-
 drivers/serial/serial_s3c44b0.c              |   65 +-------------------------
 2 files changed, 4 insertions(+), 64 deletions(-)
arch/arm/include/asm/arch-s3c44b0/hardware.h +2 -1
index 146e265..38ff32c 100644
@@ -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     */
drivers/serial/serial_s3c44b0.c +2 -63

77 lines not on this page · raw message

Signature · 1 line
1.6.0.4
#2 lore raw

PATCH 1/1 bug fix and cleanup for s3c44b0 serial driver

Dear Minkyu Kang,

Quoted 10 lines · In message <[email protected]> Brian…

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 <[email protected]>
---
 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

Signature · 6 lines
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"We have the right to survive!"
"Not be killing others."
	-- Deela and Kirk, "Wink of An Eye", stardate 5710.5
#3 lore raw

PATCH 1/1 bug fix and cleanup for s3c44b0 serial driver

In reply to Brian Cavagnolo

Dear Brian Cavagnolo,

Quoted 3 lines · Brian Cavagnolo

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?

Quoted 13 lines

board specific.

Signed-off-by Brian Cavagnolo <[email protected]>

---
?arch/arm/include/asm/arch-s3c44b0/hardware.h | ? ?3 +- ?drivers/serial/serial_s3c44b0.c ? ? ? ? ? ? ?| ? 65 +------------------------- ?2 files changed, 4 insertions(+), 64 deletions(-)

arch/arm/include/asm/arch-s3c44b0/hardware.h +0 -0
index 146e265..38ff32c 100644

arch/arm/include/asm/arch-s3c44b0/hardware.h line 15

Quoted 6 lines
arch/arm/include/asm/arch-s3c44b0/hardware.h +2 -1
@@ -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)

Quoted 11 lines

?/*****************************/
?/* CPU Wrapper Registers ? ? */

drivers/serial/serial_s3c44b0.c +0 -0
index 95d0266..e6c535c 100644
@@ -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?

Quoted 78 lines

- ? ? ? ? ? ? ? tmp = REGB(URXH0);
+ ? ? ? ? ? ? ? tmp = URXH0;
? ? ? ?}

? ? ? ?return 0;

drivers/serial/serial_s3c44b0.c +1 -63
@@ -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

Quoted 5 lines

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Thanks
Minkyu Kang.

Signature · 2 lines
from. prom.
www.promsoft.net
#4 lore raw

PATCH 1/1 bug fix and cleanup for s3c44b0 serial driver

Dear Minkyu Kang,

my five cents:

Quoted 3 lines
Quoted 3 lines
 #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?

Quoted 5 lines
Quoted 2 lines

+#define BRD(bps) (DIV_ROUND(CONFIG_S3C44B0_CLOCK_SPEED * 1000000, \

Quoted 1 line

+ (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) ?

Quoted 2 lines
Quoted 2 lines

/*****************************/
/* CPU Wrapper Registers */

I think that version of multi-line comment is not allowed...

Best Regards, Reinhard

#5 lore raw

PATCH 1/1 bug fix and cleanup for s3c44b0 serial driver

Dear Reinhard Meyer,

Quoted 9 lines · Reinhard Meyer

Dear Minkyu Kang,

my five cents:

Quoted 3 lines
Quoted 3 lines

?#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.

Quoted 10 lines
Quoted 7 lines
Quoted 4 lines

+#define BRD(bps) ? ? ? (DIV_ROUND(CONFIG_S3C44B0_CLOCK_SPEED * 1000000, \

Quoted 1 line

+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? (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 (:

Quoted 5 lines
Quoted 3 lines
Quoted 3 lines

?/*****************************/
?/* CPU Wrapper Registers ? ? */

I think that version of multi-line comment is not allowed...

Yes, so I requested clean-up patch for this file.

Quoted 3 lines

Best Regards, Reinhard

Thanks
Minkyu Kang.

Signature · 2 lines
from. prom.
www.promsoft.net