[PATCH 3/4] SATA: MV: Add support for the optional PHYs

[Andrew Lunn]

On Wed, Dec 18, 2013 at 07:13:13AM -0500, Tejun Heo wrote:
> Hello,
> 
> On Wed, Dec 18, 2013 at 12:00:10PM +0530, Kishon Vijay Abraham I wrote:
> > > @@ -4097,6 +4109,10 @@ static int mv_platform_probe(struct platform_device *pdev)
> > >  		hpriv->port_clks[port] = clk_get(&pdev->dev, port_number);
> > >  		if (!IS_ERR(hpriv->port_clks[port]))
> > >  			clk_prepare_enable(hpriv->port_clks[port]);
> > > +		sprintf(port_number, "port%d", port);
> > > +		hpriv->port_phys[port] = devm_phy_get(&pdev->dev, port_number);
> > > +		if (!IS_ERR(hpriv->port_phys[port]))
> > > +			phy_power_on(hpriv->port_phys[port]);
> 
> Shouldn't it distinguish between failures and at least produce
> warning?  ie. phy not available and phy init failed due to memory
> pressure or whatnot shouldn't be handled the same.

Phy not available is not an error, since not all variants of the SATA
IP block have the ability to control the phy. I can however add a
warning for real errors.

> > > @@ -4132,6 +4148,8 @@ err:
> > >  			clk_disable_unprepare(hpriv->port_clks[port]);
> > >  			clk_put(hpriv->port_clks[port]);
> > >  		}
> > > +		if (!IS_ERR(hpriv->port_phys[port]))
> > > +			phy_power_off(hpriv->port_phys[port]);
> 
> And I'd much prefer the array holds either NULL or valid pointer.

I was trying to keep the code similar to the clk handling. However now
that it is diverging more and more from how clk is handled, i can add
yet more divergence and overwrite the error with a NULL.

    Andrew