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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxx>
>> ---
> 
>> -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.

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.

Thanks,
Sekhar
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux PPP]     [Linux FS]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Linmodem]     [Device Mapper]     [Linux Kernel for ARM]

  Powered by Linux