PSC clock divider

PSC clock divider

[Jon Smirl]

mpc52xx_set_psc_clkdiv() has problems. It take the clk divider as a
parameter. But this divisor is not always gettting calculated
correctly. My code in i2s was doing it wrong.

Take this snippet from the SPI driver, it just assumes a fsystem of
512Mhz. fsystem is 533Mhz on my boards.

	/* default sysclk is 512MHz */
	mclken_div = (mps->sysclk ? mps->sysclk : 512000000) / MCLK;
	mpc52xx_set_psc_clkdiv(psc_id, mclken_div);

Is it also not accounting for the hardware adding one to the divisor.

I've change i2s to this:

			if (!fsystem) {
				np = of_find_matching_node(NULL, mpc52xx_cdm_ids);
				mpc52xx_cdm = of_iomap(np, 0);
				fsystem = mpc5xxx_get_bus_frequency(np);
				of_node_put(np);
				val = in_be32(&mpc52xx_cdm->rstcfg);
				if (val & (1 << 5))
					fsystem *= 8;
				else
					fsystem *= 4;
				iounmap(mpc52xx_cdm);
			}
			clkdiv = fsystem / freq;
			err = fsystem % freq;
			if (err > freq / 2)
				clkdiv++;

			dev_dbg(psc_dma->dev, "psc_i2s_set_sysclk(clkdiv %d freq error=%dHz)\n",
					clkdiv, (fsystem / clkdiv - freq));

			/* PSC is 1-6 */
			/* Hardware adds 1 to divisor */
			return mpc52xx_set_psc_clkdiv(psc_dma->id + 1, clkdiv - 1);


Should I modify mpc52xx_set_psc_clkdiv() to take in a frequency and
them move this code into mpc52xx_common.c? That allows the sysclk
parameter to be eliminated for SPI.


-- 
Jon Smirl
jonsmirl@gmail.com

Re: PSC clock divider

[Grant Likely]

On Fri, Aug 7, 2009 at 10:03 AM, Jon Smirl<jonsmirl@gmail.com> wrote:
> mpc52xx_set_psc_clkdiv() has problems. It take the clk divider as a
> parameter. But this divisor is not always gettting calculated
> correctly. My code in i2s was doing it wrong.
>
> Take this snippet from the SPI driver, it just assumes a fsystem of
> 512Mhz. fsystem is 533Mhz on my boards.
>
> =A0 =A0 =A0 =A0/* default sysclk is 512MHz */
> =A0 =A0 =A0 =A0mclken_div =3D (mps->sysclk ? mps->sysclk : 512000000) / M=
CLK;
> =A0 =A0 =A0 =A0mpc52xx_set_psc_clkdiv(psc_id, mclken_div);
>
> Is it also not accounting for the hardware adding one to the divisor.
>
> I've change i2s to this:
>
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (!fsystem) {
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0np =3D of_=
find_matching_node(NULL, mpc52xx_cdm_ids);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0mpc52xx_cd=
m =3D of_iomap(np, 0);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0fsystem =
=3D mpc5xxx_get_bus_frequency(np);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0of_node_pu=
t(np);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0val =3D in=
_be32(&mpc52xx_cdm->rstcfg);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (val & =
(1 << 5))
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0fsystem *=3D 8;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0else
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0fsystem *=3D 4;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0iounmap(mp=
c52xx_cdm);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0}
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0clkdiv =3D fsystem / freq;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0err =3D fsystem % freq;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (err > freq / 2)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0clkdiv++;
>
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0dev_dbg(psc_dma->dev, "psc=
_i2s_set_sysclk(clkdiv %d freq error=3D%dHz)\n",
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0clkdiv, (fsystem / clkdiv - freq));
>
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/* PSC is 1-6 */
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/* Hardware adds 1 to divi=
sor */
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return mpc52xx_set_psc_clk=
div(psc_dma->id + 1, clkdiv - 1);
>
>
> Should I modify mpc52xx_set_psc_clkdiv() to take in a frequency and
> them move this code into mpc52xx_common.c? That allows the sysclk
> parameter to be eliminated for SPI.

Yes, please do.

g.

--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: PSC clock divider

[Jon Smirl]

On Fri, Aug 7, 2009 at 12:15 PM, Grant Likely<grant.likely@secretlab.ca> wrote:
>> Should I modify mpc52xx_set_psc_clkdiv() to take in a frequency and
>> them move this code into mpc52xx_common.c? That allows the sysclk
>> parameter to be eliminated for SPI.
>
> Yes, please do.

Can mpc5xxx_clocks.c be eliminated and this function:

mpc5xxx_get_bus_frequency(struct device_node *node)

be moved into mpc52cc_common.c?


-- 
Jon Smirl
jonsmirl@gmail.com

Re: PSC clock divider

[Grant Likely]

On Fri, Aug 7, 2009 at 10:34 AM, Jon Smirl<jonsmirl@gmail.com> wrote:
> On Fri, Aug 7, 2009 at 12:15 PM, Grant Likely<grant.likely@secretlab.ca> wrote:
>>> Should I modify mpc52xx_set_psc_clkdiv() to take in a frequency and
>>> them move this code into mpc52xx_common.c? That allows the sysclk
>>> parameter to be eliminated for SPI.
>>
>> Yes, please do.
>
> Can mpc5xxx_clocks.c be eliminated and this function:
>
> mpc5xxx_get_bus_frequency(struct device_node *node)
>
> be moved into mpc52cc_common.c?

No.  mpc5xxx_get_bus_frequency is not mpc5200 specific.  It is used by
mpc5121 also.

g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.