| Message ID | 20231013104220.7527-1-anonolitunya@gmail.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | Staging: sm750fb:Add snakecase naming style | expand |
On Fri, Oct 13, 2023 at 01:42:15PM +0300, Dorcas AnonoLitunya wrote: > From: Dorcas Anono Litunya <anonolitunya@gmail.com> > > Change camelCase variables in file to snake_case for consistent naming > practices. Issue found by checkpatch. > > Signed-off-by: Dorcas Anono Litunya <anonolitunya@gmail.com> > --- > drivers/staging/sm750fb/ddk750_mode.c | 86 +++++++++++++-------------- > drivers/staging/sm750fb/ddk750_mode.h | 2 +- > drivers/staging/sm750fb/sm750_hw.c | 2 +- > 3 files changed, 45 insertions(+), 45 deletions(-) > > diff --git a/drivers/staging/sm750fb/ddk750_mode.c b/drivers/staging/sm750fb/ddk750_mode.c > index e00a6cb31947..f08dcab29172 100644 > --- a/drivers/staging/sm750fb/ddk750_mode.c > +++ b/drivers/staging/sm750fb/ddk750_mode.c > @@ -14,13 +14,13 @@ > * in bit 29:27 of Display Control register. > */ > static unsigned long > -displayControlAdjust_SM750LE(struct mode_parameter *pModeParam, > - unsigned long dispControl) > +display_control_adjust_SM750LE(struct mode_parameter *p_mode_param, The p stands for pointer. We don't like that naming style. Just call it mode_param. Thes are the renamed things. displayControlAdjust_SM750LE => display_control_adjust_SM750LE pModeParam => p_mode_param dispControl => disp_control programModeRegisters => program_mode_registers ddk750_setModeTiming => ddk750_set_mode_timing I feel like this would be better broken up probably into one variable per patch. It's jumping around between files. These variables are not closely related. regards, dan carpenter
On Fri, Oct 13, 2023 at 01:48:08PM +0300, Dan Carpenter wrote: > On Fri, Oct 13, 2023 at 01:42:15PM +0300, Dorcas AnonoLitunya wrote: > > From: Dorcas Anono Litunya <anonolitunya@gmail.com> > > > > Change camelCase variables in file to snake_case for consistent naming > > practices. Issue found by checkpatch. > > > > Signed-off-by: Dorcas Anono Litunya <anonolitunya@gmail.com> > > --- > > drivers/staging/sm750fb/ddk750_mode.c | 86 +++++++++++++-------------- > > drivers/staging/sm750fb/ddk750_mode.h | 2 +- > > drivers/staging/sm750fb/sm750_hw.c | 2 +- > > 3 files changed, 45 insertions(+), 45 deletions(-) > > > > diff --git a/drivers/staging/sm750fb/ddk750_mode.c b/drivers/staging/sm750fb/ddk750_mode.c > > index e00a6cb31947..f08dcab29172 100644 > > --- a/drivers/staging/sm750fb/ddk750_mode.c > > +++ b/drivers/staging/sm750fb/ddk750_mode.c > > @@ -14,13 +14,13 @@ > > * in bit 29:27 of Display Control register. > > */ > > static unsigned long > > -displayControlAdjust_SM750LE(struct mode_parameter *pModeParam, > > - unsigned long dispControl) > > +display_control_adjust_SM750LE(struct mode_parameter *p_mode_param, > > The p stands for pointer. We don't like that naming style. Just call > it mode_param. > > Thes are the renamed things. > > displayControlAdjust_SM750LE => display_control_adjust_SM750LE > pModeParam => p_mode_param > dispControl => disp_control > programModeRegisters => program_mode_registers > ddk750_setModeTiming => ddk750_set_mode_timing > > I feel like this would be better broken up probably into one variable > per patch. It's jumping around between files. These variables are not > closely related. Thanks for the feedback Dan. I will revise to do one variable per patch. However,I have an inquiry the main reason its jumping between files is because one of the functions I am modifying(the ddk_set_mode_timing) is imported and used in other files. In this case, should I do one patch per variable per file? Dorcas > > regards, > dan carpenter >
On Fri, Oct 13, 2023 at 06:56:08PM +0300, Dan Carpenter wrote: > On Fri, Oct 13, 2023 at 02:33:00PM +0300, Dorcas Litunya wrote: > > On Fri, Oct 13, 2023 at 01:48:08PM +0300, Dan Carpenter wrote: > > > On Fri, Oct 13, 2023 at 01:42:15PM +0300, Dorcas AnonoLitunya wrote: > > > > From: Dorcas Anono Litunya <anonolitunya@gmail.com> > > > > > > > > Change camelCase variables in file to snake_case for consistent naming > > > > practices. Issue found by checkpatch. > > > > > > > > Signed-off-by: Dorcas Anono Litunya <anonolitunya@gmail.com> > > > > --- > > > > drivers/staging/sm750fb/ddk750_mode.c | 86 +++++++++++++-------------- > > > > drivers/staging/sm750fb/ddk750_mode.h | 2 +- > > > > drivers/staging/sm750fb/sm750_hw.c | 2 +- > > > > 3 files changed, 45 insertions(+), 45 deletions(-) > > > > > > > > diff --git a/drivers/staging/sm750fb/ddk750_mode.c b/drivers/staging/sm750fb/ddk750_mode.c > > > > index e00a6cb31947..f08dcab29172 100644 > > > > --- a/drivers/staging/sm750fb/ddk750_mode.c > > > > +++ b/drivers/staging/sm750fb/ddk750_mode.c > > > > @@ -14,13 +14,13 @@ > > > > * in bit 29:27 of Display Control register. > > > > */ > > > > static unsigned long > > > > -displayControlAdjust_SM750LE(struct mode_parameter *pModeParam, > > > > - unsigned long dispControl) > > > > +display_control_adjust_SM750LE(struct mode_parameter *p_mode_param, > > > > > > The p stands for pointer. We don't like that naming style. Just call > > > it mode_param. > > > > > > Thes are the renamed things. > > > > > > displayControlAdjust_SM750LE => display_control_adjust_SM750LE > > > pModeParam => p_mode_param > > > dispControl => disp_control > > > programModeRegisters => program_mode_registers > > > ddk750_setModeTiming => ddk750_set_mode_timing > > > > > > I feel like this would be better broken up probably into one variable > > > per patch. It's jumping around between files. These variables are not > > > closely related. > > Thanks for the feedback Dan. I will revise to do one variable per patch. > > > > However,I have an inquiry the main reason its jumping between files is because one of the > > functions I am modifying(the ddk_set_mode_timing) is imported and used > > in other files. In this case, should I do one patch per variable per > > file? > > No that would break the build... > > It's just that I felt that in this case it's especially useful to > break it apart because some of them affect multiple files and some of > the variables are local to a given function. (You should still adjust > the header file to match even though the compiler doesn't care). Makes more sense. I will work on that. Thanks! > regards, > dan carpenter >
On Fri, Oct 13, 2023 at 02:33:00PM +0300, Dorcas Litunya wrote: > On Fri, Oct 13, 2023 at 01:48:08PM +0300, Dan Carpenter wrote: > > On Fri, Oct 13, 2023 at 01:42:15PM +0300, Dorcas AnonoLitunya wrote: > > > From: Dorcas Anono Litunya <anonolitunya@gmail.com> > > > > > > Change camelCase variables in file to snake_case for consistent naming > > > practices. Issue found by checkpatch. > > > > > > Signed-off-by: Dorcas Anono Litunya <anonolitunya@gmail.com> > > > --- > > > drivers/staging/sm750fb/ddk750_mode.c | 86 +++++++++++++-------------- > > > drivers/staging/sm750fb/ddk750_mode.h | 2 +- > > > drivers/staging/sm750fb/sm750_hw.c | 2 +- > > > 3 files changed, 45 insertions(+), 45 deletions(-) > > > > > > diff --git a/drivers/staging/sm750fb/ddk750_mode.c b/drivers/staging/sm750fb/ddk750_mode.c > > > index e00a6cb31947..f08dcab29172 100644 > > > --- a/drivers/staging/sm750fb/ddk750_mode.c > > > +++ b/drivers/staging/sm750fb/ddk750_mode.c > > > @@ -14,13 +14,13 @@ > > > * in bit 29:27 of Display Control register. > > > */ > > > static unsigned long > > > -displayControlAdjust_SM750LE(struct mode_parameter *pModeParam, > > > - unsigned long dispControl) > > > +display_control_adjust_SM750LE(struct mode_parameter *p_mode_param, > > > > The p stands for pointer. We don't like that naming style. Just call > > it mode_param. > > > > Thes are the renamed things. > > > > displayControlAdjust_SM750LE => display_control_adjust_SM750LE > > pModeParam => p_mode_param > > dispControl => disp_control > > programModeRegisters => program_mode_registers > > ddk750_setModeTiming => ddk750_set_mode_timing > > > > I feel like this would be better broken up probably into one variable > > per patch. It's jumping around between files. These variables are not > > closely related. > Thanks for the feedback Dan. I will revise to do one variable per patch. > > However,I have an inquiry the main reason its jumping between files is because one of the > functions I am modifying(the ddk_set_mode_timing) is imported and used > in other files. In this case, should I do one patch per variable per > file? No that would break the build... It's just that I felt that in this case it's especially useful to break it apart because some of them affect multiple files and some of the variables are local to a given function. (You should still adjust the header file to match even though the compiler doesn't care). regards, dan carpenter
diff --git a/drivers/staging/sm750fb/ddk750_mode.c b/drivers/staging/sm750fb/ddk750_mode.c index e00a6cb31947..f08dcab29172 100644 --- a/drivers/staging/sm750fb/ddk750_mode.c +++ b/drivers/staging/sm750fb/ddk750_mode.c @@ -14,13 +14,13 @@ * in bit 29:27 of Display Control register. */ static unsigned long -displayControlAdjust_SM750LE(struct mode_parameter *pModeParam, - unsigned long dispControl) +display_control_adjust_SM750LE(struct mode_parameter *p_mode_param, + unsigned long disp_control) { unsigned long x, y; - x = pModeParam->horizontal_display_end; - y = pModeParam->vertical_display_end; + x = p_mode_param->horizontal_display_end; + y = p_mode_param->vertical_display_end; /* * SM750LE has to set up the top-left and bottom-right @@ -42,41 +42,41 @@ displayControlAdjust_SM750LE(struct mode_parameter *pModeParam, */ /* Clear bit 29:27 of display control register */ - dispControl &= ~CRT_DISPLAY_CTRL_CLK_MASK; + disp_control &= ~CRT_DISPLAY_CTRL_CLK_MASK; /* Set bit 29:27 of display control register for the right clock */ /* Note that SM750LE only need to supported 7 resolutions. */ if (x == 800 && y == 600) - dispControl |= CRT_DISPLAY_CTRL_CLK_PLL41; + disp_control |= CRT_DISPLAY_CTRL_CLK_PLL41; else if (x == 1024 && y == 768) - dispControl |= CRT_DISPLAY_CTRL_CLK_PLL65; + disp_control |= CRT_DISPLAY_CTRL_CLK_PLL65; else if (x == 1152 && y == 864) - dispControl |= CRT_DISPLAY_CTRL_CLK_PLL80; + disp_control |= CRT_DISPLAY_CTRL_CLK_PLL80; else if (x == 1280 && y == 768) - dispControl |= CRT_DISPLAY_CTRL_CLK_PLL80; + disp_control |= CRT_DISPLAY_CTRL_CLK_PLL80; else if (x == 1280 && y == 720) - dispControl |= CRT_DISPLAY_CTRL_CLK_PLL74; + disp_control |= CRT_DISPLAY_CTRL_CLK_PLL74; else if (x == 1280 && y == 960) - dispControl |= CRT_DISPLAY_CTRL_CLK_PLL108; + disp_control |= CRT_DISPLAY_CTRL_CLK_PLL108; else if (x == 1280 && y == 1024) - dispControl |= CRT_DISPLAY_CTRL_CLK_PLL108; + disp_control |= CRT_DISPLAY_CTRL_CLK_PLL108; else /* default to VGA clock */ - dispControl |= CRT_DISPLAY_CTRL_CLK_PLL25; + disp_control |= CRT_DISPLAY_CTRL_CLK_PLL25; /* Set bit 25:24 of display controller */ - dispControl |= (CRT_DISPLAY_CTRL_CRTSELECT | CRT_DISPLAY_CTRL_RGBBIT); + disp_control |= (CRT_DISPLAY_CTRL_CRTSELECT | CRT_DISPLAY_CTRL_RGBBIT); /* Set bit 14 of display controller */ - dispControl |= DISPLAY_CTRL_CLOCK_PHASE; + disp_control |= DISPLAY_CTRL_CLOCK_PHASE; - poke32(CRT_DISPLAY_CTRL, dispControl); + poke32(CRT_DISPLAY_CTRL, disp_control); - return dispControl; + return disp_control; } /* only timing related registers will be programed */ -static int programModeRegisters(struct mode_parameter *pModeParam, - struct pll_value *pll) +static int program_mode_registers(struct mode_parameter *p_mode_param, + struct pll_value *pll) { int ret = 0; int cnt = 0; @@ -86,46 +86,46 @@ static int programModeRegisters(struct mode_parameter *pModeParam, /* programe secondary pixel clock */ poke32(CRT_PLL_CTRL, sm750_format_pll_reg(pll)); - tmp = ((pModeParam->horizontal_total - 1) << + tmp = ((p_mode_param->horizontal_total - 1) << CRT_HORIZONTAL_TOTAL_TOTAL_SHIFT) & CRT_HORIZONTAL_TOTAL_TOTAL_MASK; - tmp |= (pModeParam->horizontal_display_end - 1) & + tmp |= (p_mode_param->horizontal_display_end - 1) & CRT_HORIZONTAL_TOTAL_DISPLAY_END_MASK; poke32(CRT_HORIZONTAL_TOTAL, tmp); - tmp = (pModeParam->horizontal_sync_width << + tmp = (p_mode_param->horizontal_sync_width << CRT_HORIZONTAL_SYNC_WIDTH_SHIFT) & CRT_HORIZONTAL_SYNC_WIDTH_MASK; - tmp |= (pModeParam->horizontal_sync_start - 1) & + tmp |= (p_mode_param->horizontal_sync_start - 1) & CRT_HORIZONTAL_SYNC_START_MASK; poke32(CRT_HORIZONTAL_SYNC, tmp); - tmp = ((pModeParam->vertical_total - 1) << + tmp = ((p_mode_param->vertical_total - 1) << CRT_VERTICAL_TOTAL_TOTAL_SHIFT) & CRT_VERTICAL_TOTAL_TOTAL_MASK; - tmp |= (pModeParam->vertical_display_end - 1) & + tmp |= (p_mode_param->vertical_display_end - 1) & CRT_VERTICAL_TOTAL_DISPLAY_END_MASK; poke32(CRT_VERTICAL_TOTAL, tmp); - tmp = ((pModeParam->vertical_sync_height << + tmp = ((p_mode_param->vertical_sync_height << CRT_VERTICAL_SYNC_HEIGHT_SHIFT)) & CRT_VERTICAL_SYNC_HEIGHT_MASK; - tmp |= (pModeParam->vertical_sync_start - 1) & + tmp |= (p_mode_param->vertical_sync_start - 1) & CRT_VERTICAL_SYNC_START_MASK; poke32(CRT_VERTICAL_SYNC, tmp); tmp = DISPLAY_CTRL_TIMING | DISPLAY_CTRL_PLANE; - if (pModeParam->vertical_sync_polarity) + if (p_mode_param->vertical_sync_polarity) tmp |= DISPLAY_CTRL_VSYNC_PHASE; - if (pModeParam->horizontal_sync_polarity) + if (p_mode_param->horizontal_sync_polarity) tmp |= DISPLAY_CTRL_HSYNC_PHASE; if (sm750_get_chip_type() == SM750LE) { - displayControlAdjust_SM750LE(pModeParam, tmp); + display_control_adjust_SM750LE(p_mode_param, tmp); } else { reg = peek32(CRT_DISPLAY_CTRL) & ~(DISPLAY_CTRL_VSYNC_PHASE | @@ -140,40 +140,40 @@ static int programModeRegisters(struct mode_parameter *pModeParam, poke32(PANEL_PLL_CTRL, sm750_format_pll_reg(pll)); - reg = ((pModeParam->horizontal_total - 1) << + reg = ((p_mode_param->horizontal_total - 1) << PANEL_HORIZONTAL_TOTAL_TOTAL_SHIFT) & PANEL_HORIZONTAL_TOTAL_TOTAL_MASK; - reg |= ((pModeParam->horizontal_display_end - 1) & + reg |= ((p_mode_param->horizontal_display_end - 1) & PANEL_HORIZONTAL_TOTAL_DISPLAY_END_MASK); poke32(PANEL_HORIZONTAL_TOTAL, reg); poke32(PANEL_HORIZONTAL_SYNC, - ((pModeParam->horizontal_sync_width << + ((p_mode_param->horizontal_sync_width << PANEL_HORIZONTAL_SYNC_WIDTH_SHIFT) & PANEL_HORIZONTAL_SYNC_WIDTH_MASK) | - ((pModeParam->horizontal_sync_start - 1) & + ((p_mode_param->horizontal_sync_start - 1) & PANEL_HORIZONTAL_SYNC_START_MASK)); poke32(PANEL_VERTICAL_TOTAL, - (((pModeParam->vertical_total - 1) << + (((p_mode_param->vertical_total - 1) << PANEL_VERTICAL_TOTAL_TOTAL_SHIFT) & PANEL_VERTICAL_TOTAL_TOTAL_MASK) | - ((pModeParam->vertical_display_end - 1) & + ((p_mode_param->vertical_display_end - 1) & PANEL_VERTICAL_TOTAL_DISPLAY_END_MASK)); poke32(PANEL_VERTICAL_SYNC, - ((pModeParam->vertical_sync_height << + ((p_mode_param->vertical_sync_height << PANEL_VERTICAL_SYNC_HEIGHT_SHIFT) & PANEL_VERTICAL_SYNC_HEIGHT_MASK) | - ((pModeParam->vertical_sync_start - 1) & + ((p_mode_param->vertical_sync_start - 1) & PANEL_VERTICAL_SYNC_START_MASK)); tmp = DISPLAY_CTRL_TIMING | DISPLAY_CTRL_PLANE; - if (pModeParam->vertical_sync_polarity) + if (p_mode_param->vertical_sync_polarity) tmp |= DISPLAY_CTRL_VSYNC_PHASE; - if (pModeParam->horizontal_sync_polarity) + if (p_mode_param->horizontal_sync_polarity) tmp |= DISPLAY_CTRL_HSYNC_PHASE; - if (pModeParam->clock_phase_polarity) + if (p_mode_param->clock_phase_polarity) tmp |= DISPLAY_CTRL_CLOCK_PHASE; reserved = PANEL_DISPLAY_CTRL_RESERVED_MASK | @@ -207,7 +207,7 @@ static int programModeRegisters(struct mode_parameter *pModeParam, return ret; } -int ddk750_setModeTiming(struct mode_parameter *parm, enum clock_type clock) +int ddk750_set_mode_timing(struct mode_parameter *parm, enum clock_type clock) { struct pll_value pll; @@ -220,6 +220,6 @@ int ddk750_setModeTiming(struct mode_parameter *parm, enum clock_type clock) outb_p(0x88, 0x3d4); outb_p(0x06, 0x3d5); } - programModeRegisters(parm, &pll); + program_mode_registers(parm, &pll); return 0; } diff --git a/drivers/staging/sm750fb/ddk750_mode.h b/drivers/staging/sm750fb/ddk750_mode.h index 2df78a0937b2..1b70885f85e5 100644 --- a/drivers/staging/sm750fb/ddk750_mode.h +++ b/drivers/staging/sm750fb/ddk750_mode.h @@ -33,5 +33,5 @@ struct mode_parameter { enum spolarity clock_phase_polarity; }; -int ddk750_setModeTiming(struct mode_parameter *parm, enum clock_type clock); +int ddk750_set_mode_timing(struct mode_parameter *parm, enum clock_type clock); #endif diff --git a/drivers/staging/sm750fb/sm750_hw.c b/drivers/staging/sm750fb/sm750_hw.c index 71247eaf26ee..4bc89218c11c 100644 --- a/drivers/staging/sm750fb/sm750_hw.c +++ b/drivers/staging/sm750fb/sm750_hw.c @@ -305,7 +305,7 @@ int hw_sm750_crtc_setMode(struct lynxfb_crtc *crtc, clock = SECONDARY_PLL; pr_debug("Request pixel clock = %lu\n", modparm.pixel_clock); - ret = ddk750_setModeTiming(&modparm, clock); + ret = ddk750_set_mode_timing(&modparm, clock); if (ret) { pr_err("Set mode timing failed\n"); goto exit;