RTC is one submodule of Mediatek MT6397 PMIC chip[1]. This series
support RTC driver that work with Mediatek SoC like MT8135, MT8173.
It implements second counter and also provide alarm function.
This series base on 4.1-rc1, Test ok on MT8173 platform.
[1] https://lkml.org/lkml/2015/1/23/325
Change in v3:
1. Replace magic number in mt6397-core.c
2. Add comment for some equation and write trigger.
3. Use regmap_bulk_read and regmap_bulk_write to avoid muliple regmap_read
and regmap_write
4. Replace devm_request_threaded_irq with request_threaded_irq and add
irq_dispose_mapping
5. Fix Tomasz Figa review comment.
Change in v2:
1. Move RTC address and interrupt to mt6397-core.c, and register
these resource in mfd_cell.
http://lists.infradead.org/pipermail/linux-arm-kernel/2015-February/323239.html
2. Remove dt-binding document due to register resouce in mfd_cell, not from
device tree.
3. Update MAINTAINER file to add Mediatek RTC mainainter.
4. Add prefix mtk_ to some internal functions.
5. Fix racy condition
6. Check return value of regmap_read and regmap_write
7. Remove some unnecessary register readback, clear, then write.
8. Add disable alarm in mtk_rtc_set_alarm function
9. Fix Uwe Kleine-König review comment
Eddie Huang (2):
mfd: provide RTC resource in MT6397 MFD
MAINTAINERS: add Mediatek RTC driver
Tianping Fang (1):
rtc: mediatek: Add MT6397 RTC driver
MAINTAINERS | 7 +
drivers/mfd/mt6397-core.c | 18 +++
drivers/rtc/Kconfig | 10 ++
drivers/rtc/Makefile | 1 +
drivers/rtc/rtc-mt6397.c | 388 ++++++++++++++++++++++++++++++++++++++++++++++
5 files changed, 424 insertions(+)
create mode 100644 drivers/rtc/rtc-mt6397.c
--
1.8.1.1.dirty
Provide MT6397 RTC interrupt, base address, and register in
MT6397 MFD.
Signed-off-by: Eddie Huang <[email protected]>
---
drivers/mfd/mt6397-core.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/drivers/mfd/mt6397-core.c b/drivers/mfd/mt6397-core.c
index 09bc780..08cfbd1 100644
--- a/drivers/mfd/mt6397-core.c
+++ b/drivers/mfd/mt6397-core.c
@@ -21,9 +21,27 @@
#include <linux/mfd/mt6397/core.h>
#include <linux/mfd/mt6397/registers.h>
+#define MT6397_RTC_BASE 0xe000
+#define MT6397_RTC_SIZE 0x3e
+
+static const struct resource mt6397_rtc_resources[] = {
+ {
+ .start = MT6397_RTC_BASE,
+ .end = MT6397_RTC_BASE + MT6397_RTC_SIZE,
+ .flags = IORESOURCE_MEM,
+ },
+ {
+ .start = MT6397_IRQ_RTC,
+ .end = MT6397_IRQ_RTC,
+ .flags = IORESOURCE_IRQ,
+ },
+};
+
static const struct mfd_cell mt6397_devs[] = {
{
.name = "mt6397-rtc",
+ .num_resources = ARRAY_SIZE(mt6397_rtc_resources),
+ .resources = mt6397_rtc_resources,
.of_compatible = "mediatek,mt6397-rtc",
}, {
.name = "mt6397-regulator",
--
1.8.1.1.dirty
From: Tianping Fang <[email protected]>
Add Mediatek MT6397 RTC driver
Signed-off-by: Tianping Fang <[email protected]>
Signed-off-by: Eddie Huang <[email protected]>
---
drivers/rtc/Kconfig | 10 ++
drivers/rtc/Makefile | 1 +
drivers/rtc/rtc-mt6397.c | 388 +++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 399 insertions(+)
create mode 100644 drivers/rtc/rtc-mt6397.c
diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 6149ae0..9608fcb 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -1520,6 +1520,16 @@ config RTC_DRV_MOXART
This driver can also be built as a module. If so, the module
will be called rtc-moxart
+config RTC_DRV_MT6397
+ tristate "Mediatek Real Time Clock driver"
+ depends on MFD_MT6397 || COMPILE_TEST
+ help
+ This selects the Mediatek(R) RTC driver. RTC is part of Mediatek
+ MT6397 PMIC. You should enable MT6397 PMIC MFD before select
+ Mediatek(R) RTC driver.
+
+ If you want to use Mediatek(R) RTC interface, select Y or M here.
+
config RTC_DRV_XGENE
tristate "APM X-Gene RTC"
depends on HAS_IOMEM
diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
index c31731c..6ba0ce2 100644
--- a/drivers/rtc/Makefile
+++ b/drivers/rtc/Makefile
@@ -154,3 +154,4 @@ obj-$(CONFIG_RTC_DRV_X1205) += rtc-x1205.o
obj-$(CONFIG_RTC_DRV_XGENE) += rtc-xgene.o
obj-$(CONFIG_RTC_DRV_SIRFSOC) += rtc-sirfsoc.o
obj-$(CONFIG_RTC_DRV_MOXART) += rtc-moxart.o
+obj-$(CONFIG_RTC_DRV_MT6397) += rtc-mt6397.o
diff --git a/drivers/rtc/rtc-mt6397.c b/drivers/rtc/rtc-mt6397.c
new file mode 100644
index 0000000..0d96d02
--- /dev/null
+++ b/drivers/rtc/rtc-mt6397.c
@@ -0,0 +1,388 @@
+/*
+* Copyright (c) 2014-2015 MediaTek Inc.
+* Author: Tianping.Fang <[email protected]>
+*
+* This program is free software; you can redistribute it and/or modify
+* it under the terms of the GNU General Public License 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.
+*/
+
+#include <linux/delay.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/rtc.h>
+#include <linux/irqdomain.h>
+#include <linux/platform_device.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/io.h>
+#include <linux/mfd/mt6397/core.h>
+
+#define RTC_BBPU 0x0000
+#define RTC_BBPU_CBUSY BIT(6)
+
+#define RTC_WRTGR 0x003c
+
+#define RTC_IRQ_STA 0x0002
+#define RTC_IRQ_STA_AL BIT(0)
+#define RTC_IRQ_STA_LP BIT(3)
+
+#define RTC_IRQ_EN 0x0004
+#define RTC_IRQ_EN_AL BIT(0)
+#define RTC_IRQ_EN_ONESHOT BIT(2)
+#define RTC_IRQ_EN_LP BIT(3)
+#define RTC_IRQ_EN_ONESHOT_AL (RTC_IRQ_EN_ONESHOT | RTC_IRQ_EN_AL)
+
+#define RTC_AL_MASK 0x0008
+#define RTC_AL_MASK_DOW BIT(4)
+
+#define RTC_TC_SEC 0x000a
+/* Min, Hour, Dom... register offset to RTC_TC_SEC */
+#define RTC_OFFSET_SEC 0
+#define RTC_OFFSET_MIN 1
+#define RTC_OFFSET_HOUR 2
+#define RTC_OFFSET_DOM 3
+#define RTC_OFFSET_DOW 4
+#define RTC_OFFSET_MTH 5
+#define RTC_OFFSET_YEAR 6
+#define RTC_OFFSET_COUNT 7
+
+#define RTC_AL_SEC 0x0018
+
+#define RTC_PDN2 0x002e
+#define RTC_PDN2_PWRON_ALARM BIT(4)
+
+#define RTC_MIN_YEAR 1968
+#define RTC_BASE_YEAR 1900
+#define RTC_NUM_YEARS 128
+#define RTC_MIN_YEAR_OFFSET (RTC_MIN_YEAR - RTC_BASE_YEAR)
+
+struct mt6397_rtc {
+ struct device *dev;
+ struct rtc_device *rtc_dev;
+ struct mutex lock;
+ struct regmap *regmap;
+ int irq;
+ u32 addr_base;
+};
+
+static int mtk_rtc_write_trigger(struct mt6397_rtc *rtc)
+{
+ unsigned long timeout = jiffies + HZ;
+ int ret;
+ u32 data;
+
+ ret = regmap_write(rtc->regmap, rtc->addr_base + RTC_WRTGR, 1);
+ if (ret < 0)
+ return ret;
+
+ do {
+ cpu_relax();
+ ret = regmap_read(rtc->regmap, rtc->addr_base + RTC_BBPU,
+ &data);
+ if (ret < 0)
+ goto exit;
+ } while ((data & RTC_BBPU_CBUSY) && time_after(timeout, jiffies));
+
+exit:
+ return ret;
+}
+
+static irqreturn_t mtk_rtc_irq_handler_thread(int irq, void *data)
+{
+ struct mt6397_rtc *rtc = data;
+ u32 irqsta, irqen;
+ int ret;
+
+ ret = regmap_read(rtc->regmap, rtc->addr_base + RTC_IRQ_STA, &irqsta);
+ if ((ret >= 0) && (irqsta & RTC_IRQ_STA_AL)) {
+ rtc_update_irq(rtc->rtc_dev, 1, RTC_IRQF | RTC_AF);
+ irqen = irqsta & ~RTC_IRQ_EN_AL;
+ mutex_lock(&rtc->lock);
+ if (regmap_write(rtc->regmap, rtc->addr_base + RTC_IRQ_EN,
+ irqen) < 0)
+ mtk_rtc_write_trigger(rtc);
+ mutex_unlock(&rtc->lock);
+
+ return IRQ_HANDLED;
+ }
+
+ return IRQ_NONE;
+}
+
+static int __mtk_rtc_read_time(struct mt6397_rtc *rtc,
+ struct rtc_time *tm, int *sec)
+{
+ int ret;
+ u16 data[RTC_OFFSET_COUNT];
+
+ mutex_lock(&rtc->lock);
+ ret = regmap_bulk_read(rtc->regmap, rtc->addr_base + RTC_TC_SEC,
+ data, RTC_OFFSET_COUNT);
+ if (ret < 0)
+ goto exit;
+
+ tm->tm_sec = data[RTC_OFFSET_SEC];
+ tm->tm_min = data[RTC_OFFSET_MIN];
+ tm->tm_hour = data[RTC_OFFSET_HOUR];
+ tm->tm_mday = data[RTC_OFFSET_DOM];
+ tm->tm_mon = data[RTC_OFFSET_MTH];
+ tm->tm_year = data[RTC_OFFSET_YEAR];
+
+ ret = regmap_read(rtc->regmap, rtc->addr_base + RTC_TC_SEC, sec);
+exit:
+ mutex_unlock(&rtc->lock);
+ return ret;
+}
+
+static int mtk_rtc_read_time(struct device *dev, struct rtc_time *tm)
+{
+ time64_t time;
+ struct mt6397_rtc *rtc = dev_get_drvdata(dev);
+ int sec, ret;
+
+ do {
+ ret = __mtk_rtc_read_time(rtc, tm, &sec);
+ if (ret < 0)
+ goto exit;
+ } while (sec < tm->tm_sec);
+
+ /* HW register use 7 bits to store year data, minus
+ * RTC_MIN_YEAR_OFFSET before write year data to register, and plus
+ * RTC_MIN_YEAR_OFFSET back after read year from register
+ */
+ tm->tm_year += RTC_MIN_YEAR_OFFSET;
+
+ /* HW register start mon from one, but tm_mon start from zero. */
+ tm->tm_mon--;
+ time = rtc_tm_to_time64(tm);
+
+ /* rtc_tm_to_time64 covert Gregorian date to seconds since
+ * 01-01-1970 00:00:00, and this date is Thursday.
+ */
+ tm->tm_wday = (time / 86400 + 4) % 7;
+
+exit:
+ return ret;
+}
+
+static int mtk_rtc_set_time(struct device *dev, struct rtc_time *tm)
+{
+ struct mt6397_rtc *rtc = dev_get_drvdata(dev);
+ int ret;
+ u16 data[RTC_OFFSET_COUNT];
+
+ tm->tm_year -= RTC_MIN_YEAR_OFFSET;
+ tm->tm_mon++;
+
+ data[RTC_OFFSET_SEC] = tm->tm_sec;
+ data[RTC_OFFSET_MIN] = tm->tm_min;
+ data[RTC_OFFSET_HOUR] = tm->tm_hour;
+ data[RTC_OFFSET_DOM] = tm->tm_mday;
+ data[RTC_OFFSET_MTH] = tm->tm_mon;
+ data[RTC_OFFSET_YEAR] = tm->tm_year;
+
+ mutex_lock(&rtc->lock);
+ ret = regmap_bulk_write(rtc->regmap, rtc->addr_base + RTC_TC_SEC,
+ data, RTC_OFFSET_COUNT);
+ if (ret < 0)
+ goto exit;
+
+ /* Time register write to hardware after call trigger function */
+ ret = mtk_rtc_write_trigger(rtc);
+
+exit:
+ mutex_unlock(&rtc->lock);
+ return ret;
+}
+
+static int mtk_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alm)
+{
+ struct rtc_time *tm = &alm->time;
+ struct mt6397_rtc *rtc = dev_get_drvdata(dev);
+ u32 irqen, pdn2;
+ int ret;
+ u16 data[RTC_OFFSET_COUNT];
+
+ mutex_lock(&rtc->lock);
+ ret = regmap_read(rtc->regmap, rtc->addr_base + RTC_IRQ_EN, &irqen);
+ if (ret < 0)
+ goto err_exit;
+ ret = regmap_read(rtc->regmap, rtc->addr_base + RTC_PDN2, &pdn2);
+ if (ret < 0)
+ goto err_exit;
+
+ ret = regmap_bulk_read(rtc->regmap, rtc->addr_base + RTC_AL_SEC,
+ data, RTC_OFFSET_COUNT);
+ if (ret < 0)
+ goto err_exit;
+
+ alm->enabled = !!(irqen & RTC_IRQ_EN_AL);
+ alm->pending = !!(pdn2 & RTC_PDN2_PWRON_ALARM);
+ mutex_unlock(&rtc->lock);
+
+ tm->tm_sec = data[RTC_OFFSET_SEC];
+ tm->tm_min = data[RTC_OFFSET_MIN];
+ tm->tm_hour = data[RTC_OFFSET_HOUR];
+ tm->tm_mday = data[RTC_OFFSET_DOM];
+ tm->tm_mon = data[RTC_OFFSET_MTH];
+ tm->tm_year = data[RTC_OFFSET_YEAR];
+
+ tm->tm_year += RTC_MIN_YEAR_OFFSET;
+ tm->tm_mon--;
+
+ return 0;
+err_exit:
+ mutex_unlock(&rtc->lock);
+ return ret;
+}
+
+static int mtk_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alm)
+{
+ struct rtc_time *tm = &alm->time;
+ struct mt6397_rtc *rtc = dev_get_drvdata(dev);
+ int ret;
+ u16 data[RTC_OFFSET_COUNT];
+
+ tm->tm_year -= RTC_MIN_YEAR_OFFSET;
+ tm->tm_mon++;
+
+ data[RTC_OFFSET_SEC] = tm->tm_sec;
+ data[RTC_OFFSET_MIN] = tm->tm_min;
+ data[RTC_OFFSET_HOUR] = tm->tm_hour;
+ data[RTC_OFFSET_DOM] = tm->tm_mday;
+ data[RTC_OFFSET_MTH] = tm->tm_mon;
+ data[RTC_OFFSET_YEAR] = tm->tm_year;
+
+ mutex_lock(&rtc->lock);
+ if (alm->enabled) {
+ ret = regmap_bulk_write(rtc->regmap,
+ rtc->addr_base + RTC_AL_SEC,
+ data, RTC_OFFSET_COUNT);
+ if (ret < 0)
+ goto exit;
+ ret = regmap_write(rtc->regmap, rtc->addr_base + RTC_AL_MASK,
+ RTC_AL_MASK_DOW);
+ if (ret < 0)
+ goto exit;
+ ret = regmap_update_bits(rtc->regmap,
+ rtc->addr_base + RTC_IRQ_EN,
+ RTC_IRQ_EN_ONESHOT_AL, RTC_IRQ_EN_ONESHOT_AL);
+ if (ret < 0)
+ goto exit;
+ } else {
+ ret = regmap_update_bits(rtc->regmap,
+ rtc->addr_base + RTC_IRQ_EN,
+ RTC_IRQ_EN_ONESHOT_AL, 0);
+ if (ret < 0)
+ goto exit;
+ }
+
+ /* All alarm time register write to hardware after calling
+ * mtk_rtc_write_trigger. This can avoid race condition if alarm
+ * occur happen during writing alarm time register.
+ */
+ ret = mtk_rtc_write_trigger(rtc);
+exit:
+ mutex_unlock(&rtc->lock);
+ return ret;
+}
+
+static struct rtc_class_ops mtk_rtc_ops = {
+ .read_time = mtk_rtc_read_time,
+ .set_time = mtk_rtc_set_time,
+ .read_alarm = mtk_rtc_read_alarm,
+ .set_alarm = mtk_rtc_set_alarm,
+};
+
+static int mtk_rtc_probe(struct platform_device *pdev)
+{
+ struct resource *res;
+ struct mt6397_chip *mt6397_chip = dev_get_drvdata(pdev->dev.parent);
+ struct mt6397_rtc *rtc;
+ int ret;
+
+ rtc = devm_kzalloc(&pdev->dev, sizeof(struct mt6397_rtc), GFP_KERNEL);
+ if (!rtc)
+ return -ENOMEM;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ rtc->addr_base = res->start;
+
+ res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+ rtc->irq = irq_create_mapping(mt6397_chip->irq_domain, res->start);
+ if (rtc->irq <= 0)
+ return -EINVAL;
+
+ rtc->regmap = mt6397_chip->regmap;
+ rtc->dev = &pdev->dev;
+ mutex_init(&rtc->lock);
+
+ platform_set_drvdata(pdev, rtc);
+
+ ret = request_threaded_irq(rtc->irq, NULL,
+ mtk_rtc_irq_handler_thread,
+ IRQF_ONESHOT | IRQF_TRIGGER_HIGH,
+ "mt6397-rtc", rtc);
+ if (ret) {
+ dev_err(&pdev->dev, "Failed to request alarm IRQ: %d: %d\n",
+ rtc->irq, ret);
+ goto out_dispose_irq;
+ }
+
+ rtc->rtc_dev = rtc_device_register("mt6397-rtc", &pdev->dev,
+ &mtk_rtc_ops, THIS_MODULE);
+ if (IS_ERR(rtc->rtc_dev)) {
+ dev_err(&pdev->dev, "register rtc device failed\n");
+ ret = PTR_ERR(rtc->rtc_dev);
+ goto out_free_irq;
+ }
+
+ device_init_wakeup(&pdev->dev, 1);
+
+ return 0;
+
+out_free_irq:
+ free_irq(rtc->irq, rtc->rtc_dev);
+out_dispose_irq:
+ irq_dispose_mapping(rtc->irq);
+ return ret;
+}
+
+static int mtk_rtc_remove(struct platform_device *pdev)
+{
+ struct mt6397_rtc *rtc = platform_get_drvdata(pdev);
+
+ rtc_device_unregister(rtc->rtc_dev);
+ free_irq(rtc->irq, rtc->rtc_dev);
+ irq_dispose_mapping(rtc->irq);
+
+ return 0;
+}
+
+static const struct of_device_id mt6397_rtc_of_match[] = {
+ { .compatible = "mediatek,mt6397-rtc", },
+ { }
+};
+
+static struct platform_driver mtk_rtc_driver = {
+ .driver = {
+ .name = "mt6397-rtc",
+ .of_match_table = mt6397_rtc_of_match,
+ },
+ .probe = mtk_rtc_probe,
+ .remove = mtk_rtc_remove,
+};
+
+module_platform_driver(mtk_rtc_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Tianping Fang <[email protected]>");
+MODULE_DESCRIPTION("RTC Driver for MediaTek MT6397 PMIC");
+MODULE_ALIAS("platform:mt6397-rtc");
--
1.8.1.1.dirty
Add Mediatek RTC driver to maintainer entry.
Signed-off-by: Eddie Huang <[email protected]>
---
MAINTAINERS | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 2e5bbc0..eb80610 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1223,6 +1223,13 @@ W: http://www.digriz.org.uk/ts78xx/kernel
S: Maintained
F: arch/arm/mach-orion5x/ts78xx-*
+ARM/Mediatek RTC DRIVER
+M: Eddie Huang <[email protected]>
+L: [email protected] (moderated for non-subscribers)
+L: [email protected] (moderated for non-subscribers)
+S: Maintained
+F: drivers/rtc/rtc-mt*
+
ARM/Mediatek SoC support
M: Matthias Brugger <[email protected]>
L: [email protected] (moderated for non-subscribers)
--
1.8.1.1.dirty
Hello,
On Tue, Apr 28, 2015 at 03:35:54PM +0800, Eddie Huang wrote:
> Provide MT6397 RTC interrupt, base address, and register in
> MT6397 MFD.
>
> Signed-off-by: Eddie Huang <[email protected]>
> ---
> drivers/mfd/mt6397-core.c | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/drivers/mfd/mt6397-core.c b/drivers/mfd/mt6397-core.c
> index 09bc780..08cfbd1 100644
> --- a/drivers/mfd/mt6397-core.c
> +++ b/drivers/mfd/mt6397-core.c
> @@ -21,9 +21,27 @@
> #include <linux/mfd/mt6397/core.h>
> #include <linux/mfd/mt6397/registers.h>
>
> +#define MT6397_RTC_BASE 0xe000
> +#define MT6397_RTC_SIZE 0x3e
> +
> +static const struct resource mt6397_rtc_resources[] = {
> + {
> + .start = MT6397_RTC_BASE,
> + .end = MT6397_RTC_BASE + MT6397_RTC_SIZE,
> + .flags = IORESOURCE_MEM,
It's definitly a matter of taste if you align the rhs here, but if you
do, do it consitently. That is, either make sure that all equal signs
are in a single column (at least per struct resource), or use a single
space between variable name and =.
If you want to hear my personal preference: I always use a single space.
As if you have to add
.somelongvariablename = somevalue
you have to touch all other lines, too, which is ugly.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |
On Tue, 28 Apr 2015, Eddie Huang wrote:
> Provide MT6397 RTC interrupt, base address, and register in
> MT6397 MFD.
>
> Signed-off-by: Eddie Huang <[email protected]>
> ---
> drivers/mfd/mt6397-core.c | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
*after* Uwe's concerns have been attended to, you may add my:
Acked-by: Lee Jones <[email protected]>
FWIW, I quite like straight lines. :)
> diff --git a/drivers/mfd/mt6397-core.c b/drivers/mfd/mt6397-core.c
> index 09bc780..08cfbd1 100644
> --- a/drivers/mfd/mt6397-core.c
> +++ b/drivers/mfd/mt6397-core.c
> @@ -21,9 +21,27 @@
> #include <linux/mfd/mt6397/core.h>
> #include <linux/mfd/mt6397/registers.h>
>
> +#define MT6397_RTC_BASE 0xe000
> +#define MT6397_RTC_SIZE 0x3e
> +
> +static const struct resource mt6397_rtc_resources[] = {
> + {
> + .start = MT6397_RTC_BASE,
> + .end = MT6397_RTC_BASE + MT6397_RTC_SIZE,
> + .flags = IORESOURCE_MEM,
> + },
> + {
> + .start = MT6397_IRQ_RTC,
> + .end = MT6397_IRQ_RTC,
> + .flags = IORESOURCE_IRQ,
> + },
> +};
> +
> static const struct mfd_cell mt6397_devs[] = {
> {
> .name = "mt6397-rtc",
> + .num_resources = ARRAY_SIZE(mt6397_rtc_resources),
> + .resources = mt6397_rtc_resources,
> .of_compatible = "mediatek,mt6397-rtc",
> }, {
> .name = "mt6397-regulator",
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
Hi,
This looks mostly good. Could you align the wrapped function parameters
to the open parenthesis (use checkpatch --strict)?
On 28/04/2015 at 15:35:55 +0800, Eddie Huang wrote :
> +static int mtk_rtc_write_trigger(struct mt6397_rtc *rtc)
> +{
> + unsigned long timeout = jiffies + HZ;
> + int ret;
> + u32 data;
> +
> + ret = regmap_write(rtc->regmap, rtc->addr_base + RTC_WRTGR, 1);
> + if (ret < 0)
> + return ret;
> +
> + do {
> + cpu_relax();
> + ret = regmap_read(rtc->regmap, rtc->addr_base + RTC_BBPU,
> + &data);
> + if (ret < 0)
> + goto exit;
> + } while ((data & RTC_BBPU_CBUSY) && time_after(timeout, jiffies));
> +
Shouldn't you return -ETIMEDOUT if the loop breaks because of time_after?
Thanks,
--
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
Hi,
On 28/04/2015 at 15:35:56 +0800, Eddie Huang wrote :
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 2e5bbc0..eb80610 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1223,6 +1223,13 @@ W: http://www.digriz.org.uk/ts78xx/kernel
> S: Maintained
> F: arch/arm/mach-orion5x/ts78xx-*
>
> +ARM/Mediatek RTC DRIVER
> +M: Eddie Huang <[email protected]>
> +L: [email protected] (moderated for non-subscribers)
> +L: [email protected] (moderated for non-subscribers)
> +S: Maintained
> +F: drivers/rtc/rtc-mt*
I feel like this is a bit broad. Unless you want to really maintain
rtc-mt* which I guess may not be only Mediatek in the future, I would
use the full filename here. You will probably remember to update it if
you add more, that may not be the case for other developers.
--
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
On Tue, 2015-05-05 at 22:00 +0200, Alexandre Belloni wrote:
> Hi,
>
> This looks mostly good. Could you align the wrapped function parameters
> to the open parenthesis (use checkpatch --strict)?
>
> On 28/04/2015 at 15:35:55 +0800, Eddie Huang wrote :
> > +static int mtk_rtc_write_trigger(struct mt6397_rtc *rtc)
> > +{
> > + unsigned long timeout = jiffies + HZ;
> > + int ret;
> > + u32 data;
> > +
> > + ret = regmap_write(rtc->regmap, rtc->addr_base + RTC_WRTGR, 1);
> > + if (ret < 0)
> > + return ret;
> > +
> > + do {
> > + cpu_relax();
> > + ret = regmap_read(rtc->regmap, rtc->addr_base + RTC_BBPU,
> > + &data);
> > + if (ret < 0)
> > + goto exit;
> > + } while ((data & RTC_BBPU_CBUSY) && time_after(timeout, jiffies));
> > +
>
> Shouldn't you return -ETIMEDOUT if the loop breaks because of time_after?
Probably yes.
I believe as written the time_after test is too much
for my little brain. I would have used time_before
and reversed the args.
I suggest moving the time_after() test into the loop,
use break; and remove the exit label too.
Maybe something like:
while (1) {
ret = regmap_read(rtc->regmap, rtc->addr_base + RTC_BBPU,
&data);
if (ret < 0)
break;
if (!(data & RTC_BBPU_CBUSY))
break;
if (time_after(jiffies, timeout)) {
ret = -ETIMEDOUT;
break;
}
cpu_relax();
}
return ret;
}
On 05/05/2015 at 13:44:21 -0700, Joe Perches wrote :
> I suggest moving the time_after() test into the loop,
> use break; and remove the exit label too.
>
> Maybe something like:
>
> while (1) {
> ret = regmap_read(rtc->regmap, rtc->addr_base + RTC_BBPU,
> &data);
> if (ret < 0)
> break;
> if (!(data & RTC_BBPU_CBUSY))
> break;
> if (time_after(jiffies, timeout)) {
> ret = -ETIMEDOUT;
> break;
> }
> cpu_relax();
> }
>
> return ret;
That certainly looks more readable.
--
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
Hi Joe and Alexandre,
On Tue, 2015-05-05 at 23:01 +0200, Alexandre Belloni wrote:
> On 05/05/2015 at 13:44:21 -0700, Joe Perches wrote :
> > I suggest moving the time_after() test into the loop,
> > use break; and remove the exit label too.
> >
> > Maybe something like:
> >
> > while (1) {
> > ret = regmap_read(rtc->regmap, rtc->addr_base + RTC_BBPU,
> > &data);
> > if (ret < 0)
> > break;
> > if (!(data & RTC_BBPU_CBUSY))
> > break;
> > if (time_after(jiffies, timeout)) {
> > ret = -ETIMEDOUT;
> > break;
> > }
> > cpu_relax();
> > }
> >
> > return ret;
>
> That certainly looks more readable.
>
Thanks correct me that I put wrong paramters in time_after, and give me
good example. I will adopt your suggestion in next round.
Eddie
Hi Alexandre,
On Tue, 2015-05-05 at 22:03 +0200, Alexandre Belloni wrote:
> Hi,
>
> On 28/04/2015 at 15:35:56 +0800, Eddie Huang wrote :
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 2e5bbc0..eb80610 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -1223,6 +1223,13 @@ W: http://www.digriz.org.uk/ts78xx/kernel
> > S: Maintained
> > F: arch/arm/mach-orion5x/ts78xx-*
> >
> > +ARM/Mediatek RTC DRIVER
> > +M: Eddie Huang <[email protected]>
> > +L: [email protected] (moderated for non-subscribers)
> > +L: [email protected] (moderated for non-subscribers)
> > +S: Maintained
> > +F: drivers/rtc/rtc-mt*
>
> I feel like this is a bit broad. Unless you want to really maintain
> rtc-mt* which I guess may not be only Mediatek in the future, I would
> use the full filename here. You will probably remember to update it if
> you add more, that may not be the case for other developers.
>
Yes, you are right, the rtc-mt* is too broad, use full filename is a
better choice right now.
Eddie