On 03/08/10 15:34, Alan Cox wrote:
> (And the correct patch attached this time)
> 
> From: Wen Wang <[email protected]>
> 
> Initial release of the driver. Updated and verified on hardware.
> 
> Cleaned up as follows
> 
> Alan Cox:
>    Squash down the switches into tables, and use the PCI ident field. We
>    could perhaps take this further and put the platform and port number into
>    this.
>    uint32t -> u32
>    bracketing of case statements
>    spacing and '!' usage
>    Check the speed (which is now 0/1/2) is valid and ignore otherwise.
> 
> Arjan van de Ven:
>    Initial power management hooks
> 
> 
> Signed-off-by: Wen Wang <[email protected]>
> Signed-off-by: Arjan van de Ven <[email protected]>
> Signed-off-by: Alan Cox <[email protected]>
> ---
> 
>  drivers/i2c/busses/Kconfig    |    9 
>  drivers/i2c/busses/Makefile   |    1 
>  drivers/i2c/busses/i2c-mrst.c | 1082 
> +++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 1092 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/i2c/busses/i2c-mrst.c
> 
> 
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index 15a9702..46b9acb 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -420,6 +420,15 @@ config I2C_IXP2000
>         This driver is deprecated and will be dropped soon. Use i2c-gpio
>         instead.
>  
> +config I2C_MRST
> +     tristate "Intel Moorestown/Medfield Platform I2C controller"
> +     help
> +       Say Y here if you have an Intel Moorestown/Medfield platform I2C
> +       controller.
> +
> +       This support is also available as a module. If so, the module
> +       will be called i2c-mrst.

I would much prefer this to be called i2c-moorsetown, we have modern
systems which can handle >8 character names.

> diff --git a/drivers/i2c/busses/i2c-mrst.c b/drivers/i2c/busses/i2c-mrst.c
> new file mode 100644
> index 0000000..79f45fb
> --- /dev/null
> +++ b/drivers/i2c/busses/i2c-mrst.c
> @@ -0,0 +1,1082 @@
> +/*
> + * Support for Moorestown/Medfield I2C chip
> + *
> + * Copyright (c) 2009 Intel Corporation.
> + * Copyright (c) 2009 Synopsys. Inc.

Hmm, if this is a synopsys block, then is it a standard one and if so
can we get some standard support?

> +struct mrst_i2c_private {
> +     struct i2c_adapter adap;
> +     /* Register base address */
> +     void __iomem *base;
> +     /* Speed mode */
> +     int speed;
> +     int pm_state;
> +     struct completion complete;
> +     int abort;
> +     u8 *rx_buf;
> +     int rx_buf_len;
> +     int status;
> +     struct i2c_msg *msg;
> +     enum platform_enum platform;
> +     struct mutex lock;
> +     spinlock_t slock;
> +};
> +

Would have been nice to have some sort of documentatioin

> +static int ctl_num;
> +module_param_array(speed_mode, int, &ctl_num, S_IRUGO);
> +MODULE_PARM_DESC(speed_mode, "Set the speed of the i2c interface (0-2)");
> +
> +static inline u32 mrst_i2c_read(void __iomem *reg)
> +{
> +     return __raw_readl(reg);
> +}

ioremap, but using __raw_read and __raw_write? readl/writel
should be used. if they can't be used, then please explain
why.

> +
> +static inline void mrst_i2c_write(void __iomem *reg, u32 val)
> +{
> +     __raw_writel(val, reg);
> +}
> +

> +/**
> + * mrst_i2c_address_neq - To check if the addresses for different i2c 
> messages
> + * are equal.
> + * @p1: first i2c_msg
> + * @p2: second i2c_msg
> + *
> + * Return Values:
> + * 0  if addresses are equal
> + * 1  if not equal
> + *
> + * Within a single transfer, I2C client may need to send its address more
> + * than one time. So a check if the addresses match  is needed.
> + */
> +static inline int mrst_i2c_address_neq(const struct i2c_msg *p1,
> +                                    const struct i2c_msg *p2)
> +{
> +     if (p1->addr != p2->addr)
> +             return 1;
> +     if ((p1->flags ^ p2->flags) & I2C_M_TEN)
> +             return 1;
> +     return 0;
> +}

would have been better to return bool.



> +
> +/**
> + * xfer_write - Internal function to implement master write transfer.
> + * @adap: i2c_adapter struct pointer
> + * @buf: buffer in i2c_msg
> + * @length: number of bytes to be read
> + *
> + * Return Values:
> + * 0 if the read transfer succeeds
> + * -ETIMEDOUT        if cannot read the "raw" interrupt register
> + * -EINVAL   if transfer abort occured
> + *
> + * For every byte, a "WRITE" command will be loaded into IC_DATA_CMD prior to
> + * data transfer. The actual "write" operation will be performed when the
> + * RX_FULL interrupt signal occurs.
> + *
> + * Note there may be two interrupt signals captured, one should read
> + * IC_RAW_INTR_STAT to separate between errors and actual data.
> + */
> +static int xfer_write(struct i2c_adapter *adap,
> +                   unsigned char *buf, int length)
> +{
> +     struct mrst_i2c_private *i2c = i2c_get_adapdata(adap);
> +     int i, err;
> +
> +     mrst_i2c_write(i2c->base + IC_INTR_MASK, 0x0050);
> +     mrst_i2c_read(i2c->base + IC_CLR_INTR);
> +
> +     if (length >= 256) {
> +             dev_err(&adap->dev,
> +                     "I2C FIFO cannot support larger than 256 bytes\n");
> +             return -EMSGSIZE;
> +     }
> +
> +     INIT_COMPLETION(i2c->complete);
> +     for (i = 0; i < length; i++)
> +             mrst_i2c_write(i2c->base + IC_DATA_CMD,
> +                            (uint16_t)(*(buf + i)));

you say length in bytes, but write u16?
also, would be neater to have a u16 *buf?

> +     i2c->status = STATUS_WRITE_START;
> +     err = wait_for_completion_interruptible_timeout(&i2c->complete, HZ);
> +     if (!err) {
> +             dev_err(&adap->dev, "Timeout for ACK from I2C slave device\n");
> +             mrst_i2c_hwinit(i2c);
> +             return -ETIMEDOUT;
> +     } else {
> +             if (i2c->status == STATUS_WRITE_SUCCESS)
> +                     return 0;
> +             else
> +                     return -EIO;
> +     }
> +}


> +
> +/**
> + * mrst_i2c_probe - I2C controller initialization routine
> + * @dev: pci device
> + * @id: device id
> + *
> + * Return Values:
> + * 0         success
> + * -ENODEV   If cannot allocate pci resource
> + * -ENOMEM   If the register base remapping failed, or
> + *           if kzalloc failed
> + *
> + * Initialization steps:
> + * 1. Request for PCI resource
> + * 2. Remap the start address of PCI resource to register base
> + * 3. Request for device memory region
> + * 4. Fill in the struct members of mrst_i2c_private
> + * 5. Call mrst_i2c_hwinit() for hardware initialization
> + * 6. Register I2C adapter in i2c-core
> + */
> +static int __devinit mrst_i2c_probe(struct pci_dev *dev,
> +                                 const struct pci_device_id *id)
> +{
> +     struct mrst_i2c_private *mrst;
> +     unsigned long start, len;
> +     int err, busnum;
> +     void __iomem *base = NULL;
> +
> +     dev_dbg(&dev->dev, "Get into probe function for I2C\n");
> +     err = pci_enable_device(dev);
> +     if (err) {
> +             dev_err(&dev->dev, "Failed to enable I2C PCI device (%d)\n",
> +                     err);
> +             goto exit;
> +     }
> +
> +     /* Determine the address of the I2C area */
> +     start = pci_resource_start(dev, 0);
> +     len = pci_resource_len(dev, 0);
> +     if (!start || len == 0) {
> +             dev_err(&dev->dev, "base address not set\n");
> +             err = -ENODEV;
> +             goto exit;
> +     }
> +     dev_dbg(&dev->dev, "%s i2c resource start 0x%lx, len=%ld\n",
> +             PLATFORM, start, len);
> +
> +     err = pci_request_region(dev, 0, DRIVER_NAME);
> +     if (err) {
> +             dev_err(&dev->dev, "failed to request I2C region "
> +                     "0x%lx-0x%lx\n", start,
> +                     (unsigned long)pci_resource_end(dev, 0));
> +             goto exit;
> +     }
> +
> +     base = ioremap_nocache(start, len);
> +     if (!base) {
> +             dev_err(&dev->dev, "I/O memory remapping failed\n");
> +             err = -ENOMEM;
> +             goto fail0;
> +     }
> +
> +     /* Allocate the per-device data structure, mrst_i2c_private */
> +     mrst = kzalloc(sizeof(struct mrst_i2c_private), GFP_KERNEL);
> +     if (mrst == NULL) {
> +             dev_err(&dev->dev, "can't allocate interface\n");
> +             err = -ENOMEM;
> +             goto fail1;
> +     }
> +
> +     /* Initialize struct members */
> +     snprintf(mrst->adap.name, sizeof(&mrst->adap.name),
> +             "MRST/Medfield I2C at %lx", start);
> +     mrst->adap.owner = THIS_MODULE;
> +     mrst->adap.algo = &mrst_i2c_algorithm;
> +     mrst->adap.dev.parent = &dev->dev;
> +     mrst->base = base;
> +     mrst->speed = STANDARD;
> +     mrst->pm_state = ACTIVE;
> +     mrst->abort = 0;
> +     mrst->rx_buf_len = 0;
> +     mrst->status = STATUS_IDLE;
> +
> +     pci_set_drvdata(dev, mrst);
> +     i2c_set_adapdata(&mrst->adap, mrst);
> +
> +     mrst->adap.nr = busnum = id->driver_data;
> +     if (dev->device <= 0x0804)
> +             mrst->platform = MOORESTOWN;
> +     else
> +             mrst->platform = MEDFIELD;
> +
> +     dev_dbg(&dev->dev, "I2C%d\n", busnum);
> +
> +     if (ctl_num > busnum) {
> +             if (speed_mode[busnum] < 0 || speed_mode[busnum] >= NUM_SPEEDS)
> +                     dev_warn(&dev->dev, "invalid speed %d ignored.\n",
> +                                                     speed_mode[busnum]);
> +             else
> +                     mrst->speed = speed_mode[busnum];
> +     }
> +
> +     /* Initialize i2c controller */
> +     err = mrst_i2c_hwinit(mrst);
> +     if (err < 0) {
> +             dev_err(&dev->dev, "I2C interface initialization failed\n");
> +             goto fail2;
> +     }
> +
> +     mutex_init(&mrst->lock);
> +     init_completion(&mrst->complete);
> +     err = request_irq(dev->irq, mrst_i2c_isr, IRQF_DISABLED,
> +                       mrst->adap.name, mrst);

are you sure you want this, IRQF_DISABLED is marked DEPRECATED.

> +     if (err) {
> +             dev_err(&dev->dev, "Failed to request IRQ for I2C controller: "
> +                     "%s", mrst->adap.name);
> +             goto fail2;
> +     }

> +static void __devexit mrst_i2c_remove(struct pci_dev *dev)
> +{
> +     struct mrst_i2c_private *mrst = (struct mrst_i2c_private *)
> +                                     pci_get_drvdata(dev);

no need to cast.

> +     mrst_i2c_disable(&mrst->adap);
> +     if (i2c_del_adapter(&mrst->adap))
> +             dev_err(&dev->dev, "Failed to delete i2c adapter");
> +
> +     free_irq(dev->irq, mrst);
> +     pci_set_drvdata(dev, NULL);
> +     iounmap(mrst->base);
> +     kfree(mrst);
> +     pci_release_region(dev, 0);
> +}
> +
> +static struct pci_device_id mrst_i2c_ids[] = {
> +     /* Moorestown */
> +     {PCI_VDEVICE(INTEL, 0x0802), 0 },
> +     {PCI_VDEVICE(INTEL, 0x0803), 1 },
> +     {PCI_VDEVICE(INTEL, 0x0804), 2 },
> +     /* Medfield */
> +     {PCI_VDEVICE(INTEL, 0x0817), 3,},
> +     {PCI_VDEVICE(INTEL, 0x0818), 4 },
> +     {PCI_VDEVICE(INTEL, 0x0819), 5 },
> +     {PCI_VDEVICE(INTEL, 0x082C), 0 },
> +     {PCI_VDEVICE(INTEL, 0x082D), 1 },
> +     {PCI_VDEVICE(INTEL, 0x082E), 2 },
> +     {0,}

style, "{ "

> +};
> +MODULE_DEVICE_TABLE(pci, mrst_i2c_ids);



--
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

Reply via email to