ARM/mx35/3ds: gpio: add mc9s08dz60 gpio function

PATCH Superseded wu guoxing 4 participants 7 messages Nov 4, 2011 – Nov 10, 2011 GPIO

Share & tools

wu guoxing

author

2011-11-04 10:12 UTC #0 lore raw

PATCH v3 1/1 ARM/mx35/3ds: gpio: add mc9s08dz60 gpio function

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 <[email protected]>

---
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 |  167 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 174 insertions(+), 0 deletions(-)
 create mode 100644 drivers/gpio/gpio-mc9s08dz60.c

drivers/gpio/Kconfig +6 -0

index d539efd..7f6beee 100644
@@ -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

drivers/gpio/Makefile +1 -0

index 9588948..85b8799 100644
@@ -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

drivers/gpio/gpio-mc9s08dz60.c new file +167 -0

167 lines not on this page · raw message

Signature · 1 line

1.7.1

Sascha Hauer

2011-11-08 15:04 UTC #1 lore raw

PATCH v3 1/1 ARM/mx35/3ds: gpio: add mc9s08dz60 gpio function

Quoted 4 lines · 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 <[email protected]>

Looks good to me.

Reviewed-by Sascha Hauer <[email protected]>

Quoted 215 lines

---
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 |  167 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 174 insertions(+), 0 deletions(-)
 create mode 100644 drivers/gpio/gpio-mc9s08dz60.c

drivers/gpio/Kconfig +6 -0

index d539efd..7f6beee 100644
@@ -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

drivers/gpio/Makefile +1 -0

index 9588948..85b8799 100644
@@ -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

drivers/gpio/gpio-mc9s08dz60.c new file +167 -0

new file mode 100644
index 0000000..c41eb16
@@ -0,0 +1,167 @@
+/*
+ * Copyright 2009-2011 Freescale Semiconductor, Inc. All Rights Reserved.
+ *
+ * Author: Wu Guoxing <[email protected]>
+ *
+ * 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 void mc9s08dz60_set_value(struct gpio_chip *gc, unsigned offset, int val)
+{
+	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) {
+		if (val)
+			value |= 1 << bit;
+		else
+			value &= ~(1 << bit);
+
+		i2c_smbus_write_byte_data(mc9s->client, reg, value);
+	}
+}
+
+static int mc9s08dz60_direction_output(struct gpio_chip *gc,
+				       unsigned offset, int val)
+{
+	mc9s08dz60_set_value(gc, offset, val);
+	return 0;
+}
+
+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;
+
+	ret = gpiochip_add(&mc9s->chip);
+	if (ret)
+		goto error;
+
+	i2c_set_clientdata(client, mc9s);
+
+	return 0;
+
+error:
+	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 <[email protected]>");
+MODULE_DESCRIPTION("mc9s08dz60 gpio function on mx35 3ds board");
+MODULE_LICENSE("GPL v2");

Signature · 1 line

1.7.1

Signature · 4 lines

Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Marc Kleine-Budde

2011-11-09 15:46 UTC #2 lore raw

PATCH v3 1/1 ARM/mx35/3ds: gpio: add mc9s08dz60 gpio function

In reply to wu guoxing

Quoted 2 lines · 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.

see comments inline

Marc

Quoted 109 lines

Signed-off-by Wu Guoxing <[email protected]>

---
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 |  167 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 174 insertions(+), 0 deletions(-)
 create mode 100644 drivers/gpio/gpio-mc9s08dz60.c

drivers/gpio/Kconfig +6 -0

index d539efd..7f6beee 100644
@@ -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

drivers/gpio/Makefile +1 -0

index 9588948..85b8799 100644
@@ -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

drivers/gpio/gpio-mc9s08dz60.c new file +64 -0

new file mode 100644
index 0000000..c41eb16
@@ -0,0 +1,167 @@
+/*
+ * Copyright 2009-2011 Freescale Semiconductor, Inc. All Rightos Reserved.
+ *
+ * Author: Wu Guoxing <[email protected]>
+ *
+ * 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 void mc9s08dz60_set_value(struct gpio_chip *gc, unsigned offset, int val)
+{
+	u8 reg, bit;
+	s32 value;
+	struct mc9s08dz60 *mc9s = to_mc9s08dz60(gc);
+

drivers/gpio/gpio-mc9s08dz60.c line 74

Quoted 10 lines

drivers/gpio/gpio-mc9s08dz60.c new file +10 -0

+	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);
+
+		i2c_smbus_write_byte_data(mc9s->client, reg, value);
+	}

what about propagating potential error values here?

drivers/gpio/gpio-mc9s08dz60.c line 81

Quoted 7 lines

drivers/gpio/gpio-mc9s08dz60.c new file +7 -0

+}
+
+static int mc9s08dz60_direction_output(struct gpio_chip *gc,
+				       unsigned offset, int val)
+{
+	mc9s08dz60_set_value(gc, offset, val);
+	return 0;

dito

Quoted 18 lines

drivers/gpio/gpio-mc9s08dz60.c new file +18 -0

+}
+
+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;

drivers/gpio/gpio-mc9s08dz60.c line 109

Quoted 10 lines

drivers/gpio/gpio-mc9s08dz60.c new file +10 -0

+	mc9s->chip.get = mc9s08dz60_get_value;
+	mc9s->chip.set = mc9s08dz60_set_value;
+	mc9s->chip.direction_output = mc9s08dz60_direction_output;
+	mc9s->client = client;
+
+	ret = gpiochip_add(&mc9s->chip);
+	if (ret)
+		goto error;
+
+	i2c_set_clientdata(client, mc9s);

Is it racy if you first add the gpio chip and then set the i2c client data?

Quoted 12 lines

drivers/gpio/gpio-mc9s08dz60.c new file +12 -0

+
+	return 0;
+
+error:
+	kfree(mc9s);
+	return ret;
+}
+
+static int mc9s08dz60_remove(struct i2c_client *client)
+{
+	struct mc9s08dz60 *mc9s;
+	int ret;

drivers/gpio/gpio-mc9s08dz60.c line 131

Quoted 10 lines

drivers/gpio/gpio-mc9s08dz60.c new file +10 -0

+
+	mc9s = i2c_get_clientdata(client);
+
+	i2c_set_clientdata(client, NULL);
+
+	ret = gpiochip_remove(&mc9s->chip);
+	if (!ret)
+		kfree(mc9s);
+
+	return 0;

return 0 if gpiochip_remove() fails?

Quoted 36 lines

drivers/gpio/gpio-mc9s08dz60.c new file +36 -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 <[email protected]>");
+MODULE_DESCRIPTION("mc9s08dz60 gpio function on mx35 3ds board");
+MODULE_LICENSE("GPL v2");

Signature · 12 lines

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/20111109/c6439cd2/attachment.sig>

Fabio Estevam

2011-11-09 16:51 UTC #3 lore raw

PATCH v3 1/1 ARM/mx35/3ds: gpio: add mc9s08dz60 gpio function

Quoted 8 lines · Marc Kleine-Budde

Quoted 8 lines

drivers/gpio/gpio-mc9s08dz60.c new file +8 -0

+static int mc9s08dz60_remove(struct i2c_client *client)
+{
+ ? ? struct mc9s08dz60 *mc9s;
+ ? ? int ret;
+
+ ? ? mc9s = i2c_get_clientdata(client);
+
+ ? ? i2c_set_clientdata(client, NULL);

Quoted 8 lines

Review
Quoted 6 lines
(quoted diff) +6 -0
+
+ ? ? ret = gpiochip_remove(&mc9s->chip);
+ ? ? if (!ret)
+ ? ? ? ? ? ? kfree(mc9s);
+
+ ? ? return 0;
return 0 if gpiochip_remove() fails?

See Russell's comment about returning error on this remove function:
http://www.spinics.net/lists/arm-kernel/msg146501.html

Regards,

Fabio Estevam

Marc Kleine-Budde

2011-11-09 19:31 UTC #4 lore raw

PATCH v3 1/1 ARM/mx35/3ds: gpio: add mc9s08dz60 gpio function

Quoted 18 lines · Fabio Estevam

Quoted 8 lines · Marc Kleine-Budde
Quoted 8 lines
drivers/gpio/gpio-mc9s08dz60.c new file +8 -0
+static int mc9s08dz60_remove(struct i2c_client *client)
+{
+     struct mc9s08dz60 *mc9s;
+     int ret;
+
+     mc9s = i2c_get_clientdata(client);
+
+     i2c_set_clientdata(client, NULL);
Quoted 8 lines
drivers/gpio/gpio-mc9s08dz60.c line 131
Quoted 6 lines
drivers/gpio/gpio-mc9s08dz60.c new file +6 -0
+
+     ret = gpiochip_remove(&mc9s->chip);
+     if (!ret)
+             kfree(mc9s);
+
+     return 0;
return 0 if gpiochip_remove() fails?

Quoted 2 lines

See Russell's comment about returning error on this remove function:
http://www.spinics.net/lists/arm-kernel/msg146501.html

tnx for the hint - I must have missed that comment.

Marc

Signature · 12 lines

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/20111109/76e79ff8/attachment.sig>

Sascha Hauer

2011-11-09 18:19 UTC #5 lore raw

PATCH v3 1/1 ARM/mx35/3ds: gpio: add mc9s08dz60 gpio function

In reply to Marc Kleine-Budde

Quoted 120 lines · Marc Kleine-Budde

Quoted 2 lines · 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.

see comments inline

Marc

Quoted 113 lines

Signed-off-by Wu Guoxing <[email protected]>

---
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 |  167 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 174 insertions(+), 0 deletions(-)
 create mode 100644 drivers/gpio/gpio-mc9s08dz60.c

drivers/gpio/Kconfig +6 -0

index d539efd..7f6beee 100644
@@ -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

drivers/gpio/Makefile +1 -0

index 9588948..85b8799 100644
@@ -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

drivers/gpio/gpio-mc9s08dz60.c new file +68 -0

new file mode 100644
index 0000000..c41eb16
@@ -0,0 +1,167 @@
+/*
+ * Copyright 2009-2011 Freescale Semiconductor, Inc. All Rightos Reserved.
+ *
+ * Author: Wu Guoxing <[email protected]>
+ *
+ * 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 void mc9s08dz60_set_value(struct gpio_chip *gc, unsigned offset, int val)
+{
+	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) {
+		if (val)

Quoted 8 lines

drivers/gpio/gpio-mc9s08dz60.c line 74
Quoted 6 lines
drivers/gpio/gpio-mc9s08dz60.c new file +6 -0
+			value |= 1 << bit;
+		else
+			value &= ~(1 << bit);
+
+		i2c_smbus_write_byte_data(mc9s->client, reg, value);
+	}
what about propagating potential error values here?

This is a void function.

Sascha

Signature · 4 lines

Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Wu Guoxing-B39297

author

2011-11-10 03:15 UTC #6 lore raw

PATCH v3 1/1 ARM/mx35/3ds: gpio: add mc9s08dz60 gpio function

In reply to Marc Kleine-Budde

Hi Marc:

I have changed the position of i2c_set_clientdata and add a return value for mc9s08dz60_direction_output in the v4 patch.
Also, please review the latest v4 patch that sent, the other two are sent by mistake.

Thanks for the comments!

Best Regards
Wu Guoxing

-----Original Message-----
From: Marc Kleine-Budde [mailto:mkl at pengutronix.de]
Sent: Wednesday, November 09, 2011 11:46 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 v3] ARM/mx35/3ds: gpio: add mc9s08dz60 gpio function

Quoted 2 lines · 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.

see comments inline

Marc

Quoted 109 lines

Signed-off-by Wu Guoxing <[email protected]>

---
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 |  167 
++++++++++++++++++++++++++++++++++++++++
 3 files changed, 174 insertions(+), 0 deletions(-)  create mode

100644 drivers/gpio/gpio-mc9s08dz60.c

drivers/gpio/Kconfig index renamed from drivers/gpio/Kconfig +0 -0

d539efd..7f6beee 100644

drivers/gpio/Kconfig +6 -0

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

drivers/gpio/Makefile index renamed from drivers/gpio/Makefile +0 -0

9588948..85b8799 100644

drivers/gpio/Makefile +1 -0

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

(quoted diff) +0 -0

b/drivers/gpio/gpio-mc9s08dz60.c new file mode 100644 index 0000000..c41eb16

drivers/gpio/gpio-mc9s08dz60.c new file +63 -0

@@ -0,0 +1,167 @@
+/*
+ * Copyright 2009-2011 Freescale Semiconductor, Inc. All Rightos Reserved.
+ *
+ * Author: Wu Guoxing <[email protected]>
+ *
+ * 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 void mc9s08dz60_set_value(struct gpio_chip *gc, unsigned 
+offset, int val) {
+	u8 reg, bit;
+	s32 value;
+	struct mc9s08dz60 *mc9s = to_mc9s08dz60(gc);
+

drivers/gpio/gpio-mc9s08dz60.c line 73

Quoted 10 lines

drivers/gpio/gpio-mc9s08dz60.c new file +10 -0

+	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);
+
+		i2c_smbus_write_byte_data(mc9s->client, reg, value);
+	}

what about propagating potential error values here?

drivers/gpio/gpio-mc9s08dz60.c line 81

Quoted 7 lines

drivers/gpio/gpio-mc9s08dz60.c new file +7 -0

+}
+
+static int mc9s08dz60_direction_output(struct gpio_chip *gc,
+				       unsigned offset, int val)
+{
+	mc9s08dz60_set_value(gc, offset, val);
+	return 0;

dito

Quoted 17 lines

drivers/gpio/gpio-mc9s08dz60.c new file +17 -0

+}
+
+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;

drivers/gpio/gpio-mc9s08dz60.c line 109

Quoted 10 lines

drivers/gpio/gpio-mc9s08dz60.c new file +10 -0

+	mc9s->chip.get = mc9s08dz60_get_value;
+	mc9s->chip.set = mc9s08dz60_set_value;
+	mc9s->chip.direction_output = mc9s08dz60_direction_output;
+	mc9s->client = client;
+
+	ret = gpiochip_add(&mc9s->chip);
+	if (ret)
+		goto error;
+
+	i2c_set_clientdata(client, mc9s);

Is it racy if you first add the gpio chip and then set the i2c client data?

Quoted 11 lines

drivers/gpio/gpio-mc9s08dz60.c new file +11 -0

+
+	return 0;
+
+error:
+	kfree(mc9s);
+	return ret;
+}
+
+static int mc9s08dz60_remove(struct i2c_client *client) {
+	struct mc9s08dz60 *mc9s;
+	int ret;

drivers/gpio/gpio-mc9s08dz60.c line 131

Quoted 10 lines

drivers/gpio/gpio-mc9s08dz60.c new file +10 -0

+
+	mc9s = i2c_get_clientdata(client);
+
+	i2c_set_clientdata(client, NULL);
+
+	ret = gpiochip_remove(&mc9s->chip);
+	if (!ret)
+		kfree(mc9s);
+
+	return 0;

return 0 if gpiochip_remove() fails?

Quoted 34 lines

drivers/gpio/gpio-mc9s08dz60.c new file +34 -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 <[email protected]>"); 
+MODULE_DESCRIPTION("mc9s08dz60 gpio function on mx35 3ds board"); 
+MODULE_LICENSE("GPL v2");

Signature · 4 lines

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   |