diff mbox series

[V5,2/3] dt-bindings: update riscv plic compatible string

Message ID 20211024013303.3499461-3-guoren@kernel.org (mailing list archive)
State New, archived
Headers show
Series Add thead,c900-plic support | expand

Commit Message

Guo Ren Oct. 24, 2021, 1:33 a.m. UTC
From: Guo Ren <guoren@linux.alibaba.com>

Add the compatible string "thead,c900-plic" to the riscv plic
bindings to support allwinner d1 SOC which contains c906 core.

Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
Cc: Anup Patel <anup@brainfault.org>
Cc: Atish Patra <atish.patra@wdc.com>
Cc: Heiko Stuebner <heiko@sntech.de>
Cc: Rob Herring <robh@kernel.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Palmer Dabbelt <palmerdabbelt@google.com>

---

Changes since V5:
 - Add DT list
 - Fixup compatible string
 - Remove allwinner-d1 compatible
 - make dt_binding_check

Changes since V4:
 - Update description in errata style
 - Update enum suggested by Anup, Heiko, Samuel

Changes since V3:
 - Rename "c9xx" to "c900"
 - Add thead,c900-plic in the description section
---
 .../interrupt-controller/sifive,plic-1.0.0.yaml   | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

Comments

Anup Patel Oct. 24, 2021, 7:35 a.m. UTC | #1
On Sun, Oct 24, 2021 at 7:03 AM <guoren@kernel.org> wrote:
>
> From: Guo Ren <guoren@linux.alibaba.com>
>
> Add the compatible string "thead,c900-plic" to the riscv plic
> bindings to support allwinner d1 SOC which contains c906 core.
>
> Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> Cc: Anup Patel <anup@brainfault.org>
> Cc: Atish Patra <atish.patra@wdc.com>
> Cc: Heiko Stuebner <heiko@sntech.de>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Palmer Dabbelt <palmerdabbelt@google.com>
>
> ---
>
> Changes since V5:
>  - Add DT list
>  - Fixup compatible string
>  - Remove allwinner-d1 compatible
>  - make dt_binding_check
>
> Changes since V4:
>  - Update description in errata style
>  - Update enum suggested by Anup, Heiko, Samuel
>
> Changes since V3:
>  - Rename "c9xx" to "c900"
>  - Add thead,c900-plic in the description section
> ---
>  .../interrupt-controller/sifive,plic-1.0.0.yaml   | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml
> index 08d5a57ce00f..18b97bfd7954 100644
> --- a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml
> +++ b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml
> @@ -35,6 +35,10 @@ description:
>    contains a specific memory layout, which is documented in chapter 8 of the
>    SiFive U5 Coreplex Series Manual <https://static.dev.sifive.com/U54-MC-RVCoreIP.pdf>.
>
> +  The thead,c900-plic couldn't complete masked irq source which has been disabled in
> +  enable register. Add thead_plic_chip which fix up c906-plic irq source completion
> +  problem by unmask/mask wrapper.
> +

This is an incomplete description about how T-HEAD PLIC is different from
RISC-V PLIC.

I would suggest the following:

The T-HEAD C9xx SoC implements a modified/custom T-HEAD PLIC specification
which will mask current IRQ upon read to CLAIM register and will unmask the IRQ
upon write to CLAIM register. The thead,c900-plic compatible string
represents the
custom T-HEAD PLIC specification.

Regards,
Anup

>  maintainers:
>    - Sagar Kadam <sagar.kadam@sifive.com>
>    - Paul Walmsley  <paul.walmsley@sifive.com>
> @@ -42,11 +46,16 @@ maintainers:
>
>  properties:
>    compatible:
> -    items:
> +   oneOf:
> +    - items:
>        - enum:
> -          - sifive,fu540-c000-plic
> -          - canaan,k210-plic
> +        - sifive,fu540-c000-plic
> +        - canaan,k210-plic
>        - const: sifive,plic-1.0.0
> +    - items:
> +      - enum:
> +        - allwinner,sun20i-d1-plic
> +      - const: thead,c900-plic
>
>    reg:
>      maxItems: 1
> --
> 2.25.1
>
Guo Ren Oct. 24, 2021, 9:01 a.m. UTC | #2
On Sun, Oct 24, 2021 at 3:35 PM Anup Patel <anup@brainfault.org> wrote:
>
> On Sun, Oct 24, 2021 at 7:03 AM <guoren@kernel.org> wrote:
> >
> > From: Guo Ren <guoren@linux.alibaba.com>
> >
> > Add the compatible string "thead,c900-plic" to the riscv plic
> > bindings to support allwinner d1 SOC which contains c906 core.
> >
> > Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> > Cc: Anup Patel <anup@brainfault.org>
> > Cc: Atish Patra <atish.patra@wdc.com>
> > Cc: Heiko Stuebner <heiko@sntech.de>
> > Cc: Rob Herring <robh@kernel.org>
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Cc: Palmer Dabbelt <palmerdabbelt@google.com>
> >
> > ---
> >
> > Changes since V5:
> >  - Add DT list
> >  - Fixup compatible string
> >  - Remove allwinner-d1 compatible
> >  - make dt_binding_check
> >
> > Changes since V4:
> >  - Update description in errata style
> >  - Update enum suggested by Anup, Heiko, Samuel
> >
> > Changes since V3:
> >  - Rename "c9xx" to "c900"
> >  - Add thead,c900-plic in the description section
> > ---
> >  .../interrupt-controller/sifive,plic-1.0.0.yaml   | 15 ++++++++++++---
> >  1 file changed, 12 insertions(+), 3 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml
> > index 08d5a57ce00f..18b97bfd7954 100644
> > --- a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml
> > +++ b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml
> > @@ -35,6 +35,10 @@ description:
> >    contains a specific memory layout, which is documented in chapter 8 of the
> >    SiFive U5 Coreplex Series Manual <https://static.dev.sifive.com/U54-MC-RVCoreIP.pdf>.
> >
> > +  The thead,c900-plic couldn't complete masked irq source which has been disabled in
> > +  enable register. Add thead_plic_chip which fix up c906-plic irq source completion
> > +  problem by unmask/mask wrapper.
> > +
>
> This is an incomplete description about how T-HEAD PLIC is different from
> RISC-V PLIC.
>
> I would suggest the following:
>
> The T-HEAD C9xx SoC implements a modified/custom T-HEAD PLIC specification
> which will mask current IRQ upon read to CLAIM register and will unmask the IRQ
> upon write to CLAIM register. The thead,c900-plic compatible string
> represents the
> custom T-HEAD PLIC specification.
The patch fixup the problem that when "thead,c900-plic" couldn't
complete masked irq source which has been disabled.

This patch is different from the last one in that there is no
relationship with the auto-mask feature.

>
> Regards,
> Anup
>
> >  maintainers:
> >    - Sagar Kadam <sagar.kadam@sifive.com>
> >    - Paul Walmsley  <paul.walmsley@sifive.com>
> > @@ -42,11 +46,16 @@ maintainers:
> >
> >  properties:
> >    compatible:
> > -    items:
> > +   oneOf:
> > +    - items:
> >        - enum:
> > -          - sifive,fu540-c000-plic
> > -          - canaan,k210-plic
> > +        - sifive,fu540-c000-plic
> > +        - canaan,k210-plic
> >        - const: sifive,plic-1.0.0
> > +    - items:
> > +      - enum:
> > +        - allwinner,sun20i-d1-plic
> > +      - const: thead,c900-plic
> >
> >    reg:
> >      maxItems: 1
> > --
> > 2.25.1
> >
Anup Patel Oct. 24, 2021, 9:18 a.m. UTC | #3
On Sun, Oct 24, 2021 at 2:31 PM Guo Ren <guoren@kernel.org> wrote:
>
> On Sun, Oct 24, 2021 at 3:35 PM Anup Patel <anup@brainfault.org> wrote:
> >
> > On Sun, Oct 24, 2021 at 7:03 AM <guoren@kernel.org> wrote:
> > >
> > > From: Guo Ren <guoren@linux.alibaba.com>
> > >
> > > Add the compatible string "thead,c900-plic" to the riscv plic
> > > bindings to support allwinner d1 SOC which contains c906 core.
> > >
> > > Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> > > Cc: Anup Patel <anup@brainfault.org>
> > > Cc: Atish Patra <atish.patra@wdc.com>
> > > Cc: Heiko Stuebner <heiko@sntech.de>
> > > Cc: Rob Herring <robh@kernel.org>
> > > Cc: Rob Herring <robh+dt@kernel.org>
> > > Cc: Palmer Dabbelt <palmerdabbelt@google.com>
> > >
> > > ---
> > >
> > > Changes since V5:
> > >  - Add DT list
> > >  - Fixup compatible string
> > >  - Remove allwinner-d1 compatible
> > >  - make dt_binding_check
> > >
> > > Changes since V4:
> > >  - Update description in errata style
> > >  - Update enum suggested by Anup, Heiko, Samuel
> > >
> > > Changes since V3:
> > >  - Rename "c9xx" to "c900"
> > >  - Add thead,c900-plic in the description section
> > > ---
> > >  .../interrupt-controller/sifive,plic-1.0.0.yaml   | 15 ++++++++++++---
> > >  1 file changed, 12 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml
> > > index 08d5a57ce00f..18b97bfd7954 100644
> > > --- a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml
> > > +++ b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml
> > > @@ -35,6 +35,10 @@ description:
> > >    contains a specific memory layout, which is documented in chapter 8 of the
> > >    SiFive U5 Coreplex Series Manual <https://static.dev.sifive.com/U54-MC-RVCoreIP.pdf>.
> > >
> > > +  The thead,c900-plic couldn't complete masked irq source which has been disabled in
> > > +  enable register. Add thead_plic_chip which fix up c906-plic irq source completion
> > > +  problem by unmask/mask wrapper.
> > > +
> >
> > This is an incomplete description about how T-HEAD PLIC is different from
> > RISC-V PLIC.
> >
> > I would suggest the following:
> >
> > The T-HEAD C9xx SoC implements a modified/custom T-HEAD PLIC specification
> > which will mask current IRQ upon read to CLAIM register and will unmask the IRQ
> > upon write to CLAIM register. The thead,c900-plic compatible string
> > represents the
> > custom T-HEAD PLIC specification.
> The patch fixup the problem that when "thead,c900-plic" couldn't
> complete masked irq source which has been disabled.
>
> This patch is different from the last one in that there is no
> relationship with the auto-mask feature.

This patch adds compatible string for T-HEAD PLIC so it
should describe how T-HEAD PLIC is different from RISC-V
PLIC. The DT bindings document describes HW and not
the software work-around implemented using DT bindings.

Your irqchip patch uses T-HEAD PLIC compatible string to
implement a work-around.

In other words, this patch is different from the irqchip patch.

Regards,
Anup

>
> >
> > Regards,
> > Anup
> >
> > >  maintainers:
> > >    - Sagar Kadam <sagar.kadam@sifive.com>
> > >    - Paul Walmsley  <paul.walmsley@sifive.com>
> > > @@ -42,11 +46,16 @@ maintainers:
> > >
> > >  properties:
> > >    compatible:
> > > -    items:
> > > +   oneOf:
> > > +    - items:
> > >        - enum:
> > > -          - sifive,fu540-c000-plic
> > > -          - canaan,k210-plic
> > > +        - sifive,fu540-c000-plic
> > > +        - canaan,k210-plic
> > >        - const: sifive,plic-1.0.0
> > > +    - items:
> > > +      - enum:
> > > +        - allwinner,sun20i-d1-plic
> > > +      - const: thead,c900-plic
> > >
> > >    reg:
> > >      maxItems: 1
> > > --
> > > 2.25.1
> > >
>
>
>
> --
> Best Regards
>  Guo Ren
>
> ML: https://lore.kernel.org/linux-csky/
Guo Ren Oct. 24, 2021, 9:35 a.m. UTC | #4
On Sun, Oct 24, 2021 at 5:18 PM Anup Patel <anup@brainfault.org> wrote:
>
> On Sun, Oct 24, 2021 at 2:31 PM Guo Ren <guoren@kernel.org> wrote:
> >
> > On Sun, Oct 24, 2021 at 3:35 PM Anup Patel <anup@brainfault.org> wrote:
> > >
> > > On Sun, Oct 24, 2021 at 7:03 AM <guoren@kernel.org> wrote:
> > > >
> > > > From: Guo Ren <guoren@linux.alibaba.com>
> > > >
> > > > Add the compatible string "thead,c900-plic" to the riscv plic
> > > > bindings to support allwinner d1 SOC which contains c906 core.
> > > >
> > > > Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> > > > Cc: Anup Patel <anup@brainfault.org>
> > > > Cc: Atish Patra <atish.patra@wdc.com>
> > > > Cc: Heiko Stuebner <heiko@sntech.de>
> > > > Cc: Rob Herring <robh@kernel.org>
> > > > Cc: Rob Herring <robh+dt@kernel.org>
> > > > Cc: Palmer Dabbelt <palmerdabbelt@google.com>
> > > >
> > > > ---
> > > >
> > > > Changes since V5:
> > > >  - Add DT list
> > > >  - Fixup compatible string
> > > >  - Remove allwinner-d1 compatible
> > > >  - make dt_binding_check
> > > >
> > > > Changes since V4:
> > > >  - Update description in errata style
> > > >  - Update enum suggested by Anup, Heiko, Samuel
> > > >
> > > > Changes since V3:
> > > >  - Rename "c9xx" to "c900"
> > > >  - Add thead,c900-plic in the description section
> > > > ---
> > > >  .../interrupt-controller/sifive,plic-1.0.0.yaml   | 15 ++++++++++++---
> > > >  1 file changed, 12 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml
> > > > index 08d5a57ce00f..18b97bfd7954 100644
> > > > --- a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml
> > > > +++ b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml
> > > > @@ -35,6 +35,10 @@ description:
> > > >    contains a specific memory layout, which is documented in chapter 8 of the
> > > >    SiFive U5 Coreplex Series Manual <https://static.dev.sifive.com/U54-MC-RVCoreIP.pdf>.
> > > >
> > > > +  The thead,c900-plic couldn't complete masked irq source which has been disabled in
> > > > +  enable register. Add thead_plic_chip which fix up c906-plic irq source completion
> > > > +  problem by unmask/mask wrapper.
> > > > +
> > >
> > > This is an incomplete description about how T-HEAD PLIC is different from
> > > RISC-V PLIC.
> > >
> > > I would suggest the following:
> > >
> > > The T-HEAD C9xx SoC implements a modified/custom T-HEAD PLIC specification
> > > which will mask current IRQ upon read to CLAIM register and will unmask the IRQ
> > > upon write to CLAIM register. The thead,c900-plic compatible string
> > > represents the
> > > custom T-HEAD PLIC specification.
> > The patch fixup the problem that when "thead,c900-plic" couldn't
> > complete masked irq source which has been disabled.
> >
> > This patch is different from the last one in that there is no
> > relationship with the auto-mask feature.
>
> This patch adds compatible string for T-HEAD PLIC so it
> should describe how T-HEAD PLIC is different from RISC-V
> PLIC. The DT bindings document describes HW and not
> the software work-around implemented using DT bindings.
>
> Your irqchip patch uses T-HEAD PLIC compatible string to
> implement a work-around.
>
> In other words, this patch is different from the irqchip patch.

How about below:

The thead,c900-plic compatible string represents the custom T-HEAD
PLIC specification.
 - It couldn't complete masked irq source which has been disabled in
enable register. Add thead_plic_chip which fix up c906-plic irq source
completion problem by unmask/mask wrapper.
 - It implements a modified/custom T-HEAD PLIC specification which
will mask current IRQ upon read to CLAIM register and will unmask the
IRQ upon write to CLAIM register. But the feature wasn't utilized by
software.

>
> Regards,
> Anup
>
> >
> > >
> > > Regards,
> > > Anup
> > >
> > > >  maintainers:
> > > >    - Sagar Kadam <sagar.kadam@sifive.com>
> > > >    - Paul Walmsley  <paul.walmsley@sifive.com>
> > > > @@ -42,11 +46,16 @@ maintainers:
> > > >
> > > >  properties:
> > > >    compatible:
> > > > -    items:
> > > > +   oneOf:
> > > > +    - items:
> > > >        - enum:
> > > > -          - sifive,fu540-c000-plic
> > > > -          - canaan,k210-plic
> > > > +        - sifive,fu540-c000-plic
> > > > +        - canaan,k210-plic
> > > >        - const: sifive,plic-1.0.0
> > > > +    - items:
> > > > +      - enum:
> > > > +        - allwinner,sun20i-d1-plic
> > > > +      - const: thead,c900-plic
> > > >
> > > >    reg:
> > > >      maxItems: 1
> > > > --
> > > > 2.25.1
> > > >
> >
> >
> >
> > --
> > Best Regards
> >  Guo Ren
> >
> > ML: https://lore.kernel.org/linux-csky/
Anup Patel Oct. 24, 2021, 9:52 a.m. UTC | #5
On Sun, Oct 24, 2021 at 3:05 PM Guo Ren <guoren@kernel.org> wrote:
>
> On Sun, Oct 24, 2021 at 5:18 PM Anup Patel <anup@brainfault.org> wrote:
> >
> > On Sun, Oct 24, 2021 at 2:31 PM Guo Ren <guoren@kernel.org> wrote:
> > >
> > > On Sun, Oct 24, 2021 at 3:35 PM Anup Patel <anup@brainfault.org> wrote:
> > > >
> > > > On Sun, Oct 24, 2021 at 7:03 AM <guoren@kernel.org> wrote:
> > > > >
> > > > > From: Guo Ren <guoren@linux.alibaba.com>
> > > > >
> > > > > Add the compatible string "thead,c900-plic" to the riscv plic
> > > > > bindings to support allwinner d1 SOC which contains c906 core.
> > > > >
> > > > > Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> > > > > Cc: Anup Patel <anup@brainfault.org>
> > > > > Cc: Atish Patra <atish.patra@wdc.com>
> > > > > Cc: Heiko Stuebner <heiko@sntech.de>
> > > > > Cc: Rob Herring <robh@kernel.org>
> > > > > Cc: Rob Herring <robh+dt@kernel.org>
> > > > > Cc: Palmer Dabbelt <palmerdabbelt@google.com>
> > > > >
> > > > > ---
> > > > >
> > > > > Changes since V5:
> > > > >  - Add DT list
> > > > >  - Fixup compatible string
> > > > >  - Remove allwinner-d1 compatible
> > > > >  - make dt_binding_check
> > > > >
> > > > > Changes since V4:
> > > > >  - Update description in errata style
> > > > >  - Update enum suggested by Anup, Heiko, Samuel
> > > > >
> > > > > Changes since V3:
> > > > >  - Rename "c9xx" to "c900"
> > > > >  - Add thead,c900-plic in the description section
> > > > > ---
> > > > >  .../interrupt-controller/sifive,plic-1.0.0.yaml   | 15 ++++++++++++---
> > > > >  1 file changed, 12 insertions(+), 3 deletions(-)
> > > > >
> > > > > diff --git a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml
> > > > > index 08d5a57ce00f..18b97bfd7954 100644
> > > > > --- a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml
> > > > > +++ b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml
> > > > > @@ -35,6 +35,10 @@ description:
> > > > >    contains a specific memory layout, which is documented in chapter 8 of the
> > > > >    SiFive U5 Coreplex Series Manual <https://static.dev.sifive.com/U54-MC-RVCoreIP.pdf>.
> > > > >
> > > > > +  The thead,c900-plic couldn't complete masked irq source which has been disabled in
> > > > > +  enable register. Add thead_plic_chip which fix up c906-plic irq source completion
> > > > > +  problem by unmask/mask wrapper.
> > > > > +
> > > >
> > > > This is an incomplete description about how T-HEAD PLIC is different from
> > > > RISC-V PLIC.
> > > >
> > > > I would suggest the following:
> > > >
> > > > The T-HEAD C9xx SoC implements a modified/custom T-HEAD PLIC specification
> > > > which will mask current IRQ upon read to CLAIM register and will unmask the IRQ
> > > > upon write to CLAIM register. The thead,c900-plic compatible string
> > > > represents the
> > > > custom T-HEAD PLIC specification.
> > > The patch fixup the problem that when "thead,c900-plic" couldn't
> > > complete masked irq source which has been disabled.
> > >
> > > This patch is different from the last one in that there is no
> > > relationship with the auto-mask feature.
> >
> > This patch adds compatible string for T-HEAD PLIC so it
> > should describe how T-HEAD PLIC is different from RISC-V
> > PLIC. The DT bindings document describes HW and not
> > the software work-around implemented using DT bindings.
> >
> > Your irqchip patch uses T-HEAD PLIC compatible string to
> > implement a work-around.
> >
> > In other words, this patch is different from the irqchip patch.
>
> How about below:
>
> The thead,c900-plic compatible string represents the custom T-HEAD
> PLIC specification.
>  - It couldn't complete masked irq source which has been disabled in
> enable register. Add thead_plic_chip which fix up c906-plic irq source
> completion problem by unmask/mask wrapper.

This first bullet is not required because it describes how it is used
in irqchip driver to fix issues. This info has to go in your driver fix
patch.

>  - It implements a modified/custom T-HEAD PLIC specification which
> will mask current IRQ upon read to CLAIM register and will unmask the
> IRQ upon write to CLAIM register. But the feature wasn't utilized by
> software.

Please don't advertise non-compliance with RISC-V PLIC spec as feature.

What I had suggest before seems better.

Regards,
Anup

>
> >
> > Regards,
> > Anup
> >
> > >
> > > >
> > > > Regards,
> > > > Anup
> > > >
> > > > >  maintainers:
> > > > >    - Sagar Kadam <sagar.kadam@sifive.com>
> > > > >    - Paul Walmsley  <paul.walmsley@sifive.com>
> > > > > @@ -42,11 +46,16 @@ maintainers:
> > > > >
> > > > >  properties:
> > > > >    compatible:
> > > > > -    items:
> > > > > +   oneOf:
> > > > > +    - items:
> > > > >        - enum:
> > > > > -          - sifive,fu540-c000-plic
> > > > > -          - canaan,k210-plic
> > > > > +        - sifive,fu540-c000-plic
> > > > > +        - canaan,k210-plic
> > > > >        - const: sifive,plic-1.0.0
> > > > > +    - items:
> > > > > +      - enum:
> > > > > +        - allwinner,sun20i-d1-plic
> > > > > +      - const: thead,c900-plic
> > > > >
> > > > >    reg:
> > > > >      maxItems: 1
> > > > > --
> > > > > 2.25.1
> > > > >
> > >
> > >
> > >
> > > --
> > > Best Regards
> > >  Guo Ren
> > >
> > > ML: https://lore.kernel.org/linux-csky/
>
>
>
> --
> Best Regards
>  Guo Ren
>
> ML: https://lore.kernel.org/linux-csky/
Guo Ren Oct. 24, 2021, 10:04 a.m. UTC | #6
On Sun, Oct 24, 2021 at 5:53 PM Anup Patel <anup@brainfault.org> wrote:
>
> On Sun, Oct 24, 2021 at 3:05 PM Guo Ren <guoren@kernel.org> wrote:
> >
> > On Sun, Oct 24, 2021 at 5:18 PM Anup Patel <anup@brainfault.org> wrote:
> > >
> > > On Sun, Oct 24, 2021 at 2:31 PM Guo Ren <guoren@kernel.org> wrote:
> > > >
> > > > On Sun, Oct 24, 2021 at 3:35 PM Anup Patel <anup@brainfault.org> wrote:
> > > > >
> > > > > On Sun, Oct 24, 2021 at 7:03 AM <guoren@kernel.org> wrote:
> > > > > >
> > > > > > From: Guo Ren <guoren@linux.alibaba.com>
> > > > > >
> > > > > > Add the compatible string "thead,c900-plic" to the riscv plic
> > > > > > bindings to support allwinner d1 SOC which contains c906 core.
> > > > > >
> > > > > > Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> > > > > > Cc: Anup Patel <anup@brainfault.org>
> > > > > > Cc: Atish Patra <atish.patra@wdc.com>
> > > > > > Cc: Heiko Stuebner <heiko@sntech.de>
> > > > > > Cc: Rob Herring <robh@kernel.org>
> > > > > > Cc: Rob Herring <robh+dt@kernel.org>
> > > > > > Cc: Palmer Dabbelt <palmerdabbelt@google.com>
> > > > > >
> > > > > > ---
> > > > > >
> > > > > > Changes since V5:
> > > > > >  - Add DT list
> > > > > >  - Fixup compatible string
> > > > > >  - Remove allwinner-d1 compatible
> > > > > >  - make dt_binding_check
> > > > > >
> > > > > > Changes since V4:
> > > > > >  - Update description in errata style
> > > > > >  - Update enum suggested by Anup, Heiko, Samuel
> > > > > >
> > > > > > Changes since V3:
> > > > > >  - Rename "c9xx" to "c900"
> > > > > >  - Add thead,c900-plic in the description section
> > > > > > ---
> > > > > >  .../interrupt-controller/sifive,plic-1.0.0.yaml   | 15 ++++++++++++---
> > > > > >  1 file changed, 12 insertions(+), 3 deletions(-)
> > > > > >
> > > > > > diff --git a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml
> > > > > > index 08d5a57ce00f..18b97bfd7954 100644
> > > > > > --- a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml
> > > > > > +++ b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml
> > > > > > @@ -35,6 +35,10 @@ description:
> > > > > >    contains a specific memory layout, which is documented in chapter 8 of the
> > > > > >    SiFive U5 Coreplex Series Manual <https://static.dev.sifive.com/U54-MC-RVCoreIP.pdf>.
> > > > > >
> > > > > > +  The thead,c900-plic couldn't complete masked irq source which has been disabled in
> > > > > > +  enable register. Add thead_plic_chip which fix up c906-plic irq source completion
> > > > > > +  problem by unmask/mask wrapper.
> > > > > > +
> > > > >
> > > > > This is an incomplete description about how T-HEAD PLIC is different from
> > > > > RISC-V PLIC.
> > > > >
> > > > > I would suggest the following:
> > > > >
> > > > > The T-HEAD C9xx SoC implements a modified/custom T-HEAD PLIC specification
> > > > > which will mask current IRQ upon read to CLAIM register and will unmask the IRQ
> > > > > upon write to CLAIM register. The thead,c900-plic compatible string
> > > > > represents the
> > > > > custom T-HEAD PLIC specification.
> > > > The patch fixup the problem that when "thead,c900-plic" couldn't
> > > > complete masked irq source which has been disabled.
> > > >
> > > > This patch is different from the last one in that there is no
> > > > relationship with the auto-mask feature.
> > >
> > > This patch adds compatible string for T-HEAD PLIC so it
> > > should describe how T-HEAD PLIC is different from RISC-V
> > > PLIC. The DT bindings document describes HW and not
> > > the software work-around implemented using DT bindings.
> > >
> > > Your irqchip patch uses T-HEAD PLIC compatible string to
> > > implement a work-around.
> > >
> > > In other words, this patch is different from the irqchip patch.
> >
> > How about below:
> >
> > The thead,c900-plic compatible string represents the custom T-HEAD
> > PLIC specification.
> >  - It couldn't complete masked irq source which has been disabled in
> > enable register. Add thead_plic_chip which fix up c906-plic irq source
> > completion problem by unmask/mask wrapper.
>
> This first bullet is not required because it describes how it is used
> in irqchip driver to fix issues. This info has to go in your driver fix
> patch.
>
> >  - It implements a modified/custom T-HEAD PLIC specification which
> > will mask current IRQ upon read to CLAIM register and will unmask the
> > IRQ upon write to CLAIM register. But the feature wasn't utilized by
> > software.
>
> Please don't advertise non-compliance with RISC-V PLIC spec as feature.
>
> What I had suggest before seems better.
Okay, just put these in the description in the next version.

The T-HEAD C9xx SoC implements a modified/custom T-HEAD PLIC
specification which will mask current IRQ upon read to CLAIM register
and will unmask the IRQ upon write to CLAIM register. The
thead,c900-plic compatible string represents the custom T-HEAD PLIC
specification.

>
> Regards,
> Anup
>
> >
> > >
> > > Regards,
> > > Anup
> > >
> > > >
> > > > >
> > > > > Regards,
> > > > > Anup
> > > > >
> > > > > >  maintainers:
> > > > > >    - Sagar Kadam <sagar.kadam@sifive.com>
> > > > > >    - Paul Walmsley  <paul.walmsley@sifive.com>
> > > > > > @@ -42,11 +46,16 @@ maintainers:
> > > > > >
> > > > > >  properties:
> > > > > >    compatible:
> > > > > > -    items:
> > > > > > +   oneOf:
> > > > > > +    - items:
> > > > > >        - enum:
> > > > > > -          - sifive,fu540-c000-plic
> > > > > > -          - canaan,k210-plic
> > > > > > +        - sifive,fu540-c000-plic
> > > > > > +        - canaan,k210-plic
> > > > > >        - const: sifive,plic-1.0.0
> > > > > > +    - items:
> > > > > > +      - enum:
> > > > > > +        - allwinner,sun20i-d1-plic
> > > > > > +      - const: thead,c900-plic
> > > > > >
> > > > > >    reg:
> > > > > >      maxItems: 1
> > > > > > --
> > > > > > 2.25.1
> > > > > >
> > > >
> > > >
> > > >
> > > > --
> > > > Best Regards
> > > >  Guo Ren
> > > >
> > > > ML: https://lore.kernel.org/linux-csky/
> >
> >
> >
> > --
> > Best Regards
> >  Guo Ren
> >
> > ML: https://lore.kernel.org/linux-csky/
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml
index 08d5a57ce00f..18b97bfd7954 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml
+++ b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml
@@ -35,6 +35,10 @@  description:
   contains a specific memory layout, which is documented in chapter 8 of the
   SiFive U5 Coreplex Series Manual <https://static.dev.sifive.com/U54-MC-RVCoreIP.pdf>.
 
+  The thead,c900-plic couldn't complete masked irq source which has been disabled in
+  enable register. Add thead_plic_chip which fix up c906-plic irq source completion
+  problem by unmask/mask wrapper.
+
 maintainers:
   - Sagar Kadam <sagar.kadam@sifive.com>
   - Paul Walmsley  <paul.walmsley@sifive.com>
@@ -42,11 +46,16 @@  maintainers:
 
 properties:
   compatible:
-    items:
+   oneOf:
+    - items:
       - enum:
-          - sifive,fu540-c000-plic
-          - canaan,k210-plic
+        - sifive,fu540-c000-plic
+        - canaan,k210-plic
       - const: sifive,plic-1.0.0
+    - items:
+      - enum:
+        - allwinner,sun20i-d1-plic
+      - const: thead,c900-plic
 
   reg:
     maxItems: 1