On Mon, Mar 30, 2009 at 02:32:52PM -0500, Hugo Villeneuve wrote:
> On Mon, 30 Mar 2009 10:14:28 -0700
> "Mark A. Greer" <mgr...@mvista.com> wrote:
> 
> > On Sat, Mar 28, 2009 at 11:33:58PM -0400, Hugo Villeneuve wrote:
> > > On Sat, 28 Mar 2009 19:05:13 -0700

> > > Hi Mark,
> > > In doing so, it seems you are re-introducing the bug that I fixed
> > > in commit aeb81be782b838f96b1eca90ff49b217035b8461
> > > 
> > > Can you please check that and correct this patch?
> > 
> > Hi Hugo.
> > 
> > I thought I captured the essence of your patch.  Which, AFAICT, is to
> > replace the 'p->flags = 0' with a 'continue' inside the check if the
> > uart is enabled.
> 
> Partly true, there is more to it tought...
> 
> My patch has a static array containing the UART infos, and the a dynamic 
> array which is filled based on the number of UARTs enabled.
> 
> p should be a pointer where to WRITE the platform data
> i should be an index for READING static infos.
> 
> For example, your code will not work if you only enable UART0 and UART2:
> 
> i           p
> ===============================
> 0  dev->platform_data[0]  p points to UART0 data
> 1  dev->platform_data[1]  UART1 is not enabled, so p is not incremented 
> (continue statement)
> 2  dev->platform_data[1]  Wrong, your p still point to UART1 data, not UART2
> 
> So if you want your patch to work, you simply have to define like I did:
> 
> static struct plat_serial8250_port serial_platform_data[DAVINCI_MAX_NR_UARTS 
> + 1];
> 
> and initialize your p pointer to point to it, and read static data from 
> dev->platform_data[i].

As I look closer, I don't like this solution.  The problem is that
everyone (including the firmware) refers to the 3rd uart as uart2
but when you don't add uart1 in the kernel, uart2 will be referred
to as uart1 (ttyS1) by linux.  So, at a u-boot prompt, that uart
is known as uart2 but at a kernel prompt, its knows as uart1.
Also the documentation will refer to it as uart2 but anyone using
linux will have to somehow know that when the doc says uart2, that
really means uart1 on linux.

Not good.

The serial infrastructure is pretty limited WRT this (I have same issue
on da830 evm--I only want to use uart2).  It pretty much forces us to
set up/power up all 3 (if they aren't powered up the serial subsys hangs
when it accesses the uart), let the serial subsystem query the uart, etc.,
then power down the ones not wanted (and not set up the pinmux for the
unwanted uarts).  I don't know of a good hook to power the unwanted ones
though...well, maybe late_initcall()?

I'll look/play around to see if I can come up with something.

Mark
--

_______________________________________________
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source

Reply via email to