[PATCH v1 0/4] add alarm support for the rtc-ab-eoz9

[PATCH v1 0/4] add alarm support for the rtc-ab-eoz9

[Liam Beguin]

From: Liam Beguin <lvb@xiphos.com>

Add alarm support for the rtc-ab-eoz9.

The board used to test this series has the interrupt line of the RTC
connected to a circuit controlling the power of the board.

An event on the interrupt line while the board is off will power it on.
Because of these hardware limitations, the irq handler added in this
patch wasn't fully tested.

The alarm functionality was tested, with:

	$ date "2010-10-10 10:10"
	Sun Oct 10 10:10:00 UTC 2010
	$ /usr/sbin/rtcwake -u -d /dev/rtc0  -s20 --mode off
	[ ... ]
	$ # power on after 20 seconds


Liam Beguin (4):
  rtc: ab-eoz9: make use of regmap local variable
  rtc: ab-eoz9: set regmap max_register
  rtc: ab-eoz9: add alarm support
  rtc: ab-eoz9: make use of RTC_FEATURE_ALARM

 drivers/rtc/rtc-ab-eoz9.c | 149 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 141 insertions(+), 8 deletions(-)


base-commit: e49d033bddf5b565044e2abe4241353959bc9120
-- 
2.30.1.489.g328c10930387

[PATCH v1 1/4] rtc: ab-eoz9: make use of regmap local variable

[Liam Beguin]

From: Liam Beguin <lvb@xiphos.com>

Make use of the regmap local variable to shorten long lines.

Signed-off-by: Liam Beguin <lvb@xiphos.com>
---
 drivers/rtc/rtc-ab-eoz9.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/rtc/rtc-ab-eoz9.c b/drivers/rtc/rtc-ab-eoz9.c
index b20d8f26dcdb..2568984fb8d3 100644
--- a/drivers/rtc/rtc-ab-eoz9.c
+++ b/drivers/rtc/rtc-ab-eoz9.c
@@ -124,6 +124,7 @@ static int abeoz9_reset_validity(struct regmap *regmap)
 static int abeoz9_rtc_get_time(struct device *dev, struct rtc_time *tm)
 {
 	struct abeoz9_rtc_data *data = dev_get_drvdata(dev);
+	struct regmap *regmap = data->regmap;
 	u8 regs[ABEOZ9_SEC_LEN];
 	int ret;
 
@@ -131,9 +132,7 @@ static int abeoz9_rtc_get_time(struct device *dev, struct rtc_time *tm)
 	if (ret)
 		return ret;
 
-	ret = regmap_bulk_read(data->regmap, ABEOZ9_REG_SEC,
-			       regs,
-			       sizeof(regs));
+	ret = regmap_bulk_read(regmap, ABEOZ9_REG_SEC, regs, sizeof(regs));
 	if (ret) {
 		dev_err(dev, "reading RTC time failed (%d)\n", ret);
 		return ret;
@@ -174,10 +173,7 @@ static int abeoz9_rtc_set_time(struct device *dev, struct rtc_time *tm)
 	regs[ABEOZ9_REG_MONTHS - ABEOZ9_REG_SEC] = bin2bcd(tm->tm_mon + 1);
 	regs[ABEOZ9_REG_YEARS - ABEOZ9_REG_SEC] = bin2bcd(tm->tm_year - 100);
 
-	ret = regmap_bulk_write(data->regmap, ABEOZ9_REG_SEC,
-				regs,
-				sizeof(regs));
-
+	ret = regmap_bulk_write(regmap, ABEOZ9_REG_SEC, regs, sizeof(regs));
 	if (ret) {
 		dev_err(dev, "set RTC time failed (%d)\n", ret);
 		return ret;
-- 
2.30.1.489.g328c10930387

[PATCH v1 2/4] rtc: ab-eoz9: set regmap max_register

[Liam Beguin]

From: Liam Beguin <lvb@xiphos.com>

Set regmap's max_register property to allow users to dump registers
using debufgs.

Signed-off-by: Liam Beguin <lvb@xiphos.com>
---
 drivers/rtc/rtc-ab-eoz9.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/rtc/rtc-ab-eoz9.c b/drivers/rtc/rtc-ab-eoz9.c
index 2568984fb8d3..379a231f673c 100644
--- a/drivers/rtc/rtc-ab-eoz9.c
+++ b/drivers/rtc/rtc-ab-eoz9.c
@@ -260,6 +260,7 @@ static const struct rtc_class_ops rtc_ops = {
 static const struct regmap_config abeoz9_rtc_regmap_config = {
 	.reg_bits = 8,
 	.val_bits = 8,
+	.max_register = 0x3f,
 };
 
 #if IS_REACHABLE(CONFIG_HWMON)
-- 
2.30.1.489.g328c10930387

[PATCH v1 3/4] rtc: ab-eoz9: add alarm support

[Liam Beguin]

From: Liam Beguin <lvb@xiphos.com>

Add alarm support for the rtc-ab-eoz9.

Signed-off-by: Liam Beguin <lvb@xiphos.com>
---
 drivers/rtc/rtc-ab-eoz9.c | 140 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 140 insertions(+)

diff --git a/drivers/rtc/rtc-ab-eoz9.c b/drivers/rtc/rtc-ab-eoz9.c
index 379a231f673c..4c8ba62fb1c6 100644
--- a/drivers/rtc/rtc-ab-eoz9.c
+++ b/drivers/rtc/rtc-ab-eoz9.c
@@ -11,6 +11,7 @@
 #include <linux/bcd.h>
 #include <linux/of.h>
 #include <linux/regmap.h>
+#include <linux/bitfield.h>
 #include <linux/hwmon.h>
 #include <linux/hwmon-sysfs.h>
 
@@ -57,6 +58,24 @@
 
 #define ABEOZ9_SEC_LEN			7
 
+#define ABEOZ9_REG_ALARM_SEC		0x10
+#define ABEOZ9_BIT_ALARM_SEC		GENMASK(6, 0)
+#define ABEOZ9_REG_ALARM_MIN		0x11
+#define ABEOZ9_BIT_ALARM_MIN		GENMASK(6, 0)
+#define ABEOZ9_REG_ALARM_HOURS		0x12
+#define ABEOZ9_BIT_ALARM_HOURS_PM	BIT(5)
+#define ABEOZ9_BIT_ALARM_HOURS		GENMASK(4, 0)
+#define ABEOZ9_REG_ALARM_DAYS		0x13
+#define ABEOZ9_BIT_ALARM_DAYS		GENMASK(5, 0)
+#define ABEOZ9_REG_ALARM_WEEKDAYS	0x14
+#define ABEOZ9_BIT_ALARM_WEEKDAYS	GENMASK(2, 0)
+#define ABEOZ9_REG_ALARM_MONTHS		0x15
+#define ABEOZ9_BIT_ALARM_MONTHS		GENMASK(4, 0)
+#define ABEOZ9_REG_ALARM_YEARS		0x16
+
+#define ABEOZ9_ALARM_LEN		7
+#define ABEOZ9_BIT_ALARM_AE		BIT(7)
+
 #define ABEOZ9_REG_REG_TEMP		0x20
 #define ABEOZ953_TEMP_MAX		120
 #define ABEOZ953_TEMP_MIN		-60
@@ -182,6 +201,102 @@ static int abeoz9_rtc_set_time(struct device *dev, struct rtc_time *tm)
 	return abeoz9_reset_validity(regmap);
 }
 
+static int abeoz9_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alarm)
+{
+	struct abeoz9_rtc_data *data = dev_get_drvdata(dev);
+	struct regmap *regmap = data->regmap;
+	u8 regs[ABEOZ9_ALARM_LEN];
+	u8 val[2];
+	int ret;
+
+	ret = abeoz9_check_validity(dev);
+	if (ret)
+		return ret;
+
+	ret = regmap_bulk_read(regmap, ABEOZ9_REG_CTRL_INT, val, sizeof(val));
+	if (ret)
+		return ret;
+
+	alarm->enabled = val[0] & ABEOZ9_REG_CTRL_INT_AIE;
+	alarm->pending = val[1] & ABEOZ9_REG_CTRL_INT_FLAG_AF;
+
+	ret = regmap_bulk_read(regmap, ABEOZ9_REG_ALARM_SEC, regs, sizeof(regs));
+	if (ret)
+		return ret;
+
+	alarm->time.tm_sec = bcd2bin(FIELD_GET(ABEOZ9_BIT_ALARM_SEC, regs[0]));
+	alarm->time.tm_min = bcd2bin(FIELD_GET(ABEOZ9_BIT_ALARM_MIN, regs[1]));
+	alarm->time.tm_hour = bcd2bin(FIELD_GET(ABEOZ9_BIT_ALARM_HOURS, regs[2]));
+	if (FIELD_GET(ABEOZ9_BIT_ALARM_HOURS_PM, regs[2]))
+		alarm->time.tm_hour += 12;
+
+	alarm->time.tm_mday = bcd2bin(FIELD_GET(ABEOZ9_BIT_ALARM_DAYS, regs[3]));
+
+	return 0;
+}
+
+static int abeoz9_rtc_alarm_irq_enable(struct device *dev, u32 enable)
+{
+	struct abeoz9_rtc_data *data = dev_get_drvdata(dev);
+
+	return regmap_update_bits(data->regmap, ABEOZ9_REG_CTRL_INT,
+				  ABEOZ9_REG_CTRL_INT_AIE,
+				  FIELD_PREP(ABEOZ9_REG_CTRL_INT_AIE, enable));
+}
+
+static int abeoz9_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alarm)
+{
+	struct abeoz9_rtc_data *data = dev_get_drvdata(dev);
+	u8 regs[ABEOZ9_ALARM_LEN] = {0};
+	int ret;
+
+	ret = regmap_update_bits(data->regmap, ABEOZ9_REG_CTRL_INT_FLAG,
+				 ABEOZ9_REG_CTRL_INT_FLAG_AF, 0);
+	if (ret)
+		return ret;
+
+	regs[0] = ABEOZ9_BIT_ALARM_AE | FIELD_PREP(ABEOZ9_BIT_ALARM_SEC,
+						   bin2bcd(alarm->time.tm_sec));
+	regs[1] = ABEOZ9_BIT_ALARM_AE | FIELD_PREP(ABEOZ9_BIT_ALARM_MIN,
+						   bin2bcd(alarm->time.tm_min));
+	regs[2] = ABEOZ9_BIT_ALARM_AE | FIELD_PREP(ABEOZ9_BIT_ALARM_HOURS,
+						   bin2bcd(alarm->time.tm_hour));
+	regs[3] = ABEOZ9_BIT_ALARM_AE | FIELD_PREP(ABEOZ9_BIT_ALARM_DAYS,
+						   bin2bcd(alarm->time.tm_mday));
+
+	ret = regmap_bulk_write(data->regmap, ABEOZ9_REG_ALARM_SEC, regs,
+				sizeof(regs));
+	if (ret)
+		return ret;
+
+	ret = abeoz9_reset_validity(data->regmap);
+	if (ret)
+		return ret;
+
+	return abeoz9_rtc_alarm_irq_enable(dev, alarm->enabled);
+}
+
+static irqreturn_t abeoz9_rtc_irq(int irq, void *dev)
+{
+	struct abeoz9_rtc_data *data = dev_get_drvdata(dev);
+	unsigned int val;
+	int ret;
+
+	ret = regmap_read(data->regmap, ABEOZ9_REG_CTRL_INT_FLAG, &val);
+	if (ret)
+		return IRQ_NONE;
+
+	if (!FIELD_GET(ABEOZ9_REG_CTRL_INT_FLAG_AF, val))
+		return IRQ_NONE;
+
+	regmap_update_bits(data->regmap, ABEOZ9_REG_CTRL_INT_FLAG,
+			   ABEOZ9_REG_CTRL_INT_FLAG_AF, 0);
+
+	rtc_update_irq(data->rtc, 1, RTC_IRQF | RTC_AF);
+
+	return IRQ_HANDLED;
+}
+
 static int abeoz9_trickle_parse_dt(struct device_node *node)
 {
 	u32 ohms = 0;
@@ -257,6 +372,14 @@ static const struct rtc_class_ops rtc_ops = {
 	.set_time  = abeoz9_rtc_set_time,
 };
 
+static const struct rtc_class_ops rtc_alarm_ops = {
+	.read_time = abeoz9_rtc_get_time,
+	.set_time = abeoz9_rtc_set_time,
+	.read_alarm = abeoz9_rtc_read_alarm,
+	.set_alarm = abeoz9_rtc_set_alarm,
+	.alarm_irq_enable = abeoz9_rtc_alarm_irq_enable,
+};
+
 static const struct regmap_config abeoz9_rtc_regmap_config = {
 	.reg_bits = 8,
 	.val_bits = 8,
@@ -416,6 +539,23 @@ static int abeoz9_probe(struct i2c_client *client,
 	data->rtc->ops = &rtc_ops;
 	data->rtc->range_min = RTC_TIMESTAMP_BEGIN_2000;
 	data->rtc->range_max = RTC_TIMESTAMP_END_2099;
+	data->rtc->uie_unsupported = 1;
+
+	if (client->irq > 0) {
+		ret = devm_request_threaded_irq(dev, client->irq, NULL,
+						abeoz9_rtc_irq,
+						IRQF_TRIGGER_LOW | IRQF_ONESHOT,
+						dev_name(dev), dev);
+		if (ret) {
+			dev_err(dev, "failed to request alarm irq\n");
+			return ret;
+		}
+	}
+
+	if (client->irq > 0 || device_property_read_bool(dev, "wakeup-source")) {
+		ret = device_init_wakeup(dev, true);
+		data->rtc->ops = &rtc_alarm_ops;
+	}
 
 	ret = devm_rtc_register_device(data->rtc);
 	if (ret)
-- 
2.30.1.489.g328c10930387

[PATCH v1 4/4] rtc: ab-eoz9: make use of RTC_FEATURE_ALARM

[Liam Beguin]

From: Liam Beguin <lvb@xiphos.com>

Move the alarm callbacks in rtc_ops and use RTC_FEATURE_ALARM to notify
the core whether alarm capabilities are available or not.

Signed-off-by: Liam Beguin <lvb@xiphos.com>
---
 drivers/rtc/rtc-ab-eoz9.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/rtc/rtc-ab-eoz9.c b/drivers/rtc/rtc-ab-eoz9.c
index 4c8ba62fb1c6..a8256747d95d 100644
--- a/drivers/rtc/rtc-ab-eoz9.c
+++ b/drivers/rtc/rtc-ab-eoz9.c
@@ -368,11 +368,6 @@ static int abeoz9_rtc_setup(struct device *dev, struct device_node *node)
 }
 
 static const struct rtc_class_ops rtc_ops = {
-	.read_time = abeoz9_rtc_get_time,
-	.set_time  = abeoz9_rtc_set_time,
-};
-
-static const struct rtc_class_ops rtc_alarm_ops = {
 	.read_time = abeoz9_rtc_get_time,
 	.set_time = abeoz9_rtc_set_time,
 	.read_alarm = abeoz9_rtc_read_alarm,
@@ -540,6 +535,7 @@ static int abeoz9_probe(struct i2c_client *client,
 	data->rtc->range_min = RTC_TIMESTAMP_BEGIN_2000;
 	data->rtc->range_max = RTC_TIMESTAMP_END_2099;
 	data->rtc->uie_unsupported = 1;
+	clear_bit(RTC_FEATURE_ALARM, data->rtc->features);
 
 	if (client->irq > 0) {
 		ret = devm_request_threaded_irq(dev, client->irq, NULL,
@@ -554,7 +550,7 @@ static int abeoz9_probe(struct i2c_client *client,
 
 	if (client->irq > 0 || device_property_read_bool(dev, "wakeup-source")) {
 		ret = device_init_wakeup(dev, true);
-		data->rtc->ops = &rtc_alarm_ops;
+		set_bit(RTC_FEATURE_ALARM, data->rtc->features);
 	}
 
 	ret = devm_rtc_register_device(data->rtc);
-- 
2.30.1.489.g328c10930387

Re: [PATCH v1 1/4] rtc: ab-eoz9: make use of regmap local variable

[Alexandre Belloni]

Hello,

On 05/04/2021 10:13:31-0400, Liam Beguin wrote:
> From: Liam Beguin <lvb@xiphos.com>
> 
> Make use of the regmap local variable to shorten long lines.
> 

As you are still using data->regmap in the subsequent patches, I don't
see the point of this one. Moreover, you could have everything on one
line without the intermediate variable.

> Signed-off-by: Liam Beguin <lvb@xiphos.com>
> ---
>  drivers/rtc/rtc-ab-eoz9.c | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/rtc/rtc-ab-eoz9.c b/drivers/rtc/rtc-ab-eoz9.c
> index b20d8f26dcdb..2568984fb8d3 100644
> --- a/drivers/rtc/rtc-ab-eoz9.c
> +++ b/drivers/rtc/rtc-ab-eoz9.c
> @@ -124,6 +124,7 @@ static int abeoz9_reset_validity(struct regmap *regmap)
>  static int abeoz9_rtc_get_time(struct device *dev, struct rtc_time *tm)
>  {
>  	struct abeoz9_rtc_data *data = dev_get_drvdata(dev);
> +	struct regmap *regmap = data->regmap;
>  	u8 regs[ABEOZ9_SEC_LEN];
>  	int ret;
>  
> @@ -131,9 +132,7 @@ static int abeoz9_rtc_get_time(struct device *dev, struct rtc_time *tm)
>  	if (ret)
>  		return ret;
>  
> -	ret = regmap_bulk_read(data->regmap, ABEOZ9_REG_SEC,
> -			       regs,
> -			       sizeof(regs));
> +	ret = regmap_bulk_read(regmap, ABEOZ9_REG_SEC, regs, sizeof(regs));
>  	if (ret) {
>  		dev_err(dev, "reading RTC time failed (%d)\n", ret);
>  		return ret;
> @@ -174,10 +173,7 @@ static int abeoz9_rtc_set_time(struct device *dev, struct rtc_time *tm)
>  	regs[ABEOZ9_REG_MONTHS - ABEOZ9_REG_SEC] = bin2bcd(tm->tm_mon + 1);
>  	regs[ABEOZ9_REG_YEARS - ABEOZ9_REG_SEC] = bin2bcd(tm->tm_year - 100);
>  
> -	ret = regmap_bulk_write(data->regmap, ABEOZ9_REG_SEC,
> -				regs,
> -				sizeof(regs));
> -
> +	ret = regmap_bulk_write(regmap, ABEOZ9_REG_SEC, regs, sizeof(regs));
>  	if (ret) {
>  		dev_err(dev, "set RTC time failed (%d)\n", ret);
>  		return ret;
> -- 
> 2.30.1.489.g328c10930387
> 

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

Re: [PATCH v1 3/4] rtc: ab-eoz9: add alarm support

[Alexandre Belloni]

On 05/04/2021 10:13:33-0400, Liam Beguin wrote:
> From: Liam Beguin <lvb@xiphos.com>
> 
> Add alarm support for the rtc-ab-eoz9.
> 
> Signed-off-by: Liam Beguin <lvb@xiphos.com>
> ---
>  drivers/rtc/rtc-ab-eoz9.c | 140 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 140 insertions(+)
> 
> diff --git a/drivers/rtc/rtc-ab-eoz9.c b/drivers/rtc/rtc-ab-eoz9.c
> index 379a231f673c..4c8ba62fb1c6 100644
> --- a/drivers/rtc/rtc-ab-eoz9.c
> +++ b/drivers/rtc/rtc-ab-eoz9.c
> @@ -11,6 +11,7 @@
>  #include <linux/bcd.h>
>  #include <linux/of.h>
>  #include <linux/regmap.h>
> +#include <linux/bitfield.h>
>  #include <linux/hwmon.h>
>  #include <linux/hwmon-sysfs.h>
>  
> @@ -57,6 +58,24 @@
>  
>  #define ABEOZ9_SEC_LEN			7
>  
> +#define ABEOZ9_REG_ALARM_SEC		0x10
> +#define ABEOZ9_BIT_ALARM_SEC		GENMASK(6, 0)
> +#define ABEOZ9_REG_ALARM_MIN		0x11
> +#define ABEOZ9_BIT_ALARM_MIN		GENMASK(6, 0)
> +#define ABEOZ9_REG_ALARM_HOURS		0x12
> +#define ABEOZ9_BIT_ALARM_HOURS_PM	BIT(5)
> +#define ABEOZ9_BIT_ALARM_HOURS		GENMASK(4, 0)
> +#define ABEOZ9_REG_ALARM_DAYS		0x13
> +#define ABEOZ9_BIT_ALARM_DAYS		GENMASK(5, 0)
> +#define ABEOZ9_REG_ALARM_WEEKDAYS	0x14
> +#define ABEOZ9_BIT_ALARM_WEEKDAYS	GENMASK(2, 0)
> +#define ABEOZ9_REG_ALARM_MONTHS		0x15
> +#define ABEOZ9_BIT_ALARM_MONTHS		GENMASK(4, 0)
> +#define ABEOZ9_REG_ALARM_YEARS		0x16
> +
> +#define ABEOZ9_ALARM_LEN		7
> +#define ABEOZ9_BIT_ALARM_AE		BIT(7)
> +
>  #define ABEOZ9_REG_REG_TEMP		0x20
>  #define ABEOZ953_TEMP_MAX		120
>  #define ABEOZ953_TEMP_MIN		-60
> @@ -182,6 +201,102 @@ static int abeoz9_rtc_set_time(struct device *dev, struct rtc_time *tm)
>  	return abeoz9_reset_validity(regmap);
>  }
>  
> +static int abeoz9_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alarm)
> +{
> +	struct abeoz9_rtc_data *data = dev_get_drvdata(dev);
> +	struct regmap *regmap = data->regmap;
> +	u8 regs[ABEOZ9_ALARM_LEN];
> +	u8 val[2];
> +	int ret;
> +
> +	ret = abeoz9_check_validity(dev);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_bulk_read(regmap, ABEOZ9_REG_CTRL_INT, val, sizeof(val));
> +	if (ret)
> +		return ret;
> +
> +	alarm->enabled = val[0] & ABEOZ9_REG_CTRL_INT_AIE;
> +	alarm->pending = val[1] & ABEOZ9_REG_CTRL_INT_FLAG_AF;
> +
> +	ret = regmap_bulk_read(regmap, ABEOZ9_REG_ALARM_SEC, regs, sizeof(regs));
> +	if (ret)
> +		return ret;
> +
> +	alarm->time.tm_sec = bcd2bin(FIELD_GET(ABEOZ9_BIT_ALARM_SEC, regs[0]));
> +	alarm->time.tm_min = bcd2bin(FIELD_GET(ABEOZ9_BIT_ALARM_MIN, regs[1]));
> +	alarm->time.tm_hour = bcd2bin(FIELD_GET(ABEOZ9_BIT_ALARM_HOURS, regs[2]));
> +	if (FIELD_GET(ABEOZ9_BIT_ALARM_HOURS_PM, regs[2]))
> +		alarm->time.tm_hour += 12;
> +
> +	alarm->time.tm_mday = bcd2bin(FIELD_GET(ABEOZ9_BIT_ALARM_DAYS, regs[3]));
> +
> +	return 0;
> +}
> +
> +static int abeoz9_rtc_alarm_irq_enable(struct device *dev, u32 enable)
> +{
> +	struct abeoz9_rtc_data *data = dev_get_drvdata(dev);
> +
> +	return regmap_update_bits(data->regmap, ABEOZ9_REG_CTRL_INT,
> +				  ABEOZ9_REG_CTRL_INT_AIE,
> +				  FIELD_PREP(ABEOZ9_REG_CTRL_INT_AIE, enable));
> +}
> +
> +static int abeoz9_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alarm)
> +{
> +	struct abeoz9_rtc_data *data = dev_get_drvdata(dev);
> +	u8 regs[ABEOZ9_ALARM_LEN] = {0};
> +	int ret;
> +
> +	ret = regmap_update_bits(data->regmap, ABEOZ9_REG_CTRL_INT_FLAG,
> +				 ABEOZ9_REG_CTRL_INT_FLAG_AF, 0);
> +	if (ret)
> +		return ret;
> +
> +	regs[0] = ABEOZ9_BIT_ALARM_AE | FIELD_PREP(ABEOZ9_BIT_ALARM_SEC,
> +						   bin2bcd(alarm->time.tm_sec));
> +	regs[1] = ABEOZ9_BIT_ALARM_AE | FIELD_PREP(ABEOZ9_BIT_ALARM_MIN,
> +						   bin2bcd(alarm->time.tm_min));
> +	regs[2] = ABEOZ9_BIT_ALARM_AE | FIELD_PREP(ABEOZ9_BIT_ALARM_HOURS,
> +						   bin2bcd(alarm->time.tm_hour));
> +	regs[3] = ABEOZ9_BIT_ALARM_AE | FIELD_PREP(ABEOZ9_BIT_ALARM_DAYS,
> +						   bin2bcd(alarm->time.tm_mday));
> +
> +	ret = regmap_bulk_write(data->regmap, ABEOZ9_REG_ALARM_SEC, regs,
> +				sizeof(regs));
> +	if (ret)
> +		return ret;
> +
> +	ret = abeoz9_reset_validity(data->regmap);

This is not right, this must not be used unless the time set on the RTC
is correct which you don't know at that time.

> +	if (ret)
> +		return ret;
> +
> +	return abeoz9_rtc_alarm_irq_enable(dev, alarm->enabled);
> +}
> +
> +static irqreturn_t abeoz9_rtc_irq(int irq, void *dev)
> +{
> +	struct abeoz9_rtc_data *data = dev_get_drvdata(dev);
> +	unsigned int val;
> +	int ret;
> +
> +	ret = regmap_read(data->regmap, ABEOZ9_REG_CTRL_INT_FLAG, &val);
> +	if (ret)
> +		return IRQ_NONE;
> +
> +	if (!FIELD_GET(ABEOZ9_REG_CTRL_INT_FLAG_AF, val))
> +		return IRQ_NONE;
> +
> +	regmap_update_bits(data->regmap, ABEOZ9_REG_CTRL_INT_FLAG,
> +			   ABEOZ9_REG_CTRL_INT_FLAG_AF, 0);
> +
> +	rtc_update_irq(data->rtc, 1, RTC_IRQF | RTC_AF);
> +
> +	return IRQ_HANDLED;
> +}
> +
>  static int abeoz9_trickle_parse_dt(struct device_node *node)
>  {
>  	u32 ohms = 0;
> @@ -257,6 +372,14 @@ static const struct rtc_class_ops rtc_ops = {
>  	.set_time  = abeoz9_rtc_set_time,
>  };
>  
> +static const struct rtc_class_ops rtc_alarm_ops = {
> +	.read_time = abeoz9_rtc_get_time,
> +	.set_time = abeoz9_rtc_set_time,
> +	.read_alarm = abeoz9_rtc_read_alarm,
> +	.set_alarm = abeoz9_rtc_set_alarm,
> +	.alarm_irq_enable = abeoz9_rtc_alarm_irq_enable,
> +};
> +
>  static const struct regmap_config abeoz9_rtc_regmap_config = {
>  	.reg_bits = 8,
>  	.val_bits = 8,
> @@ -416,6 +539,23 @@ static int abeoz9_probe(struct i2c_client *client,
>  	data->rtc->ops = &rtc_ops;
>  	data->rtc->range_min = RTC_TIMESTAMP_BEGIN_2000;
>  	data->rtc->range_max = RTC_TIMESTAMP_END_2099;
> +	data->rtc->uie_unsupported = 1;
> +
> +	if (client->irq > 0) {
> +		ret = devm_request_threaded_irq(dev, client->irq, NULL,
> +						abeoz9_rtc_irq,
> +						IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> +						dev_name(dev), dev);
> +		if (ret) {
> +			dev_err(dev, "failed to request alarm irq\n");
> +			return ret;
> +		}
> +	}
> +
> +	if (client->irq > 0 || device_property_read_bool(dev, "wakeup-source")) {
> +		ret = device_init_wakeup(dev, true);
> +		data->rtc->ops = &rtc_alarm_ops;
> +	}
>  
>  	ret = devm_rtc_register_device(data->rtc);
>  	if (ret)
> -- 
> 2.30.1.489.g328c10930387
> 

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

Re: [PATCH v1 4/4] rtc: ab-eoz9: make use of RTC_FEATURE_ALARM

[Alexandre Belloni]

On 05/04/2021 10:13:34-0400, Liam Beguin wrote:
> From: Liam Beguin <lvb@xiphos.com>
> 
> Move the alarm callbacks in rtc_ops and use RTC_FEATURE_ALARM to notify
> the core whether alarm capabilities are available or not.
> 

Please squash that one with the previous one.

alse, the series looks good, thanks for working on that!

> Signed-off-by: Liam Beguin <lvb@xiphos.com>
> ---
>  drivers/rtc/rtc-ab-eoz9.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/rtc/rtc-ab-eoz9.c b/drivers/rtc/rtc-ab-eoz9.c
> index 4c8ba62fb1c6..a8256747d95d 100644
> --- a/drivers/rtc/rtc-ab-eoz9.c
> +++ b/drivers/rtc/rtc-ab-eoz9.c
> @@ -368,11 +368,6 @@ static int abeoz9_rtc_setup(struct device *dev, struct device_node *node)
>  }
>  
>  static const struct rtc_class_ops rtc_ops = {
> -	.read_time = abeoz9_rtc_get_time,
> -	.set_time  = abeoz9_rtc_set_time,
> -};
> -
> -static const struct rtc_class_ops rtc_alarm_ops = {
>  	.read_time = abeoz9_rtc_get_time,
>  	.set_time = abeoz9_rtc_set_time,
>  	.read_alarm = abeoz9_rtc_read_alarm,
> @@ -540,6 +535,7 @@ static int abeoz9_probe(struct i2c_client *client,
>  	data->rtc->range_min = RTC_TIMESTAMP_BEGIN_2000;
>  	data->rtc->range_max = RTC_TIMESTAMP_END_2099;
>  	data->rtc->uie_unsupported = 1;
> +	clear_bit(RTC_FEATURE_ALARM, data->rtc->features);
>  
>  	if (client->irq > 0) {
>  		ret = devm_request_threaded_irq(dev, client->irq, NULL,
> @@ -554,7 +550,7 @@ static int abeoz9_probe(struct i2c_client *client,
>  
>  	if (client->irq > 0 || device_property_read_bool(dev, "wakeup-source")) {
>  		ret = device_init_wakeup(dev, true);
> -		data->rtc->ops = &rtc_alarm_ops;
> +		set_bit(RTC_FEATURE_ALARM, data->rtc->features);
>  	}
>  
>  	ret = devm_rtc_register_device(data->rtc);
> -- 
> 2.30.1.489.g328c10930387
> 

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

Re: [PATCH v1 1/4] rtc: ab-eoz9: make use of regmap local variable

[Liam Beguin]

Hi Alexandre,

On Mon Apr 5, 2021 at 1:36 PM EDT, Alexandre Belloni wrote:
> Hello,
>
> On 05/04/2021 10:13:31-0400, Liam Beguin wrote:
> > From: Liam Beguin <lvb@xiphos.com>
> > 
> > Make use of the regmap local variable to shorten long lines.
> > 
>
> As you are still using data->regmap in the subsequent patches, I don't
> see the point of this one. Moreover, you could have everything on one
> line without the intermediate variable.
>

I'm not particularly attached to this change. It only started because
regmap wasn't used in abeoz9_rtc_set_time. I'll drop it.

Liam

> > Signed-off-by: Liam Beguin <lvb@xiphos.com>
> > ---
> >  drivers/rtc/rtc-ab-eoz9.c | 10 +++-------
> >  1 file changed, 3 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/rtc/rtc-ab-eoz9.c b/drivers/rtc/rtc-ab-eoz9.c
> > index b20d8f26dcdb..2568984fb8d3 100644
> > --- a/drivers/rtc/rtc-ab-eoz9.c
> > +++ b/drivers/rtc/rtc-ab-eoz9.c
> > @@ -124,6 +124,7 @@ static int abeoz9_reset_validity(struct regmap *regmap)
> >  static int abeoz9_rtc_get_time(struct device *dev, struct rtc_time *tm)
> >  {
> >  	struct abeoz9_rtc_data *data = dev_get_drvdata(dev);
> > +	struct regmap *regmap = data->regmap;
> >  	u8 regs[ABEOZ9_SEC_LEN];
> >  	int ret;
> >  
> > @@ -131,9 +132,7 @@ static int abeoz9_rtc_get_time(struct device *dev, struct rtc_time *tm)
> >  	if (ret)
> >  		return ret;
> >  
> > -	ret = regmap_bulk_read(data->regmap, ABEOZ9_REG_SEC,
> > -			       regs,
> > -			       sizeof(regs));
> > +	ret = regmap_bulk_read(regmap, ABEOZ9_REG_SEC, regs, sizeof(regs));
> >  	if (ret) {
> >  		dev_err(dev, "reading RTC time failed (%d)\n", ret);
> >  		return ret;
> > @@ -174,10 +173,7 @@ static int abeoz9_rtc_set_time(struct device *dev, struct rtc_time *tm)
> >  	regs[ABEOZ9_REG_MONTHS - ABEOZ9_REG_SEC] = bin2bcd(tm->tm_mon + 1);
> >  	regs[ABEOZ9_REG_YEARS - ABEOZ9_REG_SEC] = bin2bcd(tm->tm_year - 100);
> >  
> > -	ret = regmap_bulk_write(data->regmap, ABEOZ9_REG_SEC,
> > -				regs,
> > -				sizeof(regs));
> > -
> > +	ret = regmap_bulk_write(regmap, ABEOZ9_REG_SEC, regs, sizeof(regs));
> >  	if (ret) {
> >  		dev_err(dev, "set RTC time failed (%d)\n", ret);
> >  		return ret;
> > -- 
> > 2.30.1.489.g328c10930387
> > 
>
> --
> Alexandre Belloni, co-owner and COO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

Re: [PATCH v1 3/4] rtc: ab-eoz9: add alarm support

[Liam Beguin]

Hi Alexandre,

On Mon Apr 5, 2021 at 1:42 PM EDT, Alexandre Belloni wrote:
> On 05/04/2021 10:13:33-0400, Liam Beguin wrote:
> > From: Liam Beguin <lvb@xiphos.com>
> > 
> > Add alarm support for the rtc-ab-eoz9.
> > 
> > Signed-off-by: Liam Beguin <lvb@xiphos.com>
> > ---
> >  drivers/rtc/rtc-ab-eoz9.c | 140 ++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 140 insertions(+)
> > 
> > diff --git a/drivers/rtc/rtc-ab-eoz9.c b/drivers/rtc/rtc-ab-eoz9.c
> > index 379a231f673c..4c8ba62fb1c6 100644
> > --- a/drivers/rtc/rtc-ab-eoz9.c
> > +++ b/drivers/rtc/rtc-ab-eoz9.c
> > @@ -11,6 +11,7 @@
> >  #include <linux/bcd.h>
> >  #include <linux/of.h>
> >  #include <linux/regmap.h>
> > +#include <linux/bitfield.h>
> >  #include <linux/hwmon.h>
> >  #include <linux/hwmon-sysfs.h>
> >  
> > @@ -57,6 +58,24 @@
> >  
> >  #define ABEOZ9_SEC_LEN			7
> >  
> > +#define ABEOZ9_REG_ALARM_SEC		0x10
> > +#define ABEOZ9_BIT_ALARM_SEC		GENMASK(6, 0)
> > +#define ABEOZ9_REG_ALARM_MIN		0x11
> > +#define ABEOZ9_BIT_ALARM_MIN		GENMASK(6, 0)
> > +#define ABEOZ9_REG_ALARM_HOURS		0x12
> > +#define ABEOZ9_BIT_ALARM_HOURS_PM	BIT(5)
> > +#define ABEOZ9_BIT_ALARM_HOURS		GENMASK(4, 0)
> > +#define ABEOZ9_REG_ALARM_DAYS		0x13
> > +#define ABEOZ9_BIT_ALARM_DAYS		GENMASK(5, 0)
> > +#define ABEOZ9_REG_ALARM_WEEKDAYS	0x14
> > +#define ABEOZ9_BIT_ALARM_WEEKDAYS	GENMASK(2, 0)
> > +#define ABEOZ9_REG_ALARM_MONTHS		0x15
> > +#define ABEOZ9_BIT_ALARM_MONTHS		GENMASK(4, 0)
> > +#define ABEOZ9_REG_ALARM_YEARS		0x16
> > +
> > +#define ABEOZ9_ALARM_LEN		7
> > +#define ABEOZ9_BIT_ALARM_AE		BIT(7)
> > +
> >  #define ABEOZ9_REG_REG_TEMP		0x20
> >  #define ABEOZ953_TEMP_MAX		120
> >  #define ABEOZ953_TEMP_MIN		-60
> > @@ -182,6 +201,102 @@ static int abeoz9_rtc_set_time(struct device *dev, struct rtc_time *tm)
> >  	return abeoz9_reset_validity(regmap);
> >  }
> >  
> > +static int abeoz9_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alarm)
> > +{
> > +	struct abeoz9_rtc_data *data = dev_get_drvdata(dev);
> > +	struct regmap *regmap = data->regmap;
> > +	u8 regs[ABEOZ9_ALARM_LEN];
> > +	u8 val[2];
> > +	int ret;
> > +
> > +	ret = abeoz9_check_validity(dev);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = regmap_bulk_read(regmap, ABEOZ9_REG_CTRL_INT, val, sizeof(val));
> > +	if (ret)
> > +		return ret;
> > +
> > +	alarm->enabled = val[0] & ABEOZ9_REG_CTRL_INT_AIE;
> > +	alarm->pending = val[1] & ABEOZ9_REG_CTRL_INT_FLAG_AF;
> > +
> > +	ret = regmap_bulk_read(regmap, ABEOZ9_REG_ALARM_SEC, regs, sizeof(regs));
> > +	if (ret)
> > +		return ret;
> > +
> > +	alarm->time.tm_sec = bcd2bin(FIELD_GET(ABEOZ9_BIT_ALARM_SEC, regs[0]));
> > +	alarm->time.tm_min = bcd2bin(FIELD_GET(ABEOZ9_BIT_ALARM_MIN, regs[1]));
> > +	alarm->time.tm_hour = bcd2bin(FIELD_GET(ABEOZ9_BIT_ALARM_HOURS, regs[2]));
> > +	if (FIELD_GET(ABEOZ9_BIT_ALARM_HOURS_PM, regs[2]))
> > +		alarm->time.tm_hour += 12;
> > +
> > +	alarm->time.tm_mday = bcd2bin(FIELD_GET(ABEOZ9_BIT_ALARM_DAYS, regs[3]));
> > +
> > +	return 0;
> > +}
> > +
> > +static int abeoz9_rtc_alarm_irq_enable(struct device *dev, u32 enable)
> > +{
> > +	struct abeoz9_rtc_data *data = dev_get_drvdata(dev);
> > +
> > +	return regmap_update_bits(data->regmap, ABEOZ9_REG_CTRL_INT,
> > +				  ABEOZ9_REG_CTRL_INT_AIE,
> > +				  FIELD_PREP(ABEOZ9_REG_CTRL_INT_AIE, enable));
> > +}
> > +
> > +static int abeoz9_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alarm)
> > +{
> > +	struct abeoz9_rtc_data *data = dev_get_drvdata(dev);
> > +	u8 regs[ABEOZ9_ALARM_LEN] = {0};
> > +	int ret;
> > +
> > +	ret = regmap_update_bits(data->regmap, ABEOZ9_REG_CTRL_INT_FLAG,
> > +				 ABEOZ9_REG_CTRL_INT_FLAG_AF, 0);
> > +	if (ret)
> > +		return ret;
> > +
> > +	regs[0] = ABEOZ9_BIT_ALARM_AE | FIELD_PREP(ABEOZ9_BIT_ALARM_SEC,
> > +						   bin2bcd(alarm->time.tm_sec));
> > +	regs[1] = ABEOZ9_BIT_ALARM_AE | FIELD_PREP(ABEOZ9_BIT_ALARM_MIN,
> > +						   bin2bcd(alarm->time.tm_min));
> > +	regs[2] = ABEOZ9_BIT_ALARM_AE | FIELD_PREP(ABEOZ9_BIT_ALARM_HOURS,
> > +						   bin2bcd(alarm->time.tm_hour));
> > +	regs[3] = ABEOZ9_BIT_ALARM_AE | FIELD_PREP(ABEOZ9_BIT_ALARM_DAYS,
> > +						   bin2bcd(alarm->time.tm_mday));
> > +
> > +	ret = regmap_bulk_write(data->regmap, ABEOZ9_REG_ALARM_SEC, regs,
> > +				sizeof(regs));
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = abeoz9_reset_validity(data->regmap);
>
> This is not right, this must not be used unless the time set on the RTC
> is correct which you don't know at that time.
>

Understood, will drop.

Liam

> > +	if (ret)
> > +		return ret;
> > +
> > +	return abeoz9_rtc_alarm_irq_enable(dev, alarm->enabled);
> > +}
> > +
> > +static irqreturn_t abeoz9_rtc_irq(int irq, void *dev)
> > +{
> > +	struct abeoz9_rtc_data *data = dev_get_drvdata(dev);
> > +	unsigned int val;
> > +	int ret;
> > +
> > +	ret = regmap_read(data->regmap, ABEOZ9_REG_CTRL_INT_FLAG, &val);
> > +	if (ret)
> > +		return IRQ_NONE;
> > +
> > +	if (!FIELD_GET(ABEOZ9_REG_CTRL_INT_FLAG_AF, val))
> > +		return IRQ_NONE;
> > +
> > +	regmap_update_bits(data->regmap, ABEOZ9_REG_CTRL_INT_FLAG,
> > +			   ABEOZ9_REG_CTRL_INT_FLAG_AF, 0);
> > +
> > +	rtc_update_irq(data->rtc, 1, RTC_IRQF | RTC_AF);
> > +
> > +	return IRQ_HANDLED;
> > +}
> > +
> >  static int abeoz9_trickle_parse_dt(struct device_node *node)
> >  {
> >  	u32 ohms = 0;
> > @@ -257,6 +372,14 @@ static const struct rtc_class_ops rtc_ops = {
> >  	.set_time  = abeoz9_rtc_set_time,
> >  };
> >  
> > +static const struct rtc_class_ops rtc_alarm_ops = {
> > +	.read_time = abeoz9_rtc_get_time,
> > +	.set_time = abeoz9_rtc_set_time,
> > +	.read_alarm = abeoz9_rtc_read_alarm,
> > +	.set_alarm = abeoz9_rtc_set_alarm,
> > +	.alarm_irq_enable = abeoz9_rtc_alarm_irq_enable,
> > +};
> > +
> >  static const struct regmap_config abeoz9_rtc_regmap_config = {
> >  	.reg_bits = 8,
> >  	.val_bits = 8,
> > @@ -416,6 +539,23 @@ static int abeoz9_probe(struct i2c_client *client,
> >  	data->rtc->ops = &rtc_ops;
> >  	data->rtc->range_min = RTC_TIMESTAMP_BEGIN_2000;
> >  	data->rtc->range_max = RTC_TIMESTAMP_END_2099;
> > +	data->rtc->uie_unsupported = 1;
> > +
> > +	if (client->irq > 0) {
> > +		ret = devm_request_threaded_irq(dev, client->irq, NULL,
> > +						abeoz9_rtc_irq,
> > +						IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> > +						dev_name(dev), dev);
> > +		if (ret) {
> > +			dev_err(dev, "failed to request alarm irq\n");
> > +			return ret;
> > +		}
> > +	}
> > +
> > +	if (client->irq > 0 || device_property_read_bool(dev, "wakeup-source")) {
> > +		ret = device_init_wakeup(dev, true);
> > +		data->rtc->ops = &rtc_alarm_ops;
> > +	}
> >  
> >  	ret = devm_rtc_register_device(data->rtc);
> >  	if (ret)
> > -- 
> > 2.30.1.489.g328c10930387
> > 
>
> --
> Alexandre Belloni, co-owner and COO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

Re: [PATCH v1 4/4] rtc: ab-eoz9: make use of RTC_FEATURE_ALARM

[Liam Beguin]

Hi Alexandre,

On Mon Apr 5, 2021 at 1:43 PM EDT, Alexandre Belloni wrote:
> On 05/04/2021 10:13:34-0400, Liam Beguin wrote:
> > From: Liam Beguin <lvb@xiphos.com>
> > 
> > Move the alarm callbacks in rtc_ops and use RTC_FEATURE_ALARM to notify
> > the core whether alarm capabilities are available or not.
> > 
>
> Please squash that one with the previous one.
>

I kept this separate because I'm currently working on v5.4 which doesn't
have RTC_FEATURE_ALARM. If it's okay with you I'd like to keep it like
this to make it easier to backport.

> alse, the series looks good, thanks for working on that!
>

Thanks,
Liam

> > Signed-off-by: Liam Beguin <lvb@xiphos.com>
> > ---
> >  drivers/rtc/rtc-ab-eoz9.c | 8 ++------
> >  1 file changed, 2 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/rtc/rtc-ab-eoz9.c b/drivers/rtc/rtc-ab-eoz9.c
> > index 4c8ba62fb1c6..a8256747d95d 100644
> > --- a/drivers/rtc/rtc-ab-eoz9.c
> > +++ b/drivers/rtc/rtc-ab-eoz9.c
> > @@ -368,11 +368,6 @@ static int abeoz9_rtc_setup(struct device *dev, struct device_node *node)
> >  }
> >  
> >  static const struct rtc_class_ops rtc_ops = {
> > -	.read_time = abeoz9_rtc_get_time,
> > -	.set_time  = abeoz9_rtc_set_time,
> > -};
> > -
> > -static const struct rtc_class_ops rtc_alarm_ops = {
> >  	.read_time = abeoz9_rtc_get_time,
> >  	.set_time = abeoz9_rtc_set_time,
> >  	.read_alarm = abeoz9_rtc_read_alarm,
> > @@ -540,6 +535,7 @@ static int abeoz9_probe(struct i2c_client *client,
> >  	data->rtc->range_min = RTC_TIMESTAMP_BEGIN_2000;
> >  	data->rtc->range_max = RTC_TIMESTAMP_END_2099;
> >  	data->rtc->uie_unsupported = 1;
> > +	clear_bit(RTC_FEATURE_ALARM, data->rtc->features);
> >  
> >  	if (client->irq > 0) {
> >  		ret = devm_request_threaded_irq(dev, client->irq, NULL,
> > @@ -554,7 +550,7 @@ static int abeoz9_probe(struct i2c_client *client,
> >  
> >  	if (client->irq > 0 || device_property_read_bool(dev, "wakeup-source")) {
> >  		ret = device_init_wakeup(dev, true);
> > -		data->rtc->ops = &rtc_alarm_ops;
> > +		set_bit(RTC_FEATURE_ALARM, data->rtc->features);
> >  	}
> >  
> >  	ret = devm_rtc_register_device(data->rtc);
> > -- 
> > 2.30.1.489.g328c10930387
> > 
>
> --
> Alexandre Belloni, co-owner and COO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com