Re: [EXT] Re: [PATCH v3 2/2] serial: 8250_dw: add Ambarella CV75 quirks

[Andy Shevchenko]

On Fri, Aug 14, 2026 at 12:39:12PM +0000, Long Zhao wrote:

> OK, Andy. We will switch to 8250_ambarella.c instead in V4.

You also may consider a quirk in 8250_of.c if it's quite standard OF-enumerated
8250-compatible device.

> 在 2026年8月14日，20:34，Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> 写道：
> 
> On Fri, Aug 14, 2026 at 12:10:34PM +0000, Long Zhao wrote:
> 
> > Originally we submitted a dedicated Ambarella UART driver. During review,
> > Linus pointed out that the register layout is basically 8250-compatible
> > and suggested reusing the existing 8250 code instead of maintaining a
> > separate driver. That is how we ended up with the current approach of
> > binding through 8250_dw plus a small set of Ambarella quirks.
> 
> Right, please re-read carefully what he wrote
> 
> "This needs to be rewritten using the 8250 library in the kernel
> under drivers/tty/serial/8250/8250_ambarella.c
> or similar."
> 
> You already confirmed that your IP has nothing to do with the Synopsys. Why is
> the 8250_dw touched?!
> 
> > [1] https://urldefense.com/v3/__https://lore.kernel.org/r/CAD**AjLm6HzEU1TM-c=Vtjv3FJEBecVP7TyxSgq_Q0zF-YwT6NA@xxxxxxxxxxxxxx/__;Kys!!PeEy7nZLVv0!j8LqPRT2bzim1iC2xrU0AW4QUYvJ4oc4JqHIP3DMrOJVI-SdMHNYj2KIirs8ySAtvDwx1-Glz56DsGX6v1vzzCpWOGOb9LYXjg$
> 
> > -----Original Message-----
> > From: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> > Sent: Friday, August 14, 2026 19:53
> > On Fri, Aug 14, 2026 at 09:33:45AM +0000, Long Zhao wrote:
> >> From: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> >> Sent: Friday, August 14, 2026 16:56
> >> On Thu, Aug 13, 2026 at 06:35:13PM +0800, Long Zhao via B4 Relay wrote:
> >> 
> >>> Add CV75 platform data with an ETOI IER mask and skip-set-rate so
> >>> the DesignWare 8250 driver can drive the Ambarella UART for console use.
> > 
> >> Thanks for the review. Agreed the commit message was too brief.

...

> >>> +    /* Keep variant-specific bits in the shadow across subsequent
> >>> +updates. */
> >> 
> >> This says what code is doing. Instead it must say "why?" the code is doing this.
> >> 
> >>> +    up->ier |= d->pdata->ier_mask;
> >>> +    serial_port_out(p, UART_IER, up->ier);

...

> >> In case there is an HW bug, where can we get the errata, please? (For
> >> a bare minimum add a reference to the Errata document you give to the
> >> customers.)
> > 
> >> This is not a silicon erratum. The Ambarella UART is not Synopsys
> >> DesignWare IP; it is an Ambarella 8250-compatible UART. We still use
> >> the snps-dw-apb-uart binding / 8250_dw because the programming model
> >> matches closely enough, with two quirks (ETOI IER bit and
> >> SKIP_SET_RATE). I will expand the commit message and rewrite the
> >> comment to explain why in the next revision.
> > 
> > Okay, what does it make to think that 8250_dw is a suitable choice?
> > What are the features that are repetitive would be if you use a custom
> > 8250-based driver (using the generic 8250 code)?

-- 
With Best Regards,
Andy Shevchenko