+Jan

On 5/18/22 12:18, Aswath Govindraju wrote:

[CAUTION: External Email]

generic_phy_power_off(), needs to be called in dwc3_glue_remove() while exiting as we powering on the phy in the dwc3_glue_probe(). Therefore, instantiate struct phy in dwc3_glue_data to use in dwc3_glue_probe() as well as dwc3_glue_remove().

In cases where "usb3-phy" is not present in the phy-names property, generic_phy_get_by_name() returns -ENODATA. Therefore, add condition to not return when the error code is -ENODATA too.

Also, generic_phy_init() and generic_phy_power_on() functions, have checks to verify if the struct phy argument passed is valid. Therefore, remove additional checks added for these in the dwc3_glue_probe().

Fixes: 142d50fbce7c ("usb: dwc3: Add support for usb3-phy PHY configuration") Signed-off-by: Aswath Govindraju a-govindraju@ti.com

---

drivers/usb/dwc3/dwc3-generic.c | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-generic.c b/drivers/usb/dwc3/dwc3-generic.c index 6e1a1d066b40..f74d710a2447 100644 --- a/drivers/usb/dwc3/dwc3-generic.c +++ b/drivers/usb/dwc3/dwc3-generic.c @@ -30,6 +30,7 @@ struct dwc3_glue_data { struct clk_bulk clks; struct reset_ctl_bulk resets;

•   struct phy phy;
     fdt_addr_t regs;
  

  };

@@ -458,21 +459,21 @@ static int dwc3_glue_probe(struct udevice *dev) { struct dwc3_glue_ops *ops = (struct dwc3_glue_ops *)dev_get_driver_data(dev); struct dwc3_glue_data *glue = dev_get_plat(dev);

•   struct phy *phy = &glue->phy;
     struct udevice *child = NULL;
     int index = 0;
     int ret;
  

•   struct phy phy;
  
  

•   ret = generic_phy_get_by_name(dev, "usb3-phy", &phy);
  

•   if (!ret) {
  

•           ret = generic_phy_init(&phy);
  

•           if (ret)
  

•                   return ret;
  

•   } else if (ret != -ENOENT) {
  

•   ret = generic_phy_get_by_name(dev, "usb3-phy", phy);
  

•   if (ret && ret != -ENOENT && ret != -ENODATA) {
             debug("could not get phy (err %d)\n", ret);
             return ret;
     }
  
  

•   ret = generic_phy_init(phy);
  

•   if (ret)
  

•           return ret;
  

•    glue->regs = dev_read_addr(dev);
  
     ret = dwc3_glue_clk_init(dev, glue);
  

@@ -483,11 +484,9 @@ static int dwc3_glue_probe(struct udevice *dev) if (ret) return ret;

•   if (phy.dev) {
  

•           ret = generic_phy_power_on(&phy);
  

•           if (ret)
  

•                   return ret;
  

•   }
  

•   ret = generic_phy_power_on(phy);
  

•   if (ret)
  

•           return ret;
  
     ret = device_find_first_child(dev, &child);
     if (ret)
  

@@ -516,6 +515,8 @@ static int dwc3_glue_remove(struct udevice *dev) { struct dwc3_glue_data *glue = dev_get_plat(dev);

•   generic_phy_power_off(&glue->phy);
  

•    reset_release_bulk(&glue->resets);
  
     clk_release_bulk(&glue->clks);
  

-- 2.17.1

Jan sent one patch for this to handle ENODATA case. You don't need to fill NULL phy.dev to NULL because in your case it should be NUll already. https://lore.kernel.org/all/c5a71c30-e55d-c8ab-d372-e5aaa859cf2a@siemens.com...

That's why I am fine with this patch too.

Acked-by: Michal Simek michal.simek@amd.com Tested-by: Michal Simek michal.simek@amd.com

Thanks, Michal