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