[PATCH] serial: 8250_mtk: Add ACPI support

[PATCH] serial: 8250_mtk: Add ACPI support

[Yenchia Chen]

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(-)

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,
-- 
2.45.2

Re: [PATCH] serial: 8250_mtk: Add ACPI support

[Jiri Slaby]

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,
-- 
js
suse labs

Re: [PATCH] serial: 8250_mtk: Add ACPI support

[Jiri Slaby]

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.


-- 
js
suse labs

Re: [PATCH] serial: 8250_mtk: Add ACPI support

[kernel test robot]

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	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH] serial: 8250_mtk: Add ACPI support

[kernel test robot]

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	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH] serial: 8250_mtk: Add ACPI support

[Yenchia Chen]

> 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.

Re: [PATCH] serial: 8250_mtk: Add ACPI support

[Jonathan Cameron]

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	
>