[PATCH v4] ARM/mx35/3ds: gpio: add mc9s08dz60 gpio function

[PATCH v4] ARM/mx35/3ds: gpio: add mc9s08dz60 gpio function

[wu guoxing]

we only use the gpio function of mc9s08dz60 mcu chip, so just
add the gpio driver, as this mcu will never be used in other board.

Signed-off-by: Wu Guoxing <b39297@freescale.com>
changes since v3:
	1. set i2c client data before gpiochip_add
	2. return error value in mc9s08dz60_direction_output
v3 Changes since v1:
	1. add can_sleep = 1
	2. removed some useless return checks and local variable
	3. removed the static variable for i2c client.
	4. make init&exit function static

---
 drivers/gpio/Kconfig           |    6 ++
 drivers/gpio/Makefile          |    1 +
 drivers/gpio/gpio-mc9s08dz60.c |  177 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 184 insertions(+), 0 deletions(-)
 create mode 100644 drivers/gpio/gpio-mc9s08dz60.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index d539efd..7f6beee 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -487,4 +487,10 @@ config GPIO_TPS65910
 	help
 	  Select this option to enable GPIO driver for the TPS65910
 	  chip family.
+
+config GPIO_MC9S08DZ60
+	bool "MX35 3DS BOARD MC9S08DZ60 GPIO functions"
+	depends on I2C && MACH_MX35_3DS
+	help
+	  Select this to enable the MC9S08DZ60 GPIO driver
 endif
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 9588948..85b8799 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -60,3 +60,4 @@ obj-$(CONFIG_GPIO_WM831X)	+= gpio-wm831x.o
 obj-$(CONFIG_GPIO_WM8350)	+= gpio-wm8350.o
 obj-$(CONFIG_GPIO_WM8994)	+= gpio-wm8994.o
 obj-$(CONFIG_GPIO_XILINX)	+= gpio-xilinx.o
+obj-$(CONFIG_GPIO_MC9S08DZ60) += gpio-mc9s08dz60.o
diff --git a/drivers/gpio/gpio-mc9s08dz60.c b/drivers/gpio/gpio-mc9s08dz60.c
new file mode 100644
index 0000000..0122ef8
--- /dev/null
+++ b/drivers/gpio/gpio-mc9s08dz60.c
@@ -0,0 +1,177 @@
+/*
+ * Copyright 2009-2011 Freescale Semiconductor, Inc. All Rights Reserved.
+ *
+ * Author: Wu Guoxing <b39297@freescale.com>
+ *
+ * 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 of the License, 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.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/i2c.h>
+#include <linux/gpio.h>
+
+#define GPIO_GROUP_NUM 2
+#define GPIO_NUM_PER_GROUP 8
+#define GPIO_NUM (GPIO_GROUP_NUM*GPIO_NUM_PER_GROUP)
+
+struct mc9s08dz60 {
+	struct i2c_client *client;
+	struct gpio_chip chip;
+};
+
+static struct mc9s08dz60 *to_mc9s08dz60(struct gpio_chip *gc)
+{
+	return container_of(gc, struct mc9s08dz60, chip);
+}
+
+
+static void gpio_to_reg_and_bit(int offset, u8 *reg, u8 *bit)
+{
+	*reg = 0x20 + offset / GPIO_NUM_PER_GROUP;
+	*bit = offset % GPIO_NUM_PER_GROUP;
+}
+
+static int mc9s08dz60_get_value(struct gpio_chip *gc, unsigned offset)
+{
+	int ret = 0;
+	u8 reg, bit;
+	s32 value;
+	struct mc9s08dz60 *mc9s = to_mc9s08dz60(gc);
+
+	gpio_to_reg_and_bit(offset, &reg, &bit);
+	value = i2c_smbus_read_byte_data(mc9s->client, reg);
+	if (value >= 0)
+		ret = (value >> bit) & 0x1;
+
+	return ret;
+}
+
+static int _mc9s08dz60_set(struct mc9s08dz60 *mc9s, unsigned offset, int val)
+{
+	u8 reg, bit;
+	s32 value;
+
+	gpio_to_reg_and_bit(offset, &reg, &bit);
+	value = i2c_smbus_read_byte_data(mc9s->client, reg);
+	if (value >= 0) {
+		if (val)
+			value |= 1 << bit;
+		else
+			value &= ~(1 << bit);
+
+		return i2c_smbus_write_byte_data(mc9s->client, reg, value);
+	} else
+		return value;
+
+}
+
+
+static void mc9s08dz60_set_value(struct gpio_chip *gc, unsigned offset, int val)
+{
+	struct mc9s08dz60 *mc9s = to_mc9s08dz60(gc);
+
+	_mc9s08dz60_set(mc9s, offset, val);
+}
+
+static int mc9s08dz60_direction_output(struct gpio_chip *gc,
+				       unsigned offset, int val)
+{
+	struct mc9s08dz60 *mc9s = to_mc9s08dz60(gc);
+
+	return _mc9s08dz60_set(mc9s, offset, val);
+}
+
+static int mc9s08dz60_probe(struct i2c_client *client,
+			    const struct i2c_device_id *id)
+{
+	int ret = 0;
+	struct mc9s08dz60 *mc9s;
+
+	mc9s = kzalloc(sizeof(*mc9s), GFP_KERNEL);
+	if (!mc9s)
+		return -ENOMEM;
+
+	mc9s->chip.label = client->name;
+	mc9s->chip.base = -1;
+	mc9s->chip.dev = &client->dev;
+	mc9s->chip.owner = THIS_MODULE;
+	mc9s->chip.ngpio = GPIO_NUM;
+	mc9s->chip.can_sleep = 1;
+	mc9s->chip.get = mc9s08dz60_get_value;
+	mc9s->chip.set = mc9s08dz60_set_value;
+	mc9s->chip.direction_output = mc9s08dz60_direction_output;
+	mc9s->client = client;
+	i2c_set_clientdata(client, mc9s);
+
+	ret = gpiochip_add(&mc9s->chip);
+	if (ret)
+		goto error;
+
+	return 0;
+
+error:
+	i2c_set_clientdata(client, NULL);
+	kfree(mc9s);
+	return ret;
+}
+
+static int mc9s08dz60_remove(struct i2c_client *client)
+{
+	struct mc9s08dz60 *mc9s;
+	int ret;
+
+	mc9s = i2c_get_clientdata(client);
+
+	i2c_set_clientdata(client, NULL);
+
+	ret = gpiochip_remove(&mc9s->chip);
+	if (!ret)
+		kfree(mc9s);
+
+	return 0;
+
+}
+
+static const struct i2c_device_id mc9s08dz60_id[] = {
+	{"mc9s08dz60", 0},
+	{},
+};
+
+MODULE_DEVICE_TABLE(i2c, mc9s08dz60_id);
+
+static struct i2c_driver mc9s08dz60_i2c_driver = {
+	.driver = {.owner = THIS_MODULE,
+		   .name = "mc9s08dz60",
+		   },
+	.probe = mc9s08dz60_probe,
+	.remove = mc9s08dz60_remove,
+	.id_table = mc9s08dz60_id,
+};
+
+static int __init mc9s08dz60_init(void)
+{
+	return i2c_add_driver(&mc9s08dz60_i2c_driver);
+}
+
+static void __exit mc9s08dz60_exit(void)
+{
+	i2c_del_driver(&mc9s08dz60_i2c_driver);
+}
+
+module_init(mc9s08dz60_init);
+module_exit(mc9s08dz60_exit);
+
+MODULE_AUTHOR("Freescale Semiconductor, Inc."
+		"Wu Guoxing <b39297@freescale.com>");
+MODULE_DESCRIPTION("mc9s08dz60 gpio function on mx35 3ds board");
+MODULE_LICENSE("GPL v2");
-- 
1.7.1

[PATCH v4] ARM/mx35/3ds: gpio: add mc9s08dz60 gpio function

[y at shlinux1.ap.freescale.net]

From: wu guoxing <b39297@freescale.com>

we only use the gpio function of mc9s08dz60 mcu chip, so just
add the gpio driver, as this mcu will never be used in other board.

Signed-off-by: Wu Guoxing <b39297@freescale.com>
---
changes since v3:
	1. set i2c client data before gpiochip_add
	2. return error value in mc9s08dz60_direction_output
v3 Changes since v1:
	1. add can_sleep = 1
	2. removed some useless return checks and local variable
	3. removed the static variable for i2c client.
	4. make init&exit function static

 drivers/gpio/Kconfig           |    6 ++
 drivers/gpio/Makefile          |    1 +
 drivers/gpio/gpio-mc9s08dz60.c |  177 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 184 insertions(+), 0 deletions(-)
 create mode 100644 drivers/gpio/gpio-mc9s08dz60.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index d539efd..7f6beee 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -487,4 +487,10 @@ config GPIO_TPS65910
 	help
 	  Select this option to enable GPIO driver for the TPS65910
 	  chip family.
+
+config GPIO_MC9S08DZ60
+	bool "MX35 3DS BOARD MC9S08DZ60 GPIO functions"
+	depends on I2C && MACH_MX35_3DS
+	help
+	  Select this to enable the MC9S08DZ60 GPIO driver
 endif
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 9588948..85b8799 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -60,3 +60,4 @@ obj-$(CONFIG_GPIO_WM831X)	+= gpio-wm831x.o
 obj-$(CONFIG_GPIO_WM8350)	+= gpio-wm8350.o
 obj-$(CONFIG_GPIO_WM8994)	+= gpio-wm8994.o
 obj-$(CONFIG_GPIO_XILINX)	+= gpio-xilinx.o
+obj-$(CONFIG_GPIO_MC9S08DZ60) += gpio-mc9s08dz60.o
diff --git a/drivers/gpio/gpio-mc9s08dz60.c b/drivers/gpio/gpio-mc9s08dz60.c
new file mode 100644
index 0000000..0122ef8
--- /dev/null
+++ b/drivers/gpio/gpio-mc9s08dz60.c
@@ -0,0 +1,177 @@
+/*
+ * Copyright 2009-2011 Freescale Semiconductor, Inc. All Rights Reserved.
+ *
+ * Author: Wu Guoxing <b39297@freescale.com>
+ *
+ * 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 of the License, 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.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/i2c.h>
+#include <linux/gpio.h>
+
+#define GPIO_GROUP_NUM 2
+#define GPIO_NUM_PER_GROUP 8
+#define GPIO_NUM (GPIO_GROUP_NUM*GPIO_NUM_PER_GROUP)
+
+struct mc9s08dz60 {
+	struct i2c_client *client;
+	struct gpio_chip chip;
+};
+
+static struct mc9s08dz60 *to_mc9s08dz60(struct gpio_chip *gc)
+{
+	return container_of(gc, struct mc9s08dz60, chip);
+}
+
+
+static void gpio_to_reg_and_bit(int offset, u8 *reg, u8 *bit)
+{
+	*reg = 0x20 + offset / GPIO_NUM_PER_GROUP;
+	*bit = offset % GPIO_NUM_PER_GROUP;
+}
+
+static int mc9s08dz60_get_value(struct gpio_chip *gc, unsigned offset)
+{
+	int ret = 0;
+	u8 reg, bit;
+	s32 value;
+	struct mc9s08dz60 *mc9s = to_mc9s08dz60(gc);
+
+	gpio_to_reg_and_bit(offset, &reg, &bit);
+	value = i2c_smbus_read_byte_data(mc9s->client, reg);
+	if (value >= 0)
+		ret = (value >> bit) & 0x1;
+
+	return ret;
+}
+
+static int _mc9s08dz60_set(struct mc9s08dz60 *mc9s, unsigned offset, int val)
+{
+	u8 reg, bit;
+	s32 value;
+
+	gpio_to_reg_and_bit(offset, &reg, &bit);
+	value = i2c_smbus_read_byte_data(mc9s->client, reg);
+	if (value >= 0) {
+		if (val)
+			value |= 1 << bit;
+		else
+			value &= ~(1 << bit);
+
+		return i2c_smbus_write_byte_data(mc9s->client, reg, value);
+	} else
+		return value;
+
+}
+
+
+static void mc9s08dz60_set_value(struct gpio_chip *gc, unsigned offset, int val)
+{
+	struct mc9s08dz60 *mc9s = to_mc9s08dz60(gc);
+
+	_mc9s08dz60_set(mc9s, offset, val);
+}
+
+static int mc9s08dz60_direction_output(struct gpio_chip *gc,
+				       unsigned offset, int val)
+{
+	struct mc9s08dz60 *mc9s = to_mc9s08dz60(gc);
+
+	return _mc9s08dz60_set(mc9s, offset, val);
+}
+
+static int mc9s08dz60_probe(struct i2c_client *client,
+			    const struct i2c_device_id *id)
+{
+	int ret = 0;
+	struct mc9s08dz60 *mc9s;
+
+	mc9s = kzalloc(sizeof(*mc9s), GFP_KERNEL);
+	if (!mc9s)
+		return -ENOMEM;
+
+	mc9s->chip.label = client->name;
+	mc9s->chip.base = -1;
+	mc9s->chip.dev = &client->dev;
+	mc9s->chip.owner = THIS_MODULE;
+	mc9s->chip.ngpio = GPIO_NUM;
+	mc9s->chip.can_sleep = 1;
+	mc9s->chip.get = mc9s08dz60_get_value;
+	mc9s->chip.set = mc9s08dz60_set_value;
+	mc9s->chip.direction_output = mc9s08dz60_direction_output;
+	mc9s->client = client;
+	i2c_set_clientdata(client, mc9s);
+
+	ret = gpiochip_add(&mc9s->chip);
+	if (ret)
+		goto error;
+
+	return 0;
+
+error:
+	i2c_set_clientdata(client, NULL);
+	kfree(mc9s);
+	return ret;
+}
+
+static int mc9s08dz60_remove(struct i2c_client *client)
+{
+	struct mc9s08dz60 *mc9s;
+	int ret;
+
+	mc9s = i2c_get_clientdata(client);
+
+	i2c_set_clientdata(client, NULL);
+
+	ret = gpiochip_remove(&mc9s->chip);
+	if (!ret)
+		kfree(mc9s);
+
+	return 0;
+
+}
+
+static const struct i2c_device_id mc9s08dz60_id[] = {
+	{"mc9s08dz60", 0},
+	{},
+};
+
+MODULE_DEVICE_TABLE(i2c, mc9s08dz60_id);
+
+static struct i2c_driver mc9s08dz60_i2c_driver = {
+	.driver = {.owner = THIS_MODULE,
+		   .name = "mc9s08dz60",
+		   },
+	.probe = mc9s08dz60_probe,
+	.remove = mc9s08dz60_remove,
+	.id_table = mc9s08dz60_id,
+};
+
+static int __init mc9s08dz60_init(void)
+{
+	return i2c_add_driver(&mc9s08dz60_i2c_driver);
+}
+
+static void __exit mc9s08dz60_exit(void)
+{
+	i2c_del_driver(&mc9s08dz60_i2c_driver);
+}
+
+module_init(mc9s08dz60_init);
+module_exit(mc9s08dz60_exit);
+
+MODULE_AUTHOR("Freescale Semiconductor, Inc."
+		"Wu Guoxing <b39297@freescale.com>");
+MODULE_DESCRIPTION("mc9s08dz60 gpio function on mx35 3ds board");
+MODULE_LICENSE("GPL v2");
-- 
1.7.1

[PATCH v4] ARM/mx35/3ds: gpio: add mc9s08dz60 gpio function

[wu guoxing]

we only use the gpio function of mc9s08dz60 mcu chip, so just
add the gpio driver, as this mcu will never be used in other board.

Signed-off-by: Wu Guoxing <b39297@freescale.com>
---
changes since v3:
	1. set i2c client data before gpiochip_add
	2. return error value in mc9s08dz60_direction_output
v3 Changes since v1:
	1. add can_sleep = 1
	2. removed some useless return checks and local variable
	3. removed the static variable for i2c client.
	4. make init&exit function static

 drivers/gpio/Kconfig           |    6 ++
 drivers/gpio/Makefile          |    1 +
 drivers/gpio/gpio-mc9s08dz60.c |  177 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 184 insertions(+), 0 deletions(-)
 create mode 100644 drivers/gpio/gpio-mc9s08dz60.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index d539efd..7f6beee 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -487,4 +487,10 @@ config GPIO_TPS65910
 	help
 	  Select this option to enable GPIO driver for the TPS65910
 	  chip family.
+
+config GPIO_MC9S08DZ60
+	bool "MX35 3DS BOARD MC9S08DZ60 GPIO functions"
+	depends on I2C && MACH_MX35_3DS
+	help
+	  Select this to enable the MC9S08DZ60 GPIO driver
 endif
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 9588948..85b8799 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -60,3 +60,4 @@ obj-$(CONFIG_GPIO_WM831X)	+= gpio-wm831x.o
 obj-$(CONFIG_GPIO_WM8350)	+= gpio-wm8350.o
 obj-$(CONFIG_GPIO_WM8994)	+= gpio-wm8994.o
 obj-$(CONFIG_GPIO_XILINX)	+= gpio-xilinx.o
+obj-$(CONFIG_GPIO_MC9S08DZ60) += gpio-mc9s08dz60.o
diff --git a/drivers/gpio/gpio-mc9s08dz60.c b/drivers/gpio/gpio-mc9s08dz60.c
new file mode 100644
index 0000000..0122ef8
--- /dev/null
+++ b/drivers/gpio/gpio-mc9s08dz60.c
@@ -0,0 +1,177 @@
+/*
+ * Copyright 2009-2011 Freescale Semiconductor, Inc. All Rights Reserved.
+ *
+ * Author: Wu Guoxing <b39297@freescale.com>
+ *
+ * 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 of the License, 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.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/i2c.h>
+#include <linux/gpio.h>
+
+#define GPIO_GROUP_NUM 2
+#define GPIO_NUM_PER_GROUP 8
+#define GPIO_NUM (GPIO_GROUP_NUM*GPIO_NUM_PER_GROUP)
+
+struct mc9s08dz60 {
+	struct i2c_client *client;
+	struct gpio_chip chip;
+};
+
+static struct mc9s08dz60 *to_mc9s08dz60(struct gpio_chip *gc)
+{
+	return container_of(gc, struct mc9s08dz60, chip);
+}
+
+
+static void gpio_to_reg_and_bit(int offset, u8 *reg, u8 *bit)
+{
+	*reg = 0x20 + offset / GPIO_NUM_PER_GROUP;
+	*bit = offset % GPIO_NUM_PER_GROUP;
+}
+
+static int mc9s08dz60_get_value(struct gpio_chip *gc, unsigned offset)
+{
+	int ret = 0;
+	u8 reg, bit;
+	s32 value;
+	struct mc9s08dz60 *mc9s = to_mc9s08dz60(gc);
+
+	gpio_to_reg_and_bit(offset, &reg, &bit);
+	value = i2c_smbus_read_byte_data(mc9s->client, reg);
+	if (value >= 0)
+		ret = (value >> bit) & 0x1;
+
+	return ret;
+}
+
+static int _mc9s08dz60_set(struct mc9s08dz60 *mc9s, unsigned offset, int val)
+{
+	u8 reg, bit;
+	s32 value;
+
+	gpio_to_reg_and_bit(offset, &reg, &bit);
+	value = i2c_smbus_read_byte_data(mc9s->client, reg);
+	if (value >= 0) {
+		if (val)
+			value |= 1 << bit;
+		else
+			value &= ~(1 << bit);
+
+		return i2c_smbus_write_byte_data(mc9s->client, reg, value);
+	} else
+		return value;
+
+}
+
+
+static void mc9s08dz60_set_value(struct gpio_chip *gc, unsigned offset, int val)
+{
+	struct mc9s08dz60 *mc9s = to_mc9s08dz60(gc);
+
+	_mc9s08dz60_set(mc9s, offset, val);
+}
+
+static int mc9s08dz60_direction_output(struct gpio_chip *gc,
+				       unsigned offset, int val)
+{
+	struct mc9s08dz60 *mc9s = to_mc9s08dz60(gc);
+
+	return _mc9s08dz60_set(mc9s, offset, val);
+}
+
+static int mc9s08dz60_probe(struct i2c_client *client,
+			    const struct i2c_device_id *id)
+{
+	int ret = 0;
+	struct mc9s08dz60 *mc9s;
+
+	mc9s = kzalloc(sizeof(*mc9s), GFP_KERNEL);
+	if (!mc9s)
+		return -ENOMEM;
+
+	mc9s->chip.label = client->name;
+	mc9s->chip.base = -1;
+	mc9s->chip.dev = &client->dev;
+	mc9s->chip.owner = THIS_MODULE;
+	mc9s->chip.ngpio = GPIO_NUM;
+	mc9s->chip.can_sleep = 1;
+	mc9s->chip.get = mc9s08dz60_get_value;
+	mc9s->chip.set = mc9s08dz60_set_value;
+	mc9s->chip.direction_output = mc9s08dz60_direction_output;
+	mc9s->client = client;
+	i2c_set_clientdata(client, mc9s);
+
+	ret = gpiochip_add(&mc9s->chip);
+	if (ret)
+		goto error;
+
+	return 0;
+
+error:
+	i2c_set_clientdata(client, NULL);
+	kfree(mc9s);
+	return ret;
+}
+
+static int mc9s08dz60_remove(struct i2c_client *client)
+{
+	struct mc9s08dz60 *mc9s;
+	int ret;
+
+	mc9s = i2c_get_clientdata(client);
+
+	i2c_set_clientdata(client, NULL);
+
+	ret = gpiochip_remove(&mc9s->chip);
+	if (!ret)
+		kfree(mc9s);
+
+	return 0;
+
+}
+
+static const struct i2c_device_id mc9s08dz60_id[] = {
+	{"mc9s08dz60", 0},
+	{},
+};
+
+MODULE_DEVICE_TABLE(i2c, mc9s08dz60_id);
+
+static struct i2c_driver mc9s08dz60_i2c_driver = {
+	.driver = {.owner = THIS_MODULE,
+		   .name = "mc9s08dz60",
+		   },
+	.probe = mc9s08dz60_probe,
+	.remove = mc9s08dz60_remove,
+	.id_table = mc9s08dz60_id,
+};
+
+static int __init mc9s08dz60_init(void)
+{
+	return i2c_add_driver(&mc9s08dz60_i2c_driver);
+}
+
+static void __exit mc9s08dz60_exit(void)
+{
+	i2c_del_driver(&mc9s08dz60_i2c_driver);
+}
+
+module_init(mc9s08dz60_init);
+module_exit(mc9s08dz60_exit);
+
+MODULE_AUTHOR("Freescale Semiconductor, Inc."
+		"Wu Guoxing <b39297@freescale.com>");
+MODULE_DESCRIPTION("mc9s08dz60 gpio function on mx35 3ds board");
+MODULE_LICENSE("GPL v2");
-- 
1.7.1

[PATCH v4] ARM/mx35/3ds: gpio: add mc9s08dz60 gpio function

[Marc Kleine-Budde]

On 11/10/2011 03:24 AM, wu guoxing wrote:
> we only use the gpio function of mc9s08dz60 mcu chip, so just
> add the gpio driver, as this mcu will never be used in other board.
> 
> Signed-off-by: Wu Guoxing <b39297@freescale.com>
> ---
> changes since v3:
> 	1. set i2c client data before gpiochip_add

exchange the two in the remove function, too.
First shut down the gpio chip, then unset set i2c client data.

Marc

> 	2. return error value in mc9s08dz60_direction_output
> v3 Changes since v1:
> 	1. add can_sleep = 1
> 	2. removed some useless return checks and local variable
> 	3. removed the static variable for i2c client.
> 	4. make init&exit function static
> 
>  drivers/gpio/Kconfig           |    6 ++
>  drivers/gpio/Makefile          |    1 +
>  drivers/gpio/gpio-mc9s08dz60.c |  177 ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 184 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/gpio/gpio-mc9s08dz60.c
> 
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index d539efd..7f6beee 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -487,4 +487,10 @@ config GPIO_TPS65910
>  	help
>  	  Select this option to enable GPIO driver for the TPS65910
>  	  chip family.
> +
> +config GPIO_MC9S08DZ60
> +	bool "MX35 3DS BOARD MC9S08DZ60 GPIO functions"
> +	depends on I2C && MACH_MX35_3DS
> +	help
> +	  Select this to enable the MC9S08DZ60 GPIO driver
>  endif
> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
> index 9588948..85b8799 100644
> --- a/drivers/gpio/Makefile
> +++ b/drivers/gpio/Makefile
> @@ -60,3 +60,4 @@ obj-$(CONFIG_GPIO_WM831X)	+= gpio-wm831x.o
>  obj-$(CONFIG_GPIO_WM8350)	+= gpio-wm8350.o
>  obj-$(CONFIG_GPIO_WM8994)	+= gpio-wm8994.o
>  obj-$(CONFIG_GPIO_XILINX)	+= gpio-xilinx.o
> +obj-$(CONFIG_GPIO_MC9S08DZ60) += gpio-mc9s08dz60.o
> diff --git a/drivers/gpio/gpio-mc9s08dz60.c b/drivers/gpio/gpio-mc9s08dz60.c
> new file mode 100644
> index 0000000..0122ef8
> --- /dev/null
> +++ b/drivers/gpio/gpio-mc9s08dz60.c
> @@ -0,0 +1,177 @@
> +/*
> + * Copyright 2009-2011 Freescale Semiconductor, Inc. All Rights Reserved.
> + *
> + * Author: Wu Guoxing <b39297@freescale.com>
> + *
> + * 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 of the License, 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.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/i2c.h>
> +#include <linux/gpio.h>
> +
> +#define GPIO_GROUP_NUM 2
> +#define GPIO_NUM_PER_GROUP 8
> +#define GPIO_NUM (GPIO_GROUP_NUM*GPIO_NUM_PER_GROUP)
> +
> +struct mc9s08dz60 {
> +	struct i2c_client *client;
> +	struct gpio_chip chip;
> +};
> +
> +static struct mc9s08dz60 *to_mc9s08dz60(struct gpio_chip *gc)
> +{
> +	return container_of(gc, struct mc9s08dz60, chip);
> +}
> +
> +
> +static void gpio_to_reg_and_bit(int offset, u8 *reg, u8 *bit)
> +{
> +	*reg = 0x20 + offset / GPIO_NUM_PER_GROUP;
> +	*bit = offset % GPIO_NUM_PER_GROUP;
> +}
> +
> +static int mc9s08dz60_get_value(struct gpio_chip *gc, unsigned offset)
> +{
> +	int ret = 0;
> +	u8 reg, bit;
> +	s32 value;
> +	struct mc9s08dz60 *mc9s = to_mc9s08dz60(gc);
> +
> +	gpio_to_reg_and_bit(offset, &reg, &bit);
> +	value = i2c_smbus_read_byte_data(mc9s->client, reg);
> +	if (value >= 0)
> +		ret = (value >> bit) & 0x1;
> +
> +	return ret;
> +}
> +
> +static int _mc9s08dz60_set(struct mc9s08dz60 *mc9s, unsigned offset, int val)
> +{
> +	u8 reg, bit;
> +	s32 value;
> +
> +	gpio_to_reg_and_bit(offset, &reg, &bit);
> +	value = i2c_smbus_read_byte_data(mc9s->client, reg);
> +	if (value >= 0) {
> +		if (val)
> +			value |= 1 << bit;
> +		else
> +			value &= ~(1 << bit);
> +
> +		return i2c_smbus_write_byte_data(mc9s->client, reg, value);
> +	} else
> +		return value;
> +
> +}
> +
> +
> +static void mc9s08dz60_set_value(struct gpio_chip *gc, unsigned offset, int val)
> +{
> +	struct mc9s08dz60 *mc9s = to_mc9s08dz60(gc);
> +
> +	_mc9s08dz60_set(mc9s, offset, val);
> +}
> +
> +static int mc9s08dz60_direction_output(struct gpio_chip *gc,
> +				       unsigned offset, int val)
> +{
> +	struct mc9s08dz60 *mc9s = to_mc9s08dz60(gc);
> +
> +	return _mc9s08dz60_set(mc9s, offset, val);
> +}
> +
> +static int mc9s08dz60_probe(struct i2c_client *client,
> +			    const struct i2c_device_id *id)
> +{
> +	int ret = 0;
> +	struct mc9s08dz60 *mc9s;
> +
> +	mc9s = kzalloc(sizeof(*mc9s), GFP_KERNEL);
> +	if (!mc9s)
> +		return -ENOMEM;
> +
> +	mc9s->chip.label = client->name;
> +	mc9s->chip.base = -1;
> +	mc9s->chip.dev = &client->dev;
> +	mc9s->chip.owner = THIS_MODULE;
> +	mc9s->chip.ngpio = GPIO_NUM;
> +	mc9s->chip.can_sleep = 1;
> +	mc9s->chip.get = mc9s08dz60_get_value;
> +	mc9s->chip.set = mc9s08dz60_set_value;
> +	mc9s->chip.direction_output = mc9s08dz60_direction_output;
> +	mc9s->client = client;
> +	i2c_set_clientdata(client, mc9s);
> +
> +	ret = gpiochip_add(&mc9s->chip);
> +	if (ret)
> +		goto error;
> +
> +	return 0;
> +
> +error:
> +	i2c_set_clientdata(client, NULL);
> +	kfree(mc9s);
> +	return ret;
> +}
> +
> +static int mc9s08dz60_remove(struct i2c_client *client)
> +{
> +	struct mc9s08dz60 *mc9s;
> +	int ret;
> +
> +	mc9s = i2c_get_clientdata(client);
> +
> +	i2c_set_clientdata(client, NULL);
> +
> +	ret = gpiochip_remove(&mc9s->chip);
> +	if (!ret)
> +		kfree(mc9s);
> +
> +	return 0;
> +
> +}
> +
> +static const struct i2c_device_id mc9s08dz60_id[] = {
> +	{"mc9s08dz60", 0},
> +	{},
> +};
> +
> +MODULE_DEVICE_TABLE(i2c, mc9s08dz60_id);
> +
> +static struct i2c_driver mc9s08dz60_i2c_driver = {
> +	.driver = {.owner = THIS_MODULE,
> +		   .name = "mc9s08dz60",
> +		   },
> +	.probe = mc9s08dz60_probe,
> +	.remove = mc9s08dz60_remove,
> +	.id_table = mc9s08dz60_id,
> +};
> +
> +static int __init mc9s08dz60_init(void)
> +{
> +	return i2c_add_driver(&mc9s08dz60_i2c_driver);
> +}
> +
> +static void __exit mc9s08dz60_exit(void)
> +{
> +	i2c_del_driver(&mc9s08dz60_i2c_driver);
> +}
> +
> +module_init(mc9s08dz60_init);
> +module_exit(mc9s08dz60_exit);
> +
> +MODULE_AUTHOR("Freescale Semiconductor, Inc."
> +		"Wu Guoxing <b39297@freescale.com>");
> +MODULE_DESCRIPTION("mc9s08dz60 gpio function on mx35 3ds board");
> +MODULE_LICENSE("GPL v2");


-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 262 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20111110/a384a383/attachment.sig>

[PATCH v4] ARM/mx35/3ds: gpio: add mc9s08dz60 gpio function

[Wolfram Sang]

On Thu, Nov 10, 2011 at 11:13:42AM +0100, Marc Kleine-Budde wrote:
> On 11/10/2011 03:24 AM, wu guoxing wrote:
> > we only use the gpio function of mc9s08dz60 mcu chip, so just
> > add the gpio driver, as this mcu will never be used in other board.
> > 
> > Signed-off-by: Wu Guoxing <b39297@freescale.com>
> > ---
> > changes since v3:
> > 	1. set i2c client data before gpiochip_add
> 
> exchange the two in the remove function, too.
> First shut down the gpio chip, then unset set i2c client data.

and don't set clientdata to NULL.

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20111110/6c116f76/attachment.sig>

[PATCH v4] ARM/mx35/3ds: gpio: add mc9s08dz60 gpio function

[Wu Guoxing-B39297]

Hi Marc:

  I changed the code to unset i2c client data just when gipochip_remove successes,
Otherwise, there will be problems next time the remove function is called.

  Please review the v5 patch.

  Thanks!

Best regards
Wu Guoxing

-----Original Message-----
From: Marc Kleine-Budde [mailto:mkl at pengutronix.de] 
Sent: Thursday, November 10, 2011 6:14 PM
To: Wu Guoxing-B39297
Cc: linux-arm-kernel at lists.infradead.org; grant.likely at secretlab.ca; s.hauer at pengutronix.de; shawn.guo at linaro.org
Subject: Re: [PATCH v4] ARM/mx35/3ds: gpio: add mc9s08dz60 gpio function

On 11/10/2011 03:24 AM, wu guoxing wrote:
> we only use the gpio function of mc9s08dz60 mcu chip, so just add the 
> gpio driver, as this mcu will never be used in other board.
> 
> Signed-off-by: Wu Guoxing <b39297@freescale.com>
> ---
> changes since v3:
> 	1. set i2c client data before gpiochip_add

exchange the two in the remove function, too.
First shut down the gpio chip, then unset set i2c client data.

Marc

> 	2. return error value in mc9s08dz60_direction_output
> v3 Changes since v1:
> 	1. add can_sleep = 1
> 	2. removed some useless return checks and local variable
> 	3. removed the static variable for i2c client.
> 	4. make init&exit function static
> 
>  drivers/gpio/Kconfig           |    6 ++
>  drivers/gpio/Makefile          |    1 +
>  drivers/gpio/gpio-mc9s08dz60.c |  177 
> ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 184 insertions(+), 0 deletions(-)  create mode 
> 100644 drivers/gpio/gpio-mc9s08dz60.c
> 
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index 
> d539efd..7f6beee 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -487,4 +487,10 @@ config GPIO_TPS65910
>  	help
>  	  Select this option to enable GPIO driver for the TPS65910
>  	  chip family.
> +
> +config GPIO_MC9S08DZ60
> +	bool "MX35 3DS BOARD MC9S08DZ60 GPIO functions"
> +	depends on I2C && MACH_MX35_3DS
> +	help
> +	  Select this to enable the MC9S08DZ60 GPIO driver
>  endif
> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile index 
> 9588948..85b8799 100644
> --- a/drivers/gpio/Makefile
> +++ b/drivers/gpio/Makefile
> @@ -60,3 +60,4 @@ obj-$(CONFIG_GPIO_WM831X)	+= gpio-wm831x.o
>  obj-$(CONFIG_GPIO_WM8350)	+= gpio-wm8350.o
>  obj-$(CONFIG_GPIO_WM8994)	+= gpio-wm8994.o
>  obj-$(CONFIG_GPIO_XILINX)	+= gpio-xilinx.o
> +obj-$(CONFIG_GPIO_MC9S08DZ60) += gpio-mc9s08dz60.o
> diff --git a/drivers/gpio/gpio-mc9s08dz60.c 
> b/drivers/gpio/gpio-mc9s08dz60.c new file mode 100644 index 
> 0000000..0122ef8
> --- /dev/null
> +++ b/drivers/gpio/gpio-mc9s08dz60.c
> @@ -0,0 +1,177 @@
> +/*
> + * Copyright 2009-2011 Freescale Semiconductor, Inc. All Rights Reserved.
> + *
> + * Author: Wu Guoxing <b39297@freescale.com>
> + *
> + * 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 of the License, 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.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/i2c.h>
> +#include <linux/gpio.h>
> +
> +#define GPIO_GROUP_NUM 2
> +#define GPIO_NUM_PER_GROUP 8
> +#define GPIO_NUM (GPIO_GROUP_NUM*GPIO_NUM_PER_GROUP)
> +
> +struct mc9s08dz60 {
> +	struct i2c_client *client;
> +	struct gpio_chip chip;
> +};
> +
> +static struct mc9s08dz60 *to_mc9s08dz60(struct gpio_chip *gc) {
> +	return container_of(gc, struct mc9s08dz60, chip); }
> +
> +
> +static void gpio_to_reg_and_bit(int offset, u8 *reg, u8 *bit) {
> +	*reg = 0x20 + offset / GPIO_NUM_PER_GROUP;
> +	*bit = offset % GPIO_NUM_PER_GROUP;
> +}
> +
> +static int mc9s08dz60_get_value(struct gpio_chip *gc, unsigned 
> +offset) {
> +	int ret = 0;
> +	u8 reg, bit;
> +	s32 value;
> +	struct mc9s08dz60 *mc9s = to_mc9s08dz60(gc);
> +
> +	gpio_to_reg_and_bit(offset, &reg, &bit);
> +	value = i2c_smbus_read_byte_data(mc9s->client, reg);
> +	if (value >= 0)
> +		ret = (value >> bit) & 0x1;
> +
> +	return ret;
> +}
> +
> +static int _mc9s08dz60_set(struct mc9s08dz60 *mc9s, unsigned offset, 
> +int val) {
> +	u8 reg, bit;
> +	s32 value;
> +
> +	gpio_to_reg_and_bit(offset, &reg, &bit);
> +	value = i2c_smbus_read_byte_data(mc9s->client, reg);
> +	if (value >= 0) {
> +		if (val)
> +			value |= 1 << bit;
> +		else
> +			value &= ~(1 << bit);
> +
> +		return i2c_smbus_write_byte_data(mc9s->client, reg, value);
> +	} else
> +		return value;
> +
> +}
> +
> +
> +static void mc9s08dz60_set_value(struct gpio_chip *gc, unsigned 
> +offset, int val) {
> +	struct mc9s08dz60 *mc9s = to_mc9s08dz60(gc);
> +
> +	_mc9s08dz60_set(mc9s, offset, val);
> +}
> +
> +static int mc9s08dz60_direction_output(struct gpio_chip *gc,
> +				       unsigned offset, int val)
> +{
> +	struct mc9s08dz60 *mc9s = to_mc9s08dz60(gc);
> +
> +	return _mc9s08dz60_set(mc9s, offset, val); }
> +
> +static int mc9s08dz60_probe(struct i2c_client *client,
> +			    const struct i2c_device_id *id) {
> +	int ret = 0;
> +	struct mc9s08dz60 *mc9s;
> +
> +	mc9s = kzalloc(sizeof(*mc9s), GFP_KERNEL);
> +	if (!mc9s)
> +		return -ENOMEM;
> +
> +	mc9s->chip.label = client->name;
> +	mc9s->chip.base = -1;
> +	mc9s->chip.dev = &client->dev;
> +	mc9s->chip.owner = THIS_MODULE;
> +	mc9s->chip.ngpio = GPIO_NUM;
> +	mc9s->chip.can_sleep = 1;
> +	mc9s->chip.get = mc9s08dz60_get_value;
> +	mc9s->chip.set = mc9s08dz60_set_value;
> +	mc9s->chip.direction_output = mc9s08dz60_direction_output;
> +	mc9s->client = client;
> +	i2c_set_clientdata(client, mc9s);
> +
> +	ret = gpiochip_add(&mc9s->chip);
> +	if (ret)
> +		goto error;
> +
> +	return 0;
> +
> +error:
> +	i2c_set_clientdata(client, NULL);
> +	kfree(mc9s);
> +	return ret;
> +}
> +
> +static int mc9s08dz60_remove(struct i2c_client *client) {
> +	struct mc9s08dz60 *mc9s;
> +	int ret;
> +
> +	mc9s = i2c_get_clientdata(client);
> +
> +	i2c_set_clientdata(client, NULL);
> +
> +	ret = gpiochip_remove(&mc9s->chip);
> +	if (!ret)
> +		kfree(mc9s);
> +
> +	return 0;
> +
> +}
> +
> +static const struct i2c_device_id mc9s08dz60_id[] = {
> +	{"mc9s08dz60", 0},
> +	{},
> +};
> +
> +MODULE_DEVICE_TABLE(i2c, mc9s08dz60_id);
> +
> +static struct i2c_driver mc9s08dz60_i2c_driver = {
> +	.driver = {.owner = THIS_MODULE,
> +		   .name = "mc9s08dz60",
> +		   },
> +	.probe = mc9s08dz60_probe,
> +	.remove = mc9s08dz60_remove,
> +	.id_table = mc9s08dz60_id,
> +};
> +
> +static int __init mc9s08dz60_init(void) {
> +	return i2c_add_driver(&mc9s08dz60_i2c_driver);
> +}
> +
> +static void __exit mc9s08dz60_exit(void) {
> +	i2c_del_driver(&mc9s08dz60_i2c_driver);
> +}
> +
> +module_init(mc9s08dz60_init);
> +module_exit(mc9s08dz60_exit);
> +
> +MODULE_AUTHOR("Freescale Semiconductor, Inc."
> +		"Wu Guoxing <b39297@freescale.com>"); 
> +MODULE_DESCRIPTION("mc9s08dz60 gpio function on mx35 3ds board"); 
> +MODULE_LICENSE("GPL v2");


-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |

[PATCH v4] ARM/mx35/3ds: gpio: add mc9s08dz60 gpio function

[Wu Guoxing-B39297]

Hi Wolfram:

  I changed the code to unset i2c client data just when gipochip_remove successes,
Otherwise, there will be problems next time the remove function is called.

  Please review the v5 patch.

  Thanks!

Best Regards
Wu Guoxing

-----Original Message-----
From: Wolfram Sang [mailto:w.sang at pengutronix.de] 
Sent: Thursday, November 10, 2011 6:21 PM
To: Marc Kleine-Budde
Cc: Wu Guoxing-B39297; grant.likely at secretlab.ca; s.hauer at pengutronix.de; shawn.guo at linaro.org; linux-arm-kernel at lists.infradead.org
Subject: Re: [PATCH v4] ARM/mx35/3ds: gpio: add mc9s08dz60 gpio function

On Thu, Nov 10, 2011 at 11:13:42AM +0100, Marc Kleine-Budde wrote:
> On 11/10/2011 03:24 AM, wu guoxing wrote:
> > we only use the gpio function of mc9s08dz60 mcu chip, so just add 
> > the gpio driver, as this mcu will never be used in other board.
> > 
> > Signed-off-by: Wu Guoxing <b39297@freescale.com>
> > ---
> > changes since v3:
> > 	1. set i2c client data before gpiochip_add
> 
> exchange the two in the remove function, too.
> First shut down the gpio chip, then unset set i2c client data.

and don't set clientdata to NULL.

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[PATCH v4] ARM/mx35/3ds: gpio: add mc9s08dz60 gpio function

[Wolfram Sang]

>   I changed the code to unset i2c client data just when gipochip_remove successes,
> Otherwise, there will be problems next time the remove function is called.

The i2c-core will do it on probe-failure and on remove. Check e4a7b9b0.

Thanks,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20111110/615d0fc8/attachment.sig>

[PATCH v4] ARM/mx35/3ds: gpio: add mc9s08dz60 gpio function

[Wu Guoxing-B39297]

Hi Wolfram & Marc:
  I sent out the v6 patch to remove this from both probe and remove functions
Also, in remove function, changed to return the error code.

  Please review the V6 patch.

  Thanks for the comments!

Best Regards
Wu Guoxing

-----Original Message-----
From: Wolfram Sang [mailto:w.sang at pengutronix.de] 
Sent: Thursday, November 10, 2011 7:09 PM
To: Wu Guoxing-B39297
Cc: Marc Kleine-Budde; grant.likely at secretlab.ca; s.hauer at pengutronix.de; shawn.guo at linaro.org; linux-arm-kernel at lists.infradead.org
Subject: Re: [PATCH v4] ARM/mx35/3ds: gpio: add mc9s08dz60 gpio function

>   I changed the code to unset i2c client data just when 
> gipochip_remove successes, Otherwise, there will be problems next time the remove function is called.

The i2c-core will do it on probe-failure and on remove. Check e4a7b9b0.

Thanks,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[PATCH v4] ARM/mx35/3ds: gpio: add mc9s08dz60 gpio function

[Grant Likely]

On Thu, Nov 10, 2011 at 10:24:10AM +0800, y at shlinux1.ap.freescale.net wrote:
> From: wu guoxing <b39297@freescale.com>
> 
> we only use the gpio function of mc9s08dz60 mcu chip, so just
> add the gpio driver, as this mcu will never be used in other board.
> 
> Signed-off-by: Wu Guoxing <b39297@freescale.com>
> ---
> changes since v3:
> 	1. set i2c client data before gpiochip_add
> 	2. return error value in mc9s08dz60_direction_output
> v3 Changes since v1:
> 	1. add can_sleep = 1
> 	2. removed some useless return checks and local variable
> 	3. removed the static variable for i2c client.
> 	4. make init&exit function static
> 
>  drivers/gpio/Kconfig           |    6 ++
>  drivers/gpio/Makefile          |    1 +
>  drivers/gpio/gpio-mc9s08dz60.c |  177 ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 184 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/gpio/gpio-mc9s08dz60.c
> 
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index d539efd..7f6beee 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -487,4 +487,10 @@ config GPIO_TPS65910
>  	help
>  	  Select this option to enable GPIO driver for the TPS65910
>  	  chip family.
> +
> +config GPIO_MC9S08DZ60
> +	bool "MX35 3DS BOARD MC9S08DZ60 GPIO functions"
> +	depends on I2C && MACH_MX35_3DS
> +	help
> +	  Select this to enable the MC9S08DZ60 GPIO driver
>  endif
> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
> index 9588948..85b8799 100644
> --- a/drivers/gpio/Makefile
> +++ b/drivers/gpio/Makefile
> @@ -60,3 +60,4 @@ obj-$(CONFIG_GPIO_WM831X)	+= gpio-wm831x.o
>  obj-$(CONFIG_GPIO_WM8350)	+= gpio-wm8350.o
>  obj-$(CONFIG_GPIO_WM8994)	+= gpio-wm8994.o
>  obj-$(CONFIG_GPIO_XILINX)	+= gpio-xilinx.o
> +obj-$(CONFIG_GPIO_MC9S08DZ60) += gpio-mc9s08dz60.o

This list is sorted alphabetically.  Please keep it that way.

> diff --git a/drivers/gpio/gpio-mc9s08dz60.c b/drivers/gpio/gpio-mc9s08dz60.c
> new file mode 100644
> index 0000000..0122ef8
> --- /dev/null
> +++ b/drivers/gpio/gpio-mc9s08dz60.c
> @@ -0,0 +1,177 @@
> +/*
> + * Copyright 2009-2011 Freescale Semiconductor, Inc. All Rights Reserved.
> + *
> + * Author: Wu Guoxing <b39297@freescale.com>
> + *
> + * 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 of the License, 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.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/i2c.h>
> +#include <linux/gpio.h>
> +
> +#define GPIO_GROUP_NUM 2
> +#define GPIO_NUM_PER_GROUP 8
> +#define GPIO_NUM (GPIO_GROUP_NUM*GPIO_NUM_PER_GROUP)
> +
> +struct mc9s08dz60 {
> +	struct i2c_client *client;
> +	struct gpio_chip chip;
> +};
> +
> +static struct mc9s08dz60 *to_mc9s08dz60(struct gpio_chip *gc)

static inline

> +{
> +	return container_of(gc, struct mc9s08dz60, chip);
> +}
> +
> +
> +static void gpio_to_reg_and_bit(int offset, u8 *reg, u8 *bit)

Protect against namespace collisions and put a mc9s... prefix on this
function.

> +{
> +	*reg = 0x20 + offset / GPIO_NUM_PER_GROUP;
> +	*bit = offset % GPIO_NUM_PER_GROUP;
> +}
> +
> +static int mc9s08dz60_get_value(struct gpio_chip *gc, unsigned offset)
> +{
> +	int ret = 0;
> +	u8 reg, bit;
> +	s32 value;
> +	struct mc9s08dz60 *mc9s = to_mc9s08dz60(gc);
> +
> +	gpio_to_reg_and_bit(offset, &reg, &bit);
> +	value = i2c_smbus_read_byte_data(mc9s->client, reg);
> +	if (value >= 0)
> +		ret = (value >> bit) & 0x1;
> +
> +	return ret;

	return (value >= 0) ? (value >> bit) & 1 : 0;

> +}
> +
> +static int _mc9s08dz60_set(struct mc9s08dz60 *mc9s, unsigned offset, int val)

Single leader underscore '_' is reserved.  Use either two underscores
'__' or none at all.

> +{
> +	u8 reg, bit;
> +	s32 value;
> +
> +	gpio_to_reg_and_bit(offset, &reg, &bit);
> +	value = i2c_smbus_read_byte_data(mc9s->client, reg);
> +	if (value >= 0) {
> +		if (val)
> +			value |= 1 << bit;
> +		else
> +			value &= ~(1 << bit);
> +
> +		return i2c_smbus_write_byte_data(mc9s->client, reg, value);
> +	} else
> +		return value;
> +
> +}
> +
> +
> +static void mc9s08dz60_set_value(struct gpio_chip *gc, unsigned offset, int val)
> +{
> +	struct mc9s08dz60 *mc9s = to_mc9s08dz60(gc);
> +
> +	_mc9s08dz60_set(mc9s, offset, val);
> +}
> +
> +static int mc9s08dz60_direction_output(struct gpio_chip *gc,
> +				       unsigned offset, int val)
> +{
> +	struct mc9s08dz60 *mc9s = to_mc9s08dz60(gc);
> +
> +	return _mc9s08dz60_set(mc9s, offset, val);
> +}
> +
> +static int mc9s08dz60_probe(struct i2c_client *client,
> +			    const struct i2c_device_id *id)
> +{
> +	int ret = 0;
> +	struct mc9s08dz60 *mc9s;
> +
> +	mc9s = kzalloc(sizeof(*mc9s), GFP_KERNEL);
> +	if (!mc9s)
> +		return -ENOMEM;
> +
> +	mc9s->chip.label = client->name;
> +	mc9s->chip.base = -1;
> +	mc9s->chip.dev = &client->dev;
> +	mc9s->chip.owner = THIS_MODULE;
> +	mc9s->chip.ngpio = GPIO_NUM;
> +	mc9s->chip.can_sleep = 1;
> +	mc9s->chip.get = mc9s08dz60_get_value;
> +	mc9s->chip.set = mc9s08dz60_set_value;
> +	mc9s->chip.direction_output = mc9s08dz60_direction_output;
> +	mc9s->client = client;
> +	i2c_set_clientdata(client, mc9s);
> +
> +	ret = gpiochip_add(&mc9s->chip);
> +	if (ret)
> +		goto error;
> +
> +	return 0;
> +
> +error:

Tip: insert one space before a label so that diff will show the
function name on context output instead of the label name.

> +	i2c_set_clientdata(client, NULL);
> +	kfree(mc9s);
> +	return ret;
> +}
> +
> +static int mc9s08dz60_remove(struct i2c_client *client)
> +{
> +	struct mc9s08dz60 *mc9s;
> +	int ret;
> +
> +	mc9s = i2c_get_clientdata(client);
> +
> +	i2c_set_clientdata(client, NULL);
> +
> +	ret = gpiochip_remove(&mc9s->chip);
> +	if (!ret)
> +		kfree(mc9s);
> +
> +	return 0;
> +
> +}
> +
> +static const struct i2c_device_id mc9s08dz60_id[] = {
> +	{"mc9s08dz60", 0},
> +	{},
> +};
> +
> +MODULE_DEVICE_TABLE(i2c, mc9s08dz60_id);
> +
> +static struct i2c_driver mc9s08dz60_i2c_driver = {
> +	.driver = {.owner = THIS_MODULE,
> +		   .name = "mc9s08dz60",
> +		   },

Please use this indentation for static initializers:
	.driver = {
		.owner = THIS_MODULE,
		.name = "mc9s08dz60",
	},

> +	.probe = mc9s08dz60_probe,
> +	.remove = mc9s08dz60_remove,
> +	.id_table = mc9s08dz60_id,
> +};
> +
> +static int __init mc9s08dz60_init(void)
> +{
> +	return i2c_add_driver(&mc9s08dz60_i2c_driver);
> +}
> +
> +static void __exit mc9s08dz60_exit(void)
> +{
> +	i2c_del_driver(&mc9s08dz60_i2c_driver);
> +}
> +
> +module_init(mc9s08dz60_init);
> +module_exit(mc9s08dz60_exit);

module_i2c_driver() is preferred now.

> +
> +MODULE_AUTHOR("Freescale Semiconductor, Inc."
> +		"Wu Guoxing <b39297@freescale.com>");

There is no space between "...Inc." and "Wu...", which is probably not
what you want.  :-)

Otherwise looks good.  One respin and it should be ready for merging.

g.

[PATCH v4] ARM/mx35/3ds: gpio: add mc9s08dz60 gpio function

[Wu Guoxing]

Hi Grant:
    I have send out a new patch according to your commnets, please review.
  
    Thanks for the comments!

Wu Guoxing

On Mon, Jan 23, 2012 at 06:13:06AM -0700, Grant Likely wrote:
> On Thu, Nov 10, 2011 at 10:24:10AM +0800, y at shlinux1.ap.freescale.net wrote:
> > From: wu guoxing <b39297@freescale.com>
> > 
> > we only use the gpio function of mc9s08dz60 mcu chip, so just
> > add the gpio driver, as this mcu will never be used in other board.
> > 
> > Signed-off-by: Wu Guoxing <b39297@freescale.com>
> > ---
> > changes since v3:
> > 	1. set i2c client data before gpiochip_add
> > 	2. return error value in mc9s08dz60_direction_output
> > v3 Changes since v1:
> > 	1. add can_sleep = 1
> > 	2. removed some useless return checks and local variable
> > 	3. removed the static variable for i2c client.
> > 	4. make init&exit function static
> > 
> >  drivers/gpio/Kconfig           |    6 ++
> >  drivers/gpio/Makefile          |    1 +
> >  drivers/gpio/gpio-mc9s08dz60.c |  177 ++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 184 insertions(+), 0 deletions(-)
> >  create mode 100644 drivers/gpio/gpio-mc9s08dz60.c
> > 
> > diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> > index d539efd..7f6beee 100644
> > --- a/drivers/gpio/Kconfig
> > +++ b/drivers/gpio/Kconfig
> > @@ -487,4 +487,10 @@ config GPIO_TPS65910
> >  	help
> >  	  Select this option to enable GPIO driver for the TPS65910
> >  	  chip family.
> > +
> > +config GPIO_MC9S08DZ60
> > +	bool "MX35 3DS BOARD MC9S08DZ60 GPIO functions"
> > +	depends on I2C && MACH_MX35_3DS
> > +	help
> > +	  Select this to enable the MC9S08DZ60 GPIO driver
> >  endif
> > diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
> > index 9588948..85b8799 100644
> > --- a/drivers/gpio/Makefile
> > +++ b/drivers/gpio/Makefile
> > @@ -60,3 +60,4 @@ obj-$(CONFIG_GPIO_WM831X)	+= gpio-wm831x.o
> >  obj-$(CONFIG_GPIO_WM8350)	+= gpio-wm8350.o
> >  obj-$(CONFIG_GPIO_WM8994)	+= gpio-wm8994.o
> >  obj-$(CONFIG_GPIO_XILINX)	+= gpio-xilinx.o
> > +obj-$(CONFIG_GPIO_MC9S08DZ60) += gpio-mc9s08dz60.o
> 
> This list is sorted alphabetically.  Please keep it that way.
> 
> > diff --git a/drivers/gpio/gpio-mc9s08dz60.c b/drivers/gpio/gpio-mc9s08dz60.c
> > new file mode 100644
> > index 0000000..0122ef8
> > --- /dev/null
> > +++ b/drivers/gpio/gpio-mc9s08dz60.c
> > @@ -0,0 +1,177 @@
> > +/*
> > + * Copyright 2009-2011 Freescale Semiconductor, Inc. All Rights Reserved.
> > + *
> > + * Author: Wu Guoxing <b39297@freescale.com>
> > + *
> > + * 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 of the License, 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.
> > + */
> > +
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/slab.h>
> > +#include <linux/i2c.h>
> > +#include <linux/gpio.h>
> > +
> > +#define GPIO_GROUP_NUM 2
> > +#define GPIO_NUM_PER_GROUP 8
> > +#define GPIO_NUM (GPIO_GROUP_NUM*GPIO_NUM_PER_GROUP)
> > +
> > +struct mc9s08dz60 {
> > +	struct i2c_client *client;
> > +	struct gpio_chip chip;
> > +};
> > +
> > +static struct mc9s08dz60 *to_mc9s08dz60(struct gpio_chip *gc)
> 
> static inline
> 
> > +{
> > +	return container_of(gc, struct mc9s08dz60, chip);
> > +}
> > +
> > +
> > +static void gpio_to_reg_and_bit(int offset, u8 *reg, u8 *bit)
> 
> Protect against namespace collisions and put a mc9s... prefix on this
> function.
> 
> > +{
> > +	*reg = 0x20 + offset / GPIO_NUM_PER_GROUP;
> > +	*bit = offset % GPIO_NUM_PER_GROUP;
> > +}
> > +
> > +static int mc9s08dz60_get_value(struct gpio_chip *gc, unsigned offset)
> > +{
> > +	int ret = 0;
> > +	u8 reg, bit;
> > +	s32 value;
> > +	struct mc9s08dz60 *mc9s = to_mc9s08dz60(gc);
> > +
> > +	gpio_to_reg_and_bit(offset, &reg, &bit);
> > +	value = i2c_smbus_read_byte_data(mc9s->client, reg);
> > +	if (value >= 0)
> > +		ret = (value >> bit) & 0x1;
> > +
> > +	return ret;
> 
> 	return (value >= 0) ? (value >> bit) & 1 : 0;
> 
> > +}
> > +
> > +static int _mc9s08dz60_set(struct mc9s08dz60 *mc9s, unsigned offset, int val)
> 
> Single leader underscore '_' is reserved.  Use either two underscores
> '__' or none at all.
> 
> > +{
> > +	u8 reg, bit;
> > +	s32 value;
> > +
> > +	gpio_to_reg_and_bit(offset, &reg, &bit);
> > +	value = i2c_smbus_read_byte_data(mc9s->client, reg);
> > +	if (value >= 0) {
> > +		if (val)
> > +			value |= 1 << bit;
> > +		else
> > +			value &= ~(1 << bit);
> > +
> > +		return i2c_smbus_write_byte_data(mc9s->client, reg, value);
> > +	} else
> > +		return value;
> > +
> > +}
> > +
> > +
> > +static void mc9s08dz60_set_value(struct gpio_chip *gc, unsigned offset, int val)
> > +{
> > +	struct mc9s08dz60 *mc9s = to_mc9s08dz60(gc);
> > +
> > +	_mc9s08dz60_set(mc9s, offset, val);
> > +}
> > +
> > +static int mc9s08dz60_direction_output(struct gpio_chip *gc,
> > +				       unsigned offset, int val)
> > +{
> > +	struct mc9s08dz60 *mc9s = to_mc9s08dz60(gc);
> > +
> > +	return _mc9s08dz60_set(mc9s, offset, val);
> > +}
> > +
> > +static int mc9s08dz60_probe(struct i2c_client *client,
> > +			    const struct i2c_device_id *id)
> > +{
> > +	int ret = 0;
> > +	struct mc9s08dz60 *mc9s;
> > +
> > +	mc9s = kzalloc(sizeof(*mc9s), GFP_KERNEL);
> > +	if (!mc9s)
> > +		return -ENOMEM;
> > +
> > +	mc9s->chip.label = client->name;
> > +	mc9s->chip.base = -1;
> > +	mc9s->chip.dev = &client->dev;
> > +	mc9s->chip.owner = THIS_MODULE;
> > +	mc9s->chip.ngpio = GPIO_NUM;
> > +	mc9s->chip.can_sleep = 1;
> > +	mc9s->chip.get = mc9s08dz60_get_value;
> > +	mc9s->chip.set = mc9s08dz60_set_value;
> > +	mc9s->chip.direction_output = mc9s08dz60_direction_output;
> > +	mc9s->client = client;
> > +	i2c_set_clientdata(client, mc9s);
> > +
> > +	ret = gpiochip_add(&mc9s->chip);
> > +	if (ret)
> > +		goto error;
> > +
> > +	return 0;
> > +
> > +error:
> 
> Tip: insert one space before a label so that diff will show the
> function name on context output instead of the label name.
> 
> > +	i2c_set_clientdata(client, NULL);
> > +	kfree(mc9s);
> > +	return ret;
> > +}
> > +
> > +static int mc9s08dz60_remove(struct i2c_client *client)
> > +{
> > +	struct mc9s08dz60 *mc9s;
> > +	int ret;
> > +
> > +	mc9s = i2c_get_clientdata(client);
> > +
> > +	i2c_set_clientdata(client, NULL);
> > +
> > +	ret = gpiochip_remove(&mc9s->chip);
> > +	if (!ret)
> > +		kfree(mc9s);
> > +
> > +	return 0;
> > +
> > +}
> > +
> > +static const struct i2c_device_id mc9s08dz60_id[] = {
> > +	{"mc9s08dz60", 0},
> > +	{},
> > +};
> > +
> > +MODULE_DEVICE_TABLE(i2c, mc9s08dz60_id);
> > +
> > +static struct i2c_driver mc9s08dz60_i2c_driver = {
> > +	.driver = {.owner = THIS_MODULE,
> > +		   .name = "mc9s08dz60",
> > +		   },
> 
> Please use this indentation for static initializers:
> 	.driver = {
> 		.owner = THIS_MODULE,
> 		.name = "mc9s08dz60",
> 	},
> 
> > +	.probe = mc9s08dz60_probe,
> > +	.remove = mc9s08dz60_remove,
> > +	.id_table = mc9s08dz60_id,
> > +};
> > +
> > +static int __init mc9s08dz60_init(void)
> > +{
> > +	return i2c_add_driver(&mc9s08dz60_i2c_driver);
> > +}
> > +
> > +static void __exit mc9s08dz60_exit(void)
> > +{
> > +	i2c_del_driver(&mc9s08dz60_i2c_driver);
> > +}
> > +
> > +module_init(mc9s08dz60_init);
> > +module_exit(mc9s08dz60_exit);
> 
> module_i2c_driver() is preferred now.
> 
> > +
> > +MODULE_AUTHOR("Freescale Semiconductor, Inc."
> > +		"Wu Guoxing <b39297@freescale.com>");
> 
> There is no space between "...Inc." and "Wu...", which is probably not
> what you want.  :-)
> 
> Otherwise looks good.  One respin and it should be ready for merging.
> 
> g.
>