Re: [PATCH v7 2/5] MFD: RK808: Add new mfd driver for RK808

On Mon, 01 Sep 2014, Chris Zhong wrote:

The RK808 chip is a power management IC for multimedia and handheld
devices. It contains the following components:

- Regulators
- RTC
- Clkout

The RK808 core driver is registered as a platform driver and provides
communication through I2C with the host device for the different
components.

Signed-off-by: Chris Zhong <redacted>
Signed-off-by: Zhang Qing <redacted>

Couple of nits.  Once fixed you can apply my:

Acked-by: Lee Jones <redacted>

[...]

+/*
+ * MFD core driver for Rockchip RK808
+ *
+ * Copyright (c) 2014, Fuzhou Rockchip Electronics Co., Ltd

Author?

+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ */

[...]

+static int rk808_probe(struct i2c_client *client,
+		       const struct i2c_device_id *id)
+{
+	int i;
+	int ret;
+	int pm_off = 0;
+	struct rk808 *rk808;
+	struct device_node *np = client->dev.of_node;

Reverse these declarations please, structs at the top etc.

[...]

+	rk808->i2c = client;
+	i2c_set_clientdata(client, rk808);

'\n' here.

+	ret = mfd_add_devices(&client->dev, -1,
+			      rk808s, ARRAY_SIZE(rk808s),
+			      NULL, 0, regmap_irq_get_domain(rk808->irq_data));
+	if (ret) {
+		dev_err(&client->dev, "failed to add MFD devices %d\n", ret);
+		goto err_irq;
+	}
+
+	if (np) {

There has to be an 'np'.  The driver depends on OF.

+		pm_off = of_property_read_bool(np,
+					"rockchip,system-power-controller");
+		if (pm_off && !pm_power_off) {
+			rk808_i2c_client = client;
+			pm_power_off = rk808_device_shutdown;
+		}
+	}
+
+	return 0;
+
+err_irq:
+	regmap_del_irq_chip(client->irq, rk808->irq_data);
+	return ret;
+}

[...]

+static struct i2c_driver rk808_i2c_driver = {
+	.driver = {
+		.name = "rk808",
+		.of_match_table = of_match_ptr(rk808_of_match),

No need to use of_match_ptr() now that you depend on OF.

+	},
+	.probe    = rk808_probe,
+	.remove   = rk808_remove,
+	.id_table = rk808_ids,
+};
+
+module_i2c_driver(rk808_i2c_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Chris Zhong [off-list ref]");
+MODULE_AUTHOR("Zhang Qing [off-list ref]");
+MODULE_DESCRIPTION("RK808 PMIC driver");

[...]

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html