[PATCH v4 2/2] staging: fbtft: convert a macro to a function.

[PATCH v4 2/2] staging: fbtft: convert a macro to a function.

[Bhagyashri Dighole]

Convert a macro to an inline function to improve type safety and make
the code simpler.

Signed-off-by: Bhagyashri Dighole <digholebhagyashri@gmail.com>
---
Changes in v4:
  -- Modify log messages.

Changes in v3:
  -- Change the subject line according to log messages.
  -- Remove extra spaces.

Changes in v2:
  -- Make the converion from macro to inline function

 drivers/staging/fbtft/fb_watterott.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/fbtft/fb_watterott.c b/drivers/staging/fbtft/fb_watterott.c
index 502d67f..40e5427 100644
--- a/drivers/staging/fbtft/fb_watterott.c
+++ b/drivers/staging/fbtft/fb_watterott.c
@@ -90,9 +90,10 @@ static int write_vmem(struct fbtft_par *par, size_t offset, size_t len)
 	return 0;
 }
 
-#define RGB565toRGB332(c) ((((c) & 0xE000) >> 8) |\
-			   (((c) & 000700) >> 6) |\
-			   (((c) & 0x0018) >> 3))
+inline int rgb565_to_rgb332(u16 c)
+{
+	return ((c & 0xE000) >> 8)| ((c & 000700) >> 6)| ((c & 0x0018) >> 3);
+}
 
 static int write_vmem_8bit(struct fbtft_par *par, size_t offset, size_t len)
 {
@@ -116,7 +117,7 @@ static int write_vmem_8bit(struct fbtft_par *par, size_t offset, size_t len)
 	for (i = start_line; i <= end_line; i++) {
 		pos[1] = cpu_to_be16(i);
 		for (j = 0; j < par->info->var.xres; j++) {
-			buf8[j] = RGB565toRGB332(*vmem16);
+			buf8[j] = rgb565_to_rgb332(*vmem16);
 			vmem16++;
 		}
 		ret = par->fbtftops.write(par,
-- 
2.7.4

Re: [PATCH v4 2/2] staging: fbtft: convert a macro to a function.

[Greg Kroah-Hartman]

On Wed, Mar 06, 2019 at 04:45:12PM +0530, Bhagyashri Dighole wrote:
> Convert a macro to an inline function to improve type safety and make
> the code simpler.
> 
> Signed-off-by: Bhagyashri Dighole <digholebhagyashri@gmail.com>
> ---
> Changes in v4:
>   -- Modify log messages.
> 
> Changes in v3:
>   -- Change the subject line according to log messages.
>   -- Remove extra spaces.
> 
> Changes in v2:
>   -- Make the converion from macro to inline function
> 
>  drivers/staging/fbtft/fb_watterott.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/fbtft/fb_watterott.c b/drivers/staging/fbtft/fb_watterott.c
> index 502d67f..40e5427 100644
> --- a/drivers/staging/fbtft/fb_watterott.c
> +++ b/drivers/staging/fbtft/fb_watterott.c
> @@ -90,9 +90,10 @@ static int write_vmem(struct fbtft_par *par, size_t offset, size_t len)
>  	return 0;
>  }
>  
> -#define RGB565toRGB332(c) ((((c) & 0xE000) >> 8) |\
> -			   (((c) & 000700) >> 6) |\
> -			   (((c) & 0x0018) >> 3))
> +inline int rgb565_to_rgb332(u16 c)
> +{
> +	return ((c & 0xE000) >> 8)| ((c & 000700) >> 6)| ((c & 0x0018) >> 3);

Odd placement of '|' right?  checkpatch.pl didn't complain about this?

Please put a space on both sides of that operator.

thanks,

greg k-h

Re: [PATCH v4 2/2] staging: fbtft: convert a macro to a function.

[Bhagyashri Dighole]

[-- Attachment #1: Type: text/plain, Size: 1640 bytes --]

On Fri, Mar 8, 2019 at 1:00 AM Greg Kroah-Hartman <
gregkh@linuxfoundation.org> wrote:

> On Wed, Mar 06, 2019 at 04:45:12PM +0530, Bhagyashri Dighole wrote:
> > Convert a macro to an inline function to improve type safety and make
> > the code simpler.
> >
> > Signed-off-by: Bhagyashri Dighole <digholebhagyashri@gmail.com>
> > ---
> > Changes in v4:
> >   -- Modify log messages.
> >
> > Changes in v3:
> >   -- Change the subject line according to log messages.
> >   -- Remove extra spaces.
> >
> > Changes in v2:
> >   -- Make the converion from macro to inline function
> >
> >  drivers/staging/fbtft/fb_watterott.c | 9 +++++----
> >  1 file changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/staging/fbtft/fb_watterott.c
> b/drivers/staging/fbtft/fb_watterott.c
> > index 502d67f..40e5427 100644
> > --- a/drivers/staging/fbtft/fb_watterott.c
> > +++ b/drivers/staging/fbtft/fb_watterott.c
> > @@ -90,9 +90,10 @@ static int write_vmem(struct fbtft_par *par, size_t
> offset, size_t len)
> >       return 0;
> >  }
> >
> > -#define RGB565toRGB332(c) ((((c) & 0xE000) >> 8) |\
> > -                        (((c) & 000700) >> 6) |\
> > -                        (((c) & 0x0018) >> 3))
> > +inline int rgb565_to_rgb332(u16 c)
> > +{
> > +     return ((c & 0xE000) >> 8)| ((c & 000700) >> 6)| ((c & 0x0018) >>
> 3);
>
> Odd placement of '|' right?  checkpatch.pl didn't complain about this?
>

Yes, checkpatch.pl complain about it, but as per the suggestion i got,
I need to skip one space because line is about to reach 80 characters.

>
> Please put a space on both sides of that operator.
>
> thanks,
>
> greg k-h
>

[-- Attachment #2: Type: text/html, Size: 2634 bytes --]

Re: [PATCH v4 2/2] staging: fbtft: convert a macro to a function.

[Greg Kroah-Hartman]

On Fri, Mar 08, 2019 at 11:35:02AM +0530, Bhagyashri Dighole wrote:
> On Fri, Mar 8, 2019 at 1:00 AM Greg Kroah-Hartman <
> gregkh@linuxfoundation.org> wrote:
> 
> > On Wed, Mar 06, 2019 at 04:45:12PM +0530, Bhagyashri Dighole wrote:
> > > Convert a macro to an inline function to improve type safety and make
> > > the code simpler.
> > >
> > > Signed-off-by: Bhagyashri Dighole <digholebhagyashri@gmail.com>
> > > ---
> > > Changes in v4:
> > >   -- Modify log messages.
> > >
> > > Changes in v3:
> > >   -- Change the subject line according to log messages.
> > >   -- Remove extra spaces.
> > >
> > > Changes in v2:
> > >   -- Make the converion from macro to inline function
> > >
> > >  drivers/staging/fbtft/fb_watterott.c | 9 +++++----
> > >  1 file changed, 5 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/staging/fbtft/fb_watterott.c
> > b/drivers/staging/fbtft/fb_watterott.c
> > > index 502d67f..40e5427 100644
> > > --- a/drivers/staging/fbtft/fb_watterott.c
> > > +++ b/drivers/staging/fbtft/fb_watterott.c
> > > @@ -90,9 +90,10 @@ static int write_vmem(struct fbtft_par *par, size_t
> > offset, size_t len)
> > >       return 0;
> > >  }
> > >
> > > -#define RGB565toRGB332(c) ((((c) & 0xE000) >> 8) |\
> > > -                        (((c) & 000700) >> 6) |\
> > > -                        (((c) & 0x0018) >> 3))
> > > +inline int rgb565_to_rgb332(u16 c)
> > > +{
> > > +     return ((c & 0xE000) >> 8)| ((c & 000700) >> 6)| ((c & 0x0018) >>
> > 3);
> >
> > Odd placement of '|' right?  checkpatch.pl didn't complain about this?
> >
> 
> Yes, checkpatch.pl complain about it, but as per the suggestion i got,
> I need to skip one space because line is about to reach 80 characters.

Don't make one coding style change just to have to send a follow-on
coding style change for that modification.  The goal is to reduce the
number of warnings, not just constantly send new patches :)

thanks,

greg k-h

Re: [PATCH v4 2/2] staging: fbtft: convert a macro to a function.

[Bhagyashri Dighole]

[-- Attachment #1: Type: text/plain, Size: 2149 bytes --]

On Fri, Mar 8, 2019 at 11:51 AM Greg Kroah-Hartman <
gregkh@linuxfoundation.org> wrote:

> On Fri, Mar 08, 2019 at 11:35:02AM +0530, Bhagyashri Dighole wrote:
> > On Fri, Mar 8, 2019 at 1:00 AM Greg Kroah-Hartman <
> > gregkh@linuxfoundation.org> wrote:
> >
> > > On Wed, Mar 06, 2019 at 04:45:12PM +0530, Bhagyashri Dighole wrote:
> > > > Convert a macro to an inline function to improve type safety and make
> > > > the code simpler.
> > > >
> > > > Signed-off-by: Bhagyashri Dighole <digholebhagyashri@gmail.com>
> > > > ---
> > > > Changes in v4:
> > > >   -- Modify log messages.
> > > >
> > > > Changes in v3:
> > > >   -- Change the subject line according to log messages.
> > > >   -- Remove extra spaces.
> > > >
> > > > Changes in v2:
> > > >   -- Make the converion from macro to inline function
> > > >
> > > >  drivers/staging/fbtft/fb_watterott.c | 9 +++++----
> > > >  1 file changed, 5 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/drivers/staging/fbtft/fb_watterott.c
> > > b/drivers/staging/fbtft/fb_watterott.c
> > > > index 502d67f..40e5427 100644
> > > > --- a/drivers/staging/fbtft/fb_watterott.c
> > > > +++ b/drivers/staging/fbtft/fb_watterott.c
> > > > @@ -90,9 +90,10 @@ static int write_vmem(struct fbtft_par *par,
> size_t
> > > offset, size_t len)
> > > >       return 0;
> > > >  }
> > > >
> > > > -#define RGB565toRGB332(c) ((((c) & 0xE000) >> 8) |\
> > > > -                        (((c) & 000700) >> 6) |\
> > > > -                        (((c) & 0x0018) >> 3))
> > > > +inline int rgb565_to_rgb332(u16 c)
> > > > +{
> > > > +     return ((c & 0xE000) >> 8)| ((c & 000700) >> 6)| ((c & 0x0018)
> >>
> > > 3);
> > >
> > > Odd placement of '|' right?  checkpatch.pl didn't complain about this?
> > >
> >
> > Yes, checkpatch.pl complain about it, but as per the suggestion i got,
> > I need to skip one space because line is about to reach 80 characters.
>
> Don't make one coding style change just to have to send a follow-on
> coding style change for that modification.  The goal is to reduce the
> number of warnings, not just constantly send new patches :)
>

Okay.
Thanks.

>
> thanks,
>
> greg k-h
>

[-- Attachment #2: Type: text/html, Size: 3536 bytes --]

Re: [Outreachy kernel] Re: [PATCH v4 2/2] staging: fbtft: convert a macro to a function.

[Julia Lawall]

[-- Attachment #1: Type: text/plain, Size: 2609 bytes --]



On Fri, 8 Mar 2019, Bhagyashri Dighole wrote:

>
>
> On Fri, Mar 8, 2019 at 1:00 AM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
>       On Wed, Mar 06, 2019 at 04:45:12PM +0530, Bhagyashri Dighole
>       wrote:
>       > Convert a macro to an inline function to improve type safety
>       and make
>       > the code simpler.
>       >
>       > Signed-off-by: Bhagyashri Dighole
>       <digholebhagyashri@gmail.com>
>       > ---
>       > Changes in v4:
>       >   -- Modify log messages.
>       >
>       > Changes in v3:
>       >   -- Change the subject line according to log messages.
>       >   -- Remove extra spaces.
>       >
>       > Changes in v2:
>       >   -- Make the converion from macro to inline function
>       >
>       >  drivers/staging/fbtft/fb_watterott.c | 9 +++++----
>       >  1 file changed, 5 insertions(+), 4 deletions(-)
>       >
>       > diff --git a/drivers/staging/fbtft/fb_watterott.c
>       b/drivers/staging/fbtft/fb_watterott.c
>       > index 502d67f..40e5427 100644
>       > --- a/drivers/staging/fbtft/fb_watterott.c
>       > +++ b/drivers/staging/fbtft/fb_watterott.c
>       > @@ -90,9 +90,10 @@ static int write_vmem(struct fbtft_par
>       *par, size_t offset, size_t len)
>       >       return 0;
>       >  }
>       > 
>       > -#define RGB565toRGB332(c) ((((c) & 0xE000) >> 8) |\
>       > -                        (((c) & 000700) >> 6) |\
>       > -                        (((c) & 0x0018) >> 3))
>       > +inline int rgb565_to_rgb332(u16 c)
>       > +{
>       > +     return ((c & 0xE000) >> 8)| ((c & 000700) >> 6)| ((c &
>       0x0018) >> 3);
>
>       Odd placement of '|' right?  checkpatch.pl didn't complain about
>       this?
>
>
> Yes, checkpatch.pl complain about it, but as per the suggestion i got,
> I need to skip one space because line is about to reach 80 characters.

A newline would be fine, if needed.

julia

>
>       Please put a space on both sides of that operator.
>
>       thanks,
>
>       greg k-h
>
> --
> 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 post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visithttps://groups.google.com/d/msgid/outreachy-kernel/CAOMPgjgKXCwp80uTW2P7W0k
> bUYwRKpM6N3QKXnYERM_LxM5Bsw%40mail.gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>
>

[PATCH v4 2/2] staging: fbtft: convert a macro to a function.

[Bhagyashri Dighole]

Convert a macro to an inline function to improve type safety and make
the code simpler.

Signed-off-by: Bhagyashri Dighole <digholebhagyashri@gmail.com>
---
Changes in v4:
  -- Modify log messages.
  -- Use one space around operators.

Changes in v3:
  -- Change the subject line according to log messages.
  -- Remove extra spaces.

Changes in v2:
  -- Make the converion from macro to inline function

 drivers/staging/fbtft/fb_watterott.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/fbtft/fb_watterott.c b/drivers/staging/fbtft/fb_watterott.c
index 502d67f..27cc8ea 100644
--- a/drivers/staging/fbtft/fb_watterott.c
+++ b/drivers/staging/fbtft/fb_watterott.c
@@ -90,9 +90,10 @@ static int write_vmem(struct fbtft_par *par, size_t offset, size_t len)
 	return 0;
 }
 
-#define RGB565toRGB332(c) ((((c) & 0xE000) >> 8) |\
-			   (((c) & 000700) >> 6) |\
-			   (((c) & 0x0018) >> 3))
+static inline int rgb565_to_rgb332(u16 c)
+{
+	return ((c & 0xE000) >> 8) | ((c & 000700) >> 6) | ((c & 0x0018) >> 3);
+}
 
 static int write_vmem_8bit(struct fbtft_par *par, size_t offset, size_t len)
 {
@@ -116,7 +117,7 @@ static int write_vmem_8bit(struct fbtft_par *par, size_t offset, size_t len)
 	for (i = start_line; i <= end_line; i++) {
 		pos[1] = cpu_to_be16(i);
 		for (j = 0; j < par->info->var.xres; j++) {
-			buf8[j] = RGB565toRGB332(*vmem16);
+			buf8[j] = rgb565_to_rgb332(*vmem16);
 			vmem16++;
 		}
 		ret = par->fbtftops.write(par,
-- 
2.7.4