[lm-sensors] [PATCH] OpenCores I2C bus driver

Rudolf Marek r.marek at sh.cvut.cz
Sun May 14 11:09:16 CEST 2006


Hello,

Sorry for delay we all had lot of other stuff to do. I checked your driver and
it seems very good. Please check my comments in the code.

Regards
Rudolf


> Index: linux/drivers/i2c/busses/Kconfig
> ===================================================================
> --- linux.orig/drivers/i2c/busses/Kconfig	2006-04-19 10:24:35.000000000 +0200
> +++ linux/drivers/i2c/busses/Kconfig	2006-04-19 10:24:38.000000000 +0200
> @@ -517,4 +517,15 @@
>  	  This driver can also be built as a module.  If so, the module
>  	  will be called i2c-mv64xxx.
>  
> +config I2C_OCORES
> +	tristate "OpenCores I2C Controller"
> +	depends on I2C

Really no experimental? How long is driver used?

> +	help
> +	  If you say yes to this option, support will be included for the
> +	  OpenCores I2C controller. For details see
> +	  http://www.opencores.org/projects.cgi/web/i2c/overview
> +
> +	  This driver can also be built as a module.  If so, the module
> +	  will be called i2c-ocores.
> +
>  endmenu
> Index: linux/drivers/i2c/busses/Makefile
> ===================================================================
> --- linux.orig/drivers/i2c/busses/Makefile	2006-03-20 06:53:29.000000000 +0100
> +++ linux/drivers/i2c/busses/Makefile	2006-04-19 10:24:38.000000000 +0200
> @@ -40,6 +40,7 @@
>  obj-$(CONFIG_I2C_VIA)		+= i2c-via.o
>  obj-$(CONFIG_I2C_VIAPRO)	+= i2c-viapro.o
>  obj-$(CONFIG_I2C_VOODOO3)	+= i2c-voodoo3.o
> +obj-$(CONFIG_I2C_OCORES)	+= i2c-ocores.o

I think rest of files is alphabetic order

>  obj-$(CONFIG_SCx200_ACB)	+= scx200_acb.o
>  obj-$(CONFIG_SCx200_I2C)	+= scx200_i2c.o
>  
> Index: linux/drivers/i2c/busses/i2c-ocores.c
> ===================================================================
> --- /dev/null	1970-01-01 00:00:00.000000000 +0000
> +++ linux/drivers/i2c/busses/i2c-ocores.c	2006-04-21 10:51:05.000000000 +0200
> @@ -0,0 +1,358 @@
> +/*
> + * i2c-ocores.c: I2C bus driver for OpenCores I2C controller
> + * (http://www.opencores.org/projects.cgi/web/i2c/overview).
> + *
> + * Peter Korsgaard <jacmet at sunsite.dk>

Should have (C) and year

> + *
> + * This file is licensed under the terms of the GNU General Public License
> + * version 2.  This program is licensed "as is" without any warranty of any
> + * kind, whether express or implied.
> + */
> +
> +#include <linux/config.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/sched.h>
> +#include <linux/init.h>
> +#include <linux/errno.h>
> +#include <linux/platform_device.h>
> +
> +#include <asm/io.h>
> +#include <linux/i2c.h>
> +#include <linux/interrupt.h>
> +#include <linux/wait.h>
> +#include <linux/i2c-ocores.h>
> +
> +struct ocores_i2c {
> +	void __iomem *base;
> +	int regstep;
> +	wait_queue_head_t wait;
> +	struct i2c_adapter adap;
> +	struct i2c_msg *msg;
> +	int pos;
> +	int nmsgs;
> +	int state; /* see STATE_ */
> +};
> +
> +/* registers */
> +#define OCI2C_PRELOW 		0
> +#define OCI2C_PREHIGH 		1
> +#define OCI2C_CONTROL 		2
> +#define OCI2C_DATA 		3
> +#define OCI2C_CMD     		4
> +#define OCI2C_STATUS  		4
> +
> +#define OCI2C_CMD_START		0x91
> +#define OCI2C_CMD_STOP		0x41
> +#define OCI2C_CMD_READ		0x21
> +#define OCI2C_CMD_WRITE		0x11
> +#define OCI2C_CMD_READ_ACK	0x21
> +#define OCI2C_CMD_READ_NACK	0x29
> +#define OCI2C_CMD_IACK		0x01
> +
> +#define OCI2C_STAT_BUSY		0x40
> +#define OCI2C_STAT_TIP		0x02
> +#define OCI2C_STAT_NACK		0x80
> +#define OCI2C_STAT_ARBLOST	0x20
> +#define OCI2C_STAT_IF		0x01
> +
> +#define STATE_DONE  0
> +#define STATE_START 1
> +#define STATE_WRITE 2
> +#define STATE_READ  3
> +#define STATE_ERROR 4
> +
> +static __inline__ void oc_setreg(struct ocores_i2c *i2c, int reg, u8 value)
> +{
> +	iowrite8(value, i2c->base + reg * i2c->regstep);
> +}
> +
> +static __inline__ u8 oc_getreg(struct ocores_i2c *i2c, int reg)
> +{
> +	return ioread8(i2c->base + reg * i2c->regstep);
> +}
> +
> +static void ocores_process(struct ocores_i2c *i2c)
> +{
> +	struct i2c_msg *msg = i2c->msg;
> +	u8 stat = oc_getreg(i2c, OCI2C_STATUS);
> +
> +	if ((i2c->state == STATE_DONE) || (i2c->state == STATE_ERROR))
> +	{
> +		/* stop has been sent */
> +		oc_setreg(i2c, OCI2C_CMD, OCI2C_CMD_IACK);
> +		wake_up_interruptible(&i2c->wait);
> +		return;
> +	}
> +
> +	/* error? */
> +	if (stat & OCI2C_STAT_ARBLOST)
> +	{
> +		i2c->state = STATE_ERROR;
> +		oc_setreg(i2c, OCI2C_CMD, OCI2C_CMD_STOP);
> +		return;
> +	}
> +
> +	if ((i2c->state == STATE_START) || (i2c->state == STATE_WRITE))
> +	{
> +		i2c->state =
> +			(msg->flags & I2C_M_RD) ? STATE_READ : STATE_WRITE;
> +
> +		if (stat & OCI2C_STAT_NACK)
> +		{
> +			i2c->state = STATE_ERROR;
> +			oc_setreg(i2c, OCI2C_CMD, OCI2C_CMD_STOP);
> +			return;
> +		}
> +	}
> +	else
> +		msg->buf[i2c->pos++] = oc_getreg(i2c, OCI2C_DATA);
> +
> +	/* end of msg? */
> +	if (i2c->pos == msg->len)
> +	{
> +		i2c->nmsgs--;
> +		i2c->msg++;
> +		i2c->pos = 0;
> +		msg = i2c->msg;
> +
> +		if (i2c->nmsgs) /* end? */
> +		{
> +			/* send start? */
> +			if (!(msg->flags & I2C_M_NOSTART))
> +			{
> +				u8 addr = (msg->addr << 1);
> +
> +				if (msg->flags & I2C_M_RD)
> +					addr |= 1;
> +
> +				i2c->state = STATE_START;
> +
> +				oc_setreg(i2c, OCI2C_DATA, addr);
> +				oc_setreg(i2c, OCI2C_CMD,  OCI2C_CMD_START);
> +				return;
> +			}
> +			else
> +				i2c->state = (msg->flags & I2C_M_RD)
> +					? STATE_READ : STATE_WRITE;
> +		}
> +		else
> +		{
> +			i2c->state = STATE_DONE;
> +			oc_setreg(i2c, OCI2C_CMD, OCI2C_CMD_STOP);
> +			return;
> +		}
> +	}
> +
> +	if (i2c->state == STATE_READ)
> +	{
> +		oc_setreg(i2c, OCI2C_CMD, i2c->pos == (msg->len-1) ?
> +			  OCI2C_CMD_READ_NACK : OCI2C_CMD_READ_ACK);
> +	}
> +	else
> +	{
> +		oc_setreg(i2c, OCI2C_DATA, msg->buf[i2c->pos++]);
> +		oc_setreg(i2c, OCI2C_CMD, OCI2C_CMD_WRITE);
> +	}
> +}
> +
> +static irqreturn_t ocores_isr(int irq, void *dev_id, struct pt_regs *regs)
> +{
> +	struct ocores_i2c *i2c = dev_id;
> +
> +	ocores_process(i2c);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int ocores_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
> +{
> +	struct ocores_i2c *i2c = i2c_get_adapdata(adap);
> +
> +	i2c->msg   = msgs;
> +	i2c->pos   = 0;
> +	i2c->nmsgs = num;
> +	i2c->state = STATE_START;
> +
> +	oc_setreg(i2c, OCI2C_DATA,
> +			(i2c->msg->addr << 1) |
> +			((i2c->msg->flags & I2C_M_RD) ? 1:0));
> +
> +	oc_setreg(i2c, OCI2C_CMD, OCI2C_CMD_START);
> +
> +	if (wait_event_interruptible_timeout(i2c->wait,
> +					     (i2c->state == STATE_ERROR) ||
> +					     (i2c->state == STATE_DONE), HZ*5))
> +		return (i2c->state == STATE_DONE) ? num : -EIO;
> +	else
> +		return -ETIMEDOUT;
> +}
> +
> +static void ocores_init(struct ocores_i2c *i2c,
> +			struct ocores_i2c_platform_data *pdata)
> +{
> +	int prescale;
> +
> +	/* make sure the device is disabled */
> +	oc_setreg(i2c, OCI2C_CONTROL, 0);

Well I started to read the driver from a flow and this is the first occurrence
to write to some register :). I think much better handling of reserved bits is
to read them from device change bits I know and then write all back.

I'm also surprised that the device has no ID/revision regs which might be handy
for future extensions

> +
> +	prescale = (pdata->clock_khz / (5*100)) - 1;

No checks for evil values?

> +	oc_setreg(i2c, OCI2C_PRELOW,  prescale & 0xff);
> +	oc_setreg(i2c, OCI2C_PREHIGH, prescale >> 8);
> +
> +	/* Init the device */
> +	oc_setreg(i2c, OCI2C_CMD,     OCI2C_CMD_IACK);
> +	oc_setreg(i2c, OCI2C_CONTROL, 0xc0);

and here again
> +}
> +
> +
> +static u32 ocores_func(struct i2c_adapter *adap)
> +{
> +	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
> +}
> +
> +static struct i2c_algorithm ocores_algorithm = {
> +	.master_xfer	= ocores_xfer,
> +	.functionality	= ocores_func,
> +};
> +
> +static struct i2c_adapter ocores_adapter = {
> +	.owner		= THIS_MODULE,
> +	.name		= "i2c-ocores",
> +	.class		= I2C_CLASS_HWMON,
> +	.algo		= &ocores_algorithm,
> +	.timeout	= 2,
> +	.retries	= 1
> +};
> +
> +
> +static int __devinit ocores_i2c_probe(struct platform_device *pdev)
> +{
> +	struct ocores_i2c* i2c;
> +	struct ocores_i2c_platform_data* pdata;
> +	struct resource *res, *res2;
> +	int ret;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res)
> +		return -ENODEV;
> +
> +	res2 = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> +	if (!res2)
> +		return -ENODEV;
> +
> +	pdata = (struct ocores_i2c_platform_data*) pdev->dev.platform_data;
> +	if (!pdata)
> +		return -ENODEV;
> +
> +	i2c = kzalloc(sizeof(*i2c), GFP_KERNEL);
> +	if (!i2c)
> +		return -ENOMEM;
> +
> +	if (!request_mem_region(res->start, res->end - res->start + 1,
> +				pdev->name)) {
> +		dev_err(&pdev->dev, "Memory region busy\n");
> +		ret = -EBUSY;
> +		goto request_mem_failed;
> +	}
> +
> +	i2c->base = ioremap(res->start, res->end - res->start + 1);
> +	if (!i2c->base)
> +	{
> +		dev_err(&pdev->dev, "Unable to map registers\n");
> +		ret = -EIO;
> +		goto map_failed;
> +	}
> +
> +	i2c->regstep = pdata->regstep;
> +	ocores_init(i2c, pdata);
> +
> +	init_waitqueue_head(&i2c->wait);
> +	ret = request_irq(res2->start, ocores_isr, 0,
> +			  pdev->name, i2c);
> +	if (ret)
> +	{
> +		dev_err(&pdev->dev, "Cannot claim IRQ\n");
> +		goto request_irq_failed;
> +	}
> +
> +	/* hook up driver to tree */
> +	platform_set_drvdata(pdev, i2c);
> +	i2c->adap = ocores_adapter;
> +	i2c_set_adapdata(&i2c->adap, i2c);
> +	i2c->adap.dev.parent = &pdev->dev;
> +
> +	/* 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;
> +	}
> +
> +	return 0;
> +
> + add_adapter_failed:
> +	free_irq(res2->start, i2c);
> + request_irq_failed:
> +	iounmap(i2c->base);
> + map_failed:
> +	release_mem_region(res->start, res->end - res->start + 1);
> + request_mem_failed:
> +	kfree(i2c);
> +
> +	return ret;
> +}
> +
> +static int __devexit ocores_i2c_remove(struct platform_device* pdev)
> +{
> +	struct ocores_i2c *i2c = platform_get_drvdata(pdev);
> +	struct resource *res;
> +
> +	/* disable i2c logic */
> +	oc_setreg(i2c, OCI2C_CONTROL, 0);
> +
> +	/* remove adapter & data */
> +	i2c_del_adapter(&i2c->adap);
> +	platform_set_drvdata(pdev, NULL);
> +
> +	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> +	if (res)
> +		free_irq(res->start, i2c);
> +
> +	iounmap(i2c->base);
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (res)
> +		release_mem_region(res->start, res->end - res->start + 1);
> +
> +	kfree(i2c);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver ocores_i2c_driver = {
> +	.probe  = ocores_i2c_probe,
> +	.remove = ocores_i2c_remove,
> +	.driver = {
> +		.owner = THIS_MODULE,
> +		.name = "ocores-i2c",
> +	},
> +};
> +
> +static int __init ocores_i2c_init(void)
> +{
> +	return platform_driver_register(&ocores_i2c_driver);
> +}
> +
> +static void __exit ocores_i2c_exit(void)
> +{
> +	platform_driver_unregister(&ocores_i2c_driver);
> +}
> +
> +module_init(ocores_i2c_init);
> +module_exit(ocores_i2c_exit);
> +
> +MODULE_AUTHOR("Peter Korsgaard <jacmet at sunsite.dk>");
> +MODULE_DESCRIPTION("OpenCores I2C bus driver");
> +MODULE_LICENSE("GPL");
> Index: linux/include/linux/i2c-ocores.h
> ===================================================================
> --- /dev/null	1970-01-01 00:00:00.000000000 +0000
> +++ linux/include/linux/i2c-ocores.h	2006-04-21 10:54:43.000000000 +0200
> @@ -0,0 +1,20 @@
> +/*
> + * i2c-ocores.h - definitions for the i2c-ocores interface
> + *
> + * Peter Korsgaard <jacmet at sunsite.dk>
> + *

 (C) 2006 perhaps

> + * This file is licensed under the terms of the GNU General Public License
> + * version 2.  This program is licensed "as is" without any warranty of any
> + * kind, whether express or implied.
> + */
> +
> +#ifndef _LINUX_I2C_OCORES_H
> +#define _LINUX_I2C_OCORES_H
> +
> +struct ocores_i2c_platform_data {
> +	u32 regstep;   /* distance between registers */
> +	u32 clock_khz; /* input clock in KHz */
> +};
> +
> +#endif /* _LINUX_I2C_OCORES_H */
> 




More information about the lm-sensors mailing list