[PATCH 0/4] staging: fbtft: code cleanup patchset - checkpatch

[PATCH 0/4] staging: fbtft: code cleanup patchset - checkpatch

[Deepak R Varma]

Implement code formatting corrections as flagged by checkpatch
script. Common issues around lines exceeding 80 column limit and similar
formatting issues for the fbtft driver files are considered in the scope
of this patchset.

Note that top two patches in the list below were sent to the mailing
list for review as individual patches earlier. As I found additional
similar issues in the same area, I am requesting to combine these
related pathes in this patchset.

Deepak R Varma (4):
  staging: fbtft: Reformat line over 80 characters
  staging: fbtft: Reformat long macro definitions
  staging: fbtft: simplify array index computation
  staging: fbtft: Resolve likely precedence issues

 drivers/staging/fbtft/fbtft-core.c  |  4 +++-
 drivers/staging/fbtft/fbtft-sysfs.c |  6 +++++-
 drivers/staging/fbtft/fbtft.h       | 18 +++++++++++++-----
 3 files changed, 21 insertions(+), 7 deletions(-)

-- 
2.17.1

[PATCH 1/4] staging: fbtft: Reformat line over 80 characters

[Deepak R Varma]

A long variable name beyond 80 characters gets exended into the next
line. Reformating the line makes it more readable. Also adding an extra
line for the next instruction following current if block helps
understand it better.

Signed-off-by: Deepak R Varma <mh12gx2825@gmail.com>
---
 drivers/staging/fbtft/fbtft-core.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c
index d3e098b41b1a..4f362dad4436 100644
--- a/drivers/staging/fbtft/fbtft-core.c
+++ b/drivers/staging/fbtft/fbtft-core.c
@@ -1186,7 +1186,9 @@ static struct fbtft_platform_data *fbtft_properties_read(struct device *dev)
 	if (device_property_present(dev, "led-gpios"))
 		pdata->display.backlight = 1;
 	if (device_property_present(dev, "init"))
-		pdata->display.fbtftops.init_display = fbtft_init_display_from_property;
+		pdata->display.fbtftops.init_display =
+			fbtft_init_display_from_property;
+
 	pdata->display.fbtftops.request_gpios = fbtft_request_gpios;
 
 	return pdata;
-- 
2.17.1

[PATCH 2/4] staging: fbtft: Reformat long macro definitions

[Deepak R Varma]

Multiple macro definitions crossings 80 character line length nakes it
hard to understand. Reformating the these lines makes the code more
readable and easy to understand. Issue flagged by checkpatch script.

Signed-off-by: Deepak R Varma <mh12gx2825@gmail.com>
---
 drivers/staging/fbtft/fbtft.h | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/fbtft/fbtft.h b/drivers/staging/fbtft/fbtft.h
index 5f782da51959..81da30f4062e 100644
--- a/drivers/staging/fbtft/fbtft.h
+++ b/drivers/staging/fbtft/fbtft.h
@@ -348,9 +348,17 @@ module_exit(fbtft_driver_module_exit);
 
 /* shorthand debug levels */
 #define DEBUG_LEVEL_1	DEBUG_REQUEST_GPIOS
-#define DEBUG_LEVEL_2	(DEBUG_LEVEL_1 | DEBUG_DRIVER_INIT_FUNCTIONS | DEBUG_TIME_FIRST_UPDATE)
-#define DEBUG_LEVEL_3	(DEBUG_LEVEL_2 | DEBUG_RESET | DEBUG_INIT_DISPLAY | DEBUG_BLANK | DEBUG_REQUEST_GPIOS | DEBUG_FREE_GPIOS | DEBUG_VERIFY_GPIOS | DEBUG_BACKLIGHT | DEBUG_SYSFS)
-#define DEBUG_LEVEL_4	(DEBUG_LEVEL_2 | DEBUG_FB_READ | DEBUG_FB_WRITE | DEBUG_FB_FILLRECT | DEBUG_FB_COPYAREA | DEBUG_FB_IMAGEBLIT | DEBUG_FB_BLANK)
+#define DEBUG_LEVEL_2	(DEBUG_LEVEL_1 | DEBUG_DRIVER_INIT_FUNCTIONS        \
+				       | DEBUG_TIME_FIRST_UPDATE)
+#define DEBUG_LEVEL_3	(DEBUG_LEVEL_2 | DEBUG_RESET | DEBUG_INIT_DISPLAY   \
+				       | DEBUG_BLANK | DEBUG_REQUEST_GPIOS  \
+				       | DEBUG_FREE_GPIOS                   \
+				       | DEBUG_VERIFY_GPIOS                 \
+				       | DEBUG_BACKLIGHT | DEBUG_SYSFS)
+#define DEBUG_LEVEL_4	(DEBUG_LEVEL_2 | DEBUG_FB_READ | DEBUG_FB_WRITE     \
+				       | DEBUG_FB_FILLRECT                  \
+				       | DEBUG_FB_COPYAREA                  \
+				       | DEBUG_FB_IMAGEBLIT | DEBUG_FB_BLANK)
 #define DEBUG_LEVEL_5	(DEBUG_LEVEL_3 | DEBUG_UPDATE_DISPLAY)
 #define DEBUG_LEVEL_6	(DEBUG_LEVEL_4 | DEBUG_LEVEL_5)
 #define DEBUG_LEVEL_7	0xFFFFFFFF
-- 
2.17.1

[PATCH 3/4] staging: fbtft: simplify array index computation

[Deepak R Varma]

An array index is being computed by mathematical calculation on the
Lvalue side of the expression. This also further results in the staement
exceeding 80 character statement length.

A local variable can store the value of the array index computation. The
variable can then be used as array index. This improves readability of
the code and also address 80 character warning raised by checkpatch.

Signed-off-by: Deepak R Varma <mh12gx2825@gmail.com>
---
 drivers/staging/fbtft/fbtft-sysfs.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/fbtft/fbtft-sysfs.c b/drivers/staging/fbtft/fbtft-sysfs.c
index 2a5c630dab87..4d505a13f7ab 100644
--- a/drivers/staging/fbtft/fbtft-sysfs.c
+++ b/drivers/staging/fbtft/fbtft-sysfs.c
@@ -25,6 +25,7 @@ int fbtft_gamma_parse_str(struct fbtft_par *par, u32 *curves,
 	unsigned long val = 0;
 	int ret = 0;
 	int curve_counter, value_counter;
+	int counter = 0;
 
 	fbtft_par_dbg(DEBUG_SYSFS, par, "%s() str=\n", __func__);
 
@@ -68,7 +69,10 @@ int fbtft_gamma_parse_str(struct fbtft_par *par, u32 *curves,
 			ret = get_next_ulong(&curve_p, &val, " ", 16);
 			if (ret)
 				goto out;
-			curves[curve_counter * par->gamma.num_values + value_counter] = val;
+
+			counter = curve_counter * par->gamma.num_values +
+					value_counter;
+			curves[counter] = val;
 			value_counter++;
 		}
 		if (value_counter != par->gamma.num_values) {
-- 
2.17.1

[PATCH 4/4] staging: fbtft: Resolve likely precedence issues

[Deepak R Varma]

Macro fbtft_par_dbg uses arguments supplied for dereferencing in the
expanded form. It can become a operator precedence issue if arguments
are used without proper wrapping. Putting arguments within braces and
then dereferencing would avoid operator precedence issues. Issue flagged
by checkpatch script.

Signed-off-by: Deepak R Varma <mh12gx2825@gmail.com>
---
 drivers/staging/fbtft/fbtft.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/fbtft/fbtft.h b/drivers/staging/fbtft/fbtft.h
index 81da30f4062e..76f8c090a837 100644
--- a/drivers/staging/fbtft/fbtft.h
+++ b/drivers/staging/fbtft/fbtft.h
@@ -406,8 +406,8 @@ do {                                                         \
 
 #define fbtft_par_dbg(level, par, format, arg...)            \
 do {                                                         \
-	if (unlikely(par->debug & level))                    \
-		dev_info(par->info->device, format, ##arg);  \
+	if (unlikely((par)->debug & (level)))                    \
+		dev_info((par)->info->device, format, ##arg);  \
 } while (0)
 
 #define fbtft_par_dbg_hex(level, par, dev, type, buf, num, format, arg...) \
-- 
2.17.1

Re: [Outreachy kernel] [PATCH 4/4] staging: fbtft: Resolve likely precedence issues

[Julia Lawall]

On Fri, 13 Mar 2020, Deepak R Varma wrote:

> Macro fbtft_par_dbg uses arguments supplied for dereferencing in the
> expanded form. It can become a operator precedence issue if arguments
> are used without proper wrapping. Putting arguments within braces and
> then dereferencing would avoid operator precedence issues. Issue flagged
> by checkpatch script.

() are parentheses, not braces.  Braces are {}.

The log message could be more concise, allowing the reader to find the
important information more quickly.

Put parentheses around uses of macro parameters to avoid possible
precedence issues.  Problem detected by checkpatch.

The subject line is also misleading.  If you look at the uses of this
macro, it seems that the level argument is always made by calling BIT,
which parenthesizes its result, or is a number, or is a bit or that is
wrapped in parentheses.  The par argument is always a variable.  So
currently there does not appear to be any precedence issue.  So "likely"
does not seem appropriate.  Maybe "potential" would be better.

julia



>
> Signed-off-by: Deepak R Varma <mh12gx2825@gmail.com>
> ---
>  drivers/staging/fbtft/fbtft.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/fbtft/fbtft.h b/drivers/staging/fbtft/fbtft.h
> index 81da30f4062e..76f8c090a837 100644
> --- a/drivers/staging/fbtft/fbtft.h
> +++ b/drivers/staging/fbtft/fbtft.h
> @@ -406,8 +406,8 @@ do {                                                         \
>
>  #define fbtft_par_dbg(level, par, format, arg...)            \
>  do {                                                         \
> -	if (unlikely(par->debug & level))                    \
> -		dev_info(par->info->device, format, ##arg);  \
> +	if (unlikely((par)->debug & (level)))                    \
> +		dev_info((par)->info->device, format, ##arg);  \
>  } while (0)
>
>  #define fbtft_par_dbg_hex(level, par, dev, type, buf, num, format, arg...) \
> --
> 2.17.1
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/b216292fa529e2586d0a7e17cdd6fbd19cbb9235.1584045569.git.mh12gx2825%40gmail.com.
>

Re: [Outreachy kernel] [PATCH 4/4] staging: fbtft: Resolve likely precedence issues

[Deepak Varma]

[-- Attachment #1.1: Type: text/plain, Size: 2816 bytes --]



On Friday, 13 March 2020 12:31:36 UTC+5:30, Julia Lawall wrote:
>
>
>
> On Fri, 13 Mar 2020, Deepak R Varma wrote: 
>
> > Macro fbtft_par_dbg uses arguments supplied for dereferencing in the 
> > expanded form. It can become a operator precedence issue if arguments 
> > are used without proper wrapping. Putting arguments within braces and 
> > then dereferencing would avoid operator precedence issues. Issue flagged 
> > by checkpatch script. 
>
> () are parentheses, not braces.  Braces are {}. 
>
> The log message could be more concise, allowing the reader to find the 
> important information more quickly. 
>
> Put parentheses around uses of macro parameters to avoid possible 
> precedence issues.  Problem detected by checkpatch. 
>
> The subject line is also misleading.  If you look at the uses of this 
> macro, it seems that the level argument is always made by calling BIT, 
> which parenthesizes its result, or is a number, or is a bit or that is 
> wrapped in parentheses.  The par argument is always a variable.  So 
> currently there does not appear to be any precedence issue.  So "likely" 
> does not seem appropriate.  Maybe "potential" would be better. 
>
> julia 
>

Hello Julia,
Thank you for the suggestions. Useful comments. I will update the patchset 
accordingly and resend v2.

Deepak.
  

>
>
>
> > 
> > Signed-off-by: Deepak R Varma <mh12g...@gmail.com <javascript:>> 
> > --- 
> >  drivers/staging/fbtft/fbtft.h | 4 ++-- 
> >  1 file changed, 2 insertions(+), 2 deletions(-) 
> > 
> > diff --git a/drivers/staging/fbtft/fbtft.h 
> b/drivers/staging/fbtft/fbtft.h 
> > index 81da30f4062e..76f8c090a837 100644 
> > --- a/drivers/staging/fbtft/fbtft.h 
> > +++ b/drivers/staging/fbtft/fbtft.h 
> > @@ -406,8 +406,8 @@ do {                                                 
>         \ 
> > 
> >  #define fbtft_par_dbg(level, par, format, arg...)            \ 
> >  do {                                                         \ 
> > -        if (unlikely(par->debug & level))                    \ 
> > -                dev_info(par->info->device, format, ##arg);  \ 
> > +        if (unlikely((par)->debug & (level)))                    \ 
> > +                dev_info((par)->info->device, format, ##arg);  \ 
> >  } while (0) 
> > 
> >  #define fbtft_par_dbg_hex(level, par, dev, type, buf, num, format, 
> arg...) \ 
> > -- 
> > 2.17.1 
> > 
> > -- 
> > You received this message because you are subscribed to the Google 
> Groups "outreachy-kernel" group. 
> > To unsubscribe from this group and stop receiving emails from it, send 
> an email to outreach...@googlegroups.com <javascript:>. 
> > To view this discussion on the web visit 
> https://groups.google.com/d/msgid/outreachy-kernel/b216292fa529e2586d0a7e17cdd6fbd19cbb9235.1584045569.git.mh12gx2825%40gmail.com. 
>
> > 
>

[-- Attachment #1.2: Type: text/html, Size: 4561 bytes --]