All of lore.kernel.org
 help / color / mirror / Atom feed
From: Conor Dooley <conor.dooley@microchip.com>
To: Anup Patel <apatel@ventanamicro.com>
Cc: Conor Dooley <conor@kernel.org>, Anup Patel <anup@brainfault.org>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Paul Walmsley <paul.walmsley@sifive.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Marc Zyngier <maz@kernel.org>, Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Atish Patra <atishp@atishpatra.org>,
	Alistair Francis <Alistair.Francis@wdc.com>,
	<linux-riscv@lists.infradead.org>, <linux-kernel@vger.kernel.org>,
	<devicetree@vger.kernel.org>
Subject: Re: [PATCH v2 1/9] RISC-V: Add AIA related CSR defines
Date: Fri, 27 Jan 2023 14:20:28 +0000	[thread overview]
Message-ID: <Y9PdrOvb8bYXpgVw@wendy> (raw)
In-Reply-To: <CAK9=C2VzJvpQLPedc+ruUnw8xDDDaC6_Vmj6qg1nXv+iqU-AfQ@mail.gmail.com>


[-- Attachment #1.1: Type: text/plain, Size: 1859 bytes --]

On Fri, Jan 27, 2023 at 05:28:57PM +0530, Anup Patel wrote:
> On Wed, Jan 18, 2023 at 2:12 AM Conor Dooley <conor@kernel.org> wrote:

> > > > > +/* AIA CSR bits */
> > > > > +#define TOPI_IID_SHIFT               16
> > > > > +#define TOPI_IID_MASK                0xfff
> >
> > While I think of it, it'd be worth noting that these are generic across
> > all of topi, mtopi etc. Initially I thought that this mask was wrong as
> > the topi section says:
> >         bits 25:16 Interrupt identity (source number)
> >         bits 7:0 Interrupt priority
> 
> These defines are for the AIA CSRs and not AIA APLIC IDC registers.
> 
> As per the latest frozen spec, the mtopi/stopi/vstopi has following bits:
>     bits: 27:16 IID
>     bits: 7:0 IPRIO

I know, that those ones use those bits, hence leaving an R-b for the
patch - but your define says TOPI, which it is *not* accurate for.
That is confusing and should be noted.

> > What I was advocating for was picking one style and sticking to it.
> > These copy-paste from docs things are tedious and error prone to review,
> > and I don't think having multiple styles is helpful.
> 
> On the other hand, I think we should let developers choose a style
> which is better suited for a particular register field instead enforcing
> it here. The best we can do is follow a naming convention for defines.

Well shall have to agree to disagree I suppose!

> > Tedious as it was, I did check all the numbers though, so in that
> > respect:
> > Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> 
> BTW, this patch is shared with KVM AIA CSR series so most likely
> I will take this patch through that series.

Since the path which it gets applied is between you and Palmer to
decide, feel free to add the R-b whichever way the patch ends up going!

Thanks,
Conor.


[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

WARNING: multiple messages have this Message-ID (diff)
From: Conor Dooley <conor.dooley@microchip.com>
To: Anup Patel <apatel@ventanamicro.com>
Cc: Conor Dooley <conor@kernel.org>, Anup Patel <anup@brainfault.org>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Paul Walmsley <paul.walmsley@sifive.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Marc Zyngier <maz@kernel.org>, Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Atish Patra <atishp@atishpatra.org>,
	Alistair Francis <Alistair.Francis@wdc.com>,
	<linux-riscv@lists.infradead.org>, <linux-kernel@vger.kernel.org>,
	<devicetree@vger.kernel.org>
Subject: Re: [PATCH v2 1/9] RISC-V: Add AIA related CSR defines
Date: Fri, 27 Jan 2023 14:20:28 +0000	[thread overview]
Message-ID: <Y9PdrOvb8bYXpgVw@wendy> (raw)
In-Reply-To: <CAK9=C2VzJvpQLPedc+ruUnw8xDDDaC6_Vmj6qg1nXv+iqU-AfQ@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1859 bytes --]

On Fri, Jan 27, 2023 at 05:28:57PM +0530, Anup Patel wrote:
> On Wed, Jan 18, 2023 at 2:12 AM Conor Dooley <conor@kernel.org> wrote:

> > > > > +/* AIA CSR bits */
> > > > > +#define TOPI_IID_SHIFT               16
> > > > > +#define TOPI_IID_MASK                0xfff
> >
> > While I think of it, it'd be worth noting that these are generic across
> > all of topi, mtopi etc. Initially I thought that this mask was wrong as
> > the topi section says:
> >         bits 25:16 Interrupt identity (source number)
> >         bits 7:0 Interrupt priority
> 
> These defines are for the AIA CSRs and not AIA APLIC IDC registers.
> 
> As per the latest frozen spec, the mtopi/stopi/vstopi has following bits:
>     bits: 27:16 IID
>     bits: 7:0 IPRIO

I know, that those ones use those bits, hence leaving an R-b for the
patch - but your define says TOPI, which it is *not* accurate for.
That is confusing and should be noted.

> > What I was advocating for was picking one style and sticking to it.
> > These copy-paste from docs things are tedious and error prone to review,
> > and I don't think having multiple styles is helpful.
> 
> On the other hand, I think we should let developers choose a style
> which is better suited for a particular register field instead enforcing
> it here. The best we can do is follow a naming convention for defines.

Well shall have to agree to disagree I suppose!

> > Tedious as it was, I did check all the numbers though, so in that
> > respect:
> > Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> 
> BTW, this patch is shared with KVM AIA CSR series so most likely
> I will take this patch through that series.

Since the path which it gets applied is between you and Palmer to
decide, feel free to add the R-b whichever way the patch ends up going!

Thanks,
Conor.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

  reply	other threads:[~2023-01-27 14:21 UTC|newest]

Thread overview: 72+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-03 14:14 [PATCH v2 0/9] Linux RISC-V AIA Support Anup Patel
2023-01-03 14:14 ` Anup Patel
2023-01-03 14:14 ` [PATCH v2 1/9] RISC-V: Add AIA related CSR defines Anup Patel
2023-01-03 14:14   ` Anup Patel
2023-01-04 23:07   ` Conor Dooley
2023-01-04 23:07     ` Conor Dooley
2023-01-09  5:09     ` Anup Patel
2023-01-09  5:09       ` Anup Patel
2023-01-17 20:42       ` Conor Dooley
2023-01-17 20:42         ` Conor Dooley
2023-01-27 11:58         ` Anup Patel
2023-01-27 11:58           ` Anup Patel
2023-01-27 14:20           ` Conor Dooley [this message]
2023-01-27 14:20             ` Conor Dooley
2023-01-03 14:14 ` [PATCH v2 2/9] RISC-V: Detect AIA CSRs from ISA string Anup Patel
2023-01-03 14:14   ` Anup Patel
2023-01-03 14:14 ` [PATCH v2 3/9] irqchip/riscv-intc: Add support for RISC-V AIA Anup Patel
2023-01-03 14:14   ` Anup Patel
2023-01-13  9:39   ` Marc Zyngier
2023-01-13  9:39     ` Marc Zyngier
2023-01-03 14:14 ` [PATCH v2 4/9] dt-bindings: interrupt-controller: Add RISC-V incoming MSI controller Anup Patel
2023-01-03 14:14   ` Anup Patel
2023-01-04 23:21   ` Conor Dooley
2023-01-04 23:21     ` Conor Dooley
2023-02-20  3:15     ` Anup Patel
2023-02-20  3:15       ` Anup Patel
2023-01-12 20:49   ` Rob Herring
2023-01-12 20:49     ` Rob Herring
2023-02-20  3:20     ` Anup Patel
2023-02-20  3:20       ` Anup Patel
2023-02-19 11:17   ` Vivian Wang
2023-02-19 11:17     ` Vivian Wang
2023-02-20  3:31     ` Anup Patel
2023-02-20  3:31       ` Anup Patel
2023-01-03 14:14 ` [PATCH v2 5/9] irqchip: Add RISC-V incoming MSI controller driver Anup Patel
2023-01-03 14:14   ` Anup Patel
2023-01-13 10:10   ` Marc Zyngier
2023-01-13 10:10     ` Marc Zyngier
2023-05-01  8:28     ` Anup Patel
2023-05-01  8:28       ` Anup Patel
2023-05-01  8:44       ` Marc Zyngier
2023-05-01  8:44         ` Marc Zyngier
     [not found]   ` <CAPqJEFqhd-=-RYepKqnco7HySoxk7AhEctL+vzNozMSWe0mv7A@mail.gmail.com>
     [not found]     ` <CABvJ_xhcuC92A_oo1mWQoRvtRzE8XXx9bbXKs7N7wKm0=Z3_Cw@mail.gmail.com>
2023-01-18  3:49       ` Fwd: " Vincent Chen
2023-01-18  3:49         ` Vincent Chen
2023-01-18  4:20         ` Anup Patel
2023-01-18  4:20           ` Anup Patel
2023-01-03 14:14 ` [PATCH v2 6/9] dt-bindings: interrupt-controller: Add RISC-V advanced PLIC Anup Patel
2023-01-03 14:14   ` Anup Patel
2023-01-04 22:16   ` Conor Dooley
2023-01-04 22:16     ` Conor Dooley
2023-02-20  4:36     ` Anup Patel
2023-02-20  4:36       ` Anup Patel
2023-02-20 10:32       ` Conor Dooley
2023-02-20 10:32         ` Conor Dooley
2023-02-20 10:56         ` Conor Dooley
2023-02-20 10:56           ` Conor Dooley
2023-01-12 21:02   ` Rob Herring
2023-01-12 21:02     ` Rob Herring
2023-02-19 11:48   ` Vivian Wang
2023-02-19 11:48     ` Vivian Wang
2023-02-20  5:09     ` Anup Patel
2023-02-20  5:09       ` Anup Patel
2023-01-03 14:14 ` [PATCH v2 7/9] irqchip: Add RISC-V advanced PLIC driver Anup Patel
2023-01-03 14:14   ` Anup Patel
     [not found]   ` <CAPqJEFpmAvWiOdackxYwSPBfjo4DnTHXrXVSCC4snMn8tnZXPw@mail.gmail.com>
     [not found]     ` <CABvJ_xhjMa8xTsO-Qa23TOqxPpYxyBYSfV6TmKney-Gp3oi8cA@mail.gmail.com>
2023-01-17  7:09       ` Fwd: " Vincent Chen
2023-01-17  7:09         ` Vincent Chen
2023-01-18  4:37         ` Anup Patel
2023-01-18  4:37           ` Anup Patel
2023-01-03 14:14 ` [PATCH v2 8/9] RISC-V: Select APLIC and IMSIC drivers Anup Patel
2023-01-03 14:14   ` Anup Patel
2023-01-03 14:14 ` [PATCH v2 9/9] MAINTAINERS: Add entry for RISC-V AIA drivers Anup Patel
2023-01-03 14:14   ` Anup Patel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Y9PdrOvb8bYXpgVw@wendy \
    --to=conor.dooley@microchip.com \
    --cc=Alistair.Francis@wdc.com \
    --cc=anup@brainfault.org \
    --cc=apatel@ventanamicro.com \
    --cc=atishp@atishpatra.org \
    --cc=conor@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=krzysztof.kozlowski+dt@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=maz@kernel.org \
    --cc=palmer@dabbelt.com \
    --cc=paul.walmsley@sifive.com \
    --cc=robh+dt@kernel.org \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.