[U-Boot] [PATCH v2] serial: ns16550: Add RX interrupt buffer support

[Bin Meng]

+Simon,

Hi Stefan,

On Mon, Jul 17, 2017 at 7:18 PM, Stefan Roese <sr at denx.de> wrote:
> Hi Bin,
>
>
> On 17.07.2017 11:43, Stefan Roese wrote:
>>
>> On 17.07.2017 11:26, Bin Meng wrote:
>>
>> <snip>
>>
>>>>>> +       }
>>>>>> +#endif
>>>>>> +
>>>>>>           return 0;
>>>>>>    }
>>>>>>
>>>>>> @@ -459,6 +559,15 @@ int ns16550_serial_ofdata_to_platdata(struct
>>>>>> udevice
>>>>>> *dev)
>>>>>>           if (port_type == PORT_JZ4780)
>>>>>>                   plat->fcr |= UART_FCR_UME;
>>>>>>
>>>>>> +#if CONFIG_IS_ENABLED(SERIAL_IRQ_BUFFER)
>>>>>> +       plat->irq = fdtdec_get_int(gd->fdt_blob, dev_of_offset(dev),
>>>>>> +                                  "interrupts", 0);
>>>>>
>>>>>
>>>>>
>>>>> Should we handle NS16550 on the PCI bus? If not, better put a comment
>>>>> here.
>>>>
>>>>
>>>>
>>>> Actually I'm also using this RX IRQ buffer for the HS-UART on BayTrail
>>>> connected via PCI (pciuart0: uart at 1e,3). I've added the interrupt
>>>> property to the DT for this to work for now. I'll check, if I can read
>>>> the interrupt from the PCI config space instead.
>>>>
>>>
>>> Yes, the interrupt line register is programmed with the value of its
>>> interrupt vector in irq_router_probe(). But you should wait for the
>>> IRQ device to be probed before NS16550.
>>
>>
>> I'm currently working on this and this dependency with the PCI interrupt
>> assignment is problematic. The serial driver is (intentionally) probed
>> earlier and the PCI interrupts are still unassigned at that stage.
>>
>> I currently have no real good idea on how to solve this. We could
>> add a new "probe_late() / probe_irqs_enabled()" DM function for such
>> cases, but this seems overly complex and adds new bloat to the DM
>> infrastructure.
>>
>> Do you have some other (simpler) ideas on how to solve this?
>
>
> Another idea would be, to write the PCI interrupt vectors earlier
> in the boot stage, before the serial driver is probed. The interrupts
> don't need to be enabled for this.
>
> What do you think of this idea?
>

Yep, I believe we can move the following codes from
arch/x86/cpu/i386/interrupts.c

/* Try to set up the interrupt router, but don't require one */
ret = uclass_first_device_err(UCLASS_IRQ, &dev);
if (ret && ret != -ENODEV)
return ret;

to arch_cpu_init_dm() in board_f.c.

Simon, do you have other better solution for this?

> Thanks for testing and confirming. Currently I have no idea, where
> this problem might come from. And my time is limited right now,
> as I'll be on vacation for 10 days starting on Wednesday. I still think
> that this RX interrupt buffer support is a big improvement over the
> "polling only" mode, even in this current version. I can re-visit this
> driver in a few weeks to work on a fix for the issue that you have
> found.

Sure, let's re-visit this in the coming days.

Regards,
Bin