| Message ID | 20200722095740.28560-11-gshan@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Refactor SDEI client driver | expand |
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) > {
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
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 >
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
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) {
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(-)