[PATCH v2 1/2] dt-bindings: serial: snps-dw-apb-uart: Add Sophgo SG2044 uarts

Inochi Amaoto posted 2 patches 1 month ago
There is a newer version of this series
[PATCH v2 1/2] dt-bindings: serial: snps-dw-apb-uart: Add Sophgo SG2044 uarts
Posted by Inochi Amaoto 1 month ago
The UART of SG2044 is modified version of the standard Synopsys
DesignWare UART. The UART on SG2044 relys on the internal divisor
and can not set right clock rate for the common bitrates.

Add compatibles string for the Sophgo SG2044 uarts.

Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
---
 .../devicetree/bindings/serial/snps-dw-apb-uart.yaml          | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml b/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml
index 4cdb0dcaccf3..6963f89a1848 100644
--- a/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml
+++ b/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml
@@ -58,6 +58,10 @@ properties:
               - brcm,bcm11351-dw-apb-uart
               - brcm,bcm21664-dw-apb-uart
           - const: snps,dw-apb-uart
+      - items:
+          - enum:
+              - sophgo,sg2044-uart
+          - const: snps,dw-apb-uart
       - items:
           - enum:
               - starfive,jh7100-hsuart
-- 
2.47.0
Re: [PATCH v2 1/2] dt-bindings: serial: snps-dw-apb-uart: Add Sophgo SG2044 uarts
Posted by Conor Dooley 1 month ago
On Mon, Oct 21, 2024 at 03:26:05PM +0800, Inochi Amaoto wrote:
> The UART of SG2044 is modified version of the standard Synopsys
> DesignWare UART. The UART on SG2044 relys on the internal divisor
> and can not set right clock rate for the common bitrates.
> 
> Add compatibles string for the Sophgo SG2044 uarts.
> 
> Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
> ---
>  .../devicetree/bindings/serial/snps-dw-apb-uart.yaml          | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml b/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml
> index 4cdb0dcaccf3..6963f89a1848 100644
> --- a/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml
> +++ b/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml
> @@ -58,6 +58,10 @@ properties:
>                - brcm,bcm11351-dw-apb-uart
>                - brcm,bcm21664-dw-apb-uart
>            - const: snps,dw-apb-uart
> +      - items:
> +          - enum:
> +              - sophgo,sg2044-uart
> +          - const: snps,dw-apb-uart

Why does each vendor have an items entry of its own? Seems like needless
clutter of the file IMO, except for the renesas bit.


Cheers,
Conor.
Re: [PATCH v2 1/2] dt-bindings: serial: snps-dw-apb-uart: Add Sophgo SG2044 uarts
Posted by Inochi Amaoto 1 month ago
On Mon, Oct 21, 2024 at 01:10:52PM +0100, Conor Dooley wrote:
> On Mon, Oct 21, 2024 at 03:26:05PM +0800, Inochi Amaoto wrote:
> > The UART of SG2044 is modified version of the standard Synopsys
> > DesignWare UART. The UART on SG2044 relys on the internal divisor
> > and can not set right clock rate for the common bitrates.
> > 
> > Add compatibles string for the Sophgo SG2044 uarts.
> > 
> > Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
> > ---
> >  .../devicetree/bindings/serial/snps-dw-apb-uart.yaml          | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml b/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml
> > index 4cdb0dcaccf3..6963f89a1848 100644
> > --- a/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml
> > +++ b/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml
> > @@ -58,6 +58,10 @@ properties:
> >                - brcm,bcm11351-dw-apb-uart
> >                - brcm,bcm21664-dw-apb-uart
> >            - const: snps,dw-apb-uart
> > +      - items:
> > +          - enum:
> > +              - sophgo,sg2044-uart
> > +          - const: snps,dw-apb-uart
> 
> Why does each vendor have an items entry of its own? Seems like needless
> clutter of the file IMO, except for the renesas bit.
> 
> 
> Cheers,
> Conor.


I just follow others when writing this binding. I think it may need
another patch to fix this problem, right?

Regards,
Inochi
Re: [PATCH v2 1/2] dt-bindings: serial: snps-dw-apb-uart: Add Sophgo SG2044 uarts
Posted by Conor Dooley 1 month ago
On Mon, Oct 21, 2024 at 08:18:58PM +0800, Inochi Amaoto wrote:
> On Mon, Oct 21, 2024 at 01:10:52PM +0100, Conor Dooley wrote:
> > On Mon, Oct 21, 2024 at 03:26:05PM +0800, Inochi Amaoto wrote:
> > > The UART of SG2044 is modified version of the standard Synopsys
> > > DesignWare UART. The UART on SG2044 relys on the internal divisor
> > > and can not set right clock rate for the common bitrates.
> > > 
> > > Add compatibles string for the Sophgo SG2044 uarts.
> > > 
> > > Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
> > > ---
> > >  .../devicetree/bindings/serial/snps-dw-apb-uart.yaml          | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml b/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml
> > > index 4cdb0dcaccf3..6963f89a1848 100644
> > > --- a/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml
> > > +++ b/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml
> > > @@ -58,6 +58,10 @@ properties:
> > >                - brcm,bcm11351-dw-apb-uart
> > >                - brcm,bcm21664-dw-apb-uart
> > >            - const: snps,dw-apb-uart
> > > +      - items:
> > > +          - enum:
> > > +              - sophgo,sg2044-uart
> > > +          - const: snps,dw-apb-uart
> > 
> > Why does each vendor have an items entry of its own? Seems like needless
> > clutter of the file IMO, except for the renesas bit.
> 
> I just follow others when writing this binding. I think it may need
> another patch to fix this problem, right?

Yeah. But I'd hold off to see if someone gives a rationale for it being
done this way before sending that. I've not deleted this thread, and
will send an ack if someone justifies why the binding is written like
this.
Re: [PATCH v2 1/2] dt-bindings: serial: snps-dw-apb-uart: Add Sophgo SG2044 uarts
Posted by Rob Herring 1 month ago
On Mon, Oct 21, 2024 at 01:21:58PM +0100, Conor Dooley wrote:
> On Mon, Oct 21, 2024 at 08:18:58PM +0800, Inochi Amaoto wrote:
> > On Mon, Oct 21, 2024 at 01:10:52PM +0100, Conor Dooley wrote:
> > > On Mon, Oct 21, 2024 at 03:26:05PM +0800, Inochi Amaoto wrote:
> > > > The UART of SG2044 is modified version of the standard Synopsys
> > > > DesignWare UART. The UART on SG2044 relys on the internal divisor
> > > > and can not set right clock rate for the common bitrates.
> > > > 
> > > > Add compatibles string for the Sophgo SG2044 uarts.
> > > > 
> > > > Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
> > > > ---
> > > >  .../devicetree/bindings/serial/snps-dw-apb-uart.yaml          | 4 ++++
> > > >  1 file changed, 4 insertions(+)
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml b/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml
> > > > index 4cdb0dcaccf3..6963f89a1848 100644
> > > > --- a/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml
> > > > +++ b/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml
> > > > @@ -58,6 +58,10 @@ properties:
> > > >                - brcm,bcm11351-dw-apb-uart
> > > >                - brcm,bcm21664-dw-apb-uart
> > > >            - const: snps,dw-apb-uart
> > > > +      - items:
> > > > +          - enum:
> > > > +              - sophgo,sg2044-uart
> > > > +          - const: snps,dw-apb-uart
> > > 
> > > Why does each vendor have an items entry of its own? Seems like needless
> > > clutter of the file IMO, except for the renesas bit.
> > 
> > I just follow others when writing this binding. I think it may need
> > another patch to fix this problem, right?
> 
> Yeah. But I'd hold off to see if someone gives a rationale for it being
> done this way before sending that. I've not deleted this thread, and
> will send an ack if someone justifies why the binding is written like
> this.

No reason to be separate.

Rob
Re: [PATCH v2 1/2] dt-bindings: serial: snps-dw-apb-uart: Add Sophgo SG2044 uarts
Posted by Inochi Amaoto 1 month ago
On Mon, Oct 21, 2024 at 01:21:58PM +0100, Conor Dooley wrote:
> On Mon, Oct 21, 2024 at 08:18:58PM +0800, Inochi Amaoto wrote:
> > On Mon, Oct 21, 2024 at 01:10:52PM +0100, Conor Dooley wrote:
> > > On Mon, Oct 21, 2024 at 03:26:05PM +0800, Inochi Amaoto wrote:
> > > > The UART of SG2044 is modified version of the standard Synopsys
> > > > DesignWare UART. The UART on SG2044 relys on the internal divisor
> > > > and can not set right clock rate for the common bitrates.
> > > > 
> > > > Add compatibles string for the Sophgo SG2044 uarts.
> > > > 
> > > > Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
> > > > ---
> > > >  .../devicetree/bindings/serial/snps-dw-apb-uart.yaml          | 4 ++++
> > > >  1 file changed, 4 insertions(+)
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml b/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml
> > > > index 4cdb0dcaccf3..6963f89a1848 100644
> > > > --- a/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml
> > > > +++ b/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml
> > > > @@ -58,6 +58,10 @@ properties:
> > > >                - brcm,bcm11351-dw-apb-uart
> > > >                - brcm,bcm21664-dw-apb-uart
> > > >            - const: snps,dw-apb-uart
> > > > +      - items:
> > > > +          - enum:
> > > > +              - sophgo,sg2044-uart
> > > > +          - const: snps,dw-apb-uart
> > > 
> > > Why does each vendor have an items entry of its own? Seems like needless
> > > clutter of the file IMO, except for the renesas bit.
> > 
> > I just follow others when writing this binding. I think it may need
> > another patch to fix this problem, right?
> 
> Yeah. But I'd hold off to see if someone gives a rationale for it being
> done this way before sending that. I've not deleted this thread, and
> will send an ack if someone justifies why the binding is written like
> this.

Thanks.
Re: [PATCH v2 1/2] dt-bindings: serial: snps-dw-apb-uart: Add Sophgo SG2044 uarts
Posted by Conor Dooley 1 month ago
On Mon, Oct 21, 2024 at 08:23:30PM +0800, Inochi Amaoto wrote:
> On Mon, Oct 21, 2024 at 01:21:58PM +0100, Conor Dooley wrote:
> > On Mon, Oct 21, 2024 at 08:18:58PM +0800, Inochi Amaoto wrote:
> > > On Mon, Oct 21, 2024 at 01:10:52PM +0100, Conor Dooley wrote:
> > > > On Mon, Oct 21, 2024 at 03:26:05PM +0800, Inochi Amaoto wrote:
> > > > > The UART of SG2044 is modified version of the standard Synopsys
> > > > > DesignWare UART. The UART on SG2044 relys on the internal divisor
> > > > > and can not set right clock rate for the common bitrates.
> > > > > 
> > > > > Add compatibles string for the Sophgo SG2044 uarts.
> > > > > 
> > > > > Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
> > > > > ---
> > > > >  .../devicetree/bindings/serial/snps-dw-apb-uart.yaml          | 4 ++++
> > > > >  1 file changed, 4 insertions(+)
> > > > > 
> > > > > diff --git a/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml b/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml
> > > > > index 4cdb0dcaccf3..6963f89a1848 100644
> > > > > --- a/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml
> > > > > +++ b/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml
> > > > > @@ -58,6 +58,10 @@ properties:
> > > > >                - brcm,bcm11351-dw-apb-uart
> > > > >                - brcm,bcm21664-dw-apb-uart
> > > > >            - const: snps,dw-apb-uart
> > > > > +      - items:
> > > > > +          - enum:
> > > > > +              - sophgo,sg2044-uart
> > > > > +          - const: snps,dw-apb-uart
> > > > 
> > > > Why does each vendor have an items entry of its own? Seems like needless
> > > > clutter of the file IMO, except for the renesas bit.
> > > 
> > > I just follow others when writing this binding. I think it may need
> > > another patch to fix this problem, right?
> > 
> > Yeah. But I'd hold off to see if someone gives a rationale for it being
> > done this way before sending that. I've not deleted this thread, and
> > will send an ack if someone justifies why the binding is written like
> > this.

Well, Rob doesn't think they should be separate so please add that
additional patch in your next version.

Thanks,
Conor.
Re: [PATCH v2 1/2] dt-bindings: serial: snps-dw-apb-uart: Add Sophgo SG2044 uarts
Posted by Inochi Amaoto 1 month ago
On Tue, Oct 22, 2024 at 06:25:00PM +0100, Conor Dooley wrote:
> On Mon, Oct 21, 2024 at 08:23:30PM +0800, Inochi Amaoto wrote:
> > On Mon, Oct 21, 2024 at 01:21:58PM +0100, Conor Dooley wrote:
> > > On Mon, Oct 21, 2024 at 08:18:58PM +0800, Inochi Amaoto wrote:
> > > > On Mon, Oct 21, 2024 at 01:10:52PM +0100, Conor Dooley wrote:
> > > > > On Mon, Oct 21, 2024 at 03:26:05PM +0800, Inochi Amaoto wrote:
> > > > > > The UART of SG2044 is modified version of the standard Synopsys
> > > > > > DesignWare UART. The UART on SG2044 relys on the internal divisor
> > > > > > and can not set right clock rate for the common bitrates.
> > > > > > 
> > > > > > Add compatibles string for the Sophgo SG2044 uarts.
> > > > > > 
> > > > > > Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
> > > > > > ---
> > > > > >  .../devicetree/bindings/serial/snps-dw-apb-uart.yaml          | 4 ++++
> > > > > >  1 file changed, 4 insertions(+)
> > > > > > 
> > > > > > diff --git a/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml b/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml
> > > > > > index 4cdb0dcaccf3..6963f89a1848 100644
> > > > > > --- a/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml
> > > > > > +++ b/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml
> > > > > > @@ -58,6 +58,10 @@ properties:
> > > > > >                - brcm,bcm11351-dw-apb-uart
> > > > > >                - brcm,bcm21664-dw-apb-uart
> > > > > >            - const: snps,dw-apb-uart
> > > > > > +      - items:
> > > > > > +          - enum:
> > > > > > +              - sophgo,sg2044-uart
> > > > > > +          - const: snps,dw-apb-uart
> > > > > 
> > > > > Why does each vendor have an items entry of its own? Seems like needless
> > > > > clutter of the file IMO, except for the renesas bit.
> > > > 
> > > > I just follow others when writing this binding. I think it may need
> > > > another patch to fix this problem, right?
> > > 
> > > Yeah. But I'd hold off to see if someone gives a rationale for it being
> > > done this way before sending that. I've not deleted this thread, and
> > > will send an ack if someone justifies why the binding is written like
> > > this.
> 
> Well, Rob doesn't think they should be separate so please add that
> additional patch in your next version.
> 
> Thanks,
> Conor.

It is OK for me. I will add a fix patch in the next version. Can
I add you with suggested-by tag in this fix patch?

Regards,
Inochi
Re: [PATCH v2 1/2] dt-bindings: serial: snps-dw-apb-uart: Add Sophgo SG2044 uarts
Posted by Conor Dooley 1 month ago
On Wed, Oct 23, 2024 at 08:32:42AM +0800, Inochi Amaoto wrote:
> On Tue, Oct 22, 2024 at 06:25:00PM +0100, Conor Dooley wrote:
> > On Mon, Oct 21, 2024 at 08:23:30PM +0800, Inochi Amaoto wrote:
> > > On Mon, Oct 21, 2024 at 01:21:58PM +0100, Conor Dooley wrote:
> > > > On Mon, Oct 21, 2024 at 08:18:58PM +0800, Inochi Amaoto wrote:
> > > > > On Mon, Oct 21, 2024 at 01:10:52PM +0100, Conor Dooley wrote:
> > > > > > On Mon, Oct 21, 2024 at 03:26:05PM +0800, Inochi Amaoto wrote:
> > > > > > > The UART of SG2044 is modified version of the standard Synopsys
> > > > > > > DesignWare UART. The UART on SG2044 relys on the internal divisor
> > > > > > > and can not set right clock rate for the common bitrates.
> > > > > > > 
> > > > > > > Add compatibles string for the Sophgo SG2044 uarts.
> > > > > > > 
> > > > > > > Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
> > > > > > > ---
> > > > > > >  .../devicetree/bindings/serial/snps-dw-apb-uart.yaml          | 4 ++++
> > > > > > >  1 file changed, 4 insertions(+)
> > > > > > > 
> > > > > > > diff --git a/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml b/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml
> > > > > > > index 4cdb0dcaccf3..6963f89a1848 100644
> > > > > > > --- a/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml
> > > > > > > +++ b/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml
> > > > > > > @@ -58,6 +58,10 @@ properties:
> > > > > > >                - brcm,bcm11351-dw-apb-uart
> > > > > > >                - brcm,bcm21664-dw-apb-uart
> > > > > > >            - const: snps,dw-apb-uart
> > > > > > > +      - items:
> > > > > > > +          - enum:
> > > > > > > +              - sophgo,sg2044-uart
> > > > > > > +          - const: snps,dw-apb-uart
> > > > > > 
> > > > > > Why does each vendor have an items entry of its own? Seems like needless
> > > > > > clutter of the file IMO, except for the renesas bit.
> > > > > 
> > > > > I just follow others when writing this binding. I think it may need
> > > > > another patch to fix this problem, right?
> > > > 
> > > > Yeah. But I'd hold off to see if someone gives a rationale for it being
> > > > done this way before sending that. I've not deleted this thread, and
> > > > will send an ack if someone justifies why the binding is written like
> > > > this.
> > 
> > Well, Rob doesn't think they should be separate so please add that
> > additional patch in your next version.
> > 
> > Thanks,
> > Conor.
> 
> It is OK for me. I will add a fix patch in the next version. Can
> I add you with suggested-by tag in this fix patch?

If you want, but I don't really care for one.