Re: [linux-sunxi] Re: [PATCH 04/10] net: stmmac: sunxi platfrom extensionsfor GMAC in Allwinner A20 SoC's

[Hans de Goede]

Hi,

On 12/09/2013 06:56 PM, Chen-Yu Tsai wrote:
> Hi,
>
> On Tue, Dec 10, 2013 at 12:16 AM, Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
> > Hi,
> >
> >
> > On 12/09/2013 12:10 PM, srinivas kandagatla wrote:
> > > Hi Chen,
> > > Good to know that Allwinner uses gmac.
> > >
> > > On ST SoC, we have very similar requirements, before we merge any of
> > > these changes I think we need to come up with common way to solve both
> > > Allwinner and ST SOCs use cases.
> > >
> > > I have already posted few patches on to net-dev
> > > http://lkml.org/lkml/2013/11/12/243 to add Glue driver on top of stmmac
> > > driver.
> > >
> > >
> > > There are few reasons for the way I have done it.
> > >
> > > 1> I did not want to modify gmac driver in any sense to when a new SOC
> > > support is added.
> > > 2> As the SOC specific glue logic sits on top of standard GMAC IP, it
> > > makes sense to represent it proper harware hierarchy.
> >
> >
> > On top often is not the correct term / how things are done in practice.
> >
> > Most of the time the glue are modifications to the ip block, iow in
> > hardware they are not nested / hierarchical at all. We've had similar
> > constructs for ahci platform drivers and there we are actively trying to
> > move away from the whole nested platform devices as that has various
> > issues:
> >
> > 1) It is wrong / does not reflect reality
> > 2) It breaks deferred probing which is often used on SOCs
> >
> > I actually think Wens' approach using a SOC specific init function
> > in platform data is sound, and this is also used a lot else where.
> >
> > As for using the nested approach elsewhere, I only know about AHCI
> > platform driver doing that, and there we are actively trying to move
> > away from it.
> >
> >
> > Now reading this has also made me take a closer look at wens' patch
> > for this. Wens, I see that you directly modify registers in the ccm
> > that is a big no-no instead you should add a helper function to
> > sunxi-clk.c and use that, see ie:
> > https://bitbucket.org/emiliolopez/linux/commits/2b95847d9aa4aa13317dd7358ffcbd951dcb5eff?at=master
>
> Yes, this has been raised by Maxime. The odd "GMAC_IF_TYPE_RGMII" or
> "gmac interface type bit" has been bugging me.
>
> Additionally, the TX clock has 2 inputs (not counting MII [1]).
> The internal one is most likely controlled by the GMAC. The clock
> rate is set internally to match the link speed. The external clock
> source has controllable dividers to get the correct clock rate.
> This shouldn't be hard to model with CCF though.
>
> In hardware, this is probably a mux between the GMAC clock generator
> and the GMAC data transmit logic.
>
> My current plan is to choose MII when the clock is disabled,
> and choose either of the inputs when it is enabled. I will
> have to learn more about the CCF first.

OK, in this case I would be tempted to just go with a custom sunxi
function in the sunx-clk mode like what we have for the mmc stuff,
but if you think you can model this with the regular clock stuff,
that is of course fine too :)

Regards,

Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/