All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V2 0/2] MFD and GPIO for STA2X11
@ 2012-02-16 13:00 Alessandro Rubini
  2012-02-16 13:00 ` [PATCH V2 1/2] mfd: Add driver for STA2X11 MFD block Alessandro Rubini
  2012-02-16 13:00 ` [PATCH V2 2/2] gpio: add STA2X11 GPIO block Alessandro Rubini
  0 siblings, 2 replies; 14+ messages in thread
From: Alessandro Rubini @ 2012-02-16 13:00 UTC (permalink / raw)
  To: linux-kernel; +Cc: giancarlo.asnaghi, alan, sameo, grant.likely, linus.walleij

These two patches introduce the multi-function device and GPIO support for
the STA2X11 I/O Hub.  Core PCI support is already in linux-next.

I sent version 1 on Jan 26th as RFC and got no comments back. This new
pair fixes a number of minor details and is rebased to the new
linux-next.

This time I Cc'd both patches to all interested parties, (last time
MFD people only got MFD and GPIO people only got GPIO), because the
relative order must be preserved.

Alessandro Rubini (2):
  mfd: Add driver for STA2X11 MFD block
  gpio: add STA2X11 GPIO block

 arch/x86/include/asm/sta2x11.h  |   12 +
 drivers/gpio/Kconfig            |    9 +
 drivers/gpio/Makefile           |    1 +
 drivers/gpio/gpio-sta2x11.c     |  443 ++++++++++++++++++++++++++++++++++++
 drivers/mfd/Kconfig             |    6 +
 drivers/mfd/Makefile            |    1 +
 drivers/mfd/sta2x11-mfd.c       |  470 +++++++++++++++++++++++++++++++++++++++
 include/linux/mfd/sta2x11-mfd.h |  324 +++++++++++++++++++++++++++
 8 files changed, 1266 insertions(+), 0 deletions(-)
 create mode 100644 arch/x86/include/asm/sta2x11.h
 create mode 100644 drivers/gpio/gpio-sta2x11.c
 create mode 100644 drivers/mfd/sta2x11-mfd.c
 create mode 100644 include/linux/mfd/sta2x11-mfd.h

-- 
1.7.7.2

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH V2 1/2] mfd: Add driver for STA2X11 MFD block
  2012-02-16 13:00 [PATCH V2 0/2] MFD and GPIO for STA2X11 Alessandro Rubini
@ 2012-02-16 13:00 ` Alessandro Rubini
  2012-02-16 19:29   ` Linus Walleij
                     ` (2 more replies)
  2012-02-16 13:00 ` [PATCH V2 2/2] gpio: add STA2X11 GPIO block Alessandro Rubini
  1 sibling, 3 replies; 14+ messages in thread
From: Alessandro Rubini @ 2012-02-16 13:00 UTC (permalink / raw)
  To: linux-kernel; +Cc: giancarlo.asnaghi, alan, sameo, grant.likely, linus.walleij

This also introduces <asm/sta2x11.h> to export a function that is
already in linux-next, and will increase with other prototypes and
constants over time.

Signed-off-by: Alessandro Rubini <rubini@gnudd.com>
Acked-by: Giancarlo Asnaghi <giancarlo.asnaghi@st.com>
Cc: Alan Cox <alan@linux.intel.com>
---
 arch/x86/include/asm/sta2x11.h  |   12 +
 drivers/mfd/Kconfig             |    6 +
 drivers/mfd/Makefile            |    1 +
 drivers/mfd/sta2x11-mfd.c       |  470 +++++++++++++++++++++++++++++++++++++++
 include/linux/mfd/sta2x11-mfd.h |  324 +++++++++++++++++++++++++++
 5 files changed, 813 insertions(+), 0 deletions(-)
 create mode 100644 arch/x86/include/asm/sta2x11.h
 create mode 100644 drivers/mfd/sta2x11-mfd.c
 create mode 100644 include/linux/mfd/sta2x11-mfd.h

diff --git a/arch/x86/include/asm/sta2x11.h b/arch/x86/include/asm/sta2x11.h
new file mode 100644
index 0000000..e9d32df
--- /dev/null
+++ b/arch/x86/include/asm/sta2x11.h
@@ -0,0 +1,12 @@
+/*
+ * Header file for STMicroelectronics ConneXt (STA2X11) IOHub
+ */
+#ifndef __ASM_STA2X11_H
+#define __ASM_STA2X11_H
+
+#include <linux/pci.h>
+
+/* This needs to be called from the MFD to configure its sub-devices */
+struct sta2x11_instance *sta2x11_get_instance(struct pci_dev *pdev);
+
+#endif /* __ASM_STA2X11_H */
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index bd60ce0..9e6730c 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -830,6 +830,12 @@ config MFD_INTEL_MSIC
 	  Passage) chip. This chip embeds audio, battery, GPIO, etc.
 	  devices used in Intel Medfield platforms.
 
+config MFD_STA2X11
+	bool "STA2X11 multi function device support"
+	depends on STA2X11
+	select MFD_CORE
+	select GPIO_STA2X11
+
 endmenu
 endif
 
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index b953bab..c5db71b 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_MFD_DAVINCI_VOICECODEC)	+= davinci_voicecodec.o
 obj-$(CONFIG_MFD_DM355EVM_MSP)	+= dm355evm_msp.o
 obj-$(CONFIG_MFD_TI_SSP)	+= ti-ssp.o
 
+obj-$(CONFIG_MFD_STA2X11)	+= sta2x11-mfd.o
 obj-$(CONFIG_MFD_STMPE)		+= stmpe.o
 obj-$(CONFIG_STMPE_I2C)		+= stmpe-i2c.o
 obj-$(CONFIG_STMPE_SPI)		+= stmpe-spi.o
diff --git a/drivers/mfd/sta2x11-mfd.c b/drivers/mfd/sta2x11-mfd.c
new file mode 100644
index 0000000..0d5943f
--- /dev/null
+++ b/drivers/mfd/sta2x11-mfd.c
@@ -0,0 +1,470 @@
+/*
+ * Copyright (c) 2009-2011 Wind River Systems, Inc.
+ * Copyright (c) 2011 ST Microelectronics (Alessandro Rubini)
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * 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/module.h>
+#include <linux/spinlock.h>
+#include <linux/errno.h>
+#include <linux/device.h>
+#include <linux/slab.h>
+#include <linux/list.h>
+#include <linux/io.h>
+#include <linux/ioport.h>
+#include <linux/pci.h>
+#include <linux/debugfs.h>
+#include <linux/seq_file.h>
+#include <linux/platform_device.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/sta2x11-mfd.h>
+
+#include <asm/sta2x11.h>
+
+/* This describes STA2X11 MFD chip for us, we may have several */
+struct sta2x11_mfd {
+	struct sta2x11_instance *instance;
+	spinlock_t lock;
+	struct list_head list;
+	void __iomem *sctl_regs;
+	void __iomem *apbreg_regs;
+};
+
+static LIST_HEAD(sta2x11_mfd_list);
+
+/* Three functions to act on the list */
+static struct sta2x11_mfd *sta2x11_mfd_find(struct pci_dev *pdev)
+{
+	struct sta2x11_instance *instance;
+	struct sta2x11_mfd *mfd;
+
+	if (!pdev && !list_empty(&sta2x11_mfd_list)) {
+		pr_warning("%s: Unspecified device, "
+			    "using first instance\n", __func__);
+		return list_entry(sta2x11_mfd_list.next,
+				  struct sta2x11_mfd, list);
+	}
+
+	instance = sta2x11_get_instance(pdev);
+	if (!instance)
+		return NULL;
+	list_for_each_entry(mfd, &sta2x11_mfd_list, list) {
+		if (mfd->instance == instance)
+			return mfd;
+	}
+	return NULL;
+}
+
+static int __devinit sta2x11_mfd_add(struct pci_dev *pdev, gfp_t flags)
+{
+	struct sta2x11_mfd *mfd = sta2x11_mfd_find(pdev);
+	struct sta2x11_instance *instance;
+
+	if (mfd)
+		return -EBUSY;
+	instance = sta2x11_get_instance(pdev);
+	if (!instance)
+		return -EINVAL;
+	mfd = kzalloc(sizeof(*mfd), flags);
+	if (!mfd)
+		return -ENOMEM;
+	INIT_LIST_HEAD(&mfd->list);
+	spin_lock_init(&mfd->lock);
+	mfd->instance = instance;
+	list_add(&mfd->list, &sta2x11_mfd_list);
+	return 0;
+}
+
+static int __devexit mfd_remove(struct pci_dev *pdev)
+{
+	struct sta2x11_mfd *mfd = sta2x11_mfd_find(pdev);
+
+	if (!mfd)
+		return -ENODEV;
+	list_del(&mfd->list);
+	kfree(mfd);
+	return 0;
+}
+
+/* These two functions are exported and are not expected to fail */
+u32 sta2x11_sctl_mask(struct pci_dev *pdev, u32 reg, u32 mask, u32 val)
+{
+	struct sta2x11_mfd *mfd = sta2x11_mfd_find(pdev);
+	u32 r;
+	unsigned long flags;
+
+	if (!mfd) {
+		dev_warn(&pdev->dev, ": can't access sctl regs\n");
+		return 0;
+	}
+	if (!mfd->sctl_regs) {
+		dev_warn(&pdev->dev, ": system ctl not initialized\n");
+		return 0;
+	}
+	spin_lock_irqsave(&mfd->lock, flags);
+	r = readl(mfd->sctl_regs + reg);
+	r &= ~mask;
+	r |= val;
+	if (mask)
+		writel(r, mfd->sctl_regs + reg);
+	spin_unlock_irqrestore(&mfd->lock, flags);
+	return r;
+}
+EXPORT_SYMBOL(sta2x11_sctl_mask);
+
+u32 sta2x11_apbreg_mask(struct pci_dev *pdev, u32 reg, u32 mask, u32 val)
+{
+	struct sta2x11_mfd *mfd = sta2x11_mfd_find(pdev);
+	u32 r;
+	unsigned long flags;
+
+	if (!mfd) {
+		dev_warn(&pdev->dev, ": can't access apb regs\n");
+		return 0;
+	}
+	if (!mfd->apbreg_regs) {
+		dev_warn(&pdev->dev, ": apb bridge not initialized\n");
+		return 0;
+	}
+	spin_lock_irqsave(&mfd->lock, flags);
+	r = readl(mfd->apbreg_regs + reg);
+	r &= ~mask;
+	r |= val;
+	if (mask)
+		writel(r, mfd->apbreg_regs + reg);
+	spin_unlock_irqrestore(&mfd->lock, flags);
+	return r;
+}
+EXPORT_SYMBOL(sta2x11_apbreg_mask);
+
+/* Two debugfs files, for our registers (FIXME: one instance only) */
+#define REG(regname) {.name = #regname, .offset = SCTL_ ## regname}
+static struct debugfs_reg32 sta2x11_sctl_regs[] = {
+	REG(SCCTL), REG(ARMCFG), REG(SCPLLCTL), REG(SCPLLFCTRL),
+	REG(SCRESFRACT), REG(SCRESCTRL1), REG(SCRESXTRL2), REG(SCPEREN0),
+	REG(SCPEREN1), REG(SCPEREN2), REG(SCGRST), REG(SCPCIPMCR1),
+	REG(SCPCIPMCR2), REG(SCPCIPMSR1), REG(SCPCIPMSR2), REG(SCPCIPMSR3),
+	REG(SCINTREN), REG(SCRISR), REG(SCCLKSTAT0), REG(SCCLKSTAT1),
+	REG(SCCLKSTAT2), REG(SCRSTSTA),
+};
+#undef REG
+
+static struct debugfs_regset32 sctl_regset = {
+	.regs = sta2x11_sctl_regs,
+	.nregs = ARRAY_SIZE(sta2x11_sctl_regs),
+};
+
+#define REG(regname) {.name = #regname, .offset = regname}
+static struct debugfs_reg32 sta2x11_apbreg_regs[] = {
+	REG(APBREG_BSR), REG(APBREG_PAER), REG(APBREG_PWAC), REG(APBREG_PRAC),
+	REG(APBREG_PCG), REG(APBREG_PUR), REG(APBREG_EMU_PCG),
+};
+#undef REG
+
+static struct debugfs_regset32 apbreg_regset = {
+	.regs = sta2x11_apbreg_regs,
+	.nregs = ARRAY_SIZE(sta2x11_apbreg_regs),
+};
+
+static struct dentry *sta2x11_sctl_debugfs;
+static struct dentry *sta2x11_apbreg_debugfs;
+
+/* Probe for the two platform devices */
+static int sta2x11_sctl_probe(struct platform_device *dev)
+{
+	struct pci_dev **pdev;
+	struct sta2x11_mfd *mfd;
+	struct resource *res;
+
+	pdev = dev->dev.platform_data;
+	mfd = sta2x11_mfd_find(*pdev);
+	if (!mfd)
+		return -ENODEV;
+
+	res = platform_get_resource(dev, IORESOURCE_MEM, 0);
+	if (!res)
+		return -ENOMEM;
+
+	if (!request_mem_region(res->start, resource_size(res),
+				"sta2x11-sctl"))
+		return -EBUSY;
+
+	mfd->sctl_regs = ioremap(res->start, resource_size(res));
+	if (!mfd->sctl_regs) {
+		release_mem_region(res->start, resource_size(res));
+		return -ENOMEM;
+	}
+	sctl_regset.base = mfd->sctl_regs;
+	sta2x11_sctl_debugfs = debugfs_create_regset32("sta2x11-sctl",
+						  S_IFREG | S_IRUGO,
+						  NULL, &sctl_regset);
+	return 0;
+}
+
+static int sta2x11_apbreg_probe(struct platform_device *dev)
+{
+	struct pci_dev **pdev;
+	struct sta2x11_mfd *mfd;
+	struct resource *res;
+
+	pdev = dev->dev.platform_data;
+	dev_dbg(&dev->dev, "%s: pdata is %p\n", __func__, pdev);
+	dev_dbg(&dev->dev, "%s: *pdata is %p\n", __func__, *pdev);
+
+	mfd = sta2x11_mfd_find(*pdev);
+	if (!mfd)
+		return -ENODEV;
+
+	res = platform_get_resource(dev, IORESOURCE_MEM, 0);
+	if (!res)
+		return -ENOMEM;
+
+	if (!request_mem_region(res->start, resource_size(res),
+				"sta2x11-apbreg"))
+		return -EBUSY;
+
+	mfd->apbreg_regs = ioremap(res->start, resource_size(res));
+	if (!mfd->apbreg_regs) {
+		release_mem_region(res->start, resource_size(res));
+		return -ENOMEM;
+	}
+	dev_dbg(&dev->dev, "%s: regbase %p\n", __func__, mfd->apbreg_regs);
+
+	apbreg_regset.base = mfd->apbreg_regs;
+	sta2x11_apbreg_debugfs = debugfs_create_regset32("sta2x11-apbreg",
+						  S_IFREG | S_IRUGO,
+						  NULL, &apbreg_regset);
+	return 0;
+}
+
+/* The two platform drivers */
+static struct platform_driver sta2x11_sctl_platform_driver = {
+	.driver = {
+		.name	= "sta2x11-sctl",
+		.owner	= THIS_MODULE,
+	},
+	.probe		= sta2x11_sctl_probe,
+};
+
+static int __init sta2x11_sctl_init(void)
+{
+	pr_info("%s\n", __func__);
+	return platform_driver_register(&sta2x11_sctl_platform_driver);
+}
+
+static struct platform_driver sta2x11_platform_driver = {
+	.driver = {
+		.name	= "sta2x11-apbreg",
+		.owner	= THIS_MODULE,
+	},
+	.probe		= sta2x11_apbreg_probe,
+};
+
+static int __init sta2x11_apbreg_init(void)
+{
+	pr_info("%s\n", __func__);
+	return platform_driver_register(&sta2x11_platform_driver);
+}
+
+/*
+ * What follows is the PCI device that hosts the above two pdevs.
+ * Each logic block is 4kB and they are all consecutive: we use this info.
+ */
+
+/* Bar 0 */
+enum bar0_cells {
+	GPIO_0 = 0,
+	GPIO_1,
+	GPIO_2,
+	GPIO_3,
+	SCTL,
+	SCR,
+	TIME,
+};
+/* Bar 1 */
+enum bar1_cells {
+	APBREG = 0,
+};
+#define CELL_4K(_name, _cell) { \
+		.name = _name, \
+		.start = _cell * 4096, .end = _cell * 4096 + 4095, \
+		.flags = IORESOURCE_MEM, \
+		}
+
+static const __devinitconst struct resource gpio_resources[] = {
+	{
+		.name = "sta2x11_gpio", /* 4 consecutive cells, 1 driver */
+		.start = 0,
+		.end = (4 * 4096) - 1,
+		.flags = IORESOURCE_MEM,
+	}
+};
+static const __devinitconst struct resource sctl_resources[] = {
+	CELL_4K("sta2x11-sctl", SCTL),
+};
+static const __devinitconst struct resource scr_resources[] = {
+	CELL_4K("sta2x11-scr", SCR),
+};
+static const __devinitconst struct resource time_resources[] = {
+	CELL_4K("sta2x11-time", TIME),
+};
+
+static const __devinitconst struct resource apbreg_resources[] = {
+	CELL_4K("sta2x11-apbreg", APBREG),
+};
+
+#define DEV(_name, _r) \
+	{ .name = _name, .num_resources = ARRAY_SIZE(_r), .resources = _r, }
+
+static __devinitdata struct mfd_cell sta2x11_mfd_bar0[] = {
+	DEV("sta2x11-gpio", gpio_resources), /* offset 0: we add pdata later */
+	DEV("sta2x11-sctl", sctl_resources),
+	DEV("sta2x11-scr", scr_resources),
+	DEV("sta2x11-time", time_resources),
+};
+
+static __devinitdata struct mfd_cell sta2x11_mfd_bar1[] = {
+	DEV("sta2x11-apbreg", apbreg_resources),
+};
+
+static int sta2x11_mfd_suspend(struct pci_dev *pdev, pm_message_t state)
+{
+	pci_save_state(pdev);
+	pci_disable_device(pdev);
+	pci_set_power_state(pdev, pci_choose_state(pdev, state));
+
+	return 0;
+}
+
+static int sta2x11_mfd_resume(struct pci_dev *pdev)
+{
+	int err;
+
+	pci_set_power_state(pdev, 0);
+	err = pci_enable_device(pdev);
+	if (err)
+		return err;
+	pci_restore_state(pdev);
+
+	return 0;
+}
+
+static int __devinit sta2x11_mfd_probe(struct pci_dev *pdev,
+				       const struct pci_device_id *pci_id)
+{
+	int err, i;
+	struct sta2x11_gpio_pdata *gpio_data;
+
+	dev_info(&pdev->dev, "%s\n", __func__);
+
+	err = pci_enable_device(pdev);
+	if (err) {
+		dev_err(&pdev->dev, "Can't enable device.\n");
+		return err;
+	}
+
+	err = pci_enable_msi(pdev);
+	if (err)
+		dev_info(&pdev->dev, "Enable msi failed\n");
+
+	/* Read gpio config data as pci device's platform data */
+	gpio_data = dev_get_platdata(&pdev->dev);
+	if (!gpio_data)
+		dev_warn(&pdev->dev, "no gpio configuration\n");
+
+#if 1
+	dev_dbg(&pdev->dev, "%s, gpio_data = %p (%p)\n", __func__,
+		gpio_data, &gpio_data);
+#endif
+	dev_dbg(&pdev->dev, "%s, pdev = %p (%p)\n", __func__,
+		pdev, &pdev);
+
+
+	/* platform data is the pci device for all of them */
+	for (i = 0; i < ARRAY_SIZE(sta2x11_mfd_bar0); i++) {
+		sta2x11_mfd_bar0[i].pdata_size = sizeof(pdev);
+		sta2x11_mfd_bar0[i].platform_data = &pdev;
+	}
+	sta2x11_mfd_bar1[0].pdata_size = sizeof(pdev);
+	sta2x11_mfd_bar1[0].platform_data = &pdev;
+
+	/* Record this pdev before mfd_add_devices: their probe looks for it */
+	sta2x11_mfd_add(pdev, GFP_ATOMIC);
+
+
+	err = mfd_add_devices(&pdev->dev, -1,
+			      sta2x11_mfd_bar0,
+			      ARRAY_SIZE(sta2x11_mfd_bar0),
+			      &pdev->resource[0],
+			      0);
+	if (err) {
+		dev_err(&pdev->dev, "mfd_add_devices[0] failed: %d\n", err);
+		goto err_disable;
+	}
+
+	err = mfd_add_devices(&pdev->dev, -1,
+			      sta2x11_mfd_bar1,
+			      ARRAY_SIZE(sta2x11_mfd_bar1),
+			      &pdev->resource[1],
+			      0);
+	if (err) {
+		dev_err(&pdev->dev, "mfd_add_devices[1] failed: %d\n", err);
+		goto err_disable;
+	}
+
+	return 0;
+
+err_disable:
+	pci_disable_device(pdev);
+	pci_disable_msi(pdev);
+	/* mfd_remove(pdev); -- it is in an exit section, I can't call it */
+	return err;
+}
+
+static DEFINE_PCI_DEVICE_TABLE(sta2x11_mfd_tbl) = {
+	{PCI_DEVICE(PCI_VENDOR_ID_STMICRO, PCI_DEVICE_ID_STMICRO_GPIO)},
+	{0,},
+};
+
+static struct pci_driver sta2x11_mfd_driver = {
+	.name =		"sta2x11-mfd",
+	.id_table =	sta2x11_mfd_tbl,
+	.probe =	sta2x11_mfd_probe,
+	.suspend =	sta2x11_mfd_suspend,
+	.resume =	sta2x11_mfd_resume,
+};
+
+static int __init sta2x11_mfd_init(void)
+{
+	pr_info("%s\n", __func__);
+	return pci_register_driver(&sta2x11_mfd_driver);
+}
+
+/*
+ * All of this must be ready before "normal" devices like MMCI appear.
+ * But MFD (the pci device) can't be too early. The following choice
+ * prepares platform drivers very early and probe the PCI device later,
+ * but before other PCI devices.
+ */
+subsys_initcall(sta2x11_apbreg_init);
+subsys_initcall(sta2x11_sctl_init);
+rootfs_initcall(sta2x11_mfd_init);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Wind River");
+MODULE_DESCRIPTION("STA2x11 mfd for GPIO, SCTL and APBREG");
+MODULE_DEVICE_TABLE(pci, sta2x11_mfd_tbl);
diff --git a/include/linux/mfd/sta2x11-mfd.h b/include/linux/mfd/sta2x11-mfd.h
new file mode 100644
index 0000000..9c603e4
--- /dev/null
+++ b/include/linux/mfd/sta2x11-mfd.h
@@ -0,0 +1,324 @@
+/*
+ * Copyright (c) 2009-2011 Wind River Systems, Inc.
+ * Copyright (c) 2011 ST Microelectronics (Alessandro Rubini)
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * 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
+ *
+ * The STMicroelectronics ConneXt (STA2X11) chip has several unrelated
+ * functions in one PCI endpoint functions. This driver simply
+ * registers the platform devices in this iomemregion and exports a few
+ * functions to access common registers
+ */
+
+#ifndef __STA2X11_MFD_H
+#define __STA2X11_MFD_H
+#include <linux/types.h>
+#include <linux/pci.h>
+
+/*
+ * The MFD PCI block includes the GPIO peripherals and other register blocks.
+ * For GPIO, we have 32*4 bits (I use "gsta" for "gpio sta2x11".)
+ */
+#define GSTA_GPIO_PER_BLOCK	32
+#define GSTA_NR_BLOCKS		4
+#define GSTA_NR_GPIO		(GSTA_GPIO_PER_BLOCK * GSTA_NR_BLOCKS)
+
+/* Pinconfig is set by the board definition: altfunc, pull-up, pull-down */
+struct sta2x11_gpio_pdata {
+	unsigned pinconfig[GSTA_NR_GPIO];
+};
+
+/* Macros below lifted from sh_pfc.h, with minor differences */
+#define PINMUX_TYPE_NONE		0
+#define PINMUX_TYPE_FUNCTION		1
+#define PINMUX_TYPE_OUTPUT_LOW		2
+#define PINMUX_TYPE_OUTPUT_HIGH		3
+#define PINMUX_TYPE_INPUT		4
+#define PINMUX_TYPE_INPUT_PULLUP	5
+#define PINMUX_TYPE_INPUT_PULLDOWN	6
+
+/* Give names to GPIO pins, like PXA does, taken from the manual */
+#define GPIO0			0
+#define GPIO1			1
+#define GPIO2			2
+#define GPIO3			3
+#define GPIO4			4
+#define GPIO5			5
+#define GPIO6			6
+#define GPIO7			7
+#define GPIO8_RGBOUT_RED7	8
+#define GPIO9_RGBOUT_RED6	9
+#define GPIO10_RGBOUT_RED5	10
+#define GPIO11_RGBOUT_RED4	11
+#define GPIO12_RGBOUT_RED3	12
+#define GPIO13_RGBOUT_RED2	13
+#define GPIO14_RGBOUT_RED1	14
+#define GPIO15_RGBOUT_RED0	15
+#define GPIO16_RGBOUT_GREEN7	16
+#define GPIO17_RGBOUT_GREEN6	17
+#define GPIO18_RGBOUT_GREEN5	18
+#define GPIO19_RGBOUT_GREEN4	19
+#define GPIO20_RGBOUT_GREEN3	20
+#define GPIO21_RGBOUT_GREEN2	21
+#define GPIO22_RGBOUT_GREEN1	22
+#define GPIO23_RGBOUT_GREEN0	23
+#define GPIO24_RGBOUT_BLUE7	24
+#define GPIO25_RGBOUT_BLUE6	25
+#define GPIO26_RGBOUT_BLUE5	26
+#define GPIO27_RGBOUT_BLUE4	27
+#define GPIO28_RGBOUT_BLUE3	28
+#define GPIO29_RGBOUT_BLUE2	29
+#define GPIO30_RGBOUT_BLUE1	30
+#define GPIO31_RGBOUT_BLUE0	31
+#define GPIO32_RGBOUT_VSYNCH	32
+#define GPIO33_RGBOUT_HSYNCH	33
+#define GPIO34_RGBOUT_DEN	34
+#define GPIO35_ETH_CRS_DV	35
+#define GPIO36_ETH_TXD1		36
+#define GPIO37_ETH_TXD0		37
+#define GPIO38_ETH_TX_EN	38
+#define GPIO39_MDIO		39
+#define GPIO40_ETH_REF_CLK	40
+#define GPIO41_ETH_RXD1		41
+#define GPIO42_ETH_RXD0		42
+#define GPIO43_MDC		43
+#define GPIO44_CAN_TX		44
+#define GPIO45_CAN_RX		45
+#define GPIO46_MLB_DAT		46
+#define GPIO47_MLB_SIG		47
+#define GPIO48_SPI0_CLK		48
+#define GPIO49_SPI0_TXD		49
+#define GPIO50_SPI0_RXD		50
+#define GPIO51_SPI0_FRM		51
+#define GPIO52_SPI1_CLK		52
+#define GPIO53_SPI1_TXD		53
+#define GPIO54_SPI1_RXD		54
+#define GPIO55_SPI1_FRM		55
+#define GPIO56_SPI2_CLK		56
+#define GPIO57_SPI2_TXD		57
+#define GPIO58_SPI2_RXD		58
+#define GPIO59_SPI2_FRM		59
+#define GPIO60_I2C0_SCL		60
+#define GPIO61_I2C0_SDA		61
+#define GPIO62_I2C1_SCL		62
+#define GPIO63_I2C1_SDA		63
+#define GPIO64_I2C2_SCL		64
+#define GPIO65_I2C2_SDA		65
+#define GPIO66_I2C3_SCL		66
+#define GPIO67_I2C3_SDA		67
+#define GPIO68_MSP0_RCK		68
+#define GPIO69_MSP0_RXD		69
+#define GPIO70_MSP0_RFS		70
+#define GPIO71_MSP0_TCK		71
+#define GPIO72_MSP0_TXD		72
+#define GPIO73_MSP0_TFS		73
+#define GPIO74_MSP0_SCK		74
+#define GPIO75_MSP1_CK		75
+#define GPIO76_MSP1_RXD		76
+#define GPIO77_MSP1_FS		77
+#define GPIO78_MSP1_TXD		78
+#define GPIO79_MSP2_CK		79
+#define GPIO80_MSP2_RXD		80
+#define GPIO81_MSP2_FS		81
+#define GPIO82_MSP2_TXD		82
+#define GPIO83_MSP3_CK		83
+#define GPIO84_MSP3_RXD		84
+#define GPIO85_MSP3_FS		85
+#define GPIO86_MSP3_TXD		86
+#define GPIO87_MSP4_CK		87
+#define GPIO88_MSP4_RXD		88
+#define GPIO89_MSP4_FS		89
+#define GPIO90_MSP4_TXD		90
+#define GPIO91_MSP5_CK		91
+#define GPIO92_MSP5_RXD		92
+#define GPIO93_MSP5_FS		93
+#define GPIO94_MSP5_TXD		94
+#define GPIO95_SDIO3_DAT3	95
+#define GPIO96_SDIO3_DAT2	96
+#define GPIO97_SDIO3_DAT1	97
+#define GPIO98_SDIO3_DAT0	98
+#define GPIO99_SDIO3_CLK	99
+#define GPIO100_SDIO3_CMD	100
+#define GPIO101			101
+#define GPIO102			102
+#define GPIO103			103
+#define GPIO104			104
+#define GPIO105_SDIO2_DAT3	105
+#define GPIO106_SDIO2_DAT2	106
+#define GPIO107_SDIO2_DAT1	107
+#define GPIO108_SDIO2_DAT0	108
+#define GPIO109_SDIO2_CLK	109
+#define GPIO110_SDIO2_CMD	110
+#define GPIO111			111
+#define GPIO112			112
+#define GPIO113			113
+#define GPIO114			114
+#define GPIO115_SDIO1_DAT3	115
+#define GPIO116_SDIO1_DAT2	116
+#define GPIO117_SDIO1_DAT1	117
+#define GPIO118_SDIO1_DAT0	118
+#define GPIO119_SDIO1_CLK	119
+#define GPIO120_SDIO1_CMD	120
+#define GPIO121			121
+#define GPIO122			122
+#define GPIO123			123
+#define GPIO124			124
+#define GPIO125_UART2_TXD	125
+#define GPIO126_UART2_RXD	126
+#define GPIO127_UART3_TXD	127
+
+/*
+ * The APB bridge has its own registers, needed by our users as well.
+ * They are accessed with the following read/mask/write function.
+ */
+u32 sta2x11_apbreg_mask(struct pci_dev *pdev, u32 reg, u32 mask, u32 val);
+
+/* CAN and MLB */
+#define APBREG_BSR	0x00	/* Bridge Status Reg */
+#define APBREG_PAER	0x08	/* Peripherals Address Error Reg */
+#define APBREG_PWAC	0x20	/* Peripheral Write Access Control reg */
+#define APBREG_PRAC	0x40	/* Peripheral Read Access Control reg */
+#define APBREG_PCG	0x60	/* Peripheral Clock Gating Reg */
+#define APBREG_PUR	0x80	/* Peripheral Under Reset Reg */
+#define APBREG_EMU_PCG	0xA0	/* Emulator Peripheral Clock Gating Reg */
+
+#define APBREG_CAN	(1 << 1)
+#define APBREG_MLB	(1 << 3)
+
+/* SARAC */
+#define APBREG_BSR_SARAC     0x100 /* Bridge Status Reg */
+#define APBREG_PAER_SARAC    0x108 /* Peripherals Address Error Reg */
+#define APBREG_PWAC_SARAC    0x120 /* Peripheral Write Access Control reg */
+#define APBREG_PRAC_SARAC    0x140 /* Peripheral Read Access Control reg */
+#define APBREG_PCG_SARAC     0x160 /* Peripheral Clock Gating Reg */
+#define APBREG_PUR_SARAC     0x180 /* Peripheral Under Reset Reg */
+#define APBREG_EMU_PCG_SARAC 0x1A0 /* Emulator Peripheral Clock Gating Reg */
+
+#define APBREG_SARAC	(1 << 2)
+
+/*
+ * The system controller has its own registers. Some of these are accessed
+ * by out users as well, using the following read/mask/write/function
+ */
+u32 sta2x11_sctl_mask(struct pci_dev *pdev, u32 reg, u32 mask, u32 val);
+
+#define SCTL_SCCTL		0x00	/* System controller control register */
+#define SCTL_ARMCFG		0x04	/* ARM configuration register */
+#define SCTL_SCPLLCTL		0x08	/* PLL control status register */
+#define SCTL_SCPLLFCTRL		0x0c	/* PLL frequency control register */
+#define SCTL_SCRESFRACT		0x10	/* PLL fractional input register */
+#define SCTL_SCRESCTRL1		0x14	/* Peripheral reset control 1 */
+#define SCTL_SCRESXTRL2		0x18	/* Peripheral reset control 2 */
+#define SCTL_SCPEREN0		0x1c	/* Peripheral clock enable register 0 */
+#define SCTL_SCPEREN1		0x20	/* Peripheral clock enable register 1 */
+#define SCTL_SCPEREN2		0x24	/* Peripheral clock enable register 2 */
+#define SCTL_SCGRST		0x28	/* Peripheral global reset */
+#define SCTL_SCPCIPMCR1		0x30	/* PCI power management control 1 */
+#define SCTL_SCPCIPMCR2		0x34	/* PCI power management control 2 */
+#define SCTL_SCPCIPMSR1		0x38	/* PCI power management status 1 */
+#define SCTL_SCPCIPMSR2		0x3c	/* PCI power management status 2 */
+#define SCTL_SCPCIPMSR3		0x40	/* PCI power management status 3 */
+#define SCTL_SCINTREN		0x44	/* Interrupt enable */
+#define SCTL_SCRISR		0x48	/* RAW interrupt status */
+#define SCTL_SCCLKSTAT0		0x4c	/* Peripheral clocks status 0 */
+#define SCTL_SCCLKSTAT1		0x50	/* Peripheral clocks status 1 */
+#define SCTL_SCCLKSTAT2		0x54	/* Peripheral clocks status 2 */
+#define SCTL_SCRSTSTA		0x58	/* Reset status register */
+
+#define SCTL_SCRESCTRL1_USB_PHY_POR	(1 << 0)
+#define SCTL_SCRESCTRL1_USB_OTG	(1 << 1)
+#define SCTL_SCRESCTRL1_USB_HRST	(1 << 2)
+#define SCTL_SCRESCTRL1_USB_PHY_HOST	(1 << 3)
+#define SCTL_SCRESCTRL1_SATAII	(1 << 4)
+#define SCTL_SCRESCTRL1_VIP		(1 << 5)
+#define SCTL_SCRESCTRL1_PER_MMC0	(1 << 6)
+#define SCTL_SCRESCTRL1_PER_MMC1	(1 << 7)
+#define SCTL_SCRESCTRL1_PER_GPIO0	(1 << 8)
+#define SCTL_SCRESCTRL1_PER_GPIO1	(1 << 9)
+#define SCTL_SCRESCTRL1_PER_GPIO2	(1 << 10)
+#define SCTL_SCRESCTRL1_PER_GPIO3	(1 << 11)
+#define SCTL_SCRESCTRL1_PER_MTU0	(1 << 12)
+#define SCTL_SCRESCTRL1_KER_SPI0	(1 << 13)
+#define SCTL_SCRESCTRL1_KER_SPI1	(1 << 14)
+#define SCTL_SCRESCTRL1_KER_SPI2	(1 << 15)
+#define SCTL_SCRESCTRL1_KER_MCI0	(1 << 16)
+#define SCTL_SCRESCTRL1_KER_MCI1	(1 << 17)
+#define SCTL_SCRESCTRL1_PRE_HSI2C0	(1 << 18)
+#define SCTL_SCRESCTRL1_PER_HSI2C1	(1 << 19)
+#define SCTL_SCRESCTRL1_PER_HSI2C2	(1 << 20)
+#define SCTL_SCRESCTRL1_PER_HSI2C3	(1 << 21)
+#define SCTL_SCRESCTRL1_PER_MSP0	(1 << 22)
+#define SCTL_SCRESCTRL1_PER_MSP1	(1 << 23)
+#define SCTL_SCRESCTRL1_PER_MSP2	(1 << 24)
+#define SCTL_SCRESCTRL1_PER_MSP3	(1 << 25)
+#define SCTL_SCRESCTRL1_PER_MSP4	(1 << 26)
+#define SCTL_SCRESCTRL1_PER_MSP5	(1 << 27)
+#define SCTL_SCRESCTRL1_PER_MMC	(1 << 28)
+#define SCTL_SCRESCTRL1_KER_MSP0	(1 << 29)
+#define SCTL_SCRESCTRL1_KER_MSP1	(1 << 30)
+#define SCTL_SCRESCTRL1_KER_MSP2	(1 << 31)
+
+#define SCTL_SCPEREN0_UART0		(1 << 0)
+#define SCTL_SCPEREN0_UART1		(1 << 1)
+#define SCTL_SCPEREN0_UART2		(1 << 2)
+#define SCTL_SCPEREN0_UART3		(1 << 3)
+#define SCTL_SCPEREN0_MSP0		(1 << 4)
+#define SCTL_SCPEREN0_MSP1		(1 << 5)
+#define SCTL_SCPEREN0_MSP2		(1 << 6)
+#define SCTL_SCPEREN0_MSP3		(1 << 7)
+#define SCTL_SCPEREN0_MSP4		(1 << 8)
+#define SCTL_SCPEREN0_MSP5		(1 << 9)
+#define SCTL_SCPEREN0_SPI0		(1 << 10)
+#define SCTL_SCPEREN0_SPI1		(1 << 11)
+#define SCTL_SCPEREN0_SPI2		(1 << 12)
+#define SCTL_SCPEREN0_I2C0		(1 << 13)
+#define SCTL_SCPEREN0_I2C1		(1 << 14)
+#define SCTL_SCPEREN0_I2C2		(1 << 15)
+#define SCTL_SCPEREN0_I2C3		(1 << 16)
+#define SCTL_SCPEREN0_SVDO_LVDS		(1 << 17)
+#define SCTL_SCPEREN0_USB_HOST		(1 << 18)
+#define SCTL_SCPEREN0_USB_OTG		(1 << 19)
+#define SCTL_SCPEREN0_MCI0		(1 << 20)
+#define SCTL_SCPEREN0_MCI1		(1 << 21)
+#define SCTL_SCPEREN0_MCI2		(1 << 22)
+#define SCTL_SCPEREN0_MCI3		(1 << 23)
+#define SCTL_SCPEREN0_SATA		(1 << 24)
+#define SCTL_SCPEREN0_ETHERNET		(1 << 25)
+#define SCTL_SCPEREN0_VIC		(1 << 26)
+#define SCTL_SCPEREN0_DMA_AUDIO		(1 << 27)
+#define SCTL_SCPEREN0_DMA_SOC		(1 << 28)
+#define SCTL_SCPEREN0_RAM		(1 << 29)
+#define SCTL_SCPEREN0_VIP		(1 << 30)
+#define SCTL_SCPEREN0_ARM		(1 << 31)
+
+#define SCTL_SCPEREN1_UART0		(1 << 0)
+#define SCTL_SCPEREN1_UART1		(1 << 1)
+#define SCTL_SCPEREN1_UART2		(1 << 2)
+#define SCTL_SCPEREN1_UART3		(1 << 3)
+#define SCTL_SCPEREN1_MSP0		(1 << 4)
+#define SCTL_SCPEREN1_MSP1		(1 << 5)
+#define SCTL_SCPEREN1_MSP2		(1 << 6)
+#define SCTL_SCPEREN1_MSP3		(1 << 7)
+#define SCTL_SCPEREN1_MSP4		(1 << 8)
+#define SCTL_SCPEREN1_MSP5		(1 << 9)
+#define SCTL_SCPEREN1_SPI0		(1 << 10)
+#define SCTL_SCPEREN1_SPI1		(1 << 11)
+#define SCTL_SCPEREN1_SPI2		(1 << 12)
+#define SCTL_SCPEREN1_I2C0		(1 << 13)
+#define SCTL_SCPEREN1_I2C1		(1 << 14)
+#define SCTL_SCPEREN1_I2C2		(1 << 15)
+#define SCTL_SCPEREN1_I2C3		(1 << 16)
+#define SCTL_SCPEREN1_USB_PHY		(1 << 17)
+
+#endif /* __STA2X11_MFD_H */
-- 
1.7.7.2

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH V2 2/2] gpio: add STA2X11 GPIO block
  2012-02-16 13:00 [PATCH V2 0/2] MFD and GPIO for STA2X11 Alessandro Rubini
  2012-02-16 13:00 ` [PATCH V2 1/2] mfd: Add driver for STA2X11 MFD block Alessandro Rubini
@ 2012-02-16 13:00 ` Alessandro Rubini
  2012-02-16 19:16   ` Linus Walleij
                     ` (6 more replies)
  1 sibling, 7 replies; 14+ messages in thread
From: Alessandro Rubini @ 2012-02-16 13:00 UTC (permalink / raw)
  To: linux-kernel; +Cc: giancarlo.asnaghi, alan, sameo, grant.likely, linus.walleij

This introduces 128 gpio bits (for each PCI device installed) with
working interrupt support.

Signed-off-by: Alessandro Rubini <rubini@gnudd.com>
Acked-by: Giancarlo Asnaghi <giancarlo.asnaghi@st.com>
Cc: Alan Cox <alan@linux.intel.com>
---
 drivers/gpio/Kconfig        |    9 +
 drivers/gpio/Makefile       |    1 +
 drivers/gpio/gpio-sta2x11.c |  443 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 453 insertions(+), 0 deletions(-)
 create mode 100644 drivers/gpio/gpio-sta2x11.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index dbb1909..dbae9c2 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -149,6 +149,14 @@ config GPIO_PXA
 	help
 	  Say yes here to support the PXA GPIO device
 
+config GPIO_STA2X11
+	bool "STA2x11/ConneXt GPIO support"
+	depends on MFD_STA2X11
+	select GENERIC_IRQ_CHIP
+	help
+	  Say yes here to support the STA2x11/ConneXt GPIO device.
+	  The GPIO module has 128 GPIO pins with alternate functions.
+
 config GPIO_XILINX
 	bool "Xilinx GPIO support"
 	depends on PPC_OF || MICROBLAZE
@@ -503,4 +511,5 @@ config GPIO_TPS65910
 	help
 	  Select this option to enable GPIO driver for the TPS65910
 	  chip family.
+
 endif
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 593bdcd..f72313d 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -47,6 +47,7 @@ obj-$(CONFIG_PLAT_SAMSUNG)	+= gpio-samsung.o
 obj-$(CONFIG_ARCH_SA1100)	+= gpio-sa1100.o
 obj-$(CONFIG_GPIO_SCH)		+= gpio-sch.o
 obj-$(CONFIG_GPIO_SODAVILLE)	+= gpio-sodaville.o
+obj-$(CONFIG_GPIO_STA2X11)	+= gpio-sta2x11.o
 obj-$(CONFIG_GPIO_STMPE)	+= gpio-stmpe.o
 obj-$(CONFIG_GPIO_SX150X)	+= gpio-sx150x.o
 obj-$(CONFIG_GPIO_TC3589X)	+= gpio-tc3589x.o
diff --git a/drivers/gpio/gpio-sta2x11.c b/drivers/gpio/gpio-sta2x11.c
new file mode 100644
index 0000000..f1885f4
--- /dev/null
+++ b/drivers/gpio/gpio-sta2x11.c
@@ -0,0 +1,443 @@
+/*
+ * STMicroelectronics ConneXt (STA2X11) GPIO driver
+ *
+ * Copyright 2012 ST Microelectronics (Alessandro Rubini)
+ * Based on gpio-ml-ioh.c, Copyright 2010 OKI Semiconductors Ltd.
+ * Also based on previous sta2x11 work, Copyright 2011 Wind River Systems, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * 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/module.h>
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/gpio.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/pci.h>
+#include <linux/platform_device.h>
+#include <linux/mfd/sta2x11-mfd.h>
+
+struct gsta_regs {
+	u32 dat;		/* 0x00 */
+	u32 dats;
+	u32 datc;
+	u32 pdis;
+	u32 dir;		/* 0x10 */
+	u32 dirs;
+	u32 dirc;
+	u32 unused_1c;
+	u32 afsela;		/* 0x20 */
+	u32 unused_24[7];
+	u32 rimsc;		/* 0x40 */
+	u32 fimsc;
+	u32 is;
+	u32 ic;
+};
+
+struct gsta_gpio {
+	spinlock_t		lock;
+	struct device		*dev;
+	void __iomem		*reg_base;
+	struct gsta_regs	*regs[GSTA_NR_BLOCKS];
+	struct gpio_chip	gpio;
+	int			irq_base;
+	/* FIXME: save the whole config here (AF, ...) */
+	unsigned		irq_type[GSTA_NR_GPIO];
+};
+
+static inline struct gsta_regs __iomem *__regs(struct gsta_gpio *chip, int nr)
+{
+	return chip->regs[nr / GSTA_GPIO_PER_BLOCK];
+}
+
+static inline u32 __bit(int nr)
+{
+	return 1U << (nr % GSTA_GPIO_PER_BLOCK);
+}
+
+/*
+ * gpio methods
+ */
+
+static void gsta_gpio_set(struct gpio_chip *gpio, unsigned nr, int val)
+{
+	struct gsta_gpio *chip = container_of(gpio, struct gsta_gpio, gpio);
+	struct gsta_regs __iomem *regs = __regs(chip, nr);
+	u32 bit = __bit(nr);
+
+	if (val)
+		writel(bit, &regs->dats);
+	else
+		writel(bit, &regs->datc);
+}
+
+static int gsta_gpio_get(struct gpio_chip *gpio, unsigned nr)
+{
+	struct gsta_gpio *chip = container_of(gpio, struct gsta_gpio, gpio);
+	struct gsta_regs __iomem *regs = __regs(chip, nr);
+	u32 bit = __bit(nr);
+
+	return readl(&regs->dat) & bit;
+}
+
+static int gsta_gpio_direction_output(struct gpio_chip *gpio, unsigned nr,
+				      int val)
+{
+	struct gsta_gpio *chip = container_of(gpio, struct gsta_gpio, gpio);
+	struct gsta_regs __iomem *regs = __regs(chip, nr);
+	u32 bit = __bit(nr);
+
+	writel(bit, &regs->dirs);
+	/* Data register after direction, otherwise pullup/down is selected */
+	if (val)
+		writel(bit, &regs->dats);
+	else
+		writel(bit, &regs->datc);
+	return 0;
+}
+
+static int gsta_gpio_direction_input(struct gpio_chip *gpio, unsigned nr)
+{
+	struct gsta_gpio *chip = container_of(gpio, struct gsta_gpio, gpio);
+	struct gsta_regs __iomem *regs = __regs(chip, nr);
+	u32 bit = __bit(nr);
+
+	writel(bit, &regs->dirc);
+	return 0;
+}
+
+static int gsta_gpio_to_irq(struct gpio_chip *gpio, unsigned offset)
+{
+	struct gsta_gpio *chip = container_of(gpio, struct gsta_gpio, gpio);
+	return chip->irq_base + offset;
+}
+
+static void gsta_gpio_setup(struct gsta_gpio *chip) /* called from probe */
+{
+	struct gpio_chip *gpio = &chip->gpio;
+
+	/*
+	 * ARCH_NR_GPIOS is currently 256 and dynamic allocation starts
+	 * from the end. However, for compatiility, we need the first
+	 * ConneXt device to start from gpio 0: it's the main chipset
+	 * on most boards so documents and drivers assume gpio0..gpio127
+	 */
+	static int gpio_base;
+
+	gpio->label = dev_name(chip->dev);
+	gpio->owner = THIS_MODULE;
+	gpio->direction_input = gsta_gpio_direction_input;
+	gpio->get = gsta_gpio_get;
+	gpio->direction_output = gsta_gpio_direction_output;
+	gpio->set = gsta_gpio_set;
+	gpio->dbg_show = NULL;
+	gpio->base = gpio_base;
+	gpio->base = 0;
+	gpio->ngpio = GSTA_NR_GPIO;
+	gpio->can_sleep = 0;
+	gpio->to_irq = gsta_gpio_to_irq;
+
+
+	/*
+	 * After the first device, turn to dynamic gpio numbers.
+	 * With ARCH_NR_GPIOS = 256 we can fit for example two steval cards
+	 */
+	if (!gpio_base)
+		gpio_base = 1;
+}
+
+/*
+ * Special method: alternate functions and pullup/pulldown. This will
+ * need to be exported to other drivers, adding a way to retrieve
+ * the gsta_gpio structure from their own pci device
+ */
+void gsta_set_config(struct gsta_gpio *chip, int nr, unsigned cfg)
+{
+	struct gsta_regs __iomem *regs = __regs(chip, nr);
+	unsigned long flags;
+	u32 bit = __bit(nr);
+	u32 val;
+	int err = 0;
+
+	printk("%s: %p %i %i\n", __func__, chip, nr, cfg);
+
+	if (cfg == PINMUX_TYPE_NONE)
+		return;
+
+	/* Alternate function or not? */
+	spin_lock_irqsave(&chip->lock, flags);
+	val = readl(&regs->afsela);
+	if (cfg == PINMUX_TYPE_FUNCTION)
+		val |= bit;
+	else
+		val &= ~bit;
+	writel(val | bit, &regs->afsela);
+	if (cfg == PINMUX_TYPE_FUNCTION) {
+		spin_unlock_irqrestore(&chip->lock, flags);
+		return;
+	}
+
+	/* not alternate function: set details */
+	switch(cfg) {
+	case PINMUX_TYPE_OUTPUT_LOW:
+		writel(bit, &regs->dirs);
+		writel(bit, &regs->datc);
+		break;
+	case PINMUX_TYPE_OUTPUT_HIGH:
+		writel(bit, &regs->dirs);
+		writel(bit, &regs->dats);
+		break;
+	case PINMUX_TYPE_INPUT:
+		writel(bit, &regs->dirc);
+		val = readl(&regs->pdis) | bit;
+		writel (val, &regs->pdis);
+		break;
+	case PINMUX_TYPE_INPUT_PULLUP:
+		writel(bit, &regs->dirc);
+		val = readl(&regs->pdis) &~ bit;
+		writel (val, &regs->pdis);
+		writel(bit, &regs->dats);
+		break;
+	case PINMUX_TYPE_INPUT_PULLDOWN:
+		writel(bit, &regs->dirc);
+		val = readl(&regs->pdis) &~ bit;
+		writel (val, &regs->pdis);
+		writel(bit, &regs->datc);
+		break;
+	default:
+		err = 1;
+	}
+	spin_unlock_irqrestore(&chip->lock, flags);
+	if (err)
+		pr_err("%s: chip %p, pin %i, cfg %i is invalid\n",
+		       __func__, chip, nr, cfg);
+}
+
+/*
+ * Irq methods
+ */
+
+static void gsta_irq_disable(struct irq_data *data)
+{
+	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(data);
+	struct gsta_gpio *chip = gc->private;
+	int nr = data->irq - chip->irq_base;
+	struct gsta_regs __iomem *regs = __regs(chip, nr);
+	u32 bit = __bit(nr);
+	u32 val;
+	unsigned long flags;
+
+	spin_lock_irqsave(&chip->lock, flags);
+	if (chip->irq_type[nr] & IRQ_TYPE_EDGE_RISING) {
+		val = readl(&regs->rimsc) & ~bit;
+		writel(val, &regs->rimsc);
+	}
+	if (chip->irq_type[nr] & IRQ_TYPE_EDGE_FALLING) {
+		val = readl(&regs->fimsc) & ~bit;
+		writel(val, &regs->fimsc);
+	}
+	spin_unlock_irqrestore(&chip->lock, flags);
+	return;
+}
+
+static void gsta_irq_enable(struct irq_data *data)
+{
+	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(data);
+	struct gsta_gpio *chip = gc->private;
+	int nr = data->irq - chip->irq_base;
+	struct gsta_regs __iomem *regs = __regs(chip, nr);
+	u32 bit = __bit(nr);
+	u32 val;
+	int type;
+	unsigned long flags;
+
+	type = chip->irq_type[nr];
+
+	spin_lock_irqsave(&chip->lock, flags);
+	val = readl(&regs->rimsc);
+	if (type & IRQ_TYPE_EDGE_RISING)
+		writel(val | bit, &regs->rimsc);
+	else
+		writel(val & ~bit, &regs->rimsc);
+	val = readl(&regs->rimsc);
+	if (type & IRQ_TYPE_EDGE_FALLING)
+		writel(val | bit, &regs->fimsc);
+	else
+		writel(val & ~bit, &regs->fimsc);
+	spin_unlock_irqrestore(&chip->lock, flags);
+	return;
+}
+
+static int gsta_irq_type(struct irq_data *d, unsigned int type)
+{
+	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+	struct gsta_gpio *chip = gc->private;
+	int nr = d->irq - chip->irq_base;
+
+	/* We only support edge interrupts */
+	if (!(type & (IRQ_TYPE_EDGE_RISING | IRQ_TYPE_EDGE_FALLING))) {
+		pr_debug("%s: unsupported type 0x%x\n", __func__, type);
+		return -EINVAL;
+	}
+
+	chip->irq_type[nr] = type; /* used for enable/disable */
+
+	gsta_irq_enable(d);
+	return 0;
+}
+
+static irqreturn_t gsta_gpio_handler(int irq, void *dev_id)
+{
+	struct gsta_gpio *chip = dev_id;
+	struct gsta_regs __iomem *regs;
+	u32 is;
+	int i, nr, base;
+	irqreturn_t ret = IRQ_NONE;
+
+	for (i = 0; i < GSTA_NR_BLOCKS; i++) {
+		regs = chip->regs[i];
+		base = chip->irq_base + i * GSTA_GPIO_PER_BLOCK;
+		while ((is = readl(&regs->is))) {
+			nr = __ffs(is);
+			irq = base + nr;
+			generic_handle_irq(irq);
+			writel(1 << nr, &regs->ic);
+			ret = IRQ_HANDLED;
+		}
+	}
+	return ret;
+}
+
+static __devinit void gsta_alloc_irq_chip(struct gsta_gpio *chip)
+{
+	struct irq_chip_generic *gc;
+	struct irq_chip_type *ct;
+
+	gc = irq_alloc_generic_chip(KBUILD_MODNAME, 1, chip->irq_base,
+				     chip->reg_base, handle_simple_irq);
+	gc->private = chip;
+	ct = gc->chip_types;
+
+	ct->chip.irq_set_type = gsta_irq_type;
+	ct->chip.irq_disable = gsta_irq_disable;
+	ct->chip.irq_enable = gsta_irq_enable;
+
+	/* FIXME: this makes at most 32 interrupts. Request 0 by now */
+	irq_setup_generic_chip(gc, 0 /* IRQ_MSK(GSTA_GPIO_PER_BLOCK) */, 0,
+			       IRQ_NOREQUEST | IRQ_NOPROBE, 0);
+
+	/* Set up all all 128 interrupts: code from setup_generic_chip */
+	{
+		struct irq_chip_type *ct = gc->chip_types;
+		int i, j;
+		for (j = 0; j < GSTA_NR_GPIO; j++) {
+			i = chip->irq_base + j;
+			irq_set_chip_and_handler(i, &ct->chip, ct->handler);
+			irq_set_chip_data(i, gc);
+			irq_modify_status(i, IRQ_NOREQUEST | IRQ_NOPROBE, 0);
+		}
+		gc->irq_cnt = i - gc->irq_base;
+	}
+}
+
+/* The platform device used here is instantiated by the MFD device */
+static int __devinit gsta_probe(struct platform_device *dev)
+{
+	int i, err;
+	struct pci_dev *pdev;
+	struct sta2x11_gpio_pdata *gpio_pdata;
+	struct gsta_gpio *chip;
+	struct resource *res;
+
+	pdev = *(struct pci_dev **)(dev->dev.platform_data);
+	gpio_pdata = dev_get_platdata(&pdev->dev);
+
+	if (gpio_pdata == NULL)
+		dev_err(&dev->dev, "no gpio config\n");
+	printk("gpio config: %p\n", gpio_pdata);
+
+	res = platform_get_resource(dev, IORESOURCE_MEM, 0);
+	request_mem_region(res->start, resource_size(res), KBUILD_MODNAME);
+
+
+	chip = kzalloc(sizeof(*chip), GFP_KERNEL);
+	chip->dev = &dev->dev;
+	chip->reg_base = ioremap(res->start, resource_size(res));
+	chip->irq_base = -1;
+	for (i = 0; i < GSTA_NR_BLOCKS; i++) {
+		chip->regs[i] = chip->reg_base + i * 4096;
+		/* disable all irqs */
+		writel(0, &chip->regs[i]->rimsc);
+		writel(0, &chip->regs[i]->fimsc);
+		writel(~0, &chip->regs[i]->ic);
+	}
+	spin_lock_init(&chip->lock);
+	gsta_gpio_setup(chip);
+	for (i = 0; i < GSTA_NR_GPIO; i++)
+		     gsta_set_config(chip, i, gpio_pdata->pinconfig[i]);
+	err = gpiochip_add(&chip->gpio);
+	if (err < 0) {
+		dev_err(&dev->dev, "sta2x11 gpio: Can't register (%i)\n",
+			-err);
+		kfree(chip);
+		return err;
+	}
+	/* 384 was used in previous code: be compatible for other drivers */
+	err = irq_alloc_descs(-1, 384, GSTA_NR_GPIO, NUMA_NO_NODE);
+
+	if (err < 0) {
+		dev_warn(&dev->dev, "sta2x11 gpio: Can't get irq base (%i)\n",
+			 -err);
+		goto err_out;
+	}
+	chip->irq_base = err;
+	gsta_alloc_irq_chip(chip);
+
+	err = request_irq(pdev->irq, gsta_gpio_handler,
+			     IRQF_SHARED, KBUILD_MODNAME, chip);
+	if (err < 0) {
+		dev_err(&dev->dev, "sta2x11 gpio: Can't request irq (%i)\n",
+			-err);
+		goto err_out;
+	}
+
+	platform_set_drvdata(dev, chip);
+	return 0;
+
+err_out:
+	if (chip->irq_base > 0)
+		irq_free_descs(chip->irq_base, GSTA_NR_GPIO);
+	kfree(chip);
+	return err;
+}
+
+static struct platform_driver sta2x11_gpio_platform_driver = {
+	.driver = {
+		.name	= "sta2x11-gpio",
+		.owner	= THIS_MODULE,
+	},
+	.probe = gsta_probe,
+};
+
+static int __init sta2x11_gpio_init(void)
+{
+	return platform_driver_register(&sta2x11_gpio_platform_driver);
+}
+
+module_init(sta2x11_gpio_init);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("sta2x11_gpio GPIO driver");
-- 
1.7.7.2

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH V2 2/2] gpio: add STA2X11 GPIO block
  2012-02-16 13:00 ` [PATCH V2 2/2] gpio: add STA2X11 GPIO block Alessandro Rubini
@ 2012-02-16 19:16   ` Linus Walleij
  2012-02-16 19:24   ` Linus Walleij
                     ` (5 subsequent siblings)
  6 siblings, 0 replies; 14+ messages in thread
From: Linus Walleij @ 2012-02-16 19:16 UTC (permalink / raw)
  To: Alessandro Rubini
  Cc: linux-kernel, giancarlo.asnaghi, alan, sameo, grant.likely,
	linus.walleij

On Thu, Feb 16, 2012 at 2:00 PM, Alessandro Rubini <rubini@gnudd.com> wrote:

> +/*
> + * Special method: alternate functions and pullup/pulldown. This will
> + * need to be exported to other drivers, adding a way to retrieve
> + * the gsta_gpio structure from their own pci device
> + */
> +void gsta_set_config(struct gsta_gpio *chip, int nr, unsigned cfg)
> +{
(...)
> +       if (cfg == PINMUX_TYPE_NONE)
(...)
> +       if (cfg == PINMUX_TYPE_FUNCTION)
(...)
> +       if (cfg == PINMUX_TYPE_FUNCTION) {
(...)
> +       case PINMUX_TYPE_OUTPUT_LOW:
(...)
> +       case PINMUX_TYPE_OUTPUT_HIGH:
(...)
> +       case PINMUX_TYPE_INPUT:
(...)
> +       case PINMUX_TYPE_INPUT_PULLUP:
(...)
> +       case PINMUX_TYPE_INPUT_PULLDOWN:

We have created the pin control subsystem to handle things like
this. (Muxing and complex control.)

Please create a driver in drivers/pinctrl/pinctrl-sta2x11.c that
expose both a pinctrl and a GPIOlib interface. The GPIOlib
interface can call into the pin control portions just fine.

We've had some refactoring in the pin control subsystem recently
so check linux-next for the latest API. The documentation in
Documentation/pinctrl.txt should be pretty uptodate, else
tell me!

I'm giving a talk about pin control at ELC tomorrow, mail
me if you want a copy of the slides!

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH V2 2/2] gpio: add STA2X11 GPIO block
  2012-02-16 13:00 ` [PATCH V2 2/2] gpio: add STA2X11 GPIO block Alessandro Rubini
  2012-02-16 19:16   ` Linus Walleij
@ 2012-02-16 19:24   ` Linus Walleij
  2012-02-16 20:24   ` Alessandro Rubini
                     ` (4 subsequent siblings)
  6 siblings, 0 replies; 14+ messages in thread
From: Linus Walleij @ 2012-02-16 19:24 UTC (permalink / raw)
  To: Alessandro Rubini
  Cc: linux-kernel, giancarlo.asnaghi, alan, sameo, grant.likely,
	linus.walleij

On Thu, Feb 16, 2012 at 2:00 PM, Alessandro Rubini <rubini@gnudd.com> wrote:

> +static void gsta_gpio_setup(struct gsta_gpio *chip) /* called from probe */
> +{
> +       struct gpio_chip *gpio = &chip->gpio;
> +
> +       /*
> +        * ARCH_NR_GPIOS is currently 256 and dynamic allocation starts
> +        * from the end. However, for compatiility, we need the first

[Nitpick: spelling]

> +        * ConneXt device to start from gpio 0: it's the main chipset
> +        * on most boards so documents and drivers assume gpio0..gpio127
> +        */
> +       static int gpio_base;
> +
> +       gpio->label = dev_name(chip->dev);
> +       gpio->owner = THIS_MODULE;
> +       gpio->direction_input = gsta_gpio_direction_input;
> +       gpio->get = gsta_gpio_get;
> +       gpio->direction_output = gsta_gpio_direction_output;
> +       gpio->set = gsta_gpio_set;
> +       gpio->dbg_show = NULL;
> +       gpio->base = gpio_base;
> +       gpio->base = 0;

What are you doing here?

It seems like you first use the provided base, then hard code it
to zero.

The GPIO pin number space is global and you need to be able to
handle the case where several controllers of this kind are plugged
in, will you not?

This looks like it assumes you only ever have one card on the
system.

Make sure that the global GPIO numberspace is properly
handled on these systems, alas I am a bit worried that this
may not be that very easy.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH V2 1/2] mfd: Add driver for STA2X11 MFD block
  2012-02-16 13:00 ` [PATCH V2 1/2] mfd: Add driver for STA2X11 MFD block Alessandro Rubini
@ 2012-02-16 19:29   ` Linus Walleij
  2012-02-23 16:19   ` Samuel Ortiz
  2012-02-23 16:25   ` Alessandro Rubini
  2 siblings, 0 replies; 14+ messages in thread
From: Linus Walleij @ 2012-02-16 19:29 UTC (permalink / raw)
  To: Alessandro Rubini
  Cc: linux-kernel, giancarlo.asnaghi, alan, sameo, grant.likely,
	linus.walleij

On Thu, Feb 16, 2012 at 2:00 PM, Alessandro Rubini <rubini@gnudd.com> wrote:

> +config MFD_STA2X11
> +       bool "STA2X11 multi function device support"
> +       depends on STA2X11
> +       select MFD_CORE
> +       select GPIO_STA2X11

Note: this selects a non-existent driver at this point. (OK no big deal.)

> +/* Give names to GPIO pins, like PXA does, taken from the manual */
> +#define GPIO0                  0
> +#define GPIO1                  1
> +#define GPIO2                  2
(...)

These are too general names I think. It should be clear that this is the
GPIO number relative to this one controller, so I would prefix them
like STA2X11_GPIO0 etc.

Overall the MFD core for STA2X11 is looking pretty good!

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH V2 2/2] gpio: add STA2X11 GPIO block
  2012-02-16 13:00 ` [PATCH V2 2/2] gpio: add STA2X11 GPIO block Alessandro Rubini
  2012-02-16 19:16   ` Linus Walleij
  2012-02-16 19:24   ` Linus Walleij
@ 2012-02-16 20:24   ` Alessandro Rubini
  2012-02-16 20:24   ` Alessandro Rubini
                     ` (3 subsequent siblings)
  6 siblings, 0 replies; 14+ messages in thread
From: Alessandro Rubini @ 2012-02-16 20:24 UTC (permalink / raw)
  To: linus.walleij; +Cc: linux-kernel, giancarlo.asnaghi, alan, sameo, grant.likely

Thank Linus for this and the others.

> [Nitpick: spelling]

ack

>> +       gpio->base = gpio_base;
>> +       gpio->base = 0;
> 
> What are you doing here?

An error. Sorry. the latter line must be removed. I'll resend.

> The GPIO pin number space is global and you need to be able to
> handle the case where several controllers of this kind are plugged
> in, will you not?

Yes, I'm pretty careful about this, and it's one of the concerns I
have with the code I got. Unfortunately I only have 1 card plugged in
a PC (and one standalone system where the sta2x11 is the main chipset
and no more chan be plugged).  Here I simply made a mistake.

> Make sure that the global GPIO numberspace is properly
> handled on these systems, alas I am a bit worried that this
> may not be that very easy.

No, it was not. But, despite tha extra line from earlier code, it should
work properly with this code layout.

/alessandro

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH V2 2/2] gpio: add STA2X11 GPIO block
  2012-02-16 13:00 ` [PATCH V2 2/2] gpio: add STA2X11 GPIO block Alessandro Rubini
                     ` (2 preceding siblings ...)
  2012-02-16 20:24   ` Alessandro Rubini
@ 2012-02-16 20:24   ` Alessandro Rubini
  2012-03-01 18:50   ` Alessandro Rubini
                     ` (2 subsequent siblings)
  6 siblings, 0 replies; 14+ messages in thread
From: Alessandro Rubini @ 2012-02-16 20:24 UTC (permalink / raw)
  To: linus.walleij; +Cc: linux-kernel, giancarlo.asnaghi, alan, sameo, grant.likely

> We have created the pin control subsystem to handle things like
> this. (Muxing and complex control.)

Ok, I'll use it.

Thanks
/alessandro

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH V2 1/2] mfd: Add driver for STA2X11 MFD block
  2012-02-16 13:00 ` [PATCH V2 1/2] mfd: Add driver for STA2X11 MFD block Alessandro Rubini
  2012-02-16 19:29   ` Linus Walleij
@ 2012-02-23 16:19   ` Samuel Ortiz
  2012-02-23 16:25   ` Alessandro Rubini
  2 siblings, 0 replies; 14+ messages in thread
From: Samuel Ortiz @ 2012-02-23 16:19 UTC (permalink / raw)
  To: Alessandro Rubini
  Cc: linux-kernel, giancarlo.asnaghi, alan, grant.likely, linus.walleij

Hi Alessandro,

A few minor comments, since it seems you're going to re-spin this one:

On Thu, Feb 16, 2012 at 02:00:40PM +0100, Alessandro Rubini wrote:
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -830,6 +830,12 @@ config MFD_INTEL_MSIC
>  	  Passage) chip. This chip embeds audio, battery, GPIO, etc.
>  	  devices used in Intel Medfield platforms.
>  
> +config MFD_STA2X11
> +	bool "STA2X11 multi function device support"
> +	depends on STA2X11
> +	select MFD_CORE
> +	select GPIO_STA2X11
I think it's better to have your GPIO driver depend on MFD_STA2X11. With the
above code, you could end up trying to build your GPIO driver with GPIOLIB not
being selected.


> +/*
> + * What follows is the PCI device that hosts the above two pdevs.
> + * Each logic block is 4kB and they are all consecutive: we use this info.
> + */
> +
> +/* Bar 0 */
> +enum bar0_cells {
> +	GPIO_0 = 0,
> +	GPIO_1,
> +	GPIO_2,
> +	GPIO_3,
> +	SCTL,
> +	SCR,
> +	TIME,
As Linus said, I'd protect that with a namespace.


> +static int __devinit sta2x11_mfd_probe(struct pci_dev *pdev,
> +				       const struct pci_device_id *pci_id)
> +{
> +	int err, i;
> +	struct sta2x11_gpio_pdata *gpio_data;
> +
> +	dev_info(&pdev->dev, "%s\n", __func__);
> +
> +	err = pci_enable_device(pdev);
> +	if (err) {
> +		dev_err(&pdev->dev, "Can't enable device.\n");
> +		return err;
> +	}
> +
> +	err = pci_enable_msi(pdev);
> +	if (err)
> +		dev_info(&pdev->dev, "Enable msi failed\n");
> +
> +	/* Read gpio config data as pci device's platform data */
> +	gpio_data = dev_get_platdata(&pdev->dev);
> +	if (!gpio_data)
> +		dev_warn(&pdev->dev, "no gpio configuration\n");
> +
> +#if 1
> +	dev_dbg(&pdev->dev, "%s, gpio_data = %p (%p)\n", __func__,
> +		gpio_data, &gpio_data);
> +#endif
You can get rid of the #if #endif here.


> +	dev_dbg(&pdev->dev, "%s, pdev = %p (%p)\n", __func__,
> +		pdev, &pdev);
> +
Extra line here


> +	/* platform data is the pci device for all of them */
> +	for (i = 0; i < ARRAY_SIZE(sta2x11_mfd_bar0); i++) {
> +		sta2x11_mfd_bar0[i].pdata_size = sizeof(pdev);
> +		sta2x11_mfd_bar0[i].platform_data = &pdev;
> +	}
> +	sta2x11_mfd_bar1[0].pdata_size = sizeof(pdev);
> +	sta2x11_mfd_bar1[0].platform_data = &pdev;
> +
> +	/* Record this pdev before mfd_add_devices: their probe looks for it */
> +	sta2x11_mfd_add(pdev, GFP_ATOMIC);
> +
> +
> +	err = mfd_add_devices(&pdev->dev, -1,
> +			      sta2x11_mfd_bar0,
> +			      ARRAY_SIZE(sta2x11_mfd_bar0),
> +			      &pdev->resource[0],
> +			      0);
> +	if (err) {
> +		dev_err(&pdev->dev, "mfd_add_devices[0] failed: %d\n", err);
> +		goto err_disable;
> +	}
> +
> +	err = mfd_add_devices(&pdev->dev, -1,
> +			      sta2x11_mfd_bar1,
> +			      ARRAY_SIZE(sta2x11_mfd_bar1),
> +			      &pdev->resource[1],
> +			      0);
> +	if (err) {
> +		dev_err(&pdev->dev, "mfd_add_devices[1] failed: %d\n", err);
> +		goto err_disable;
> +	}
> +
> +	return 0;
> +
> +err_disable:
> +	pci_disable_device(pdev);
> +	pci_disable_msi(pdev);
> +	/* mfd_remove(pdev); -- it is in an exit section, I can't call it */
Why can't you call mfd_remove_devices() here ?

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH V2 1/2] mfd: Add driver for STA2X11 MFD block
  2012-02-16 13:00 ` [PATCH V2 1/2] mfd: Add driver for STA2X11 MFD block Alessandro Rubini
  2012-02-16 19:29   ` Linus Walleij
  2012-02-23 16:19   ` Samuel Ortiz
@ 2012-02-23 16:25   ` Alessandro Rubini
  2 siblings, 0 replies; 14+ messages in thread
From: Alessandro Rubini @ 2012-02-23 16:25 UTC (permalink / raw)
  To: sameo; +Cc: linux-kernel, giancarlo.asnaghi, alan, grant.likely, linus.walleij

> A few minor comments, since it seems you're going to re-spin this one:

Thanks. Will do. The "#if 1" is clearly an oversight.
I should be able to resping it over the weekeend.

/alessandro

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH V2 2/2] gpio: add STA2X11 GPIO block
  2012-02-16 13:00 ` [PATCH V2 2/2] gpio: add STA2X11 GPIO block Alessandro Rubini
                     ` (3 preceding siblings ...)
  2012-02-16 20:24   ` Alessandro Rubini
@ 2012-03-01 18:50   ` Alessandro Rubini
  2012-03-01 20:20     ` Linus Walleij
  2012-03-02  7:48   ` Grant Likely
  2012-03-02 10:00   ` Alessandro Rubini
  6 siblings, 1 reply; 14+ messages in thread
From: Alessandro Rubini @ 2012-03-01 18:50 UTC (permalink / raw)
  To: linus.walleij
  Cc: linux-kernel, giancarlo.asnaghi, alan, sameo, grant.likely,
	linus.walleij

>> +       if (cfg == PINMUX_TYPE_FUNCTION) {
> (...)
>> +       case PINMUX_TYPE_OUTPUT_LOW:
> (...)
>> +       case PINMUX_TYPE_OUTPUT_HIGH:
> (...)
>> +       case PINMUX_TYPE_INPUT:
> (...)
>> +       case PINMUX_TYPE_INPUT_PULLUP:
> (...)
>> +       case PINMUX_TYPE_INPUT_PULLDOWN:
> 
> We have created the pin control subsystem to handle things like
> this. (Muxing and complex control.)
> 
> Please create a driver in drivers/pinctrl/pinctrl-sta2x11.c that
> expose both a pinctrl and a GPIOlib interface. The GPIOlib
> interface can call into the pin control portions just fine.

I'm sorry, after digging in it for a while I'm really lost.  This
pinctrl file should be there _instead_ of the gpio file or in addition
to it?  I see tegra has both, but u300 has only one.

Also, this pinctrl depends on CONFIG_EXPERIMENTAL at this point in time.
Is it wise the make the whole sta2x11 thing depend on experimental?

Thanks
/alessandro

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH V2 2/2] gpio: add STA2X11 GPIO block
  2012-03-01 18:50   ` Alessandro Rubini
@ 2012-03-01 20:20     ` Linus Walleij
  0 siblings, 0 replies; 14+ messages in thread
From: Linus Walleij @ 2012-03-01 20:20 UTC (permalink / raw)
  To: Alessandro Rubini
  Cc: linux-kernel, giancarlo.asnaghi, alan, sameo, grant.likely,
	linus.walleij

On Thu, Mar 1, 2012 at 7:50 PM, Alessandro Rubini <rubini@gnudd.com> wrote:

>>> +       if (cfg == PINMUX_TYPE_FUNCTION) {
>> (...)
>>> +       case PINMUX_TYPE_OUTPUT_LOW:
>> (...)
>>> +       case PINMUX_TYPE_OUTPUT_HIGH:
>> (...)
>>> +       case PINMUX_TYPE_INPUT:
>> (...)
>>> +       case PINMUX_TYPE_INPUT_PULLUP:
>> (...)
>>> +       case PINMUX_TYPE_INPUT_PULLDOWN:
>>
>> We have created the pin control subsystem to handle things like
>> this. (Muxing and complex control.)
>>
>> Please create a driver in drivers/pinctrl/pinctrl-sta2x11.c that
>> expose both a pinctrl and a GPIOlib interface. The GPIOlib
>> interface can call into the pin control portions just fine.
>
> I'm sorry, after digging in it for a while I'm really lost.  This
> pinctrl file should be there _instead_ of the gpio file or in addition
> to it?  I see tegra has both, but u300 has only one.

Instead. Just expose three interfaces from it:

struct gpio_chip
struct irc_chip (iff it supports IRQs)
struct pinctrl_desc

> Also, this pinctrl depends on CONFIG_EXPERIMENTAL at this point in time.
> Is it wise the make the whole sta2x11 thing depend on experimental?

Another option is to wait for it to be marked non-experimental
and help fixing it up I guess, we've created it due to
shortcomings in drivers/gpio...

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH V2 2/2] gpio: add STA2X11 GPIO block
  2012-02-16 13:00 ` [PATCH V2 2/2] gpio: add STA2X11 GPIO block Alessandro Rubini
                     ` (4 preceding siblings ...)
  2012-03-01 18:50   ` Alessandro Rubini
@ 2012-03-02  7:48   ` Grant Likely
  2012-03-02 10:00   ` Alessandro Rubini
  6 siblings, 0 replies; 14+ messages in thread
From: Grant Likely @ 2012-03-02  7:48 UTC (permalink / raw)
  To: Alessandro Rubini, linux-kernel
  Cc: giancarlo.asnaghi, alan, sameo, linus.walleij

On Thu, 16 Feb 2012 14:00:52 +0100, Alessandro Rubini <rubini@gnudd.com> wrote:
> This introduces 128 gpio bits (for each PCI device installed) with
> working interrupt support.
> 
> Signed-off-by: Alessandro Rubini <rubini@gnudd.com>
> Acked-by: Giancarlo Asnaghi <giancarlo.asnaghi@st.com>
> Cc: Alan Cox <alan@linux.intel.com>
> ---
>  drivers/gpio/Kconfig        |    9 +
>  drivers/gpio/Makefile       |    1 +
>  drivers/gpio/gpio-sta2x11.c |  443 +++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 453 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/gpio/gpio-sta2x11.c
> 
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index dbb1909..dbae9c2 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -149,6 +149,14 @@ config GPIO_PXA
>  	help
>  	  Say yes here to support the PXA GPIO device
>  
> +config GPIO_STA2X11
> +	bool "STA2x11/ConneXt GPIO support"
> +	depends on MFD_STA2X11
> +	select GENERIC_IRQ_CHIP
> +	help
> +	  Say yes here to support the STA2x11/ConneXt GPIO device.
> +	  The GPIO module has 128 GPIO pins with alternate functions.
> +
>  config GPIO_XILINX
>  	bool "Xilinx GPIO support"
>  	depends on PPC_OF || MICROBLAZE
> @@ -503,4 +511,5 @@ config GPIO_TPS65910
>  	help
>  	  Select this option to enable GPIO driver for the TPS65910
>  	  chip family.
> +
>  endif

Unrelated whitespace change; please remove from patch

> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
> index 593bdcd..f72313d 100644
> --- a/drivers/gpio/Makefile
> +++ b/drivers/gpio/Makefile
> @@ -47,6 +47,7 @@ obj-$(CONFIG_PLAT_SAMSUNG)	+= gpio-samsung.o
>  obj-$(CONFIG_ARCH_SA1100)	+= gpio-sa1100.o
>  obj-$(CONFIG_GPIO_SCH)		+= gpio-sch.o
>  obj-$(CONFIG_GPIO_SODAVILLE)	+= gpio-sodaville.o
> +obj-$(CONFIG_GPIO_STA2X11)	+= gpio-sta2x11.o
>  obj-$(CONFIG_GPIO_STMPE)	+= gpio-stmpe.o
>  obj-$(CONFIG_GPIO_SX150X)	+= gpio-sx150x.o
>  obj-$(CONFIG_GPIO_TC3589X)	+= gpio-tc3589x.o
> diff --git a/drivers/gpio/gpio-sta2x11.c b/drivers/gpio/gpio-sta2x11.c
> new file mode 100644
> index 0000000..f1885f4
> --- /dev/null
> +++ b/drivers/gpio/gpio-sta2x11.c
> @@ -0,0 +1,443 @@
> +/*
> + * STMicroelectronics ConneXt (STA2X11) GPIO driver
> + *
> + * Copyright 2012 ST Microelectronics (Alessandro Rubini)
> + * Based on gpio-ml-ioh.c, Copyright 2010 OKI Semiconductors Ltd.
> + * Also based on previous sta2x11 work, Copyright 2011 Wind River Systems, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * 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/module.h>
> +#include <linux/kernel.h>
> +#include <linux/slab.h>
> +#include <linux/gpio.h>
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
> +#include <linux/pci.h>
> +#include <linux/platform_device.h>
> +#include <linux/mfd/sta2x11-mfd.h>
> +
> +struct gsta_regs {
> +	u32 dat;		/* 0x00 */
> +	u32 dats;
> +	u32 datc;
> +	u32 pdis;
> +	u32 dir;		/* 0x10 */
> +	u32 dirs;
> +	u32 dirc;
> +	u32 unused_1c;
> +	u32 afsela;		/* 0x20 */
> +	u32 unused_24[7];
> +	u32 rimsc;		/* 0x40 */
> +	u32 fimsc;
> +	u32 is;
> +	u32 ic;
> +};
> +
> +struct gsta_gpio {
> +	spinlock_t		lock;
> +	struct device		*dev;
> +	void __iomem		*reg_base;
> +	struct gsta_regs	*regs[GSTA_NR_BLOCKS];

__iomem?

> +	struct gpio_chip	gpio;
> +	int			irq_base;
> +	/* FIXME: save the whole config here (AF, ...) */
> +	unsigned		irq_type[GSTA_NR_GPIO];
> +};
> +
> +static inline struct gsta_regs __iomem *__regs(struct gsta_gpio *chip, int nr)
> +{
> +	return chip->regs[nr / GSTA_GPIO_PER_BLOCK];
> +}
> +
> +static inline u32 __bit(int nr)
> +{
> +	return 1U << (nr % GSTA_GPIO_PER_BLOCK);
> +}
> +
> +/*
> + * gpio methods
> + */
> +
> +static void gsta_gpio_set(struct gpio_chip *gpio, unsigned nr, int val)
> +{
> +	struct gsta_gpio *chip = container_of(gpio, struct gsta_gpio, gpio);
> +	struct gsta_regs __iomem *regs = __regs(chip, nr);
> +	u32 bit = __bit(nr);
> +
> +	if (val)
> +		writel(bit, &regs->dats);
> +	else
> +		writel(bit, &regs->datc);
> +}
> +
> +static int gsta_gpio_get(struct gpio_chip *gpio, unsigned nr)
> +{
> +	struct gsta_gpio *chip = container_of(gpio, struct gsta_gpio, gpio);
> +	struct gsta_regs __iomem *regs = __regs(chip, nr);
> +	u32 bit = __bit(nr);
> +
> +	return readl(&regs->dat) & bit;
> +}
> +
> +static int gsta_gpio_direction_output(struct gpio_chip *gpio, unsigned nr,
> +				      int val)
> +{
> +	struct gsta_gpio *chip = container_of(gpio, struct gsta_gpio, gpio);
> +	struct gsta_regs __iomem *regs = __regs(chip, nr);
> +	u32 bit = __bit(nr);
> +
> +	writel(bit, &regs->dirs);
> +	/* Data register after direction, otherwise pullup/down is selected */
> +	if (val)
> +		writel(bit, &regs->dats);
> +	else
> +		writel(bit, &regs->datc);
> +	return 0;
> +}
> +
> +static int gsta_gpio_direction_input(struct gpio_chip *gpio, unsigned nr)
> +{
> +	struct gsta_gpio *chip = container_of(gpio, struct gsta_gpio, gpio);
> +	struct gsta_regs __iomem *regs = __regs(chip, nr);
> +	u32 bit = __bit(nr);
> +
> +	writel(bit, &regs->dirc);
> +	return 0;

These are plain-vanilla gpio accessors.  Can you use the gpio-generic.c
library?

> +}
> +
> +static int gsta_gpio_to_irq(struct gpio_chip *gpio, unsigned offset)
> +{
> +	struct gsta_gpio *chip = container_of(gpio, struct gsta_gpio, gpio);
> +	return chip->irq_base + offset;
> +}
> +
> +static void gsta_gpio_setup(struct gsta_gpio *chip) /* called from probe */
> +{
> +	struct gpio_chip *gpio = &chip->gpio;
> +
> +	/*
> +	 * ARCH_NR_GPIOS is currently 256 and dynamic allocation starts
> +	 * from the end. However, for compatiility, we need the first
> +	 * ConneXt device to start from gpio 0: it's the main chipset
> +	 * on most boards so documents and drivers assume gpio0..gpio127
> +	 */
> +	static int gpio_base;
> +
> +	gpio->label = dev_name(chip->dev);
> +	gpio->owner = THIS_MODULE;
> +	gpio->direction_input = gsta_gpio_direction_input;
> +	gpio->get = gsta_gpio_get;
> +	gpio->direction_output = gsta_gpio_direction_output;
> +	gpio->set = gsta_gpio_set;
> +	gpio->dbg_show = NULL;
> +	gpio->base = gpio_base;
> +	gpio->base = 0;
> +	gpio->ngpio = GSTA_NR_GPIO;
> +	gpio->can_sleep = 0;
> +	gpio->to_irq = gsta_gpio_to_irq;
> +
> +
> +	/*
> +	 * After the first device, turn to dynamic gpio numbers.
> +	 * With ARCH_NR_GPIOS = 256 we can fit for example two steval cards
> +	 */
> +	if (!gpio_base)
> +		gpio_base = 1;
> +}
> +
> +/*
> + * Special method: alternate functions and pullup/pulldown. This will
> + * need to be exported to other drivers, adding a way to retrieve
> + * the gsta_gpio structure from their own pci device
> + */
> +void gsta_set_config(struct gsta_gpio *chip, int nr, unsigned cfg)
> +{
> +	struct gsta_regs __iomem *regs = __regs(chip, nr);
> +	unsigned long flags;
> +	u32 bit = __bit(nr);
> +	u32 val;
> +	int err = 0;
> +
> +	printk("%s: %p %i %i\n", __func__, chip, nr, cfg);

pr_info()

> +
> +	if (cfg == PINMUX_TYPE_NONE)
> +		return;
> +
> +	/* Alternate function or not? */
> +	spin_lock_irqsave(&chip->lock, flags);
> +	val = readl(&regs->afsela);
> +	if (cfg == PINMUX_TYPE_FUNCTION)
> +		val |= bit;
> +	else
> +		val &= ~bit;
> +	writel(val | bit, &regs->afsela);
> +	if (cfg == PINMUX_TYPE_FUNCTION) {
> +		spin_unlock_irqrestore(&chip->lock, flags);
> +		return;
> +	}
> +
> +	/* not alternate function: set details */
> +	switch(cfg) {
> +	case PINMUX_TYPE_OUTPUT_LOW:
> +		writel(bit, &regs->dirs);
> +		writel(bit, &regs->datc);
> +		break;
> +	case PINMUX_TYPE_OUTPUT_HIGH:
> +		writel(bit, &regs->dirs);
> +		writel(bit, &regs->dats);
> +		break;
> +	case PINMUX_TYPE_INPUT:
> +		writel(bit, &regs->dirc);
> +		val = readl(&regs->pdis) | bit;
> +		writel (val, &regs->pdis);
> +		break;
> +	case PINMUX_TYPE_INPUT_PULLUP:
> +		writel(bit, &regs->dirc);
> +		val = readl(&regs->pdis) &~ bit;
> +		writel (val, &regs->pdis);
> +		writel(bit, &regs->dats);
> +		break;
> +	case PINMUX_TYPE_INPUT_PULLDOWN:
> +		writel(bit, &regs->dirc);
> +		val = readl(&regs->pdis) &~ bit;
> +		writel (val, &regs->pdis);
> +		writel(bit, &regs->datc);
> +		break;
> +	default:
> +		err = 1;
> +	}
> +	spin_unlock_irqrestore(&chip->lock, flags);
> +	if (err)
> +		pr_err("%s: chip %p, pin %i, cfg %i is invalid\n",
> +		       __func__, chip, nr, cfg);
> +}
> +
> +/*
> + * Irq methods
> + */
> +
> +static void gsta_irq_disable(struct irq_data *data)
> +{
> +	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(data);
> +	struct gsta_gpio *chip = gc->private;
> +	int nr = data->irq - chip->irq_base;

we have data->hwirq now.  Use it.  In fact, you should look at using irqdomain to manage the
hwirq to linuxirq number mapping for you.  irqdomain will be merged for v3.4.

> +	struct gsta_regs __iomem *regs = __regs(chip, nr);
> +	u32 bit = __bit(nr);
> +	u32 val;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&chip->lock, flags);
> +	if (chip->irq_type[nr] & IRQ_TYPE_EDGE_RISING) {
> +		val = readl(&regs->rimsc) & ~bit;
> +		writel(val, &regs->rimsc);
> +	}
> +	if (chip->irq_type[nr] & IRQ_TYPE_EDGE_FALLING) {
> +		val = readl(&regs->fimsc) & ~bit;
> +		writel(val, &regs->fimsc);
> +	}
> +	spin_unlock_irqrestore(&chip->lock, flags);
> +	return;
> +}
> +
> +static void gsta_irq_enable(struct irq_data *data)
> +{
> +	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(data);
> +	struct gsta_gpio *chip = gc->private;
> +	int nr = data->irq - chip->irq_base;
> +	struct gsta_regs __iomem *regs = __regs(chip, nr);
> +	u32 bit = __bit(nr);
> +	u32 val;
> +	int type;
> +	unsigned long flags;
> +
> +	type = chip->irq_type[nr];
> +
> +	spin_lock_irqsave(&chip->lock, flags);
> +	val = readl(&regs->rimsc);
> +	if (type & IRQ_TYPE_EDGE_RISING)
> +		writel(val | bit, &regs->rimsc);
> +	else
> +		writel(val & ~bit, &regs->rimsc);
> +	val = readl(&regs->rimsc);
> +	if (type & IRQ_TYPE_EDGE_FALLING)
> +		writel(val | bit, &regs->fimsc);
> +	else
> +		writel(val & ~bit, &regs->fimsc);
> +	spin_unlock_irqrestore(&chip->lock, flags);
> +	return;
> +}
> +
> +static int gsta_irq_type(struct irq_data *d, unsigned int type)
> +{
> +	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
> +	struct gsta_gpio *chip = gc->private;
> +	int nr = d->irq - chip->irq_base;
> +
> +	/* We only support edge interrupts */
> +	if (!(type & (IRQ_TYPE_EDGE_RISING | IRQ_TYPE_EDGE_FALLING))) {
> +		pr_debug("%s: unsupported type 0x%x\n", __func__, type);
> +		return -EINVAL;
> +	}
> +
> +	chip->irq_type[nr] = type; /* used for enable/disable */
> +
> +	gsta_irq_enable(d);
> +	return 0;
> +}
> +
> +static irqreturn_t gsta_gpio_handler(int irq, void *dev_id)
> +{
> +	struct gsta_gpio *chip = dev_id;
> +	struct gsta_regs __iomem *regs;
> +	u32 is;
> +	int i, nr, base;
> +	irqreturn_t ret = IRQ_NONE;
> +
> +	for (i = 0; i < GSTA_NR_BLOCKS; i++) {
> +		regs = chip->regs[i];
> +		base = chip->irq_base + i * GSTA_GPIO_PER_BLOCK;
> +		while ((is = readl(&regs->is))) {
> +			nr = __ffs(is);
> +			irq = base + nr;
> +			generic_handle_irq(irq);
> +			writel(1 << nr, &regs->ic);
> +			ret = IRQ_HANDLED;
> +		}
> +	}
> +	return ret;
> +}
> +
> +static __devinit void gsta_alloc_irq_chip(struct gsta_gpio *chip)
> +{
> +	struct irq_chip_generic *gc;
> +	struct irq_chip_type *ct;
> +
> +	gc = irq_alloc_generic_chip(KBUILD_MODNAME, 1, chip->irq_base,
> +				     chip->reg_base, handle_simple_irq);
> +	gc->private = chip;
> +	ct = gc->chip_types;
> +
> +	ct->chip.irq_set_type = gsta_irq_type;
> +	ct->chip.irq_disable = gsta_irq_disable;
> +	ct->chip.irq_enable = gsta_irq_enable;
> +
> +	/* FIXME: this makes at most 32 interrupts. Request 0 by now */
> +	irq_setup_generic_chip(gc, 0 /* IRQ_MSK(GSTA_GPIO_PER_BLOCK) */, 0,
> +			       IRQ_NOREQUEST | IRQ_NOPROBE, 0);
> +
> +	/* Set up all all 128 interrupts: code from setup_generic_chip */
> +	{
> +		struct irq_chip_type *ct = gc->chip_types;
> +		int i, j;
> +		for (j = 0; j < GSTA_NR_GPIO; j++) {
> +			i = chip->irq_base + j;
> +			irq_set_chip_and_handler(i, &ct->chip, ct->handler);
> +			irq_set_chip_data(i, gc);
> +			irq_modify_status(i, IRQ_NOREQUEST | IRQ_NOPROBE, 0);
> +		}
> +		gc->irq_cnt = i - gc->irq_base;
> +	}
> +}
> +
> +/* The platform device used here is instantiated by the MFD device */
> +static int __devinit gsta_probe(struct platform_device *dev)
> +{
> +	int i, err;
> +	struct pci_dev *pdev;
> +	struct sta2x11_gpio_pdata *gpio_pdata;
> +	struct gsta_gpio *chip;
> +	struct resource *res;
> +
> +	pdev = *(struct pci_dev **)(dev->dev.platform_data);
> +	gpio_pdata = dev_get_platdata(&pdev->dev);
> +
> +	if (gpio_pdata == NULL)
> +		dev_err(&dev->dev, "no gpio config\n");
> +	printk("gpio config: %p\n", gpio_pdata);
> +
> +	res = platform_get_resource(dev, IORESOURCE_MEM, 0);
> +	request_mem_region(res->start, resource_size(res), KBUILD_MODNAME);
> +
> +
> +	chip = kzalloc(sizeof(*chip), GFP_KERNEL);

devm_kzalloc() will make this code simpler (don't need to kfree on
error or remove).

> +	chip->dev = &dev->dev;
> +	chip->reg_base = ioremap(res->start, resource_size(res));
> +	chip->irq_base = -1;

This line is meaningless.

> +	for (i = 0; i < GSTA_NR_BLOCKS; i++) {
> +		chip->regs[i] = chip->reg_base + i * 4096;
> +		/* disable all irqs */
> +		writel(0, &chip->regs[i]->rimsc);
> +		writel(0, &chip->regs[i]->fimsc);
> +		writel(~0, &chip->regs[i]->ic);
> +	}
> +	spin_lock_init(&chip->lock);
> +	gsta_gpio_setup(chip);
> +	for (i = 0; i < GSTA_NR_GPIO; i++)
> +		     gsta_set_config(chip, i, gpio_pdata->pinconfig[i]);
> +	err = gpiochip_add(&chip->gpio);
> +	if (err < 0) {
> +		dev_err(&dev->dev, "sta2x11 gpio: Can't register (%i)\n",
> +			-err);
> +		kfree(chip);
> +		return err;
> +	}
> +	/* 384 was used in previous code: be compatible for other drivers */
> +	err = irq_alloc_descs(-1, 384, GSTA_NR_GPIO, NUMA_NO_NODE);

That's a lot of irqs.  Will they all be used?  How do other drivers
determine which irq number to use (is it statically assigned, or is there
a dynamic mechanism)?  If only a portion are used, then the irq_domain
linear mapping would be a win here.

> +
> +	if (err < 0) {
> +		dev_warn(&dev->dev, "sta2x11 gpio: Can't get irq base (%i)\n",
> +			 -err);
> +		goto err_out;
> +	}
> +	chip->irq_base = err;
> +	gsta_alloc_irq_chip(chip);
> +
> +	err = request_irq(pdev->irq, gsta_gpio_handler,
> +			     IRQF_SHARED, KBUILD_MODNAME, chip);
> +	if (err < 0) {
> +		dev_err(&dev->dev, "sta2x11 gpio: Can't request irq (%i)\n",
> +			-err);
> +		goto err_out;
> +	}
> +
> +	platform_set_drvdata(dev, chip);
> +	return 0;
> +
> +err_out:
> +	if (chip->irq_base > 0)
> +		irq_free_descs(chip->irq_base, GSTA_NR_GPIO);
> +	kfree(chip);
> +	return err;
> +}
> +
> +static struct platform_driver sta2x11_gpio_platform_driver = {
> +	.driver = {
> +		.name	= "sta2x11-gpio",
> +		.owner	= THIS_MODULE,
> +	},
> +	.probe = gsta_probe,
> +};
> +
> +static int __init sta2x11_gpio_init(void)
> +{
> +	return platform_driver_register(&sta2x11_gpio_platform_driver);
> +}
> +
> +module_init(sta2x11_gpio_init);

module_platform_driver()

> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("sta2x11_gpio GPIO driver");
> -- 
> 1.7.7.2

-- 
email sent from notmuch.vim plugin

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH V2 2/2] gpio: add STA2X11 GPIO block
  2012-02-16 13:00 ` [PATCH V2 2/2] gpio: add STA2X11 GPIO block Alessandro Rubini
                     ` (5 preceding siblings ...)
  2012-03-02  7:48   ` Grant Likely
@ 2012-03-02 10:00   ` Alessandro Rubini
  6 siblings, 0 replies; 14+ messages in thread
From: Alessandro Rubini @ 2012-03-02 10:00 UTC (permalink / raw)
  To: grant.likely; +Cc: linux-kernel, giancarlo.asnaghi, alan, sameo, linus.walleij

Thank you Grant for your comments. Agreed with all of them.

>> +	/* 384 was used in previous code: be compatible for other drivers */
>> +	err = irq_alloc_descs(-1, 384, GSTA_NR_GPIO, NUMA_NO_NODE);
> 
> That's a lot of irqs.  Will they all be used?

384 is the starting point, isn't it? The number is 128. One per gpio pin.
We have change-detect for mmc and other stuff that live in high gpio
numbers.

> How do other drivers determine which irq number to use (is it
> statically assigned, or is there a dynamic mechanism)?  If only a
> portion are used, then the irq_domain linear mapping would be a win
> here.

The code I received uses static numbers. The chip is the main chipset
in the typical use case, so only one is there even if it is PCI.  I
also have a PCIe card to use it as slave device, and I'm careful to
allow several of them, even if it's not the main use case. Maybe here
I fell short. I'm reposting soon, with a full change log.

This "compatibility" is something I need to run the boards with
existing code for the parts that are not cleaned up for upstream, yet.

/alessandro

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2012-03-02 10:00 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-02-16 13:00 [PATCH V2 0/2] MFD and GPIO for STA2X11 Alessandro Rubini
2012-02-16 13:00 ` [PATCH V2 1/2] mfd: Add driver for STA2X11 MFD block Alessandro Rubini
2012-02-16 19:29   ` Linus Walleij
2012-02-23 16:19   ` Samuel Ortiz
2012-02-23 16:25   ` Alessandro Rubini
2012-02-16 13:00 ` [PATCH V2 2/2] gpio: add STA2X11 GPIO block Alessandro Rubini
2012-02-16 19:16   ` Linus Walleij
2012-02-16 19:24   ` Linus Walleij
2012-02-16 20:24   ` Alessandro Rubini
2012-02-16 20:24   ` Alessandro Rubini
2012-03-01 18:50   ` Alessandro Rubini
2012-03-01 20:20     ` Linus Walleij
2012-03-02  7:48   ` Grant Likely
2012-03-02 10:00   ` Alessandro Rubini

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.