[v4,2/3] dt-bindings: serial: amlogic,meson-uart: Add compatible string for T7

Message ID 20230622084045.519203-3-tanure@linux.com
State New
Headers
Series Add Amlogic A311D2 and Khadas Vim4 Board Support |

Commit Message

Lucas Tanure June 22, 2023, 8:40 a.m. UTC
  Amlogic T7 SoCs uses the same UART controller as S4 SoCs and G12A.
There is no need for an extra compatible line in the driver, but
add T7 compatible line for documentation.

Co-developed-by: Conor Dooley <conor@kernel.org>
Signed-off-by: Lucas Tanure <tanure@linux.com>
---
 .../devicetree/bindings/serial/amlogic,meson-uart.yaml      | 6 ++++++
 1 file changed, 6 insertions(+)
  

Comments

Conor Dooley June 22, 2023, 9:26 a.m. UTC | #1
Hey Lucas,

On Thu, Jun 22, 2023 at 09:40:44AM +0100, Lucas Tanure wrote:
> Amlogic T7 SoCs uses the same UART controller as S4 SoCs and G12A.
> There is no need for an extra compatible line in the driver, but
> add T7 compatible line for documentation.
> 
> Co-developed-by: Conor Dooley <conor@kernel.org>

You can delete this, I don't need a Co-developed-by tag for review
feedback.

> Signed-off-by: Lucas Tanure <tanure@linux.com>
> ---
>  .../devicetree/bindings/serial/amlogic,meson-uart.yaml      | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml b/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
> index 01ec45b3b406..4ca4673169aa 100644
> --- a/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
> +++ b/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
> @@ -46,6 +46,12 @@ properties:
>            - amlogic,meson8b-uart
>            - amlogic,meson-gx-uart
>            - amlogic,meson-s4-uart
> +      - description: Everything-Else power domain UART controller on G12A compatible SoCs

s/Everything-Else/Always-on/
Otherwise,
Acked-by: Conor Dooley <conor.dooley@microchip.com>

Perhaps it can be fixed on application, I don't know how the amlogic
maintainers operate.

Cheers,
Conor.

> +        items:
> +          - enum:
> +              - amlogic,meson-t7-uart
> +          - const: amlogic,meson-g12a-uart
> +          - const: amlogic,meson-ao-uart
>        - description: Everything-Else power domain UART controller on G12A SoCs
>          items:
>            - const: amlogic,meson-g12a-uart
> -- 
> 2.41.0
>
  
Neil Armstrong June 22, 2023, 9:38 a.m. UTC | #2
On 22/06/2023 11:26, Conor Dooley wrote:
> Hey Lucas,
> 
> On Thu, Jun 22, 2023 at 09:40:44AM +0100, Lucas Tanure wrote:
>> Amlogic T7 SoCs uses the same UART controller as S4 SoCs and G12A.
>> There is no need for an extra compatible line in the driver, but
>> add T7 compatible line for documentation.
>>
>> Co-developed-by: Conor Dooley <conor@kernel.org>
> 
> You can delete this, I don't need a Co-developed-by tag for review
> feedback.
> 
>> Signed-off-by: Lucas Tanure <tanure@linux.com>
>> ---
>>   .../devicetree/bindings/serial/amlogic,meson-uart.yaml      | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml b/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
>> index 01ec45b3b406..4ca4673169aa 100644
>> --- a/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
>> +++ b/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
>> @@ -46,6 +46,12 @@ properties:
>>             - amlogic,meson8b-uart
>>             - amlogic,meson-gx-uart
>>             - amlogic,meson-s4-uart
>> +      - description: Everything-Else power domain UART controller on G12A compatible SoCs
> 
> s/Everything-Else/Always-on/


"Everything-Else" was the amlogic naming for the non-always-on power domain, but it seems
it's no more something used on new SoCs like the T7 family.

Anyway, the description is wrong, and it's a mess because we used "amlogic,meson-ao-uart"
for uarts on the Always-On domain, but here it's described as Everything-Else...

Lucas, is there AO_uarts on T7 ? if not drop this amlogic,meson-ao-uart for the T7 UARTs.

But if there's no more AO uart controller, you'll need to change drivers/tty/serial/meson_uart.c
and add a OF_EARLYCON_DECLARE() for amlogic,meson-t7-uart.

But still, why don't you use the amlogic,meson-s4-uart as fallback instead ?

+      - description: UART controller on T7 compatible SoCs
+        items:
+          - const: amlogic,meson-t7-uart
+          - const: amlogic,meson-s4-uart

and update meson_uart.c if there's no AO uarts anymore....

Neil

> Otherwise,
> Acked-by: Conor Dooley <conor.dooley@microchip.com>
> 
> Perhaps it can be fixed on application, I don't know how the amlogic
> maintainers operate.
> 
> Cheers,
> Conor.
> 
>> +        items:
>> +          - enum:
>> +              - amlogic,meson-t7-uart
>> +          - const: amlogic,meson-g12a-uart
>> +          - const: amlogic,meson-ao-uart
>>         - description: Everything-Else power domain UART controller on G12A SoCs
>>           items:
>>             - const: amlogic,meson-g12a-uart
>> -- 
>> 2.41.0
>>
  
Conor Dooley June 22, 2023, 10:04 a.m. UTC | #3
On Thu, Jun 22, 2023 at 11:38:49AM +0200, Neil Armstrong wrote:
> On 22/06/2023 11:26, Conor Dooley wrote:
> > On Thu, Jun 22, 2023 at 09:40:44AM +0100, Lucas Tanure wrote:
> > > Amlogic T7 SoCs uses the same UART controller as S4 SoCs and G12A.
> > > There is no need for an extra compatible line in the driver, but
> > > add T7 compatible line for documentation.
> > > 
> > > Co-developed-by: Conor Dooley <conor@kernel.org>
> > 
> > You can delete this, I don't need a Co-developed-by tag for review
> > feedback.
> > 
> > > Signed-off-by: Lucas Tanure <tanure@linux.com>
> > > ---
> > >   .../devicetree/bindings/serial/amlogic,meson-uart.yaml      | 6 ++++++
> > >   1 file changed, 6 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml b/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
> > > index 01ec45b3b406..4ca4673169aa 100644
> > > --- a/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
> > > +++ b/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
> > > @@ -46,6 +46,12 @@ properties:
> > >             - amlogic,meson8b-uart
> > >             - amlogic,meson-gx-uart
> > >             - amlogic,meson-s4-uart
> > > +      - description: Everything-Else power domain UART controller on G12A compatible SoCs
> > 
> > s/Everything-Else/Always-on/
> 
> 
> "Everything-Else" was the amlogic naming for the non-always-on power domain, but it seems
> it's no more something used on new SoCs like the T7 family.
> 
> Anyway, the description is wrong, and it's a mess because we used "amlogic,meson-ao-uart"
> for uarts on the Always-On domain, but here it's described as Everything-Else...
> 
> Lucas, is there AO_uarts on T7 ? if not drop this amlogic,meson-ao-uart for the T7 UARTs.
> 
> But if there's no more AO uart controller, you'll need to change drivers/tty/serial/meson_uart.c
> and add a OF_EARLYCON_DECLARE() for amlogic,meson-t7-uart.
> 
> But still, why don't you use the amlogic,meson-s4-uart as fallback instead ?

That's probably my fault, I suggested the g12a in my reply to v3,
because that is the one from which the match data is being copied, so it
seemed like it'd make the most sense to me as the fallback.

btw Neil, is the "meson-gx-uart" a wildcard? Dunno the hardware, so
wasn't sure.

> +      - description: UART controller on T7 compatible SoCs
> +        items:
> +          - const: amlogic,meson-t7-uart
> +          - const: amlogic,meson-s4-uart

As mentioned on v (N-1), I am fine with this either. You can keep the
Ack Lucas with this change.

> and update meson_uart.c if there's no AO uarts anymore....
> 
> Neil
> 
> > Otherwise,
> > Acked-by: Conor Dooley <conor.dooley@microchip.com>
  
Lucas Tanure June 23, 2023, 7:51 a.m. UTC | #4
On Thu, Jun 22, 2023 at 10:38 AM Neil Armstrong
<neil.armstrong@linaro.org> wrote:
>
> On 22/06/2023 11:26, Conor Dooley wrote:
> > Hey Lucas,
> >
> > On Thu, Jun 22, 2023 at 09:40:44AM +0100, Lucas Tanure wrote:
> >> Amlogic T7 SoCs uses the same UART controller as S4 SoCs and G12A.
> >> There is no need for an extra compatible line in the driver, but
> >> add T7 compatible line for documentation.
> >>
> >> Co-developed-by: Conor Dooley <conor@kernel.org>
> >
> > You can delete this, I don't need a Co-developed-by tag for review
> > feedback.
> >
> >> Signed-off-by: Lucas Tanure <tanure@linux.com>
> >> ---
> >>   .../devicetree/bindings/serial/amlogic,meson-uart.yaml      | 6 ++++++
> >>   1 file changed, 6 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml b/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
> >> index 01ec45b3b406..4ca4673169aa 100644
> >> --- a/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
> >> +++ b/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
> >> @@ -46,6 +46,12 @@ properties:
> >>             - amlogic,meson8b-uart
> >>             - amlogic,meson-gx-uart
> >>             - amlogic,meson-s4-uart
> >> +      - description: Everything-Else power domain UART controller on G12A compatible SoCs
> >
> > s/Everything-Else/Always-on/
>
>
> "Everything-Else" was the amlogic naming for the non-always-on power domain, but it seems
> it's no more something used on new SoCs like the T7 family.
>
> Anyway, the description is wrong, and it's a mess because we used "amlogic,meson-ao-uart"
> for uarts on the Always-On domain, but here it's described as Everything-Else...
>
> Lucas, is there AO_uarts on T7 ? if not drop this amlogic,meson-ao-uart for the T7 UARTs.
No, there is not an AO_uart in T7, as far as I can see from the code
and datasheet.

>
> But if there's no more AO uart controller, you'll need to change drivers/tty/serial/meson_uart.c
> and add a OF_EARLYCON_DECLARE() for amlogic,meson-t7-uart.
>
> But still, why don't you use the amlogic,meson-s4-uart as fallback instead ?
As S4 and T7 are the same as g12a, I thought it would be better to
just have g12a uart.
But sure, I can use S4 as fallback.

>
> +      - description: UART controller on T7 compatible SoCs
> +        items:
> +          - const: amlogic,meson-t7-uart
> +          - const: amlogic,meson-s4-uart
>
> and update meson_uart.c if there's no AO uarts anymore....
Ok, v5 is coming up.
>
> Neil
>
> > Otherwise,
> > Acked-by: Conor Dooley <conor.dooley@microchip.com>
> >
> > Perhaps it can be fixed on application, I don't know how the amlogic
> > maintainers operate.
> >
> > Cheers,
> > Conor.
> >
> >> +        items:
> >> +          - enum:
> >> +              - amlogic,meson-t7-uart
> >> +          - const: amlogic,meson-g12a-uart
> >> +          - const: amlogic,meson-ao-uart
> >>         - description: Everything-Else power domain UART controller on G12A SoCs
> >>           items:
> >>             - const: amlogic,meson-g12a-uart
> >> --
> >> 2.41.0
> >>
>
  

Patch

diff --git a/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml b/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
index 01ec45b3b406..4ca4673169aa 100644
--- a/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
+++ b/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
@@ -46,6 +46,12 @@  properties:
           - amlogic,meson8b-uart
           - amlogic,meson-gx-uart
           - amlogic,meson-s4-uart
+      - description: Everything-Else power domain UART controller on G12A compatible SoCs
+        items:
+          - enum:
+              - amlogic,meson-t7-uart
+          - const: amlogic,meson-g12a-uart
+          - const: amlogic,meson-ao-uart
       - description: Everything-Else power domain UART controller on G12A SoCs
         items:
           - const: amlogic,meson-g12a-uart