linux-fbdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Greg KH <gregkh@linuxfoundation.org>
To: Pavle Rohalj <pavle.rohalj@gmail.com>
Cc: sudipm.mukherjee@gmail.com, teddy.wang@siliconmotion.com,
	linux-fbdev@vger.kernel.org, linux-staging@lists.linux.dev,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 01/49] staging: sm750fb: Update dvi_ctrl_device to snake case
Date: Wed, 7 Apr 2021 10:31:21 +0200	[thread overview]
Message-ID: <YG1t2Y55oyt8qxYi@kroah.com> (raw)
In-Reply-To: <a3618331115064265bc3bbf8963a84ee8bb57fed.1617776878.git.pavle.rohalj@gmail.com>

On Tue, Apr 06, 2021 at 11:35:56PM -0700, Pavle Rohalj wrote:
> Fix "Avoid CamelCase" checkpatch.pl checks for dvi_ctrl_device structure and
> its usages.
> 
> Signed-off-by: Pavle Rohalj <pavle.rohalj@gmail.com>
> ---
>  drivers/staging/sm750fb/ddk750_dvi.c    | 30 ++++++++--------
>  drivers/staging/sm750fb/ddk750_dvi.h    | 20 +++++------
>  drivers/staging/sm750fb/ddk750_sii164.c | 48 ++++++++++++-------------
>  drivers/staging/sm750fb/ddk750_sii164.h | 20 +++++------
>  4 files changed, 59 insertions(+), 59 deletions(-)
> 
> diff --git a/drivers/staging/sm750fb/ddk750_dvi.c b/drivers/staging/sm750fb/ddk750_dvi.c
> index cd564ea40779..db19bf732482 100644
> --- a/drivers/staging/sm750fb/ddk750_dvi.c
> +++ b/drivers/staging/sm750fb/ddk750_dvi.c
> @@ -11,20 +11,20 @@
>   * function API. Please set the function pointer to NULL whenever the function
>   * is not supported.
>   */
> -static struct dvi_ctrl_device g_dcftSupportedDviController[] = {
> +static struct dvi_ctrl_device dcft_supported_dvi_controller[] = {

Why the "dcft_" prefix?  We know this is a "dvi control device" by the
fact that the type says it is :)

>  #ifdef DVI_CTRL_SII164
>  	{
> -		.pfnInit = sii164InitChip,
> -		.pfnGetVendorId = sii164GetVendorID,
> -		.pfnGetDeviceId = sii164GetDeviceID,
> +		.pfn_init = sii164_init_chip,
> +		.pfn_get_vendor_id = sii164_get_vendor_id,
> +		.pfn_get_device_id = sii164_get_device_id,
>  #ifdef SII164_FULL_FUNCTIONS
> -		.pfnResetChip = sii164ResetChip,
> -		.pfnGetChipString = sii164GetChipString,
> -		.pfnSetPower = sii164SetPower,
> -		.pfnEnableHotPlugDetection = sii164EnableHotPlugDetection,
> -		.pfnIsConnected = sii164IsConnected,
> -		.pfnCheckInterrupt = sii164CheckInterrupt,
> -		.pfnClearInterrupt = sii164ClearInterrupt,
> +		.pfn_reset_chip = sii164_reset_chip,
> +		.pfn_get_chip_string = sii164_get_chip_string,
> +		.pfn_set_power = sii164_set_power,
> +		.pfn_enable_hot_plug_detection = sii164_enable_hot_plug_detection,
> +		.pfn_is_connected = sii164_is_connected,
> +		.pfn_check_interrupt = sii164_check_interrupt,
> +		.pfn_clear_interrupt = sii164_clear_interrupt,
>  #endif
>  	},
>  #endif
> @@ -41,11 +41,11 @@ int dviInit(unsigned char edge_select,
>  	    unsigned char pll_filter_enable,
>  	    unsigned char pll_filter_value)
>  {
> -	struct dvi_ctrl_device *pCurrentDviCtrl;
> +	struct dvi_ctrl_device *current_dvi_ctrl;
>  
> -	pCurrentDviCtrl = g_dcftSupportedDviController;
> -	if (pCurrentDviCtrl->pfnInit) {
> -		return pCurrentDviCtrl->pfnInit(edge_select,
> +	current_dvi_ctrl = dcft_supported_dvi_controller;
> +	if (current_dvi_ctrl->pfn_init) {
> +		return current_dvi_ctrl->pfn_init(edge_select,
>  						bus_select,
>  						dual_edge_clk_select,
>  						hsync_enable,
> diff --git a/drivers/staging/sm750fb/ddk750_dvi.h b/drivers/staging/sm750fb/ddk750_dvi.h
> index 1c7a565b617a..4ca2591ea94b 100644
> --- a/drivers/staging/sm750fb/ddk750_dvi.h
> +++ b/drivers/staging/sm750fb/ddk750_dvi.h
> @@ -27,16 +27,16 @@ typedef void (*PFN_DVICTRL_CLEARINTERRUPT)(void);
>  
>  /* Structure to hold all the function pointer to the DVI Controller. */
>  struct dvi_ctrl_device {
> -	PFN_DVICTRL_INIT		pfnInit;
> -	PFN_DVICTRL_RESETCHIP		pfnResetChip;
> -	PFN_DVICTRL_GETCHIPSTRING	pfnGetChipString;
> -	PFN_DVICTRL_GETVENDORID		pfnGetVendorId;
> -	PFN_DVICTRL_GETDEVICEID		pfnGetDeviceId;
> -	PFN_DVICTRL_SETPOWER		pfnSetPower;
> -	PFN_DVICTRL_HOTPLUGDETECTION	pfnEnableHotPlugDetection;
> -	PFN_DVICTRL_ISCONNECTED		pfnIsConnected;
> -	PFN_DVICTRL_CHECKINTERRUPT	pfnCheckInterrupt;
> -	PFN_DVICTRL_CLEARINTERRUPT	pfnClearInterrupt;
> +	PFN_DVICTRL_INIT		pfn_init;

"pfn_" means "pointer to a function" which is not needed at all.  Just
make this be "init".

And the whole crazy "PFN_DVICTRL_INIT" also is not needed, just put the
real function prototype in here so that we don't have to unwind the mess
to look it up.

So, this line would look like:
	void (*init)(void);

Much smaller, more obvious, matches the kernel coding style, and is way
easier to understand exactly what is happening here.

Typedefs can be used to hide complexity, but here they are just adding
it, for no good reason at all.

I appreciate long patch series being sent out, but maybe make them
smaller so you do not have to redo 49 patches because you are asked to
make a change on the very first patch like here.  Perhaps stick to 20
max for a bit until you get the process down and understand more about
what the kernel programming style is?

thanks,

greg k-h

  reply	other threads:[~2021-04-07  8:31 UTC|newest]

Thread overview: 64+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-07  6:35 [PATCH v2 00/49] *** SUBJECT HERE *** Pavle Rohalj
2021-04-07  6:35 ` [PATCH v2 01/49] staging: sm750fb: Update dvi_ctrl_device to snake case Pavle Rohalj
2021-04-07  8:31   ` Greg KH [this message]
2021-04-07  8:45     ` Pavle Rohalj
2021-04-07  6:36 ` [PATCH v2 02/49] staging: sm750fb: Rename dviInit to dvi_init and update param names Pavle Rohalj
2021-04-07  8:32   ` Greg KH
2021-05-08  9:43   ` Dan Carpenter
2021-04-07  6:36 ` [PATCH v2 03/49] staging: sm750fb: Update param names of PFN_DVICTRL_INIT function pointer Pavle Rohalj
2021-04-07  6:36 ` [PATCH v2 04/49] staging: sm750fb: Remove type names in variables and type definitions Pavle Rohalj
2021-04-07  6:36 ` [PATCH v2 05/49] staging: sm750fb: Remove remaining camel case names in ddk750_dvi.h Pavle Rohalj
2021-04-07  6:36 ` [PATCH v2 06/49] staging: sm750fb: Update displayControlAdjust_SM750LE to snake case Pavle Rohalj
2021-04-07  6:36 ` [PATCH v2 07/49] staging: sm750fb: Update programModeRegisters " Pavle Rohalj
2021-04-07  6:36 ` [PATCH v2 08/49] staging: sm750fb: Update enum values in dpms " Pavle Rohalj
2021-04-07  7:24   ` Greg KH
2021-04-07  8:27     ` Pavle Rohalj
2021-04-07  8:39       ` Greg KH
2021-05-08  9:47   ` Dan Carpenter
2021-04-07  6:36 ` [PATCH v2 09/49] staging: sm750fb: Rename sm750_set_power_mode function parameter Pavle Rohalj
2021-05-08  9:51   ` Dan Carpenter
2021-04-07  6:36 ` [PATCH v2 10/49] staging: sm750fb: Rename ddk750_setModeTiming to ddk750_set_mode_timing Pavle Rohalj
2021-04-07  6:37 ` [PATCH v2 11/49] staging: sm750fb: Rename i2cWriteReg and i2cReadReg to snake case Pavle Rohalj
2021-04-07  6:37 ` [PATCH v2 12/49] staging: sm750fb: Rename vendorID local variable " Pavle Rohalj
2021-04-07  6:37 ` [PATCH v2 13/49] staging: sm750fb: Rename deviceID " Pavle Rohalj
2021-04-07  6:37 ` [PATCH v2 14/49] staging: sm750fb: Rename sii164SelectHotPlugDetectionMode " Pavle Rohalj
2021-04-07  6:37 ` [PATCH v2 15/49] staging: sm750fb: Rename gDviCtrlChipName " Pavle Rohalj
2021-04-07  6:37 ` [PATCH v2 16/49] staging: sm750fb: Update function parameter names in ddk750_sii164.c Pavle Rohalj
2021-04-07  6:37 ` [PATCH v2 17/49] staging: sm750fb: Rename local variables to snake case Pavle Rohalj
2021-04-07  6:43 ` [PATCH v2 18/49] staging: sm750fb: Rename function params of sii164_init_chip Pavle Rohalj
2021-04-07  6:43 ` [PATCH v2 19/49] staging: sm750fb: Rename function parameter of sii164_enable_hot_plug_detection Pavle Rohalj
2021-04-07  6:43 ` [PATCH v2 20/49] staging: sm750fb: Update function parameter names to snake case Pavle Rohalj
2021-04-07  6:43 ` [PATCH v2 21/49] staging: sm750fb: Rename function write_dpPort " Pavle Rohalj
2021-04-07  6:43 ` [PATCH v2 22/49] staging: sm750fb: Update local variable in sm750_hw_copyarea " Pavle Rohalj
2021-04-07  6:43 ` [PATCH v2 23/49] staging: sm750fb: Update local variables in sm750_hw_imageblit " Pavle Rohalj
2021-04-07  6:43 ` [PATCH v2 24/49] staging: sm750fb: Update local variable in sm750_hw_fillrect " Pavle Rohalj
2021-04-07  6:43 ` [PATCH v2 25/49] staging: sm750fb: Rename deGetTransparency " Pavle Rohalj
2021-04-07  6:43 ` [PATCH v2 26/49] staging: sm750fb: Update function parameter of sm750_hw_imageblit " Pavle Rohalj
2021-04-07  6:43 ` [PATCH v2 27/49] staging: sm750fb: Rename function params to snake case in sm750_accel.h Pavle Rohalj
2021-04-07  6:43 ` [PATCH v2 28/49] staging: sm750fb: Update members of lynx_cursor to snake case Pavle Rohalj
2021-04-07  6:44 ` [PATCH v2 29/49] staging: sm750fb: Rename function sm750_hw_cursor_setSize " Pavle Rohalj
2021-04-07  6:44 ` [PATCH v2 30/49] staging: sm750fb: Rename function sm750_hw_cursor_setPos " Pavle Rohalj
2021-04-07  6:44 ` [PATCH v2 31/49] staging: sm750fb: Rename function sm750_hw_cursor_setColor " Pavle Rohalj
2021-04-07  6:44 ` [PATCH v2 32/49] staging: sm750fb: Rename function sm750_hw_cursor_setData " Pavle Rohalj
2021-04-07  6:44 ` [PATCH v2 33/49] staging: sm750fb: Rename function hw_sm750_crtc_setMode " Pavle Rohalj
2021-04-07  6:45 ` [PATCH v2 34/49] staging: sm750fb: Update members of init_status struct " Pavle Rohalj
2021-04-07  6:45 ` [PATCH v2 35/49] staging: sm750fb: Update members of sm750_dev " Pavle Rohalj
2021-04-07  6:45 ` [PATCH v2 36/49] staging: sm750fb: Update members of lynxfb_crtc " Pavle Rohalj
2021-04-07  6:45 ` [PATCH v2 37/49] staging: sm750fb: Rename function hw_sm750_output_setMode " Pavle Rohalj
2021-04-07  6:45 ` [PATCH v2 38/49] staging: sm750fb: Rename function hw_sm750_setColReg " Pavle Rohalj
2021-04-07  6:46 ` [PATCH v2 39/49] staging: sm750fb: Rename functions *_setBLANK " Pavle Rohalj
2021-04-07  6:46 ` [PATCH v2 40/49] staging: sm750fb: Rename function sm750_hw_cursor_setData2 " Pavle Rohalj
2021-04-07  6:46 ` [PATCH v2 41/49] staging: sm750fb: Rename function hw_sm750_initAccel " Pavle Rohalj
2021-04-07  6:46 ` [PATCH v2 42/49] staging: sm750fb: Rename functions *_deWait " Pavle Rohalj
2021-04-07  6:46 ` [PATCH v2 43/49] staging: sm750fb: Update members of lynx_accel struct " Pavle Rohalj
2021-04-07  6:46 ` [PATCH v2 44/49] staging: sm750fb: Rename function hw_sm750_crtc_checkMode " Pavle Rohalj
2021-04-07  6:46 ` [PATCH v2 45/49] staging: sm750fb: Rename sii164_set_power function parameter Pavle Rohalj
2021-04-07  6:46 ` [PATCH v2 46/49] staging: sm750fb: Rename local variable Bpp to bpp in sm750.c Pavle Rohalj
2021-04-07  6:46 ` [PATCH v2 47/49] staging: sm750fb: Rename proc_setBLANK member of lynxfb_output struct Pavle Rohalj
2021-04-07  6:46 ` [PATCH v2 48/49] staging: sm750fb: Rename fixId to fix_id Pavle Rohalj
2021-04-07  6:46 ` [PATCH v2 49/49] staging: sm750fb: Update members of sm750_pnltype struct to snake case Pavle Rohalj
2021-04-07  7:08 ` [PATCH v2 00/49] *** SUBJECT HERE *** Greg KH
2021-04-07  7:15   ` Pavle Rohalj
2021-04-07  7:32     ` Greg KH
2021-04-07  8:32       ` Greg KH
2021-04-07  8:46         ` Pavle Rohalj

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YG1t2Y55oyt8qxYi@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=linux-fbdev@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-staging@lists.linux.dev \
    --cc=pavle.rohalj@gmail.com \
    --cc=sudipm.mukherjee@gmail.com \
    --cc=teddy.wang@siliconmotion.com \
    --subject='Re: [PATCH v2 01/49] staging: sm750fb: Update dvi_ctrl_device to snake case' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).