On Wed, Aug 09, 2023 at 10:24:57AM +0000, JeeHeng Sia wrote:
-----Original Message----- From: Conor Dooley conor@kernel.org Sent: Tuesday, August 8, 2023 9:13 PM To: JeeHeng Sia jeeheng.sia@starfivetech.com Cc: Conor Dooley conor.dooley@microchip.com; palmer@dabbelt.com; Paul Walmsley paul.walmsley@sifive.com; Atish Patra atishp@rivosinc.com; Anup Patel apatel@ventanamicro.com; Alexandre Ghiti alexghiti@rivosinc.com; Björn Töpel bjorn@rivosinc.com; Song Shuai suagrfillet@gmail.com; Petr Tesarik petrtesarik@huaweicloud.com; linux- riscv@lists.infradead.org; linux-kernel@vger.kernel.org; stable@vger.kernel.org Subject: Re: [RFT 1/2] RISC-V: handle missing "no-map" properties for OpenSBI's PMP protected regions
On Mon, Aug 07, 2023 at 12:44:07AM +0000, JeeHeng Sia wrote:
+/* SBI implementation IDs */ +#define SBI_IMP_OPENSBI 1
I would suggest to create an enum struct for the SBI Imp ID in the sbi.h file. What do you think?
I'm not really sure what the advantage of doing so is.
The macro SBI_IMP_OPENSBI seems weird (I would read it as "SBI Implementation OpenSBI").
That is what it is though, so I don't see what's weird about that.
However, if we implement an enum struct for SBI_IMP_ID
(There are numerous IDs available),
Ohh I know, but I didn't see the point adding those when I was only focusing on a single implementation.
the macro can be abbreviated to OpenSBI. By doing this, the conditional checking of the implementation ID would be more readable, as shown below: if (sbi_firmware_id != OPENSBI)
I don't see that it can become that simple, it'd still need to be prefixed with SBI_ to be consistent with any other SBI related enum, and at that point adding the extra IMP_ makes little odds.