* [PATCH] staging: fbtft: Avoid CamelCase.
@ 2019-03-05 6:00 Bhagyashri Dighole
2019-03-05 7:42 ` [Outreachy kernel] " Julia Lawall
2019-03-05 7:48 ` Greg Kroah-Hartman
0 siblings, 2 replies; 6+ messages in thread
From: Bhagyashri Dighole @ 2019-03-05 6:00 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: Outreachy, Bhagyashri
Fix coding style issues detected by checkpatch.pl `CHECK: Avoid
CamelCase`.
Signed-off-by: Bhagyashri Dighole <digholebhagyashri@gmail.com>
---
drivers/staging/fbtft/fb_watterott.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/fbtft/fb_watterott.c b/drivers/staging/fbtft/fb_watterott.c
index 0a5206d..7e9c57b 100644
--- a/drivers/staging/fbtft/fb_watterott.c
+++ b/drivers/staging/fbtft/fb_watterott.c
@@ -90,13 +90,13 @@ static int write_vmem(struct fbtft_par *par, size_t offset, size_t len)
return 0;
}
-#define RGB565toRGB323(c) ((((c) & 0xE000) >> 8) |\
+#define rgb565torgb323(c) ((((c) & 0xE000) >> 8) |\
(((c) & 000600) >> 6) |\
(((c) & 0x001C) >> 2))
-#define RGB565toRGB332(c) ((((c) & 0xE000) >> 8) |\
+#define rgb565torgb332(c) ((((c) & 0xE000) >> 8) |\
(((c) & 000700) >> 6) |\
(((c) & 0x0018) >> 3))
-#define RGB565toRGB233(c) ((((c) & 0xC000) >> 8) |\
+#define rgb565torgb233(c) ((((c) & 0xC000) >> 8) |\
(((c) & 000700) >> 5) |\
(((c) & 0x001C) >> 2))
@@ -122,7 +122,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] = rgb565torgb332(*vmem16);
vmem16++;
}
ret = par->fbtftops.write(par,
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Outreachy kernel] [PATCH] staging: fbtft: Avoid CamelCase.
2019-03-05 6:00 [PATCH] staging: fbtft: Avoid CamelCase Bhagyashri Dighole
@ 2019-03-05 7:42 ` Julia Lawall
2019-03-05 7:48 ` Greg Kroah-Hartman
1 sibling, 0 replies; 6+ messages in thread
From: Julia Lawall @ 2019-03-05 7:42 UTC (permalink / raw)
To: Bhagyashri Dighole; +Cc: Greg Kroah-Hartman, Outreachy
On Tue, 5 Mar 2019, Bhagyashri Dighole wrote:
> Fix coding style issues detected by checkpatch.pl `CHECK: Avoid
> CamelCase`.
>
> Signed-off-by: Bhagyashri Dighole <digholebhagyashri@gmail.com>
> ---
> drivers/staging/fbtft/fb_watterott.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/fbtft/fb_watterott.c b/drivers/staging/fbtft/fb_watterott.c
> index 0a5206d..7e9c57b 100644
> --- a/drivers/staging/fbtft/fb_watterott.c
> +++ b/drivers/staging/fbtft/fb_watterott.c
> @@ -90,13 +90,13 @@ static int write_vmem(struct fbtft_par *par, size_t offset, size_t len)
> return 0;
> }
>
> -#define RGB565toRGB323(c) ((((c) & 0xE000) >> 8) |\
> +#define rgb565torgb323(c) ((((c) & 0xE000) >> 8) |\
> (((c) & 000600) >> 6) |\
> (((c) & 0x001C) >> 2))
> -#define RGB565toRGB332(c) ((((c) & 0xE000) >> 8) |\
> +#define rgb565torgb332(c) ((((c) & 0xE000) >> 8) |\
> (((c) & 000700) >> 6) |\
> (((c) & 0x0018) >> 3))
> -#define RGB565toRGB233(c) ((((c) & 0xC000) >> 8) |\
> +#define rgb565torgb233(c) ((((c) & 0xC000) >> 8) |\
> (((c) & 000700) >> 5) |\
> (((c) & 0x001C) >> 2))
>
> @@ -122,7 +122,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] = rgb565torgb332(*vmem16);
Is this the only use of any of these macros?
Perhaps the others should be deleted. They have lots of magic numbers,
but I don't know if they actually encode any information about the device.
rgb565torgb332 is also a bit clunky. Consider rgb565_to_rgb332.
julia
> vmem16++;
> }
> ret = par->fbtftops.write(par,
> --
> 2.7.4
>
> --
> 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 visit https://groups.google.com/d/msgid/outreachy-kernel/20190305060005.GA7726%40bhagyashri-Lenovo-G570.
> For more options, visit https://groups.google.com/d/optout.
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] staging: fbtft: Avoid CamelCase.
2019-03-05 6:00 [PATCH] staging: fbtft: Avoid CamelCase Bhagyashri Dighole
2019-03-05 7:42 ` [Outreachy kernel] " Julia Lawall
@ 2019-03-05 7:48 ` Greg Kroah-Hartman
2019-03-05 9:42 ` Bhagyashri Dighole
1 sibling, 1 reply; 6+ messages in thread
From: Greg Kroah-Hartman @ 2019-03-05 7:48 UTC (permalink / raw)
To: Bhagyashri Dighole; +Cc: Outreachy
On Tue, Mar 05, 2019 at 11:30:05AM +0530, Bhagyashri Dighole wrote:
> Fix coding style issues detected by checkpatch.pl `CHECK: Avoid
> CamelCase`.
>
> Signed-off-by: Bhagyashri Dighole <digholebhagyashri@gmail.com>
> ---
> drivers/staging/fbtft/fb_watterott.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/fbtft/fb_watterott.c b/drivers/staging/fbtft/fb_watterott.c
> index 0a5206d..7e9c57b 100644
> --- a/drivers/staging/fbtft/fb_watterott.c
> +++ b/drivers/staging/fbtft/fb_watterott.c
> @@ -90,13 +90,13 @@ static int write_vmem(struct fbtft_par *par, size_t offset, size_t len)
> return 0;
> }
>
> -#define RGB565toRGB323(c) ((((c) & 0xE000) >> 8) |\
> +#define rgb565torgb323(c) ((((c) & 0xE000) >> 8) |\
> (((c) & 000600) >> 6) |\
> (((c) & 0x001C) >> 2))
> -#define RGB565toRGB332(c) ((((c) & 0xE000) >> 8) |\
> +#define rgb565torgb332(c) ((((c) & 0xE000) >> 8) |\
> (((c) & 000700) >> 6) |\
> (((c) & 0x0018) >> 3))
> -#define RGB565toRGB233(c) ((((c) & 0xC000) >> 8) |\
> +#define rgb565torgb233(c) ((((c) & 0xC000) >> 8) |\
> (((c) & 000700) >> 5) |\
> (((c) & 0x001C) >> 2))
>
> @@ -122,7 +122,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] = rgb565torgb332(*vmem16);
It looks like 2 of those #defines are not even used. Please make this a
patch series of two patches, the first one removing the unused #defines,
and the second changing the name.
But, this is a define, so all CAPS is normal for that.
How about changing this to an inline function instead? That would allow
you to name it rgb565_to_rgb332() which is much more understandable and
provide good typechecking.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] staging: fbtft: Avoid CamelCase.
2019-03-05 7:48 ` Greg Kroah-Hartman
@ 2019-03-05 9:42 ` Bhagyashri Dighole
2019-03-05 9:48 ` Greg Kroah-Hartman
2019-03-05 10:17 ` [Outreachy kernel] " Julia Lawall
0 siblings, 2 replies; 6+ messages in thread
From: Bhagyashri Dighole @ 2019-03-05 9:42 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: Outreachy
[-- Attachment #1: Type: text/plain, Size: 2448 bytes --]
Greg, I will create two different patches for above change.
I did not understand the context of `inline function` usage here , can you
more elaborate?
Thanks
On Tue, Mar 5, 2019 at 1:18 PM Greg Kroah-Hartman <
gregkh@linuxfoundation.org> wrote:
> On Tue, Mar 05, 2019 at 11:30:05AM +0530, Bhagyashri Dighole wrote:
> > Fix coding style issues detected by checkpatch.pl `CHECK: Avoid
> > CamelCase`.
> >
> > Signed-off-by: Bhagyashri Dighole <digholebhagyashri@gmail.com>
> > ---
> > drivers/staging/fbtft/fb_watterott.c | 8 ++++----
> > 1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/staging/fbtft/fb_watterott.c
> b/drivers/staging/fbtft/fb_watterott.c
> > index 0a5206d..7e9c57b 100644
> > --- a/drivers/staging/fbtft/fb_watterott.c
> > +++ b/drivers/staging/fbtft/fb_watterott.c
> > @@ -90,13 +90,13 @@ static int write_vmem(struct fbtft_par *par, size_t
> offset, size_t len)
> > return 0;
> > }
> >
> > -#define RGB565toRGB323(c) ((((c) & 0xE000) >> 8) |\
> > +#define rgb565torgb323(c) ((((c) & 0xE000) >> 8) |\
> > (((c) & 000600) >> 6) |\
> > (((c) & 0x001C) >> 2))
> > -#define RGB565toRGB332(c) ((((c) & 0xE000) >> 8) |\
> > +#define rgb565torgb332(c) ((((c) & 0xE000) >> 8) |\
> > (((c) & 000700) >> 6) |\
> > (((c) & 0x0018) >> 3))
> > -#define RGB565toRGB233(c) ((((c) & 0xC000) >> 8) |\
> > +#define rgb565torgb233(c) ((((c) & 0xC000) >> 8) |\
> > (((c) & 000700) >> 5) |\
> > (((c) & 0x001C) >> 2))
> >
> > @@ -122,7 +122,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] = rgb565torgb332(*vmem16);
>
> It looks like 2 of those #defines are not even used. Please make this a
> patch series of two patches, the first one removing the unused #defines,
> and the second changing the name.
>
> But, this is a define, so all CAPS is normal for that.
>
> How about changing this to an inline function instead? That would allow
> you to name it rgb565_to_rgb332() which is much more understandable and
> provide good typechecking.
>
> thanks,
>
> greg k-h
>
[-- Attachment #2: Type: text/html, Size: 3456 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] staging: fbtft: Avoid CamelCase.
2019-03-05 9:42 ` Bhagyashri Dighole
@ 2019-03-05 9:48 ` Greg Kroah-Hartman
2019-03-05 10:17 ` [Outreachy kernel] " Julia Lawall
1 sibling, 0 replies; 6+ messages in thread
From: Greg Kroah-Hartman @ 2019-03-05 9:48 UTC (permalink / raw)
To: Bhagyashri Dighole; +Cc: Outreachy
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?
A: No.
Q: Should I include quotations after my reply?
http://daringfireball.net/2007/07/on_top
On Tue, Mar 05, 2019 at 03:12:40PM +0530, Bhagyashri Dighole wrote:
> Greg, I will create two different patches for above change.
Nothing is "above" this line :)
>
> I did not understand the context of `inline function` usage here , can you
> more elaborate?
I have no context for this comment, sorry, I do not know what you mean
:(
greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Outreachy kernel] Re: [PATCH] staging: fbtft: Avoid CamelCase.
2019-03-05 9:42 ` Bhagyashri Dighole
2019-03-05 9:48 ` Greg Kroah-Hartman
@ 2019-03-05 10:17 ` Julia Lawall
1 sibling, 0 replies; 6+ messages in thread
From: Julia Lawall @ 2019-03-05 10:17 UTC (permalink / raw)
To: Bhagyashri Dighole; +Cc: Greg Kroah-Hartman, Outreachy
[-- Attachment #1: Type: text/plain, Size: 4591 bytes --]
On Tue, 5 Mar 2019, Bhagyashri Dighole wrote:
> Greg, I will create two different patches for above change.
>
> I did not understand the context of `inline function` usage here , can you
> more elaborate?
rgb565torgb233 is a thing that takes an argument and returns a value. It
is implemented as a macro, but it could be a function just as well. If
you put inline in front of the definition of the function, you should get
the same performance as with a macro.
Macros are mostly useful when the generated code doesn't really correspond
to C syntax or when the code has to work on values of different types.
But here what is generated is a complete expression, and since there is
only one call, there is only one possible argument type. So it should
really be a function. When you make it into a function, you can even make
the code simpler, by dropping the parentheses around c. Because there is
no danger that c will expand into something that has strange priorities.
In a function, a variable doesn't expand at all, it just remains as a
variable. So there is a big win all around for making it into a function.
julia
>
> Thanks
>
> On Tue, Mar 5, 2019 at 1:18 PM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> On Tue, Mar 05, 2019 at 11:30:05AM +0530, Bhagyashri Dighole
> wrote:
> > Fix coding style issues detected by checkpatch.pl `CHECK:
> Avoid
> > CamelCase`.
> >
> > Signed-off-by: Bhagyashri Dighole
> <digholebhagyashri@gmail.com>
> > ---
> >ￜ drivers/staging/fbtft/fb_watterott.c | 8 ++++----
> >ￜ 1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/staging/fbtft/fb_watterott.c
> b/drivers/staging/fbtft/fb_watterott.c
> > index 0a5206d..7e9c57b 100644
> > --- a/drivers/staging/fbtft/fb_watterott.c
> > +++ b/drivers/staging/fbtft/fb_watterott.c
> > @@ -90,13 +90,13 @@ static int write_vmem(struct fbtft_par
> *par, size_t offset, size_t len)
> >ￜ ￜ ￜ ￜreturn 0;
> >ￜ }
> >ￜ
> > -#define RGB565toRGB323(c) ((((c) & 0xE000) >> 8) |\
> > +#define rgb565torgb323(c) ((((c) & 0xE000) >> 8) |\
> >ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜ (((c) & 000600) >> 6) |\
> >ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜ (((c) & 0x001C) >> 2))
> > -#define RGB565toRGB332(c) ((((c) & 0xE000) >> 8) |\
> > +#define rgb565torgb332(c) ((((c) & 0xE000) >> 8) |\
> >ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜ (((c) & 000700) >> 6) |\
> >ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜ (((c) & 0x0018) >> 3))
> > -#define RGB565toRGB233(c) ((((c) & 0xC000) >> 8) |\
> > +#define rgb565torgb233(c) ((((c) & 0xC000) >> 8) |\
> >ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜ (((c) & 000700) >> 5) |\
> >ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜ (((c) & 0x001C) >> 2))
> >ￜ
> > @@ -122,7 +122,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] = rgb565torgb332(*vmem16);
>
> It looks like 2 of those #defines are not even used.ￜ Please
> make this a
> patch series of two patches, the first one removing the unused
> #defines,
> and the second changing the name.
>
> But, this is a define, so all CAPS is normal for that.
>
> How about changing this to an inline function instead?ￜ That
> would allow
> you to name it rgb565_to_rgb332() which is much more
> understandable and
> provide good typechecking.
>
> 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/CAOMPgjhUj9jUqmL33eqLaMK
> fPsO50csyOdr0xMMsnuKewtMiQg%40mail.gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-03-05 10:17 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-05 6:00 [PATCH] staging: fbtft: Avoid CamelCase Bhagyashri Dighole
2019-03-05 7:42 ` [Outreachy kernel] " Julia Lawall
2019-03-05 7:48 ` Greg Kroah-Hartman
2019-03-05 9:42 ` Bhagyashri Dighole
2019-03-05 9:48 ` Greg Kroah-Hartman
2019-03-05 10:17 ` [Outreachy kernel] " Julia Lawall
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.