Hi Dan,
Thanks for catching that.
On 10/16/2014 10:55 AM, Dan Carpenter wrote:
> We recently added a new error path to this function and it is missing an
> unlock.
>
> Fixes: 798187920a6e ('drivers/rtc/rtc-pm8xxx.c: rework to support pm8941 rtc')
> Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
>
> diff --git a/drivers/rtc/rtc-pm8xxx.c b/drivers/rtc/rtc-pm8xxx.c
> index af88817..dd98895 100644
> --- a/drivers/rtc/rtc-pm8xxx.c
> +++ b/drivers/rtc/rtc-pm8xxx.c
> @@ -86,6 +86,7 @@ static int pm8xxx_rtc_set_time(struct device *dev, struct rtc_time *tm)
> unsigned int ctrl_reg;
> struct pm8xxx_rtc *rtc_dd = dev_get_drvdata(dev);
> const struct pm8xxx_rtc_regs *regs = rtc_dd->regs;
> + int locked;
>
> if (!rtc_dd->allow_set_time)
> return -EACCES;
> @@ -100,6 +101,7 @@ static int pm8xxx_rtc_set_time(struct device *dev, struct rtc_time *tm)
> dev_dbg(dev, "Seconds value to be written to RTC = %lu\n", secs);
>
> spin_lock_irqsave(&rtc_dd->ctrl_reg_lock, irq_flags);
> + locked = 1;
Sparse complaining in this function too:
warning: context imbalance in 'pm8xxx_rtc_set_time' - different lock
contexts for basic block
I'm wondering for a better fix to this. Isn't better to avoid this
conditional call to spin_unlock_irqrestore() and lock regmap writes
every time without care is the alarm is enabled or not.
>
> rc = regmap_read(rtc_dd->regmap, regs->ctrl, &ctrl_reg);
> if (rc)
> @@ -115,6 +117,7 @@ static int pm8xxx_rtc_set_time(struct device *dev, struct rtc_time *tm)
> }
> } else {
> spin_unlock_irqrestore(&rtc_dd->ctrl_reg_lock, irq_flags);
> + locked = 0;
> }
>
> /* Write 0 to Byte[0] */
> @@ -149,7 +152,7 @@ static int pm8xxx_rtc_set_time(struct device *dev, struct rtc_time *tm)
> }
>
> rtc_rw_fail:
> - if (alarm_enabled)
> + if (locked)
> spin_unlock_irqrestore(&rtc_dd->ctrl_reg_lock, irq_flags);
>
> return rc;
>
--
regards,
Stan
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
