linux-riscv.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Paul Walmsley <paul.walmsley@sifive.com>
To: Olof Johansson <olof@lixom.net>
Cc: damien.lemoal@wdc.com, Albert Ou <aou@eecs.berkeley.edu>,
	jason@lakedaemon.net, maz@kernel.org,
	Daniel Lezcano <daniel.lezcano@linaro.org>,
	linux-kernel@vger.kernel.org, Christoph Hellwig <hch@lst.de>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	linux-riscv@lists.infradead.org
Subject: Re: [PATCH] riscv: change CSR M/S defines to use "X" for prefix
Date: Wed, 18 Dec 2019 10:35:59 -0800 (PST)	[thread overview]
Message-ID: <alpine.DEB.2.21.9999.1912181030090.14542@viisi.sifive.com> (raw)
In-Reply-To: <20191218170603.58256-1-olof@lixom.net>

On Wed, 18 Dec 2019, Olof Johansson wrote:

> Commit a4c3733d32a7 ("riscv: abstract out CSR names for supervisor vs
> machine mode") introduced new non-S/M-specific defines for some of the
> CSRs and fields that differ for when you run the kernel in machine or
> supervisor mode.
> 
> One of those was "IRQ_TIMER" (instead of IRQ_S_TIMER/IRQ_M_MTIMER),
> which was generic enough to cause conflicts with other defines in
> drivers. Since it was in csr.h, it ended up getting pulled in through
> fairly generic include files, etc.
> 
> I looked at just renaming those, but for consistency I chose to rename all
> M/S symbols to using 'X' instead of 'S'/'M' in the identifiers instead,
> which gives them all less generic names.

This is what Christoph did originally.  I asked him to rename them to the 
generic versions to align with how we discuss these internally:

https://lore.kernel.org/linux-riscv/alpine.DEB.2.21.9999.1910181647110.21875@viisi.sifive.com/

I'd propose that we just start by prefixing the IRQ_TIMER macros with 
something like "RV_".  By prefixing the macros with a namespace 
identifier, that seems to solve the namespace problem more directly than 
sprinkling Xs around.  Then if it looks like there are other symbols that 
start conflicting, we can do the same in a larger patch for the other 
CSRs.

Of course we could also do the same to all CSRs up front.  Do you have a 
sense of what the current conflict situation is like with those?  The only 
one I'm aware of is with Mobiveil and predated this patch; it was fixed in 
the last merge window as far as I know.



- Paul


  reply	other threads:[~2019-12-18 18:36 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-18 17:06 [PATCH] riscv: change CSR M/S defines to use "X" for prefix Olof Johansson
2019-12-18 18:35 ` Paul Walmsley [this message]
2020-01-04  1:28 ` Paul Walmsley
2020-01-04  3:05   ` Olof Johansson
2020-01-07 11:15     ` Paul Walmsley
2020-01-23  0:22     ` Palmer Dabbelt

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=alpine.DEB.2.21.9999.1912181030090.14542@viisi.sifive.com \
    --to=paul.walmsley@sifive.com \
    --cc=aou@eecs.berkeley.edu \
    --cc=damien.lemoal@wdc.com \
    --cc=daniel.lezcano@linaro.org \
    --cc=hch@lst.de \
    --cc=jason@lakedaemon.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=maz@kernel.org \
    --cc=olof@lixom.net \
    --cc=palmer@dabbelt.com \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).