Re: [PATCH v8 4/6] xen/arm: Add the new OMAP UART driver.

[Julien Grall <julien.grall@linaro.org>]

On 08/22/2013 11:20 AM, Ian Campbell wrote:
> On Tue, 2013-08-13 at 19:14 +0800, Chen Baozi wrote:
>> TI OMAP UART introduces some features such as register access modes, which
>> makes its configuration and interrupt handling differs from 8250 compatible
>> UART. Thus, we seperate this driver from ns16550's implementation.
>>
>> Signed-off-by: Chen Baozi <baozich@gmail.com>
> 
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
> 
>> @@ -11,6 +11,7 @@ CFLAGS += -marm
>>  
>>  HAS_PL011 := y
>>  HAS_EXYNOS4210 := y
>> +HAS_OMAP := y
> 
> All three of these would be better with a _UART suffix IMHO, the
> HAS_OMAP makes it particularly obvious...
> 
> Would you mind making this change (in a follow up patch)?
> 
> [...]
>> +static int __init omap_uart_irq(struct serial_port *port)
>> +{
>> +    struct omap_uart *uart = port->uart;
>> +
>> +    return ((uart->irq.irq > 0) ? uart->irq.irq : -1);
>> +}
>> +[..]
>> +static struct uart_driver __read_mostly omap_uart_driver = {
> [...]
>> +    .irq = omap_uart_irq,
>> +    .dt_irq_get = omap_uart_dt_irq,
> [...]
> 
> This is really a question for Julien: Does a driver which is DT only
> need to provide the irq hook or is dt_irq sufficient?

dt_irq callback is enough. Other other DT drivers implement the irq
callback, which is not used by Xen on ARM.

I'm wondering if we can ifdef the irq callback (and so serial_irq) in
this case for ARM?

-- 
Julien Grall