| Message ID | 1538131878-8305-1-git-send-email-adouglas@cadence.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | [v2] PCI: cadence: Correct probe behaviour when failing to get PHY | expand |
On Fri, Sep 28, 2018 at 11:51:18AM +0100, Alan Douglas wrote: > Test the correct value to see whether the PHY get failed. > > Use devm_phy_get() instead of devm_phy_optional_get(), since it is > only called if phy name is given in devicetree and so should exist. > If failure when getting or linking PHY, put any PHYs which were > already got and unlink them. > > Fixes: dfb80534692ddc5b ("PCI: cadence: Add generic PHY support to host and EP drivers") > Signed-off-by: Alan Douglas <adouglas@cadence.com> > --- > changes since v1: > - Correct error in while loop pointed out by Colin King Is there a reason why I should NOT add Colin as a Reported-by ? Lorenzo > drivers/pci/controller/pcie-cadence.c | 20 ++++++++++++-------- > 1 file changed, 12 insertions(+), 8 deletions(-) > > diff --git a/drivers/pci/controller/pcie-cadence.c b/drivers/pci/controller/pcie-cadence.c > index 86f1b00..5865512 100644 > --- a/drivers/pci/controller/pcie-cadence.c > +++ b/drivers/pci/controller/pcie-cadence.c > @@ -190,14 +190,16 @@ int cdns_pcie_init_phy(struct device *dev, struct cdns_pcie *pcie) > > for (i = 0; i < phy_count; i++) { > of_property_read_string_index(np, "phy-names", i, &name); > - phy[i] = devm_phy_optional_get(dev, name); > - if (IS_ERR(phy)) > - return PTR_ERR(phy); > - > + phy[i] = devm_phy_get(dev, name); > + if (IS_ERR(phy[i])) { > + ret = PTR_ERR(phy[i]); > + goto err_phy; > + } > link[i] = device_link_add(dev, &phy[i]->dev, DL_FLAG_STATELESS); > if (!link[i]) { > + devm_phy_put(dev, phy[i]); > ret = -EINVAL; > - goto err_link; > + goto err_phy; > } > } > > @@ -207,13 +209,15 @@ int cdns_pcie_init_phy(struct device *dev, struct cdns_pcie *pcie) > > ret = cdns_pcie_enable_phy(pcie); > if (ret) > - goto err_link; > + goto err_phy; > > return 0; > > -err_link: > - while (--i >= 0) > +err_phy: > + while (--i >= 0) { > device_link_del(link[i]); > + devm_phy_put(dev, phy[i]); > + } > > return ret; > } > -- > 1.9.0 >
Hi, On 28 September 2018 16:31, Lorenzo Pieralisi wrote: > On Fri, Sep 28, 2018 at 11:51:18AM +0100, Alan Douglas wrote: > > Test the correct value to see whether the PHY get failed. > > > > Use devm_phy_get() instead of devm_phy_optional_get(), since it is > > only called if phy name is given in devicetree and so should exist. > > If failure when getting or linking PHY, put any PHYs which were > > already got and unlink them. > > > > Fixes: dfb80534692ddc5b ("PCI: cadence: Add generic PHY support to host and EP drivers") > > Signed-off-by: Alan Douglas <adouglas@cadence.com> > > --- > > changes since v1: > > - Correct error in while loop pointed out by Colin King > > Is there a reason why I should NOT add Colin as a Reported-by ? > No, I should have added Colin as Reported-by > Lorenzo > > > drivers/pci/controller/pcie-cadence.c | 20 ++++++++++++-------- > > 1 file changed, 12 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/pci/controller/pcie-cadence.c b/drivers/pci/controller/pcie-cadence.c > > index 86f1b00..5865512 100644 > > --- a/drivers/pci/controller/pcie-cadence.c > > +++ b/drivers/pci/controller/pcie-cadence.c > > @@ -190,14 +190,16 @@ int cdns_pcie_init_phy(struct device *dev, struct cdns_pcie *pcie) > > > > for (i = 0; i < phy_count; i++) { > > of_property_read_string_index(np, "phy-names", i, &name); > > - phy[i] = devm_phy_optional_get(dev, name); > > - if (IS_ERR(phy)) > > - return PTR_ERR(phy); > > - > > + phy[i] = devm_phy_get(dev, name); > > + if (IS_ERR(phy[i])) { > > + ret = PTR_ERR(phy[i]); > > + goto err_phy; > > + } > > link[i] = device_link_add(dev, &phy[i]->dev, DL_FLAG_STATELESS); > > if (!link[i]) { > > + devm_phy_put(dev, phy[i]); > > ret = -EINVAL; > > - goto err_link; > > + goto err_phy; > > } > > } > > > > @@ -207,13 +209,15 @@ int cdns_pcie_init_phy(struct device *dev, struct cdns_pcie *pcie) > > > > ret = cdns_pcie_enable_phy(pcie); > > if (ret) > > - goto err_link; > > + goto err_phy; > > > > return 0; > > > > -err_link: > > - while (--i >= 0) > > +err_phy: > > + while (--i >= 0) { > > device_link_del(link[i]); > > + devm_phy_put(dev, phy[i]); > > + } > > > > return ret; > > } > > -- > > 1.9.0 > >
diff --git a/drivers/pci/controller/pcie-cadence.c b/drivers/pci/controller/pcie-cadence.c index 86f1b00..5865512 100644 --- a/drivers/pci/controller/pcie-cadence.c +++ b/drivers/pci/controller/pcie-cadence.c @@ -190,14 +190,16 @@ int cdns_pcie_init_phy(struct device *dev, struct cdns_pcie *pcie) for (i = 0; i < phy_count; i++) { of_property_read_string_index(np, "phy-names", i, &name); - phy[i] = devm_phy_optional_get(dev, name); - if (IS_ERR(phy)) - return PTR_ERR(phy); - + phy[i] = devm_phy_get(dev, name); + if (IS_ERR(phy[i])) { + ret = PTR_ERR(phy[i]); + goto err_phy; + } link[i] = device_link_add(dev, &phy[i]->dev, DL_FLAG_STATELESS); if (!link[i]) { + devm_phy_put(dev, phy[i]); ret = -EINVAL; - goto err_link; + goto err_phy; } } @@ -207,13 +209,15 @@ int cdns_pcie_init_phy(struct device *dev, struct cdns_pcie *pcie) ret = cdns_pcie_enable_phy(pcie); if (ret) - goto err_link; + goto err_phy; return 0; -err_link: - while (--i >= 0) +err_phy: + while (--i >= 0) { device_link_del(link[i]); + devm_phy_put(dev, phy[i]); + } return ret; }
Test the correct value to see whether the PHY get failed. Use devm_phy_get() instead of devm_phy_optional_get(), since it is only called if phy name is given in devicetree and so should exist. If failure when getting or linking PHY, put any PHYs which were already got and unlink them. Fixes: dfb80534692ddc5b ("PCI: cadence: Add generic PHY support to host and EP drivers") Signed-off-by: Alan Douglas <adouglas@cadence.com> --- changes since v1: - Correct error in while loop pointed out by Colin King drivers/pci/controller/pcie-cadence.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-)