lkml.org 
[lkml]   [2020]   [Jan]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 4.19 521/639] rtc: pcf2127: bugfix: read rtc disables watchdog
    On Fri 2020-01-24 10:31:31, Greg Kroah-Hartman wrote:
    > From: Bruno Thomsen <bruno.thomsen@gmail.com>
    >
    > [ Upstream commit 7f43020e3bdb63d65661ed377682702f8b34d3ea ]
    >
    > The previous fix listed bulk read of registers as root cause of
    > accendential disabling of watchdog, since the watchdog counter
    > register (WD_VAL) was zeroed.
    >
    > Fixes: 3769a375ab83 rtc: pcf2127: bulk read only date and time registers.
    >
    > Tested with the same PCF2127 chip as Sean reveled root cause
    > of WD_VAL register value zeroing was caused by reading CTRL2
    > register which is one of the watchdog feature control registers.
    >
    > So the solution is to not read the first two control registers
    > (CTRL1 and CTRL2) in pcf2127_rtc_read_time as they are not
    > needed anyway. Size of local buf variable is kept to allow
    > easy usage of register defines to improve readability of code.

    Should the array be zeroed before or something? This way, one array
    contains both undefined values and valid data...

    > Debug trace line was updated after CTRL1 and CTRL2 are no longer
    > read from the chip. Also replaced magic numbers in buf access
    > with register defines.

    That part is not an improvement. Previously the code was formatted so
    that you could parse what is being printed.

    Best regards, Pavel

    > @@ -91,14 +85,12 @@ static int pcf2127_rtc_read_time(struct device *dev, struct rtc_time *tm)
    > }
    >
    > dev_dbg(dev,
    > - "%s: raw data is cr1=%02x, cr2=%02x, cr3=%02x, "
    > - "sec=%02x, min=%02x, hr=%02x, "
    > + "%s: raw data is cr3=%02x, sec=%02x, min=%02x, hr=%02x, "
    > "mday=%02x, wday=%02x, mon=%02x, year=%02x\n",
    > - __func__,
    > - buf[0], buf[1], buf[2],
    > - buf[3], buf[4], buf[5],
    > - buf[6], buf[7], buf[8], buf[9]);
    > -
    > + __func__, buf[PCF2127_REG_CTRL3], buf[PCF2127_REG_SC],
    > + buf[PCF2127_REG_MN], buf[PCF2127_REG_HR],
    > + buf[PCF2127_REG_DM], buf[PCF2127_REG_DW],
    > + buf[PCF2127_REG_MO], buf[PCF2127_REG_YR]);
    >
    > tm->tm_sec = bcd2bin(buf[PCF2127_REG_SC] & 0x7F);
    > tm->tm_min = bcd2bin(buf[PCF2127_REG_MN] & 0x7F);
    > --
    > 2.20.1
    >
    >

    --
    (english) http://www.livejournal.com/~pavelmachek
    (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
    [unhandled content-type:application/pgp-signature]
    \
     
     \ /
      Last update: 2020-01-26 11:27    [W:4.016 / U:0.048 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site