On Mon, Sep 21, 2009 at 03:56:11PM +0200, Richard Röjfors wrote:
> This patch adds support for the Xilinx XPS IIC Bus Interface.
>
> The driver uses the dynamic mode, supporting to put several
> I2C messages in the FIFO to reduce the number of interrupts.
>
> It has the same feature as ocores, it can be passed a list
> of devices that will be added when the bus is probed.
>
> Signed-off-by: Richard Röjfors <[email protected]>
> ---
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index 8206442..6b291e8 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -433,6 +433,16 @@ config I2C_OCORES
> This driver can also be built as a module. If so, the module
> will be called i2c-ocores.
>
> +config I2C_XILINX
> + tristate "Xilinx I2C Controller"
> + depends on EXPERIMENTAL && HAS_IOMEM
> + help
> + If you say yes to this option, support will be included for the
> + Xilinx I2C controller.
> +
> + This driver can also be built as a module. If so, the module
> + will be called xilinx_i2c.
> +
> config I2C_OMAP
> tristate "OMAP I2C adapter"
> depends on ARCH_OMAP
> diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
> index e654263..a2ce5b8 100644
> --- a/drivers/i2c/busses/Makefile
> +++ b/drivers/i2c/busses/Makefile
> @@ -40,6 +40,7 @@ obj-$(CONFIG_I2C_IXP2000) += i2c-ixp2000.o
> obj-$(CONFIG_I2C_MPC) += i2c-mpc.o
> obj-$(CONFIG_I2C_MV64XXX) += i2c-mv64xxx.o
> obj-$(CONFIG_I2C_OCORES) += i2c-ocores.o
> +obj-$(CONFIG_I2C_XILINX) += i2c-xiic.o
> obj-$(CONFIG_I2C_OMAP) += i2c-omap.o
> obj-$(CONFIG_I2C_PASEMI) += i2c-pasemi.o
> obj-$(CONFIG_I2C_PNX) += i2c-pnx.o
> diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
> new file mode 100644
> index 0000000..7b1e618
> --- /dev/null
> +++ b/drivers/i2c/busses/i2c-xiic.c
> @@ -0,0 +1,800 @@
> +/*
> + * i2c-xiic.c
> + * Copyright (c) 2009 Intel Corporation
is this the right copyirhgt entity?
> + * 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., 675 Mass Ave, Cambridge, MA 02139, USA.
> + */
> +
> +/* Supports:
> + * Xilinx IIC
> + */
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/errno.h>
> +#include <linux/platform_device.h>
> +#include <linux/i2c.h>
> +#include <linux/interrupt.h>
> +#include <linux/wait.h>
> +#include <linux/i2c-xiic.h>
> +#include <linux/io.h>
> +
> +#define DRIVER_NAME "xiic-i2c"
> +
> +struct xiic_i2c {
> + void __iomem *base;
> + wait_queue_head_t wait;
> + struct i2c_adapter adap;
> + struct i2c_msg *tx_msg;
> + spinlock_t lock; /* mutual exclusion */
> + unsigned int tx_pos;
> + unsigned int nmsgs;
> + int state; /* see STATE_ */
> +
> + struct i2c_msg *rx_msg; /* current RX message */
> + int rx_pos;
> +};
It would be nice (but not esential) to see some documentation
on this structure.
> +#define STATE_DONE 0x00
> +#define STATE_ERROR 0x01
> +#define STATE_START 0x02
it would be great to see these as an enum (say, enum xilinx_i2c_state)
to represent the state.
> +#define xiic_irq_dis(i2c, mask) \
> + xiic_setreg32(i2c, XIIC_IIER_OFFSET, \
> + xiic_getreg32(i2c, XIIC_IIER_OFFSET) & ~(mask))
These should be 'static inline' functions, that would avoid the
whole \ continuation business.
> +static irqreturn_t xiic_isr(int irq, void *dev_id)
> +{
> + struct xiic_i2c *i2c = dev_id;
blank sould be here.
> +static int __devinit xiic_i2c_probe(struct platform_device *pdev)
> +{
> + struct xiic_i2c *i2c;
> + struct xiic_i2c_platform_data *pdata;
> + struct resource *res;
> + int ret, irq;
> + u8 i;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!res)
> + return -ENODEV;
> +
> + irq = platform_get_irq(pdev, 0);
> + if (irq < 0)
> + return -ENODEV;
-ENODEV is not a good error here, you won't get any report from the
driver core binding if you return this or (iirc, -EIO). Personally
I like returning -ENOENT, but this can be objectionable to others as
it is an file-system error.
> + pdata = (struct xiic_i2c_platform_data *) pdev->dev.platform_data;
> + if (!pdata)
> + return -ENODEV;
how about -EINVAL here.
> + i2c = kzalloc(sizeof(*i2c), GFP_KERNEL);
> + if (!i2c)
> + return -ENOMEM;
> +
> + if (!request_mem_region(res->start, resource_size(res), pdev->name)) {
> + dev_err(&pdev->dev, "Memory region busy\n");
> + ret = -EBUSY;
> + goto request_mem_failed;
> + }
> +
> + i2c->base = ioremap(res->start, resource_size(res));
> + if (!i2c->base) {
> + dev_err(&pdev->dev, "Unable to map registers\n");
> + ret = -EIO;
> + goto map_failed;
> + }
> +
> + /* hook up driver to tree */
> + platform_set_drvdata(pdev, i2c);
> + i2c->adap = xiic_adapter;
> + i2c_set_adapdata(&i2c->adap, i2c);
> + i2c->adap.dev.parent = &pdev->dev;
> +
> + xiic_reinit(i2c);
> +
> + spin_lock_init(&i2c->lock);
> + init_waitqueue_head(&i2c->wait);
> + ret = request_irq(irq, xiic_isr, 0, pdev->name, i2c);
> + if (ret) {
> + dev_err(&pdev->dev, "Cannot claim IRQ\n");
> + goto request_irq_failed;
> + }
Prints here but not for missing resources? at least you could
make a generic 'resource missing' error to tell users.
> + /* add i2c adapter to i2c tree */
> + ret = i2c_add_adapter(&i2c->adap);
> + if (ret) {
> + dev_err(&pdev->dev, "Failed to add adapter\n");
> + goto add_adapter_failed;
> + }
> +
> + /* add in known devices to the bus */
> + for (i = 0; i < pdata->num_devices; i++)
> + i2c_new_device(&i2c->adap, pdata->devices + i);
> +
> + return 0;
> +
> +add_adapter_failed:
> + free_irq(irq, i2c);
> +request_irq_failed:
> + xiic_deinit(i2c);
> + iounmap(i2c->base);
> +map_failed:
> + release_mem_region(res->start, resource_size(res));
> +request_mem_failed:
> + kfree(i2c);
> +
> + return ret;
> +}
> +++ b/include/linux/i2c-xiic.h
> @@ -0,0 +1,31 @@
> +/*
> + * i2c-xiic.h
> + * Copyright (c) 2009 Intel Corporation
is this the right copyright?
> + * 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., 675 Mass Ave, Cambridge, MA 02139, USA.
> + */
> +
> +/* Supports:
> + * Xilinx IIC
> + */
> +
> +#ifndef _LINUX_I2C_XIIC_H
> +#define _LINUX_I2C_XIIC_H
> +
> +struct xiic_i2c_platform_data {
> + u8 num_devices; /* number of devices in the devices list */
> + struct i2c_board_info const *devices; /* devices connected to the bus */
> +};
kernel style documentation here people
> +#endif /* _LINUX_I2C_XIIC_H */
--
Ben
Q: What's a light-year?
A: One-third less calories than a regular year.
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
