linux-rtc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Alexandre Belloni <alexandre.belloni@bootlin.com>
To: Bruno Thomsen <bruno.thomsen@gmail.com>
Cc: linux-rtc@vger.kernel.org, a.zummo@towertech.it,
	bth@kamstrup.com, u.kleine-koenig@pengutronix.de
Subject: Re: [PATCH 2/4] rtc: pcf2127: cleanup register and bit defines
Date: Tue, 23 Jul 2019 20:42:05 +0200	[thread overview]
Message-ID: <20190723184205.GL24911@piout.net> (raw)
In-Reply-To: <20190722155811.11980-3-bruno.thomsen@gmail.com>

On 22/07/2019 17:58:09+0200, Bruno Thomsen wrote:
> Cleanup defines before adding new features to driver.
> 

I think you need to elaborate on what is wrong with the current defines
because they seem fine to me as-is.

> Signed-off-by: Bruno Thomsen <bruno.thomsen@gmail.com>
> ---
>  drivers/rtc/rtc-pcf2127.c | 59 ++++++++++++++++++++++-----------------
>  1 file changed, 33 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/rtc/rtc-pcf2127.c b/drivers/rtc/rtc-pcf2127.c
> index 58eb96506e4b..cd8def79b379 100644
> --- a/drivers/rtc/rtc-pcf2127.c
> +++ b/drivers/rtc/rtc-pcf2127.c
> @@ -19,26 +19,32 @@
>  #include <linux/of.h>
>  #include <linux/regmap.h>
>  
> -#define PCF2127_REG_CTRL1       (0x00)  /* Control Register 1 */
> -#define PCF2127_REG_CTRL2       (0x01)  /* Control Register 2 */
> -
> -#define PCF2127_REG_CTRL3       (0x02)  /* Control Register 3 */
> -#define PCF2127_REG_CTRL3_BLF		BIT(2)
> -
> -#define PCF2127_REG_SC          (0x03)  /* datetime */
> -#define PCF2127_REG_MN          (0x04)
> -#define PCF2127_REG_HR          (0x05)
> -#define PCF2127_REG_DM          (0x06)
> -#define PCF2127_REG_DW          (0x07)
> -#define PCF2127_REG_MO          (0x08)
> -#define PCF2127_REG_YR          (0x09)
> -
> -/* the pcf2127 has 512 bytes nvmem, pcf2129 doesn't */
> -#define PCF2127_REG_RAM_addr_MSB       0x1a
> -#define PCF2127_REG_RAM_wrt_cmd        0x1c
> -#define PCF2127_REG_RAM_rd_cmd         0x1d
> +/* Control register 1 */
> +#define PCF2127_REG_CTRL1		0x00
> +/* Control register 2 */
> +#define PCF2127_REG_CTRL2		0x01
> +/* Control register 3 */
> +#define PCF2127_REG_CTRL3		0x02
> +#define PCF2127_BIT_CTRL3_BLF			BIT(2)
> +/* Time and date registers */
> +#define PCF2127_REG_SC			0x03
> +#define PCF2127_BIT_SC_OSF			BIT(7)
> +#define PCF2127_REG_MN			0x04
> +#define PCF2127_REG_HR			0x05
> +#define PCF2127_REG_DM			0x06
> +#define PCF2127_REG_DW			0x07
> +#define PCF2127_REG_MO			0x08
> +#define PCF2127_REG_YR			0x09
> +/*
> + * RAM registers
> + * PCF2127 has 512 bytes general-purpose static RAM (SRAM) that is
> + * battery backed and can survive a power outage.
> + * PCF2129 doesn't have this feature.
> + */
> +#define PCF2127_REG_RAM_ADDR_MSB	0x1A
> +#define PCF2127_REG_RAM_WRT_CMD		0x1C
> +#define PCF2127_REG_RAM_RD_CMD		0x1D
>  
> -#define PCF2127_OSF             BIT(7)  /* Oscillator Fail flag */
>  
>  struct pcf2127 {
>  	struct rtc_device *rtc;
> @@ -73,11 +79,12 @@ static int pcf2127_rtc_read_time(struct device *dev, struct rtc_time *tm)
>  		return ret;
>  	}
>  
> -	if (buf[PCF2127_REG_CTRL3] & PCF2127_REG_CTRL3_BLF)
> +	if (buf[PCF2127_REG_CTRL3] & PCF2127_BIT_CTRL3_BLF)
>  		dev_info(dev,
>  			"low voltage detected, check/replace RTC battery.\n");
>  
> -	if (buf[PCF2127_REG_SC] & PCF2127_OSF) {
> +	/* Clock integrity is not guaranteed when OSF flag is set. */
> +	if (buf[PCF2127_REG_SC] & PCF2127_BIT_SC_OSF) {
>  		/*
>  		 * no need clear the flag here,
>  		 * it will be cleared once the new date is saved
> @@ -166,7 +173,7 @@ static int pcf2127_rtc_ioctl(struct device *dev,
>  		if (ret)
>  			return ret;
>  
> -		touser = touser & PCF2127_REG_CTRL3_BLF ? 1 : 0;
> +		touser = touser & PCF2127_BIT_CTRL3_BLF ? 1 : 0;
>  
>  		if (copy_to_user((void __user *)arg, &touser, sizeof(int)))
>  			return -EFAULT;
> @@ -192,12 +199,12 @@ static int pcf2127_nvmem_read(void *priv, unsigned int offset,
>  	int ret;
>  	unsigned char offsetbuf[] = { offset >> 8, offset };
>  
> -	ret = regmap_bulk_write(pcf2127->regmap, PCF2127_REG_RAM_addr_MSB,
> +	ret = regmap_bulk_write(pcf2127->regmap, PCF2127_REG_RAM_ADDR_MSB,
>  				offsetbuf, 2);
>  	if (ret)
>  		return ret;
>  
> -	ret = regmap_bulk_read(pcf2127->regmap, PCF2127_REG_RAM_rd_cmd,
> +	ret = regmap_bulk_read(pcf2127->regmap, PCF2127_REG_RAM_RD_CMD,
>  			       val, bytes);
>  
>  	return ret ?: bytes;
> @@ -210,12 +217,12 @@ static int pcf2127_nvmem_write(void *priv, unsigned int offset,
>  	int ret;
>  	unsigned char offsetbuf[] = { offset >> 8, offset };
>  
> -	ret = regmap_bulk_write(pcf2127->regmap, PCF2127_REG_RAM_addr_MSB,
> +	ret = regmap_bulk_write(pcf2127->regmap, PCF2127_REG_RAM_ADDR_MSB,
>  				offsetbuf, 2);
>  	if (ret)
>  		return ret;
>  
> -	ret = regmap_bulk_write(pcf2127->regmap, PCF2127_REG_RAM_wrt_cmd,
> +	ret = regmap_bulk_write(pcf2127->regmap, PCF2127_REG_RAM_WRT_CMD,
>  				val, bytes);
>  
>  	return ret ?: bytes;
> -- 
> 2.21.0
> 

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

  reply	other threads:[~2019-07-23 18:42 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-22 15:58 [PATCH 0/4] rtc: pcf2127: tamper timestamp and watchdog feature support Bruno Thomsen
2019-07-22 15:58 ` [PATCH 1/4] rtc: pcf2127: convert to devm_rtc_allocate_device Bruno Thomsen
2019-07-22 15:58 ` [PATCH 2/4] rtc: pcf2127: cleanup register and bit defines Bruno Thomsen
2019-07-23 18:42   ` Alexandre Belloni [this message]
2019-07-22 15:58 ` [PATCH 3/4] rtc: pcf2127: add tamper detection support Bruno Thomsen
2019-07-22 15:58 ` [PATCH 4/4] rtc: pcf2127: add watchdog feature support Bruno Thomsen
2019-07-23 18:48   ` Alexandre Belloni
2019-07-24  7:18     ` Bruno Thomsen
2019-07-24 13:00       ` Alexandre Belloni
2019-07-23 14:13 ` [PATCH 0/4] rtc: pcf2127: tamper timestamp and " Bruno Thomsen
2019-07-23 18:40   ` Alexandre Belloni

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190723184205.GL24911@piout.net \
    --to=alexandre.belloni@bootlin.com \
    --cc=a.zummo@towertech.it \
    --cc=bruno.thomsen@gmail.com \
    --cc=bth@kamstrup.com \
    --cc=linux-rtc@vger.kernel.org \
    --cc=u.kleine-koenig@pengutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).