On Sun, Jan 26, 2020 at 11:26:35AM +0100, Pavel Machek wrote: > On Fri 2020-01-24 10:31:31, Greg Kroah-Hartman wrote: > > From: Bruno Thomsen <bruno.thomsen@xxxxxxxxx> > > > > [ 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. Meta-comment: Making review comments on patches that are already in Linus's tree, is fine, but making them here seems a bit odd. Please just go find the original patch and respond there, as there's not much you can do here _way_ after the fact, as the change is already committed. thanks, greg k-h
