Hi Simon
On 05/15/2017 05:03 AM, Simon Glass wrote:
Hi Patrice,
On 12 May 2017 at 11:27, patrice.chotard@st.com wrote:
From: Patrice Chotard patrice.chotard@st.com
Add error path to disable enabled clocks and to assert deasserted resets Populate the remove callback
Signed-off-by: Patrice Chotard patrice.chotard@st.com
v2: _ split previous path 1, add error path and .remove callback
drivers/usb/host/ehci-generic.c | 81 +++++++++++++++++++++++++++++++++++------ 1 file changed, 70 insertions(+), 11 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Please see below.
diff --git a/drivers/usb/host/ehci-generic.c b/drivers/usb/host/ehci-generic.c index 2190adb..0c29f63 100644 --- a/drivers/usb/host/ehci-generic.c +++ b/drivers/usb/host/ehci-generic.c @@ -11,6 +11,9 @@ #include <dm.h> #include "ehci.h"
+#define EHCI_MAX_CLOCKS 3 +#define EHCI_MAX_RESETS 3
/*
- Even though here we don't explicitly use "struct ehci_ctrl"
- ehci_register() expects it to be the first thing that resides in
@@ -18,35 +21,74 @@ */ struct generic_ehci { struct ehci_ctrl ctrl;
struct clk clks[EHCI_MAX_CLOCKS];struct reset_ctl resets[EHCI_MAX_RESETS];};
+static void ehci_assert_resets(struct udevice *dev) {
struct generic_ehci *priv = dev_get_priv(dev);You could pass priv instead of dev to avoid this line.
Agree, will fix it
struct reset_ctl reset;int i;for (i = EHCI_MAX_RESETS; i >= 0; --i) {reset = priv->resets[i];if (reset.dev) {reset_request(&reset);reset_assert(&reset);reset_free(&reset);}}+}
+static void ehci_disable_clocks(struct udevice *dev) {
struct generic_ehci *priv = dev_get_priv(dev);struct clk clk;int i;for (i = EHCI_MAX_CLOCKS; i >= 0; --i) {Do you think it is worth adding a function in the uclass to do this to a list of clks?
I will have a look at this
Patrice
clk = priv->clks[i];if (clk.dev) {clk_request(clk.dev, &clk);clk_disable(&clk);clk_free(&clk);}}+}
static int ehci_usb_probe(struct udevice *dev) {
struct generic_ehci *priv = dev_get_priv(dev); struct ehci_hccr *hccr; struct ehci_hcor *hcor;
int i;
int i, ret;
for (i = 0; ; i++) {struct clk clk;int ret;
for (i = 0; i < EHCI_MAX_CLOCKS; i++) {struct clk clk = priv->clks[i]; ret = clk_get_by_index(dev, i, &clk); if (ret < 0) break;
if (clk_enable(&clk))
if (clk_enable(&clk)) { printf("failed to enable clock %d\n", i);clk_free(&clk);goto clk_err;} clk_free(&clk); }
for (i = 0; ; i++) {struct reset_ctl reset;int ret;
for (i = 0; i < EHCI_MAX_RESETS ; i++) {struct reset_ctl reset = priv->resets[i]; ret = reset_get_by_index(dev, i, &reset); if (ret < 0) break;
if (reset_deassert(&reset))
if (reset_deassert(&reset)) { printf("failed to deassert reset %d\n", i);reset_free(&reset);goto reset_err;} reset_free(&reset); }@@ -54,7 +96,24 @@ static int ehci_usb_probe(struct udevice *dev) hcor = (struct ehci_hcor *)((uintptr_t)hccr + HC_LENGTH(ehci_readl(&hccr->cr_capbase)));
return ehci_register(dev, hccr, hcor, NULL, 0, USB_INIT_HOST);
ret = ehci_register(dev, hccr, hcor, NULL, 0, USB_INIT_HOST);if (!ret)return ret;+reset_err:
ehci_assert_resets(dev);+clk_err:
ehci_disable_clocks(dev);return ret;+}
+static int ehci_usb_remove(struct udevice *dev) {
ehci_assert_resets(dev);ehci_disable_clocks(dev);return ehci_deregister(dev);}
static const struct udevice_id ehci_usb_ids[] = { @@ -67,7 +126,7 @@ U_BOOT_DRIVER(ehci_generic) = { .id = UCLASS_USB, .of_match = ehci_usb_ids, .probe = ehci_usb_probe,
.remove = ehci_deregister,
.remove = ehci_usb_remove, .ops = &ehci_usb_ops, .priv_auto_alloc_size = sizeof(struct generic_ehci), .flags = DM_FLAG_ALLOC_PRIV_DMA,-- 1.9.1
Regards, Simon