[PATCH v2 00/19] serial: samsung: gs101 updates and winter cleanup

* [PATCH v2 00/19] serial: samsung: gs101 updates and winter cleanup
@ 2024-01-19 10:45 Tudor Ambarus
  2024-01-19 10:45 ` [PATCH v2 01/19] tty: serial: samsung: fix tx_empty() to return TIOCSER_TEMT Tudor Ambarus
                   ` (18 more replies)
  0 siblings, 19 replies; 22+ messages in thread
From: Tudor Ambarus @ 2024-01-19 10:45 UTC (permalink / raw)
  To: gregkh, jirislaby, robh+dt, krzysztof.kozlowski+dt, conor+dt
  Cc: alim.akhtar, linux-kernel, linux-serial, devicetree,
	linux-arm-kernel, linux-samsung-soc, andre.draszik, kernel-team,
	peter.griffin, semen.protsenko, willmcvicker, Tudor Ambarus

Changes in v2:
- put as first patch in the series the patch that fixes tx_empty()
  to return TIOCSER_TEMT. Did that so that it can be easily backported
  to the stable kernels without other dependencies. Add fixes tag for
  the same patch.
- follow with a dt-bindings patch that was missed in the initial submission.
  Krzysztof asked to be queued through the tty tree.
- split a 81 length line in 2 lines in patch
  ``tty: serial: samsung: don't compare with zero an if (bitwise expression)``
  Update the commit message.
- drop extra ``!!`` on methods already returning bool
- update commit message and be more verbose in the patch that shrinks
  the clock selection to 8 clocks.
- use bool for has_divslot instead of bitfield. We don't expect more
  flags soon that would bypass the first cacheline of
  ``struct s3c24xx_uart_info``. Bitfields operations incur performance
  penalty when set or read as compared to direct types, bool shall be
  fine for now.

Hi,

The patch set is intended for v6.9 and is expected to be queued through
Greg's tty tree.

The patch set includes updates for GS101 so that we infer the IO type
from the compatible. This is because the GS101 Peripheral Blocks, which
include the serial, only allow 32-bit register accesses. So instead of
specifying the reg-io-width = 4 property everywhere, deduce the iotype
from the compatible. The GS101 patches were previously proposed at:
Link: https://lore.kernel.org/linux-arm-kernel/20240109125814.3691033-1-tudor.ambarus@linaro.org/

The patch set includes some cleanup changes that started as a
consequence of trying to reduce the memory footprint of the
``struct s3c24xx_uart_info``. For arm32 the struct was not as bad
defined as for arm64, because all its members could fit in the same
cacheline. But for arm64 we started from:

struct s3c24xx_uart_info {
	const char  *              name;                 /*     0     8 */
	enum s3c24xx_port_type     type;                 /*     8     4 */
	unsigned int               port_type;            /*    12     4 */
	unsigned int               fifosize;             /*    16     4 */

	/* XXX 4 bytes hole, try to pack */

	long unsigned int          rx_fifomask;          /*    24     8 */
	long unsigned int          rx_fifoshift;         /*    32     8 */
	long unsigned int          rx_fifofull;          /*    40     8 */
	long unsigned int          tx_fifomask;          /*    48     8 */
	long unsigned int          tx_fifoshift;         /*    56     8 */
	/* --- cacheline 1 boundary (64 bytes) --- */
	long unsigned int          tx_fifofull;          /*    64     8 */
	unsigned int               def_clk_sel;          /*    72     4 */

	/* XXX 4 bytes hole, try to pack */

	long unsigned int          num_clks;             /*    80     8 */
	long unsigned int          clksel_mask;          /*    88     8 */
	long unsigned int          clksel_shift;         /*    96     8 */
	long unsigned int          ucon_mask;            /*   104     8 */
	unsigned int               has_divslot:1;        /*   112: 0  4 */

	/* size: 120, cachelines: 2, members: 16 */
	/* sum members: 104, holes: 2, sum holes: 8 */
	/* sum bitfield members: 1 bits (0 bytes) */
	/* padding: 4 */
	/* bit_padding: 31 bits */
	/* last cacheline: 56 bytes */
};

and after the cleaning we get to:
struct s3c24xx_uart_info {
	const char  *              name;                 /*     0     8 */
	enum s3c24xx_port_type     type;                 /*     8     4 */
	unsigned int               port_type;            /*    12     4 */
	unsigned int               fifosize;             /*    16     4 */
	u32                        rx_fifomask;          /*    20     4 */
	u32                        rx_fifoshift;         /*    24     4 */
	u32                        rx_fifofull;          /*    28     4 */
	u32                        tx_fifomask;          /*    32     4 */
	u32                        tx_fifoshift;         /*    36     4 */
	u32                        tx_fifofull;          /*    40     4 */
	u32                        clksel_mask;          /*    44     4 */
	u32                        clksel_shift;         /*    48     4 */
	u32                        ucon_mask;            /*    52     4 */
	u8                         def_clk_sel;          /*    56     1 */
	u8                         num_clks;             /*    57     1 */
	u8                         iotype;               /*    58     1 */
	bool                       has_divslot;          /*    59     1 */

	/* size: 64, cachelines: 1, members: 17 */
	/* padding: 4 */
};

Also note that sorting the include files in alphabetic order in the
driver revealed some problems that were fixed with the following
patches:
Link: https://lore.kernel.org/linux-arm-kernel/20240110074007.4020016-1-tudor.ambarus@linaro.org/
Link: https://lore.kernel.org/linux-kernel/20240109141045.3704627-1-tudor.ambarus@linaro.org/

Cheers,
ta

Tudor Ambarus (19):
  tty: serial: samsung: fix tx_empty() to return TIOCSER_TEMT
  dt-bindings: serial: samsung: do not allow reg-io-width for gs101
  tty: serial: samsung: prepare for different IO types
  tty: serial: samsung: set UPIO_MEM32 iotype for gs101
  tty: serial: samsung: add gs101 earlycon support
  tty: serial: samsung: sort headers alphabetically
  tty: serial: samsung: explicitly include <linux/types.h>
  tty: serial: samsung: use u32 for register interactions
  tty: serial: samsung: remove braces on single statement block
  tty: serial: samsung: move open brace '{' on the next line
  tty: serial: samsung: drop superfluous comment
  tty: serial: samsung: make max_count unsigned int
  tty: serial: samsung: don't compare with zero an if (bitwise
    expression)
  tty: serial: samsung: return bool for s3c24xx_serial_txempty_nofifo()
  tty: serial: samsung: return bool for s3c24xx_serial_console_txrdy()
  tty: serial: samsung: change return type for
    s3c24xx_serial_rx_fifocnt()
  tty: serial: samsung: shrink the clock selection to 8 clocks
  tty: serial: samsung: change has_divslot type to bool
  tty: serial: samsung: shrink memory footprint of ``struct
    s3c24xx_uart_info``

 .../bindings/serial/samsung_uart.yaml         |   2 +
 drivers/tty/serial/samsung_tty.c              | 245 ++++++++++--------
 2 files changed, 141 insertions(+), 106 deletions(-)

-- 
2.43.0.429.g432eaa2c6b-goog

^ permalink raw reply	[flat|nested] 22+ messages in thread