| Message ID | 20221222224104.3449841-1-vineetg@rivosinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Palmer Dabbelt |
| Headers | show |
| Series | riscv: add base extensions to enum riscv_isa_ext_id | expand |
| Context | Check | Description |
|---|---|---|
| conchuod/patch_count | success | Link |
| conchuod/cover_letter | success | Single patches do not need cover letters |
| conchuod/tree_selection | success | Guessed tree name to be for-next |
| conchuod/fixes_present | success | Fixes tag not required for -next series |
| conchuod/maintainers_pattern | success | MAINTAINERS pattern errors before the patch: 12 and now 12 |
| conchuod/verify_signedoff | success | Signed-off-by tag matches author and committer |
| conchuod/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| conchuod/module_param | success | Was 0 now: 0 |
| conchuod/alphanumeric_selects | success | Out of order selects before the patch: 57 and now 57 |
| conchuod/build_rv32_defconfig | success | Build OK |
| conchuod/build_warn_rv64 | success | Errors and warnings before: 0 this patch: 0 |
| conchuod/dtb_warn_rv64 | success | Errors and warnings before: 0 this patch: 0 |
| conchuod/header_inline | success | No static functions without inline keyword in header files |
| conchuod/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 31 lines checked |
| conchuod/source_inline | success | Was 0 now: 0 |
| conchuod/build_rv64_nommu_k210_defconfig | success | Build OK |
| conchuod/verify_fixes | success | No Fixes tag |
| conchuod/build_rv64_nommu_virt_defconfig | success | Build OK |
Hi Vineet, On Thu, Dec 22, 2022 at 02:41:04PM -0800, Vineet Gupta wrote: > This allows for using the enum in general to refer to any extension. What is the point of this patch? It's hard to understand the rationale without having any user, especially... > > Signed-off-by: Vineet Gupta <vineetg@rivosinc.com> > --- > arch/riscv/include/asm/hwcap.h | 19 +++++++++---------- > 1 file changed, 9 insertions(+), 10 deletions(-) > > diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h > index b22525290073..b861e711e3ac 100644 > --- a/arch/riscv/include/asm/hwcap.h > +++ b/arch/riscv/include/asm/hwcap.h > @@ -26,16 +26,6 @@ enum { > > extern unsigned long elf_hwcap; > > -#define RISCV_ISA_EXT_a ('a' - 'a') > -#define RISCV_ISA_EXT_c ('c' - 'a') > -#define RISCV_ISA_EXT_d ('d' - 'a') > -#define RISCV_ISA_EXT_f ('f' - 'a') > -#define RISCV_ISA_EXT_h ('h' - 'a') > -#define RISCV_ISA_EXT_i ('i' - 'a') > -#define RISCV_ISA_EXT_m ('m' - 'a') > -#define RISCV_ISA_EXT_s ('s' - 'a') > -#define RISCV_ISA_EXT_u ('u' - 'a') > - > /* > * Increse this to higher value as kernel support more ISA extensions. > */ > @@ -53,6 +43,15 @@ extern unsigned long elf_hwcap; > * available logical extension id. > */ > enum riscv_isa_ext_id { > + RISCV_ISA_EXT_a = ('a' - 'a'), > + RISCV_ISA_EXT_c = ('c' - 'a'), > + RISCV_ISA_EXT_d = ('d' - 'a'), > + RISCV_ISA_EXT_f = ('f' - 'a'), > + RISCV_ISA_EXT_h = ('h' - 'a'), > + RISCV_ISA_EXT_i = ('i' - 'a'), > + RISCV_ISA_EXT_m = ('m' - 'a'), > + RISCV_ISA_EXT_s = ('s' - 'a'), > + RISCV_ISA_EXT_u = ('u' - 'a'), ... as it is diametrically opposed to another patchset which deletes this enum, with a rationale. https://lore.kernel.org/linux-riscv/20221204174632.3677-5-jszhang@kernel.org/ +CC Jisheng. Also, please run get_maintainer.pl, there's a bunch of people touching this area at the moment & CCing them would be great. Thanks, Conor. btw, is the rivos list intentionally on CC? > RISCV_ISA_EXT_SSCOFPMF = RISCV_ISA_EXT_BASE, > RISCV_ISA_EXT_SVPBMT, > RISCV_ISA_EXT_ZICBOM, > -- > 2.34.1 > > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv
I got distracted, actually +CC Jisheng.. On Thu, Dec 22, 2022 at 10:53:59PM +0000, Conor Dooley wrote: > Hi Vineet, > > On Thu, Dec 22, 2022 at 02:41:04PM -0800, Vineet Gupta wrote: > > This allows for using the enum in general to refer to any extension. > > What is the point of this patch? It's hard to understand the rationale > without having any user, especially... > > > > > Signed-off-by: Vineet Gupta <vineetg@rivosinc.com> > > --- > > arch/riscv/include/asm/hwcap.h | 19 +++++++++---------- > > 1 file changed, 9 insertions(+), 10 deletions(-) > > > > diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h > > index b22525290073..b861e711e3ac 100644 > > --- a/arch/riscv/include/asm/hwcap.h > > +++ b/arch/riscv/include/asm/hwcap.h > > @@ -26,16 +26,6 @@ enum { > > > > extern unsigned long elf_hwcap; > > > > -#define RISCV_ISA_EXT_a ('a' - 'a') > > -#define RISCV_ISA_EXT_c ('c' - 'a') > > -#define RISCV_ISA_EXT_d ('d' - 'a') > > -#define RISCV_ISA_EXT_f ('f' - 'a') > > -#define RISCV_ISA_EXT_h ('h' - 'a') > > -#define RISCV_ISA_EXT_i ('i' - 'a') > > -#define RISCV_ISA_EXT_m ('m' - 'a') > > -#define RISCV_ISA_EXT_s ('s' - 'a') > > -#define RISCV_ISA_EXT_u ('u' - 'a') > > - > > /* > > * Increse this to higher value as kernel support more ISA extensions. > > */ > > @@ -53,6 +43,15 @@ extern unsigned long elf_hwcap; > > * available logical extension id. > > */ > > enum riscv_isa_ext_id { > > + RISCV_ISA_EXT_a = ('a' - 'a'), > > + RISCV_ISA_EXT_c = ('c' - 'a'), > > + RISCV_ISA_EXT_d = ('d' - 'a'), > > + RISCV_ISA_EXT_f = ('f' - 'a'), > > + RISCV_ISA_EXT_h = ('h' - 'a'), > > + RISCV_ISA_EXT_i = ('i' - 'a'), > > + RISCV_ISA_EXT_m = ('m' - 'a'), > > + RISCV_ISA_EXT_s = ('s' - 'a'), > > + RISCV_ISA_EXT_u = ('u' - 'a'), > > ... as it is diametrically opposed to another patchset which deletes > this enum, with a rationale. > https://lore.kernel.org/linux-riscv/20221204174632.3677-5-jszhang@kernel.org/ > > +CC Jisheng. > > Also, please run get_maintainer.pl, there's a bunch of people touching > this area at the moment & CCing them would be great. > > Thanks, > Conor. > > btw, is the rivos list intentionally on CC? > > > RISCV_ISA_EXT_SSCOFPMF = RISCV_ISA_EXT_BASE, > > RISCV_ISA_EXT_SVPBMT, > > RISCV_ISA_EXT_ZICBOM, > > -- > > 2.34.1 > > > > > > _______________________________________________ > > linux-riscv mailing list > > linux-riscv@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-riscv
On 12/22/22 14:53, Conor Dooley wrote: > Hi Vineet, > > On Thu, Dec 22, 2022 at 02:41:04PM -0800, Vineet Gupta wrote: >> This allows for using the enum in general to refer to any extension. > What is the point of this patch? It's hard to understand the rationale > without having any user, especially... Right I'm trying to build a cpu feature enable API which required handling of all extensions. Ideally yes this needs to go there. But it seemed more like a cleanup type patch to warrant a standalone submission. But I guess I can wait for dust to settle. > >> + RISCV_ISA_EXT_s = ('s' - 'a'), >> + RISCV_ISA_EXT_u = ('u' - 'a'), > ... as it is diametrically opposed to another patchset which deletes > this enum, with a rationale. > https://lore.kernel.org/linux-riscv/20221204174632.3677-5-jszhang@kernel.org/ > > +CC Jisheng. > > Also, please run get_maintainer.pl, there's a bunch of people touching > this area at the moment & CCing them would be great. Ok sure. > Thanks, > Conor. > > btw, is the rivos list intentionally on CC? Yep.
On Thu, Dec 22, 2022 at 03:13:56PM -0800, Vineet Gupta wrote: > > > On 12/22/22 14:53, Conor Dooley wrote: > > Hi Vineet, > > > > On Thu, Dec 22, 2022 at 02:41:04PM -0800, Vineet Gupta wrote: > > > This allows for using the enum in general to refer to any extension. > > What is the point of this patch? It's hard to understand the rationale > > without having any user, especially... > > Right I'm trying to build a cpu feature enable API which required handling > of all extensions. Does it matter to you if it's defines or an enum, as long as every extension is exposed to you via the same mechanism? > Ideally yes this needs to go there. But it seemed more like a cleanup type > patch to warrant a standalone submission. Meh, dw. Not expecting people to know about every patch under the sun.
On 12/22/22 15:35, Conor Dooley wrote: > On Thu, Dec 22, 2022 at 03:13:56PM -0800, Vineet Gupta wrote: >> >> On 12/22/22 14:53, Conor Dooley wrote: >>> Hi Vineet, >>> >>> On Thu, Dec 22, 2022 at 02:41:04PM -0800, Vineet Gupta wrote: >>>> This allows for using the enum in general to refer to any extension. >>> What is the point of this patch? It's hard to understand the rationale >>> without having any user, especially... >> Right I'm trying to build a cpu feature enable API which required handling >> of all extensions. > Does it matter to you if it's defines or an enum, as long as every > extension is exposed to you via the same mechanism? No, I just went with what v6.1 release code base had. > >> Ideally yes this needs to go there. But it seemed more like a cleanup type >> patch to warrant a standalone submission. > Meh, dw. Not expecting people to know about every patch under the sun. No this patch doesn't require people to know what series it facilitates since IMO it is a trivial cleanup - fixing 2 disparate ways of defining extensions. And as I said it is not urgent, specially with other changes in flight. Thx, -Vineet
On 23 December 2022 00:03:18 GMT, Vineet Gupta <vineetg@rivosinc.com> wrote: > > >On 12/22/22 15:35, Conor Dooley wrote: >> On Thu, Dec 22, 2022 at 03:13:56PM -0800, Vineet Gupta wrote: >>> >>> On 12/22/22 14:53, Conor Dooley wrote: >>>> Hi Vineet, >>>> >>>> On Thu, Dec 22, 2022 at 02:41:04PM -0800, Vineet Gupta wrote: >>>>> This allows for using the enum in general to refer to any extension. >>>> What is the point of this patch? It's hard to understand the rationale >>>> without having any user, especially... >>> Right I'm trying to build a cpu feature enable API which required handling >>> of all extensions. >> Does it matter to you if it's defines or an enum, as long as every >> extension is exposed to you via the same mechanism? > >No, I just went with what v6.1 release code base had. > >> >>> Ideally yes this needs to go there. But it seemed more like a cleanup type >>> patch to warrant a standalone submission. >> Meh, dw. Not expecting people to know about every patch under the sun. > >No this patch doesn't require people to know what series it facilitates since IMO it is a trivial cleanup - fixing 2 disparate ways of defining extensions. I meant it's not a problem that you weren't aware of what the other series is doing. >And as I said it is not urgent, specially with other changes in flight. > >Thx, >-Vineet
diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h index b22525290073..b861e711e3ac 100644 --- a/arch/riscv/include/asm/hwcap.h +++ b/arch/riscv/include/asm/hwcap.h @@ -26,16 +26,6 @@ enum { extern unsigned long elf_hwcap; -#define RISCV_ISA_EXT_a ('a' - 'a') -#define RISCV_ISA_EXT_c ('c' - 'a') -#define RISCV_ISA_EXT_d ('d' - 'a') -#define RISCV_ISA_EXT_f ('f' - 'a') -#define RISCV_ISA_EXT_h ('h' - 'a') -#define RISCV_ISA_EXT_i ('i' - 'a') -#define RISCV_ISA_EXT_m ('m' - 'a') -#define RISCV_ISA_EXT_s ('s' - 'a') -#define RISCV_ISA_EXT_u ('u' - 'a') - /* * Increse this to higher value as kernel support more ISA extensions. */ @@ -53,6 +43,15 @@ extern unsigned long elf_hwcap; * available logical extension id. */ enum riscv_isa_ext_id { + RISCV_ISA_EXT_a = ('a' - 'a'), + RISCV_ISA_EXT_c = ('c' - 'a'), + RISCV_ISA_EXT_d = ('d' - 'a'), + RISCV_ISA_EXT_f = ('f' - 'a'), + RISCV_ISA_EXT_h = ('h' - 'a'), + RISCV_ISA_EXT_i = ('i' - 'a'), + RISCV_ISA_EXT_m = ('m' - 'a'), + RISCV_ISA_EXT_s = ('s' - 'a'), + RISCV_ISA_EXT_u = ('u' - 'a'), RISCV_ISA_EXT_SSCOFPMF = RISCV_ISA_EXT_BASE, RISCV_ISA_EXT_SVPBMT, RISCV_ISA_EXT_ZICBOM,
This allows for using the enum in general to refer to any extension. Signed-off-by: Vineet Gupta <vineetg@rivosinc.com> --- arch/riscv/include/asm/hwcap.h | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-)