* [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.