* [PATCH 0/5] Staging: panel: TODO fixes
@ 2015-12-29 20:04 Ksenija Stanojevic
2015-12-29 20:05 ` [PATCH 1/5] Staging: panel: Use u8 type Ksenija Stanojevic
` (4 more replies)
0 siblings, 5 replies; 9+ messages in thread
From: Ksenija Stanojevic @ 2015-12-29 20:04 UTC (permalink / raw)
To: gregkh; +Cc: willy, devel, linux-kernel, Ksenija Stanojevic
This patchset is based on patch sent by Bijosh Thykkoottathil.
Here I tried to address all suggestions made by Dan and Willy.
Signed-off-by: Ksenija Stanojevic <ksenija.stanojevic@gmail.com>
Ksenija Stanojevic (5):
Staging: panel: Use u8 type
Staging: panel: Remove typedef pmask_t
Staging: panel: Remove ULL
Staging: panel: Reduce value range for *name
Staging: panel: Make statement more readable
drivers/staging/panel/panel.c | 46 +++++++++++++++++++++----------------------
1 file changed, 22 insertions(+), 24 deletions(-)
--
1.9.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/5] Staging: panel: Use u8 type
2015-12-29 20:04 [PATCH 0/5] Staging: panel: TODO fixes Ksenija Stanojevic
@ 2015-12-29 20:05 ` Ksenija Stanojevic
2015-12-29 20:07 ` [PATCH 2/5] Staging: panel: Remove typedef pmask_t Ksenija Stanojevic
` (3 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Ksenija Stanojevic @ 2015-12-29 20:05 UTC (permalink / raw)
To: gregkh; +Cc: willy, devel, linux-kernel, Ksenija Stanojevic
Declare om, im, omask and imask as u8 to remove any confusion if
that describes the 8 bits of the data bus on the parallel port.
Also change return type of lcd_write_data() to u8.
Signed-off-by: Ksenija Stanojevic <ksenija.stanojevic@gmail.com>
---
drivers/staging/panel/panel.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/panel/panel.c b/drivers/staging/panel/panel.c
index 04d86f3..8bc604d 100644
--- a/drivers/staging/panel/panel.c
+++ b/drivers/staging/panel/panel.c
@@ -2042,11 +2042,11 @@ static void init_scan_timer(void)
* corresponding to out and in bits respectively.
* returns 1 if ok, 0 if error (in which case, nothing is written).
*/
-static int input_name2mask(const char *name, pmask_t *mask, pmask_t *value,
- char *imask, char *omask)
+static u8 input_name2mask(const char *name, pmask_t *mask, pmask_t *value,
+ u8 *imask, u8 *omask)
{
static char sigtab[10] = "EeSsPpAaBb";
- char im, om;
+ u8 im, om;
pmask_t m, v;
om = 0ULL;
--
1.9.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/5] Staging: panel: Remove typedef pmask_t
2015-12-29 20:04 [PATCH 0/5] Staging: panel: TODO fixes Ksenija Stanojevic
2015-12-29 20:05 ` [PATCH 1/5] Staging: panel: Use u8 type Ksenija Stanojevic
@ 2015-12-29 20:07 ` Ksenija Stanojevic
2015-12-29 20:08 ` [PATCH 3/5] Staging: panel: Remove ULL Ksenija Stanojevic
` (2 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Ksenija Stanojevic @ 2015-12-29 20:07 UTC (permalink / raw)
To: gregkh; +Cc: willy, devel, linux-kernel, Ksenija Stanojevic
Use __u64 instead of pmask_t and remove pmask_t since is useless.
Signed-off-by: Ksenija Stanojevic <ksenija.stanojevic@gmail.com>
---
drivers/staging/panel/panel.c | 24 +++++++++++-------------
1 file changed, 11 insertions(+), 13 deletions(-)
diff --git a/drivers/staging/panel/panel.c b/drivers/staging/panel/panel.c
index 8bc604d..7138ee7 100644
--- a/drivers/staging/panel/panel.c
+++ b/drivers/staging/panel/panel.c
@@ -172,8 +172,6 @@ static __u8 scan_mask_o;
/* logical or of the input bits involved in the scan matrix */
static __u8 scan_mask_i;
-typedef __u64 pmask_t;
-
enum input_type {
INPUT_TYPE_STD,
INPUT_TYPE_KBD,
@@ -188,8 +186,8 @@ enum input_state {
struct logical_input {
struct list_head list;
- pmask_t mask;
- pmask_t value;
+ __u64 mask;
+ __u64 value;
enum input_type type;
enum input_state state;
__u8 rise_time, fall_time;
@@ -219,19 +217,19 @@ static LIST_HEAD(logical_inputs); /* list of all defined logical inputs */
* corresponds to the ground.
* Within each group, bits are stored in the same order as read on the port :
* BAPSE (busy=4, ack=3, paper empty=2, select=1, error=0).
- * So, each __u64 (or pmask_t) is represented like this :
+ * So, each __u64 is represented like this :
* 0000000000000000000BAPSEBAPSEBAPSEBAPSEBAPSEBAPSEBAPSEBAPSEBAPSE
* <-----unused------><gnd><d07><d06><d05><d04><d03><d02><d01><d00>
*/
/* what has just been read from the I/O ports */
-static pmask_t phys_read;
+static __u64 phys_read;
/* previous phys_read */
-static pmask_t phys_read_prev;
+static __u64 phys_read_prev;
/* stabilized phys_read (phys_read|phys_read_prev) */
-static pmask_t phys_curr;
+static __u64 phys_curr;
/* previous phys_curr */
-static pmask_t phys_prev;
+static __u64 phys_prev;
/* 0 means that at least one logical signal needs be computed */
static char inputs_stable;
@@ -1789,7 +1787,7 @@ static void phys_scan_contacts(void)
gndmask = PNL_PINPUT(r_str(pprt)) & scan_mask_i;
/* grounded inputs are signals 40-44 */
- phys_read |= (pmask_t)gndmask << 40;
+ phys_read |= (__u64)gndmask << 40;
if (bitmask != gndmask) {
/*
@@ -1805,7 +1803,7 @@ static void phys_scan_contacts(void)
w_dtr(pprt, oldval & ~bitval); /* enable this output */
bitmask = PNL_PINPUT(r_str(pprt)) & ~gndmask;
- phys_read |= (pmask_t)bitmask << (5 * bit);
+ phys_read |= (__u64)bitmask << (5 * bit);
}
w_dtr(pprt, oldval); /* disable all outputs */
}
@@ -2042,12 +2040,12 @@ static void init_scan_timer(void)
* corresponding to out and in bits respectively.
* returns 1 if ok, 0 if error (in which case, nothing is written).
*/
-static u8 input_name2mask(const char *name, pmask_t *mask, pmask_t *value,
+static u8 input_name2mask(const char *name, __u64 *mask, __u64 *value,
u8 *imask, u8 *omask)
{
static char sigtab[10] = "EeSsPpAaBb";
u8 im, om;
- pmask_t m, v;
+ __u64 m, v;
om = 0ULL;
im = 0ULL;
--
1.9.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 3/5] Staging: panel: Remove ULL
2015-12-29 20:04 [PATCH 0/5] Staging: panel: TODO fixes Ksenija Stanojevic
2015-12-29 20:05 ` [PATCH 1/5] Staging: panel: Use u8 type Ksenija Stanojevic
2015-12-29 20:07 ` [PATCH 2/5] Staging: panel: Remove typedef pmask_t Ksenija Stanojevic
@ 2015-12-29 20:08 ` Ksenija Stanojevic
2015-12-29 20:59 ` Ilia Mirkin
2015-12-29 20:09 ` [PATCH 4/5] Staging: panel: Reduce value range for *name Ksenija Stanojevic
2015-12-29 20:11 ` [PATCH 5/5] Staging: panel: Make statement more readable Ksenija Stanojevic
4 siblings, 1 reply; 9+ messages in thread
From: Ksenija Stanojevic @ 2015-12-29 20:08 UTC (permalink / raw)
To: gregkh; +Cc: willy, devel, linux-kernel, Ksenija Stanojevic
Remove ULL since it's useless.
Signed-off-by: Ksenija Stanojevic <ksenija.stanojevic@gmail.com>
---
drivers/staging/panel/panel.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/panel/panel.c b/drivers/staging/panel/panel.c
index 7138ee7..6dc3efb 100644
--- a/drivers/staging/panel/panel.c
+++ b/drivers/staging/panel/panel.c
@@ -2047,10 +2047,10 @@ static u8 input_name2mask(const char *name, __u64 *mask, __u64 *value,
u8 im, om;
__u64 m, v;
- om = 0ULL;
- im = 0ULL;
- m = 0ULL;
- v = 0ULL;
+ om = 0;
+ im = 0;
+ m = 0;
+ v = 0;
while (*name) {
int in, out, bit, neg;
@@ -2076,9 +2076,9 @@ static u8 input_name2mask(const char *name, __u64 *mask, __u64 *value,
bit = (out * 5) + in;
- m |= 1ULL << bit;
+ m |= 1 << bit;
if (!neg)
- v |= 1ULL << bit;
+ v |= 1 << bit;
name++;
}
*mask = m;
--
1.9.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 4/5] Staging: panel: Reduce value range for *name
2015-12-29 20:04 [PATCH 0/5] Staging: panel: TODO fixes Ksenija Stanojevic
` (2 preceding siblings ...)
2015-12-29 20:08 ` [PATCH 3/5] Staging: panel: Remove ULL Ksenija Stanojevic
@ 2015-12-29 20:09 ` Ksenija Stanojevic
2015-12-29 20:11 ` [PATCH 5/5] Staging: panel: Make statement more readable Ksenija Stanojevic
4 siblings, 0 replies; 9+ messages in thread
From: Ksenija Stanojevic @ 2015-12-29 20:09 UTC (permalink / raw)
To: gregkh; +Cc: willy, devel, linux-kernel, Ksenija Stanojevic
out is 0-9 so it's too much for om, therefore reduce value range for
*name from '0'-'9' to '0'-'7'.
Signed-off-by: Ksenija Stanojevic <ksenija.stanojevic@gmail.com>
---
drivers/staging/panel/panel.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/panel/panel.c b/drivers/staging/panel/panel.c
index 6dc3efb..70cb9f3 100644
--- a/drivers/staging/panel/panel.c
+++ b/drivers/staging/panel/panel.c
@@ -2065,7 +2065,7 @@ static u8 input_name2mask(const char *name, __u64 *mask, __u64 *value,
im |= BIT(in);
name++;
- if (isdigit(*name)) {
+ if (*name >= '0' && *name <= '7') {
out = *name - '0';
om |= BIT(out);
} else if (*name == '-') {
--
1.9.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 5/5] Staging: panel: Make statement more readable
2015-12-29 20:04 [PATCH 0/5] Staging: panel: TODO fixes Ksenija Stanojevic
` (3 preceding siblings ...)
2015-12-29 20:09 ` [PATCH 4/5] Staging: panel: Reduce value range for *name Ksenija Stanojevic
@ 2015-12-29 20:11 ` Ksenija Stanojevic
2016-01-02 6:53 ` Willy Tarreau
4 siblings, 1 reply; 9+ messages in thread
From: Ksenija Stanojevic @ 2015-12-29 20:11 UTC (permalink / raw)
To: gregkh; +Cc: willy, devel, linux-kernel, Ksenija Stanojevic
Broke statement into 3 different lines to make it more readable.
Signeded-off-by: Ksenija Stanojevic <ksenija.stanojevic@gmail.com>
---
drivers/staging/panel/panel.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/panel/panel.c b/drivers/staging/panel/panel.c
index 70cb9f3..207d8d5 100644
--- a/drivers/staging/panel/panel.c
+++ b/drivers/staging/panel/panel.c
@@ -2054,8 +2054,8 @@ static u8 input_name2mask(const char *name, __u64 *mask, __u64 *value,
while (*name) {
int in, out, bit, neg;
- for (in = 0; (in < sizeof(sigtab)) && (sigtab[in] != *name);
- in++)
+ for (in = 0;
+ (in < sizeof(sigtab)) && (sigtab[in] != *name); in++)
;
if (in >= sizeof(sigtab))
--
1.9.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 3/5] Staging: panel: Remove ULL
2015-12-29 20:08 ` [PATCH 3/5] Staging: panel: Remove ULL Ksenija Stanojevic
@ 2015-12-29 20:59 ` Ilia Mirkin
2015-12-29 23:35 ` Willy Tarreau
0 siblings, 1 reply; 9+ messages in thread
From: Ilia Mirkin @ 2015-12-29 20:59 UTC (permalink / raw)
To: Ksenija Stanojevic; +Cc: Greg Kroah-Hartman, willy, devel, linux-kernel
On Tue, Dec 29, 2015 at 3:08 PM, Ksenija Stanojevic
<ksenija.stanojevic@gmail.com> wrote:
> Remove ULL since it's useless.
>
> Signed-off-by: Ksenija Stanojevic <ksenija.stanojevic@gmail.com>
> ---
> drivers/staging/panel/panel.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/staging/panel/panel.c b/drivers/staging/panel/panel.c
> index 7138ee7..6dc3efb 100644
> --- a/drivers/staging/panel/panel.c
> +++ b/drivers/staging/panel/panel.c
> @@ -2047,10 +2047,10 @@ static u8 input_name2mask(const char *name, __u64 *mask, __u64 *value,
> u8 im, om;
> __u64 m, v;
>
> - om = 0ULL;
> - im = 0ULL;
> - m = 0ULL;
> - v = 0ULL;
> + om = 0;
> + im = 0;
> + m = 0;
> + v = 0;
> while (*name) {
> int in, out, bit, neg;
>
> @@ -2076,9 +2076,9 @@ static u8 input_name2mask(const char *name, __u64 *mask, __u64 *value,
>
> bit = (out * 5) + in;
>
> - m |= 1ULL << bit;
> + m |= 1 << bit;
m and v are 64-bit, if bit can be >= 32, then you definitely do need
the ULL here...
> if (!neg)
> - v |= 1ULL << bit;
> + v |= 1 << bit;
> name++;
> }
> *mask = m;
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/5] Staging: panel: Remove ULL
2015-12-29 20:59 ` Ilia Mirkin
@ 2015-12-29 23:35 ` Willy Tarreau
0 siblings, 0 replies; 9+ messages in thread
From: Willy Tarreau @ 2015-12-29 23:35 UTC (permalink / raw)
To: Ilia Mirkin
Cc: Ksenija Stanojevic, Greg Kroah-Hartman, willy, devel, linux-kernel
On Tue, Dec 29, 2015 at 03:59:26PM -0500, Ilia Mirkin wrote:
> On Tue, Dec 29, 2015 at 3:08 PM, Ksenija Stanojevic
> <ksenija.stanojevic@gmail.com> wrote:
> > Remove ULL since it's useless.
> >
> > Signed-off-by: Ksenija Stanojevic <ksenija.stanojevic@gmail.com>
> > ---
> > drivers/staging/panel/panel.c | 12 ++++++------
> > 1 file changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/staging/panel/panel.c b/drivers/staging/panel/panel.c
> > index 7138ee7..6dc3efb 100644
> > --- a/drivers/staging/panel/panel.c
> > +++ b/drivers/staging/panel/panel.c
> > @@ -2047,10 +2047,10 @@ static u8 input_name2mask(const char *name, __u64 *mask, __u64 *value,
> > u8 im, om;
> > __u64 m, v;
> >
> > - om = 0ULL;
> > - im = 0ULL;
> > - m = 0ULL;
> > - v = 0ULL;
> > + om = 0;
> > + im = 0;
> > + m = 0;
> > + v = 0;
> > while (*name) {
> > int in, out, bit, neg;
> >
> > @@ -2076,9 +2076,9 @@ static u8 input_name2mask(const char *name, __u64 *mask, __u64 *value,
> >
> > bit = (out * 5) + in;
> >
> > - m |= 1ULL << bit;
> > + m |= 1 << bit;
>
> m and v are 64-bit, if bit can be >= 32, then you definitely do need
> the ULL here...
If that's it, indeed. I thought they were 8-bit from a previous patch.
I'm sorry for having suggested this cleanup, I've not put my nose in
this code for quite a while I must confess :-/
Regards,
Willy
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 5/5] Staging: panel: Make statement more readable
2015-12-29 20:11 ` [PATCH 5/5] Staging: panel: Make statement more readable Ksenija Stanojevic
@ 2016-01-02 6:53 ` Willy Tarreau
0 siblings, 0 replies; 9+ messages in thread
From: Willy Tarreau @ 2016-01-02 6:53 UTC (permalink / raw)
To: Ksenija Stanojevic; +Cc: gregkh, devel, linux-kernel
Hi Ksenija,
several points, see below.
On Tue, Dec 29, 2015 at 09:11:03PM +0100, Ksenija Stanojevic wrote:
> Broke statement into 3 different lines to make it more readable.
>
> Signeded-off-by: Ksenija Stanojevic <ksenija.stanojevic@gmail.com>
^^
Extra "ed". Don't waste your time typing it by hand, several git commands
(including git-commit and git-format-patch) will do it for you when you
pass "-s".
> diff --git a/drivers/staging/panel/panel.c b/drivers/staging/panel/panel.c
> index 70cb9f3..207d8d5 100644
> --- a/drivers/staging/panel/panel.c
> +++ b/drivers/staging/panel/panel.c
> @@ -2054,8 +2054,8 @@ static u8 input_name2mask(const char *name, __u64 *mask, __u64 *value,
> while (*name) {
> int in, out, bit, neg;
>
> - for (in = 0; (in < sizeof(sigtab)) && (sigtab[in] != *name);
> - in++)
> + for (in = 0;
> + (in < sizeof(sigtab)) && (sigtab[in] != *name); in++)
> ;
>
> if (in >= sizeof(sigtab))
well, this one is still ugly in my opinion. I've just looked at the code
and it was crap originally :
- sigtab[] is declared as static and without a trailing zero
- the for loop uses extra parenthesis and basically only reimplements
strchr()
- the end condition is tested again after the for loop
=> I'd rather use strchr(), and clean up this part, approx like this, but
do it as you want :
- static char sigtab[10] = "EeSsPpAaBb";
+ const char sigtab[] = "EeSsPpAaBb";
...
while (*name) {
int in, out, bit, neg;
+ const char *idx;
- for (in = 0; (in < sizeof(sigtab)) && (sigtab[in] != *name);
- in++)
-
- if (in >= sizeof(sigtab))
- return 0;
+ idx = strchr(sigtab, *name);
+ if (!idx)
+ return 0;
+
+ in = idx - sigtab;
I think it's more readable this way.
Regards,
Willy
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2016-01-02 6:53 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-29 20:04 [PATCH 0/5] Staging: panel: TODO fixes Ksenija Stanojevic
2015-12-29 20:05 ` [PATCH 1/5] Staging: panel: Use u8 type Ksenija Stanojevic
2015-12-29 20:07 ` [PATCH 2/5] Staging: panel: Remove typedef pmask_t Ksenija Stanojevic
2015-12-29 20:08 ` [PATCH 3/5] Staging: panel: Remove ULL Ksenija Stanojevic
2015-12-29 20:59 ` Ilia Mirkin
2015-12-29 23:35 ` Willy Tarreau
2015-12-29 20:09 ` [PATCH 4/5] Staging: panel: Reduce value range for *name Ksenija Stanojevic
2015-12-29 20:11 ` [PATCH 5/5] Staging: panel: Make statement more readable Ksenija Stanojevic
2016-01-02 6:53 ` Willy Tarreau
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).