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

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

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

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

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

-- 
2.45.2

[PATCH 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  | 21 +++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
index 4171f524a928..45fcab9e186d 100644
--- a/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
+++ b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
@@ -34,6 +34,14 @@ properties:
   interrupts:
     maxItems: 1
 
+  clocks:
+    maxItems: 2
+
+  clock-names:
+    items:
+      - const: ipg
+      - const: lin
+
 required:
   - compatible
   - reg
@@ -48,3 +56,16 @@ examples:
         reg = <0x40053000 0x1000>;
         interrupts = <0 59 4>;
     };
+
+  - |
+    #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 14>, <&clks 13>;
+        clock-names = "lin", "ipg";
+    };
-- 
2.45.2

[PATCH 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 1/2] dt-bindings: linflexuart: add clock definitions

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1690 bytes --]

On Tue, Sep 24, 2024 at 05:12:46PM +0300, 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.
> 
> Signed-off-by: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>
> ---
>  .../bindings/serial/fsl,s32-linflexuart.yaml  | 21 +++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
> index 4171f524a928..45fcab9e186d 100644
> --- a/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
> +++ b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
> @@ -34,6 +34,14 @@ properties:
>    interrupts:
>      maxItems: 1
>  
> +  clocks:
> +    maxItems: 2
> +
> +  clock-names:
> +    items:
> +      - const: ipg
> +      - const: lin

Can all devices have 2 clocks, or just the s32g2?

> +
>  required:
>    - compatible
>    - reg
> @@ -48,3 +56,16 @@ examples:
>          reg = <0x40053000 0x1000>;
>          interrupts = <0 59 4>;
>      };
> +
> +  - |
> +    #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 14>, <&clks 13>;
> +        clock-names = "lin", "ipg";
> +    };
> -- 
> 2.45.2
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Ciprian Marian Costea]

On 9/24/2024 5:24 PM, Conor Dooley wrote:
> On Tue, Sep 24, 2024 at 05:12:46PM +0300, 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.
>>
>> Signed-off-by: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>
>> ---
>>   .../bindings/serial/fsl,s32-linflexuart.yaml  | 21 +++++++++++++++++++
>>   1 file changed, 21 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
>> index 4171f524a928..45fcab9e186d 100644
>> --- a/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
>> +++ b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
>> @@ -34,6 +34,14 @@ properties:
>>     interrupts:
>>       maxItems: 1
>>   
>> +  clocks:
>> +    maxItems: 2
>> +
>> +  clock-names:
>> +    items:
>> +      - const: ipg
>> +      - const: lin
> 
> Can all devices have 2 clocks, or just the s32g2?
> 

Hello Conor,

All devices (S32G2, S32G3 and S32V234) have 2 clocks for LINFlexD module.
They are: "lin" which is the frequency of the baud clock and "ipg" which 
drives the access to the LINFlexD iomapped registers.


Best Regards,
Ciprian

>> +
>>   required:
>>     - compatible
>>     - reg
>> @@ -48,3 +56,16 @@ examples:
>>           reg = <0x40053000 0x1000>;
>>           interrupts = <0 59 4>;
>>       };
>> +
>> +  - |
>> +    #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 14>, <&clks 13>;
>> +        clock-names = "lin", "ipg";
>> +    };
>> -- 
>> 2.45.2
>>

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

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1638 bytes --]

On Tue, Sep 24, 2024 at 05:52:13PM +0300, Ciprian Marian Costea wrote:
> On 9/24/2024 5:24 PM, Conor Dooley wrote:
> > On Tue, Sep 24, 2024 at 05:12:46PM +0300, 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.
> > > 
> > > Signed-off-by: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>
> > > ---
> > >   .../bindings/serial/fsl,s32-linflexuart.yaml  | 21 +++++++++++++++++++
> > >   1 file changed, 21 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
> > > index 4171f524a928..45fcab9e186d 100644
> > > --- a/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
> > > +++ b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
> > > @@ -34,6 +34,14 @@ properties:
> > >     interrupts:
> > >       maxItems: 1
> > > +  clocks:
> > > +    maxItems: 2
> > > +
> > > +  clock-names:
> > > +    items:
> > > +      - const: ipg
> > > +      - const: lin
> > 
> > Can all devices have 2 clocks, or just the s32g2?
> > 
> 
> All devices (S32G2, S32G3 and S32V234) have 2 clocks for LINFlexD module.

I see. How come the driver is capable of working without them?

> They are: "lin" which is the frequency of the baud clock and "ipg" which
> drives the access to the LINFlexD iomapped registers.

It would be good to have an items list in the clocks property with that
information.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Ciprian Marian Costea]

On 9/24/2024 6:01 PM, Conor Dooley wrote:
> On Tue, Sep 24, 2024 at 05:52:13PM +0300, Ciprian Marian Costea wrote:
>> On 9/24/2024 5:24 PM, Conor Dooley wrote:
>>> On Tue, Sep 24, 2024 at 05:12:46PM +0300, 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.
>>>>
>>>> Signed-off-by: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>
>>>> ---
>>>>    .../bindings/serial/fsl,s32-linflexuart.yaml  | 21 +++++++++++++++++++
>>>>    1 file changed, 21 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
>>>> index 4171f524a928..45fcab9e186d 100644
>>>> --- a/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
>>>> +++ b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
>>>> @@ -34,6 +34,14 @@ properties:
>>>>      interrupts:
>>>>        maxItems: 1
>>>> +  clocks:
>>>> +    maxItems: 2
>>>> +
>>>> +  clock-names:
>>>> +    items:
>>>> +      - const: ipg
>>>> +      - const: lin
>>>
>>> Can all devices have 2 clocks, or just the s32g2?
>>>
>>
>> All devices (S32G2, S32G3 and S32V234) have 2 clocks for LINFlexD module.
> 
> I see. How come the driver is capable of working without them?
> 

The driver was working because the LINFlexD clocks were configured and 
kept enabled by the downstream bootloader (TF-A and U-Boot). This is not 
ideal since LINFlexD Linux driver should manage those clocks 
independently and not rely on a previous bootloader configuration (hence 
the need for this current patchset).

>> They are: "lin" which is the frequency of the baud clock and "ipg" which
>> drives the access to the LINFlexD iomapped registers.
> 
> It would be good to have an items list in the clocks property with that
> information.

Thanks for this suggestion. I would add such information in V2.

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

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2248 bytes --]

On Tue, Sep 24, 2024 at 06:17:11PM +0300, Ciprian Marian Costea wrote:
> On 9/24/2024 6:01 PM, Conor Dooley wrote:
> > On Tue, Sep 24, 2024 at 05:52:13PM +0300, Ciprian Marian Costea wrote:
> > > On 9/24/2024 5:24 PM, Conor Dooley wrote:
> > > > On Tue, Sep 24, 2024 at 05:12:46PM +0300, 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.
> > > > > 
> > > > > Signed-off-by: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>
> > > > > ---
> > > > >    .../bindings/serial/fsl,s32-linflexuart.yaml  | 21 +++++++++++++++++++
> > > > >    1 file changed, 21 insertions(+)
> > > > > 
> > > > > diff --git a/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
> > > > > index 4171f524a928..45fcab9e186d 100644
> > > > > --- a/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
> > > > > +++ b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
> > > > > @@ -34,6 +34,14 @@ properties:
> > > > >      interrupts:
> > > > >        maxItems: 1
> > > > > +  clocks:
> > > > > +    maxItems: 2
> > > > > +
> > > > > +  clock-names:
> > > > > +    items:
> > > > > +      - const: ipg
> > > > > +      - const: lin
> > > > 
> > > > Can all devices have 2 clocks, or just the s32g2?
> > > > 
> > > 
> > > All devices (S32G2, S32G3 and S32V234) have 2 clocks for LINFlexD module.
> > 
> > I see. How come the driver is capable of working without them?
> > 
> 
> The driver was working because the LINFlexD clocks were configured and kept
> enabled by the downstream bootloader (TF-A and U-Boot). This is not ideal
> since LINFlexD Linux driver should manage those clocks independently and not
> rely on a previous bootloader configuration (hence the need for this current
> patchset).

I'd also mark them as required in the binding too, but the driver will
still have to account for them being missing, for backwards
compatibility reasons. Add a comment explaining the history to the
commit message when you do that.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Ciprian Marian Costea]

On 9/24/2024 6:27 PM, Conor Dooley wrote:
> On Tue, Sep 24, 2024 at 06:17:11PM +0300, Ciprian Marian Costea wrote:
>> On 9/24/2024 6:01 PM, Conor Dooley wrote:
>>> On Tue, Sep 24, 2024 at 05:52:13PM +0300, Ciprian Marian Costea wrote:
>>>> On 9/24/2024 5:24 PM, Conor Dooley wrote:
>>>>> On Tue, Sep 24, 2024 at 05:12:46PM +0300, 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.
>>>>>>
>>>>>> Signed-off-by: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>
>>>>>> ---
>>>>>>     .../bindings/serial/fsl,s32-linflexuart.yaml  | 21 +++++++++++++++++++
>>>>>>     1 file changed, 21 insertions(+)
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
>>>>>> index 4171f524a928..45fcab9e186d 100644
>>>>>> --- a/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
>>>>>> +++ b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
>>>>>> @@ -34,6 +34,14 @@ properties:
>>>>>>       interrupts:
>>>>>>         maxItems: 1
>>>>>> +  clocks:
>>>>>> +    maxItems: 2
>>>>>> +
>>>>>> +  clock-names:
>>>>>> +    items:
>>>>>> +      - const: ipg
>>>>>> +      - const: lin
>>>>>
>>>>> Can all devices have 2 clocks, or just the s32g2?
>>>>>
>>>>
>>>> All devices (S32G2, S32G3 and S32V234) have 2 clocks for LINFlexD module.
>>>
>>> I see. How come the driver is capable of working without them?
>>>
>>
>> The driver was working because the LINFlexD clocks were configured and kept
>> enabled by the downstream bootloader (TF-A and U-Boot). This is not ideal
>> since LINFlexD Linux driver should manage those clocks independently and not
>> rely on a previous bootloader configuration (hence the need for this current
>> patchset).
> 
> I'd also mark them as required in the binding too, but the driver will
> still have to account for them being missing, for backwards
> compatibility reasons. Add a comment explaining the history to the
> commit message when you do that.

Already in the second patch from this patchset the clocking support was 
added in the LINFlexD driver as optional, indeed for backwards 
compatibility.
I presumed that because of this optional clock enablement, I should not 
add the clocks as required in the bindings, but I will do so in V2. Thanks.

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

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 3003 bytes --]

On Tue, Sep 24, 2024 at 06:32:30PM +0300, Ciprian Marian Costea wrote:
> On 9/24/2024 6:27 PM, Conor Dooley wrote:
> > On Tue, Sep 24, 2024 at 06:17:11PM +0300, Ciprian Marian Costea wrote:
> > > On 9/24/2024 6:01 PM, Conor Dooley wrote:
> > > > On Tue, Sep 24, 2024 at 05:52:13PM +0300, Ciprian Marian Costea wrote:
> > > > > On 9/24/2024 5:24 PM, Conor Dooley wrote:
> > > > > > On Tue, Sep 24, 2024 at 05:12:46PM +0300, 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.
> > > > > > > 
> > > > > > > Signed-off-by: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>
> > > > > > > ---
> > > > > > >     .../bindings/serial/fsl,s32-linflexuart.yaml  | 21 +++++++++++++++++++
> > > > > > >     1 file changed, 21 insertions(+)
> > > > > > > 
> > > > > > > diff --git a/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
> > > > > > > index 4171f524a928..45fcab9e186d 100644
> > > > > > > --- a/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
> > > > > > > +++ b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
> > > > > > > @@ -34,6 +34,14 @@ properties:
> > > > > > >       interrupts:
> > > > > > >         maxItems: 1
> > > > > > > +  clocks:
> > > > > > > +    maxItems: 2
> > > > > > > +
> > > > > > > +  clock-names:
> > > > > > > +    items:
> > > > > > > +      - const: ipg
> > > > > > > +      - const: lin
> > > > > > 
> > > > > > Can all devices have 2 clocks, or just the s32g2?
> > > > > > 
> > > > > 
> > > > > All devices (S32G2, S32G3 and S32V234) have 2 clocks for LINFlexD module.
> > > > 
> > > > I see. How come the driver is capable of working without them?
> > > > 
> > > 
> > > The driver was working because the LINFlexD clocks were configured and kept
> > > enabled by the downstream bootloader (TF-A and U-Boot). This is not ideal
> > > since LINFlexD Linux driver should manage those clocks independently and not
> > > rely on a previous bootloader configuration (hence the need for this current
> > > patchset).
> > 
> > I'd also mark them as required in the binding too, but the driver will
> > still have to account for them being missing, for backwards
> > compatibility reasons. Add a comment explaining the history to the
> > commit message when you do that.
> 
> Already in the second patch from this patchset the clocking support was
> added in the LINFlexD driver as optional, indeed for backwards
> compatibility.

Oh great.

> I presumed that because of this optional clock enablement, I should not add
> the clocks as required in the bindings, but I will do so in V2. Thanks.

IMO required is correct, since it would not have worked properly if the
bootloader hadn't done the setup.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]