Re: [PATCH v2 04/12] tty: serial: samsung: prepare for different IO types

[Greg KH <gregkh@linuxfoundation.org>]

On Thu, Jan 04, 2024 at 03:41:28PM +0000, Tudor Ambarus wrote:
> 
> 
> On 1/4/24 15:32, Greg KH wrote:
> > On Thu, Dec 28, 2023 at 12:57:57PM +0000, Tudor Ambarus wrote:
> >> GS101's Connectivity Peripheral blocks (peric0/1 blocks) which
> >> include the I3C and USI (I2C, SPI, UART) only allow 32-bit
> >> register accesses. If using 8-bit register accesses, a SError
> >> Interrupt is raised causing the system unusable.
> >>
> >> Instead of specifying the reg-io-width = 4 everywhere, for each node,
> >> the requirement should be deduced from the compatible.
> >>
> >> Prepare the samsung tty driver to allow IO types different than
> >> UPIO_MEM. ``struct uart_port::iotype`` is an unsigned char where all
> >> its 8 bits are exposed to uapi. We can't make NULL checks on it to
> >> verify if it's set, thus always set it from the driver's data.
> >>
> >> Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
> >> ---
> >> v2: new patch
> >>
> >>  drivers/tty/serial/samsung_tty.c | 9 ++++++++-
> >>  1 file changed, 8 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c
> >> index 66bd6c090ace..97ce4b2424af 100644
> >> --- a/drivers/tty/serial/samsung_tty.c
> >> +++ b/drivers/tty/serial/samsung_tty.c
> >> @@ -72,6 +72,7 @@ struct s3c24xx_uart_info {
> >>  	const char		*name;
> >>  	enum s3c24xx_port_type	type;
> >>  	unsigned int		port_type;
> >> +	unsigned char		iotype;
> >>  	unsigned int		fifosize;
> >>  	unsigned long		rx_fifomask;
> >>  	unsigned long		rx_fifoshift;
> > 
> > Is there a reason you are trying to add unused memory spaces to this
> > structure for no valid reason?  I don't think you could have picked a
> > more incorrect place in there to add this :)
> > 
> > Please fix.
> > 
> 
> Will put it after "const char *name".

If you do, spend some time with the tool, pahole, and see if that's
really the best place for it or not.  Might be, might not be, but you
should verify it please.

thanks,

greg k-h