Re: [PATCH v7 06/12] dt-bindings: serial: snps-dw-apb-uart: add Allwinner H616

[Andre Przywara <andre.przywara@arm.com>]

Hi,

On 11/3/26 22:39, James Hilliard wrote:
> The H616 UART has 256-byte transmit and receive FIFOs, but its DesignWare
> component parameter register does not report the FIFO depth. Its optional
> register layout also differs from the generic DesignWare integration.
> 
> Add a specific compatible to identify this fixed configuration, retaining
> snps,dw-apb-uart as a fallback.

I think we should not focus too much on single SoCs here, all the 
Allwinner UARTs need some care. We should have had SoC specific 
compatible strings (plus the fallback) from the beginning, but we 
didn't, mostly for historic reasons and because it just worked (TM).

So just from looking at the Allwinner A10 (the first one really 
supported mainline, since 2012): the manual reports a FIFO size of 64 
bytes, and it has none of the DW config registers.

So I'd suggest:
- We add compatible strings for *all* Allwinner SoC's UARTs in here, 
regardless of whether we currently need a quirk for each of them or not. 
This is now regarded best practise for DT anyway, we just catch up on this.

- The FIFO depth is all over the place for the various SoCs, and most of 
them do not implement the CPR register to announce the size. So we 
should use the already existing generic DT fifo-size property to 
describe the size. This is the simplest, yet most flexible solution, and 
avoids hardcoding implementation specific values in the driver. And it's 
friendly to other users of the DT nodes, be it U-Boot, the BSDs or 
whatever else is out there. IIUC, that should already work out of the 
box with the current Linux driver?
Since we need to touch the DTs anyway, I think this is the best way forward.

Cheers,
Andre

> Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
> ---
>   Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml b/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml
> index e2ca126d58ba..fc1289073241 100644
> --- a/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml
> +++ b/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml
> @@ -60,6 +60,7 @@ properties:
>             - const: renesas,rzn1-uart
>         - items:
>             - enum:
> +              - allwinner,sun50i-h616-uart
>                 - anlogic,dr1v90-uart
>                 - brcm,bcm11351-dw-apb-uart
>                 - brcm,bcm21664-dw-apb-uart
>