| Message ID | 20231129060043.368874-6-jeeheng.sia@starfivetech.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Initial device tree support for StarFive JH8100 SoC | expand |
On 29/11/2023 07:00, Sia Jee Heng wrote: > Add new compatible string for UART in the StarFive JH8100 SoC. > > Signed-off-by: Sia Jee Heng <jeeheng.sia@starfivetech.com> > Reviewed-by: Ley Foon Tan <leyfoon.tan@starfivetech.com> The patch is quite different than v1. Are you sure the review is applicable? If it was given for v2, where is it? > --- > Documentation/devicetree/bindings/serial/cdns,uart.yaml | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/Documentation/devicetree/bindings/serial/cdns,uart.yaml b/Documentation/devicetree/bindings/serial/cdns,uart.yaml > index e35ad1109efc..0d05305778f2 100644 > --- a/Documentation/devicetree/bindings/serial/cdns,uart.yaml > +++ b/Documentation/devicetree/bindings/serial/cdns,uart.yaml > @@ -20,6 +20,10 @@ properties: > items: > - const: xlnx,zynqmp-uart > - const: cdns,uart-r1p12 > + - description: UART controller for StarFive JH8100 SoC This is duplicating compatible, drop. > + items: > + - const: starfive,jh8100-uart > + - const: cdns,uart-r1p8 Don't add things to the end of the list, but keep order. I would suggest to put it at the beginning, so before Xilinx. Best regards, Krzysztof
> -----Original Message----- > From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > Sent: Wednesday, November 29, 2023 4:26 PM > To: JeeHeng Sia <jeeheng.sia@starfivetech.com>; kernel@esmil.dk; robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org; > krzk@kernel.org; conor+dt@kernel.org; paul.walmsley@sifive.com; palmer@dabbelt.com; aou@eecs.berkeley.edu; > daniel.lezcano@linaro.org; tglx@linutronix.de; conor@kernel.org; anup@brainfault.org; gregkh@linuxfoundation.org; > jirislaby@kernel.org; michal.simek@amd.com; Michael Zhu <michael.zhu@starfivetech.com>; drew@beagleboard.org > Cc: devicetree@vger.kernel.org; linux-riscv@lists.infradead.org; linux-kernel@vger.kernel.org; Leyfoon Tan > <leyfoon.tan@starfivetech.com> > Subject: Re: [PATCH v2 5/6] dt-bindings: serial: cdns: Add new compatible string for StarFive JH8100 UART > > On 29/11/2023 07:00, Sia Jee Heng wrote: > > Add new compatible string for UART in the StarFive JH8100 SoC. > > > > Signed-off-by: Sia Jee Heng <jeeheng.sia@starfivetech.com> > > Reviewed-by: Ley Foon Tan <leyfoon.tan@starfivetech.com> > > The patch is quite different than v1. Are you sure the review is > applicable? If it was given for v2, where is it? This patch is impacted by the comment suggesting the exclusion of patch 5 in V1. In V2, this patch adds compatible for cdns-uart-r1p8, allowing us to continue using the cdns uart. > > > --- > > Documentation/devicetree/bindings/serial/cdns,uart.yaml | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/serial/cdns,uart.yaml b/Documentation/devicetree/bindings/serial/cdns,uart.yaml > > index e35ad1109efc..0d05305778f2 100644 > > --- a/Documentation/devicetree/bindings/serial/cdns,uart.yaml > > +++ b/Documentation/devicetree/bindings/serial/cdns,uart.yaml > > @@ -20,6 +20,10 @@ properties: > > items: > > - const: xlnx,zynqmp-uart > > - const: cdns,uart-r1p12 > > + - description: UART controller for StarFive JH8100 SoC > > This is duplicating compatible, drop. Do you mean drop compatible for starfive,jh8100-uart ? > > > + items: > > + - const: starfive,jh8100-uart > > + - const: cdns,uart-r1p8 > > Don't add things to the end of the list, but keep order. I would suggest > to put it at the beginning, so before Xilinx. I'm trying to get what you're asking, but it's a bit confusing for me. So, I thought it might be easier if I just share the code below. Please let me know if this addresses your comment? properties: compatible: oneOf: - description: UART controller for StarFive JH8100 SoC items: - const: cdns,uart-r1p8 - description: UART controller for Zynq-7xxx SoC items: - const: xlnx,xuartps - const: cdns,uart-r1p8 - description: UART controller for Zynq Ultrascale+ MPSoC items: - const: xlnx,zynqmp-uart - const: cdns,uart-r1p12 > > > Best regards, > Krzysztof
On 29/11/2023 11:33, JeeHeng Sia wrote: > > >> -----Original Message----- >> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >> Sent: Wednesday, November 29, 2023 4:26 PM >> To: JeeHeng Sia <jeeheng.sia@starfivetech.com>; kernel@esmil.dk; robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org; >> krzk@kernel.org; conor+dt@kernel.org; paul.walmsley@sifive.com; palmer@dabbelt.com; aou@eecs.berkeley.edu; >> daniel.lezcano@linaro.org; tglx@linutronix.de; conor@kernel.org; anup@brainfault.org; gregkh@linuxfoundation.org; >> jirislaby@kernel.org; michal.simek@amd.com; Michael Zhu <michael.zhu@starfivetech.com>; drew@beagleboard.org >> Cc: devicetree@vger.kernel.org; linux-riscv@lists.infradead.org; linux-kernel@vger.kernel.org; Leyfoon Tan >> <leyfoon.tan@starfivetech.com> >> Subject: Re: [PATCH v2 5/6] dt-bindings: serial: cdns: Add new compatible string for StarFive JH8100 UART >> >> On 29/11/2023 07:00, Sia Jee Heng wrote: >>> Add new compatible string for UART in the StarFive JH8100 SoC. >>> >>> Signed-off-by: Sia Jee Heng <jeeheng.sia@starfivetech.com> >>> Reviewed-by: Ley Foon Tan <leyfoon.tan@starfivetech.com> >> >> The patch is quite different than v1. Are you sure the review is >> applicable? If it was given for v2, where is it? > This patch is impacted by the comment suggesting the exclusion of patch 5 in V1. In V2, this patch adds compatible for cdns-uart-r1p8, allowing us to continue using the cdns uart. Please wrap your replies. How does this answer my concern about review tag? Do you understand that my comments are inline under the exact line which is questioned? >> >>> --- >>> Documentation/devicetree/bindings/serial/cdns,uart.yaml | 4 ++++ >>> 1 file changed, 4 insertions(+) >>> >>> diff --git a/Documentation/devicetree/bindings/serial/cdns,uart.yaml b/Documentation/devicetree/bindings/serial/cdns,uart.yaml >>> index e35ad1109efc..0d05305778f2 100644 >>> --- a/Documentation/devicetree/bindings/serial/cdns,uart.yaml >>> +++ b/Documentation/devicetree/bindings/serial/cdns,uart.yaml >>> @@ -20,6 +20,10 @@ properties: >>> items: >>> - const: xlnx,zynqmp-uart >>> - const: cdns,uart-r1p12 >>> + - description: UART controller for StarFive JH8100 SoC >> >> This is duplicating compatible, drop. > Do you mean drop compatible for starfive,jh8100-uart ? No, drop description and use directly " - items" >> >>> + items: >>> + - const: starfive,jh8100-uart >>> + - const: cdns,uart-r1p8 >> >> Don't add things to the end of the list, but keep order. I would suggest >> to put it at the beginning, so before Xilinx. > I'm trying to get what you're asking, but it's a bit confusing for me. So, I thought it might be easier if I just share the code below. Please let me know if this addresses your comment? > properties: > compatible: > oneOf: > - description: UART controller for StarFive JH8100 SoC > items: > - const: cdns,uart-r1p8 Order is fixed, thanks. But drop description and bring back specific compatible. You must have specific compatibles, always. Best regards, Krzysztof
> -----Original Message----- > From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > Sent: Wednesday, November 29, 2023 6:53 PM > To: JeeHeng Sia <jeeheng.sia@starfivetech.com>; kernel@esmil.dk; robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org; > krzk@kernel.org; conor+dt@kernel.org; paul.walmsley@sifive.com; palmer@dabbelt.com; aou@eecs.berkeley.edu; > daniel.lezcano@linaro.org; tglx@linutronix.de; conor@kernel.org; anup@brainfault.org; gregkh@linuxfoundation.org; > jirislaby@kernel.org; michal.simek@amd.com; Michael Zhu <michael.zhu@starfivetech.com>; drew@beagleboard.org > Cc: devicetree@vger.kernel.org; linux-riscv@lists.infradead.org; linux-kernel@vger.kernel.org; Leyfoon Tan > <leyfoon.tan@starfivetech.com> > Subject: Re: [PATCH v2 5/6] dt-bindings: serial: cdns: Add new compatible string for StarFive JH8100 UART > > On 29/11/2023 11:33, JeeHeng Sia wrote: > > > > > >> -----Original Message----- > >> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > >> Sent: Wednesday, November 29, 2023 4:26 PM > >> To: JeeHeng Sia <jeeheng.sia@starfivetech.com>; kernel@esmil.dk; robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org; > >> krzk@kernel.org; conor+dt@kernel.org; paul.walmsley@sifive.com; palmer@dabbelt.com; aou@eecs.berkeley.edu; > >> daniel.lezcano@linaro.org; tglx@linutronix.de; conor@kernel.org; anup@brainfault.org; gregkh@linuxfoundation.org; > >> jirislaby@kernel.org; michal.simek@amd.com; Michael Zhu <michael.zhu@starfivetech.com>; drew@beagleboard.org > >> Cc: devicetree@vger.kernel.org; linux-riscv@lists.infradead.org; linux-kernel@vger.kernel.org; Leyfoon Tan > >> <leyfoon.tan@starfivetech.com> > >> Subject: Re: [PATCH v2 5/6] dt-bindings: serial: cdns: Add new compatible string for StarFive JH8100 UART > >> > >> On 29/11/2023 07:00, Sia Jee Heng wrote: > >>> Add new compatible string for UART in the StarFive JH8100 SoC. > >>> > >>> Signed-off-by: Sia Jee Heng <jeeheng.sia@starfivetech.com> > >>> Reviewed-by: Ley Foon Tan <leyfoon.tan@starfivetech.com> > >> > >> The patch is quite different than v1. Are you sure the review is > >> applicable? If it was given for v2, where is it? > > This patch is impacted by the comment suggesting the exclusion of patch 5 in V1. In V2, this patch adds compatible for cdns-uart- > r1p8, allowing us to continue using the cdns uart. > > Please wrap your replies. I am sorry, but may I know the preferred length of characters? > > How does this answer my concern about review tag? My bad. I should have obtained your consensus before proceeding with the changes to this patch, especially after dropping patch 5 as requested in version 1. > > Do you understand that my comments are inline under the exact line which > is questioned? Yes, lesson learned. > > >> > >>> --- > >>> Documentation/devicetree/bindings/serial/cdns,uart.yaml | 4 ++++ > >>> 1 file changed, 4 insertions(+) > >>> > >>> diff --git a/Documentation/devicetree/bindings/serial/cdns,uart.yaml > b/Documentation/devicetree/bindings/serial/cdns,uart.yaml > >>> index e35ad1109efc..0d05305778f2 100644 > >>> --- a/Documentation/devicetree/bindings/serial/cdns,uart.yaml > >>> +++ b/Documentation/devicetree/bindings/serial/cdns,uart.yaml > >>> @@ -20,6 +20,10 @@ properties: > >>> items: > >>> - const: xlnx,zynqmp-uart > >>> - const: cdns,uart-r1p12 > >>> + - description: UART controller for StarFive JH8100 SoC > >> > >> This is duplicating compatible, drop. > > Do you mean drop compatible for starfive,jh8100-uart ? > > No, drop description and use directly " - items" Ok. > > >> > >>> + items: > >>> + - const: starfive,jh8100-uart > >>> + - const: cdns,uart-r1p8 > >> > >> Don't add things to the end of the list, but keep order. I would suggest > >> to put it at the beginning, so before Xilinx. > > I'm trying to get what you're asking, but it's a bit confusing for me. So, I thought it might be easier if I just share the code below. > Please let me know if this addresses your comment? > > properties: > > compatible: > > oneOf: > > - description: UART controller for StarFive JH8100 SoC > > items: > > - const: cdns,uart-r1p8 > > Order is fixed, thanks. But drop description and bring back specific > compatible. You must have specific compatibles, always. Noted. > > > > Best regards, > Krzysztof
diff --git a/Documentation/devicetree/bindings/serial/cdns,uart.yaml b/Documentation/devicetree/bindings/serial/cdns,uart.yaml index e35ad1109efc..0d05305778f2 100644 --- a/Documentation/devicetree/bindings/serial/cdns,uart.yaml +++ b/Documentation/devicetree/bindings/serial/cdns,uart.yaml @@ -20,6 +20,10 @@ properties: items: - const: xlnx,zynqmp-uart - const: cdns,uart-r1p12 + - description: UART controller for StarFive JH8100 SoC + items: + - const: starfive,jh8100-uart + - const: cdns,uart-r1p8 reg: maxItems: 1