rtc: ds1307: improve weekday handling

The current code for checking and fixing the weekday in ds1307_probe
faces some issues:
- This check is applied to all chips even if its applicable (AFAIK)
  to mcp794xx only
- The check uses MCP794XX constants for registers and bits even though
  it's executed also on other chips (ok, this could be fixed easily)
- It relies on tm_wday being properly populated when core calls set_time
  and set_alarm. This is not guaranteed at all.

First two issue we could solve by moving the check to the
mcp794xx-specific initialization (where also VBATEN flag is set).

The proposed alternative is in the set_alarm path for mcp794xx only and
calculates the alarm weekday based on the current weekday in the RTC
timekeeping regs and the difference between alarm date and current date.
So we are fine with any weekday even if it doesn't match the date.

Still there are cases where this could fail, e.g.:
- rtc date/time + weekday have power-on-reset default values
- alarm is set to actual date/time + x
- set_time is called (may change diff between rtc weekday and actual
  weekday)

But similar issues we have with the current code too:
- rtc date/time + weekday have power-on-reset default values
- alarm is set to rtc date/time + x
- set_time is called before the alarm triggers

Using random rtc date/time with relative alarms simply can interfere
with set_time. I'm not totally convinced of either option yet.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>

1 files changed, 26 insertions, 26 deletions

diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index e7d9215c9201..1cca20fdd025 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -787,8 +787,6 @@ static int rx8130_alarm_irq_enable(struct device *dev, unsigned int enabled)
  * Alarm support for mcp794xx devices.
  */
 
-#define MCP794XX_REG_WEEKDAY		0x3
-#define MCP794XX_REG_WEEKDAY_WDAY_MASK	0x7
 #define MCP794XX_REG_CONTROL		0x07
 #	define MCP794XX_BIT_ALM0_EN	0x10
 #	define MCP794XX_BIT_ALM1_EN	0x20
@@ -877,15 +875,38 @@ static int mcp794xx_read_alarm(struct device *dev, struct rtc_wkalrm *t)
 	return 0;
 }
 
+/*
+ * We may have a random RTC weekday, therefore calculate alarm weekday based
+ * on current weekday we read from the RTC timekeeping regs
+ */
+static int mcp794xx_alm_weekday(struct device *dev, struct rtc_time *tm_alarm)
+{
+	struct rtc_time tm_now;
+	int days_now, days_alarm, ret;
+
+	ret = ds1307_get_time(dev, &tm_now);
+	if (ret)
+		return ret;
+
+	days_now = div_s64(rtc_tm_to_time64(&tm_now), 24 * 60 * 60);
+	days_alarm = div_s64(rtc_tm_to_time64(tm_alarm), 24 * 60 * 60);
+
+	return (tm_now.tm_wday + days_alarm - days_now) % 7 + 1;
+}
+
 static int mcp794xx_set_alarm(struct device *dev, struct rtc_wkalrm *t)
 {
 	struct ds1307 *ds1307 = dev_get_drvdata(dev);
 	unsigned char regs[10];
-	int ret;
+	int wday, ret;
 
 	if (!test_bit(HAS_ALARM, &ds1307->flags))
 		return -EINVAL;
 
+	wday = mcp794xx_alm_weekday(dev, &t->time);
+	if (wday < 0)
+		return wday;
+
 	dev_dbg(dev, "%s, sec=%d min=%d hour=%d wday=%d mday=%d mon=%d "
 		"enabled=%d pending=%d\n", __func__,
 		t->time.tm_sec, t->time.tm_min, t->time.tm_hour,
@@ -902,7 +923,7 @@ static int mcp794xx_set_alarm(struct device *dev, struct rtc_wkalrm *t)
 	regs[3] = bin2bcd(t->time.tm_sec);
 	regs[4] = bin2bcd(t->time.tm_min);
 	regs[5] = bin2bcd(t->time.tm_hour);
-	regs[6] = bin2bcd(t->time.tm_wday + 1);
+	regs[6] = wday;
 	regs[7] = bin2bcd(t->time.tm_mday);
 	regs[8] = bin2bcd(t->time.tm_mon + 1);
 
@@ -1354,14 +1375,12 @@ static int ds1307_probe(struct i2c_client *client,
 {
 	struct ds1307		*ds1307;
 	int			err = -ENODEV;
-	int			tmp, wday;
+	int			tmp;
 	const struct chip_desc	*chip;
 	bool			want_irq;
 	bool			ds1307_can_wakeup_device = false;
 	unsigned char		regs[8];
 	struct ds1307_platform_data *pdata = dev_get_platdata(&client->dev);
-	struct rtc_time		tm;
-	unsigned long		timestamp;
 	u8			trickle_charger_setup = 0;
 
 	ds1307 = devm_kzalloc(&client->dev, sizeof(struct ds1307), GFP_KERNEL);
@@ -1641,25 +1660,6 @@ read_rtc:
 			     bin2bcd(tmp));
 	}
 
-	/*
-	 * Some IPs have weekday reset value = 0x1 which might not correct
-	 * hence compute the wday using the current date/month/year values
-	 */
-	ds1307_get_time(ds1307->dev, &tm);
-	wday = tm.tm_wday;
-	timestamp = rtc_tm_to_time64(&tm);
-	rtc_time64_to_tm(timestamp, &tm);
-
-	/*
-	 * Check if reset wday is different from the computed wday
-	 * If different then set the wday which we computed using
-	 * timestamp
-	 */
-	if (wday != tm.tm_wday)
-		regmap_update_bits(ds1307->regmap, MCP794XX_REG_WEEKDAY,
-				   MCP794XX_REG_WEEKDAY_WDAY_MASK,
-				   tm.tm_wday + 1);
-
 	if (want_irq || ds1307_can_wakeup_device) {
 		device_set_wakeup_capable(ds1307->dev, true);
 		set_bit(HAS_ALARM, &ds1307->flags);