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 -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html