[PATCH] staging: greybus: add blank line after struct

Menna Mahmoud posted 1 patch 2 years, 10 months ago
There is a newer version of this series
drivers/staging/greybus/gbphy.h | 2 ++
1 file changed, 2 insertions(+)
[PATCH] staging: greybus: add blank line after struct
Posted by Menna Mahmoud 2 years, 10 months ago
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 <eng.mennamahmoud.mm@gmail.com>
---
 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
Re: [PATCH] staging: greybus: add blank line after struct
Posted by Julia Lawall 2 years, 10 months ago

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 <eng.mennamahmoud.mm@gmail.com>
> ---
>  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
>
>
>
Re: [PATCH] staging: greybus: add blank line after struct
Posted by Menna Mahmoud 2 years, 10 months ago
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 <eng.mennamahmoud.mm@gmail.com>
>> ---
>>   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
>>
>>
>>
Re: [PATCH] staging: greybus: add blank line after struct
Posted by Julia Lawall 2 years, 10 months ago

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 <eng.mennamahmoud.mm@gmail.com>
> > > ---
> > >   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
> > >
> > >
> > >
>
Re: [PATCH] staging: greybus: add blank line after struct
Posted by Menna Mahmoud 2 years, 10 months ago
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 <eng.mennamahmoud.mm@gmail.com>
>>>> ---
>>>>    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
>>>>
>>>>
>>>>
> >
Re: [PATCH] staging: greybus: add blank line after struct
Posted by Julia Lawall 2 years, 10 months ago

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 <eng.mennamahmoud.mm@gmail.com>
> > > > > ---
> > > > >    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
> > > > >
> > > > >
> > > > >
> > >
>
Re: [PATCH] staging: greybus: add blank line after struct
Posted by Menna Mahmoud 2 years, 10 months ago
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 <eng.mennamahmoud.mm@gmail.com>
>>>>>> ---
>>>>>>     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
>>>>>>
>>>>>>
>>>>>>
> >