* [PATCH 2.6.39-rc7] RTC: rtc-ds1307 EEPROM support for ds1388 chip
@ 2011-05-23 5:18 Austin Boyle
2011-06-14 4:14 ` Austin Boyle
0 siblings, 1 reply; 9+ messages in thread
From: Austin Boyle @ 2011-05-23 5:18 UTC (permalink / raw)
To: Alessandro Zummo; +Cc: rtc-linux, linux-kernel
This is my patch which extends the NVRAM access functions in the DS1307
driver to support the 512 bytes of EEPROM on the DS1388 chip. Could you
please review the patch? If there are any problems with the way I have
implemented this or suggested improvements I will gladly modify. Thanks!
[PATCH 2.6.39-rc7] RTC: rtc-ds1307 EEPROM support for ds1388 chip
This patch adds support for different sized NVRAM, such as
the 512 bytes of EEPROM on the DS1388 chip.
Signed-off-by: Austin Boyle <austin.boyle@aviatnet.com>
---
diff -upNr a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
--- a/drivers/rtc/rtc-ds1307.c 2011-05-19 13:52:35.114845829 +1200
+++ b/drivers/rtc/rtc-ds1307.c 2011-05-23 16:46:19.168163336 +1200
@@ -95,15 +95,19 @@ enum ds_type {
# define RX8025_BIT_VDET 0x40
# define RX8025_BIT_XST 0x20
+#define DS1307_MAX_NUM_ADDRS 3
struct ds1307 {
u8 offset; /* register's offset */
+ u16 nvram_size;
+ u16 nvram_offset;
+ u8 num_addrs;
u8 regs[11];
enum ds_type type;
unsigned long flags;
#define HAS_NVRAM 0 /* bit 0 == sysfs file active */
#define HAS_ALARM 1 /* bit 1 == irq claimed */
- struct i2c_client *client;
+ struct i2c_client *client[DS1307_MAX_NUM_ADDRS];
struct rtc_device *rtc;
struct work_struct work;
s32 (*read_block_data)(const struct i2c_client *client, u8 command,
@@ -113,23 +117,26 @@ struct ds1307 {
};
struct chip_desc {
- unsigned nvram56:1;
+ unsigned nvram:1;
unsigned alarm:1;
};
static const struct chip_desc chips[] = {
[ds_1307] = {
- .nvram56 = 1,
+ .nvram = 1,
},
[ds_1337] = {
.alarm = 1,
},
[ds_1338] = {
- .nvram56 = 1,
+ .nvram = 1,
},
[ds_1339] = {
.alarm = 1,
},
+[ds_1388] = {
+ .nvram = 1,
+},
[ds_1340] = {
},
[ds_3231] = {
@@ -248,7 +255,7 @@ static void ds1307_work(struct work_stru
int stat, control;
ds1307 = container_of(work, struct ds1307, work);
- client = ds1307->client;
+ client = ds1307->client[0];
lock = &ds1307->rtc->ops_lock;
mutex_lock(lock);
@@ -294,7 +301,7 @@ static int ds1307_get_time(struct device
int tmp;
/* read the RTC date and time registers all at once */
- tmp = ds1307->read_block_data(ds1307->client,
+ tmp = ds1307->read_block_data(ds1307->client[0],
ds1307->offset, 7, ds1307->regs);
if (tmp != 7) {
dev_err(dev, "%s error %d\n", "read", tmp);
@@ -372,7 +379,7 @@ static int ds1307_set_time(struct device
"write", buf[0], buf[1], buf[2], buf[3],
buf[4], buf[5], buf[6]);
- result = ds1307->write_block_data(ds1307->client,
+ result = ds1307->write_block_data(ds1307->client[0],
ds1307->offset, 7, buf);
if (result < 0) {
dev_err(dev, "%s error %d\n", "write", result);
@@ -530,8 +537,6 @@ static const struct rtc_class_ops ds13xx
/*----------------------------------------------------------------------*/
-#define NVRAM_SIZE 56
-
static ssize_t
ds1307_nvram_read(struct file *filp, struct kobject *kobj,
struct bin_attribute *attr,
@@ -540,18 +545,39 @@ ds1307_nvram_read(struct file *filp, str
struct i2c_client *client;
struct ds1307 *ds1307;
int result;
+ unsigned int addr = 0;
client = kobj_to_i2c_client(kobj);
ds1307 = i2c_get_clientdata(client);
- if (unlikely(off >= NVRAM_SIZE))
+ if (unlikely(off >= ds1307->nvram_size))
return 0;
- if ((off + count) > NVRAM_SIZE)
- count = NVRAM_SIZE - off;
+ if ((off + count) > ds1307->nvram_size)
+ count = ds1307->nvram_size - off;
if (unlikely(!count))
return count;
- result = ds1307->read_block_data(client, 8 + off, count, buf);
+ if (ds1307->nvram_offset < 0xff) { /* ds1307, ds1338 */
+ result = ds1307->read_block_data(client, ds1307->nvram_offset + off,
count, buf);
+ } else { /* ds1388 */
+ if ((off <= 0x00ff) && (off + count > 0x00ff)) { /* read spans two
blocks */
+ client = ds1307->client[1]; /* first block */
+ addr = (unsigned int)off;
+ result = ds1307->read_block_data(client, addr, (0x100-addr), buf);
+ client = ds1307->client[2]; /* second block */
+ result += ds1307->read_block_data(client, 0x00, (count+addr-0x100),
(buf+0x100-addr));
+
+ } else { /* read spans one blocks */
+ if (off <= 0x00ff) {
+ client = ds1307->client[1];
+ addr = (unsigned int)off;
+ } else if (off <= 0x0200) {
+ client = ds1307->client[2];
+ addr = (unsigned int)off - 0x100;
+ }
+ result = ds1307->read_block_data(client, addr, count, buf);
+ }
+ }
if (result < 0)
dev_err(&client->dev, "%s error %d\n", "nvram read", result);
return result;
@@ -565,18 +591,40 @@ ds1307_nvram_write(struct file *filp, st
struct i2c_client *client;
struct ds1307 *ds1307;
int result;
+ unsigned int addr = 0;
client = kobj_to_i2c_client(kobj);
ds1307 = i2c_get_clientdata(client);
- if (unlikely(off >= NVRAM_SIZE))
+ if (unlikely(off >= ds1307->nvram_size))
return -EFBIG;
- if ((off + count) > NVRAM_SIZE)
- count = NVRAM_SIZE - off;
+ if ((off + count) > ds1307->nvram_size)
+ count = ds1307->nvram_size - off;
if (unlikely(!count))
return count;
- result = ds1307->write_block_data(client, 8 + off, count, buf);
+ if (ds1307->nvram_offset < 0xff) { /* ds1307, ds1338 */
+ result = ds1307->write_block_data(client, ds1307->nvram_offset + off,
count, buf);
+ } else { /* ds1388 */
+ if ((off <= 0x00ff) && ((off + count) > 0x00ff)) { /* read spans two
blocks */
+ client = ds1307->client[1]; /* first block */
+ addr = (unsigned int)off;
+ result = ds1307->write_block_data(client, addr, (0x100-addr), buf);
+ client = ds1307->client[2]; /* second block */
+ result = ds1307->write_block_data(client, 0x00, (addr+count-0x100),
(buf+0x100-addr));
+
+ } else { /* Device address/offset translation */
+ dev_dbg(&client->dev, "Write spans one block\n");
+ if (off <= 0x00ff) {
+ client = ds1307->client[1];
+ addr = (unsigned int)off;
+ } else if (off <= 0x0200) {
+ client = ds1307->client[2];
+ addr = (unsigned int)off - 0x100;
+ }
+ result = ds1307->write_block_data(client, addr, count, buf);
+ }
+ }
if (result < 0) {
dev_err(&client->dev, "%s error %d\n", "nvram write", result);
return result;
@@ -592,7 +640,6 @@ static struct bin_attribute nvram = {
.read = ds1307_nvram_read,
.write = ds1307_nvram_write,
- .size = NVRAM_SIZE,
};
/*----------------------------------------------------------------------*/
@@ -605,6 +652,7 @@ static int __devinit ds1307_probe(struct
struct ds1307 *ds1307;
int err = -ENODEV;
int tmp;
+ int i;
const struct chip_desc *chip = &chips[id->driver_data];
struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent);
int want_irq = false;
@@ -624,9 +672,10 @@ static int __devinit ds1307_probe(struct
i2c_set_clientdata(client, ds1307);
- ds1307->client = client;
+ ds1307->client[0] = client;
ds1307->type = id->driver_data;
ds1307->offset = 0;
+ ds1307->num_addrs = 1;
buf = ds1307->regs;
if (i2c_check_functionality(adapter, I2C_FUNC_SMBUS_I2C_BLOCK)) {
@@ -642,12 +691,12 @@ static int __devinit ds1307_probe(struct
case ds_1339:
case ds_3231:
/* has IRQ? */
- if (ds1307->client->irq > 0 && chip->alarm) {
+ if (client->irq > 0 && chip->alarm) {
INIT_WORK(&ds1307->work, ds1307_work);
want_irq = true;
}
/* get registers that the "rtc" read below won't read... */
- tmp = ds1307->read_block_data(ds1307->client,
+ tmp = ds1307->read_block_data(client,
DS1337_REG_CONTROL, 2, buf);
if (tmp != 2) {
pr_debug("read error %d\n", tmp);
@@ -681,7 +730,7 @@ static int __devinit ds1307_probe(struct
break;
case rx_8025:
- tmp = i2c_smbus_read_i2c_block_data(ds1307->client,
+ tmp = i2c_smbus_read_i2c_block_data(client,
RX8025_REG_CTRL1 << 4 | 0x08, 2, buf);
if (tmp != 2) {
pr_debug("read error %d\n", tmp);
@@ -747,6 +796,14 @@ static int __devinit ds1307_probe(struct
break;
case ds_1388:
ds1307->offset = 1; /* Seconds starts at 1 */
+ ds1307->num_addrs = 3; /* uses 3 I2C addrs */
+ ds1307->nvram_size = 512; /* 512 bytes EEPROM */
+ ds1307->nvram_offset = 0xff;
+ break;
+ case ds_1307:
+ case ds_1338:
+ ds1307->nvram_size = 56; /* 56 bytes NVRAM */
+ ds1307->nvram_offset = 8;
break;
default:
break;
@@ -754,7 +811,7 @@ static int __devinit ds1307_probe(struct
read_rtc:
/* read RTC registers */
- tmp = ds1307->read_block_data(ds1307->client, ds1307->offset, 8, buf);
+ tmp = ds1307->read_block_data(client, ds1307->offset, 8, buf);
if (tmp != 8) {
pr_debug("read error %d\n", tmp);
err = -EIO;
@@ -866,7 +923,8 @@ read_rtc:
dev_dbg(&client->dev, "got IRQ %d\n", client->irq);
}
- if (chip->nvram56) {
+ if (chip->nvram) {
+ nvram.size = ds1307->nvram_size;
err = sysfs_create_bin_file(&client->dev.kobj, &nvram);
if (err == 0) {
set_bit(HAS_NVRAM, &ds1307->flags);
@@ -874,8 +932,26 @@ read_rtc:
}
}
+ /* use dummy devices for multiple address chips */
+ for (i = 1; i < ds1307->num_addrs; i++) {
+ ds1307->client[i] = i2c_new_dummy(client->adapter, client->addr + i);
+ if (!ds1307->client[i]) {
+ dev_err(&client->dev, "address 0x%02x unavailable\n",
+ client->addr + i);
+ err = -EADDRINUSE;
+ goto err_clients;
+ }
+ i2c_set_clientdata(ds1307->client[i], ds1307);
+ }
+
return 0;
+err_clients:
+ for (i = 1; i < ds1307->num_addrs; i++) {
+ if (ds1307->client[i]) {
+ i2c_unregister_device(ds1307->client[i]);
+ }
+ }
exit_irq:
rtc_device_unregister(ds1307->rtc);
exit_free:
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2.6.39-rc7] RTC: rtc-ds1307 EEPROM support for ds1388 chip
2011-05-23 5:18 [PATCH 2.6.39-rc7] RTC: rtc-ds1307 EEPROM support for ds1388 chip Austin Boyle
@ 2011-06-14 4:14 ` Austin Boyle
2011-07-17 23:32 ` Austin Boyle
2011-07-22 21:54 ` Andrew Morton
0 siblings, 2 replies; 9+ messages in thread
From: Austin Boyle @ 2011-06-14 4:14 UTC (permalink / raw)
To: Alessandro Zummo; +Cc: rtc-linux, linux-kernel
On Mon, 2011-05-23 at 17:18 +1200, Austin Boyle wrote:
> This is my patch which extends the NVRAM access functions in the DS1307
> driver to support the 512 bytes of EEPROM on the DS1388 chip. Could you
> please review the patch? If there are any problems with the way I have
> implemented this or suggested improvements I will gladly modify. Thanks!
This is an updated version of the patch which prints the correct NVRAM
size at init. Could someone please give me some feedback?
[PATCH 3.0-rc3] RTC: rtc-ds1307: EEPROM support for ds1388 chip
This patch adds support for different sized NVRAM, such as
the 512 bytes of EEPROM on the DS1388 chip.
Cc: Alessandro Zummo <a.zummo@towertech.it>
Signed-off-by: Austin Boyle <austin.boyle@aviatnet.com>
---
diff -upNr a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
--- a/drivers/rtc/rtc-ds1307.c 2011-06-14 15:56:45.221497487 +1200
+++ b/drivers/rtc/rtc-ds1307.c 2011-06-14 15:57:28.803510835 +1200
@@ -95,15 +95,19 @@ enum ds_type {
# define RX8025_BIT_VDET 0x40
# define RX8025_BIT_XST 0x20
+#define DS1307_MAX_NUM_ADDRS 3
struct ds1307 {
u8 offset; /* register's offset */
+ u16 nvram_size;
+ u16 nvram_offset;
+ u8 num_addrs;
u8 regs[11];
enum ds_type type;
unsigned long flags;
#define HAS_NVRAM 0 /* bit 0 == sysfs file active */
#define HAS_ALARM 1 /* bit 1 == irq claimed */
- struct i2c_client *client;
+ struct i2c_client *client[DS1307_MAX_NUM_ADDRS];
struct rtc_device *rtc;
struct work_struct work;
s32 (*read_block_data)(const struct i2c_client *client, u8 command,
@@ -113,23 +117,26 @@ struct ds1307 {
};
struct chip_desc {
- unsigned nvram56:1;
+ unsigned nvram:1;
unsigned alarm:1;
};
static const struct chip_desc chips[] = {
[ds_1307] = {
- .nvram56 = 1,
+ .nvram = 1,
},
[ds_1337] = {
.alarm = 1,
},
[ds_1338] = {
- .nvram56 = 1,
+ .nvram = 1,
},
[ds_1339] = {
.alarm = 1,
},
+[ds_1388] = {
+ .nvram = 1,
+},
[ds_1340] = {
},
[ds_3231] = {
@@ -248,7 +255,7 @@ static void ds1307_work(struct work_stru
int stat, control;
ds1307 = container_of(work, struct ds1307, work);
- client = ds1307->client;
+ client = ds1307->client[0];
lock = &ds1307->rtc->ops_lock;
mutex_lock(lock);
@@ -294,7 +301,7 @@ static int ds1307_get_time(struct device
int tmp;
/* read the RTC date and time registers all at once */
- tmp = ds1307->read_block_data(ds1307->client,
+ tmp = ds1307->read_block_data(ds1307->client[0],
ds1307->offset, 7, ds1307->regs);
if (tmp != 7) {
dev_err(dev, "%s error %d\n", "read", tmp);
@@ -372,7 +379,7 @@ static int ds1307_set_time(struct device
"write", buf[0], buf[1], buf[2], buf[3],
buf[4], buf[5], buf[6]);
- result = ds1307->write_block_data(ds1307->client,
+ result = ds1307->write_block_data(ds1307->client[0],
ds1307->offset, 7, buf);
if (result < 0) {
dev_err(dev, "%s error %d\n", "write", result);
@@ -530,8 +537,6 @@ static const struct rtc_class_ops ds13xx
/*----------------------------------------------------------------------*/
-#define NVRAM_SIZE 56
-
static ssize_t
ds1307_nvram_read(struct file *filp, struct kobject *kobj,
struct bin_attribute *attr,
@@ -540,18 +545,39 @@ ds1307_nvram_read(struct file *filp, str
struct i2c_client *client;
struct ds1307 *ds1307;
int result;
+ unsigned int addr = 0;
client = kobj_to_i2c_client(kobj);
ds1307 = i2c_get_clientdata(client);
- if (unlikely(off >= NVRAM_SIZE))
+ if (unlikely(off >= ds1307->nvram_size))
return 0;
- if ((off + count) > NVRAM_SIZE)
- count = NVRAM_SIZE - off;
+ if ((off + count) > ds1307->nvram_size)
+ count = ds1307->nvram_size - off;
if (unlikely(!count))
return count;
- result = ds1307->read_block_data(client, 8 + off, count, buf);
+ if (ds1307->nvram_offset < 0xff) { /* ds1307, ds1338 */
+ result = ds1307->read_block_data(client, ds1307->nvram_offset + off,
count, buf);
+ } else { /* ds1388 */
+ if ((off <= 0x00ff) && (off + count > 0x00ff)) { /* read spans two
blocks */
+ client = ds1307->client[1]; /* first block */
+ addr = (unsigned int)off;
+ result = ds1307->read_block_data(client, addr, (0x100-addr), buf);
+ client = ds1307->client[2]; /* second block */
+ result += ds1307->read_block_data(client, 0x00, (count+addr-0x100),
(buf+0x100-addr));
+
+ } else { /* read spans one blocks */
+ if (off <= 0x00ff) {
+ client = ds1307->client[1];
+ addr = (unsigned int)off;
+ } else if (off <= 0x0200) {
+ client = ds1307->client[2];
+ addr = (unsigned int)off - 0x100;
+ }
+ result = ds1307->read_block_data(client, addr, count, buf);
+ }
+ }
if (result < 0)
dev_err(&client->dev, "%s error %d\n", "nvram read", result);
return result;
@@ -565,18 +591,40 @@ ds1307_nvram_write(struct file *filp, st
struct i2c_client *client;
struct ds1307 *ds1307;
int result;
+ unsigned int addr = 0;
client = kobj_to_i2c_client(kobj);
ds1307 = i2c_get_clientdata(client);
- if (unlikely(off >= NVRAM_SIZE))
+ if (unlikely(off >= ds1307->nvram_size))
return -EFBIG;
- if ((off + count) > NVRAM_SIZE)
- count = NVRAM_SIZE - off;
+ if ((off + count) > ds1307->nvram_size)
+ count = ds1307->nvram_size - off;
if (unlikely(!count))
return count;
- result = ds1307->write_block_data(client, 8 + off, count, buf);
+ if (ds1307->nvram_offset < 0xff) { /* ds1307, ds1338 */
+ result = ds1307->write_block_data(client, ds1307->nvram_offset + off,
count, buf);
+ } else { /* ds1388 */
+ if ((off <= 0x00ff) && ((off + count) > 0x00ff)) { /* read spans two
blocks */
+ client = ds1307->client[1]; /* first block */
+ addr = (unsigned int)off;
+ result = ds1307->write_block_data(client, addr, (0x100-addr), buf);
+ client = ds1307->client[2]; /* second block */
+ result = ds1307->write_block_data(client, 0x00, (addr+count-0x100),
(buf+0x100-addr));
+
+ } else { /* Device address/offset translation */
+ dev_dbg(&client->dev, "Write spans one block\n");
+ if (off <= 0x00ff) {
+ client = ds1307->client[1];
+ addr = (unsigned int)off;
+ } else if (off <= 0x0200) {
+ client = ds1307->client[2];
+ addr = (unsigned int)off - 0x100;
+ }
+ result = ds1307->write_block_data(client, addr, count, buf);
+ }
+ }
if (result < 0) {
dev_err(&client->dev, "%s error %d\n", "nvram write", result);
return result;
@@ -592,7 +640,6 @@ static struct bin_attribute nvram = {
.read = ds1307_nvram_read,
.write = ds1307_nvram_write,
- .size = NVRAM_SIZE,
};
/*----------------------------------------------------------------------*/
@@ -605,6 +652,7 @@ static int __devinit ds1307_probe(struct
struct ds1307 *ds1307;
int err = -ENODEV;
int tmp;
+ int i;
const struct chip_desc *chip = &chips[id->driver_data];
struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent);
int want_irq = false;
@@ -624,9 +672,10 @@ static int __devinit ds1307_probe(struct
i2c_set_clientdata(client, ds1307);
- ds1307->client = client;
+ ds1307->client[0] = client;
ds1307->type = id->driver_data;
ds1307->offset = 0;
+ ds1307->num_addrs = 1;
buf = ds1307->regs;
if (i2c_check_functionality(adapter, I2C_FUNC_SMBUS_I2C_BLOCK)) {
@@ -642,12 +691,12 @@ static int __devinit ds1307_probe(struct
case ds_1339:
case ds_3231:
/* has IRQ? */
- if (ds1307->client->irq > 0 && chip->alarm) {
+ if (client->irq > 0 && chip->alarm) {
INIT_WORK(&ds1307->work, ds1307_work);
want_irq = true;
}
/* get registers that the "rtc" read below won't read... */
- tmp = ds1307->read_block_data(ds1307->client,
+ tmp = ds1307->read_block_data(client,
DS1337_REG_CONTROL, 2, buf);
if (tmp != 2) {
pr_debug("read error %d\n", tmp);
@@ -681,7 +730,7 @@ static int __devinit ds1307_probe(struct
break;
case rx_8025:
- tmp = i2c_smbus_read_i2c_block_data(ds1307->client,
+ tmp = i2c_smbus_read_i2c_block_data(client,
RX8025_REG_CTRL1 << 4 | 0x08, 2, buf);
if (tmp != 2) {
pr_debug("read error %d\n", tmp);
@@ -747,6 +796,14 @@ static int __devinit ds1307_probe(struct
break;
case ds_1388:
ds1307->offset = 1; /* Seconds starts at 1 */
+ ds1307->num_addrs = 3; /* uses 3 I2C addrs */
+ ds1307->nvram_size = 512; /* 512 bytes EEPROM */
+ ds1307->nvram_offset = 0xff;
+ break;
+ case ds_1307:
+ case ds_1338:
+ ds1307->nvram_size = 56; /* 56 bytes NVRAM */
+ ds1307->nvram_offset = 8;
break;
default:
break;
@@ -754,7 +811,7 @@ static int __devinit ds1307_probe(struct
read_rtc:
/* read RTC registers */
- tmp = ds1307->read_block_data(ds1307->client, ds1307->offset, 8, buf);
+ tmp = ds1307->read_block_data(client, ds1307->offset, 8, buf);
if (tmp != 8) {
pr_debug("read error %d\n", tmp);
err = -EIO;
@@ -866,16 +923,35 @@ read_rtc:
dev_dbg(&client->dev, "got IRQ %d\n", client->irq);
}
- if (chip->nvram56) {
+ if (chip->nvram) {
+ nvram.size = ds1307->nvram_size;
err = sysfs_create_bin_file(&client->dev.kobj, &nvram);
if (err == 0) {
set_bit(HAS_NVRAM, &ds1307->flags);
- dev_info(&client->dev, "56 bytes nvram\n");
+ dev_info(&client->dev, "%d bytes nvram\n", nvram.size);
+ }
+ }
+
+ /* use dummy devices for multiple address chips */
+ for (i = 1; i < ds1307->num_addrs; i++) {
+ ds1307->client[i] = i2c_new_dummy(client->adapter, client->addr + i);
+ if (!ds1307->client[i]) {
+ dev_err(&client->dev, "address 0x%02x unavailable\n",
+ client->addr + i);
+ err = -EADDRINUSE;
+ goto err_clients;
}
+ i2c_set_clientdata(ds1307->client[i], ds1307);
}
return 0;
+err_clients:
+ for (i = 1; i < ds1307->num_addrs; i++) {
+ if (ds1307->client[i]) {
+ i2c_unregister_device(ds1307->client[i]);
+ }
+ }
exit_irq:
rtc_device_unregister(ds1307->rtc);
exit_free:
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2.6.39-rc7] RTC: rtc-ds1307 EEPROM support for ds1388 chip
2011-06-14 4:14 ` Austin Boyle
@ 2011-07-17 23:32 ` Austin Boyle
2011-07-22 10:19 ` Joakim Tjernlund
2011-07-22 21:54 ` Andrew Morton
1 sibling, 1 reply; 9+ messages in thread
From: Austin Boyle @ 2011-07-17 23:32 UTC (permalink / raw)
To: Joakim Tjernlund
Cc: Alessandro Zummo, rtc-linux, linux-kernel, Wolfram Sang,
David Brownell, Andrew Morton
Hi Joakim,
I see that you were responsible for adding support for the ds1388 RTC in
the ds1307 driver. Thank you for your work, I am using the driver for
this IC myself. I have been trying to submit a patch to add support for
the 512 bytes of EEPROM available on the ds1388 but have not received
any responses. Would you be willing to review my patch? I can send
through another version applied to the very latest kernel. Are the RTC
patches all submitted through Alessandro Zummo who is listed as the
maintainer or should they be sent directly to Linus?
Kind Regards,
Austin Boyle.
On Tue, 2011-06-14 at 16:15 +1200, Austin Boyle wrote:
> On Mon, 2011-05-23 at 17:18 +1200, Austin Boyle wrote:
> > This is my patch which extends the NVRAM access functions in the DS1307
> > driver to support the 512 bytes of EEPROM on the DS1388 chip. Could you
> > please review the patch? If there are any problems with the way I have
> > implemented this or suggested improvements I will gladly modify. Thanks!
>
> This is an updated version of the patch which prints the correct NVRAM
> size at init. Could someone please give me some feedback?
>
> [PATCH 3.0-rc3] RTC: rtc-ds1307: EEPROM support for ds1388 chip
>
> This patch adds support for different sized NVRAM, such as
> the 512 bytes of EEPROM on the DS1388 chip.
>
> Cc: Alessandro Zummo <a.zummo@towertech.it>
> Signed-off-by: Austin Boyle <austin.boyle@aviatnet.com>
> ---
> diff -upNr a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
> --- a/drivers/rtc/rtc-ds1307.c 2011-06-14 15:56:45.221497487 +1200
> +++ b/drivers/rtc/rtc-ds1307.c 2011-06-14 15:57:28.803510835 +1200
> @@ -95,15 +95,19 @@ enum ds_type {
> # define RX8025_BIT_VDET 0x40
> # define RX8025_BIT_XST 0x20
>
> +#define DS1307_MAX_NUM_ADDRS 3
>
> struct ds1307 {
> u8 offset; /* register's offset */
> + u16 nvram_size;
> + u16 nvram_offset;
> + u8 num_addrs;
> u8 regs[11];
> enum ds_type type;
> unsigned long flags;
> #define HAS_NVRAM 0 /* bit 0 == sysfs file active */
> #define HAS_ALARM 1 /* bit 1 == irq claimed */
> - struct i2c_client *client;
> + struct i2c_client *client[DS1307_MAX_NUM_ADDRS];
> struct rtc_device *rtc;
> struct work_struct work;
> s32 (*read_block_data)(const struct i2c_client *client, u8 command,
> @@ -113,23 +117,26 @@ struct ds1307 {
> };
>
> struct chip_desc {
> - unsigned nvram56:1;
> + unsigned nvram:1;
> unsigned alarm:1;
> };
>
> static const struct chip_desc chips[] = {
> [ds_1307] = {
> - .nvram56 = 1,
> + .nvram = 1,
> },
> [ds_1337] = {
> .alarm = 1,
> },
> [ds_1338] = {
> - .nvram56 = 1,
> + .nvram = 1,
> },
> [ds_1339] = {
> .alarm = 1,
> },
> +[ds_1388] = {
> + .nvram = 1,
> +},
> [ds_1340] = {
> },
> [ds_3231] = {
> @@ -248,7 +255,7 @@ static void ds1307_work(struct work_stru
> int stat, control;
>
> ds1307 = container_of(work, struct ds1307, work);
> - client = ds1307->client;
> + client = ds1307->client[0];
> lock = &ds1307->rtc->ops_lock;
>
> mutex_lock(lock);
> @@ -294,7 +301,7 @@ static int ds1307_get_time(struct device
> int tmp;
>
> /* read the RTC date and time registers all at once */
> - tmp = ds1307->read_block_data(ds1307->client,
> + tmp = ds1307->read_block_data(ds1307->client[0],
> ds1307->offset, 7, ds1307->regs);
> if (tmp != 7) {
> dev_err(dev, "%s error %d\n", "read", tmp);
> @@ -372,7 +379,7 @@ static int ds1307_set_time(struct device
> "write", buf[0], buf[1], buf[2], buf[3],
> buf[4], buf[5], buf[6]);
>
> - result = ds1307->write_block_data(ds1307->client,
> + result = ds1307->write_block_data(ds1307->client[0],
> ds1307->offset, 7, buf);
> if (result < 0) {
> dev_err(dev, "%s error %d\n", "write", result);
> @@ -530,8 +537,6 @@ static const struct rtc_class_ops ds13xx
>
> /*----------------------------------------------------------------------*/
>
> -#define NVRAM_SIZE 56
> -
> static ssize_t
> ds1307_nvram_read(struct file *filp, struct kobject *kobj,
> struct bin_attribute *attr,
> @@ -540,18 +545,39 @@ ds1307_nvram_read(struct file *filp, str
> struct i2c_client *client;
> struct ds1307 *ds1307;
> int result;
> + unsigned int addr = 0;
>
> client = kobj_to_i2c_client(kobj);
> ds1307 = i2c_get_clientdata(client);
>
> - if (unlikely(off >= NVRAM_SIZE))
> + if (unlikely(off >= ds1307->nvram_size))
> return 0;
> - if ((off + count) > NVRAM_SIZE)
> - count = NVRAM_SIZE - off;
> + if ((off + count) > ds1307->nvram_size)
> + count = ds1307->nvram_size - off;
> if (unlikely(!count))
> return count;
>
> - result = ds1307->read_block_data(client, 8 + off, count, buf);
> + if (ds1307->nvram_offset < 0xff) { /* ds1307, ds1338 */
> + result = ds1307->read_block_data(client, ds1307->nvram_offset + off,
> count, buf);
> + } else { /* ds1388 */
> + if ((off <= 0x00ff) && (off + count > 0x00ff)) { /* read spans two
> blocks */
> + client = ds1307->client[1]; /* first block */
> + addr = (unsigned int)off;
> + result = ds1307->read_block_data(client, addr, (0x100-addr), buf);
> + client = ds1307->client[2]; /* second block */
> + result += ds1307->read_block_data(client, 0x00, (count+addr-0x100),
> (buf+0x100-addr));
> +
> + } else { /* read spans one blocks */
> + if (off <= 0x00ff) {
> + client = ds1307->client[1];
> + addr = (unsigned int)off;
> + } else if (off <= 0x0200) {
> + client = ds1307->client[2];
> + addr = (unsigned int)off - 0x100;
> + }
> + result = ds1307->read_block_data(client, addr, count, buf);
> + }
> + }
> if (result < 0)
> dev_err(&client->dev, "%s error %d\n", "nvram read", result);
> return result;
> @@ -565,18 +591,40 @@ ds1307_nvram_write(struct file *filp, st
> struct i2c_client *client;
> struct ds1307 *ds1307;
> int result;
> + unsigned int addr = 0;
>
> client = kobj_to_i2c_client(kobj);
> ds1307 = i2c_get_clientdata(client);
>
> - if (unlikely(off >= NVRAM_SIZE))
> + if (unlikely(off >= ds1307->nvram_size))
> return -EFBIG;
> - if ((off + count) > NVRAM_SIZE)
> - count = NVRAM_SIZE - off;
> + if ((off + count) > ds1307->nvram_size)
> + count = ds1307->nvram_size - off;
> if (unlikely(!count))
> return count;
>
> - result = ds1307->write_block_data(client, 8 + off, count, buf);
> + if (ds1307->nvram_offset < 0xff) { /* ds1307, ds1338 */
> + result = ds1307->write_block_data(client, ds1307->nvram_offset + off,
> count, buf);
> + } else { /* ds1388 */
> + if ((off <= 0x00ff) && ((off + count) > 0x00ff)) { /* read spans two
> blocks */
> + client = ds1307->client[1]; /* first block */
> + addr = (unsigned int)off;
> + result = ds1307->write_block_data(client, addr, (0x100-addr), buf);
> + client = ds1307->client[2]; /* second block */
> + result = ds1307->write_block_data(client, 0x00, (addr+count-0x100),
> (buf+0x100-addr));
> +
> + } else { /* Device address/offset translation */
> + dev_dbg(&client->dev, "Write spans one block\n");
> + if (off <= 0x00ff) {
> + client = ds1307->client[1];
> + addr = (unsigned int)off;
> + } else if (off <= 0x0200) {
> + client = ds1307->client[2];
> + addr = (unsigned int)off - 0x100;
> + }
> + result = ds1307->write_block_data(client, addr, count, buf);
> + }
> + }
> if (result < 0) {
> dev_err(&client->dev, "%s error %d\n", "nvram write", result);
> return result;
> @@ -592,7 +640,6 @@ static struct bin_attribute nvram = {
>
> .read = ds1307_nvram_read,
> .write = ds1307_nvram_write,
> - .size = NVRAM_SIZE,
> };
>
> /*----------------------------------------------------------------------*/
> @@ -605,6 +652,7 @@ static int __devinit ds1307_probe(struct
> struct ds1307 *ds1307;
> int err = -ENODEV;
> int tmp;
> + int i;
> const struct chip_desc *chip = &chips[id->driver_data];
> struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent);
> int want_irq = false;
> @@ -624,9 +672,10 @@ static int __devinit ds1307_probe(struct
>
> i2c_set_clientdata(client, ds1307);
>
> - ds1307->client = client;
> + ds1307->client[0] = client;
> ds1307->type = id->driver_data;
> ds1307->offset = 0;
> + ds1307->num_addrs = 1;
>
> buf = ds1307->regs;
> if (i2c_check_functionality(adapter, I2C_FUNC_SMBUS_I2C_BLOCK)) {
> @@ -642,12 +691,12 @@ static int __devinit ds1307_probe(struct
> case ds_1339:
> case ds_3231:
> /* has IRQ? */
> - if (ds1307->client->irq > 0 && chip->alarm) {
> + if (client->irq > 0 && chip->alarm) {
> INIT_WORK(&ds1307->work, ds1307_work);
> want_irq = true;
> }
> /* get registers that the "rtc" read below won't read... */
> - tmp = ds1307->read_block_data(ds1307->client,
> + tmp = ds1307->read_block_data(client,
> DS1337_REG_CONTROL, 2, buf);
> if (tmp != 2) {
> pr_debug("read error %d\n", tmp);
> @@ -681,7 +730,7 @@ static int __devinit ds1307_probe(struct
> break;
>
> case rx_8025:
> - tmp = i2c_smbus_read_i2c_block_data(ds1307->client,
> + tmp = i2c_smbus_read_i2c_block_data(client,
> RX8025_REG_CTRL1 << 4 | 0x08, 2, buf);
> if (tmp != 2) {
> pr_debug("read error %d\n", tmp);
> @@ -747,6 +796,14 @@ static int __devinit ds1307_probe(struct
> break;
> case ds_1388:
> ds1307->offset = 1; /* Seconds starts at 1 */
> + ds1307->num_addrs = 3; /* uses 3 I2C addrs */
> + ds1307->nvram_size = 512; /* 512 bytes EEPROM */
> + ds1307->nvram_offset = 0xff;
> + break;
> + case ds_1307:
> + case ds_1338:
> + ds1307->nvram_size = 56; /* 56 bytes NVRAM */
> + ds1307->nvram_offset = 8;
> break;
> default:
> break;
> @@ -754,7 +811,7 @@ static int __devinit ds1307_probe(struct
>
> read_rtc:
> /* read RTC registers */
> - tmp = ds1307->read_block_data(ds1307->client, ds1307->offset, 8, buf);
> + tmp = ds1307->read_block_data(client, ds1307->offset, 8, buf);
> if (tmp != 8) {
> pr_debug("read error %d\n", tmp);
> err = -EIO;
> @@ -866,16 +923,35 @@ read_rtc:
> dev_dbg(&client->dev, "got IRQ %d\n", client->irq);
> }
>
> - if (chip->nvram56) {
> + if (chip->nvram) {
> + nvram.size = ds1307->nvram_size;
> err = sysfs_create_bin_file(&client->dev.kobj, &nvram);
> if (err == 0) {
> set_bit(HAS_NVRAM, &ds1307->flags);
> - dev_info(&client->dev, "56 bytes nvram\n");
> + dev_info(&client->dev, "%d bytes nvram\n", nvram.size);
> + }
> + }
> +
> + /* use dummy devices for multiple address chips */
> + for (i = 1; i < ds1307->num_addrs; i++) {
> + ds1307->client[i] = i2c_new_dummy(client->adapter, client->addr + i);
> + if (!ds1307->client[i]) {
> + dev_err(&client->dev, "address 0x%02x unavailable\n",
> + client->addr + i);
> + err = -EADDRINUSE;
> + goto err_clients;
> }
> + i2c_set_clientdata(ds1307->client[i], ds1307);
> }
>
> return 0;
>
> +err_clients:
> + for (i = 1; i < ds1307->num_addrs; i++) {
> + if (ds1307->client[i]) {
> + i2c_unregister_device(ds1307->client[i]);
> + }
> + }
> exit_irq:
> rtc_device_unregister(ds1307->rtc);
> exit_free:
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2.6.39-rc7] RTC: rtc-ds1307 EEPROM support for ds1388 chip
2011-07-17 23:32 ` Austin Boyle
@ 2011-07-22 10:19 ` Joakim Tjernlund
0 siblings, 0 replies; 9+ messages in thread
From: Joakim Tjernlund @ 2011-07-22 10:19 UTC (permalink / raw)
To: Austin Boyle
Cc: Andrew Morton, Alessandro Zummo, David Brownell, linux-kernel,
rtc-linux, Wolfram Sang
Austin Boyle <Austin.Boyle@aviatnet.com> wrote on 2011/07/18 01:32:25:
>
> Hi Joakim,
>
> I see that you were responsible for adding support for the ds1388 RTC in
> the ds1307 driver. Thank you for your work, I am using the driver for
> this IC myself. I have been trying to submit a patch to add support for
> the 512 bytes of EEPROM available on the ds1388 but have not received
> any responses. Would you be willing to review my patch? I can send
> through another version applied to the very latest kernel. Are the RTC
> patches all submitted through Alessandro Zummo who is listed as the
> maintainer or should they be sent directly to Linus?
Hi Austin,
I am on vacation and won't be able to review your patch for a long time. I think
Andrew Morton is your best bet if Alessandro is unavailable though.
Jocke
>
> Kind Regards,
> Austin Boyle.
>
> On Tue, 2011-06-14 at 16:15 +1200, Austin Boyle wrote:
> > On Mon, 2011-05-23 at 17:18 +1200, Austin Boyle wrote:
> > > This is my patch which extends the NVRAM access functions in the DS1307
> > > driver to support the 512 bytes of EEPROM on the DS1388 chip. Could you
> > > please review the patch? If there are any problems with the way I have
> > > implemented this or suggested improvements I will gladly modify. Thanks!
> >
> > This is an updated version of the patch which prints the correct NVRAM
> > size at init. Could someone please give me some feedback?
> >
> > [PATCH 3.0-rc3] RTC: rtc-ds1307: EEPROM support for ds1388 chip
> >
> > This patch adds support for different sized NVRAM, such as
> > the 512 bytes of EEPROM on the DS1388 chip.
> >
> > Cc: Alessandro Zummo <a.zummo@towertech.it>
> > Signed-off-by: Austin Boyle <austin.boyle@aviatnet.com>
> > ---
> > diff -upNr a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
> > --- a/drivers/rtc/rtc-ds1307.c 2011-06-14 15:56:45.221497487 +1200
> > +++ b/drivers/rtc/rtc-ds1307.c 2011-06-14 15:57:28.803510835 +1200
> > @@ -95,15 +95,19 @@ enum ds_type {
> > # define RX8025_BIT_VDET 0x40
> > # define RX8025_BIT_XST 0x20
> >
> > +#define DS1307_MAX_NUM_ADDRS 3
> >
> > struct ds1307 {
> > u8 offset; /* register's offset */
> > + u16 nvram_size;
> > + u16 nvram_offset;
> > + u8 num_addrs;
> > u8 regs[11];
> > enum ds_type type;
> > unsigned long flags;
> > #define HAS_NVRAM 0 /* bit 0 == sysfs file active */
> > #define HAS_ALARM 1 /* bit 1 == irq claimed */
> > - struct i2c_client *client;
> > + struct i2c_client *client[DS1307_MAX_NUM_ADDRS];
> > struct rtc_device *rtc;
> > struct work_struct work;
> > s32 (*read_block_data)(const struct i2c_client *client, u8 command,
> > @@ -113,23 +117,26 @@ struct ds1307 {
> > };
> >
> > struct chip_desc {
> > - unsigned nvram56:1;
> > + unsigned nvram:1;
> > unsigned alarm:1;
> > };
> >
> > static const struct chip_desc chips[] = {
> > [ds_1307] = {
> > - .nvram56 = 1,
> > + .nvram = 1,
> > },
> > [ds_1337] = {
> > .alarm = 1,
> > },
> > [ds_1338] = {
> > - .nvram56 = 1,
> > + .nvram = 1,
> > },
> > [ds_1339] = {
> > .alarm = 1,
> > },
> > +[ds_1388] = {
> > + .nvram = 1,
> > +},
> > [ds_1340] = {
> > },
> > [ds_3231] = {
> > @@ -248,7 +255,7 @@ static void ds1307_work(struct work_stru
> > int stat, control;
> >
> > ds1307 = container_of(work, struct ds1307, work);
> > - client = ds1307->client;
> > + client = ds1307->client[0];
> > lock = &ds1307->rtc->ops_lock;
> >
> > mutex_lock(lock);
> > @@ -294,7 +301,7 @@ static int ds1307_get_time(struct device
> > int tmp;
> >
> > /* read the RTC date and time registers all at once */
> > - tmp = ds1307->read_block_data(ds1307->client,
> > + tmp = ds1307->read_block_data(ds1307->client[0],
> > ds1307->offset, 7, ds1307->regs);
> > if (tmp != 7) {
> > dev_err(dev, "%s error %d\n", "read", tmp);
> > @@ -372,7 +379,7 @@ static int ds1307_set_time(struct device
> > "write", buf[0], buf[1], buf[2], buf[3],
> > buf[4], buf[5], buf[6]);
> >
> > - result = ds1307->write_block_data(ds1307->client,
> > + result = ds1307->write_block_data(ds1307->client[0],
> > ds1307->offset, 7, buf);
> > if (result < 0) {
> > dev_err(dev, "%s error %d\n", "write", result);
> > @@ -530,8 +537,6 @@ static const struct rtc_class_ops ds13xx
> >
> > /*----------------------------------------------------------------------*/
> >
> > -#define NVRAM_SIZE 56
> > -
> > static ssize_t
> > ds1307_nvram_read(struct file *filp, struct kobject *kobj,
> > struct bin_attribute *attr,
> > @@ -540,18 +545,39 @@ ds1307_nvram_read(struct file *filp, str
> > struct i2c_client *client;
> > struct ds1307 *ds1307;
> > int result;
> > + unsigned int addr = 0;
> >
> > client = kobj_to_i2c_client(kobj);
> > ds1307 = i2c_get_clientdata(client);
> >
> > - if (unlikely(off >= NVRAM_SIZE))
> > + if (unlikely(off >= ds1307->nvram_size))
> > return 0;
> > - if ((off + count) > NVRAM_SIZE)
> > - count = NVRAM_SIZE - off;
> > + if ((off + count) > ds1307->nvram_size)
> > + count = ds1307->nvram_size - off;
> > if (unlikely(!count))
> > return count;
> >
> > - result = ds1307->read_block_data(client, 8 + off, count, buf);
> > + if (ds1307->nvram_offset < 0xff) { /* ds1307, ds1338 */
> > + result = ds1307->read_block_data(client, ds1307->nvram_offset + off,
> > count, buf);
> > + } else { /* ds1388 */
> > + if ((off <= 0x00ff) && (off + count > 0x00ff)) { /* read spans two
> > blocks */
> > + client = ds1307->client[1]; /* first block */
> > + addr = (unsigned int)off;
> > + result = ds1307->read_block_data(client, addr, (0x100-addr), buf);
> > + client = ds1307->client[2]; /* second block */
> > + result += ds1307->read_block_data(client, 0x00, (count+addr-0x100),
> > (buf+0x100-addr));
> > +
> > + } else { /* read spans one blocks */
> > + if (off <= 0x00ff) {
> > + client = ds1307->client[1];
> > + addr = (unsigned int)off;
> > + } else if (off <= 0x0200) {
> > + client = ds1307->client[2];
> > + addr = (unsigned int)off - 0x100;
> > + }
> > + result = ds1307->read_block_data(client, addr, count, buf);
> > + }
> > + }
> > if (result < 0)
> > dev_err(&client->dev, "%s error %d\n", "nvram read", result);
> > return result;
> > @@ -565,18 +591,40 @@ ds1307_nvram_write(struct file *filp, st
> > struct i2c_client *client;
> > struct ds1307 *ds1307;
> > int result;
> > + unsigned int addr = 0;
> >
> > client = kobj_to_i2c_client(kobj);
> > ds1307 = i2c_get_clientdata(client);
> >
> > - if (unlikely(off >= NVRAM_SIZE))
> > + if (unlikely(off >= ds1307->nvram_size))
> > return -EFBIG;
> > - if ((off + count) > NVRAM_SIZE)
> > - count = NVRAM_SIZE - off;
> > + if ((off + count) > ds1307->nvram_size)
> > + count = ds1307->nvram_size - off;
> > if (unlikely(!count))
> > return count;
> >
> > - result = ds1307->write_block_data(client, 8 + off, count, buf);
> > + if (ds1307->nvram_offset < 0xff) { /* ds1307, ds1338 */
> > + result = ds1307->write_block_data(client, ds1307->nvram_offset + off,
> > count, buf);
> > + } else { /* ds1388 */
> > + if ((off <= 0x00ff) && ((off + count) > 0x00ff)) { /* read spans two
> > blocks */
> > + client = ds1307->client[1]; /* first block */
> > + addr = (unsigned int)off;
> > + result = ds1307->write_block_data(client, addr, (0x100-addr), buf);
> > + client = ds1307->client[2]; /* second block */
> > + result = ds1307->write_block_data(client, 0x00, (addr+count-0x100),
> > (buf+0x100-addr));
> > +
> > + } else { /* Device address/offset translation */
> > + dev_dbg(&client->dev, "Write spans one block\n");
> > + if (off <= 0x00ff) {
> > + client = ds1307->client[1];
> > + addr = (unsigned int)off;
> > + } else if (off <= 0x0200) {
> > + client = ds1307->client[2];
> > + addr = (unsigned int)off - 0x100;
> > + }
> > + result = ds1307->write_block_data(client, addr, count, buf);
> > + }
> > + }
> > if (result < 0) {
> > dev_err(&client->dev, "%s error %d\n", "nvram write", result);
> > return result;
> > @@ -592,7 +640,6 @@ static struct bin_attribute nvram = {
> >
> > .read = ds1307_nvram_read,
> > .write = ds1307_nvram_write,
> > - .size = NVRAM_SIZE,
> > };
> >
> > /*----------------------------------------------------------------------*/
> > @@ -605,6 +652,7 @@ static int __devinit ds1307_probe(struct
> > struct ds1307 *ds1307;
> > int err = -ENODEV;
> > int tmp;
> > + int i;
> > const struct chip_desc *chip = &chips[id->driver_data];
> > struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent);
> > int want_irq = false;
> > @@ -624,9 +672,10 @@ static int __devinit ds1307_probe(struct
> >
> > i2c_set_clientdata(client, ds1307);
> >
> > - ds1307->client = client;
> > + ds1307->client[0] = client;
> > ds1307->type = id->driver_data;
> > ds1307->offset = 0;
> > + ds1307->num_addrs = 1;
> >
> > buf = ds1307->regs;
> > if (i2c_check_functionality(adapter, I2C_FUNC_SMBUS_I2C_BLOCK)) {
> > @@ -642,12 +691,12 @@ static int __devinit ds1307_probe(struct
> > case ds_1339:
> > case ds_3231:
> > /* has IRQ? */
> > - if (ds1307->client->irq > 0 && chip->alarm) {
> > + if (client->irq > 0 && chip->alarm) {
> > INIT_WORK(&ds1307->work, ds1307_work);
> > want_irq = true;
> > }
> > /* get registers that the "rtc" read below won't read... */
> > - tmp = ds1307->read_block_data(ds1307->client,
> > + tmp = ds1307->read_block_data(client,
> > DS1337_REG_CONTROL, 2, buf);
> > if (tmp != 2) {
> > pr_debug("read error %d\n", tmp);
> > @@ -681,7 +730,7 @@ static int __devinit ds1307_probe(struct
> > break;
> >
> > case rx_8025:
> > - tmp = i2c_smbus_read_i2c_block_data(ds1307->client,
> > + tmp = i2c_smbus_read_i2c_block_data(client,
> > RX8025_REG_CTRL1 << 4 | 0x08, 2, buf);
> > if (tmp != 2) {
> > pr_debug("read error %d\n", tmp);
> > @@ -747,6 +796,14 @@ static int __devinit ds1307_probe(struct
> > break;
> > case ds_1388:
> > ds1307->offset = 1; /* Seconds starts at 1 */
> > + ds1307->num_addrs = 3; /* uses 3 I2C addrs */
> > + ds1307->nvram_size = 512; /* 512 bytes EEPROM */
> > + ds1307->nvram_offset = 0xff;
> > + break;
> > + case ds_1307:
> > + case ds_1338:
> > + ds1307->nvram_size = 56; /* 56 bytes NVRAM */
> > + ds1307->nvram_offset = 8;
> > break;
> > default:
> > break;
> > @@ -754,7 +811,7 @@ static int __devinit ds1307_probe(struct
> >
> > read_rtc:
> > /* read RTC registers */
> > - tmp = ds1307->read_block_data(ds1307->client, ds1307->offset, 8, buf);
> > + tmp = ds1307->read_block_data(client, ds1307->offset, 8, buf);
> > if (tmp != 8) {
> > pr_debug("read error %d\n", tmp);
> > err = -EIO;
> > @@ -866,16 +923,35 @@ read_rtc:
> > dev_dbg(&client->dev, "got IRQ %d\n", client->irq);
> > }
> >
> > - if (chip->nvram56) {
> > + if (chip->nvram) {
> > + nvram.size = ds1307->nvram_size;
> > err = sysfs_create_bin_file(&client->dev.kobj, &nvram);
> > if (err == 0) {
> > set_bit(HAS_NVRAM, &ds1307->flags);
> > - dev_info(&client->dev, "56 bytes nvram\n");
> > + dev_info(&client->dev, "%d bytes nvram\n", nvram.size);
> > + }
> > + }
> > +
> > + /* use dummy devices for multiple address chips */
> > + for (i = 1; i < ds1307->num_addrs; i++) {
> > + ds1307->client[i] = i2c_new_dummy(client->adapter, client->addr + i);
> > + if (!ds1307->client[i]) {
> > + dev_err(&client->dev, "address 0x%02x unavailable\n",
> > + client->addr + i);
> > + err = -EADDRINUSE;
> > + goto err_clients;
> > }
> > + i2c_set_clientdata(ds1307->client[i], ds1307);
> > }
> >
> > return 0;
> >
> > +err_clients:
> > + for (i = 1; i < ds1307->num_addrs; i++) {
> > + if (ds1307->client[i]) {
> > + i2c_unregister_device(ds1307->client[i]);
> > + }
> > + }
> > exit_irq:
> > rtc_device_unregister(ds1307->rtc);
> > exit_free:
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2.6.39-rc7] RTC: rtc-ds1307 EEPROM support for ds1388 chip
2011-06-14 4:14 ` Austin Boyle
2011-07-17 23:32 ` Austin Boyle
@ 2011-07-22 21:54 ` Andrew Morton
2011-07-31 22:30 ` Austin Boyle
1 sibling, 1 reply; 9+ messages in thread
From: Andrew Morton @ 2011-07-22 21:54 UTC (permalink / raw)
To: Austin Boyle; +Cc: Alessandro Zummo, rtc-linux, linux-kernel
On Tue, 14 Jun 2011 16:14:57 +1200
Austin Boyle <Austin.Boyle@aviatnet.com> wrote:
> On Mon, 2011-05-23 at 17:18 +1200, Austin Boyle wrote:
> > This is my patch which extends the NVRAM access functions in the DS1307
> > driver to support the 512 bytes of EEPROM on the DS1388 chip. Could you
> > please review the patch? If there are any problems with the way I have
> > implemented this or suggested improvements I will gladly modify. Thanks!
>
> This is an updated version of the patch which prints the correct NVRAM
> size at init. Could someone please give me some feedback?
>
> [PATCH 3.0-rc3] RTC: rtc-ds1307: EEPROM support for ds1388 chip
>
> This patch adds support for different sized NVRAM, such as
> the 512 bytes of EEPROM on the DS1388 chip.
The patch is wordwrapped - please fix your email client.
The patch makes rather a mess of a decent-looking driver. Please use
scripts/checkpatch.pl and review its report.
>
> ...
>
> @@ -540,18 +545,39 @@ ds1307_nvram_read(struct file *filp, str
> struct i2c_client *client;
> struct ds1307 *ds1307;
> int result;
> + unsigned int addr = 0;
>
> client = kobj_to_i2c_client(kobj);
> ds1307 = i2c_get_clientdata(client);
>
> - if (unlikely(off >= NVRAM_SIZE))
> + if (unlikely(off >= ds1307->nvram_size))
> return 0;
> - if ((off + count) > NVRAM_SIZE)
> - count = NVRAM_SIZE - off;
> + if ((off + count) > ds1307->nvram_size)
> + count = ds1307->nvram_size - off;
> if (unlikely(!count))
> return count;
>
> - result = ds1307->read_block_data(client, 8 + off, count, buf);
> + if (ds1307->nvram_offset < 0xff) { /* ds1307, ds1338 */
> + result = ds1307->read_block_data(client, ds1307->nvram_offset + off,
> count, buf);
> + } else { /* ds1388 */
> + if ((off <= 0x00ff) && (off + count > 0x00ff)) { /* read spans two
> blocks */
> + client = ds1307->client[1]; /* first block */
> + addr = (unsigned int)off;
> + result = ds1307->read_block_data(client, addr, (0x100-addr), buf);
Ignores a possible error return value?
> + client = ds1307->client[2]; /* second block */
> + result += ds1307->read_block_data(client, 0x00, (count+addr-0x100),
> (buf+0x100-addr));
> +
> + } else { /* read spans one blocks */
> + if (off <= 0x00ff) {
> + client = ds1307->client[1];
> + addr = (unsigned int)off;
> + } else if (off <= 0x0200) {
> + client = ds1307->client[2];
> + addr = (unsigned int)off - 0x100;
> + }
> + result = ds1307->read_block_data(client, addr, count, buf);
> + }
> + }
> if (result < 0)
> dev_err(&client->dev, "%s error %d\n", "nvram read", result);
> return result;
> @@ -565,18 +591,40 @@ ds1307_nvram_write(struct file *filp, st
> struct i2c_client *client;
> struct ds1307 *ds1307;
> int result;
> + unsigned int addr = 0;
>
> client = kobj_to_i2c_client(kobj);
> ds1307 = i2c_get_clientdata(client);
>
> - if (unlikely(off >= NVRAM_SIZE))
> + if (unlikely(off >= ds1307->nvram_size))
> return -EFBIG;
> - if ((off + count) > NVRAM_SIZE)
> - count = NVRAM_SIZE - off;
> + if ((off + count) > ds1307->nvram_size)
> + count = ds1307->nvram_size - off;
> if (unlikely(!count))
> return count;
>
> - result = ds1307->write_block_data(client, 8 + off, count, buf);
> + if (ds1307->nvram_offset < 0xff) { /* ds1307, ds1338 */
> + result = ds1307->write_block_data(client, ds1307->nvram_offset + off,
> count, buf);
> + } else { /* ds1388 */
> + if ((off <= 0x00ff) && ((off + count) > 0x00ff)) { /* read spans two
> blocks */
> + client = ds1307->client[1]; /* first block */
> + addr = (unsigned int)off;
> + result = ds1307->write_block_data(client, addr, (0x100-addr), buf);
Error checking?
> + client = ds1307->client[2]; /* second block */
> + result = ds1307->write_block_data(client, 0x00, (addr+count-0x100),
> (buf+0x100-addr));
> +
> + } else { /* Device address/offset translation */
> + dev_dbg(&client->dev, "Write spans one block\n");
> + if (off <= 0x00ff) {
A single space before the {
> + client = ds1307->client[1];
> + addr = (unsigned int)off;
> + } else if (off <= 0x0200) {
ditto
> + client = ds1307->client[2];
> + addr = (unsigned int)off - 0x100;
> + }
> + result = ds1307->write_block_data(client, addr, count, buf);
> + }
> + }
> if (result < 0) {
> dev_err(&client->dev, "%s error %d\n", "nvram write", result);
> return result;
>
> ...
>
> @@ -866,16 +923,35 @@ read_rtc:
> dev_dbg(&client->dev, "got IRQ %d\n", client->irq);
> }
>
> - if (chip->nvram56) {
> + if (chip->nvram) {
> + nvram.size = ds1307->nvram_size;
> err = sysfs_create_bin_file(&client->dev.kobj, &nvram);
> if (err == 0) {
> set_bit(HAS_NVRAM, &ds1307->flags);
> - dev_info(&client->dev, "56 bytes nvram\n");
> + dev_info(&client->dev, "%d bytes nvram\n", nvram.size);
> + }
> + }
> +
> + /* use dummy devices for multiple address chips */
> + for (i = 1; i < ds1307->num_addrs; i++) {
> + ds1307->client[i] = i2c_new_dummy(client->adapter, client->addr + i);
> + if (!ds1307->client[i]) {
> + dev_err(&client->dev, "address 0x%02x unavailable\n",
> + client->addr + i);
> + err = -EADDRINUSE;
EADDRINUSE is a networking error code. Returning that from an
rtc-related operation might mislead the operator.
> + goto err_clients;
> }
> + i2c_set_clientdata(ds1307->client[i], ds1307);
> }
>
> return 0;
>
> +err_clients:
> + for (i = 1; i < ds1307->num_addrs; i++) {
> + if (ds1307->client[i]) {
> + i2c_unregister_device(ds1307->client[i]);
> + }
We normally omit braces around a single statement.
The `if (ds1307->client[i])' test should be unneeded anyway - just
unregister client[1] though client[i - 1].
Why isn't client[0] used here btw?
> + }
> exit_irq:
> rtc_device_unregister(ds1307->rtc);
> exit_free:
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2.6.39-rc7] RTC: rtc-ds1307 EEPROM support for ds1388 chip
2011-07-22 21:54 ` Andrew Morton
@ 2011-07-31 22:30 ` Austin Boyle
2011-09-23 23:04 ` Andrew Morton
0 siblings, 1 reply; 9+ messages in thread
From: Austin Boyle @ 2011-07-31 22:30 UTC (permalink / raw)
To: Andrew Morton; +Cc: Alessandro Zummo, Joakim Tjernlund, rtc-linux, linux-kernel
Hi Andrew,
Thank you very much for your feedback. I have corrected the issues you raised as follows.
The updated patch has been appended.
On Fri, 2011-07-22 at 14:54 -0700, Andrew Morton wrote:
> On Tue, 14 Jun 2011 16:14:57 +1200
> Austin Boyle <Austin.Boyle@aviatnet.com> wrote:
>
> > On Mon, 2011-05-23 at 17:18 +1200, Austin Boyle wrote:
> > > This is my patch which extends the NVRAM access functions in the DS1307
> > > driver to support the 512 bytes of EEPROM on the DS1388 chip. Could you
> > > please review the patch? If there are any problems with the way I have
> > > implemented this or suggested improvements I will gladly modify. Thanks!
> >
> > This is an updated version of the patch which prints the correct NVRAM
> > size at init. Could someone please give me some feedback?
> >
> > [PATCH 3.0-rc3] RTC: rtc-ds1307: EEPROM support for ds1388 chip
> >
> > This patch adds support for different sized NVRAM, such as
> > the 512 bytes of EEPROM on the DS1388 chip.
>
> The patch is wordwrapped - please fix your email client.
Done.
>
> The patch makes rather a mess of a decent-looking driver. Please use
> scripts/checkpatch.pl and review its report.
>
Fixed all errors and warnings.
> >
> > ...
> >
> > @@ -540,18 +545,39 @@ ds1307_nvram_read(struct file *filp, str
> > struct i2c_client *client;
> > struct ds1307 *ds1307;
> > int result;
> > + unsigned int addr = 0;
> >
> > client = kobj_to_i2c_client(kobj);
> > ds1307 = i2c_get_clientdata(client);
> >
> > - if (unlikely(off >= NVRAM_SIZE))
> > + if (unlikely(off >= ds1307->nvram_size))
> > return 0;
> > - if ((off + count) > NVRAM_SIZE)
> > - count = NVRAM_SIZE - off;
> > + if ((off + count) > ds1307->nvram_size)
> > + count = ds1307->nvram_size - off;
> > if (unlikely(!count))
> > return count;
> >
> > - result = ds1307->read_block_data(client, 8 + off, count, buf);
> > + if (ds1307->nvram_offset < 0xff) { /* ds1307, ds1338 */
> > + result = ds1307->read_block_data(client, ds1307->nvram_offset + off,
> > count, buf);
> > + } else { /* ds1388 */
> > + if ((off <= 0x00ff) && (off + count > 0x00ff)) { /* read spans two
> > blocks */
> > + client = ds1307->client[1]; /* first block */
> > + addr = (unsigned int)off;
> > + result = ds1307->read_block_data(client, addr, (0x100-addr), buf);
>
> Ignores a possible error return value?
Added check.
>
> > + client = ds1307->client[2]; /* second block */
> > + result += ds1307->read_block_data(client, 0x00, (count+addr-0x100),
> > (buf+0x100-addr));
> > +
> > + } else { /* read spans one blocks */
> > + if (off <= 0x00ff) {
> > + client = ds1307->client[1];
> > + addr = (unsigned int)off;
> > + } else if (off <= 0x0200) {
> > + client = ds1307->client[2];
> > + addr = (unsigned int)off - 0x100;
> > + }
> > + result = ds1307->read_block_data(client, addr, count, buf);
> > + }
> > + }
> > if (result < 0)
> > dev_err(&client->dev, "%s error %d\n", "nvram read", result);
> > return result;
> > @@ -565,18 +591,40 @@ ds1307_nvram_write(struct file *filp, st
> > struct i2c_client *client;
> > struct ds1307 *ds1307;
> > int result;
> > + unsigned int addr = 0;
> >
> > client = kobj_to_i2c_client(kobj);
> > ds1307 = i2c_get_clientdata(client);
> >
> > - if (unlikely(off >= NVRAM_SIZE))
> > + if (unlikely(off >= ds1307->nvram_size))
> > return -EFBIG;
> > - if ((off + count) > NVRAM_SIZE)
> > - count = NVRAM_SIZE - off;
> > + if ((off + count) > ds1307->nvram_size)
> > + count = ds1307->nvram_size - off;
> > if (unlikely(!count))
> > return count;
> >
> > - result = ds1307->write_block_data(client, 8 + off, count, buf);
> > + if (ds1307->nvram_offset < 0xff) { /* ds1307, ds1338 */
> > + result = ds1307->write_block_data(client, ds1307->nvram_offset + off,
> > count, buf);
> > + } else { /* ds1388 */
> > + if ((off <= 0x00ff) && ((off + count) > 0x00ff)) { /* read spans two
> > blocks */
> > + client = ds1307->client[1]; /* first block */
> > + addr = (unsigned int)off;
> > + result = ds1307->write_block_data(client, addr, (0x100-addr), buf);
>
> Error checking?
Added.
>
> > + client = ds1307->client[2]; /* second block */
> > + result = ds1307->write_block_data(client, 0x00, (addr+count-0x100),
> > (buf+0x100-addr));
> > +
> > + } else { /* Device address/offset translation */
> > + dev_dbg(&client->dev, "Write spans one block\n");
> > + if (off <= 0x00ff) {
>
> A single space before the {
Fixed.
>
> > + client = ds1307->client[1];
> > + addr = (unsigned int)off;
> > + } else if (off <= 0x0200) {
>
> ditto
Fixed.
>
> > + client = ds1307->client[2];
> > + addr = (unsigned int)off - 0x100;
> > + }
> > + result = ds1307->write_block_data(client, addr, count, buf);
> > + }
> > + }
> > if (result < 0) {
> > dev_err(&client->dev, "%s error %d\n", "nvram write", result);
> > return result;
> >
> > ...
> >
> > @@ -866,16 +923,35 @@ read_rtc:
> > dev_dbg(&client->dev, "got IRQ %d\n", client->irq);
> > }
> >
> > - if (chip->nvram56) {
> > + if (chip->nvram) {
> > + nvram.size = ds1307->nvram_size;
> > err = sysfs_create_bin_file(&client->dev.kobj, &nvram);
> > if (err == 0) {
> > set_bit(HAS_NVRAM, &ds1307->flags);
> > - dev_info(&client->dev, "56 bytes nvram\n");
> > + dev_info(&client->dev, "%d bytes nvram\n", nvram.size);
> > + }
> > + }
> > +
> > + /* use dummy devices for multiple address chips */
> > + for (i = 1; i < ds1307->num_addrs; i++) {
> > + ds1307->client[i] = i2c_new_dummy(client->adapter, client->addr + i);
> > + if (!ds1307->client[i]) {
> > + dev_err(&client->dev, "address 0x%02x unavailable\n",
> > + client->addr + i);
> > + err = -EADDRINUSE;
>
> EADDRINUSE is a networking error code. Returning that from an
> rtc-related operation might mislead the operator.
Fixed. Now returns -EBUSY, the same as rtc-s35390a does.
>
>
> > + goto err_clients;
> > }
> > + i2c_set_clientdata(ds1307->client[i], ds1307);
> > }
> >
> > return 0;
> >
> > +err_clients:
> > + for (i = 1; i < ds1307->num_addrs; i++) {
> > + if (ds1307->client[i]) {
> > + i2c_unregister_device(ds1307->client[i]);
> > + }
>
> We normally omit braces around a single statement.
Fixed.
>
> The `if (ds1307->client[i])' test should be unneeded anyway - just
> unregister client[1] though client[i - 1].
>
> Why isn't client[0] used here btw?
>
This loop is only unregistering the dummy i2c devices that were created. The same behaviour as rtc-s35390a.
---
[PATCH 3.0] RTC: EEPROM support for DS1388
This patch adds support for different sized NVRAM, such as
the 512 bytes of EEPROM on the DS1388 chip.
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
Signed-off-by: Austin Boyle <austin.boyle@aviatnet.com>
---
diff -upNr a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
--- a/drivers/rtc/rtc-ds1307.c 2011-06-29 11:59:29.275414436 +1200
+++ b/drivers/rtc/rtc-ds1307.c 2011-08-01 10:17:42.415351158 +1200
@@ -95,15 +95,19 @@ enum ds_type {
# define RX8025_BIT_VDET 0x40
# define RX8025_BIT_XST 0x20
+#define DS1307_MAX_NUM_ADDRS 3
struct ds1307 {
u8 offset; /* register's offset */
+ u16 nvram_size;
+ u16 nvram_offset;
+ u8 num_addrs;
u8 regs[11];
enum ds_type type;
unsigned long flags;
#define HAS_NVRAM 0 /* bit 0 == sysfs file active */
#define HAS_ALARM 1 /* bit 1 == irq claimed */
- struct i2c_client *client;
+ struct i2c_client *client[DS1307_MAX_NUM_ADDRS];
struct rtc_device *rtc;
struct work_struct work;
s32 (*read_block_data)(const struct i2c_client *client, u8 command,
@@ -113,23 +117,26 @@ struct ds1307 {
};
struct chip_desc {
- unsigned nvram56:1;
+ unsigned nvram:1;
unsigned alarm:1;
};
static const struct chip_desc chips[] = {
[ds_1307] = {
- .nvram56 = 1,
+ .nvram = 1,
},
[ds_1337] = {
.alarm = 1,
},
[ds_1338] = {
- .nvram56 = 1,
+ .nvram = 1,
},
[ds_1339] = {
.alarm = 1,
},
+[ds_1388] = {
+ .nvram = 1,
+},
[ds_1340] = {
},
[ds_3231] = {
@@ -249,7 +256,7 @@ static void ds1307_work(struct work_stru
int stat, control;
ds1307 = container_of(work, struct ds1307, work);
- client = ds1307->client;
+ client = ds1307->client[0];
lock = &ds1307->rtc->ops_lock;
mutex_lock(lock);
@@ -295,7 +302,7 @@ static int ds1307_get_time(struct device
int tmp;
/* read the RTC date and time registers all at once */
- tmp = ds1307->read_block_data(ds1307->client,
+ tmp = ds1307->read_block_data(ds1307->client[0],
ds1307->offset, 7, ds1307->regs);
if (tmp != 7) {
dev_err(dev, "%s error %d\n", "read", tmp);
@@ -373,7 +380,7 @@ static int ds1307_set_time(struct device
"write", buf[0], buf[1], buf[2], buf[3],
buf[4], buf[5], buf[6]);
- result = ds1307->write_block_data(ds1307->client,
+ result = ds1307->write_block_data(ds1307->client[0],
ds1307->offset, 7, buf);
if (result < 0) {
dev_err(dev, "%s error %d\n", "write", result);
@@ -531,8 +538,6 @@ static const struct rtc_class_ops ds13xx
/*----------------------------------------------------------------------*/
-#define NVRAM_SIZE 56
-
static ssize_t
ds1307_nvram_read(struct file *filp, struct kobject *kobj,
struct bin_attribute *attr,
@@ -541,19 +546,49 @@ ds1307_nvram_read(struct file *filp, str
struct i2c_client *client;
struct ds1307 *ds1307;
int result;
+ unsigned int addr = 0;
client = kobj_to_i2c_client(kobj);
ds1307 = i2c_get_clientdata(client);
- if (unlikely(off >= NVRAM_SIZE))
+ if (unlikely(off >= ds1307->nvram_size))
return 0;
- if ((off + count) > NVRAM_SIZE)
- count = NVRAM_SIZE - off;
+ if ((off + count) > ds1307->nvram_size)
+ count = ds1307->nvram_size - off;
if (unlikely(!count))
return count;
- result = ds1307->read_block_data(client, 8 + off, count, buf);
- if (result < 0)
+ if (ds1307->nvram_offset < 0xff) {
+ result = ds1307->read_block_data(client,
+ ds1307->nvram_offset + off,
+ count, buf);
+ } else {
+ /* read spans two blocks */
+ if ((off <= 0x00ff) && (off + count > 0x00ff)) {
+ client = ds1307->client[1];
+ addr = (unsigned int)off;
+ result = ds1307->read_block_data(client, addr,
+ (0x100-addr), buf);
+ if (result < 0)
+ goto err;
+ client = ds1307->client[2];
+ result = ds1307->read_block_data(client, 0x00,
+ (count+addr-0x100),
+ (buf+0x100-addr));
+ /* read spans one blocks */
+ } else {
+ if (off <= 0x00ff) {
+ client = ds1307->client[1];
+ addr = (unsigned int)off;
+ } else if (off <= 0x0200) {
+ client = ds1307->client[2];
+ addr = (unsigned int)off - 0x100;
+ }
+ result = ds1307->read_block_data(client, addr,
+ count, buf);
+ }
+ }
+err: if (result < 0)
dev_err(&client->dev, "%s error %d\n", "nvram read", result);
return result;
}
@@ -566,19 +601,49 @@ ds1307_nvram_write(struct file *filp, st
struct i2c_client *client;
struct ds1307 *ds1307;
int result;
+ unsigned int addr = 0;
client = kobj_to_i2c_client(kobj);
ds1307 = i2c_get_clientdata(client);
- if (unlikely(off >= NVRAM_SIZE))
+ if (unlikely(off >= ds1307->nvram_size))
return -EFBIG;
- if ((off + count) > NVRAM_SIZE)
- count = NVRAM_SIZE - off;
+ if ((off + count) > ds1307->nvram_size)
+ count = ds1307->nvram_size - off;
if (unlikely(!count))
return count;
- result = ds1307->write_block_data(client, 8 + off, count, buf);
- if (result < 0) {
+ if (ds1307->nvram_offset < 0xff) {
+ result = ds1307->write_block_data(client,
+ ds1307->nvram_offset + off,
+ count, buf);
+ } else {
+ /* read spans two blocks */
+ if ((off <= 0x00ff) && ((off + count) > 0x00ff)) {
+ client = ds1307->client[1];
+ addr = (unsigned int)off;
+ result = ds1307->write_block_data(client, addr,
+ (0x100-addr), buf);
+ if (result < 0)
+ goto err;
+ client = ds1307->client[2];
+ result = ds1307->write_block_data(client, 0x00,
+ (addr+count-0x100),
+ (buf+0x100-addr));
+ /* read spans one blocks */
+ } else {
+ if (off <= 0x00ff) {
+ client = ds1307->client[1];
+ addr = (unsigned int)off;
+ } else if (off <= 0x0200) {
+ client = ds1307->client[2];
+ addr = (unsigned int)off - 0x100;
+ }
+ result = ds1307->write_block_data(client, addr,
+ count, buf);
+ }
+ }
+err: if (result < 0) {
dev_err(&client->dev, "%s error %d\n", "nvram write", result);
return result;
}
@@ -593,7 +658,6 @@ static struct bin_attribute nvram = {
.read = ds1307_nvram_read,
.write = ds1307_nvram_write,
- .size = NVRAM_SIZE,
};
/*----------------------------------------------------------------------*/
@@ -606,6 +670,7 @@ static int __devinit ds1307_probe(struct
struct ds1307 *ds1307;
int err = -ENODEV;
int tmp;
+ int i;
const struct chip_desc *chip = &chips[id->driver_data];
struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent);
int want_irq = false;
@@ -625,9 +690,10 @@ static int __devinit ds1307_probe(struct
i2c_set_clientdata(client, ds1307);
- ds1307->client = client;
+ ds1307->client[0] = client;
ds1307->type = id->driver_data;
ds1307->offset = 0;
+ ds1307->num_addrs = 1;
buf = ds1307->regs;
if (i2c_check_functionality(adapter, I2C_FUNC_SMBUS_I2C_BLOCK)) {
@@ -643,12 +709,12 @@ static int __devinit ds1307_probe(struct
case ds_1339:
case ds_3231:
/* has IRQ? */
- if (ds1307->client->irq > 0 && chip->alarm) {
+ if (client->irq > 0 && chip->alarm) {
INIT_WORK(&ds1307->work, ds1307_work);
want_irq = true;
}
/* get registers that the "rtc" read below won't read... */
- tmp = ds1307->read_block_data(ds1307->client,
+ tmp = ds1307->read_block_data(client,
DS1337_REG_CONTROL, 2, buf);
if (tmp != 2) {
pr_debug("read error %d\n", tmp);
@@ -682,7 +748,7 @@ static int __devinit ds1307_probe(struct
break;
case rx_8025:
- tmp = i2c_smbus_read_i2c_block_data(ds1307->client,
+ tmp = i2c_smbus_read_i2c_block_data(client,
RX8025_REG_CTRL1 << 4 | 0x08, 2, buf);
if (tmp != 2) {
pr_debug("read error %d\n", tmp);
@@ -748,6 +814,14 @@ static int __devinit ds1307_probe(struct
break;
case ds_1388:
ds1307->offset = 1; /* Seconds starts at 1 */
+ ds1307->num_addrs = 3; /* uses 3 I2C addrs */
+ ds1307->nvram_size = 512; /* 512 bytes EEPROM */
+ ds1307->nvram_offset = 0xff;
+ break;
+ case ds_1307:
+ case ds_1338:
+ ds1307->nvram_size = 56; /* 56 bytes NVRAM */
+ ds1307->nvram_offset = 8;
break;
default:
break;
@@ -755,7 +829,7 @@ static int __devinit ds1307_probe(struct
read_rtc:
/* read RTC registers */
- tmp = ds1307->read_block_data(ds1307->client, ds1307->offset, 8, buf);
+ tmp = ds1307->read_block_data(client, ds1307->offset, 8, buf);
if (tmp != 8) {
pr_debug("read error %d\n", tmp);
err = -EIO;
@@ -867,16 +941,34 @@ read_rtc:
dev_dbg(&client->dev, "got IRQ %d\n", client->irq);
}
- if (chip->nvram56) {
+ if (chip->nvram) {
+ nvram.size = ds1307->nvram_size;
err = sysfs_create_bin_file(&client->dev.kobj, &nvram);
if (err == 0) {
set_bit(HAS_NVRAM, &ds1307->flags);
- dev_info(&client->dev, "56 bytes nvram\n");
+ dev_info(&client->dev, "%d bytes nvram\n", nvram.size);
+ }
+ }
+
+ /* use dummy devices for multiple address chips */
+ for (i = 1; i < ds1307->num_addrs; i++) {
+ ds1307->client[i] = i2c_new_dummy(client->adapter,
+ client->addr + i);
+ if (!ds1307->client[i]) {
+ dev_err(&client->dev, "Address %02x unavailable\n",
+ client->addr + i);
+ err = -EBUSY;
+ goto exit_dummy;
}
+ i2c_set_clientdata(ds1307->client[i], ds1307);
}
return 0;
+exit_dummy:
+ for (i = 1; i < ds1307->num_addrs; i++)
+ if (ds1307->client[i])
+ i2c_unregister_device(ds1307->client[i]);
exit_irq:
rtc_device_unregister(ds1307->rtc);
exit_free:
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2.6.39-rc7] RTC: rtc-ds1307 EEPROM support for ds1388 chip
2011-07-31 22:30 ` Austin Boyle
@ 2011-09-23 23:04 ` Andrew Morton
2011-09-24 6:53 ` [rtc-linux] " Wolfram Sang
2011-09-25 10:41 ` Austin Boyle
0 siblings, 2 replies; 9+ messages in thread
From: Andrew Morton @ 2011-09-23 23:04 UTC (permalink / raw)
To: Austin Boyle
Cc: Alessandro Zummo, Joakim Tjernlund, rtc-linux, linux-kernel,
Andrew Morton
On Mon, 1 Aug 2011 10:30:57 +1200
Austin Boyle <Austin.Boyle@Aviatnet.com> wrote:
> [PATCH 3.0] RTC: EEPROM support for DS1388
>
> This patch adds support for different sized NVRAM, such as
> the 512 bytes of EEPROM on the DS1388 chip.
This patch is causing problems in today's linux-next, with x86_64 allmodconfig:
drivers/rtc/rtc-ds1307.c: In function 'ds1307_probe':
drivers/rtc/rtc-ds1307.c:796: warning: passing argument 1 of 'i2c_smbus_read_i2c_block_data' from incompatible pointer type
include/linux/i2c.h:98: note: expected 'const struct i2c_client *' but argument is of type 'struct i2c_client **'
drivers/rtc/rtc-ds1307.c:949: warning: format '%d' expects type 'int', but argument 3 has type 'size_t'
Please take a look? You'll need %z for the printk warning.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [rtc-linux] Re: [PATCH 2.6.39-rc7] RTC: rtc-ds1307 EEPROM support for ds1388 chip
2011-09-23 23:04 ` Andrew Morton
@ 2011-09-24 6:53 ` Wolfram Sang
2011-09-25 10:41 ` Austin Boyle
1 sibling, 0 replies; 9+ messages in thread
From: Wolfram Sang @ 2011-09-24 6:53 UTC (permalink / raw)
To: rtc-linux
Cc: Austin Boyle, Alessandro Zummo, Joakim Tjernlund, linux-kernel,
Andrew Morton
[-- Attachment #1: Type: text/plain, Size: 1082 bytes --]
On Fri, Sep 23, 2011 at 04:04:45PM -0700, Andrew Morton wrote:
> On Mon, 1 Aug 2011 10:30:57 +1200
> Austin Boyle <Austin.Boyle@Aviatnet.com> wrote:
>
> > [PATCH 3.0] RTC: EEPROM support for DS1388
> >
> > This patch adds support for different sized NVRAM, such as
> > the 512 bytes of EEPROM on the DS1388 chip.
>
> This patch is causing problems in today's linux-next, with x86_64 allmodconfig:
Sorry, I missed the patch so far. The whole approach is not correct.
The EEPROM uses a different I2C slave address (0x69+0x6a) than the RTC (0x68).
So, you should be able to use the ds1307-driver on address 0x68 (maybe with
minor modifications, didn't check that) and the eeprom part using the
at24-driver on address 0x69, selecting the 24c02-type. Despite saving redundant
code, at24 handles peculiarities of EEPROMs much better. Can you please report
if that works for you?
Regards,
Wolfram
--
Pengutronix e.K. | Wolfram Sang |
Industrial Linux Solutions | http://www.pengutronix.de/ |
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2.6.39-rc7] RTC: rtc-ds1307 EEPROM support for ds1388 chip
2011-09-23 23:04 ` Andrew Morton
2011-09-24 6:53 ` [rtc-linux] " Wolfram Sang
@ 2011-09-25 10:41 ` Austin Boyle
1 sibling, 0 replies; 9+ messages in thread
From: Austin Boyle @ 2011-09-25 10:41 UTC (permalink / raw)
To: rtc-linux
Cc: Andrew Morton, Wolfram Sang, Alessandro Zummo, Joakim Tjernlund,
linux-kernel, Andrew Morton
I understand that this patch may not be necessary. I will test the
ds1388 EEPROM with the at24-driver as Wolfram suggested.
For completeness, here is the patch applied to the current kernel tree
with the warnings fixed. I have also added the code to unregister the
dummy devices properly in module remove.
---
[PATCH 3.1-rc7] RTC: EEPROM support for DS1388
This patch adds support for different sized NVRAM, such as
the 512 bytes of EEPROM on the DS1388 chip.
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
Signed-off-by: Austin Boyle <austin.boyle@aviatnet.com>
---
--- a/drivers/rtc/rtc-ds1307.c 2011-06-29 11:59:29.275414436 +1200
+++ b/drivers/rtc/rtc-ds1307.c 2011-09-25 22:39:15.598097897 +1300
@@ -95,15 +95,19 @@ enum ds_type {
# define RX8025_BIT_VDET 0x40
# define RX8025_BIT_XST 0x20
+#define DS1307_MAX_NUM_ADDRS 3
struct ds1307 {
u8 offset; /* register's offset */
+ u16 nvram_size;
+ u16 nvram_offset;
+ u8 num_addrs;
u8 regs[11];
enum ds_type type;
unsigned long flags;
#define HAS_NVRAM 0 /* bit 0 == sysfs file active */
#define HAS_ALARM 1 /* bit 1 == irq claimed */
- struct i2c_client *client;
+ struct i2c_client *client[DS1307_MAX_NUM_ADDRS];
struct rtc_device *rtc;
struct work_struct work;
s32 (*read_block_data)(const struct i2c_client *client, u8 command,
@@ -113,23 +117,26 @@ struct ds1307 {
};
struct chip_desc {
- unsigned nvram56:1;
+ unsigned nvram:1;
unsigned alarm:1;
};
static const struct chip_desc chips[] = {
[ds_1307] = {
- .nvram56 = 1,
+ .nvram = 1,
},
[ds_1337] = {
.alarm = 1,
},
[ds_1338] = {
- .nvram56 = 1,
+ .nvram = 1,
},
[ds_1339] = {
.alarm = 1,
},
+[ds_1388] = {
+ .nvram = 1,
+},
[ds_1340] = {
},
[ds_3231] = {
@@ -249,7 +256,7 @@ static void ds1307_work(struct work_stru
int stat, control;
ds1307 = container_of(work, struct ds1307, work);
- client = ds1307->client;
+ client = ds1307->client[0];
lock = &ds1307->rtc->ops_lock;
mutex_lock(lock);
@@ -295,7 +302,7 @@ static int ds1307_get_time(struct device
int tmp;
/* read the RTC date and time registers all at once */
- tmp = ds1307->read_block_data(ds1307->client,
+ tmp = ds1307->read_block_data(ds1307->client[0],
ds1307->offset, 7, ds1307->regs);
if (tmp != 7) {
dev_err(dev, "%s error %d\n", "read", tmp);
@@ -373,7 +380,7 @@ static int ds1307_set_time(struct device
"write", buf[0], buf[1], buf[2], buf[3],
buf[4], buf[5], buf[6]);
- result = ds1307->write_block_data(ds1307->client,
+ result = ds1307->write_block_data(ds1307->client[0],
ds1307->offset, 7, buf);
if (result < 0) {
dev_err(dev, "%s error %d\n", "write", result);
@@ -531,8 +538,6 @@ static const struct rtc_class_ops ds13xx
/*----------------------------------------------------------------------*/
-#define NVRAM_SIZE 56
-
static ssize_t
ds1307_nvram_read(struct file *filp, struct kobject *kobj,
struct bin_attribute *attr,
@@ -541,19 +546,49 @@ ds1307_nvram_read(struct file *filp, str
struct i2c_client *client;
struct ds1307 *ds1307;
int result;
+ unsigned int addr = 0;
client = kobj_to_i2c_client(kobj);
ds1307 = i2c_get_clientdata(client);
- if (unlikely(off >= NVRAM_SIZE))
+ if (unlikely(off >= ds1307->nvram_size))
return 0;
- if ((off + count) > NVRAM_SIZE)
- count = NVRAM_SIZE - off;
+ if ((off + count) > ds1307->nvram_size)
+ count = ds1307->nvram_size - off;
if (unlikely(!count))
return count;
- result = ds1307->read_block_data(client, 8 + off, count, buf);
- if (result < 0)
+ if (ds1307->nvram_offset < 0xff) {
+ result = ds1307->read_block_data(client,
+ ds1307->nvram_offset + off,
+ count, buf);
+ } else {
+ /* read spans two blocks */
+ if ((off <= 0x00ff) && (off + count > 0x00ff)) {
+ client = ds1307->client[1];
+ addr = (unsigned int)off;
+ result = ds1307->read_block_data(client, addr,
+ (0x100-addr), buf);
+ if (result < 0)
+ goto err;
+ client = ds1307->client[2];
+ result = ds1307->read_block_data(client, 0x00,
+ (count+addr-0x100),
+ (buf+0x100-addr));
+ /* read spans one blocks */
+ } else {
+ if (off <= 0x00ff) {
+ client = ds1307->client[1];
+ addr = (unsigned int)off;
+ } else if (off <= 0x0200) {
+ client = ds1307->client[2];
+ addr = (unsigned int)off - 0x100;
+ }
+ result = ds1307->read_block_data(client, addr,
+ count, buf);
+ }
+ }
+err: if (result < 0)
dev_err(&client->dev, "%s error %d\n", "nvram read", result);
return result;
}
@@ -566,19 +601,49 @@ ds1307_nvram_write(struct file *filp, st
struct i2c_client *client;
struct ds1307 *ds1307;
int result;
+ unsigned int addr = 0;
client = kobj_to_i2c_client(kobj);
ds1307 = i2c_get_clientdata(client);
- if (unlikely(off >= NVRAM_SIZE))
+ if (unlikely(off >= ds1307->nvram_size))
return -EFBIG;
- if ((off + count) > NVRAM_SIZE)
- count = NVRAM_SIZE - off;
+ if ((off + count) > ds1307->nvram_size)
+ count = ds1307->nvram_size - off;
if (unlikely(!count))
return count;
- result = ds1307->write_block_data(client, 8 + off, count, buf);
- if (result < 0) {
+ if (ds1307->nvram_offset < 0xff) {
+ result = ds1307->write_block_data(client,
+ ds1307->nvram_offset + off,
+ count, buf);
+ } else {
+ /* read spans two blocks */
+ if ((off <= 0x00ff) && ((off + count) > 0x00ff)) {
+ client = ds1307->client[1];
+ addr = (unsigned int)off;
+ result = ds1307->write_block_data(client, addr,
+ (0x100-addr), buf);
+ if (result < 0)
+ goto err;
+ client = ds1307->client[2];
+ result = ds1307->write_block_data(client, 0x00,
+ (addr+count-0x100),
+ (buf+0x100-addr));
+ /* read spans one blocks */
+ } else {
+ if (off <= 0x00ff) {
+ client = ds1307->client[1];
+ addr = (unsigned int)off;
+ } else if (off <= 0x0200) {
+ client = ds1307->client[2];
+ addr = (unsigned int)off - 0x100;
+ }
+ result = ds1307->write_block_data(client, addr,
+ count, buf);
+ }
+ }
+err: if (result < 0) {
dev_err(&client->dev, "%s error %d\n", "nvram write", result);
return result;
}
@@ -593,7 +658,6 @@ static struct bin_attribute nvram = {
.read = ds1307_nvram_read,
.write = ds1307_nvram_write,
- .size = NVRAM_SIZE,
};
/*----------------------------------------------------------------------*/
@@ -606,6 +670,7 @@ static int __devinit ds1307_probe(struct
struct ds1307 *ds1307;
int err = -ENODEV;
int tmp;
+ int i;
const struct chip_desc *chip = &chips[id->driver_data];
struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent);
int want_irq = false;
@@ -625,9 +690,10 @@ static int __devinit ds1307_probe(struct
i2c_set_clientdata(client, ds1307);
- ds1307->client = client;
+ ds1307->client[0] = client;
ds1307->type = id->driver_data;
ds1307->offset = 0;
+ ds1307->num_addrs = 1;
buf = ds1307->regs;
if (i2c_check_functionality(adapter, I2C_FUNC_SMBUS_I2C_BLOCK)) {
@@ -643,12 +709,12 @@ static int __devinit ds1307_probe(struct
case ds_1339:
case ds_3231:
/* has IRQ? */
- if (ds1307->client->irq > 0 && chip->alarm) {
+ if (client->irq > 0 && chip->alarm) {
INIT_WORK(&ds1307->work, ds1307_work);
want_irq = true;
}
/* get registers that the "rtc" read below won't read... */
- tmp = ds1307->read_block_data(ds1307->client,
+ tmp = ds1307->read_block_data(client,
DS1337_REG_CONTROL, 2, buf);
if (tmp != 2) {
pr_debug("read error %d\n", tmp);
@@ -682,7 +748,7 @@ static int __devinit ds1307_probe(struct
break;
case rx_8025:
- tmp = i2c_smbus_read_i2c_block_data(ds1307->client,
+ tmp = i2c_smbus_read_i2c_block_data(client,
RX8025_REG_CTRL1 << 4 | 0x08, 2, buf);
if (tmp != 2) {
pr_debug("read error %d\n", tmp);
@@ -726,7 +792,7 @@ static int __devinit ds1307_probe(struct
ds1307->regs[0] |
RX8025_BIT_2412);
- tmp = i2c_smbus_read_i2c_block_data(ds1307->client,
+ tmp = i2c_smbus_read_i2c_block_data(client,
RX8025_REG_CTRL1 << 4 | 0x08, 2, buf);
if (tmp != 2) {
pr_debug("read error %d\n", tmp);
@@ -748,6 +814,14 @@ static int __devinit ds1307_probe(struct
break;
case ds_1388:
ds1307->offset = 1; /* Seconds starts at 1 */
+ ds1307->num_addrs = 3; /* uses 3 I2C addrs */
+ ds1307->nvram_size = 512; /* 512 bytes EEPROM */
+ ds1307->nvram_offset = 0xff;
+ break;
+ case ds_1307:
+ case ds_1338:
+ ds1307->nvram_size = 56; /* 56 bytes NVRAM */
+ ds1307->nvram_offset = 8;
break;
default:
break;
@@ -755,7 +829,7 @@ static int __devinit ds1307_probe(struct
read_rtc:
/* read RTC registers */
- tmp = ds1307->read_block_data(ds1307->client, ds1307->offset, 8, buf);
+ tmp = ds1307->read_block_data(client, ds1307->offset, 8, buf);
if (tmp != 8) {
pr_debug("read error %d\n", tmp);
err = -EIO;
@@ -867,16 +941,34 @@ read_rtc:
dev_dbg(&client->dev, "got IRQ %d\n", client->irq);
}
- if (chip->nvram56) {
+ if (chip->nvram) {
+ nvram.size = ds1307->nvram_size;
err = sysfs_create_bin_file(&client->dev.kobj, &nvram);
if (err == 0) {
set_bit(HAS_NVRAM, &ds1307->flags);
- dev_info(&client->dev, "56 bytes nvram\n");
+ dev_info(&client->dev, "%zd bytes nvram\n", nvram.size);
}
}
+ /* use dummy devices for multiple address chips */
+ for (i = 1; i < ds1307->num_addrs; i++) {
+ ds1307->client[i] = i2c_new_dummy(client->adapter,
+ client->addr + i);
+ if (!ds1307->client[i]) {
+ dev_err(&client->dev, "Address %02x unavailable\n",
+ client->addr + i);
+ err = -EBUSY;
+ goto exit_dummy;
+ }
+ i2c_set_clientdata(ds1307->client[i], ds1307);
+ }
+
return 0;
+exit_dummy:
+ for (i = 1; i < ds1307->num_addrs; i++)
+ if (ds1307->client[i])
+ i2c_unregister_device(ds1307->client[i]);
exit_irq:
rtc_device_unregister(ds1307->rtc);
exit_free:
@@ -887,6 +979,7 @@ exit_free:
static int __devexit ds1307_remove(struct i2c_client *client)
{
struct ds1307 *ds1307 = i2c_get_clientdata(client);
+ int i;
if (test_and_clear_bit(HAS_ALARM, &ds1307->flags)) {
free_irq(client->irq, client);
@@ -896,6 +989,10 @@ static int __devexit ds1307_remove(struc
if (test_and_clear_bit(HAS_NVRAM, &ds1307->flags))
sysfs_remove_bin_file(&client->dev.kobj, &nvram);
+ for (i = 1; i < ds1307->num_addrs; i++)
+ if (ds1307->client[i])
+ i2c_unregister_device(ds1307->client[i]);
+
rtc_device_unregister(ds1307->rtc);
kfree(ds1307);
return 0;
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2011-09-25 10:47 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-05-23 5:18 [PATCH 2.6.39-rc7] RTC: rtc-ds1307 EEPROM support for ds1388 chip Austin Boyle
2011-06-14 4:14 ` Austin Boyle
2011-07-17 23:32 ` Austin Boyle
2011-07-22 10:19 ` Joakim Tjernlund
2011-07-22 21:54 ` Andrew Morton
2011-07-31 22:30 ` Austin Boyle
2011-09-23 23:04 ` Andrew Morton
2011-09-24 6:53 ` [rtc-linux] " Wolfram Sang
2011-09-25 10:41 ` Austin Boyle
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox