[PATCH 0/2] staging: greybus: Fix parameters alignment and strings concatenation

[PATCH 0/2] staging: greybus: Fix parameters alignment and strings concatenation

[Cristian Sicilia]

[-- Attachment #1: Type: text/plain, Size: 384 bytes --]

First patch align some parameters with parenthesis.
Second patch will add some spaces between string.

Cristian Sicilia (2):
  staging: greybus: Align function call parameters to parenthesis
  staging: greybus: Added space between string concatenated

 drivers/staging/greybus/loopback.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

-- 
2.7.4


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH 1/2] staging: greybus: Align function call parameters to parenthesis

[Cristian Sicilia]

[-- Attachment #1: Type: text/plain, Size: 1720 bytes --]

Aligned some parameters to the previous parenthesis.

Signed-off-by: Cristian Sicilia <sicilia.cristian@gmail.com>
---
 drivers/staging/greybus/loopback.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/greybus/loopback.c b/drivers/staging/greybus/loopback.c
index e4d42c1..1085e06 100644
--- a/drivers/staging/greybus/loopback.c
+++ b/drivers/staging/greybus/loopback.c
@@ -638,7 +638,8 @@ static int gb_loopback_async_transfer(struct gb_loopback *gb, u32 len)
 	retval = gb_loopback_async_operation(gb, GB_LOOPBACK_TYPE_TRANSFER,
 					     request, len + sizeof(*request),
 					     len + response_len,
-					     gb_loopback_async_transfer_complete);
+					     gb_loopback_async_transfer_complete
+					     );
 	if (retval)
 		goto gb_error;
 
@@ -682,7 +683,8 @@ static int gb_loopback_request_handler(struct gb_operation *operation)
 		}
 
 		if (!gb_operation_response_alloc(operation,
-				len + sizeof(*response), GFP_KERNEL)) {
+						 len + sizeof(*response),
+						 GFP_KERNEL)) {
 			dev_err(dev, "error allocating response\n");
 			return -ENOMEM;
 		}
@@ -882,7 +884,7 @@ static int gb_loopback_fn(void *data)
 				gb->type = 0;
 				gb->send_count = 0;
 				sysfs_notify(&gb->dev->kobj,  NULL,
-						"iteration_count");
+					     "iteration_count");
 				dev_dbg(&bundle->dev, "load test complete\n");
 			} else {
 				dev_dbg(&bundle->dev,
@@ -1066,7 +1068,7 @@ static int gb_loopback_probe(struct gb_bundle *bundle,
 
 	/* Allocate kfifo */
 	if (kfifo_alloc(&gb->kfifo_lat, kfifo_depth * sizeof(u32),
-			  GFP_KERNEL)) {
+			GFP_KERNEL)) {
 		retval = -ENOMEM;
 		goto out_conn;
 	}
-- 
2.7.4


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH 2/2] staging: greybus: Added space between string concatenated

[Cristian Sicilia]

[-- Attachment #1: Type: text/plain, Size: 1592 bytes --]

Some concatenated strings are now spaced.

Signed-off-by: Cristian Sicilia <sicilia.cristian@gmail.com>
---
 drivers/staging/greybus/loopback.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/greybus/loopback.c b/drivers/staging/greybus/loopback.c
index 1085e06..acfa392 100644
--- a/drivers/staging/greybus/loopback.c
+++ b/drivers/staging/greybus/loopback.c
@@ -141,7 +141,7 @@ static ssize_t name##_##field##_show(struct device *dev,	\
 	/* Report 0 for min and max if no transfer successed */		\
 	if (!gb->requests_completed)					\
 		return sprintf(buf, "0\n");				\
-	return sprintf(buf, "%"#type"\n", gb->name.field);	\
+	return sprintf(buf, "%" #type "\n", gb->name.field);		\
 }									\
 static DEVICE_ATTR_RO(name##_##field)
 
@@ -176,7 +176,7 @@ static ssize_t field##_show(struct device *dev,				\
 			    char *buf)					\
 {									\
 	struct gb_loopback *gb = dev_get_drvdata(dev);			\
-	return sprintf(buf, "%"#type"\n", gb->field);			\
+	return sprintf(buf, "%" #type "\n", gb->field);			\
 }									\
 static ssize_t field##_store(struct device *dev,			\
 			    struct device_attribute *attr,		\
@@ -212,7 +212,7 @@ static ssize_t field##_show(struct device *dev,				\
 			    char *buf)					\
 {									\
 	struct gb_loopback *gb = dev_get_drvdata(dev);			\
-	return sprintf(buf, "%"#type"\n", gb->field);			\
+	return sprintf(buf, "%" #type "\n", gb->field);			\
 }									\
 static ssize_t field##_store(struct device *dev,			\
 			    struct device_attribute *attr,		\
-- 
2.7.4


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 1/2] staging: greybus: Align function call parameters to parenthesis

[Greg Kroah-Hartman]

On Tue, Dec 04, 2018 at 09:58:14PM +0100, Cristian Sicilia wrote:
> Aligned some parameters to the previous parenthesis.
> 
> Signed-off-by: Cristian Sicilia <sicilia.cristian@gmail.com>
> ---
>  drivers/staging/greybus/loopback.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/greybus/loopback.c b/drivers/staging/greybus/loopback.c
> index e4d42c1..1085e06 100644
> --- a/drivers/staging/greybus/loopback.c
> +++ b/drivers/staging/greybus/loopback.c
> @@ -638,7 +638,8 @@ static int gb_loopback_async_transfer(struct gb_loopback *gb, u32 len)
>  	retval = gb_loopback_async_operation(gb, GB_LOOPBACK_TYPE_TRANSFER,
>  					     request, len + sizeof(*request),
>  					     len + response_len,
> -					     gb_loopback_async_transfer_complete);
> +					     gb_loopback_async_transfer_complete
> +					     );

Ick, why do this?  That's not really needed.

thanks,

greg k-h

Re: [PATCH 2/2] staging: greybus: Added space between string concatenated

[Bryan O'Donoghue]

On 04/12/2018 20:58, Cristian Sicilia wrote:
> Some concatenated strings are now spaced.
> 
> Signed-off-by: Cristian Sicilia <sicilia.cristian@gmail.com>
> ---
>   drivers/staging/greybus/loopback.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/greybus/loopback.c b/drivers/staging/greybus/loopback.c
> index 1085e06..acfa392 100644
> --- a/drivers/staging/greybus/loopback.c
> +++ b/drivers/staging/greybus/loopback.c
> @@ -141,7 +141,7 @@ static ssize_t name##_##field##_show(struct device *dev,	\
>   	/* Report 0 for min and max if no transfer successed */		\
>   	if (!gb->requests_completed)					\
>   		return sprintf(buf, "0\n");				\
> -	return sprintf(buf, "%"#type"\n", gb->name.field);	\
> +	return sprintf(buf, "%" #type "\n", gb->name.field);		\
>   }									\
>   static DEVICE_ATTR_RO(name##_##field)
>   
> @@ -176,7 +176,7 @@ static ssize_t field##_show(struct device *dev,				\
>   			    char *buf)					\
>   {									\
>   	struct gb_loopback *gb = dev_get_drvdata(dev);			\
> -	return sprintf(buf, "%"#type"\n", gb->field);			\
> +	return sprintf(buf, "%" #type "\n", gb->field);			\
>   }									\
>   static ssize_t field##_store(struct device *dev,			\
>   			    struct device_attribute *attr,		\
> @@ -212,7 +212,7 @@ static ssize_t field##_show(struct device *dev,				\
>   			    char *buf)					\
>   {									\
>   	struct gb_loopback *gb = dev_get_drvdata(dev);			\
> -	return sprintf(buf, "%"#type"\n", gb->field);			\
> +	return sprintf(buf, "%" #type "\n", gb->field);			\
>   }									\
>   static ssize_t field##_store(struct device *dev,			\
>   			    struct device_attribute *attr,		\
> 
Eh.

But doesn't all of this add an extra two space to the resultant string ?

Not what we want.

---
bod

Re: [PATCH 2/2] staging: greybus: Added space between string concatenated

[Sicilia Cristian]

[-- Attachment #1: Type: text/plain, Size: 2554 bytes --]

On Wed, Dec 05, 2018 at 10:12:24AM +0000, Bryan O'Donoghue wrote:
> On 04/12/2018 20:58, Cristian Sicilia wrote:
> >Some concatenated strings are now spaced.
> >
> >Signed-off-by: Cristian Sicilia <sicilia.cristian@gmail.com>
> >---
> >  drivers/staging/greybus/loopback.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> >diff --git a/drivers/staging/greybus/loopback.c b/drivers/staging/greybus/loopback.c
> >index 1085e06..acfa392 100644
> >--- a/drivers/staging/greybus/loopback.c
> >+++ b/drivers/staging/greybus/loopback.c
> >@@ -141,7 +141,7 @@ static ssize_t name##_##field##_show(struct device *dev,	\
> >  	/* Report 0 for min and max if no transfer successed */		\
> >  	if (!gb->requests_completed)					\
> >  		return sprintf(buf, "0\n");				\
> >-	return sprintf(buf, "%"#type"\n", gb->name.field);	\
> >+	return sprintf(buf, "%" #type "\n", gb->name.field);		\
> >  }									\
> >  static DEVICE_ATTR_RO(name##_##field)
> >@@ -176,7 +176,7 @@ static ssize_t field##_show(struct device *dev,				\
> >  			    char *buf)					\
> >  {									\
> >  	struct gb_loopback *gb = dev_get_drvdata(dev);			\
> >-	return sprintf(buf, "%"#type"\n", gb->field);			\
> >+	return sprintf(buf, "%" #type "\n", gb->field);			\
> >  }									\
> >  static ssize_t field##_store(struct device *dev,			\
> >  			    struct device_attribute *attr,		\
> >@@ -212,7 +212,7 @@ static ssize_t field##_show(struct device *dev,				\
> >  			    char *buf)					\
> >  {									\
> >  	struct gb_loopback *gb = dev_get_drvdata(dev);			\
> >-	return sprintf(buf, "%"#type"\n", gb->field);			\
> >+	return sprintf(buf, "%" #type "\n", gb->field);			\
> >  }									\
> >  static ssize_t field##_store(struct device *dev,			\
> >  			    struct device_attribute *attr,		\
> >
> Eh.
> 
> But doesn't all of this add an extra two space to the resultant string ?
> 
> Not what we want.

It doesn't change the result string, if I well understand the question your
doubt is if there are some space between % and type or between type and
end of line:

This (supposing type=u and field=min):
 return sprintf(buf, "%"#type"\n", gb->field);
Will expanded in this
 return sprintf(buf, "%""u""\n", gb->min);
That's is like:
 return sprintf(buf, "%u\n", gb->min);

This (supposing type=u and field=min):
return sprintf(buf, "%" #type "\n", gb->field);
Will expanded in this
 return sprintf(buf, "%" "u" "\n", gb->min);
That's is like:
 return sprintf(buf, "%u\n", gb->min);

> 
> ---
> bod

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 1/2] staging: greybus: Align function call parameters to parenthesis

[Sicilia Cristian]

[-- Attachment #1: Type: text/plain, Size: 1113 bytes --]

On Wed, Dec 05, 2018 at 09:46:03AM +0100, Greg Kroah-Hartman wrote:
> On Tue, Dec 04, 2018 at 09:58:14PM +0100, Cristian Sicilia wrote:
> > Aligned some parameters to the previous parenthesis.
> > 
> > Signed-off-by: Cristian Sicilia <sicilia.cristian@gmail.com>
> > ---
> >  drivers/staging/greybus/loopback.c | 10 ++++++----
> >  1 file changed, 6 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/staging/greybus/loopback.c b/drivers/staging/greybus/loopback.c
> > index e4d42c1..1085e06 100644
> > --- a/drivers/staging/greybus/loopback.c
> > +++ b/drivers/staging/greybus/loopback.c
> > @@ -638,7 +638,8 @@ static int gb_loopback_async_transfer(struct gb_loopback *gb, u32 len)
> >  	retval = gb_loopback_async_operation(gb, GB_LOOPBACK_TYPE_TRANSFER,
> >  					     request, len + sizeof(*request),
> >  					     len + response_len,
> > -					     gb_loopback_async_transfer_complete);
> > +					     gb_loopback_async_transfer_complete
> > +					     );
> 
> Ick, why do this?  That's not really needed.
> 

Ok, better to leave it unchanged

> thanks,
> 
> greg k-h

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 2/2] staging: greybus: Added space between string concatenated

[Bryan O'Donoghue]

On 05/12/2018 21:00, Sicilia Cristian wrote:
> It doesn't change the result string

So why do it then ?

Re: [PATCH 2/2] staging: greybus: Added space between string concatenated

[Greg Kroah-Hartman]

On Thu, Dec 06, 2018 at 09:43:46AM +0000, Bryan O'Donoghue wrote:
> On 05/12/2018 21:00, Sicilia Cristian wrote:
> > It doesn't change the result string
> 
> So why do it then ?

Because it is easier to read this way and odds are checkpatch is
happier.

thanks,

greg k-h

Re: [PATCH 2/2] staging: greybus: Added space between string concatenated

[Bryan O'Donoghue]

On 06/12/2018 15:05, Greg Kroah-Hartman wrote:
> On Thu, Dec 06, 2018 at 09:43:46AM +0000, Bryan O'Donoghue wrote:
>> On 05/12/2018 21:00, Sicilia Cristian wrote:
>>> It doesn't change the result string
>>
>> So why do it then ?
> 
> Because it is easier to read this way and odds are checkpatch is
> happier.

Fine.

Sicilia could you please amend your commit.

'This patch fixes the checkpatch error "CHECK: Concatenated strings 
should use spaces between elements"'

and then feel free to add my

Acked-by: Bryan O'Donoghue <pure.logic@nexus-software.ie>

---
bod

Re: [PATCH 2/2] staging: greybus: Added space between string concatenated

[Greg Kroah-Hartman]

On Thu, Dec 06, 2018 at 04:14:53PM +0000, Bryan O'Donoghue wrote:
> On 06/12/2018 15:05, Greg Kroah-Hartman wrote:
> > On Thu, Dec 06, 2018 at 09:43:46AM +0000, Bryan O'Donoghue wrote:
> > > On 05/12/2018 21:00, Sicilia Cristian wrote:
> > > > It doesn't change the result string
> > > 
> > > So why do it then ?
> > 
> > Because it is easier to read this way and odds are checkpatch is
> > happier.
> 
> Fine.
> 
> Sicilia could you please amend your commit.
> 
> 'This patch fixes the checkpatch error "CHECK: Concatenated strings should
> use spaces between elements"'
> 
> and then feel free to add my
> 
> Acked-by: Bryan O'Donoghue <pure.logic@nexus-software.ie>

It's already in my tree, no need to ammend it, sorry :)

greg k-h

Re: [PATCH 2/2] staging: greybus: Added space between string concatenated

[Bryan O'Donoghue]

On 06/12/2018 16:23, Greg Kroah-Hartman wrote:
> On Thu, Dec 06, 2018 at 04:14:53PM +0000, Bryan O'Donoghue wrote:
>> On 06/12/2018 15:05, Greg Kroah-Hartman wrote:
>>> On Thu, Dec 06, 2018 at 09:43:46AM +0000, Bryan O'Donoghue wrote:
>>>> On 05/12/2018 21:00, Sicilia Cristian wrote:
>>>>> It doesn't change the result string
>>>>
>>>> So why do it then ?
>>>
>>> Because it is easier to read this way and odds are checkpatch is
>>> happier.
>>
>> Fine.
>>
>> Sicilia could you please amend your commit.
>>
>> 'This patch fixes the checkpatch error "CHECK: Concatenated strings should
>> use spaces between elements"'
>>
>> and then feel free to add my
>>
>> Acked-by: Bryan O'Donoghue <pure.logic@nexus-software.ie>
> 
> It's already in my tree, no need to ammend it, sorry :)
> 
> greg k-h
> 

thanks greg !