2023-03-19 11:08:44

by Menna Mahmoud

[permalink] [raw]
Subject: [PATCH] staging: greybus: add blank line after struct

add blank line after struct for readability as
reported by checkpatch script

" CHECK: Please use a blank line after function/struct/union/enum
declarations"

Signed-off-by: Menna Mahmoud <[email protected]>
---
drivers/staging/greybus/gbphy.h | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/staging/greybus/gbphy.h b/drivers/staging/greybus/gbphy.h
index d4a225b76338..1de510499480 100644
--- a/drivers/staging/greybus/gbphy.h
+++ b/drivers/staging/greybus/gbphy.h
@@ -15,6 +15,7 @@ struct gbphy_device {
struct list_head list;
struct device dev;
};
+
#define to_gbphy_dev(d) container_of(d, struct gbphy_device, dev)

static inline void *gb_gbphy_get_data(struct gbphy_device *gdev)
@@ -43,6 +44,7 @@ struct gbphy_driver {

struct device_driver driver;
};
+
#define to_gbphy_driver(d) container_of(d, struct gbphy_driver, driver)

int gb_gbphy_register_driver(struct gbphy_driver *driver,
--
2.34.1



2023-03-19 11:19:14

by Julia Lawall

[permalink] [raw]
Subject: Re: [PATCH] staging: greybus: add blank line after struct



On Sun, 19 Mar 2023, Menna Mahmoud wrote:

> add blank line after struct for readability as

The log message should start with a capital letter, so "Add".

> reported by checkpatch script

"reported by checkpatch" or "reported by the checkpatch script".
The first is more concise, and it doesn't really matter whether checkpatch
is a script or something else.

> " CHECK: Please use a blank line after function/struct/union/enum
> declarations"

I guess the #define was concatenated to the end of the definition to show
that it is closely related to the definition. With the #define, it seems
rather natural, but the better soltution would be to make a static inline
function in both cases. There would naturally be a blank line before a
function definition as well.

julia

>
> Signed-off-by: Menna Mahmoud <[email protected]>
> ---
> drivers/staging/greybus/gbphy.h | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/staging/greybus/gbphy.h b/drivers/staging/greybus/gbphy.h
> index d4a225b76338..1de510499480 100644
> --- a/drivers/staging/greybus/gbphy.h
> +++ b/drivers/staging/greybus/gbphy.h
> @@ -15,6 +15,7 @@ struct gbphy_device {
> struct list_head list;
> struct device dev;
> };
> +
> #define to_gbphy_dev(d) container_of(d, struct gbphy_device, dev)
>
> static inline void *gb_gbphy_get_data(struct gbphy_device *gdev)
> @@ -43,6 +44,7 @@ struct gbphy_driver {
>
> struct device_driver driver;
> };
> +
> #define to_gbphy_driver(d) container_of(d, struct gbphy_driver, driver)
>
> int gb_gbphy_register_driver(struct gbphy_driver *driver,
> --
> 2.34.1
>
>
>

2023-03-19 11:28:15

by Menna Mahmoud

[permalink] [raw]
Subject: Re: [PATCH] staging: greybus: add blank line after struct


On ١٩‏/٣‏/٢٠٢٣ ١٣:١٩, Julia Lawall wrote:
>
> On Sun, 19 Mar 2023, Menna Mahmoud wrote:
>
>> add blank line after struct for readability as
> The log message should start with a capital letter, so "Add".


Okay, I will fix it.

>
>> reported by checkpatch script
> "reported by checkpatch" or "reported by the checkpatch script".
> The first is more concise, and it doesn't really matter whether checkpatch
> is a script or something else.


got it.

>> " CHECK: Please use a blank line after function/struct/union/enum
>> declarations"
> I guess the #define was concatenated to the end of the definition to show
> that it is closely related to the definition. With the #define, it seems
> rather natural, but the better soltution would be to make a static inline
> function in both cases. There would naturally be a blank line before a
> function definition as well.


got your point, so, should i ignore this?


Menna

>
> julia
>
>> Signed-off-by: Menna Mahmoud <[email protected]>
>> ---
>> drivers/staging/greybus/gbphy.h | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/staging/greybus/gbphy.h b/drivers/staging/greybus/gbphy.h
>> index d4a225b76338..1de510499480 100644
>> --- a/drivers/staging/greybus/gbphy.h
>> +++ b/drivers/staging/greybus/gbphy.h
>> @@ -15,6 +15,7 @@ struct gbphy_device {
>> struct list_head list;
>> struct device dev;
>> };
>> +
>> #define to_gbphy_dev(d) container_of(d, struct gbphy_device, dev)
>>
>> static inline void *gb_gbphy_get_data(struct gbphy_device *gdev)
>> @@ -43,6 +44,7 @@ struct gbphy_driver {
>>
>> struct device_driver driver;
>> };
>> +
>> #define to_gbphy_driver(d) container_of(d, struct gbphy_driver, driver)
>>
>> int gb_gbphy_register_driver(struct gbphy_driver *driver,
>> --
>> 2.34.1
>>
>>
>>

2023-03-19 11:36:55

by Julia Lawall

[permalink] [raw]
Subject: Re: [PATCH] staging: greybus: add blank line after struct



On Sun, 19 Mar 2023, Menna Mahmoud wrote:

>
> On ١٩/٣/٢٠٢٣ ١٣:١٩, Julia Lawall wrote:
> >
> > On Sun, 19 Mar 2023, Menna Mahmoud wrote:
> >
> > > add blank line after struct for readability as
> > The log message should start with a capital letter, so "Add".
>
>
> Okay, I will fix it.
>
> >
> > > reported by checkpatch script
> > "reported by checkpatch" or "reported by the checkpatch script".
> > The first is more concise, and it doesn't really matter whether checkpatch
> > is a script or something else.
>
>
> got it.
>
> > > " CHECK: Please use a blank line after function/struct/union/enum
> > > declarations"
> > I guess the #define was concatenated to the end of the definition to show
> > that it is closely related to the definition. With the #define, it seems
> > rather natural, but the better soltution would be to make a static inline
> > function in both cases. There would naturally be a blank line before a
> > function definition as well.
>
>
> got your point, so, should i ignore this?

Not sure what you mean by ignore. If you rewrite the #define as a
function, an use the natural placement for a function definition, then the
checkpatch warning will go away as a side effect.

julia

>
>
> Menna
>
> >
> > julia
> >
> > > Signed-off-by: Menna Mahmoud <[email protected]>
> > > ---
> > > drivers/staging/greybus/gbphy.h | 2 ++
> > > 1 file changed, 2 insertions(+)
> > >
> > > diff --git a/drivers/staging/greybus/gbphy.h
> > > b/drivers/staging/greybus/gbphy.h
> > > index d4a225b76338..1de510499480 100644
> > > --- a/drivers/staging/greybus/gbphy.h
> > > +++ b/drivers/staging/greybus/gbphy.h
> > > @@ -15,6 +15,7 @@ struct gbphy_device {
> > > struct list_head list;
> > > struct device dev;
> > > };
> > > +
> > > #define to_gbphy_dev(d) container_of(d, struct gbphy_device, dev)
> > >
> > > static inline void *gb_gbphy_get_data(struct gbphy_device *gdev)
> > > @@ -43,6 +44,7 @@ struct gbphy_driver {
> > >
> > > struct device_driver driver;
> > > };
> > > +
> > > #define to_gbphy_driver(d) container_of(d, struct gbphy_driver, driver)
> > >
> > > int gb_gbphy_register_driver(struct gbphy_driver *driver,
> > > --
> > > 2.34.1
> > >
> > >
> > >
>

2023-03-19 11:41:32

by Menna Mahmoud

[permalink] [raw]
Subject: Re: [PATCH] staging: greybus: add blank line after struct


On ١٩‏/٣‏/٢٠٢٣ ١٣:٣٦, Julia Lawall wrote:
>
> On Sun, 19 Mar 2023, Menna Mahmoud wrote:
>
>> On ١٩/٣/٢٠٢٣ ١٣:١٩, Julia Lawall wrote:
>>> On Sun, 19 Mar 2023, Menna Mahmoud wrote:
>>>
>>>> add blank line after struct for readability as
>>> The log message should start with a capital letter, so "Add".
>>
>> Okay, I will fix it.
>>
>>>> reported by checkpatch script
>>> "reported by checkpatch" or "reported by the checkpatch script".
>>> The first is more concise, and it doesn't really matter whether checkpatch
>>> is a script or something else.
>>
>> got it.
>>
>>>> " CHECK: Please use a blank line after function/struct/union/enum
>>>> declarations"
>>> I guess the #define was concatenated to the end of the definition to show
>>> that it is closely related to the definition. With the #define, it seems
>>> rather natural, but the better soltution would be to make a static inline
>>> function in both cases. There would naturally be a blank line before a
>>> function definition as well.
>>
>> got your point, so, should i ignore this?
> Not sure what you mean by ignore. If you rewrite the #define as a
> function, an use the natural placement for a function definition, then the
> checkpatch warning will go away as a side effect.


I mean ignore this patch and make another patch with rewrite #define as
you suggested.


Menna

>
> julia
>
>>
>> Menna
>>
>>> julia
>>>
>>>> Signed-off-by: Menna Mahmoud <[email protected]>
>>>> ---
>>>> drivers/staging/greybus/gbphy.h | 2 ++
>>>> 1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/drivers/staging/greybus/gbphy.h
>>>> b/drivers/staging/greybus/gbphy.h
>>>> index d4a225b76338..1de510499480 100644
>>>> --- a/drivers/staging/greybus/gbphy.h
>>>> +++ b/drivers/staging/greybus/gbphy.h
>>>> @@ -15,6 +15,7 @@ struct gbphy_device {
>>>> struct list_head list;
>>>> struct device dev;
>>>> };
>>>> +
>>>> #define to_gbphy_dev(d) container_of(d, struct gbphy_device, dev)
>>>>
>>>> static inline void *gb_gbphy_get_data(struct gbphy_device *gdev)
>>>> @@ -43,6 +44,7 @@ struct gbphy_driver {
>>>>
>>>> struct device_driver driver;
>>>> };
>>>> +
>>>> #define to_gbphy_driver(d) container_of(d, struct gbphy_driver, driver)
>>>>
>>>> int gb_gbphy_register_driver(struct gbphy_driver *driver,
>>>> --
>>>> 2.34.1
>>>>
>>>>
>>>>
> >

2023-03-19 11:46:48

by Julia Lawall

[permalink] [raw]
Subject: Re: [PATCH] staging: greybus: add blank line after struct



On Sun, 19 Mar 2023, Menna Mahmoud wrote:

>
> On ١٩/٣/٢٠٢٣ ١٣:٣٦, Julia Lawall wrote:
> >
> > On Sun, 19 Mar 2023, Menna Mahmoud wrote:
> >
> > > On ١٩/٣/٢٠٢٣ ١٣:١٩, Julia Lawall wrote:
> > > > On Sun, 19 Mar 2023, Menna Mahmoud wrote:
> > > >
> > > > > add blank line after struct for readability as
> > > > The log message should start with a capital letter, so "Add".
> > >
> > > Okay, I will fix it.
> > >
> > > > > reported by checkpatch script
> > > > "reported by checkpatch" or "reported by the checkpatch script".
> > > > The first is more concise, and it doesn't really matter whether
> > > > checkpatch
> > > > is a script or something else.
> > >
> > > got it.
> > >
> > > > > " CHECK: Please use a blank line after function/struct/union/enum
> > > > > declarations"
> > > > I guess the #define was concatenated to the end of the definition to
> > > > show
> > > > that it is closely related to the definition. With the #define, it
> > > > seems
> > > > rather natural, but the better soltution would be to make a static
> > > > inline
> > > > function in both cases. There would naturally be a blank line before a
> > > > function definition as well.
> > >
> > > got your point, so, should i ignore this?
> > Not sure what you mean by ignore. If you rewrite the #define as a
> > function, an use the natural placement for a function definition, then the
> > checkpatch warning will go away as a side effect.
>
>
> I mean ignore this patch and make another patch with rewrite #define as you
> suggested.

Yes :) That's fine ("drop" would be better than "ignore").

julia

>
>
> Menna
>
> >
> > julia
> >
> > >
> > > Menna
> > >
> > > > julia
> > > >
> > > > > Signed-off-by: Menna Mahmoud <[email protected]>
> > > > > ---
> > > > > drivers/staging/greybus/gbphy.h | 2 ++
> > > > > 1 file changed, 2 insertions(+)
> > > > >
> > > > > diff --git a/drivers/staging/greybus/gbphy.h
> > > > > b/drivers/staging/greybus/gbphy.h
> > > > > index d4a225b76338..1de510499480 100644
> > > > > --- a/drivers/staging/greybus/gbphy.h
> > > > > +++ b/drivers/staging/greybus/gbphy.h
> > > > > @@ -15,6 +15,7 @@ struct gbphy_device {
> > > > > struct list_head list;
> > > > > struct device dev;
> > > > > };
> > > > > +
> > > > > #define to_gbphy_dev(d) container_of(d, struct gbphy_device, dev)
> > > > >
> > > > > static inline void *gb_gbphy_get_data(struct gbphy_device *gdev)
> > > > > @@ -43,6 +44,7 @@ struct gbphy_driver {
> > > > >
> > > > > struct device_driver driver;
> > > > > };
> > > > > +
> > > > > #define to_gbphy_driver(d) container_of(d, struct gbphy_driver,
> > > > > driver)
> > > > >
> > > > > int gb_gbphy_register_driver(struct gbphy_driver *driver,
> > > > > --
> > > > > 2.34.1
> > > > >
> > > > >
> > > > >
> > >
>

2023-03-19 11:53:50

by Menna Mahmoud

[permalink] [raw]
Subject: Re: [PATCH] staging: greybus: add blank line after struct


On ١٩‏/٣‏/٢٠٢٣ ١٣:٤٦, Julia Lawall wrote:
>
> On Sun, 19 Mar 2023, Menna Mahmoud wrote:
>
>> On ١٩/٣/٢٠٢٣ ١٣:٣٦, Julia Lawall wrote:
>>> On Sun, 19 Mar 2023, Menna Mahmoud wrote:
>>>
>>>> On ١٩/٣/٢٠٢٣ ١٣:١٩, Julia Lawall wrote:
>>>>> On Sun, 19 Mar 2023, Menna Mahmoud wrote:
>>>>>
>>>>>> add blank line after struct for readability as
>>>>> The log message should start with a capital letter, so "Add".
>>>> Okay, I will fix it.
>>>>
>>>>>> reported by checkpatch script
>>>>> "reported by checkpatch" or "reported by the checkpatch script".
>>>>> The first is more concise, and it doesn't really matter whether
>>>>> checkpatch
>>>>> is a script or something else.
>>>> got it.
>>>>
>>>>>> " CHECK: Please use a blank line after function/struct/union/enum
>>>>>> declarations"
>>>>> I guess the #define was concatenated to the end of the definition to
>>>>> show
>>>>> that it is closely related to the definition. With the #define, it
>>>>> seems
>>>>> rather natural, but the better soltution would be to make a static
>>>>> inline
>>>>> function in both cases. There would naturally be a blank line before a
>>>>> function definition as well.
>>>> got your point, so, should i ignore this?
>>> Not sure what you mean by ignore. If you rewrite the #define as a
>>> function, an use the natural placement for a function definition, then the
>>> checkpatch warning will go away as a side effect.
>>
>> I mean ignore this patch and make another patch with rewrite #define as you
>> suggested.
> Yes :) That's fine ("drop" would be better than "ignore").
>
> julia
>
Okay :D, Thanks Julia.

Menna

>>
>> Menna
>>
>>> julia
>>>
>>>> Menna
>>>>
>>>>> julia
>>>>>
>>>>>> Signed-off-by: Menna Mahmoud <[email protected]>
>>>>>> ---
>>>>>> drivers/staging/greybus/gbphy.h | 2 ++
>>>>>> 1 file changed, 2 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/staging/greybus/gbphy.h
>>>>>> b/drivers/staging/greybus/gbphy.h
>>>>>> index d4a225b76338..1de510499480 100644
>>>>>> --- a/drivers/staging/greybus/gbphy.h
>>>>>> +++ b/drivers/staging/greybus/gbphy.h
>>>>>> @@ -15,6 +15,7 @@ struct gbphy_device {
>>>>>> struct list_head list;
>>>>>> struct device dev;
>>>>>> };
>>>>>> +
>>>>>> #define to_gbphy_dev(d) container_of(d, struct gbphy_device, dev)
>>>>>>
>>>>>> static inline void *gb_gbphy_get_data(struct gbphy_device *gdev)
>>>>>> @@ -43,6 +44,7 @@ struct gbphy_driver {
>>>>>>
>>>>>> struct device_driver driver;
>>>>>> };
>>>>>> +
>>>>>> #define to_gbphy_driver(d) container_of(d, struct gbphy_driver,
>>>>>> driver)
>>>>>>
>>>>>> int gb_gbphy_register_driver(struct gbphy_driver *driver,
>>>>>> --
>>>>>> 2.34.1
>>>>>>
>>>>>>
>>>>>>
> >