lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <OFAFFA8CD2.6C655050-ONC12578D5.00387715-C12578D5.0038C1AB@transmode.se>
Date:	Fri, 22 Jul 2011 12:19:55 +0200
From:	Joakim Tjernlund <joakim.tjernlund@...nsmode.se>
To:	Austin Boyle <Austin.Boyle@...atnet.com>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Alessandro Zummo <a.zummo@...ertech.it>,
	David Brownell <dbrownell@...rs.sourceforge.net>,
	linux-kernel@...r.kernel.org, rtc-linux@...glegroups.com,
	Wolfram Sang <w.sang@...gutronix.de>
Subject: Re: [PATCH 2.6.39-rc7] RTC: rtc-ds1307 EEPROM support for ds1388 chip



Austin Boyle <Austin.Boyle@...atnet.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@...ertech.it>
> > Signed-off-by: Austin Boyle <austin.boyle@...atnet.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:
>
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ