Message ID | 41B1320691916DE6+20250109120808.559950-1-wangyuli@uniontech.com |
---|---|
State | New |
Headers | show |
Series | serial: 8250_it8768e: Create iTE IT8768E specific 8250 driver | expand |
On Thu, Jan 09, 2025 at 01:40:14PM +0100, Arnd Bergmann wrote: > On Thu, Jan 9, 2025, at 13:08, WangYuli wrote: > > [ General description per its product manual: ] > > The IT8768E-I is a highly integrated Super I/O using the Low Pin > > Count Interface. The device’s LPC interface complies with Intel > > "LPC Interface Specification Rev. 1.1”. The IT8768E-I is ACPI & > > LANDesk compliant. > > > > Integrated in the IT8768E-I are five logical devices, which can > > be individually enabled or disabled via software configuration > > registers, and four 16C550standard compatible enhanced UARTs > > perofrmacing asynchronous communication. The devices also provide > > GPIO port controlling up to 12 GPIO pins. > > > > The IT8768E-I utilizes power-saving circuitry to reduce power > > consumption, and once a logical device is disabled, the inputs > > are inhibited with the clock disabled and the outputs are > > tri-stated. The device requires a single 24/48 MHz clock input > > and operates with +3.3V power supply. The IT8768E-I is available > > in 48-pin LQFP. > > > > It has been determined that this chip is currently employed within > > YIHUA STS-320 intelligent teller terminals utilizing > > PCBA F21-2401 D2000 MB VerA LF motherboards. > > Can you explain why this isn't done as part of > drivers/tty/serial/8250/8250_pnp.c? I assume you meant 8250_platform.c. PNP is for devices went through legacy PNP enumeration, most likely having IOPORT instead of IOMEM. Recently 8250_platform.c was expanded to cover ACPI IDs and it seems they have proper ID allocated for their device, so that's where it seems best to fit. > I see nothing unusual in here that requires a custom driver. +1.
On Mon, Jan 13, 2025, at 10:59, Andy Shevchenko wrote: > On Thu, Jan 09, 2025 at 01:40:14PM +0100, Arnd Bergmann wrote: >> On Thu, Jan 9, 2025, at 13:08, WangYuli wrote: >> >> Can you explain why this isn't done as part of >> drivers/tty/serial/8250/8250_pnp.c? > > I assume you meant 8250_platform.c. PNP is for devices went through legacy PNP > enumeration, most likely having IOPORT instead of IOMEM. No, I meant the 8250_pnp.c file. > Recently 8250_platform.c was expanded to cover ACPI IDs and it seems they have > proper ID allocated for their device, so that's where it seems best to fit. I don't think we should expand the use of 8250_platform.c any more than it is already used for. The ACPI device ID stuff in there is really only for one specific platform and should probably get moved out as well, the rest is there for hardwired "plat_serial8250_port" devices on 25+ year old machines that predate any type of firmware (pnpbios, acpibios, of) or hardware (ispnp, pci, ...) autodetection for their uarts. Arnd
On Mon, Jan 13, 2025 at 11:36:08AM +0100, Arnd Bergmann wrote: > On Mon, Jan 13, 2025, at 10:59, Andy Shevchenko wrote: > > On Thu, Jan 09, 2025 at 01:40:14PM +0100, Arnd Bergmann wrote: > >> On Thu, Jan 9, 2025, at 13:08, WangYuli wrote: > >> > >> Can you explain why this isn't done as part of > >> drivers/tty/serial/8250/8250_pnp.c? > > > > I assume you meant 8250_platform.c. PNP is for devices went through legacy PNP > > enumeration, most likely having IOPORT instead of IOMEM. > > No, I meant the 8250_pnp.c file. I am puzzled then. How should it work? PNP ID != ACPI HID that's provided in this patch commit message. On top of that, PNP driver uses _legacy_ PMP bus and infrastructure. > > Recently 8250_platform.c was expanded to cover ACPI IDs and it seems they have > > proper ID allocated for their device, so that's where it seems best to fit. > > I don't think we should expand the use of 8250_platform.c > any more than it is already used for. The ACPI device ID stuff in > there is really only for one specific platform and should probably > get moved out as well, the rest is there for hardwired > "plat_serial8250_port" devices on 25+ year old machines that > predate any type of firmware (pnpbios, acpibios, of) or hardware > (ispnp, pci, ...) autodetection for their uarts. Okay, but I do not see any better fit. Again, PNP is not a fit here or please elaborate how as I'm lost.
On Mon, Jan 13, 2025, at 13:20, Andy Shevchenko wrote: > On Mon, Jan 13, 2025 at 11:36:08AM +0100, Arnd Bergmann wrote: >> On Mon, Jan 13, 2025, at 10:59, Andy Shevchenko wrote: >> > On Thu, Jan 09, 2025 at 01:40:14PM +0100, Arnd Bergmann wrote: >> >> On Thu, Jan 9, 2025, at 13:08, WangYuli wrote: >> >> >> >> Can you explain why this isn't done as part of >> >> drivers/tty/serial/8250/8250_pnp.c? >> > >> > I assume you meant 8250_platform.c. PNP is for devices went through legacy PNP >> > enumeration, most likely having IOPORT instead of IOMEM. >> >> No, I meant the 8250_pnp.c file. > > I am puzzled then. How should it work? PNP ID != ACPI HID that's provided in > this patch commit message. On top of that, PNP driver uses _legacy_ PMP bus > and infrastructure. I guess I don't understand enough about ACPI then, I always assumed these were the same identifiers. I do see that CONFIG_ACPI force-enables CONFIG_PNP, and I see the examples in https://uefi.org/htmlspecs/ACPI_Spec_6_4_html/06_Device_Configuration/ evice_Configuration.html refer to _HID values in the form of "PNP####". Arnd
On Mon, Jan 13, 2025 at 02:12:01PM +0100, Arnd Bergmann wrote: > On Mon, Jan 13, 2025, at 13:20, Andy Shevchenko wrote: > > On Mon, Jan 13, 2025 at 11:36:08AM +0100, Arnd Bergmann wrote: > >> On Mon, Jan 13, 2025, at 10:59, Andy Shevchenko wrote: > >> > On Thu, Jan 09, 2025 at 01:40:14PM +0100, Arnd Bergmann wrote: > >> >> On Thu, Jan 9, 2025, at 13:08, WangYuli wrote: > >> >> > >> >> Can you explain why this isn't done as part of > >> >> drivers/tty/serial/8250/8250_pnp.c? > >> > > >> > I assume you meant 8250_platform.c. PNP is for devices went through legacy PNP > >> > enumeration, most likely having IOPORT instead of IOMEM. > >> > >> No, I meant the 8250_pnp.c file. > > > > I am puzzled then. How should it work? PNP ID != ACPI HID that's provided in > > this patch commit message. On top of that, PNP driver uses _legacy_ PMP bus > > and infrastructure. > > I guess I don't understand enough about ACPI then, I always > assumed these were the same identifiers. I do see that > CONFIG_ACPI force-enables CONFIG_PNP, and I see the examples in > https://uefi.org/htmlspecs/ACPI_Spec_6_4_html/06_Device_Configuration/ evice_Configuration.html > refer to _HID values in the form of "PNP####". Check the implementation of the pnp_add_id(), for example. Besides that PNP device has only 24-bit DMA mask by default, which is due to legacy DMA controllers. In any case the differences between PNP IDs and ACPI IDs are described in the specification.
diff --git a/drivers/tty/serial/8250/8250_it8768e.c b/drivers/tty/serial/8250/8250_it8768e.c new file mode 100644 index 000000000000..c2c0aeed3918 --- /dev/null +++ b/drivers/tty/serial/8250/8250_it8768e.c @@ -0,0 +1,138 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Probe for 8250/16550-type iTE IT8768E serial ports. + * + * Based on drivers/char/serial.c which is in the history, by Linus Torvalds, Theodore Ts'o. + * + * Copyright (C) 2025 Uniontech Ltd. + * Author: WangYuli <wangyuli@uniontech.com> + */ +#include <linux/bitops.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/pci.h> +#include <linux/pnp.h> +#include <linux/serial_core.h> +#include <linux/string.h> + +#include <asm/byteorder.h> + +#include "8250.h" + +struct it8768e_data { + struct uart_8250_port uart; + int line; +}; + +static int it8768e_probe(struct platform_device *pdev) +{ + struct it8768e_data *data; + struct resource *res; + void *__iomem sio_base; + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (!res) { + dev_err(&pdev->dev, "memory resource not found\n"); + return -EINVAL; + } + + sio_base = devm_ioremap(&pdev->dev, res->start, resource_size(res)); + if (!sio_base) { + dev_err(&pdev->dev, "devm_ioremap error\n"); + return -ENOMEM; + } + + data = devm_kcalloc(&pdev->dev, 1, + sizeof(struct it8768e_data), + GFP_KERNEL); + if (!data) { + dev_err(&pdev->dev, "Failed to alloc private mem struct.\n"); + return -ENOMEM; + } + + spin_lock_init(&data->uart.port.lock); + data->uart.port.dev = &pdev->dev; + data->uart.port.regshift = 0; + data->uart.port.iotype = UPIO_MEM; + data->uart.port.type = PORT_16550A; + data->uart.port.membase = sio_base; + data->uart.port.mapbase = res->start; + data->uart.port.uartclk = 1843200; + data->uart.port.flags = UPF_FIXED_PORT | UPF_FIXED_TYPE | UPF_SKIP_TEST; + + data->line = serial8250_register_8250_port(&data->uart); + if (data->line < 0) { + dev_err(&pdev->dev, + "unable to resigter 8250 port (MEM%llx): %d\n", + (unsigned long long)res->start, 0); + return data->line; + } + + dev_set_drvdata(&pdev->dev, data); + return 0; +} + +static void it8768e_remove(struct platform_device *pdev) +{ + struct it8768e_data *data = dev_get_drvdata(&pdev->dev); + + if (!data) + return; + + del_timer(&data->uart.timer); + serial8250_unregister_port(data->line); +} + +#ifdef CONFIG_PM +static int it8768e_suspend(struct platform_device *pdev, pm_message_t state) +{ + struct it8768e_data *data = dev_get_drvdata(&pdev->dev); + + if (!data) + return -ENODEV; + + serial8250_suspend_port(data->line); + return 0; +} + +static int it8768e_resume(struct platform_device *pdev) +{ + struct it8768e_data *data = dev_get_drvdata(&pdev->dev); + + if (!data) + return -ENODEV; + + serial8250_resume_port(data->line); + + return 0; +} +#else +#define it8768e_suspend NULL +#define it8768e_resume NULL +#endif /* PM */ + +#ifdef CONFIG_ACPI +static const struct acpi_device_id it8768e_acpi_ids[] = { + { .id = "ITES0001" }, + {} +}; +MODULE_DEVICE_TABLE(acpi, it8768e_acpi_ids); +#else +#define it8768e_acpi_ids NULL +#endif /* ACPI */ + +static struct platform_driver it8768e_driver = { + .probe = it8768e_probe, + .remove = it8768e_remove, + .suspend = it8768e_suspend, + .resume = it8768e_resume, + .driver = { + .name = "it8768e", + .acpi_match_table = it8768e_acpi_ids, + }, +}; +module_platform_driver(it8768e_driver); + +MODULE_AUTHOR("WangYuli"); +MODULE_DESCRIPTION("8250 uart driver for iTE IT8768E"); +MODULE_LICENSE("GPL"); diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig index 55d26d16df9b..72e7f33d61d6 100644 --- a/drivers/tty/serial/8250/Kconfig +++ b/drivers/tty/serial/8250/Kconfig @@ -569,6 +569,12 @@ config SERIAL_8250_BCM7271 including DMA support and high accuracy BAUD rates, say Y to this option. If unsure, say N. +config SERIAL_8250_IT8768E + tristate "8250 support for iTE IT8768E uart" + depends on SERIAL_8250 + help + This option is used for iTE IT8768E serial ports. + config SERIAL_OF_PLATFORM tristate "Devicetree based probing for 8250 ports" depends on SERIAL_8250 && OF diff --git a/drivers/tty/serial/8250/Makefile b/drivers/tty/serial/8250/Makefile index 1516de629b61..97bbf05a49bd 100644 --- a/drivers/tty/serial/8250/Makefile +++ b/drivers/tty/serial/8250/Makefile @@ -35,6 +35,7 @@ obj-$(CONFIG_SERIAL_8250_HP300) += 8250_hp300.o obj-$(CONFIG_SERIAL_8250_HUB6) += 8250_hub6.o obj-$(CONFIG_SERIAL_8250_INGENIC) += 8250_ingenic.o obj-$(CONFIG_SERIAL_8250_IOC3) += 8250_ioc3.o +obj-$(CONFIG_SERIAL_8250_IT8768E) += 8250_it8768e.o obj-$(CONFIG_SERIAL_8250_LPC18XX) += 8250_lpc18xx.o obj-$(CONFIG_SERIAL_8250_LPSS) += 8250_lpss.o obj-$(CONFIG_SERIAL_8250_MEN_MCB) += 8250_men_mcb.o