* [PATCH v4 0/2] Airoha UART support
@ 2025-02-09 21:02 Benjamin Larsson
2025-02-09 21:02 ` [PATCH 1/2] dt-bindings: serial: 8250: Add Airoha compatibles Benjamin Larsson
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Benjamin Larsson @ 2025-02-09 21:02 UTC (permalink / raw)
To: linux-serial, devicetree; +Cc: ansuelsmth, lorenzo, gregkh, Benjamin Larsson
The Airoha familty of SoCs have a UART hardware that is 16550-compatible
with the exception of the baud rate settings.
This patch implements code for calculating the baud rate for the Airoha
UART and HSUART.
Signed-off-by: Benjamin Larsson <benjamin.larsson@genexis.eu>
---
Changes in v4:
- Removed COMPILE_TEST from Kconfig
- Removed Kconfig option to build as module
Changes in v3:
- Reworded commit message
- Restructured comment text
- Fixed kernel-doc warning
- Fixed kernel test robot build error and build warning
Changes in v2:
- Removed ifdef use in .c files
- Removed uart port defines from user-space headers
- Reworded commit message
- Added code documentation
Benjamin Larsson (2):
dt-bindings: serial: 8250: Add Airoha compatibles
serial: Airoha SoC UART and HSUART support
.../devicetree/bindings/serial/8250.yaml | 2 +
drivers/tty/serial/8250/8250.h | 15 ++++
drivers/tty/serial/8250/8250_airoha.c | 83 +++++++++++++++++++
drivers/tty/serial/8250/8250_of.c | 2 +
drivers/tty/serial/8250/8250_port.c | 27 ++++++
drivers/tty/serial/8250/Kconfig | 10 +++
drivers/tty/serial/8250/Makefile | 1 +
7 files changed, 140 insertions(+)
create mode 100644 drivers/tty/serial/8250/8250_airoha.c
--
2.34.1
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH 1/2] dt-bindings: serial: 8250: Add Airoha compatibles 2025-02-09 21:02 [PATCH v4 0/2] Airoha UART support Benjamin Larsson @ 2025-02-09 21:02 ` Benjamin Larsson 2025-02-09 21:02 ` [PATCH 2/2] serial: Airoha SoC UART and HSUART support Benjamin Larsson 2025-02-10 6:14 ` [PATCH v4 0/2] Airoha UART support Greg KH 2 siblings, 0 replies; 11+ messages in thread From: Benjamin Larsson @ 2025-02-09 21:02 UTC (permalink / raw) To: linux-serial, devicetree Cc: ansuelsmth, lorenzo, gregkh, Benjamin Larsson, Krzysztof Kozlowski The Airoha SoC family have a mostly 16550-compatible UART and High-Speed UART hardware with the exception of custom baud rate settings register. Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Signed-off-by: Benjamin Larsson <benjamin.larsson@genexis.eu> --- Documentation/devicetree/bindings/serial/8250.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/serial/8250.yaml b/Documentation/devicetree/bindings/serial/8250.yaml index 692aa05500fd..2fbb972e5460 100644 --- a/Documentation/devicetree/bindings/serial/8250.yaml +++ b/Documentation/devicetree/bindings/serial/8250.yaml @@ -63,6 +63,8 @@ properties: - const: mrvl,pxa-uart - const: nuvoton,wpcm450-uart - const: nuvoton,npcm750-uart + - const: airoha,airoha-uart + - const: airoha,airoha-hsuart - const: nvidia,tegra20-uart - const: nxp,lpc3220-uart - items: -- 2.34.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/2] serial: Airoha SoC UART and HSUART support 2025-02-09 21:02 [PATCH v4 0/2] Airoha UART support Benjamin Larsson 2025-02-09 21:02 ` [PATCH 1/2] dt-bindings: serial: 8250: Add Airoha compatibles Benjamin Larsson @ 2025-02-09 21:02 ` Benjamin Larsson 2025-03-22 21:42 ` Andy Shevchenko 2025-02-10 6:14 ` [PATCH v4 0/2] Airoha UART support Greg KH 2 siblings, 1 reply; 11+ messages in thread From: Benjamin Larsson @ 2025-02-09 21:02 UTC (permalink / raw) To: linux-serial, devicetree; +Cc: ansuelsmth, lorenzo, gregkh, Benjamin Larsson Support for Airoha AN7581 SoC UART and HSUART baud rate calculation routine. Signed-off-by: Benjamin Larsson <benjamin.larsson@genexis.eu> --- drivers/tty/serial/8250/8250.h | 15 +++++ drivers/tty/serial/8250/8250_airoha.c | 81 +++++++++++++++++++++++++++ drivers/tty/serial/8250/8250_of.c | 2 + drivers/tty/serial/8250/8250_port.c | 26 +++++++++ drivers/tty/serial/8250/Kconfig | 10 ++++ drivers/tty/serial/8250/Makefile | 1 + 6 files changed, 135 insertions(+) create mode 100644 drivers/tty/serial/8250/8250_airoha.c diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h index e5310c65cf52..dd762289fa25 100644 --- a/drivers/tty/serial/8250/8250.h +++ b/drivers/tty/serial/8250/8250.h @@ -314,6 +314,21 @@ static inline int serial8250_in_MCR(struct uart_8250_port *up) return mctrl; } +/* uart_config[] table port type defines */ +/* Airoha UART */ +#define PORT_AIROHA 124 + +/* Airoha HSUART */ +#define PORT_AIROHA_HS 125 + +#ifdef CONFIG_SERIAL_8250_AIROHA +void airoha8250_set_baud_rate(struct uart_port *port, + unsigned int baud, unsigned int hs); +#else +static inline void airoha8250_set_baud_rate(struct uart_port *port, + unsigned int baud, unsigned int hs) { } +#endif + #ifdef CONFIG_SERIAL_8250_PNP int serial8250_pnp_init(void); void serial8250_pnp_exit(void); diff --git a/drivers/tty/serial/8250/8250_airoha.c b/drivers/tty/serial/8250/8250_airoha.c new file mode 100644 index 000000000000..51e675605741 --- /dev/null +++ b/drivers/tty/serial/8250/8250_airoha.c @@ -0,0 +1,81 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/* + * Airoha UART baud rate calculation function + * + * Copyright (c) 2025 Genexis Sweden AB + * Author: Benjamin Larsson <benjamin.larsson@genexis.eu> + */ + +#include "8250.h" + +/* The Airoha UART is 16550-compatible except for the baud rate calculation. */ + +/* Airoha UART registers */ +#define UART_AIROHA_BRDL 0 +#define UART_AIROHA_BRDH 1 +#define UART_AIROHA_XINCLKDR 10 +#define UART_AIROHA_XYD 11 + +#define XYD_Y 65000 +#define XINDIV_CLOCK 20000000 +#define UART_BRDL_20M 0x01 +#define UART_BRDH_20M 0x00 + +static const int clock_div_tab[] = { 10, 4, 2}; +static const int clock_div_reg[] = { 4, 2, 1}; + +/** + * airoha8250_set_baud_rate() - baud rate calculation routine + * @port: uart port + * @baud: requested uart baud rate + * @hs: uart type selector, 0 for regular uart and 1 for high-speed uart + * + * crystal_clock = 20 MHz (fixed frequency) + * xindiv_clock = crystal_clock / clock_div + * (x/y) = XYD, 32 bit register with 16 bits of x and then 16 bits of y + * clock_div = XINCLK_DIVCNT (default set to 10 (0x4)), + * - 3 bit register [ 1, 2, 4, 8, 10, 12, 16, 20 ] + * + * baud_rate = ((xindiv_clock) * (x/y)) / ([BRDH,BRDL] * 16) + * + * Selecting divider needs to fulfill + * 1.8432 MHz <= xindiv_clk <= APB clock / 2 + * The clocks are unknown but a divider of value 1 did not result in a valid + * waveform. + * + * XYD_y seems to need to be larger then XYD_x for proper waveform generation. + * Setting [BRDH,BRDL] to [0,1] and XYD_y to 65000 gives even values + * for usual baud rates. + */ + +void airoha8250_set_baud_rate(struct uart_port *port, + unsigned int baud, unsigned int hs) +{ + struct uart_8250_port *up = up_to_u8250p(port); + unsigned int xyd_x, nom, denom; + int i; + + /* set DLAB to access the baud rate divider registers (BRDH, BRDL) */ + serial_port_out(port, UART_LCR, up->lcr | UART_LCR_DLAB); + /* set baud rate calculation defaults */ + /* set BRDIV ([BRDH,BRDL]) to 1 */ + serial_port_out(port, UART_AIROHA_BRDL, UART_BRDL_20M); + serial_port_out(port, UART_AIROHA_BRDH, UART_BRDH_20M); + /* calculate XYD_x and XINCLKDR register by searching + * through a table of crystal_clock divisors + * + * for the HSUART xyd_x needs to be scaled by a factor of 2 + */ + for (i = 0 ; i < ARRAY_SIZE(clock_div_tab) ; i++) { + denom = (XINDIV_CLOCK/40) / clock_div_tab[i]; + nom = baud * (XYD_Y/40); + xyd_x = ((nom/denom) << 4) >> hs; + if (xyd_x < XYD_Y) + break; + } + serial_port_out(port, UART_AIROHA_XINCLKDR, clock_div_reg[i]); + serial_port_out(port, UART_AIROHA_XYD, (xyd_x<<16) | XYD_Y); + /* unset DLAB */ + serial_port_out(port, UART_LCR, up->lcr); +} diff --git a/drivers/tty/serial/8250/8250_of.c b/drivers/tty/serial/8250/8250_of.c index 64aed7efc569..5315bc1bc06d 100644 --- a/drivers/tty/serial/8250/8250_of.c +++ b/drivers/tty/serial/8250/8250_of.c @@ -341,6 +341,8 @@ static const struct of_device_id of_platform_serial_table[] = { { .compatible = "ti,da830-uart", .data = (void *)PORT_DA830, }, { .compatible = "nuvoton,wpcm450-uart", .data = (void *)PORT_NPCM, }, { .compatible = "nuvoton,npcm750-uart", .data = (void *)PORT_NPCM, }, + { .compatible = "airoha,airoha-uart", .data = (void *)PORT_AIROHA, }, + { .compatible = "airoha,airoha-hsuart", .data = (void *)PORT_AIROHA_HS, }, { /* end of list */ }, }; MODULE_DEVICE_TABLE(of, of_platform_serial_table); diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c index 1ea52fce9bf1..040659de35a2 100644 --- a/drivers/tty/serial/8250/8250_port.c +++ b/drivers/tty/serial/8250/8250_port.c @@ -319,6 +319,24 @@ static const struct serial8250_config uart_config[] = { .rxtrig_bytes = {1, 8, 16, 30}, .flags = UART_CAP_FIFO | UART_CAP_AFE, }, + /* From here on after additional uart config port defines are placed in 8250.h + */ + [PORT_AIROHA] = { + .name = "Airoha UART", + .fifo_size = 8, + .tx_loadsz = 1, + .fcr = UART_FCR_ENABLE_FIFO | UART_FCR_R_TRIG_01 | UART_FCR_CLEAR_RCVR, + .rxtrig_bytes = {1, 4}, + .flags = UART_CAP_FIFO, + }, + [PORT_AIROHA_HS] = { + .name = "Airoha HSUART", + .fifo_size = 128, + .tx_loadsz = 128, + .fcr = UART_FCR_ENABLE_FIFO | UART_FCR_R_TRIG_01 | UART_FCR_CLEAR_RCVR, + .rxtrig_bytes = {1, 4}, + .flags = UART_CAP_FIFO, + }, }; /* Uart divisor latch read */ @@ -2847,6 +2865,14 @@ serial8250_do_set_termios(struct uart_port *port, struct ktermios *termios, serial8250_set_divisor(port, baud, quot, frac); + /* + * Airoha SoCs have custom registers for baud rate settings + */ + if (port->type == PORT_AIROHA) + airoha8250_set_baud_rate(port, baud, 0); + if (port->type == PORT_AIROHA_HS) + airoha8250_set_baud_rate(port, baud, 1); + /* * LCR DLAB must be set to enable 64-byte FIFO mode. If the FCR * is written without DLAB set, this mode will be disabled. diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig index 55d26d16df9b..9cf16d392aca 100644 --- a/drivers/tty/serial/8250/Kconfig +++ b/drivers/tty/serial/8250/Kconfig @@ -356,6 +356,16 @@ config SERIAL_8250_ACORN system, say Y to this option. The driver can handle 1, 2, or 3 port cards. If unsure, say N. +config SERIAL_8250_AIROHA + bool "Airoha UART support" + depends on ARCH_AIROHA && OF && SERIAL_8250 + help + Selecting this option enables an Airoha SoC specific baud rate + calculation routine on an otherwise 16550 compatible UART hardware. + + If you have an Airoha based board and want to use the serial port, + say Y to this option. If unsure, say N. + config SERIAL_8250_BCM2835AUX tristate "BCM2835 auxiliar mini UART support" depends on ARCH_BCM2835 || COMPILE_TEST diff --git a/drivers/tty/serial/8250/Makefile b/drivers/tty/serial/8250/Makefile index 1516de629b61..b7f07d5c4cca 100644 --- a/drivers/tty/serial/8250/Makefile +++ b/drivers/tty/serial/8250/Makefile @@ -20,6 +20,7 @@ obj-$(CONFIG_SERIAL_8250_CONSOLE) += 8250_early.o obj-$(CONFIG_SERIAL_8250_ACCENT) += 8250_accent.o obj-$(CONFIG_SERIAL_8250_ACORN) += 8250_acorn.o +obj-$(CONFIG_SERIAL_8250_AIROHA) += 8250_airoha.o obj-$(CONFIG_SERIAL_8250_ASPEED_VUART) += 8250_aspeed_vuart.o obj-$(CONFIG_SERIAL_8250_BCM2835AUX) += 8250_bcm2835aux.o obj-$(CONFIG_SERIAL_8250_BCM7271) += 8250_bcm7271.o -- 2.34.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] serial: Airoha SoC UART and HSUART support 2025-02-09 21:02 ` [PATCH 2/2] serial: Airoha SoC UART and HSUART support Benjamin Larsson @ 2025-03-22 21:42 ` Andy Shevchenko 2025-03-25 14:19 ` Benjamin Larsson 0 siblings, 1 reply; 11+ messages in thread From: Andy Shevchenko @ 2025-03-22 21:42 UTC (permalink / raw) To: Benjamin Larsson; +Cc: linux-serial, devicetree, ansuelsmth, lorenzo, gregkh Sun, Feb 09, 2025 at 10:02:41PM +0100, Benjamin Larsson kirjoitti: > Support for Airoha AN7581 SoC UART and HSUART baud rate > calculation routine. ... > drivers/tty/serial/8250/8250_port.c | 26 +++++++++ > + /* > + * Airoha SoCs have custom registers for baud rate settings > + */ > + if (port->type == PORT_AIROHA) > + airoha8250_set_baud_rate(port, baud, 0); > + if (port->type == PORT_AIROHA_HS) > + airoha8250_set_baud_rate(port, baud, 1); Why is this here? Please, make it stay in your module. ... > +config SERIAL_8250_AIROHA > + bool "Airoha UART support" Why bool? > + depends on ARCH_AIROHA && OF && SERIAL_8250 What is the purpose of the OF dependency? -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] serial: Airoha SoC UART and HSUART support 2025-03-22 21:42 ` Andy Shevchenko @ 2025-03-25 14:19 ` Benjamin Larsson 2025-03-25 14:23 ` Andy Shevchenko 0 siblings, 1 reply; 11+ messages in thread From: Benjamin Larsson @ 2025-03-25 14:19 UTC (permalink / raw) To: Andy Shevchenko; +Cc: linux-serial, devicetree, ansuelsmth, lorenzo, gregkh Hi. On 2025-03-22 22:42, Andy Shevchenko wrote: > [You don't often get email from andy.shevchenko@gmail.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ] > > Sun, Feb 09, 2025 at 10:02:41PM +0100, Benjamin Larsson kirjoitti: >> Support for Airoha AN7581 SoC UART and HSUART baud rate >> calculation routine. > ... > >> drivers/tty/serial/8250/8250_port.c | 26 +++++++++ >> + /* >> + * Airoha SoCs have custom registers for baud rate settings >> + */ >> + if (port->type == PORT_AIROHA) >> + airoha8250_set_baud_rate(port, baud, 0); >> + if (port->type == PORT_AIROHA_HS) >> + airoha8250_set_baud_rate(port, baud, 1); > Why is this here? Please, make it stay in your module. I dont add an extra module I extend the already existing one. > ... > >> +config SERIAL_8250_AIROHA >> + bool "Airoha UART support" > Why bool? Because it is just an extension of an existing module. > >> + depends on ARCH_AIROHA && OF && SERIAL_8250 > What is the purpose of the OF dependency? I thought it was needed for dts support. I'll remove it. > > -- > With Best Regards, > Andy Shevchenko > > MvH Benajmin Larsson ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] serial: Airoha SoC UART and HSUART support 2025-03-25 14:19 ` Benjamin Larsson @ 2025-03-25 14:23 ` Andy Shevchenko 0 siblings, 0 replies; 11+ messages in thread From: Andy Shevchenko @ 2025-03-25 14:23 UTC (permalink / raw) To: Benjamin Larsson; +Cc: linux-serial, devicetree, ansuelsmth, lorenzo, gregkh On Tue, Mar 25, 2025 at 4:19 PM Benjamin Larsson <benjamin.larsson@genexis.eu> wrote: > On 2025-03-22 22:42, Andy Shevchenko wrote: > > Sun, Feb 09, 2025 at 10:02:41PM +0100, Benjamin Larsson kirjoitti: ... > >> drivers/tty/serial/8250/8250_port.c | 26 +++++++++ > >> + /* > >> + * Airoha SoCs have custom registers for baud rate settings > >> + */ > >> + if (port->type == PORT_AIROHA) > >> + airoha8250_set_baud_rate(port, baud, 0); > >> + if (port->type == PORT_AIROHA_HS) > >> + airoha8250_set_baud_rate(port, baud, 1); > > Why is this here? Please, make it stay in your module. > > I dont add an extra module I extend the already existing one. We are trying hard to get rid of custom quirks in 8250_pci.c. That's how a few 8250_*.c appeared during the past several years. Please, follow the same approach. ... > >> +config SERIAL_8250_AIROHA > >> + bool "Airoha UART support" > > Why bool? > > Because it is just an extension of an existing module. Please, make a new real module instead. > >> + depends on ARCH_AIROHA && OF && SERIAL_8250 > > What is the purpose of the OF dependency? > > I thought it was needed for dts support. I'll remove it. It's a runtime dependency and not a compile time, right? So, there is no impediment to compile test on the configurations that have CONFIG_OF=n. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v4 0/2] Airoha UART support 2025-02-09 21:02 [PATCH v4 0/2] Airoha UART support Benjamin Larsson 2025-02-09 21:02 ` [PATCH 1/2] dt-bindings: serial: 8250: Add Airoha compatibles Benjamin Larsson 2025-02-09 21:02 ` [PATCH 2/2] serial: Airoha SoC UART and HSUART support Benjamin Larsson @ 2025-02-10 6:14 ` Greg KH 2025-03-21 20:37 ` Benjamin Larsson 2 siblings, 1 reply; 11+ messages in thread From: Greg KH @ 2025-02-10 6:14 UTC (permalink / raw) To: Benjamin Larsson; +Cc: linux-serial, devicetree, ansuelsmth, lorenzo On Sun, Feb 09, 2025 at 10:02:39PM +0100, Benjamin Larsson wrote: > The Airoha familty of SoCs have a UART hardware that is 16550-compatible > with the exception of the baud rate settings. > > This patch implements code for calculating the baud rate for the Airoha > UART and HSUART. > > Signed-off-by: Benjamin Larsson <benjamin.larsson@genexis.eu> > --- > Changes in v4: > - Removed COMPILE_TEST from Kconfig No, please don't do that. > - Removed Kconfig option to build as module No do that, you want your code as a module so that it can work in a system that is built as a "generic system image" that does not force your driver to be built into the main kernel image, wasting memory if the hardware is not present. thanks, greg k-h ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v4 0/2] Airoha UART support 2025-02-10 6:14 ` [PATCH v4 0/2] Airoha UART support Greg KH @ 2025-03-21 20:37 ` Benjamin Larsson 2025-03-22 21:45 ` Andy Shevchenko 0 siblings, 1 reply; 11+ messages in thread From: Benjamin Larsson @ 2025-03-21 20:37 UTC (permalink / raw) To: Greg KH; +Cc: linux-serial, devicetree, ansuelsmth, lorenzo Hi, I seem to have missed that you commented on the patch. On 10/02/2025 07:14, Greg KH wrote: > On Sun, Feb 09, 2025 at 10:02:39PM +0100, Benjamin Larsson wrote: >> The Airoha familty of SoCs have a UART hardware that is 16550-compatible >> with the exception of the baud rate settings. >> >> This patch implements code for calculating the baud rate for the Airoha >> UART and HSUART. >> >> Signed-off-by: Benjamin Larsson <benjamin.larsson@genexis.eu> >> --- >> Changes in v4: >> - Removed COMPILE_TEST from Kconfig > > No, please don't do that. > >> - Removed Kconfig option to build as module > > No do that, you want your code as a module so that it can work in a > system that is built as a "generic system image" that does not force > your driver to be built into the main kernel image, wasting memory if > the hardware is not present. > > thanks, > > greg k-h I would argue that I follow the current flow of the code. In 8250.h we have: CONFIG_SERIAL_8250_PNP CONFIG_SERIAL_8250_RSA CONFIG_SERIAL_8250_FINTEK none of those enables COMPILE_TEST or the option to compile as a module. Neither the Airoha code or the other code is not intended to be its own separate module, it is to be part of the 8250-driver. The 8250-driver can be loaded as a module with or without the Airoha baud rate code. Implementing COMPILE_TEST when the 8250-driver does not support it seems tricky. All the ways I could think of would result in messy code and logic. I came to the conclusion that a smaller patch that reuses the current logic was preferable. If that argument is not good enough then I need some guidance how to implement something what would be accepted. MvH Benjamin Larsson ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v4 0/2] Airoha UART support 2025-03-21 20:37 ` Benjamin Larsson @ 2025-03-22 21:45 ` Andy Shevchenko 2025-03-25 14:45 ` Benjamin Larsson 0 siblings, 1 reply; 11+ messages in thread From: Andy Shevchenko @ 2025-03-22 21:45 UTC (permalink / raw) To: Benjamin Larsson; +Cc: Greg KH, linux-serial, devicetree, ansuelsmth, lorenzo Fri, Mar 21, 2025 at 09:37:09PM +0100, Benjamin Larsson kirjoitti: > On 10/02/2025 07:14, Greg KH wrote: > > On Sun, Feb 09, 2025 at 10:02:39PM +0100, Benjamin Larsson wrote: ... > I would argue that I follow the current flow of the code. In 8250.h we have: > > CONFIG_SERIAL_8250_PNP > CONFIG_SERIAL_8250_RSA These are historically parts of the main driver, RSA code theoretically can be removed. > CONFIG_SERIAL_8250_FINTEK I would love to see this being not part of main driver. > none of those enables COMPILE_TEST or the option to compile as a module. They all together may be compiled as a main driver module. Again, this is all historical and new code would need a very good justification why it can be held in a separate module. > Neither the Airoha code or the other code is not intended to be its own > separate module, Why not? > it is to be part of the 8250-driver. The 8250-driver can be > loaded as a module with or without the Airoha baud rate code. > > Implementing COMPILE_TEST when the 8250-driver does not support it seems > tricky. All the ways I could think of would result in messy code and logic. > I came to the conclusion that a smaller patch that reuses the current logic > was preferable. If that argument is not good enough then I need some > guidance how to implement something what would be accepted. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v4 0/2] Airoha UART support 2025-03-22 21:45 ` Andy Shevchenko @ 2025-03-25 14:45 ` Benjamin Larsson 2025-03-25 14:54 ` Andy Shevchenko 0 siblings, 1 reply; 11+ messages in thread From: Benjamin Larsson @ 2025-03-25 14:45 UTC (permalink / raw) To: Andy Shevchenko; +Cc: Greg KH, linux-serial, devicetree, ansuelsmth, lorenzo On 2025-03-22 22:45, Andy Shevchenko wrote: > [You don't often get email from andy.shevchenko@gmail.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ] > > Fri, Mar 21, 2025 at 09:37:09PM +0100, Benjamin Larsson kirjoitti: >> On 10/02/2025 07:14, Greg KH wrote: >>> On Sun, Feb 09, 2025 at 10:02:39PM +0100, Benjamin Larsson wrote: > ... > >> I would argue that I follow the current flow of the code. In 8250.h we have: >> >> CONFIG_SERIAL_8250_PNP >> CONFIG_SERIAL_8250_RSA > These are historically parts of the main driver, RSA code theoretically > can be removed. > >> CONFIG_SERIAL_8250_FINTEK > I would love to see this being not part of main driver. > >> none of those enables COMPILE_TEST or the option to compile as a module. > They all together may be compiled as a main driver module. > Again, this is all historical and new code would need a very good justification > why it can be held in a separate module. Hmm, ok I'll see if I understand you. I should create 8250_airoha.c by using 8250_fsl.c as a template. Adapt probe for Airoha by replacing .set_termios with a wrapper that calls the custom baud rate calculation code? > >> Neither the Airoha code or the other code is not intended to be its own >> separate module, > Why not? Well I mean that was my intent. > >> it is to be part of the 8250-driver. The 8250-driver can be >> loaded as a module with or without the Airoha baud rate code. >> >> Implementing COMPILE_TEST when the 8250-driver does not support it seems >> tricky. All the ways I could think of would result in messy code and logic. >> I came to the conclusion that a smaller patch that reuses the current logic >> was preferable. If that argument is not good enough then I need some >> guidance how to implement something what would be accepted. > -- > With Best Regards, > Andy Shevchenko > > MvH Benjamin Larsson ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v4 0/2] Airoha UART support 2025-03-25 14:45 ` Benjamin Larsson @ 2025-03-25 14:54 ` Andy Shevchenko 0 siblings, 0 replies; 11+ messages in thread From: Andy Shevchenko @ 2025-03-25 14:54 UTC (permalink / raw) To: Benjamin Larsson; +Cc: Greg KH, linux-serial, devicetree, ansuelsmth, lorenzo On Tue, Mar 25, 2025 at 4:45 PM Benjamin Larsson <benjamin.larsson@genexis.eu> wrote: > On 2025-03-22 22:45, Andy Shevchenko wrote: > > [You don't often get email from andy.shevchenko@gmail.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ] > > Fri, Mar 21, 2025 at 09:37:09PM +0100, Benjamin Larsson kirjoitti: > >> On 10/02/2025 07:14, Greg KH wrote: > >>> On Sun, Feb 09, 2025 at 10:02:39PM +0100, Benjamin Larsson wrote: ... > >> I would argue that I follow the current flow of the code. In 8250.h we have: > >> > >> CONFIG_SERIAL_8250_PNP > >> CONFIG_SERIAL_8250_RSA > > These are historically parts of the main driver, RSA code theoretically > > can be removed. > > > >> CONFIG_SERIAL_8250_FINTEK > > I would love to see this being not part of main driver. > > > >> none of those enables COMPILE_TEST or the option to compile as a module. > > They all together may be compiled as a main driver module. > > Again, this is all historical and new code would need a very good justification > > why it can be held in a separate module. > > Hmm, ok I'll see if I understand you. I should create 8250_airoha.c by > using 8250_fsl.c as a template. Adapt probe for Airoha by replacing > .set_termios with a wrapper that calls the custom baud rate calculation > code? Not sure that _fsl is the best example (we have so far: 8250_exar, 8250_lpss, 8250_mid, ...), but overall yes, that's my expectation. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2025-03-25 14:55 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-02-09 21:02 [PATCH v4 0/2] Airoha UART support Benjamin Larsson 2025-02-09 21:02 ` [PATCH 1/2] dt-bindings: serial: 8250: Add Airoha compatibles Benjamin Larsson 2025-02-09 21:02 ` [PATCH 2/2] serial: Airoha SoC UART and HSUART support Benjamin Larsson 2025-03-22 21:42 ` Andy Shevchenko 2025-03-25 14:19 ` Benjamin Larsson 2025-03-25 14:23 ` Andy Shevchenko 2025-02-10 6:14 ` [PATCH v4 0/2] Airoha UART support Greg KH 2025-03-21 20:37 ` Benjamin Larsson 2025-03-22 21:45 ` Andy Shevchenko 2025-03-25 14:45 ` Benjamin Larsson 2025-03-25 14:54 ` Andy Shevchenko
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox