On Thu, Oct 08, 2015 at 12:58:04PM +0530, Sudip Mukherjee wrote:
> On Tue, Oct 06, 2015 at 09:42:15AM +0100, Mike Rapoport wrote:
> > The getChipClock function is used only to get MXCLK frequency, which
> > makes most of getPllValue function unused and thus. The detection of
> > MXCLK frequency may be implemented directly in getChipClock rendering
> > getPllValue and calcPLL unused.
> >
> > Signed-off-by: Mike Rapoport <mike.rapoport@xxxxxxxxx>
> > ---
>
> You have also missed saving the values in pPLL->inputFreq and
> pPLL->clockType = clockType. Both have been used later.
the pPLL pointer that is passed to getPllValue points to a local
variable in getChipClock. If we merge these funcions, the variable and
the pointer are not needed.
> Is it ok to remove the PLL calculation of all the different type of
> display devices? Currently only PANEL_PLL_CTRL is being used for
> calculation and ofcourse we also donot keep in kernel that we donot use.
I think we can remove the unused code and then generalize the clock
calculations if anybody will ever need it.
> Maybe we can do some thing like:
>
> diff --git a/drivers/staging/sm750fb/ddk750_chip.c b/drivers/staging/sm750fb/ddk750_chip.c
> index ad2f1c0..4b60894 100644
> --- a/drivers/staging/sm750fb/ddk750_chip.c
> +++ b/drivers/staging/sm750fb/ddk750_chip.c
> @@ -39,11 +39,17 @@ static unsigned int getChipClock(void)
> {
> unsigned int pll_reg;
> unsigned int M, N, OD, POD;
> + clock_type_t clk_type = PANEL_PLL_CTRL;
> + /*
> + * Different register location based on display device type:
> + * MXCLK_PLL_CTRL, PANEL_PLL_CTRL, CRT_PLL_CTRL, VGA_PLL0_CTRL,
> + * VGA_PLL1_CTRL
> + */
>
> if (getChipType() == SM750LE)
> return MHz(130);
>
> - pll_reg = PEEK32(MXCLK_PLL_CTRL);
> + pll_reg = PEEK32(clk_type);
> M = FIELD_GET(pll_reg, PANEL_PLL_CTRL, M);
> N = FIELD_GET(pll_reg, PANEL_PLL_CTRL, N);
> OD = FIELD_GET(pll_reg, PANEL_PLL_CTRL, OD);
>
> ---
>
> This is on top of your patch. So now we have the different possible
> values in the comments, so whenever, if anyone is working on a different
> device atleast they will not have to search and find out what the other
> values can be.
>
> regards
> sudip
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
