[PATCH v2 1/5] ARM: davinci: uart: move to devid based clk_get

[Manjunathappa, Prakash]

Hi Sekhar,

Thanks for the review.

On Thu, Jun 06, 2013 at 16:14:19, Nori, Sekhar wrote:
> 
> 
> On 6/6/2013 4:02 PM, Sekhar Nori wrote:
> > Hi Prakash,
> > 
> > It appears that this patch was not tested thoroughly. See below:
> > 
> > On 5/28/2013 1:58 PM, Manjunathappa, Prakash wrote:
> >> For modules having single clock, clk_get should be done with dev_id.
> >> But current davinci implementation handles multiple instances
> >> of the UART devices with single platform_device_register. Hence clk_get
> >> is based on con_id rather than dev_id, this is not correct. Do
> >> platform_device_register for each instance and clk_get on dev_id.
> >>
> >> Signed-off-by: Manjunathappa, Prakash <prakash.pm at ti.com>
> >> ---
> > 
> >> -static struct plat_serial8250_port da8xx_serial_pdata[] = {
> >> +static struct plat_serial8250_port da8xx_serial0_pdata[] = {
> >>  	{
> >>  		.mapbase	= DA8XX_UART0_BASE,
> >>  		.irq		= IRQ_DA8XX_UARTINT0,
> >> @@ -75,7 +75,9 @@ static struct plat_serial8250_port da8xx_serial_pdata[] = {
> >>  					UPF_IOREMAP,
> >>  		.iotype		= UPIO_MEM,
> >>  		.regshift	= 2,
> >> -	},
> >> +	}
> >> +};
> >> +static struct plat_serial8250_port da8xx_serial1_pdata[] = {
> >>  	{
> >>  		.mapbase	= DA8XX_UART1_BASE,
> >>  		.irq		= IRQ_DA8XX_UARTINT1,
> >> @@ -83,7 +85,9 @@ static struct plat_serial8250_port da8xx_serial_pdata[] = {
> >>  					UPF_IOREMAP,
> >>  		.iotype		= UPIO_MEM,
> >>  		.regshift	= 2,
> >> -	},
> >> +	}
> >> +};
> >> +static struct plat_serial8250_port da8xx_serial2_pdata[] = {
> >>  	{
> >>  		.mapbase	= DA8XX_UART2_BASE,
> >>  		.irq		= IRQ_DA8XX_UARTINT2,
> >> @@ -91,18 +95,31 @@ static struct plat_serial8250_port da8xx_serial_pdata[] = {
> >>  					UPF_IOREMAP,
> >>  		.iotype		= UPIO_MEM,
> >>  		.regshift	= 2,
> >> -	},
> >> -	{
> >> -		.flags	= 0,
> >> -	},
> >> +	}
> > 
> > 8250_core.c relies on sentinel value with p->flags = 0 to terminate
> > looking for more ports. You don't have sentinel values in any of pdata
> > the arrays you introduced so the code basically goes looking into areas
> > of memory not its own. This caused bunch of "cannot register port"
> > errors for me but I can easily imagine more serious errors.

Will fix this.

> 
> Similarly, you check for platform_data being NULL to go over the list of
> serial devices in the code in serial.c you introduced, but don't have a
> sentinel when you define the serial device array.
> 

I have fixed this and above, posted patch here:
http://davinci-linux-open-source.1494791.n2.nabble.com/PATCH-v3-0-5-ARM-davinci-fix-UART-clock-enabling-tt7583378.html

Thanks,
Prakash