Dr. H. Nikolaus Schaller wrote:
> Hi Graham,
> after deeper looking into the code I am more and more wondering how you
> could make it work (or I misunderstood something completely).
Well it does in fact work for me with the pcf8563. FYI, this is with a 2.6.29
kernel I hacked to play with android (which is unusably slow on the jz4730).
See below my explanation of why the current i2c_jz_xfer() is broken....
static int i2c_jz_xfer(struct i2c_adapter *adap, struct i2c_msg *pmsg, int num)
{
int ret, i;
dev_dbg(&adap->dev, "jz47xx_xfer: processing %d messages:\n", num);
for (i = 0; i < num; i++) {
dev_dbg(&adap->dev, " #%d: %sing %d byte%s %s 0x%02x\n", i,
pmsg->flags & I2C_M_RD ? "read" : "writ",
pmsg->len, pmsg->len > 1 ? "s" : "",
pmsg->flags & I2C_M_RD ? "from" : "to", pmsg->addr);
if (pmsg->len && pmsg->buf) { /* sanity check */
if (pmsg->flags & I2C_M_RD){
ret = xfer_read(pmsg->addr, pmsg->buf,
pmsg->len);
}else{
ret = xfer_write(pmsg->addr, pmsg->buf,
pmsg->len);
}
if (ret)
return ret;
/* Wait until transfer is finished */
}
dev_dbg(&adap->dev, "transfer complete\n");
pmsg++; /* next message */
}
return i;
}
The xfer_read and xfer_write functions return the number of bytes read/written.
So if they successfully do a transfer, the loop here will be broken and the
function will return. So subsequent transfers in the i2c message will
not occur. Hence the change below is required for drivers that have more than
one transfer in any given i2c message.
- if (ret)
- return ret;
+ if (ret != pmsg->len)
+ return -1;
>
> The reason is that the drivers/i2c/busses/i2c-jz47xx.c driver is
> handling some strange 'unsigned long sub_addr' and sending that right
> after sending the device address. This value is initialized to 0 and
> sometimes incremented. But I could not find a setter for it.
I have also seen this sub_addr and it is totally confusing. I can't see any
way for it to be incremented while used in conjunction with the pcf8563 driver.
>
> Now, the PCF8563 driver calls
>
> unsigned char buf[13] = { PCF8563_REG_ST1 };
>
> struct i2c_msg msgs[] = {
> { client->addr, 0, 1, buf }, /* setup read ptr */
> { client->addr, I2C_M_RD, 13, buf }, /* read status + date */
> };
>
> /* read registers */
> if ((i2c_transfer(client->adapter, msgs, 2)) != 2) {
> dev_err(&client->dev, "%s: read error\n", __FUNCTION__);
> return -EIO;
> }
>
> which I interpret as the driver is setting (writing) the 1 byte
> PCF8563_REG_ST1 into the "read ptr" (aka sub_addr) and then trying to
> read back 13 bytes.
>
> IMHO not ok is that each xfer sends the device address *plus* this
> strange sub_addr byte (which is initialized to 0) and then followed by
> the number of bytes specified in the message.
>
> And, I understand the struct i2c_msg
>
>
> http://ww2.cs.fsu.edu/~rosentha/linux/2.6.26.5/docs/DocBook/kernel-api/re1210.html
>
>
> as a message *without* any subaddress (unless encoded into the message
> body). Or am I wrong?
>
> Here I found a description how subadressing should work on the i2c side
> (in case or writes or reads):
>
> http://www.8051projects.net/i2c-twi-tutorial/write-read-i2c-bus.php
>
> I have got the impression that the i2c-jz47xx.c is just halfway
> extracted/adapted from the i2c.c driver (which explicitly takes a
> subaddress and is controlled correctly from the char-style pcf driver)
> and hacked to make some EEPROM work. But the driver should probably also
> recognize flags like I2C_M_NOSTART or I2C_M_NO_RD_ACK to correctly
> interwork with the i2c-tools.
>
> So it needs some deeper rework, but I does not appear to be very
> difficult to do. So I will roll up my sleeves :)
Well, I've already attempted to rewrite the i2c driver in a much cleaner way,
for use with 2.6.33, unfortunately I couldn't get it to work. I've attached
the file for your perusal in case you find it useful.
The interrupt mechanism is undocumented in the jz i2c data sheet, so I sort
of guessed at how it would work. The interrupts could be removed from the
read path and waits used like in the write path. That is how I originally
had the driver, but it didn't make any difference to the functionality.
>
> BR,
> Nikolaus
>
>
-Graham
/*
* JZ47xx I2C driver.
*
* Copyright (C) 2010 Ubiq Technologies Pty Ltd <graham.go...@gmail.com>
*
* 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
* published by the Free Software Foundation.
*/
#include <linux/init.h>
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/platform_device.h>
#include <linux/clk.h>
#include <linux/i2c.h>
#include <linux/interrupt.h>
#include <linux/completion.h>
#include <xburst.h>
#define REG_DATA 0x00
#define REG_CTRL 0x04
#define REG_STATUS 0x08
#define REG_CLOCK 0x0C
#define CTRL_ENABLE BIT(0)
#define CTRL_TX_ACK BIT(1)
#define CTRL_STOP BIT(2)
#define CTRL_START BIT(3)
#define CTRL_IEN BIT(4)
#define STATUS_RX_ACK BIT(0)
#define STATUS_DATA_VALID BIT(1)
#define STATUS_TX_DONE BIT(2)
#define STATUS_BUSY BIT(3)
#define STATUS_STX BIT(4)
struct jz_i2c_data {
void __iomem *base;
struct i2c_adapter adapter;
struct completion data_valid;
struct completion tx_done;
u8 data;
};
#define reg_set_bit(reg, bit) \
writeb(readb(jid->base + reg) | (bit), jid->base + reg)
#define reg_clear_bit(reg, bit) \
writeb(readb(jid->base + reg) & ~(bit), jid->base + reg)
static int
jz_i2c_read(struct jz_i2c_data *jid, u8 addr, u8 *buf, int len)
{
u8 status;
/* send start */
reg_set_bit(REG_CTRL, CTRL_START);
/* send ack */
reg_clear_bit(REG_CTRL, CTRL_TX_ACK);
/* write slave address */
writeb((addr << 1) | 0x1, jid->base + REG_DATA);
/* tell controller to shift out data */
reg_set_bit(REG_STATUS, STATUS_DATA_VALID);
init_completion(&jid->tx_done);
reg_set_bit(REG_CTRL, CTRL_IEN);
if (!wait_for_completion_timeout(&jid->tx_done,
msecs_to_jiffies(10))) {
printk(KERN_ERR "%s: address transmission timeout\n",
__FUNCTION__);
return -ETIMEDOUT;
}
status = readb(jid->base + REG_STATUS);
if (status & STATUS_RX_ACK) {
printk(KERN_ERR "%s: no response from slave at 0x%02x",
__FUNCTION__, addr);
return -ENODEV;
}
while (len) {
if (len == 1) {
/* last byte, don't ack */
reg_set_bit(REG_CTRL, CTRL_TX_ACK);
}
init_completion(&jid->data_valid);
reg_set_bit(REG_CTRL, CTRL_IEN);
if (!wait_for_completion_timeout(&jid->data_valid,
msecs_to_jiffies(10))) {
printk(KERN_ERR "%s: read timeout.\n", __FUNCTION__);
return -ETIMEDOUT;
}
if (len == 1) {
/* last byte, send stop */
reg_set_bit(REG_CTRL, CTRL_STOP);
}
*(buf++) = jid->data;
reg_clear_bit(REG_STATUS, STATUS_DATA_VALID);
len--;
}
return 0;
}
static int
jz_i2c_write(struct jz_i2c_data *jid, u8 addr, u8 *buf, int len)
{
u8 status;
int send_addr = 1;
int timeout;
/* send start */
reg_set_bit(REG_CTRL, CTRL_START);
while (len) {
if (send_addr) {
/* write slave address */
writeb((addr << 1), jid->base + REG_DATA);
send_addr = 0;
} else {
writeb(*(buf++), jid->base + REG_DATA);
len--;
}
/* tell controller it can shift out some data */
reg_set_bit(REG_STATUS, STATUS_DATA_VALID);
timeout = 10000;
do {
status = readb(jid->base + REG_STATUS);
if (unlikely(status & STATUS_RX_ACK)) {
printk(KERN_ERR "%s: slave NACK while sending
data\n",
__FUNCTION__);
return -ENODEV;
}
if (unlikely(--timeout == 0)) {
printk(KERN_ERR "%s: timeout sending data\n",
__FUNCTION__);
return -ETIMEDOUT;
}
} while ((status & STATUS_DATA_VALID));
}
/* send stop */
reg_set_bit(REG_CTRL, CTRL_STOP);
timeout = 10000;
do {
status = readb(jid->base + REG_STATUS);
if (unlikely(status & STATUS_RX_ACK)) {
printk(KERN_ERR "%s: slave NACK after sending STOP\n",
__FUNCTION__);
return -ENODEV;
}
if (unlikely(--timeout == 0)) {
printk(KERN_ERR "%s: TX_DONE timeout\n",
__FUNCTION__);
return -ETIMEDOUT;
}
} while ((status & STATUS_TX_DONE) == 0);
return 0;
}
static int
jz_i2c_xfer(struct i2c_adapter *i2c_adap, struct i2c_msg *msgs, int num)
{
struct jz_i2c_data *jid = i2c_adap->algo_data;
struct i2c_msg *m;
int i, ret = 0;
for (i=0; i<num; i++) {
m = &msgs[i];
if (m->flags & I2C_M_RD)
ret = jz_i2c_read(jid, m->addr, m->buf, m->len);
else
ret = jz_i2c_write(jid, m->addr, m->buf, m->len);
if (ret)
break;
}
return ret ? ret : num;
}
static u32
jz_i2c_func(struct i2c_adapter *adap)
{
return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
}
static struct i2c_algorithm jz_i2c_algo = {
.master_xfer = jz_i2c_xfer,
.functionality = jz_i2c_func,
};
static irqreturn_t jz_i2c_irq_handler(int irq, void *dev_id)
{
struct jz_i2c_data *jid = dev_id;
u8 status;
/*
printk(KERN_INFO "%s: status=0x%02x, ctrl=0x%02x\n",
__FUNCTION__,
readb(jid->base + REG_STATUS),
readb(jid->base + REG_CTRL));
*/
reg_clear_bit(REG_CTRL, CTRL_IEN);
status = readb(jid->base + REG_STATUS);
if (status & STATUS_TX_DONE) {
complete(&jid->tx_done);
}
if (status & STATUS_DATA_VALID) {
jid->data = readb(jid->base + REG_DATA);
reg_clear_bit(REG_STATUS, STATUS_DATA_VALID);
complete(&jid->data_valid);
}
return IRQ_HANDLED;
}
static void
jz_i2c_hw_init(struct jz_i2c_data *jid)
{
u32 extal = clk_get_rate(clk_get(NULL, "ext"));
u16 i2c_rate;
/* set clock to 10000Hz */
i2c_rate = (extal / (16*10000)) - 1;
writew(i2c_rate, jid->base + REG_CLOCK);
reg_set_bit(REG_CTRL, CTRL_ENABLE);
}
static int __devinit
jz_i2c_probe(struct platform_device *pdev)
{
struct jz_i2c_data *jid;
struct resource *r;
int irq, ret;
r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (r == NULL) {
ret = -ENODEV;
goto err0;
}
jid = kzalloc(sizeof(struct jz_i2c_data), GFP_KERNEL);
if (jid == NULL) {
ret = -ENOMEM;
goto err0;
}
jid->base = ioremap_nocache(r->start, resource_size(r));
if (jid->base == NULL) {
ret = -EBUSY;
goto err1;
}
jid->adapter.nr = 0;
jid->adapter.algo = &jz_i2c_algo;
jid->adapter.algo_data = jid;
jid->adapter.dev.parent = &pdev->dev;
strlcpy(jid->adapter.name, "", sizeof(jid->adapter.name));
jz_i2c_hw_init(jid);
ret = i2c_add_numbered_adapter(&jid->adapter);
if (ret)
goto err2;
platform_set_drvdata(pdev, jid);
r = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
if (r == NULL) {
ret = -ENODEV;
goto err3;
}
init_completion(&jid->tx_done);
init_completion(&jid->data_valid);
irq = r->start;
if (request_irq(irq, jz_i2c_irq_handler, 0, pdev->name, jid)) {
printk(KERN_ERR "%s: failed to request irq %d\n",
__FUNCTION__, irq);
goto err3;
}
return 0;
err3:
i2c_del_adapter(&jid->adapter);
err2:
reg_clear_bit(REG_CTRL, CTRL_ENABLE);
iounmap(jid->base);
err1:
kfree(jid);
err0:
return ret;
}
static int __devexit
jz_i2c_remove(struct platform_device *pdev)
{
struct jz_i2c_data *jid = platform_get_drvdata(pdev);
i2c_del_adapter(&jid->adapter);
reg_clear_bit(REG_CTRL, CTRL_ENABLE);
iounmap(jid->base);
kfree(jid);
return 0;
}
static struct platform_driver jz_i2c_driver = {
.probe = jz_i2c_probe,
.remove = jz_i2c_probe,
.driver = {
.name = "jz-i2c",
.owner = THIS_MODULE,
},
};
static int __init
jz_i2c_init(void)
{
return platform_driver_register(&jz_i2c_driver);
}
static void __exit
jz_i2c_exit(void)
{
platform_driver_unregister(&jz_i2c_driver);
}
module_init(jz_i2c_init);
module_exit(jz_i2c_exit);
MODULE_AUTHOR("Graham Gower <graham.go...@gmail.com>");
MODULE_DESCRIPTION("I2C bus driver for jz47xx");
MODULE_LICENSE("GPL");
MODULE_ALIAS("platform:jz-i2c");
_______________________________________________
Mipsbook-devel mailing list
Mipsbook-devel@linuxtogo.org
http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/mipsbook-devel
