> Date: Sun, 12 Nov 2017 23:53:10 +0200
> From: Artturi Alm <artturi....@gmail.com>
> 
> On Sat, Oct 07, 2017 at 10:16:05AM +0300, Artturi Alm wrote:
> > Hi,
> > 
> > 
> > what was the cause of these delays? i just spotted this, so untested on
> > likely more affected HW(sunxi A64/H3 and rockchips), but just for discussion
> > about the delays/ordering? are they really needed like that?
> > 
> > i don't remember having read anything about the order of these from
> > devicetree-binding .txt's i have read so far from u-boot/linux.
> > 
> > "Make sure that the reset signal has been released
> > before the release of module clock gating;"
> > 
> > above can be found from under the short CCU "x.3.6 Programming 
> > Guidelines"[0].
> > maybe this ordering should be verified from somewhere else, as atleast
> > for my sorry non-nativeNlazy grammar i could understand wrong, what is being
> > meant w/"the release" above.. but even w/above discarded, my intuition does
> > favor deasserting reset before gating sclki, hence the diff.
> > 
> > the diff below booted faster with no observable changes in dmesg on cubie2,
> > and otherwise succesfully on panda and wandboard.
> > 
> > -Artturi
> > 
> 
> Ping?
> 
> i think i've passed by sources in both fbsd&nbsd since email above
> suggesting the order should be like in the diff.
> does anyone want me to provide more on my findings about this,
> or anything else?

As far as I can tell, the Linux code always enables the clock before
deasserting the reset.  If the ordering is SoC-specific, I guess the
clock/reset controller driver should handle it as some of these
drivers are not SoC-specific.

For that reason I'm going to commit the sxitwi diff with the
"canonical" ordering.

Cheers,

Mark

> > [0] sources:
> > A64 User Manual(Revision 1.0):
> > "3.3.6.4. Gating and reset"     Page 147
> > H3 Datasheet(Revision1.2):
> > "4.3.6.4. Gating and reset"     Page 142
> > 
> > 
> > 
> > diff --git a/sys/dev/fdt/ehci_fdt.c b/sys/dev/fdt/ehci_fdt.c
> > index 55fe75f1a9c..fae54ac11ee 100644
> > --- a/sys/dev/fdt/ehci_fdt.c
> > +++ b/sys/dev/fdt/ehci_fdt.c
> > @@ -91,9 +91,10 @@ ehci_fdt_attach(struct device *parent, struct device 
> > *self, void *aux)
> >  
> >     pinctrl_byname(sc->sc_node, "default");
> >  
> > -   clock_enable_all(sc->sc_node);
> >     reset_deassert_all(sc->sc_node);
> >  
> > +   clock_enable_all(sc->sc_node);
> > +
> >     /* Disable interrupts, so we don't get any spurious ones. */
> >     sc->sc.sc_offs = EREAD1(&sc->sc, EHCI_CAPLENGTH);
> >     EOWRITE2(&sc->sc, EHCI_USBINTR, 0);
> > diff --git a/sys/dev/fdt/if_dwge_fdt.c b/sys/dev/fdt/if_dwge_fdt.c
> > index edfe5acb992..00668980f4a 100644
> > --- a/sys/dev/fdt/if_dwge_fdt.c
> > +++ b/sys/dev/fdt/if_dwge_fdt.c
> > @@ -125,10 +125,10 @@ dwge_fdt_attach(struct device *parent, struct device 
> > *self, void *aux)
> >     else if (OF_is_compatible(faa->fa_node, "rockchip,rk3399-gmac"))
> >             dwge_fdt_attach_rockchip(fsc);
> >  
> > +   reset_deassert(faa->fa_node, "stmmaceth");
> > +
> >     /* Enable clock. */
> >     clock_enable(faa->fa_node, "stmmaceth");
> > -   reset_deassert(faa->fa_node, "stmmaceth");
> > -   delay(5000);
> >  
> >     /* Power up PHY. */
> >     phy_supply = OF_getpropint(faa->fa_node, "phy-supply", 0);
> > diff --git a/sys/dev/fdt/if_dwxe.c b/sys/dev/fdt/if_dwxe.c
> > index 22a383c06ef..6e303745ec3 100644
> > --- a/sys/dev/fdt/if_dwxe.c
> > +++ b/sys/dev/fdt/if_dwxe.c
> > @@ -376,10 +376,10 @@ dwxe_attach(struct device *parent, struct device 
> > *self, void *aux)
> >  
> >     pinctrl_byname(faa->fa_node, "default");
> >  
> > +   reset_deassert(faa->fa_node, "stmmaceth");
> > +
> >     /* Enable clock. */
> >     clock_enable(faa->fa_node, "stmmaceth");
> > -   reset_deassert(faa->fa_node, "stmmaceth");
> > -   delay(5000);
> >  
> >     /* Power up PHY. */
> >     phy_supply = OF_getpropint(faa->fa_node, "phy-supply", 0);
> > diff --git a/sys/dev/fdt/sximmc.c b/sys/dev/fdt/sximmc.c
> > index 7acb8f55d56..9ed2ae09fe4 100644
> > --- a/sys/dev/fdt/sximmc.c
> > +++ b/sys/dev/fdt/sximmc.c
> > @@ -366,11 +366,10 @@ sximmc_attach(struct device *parent, struct device 
> > *self, void *aux)
> >  
> >     pinctrl_byname(faa->fa_node, "default");
> >  
> > +   reset_deassert_all(faa->fa_node);
> > +
> >     /* enable clock */
> >     clock_enable(faa->fa_node, NULL);
> > -   delay(5000);
> > -
> > -   reset_deassert_all(faa->fa_node);
> >  
> >     /*
> >      * The FIFO register is in a different location on the
> > diff --git a/sys/dev/fdt/sxitwi.c b/sys/dev/fdt/sxitwi.c
> > index f53f2bfd594..c80bdc2ab06 100644
> > --- a/sys/dev/fdt/sxitwi.c
> > +++ b/sys/dev/fdt/sxitwi.c
> > @@ -218,6 +218,8 @@ sxitwi_attach(struct device *parent, struct device 
> > *self, void *aux)
> >  
> >     pinctrl_byname(faa->fa_node, "default");
> >  
> > +   reset_deassert_all(faa->fa_node);
> > +
> >     /* Enable clock */
> >     clock_enable(faa->fa_node, NULL);
> >  
> 

Reply via email to