* [PATCH 1/3] mfd: add support for Janz CMOD-IO PCI MODULbus Carrier Board
@ 2010-03-18 16:38 Ira W. Snyder
2010-03-19 9:13 ` Wolfgang Grandegger
2010-03-19 16:38 ` Samuel Ortiz
0 siblings, 2 replies; 10+ messages in thread
From: Ira W. Snyder @ 2010-03-18 16:38 UTC (permalink / raw
To: linux-kernel; +Cc: netdev, sameo, socketcan-core
The Janz CMOD-IO PCI MODULbus carrier board is a PCI to MODULbus bridge,
which may host many different types of MODULbus daughterboards, including
CAN and GPIO controllers.
Signed-off-by: Ira W. Snyder <iws@ovro.caltech.edu>
Cc: Samuel Ortiz <sameo@linux.intel.com>
---
drivers/mfd/Kconfig | 8 +
drivers/mfd/Makefile | 1 +
drivers/mfd/janz-cmodio.c | 339 +++++++++++++++++++++++++++++++++++++++++++++
include/linux/mfd/janz.h | 54 +++++++
4 files changed, 402 insertions(+), 0 deletions(-)
create mode 100644 drivers/mfd/janz-cmodio.c
create mode 100644 include/linux/mfd/janz.h
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 8782978..f1858d7 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -27,6 +27,14 @@ config MFD_SM501_GPIO
lines on the SM501. The platform data is used to supply the
base number for the first GPIO line to register.
+config MFD_JANZ_CMODIO
+ tristate "Support for Janz CMOD-IO PCI MODULbus Carrier Board"
+ ---help---
+ This is the core driver for the Janz CMOD-IO PCI MODULbus
+ carrier board. This device is a PCI to MODULbus bridge which may
+ host many different types of MODULbus daughterboards, including
+ CAN and GPIO controllers.
+
config MFD_ASIC3
bool "Support for Compaq ASIC3"
depends on GENERIC_HARDIRQS && GPIOLIB && ARM
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index e09eb48..e8fa905 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -3,6 +3,7 @@
#
obj-$(CONFIG_MFD_SM501) += sm501.o
+obj-$(CONFIG_MFD_JANZ_CMODIO) += janz-cmodio.o
obj-$(CONFIG_MFD_ASIC3) += asic3.o tmio_core.o
obj-$(CONFIG_MFD_SH_MOBILE_SDHI) += sh_mobile_sdhi.o
diff --git a/drivers/mfd/janz-cmodio.c b/drivers/mfd/janz-cmodio.c
new file mode 100644
index 0000000..914280e
--- /dev/null
+++ b/drivers/mfd/janz-cmodio.c
@@ -0,0 +1,339 @@
+/*
+ * Janz CMOD-IO MODULbus Carrier Board PCI Driver
+ *
+ * Copyright (c) 2010 Ira W. Snyder <iws@ovro.caltech.edu>
+ *
+ * Lots of inspiration and code was copied from drivers/mfd/sm501.c
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/pci.h>
+#include <linux/interrupt.h>
+#include <linux/delay.h>
+#include <linux/platform_device.h>
+
+#include <linux/mfd/janz.h>
+
+#define DRV_NAME "janz-cmodio"
+
+/* Size of each MODULbus module in PCI BAR4 */
+#define CMODIO_MODULBUS_SIZE 0x200
+
+/* Maximum number of MODULbus modules on a CMOD-IO carrier board */
+#define CMODIO_MAX_MODULES 4
+
+/* Module Parameters */
+static unsigned int num_modules = CMODIO_MAX_MODULES;
+static unsigned char *modules[CMODIO_MAX_MODULES] = {
+ "janz-ican3",
+ "janz-ican3",
+ "",
+ "janz-ttl",
+};
+
+module_param_array(modules, charp, &num_modules, S_IRUGO);
+MODULE_PARM_DESC(modules, "MODULbus modules attached to the carrier board");
+
+struct cmodio_device {
+ /* Parent PCI device */
+ struct pci_dev *pdev;
+
+ /* PLX control registers */
+ struct janz_cmodio_onboard_regs __iomem *ctrl;
+
+ /* hex switch position */
+ u8 hex;
+
+ /* Subdevice ID numbers */
+ unsigned int subdev_id;
+};
+
+/*
+ * Subdevice Support
+ */
+
+static int cmodio_remove_subdev(struct device *dev, void *data)
+{
+ platform_device_unregister(to_platform_device(dev));
+ return 0;
+}
+
+static void cmodio_subdev_release(struct device *dev)
+{
+ kfree(to_platform_device(dev));
+}
+
+static struct platform_device *cmodio_create_subdev(struct cmodio_device *priv,
+ char *name,
+ unsigned int res_count,
+ unsigned int pdata_size)
+{
+ struct platform_device *pdev;
+ size_t res_size;
+
+ res_size = sizeof(struct resource) * res_count;
+ pdev = kzalloc(sizeof(*pdev) + res_size + pdata_size, GFP_KERNEL);
+ if (!pdev)
+ return NULL;
+
+ pdev->dev.release = cmodio_subdev_release;
+ pdev->dev.parent = &priv->pdev->dev;
+ pdev->name = name;
+
+ if (res_count) {
+ pdev->resource = (struct resource *)(pdev + 1);
+ pdev->num_resources = res_count;
+ }
+
+ if (pdata_size)
+ pdev->dev.platform_data = (void *)(pdev + 1) + res_size;
+
+ return pdev;
+}
+
+/* Create a memory resource for a subdevice */
+static void cmodio_create_mem(struct resource *parent, struct resource *res,
+ resource_size_t offset, resource_size_t size)
+{
+ res->flags = IORESOURCE_MEM;
+ res->parent = parent;
+ res->start = parent->start + offset;
+ res->end = parent->start + offset + size - 1;
+}
+
+/* Create an IRQ resource for a subdevice */
+static void cmodio_create_irq(struct resource *res, unsigned int irq)
+{
+ res->flags = IORESOURCE_IRQ;
+ res->parent = NULL;
+ res->start = irq;
+ res->end = irq;
+}
+
+static int __devinit cmodio_probe_subdevice(struct cmodio_device *priv,
+ char *name, unsigned int modno)
+{
+ struct janz_platform_data *pdata;
+ struct platform_device *pdev;
+ resource_size_t offset, size;
+ struct pci_dev *pci;
+ int ret;
+
+ pci = priv->pdev;
+ pdev = cmodio_create_subdev(priv, name, 3, sizeof(*pdata));
+ if (!pdev) {
+ dev_err(&pci->dev, "MODULbus slot %d alloc failed\n", modno);
+ ret = -ENOMEM;
+ goto out_return;
+ }
+
+ pdata = pdev->dev.platform_data;
+ pdata->modno = modno;
+ pdev->id = priv->subdev_id++;
+
+ /* MODULbus registers -- PCI BAR3 is big-endian MODULbus access */
+ offset = CMODIO_MODULBUS_SIZE * modno;
+ size = CMODIO_MODULBUS_SIZE;
+ cmodio_create_mem(&pci->resource[3], &pdev->resource[0], offset, size);
+
+ /* PLX Control Registers -- PCI BAR4 is interrupt and other registers */
+ offset = 0;
+ size = resource_size(&pci->resource[4]);
+ cmodio_create_mem(&pci->resource[4], &pdev->resource[1], offset, size);
+
+ /* IRQ */
+ cmodio_create_irq(&pdev->resource[2], pci->irq);
+
+ /* Register the device */
+ ret = platform_device_register(pdev);
+ if (ret) {
+ dev_err(&pci->dev, "MODULbus slot %d register failed\n", modno);
+ goto out_free;
+ }
+
+ return 0;
+
+out_free:
+ cmodio_subdev_release(&pdev->dev);
+out_return:
+ return ret;
+}
+
+/* Probe each submodule using kernel parameters */
+static int __devinit cmodio_probe_submodules(struct cmodio_device *priv)
+{
+ char *name;
+ int i;
+
+ for (i = 0; i < num_modules; i++) {
+ name = modules[i];
+ if (!strcmp(name, ""))
+ continue;
+
+ dev_dbg(&priv->pdev->dev, "MODULbus %d: name %s\n", i, name);
+ cmodio_probe_subdevice(priv, name, i);
+ }
+
+ return 0;
+}
+
+/*
+ * SYSFS Attributes
+ */
+
+static ssize_t mbus_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct cmodio_device *priv = dev_get_drvdata(dev);
+
+ return snprintf(buf, PAGE_SIZE, "%x\n", priv->hex);
+}
+
+static DEVICE_ATTR(modulbus_number, S_IRUGO, mbus_show, NULL);
+
+static struct attribute *cmodio_sysfs_attrs[] = {
+ &dev_attr_modulbus_number.attr,
+ NULL,
+};
+
+static const struct attribute_group cmodio_sysfs_attr_group = {
+ .attrs = cmodio_sysfs_attrs,
+};
+
+/*
+ * PCI Driver
+ */
+
+static int __devinit cmodio_pci_probe(struct pci_dev *dev,
+ const struct pci_device_id *id)
+{
+ struct cmodio_device *priv;
+ int ret;
+
+ priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+ if (!priv) {
+ dev_err(&dev->dev, "unable to allocate private data\n");
+ ret = -ENOMEM;
+ goto out_return;
+ }
+
+ pci_set_drvdata(dev, priv);
+ priv->pdev = dev;
+
+ /* Hardware Initialization */
+ ret = pci_enable_device(dev);
+ if (ret) {
+ dev_err(&dev->dev, "unable to enable device\n");
+ goto out_free_priv;
+ }
+
+ pci_set_master(dev);
+ ret = pci_request_regions(dev, DRV_NAME);
+ if (ret) {
+ dev_err(&dev->dev, "unable to request regions\n");
+ goto out_pci_disable_device;
+ }
+
+ /* Onboard configuration registers */
+ priv->ctrl = pci_ioremap_bar(dev, 4);
+ if (!priv->ctrl) {
+ dev_err(&dev->dev, "unable to remap onboard regs\n");
+ ret = -ENOMEM;
+ goto out_pci_release_regions;
+ }
+
+ /* Read the hex switch on the carrier board */
+ priv->hex = ioread8(&priv->ctrl->int_enable);
+
+ /* Add the MODULbus number (hex switch value) to the device's sysfs */
+ ret = sysfs_create_group(&dev->dev.kobj, &cmodio_sysfs_attr_group);
+ if (ret) {
+ dev_err(&dev->dev, "unable to create sysfs attributes\n");
+ goto out_unmap_ctrl;
+ }
+
+ /*
+ * Disable all interrupt lines, each submodule will enable its
+ * own interrupt line if needed
+ */
+ iowrite8(0xf, &priv->ctrl->int_disable);
+
+ /* Register drivers for all submodules */
+ ret = cmodio_probe_submodules(priv);
+ if (ret) {
+ dev_err(&dev->dev, "unable to probe submodules\n");
+ goto out_sysfs_remove_group;
+ }
+
+ return 0;
+
+out_sysfs_remove_group:
+ sysfs_remove_group(&dev->dev.kobj, &cmodio_sysfs_attr_group);
+out_unmap_ctrl:
+ iounmap(priv->ctrl);
+out_pci_release_regions:
+ pci_release_regions(dev);
+out_pci_disable_device:
+ pci_disable_device(dev);
+out_free_priv:
+ kfree(priv);
+out_return:
+ return ret;
+}
+
+static void __devexit cmodio_pci_remove(struct pci_dev *dev)
+{
+ struct cmodio_device *priv = pci_get_drvdata(dev);
+
+ device_for_each_child(&dev->dev, NULL, cmodio_remove_subdev);
+ sysfs_remove_group(&dev->dev.kobj, &cmodio_sysfs_attr_group);
+ iounmap(priv->ctrl);
+ pci_release_regions(dev);
+ pci_disable_device(dev);
+ kfree(priv);
+}
+
+#define PCI_VENDOR_ID_JANZ 0x13c3
+
+/* The list of devices that this module will support */
+static DEFINE_PCI_DEVICE_TABLE(cmodio_pci_ids) = {
+ { PCI_VENDOR_ID_PLX, PCI_DEVICE_ID_PLX_9030, PCI_VENDOR_ID_JANZ, 0x0101 },
+ { PCI_VENDOR_ID_PLX, PCI_DEVICE_ID_PLX_9050, PCI_VENDOR_ID_JANZ, 0x0100 },
+ { 0, }
+};
+MODULE_DEVICE_TABLE(pci, cmodio_pci_ids);
+
+static struct pci_driver cmodio_pci_driver = {
+ .name = DRV_NAME,
+ .id_table = cmodio_pci_ids,
+ .probe = cmodio_pci_probe,
+ .remove = __devexit_p(cmodio_pci_remove),
+};
+
+/*
+ * Module Init / Exit
+ */
+
+static int __init cmodio_init(void)
+{
+ return pci_register_driver(&cmodio_pci_driver);
+}
+
+static void __exit cmodio_exit(void)
+{
+ pci_unregister_driver(&cmodio_pci_driver);
+}
+
+MODULE_AUTHOR("Ira W. Snyder <iws@ovro.caltech.edu>");
+MODULE_DESCRIPTION("Janz CMOD-IO PCI MODULbus Carrier Board Driver");
+MODULE_LICENSE("GPL");
+
+module_init(cmodio_init);
+module_exit(cmodio_exit);
diff --git a/include/linux/mfd/janz.h b/include/linux/mfd/janz.h
new file mode 100644
index 0000000..e9994c4
--- /dev/null
+++ b/include/linux/mfd/janz.h
@@ -0,0 +1,54 @@
+/*
+ * Common Definitions for Janz MODULbus devices
+ *
+ * Copyright (c) 2010 Ira W. Snyder <iws@ovro.caltech.edu>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ */
+
+#ifndef JANZ_H
+#define JANZ_H
+
+struct janz_platform_data {
+ /* MODULbus Module Number */
+ unsigned int modno;
+};
+
+/* PLX bridge chip onboard registers */
+struct janz_cmodio_onboard_regs {
+ u8 unused1;
+
+ /*
+ * Read access: interrupt status
+ * Write access: interrupt disable
+ */
+ u8 int_disable;
+ u8 unused2;
+
+ /*
+ * Read access: MODULbus number (hex switch)
+ * Write access: interrupt enable
+ */
+ u8 int_enable;
+ u8 unused3;
+
+ /* write-only */
+ u8 reset_assert;
+ u8 unused4;
+
+ /* write-only */
+ u8 reset_deassert;
+ u8 unused5;
+
+ /* read-write access to serial EEPROM */
+ u8 eep;
+ u8 unused6;
+
+ /* write-only access to EEPROM chip select */
+ u8 enid;
+};
+
+#endif /* JANZ_H */
--
1.5.4.3
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] mfd: add support for Janz CMOD-IO PCI MODULbus Carrier Board
2010-03-18 16:38 [PATCH 1/3] mfd: add support for Janz CMOD-IO PCI MODULbus Carrier Board Ira W. Snyder
@ 2010-03-19 9:13 ` Wolfgang Grandegger
2010-03-19 15:13 ` Ira W. Snyder
2010-03-19 16:38 ` Samuel Ortiz
1 sibling, 1 reply; 10+ messages in thread
From: Wolfgang Grandegger @ 2010-03-19 9:13 UTC (permalink / raw
To: Ira W. Snyder; +Cc: linux-kernel, socketcan-core, netdev, sameo
Ira W. Snyder wrote:
> The Janz CMOD-IO PCI MODULbus carrier board is a PCI to MODULbus bridge,
> which may host many different types of MODULbus daughterboards, including
> CAN and GPIO controllers.
>
> Signed-off-by: Ira W. Snyder <iws@ovro.caltech.edu>
> Cc: Samuel Ortiz <sameo@linux.intel.com>
You can add my "Reviewed-by: Wolfgang Grandegger <wg@grandegger.com>".
Just one concern:
> ---
> drivers/mfd/Kconfig | 8 +
> drivers/mfd/Makefile | 1 +
> drivers/mfd/janz-cmodio.c | 339 +++++++++++++++++++++++++++++++++++++++++++++
> include/linux/mfd/janz.h | 54 +++++++
> 4 files changed, 402 insertions(+), 0 deletions(-)
> create mode 100644 drivers/mfd/janz-cmodio.c
> create mode 100644 include/linux/mfd/janz.h
>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 8782978..f1858d7 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -27,6 +27,14 @@ config MFD_SM501_GPIO
> lines on the SM501. The platform data is used to supply the
> base number for the first GPIO line to register.
>
> +config MFD_JANZ_CMODIO
> + tristate "Support for Janz CMOD-IO PCI MODULbus Carrier Board"
> + ---help---
> + This is the core driver for the Janz CMOD-IO PCI MODULbus
> + carrier board. This device is a PCI to MODULbus bridge which may
> + host many different types of MODULbus daughterboards, including
> + CAN and GPIO controllers.
> +
> config MFD_ASIC3
> bool "Support for Compaq ASIC3"
> depends on GENERIC_HARDIRQS && GPIOLIB && ARM
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index e09eb48..e8fa905 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -3,6 +3,7 @@
> #
>
> obj-$(CONFIG_MFD_SM501) += sm501.o
> +obj-$(CONFIG_MFD_JANZ_CMODIO) += janz-cmodio.o
> obj-$(CONFIG_MFD_ASIC3) += asic3.o tmio_core.o
> obj-$(CONFIG_MFD_SH_MOBILE_SDHI) += sh_mobile_sdhi.o
>
> diff --git a/drivers/mfd/janz-cmodio.c b/drivers/mfd/janz-cmodio.c
> new file mode 100644
> index 0000000..914280e
> --- /dev/null
> +++ b/drivers/mfd/janz-cmodio.c
> @@ -0,0 +1,339 @@
> +/*
> + * Janz CMOD-IO MODULbus Carrier Board PCI Driver
> + *
> + * Copyright (c) 2010 Ira W. Snyder <iws@ovro.caltech.edu>
> + *
> + * Lots of inspiration and code was copied from drivers/mfd/sm501.c
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; either version 2 of the License, or (at your
> + * option) any later version.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/pci.h>
> +#include <linux/interrupt.h>
> +#include <linux/delay.h>
> +#include <linux/platform_device.h>
> +
> +#include <linux/mfd/janz.h>
> +
> +#define DRV_NAME "janz-cmodio"
> +
> +/* Size of each MODULbus module in PCI BAR4 */
> +#define CMODIO_MODULBUS_SIZE 0x200
> +
> +/* Maximum number of MODULbus modules on a CMOD-IO carrier board */
> +#define CMODIO_MAX_MODULES 4
> +
> +/* Module Parameters */
> +static unsigned int num_modules = CMODIO_MAX_MODULES;
> +static unsigned char *modules[CMODIO_MAX_MODULES] = {
> + "janz-ican3",
> + "janz-ican3",
> + "",
> + "janz-ttl",
> +};
This is probably not a good default but just your private configuration.
Wolfgang.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] mfd: add support for Janz CMOD-IO PCI MODULbus Carrier Board
2010-03-19 9:13 ` Wolfgang Grandegger
@ 2010-03-19 15:13 ` Ira W. Snyder
[not found] ` <20100319151326.GA13672-lulEs6mt1IksTUYHLfqkUA@public.gmane.org>
0 siblings, 1 reply; 10+ messages in thread
From: Ira W. Snyder @ 2010-03-19 15:13 UTC (permalink / raw
To: Wolfgang Grandegger; +Cc: linux-kernel, socketcan-core, netdev, sameo
On Fri, Mar 19, 2010 at 10:13:10AM +0100, Wolfgang Grandegger wrote:
> Ira W. Snyder wrote:
> > The Janz CMOD-IO PCI MODULbus carrier board is a PCI to MODULbus bridge,
> > which may host many different types of MODULbus daughterboards, including
> > CAN and GPIO controllers.
> >
> > Signed-off-by: Ira W. Snyder <iws@ovro.caltech.edu>
> > Cc: Samuel Ortiz <sameo@linux.intel.com>
>
> You can add my "Reviewed-by: Wolfgang Grandegger <wg@grandegger.com>".
>
> Just one concern:
>
> > ---
> > drivers/mfd/Kconfig | 8 +
> > drivers/mfd/Makefile | 1 +
> > drivers/mfd/janz-cmodio.c | 339 +++++++++++++++++++++++++++++++++++++++++++++
> > include/linux/mfd/janz.h | 54 +++++++
> > 4 files changed, 402 insertions(+), 0 deletions(-)
> > create mode 100644 drivers/mfd/janz-cmodio.c
> > create mode 100644 include/linux/mfd/janz.h
> >
> > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> > index 8782978..f1858d7 100644
> > --- a/drivers/mfd/Kconfig
> > +++ b/drivers/mfd/Kconfig
> > @@ -27,6 +27,14 @@ config MFD_SM501_GPIO
> > lines on the SM501. The platform data is used to supply the
> > base number for the first GPIO line to register.
> >
> > +config MFD_JANZ_CMODIO
> > + tristate "Support for Janz CMOD-IO PCI MODULbus Carrier Board"
> > + ---help---
> > + This is the core driver for the Janz CMOD-IO PCI MODULbus
> > + carrier board. This device is a PCI to MODULbus bridge which may
> > + host many different types of MODULbus daughterboards, including
> > + CAN and GPIO controllers.
> > +
> > config MFD_ASIC3
> > bool "Support for Compaq ASIC3"
> > depends on GENERIC_HARDIRQS && GPIOLIB && ARM
> > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> > index e09eb48..e8fa905 100644
> > --- a/drivers/mfd/Makefile
> > +++ b/drivers/mfd/Makefile
> > @@ -3,6 +3,7 @@
> > #
> >
> > obj-$(CONFIG_MFD_SM501) += sm501.o
> > +obj-$(CONFIG_MFD_JANZ_CMODIO) += janz-cmodio.o
> > obj-$(CONFIG_MFD_ASIC3) += asic3.o tmio_core.o
> > obj-$(CONFIG_MFD_SH_MOBILE_SDHI) += sh_mobile_sdhi.o
> >
> > diff --git a/drivers/mfd/janz-cmodio.c b/drivers/mfd/janz-cmodio.c
> > new file mode 100644
> > index 0000000..914280e
> > --- /dev/null
> > +++ b/drivers/mfd/janz-cmodio.c
> > @@ -0,0 +1,339 @@
> > +/*
> > + * Janz CMOD-IO MODULbus Carrier Board PCI Driver
> > + *
> > + * Copyright (c) 2010 Ira W. Snyder <iws@ovro.caltech.edu>
> > + *
> > + * Lots of inspiration and code was copied from drivers/mfd/sm501.c
> > + *
> > + * This program is free software; you can redistribute it and/or modify it
> > + * under the terms of the GNU General Public License as published by the
> > + * Free Software Foundation; either version 2 of the License, or (at your
> > + * option) any later version.
> > + */
> > +
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/init.h>
> > +#include <linux/pci.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/delay.h>
> > +#include <linux/platform_device.h>
> > +
> > +#include <linux/mfd/janz.h>
> > +
> > +#define DRV_NAME "janz-cmodio"
> > +
> > +/* Size of each MODULbus module in PCI BAR4 */
> > +#define CMODIO_MODULBUS_SIZE 0x200
> > +
> > +/* Maximum number of MODULbus modules on a CMOD-IO carrier board */
> > +#define CMODIO_MAX_MODULES 4
> > +
> > +/* Module Parameters */
> > +static unsigned int num_modules = CMODIO_MAX_MODULES;
> > +static unsigned char *modules[CMODIO_MAX_MODULES] = {
> > + "janz-ican3",
> > + "janz-ican3",
> > + "",
> > + "janz-ttl",
> > +};
>
> This is probably not a good default but just your private configuration.
>
Yep, that's the configuration I have. Do you have any suggestions for a
better default? I could make them all blank strings, which means "no
daughtercard in this slot". That is my only idea for a different
default.
Thanks for the review!
Ira
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] mfd: add support for Janz CMOD-IO PCI MODULbus Carrier Board
[not found] ` <20100319151326.GA13672-lulEs6mt1IksTUYHLfqkUA@public.gmane.org>
@ 2010-03-19 15:35 ` Wolfgang Grandegger
0 siblings, 0 replies; 10+ messages in thread
From: Wolfgang Grandegger @ 2010-03-19 15:35 UTC (permalink / raw
To: Ira W. Snyder
Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, sameo-VuQAYsv1563Yd54FQh9/CA
Ira W. Snyder wrote:
> On Fri, Mar 19, 2010 at 10:13:10AM +0100, Wolfgang Grandegger wrote:
>> Ira W. Snyder wrote:
>>> The Janz CMOD-IO PCI MODULbus carrier board is a PCI to MODULbus bridge,
>>> which may host many different types of MODULbus daughterboards, including
>>> CAN and GPIO controllers.
>>>
>>> Signed-off-by: Ira W. Snyder <iws-lulEs6mt1IksTUYHLfqkUA@public.gmane.org>
>>> Cc: Samuel Ortiz <sameo-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
>> You can add my "Reviewed-by: Wolfgang Grandegger <wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>".
>>
>> Just one concern:
>>
>>> ---
>>> drivers/mfd/Kconfig | 8 +
>>> drivers/mfd/Makefile | 1 +
>>> drivers/mfd/janz-cmodio.c | 339 +++++++++++++++++++++++++++++++++++++++++++++
>>> include/linux/mfd/janz.h | 54 +++++++
>>> 4 files changed, 402 insertions(+), 0 deletions(-)
>>> create mode 100644 drivers/mfd/janz-cmodio.c
>>> create mode 100644 include/linux/mfd/janz.h
>>>
>>> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
>>> index 8782978..f1858d7 100644
>>> --- a/drivers/mfd/Kconfig
>>> +++ b/drivers/mfd/Kconfig
>>> @@ -27,6 +27,14 @@ config MFD_SM501_GPIO
>>> lines on the SM501. The platform data is used to supply the
>>> base number for the first GPIO line to register.
>>>
>>> +config MFD_JANZ_CMODIO
>>> + tristate "Support for Janz CMOD-IO PCI MODULbus Carrier Board"
>>> + ---help---
>>> + This is the core driver for the Janz CMOD-IO PCI MODULbus
>>> + carrier board. This device is a PCI to MODULbus bridge which may
>>> + host many different types of MODULbus daughterboards, including
>>> + CAN and GPIO controllers.
>>> +
>>> config MFD_ASIC3
>>> bool "Support for Compaq ASIC3"
>>> depends on GENERIC_HARDIRQS && GPIOLIB && ARM
>>> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
>>> index e09eb48..e8fa905 100644
>>> --- a/drivers/mfd/Makefile
>>> +++ b/drivers/mfd/Makefile
>>> @@ -3,6 +3,7 @@
>>> #
>>>
>>> obj-$(CONFIG_MFD_SM501) += sm501.o
>>> +obj-$(CONFIG_MFD_JANZ_CMODIO) += janz-cmodio.o
>>> obj-$(CONFIG_MFD_ASIC3) += asic3.o tmio_core.o
>>> obj-$(CONFIG_MFD_SH_MOBILE_SDHI) += sh_mobile_sdhi.o
>>>
>>> diff --git a/drivers/mfd/janz-cmodio.c b/drivers/mfd/janz-cmodio.c
>>> new file mode 100644
>>> index 0000000..914280e
>>> --- /dev/null
>>> +++ b/drivers/mfd/janz-cmodio.c
>>> @@ -0,0 +1,339 @@
>>> +/*
>>> + * Janz CMOD-IO MODULbus Carrier Board PCI Driver
>>> + *
>>> + * Copyright (c) 2010 Ira W. Snyder <iws-lulEs6mt1IksTUYHLfqkUA@public.gmane.org>
>>> + *
>>> + * Lots of inspiration and code was copied from drivers/mfd/sm501.c
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify it
>>> + * under the terms of the GNU General Public License as published by the
>>> + * Free Software Foundation; either version 2 of the License, or (at your
>>> + * option) any later version.
>>> + */
>>> +
>>> +#include <linux/kernel.h>
>>> +#include <linux/module.h>
>>> +#include <linux/init.h>
>>> +#include <linux/pci.h>
>>> +#include <linux/interrupt.h>
>>> +#include <linux/delay.h>
>>> +#include <linux/platform_device.h>
>>> +
>>> +#include <linux/mfd/janz.h>
>>> +
>>> +#define DRV_NAME "janz-cmodio"
>>> +
>>> +/* Size of each MODULbus module in PCI BAR4 */
>>> +#define CMODIO_MODULBUS_SIZE 0x200
>>> +
>>> +/* Maximum number of MODULbus modules on a CMOD-IO carrier board */
>>> +#define CMODIO_MAX_MODULES 4
>>> +
>>> +/* Module Parameters */
>>> +static unsigned int num_modules = CMODIO_MAX_MODULES;
>>> +static unsigned char *modules[CMODIO_MAX_MODULES] = {
>>> + "janz-ican3",
>>> + "janz-ican3",
>>> + "",
>>> + "janz-ttl",
>>> +};
>> This is probably not a good default but just your private configuration.
>>
>
> Yep, that's the configuration I have. Do you have any suggestions for a
> better default? I could make them all blank strings, which means "no
> daughtercard in this slot". That is my only idea for a different
> default.
That's probably better. And if no boards are defined via module
parameter, return with an error and print an error message telling the
user what to do. Also maybe s/""/"empty"/ and a better the
MODULE_DESCRIPTION could be useful.
Wolfgang.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] mfd: add support for Janz CMOD-IO PCI MODULbus Carrier Board
2010-03-18 16:38 [PATCH 1/3] mfd: add support for Janz CMOD-IO PCI MODULbus Carrier Board Ira W. Snyder
2010-03-19 9:13 ` Wolfgang Grandegger
@ 2010-03-19 16:38 ` Samuel Ortiz
[not found] ` <20100319163849.GB30409-jcdQHdrhKHMdnm+yROfE0A@public.gmane.org>
1 sibling, 1 reply; 10+ messages in thread
From: Samuel Ortiz @ 2010-03-19 16:38 UTC (permalink / raw
To: Ira W. Snyder; +Cc: linux-kernel, netdev, socketcan-core
Hi Ira,
Some comments below:
On Thu, Mar 18, 2010 at 09:38:42AM -0700, Ira W. Snyder wrote:
> +/* Module Parameters */
> +static unsigned int num_modules = CMODIO_MAX_MODULES;
> +static unsigned char *modules[CMODIO_MAX_MODULES] = {
> + "janz-ican3",
> + "janz-ican3",
> + "",
> + "janz-ttl",
> +};
That's not very nice, but I honestly dont know how to make it less ugly...
At least this should be all left blank by default, as Wolfgang hinted.
> +/*
> + * Subdevice Support
> + */
Please use the mfd-core API for building and registering platform sub devices.
The pieces of code below should shrink significantly.
> +static int cmodio_remove_subdev(struct device *dev, void *data)
> +{
> + platform_device_unregister(to_platform_device(dev));
> + return 0;
> +}
> +
> +static void cmodio_subdev_release(struct device *dev)
> +{
> + kfree(to_platform_device(dev));
> +}
> +
> +static struct platform_device *cmodio_create_subdev(struct cmodio_device *priv,
> + char *name,
> + unsigned int res_count,
> + unsigned int pdata_size)
> +{
> + struct platform_device *pdev;
> + size_t res_size;
> +
> + res_size = sizeof(struct resource) * res_count;
> + pdev = kzalloc(sizeof(*pdev) + res_size + pdata_size, GFP_KERNEL);
> + if (!pdev)
> + return NULL;
> +
> + pdev->dev.release = cmodio_subdev_release;
> + pdev->dev.parent = &priv->pdev->dev;
> + pdev->name = name;
> +
> + if (res_count) {
> + pdev->resource = (struct resource *)(pdev + 1);
> + pdev->num_resources = res_count;
> + }
> +
> + if (pdata_size)
> + pdev->dev.platform_data = (void *)(pdev + 1) + res_size;
> +
> + return pdev;
> +}
> +
> +/* Create a memory resource for a subdevice */
> +static void cmodio_create_mem(struct resource *parent, struct resource *res,
> + resource_size_t offset, resource_size_t size)
> +{
> + res->flags = IORESOURCE_MEM;
> + res->parent = parent;
> + res->start = parent->start + offset;
> + res->end = parent->start + offset + size - 1;
> +}
> +
> +/* Create an IRQ resource for a subdevice */
> +static void cmodio_create_irq(struct resource *res, unsigned int irq)
> +{
> + res->flags = IORESOURCE_IRQ;
> + res->parent = NULL;
> + res->start = irq;
> + res->end = irq;
> +}
> +
> +static int __devinit cmodio_probe_subdevice(struct cmodio_device *priv,
> + char *name, unsigned int modno)
> +{
> + struct janz_platform_data *pdata;
> + struct platform_device *pdev;
> + resource_size_t offset, size;
> + struct pci_dev *pci;
> + int ret;
> +
> + pci = priv->pdev;
> + pdev = cmodio_create_subdev(priv, name, 3, sizeof(*pdata));
> + if (!pdev) {
> + dev_err(&pci->dev, "MODULbus slot %d alloc failed\n", modno);
> + ret = -ENOMEM;
> + goto out_return;
> + }
> +
> + pdata = pdev->dev.platform_data;
> + pdata->modno = modno;
> + pdev->id = priv->subdev_id++;
> +
> + /* MODULbus registers -- PCI BAR3 is big-endian MODULbus access */
> + offset = CMODIO_MODULBUS_SIZE * modno;
> + size = CMODIO_MODULBUS_SIZE;
> + cmodio_create_mem(&pci->resource[3], &pdev->resource[0], offset, size);
> +
> + /* PLX Control Registers -- PCI BAR4 is interrupt and other registers */
> + offset = 0;
> + size = resource_size(&pci->resource[4]);
> + cmodio_create_mem(&pci->resource[4], &pdev->resource[1], offset, size);
> +
> + /* IRQ */
> + cmodio_create_irq(&pdev->resource[2], pci->irq);
> +
> + /* Register the device */
> + ret = platform_device_register(pdev);
> + if (ret) {
> + dev_err(&pci->dev, "MODULbus slot %d register failed\n", modno);
> + goto out_free;
> + }
> +
> + return 0;
> +
> +out_free:
> + cmodio_subdev_release(&pdev->dev);
> +out_return:
> + return ret;
> +}
> +
> +/* Probe each submodule using kernel parameters */
> +static int __devinit cmodio_probe_submodules(struct cmodio_device *priv)
> +{
> + char *name;
> + int i;
> +
> + for (i = 0; i < num_modules; i++) {
> + name = modules[i];
> + if (!strcmp(name, ""))
> + continue;
> +
> + dev_dbg(&priv->pdev->dev, "MODULbus %d: name %s\n", i, name);
> + cmodio_probe_subdevice(priv, name, i);
> + }
> +
> + return 0;
> +}
> +/*
> + * PCI Driver
> + */
> +
> +static int __devinit cmodio_pci_probe(struct pci_dev *dev,
> + const struct pci_device_id *id)
> +{
> + struct cmodio_device *priv;
> + int ret;
> +
> + priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> + if (!priv) {
> + dev_err(&dev->dev, "unable to allocate private data\n");
> + ret = -ENOMEM;
> + goto out_return;
> + }
> +
> + pci_set_drvdata(dev, priv);
> + priv->pdev = dev;
> +
> + /* Hardware Initialization */
> + ret = pci_enable_device(dev);
> + if (ret) {
> + dev_err(&dev->dev, "unable to enable device\n");
> + goto out_free_priv;
> + }
> +
> + pci_set_master(dev);
> + ret = pci_request_regions(dev, DRV_NAME);
> + if (ret) {
> + dev_err(&dev->dev, "unable to request regions\n");
> + goto out_pci_disable_device;
> + }
> +
> + /* Onboard configuration registers */
> + priv->ctrl = pci_ioremap_bar(dev, 4);
Why 4 ?
> +#define PCI_VENDOR_ID_JANZ 0x13c3
That probably belongs to pci_ids.h
Cheers,
Samuel.
--
Intel Open Source Technology Centre
http://oss.intel.com/
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] mfd: add support for Janz CMOD-IO PCI MODULbus Carrier Board
[not found] ` <20100319163849.GB30409-jcdQHdrhKHMdnm+yROfE0A@public.gmane.org>
@ 2010-03-19 18:22 ` Ira W. Snyder
2010-03-25 22:59 ` Samuel Ortiz
0 siblings, 1 reply; 10+ messages in thread
From: Ira W. Snyder @ 2010-03-19 18:22 UTC (permalink / raw
To: Samuel Ortiz
Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
On Fri, Mar 19, 2010 at 05:38:51PM +0100, Samuel Ortiz wrote:
> Hi Ira,
>
> Some comments below:
>
> On Thu, Mar 18, 2010 at 09:38:42AM -0700, Ira W. Snyder wrote:
> > +/* Module Parameters */
> > +static unsigned int num_modules = CMODIO_MAX_MODULES;
> > +static unsigned char *modules[CMODIO_MAX_MODULES] = {
> > + "janz-ican3",
> > + "janz-ican3",
> > + "",
> > + "janz-ttl",
> > +};
> That's not very nice, but I honestly dont know how to make it less ugly...
> At least this should be all left blank by default, as Wolfgang hinted.
>
Agreed, I've made the change in my tree.
> > +/*
> > + * Subdevice Support
> > + */
> Please use the mfd-core API for building and registering platform sub devices.
> The pieces of code below should shrink significantly.
>
Using this framework, how is it possible to create the devices that I
do down below. For each subdevice, I need three resources:
1) MODULbus registers -- PCI BAR3 + (0x200 * module_num)
2) PLX Control Registers -- PCI BAR4
3) IRQ
Specifically, the way IORESOURCE_MEM resources are copied seems wrong.
They start at the base address of only one resource and use the offsets
provided in the struct mfd_cell. See the if-statement at
drivers/mfd/mfd-core.c line 48.
I need two use two different parent resources. The mfd_add_devices()
function doesn't support this.
> > +static int cmodio_remove_subdev(struct device *dev, void *data)
> > +{
> > + platform_device_unregister(to_platform_device(dev));
> > + return 0;
> > +}
> > +
> > +static void cmodio_subdev_release(struct device *dev)
> > +{
> > + kfree(to_platform_device(dev));
> > +}
> > +
> > +static struct platform_device *cmodio_create_subdev(struct cmodio_device *priv,
> > + char *name,
> > + unsigned int res_count,
> > + unsigned int pdata_size)
> > +{
> > + struct platform_device *pdev;
> > + size_t res_size;
> > +
> > + res_size = sizeof(struct resource) * res_count;
> > + pdev = kzalloc(sizeof(*pdev) + res_size + pdata_size, GFP_KERNEL);
> > + if (!pdev)
> > + return NULL;
> > +
> > + pdev->dev.release = cmodio_subdev_release;
> > + pdev->dev.parent = &priv->pdev->dev;
> > + pdev->name = name;
> > +
> > + if (res_count) {
> > + pdev->resource = (struct resource *)(pdev + 1);
> > + pdev->num_resources = res_count;
> > + }
> > +
> > + if (pdata_size)
> > + pdev->dev.platform_data = (void *)(pdev + 1) + res_size;
> > +
> > + return pdev;
> > +}
> > +
> > +/* Create a memory resource for a subdevice */
> > +static void cmodio_create_mem(struct resource *parent, struct resource *res,
> > + resource_size_t offset, resource_size_t size)
> > +{
> > + res->flags = IORESOURCE_MEM;
> > + res->parent = parent;
> > + res->start = parent->start + offset;
> > + res->end = parent->start + offset + size - 1;
> > +}
> > +
> > +/* Create an IRQ resource for a subdevice */
> > +static void cmodio_create_irq(struct resource *res, unsigned int irq)
> > +{
> > + res->flags = IORESOURCE_IRQ;
> > + res->parent = NULL;
> > + res->start = irq;
> > + res->end = irq;
> > +}
> > +
> > +static int __devinit cmodio_probe_subdevice(struct cmodio_device *priv,
> > + char *name, unsigned int modno)
> > +{
> > + struct janz_platform_data *pdata;
> > + struct platform_device *pdev;
> > + resource_size_t offset, size;
> > + struct pci_dev *pci;
> > + int ret;
> > +
> > + pci = priv->pdev;
> > + pdev = cmodio_create_subdev(priv, name, 3, sizeof(*pdata));
> > + if (!pdev) {
> > + dev_err(&pci->dev, "MODULbus slot %d alloc failed\n", modno);
> > + ret = -ENOMEM;
> > + goto out_return;
> > + }
> > +
> > + pdata = pdev->dev.platform_data;
> > + pdata->modno = modno;
> > + pdev->id = priv->subdev_id++;
> > +
> > + /* MODULbus registers -- PCI BAR3 is big-endian MODULbus access */
> > + offset = CMODIO_MODULBUS_SIZE * modno;
> > + size = CMODIO_MODULBUS_SIZE;
> > + cmodio_create_mem(&pci->resource[3], &pdev->resource[0], offset, size);
> > +
> > + /* PLX Control Registers -- PCI BAR4 is interrupt and other registers */
> > + offset = 0;
> > + size = resource_size(&pci->resource[4]);
> > + cmodio_create_mem(&pci->resource[4], &pdev->resource[1], offset, size);
> > +
> > + /* IRQ */
> > + cmodio_create_irq(&pdev->resource[2], pci->irq);
> > +
> > + /* Register the device */
> > + ret = platform_device_register(pdev);
> > + if (ret) {
> > + dev_err(&pci->dev, "MODULbus slot %d register failed\n", modno);
> > + goto out_free;
> > + }
> > +
> > + return 0;
> > +
> > +out_free:
> > + cmodio_subdev_release(&pdev->dev);
> > +out_return:
> > + return ret;
> > +}
> > +
> > +/* Probe each submodule using kernel parameters */
> > +static int __devinit cmodio_probe_submodules(struct cmodio_device *priv)
> > +{
> > + char *name;
> > + int i;
> > +
> > + for (i = 0; i < num_modules; i++) {
> > + name = modules[i];
> > + if (!strcmp(name, ""))
> > + continue;
> > +
> > + dev_dbg(&priv->pdev->dev, "MODULbus %d: name %s\n", i, name);
> > + cmodio_probe_subdevice(priv, name, i);
> > + }
> > +
> > + return 0;
> > +}
>
>
> > +/*
> > + * PCI Driver
> > + */
> > +
> > +static int __devinit cmodio_pci_probe(struct pci_dev *dev,
> > + const struct pci_device_id *id)
> > +{
> > + struct cmodio_device *priv;
> > + int ret;
> > +
> > + priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> > + if (!priv) {
> > + dev_err(&dev->dev, "unable to allocate private data\n");
> > + ret = -ENOMEM;
> > + goto out_return;
> > + }
> > +
> > + pci_set_drvdata(dev, priv);
> > + priv->pdev = dev;
> > +
> > + /* Hardware Initialization */
> > + ret = pci_enable_device(dev);
> > + if (ret) {
> > + dev_err(&dev->dev, "unable to enable device\n");
> > + goto out_free_priv;
> > + }
> > +
> > + pci_set_master(dev);
> > + ret = pci_request_regions(dev, DRV_NAME);
> > + if (ret) {
> > + dev_err(&dev->dev, "unable to request regions\n");
> > + goto out_pci_disable_device;
> > + }
> > +
> > + /* Onboard configuration registers */
> > + priv->ctrl = pci_ioremap_bar(dev, 4);
> Why 4 ?
>
>
Because that is how the device works ;) There is a comment up above that
describes them as the "PLX control registers". Are you suggesting that I
add a comment here too?
I think lots of other PCI devices just use hard-coded numbers here.
They're device specific. You won't be able to program for the device at
all if you don't know the meaning of each PCI BAR.
> > +#define PCI_VENDOR_ID_JANZ 0x13c3
> That probably belongs to pci_ids.h
>
Should I add a patch to the series for this?
Thanks,
Ira
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] mfd: add support for Janz CMOD-IO PCI MODULbus Carrier Board
2010-03-19 18:22 ` Ira W. Snyder
@ 2010-03-25 22:59 ` Samuel Ortiz
[not found] ` <20100325225929.GA20618-jcdQHdrhKHMdnm+yROfE0A@public.gmane.org>
0 siblings, 1 reply; 10+ messages in thread
From: Samuel Ortiz @ 2010-03-25 22:59 UTC (permalink / raw
To: Ira W. Snyder; +Cc: linux-kernel, netdev, socketcan-core
Hi Ira,
First of all, sorry for the late reply. Then my answers:
On Fri, Mar 19, 2010 at 11:22:09AM -0700, Ira W. Snyder wrote:
>
> > > +/*
> > > + * Subdevice Support
> > > + */
> > Please use the mfd-core API for building and registering platform sub devices.
> > The pieces of code below should shrink significantly.
> >
>
> Using this framework, how is it possible to create the devices that I
> do down below. For each subdevice, I need three resources:
>
> 1) MODULbus registers -- PCI BAR3 + (0x200 * module_num)
> 2) PLX Control Registers -- PCI BAR4
> 3) IRQ
>
> Specifically, the way IORESOURCE_MEM resources are copied seems wrong.
> They start at the base address of only one resource and use the offsets
> provided in the struct mfd_cell. See the if-statement at
> drivers/mfd/mfd-core.c line 48.
>
> I need two use two different parent resources. The mfd_add_devices()
> function doesn't support this.
I would still like you to use the mfd-core API. Here is my proposal:
1) I modify mfd_add_device() to support a NULL mem_base argument. When
mem_base is NULL, we would have:
res[r].parent = NULL and res[r].start = cell->resources[r].start;
The platform code will use iomem_resource as the parent for this resource.
2) Your mfd_cell cells would have 3 resources, and you just need to set the
IORESOURCE_MEM ones at probe time, with pci->resource[n]->start + offset as
the start field.
Would that make sense to you ?
> > > + /* Onboard configuration registers */
> > > + priv->ctrl = pci_ioremap_bar(dev, 4);
> > Why 4 ?
> >
> >
>
> Because that is how the device works ;) There is a comment up above that
> describes them as the "PLX control registers". Are you suggesting that I
> add a comment here too?
No, that's ok, I missed the comment.
> > > +#define PCI_VENDOR_ID_JANZ 0x13c3
> > That probably belongs to pci_ids.h
> >
>
> Should I add a patch to the series for this?
Either that or merge the pci_ids.h changes with this patch.
Cheers,
Samuel.
--
Intel Open Source Technology Centre
http://oss.intel.com/
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] mfd: add support for Janz CMOD-IO PCI MODULbus Carrier Board
[not found] ` <20100325225929.GA20618-jcdQHdrhKHMdnm+yROfE0A@public.gmane.org>
@ 2010-03-25 23:22 ` Ira W. Snyder
2010-03-26 0:26 ` Samuel Ortiz
0 siblings, 1 reply; 10+ messages in thread
From: Ira W. Snyder @ 2010-03-25 23:22 UTC (permalink / raw
To: Samuel Ortiz
Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
On Thu, Mar 25, 2010 at 11:59:30PM +0100, Samuel Ortiz wrote:
> Hi Ira,
>
> First of all, sorry for the late reply. Then my answers:
>
> On Fri, Mar 19, 2010 at 11:22:09AM -0700, Ira W. Snyder wrote:
> >
> > > > +/*
> > > > + * Subdevice Support
> > > > + */
> > > Please use the mfd-core API for building and registering platform sub devices.
> > > The pieces of code below should shrink significantly.
> > >
> >
> > Using this framework, how is it possible to create the devices that I
> > do down below. For each subdevice, I need three resources:
> >
> > 1) MODULbus registers -- PCI BAR3 + (0x200 * module_num)
> > 2) PLX Control Registers -- PCI BAR4
> > 3) IRQ
> >
> > Specifically, the way IORESOURCE_MEM resources are copied seems wrong.
> > They start at the base address of only one resource and use the offsets
> > provided in the struct mfd_cell. See the if-statement at
> > drivers/mfd/mfd-core.c line 48.
> >
> > I need two use two different parent resources. The mfd_add_devices()
> > function doesn't support this.
> I would still like you to use the mfd-core API. Here is my proposal:
>
> 1) I modify mfd_add_device() to support a NULL mem_base argument. When
> mem_base is NULL, we would have:
>
> res[r].parent = NULL and res[r].start = cell->resources[r].start;
>
> The platform code will use iomem_resource as the parent for this resource.
>
I don't know the implications of using iomem_resource as the parent
resource. If you think it is ok, I have no objections.
If it helps, I can provide the PCI resource as a parent resource in my
resources. Then, when mem_base is NULL, the mfd-core could do this:
res[r].parent = cell->resources[r].parent
This is basically what I did in my patch. I used the PCI resource as the
parent of all child resources. I don't know if that is safe, but it
works. :)
The mfd_add_device() function does this for IORESOURCE_IO resources. It
only tries to be smart for IORESOURCE_MEM and IORESOURCE_IRQ resources.
> 2) Your mfd_cell cells would have 3 resources, and you just need to set the
> IORESOURCE_MEM ones at probe time, with pci->resource[n]->start + offset as
> the start field.
>
> Would that make sense to you ?
>
Yep, that makes sense.
> > > > + /* Onboard configuration registers */
> > > > + priv->ctrl = pci_ioremap_bar(dev, 4);
> > > Why 4 ?
> > >
> > >
> >
> > Because that is how the device works ;) There is a comment up above that
> > describes them as the "PLX control registers". Are you suggesting that I
> > add a comment here too?
> No, that's ok, I missed the comment.
>
> > > > +#define PCI_VENDOR_ID_JANZ 0x13c3
> > > That probably belongs to pci_ids.h
> > >
> >
> > Should I add a patch to the series for this?
> Either that or merge the pci_ids.h changes with this patch.
>
I guess it is a trivial enough change to merge with this patch.
I'll wait for your patch to the mfd-core API before making changes and
sending out the next round of updates.
Thanks for replying,
Ira
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] mfd: add support for Janz CMOD-IO PCI MODULbus Carrier Board
2010-03-25 23:22 ` Ira W. Snyder
@ 2010-03-26 0:26 ` Samuel Ortiz
0 siblings, 0 replies; 10+ messages in thread
From: Samuel Ortiz @ 2010-03-26 0:26 UTC (permalink / raw
To: Ira W. Snyder; +Cc: linux-kernel, netdev, socketcan-core
On Thu, Mar 25, 2010 at 04:22:44PM -0700, Ira W. Snyder wrote:
> On Thu, Mar 25, 2010 at 11:59:30PM +0100, Samuel Ortiz wrote:
> > Hi Ira,
> >
> > First of all, sorry for the late reply. Then my answers:
> >
> > On Fri, Mar 19, 2010 at 11:22:09AM -0700, Ira W. Snyder wrote:
> > >
> > > > > +/*
> > > > > + * Subdevice Support
> > > > > + */
> > > > Please use the mfd-core API for building and registering platform sub devices.
> > > > The pieces of code below should shrink significantly.
> > > >
> > >
> > > Using this framework, how is it possible to create the devices that I
> > > do down below. For each subdevice, I need three resources:
> > >
> > > 1) MODULbus registers -- PCI BAR3 + (0x200 * module_num)
> > > 2) PLX Control Registers -- PCI BAR4
> > > 3) IRQ
> > >
> > > Specifically, the way IORESOURCE_MEM resources are copied seems wrong.
> > > They start at the base address of only one resource and use the offsets
> > > provided in the struct mfd_cell. See the if-statement at
> > > drivers/mfd/mfd-core.c line 48.
> > >
> > > I need two use two different parent resources. The mfd_add_devices()
> > > function doesn't support this.
> > I would still like you to use the mfd-core API. Here is my proposal:
> >
> > 1) I modify mfd_add_device() to support a NULL mem_base argument. When
> > mem_base is NULL, we would have:
> >
> > res[r].parent = NULL and res[r].start = cell->resources[r].start;
> >
> > The platform code will use iomem_resource as the parent for this resource.
> >
>
> I don't know the implications of using iomem_resource as the parent
> resource. If you think it is ok, I have no objections.
>
> If it helps, I can provide the PCI resource as a parent resource in my
> resources. Then, when mem_base is NULL, the mfd-core could do this:
>
> res[r].parent = cell->resources[r].parent
>
> This is basically what I did in my patch. I used the PCI resource as the
> parent of all child resources. I don't know if that is safe, but it
> works. :)
>
> The mfd_add_device() function does this for IORESOURCE_IO resources. It
> only tries to be smart for IORESOURCE_MEM and IORESOURCE_IRQ resources.
I pushed an mfd-core change that basically falls back to the default resource
copying when mem_base is NULL. That should allow you to use the API now.
> > > > > +#define PCI_VENDOR_ID_JANZ 0x13c3
> > > > That probably belongs to pci_ids.h
> > > >
> > >
> > > Should I add a patch to the series for this?
> > Either that or merge the pci_ids.h changes with this patch.
> >
>
> I guess it is a trivial enough change to merge with this patch.
>
> I'll wait for your patch to the mfd-core API before making changes and
> sending out the next round of updates.
Very nice. The above mentioned change in my for-next branch, commit
6802a325f541bbea3168cf61ba239443193e1f9a.
Cheers,
Samuel.
--
Intel Open Source Technology Centre
http://oss.intel.com/
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/3] mfd: add support for Janz CMOD-IO PCI MODULbus Carrier Board
@ 2010-03-29 16:58 Ira W. Snyder
0 siblings, 0 replies; 10+ messages in thread
From: Ira W. Snyder @ 2010-03-29 16:58 UTC (permalink / raw
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
netdev-u79uwXL29TY76Z2rM5mHXA, sameo-VuQAYsv1563Yd54FQh9/CA
The Janz CMOD-IO PCI MODULbus carrier board is a PCI to MODULbus bridge,
which may host many different types of MODULbus daughterboards, including
CAN and GPIO controllers.
Signed-off-by: Ira W. Snyder <iws-lulEs6mt1IksTUYHLfqkUA@public.gmane.org>
Reviewed-by: Wolfgang Grandegger <wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
---
drivers/mfd/Kconfig | 10 ++
drivers/mfd/Makefile | 1 +
drivers/mfd/janz-cmodio.c | 302 +++++++++++++++++++++++++++++++++++++++++++++
include/linux/mfd/janz.h | 54 ++++++++
include/linux/pci_ids.h | 2 +
5 files changed, 369 insertions(+), 0 deletions(-)
create mode 100644 drivers/mfd/janz-cmodio.c
create mode 100644 include/linux/mfd/janz.h
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 02fcd09..41329b4 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -408,6 +408,16 @@ config MFD_RDC321X
southbridge which provides access to GPIOs and Watchdog using the
southbridge PCI device configuration space.
+config MFD_JANZ_CMODIO
+ tristate "Support for Janz CMOD-IO PCI MODULbus Carrier Board"
+ select MFD_CORE
+ depends on PCI
+ help
+ This is the core driver for the Janz CMOD-IO PCI MODULbus
+ carrier board. This device is a PCI to MODULbus bridge which may
+ host many different types of MODULbus daughterboards, including
+ CAN and GPIO controllers.
+
endmenu
menu "Multimedia Capabilities Port drivers"
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index f5daffe..42a35e4 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -64,3 +64,4 @@ obj-$(CONFIG_MFD_TIMBERDALE) += timberdale.o
obj-$(CONFIG_PMIC_ADP5520) += adp5520.o
obj-$(CONFIG_LPC_SCH) += lpc_sch.o
obj-$(CONFIG_MFD_RDC321X) += rdc321x-southbridge.o
+obj-$(CONFIG_MFD_JANZ_CMODIO) += janz-cmodio.o
diff --git a/drivers/mfd/janz-cmodio.c b/drivers/mfd/janz-cmodio.c
new file mode 100644
index 0000000..ac7d59b
--- /dev/null
+++ b/drivers/mfd/janz-cmodio.c
@@ -0,0 +1,302 @@
+/*
+ * Janz CMOD-IO MODULbus Carrier Board PCI Driver
+ *
+ * Copyright (c) 2010 Ira W. Snyder <iws-lulEs6mt1IksTUYHLfqkUA@public.gmane.org>
+ *
+ * Lots of inspiration and code was copied from drivers/mfd/sm501.c
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/pci.h>
+#include <linux/interrupt.h>
+#include <linux/delay.h>
+#include <linux/platform_device.h>
+#include <linux/mfd/core.h>
+
+#include <linux/mfd/janz.h>
+
+#define DRV_NAME "janz-cmodio"
+
+/* Size of each MODULbus module in PCI BAR4 */
+#define CMODIO_MODULBUS_SIZE 0x200
+
+/* Maximum number of MODULbus modules on a CMOD-IO carrier board */
+#define CMODIO_MAX_MODULES 4
+
+/* Module Parameters */
+static unsigned int num_modules = CMODIO_MAX_MODULES;
+static unsigned char *modules[CMODIO_MAX_MODULES] = {
+ "empty", "empty", "empty", "empty",
+};
+
+module_param_array(modules, charp, &num_modules, S_IRUGO);
+MODULE_PARM_DESC(modules, "MODULbus modules attached to the carrier board");
+
+/* Unique Device Id */
+static unsigned int cmodio_id;
+
+struct cmodio_device {
+ /* Parent PCI device */
+ struct pci_dev *pdev;
+
+ /* PLX control registers */
+ struct janz_cmodio_onboard_regs __iomem *ctrl;
+
+ /* hex switch position */
+ u8 hex;
+
+ /* mfd-core API */
+ struct mfd_cell cells[CMODIO_MAX_MODULES];
+ struct resource resources[3 * CMODIO_MAX_MODULES];
+ struct janz_platform_data pdata[CMODIO_MAX_MODULES];
+};
+
+/*
+ * Subdevices using the mfd-core API
+ */
+
+static int __devinit cmodio_setup_subdevice(struct cmodio_device *priv,
+ char *name, unsigned int devno,
+ unsigned int modno)
+{
+ struct janz_platform_data *pdata;
+ struct mfd_cell *cell;
+ struct resource *res;
+ struct pci_dev *pci;
+
+ pci = priv->pdev;
+ cell = &priv->cells[devno];
+ res = &priv->resources[devno * 3];
+ pdata = &priv->pdata[devno];
+
+ cell->name = name;
+ cell->resources = res;
+ cell->num_resources = 3;
+
+ /* Setup the subdevice ID -- must be unique */
+ cell->id = cmodio_id++;
+
+ /* Add platform data */
+ pdata->modno = modno;
+ cell->platform_data = pdata;
+ cell->data_size = sizeof(*pdata);
+
+ /* MODULbus registers -- PCI BAR3 is big-endian MODULbus access */
+ res->flags = IORESOURCE_MEM;
+ res->parent = &pci->resource[3];
+ res->start = pci->resource[3].start + (CMODIO_MODULBUS_SIZE * modno);
+ res->end = res->start + CMODIO_MODULBUS_SIZE - 1;
+ res++;
+
+ /* PLX Control Registers -- PCI BAR4 is interrupt and other registers */
+ res->flags = IORESOURCE_MEM;
+ res->parent = &pci->resource[4];
+ res->start = pci->resource[4].start;
+ res->end = pci->resource[4].end;
+ res++;
+
+ /*
+ * IRQ
+ *
+ * The start and end fields are used as an offset to the irq_base
+ * parameter passed into the mfd_add_devices() function call. All
+ * devices share the same IRQ.
+ */
+ res->flags = IORESOURCE_IRQ;
+ res->parent = NULL;
+ res->start = 0;
+ res->end = 0;
+ res++;
+
+ return 0;
+}
+
+/* Probe each submodule using kernel parameters */
+static int __devinit cmodio_probe_submodules(struct cmodio_device *priv)
+{
+ struct pci_dev *pdev = priv->pdev;
+ unsigned int num_probed = 0;
+ char *name;
+ int i;
+
+ for (i = 0; i < num_modules; i++) {
+ name = modules[i];
+ if (!strcmp(name, "") || !strcmp(name, "empty"))
+ continue;
+
+ dev_dbg(&priv->pdev->dev, "MODULbus %d: name %s\n", i, name);
+ cmodio_setup_subdevice(priv, name, num_probed, i);
+ num_probed++;
+ }
+
+ /* print an error message if no modules were probed */
+ if (num_probed == 0) {
+ dev_err(&priv->pdev->dev, "no MODULbus modules specified, "
+ "please set the ``modules'' kernel "
+ "parameter according to your "
+ "hardware configuration\n");
+ return -ENODEV;
+ }
+
+ return mfd_add_devices(&pdev->dev, 0, priv->cells,
+ num_probed, NULL, pdev->irq);
+}
+
+/*
+ * SYSFS Attributes
+ */
+
+static ssize_t mbus_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct cmodio_device *priv = dev_get_drvdata(dev);
+
+ return snprintf(buf, PAGE_SIZE, "%x\n", priv->hex);
+}
+
+static DEVICE_ATTR(modulbus_number, S_IRUGO, mbus_show, NULL);
+
+static struct attribute *cmodio_sysfs_attrs[] = {
+ &dev_attr_modulbus_number.attr,
+ NULL,
+};
+
+static const struct attribute_group cmodio_sysfs_attr_group = {
+ .attrs = cmodio_sysfs_attrs,
+};
+
+/*
+ * PCI Driver
+ */
+
+static int __devinit cmodio_pci_probe(struct pci_dev *dev,
+ const struct pci_device_id *id)
+{
+ struct cmodio_device *priv;
+ int ret;
+
+ priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+ if (!priv) {
+ dev_err(&dev->dev, "unable to allocate private data\n");
+ ret = -ENOMEM;
+ goto out_return;
+ }
+
+ pci_set_drvdata(dev, priv);
+ priv->pdev = dev;
+
+ /* Hardware Initialization */
+ ret = pci_enable_device(dev);
+ if (ret) {
+ dev_err(&dev->dev, "unable to enable device\n");
+ goto out_free_priv;
+ }
+
+ pci_set_master(dev);
+ ret = pci_request_regions(dev, DRV_NAME);
+ if (ret) {
+ dev_err(&dev->dev, "unable to request regions\n");
+ goto out_pci_disable_device;
+ }
+
+ /* Onboard configuration registers */
+ priv->ctrl = pci_ioremap_bar(dev, 4);
+ if (!priv->ctrl) {
+ dev_err(&dev->dev, "unable to remap onboard regs\n");
+ ret = -ENOMEM;
+ goto out_pci_release_regions;
+ }
+
+ /* Read the hex switch on the carrier board */
+ priv->hex = ioread8(&priv->ctrl->int_enable);
+
+ /* Add the MODULbus number (hex switch value) to the device's sysfs */
+ ret = sysfs_create_group(&dev->dev.kobj, &cmodio_sysfs_attr_group);
+ if (ret) {
+ dev_err(&dev->dev, "unable to create sysfs attributes\n");
+ goto out_unmap_ctrl;
+ }
+
+ /*
+ * Disable all interrupt lines, each submodule will enable its
+ * own interrupt line if needed
+ */
+ iowrite8(0xf, &priv->ctrl->int_disable);
+
+ /* Register drivers for all submodules */
+ ret = cmodio_probe_submodules(priv);
+ if (ret) {
+ dev_err(&dev->dev, "unable to probe submodules\n");
+ goto out_sysfs_remove_group;
+ }
+
+ return 0;
+
+out_sysfs_remove_group:
+ sysfs_remove_group(&dev->dev.kobj, &cmodio_sysfs_attr_group);
+out_unmap_ctrl:
+ iounmap(priv->ctrl);
+out_pci_release_regions:
+ pci_release_regions(dev);
+out_pci_disable_device:
+ pci_disable_device(dev);
+out_free_priv:
+ kfree(priv);
+out_return:
+ return ret;
+}
+
+static void __devexit cmodio_pci_remove(struct pci_dev *dev)
+{
+ struct cmodio_device *priv = pci_get_drvdata(dev);
+
+ mfd_remove_devices(&dev->dev);
+ sysfs_remove_group(&dev->dev.kobj, &cmodio_sysfs_attr_group);
+ iounmap(priv->ctrl);
+ pci_release_regions(dev);
+ pci_disable_device(dev);
+ kfree(priv);
+}
+
+/* The list of devices that this module will support */
+static DEFINE_PCI_DEVICE_TABLE(cmodio_pci_ids) = {
+ { PCI_VENDOR_ID_PLX, PCI_DEVICE_ID_PLX_9030, PCI_VENDOR_ID_JANZ, 0x0101 },
+ { PCI_VENDOR_ID_PLX, PCI_DEVICE_ID_PLX_9050, PCI_VENDOR_ID_JANZ, 0x0100 },
+ { 0, }
+};
+MODULE_DEVICE_TABLE(pci, cmodio_pci_ids);
+
+static struct pci_driver cmodio_pci_driver = {
+ .name = DRV_NAME,
+ .id_table = cmodio_pci_ids,
+ .probe = cmodio_pci_probe,
+ .remove = __devexit_p(cmodio_pci_remove),
+};
+
+/*
+ * Module Init / Exit
+ */
+
+static int __init cmodio_init(void)
+{
+ return pci_register_driver(&cmodio_pci_driver);
+}
+
+static void __exit cmodio_exit(void)
+{
+ pci_unregister_driver(&cmodio_pci_driver);
+}
+
+MODULE_AUTHOR("Ira W. Snyder <iws-lulEs6mt1IksTUYHLfqkUA@public.gmane.org>");
+MODULE_DESCRIPTION("Janz CMOD-IO PCI MODULbus Carrier Board Driver");
+MODULE_LICENSE("GPL");
+
+module_init(cmodio_init);
+module_exit(cmodio_exit);
diff --git a/include/linux/mfd/janz.h b/include/linux/mfd/janz.h
new file mode 100644
index 0000000..e9994c4
--- /dev/null
+++ b/include/linux/mfd/janz.h
@@ -0,0 +1,54 @@
+/*
+ * Common Definitions for Janz MODULbus devices
+ *
+ * Copyright (c) 2010 Ira W. Snyder <iws-lulEs6mt1IksTUYHLfqkUA@public.gmane.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; either version 2 of the License, or (at your
+ * option) any later version.
+ */
+
+#ifndef JANZ_H
+#define JANZ_H
+
+struct janz_platform_data {
+ /* MODULbus Module Number */
+ unsigned int modno;
+};
+
+/* PLX bridge chip onboard registers */
+struct janz_cmodio_onboard_regs {
+ u8 unused1;
+
+ /*
+ * Read access: interrupt status
+ * Write access: interrupt disable
+ */
+ u8 int_disable;
+ u8 unused2;
+
+ /*
+ * Read access: MODULbus number (hex switch)
+ * Write access: interrupt enable
+ */
+ u8 int_enable;
+ u8 unused3;
+
+ /* write-only */
+ u8 reset_assert;
+ u8 unused4;
+
+ /* write-only */
+ u8 reset_deassert;
+ u8 unused5;
+
+ /* read-write access to serial EEPROM */
+ u8 eep;
+ u8 unused6;
+
+ /* write-only access to EEPROM chip select */
+ u8 enid;
+};
+
+#endif /* JANZ_H */
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index 9f688d2..5b42bb2 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -2718,3 +2718,5 @@
#define PCI_DEVICE_ID_RME_DIGI32 0x9896
#define PCI_DEVICE_ID_RME_DIGI32_PRO 0x9897
#define PCI_DEVICE_ID_RME_DIGI32_8 0x9898
+
+#define PCI_VENDOR_ID_JANZ 0x13c3
--
1.5.4.3
^ permalink raw reply related [flat|nested] 10+ messages in thread
end of thread, other threads:[~2010-03-29 16:58 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-03-18 16:38 [PATCH 1/3] mfd: add support for Janz CMOD-IO PCI MODULbus Carrier Board Ira W. Snyder
2010-03-19 9:13 ` Wolfgang Grandegger
2010-03-19 15:13 ` Ira W. Snyder
[not found] ` <20100319151326.GA13672-lulEs6mt1IksTUYHLfqkUA@public.gmane.org>
2010-03-19 15:35 ` Wolfgang Grandegger
2010-03-19 16:38 ` Samuel Ortiz
[not found] ` <20100319163849.GB30409-jcdQHdrhKHMdnm+yROfE0A@public.gmane.org>
2010-03-19 18:22 ` Ira W. Snyder
2010-03-25 22:59 ` Samuel Ortiz
[not found] ` <20100325225929.GA20618-jcdQHdrhKHMdnm+yROfE0A@public.gmane.org>
2010-03-25 23:22 ` Ira W. Snyder
2010-03-26 0:26 ` Samuel Ortiz
-- strict thread matches above, loose matches on Subject: below --
2010-03-29 16:58 Ira W. Snyder
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).