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

[andriy.shevchenko@xxxxxxxxxxxxxxx]

On Wed, Jan 29, 2025 at 08:04:27AM +0000, Kartik Rajput wrote:
> On Wed, 2025-01-29 at 09:07 +0200, Andy Shevchenko wrote:
> > On Tue, Jan 28, 2025 at 12:16:33PM +0530, Kartik Rajput wrote:

...

> > > +     ret = of_property_read_u32(np, "current-speed", &tup-
> > > >baudrate);
> > 
> > Why not clock-frequency? But if needed, add this to the above
> > mentioned API as
> > I know more than one driver may utilise this.
> 
> "current-speed" is to specify the baudrate at which the UTC is
> operating. Not sure if "clock-frequency" is the ideal property for this
> as we are not configuring any clocks in the driver.

I didn't say anything about configuring clocks. The clock-frequency property is
standard way to provide a functional frequency of the UART (usually some MHz
crystal) which you can divide by the known HW coefficients and get the baud rate
(but yes, I agree that this is HW-dependent and needs to be thought through).

> Also, to add this in uart_read_port_properties() would require updating
> uart_port stucture to store the baudrate value. Would this be fine?

Sure, it requires some members to be defined beforehand.

> Asking because I do not see termios related configurations stored in
> uart_port structure.

Yeah, the only one is uartclk. But again, if you need current-speed to be
parsed, consider doing this in the uart_read_port_properties() and saving
as uartclk in the known way.
Technically it's possible to add a new member to uart_port, but it has
a lot of downsides as I can see: additional memory, added ambiguity to
how parse uartclk and current speed if they both are defined.

-- 
With Best Regards,
Andy Shevchenko