All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Input: synaptics-rmi4 - use %ph to form F34 configuration ID
@ 2017-05-30  4:49 Dmitry Torokhov
  2017-05-30  8:23 ` Benjamin Tissoires
  0 siblings, 1 reply; 8+ messages in thread
From: Dmitry Torokhov @ 2017-05-30  4:49 UTC (permalink / raw)
  To: linux-input; +Cc: Nick Dyer, Benjamin Tissoires, Dan Carpenter, linux-kernel

Instead of printing bytes one by one, let's use %phN to print the buffer in
one go.

Also use hweight8 to count number of partitions instead of inspecting it
bit by bit.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/rmi4/rmi_f34v7.c | 22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/drivers/input/rmi4/rmi_f34v7.c b/drivers/input/rmi4/rmi_f34v7.c
index ae2db1c3aebf..3991d2943660 100644
--- a/drivers/input/rmi4/rmi_f34v7.c
+++ b/drivers/input/rmi4/rmi_f34v7.c
@@ -9,13 +9,14 @@
  * the Free Software Foundation.
  */
 
+#include <linux/bitops.h>
 #include <linux/kernel.h>
 #include <linux/rmi.h>
 #include <linux/firmware.h>
-#include <asm/unaligned.h>
 #include <linux/delay.h>
 #include <linux/slab.h>
 #include <linux/jiffies.h>
+#include <asm/unaligned.h>
 
 #include "rmi_driver.h"
 #include "rmi_f34.h"
@@ -464,7 +465,7 @@ static int rmi_f34v7_read_queries_bl_version(struct f34_data *f34)
 static int rmi_f34v7_read_queries(struct f34_data *f34)
 {
 	int ret;
-	int i, j;
+	int i;
 	u8 base;
 	int offset;
 	u8 *ptable;
@@ -519,9 +520,6 @@ static int rmi_f34v7_read_queries(struct f34_data *f34)
 
 	if (query_0 & HAS_CONFIG_ID) {
 		u8 f34_ctrl[CONFIG_ID_SIZE];
-		int i = 0;
-		u8 *p = f34->configuration_id;
-		*p = '\0';
 
 		ret = rmi_read_block(f34->fn->rmi_dev,
 				f34->fn->fd.control_base_addr,
@@ -531,13 +529,11 @@ static int rmi_f34v7_read_queries(struct f34_data *f34)
 			return ret;
 
 		/* Eat leading zeros */
-		while (i < sizeof(f34_ctrl) && !f34_ctrl[i])
-			i++;
+		for (i = 0; i < sizeof(f34_ctrl) - 1 && !f34_ctrl[i]; i++)
+			/* Empty */;
 
-		for (; i < sizeof(f34_ctrl); i++)
-			p += snprintf(p, f34->configuration_id
-				      + sizeof(f34->configuration_id) - p,
-				      "%02X", f34_ctrl[i]);
+		snprintf(f34->configuration_id, sizeof(f34->configuration_id),
+			 "%*phN", (int)sizeof(f34_ctrl) - i, f34_ctrl + i);
 
 		rmi_dbg(RMI_DEBUG_FN, &f34->fn->dev, "Configuration ID: %s\n",
 			f34->configuration_id);
@@ -545,9 +541,7 @@ static int rmi_f34v7_read_queries(struct f34_data *f34)
 
 	f34->v7.partitions = 0;
 	for (i = 0; i < sizeof(query_1_7.partition_support); i++)
-		for (j = 0; j < 8; j++)
-			if (query_1_7.partition_support[i] & (1 << j))
-				f34->v7.partitions++;
+		f34->v7.partitions += hweight8(query_1_7.partition_support[i]);
 
 	rmi_dbg(RMI_DEBUG_FN, &f34->fn->dev, "%s: Supported partitions: %*ph\n",
 		__func__, sizeof(query_1_7.partition_support),
-- 
2.13.0.219.gdb65acc882-goog


-- 
Dmitry

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] Input: synaptics-rmi4 - use %ph to form F34 configuration ID
  2017-05-30  4:49 [PATCH] Input: synaptics-rmi4 - use %ph to form F34 configuration ID Dmitry Torokhov
@ 2017-05-30  8:23 ` Benjamin Tissoires
  2017-05-30 16:59   ` Dmitry Torokhov
  0 siblings, 1 reply; 8+ messages in thread
From: Benjamin Tissoires @ 2017-05-30  8:23 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: linux-input, Nick Dyer, Dan Carpenter, linux-kernel

On May 29 2017 or thereabouts, Dmitry Torokhov wrote:
> Instead of printing bytes one by one, let's use %phN to print the buffer in
> one go.
> 
> Also use hweight8 to count number of partitions instead of inspecting it
> bit by bit.
> 
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---

Looks good to me:
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

Cheers,
Benjamin

>  drivers/input/rmi4/rmi_f34v7.c | 22 ++++++++--------------
>  1 file changed, 8 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/input/rmi4/rmi_f34v7.c b/drivers/input/rmi4/rmi_f34v7.c
> index ae2db1c3aebf..3991d2943660 100644
> --- a/drivers/input/rmi4/rmi_f34v7.c
> +++ b/drivers/input/rmi4/rmi_f34v7.c
> @@ -9,13 +9,14 @@
>   * the Free Software Foundation.
>   */
>  
> +#include <linux/bitops.h>
>  #include <linux/kernel.h>
>  #include <linux/rmi.h>
>  #include <linux/firmware.h>
> -#include <asm/unaligned.h>
>  #include <linux/delay.h>
>  #include <linux/slab.h>
>  #include <linux/jiffies.h>
> +#include <asm/unaligned.h>
>  
>  #include "rmi_driver.h"
>  #include "rmi_f34.h"
> @@ -464,7 +465,7 @@ static int rmi_f34v7_read_queries_bl_version(struct f34_data *f34)
>  static int rmi_f34v7_read_queries(struct f34_data *f34)
>  {
>  	int ret;
> -	int i, j;
> +	int i;
>  	u8 base;
>  	int offset;
>  	u8 *ptable;
> @@ -519,9 +520,6 @@ static int rmi_f34v7_read_queries(struct f34_data *f34)
>  
>  	if (query_0 & HAS_CONFIG_ID) {
>  		u8 f34_ctrl[CONFIG_ID_SIZE];
> -		int i = 0;
> -		u8 *p = f34->configuration_id;
> -		*p = '\0';
>  
>  		ret = rmi_read_block(f34->fn->rmi_dev,
>  				f34->fn->fd.control_base_addr,
> @@ -531,13 +529,11 @@ static int rmi_f34v7_read_queries(struct f34_data *f34)
>  			return ret;
>  
>  		/* Eat leading zeros */
> -		while (i < sizeof(f34_ctrl) && !f34_ctrl[i])
> -			i++;
> +		for (i = 0; i < sizeof(f34_ctrl) - 1 && !f34_ctrl[i]; i++)
> +			/* Empty */;
>  
> -		for (; i < sizeof(f34_ctrl); i++)
> -			p += snprintf(p, f34->configuration_id
> -				      + sizeof(f34->configuration_id) - p,
> -				      "%02X", f34_ctrl[i]);
> +		snprintf(f34->configuration_id, sizeof(f34->configuration_id),
> +			 "%*phN", (int)sizeof(f34_ctrl) - i, f34_ctrl + i);
>  
>  		rmi_dbg(RMI_DEBUG_FN, &f34->fn->dev, "Configuration ID: %s\n",
>  			f34->configuration_id);
> @@ -545,9 +541,7 @@ static int rmi_f34v7_read_queries(struct f34_data *f34)
>  
>  	f34->v7.partitions = 0;
>  	for (i = 0; i < sizeof(query_1_7.partition_support); i++)
> -		for (j = 0; j < 8; j++)
> -			if (query_1_7.partition_support[i] & (1 << j))
> -				f34->v7.partitions++;
> +		f34->v7.partitions += hweight8(query_1_7.partition_support[i]);
>  
>  	rmi_dbg(RMI_DEBUG_FN, &f34->fn->dev, "%s: Supported partitions: %*ph\n",
>  		__func__, sizeof(query_1_7.partition_support),
> -- 
> 2.13.0.219.gdb65acc882-goog
> 
> 
> -- 
> Dmitry

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Input: synaptics-rmi4 - use %ph to form F34 configuration ID
  2017-05-30  8:23 ` Benjamin Tissoires
@ 2017-05-30 16:59   ` Dmitry Torokhov
  2017-05-30 17:23     ` Joe Perches
  2017-05-31 21:11     ` Nick Dyer
  0 siblings, 2 replies; 8+ messages in thread
From: Dmitry Torokhov @ 2017-05-30 16:59 UTC (permalink / raw)
  To: Benjamin Tissoires; +Cc: linux-input, Nick Dyer, Dan Carpenter, linux-kernel

On Tue, May 30, 2017 at 10:23:58AM +0200, Benjamin Tissoires wrote:
> On May 29 2017 or thereabouts, Dmitry Torokhov wrote:
> > Instead of printing bytes one by one, let's use %phN to print the buffer in
> > one go.
> > 
> > Also use hweight8 to count number of partitions instead of inspecting it
> > bit by bit.
> > 
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > ---
> 
> Looks good to me:
> Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

Hm, I just realized taht we'd go from upper to lowercase hex digits. I
think it should be OK, but I'd like to hear interested parties (Nick).

Or we'd need to introduce %pH I guess.

> 
> Cheers,
> Benjamin
> 
> >  drivers/input/rmi4/rmi_f34v7.c | 22 ++++++++--------------
> >  1 file changed, 8 insertions(+), 14 deletions(-)
> > 
> > diff --git a/drivers/input/rmi4/rmi_f34v7.c b/drivers/input/rmi4/rmi_f34v7.c
> > index ae2db1c3aebf..3991d2943660 100644
> > --- a/drivers/input/rmi4/rmi_f34v7.c
> > +++ b/drivers/input/rmi4/rmi_f34v7.c
> > @@ -9,13 +9,14 @@
> >   * the Free Software Foundation.
> >   */
> >  
> > +#include <linux/bitops.h>
> >  #include <linux/kernel.h>
> >  #include <linux/rmi.h>
> >  #include <linux/firmware.h>
> > -#include <asm/unaligned.h>
> >  #include <linux/delay.h>
> >  #include <linux/slab.h>
> >  #include <linux/jiffies.h>
> > +#include <asm/unaligned.h>
> >  
> >  #include "rmi_driver.h"
> >  #include "rmi_f34.h"
> > @@ -464,7 +465,7 @@ static int rmi_f34v7_read_queries_bl_version(struct f34_data *f34)
> >  static int rmi_f34v7_read_queries(struct f34_data *f34)
> >  {
> >  	int ret;
> > -	int i, j;
> > +	int i;
> >  	u8 base;
> >  	int offset;
> >  	u8 *ptable;
> > @@ -519,9 +520,6 @@ static int rmi_f34v7_read_queries(struct f34_data *f34)
> >  
> >  	if (query_0 & HAS_CONFIG_ID) {
> >  		u8 f34_ctrl[CONFIG_ID_SIZE];
> > -		int i = 0;
> > -		u8 *p = f34->configuration_id;
> > -		*p = '\0';
> >  
> >  		ret = rmi_read_block(f34->fn->rmi_dev,
> >  				f34->fn->fd.control_base_addr,
> > @@ -531,13 +529,11 @@ static int rmi_f34v7_read_queries(struct f34_data *f34)
> >  			return ret;
> >  
> >  		/* Eat leading zeros */
> > -		while (i < sizeof(f34_ctrl) && !f34_ctrl[i])
> > -			i++;
> > +		for (i = 0; i < sizeof(f34_ctrl) - 1 && !f34_ctrl[i]; i++)
> > +			/* Empty */;
> >  
> > -		for (; i < sizeof(f34_ctrl); i++)
> > -			p += snprintf(p, f34->configuration_id
> > -				      + sizeof(f34->configuration_id) - p,
> > -				      "%02X", f34_ctrl[i]);
> > +		snprintf(f34->configuration_id, sizeof(f34->configuration_id),
> > +			 "%*phN", (int)sizeof(f34_ctrl) - i, f34_ctrl + i);
> >  
> >  		rmi_dbg(RMI_DEBUG_FN, &f34->fn->dev, "Configuration ID: %s\n",
> >  			f34->configuration_id);
> > @@ -545,9 +541,7 @@ static int rmi_f34v7_read_queries(struct f34_data *f34)
> >  
> >  	f34->v7.partitions = 0;
> >  	for (i = 0; i < sizeof(query_1_7.partition_support); i++)
> > -		for (j = 0; j < 8; j++)
> > -			if (query_1_7.partition_support[i] & (1 << j))
> > -				f34->v7.partitions++;
> > +		f34->v7.partitions += hweight8(query_1_7.partition_support[i]);
> >  
> >  	rmi_dbg(RMI_DEBUG_FN, &f34->fn->dev, "%s: Supported partitions: %*ph\n",
> >  		__func__, sizeof(query_1_7.partition_support),
> > -- 
> > 2.13.0.219.gdb65acc882-goog
> > 
> > 
> > -- 
> > Dmitry

-- 
Dmitry

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Input: synaptics-rmi4 - use %ph to form F34 configuration ID
  2017-05-30 16:59   ` Dmitry Torokhov
@ 2017-05-30 17:23     ` Joe Perches
  2017-05-30 19:04       ` Andy Shevchenko
  2017-05-31 21:11     ` Nick Dyer
  1 sibling, 1 reply; 8+ messages in thread
From: Joe Perches @ 2017-05-30 17:23 UTC (permalink / raw)
  To: Dmitry Torokhov, Benjamin Tissoires, Andy Shevchenko
  Cc: linux-input, Nick Dyer, Dan Carpenter, linux-kernel

On Tue, 2017-05-30 at 09:59 -0700, Dmitry Torokhov wrote:
> On Tue, May 30, 2017 at 10:23:58AM +0200, Benjamin Tissoires wrote:
> > On May 29 2017 or thereabouts, Dmitry Torokhov wrote:
> > > Instead of printing bytes one by one, let's use %phN to print the buffer in
> > > one go.
> > > 
> > > Also use hweight8 to count number of partitions instead of inspecting it
> > > bit by bit.
> > > 
> > > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > > ---
> > 
> > Looks good to me:
> > Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> 
> Hm, I just realized taht we'd go from upper to lowercase hex digits. I
> think it should be OK, but I'd like to hear interested parties (Nick).
> 
> Or we'd need to introduce %pH I guess.

As it's debugging output, I'd hazard to guess it doesn't matter.

If someone _really_ needs upper-case output in *ph, I'd also
suggest adding another qualifier to keep the number of letter
prefixes used down.  %ph[CDNU].

A pity that %ph[cdnCDN] isn't possible as that's the style used
by %pU[blBL] for lower/upper case output.

There are < 80 uses of %[\*\d]+ph[CDN], maybe it'd be reasonable
to do a treewide sed to lowercase for them.  Dunno.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Input: synaptics-rmi4 - use %ph to form F34 configuration ID
  2017-05-30 17:23     ` Joe Perches
@ 2017-05-30 19:04       ` Andy Shevchenko
  2017-05-30 19:17         ` Joe Perches
  0 siblings, 1 reply; 8+ messages in thread
From: Andy Shevchenko @ 2017-05-30 19:04 UTC (permalink / raw)
  To: Joe Perches, Dmitry Torokhov, Benjamin Tissoires
  Cc: linux-input, Nick Dyer, Dan Carpenter, linux-kernel

On Tue, 2017-05-30 at 10:23 -0700, Joe Perches wrote:
> On Tue, 2017-05-30 at 09:59 -0700, Dmitry Torokhov wrote:
> > On Tue, May 30, 2017 at 10:23:58AM +0200, Benjamin Tissoires wrote:
> > > On May 29 2017 or thereabouts, Dmitry Torokhov wrote:
> > > > 

> > Or we'd need to introduce %pH I guess.
> 
> As it's debugging output, I'd hazard to guess it doesn't matter.
> 
> If someone _really_ needs upper-case output in *ph,

Starting from the same...

> 
> A pity that %ph[cdnCDN] isn't possible as that's the style used
> by %pU[blBL] for lower/upper case output.
> 
> There are < 80 uses of %[\*\d]+ph[CDN], maybe it'd be reasonable
> to do a treewide sed to lowercase for them.  Dunno.

And I think less intrusive would be 
%pH[CDN]
since the latter proposal misses plain %*ph use.

But for now I would rather avoid it at all, I'm pretty sure there very
rare cases when one needs
1) to print a dump, *and*
2) it should be upper cased.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Input: synaptics-rmi4 - use %ph to form F34 configuration ID
  2017-05-30 19:04       ` Andy Shevchenko
@ 2017-05-30 19:17         ` Joe Perches
  0 siblings, 0 replies; 8+ messages in thread
From: Joe Perches @ 2017-05-30 19:17 UTC (permalink / raw)
  To: Andy Shevchenko, Dmitry Torokhov, Benjamin Tissoires
  Cc: linux-input, Nick Dyer, Dan Carpenter, linux-kernel

On Tue, 2017-05-30 at 22:04 +0300, Andy Shevchenko wrote:
> On Tue, 2017-05-30 at 10:23 -0700, Joe Perches wrote:
> > On Tue, 2017-05-30 at 09:59 -0700, Dmitry Torokhov wrote:
> > > On Tue, May 30, 2017 at 10:23:58AM +0200, Benjamin Tissoires wrote:
> > > > On May 29 2017 or thereabouts, Dmitry Torokhov wrote:
> > > > > 
> > > Or we'd need to introduce %pH I guess.
> > 
> > As it's debugging output, I'd hazard to guess it doesn't matter.
> > 
> > If someone _really_ needs upper-case output in *ph,
> 
> Starting from the same...
> > 
> > A pity that %ph[cdnCDN] isn't possible as that's the style used
> > by %pU[blBL] for lower/upper case output.
> > 
> > There are < 80 uses of %[\*\d]+ph[CDN], maybe it'd be reasonable
> > to do a treewide sed to lowercase for them.  Dunno.

It's more like ~300 when the grep is done properly,
-E needed to be -P, so I believe that's too many to
bother with a treewide sed.

> And I think less intrusive would be 
> %pH[CDN]
> since the latter proposal misses plain %*ph use.

No it wouldn't necessarily.
%*phU could still be upper case too.

> But for now I would rather avoid it at all, I'm pretty sure there very
> rare cases when one needs
> 1) to print a dump, *and*
> 2) it should be upper cased.

Doubtful, but maybe there is a seq_ use somewhere
that could be converted.

Anyway I think the proposed change in the original
patch is fine and nothing else needs to be done.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Input: synaptics-rmi4 - use %ph to form F34 configuration ID
  2017-05-30 16:59   ` Dmitry Torokhov
  2017-05-30 17:23     ` Joe Perches
@ 2017-05-31 21:11     ` Nick Dyer
  2017-06-02  5:10       ` Dmitry Torokhov
  1 sibling, 1 reply; 8+ messages in thread
From: Nick Dyer @ 2017-05-31 21:11 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Benjamin Tissoires, linux-input, Dan Carpenter, linux-kernel,
	Chris Healy

On Tue, May 30, 2017 at 09:59:13AM -0700, Dmitry Torokhov wrote:
> On Tue, May 30, 2017 at 10:23:58AM +0200, Benjamin Tissoires wrote:
> > On May 29 2017 or thereabouts, Dmitry Torokhov wrote:
> > > Instead of printing bytes one by one, let's use %phN to print the buffer in
> > > one go.
> > > 
> > > Also use hweight8 to count number of partitions instead of inspecting it
> > > bit by bit.
> > > 
> > > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > > ---
> > 
> > Looks good to me:
> > Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> 
> Hm, I just realized taht we'd go from upper to lowercase hex digits. I
> think it should be OK, but I'd like to hear interested parties (Nick).
> 
> Or we'd need to introduce %pH I guess.

Hi Dmitry-

I can't see any issue with this: all the firmwares I've encountered are
purely numerical anyway.

Tested-by: Nick Dyer <nick@shmanahar.org>

> 
> > 
> > Cheers,
> > Benjamin
> > 
> > >  drivers/input/rmi4/rmi_f34v7.c | 22 ++++++++--------------
> > >  1 file changed, 8 insertions(+), 14 deletions(-)
> > > 
> > > diff --git a/drivers/input/rmi4/rmi_f34v7.c b/drivers/input/rmi4/rmi_f34v7.c
> > > index ae2db1c3aebf..3991d2943660 100644
> > > --- a/drivers/input/rmi4/rmi_f34v7.c
> > > +++ b/drivers/input/rmi4/rmi_f34v7.c
> > > @@ -9,13 +9,14 @@
> > >   * the Free Software Foundation.
> > >   */
> > >  
> > > +#include <linux/bitops.h>
> > >  #include <linux/kernel.h>
> > >  #include <linux/rmi.h>
> > >  #include <linux/firmware.h>
> > > -#include <asm/unaligned.h>
> > >  #include <linux/delay.h>
> > >  #include <linux/slab.h>
> > >  #include <linux/jiffies.h>
> > > +#include <asm/unaligned.h>
> > >  
> > >  #include "rmi_driver.h"
> > >  #include "rmi_f34.h"
> > > @@ -464,7 +465,7 @@ static int rmi_f34v7_read_queries_bl_version(struct f34_data *f34)
> > >  static int rmi_f34v7_read_queries(struct f34_data *f34)
> > >  {
> > >  	int ret;
> > > -	int i, j;
> > > +	int i;
> > >  	u8 base;
> > >  	int offset;
> > >  	u8 *ptable;
> > > @@ -519,9 +520,6 @@ static int rmi_f34v7_read_queries(struct f34_data *f34)
> > >  
> > >  	if (query_0 & HAS_CONFIG_ID) {
> > >  		u8 f34_ctrl[CONFIG_ID_SIZE];
> > > -		int i = 0;
> > > -		u8 *p = f34->configuration_id;
> > > -		*p = '\0';
> > >  
> > >  		ret = rmi_read_block(f34->fn->rmi_dev,
> > >  				f34->fn->fd.control_base_addr,
> > > @@ -531,13 +529,11 @@ static int rmi_f34v7_read_queries(struct f34_data *f34)
> > >  			return ret;
> > >  
> > >  		/* Eat leading zeros */
> > > -		while (i < sizeof(f34_ctrl) && !f34_ctrl[i])
> > > -			i++;
> > > +		for (i = 0; i < sizeof(f34_ctrl) - 1 && !f34_ctrl[i]; i++)
> > > +			/* Empty */;
> > >  
> > > -		for (; i < sizeof(f34_ctrl); i++)
> > > -			p += snprintf(p, f34->configuration_id
> > > -				      + sizeof(f34->configuration_id) - p,
> > > -				      "%02X", f34_ctrl[i]);
> > > +		snprintf(f34->configuration_id, sizeof(f34->configuration_id),
> > > +			 "%*phN", (int)sizeof(f34_ctrl) - i, f34_ctrl + i);
> > >  
> > >  		rmi_dbg(RMI_DEBUG_FN, &f34->fn->dev, "Configuration ID: %s\n",
> > >  			f34->configuration_id);
> > > @@ -545,9 +541,7 @@ static int rmi_f34v7_read_queries(struct f34_data *f34)
> > >  
> > >  	f34->v7.partitions = 0;
> > >  	for (i = 0; i < sizeof(query_1_7.partition_support); i++)
> > > -		for (j = 0; j < 8; j++)
> > > -			if (query_1_7.partition_support[i] & (1 << j))
> > > -				f34->v7.partitions++;
> > > +		f34->v7.partitions += hweight8(query_1_7.partition_support[i]);
> > >  
> > >  	rmi_dbg(RMI_DEBUG_FN, &f34->fn->dev, "%s: Supported partitions: %*ph\n",
> > >  		__func__, sizeof(query_1_7.partition_support),
> > > -- 
> > > 2.13.0.219.gdb65acc882-goog
> > > 
> > > 
> > > -- 
> > > Dmitry
> 
> -- 
> Dmitry

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Input: synaptics-rmi4 - use %ph to form F34 configuration ID
  2017-05-31 21:11     ` Nick Dyer
@ 2017-06-02  5:10       ` Dmitry Torokhov
  0 siblings, 0 replies; 8+ messages in thread
From: Dmitry Torokhov @ 2017-06-02  5:10 UTC (permalink / raw)
  To: Nick Dyer
  Cc: Benjamin Tissoires, linux-input, Dan Carpenter, linux-kernel,
	Chris Healy

On Wed, May 31, 2017 at 10:11:14PM +0100, Nick Dyer wrote:
> On Tue, May 30, 2017 at 09:59:13AM -0700, Dmitry Torokhov wrote:
> > On Tue, May 30, 2017 at 10:23:58AM +0200, Benjamin Tissoires wrote:
> > > On May 29 2017 or thereabouts, Dmitry Torokhov wrote:
> > > > Instead of printing bytes one by one, let's use %phN to print the buffer in
> > > > one go.
> > > > 
> > > > Also use hweight8 to count number of partitions instead of inspecting it
> > > > bit by bit.
> > > > 
> > > > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > > > ---
> > > 
> > > Looks good to me:
> > > Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> > 
> > Hm, I just realized taht we'd go from upper to lowercase hex digits. I
> > think it should be OK, but I'd like to hear interested parties (Nick).
> > 
> > Or we'd need to introduce %pH I guess.
> 
> Hi Dmitry-
> 
> I can't see any issue with this: all the firmwares I've encountered are
> purely numerical anyway.
> 
> Tested-by: Nick Dyer <nick@shmanahar.org>

Thanks Nick!

> 
> > 
> > > 
> > > Cheers,
> > > Benjamin
> > > 
> > > >  drivers/input/rmi4/rmi_f34v7.c | 22 ++++++++--------------
> > > >  1 file changed, 8 insertions(+), 14 deletions(-)
> > > > 
> > > > diff --git a/drivers/input/rmi4/rmi_f34v7.c b/drivers/input/rmi4/rmi_f34v7.c
> > > > index ae2db1c3aebf..3991d2943660 100644
> > > > --- a/drivers/input/rmi4/rmi_f34v7.c
> > > > +++ b/drivers/input/rmi4/rmi_f34v7.c
> > > > @@ -9,13 +9,14 @@
> > > >   * the Free Software Foundation.
> > > >   */
> > > >  
> > > > +#include <linux/bitops.h>
> > > >  #include <linux/kernel.h>
> > > >  #include <linux/rmi.h>
> > > >  #include <linux/firmware.h>
> > > > -#include <asm/unaligned.h>
> > > >  #include <linux/delay.h>
> > > >  #include <linux/slab.h>
> > > >  #include <linux/jiffies.h>
> > > > +#include <asm/unaligned.h>
> > > >  
> > > >  #include "rmi_driver.h"
> > > >  #include "rmi_f34.h"
> > > > @@ -464,7 +465,7 @@ static int rmi_f34v7_read_queries_bl_version(struct f34_data *f34)
> > > >  static int rmi_f34v7_read_queries(struct f34_data *f34)
> > > >  {
> > > >  	int ret;
> > > > -	int i, j;
> > > > +	int i;
> > > >  	u8 base;
> > > >  	int offset;
> > > >  	u8 *ptable;
> > > > @@ -519,9 +520,6 @@ static int rmi_f34v7_read_queries(struct f34_data *f34)
> > > >  
> > > >  	if (query_0 & HAS_CONFIG_ID) {
> > > >  		u8 f34_ctrl[CONFIG_ID_SIZE];
> > > > -		int i = 0;
> > > > -		u8 *p = f34->configuration_id;
> > > > -		*p = '\0';
> > > >  
> > > >  		ret = rmi_read_block(f34->fn->rmi_dev,
> > > >  				f34->fn->fd.control_base_addr,
> > > > @@ -531,13 +529,11 @@ static int rmi_f34v7_read_queries(struct f34_data *f34)
> > > >  			return ret;
> > > >  
> > > >  		/* Eat leading zeros */
> > > > -		while (i < sizeof(f34_ctrl) && !f34_ctrl[i])
> > > > -			i++;
> > > > +		for (i = 0; i < sizeof(f34_ctrl) - 1 && !f34_ctrl[i]; i++)
> > > > +			/* Empty */;
> > > >  
> > > > -		for (; i < sizeof(f34_ctrl); i++)
> > > > -			p += snprintf(p, f34->configuration_id
> > > > -				      + sizeof(f34->configuration_id) - p,
> > > > -				      "%02X", f34_ctrl[i]);
> > > > +		snprintf(f34->configuration_id, sizeof(f34->configuration_id),
> > > > +			 "%*phN", (int)sizeof(f34_ctrl) - i, f34_ctrl + i);
> > > >  
> > > >  		rmi_dbg(RMI_DEBUG_FN, &f34->fn->dev, "Configuration ID: %s\n",
> > > >  			f34->configuration_id);
> > > > @@ -545,9 +541,7 @@ static int rmi_f34v7_read_queries(struct f34_data *f34)
> > > >  
> > > >  	f34->v7.partitions = 0;
> > > >  	for (i = 0; i < sizeof(query_1_7.partition_support); i++)
> > > > -		for (j = 0; j < 8; j++)
> > > > -			if (query_1_7.partition_support[i] & (1 << j))
> > > > -				f34->v7.partitions++;
> > > > +		f34->v7.partitions += hweight8(query_1_7.partition_support[i]);
> > > >  
> > > >  	rmi_dbg(RMI_DEBUG_FN, &f34->fn->dev, "%s: Supported partitions: %*ph\n",
> > > >  		__func__, sizeof(query_1_7.partition_support),
> > > > -- 
> > > > 2.13.0.219.gdb65acc882-goog
> > > > 
> > > > 
> > > > -- 
> > > > Dmitry
> > 
> > -- 
> > Dmitry

-- 
Dmitry

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2017-06-02  5:10 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-30  4:49 [PATCH] Input: synaptics-rmi4 - use %ph to form F34 configuration ID Dmitry Torokhov
2017-05-30  8:23 ` Benjamin Tissoires
2017-05-30 16:59   ` Dmitry Torokhov
2017-05-30 17:23     ` Joe Perches
2017-05-30 19:04       ` Andy Shevchenko
2017-05-30 19:17         ` Joe Perches
2017-05-31 21:11     ` Nick Dyer
2017-06-02  5:10       ` Dmitry Torokhov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.