This can also do without the ifdef and use IS_ENABLED instead and
for better readability, getting rid of that switch also seems
waranted.
Signed-off-by: Heiko Stuebner <[email protected]>
---
arch/riscv/kernel/cpufeature.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index 553d755483ed..764ea220161f 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -253,16 +253,13 @@ void __init riscv_fill_hwcap(void)
#ifdef CONFIG_RISCV_ALTERNATIVE
static bool __init_or_module cpufeature_probe_svpbmt(unsigned int stage)
{
-#ifdef CONFIG_RISCV_ISA_SVPBMT
- switch (stage) {
- case RISCV_ALTERNATIVES_EARLY_BOOT:
+ if (!IS_ENABLED(CONFIG_RISCV_ISA_SVPBMT))
return false;
- default:
- return riscv_isa_extension_available(NULL, SVPBMT);
- }
-#endif
- return false;
+ if (stage == RISCV_ALTERNATIVES_EARLY_BOOT)
+ return false;
+
+ return riscv_isa_extension_available(NULL, SVPBMT);
}
static bool __init_or_module cpufeature_probe_zicbom(unsigned int stage)
--
2.35.1
The t-head variant of page-based memory types should also check first
for the enabled kernel config option.
Signed-off-by: Heiko Stuebner <[email protected]>
---
arch/riscv/errata/thead/errata.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/riscv/errata/thead/errata.c b/arch/riscv/errata/thead/errata.c
index a6f4bd8ccf3f..902e12452821 100644
--- a/arch/riscv/errata/thead/errata.c
+++ b/arch/riscv/errata/thead/errata.c
@@ -17,6 +17,9 @@
static bool errata_probe_pbmt(unsigned int stage,
unsigned long arch_id, unsigned long impid)
{
+ if (!IS_ENABLED(CONFIG_ERRATA_THEAD_PBMT))
+ return false;
+
if (arch_id != 0 || impid != 0)
return false;
--
2.35.1
Reviewed-by: Guo Ren <[email protected]>
On Fri, Sep 2, 2022 at 6:28 AM Heiko Stuebner <[email protected]> wrote:
>
> This can also do without the ifdef and use IS_ENABLED instead and
> for better readability, getting rid of that switch also seems
> waranted.
>
> Signed-off-by: Heiko Stuebner <[email protected]>
> ---
> arch/riscv/kernel/cpufeature.c | 13 +++++--------
> 1 file changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index 553d755483ed..764ea220161f 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -253,16 +253,13 @@ void __init riscv_fill_hwcap(void)
> #ifdef CONFIG_RISCV_ALTERNATIVE
> static bool __init_or_module cpufeature_probe_svpbmt(unsigned int stage)
> {
> -#ifdef CONFIG_RISCV_ISA_SVPBMT
> - switch (stage) {
> - case RISCV_ALTERNATIVES_EARLY_BOOT:
> + if (!IS_ENABLED(CONFIG_RISCV_ISA_SVPBMT))
> return false;
> - default:
> - return riscv_isa_extension_available(NULL, SVPBMT);
> - }
> -#endif
>
> - return false;
> + if (stage == RISCV_ALTERNATIVES_EARLY_BOOT)
> + return false;
> +
> + return riscv_isa_extension_available(NULL, SVPBMT);
> }
>
> static bool __init_or_module cpufeature_probe_zicbom(unsigned int stage)
> --
> 2.35.1
>
--
Best Regards
Guo Ren
Is it a Fixes?
On Fri, Sep 2, 2022 at 6:28 AM Heiko Stuebner <[email protected]> wrote:
>
> The t-head variant of page-based memory types should also check first
> for the enabled kernel config option.
>
> Signed-off-by: Heiko Stuebner <[email protected]>
> ---
> arch/riscv/errata/thead/errata.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/arch/riscv/errata/thead/errata.c b/arch/riscv/errata/thead/errata.c
> index a6f4bd8ccf3f..902e12452821 100644
> --- a/arch/riscv/errata/thead/errata.c
> +++ b/arch/riscv/errata/thead/errata.c
> @@ -17,6 +17,9 @@
> static bool errata_probe_pbmt(unsigned int stage,
> unsigned long arch_id, unsigned long impid)
> {
> + if (!IS_ENABLED(CONFIG_ERRATA_THEAD_PBMT))
> + return false;
> +
> if (arch_id != 0 || impid != 0)
> return false;
>
> --
> 2.35.1
>
--
Best Regards
Guo Ren
On 02/09/2022 02:06, Guo Ren wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> Is it a Fixes?
Looks like one to me, seems a fixes tag would be good to
have here... Either way:
Reviewed-by: Conor Dooley <[email protected]>
>
> On Fri, Sep 2, 2022 at 6:28 AM Heiko Stuebner <[email protected]> wrote:
>>
>> The t-head variant of page-based memory types should also check first
>> for the enabled kernel config option.
>>
>> Signed-off-by: Heiko Stuebner <[email protected]>
>> ---
>> arch/riscv/errata/thead/errata.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/arch/riscv/errata/thead/errata.c b/arch/riscv/errata/thead/errata.c
>> index a6f4bd8ccf3f..902e12452821 100644
>> --- a/arch/riscv/errata/thead/errata.c
>> +++ b/arch/riscv/errata/thead/errata.c
>> @@ -17,6 +17,9 @@
>> static bool errata_probe_pbmt(unsigned int stage,
>> unsigned long arch_id, unsigned long impid)
>> {
>> + if (!IS_ENABLED(CONFIG_ERRATA_THEAD_PBMT))
>> + return false;
>> +
>> if (arch_id != 0 || impid != 0)
>> return false;
>>
>> --
>> 2.35.1
>>
>
>
> --
> Best Regards
> Guo Ren
On 01/09/2022 23:27, Heiko Stuebner wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> This can also do without the ifdef and use IS_ENABLED instead and
> for better readability, getting rid of that switch also seems
> waranted.
The change itself looks great to me, but the commit message here
does not stand by itself - the "also" stuff reads a bit oddly.
How about:
---8<---
For better readability (and compile time coverage) use IS_ENABLED
instead of ifdef and drop the new unneeded switch statement.
---8<---
Call me biased, but I much prefer how this looks now...
Reviewed-by: Conor Dooley <[email protected]>
>
> Signed-off-by: Heiko Stuebner <[email protected]>
> ---
> arch/riscv/kernel/cpufeature.c | 13 +++++--------
> 1 file changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index 553d755483ed..764ea220161f 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -253,16 +253,13 @@ void __init riscv_fill_hwcap(void)
> #ifdef CONFIG_RISCV_ALTERNATIVE
> static bool __init_or_module cpufeature_probe_svpbmt(unsigned int stage)
> {
> -#ifdef CONFIG_RISCV_ISA_SVPBMT
> - switch (stage) {
> - case RISCV_ALTERNATIVES_EARLY_BOOT:
> + if (!IS_ENABLED(CONFIG_RISCV_ISA_SVPBMT))
> return false;
> - default:
> - return riscv_isa_extension_available(NULL, SVPBMT);
> - }
> -#endif
>
> - return false;
> + if (stage == RISCV_ALTERNATIVES_EARLY_BOOT)
> + return false;
> +
> + return riscv_isa_extension_available(NULL, SVPBMT);
> }
>
> static bool __init_or_module cpufeature_probe_zicbom(unsigned int stage)
> --
> 2.35.1
>
Hi Heiko,
Please use a cover-letter for a patch series. They allow the series to be
threaded better and people can reply to the cover-letter with series-wide
comments. For example, I'd like to reply to a cover-letter now with
For the series
Reviewed-by: Andrew Jones <[email protected]>
but now it looks like I need to go back and reply to each patch
separately.
Thanks,
drew
On Fri, Sep 02, 2022 at 12:27:41AM +0200, Heiko Stuebner wrote:
> This can also do without the ifdef and use IS_ENABLED instead and
> for better readability, getting rid of that switch also seems
> waranted.
>
> Signed-off-by: Heiko Stuebner <[email protected]>
> ---
> arch/riscv/kernel/cpufeature.c | 13 +++++--------
> 1 file changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index 553d755483ed..764ea220161f 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -253,16 +253,13 @@ void __init riscv_fill_hwcap(void)
> #ifdef CONFIG_RISCV_ALTERNATIVE
> static bool __init_or_module cpufeature_probe_svpbmt(unsigned int stage)
> {
> -#ifdef CONFIG_RISCV_ISA_SVPBMT
> - switch (stage) {
> - case RISCV_ALTERNATIVES_EARLY_BOOT:
> + if (!IS_ENABLED(CONFIG_RISCV_ISA_SVPBMT))
> return false;
> - default:
> - return riscv_isa_extension_available(NULL, SVPBMT);
> - }
> -#endif
>
> - return false;
> + if (stage == RISCV_ALTERNATIVES_EARLY_BOOT)
> + return false;
> +
> + return riscv_isa_extension_available(NULL, SVPBMT);
> }
>
> static bool __init_or_module cpufeature_probe_zicbom(unsigned int stage)
> --
> 2.35.1
>
>
> _______________________________________________
> linux-riscv mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-riscv
On Fri, Sep 02, 2022 at 12:27:41AM +0200, Heiko Stuebner wrote:
> This can also do without the ifdef and use IS_ENABLED instead and
> for better readability, getting rid of that switch also seems
> waranted.
>
> Signed-off-by: Heiko Stuebner <[email protected]>
> ---
> arch/riscv/kernel/cpufeature.c | 13 +++++--------
> 1 file changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index 553d755483ed..764ea220161f 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -253,16 +253,13 @@ void __init riscv_fill_hwcap(void)
> #ifdef CONFIG_RISCV_ALTERNATIVE
> static bool __init_or_module cpufeature_probe_svpbmt(unsigned int stage)
> {
> -#ifdef CONFIG_RISCV_ISA_SVPBMT
> - switch (stage) {
> - case RISCV_ALTERNATIVES_EARLY_BOOT:
> + if (!IS_ENABLED(CONFIG_RISCV_ISA_SVPBMT))
> return false;
> - default:
> - return riscv_isa_extension_available(NULL, SVPBMT);
> - }
> -#endif
>
> - return false;
> + if (stage == RISCV_ALTERNATIVES_EARLY_BOOT)
> + return false;
> +
> + return riscv_isa_extension_available(NULL, SVPBMT);
> }
>
> static bool __init_or_module cpufeature_probe_zicbom(unsigned int stage)
> --
> 2.35.1
>
Reviewed-by: Andrew Jones <[email protected]>
On Fri, Sep 02, 2022 at 12:27:44AM +0200, Heiko Stuebner wrote:
> The t-head variant of page-based memory types should also check first
> for the enabled kernel config option.
>
> Signed-off-by: Heiko Stuebner <[email protected]>
> ---
> arch/riscv/errata/thead/errata.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/arch/riscv/errata/thead/errata.c b/arch/riscv/errata/thead/errata.c
> index a6f4bd8ccf3f..902e12452821 100644
> --- a/arch/riscv/errata/thead/errata.c
> +++ b/arch/riscv/errata/thead/errata.c
> @@ -17,6 +17,9 @@
> static bool errata_probe_pbmt(unsigned int stage,
> unsigned long arch_id, unsigned long impid)
> {
> + if (!IS_ENABLED(CONFIG_ERRATA_THEAD_PBMT))
> + return false;
> +
> if (arch_id != 0 || impid != 0)
> return false;
>
> --
> 2.35.1
>
Reviewed-by: Andrew Jones <[email protected]>
Am Freitag, 2. September 2022, 11:33:27 CEST schrieb [email protected]:
> On 02/09/2022 02:06, Guo Ren wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >
> > Is it a Fixes?
>
> Looks like one to me, seems a fixes tag would be good to
> have here... Either way:
> Reviewed-by: Conor Dooley <[email protected]>
The alternative itself also is protected by the kconfig-option,
so even if probe says "yes", nothing will be patched when
CONFIG_ERRATA_THEAD_PBMT is not enabled.
So for the memory-types it's more a change to keep it consistent
with the other extensions. But I guess we can add the fixes-tag
anyway, as it makes sure that doesn't get copy-pasted somewhere
else :-)
Heiko
> >
> > On Fri, Sep 2, 2022 at 6:28 AM Heiko Stuebner <[email protected]> wrote:
> >>
> >> The t-head variant of page-based memory types should also check first
> >> for the enabled kernel config option.
> >>
> >> Signed-off-by: Heiko Stuebner <[email protected]>
> >> ---
> >> arch/riscv/errata/thead/errata.c | 3 +++
> >> 1 file changed, 3 insertions(+)
> >>
> >> diff --git a/arch/riscv/errata/thead/errata.c b/arch/riscv/errata/thead/errata.c
> >> index a6f4bd8ccf3f..902e12452821 100644
> >> --- a/arch/riscv/errata/thead/errata.c
> >> +++ b/arch/riscv/errata/thead/errata.c
> >> @@ -17,6 +17,9 @@
> >> static bool errata_probe_pbmt(unsigned int stage,
> >> unsigned long arch_id, unsigned long impid)
> >> {
> >> + if (!IS_ENABLED(CONFIG_ERRATA_THEAD_PBMT))
> >> + return false;
> >> +
> >> if (arch_id != 0 || impid != 0)
> >> return false;
> >>
> >> --
> >> 2.35.1
> >>
> >
> >
> > --
> > Best Regards
> > Guo Ren
>
>
Am Freitag, 2. September 2022, 11:49:39 CEST schrieb Andrew Jones:
> Hi Heiko,
>
> Please use a cover-letter for a patch series. They allow the series to be
> threaded better and people can reply to the cover-letter with series-wide
> comments. For example, I'd like to reply to a cover-letter now with
>
> For the series
>
> Reviewed-by: Andrew Jones <[email protected]>
>
> but now it looks like I need to go back and reply to each patch
> separately.
I'm not sure if tooling like b4 can handle Reviewed-by's in cover-letters.
At least some time back it couldn't, so am not sure if that was added
meanwhile. So tags added to cover-letters might even get lost.
But I'll add a cover-letter nevertheless - need a place for the v2 changelog
anyway :-)
Heiko
>
> Thanks,
> drew
>
> On Fri, Sep 02, 2022 at 12:27:41AM +0200, Heiko Stuebner wrote:
> > This can also do without the ifdef and use IS_ENABLED instead and
> > for better readability, getting rid of that switch also seems
> > waranted.
> >
> > Signed-off-by: Heiko Stuebner <[email protected]>
> > ---
> > arch/riscv/kernel/cpufeature.c | 13 +++++--------
> > 1 file changed, 5 insertions(+), 8 deletions(-)
> >
> > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> > index 553d755483ed..764ea220161f 100644
> > --- a/arch/riscv/kernel/cpufeature.c
> > +++ b/arch/riscv/kernel/cpufeature.c
> > @@ -253,16 +253,13 @@ void __init riscv_fill_hwcap(void)
> > #ifdef CONFIG_RISCV_ALTERNATIVE
> > static bool __init_or_module cpufeature_probe_svpbmt(unsigned int stage)
> > {
> > -#ifdef CONFIG_RISCV_ISA_SVPBMT
> > - switch (stage) {
> > - case RISCV_ALTERNATIVES_EARLY_BOOT:
> > + if (!IS_ENABLED(CONFIG_RISCV_ISA_SVPBMT))
> > return false;
> > - default:
> > - return riscv_isa_extension_available(NULL, SVPBMT);
> > - }
> > -#endif
> >
> > - return false;
> > + if (stage == RISCV_ALTERNATIVES_EARLY_BOOT)
> > + return false;
> > +
> > + return riscv_isa_extension_available(NULL, SVPBMT);
> > }
> >
> > static bool __init_or_module cpufeature_probe_zicbom(unsigned int stage)
>
On 02/09/2022 16:12, Heiko Stübner wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> Am Freitag, 2. September 2022, 11:49:39 CEST schrieb Andrew Jones:
>> Hi Heiko,
>>
>> Please use a cover-letter for a patch series. They allow the series to be
>> threaded better and people can reply to the cover-letter with series-wide
>> comments. For example, I'd like to reply to a cover-letter now with
>>
>> For the series
>>
>> Reviewed-by: Andrew Jones <[email protected]>
>>
>> but now it looks like I need to go back and reply to each patch
>> separately.
>
> I'm not sure if tooling like b4 can handle Reviewed-by's in cover-letters.
Yup, it can! At least `b4 {am,shazam} -t` will.
I am not sure if the new `b4 trailers` does.
> At least some time back it couldn't, so am not sure if that was added
> meanwhile. So tags added to cover-letters might even get lost.
>
> But I'll add a cover-letter nevertheless - need a place for the v2 changelog
> anyway :-)
>
> Heiko
>
>
>>
>> Thanks,
>> drew
>>
>> On Fri, Sep 02, 2022 at 12:27:41AM +0200, Heiko Stuebner wrote:
>>> This can also do without the ifdef and use IS_ENABLED instead and
>>> for better readability, getting rid of that switch also seems
>>> waranted.
>>>
>>> Signed-off-by: Heiko Stuebner <[email protected]>
>>> ---
>>> arch/riscv/kernel/cpufeature.c | 13 +++++--------
>>> 1 file changed, 5 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
>>> index 553d755483ed..764ea220161f 100644
>>> --- a/arch/riscv/kernel/cpufeature.c
>>> +++ b/arch/riscv/kernel/cpufeature.c
>>> @@ -253,16 +253,13 @@ void __init riscv_fill_hwcap(void)
>>> #ifdef CONFIG_RISCV_ALTERNATIVE
>>> static bool __init_or_module cpufeature_probe_svpbmt(unsigned int stage)
>>> {
>>> -#ifdef CONFIG_RISCV_ISA_SVPBMT
>>> - switch (stage) {
>>> - case RISCV_ALTERNATIVES_EARLY_BOOT:
>>> + if (!IS_ENABLED(CONFIG_RISCV_ISA_SVPBMT))
>>> return false;
>>> - default:
>>> - return riscv_isa_extension_available(NULL, SVPBMT);
>>> - }
>>> -#endif
>>>
>>> - return false;
>>> + if (stage == RISCV_ALTERNATIVES_EARLY_BOOT)
>>> + return false;
>>> +
>>> + return riscv_isa_extension_available(NULL, SVPBMT);
>>> }
>>>
>>> static bool __init_or_module cpufeature_probe_zicbom(unsigned int stage)
>>
>
>
>
>
Am Freitag, 2. September 2022, 17:26:21 CEST schrieb [email protected]:
> On 02/09/2022 16:12, Heiko St?bner wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >
> > Am Freitag, 2. September 2022, 11:49:39 CEST schrieb Andrew Jones:
> >> Hi Heiko,
> >>
> >> Please use a cover-letter for a patch series. They allow the series to be
> >> threaded better and people can reply to the cover-letter with series-wide
> >> comments. For example, I'd like to reply to a cover-letter now with
> >>
> >> For the series
> >>
> >> Reviewed-by: Andrew Jones <[email protected]>
> >>
> >> but now it looks like I need to go back and reply to each patch
> >> separately.
> >
> > I'm not sure if tooling like b4 can handle Reviewed-by's in cover-letters.
>
> Yup, it can! At least `b4 {am,shazam} -t` will.
> I am not sure if the new `b4 trailers` does.
That is great to know ... gotta love b4 :-)
>
> > At least some time back it couldn't, so am not sure if that was added
> > meanwhile. So tags added to cover-letters might even get lost.
> >
> > But I'll add a cover-letter nevertheless - need a place for the v2 changelog
> > anyway :-)
> >
> > Heiko
> >
> >
> >>
> >> Thanks,
> >> drew
> >>
> >> On Fri, Sep 02, 2022 at 12:27:41AM +0200, Heiko Stuebner wrote:
> >>> This can also do without the ifdef and use IS_ENABLED instead and
> >>> for better readability, getting rid of that switch also seems
> >>> waranted.
> >>>
> >>> Signed-off-by: Heiko Stuebner <[email protected]>
> >>> ---
> >>> arch/riscv/kernel/cpufeature.c | 13 +++++--------
> >>> 1 file changed, 5 insertions(+), 8 deletions(-)
> >>>
> >>> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> >>> index 553d755483ed..764ea220161f 100644
> >>> --- a/arch/riscv/kernel/cpufeature.c
> >>> +++ b/arch/riscv/kernel/cpufeature.c
> >>> @@ -253,16 +253,13 @@ void __init riscv_fill_hwcap(void)
> >>> #ifdef CONFIG_RISCV_ALTERNATIVE
> >>> static bool __init_or_module cpufeature_probe_svpbmt(unsigned int stage)
> >>> {
> >>> -#ifdef CONFIG_RISCV_ISA_SVPBMT
> >>> - switch (stage) {
> >>> - case RISCV_ALTERNATIVES_EARLY_BOOT:
> >>> + if (!IS_ENABLED(CONFIG_RISCV_ISA_SVPBMT))
> >>> return false;
> >>> - default:
> >>> - return riscv_isa_extension_available(NULL, SVPBMT);
> >>> - }
> >>> -#endif
> >>>
> >>> - return false;
> >>> + if (stage == RISCV_ALTERNATIVES_EARLY_BOOT)
> >>> + return false;
> >>> +
> >>> + return riscv_isa_extension_available(NULL, SVPBMT);
> >>> }
> >>>
> >>> static bool __init_or_module cpufeature_probe_zicbom(unsigned int stage)
> >>
> >
> >
> >
> >
>
>
On Fri, Sep 02, 2022 at 03:26:21PM +0000, [email protected] wrote:
> >> Reviewed-by: Andrew Jones <[email protected]>
> >>
> >> but now it looks like I need to go back and reply to each patch
> >> separately.
> >
> > I'm not sure if tooling like b4 can handle Reviewed-by's in cover-letters.
>
> Yup, it can! At least `b4 {am,shazam} -t` will.
> I am not sure if the new `b4 trailers` does.
Yes, it's the default behaviour for "b4 trailers". It'll probably become the
default for "b4 am/shazam" at some point, too.
-K