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

[Stefan Roese <sr@denx.de>]

Hi Bin,

On 17.07.2017 16:49, Bin Meng wrote:
> +Simon,
> 
> Hi Stefan,
> 
> On Mon, Jul 17, 2017 at 7:18 PM, Stefan Roese <sr@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.

This does not seem to work. This IRQ routing code is still only
called pretty late in the init process:

U-Boot 2017.09-rc1-00210-gab169101b0-dirty (Aug 10 2017 - 11:29:37 +0200)

CPU: x86_64, vendor Intel, device 30679h
DRAM:  4 GiB
MMC:   pci_mmc: 0, pci_mmc: 1, pci_mmc: 2
SF: Detected w25q64cv with page size 256 Bytes, erase size 4 KiB, total 8 MiB
Model: theadorable-x86-DFI-BT700
SF: Detected w25q64cv with page size 256 Bytes, erase size 4 KiB, total 8 MiB
Net:   No ethernet found.
irq_router_common_init (245)
Assigning IRQ 5 to PCI device 0.2.0 (INTA)
Assigning IRQ 5 to PCI device 0.11.0 (INTA)
Assigning IRQ 5 to PCI device 0.12.0 (INTA)
Assigning IRQ 5 to PCI device 0.13.0 (INTA)
Assigning IRQ 5 to PCI device 0.14.0 (INTA)
Assigning IRQ 5 to PCI device 0.15.0 (INTA)
Assigning IRQ 5 to PCI device 0.16.0 (INTA)
Assigning IRQ 5 to PCI device 0.17.0 (INTA)
Assigning IRQ 5 to PCI device 0.18.0 (INTA)
Assigning IRQ 7 to PCI device 0.18.1 (INTC)
Assigning IRQ 10 to PCI device 0.18.2 (INTD)
Assigning IRQ 6 to PCI device 0.18.3 (INTB)
Assigning IRQ 5 to PCI device 0.18.4 (INTA)
Assigning IRQ 7 to PCI device 0.18.5 (INTC)
Assigning IRQ 10 to PCI device 0.18.6 (INTD)
Assigning IRQ 6 to PCI device 0.18.7 (INTB)
Assigning IRQ 5 to PCI device 0.1c.0 (INTA)
Assigning IRQ 5 to PCI device 0.1e.0 (INTA)
Assigning IRQ 10 to PCI device 0.1e.1 (INTD)
Assigning IRQ 6 to PCI device 0.1e.2 (INTB)
Assigning IRQ 7 to PCI device 0.1e.3 (INTC)
Assigning IRQ 10 to PCI device 0.1e.4 (INTD)
Assigning IRQ 6 to PCI device 0.1e.5 (INTB)
Assigning IRQ 6 to PCI device 0.1f.3 (INTB)
scanning bus for devices...

I see that this PCI interrupt routing is taken from the dts file. An
easy solution would be, to just add an interrupt entry to the PCI
UART device tree node:

------------------------- arch/x86/dts/dfi-bt700.dtsi -------------------------
index b62e00ff1f..1ccdf5d24b 100644
@@ -115,6 +115,7 @@
 			reg-shift = <2>;
 			clock-frequency = <58982400>;
 			current-speed = <115200>;
+			interrupts = <7>;
 		};

No need to change any of the interrupt related code and its
init sequence this way.

What do you think? Would this be acceptable?

Thanks,
Stefan