* [PATCH 1/6] rtc: pm8xxx: fixup checkpatch/style issues
[not found] <1394047776-13827-1-git-send-email-joshc@codeaurora.org>
@ 2014-03-05 19:29 ` Josh Cartwright
2014-03-05 21:02 ` Stephen Boyd
2014-03-05 19:29 ` [PATCH 2/6] rtc: pm8xxx: use regmap API for register accesses Josh Cartwright
` (4 subsequent siblings)
5 siblings, 1 reply; 20+ messages in thread
From: Josh Cartwright @ 2014-03-05 19:29 UTC (permalink / raw)
To: Alessandro Zummo; +Cc: linux-arm-msm, Stephen Boyd, rtc-linux, linux-kernel
Before performing additional cleanups to this driver, do the easy
cleanups first.
Signed-off-by: Josh Cartwright <joshc@codeaurora.org>
---
drivers/rtc/rtc-pm8xxx.c | 88 +++++++++++++++++++++++++-----------------------
1 file changed, 45 insertions(+), 43 deletions(-)
diff --git a/drivers/rtc/rtc-pm8xxx.c b/drivers/rtc/rtc-pm8xxx.c
index bd76ffe9..772b070 100644
--- a/drivers/rtc/rtc-pm8xxx.c
+++ b/drivers/rtc/rtc-pm8xxx.c
@@ -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,7 +245,7 @@ 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;
@@ -253,7 +253,7 @@ static int pm8xxx_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alarm)
ctrl_reg = rtc_dd->ctrl_reg;
ctrl_reg = alarm->enabled ? (ctrl_reg | PM8xxx_RTC_ALARM_ENABLE) :
- (ctrl_reg & ~PM8xxx_RTC_ALARM_ENABLE);
+ (ctrl_reg & ~PM8xxx_RTC_ALARM_ENABLE);
rc = pm8xxx_write_wrapper(rtc_dd, &ctrl_reg, rtc_dd->rtc_base, 1);
if (rc < 0) {
@@ -264,9 +264,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 +280,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 +297,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;
}
@@ -313,8 +313,8 @@ static int pm8xxx_rtc_alarm_irq_enable(struct device *dev, unsigned int enable)
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);
+ ctrl_reg = enable ? (ctrl_reg | PM8xxx_RTC_ALARM_ENABLE) :
+ (ctrl_reg & ~PM8xxx_RTC_ALARM_ENABLE);
rc = pm8xxx_write_wrapper(rtc_dd, &ctrl_reg, rtc_dd->rtc_base, 1);
if (rc < 0) {
@@ -354,8 +354,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 +364,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 +409,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 +436,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 +505,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,
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 2/6] rtc: pm8xxx: use regmap API for register accesses
[not found] <1394047776-13827-1-git-send-email-joshc@codeaurora.org>
2014-03-05 19:29 ` [PATCH 1/6] rtc: pm8xxx: fixup checkpatch/style issues Josh Cartwright
@ 2014-03-05 19:29 ` Josh Cartwright
2014-03-05 20:59 ` Stephen Boyd
2014-03-05 19:29 ` [PATCH 3/6] rtc: pm8xxx: use devm_request_any_context_irq Josh Cartwright
` (3 subsequent siblings)
5 siblings, 1 reply; 20+ messages in thread
From: Josh Cartwright @ 2014-03-05 19:29 UTC (permalink / raw)
To: Alessandro Zummo; +Cc: linux-arm-msm, Stephen Boyd, rtc-linux, linux-kernel
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.
Signed-off-by: Josh Cartwright <joshc@codeaurora.org>
---
drivers/rtc/rtc-pm8xxx.c | 145 +++++++++++++++++++----------------------------
1 file changed, 57 insertions(+), 88 deletions(-)
diff --git a/drivers/rtc/rtc-pm8xxx.c b/drivers/rtc/rtc-pm8xxx.c
index 772b070..f6d140a 100644
--- a/drivers/rtc/rtc-pm8xxx.c
+++ b/drivers/rtc/rtc-pm8xxx.c
@@ -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, ®, 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, ®, rtc_dd->rtc_read_base, 1);
+ rc = regmap_read(rtc_dd->regmap, rtc_dd->rtc_read_base, ®);
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;
}
@@ -255,8 +217,8 @@ static int pm8xxx_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alarm)
ctrl_reg = alarm->enabled ? (ctrl_reg | PM8xxx_RTC_ALARM_ENABLE) :
(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;
}
@@ -279,9 +241,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;
}
@@ -316,8 +278,8 @@ static int pm8xxx_rtc_alarm_irq_enable(struct device *dev, unsigned int enable)
ctrl_reg = enable ? (ctrl_reg | PM8xxx_RTC_ALARM_ENABLE) :
(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;
}
@@ -339,7 +301,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;
@@ -351,8 +313,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");
@@ -363,18 +325,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");
@@ -385,7 +349,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;
@@ -402,6 +366,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");
@@ -425,17 +395,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;
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 3/6] rtc: pm8xxx: use devm_request_any_context_irq
[not found] <1394047776-13827-1-git-send-email-joshc@codeaurora.org>
2014-03-05 19:29 ` [PATCH 1/6] rtc: pm8xxx: fixup checkpatch/style issues Josh Cartwright
2014-03-05 19:29 ` [PATCH 2/6] rtc: pm8xxx: use regmap API for register accesses Josh Cartwright
@ 2014-03-05 19:29 ` Josh Cartwright
2014-03-05 20:59 ` Stephen Boyd
2014-03-05 19:29 ` [PATCH 4/6] rtc: pm8xxx: add support for devicetree Josh Cartwright
` (2 subsequent siblings)
5 siblings, 1 reply; 20+ messages in thread
From: Josh Cartwright @ 2014-03-05 19:29 UTC (permalink / raw)
To: Alessandro Zummo; +Cc: linux-arm-msm, Stephen Boyd, rtc-linux, linux-kernel
Make use of the devm_* variant of request_any_context_irq to allow for
elimination of remove().
Signed-off-by: Josh Cartwright <joshc@codeaurora.org>
---
drivers/rtc/rtc-pm8xxx.c | 18 ++++--------------
1 file changed, 4 insertions(+), 14 deletions(-)
diff --git a/drivers/rtc/rtc-pm8xxx.c b/drivers/rtc/rtc-pm8xxx.c
index f6d140a..ed3fe83 100644
--- a/drivers/rtc/rtc-pm8xxx.c
+++ b/drivers/rtc/rtc-pm8xxx.c
@@ -427,9 +427,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;
@@ -442,16 +443,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)
{
@@ -480,7 +471,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,
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 4/6] rtc: pm8xxx: add support for devicetree
[not found] <1394047776-13827-1-git-send-email-joshc@codeaurora.org>
` (2 preceding siblings ...)
2014-03-05 19:29 ` [PATCH 3/6] rtc: pm8xxx: use devm_request_any_context_irq Josh Cartwright
@ 2014-03-05 19:29 ` Josh Cartwright
2014-03-05 20:59 ` Stephen Boyd
2014-03-06 9:18 ` Lee Jones
2014-03-05 19:29 ` [PATCH 5/6] rtc: pm8xxx: move device_init_wakeup() before rtc_register Josh Cartwright
2014-03-05 19:29 ` [PATCH 6/6] documentation: bindings: document PMIC8921/8058 RTC Josh Cartwright
5 siblings, 2 replies; 20+ messages in thread
From: Josh Cartwright @ 2014-03-05 19:29 UTC (permalink / raw)
To: Alessandro Zummo, David Brown, Daniel Walker, Bryan Huntsman,
Grant Likely, Rob Herring
Cc: linux-arm-msm, Stephen Boyd, Samuel Ortiz, Lee Jones,
linux-kernel, rtc-linux, devicetree
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 <joshc@codeaurora.org>
---
drivers/rtc/rtc-pm8xxx.c | 54 +++++++++++++++++++++++-------------------
include/linux/mfd/pm8xxx/rtc.h | 25 -------------------
2 files changed, 30 insertions(+), 49 deletions(-)
delete mode 100644 include/linux/mfd/pm8xxx/rtc.h
diff --git a/drivers/rtc/rtc-pm8xxx.c b/drivers/rtc/rtc-pm8xxx.c
index ed3fe83..cb5576f 100644
--- a/drivers/rtc/rtc-pm8xxx.c
+++ b/drivers/rtc/rtc-pm8xxx.c
@@ -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++) {
@@ -291,8 +293,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,
@@ -346,18 +349,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)
@@ -372,20 +383,16 @@ static int pm8xxx_rtc_probe(struct platform_device *pdev)
return -ENXIO;
}
- rtc_dd->rtc_alarm_irq = platform_get_irq(pdev, 0);
+ rtc_dd->rtc_alarm_irq = platform_get_irq_byname(pdev, "alarm");
if (rtc_dd->rtc_alarm_irq < 0) {
dev_err(&pdev->dev, "Alarm IRQ resource absent!\n");
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,
+ "linux,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;
@@ -412,8 +419,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);
@@ -472,9 +477,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,
},
};
diff --git a/include/linux/mfd/pm8xxx/rtc.h b/include/linux/mfd/pm8xxx/rtc.h
deleted file mode 100644
index 14f1983..0000000
--- a/include/linux/mfd/pm8xxx/rtc.h
+++ /dev/null
@@ -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__ */
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 5/6] rtc: pm8xxx: move device_init_wakeup() before rtc_register
[not found] <1394047776-13827-1-git-send-email-joshc@codeaurora.org>
` (3 preceding siblings ...)
2014-03-05 19:29 ` [PATCH 4/6] rtc: pm8xxx: add support for devicetree Josh Cartwright
@ 2014-03-05 19:29 ` Josh Cartwright
2014-03-05 20:59 ` Stephen Boyd
2014-03-05 19:29 ` [PATCH 6/6] documentation: bindings: document PMIC8921/8058 RTC Josh Cartwright
5 siblings, 1 reply; 20+ messages in thread
From: Josh Cartwright @ 2014-03-05 19:29 UTC (permalink / raw)
To: Alessandro Zummo; +Cc: linux-arm-msm, Stephen Boyd, rtc-linux, linux-kernel
Setup wakeup capability before rtc_register to ensure the rtc class core
properly sets up our 'wakealarm' sysfs attribute.
Signed-off-by: Josh Cartwright <joshc@codeaurora.org>
---
drivers/rtc/rtc-pm8xxx.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/rtc/rtc-pm8xxx.c b/drivers/rtc/rtc-pm8xxx.c
index cb5576f..761be65 100644
--- a/drivers/rtc/rtc-pm8xxx.c
+++ b/drivers/rtc/rtc-pm8xxx.c
@@ -422,6 +422,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);
@@ -441,8 +443,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;
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 6/6] documentation: bindings: document PMIC8921/8058 RTC
[not found] <1394047776-13827-1-git-send-email-joshc@codeaurora.org>
` (4 preceding siblings ...)
2014-03-05 19:29 ` [PATCH 5/6] rtc: pm8xxx: move device_init_wakeup() before rtc_register Josh Cartwright
@ 2014-03-05 19:29 ` Josh Cartwright
2014-03-05 20:58 ` Stephen Boyd
5 siblings, 1 reply; 20+ messages in thread
From: Josh Cartwright @ 2014-03-05 19:29 UTC (permalink / raw)
To: Alessandro Zummo, linux-kernel
Cc: linux-arm-msm, Stephen Boyd, Rob Herring, Pawel Moll,
Mark Rutland, Ian Campbell, Kumar Gala, Rob Landley, devicetree,
linux-doc
This RTC is found on the Qualcomm 8921 and 8058 PMICs.
Signed-off-by: Josh Cartwright <joshc@codeaurora.org>
---
.../devicetree/bindings/rtc/qcom,pm8xxx-rtc.txt | 29 ++++++++++++++++++++++
1 file changed, 29 insertions(+)
create mode 100644 Documentation/devicetree/bindings/rtc/qcom,pm8xxx-rtc.txt
diff --git a/Documentation/devicetree/bindings/rtc/qcom,pm8xxx-rtc.txt b/Documentation/devicetree/bindings/rtc/qcom,pm8xxx-rtc.txt
new file mode 100644
index 0000000..699bd30
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/qcom,pm8xxx-rtc.txt
@@ -0,0 +1,29 @@
+* 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
+- reg-names: corresponding reg names for the regions listed in the 'reg'
+ property, must contain:
+ "rtc_base" - base of the RTC register region
+- interrupts: interrupt list for the RTC, must contain a single interrupt
+ specifier for the alarm interrupt
+- interrupt-names: corresponding interrupt names for the interrupts listed in
+ the 'interrupts' property, must contain:
+ "alarm" - summary interrupt for PMIC peripherals
+
+Option properties:
+- linux,allow-set-time: indicates that the setting of RTC time is allowed by
+ the host CPU
+
+Example:
+
+ rtc {
+ compatible = "qcom,pm8921-rtc";
+ reg = <0x11D>;
+ reg-names = "rtc_base";
+ interrupts = <0x27 0>;
+ interrupt-names = "alarm";
+ };
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 6/6] documentation: bindings: document PMIC8921/8058 RTC
2014-03-05 19:29 ` [PATCH 6/6] documentation: bindings: document PMIC8921/8058 RTC Josh Cartwright
@ 2014-03-05 20:58 ` Stephen Boyd
2014-03-06 0:00 ` Josh Cartwright
0 siblings, 1 reply; 20+ messages in thread
From: Stephen Boyd @ 2014-03-05 20:58 UTC (permalink / raw)
To: Josh Cartwright
Cc: Alessandro Zummo, linux-kernel, linux-arm-msm, Rob Herring,
Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Rob Landley,
devicetree, linux-doc
On 03/05/14 11:29, Josh Cartwright wrote:
> diff --git a/Documentation/devicetree/bindings/rtc/qcom,pm8xxx-rtc.txt b/Documentation/devicetree/bindings/rtc/qcom,pm8xxx-rtc.txt
> new file mode 100644
> index 0000000..699bd30
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/rtc/qcom,pm8xxx-rtc.txt
> @@ -0,0 +1,29 @@
> +* 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
> +- reg-names: corresponding reg names for the regions listed in the 'reg'
> + property, must contain:
> + "rtc_base" - base of the RTC register region
optional reg-names?
> +- interrupts: interrupt list for the RTC, must contain a single interrupt
> + specifier for the alarm interrupt
> +- interrupt-names: corresponding interrupt names for the interrupts listed in
> + the 'interrupts' property, must contain:
> + "alarm" - summary interrupt for PMIC peripherals
optional interrupt-names?
> +
> +Option properties:
> +- linux,allow-set-time: indicates that the setting of RTC time is allowed by
> + the host CPU
Is this a "linux" property? It seems like something that other OSes
would want to know about and doesn't require any explicit knowledge
about operating system things (like keymaps for example).
> +
> +Example:
> +
> + rtc {
rtc@11d
> + compatible = "qcom,pm8921-rtc";
> + reg = <0x11D>;
> + reg-names = "rtc_base";
> + interrupts = <0x27 0>;
> + interrupt-names = "alarm";
> + };
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 4/6] rtc: pm8xxx: add support for devicetree
2014-03-05 19:29 ` [PATCH 4/6] rtc: pm8xxx: add support for devicetree Josh Cartwright
@ 2014-03-05 20:59 ` Stephen Boyd
2014-03-06 9:18 ` Lee Jones
1 sibling, 0 replies; 20+ messages in thread
From: Stephen Boyd @ 2014-03-05 20:59 UTC (permalink / raw)
To: Josh Cartwright
Cc: Alessandro Zummo, David Brown, Daniel Walker, Bryan Huntsman,
Grant Likely, Rob Herring, linux-arm-msm, Samuel Ortiz, Lee Jones,
linux-kernel, rtc-linux, devicetree
On 03/05/14 11:29, Josh Cartwright 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
>
> Signed-off-by: Josh Cartwright <joshc@codeaurora.org>
>
Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 5/6] rtc: pm8xxx: move device_init_wakeup() before rtc_register
2014-03-05 19:29 ` [PATCH 5/6] rtc: pm8xxx: move device_init_wakeup() before rtc_register Josh Cartwright
@ 2014-03-05 20:59 ` Stephen Boyd
0 siblings, 0 replies; 20+ messages in thread
From: Stephen Boyd @ 2014-03-05 20:59 UTC (permalink / raw)
To: Josh Cartwright; +Cc: Alessandro Zummo, linux-arm-msm, rtc-linux, linux-kernel
On 03/05/14 11:29, Josh Cartwright wrote:
> Setup wakeup capability before rtc_register to ensure the rtc class core
> properly sets up our 'wakealarm' sysfs attribute.
>
> Signed-off-by: Josh Cartwright <joshc@codeaurora.org>
Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/6] rtc: pm8xxx: use regmap API for register accesses
2014-03-05 19:29 ` [PATCH 2/6] rtc: pm8xxx: use regmap API for register accesses Josh Cartwright
@ 2014-03-05 20:59 ` Stephen Boyd
0 siblings, 0 replies; 20+ messages in thread
From: Stephen Boyd @ 2014-03-05 20:59 UTC (permalink / raw)
To: Josh Cartwright; +Cc: Alessandro Zummo, linux-arm-msm, rtc-linux, linux-kernel
On 03/05/14 11:29, Josh Cartwright 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.
>
> Signed-off-by: Josh Cartwright <joshc@codeaurora.org>
Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/6] rtc: pm8xxx: use devm_request_any_context_irq
2014-03-05 19:29 ` [PATCH 3/6] rtc: pm8xxx: use devm_request_any_context_irq Josh Cartwright
@ 2014-03-05 20:59 ` Stephen Boyd
0 siblings, 0 replies; 20+ messages in thread
From: Stephen Boyd @ 2014-03-05 20:59 UTC (permalink / raw)
To: Josh Cartwright; +Cc: Alessandro Zummo, linux-arm-msm, rtc-linux, linux-kernel
On 03/05/14 11:29, Josh Cartwright wrote:
> Make use of the devm_* variant of request_any_context_irq to allow for
> elimination of remove().
>
> Signed-off-by: Josh Cartwright <joshc@codeaurora.org>
Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/6] rtc: pm8xxx: fixup checkpatch/style issues
2014-03-05 19:29 ` [PATCH 1/6] rtc: pm8xxx: fixup checkpatch/style issues Josh Cartwright
@ 2014-03-05 21:02 ` Stephen Boyd
2014-03-05 21:54 ` Joe Perches
2014-03-05 23:51 ` Josh Cartwright
0 siblings, 2 replies; 20+ messages in thread
From: Stephen Boyd @ 2014-03-05 21:02 UTC (permalink / raw)
To: Josh Cartwright; +Cc: Alessandro Zummo, linux-arm-msm, rtc-linux, linux-kernel
On 03/05/14 11:29, Josh Cartwright wrote:
> Before performing additional cleanups to this driver, do the easy
> cleanups first.
>
> Signed-off-by: Josh Cartwright <joshc@codeaurora.org>
Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
> @@ -253,7 +253,7 @@ static int pm8xxx_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alarm)
>
> ctrl_reg = rtc_dd->ctrl_reg;
> ctrl_reg = alarm->enabled ? (ctrl_reg | PM8xxx_RTC_ALARM_ENABLE) :
> - (ctrl_reg & ~PM8xxx_RTC_ALARM_ENABLE);
> + (ctrl_reg & ~PM8xxx_RTC_ALARM_ENABLE);
>
> rc = pm8xxx_write_wrapper(rtc_dd, &ctrl_reg, rtc_dd->rtc_base, 1);
> if (rc < 0) {
This could be better style with more lines:
if (alarm->enabled)
ctrl_reg |= PM8xxx_RTC_ALARM_ENABLE;
else
ctrl_reg &= ~PM8xxx_RTC_ALARM_ENABLE;
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/6] rtc: pm8xxx: fixup checkpatch/style issues
2014-03-05 21:02 ` Stephen Boyd
@ 2014-03-05 21:54 ` Joe Perches
2014-03-05 23:51 ` Josh Cartwright
1 sibling, 0 replies; 20+ messages in thread
From: Joe Perches @ 2014-03-05 21:54 UTC (permalink / raw)
To: Stephen Boyd
Cc: Josh Cartwright, Alessandro Zummo, linux-arm-msm, rtc-linux,
linux-kernel
On Wed, 2014-03-05 at 13:02 -0800, Stephen Boyd wrote:
> On 03/05/14 11:29, Josh Cartwright wrote:
> > Before performing additional cleanups to this driver, do the easy
> > cleanups first.
> >
> > Signed-off-by: Josh Cartwright <joshc@codeaurora.org>
>
> Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
>
> > @@ -253,7 +253,7 @@ static int pm8xxx_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alarm)
> >
> > ctrl_reg = rtc_dd->ctrl_reg;
> > ctrl_reg = alarm->enabled ? (ctrl_reg | PM8xxx_RTC_ALARM_ENABLE) :
> > - (ctrl_reg & ~PM8xxx_RTC_ALARM_ENABLE);
> > + (ctrl_reg & ~PM8xxx_RTC_ALARM_ENABLE);
> >
> > rc = pm8xxx_write_wrapper(rtc_dd, &ctrl_reg, rtc_dd->rtc_base, 1);
> > if (rc < 0) {
>
> This could be better style with more lines:
>
> if (alarm->enabled)
> ctrl_reg |= PM8xxx_RTC_ALARM_ENABLE;
> else
> ctrl_reg &= ~PM8xxx_RTC_ALARM_ENABLE;
>
I agree.
Maybe something like a set_or_clear_bits macro similar to clamp
could be useful.
Maybe:
#define set_or_clear_bits(value, test, bits) \
({ \
typeof value _value; \
if (test) \
_value = (value) | (bits); \
else \
_value = (value) & (~(bits)); \
_value; \
})
so the code above could be something like:
ctrl_reg = set_or_clear_bits(ctrl_reg, alarm->enabled, PM8xxx_RTC_ALARM_ENABLE);
(or some other better macro name, or maybe not type
the value name twice and have it set by the macro)
There's at least a few dozen like alarm->enabled use
above that could be converted in drivers/.
( a few false positives here too )
$ grep-2.5.4 -rP --include=*.[ch] "([\w\.\>\[\]\-]+)\s*=[^;\?]+\?\s*\(?\s*\1\s*[\&\|][^;]+;" drivers/
drivers/input/misc/twl4030-vibra.c: reg = (dir) ? (reg | TWL4030_VIBRA_DIR) :
(reg & ~TWL4030_VIBRA_DIR);
drivers/ata/libata-scsi.c: tf->device = dev->devno ?
tf->device | ATA_DEV1 : tf->device & ~ATA_DEV1;
drivers/ata/pata_samsung_cf.c: reg = mode ? (reg & ~S3C_ATA_CFG_SWAP) : (reg | S3C_ATA_CFG_SWAP);
drivers/ata/pata_samsung_cf.c: temp = state ? (temp | 1) : (temp & ~1);
drivers/staging/iio/accel/lis3l02dq_core.c: control = val & 0x3f ?
(control | LIS3L02DQ_REG_CTRL_2_ENABLE_INTERRUPT) :
(control & ~LIS3L02DQ_REG_CTRL_2_ENABLE_INTERRUPT);
drivers/staging/media/sn9c102/sn9c102_core.c: rect->left = (s->_rect.left & 1L) ? rect->left | 1L : rect->left & ~1L;
drivers/staging/media/sn9c102/sn9c102_core.c: rect->top = (s->_rect.top & 1L) ? rect->top | 1L : rect->top & ~1L;
drivers/message/fusion/mptsas.c: ioc->device_missing_delay = (device_missing_delay & MPI_SAS_IOUNIT1_REPORT_MISSING_UNIT_16) ?
(device_missing_delay & MPI_SAS_IOUNIT1_REPORT_MISSING_TIMEOUT_MASK) * 16 :
device_missing_delay & MPI_SAS_IOUNIT1_REPORT_MISSING_TIMEOUT_MASK;
drivers/gpio/gpio-max732x.c: reg_out = (val) ? reg_out | mask : reg_out & ~mask;
drivers/net/ethernet/broadcom/bnx2x/bnx2x_sp.c: mac_filters->ucast_drop_all = drop_all_ucast ?
mac_filters->ucast_drop_all | mask :
mac_filters->ucast_drop_all & ~mask;
drivers/net/ethernet/broadcom/bnx2x/bnx2x_sp.c: mac_filters->mcast_drop_all = drop_all_mcast ?
mac_filters->mcast_drop_all | mask :
mac_filters->mcast_drop_all & ~mask;
drivers/net/ethernet/broadcom/bnx2x/bnx2x_sp.c: mac_filters->ucast_accept_all = accp_all_ucast ?
mac_filters->ucast_accept_all | mask :
mac_filters->ucast_accept_all & ~mask;
drivers/net/ethernet/broadcom/bnx2x/bnx2x_sp.c: mac_filters->mcast_accept_all = accp_all_mcast ?
mac_filters->mcast_accept_all | mask :
mac_filters->mcast_accept_all & ~mask;
drivers/net/ethernet/broadcom/bnx2x/bnx2x_sp.c: mac_filters->bcast_accept_all = accp_all_bcast ?
mac_filters->bcast_accept_all | mask :
mac_filters->bcast_accept_all & ~mask;
drivers/net/ethernet/broadcom/bnx2x/bnx2x_sp.c: mac_filters->unmatched_unicast = unmatched_unicast ?
mac_filters->unmatched_unicast | mask :
mac_filters->unmatched_unicast & ~mask;
drivers/net/ethernet/broadcom/bnx2x/bnx2x_init.h: reg_bit_map = new_cos ?
(reg_bit_map | q_bit_map) :
(reg_bit_map & (~q_bit_map));
drivers/ide/pdc202xx_old.c: word_count = (rq_data_dir(rq) == READ) ?
word_count | 0x05000000 :
word_count | 0x06000000;
drivers/spi/spi-fsl-spi.c: slvsel = on ? (slvsel | (1 << cs)) : (slvsel & ~(1 << cs));
drivers/rtc/rtc-pm8xxx.c: ctrl_reg = alarm->enabled ? (ctrl_reg | PM8xxx_RTC_ALARM_ENABLE) :
(ctrl_reg & ~PM8xxx_RTC_ALARM_ENABLE);
drivers/rtc/rtc-pm8xxx.c: ctrl_reg = (enable) ? (ctrl_reg | PM8xxx_RTC_ALARM_ENABLE) :
(ctrl_reg & ~PM8xxx_RTC_ALARM_ENABLE);
drivers/platform/x86/acer-wmi.c: set_params.devices = (value) ? (devices | device) : (devices & ~device);
drivers/media/i2c/ov9650.c: reg = awb ? reg | REG_COM8 : reg & ~REG_COM8;
drivers/media/i2c/ov9650.c: com14 = value ? com14 | COM14_EDGE_EN : com14 & ~COM14_EDGE_EN;
drivers/media/i2c/ov9650.c: reg = value ? reg | COM23_TEST_MODE : reg & ~COM23_TEST_MODE;
drivers/media/i2c/s5k6aa.c: reg = awb ? reg | AALG_WB_EN_MASK : reg & ~AALG_WB_EN_MASK;
drivers/media/usb/dvb-usb-v2/lmedm04.c: cold = (cold > 0) ? (cold & 1) : 0;
drivers/media/usb/gspca/m5602/m5602_s5k83a.c: data[0] = (vflip) ? data[0] | 0x40 : data[0];
drivers/media/usb/gspca/m5602/m5602_s5k83a.c: data[0] = (hflip) ? data[0] | 0x80 : data[0];
drivers/media/dvb-frontends/dib3000mb.c: pid = (onoff ? pid | DIB3000_ACTIVATE_PID_FILTERING : 0);
drivers/video/s1d13xxxfb.c: regno=((regno & 1) ? (regno & ~1L) : (regno + 1));
drivers/video/s1d13xxxfb.c: regno=((regno & 1) ? (regno & ~1L) : (regno + 1));
drivers/video/i810/i810_main.c: val = (mode == OFF) ? val | SCR_OFF :
val & ~SCR_OFF;
drivers/video/i810/i810_main.c: reg = (mode == OFF) ? reg & ~0x80 :
reg | 0x80;
drivers/video/i810/i810_main.c: temp = (mode == ON) ? temp | CURSOR_ENABLE_MASK :
temp & ~CURSOR_ENABLE_MASK;
drivers/atm/eni.c: tonga = (address >> j) & 1 ? tonga | SEPROM_DATA :
tonga & ~SEPROM_DATA;
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/6] rtc: pm8xxx: fixup checkpatch/style issues
2014-03-05 21:02 ` Stephen Boyd
2014-03-05 21:54 ` Joe Perches
@ 2014-03-05 23:51 ` Josh Cartwright
1 sibling, 0 replies; 20+ messages in thread
From: Josh Cartwright @ 2014-03-05 23:51 UTC (permalink / raw)
To: Stephen Boyd
Cc: Josh Cartwright, Alessandro Zummo, linux-arm-msm, rtc-linux,
linux-kernel
On Wed, Mar 05, 2014 at 01:02:29PM -0800, Stephen Boyd wrote:
> On 03/05/14 11:29, Josh Cartwright wrote:
> > Before performing additional cleanups to this driver, do the easy
> > cleanups first.
> >
> > Signed-off-by: Josh Cartwright <joshc@codeaurora.org>
>
> Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
Thanks for the reviews.
> > @@ -253,7 +253,7 @@ static int pm8xxx_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alarm)
> >
> > ctrl_reg = rtc_dd->ctrl_reg;
> > ctrl_reg = alarm->enabled ? (ctrl_reg | PM8xxx_RTC_ALARM_ENABLE) :
> > - (ctrl_reg & ~PM8xxx_RTC_ALARM_ENABLE);
> > + (ctrl_reg & ~PM8xxx_RTC_ALARM_ENABLE);
> >
> > rc = pm8xxx_write_wrapper(rtc_dd, &ctrl_reg, rtc_dd->rtc_base, 1);
> > if (rc < 0) {
>
> This could be better style with more lines:
>
> if (alarm->enabled)
> ctrl_reg |= PM8xxx_RTC_ALARM_ENABLE;
> else
> ctrl_reg &= ~PM8xxx_RTC_ALARM_ENABLE;
Agreed, I'll clean this up in a v2.
I might go back and see just how much can be cleanup up to use the
regmap_update_bits() API as well.
Thanks,
Josh
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 6/6] documentation: bindings: document PMIC8921/8058 RTC
2014-03-05 20:58 ` Stephen Boyd
@ 2014-03-06 0:00 ` Josh Cartwright
2014-03-06 1:31 ` Stephen Boyd
2014-03-10 15:35 ` Rob Herring
0 siblings, 2 replies; 20+ messages in thread
From: Josh Cartwright @ 2014-03-06 0:00 UTC (permalink / raw)
To: Stephen Boyd
Cc: Alessandro Zummo, linux-kernel, linux-arm-msm, Rob Herring,
Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Rob Landley,
devicetree, linux-doc
On Wed, Mar 05, 2014 at 12:58:55PM -0800, Stephen Boyd wrote:
> On 03/05/14 11:29, Josh Cartwright wrote:
> > diff --git a/Documentation/devicetree/bindings/rtc/qcom,pm8xxx-rtc.txt b/Documentation/devicetree/bindings/rtc/qcom,pm8xxx-rtc.txt
> > new file mode 100644
> > index 0000000..699bd30
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/rtc/qcom,pm8xxx-rtc.txt
> > @@ -0,0 +1,29 @@
> > +* 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
> > +- reg-names: corresponding reg names for the regions listed in the 'reg'
> > + property, must contain:
> > + "rtc_base" - base of the RTC register region
>
> optional reg-names?
>
> > +- interrupts: interrupt list for the RTC, must contain a single interrupt
> > + specifier for the alarm interrupt
> > +- interrupt-names: corresponding interrupt names for the interrupts listed in
> > + the 'interrupts' property, must contain:
> > + "alarm" - summary interrupt for PMIC peripherals
>
> optional interrupt-names?
It isn't clear to me why these should be made optional, I hope Rob
provides some clarification in the sdhci-msm thread.
> > +
> > +Option properties:
Woops, this should be "Optional" :).
> > +- linux,allow-set-time: indicates that the setting of RTC time is allowed by
> > + the host CPU
>
> Is this a "linux" property? It seems like something that other OSes
> would want to know about and doesn't require any explicit knowledge
> about operating system things (like keymaps for example).
Yeah, I wasn't quite sure how to name this property. It's
Linux-specific in the sense that the underlying operation method is
"set_time", but I agree this should be named something else...
How do you feel about simply "allow-set-time"?
>
> > +
> > +Example:
> > +
> > + rtc {
>
> rtc@11d
Yep, thanks.
> > + compatible = "qcom,pm8921-rtc";
> > + reg = <0x11D>;
> > + reg-names = "rtc_base";
> > + interrupts = <0x27 0>;
> > + interrupt-names = "alarm";
> > + };
> >
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 6/6] documentation: bindings: document PMIC8921/8058 RTC
2014-03-06 0:00 ` Josh Cartwright
@ 2014-03-06 1:31 ` Stephen Boyd
2014-03-07 19:01 ` Josh Cartwright
2014-03-10 15:35 ` Rob Herring
1 sibling, 1 reply; 20+ messages in thread
From: Stephen Boyd @ 2014-03-06 1:31 UTC (permalink / raw)
To: Josh Cartwright
Cc: Alessandro Zummo, linux-kernel, linux-arm-msm, Rob Herring,
Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Rob Landley,
devicetree, linux-doc
On 03/05/14 16:00, Josh Cartwright wrote:
> On Wed, Mar 05, 2014 at 12:58:55PM -0800, Stephen Boyd wrote:
>> On 03/05/14 11:29, Josh Cartwright wrote:
>>> diff --git a/Documentation/devicetree/bindings/rtc/qcom,pm8xxx-rtc.txt b/Documentation/devicetree/bindings/rtc/qcom,pm8xxx-rtc.txt
>>> new file mode 100644
>>> index 0000000..699bd30
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/rtc/qcom,pm8xxx-rtc.txt
>>> @@ -0,0 +1,29 @@
>>> +* 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
>>> +- reg-names: corresponding reg names for the regions listed in the 'reg'
>>> + property, must contain:
>>> + "rtc_base" - base of the RTC register region
>> optional reg-names?
>>
>>> +- interrupts: interrupt list for the RTC, must contain a single interrupt
>>> + specifier for the alarm interrupt
>>> +- interrupt-names: corresponding interrupt names for the interrupts listed in
>>> + the 'interrupts' property, must contain:
>>> + "alarm" - summary interrupt for PMIC peripherals
>> optional interrupt-names?
> It isn't clear to me why these should be made optional, I hope Rob
> provides some clarification in the sdhci-msm thread.
Looks like the driver isn't using either of these properties, so I'm not
sure why they're needed. Maybe they should just be removed.
>
>
>>> +- linux,allow-set-time: indicates that the setting of RTC time is allowed by
>>> + the host CPU
>> Is this a "linux" property? It seems like something that other OSes
>> would want to know about and doesn't require any explicit knowledge
>> about operating system things (like keymaps for example).
> Yeah, I wasn't quite sure how to name this property. It's
> Linux-specific in the sense that the underlying operation method is
> "set_time", but I agree this should be named something else...
>
> How do you feel about simply "allow-set-time"?
Sounds ok to me.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 4/6] rtc: pm8xxx: add support for devicetree
2014-03-05 19:29 ` [PATCH 4/6] rtc: pm8xxx: add support for devicetree Josh Cartwright
2014-03-05 20:59 ` Stephen Boyd
@ 2014-03-06 9:18 ` Lee Jones
1 sibling, 0 replies; 20+ messages in thread
From: Lee Jones @ 2014-03-06 9:18 UTC (permalink / raw)
To: Josh Cartwright
Cc: Alessandro Zummo, David Brown, Daniel Walker, Bryan Huntsman,
Grant Likely, Rob Herring, linux-arm-msm, Stephen Boyd,
Samuel Ortiz, linux-kernel, rtc-linux, devicetree
> 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 <joshc@codeaurora.org>
> ---
> drivers/rtc/rtc-pm8xxx.c | 54 +++++++++++++++++++++++-------------------
> include/linux/mfd/pm8xxx/rtc.h | 25 -------------------
For the MFD changes@:
Acked-by: Lee Jones <lee.jones@linaro.org>
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 6/6] documentation: bindings: document PMIC8921/8058 RTC
2014-03-06 1:31 ` Stephen Boyd
@ 2014-03-07 19:01 ` Josh Cartwright
0 siblings, 0 replies; 20+ messages in thread
From: Josh Cartwright @ 2014-03-07 19:01 UTC (permalink / raw)
To: Stephen Boyd
Cc: Alessandro Zummo, linux-kernel, linux-arm-msm, Rob Herring,
Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Rob Landley,
devicetree, linux-doc
On Wed, Mar 05, 2014 at 05:31:27PM -0800, Stephen Boyd wrote:
> On 03/05/14 16:00, Josh Cartwright wrote:
> > On Wed, Mar 05, 2014 at 12:58:55PM -0800, Stephen Boyd wrote:
> >> On 03/05/14 11:29, Josh Cartwright wrote:
> >>> diff --git a/Documentation/devicetree/bindings/rtc/qcom,pm8xxx-rtc.txt b/Documentation/devicetree/bindings/rtc/qcom,pm8xxx-rtc.txt
> >>> new file mode 100644
> >>> index 0000000..699bd30
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/rtc/qcom,pm8xxx-rtc.txt
> >>> @@ -0,0 +1,29 @@
> >>> +* 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
> >>> +- reg-names: corresponding reg names for the regions listed in the 'reg'
> >>> + property, must contain:
> >>> + "rtc_base" - base of the RTC register region
> >> optional reg-names?
> >>
> >>> +- interrupts: interrupt list for the RTC, must contain a single interrupt
> >>> + specifier for the alarm interrupt
> >>> +- interrupt-names: corresponding interrupt names for the interrupts listed in
> >>> + the 'interrupts' property, must contain:
> >>> + "alarm" - summary interrupt for PMIC peripherals
> >> optional interrupt-names?
> > It isn't clear to me why these should be made optional, I hope Rob
> > provides some clarification in the sdhci-msm thread.
>
> Looks like the driver isn't using either of these properties, so I'm not
> sure why they're needed. Maybe they should just be removed.
The driver does make use of platform_get_irq_byname(pdev, "alarm"), and
I expect to make use of platform_get_resource_byname(pdev, IORESOURCE_REG, "rtc_base")
in the near future.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 6/6] documentation: bindings: document PMIC8921/8058 RTC
2014-03-06 0:00 ` Josh Cartwright
2014-03-06 1:31 ` Stephen Boyd
@ 2014-03-10 15:35 ` Rob Herring
2014-03-10 17:05 ` Josh Cartwright
1 sibling, 1 reply; 20+ messages in thread
From: Rob Herring @ 2014-03-10 15:35 UTC (permalink / raw)
To: Josh Cartwright
Cc: Stephen Boyd, Alessandro Zummo, linux-kernel@vger.kernel.org,
linux-arm-msm, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, Rob Landley, devicetree@vger.kernel.org,
linux-doc@vger.kernel.org
On Wed, Mar 5, 2014 at 6:00 PM, Josh Cartwright <joshc@codeaurora.org> wrote:
> On Wed, Mar 05, 2014 at 12:58:55PM -0800, Stephen Boyd wrote:
>> On 03/05/14 11:29, Josh Cartwright wrote:
>> > diff --git a/Documentation/devicetree/bindings/rtc/qcom,pm8xxx-rtc.txt b/Documentation/devicetree/bindings/rtc/qcom,pm8xxx-rtc.txt
>> > new file mode 100644
>> > index 0000000..699bd30
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/rtc/qcom,pm8xxx-rtc.txt
>> > @@ -0,0 +1,29 @@
>> > +* 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
>> > +- reg-names: corresponding reg names for the regions listed in the 'reg'
>> > + property, must contain:
>> > + "rtc_base" - base of the RTC register region
>>
>> optional reg-names?
>>
>> > +- interrupts: interrupt list for the RTC, must contain a single interrupt
>> > + specifier for the alarm interrupt
>> > +- interrupt-names: corresponding interrupt names for the interrupts listed in
>> > + the 'interrupts' property, must contain:
>> > + "alarm" - summary interrupt for PMIC peripherals
>>
>> optional interrupt-names?
>
> It isn't clear to me why these should be made optional, I hope Rob
> provides some clarification in the sdhci-msm thread.
Because reg and interrupt names are relatively new and reluctantly
added by DT maintainers. Personally, I think it was a mistake and it
is simply Linux specific information leaking into the DT, but it did
make transition to DT easier. The requirement is still the ordering of
reg and interrupts fields must be defined and you cannot rely on the
names to define the order. It is quite pointless here since you only
have 1 field.
Rob
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 6/6] documentation: bindings: document PMIC8921/8058 RTC
2014-03-10 15:35 ` Rob Herring
@ 2014-03-10 17:05 ` Josh Cartwright
0 siblings, 0 replies; 20+ messages in thread
From: Josh Cartwright @ 2014-03-10 17:05 UTC (permalink / raw)
To: Rob Herring
Cc: Stephen Boyd, Alessandro Zummo, linux-kernel@vger.kernel.org,
linux-arm-msm, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, Rob Landley, devicetree@vger.kernel.org,
linux-doc@vger.kernel.org
Hey Rob-
Thanks for the reply.
On Mon, Mar 10, 2014 at 10:35:25AM -0500, Rob Herring wrote:
> On Wed, Mar 5, 2014 at 6:00 PM, Josh Cartwright <joshc@codeaurora.org> wrote:
> > On Wed, Mar 05, 2014 at 12:58:55PM -0800, Stephen Boyd wrote:
> >> On 03/05/14 11:29, Josh Cartwright wrote:
> >> > +- interrupts: interrupt list for the RTC, must contain a single interrupt
> >> > + specifier for the alarm interrupt
> >> > +- interrupt-names: corresponding interrupt names for the interrupts listed in
> >> > + the 'interrupts' property, must contain:
> >> > + "alarm" - summary interrupt for PMIC peripherals
> >>
> >> optional interrupt-names?
> >
> > It isn't clear to me why these should be made optional, I hope Rob
> > provides some clarification in the sdhci-msm thread.
>
> Because reg and interrupt names are relatively new and reluctantly
> added by DT maintainers. Personally, I think it was a mistake and it
> is simply Linux specific information leaking into the DT, but it did
> make transition to DT easier.
I don't necessarily buy the Linux-specific argument in general. If a
devices' datasheet clearly gives names to register regions and
interrupts, what about reflecting these names in the bindings is
Linux-specific?
Now, there are probably abuses of this, where the reg-names and
interrupt-names are abused to ensure driver compatibility with devices
described in board files, and only in that case will I agree is
Linux-specific and should be strongly discouraged.
> The requirement is still the ordering of reg and interrupts fields
> must be defined and you cannot rely on the names to define the order.
Should this requirement also exist for other <foo>-names properties?
> It is quite pointless here since you only have 1 field.
Indeed in the interrupt case it is worthless, as there is only one alarm
interrupt. However for registers I do plan to extend this binding in
the future to document a newer RTC which does split registers across
multiple named address regions.
Thanks again,
Josh
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2014-03-10 17:08 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1394047776-13827-1-git-send-email-joshc@codeaurora.org>
2014-03-05 19:29 ` [PATCH 1/6] rtc: pm8xxx: fixup checkpatch/style issues Josh Cartwright
2014-03-05 21:02 ` Stephen Boyd
2014-03-05 21:54 ` Joe Perches
2014-03-05 23:51 ` Josh Cartwright
2014-03-05 19:29 ` [PATCH 2/6] rtc: pm8xxx: use regmap API for register accesses Josh Cartwright
2014-03-05 20:59 ` Stephen Boyd
2014-03-05 19:29 ` [PATCH 3/6] rtc: pm8xxx: use devm_request_any_context_irq Josh Cartwright
2014-03-05 20:59 ` Stephen Boyd
2014-03-05 19:29 ` [PATCH 4/6] rtc: pm8xxx: add support for devicetree Josh Cartwright
2014-03-05 20:59 ` Stephen Boyd
2014-03-06 9:18 ` Lee Jones
2014-03-05 19:29 ` [PATCH 5/6] rtc: pm8xxx: move device_init_wakeup() before rtc_register Josh Cartwright
2014-03-05 20:59 ` Stephen Boyd
2014-03-05 19:29 ` [PATCH 6/6] documentation: bindings: document PMIC8921/8058 RTC Josh Cartwright
2014-03-05 20:58 ` Stephen Boyd
2014-03-06 0:00 ` Josh Cartwright
2014-03-06 1:31 ` Stephen Boyd
2014-03-07 19:01 ` Josh Cartwright
2014-03-10 15:35 ` Rob Herring
2014-03-10 17:05 ` Josh Cartwright
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox