In 8250_fintek.c probe_setup_port(), we'll detect the IRQ trigger mode by
irq_get_irq_data() and pass it to fintek_8250_set_irq_mode(). If detected
Edge mode, we'll set the UART with Edge/High mode, otherwise Level/Low.
But in some motherboard, The APIC maybe setting to Level/High. In this case
the driver will setting wrong configuration into UART. So we add a option
to kernel parameter to control the driver as following:
fintek_uart_irq_mode_override= [SERIAL]
{default, bios}
If the parameter is "default", the driver will using
former IRQ override methed(By IRQ trigger type).
otherwise, we'll don't change the UART IRQ setting.
Signed-off-by: Ji-Ze Hong (Peter Hong) <[email protected]>
---
drivers/tty/serial/8250/8250_fintek.c | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/drivers/tty/serial/8250/8250_fintek.c b/drivers/tty/serial/8250/8250_fintek.c
index dba5950b8d0e..c5fea0a7c79b 100644
--- a/drivers/tty/serial/8250/8250_fintek.c
+++ b/drivers/tty/serial/8250/8250_fintek.c
@@ -92,6 +92,9 @@
#define F81866_UART_CLK_18_432MHZ BIT(0)
#define F81866_UART_CLK_24MHZ BIT(1)
+#define FINTEK_IRQ_MODE_BY_DETECT 0
+#define FINTEK_IRQ_MODE_BY_BIOS 1
+
struct fintek_8250 {
u16 pid;
u16 base_port;
@@ -99,6 +102,24 @@ struct fintek_8250 {
u8 key;
};
+static int not_override_irq_mode = FINTEK_IRQ_MODE_BY_DETECT;
+
+static int __init parse_uart_irq_mode_override(char *arg)
+{
+ if (!arg)
+ return -EINVAL;
+
+ if (strcmp(arg, "bios") == 0)
+ not_override_irq_mode = FINTEK_IRQ_MODE_BY_BIOS;
+ else if (strcmp(arg, "default") == 0)
+ not_override_irq_mode = FINTEK_IRQ_MODE_BY_DETECT;
+ else
+ return -EINVAL;
+
+ return 0;
+}
+early_param("fintek_uart_irq_mode_override", parse_uart_irq_mode_override);
+
static u8 sio_read_reg(struct fintek_8250 *pdata, u8 reg)
{
outb(reg, pdata->base_port + ADDR_PORT);
@@ -248,6 +269,12 @@ static int fintek_8250_rs485_config(struct uart_port *port,
static void fintek_8250_set_irq_mode(struct fintek_8250 *pdata, bool is_level)
{
+ if (not_override_irq_mode == FINTEK_IRQ_MODE_BY_BIOS) {
+ pr_info("Fintek UART(%04x) irq mode is using BIOS default",
+ pdata->pid);
+ return;
+ }
+
sio_write_reg(pdata, LDN, pdata->index);
switch (pdata->pid) {
--
2.17.1
On Fri, Feb 17, 2023 at 04:49:53PM +0800, Ji-Ze Hong (Peter Hong) wrote:
> In 8250_fintek.c probe_setup_port(), we'll detect the IRQ trigger mode by
> irq_get_irq_data() and pass it to fintek_8250_set_irq_mode(). If detected
> Edge mode, we'll set the UART with Edge/High mode, otherwise Level/Low.
>
> But in some motherboard, The APIC maybe setting to Level/High. In this case
> the driver will setting wrong configuration into UART. So we add a option
> to kernel parameter to control the driver as following:
>
> fintek_uart_irq_mode_override= [SERIAL]
> {default, bios}
> If the parameter is "default", the driver will using
> former IRQ override methed(By IRQ trigger type).
> otherwise, we'll don't change the UART IRQ setting.
>
> Signed-off-by: Ji-Ze Hong (Peter Hong) <[email protected]>
> ---
> drivers/tty/serial/8250/8250_fintek.c | 27 +++++++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
>
> diff --git a/drivers/tty/serial/8250/8250_fintek.c b/drivers/tty/serial/8250/8250_fintek.c
> index dba5950b8d0e..c5fea0a7c79b 100644
> --- a/drivers/tty/serial/8250/8250_fintek.c
> +++ b/drivers/tty/serial/8250/8250_fintek.c
> @@ -92,6 +92,9 @@
> #define F81866_UART_CLK_18_432MHZ BIT(0)
> #define F81866_UART_CLK_24MHZ BIT(1)
>
> +#define FINTEK_IRQ_MODE_BY_DETECT 0
> +#define FINTEK_IRQ_MODE_BY_BIOS 1
> +
> struct fintek_8250 {
> u16 pid;
> u16 base_port;
> @@ -99,6 +102,24 @@ struct fintek_8250 {
> u8 key;
> };
>
> +static int not_override_irq_mode = FINTEK_IRQ_MODE_BY_DETECT;
> +
> +static int __init parse_uart_irq_mode_override(char *arg)
> +{
> + if (!arg)
> + return -EINVAL;
> +
> + if (strcmp(arg, "bios") == 0)
> + not_override_irq_mode = FINTEK_IRQ_MODE_BY_BIOS;
> + else if (strcmp(arg, "default") == 0)
> + not_override_irq_mode = FINTEK_IRQ_MODE_BY_DETECT;
> + else
> + return -EINVAL;
> +
> + return 0;
> +}
> +early_param("fintek_uart_irq_mode_override", parse_uart_irq_mode_override);
Sorry, but no, this will not work with multiple devices. Please fix the
bios to handle this all properly, do not require users to manually fix
up problems for you (i.e. fix it once, not in thousands of individual
systems.)
> +
> static u8 sio_read_reg(struct fintek_8250 *pdata, u8 reg)
> {
> outb(reg, pdata->base_port + ADDR_PORT);
> @@ -248,6 +269,12 @@ static int fintek_8250_rs485_config(struct uart_port *port,
>
> static void fintek_8250_set_irq_mode(struct fintek_8250 *pdata, bool is_level)
> {
> + if (not_override_irq_mode == FINTEK_IRQ_MODE_BY_BIOS) {
> + pr_info("Fintek UART(%04x) irq mode is using BIOS default",
> + pdata->pid);
Note, this is a driver, always use dev_*() calls, never pr_*() calls.
thanks,
greg k-h
On Fri, Feb 17, 2023 at 04:49:53PM +0800, Ji-Ze Hong (Peter Hong) wrote:
> In 8250_fintek.c probe_setup_port(), we'll detect the IRQ trigger mode by
> irq_get_irq_data() and pass it to fintek_8250_set_irq_mode(). If detected
> Edge mode, we'll set the UART with Edge/High mode, otherwise Level/Low.
>
> But in some motherboard, The APIC maybe setting to Level/High. In this case
> the driver will setting wrong configuration into UART. So we add a option
> to kernel parameter to control the driver as following:
>
> fintek_uart_irq_mode_override= [SERIAL]
> {default, bios}
> If the parameter is "default", the driver will using
> former IRQ override methed(By IRQ trigger type).
> otherwise, we'll don't change the UART IRQ setting.
>
> Signed-off-by: Ji-Ze Hong (Peter Hong) <[email protected]>
Also note, your From: line does not match this line, so I couldn't take
this patch anyway :(
And why are you sending patches through a random gmail account and not
your fintek.com.tw address? Please use that one, so that we can easily
verify that this is coming from the proper address and no one is
impersonating you.
thanks,
greg k-h
Hi Ji-Ze,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on tty/tty-testing]
[also build test ERROR on tty/tty-next tty/tty-linus usb/usb-testing usb/usb-next usb/usb-linus linus/master v6.2-rc8]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Ji-Ze-Hong-Peter-Hong/serial-8250_fintek-Add-using-BIOS-IRQ-default-setting/20230217-165155
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tty-testing
patch link: https://lore.kernel.org/r/20230217084953.2580-1-hpeter%2Blinux_kernel%40gmail.com
patch subject: [PATCH] serial: 8250_fintek: Add using BIOS IRQ default setting
config: powerpc-allmodconfig (https://download.01.org/0day-ci/archive/20230217/[email protected]/config)
compiler: powerpc-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/fd786fba8247c675fb90d00d076235cbd85842e6
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Ji-Ze-Hong-Peter-Hong/serial-8250_fintek-Add-using-BIOS-IRQ-default-setting/20230217-165155
git checkout fd786fba8247c675fb90d00d076235cbd85842e6
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=powerpc olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=powerpc SHELL=/bin/bash drivers/tty/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
| Link: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All errors (new ones prefixed by >>):
>> drivers/tty/serial/8250/8250_fintek.c:121:13: error: expected declaration specifiers or '...' before string constant
121 | early_param("fintek_uart_irq_mode_override", parse_uart_irq_mode_override);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/tty/serial/8250/8250_fintek.c:121:46: error: expected declaration specifiers or '...' before 'parse_uart_irq_mode_override'
121 | early_param("fintek_uart_irq_mode_override", parse_uart_irq_mode_override);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/tty/serial/8250/8250_fintek.c:107:19: warning: 'parse_uart_irq_mode_override' defined but not used [-Wunused-function]
107 | static int __init parse_uart_irq_mode_override(char *arg)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
vim +121 drivers/tty/serial/8250/8250_fintek.c
106
107 static int __init parse_uart_irq_mode_override(char *arg)
108 {
109 if (!arg)
110 return -EINVAL;
111
112 if (strcmp(arg, "bios") == 0)
113 not_override_irq_mode = FINTEK_IRQ_MODE_BY_BIOS;
114 else if (strcmp(arg, "default") == 0)
115 not_override_irq_mode = FINTEK_IRQ_MODE_BY_DETECT;
116 else
117 return -EINVAL;
118
119 return 0;
120 }
> 121 early_param("fintek_uart_irq_mode_override", parse_uart_irq_mode_override);
122
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests