[U-Boot] [PATCH] serial: ns16550: fix different reg size access

Prafulla Wadaskar
2011-04-01T18:59:25Z

> -----Original Message-----
> From: u-boot-bounces at lists.denx.de [mailto:u-boot-bounces at lists.denx.de]
> On Behalf Of Lei Wen
> Sent: Friday, April 01, 2011 8:04 PM
> To: Wolfgang Denk
> Cc: Lei Wen; u-boot at lists.denx.de
> Subject: Re: [U-Boot] [PATCH] serial: ns16550: fix different reg size
> access
>
> Hi Wolfgang,
>
> On Fri, Apr 1, 2011 at 10:25 PM, Wolfgang Denk <wd@denx.de> wrote:
> > Dear Lei Wen,
> >
> > In message <AANLkTi=q-cj-
> Q5iNqiQpEddkH1DxUAR1H_5wenaES7bE at mail.gmail.com> you wrote:
> >>
> >> >> > I don't think so. You still use just a single writel() call
> then. ?To
> >> >> > leave the other bits untouched, you would have to perform a
> readl()
> >> >> > first, then insert one data byte, and then write it back. ?Your
> patch
> >> >> > does not do that.
> >> >>
> >> >> My original patch is like below, so where it call writel?...
> >> >> +#elif (CONFIG_SYS_NS16550_REG_SIZE == 1) ||
> (CONFIG_SYS_NS16550_REG_SIZE == -1)
> >> >> +#ifdef CONFIG_SYS_NS16550_PORT_MAPPED
> >> >> +#define serial_out(x, y) ? ? ? outb(x, y)
> >> >> +#define serial_in(y) ? ? ? ? ? inb(y)
> >> >> +#else
> >> >> +#define serial_out(x, y) ? ? ? writeb(x, y)
> >> >> +#define serial_in(y) ? ? ? ? ? readb(y)
> >> >
> >> > If you use writeb() [as the current driver would do as well}, then
> how
> >> > do you expect to set this bit 8 (which is in the next byte) to 0 as
> >> > you claim you have to?
> >>
> >> As I explain, if set CONFIG_SYS_NS16550_REG_SIZE to 4, and
> >> set CONFIG_SYS_NS16550_MAX_REG_SIZE also to 4, then the serial_out
> >> becomes writel. :)
> >
> > Right - which is exactly what I said, and which you denied.
> >
> > I give up, I have other things to do as well.
>
> Just a question to clarify... What your point I denied, that is really
> confused me...
> I think in this thread, I explain to you, my patch could recover what
> original
> CONFIG_SYS_NS16550_REG_SIZE means...
>
> Since you reject, I have nothing else to say...

Hi Lei
I understood this thread correctly as-
1. ns16550 is standard IP used across several SoC and has driver in place.
2. Your specific implementation of the same IP on your specific SoC need bit9 or x register to be set to 0 to work this IP correctly on your h/w.
3. but doing in ns16550 driver may brake other implementations.

(correct me if I am wrong)

So what I suggest here-
1. do not disturb/touch nx16550 driver at all.
2. write a small init code in cpu.c (specific to this SoC/arch) to reset this bit only.

This would be straight forward and will satisfy your need and acceptance too.

Regards..
Prafulla . .