When you send patch-sets, you should send them connected to one
another AKA threaded.  That way, when we're reviewing we can look at
the other patches in the set for reference.  See the man page for `git
send-email` for details.

<commit log>

Signed-off-by: Gyungoh Yoo <redacted>
---
 Documentation/devicetree/bindings/mfd/sky81452.txt |  24 +++++

Binding documents should be sent separately:

See: Documentation/devicetree/bindings/submitting-patches.txt

 .../devicetree/bindings/vendor-prefixes.txt        |   1 +

This can go in with the documentation.

Lower case.

+		compatible = "skyworks,sky81452";

"reg"?

You also need to document the 'compatible' and 'reg' properties.

+		backlight {
+			...
+		};
+
+		regulator {
+			...
+		};
+	};

I think it would be helpful to place a fully populated example in
here.

MFD_SKY81452

+	tristate "Skyworks Solutions SKY81452"
+	select MFD_CORE
+	select REGMAP_I2C
+	depends on I2C=y

Why do you need I2C to be built-in, rather than as a module?

Have you tested this with DT?

You're not passing the compatible string and not using
of_platform_populate() so I'm struggling to see how it would work
properly.

+		},
+		{
+			.name = "sky81452-regulator",
+			.platform_data = pdata->regulator_init_data,
+			.pdata_size = sizeof(*pdata->regulator_init_data),
+		},
+	};

Please declare this outside of the function?

+	return mfd_add_devices(dev, -1, cells, ARRAY_SIZE(cells),
+			NULL, 0, NULL);

This doesn't really need to be in a function of its own.  Please put
it in .probe().  Also check for the return value and present the user
with an error message if it fails.

+}
+
+static int sky81452_probe(struct i2c_client *client,
+		const struct i2c_device_id *id)

Line up against '('.

+{
+	struct device *dev = &client->dev;
+	const struct sky81452_platform_data *pdata = dev_get_platdata(dev);
+	struct regmap *map;

Can you call this 'regmap' for clarity.

+	if (!pdata) {
+		pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+		if (!pdata)
+			return -ENOMEM;
+	}
+
+	map = devm_regmap_init_i2c(client, &sky81452_config);
+	if (IS_ERR(map))
+		return PTR_ERR(map);

I'm not sure you want this to fail silently.

+	i2c_set_clientdata(client, map);
+
+	return sky81452_register_devices(dev, pdata);
+}
+
+static int sky81452_remove(struct i2c_client *client)
+{
+	mfd_remove_devices(&client->dev);
+	return 0;
+}
+
+static const struct i2c_device_id sky81452_ids[] = {
+	{ "sky81452", 0 },

Remove the second attribute, it's unused.

+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, sky81452_ids);
+
+#ifdef CONFIG_OF
+static const struct of_device_id sky81452_of_match[] = {
+	{ .compatible = "skyworks,sky81452", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, sky81452_of_match);
+#endif

You can drop the #differy the compiler should sort that out on the
back of of_match_ptr().

+static struct i2c_driver sky81452_driver = {
+	.driver = {
+		.name = "sky81452",
+		.of_match_table = of_match_ptr(sky81452_of_match),
+	},
+	.probe = sky81452_probe,
+	.remove = sky81452_remove,
+	.id_table = sky81452_ids,
+};
+
+module_i2c_driver(sky81452_driver);
+
+MODULE_DESCRIPTION("Skyworks SKY81452 MFD driver");
+MODULE_AUTHOR("Gyungoh Yoo [off-list ref]");
+MODULE_LICENSE("GPL");

I think you want v2.

If it's a just a backlight driver, why is it in MFD?

You'd best expand the description here.

+ * Copyright 2014 Skyworks Solutions Inc.
+ * Author : Gyungoh Yoo [off-list ref]
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2, or (at your option) any
+ * later version.
+ *
+ * This program is distributed in the hope that 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.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef _SKY81452_H
+#define _SKY81452_H
+
+#include "linux/sky81452-backlight.h"
+#include "linux/regulator/machine.h"

s/"/< and >

+struct sky81452_platform_data {
+	struct sky81452_bl_platform_data *bl_pdata;
+	struct regulator_init_data *regulator_init_data;
+};
+
+#endif

-- 
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