[PATCH 1/2] usb: host: ehci-generic: Make usage of clock/reset bulk() API

Make usage of clock and reset bulk API in order to simplify the code
Signed-off-by: Patrice Chotard patrice.chotard@foss.st.com ---
drivers/usb/host/ehci-generic.c | 97 ++++++++++----------------------- 1 file changed, 29 insertions(+), 68 deletions(-)
diff --git a/drivers/usb/host/ehci-generic.c b/drivers/usb/host/ehci-generic.c index 4c28a69b98..5856e898a8 100644 --- a/drivers/usb/host/ehci-generic.c +++ b/drivers/usb/host/ehci-generic.c @@ -23,14 +23,12 @@ */ struct generic_ehci { struct ehci_ctrl ctrl; - struct clk *clocks; - struct reset_ctl *resets; + struct clk_bulk clocks; + struct reset_ctl_bulk resets; struct phy phy; #ifdef CONFIG_DM_REGULATOR struct udevice *vbus_supply; #endif - int clock_count; - int reset_count; };
#ifdef CONFIG_DM_REGULATOR @@ -81,68 +79,31 @@ 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, err, ret, clock_nb, reset_nb; + int err, ret;
err = 0; - priv->clock_count = 0; - clock_nb = ofnode_count_phandle_with_args(dev_ofnode(dev), "clocks", - "#clock-cells", 0); - if (clock_nb > 0) { - priv->clocks = devm_kcalloc(dev, clock_nb, sizeof(struct clk), - GFP_KERNEL); - if (!priv->clocks) - return -ENOMEM; - - for (i = 0; i < clock_nb; i++) { - err = clk_get_by_index(dev, i, &priv->clocks[i]); - - if (err < 0) - break; - err = clk_enable(&priv->clocks[i]); - if (err && err != -ENOSYS) { - dev_err(dev, "failed to enable clock %d\n", i); - clk_free(&priv->clocks[i]); - goto clk_err; - } - priv->clock_count++; - } - } else { - if (clock_nb != -ENOENT) { - dev_err(dev, "failed to get clock phandle(%d)\n", - clock_nb); - return clock_nb; - } + ret = clk_get_bulk(dev, &priv->clocks); + if (ret) { + dev_err(dev, "Failed to get clocks\n"); + return ret; }
- priv->reset_count = 0; - reset_nb = ofnode_count_phandle_with_args(dev_ofnode(dev), "resets", - "#reset-cells", 0); - if (reset_nb > 0) { - priv->resets = devm_kcalloc(dev, reset_nb, - sizeof(struct reset_ctl), - GFP_KERNEL); - if (!priv->resets) - return -ENOMEM; - - for (i = 0; i < reset_nb; i++) { - err = reset_get_by_index(dev, i, &priv->resets[i]); - if (err < 0) - break; - - if (reset_deassert(&priv->resets[i])) { - dev_err(dev, "failed to deassert reset %d\n", - i); - reset_free(&priv->resets[i]); - goto reset_err; - } - priv->reset_count++; - } - } else { - if (reset_nb != -ENOENT) { - dev_err(dev, "failed to get reset phandle(%d)\n", - reset_nb); - goto clk_err; - } + err = clk_enable_bulk(&priv->clocks); + if (err) { + dev_err(dev, "Failed to enable clocks\n"); + goto clk_err; + } + + err = reset_get_bulk(dev, &priv->resets); + if (err) { + dev_err(dev, "Failed to get resets\n"); + goto clk_err; + } + + err = reset_deassert_bulk(&priv->resets); + if (err) { + dev_err(dev, "Failed to get deassert resets\n"); + goto reset_err; }
err = ehci_enable_vbus_supply(dev); @@ -174,13 +135,13 @@ regulator_err: dev_err(dev, "failed to disable VBUS supply\n");
reset_err: - ret = reset_release_all(priv->resets, priv->reset_count); + ret = reset_release_bulk(&priv->resets); if (ret) - dev_err(dev, "failed to assert all resets\n"); + dev_err(dev, "failed to release resets\n"); clk_err: - ret = clk_release_all(priv->clocks, priv->clock_count); + ret = clk_release_bulk(&priv->clocks); if (ret) - dev_err(dev, "failed to disable all clocks\n"); + dev_err(dev, "failed to release clocks\n");
return err; } @@ -202,11 +163,11 @@ static int ehci_usb_remove(struct udevice *dev) if (ret) return ret;
- ret = reset_release_all(priv->resets, priv->reset_count); + ret = reset_release_bulk(&priv->resets); if (ret) return ret;
- return clk_release_all(priv->clocks, priv->clock_count); + return clk_release_bulk(&priv->clocks); }
static const struct udevice_id ehci_usb_ids[] = {

Currently, DM_REGULATOR flag is no more needed to protect no DM core, remove it.
Signed-off-by: Patrice Chotard patrice.chotard@foss.st.com ---
drivers/usb/host/ehci-generic.c | 14 -------------- 1 file changed, 14 deletions(-)
diff --git a/drivers/usb/host/ehci-generic.c b/drivers/usb/host/ehci-generic.c index 5856e898a8..9c385d8c33 100644 --- a/drivers/usb/host/ehci-generic.c +++ b/drivers/usb/host/ehci-generic.c @@ -26,12 +26,9 @@ struct generic_ehci { struct clk_bulk clocks; struct reset_ctl_bulk resets; struct phy phy; -#ifdef CONFIG_DM_REGULATOR struct udevice *vbus_supply; -#endif };
-#ifdef CONFIG_DM_REGULATOR static int ehci_enable_vbus_supply(struct udevice *dev) { struct generic_ehci *priv = dev_get_priv(dev); @@ -62,17 +59,6 @@ static int ehci_disable_vbus_supply(struct generic_ehci *priv) else return 0; } -#else -static int ehci_enable_vbus_supply(struct udevice *dev) -{ - return 0; -} - -static int ehci_disable_vbus_supply(struct generic_ehci *priv) -{ - return 0; -} -#endif
static int ehci_usb_probe(struct udevice *dev) {

On 5/5/22 15:17, Patrice Chotard wrote:
Currently, DM_REGULATOR flag is no more needed to protect no DM core, remove it.
DM_REGULATOR flag is actually Kconfig symbol.
Why is it no longer needed ? (answer should be in the commit message)

HI Marek
On 5/5/22 15:45, Marek Vasut wrote:
On 5/5/22 15:17, Patrice Chotard wrote:
Currently, DM_REGULATOR flag is no more needed to protect no DM core, remove it.
DM_REGULATOR flag is actually Kconfig symbol.
Why is it no longer needed ? (answer should be in the commit message)
Ok, i will update the commit message as following :
Since commit 16cc5ad0b439 ("power: regulator: add dummy helper") regulator dummy helper are always available even if DM_REGULATOR is not set. DM_REGULATOR flag is no more needed to protect no DM core, remove it.
Is it OK for you ?
Patrice

On 5/5/22 16:58, Patrice CHOTARD wrote:
HI Marek
On 5/5/22 15:45, Marek Vasut wrote:
On 5/5/22 15:17, Patrice Chotard wrote:
Currently, DM_REGULATOR flag is no more needed to protect no DM core, remove it.
DM_REGULATOR flag is actually Kconfig symbol.
Why is it no longer needed ? (answer should be in the commit message)
Ok, i will update the commit message as following :
Since commit 16cc5ad0b439 ("power: regulator: add dummy helper") regulator dummy helper are always available even if DM_REGULATOR is not set. DM_REGULATOR flag is no more needed to protect no DM core, remove it.
Is it OK for you ?
Yes please

On 5/5/22 15:17, Patrice Chotard wrote:
[...]
@@ -81,68 +79,31 @@ 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, err, ret, clock_nb, reset_nb;
int err, ret;
err = 0;
- priv->clock_count = 0;
- clock_nb = ofnode_count_phandle_with_args(dev_ofnode(dev), "clocks",
"#clock-cells", 0);
- if (clock_nb > 0) {
priv->clocks = devm_kcalloc(dev, clock_nb, sizeof(struct clk),
GFP_KERNEL);
if (!priv->clocks)
return -ENOMEM;
for (i = 0; i < clock_nb; i++) {
err = clk_get_by_index(dev, i, &priv->clocks[i]);
if (err < 0)
break;
err = clk_enable(&priv->clocks[i]);
if (err && err != -ENOSYS) {
dev_err(dev, "failed to enable clock %d\n", i);
clk_free(&priv->clocks[i]);
goto clk_err;
}
priv->clock_count++;
}
- } else {
if (clock_nb != -ENOENT) {
dev_err(dev, "failed to get clock phandle(%d)\n",
clock_nb);
return clock_nb;
}
- ret = clk_get_bulk(dev, &priv->clocks);
- if (ret) {
dev_err(dev, "Failed to get clocks\n");
Print the error code in the error message, so the user can immediately determine what went wrong without rebuilding the code with extra debug prints (and that goes for other messages and other drivers too, the error code is useful there).
dev_err(dev, "Failed to get clocks (ret=%d)\n", ret);
[...]

Hi Marek
On 5/5/22 15:44, Marek Vasut wrote:
On 5/5/22 15:17, Patrice Chotard wrote:
[...]
@@ -81,68 +79,31 @@ 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, err, ret, clock_nb, reset_nb; + int err, ret; err = 0; - priv->clock_count = 0; - clock_nb = ofnode_count_phandle_with_args(dev_ofnode(dev), "clocks", - "#clock-cells", 0); - if (clock_nb > 0) { - priv->clocks = devm_kcalloc(dev, clock_nb, sizeof(struct clk), - GFP_KERNEL); - if (!priv->clocks) - return -ENOMEM;
- for (i = 0; i < clock_nb; i++) { - err = clk_get_by_index(dev, i, &priv->clocks[i]);
- if (err < 0) - break; - err = clk_enable(&priv->clocks[i]); - if (err && err != -ENOSYS) { - dev_err(dev, "failed to enable clock %d\n", i); - clk_free(&priv->clocks[i]); - goto clk_err; - } - priv->clock_count++; - } - } else { - if (clock_nb != -ENOENT) { - dev_err(dev, "failed to get clock phandle(%d)\n", - clock_nb); - return clock_nb; - } + ret = clk_get_bulk(dev, &priv->clocks); + if (ret) { + dev_err(dev, "Failed to get clocks\n");
Print the error code in the error message, so the user can immediately determine what went wrong without rebuilding the code with extra debug prints (and that goes for other messages and other drivers too, the error code is useful there).
dev_err(dev, "Failed to get clocks (ret=%d)\n", ret);
[...]
Right, i will add the ret value in dev_err(); Thanks Patrice
participants (3)
-
Marek Vasut
-
Patrice CHOTARD
-
Patrice Chotard