Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1652806 > unrolled thread

[PATCH] Input: synaptics-rmi4 - use %ph to form F34 configuration ID

Back to article view | Back to linux.kernel

Contents

  [PATCH] Input: synaptics-rmi4 - use %ph to form F34 configuration ID Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2017-05-30 06:50 +0200
    Re: [PATCH] Input: synaptics-rmi4 - use %ph to form F34  configuration ID Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2017-05-30 10:30 +0200
      Re: [PATCH] Input: synaptics-rmi4 - use %ph to form F34  configuration ID Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2017-05-30 19:00 +0200
        Re: [PATCH] Input: synaptics-rmi4 - use %ph to form F34  configuration ID Joe Perches <joe@perches.com> - 2017-05-30 19:30 +0200
          Re: [PATCH] Input: synaptics-rmi4 - use %ph to form F34  configuration ID Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2017-05-30 21:10 +0200
            Re: [PATCH] Input: synaptics-rmi4 - use %ph to form F34  configuration ID Joe Perches <joe@perches.com> - 2017-05-30 21:20 +0200
        Re: [PATCH] Input: synaptics-rmi4 - use %ph to form F34  configuration ID Nick Dyer <nick@shmanahar.org> - 2017-05-31 23:20 +0200
          Re: [PATCH] Input: synaptics-rmi4 - use %ph to form F34  configuration ID Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2017-06-02 07:20 +0200

#1652806 — [PATCH] Input: synaptics-rmi4 - use %ph to form F34 configuration ID

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

[toc] | [next] | [standalone]

#1652941 — Re: [PATCH] Input: synaptics-rmi4 - use %ph to form F34 configuration ID

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

[toc] | [prev] | [next] | [standalone]

#1653398 — Re: [PATCH] Input: synaptics-rmi4 - use %ph to form F34 configuration ID

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

[toc] | [prev] | [next] | [standalone]

#1653424 — Re: [PATCH] Input: synaptics-rmi4 - use %ph to form F34 configuration ID

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.

[toc] | [prev] | [next] | [standalone]

#1653502 — Re: [PATCH] Input: synaptics-rmi4 - use %ph to form F34 configuration ID

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

[toc] | [prev] | [next] | [standalone]

#1653511 — Re: [PATCH] Input: synaptics-rmi4 - use %ph to form F34 configuration ID

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.

[toc] | [prev] | [next] | [standalone]

#1654634 — Re: [PATCH] Input: synaptics-rmi4 - use %ph to form F34 configuration ID

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

[toc] | [prev] | [next] | [standalone]

#1655895 — Re: [PATCH] Input: synaptics-rmi4 - use %ph to form F34 configuration ID

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web