[PATCH V3 1/2] dt-bindings: update riscv plic compatible string

[PATCH V3 1/2] dt-bindings: update riscv plic compatible string

[guoren]

From: Guo Ren <guoren@linux.alibaba.com>

Add the compatible string "thead,c900-plic" to the riscv plic
bindings to support SOCs with thead,c9xx processor cores.

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

---

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

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..82629832e5a5 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,11 @@ 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>.
 
+  While the "thead,c900-plic" would mask IRQ with readl(claim), so it needn't
+  mask/unmask which needed in RISC-V PLIC. When in IRQS_ONESHOT & IRQCHIP_EOI_THREADED
+  path, unnecessary mask operation would cause a blocking irq bug in thead,c900-plic.
+  Because when IRQ is disabled in c900, writel(hwirq, claim) would be invalid.
+
 maintainers:
   - Sagar Kadam <sagar.kadam@sifive.com>
   - Paul Walmsley  <paul.walmsley@sifive.com>
@@ -46,6 +51,7 @@ properties:
       - enum:
           - sifive,fu540-c000-plic
           - canaan,k210-plic
+          - thead,c900-plic
       - const: sifive,plic-1.0.0
 
   reg:
-- 
2.25.1

[PATCH V3 2/2] irqchip/sifive-plic: Add thead,c900-plic support

[guoren]

From: Guo Ren <guoren@linux.alibaba.com>

thead,c900-plic would mask IRQ with readl(claim), so it needn't
mask/unmask which needed in RISC-V PLIC.

When in IRQS_ONESHOT & IRQCHIP_EOI_THREADED path, unnecessary mask
operation would cause a blocking irq bug in thead,c900-plic. Because
when IRQ is disabled in c900, writel(hwirq, claim) would be invalid.

Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
Cc: Anup Patel <anup@brainfault.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Marc Zyngier <maz@kernel.org>
Cc: Palmer Dabbelt <palmer@dabbelt.com>
Cc: Atish Patra <atish.patra@wdc.com>

---

Changes since V3:
 - Rename "c9xx" to "c900"
 - Add sifive_plic_chip and thead_plic_chip for difference

Changes since V2:
 - Add a separate compatible string "thead,c9xx-plic"
 - set irq_mask/unmask of "plic_chip" to NULL and point
   irq_enable/disable of "plic_chip" to plic_irq_mask/unmask
 - Add a detailed comment block in plic_init() about the
   differences in Claim/Completion process of RISC-V PLIC and C9xx
   PLIC.
---
 drivers/irqchip/irq-sifive-plic.c | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c
index cf74cfa82045..5b806d823df7 100644
--- a/drivers/irqchip/irq-sifive-plic.c
+++ b/drivers/irqchip/irq-sifive-plic.c
@@ -166,7 +166,7 @@ static void plic_irq_eoi(struct irq_data *d)
 	writel(d->hwirq, handler->hart_base + CONTEXT_CLAIM);
 }
 
-static struct irq_chip plic_chip = {
+static struct irq_chip sifive_plic_chip = {
 	.name		= "SiFive PLIC",
 	.irq_mask	= plic_irq_mask,
 	.irq_unmask	= plic_irq_unmask,
@@ -176,12 +176,24 @@ static struct irq_chip plic_chip = {
 #endif
 };
 
+static struct irq_chip thead_plic_chip = {
+	.name		= "T-Head PLIC",
+	.irq_disable	= plic_irq_mask,
+	.irq_enable	= plic_irq_unmask,
+	.irq_eoi	= plic_irq_eoi,
+#ifdef CONFIG_SMP
+	.irq_set_affinity = plic_set_affinity,
+#endif
+};
+
+static struct irq_chip *def_plic_chip = &sifive_plic_chip;
+
 static int plic_irqdomain_map(struct irq_domain *d, unsigned int irq,
 			      irq_hw_number_t hwirq)
 {
 	struct plic_priv *priv = d->host_data;
 
-	irq_domain_set_info(d, irq, hwirq, &plic_chip, d->host_data,
+	irq_domain_set_info(d, irq, hwirq, def_plic_chip, d->host_data,
 			    handle_fasteoi_irq, NULL, NULL);
 	irq_set_noprobe(irq);
 	irq_set_affinity(irq, &priv->lmask);
@@ -390,5 +402,14 @@ static int __init plic_init(struct device_node *node,
 	return error;
 }
 
+static int __init thead_c900_plic_init(struct device_node *node,
+		struct device_node *parent)
+{
+	def_plic_chip = &thead_plic_chip;
+
+	return plic_init(node, parent);
+}
+
 IRQCHIP_DECLARE(sifive_plic, "sifive,plic-1.0.0", plic_init);
 IRQCHIP_DECLARE(riscv_plic0, "riscv,plic0", plic_init); /* for legacy systems */
+IRQCHIP_DECLARE(thead_c900_plic, "thead,c900-plic", thead_c900_plic_init);
-- 
2.25.1

Re: [PATCH V3 2/2] irqchip/sifive-plic: Add thead,c900-plic support

[Anup Patel]

On Wed, Oct 13, 2021 at 6:52 AM <guoren@kernel.org> wrote:
>
> From: Guo Ren <guoren@linux.alibaba.com>
>
> thead,c900-plic would mask IRQ with readl(claim), so it needn't
> mask/unmask which needed in RISC-V PLIC.
>
> When in IRQS_ONESHOT & IRQCHIP_EOI_THREADED path, unnecessary mask
> operation would cause a blocking irq bug in thead,c900-plic. Because
> when IRQ is disabled in c900, writel(hwirq, claim) would be invalid.

This is a totally incorrect description.

Instead of this, the commit description should say the following:

1) The irq_mask/unmask() is used by handle_fasteoi_irq() is mostly
for ONESHOT irqs and there is no limitation in the RISC-V PLIC driver
due to use of irq_mask/unmask() callbacks. In fact, a lot of irqchip
drivers using handle_fasteoi_irq() also implement irq_mask/unmask().

2) The C9xx PLIC does not comply with the interrupt claim/completion
process defined by the RISC-V PLIC specification because C9xx PLIC
will mask an IRQ when it is claimed by PLIC driver (i.e. readl(claim) and
the IRQ will be unmasked upon completion by PLIC driver (i.e. writel(claim).
This behaviour breaks the handling of IRQS_ONESHOT by the generic
handle_fasteoi_irq() used in the PLIC driver.

3) This patch adds an errata fix for IRQS_ONESHOT handling on
C9xx PLIC by using irq_enable/disable() callbacks instead of
irq_mask/unmask().

In general, non-compliance of the C9xx PLIC should be treated as
an errata so please don't project it as a feature.

>
> Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> Cc: Anup Patel <anup@brainfault.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: Palmer Dabbelt <palmer@dabbelt.com>
> Cc: Atish Patra <atish.patra@wdc.com>
>
> ---
>
> Changes since V3:
>  - Rename "c9xx" to "c900"
>  - Add sifive_plic_chip and thead_plic_chip for difference
>
> Changes since V2:
>  - Add a separate compatible string "thead,c9xx-plic"
>  - set irq_mask/unmask of "plic_chip" to NULL and point
>    irq_enable/disable of "plic_chip" to plic_irq_mask/unmask
>  - Add a detailed comment block in plic_init() about the
>    differences in Claim/Completion process of RISC-V PLIC and C9xx
>    PLIC.
> ---
>  drivers/irqchip/irq-sifive-plic.c | 25 +++++++++++++++++++++++--
>  1 file changed, 23 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c
> index cf74cfa82045..5b806d823df7 100644
> --- a/drivers/irqchip/irq-sifive-plic.c
> +++ b/drivers/irqchip/irq-sifive-plic.c
> @@ -166,7 +166,7 @@ static void plic_irq_eoi(struct irq_data *d)
>         writel(d->hwirq, handler->hart_base + CONTEXT_CLAIM);
>  }
>
> -static struct irq_chip plic_chip = {
> +static struct irq_chip sifive_plic_chip = {
>         .name           = "SiFive PLIC",
>         .irq_mask       = plic_irq_mask,
>         .irq_unmask     = plic_irq_unmask,
> @@ -176,12 +176,24 @@ static struct irq_chip plic_chip = {
>  #endif
>  };
>

Please add a detailed comment block as described by point#2
above.

> +static struct irq_chip thead_plic_chip = {
> +       .name           = "T-Head PLIC",
> +       .irq_disable    = plic_irq_mask,
> +       .irq_enable     = plic_irq_unmask,
> +       .irq_eoi        = plic_irq_eoi,
> +#ifdef CONFIG_SMP
> +       .irq_set_affinity = plic_set_affinity,
> +#endif
> +};
> +
> +static struct irq_chip *def_plic_chip = &sifive_plic_chip;
> +
>  static int plic_irqdomain_map(struct irq_domain *d, unsigned int irq,
>                               irq_hw_number_t hwirq)
>  {
>         struct plic_priv *priv = d->host_data;
>
> -       irq_domain_set_info(d, irq, hwirq, &plic_chip, d->host_data,
> +       irq_domain_set_info(d, irq, hwirq, def_plic_chip, d->host_data,
>                             handle_fasteoi_irq, NULL, NULL);
>         irq_set_noprobe(irq);
>         irq_set_affinity(irq, &priv->lmask);
> @@ -390,5 +402,14 @@ static int __init plic_init(struct device_node *node,
>         return error;
>  }
>
> +static int __init thead_c900_plic_init(struct device_node *node,
> +               struct device_node *parent)
> +{
> +       def_plic_chip = &thead_plic_chip;
> +
> +       return plic_init(node, parent);
> +}
> +
>  IRQCHIP_DECLARE(sifive_plic, "sifive,plic-1.0.0", plic_init);
>  IRQCHIP_DECLARE(riscv_plic0, "riscv,plic0", plic_init); /* for legacy systems */
> +IRQCHIP_DECLARE(thead_c900_plic, "thead,c900-plic", thead_c900_plic_init);
> --
> 2.25.1
>

Regards,
Anup

Re: [PATCH V3 1/2] dt-bindings: update riscv plic compatible string

[Anup Patel]

On Wed, Oct 13, 2021 at 6:52 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 SOCs with thead,c9xx processor cores.
>
> Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Palmer Dabbelt <palmerdabbelt@google.com>
> Cc: Anup Patel <anup@brainfault.org>
> Cc: Atish Patra <atish.patra@wdc.com>
>
> ---
>
> Changes since V3:
>  - Rename "c9xx" to "c900"
>  - Add thead,c900-plic in the description section
> ---
>  .../bindings/interrupt-controller/sifive,plic-1.0.0.yaml    | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> 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..82629832e5a5 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,11 @@ 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>.
>
> +  While the "thead,c900-plic" would mask IRQ with readl(claim), so it needn't
> +  mask/unmask which needed in RISC-V PLIC. When in IRQS_ONESHOT & IRQCHIP_EOI_THREADED
> +  path, unnecessary mask operation would cause a blocking irq bug in thead,c900-plic.
> +  Because when IRQ is disabled in c900, writel(hwirq, claim) would be invalid.

This is a totally incorrect description of the errata required for C9xx PLIC.

Please don't project non-compliance as a feature of C9xx PLIC.

> +
>  maintainers:
>    - Sagar Kadam <sagar.kadam@sifive.com>
>    - Paul Walmsley  <paul.walmsley@sifive.com>
> @@ -46,6 +51,7 @@ properties:
>        - enum:
>            - sifive,fu540-c000-plic
>            - canaan,k210-plic
> +          - thead,c900-plic
>        - const: sifive,plic-1.0.0

The PLIC DT node requires two compatible string:
<implementation_compat>, <spec_compat>

The C9xx PLIC is not RISC-V PLIC so, the DT node should
be: "thead,c900-plic", "thead,c9xx-plic"

You need to change "- const: sifive,plic-1.0.0" to
- enum:
    - sifive,plic-1.0.0
    - thead,c9xx-plic

>
>    reg:
> --
> 2.25.1
>

Regards,
Anup

Re: [PATCH V3 2/2] irqchip/sifive-plic: Add thead,c900-plic support

[Guo Ren]

On Wed, Oct 13, 2021 at 1:04 PM Anup Patel <anup@brainfault.org> wrote:
>
> On Wed, Oct 13, 2021 at 6:52 AM <guoren@kernel.org> wrote:
> >
> > From: Guo Ren <guoren@linux.alibaba.com>
> >
> > thead,c900-plic would mask IRQ with readl(claim), so it needn't
> > mask/unmask which needed in RISC-V PLIC.
> >
> > When in IRQS_ONESHOT & IRQCHIP_EOI_THREADED path, unnecessary mask
> > operation would cause a blocking irq bug in thead,c900-plic. Because
> > when IRQ is disabled in c900, writel(hwirq, claim) would be invalid.
>
> This is a totally incorrect description.
>
> Instead of this, the commit description should say the following:
>
> 1) The irq_mask/unmask() is used by handle_fasteoi_irq() is mostly
> for ONESHOT irqs and there is no limitation in the RISC-V PLIC driver
> due to use of irq_mask/unmask() callbacks. In fact, a lot of irqchip
> drivers using handle_fasteoi_irq() also implement irq_mask/unmask().
Add irq_mask/unmask in handle_fasteoi_irq() would decrease the
performance, we also need to add this fact in the description.
>
> 2) The C9xx PLIC does not comply with the interrupt claim/completion
> process defined by the RISC-V PLIC specification because C9xx PLIC
> will mask an IRQ when it is claimed by PLIC driver (i.e. readl(claim) and
> the IRQ will be unmasked upon completion by PLIC driver (i.e. writel(claim).
> This behaviour breaks the handling of IRQS_ONESHOT by the generic
> handle_fasteoi_irq() used in the PLIC driver.
>
> 3) This patch adds an errata fix for IRQS_ONESHOT handling on
> C9xx PLIC by using irq_enable/disable() callbacks instead of
> irq_mask/unmask().
>
> In general, non-compliance of the C9xx PLIC should be treated as
> an errata so please don't project it as a feature.
>
> >
> > Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> > Cc: Anup Patel <anup@brainfault.org>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: Marc Zyngier <maz@kernel.org>
> > Cc: Palmer Dabbelt <palmer@dabbelt.com>
> > Cc: Atish Patra <atish.patra@wdc.com>
> >
> > ---
> >
> > Changes since V3:
> >  - Rename "c9xx" to "c900"
> >  - Add sifive_plic_chip and thead_plic_chip for difference
> >
> > Changes since V2:
> >  - Add a separate compatible string "thead,c9xx-plic"
> >  - set irq_mask/unmask of "plic_chip" to NULL and point
> >    irq_enable/disable of "plic_chip" to plic_irq_mask/unmask
> >  - Add a detailed comment block in plic_init() about the
> >    differences in Claim/Completion process of RISC-V PLIC and C9xx
> >    PLIC.
> > ---
> >  drivers/irqchip/irq-sifive-plic.c | 25 +++++++++++++++++++++++--
> >  1 file changed, 23 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c
> > index cf74cfa82045..5b806d823df7 100644
> > --- a/drivers/irqchip/irq-sifive-plic.c
> > +++ b/drivers/irqchip/irq-sifive-plic.c
> > @@ -166,7 +166,7 @@ static void plic_irq_eoi(struct irq_data *d)
> >         writel(d->hwirq, handler->hart_base + CONTEXT_CLAIM);
> >  }
> >
> > -static struct irq_chip plic_chip = {
> > +static struct irq_chip sifive_plic_chip = {
> >         .name           = "SiFive PLIC",
> >         .irq_mask       = plic_irq_mask,
> >         .irq_unmask     = plic_irq_unmask,
> > @@ -176,12 +176,24 @@ static struct irq_chip plic_chip = {
> >  #endif
> >  };
> >
>
> Please add a detailed comment block as described by point#2
> above.
>
> > +static struct irq_chip thead_plic_chip = {
> > +       .name           = "T-Head PLIC",
> > +       .irq_disable    = plic_irq_mask,
> > +       .irq_enable     = plic_irq_unmask,
> > +       .irq_eoi        = plic_irq_eoi,
> > +#ifdef CONFIG_SMP
> > +       .irq_set_affinity = plic_set_affinity,
> > +#endif
> > +};
> > +
> > +static struct irq_chip *def_plic_chip = &sifive_plic_chip;
> > +
> >  static int plic_irqdomain_map(struct irq_domain *d, unsigned int irq,
> >                               irq_hw_number_t hwirq)
> >  {
> >         struct plic_priv *priv = d->host_data;
> >
> > -       irq_domain_set_info(d, irq, hwirq, &plic_chip, d->host_data,
> > +       irq_domain_set_info(d, irq, hwirq, def_plic_chip, d->host_data,
> >                             handle_fasteoi_irq, NULL, NULL);
> >         irq_set_noprobe(irq);
> >         irq_set_affinity(irq, &priv->lmask);
> > @@ -390,5 +402,14 @@ static int __init plic_init(struct device_node *node,
> >         return error;
> >  }
> >
> > +static int __init thead_c900_plic_init(struct device_node *node,
> > +               struct device_node *parent)
> > +{
> > +       def_plic_chip = &thead_plic_chip;
> > +
> > +       return plic_init(node, parent);
> > +}
> > +
> >  IRQCHIP_DECLARE(sifive_plic, "sifive,plic-1.0.0", plic_init);
> >  IRQCHIP_DECLARE(riscv_plic0, "riscv,plic0", plic_init); /* for legacy systems */
> > +IRQCHIP_DECLARE(thead_c900_plic, "thead,c900-plic", thead_c900_plic_init);
> > --
> > 2.25.1
> >
>
> Regards,
> Anup



-- 
Best Regards
 Guo Ren

ML: https://lore.kernel.org/linux-csky/

Re: [PATCH V3 1/2] dt-bindings: update riscv plic compatible string

[Heiko Stübner]

Hi Anup,

Am Mittwoch, 13. Oktober 2021, 07:11:46 CEST schrieb Anup Patel:
> On Wed, Oct 13, 2021 at 6:52 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 SOCs with thead,c9xx processor cores.
> >
> > Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> > Cc: Rob Herring <robh@kernel.org>
> > Cc: Palmer Dabbelt <palmerdabbelt@google.com>
> > Cc: Anup Patel <anup@brainfault.org>
> > Cc: Atish Patra <atish.patra@wdc.com>
> >
> > ---
> >
> > Changes since V3:
> >  - Rename "c9xx" to "c900"
> >  - Add thead,c900-plic in the description section
> > ---
> >  .../bindings/interrupt-controller/sifive,plic-1.0.0.yaml    | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > 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..82629832e5a5 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,11 @@ 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>.
> >
> > +  While the "thead,c900-plic" would mask IRQ with readl(claim), so it needn't
> > +  mask/unmask which needed in RISC-V PLIC. When in IRQS_ONESHOT & IRQCHIP_EOI_THREADED
> > +  path, unnecessary mask operation would cause a blocking irq bug in thead,c900-plic.
> > +  Because when IRQ is disabled in c900, writel(hwirq, claim) would be invalid.
> 
> This is a totally incorrect description of the errata required for C9xx PLIC.
> 
> Please don't project non-compliance as a feature of C9xx PLIC.
> 
> > +
> >  maintainers:
> >    - Sagar Kadam <sagar.kadam@sifive.com>
> >    - Paul Walmsley  <paul.walmsley@sifive.com>
> > @@ -46,6 +51,7 @@ properties:
> >        - enum:
> >            - sifive,fu540-c000-plic
> >            - canaan,k210-plic
> > +          - thead,c900-plic

we still want specific SoC names in the compatible, the "c900"
is still a sort-of placeholder.


> >        - const: sifive,plic-1.0.0
> 
> The PLIC DT node requires two compatible string:
> <implementation_compat>, <spec_compat>
> 
> The C9xx PLIC is not RISC-V PLIC so, the DT node should
> be: "thead,c900-plic", "thead,c9xx-plic"
> 
> You need to change "- const: sifive,plic-1.0.0" to
> - enum:
>     - sifive,plic-1.0.0
>     - thead,c9xx-plic

Re: [PATCH V3 1/2] dt-bindings: update riscv plic compatible string

[Anup Patel]

On Wed, Oct 13, 2021 at 2:27 PM Heiko Stübner <heiko@sntech.de> wrote:
>
> Hi Anup,
>
> Am Mittwoch, 13. Oktober 2021, 07:11:46 CEST schrieb Anup Patel:
> > On Wed, Oct 13, 2021 at 6:52 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 SOCs with thead,c9xx processor cores.
> > >
> > > Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> > > Cc: Rob Herring <robh@kernel.org>
> > > Cc: Palmer Dabbelt <palmerdabbelt@google.com>
> > > Cc: Anup Patel <anup@brainfault.org>
> > > Cc: Atish Patra <atish.patra@wdc.com>
> > >
> > > ---
> > >
> > > Changes since V3:
> > >  - Rename "c9xx" to "c900"
> > >  - Add thead,c900-plic in the description section
> > > ---
> > >  .../bindings/interrupt-controller/sifive,plic-1.0.0.yaml    | 6 ++++++
> > >  1 file changed, 6 insertions(+)
> > >
> > > 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..82629832e5a5 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,11 @@ 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>.
> > >
> > > +  While the "thead,c900-plic" would mask IRQ with readl(claim), so it needn't
> > > +  mask/unmask which needed in RISC-V PLIC. When in IRQS_ONESHOT & IRQCHIP_EOI_THREADED
> > > +  path, unnecessary mask operation would cause a blocking irq bug in thead,c900-plic.
> > > +  Because when IRQ is disabled in c900, writel(hwirq, claim) would be invalid.
> >
> > This is a totally incorrect description of the errata required for C9xx PLIC.
> >
> > Please don't project non-compliance as a feature of C9xx PLIC.
> >
> > > +
> > >  maintainers:
> > >    - Sagar Kadam <sagar.kadam@sifive.com>
> > >    - Paul Walmsley  <paul.walmsley@sifive.com>
> > > @@ -46,6 +51,7 @@ properties:
> > >        - enum:
> > >            - sifive,fu540-c000-plic
> > >            - canaan,k210-plic
> > > +          - thead,c900-plic
>
> we still want specific SoC names in the compatible, the "c900"
> is still a sort-of placeholder.

Yes, we need "c900" compatible string as well. The "c9xx"
compatible string is for the custom PLIC spec followed by T-HEAD.

>
>
> > >        - const: sifive,plic-1.0.0
> >
> > The PLIC DT node requires two compatible string:
> > <implementation_compat>, <spec_compat>
> >
> > The C9xx PLIC is not RISC-V PLIC so, the DT node should
> > be: "thead,c900-plic", "thead,c9xx-plic"
> >
> > You need to change "- const: sifive,plic-1.0.0" to
> > - enum:
> >     - sifive,plic-1.0.0
> >     - thead,c9xx-plic
>
>

Regards,
Anup

Re: [PATCH V3 1/2] dt-bindings: update riscv plic compatible string

[Heiko Stübner]

Am Mittwoch, 13. Oktober 2021, 11:11:26 CEST schrieb Anup Patel:
> On Wed, Oct 13, 2021 at 2:27 PM Heiko Stübner <heiko@sntech.de> wrote:
> >
> > Hi Anup,
> >
> > Am Mittwoch, 13. Oktober 2021, 07:11:46 CEST schrieb Anup Patel:
> > > On Wed, Oct 13, 2021 at 6:52 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 SOCs with thead,c9xx processor cores.
> > > >
> > > > Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> > > > Cc: Rob Herring <robh@kernel.org>
> > > > Cc: Palmer Dabbelt <palmerdabbelt@google.com>
> > > > Cc: Anup Patel <anup@brainfault.org>
> > > > Cc: Atish Patra <atish.patra@wdc.com>
> > > >
> > > > ---
> > > >
> > > > Changes since V3:
> > > >  - Rename "c9xx" to "c900"
> > > >  - Add thead,c900-plic in the description section
> > > > ---
> > > >  .../bindings/interrupt-controller/sifive,plic-1.0.0.yaml    | 6 ++++++
> > > >  1 file changed, 6 insertions(+)
> > > >
> > > > 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..82629832e5a5 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,11 @@ 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>.
> > > >
> > > > +  While the "thead,c900-plic" would mask IRQ with readl(claim), so it needn't
> > > > +  mask/unmask which needed in RISC-V PLIC. When in IRQS_ONESHOT & IRQCHIP_EOI_THREADED
> > > > +  path, unnecessary mask operation would cause a blocking irq bug in thead,c900-plic.
> > > > +  Because when IRQ is disabled in c900, writel(hwirq, claim) would be invalid.
> > >
> > > This is a totally incorrect description of the errata required for C9xx PLIC.
> > >
> > > Please don't project non-compliance as a feature of C9xx PLIC.
> > >
> > > > +
> > > >  maintainers:
> > > >    - Sagar Kadam <sagar.kadam@sifive.com>
> > > >    - Paul Walmsley  <paul.walmsley@sifive.com>
> > > > @@ -46,6 +51,7 @@ properties:
> > > >        - enum:
> > > >            - sifive,fu540-c000-plic
> > > >            - canaan,k210-plic
> > > > +          - thead,c900-plic
> >
> > we still want specific SoC names in the compatible, the "c900"
> > is still a sort-of placeholder.
> 
> Yes, we need "c900" compatible string as well. The "c9xx"
> compatible string is for the custom PLIC spec followed by T-HEAD.

What I meant was that the soc-specific string should name the
actual SoC (c906, c910) and not some imaginary chip ;-)

See for example mali gpu bindings for a similar reference
in devicetree/bindings/gpu/arm,mali-bifrost.yaml .




> 
> >
> >
> > > >        - const: sifive,plic-1.0.0
> > >
> > > The PLIC DT node requires two compatible string:
> > > <implementation_compat>, <spec_compat>
> > >
> > > The C9xx PLIC is not RISC-V PLIC so, the DT node should
> > > be: "thead,c900-plic", "thead,c9xx-plic"
> > >
> > > You need to change "- const: sifive,plic-1.0.0" to
> > > - enum:
> > >     - sifive,plic-1.0.0
> > >     - thead,c9xx-plic
> >
> >
> 
> Regards,
> Anup
> 

Re: [PATCH V3 1/2] dt-bindings: update riscv plic compatible string

[Anup Patel]

On Wed, Oct 13, 2021 at 2:44 PM Heiko Stübner <heiko@sntech.de> wrote:
>
> Am Mittwoch, 13. Oktober 2021, 11:11:26 CEST schrieb Anup Patel:
> > On Wed, Oct 13, 2021 at 2:27 PM Heiko Stübner <heiko@sntech.de> wrote:
> > >
> > > Hi Anup,
> > >
> > > Am Mittwoch, 13. Oktober 2021, 07:11:46 CEST schrieb Anup Patel:
> > > > On Wed, Oct 13, 2021 at 6:52 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 SOCs with thead,c9xx processor cores.
> > > > >
> > > > > Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> > > > > Cc: Rob Herring <robh@kernel.org>
> > > > > Cc: Palmer Dabbelt <palmerdabbelt@google.com>
> > > > > Cc: Anup Patel <anup@brainfault.org>
> > > > > Cc: Atish Patra <atish.patra@wdc.com>
> > > > >
> > > > > ---
> > > > >
> > > > > Changes since V3:
> > > > >  - Rename "c9xx" to "c900"
> > > > >  - Add thead,c900-plic in the description section
> > > > > ---
> > > > >  .../bindings/interrupt-controller/sifive,plic-1.0.0.yaml    | 6 ++++++
> > > > >  1 file changed, 6 insertions(+)
> > > > >
> > > > > 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..82629832e5a5 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,11 @@ 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>.
> > > > >
> > > > > +  While the "thead,c900-plic" would mask IRQ with readl(claim), so it needn't
> > > > > +  mask/unmask which needed in RISC-V PLIC. When in IRQS_ONESHOT & IRQCHIP_EOI_THREADED
> > > > > +  path, unnecessary mask operation would cause a blocking irq bug in thead,c900-plic.
> > > > > +  Because when IRQ is disabled in c900, writel(hwirq, claim) would be invalid.
> > > >
> > > > This is a totally incorrect description of the errata required for C9xx PLIC.
> > > >
> > > > Please don't project non-compliance as a feature of C9xx PLIC.
> > > >
> > > > > +
> > > > >  maintainers:
> > > > >    - Sagar Kadam <sagar.kadam@sifive.com>
> > > > >    - Paul Walmsley  <paul.walmsley@sifive.com>
> > > > > @@ -46,6 +51,7 @@ properties:
> > > > >        - enum:
> > > > >            - sifive,fu540-c000-plic
> > > > >            - canaan,k210-plic
> > > > > +          - thead,c900-plic
> > >
> > > we still want specific SoC names in the compatible, the "c900"
> > > is still a sort-of placeholder.
> >
> > Yes, we need "c900" compatible string as well. The "c9xx"
> > compatible string is for the custom PLIC spec followed by T-HEAD.
>
> What I meant was that the soc-specific string should name the
> actual SoC (c906, c910) and not some imaginary chip ;-)

Ahh, yes. It should be an actual soc name in the compatible
string.

For example, SiFive uses "fu540" string to identify some of the
devices on both SiFive unleashed and SiFive unmatched boards.

I was under the impression that "c900" is an actual SoC name.

Regards,
Anup

>
> See for example mali gpu bindings for a similar reference
> in devicetree/bindings/gpu/arm,mali-bifrost.yaml .
>
>
>
>
> >
> > >
> > >
> > > > >        - const: sifive,plic-1.0.0
> > > >
> > > > The PLIC DT node requires two compatible string:
> > > > <implementation_compat>, <spec_compat>
> > > >
> > > > The C9xx PLIC is not RISC-V PLIC so, the DT node should
> > > > be: "thead,c900-plic", "thead,c9xx-plic"
> > > >
> > > > You need to change "- const: sifive,plic-1.0.0" to
> > > > - enum:
> > > >     - sifive,plic-1.0.0
> > > >     - thead,c9xx-plic
> > >
> > >
> >
> > Regards,
> > Anup
> >
>
>
>
>

Re: [PATCH V3 1/2] dt-bindings: update riscv plic compatible string

[Heiko Stübner]

Am Mittwoch, 13. Oktober 2021, 11:19:53 CEST schrieb Anup Patel:
> On Wed, Oct 13, 2021 at 2:44 PM Heiko Stübner <heiko@sntech.de> wrote:
> >
> > Am Mittwoch, 13. Oktober 2021, 11:11:26 CEST schrieb Anup Patel:
> > > On Wed, Oct 13, 2021 at 2:27 PM Heiko Stübner <heiko@sntech.de> wrote:
> > > >
> > > > Hi Anup,
> > > >
> > > > Am Mittwoch, 13. Oktober 2021, 07:11:46 CEST schrieb Anup Patel:
> > > > > On Wed, Oct 13, 2021 at 6:52 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 SOCs with thead,c9xx processor cores.
> > > > > >
> > > > > > Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> > > > > > Cc: Rob Herring <robh@kernel.org>
> > > > > > Cc: Palmer Dabbelt <palmerdabbelt@google.com>
> > > > > > Cc: Anup Patel <anup@brainfault.org>
> > > > > > Cc: Atish Patra <atish.patra@wdc.com>
> > > > > >
> > > > > > ---
> > > > > >
> > > > > > Changes since V3:
> > > > > >  - Rename "c9xx" to "c900"
> > > > > >  - Add thead,c900-plic in the description section
> > > > > > ---
> > > > > >  .../bindings/interrupt-controller/sifive,plic-1.0.0.yaml    | 6 ++++++
> > > > > >  1 file changed, 6 insertions(+)
> > > > > >
> > > > > > 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..82629832e5a5 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,11 @@ 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>.
> > > > > >
> > > > > > +  While the "thead,c900-plic" would mask IRQ with readl(claim), so it needn't
> > > > > > +  mask/unmask which needed in RISC-V PLIC. When in IRQS_ONESHOT & IRQCHIP_EOI_THREADED
> > > > > > +  path, unnecessary mask operation would cause a blocking irq bug in thead,c900-plic.
> > > > > > +  Because when IRQ is disabled in c900, writel(hwirq, claim) would be invalid.
> > > > >
> > > > > This is a totally incorrect description of the errata required for C9xx PLIC.
> > > > >
> > > > > Please don't project non-compliance as a feature of C9xx PLIC.
> > > > >
> > > > > > +
> > > > > >  maintainers:
> > > > > >    - Sagar Kadam <sagar.kadam@sifive.com>
> > > > > >    - Paul Walmsley  <paul.walmsley@sifive.com>
> > > > > > @@ -46,6 +51,7 @@ properties:
> > > > > >        - enum:
> > > > > >            - sifive,fu540-c000-plic
> > > > > >            - canaan,k210-plic
> > > > > > +          - thead,c900-plic
> > > >
> > > > we still want specific SoC names in the compatible, the "c900"
> > > > is still a sort-of placeholder.
> > >
> > > Yes, we need "c900" compatible string as well. The "c9xx"
> > > compatible string is for the custom PLIC spec followed by T-HEAD.
> >
> > What I meant was that the soc-specific string should name the
> > actual SoC (c906, c910) and not some imaginary chip ;-)
> 
> Ahh, yes. It should be an actual soc name in the compatible
> string.
> 
> For example, SiFive uses "fu540" string to identify some of the
> devices on both SiFive unleashed and SiFive unmatched boards.
> 
> I was under the impression that "c900" is an actual SoC name.
> 
> Regards,
> Anup
> 
> >
> > See for example mali gpu bindings for a similar reference
> > in devicetree/bindings/gpu/arm,mali-bifrost.yaml .
> >
> >
> >
> >
> > >
> > > >
> > > >
> > > > > >        - const: sifive,plic-1.0.0
> > > > >
> > > > > The PLIC DT node requires two compatible string:
> > > > > <implementation_compat>, <spec_compat>
> > > > >
> > > > > The C9xx PLIC is not RISC-V PLIC so, the DT node should
> > > > > be: "thead,c900-plic", "thead,c9xx-plic"
> > > > >
> > > > > You need to change "- const: sifive,plic-1.0.0" to
> > > > > - enum:
> > > > >     - sifive,plic-1.0.0
> > > > >     - thead,c9xx-plic

isn't XuanTie the series containing the c906 and c910?
So maybe
	thead,xuantie-plic
for the spec compatible.

So doing in full
	compatible = "thead,c906-plic", "thead,xuantie-plic"


Heiko

Re: [PATCH V3 1/2] dt-bindings: update riscv plic compatible string

[Anup Patel]

On Wed, Oct 13, 2021 at 3:13 PM Heiko Stübner <heiko@sntech.de> wrote:
>
> Am Mittwoch, 13. Oktober 2021, 11:19:53 CEST schrieb Anup Patel:
> > On Wed, Oct 13, 2021 at 2:44 PM Heiko Stübner <heiko@sntech.de> wrote:
> > >
> > > Am Mittwoch, 13. Oktober 2021, 11:11:26 CEST schrieb Anup Patel:
> > > > On Wed, Oct 13, 2021 at 2:27 PM Heiko Stübner <heiko@sntech.de> wrote:
> > > > >
> > > > > Hi Anup,
> > > > >
> > > > > Am Mittwoch, 13. Oktober 2021, 07:11:46 CEST schrieb Anup Patel:
> > > > > > On Wed, Oct 13, 2021 at 6:52 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 SOCs with thead,c9xx processor cores.
> > > > > > >
> > > > > > > Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> > > > > > > Cc: Rob Herring <robh@kernel.org>
> > > > > > > Cc: Palmer Dabbelt <palmerdabbelt@google.com>
> > > > > > > Cc: Anup Patel <anup@brainfault.org>
> > > > > > > Cc: Atish Patra <atish.patra@wdc.com>
> > > > > > >
> > > > > > > ---
> > > > > > >
> > > > > > > Changes since V3:
> > > > > > >  - Rename "c9xx" to "c900"
> > > > > > >  - Add thead,c900-plic in the description section
> > > > > > > ---
> > > > > > >  .../bindings/interrupt-controller/sifive,plic-1.0.0.yaml    | 6 ++++++
> > > > > > >  1 file changed, 6 insertions(+)
> > > > > > >
> > > > > > > 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..82629832e5a5 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,11 @@ 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>.
> > > > > > >
> > > > > > > +  While the "thead,c900-plic" would mask IRQ with readl(claim), so it needn't
> > > > > > > +  mask/unmask which needed in RISC-V PLIC. When in IRQS_ONESHOT & IRQCHIP_EOI_THREADED
> > > > > > > +  path, unnecessary mask operation would cause a blocking irq bug in thead,c900-plic.
> > > > > > > +  Because when IRQ is disabled in c900, writel(hwirq, claim) would be invalid.
> > > > > >
> > > > > > This is a totally incorrect description of the errata required for C9xx PLIC.
> > > > > >
> > > > > > Please don't project non-compliance as a feature of C9xx PLIC.
> > > > > >
> > > > > > > +
> > > > > > >  maintainers:
> > > > > > >    - Sagar Kadam <sagar.kadam@sifive.com>
> > > > > > >    - Paul Walmsley  <paul.walmsley@sifive.com>
> > > > > > > @@ -46,6 +51,7 @@ properties:
> > > > > > >        - enum:
> > > > > > >            - sifive,fu540-c000-plic
> > > > > > >            - canaan,k210-plic
> > > > > > > +          - thead,c900-plic
> > > > >
> > > > > we still want specific SoC names in the compatible, the "c900"
> > > > > is still a sort-of placeholder.
> > > >
> > > > Yes, we need "c900" compatible string as well. The "c9xx"
> > > > compatible string is for the custom PLIC spec followed by T-HEAD.
> > >
> > > What I meant was that the soc-specific string should name the
> > > actual SoC (c906, c910) and not some imaginary chip ;-)
> >
> > Ahh, yes. It should be an actual soc name in the compatible
> > string.
> >
> > For example, SiFive uses "fu540" string to identify some of the
> > devices on both SiFive unleashed and SiFive unmatched boards.
> >
> > I was under the impression that "c900" is an actual SoC name.
> >
> > Regards,
> > Anup
> >
> > >
> > > See for example mali gpu bindings for a similar reference
> > > in devicetree/bindings/gpu/arm,mali-bifrost.yaml .
> > >
> > >
> > >
> > >
> > > >
> > > > >
> > > > >
> > > > > > >        - const: sifive,plic-1.0.0
> > > > > >
> > > > > > The PLIC DT node requires two compatible string:
> > > > > > <implementation_compat>, <spec_compat>
> > > > > >
> > > > > > The C9xx PLIC is not RISC-V PLIC so, the DT node should
> > > > > > be: "thead,c900-plic", "thead,c9xx-plic"
> > > > > >
> > > > > > You need to change "- const: sifive,plic-1.0.0" to
> > > > > > - enum:
> > > > > >     - sifive,plic-1.0.0
> > > > > >     - thead,c9xx-plic
>
> isn't XuanTie the series containing the c906 and c910?
> So maybe
>         thead,xuantie-plic
> for the spec compatible.
>
> So doing in full
>         compatible = "thead,c906-plic", "thead,xuantie-plic"

This is a much better suggestion. I will let Guo decide.

Regards,
Anup

>
>
> Heiko
>
>

Re: [PATCH V3 1/2] dt-bindings: update riscv plic compatible string

[Heiko Stübner]

Am Mittwoch, 13. Oktober 2021, 11:49:20 CEST schrieb Anup Patel:
> On Wed, Oct 13, 2021 at 3:13 PM Heiko Stübner <heiko@sntech.de> wrote:
> >
> > Am Mittwoch, 13. Oktober 2021, 11:19:53 CEST schrieb Anup Patel:
> > > On Wed, Oct 13, 2021 at 2:44 PM Heiko Stübner <heiko@sntech.de> wrote:
> > > >
> > > > Am Mittwoch, 13. Oktober 2021, 11:11:26 CEST schrieb Anup Patel:
> > > > > On Wed, Oct 13, 2021 at 2:27 PM Heiko Stübner <heiko@sntech.de> wrote:
> > > > > >
> > > > > > Hi Anup,
> > > > > >
> > > > > > Am Mittwoch, 13. Oktober 2021, 07:11:46 CEST schrieb Anup Patel:
> > > > > > > On Wed, Oct 13, 2021 at 6:52 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 SOCs with thead,c9xx processor cores.
> > > > > > > >
> > > > > > > > Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> > > > > > > > Cc: Rob Herring <robh@kernel.org>
> > > > > > > > Cc: Palmer Dabbelt <palmerdabbelt@google.com>
> > > > > > > > Cc: Anup Patel <anup@brainfault.org>
> > > > > > > > Cc: Atish Patra <atish.patra@wdc.com>
> > > > > > > >
> > > > > > > > ---
> > > > > > > >
> > > > > > > > Changes since V3:
> > > > > > > >  - Rename "c9xx" to "c900"
> > > > > > > >  - Add thead,c900-plic in the description section
> > > > > > > > ---
> > > > > > > >  .../bindings/interrupt-controller/sifive,plic-1.0.0.yaml    | 6 ++++++
> > > > > > > >  1 file changed, 6 insertions(+)
> > > > > > > >
> > > > > > > > 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..82629832e5a5 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,11 @@ 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>.
> > > > > > > >
> > > > > > > > +  While the "thead,c900-plic" would mask IRQ with readl(claim), so it needn't
> > > > > > > > +  mask/unmask which needed in RISC-V PLIC. When in IRQS_ONESHOT & IRQCHIP_EOI_THREADED
> > > > > > > > +  path, unnecessary mask operation would cause a blocking irq bug in thead,c900-plic.
> > > > > > > > +  Because when IRQ is disabled in c900, writel(hwirq, claim) would be invalid.
> > > > > > >
> > > > > > > This is a totally incorrect description of the errata required for C9xx PLIC.
> > > > > > >
> > > > > > > Please don't project non-compliance as a feature of C9xx PLIC.
> > > > > > >
> > > > > > > > +
> > > > > > > >  maintainers:
> > > > > > > >    - Sagar Kadam <sagar.kadam@sifive.com>
> > > > > > > >    - Paul Walmsley  <paul.walmsley@sifive.com>
> > > > > > > > @@ -46,6 +51,7 @@ properties:
> > > > > > > >        - enum:
> > > > > > > >            - sifive,fu540-c000-plic
> > > > > > > >            - canaan,k210-plic
> > > > > > > > +          - thead,c900-plic
> > > > > >
> > > > > > we still want specific SoC names in the compatible, the "c900"
> > > > > > is still a sort-of placeholder.
> > > > >
> > > > > Yes, we need "c900" compatible string as well. The "c9xx"
> > > > > compatible string is for the custom PLIC spec followed by T-HEAD.
> > > >
> > > > What I meant was that the soc-specific string should name the
> > > > actual SoC (c906, c910) and not some imaginary chip ;-)
> > >
> > > Ahh, yes. It should be an actual soc name in the compatible
> > > string.
> > >
> > > For example, SiFive uses "fu540" string to identify some of the
> > > devices on both SiFive unleashed and SiFive unmatched boards.
> > >
> > > I was under the impression that "c900" is an actual SoC name.
> > >
> > > Regards,
> > > Anup
> > >
> > > >
> > > > See for example mali gpu bindings for a similar reference
> > > > in devicetree/bindings/gpu/arm,mali-bifrost.yaml .
> > > >
> > > >
> > > >
> > > >
> > > > >
> > > > > >
> > > > > >
> > > > > > > >        - const: sifive,plic-1.0.0
> > > > > > >
> > > > > > > The PLIC DT node requires two compatible string:
> > > > > > > <implementation_compat>, <spec_compat>
> > > > > > >
> > > > > > > The C9xx PLIC is not RISC-V PLIC so, the DT node should
> > > > > > > be: "thead,c900-plic", "thead,c9xx-plic"
> > > > > > >
> > > > > > > You need to change "- const: sifive,plic-1.0.0" to
> > > > > > > - enum:
> > > > > > >     - sifive,plic-1.0.0
> > > > > > >     - thead,c9xx-plic
> >
> > isn't XuanTie the series containing the c906 and c910?
> > So maybe
> >         thead,xuantie-plic
> > for the spec compatible.
> >
> > So doing in full
> >         compatible = "thead,c906-plic", "thead,xuantie-plic"
> 
> This is a much better suggestion. I will let Guo decide.

In any case, we'll also need a new entry in
	devicetree/bindings/vendor-prefixes.yaml
for the "thead" prefix in a separate patch, as it looks like such a
thing is also still missing.

Heiko

Re: [PATCH V3 1/2] dt-bindings: update riscv plic compatible string

[Guo Ren]

On Wed, Oct 13, 2021 at 4:57 PM Heiko Stübner <heiko@sntech.de> wrote:
>
> Hi Anup,
>
> Am Mittwoch, 13. Oktober 2021, 07:11:46 CEST schrieb Anup Patel:
> > On Wed, Oct 13, 2021 at 6:52 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 SOCs with thead,c9xx processor cores.
> > >
> > > Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> > > Cc: Rob Herring <robh@kernel.org>
> > > Cc: Palmer Dabbelt <palmerdabbelt@google.com>
> > > Cc: Anup Patel <anup@brainfault.org>
> > > Cc: Atish Patra <atish.patra@wdc.com>
> > >
> > > ---
> > >
> > > Changes since V3:
> > >  - Rename "c9xx" to "c900"
> > >  - Add thead,c900-plic in the description section
> > > ---
> > >  .../bindings/interrupt-controller/sifive,plic-1.0.0.yaml    | 6 ++++++
> > >  1 file changed, 6 insertions(+)
> > >
> > > 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..82629832e5a5 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,11 @@ 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>.
> > >
> > > +  While the "thead,c900-plic" would mask IRQ with readl(claim), so it needn't
> > > +  mask/unmask which needed in RISC-V PLIC. When in IRQS_ONESHOT & IRQCHIP_EOI_THREADED
> > > +  path, unnecessary mask operation would cause a blocking irq bug in thead,c900-plic.
> > > +  Because when IRQ is disabled in c900, writel(hwirq, claim) would be invalid.
> >
> > This is a totally incorrect description of the errata required for C9xx PLIC.
> >
> > Please don't project non-compliance as a feature of C9xx PLIC.
> >
> > > +
> > >  maintainers:
> > >    - Sagar Kadam <sagar.kadam@sifive.com>
> > >    - Paul Walmsley  <paul.walmsley@sifive.com>
> > > @@ -46,6 +51,7 @@ properties:
> > >        - enum:
> > >            - sifive,fu540-c000-plic
> > >            - canaan,k210-plic
> > > +          - thead,c900-plic
>
> we still want specific SoC names in the compatible, the "c900"
> is still a sort-of placeholder.
c900 is not a SOC name, but a CPU name. For soc name, it should be
"allwinner,d1-plic".

>
>
> > >        - const: sifive,plic-1.0.0
> >
> > The PLIC DT node requires two compatible string:
> > <implementation_compat>, <spec_compat>
> >
> > The C9xx PLIC is not RISC-V PLIC so, the DT node should
> > be: "thead,c900-plic", "thead,c9xx-plic"
> >
> > You need to change "- const: sifive,plic-1.0.0" to
> > - enum:
> >     - sifive,plic-1.0.0
> >     - thead,c9xx-plic
>
>


-- 
Best Regards
 Guo Ren

ML: https://lore.kernel.org/linux-csky/

Re: [PATCH V3 1/2] dt-bindings: update riscv plic compatible string

[Marc Zyngier]

On 2021-10-13 02:21, 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 SOCs with thead,c9xx processor cores.
> 
> Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Palmer Dabbelt <palmerdabbelt@google.com>
> Cc: Anup Patel <anup@brainfault.org>
> Cc: Atish Patra <atish.patra@wdc.com>

Please add a cover letter when sending more than a single patch.

This isn't complicated, and git will do it for you if you kindly ask.

         M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH V3 1/2] dt-bindings: update riscv plic compatible string

[Guo Ren]

On Wed, Oct 13, 2021 at 8:39 PM Marc Zyngier <maz@kernel.org> wrote:
>
> On 2021-10-13 02:21, 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 SOCs with thead,c9xx processor cores.
> >
> > Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> > Cc: Rob Herring <robh@kernel.org>
> > Cc: Palmer Dabbelt <palmerdabbelt@google.com>
> > Cc: Anup Patel <anup@brainfault.org>
> > Cc: Atish Patra <atish.patra@wdc.com>
>
> Please add a cover letter when sending more than a single patch.
>
> This isn't complicated, and git will do it for you if you kindly ask.
Okay

>
>          M.
> --
> Jazz is not dead. It just smells funny...



-- 
Best Regards
 Guo Ren

ML: https://lore.kernel.org/linux-csky/

Re: [PATCH V3 1/2] dt-bindings: update riscv plic compatible string

[Guo Ren]

On Wed, Oct 13, 2021 at 5:43 PM Heiko Stübner <heiko@sntech.de> wrote:
>
> Am Mittwoch, 13. Oktober 2021, 11:19:53 CEST schrieb Anup Patel:
> > On Wed, Oct 13, 2021 at 2:44 PM Heiko Stübner <heiko@sntech.de> wrote:
> > >
> > > Am Mittwoch, 13. Oktober 2021, 11:11:26 CEST schrieb Anup Patel:
> > > > On Wed, Oct 13, 2021 at 2:27 PM Heiko Stübner <heiko@sntech.de> wrote:
> > > > >
> > > > > Hi Anup,
> > > > >
> > > > > Am Mittwoch, 13. Oktober 2021, 07:11:46 CEST schrieb Anup Patel:
> > > > > > On Wed, Oct 13, 2021 at 6:52 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 SOCs with thead,c9xx processor cores.
> > > > > > >
> > > > > > > Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> > > > > > > Cc: Rob Herring <robh@kernel.org>
> > > > > > > Cc: Palmer Dabbelt <palmerdabbelt@google.com>
> > > > > > > Cc: Anup Patel <anup@brainfault.org>
> > > > > > > Cc: Atish Patra <atish.patra@wdc.com>
> > > > > > >
> > > > > > > ---
> > > > > > >
> > > > > > > Changes since V3:
> > > > > > >  - Rename "c9xx" to "c900"
> > > > > > >  - Add thead,c900-plic in the description section
> > > > > > > ---
> > > > > > >  .../bindings/interrupt-controller/sifive,plic-1.0.0.yaml    | 6 ++++++
> > > > > > >  1 file changed, 6 insertions(+)
> > > > > > >
> > > > > > > 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..82629832e5a5 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,11 @@ 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>.
> > > > > > >
> > > > > > > +  While the "thead,c900-plic" would mask IRQ with readl(claim), so it needn't
> > > > > > > +  mask/unmask which needed in RISC-V PLIC. When in IRQS_ONESHOT & IRQCHIP_EOI_THREADED
> > > > > > > +  path, unnecessary mask operation would cause a blocking irq bug in thead,c900-plic.
> > > > > > > +  Because when IRQ is disabled in c900, writel(hwirq, claim) would be invalid.
> > > > > >
> > > > > > This is a totally incorrect description of the errata required for C9xx PLIC.
> > > > > >
> > > > > > Please don't project non-compliance as a feature of C9xx PLIC.
> > > > > >
> > > > > > > +
> > > > > > >  maintainers:
> > > > > > >    - Sagar Kadam <sagar.kadam@sifive.com>
> > > > > > >    - Paul Walmsley  <paul.walmsley@sifive.com>
> > > > > > > @@ -46,6 +51,7 @@ properties:
> > > > > > >        - enum:
> > > > > > >            - sifive,fu540-c000-plic
> > > > > > >            - canaan,k210-plic
> > > > > > > +          - thead,c900-plic
> > > > >
> > > > > we still want specific SoC names in the compatible, the "c900"
> > > > > is still a sort-of placeholder.
> > > >
> > > > Yes, we need "c900" compatible string as well. The "c9xx"
> > > > compatible string is for the custom PLIC spec followed by T-HEAD.
> > >
> > > What I meant was that the soc-specific string should name the
> > > actual SoC (c906, c910) and not some imaginary chip ;-)
> >
> > Ahh, yes. It should be an actual soc name in the compatible
> > string.
> >
> > For example, SiFive uses "fu540" string to identify some of the
> > devices on both SiFive unleashed and SiFive unmatched boards.
> >
> > I was under the impression that "c900" is an actual SoC name.
> >
> > Regards,
> > Anup
> >
> > >
> > > See for example mali gpu bindings for a similar reference
> > > in devicetree/bindings/gpu/arm,mali-bifrost.yaml .
> > >
> > >
> > >
> > >
> > > >
> > > > >
> > > > >
> > > > > > >        - const: sifive,plic-1.0.0
> > > > > >
> > > > > > The PLIC DT node requires two compatible string:
> > > > > > <implementation_compat>, <spec_compat>
> > > > > >
> > > > > > The C9xx PLIC is not RISC-V PLIC so, the DT node should
> > > > > > be: "thead,c900-plic", "thead,c9xx-plic"
> > > > > >
> > > > > > You need to change "- const: sifive,plic-1.0.0" to
> > > > > > - enum:
> > > > > >     - sifive,plic-1.0.0
> > > > > >     - thead,c9xx-plic
>
> isn't XuanTie the series containing the c906 and c910?
XuanTie contain two CPU series:
riscv: c906, c910
csky: c807, c810, c860

> So maybe
>         thead,xuantie-plic
> for the spec compatible.
>
> So doing in full
>         compatible = "thead,c906-plic", "thead,xuantie-plic"
How about:
compatible = "allwinner,d1-plic", "thead,c900-plic"

>
>
> Heiko
>
>


-- 
Best Regards
 Guo Ren

ML: https://lore.kernel.org/linux-csky/

Re: [PATCH V3 1/2] dt-bindings: update riscv plic compatible string

[Heiko Stuebner]

Hi,

Am Mittwoch, 13. Oktober 2021, 14:49:57 CEST schrieb Guo Ren:
> On Wed, Oct 13, 2021 at 5:43 PM Heiko Stübner <heiko@sntech.de> wrote:
> >
> > Am Mittwoch, 13. Oktober 2021, 11:19:53 CEST schrieb Anup Patel:
> > > On Wed, Oct 13, 2021 at 2:44 PM Heiko Stübner <heiko@sntech.de> wrote:
> > > >
> > > > Am Mittwoch, 13. Oktober 2021, 11:11:26 CEST schrieb Anup Patel:
> > > > > On Wed, Oct 13, 2021 at 2:27 PM Heiko Stübner <heiko@sntech.de> wrote:
> > > > > >
> > > > > > Hi Anup,
> > > > > >
> > > > > > Am Mittwoch, 13. Oktober 2021, 07:11:46 CEST schrieb Anup Patel:
> > > > > > > On Wed, Oct 13, 2021 at 6:52 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 SOCs with thead,c9xx processor cores.
> > > > > > > >
> > > > > > > > Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> > > > > > > > Cc: Rob Herring <robh@kernel.org>
> > > > > > > > Cc: Palmer Dabbelt <palmerdabbelt@google.com>
> > > > > > > > Cc: Anup Patel <anup@brainfault.org>
> > > > > > > > Cc: Atish Patra <atish.patra@wdc.com>
> > > > > > > >
> > > > > > > > ---
> > > > > > > >
> > > > > > > > Changes since V3:
> > > > > > > >  - Rename "c9xx" to "c900"
> > > > > > > >  - Add thead,c900-plic in the description section
> > > > > > > > ---
> > > > > > > >  .../bindings/interrupt-controller/sifive,plic-1.0.0.yaml    | 6 ++++++
> > > > > > > >  1 file changed, 6 insertions(+)
> > > > > > > >
> > > > > > > > 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..82629832e5a5 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,11 @@ 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>.
> > > > > > > >
> > > > > > > > +  While the "thead,c900-plic" would mask IRQ with readl(claim), so it needn't
> > > > > > > > +  mask/unmask which needed in RISC-V PLIC. When in IRQS_ONESHOT & IRQCHIP_EOI_THREADED
> > > > > > > > +  path, unnecessary mask operation would cause a blocking irq bug in thead,c900-plic.
> > > > > > > > +  Because when IRQ is disabled in c900, writel(hwirq, claim) would be invalid.
> > > > > > >
> > > > > > > This is a totally incorrect description of the errata required for C9xx PLIC.
> > > > > > >
> > > > > > > Please don't project non-compliance as a feature of C9xx PLIC.
> > > > > > >
> > > > > > > > +
> > > > > > > >  maintainers:
> > > > > > > >    - Sagar Kadam <sagar.kadam@sifive.com>
> > > > > > > >    - Paul Walmsley  <paul.walmsley@sifive.com>
> > > > > > > > @@ -46,6 +51,7 @@ properties:
> > > > > > > >        - enum:
> > > > > > > >            - sifive,fu540-c000-plic
> > > > > > > >            - canaan,k210-plic
> > > > > > > > +          - thead,c900-plic
> > > > > >
> > > > > > we still want specific SoC names in the compatible, the "c900"
> > > > > > is still a sort-of placeholder.
> > > > >
> > > > > Yes, we need "c900" compatible string as well. The "c9xx"
> > > > > compatible string is for the custom PLIC spec followed by T-HEAD.
> > > >
> > > > What I meant was that the soc-specific string should name the
> > > > actual SoC (c906, c910) and not some imaginary chip ;-)
> > >
> > > Ahh, yes. It should be an actual soc name in the compatible
> > > string.
> > >
> > > For example, SiFive uses "fu540" string to identify some of the
> > > devices on both SiFive unleashed and SiFive unmatched boards.
> > >
> > > I was under the impression that "c900" is an actual SoC name.
> > >
> > > Regards,
> > > Anup
> > >
> > > >
> > > > See for example mali gpu bindings for a similar reference
> > > > in devicetree/bindings/gpu/arm,mali-bifrost.yaml .
> > > >
> > > >
> > > >
> > > >
> > > > >
> > > > > >
> > > > > >
> > > > > > > >        - const: sifive,plic-1.0.0
> > > > > > >
> > > > > > > The PLIC DT node requires two compatible string:
> > > > > > > <implementation_compat>, <spec_compat>
> > > > > > >
> > > > > > > The C9xx PLIC is not RISC-V PLIC so, the DT node should
> > > > > > > be: "thead,c900-plic", "thead,c9xx-plic"
> > > > > > >
> > > > > > > You need to change "- const: sifive,plic-1.0.0" to
> > > > > > > - enum:
> > > > > > >     - sifive,plic-1.0.0
> > > > > > >     - thead,c9xx-plic
> >
> > isn't XuanTie the series containing the c906 and c910?
> XuanTie contain two CPU series:
> riscv: c906, c910
> csky: c807, c810, c860
> 
> > So maybe
> >         thead,xuantie-plic
> > for the spec compatible.
> >
> > So doing in full
> >         compatible = "thead,c906-plic", "thead,xuantie-plic"
> How about:
> compatible = "allwinner,d1-plic", "thead,c900-plic"

This looks sensible.

- I guess the question in general is, is the PLIC part of the core spec
or part of the soc. In other words will all SoCs that use C9xx cores,
use this specific PLIC characteristic?

- If all C9xx-based SoCs will use this PLIC, I guess that thead,c900-plic
  in your compatible above sounds pretty good.

- Should it be thead,* or t-head,* for the vendor-prefix?
  (domain seems to be t-head.cn)


Heiko

Re: [PATCH V3 1/2] dt-bindings: update riscv plic compatible string

[Guo Ren]

On Thu, Oct 14, 2021 at 8:26 AM Heiko Stuebner <heiko@sntech.de> wrote:
>
> Hi,
>
> Am Mittwoch, 13. Oktober 2021, 14:49:57 CEST schrieb Guo Ren:
> > On Wed, Oct 13, 2021 at 5:43 PM Heiko Stübner <heiko@sntech.de> wrote:
> > >
> > > Am Mittwoch, 13. Oktober 2021, 11:19:53 CEST schrieb Anup Patel:
> > > > On Wed, Oct 13, 2021 at 2:44 PM Heiko Stübner <heiko@sntech.de> wrote:
> > > > >
> > > > > Am Mittwoch, 13. Oktober 2021, 11:11:26 CEST schrieb Anup Patel:
> > > > > > On Wed, Oct 13, 2021 at 2:27 PM Heiko Stübner <heiko@sntech.de> wrote:
> > > > > > >
> > > > > > > Hi Anup,
> > > > > > >
> > > > > > > Am Mittwoch, 13. Oktober 2021, 07:11:46 CEST schrieb Anup Patel:
> > > > > > > > On Wed, Oct 13, 2021 at 6:52 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 SOCs with thead,c9xx processor cores.
> > > > > > > > >
> > > > > > > > > Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> > > > > > > > > Cc: Rob Herring <robh@kernel.org>
> > > > > > > > > Cc: Palmer Dabbelt <palmerdabbelt@google.com>
> > > > > > > > > Cc: Anup Patel <anup@brainfault.org>
> > > > > > > > > Cc: Atish Patra <atish.patra@wdc.com>
> > > > > > > > >
> > > > > > > > > ---
> > > > > > > > >
> > > > > > > > > Changes since V3:
> > > > > > > > >  - Rename "c9xx" to "c900"
> > > > > > > > >  - Add thead,c900-plic in the description section
> > > > > > > > > ---
> > > > > > > > >  .../bindings/interrupt-controller/sifive,plic-1.0.0.yaml    | 6 ++++++
> > > > > > > > >  1 file changed, 6 insertions(+)
> > > > > > > > >
> > > > > > > > > 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..82629832e5a5 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,11 @@ 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>.
> > > > > > > > >
> > > > > > > > > +  While the "thead,c900-plic" would mask IRQ with readl(claim), so it needn't
> > > > > > > > > +  mask/unmask which needed in RISC-V PLIC. When in IRQS_ONESHOT & IRQCHIP_EOI_THREADED
> > > > > > > > > +  path, unnecessary mask operation would cause a blocking irq bug in thead,c900-plic.
> > > > > > > > > +  Because when IRQ is disabled in c900, writel(hwirq, claim) would be invalid.
> > > > > > > >
> > > > > > > > This is a totally incorrect description of the errata required for C9xx PLIC.
> > > > > > > >
> > > > > > > > Please don't project non-compliance as a feature of C9xx PLIC.
> > > > > > > >
> > > > > > > > > +
> > > > > > > > >  maintainers:
> > > > > > > > >    - Sagar Kadam <sagar.kadam@sifive.com>
> > > > > > > > >    - Paul Walmsley  <paul.walmsley@sifive.com>
> > > > > > > > > @@ -46,6 +51,7 @@ properties:
> > > > > > > > >        - enum:
> > > > > > > > >            - sifive,fu540-c000-plic
> > > > > > > > >            - canaan,k210-plic
> > > > > > > > > +          - thead,c900-plic
> > > > > > >
> > > > > > > we still want specific SoC names in the compatible, the "c900"
> > > > > > > is still a sort-of placeholder.
> > > > > >
> > > > > > Yes, we need "c900" compatible string as well. The "c9xx"
> > > > > > compatible string is for the custom PLIC spec followed by T-HEAD.
> > > > >
> > > > > What I meant was that the soc-specific string should name the
> > > > > actual SoC (c906, c910) and not some imaginary chip ;-)
> > > >
> > > > Ahh, yes. It should be an actual soc name in the compatible
> > > > string.
> > > >
> > > > For example, SiFive uses "fu540" string to identify some of the
> > > > devices on both SiFive unleashed and SiFive unmatched boards.
> > > >
> > > > I was under the impression that "c900" is an actual SoC name.
> > > >
> > > > Regards,
> > > > Anup
> > > >
> > > > >
> > > > > See for example mali gpu bindings for a similar reference
> > > > > in devicetree/bindings/gpu/arm,mali-bifrost.yaml .
> > > > >
> > > > >
> > > > >
> > > > >
> > > > > >
> > > > > > >
> > > > > > >
> > > > > > > > >        - const: sifive,plic-1.0.0
> > > > > > > >
> > > > > > > > The PLIC DT node requires two compatible string:
> > > > > > > > <implementation_compat>, <spec_compat>
> > > > > > > >
> > > > > > > > The C9xx PLIC is not RISC-V PLIC so, the DT node should
> > > > > > > > be: "thead,c900-plic", "thead,c9xx-plic"
> > > > > > > >
> > > > > > > > You need to change "- const: sifive,plic-1.0.0" to
> > > > > > > > - enum:
> > > > > > > >     - sifive,plic-1.0.0
> > > > > > > >     - thead,c9xx-plic
> > >
> > > isn't XuanTie the series containing the c906 and c910?
> > XuanTie contain two CPU series:
> > riscv: c906, c910
> > csky: c807, c810, c860
> >
> > > So maybe
> > >         thead,xuantie-plic
> > > for the spec compatible.
> > >
> > > So doing in full
> > >         compatible = "thead,c906-plic", "thead,xuantie-plic"
> > How about:
> > compatible = "allwinner,d1-plic", "thead,c900-plic"
>
> This looks sensible.
>
> - I guess the question in general is, is the PLIC part of the core spec
> or part of the soc. In other words will all SoCs that use C9xx cores,
> use this specific PLIC characteristic?
Yes, unless soc wants to customize.

>
> - If all C9xx-based SoCs will use this PLIC, I guess that thead,c900-plic
>   in your compatible above sounds pretty good.
>
> - Should it be thead,* or t-head,* for the vendor-prefix?
T-Head Semiconductor Co., Ltd.
ref: https://github.com/T-head-Semi

So it's "thead" for vendor-prefix

>   (domain seems to be t-head.cn)
>
>
> Heiko
>
>

-- 
Best Regards
 Guo Ren

ML: https://lore.kernel.org/linux-csky/

Re: [PATCH V3 1/2] dt-bindings: update riscv plic compatible string

[Samuel Holland]

On 10/13/21 7:49 AM, Guo Ren wrote:
>>>>>>>>        - const: sifive,plic-1.0.0
>>>>>>>
>>>>>>> The PLIC DT node requires two compatible string:
>>>>>>> <implementation_compat>, <spec_compat>
>>>>>>>
>>>>>>> The C9xx PLIC is not RISC-V PLIC so, the DT node should
>>>>>>> be: "thead,c900-plic", "thead,c9xx-plic"
>>>>>>>
>>>>>>> You need to change "- const: sifive,plic-1.0.0" to
>>>>>>> - enum:
>>>>>>>     - sifive,plic-1.0.0
>>>>>>>     - thead,c9xx-plic
>>
>> isn't XuanTie the series containing the c906 and c910?
> XuanTie contain two CPU series:
> riscv: c906, c910
> csky: c807, c810, c860
> 
>> So maybe
>>         thead,xuantie-plic
>> for the spec compatible.
>>
>> So doing in full
>>         compatible = "thead,c906-plic", "thead,xuantie-plic"
> How about:
> compatible = "allwinner,d1-plic", "thead,c900-plic"

To follow the <family>-<soc>-<device> pattern of existing Allwinner
compatibles, the first string should be "allwinner,sun20i-d1-plic".
Otherwise, this looks to me like the right thing to do.

Regards,
Samuel

Re: [PATCH V3 1/2] dt-bindings: update riscv plic compatible string

[Guo Ren]

On Thu, Oct 14, 2021 at 12:21 PM Samuel Holland <samuel@sholland.org> wrote:
>
> On 10/13/21 7:49 AM, Guo Ren wrote:
> >>>>>>>>        - const: sifive,plic-1.0.0
> >>>>>>>
> >>>>>>> The PLIC DT node requires two compatible string:
> >>>>>>> <implementation_compat>, <spec_compat>
> >>>>>>>
> >>>>>>> The C9xx PLIC is not RISC-V PLIC so, the DT node should
> >>>>>>> be: "thead,c900-plic", "thead,c9xx-plic"
> >>>>>>>
> >>>>>>> You need to change "- const: sifive,plic-1.0.0" to
> >>>>>>> - enum:
> >>>>>>>     - sifive,plic-1.0.0
> >>>>>>>     - thead,c9xx-plic
> >>
> >> isn't XuanTie the series containing the c906 and c910?
> > XuanTie contain two CPU series:
> > riscv: c906, c910
> > csky: c807, c810, c860
> >
> >> So maybe
> >>         thead,xuantie-plic
> >> for the spec compatible.
> >>
> >> So doing in full
> >>         compatible = "thead,c906-plic", "thead,xuantie-plic"
> > How about:
> > compatible = "allwinner,d1-plic", "thead,c900-plic"
>
> To follow the <family>-<soc>-<device> pattern of existing Allwinner
> compatibles, the first string should be "allwinner,sun20i-d1-plic".
Thx, Got it.
> Otherwise, this looks to me like the right thing to do.
>
> Regards,
> Samuel



-- 
Best Regards
 Guo Ren

ML: https://lore.kernel.org/linux-csky/