From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751496AbcLEPYO (ORCPT ); Mon, 5 Dec 2016 10:24:14 -0500 Received: from mail.free-electrons.com ([62.4.15.54]:52985 "EHLO mail.free-electrons.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751080AbcLEPYM (ORCPT ); Mon, 5 Dec 2016 10:24:12 -0500 Date: Mon, 5 Dec 2016 16:24:10 +0100 From: Alexandre Belloni To: Emil Bartczak Cc: a.zummo@towertech.it, rtc-linux@googlegroups.com, linux-kernel@vger.kernel.org Subject: Re: [PATCH 3/4] rtc: mcp795: fix month write resetting date to 1. Message-ID: <20161205152410.qj64rw67jdccyrab@piout.net> References: <6a1e8548654fd8a5b65ba98f59b2b5ccb1544f7f.1480939487.git.emilbart@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <6a1e8548654fd8a5b65ba98f59b2b5ccb1544f7f.1480939487.git.emilbart@gmail.com> User-Agent: NeoMutt/20161104 (1.7.1) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 05/12/2016 at 14:11:52 +0100, Emil Bartczak wrote : > According to Microchip errata some combinations of date and month > values may result in the date being reset to 1, even if the date > is also written with the month (for example 31-07 or 31-08). > As a workaround avoid writing date and month values within the same > Write command. Instead, terminate the Write command after loading > the date and begin a new command to write the month. In addition, > disable the oscillator before loading the new values. This is done > by ensuring both the ST and EXTOSC bits are cleared and waiting for > the OSCON bit to clear. > --- > drivers/rtc/rtc-mcp795.c | 103 +++++++++++++++++++++++++++++++++++++---------- > 1 file changed, 82 insertions(+), 21 deletions(-) > > diff --git a/drivers/rtc/rtc-mcp795.c b/drivers/rtc/rtc-mcp795.c > index d15072c..c9ad46c 100644 > --- a/drivers/rtc/rtc-mcp795.c > +++ b/drivers/rtc/rtc-mcp795.c > @@ -21,25 +21,34 @@ > #include > #include > #include > +#include > > /* MCP795 Instructions, see datasheet table 3-1 */ > -#define MCP795_EEREAD 0x03 > -#define MCP795_EEWRITE 0x02 > -#define MCP795_EEWRDI 0x04 > -#define MCP795_EEWREN 0x06 > -#define MCP795_SRREAD 0x05 > -#define MCP795_SRWRITE 0x01 > -#define MCP795_READ 0x13 > -#define MCP795_WRITE 0x12 > -#define MCP795_UNLOCK 0x14 > -#define MCP795_IDWRITE 0x32 > -#define MCP795_IDREAD 0x33 > -#define MCP795_CLRWDT 0x44 > -#define MCP795_CLRRAM 0x54 > - > -#define MCP795_ST_BIT 0x80 > -#define MCP795_24_BIT 0x40 > -#define MCP795_LP_BIT 0x20 > +#define MCP795_EEREAD 0x03 > +#define MCP795_EEWRITE 0x02 > +#define MCP795_EEWRDI 0x04 > +#define MCP795_EEWREN 0x06 > +#define MCP795_SRREAD 0x05 > +#define MCP795_SRWRITE 0x01 > +#define MCP795_READ 0x13 > +#define MCP795_WRITE 0x12 > +#define MCP795_UNLOCK 0x14 > +#define MCP795_IDWRITE 0x32 > +#define MCP795_IDREAD 0x33 > +#define MCP795_CLRWDT 0x44 > +#define MCP795_CLRRAM 0x54 > + Please don't reindent, they are a separate block anyway. > +/* MCP795 RTCC registers, see datasheet table 4-1 */ > +#define MCP795_REG_SECONDS 0x01 > +#define MCP795_REG_DAY 0x04 > +#define MCP795_REG_MONTH 0x06 > +#define MCP795_REG_CONTROL 0x08 > + > +#define MCP795_ST_BIT 0x80 > +#define MCP795_24_BIT 0x40 > +#define MCP795_LP_BIT 0x20 > +#define MCP795_EXTOSC_BIT 0x08 > +#define MCP795_OSCON_BIT 0x20 Please use BIT() and that is valid for the first patch too). > > static int mcp795_rtcc_read(struct device *dev, u8 addr, u8 *buf, u8 count) > { > @@ -94,14 +103,51 @@ static int mcp795_rtcc_set_bits(struct device *dev, u8 addr, u8 mask, u8 state) > return ret; > } > > +static int mcp795_stop_oscillator(struct device *dev) > +{ > + int retries = 5; > + int ret; > + u8 data; > + > + ret = mcp795_rtcc_set_bits(dev, MCP795_REG_SECONDS, MCP795_ST_BIT, 0); > + if (ret) > + return ret; > + ret = mcp795_rtcc_set_bits(dev, MCP795_REG_CONTROL, MCP795_EXTOSC_BIT, 0); > + if (ret) > + return ret; > + do { > + usleep_range(700, 800); > + ret = mcp795_rtcc_read(dev, MCP795_REG_DAY, > + &data, sizeof(data)); > + if (ret) > + break; > + if (!(data & MCP795_OSCON_BIT)) > + break; > + > + } while (--retries); > + > + return !retries ? -EIO : ret; > +} > + > +static int mcp795_start_oscillator(struct device *dev) > +{ > + return mcp795_rtcc_set_bits(dev, MCP795_REG_SECONDS, > + MCP795_ST_BIT, MCP795_ST_BIT); You probably want to restore EXTOSC to its previous value here. -- Alexandre Belloni, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com