[PATCH v2 1/8] phy: generic: add error trace to detect PHY issue in uclass
Add an error trace for PHY errors directly in generic phy functions provided by PHY uclass.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com ---
This patch is requested by Marek Vasut to avoid code duplication in usb host serie for dwc2:
See http://patchwork.ozlabs.org/patch/1176048/#2297595 [U-Boot,RESEND,1/5] usb: host: dwc2: add phy support
Changes in v2: - Rebase and add include dm/device_compat.h
drivers/phy/phy-uclass.c | 41 +++++++++++++++++++++++++++++++++++----- 1 file changed, 36 insertions(+), 5 deletions(-)
diff --git a/drivers/phy/phy-uclass.c b/drivers/phy/phy-uclass.c index e201a90c8c..f4a602fbd0 100644 --- a/drivers/phy/phy-uclass.c +++ b/drivers/phy/phy-uclass.c @@ -7,6 +7,7 @@ #include <common.h> #include <dm.h> #include <generic-phy.h> +#include <dm/device_compat.h>
static inline struct phy_ops *phy_dev_ops(struct udevice *dev) { @@ -109,56 +110,86 @@ int generic_phy_get_by_name(struct udevice *dev, const char *phy_name, int generic_phy_init(struct phy *phy) { struct phy_ops const *ops; + int ret;
if (!phy) return 0; ops = phy_dev_ops(phy->dev);
- return ops->init ? ops->init(phy) : 0; + ret = ops->init ? ops->init(phy) : 0; + if (ret) + dev_err(phy->dev, "PHY: Failed to init %s: %d.\n", + phy->dev->name, ret); + + return ret; }
int generic_phy_reset(struct phy *phy) { struct phy_ops const *ops; + int ret;
if (!phy) return 0; ops = phy_dev_ops(phy->dev);
- return ops->reset ? ops->reset(phy) : 0; + ret = ops->reset ? ops->reset(phy) : 0; + if (ret) + dev_err(phy->dev, "PHY: Failed to reset %s: %d.\n", + phy->dev->name, ret); + + return ret; }
int generic_phy_exit(struct phy *phy) { struct phy_ops const *ops; + int ret;
if (!phy) return 0; ops = phy_dev_ops(phy->dev);
- return ops->exit ? ops->exit(phy) : 0; + ret = ops->exit ? ops->exit(phy) : 0; + if (ret) + dev_err(phy->dev, "PHY: Failed to exit %s: %d.\n", + phy->dev->name, ret); + + return ret; }
int generic_phy_power_on(struct phy *phy) { struct phy_ops const *ops; + int ret;
if (!phy) return 0; ops = phy_dev_ops(phy->dev);
- return ops->power_on ? ops->power_on(phy) : 0; + ret = ops->power_on ? ops->power_on(phy) : 0; + if (ret) + dev_err(phy->dev, "PHY: Failed to power on %s: %d.\n", + phy->dev->name, ret); + + return ret; }
int generic_phy_power_off(struct phy *phy) { struct phy_ops const *ops; + int ret;
if (!phy) return 0; ops = phy_dev_ops(phy->dev);
- return ops->power_off ? ops->power_off(phy) : 0; + ret = ops->power_off ? ops->power_off(phy) : 0; + if (ret) + dev_err(phy->dev, "PHY: Failed to power off %s: %d.\n", + phy->dev->name, ret); + + return ret; }
UCLASS_DRIVER(phy) = {
As the error message is now displayed by generic phy functions, the dev_err can be change to dev_dbg.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com ---
Changes in v2: None
drivers/usb/gadget/dwc2_udc_otg.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/gadget/dwc2_udc_otg.c b/drivers/usb/gadget/dwc2_udc_otg.c index 496abf38e7..cfe564432f 100644 --- a/drivers/usb/gadget/dwc2_udc_otg.c +++ b/drivers/usb/gadget/dwc2_udc_otg.c @@ -983,7 +983,7 @@ int dwc2_phy_setup(struct udevice *dev, struct phy **array, int *num_phys) for (i = 0; i < count; i++) { ret = generic_phy_init(&usb_phys[i]); if (ret) { - dev_err(dev, "Can't init USB PHY%d for %s\n", + dev_dbg(dev, "Can't init USB PHY%d for %s\n", i, dev->name); goto phys_init_err; } @@ -992,7 +992,7 @@ int dwc2_phy_setup(struct udevice *dev, struct phy **array, int *num_phys) for (i = 0; i < count; i++) { ret = generic_phy_power_on(&usb_phys[i]); if (ret) { - dev_err(dev, "Can't power USB PHY%d for %s\n", + dev_dbg(dev, "Can't power USB PHY%d for %s\n", i, dev->name); goto phys_poweron_err; } @@ -1030,7 +1030,7 @@ void dwc2_phy_shutdown(struct udevice *dev, struct phy *usb_phys, int num_phys) ret = generic_phy_power_off(&usb_phys[i]); ret |= generic_phy_exit(&usb_phys[i]); if (ret) { - dev_err(dev, "Can't shutdown USB PHY%d for %s\n", + dev_dbg(dev, "Can't shutdown USB PHY%d for %s\n", i, dev->name); } }
As the error message is now displayed by generic phy functions, the pr_err can be change to pr_idebug.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com ---
Changes in v2: None
board/sunxi/board.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/board/sunxi/board.c b/board/sunxi/board.c index 6afea6ef42..9d27f9ceac 100644 --- a/board/sunxi/board.c +++ b/board/sunxi/board.c @@ -708,7 +708,7 @@ int g_dnl_board_usb_cable_connected(void)
ret = generic_phy_init(&phy); if (ret) { - pr_err("failed to init %s USB PHY\n", dev->name); + pr_debug("failed to init %s USB PHY\n", dev->name); return ret; }
As the error message is now displayed by generic phy functions, the dev_err can be change to dev_dbg.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com ---
Changes in v2: None
drivers/usb/host/ohci-generic.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/host/ohci-generic.c b/drivers/usb/host/ohci-generic.c index 04d5fdb2a8..09fcbf2a01 100644 --- a/drivers/usb/host/ohci-generic.c +++ b/drivers/usb/host/ohci-generic.c @@ -40,13 +40,13 @@ static int ohci_setup_phy(struct udevice *dev, int index) } else { ret = generic_phy_init(&priv->phy); if (ret) { - dev_err(dev, "failed to init usb phy\n"); + dev_dbg(dev, "failed to init usb phy\n"); return ret; }
ret = generic_phy_power_on(&priv->phy); if (ret) { - dev_err(dev, "failed to power on usb phy\n"); + dev_dbg(dev, "failed to power on usb phy\n"); return generic_phy_exit(&priv->phy); } } @@ -62,13 +62,13 @@ static int ohci_shutdown_phy(struct udevice *dev) if (generic_phy_valid(&priv->phy)) { ret = generic_phy_power_off(&priv->phy); if (ret) { - dev_err(dev, "failed to power off usb phy\n"); + dev_dbg(dev, "failed to power off usb phy\n"); return ret; }
ret = generic_phy_exit(&priv->phy); if (ret) { - dev_err(dev, "failed to power off usb phy\n"); + dev_dbg(dev, "failed to power off usb phy\n"); return ret; } }
As the error message is now displayed by generic phy functions, the pr_err can be change to pr_debug.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com ---
Changes in v2: None
drivers/usb/host/ehci-hcd.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c index 1cc02052f5..9ebebddf7d 100644 --- a/drivers/usb/host/ehci-hcd.c +++ b/drivers/usb/host/ehci-hcd.c @@ -1699,13 +1699,13 @@ int ehci_setup_phy(struct udevice *dev, struct phy *phy, int index) } else { ret = generic_phy_init(phy); if (ret) { - dev_err(dev, "failed to init usb phy\n"); + dev_dbg(dev, "failed to init usb phy\n"); return ret; }
ret = generic_phy_power_on(phy); if (ret) { - dev_err(dev, "failed to power on usb phy\n"); + dev_dbg(dev, "failed to power on usb phy\n"); return generic_phy_exit(phy); } } @@ -1723,13 +1723,13 @@ int ehci_shutdown_phy(struct udevice *dev, struct phy *phy) if (generic_phy_valid(phy)) { ret = generic_phy_power_off(phy); if (ret) { - dev_err(dev, "failed to power off usb phy\n"); + dev_dbg(dev, "failed to power off usb phy\n"); return ret; }
ret = generic_phy_exit(phy); if (ret) { - dev_err(dev, "failed to power off usb phy\n"); + dev_dbg(dev, "failed to power off usb phy\n"); return ret; } }
As the error message is now displayed by generic phy functions, the pr_err can be change to pr_debug.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com ---
Changes in v2: None
drivers/usb/dwc3/core.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index c5066529b7..6304036c00 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -867,8 +867,8 @@ int dwc3_setup_phy(struct udevice *dev, struct phy **array, int *num_phys) for (i = 0; i < count; i++) { ret = generic_phy_init(&usb_phys[i]); if (ret) { - pr_err("Can't init USB PHY%d for %s\n", - i, dev->name); + pr_debug("Can't init USB PHY%d for %s\n", + i, dev->name); goto phys_init_err; } } @@ -876,8 +876,8 @@ int dwc3_setup_phy(struct udevice *dev, struct phy **array, int *num_phys) for (i = 0; i < count; i++) { ret = generic_phy_power_on(&usb_phys[i]); if (ret) { - pr_err("Can't power USB PHY%d for %s\n", - i, dev->name); + pr_debug("Can't power USB PHY%d for %s\n", + i, dev->name); goto phys_poweron_err; } } @@ -913,8 +913,8 @@ int dwc3_shutdown_phy(struct udevice *dev, struct phy *usb_phys, int num_phys) ret = generic_phy_power_off(&usb_phys[i]); ret |= generic_phy_exit(&usb_phys[i]); if (ret) { - pr_err("Can't shutdown USB PHY%d for %s\n", - i, dev->name); + pr_debug("Can't shutdown USB PHY%d for %s\n", + i, dev->name); } }
On 2/18/20 9:38 AM, Patrick Delaunay wrote:
As the error message is now displayed by generic phy functions, the pr_err can be change to pr_debug.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com
Changes in v2: None
drivers/usb/dwc3/core.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index c5066529b7..6304036c00 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -867,8 +867,8 @@ int dwc3_setup_phy(struct udevice *dev, struct phy **array, int *num_phys) for (i = 0; i < count; i++) { ret = generic_phy_init(&usb_phys[i]); if (ret) {
pr_err("Can't init USB PHY%d for %s\n",i, dev->name);
pr_debug("Can't init USB PHY%d for %s\n",i, dev->name);
dev_dbg() , since you have a dev pointer.
As the error message is now displayed by generic phy functions, the pr_err can be change to pr_debug.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com ---
Changes in v2: None
drivers/ata/dwc_ahci.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/ata/dwc_ahci.c b/drivers/ata/dwc_ahci.c index 017650ae46..3c2a3ac201 100644 --- a/drivers/ata/dwc_ahci.c +++ b/drivers/ata/dwc_ahci.c @@ -62,13 +62,13 @@ static int dwc_ahci_probe(struct udevice *dev)
ret = generic_phy_init(&phy); if (ret) { - pr_err("unable to initialize the sata phy\n"); + pr_debug("unable to initialize the sata phy\n"); return ret; }
ret = generic_phy_power_on(&phy); if (ret) { - pr_err("unable to power on the sata phy\n"); + pr_debug("unable to power on the sata phy\n"); return ret; }
As the error message is now displayed by generic phy functions, the dev_err/pr_err can be change to dev_dbg/pr_debug.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com ---
Changes in v2: - Added patch after rebase: new generic_phy API usage
drivers/usb/musb-new/sunxi.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/musb-new/sunxi.c b/drivers/usb/musb-new/sunxi.c index 98bf736978..8da61f818e 100644 --- a/drivers/usb/musb-new/sunxi.c +++ b/drivers/usb/musb-new/sunxi.c @@ -254,7 +254,7 @@ static int sunxi_musb_enable(struct musb *musb)
ret = generic_phy_power_on(&glue->phy); if (ret) { - pr_err("failed to power on USB PHY\n"); + pr_debug("failed to power on USB PHY\n"); return ret; } } @@ -278,7 +278,7 @@ static void sunxi_musb_disable(struct musb *musb) if (is_host_enabled(musb)) { ret = generic_phy_power_off(&glue->phy); if (ret) { - pr_err("failed to power off USB PHY\n"); + pr_debug("failed to power off USB PHY\n"); return; } } @@ -312,7 +312,7 @@ static int sunxi_musb_init(struct musb *musb)
ret = generic_phy_init(&glue->phy); if (ret) { - dev_err(dev, "failed to init USB PHY\n"); + dev_dbg(dev, "failed to init USB PHY\n"); goto err_rst; }
@@ -349,7 +349,7 @@ static int sunxi_musb_exit(struct musb *musb) if (generic_phy_valid(&glue->phy)) { ret = generic_phy_exit(&glue->phy); if (ret) { - dev_err(dev, "failed to power off usb phy\n"); + dev_dbg(dev, "failed to power off usb phy\n"); return ret; } }
On 2/18/20 9:38 AM, Patrick Delaunay wrote: [...]
static inline struct phy_ops *phy_dev_ops(struct udevice *dev) { @@ -109,56 +110,86 @@ int generic_phy_get_by_name(struct udevice *dev, const char *phy_name, int generic_phy_init(struct phy *phy) { struct phy_ops const *ops;
int ret;
if (!phy) return 0; ops = phy_dev_ops(phy->dev);
- return ops->init ? ops->init(phy) : 0;
- ret = ops->init ? ops->init(phy) : 0;
if (!ops->init) return 0; ret = ops->init(); if (ret) dev_err...
return ret;
Please fix globally.
- if (ret)
dev_err(phy->dev, "PHY: Failed to init %s: %d.\n",phy->dev->name, ret);- return ret;
[...]
Hi Marek,
From: Marek Vasut marex@denx.de Sent: mardi 18 février 2020 18:40
On 2/18/20 9:38 AM, Patrick Delaunay wrote: [...]
static inline struct phy_ops *phy_dev_ops(struct udevice *dev) { @@ -109,56 +110,86 @@ int generic_phy_get_by_name(struct udevice *dev, const char *phy_name, int generic_phy_init(struct phy *phy) { struct phy_ops const *ops;
int ret;
if (!phy) return 0; ops = phy_dev_ops(phy->dev);
- return ops->init ? ops->init(phy) : 0;
- ret = ops->init ? ops->init(phy) : 0;
if (!ops->init) return 0; ret = ops->init(); if (ret) dev_err...
return ret;
Please fix globally.
Yes it is more clear, I am the v3 serie
- if (ret)
dev_err(phy->dev, "PHY: Failed to init %s: %d.\n",phy->dev->name, ret);- return ret;
[...]
Regards,
Patrick
participants (3)
-
Marek Vasut -
Patrick DELAUNAY -
Patrick Delaunay