[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <76ed70d0-c892-780f-338d-da8f35fa14c6@linux.ibm.com>
Date: Wed, 18 Mar 2020 11:20:01 +0530
From: Ravi Bangoria <ravi.bangoria@...ux.ibm.com>
To: Christophe Leroy <christophe.leroy@....fr>
Cc: mpe@...erman.id.au, mikey@...ling.org, apopple@...ux.ibm.com,
paulus@...ba.org, npiggin@...il.com,
naveen.n.rao@...ux.vnet.ibm.com, peterz@...radead.org,
jolsa@...nel.org, oleg@...hat.com, fweisbec@...il.com,
mingo@...nel.org, linuxppc-dev@...ts.ozlabs.org,
linux-kernel@...r.kernel.org,
Ravi Bangoria <ravi.bangoria@...ux.ibm.com>
Subject: Re: [PATCH 03/15] powerpc/watchpoint: Introduce function to get nr
watchpoints dynamically
>> diff --git a/arch/powerpc/include/asm/hw_breakpoint.h b/arch/powerpc/include/asm/hw_breakpoint.h
>> index f2f8d8aa8e3b..741c4f7573c4 100644
>> --- a/arch/powerpc/include/asm/hw_breakpoint.h
>> +++ b/arch/powerpc/include/asm/hw_breakpoint.h
>> @@ -43,6 +43,8 @@ struct arch_hw_breakpoint {
>> #define DABR_MAX_LEN 8
>> #define DAWR_MAX_LEN 512
>> +extern int nr_wp_slots(void);
>
> 'extern' keyword is unneeded and irrelevant here. Please remove it. Even checkpatch is unhappy (https://openpower.xyz/job/snowpatch/job/snowpatch-linux-checkpatch/12172//artifact/linux/checkpatch.log)
Sure.
...
>> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
>> index 110db94cdf3c..6d4b029532e2 100644
>> --- a/arch/powerpc/kernel/process.c
>> +++ b/arch/powerpc/kernel/process.c
>> @@ -835,6 +835,12 @@ static inline bool hw_brk_match(struct arch_hw_breakpoint *a,
>> return true;
>> }
>> +/* Returns total number of data breakpoints available. */
>> +int nr_wp_slots(void)
>> +{
>> + return HBP_NUM_MAX;
>> +}
>> +
>
> This is not worth a global function. At least it should be a static function located in hw_breakpoint.c. But it would be even better to have it as a static inline in asm/hw_breakpoint.h
Makes sense. Will change it.
Thanks.
Powered by blists - more mailing lists