2024-01-12 23:50:41

by Atish Patra

[permalink] [raw]
Subject: Re: [PATCH v7 04/16] dt-bindings: riscv: Add Andes interrupt controller compatible string

On Tue, Jan 9, 2024 at 11:40 PM Yu Chien Peter Lin
<[email protected]> wrote:
>
> Add "andestech,cpu-intc" compatible string to indicate that
> Andes specific local interrupt is supported on the core,
> e.g. AX45MP cores have 3 types of non-standard local interrupt
> which can be handled in supervisor mode:
>

PATCH1 in this series renames everything from ANDESTECH to ANDES to be
consistent.
Here you are adding DT binding with "andestech". Is there any
fundamental difference between these two to demand
this change ?

FYI: I don't mind either way as I don't have skin in the game. Just
something I noticed while reviewing the series.

> - Slave port ECC error interrupt
> - Bus write transaction error interrupt
> - Performance monitor overflow interrupt
>
> These interrupts are enabled/disabled via a custom register
> SLIE instead of the standard interrupt enable register SIE.
>
> Signed-off-by: Yu Chien Peter Lin <[email protected]>
> Acked-by: Conor Dooley <[email protected]>
> Reviewed-by: Lad Prabhakar <[email protected]>
> ---
> Changes v1 -> v2:
> - New patch
> Changes v2 -> v3:
> - Updated commit message
> - Fixed possible compatibles for Andes INTC
> Changes v3 -> v4:
> - Add const entry instead of enum (Suggested by Conor)
> Changes v4 -> v5:
> - Include Conor's Acked-by
> - Include Prabhakar's Reviewed-by
> Changes v5 -> v6:
> - No change
> Changes v6 -> v7:
> - No change
> ---
> Documentation/devicetree/bindings/riscv/cpus.yaml | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/riscv/cpus.yaml b/Documentation/devicetree/bindings/riscv/cpus.yaml
> index 23646b684ea2..33c2b620a59f 100644
> --- a/Documentation/devicetree/bindings/riscv/cpus.yaml
> +++ b/Documentation/devicetree/bindings/riscv/cpus.yaml
> @@ -101,7 +101,11 @@ properties:
> const: 1
>
> compatible:
> - const: riscv,cpu-intc
> + oneOf:
> + - items:
> + - const: andestech,cpu-intc
> + - const: riscv,cpu-intc
> + - const: riscv,cpu-intc
>
> interrupt-controller: true
>
> --
> 2.34.1
>


--
Regards,
Atish


2024-01-13 00:19:24

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v7 04/16] dt-bindings: riscv: Add Andes interrupt controller compatible string

On Fri, Jan 12, 2024 at 03:50:15PM -0800, Atish Patra wrote:
> On Tue, Jan 9, 2024 at 11:40 PM Yu Chien Peter Lin
> <[email protected]> wrote:
> >
> > Add "andestech,cpu-intc" compatible string to indicate that
> > Andes specific local interrupt is supported on the core,
> > e.g. AX45MP cores have 3 types of non-standard local interrupt
> > which can be handled in supervisor mode:
> >
>
> PATCH1 in this series renames everything from ANDESTECH to ANDES to be
> consistent.
> Here you are adding DT binding with "andestech". Is there any
> fundamental difference between these two to demand
> this change ?

This one is the vendor prefix and the company has been assigned
"andestech" and they're not getting a second one. The rename of the
variables is a bit gratuitous tbf.

Cheers,
Conor.

> FYI: I don't mind either way as I don't have skin in the game. Just
> something I noticed while reviewing the series.
>
> > - Slave port ECC error interrupt
> > - Bus write transaction error interrupt
> > - Performance monitor overflow interrupt
> >
> > These interrupts are enabled/disabled via a custom register
> > SLIE instead of the standard interrupt enable register SIE.
> >
> > Signed-off-by: Yu Chien Peter Lin <[email protected]>
> > Acked-by: Conor Dooley <[email protected]>
> > Reviewed-by: Lad Prabhakar <[email protected]>
> > ---
> > Changes v1 -> v2:
> > - New patch
> > Changes v2 -> v3:
> > - Updated commit message
> > - Fixed possible compatibles for Andes INTC
> > Changes v3 -> v4:
> > - Add const entry instead of enum (Suggested by Conor)
> > Changes v4 -> v5:
> > - Include Conor's Acked-by
> > - Include Prabhakar's Reviewed-by
> > Changes v5 -> v6:
> > - No change
> > Changes v6 -> v7:
> > - No change
> > ---
> > Documentation/devicetree/bindings/riscv/cpus.yaml | 6 +++++-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/riscv/cpus.yaml b/Documentation/devicetree/bindings/riscv/cpus.yaml
> > index 23646b684ea2..33c2b620a59f 100644
> > --- a/Documentation/devicetree/bindings/riscv/cpus.yaml
> > +++ b/Documentation/devicetree/bindings/riscv/cpus.yaml
> > @@ -101,7 +101,11 @@ properties:
> > const: 1
> >
> > compatible:
> > - const: riscv,cpu-intc
> > + oneOf:
> > + - items:
> > + - const: andestech,cpu-intc
> > + - const: riscv,cpu-intc
> > + - const: riscv,cpu-intc
> >
> > interrupt-controller: true
> >
> > --
> > 2.34.1
> >
>
>
> --
> Regards,
> Atish


Attachments:
(No filename) (2.61 kB)
signature.asc (235.00 B)
Download all attachments

2024-01-13 00:32:21

by Atish Patra

[permalink] [raw]
Subject: Re: [PATCH v7 04/16] dt-bindings: riscv: Add Andes interrupt controller compatible string

On Fri, Jan 12, 2024 at 4:19 PM Conor Dooley <[email protected]> wrote:
>
> On Fri, Jan 12, 2024 at 03:50:15PM -0800, Atish Patra wrote:
> > On Tue, Jan 9, 2024 at 11:40 PM Yu Chien Peter Lin
> > <[email protected]> wrote:
> > >
> > > Add "andestech,cpu-intc" compatible string to indicate that
> > > Andes specific local interrupt is supported on the core,
> > > e.g. AX45MP cores have 3 types of non-standard local interrupt
> > > which can be handled in supervisor mode:
> > >
> >
> > PATCH1 in this series renames everything from ANDESTECH to ANDES to be
> > consistent.
> > Here you are adding DT binding with "andestech". Is there any
> > fundamental difference between these two to demand
> > this change ?
>
> This one is the vendor prefix and the company has been assigned
> "andestech" and they're not getting a second one. The rename of the

Thanks for the clarification.

> variables is a bit gratuitous tbf.
>

I felt the same.

> Cheers,
> Conor.
>
> > FYI: I don't mind either way as I don't have skin in the game. Just
> > something I noticed while reviewing the series.
> >
> > > - Slave port ECC error interrupt
> > > - Bus write transaction error interrupt
> > > - Performance monitor overflow interrupt
> > >
> > > These interrupts are enabled/disabled via a custom register
> > > SLIE instead of the standard interrupt enable register SIE.
> > >
> > > Signed-off-by: Yu Chien Peter Lin <[email protected]>
> > > Acked-by: Conor Dooley <[email protected]>
> > > Reviewed-by: Lad Prabhakar <[email protected]>
> > > ---
> > > Changes v1 -> v2:
> > > - New patch
> > > Changes v2 -> v3:
> > > - Updated commit message
> > > - Fixed possible compatibles for Andes INTC
> > > Changes v3 -> v4:
> > > - Add const entry instead of enum (Suggested by Conor)
> > > Changes v4 -> v5:
> > > - Include Conor's Acked-by
> > > - Include Prabhakar's Reviewed-by
> > > Changes v5 -> v6:
> > > - No change
> > > Changes v6 -> v7:
> > > - No change
> > > ---
> > > Documentation/devicetree/bindings/riscv/cpus.yaml | 6 +++++-
> > > 1 file changed, 5 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/riscv/cpus.yaml b/Documentation/devicetree/bindings/riscv/cpus.yaml
> > > index 23646b684ea2..33c2b620a59f 100644
> > > --- a/Documentation/devicetree/bindings/riscv/cpus.yaml
> > > +++ b/Documentation/devicetree/bindings/riscv/cpus.yaml
> > > @@ -101,7 +101,11 @@ properties:
> > > const: 1
> > >
> > > compatible:
> > > - const: riscv,cpu-intc
> > > + oneOf:
> > > + - items:
> > > + - const: andestech,cpu-intc
> > > + - const: riscv,cpu-intc
> > > + - const: riscv,cpu-intc
> > >
> > > interrupt-controller: true
> > >
> > > --
> > > 2.34.1
> > >
> >
> >
> > --
> > Regards,
> > Atish



--
Regards,
Atish