Re: [EXT] Re: [PATCH v3 2/2] serial: 8250_dw: add Ambarella CV75 quirks
Andy Shevchenko <[email protected]>
| Newsgroups | org.kernel.vger.linux-devicetree,org.kernel.vger.linux-kernel,org.kernel.vger.linux-serial |
|---|---|
| Organization | Intel Finland Oy - BIC 0357606-4 - c/o Alberga Business Park, 6 krs, Bertel Jungin Aukio 5, 02600 Espoo |
| Message-ID | <[email protected]> |
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 <[email protected]> 写道: > > 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**[email protected]/__;Kys!!PeEy7nZLVv0!j8LqPRT2bzim1iC2xrU0AW4QUYvJ4oc4JqHIP3DMrOJVI-SdMHNYj2KIirs8ySAtvDwx1-Glz56DsGX6v1vzzCpWOGOb9LYXjg$ > > > -----Original Message----- > > From: Andy Shevchenko <[email protected]> > > Sent: Friday, August 14, 2026 19:53 > > On Fri, Aug 14, 2026 at 09:33:45AM +0000, Long Zhao wrote: > >> From: Andy Shevchenko <[email protected]> > >> 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