On Sat, Mar 13, 2010 at 04:08:20, Kevin Hilman wrote:
> Sriramakrishnan <s...@ti.com> writes:
>
> > From: Sekhar Nori <nsek...@ti.com>
> >
> > The patch "TI DaVinci EMAC: Add EMAC PHY clock handling" adds
> > support for enabling and disabling the EMAC PHY clock.
> >
> > The PHY clock on all DaVinci boards is derived from a fixed
> > on board clock. This patch adds the PHY clock definition to
> > the clock tree for all the DaVinci boards using EMAC. Also,
> > the existing input to EMAC module is differentiated from the
> > PHY clock using the clock name "emac_clk".
> >
> > Without this patch ethernet fails to initialize since it cannot
> > get the PHY clock and EMAC clock.
> >
> > Tested on EVM boards for DM365, DM6467, DM644x, DA830 and DA850.
> >
> > Signed-off-by: Sekhar Nori <nsek...@ti.com>
[...]
> >
> > +#define EMAC_PHY_CLK_RATE 50000000
> > +
> > +static struct clk emac_phy = {
> > + .name = "emac_phy",
> > + .rate = EMAC_PHY_CLK_RATE,
> > +};
> > +
> > +static struct clk_lookup emac_phy_clks[] = {
> > + CLK("davinci_emac.1", "phy_clk", &emac_phy),
>
[...]
>
> > + CLK(NULL, NULL, NULL),
> > +};
> > +
>
> I'm not crazy about the clock definitions in the board files. I
> assume you put it here instead of <soc>.c is because each clock
> has a board specific rate.
That and the fact that none of the DaVinci devices have an
integrated PHY on the SoC. The clock comes from an onboard
oscillator too. So, it seemed odd to include phy clock structure
in the SoC specific file.
>
> Instead, what I'd rather see is the clock defined once for each
> <soc>.c with a custom set_rate hook. The default rate could
> be a per-SoC default (25MHz looks common) and any board files
> that don't use the default can use clk_set_rate() to change it.
>
> This approach should simplfy things and minimize changes to board
> files.
Right, that surely will lead to a much simpler patch so I will
go ahead and change. Also, only DA850 offers a choice between
MII (25MHz) and RMII (50MHz) phy. So, the set_rate needs to be
implemented only for this SoC.
Thanks,
Sekhar
_______________________________________________
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
