| Message ID | 20200218083503.5468-3-patrick.delaunay@st.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | usb: host: dwc2: use driver model for PHY and CLOCK | expand |
Am 18.02.2020 um 09:35 schrieb Patrick Delaunay: > Use generic phy to initialize the PHY associated to the > DWC2 device and available in the device tree. > > This patch don't added dependency because when CONFIG_PHY > is not activated, the generic PHY function are stubbed. > > Signed-off-by: Patrick Delaunay <patrick.delaunay at st.com> > --- > > Changes in v4: None > Changes in v3: None > Changes in v2: > - update dev_err > - update commit message > - change dev_err to dev_dbg for PHY function call > - treat dwc2_shutdown_phy error > > drivers/usb/host/dwc2.c | 66 +++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 66 insertions(+) > > diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c > index e4efaf1e59..5e7ffaddd9 100644 > --- a/drivers/usb/host/dwc2.c > +++ b/drivers/usb/host/dwc2.c > @@ -8,6 +8,7 @@ > #include <cpu_func.h> > #include <dm.h> > #include <errno.h> > +#include <generic-phy.h> > #include <usb.h> > #include <malloc.h> > #include <memalign.h> > @@ -37,6 +38,7 @@ struct dwc2_priv { > #ifdef CONFIG_DM_REGULATOR > struct udevice *vbus_supply; > #endif > + struct phy phy; > #else > uint8_t *aligned_buffer; > uint8_t *status_buffer; > @@ -1322,13 +1324,71 @@ static int dwc2_usb_ofdata_to_platdata(struct udevice *dev) > return 0; > } > > +static int dwc2_setup_phy(struct udevice *dev) > +{ > + struct dwc2_priv *priv = dev_get_priv(dev); > + int ret; > + > + ret = generic_phy_get_by_index(dev, 0, &priv->phy); > + if (ret) { > + if (ret != -ENOENT) { Could you invert this logic and add a comment like "no PHY" or something? > + dev_err(dev, "Failed to get USB PHY: %d.\n", ret); > + return ret; > + } > + return 0; > + } > + > + ret = generic_phy_init(&priv->phy); > + if (ret) { > + dev_dbg(dev, "Failed to init USB PHY: %d.\n", ret); > + return ret; > + } > + > + ret = generic_phy_power_on(&priv->phy); > + if (ret) { > + dev_dbg(dev, "Failed to power on USB PHY: %d.\n", ret); > + generic_phy_exit(&priv->phy); > + return ret; > + } > + > + return 0; > +} > + > +static int dwc2_shutdown_phy(struct udevice *dev) > +{ > + struct dwc2_priv *priv = dev_get_priv(dev); > + int ret; > + > + if (!generic_phy_valid(&priv->phy)) A comment saying that this is for platforms without a phy driver would be nice. Other than that: Reviewed-by: Simon Goldschmidt <simon.k.r.goldschmidt at gmail.com> > + return 0; > + > + ret = generic_phy_power_off(&priv->phy); > + if (ret) { > + dev_dbg(dev, "Failed to power off USB PHY: %d.\n", ret); > + return ret; > + } > + > + ret = generic_phy_exit(&priv->phy); > + if (ret) { > + dev_dbg(dev, "Failed to power off USB PHY: %d.\n", ret); > + return ret; > + } > + > + return 0; > +} > + > static int dwc2_usb_probe(struct udevice *dev) > { > struct dwc2_priv *priv = dev_get_priv(dev); > struct usb_bus_priv *bus_priv = dev_get_uclass_priv(dev); > + int ret; > > bus_priv->desc_before_addr = true; > > + ret = dwc2_setup_phy(dev); > + if (ret) > + return ret; > + > return dwc2_init_common(dev, priv); > } > > @@ -1341,6 +1401,12 @@ static int dwc2_usb_remove(struct udevice *dev) > if (ret) > return ret; > > + ret = dwc2_shutdown_phy(dev); > + if (ret) { > + dev_dbg(dev, "Failed to shutdown USB PHY: %d.\n", ret); > + return ret; > + } > + > dwc2_uninit_common(priv->regs); > > reset_release_bulk(&priv->resets); >
> -----Original Message----- > From: Simon Goldschmidt <simon.k.r.goldschmidt at gmail.com> > Sent: mercredi 4 mars 2020 20:52 > To: Patrick DELAUNAY <patrick.delaunay at st.com>; u-boot at lists.denx.de > Cc: ley.foon.tan at intel.com; b.galvani at gmail.com; Daniel Schwierzeck > <daniel.schwierzeck at gmail.com>; Marek Vasut <marex at denx.de>; Michal > Suchanek <msuchanek at suse.de>; Simon Glass <sjg at chromium.org>; U-Boot > STM32 <uboot-stm32 at st-md-mailman.stormreply.com> > Subject: Re: [PATCH v4 2/5] usb: host: dwc2: add phy support > Importance: High > > Am 18.02.2020 um 09:35 schrieb Patrick Delaunay: > > Use generic phy to initialize the PHY associated to the > > DWC2 device and available in the device tree. > > > > This patch don't added dependency because when CONFIG_PHY is not > > activated, the generic PHY function are stubbed. > > > > Signed-off-by: Patrick Delaunay <patrick.delaunay at st.com> > > --- > > > > Changes in v4: None > > Changes in v3: None > > Changes in v2: > > - update dev_err > > - update commit message > > - change dev_err to dev_dbg for PHY function call > > - treat dwc2_shutdown_phy error > > > > drivers/usb/host/dwc2.c | 66 > > +++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 66 insertions(+) > > > > diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c index > > e4efaf1e59..5e7ffaddd9 100644 > > --- a/drivers/usb/host/dwc2.c > > +++ b/drivers/usb/host/dwc2.c > > @@ -8,6 +8,7 @@ > > #include <cpu_func.h> > > #include <dm.h> > > #include <errno.h> > > +#include <generic-phy.h> > > #include <usb.h> > > #include <malloc.h> > > #include <memalign.h> > > @@ -37,6 +38,7 @@ struct dwc2_priv { > > #ifdef CONFIG_DM_REGULATOR > > struct udevice *vbus_supply; > > #endif > > + struct phy phy; > > #else > > uint8_t *aligned_buffer; > > uint8_t *status_buffer; > > @@ -1322,13 +1324,71 @@ static int dwc2_usb_ofdata_to_platdata(struct > udevice *dev) > > return 0; > > } > > > > +static int dwc2_setup_phy(struct udevice *dev) { > > + struct dwc2_priv *priv = dev_get_priv(dev); > > + int ret; > > + > > + ret = generic_phy_get_by_index(dev, 0, &priv->phy); > > + if (ret) { > > + if (ret != -ENOENT) { > > Could you invert this logic and add a comment like "no PHY" or something? Yes in V5, it is more clear ret = generic_phy_get_by_index(dev, 0, &priv->phy); if (ret) { if (ret == -ENOENT) return 0; /* no PHY, nothing to do */ dev_err(dev, "Failed to get USB PHY: %d.\n", ret); return ret; } > > + dev_err(dev, "Failed to get USB PHY: %d.\n", ret); > > + return ret; > > + } > > + return 0; > > + } > > + > > + ret = generic_phy_init(&priv->phy); > > + if (ret) { > > + dev_dbg(dev, "Failed to init USB PHY: %d.\n", ret); > > + return ret; > > + } > > + > > + ret = generic_phy_power_on(&priv->phy); > > + if (ret) { > > + dev_dbg(dev, "Failed to power on USB PHY: %d.\n", ret); > > + generic_phy_exit(&priv->phy); > > + return ret; > > + } > > + > > + return 0; > > +} > > + > > +static int dwc2_shutdown_phy(struct udevice *dev) { > > + struct dwc2_priv *priv = dev_get_priv(dev); > > + int ret; > > + > > + if (!generic_phy_valid(&priv->phy)) > > A comment saying that this is for platforms without a phy driver would be nice. Yes in V5: static int dwc2_shutdown_phy(struct udevice *dev) { struct dwc2_priv *priv = dev_get_priv(dev); int ret; /* PHY is not valid when generic_phy_get_by_index() = -ENOENT */ if (!generic_phy_valid(&priv->phy)) return 0; /* no PHY, nothing to do */ > Other than that: > Reviewed-by: Simon Goldschmidt <simon.k.r.goldschmidt at gmail.com> Thanks Regards Patrick
diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c index e4efaf1e59..5e7ffaddd9 100644 --- a/drivers/usb/host/dwc2.c +++ b/drivers/usb/host/dwc2.c @@ -8,6 +8,7 @@ #include <cpu_func.h> #include <dm.h> #include <errno.h> +#include <generic-phy.h> #include <usb.h> #include <malloc.h> #include <memalign.h> @@ -37,6 +38,7 @@ struct dwc2_priv { #ifdef CONFIG_DM_REGULATOR struct udevice *vbus_supply; #endif + struct phy phy; #else uint8_t *aligned_buffer; uint8_t *status_buffer; @@ -1322,13 +1324,71 @@ static int dwc2_usb_ofdata_to_platdata(struct udevice *dev) return 0; } +static int dwc2_setup_phy(struct udevice *dev) +{ + struct dwc2_priv *priv = dev_get_priv(dev); + int ret; + + ret = generic_phy_get_by_index(dev, 0, &priv->phy); + if (ret) { + if (ret != -ENOENT) { + dev_err(dev, "Failed to get USB PHY: %d.\n", ret); + return ret; + } + return 0; + } + + ret = generic_phy_init(&priv->phy); + if (ret) { + dev_dbg(dev, "Failed to init USB PHY: %d.\n", ret); + return ret; + } + + ret = generic_phy_power_on(&priv->phy); + if (ret) { + dev_dbg(dev, "Failed to power on USB PHY: %d.\n", ret); + generic_phy_exit(&priv->phy); + return ret; + } + + return 0; +} + +static int dwc2_shutdown_phy(struct udevice *dev) +{ + struct dwc2_priv *priv = dev_get_priv(dev); + int ret; + + if (!generic_phy_valid(&priv->phy)) + return 0; + + ret = generic_phy_power_off(&priv->phy); + if (ret) { + dev_dbg(dev, "Failed to power off USB PHY: %d.\n", ret); + return ret; + } + + ret = generic_phy_exit(&priv->phy); + if (ret) { + dev_dbg(dev, "Failed to power off USB PHY: %d.\n", ret); + return ret; + } + + return 0; +} + static int dwc2_usb_probe(struct udevice *dev) { struct dwc2_priv *priv = dev_get_priv(dev); struct usb_bus_priv *bus_priv = dev_get_uclass_priv(dev); + int ret; bus_priv->desc_before_addr = true; + ret = dwc2_setup_phy(dev); + if (ret) + return ret; + return dwc2_init_common(dev, priv); } @@ -1341,6 +1401,12 @@ static int dwc2_usb_remove(struct udevice *dev) if (ret) return ret; + ret = dwc2_shutdown_phy(dev); + if (ret) { + dev_dbg(dev, "Failed to shutdown USB PHY: %d.\n", ret); + return ret; + } + dwc2_uninit_common(priv->regs); reset_release_bulk(&priv->resets);
Use generic phy to initialize the PHY associated to the DWC2 device and available in the device tree. This patch don't added dependency because when CONFIG_PHY is not activated, the generic PHY function are stubbed. Signed-off-by: Patrick Delaunay <patrick.delaunay at st.com> --- Changes in v4: None Changes in v3: None Changes in v2: - update dev_err - update commit message - change dev_err to dev_dbg for PHY function call - treat dwc2_shutdown_phy error drivers/usb/host/dwc2.c | 66 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+)