Re: [PATCH 3/3] ata: sata_mv: Fix probe failures with optional phys

[Ezequiel Garcia <ezequiel.garcia@free-electrons.com>]

On Sat, Feb 01, 2014 at 03:15:57PM +0100, Andrew Lunn wrote:
> Make use of devm_phy_optional() in order to fix probe failures on
> Armada 370, XP and others, when there is no phy driver available.
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> ---
>  drivers/ata/sata_mv.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
> index eaa21eddbe70..26021cf077a8 100644
> --- a/drivers/ata/sata_mv.c
> +++ b/drivers/ata/sata_mv.c
> @@ -4111,12 +4111,14 @@ static int mv_platform_probe(struct platform_device *pdev)
>  			clk_prepare_enable(hpriv->port_clks[port]);
>  
>  		sprintf(port_number, "port%d", port);
> -		hpriv->port_phys[port] = devm_phy_get(&pdev->dev, port_number);
> +		hpriv->port_phys[port] = devm_phy_optional_get(&pdev->dev,
> +							       port_number);
>  		if (IS_ERR(hpriv->port_phys[port])) {
>  			rc = PTR_ERR(hpriv->port_phys[port]);
>  			hpriv->port_phys[port] = NULL;
> -			if ((rc != -EPROBE_DEFER) && (rc != -ENODEV))
> -				dev_warn(&pdev->dev, "error getting phy");
> +			if (rc != -EPROBE_DEFER)
> +				dev_warn(&pdev->dev, "error getting phy %d",
> +					rc);
>  			goto err;

IMHO, this new series look much better. However, I still think the above code
is highly confusing (took me some time to see why you don't print the warning
on PROBE_DEFER, but do the goto in all cases).

Would it be too much to ask to add some comments to it? Your previous
explanation about why we need to fail on EPROBE_DEFER, to allow the phy
driver to load, was great. Adding some of that here would be nice.

I'll test this next week.
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com