Re: [PATCH v2] staging: sm750: Rename function to avoid CamelCase

Re: [PATCH v2] staging: sm750: Rename function to avoid CamelCase

[Kloudifold]

Hi Alison,

Thanks a lot for your help, so if I send my v3 patch, should it be like
below? (Philipp said "please make the Subject more unique", but the
Subject could be only a few permutations of very few words, I can't see
how it could be any more unique.)

Subject: [PATCH v2] staging: sm750: Rename functions from CamelCase to snake case

sm750 driver has sm750_hw_cursor_* functions, which are named in
camelcase. Rename them to snake case to follow the function naming
convention.

Changes in v3:
- Add this changelog (Philipp)
- Move lkp tags and link to the correct location in commit log (Alison)
- Update the commit msg (Philip)
- Update the commit log (Bagas, Julia)

Changes in v2:
- Use new function names in call sites (LKP)

Signed-off-by: Kloudifold <cloudifold.3125@gmail.com>
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303110849.X24WnHnM-lkp@intel.com/
---
git diff message
---

Re: [PATCH v2] staging: sm750: Rename function to avoid CamelCase

[Julia Lawall]

On Wed, 15 Mar 2023, Kloudifold wrote:

> Hi Alison,
>
> Thanks a lot for your help, so if I send my v3 patch, should it be like
> below? (Philipp said "please make the Subject more unique", but the
> Subject could be only a few permutations of very few words, I can't see
> how it could be any more unique.)
>
> Subject: [PATCH v2] staging: sm750: Rename functions from CamelCase to snake case
>
> sm750 driver has sm750_hw_cursor_* functions, which are named in
> camelcase. Rename them to snake case to follow the function naming
> convention.
>
> Changes in v3:
> - Add this changelog (Philipp)
> - Move lkp tags and link to the correct location in commit log (Alison)
> - Update the commit msg (Philip)
> - Update the commit log (Bagas, Julia)
>
> Changes in v2:
> - Use new function names in call sites (LKP)

All these changes should go below the --- line, not above it like this.
The history of changes is for people who are following along in the email
discussion.  Once the patch is accepted, they won't be meaningful.  So
they should be the in the part that disappears, which is the part below
the ---.

julia

>
> Signed-off-by: Kloudifold <cloudifold.3125@gmail.com>
> | Reported-by: kernel test robot <lkp@intel.com>
> | Link: https://lore.kernel.org/oe-kbuild-all/202303110849.X24WnHnM-lkp@intel.com/
> ---
> git diff message
> ---
>
>

Re: [PATCH v2] staging: sm750: Rename function to avoid CamelCase

[Deepak R Varma]

On Wed, Mar 15, 2023 at 11:35:37PM +0800, Kloudifold wrote:
> Hi Alison,
> 
> Thanks a lot for your help, so if I send my v3 patch, should it be like
> below? (Philipp said "please make the Subject more unique", but the
> Subject could be only a few permutations of very few words, I can't see
> how it could be any more unique.)
> 
> Subject: [PATCH v2] staging: sm750: Rename functions from CamelCase to snake case
> 
> sm750 driver has sm750_hw_cursor_* functions, which are named in
> camelcase. Rename them to snake case to follow the function naming
> convention.

You may want to review similar patches submitted in the past by the developers
and get hints on how it was named then. Use it to improve your understanding,
let your own thoughtfulness prevail.

https://lore.kernel.org/?q=&a=search+all+inboxes

Thanks,
Deepak.

> 
> Changes in v3:
> - Add this changelog (Philipp)
> - Move lkp tags and link to the correct location in commit log (Alison)
> - Update the commit msg (Philip)
> - Update the commit log (Bagas, Julia)
> 
> Changes in v2:
> - Use new function names in call sites (LKP)
> 
> Signed-off-by: Kloudifold <cloudifold.3125@gmail.com>
> | Reported-by: kernel test robot <lkp@intel.com>
> | Link: https://lore.kernel.org/oe-kbuild-all/202303110849.X24WnHnM-lkp@intel.com/
> ---
> git diff message
> ---
> 

Re: [PATCH v2] staging: sm750: Rename function to avoid CamelCase

[Bagas Sanjaya]

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

On Wed, Mar 15, 2023 at 11:35:37PM +0800, Kloudifold wrote:
> Subject: [PATCH v2] staging: sm750: Rename functions from CamelCase to snake case

If I only see the subject above, which functions are renamed? I have
to see the actual diff if the patch description doesn't answer that.
In this case, I think the proper subject would be `[PATCH v3] staging:
sm750: Rename sm750_hw_cursor_* functions to snake_case`.

> | Reported-by: kernel test robot <lkp@intel.com>
> | Link: https://lore.kernel.org/oe-kbuild-all/202303110849.X24WnHnM-lkp@intel.com/

kernel test robot (lkp) reported errors on your submitted patch, not
on commits already in the tree, so both Reported-by: and Link: trailers
above are not appropriate.

If you want to add trailers from someone else (like kernel test robot
above), you just need to write e.g. (ignore markdown):

```
Link: <link>
Reported-by: <identity>
Tested-by: <identity>
```

In other words, trim the preceding ``| ``.

See you in v3!

-- 
An old man doll... just what I always wanted! - Clara

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2] staging: sm750: Rename function to avoid CamelCase

[Ira Weiny]

Bagas Sanjaya wrote:
> On Wed, Mar 15, 2023 at 11:35:37PM +0800, Kloudifold wrote:
> > Subject: [PATCH v2] staging: sm750: Rename functions from CamelCase to snake case
> 
> If I only see the subject above, which functions are renamed? I have
> to see the actual diff if the patch description doesn't answer that.
> In this case, I think the proper subject would be `[PATCH v3] staging:
> sm750: Rename sm750_hw_cursor_* functions to snake_case`.

No.  The oneliner is a brief overview.

It is perfectly appropriate to have to look at the diff to see what was
done.  The commit message is to state what the problem was and how it was
fixed.

Ira

Re: [PATCH v2] staging: sm750: Rename function to avoid CamelCase

[Bagas Sanjaya]

On 3/18/23 12:14, Ira Weiny wrote:
> Bagas Sanjaya wrote:
>> On Wed, Mar 15, 2023 at 11:35:37PM +0800, Kloudifold wrote:
>>> Subject: [PATCH v2] staging: sm750: Rename functions from CamelCase to snake case
>>
>> If I only see the subject above, which functions are renamed? I have
>> to see the actual diff if the patch description doesn't answer that.
>> In this case, I think the proper subject would be `[PATCH v3] staging:
>> sm750: Rename sm750_hw_cursor_* functions to snake_case`.
> 
> No.  The oneliner is a brief overview.
> 
> It is perfectly appropriate to have to look at the diff to see what was
> done.  The commit message is to state what the problem was and how it was
> fixed.
> 

OK but I prefer to be more explicit in the subject and the description
so that I don't have to guess :)

-- 
An old man doll... just what I always wanted! - Clara

Re: [PATCH v2] staging: sm750: Rename function to avoid CamelCase

[Bagas Sanjaya]

On 3/15/23 11:54, Greg KH wrote:
>> I thought this style cleanup also warrants Fixes: since
>> it fixes coding style problem on existing code (see
>> Documentation/process/submitting-patches.rst).
> 
> No, sorry, Fixes: is for real bugs, not style issues.

OK, renouncing the tag.

-- 
An old man doll... just what I always wanted! - Clara

Re: [PATCH v2] staging: sm750: Rename function to avoid CamelCase

[Greg KH]

On Wed, Mar 15, 2023 at 10:08:27AM +0700, Bagas Sanjaya wrote:
> On 3/14/23 22:29, Alison Schofield wrote:
> >> Fixes: <commit that introduces these camelcase function>
> >> ```
> > 
> > Why are you asking for a Fixes tag on this? 
> > Seems unusual for a staging driver cleanup.
> > 
> 
> I thought this style cleanup also warrants Fixes: since
> it fixes coding style problem on existing code (see
> Documentation/process/submitting-patches.rst).

No, sorry, Fixes: is for real bugs, not style issues.

Re: [PATCH v2] staging: sm750: Rename function to avoid CamelCase

[Bagas Sanjaya]

On 3/14/23 22:29, Alison Schofield wrote:
>> Fixes: <commit that introduces these camelcase function>
>> ```
> 
> Why are you asking for a Fixes tag on this? 
> Seems unusual for a staging driver cleanup.
> 

I thought this style cleanup also warrants Fixes: since
it fixes coding style problem on existing code (see
Documentation/process/submitting-patches.rst).

-- 
An old man doll... just what I always wanted! - Clara

Re: [PATCH v2] staging: sm750: Rename function to avoid CamelCase

[Greg KH]

On Tue, Mar 14, 2023 at 09:04:53AM -0700, Alison Schofield wrote:
> On Tue, Mar 14, 2023 at 01:07:59AM +0100, Philipp Hortmann wrote:
> > On 3/13/23 21:43, Kloudifold wrote:
> > > The sm750 driver had some functions named in CamelCase,
> > > which violates the kernel's naming convention.
> > > In this patch, I rename these functions to snake case,
> > > which is the expected style.
> > 
> > Hi,
> > please make the Subject more unique. Many patches could have this name.
> > Look for examples in:
> > https://lore.kernel.org/linux-staging/
> > 
> > Please use your full name consisting of a given name and a family name in
> > front of your email. Or is this not possible and "Kloudifold" is both?
> 
> Kloudifold,
> 
> It'll probably take a bit for the change, allowing pseudonyms, to become
> common knowledge. When the question comes up, you can just point to the
> latest update to Documentation/process/submitting-patches.rst as I've
> done below.
> 
> Phillip,
> I commented similar to you, and then discovered this:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=d4563201f33a022fc0353033d9dfeb1606a88330
> 
> Pseudonyms now allowed.

"Known pseudonyms" are now allowed.  So that means that somehow I need
to "know" this is actually a pseudonym and that it is unique and so on.
As you are dealing with outreachy already, odds are you have to use a
non-pseudonym for them in order to track patches and payments and the
like, so please use the identifier you use with outreachy.

thanks,

greg k-h

Re: [PATCH v2] staging: sm750: Rename function to avoid CamelCase

[Alison Schofield]

On Tue, Mar 14, 2023 at 01:07:59AM +0100, Philipp Hortmann wrote:
> On 3/13/23 21:43, Kloudifold wrote:
> > The sm750 driver had some functions named in CamelCase,
> > which violates the kernel's naming convention.
> > In this patch, I rename these functions to snake case,
> > which is the expected style.
> 
> Hi,
> please make the Subject more unique. Many patches could have this name.
> Look for examples in:
> https://lore.kernel.org/linux-staging/
> 
> Please use your full name consisting of a given name and a family name in
> front of your email. Or is this not possible and "Kloudifold" is both?

Kloudifold,

It'll probably take a bit for the change, allowing pseudonyms, to become
common knowledge. When the question comes up, you can just point to the
latest update to Documentation/process/submitting-patches.rst as I've
done below.

Phillip,
I commented similar to you, and then discovered this:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=d4563201f33a022fc0353033d9dfeb1606a88330

Pseudonyms now allowed.

Alison

> > This v2 patch was prompted by an error reported by the
> > Linux test robot, which detected the compile error.
> > | Reported-by: kernel test robot <lkp@intel.com>
> > | Link: https://lore.kernel.org/oe-kbuild-all/202303110849.X24WnHnM-lkp@intel.com/
> > 
> > Signed-off-by: Kloudifold <cloudifold.3125@gmail.com>
> 
> The explanation what has changed needs to be below the ---
> but your Signed-off-by: is not allowed to be below this ---
> Look for examples of v2 in:
> https://lore.kernel.org/linux-staging/
> 
> Thanks
> 
> Bye Philipp
> 

Re: [PATCH v2] staging: sm750: Rename function to avoid CamelCase

[Alison Schofield]

On Tue, Mar 14, 2023 at 08:44:45AM -0700, Alison Schofield wrote:
> On Tue, Mar 14, 2023 at 04:43:20AM +0800, Kloudifold wrote:
> > The sm750 driver had some functions named in CamelCase,
> > which violates the kernel's naming convention.
> > In this patch, I rename these functions to snake case,
> > which is the expected style.
> > 
> > This v2 patch was prompted by an error reported by the
> > Linux test robot, which detected the compile error.
> > | Reported-by: kernel test robot <lkp@intel.com>
> > | Link: https://lore.kernel.org/oe-kbuild-all/202303110849.X24WnHnM-lkp@intel.com/
> 
> The tags from lkp go with your tag below - 
> 
>   Reported-by: kernel test robot <lkp@intel.com>
>   Link: https://lore.kernel.org/oe-kbuild-all/202303110849.X24WnHnM-lkp@intel.com/
> > Signed-off-by: Kloudifold <cloudifold.3125@gmail.com>
> > ---
> 
> Your next changelog, will look something like this:
> 
> Changes in v3:
> - Add this changelog (Philipp)
> - Move lkp tags and link to the correct location in commit log (Alison)
> - Update the commit msg (Philip)
> - Update the commit log (Bagas, Julia)
> 
> Changes in v2:
> - Use new function names in call sites (LKP)
> 
> 
> 
> Khadaji,
oops...sorry about that - Kloudifold,

> Did you see the compile errors?  What was the deal with that?
> Thanks,
> Alison
> 
> 
> >  drivers/staging/sm750fb/sm750.c        | 22 +++++++++++-----------
> >  drivers/staging/sm750fb/sm750_cursor.c | 14 +++++++-------
> >  drivers/staging/sm750fb/sm750_cursor.h | 12 ++++++------
> >  3 files changed, 24 insertions(+), 24 deletions(-)
> > 
> > diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
> > index effc7fcc3..9ddcd7b7d 100644
> > --- a/drivers/staging/sm750fb/sm750.c
> > +++ b/drivers/staging/sm750fb/sm750.c
> > @@ -121,14 +121,14 @@ static int lynxfb_ops_cursor(struct fb_info *info, struct fb_cursor *fbcursor)
> >  
> >  	sm750_hw_cursor_disable(cursor);
> >  	if (fbcursor->set & FB_CUR_SETSIZE)
> > -		sm750_hw_cursor_setSize(cursor,
> > -					fbcursor->image.width,
> > -					fbcursor->image.height);
> > +		sm750_hw_cursor_set_size(cursor,
> > +					 fbcursor->image.width,
> > +					 fbcursor->image.height);
> >  
> >  	if (fbcursor->set & FB_CUR_SETPOS)
> > -		sm750_hw_cursor_setPos(cursor,
> > -				       fbcursor->image.dx - info->var.xoffset,
> > -				       fbcursor->image.dy - info->var.yoffset);
> > +		sm750_hw_cursor_set_pos(cursor,
> > +				        fbcursor->image.dx - info->var.xoffset,
> > +				        fbcursor->image.dy - info->var.yoffset);
> >  
> >  	if (fbcursor->set & FB_CUR_SETCMAP) {
> >  		/* get the 16bit color of kernel means */
> > @@ -142,14 +142,14 @@ static int lynxfb_ops_cursor(struct fb_info *info, struct fb_cursor *fbcursor)
> >  		     ((info->cmap.green[fbcursor->image.bg_color] & 0xfc00) >> 5) |
> >  		     ((info->cmap.blue[fbcursor->image.bg_color] & 0xf800) >> 11);
> >  
> > -		sm750_hw_cursor_setColor(cursor, fg, bg);
> > +		sm750_hw_cursor_set_color(cursor, fg, bg);
> >  	}
> >  
> >  	if (fbcursor->set & (FB_CUR_SETSHAPE | FB_CUR_SETIMAGE)) {
> > -		sm750_hw_cursor_setData(cursor,
> > -					fbcursor->rop,
> > -					fbcursor->image.data,
> > -					fbcursor->mask);
> > +		sm750_hw_cursor_set_data(cursor,
> > +					 fbcursor->rop,
> > +					 fbcursor->image.data,
> > +					 fbcursor->mask);
> >  	}
> >  
> >  	if (fbcursor->enable)
> > diff --git a/drivers/staging/sm750fb/sm750_cursor.c b/drivers/staging/sm750fb/sm750_cursor.c
> > index 43e6f52c2..ff643e33f 100644
> > --- a/drivers/staging/sm750fb/sm750_cursor.c
> > +++ b/drivers/staging/sm750fb/sm750_cursor.c
> > @@ -58,13 +58,13 @@ void sm750_hw_cursor_disable(struct lynx_cursor *cursor)
> >  	poke32(HWC_ADDRESS, 0);
> >  }
> >  
> > -void sm750_hw_cursor_setSize(struct lynx_cursor *cursor, int w, int h)
> > +void sm750_hw_cursor_set_size(struct lynx_cursor *cursor, int w, int h)
> >  {
> >  	cursor->w = w;
> >  	cursor->h = h;
> >  }
> >  
> > -void sm750_hw_cursor_setPos(struct lynx_cursor *cursor, int x, int y)
> > +void sm750_hw_cursor_set_pos(struct lynx_cursor *cursor, int x, int y)
> >  {
> >  	u32 reg;
> >  
> > @@ -73,7 +73,7 @@ void sm750_hw_cursor_setPos(struct lynx_cursor *cursor, int x, int y)
> >  	poke32(HWC_LOCATION, reg);
> >  }
> >  
> > -void sm750_hw_cursor_setColor(struct lynx_cursor *cursor, u32 fg, u32 bg)
> > +void sm750_hw_cursor_set_color(struct lynx_cursor *cursor, u32 fg, u32 bg)
> >  {
> >  	u32 reg = (fg << HWC_COLOR_12_2_RGB565_SHIFT) &
> >  		HWC_COLOR_12_2_RGB565_MASK;
> > @@ -82,8 +82,8 @@ void sm750_hw_cursor_setColor(struct lynx_cursor *cursor, u32 fg, u32 bg)
> >  	poke32(HWC_COLOR_3, 0xffe0);
> >  }
> >  
> > -void sm750_hw_cursor_setData(struct lynx_cursor *cursor, u16 rop,
> > -			     const u8 *pcol, const u8 *pmsk)
> > +void sm750_hw_cursor_set_data(struct lynx_cursor *cursor, u16 rop,
> > +			      const u8 *pcol, const u8 *pmsk)
> >  {
> >  	int i, j, count, pitch, offset;
> >  	u8 color, mask, opr;
> > @@ -132,8 +132,8 @@ void sm750_hw_cursor_setData(struct lynx_cursor *cursor, u16 rop,
> >  	}
> >  }
> >  
> > -void sm750_hw_cursor_setData2(struct lynx_cursor *cursor, u16 rop,
> > -			      const u8 *pcol, const u8 *pmsk)
> > +void sm750_hw_cursor_set_data2(struct lynx_cursor *cursor, u16 rop,
> > +			       const u8 *pcol, const u8 *pmsk)
> >  {
> >  	int i, j, count, pitch, offset;
> >  	u8 color, mask;
> > diff --git a/drivers/staging/sm750fb/sm750_cursor.h b/drivers/staging/sm750fb/sm750_cursor.h
> > index b59643dd6..88fa02f63 100644
> > --- a/drivers/staging/sm750fb/sm750_cursor.h
> > +++ b/drivers/staging/sm750fb/sm750_cursor.h
> > @@ -5,11 +5,11 @@
> >  /* hw_cursor_xxx works for voyager,718 and 750 */
> >  void sm750_hw_cursor_enable(struct lynx_cursor *cursor);
> >  void sm750_hw_cursor_disable(struct lynx_cursor *cursor);
> > -void sm750_hw_cursor_setSize(struct lynx_cursor *cursor, int w, int h);
> > -void sm750_hw_cursor_setPos(struct lynx_cursor *cursor, int x, int y);
> > -void sm750_hw_cursor_setColor(struct lynx_cursor *cursor, u32 fg, u32 bg);
> > -void sm750_hw_cursor_setData(struct lynx_cursor *cursor, u16 rop,
> > -			     const u8 *data, const u8 *mask);
> > -void sm750_hw_cursor_setData2(struct lynx_cursor *cursor, u16 rop,
> > +void sm750_hw_cursor_set_size(struct lynx_cursor *cursor, int w, int h);
> > +void sm750_hw_cursor_set_pos(struct lynx_cursor *cursor, int x, int y);
> > +void sm750_hw_cursor_set_color(struct lynx_cursor *cursor, u32 fg, u32 bg);
> > +void sm750_hw_cursor_set_data(struct lynx_cursor *cursor, u16 rop,
> >  			      const u8 *data, const u8 *mask);
> > +void sm750_hw_cursor_set_data2(struct lynx_cursor *cursor, u16 rop,
> > +			       const u8 *data, const u8 *mask);
> >  #endif
> > -- 
> > 2.39.2
> > 
> > 
> 

Re: [PATCH v2] staging: sm750: Rename function to avoid CamelCase

[Alison Schofield]

On Tue, Mar 14, 2023 at 04:43:20AM +0800, Kloudifold wrote:
> The sm750 driver had some functions named in CamelCase,
> which violates the kernel's naming convention.
> In this patch, I rename these functions to snake case,
> which is the expected style.
> 
> This v2 patch was prompted by an error reported by the
> Linux test robot, which detected the compile error.
> | Reported-by: kernel test robot <lkp@intel.com>
> | Link: https://lore.kernel.org/oe-kbuild-all/202303110849.X24WnHnM-lkp@intel.com/

The tags from lkp go with your tag below - 

  Reported-by: kernel test robot <lkp@intel.com>
  Link: https://lore.kernel.org/oe-kbuild-all/202303110849.X24WnHnM-lkp@intel.com/
> Signed-off-by: Kloudifold <cloudifold.3125@gmail.com>
> ---

Your next changelog, will look something like this:

Changes in v3:
- Add this changelog (Philipp)
- Move lkp tags and link to the correct location in commit log (Alison)
- Update the commit msg (Philip)
- Update the commit log (Bagas, Julia)

Changes in v2:
- Use new function names in call sites (LKP)



Khadaji,
Did you see the compile errors?  What was the deal with that?
Thanks,
Alison


>  drivers/staging/sm750fb/sm750.c        | 22 +++++++++++-----------
>  drivers/staging/sm750fb/sm750_cursor.c | 14 +++++++-------
>  drivers/staging/sm750fb/sm750_cursor.h | 12 ++++++------
>  3 files changed, 24 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
> index effc7fcc3..9ddcd7b7d 100644
> --- a/drivers/staging/sm750fb/sm750.c
> +++ b/drivers/staging/sm750fb/sm750.c
> @@ -121,14 +121,14 @@ static int lynxfb_ops_cursor(struct fb_info *info, struct fb_cursor *fbcursor)
>  
>  	sm750_hw_cursor_disable(cursor);
>  	if (fbcursor->set & FB_CUR_SETSIZE)
> -		sm750_hw_cursor_setSize(cursor,
> -					fbcursor->image.width,
> -					fbcursor->image.height);
> +		sm750_hw_cursor_set_size(cursor,
> +					 fbcursor->image.width,
> +					 fbcursor->image.height);
>  
>  	if (fbcursor->set & FB_CUR_SETPOS)
> -		sm750_hw_cursor_setPos(cursor,
> -				       fbcursor->image.dx - info->var.xoffset,
> -				       fbcursor->image.dy - info->var.yoffset);
> +		sm750_hw_cursor_set_pos(cursor,
> +				        fbcursor->image.dx - info->var.xoffset,
> +				        fbcursor->image.dy - info->var.yoffset);
>  
>  	if (fbcursor->set & FB_CUR_SETCMAP) {
>  		/* get the 16bit color of kernel means */
> @@ -142,14 +142,14 @@ static int lynxfb_ops_cursor(struct fb_info *info, struct fb_cursor *fbcursor)
>  		     ((info->cmap.green[fbcursor->image.bg_color] & 0xfc00) >> 5) |
>  		     ((info->cmap.blue[fbcursor->image.bg_color] & 0xf800) >> 11);
>  
> -		sm750_hw_cursor_setColor(cursor, fg, bg);
> +		sm750_hw_cursor_set_color(cursor, fg, bg);
>  	}
>  
>  	if (fbcursor->set & (FB_CUR_SETSHAPE | FB_CUR_SETIMAGE)) {
> -		sm750_hw_cursor_setData(cursor,
> -					fbcursor->rop,
> -					fbcursor->image.data,
> -					fbcursor->mask);
> +		sm750_hw_cursor_set_data(cursor,
> +					 fbcursor->rop,
> +					 fbcursor->image.data,
> +					 fbcursor->mask);
>  	}
>  
>  	if (fbcursor->enable)
> diff --git a/drivers/staging/sm750fb/sm750_cursor.c b/drivers/staging/sm750fb/sm750_cursor.c
> index 43e6f52c2..ff643e33f 100644
> --- a/drivers/staging/sm750fb/sm750_cursor.c
> +++ b/drivers/staging/sm750fb/sm750_cursor.c
> @@ -58,13 +58,13 @@ void sm750_hw_cursor_disable(struct lynx_cursor *cursor)
>  	poke32(HWC_ADDRESS, 0);
>  }
>  
> -void sm750_hw_cursor_setSize(struct lynx_cursor *cursor, int w, int h)
> +void sm750_hw_cursor_set_size(struct lynx_cursor *cursor, int w, int h)
>  {
>  	cursor->w = w;
>  	cursor->h = h;
>  }
>  
> -void sm750_hw_cursor_setPos(struct lynx_cursor *cursor, int x, int y)
> +void sm750_hw_cursor_set_pos(struct lynx_cursor *cursor, int x, int y)
>  {
>  	u32 reg;
>  
> @@ -73,7 +73,7 @@ void sm750_hw_cursor_setPos(struct lynx_cursor *cursor, int x, int y)
>  	poke32(HWC_LOCATION, reg);
>  }
>  
> -void sm750_hw_cursor_setColor(struct lynx_cursor *cursor, u32 fg, u32 bg)
> +void sm750_hw_cursor_set_color(struct lynx_cursor *cursor, u32 fg, u32 bg)
>  {
>  	u32 reg = (fg << HWC_COLOR_12_2_RGB565_SHIFT) &
>  		HWC_COLOR_12_2_RGB565_MASK;
> @@ -82,8 +82,8 @@ void sm750_hw_cursor_setColor(struct lynx_cursor *cursor, u32 fg, u32 bg)
>  	poke32(HWC_COLOR_3, 0xffe0);
>  }
>  
> -void sm750_hw_cursor_setData(struct lynx_cursor *cursor, u16 rop,
> -			     const u8 *pcol, const u8 *pmsk)
> +void sm750_hw_cursor_set_data(struct lynx_cursor *cursor, u16 rop,
> +			      const u8 *pcol, const u8 *pmsk)
>  {
>  	int i, j, count, pitch, offset;
>  	u8 color, mask, opr;
> @@ -132,8 +132,8 @@ void sm750_hw_cursor_setData(struct lynx_cursor *cursor, u16 rop,
>  	}
>  }
>  
> -void sm750_hw_cursor_setData2(struct lynx_cursor *cursor, u16 rop,
> -			      const u8 *pcol, const u8 *pmsk)
> +void sm750_hw_cursor_set_data2(struct lynx_cursor *cursor, u16 rop,
> +			       const u8 *pcol, const u8 *pmsk)
>  {
>  	int i, j, count, pitch, offset;
>  	u8 color, mask;
> diff --git a/drivers/staging/sm750fb/sm750_cursor.h b/drivers/staging/sm750fb/sm750_cursor.h
> index b59643dd6..88fa02f63 100644
> --- a/drivers/staging/sm750fb/sm750_cursor.h
> +++ b/drivers/staging/sm750fb/sm750_cursor.h
> @@ -5,11 +5,11 @@
>  /* hw_cursor_xxx works for voyager,718 and 750 */
>  void sm750_hw_cursor_enable(struct lynx_cursor *cursor);
>  void sm750_hw_cursor_disable(struct lynx_cursor *cursor);
> -void sm750_hw_cursor_setSize(struct lynx_cursor *cursor, int w, int h);
> -void sm750_hw_cursor_setPos(struct lynx_cursor *cursor, int x, int y);
> -void sm750_hw_cursor_setColor(struct lynx_cursor *cursor, u32 fg, u32 bg);
> -void sm750_hw_cursor_setData(struct lynx_cursor *cursor, u16 rop,
> -			     const u8 *data, const u8 *mask);
> -void sm750_hw_cursor_setData2(struct lynx_cursor *cursor, u16 rop,
> +void sm750_hw_cursor_set_size(struct lynx_cursor *cursor, int w, int h);
> +void sm750_hw_cursor_set_pos(struct lynx_cursor *cursor, int x, int y);
> +void sm750_hw_cursor_set_color(struct lynx_cursor *cursor, u32 fg, u32 bg);
> +void sm750_hw_cursor_set_data(struct lynx_cursor *cursor, u16 rop,
>  			      const u8 *data, const u8 *mask);
> +void sm750_hw_cursor_set_data2(struct lynx_cursor *cursor, u16 rop,
> +			       const u8 *data, const u8 *mask);
>  #endif
> -- 
> 2.39.2
> 
> 

Re: [PATCH v2] staging: sm750: Rename function to avoid CamelCase

[Alison Schofield]

On Tue, Mar 14, 2023 at 11:55:02AM +0700, Bagas Sanjaya wrote:
> On Tue, Mar 14, 2023 at 04:43:20AM +0800, Kloudifold wrote:
> > The sm750 driver had some functions named in CamelCase,
> > which violates the kernel's naming convention.
> > In this patch, I rename these functions to snake case,
> > which is the expected style.
> > 
> > This v2 patch was prompted by an error reported by the
> > Linux test robot, which detected the compile error.
> > | Reported-by: kernel test robot <lkp@intel.com>
> > | Link: https://lore.kernel.org/oe-kbuild-all/202303110849.X24WnHnM-lkp@intel.com/
> 
> Hi,
> 
> I'd like to write the patch description as (ignore markdown):
> 
> ```
> sm750 driver has sm750_hw_cursor_* functions, which are named in
> camelcase. Rename them to snake case to follow the function naming
> convention.
> 
> Fixes: <commit that introduces these camelcase function>
> ```

Why are you asking for a Fixes tag on this? 
Seems unusual for a staging driver cleanup.

Alison


> 
> Thanks.
> 
> -- 
> An old man doll... just what I always wanted! - Clara

Re: [PATCH v2] staging: sm750: Rename function to avoid CamelCase

[Bagas Sanjaya]

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

On Tue, Mar 14, 2023 at 04:43:20AM +0800, Kloudifold wrote:
> The sm750 driver had some functions named in CamelCase,
> which violates the kernel's naming convention.
> In this patch, I rename these functions to snake case,
> which is the expected style.
> 
> This v2 patch was prompted by an error reported by the
> Linux test robot, which detected the compile error.
> | Reported-by: kernel test robot <lkp@intel.com>
> | Link: https://lore.kernel.org/oe-kbuild-all/202303110849.X24WnHnM-lkp@intel.com/

Hi,

I'd like to write the patch description as (ignore markdown):

```
sm750 driver has sm750_hw_cursor_* functions, which are named in
camelcase. Rename them to snake case to follow the function naming
convention.

Fixes: <commit that introduces these camelcase function>
```

Thanks.

-- 
An old man doll... just what I always wanted! - Clara

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2] staging: sm750: Rename function to avoid CamelCase

[Philipp Hortmann]

On 3/13/23 21:43, Kloudifold wrote:
> The sm750 driver had some functions named in CamelCase,
> which violates the kernel's naming convention.
> In this patch, I rename these functions to snake case,
> which is the expected style.

Hi,
please make the Subject more unique. Many patches could have this name.
Look for examples in:
https://lore.kernel.org/linux-staging/

Please use your full name consisting of a given name and a family name 
in front of your email. Or is this not possible and "Kloudifold" is both?

> This v2 patch was prompted by an error reported by the
> Linux test robot, which detected the compile error.
> | Reported-by: kernel test robot <lkp@intel.com>
> | Link: https://lore.kernel.org/oe-kbuild-all/202303110849.X24WnHnM-lkp@intel.com/
> 
> Signed-off-by: Kloudifold <cloudifold.3125@gmail.com>

The explanation what has changed needs to be below the ---
but your Signed-off-by: is not allowed to be below this ---
Look for examples of v2 in:
https://lore.kernel.org/linux-staging/

Thanks

Bye Philipp

Re: [PATCH v2] staging: sm750: Rename function to avoid CamelCase

[Julia Lawall]

On Tue, 14 Mar 2023, Kloudifold wrote:

> The sm750 driver had some functions named in CamelCase,
> which violates the kernel's naming convention.
> In this patch, I rename these functions to snake case,
> which is the expected style.

Please go back and read the Outreachy tutorial, specifically the Patch
Philosophy page:

https://kernelnewbies.org/PatchPhilosophy

The log message should be in the imperative.  You should try to avoid the
use of "I".  "In this patch" is not necessary.  Mentioning the name of the
driver is not necessary, since it appears in the subject line and in the
list of changed files below.

> This v2 patch was prompted by an error reported by the
> Linux test robot, which detected the compile error.

This is not easy to understand.  What do you mean by a compile error.
Normally the use or non use of Camel case would be a coding style
violation, but would not provoke a compiler error.

julia

> | Reported-by: kernel test robot <lkp@intel.com>
> | Link: https://lore.kernel.org/oe-kbuild-all/202303110849.X24WnHnM-lkp@intel.com/
>
> Signed-off-by: Kloudifold <cloudifold.3125@gmail.com>
> ---
>  drivers/staging/sm750fb/sm750.c        | 22 +++++++++++-----------
>  drivers/staging/sm750fb/sm750_cursor.c | 14 +++++++-------
>  drivers/staging/sm750fb/sm750_cursor.h | 12 ++++++------
>  3 files changed, 24 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
> index effc7fcc3..9ddcd7b7d 100644
> --- a/drivers/staging/sm750fb/sm750.c
> +++ b/drivers/staging/sm750fb/sm750.c
> @@ -121,14 +121,14 @@ static int lynxfb_ops_cursor(struct fb_info *info, struct fb_cursor *fbcursor)
>
>  	sm750_hw_cursor_disable(cursor);
>  	if (fbcursor->set & FB_CUR_SETSIZE)
> -		sm750_hw_cursor_setSize(cursor,
> -					fbcursor->image.width,
> -					fbcursor->image.height);
> +		sm750_hw_cursor_set_size(cursor,
> +					 fbcursor->image.width,
> +					 fbcursor->image.height);
>
>  	if (fbcursor->set & FB_CUR_SETPOS)
> -		sm750_hw_cursor_setPos(cursor,
> -				       fbcursor->image.dx - info->var.xoffset,
> -				       fbcursor->image.dy - info->var.yoffset);
> +		sm750_hw_cursor_set_pos(cursor,
> +				        fbcursor->image.dx - info->var.xoffset,
> +				        fbcursor->image.dy - info->var.yoffset);
>
>  	if (fbcursor->set & FB_CUR_SETCMAP) {
>  		/* get the 16bit color of kernel means */
> @@ -142,14 +142,14 @@ static int lynxfb_ops_cursor(struct fb_info *info, struct fb_cursor *fbcursor)
>  		     ((info->cmap.green[fbcursor->image.bg_color] & 0xfc00) >> 5) |
>  		     ((info->cmap.blue[fbcursor->image.bg_color] & 0xf800) >> 11);
>
> -		sm750_hw_cursor_setColor(cursor, fg, bg);
> +		sm750_hw_cursor_set_color(cursor, fg, bg);
>  	}
>
>  	if (fbcursor->set & (FB_CUR_SETSHAPE | FB_CUR_SETIMAGE)) {
> -		sm750_hw_cursor_setData(cursor,
> -					fbcursor->rop,
> -					fbcursor->image.data,
> -					fbcursor->mask);
> +		sm750_hw_cursor_set_data(cursor,
> +					 fbcursor->rop,
> +					 fbcursor->image.data,
> +					 fbcursor->mask);
>  	}
>
>  	if (fbcursor->enable)
> diff --git a/drivers/staging/sm750fb/sm750_cursor.c b/drivers/staging/sm750fb/sm750_cursor.c
> index 43e6f52c2..ff643e33f 100644
> --- a/drivers/staging/sm750fb/sm750_cursor.c
> +++ b/drivers/staging/sm750fb/sm750_cursor.c
> @@ -58,13 +58,13 @@ void sm750_hw_cursor_disable(struct lynx_cursor *cursor)
>  	poke32(HWC_ADDRESS, 0);
>  }
>
> -void sm750_hw_cursor_setSize(struct lynx_cursor *cursor, int w, int h)
> +void sm750_hw_cursor_set_size(struct lynx_cursor *cursor, int w, int h)
>  {
>  	cursor->w = w;
>  	cursor->h = h;
>  }
>
> -void sm750_hw_cursor_setPos(struct lynx_cursor *cursor, int x, int y)
> +void sm750_hw_cursor_set_pos(struct lynx_cursor *cursor, int x, int y)
>  {
>  	u32 reg;
>
> @@ -73,7 +73,7 @@ void sm750_hw_cursor_setPos(struct lynx_cursor *cursor, int x, int y)
>  	poke32(HWC_LOCATION, reg);
>  }
>
> -void sm750_hw_cursor_setColor(struct lynx_cursor *cursor, u32 fg, u32 bg)
> +void sm750_hw_cursor_set_color(struct lynx_cursor *cursor, u32 fg, u32 bg)
>  {
>  	u32 reg = (fg << HWC_COLOR_12_2_RGB565_SHIFT) &
>  		HWC_COLOR_12_2_RGB565_MASK;
> @@ -82,8 +82,8 @@ void sm750_hw_cursor_setColor(struct lynx_cursor *cursor, u32 fg, u32 bg)
>  	poke32(HWC_COLOR_3, 0xffe0);
>  }
>
> -void sm750_hw_cursor_setData(struct lynx_cursor *cursor, u16 rop,
> -			     const u8 *pcol, const u8 *pmsk)
> +void sm750_hw_cursor_set_data(struct lynx_cursor *cursor, u16 rop,
> +			      const u8 *pcol, const u8 *pmsk)
>  {
>  	int i, j, count, pitch, offset;
>  	u8 color, mask, opr;
> @@ -132,8 +132,8 @@ void sm750_hw_cursor_setData(struct lynx_cursor *cursor, u16 rop,
>  	}
>  }
>
> -void sm750_hw_cursor_setData2(struct lynx_cursor *cursor, u16 rop,
> -			      const u8 *pcol, const u8 *pmsk)
> +void sm750_hw_cursor_set_data2(struct lynx_cursor *cursor, u16 rop,
> +			       const u8 *pcol, const u8 *pmsk)
>  {
>  	int i, j, count, pitch, offset;
>  	u8 color, mask;
> diff --git a/drivers/staging/sm750fb/sm750_cursor.h b/drivers/staging/sm750fb/sm750_cursor.h
> index b59643dd6..88fa02f63 100644
> --- a/drivers/staging/sm750fb/sm750_cursor.h
> +++ b/drivers/staging/sm750fb/sm750_cursor.h
> @@ -5,11 +5,11 @@
>  /* hw_cursor_xxx works for voyager,718 and 750 */
>  void sm750_hw_cursor_enable(struct lynx_cursor *cursor);
>  void sm750_hw_cursor_disable(struct lynx_cursor *cursor);
> -void sm750_hw_cursor_setSize(struct lynx_cursor *cursor, int w, int h);
> -void sm750_hw_cursor_setPos(struct lynx_cursor *cursor, int x, int y);
> -void sm750_hw_cursor_setColor(struct lynx_cursor *cursor, u32 fg, u32 bg);
> -void sm750_hw_cursor_setData(struct lynx_cursor *cursor, u16 rop,
> -			     const u8 *data, const u8 *mask);
> -void sm750_hw_cursor_setData2(struct lynx_cursor *cursor, u16 rop,
> +void sm750_hw_cursor_set_size(struct lynx_cursor *cursor, int w, int h);
> +void sm750_hw_cursor_set_pos(struct lynx_cursor *cursor, int x, int y);
> +void sm750_hw_cursor_set_color(struct lynx_cursor *cursor, u32 fg, u32 bg);
> +void sm750_hw_cursor_set_data(struct lynx_cursor *cursor, u16 rop,
>  			      const u8 *data, const u8 *mask);
> +void sm750_hw_cursor_set_data2(struct lynx_cursor *cursor, u16 rop,
> +			       const u8 *data, const u8 *mask);
>  #endif
> --
> 2.39.2
>
>
>

[PATCH v2] staging: sm750: Rename function to avoid CamelCase

[Kloudifold]

The sm750 driver had some functions named in CamelCase,
which violates the kernel's naming convention.
In this patch, I rename these functions to snake case,
which is the expected style.

This v2 patch was prompted by an error reported by the
Linux test robot, which detected the compile error.
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303110849.X24WnHnM-lkp@intel.com/

Signed-off-by: Kloudifold <cloudifold.3125@gmail.com>
---
 drivers/staging/sm750fb/sm750.c        | 22 +++++++++++-----------
 drivers/staging/sm750fb/sm750_cursor.c | 14 +++++++-------
 drivers/staging/sm750fb/sm750_cursor.h | 12 ++++++------
 3 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
index effc7fcc3..9ddcd7b7d 100644
--- a/drivers/staging/sm750fb/sm750.c
+++ b/drivers/staging/sm750fb/sm750.c
@@ -121,14 +121,14 @@ static int lynxfb_ops_cursor(struct fb_info *info, struct fb_cursor *fbcursor)
 
 	sm750_hw_cursor_disable(cursor);
 	if (fbcursor->set & FB_CUR_SETSIZE)
-		sm750_hw_cursor_setSize(cursor,
-					fbcursor->image.width,
-					fbcursor->image.height);
+		sm750_hw_cursor_set_size(cursor,
+					 fbcursor->image.width,
+					 fbcursor->image.height);
 
 	if (fbcursor->set & FB_CUR_SETPOS)
-		sm750_hw_cursor_setPos(cursor,
-				       fbcursor->image.dx - info->var.xoffset,
-				       fbcursor->image.dy - info->var.yoffset);
+		sm750_hw_cursor_set_pos(cursor,
+				        fbcursor->image.dx - info->var.xoffset,
+				        fbcursor->image.dy - info->var.yoffset);
 
 	if (fbcursor->set & FB_CUR_SETCMAP) {
 		/* get the 16bit color of kernel means */
@@ -142,14 +142,14 @@ static int lynxfb_ops_cursor(struct fb_info *info, struct fb_cursor *fbcursor)
 		     ((info->cmap.green[fbcursor->image.bg_color] & 0xfc00) >> 5) |
 		     ((info->cmap.blue[fbcursor->image.bg_color] & 0xf800) >> 11);
 
-		sm750_hw_cursor_setColor(cursor, fg, bg);
+		sm750_hw_cursor_set_color(cursor, fg, bg);
 	}
 
 	if (fbcursor->set & (FB_CUR_SETSHAPE | FB_CUR_SETIMAGE)) {
-		sm750_hw_cursor_setData(cursor,
-					fbcursor->rop,
-					fbcursor->image.data,
-					fbcursor->mask);
+		sm750_hw_cursor_set_data(cursor,
+					 fbcursor->rop,
+					 fbcursor->image.data,
+					 fbcursor->mask);
 	}
 
 	if (fbcursor->enable)
diff --git a/drivers/staging/sm750fb/sm750_cursor.c b/drivers/staging/sm750fb/sm750_cursor.c
index 43e6f52c2..ff643e33f 100644
--- a/drivers/staging/sm750fb/sm750_cursor.c
+++ b/drivers/staging/sm750fb/sm750_cursor.c
@@ -58,13 +58,13 @@ void sm750_hw_cursor_disable(struct lynx_cursor *cursor)
 	poke32(HWC_ADDRESS, 0);
 }
 
-void sm750_hw_cursor_setSize(struct lynx_cursor *cursor, int w, int h)
+void sm750_hw_cursor_set_size(struct lynx_cursor *cursor, int w, int h)
 {
 	cursor->w = w;
 	cursor->h = h;
 }
 
-void sm750_hw_cursor_setPos(struct lynx_cursor *cursor, int x, int y)
+void sm750_hw_cursor_set_pos(struct lynx_cursor *cursor, int x, int y)
 {
 	u32 reg;
 
@@ -73,7 +73,7 @@ void sm750_hw_cursor_setPos(struct lynx_cursor *cursor, int x, int y)
 	poke32(HWC_LOCATION, reg);
 }
 
-void sm750_hw_cursor_setColor(struct lynx_cursor *cursor, u32 fg, u32 bg)
+void sm750_hw_cursor_set_color(struct lynx_cursor *cursor, u32 fg, u32 bg)
 {
 	u32 reg = (fg << HWC_COLOR_12_2_RGB565_SHIFT) &
 		HWC_COLOR_12_2_RGB565_MASK;
@@ -82,8 +82,8 @@ void sm750_hw_cursor_setColor(struct lynx_cursor *cursor, u32 fg, u32 bg)
 	poke32(HWC_COLOR_3, 0xffe0);
 }
 
-void sm750_hw_cursor_setData(struct lynx_cursor *cursor, u16 rop,
-			     const u8 *pcol, const u8 *pmsk)
+void sm750_hw_cursor_set_data(struct lynx_cursor *cursor, u16 rop,
+			      const u8 *pcol, const u8 *pmsk)
 {
 	int i, j, count, pitch, offset;
 	u8 color, mask, opr;
@@ -132,8 +132,8 @@ void sm750_hw_cursor_setData(struct lynx_cursor *cursor, u16 rop,
 	}
 }
 
-void sm750_hw_cursor_setData2(struct lynx_cursor *cursor, u16 rop,
-			      const u8 *pcol, const u8 *pmsk)
+void sm750_hw_cursor_set_data2(struct lynx_cursor *cursor, u16 rop,
+			       const u8 *pcol, const u8 *pmsk)
 {
 	int i, j, count, pitch, offset;
 	u8 color, mask;
diff --git a/drivers/staging/sm750fb/sm750_cursor.h b/drivers/staging/sm750fb/sm750_cursor.h
index b59643dd6..88fa02f63 100644
--- a/drivers/staging/sm750fb/sm750_cursor.h
+++ b/drivers/staging/sm750fb/sm750_cursor.h
@@ -5,11 +5,11 @@
 /* hw_cursor_xxx works for voyager,718 and 750 */
 void sm750_hw_cursor_enable(struct lynx_cursor *cursor);
 void sm750_hw_cursor_disable(struct lynx_cursor *cursor);
-void sm750_hw_cursor_setSize(struct lynx_cursor *cursor, int w, int h);
-void sm750_hw_cursor_setPos(struct lynx_cursor *cursor, int x, int y);
-void sm750_hw_cursor_setColor(struct lynx_cursor *cursor, u32 fg, u32 bg);
-void sm750_hw_cursor_setData(struct lynx_cursor *cursor, u16 rop,
-			     const u8 *data, const u8 *mask);
-void sm750_hw_cursor_setData2(struct lynx_cursor *cursor, u16 rop,
+void sm750_hw_cursor_set_size(struct lynx_cursor *cursor, int w, int h);
+void sm750_hw_cursor_set_pos(struct lynx_cursor *cursor, int x, int y);
+void sm750_hw_cursor_set_color(struct lynx_cursor *cursor, u32 fg, u32 bg);
+void sm750_hw_cursor_set_data(struct lynx_cursor *cursor, u16 rop,
 			      const u8 *data, const u8 *mask);
+void sm750_hw_cursor_set_data2(struct lynx_cursor *cursor, u16 rop,
+			       const u8 *data, const u8 *mask);
 #endif
-- 
2.39.2