linux-kernel - Re: [PATCH 3/4] rtc: mcp795: fix month write resetting date to 1.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20161205152410.qj64rw67jdccyrab@piout.net>
Date:   Mon, 5 Dec 2016 16:24:10 +0100
From:   Alexandre Belloni <alexandre.belloni@...e-electrons.com>
To:     Emil Bartczak <emilbart@...il.com>
Cc:     a.zummo@...ertech.it, rtc-linux@...glegroups.com,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/4] rtc: mcp795: fix month write resetting date to 1.

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.

> +/* 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