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" <[email protected]> wrote:
> 
> > On Sat, Mar 28, 2009 at 11:33:58PM -0400, Hugo Villeneuve wrote:
> > > On Sat, 28 Mar 2009 19:05:13 -0700
> > > "Mark A. Greer" <[email protected]> wrote:
> > > 
> > > > From: Mark A. Greer <[email protected]>
> > > > 
> > > > Currently, there is one set of platform_device and platform_data
> > > > structures for all DaVinci SoCs.  The differences in the data
> > > > between the various SoCs is handled by davinci_serial_init()
> > > > by checking the SoC type.  However, as new SoCs appear, this
> > > > routine will become more & more cluttered.
> > > > 
> > > > To clean up the routine and make it easier to add support for new
> > > > SoCs, move the platform_device and platform_data structures into
> > > > the SoC-specific code and use the SoC infrastructure to provide
> > > > access to the data.
> > > 
> > > 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].

Ah, I'm with you now.  Obviously, I didn't look close enough.
I'll fix.

Mark
--

_______________________________________________
Davinci-linux-open-source mailing list
[email protected]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source

Reply via email to