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
