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? -Artturi > [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); >