[v2,10/17] drivers/firmware/sdei: Cleanup on cross call function
diff mbox series

Message ID 20200722095740.28560-11-gshan@redhat.com
State New
Headers show
Series
  • Refactor SDEI client driver
Related show

Commit Message

Gavin Shan July 22, 2020, 9:57 a.m. UTC
This applies cleanup on the corss call functions, no functional
changes is introduced:

   * Cleanup struct sdei_crosscall_arg to use tab between fields
     and their types
   * Refactor CROSSCALL_INIT to use "do { ... } while (0)" to be
     compatible with scripts/checkpatch.pl
   * Remove unnecessary space before @event in sdei_do_cross_call()

Signed-off-by: Gavin Shan <gshan@redhat.com>
---
v2: Drop changes to sdei_cross_call_return()
    Remove unnecessary space before @event in sdei_do_cross_call()
---
 drivers/firmware/arm_sdei.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

Comments

Jonathan Cameron July 23, 2020, 3:52 p.m. UTC | #1
On Wed, 22 Jul 2020 19:57:33 +1000
Gavin Shan <gshan@redhat.com> wrote:

> This applies cleanup on the corss call functions, no functional

cross

> changes is introduced:

changes are introduced:

> 
>    * Cleanup struct sdei_crosscall_arg to use tab between fields
>      and their types
Hmm. I guess there is a consistency argument for doing this but generally
that one doesn't look like it actually has much benefit.

>    * Refactor CROSSCALL_INIT to use "do { ... } while (0)" to be
>      compatible with scripts/checkpatch.pl
Good to tidy that up.

>    * Remove unnecessary space before @event in sdei_do_cross_call()

Good change, but seems to have gotten lost in this patch.

> 
> Signed-off-by: Gavin Shan <gshan@redhat.com>
> ---
> v2: Drop changes to sdei_cross_call_return()
>     Remove unnecessary space before @event in sdei_do_cross_call()
> ---
>  drivers/firmware/arm_sdei.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/firmware/arm_sdei.c b/drivers/firmware/arm_sdei.c
> index d03371161aaf..04dc144a8f31 100644
> --- a/drivers/firmware/arm_sdei.c
> +++ b/drivers/firmware/arm_sdei.c
> @@ -73,14 +73,17 @@ static LIST_HEAD(sdei_list);
>  
>  /* Private events are registered/enabled via IPI passing one of these */
>  struct sdei_crosscall_args {
> -	struct sdei_event *event;
> -	atomic_t errors;
> -	int first_error;
> +	struct sdei_event	*event;
> +	atomic_t		errors;
> +	int			first_error;
>  };
>  
> -#define CROSSCALL_INIT(arg, event)	(arg.event = event, \
> -					 arg.first_error = 0, \
> -					 atomic_set(&arg.errors, 0))
> +#define CROSSCALL_INIT(arg, event)		\
> +	do {					\
> +		arg.event = event;		\
> +		arg.first_error = 0;		\
> +		atomic_set(&arg.errors, 0);	\
> +	} while (0)
>  
>  static inline int sdei_do_cross_call(void *fn, struct sdei_event * event)
>  {
Gavin Shan July 27, 2020, 12:33 a.m. UTC | #2
Hi Jonathan,

On 7/24/20 1:52 AM, Jonathan Cameron wrote:
> On Wed, 22 Jul 2020 19:57:33 +1000
> Gavin Shan <gshan@redhat.com> wrote:
> 
>> This applies cleanup on the corss call functions, no functional
> 
> cross
> 

It will be fixed in v3.

>> changes is introduced:
> 
> changes are introduced:
> 

It will be fixed in v3.

>>
>>     * Cleanup struct sdei_crosscall_arg to use tab between fields
>>       and their types
> Hmm. I guess there is a consistency argument for doing this but generally
> that one doesn't look like it actually has much benefit.
> 

Sorry, I guess you're talking about "./scripts/cleanpatch"? As stated
in the head of the script, the result would be destructive. So I think
manual changes would be more reliable. Lets keep this in v3 if you agree.

Extracted from "scripts/cleanpatch":

# Clean a patch file -- or directory of patch files -- of stealth whitespace.
# WARNING: this can be a highly destructive operation.  Use with caution.
#


>>     * Refactor CROSSCALL_INIT to use "do { ... } while (0)" to be
>>       compatible with scripts/checkpatch.pl
> Good to tidy that up.
> 

Thanks.

>>     * Remove unnecessary space before @event in sdei_do_cross_call()
> 
> Good change, but seems to have gotten lost in this patch.
> 

Thanks.

>>
>> Signed-off-by: Gavin Shan <gshan@redhat.com>
>> ---
>> v2: Drop changes to sdei_cross_call_return()
>>      Remove unnecessary space before @event in sdei_do_cross_call()
>> ---
>>   drivers/firmware/arm_sdei.c | 15 +++++++++------
>>   1 file changed, 9 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/firmware/arm_sdei.c b/drivers/firmware/arm_sdei.c
>> index d03371161aaf..04dc144a8f31 100644
>> --- a/drivers/firmware/arm_sdei.c
>> +++ b/drivers/firmware/arm_sdei.c
>> @@ -73,14 +73,17 @@ static LIST_HEAD(sdei_list);
>>   
>>   /* Private events are registered/enabled via IPI passing one of these */
>>   struct sdei_crosscall_args {
>> -	struct sdei_event *event;
>> -	atomic_t errors;
>> -	int first_error;
>> +	struct sdei_event	*event;
>> +	atomic_t		errors;
>> +	int			first_error;
>>   };
>>   
>> -#define CROSSCALL_INIT(arg, event)	(arg.event = event, \
>> -					 arg.first_error = 0, \
>> -					 atomic_set(&arg.errors, 0))
>> +#define CROSSCALL_INIT(arg, event)		\
>> +	do {					\
>> +		arg.event = event;		\
>> +		arg.first_error = 0;		\
>> +		atomic_set(&arg.errors, 0);	\
>> +	} while (0)
>>   
>>   static inline int sdei_do_cross_call(void *fn, struct sdei_event * event)
>>   {
> 

Thanks,
Gavin
Jonathan Cameron July 27, 2020, 8:58 a.m. UTC | #3
On Mon, 27 Jul 2020 10:33:46 +1000
Gavin Shan <gshan@redhat.com> wrote:

> Hi Jonathan,
> 
> On 7/24/20 1:52 AM, Jonathan Cameron wrote:
> > On Wed, 22 Jul 2020 19:57:33 +1000
> > Gavin Shan <gshan@redhat.com> wrote:
> >   
> >> This applies cleanup on the corss call functions, no functional  
> > 
> > cross
> >   
> 
> It will be fixed in v3.
> 
> >> changes is introduced:  
> > 
> > changes are introduced:
> >   
> 
> It will be fixed in v3.
> 
> >>
> >>     * Cleanup struct sdei_crosscall_arg to use tab between fields
> >>       and their types  
> > Hmm. I guess there is a consistency argument for doing this but generally
> > that one doesn't look like it actually has much benefit.
> >   
> 
> Sorry, I guess you're talking about "./scripts/cleanpatch"? As stated
> in the head of the script, the result would be destructive. So I think
> manual changes would be more reliable. Lets keep this in v3 if you agree.
> 
> Extracted from "scripts/cleanpatch":

No I was talking about doing this change at all.  Generally I'd say don't
bother making changes like this to existing code.  They just add noise.

> 
> # Clean a patch file -- or directory of patch files -- of stealth whitespace.
> # WARNING: this can be a highly destructive operation.  Use with caution.
> #
> 
> 
> >>     * Refactor CROSSCALL_INIT to use "do { ... } while (0)" to be
> >>       compatible with scripts/checkpatch.pl  
> > Good to tidy that up.
> >   
> 
> Thanks.
> 
> >>     * Remove unnecessary space before @event in sdei_do_cross_call()  
> > 
> > Good change, but seems to have gotten lost in this patch.
> >   
> 
> Thanks.
> 
> >>
> >> Signed-off-by: Gavin Shan <gshan@redhat.com>
> >> ---
> >> v2: Drop changes to sdei_cross_call_return()
> >>      Remove unnecessary space before @event in sdei_do_cross_call()
> >> ---
> >>   drivers/firmware/arm_sdei.c | 15 +++++++++------
> >>   1 file changed, 9 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/drivers/firmware/arm_sdei.c b/drivers/firmware/arm_sdei.c
> >> index d03371161aaf..04dc144a8f31 100644
> >> --- a/drivers/firmware/arm_sdei.c
> >> +++ b/drivers/firmware/arm_sdei.c
> >> @@ -73,14 +73,17 @@ static LIST_HEAD(sdei_list);
> >>   
> >>   /* Private events are registered/enabled via IPI passing one of these */
> >>   struct sdei_crosscall_args {
> >> -	struct sdei_event *event;
> >> -	atomic_t errors;
> >> -	int first_error;
> >> +	struct sdei_event	*event;
> >> +	atomic_t		errors;
> >> +	int			first_error;
> >>   };
> >>   
> >> -#define CROSSCALL_INIT(arg, event)	(arg.event = event, \
> >> -					 arg.first_error = 0, \
> >> -					 atomic_set(&arg.errors, 0))
> >> +#define CROSSCALL_INIT(arg, event)		\
> >> +	do {					\
> >> +		arg.event = event;		\
> >> +		arg.first_error = 0;		\
> >> +		atomic_set(&arg.errors, 0);	\
> >> +	} while (0)
> >>   
> >>   static inline int sdei_do_cross_call(void *fn, struct sdei_event * event)
> >>   {  
> >   
> 
> Thanks,
> Gavin
>
Gavin Shan July 27, 2020, 9:45 a.m. UTC | #4
Hi Jonathan,

On 7/27/20 6:58 PM, Jonathan Cameron wrote:
> On Mon, 27 Jul 2020 10:33:46 +1000
> Gavin Shan <gshan@redhat.com> wrote:
>> On 7/24/20 1:52 AM, Jonathan Cameron wrote:
>>> On Wed, 22 Jul 2020 19:57:33 +1000
>>> Gavin Shan <gshan@redhat.com> wrote:
>>>    
>>>> This applies cleanup on the corss call functions, no functional
>>>
>>> cross
>>>    
>>
>> It will be fixed in v3.
>>
>>>> changes is introduced:
>>>
>>> changes are introduced:
>>>    
>>
>> It will be fixed in v3.
>>
>>>>
>>>>      * Cleanup struct sdei_crosscall_arg to use tab between fields
>>>>        and their types
>>> Hmm. I guess there is a consistency argument for doing this but generally
>>> that one doesn't look like it actually has much benefit.
>>>    
>>
>> Sorry, I guess you're talking about "./scripts/cleanpatch"? As stated
>> in the head of the script, the result would be destructive. So I think
>> manual changes would be more reliable. Lets keep this in v3 if you agree.
>>
>> Extracted from "scripts/cleanpatch":
> 
> No I was talking about doing this change at all.  Generally I'd say don't
> bother making changes like this to existing code.  They just add noise.
> 

I will drop the particular changes in v3 then :)

>>
>> # Clean a patch file -- or directory of patch files -- of stealth whitespace.
>> # WARNING: this can be a highly destructive operation.  Use with caution.
>> #
>>
>>
>>>>      * Refactor CROSSCALL_INIT to use "do { ... } while (0)" to be
>>>>        compatible with scripts/checkpatch.pl
>>> Good to tidy that up.
>>>    
>>
>> Thanks.
>>
>>>>      * Remove unnecessary space before @event in sdei_do_cross_call()
>>>
>>> Good change, but seems to have gotten lost in this patch.
>>>    
>>
>> Thanks.
>>
>>>>
>>>> Signed-off-by: Gavin Shan <gshan@redhat.com>
>>>> ---
>>>> v2: Drop changes to sdei_cross_call_return()
>>>>       Remove unnecessary space before @event in sdei_do_cross_call()
>>>> ---
>>>>    drivers/firmware/arm_sdei.c | 15 +++++++++------
>>>>    1 file changed, 9 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/drivers/firmware/arm_sdei.c b/drivers/firmware/arm_sdei.c
>>>> index d03371161aaf..04dc144a8f31 100644
>>>> --- a/drivers/firmware/arm_sdei.c
>>>> +++ b/drivers/firmware/arm_sdei.c
>>>> @@ -73,14 +73,17 @@ static LIST_HEAD(sdei_list);
>>>>    
>>>>    /* Private events are registered/enabled via IPI passing one of these */
>>>>    struct sdei_crosscall_args {
>>>> -	struct sdei_event *event;
>>>> -	atomic_t errors;
>>>> -	int first_error;
>>>> +	struct sdei_event	*event;
>>>> +	atomic_t		errors;
>>>> +	int			first_error;
>>>>    };
>>>>    
>>>> -#define CROSSCALL_INIT(arg, event)	(arg.event = event, \
>>>> -					 arg.first_error = 0, \
>>>> -					 atomic_set(&arg.errors, 0))
>>>> +#define CROSSCALL_INIT(arg, event)		\
>>>> +	do {					\
>>>> +		arg.event = event;		\
>>>> +		arg.first_error = 0;		\
>>>> +		atomic_set(&arg.errors, 0);	\
>>>> +	} while (0)
>>>>    
>>>>    static inline int sdei_do_cross_call(void *fn, struct sdei_event * event)
>>>>    {
>>>    
>>

Thanks,
Gavin

Patch
diff mbox series

diff --git a/drivers/firmware/arm_sdei.c b/drivers/firmware/arm_sdei.c
index d03371161aaf..04dc144a8f31 100644
--- a/drivers/firmware/arm_sdei.c
+++ b/drivers/firmware/arm_sdei.c
@@ -73,14 +73,17 @@  static LIST_HEAD(sdei_list);
 
 /* Private events are registered/enabled via IPI passing one of these */
 struct sdei_crosscall_args {
-	struct sdei_event *event;
-	atomic_t errors;
-	int first_error;
+	struct sdei_event	*event;
+	atomic_t		errors;
+	int			first_error;
 };
 
-#define CROSSCALL_INIT(arg, event)	(arg.event = event, \
-					 arg.first_error = 0, \
-					 atomic_set(&arg.errors, 0))
+#define CROSSCALL_INIT(arg, event)		\
+	do {					\
+		arg.event = event;		\
+		arg.first_error = 0;		\
+		atomic_set(&arg.errors, 0);	\
+	} while (0)
 
 static inline int sdei_do_cross_call(void *fn, struct sdei_event * event)
 {