From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jamie Iles Subject: Re: [PATCH] I2C: add CSR SiRFprimaII on-chip I2C controllers driver Date: Wed, 2 Nov 2011 10:39:04 +0000 Message-ID: <20111102103904.GB4937@totoro> References: <1320228610-18129-1-git-send-email-Barry.Song@csr.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: Content-Disposition: inline In-Reply-To: <1320228610-18129-1-git-send-email-Barry.Song-kQvG35nSl+M@public.gmane.org> Sender: linux-i2c-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Barry Song Cc: ben-linux-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org, linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Barry Song , Xiangzhen Ye , workgroup.linux-kQvG35nSl+M@public.gmane.org, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, Zhiwu Song List-Id: linux-i2c@vger.kernel.org Hi Barry, I'm not too familiar with I2C, so just a couple of general comments=20 inline, but otherwise looks good to me. Jamie On Wed, Nov 02, 2011 at 03:10:10AM -0700, Barry Song wrote: > From: Zhiwu Song >=20 > SiRFprimaII is the latest generation application processor from CSR=E2= =80=99s > multi-function SoC product family. > The SoC support codes are in arch/arm/mach-prima2 from Linux mainline > 3.0. > There are two I2C controllers on primaII, features include: > =E2=96=A0 Two I2C controller modules are on chip > =E2=96=A0 RISC I/O bus read write register > =E2=96=A0 Up to 16 bytes data buffer for issuing commands and writing= data at the same time > =E2=96=A0 Up to 16 commands, and receiving read data 16 bytes at a ti= me > =E2=96=A0 Error INT report (ACK check) > =E2=96=A0 No-ACK bus protocols (SCCB bus protocols) >=20 > Signed-off-by: Zhiwu Song > Signed-off-by: Xiangzhen Ye > Signed-off-by: Barry Song > --- [...] > diff --git a/drivers/i2c/busses/i2c-sirf.c b/drivers/i2c/busses/i2c-s= irf.c > new file mode 100644 > index 0000000..d4ccba7 > --- /dev/null > +++ b/drivers/i2c/busses/i2c-sirf.c > @@ -0,0 +1,425 @@ > +/* > + * I2C bus driver for CSR SiRFprimaII > + * > + * Copyright (c) 2011 Cambridge Silicon Radio Limited, a CSR plc gro= up company. > + * > + * Licensed under GPLv2 or later. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include "i2c-sirf.h" > + > +#define SIRFSOC_I2C_DEFAULT_SPEED 100000 > + > +struct sirfsoc_i2c { > + void *base; This should be void __iomem *base; > + struct clk *clk; > + unsigned long speed; /* I2C SCL frequency */ > + int irq; > + u32 cmd_ptr; /* Current position in CMD buffer */ > + u8 *buf; /* Buffer passed by user */ > + u32 msg_len; /* Message length */ > + u32 finished_len; /* number of bytes read/written */ > + u32 read_cmd_len; /* number of read cmd sent */ > + int msg_read; /* 1 indicates a read message */ > + int err_status; /* 1 indicates an error on bus */ > + > + u32 sda_delay; /* For suspend/resume */ > + u32 clk_div; > + int last; /* Last message in transfer, STOP cmd can be sent */ > + > + struct completion done; /* indicates completion of message transfer= */ > + struct i2c_adapter *adapter; > +}; > + > +static void i2c_sirfsoc_read_data(struct sirfsoc_i2c *siic) > +{ > + u32 data =3D 0; > + int i =3D 0; > + > + for (i =3D 0; i < siic->read_cmd_len; i++) { > + if (!(i & 0x3)) > + data =3D readl(siic->base + SIRFSOC_I2C_DATA_BUF + i); > + siic->buf[siic->finished_len++] =3D > + (unsigned char)((data & SIRFSOC_I2C_DATA_MASK(i)) >> > + SIRFSOC_I2C_DATA_SHIFT(i)); > + > + BUG_ON(siic->finished_len > siic->msg_len); > + } > +} > + > +static void i2c_sirfsoc_queue_cmd(struct sirfsoc_i2c *siic) > +{ > + u32 regval; > + int i =3D 0; > + > + if (siic->msg_read) { > + while (((siic->finished_len + i) < siic->msg_len) > + && (siic->cmd_ptr < SIRFSOC_I2C_CMD_BUF_MAX)) { > + regval =3D SIRFSOC_I2C_READ | SIRFSOC_I2C_CMD_RP(0); > + if (((siic->finished_len + i) =3D=3D > + (siic->msg_len - 1)) && siic->last) > + regval |=3D SIRFSOC_I2C_STOP | SIRFSOC_I2C_NACK; > + writel(regval, > + siic->base + SIRFSOC_I2C_CMD(siic->cmd_ptr++)); > + i++; > + } > + > + siic->read_cmd_len =3D i; > + } else { > + while ((siic->cmd_ptr < SIRFSOC_I2C_CMD_BUF_MAX - 1) > + && (siic->finished_len < siic->msg_len)) { > + regval =3D SIRFSOC_I2C_WRITE | SIRFSOC_I2C_CMD_RP(0); > + if ((siic->finished_len =3D=3D (siic->msg_len - 1)) > + && siic->last) > + regval |=3D SIRFSOC_I2C_STOP; > + writel(regval, > + siic->base + SIRFSOC_I2C_CMD(siic->cmd_ptr++)); > + writel(siic->buf[siic->finished_len++], > + siic->base + SIRFSOC_I2C_CMD(siic->cmd_ptr++)); > + } > + } > + siic->cmd_ptr =3D 0; > + > + /* Trigger the transfer */ > + writel(SIRFSOC_I2C_START_CMD, siic->base + SIRFSOC_I2C_CMD_START); > +} > + > +static irqreturn_t i2c_sirfsoc_irq(int irq, void *dev_id) > +{ > + struct sirfsoc_i2c *siic =3D (struct sirfsoc_i2c *)dev_id; > + u32 i2c_stat =3D readl(siic->base + SIRFSOC_I2C_STATUS); > + > + if (i2c_stat & SIRFSOC_I2C_STAT_ERR) { > + /* Error conditions */ > + siic->err_status =3D 1; > + writel(SIRFSOC_I2C_STAT_ERR, siic->base + SIRFSOC_I2C_STATUS); > + > + if (i2c_stat & SIRFSOC_I2C_STAT_NACK) > + dev_err(&siic->adapter->dev, "ACK not received\n"); > + else > + dev_err(&siic->adapter->dev, "I2C error\n"); > + > + complete(&siic->done); > + } else if (i2c_stat & SIRFSOC_I2C_STAT_CMD_DONE) { > + /* CMD buffer execution complete */ > + if (siic->msg_read) > + i2c_sirfsoc_read_data(siic); > + if (siic->finished_len =3D=3D siic->msg_len) > + complete(&siic->done); > + else /* Fill a new CMD buffer for left data */ > + i2c_sirfsoc_queue_cmd(siic); > + > + writel(SIRFSOC_I2C_STAT_CMD_DONE, siic->base + SIRFSOC_I2C_STATUS)= ; > + } > + > + return IRQ_HANDLED; > +} > + > +static void i2c_sirfsoc_set_address(struct sirfsoc_i2c *siic, > + struct i2c_msg *msg) > +{ > + unsigned char addr; > + u32 regval =3D SIRFSOC_I2C_START | SIRFSOC_I2C_CMD_RP(0) | SIRFSOC_= I2C_WRITE; > + > + /* no data and last message -> add STOP */ > + if (siic->last && (msg->len =3D=3D 0)) > + regval |=3D SIRFSOC_I2C_STOP; > + > + writel(regval, siic->base + SIRFSOC_I2C_CMD(siic->cmd_ptr++)); > + > + addr =3D msg->addr << 1; /* Generate address */ > + if (msg->flags & I2C_M_RD) > + addr |=3D 1; > + if (msg->flags & I2C_M_REV_DIR_ADDR) /* Reverse direction bit */ > + addr ^=3D 1; > + > + writel(addr, siic->base + SIRFSOC_I2C_CMD(siic->cmd_ptr++)); > +} > + > +static int i2c_sirfsoc_xfer_msg(struct sirfsoc_i2c *siic, struct i2c= _msg *msg) > +{ > + u32 regval =3D readl(siic->base + SIRFSOC_I2C_CTRL); > + int timeout =3D (msg->len + 1) * 50; > + int ret =3D 0; > + > + i2c_sirfsoc_set_address(siic, msg); > + > + writel(regval | SIRFSOC_I2C_CMD_DONE_EN | SIRFSOC_I2C_ERR_INT_EN, > + siic->base + SIRFSOC_I2C_CTRL); > + i2c_sirfsoc_queue_cmd(siic); > + > + if (wait_for_completion_timeout(&siic->done, timeout) =3D=3D 0) { > + siic->err_status =3D 1; > + dev_err(&siic->adapter->dev, "Transfer timeout\n"); > + } > + > + writel(regval & ~(SIRFSOC_I2C_CMD_DONE_EN | SIRFSOC_I2C_ERR_INT_EN)= , > + siic->base + SIRFSOC_I2C_CTRL); > + writel(0, siic->base + SIRFSOC_I2C_CMD_START); > + > + if (siic->err_status) { > + writel(readl(siic->base + SIRFSOC_I2C_CTRL) | SIRFSOC_I2C_RESET, > + siic->base + SIRFSOC_I2C_CTRL); > + while (readl(siic->base + SIRFSOC_I2C_CTRL) & SIRFSOC_I2C_RESET) > + cpu_relax(); > + > + ret =3D -EIO; > + } > + > + return ret; > +} > + > +static u32 i2c_sirfsoc_func(struct i2c_adapter *adap) > +{ > + return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL; > +} > + > +static int i2c_sirfsoc_xfer(struct i2c_adapter *adap, struct i2c_msg= *msgs, > + int num) > +{ > + struct sirfsoc_i2c *siic =3D adap->algo_data; > + int i, ret; > + > + for (i =3D 0; i < num; i++) { > + siic->buf =3D msgs[i].buf; > + siic->msg_len =3D msgs[i].len; > + > + if (msgs[i].flags & I2C_M_RD) > + siic->msg_read =3D 1; > + else > + siic->msg_read =3D 0; > + > + siic->err_status =3D 0; > + siic->cmd_ptr =3D 0; > + siic->finished_len =3D 0; > + if (i =3D=3D (num - 1)) > + siic->last =3D 1; > + else > + siic->last =3D 0; > + > + ret =3D i2c_sirfsoc_xfer_msg(siic, &msgs[i]); > + if (ret) > + return ret; > + } > + > + return num; > +} > + > +/* I2C algorithms associated with this master controller driver */ > +static struct i2c_algorithm i2c_sirfsoc_algo =3D { > + .master_xfer =3D i2c_sirfsoc_xfer, > + .functionality =3D i2c_sirfsoc_func, > +}; const? > + > +static int __devinit i2c_sirfsoc_probe(struct platform_device *pdev) > +{ > + struct sirfsoc_i2c *siic; > + struct i2c_adapter *new_adapter; > + struct resource *mem_res; > + struct clk *clk; > + int ctrl_speed; > + > + int err; > + u32 regval; > + > + clk =3D clk_get(&pdev->dev, NULL); > + if (IS_ERR(clk)) { > + err =3D PTR_ERR(clk); > + dev_err(&pdev->dev, "Clock get failed\n"); > + goto out; > + } > + > + clk_enable(clk); The return value of clk_enable() should really be checked. > + ctrl_speed =3D clk_get_rate(clk); > + > + new_adapter =3D kzalloc(sizeof(*new_adapter), GFP_KERNEL); If you do: new_adapter =3D devm_kzalloc(&pdev->dev, sizeof(*new_adapter),=20 GFP_KERNEL); then devres will handle cleanup on failure for you and also device=20 removal so you can get rid of a lot of the error handling and just=20 return early. The same applies for the ioremap, irq requesting and=20 other allocations. > + if (!new_adapter) { > + dev_err(&pdev->dev, > + "Can't allocate new i2c adapter!\n"); > + err =3D -ENOMEM; > + goto clk_out; > + } > + > + siic =3D kzalloc(sizeof(*siic), GFP_KERNEL); > + if (!siic) { > + dev_err(&pdev->dev, "Can't allocate driver data\n"); > + err =3D -ENOMEM; > + goto free_adapter; > + } > + new_adapter->class =3D I2C_CLASS_HWMON | I2C_CLASS_DDC | I2C_CLASS_= SPD; > + siic->adapter =3D new_adapter; > + > + mem_res =3D platform_get_resource(pdev, IORESOURCE_MEM, 0); > + if (mem_res =3D=3D NULL) { > + dev_err(&pdev->dev, "Unable to get MEM resource\n"); > + err =3D -EINVAL; > + goto free_data; > + } > + > + siic->base =3D > + ioremap(mem_res->start, (mem_res->end - mem_res->start + 1)); > + if (siic->base =3D=3D NULL) { > + dev_err(&pdev->dev, "IO remap failed!\n"); > + err =3D -ENOMEM; > + goto free_data; > + } > + > + siic->irq =3D platform_get_irq(pdev, 0); > + if (!siic->irq) { > + err =3D -EINVAL; > + goto free_base; > + } > + err =3D request_irq(siic->irq, i2c_sirfsoc_irq, 0, > + dev_name(&pdev->dev), siic); > + if (err) > + goto free_base; > + > + new_adapter->algo =3D &i2c_sirfsoc_algo; > + new_adapter->algo_data =3D siic; > + > + new_adapter->dev.parent =3D &pdev->dev; > + new_adapter->nr =3D pdev->id; > + > + strlcpy(new_adapter->name, "sirfsoc-i2c", sizeof(new_adapter->name)= ); > + > + platform_set_drvdata(pdev, new_adapter); > + init_completion(&siic->done); > + > + /* Controller Initalisation */ > + > + writel(SIRFSOC_I2C_RESET, siic->base + SIRFSOC_I2C_CTRL); > + while (readl(siic->base + SIRFSOC_I2C_CTRL) & SIRFSOC_I2C_RESET) > + cpu_relax(); > + writel(SIRFSOC_I2C_CORE_EN | SIRFSOC_I2C_MASTER_MODE, > + siic->base + SIRFSOC_I2C_CTRL); > + > + siic->clk =3D clk; > + siic->speed =3D SIRFSOC_I2C_DEFAULT_SPEED; > + if (siic->speed < 100000) > + regval =3D > + (2 * ctrl_speed) / (2 * siic->speed * 11); > + else > + regval =3D ctrl_speed / (siic->speed * 5); > + > + writel(regval, siic->base + SIRFSOC_I2C_CLK_CTRL); > + if (regval > 0xFF) > + writel(0xFF, siic->base + SIRFSOC_I2C_SDA_DELAY); > + else > + writel(regval, siic->base + SIRFSOC_I2C_SDA_DELAY); > + > + err =3D i2c_add_numbered_adapter(new_adapter); > + if (err < 0) { > + dev_err(&pdev->dev, "Can't add new i2c adapter\n"); > + goto free_irq; > + } > + > + dev_info(&pdev->dev, "I2C adapter ready to operate\n"); > + > + return 0; > + > +free_irq: > + free_irq(siic->irq, siic); > +free_base: > + iounmap(siic->base); > +free_data: > + kfree(siic); > +free_adapter: > + kfree(new_adapter); > +clk_out: > + clk_disable(clk); > + clk_put(clk); > +out: > + return err; > +} > + > +static int __devexit i2c_sirfsoc_remove(struct platform_device *pdev= ) > +{ > + struct i2c_adapter *adapter =3D platform_get_drvdata(pdev); > + struct sirfsoc_i2c *siic =3D adapter->algo_data; > + > + writel(SIRFSOC_I2C_RESET, siic->base + SIRFSOC_I2C_CTRL); > + i2c_del_adapter(adapter); > + free_irq(siic->irq, siic); > + iounmap(siic->base); > + kfree(siic); > + kfree(adapter); > + clk_disable(siic->clk); > + clk_put(siic->clk); > + return 0; > +} > + > +#ifdef CONFIG_PM > +static int i2c_sirfsoc_suspend(struct platform_device *pdev, pm_mess= age_t msg) > +{ > + struct i2c_adapter *adapter =3D platform_get_drvdata(pdev); > + struct sirfsoc_i2c *siic =3D adapter->algo_data; > + > + siic->sda_delay =3D readl(siic->base + SIRFSOC_I2C_SDA_DELAY); > + siic->clk_div =3D readl(siic->base + SIRFSOC_I2C_CLK_CTRL); > + return 0; > +} > + > +static int i2c_sirfsoc_resume(struct platform_device *pdev) > +{ > + struct i2c_adapter *adapter =3D platform_get_drvdata(pdev); > + struct sirfsoc_i2c *siic =3D adapter->algo_data; > + > + writel(SIRFSOC_I2C_RESET, siic->base + SIRFSOC_I2C_CTRL); > + writel(SIRFSOC_I2C_CORE_EN | SIRFSOC_I2C_MASTER_MODE, > + siic->base + SIRFSOC_I2C_CTRL); > + writel(siic->clk_div, siic->base + SIRFSOC_I2C_CLK_CTRL); > + writel(siic->sda_delay, siic->base + SIRFSOC_I2C_SDA_DELAY); > + return 0; > +} > +#else > +#define i2c_sirfsoc_suspend NULL > +#define i2c_sirfsoc_resume NULL > +#endif > + > +static const struct of_device_id sirfsoc_i2c_of_match[] __devinitcon= st =3D { > + { .compatible =3D "sirf,prima2-i2c", }, > + {}, > +}; > +MODULE_DEVICE_TABLE(of, sirfsoc_i2c_of_match); > + > +static struct platform_driver i2c_sirfsoc_driver =3D { > + .driver =3D { > + .name =3D "sirfsoc_i2c", > + .owner =3D THIS_MODULE, > + .of_match_table =3D sirfsoc_i2c_of_match, > + }, > + .probe =3D i2c_sirfsoc_probe, > + .remove =3D __devexit_p(i2c_sirfsoc_remove), > + .suspend =3D i2c_sirfsoc_suspend, > + .resume =3D i2c_sirfsoc_resume, > +}; > + > +static int __init i2c_sirfsoc_init(void) > +{ > + return platform_driver_register(&i2c_sirfsoc_driver); > +} > +arch_initcall(i2c_sirfsoc_init); > + > +static void __exit i2c_sirfsoc_exit(void) > +{ > + platform_driver_unregister(&i2c_sirfsoc_driver); > +} > +module_exit(i2c_sirfsoc_exit); > + > +MODULE_DESCRIPTION("SiRF SoC I2C master controller driver"); > +MODULE_AUTHOR("Zhiwu Song , " > + "Xiangzhen Ye "); > +MODULE_LICENSE("GPL v2"); > diff --git a/drivers/i2c/busses/i2c-sirf.h b/drivers/i2c/busses/i2c-s= irf.h > new file mode 100644 > index 0000000..28f4ada > --- /dev/null > +++ b/drivers/i2c/busses/i2c-sirf.h > @@ -0,0 +1,61 @@ > +/* > + * I2C bus drivers for CSR SiRFprimaII > + * > + * Copyright (c) 2011 Cambridge Silicon Radio Limited, a CSR plc gro= up company. > + * > + * Licensed under GPLv2 or later. > + */ > + > +#ifndef _SIRFSOC_I2C_BUS_H_ > +#define _SIRFSOC_I2C_BUS_H_ > + > +#include > + > +#define SIRFSOC_I2C_CLK_CTRL 0x00 It may be worth moving the contents of this file into the .c file as=20 they don't get used anywhere else.