Re: [PATCH RFC 2/2] serial: 8250: Add PRUSS UART driver

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Thu, May 08, 2025 at 05:09:03PM -0500, Judith Mendez wrote:
> On 5/2/25 4:50 AM, Andy Shevchenko wrote:
> > On Wed, Apr 30, 2025 at 07:31:13PM -0500, Judith Mendez wrote:

...

> > The list of the inclusions is semi-random. Please, follow the IWYU principle.

This and other comments left unanswered, why? What does this mean? Usual way is
to remove the context with all what you are agree on and discuss only stuff
that needs more elaboration.

Ah, I see now the P.S., but please also remove the context you agree with next
time.

> > > +#include <linux/clk.h>
> > > +#include <linux/module.h>
> > > +#include <linux/serial_reg.h>
> > > +#include <linux/serial_core.h>
> > 
> > > +#include <linux/of_irq.h>
> > > +#include <linux/of_address.h>
> > > +#include <linux/of_platform.h>
> > 
> > Please, no of*.h in a new code.
> 
> Will only keep linux/of_platform.h for of_device_id struct.

Hmm... Is it really the header where it's defined? (I know the answer,
but want you to perform some research.)

> > > +#include <linux/remoteproc.h>
> > 
> > Keep them ordered as well.
> > 
> > + blank line here.
> > 
> > > +#include "8250.h"

...

> > > +	/* Old custom speed handling */
> > > +	if (baud == 38400 && (port->flags & UPF_SPD_MASK) == UPF_SPD_CUST) {
> > > +		quot = port->custom_divisor & UART_DIV_MAX;
> > > +		if (port->custom_divisor & (1 << 16))
> > > +			*frac = PRUSS_UART_MDR_13X_MODE;
> > > +		else
> > > +			*frac = PRUSS_UART_MDR_16X_MODE;
> > > +
> > > +		return quot;
> > > +	}
> > 
> > Why?! Please, try to avoid adding more drivers with this ugly hack.
> 
> My understanding is that this is not a hack, for 38400 we need to pass
> as custom baud. What is the alternative here?

BOTHER. The 38400 is a hack, you lie to the stakeholders that you are at 38.4,
while in real life it means anything.

> I see other drivers are doing this as well, will look into this further
> but not sure if there is a better solution for this.

BOTHER is the solution. Not perfect, but the existing one.

-- 
With Best Regards,
Andy Shevchenko