From mboxrd@z Thu Jan 1 00:00:00 1970 From: Lothar =?UTF-8?B?V2HDn21hbm4=?= Date: Tue, 20 Jun 2017 13:32:58 +0200 Subject: [U-Boot] [PATCH v7 06/10] usb: host: ehci-generic: add error path and .remove callback In-Reply-To: <1497952751-28477-7-git-send-email-patrice.chotard@st.com> References: <1497952751-28477-1-git-send-email-patrice.chotard@st.com> <1497952751-28477-7-git-send-email-patrice.chotard@st.com> Message-ID: <20170620133258.114eba5c@karo-electronics.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: quoted-printable To: u-boot@lists.denx.de Hi, On Tue, 20 Jun 2017 11:59:07 +0200 patrice.chotard at st.com wrote: > From: Patrice Chotard >=20 > Use an array to save enabled clocks reference and deasserted resets > in order to respectively disabled and asserted them in case of error > during probe() or during driver removal. >=20 > Signed-off-by: Patrice Chotard > --- >=20 > v7: _ replace clk_count() and reset_count() by ofnode_count_phandle_with_= args() >=20 > v6: _ none >=20 > v5: _ none >=20 > v4: _ update the memory allocation for deasserted resets and enabled > clocks reference list. Replace lists by arrays. > _ usage of new RESET and CLOCK methods clk_count(), reset_count(), > reset_assert_all() and clk_disable_all(). >=20 > v3: _ keep enabled clocks and deasserted resets reference in list in orde= r to > disable clock or assert resets in error path or in .remove callback > _ use struct generic_ehci * instead of struct udevice * as parameter for > ehci_release_resets() and ehci_release_clocks() >=20 > drivers/usb/host/ehci-generic.c | 125 ++++++++++++++++++++++++++++++++--= ------ > 1 file changed, 101 insertions(+), 24 deletions(-) >=20 > diff --git a/drivers/usb/host/ehci-generic.c b/drivers/usb/host/ehci-gene= ric.c > index 2116ae1..388b242 100644 > --- a/drivers/usb/host/ehci-generic.c > +++ b/drivers/usb/host/ehci-generic.c > @@ -11,6 +11,8 @@ > #include > #include "ehci.h" > =20 > +#include > + > /* > * Even though here we don't explicitly use "struct ehci_ctrl" > * ehci_register() expects it to be the first thing that resides in > @@ -18,43 +20,119 @@ > */ > struct generic_ehci { > struct ehci_ctrl ctrl; > + struct clk *clocks; > + struct reset_ctl *resets; > + int clock_count; > + int reset_count; > }; > =20 > static int ehci_usb_probe(struct udevice *dev) > { > + struct generic_ehci *priv =3D dev_get_priv(dev); > struct ehci_hccr *hccr; > struct ehci_hcor *hcor; > - int i; > - > - for (i =3D 0; ; i++) { > - struct clk clk; > - int ret; > - > - ret =3D clk_get_by_index(dev, i, &clk); > - if (ret < 0) > - break; > - if (clk_enable(&clk)) > - error("failed to enable clock %d\n", i); > - clk_free(&clk); > - } > + int i, err, ret, clock_nb, reset_nb; > + > + err =3D 0; > + priv->clock_count =3D 0; > + clock_nb =3D ofnode_count_phandle_with_args(dev_ofnode(dev), "clocks", > + "#clock-cells"); > + if (clock_nb > 0) { > + priv->clocks =3D devm_kmalloc(dev, sizeof(struct clk) * clock_nb, > + GFP_KERNEL); > + if (!priv->clocks) { > + error("Can't allocate resource\n"); > + return -ENOMEM; > + } > + > + for (i =3D 0; i < clock_nb; i++) { > + err =3D clk_get_by_index(dev, i, &priv->clocks[i]); > + > + if (err < 0) > + break; > =20 > - for (i =3D 0; ; i++) { > - struct reset_ctl reset; > - int ret; > + priv->clock_count++; > =20 > - ret =3D reset_get_by_index(dev, i, &reset); > - if (ret < 0) > - break; > - if (reset_deassert(&reset)) > - error("failed to deassert reset %d\n", i); > - reset_free(&reset); > + if (clk_enable(&priv->clocks[i])) { > + error("failed to enable clock %d\n", i); > + clk_free(&priv->clocks[i]); > You free the clock here and again with clk_disable_all() in the error path. Also you do not have a valid error code in 'err' which is returned at the end of the function! > + goto clk_err; > + } > + clk_free(&priv->clocks[i]); > + } > + } else { > + if (clock_nb !=3D -ENOENT) { > + error("failed to get clock phandle(%d)\n", clock_nb); > + return clock_nb; > + } > } > =20 > + priv->reset_count =3D 0; > + reset_nb =3D ofnode_count_phandle_with_args(dev_ofnode(dev), "resets", > + "#reset-cells"); > + if (reset_nb > 0) { > + priv->resets =3D devm_kmalloc(dev, > + sizeof(struct reset_ctl) * reset_nb, > + GFP_KERNEL); > + if (!priv->resets) { > + error("Can't allocate resource\n"); > + return -ENOMEM; > + } > + > + for (i =3D 0; i < reset_nb; i++) { > + err =3D reset_get_by_index(dev, i, &priv->resets[i]); > + if (err < 0) > + break; > + > + priv->reset_count++; > + > + if (reset_deassert(&priv->resets[i])) { > + error("failed to deassert reset %d\n", i); > + reset_free(&priv->resets[i]); > + goto reset_err; > + } > + reset_free(&priv->resets[i]); > + } > + } else { > + if (reset_nb !=3D -ENOENT) { > + error("failed to get reset phandle(%d)\n", reset_nb); > + goto clk_err; > + } > + } > hccr =3D map_physmem(devfdt_get_addr(dev), 0x100, MAP_NOCACHE); > hcor =3D (struct ehci_hcor *)((uintptr_t)hccr + > HC_LENGTH(ehci_readl(&hccr->cr_capbase))); > =20 > - return ehci_register(dev, hccr, hcor, NULL, 0, USB_INIT_HOST); > + err =3D ehci_register(dev, hccr, hcor, NULL, 0, USB_INIT_HOST); > + if (!err) > + return err; > + > +reset_err: > + ret =3D reset_assert_all(priv->resets, priv->reset_count); > + if (ret) > + return ret; > You should do the clock cleanup even though the reset cleanup failed. > +clk_err: > + ret =3D clk_disable_all(priv->clocks, priv->clock_count); > + if (ret) > + return ret; > You should return the error code of the operation that failed initially (err), not some error code that has occured during cleanup. > + return err; > +} > + > +static int ehci_usb_remove(struct udevice *dev) > +{ > + struct generic_ehci *priv =3D dev_get_priv(dev); > + int ret; > + > + ret =3D ehci_deregister(dev); > + if (ret) > + return ret; > + > + ret =3D reset_assert_all(priv->resets, priv->reset_count); > + if (ret) > + return ret; > + > + return clk_disable_all(priv->clocks, priv->clock_count); > } > =20 > static const struct udevice_id ehci_usb_ids[] =3D { > @@ -67,7 +145,7 @@ U_BOOT_DRIVER(ehci_generic) =3D { > .id =3D UCLASS_USB, > .of_match =3D ehci_usb_ids, > .probe =3D ehci_usb_probe, > - .remove =3D ehci_deregister, > + .remove =3D ehci_usb_remove, > .ops =3D &ehci_usb_ops, > .priv_auto_alloc_size =3D sizeof(struct generic_ehci), > .flags =3D DM_FLAG_ALLOC_PRIV_DMA, Lothar Wa=C3=9Fmann