rtc: make the pm8xxx RTC driver usable

PATCH In review Josh Cartwright 5 participants 18 messages Mar 10, 2014 – Mar 21, 2014

Subsystems (4)

Device tree bindings Arm Qualcomm mailing list Multifunction devices (MFD) Real time clock (RTC)

lore

Share & tools

lore

Josh Cartwright

author

2014-03-10 18:44 UTC #0 lore raw

v2 cover rtc: make the pm8xxx RTC driver usable

This patchset is based on Stephen Boyd's PM8921 modernization/cleanups[1], and allows for this RTC driver to be usable again.

Changes since v1:
* Further style cleanups as suggested by Stephen Boyd
* Dropped the use of reg-names/interrupt-names properties entirely

[1]: http://lkml.kernel.org/g/[email protected]

Josh Cartwright (6):
rtc: pm8xxx: fixup checkpatch/style issues
rtc: pm8xxx: use regmap API for register accesses
rtc: pm8xxx: use devm_request_any_context_irq
rtc: pm8xxx: add support for devicetree
rtc: pm8xxx: move device_init_wakeup() before rtc_register
documentation: bindings: document PMIC8921/8058 RTC

 .../devicetree/bindings/rtc/qcom,pm8xxx-rtc.txt    |  21 ++
 drivers/rtc/rtc-pm8xxx.c                           | 284 ++++++++++-----------
 include/linux/mfd/pm8xxx/rtc.h                     |  25 --
 3 files changed, 150 insertions(+), 180 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/rtc/qcom,pm8xxx-rtc.txt
 delete mode 100644 include/linux/mfd/pm8xxx/rtc.h

Signature · 2 lines

Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

Josh Cartwright

author

2014-03-10 18:44 UTC #1 lore raw

PATCH v2 1/6 rtc: pm8xxx: fixup checkpatch/style issues

Before performing additional cleanups to this driver, do the easy cleanups first.

Signed-off-by Josh Cartwright <[email protected]>

Reviewed-by Stephen Boyd <[email protected]>

---
 drivers/rtc/rtc-pm8xxx.c | 97 ++++++++++++++++++++++++++----------------------
 1 file changed, 53 insertions(+), 44 deletions(-)

drivers/rtc/rtc-pm8xxx.c +53 ~~-44~~

index bd76ffe9..af60ee4 100644
@@ -53,7 +53,7 @@ struct pm8xxx_rtc {
 	int rtc_read_base;
 	int rtc_write_base;
 	int alarm_rw_base;
-	u8  ctrl_reg;
+	u8 ctrl_reg;
 	struct device *rtc_dev;
 	spinlock_t ctrl_reg_lock;
 };

@@ -63,7 +63,7 @@ struct pm8xxx_rtc {
  * hardware limitation.
  */
 static int pm8xxx_read_wrapper(struct pm8xxx_rtc *rtc_dd, u8 *rtc_val,
-		int base, int count)
+			       int base, int count)
 {
 	int i, rc;
 	struct device *parent = rtc_dd->rtc_dev->parent;

@@ -80,7 +80,7 @@ static int pm8xxx_read_wrapper(struct pm8xxx_rtc *rtc_dd, u8 *rtc_val,
 }
 
 static int pm8xxx_write_wrapper(struct pm8xxx_rtc *rtc_dd, u8 *rtc_val,
-		int base, int count)
+				int base, int count)
 {
 	int i, rc;
 	struct device *parent = rtc_dd->rtc_dev->parent;

@@ -126,15 +126,15 @@ static int pm8xxx_rtc_set_time(struct device *dev, struct rtc_time *tm)
 		alarm_enabled = 1;
 		ctrl_reg &= ~PM8xxx_RTC_ALARM_ENABLE;
 		rc = pm8xxx_write_wrapper(rtc_dd, &ctrl_reg, rtc_dd->rtc_base,
-				1);
+					  1);
 		if (rc < 0) {
-			dev_err(dev, "Write to RTC control register "
-								"failed\n");
+			dev_err(dev, "Write to RTC control register failed\n");
 			goto rtc_rw_fail;
 		}
 		rtc_dd->ctrl_reg = ctrl_reg;
-	} else
+	} else {
 		spin_unlock_irqrestore(&rtc_dd->ctrl_reg_lock, irq_flags);
+	}
 
 	/* Write 0 to Byte[0] */
 	reg = 0;

@@ -146,7 +146,7 @@ static int pm8xxx_rtc_set_time(struct device *dev, struct rtc_time *tm)
 
 	/* Write Byte[1], Byte[2], Byte[3] */
 	rc = pm8xxx_write_wrapper(rtc_dd, value + 1,
-					rtc_dd->rtc_write_base + 1, 3);
+				  rtc_dd->rtc_write_base + 1, 3);
 	if (rc < 0) {
 		dev_err(dev, "Write to RTC write data register failed\n");
 		goto rtc_rw_fail;

@@ -162,10 +162,9 @@ static int pm8xxx_rtc_set_time(struct device *dev, struct rtc_time *tm)
 	if (alarm_enabled) {
 		ctrl_reg |= PM8xxx_RTC_ALARM_ENABLE;
 		rc = pm8xxx_write_wrapper(rtc_dd, &ctrl_reg, rtc_dd->rtc_base,
-									1);
+					  1);
 		if (rc < 0) {
-			dev_err(dev, "Write to RTC control register "
-								"failed\n");
+			dev_err(dev, "Write to RTC control register failed\n");
 			goto rtc_rw_fail;
 		}
 		rtc_dd->ctrl_reg = ctrl_reg;

@@ -186,7 +185,7 @@ static int pm8xxx_rtc_read_time(struct device *dev, struct rtc_time *tm)
 	struct pm8xxx_rtc *rtc_dd = dev_get_drvdata(dev);
 
 	rc = pm8xxx_read_wrapper(rtc_dd, value, rtc_dd->rtc_read_base,
-							NUM_8_BIT_RTC_REGS);
+				 NUM_8_BIT_RTC_REGS);
 	if (rc < 0) {
 		dev_err(dev, "RTC read data register failed\n");
 		return rc;

@@ -204,7 +203,8 @@ static int pm8xxx_rtc_read_time(struct device *dev, struct rtc_time *tm)
 
 	if (unlikely(reg < value[0])) {
 		rc = pm8xxx_read_wrapper(rtc_dd, value,
-				rtc_dd->rtc_read_base, NUM_8_BIT_RTC_REGS);
+					 rtc_dd->rtc_read_base,
+					 NUM_8_BIT_RTC_REGS);
 		if (rc < 0) {
 			dev_err(dev, "RTC read data register failed\n");
 			return rc;

@@ -222,8 +222,8 @@ static int pm8xxx_rtc_read_time(struct device *dev, struct rtc_time *tm)
 	}
 
 	dev_dbg(dev, "secs = %lu, h:m:s == %d:%d:%d, d/m/y = %d/%d/%d\n",
-				secs, tm->tm_hour, tm->tm_min, tm->tm_sec,
-				tm->tm_mday, tm->tm_mon, tm->tm_year);
+		secs, tm->tm_hour, tm->tm_min, tm->tm_sec,
+		tm->tm_mday, tm->tm_mon, tm->tm_year);
 
 	return 0;
 }

@@ -245,15 +245,18 @@ static int pm8xxx_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alarm)
 	spin_lock_irqsave(&rtc_dd->ctrl_reg_lock, irq_flags);
 
 	rc = pm8xxx_write_wrapper(rtc_dd, value, rtc_dd->alarm_rw_base,
-							NUM_8_BIT_RTC_REGS);
+				  NUM_8_BIT_RTC_REGS);
 	if (rc < 0) {
 		dev_err(dev, "Write to RTC ALARM register failed\n");
 		goto rtc_rw_fail;
 	}
 
 	ctrl_reg = rtc_dd->ctrl_reg;
-	ctrl_reg = alarm->enabled ? (ctrl_reg | PM8xxx_RTC_ALARM_ENABLE) :
-					(ctrl_reg & ~PM8xxx_RTC_ALARM_ENABLE);
+
+	if (alarm->enabled)
+		ctrl_reg |= PM8xxx_RTC_ALARM_ENABLE;
+	else
+		ctrl_reg &= ~PM8xxx_RTC_ALARM_ENABLE;
 
 	rc = pm8xxx_write_wrapper(rtc_dd, &ctrl_reg, rtc_dd->rtc_base, 1);
 	if (rc < 0) {

@@ -264,9 +267,9 @@ static int pm8xxx_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alarm)
 	rtc_dd->ctrl_reg = ctrl_reg;
 
 	dev_dbg(dev, "Alarm Set for h:r:s=%d:%d:%d, d/m/y=%d/%d/%d\n",
-				alarm->time.tm_hour, alarm->time.tm_min,
-				alarm->time.tm_sec, alarm->time.tm_mday,
-				alarm->time.tm_mon, alarm->time.tm_year);
+		alarm->time.tm_hour, alarm->time.tm_min,
+		alarm->time.tm_sec, alarm->time.tm_mday,
+		alarm->time.tm_mon, alarm->time.tm_year);
 rtc_rw_fail:
 	spin_unlock_irqrestore(&rtc_dd->ctrl_reg_lock, irq_flags);
 	return rc;

@@ -280,7 +283,7 @@ static int pm8xxx_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alarm)
 	struct pm8xxx_rtc *rtc_dd = dev_get_drvdata(dev);
 
 	rc = pm8xxx_read_wrapper(rtc_dd, value, rtc_dd->alarm_rw_base,
-			NUM_8_BIT_RTC_REGS);
+				 NUM_8_BIT_RTC_REGS);
 	if (rc < 0) {
 		dev_err(dev, "RTC alarm time read failed\n");
 		return rc;

@@ -297,9 +300,9 @@ static int pm8xxx_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alarm)
 	}
 
 	dev_dbg(dev, "Alarm set for - h:r:s=%d:%d:%d, d/m/y=%d/%d/%d\n",
-				alarm->time.tm_hour, alarm->time.tm_min,
-				alarm->time.tm_sec, alarm->time.tm_mday,
-				alarm->time.tm_mon, alarm->time.tm_year);
+		alarm->time.tm_hour, alarm->time.tm_min,
+		alarm->time.tm_sec, alarm->time.tm_mday,
+		alarm->time.tm_mon, alarm->time.tm_year);
 
 	return 0;
 }

@@ -312,9 +315,13 @@ static int pm8xxx_rtc_alarm_irq_enable(struct device *dev, unsigned int enable)
 	u8 ctrl_reg;
 
 	spin_lock_irqsave(&rtc_dd->ctrl_reg_lock, irq_flags);
+
 	ctrl_reg = rtc_dd->ctrl_reg;
-	ctrl_reg = (enable) ? (ctrl_reg | PM8xxx_RTC_ALARM_ENABLE) :
-				(ctrl_reg & ~PM8xxx_RTC_ALARM_ENABLE);
+
+	if (enable)
+		ctrl_reg |= PM8xxx_RTC_ALARM_ENABLE;
+	else
+		ctrl_reg &= ~PM8xxx_RTC_ALARM_ENABLE;
 
 	rc = pm8xxx_write_wrapper(rtc_dd, &ctrl_reg, rtc_dd->rtc_base, 1);
 	if (rc < 0) {

@@ -354,8 +361,8 @@ static irqreturn_t pm8xxx_alarm_trigger(int irq, void *dev_id)
 	rc = pm8xxx_write_wrapper(rtc_dd, &ctrl_reg, rtc_dd->rtc_base, 1);
 	if (rc < 0) {
 		spin_unlock_irqrestore(&rtc_dd->ctrl_reg_lock, irq_flags);
-		dev_err(rtc_dd->rtc_dev, "Write to RTC control register "
-								"failed\n");
+		dev_err(rtc_dd->rtc_dev,
+			"Write to RTC control register failed\n");
 		goto rtc_alarm_handled;
 	}

@@ -364,19 +371,19 @@ static irqreturn_t pm8xxx_alarm_trigger(int irq, void *dev_id)
 
 	/* Clear RTC alarm register */
 	rc = pm8xxx_read_wrapper(rtc_dd, &ctrl_reg, rtc_dd->rtc_base +
-						PM8XXX_ALARM_CTRL_OFFSET, 1);
+				 PM8XXX_ALARM_CTRL_OFFSET, 1);
 	if (rc < 0) {
-		dev_err(rtc_dd->rtc_dev, "RTC Alarm control register read "
-								"failed\n");
+		dev_err(rtc_dd->rtc_dev,
+			"RTC Alarm control register read failed\n");
 		goto rtc_alarm_handled;
 	}
 
 	ctrl_reg &= ~PM8xxx_RTC_ALARM_CLEAR;
 	rc = pm8xxx_write_wrapper(rtc_dd, &ctrl_reg, rtc_dd->rtc_base +
-						PM8XXX_ALARM_CTRL_OFFSET, 1);
+				  PM8XXX_ALARM_CTRL_OFFSET, 1);
 	if (rc < 0)
-		dev_err(rtc_dd->rtc_dev, "Write to RTC Alarm control register"
-								" failed\n");
+		dev_err(rtc_dd->rtc_dev,
+			"Write to RTC Alarm control register failed\n");
 
 rtc_alarm_handled:
 	return IRQ_HANDLED;

@@ -409,7 +416,7 @@ static int pm8xxx_rtc_probe(struct platform_device *pdev)
 	}
 
 	rtc_resource = platform_get_resource_byname(pdev, IORESOURCE_IO,
-							"pmic_rtc_base");
+						    "pmic_rtc_base");
 	if (!(rtc_resource && rtc_resource->start)) {
 		dev_err(&pdev->dev, "RTC IO resource absent!\n");
 		return -ENXIO;

@@ -436,31 +443,31 @@ static int pm8xxx_rtc_probe(struct platform_device *pdev)
 		rc = pm8xxx_write_wrapper(rtc_dd, &ctrl_reg, rtc_dd->rtc_base,
 									1);
 		if (rc < 0) {
-			dev_err(&pdev->dev, "Write to RTC control register "
-								"failed\n");
+			dev_err(&pdev->dev,
+				"Write to RTC control register failed\n");
 			return rc;
 		}
 	}
 
 	rtc_dd->ctrl_reg = ctrl_reg;
-	if (rtc_write_enable == true)
+	if (rtc_write_enable)
 		pm8xxx_rtc_ops.set_time = pm8xxx_rtc_set_time;
 
 	platform_set_drvdata(pdev, rtc_dd);
 
 	/* Register the RTC device */
 	rtc_dd->rtc = devm_rtc_device_register(&pdev->dev, "pm8xxx_rtc",
-				&pm8xxx_rtc_ops, THIS_MODULE);
+					       &pm8xxx_rtc_ops, THIS_MODULE);
 	if (IS_ERR(rtc_dd->rtc)) {
 		dev_err(&pdev->dev, "%s: RTC registration failed (%ld)\n",
-					__func__, PTR_ERR(rtc_dd->rtc));
+			__func__, PTR_ERR(rtc_dd->rtc));
 		return PTR_ERR(rtc_dd->rtc);
 	}
 
 	/* Request the alarm IRQ */
 	rc = request_any_context_irq(rtc_dd->rtc_alarm_irq,
-				 pm8xxx_alarm_trigger, IRQF_TRIGGER_RISING,
-				 "pm8xxx_rtc_alarm", rtc_dd);
+				     pm8xxx_alarm_trigger, IRQF_TRIGGER_RISING,
+				     "pm8xxx_rtc_alarm", rtc_dd);
 	if (rc < 0) {
 		dev_err(&pdev->dev, "Request IRQ failed (%d)\n", rc);
 		return rc;

@@ -505,7 +512,9 @@ static int pm8xxx_rtc_suspend(struct device *dev)
 }
 #endif
 
-static SIMPLE_DEV_PM_OPS(pm8xxx_rtc_pm_ops, pm8xxx_rtc_suspend, pm8xxx_rtc_resume);
+static SIMPLE_DEV_PM_OPS(pm8xxx_rtc_pm_ops,
+			 pm8xxx_rtc_suspend,
+			 pm8xxx_rtc_resume);
 
 static struct platform_driver pm8xxx_rtc_driver = {
 	.probe		= pm8xxx_rtc_probe,

Signature · 2 lines

Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

Josh Cartwright

author

2014-03-10 18:44 UTC #2 lore raw

PATCH v2 2/6 rtc: pm8xxx: use regmap API for register accesses

In reply to Josh Cartwright

Now that the parent mfd driver has been made to work again, and has been reworked to create a regmap instance intended for it's children to use, rework the pm8xxx driver to use the regmap API for it's register accesses.

Reviewed-by Stephen Boyd <[email protected]>

Signed-off-by Josh Cartwright <[email protected]>

---
 drivers/rtc/rtc-pm8xxx.c | 145 +++++++++++++++++++----------------------------
 1 file changed, 57 insertions(+), 88 deletions(-)

drivers/rtc/rtc-pm8xxx.c +57 ~~-88~~

index af60ee4..cdc9dc2 100644
@@ -13,11 +13,12 @@
 #include <linux/module.h>
 #include <linux/init.h>
 #include <linux/rtc.h>
+#include <linux/platform_device.h>
 #include <linux/pm.h>
+#include <linux/regmap.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
 
-#include <linux/mfd/pm8xxx/core.h>
 #include <linux/mfd/pm8xxx/rtc.h>

@@ -37,6 +38,7 @@
 /**
  * struct pm8xxx_rtc -  rtc driver internal structure
  * @rtc:		rtc device for this driver.
+ * @regmap:		regmap used to access RTC registers
  * @rtc_alarm_irq:	rtc alarm irq number.
  * @rtc_base:		address of rtc control register.
  * @rtc_read_base:	base address of read registers.

@@ -48,6 +50,7 @@
  */
 struct pm8xxx_rtc {
 	struct rtc_device *rtc;
+	struct regmap *regmap;
 	int rtc_alarm_irq;
 	int rtc_base;
 	int rtc_read_base;

@@ -59,44 +62,6 @@ struct pm8xxx_rtc {
 };
 
 /*
- * The RTC registers need to be read/written one byte at a time. This is a
- * hardware limitation.
- */
-static int pm8xxx_read_wrapper(struct pm8xxx_rtc *rtc_dd, u8 *rtc_val,
-			       int base, int count)
-{
-	int i, rc;
-	struct device *parent = rtc_dd->rtc_dev->parent;
-
-	for (i = 0; i < count; i++) {
-		rc = pm8xxx_readb(parent, base + i, &rtc_val[i]);
-		if (rc < 0) {
-			dev_err(rtc_dd->rtc_dev, "PMIC read failed\n");
-			return rc;
-		}
-	}
-
-	return 0;
-}
-
-static int pm8xxx_write_wrapper(struct pm8xxx_rtc *rtc_dd, u8 *rtc_val,
-				int base, int count)
-{
-	int i, rc;
-	struct device *parent = rtc_dd->rtc_dev->parent;
-
-	for (i = 0; i < count; i++) {
-		rc = pm8xxx_writeb(parent, base + i, rtc_val[i]);
-		if (rc < 0) {
-			dev_err(rtc_dd->rtc_dev, "PMIC write failed\n");
-			return rc;
-		}
-	}
-
-	return 0;
-}
-
-/*
  * Steps to write the RTC registers.
  * 1. Disable alarm if enabled.
  * 2. Write 0x00 to LSB.

@@ -107,7 +72,7 @@ static int pm8xxx_rtc_set_time(struct device *dev, struct rtc_time *tm)
 {
 	int rc, i;
 	unsigned long secs, irq_flags;
-	u8 value[NUM_8_BIT_RTC_REGS], reg = 0, alarm_enabled = 0, ctrl_reg;
+	u8 value[NUM_8_BIT_RTC_REGS], alarm_enabled = 0, ctrl_reg;
 	struct pm8xxx_rtc *rtc_dd = dev_get_drvdata(dev);
 
 	rtc_tm_to_time(tm, &secs);

@@ -125,9 +90,8 @@ static int pm8xxx_rtc_set_time(struct device *dev, struct rtc_time *tm)
 	if (ctrl_reg & PM8xxx_RTC_ALARM_ENABLE) {
 		alarm_enabled = 1;
 		ctrl_reg &= ~PM8xxx_RTC_ALARM_ENABLE;
-		rc = pm8xxx_write_wrapper(rtc_dd, &ctrl_reg, rtc_dd->rtc_base,
-					  1);
-		if (rc < 0) {
+		rc = regmap_write(rtc_dd->regmap, rtc_dd->rtc_base, ctrl_reg);
+		if (rc) {
 			dev_err(dev, "Write to RTC control register failed\n");
 			goto rtc_rw_fail;
 		}

@@ -137,33 +101,31 @@ static int pm8xxx_rtc_set_time(struct device *dev, struct rtc_time *tm)
 	}
 
 	/* Write 0 to Byte[0] */
-	reg = 0;
-	rc = pm8xxx_write_wrapper(rtc_dd, &reg, rtc_dd->rtc_write_base, 1);
-	if (rc < 0) {
+	rc = regmap_write(rtc_dd->regmap, rtc_dd->rtc_write_base, 0);
+	if (rc) {
 		dev_err(dev, "Write to RTC write data register failed\n");
 		goto rtc_rw_fail;
 	}
 
 	/* Write Byte[1], Byte[2], Byte[3] */
-	rc = pm8xxx_write_wrapper(rtc_dd, value + 1,
-				  rtc_dd->rtc_write_base + 1, 3);
-	if (rc < 0) {
+	rc = regmap_bulk_write(rtc_dd->regmap, rtc_dd->rtc_write_base + 1,
+			       &value[1], sizeof(value) - 1);
+	if (rc) {
 		dev_err(dev, "Write to RTC write data register failed\n");
 		goto rtc_rw_fail;
 	}
 
 	/* Write Byte[0] */
-	rc = pm8xxx_write_wrapper(rtc_dd, value, rtc_dd->rtc_write_base, 1);
-	if (rc < 0) {
+	rc = regmap_write(rtc_dd->regmap, rtc_dd->rtc_write_base, value[0]);
+	if (rc) {
 		dev_err(dev, "Write to RTC write data register failed\n");
 		goto rtc_rw_fail;
 	}
 
 	if (alarm_enabled) {
 		ctrl_reg |= PM8xxx_RTC_ALARM_ENABLE;
-		rc = pm8xxx_write_wrapper(rtc_dd, &ctrl_reg, rtc_dd->rtc_base,
-					  1);
-		if (rc < 0) {
+		rc = regmap_write(rtc_dd->regmap, rtc_dd->rtc_base, ctrl_reg);
+		if (rc) {
 			dev_err(dev, "Write to RTC control register failed\n");
 			goto rtc_rw_fail;
 		}

@@ -180,13 +142,14 @@ rtc_rw_fail:
 static int pm8xxx_rtc_read_time(struct device *dev, struct rtc_time *tm)
 {
 	int rc;
-	u8 value[NUM_8_BIT_RTC_REGS], reg;
+	u8 value[NUM_8_BIT_RTC_REGS];
 	unsigned long secs;
+	unsigned int reg;
 	struct pm8xxx_rtc *rtc_dd = dev_get_drvdata(dev);
 
-	rc = pm8xxx_read_wrapper(rtc_dd, value, rtc_dd->rtc_read_base,
-				 NUM_8_BIT_RTC_REGS);
-	if (rc < 0) {
+	rc = regmap_bulk_read(rtc_dd->regmap, rtc_dd->rtc_read_base,
+			      value, sizeof(value));
+	if (rc) {
 		dev_err(dev, "RTC read data register failed\n");
 		return rc;
 	}

@@ -195,17 +158,16 @@ static int pm8xxx_rtc_read_time(struct device *dev, struct rtc_time *tm)
 	 * Read the LSB again and check if there has been a carry over.
 	 * If there is, redo the read operation.
 	 */
-	rc = pm8xxx_read_wrapper(rtc_dd, &reg, rtc_dd->rtc_read_base, 1);
+	rc = regmap_read(rtc_dd->regmap, rtc_dd->rtc_read_base, &reg);
 	if (rc < 0) {
 		dev_err(dev, "RTC read data register failed\n");
 		return rc;
 	}
 
 	if (unlikely(reg < value[0])) {
-		rc = pm8xxx_read_wrapper(rtc_dd, value,
-					 rtc_dd->rtc_read_base,
-					 NUM_8_BIT_RTC_REGS);
-		if (rc < 0) {
+		rc = regmap_bulk_read(rtc_dd->regmap, rtc_dd->rtc_read_base,
+				      value, sizeof(value));
+		if (rc) {
 			dev_err(dev, "RTC read data register failed\n");
 			return rc;
 		}

@@ -244,9 +206,9 @@ static int pm8xxx_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alarm)
 
 	spin_lock_irqsave(&rtc_dd->ctrl_reg_lock, irq_flags);
 
-	rc = pm8xxx_write_wrapper(rtc_dd, value, rtc_dd->alarm_rw_base,
-				  NUM_8_BIT_RTC_REGS);
-	if (rc < 0) {
+	rc = regmap_bulk_write(rtc_dd->regmap, rtc_dd->alarm_rw_base, value,
+			       sizeof(value));
+	if (rc) {
 		dev_err(dev, "Write to RTC ALARM register failed\n");
 		goto rtc_rw_fail;
 	}

@@ -258,8 +220,8 @@ static int pm8xxx_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alarm)
 	else
 		ctrl_reg &= ~PM8xxx_RTC_ALARM_ENABLE;
 
-	rc = pm8xxx_write_wrapper(rtc_dd, &ctrl_reg, rtc_dd->rtc_base, 1);
-	if (rc < 0) {
+	rc = regmap_write(rtc_dd->regmap, rtc_dd->rtc_base, ctrl_reg);
+	if (rc) {
 		dev_err(dev, "Write to RTC control register failed\n");
 		goto rtc_rw_fail;
 	}

@@ -282,9 +244,9 @@ static int pm8xxx_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alarm)
 	unsigned long secs;
 	struct pm8xxx_rtc *rtc_dd = dev_get_drvdata(dev);
 
-	rc = pm8xxx_read_wrapper(rtc_dd, value, rtc_dd->alarm_rw_base,
-				 NUM_8_BIT_RTC_REGS);
-	if (rc < 0) {
+	rc = regmap_bulk_read(rtc_dd->regmap, rtc_dd->alarm_rw_base, value,
+			      sizeof(value));
+	if (rc) {
 		dev_err(dev, "RTC alarm time read failed\n");
 		return rc;
 	}

@@ -323,8 +285,8 @@ static int pm8xxx_rtc_alarm_irq_enable(struct device *dev, unsigned int enable)
 	else
 		ctrl_reg &= ~PM8xxx_RTC_ALARM_ENABLE;
 
-	rc = pm8xxx_write_wrapper(rtc_dd, &ctrl_reg, rtc_dd->rtc_base, 1);
-	if (rc < 0) {
+	rc = regmap_write(rtc_dd->regmap, rtc_dd->rtc_base, ctrl_reg);
+	if (rc) {
 		dev_err(dev, "Write to RTC control register failed\n");
 		goto rtc_rw_fail;
 	}

@@ -346,7 +308,7 @@ static struct rtc_class_ops pm8xxx_rtc_ops = {
 static irqreturn_t pm8xxx_alarm_trigger(int irq, void *dev_id)
 {
 	struct pm8xxx_rtc *rtc_dd = dev_id;
-	u8 ctrl_reg;
+	unsigned int ctrl_reg;
 	int rc;
 	unsigned long irq_flags;

@@ -358,8 +320,8 @@ static irqreturn_t pm8xxx_alarm_trigger(int irq, void *dev_id)
 	ctrl_reg = rtc_dd->ctrl_reg;
 	ctrl_reg &= ~PM8xxx_RTC_ALARM_ENABLE;
 
-	rc = pm8xxx_write_wrapper(rtc_dd, &ctrl_reg, rtc_dd->rtc_base, 1);
-	if (rc < 0) {
+	rc = regmap_write(rtc_dd->regmap, rtc_dd->rtc_base, ctrl_reg);
+	if (rc) {
 		spin_unlock_irqrestore(&rtc_dd->ctrl_reg_lock, irq_flags);
 		dev_err(rtc_dd->rtc_dev,
 			"Write to RTC control register failed\n");

@@ -370,18 +332,20 @@ static irqreturn_t pm8xxx_alarm_trigger(int irq, void *dev_id)
 	spin_unlock_irqrestore(&rtc_dd->ctrl_reg_lock, irq_flags);
 
 	/* Clear RTC alarm register */
-	rc = pm8xxx_read_wrapper(rtc_dd, &ctrl_reg, rtc_dd->rtc_base +
-				 PM8XXX_ALARM_CTRL_OFFSET, 1);
-	if (rc < 0) {
+	rc = regmap_read(rtc_dd->regmap,
+			 rtc_dd->rtc_base + PM8XXX_ALARM_CTRL_OFFSET,
+			 &ctrl_reg);
+	if (rc) {
 		dev_err(rtc_dd->rtc_dev,
 			"RTC Alarm control register read failed\n");
 		goto rtc_alarm_handled;
 	}
 
 	ctrl_reg &= ~PM8xxx_RTC_ALARM_CLEAR;
-	rc = pm8xxx_write_wrapper(rtc_dd, &ctrl_reg, rtc_dd->rtc_base +
-				  PM8XXX_ALARM_CTRL_OFFSET, 1);
-	if (rc < 0)
+	rc = regmap_write(rtc_dd->regmap,
+			  rtc_dd->rtc_base + PM8XXX_ALARM_CTRL_OFFSET,
+			  ctrl_reg);
+	if (rc)
 		dev_err(rtc_dd->rtc_dev,
 			"Write to RTC Alarm control register failed\n");

@@ -392,7 +356,7 @@ rtc_alarm_handled:
 static int pm8xxx_rtc_probe(struct platform_device *pdev)
 {
 	int rc;
-	u8 ctrl_reg;
+	unsigned int ctrl_reg;
 	bool rtc_write_enable = false;
 	struct pm8xxx_rtc *rtc_dd;
 	struct resource *rtc_resource;

@@ -409,6 +373,12 @@ static int pm8xxx_rtc_probe(struct platform_device *pdev)
 	/* Initialise spinlock to protect RTC control register */
 	spin_lock_init(&rtc_dd->ctrl_reg_lock);
 
+	rtc_dd->regmap = dev_get_regmap(pdev->dev.parent, NULL);
+	if (!rtc_dd->regmap) {
+		dev_err(&pdev->dev, "Parent regmap unavailable.\n");
+		return -ENXIO;
+	}
+
 	rtc_dd->rtc_alarm_irq = platform_get_irq(pdev, 0);
 	if (rtc_dd->rtc_alarm_irq < 0) {
 		dev_err(&pdev->dev, "Alarm IRQ resource absent!\n");

@@ -432,17 +402,16 @@ static int pm8xxx_rtc_probe(struct platform_device *pdev)
 	rtc_dd->rtc_dev = &pdev->dev;
 
 	/* Check if the RTC is on, else turn it on */
-	rc = pm8xxx_read_wrapper(rtc_dd, &ctrl_reg, rtc_dd->rtc_base, 1);
-	if (rc < 0) {
+	rc = regmap_read(rtc_dd->regmap, rtc_dd->rtc_base, &ctrl_reg);
+	if (rc) {
 		dev_err(&pdev->dev, "RTC control register read failed!\n");
 		return rc;
 	}
 
 	if (!(ctrl_reg & PM8xxx_RTC_ENABLE)) {
 		ctrl_reg |= PM8xxx_RTC_ENABLE;
-		rc = pm8xxx_write_wrapper(rtc_dd, &ctrl_reg, rtc_dd->rtc_base,
-									1);
-		if (rc < 0) {
+		rc = regmap_write(rtc_dd->regmap, rtc_dd->rtc_base, ctrl_reg);
+		if (rc) {
 			dev_err(&pdev->dev,
 				"Write to RTC control register failed\n");
 			return rc;

Signature · 2 lines

Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

Andrew Morton

2014-03-10 20:47 UTC #3 lore raw

Re: [PATCH v2 2/6] rtc: pm8xxx: use regmap API for register accesses

Quoted 4 lines · On Mon, 10 Mar 2014 13:44:45 -0500 Josh Cartwright <[email protected]> wrote:

Now that the parent mfd driver has been made to work again, and has been reworked to create a regmap instance intended for it's children to use, rework the pm8xxx driver to use the regmap API for it's register accesses.

Do we need a

depends on REGMAP

in Kconfig?

Josh Cartwright

author

2014-03-10 21:35 UTC #4 lore raw

Re: [PATCH v2 2/6] rtc: pm8xxx: use regmap API for register accesses

Quoted 11 lines · Andrew Morton

Quoted 4 lines · On Mon, 10 Mar 2014 13:44:45 -0500 Josh Cartwright <[email protected]> wrote:
Now that the parent mfd driver has been made to work again, and has been reworked to create a regmap instance intended for it's children to use, rework the pm8xxx driver to use the regmap API for it's register accesses.
Do we need a
depends on REGMAP
in Kconfig?

Good question. This driver already gets the REGMAP dependency through it's parent (MFD_PM8XXX), which does 'select REGMAP'.

I recall there being some history as to why individual drivers shouldn't explicitly depend on REGMAP, as it's the parent drivers/subsystem responsibility to select it when needed, however I can't recall details. Perhaps it's due to REGMAP not being a user-selectable option.

(FWIW, I did force a build of this driver with !REGMAP, and it builds just fine with the regmap_write()/_read() stubs, but of course it wouldn't do anything meaningful at runtime)

Thanks,
Josh

Signature · 2 lines

Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

Andrew Morton

2014-03-10 21:43 UTC #5 lore raw

Re: [PATCH v2 2/6] rtc: pm8xxx: use regmap API for register accesses

Quoted 7 lines · On Mon, 10 Mar 2014 16:35:24 -0500 Josh Cartwright <[email protected]> wrote:

Quoted 6 lines · Andrew Morton
Quoted 4 lines · On Mon, 10 Mar 2014 13:44:45 -0500 Josh Cartwright <[email protected]> wrote:
Now that the parent mfd driver has been made to work again, and has been reworked to create a regmap instance intended for it's children to use, rework the pm8xxx driver to use the regmap API for it's register accesses.

Quoted 8 lines

Quoted 5 lines
Do we need a
depends on REGMAP
in Kconfig?
Good question. This driver already gets the REGMAP dependency through it's parent (MFD_PM8XXX), which does 'select REGMAP'.

OK, thanks, that sounds sufficient.

Mark Brown

2014-03-10 23:32 UTC #6 lore raw

Re: [PATCH v2 2/6] rtc: pm8xxx: use regmap API for register accesses

In reply to Josh Cartwright

Quoted 8 lines · Josh Cartwright

Quoted 5 lines · Andrew Morton
Do we need a
depends on REGMAP
in Kconfig?

Quoted 7 lines

Good question. This driver already gets the REGMAP dependency through it's parent (MFD_PM8XXX), which does 'select REGMAP'.
I recall there being some history as to why individual drivers shouldn't explicitly depend on REGMAP, as it's the parent drivers/subsystem responsibility to select it when needed, however I can't recall details. Perhaps it's due to REGMAP not being a user-selectable option.

Yeah, it should be a select of regmap because it's more of a helper library than a user visible feature and it's usually only on the MFD because you can't enable the function device without the core anyway.

Josh Cartwright

author

2014-03-10 18:47 UTC #7 lore raw

PATCH v2 3/6 rtc: pm8xxx: use devm_request_any_context_irq

In reply to Josh Cartwright

Make use of the devm_* variant of request_any_context_irq to allow for elimination of remove().

Signed-off-by Josh Cartwright <[email protected]>

Reviewed-by Stephen Boyd <[email protected]>

---
 drivers/rtc/rtc-pm8xxx.c | 18 ++++--------------
 1 file changed, 4 insertions(+), 14 deletions(-)

drivers/rtc/rtc-pm8xxx.c +4 ~~-14~~

index cdc9dc2..91ac2e3 100644
@@ -434,9 +434,10 @@ static int pm8xxx_rtc_probe(struct platform_device *pdev)
 	}
 
 	/* Request the alarm IRQ */
-	rc = request_any_context_irq(rtc_dd->rtc_alarm_irq,
-				     pm8xxx_alarm_trigger, IRQF_TRIGGER_RISING,
-				     "pm8xxx_rtc_alarm", rtc_dd);
+	rc = devm_request_any_context_irq(&pdev->dev, rtc_dd->rtc_alarm_irq,
+					  pm8xxx_alarm_trigger,
+					  IRQF_TRIGGER_RISING,
+					  "pm8xxx_rtc_alarm", rtc_dd);
 	if (rc < 0) {
 		dev_err(&pdev->dev, "Request IRQ failed (%d)\n", rc);
 		return rc;

@@ -449,16 +450,6 @@ static int pm8xxx_rtc_probe(struct platform_device *pdev)
 	return 0;
 }
 
-static int pm8xxx_rtc_remove(struct platform_device *pdev)
-{
-	struct pm8xxx_rtc *rtc_dd = platform_get_drvdata(pdev);
-
-	device_init_wakeup(&pdev->dev, 0);
-	free_irq(rtc_dd->rtc_alarm_irq, rtc_dd);
-
-	return 0;
-}
-
 #ifdef CONFIG_PM_SLEEP
 static int pm8xxx_rtc_resume(struct device *dev)
 {

@@ -487,7 +478,6 @@ static SIMPLE_DEV_PM_OPS(pm8xxx_rtc_pm_ops,
 
 static struct platform_driver pm8xxx_rtc_driver = {
 	.probe		= pm8xxx_rtc_probe,
-	.remove		= pm8xxx_rtc_remove,
 	.driver	= {
 		.name	= PM8XXX_RTC_DEV_NAME,
 		.owner	= THIS_MODULE,

Signature · 2 lines

Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

Josh Cartwright

author

2014-03-10 18:44 UTC #8 lore raw

PATCH v2 4/6 rtc: pm8xxx: add support for devicetree

In reply to Josh Cartwright

Add support for describing the PM8921/PM8058 RTC in device tree.

Additionally:
- drop support for describing the RTC using platform data, as there are no current in tree users who do so.
- make allow_set_time a device-specific flag, instead of mucking with the rtc_ops

Signed-off-by Josh Cartwright <[email protected]>

Reviewed-by Stephen Boyd <[email protected]>

Acked-by Lee Jones <[email protected]>

---
 drivers/rtc/rtc-pm8xxx.c       | 52 +++++++++++++++++++++++-------------------
 include/linux/mfd/pm8xxx/rtc.h | 25 --------------------
 2 files changed, 29 insertions(+), 48 deletions(-)
 delete mode 100644 include/linux/mfd/pm8xxx/rtc.h

drivers/rtc/rtc-pm8xxx.c +29 ~~-23~~

index 91ac2e3..6e3cd34 100644
@@ -9,7 +9,7 @@
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
  */
-
+#include <linux/of.h>
 #include <linux/module.h>
 #include <linux/init.h>
 #include <linux/rtc.h>

@@ -19,9 +19,6 @@
 #include <linux/slab.h>
 #include <linux/spinlock.h>
 
-#include <linux/mfd/pm8xxx/rtc.h>
-
-
 /* RTC Register offsets from RTC CTRL REG */
 #define PM8XXX_ALARM_CTRL_OFFSET	0x01
 #define PM8XXX_RTC_WRITE_OFFSET		0x02

@@ -39,6 +36,7 @@
  * struct pm8xxx_rtc -  rtc driver internal structure
  * @rtc:		rtc device for this driver.
  * @regmap:		regmap used to access RTC registers
+ * @allow_set_time:	indicates whether writing to the RTC is allowed
  * @rtc_alarm_irq:	rtc alarm irq number.
  * @rtc_base:		address of rtc control register.
  * @rtc_read_base:	base address of read registers.

@@ -51,6 +49,7 @@
 struct pm8xxx_rtc {
 	struct rtc_device *rtc;
 	struct regmap *regmap;
+	bool allow_set_time;
 	int rtc_alarm_irq;
 	int rtc_base;
 	int rtc_read_base;

@@ -75,6 +74,9 @@ static int pm8xxx_rtc_set_time(struct device *dev, struct rtc_time *tm)
 	u8 value[NUM_8_BIT_RTC_REGS], alarm_enabled = 0, ctrl_reg;
 	struct pm8xxx_rtc *rtc_dd = dev_get_drvdata(dev);
 
+	if (!rtc_dd->allow_set_time)
+		return -EACCES;
+
 	rtc_tm_to_time(tm, &secs);
 
 	for (i = 0; i < NUM_8_BIT_RTC_REGS; i++) {

@@ -298,8 +300,9 @@ rtc_rw_fail:
 	return rc;
 }
 
-static struct rtc_class_ops pm8xxx_rtc_ops = {
+static const struct rtc_class_ops pm8xxx_rtc_ops = {
 	.read_time	= pm8xxx_rtc_read_time,
+	.set_time	= pm8xxx_rtc_set_time,
 	.set_alarm	= pm8xxx_rtc_set_alarm,
 	.read_alarm	= pm8xxx_rtc_read_alarm,
 	.alarm_irq_enable = pm8xxx_rtc_alarm_irq_enable,

@@ -353,18 +356,26 @@ rtc_alarm_handled:
 	return IRQ_HANDLED;
 }
 
+/*
+ * Hardcoded RTC bases until IORESOURCE_REG mapping is figured out
+ */
+static const struct of_device_id pm8xxx_id_table[] = {
+	{ .compatible = "qcom,pm8921-rtc", .data = (void *) 0x11D },
+	{ .compatible = "qcom,pm8058-rtc", .data = (void *) 0x1E8 },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, pm8xxx_id_table);
+
 static int pm8xxx_rtc_probe(struct platform_device *pdev)
 {
 	int rc;
 	unsigned int ctrl_reg;
-	bool rtc_write_enable = false;
 	struct pm8xxx_rtc *rtc_dd;
-	struct resource *rtc_resource;
-	const struct pm8xxx_rtc_platform_data *pdata =
-						dev_get_platdata(&pdev->dev);
+	const struct of_device_id *match;
 
-	if (pdata != NULL)
-		rtc_write_enable = pdata->rtc_write_enable;
+	match = of_match_node(pm8xxx_id_table, pdev->dev.of_node);
+	if (!match)
+		return -ENXIO;
 
 	rtc_dd = devm_kzalloc(&pdev->dev, sizeof(*rtc_dd), GFP_KERNEL);
 	if (rtc_dd == NULL)

@@ -385,14 +396,10 @@ static int pm8xxx_rtc_probe(struct platform_device *pdev)
 		return -ENXIO;
 	}
 
-	rtc_resource = platform_get_resource_byname(pdev, IORESOURCE_IO,
-						    "pmic_rtc_base");
-	if (!(rtc_resource && rtc_resource->start)) {
-		dev_err(&pdev->dev, "RTC IO resource absent!\n");
-		return -ENXIO;
-	}
+	rtc_dd->allow_set_time = of_property_read_bool(pdev->dev.of_node,
+						      "allow-set-time");
 
-	rtc_dd->rtc_base = rtc_resource->start;
+	rtc_dd->rtc_base = (long) match->data;
 
 	/* Setup RTC register addresses */
 	rtc_dd->rtc_write_base = rtc_dd->rtc_base + PM8XXX_RTC_WRITE_OFFSET;

@@ -419,8 +426,6 @@ static int pm8xxx_rtc_probe(struct platform_device *pdev)
 	}
 
 	rtc_dd->ctrl_reg = ctrl_reg;
-	if (rtc_write_enable)
-		pm8xxx_rtc_ops.set_time = pm8xxx_rtc_set_time;
 
 	platform_set_drvdata(pdev, rtc_dd);

@@ -479,9 +484,10 @@ static SIMPLE_DEV_PM_OPS(pm8xxx_rtc_pm_ops,
 static struct platform_driver pm8xxx_rtc_driver = {
 	.probe		= pm8xxx_rtc_probe,
 	.driver	= {
-		.name	= PM8XXX_RTC_DEV_NAME,
-		.owner	= THIS_MODULE,
-		.pm	= &pm8xxx_rtc_pm_ops,
+		.name		= "rtc-pm8xxx",
+		.owner		= THIS_MODULE,
+		.pm		= &pm8xxx_rtc_pm_ops,
+		.of_match_table	= pm8xxx_id_table,
 	},
 };

include/linux/mfd/pm8xxx/rtc.h deleted +0 ~~-25~~

deleted file mode 100644
index 14f1983..0000000
@@ -1,25 +0,0 @@
-/* Copyright (c) 2010-2011, Code Aurora Forum. All rights reserved.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 and
- * only version 2 as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- */
-
-#ifndef __RTC_PM8XXX_H__
-#define __RTC_PM8XXX_H__
-
-#define PM8XXX_RTC_DEV_NAME     "rtc-pm8xxx"
-/**
- * struct pm8xxx_rtc_pdata - RTC driver platform data
- * @rtc_write_enable: variable stating RTC write capability
- */
-struct pm8xxx_rtc_platform_data {
-	bool rtc_write_enable;
-};
-
-#endif /* __RTC_PM8XXX_H__ */

Signature · 2 lines

Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

Andrew Morton

2014-03-10 20:47 UTC #9 lore raw

Re: [PATCH v2 4/6] rtc: pm8xxx: add support for devicetree

Quoted 7 lines · On Mon, 10 Mar 2014 13:44:47 -0500 Josh Cartwright <[email protected]> wrote:

Add support for describing the PM8921/PM8058 RTC in device tree.
Additionally:
- drop support for describing the RTC using platform data, as there are no current in tree users who do so.
- make allow_set_time a device-specific flag, instead of mucking with the rtc_ops

Is this tested with and without CONFIG_OF?

Josh Cartwright

author

2014-03-10 21:56 UTC #10 lore raw

Re: [PATCH v2 4/6] rtc: pm8xxx: add support for devicetree

Quoted 11 lines · Andrew Morton

Quoted 7 lines · On Mon, 10 Mar 2014 13:44:47 -0500 Josh Cartwright <[email protected]> wrote:
Add support for describing the PM8921/PM8058 RTC in device tree.
Additionally:
- drop support for describing the RTC using platform data, as there are no current in tree users who do so.
- make allow_set_time a device-specific flag, instead of mucking with the rtc_ops
Is this tested with and without CONFIG_OF?

It's been runtime tested with CONFIG_OF and I ensured it at least built when !CONFIG_OF, although the drivers useless in that configuration.

Signature · 2 lines

Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

Josh Cartwright

author

2014-03-10 18:44 UTC #11 lore raw

PATCH v2 5/6 rtc: pm8xxx: move device_init_wakeup() before rtc_register

In reply to Josh Cartwright

Setup wakeup capability before rtc_register to ensure the rtc class core properly sets up our 'wakealarm' sysfs attribute.

Signed-off-by Josh Cartwright <[email protected]>

Reviewed-by Stephen Boyd <[email protected]>

---
 drivers/rtc/rtc-pm8xxx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

drivers/rtc/rtc-pm8xxx.c +2 -2

index 6e3cd34..197699f 100644
@@ -429,6 +429,8 @@ static int pm8xxx_rtc_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, rtc_dd);
 
+	device_init_wakeup(&pdev->dev, 1);
+
 	/* Register the RTC device */
 	rtc_dd->rtc = devm_rtc_device_register(&pdev->dev, "pm8xxx_rtc",
 					       &pm8xxx_rtc_ops, THIS_MODULE);

@@ -448,8 +450,6 @@ static int pm8xxx_rtc_probe(struct platform_device *pdev)
 		return rc;
 	}
 
-	device_init_wakeup(&pdev->dev, 1);
-
 	dev_dbg(&pdev->dev, "Probe success !!\n");
 
 	return 0;

Signature · 2 lines

Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

Josh Cartwright

author

2014-03-10 18:44 UTC #12 lore raw

PATCH v2 6/6 documentation: bindings: document PMIC8921/8058 RTC

In reply to Josh Cartwright

This RTC is found on the Qualcomm 8921 and 8058 PMICs.

Signed-off-by Josh Cartwright <[email protected]>

---
 .../devicetree/bindings/rtc/qcom,pm8xxx-rtc.txt     | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/rtc/qcom,pm8xxx-rtc.txt

Documentation/devicetree/bindings/rtc/qcom,pm8xxx-rtc.txt new file +21 -0

new file mode 100644
index 0000000..3eaa582
@@ -0,0 +1,21 @@
+* Real-Time Clock for Qualcomm 8058/8921 PMICs
+
+Required properties:
+- compatible: should be one of the following.
+    * "qcom,pm8058-rtc"
+    * "qcom,pm8921-rtc"
+- reg: base address of the register region
+- interrupts: interrupt list for the RTC, must contain a single interrupt
+              specifier for the alarm interrupt
+
+Optional properties:
+- allow-set-time: indicates that the setting of RTC time is allowed by
+                  the host CPU
+
+Example:
+
+	rtc@11d {
+		compatible = "qcom,pm8921-rtc";
+		reg = <0x11d>;
+		interrupts = <0x27 0>;
+	};

Signature · 2 lines

Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

Rob Herring

2014-03-10 21:22 UTC #13 lore raw

Re: [PATCH v2 6/6] documentation: bindings: document PMIC8921/8058 RTC

Quoted 14 lines · Josh Cartwright

This RTC is found on the Qualcomm 8921 and 8058 PMICs.
Signed-off-by Josh Cartwright <[email protected]>
---
 .../devicetree/bindings/rtc/qcom,pm8xxx-rtc.txt     | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/rtc/qcom,pm8xxx-rtc.txt
Documentation/devicetree/bindings/rtc/qcom,pm8xxx-rtc.txt new file +0 -0
new file mode 100644
index 0000000..3eaa582

Documentation/devicetree/bindings/rtc/qcom,pm8xxx-rtc.txt line 7

Quoted 7 lines

Documentation/devicetree/bindings/rtc/qcom,pm8xxx-rtc.txt new file +7 -0

@@ -0,0 +1,21 @@
+* Real-Time Clock for Qualcomm 8058/8921 PMICs
+
+Required properties:
+- compatible: should be one of the following.
+    * "qcom,pm8058-rtc"
+    * "qcom,pm8921-rtc"
+- reg: base address of the register region

Wait a second. This is all part of an MFD device. I think this whole binding does not make sense on it's own. The DT should describe the PMIC, not necessarily some sub-component of it. Perhaps there are sub nodes of the PMIC node, but that would be all part of the PMIC binding. Having sub drivers for an MFD is really a Linux problem that doesn't belong in DT.

Rob

Josh Cartwright

author

2014-03-10 22:11 UTC #14 lore raw

Re: [PATCH v2 6/6] documentation: bindings: document PMIC8921/8058 RTC

Quoted 21 lines · Rob Herring

Quoted 20 lines · Josh Cartwright
This RTC is found on the Qualcomm 8921 and 8058 PMICs.
Signed-off-by Josh Cartwright <[email protected]>
---
 .../devicetree/bindings/rtc/qcom,pm8xxx-rtc.txt     | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/rtc/qcom,pm8xxx-rtc.txt
Documentation/devicetree/bindings/rtc/qcom,pm8xxx-rtc.txt new file +6 -0
new file mode 100644
index 0000000..3eaa582
@@ -0,0 +1,21 @@
+* Real-Time Clock for Qualcomm 8058/8921 PMICs
+
+Required properties:
+- compatible: should be one of the following.
+    * "qcom,pm8058-rtc"
+    * "qcom,pm8921-rtc"

Quoted 8 lines

Documentation/devicetree/bindings/rtc/qcom,pm8xxx-rtc.txt line 7
Quoted 1 line
Documentation/devicetree/bindings/rtc/qcom,pm8xxx-rtc.txt new file +1 -0
+- reg: base address of the register region
Wait a second. This is all part of an MFD device. I think this whole binding does not make sense on it's own. The DT should describe the PMIC, not necessarily some sub-component of it. Perhaps there are sub nodes of the PMIC node, but that would be all part of the PMIC binding. Having sub drivers for an MFD is really a Linux problem that doesn't belong in DT.

Okay, this makes sense, I can move this documentation to Documentation/bindings/mfd/qcom,pm8xxx.txt where the PMIC binding currently is.

Note that this does introduce a dependency on Lee Jones' mfd tree, where DT support for the parent pm8921 driver is pending for 3.15.

Andrew, how would you prefer I proceed? Would you like me to respin this patch? The whole series? An incremental patch?

Thanks,
Josh

Signature · 6 lines

Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Andrew Morton

2014-03-10 22:18 UTC #15 lore raw

Re: [PATCH v2 6/6] documentation: bindings: document PMIC8921/8058 RTC

Quoted 35 lines · On Mon, 10 Mar 2014 17:11:32 -0500 Josh Cartwright <[email protected]> wrote:

Quoted 21 lines · Rob Herring
Quoted 20 lines · Josh Cartwright
This RTC is found on the Qualcomm 8921 and 8058 PMICs.
Signed-off-by Josh Cartwright <[email protected]>
---
 .../devicetree/bindings/rtc/qcom,pm8xxx-rtc.txt     | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/rtc/qcom,pm8xxx-rtc.txt
Documentation/devicetree/bindings/rtc/qcom,pm8xxx-rtc.txt new file +6 -0
new file mode 100644
index 0000000..3eaa582
@@ -0,0 +1,21 @@
+* Real-Time Clock for Qualcomm 8058/8921 PMICs
+
+Required properties:
+- compatible: should be one of the following.
+    * "qcom,pm8058-rtc"
+    * "qcom,pm8921-rtc"
Quoted 8 lines
Documentation/devicetree/bindings/rtc/qcom,pm8xxx-rtc.txt line 7
Quoted 1 line
Documentation/devicetree/bindings/rtc/qcom,pm8xxx-rtc.txt new file +1 -0
+- reg: base address of the register region
Wait a second. This is all part of an MFD device. I think this whole binding does not make sense on it's own. The DT should describe the PMIC, not necessarily some sub-component of it. Perhaps there are sub nodes of the PMIC node, but that would be all part of the PMIC binding. Having sub drivers for an MFD is really a Linux problem that doesn't belong in DT.
Okay, this makes sense, I can move this documentation to Documentation/bindings/mfd/qcom,pm8xxx.txt where the PMIC binding currently is.

Quoted 5 lines

Note that this does introduce a dependency on Lee Jones' mfd tree, where DT support for the parent pm8921 driver is pending for 3.15.
Andrew, how would you prefer I proceed? Would you like me to respin this patch? The whole series? An incremental patch?

A new version of [6/6] sounds appropriate.

Josh Cartwright

author

2014-03-12 17:17 UTC #16 lore raw

PATCH v3 6/6 mfd: devicetree: bindings: add pm8xxx RTC description

The PM8xxx family of PMICs contain an RTC. This RTC is described as a subnode of the PM8xxx. Document these bindings, and replace the pwrkey node in the example with the RTC, which is now described in this document.

While we're here, add a short description to the device tree bindings describing what the the PM8xxx devices are and how they are expected to be used.

Signed-off-by Josh Cartwright <[email protected]>

---
Andrew-

Here's a new 6/6. Like mentioned earlier, this is based on the current MFD document that's in Lee's tree pending for 3.15. It may be best to get yours and Rob's Ack and have Lee take it through his tree.

Thanks,
Josh

 .../devicetree/bindings/mfd/qcom,pm8xxx.txt        | 45 +++++++++++++++++++---
 1 file changed, 39 insertions(+), 6 deletions(-)

Documentation/devicetree/bindings/mfd/qcom,pm8xxx.txt +39 -6

index e3fe625..03518dc 100644
@@ -1,6 +1,9 @@
 Qualcomm PM8xxx PMIC multi-function devices
 
-PROPERTIES
+The PM8xxx family of Power Management ICs are used to provide regulated
+voltages and other various functionality to Qualcomm SoCs.
+
+= PROPERTIES
 
 - compatible:
 	Usage: required

@@ -45,7 +48,37 @@ PROPERTIES
 	Value type: <empty>
 	Definition: identifies this node as an interrupt controller
 
-EXAMPLE
+= SUBCOMPONENTS
+
+The PMIC contains multiple independent functions, each described in a subnode.
+The below bindings specify the set of valid subnodes.
+
+== Real-Time Clock
+
+- compatible:
+	Usage: required
+	Value type: <string>
+	Definition: must be one of:
+		    "qcom,pm8058-rtc"
+		    "qcom,pm8921-rtc"
+
+- reg:
+	Usage: required
+	Value type: <prop-encoded-array>
+	Definition: single entry specifying the base address of the RTC registers
+
+- interrupts:
+	Usage: required
+	Value type: <prop-encoded-array>
+	Definition: single entry specifying the RTC's alarm interrupt
+
+- allow-set-time:
+	Usage: optional
+	Value type: <empty>
+	Definition: indicates that the setting of RTC time is allowed by
+		    the host CPU
+
+= EXAMPLE
 
 	pmicintc: pmic@0 {
 		compatible = "qcom,pm8921";

@@ -55,9 +88,9 @@ EXAMPLE
 		#address-cells = <1>;
 		#size-cells = <0>;
 
-		pwrkey {
-			compatible = "qcom,pm8921-pwrkey";
-			interrupt-parent = <&pmicintc>;
-			interrupts = <50 1>, <51 1>;
+		rtc@11d {
+			compatible = "qcom,pm8921-rtc";
+			reg = <0x11d>;
+			interrupts = <0x27 0>;
 		};
 	};

Signature · 7 lines

Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Lee Jones

2014-03-21 08:12 UTC #17 lore raw

Re: [PATCH v3 6/6] mfd: devicetree: bindings: add pm8xxx RTC description

Quoted 9 lines

The PM8xxx family of PMICs contain an RTC. This RTC is described as a subnode of the PM8xxx. Document these bindings, and replace the pwrkey node in the example with the RTC, which is now described in this document.
While we're here, add a short description to the device tree bindings describing what the the PM8xxx devices are and how they are expected to be used.

Quoted 7 lines

Signed-off-by Josh Cartwright <[email protected]>
---
Andrew-
Here's a new 6/6. Like mentioned earlier, this is based on the current MFD document that's in Lee's tree pending for 3.15. It may be best to get yours and Rob's Ack and have Lee take it through his tree.

After discussing with Andrew, I think this is better to go through my tree as you suggested, in order to mitigate any merge conflicts during the merge window.

Patch applied to the MFD tree with Andrew's Ack, thanks.

Signature · 4 lines

Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog