Re: [PATCH] ATA: SATA_MV: Fix probe failure when no phy exists

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

On Thu, Jan 30, 2014 at 09:50:35PM +0100, Andrew Lunn wrote:
> Armada 370 and XP do not have a SATA phy driver.  The generic phy
> layer does not cleanly support optional phys. It is not possible to
> determine from the error code if there is expected to be a phy
> according to DT, but it cannot be found, or no phy is listed in
> DT. All that can be determined is that a phy is expected, but the
> driver has not been loaded yet, in which case -EPROBE_DEFER is
> returned. Thus for 370 and XP the driver failed to probe.  Play safe,
> consider all errors except -EPROBE_DEFER to be none fatal and keep
> going, and in the case of -EPROBE_DEFER exit the probe function with
> that error code.
> 
> Tested on Kirkwood with a sata phy driver and on 370 without a sata
> phy driver.
> 
> Reported-by: Jean Pihet <jean.pihet@linaro.org>
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> Tested-by: Gregory Clement <gregory.clement@free-electrons.com>
> ---
>  drivers/ata/sata_mv.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
> index eaa21eddbe70..148ff5a82c8b 100644
> --- a/drivers/ata/sata_mv.c
> +++ b/drivers/ata/sata_mv.c
> @@ -4115,9 +4115,8 @@ static int mv_platform_probe(struct platform_device *pdev)
>  		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");
> -			goto err;
> +			if (rc == -EPROBE_DEFER)
> +				goto err;

It feels a bit fishy to check for a specific errno.

How about not considering the lack of phy an error in all cases? In
other words, remove the check completely.

Isn't the phy used only for power saving purposes? Or do we want this
for another purpose?

Or as a different solution, can't we check for the compatible-string
and only try to get a phy for orion-sata?
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com