[PATCH v5 2/4] dt-bindings: serial: amlogic,meson-uart: Add compatible string for T7

Lucas Tanure posted 4 patches 2 years, 7 months ago
There is a newer version of this series
[PATCH v5 2/4] dt-bindings: serial: amlogic,meson-uart: Add compatible string for T7
Posted by Lucas Tanure 2 years, 7 months ago
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.

Signed-off-by: Lucas Tanure <tanure@linux.com>
---
 .../devicetree/bindings/serial/amlogic,meson-uart.yaml        | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml b/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
index 01ec45b3b406..ad970c9ed1c7 100644
--- a/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
+++ b/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
@@ -50,6 +50,10 @@ properties:
         items:
           - const: amlogic,meson-g12a-uart
           - const: amlogic,meson-gx-uart
+      - description: UART controller on T7 compatible SoCs
+        items:
+          - const: amlogic,meson-t7-uart
+          - const: amlogic,meson-s4-uart
 
   reg:
     maxItems: 1
-- 
2.41.0
Re: [PATCH v5 2/4] dt-bindings: serial: amlogic,meson-uart: Add compatible string for T7
Posted by Krzysztof Kozlowski 2 years, 7 months ago
On 23/06/2023 10:12, 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.
> 
> Signed-off-by: Lucas Tanure <tanure@linux.com>
> ---
>  .../devicetree/bindings/serial/amlogic,meson-uart.yaml        | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml b/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
> index 01ec45b3b406..ad970c9ed1c7 100644
> --- a/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
> +++ b/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
> @@ -50,6 +50,10 @@ properties:
>          items:
>            - const: amlogic,meson-g12a-uart
>            - const: amlogic,meson-gx-uart
> +      - description: UART controller on T7 compatible SoCs

Your description is rather incorrect. This is UART on SoCs compatible
with S4, not with T7. Otherwise what do you expect to grow later when
adding more compatible devices? Just drop the description, it's kind of
obvious when done correctly (but can be misleading if done wrong).

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


Best regards,
Krzysztof
Re: [PATCH v5 2/4] dt-bindings: serial: amlogic,meson-uart: Add compatible string for T7
Posted by Lucas Tanure 2 years, 7 months ago
On Fri, Jun 23, 2023 at 9:51 AM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 23/06/2023 10:12, 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.
> >
> > Signed-off-by: Lucas Tanure <tanure@linux.com>
> > ---
> >  .../devicetree/bindings/serial/amlogic,meson-uart.yaml        | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml b/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
> > index 01ec45b3b406..ad970c9ed1c7 100644
> > --- a/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
> > +++ b/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
> > @@ -50,6 +50,10 @@ properties:
> >          items:
> >            - const: amlogic,meson-g12a-uart
> >            - const: amlogic,meson-gx-uart
> > +      - description: UART controller on T7 compatible SoCs
>
> Your description is rather incorrect. This is UART on SoCs compatible
> with S4, not with T7. Otherwise what do you expect to grow later when
> adding more compatible devices? Just drop the description, it's kind of
> obvious when done correctly (but can be misleading if done wrong).
>
> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>
Sorry, but S4 is already added in another way, which accepts just an
S4 compatible string.
But for T7 we need a fallback.
Could you let me know what you're asking here? Redo S4 and add T7? Or
do T7 in another different way that I didn't get?
Do you want a v6 patch series? If yes, could you be more clear about
how you want it?

>
> Best regards,
> Krzysztof
>
Re: [PATCH v5 2/4] dt-bindings: serial: amlogic,meson-uart: Add compatible string for T7
Posted by Krzysztof Kozlowski 2 years, 7 months ago
On 23/06/2023 19:53, Lucas Tanure wrote:
> On Fri, Jun 23, 2023 at 9:51 AM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 23/06/2023 10:12, 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.
>>>
>>> Signed-off-by: Lucas Tanure <tanure@linux.com>
>>> ---
>>>  .../devicetree/bindings/serial/amlogic,meson-uart.yaml        | 4 ++++
>>>  1 file changed, 4 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml b/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
>>> index 01ec45b3b406..ad970c9ed1c7 100644
>>> --- a/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
>>> +++ b/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
>>> @@ -50,6 +50,10 @@ properties:
>>>          items:
>>>            - const: amlogic,meson-g12a-uart
>>>            - const: amlogic,meson-gx-uart
>>> +      - description: UART controller on T7 compatible SoCs
>>
>> Your description is rather incorrect. This is UART on SoCs compatible
>> with S4, not with T7. Otherwise what do you expect to grow later when
>> adding more compatible devices? Just drop the description, it's kind of
>> obvious when done correctly (but can be misleading if done wrong).
>>
>> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>
> Sorry, but S4 is already added in another way, which accepts just an
> S4 compatible string.
> But for T7 we need a fallback.
> Could you let me know what you're asking here? Redo S4 and add T7? Or
> do T7 in another different way that I didn't get?

I comment only about the description, so why touching anything else? You
did not add here T7 compatible SoCs. You added here S4 compatible SoCs.

> Do you want a v6 patch series? If yes, could you be more clear about
> how you want it?

No need. If you are going to send v6, you can as well drop the description.


---

This is an automated instruction, just in case, because many review tags
are being ignored. If you do not know the process, here is a short
explanation:

Please add Acked-by/Reviewed-by/Tested-by tags when posting new
versions, under or above your Signed-off-by tag. Tools like b4 can help
here. However, there's no need to repost patches *only* to add the tags.
The upstream maintainer will do that for acks received on the version
they apply.

https://elixir.bootlin.com/linux/v5.17/source/Documentation/process/submitting-patches.rst#L540

Best regards,
Krzysztof

Re: [PATCH v5 2/4] dt-bindings: serial: amlogic,meson-uart: Add compatible string for T7
Posted by Lucas Tanure 2 years, 7 months ago
On Fri, Jun 23, 2023 at 6:56 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 23/06/2023 19:53, Lucas Tanure wrote:
> > On Fri, Jun 23, 2023 at 9:51 AM Krzysztof Kozlowski
> > <krzysztof.kozlowski@linaro.org> wrote:
> >>
> >> On 23/06/2023 10:12, 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.
> >>>
> >>> Signed-off-by: Lucas Tanure <tanure@linux.com>
> >>> ---
> >>>  .../devicetree/bindings/serial/amlogic,meson-uart.yaml        | 4 ++++
> >>>  1 file changed, 4 insertions(+)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml b/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
> >>> index 01ec45b3b406..ad970c9ed1c7 100644
> >>> --- a/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
> >>> +++ b/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
> >>> @@ -50,6 +50,10 @@ properties:
> >>>          items:
> >>>            - const: amlogic,meson-g12a-uart
> >>>            - const: amlogic,meson-gx-uart
> >>> +      - description: UART controller on T7 compatible SoCs
> >>
> >> Your description is rather incorrect. This is UART on SoCs compatible
> >> with S4, not with T7. Otherwise what do you expect to grow later when
> >> adding more compatible devices? Just drop the description, it's kind of
> >> obvious when done correctly (but can be misleading if done wrong).
> >>
> >> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> >>
> > Sorry, but S4 is already added in another way, which accepts just an
> > S4 compatible string.
> > But for T7 we need a fallback.
> > Could you let me know what you're asking here? Redo S4 and add T7? Or
> > do T7 in another different way that I didn't get?
>
> I comment only about the description, so why touching anything else? You
> did not add here T7 compatible SoCs. You added here S4 compatible SoCs.
>
> > Do you want a v6 patch series? If yes, could you be more clear about
> > how you want it?
>
> No need. If you are going to send v6, you can as well drop the description.
>
I can't just remove that line, as it doesn't pass the checks.
I will change it to S4.

>
> ---
>
> This is an automated instruction, just in case, because many review tags
> are being ignored. If you do not know the process, here is a short
> explanation:
>
> Please add Acked-by/Reviewed-by/Tested-by tags when posting new
> versions, under or above your Signed-off-by tag. Tools like b4 can help
> here. However, there's no need to repost patches *only* to add the tags.
> The upstream maintainer will do that for acks received on the version
> they apply.
>
> https://elixir.bootlin.com/linux/v5.17/source/Documentation/process/submitting-patches.rst#L540
>
> Best regards,
> Krzysztof
>