On Fri, Aug 06, 2010 at 01:33:24PM +0900, Masayuki Ohtak wrote:
> I2C driver of Topcliff PCH
> 
> Topcliff PCH is the platform controller hub that is going to be used in
> Intel's upcoming general embedded platform. All IO peripherals in
> Topcliff PCH are actually devices sitting on AMBA bus. 
> Topcliff PCH has I2C I/F. Using this I/F, it is able to access system
> devices connected to I2C.
> 
> Signed-off-by: Masayuki Ohtake <masa-k...@dsn.okisemi.com>


Why not run this by the i2c maintainer and developers as well?

You do know about the scripts/get_maintainer.pl program to tell you who
to copy when sending a patch, right?  Please use that.

> 
> ---
>  drivers/i2c/busses/Kconfig   |    8 +
>  drivers/i2c/busses/Makefile  |    3 +
>  drivers/i2c/busses/i2c-pch.c |  910 
> ++++++++++++++++++++++++++++++++++++++++++
>  drivers/i2c/busses/i2c-pch.h |  147 +++++++
>  drivers/i2c/i2c-dev.c        |   21 +
>  5 files changed, 1089 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/i2c/busses/i2c-pch.c
>  create mode 100644 drivers/i2c/busses/i2c-pch.h
> 
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index 5f318ce..98e7201 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -7,6 +7,14 @@ menu "I2C Hardware Bus support"
>  comment "PC SMBus host controller drivers"
>       depends on PCI
>  
> +config PCH_I2C
> +     tristate "PCH I2C"
> +     depends on PCI
> +     help
> +       This driver is for PCH I2C of Topcliff which is an IOH for x86
> +       embedded processor.
> +       This driver can access PCH I2C bus device.
> +
>  config I2C_ALI1535
>       tristate "ALI 1535"
>       depends on PCI
> diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
> index 302c551..3e6b8d6 100644
> --- a/drivers/i2c/busses/Makefile
> +++ b/drivers/i2c/busses/Makefile
> @@ -75,3 +75,6 @@ obj-$(CONFIG_SCx200_I2C)    += scx200_i2c.o
>  ifeq ($(CONFIG_I2C_DEBUG_BUS),y)
>  EXTRA_CFLAGS += -DDEBUG
>  endif
> +
> +obj-$(CONFIG_PCH_I2C) += pch_i2c.o
> +pch_i2c-objs := i2c-pch.o

Why not just name your file i2c-pci.c and then you don't need this
two-step Makefile mess?

> diff --git a/drivers/i2c/busses/i2c-pch.c b/drivers/i2c/busses/i2c-pch.c
> new file mode 100644
> index 0000000..7939781
> --- /dev/null
> +++ b/drivers/i2c/busses/i2c-pch.c
> @@ -0,0 +1,910 @@
> +/*
> + * Copyright (C) 2010 OKI SEMICONDUCTOR Co., LTD.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; version 2 of the License.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307, USA.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/delay.h>
> +#include <linux/init.h>
> +#include <linux/errno.h>
> +#include <linux/i2c.h>
> +#include <linux/fs.h>
> +#include <linux/io.h>
> +#include <linux/types.h>
> +#include <linux/interrupt.h>
> +#include <linux/jiffies.h>
> +#include <linux/pci.h>
> +#include <linux/mutex.h>
> +#include <linux/ktime.h>
> +
> +#include "i2c-pch.h"

Why do you need a .h file for this driver?

<snip>

> diff --git a/drivers/i2c/i2c-dev.c b/drivers/i2c/i2c-dev.c
> index f4110aa..53e13de 100644
> --- a/drivers/i2c/i2c-dev.c
> +++ b/drivers/i2c/i2c-dev.c
> @@ -36,6 +36,7 @@
>  #include <linux/i2c-dev.h>
>  #include <linux/jiffies.h>
>  #include <linux/uaccess.h>
> +#include "busses/i2c-pch.h"

Note how no other i2c driver needs to be included here...

>  static struct i2c_driver i2cdev_driver;
>  
> @@ -372,6 +373,12 @@ static long i2cdev_ioctl(struct file *file, unsigned int 
> cmd, unsigned long arg)
>       struct i2c_client *client = file->private_data;
>       unsigned long funcs;
>  
> +     unsigned long pch_mode;
> +     int ret;
> +
> +     struct i2c_msg msg;
> +     unsigned char msgbuf[1];
> +
>       dev_dbg(&client->adapter->dev, "ioctl, cmd=0x%02x, arg=0x%02lx\n",
>               cmd, arg);
>  
> @@ -427,6 +434,20 @@ static long i2cdev_ioctl(struct file *file, unsigned int 
> cmd, unsigned long arg)
>                */
>               client->adapter->timeout = msecs_to_jiffies(arg * 10);
>               break;
> +     case I2C_MODE_SEL:
> +             pch_mode = arg;
> +
> +             if (pch_mode <= 4) {
> +                     msgbuf[0] = pch_mode;
> +                     msg.buf = msgbuf;
> +                     msg.len = 1;
> +                     msg.flags = 0;
> +                     ret = i2c_transfer(client->adapter, &msg, 1);
> +             } else {
> +                     printk(KERN_ERR "I2C mode sel:Invalid mode\n");
> +                     ret = -EINVAL;
> +             }
> +             return ret;

I really doubt you are allowed to add a new ioctl to the core i2c layer.
You will have to run this by the i2c maintainer to get it accepted.

good luck,

greg k-h
_______________________________________________
MeeGo-dev mailing list
MeeGo-dev@meego.com
http://lists.meego.com/listinfo/meego-dev

Reply via email to