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

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

[Ciprian Costea]

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

LINFlexD UART driver is used by S32 SoC family.
Add clocking support as optional in order to not break
existing support for S32V234 SoC.

A separate patch adding clock definitions to
the S32G2/S32G3 based boards devicetree will be sent
for review.

Changes in V3:
- Fixed an error reported by 'dt_bindings_check'

Changes in V2:
- Updated bindings by adding more information related to
required LINFlexD clocks

Ciprian Marian Costea (2):
  dt-bindings: linflexuart: add clock definitions
  serial: fsl_linflexuart: add clock support

 .../bindings/serial/fsl,s32-linflexuart.yaml  | 29 ++++++++
 drivers/tty/serial/fsl_linflexuart.c          | 67 ++++++++++++++-----
 2 files changed, 80 insertions(+), 16 deletions(-)

-- 
2.45.2

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

[Ciprian Costea]

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

Add clock definitions for NXP LINFlexD UART bindings
and update the binding examples with S32G2 node.

Signed-off-by: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>
---
 .../bindings/serial/fsl,s32-linflexuart.yaml  | 29 +++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
index 4171f524a928..2716a9cd6c22 100644
--- a/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
+++ b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
@@ -34,10 +34,24 @@ properties:
   interrupts:
     maxItems: 1
 
+  clocks:
+    items:
+      - description:
+          ipg clock drives the access to the LINFlexD
+          iomapped registers
+      - description: lin is the frequency of the baud clock
+
+  clock-names:
+    items:
+      - const: ipg
+      - const: lin
+
 required:
   - compatible
   - reg
   - interrupts
+  - clocks
+  - clock-names
 
 unevaluatedProperties: false
 
@@ -47,4 +61,19 @@ examples:
         compatible = "fsl,s32v234-linflexuart";
         reg = <0x40053000 0x1000>;
         interrupts = <0 59 4>;
+        clocks = <&clks 132>, <&clks 131>;
+        clock-names = "ipg", "lin";
+    };
+
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    serial@401c8000 {
+        compatible = "nxp,s32g2-linflexuart",
+                     "fsl,s32v234-linflexuart";
+        reg = <0x401c8000 0x3000>;
+        interrupts = <GIC_SPI 82 IRQ_TYPE_EDGE_RISING>;
+        clocks = <&clks 13>, <&clks 14>;
+        clock-names = "ipg", "lin";
     };
-- 
2.45.2

Re: [PATCH v3 1/2] dt-bindings: linflexuart: add clock definitions

[Krzysztof Kozlowski]

On 07/11/2024 12:46, Ciprian Costea wrote:
> From: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>
> 
> Add clock definitions for NXP LINFlexD UART bindings
> and update the binding examples with S32G2 node.

Why?

What you are doing we see easily from the diff. I do not see why. Your
commit msg must explain this.

> 
> Signed-off-by: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>

Please use subject prefixes matching the subsystem. You can get them for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching. For bindings, the preferred subjects are
explained here:
https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters

Missing serial prefix and final is fsl,s32-linflexuart, not linflexuart

> ---
>  .../bindings/serial/fsl,s32-linflexuart.yaml  | 29 +++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
> index 4171f524a928..2716a9cd6c22 100644
> --- a/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
> +++ b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
> @@ -34,10 +34,24 @@ properties:
>    interrupts:
>      maxItems: 1
>  
> +  clocks:
> +    items:
> +      - description:
> +          ipg clock drives the access to the LINFlexD
> +          iomapped registers
> +      - description: lin is the frequency of the baud clock
> +
> +  clock-names:
> +    items:
> +      - const: ipg
> +      - const: lin
> +
>  required:
>    - compatible
>    - reg
>    - interrupts
> +  - clocks
> +  - clock-names
>  
>  unevaluatedProperties: false
>  
> @@ -47,4 +61,19 @@ examples:
>          compatible = "fsl,s32v234-linflexuart";
>          reg = <0x40053000 0x1000>;
>          interrupts = <0 59 4>;
> +        clocks = <&clks 132>, <&clks 131>;
> +        clock-names = "ipg", "lin";
> +    };
> +
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    serial@401c8000 {
> +        compatible = "nxp,s32g2-linflexuart",
> +                     "fsl,s32v234-linflexuart";
> +        reg = <0x401c8000 0x3000>;
> +        interrupts = <GIC_SPI 82 IRQ_TYPE_EDGE_RISING>;
> +        clocks = <&clks 13>, <&clks 14>;

Nope, drop the example. Not explained why you need it and really no point.

Best regards,
Krzysztof

Re: [PATCH v3 1/2] dt-bindings: linflexuart: add clock definitions

[Ciprian Marian Costea]

On 11/7/2024 1:53 PM, Krzysztof Kozlowski wrote:
> On 07/11/2024 12:46, Ciprian Costea wrote:
>> From: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>
>>
>> Add clock definitions for NXP LINFlexD UART bindings
>> and update the binding examples with S32G2 node.
> 
> Why?
> 
> What you are doing we see easily from the diff. I do not see why. Your
> commit msg must explain this.
> 
>>
>> Signed-off-by: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>
> 
> Please use subject prefixes matching the subsystem. You can get them for
> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
> your patch is touching. For bindings, the preferred subjects are
> explained here:
> https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters
> 
> Missing serial prefix and final is fsl,s32-linflexuart, not linflexuart
> 

Hello Krzysztof,

Thank you for your remarks. I will try to address them/add more context 
for these changes in V4.

Regards,
Ciprian

>> ---
>>   .../bindings/serial/fsl,s32-linflexuart.yaml  | 29 +++++++++++++++++++
>>   1 file changed, 29 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
>> index 4171f524a928..2716a9cd6c22 100644
>> --- a/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
>> +++ b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
>> @@ -34,10 +34,24 @@ properties:
>>     interrupts:
>>       maxItems: 1
>>   
>> +  clocks:
>> +    items:
>> +      - description:
>> +          ipg clock drives the access to the LINFlexD
>> +          iomapped registers
>> +      - description: lin is the frequency of the baud clock
>> +
>> +  clock-names:
>> +    items:
>> +      - const: ipg
>> +      - const: lin
>> +
>>   required:
>>     - compatible
>>     - reg
>>     - interrupts
>> +  - clocks
>> +  - clock-names
>>   
>>   unevaluatedProperties: false
>>   
>> @@ -47,4 +61,19 @@ examples:
>>           compatible = "fsl,s32v234-linflexuart";
>>           reg = <0x40053000 0x1000>;
>>           interrupts = <0 59 4>;
>> +        clocks = <&clks 132>, <&clks 131>;
>> +        clock-names = "ipg", "lin";
>> +    };
>> +
>> +  - |
>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>> +    #include <dt-bindings/interrupt-controller/irq.h>
>> +
>> +    serial@401c8000 {
>> +        compatible = "nxp,s32g2-linflexuart",
>> +                     "fsl,s32v234-linflexuart";
>> +        reg = <0x401c8000 0x3000>;
>> +        interrupts = <GIC_SPI 82 IRQ_TYPE_EDGE_RISING>;
>> +        clocks = <&clks 13>, <&clks 14>;
> 
> Nope, drop the example. Not explained why you need it and really no point.
> 
> Best regards,
> Krzysztof
> 

Re: [PATCH v3 1/2] dt-bindings: linflexuart: add clock definitions

[Krzysztof Kozlowski]

On 07/11/2024 12:46, Ciprian Costea wrote:
>  
> +  clocks:
> +    items:
> +      - description:
> +          ipg clock drives the access to the LINFlexD
> +          iomapped registers
> +      - description: lin is the frequency of the baud clock
> +
> +  clock-names:
> +    items:
> +      - const: ipg
> +      - const: lin
> +
>  required:
>    - compatible
>    - reg
>    - interrupts
> +  - clocks
> +  - clock-names

Ah, and that's an ABI break. That's a NAK unless explained in commit msg
(including impact on users).

Best regards,
Krzysztof

Re: [PATCH v3 1/2] dt-bindings: linflexuart: add clock definitions

[Ciprian Marian Costea]

On 11/7/2024 1:53 PM, Krzysztof Kozlowski wrote:
> On 07/11/2024 12:46, Ciprian Costea wrote:
>>   
>> +  clocks:
>> +    items:
>> +      - description:
>> +          ipg clock drives the access to the LINFlexD
>> +          iomapped registers
>> +      - description: lin is the frequency of the baud clock
>> +
>> +  clock-names:
>> +    items:
>> +      - const: ipg
>> +      - const: lin
>> +
>>   required:
>>     - compatible
>>     - reg
>>     - interrupts
>> +  - clocks
>> +  - clock-names
> 
> Ah, and that's an ABI break. That's a NAK unless explained in commit msg
> (including impact on users).
> 
> Best regards,
> Krzysztof
> 

Thanks for your review. I will try to add context for these changes in 
bindings in V4.

Best Regards,
Ciprian

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

[Ciprian Costea]

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

[Frank Li]

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

[Ciprian Marian Costea]

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