| Message ID | 20170319154920.GA12736@arushi-HP-Pavilion-Notebook (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sun, Mar 19, 2017 at 09:19:20PM +0530, Arushi Singhal wrote: > New variables are added to make the code more readable. > > Signed-off-by: Arushi Singhal <arushisinghal19971997@gmail.com> > --- > changes in v4 > -try to make the code much more readable. > - defined the variable at the top of a block. > --- > drivers/staging/sm750fb/ddk750_mode.c | 57 +++++++++++++++++++---------------- > 1 file changed, 31 insertions(+), 26 deletions(-) > > diff --git a/drivers/staging/sm750fb/ddk750_mode.c b/drivers/staging/sm750fb/ddk750_mode.c > index eea5aef2956f..6517e770e0a7 100644 > --- a/drivers/staging/sm750fb/ddk750_mode.c > +++ b/drivers/staging/sm750fb/ddk750_mode.c > @@ -76,38 +76,43 @@ static int programModeRegisters(mode_parameter_t *pModeParam, struct pll_value * > { > int ret = 0; > int cnt = 0; > - unsigned int tmp, reg; > + unsigned int tmp, reg, temp; Let's not have "tmp" and "temp" both. Generally "tmp" is better because you can't confuse it with temperature. > > if (pll->clockType == SECONDARY_PLL) { > /* programe secondary pixel clock */ > poke32(CRT_PLL_CTRL, sm750_format_pll_reg(pll)); > - poke32(CRT_HORIZONTAL_TOTAL, > - (((pModeParam->horizontal_total - 1) << > - CRT_HORIZONTAL_TOTAL_TOTAL_SHIFT) & > - CRT_HORIZONTAL_TOTAL_TOTAL_MASK) | > - ((pModeParam->horizontal_display_end - 1) & > - CRT_HORIZONTAL_TOTAL_DISPLAY_END_MASK)); > - > - poke32(CRT_HORIZONTAL_SYNC, > - ((pModeParam->horizontal_sync_width << > - CRT_HORIZONTAL_SYNC_WIDTH_SHIFT) & > - CRT_HORIZONTAL_SYNC_WIDTH_MASK) | > - ((pModeParam->horizontal_sync_start - 1) & > - CRT_HORIZONTAL_SYNC_START_MASK)); > > - poke32(CRT_VERTICAL_TOTAL, > - (((pModeParam->vertical_total - 1) << > - CRT_VERTICAL_TOTAL_TOTAL_SHIFT) & > - CRT_VERTICAL_TOTAL_TOTAL_MASK) | > - ((pModeParam->vertical_display_end - 1) & > - CRT_VERTICAL_TOTAL_DISPLAY_END_MASK)); > + temp = (pModeParam->horizontal_total - 1) << > + CRT_HORIZONTAL_TOTAL_TOTAL_SHIFT; > + temp = temp & CRT_HORIZONTAL_TOTAL_TOTAL_MASK; These two lines are really part of the same thing and they should be done together. tmp = ((pModeParam->horizontal_total - 1) << CRT_HORIZONTAL_TOTAL_TOTAL_SHIFT) & CRT_HORIZONTAL_TOTAL_TOTAL_MASK; > + temp = temp | ((pModeParam->horizontal_display_end - 1) & > + CRT_HORIZONTAL_TOTAL_DISPLAY_END_MASK); tmp |= (pModeParam->horizontal_display_end - 1) & CRT_HORIZONTAL_TOTAL_DISPLAY_END_MASK; Use |=. Remove the extra parenthesis. regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Mar 20, 2017 at 06:25:19PM +0530, Arushi Singhal wrote: > On Mon, Mar 20, 2017 at 5:58 PM, Dan Carpenter <dan.carpenter@oracle.com> > wrote: > > > On Sun, Mar 19, 2017 at 09:19:20PM +0530, Arushi Singhal wrote: > > > New variables are added to make the code more readable. > > > > > > Signed-off-by: Arushi Singhal <arushisinghal19971997@gmail.com> > > > --- > > > changes in v4 > > > -try to make the code much more readable. > > > - defined the variable at the top of a block. > > > --- > > > drivers/staging/sm750fb/ddk750_mode.c | 57 > > +++++++++++++++++++---------------- > > > 1 file changed, 31 insertions(+), 26 deletions(-) > > > > > > diff --git a/drivers/staging/sm750fb/ddk750_mode.c > > b/drivers/staging/sm750fb/ddk750_mode.c > > > index eea5aef2956f..6517e770e0a7 100644 > > > --- a/drivers/staging/sm750fb/ddk750_mode.c > > > +++ b/drivers/staging/sm750fb/ddk750_mode.c > > > @@ -76,38 +76,43 @@ static int programModeRegisters(mode_parameter_t > > *pModeParam, struct pll_value * > > > { > > > int ret = 0; > > > int cnt = 0; > > > - unsigned int tmp, reg; > > > + unsigned int tmp, reg, temp; > > > > Let's not have "tmp" and "temp" both. Generally "tmp" is better because > > you can't confuse it with temperature. > > > > Hi Dan > I have not added the tmp variable. > So is it good to use any other variable like "a" instead of temp. Just re-use "tmp". No need to add "temp". regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/staging/sm750fb/ddk750_mode.c b/drivers/staging/sm750fb/ddk750_mode.c index eea5aef2956f..6517e770e0a7 100644 --- a/drivers/staging/sm750fb/ddk750_mode.c +++ b/drivers/staging/sm750fb/ddk750_mode.c @@ -76,38 +76,43 @@ static int programModeRegisters(mode_parameter_t *pModeParam, struct pll_value * { int ret = 0; int cnt = 0; - unsigned int tmp, reg; + unsigned int tmp, reg, temp; if (pll->clockType == SECONDARY_PLL) { /* programe secondary pixel clock */ poke32(CRT_PLL_CTRL, sm750_format_pll_reg(pll)); - poke32(CRT_HORIZONTAL_TOTAL, - (((pModeParam->horizontal_total - 1) << - CRT_HORIZONTAL_TOTAL_TOTAL_SHIFT) & - CRT_HORIZONTAL_TOTAL_TOTAL_MASK) | - ((pModeParam->horizontal_display_end - 1) & - CRT_HORIZONTAL_TOTAL_DISPLAY_END_MASK)); - - poke32(CRT_HORIZONTAL_SYNC, - ((pModeParam->horizontal_sync_width << - CRT_HORIZONTAL_SYNC_WIDTH_SHIFT) & - CRT_HORIZONTAL_SYNC_WIDTH_MASK) | - ((pModeParam->horizontal_sync_start - 1) & - CRT_HORIZONTAL_SYNC_START_MASK)); - poke32(CRT_VERTICAL_TOTAL, - (((pModeParam->vertical_total - 1) << - CRT_VERTICAL_TOTAL_TOTAL_SHIFT) & - CRT_VERTICAL_TOTAL_TOTAL_MASK) | - ((pModeParam->vertical_display_end - 1) & - CRT_VERTICAL_TOTAL_DISPLAY_END_MASK)); + temp = (pModeParam->horizontal_total - 1) << + CRT_HORIZONTAL_TOTAL_TOTAL_SHIFT; + temp = temp & CRT_HORIZONTAL_TOTAL_TOTAL_MASK; + temp = temp | ((pModeParam->horizontal_display_end - 1) & + CRT_HORIZONTAL_TOTAL_DISPLAY_END_MASK); - poke32(CRT_VERTICAL_SYNC, - ((pModeParam->vertical_sync_height << - CRT_VERTICAL_SYNC_HEIGHT_SHIFT) & - CRT_VERTICAL_SYNC_HEIGHT_MASK) | - ((pModeParam->vertical_sync_start - 1) & - CRT_VERTICAL_SYNC_START_MASK)); + poke32(CRT_HORIZONTAL_TOTAL, temp); + + temp = (pModeParam->horizontal_sync_width << + CRT_HORIZONTAL_SYNC_WIDTH_SHIFT); + temp = temp & CRT_HORIZONTAL_SYNC_WIDTH_MASK; + temp = temp | ((pModeParam->horizontal_sync_start - 1) & + CRT_HORIZONTAL_SYNC_START_MASK); + + poke32(CRT_HORIZONTAL_SYNC, temp); + + temp = (pModeParam->vertical_total - 1) << + CRT_VERTICAL_TOTAL_TOTAL_SHIFT; + temp = temp & CRT_VERTICAL_TOTAL_TOTAL_MASK; + temp = temp | ((pModeParam->vertical_display_end - 1) & + CRT_VERTICAL_TOTAL_DISPLAY_END_MASK); + + poke32(CRT_VERTICAL_TOTAL, temp); + + temp = pModeParam->vertical_sync_height << + CRT_VERTICAL_SYNC_HEIGHT_SHIFT; + temp = temp & CRT_VERTICAL_SYNC_HEIGHT_MASK; + temp = temp | ((pModeParam->vertical_sync_start - 1) & + CRT_VERTICAL_SYNC_START_MASK); + + poke32(CRT_VERTICAL_SYNC, temp); tmp = DISPLAY_CTRL_TIMING | DISPLAY_CTRL_PLANE; if (pModeParam->vertical_sync_polarity)
New variables are added to make the code more readable. Signed-off-by: Arushi Singhal <arushisinghal19971997@gmail.com> --- changes in v4 -try to make the code much more readable. - defined the variable at the top of a block. --- drivers/staging/sm750fb/ddk750_mode.c | 57 +++++++++++++++++++---------------- 1 file changed, 31 insertions(+), 26 deletions(-)