Skip to contentSkip to conversation

serial: 8250_fintek.c: Report chipID

PATCH Superseded Trevor Woerner 2 participants 3 messages Mar 11, 2022 TTY layer and serial drivers

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

PATCH 2/3 serial: 8250_fintek.c: Report chipID

Provide some feedback to confirm this driver is enabled, and specify which chip was detected.

---
 drivers/tty/serial/8250/8250_fintek.c | 11 +++++++++++
 1 file changed, 11 insertions(+)
drivers/tty/serial/8250/8250_fintek.c +11 -0
index d9f0e546b1a1..03ad2354d808 100644
@@ -155,11 +155,22 @@ static int fintek_8250_check_id(struct fintek_8250 *pdata)
 
 	switch (chip) {
 	case CHIP_ID_F81865:
+		pr_info("Fintek F81865\n");
+		break;
 	case CHIP_ID_F81866:
+		pr_info("Fintek F81866\n");
+		break;
 	case CHIP_ID_F81966:
+		pr_info("Fintek F81966\n");
+		break;
 	case CHIP_ID_F81216AD:
+		pr_info("Fintek F81216AD\n");
+		break;
 	case CHIP_ID_F81216H:
+		pr_info("Fintek F81216H\n");
+		break;
 	case CHIP_ID_F81216:
+		pr_info("Fintek F81216\n");
 		break;
 	default:
 		return -ENODEV;
Signature · 1 line
2.34.1.75.gabe6bb3905
maintainer
#1 lore raw
Quoted 2 lines · Trevor Woerner

Provide some feedback to confirm this driver is enabled, and specify which chip was detected.

No, we don't do that. In fact, the output is mostly useless as it doesn't even tell the user what device this is about.

Quoted 22 lines
---
  drivers/tty/serial/8250/8250_fintek.c | 11 +++++++++++
  1 file changed, 11 insertions(+)
drivers/tty/serial/8250/8250_fintek.c +6 -0
index d9f0e546b1a1..03ad2354d808 100644
@@ -155,11 +155,22 @@ static int fintek_8250_check_id(struct fintek_8250 *pdata)
 
  	switch (chip) {
  	case CHIP_ID_F81865:
+		pr_info("Fintek F81865\n");
+		break;
  	case CHIP_ID_F81866:
+		pr_info("Fintek F81866\n");
+		break;
  	case CHIP_ID_F81966:
+		pr_info("Fintek F81966\n");
+		break;
  	case CHIP_ID_F81216AD:

drivers/tty/serial/8250/8250_fintek.c static int fintek_8250_check_id(struct fintek_8250 *pdata) · line 176

Quoted 10 lines
drivers/tty/serial/8250/8250_fintek.c +5 -0
+		pr_info("Fintek F81216AD\n");
+		break;
  	case CHIP_ID_F81216H:
+		pr_info("Fintek F81216H\n");
+		break;
  	case CHIP_ID_F81216:
+		pr_info("Fintek F81216\n");
  		break;
  	default:
  		return -ENODEV;

thanks,

Signature · 1 line
js
#2 lore raw

Hi Jiri,

Thanks for your review.

Quoted 6 lines · Jiri Slaby
Quoted 2 lines · Trevor Woerner

Provide some feedback to confirm this driver is enabled, and specify which chip was detected.

No, we don't do that. In fact, the output is mostly useless as it doesn't even tell the user what device this is about.

With this patch, when looking at the kernel's bootup messages one would see:

	[    1.809223] Serial: 8250/16550 driver, 5 ports, IRQ sharing enabled
	[    1.814420] Fintek F81865
	[    1.815783] 00:04: ttyS1 at I/O 0x2f8 (irq = 3, base_baud = 115200) is a 16550A
	[    1.822078] Fintek F81865
	[    1.823453] 00:05: ttyS2 at I/O 0x3e8 (irq = 10, base_baud = 115200) is a 16550A
	[    1.829813] Fintek F81865
	[    1.831194] 00:06: ttyS3 at I/O 0x2e8 (irq = 10, base_baud = 115200) is a 16550A
	[    1.837556] Fintek F81865
	[    1.838943] 00:07: ttyS4 at I/O 0x2c0 (irq = 10, base_baud = 115200) is a 16550A
	[    1.845350] Fintek F81865
	[    1.846703] 00:0c: ttyS0 at I/O 0x3f8 (irq = 4, base_baud = 115200) is a 16550A

…so the grouping would hopefully be a clue, and anyone looking up what a fintek f81865 is would see that it's a SuperIO chip with 6 UARTs.

Ideally that information would all be on one line for each port. The first line is what I've added, which specifies the actual device, and the second line comes from serial_core.c's uart_report_port(). If there were a way to pass the fintek information to serial_core, or a way to turn off serial_core's uart_report_port() so I could report it in the fintek driver that would be better, but neither of those exist.

If I want to know if a kernel has support for a device on my board I would normally grep the dmesg output and look for a product id, a chip id, or a manufacturer's name. I thought it was strange that with this driver enabled, there's no hint that the kernel knows anything about it.

Quoted 35 lines
Quoted 21 lines
---
  drivers/tty/serial/8250/8250_fintek.c | 11 +++++++++++
  1 file changed, 11 insertions(+)
drivers/tty/serial/8250/8250_fintek.c +6 -0
index d9f0e546b1a1..03ad2354d808 100644
@@ -155,11 +155,22 @@ static int fintek_8250_check_id(struct fintek_8250 *pdata)
  	switch (chip) {
  	case CHIP_ID_F81865:
+		pr_info("Fintek F81865\n");
+		break;
  	case CHIP_ID_F81866:
+		pr_info("Fintek F81866\n");
+		break;
  	case CHIP_ID_F81966:
+		pr_info("Fintek F81966\n");
+		break;
  	case CHIP_ID_F81216AD:

drivers/tty/serial/8250/8250_fintek.c static int fintek_8250_check_id(struct fintek_8250 *pdata) · line 175

Quoted 10 lines
drivers/tty/serial/8250/8250_fintek.c +5 -0
+		pr_info("Fintek F81216AD\n");
+		break;
  	case CHIP_ID_F81216H:
+		pr_info("Fintek F81216H\n");
+		break;
  	case CHIP_ID_F81216:
+		pr_info("Fintek F81216\n");
  		break;
  	default:
  		return -ENODEV;

thanks,

Signature · 1 line
js