Am 28.08.2017 um 21:15 schrieb Alexandre Belloni:
> On 28/08/2017 at 20:37:21 +0200, Heiner Kallweit wrote:
>> mcp794xx as one chip supported by this driver needs the weekday for
>> alarm matching. RTC core ignores the weekday so we can't rely on
>> the values we receive in member tm_wday of struct rtc_time.
>> Therefore calculate the weekday from date/time when setting the
>> time and setting the alarm time for mcp794xx.
>>
>> When having this in place we don't have to check the weekday
>> at each driver load.
>> After a chip reset date/time and weekday may be out of sync but
>> in this case date/time need to be set anyway.
>>
>
> Nope, the core issue is that you can actually set an alarm in the future
> without setting the time beforehand so this as to be fixed in the probe
> (this was the issue as reported at the time of the fix).
>
Ah, found the original thread where this was discussed.
Still I don't really understand the problem. mcp794xx matches based on
secs, min, hour, wday, mday, month. When I use "rtcwake -s 5" I expect
the alarm to trigger 5 secs from now. How can this ever work if the
RTC has random date/time (wday being valid or not)?
I'd tend to say that if we know the RTC time is incorrect we shouldn't
allow setting an alarm.
At least I wouldn't dare to program a bomb explosion time if I stand in
front of it and know that the bomb's clock has a random value ;)
>> Signed-off-by: Heiner Kallweit <hkallweit1@xxxxxxxxx>
>> ---
>> drivers/rtc/rtc-ds1307.c | 39 +++++++++++++++------------------------
>> 1 file changed, 15 insertions(+), 24 deletions(-)
>>
>> diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
>> index 9d680d36..83b8c997 100644
>> --- a/drivers/rtc/rtc-ds1307.c
>> +++ b/drivers/rtc/rtc-ds1307.c
>> @@ -437,6 +437,18 @@ static int ds1307_get_time(struct device *dev, struct rtc_time *t)
>> return rtc_valid_tm(t);
>> }
>>
>> +/*
>> + * Certain chips need the weekday for alarm matching and tm->t_wday
>> + * may be not or not properly populated
>> + */
>> +static int ds1307_get_weekday(struct rtc_time *tm)
>> +{
>> + time64_t secs64 = rtc_tm_to_time64(tm);
>> + int days = div_s64(secs64, 24 * 60 * 60);
>> +
>> + return (days + 4) % 7 + 1;
>> +}
>> +
>> static int ds1307_set_time(struct device *dev, struct rtc_time *t)
>> {
>> struct ds1307 *ds1307 = dev_get_drvdata(dev);
>> @@ -465,7 +477,7 @@ static int ds1307_set_time(struct device *dev, struct rtc_time *t)
>> regs[DS1307_REG_SECS] = bin2bcd(t->tm_sec);
>> regs[DS1307_REG_MIN] = bin2bcd(t->tm_min);
>> regs[DS1307_REG_HOUR] = bin2bcd(t->tm_hour);
>> - regs[DS1307_REG_WDAY] = bin2bcd(t->tm_wday + 1);
>> + regs[DS1307_REG_WDAY] = ds1307_get_weekday(t);
>> regs[DS1307_REG_MDAY] = bin2bcd(t->tm_mday);
>> regs[DS1307_REG_MONTH] = bin2bcd(t->tm_mon + 1);
>>
>> @@ -902,7 +914,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] = ds1307_get_weekday(&t->time);
>> regs[7] = bin2bcd(t->time.tm_mday);
>> regs[8] = bin2bcd(t->time.tm_mon + 1);
>>
>> @@ -1355,14 +1367,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);
>> @@ -1642,25 +1652,6 @@ static int ds1307_probe(struct i2c_client *client,
>> 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);
>> --
>> 2.14.1
>>
>
