RE: [PATCH] rtc: tps65910: Add RTC driver for TPS65910 PMIC RTC
Hi Andrew,
Already an updated patch v3 (http://lkml.org/lkml/2012/8/8/221 ) was pushed to
fix these issues.
Plz review and merged that.
Thanks,
Venu
> -Original Message-
> From: Andrew Morton [mailto:a...@linux-foundation.org]
> Sent: Wednesday, August 15, 2012 5:22 AM
> To: Venu Byravarasu
> Cc: a.zu...@towertech.it; sa...@linux.intel.com;
> broo...@opensource.wolfsonmicro.com; Laxman Dewangan;
> kyle.ma...@fuel7.com; rtc-li...@googlegroups.com; linux-
> ker...@vger.kernel.org
> Subject: Re: [PATCH] rtc: tps65910: Add RTC driver for TPS65910 PMIC RTC
>
> On Thu, 26 Jul 2012 12:05:19 +0530
> Venu Byravarasu wrote:
>
> > TPS65910 PMIC is a MFD with RTC as one of the device.
> > Adding RTC driver for supporting RTC device present
> > inside TPS65910 PMIC.
> >
> > Only support for RTC alarm is implemented as part of this patch.
>
> It needs a build fix:
>
> drivers/rtc/rtc-tps65910.c: In function 'tps65910_rtc_suspend':
> drivers/rtc/rtc-tps65910.c:313: error: request for member 'irqstat' in
> something not a structure or union
> drivers/rtc/rtc-tps65910.c: In function 'tps65910_rtc_resume':
> drivers/rtc/rtc-tps65910.c:327: error: request for member 'irqstat' in
> something not a structure or union
>
> --- a/drivers/rtc/rtc-tps65910.c~rtc-tps65910-add-rtc-driver-for-tps65910-
> pmic-rtc-fix
> +++ a/drivers/rtc/rtc-tps65910.c
> @@ -310,7 +310,7 @@ static int tps65910_rtc_suspend(struct p
>
> /* Store current list of enabled interrupts*/
> ret = regmap_read(tps->regmap, TPS65910_RTC_INTERRUPTS,
> - >rtc.irqstat);
> + >rtc->irqstat);
> if (ret < 0)
> return ret;
>
> @@ -324,7 +324,7 @@ static int tps65910_rtc_resume(struct pl
>
> /* Restore list of enabled interrupts before suspend */
> return regmap_write(tps->regmap, TPS65910_RTC_INTERRUPTS,
> - tps->rtc.irqstat);
> + tps->rtc->irqstat);
> }
>
> static const struct dev_pm_ops tps65910_rtc_pm_ops = {
>
>
> but it still has problems:
>
> drivers/rtc/rtc-tps65910.c:331: warning: initialization from incompatible
> pointer type
> drivers/rtc/rtc-tps65910.c:332: warning: initialization from incompatible
> pointer type
>
> fix and resend, please?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] rtc: tps65910: Add RTC driver for TPS65910 PMIC RTC
Hi Andrew,
Already an updated patch v3 (http://lkml.org/lkml/2012/8/8/221 ) was pushed to
fix these issues.
Plz review and merged that.
Thanks,
Venu
-Original Message-
From: Andrew Morton [mailto:a...@linux-foundation.org]
Sent: Wednesday, August 15, 2012 5:22 AM
To: Venu Byravarasu
Cc: a.zu...@towertech.it; sa...@linux.intel.com;
broo...@opensource.wolfsonmicro.com; Laxman Dewangan;
kyle.ma...@fuel7.com; rtc-li...@googlegroups.com; linux-
ker...@vger.kernel.org
Subject: Re: [PATCH] rtc: tps65910: Add RTC driver for TPS65910 PMIC RTC
On Thu, 26 Jul 2012 12:05:19 +0530
Venu Byravarasu vbyravar...@nvidia.com wrote:
TPS65910 PMIC is a MFD with RTC as one of the device.
Adding RTC driver for supporting RTC device present
inside TPS65910 PMIC.
Only support for RTC alarm is implemented as part of this patch.
It needs a build fix:
drivers/rtc/rtc-tps65910.c: In function 'tps65910_rtc_suspend':
drivers/rtc/rtc-tps65910.c:313: error: request for member 'irqstat' in
something not a structure or union
drivers/rtc/rtc-tps65910.c: In function 'tps65910_rtc_resume':
drivers/rtc/rtc-tps65910.c:327: error: request for member 'irqstat' in
something not a structure or union
--- a/drivers/rtc/rtc-tps65910.c~rtc-tps65910-add-rtc-driver-for-tps65910-
pmic-rtc-fix
+++ a/drivers/rtc/rtc-tps65910.c
@@ -310,7 +310,7 @@ static int tps65910_rtc_suspend(struct p
/* Store current list of enabled interrupts*/
ret = regmap_read(tps-regmap, TPS65910_RTC_INTERRUPTS,
- tps-rtc.irqstat);
+ tps-rtc-irqstat);
if (ret 0)
return ret;
@@ -324,7 +324,7 @@ static int tps65910_rtc_resume(struct pl
/* Restore list of enabled interrupts before suspend */
return regmap_write(tps-regmap, TPS65910_RTC_INTERRUPTS,
- tps-rtc.irqstat);
+ tps-rtc-irqstat);
}
static const struct dev_pm_ops tps65910_rtc_pm_ops = {
but it still has problems:
drivers/rtc/rtc-tps65910.c:331: warning: initialization from incompatible
pointer type
drivers/rtc/rtc-tps65910.c:332: warning: initialization from incompatible
pointer type
fix and resend, please?
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] rtc: tps65910: Add RTC driver for TPS65910 PMIC RTC
On Thu, 26 Jul 2012 12:05:19 +0530
Venu Byravarasu wrote:
> TPS65910 PMIC is a MFD with RTC as one of the device.
> Adding RTC driver for supporting RTC device present
> inside TPS65910 PMIC.
>
> Only support for RTC alarm is implemented as part of this patch.
It needs a build fix:
drivers/rtc/rtc-tps65910.c: In function 'tps65910_rtc_suspend':
drivers/rtc/rtc-tps65910.c:313: error: request for member 'irqstat' in
something not a structure or union
drivers/rtc/rtc-tps65910.c: In function 'tps65910_rtc_resume':
drivers/rtc/rtc-tps65910.c:327: error: request for member 'irqstat' in
something not a structure or union
---
a/drivers/rtc/rtc-tps65910.c~rtc-tps65910-add-rtc-driver-for-tps65910-pmic-rtc-fix
+++ a/drivers/rtc/rtc-tps65910.c
@@ -310,7 +310,7 @@ static int tps65910_rtc_suspend(struct p
/* Store current list of enabled interrupts*/
ret = regmap_read(tps->regmap, TPS65910_RTC_INTERRUPTS,
- >rtc.irqstat);
+ >rtc->irqstat);
if (ret < 0)
return ret;
@@ -324,7 +324,7 @@ static int tps65910_rtc_resume(struct pl
/* Restore list of enabled interrupts before suspend */
return regmap_write(tps->regmap, TPS65910_RTC_INTERRUPTS,
- tps->rtc.irqstat);
+ tps->rtc->irqstat);
}
static const struct dev_pm_ops tps65910_rtc_pm_ops = {
but it still has problems:
drivers/rtc/rtc-tps65910.c:331: warning: initialization from incompatible
pointer type
drivers/rtc/rtc-tps65910.c:332: warning: initialization from incompatible
pointer type
fix and resend, please?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] rtc: tps65910: Add RTC driver for TPS65910 PMIC RTC
On Thu, 26 Jul 2012 12:05:19 +0530
Venu Byravarasu vbyravar...@nvidia.com wrote:
TPS65910 PMIC is a MFD with RTC as one of the device.
Adding RTC driver for supporting RTC device present
inside TPS65910 PMIC.
Only support for RTC alarm is implemented as part of this patch.
It needs a build fix:
drivers/rtc/rtc-tps65910.c: In function 'tps65910_rtc_suspend':
drivers/rtc/rtc-tps65910.c:313: error: request for member 'irqstat' in
something not a structure or union
drivers/rtc/rtc-tps65910.c: In function 'tps65910_rtc_resume':
drivers/rtc/rtc-tps65910.c:327: error: request for member 'irqstat' in
something not a structure or union
---
a/drivers/rtc/rtc-tps65910.c~rtc-tps65910-add-rtc-driver-for-tps65910-pmic-rtc-fix
+++ a/drivers/rtc/rtc-tps65910.c
@@ -310,7 +310,7 @@ static int tps65910_rtc_suspend(struct p
/* Store current list of enabled interrupts*/
ret = regmap_read(tps-regmap, TPS65910_RTC_INTERRUPTS,
- tps-rtc.irqstat);
+ tps-rtc-irqstat);
if (ret 0)
return ret;
@@ -324,7 +324,7 @@ static int tps65910_rtc_resume(struct pl
/* Restore list of enabled interrupts before suspend */
return regmap_write(tps-regmap, TPS65910_RTC_INTERRUPTS,
- tps-rtc.irqstat);
+ tps-rtc-irqstat);
}
static const struct dev_pm_ops tps65910_rtc_pm_ops = {
but it still has problems:
drivers/rtc/rtc-tps65910.c:331: warning: initialization from incompatible
pointer type
drivers/rtc/rtc-tps65910.c:332: warning: initialization from incompatible
pointer type
fix and resend, please?
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] rtc: tps65910: Add RTC driver for TPS65910 PMIC RTC
On 07/30/12 03:07, Venu Byravarasu wrote:
>
> +
> +static int __devinit tps65910_rtc_probe(struct platform_device *pdev)
> +{
> + struct tps65910 *tps65910 = NULL;
> + struct tps65910_rtc *tps_rtc = NULL;
> + struct tps65910_board *pmic_plat_data;
> + int ret = -EINVAL;
> + int irq = 0;
> + u32 rtc_reg;
>> It seems like all the above assignments are useless as they're
>> overwritten later in this function. Can you remove the assignments?
>>
> Some of the non-intelligent compilers/tools complain as variables
> may get used uninitialized. Hence to avoid such complaints, initialized
> them to some default values.
> What harm do you see if I have local variables initialized during their
> declaration?
If you return early from a function and forget to give a variable a
value you actually want, you may use it pre-initialized with a bad
value. I would be surprised if a compiler complained about these ones
because they are simple assignments and they aren't under conditional
paths. If there is still a problem, you can use the uninitialized_var()
macro but I don't see why you would need to.
--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] rtc: tps65910: Add RTC driver for TPS65910 PMIC RTC
Thanks Stephen for your comments.
Plz see my comments inline.
> -Original Message-
> From: Stephen Boyd [mailto:sb...@codeaurora.org]
> Sent: Monday, July 30, 2012 2:47 PM
> To: Venu Byravarasu
> Cc: a.zu...@towertech.it; sa...@linux.intel.com;
> broo...@opensource.wolfsonmicro.com; Laxman Dewangan;
> kyle.ma...@fuel7.com; rtc-li...@googlegroups.com; linux-
> ker...@vger.kernel.org
> Subject: Re: [PATCH] rtc: tps65910: Add RTC driver for TPS65910 PMIC RTC
>
> On 7/25/2012 11:35 PM, Venu Byravarasu wrote:
> > +
> > +static struct rtc_class_ops tps65910_rtc_ops = {
>
> const?
Will add it in my next patch.
>
> > + .read_time = tps65910_rtc_read_time,
> > + .set_time = tps65910_rtc_set_time,
> > + .read_alarm = tps65910_rtc_read_alarm,
> > + .set_alarm = tps65910_rtc_set_alarm,
> > + .alarm_irq_enable = tps65910_rtc_alarm_irq_enable,
> > +};
> > +
> > +static int __devinit tps65910_rtc_probe(struct platform_device *pdev)
> > +{
> > + struct tps65910 *tps65910 = NULL;
> > + struct tps65910_rtc *tps_rtc = NULL;
> > + struct tps65910_board *pmic_plat_data;
> > + int ret = -EINVAL;
> > + int irq = 0;
> > + u32 rtc_reg;
>
> It seems like all the above assignments are useless as they're
> overwritten later in this function. Can you remove the assignments?
>
Some of the non-intelligent compilers/tools complain as variables
may get used uninitialized. Hence to avoid such complaints, initialized
them to some default values.
What harm do you see if I have local variables initialized during their
declaration?
> > +
> > + tps65910 = dev_get_drvdata(pdev->dev.parent);
> > +
> > + tps_rtc = devm_kzalloc(>dev, sizeof(struct tps65910_rtc),
> > + GFP_KERNEL);
> > + if (!tps_rtc)
> > + return -ENOMEM;
> > +
> > + /* Clear pending interrupts */
> > + ret = regmap_read(tps65910->regmap, TPS65910_RTC_STATUS,
> _reg);
> > + if (ret < 0)
> > + return ret;
> > +
> > + ret = regmap_write(tps65910->regmap, TPS65910_RTC_STATUS,
> rtc_reg);
> > + if (ret < 0)
> > + return ret;
> > +
> > + dev_dbg(>dev, "Enabling tps65910-RTC.\n");
>
> Hmph, looks more like stopping the RTC.
>
No, the register is a misnomer here.
As per data sheet of TPS65910, setting this bit will start RTC,
instead of stopping as its name suggests.
> > + rtc_reg = TPS65910_RTC_CTRL_STOP_RTC;
> > + ret = regmap_write(tps65910->regmap, TPS65910_RTC_CTRL,
> rtc_reg);
> > + if (ret < 0)
> > + return ret;
> > +
> > + pmic_plat_data = dev_get_platdata(tps65910->dev);
> > + irq = pmic_plat_data->irq_base;
> > + if (irq <= 0) {
> > + dev_warn(>dev, "Wake up is not possible as irq =
> %d\n",
> > + irq);
> > + return ret;
> > + }
> > +
> > + irq += TPS65910_IRQ_RTC_ALARM;
> > + ret = devm_request_threaded_irq(>dev, irq, NULL,
> > + tps65910_rtc_interrupt, IRQF_TRIGGER_LOW,
> > + dev_name(_rtc->rtc->dev), >dev);
>
> How does this work? It doesn't look like tps_rtc->rtc is assigned until
> down there at the rtc_device_register() call.
>
Somehow this got skipped. Thanks for pointing out.
Will fix and push as part of next patch.
> > + if (ret < 0) {
> > + dev_err(>dev, "IRQ is not free.\n");
> > + return ret;
> > + }
> > + device_init_wakeup(>dev, 1);
> > +
> > + tps_rtc->rtc = rtc_device_register(pdev->name, >dev,
> > + _rtc_ops, THIS_MODULE);
> > + if (IS_ERR(tps_rtc->rtc)) {
> > + ret = PTR_ERR(tps_rtc->rtc);
> > + dev_err(>dev, "RTC device register: err %d\n", ret);
> > + return ret;
> > + }
> > +
>
> --
> Sent by an employee of the Qualcomm Innovation Center, Inc.
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
> Forum.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] rtc: tps65910: Add RTC driver for TPS65910 PMIC RTC
On 7/25/2012 11:35 PM, Venu Byravarasu wrote:
> +
> +static struct rtc_class_ops tps65910_rtc_ops = {
const?
> + .read_time = tps65910_rtc_read_time,
> + .set_time = tps65910_rtc_set_time,
> + .read_alarm = tps65910_rtc_read_alarm,
> + .set_alarm = tps65910_rtc_set_alarm,
> + .alarm_irq_enable = tps65910_rtc_alarm_irq_enable,
> +};
> +
> +static int __devinit tps65910_rtc_probe(struct platform_device *pdev)
> +{
> + struct tps65910 *tps65910 = NULL;
> + struct tps65910_rtc *tps_rtc = NULL;
> + struct tps65910_board *pmic_plat_data;
> + int ret = -EINVAL;
> + int irq = 0;
> + u32 rtc_reg;
It seems like all the above assignments are useless as they're
overwritten later in this function. Can you remove the assignments?
> +
> + tps65910 = dev_get_drvdata(pdev->dev.parent);
> +
> + tps_rtc = devm_kzalloc(>dev, sizeof(struct tps65910_rtc),
> + GFP_KERNEL);
> + if (!tps_rtc)
> + return -ENOMEM;
> +
> + /* Clear pending interrupts */
> + ret = regmap_read(tps65910->regmap, TPS65910_RTC_STATUS, _reg);
> + if (ret < 0)
> + return ret;
> +
> + ret = regmap_write(tps65910->regmap, TPS65910_RTC_STATUS, rtc_reg);
> + if (ret < 0)
> + return ret;
> +
> + dev_dbg(>dev, "Enabling tps65910-RTC.\n");
Hmph, looks more like stopping the RTC.
> + rtc_reg = TPS65910_RTC_CTRL_STOP_RTC;
> + ret = regmap_write(tps65910->regmap, TPS65910_RTC_CTRL, rtc_reg);
> + if (ret < 0)
> + return ret;
> +
> + pmic_plat_data = dev_get_platdata(tps65910->dev);
> + irq = pmic_plat_data->irq_base;
> + if (irq <= 0) {
> + dev_warn(>dev, "Wake up is not possible as irq = %d\n",
> + irq);
> + return ret;
> + }
> +
> + irq += TPS65910_IRQ_RTC_ALARM;
> + ret = devm_request_threaded_irq(>dev, irq, NULL,
> + tps65910_rtc_interrupt, IRQF_TRIGGER_LOW,
> + dev_name(_rtc->rtc->dev), >dev);
How does this work? It doesn't look like tps_rtc->rtc is assigned until
down there at the rtc_device_register() call.
> + if (ret < 0) {
> + dev_err(>dev, "IRQ is not free.\n");
> + return ret;
> + }
> + device_init_wakeup(>dev, 1);
> +
> + tps_rtc->rtc = rtc_device_register(pdev->name, >dev,
> + _rtc_ops, THIS_MODULE);
> + if (IS_ERR(tps_rtc->rtc)) {
> + ret = PTR_ERR(tps_rtc->rtc);
> + dev_err(>dev, "RTC device register: err %d\n", ret);
> + return ret;
> + }
> +
--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] rtc: tps65910: Add RTC driver for TPS65910 PMIC RTC
On 7/25/2012 11:35 PM, Venu Byravarasu wrote:
+
+static struct rtc_class_ops tps65910_rtc_ops = {
const?
+ .read_time = tps65910_rtc_read_time,
+ .set_time = tps65910_rtc_set_time,
+ .read_alarm = tps65910_rtc_read_alarm,
+ .set_alarm = tps65910_rtc_set_alarm,
+ .alarm_irq_enable = tps65910_rtc_alarm_irq_enable,
+};
+
+static int __devinit tps65910_rtc_probe(struct platform_device *pdev)
+{
+ struct tps65910 *tps65910 = NULL;
+ struct tps65910_rtc *tps_rtc = NULL;
+ struct tps65910_board *pmic_plat_data;
+ int ret = -EINVAL;
+ int irq = 0;
+ u32 rtc_reg;
It seems like all the above assignments are useless as they're
overwritten later in this function. Can you remove the assignments?
+
+ tps65910 = dev_get_drvdata(pdev-dev.parent);
+
+ tps_rtc = devm_kzalloc(pdev-dev, sizeof(struct tps65910_rtc),
+ GFP_KERNEL);
+ if (!tps_rtc)
+ return -ENOMEM;
+
+ /* Clear pending interrupts */
+ ret = regmap_read(tps65910-regmap, TPS65910_RTC_STATUS, rtc_reg);
+ if (ret 0)
+ return ret;
+
+ ret = regmap_write(tps65910-regmap, TPS65910_RTC_STATUS, rtc_reg);
+ if (ret 0)
+ return ret;
+
+ dev_dbg(pdev-dev, Enabling tps65910-RTC.\n);
Hmph, looks more like stopping the RTC.
+ rtc_reg = TPS65910_RTC_CTRL_STOP_RTC;
+ ret = regmap_write(tps65910-regmap, TPS65910_RTC_CTRL, rtc_reg);
+ if (ret 0)
+ return ret;
+
+ pmic_plat_data = dev_get_platdata(tps65910-dev);
+ irq = pmic_plat_data-irq_base;
+ if (irq = 0) {
+ dev_warn(pdev-dev, Wake up is not possible as irq = %d\n,
+ irq);
+ return ret;
+ }
+
+ irq += TPS65910_IRQ_RTC_ALARM;
+ ret = devm_request_threaded_irq(pdev-dev, irq, NULL,
+ tps65910_rtc_interrupt, IRQF_TRIGGER_LOW,
+ dev_name(tps_rtc-rtc-dev), pdev-dev);
How does this work? It doesn't look like tps_rtc-rtc is assigned until
down there at the rtc_device_register() call.
+ if (ret 0) {
+ dev_err(pdev-dev, IRQ is not free.\n);
+ return ret;
+ }
+ device_init_wakeup(pdev-dev, 1);
+
+ tps_rtc-rtc = rtc_device_register(pdev-name, pdev-dev,
+ tps65910_rtc_ops, THIS_MODULE);
+ if (IS_ERR(tps_rtc-rtc)) {
+ ret = PTR_ERR(tps_rtc-rtc);
+ dev_err(pdev-dev, RTC device register: err %d\n, ret);
+ return ret;
+ }
+
--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] rtc: tps65910: Add RTC driver for TPS65910 PMIC RTC
Thanks Stephen for your comments.
Plz see my comments inline.
-Original Message-
From: Stephen Boyd [mailto:sb...@codeaurora.org]
Sent: Monday, July 30, 2012 2:47 PM
To: Venu Byravarasu
Cc: a.zu...@towertech.it; sa...@linux.intel.com;
broo...@opensource.wolfsonmicro.com; Laxman Dewangan;
kyle.ma...@fuel7.com; rtc-li...@googlegroups.com; linux-
ker...@vger.kernel.org
Subject: Re: [PATCH] rtc: tps65910: Add RTC driver for TPS65910 PMIC RTC
On 7/25/2012 11:35 PM, Venu Byravarasu wrote:
+
+static struct rtc_class_ops tps65910_rtc_ops = {
const?
Will add it in my next patch.
+ .read_time = tps65910_rtc_read_time,
+ .set_time = tps65910_rtc_set_time,
+ .read_alarm = tps65910_rtc_read_alarm,
+ .set_alarm = tps65910_rtc_set_alarm,
+ .alarm_irq_enable = tps65910_rtc_alarm_irq_enable,
+};
+
+static int __devinit tps65910_rtc_probe(struct platform_device *pdev)
+{
+ struct tps65910 *tps65910 = NULL;
+ struct tps65910_rtc *tps_rtc = NULL;
+ struct tps65910_board *pmic_plat_data;
+ int ret = -EINVAL;
+ int irq = 0;
+ u32 rtc_reg;
It seems like all the above assignments are useless as they're
overwritten later in this function. Can you remove the assignments?
Some of the non-intelligent compilers/tools complain as variables
may get used uninitialized. Hence to avoid such complaints, initialized
them to some default values.
What harm do you see if I have local variables initialized during their
declaration?
+
+ tps65910 = dev_get_drvdata(pdev-dev.parent);
+
+ tps_rtc = devm_kzalloc(pdev-dev, sizeof(struct tps65910_rtc),
+ GFP_KERNEL);
+ if (!tps_rtc)
+ return -ENOMEM;
+
+ /* Clear pending interrupts */
+ ret = regmap_read(tps65910-regmap, TPS65910_RTC_STATUS,
rtc_reg);
+ if (ret 0)
+ return ret;
+
+ ret = regmap_write(tps65910-regmap, TPS65910_RTC_STATUS,
rtc_reg);
+ if (ret 0)
+ return ret;
+
+ dev_dbg(pdev-dev, Enabling tps65910-RTC.\n);
Hmph, looks more like stopping the RTC.
No, the register is a misnomer here.
As per data sheet of TPS65910, setting this bit will start RTC,
instead of stopping as its name suggests.
+ rtc_reg = TPS65910_RTC_CTRL_STOP_RTC;
+ ret = regmap_write(tps65910-regmap, TPS65910_RTC_CTRL,
rtc_reg);
+ if (ret 0)
+ return ret;
+
+ pmic_plat_data = dev_get_platdata(tps65910-dev);
+ irq = pmic_plat_data-irq_base;
+ if (irq = 0) {
+ dev_warn(pdev-dev, Wake up is not possible as irq =
%d\n,
+ irq);
+ return ret;
+ }
+
+ irq += TPS65910_IRQ_RTC_ALARM;
+ ret = devm_request_threaded_irq(pdev-dev, irq, NULL,
+ tps65910_rtc_interrupt, IRQF_TRIGGER_LOW,
+ dev_name(tps_rtc-rtc-dev), pdev-dev);
How does this work? It doesn't look like tps_rtc-rtc is assigned until
down there at the rtc_device_register() call.
Somehow this got skipped. Thanks for pointing out.
Will fix and push as part of next patch.
+ if (ret 0) {
+ dev_err(pdev-dev, IRQ is not free.\n);
+ return ret;
+ }
+ device_init_wakeup(pdev-dev, 1);
+
+ tps_rtc-rtc = rtc_device_register(pdev-name, pdev-dev,
+ tps65910_rtc_ops, THIS_MODULE);
+ if (IS_ERR(tps_rtc-rtc)) {
+ ret = PTR_ERR(tps_rtc-rtc);
+ dev_err(pdev-dev, RTC device register: err %d\n, ret);
+ return ret;
+ }
+
--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] rtc: tps65910: Add RTC driver for TPS65910 PMIC RTC
On 07/30/12 03:07, Venu Byravarasu wrote:
+
+static int __devinit tps65910_rtc_probe(struct platform_device *pdev)
+{
+ struct tps65910 *tps65910 = NULL;
+ struct tps65910_rtc *tps_rtc = NULL;
+ struct tps65910_board *pmic_plat_data;
+ int ret = -EINVAL;
+ int irq = 0;
+ u32 rtc_reg;
It seems like all the above assignments are useless as they're
overwritten later in this function. Can you remove the assignments?
Some of the non-intelligent compilers/tools complain as variables
may get used uninitialized. Hence to avoid such complaints, initialized
them to some default values.
What harm do you see if I have local variables initialized during their
declaration?
If you return early from a function and forget to give a variable a
value you actually want, you may use it pre-initialized with a bad
value. I would be surprised if a compiler complained about these ones
because they are simple assignments and they aren't under conditional
paths. If there is still a problem, you can use the uninitialized_var()
macro but I don't see why you would need to.
--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] rtc: tps65910: Add RTC driver for TPS65910 PMIC RTC
Hi Venu, On Thu, Jul 26, 2012 at 12:05:19PM +0530, Venu Byravarasu wrote: > TPS65910 PMIC is a MFD with RTC as one of the device. > Adding RTC driver for supporting RTC device present > inside TPS65910 PMIC. > > Only support for RTC alarm is implemented as part of this patch. > > Signed-off-by: Venu Byravarasu > --- > > drivers/rtc/Kconfig | 10 ++ > drivers/rtc/Makefile |1 + > drivers/rtc/rtc-tps65910.c | 353 > ++ > include/linux/mfd/tps65910.h | 10 ++ > 4 files changed, 374 insertions(+), 0 deletions(-) > create mode 100644 drivers/rtc/rtc-tps65910.c For the MFD parts: Acked-by: Samuel Ortiz Cheers, Samuel. -- Intel Open Source Technology Centre http://oss.intel.com/ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] rtc: tps65910: Add RTC driver for TPS65910 PMIC RTC
Hi Venu, On Thu, Jul 26, 2012 at 12:05:19PM +0530, Venu Byravarasu wrote: TPS65910 PMIC is a MFD with RTC as one of the device. Adding RTC driver for supporting RTC device present inside TPS65910 PMIC. Only support for RTC alarm is implemented as part of this patch. Signed-off-by: Venu Byravarasu vbyravar...@nvidia.com --- drivers/rtc/Kconfig | 10 ++ drivers/rtc/Makefile |1 + drivers/rtc/rtc-tps65910.c | 353 ++ include/linux/mfd/tps65910.h | 10 ++ 4 files changed, 374 insertions(+), 0 deletions(-) create mode 100644 drivers/rtc/rtc-tps65910.c For the MFD parts: Acked-by: Samuel Ortiz sa...@linux.intel.com Cheers, Samuel. -- Intel Open Source Technology Centre http://oss.intel.com/ -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
