Hi,
On Mon, Dec 05, 2016 at 04:24:10PM +0100, Alexandre Belloni wrote:
> 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 <linux/spi/spi.h>
> > #include <linux/rtc.h>
> > #include <linux/of.h>
> > +#include <linux/delay.h>
> >
> > /* 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.
Ok,
>
> > +/* 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).
Ok, I will fix it in the next patchset.
>
> >
> > 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.
I came to the conclusion that it is better to remove clearing of bit EXTOSC in
function mcp795_stop_oscillator.
Because regarding datasheet, after power up the RTC chip (or reset) that bit
has value 0.
What do you think?
>
>
> --
> Alexandre Belloni, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com
Emil,
