> 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); > > >