On Tue, May 20, 2014 at 04:06:52PM +0200, Antoine T?nart wrote:

Hi,

On Tue, May 20, 2014 at 02:34:20PM +0200, Bartlomiej Zolnierkiewicz wrote:

quoted

On Tuesday, May 20, 2014 11:04:29 AM Antoine T?nart wrote:

quoted

+
+static struct phy_berlin_desc desc[] = {
+	{ .val = POWER_DOWN_PHY0 },
+	{ .val = POWER_DOWN_PHY1 },

Only .val entry of struct phy_berlin_desc is initialized and needed,
it seems that u32 vals[] should be used instead of desc[].

Sure.

quoted

quoted

+
+static int phy_berlin_sata_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct phy *phy;
+	struct phy_provider *phy_provider;
+	struct phy_berlin_priv *priv;
+	struct resource *res;
+	int i;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);

We also need to check res isn't NULL here.

quoted

quoted

+	priv->base = devm_ioremap(dev, res->start, resource_size(res));
+	if (IS_ERR(priv->base))
+		return PTR_ERR(priv->base);

devm_ioremap() (contrary to devm_ioremap_resource()) returns a valid
pointer or NULL so return value checking should be fixed.

I'll fix this.

Antoine

-- 
Antoine T?nart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com