[v3] add NXP LINFlexD UART clock support for S32G2/S32G3

[PATCH v3 2/2] serial: fsl_linflexuart: add clock support

Posted by Ciprian Costea 2 weeks, 2 days ago

From: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>

Add clocking support to the NXP LINFlexD UART driver.
It is used by S32G2 and S32G3 SoCs.
Clocking support is added as optional in order to not break
existing support for S32V234 SoC.

Signed-off-by: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>
---
 drivers/tty/serial/fsl_linflexuart.c | 67 +++++++++++++++++++++-------
 1 file changed, 51 insertions(+), 16 deletions(-)

diff --git a/drivers/tty/serial/fsl_linflexuart.c b/drivers/tty/serial/fsl_linflexuart.c
index e972df4b188d..23aed3bbff6c 100644
--- a/drivers/tty/serial/fsl_linflexuart.c
+++ b/drivers/tty/serial/fsl_linflexuart.c
@@ -3,9 +3,10 @@
  * Freescale LINFlexD UART serial port driver
  *
  * Copyright 2012-2016 Freescale Semiconductor, Inc.
- * Copyright 2017-2019 NXP
+ * Copyright 2017-2019, 2024 NXP
  */
 
+#include <linux/clk.h>
 #include <linux/console.h>
 #include <linux/io.h>
 #include <linux/irq.h>
@@ -120,6 +121,12 @@
 
 #define PREINIT_DELAY			2000 /* us */
 
+struct linflex_port {
+	struct uart_port port;
+	struct clk *clk_lin;
+	struct clk *clk_ipg;
+};
+
 static const struct of_device_id linflex_dt_ids[] = {
 	{
 		.compatible = "fsl,s32v234-linflexuart",
@@ -807,12 +814,13 @@ static struct uart_driver linflex_reg = {
 static int linflex_probe(struct platform_device *pdev)
 {
 	struct device_node *np = pdev->dev.of_node;
+	struct linflex_port *lfport;
 	struct uart_port *sport;
 	struct resource *res;
 	int ret;
 
-	sport = devm_kzalloc(&pdev->dev, sizeof(*sport), GFP_KERNEL);
-	if (!sport)
+	lfport = devm_kzalloc(&pdev->dev, sizeof(*lfport), GFP_KERNEL);
+	if (!lfport)
 		return -ENOMEM;
 
 	ret = of_alias_get_id(np, "serial");
@@ -826,6 +834,7 @@ static int linflex_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
+	sport = &lfport->port;
 	sport->line = ret;
 
 	sport->membase = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
@@ -844,39 +853,65 @@ static int linflex_probe(struct platform_device *pdev)
 	sport->flags = UPF_BOOT_AUTOCONF;
 	sport->has_sysrq = IS_ENABLED(CONFIG_SERIAL_FSL_LINFLEXUART_CONSOLE);
 
-	linflex_ports[sport->line] = sport;
+	lfport->clk_lin = devm_clk_get_optional_enabled(&pdev->dev, "lin");
+	if (IS_ERR(lfport->clk_lin))
+		return dev_err_probe(&pdev->dev, PTR_ERR(lfport->clk_lin),
+				"Failed to get linflexuart clk\n");
 
-	platform_set_drvdata(pdev, sport);
+	lfport->clk_ipg = devm_clk_get_optional_enabled(&pdev->dev, "ipg");
+	if (IS_ERR(lfport->clk_ipg))
+		return dev_err_probe(&pdev->dev, PTR_ERR(lfport->clk_ipg),
+				"Failed to get linflexuart ipg clk\n");
+
+	linflex_ports[sport->line] = sport;
+	platform_set_drvdata(pdev, lfport);
 
 	return uart_add_one_port(&linflex_reg, sport);
 }
 
 static void linflex_remove(struct platform_device *pdev)
 {
-	struct uart_port *sport = platform_get_drvdata(pdev);
+	struct linflex_port *lfport = platform_get_drvdata(pdev);
 
-	uart_remove_one_port(&linflex_reg, sport);
+	uart_remove_one_port(&linflex_reg, &lfport->port);
 }
 
-#ifdef CONFIG_PM_SLEEP
-static int linflex_suspend(struct device *dev)
+static int __maybe_unused linflex_suspend(struct device *dev)
 {
-	struct uart_port *sport = dev_get_drvdata(dev);
+	struct linflex_port *lfport = dev_get_drvdata(dev);
+
+	uart_suspend_port(&linflex_reg, &lfport->port);
 
-	uart_suspend_port(&linflex_reg, sport);
+	clk_disable_unprepare(lfport->clk_lin);
+	clk_disable_unprepare(lfport->clk_ipg);
 
 	return 0;
 }
 
-static int linflex_resume(struct device *dev)
+static int __maybe_unused linflex_resume(struct device *dev)
 {
-	struct uart_port *sport = dev_get_drvdata(dev);
+	struct linflex_port *lfport = dev_get_drvdata(dev);
+	int ret;
 
-	uart_resume_port(&linflex_reg, sport);
+	if (lfport->clk_lin) {
+		ret = clk_prepare_enable(lfport->clk_lin);
+		if (ret) {
+			dev_err(dev, "Failed to enable linflexuart clk: %d\n", ret);
+			return ret;
+		}
+	}
 
-	return 0;
+	if (lfport->clk_ipg) {
+		ret = clk_prepare_enable(lfport->clk_ipg);
+		if (ret) {
+			dev_err(dev, "Failed to enable linflexuart ipg clk: %d\n", ret);
+			clk_disable_unprepare(lfport->clk_lin);
+			return ret;
+		}
+	}
+
+	return uart_resume_port(&linflex_reg, &lfport->port);
 }
-#endif
 
 static SIMPLE_DEV_PM_OPS(linflex_pm_ops, linflex_suspend, linflex_resume);
 
-- 
2.45.2

Re: [PATCH v3 2/2] serial: fsl_linflexuart: add clock support

Posted by Frank Li 2 weeks, 2 days ago

On Thu, Nov 07, 2024 at 01:46:11PM +0200, Ciprian Costea wrote:
> From: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>
>
> Add clocking support to the NXP LINFlexD UART driver.
> It is used by S32G2 and S32G3 SoCs.
> Clocking support is added as optional in order to not break
> existing support for S32V234 SoC.

wrap at 75 char.

Add optional clock 'lin' and 'ipg' support to NXP LINFlexD UART driver,
which used by S22G2 and S32G3.

>
> Signed-off-by: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>
> ---
>  drivers/tty/serial/fsl_linflexuart.c | 67 +++++++++++++++++++++-------
>  1 file changed, 51 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/tty/serial/fsl_linflexuart.c b/drivers/tty/serial/fsl_linflexuart.c
> index e972df4b188d..23aed3bbff6c 100644
> --- a/drivers/tty/serial/fsl_linflexuart.c
> +++ b/drivers/tty/serial/fsl_linflexuart.c
> @@ -3,9 +3,10 @@
>   * Freescale LINFlexD UART serial port driver
>   *
>   * Copyright 2012-2016 Freescale Semiconductor, Inc.
> - * Copyright 2017-2019 NXP
> + * Copyright 2017-2019, 2024 NXP
>   */
>
> +#include <linux/clk.h>
>  #include <linux/console.h>
>  #include <linux/io.h>
>  #include <linux/irq.h>
> @@ -120,6 +121,12 @@
>
>  #define PREINIT_DELAY			2000 /* us */
>
> +struct linflex_port {
> +	struct uart_port port;
> +	struct clk *clk_lin;
> +	struct clk *clk_ipg;

Please clk bulk

> +};
> +
>  static const struct of_device_id linflex_dt_ids[] = {
>  	{
>  		.compatible = "fsl,s32v234-linflexuart",
> @@ -807,12 +814,13 @@ static struct uart_driver linflex_reg = {
>  static int linflex_probe(struct platform_device *pdev)
>  {
>  	struct device_node *np = pdev->dev.of_node;
> +	struct linflex_port *lfport;
>  	struct uart_port *sport;
>  	struct resource *res;
>  	int ret;
>
> -	sport = devm_kzalloc(&pdev->dev, sizeof(*sport), GFP_KERNEL);
> -	if (!sport)
> +	lfport = devm_kzalloc(&pdev->dev, sizeof(*lfport), GFP_KERNEL);
> +	if (!lfport)
>  		return -ENOMEM;
>
>  	ret = of_alias_get_id(np, "serial");
> @@ -826,6 +834,7 @@ static int linflex_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  	}
>
> +	sport = &lfport->port;
>  	sport->line = ret;
>
>  	sport->membase = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
> @@ -844,39 +853,65 @@ static int linflex_probe(struct platform_device *pdev)
>  	sport->flags = UPF_BOOT_AUTOCONF;
>  	sport->has_sysrq = IS_ENABLED(CONFIG_SERIAL_FSL_LINFLEXUART_CONSOLE);
>
> -	linflex_ports[sport->line] = sport;
> +	lfport->clk_lin = devm_clk_get_optional_enabled(&pdev->dev, "lin");
> +	if (IS_ERR(lfport->clk_lin))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(lfport->clk_lin),
> +				"Failed to get linflexuart clk\n");
>
> -	platform_set_drvdata(pdev, sport);
> +	lfport->clk_ipg = devm_clk_get_optional_enabled(&pdev->dev, "ipg");
> +	if (IS_ERR(lfport->clk_ipg))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(lfport->clk_ipg),
> +				"Failed to get linflexuart ipg clk\n");
> +

use clk bulk API to simple code.

> +	linflex_ports[sport->line] = sport;
> +	platform_set_drvdata(pdev, lfport);
>
>  	return uart_add_one_port(&linflex_reg, sport);
>  }
>
>  static void linflex_remove(struct platform_device *pdev)
>  {
> -	struct uart_port *sport = platform_get_drvdata(pdev);
> +	struct linflex_port *lfport = platform_get_drvdata(pdev);
>
> -	uart_remove_one_port(&linflex_reg, sport);
> +	uart_remove_one_port(&linflex_reg, &lfport->port);
>  }
>
> -#ifdef CONFIG_PM_SLEEP

Not related change. If change CONFIG_PM, please use sperate patch.
Please ref
https://lore.kernel.org/imx/1713848917-13380-1-git-send-email-shengjiu.wang@nxp.com/

> -static int linflex_suspend(struct device *dev)
> +static int __maybe_unused linflex_suspend(struct device *dev)
>  {
> -	struct uart_port *sport = dev_get_drvdata(dev);
> +	struct linflex_port *lfport = dev_get_drvdata(dev);
> +
> +	uart_suspend_port(&linflex_reg, &lfport->port);
>
> -	uart_suspend_port(&linflex_reg, sport);
> +	clk_disable_unprepare(lfport->clk_lin);
> +	clk_disable_unprepare(lfport->clk_ipg);
>
>  	return 0;
>  }
>
> -static int linflex_resume(struct device *dev)
> +static int __maybe_unused linflex_resume(struct device *dev)
>  {
> -	struct uart_port *sport = dev_get_drvdata(dev);
> +	struct linflex_port *lfport = dev_get_drvdata(dev);
> +	int ret;
>
> -	uart_resume_port(&linflex_reg, sport);
> +	if (lfport->clk_lin) {
> +		ret = clk_prepare_enable(lfport->clk_lin);
> +		if (ret) {
> +			dev_err(dev, "Failed to enable linflexuart clk: %d\n", ret);
> +			return ret;
> +		}
> +	}
>
> -	return 0;
> +	if (lfport->clk_ipg) {
> +		ret = clk_prepare_enable(lfport->clk_ipg);
> +		if (ret) {
> +			dev_err(dev, "Failed to enable linflexuart ipg clk: %d\n", ret);
> +			clk_disable_unprepare(lfport->clk_lin);
> +			return ret;
> +		}
> +	}

use clk bulk api.

> +
> +	return uart_resume_port(&linflex_reg, &lfport->port);
>  }
> -#endif
>
>  static SIMPLE_DEV_PM_OPS(linflex_pm_ops, linflex_suspend, linflex_resume);
>
> --
> 2.45.2
>

Re: [PATCH v3 2/2] serial: fsl_linflexuart: add clock support

Posted by Ciprian Marian Costea 2 weeks, 2 days ago

On 11/7/2024 5:19 PM, Frank Li wrote:

Hello Frank,

Thank your for your review.

> On Thu, Nov 07, 2024 at 01:46:11PM +0200, Ciprian Costea wrote:
>> From: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>
>>
>> Add clocking support to the NXP LINFlexD UART driver.
>> It is used by S32G2 and S32G3 SoCs.
>> Clocking support is added as optional in order to not break
>> existing support for S32V234 SoC.
> 
> wrap at 75 char.
> 
> Add optional clock 'lin' and 'ipg' support to NXP LINFlexD UART driver,
> which used by S22G2 and S32G3.
>
I will address this in V4.

>>
>> Signed-off-by: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>
>> ---
>>   drivers/tty/serial/fsl_linflexuart.c | 67 +++++++++++++++++++++-------
>>   1 file changed, 51 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/tty/serial/fsl_linflexuart.c b/drivers/tty/serial/fsl_linflexuart.c
>> index e972df4b188d..23aed3bbff6c 100644
>> --- a/drivers/tty/serial/fsl_linflexuart.c
>> +++ b/drivers/tty/serial/fsl_linflexuart.c
>> @@ -3,9 +3,10 @@
>>    * Freescale LINFlexD UART serial port driver
>>    *
>>    * Copyright 2012-2016 Freescale Semiconductor, Inc.
>> - * Copyright 2017-2019 NXP
>> + * Copyright 2017-2019, 2024 NXP
>>    */
>>
>> +#include <linux/clk.h>
>>   #include <linux/console.h>
>>   #include <linux/io.h>
>>   #include <linux/irq.h>
>> @@ -120,6 +121,12 @@
>>
>>   #define PREINIT_DELAY			2000 /* us */
>>
>> +struct linflex_port {
>> +	struct uart_port port;
>> +	struct clk *clk_lin;
>> +	struct clk *clk_ipg;
> 
> Please clk bulk
> 

Thank you for this suggestion. I will switch to 'clk bulk' api usage in V4.

>> +};
>> +
>>   static const struct of_device_id linflex_dt_ids[] = {
>>   	{
>>   		.compatible = "fsl,s32v234-linflexuart",
>> @@ -807,12 +814,13 @@ static struct uart_driver linflex_reg = {
>>   static int linflex_probe(struct platform_device *pdev)
>>   {
>>   	struct device_node *np = pdev->dev.of_node;
>> +	struct linflex_port *lfport;
>>   	struct uart_port *sport;
>>   	struct resource *res;
>>   	int ret;
>>
>> -	sport = devm_kzalloc(&pdev->dev, sizeof(*sport), GFP_KERNEL);
>> -	if (!sport)
>> +	lfport = devm_kzalloc(&pdev->dev, sizeof(*lfport), GFP_KERNEL);
>> +	if (!lfport)
>>   		return -ENOMEM;
>>
>>   	ret = of_alias_get_id(np, "serial");
>> @@ -826,6 +834,7 @@ static int linflex_probe(struct platform_device *pdev)
>>   		return -ENOMEM;
>>   	}
>>
>> +	sport = &lfport->port;
>>   	sport->line = ret;
>>
>>   	sport->membase = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
>> @@ -844,39 +853,65 @@ static int linflex_probe(struct platform_device *pdev)
>>   	sport->flags = UPF_BOOT_AUTOCONF;
>>   	sport->has_sysrq = IS_ENABLED(CONFIG_SERIAL_FSL_LINFLEXUART_CONSOLE);
>>
>> -	linflex_ports[sport->line] = sport;
>> +	lfport->clk_lin = devm_clk_get_optional_enabled(&pdev->dev, "lin");
>> +	if (IS_ERR(lfport->clk_lin))
>> +		return dev_err_probe(&pdev->dev, PTR_ERR(lfport->clk_lin),
>> +				"Failed to get linflexuart clk\n");
>>
>> -	platform_set_drvdata(pdev, sport);
>> +	lfport->clk_ipg = devm_clk_get_optional_enabled(&pdev->dev, "ipg");
>> +	if (IS_ERR(lfport->clk_ipg))
>> +		return dev_err_probe(&pdev->dev, PTR_ERR(lfport->clk_ipg),
>> +				"Failed to get linflexuart ipg clk\n");
>> +
> 
> use clk bulk API to simple code. >

Thanks. I will switch to 'clk_bulk' in V4.

>> +	linflex_ports[sport->line] = sport;
>> +	platform_set_drvdata(pdev, lfport);
>>
>>   	return uart_add_one_port(&linflex_reg, sport);
>>   }
>>
>>   static void linflex_remove(struct platform_device *pdev)
>>   {
>> -	struct uart_port *sport = platform_get_drvdata(pdev);
>> +	struct linflex_port *lfport = platform_get_drvdata(pdev);
>>
>> -	uart_remove_one_port(&linflex_reg, sport);
>> +	uart_remove_one_port(&linflex_reg, &lfport->port);
>>   }
>>
>> -#ifdef CONFIG_PM_SLEEP
> 
> Not related change. If change CONFIG_PM, please use sperate patch.
> Please ref
> https://lore.kernel.org/imx/1713848917-13380-1-git-send-email-shengjiu.wang@nxp.com/
> 

Ok. I will consider not making any PM cosmetic changes/refactor in this 
patchset.

>> -static int linflex_suspend(struct device *dev)
>> +static int __maybe_unused linflex_suspend(struct device *dev)
>>   {
>> -	struct uart_port *sport = dev_get_drvdata(dev);
>> +	struct linflex_port *lfport = dev_get_drvdata(dev);
>> +
>> +	uart_suspend_port(&linflex_reg, &lfport->port);
>>
>> -	uart_suspend_port(&linflex_reg, sport);
>> +	clk_disable_unprepare(lfport->clk_lin);
>> +	clk_disable_unprepare(lfport->clk_ipg);
>>
>>   	return 0;
>>   }
>>
>> -static int linflex_resume(struct device *dev)
>> +static int __maybe_unused linflex_resume(struct device *dev)
>>   {
>> -	struct uart_port *sport = dev_get_drvdata(dev);
>> +	struct linflex_port *lfport = dev_get_drvdata(dev);
>> +	int ret;
>>
>> -	uart_resume_port(&linflex_reg, sport);
>> +	if (lfport->clk_lin) {
>> +		ret = clk_prepare_enable(lfport->clk_lin);
>> +		if (ret) {
>> +			dev_err(dev, "Failed to enable linflexuart clk: %d\n", ret);
>> +			return ret;
>> +		}
>> +	}
>>
>> -	return 0;
>> +	if (lfport->clk_ipg) {
>> +		ret = clk_prepare_enable(lfport->clk_ipg);
>> +		if (ret) {
>> +			dev_err(dev, "Failed to enable linflexuart ipg clk: %d\n", ret);
>> +			clk_disable_unprepare(lfport->clk_lin);
>> +			return ret;
>> +		}
>> +	}
> 
> use clk bulk api.

Yes, I will update accordingly in V4.

Best Regards,
Ciprian

> 
>> +
>> +	return uart_resume_port(&linflex_reg, &lfport->port);
>>   }
>> -#endif
>>
>>   static SIMPLE_DEV_PM_OPS(linflex_pm_ops, linflex_suspend, linflex_resume);
>>
>> --
>> 2.45.2
>>

[PATCH v3 1/2] dt-bindings: linflexuart: add clock definitions
[PATCH v3 2/2] serial: fsl_linflexuart: add clock support