| Message ID | 20190401115232.453-4-linux.amoon@gmail.com |
|---|---|
| State | RFC |
| Delegated to: | Lukasz Majewski |
| Headers | show |
| Series | Odroid U3 usb initialization | expand |
On Mon, 1 Apr 2019 at 13:53, Anand Moon <linux.amoon@gmail.com> wrote: > > Some host controllers need addidional re-initialization Please run spell-check. > after ehci_reset() so we add .init_after_reset callback > which is requires to reinit the phy after controller reset. s/requires/required/ .... but you do not re-init the phy. The exynos_usb_init() performs the reset of usb3503 USB hub! > > Signed-off-by: Anand Moon <linux.amoon@gmail.com> > --- > drivers/usb/host/ehci-exynos.c | 20 +++++++++++++++++++- > 1 file changed, 19 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c > index b0f7bd4936..e6a542e092 100644 > --- a/drivers/usb/host/ehci-exynos.c > +++ b/drivers/usb/host/ehci-exynos.c > @@ -143,6 +143,23 @@ static void exynos5_setup_usb_phy(struct exynos_usb_phy *usb) > EHCICTRL_ENAINCR16); > } > > +static int ehci_exynos_init_after_reset(struct ehci_ctrl *ehcntl) > +{ > + if (cpu_is_exynos4()) { > + if (proid_is_exynos4412()) { No need for double indentation. > + /* > + * "usb reset" cmd: restart re-initialize the usb driver Just "reinitialize", not restart reinitialize. Best regards, Krzysztof > + */ > + exynos_usb_init(); > + } > + } > + return 0; > +} > + > +static const struct ehci_ops exynos_ehci_ops = { > + .init_after_reset = ehci_exynos_init_after_reset, > +}; > + > static void exynos4412_setup_usb_phy(struct exynos4412_usb_phy *usb) > { > writel(CLK_24MHZ, &usb->usbphyclk); > @@ -234,7 +251,8 @@ static int ehci_usb_probe(struct udevice *dev) > hcor = (struct ehci_hcor *)((uint32_t)ctx->hcd + > HC_LENGTH(ehci_readl(&ctx->hcd->cr_capbase))); > > - return ehci_register(dev, ctx->hcd, hcor, NULL, 0, USB_INIT_HOST); > + return ehci_register(dev, ctx->hcd, hcor, &exynos_ehci_ops, > + 0, USB_INIT_HOST); > } > > static int ehci_usb_remove(struct udevice *dev) > -- > 2.21.0 >
Hi Krzysztof, On Mon, 1 Apr 2019 at 18:25, Krzysztof Kozlowski <krzk@kernel.org> wrote: > > On Mon, 1 Apr 2019 at 13:53, Anand Moon <linux.amoon@gmail.com> wrote: > > > > Some host controllers need addidional re-initialization > > Please run spell-check. > > > after ehci_reset() so we add .init_after_reset callback > > which is requires to reinit the phy after controller reset. > > s/requires/required/ I did run checkpatch before on this, It did not spotted and error or warning. > .... but you do not re-init the phy. The exynos_usb_init() performs > the reset of usb3503 USB hub! > Yes that is needed as we do not get the usb back after "usb reset" command. > > > > Signed-off-by: Anand Moon <linux.amoon@gmail.com> > > --- > > drivers/usb/host/ehci-exynos.c | 20 +++++++++++++++++++- > > 1 file changed, 19 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c > > index b0f7bd4936..e6a542e092 100644 > > --- a/drivers/usb/host/ehci-exynos.c > > +++ b/drivers/usb/host/ehci-exynos.c > > @@ -143,6 +143,23 @@ static void exynos5_setup_usb_phy(struct exynos_usb_phy *usb) > > EHCICTRL_ENAINCR16); > > } > > > > +static int ehci_exynos_init_after_reset(struct ehci_ctrl *ehcntl) > > +{ > > + if (cpu_is_exynos4()) { > > + if (proid_is_exynos4412()) { > > No need for double indentation. > > > + /* > > + * "usb reset" cmd: restart re-initialize the usb driver > > Just "reinitialize", not restart reinitialize. > Ok. > Best regards, > Krzysztof > > > + */ > > + exynos_usb_init(); > > + } > > + } > > + return 0; > > +} > > + > > +static const struct ehci_ops exynos_ehci_ops = { > > + .init_after_reset = ehci_exynos_init_after_reset, > > +}; > > + > > static void exynos4412_setup_usb_phy(struct exynos4412_usb_phy *usb) > > { > > writel(CLK_24MHZ, &usb->usbphyclk); > > @@ -234,7 +251,8 @@ static int ehci_usb_probe(struct udevice *dev) > > hcor = (struct ehci_hcor *)((uint32_t)ctx->hcd + > > HC_LENGTH(ehci_readl(&ctx->hcd->cr_capbase))); > > > > - return ehci_register(dev, ctx->hcd, hcor, NULL, 0, USB_INIT_HOST); > > + return ehci_register(dev, ctx->hcd, hcor, &exynos_ehci_ops, > > + 0, USB_INIT_HOST); > > } > > > > static int ehci_usb_remove(struct udevice *dev) > > -- > > 2.21.0 > > Best Regards -Anand
On Mon, 1 Apr 2019 at 18:05, Anand Moon <linux.amoon@gmail.com> wrote: > > Hi Krzysztof, > > On Mon, 1 Apr 2019 at 18:25, Krzysztof Kozlowski <krzk@kernel.org> wrote: > > > > On Mon, 1 Apr 2019 at 13:53, Anand Moon <linux.amoon@gmail.com> wrote: > > > > > > Some host controllers need addidional re-initialization > > > > Please run spell-check. > > > > > after ehci_reset() so we add .init_after_reset callback > > > which is requires to reinit the phy after controller reset. > > > > s/requires/required/ > > I did run checkpatch before on this, It did not spotted and error or warning. > > > .... but you do not re-init the phy. The exynos_usb_init() performs > > the reset of usb3503 USB hub! > > > > Yes that is needed as we do not get the usb back after "usb reset" command. Then please update the commit message to describe what exactly you are doing and what you want to achieve. Someone might think that you are doing initialization in init-after-reset... but you want to perform different reset after reset of ehci... and while writing it maybe you will notice that it is a hack and probably not the best way to do it. Best regards, Krzysztof
diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c index b0f7bd4936..e6a542e092 100644 --- a/drivers/usb/host/ehci-exynos.c +++ b/drivers/usb/host/ehci-exynos.c @@ -143,6 +143,23 @@ static void exynos5_setup_usb_phy(struct exynos_usb_phy *usb) EHCICTRL_ENAINCR16); } +static int ehci_exynos_init_after_reset(struct ehci_ctrl *ehcntl) +{ + if (cpu_is_exynos4()) { + if (proid_is_exynos4412()) { + /* + * "usb reset" cmd: restart re-initialize the usb driver + */ + exynos_usb_init(); + } + } + return 0; +} + +static const struct ehci_ops exynos_ehci_ops = { + .init_after_reset = ehci_exynos_init_after_reset, +}; + static void exynos4412_setup_usb_phy(struct exynos4412_usb_phy *usb) { writel(CLK_24MHZ, &usb->usbphyclk); @@ -234,7 +251,8 @@ static int ehci_usb_probe(struct udevice *dev) hcor = (struct ehci_hcor *)((uint32_t)ctx->hcd + HC_LENGTH(ehci_readl(&ctx->hcd->cr_capbase))); - return ehci_register(dev, ctx->hcd, hcor, NULL, 0, USB_INIT_HOST); + return ehci_register(dev, ctx->hcd, hcor, &exynos_ehci_ops, + 0, USB_INIT_HOST); } static int ehci_usb_remove(struct udevice *dev)
Some host controllers need addidional re-initialization after ehci_reset() so we add .init_after_reset callback which is requires to reinit the phy after controller reset. Signed-off-by: Anand Moon <linux.amoon@gmail.com> --- drivers/usb/host/ehci-exynos.c | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-)