* [PATCH v2 1/2] i2c: xiic: Add OF support for Xilinx i2c bus interface @ 2011-02-22 14:22 ` Michal Simek 0 siblings, 0 replies; 7+ messages in thread From: Michal Simek @ 2011-02-22 14:22 UTC (permalink / raw) To: linux-i2c-u79uwXL29TY76Z2rM5mHXA Cc: grant.likely-s3s/WqlpOiPyB63q8FvJNQ, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, john.williams-g5w7nrANp4BDPfheJLI6IQ, linux-kernel-u79uwXL29TY76Z2rM5mHXA, John.Linn-gjFFaj9aHVfQT0dZR+AlfA, Michal Simek Add OF support to Xilinx i2c bus interface. It is used on Microblaze systems. Signed-off-by: Michal Simek <monstr-pSz03upnqPeHXe+LvDLADg@public.gmane.org> --- drivers/i2c/busses/i2c-xiic.c | 46 ++++++++++++++++++++++++++++++++++------ 1 files changed, 39 insertions(+), 7 deletions(-) diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c index a9c419e..4166570 100644 --- a/drivers/i2c/busses/i2c-xiic.c +++ b/drivers/i2c/busses/i2c-xiic.c @@ -2,6 +2,8 @@ * i2c-xiic.c * Copyright (c) 2002-2007 Xilinx Inc. * Copyright (c) 2009-2010 Intel Corporation + * Copyright (c) 2010-2011 Michal Simek <monstr-pSz03upnqPeHXe+LvDLADg@public.gmane.org> + * Copyright (c) 2010-2011 PetaLogix * * 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 @@ -41,6 +43,13 @@ #include <linux/io.h> #include <linux/slab.h> +#ifdef CONFIG_OF +#include <linux/of.h> +#include <linux/of_platform.h> +#include <linux/of_i2c.h> +#include <linux/of_address.h> +#endif + #define DRIVER_NAME "xiic-i2c" enum xilinx_i2c_state { @@ -426,7 +435,7 @@ static void xiic_process(struct xiic_i2c *i2c) xiic_wakeup(i2c, STATE_ERROR); } else if (pend & (XIIC_INTR_TX_EMPTY_MASK | XIIC_INTR_TX_HALF_MASK)) { - /* Transmit register/FIFO is empty or ½ empty */ + /* Transmit register/FIFO is empty or 1/2 empty */ clr = pend & (XIIC_INTR_TX_EMPTY_MASK | XIIC_INTR_TX_HALF_MASK); @@ -691,10 +700,12 @@ static struct i2c_adapter xiic_adapter = { static int __devinit xiic_i2c_probe(struct platform_device *pdev) { struct xiic_i2c *i2c; +#ifndef CONFIG_OF struct xiic_i2c_platform_data *pdata; + u8 i; +#endif struct resource *res; int ret, irq; - u8 i; res = platform_get_resource(pdev, IORESOURCE_MEM, 0); if (!res) @@ -704,10 +715,6 @@ static int __devinit xiic_i2c_probe(struct platform_device *pdev) if (irq < 0) goto resource_missing; - pdata = (struct xiic_i2c_platform_data *) pdev->dev.platform_data; - if (!pdata) - return -EINVAL; - i2c = kzalloc(sizeof(*i2c), GFP_KERNEL); if (!i2c) return -ENOMEM; @@ -730,7 +737,9 @@ static int __devinit xiic_i2c_probe(struct platform_device *pdev) i2c->adap = xiic_adapter; i2c_set_adapdata(&i2c->adap, i2c); i2c->adap.dev.parent = &pdev->dev; - +#ifdef CONFIG_OF + i2c->adap.dev.of_node = of_node_get(pdev->dev.of_node); +#endif xiic_reinit(i2c); spin_lock_init(&i2c->lock); @@ -748,9 +757,20 @@ static int __devinit xiic_i2c_probe(struct platform_device *pdev) goto add_adapter_failed; } + dev_info(&pdev->dev, "mapped to 0x%08X, irq=%d\n", + (unsigned int)i2c->base, irq); + +#ifndef CONFIG_OF + pdata = (struct xiic_i2c_platform_data *) pdev->dev.platform_data; + if (!pdata) + return -EINVAL; + /* add in known devices to the bus */ for (i = 0; i < pdata->num_devices; i++) i2c_new_device(&i2c->adap, pdata->devices + i); +#else + of_i2c_register_devices(&i2c->adap); +#endif return 0; @@ -799,12 +819,24 @@ static int __devexit xiic_i2c_remove(struct platform_device* pdev) /* work with hotplug and coldplug */ MODULE_ALIAS("platform:"DRIVER_NAME); +#ifdef CONFIG_OF +/* Match table for of_platform binding */ +static struct of_device_id __devinitdata xilinx_iic_of_match[] = { + { .compatible = "xlnx,xps-iic-2.00.a", }, + {}, +}; +MODULE_DEVICE_TABLE(of, xilinx_iic_of_match); +#endif + static struct platform_driver xiic_i2c_driver = { .probe = xiic_i2c_probe, .remove = __devexit_p(xiic_i2c_remove), .driver = { .owner = THIS_MODULE, .name = DRIVER_NAME, +#ifdef CONFIG_OF + .of_match_table = xilinx_iic_of_match, +#endif }, }; -- 1.5.5.6 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2 1/2] i2c: xiic: Add OF support for Xilinx i2c bus interface @ 2011-02-22 14:22 ` Michal Simek 0 siblings, 0 replies; 7+ messages in thread From: Michal Simek @ 2011-02-22 14:22 UTC (permalink / raw) To: linux-i2c Cc: grant.likely, devicetree-discuss, john.williams, linux-kernel, John.Linn, Michal Simek [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1: Type: text/plain, Size: 3640 bytes --] Add OF support to Xilinx i2c bus interface. It is used on Microblaze systems. Signed-off-by: Michal Simek <monstr@monstr.eu> --- drivers/i2c/busses/i2c-xiic.c | 46 ++++++++++++++++++++++++++++++++++------ 1 files changed, 39 insertions(+), 7 deletions(-) diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c index a9c419e..4166570 100644 --- a/drivers/i2c/busses/i2c-xiic.c +++ b/drivers/i2c/busses/i2c-xiic.c @@ -2,6 +2,8 @@ * i2c-xiic.c * Copyright (c) 2002-2007 Xilinx Inc. * Copyright (c) 2009-2010 Intel Corporation + * Copyright (c) 2010-2011 Michal Simek <monstr@monstr.eu> + * Copyright (c) 2010-2011 PetaLogix * * 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 @@ -41,6 +43,13 @@ #include <linux/io.h> #include <linux/slab.h> +#ifdef CONFIG_OF +#include <linux/of.h> +#include <linux/of_platform.h> +#include <linux/of_i2c.h> +#include <linux/of_address.h> +#endif + #define DRIVER_NAME "xiic-i2c" enum xilinx_i2c_state { @@ -426,7 +435,7 @@ static void xiic_process(struct xiic_i2c *i2c) xiic_wakeup(i2c, STATE_ERROR); } else if (pend & (XIIC_INTR_TX_EMPTY_MASK | XIIC_INTR_TX_HALF_MASK)) { - /* Transmit register/FIFO is empty or ½ empty */ + /* Transmit register/FIFO is empty or 1/2 empty */ clr = pend & (XIIC_INTR_TX_EMPTY_MASK | XIIC_INTR_TX_HALF_MASK); @@ -691,10 +700,12 @@ static struct i2c_adapter xiic_adapter = { static int __devinit xiic_i2c_probe(struct platform_device *pdev) { struct xiic_i2c *i2c; +#ifndef CONFIG_OF struct xiic_i2c_platform_data *pdata; + u8 i; +#endif struct resource *res; int ret, irq; - u8 i; res = platform_get_resource(pdev, IORESOURCE_MEM, 0); if (!res) @@ -704,10 +715,6 @@ static int __devinit xiic_i2c_probe(struct platform_device *pdev) if (irq < 0) goto resource_missing; - pdata = (struct xiic_i2c_platform_data *) pdev->dev.platform_data; - if (!pdata) - return -EINVAL; - i2c = kzalloc(sizeof(*i2c), GFP_KERNEL); if (!i2c) return -ENOMEM; @@ -730,7 +737,9 @@ static int __devinit xiic_i2c_probe(struct platform_device *pdev) i2c->adap = xiic_adapter; i2c_set_adapdata(&i2c->adap, i2c); i2c->adap.dev.parent = &pdev->dev; - +#ifdef CONFIG_OF + i2c->adap.dev.of_node = of_node_get(pdev->dev.of_node); +#endif xiic_reinit(i2c); spin_lock_init(&i2c->lock); @@ -748,9 +757,20 @@ static int __devinit xiic_i2c_probe(struct platform_device *pdev) goto add_adapter_failed; } + dev_info(&pdev->dev, "mapped to 0x%08X, irq=%d\n", + (unsigned int)i2c->base, irq); + +#ifndef CONFIG_OF + pdata = (struct xiic_i2c_platform_data *) pdev->dev.platform_data; + if (!pdata) + return -EINVAL; + /* add in known devices to the bus */ for (i = 0; i < pdata->num_devices; i++) i2c_new_device(&i2c->adap, pdata->devices + i); +#else + of_i2c_register_devices(&i2c->adap); +#endif return 0; @@ -799,12 +819,24 @@ static int __devexit xiic_i2c_remove(struct platform_device* pdev) /* work with hotplug and coldplug */ MODULE_ALIAS("platform:"DRIVER_NAME); +#ifdef CONFIG_OF +/* Match table for of_platform binding */ +static struct of_device_id __devinitdata xilinx_iic_of_match[] = { + { .compatible = "xlnx,xps-iic-2.00.a", }, + {}, +}; +MODULE_DEVICE_TABLE(of, xilinx_iic_of_match); +#endif + static struct platform_driver xiic_i2c_driver = { .probe = xiic_i2c_probe, .remove = __devexit_p(xiic_i2c_remove), .driver = { .owner = THIS_MODULE, .name = DRIVER_NAME, +#ifdef CONFIG_OF + .of_match_table = xilinx_iic_of_match, +#endif }, }; -- 1.5.5.6 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2 2/2] i2c: xiic: Use 32bit accesses only 2011-02-22 14:22 ` Michal Simek (?) @ 2011-02-22 14:22 ` Michal Simek -1 siblings, 0 replies; 7+ messages in thread From: Michal Simek @ 2011-02-22 14:22 UTC (permalink / raw) To: linux-i2c Cc: grant.likely, devicetree-discuss, john.williams, linux-kernel, John.Linn, Michal Simek i2c driver is used for LE/BE that's why is useful to use 32bit accesses. Then it is not necessary to solve any endian issues. Signed-off-by: Michal Simek <monstr@monstr.eu> --- drivers/i2c/busses/i2c-xiic.c | 66 +++++++++++++++++------------------------ 1 files changed, 27 insertions(+), 39 deletions(-) diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c index 4166570..f6367c2 100644 --- a/drivers/i2c/busses/i2c-xiic.c +++ b/drivers/i2c/busses/i2c-xiic.c @@ -182,21 +182,6 @@ struct xiic_i2c { static void xiic_start_xfer(struct xiic_i2c *i2c); static void __xiic_start_xfer(struct xiic_i2c *i2c); -static inline void xiic_setreg8(struct xiic_i2c *i2c, int reg, u8 value) -{ - iowrite8(value, i2c->base + reg); -} - -static inline u8 xiic_getreg8(struct xiic_i2c *i2c, int reg) -{ - return ioread8(i2c->base + reg); -} - -static inline void xiic_setreg16(struct xiic_i2c *i2c, int reg, u16 value) -{ - iowrite16(value, i2c->base + reg); -} - static inline void xiic_setreg32(struct xiic_i2c *i2c, int reg, int value) { iowrite32(value, i2c->base + reg); @@ -234,10 +219,11 @@ static inline void xiic_irq_clr_en(struct xiic_i2c *i2c, u32 mask) static void xiic_clear_rx_fifo(struct xiic_i2c *i2c) { u8 sr; - for (sr = xiic_getreg8(i2c, XIIC_SR_REG_OFFSET); + for (sr = xiic_getreg32(i2c, XIIC_SR_REG_OFFSET); !(sr & XIIC_SR_RX_FIFO_EMPTY_MASK); - sr = xiic_getreg8(i2c, XIIC_SR_REG_OFFSET)) - xiic_getreg8(i2c, XIIC_DRR_REG_OFFSET); + sr = xiic_getreg32(i2c, XIIC_SR_REG_OFFSET)) { + xiic_getreg32(i2c, XIIC_DRR_REG_OFFSET); + } } static void xiic_reinit(struct xiic_i2c *i2c) @@ -245,13 +231,13 @@ static void xiic_reinit(struct xiic_i2c *i2c) xiic_setreg32(i2c, XIIC_RESETR_OFFSET, XIIC_RESET_MASK); /* Set receive Fifo depth to maximum (zero based). */ - xiic_setreg8(i2c, XIIC_RFD_REG_OFFSET, IIC_RX_FIFO_DEPTH - 1); + xiic_setreg32(i2c, XIIC_RFD_REG_OFFSET, IIC_RX_FIFO_DEPTH - 1); /* Reset Tx Fifo. */ - xiic_setreg8(i2c, XIIC_CR_REG_OFFSET, XIIC_CR_TX_FIFO_RESET_MASK); + xiic_setreg32(i2c, XIIC_CR_REG_OFFSET, XIIC_CR_TX_FIFO_RESET_MASK); /* Enable IIC Device, remove Tx Fifo reset & disable general call. */ - xiic_setreg8(i2c, XIIC_CR_REG_OFFSET, XIIC_CR_ENABLE_DEVICE_MASK); + xiic_setreg32(i2c, XIIC_CR_REG_OFFSET, XIIC_CR_ENABLE_DEVICE_MASK); /* make sure RX fifo is empty */ xiic_clear_rx_fifo(i2c); @@ -269,8 +255,9 @@ static void xiic_deinit(struct xiic_i2c *i2c) xiic_setreg32(i2c, XIIC_RESETR_OFFSET, XIIC_RESET_MASK); /* Disable IIC Device. */ - cr = xiic_getreg8(i2c, XIIC_CR_REG_OFFSET); - xiic_setreg8(i2c, XIIC_CR_REG_OFFSET, cr & ~XIIC_CR_ENABLE_DEVICE_MASK); + cr = xiic_getreg32(i2c, XIIC_CR_REG_OFFSET); + xiic_setreg32(i2c, XIIC_CR_REG_OFFSET, + cr & ~XIIC_CR_ENABLE_DEVICE_MASK); } static void xiic_read_rx(struct xiic_i2c *i2c) @@ -278,22 +265,22 @@ static void xiic_read_rx(struct xiic_i2c *i2c) u8 bytes_in_fifo; int i; - bytes_in_fifo = xiic_getreg8(i2c, XIIC_RFO_REG_OFFSET) + 1; + bytes_in_fifo = xiic_getreg32(i2c, XIIC_RFO_REG_OFFSET) + 1; dev_dbg(i2c->adap.dev.parent, "%s entry, bytes in fifo: %d, msg: %d" ", SR: 0x%x, CR: 0x%x\n", __func__, bytes_in_fifo, xiic_rx_space(i2c), - xiic_getreg8(i2c, XIIC_SR_REG_OFFSET), - xiic_getreg8(i2c, XIIC_CR_REG_OFFSET)); + xiic_getreg32(i2c, XIIC_SR_REG_OFFSET), + xiic_getreg32(i2c, XIIC_CR_REG_OFFSET)); if (bytes_in_fifo > xiic_rx_space(i2c)) bytes_in_fifo = xiic_rx_space(i2c); for (i = 0; i < bytes_in_fifo; i++) i2c->rx_msg->buf[i2c->rx_pos++] = - xiic_getreg8(i2c, XIIC_DRR_REG_OFFSET); + xiic_getreg32(i2c, XIIC_DRR_REG_OFFSET); - xiic_setreg8(i2c, XIIC_RFD_REG_OFFSET, + xiic_setreg32(i2c, XIIC_RFD_REG_OFFSET, (xiic_rx_space(i2c) > IIC_RX_FIFO_DEPTH) ? IIC_RX_FIFO_DEPTH - 1 : xiic_rx_space(i2c) - 1); } @@ -301,7 +288,7 @@ static void xiic_read_rx(struct xiic_i2c *i2c) static int xiic_tx_fifo_space(struct xiic_i2c *i2c) { /* return the actual space left in the FIFO */ - return IIC_TX_FIFO_DEPTH - xiic_getreg8(i2c, XIIC_TFO_REG_OFFSET) - 1; + return IIC_TX_FIFO_DEPTH - xiic_getreg32(i2c, XIIC_TFO_REG_OFFSET) - 1; } static void xiic_fill_tx_fifo(struct xiic_i2c *i2c) @@ -321,9 +308,9 @@ static void xiic_fill_tx_fifo(struct xiic_i2c *i2c) data |= XIIC_TX_DYN_STOP_MASK; dev_dbg(i2c->adap.dev.parent, "%s TX STOP\n", __func__); - xiic_setreg16(i2c, XIIC_DTR_REG_OFFSET, data); + xiic_setreg32(i2c, XIIC_DTR_REG_OFFSET, data); } else - xiic_setreg8(i2c, XIIC_DTR_REG_OFFSET, data); + xiic_setreg32(i2c, XIIC_DTR_REG_OFFSET, data); } } @@ -352,7 +339,8 @@ static void xiic_process(struct xiic_i2c *i2c) dev_dbg(i2c->adap.dev.parent, "%s entry, IER: 0x%x, ISR: 0x%x, " "pend: 0x%x, SR: 0x%x, msg: %p, nmsgs: %d\n", - __func__, ier, isr, pend, xiic_getreg8(i2c, XIIC_SR_REG_OFFSET), + __func__, ier, isr, pend, + xiic_getreg32(i2c, XIIC_SR_REG_OFFSET), i2c->tx_msg, i2c->nmsgs); /* Do not processes a devices interrupts if the device has no @@ -483,7 +471,7 @@ out: static int xiic_bus_busy(struct xiic_i2c *i2c) { - u8 sr = xiic_getreg8(i2c, XIIC_SR_REG_OFFSET); + u8 sr = xiic_getreg32(i2c, XIIC_SR_REG_OFFSET); return (sr & XIIC_SR_BUS_BUSY_MASK) ? -EBUSY : 0; } @@ -526,17 +514,17 @@ static void xiic_start_recv(struct xiic_i2c *i2c) rx_watermark = msg->len; if (rx_watermark > IIC_RX_FIFO_DEPTH) rx_watermark = IIC_RX_FIFO_DEPTH; - xiic_setreg8(i2c, XIIC_RFD_REG_OFFSET, rx_watermark - 1); + xiic_setreg32(i2c, XIIC_RFD_REG_OFFSET, rx_watermark - 1); if (!(msg->flags & I2C_M_NOSTART)) /* write the address */ - xiic_setreg16(i2c, XIIC_DTR_REG_OFFSET, + xiic_setreg32(i2c, XIIC_DTR_REG_OFFSET, (msg->addr << 1) | XIIC_READ_OPERATION | XIIC_TX_DYN_START_MASK); xiic_irq_clr_en(i2c, XIIC_INTR_BNB_MASK); - xiic_setreg16(i2c, XIIC_DTR_REG_OFFSET, + xiic_setreg32(i2c, XIIC_DTR_REG_OFFSET, msg->len | ((i2c->nmsgs == 1) ? XIIC_TX_DYN_STOP_MASK : 0)); if (i2c->nmsgs == 1) /* very last, enable bus not busy as well */ @@ -555,7 +543,7 @@ static void xiic_start_send(struct xiic_i2c *i2c) dev_dbg(i2c->adap.dev.parent, "%s entry, msg: %p, len: %d, " "ISR: 0x%x, CR: 0x%x\n", __func__, msg, msg->len, xiic_getreg32(i2c, XIIC_IISR_OFFSET), - xiic_getreg8(i2c, XIIC_CR_REG_OFFSET)); + xiic_getreg32(i2c, XIIC_CR_REG_OFFSET)); if (!(msg->flags & I2C_M_NOSTART)) { /* write the address */ @@ -565,7 +553,7 @@ static void xiic_start_send(struct xiic_i2c *i2c) /* no data and last message -> add STOP */ data |= XIIC_TX_DYN_STOP_MASK; - xiic_setreg16(i2c, XIIC_DTR_REG_OFFSET, data); + xiic_setreg32(i2c, XIIC_DTR_REG_OFFSET, data); } xiic_fill_tx_fifo(i2c); @@ -657,7 +645,7 @@ static int xiic_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num) int err; dev_dbg(adap->dev.parent, "%s entry SR: 0x%x\n", __func__, - xiic_getreg8(i2c, XIIC_SR_REG_OFFSET)); + xiic_getreg32(i2c, XIIC_SR_REG_OFFSET)); err = xiic_busy(i2c); if (err) -- 1.5.5.6 ^ permalink raw reply related [flat|nested] 7+ messages in thread
[parent not found: <1298384530-14994-1-git-send-email-monstr-pSz03upnqPeHXe+LvDLADg@public.gmane.org>]
* Re: [PATCH v2 1/2] i2c: xiic: Add OF support for Xilinx i2c bus interface 2011-02-22 14:22 ` Michal Simek @ 2011-02-22 15:52 ` Grant Likely -1 siblings, 0 replies; 7+ messages in thread From: Grant Likely @ 2011-02-22 15:52 UTC (permalink / raw) To: Michal Simek Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, john.williams-g5w7nrANp4BDPfheJLI6IQ, linux-kernel-u79uwXL29TY76Z2rM5mHXA, John.Linn-gjFFaj9aHVfQT0dZR+AlfA On Tue, Feb 22, 2011 at 7:22 AM, Michal Simek <monstr-pSz03upnqPeHXe+LvDLADg@public.gmane.org> wrote: > Add OF support to Xilinx i2c bus interface. > It is used on Microblaze systems. > > Signed-off-by: Michal Simek <monstr-pSz03upnqPeHXe+LvDLADg@public.gmane.org> Hi Michal, comments below... > --- > drivers/i2c/busses/i2c-xiic.c | 46 ++++++++++++++++++++++++++++++++++------ > 1 files changed, 39 insertions(+), 7 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c > index a9c419e..4166570 100644 > --- a/drivers/i2c/busses/i2c-xiic.c > +++ b/drivers/i2c/busses/i2c-xiic.c > @@ -2,6 +2,8 @@ > * i2c-xiic.c > * Copyright (c) 2002-2007 Xilinx Inc. > * Copyright (c) 2009-2010 Intel Corporation > + * Copyright (c) 2010-2011 Michal Simek <monstr-pSz03upnqPeHXe+LvDLADg@public.gmane.org> > + * Copyright (c) 2010-2011 PetaLogix This change is going to end up being pretty trivial. It would be a little odd to claim copyright on such a tiny change; particularly when it just follows the pattern already established by other drivers. > * > * 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 > @@ -41,6 +43,13 @@ > #include <linux/io.h> > #include <linux/slab.h> > > +#ifdef CONFIG_OF > +#include <linux/of.h> > +#include <linux/of_platform.h> > +#include <linux/of_i2c.h> > +#include <linux/of_address.h> > +#endif As previously mentioned; drop the #ifdef protection around these includes. It should be safe to include them unconditionally, and if it isn't then it is a bug in the headers which should be fixed. These includes should be part of the regular block of includes. > + > #define DRIVER_NAME "xiic-i2c" > > enum xilinx_i2c_state { > @@ -426,7 +435,7 @@ static void xiic_process(struct xiic_i2c *i2c) > xiic_wakeup(i2c, STATE_ERROR); > > } else if (pend & (XIIC_INTR_TX_EMPTY_MASK | XIIC_INTR_TX_HALF_MASK)) { > - /* Transmit register/FIFO is empty or ½ empty */ > + /* Transmit register/FIFO is empty or 1/2 empty */ > > clr = pend & > (XIIC_INTR_TX_EMPTY_MASK | XIIC_INTR_TX_HALF_MASK); > @@ -691,10 +700,12 @@ static struct i2c_adapter xiic_adapter = { > static int __devinit xiic_i2c_probe(struct platform_device *pdev) > { > struct xiic_i2c *i2c; > +#ifndef CONFIG_OF > struct xiic_i2c_platform_data *pdata; > + u8 i; > +#endif > struct resource *res; > int ret, irq; > - u8 i; You can drop this hunk (see below). > > res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > if (!res) > @@ -704,10 +715,6 @@ static int __devinit xiic_i2c_probe(struct platform_device *pdev) > if (irq < 0) > goto resource_missing; > > - pdata = (struct xiic_i2c_platform_data *) pdev->dev.platform_data; > - if (!pdata) > - return -EINVAL; > - > i2c = kzalloc(sizeof(*i2c), GFP_KERNEL); > if (!i2c) > return -ENOMEM; > @@ -730,7 +737,9 @@ static int __devinit xiic_i2c_probe(struct platform_device *pdev) > i2c->adap = xiic_adapter; > i2c_set_adapdata(&i2c->adap, i2c); > i2c->adap.dev.parent = &pdev->dev; > - > +#ifdef CONFIG_OF > + i2c->adap.dev.of_node = of_node_get(pdev->dev.of_node); > +#endif Drop the #ifdef here. In devicetree/next of_node is no longer protected by a #ifdef. > xiic_reinit(i2c); > > spin_lock_init(&i2c->lock); > @@ -748,9 +757,20 @@ static int __devinit xiic_i2c_probe(struct platform_device *pdev) > goto add_adapter_failed; > } > > + dev_info(&pdev->dev, "mapped to 0x%08X, irq=%d\n", > + (unsigned int)i2c->base, irq); > + > +#ifndef CONFIG_OF > + pdata = (struct xiic_i2c_platform_data *) pdev->dev.platform_data; > + if (!pdata) > + return -EINVAL; > + > /* add in known devices to the bus */ > for (i = 0; i < pdata->num_devices; i++) > i2c_new_device(&i2c->adap, pdata->devices + i); > +#else > + of_i2c_register_devices(&i2c->adap); > +#endif Do it this way instead: pdata = (struct xiic_i2c_platform_data *) pdev->dev.platform_data; if (pdata) { /* add in known devices to the bus */ for (i = 0; i < pdata->num_devices; i++) i2c_new_device(&i2c->adap, pdata->devices + i); } of_i2c_register_devices(&i2c->adap); of_i2c_register_devices() is safe to call regardless of if CONFIG_OF is selected or not. This driver should be written to handle both device tree and platform_data use cases at run time. > > return 0; > > @@ -799,12 +819,24 @@ static int __devexit xiic_i2c_remove(struct platform_device* pdev) > /* work with hotplug and coldplug */ > MODULE_ALIAS("platform:"DRIVER_NAME); > > +#ifdef CONFIG_OF > +/* Match table for of_platform binding */ > +static struct of_device_id __devinitdata xilinx_iic_of_match[] = { > + { .compatible = "xlnx,xps-iic-2.00.a", }, > + {}, > +}; > +MODULE_DEVICE_TABLE(of, xilinx_iic_of_match); If you add the following, then you can drop the #ifdef around .of_match_table below... #else #define xilinx_iic_of_match NULL > +#endif > + > static struct platform_driver xiic_i2c_driver = { > .probe = xiic_i2c_probe, > .remove = __devexit_p(xiic_i2c_remove), > .driver = { > .owner = THIS_MODULE, > .name = DRIVER_NAME, > +#ifdef CONFIG_OF > + .of_match_table = xilinx_iic_of_match, > +#endif Drop the #ifdef and base this patch on devicetree/next > }, > }; > > -- > 1.5.5.6 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ > -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/2] i2c: xiic: Add OF support for Xilinx i2c bus interface @ 2011-02-22 15:52 ` Grant Likely 0 siblings, 0 replies; 7+ messages in thread From: Grant Likely @ 2011-02-22 15:52 UTC (permalink / raw) To: Michal Simek Cc: linux-i2c, devicetree-discuss, john.williams, linux-kernel, John.Linn On Tue, Feb 22, 2011 at 7:22 AM, Michal Simek <monstr@monstr.eu> wrote: > Add OF support to Xilinx i2c bus interface. > It is used on Microblaze systems. > > Signed-off-by: Michal Simek <monstr@monstr.eu> Hi Michal, comments below... > --- > drivers/i2c/busses/i2c-xiic.c | 46 ++++++++++++++++++++++++++++++++++------ > 1 files changed, 39 insertions(+), 7 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c > index a9c419e..4166570 100644 > --- a/drivers/i2c/busses/i2c-xiic.c > +++ b/drivers/i2c/busses/i2c-xiic.c > @@ -2,6 +2,8 @@ > * i2c-xiic.c > * Copyright (c) 2002-2007 Xilinx Inc. > * Copyright (c) 2009-2010 Intel Corporation > + * Copyright (c) 2010-2011 Michal Simek <monstr@monstr.eu> > + * Copyright (c) 2010-2011 PetaLogix This change is going to end up being pretty trivial. It would be a little odd to claim copyright on such a tiny change; particularly when it just follows the pattern already established by other drivers. > * > * 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 > @@ -41,6 +43,13 @@ > #include <linux/io.h> > #include <linux/slab.h> > > +#ifdef CONFIG_OF > +#include <linux/of.h> > +#include <linux/of_platform.h> > +#include <linux/of_i2c.h> > +#include <linux/of_address.h> > +#endif As previously mentioned; drop the #ifdef protection around these includes. It should be safe to include them unconditionally, and if it isn't then it is a bug in the headers which should be fixed. These includes should be part of the regular block of includes. > + > #define DRIVER_NAME "xiic-i2c" > > enum xilinx_i2c_state { > @@ -426,7 +435,7 @@ static void xiic_process(struct xiic_i2c *i2c) > xiic_wakeup(i2c, STATE_ERROR); > > } else if (pend & (XIIC_INTR_TX_EMPTY_MASK | XIIC_INTR_TX_HALF_MASK)) { > - /* Transmit register/FIFO is empty or ½ empty */ > + /* Transmit register/FIFO is empty or 1/2 empty */ > > clr = pend & > (XIIC_INTR_TX_EMPTY_MASK | XIIC_INTR_TX_HALF_MASK); > @@ -691,10 +700,12 @@ static struct i2c_adapter xiic_adapter = { > static int __devinit xiic_i2c_probe(struct platform_device *pdev) > { > struct xiic_i2c *i2c; > +#ifndef CONFIG_OF > struct xiic_i2c_platform_data *pdata; > + u8 i; > +#endif > struct resource *res; > int ret, irq; > - u8 i; You can drop this hunk (see below). > > res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > if (!res) > @@ -704,10 +715,6 @@ static int __devinit xiic_i2c_probe(struct platform_device *pdev) > if (irq < 0) > goto resource_missing; > > - pdata = (struct xiic_i2c_platform_data *) pdev->dev.platform_data; > - if (!pdata) > - return -EINVAL; > - > i2c = kzalloc(sizeof(*i2c), GFP_KERNEL); > if (!i2c) > return -ENOMEM; > @@ -730,7 +737,9 @@ static int __devinit xiic_i2c_probe(struct platform_device *pdev) > i2c->adap = xiic_adapter; > i2c_set_adapdata(&i2c->adap, i2c); > i2c->adap.dev.parent = &pdev->dev; > - > +#ifdef CONFIG_OF > + i2c->adap.dev.of_node = of_node_get(pdev->dev.of_node); > +#endif Drop the #ifdef here. In devicetree/next of_node is no longer protected by a #ifdef. > xiic_reinit(i2c); > > spin_lock_init(&i2c->lock); > @@ -748,9 +757,20 @@ static int __devinit xiic_i2c_probe(struct platform_device *pdev) > goto add_adapter_failed; > } > > + dev_info(&pdev->dev, "mapped to 0x%08X, irq=%d\n", > + (unsigned int)i2c->base, irq); > + > +#ifndef CONFIG_OF > + pdata = (struct xiic_i2c_platform_data *) pdev->dev.platform_data; > + if (!pdata) > + return -EINVAL; > + > /* add in known devices to the bus */ > for (i = 0; i < pdata->num_devices; i++) > i2c_new_device(&i2c->adap, pdata->devices + i); > +#else > + of_i2c_register_devices(&i2c->adap); > +#endif Do it this way instead: pdata = (struct xiic_i2c_platform_data *) pdev->dev.platform_data; if (pdata) { /* add in known devices to the bus */ for (i = 0; i < pdata->num_devices; i++) i2c_new_device(&i2c->adap, pdata->devices + i); } of_i2c_register_devices(&i2c->adap); of_i2c_register_devices() is safe to call regardless of if CONFIG_OF is selected or not. This driver should be written to handle both device tree and platform_data use cases at run time. > > return 0; > > @@ -799,12 +819,24 @@ static int __devexit xiic_i2c_remove(struct platform_device* pdev) > /* work with hotplug and coldplug */ > MODULE_ALIAS("platform:"DRIVER_NAME); > > +#ifdef CONFIG_OF > +/* Match table for of_platform binding */ > +static struct of_device_id __devinitdata xilinx_iic_of_match[] = { > + { .compatible = "xlnx,xps-iic-2.00.a", }, > + {}, > +}; > +MODULE_DEVICE_TABLE(of, xilinx_iic_of_match); If you add the following, then you can drop the #ifdef around .of_match_table below... #else #define xilinx_iic_of_match NULL > +#endif > + > static struct platform_driver xiic_i2c_driver = { > .probe = xiic_i2c_probe, > .remove = __devexit_p(xiic_i2c_remove), > .driver = { > .owner = THIS_MODULE, > .name = DRIVER_NAME, > +#ifdef CONFIG_OF > + .of_match_table = xilinx_iic_of_match, > +#endif Drop the #ifdef and base this patch on devicetree/next > }, > }; > > -- > 1.5.5.6 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ > -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ^ permalink raw reply [flat|nested] 7+ messages in thread
[parent not found: <AANLkTi=FvafUgqMCF29cX7egviDwrfB=sViudLr+4Fsx-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH v2 1/2] i2c: xiic: Add OF support for Xilinx i2c bus interface 2011-02-22 15:52 ` Grant Likely @ 2011-02-22 17:52 ` Michal Simek -1 siblings, 0 replies; 7+ messages in thread From: Michal Simek @ 2011-02-22 17:52 UTC (permalink / raw) To: Grant Likely Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, john.williams-g5w7nrANp4BDPfheJLI6IQ, linux-kernel-u79uwXL29TY76Z2rM5mHXA, John.Linn-gjFFaj9aHVfQT0dZR+AlfA Grant Likely wrote: > On Tue, Feb 22, 2011 at 7:22 AM, Michal Simek <monstr-pSz03upnqPeHXe+LvDLADg@public.gmane.org> wrote: >> Add OF support to Xilinx i2c bus interface. >> It is used on Microblaze systems. >> >> Signed-off-by: Michal Simek <monstr-pSz03upnqPeHXe+LvDLADg@public.gmane.org> > > Hi Michal, comments below... > >> --- >> drivers/i2c/busses/i2c-xiic.c | 46 ++++++++++++++++++++++++++++++++++------ >> 1 files changed, 39 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c >> index a9c419e..4166570 100644 >> --- a/drivers/i2c/busses/i2c-xiic.c >> +++ b/drivers/i2c/busses/i2c-xiic.c >> @@ -2,6 +2,8 @@ >> * i2c-xiic.c >> * Copyright (c) 2002-2007 Xilinx Inc. >> * Copyright (c) 2009-2010 Intel Corporation >> + * Copyright (c) 2010-2011 Michal Simek <monstr-pSz03upnqPeHXe+LvDLADg@public.gmane.org> >> + * Copyright (c) 2010-2011 PetaLogix > > This change is going to end up being pretty trivial. It would be a > little odd to claim copyright on such a tiny change; particularly when > it just follows the pattern already established by other drivers. agree - I forget to remove it - it was in my origin patch. > >> * >> * 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 >> @@ -41,6 +43,13 @@ >> #include <linux/io.h> >> #include <linux/slab.h> >> >> +#ifdef CONFIG_OF >> +#include <linux/of.h> >> +#include <linux/of_platform.h> >> +#include <linux/of_i2c.h> >> +#include <linux/of_address.h> >> +#endif > > As previously mentioned; drop the #ifdef protection around these > includes. It should be safe to include them unconditionally, and if it > isn't then it is a bug in the headers which should be fixed. These > includes should be part of the regular block of includes. remove done > >> + >> #define DRIVER_NAME "xiic-i2c" >> >> enum xilinx_i2c_state { >> @@ -426,7 +435,7 @@ static void xiic_process(struct xiic_i2c *i2c) >> xiic_wakeup(i2c, STATE_ERROR); >> >> } else if (pend & (XIIC_INTR_TX_EMPTY_MASK | XIIC_INTR_TX_HALF_MASK)) { >> - /* Transmit register/FIFO is empty or ½ empty */ >> + /* Transmit register/FIFO is empty or 1/2 empty */ >> >> clr = pend & >> (XIIC_INTR_TX_EMPTY_MASK | XIIC_INTR_TX_HALF_MASK); >> @@ -691,10 +700,12 @@ static struct i2c_adapter xiic_adapter = { >> static int __devinit xiic_i2c_probe(struct platform_device *pdev) >> { >> struct xiic_i2c *i2c; >> +#ifndef CONFIG_OF >> struct xiic_i2c_platform_data *pdata; >> + u8 i; >> +#endif >> struct resource *res; >> int ret, irq; >> - u8 i; > > You can drop this hunk (see below). done > >> res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >> if (!res) >> @@ -704,10 +715,6 @@ static int __devinit xiic_i2c_probe(struct platform_device *pdev) >> if (irq < 0) >> goto resource_missing; >> >> - pdata = (struct xiic_i2c_platform_data *) pdev->dev.platform_data; >> - if (!pdata) >> - return -EINVAL; >> - >> i2c = kzalloc(sizeof(*i2c), GFP_KERNEL); >> if (!i2c) >> return -ENOMEM; >> @@ -730,7 +737,9 @@ static int __devinit xiic_i2c_probe(struct platform_device *pdev) >> i2c->adap = xiic_adapter; >> i2c_set_adapdata(&i2c->adap, i2c); >> i2c->adap.dev.parent = &pdev->dev; >> - >> +#ifdef CONFIG_OF >> + i2c->adap.dev.of_node = of_node_get(pdev->dev.of_node); >> +#endif > > Drop the #ifdef here. In devicetree/next of_node is no longer > protected by a #ifdef. ok, done > >> xiic_reinit(i2c); >> >> spin_lock_init(&i2c->lock); >> @@ -748,9 +757,20 @@ static int __devinit xiic_i2c_probe(struct platform_device *pdev) >> goto add_adapter_failed; >> } >> >> + dev_info(&pdev->dev, "mapped to 0x%08X, irq=%d\n", >> + (unsigned int)i2c->base, irq); >> + >> +#ifndef CONFIG_OF >> + pdata = (struct xiic_i2c_platform_data *) pdev->dev.platform_data; >> + if (!pdata) >> + return -EINVAL; >> + >> /* add in known devices to the bus */ >> for (i = 0; i < pdata->num_devices; i++) >> i2c_new_device(&i2c->adap, pdata->devices + i); >> +#else >> + of_i2c_register_devices(&i2c->adap); >> +#endif > > Do it this way instead: > > pdata = (struct xiic_i2c_platform_data *) pdev->dev.platform_data; > if (pdata) { > /* add in known devices to the bus */ > for (i = 0; i < pdata->num_devices; i++) > i2c_new_device(&i2c->adap, pdata->devices + i); > } > > of_i2c_register_devices(&i2c->adap); > > of_i2c_register_devices() is safe to call regardless of if CONFIG_OF > is selected or not. This driver should be written to handle both > device tree and platform_data use cases at run time. ok, done. I hope that it doesn't non OF version. I can't easily test it. > >> return 0; >> >> @@ -799,12 +819,24 @@ static int __devexit xiic_i2c_remove(struct platform_device* pdev) >> /* work with hotplug and coldplug */ >> MODULE_ALIAS("platform:"DRIVER_NAME); >> >> +#ifdef CONFIG_OF >> +/* Match table for of_platform binding */ >> +static struct of_device_id __devinitdata xilinx_iic_of_match[] = { >> + { .compatible = "xlnx,xps-iic-2.00.a", }, >> + {}, >> +}; >> +MODULE_DEVICE_TABLE(of, xilinx_iic_of_match); > > If you add the following, then you can drop the #ifdef around > .of_match_table below... > > #else > #define xilinx_iic_of_match NULL > >> +#endif >> + >> static struct platform_driver xiic_i2c_driver = { >> .probe = xiic_i2c_probe, >> .remove = __devexit_p(xiic_i2c_remove), >> .driver = { >> .owner = THIS_MODULE, >> .name = DRIVER_NAME, >> +#ifdef CONFIG_OF >> + .of_match_table = xilinx_iic_of_match, >> +#endif > > Drop the #ifdef and base this patch on devicetree/next specifically this I was checking in the Linus tree to know if there is any ifdef. devicetree/next branch is fine. I have sent v3 based on your devicetree/next branch. Thanks for your comments, Michal -- Michal Simek, Ing. (M.Eng) w: www.monstr.eu p: +42-0-721842854 Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/ Microblaze U-BOOT custodian ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/2] i2c: xiic: Add OF support for Xilinx i2c bus interface @ 2011-02-22 17:52 ` Michal Simek 0 siblings, 0 replies; 7+ messages in thread From: Michal Simek @ 2011-02-22 17:52 UTC (permalink / raw) To: Grant Likely Cc: linux-i2c, devicetree-discuss, john.williams, linux-kernel, John.Linn Grant Likely wrote: > On Tue, Feb 22, 2011 at 7:22 AM, Michal Simek <monstr@monstr.eu> wrote: >> Add OF support to Xilinx i2c bus interface. >> It is used on Microblaze systems. >> >> Signed-off-by: Michal Simek <monstr@monstr.eu> > > Hi Michal, comments below... > >> --- >> drivers/i2c/busses/i2c-xiic.c | 46 ++++++++++++++++++++++++++++++++++------ >> 1 files changed, 39 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c >> index a9c419e..4166570 100644 >> --- a/drivers/i2c/busses/i2c-xiic.c >> +++ b/drivers/i2c/busses/i2c-xiic.c >> @@ -2,6 +2,8 @@ >> * i2c-xiic.c >> * Copyright (c) 2002-2007 Xilinx Inc. >> * Copyright (c) 2009-2010 Intel Corporation >> + * Copyright (c) 2010-2011 Michal Simek <monstr@monstr.eu> >> + * Copyright (c) 2010-2011 PetaLogix > > This change is going to end up being pretty trivial. It would be a > little odd to claim copyright on such a tiny change; particularly when > it just follows the pattern already established by other drivers. agree - I forget to remove it - it was in my origin patch. > >> * >> * 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 >> @@ -41,6 +43,13 @@ >> #include <linux/io.h> >> #include <linux/slab.h> >> >> +#ifdef CONFIG_OF >> +#include <linux/of.h> >> +#include <linux/of_platform.h> >> +#include <linux/of_i2c.h> >> +#include <linux/of_address.h> >> +#endif > > As previously mentioned; drop the #ifdef protection around these > includes. It should be safe to include them unconditionally, and if it > isn't then it is a bug in the headers which should be fixed. These > includes should be part of the regular block of includes. remove done > >> + >> #define DRIVER_NAME "xiic-i2c" >> >> enum xilinx_i2c_state { >> @@ -426,7 +435,7 @@ static void xiic_process(struct xiic_i2c *i2c) >> xiic_wakeup(i2c, STATE_ERROR); >> >> } else if (pend & (XIIC_INTR_TX_EMPTY_MASK | XIIC_INTR_TX_HALF_MASK)) { >> - /* Transmit register/FIFO is empty or ½ empty */ >> + /* Transmit register/FIFO is empty or 1/2 empty */ >> >> clr = pend & >> (XIIC_INTR_TX_EMPTY_MASK | XIIC_INTR_TX_HALF_MASK); >> @@ -691,10 +700,12 @@ static struct i2c_adapter xiic_adapter = { >> static int __devinit xiic_i2c_probe(struct platform_device *pdev) >> { >> struct xiic_i2c *i2c; >> +#ifndef CONFIG_OF >> struct xiic_i2c_platform_data *pdata; >> + u8 i; >> +#endif >> struct resource *res; >> int ret, irq; >> - u8 i; > > You can drop this hunk (see below). done > >> res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >> if (!res) >> @@ -704,10 +715,6 @@ static int __devinit xiic_i2c_probe(struct platform_device *pdev) >> if (irq < 0) >> goto resource_missing; >> >> - pdata = (struct xiic_i2c_platform_data *) pdev->dev.platform_data; >> - if (!pdata) >> - return -EINVAL; >> - >> i2c = kzalloc(sizeof(*i2c), GFP_KERNEL); >> if (!i2c) >> return -ENOMEM; >> @@ -730,7 +737,9 @@ static int __devinit xiic_i2c_probe(struct platform_device *pdev) >> i2c->adap = xiic_adapter; >> i2c_set_adapdata(&i2c->adap, i2c); >> i2c->adap.dev.parent = &pdev->dev; >> - >> +#ifdef CONFIG_OF >> + i2c->adap.dev.of_node = of_node_get(pdev->dev.of_node); >> +#endif > > Drop the #ifdef here. In devicetree/next of_node is no longer > protected by a #ifdef. ok, done > >> xiic_reinit(i2c); >> >> spin_lock_init(&i2c->lock); >> @@ -748,9 +757,20 @@ static int __devinit xiic_i2c_probe(struct platform_device *pdev) >> goto add_adapter_failed; >> } >> >> + dev_info(&pdev->dev, "mapped to 0x%08X, irq=%d\n", >> + (unsigned int)i2c->base, irq); >> + >> +#ifndef CONFIG_OF >> + pdata = (struct xiic_i2c_platform_data *) pdev->dev.platform_data; >> + if (!pdata) >> + return -EINVAL; >> + >> /* add in known devices to the bus */ >> for (i = 0; i < pdata->num_devices; i++) >> i2c_new_device(&i2c->adap, pdata->devices + i); >> +#else >> + of_i2c_register_devices(&i2c->adap); >> +#endif > > Do it this way instead: > > pdata = (struct xiic_i2c_platform_data *) pdev->dev.platform_data; > if (pdata) { > /* add in known devices to the bus */ > for (i = 0; i < pdata->num_devices; i++) > i2c_new_device(&i2c->adap, pdata->devices + i); > } > > of_i2c_register_devices(&i2c->adap); > > of_i2c_register_devices() is safe to call regardless of if CONFIG_OF > is selected or not. This driver should be written to handle both > device tree and platform_data use cases at run time. ok, done. I hope that it doesn't non OF version. I can't easily test it. > >> return 0; >> >> @@ -799,12 +819,24 @@ static int __devexit xiic_i2c_remove(struct platform_device* pdev) >> /* work with hotplug and coldplug */ >> MODULE_ALIAS("platform:"DRIVER_NAME); >> >> +#ifdef CONFIG_OF >> +/* Match table for of_platform binding */ >> +static struct of_device_id __devinitdata xilinx_iic_of_match[] = { >> + { .compatible = "xlnx,xps-iic-2.00.a", }, >> + {}, >> +}; >> +MODULE_DEVICE_TABLE(of, xilinx_iic_of_match); > > If you add the following, then you can drop the #ifdef around > .of_match_table below... > > #else > #define xilinx_iic_of_match NULL > >> +#endif >> + >> static struct platform_driver xiic_i2c_driver = { >> .probe = xiic_i2c_probe, >> .remove = __devexit_p(xiic_i2c_remove), >> .driver = { >> .owner = THIS_MODULE, >> .name = DRIVER_NAME, >> +#ifdef CONFIG_OF >> + .of_match_table = xilinx_iic_of_match, >> +#endif > > Drop the #ifdef and base this patch on devicetree/next specifically this I was checking in the Linus tree to know if there is any ifdef. devicetree/next branch is fine. I have sent v3 based on your devicetree/next branch. Thanks for your comments, Michal -- Michal Simek, Ing. (M.Eng) w: www.monstr.eu p: +42-0-721842854 Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/ Microblaze U-BOOT custodian ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2011-02-22 17:52 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-02-22 14:22 [PATCH v2 1/2] i2c: xiic: Add OF support for Xilinx i2c bus interface Michal Simek
2011-02-22 14:22 ` Michal Simek
2011-02-22 14:22 ` [PATCH v2 2/2] i2c: xiic: Use 32bit accesses only Michal Simek
[not found] ` <1298384530-14994-1-git-send-email-monstr-pSz03upnqPeHXe+LvDLADg@public.gmane.org>
2011-02-22 15:52 ` [PATCH v2 1/2] i2c: xiic: Add OF support for Xilinx i2c bus interface Grant Likely
2011-02-22 15:52 ` Grant Likely
[not found] ` <AANLkTi=FvafUgqMCF29cX7egviDwrfB=sViudLr+4Fsx-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2011-02-22 17:52 ` Michal Simek
2011-02-22 17:52 ` Michal Simek
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.