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@xxxxxxxxxxx>
> ---
> 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
>
