| Message ID | 20250114033324.3533292-1-yenchia.chen@mediatek.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | serial: 8250_mtk: Add ACPI support | expand |
On 14. 01. 25, 4:33, Yenchia Chen wrote: > Add ACPI support to 8250_mtk driver. This makes it possible to > use uart with edk2 UEFI firmware. Could you mention what hardware this is in particular? > Signed-off-by: Yenchia Chen <yenchia.chen@mediatek.com> > --- > drivers/tty/serial/8250/8250_mtk.c | 31 ++++++++++++++++++++++++------ > 1 file changed, 25 insertions(+), 6 deletions(-) > > diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c > index b44de2ed7413..a4f1c30f77b0 100644 > --- a/drivers/tty/serial/8250/8250_mtk.c > +++ b/drivers/tty/serial/8250/8250_mtk.c > @@ -19,6 +19,7 @@ > #include <linux/dma-mapping.h> > #include <linux/tty.h> > #include <linux/tty_flip.h> > +#include <linux/acpi.h> Sort this properly. > #include "8250.h" > > @@ -519,6 +520,7 @@ static int mtk8250_probe(struct platform_device *pdev) > struct mtk8250_data *data; > struct resource *regs; > int irq, err; > + acpi_handle hdl = ACPI_HANDLE(&pdev->dev); 'hdl' sounds very weird and is not used in the tree for acpi_handles. I'd use 'acpi_handle' or 'acpi_dev_handle' in this case. > irq = platform_get_irq(pdev, 0); > if (irq < 0) > @@ -545,8 +547,12 @@ static int mtk8250_probe(struct platform_device *pdev) > err = mtk8250_probe_of(pdev, &uart.port, data); > if (err) > return err; > - } else > - return -ENODEV; > + } else { > + if (!hdl) { so this should be: } else if () { > + dev_err(&pdev->dev, "no device\n"); Why this? > + return -ENODEV; As this is self explanatory already, right? > + } > + } > > spin_lock_init(&uart.port.lock); > uart.port.mapbase = regs->start; > @@ -560,16 +566,22 @@ static int mtk8250_probe(struct platform_device *pdev) > uart.port.private_data = data; > uart.port.shutdown = mtk8250_shutdown; > uart.port.startup = mtk8250_startup; > - uart.port.set_termios = mtk8250_set_termios; > - uart.port.uartclk = clk_get_rate(data->uart_clk); > + if (hdl) { > + uart.port.uartclk = 26000000; This is a magic constant. Define a macro for this. Hint: 26 * HZ_PER_MHZ. Is it not/cannot it be part of the acpi table? What does MTKI0511 look like? > + } else { > + uart.port.set_termios = mtk8250_set_termios; > + uart.port.uartclk = clk_get_rate(data->uart_clk); > + } > #ifdef CONFIG_SERIAL_8250_DMA > if (data->dma) > uart.dma = data->dma; > #endif > > - /* Disable Rate Fix function */ > - writel(0x0, uart.port.membase + > + if (!hdl) { > + /* Disable Rate Fix function */ Why is this only for non-ACPI devices? > + writel(0x0, uart.port.membase + > (MTK_UART_RATE_FIX << uart.port.regshift)); > + } > > platform_set_drvdata(pdev, data); > > @@ -647,11 +659,18 @@ static const struct of_device_id mtk8250_of_match[] = { > }; > MODULE_DEVICE_TABLE(of, mtk8250_of_match); > > +static const struct acpi_device_id mtk8250_acpi_match[] = { > + { "MTKI0511", 0 }, > + {}, > +}; > +MODULE_DEVICE_TABLE(acpi, mtk8250_acpi_match); > + > static struct platform_driver mtk8250_platform_driver = { > .driver = { > .name = "mt6577-uart", > .pm = &mtk8250_pm_ops, > .of_match_table = mtk8250_of_match, > + .acpi_match_table = ACPI_PTR(mtk8250_acpi_match), > }, > .probe = mtk8250_probe, > .remove = mtk8250_remove, thanks,
On 14. 01. 25, 8:31, Jiri Slaby wrote: >> @@ -560,16 +566,22 @@ static int mtk8250_probe(struct platform_device >> *pdev) >> uart.port.private_data = data; >> uart.port.shutdown = mtk8250_shutdown; >> uart.port.startup = mtk8250_startup; >> - uart.port.set_termios = mtk8250_set_termios; Wait, ::set_termios() is NOT optional.
Hi Yenchia, kernel test robot noticed the following build warnings: [auto build test WARNING on tty/tty-testing] [also build test WARNING on tty/tty-next tty/tty-linus usb/usb-testing usb/usb-next usb/usb-linus linus/master v6.13-rc7 next-20250115] [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/Yenchia-Chen/serial-8250_mtk-Add-ACPI-support/20250114-113715 base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tty-testing patch link: https://lore.kernel.org/r/20250114033324.3533292-1-yenchia.chen%40mediatek.com patch subject: [PATCH] serial: 8250_mtk: Add ACPI support config: sh-allyesconfig (https://download.01.org/0day-ci/archive/20250116/202501160328.DUMWkTOc-lkp@intel.com/config) compiler: sh4-linux-gcc (GCC) 14.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250116/202501160328.DUMWkTOc-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202501160328.DUMWkTOc-lkp@intel.com/ All warnings (new ones prefixed by >>): >> drivers/tty/serial/8250/8250_mtk.c:662:36: warning: 'mtk8250_acpi_match' defined but not used [-Wunused-const-variable=] 662 | static const struct acpi_device_id mtk8250_acpi_match[] = { | ^~~~~~~~~~~~~~~~~~ vim +/mtk8250_acpi_match +662 drivers/tty/serial/8250/8250_mtk.c 661 > 662 static const struct acpi_device_id mtk8250_acpi_match[] = { 663 { "MTKI0511", 0 }, 664 {}, 665 }; 666 MODULE_DEVICE_TABLE(acpi, mtk8250_acpi_match); 667
Hi Yenchia, kernel test robot noticed the following build warnings: [auto build test WARNING on tty/tty-testing] [also build test WARNING on tty/tty-next tty/tty-linus usb/usb-testing usb/usb-next usb/usb-linus linus/master v6.13-rc7 next-20250115] [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/Yenchia-Chen/serial-8250_mtk-Add-ACPI-support/20250114-113715 base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tty-testing patch link: https://lore.kernel.org/r/20250114033324.3533292-1-yenchia.chen%40mediatek.com patch subject: [PATCH] serial: 8250_mtk: Add ACPI support config: powerpc-randconfig-001-20250116 (https://download.01.org/0day-ci/archive/20250116/202501160444.iXv8byKW-lkp@intel.com/config) compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project f5cd181ffbb7cb61d582fe130d46580d5969d47a) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250116/202501160444.iXv8byKW-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202501160444.iXv8byKW-lkp@intel.com/ All warnings (new ones prefixed by >>): >> drivers/tty/serial/8250/8250_mtk.c:662:36: warning: unused variable 'mtk8250_acpi_match' [-Wunused-const-variable] 662 | static const struct acpi_device_id mtk8250_acpi_match[] = { | ^~~~~~~~~~~~~~~~~~ 1 warning generated. vim +/mtk8250_acpi_match +662 drivers/tty/serial/8250/8250_mtk.c 661 > 662 static const struct acpi_device_id mtk8250_acpi_match[] = { 663 { "MTKI0511", 0 }, 664 {}, 665 }; 666 MODULE_DEVICE_TABLE(acpi, mtk8250_acpi_match); 667
> Wait, ::set_termios() is NOT optional. Got it, will put it back in next version. > This is a magic constant. Define a macro for this. Hint: 26 * HZ_PER_MHZ. > Is it not/cannot it be part of the acpi table? What does MTKI0511 look like? Currently clock settings are not included in ACPI table, so using fixed clock here. We'd like to change the flow as shown below to remove the dependency on the ACPI device: uart.port.set_termios = mtk8250_set_termios; uart.port.uartclk = clk_get_rate(data->uart_clk); + if (!uart.port.uartclk) + uart.port.uartclk = 26 * HZ_PER_MZ; > Why is this only for non-ACPI devices? In ACPI devices, these registers are inaccessible due to some related settings not being configured.
On Thu, 16 Jan 2025 03:45:33 +0800 kernel test robot <lkp@intel.com> wrote: > Hi Yenchia, > > kernel test robot noticed the following build warnings: > > [auto build test WARNING on tty/tty-testing] > [also build test WARNING on tty/tty-next tty/tty-linus usb/usb-testing usb/usb-next usb/usb-linus linus/master v6.13-rc7 next-20250115] > [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/Yenchia-Chen/serial-8250_mtk-Add-ACPI-support/20250114-113715 > base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tty-testing > patch link: https://lore.kernel.org/r/20250114033324.3533292-1-yenchia.chen%40mediatek.com > patch subject: [PATCH] serial: 8250_mtk: Add ACPI support > config: sh-allyesconfig (https://download.01.org/0day-ci/archive/20250116/202501160328.DUMWkTOc-lkp@intel.com/config) > compiler: sh4-linux-gcc (GCC) 14.2.0 > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250116/202501160328.DUMWkTOc-lkp@intel.com/reproduce) > > If you fix the issue in a separate patch/commit (i.e. not just a new version of > the same patch/commit), kindly add following tags > | Reported-by: kernel test robot <lkp@intel.com> > | Closes: https://lore.kernel.org/oe-kbuild-all/202501160328.DUMWkTOc-lkp@intel.com/ > > All warnings (new ones prefixed by >>): > > >> drivers/tty/serial/8250/8250_mtk.c:662:36: warning: 'mtk8250_acpi_match' defined but not used [-Wunused-const-variable=] > 662 | static const struct acpi_device_id mtk8250_acpi_match[] = { > | ^~~~~~~~~~~~~~~~~~ > > > vim +/mtk8250_acpi_match +662 drivers/tty/serial/8250/8250_mtk.c > > 661 > > 662 static const struct acpi_device_id mtk8250_acpi_match[] = { > 663 { "MTKI0511", 0 }, > 664 {}, > 665 }; I'd drop the ACPI_PTR() use and just set it unconditionally. This table is tiny so not worth complexity of doing anything else. > 666 MODULE_DEVICE_TABLE(acpi, mtk8250_acpi_match); > 667 >
diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c index b44de2ed7413..a4f1c30f77b0 100644 --- a/drivers/tty/serial/8250/8250_mtk.c +++ b/drivers/tty/serial/8250/8250_mtk.c @@ -19,6 +19,7 @@ #include <linux/dma-mapping.h> #include <linux/tty.h> #include <linux/tty_flip.h> +#include <linux/acpi.h> #include "8250.h" @@ -519,6 +520,7 @@ static int mtk8250_probe(struct platform_device *pdev) struct mtk8250_data *data; struct resource *regs; int irq, err; + acpi_handle hdl = ACPI_HANDLE(&pdev->dev); irq = platform_get_irq(pdev, 0); if (irq < 0) @@ -545,8 +547,12 @@ static int mtk8250_probe(struct platform_device *pdev) err = mtk8250_probe_of(pdev, &uart.port, data); if (err) return err; - } else - return -ENODEV; + } else { + if (!hdl) { + dev_err(&pdev->dev, "no device\n"); + return -ENODEV; + } + } spin_lock_init(&uart.port.lock); uart.port.mapbase = regs->start; @@ -560,16 +566,22 @@ static int mtk8250_probe(struct platform_device *pdev) uart.port.private_data = data; uart.port.shutdown = mtk8250_shutdown; uart.port.startup = mtk8250_startup; - uart.port.set_termios = mtk8250_set_termios; - uart.port.uartclk = clk_get_rate(data->uart_clk); + if (hdl) { + uart.port.uartclk = 26000000; + } else { + uart.port.set_termios = mtk8250_set_termios; + uart.port.uartclk = clk_get_rate(data->uart_clk); + } #ifdef CONFIG_SERIAL_8250_DMA if (data->dma) uart.dma = data->dma; #endif - /* Disable Rate Fix function */ - writel(0x0, uart.port.membase + + if (!hdl) { + /* Disable Rate Fix function */ + writel(0x0, uart.port.membase + (MTK_UART_RATE_FIX << uart.port.regshift)); + } platform_set_drvdata(pdev, data); @@ -647,11 +659,18 @@ static const struct of_device_id mtk8250_of_match[] = { }; MODULE_DEVICE_TABLE(of, mtk8250_of_match); +static const struct acpi_device_id mtk8250_acpi_match[] = { + { "MTKI0511", 0 }, + {}, +}; +MODULE_DEVICE_TABLE(acpi, mtk8250_acpi_match); + static struct platform_driver mtk8250_platform_driver = { .driver = { .name = "mt6577-uart", .pm = &mtk8250_pm_ops, .of_match_table = mtk8250_of_match, + .acpi_match_table = ACPI_PTR(mtk8250_acpi_match), }, .probe = mtk8250_probe, .remove = mtk8250_remove,
Add ACPI support to 8250_mtk driver. This makes it possible to use uart with edk2 UEFI firmware. Signed-off-by: Yenchia Chen <yenchia.chen@mediatek.com> --- drivers/tty/serial/8250/8250_mtk.c | 31 ++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-)