diff mbox series

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

Message ID 20241021072606.585878-2-inochiama@gmail.com (mailing list archive)
State Handled Elsewhere
Headers show
Series serial: 8250_dw: Introduce SG2044 uart support. | expand

Checks

Context Check Description
conchuod/vmtest-for-next-PR success PR summary
conchuod/patch-1-test-1 success .github/scripts/patches/tests/build_rv32_defconfig.sh took 110.85s
conchuod/patch-1-test-2 success .github/scripts/patches/tests/build_rv64_clang_allmodconfig.sh took 963.91s
conchuod/patch-1-test-3 success .github/scripts/patches/tests/build_rv64_gcc_allmodconfig.sh took 1134.78s
conchuod/patch-1-test-4 success .github/scripts/patches/tests/build_rv64_nommu_k210_defconfig.sh took 16.89s
conchuod/patch-1-test-5 success .github/scripts/patches/tests/build_rv64_nommu_virt_defconfig.sh took 18.45s
conchuod/patch-1-test-6 success .github/scripts/patches/tests/checkpatch.sh took 0.36s
conchuod/patch-1-test-7 success .github/scripts/patches/tests/dtb_warn_rv64.sh took 36.80s
conchuod/patch-1-test-8 success .github/scripts/patches/tests/header_inline.sh took 0.00s
conchuod/patch-1-test-9 success .github/scripts/patches/tests/kdoc.sh took 0.45s
conchuod/patch-1-test-10 success .github/scripts/patches/tests/module_param.sh took 0.01s
conchuod/patch-1-test-11 success .github/scripts/patches/tests/verify_fixes.sh took 0.00s
conchuod/patch-1-test-12 success .github/scripts/patches/tests/verify_signedoff.sh took 0.02s

Commit Message

Inochi Amaoto Oct. 21, 2024, 7:26 a.m. UTC
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(+)

Comments

Conor Dooley Oct. 21, 2024, 12:10 p.m. UTC | #1
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.
Inochi Amaoto Oct. 21, 2024, 12:18 p.m. UTC | #2
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
Conor Dooley Oct. 21, 2024, 12:21 p.m. UTC | #3
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.
Inochi Amaoto Oct. 21, 2024, 12:23 p.m. UTC | #4
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.
Rob Herring (Arm) Oct. 21, 2024, 6 p.m. UTC | #5
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
Conor Dooley Oct. 22, 2024, 5:25 p.m. UTC | #6
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.
Inochi Amaoto Oct. 23, 2024, 12:32 a.m. UTC | #7
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
Conor Dooley Oct. 23, 2024, 6:53 p.m. UTC | #8
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.
diff mbox series

Patch

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