[PATCH RFT] tty: serial: fsl_lpuart: Add register dump

[Sherry Sun]

> Hi Sherry,
> 
> Am 26.05.26 um 17:21 schrieb Sherry Sun:
> >> Subject: [PATCH RFT] tty: serial: fsl_lpuart: Add register dump
> >>
> >> Dumping the registers from userspace for debug purposes isn't
> >> trivial, because runtime PM can gate the required clocks and any read
> >> access to these registers would result in a lockup.
> >>
> >> So implement a register dump via debugfs, which deals with the runtime
> PM.
> > No need, you can disable runtime pm from user space before dump the
> registers.
> > For example: echo on >
> > /sys/bus/platform/devices/29860000.serial/power/control
> so "comfort" wouldn't be an argument for this change?
> I'm asking, because this isn't the first driver with a regdump via debugfs.

I only see this method added in the pch_uart.c driver, and that was added 14 years ago and is obsolete.
Sorry, I don't think it's elegant to use this method just for "comfort" :(

Best Regards
Sherry
> 
> Best regards
> >
> > Best Regards
> > Sherry
> >> Signed-off-by: Stefan Wahren <wahrenst at gmx.net>
> >> ---
> >>
> >> Hi, I only have access to a board (i.MX93) with 32 bit registers, so
> >> it would be great if someone could test it for 8 bit registers.
> >>
> >> Thanks
> >>
> >>   drivers/tty/serial/fsl_lpuart.c | 69
> +++++++++++++++++++++++++++++++++
> >>   1 file changed, 69 insertions(+)
> >>
> >> diff --git a/drivers/tty/serial/fsl_lpuart.c
> >> b/drivers/tty/serial/fsl_lpuart.c index 1bd7ec9c81ea..7e4b3e59414a
> >> 100644
> >> --- a/drivers/tty/serial/fsl_lpuart.c
> >> +++ b/drivers/tty/serial/fsl_lpuart.c
> >> @@ -10,6 +10,7 @@
> >>   #include <linux/circ_buf.h>
> >>   #include <linux/clk.h>
> >>   #include <linux/console.h>
> >> +#include <linux/debugfs.h>
> >>   #include <linux/delay.h>
> >>   #include <linux/dma-mapping.h>
> >>   #include <linux/dmaengine.h>
> >> @@ -23,6 +24,7 @@
> >>   #include <linux/pinctrl/consumer.h>
> >>   #include <linux/platform_device.h>
> >>   #include <linux/pm_runtime.h>
> >> +#include <linux/seq_file.h>
> >>   #include <linux/serial_core.h>
> >>   #include <linux/slab.h>
> >>   #include <linux/tty_flip.h>
> >> @@ -291,6 +293,8 @@ struct lpuart_port {
> >>   	bool			is_cs7; /* Set to true when character size is 7
> >> */
> >>   					/* and the parity is enabled
> >> 	*/
> >>   	bool			dma_idle_int;
> >> +
> >> +	struct dentry		*debugfs_dir;
> >>   };
> >>
> >>   struct lpuart_soc_data {
> >> @@ -1031,6 +1035,7 @@ static void lpuart32_rxint(struct lpuart_port
> *sport)
> >>   	}
> >>
> >>   out:
> >> +
> >>   	uart_unlock_and_check_sysrq(&sport->port);
> >>
> >>   	tty_flip_buffer_push(port);
> >> @@ -2860,6 +2865,67 @@ static int lpuart_global_reset(struct
> >> lpuart_port
> >> *sport)
> >>   	return 0;
> >>   }
> >>
> >> +#ifdef CONFIG_DEBUG_FS
> >> +
> >> +#define dump_register_hex(_seq, _reg, _sport) \ seq_printf((_seq),
> >> +"%-12s: 0x%02x\n", #_reg, readb((_sport)->port.membase + (_reg)))
> >> +
> >> +#define dump_register32_hex(_seq, _reg, _sport) \ seq_printf((_seq),
> >> +"%-12s: 0x%08x\n", #_reg, lpuart32_read(&(_sport)->port, _reg))
> >> +
> >> +static int regs_show(struct seq_file *s, void *p) {
> >> +	struct lpuart_port *sport = s->private;
> >> +
> >> +	pm_runtime_get_sync(sport->port.dev);
> >> +
> >> +	if (lpuart_is_32(sport)) {
> >> +		dump_register32_hex(s, UARTBAUD, sport);
> >> +		dump_register32_hex(s, UARTSTAT, sport);
> >> +		dump_register32_hex(s, UARTCTRL, sport);
> >> +		dump_register32_hex(s, UARTMATCH, sport);
> >> +		dump_register32_hex(s, UARTMODIR, sport);
> >> +		dump_register32_hex(s, UARTFIFO, sport);
> >> +		dump_register32_hex(s, UARTWATER, sport);
> >> +	} else {
> >> +		dump_register_hex(s, UARTBDH, sport);
> >> +		dump_register_hex(s, UARTBDL, sport);
> >> +		dump_register_hex(s, UARTCR1, sport);
> >> +		dump_register_hex(s, UARTCR2, sport);
> >> +		dump_register_hex(s, UARTSR1, sport);
> >> +		dump_register_hex(s, UARTCR3, sport);
> >> +		dump_register_hex(s, UARTDR, sport);
> >> +		dump_register_hex(s, UARTCR4, sport);
> >> +		dump_register_hex(s, UARTCR5, sport);
> >> +		dump_register_hex(s, UARTMODEM, sport);
> >> +		dump_register_hex(s, UARTPFIFO, sport);
> >> +		dump_register_hex(s, UARTCFIFO, sport);
> >> +		dump_register_hex(s, UARTSFIFO, sport);
> >> +		dump_register_hex(s, UARTTWFIFO, sport);
> >> +		dump_register_hex(s, UARTTCFIFO, sport);
> >> +		dump_register_hex(s, UARTRWFIFO, sport);
> >> +	}
> >> +
> >> +	pm_runtime_mark_last_busy(sport->port.dev);
> >> +	pm_runtime_put_autosuspend(sport->port.dev);
> >> +
> >> +	return 0;
> >> +}
> >> +
> >> +DEFINE_SHOW_ATTRIBUTE(regs);
> >> +
> >> +static void lpuart_init_debugfs(struct lpuart_port *sport) {
> >> +	sport->debugfs_dir = debugfs_create_dir(dev_name(sport-
> >>> port.dev),
> >> +						NULL);
> >> +
> >> +	debugfs_create_file("regs", 0400, sport->debugfs_dir, sport,
> >> +&regs_fops); }
> >> +
> >> +#else
> >> +static inline void lpuart_init_debugfs(struct lpuart_port *sport) {}
> >> +#endif
> >> +
> >>   static int lpuart_probe(struct platform_device *pdev)  {
> >>   	const struct lpuart_soc_data *sdata =
> >> of_device_get_match_data(&pdev->dev);
> >> @@ -2969,6 +3035,8 @@ static int lpuart_probe(struct platform_device
> >> *pdev)
> >>   	if (ret)
> >>   		goto failed_irq_request;
> >>
> >> +	lpuart_init_debugfs(sport);
> >> +
> >>   	return 0;
> >>
> >>   failed_irq_request:
> >> @@ -2987,6 +3055,7 @@ static void lpuart_remove(struct
> >> platform_device
> >> *pdev)  {
> >>   	struct lpuart_port *sport = platform_get_drvdata(pdev);
> >>
> >> +	debugfs_remove_recursive(sport->debugfs_dir);
> >>   	uart_remove_one_port(&lpuart_reg, &sport->port);
> >>
> >>   	lpuart_disable_clks(sport);
> >> --
> >> 2.43.0