Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754517Ab0GZLaa (ORCPT ); Mon, 26 Jul 2010 07:30:30 -0400 Received: from eu1sys200aog112.obsmtp.com ([207.126.144.133]:58985 "EHLO eu1sys200aog112.obsmtp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753969Ab0GZLa2 (ORCPT ); Mon, 26 Jul 2010 07:30:28 -0400 X-Greylist: delayed 1448 seconds by postgrey-1.27 at vger.kernel.org; Mon, 26 Jul 2010 07:30:26 EDT Subject: Re: [PATCH] i2c: QUP based bus driver for Qualcomm MSM chipsets From: srinidhi Reply-To: To: Kenneth Heitke Cc: "khali@linux-fr.org" , "ben-linux@fluff.org" , "linux-arm-msm@vger.kernel.org" , Crane Cai , Samuel Ortiz , Linus WALLEIJ , Ralf Baechle , "linux-i2c@vger.kernel.org" , "linux-kernel@vger.kernel.org" In-Reply-To: <1279853264-16311-1-git-send-email-kheitke@codeaurora.org> References: <1279853264-16311-1-git-send-email-kheitke@codeaurora.org> Content-Type: text/plain Organization: STE Date: Mon, 26 Jul 2010 16:35:50 +0530 Message-ID: <1280142350.4705.809.camel@bnru03> MIME-Version: 1.0 X-Mailer: Evolution 2.24.3 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 35198 Lines: 911 On Fri, 2010-07-23 at 04:47 +0200, Kenneth Heitke wrote: > This bus driver supports the QUP i2c hardware controller in the Qualcomm > MSM SOCs. The Qualcomm Universal Peripheral Engine (QUP) is a general > purpose data path engine with input/output FIFOs and an embedded i2c > mini-core. The driver supports FIFO mode (for low bandwidth applications) > and block mode (interrupt generated for each block-size data transfer). > The driver currently does not support DMA transfers. > > Signed-off-by: Kenneth Heitke > --- > drivers/i2c/busses/Kconfig | 12 + > drivers/i2c/busses/Makefile | 1 + > drivers/i2c/busses/i2c-qup.c | 1080 ++++++++++++++++++++++++++++++++++++++++++ > include/linux/i2c-msm.h | 29 ++ > 4 files changed, 1122 insertions(+), 0 deletions(-) > create mode 100644 drivers/i2c/busses/i2c-qup.c > create mode 100644 include/linux/i2c-msm.h > > diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig > index bceafbf..03d8f8f 100644 > --- a/drivers/i2c/busses/Kconfig > +++ b/drivers/i2c/busses/Kconfig > @@ -521,6 +521,18 @@ config I2C_PXA_SLAVE > is necessary for systems where the PXA may be a target on the > I2C bus. > > +config I2C_QUP > + tristate "Qualcomm MSM QUP I2C Controller" > + depends on I2C && HAVE_CLK && (ARCH_MSM7X30 || ARCH_MSM8X60 || \ > + (ARCH_QSD8X50 && MSM_SOC_REV_A)) I think HAVE_CLK is redundant here > + help > + If you say yes to this option, support will be included for the > + built-in QUP I2C interface on Qualcomm MSM family processors. > + > + The Qualcomm Universal Peripheral Engine (QUP) is a general > + purpose data path engine with input/output FIFOs and an > + embedded I2C mini-core. > + > config I2C_S3C2410 > tristate "S3C2410 I2C Driver" > depends on ARCH_S3C2410 || ARCH_S3C64XX > diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile > index 936880b..6a52572 100644 > --- a/drivers/i2c/busses/Makefile > +++ b/drivers/i2c/busses/Makefile > @@ -50,6 +50,7 @@ obj-$(CONFIG_I2C_PCA_PLATFORM) += i2c-pca-platform.o > obj-$(CONFIG_I2C_PMCMSP) += i2c-pmcmsp.o > obj-$(CONFIG_I2C_PNX) += i2c-pnx.o > obj-$(CONFIG_I2C_PXA) += i2c-pxa.o > +obj-$(CONFIG_I2C_QUP) += i2c-qup.o > obj-$(CONFIG_I2C_S3C2410) += i2c-s3c2410.o > obj-$(CONFIG_I2C_S6000) += i2c-s6000.o > obj-$(CONFIG_I2C_SH7760) += i2c-sh7760.o > diff --git a/drivers/i2c/busses/i2c-qup.c b/drivers/i2c/busses/i2c-qup.c > new file mode 100644 > index 0000000..3d7abab > --- /dev/null > +++ b/drivers/i2c/busses/i2c-qup.c > @@ -0,0 +1,1080 @@ > +/* Copyright (c) 2009-2010, Code Aurora Forum. All rights reserved. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 and > + * only 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., 51 Franklin Street, Fifth Floor, Boston, MA > + * 02110-1301, USA. > + * > + */ > +/* > + * QUP driver for Qualcomm MSM platforms > + * > + */ > + > +/* #define DEBUG */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include I do not understand why yo need to expose this controller's private data to the whole Linux world. Shouldn't this be ? > + > +MODULE_LICENSE("GPL v2"); > +MODULE_VERSION("0.2"); > +MODULE_ALIAS("platform:i2c_qup"); > +MODULE_AUTHOR("Sagar Dharia "); > + > +/* QUP Registers */ > +enum { > + QUP_CONFIG = 0x0, > + QUP_STATE = 0x4, > + QUP_IO_MODE = 0x8, > + QUP_SW_RESET = 0xC, > + QUP_OPERATIONAL = 0x18, > + QUP_ERROR_FLAGS = 0x1C, > + QUP_ERROR_FLAGS_EN = 0x20, > + QUP_MX_READ_CNT = 0x208, > + QUP_MX_INPUT_CNT = 0x200, > + QUP_MX_WR_CNT = 0x100, > + QUP_OUT_DEBUG = 0x108, > + QUP_OUT_FIFO_CNT = 0x10C, > + QUP_OUT_FIFO_BASE = 0x110, > + QUP_IN_READ_CUR = 0x20C, > + QUP_IN_DEBUG = 0x210, > + QUP_IN_FIFO_CNT = 0x214, > + QUP_IN_FIFO_BASE = 0x218, > + QUP_I2C_CLK_CTL = 0x400, > + QUP_I2C_STATUS = 0x404, > +}; anonymous? > + > +/* QUP States and reset values */ > +enum { > + QUP_RESET_STATE = 0, > + QUP_RUN_STATE = 1U, > + QUP_STATE_MASK = 3U, > + QUP_PAUSE_STATE = 3U, > + QUP_STATE_VALID = 1U << 2, > + QUP_I2C_MAST_GEN = 1U << 4, > + QUP_OPERATIONAL_RESET = 0xFF0, > + QUP_I2C_STATUS_RESET = 0xFFFFFC, > +}; anonymous, ditto for all the following enums. > + > +/* QUP OPERATIONAL FLAGS */ > +enum { > + QUP_OUT_SVC_FLAG = 1U << 8, > + QUP_IN_SVC_FLAG = 1U << 9, > + QUP_MX_INPUT_DONE = 1U << 11, > +}; > + > +/* I2C mini core related values */ > +enum { > + I2C_MINI_CORE = 2U << 8, > + I2C_N_VAL = 0xF, > +}; > + > +/* Packing Unpacking words in FIFOs , and IO modes*/ > +enum { > + QUP_WR_BLK_MODE = 1U << 10, > + QUP_RD_BLK_MODE = 1U << 12, > + QUP_UNPACK_EN = 1U << 14, > + QUP_PACK_EN = 1U << 15, > +}; > + > +/* QUP tags */ > +enum { > + QUP_OUT_NOP = 0, > + QUP_OUT_START = 1U << 8, > + QUP_OUT_DATA = 2U << 8, > + QUP_OUT_STOP = 3U << 8, > + QUP_OUT_REC = 4U << 8, > + QUP_IN_DATA = 5U << 8, > + QUP_IN_STOP = 6U << 8, > + QUP_IN_NACK = 7U << 8, > +}; > + > +/* Status, Error flags */ > +enum { > + I2C_STATUS_WR_BUFFER_FULL = 1U << 0, > + I2C_STATUS_BUS_ACTIVE = 1U << 8, > + I2C_STATUS_ERROR_MASK = 0x38000FC, > + QUP_I2C_NACK_FLAG = 1U << 3, > + QUP_IN_NOT_EMPTY = 1U << 5, > + QUP_STATUS_ERROR_FLAGS = 0x7C, > +}; > + > +/* GSBI Control Register */ > +enum { > + GSBI_I2C_PROTOCOL_CODE = 0x2 << 4, /* I2C protocol */ > +}; > + > +#define QUP_MAX_RETRIES 2000 > +#define QUP_SRC_CLK_RATE 19200000 /* Default source clock rate */ why do want this while having this module dependent on HAVE_CLK > + > +struct qup_i2c_dev { > + struct device *dev; > + void __iomem *base; /* virtual */ > + void __iomem *gsbi; /* virtual */ > + int in_irq; > + int out_irq; > + int err_irq; > + int num_irqs; > + struct clk *clk; > + struct clk *pclk; > + struct i2c_adapter adapter; > + > + struct i2c_msg *msg; > + int pos; > + int cnt; > + int err; > + int mode; > + int clk_ctl; > + int one_bit_t; > + int out_fifo_sz; > + int in_fifo_sz; > + int out_blk_sz; > + int in_blk_sz; > + int wr_sz; > + struct msm_i2c_platform_data *pdata; > + int suspended; > + int clk_state; > + struct timer_list pwr_timer; > + struct mutex mlock; > + void *complete; > +}; too many local parameters. Do you really need all of this? Moreover it is not readable. Would suggest to document it at least using kernel-doc style. > + > +#ifdef DEBUG > +static void > +qup_print_status(struct qup_i2c_dev *dev) > +{ > + uint32_t val; > + val = readl(dev->base+QUP_CONFIG); space after & before '+'. checkpatch would've complained this about.. > + dev_dbg(dev->dev, "Qup config is :0x%x\n", val); > + val = readl(dev->base+QUP_STATE); ditto > + dev_dbg(dev->dev, "Qup state is :0x%x\n", val); > + val = readl(dev->base+QUP_IO_MODE); ditto > + dev_dbg(dev->dev, "Qup mode is :0x%x\n", val); > +} > +#else > +static inline void qup_print_status(struct qup_i2c_dev *dev) > +{ > +} > +#endif > + > +static irqreturn_t > +qup_i2c_interrupt(int irq, void *devid) > +{ > + struct qup_i2c_dev *dev = devid; > + uint32_t status = readl(dev->base + QUP_I2C_STATUS); > + uint32_t errors = readl(dev->base + QUP_ERROR_FLAGS); > + uint32_t op_flgs = readl(dev->base + QUP_OPERATIONAL); > + int err = 0; > + > + if (!dev->msg) > + return IRQ_HANDLED; > + > + if (status & I2C_STATUS_ERROR_MASK) { > + dev_err(dev->dev, "QUP: I2C status flags :0x%x, irq:%d\n", > + status, irq); > + err = -status; > + /* Clear Error interrupt if it's a level triggered interrupt*/ /* ... */ > + if (dev->num_irqs == 1) > + writel(QUP_RESET_STATE, dev->base+QUP_STATE); > + goto intr_done; > + } > + > + if (errors & QUP_STATUS_ERROR_FLAGS) { > + dev_err(dev->dev, "QUP: QUP status flags :0x%x\n", errors); > + err = -errors; > + /* Clear Error interrupt if it's a level triggered interrupt*/ ditto > + if (dev->num_irqs == 1) > + writel(errors & QUP_STATUS_ERROR_FLAGS, > + dev->base + QUP_ERROR_FLAGS); > + goto intr_done; > + } > + > + if ((dev->num_irqs == 3) && (dev->msg->flags == I2C_M_RD) > + && (irq == dev->out_irq)) > + return IRQ_HANDLED; > + if (op_flgs & QUP_OUT_SVC_FLAG) > + writel(QUP_OUT_SVC_FLAG, dev->base + QUP_OPERATIONAL); > + if (dev->msg->flags == I2C_M_RD) { > + if ((op_flgs & QUP_MX_INPUT_DONE) || > + (op_flgs & QUP_IN_SVC_FLAG)) > + writel(QUP_IN_SVC_FLAG, dev->base + QUP_OPERATIONAL); > + else > + return IRQ_HANDLED; > + } > + > +intr_done: > + dev_dbg(dev->dev, "QUP intr= %d, i2c status=0x%x, qup status = 0x%x\n", > + irq, status, errors); > + qup_print_status(dev); > + dev->err = err; > + complete(dev->complete); > + return IRQ_HANDLED; > +} > + > +static void > +qup_i2c_pwr_mgmt(struct qup_i2c_dev *dev, unsigned int state) > +{ > + dev->clk_state = state; > + if (state != 0) { > + clk_enable(dev->clk); > + if (dev->pclk) > + clk_enable(dev->pclk); > + } else { > + clk_disable(dev->clk); > + if (dev->pclk) > + clk_disable(dev->pclk); > + } > +} > + > +static void > +qup_i2c_pwr_timer(unsigned long data) > +{ > + struct qup_i2c_dev *dev = (struct qup_i2c_dev *) data; > + dev_dbg(dev->dev, "QUP_Power: Inactivity based power management\n"); > + if (dev->clk_state == 1) > + qup_i2c_pwr_mgmt(dev, 0); > +} > + > +static int > +qup_i2c_poll_writeready(struct qup_i2c_dev *dev) > +{ > + uint32_t retries = 0; > + > + while (retries != QUP_MAX_RETRIES) { > + uint32_t status = readl(dev->base + QUP_I2C_STATUS); > + > + if (!(status & I2C_STATUS_WR_BUFFER_FULL)) { > + if (!(status & I2C_STATUS_BUS_ACTIVE)) > + return 0; > + else /* 1-bit delay before we check for bus busy */ > + udelay(dev->one_bit_t); > + } > + if (retries++ == 1000) > + udelay(100); > + } > + qup_print_status(dev); > + return -ETIMEDOUT; > +} > + > +static int > +qup_i2c_poll_state(struct qup_i2c_dev *dev, uint32_t state) > +{ > + uint32_t retries = 0; > + > + dev_dbg(dev->dev, "Polling Status for state:0x%x\n", state); &dev->dev > + > + while (retries != QUP_MAX_RETRIES) { > + uint32_t status = readl(dev->base + QUP_STATE); > + > + if ((status & (QUP_STATE_VALID | state)) == > + (QUP_STATE_VALID | state)) > + return 0; > + else if (retries++ == 1000) > + udelay(100); > + } > + return -ETIMEDOUT; > +} > + > +#ifdef DEBUG > +static void qup_verify_fifo(struct qup_i2c_dev *dev, uint32_t val, > + uint32_t addr, int rdwr) > +{ > + if (rdwr) > + dev_dbg(dev->dev, "RD:Wrote 0x%x to out_ff:0x%x\n", val, addr); > + else > + dev_dbg(dev->dev, "WR:Wrote 0x%x to out_ff:0x%x\n", val, addr); > +} > +#else > +static inline void qup_verify_fifo(struct qup_i2c_dev *dev, uint32_t val, > + uint32_t addr, int rdwr) > +{ > +} > +#endif > + > +static void > +qup_issue_read(struct qup_i2c_dev *dev, struct i2c_msg *msg, int *idx, > + uint32_t carry_over) > +{ > + uint16_t addr = (msg->addr << 1) | 1; > + /* QUP limit 256 bytes per read. By HW design, 0 in the 8-bit field > + * is treated as 256 byte read. > + */ multi line comments would be neater, as you have used it in other places of this code. > + uint16_t rd_len = ((dev->cnt == 256) ? 0 : dev->cnt); > + > + if (*idx % 4) { > + writel(carry_over | ((QUP_OUT_START | addr) << 16), > + dev->base + QUP_OUT_FIFO_BASE);/* + (*idx-2)); */ remove the dead commented code above and below as it is not carrying any meaningful thoughts about the calculation > + > + qup_verify_fifo(dev, carry_over | > + ((QUP_OUT_START | addr) << 16), (uint32_t)dev->base > + + QUP_OUT_FIFO_BASE + (*idx - 2), 1); qup_verify_fifo is _not_ actually verifying anything, better remove this function and its usage.. > + writel((QUP_OUT_REC | rd_len), > + dev->base + QUP_OUT_FIFO_BASE);/* + (*idx+2)); */ > + > + qup_verify_fifo(dev, (QUP_OUT_REC | rd_len), > + (uint32_t)dev->base + QUP_OUT_FIFO_BASE + (*idx + 2), 1); > + } else { > + writel(((QUP_OUT_REC | rd_len) << 16) | QUP_OUT_START | addr, > + dev->base + QUP_OUT_FIFO_BASE);/* + (*idx)); */ > + > + qup_verify_fifo(dev, QUP_OUT_REC << 16 | rd_len << 16 | > + QUP_OUT_START | addr, > + (uint32_t)dev->base + QUP_OUT_FIFO_BASE + (*idx), 1); > + } > + *idx += 4; > +} > + > +static void > +qup_issue_write(struct qup_i2c_dev *dev, struct i2c_msg *msg, int rem, > + int *idx, uint32_t *carry_over) > +{ > + int entries = dev->cnt; > + int empty_sl = dev->wr_sz - ((*idx) >> 1); > + int i = 0; > + uint32_t val = 0; > + uint32_t last_entry = 0; > + uint16_t addr = msg->addr << 1; > + > + if (dev->pos == 0) { > + if (*idx % 4) { > + writel(*carry_over | ((QUP_OUT_START | addr) << 16), > + dev->base + QUP_OUT_FIFO_BASE); > + > + qup_verify_fifo(dev, *carry_over | QUP_OUT_DATA << 16 | > + addr << 16, (uint32_t)dev->base + > + QUP_OUT_FIFO_BASE + (*idx) - 2, 0); > + } else > + val = QUP_OUT_START | addr; > + *idx += 2; > + i++; > + entries++; > + } else { > + /* Avoid setp time issue by adding 1 NOP when number of bytes s/setp/setup > + * are more than FIFO/BLOCK size. setup time issue can't appear > + * otherwise since next byte to be written will always be ready > + */ > + val = (QUP_OUT_NOP | 1); > + *idx += 2; > + i++; > + entries++; > + } > + if (entries > empty_sl) > + entries = empty_sl; > + > + for (; i < (entries - 1); i++) { > + if (*idx % 4) { > + writel(val | ((QUP_OUT_DATA | > + msg->buf[dev->pos]) << 16), > + dev->base + QUP_OUT_FIFO_BASE); > + > + qup_verify_fifo(dev, val | QUP_OUT_DATA << 16 | > + msg->buf[dev->pos] << 16, (uint32_t)dev->base + > + QUP_OUT_FIFO_BASE + (*idx) - 2, 0); > + } else > + val = QUP_OUT_DATA | msg->buf[dev->pos]; > + (*idx) += 2; > + dev->pos++; > + } > + if (dev->pos < (msg->len - 1)) > + last_entry = QUP_OUT_DATA; > + else if (rem > 1) /* not last array entry */ > + last_entry = QUP_OUT_DATA; > + else > + last_entry = QUP_OUT_STOP; > + if ((*idx % 4) == 0) { > + /* > + * If read-start and read-command end up in different fifos, it > + * may result in extra-byte being read due to extra-read cycle. > + * Avoid that by inserting NOP as the last entry of fifo only > + * if write command(s) leave 1 space in fifo. > + */ > + if (rem > 1) { > + struct i2c_msg *next = msg + 1; > + if (next->addr == msg->addr && (next->flags | I2C_M_RD) > + && *idx == ((dev->wr_sz*2) - 4)) { > + writel(((last_entry | msg->buf[dev->pos]) | > + ((1 | QUP_OUT_NOP) << 16)), dev->base + > + QUP_OUT_FIFO_BASE);/* + (*idx) - 2); */ > + *idx += 2; > + } else > + *carry_over = (last_entry | msg->buf[dev->pos]); > + } else { > + writel((last_entry | msg->buf[dev->pos]), > + dev->base + QUP_OUT_FIFO_BASE);/* + (*idx) - 2); */ > + > + qup_verify_fifo(dev, last_entry | msg->buf[dev->pos], > + (uint32_t)dev->base + QUP_OUT_FIFO_BASE + > + (*idx), 0); wrap this around, should be possible within 80 chars.. > + } > + } else { > + writel(val | ((last_entry | msg->buf[dev->pos]) << 16), > + dev->base + QUP_OUT_FIFO_BASE);/* + (*idx) - 2); */ > + > + qup_verify_fifo(dev, val | (last_entry << 16) | > + (msg->buf[dev->pos] << 16), (uint32_t)dev->base + > + QUP_OUT_FIFO_BASE + (*idx) - 2, 0); > + } > + > + *idx += 2; > + dev->pos++; > + dev->cnt = msg->len - dev->pos; > +} > + > +static int > +qup_update_state(struct qup_i2c_dev *dev, uint32_t state) > +{ > + if (qup_i2c_poll_state(dev, 0) != 0) > + return -EIO; > + writel(state, dev->base + QUP_STATE); > + if (qup_i2c_poll_state(dev, state) != 0) > + return -EIO; > + return 0; > +} > + > +static int > +qup_set_read_mode(struct qup_i2c_dev *dev, int rd_len) > +{ > + uint32_t wr_mode = (dev->wr_sz < dev->out_fifo_sz) ? > + QUP_WR_BLK_MODE : 0; > + if (rd_len > 256) { > + dev_err(dev->dev, "HW doesn't support READs > 256 bytes\n"); Shouldn't this be &dev->dev? > + return -EPROTONOSUPPORT; AFAIK this is not an i2c fault error code at all. > + } > + if (rd_len <= dev->in_fifo_sz) { > + writel(wr_mode | QUP_PACK_EN | QUP_UNPACK_EN, > + dev->base + QUP_IO_MODE); > + writel(rd_len, dev->base + QUP_MX_READ_CNT); > + } else { > + writel(wr_mode | QUP_RD_BLK_MODE | > + QUP_PACK_EN | QUP_UNPACK_EN, dev->base + QUP_IO_MODE); > + writel(rd_len, dev->base + QUP_MX_INPUT_CNT); > + } > + return 0; > +} > + > +static int > +qup_set_wr_mode(struct qup_i2c_dev *dev, int rem) > +{ > + int total_len = 0; > + int ret = 0; > + if (dev->msg->len >= (dev->out_fifo_sz - 1)) { > + total_len = dev->msg->len + 1 + > + (dev->msg->len/(dev->out_blk_sz-1)); > + writel(QUP_WR_BLK_MODE | QUP_PACK_EN | QUP_UNPACK_EN, > + dev->base + QUP_IO_MODE); > + dev->wr_sz = dev->out_blk_sz; > + } else > + writel(QUP_PACK_EN | QUP_UNPACK_EN, > + dev->base + QUP_IO_MODE); > + > + if (rem > 1) { > + struct i2c_msg *next = dev->msg + 1; > + if (next->addr == dev->msg->addr && > + next->flags == I2C_M_RD) { > + ret = qup_set_read_mode(dev, next->len); > + /* make sure read start & read command are in 1 blk */ > + if ((total_len % dev->out_blk_sz) == > + (dev->out_blk_sz - 1)) > + total_len += 3; > + else > + total_len += 2; > + } > + } > + /* WRITE COUNT register valid/used only in block mode */ > + if (dev->wr_sz == dev->out_blk_sz) > + writel(total_len, dev->base + QUP_MX_WR_CNT); > + return ret; > +} > + > +static int > +qup_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num) > +{ > + DECLARE_COMPLETION_ONSTACK(complete); > + struct qup_i2c_dev *dev = i2c_get_adapdata(adap); > + int ret; > + int rem = num; > + long timeout; > + int err; > + > + del_timer_sync(&dev->pwr_timer); > + mutex_lock(&dev->mlock); > + > + if (dev->suspended) { > + mutex_unlock(&dev->mlock); > + return -EIO; you have failed here not because of any faulty IO operation. Would suggest to use the proper error codes accordingly. > + } > + > + if (dev->clk_state == 0) { > + if (dev->clk_ctl == 0) { > + if (dev->pdata->src_clk_rate > 0) > + clk_set_rate(dev->clk, > + dev->pdata->src_clk_rate); > + else > + dev->pdata->src_clk_rate = QUP_SRC_CLK_RATE; > + } > + qup_i2c_pwr_mgmt(dev, 1); > + } > + > + /* Initialize QUP registers during first transfer */ > + if (dev->clk_ctl == 0) { > + int fs_div; > + int hs_div; > + uint32_t fifo_reg; > + writel(GSBI_I2C_PROTOCOL_CODE, dev->gsbi); > + > + fs_div = ((dev->pdata->src_clk_rate > + / dev->pdata->clk_freq) / 2) - 3; > + hs_div = 3; > + dev->clk_ctl = ((hs_div & 0x7) << 8) | (fs_div & 0xff); > + fifo_reg = readl(dev->base + QUP_IO_MODE); > + if (fifo_reg & 0x3) > + dev->out_blk_sz = (fifo_reg & 0x3) * 16; > + else > + dev->out_blk_sz = 16; > + if (fifo_reg & 0x60) > + dev->in_blk_sz = ((fifo_reg & 0x60) >> 5) * 16; > + else > + dev->in_blk_sz = 16; > + /* > + * The block/fifo size w.r.t. 'actual data' is 1/2 due to 'tag' > + * associated with each byte written/received > + */ > + dev->out_blk_sz /= 2; > + dev->in_blk_sz /= 2; > + dev->out_fifo_sz = dev->out_blk_sz * > + (2 << ((fifo_reg & 0x1C) >> 2)); > + dev->in_fifo_sz = dev->in_blk_sz * > + (2 << ((fifo_reg & 0x380) >> 7)); > + dev_dbg(dev->dev, "QUP IN:bl:%d, ff:%d, OUT:bl:%d, ff:%d\n", > + dev->in_blk_sz, dev->in_fifo_sz, > + dev->out_blk_sz, dev->out_fifo_sz); > + } > + > + if (dev->num_irqs == 3) { > + enable_irq(dev->in_irq); > + enable_irq(dev->out_irq); > + } > + enable_irq(dev->err_irq); > + writel(1, dev->base + QUP_SW_RESET); > + ret = qup_i2c_poll_state(dev, QUP_RESET_STATE); > + if (ret) { > + dev_err(dev->dev, "QUP Busy:Trying to recover\n"); > + goto out_err; > + } > + > + /* Initialize QUP registers */ > + writel(0, dev->base + QUP_CONFIG); > + writel(QUP_OPERATIONAL_RESET, dev->base + QUP_OPERATIONAL); > + writel(QUP_STATUS_ERROR_FLAGS, dev->base + QUP_ERROR_FLAGS_EN); > + > + writel(I2C_MINI_CORE | I2C_N_VAL, dev->base + QUP_CONFIG); > + > + /* Initialize I2C mini core registers */ > + writel(0, dev->base + QUP_I2C_CLK_CTL); > + writel(QUP_I2C_STATUS_RESET, dev->base + QUP_I2C_STATUS); > + > + dev->cnt = msgs->len; > + dev->pos = 0; > + dev->msg = msgs; > + while (rem) { > + bool filled = false; > + > + dev->wr_sz = dev->out_fifo_sz; > + dev->err = 0; > + dev->complete = &complete; > + > + if (qup_i2c_poll_state(dev, QUP_I2C_MAST_GEN) != 0) { > + ret = -EIO; > + goto out_err; > + } > + > + qup_print_status(dev); > + /* HW limits Read upto 256 bytes in 1 read without stop */ > + if (dev->msg->flags & I2C_M_RD) { > + ret = qup_set_read_mode(dev, dev->cnt); > + if (ret != 0) > + goto out_err; > + } else { > + ret = qup_set_wr_mode(dev, rem); > + if (ret != 0) > + goto out_err; > + /* Don't fill block till we get interrupt */ > + if (dev->wr_sz == dev->out_blk_sz) > + filled = true; > + } > + > + err = qup_update_state(dev, QUP_RUN_STATE); > + if (err < 0) { > + ret = err; > + goto out_err; > + } > + > + qup_print_status(dev); > + writel(dev->clk_ctl, dev->base + QUP_I2C_CLK_CTL); > + > + do { > + int idx = 0; > + uint32_t carry_over = 0; > + > + /* Transition to PAUSE state only possible from RUN */ > + err = qup_update_state(dev, QUP_PAUSE_STATE); > + if (err < 0) { > + ret = err; > + goto out_err; > + } > + > + qup_print_status(dev); > + /* This operation is Write, check the next operation > + * and decide mode > + */ use consistent commenting style > + while (filled == false) { > + if ((msgs->flags & I2C_M_RD) && > + (dev->cnt == msgs->len)) > + qup_issue_read(dev, msgs, &idx, > + carry_over); > + else if (!(msgs->flags & I2C_M_RD)) > + qup_issue_write(dev, msgs, rem, &idx, > + &carry_over); > + if (idx >= (dev->wr_sz << 1)) > + filled = true; > + /* Start new message */ > + if (filled == false) { > + if (msgs->flags & I2C_M_RD) > + filled = true; > + else if (rem > 1) { > + /* Only combine operations with > + * same address > + */ > + struct i2c_msg *next = msgs + 1; > + if (next->addr != msgs->addr || > + next->flags == 0) > + filled = true; > + else { > + rem--; > + msgs++; > + dev->msg = msgs; > + dev->pos = 0; > + dev->cnt = msgs->len; > + } > + } else > + filled = true; > + } > + } > + err = qup_update_state(dev, QUP_RUN_STATE); > + if (err < 0) { > + ret = err; > + goto out_err; > + } > + dev_dbg(dev->dev, "idx:%d, rem:%d, num:%d, mode:%d\n", > + idx, rem, num, dev->mode); > + > + qup_print_status(dev); > + timeout = wait_for_completion_timeout(&complete, > + msecs_to_jiffies(dev->out_fifo_sz)); what is this out_fifo_sz doing here? > + if (!timeout) { > + dev_err(dev->dev, "Transaction timed out\n"); > + writel(1, dev->base + QUP_SW_RESET); > + ret = -ETIMEDOUT; > + goto out_err; > + } > + if (dev->err) { > + if (dev->err & QUP_I2C_NACK_FLAG) > + dev_err(dev->dev, > + "I2C slave addr:0x%x not connected\n", > + dev->msg->addr); > + else > + dev_err(dev->dev, > + "QUP data xfer error %d\n", dev->err); > + ret = dev->err; > + goto out_err; > + } > + if (dev->msg->flags & I2C_M_RD) { > + int i; > + uint32_t dval = 0; > + for (i = 0; dev->pos < dev->msg->len; i++, > + dev->pos++) { > + uint32_t rd_status = readl(dev->base + > + QUP_OPERATIONAL); > + if (i % 2 == 0) { > + if ((rd_status & > + QUP_IN_NOT_EMPTY) == 0) > + break; > + dval = readl(dev->base + > + QUP_IN_FIFO_BASE); > + dev->msg->buf[dev->pos] = > + dval & 0xFF; > + } else > + dev->msg->buf[dev->pos] = > + ((dval & 0xFF0000) >> > + 16); > + } > + dev->cnt -= i; > + } else > + filled = false; /* refill output FIFO */ > + } while (dev->cnt > 0); > + if (dev->cnt == 0) { > + rem--; > + msgs++; > + if (rem) { > + dev->pos = 0; > + dev->cnt = msgs->len; > + dev->msg = msgs; > + } > + } > + /* Wait for I2C bus to be idle */ > + ret = qup_i2c_poll_writeready(dev); > + if (ret) { > + dev_err(dev->dev, > + "Error waiting for write ready\n"); > + goto out_err; > + } > + } > + > + ret = num; > + out_err: > + dev->complete = NULL; > + dev->msg = NULL; > + dev->pos = 0; > + dev->err = 0; > + dev->cnt = 0; > + disable_irq(dev->err_irq); > + if (dev->num_irqs == 3) { > + disable_irq(dev->in_irq); > + disable_irq(dev->out_irq); > + } > + dev->pwr_timer.expires = jiffies + 3*HZ; > + add_timer(&dev->pwr_timer); > + mutex_unlock(&dev->mlock); > + return ret; > +} (...) Srinidhi -- 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/