Re: [PATCH v3 2/2] serial: tegra-utc: Add driver for Tegra UART Trace Controller (UTC)

["andriy.shevchenko@linux.intel.com" <andriy.shevchenko@linux.intel.com>]

On Thu, Feb 13, 2025 at 10:11:36AM +0000, Kartik Rajput wrote:
> On Thu, 2025-02-13 at 11:55 +0200, andriy.shevchenko@linux.intel.com
> wrote:
> > On Thu, Feb 13, 2025 at 09:05:36AM +0000, Kartik Rajput wrote:
> > > On Wed, 2025-02-12 at 17:09 +0200, Andy Shevchenko wrote:
> > > > On Wed, Feb 12, 2025 at 04:11:32PM +0530, Kartik Rajput wrote:

...

> > > > > +     for (i = 0; i < len; i++) {
> > > > > +             if (!nbcon_enter_unsafe(wctxt))
> > > > > +                     break;
> > > > > +
> > > > > +             read_poll_timeout_atomic(tegra_utc_tx_readl, val,
> > > > > !(val & TEGRA_UTC_FIFO_FULL),
> > > > > +                                      0, USEC_PER_SEC, false,
> > > > > tup,
> > > > > TEGRA_UTC_FIFO_STATUS);
> > > > 
> > > > No error check?
> > > 
> > > I'm not sure about this. The case where the TX FIFO doesn't clear
> > > up,
> > > even after polling for 1 second, is highly unlikely, especially
> > > since
> > > there's no flow control involved here. Even if that did happen,
> > > writing
> > > to the TX FIFO should just result in an overflow, which is probably
> > > acceptable in this scenario.
> > 
> > Perhaps a warning (debug?) message in such a case?
> 
> I would prefer avoiding any prints in this function, as we are writing
> debug messages to the UART HW here.

Not every printf() goes to UART, but to your point the NBCON should solve that
issue to some extent. Of course, if the HW in a broken (unrecoverable) state,
nothing won't help.

> > > > > +             uart_console_write(&tup->port, wctxt->outbuf + i,
> > > > > 1,
> > > > > tegra_utc_console_putchar);
> > > > > +
> > > > > +             if (!nbcon_exit_unsafe(wctxt))
> > > > > +                     break;
> > > > > +     }

-- 
With Best Regards,
Andy Shevchenko