| Message ID | 1449848520-27379-2-git-send-email-hdegoede@redhat.com |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
Am Freitag, den 11.12.2015, 16:41 +0100 schrieb Hans de Goede: > From: Reinder de Haan <patchesrdh@mveas.com> > > At least the EHCI/OHCI found on the Allwinnner H3 SoC needs multiple > reset lines, the controller will not initialize while the reset for > its companion is still asserted, which means we need to de-assert > 2 resets for the controller to work. > > Signed-off-by: Reinder de Haan <patchesrdh@mveas.com> > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > Changes in v2: > -Use the new reset_control_[de]assert_shared reset-controller functions > --- > Documentation/devicetree/bindings/usb/usb-ehci.txt | 2 +- > drivers/usb/host/ehci-platform.c | 47 +++++++++++++--------- > 2 files changed, 30 insertions(+), 19 deletions(-) > > diff --git a/Documentation/devicetree/bindings/usb/usb-ehci.txt b/Documentation/devicetree/bindings/usb/usb-ehci.txt > index a12d601..0701812 100644 > --- a/Documentation/devicetree/bindings/usb/usb-ehci.txt > +++ b/Documentation/devicetree/bindings/usb/usb-ehci.txt > @@ -18,7 +18,7 @@ Optional properties: > - clocks : a list of phandle + clock specifier pairs > - phys : phandle + phy specifier pair > - phy-names : "usb" > - - resets : phandle + reset specifier pair > + - resets : a list of phandle + reset specifier pairs Are there documented names for these resets? Is the companion you mention the Port Control? regards Philipp -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On 11-12-15 18:13, Philipp Zabel wrote: > Am Freitag, den 11.12.2015, 16:41 +0100 schrieb Hans de Goede: >> From: Reinder de Haan <patchesrdh@mveas.com> >> >> At least the EHCI/OHCI found on the Allwinnner H3 SoC needs multiple >> reset lines, the controller will not initialize while the reset for >> its companion is still asserted, which means we need to de-assert >> 2 resets for the controller to work. >> >> Signed-off-by: Reinder de Haan <patchesrdh@mveas.com> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >> --- >> Changes in v2: >> -Use the new reset_control_[de]assert_shared reset-controller functions >> --- >> Documentation/devicetree/bindings/usb/usb-ehci.txt | 2 +- >> drivers/usb/host/ehci-platform.c | 47 +++++++++++++--------- >> 2 files changed, 30 insertions(+), 19 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/usb/usb-ehci.txt b/Documentation/devicetree/bindings/usb/usb-ehci.txt >> index a12d601..0701812 100644 >> --- a/Documentation/devicetree/bindings/usb/usb-ehci.txt >> +++ b/Documentation/devicetree/bindings/usb/usb-ehci.txt >> @@ -18,7 +18,7 @@ Optional properties: >> - clocks : a list of phandle + clock specifier pairs >> - phys : phandle + phy specifier pair >> - phy-names : "usb" >> - - resets : phandle + reset specifier pair >> + - resets : a list of phandle + reset specifier pairs > > Are there documented names for these resets? This binding is a generic ehci controller binding, so even if the names are documented for the allwinner SoC we should not use names, just like the binding is deliberately not using names for the clocks either to keep it generic, so that we can reuse the binding + driver with many different SoCs. > Is the companion you > mention the Port Control? Sort of, with USB-2, USB-1 compatibility is handled via a mux on the datalines (controlled by the EHCI controller Port Control) which muxes the lines to an USB-1 controller (typically either UHCI or OHCI) when the device does not connect after USB-2 highspeed handshaking. This USB-1 controller (or controller_S_ in some case since the USB-1 companions may have less root-ports per controller then the EHCI has root-ports) is called the companion controller. The 2 controllers are supposed to be 100% independent but on the H3 Allwinner has done something (not documented) which requires one to deassert reset on both before you can talk to either one. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Dec 11, 2015 at 04:41:59PM +0100, Hans de Goede wrote: > From: Reinder de Haan <patchesrdh@mveas.com> > > At least the EHCI/OHCI found on the Allwinnner H3 SoC needs multiple > reset lines, the controller will not initialize while the reset for > its companion is still asserted, which means we need to de-assert > 2 resets for the controller to work. > > Signed-off-by: Reinder de Haan <patchesrdh@mveas.com> > Signed-off-by: Hans de Goede <hdegoede@redhat.com> Acked-by: Rob Herring <robh@kernel.org> > --- > Changes in v2: > -Use the new reset_control_[de]assert_shared reset-controller functions > --- > Documentation/devicetree/bindings/usb/usb-ehci.txt | 2 +- > drivers/usb/host/ehci-platform.c | 47 +++++++++++++--------- > 2 files changed, 30 insertions(+), 19 deletions(-) > > diff --git a/Documentation/devicetree/bindings/usb/usb-ehci.txt b/Documentation/devicetree/bindings/usb/usb-ehci.txt > index a12d601..0701812 100644 > --- a/Documentation/devicetree/bindings/usb/usb-ehci.txt > +++ b/Documentation/devicetree/bindings/usb/usb-ehci.txt > @@ -18,7 +18,7 @@ Optional properties: > - clocks : a list of phandle + clock specifier pairs > - phys : phandle + phy specifier pair > - phy-names : "usb" > - - resets : phandle + reset specifier pair > + - resets : a list of phandle + reset specifier pairs > > Example (Sequoia 440EPx): > ehci@e0000300 { > diff --git a/drivers/usb/host/ehci-platform.c b/drivers/usb/host/ehci-platform.c > index bd7082f2..6fbf32a 100644 > --- a/drivers/usb/host/ehci-platform.c > +++ b/drivers/usb/host/ehci-platform.c > @@ -39,11 +39,12 @@ > > #define DRIVER_DESC "EHCI generic platform driver" > #define EHCI_MAX_CLKS 3 > +#define EHCI_MAX_RESETS 2 > #define hcd_to_ehci_priv(h) ((struct ehci_platform_priv *)hcd_to_ehci(h)->priv) > > struct ehci_platform_priv { > struct clk *clks[EHCI_MAX_CLKS]; > - struct reset_control *rst; > + struct reset_control *resets[EHCI_MAX_RESETS]; > struct phy **phys; > int num_phys; > bool reset_on_resume; > @@ -149,7 +150,7 @@ static int ehci_platform_probe(struct platform_device *dev) > struct usb_ehci_pdata *pdata = dev_get_platdata(&dev->dev); > struct ehci_platform_priv *priv; > struct ehci_hcd *ehci; > - int err, irq, phy_num, clk = 0; > + int err, irq, phy_num, clk = 0, rst = 0; > > if (usb_disabled()) > return -ENODEV; > @@ -232,18 +233,24 @@ static int ehci_platform_probe(struct platform_device *dev) > break; > } > } > - } > > - priv->rst = devm_reset_control_get_optional(&dev->dev, NULL); > - if (IS_ERR(priv->rst)) { > - err = PTR_ERR(priv->rst); > - if (err == -EPROBE_DEFER) > - goto err_put_clks; > - priv->rst = NULL; > - } else { > - err = reset_control_deassert(priv->rst); > - if (err) > - goto err_put_clks; > + for (rst = 0; rst < EHCI_MAX_RESETS; rst++) { > + priv->resets[rst] = > + of_reset_control_get_by_index(dev->dev.of_node, > + rst); > + if (IS_ERR(priv->resets[rst])) { > + err = PTR_ERR(priv->resets[rst]); > + if (err == -EPROBE_DEFER) > + goto err_reset; > + priv->resets[rst] = NULL; > + break; > + } > + err = reset_control_deassert_shared(priv->resets[rst]); > + if (err) { > + reset_control_put(priv->resets[rst]); > + goto err_reset; > + } > + } > } > > if (pdata->big_endian_desc) > @@ -300,8 +307,10 @@ err_power: > if (pdata->power_off) > pdata->power_off(dev); > err_reset: > - if (priv->rst) > - reset_control_assert(priv->rst); > + while (--rst >= 0) { > + reset_control_assert_shared(priv->resets[rst]); > + reset_control_put(priv->resets[rst]); > + } > err_put_clks: > while (--clk >= 0) > clk_put(priv->clks[clk]); > @@ -319,15 +328,17 @@ static int ehci_platform_remove(struct platform_device *dev) > struct usb_hcd *hcd = platform_get_drvdata(dev); > struct usb_ehci_pdata *pdata = dev_get_platdata(&dev->dev); > struct ehci_platform_priv *priv = hcd_to_ehci_priv(hcd); > - int clk; > + int clk, rst; > > usb_remove_hcd(hcd); > > if (pdata->power_off) > pdata->power_off(dev); > > - if (priv->rst) > - reset_control_assert(priv->rst); > + for (rst = 0; rst < EHCI_MAX_RESETS && priv->resets[rst]; rst++) { > + reset_control_assert_shared(priv->resets[rst]); > + reset_control_put(priv->resets[rst]); > + } > > for (clk = 0; clk < EHCI_MAX_CLKS && priv->clks[clk]; clk++) > clk_put(priv->clks[clk]); > -- > 2.5.0 > > -- > To unsubscribe from this list: send the line "unsubscribe devicetree" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Am Freitag, den 11.12.2015, 19:28 +0100 schrieb Hans de Goede: [...] > >> diff --git a/Documentation/devicetree/bindings/usb/usb-ehci.txt b/Documentation/devicetree/bindings/usb/usb-ehci.txt > >> index a12d601..0701812 100644 > >> --- a/Documentation/devicetree/bindings/usb/usb-ehci.txt > >> +++ b/Documentation/devicetree/bindings/usb/usb-ehci.txt > >> @@ -18,7 +18,7 @@ Optional properties: > >> - clocks : a list of phandle + clock specifier pairs > >> - phys : phandle + phy specifier pair > >> - phy-names : "usb" > >> - - resets : phandle + reset specifier pair > >> + - resets : a list of phandle + reset specifier pairs > > > > Are there documented names for these resets? > > This binding is a generic ehci controller binding, so even if > the names are documented for the allwinner SoC we should > not use names, just like the binding is deliberately not > using names for the clocks either to keep it generic, so > that we can reuse the binding + driver with many different SoCs. I know, I'm just interested in understanding why this is necessary ... > > Is the companion you > > mention the Port Control? > > Sort of, with USB-2, USB-1 compatibility is handled via a mux on the > datalines (controlled by the EHCI controller Port Control) which muxes > the lines to an USB-1 controller (typically either UHCI or OHCI) when the > device does not connect after USB-2 highspeed handshaking. > > This USB-1 controller (or controller_S_ in some case since the > USB-1 companions may have less root-ports per controller then the EHCI > has root-ports) is called the companion controller. > > The 2 controllers are supposed to be 100% independent but on the H3 > Allwinner has done something (not documented) which requires one to > deassert reset on both before you can talk to either one. ... so thank you for the explanation. Acked-by: Philipp Zabel <p.zabel@pengutronix.de> regards Philipp -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/usb/usb-ehci.txt b/Documentation/devicetree/bindings/usb/usb-ehci.txt index a12d601..0701812 100644 --- a/Documentation/devicetree/bindings/usb/usb-ehci.txt +++ b/Documentation/devicetree/bindings/usb/usb-ehci.txt @@ -18,7 +18,7 @@ Optional properties: - clocks : a list of phandle + clock specifier pairs - phys : phandle + phy specifier pair - phy-names : "usb" - - resets : phandle + reset specifier pair + - resets : a list of phandle + reset specifier pairs Example (Sequoia 440EPx): ehci@e0000300 { diff --git a/drivers/usb/host/ehci-platform.c b/drivers/usb/host/ehci-platform.c index bd7082f2..6fbf32a 100644 --- a/drivers/usb/host/ehci-platform.c +++ b/drivers/usb/host/ehci-platform.c @@ -39,11 +39,12 @@ #define DRIVER_DESC "EHCI generic platform driver" #define EHCI_MAX_CLKS 3 +#define EHCI_MAX_RESETS 2 #define hcd_to_ehci_priv(h) ((struct ehci_platform_priv *)hcd_to_ehci(h)->priv) struct ehci_platform_priv { struct clk *clks[EHCI_MAX_CLKS]; - struct reset_control *rst; + struct reset_control *resets[EHCI_MAX_RESETS]; struct phy **phys; int num_phys; bool reset_on_resume; @@ -149,7 +150,7 @@ static int ehci_platform_probe(struct platform_device *dev) struct usb_ehci_pdata *pdata = dev_get_platdata(&dev->dev); struct ehci_platform_priv *priv; struct ehci_hcd *ehci; - int err, irq, phy_num, clk = 0; + int err, irq, phy_num, clk = 0, rst = 0; if (usb_disabled()) return -ENODEV; @@ -232,18 +233,24 @@ static int ehci_platform_probe(struct platform_device *dev) break; } } - } - priv->rst = devm_reset_control_get_optional(&dev->dev, NULL); - if (IS_ERR(priv->rst)) { - err = PTR_ERR(priv->rst); - if (err == -EPROBE_DEFER) - goto err_put_clks; - priv->rst = NULL; - } else { - err = reset_control_deassert(priv->rst); - if (err) - goto err_put_clks; + for (rst = 0; rst < EHCI_MAX_RESETS; rst++) { + priv->resets[rst] = + of_reset_control_get_by_index(dev->dev.of_node, + rst); + if (IS_ERR(priv->resets[rst])) { + err = PTR_ERR(priv->resets[rst]); + if (err == -EPROBE_DEFER) + goto err_reset; + priv->resets[rst] = NULL; + break; + } + err = reset_control_deassert_shared(priv->resets[rst]); + if (err) { + reset_control_put(priv->resets[rst]); + goto err_reset; + } + } } if (pdata->big_endian_desc) @@ -300,8 +307,10 @@ err_power: if (pdata->power_off) pdata->power_off(dev); err_reset: - if (priv->rst) - reset_control_assert(priv->rst); + while (--rst >= 0) { + reset_control_assert_shared(priv->resets[rst]); + reset_control_put(priv->resets[rst]); + } err_put_clks: while (--clk >= 0) clk_put(priv->clks[clk]); @@ -319,15 +328,17 @@ static int ehci_platform_remove(struct platform_device *dev) struct usb_hcd *hcd = platform_get_drvdata(dev); struct usb_ehci_pdata *pdata = dev_get_platdata(&dev->dev); struct ehci_platform_priv *priv = hcd_to_ehci_priv(hcd); - int clk; + int clk, rst; usb_remove_hcd(hcd); if (pdata->power_off) pdata->power_off(dev); - if (priv->rst) - reset_control_assert(priv->rst); + for (rst = 0; rst < EHCI_MAX_RESETS && priv->resets[rst]; rst++) { + reset_control_assert_shared(priv->resets[rst]); + reset_control_put(priv->resets[rst]); + } for (clk = 0; clk < EHCI_MAX_CLKS && priv->clks[clk]; clk++) clk_put(priv->clks[clk]);