zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.48k stars 6.41k forks source link

RTC_DS1307 driver zeroes-out seconds when initialising #77354

Open clodnut opened 3 weeks ago

clodnut commented 3 weeks ago

Describe the bug

ds1307_init() zeroes-out the "SECONDS" field of the time in the DS1307 device, corrupting the time.

Details

ds1307_init() correctly clears the DS1307's "Clock Halt" bit which will be set when power is first applied to the DS1307.

However, it does this by wiping out the entire DS1307_REG_SECONDS register, which unconditionally zeroes-out the "seconds" field of the time:

    /* Make clock halt = 0 */
    err = i2c_reg_write_byte_dt(&config->i2c_bus, DS1307_REG_SECONDS, 0x00);
    if (err < 0) {
        LOG_ERR("Error: Set clock halt bit:%d\n", err);
    }

This operation should be a read/modify/write to clear only bit 7 ("CH") and only if that bit is set (so the time cannot change between read and write).

This would avoid corrupting the "seconds" field(s).

I suggest:

    /* Ensure Clock Halt bit is 0 */
    uint8_t reg = 0;
    err = i2c_reg_read_byte_dt(&config->i2c_bus, DS1307_REG_SECONDS, &reg);
    if (err < 0) {
        LOG_ERR("Error: Read SECONDS/Clock Halt register:%d", err);
    }
    if (reg & ~SECONDS_BITS){
        /* Clock Halt bit is set */
        err = i2c_reg_write_byte_dt(&config->i2c_bus, DS1307_REG_SECONDS, reg & SECONDS_BITS);
        if (err < 0) {
            LOG_ERR("Error: Write SECONDS/Clock Halt register:%d", err);
        }
    }

Tested informally and seems to work.

github-actions[bot] commented 3 weeks ago

Hi @clodnut! We appreciate you submitting your first issue for our open-source project. 🌟

Even though I'm a bot, I can assure you that the whole community is genuinely grateful for your time and effort. 🤖💙

kartben commented 3 weeks ago

Thanks for the report! Since you seem to have identified a fix, could you please submit a pull request? Thanks!