Signed-off-by: Jean Delvare <kh...@linux-fr.org>
Cc: Grant Likely <grant....@secretlab.ca>
---
Note 1: On early ICH chips, some pins are exclusively inputs or
outputs. The driver doesn't currently enforce this.
Note 2: I'm not yet sure if we want a module alias for this driver.
Many systems have the device but only a few of them will need the
driver (and an ACPI resource conflict will be reported for many
others, especially laptops I suspect.) So it might make more sense to
let consumer drivers request the i801_gpio driver as needed (which they
should do anyway, as you can't assume udev is always up and running on
all systems.)
Note 3: This is my first GPIO driver, so while it works fine for me, it
might not be perfect. I welcome comments on how to improve it.
drivers/gpio/Kconfig | 7
drivers/gpio/Makefile | 1
drivers/gpio/i801_gpio.c | 432 ++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 440 insertions(+)
--- linux-2.6.39-rc3.orig/drivers/gpio/Kconfig 2011-04-18 19:41:51.000000000 +0200
+++ linux-2.6.39-rc3/drivers/gpio/Kconfig 2011-04-18 22:36:41.000000000 +0200
@@ -322,6 +322,13 @@ config GPIO_BT8XX
If unsure, say N.
+config GPIO_I801
+ tristate "Intel 82801 (ICH) GPIO"
+ depends on PCI
+ help
+ This driver is for the GPIO pins of Intel 82801 south bridges
+ (aka ICH).
+
config GPIO_LANGWELL
bool "Intel Langwell/Penwell GPIO support"
depends on PCI && X86
--- linux-2.6.39-rc3.orig/drivers/gpio/Makefile 2011-04-18 19:41:51.000000000 +0200
+++ linux-2.6.39-rc3/drivers/gpio/Makefile 2011-04-18 22:36:41.000000000 +0200
@@ -11,6 +11,7 @@ obj-$(CONFIG_GPIOLIB) += gpiolib.o
obj-$(CONFIG_GPIO_ADP5520) += adp5520-gpio.o
obj-$(CONFIG_GPIO_ADP5588) += adp5588-gpio.o
obj-$(CONFIG_GPIO_BASIC_MMIO) += basic_mmio_gpio.o
+obj-$(CONFIG_GPIO_I801) += i801_gpio.o
obj-$(CONFIG_GPIO_LANGWELL) += langwell_gpio.o
obj-$(CONFIG_GPIO_MAX730X) += max730x.o
obj-$(CONFIG_GPIO_MAX7300) += max7300.o
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6.39-rc3/drivers/gpio/i801_gpio.c 2011-04-19 13:55:25.000000000 +0200
@@ -0,0 +1,432 @@
+/*
+ * Linux kernel driver for the Intel 82801 GPIO pins
+ * Copyright (C) 2011 Jean Delvare <kh...@linux-fr.org>
+ *
+ * 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; version 2 of the License.
+ *
+ * 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, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307, USA.
+ */
+
+#include <linux/kernel.h>
+#include <linux/acpi.h>
+#include <linux/bitops.h>
+#include <linux/gpio.h>
+#include <linux/io.h>
+#include <linux/ioport.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/pci.h>
+
+static int force;
+module_param(force, int, 0444);
+MODULE_PARM_DESC(force, "Forcibly enable access to GPIO");
+
+enum chips { ICH2, ICH4, ICH6, ICH10_CORP };
+
+/* Register definitions */
+static const u8 REG_GPIO_USE_SEL[3] = { 0x00, 0x30, 0x40 };
+static const u8 REG_GP_IO_SEL[3] = { 0x04, 0x34, 0x44 };
+static const u8 REG_GP_LVL[3] = { 0x0C, 0x38, 0x48 };
+
+/**
+ * struct i801_gpio_data - 82801 GPIO private data
+ * @base: Base I/O port
+ * @io_size: I/O region size (64 or 128)
+ * @gpio: Data for GPIO infrastructure
+ * @lock: Mutex to serialize read-modify-write cycles
+ */
+struct i801_gpio_data {
+ u32 base;
+ u32 io_size;
+ u32 use_sel[3];
+ struct gpio_chip gpio;
+ struct mutex lock;
+};
+
+static int i801_gpio_request(struct gpio_chip *gpio, unsigned nr)
+{
+ struct i801_gpio_data *data;
+ int group = nr >> 5;
+
+ data = container_of(gpio, struct i801_gpio_data, gpio);
+ nr &= 0x1f;
+
+ if (data->use_sel[group] & BIT(nr))
+ return 0;
+ else
+ return -EINVAL;
+}
+
+static void i801_gpio_set(struct gpio_chip *gpio, unsigned nr, int val)
+{
+ struct i801_gpio_data *data;
+ int group = nr >> 5;
+ u32 reg_val;
+
+ data = container_of(gpio, struct i801_gpio_data, gpio);
+ nr &= 0x1f;
+
+ mutex_lock(&data->lock);
+ reg_val = inl(data->base + REG_GP_LVL[group]);
+ if (val)
+ reg_val |= BIT(nr);
+ else
+ reg_val &= ~BIT(nr);
+ outl(reg_val, data->base + REG_GP_LVL[group]);
+ mutex_unlock(&data->lock);
+}
+
+static int i801_gpio_get(struct gpio_chip *gpio, unsigned nr)
+{
+ struct i801_gpio_data *data;
+ int group = nr >> 5;
+
+ data = container_of(gpio, struct i801_gpio_data, gpio);
+ nr &= 0x1f;
+
+ return (inl(data->base + REG_GP_LVL[group]) >> nr) & 1;
+}
+
+static int i801_gpio_direction_output(struct gpio_chip *gpio, unsigned nr,
+ int val)
+{
+ struct i801_gpio_data *data;
+ int group = nr >> 5;
+ u32 reg_val;
+
+ data = container_of(gpio, struct i801_gpio_data, gpio);
+ nr &= 0x1f;
+
+ mutex_lock(&data->lock);
+ reg_val = inl(data->base + REG_GP_IO_SEL[group]);
+ reg_val &= ~BIT(nr);
+ outl(reg_val, data->base + REG_GP_IO_SEL[group]);
+
+ reg_val = inl(data->base + REG_GP_LVL[group]);
+ if (val)
+ reg_val |= BIT(nr);
+ else
+ reg_val &= ~BIT(nr);
+ outl(reg_val, data->base + REG_GP_LVL[group]);
+ mutex_unlock(&data->lock);
+
+ return 0;
+}
+
+static int i801_gpio_direction_input(struct gpio_chip *gpio, unsigned nr)
+{
+ struct i801_gpio_data *data;
+ int group = nr >> 5;
+ u32 reg_val;
+
+ data = container_of(gpio, struct i801_gpio_data, gpio);
+ nr &= 0x1f;
+
+ mutex_lock(&data->lock);
+ reg_val = inl(data->base + REG_GP_IO_SEL[group]);
+ reg_val |= BIT(nr);
+ outl(reg_val, data->base + REG_GP_IO_SEL[group]);
+ mutex_unlock(&data->lock);
+
+ return 0;
+}
+
+static void __devinit i801_gpio_setup(struct gpio_chip *gpio,
+ struct device *dev, int ngpio)
+{
+ gpio->label = "i801_gpio";
+ gpio->dev = dev;
+ gpio->owner = THIS_MODULE;
+ gpio->request = i801_gpio_request;
+ gpio->direction_input = i801_gpio_direction_input;
+ gpio->get = i801_gpio_get;
+ gpio->direction_output = i801_gpio_direction_output;
+ gpio->set = i801_gpio_set;
+ gpio->base = -1;
+ gpio->ngpio = ngpio;
+}
+
+/*
+ * On the ICH/ICH0, ICH3, ICH4 and ICH5, some selection bits control more
+ * than one GPIO.
+ */
+static void __devinit i801_gpio_sel_fixup(struct i801_gpio_data *data,
+ u32 device)
+{
+ switch (device) {
+ case PCI_DEVICE_ID_INTEL_82801AA_0:
+ case PCI_DEVICE_ID_INTEL_82801AB_0:
+ case PCI_DEVICE_ID_INTEL_82801CA_0:
+ case PCI_DEVICE_ID_INTEL_82801CA_12:
+ case PCI_DEVICE_ID_INTEL_82801DB_0:
+ case PCI_DEVICE_ID_INTEL_82801DB_12:
+ case PCI_DEVICE_ID_INTEL_82801EB_0:
+ if (data->use_sel[0] & BIT(0))
+ data->use_sel[0] |= BIT(16);
+ if (data->use_sel[0] & BIT(1))
+ data->use_sel[0] |= BIT(17);
+ if (data->use_sel[0] & BIT(27))
+ data->use_sel[0] |= BIT(28);
+ break;
+ }
+}
+
+static __devinitdata
+const struct {
+ int ngroup;
+ int io_size;
+ u8 reg_gpiobase;
+ u8 reg_gc;
+} i801_gpio_cfg[] = {
+ [ICH2] = { 1, 64, 0x58, 0x5C },
+ [ICH4] = { 2, 64, 0x58, 0x5C },
+ [ICH6] = { 2, 64, 0x48, 0x4C },
+ [ICH10_CORP] = { 3, 128, 0x48, 0x4C },
+};
+
+static void __devinit i801_gpio_print_state(struct i801_gpio_data *data,
+ struct device *dev, int ngroup)
+{
+ int i, group;
+ u32 io_sel, lvl;
+
+ dev_dbg(dev, "Current state of GPIO pins:\n");
+ for (group = 0; group < ngroup; group++) {
+ io_sel = inl(data->base + REG_GP_IO_SEL[group]);
+ lvl = inl(data->base + REG_GP_LVL[group]);
+
+ for (i = 0; i < 32; i++) {
+ if (!(data->use_sel[group] & BIT(i)))
+ continue;
+
+ dev_dbg(dev, "GPIO%d: %s, level %d\n", group * 32 + i,
+ io_sel & BIT(i) ? "input" : "output",
+ !!(lvl & BIT(i)));
+ }
+ }
+}
+
+static int __devinit i801_gpio_probe(struct pci_dev *pdev,
+ const struct pci_device_id *id)
+{
+ struct i801_gpio_data *data;
+ u32 gpiobase;
+ u8 gc;
+ int type = id->driver_data;
+ int group, ngroup, err;
+
+ data = kzalloc(sizeof(struct i801_gpio_data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ err = pci_enable_device(pdev);
+ if (err) {
+ dev_err(&pdev->dev, "Failed to enable device (%d)\n", err);
+ goto err_free;
+ }
+
+ /* Get the base I/O address */
+ err = pci_read_config_dword(pdev, i801_gpio_cfg[type].reg_gpiobase,
+ &gpiobase);
+ if (err) {
+ dev_err(&pdev->dev, "Couldn't read %s value (%d)\n",
+ "GPIOBASE", err);
+ goto err_disable;
+ }
+ data->base = gpiobase & ~BIT(0);
+ if (!data->base) {
+ dev_err(&pdev->dev, "GPIOBASE not set\n");
+ err = -ENODEV;
+ goto err_disable;
+ }
+
+ /* Check configuration */
+ err = pci_read_config_byte(pdev, i801_gpio_cfg[type].reg_gc, &gc);
+ if (err) {
+ dev_err(&pdev->dev, "Couldn't read %s value (%d)\n",
+ "GC", err);
+ goto err_disable;
+ }
+ if (gc & BIT(0)) {
+ dev_err(&pdev->dev, "GPIO function is %s\n", "locked");
+ err = -EBUSY;
+ goto err_disable;
+ }
+ if (!(gc & BIT(4))) {
+ if (!force) {
+ dev_err(&pdev->dev, "GPIO function is %s\n",
+ "disabled");
+ err = -ENODEV;
+ goto err_disable;
+ }
+
+ gc |= BIT(4);
+ err = pci_write_config_byte(pdev,
+ i801_gpio_cfg[type].reg_gc, gc);
+ if (err) {
+ dev_err(&pdev->dev,
+ "Failed to enable GPIO function (%d)\n",
+ err);
+ err = -ENODEV;
+ goto err_disable;
+ }
+ dev_info(&pdev->dev, "Enabling GPIO function\n");
+ }
+
+ /*
+ * "Corporate" incarnations of the ICH10 have an I/O region of size
+ * 128 and 73 GPIO pins. Others ("consumer") have an I/O region of
+ * size only 64 and 61 GPIO pins, as the ICH6, ICH7, ICH8 and ICH9
+ * had.
+ */
+ data->io_size = i801_gpio_cfg[type].io_size;
+ if (!force) {
+ err = acpi_check_region(data->base, data->io_size, "i801_gpio");
+ if (err)
+ goto err_disable;
+ }
+ if (!request_region(data->base, data->io_size, "i801_gpio")) {
+ dev_err(&pdev->dev, "Failed to request I/O ports (%d)", err);
+ err = -EBUSY;
+ goto err_disable;
+ }
+
+ ngroup = i801_gpio_cfg[type].ngroup;
+ for (group = 0; group < ngroup; group++)
+ data->use_sel[group] = inl(data->base +
+ REG_GPIO_USE_SEL[group]);
+ i801_gpio_sel_fixup(data, id->device);
+ mutex_init(&data->lock);
+
+ i801_gpio_setup(&data->gpio, &pdev->dev, ngroup * 32);
+ i801_gpio_print_state(data, &pdev->dev, ngroup);
+
+ pci_set_drvdata(pdev, data);
+ err = gpiochip_add(&data->gpio);
+ if (err) {
+ dev_err(&pdev->dev, "Failed to register GPIO (%d)\n", err);
+ goto err_release;
+ }
+
+ return 0;
+
+ err_release:
+ release_region(data->base, data->io_size);
+
+ err_disable:
+ pci_disable_device(pdev);
+
+ err_free:
+ kfree(data);
+ return err;
+}
+
+static void __devexit i801_gpio_remove(struct pci_dev *pdev)
+{
+ struct i801_gpio_data *data = pci_get_drvdata(pdev);
+ int err;
+
+ err = gpiochip_remove(&data->gpio);
+ if (err)
+ dev_err(&pdev->dev, "Failed to unregister GPIO (%d)\n", err);
+
+ release_region(data->base, data->io_size);
+ pci_disable_device(pdev);
+ kfree(data);
+}
+
+/* driver_data is the number of GPIO groups */
+static DEFINE_PCI_DEVICE_TABLE(i801_gpio_pcidev_id) = {
+ { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82801AA_0),
+ .driver_data = ICH2 },
+ { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82801AB_0),
+ .driver_data = ICH2 },
+ { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82801BA_0),
+ .driver_data = ICH2 },
+ { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82801BA_10),
+ .driver_data = ICH2 },
+ { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82801CA_0),
+ .driver_data = ICH4 },
+ { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82801CA_12),
+ .driver_data = ICH4 },
+ { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82801DB_0),
+ .driver_data = ICH4 },
+ { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82801DB_12),
+ .driver_data = ICH4 },
+ { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82801EB_0),
+ .driver_data = ICH4 },
+ { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ICH6_0),
+ .driver_data = ICH6 },
+ { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ICH6_1),
+ .driver_data = ICH6 },
+ { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ICH7_0),
+ .driver_data = ICH6 },
+ { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ICH7_1),
+ .driver_data = ICH6 },
+ { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ICH7_31),
+ .driver_data = ICH6 },
+ { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ICH8_0),
+ .driver_data = ICH6 },
+ { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ICH8_1),
+ .driver_data = ICH6 },
+ { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ICH8_2),
+ .driver_data = ICH6 },
+ { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ICH8_3),
+ .driver_data = ICH6 },
+ { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ICH8_4),
+ .driver_data = ICH6 },
+ { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ICH9_1),
+ .driver_data = ICH6 },
+ { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ICH9_2),
+ .driver_data = ICH6 },
+ { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ICH9_4),
+ .driver_data = ICH6 },
+ { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ICH9_5),
+ .driver_data = ICH6 },
+ { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ICH9_7),
+ .driver_data = ICH6 },
+ { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ICH9_8),
+ .driver_data = ICH6 },
+ { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ICH10_0),
+ .driver_data = ICH10_CORP },
+ { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ICH10_1),
+ .driver_data = ICH6 },
+ { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ICH10_2),
+ .driver_data = ICH6 },
+ { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ICH10_3),
+ .driver_data = ICH10_CORP },
+ { 0, }
+};
+MODULE_DEVICE_TABLE(pci, i801_gpio_pcidev_id);
+
+static struct pci_driver i801_gpio_driver = {
+ .name = "i801_gpio",
+ .id_table = i801_gpio_pcidev_id,
+ .probe = i801_gpio_probe,
+ .remove = __devexit_p(i801_gpio_remove),
+};
+
+static int __init i801_gpio_pci_init(void)
+{
+ return pci_register_driver(&i801_gpio_driver);
+}
+module_init(i801_gpio_pci_init);
+
+static void __exit i801_gpio_pci_exit(void)
+{
+ pci_unregister_driver(&i801_gpio_driver);
+}
+module_exit(i801_gpio_pci_exit);
+
+MODULE_AUTHOR("Jean Delvare <kh...@linux-fr.org>");
+MODULE_DESCRIPTION("Intel 82801 (ICH) GPIO driver");
+MODULE_LICENSE("GPL");
--
Jean Delvare
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
I'm not at all concerned about this. Users *must* know what a pin is
when they start fiddling with it. If you ask a pin to do something
that it cannot do, then it just isn't going to work. There's no real
point in trying to enforce any behaviour in the API.
>
> Note 2: I'm not yet sure if we want a module alias for this driver.
> Many systems have the device but only a few of them will need the
> driver (and an ACPI resource conflict will be reported for many
> others, especially laptops I suspect.) So it might make more sense to
> let consumer drivers request the i801_gpio driver as needed (which they
> should do anyway, as you can't assume udev is always up and running on
> all systems.)
>
> Note 3: This is my first GPIO driver, so while it works fine for me, it
> might not be perfect. I welcome comments on how to improve it.
Your timing is impeccable. You're getting caught up in the big gpio
driver consolidation. :-)
Most gpio drivers end up looking pretty darn similar. Instead of
writing the same types of drivers over and over again, the new
approach is to try and consolidate the mmio drivers down to using
basic_mmio_gpio.c.
In this particular case, you've got a PCI device which looks to be
going into config space to get some information about how the chip is
layed out. What I would do is keep your existing pci probe & remove
hooks, but use them to create and register child basic_mmio_gpio
platform_devices for each gpio bank.
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
Disagree 8) - errors are better than silently and undetectably failing.
That might not be an error code but ideally it would WARN() or
similar so that errors get caught and fixed and you don't instead find
out something was poking the wrong pin and worked by chance a year later.
> In this particular case, you've got a PCI device which looks to be
> going into config space to get some information about how the chip is
> layed out. What I would do is keep your existing pci probe & remove
> hooks, but use them to create and register child basic_mmio_gpio
> platform_devices for each gpio bank.
That means more devices, weird sysfs heirarchies and lots of mess with
runtime power management in the general case.
If the basic_mmio_gpio code is properly abstracted it ought not to need
magic extra platform devices.
That aside it'll be less code to keep it separate than do all the messing
around with platform devices so it seems to be a gross overdoing of 'lets
make everything use the same code however big a hammer is needed'
If it was a platform device it might make sense, if the config was
platform config to create a device it would certainly make sense, in the
PCI case it's smaller, cleaner and saner not to add insane layers of glue
and indirection IMHO.
Alan
Doing a platform device is one solution that is pretty simple to
implement and I don't think adding child devices is at all a problem.
However, I'm not mandating that approach, and if you or Jean prefer to
abstract out the gpio accessors from basic_mmio_gpio(), then that is
fine by me. The key issue it to avoid merging yet another, probably
subtly broken, set of GPIO accessor functions if it can at all be
avoided.
g.
As is adding a set of subtly broken attempts to create device stacks that
then get into funny sysfs and power management tangles. As well as being
about 12K larger due to the need to suck in an extra module.
I can see the point of splitting out the accessors but if thats a module
of its own then thats another 8K we don't need to waste on a few hundred
bytes of utterly trivial code.
How about exporting some bus/device-type neutral probe function, like
we do in drivers/ata/pata_platform.c ?
That way bus-specific probe functions may call the generic routines.
And if you still want to save 8K, you could place the PCI bindings
inside the basic_mmio_gpio.
The patch below is against Linus tree + the following patches from
Jamie Iles:
basic_mmio_gpio: remove runtime width/endianness evaluation
basic_mmio_gpio: convert to platform_{get,set}_drvdata()
basic_mmio_gpio: allow overriding number of gpio
basic_mmio_gpio: request register regions
basic_mmio_gpio: detect output method at probe time
basic_mmio_gpio: support different input/output registers
basic_mmio_gpio: support direction registers
basic_mmio_gpio: convert to non-__raw* accessors
Not-yet-signed-off-by: Anton Vorontsov <cbouat...@gmail.com>
and only compile tested so far, just an idea:
---
drivers/gpio/Kconfig | 6 ++
drivers/gpio/Makefile | 1 +
drivers/gpio/basic_mmio_gpio.c | 98 ++++++++++++++++++++++----------------
include/linux/basic_mmio_gpio.h | 14 ++++++
4 files changed, 78 insertions(+), 41 deletions(-)
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 664660e..88dd650 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -70,8 +70,14 @@ config GPIO_MAX730X
comment "Memory mapped GPIO expanders:"
+config GPIO_BASIC_MMIO_CORE
+ tristate
+ help
+ Provides core functionality for basic memory-mapped GPIO controllers.
+
config GPIO_BASIC_MMIO
tristate "Basic memory-mapped GPIO controllers support"
+ select GPIO_BASIC_MMIO_CORE
help
Say yes here to support basic memory-mapped GPIO controllers.
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 3351cf8..1abf916 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_GPIOLIB) += gpiolib.o
obj-$(CONFIG_GPIO_ADP5520) += adp5520-gpio.o
obj-$(CONFIG_GPIO_ADP5588) += adp5588-gpio.o
+obj-$(CONFIG_GPIO_BASIC_MMIO_CORE) += basic_mmio_gpio.o
obj-$(CONFIG_GPIO_BASIC_MMIO) += basic_mmio_gpio.o
obj-$(CONFIG_GPIO_LANGWELL) += langwell_gpio.o
obj-$(CONFIG_GPIO_MAX730X) += max730x.o
diff --git a/drivers/gpio/basic_mmio_gpio.c b/drivers/gpio/basic_mmio_gpio.c
index b2ec45f..3deb1d7 100644
--- a/drivers/gpio/basic_mmio_gpio.c
+++ b/drivers/gpio/basic_mmio_gpio.c
@@ -294,10 +294,10 @@ static void __iomem *bgpio_request_and_map(struct device *dev,
return devm_ioremap(dev, res->start, resource_size(res));
}
-static int bgpio_setup_accessors(struct platform_device *pdev,
- struct bgpio_chip *bgc)
+static int bgpio_setup_accessors(struct device *dev,
+ struct bgpio_chip *bgc,
+ bool be)
{
- const struct platform_device_id *platid = platform_get_device_id(pdev);
switch (bgc->bits) {
case 8:
@@ -319,13 +319,11 @@ static int bgpio_setup_accessors(struct platform_device *pdev,
break;
#endif /* BITS_PER_LONG >= 64 */
default:
- dev_err(&pdev->dev, "unsupported data width %u bits\n",
- bgc->bits);
+ dev_err(dev, "unsupported data width %u bits\n", bgc->bits);
return -EINVAL;
}
- bgc->pin2mask = strcmp(platid->name, "basic-mmio-gpio-be") ?
- bgpio_pin2mask : bgpio_pin2mask_be;
+ bgc->pin2mask = be ? bgpio_pin2mask_be : bgpio_pin2mask;
return 0;
}
@@ -352,18 +350,14 @@ static int bgpio_setup_accessors(struct platform_device *pdev,
* - an input direction register (named "dirin") where a 1 bit indicates
* the GPIO is an input.
*/
-static int bgpio_setup_io(struct platform_device *pdev,
- struct bgpio_chip *bgc)
+static int bgpio_setup_io(struct device *dev,
+ struct bgpio_chip *bgc,
+ struct resource *res_dat,
+ struct resource *res_set,
+ struct resource *res_clr)
{
- struct resource *res_set;
- struct resource *res_clr;
- struct resource *res_dat;
resource_size_t dat_sz;
- res_dat = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dat");
- if (!res_dat)
- return -EINVAL;
-
dat_sz = resource_size(res_dat);
if (!is_power_of_2(dat_sz))
return -EINVAL;
@@ -372,19 +366,17 @@ static int bgpio_setup_io(struct platform_device *pdev,
if (bgc->bits > BITS_PER_LONG)
return -EINVAL;
- bgc->reg_dat = bgpio_request_and_map(&pdev->dev, res_dat);
+ bgc->reg_dat = bgpio_request_and_map(dev, res_dat);
if (!bgc->reg_dat)
return -ENOMEM;
- res_set = platform_get_resource_byname(pdev, IORESOURCE_MEM, "set");
- res_clr = platform_get_resource_byname(pdev, IORESOURCE_MEM, "clr");
if (res_set && res_clr) {
if (resource_size(res_set) != resource_size(res_clr) ||
resource_size(res_set) != resource_size(res_dat))
return -EINVAL;
- bgc->reg_set = bgpio_request_and_map(&pdev->dev, res_set);
- bgc->reg_clr = bgpio_request_and_map(&pdev->dev, res_clr);
+ bgc->reg_set = bgpio_request_and_map(dev, res_set);
+ bgc->reg_clr = bgpio_request_and_map(dev, res_clr);
if (!bgc->reg_set || !bgc->reg_clr)
return -ENOMEM;
@@ -393,7 +385,7 @@ static int bgpio_setup_io(struct platform_device *pdev,
if (resource_size(res_set) != resource_size(res_dat))
return -EINVAL;
- bgc->reg_set = bgpio_request_and_map(&pdev->dev, res_set);
+ bgc->reg_set = bgpio_request_and_map(dev, res_set);
if (!bgc->reg_set)
return -ENOMEM;
@@ -407,25 +399,21 @@ static int bgpio_setup_io(struct platform_device *pdev,
return 0;
}
-static int bgpio_setup_direction(struct platform_device *pdev,
- struct bgpio_chip *bgc)
+static int bgpio_setup_direction(struct device *dev,
+ struct bgpio_chip *bgc,
+ struct resource *res_dirout,
+ struct resource *res_dirin)
{
- struct resource *res_dirout;
- struct resource *res_dirin;
-
- res_dirout = platform_get_resource_byname(pdev, IORESOURCE_MEM,
- "dirout");
- res_dirin = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dirin");
if (res_dirout && res_dirin) {
return -EINVAL;
} else if (res_dirout) {
- bgc->reg_dir = bgpio_request_and_map(&pdev->dev, res_dirout);
+ bgc->reg_dir = bgpio_request_and_map(dev, res_dirout);
if (!bgc->reg_dir)
return -ENOMEM;
bgc->gc.direction_output = bgpio_dir_out;
bgc->gc.direction_input = bgpio_dir_in;
} else if (res_dirin) {
- bgc->reg_dir = bgpio_request_and_map(&pdev->dev, res_dirin);
+ bgc->reg_dir = bgpio_request_and_map(dev, res_dirin);
if (!bgc->reg_dir)
return -ENOMEM;
bgc->gc.direction_output = bgpio_dir_out_inv;
@@ -438,9 +426,22 @@ static int bgpio_setup_direction(struct platform_device *pdev,
return 0;
}
-static int __devinit bgpio_probe(struct platform_device *pdev)
+int __devexit __bgpio_remove(struct device *dev)
+{
+ struct bgpio_chip *bgc = dev_get_drvdata(dev);
+
+ return gpiochip_remove(&bgc->gc);
+}
+EXPORT_SYMBOL_GPL(__bgpio_remove);
+
+int __devinit __bgpio_probe(struct device *dev,
+ struct resource *res_dat,
+ struct resource *res_set,
+ struct resource *res_clr,
+ struct resource *res_dirout,
+ struct resource *res_dirin,
+ bool be)
{
- struct device *dev = &pdev->dev;
struct bgpio_pdata *pdata = dev_get_platdata(dev);
struct bgpio_chip *bgc;
int ret;
@@ -450,7 +451,7 @@ static int __devinit bgpio_probe(struct platform_device *pdev)
if (!bgc)
return -ENOMEM;
- ret = bgpio_setup_io(pdev, bgc);
+ ret = bgpio_setup_io(dev, bgc, res_dat, res_set, res_clr);
if (ret)
return ret;
@@ -463,12 +464,12 @@ static int __devinit bgpio_probe(struct platform_device *pdev)
bgc->gc.base = -1;
}
- ret = bgpio_setup_accessors(pdev, bgc);
+ ret = bgpio_setup_accessors(dev, bgc, be);
if (ret)
return ret;
spin_lock_init(&bgc->lock);
- ret = bgpio_setup_direction(pdev, bgc);
+ ret = bgpio_setup_direction(dev, bgc, res_dirout, res_dirin);
if (ret)
return ret;
@@ -478,7 +479,7 @@ static int __devinit bgpio_probe(struct platform_device *pdev)
bgc->gc.dev = dev;
bgc->gc.label = dev_name(dev);
- platform_set_drvdata(pdev, bgc);
+ dev_set_drvdata(dev, bgc);
ret = gpiochip_add(&bgc->gc);
if (ret)
@@ -486,12 +487,25 @@ static int __devinit bgpio_probe(struct platform_device *pdev)
return ret;
}
+EXPORT_SYMBOL_GPL(__bgpio_probe);
-static int __devexit bgpio_remove(struct platform_device *pdev)
+#ifdef CONFIG_GPIO_BASIC_MMIO
+
+static int __devinit bgpio_probe(struct platform_device *pdev)
{
- struct bgpio_chip *bgc = platform_get_drvdata(pdev);
+ return __bgpio_probe(&pdev->dev,
+ platform_get_resource_byname(pdev, IORESOURCE_MEM, "dat"),
+ platform_get_resource_byname(pdev, IORESOURCE_MEM, "set"),
+ platform_get_resource_byname(pdev, IORESOURCE_MEM, "clr"),
+ platform_get_resource_byname(pdev, IORESOURCE_MEM, "dirout"),
+ platform_get_resource_byname(pdev, IORESOURCE_MEM, "dirin"),
+ !strcmp(platform_get_device_id(pdev)->name,
+ "basic-mmio-gpio-be"));
+}
- return gpiochip_remove(&bgc->gc);
+static int __devexit bgpio_remove(struct platform_device *pdev)
+{
+ return __bgpio_remove(&pdev->dev);
}
static const struct platform_device_id bgpio_id_table[] = {
@@ -522,6 +536,8 @@ static void __exit bgpio_exit(void)
}
module_exit(bgpio_exit);
+#endif /* CONFIG_GPIO_BASIC_MMIO */
+
MODULE_DESCRIPTION("Driver for basic memory-mapped GPIO controllers");
MODULE_AUTHOR("Anton Vorontsov <cbouat...@gmail.com>");
MODULE_LICENSE("GPL");
diff --git a/include/linux/basic_mmio_gpio.h b/include/linux/basic_mmio_gpio.h
index f23ec73..9277e59 100644
--- a/include/linux/basic_mmio_gpio.h
+++ b/include/linux/basic_mmio_gpio.h
@@ -13,9 +13,23 @@
#ifndef __BASIC_MMIO_GPIO_H
#define __BASIC_MMIO_GPIO_H
+#include <linux/types.h>
+
struct bgpio_pdata {
int base;
int ngpio;
};
+struct device;
+struct resource;
+
+int __devexit __bgpio_remove(struct device *dev);
+int __devinit __bgpio_probe(struct device *dev,
+ struct resource *res_dat,
+ struct resource *res_set,
+ struct resource *res_clr,
+ struct resource *res_dirout,
+ struct resource *res_dirin,
+ bool be);
+
#endif /* __BASIC_MMIO_GPIO_H */
--
1.7.4.1
Not sure you want to be using resources as the basic currency, nor do you
want your generic code doing the request_region stuff. That was a nasty
mistake we made in the IDE code.
Maybe the platform code should do that bit, but the generic stuff not - in
the PCI case the caller will have done pci_request_* itself and chances
are several of the resources are the same pci device resource and
different offsets so doing request_resource on them will blow up.
Concept looks good
Alan
Oh, right. How about this:
int __devinit bgpio_probe(struct device *dev,
unsigned long sz,
void __iomem *dat,
void __iomem *set,
void __iomem *clr,
void __iomem *dirout,
void __iomem *dirin,
bool big_endian);
I.e. PCI driver can now map the whole area with a single ioremap(),
and then call bgpio_probe(), which will provide accessors and
common MMIO GPIO registration stuff.
Not-yet-signed-off-by: Anton Vorontsov <cbouat...@gmail.com>
---
drivers/gpio/Kconfig | 6 +
drivers/gpio/Makefile | 1 +
drivers/gpio/basic_mmio_gpio.c | 241 ++++++++++++++++++++++++---------------
include/linux/basic_mmio_gpio.h | 15 +++
4 files changed, 171 insertions(+), 92 deletions(-)
index b2ec45f..947734a 100644
--- a/drivers/gpio/basic_mmio_gpio.c
+++ b/drivers/gpio/basic_mmio_gpio.c
@@ -284,20 +284,10 @@ static int bgpio_dir_out_inv(struct gpio_chip *gc, unsigned int gpio, int val)
return 0;
}
-static void __iomem *bgpio_request_and_map(struct device *dev,
- struct resource *res)
+static int bgpio_setup_accessors(struct device *dev,
+ struct bgpio_chip *bgc,
+ bool be)
{
- if (!devm_request_mem_region(dev, res->start, resource_size(res),
- res->name ?: "mmio_gpio"))
- return NULL;
-
- return devm_ioremap(dev, res->start, resource_size(res));
-}
-
-static int bgpio_setup_accessors(struct platform_device *pdev,
- struct bgpio_chip *bgc)
-{
- const struct platform_device_id *platid = platform_get_device_id(pdev);
switch (bgc->bits) {
case 8:
@@ -319,13 +309,11 @@ static int bgpio_setup_accessors(struct platform_device *pdev,
break;
#endif /* BITS_PER_LONG >= 64 */
default:
- dev_err(&pdev->dev, "unsupported data width %u bits\n",
- bgc->bits);
+ dev_err(dev, "unsupported data width %u bits\n", bgc->bits);
return -EINVAL;
}
- bgc->pin2mask = strcmp(platid->name, "basic-mmio-gpio-be") ?
- bgpio_pin2mask : bgpio_pin2mask_be;
+ bgc->pin2mask = be ? bgpio_pin2mask_be : bgpio_pin2mask;
return 0;
}
@@ -352,51 +340,22 @@ static int bgpio_setup_accessors(struct platform_device *pdev,
* - an input direction register (named "dirin") where a 1 bit indicates
* the GPIO is an input.
*/
-static int bgpio_setup_io(struct platform_device *pdev,
- struct bgpio_chip *bgc)
+static int bgpio_setup_io(struct bgpio_chip *bgc,
+ void __iomem *dat,
+ void __iomem *set,
+ void __iomem *clr)
{
- struct resource *res_set;
- struct resource *res_clr;
- struct resource *res_dat;
- resource_size_t dat_sz;
-
- res_dat = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dat");
- if (!res_dat)
- return -EINVAL;
-
- dat_sz = resource_size(res_dat);
- if (!is_power_of_2(dat_sz))
- return -EINVAL;
-
- bgc->bits = dat_sz * 8;
- if (bgc->bits > BITS_PER_LONG)
- return -EINVAL;
- bgc->reg_dat = bgpio_request_and_map(&pdev->dev, res_dat);
+ bgc->reg_dat = dat;
if (!bgc->reg_dat)
- return -ENOMEM;
-
- res_set = platform_get_resource_byname(pdev, IORESOURCE_MEM, "set");
- res_clr = platform_get_resource_byname(pdev, IORESOURCE_MEM, "clr");
- if (res_set && res_clr) {
- if (resource_size(res_set) != resource_size(res_clr) ||
- resource_size(res_set) != resource_size(res_dat))
- return -EINVAL;
-
- bgc->reg_set = bgpio_request_and_map(&pdev->dev, res_set);
- bgc->reg_clr = bgpio_request_and_map(&pdev->dev, res_clr);
- if (!bgc->reg_set || !bgc->reg_clr)
- return -ENOMEM;
+ return -EINVAL;
+ if (set && clr) {
+ bgc->reg_set = set;
+ bgc->reg_clr = clr;
bgc->gc.set = bgpio_set_with_clear;
- } else if (res_set && !res_clr) {
- if (resource_size(res_set) != resource_size(res_dat))
- return -EINVAL;
-
- bgc->reg_set = bgpio_request_and_map(&pdev->dev, res_set);
- if (!bgc->reg_set)
- return -ENOMEM;
-
+ } else if (set && !clr) {
+ bgc->reg_set = set;
bgc->gc.set = bgpio_set_set;
} else {
bgc->gc.set = bgpio_set;
@@ -407,27 +366,18 @@ static int bgpio_setup_io(struct platform_device *pdev,
return 0;
}
-static int bgpio_setup_direction(struct platform_device *pdev,
- struct bgpio_chip *bgc)
+static int bgpio_setup_direction(struct bgpio_chip *bgc,
+ void __iomem *dirout,
+ void __iomem *dirin)
{
- struct resource *res_dirout;
- struct resource *res_dirin;
-
- res_dirout = platform_get_resource_byname(pdev, IORESOURCE_MEM,
- "dirout");
- res_dirin = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dirin");
- if (res_dirout && res_dirin) {
+ if (dirout && dirin) {
return -EINVAL;
- } else if (res_dirout) {
- bgc->reg_dir = bgpio_request_and_map(&pdev->dev, res_dirout);
- if (!bgc->reg_dir)
- return -ENOMEM;
+ } else if (dirout) {
+ bgc->reg_dir = dirout;
bgc->gc.direction_output = bgpio_dir_out;
bgc->gc.direction_input = bgpio_dir_in;
- } else if (res_dirin) {
- bgc->reg_dir = bgpio_request_and_map(&pdev->dev, res_dirin);
- if (!bgc->reg_dir)
- return -ENOMEM;
+ } else if (dirin) {
+ bgc->reg_dir = dirin;
bgc->gc.direction_output = bgpio_dir_out_inv;
bgc->gc.direction_input = bgpio_dir_in_inv;
} else {
@@ -438,9 +388,23 @@ static int bgpio_setup_direction(struct platform_device *pdev,
return 0;
}
-static int __devinit bgpio_probe(struct platform_device *pdev)
+int __devexit bgpio_remove(struct device *dev)
+{
+ struct bgpio_chip *bgc = dev_get_drvdata(dev);
+
+ return gpiochip_remove(&bgc->gc);
+}
+EXPORT_SYMBOL_GPL(bgpio_remove);
+
+int __devinit bgpio_probe(struct device *dev,
+ unsigned long sz,
+ void __iomem *dat,
+ void __iomem *set,
+ void __iomem *clr,
+ void __iomem *dirout,
+ void __iomem *dirin,
+ bool big_endian)
{
- struct device *dev = &pdev->dev;
struct bgpio_pdata *pdata = dev_get_platdata(dev);
struct bgpio_chip *bgc;
int ret;
@@ -450,9 +414,16 @@ static int __devinit bgpio_probe(struct platform_device *pdev)
if (!bgc)
return -ENOMEM;
- ret = bgpio_setup_io(pdev, bgc);
- if (ret)
- return ret;
+ if (!is_power_of_2(sz))
+ return -EINVAL;
+
+ bgc->bits = sz * 8;
+ if (bgc->bits > BITS_PER_LONG)
+ return -EINVAL;
+
+ spin_lock_init(&bgc->lock);
+ bgc->gc.dev = dev;
+ bgc->gc.label = dev_name(dev);
ngpio = bgc->bits;
if (pdata) {
@@ -463,22 +434,23 @@ static int __devinit bgpio_probe(struct platform_device *pdev)
bgc->gc.base = -1;
}
- ret = bgpio_setup_accessors(pdev, bgc);
+ bgc->gc.ngpio = ngpio;
+
+ ret = bgpio_setup_io(bgc, dat, set, clr);
if (ret)
return ret;
- spin_lock_init(&bgc->lock);
- ret = bgpio_setup_direction(pdev, bgc);
+ ret = bgpio_setup_accessors(dev, bgc, big_endian);
if (ret)
return ret;
- bgc->data = bgc->read_reg(bgc->reg_dat);
+ ret = bgpio_setup_direction(bgc, dirout, dirin);
+ if (ret)
+ return ret;
- bgc->gc.ngpio = ngpio;
- bgc->gc.dev = dev;
- bgc->gc.label = dev_name(dev);
+ bgc->data = bgc->read_reg(bgc->reg_dat);
- platform_set_drvdata(pdev, bgc);
+ dev_set_drvdata(dev, bgc);
ret = gpiochip_add(&bgc->gc);
if (ret)
@@ -486,12 +458,95 @@ static int __devinit bgpio_probe(struct platform_device *pdev)
return ret;
}
+EXPORT_SYMBOL_GPL(bgpio_probe);
-static int __devexit bgpio_remove(struct platform_device *pdev)
+#ifdef CONFIG_GPIO_BASIC_MMIO
+
+static void __iomem *bgpio_map(struct platform_device *pdev,
+ const char *name,
+ resource_size_t sane_sz,
+ int *err)
{
- struct bgpio_chip *bgc = platform_get_drvdata(pdev);
+ struct device *dev = &pdev->dev;
+ struct resource *r;
+ resource_size_t start;
+ resource_size_t sz;
+ void __iomem *ret;
- return gpiochip_remove(&bgc->gc);
+ *err = 0;
+
+ r = platform_get_resource_byname(pdev, IORESOURCE_MEM, name);
+ if (!r)
+ return NULL;
+
+ sz = resource_size(r);
+ if (sz != sane_sz) {
+ *err = -EINVAL;
+ return NULL;
+ }
+
+ start = r->start;
+ if (!devm_request_mem_region(dev, start, sz, r->name)) {
+ *err = -EBUSY;
+ return NULL;
+ }
+
+ ret = devm_ioremap(dev, start, sz);
+ if (!ret) {
+ *err = -ENOMEM;
+ return NULL;
+ }
+
+ return ret;
+}
+
+static int __devinit bgpio_pdev_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct resource *r;
+ void __iomem *dat;
+ void __iomem *set;
+ void __iomem *clr;
+ void __iomem *dirout;
+ void __iomem *dirin;
+ unsigned long sz;
+ bool be;
+ int err;
+
+ r = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dat");
+ if (!r)
+ return -EINVAL;
+
+ sz = resource_size(r);
+
+ dat = bgpio_map(pdev, "dat", sz, &err);
+ if (!dat)
+ return err ? err : -EINVAL;
+
+ set = bgpio_map(pdev, "set", sz, &err);
+ if (err)
+ return err;
+
+ clr = bgpio_map(pdev, "clr", sz, &err);
+ if (err)
+ return err;
+
+ dirout = bgpio_map(pdev, "dirout", sz, &err);
+ if (err)
+ return err;
+
+ dirin = bgpio_map(pdev, "dirin", sz, &err);
+ if (err)
+ return err;
+
+ be = !strcmp(platform_get_device_id(pdev)->name, "basic-mmio-gpio-be");
+
+ return bgpio_probe(dev, sz, dat, set, clr, dirout, dirin, be);
+}
+
+static int __devexit bgpio_pdev_remove(struct platform_device *pdev)
+{
+ return bgpio_remove(&pdev->dev);
}
static const struct platform_device_id bgpio_id_table[] = {
@@ -506,8 +561,8 @@ static struct platform_driver bgpio_driver = {
.name = "basic-mmio-gpio",
},
.id_table = bgpio_id_table,
- .probe = bgpio_probe,
- .remove = __devexit_p(bgpio_remove),
+ .probe = bgpio_pdev_probe,
+ .remove = __devexit_p(bgpio_pdev_remove),
};
static int __init bgpio_init(void)
@@ -522,6 +577,8 @@ static void __exit bgpio_exit(void)
}
module_exit(bgpio_exit);
+#endif /* CONFIG_GPIO_BASIC_MMIO */
+
MODULE_DESCRIPTION("Driver for basic memory-mapped GPIO controllers");
MODULE_AUTHOR("Anton Vorontsov <cbouat...@gmail.com>");
MODULE_LICENSE("GPL");
diff --git a/include/linux/basic_mmio_gpio.h b/include/linux/basic_mmio_gpio.h
index f23ec73..f3a27e4 100644
--- a/include/linux/basic_mmio_gpio.h
+++ b/include/linux/basic_mmio_gpio.h
@@ -13,9 +13,24 @@
#ifndef __BASIC_MMIO_GPIO_H
#define __BASIC_MMIO_GPIO_H
+#include <linux/types.h>
+#include <linux/compiler.h>
+
struct bgpio_pdata {
int base;
int ngpio;
};
+struct device;
+
+int __devexit bgpio_remove(struct device *dev);
+int __devinit bgpio_probe(struct device *dev,
+ unsigned long sz,
+ void __iomem *dat,
+ void __iomem *set,
+ void __iomem *clr,
+ void __iomem *dirout,
+ void __iomem *dirin,
+ bool big_endian);
+
#endif /* __BASIC_MMIO_GPIO_H */
--
1.7.4.1
--
> On Tue, Apr 19, 2011 at 06:08:29PM +0100, Alan Cox wrote:
> > > How about exporting some bus/device-type neutral probe function, like
> > > we do in drivers/ata/pata_platform.c ?
> >
> > Not sure you want to be using resources as the basic currency, nor do you
> > want your generic code doing the request_region stuff. That was a nasty
> > mistake we made in the IDE code.
> >
> > Maybe the platform code should do that bit, but the generic stuff not - in
> > the PCI case the caller will have done pci_request_* itself and chances
> > are several of the resources are the same pci device resource and
> > different offsets so doing request_resource on them will blow up.
>
> Oh, right. How about this:
>
> int __devinit bgpio_probe(struct device *dev,
> unsigned long sz,
> void __iomem *dat,
> void __iomem *set,
> void __iomem *clr,
> void __iomem *dirout,
> void __iomem *dirin,
> bool big_endian);
>
> I.e. PCI driver can now map the whole area with a single ioremap(),
> and then call bgpio_probe(), which will provide accessors and
> common MMIO GPIO registration stuff.
Looks sane and providing its all using iomap/ioread/iowrite it covers all
cases nicely.
Actually not using iomap makes more sense on second thoughts, not all
platforms have a generic iomap implementation.
So yes this all looks good.
On Tue, 19 Apr 2011 08:44:29 -0600, Grant Likely wrote:
> On Tue, Apr 19, 2011 at 6:53 AM, Jean Delvare <kh...@linux-fr.org> wrote:
> > I need this to handle SMBus multiplexing on my Asus Z8NA-D6 board. It
> > has an ICH10, I've added support for older ICH chips in case someone
> > needs it, as it was relatively simply to do that.
>
> Your timing is impeccable. You're getting caught up in the big gpio
> driver consolidation. :-)
>
> Most gpio drivers end up looking pretty darn similar. Instead of
> writing the same types of drivers over and over again, the new
> approach is to try and consolidate the mmio drivers down to using
> basic_mmio_gpio.c.
>
> In this particular case, you've got a PCI device which looks to be
> going into config space to get some information about how the chip is
> layed out. What I would do is keep your existing pci probe & remove
> hooks, but use them to create and register child basic_mmio_gpio
> platform_devices for each gpio bank.
I can see there are still discussions going on with regards to
basic_mmio_gpio. To be honest, I don't have any opinion on this. My
only concern is that I have driver code which appears to work well
enough for me and I would like it to be merged in kernel 2.6.40.
So my questions are as follows: what do I get to do for it to happen?
If there a chance that my driver as it currently exists (i.e. not using
basic_mmio_gpio) gets reviewed and merged? Or do I have to rewrite it
using basic_mmio_gpio to get a chance?
I can't see any driver currently relying on basic_mmio_gpio in the
kernel tree. Why is that, and why would my driver have to, if none else
did yet?
And a technical question (which makes me feel somewhat ashamed as I
guess I really should know the answer): the ICH is using I/O ports for
GPIO control, not a memory mapping. Would basic_mmio_gpio work for it
still?
Thanks,
--
Jean Delvare
basic_mmio_gpio would need to use iomap for this and a lot of platforms
don't support generic iomap - so no it won't.
You don't generally want want to create sub platform devices anyway
without care as it makes sysfs, pci removal and power management ugly and
takes up *more* memory than repeating the code in the first places.
Alan
On Sat, 23 Apr 2011 15:47:07 +0100, Alan Cox wrote:
> > And a technical question (which makes me feel somewhat ashamed as I
> > guess I really should know the answer): the ICH is using I/O ports for
> > GPIO control, not a memory mapping. Would basic_mmio_gpio work for it
> > still?
>
> basic_mmio_gpio would need to use iomap for this and a lot of platforms
> don't support generic iomap - so no it won't.
>
> You don't generally want want to create sub platform devices anyway
> without care as it makes sysfs, pci removal and power management ugly and
> takes up *more* memory than repeating the code in the first places.
OK, so given that I can't use basic_mmio_gpio and Alan thinks that
having an independent driver is the way to go anyway, is there any
chance to get my code reviewed and merged? I am using it since the day
I submitted it (one month ago), it appears to work fine for me, and
it is a mandatory piece to support SMBus multiplexing on many x86 server
boards. I'd like to see it happen in kernel 2.6.40.
I can resend the patch if it helps, but I did not make any change to
the code since the first submission.
Thanks,
--
Jean Delvare
Applied, thanks
g.