Re: [PATCH v2 10/17] drivers/firmware/sdei: Cleanup on cross call function

From: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
To: Gavin Shan <gshan@redhat.com>
Cc: mark.rutland@arm.com, catalin.marinas@arm.com,
	james.morse@arm.com, shan.gavin@gmail.com, will@kernel.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v2 10/17] drivers/firmware/sdei: Cleanup on cross call function
Date: Mon, 27 Jul 2020 09:58:21 +0100	[thread overview]
Message-ID: <20200727095821.00003e39@Huawei.com> (raw)
In-Reply-To: <b2175fd8-5656-b5e3-b457-70f3ea3d80a8@redhat.com>

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
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel