diff mbox

fix driver data issues in several rtc drivers

Message ID 20090922223610.1195b9ab@i1501.lan.towertech.it
State Superseded, archived
Headers show

Commit Message

Alessandro Zummo Sept. 22, 2009, 8:36 p.m. UTC
Herton Ronaldo Krzesinski recently raised up, and fixed,
 an issue with the rtc_cmos driver, which was referring
 to an inconsistent driver data.

 I have checked every single driver and fixed similar
 bugs. Driver authors are advised to verify my corrections.
 
 While the bug it's not easy to trigger, it would be fine
 if we can get this fix in the kernel asap. The patch is against
 latest git.

 Signed-off-by: Alessandro Zummo <a.zummo@towertech.it>
 Cc: Hans-Christian Egtvedt <hcegtvedt@atmel.com>
 Cc: David S. Miller <davem@davemloft.net>
 Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
 Cc: Andrew Sharp <andy.sharp@onstor.com>
 Cc: Atsushi Nemoto <anemo@mba.ocn.ne.jp>
 Cc: Alexander Bigga <ab@mycable.de>
 Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
 Cc: Mark Zhan <rongkai.zhan@windriver.com>
 Cc: Thomas Hommel <thomas.hommel@gefanuc.com>

 
 diff --git a/drivers/rtc/rtc-at32ap700x.c b/drivers/rtc/rtc-at32ap700x.c
index e1ec33e..6ccff7f 100644

Comments

Andrew Sharp Sept. 22, 2009, 8:59 p.m. UTC | #1
On Tue, 22 Sep 2009 14:36:10 -0600 Alessandro Zummo
<alessandro.zummo@towertech.it> wrote:

> 
>  Herton Ronaldo Krzesinski recently raised up, and fixed,
>  an issue with the rtc_cmos driver, which was referring
>  to an inconsistent driver data.
> 
>  I have checked every single driver and fixed similar
>  bugs. Driver authors are advised to verify my corrections.
> 
>  While the bug it's not easy to trigger, it would be fine
>  if we can get this fix in the kernel asap. The patch is against
>  latest git.
> 
>  Signed-off-by: Alessandro Zummo <a.zummo@towertech.it>
>  Cc: Hans-Christian Egtvedt <hcegtvedt@atmel.com>
>  Cc: David S. Miller <davem@davemloft.net>
>  Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
>  Cc: Andrew Sharp <andy.sharp@onstor.com>
>  Cc: Atsushi Nemoto <anemo@mba.ocn.ne.jp>
>  Cc: Alexander Bigga <ab@mycable.de>
>  Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
>  Cc: Mark Zhan <rongkai.zhan@windriver.com>
>  Cc: Thomas Hommel <thomas.hommel@gefanuc.com>
> 
> 
>  diff --git a/drivers/rtc/rtc-at32ap700x.c
> b/drivers/rtc/rtc-at32ap700x.c index e1ec33e..6ccff7f 100644
> --- a/drivers/rtc/rtc-at32ap700x.c
> +++ b/drivers/rtc/rtc-at32ap700x.c
> @@ -256,6 +256,8 @@ static int __init at32_rtc_probe(struct
> platform_device *pdev) goto out_iounmap;
>         }
> 
> +       platform_set_drvdata(pdev, rtc);
> +
>         rtc->rtc = rtc_device_register(pdev->name, &pdev->dev,
>                                 &at32_rtc_ops, THIS_MODULE);
>         if (IS_ERR(rtc->rtc)) {
> @@ -264,7 +266,6 @@ static int __init at32_rtc_probe(struct
> platform_device *pdev) goto out_free_irq;
>         }
> 
> -       platform_set_drvdata(pdev, rtc);
>         device_init_wakeup(&pdev->dev, 1);
> 
>         dev_info(&pdev->dev, "Atmel RTC for AT32AP700x at %08lx irq
> %ld\n", diff --git a/drivers/rtc/rtc-bq4802.c
> b/drivers/rtc/rtc-bq4802.c index d00a274..280fe48 100644
> --- a/drivers/rtc/rtc-bq4802.c
> +++ b/drivers/rtc/rtc-bq4802.c
> @@ -169,6 +169,8 @@ static int __devinit bq4802_probe(struct
> platform_device *pdev) goto out_free;
>         }
> 
> +       platform_set_drvdata(pdev, p);
> +
>         p->rtc = rtc_device_register("bq4802", &pdev->dev,
>                                      &bq4802_ops, THIS_MODULE);
>         if (IS_ERR(p->rtc)) {
> @@ -176,7 +178,6 @@ static int __devinit bq4802_probe(struct
> platform_device *pdev) goto out_iounmap;
>         }
> 
> -       platform_set_drvdata(pdev, p);
>         err = 0;
>  out:
>         return err;
> diff --git a/drivers/rtc/rtc-ds1286.c b/drivers/rtc/rtc-ds1286.c
> index 4fcb16b..11d6cc2 100644
> --- a/drivers/rtc/rtc-ds1286.c
> +++ b/drivers/rtc/rtc-ds1286.c
> @@ -325,7 +325,6 @@ static const struct rtc_class_ops ds1286_ops = {
> 
>  static int __devinit ds1286_probe(struct platform_device *pdev)
>  {
> -       struct rtc_device *rtc;
>         struct resource *res;
>         struct ds1286_priv *priv;
>         int ret = 0;
> @@ -349,14 +348,16 @@ static int __devinit ds1286_probe(struct
> platform_device *pdev) goto out;
>         }
>         spin_lock_init(&priv->lock);
> -       rtc = rtc_device_register("ds1286", &pdev->dev,
> +
> +       platform_set_drvdata(pdev, priv);
> +
> +       priv->rtc = rtc_device_register("ds1286", &pdev->dev,
>                                   &ds1286_ops, THIS_MODULE);
> -       if (IS_ERR(rtc)) {
> -               ret = PTR_ERR(rtc);
> +       if (IS_ERR(priv->rtc)) {
> +               ret = PTR_ERR(priv->rtc);
>                 goto out;
>         }
> -       priv->rtc = rtc;
> -       platform_set_drvdata(pdev, priv);
> +
>         return 0;
> 
>  out:
> diff --git a/drivers/rtc/rtc-ds1302.c b/drivers/rtc/rtc-ds1302.c
> index d490628..7c35d14 100644
> --- a/drivers/rtc/rtc-ds1302.c
> +++ b/drivers/rtc/rtc-ds1302.c
> @@ -143,7 +143,6 @@ static int ds1302_rtc_ioctl(struct device *dev,
> unsigned int cmd, #ifdef RTC_SET_CHARGE
>         case RTC_SET_CHARGE:
>         {
> -               struct ds1302_rtc *rtc = dev_get_drvdata(dev);
>                 int tcs_val;
> 
>                 if (copy_from_user(&tcs_val, (int __user *)arg,
> sizeof(int))) diff --git a/drivers/rtc/rtc-ds1305.c
> b/drivers/rtc/rtc-ds1305.c index 8f410e5..c76a194 100644
> --- a/drivers/rtc/rtc-ds1305.c
> +++ b/drivers/rtc/rtc-ds1305.c
> @@ -617,7 +617,6 @@ static struct bin_attribute nvram = {
>  static int __devinit ds1305_probe(struct spi_device *spi)
>  {
>         struct ds1305                   *ds1305;
> -       struct rtc_device               *rtc;
>         int                             status;
>         u8                              addr, value;
>         struct ds1305_platform_data     *pdata =
> spi->dev.platform_data; @@ -756,14 +755,13 @@ static int __devinit
> ds1305_probe(struct spi_device *spi) dev_dbg(&spi->dev, "AM/PM\n");
> 
>         /* register RTC ... from here on, ds1305->ctrl needs locking
> */
> -       rtc = rtc_device_register("ds1305", &spi->dev,
> +       ds1305->rtc = rtc_device_register("ds1305", &spi->dev,
>                         &ds1305_ops, THIS_MODULE);
> -       if (IS_ERR(rtc)) {
> -               status = PTR_ERR(rtc);
> +       if (IS_ERR(ds1305->rtc)) {
> +               status = PTR_ERR(ds1305->rtc);
>                 dev_dbg(&spi->dev, "register rtc --> %d\n", status);
>                 goto fail0;
>         }
> -       ds1305->rtc = rtc;
> 
>         /* Maybe set up alarm IRQ; be ready to handle it triggering
> right
>          * away.  NOTE that we don't share this.  The signal is
> active low, @@ -774,7 +772,7 @@ static int __devinit
> ds1305_probe(struct spi_device *spi) if (spi->irq) {
>                 INIT_WORK(&ds1305->work, ds1305_work);
>                 status = request_irq(spi->irq, ds1305_irq,
> -                               0, dev_name(&rtc->dev), ds1305);
> +                               0, dev_name(&ds1305->rtc->dev),
> ds1305); if (status < 0) {
>                         dev_dbg(&spi->dev, "request_irq %d --> %d\n",
>                                         spi->irq, status);
> @@ -794,7 +792,7 @@ static int __devinit ds1305_probe(struct
> spi_device *spi) fail2:
>         free_irq(spi->irq, ds1305);
>  fail1:
> -       rtc_device_unregister(rtc);
> +       rtc_device_unregister(ds1305->rtc);
>  fail0:
>         kfree(ds1305);
>         return status;
> @@ -802,7 +800,7 @@ fail0:
> 
>  static int __devexit ds1305_remove(struct spi_device *spi)
>  {
> -       struct ds1305   *ds1305 = spi_get_drvdata(spi);
> +       struct ds1305 *ds1305 = spi_get_drvdata(spi);
> 
>         sysfs_remove_bin_file(&spi->dev.kobj, &nvram);
> 
> diff --git a/drivers/rtc/rtc-ds1511.c b/drivers/rtc/rtc-ds1511.c
> index 0b6b773..cfdceaa 100644
> --- a/drivers/rtc/rtc-ds1511.c
> +++ b/drivers/rtc/rtc-ds1511.c
> @@ -490,7 +490,6 @@ static struct bin_attribute ds1511_nvram_attr = {
>   static int __devinit
>  ds1511_rtc_probe(struct platform_device *pdev)
>  {
> -       struct rtc_device *rtc;
>         struct resource *res;
>         struct rtc_plat_data *pdata = NULL;
>         int ret = 0;
> @@ -554,14 +553,15 @@ ds1511_rtc_probe(struct platform_device *pdev)
>                 }
>         }
> 
> -       rtc = rtc_device_register(pdev->name, &pdev->dev,
> &ds1511_rtc_ops,
> +       platform_set_drvdata(pdev, pdata);
> +
> +       pdata->rtc = rtc_device_register(pdev->name, &pdev->dev,
> &ds1511_rtc_ops, THIS_MODULE);
> -       if (IS_ERR(rtc)) {
> -               ret = PTR_ERR(rtc);
> +       if (IS_ERR(pdata->rtc)) {
> +               ret = PTR_ERR(pdata->rtc);
>                 goto out;
>         }
> -       pdata->rtc = rtc;
> -       platform_set_drvdata(pdev, pdata);
> +
>         ret = sysfs_create_bin_file(&pdev->dev.kobj,
> &ds1511_nvram_attr); if (ret) {
>                 goto out;

Acked-by: Andrew Sharp <andy.sharp@lsi.com>

/* note slight email change */

> diff --git a/drivers/rtc/rtc-ds1553.c b/drivers/rtc/rtc-ds1553.c
> index 7172885..f12f087 100644
> --- a/drivers/rtc/rtc-ds1553.c
> +++ b/drivers/rtc/rtc-ds1553.c
> @@ -288,7 +288,6 @@ static struct bin_attribute ds1553_nvram_attr = {
> 
>  static int __devinit ds1553_rtc_probe(struct platform_device *pdev)
>  {
> -       struct rtc_device *rtc;
>         struct resource *res;
>         unsigned int cen, sec;
>         struct rtc_plat_data *pdata = NULL;
> @@ -335,15 +334,17 @@ static int __devinit ds1553_rtc_probe(struct
> platform_device *pdev) }
>         }
> 
> -       rtc = rtc_device_register(pdev->name, &pdev->dev,
> +       platform_set_drvdata(pdev, pdata);
> +
> +       pdata->rtc = rtc_device_register(pdev->name, &pdev->dev,
>                                   &ds1553_rtc_ops, THIS_MODULE);
> -       if (IS_ERR(rtc)) {
> -               ret = PTR_ERR(rtc);
> +       if (IS_ERR(pdata->rtc)) {
> +               ret = PTR_ERR(pdata->rtc);
>                 goto out;
>         }
> -       pdata->rtc = rtc;
> +
>         pdata->last_jiffies = jiffies;
> -       platform_set_drvdata(pdev, pdata);
> +
>         ret = sysfs_create_bin_file(&pdev->dev.kobj,
> &ds1553_nvram_attr); if (ret)
>                 goto out;
> diff --git a/drivers/rtc/rtc-ds1742.c b/drivers/rtc/rtc-ds1742.c
> index 0924945..c9087c1 100644
> --- a/drivers/rtc/rtc-ds1742.c
> +++ b/drivers/rtc/rtc-ds1742.c
> @@ -160,7 +160,6 @@ static ssize_t ds1742_nvram_write(struct kobject
> *kobj,
> 
>  static int __devinit ds1742_rtc_probe(struct platform_device *pdev)
>  {
> -       struct rtc_device *rtc;
>         struct resource *res;
>         unsigned int cen, sec;
>         struct rtc_plat_data *pdata = NULL;
> @@ -207,15 +206,16 @@ static int __devinit ds1742_rtc_probe(struct
> platform_device *pdev) if (!(readb(ioaddr + RTC_DAY) & RTC_BATT_FLAG))
>                 dev_warn(&pdev->dev, "voltage-low detected.\n");
> 
> -       rtc = rtc_device_register(pdev->name, &pdev->dev,
> +       platform_set_drvdata(pdev, pdata);
> +
> +       pdata->rtc = rtc_device_register(pdev->name, &pdev->dev,
>                                   &ds1742_rtc_ops, THIS_MODULE);
> -       if (IS_ERR(rtc)) {
> -               ret = PTR_ERR(rtc);
> +       if (IS_ERR(pdata->rtc)) {
> +               ret = PTR_ERR(pdata->rtc);
>                 goto out;
>         }
> -       pdata->rtc = rtc;
> +
>         pdata->last_jiffies = jiffies;
> -       platform_set_drvdata(pdev, pdata);
> 
>         ret = sysfs_create_bin_file(&pdev->dev.kobj,
> &pdata->nvram_attr); if (ret) {
> diff --git a/drivers/rtc/rtc-m41t80.c b/drivers/rtc/rtc-m41t80.c
> index 60fe266..5033c70 100644
> --- a/drivers/rtc/rtc-m41t80.c
> +++ b/drivers/rtc/rtc-m41t80.c
> @@ -768,37 +768,32 @@ static int m41t80_probe(struct i2c_client
> *client, const struct i2c_device_id *id)
>  {
>         int rc = 0;
> -       struct rtc_device *rtc = NULL;
>         struct rtc_time tm;
>         struct m41t80_data *clientdata = NULL;
> 
>         if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C
>                                      | I2C_FUNC_SMBUS_BYTE_DATA)) {
> -               rc = -ENODEV;
> -               goto exit;
> +               return -ENODEV;
>         }
> 
>         dev_info(&client->dev,
>                  "chip found, driver version " DRV_VERSION "\n");
> 
>         clientdata = kzalloc(sizeof(*clientdata), GFP_KERNEL);
> -       if (!clientdata) {
> -               rc = -ENOMEM;
> -               goto exit;
> -       }
> +       if (!clientdata)
> +               return -ENOMEM;
> +
> +       clientdata->features = id->driver_data;
> +       i2c_set_clientdata(client, clientdata);
> 
> -       rtc = rtc_device_register(client->name, &client->dev,
> +
> +       clientdata->rtc = rtc_device_register(client->name,
> &client->dev, &m41t80_rtc_ops, THIS_MODULE);
> -       if (IS_ERR(rtc)) {
> -               rc = PTR_ERR(rtc);
> -               rtc = NULL;
> +       if (IS_ERR(clientdata->rtc)) {
> +               rc = PTR_ERR(clientdata->rtc);
>                 goto exit;
>         }
> 
> -       clientdata->rtc = rtc;
> -       clientdata->features = id->driver_data;
> -       i2c_set_clientdata(client, clientdata);
> -
>         /* Make sure HT (Halt Update) bit is cleared */
>         rc = i2c_smbus_read_byte_data(client, M41T80_REG_ALARM_HOUR);
>         if (rc < 0)
> @@ -861,8 +856,8 @@ ht_err:
>         goto exit;
> 
>  exit:
> -       if (rtc)
> -               rtc_device_unregister(rtc);
> +       if (clientdata->rtc)
> +               rtc_device_unregister(clientdata->rtc);
>         kfree(clientdata);
>         return rc;
>  }
> diff --git a/drivers/rtc/rtc-m48t35.c b/drivers/rtc/rtc-m48t35.c
> index 0b21975..8cb5b89 100644
> --- a/drivers/rtc/rtc-m48t35.c
> +++ b/drivers/rtc/rtc-m48t35.c
> @@ -142,7 +142,6 @@ static const struct rtc_class_ops m48t35_ops = {
> 
>  static int __devinit m48t35_probe(struct platform_device *pdev)
>  {
> -       struct rtc_device *rtc;
>         struct resource *res;
>         struct m48t35_priv *priv;
>         int ret = 0;
> @@ -171,20 +170,21 @@ static int __devinit m48t35_probe(struct
> platform_device *pdev) ret = -ENOMEM;
>                 goto out;
>         }
> +
>         spin_lock_init(&priv->lock);
> -       rtc = rtc_device_register("m48t35", &pdev->dev,
> +
> +       platform_set_drvdata(pdev, priv);
> +
> +       priv->rtc = rtc_device_register("m48t35", &pdev->dev,
>                                   &m48t35_ops, THIS_MODULE);
> -       if (IS_ERR(rtc)) {
> -               ret = PTR_ERR(rtc);
> +       if (IS_ERR(priv->rtc)) {
> +               ret = PTR_ERR(priv->rtc);
>                 goto out;
>         }
> -       priv->rtc = rtc;
> -       platform_set_drvdata(pdev, priv);
> +
>         return 0;
> 
>  out:
> -       if (priv->rtc)
> -               rtc_device_unregister(priv->rtc);
>         if (priv->reg)
>                 iounmap(priv->reg);
>         if (priv->baseaddr)
> diff --git a/drivers/rtc/rtc-m48t59.c b/drivers/rtc/rtc-m48t59.c
> index 33921a6..ede43b8 100644
> --- a/drivers/rtc/rtc-m48t59.c
> +++ b/drivers/rtc/rtc-m48t59.c
> @@ -481,6 +481,9 @@ static int __devinit m48t59_rtc_probe(struct
> platform_device *pdev) goto out;
>         }
> 
> +       spin_lock_init(&m48t59->lock);
> +       platform_set_drvdata(pdev, m48t59);
> +
>         m48t59->rtc = rtc_device_register(name, &pdev->dev, ops,
> THIS_MODULE); if (IS_ERR(m48t59->rtc)) {
>                 ret = PTR_ERR(m48t59->rtc);
> @@ -490,16 +493,14 @@ static int __devinit m48t59_rtc_probe(struct
> platform_device *pdev) m48t59_nvram_attr.size = pdata->offset;
> 
>         ret = sysfs_create_bin_file(&pdev->dev.kobj,
> &m48t59_nvram_attr);
> -       if (ret)
> +       if (ret) {
> +               rtc_device_unregister(m48t59->rtc);
>                 goto out;
> +       }
> 
> -       spin_lock_init(&m48t59->lock);
> -       platform_set_drvdata(pdev, m48t59);
>         return 0;
> 
>  out:
> -       if (!IS_ERR(m48t59->rtc))
> -               rtc_device_unregister(m48t59->rtc);
>         if (m48t59->irq != NO_IRQ)
>                 free_irq(m48t59->irq, &pdev->dev);
>         if (m48t59->ioaddr)
> diff --git a/drivers/rtc/rtc-pcf8563.c b/drivers/rtc/rtc-pcf8563.c
> index b725913..65f346b 100644
> --- a/drivers/rtc/rtc-pcf8563.c
> +++ b/drivers/rtc/rtc-pcf8563.c
> @@ -212,6 +212,8 @@ static int pcf8563_probe(struct i2c_client
> *client,
> 
>         dev_info(&client->dev, "chip found, driver version "
> DRV_VERSION "\n");
> 
> +       i2c_set_clientdata(client, pcf8563);
> +
>         pcf8563->rtc = rtc_device_register(pcf8563_driver.driver.name,
>                                 &client->dev, &pcf8563_rtc_ops,
> THIS_MODULE);
> 
> @@ -220,8 +222,6 @@ static int pcf8563_probe(struct i2c_client
> *client, goto exit_kfree;
>         }
> 
> -       i2c_set_clientdata(client, pcf8563);
> -
>         return 0;
> 
>  exit_kfree:
> diff --git a/drivers/rtc/rtc-pcf8583.c b/drivers/rtc/rtc-pcf8583.c
> index 7d33cda..2d201af 100644
> --- a/drivers/rtc/rtc-pcf8583.c
> +++ b/drivers/rtc/rtc-pcf8583.c
> @@ -277,6 +277,8 @@ static int pcf8583_probe(struct i2c_client
> *client, if (!pcf8583)
>                 return -ENOMEM;
> 
> +       i2c_set_clientdata(client, pcf8583);
> +
>         pcf8583->rtc = rtc_device_register(pcf8583_driver.driver.name,
>                         &client->dev, &pcf8583_rtc_ops, THIS_MODULE);
> 
> @@ -285,7 +287,6 @@ static int pcf8583_probe(struct i2c_client
> *client, goto exit_kfree;
>         }
> 
> -       i2c_set_clientdata(client, pcf8583);
>         return 0;
> 
>  exit_kfree:
> diff --git a/drivers/rtc/rtc-stk17ta8.c b/drivers/rtc/rtc-stk17ta8.c
> index 7d1547b..78d9d07 100644
> --- a/drivers/rtc/rtc-stk17ta8.c
> +++ b/drivers/rtc/rtc-stk17ta8.c
> @@ -288,7 +288,6 @@ static struct bin_attribute stk17ta8_nvram_attr =
> {
> 
>  static int __init stk17ta8_rtc_probe(struct platform_device *pdev)
>  {
> -       struct rtc_device *rtc;
>         struct resource *res;
>         unsigned int cal;
>         unsigned int flags;
> @@ -338,22 +337,23 @@ static int __init stk17ta8_rtc_probe(struct
> platform_device *pdev) }
>         }
> 
> -       rtc = rtc_device_register(pdev->name, &pdev->dev,
> +       pdata->last_jiffies = jiffies;
> +       platform_set_drvdata(pdev, pdata);
> +
> +       pdata->rtc = rtc_device_register(pdev->name, &pdev->dev,
>                                   &stk17ta8_rtc_ops, THIS_MODULE);
> -       if (IS_ERR(rtc)) {
> -               ret = PTR_ERR(rtc);
> +       if (IS_ERR(pdata->rtc)) {
> +               ret = PTR_ERR(pdata->rtc);
>                 goto out;
>         }
> -       pdata->rtc = rtc;
> -       pdata->last_jiffies = jiffies;
> -       platform_set_drvdata(pdev, pdata);
> +
>         ret = sysfs_create_bin_file(&pdev->dev.kobj,
> &stk17ta8_nvram_attr);
> -       if (ret)
> +       if (ret) {
> +               rtc_device_unregister(pdata->rtc);
>                 goto out;
> +       }
>         return 0;
>   out:
> -       if (pdata->rtc)
> -               rtc_device_unregister(pdata->rtc);
>         if (pdata->irq > 0)
>                 free_irq(pdata->irq, pdev);
>         if (ioaddr)
> diff --git a/drivers/rtc/rtc-tx4939.c b/drivers/rtc/rtc-tx4939.c
> index 4a6ed11..d69775c 100644
> --- a/drivers/rtc/rtc-tx4939.c
> +++ b/drivers/rtc/rtc-tx4939.c
> @@ -235,7 +235,6 @@ static struct bin_attribute tx4939_rtc_nvram_attr
> = {
> 
>  static int __init tx4939_rtc_probe(struct platform_device *pdev)
>  {
> -       struct rtc_device *rtc;
>         struct tx4939rtc_plat_data *pdata;
>         struct resource *res;
>         int irq, ret;
> @@ -263,14 +262,15 @@ static int __init tx4939_rtc_probe(struct
> platform_device *pdev) if (devm_request_irq(&pdev->dev, irq,
> tx4939_rtc_interrupt, IRQF_DISABLED, pdev->name, &pdev->dev) < 0)
>                 return -EBUSY;
> -       rtc = rtc_device_register(pdev->name, &pdev->dev,
> +       pdata->rtc = rtc_device_register(pdev->name, &pdev->dev,
>                                   &tx4939_rtc_ops, THIS_MODULE);
> -       if (IS_ERR(rtc))
> -               return PTR_ERR(rtc);
> -       pdata->rtc = rtc;
> +       if (IS_ERR(pdata->rtc))
> +               return PTR_ERR(pdata->rtc);
> +
>         ret = sysfs_create_bin_file(&pdev->dev.kobj,
> &tx4939_rtc_nvram_attr); if (ret)
> -               rtc_device_unregister(rtc);
> +               rtc_device_unregister(pdata->rtc);
> +
>         return ret;
>  }
> 
> diff --git a/drivers/rtc/rtc-v3020.c b/drivers/rtc/rtc-v3020.c
> index ad16405..6c00f75 100644
> --- a/drivers/rtc/rtc-v3020.c
> +++ b/drivers/rtc/rtc-v3020.c
> @@ -304,7 +304,6 @@ static int rtc_probe(struct platform_device *pdev)
>  {
>         struct v3020_platform_data *pdata = pdev->dev.platform_data;
>         struct v3020 *chip;
> -       struct rtc_device *rtc;
>         int retval = -EBUSY;
>         int i;
>         int temp;
> @@ -353,13 +352,12 @@ static int rtc_probe(struct platform_device
> *pdev)
> 
>         platform_set_drvdata(pdev, chip);
> 
> -       rtc = rtc_device_register("v3020",
> +       chip->rtc = rtc_device_register("v3020",
>                                 &pdev->dev, &v3020_rtc_ops,
> THIS_MODULE);
> -       if (IS_ERR(rtc)) {
> -               retval = PTR_ERR(rtc);
> +       if (IS_ERR(chip->rtc)) {
> +               retval = PTR_ERR(chip->rtc);
>                 goto err_io;
>         }
> -       chip->rtc = rtc;
> 
>         return 0;

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
-~----------~----~----~----~------~----~------~--~---
Atsushi Nemoto Sept. 24, 2009, 2:01 p.m. UTC | #2
On Tue, 22 Sep 2009 22:36:10 +0200, Alessandro Zummo <alessandro.zummo@towertech.it> wrote:
>  Herton Ronaldo Krzesinski recently raised up, and fixed,
>  an issue with the rtc_cmos driver, which was referring
>  to an inconsistent driver data.
> 
>  I have checked every single driver and fixed similar
>  bugs. Driver authors are advised to verify my corrections.
>  
>  While the bug it's not easy to trigger, it would be fine
>  if we can get this fix in the kernel asap. The patch is against
>  latest git.
> 
>  Signed-off-by: Alessandro Zummo <a.zummo@towertech.it>

Thank you.  I verified some drivers: rtc-ds1553, rtc-ds1742,
rtc-m41t80, rtc-tx4939.

> diff --git a/drivers/rtc/rtc-ds1553.c b/drivers/rtc/rtc-ds1553.c
> index 7172885..f12f087 100644
> --- a/drivers/rtc/rtc-ds1553.c
> +++ b/drivers/rtc/rtc-ds1553.c
> @@ -288,7 +288,6 @@ static struct bin_attribute ds1553_nvram_attr = {
>  
>  static int __devinit ds1553_rtc_probe(struct platform_device *pdev)
>  {
> -	struct rtc_device *rtc;
>  	struct resource *res;
>  	unsigned int cen, sec;
>  	struct rtc_plat_data *pdata = NULL;
> @@ -335,15 +334,17 @@ static int __devinit ds1553_rtc_probe(struct platform_device *pdev)
>  		}
>  	}
>  
> -	rtc = rtc_device_register(pdev->name, &pdev->dev,
> +	platform_set_drvdata(pdev, pdata);
> +
> +	pdata->rtc = rtc_device_register(pdev->name, &pdev->dev,
>  				  &ds1553_rtc_ops, THIS_MODULE);
> -	if (IS_ERR(rtc)) {
> -		ret = PTR_ERR(rtc);
> +	if (IS_ERR(pdata->rtc)) {
> +		ret = PTR_ERR(pdata->rtc);
>  		goto out;
>  	}
> -	pdata->rtc = rtc;
> +
>  	pdata->last_jiffies = jiffies;
> -	platform_set_drvdata(pdev, pdata);
> +
>  	ret = sysfs_create_bin_file(&pdev->dev.kobj, &ds1553_nvram_attr);
>  	if (ret)
>  		goto out;

If rtc_device_register() failed, you should set NULL to pdata->rtc
before the 'goto', otherwise rtc_device_unregister() will be called
with an invalid pointer.

It seems there are other problems in this driver.  The last_jiffies
should be initialized before rtc_device_register, and the interrupt
handler should be check pdata->rtc validity.

I will fix these issues so please drop this part for now.

> diff --git a/drivers/rtc/rtc-ds1742.c b/drivers/rtc/rtc-ds1742.c
> index 0924945..c9087c1 100644
> --- a/drivers/rtc/rtc-ds1742.c
> +++ b/drivers/rtc/rtc-ds1742.c
> @@ -160,7 +160,6 @@ static ssize_t ds1742_nvram_write(struct kobject *kobj,
>  
>  static int __devinit ds1742_rtc_probe(struct platform_device *pdev)
>  {
> -	struct rtc_device *rtc;
>  	struct resource *res;
>  	unsigned int cen, sec;
>  	struct rtc_plat_data *pdata = NULL;
> @@ -207,15 +206,16 @@ static int __devinit ds1742_rtc_probe(struct platform_device *pdev)
>  	if (!(readb(ioaddr + RTC_DAY) & RTC_BATT_FLAG))
>  		dev_warn(&pdev->dev, "voltage-low detected.\n");
>  
> -	rtc = rtc_device_register(pdev->name, &pdev->dev,
> +	platform_set_drvdata(pdev, pdata);
> +
> +	pdata->rtc = rtc_device_register(pdev->name, &pdev->dev,
>  				  &ds1742_rtc_ops, THIS_MODULE);
> -	if (IS_ERR(rtc)) {
> -		ret = PTR_ERR(rtc);
> +	if (IS_ERR(pdata->rtc)) {
> +		ret = PTR_ERR(pdata->rtc);
>  		goto out;
>  	}
> -	pdata->rtc = rtc;
> +
>  	pdata->last_jiffies = jiffies;
> -	platform_set_drvdata(pdev, pdata);
>  
>  	ret = sysfs_create_bin_file(&pdev->dev.kobj, &pdata->nvram_attr);
>  	if (ret) {

Ditto.  Please drop this part for now.

> diff --git a/drivers/rtc/rtc-m41t80.c b/drivers/rtc/rtc-m41t80.c
> index 60fe266..5033c70 100644
> --- a/drivers/rtc/rtc-m41t80.c
> +++ b/drivers/rtc/rtc-m41t80.c
> @@ -768,37 +768,32 @@ static int m41t80_probe(struct i2c_client *client,
>  			const struct i2c_device_id *id)
>  {
>  	int rc = 0;
> -	struct rtc_device *rtc = NULL;
>  	struct rtc_time tm;
>  	struct m41t80_data *clientdata = NULL;
>  
>  	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C
>  				     | I2C_FUNC_SMBUS_BYTE_DATA)) {
> -		rc = -ENODEV;
> -		goto exit;
> +		return -ENODEV;
>  	}
>  
>  	dev_info(&client->dev,
>  		 "chip found, driver version " DRV_VERSION "\n");
>  
>  	clientdata = kzalloc(sizeof(*clientdata), GFP_KERNEL);
> -	if (!clientdata) {
> -		rc = -ENOMEM;
> -		goto exit;
> -	}
> +	if (!clientdata)
> +		return -ENOMEM;
> +
> +	clientdata->features = id->driver_data;
> +	i2c_set_clientdata(client, clientdata);
>  
> -	rtc = rtc_device_register(client->name, &client->dev,
> +
> +	clientdata->rtc = rtc_device_register(client->name, &client->dev,
>  				  &m41t80_rtc_ops, THIS_MODULE);
> -	if (IS_ERR(rtc)) {
> -		rc = PTR_ERR(rtc);
> -		rtc = NULL;
> +	if (IS_ERR(clientdata->rtc)) {
> +		rc = PTR_ERR(clientdata->rtc);
>  		goto exit;
>  	}
>  
> -	clientdata->rtc = rtc;
> -	clientdata->features = id->driver_data;
> -	i2c_set_clientdata(client, clientdata);
> -
>  	/* Make sure HT (Halt Update) bit is cleared */
>  	rc = i2c_smbus_read_byte_data(client, M41T80_REG_ALARM_HOUR);
>  	if (rc < 0)
> @@ -861,8 +856,8 @@ ht_err:
>  	goto exit;
>  
>  exit:
> -	if (rtc)
> -		rtc_device_unregister(rtc);
> +	if (clientdata->rtc)
> +		rtc_device_unregister(clientdata->rtc);
>  	kfree(clientdata);
>  	return rc;
>  }

If rtc_device_register() failed, you should set NULL to pdata->rtc
before the 'goto'.

> diff --git a/drivers/rtc/rtc-tx4939.c b/drivers/rtc/rtc-tx4939.c
> index 4a6ed11..d69775c 100644
> --- a/drivers/rtc/rtc-tx4939.c
> +++ b/drivers/rtc/rtc-tx4939.c
> @@ -235,7 +235,6 @@ static struct bin_attribute tx4939_rtc_nvram_attr = {
>  
>  static int __init tx4939_rtc_probe(struct platform_device *pdev)
>  {
> -	struct rtc_device *rtc;
>  	struct tx4939rtc_plat_data *pdata;
>  	struct resource *res;
>  	int irq, ret;
> @@ -263,14 +262,15 @@ static int __init tx4939_rtc_probe(struct platform_device *pdev)
>  	if (devm_request_irq(&pdev->dev, irq, tx4939_rtc_interrupt,
>  			     IRQF_DISABLED, pdev->name, &pdev->dev) < 0)
>  		return -EBUSY;
> -	rtc = rtc_device_register(pdev->name, &pdev->dev,
> +	pdata->rtc = rtc_device_register(pdev->name, &pdev->dev,
>  				  &tx4939_rtc_ops, THIS_MODULE);
> -	if (IS_ERR(rtc))
> -		return PTR_ERR(rtc);
> -	pdata->rtc = rtc;
> +	if (IS_ERR(pdata->rtc))
> +		return PTR_ERR(pdata->rtc);
> +
>  	ret = sysfs_create_bin_file(&pdev->dev.kobj, &tx4939_rtc_nvram_attr);
>  	if (ret)
> -		rtc_device_unregister(rtc);
> +		rtc_device_unregister(pdata->rtc);
> +
>  	return ret;
>  }
>  

This is just a cleanup, not fix.  So the commit log is not appropriate
for this driver.  But I do not mind if this part was in the patch for
cosmetic consistency.

---
Atsushi Nemoto

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
-~----------~----~----~----~------~----~------~--~---
Andrew Morton Sept. 30, 2009, 7:15 p.m. UTC | #3
On Thu, 24 Sep 2009 23:01:58 +0900 (JST)
Atsushi Nemoto <anemo@mba.ocn.ne.jp> wrote:

> On Tue, 22 Sep 2009 22:36:10 +0200, Alessandro Zummo <alessandro.zummo@towertech.it> wrote:
> >  Herton Ronaldo Krzesinski recently raised up, and fixed,
> >  an issue with the rtc_cmos driver, which was referring
> >  to an inconsistent driver data.
> > 
> >  I have checked every single driver and fixed similar
> >  bugs. Driver authors are advised to verify my corrections.
> >  
> >  While the bug it's not easy to trigger, it would be fine
> >  if we can get this fix in the kernel asap. The patch is against
> >  latest git.

The changelog doesn't describe what's actually being fixed.

> If rtc_device_register() failed, you should set NULL to pdata->rtc
> before the 'goto', otherwise rtc_device_unregister() will be called
> with an invalid pointer.

OK, a few changes are needed.

Alessandro, please send an updated patch sometime?  Please incorporate
Hans-Christian's rtc-at32ap700x.c update also, thanks.



--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
-~----------~----~----~----~------~----~------~--~---
Alessandro Zummo Oct. 1, 2009, 12:36 p.m. UTC | #4
On Wed, 30 Sep 2009 12:15:25 -0700
Andrew Morton <akpm@linux-foundation.org> wrote:

> Alessandro, please send an updated patch sometime?  Please incorporate
> Hans-Christian's rtc-at32ap700x.c update also, thanks.
> 

 Will send asap.
diff mbox

Patch

--- a/drivers/rtc/rtc-at32ap700x.c
+++ b/drivers/rtc/rtc-at32ap700x.c
@@ -256,6 +256,8 @@  static int __init at32_rtc_probe(struct platform_device *pdev)
 		goto out_iounmap;
 	}
 
+	platform_set_drvdata(pdev, rtc);
+
 	rtc->rtc = rtc_device_register(pdev->name, &pdev->dev,
 				&at32_rtc_ops, THIS_MODULE);
 	if (IS_ERR(rtc->rtc)) {
@@ -264,7 +266,6 @@  static int __init at32_rtc_probe(struct platform_device *pdev)
 		goto out_free_irq;
 	}
 
-	platform_set_drvdata(pdev, rtc);
 	device_init_wakeup(&pdev->dev, 1);
 
 	dev_info(&pdev->dev, "Atmel RTC for AT32AP700x at %08lx irq %ld\n",
diff --git a/drivers/rtc/rtc-bq4802.c b/drivers/rtc/rtc-bq4802.c
index d00a274..280fe48 100644
--- a/drivers/rtc/rtc-bq4802.c
+++ b/drivers/rtc/rtc-bq4802.c
@@ -169,6 +169,8 @@  static int __devinit bq4802_probe(struct platform_device *pdev)
 		goto out_free;
 	}
 
+	platform_set_drvdata(pdev, p);
+
 	p->rtc = rtc_device_register("bq4802", &pdev->dev,
 				     &bq4802_ops, THIS_MODULE);
 	if (IS_ERR(p->rtc)) {
@@ -176,7 +178,6 @@  static int __devinit bq4802_probe(struct platform_device *pdev)
 		goto out_iounmap;
 	}
 
-	platform_set_drvdata(pdev, p);
 	err = 0;
 out:
 	return err;
diff --git a/drivers/rtc/rtc-ds1286.c b/drivers/rtc/rtc-ds1286.c
index 4fcb16b..11d6cc2 100644
--- a/drivers/rtc/rtc-ds1286.c
+++ b/drivers/rtc/rtc-ds1286.c
@@ -325,7 +325,6 @@  static const struct rtc_class_ops ds1286_ops = {
 
 static int __devinit ds1286_probe(struct platform_device *pdev)
 {
-	struct rtc_device *rtc;
 	struct resource *res;
 	struct ds1286_priv *priv;
 	int ret = 0;
@@ -349,14 +348,16 @@  static int __devinit ds1286_probe(struct platform_device *pdev)
 		goto out;
 	}
 	spin_lock_init(&priv->lock);
-	rtc = rtc_device_register("ds1286", &pdev->dev,
+
+	platform_set_drvdata(pdev, priv);
+
+	priv->rtc = rtc_device_register("ds1286", &pdev->dev,
 				  &ds1286_ops, THIS_MODULE);
-	if (IS_ERR(rtc)) {
-		ret = PTR_ERR(rtc);
+	if (IS_ERR(priv->rtc)) {
+		ret = PTR_ERR(priv->rtc);
 		goto out;
 	}
-	priv->rtc = rtc;
-	platform_set_drvdata(pdev, priv);
+
 	return 0;
 
 out:
diff --git a/drivers/rtc/rtc-ds1302.c b/drivers/rtc/rtc-ds1302.c
index d490628..7c35d14 100644
--- a/drivers/rtc/rtc-ds1302.c
+++ b/drivers/rtc/rtc-ds1302.c
@@ -143,7 +143,6 @@  static int ds1302_rtc_ioctl(struct device *dev, unsigned int cmd,
 #ifdef RTC_SET_CHARGE
 	case RTC_SET_CHARGE:
 	{
-		struct ds1302_rtc *rtc = dev_get_drvdata(dev);
 		int tcs_val;
 
 		if (copy_from_user(&tcs_val, (int __user *)arg, sizeof(int)))
diff --git a/drivers/rtc/rtc-ds1305.c b/drivers/rtc/rtc-ds1305.c
index 8f410e5..c76a194 100644
--- a/drivers/rtc/rtc-ds1305.c
+++ b/drivers/rtc/rtc-ds1305.c
@@ -617,7 +617,6 @@  static struct bin_attribute nvram = {
 static int __devinit ds1305_probe(struct spi_device *spi)
 {
 	struct ds1305			*ds1305;
-	struct rtc_device		*rtc;
 	int				status;
 	u8				addr, value;
 	struct ds1305_platform_data	*pdata = spi->dev.platform_data;
@@ -756,14 +755,13 @@  static int __devinit ds1305_probe(struct spi_device *spi)
 		dev_dbg(&spi->dev, "AM/PM\n");
 
 	/* register RTC ... from here on, ds1305->ctrl needs locking */
-	rtc = rtc_device_register("ds1305", &spi->dev,
+	ds1305->rtc = rtc_device_register("ds1305", &spi->dev,
 			&ds1305_ops, THIS_MODULE);
-	if (IS_ERR(rtc)) {
-		status = PTR_ERR(rtc);
+	if (IS_ERR(ds1305->rtc)) {
+		status = PTR_ERR(ds1305->rtc);
 		dev_dbg(&spi->dev, "register rtc --> %d\n", status);
 		goto fail0;
 	}
-	ds1305->rtc = rtc;
 
 	/* Maybe set up alarm IRQ; be ready to handle it triggering right
 	 * away.  NOTE that we don't share this.  The signal is active low,
@@ -774,7 +772,7 @@  static int __devinit ds1305_probe(struct spi_device *spi)
 	if (spi->irq) {
 		INIT_WORK(&ds1305->work, ds1305_work);
 		status = request_irq(spi->irq, ds1305_irq,
-				0, dev_name(&rtc->dev), ds1305);
+				0, dev_name(&ds1305->rtc->dev), ds1305);
 		if (status < 0) {
 			dev_dbg(&spi->dev, "request_irq %d --> %d\n",
 					spi->irq, status);
@@ -794,7 +792,7 @@  static int __devinit ds1305_probe(struct spi_device *spi)
 fail2:
 	free_irq(spi->irq, ds1305);
 fail1:
-	rtc_device_unregister(rtc);
+	rtc_device_unregister(ds1305->rtc);
 fail0:
 	kfree(ds1305);
 	return status;
@@ -802,7 +800,7 @@  fail0:
 
 static int __devexit ds1305_remove(struct spi_device *spi)
 {
-	struct ds1305	*ds1305 = spi_get_drvdata(spi);
+	struct ds1305 *ds1305 = spi_get_drvdata(spi);
 
 	sysfs_remove_bin_file(&spi->dev.kobj, &nvram);
 
diff --git a/drivers/rtc/rtc-ds1511.c b/drivers/rtc/rtc-ds1511.c
index 0b6b773..cfdceaa 100644
--- a/drivers/rtc/rtc-ds1511.c
+++ b/drivers/rtc/rtc-ds1511.c
@@ -490,7 +490,6 @@  static struct bin_attribute ds1511_nvram_attr = {
  static int __devinit
 ds1511_rtc_probe(struct platform_device *pdev)
 {
-	struct rtc_device *rtc;
 	struct resource *res;
 	struct rtc_plat_data *pdata = NULL;
 	int ret = 0;
@@ -554,14 +553,15 @@  ds1511_rtc_probe(struct platform_device *pdev)
 		}
 	}
 
-	rtc = rtc_device_register(pdev->name, &pdev->dev, &ds1511_rtc_ops,
+	platform_set_drvdata(pdev, pdata);
+
+	pdata->rtc = rtc_device_register(pdev->name, &pdev->dev, &ds1511_rtc_ops,
 		THIS_MODULE);
-	if (IS_ERR(rtc)) {
-		ret = PTR_ERR(rtc);
+	if (IS_ERR(pdata->rtc)) {
+		ret = PTR_ERR(pdata->rtc);
 		goto out;
 	}
-	pdata->rtc = rtc;
-	platform_set_drvdata(pdev, pdata);
+
 	ret = sysfs_create_bin_file(&pdev->dev.kobj, &ds1511_nvram_attr);
 	if (ret) {
 		goto out;
diff --git a/drivers/rtc/rtc-ds1553.c b/drivers/rtc/rtc-ds1553.c
index 7172885..f12f087 100644
--- a/drivers/rtc/rtc-ds1553.c
+++ b/drivers/rtc/rtc-ds1553.c
@@ -288,7 +288,6 @@  static struct bin_attribute ds1553_nvram_attr = {
 
 static int __devinit ds1553_rtc_probe(struct platform_device *pdev)
 {
-	struct rtc_device *rtc;
 	struct resource *res;
 	unsigned int cen, sec;
 	struct rtc_plat_data *pdata = NULL;
@@ -335,15 +334,17 @@  static int __devinit ds1553_rtc_probe(struct platform_device *pdev)
 		}
 	}
 
-	rtc = rtc_device_register(pdev->name, &pdev->dev,
+	platform_set_drvdata(pdev, pdata);
+
+	pdata->rtc = rtc_device_register(pdev->name, &pdev->dev,
 				  &ds1553_rtc_ops, THIS_MODULE);
-	if (IS_ERR(rtc)) {
-		ret = PTR_ERR(rtc);
+	if (IS_ERR(pdata->rtc)) {
+		ret = PTR_ERR(pdata->rtc);
 		goto out;
 	}
-	pdata->rtc = rtc;
+
 	pdata->last_jiffies = jiffies;
-	platform_set_drvdata(pdev, pdata);
+
 	ret = sysfs_create_bin_file(&pdev->dev.kobj, &ds1553_nvram_attr);
 	if (ret)
 		goto out;
diff --git a/drivers/rtc/rtc-ds1742.c b/drivers/rtc/rtc-ds1742.c
index 0924945..c9087c1 100644
--- a/drivers/rtc/rtc-ds1742.c
+++ b/drivers/rtc/rtc-ds1742.c
@@ -160,7 +160,6 @@  static ssize_t ds1742_nvram_write(struct kobject *kobj,
 
 static int __devinit ds1742_rtc_probe(struct platform_device *pdev)
 {
-	struct rtc_device *rtc;
 	struct resource *res;
 	unsigned int cen, sec;
 	struct rtc_plat_data *pdata = NULL;
@@ -207,15 +206,16 @@  static int __devinit ds1742_rtc_probe(struct platform_device *pdev)
 	if (!(readb(ioaddr + RTC_DAY) & RTC_BATT_FLAG))
 		dev_warn(&pdev->dev, "voltage-low detected.\n");
 
-	rtc = rtc_device_register(pdev->name, &pdev->dev,
+	platform_set_drvdata(pdev, pdata);
+
+	pdata->rtc = rtc_device_register(pdev->name, &pdev->dev,
 				  &ds1742_rtc_ops, THIS_MODULE);
-	if (IS_ERR(rtc)) {
-		ret = PTR_ERR(rtc);
+	if (IS_ERR(pdata->rtc)) {
+		ret = PTR_ERR(pdata->rtc);
 		goto out;
 	}
-	pdata->rtc = rtc;
+
 	pdata->last_jiffies = jiffies;
-	platform_set_drvdata(pdev, pdata);
 
 	ret = sysfs_create_bin_file(&pdev->dev.kobj, &pdata->nvram_attr);
 	if (ret) {
diff --git a/drivers/rtc/rtc-m41t80.c b/drivers/rtc/rtc-m41t80.c
index 60fe266..5033c70 100644
--- a/drivers/rtc/rtc-m41t80.c
+++ b/drivers/rtc/rtc-m41t80.c
@@ -768,37 +768,32 @@  static int m41t80_probe(struct i2c_client *client,
 			const struct i2c_device_id *id)
 {
 	int rc = 0;
-	struct rtc_device *rtc = NULL;
 	struct rtc_time tm;
 	struct m41t80_data *clientdata = NULL;
 
 	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C
 				     | I2C_FUNC_SMBUS_BYTE_DATA)) {
-		rc = -ENODEV;
-		goto exit;
+		return -ENODEV;
 	}
 
 	dev_info(&client->dev,
 		 "chip found, driver version " DRV_VERSION "\n");
 
 	clientdata = kzalloc(sizeof(*clientdata), GFP_KERNEL);
-	if (!clientdata) {
-		rc = -ENOMEM;
-		goto exit;
-	}
+	if (!clientdata)
+		return -ENOMEM;
+
+	clientdata->features = id->driver_data;
+	i2c_set_clientdata(client, clientdata);
 
-	rtc = rtc_device_register(client->name, &client->dev,
+
+	clientdata->rtc = rtc_device_register(client->name, &client->dev,
 				  &m41t80_rtc_ops, THIS_MODULE);
-	if (IS_ERR(rtc)) {
-		rc = PTR_ERR(rtc);
-		rtc = NULL;
+	if (IS_ERR(clientdata->rtc)) {
+		rc = PTR_ERR(clientdata->rtc);
 		goto exit;
 	}
 
-	clientdata->rtc = rtc;
-	clientdata->features = id->driver_data;
-	i2c_set_clientdata(client, clientdata);
-
 	/* Make sure HT (Halt Update) bit is cleared */
 	rc = i2c_smbus_read_byte_data(client, M41T80_REG_ALARM_HOUR);
 	if (rc < 0)
@@ -861,8 +856,8 @@  ht_err:
 	goto exit;
 
 exit:
-	if (rtc)
-		rtc_device_unregister(rtc);
+	if (clientdata->rtc)
+		rtc_device_unregister(clientdata->rtc);
 	kfree(clientdata);
 	return rc;
 }
diff --git a/drivers/rtc/rtc-m48t35.c b/drivers/rtc/rtc-m48t35.c
index 0b21975..8cb5b89 100644
--- a/drivers/rtc/rtc-m48t35.c
+++ b/drivers/rtc/rtc-m48t35.c
@@ -142,7 +142,6 @@  static const struct rtc_class_ops m48t35_ops = {
 
 static int __devinit m48t35_probe(struct platform_device *pdev)
 {
-	struct rtc_device *rtc;
 	struct resource *res;
 	struct m48t35_priv *priv;
 	int ret = 0;
@@ -171,20 +170,21 @@  static int __devinit m48t35_probe(struct platform_device *pdev)
 		ret = -ENOMEM;
 		goto out;
 	}
+
 	spin_lock_init(&priv->lock);
-	rtc = rtc_device_register("m48t35", &pdev->dev,
+
+	platform_set_drvdata(pdev, priv);
+
+	priv->rtc = rtc_device_register("m48t35", &pdev->dev,
 				  &m48t35_ops, THIS_MODULE);
-	if (IS_ERR(rtc)) {
-		ret = PTR_ERR(rtc);
+	if (IS_ERR(priv->rtc)) {
+		ret = PTR_ERR(priv->rtc);
 		goto out;
 	}
-	priv->rtc = rtc;
-	platform_set_drvdata(pdev, priv);
+
 	return 0;
 
 out:
-	if (priv->rtc)
-		rtc_device_unregister(priv->rtc);
 	if (priv->reg)
 		iounmap(priv->reg);
 	if (priv->baseaddr)
diff --git a/drivers/rtc/rtc-m48t59.c b/drivers/rtc/rtc-m48t59.c
index 33921a6..ede43b8 100644
--- a/drivers/rtc/rtc-m48t59.c
+++ b/drivers/rtc/rtc-m48t59.c
@@ -481,6 +481,9 @@  static int __devinit m48t59_rtc_probe(struct platform_device *pdev)
 		goto out;
 	}
 
+	spin_lock_init(&m48t59->lock);
+	platform_set_drvdata(pdev, m48t59);
+
 	m48t59->rtc = rtc_device_register(name, &pdev->dev, ops, THIS_MODULE);
 	if (IS_ERR(m48t59->rtc)) {
 		ret = PTR_ERR(m48t59->rtc);
@@ -490,16 +493,14 @@  static int __devinit m48t59_rtc_probe(struct platform_device *pdev)
 	m48t59_nvram_attr.size = pdata->offset;
 
 	ret = sysfs_create_bin_file(&pdev->dev.kobj, &m48t59_nvram_attr);
-	if (ret)
+	if (ret) {
+		rtc_device_unregister(m48t59->rtc);
 		goto out;
+	}
 
-	spin_lock_init(&m48t59->lock);
-	platform_set_drvdata(pdev, m48t59);
 	return 0;
 
 out:
-	if (!IS_ERR(m48t59->rtc))
-		rtc_device_unregister(m48t59->rtc);
 	if (m48t59->irq != NO_IRQ)
 		free_irq(m48t59->irq, &pdev->dev);
 	if (m48t59->ioaddr)
diff --git a/drivers/rtc/rtc-pcf8563.c b/drivers/rtc/rtc-pcf8563.c
index b725913..65f346b 100644
--- a/drivers/rtc/rtc-pcf8563.c
+++ b/drivers/rtc/rtc-pcf8563.c
@@ -212,6 +212,8 @@  static int pcf8563_probe(struct i2c_client *client,
 
 	dev_info(&client->dev, "chip found, driver version " DRV_VERSION "\n");
 
+	i2c_set_clientdata(client, pcf8563);
+
 	pcf8563->rtc = rtc_device_register(pcf8563_driver.driver.name,
 				&client->dev, &pcf8563_rtc_ops, THIS_MODULE);
 
@@ -220,8 +222,6 @@  static int pcf8563_probe(struct i2c_client *client,
 		goto exit_kfree;
 	}
 
-	i2c_set_clientdata(client, pcf8563);
-
 	return 0;
 
 exit_kfree:
diff --git a/drivers/rtc/rtc-pcf8583.c b/drivers/rtc/rtc-pcf8583.c
index 7d33cda..2d201af 100644
--- a/drivers/rtc/rtc-pcf8583.c
+++ b/drivers/rtc/rtc-pcf8583.c
@@ -277,6 +277,8 @@  static int pcf8583_probe(struct i2c_client *client,
 	if (!pcf8583)
 		return -ENOMEM;
 
+	i2c_set_clientdata(client, pcf8583);
+
 	pcf8583->rtc = rtc_device_register(pcf8583_driver.driver.name,
 			&client->dev, &pcf8583_rtc_ops, THIS_MODULE);
 
@@ -285,7 +287,6 @@  static int pcf8583_probe(struct i2c_client *client,
 		goto exit_kfree;
 	}
 
-	i2c_set_clientdata(client, pcf8583);
 	return 0;
 
 exit_kfree:
diff --git a/drivers/rtc/rtc-stk17ta8.c b/drivers/rtc/rtc-stk17ta8.c
index 7d1547b..78d9d07 100644
--- a/drivers/rtc/rtc-stk17ta8.c
+++ b/drivers/rtc/rtc-stk17ta8.c
@@ -288,7 +288,6 @@  static struct bin_attribute stk17ta8_nvram_attr = {
 
 static int __init stk17ta8_rtc_probe(struct platform_device *pdev)
 {
-	struct rtc_device *rtc;
 	struct resource *res;
 	unsigned int cal;
 	unsigned int flags;
@@ -338,22 +337,23 @@  static int __init stk17ta8_rtc_probe(struct platform_device *pdev)
 		}
 	}
 
-	rtc = rtc_device_register(pdev->name, &pdev->dev,
+	pdata->last_jiffies = jiffies;
+	platform_set_drvdata(pdev, pdata);
+
+	pdata->rtc = rtc_device_register(pdev->name, &pdev->dev,
 				  &stk17ta8_rtc_ops, THIS_MODULE);
-	if (IS_ERR(rtc)) {
-		ret = PTR_ERR(rtc);
+	if (IS_ERR(pdata->rtc)) {
+		ret = PTR_ERR(pdata->rtc);
 		goto out;
 	}
-	pdata->rtc = rtc;
-	pdata->last_jiffies = jiffies;
-	platform_set_drvdata(pdev, pdata);
+
 	ret = sysfs_create_bin_file(&pdev->dev.kobj, &stk17ta8_nvram_attr);
-	if (ret)
+	if (ret) {
+		rtc_device_unregister(pdata->rtc);
 		goto out;
+	}
 	return 0;
  out:
-	if (pdata->rtc)
-		rtc_device_unregister(pdata->rtc);
 	if (pdata->irq > 0)
 		free_irq(pdata->irq, pdev);
 	if (ioaddr)
diff --git a/drivers/rtc/rtc-tx4939.c b/drivers/rtc/rtc-tx4939.c
index 4a6ed11..d69775c 100644
--- a/drivers/rtc/rtc-tx4939.c
+++ b/drivers/rtc/rtc-tx4939.c
@@ -235,7 +235,6 @@  static struct bin_attribute tx4939_rtc_nvram_attr = {
 
 static int __init tx4939_rtc_probe(struct platform_device *pdev)
 {
-	struct rtc_device *rtc;
 	struct tx4939rtc_plat_data *pdata;
 	struct resource *res;
 	int irq, ret;
@@ -263,14 +262,15 @@  static int __init tx4939_rtc_probe(struct platform_device *pdev)
 	if (devm_request_irq(&pdev->dev, irq, tx4939_rtc_interrupt,
 			     IRQF_DISABLED, pdev->name, &pdev->dev) < 0)
 		return -EBUSY;
-	rtc = rtc_device_register(pdev->name, &pdev->dev,
+	pdata->rtc = rtc_device_register(pdev->name, &pdev->dev,
 				  &tx4939_rtc_ops, THIS_MODULE);
-	if (IS_ERR(rtc))
-		return PTR_ERR(rtc);
-	pdata->rtc = rtc;
+	if (IS_ERR(pdata->rtc))
+		return PTR_ERR(pdata->rtc);
+
 	ret = sysfs_create_bin_file(&pdev->dev.kobj, &tx4939_rtc_nvram_attr);
 	if (ret)
-		rtc_device_unregister(rtc);
+		rtc_device_unregister(pdata->rtc);
+
 	return ret;
 }
 
diff --git a/drivers/rtc/rtc-v3020.c b/drivers/rtc/rtc-v3020.c
index ad16405..6c00f75 100644
--- a/drivers/rtc/rtc-v3020.c
+++ b/drivers/rtc/rtc-v3020.c
@@ -304,7 +304,6 @@  static int rtc_probe(struct platform_device *pdev)
 {
 	struct v3020_platform_data *pdata = pdev->dev.platform_data;
 	struct v3020 *chip;
-	struct rtc_device *rtc;
 	int retval = -EBUSY;
 	int i;
 	int temp;
@@ -353,13 +352,12 @@  static int rtc_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, chip);
 
-	rtc = rtc_device_register("v3020",
+	chip->rtc = rtc_device_register("v3020",
 				&pdev->dev, &v3020_rtc_ops, THIS_MODULE);
-	if (IS_ERR(rtc)) {
-		retval = PTR_ERR(rtc);
+	if (IS_ERR(chip->rtc)) {
+		retval = PTR_ERR(chip->rtc);
 		goto err_io;
 	}
-	chip->rtc = rtc;
 
 	return 0;