From: Sean Wang <[email protected]>
Hi,
The series keeps to extend the capability of BPI-R2 board with MT7623 and
the result can as well bring benefits into the other MediaTek PMICs such
as MT6397 or SoCs.
The series sent across mfd, rtc, pm, dt-binding sub-system is for hoping
to let people have a simple cross-reference to know the exact dependency
and why those patches are being split in that way between each one.
Patch 1-3: Add dt-binding to the related devices newly or already
supported.
Patch 4-5: Extend driver with the functionality of MT6323 RTC device.
Patch 6-9 and 12-13: Add a few of trivial fixups, cleanups and
improvements.
Patch 10-11: It's a preparation for Patch 14 adding a new driver.
Patch 14: Add a new driver for a power-off driver.
Patch 15-16: Update MAINTAINERS with these new files being added.
Sean
Sean Wang (16):
dt-bindings: power: reset: mediatek: add bindings for power device
dt-bindings: rtc: mediatek: add bindings for PMIC RTC
dt-bindings: mfd: mediatek: add a description for MT6323 RTC
mfd: mt6397: add MT6323 RTC support into MT6397 driver
rtc: mediatek: add MT6323 support to RTC driver
rtc: mediatek: remove unnecessary parentheses
rtc: mediatek: replace a poll with regmap_read_poll_timeout
rtc: mediatek: remove unnecessary irq_dispose_mapping
rtc: mediatek: convert to use device managed functions
rtc: mediatek: add devm_of_platform_populate
rtc: mediatek: move the declaration into a globally visible header
file
rtc: mediatek: cleanup header files to include
rtc: mediatek: update license converting to using SPDX identifiers
power: reset: mediatek: add a power-off driver using PMIC RTC device
MAINTAINERS: update entry for ARM/Mediatek RTC DRIVER
MAINTAINERS: add an entry for MediaTek board level shutdown driver
Documentation/devicetree/bindings/mfd/mt6397.txt | 4 +-
.../bindings/power/reset/mt6397-rtc-poweroff.txt | 24 ++++
.../devicetree/bindings/rtc/rtc-mt6397.txt | 39 ++++++
MAINTAINERS | 9 ++
drivers/mfd/mt6397-core.c | 23 +++-
drivers/power/reset/Kconfig | 9 ++
drivers/power/reset/Makefile | 1 +
drivers/power/reset/mt6397-rtc-poweroff.c | 100 ++++++++++++++
drivers/rtc/rtc-mt6397.c | 145 +++++----------------
include/linux/rtc/mt6397.h | 73 +++++++++++
10 files changed, 314 insertions(+), 113 deletions(-)
create mode 100644 Documentation/devicetree/bindings/power/reset/mt6397-rtc-poweroff.txt
create mode 100644 Documentation/devicetree/bindings/rtc/rtc-mt6397.txt
create mode 100644 drivers/power/reset/mt6397-rtc-poweroff.c
create mode 100644 include/linux/rtc/mt6397.h
--
2.7.4
From: Sean Wang <[email protected]>
Clean up those included header files by removing unreferenced ones,
add missing ones the driver explicitly depends on and finally list
all #includes in alphabetical order.
Signed-off-by: Sean Wang <[email protected]>
---
drivers/rtc/rtc-mt6397.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/rtc/rtc-mt6397.c b/drivers/rtc/rtc-mt6397.c
index 015609d..f5dc70d 100644
--- a/drivers/rtc/rtc-mt6397.c
+++ b/drivers/rtc/rtc-mt6397.c
@@ -12,18 +12,15 @@
* GNU General Public License for more details.
*/
-#include <linux/delay.h>
-#include <linux/init.h>
+#include <linux/err.h>
#include <linux/interrupt.h>
+#include <linux/mfd/mt6397/core.h>
#include <linux/module.h>
+#include <linux/mutex.h>
#include <linux/of_platform.h>
+#include <linux/platform_device.h>
#include <linux/regmap.h>
#include <linux/rtc.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>
#include <linux/rtc/mt6397.h>
static int mtk_rtc_write_trigger(struct mt6397_rtc *rtc)
--
2.7.4
From: Sean Wang <[email protected]>
Add device-tree binding for the power device responsible for shutdown a
remote SoC, which is a tiny circuit block present on MediaTek PMIC based
RTC.
Signed-off-by: Sean Wang <[email protected]>
---
.../bindings/power/reset/mt6397-rtc-poweroff.txt | 24 ++++++++++++++++++++++
1 file changed, 24 insertions(+)
create mode 100644 Documentation/devicetree/bindings/power/reset/mt6397-rtc-poweroff.txt
diff --git a/Documentation/devicetree/bindings/power/reset/mt6397-rtc-poweroff.txt b/Documentation/devicetree/bindings/power/reset/mt6397-rtc-poweroff.txt
new file mode 100644
index 0000000..75a9d7d
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/reset/mt6397-rtc-poweroff.txt
@@ -0,0 +1,24 @@
+Device-Tree bindings for Power Device on MediaTek PMIC RTC
+
+The power device is responsible for externally power off or on the remote
+MediaTek SoC through the a tiny circuit block BBPU inside PMIC RTC.
+
+Required parent node:
+- rtc
+ For MediaTek PMIC RTC bindings, see:
+ Documentation/devicetree/bindings/mfd/mt6397.txt
+
+Required properties:
+- compatible: Should be one of follows
+ "mediatek,mt6323-rtc-poweroff": for MT6323 PMIC
+ "mediatek,mt6397-rtc-poweroff": for MT6397 PMIC
+
+Example:
+
+ rtc {
+ compatible = "mediatek,mt6323-rtc";
+
+ power-off {
+ compatible = "mediatek,mt6323-rtc-poweroff";
+ };
+ };
--
2.7.4
From: Sean Wang <[email protected]>
The power device is responsible for externally down or up the power
of the remote MediaTek SoC through the tiny circuit BBPU inside PMIC RTC.
Though it's a part of RTC device, it would be better to be a standalone
driver against existent RTC driver so as to make concentration on works
about power-controlling topic and help gather more improvements while the
subsystem's constantly growing.
Currently, the most basic functionality supported is to just power off the
system by writing to a special bit field in BBPU register after the system
has reached pm_poweroff.
Signed-off-by: Sean Wang <[email protected]>
---
drivers/power/reset/Kconfig | 9 +++
drivers/power/reset/Makefile | 1 +
drivers/power/reset/mt6397-rtc-poweroff.c | 100 ++++++++++++++++++++++++++++++
include/linux/rtc/mt6397.h | 1 +
4 files changed, 111 insertions(+)
create mode 100644 drivers/power/reset/mt6397-rtc-poweroff.c
diff --git a/drivers/power/reset/Kconfig b/drivers/power/reset/Kconfig
index a102e74..0bd4603 100644
--- a/drivers/power/reset/Kconfig
+++ b/drivers/power/reset/Kconfig
@@ -121,6 +121,15 @@ config POWER_RESET_LTC2952
This driver supports an external powerdown trigger and board power
down via the LTC2952. Bindings are made in the device tree.
+config POWER_RESET_MT6397_RTC
+ bool "MediaTek MT6397 RTC power-off driver"
+ help
+ This driver supports turning off a remote MediaTek SoC by
+ controlling BBPU on MT6397 or MT6323 RTC.
+
+ Select this if you're building a kernel with your MediaTek SoC
+ with an equipment with MT6397 or MT6323 PMIC.
+
config POWER_RESET_QNAP
bool "QNAP power-off driver"
depends on OF_GPIO && PLAT_ORION
diff --git a/drivers/power/reset/Makefile b/drivers/power/reset/Makefile
index dcc92f5..d45099e 100644
--- a/drivers/power/reset/Makefile
+++ b/drivers/power/reset/Makefile
@@ -11,6 +11,7 @@ obj-$(CONFIG_POWER_RESET_GPIO) += gpio-poweroff.o
obj-$(CONFIG_POWER_RESET_GPIO_RESTART) += gpio-restart.o
obj-$(CONFIG_POWER_RESET_HISI) += hisi-reboot.o
obj-$(CONFIG_POWER_RESET_MSM) += msm-poweroff.o
+obj-$(CONFIG_POWER_RESET_MT6397_RTC) += mt6397-rtc-poweroff.o
obj-$(CONFIG_POWER_RESET_PIIX4_POWEROFF) += piix4-poweroff.o
obj-$(CONFIG_POWER_RESET_LTC2952) += ltc2952-poweroff.o
obj-$(CONFIG_POWER_RESET_QNAP) += qnap-poweroff.o
diff --git a/drivers/power/reset/mt6397-rtc-poweroff.c b/drivers/power/reset/mt6397-rtc-poweroff.c
new file mode 100644
index 0000000..9b57366
--- /dev/null
+++ b/drivers/power/reset/mt6397-rtc-poweroff.c
@@ -0,0 +1,100 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Power-off using MediaTek PMIC RTC device
+ *
+ * Copyright (C) 2018 MediaTek Inc.
+ *
+ * Author: Sean Wang <[email protected]>
+ *
+ */
+
+#include <linux/err.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/rtc/mt6397.h>
+
+struct mt6397_rtc_powercon {
+ struct device *dev;
+ struct mt6397_rtc *rtc;
+};
+
+static struct mt6397_rtc_powercon *mt_powercon;
+
+static void mt6397_rtc_do_poweroff(void)
+{
+ struct mt6397_rtc_powercon *powercon = mt_powercon;
+ struct mt6397_rtc *rtc = powercon->rtc;
+ unsigned int val;
+ int ret;
+
+ regmap_write(rtc->regmap, rtc->addr_base + RTC_BBPU, RTC_BBPU_KEY);
+ regmap_write(rtc->regmap, rtc->addr_base + RTC_WRTGR, 1);
+
+ ret = regmap_read_poll_timeout(rtc->regmap,
+ rtc->addr_base + RTC_BBPU, val,
+ !(val & RTC_BBPU_CBUSY),
+ MTK_RTC_POLL_DELAY_US,
+ MTK_RTC_POLL_TIMEOUT);
+ if (ret)
+ dev_err(powercon->dev, "failed to write BBPU: %d\n", ret);
+
+ /* Wait some time until system down, otherwise, notice with a warn */
+ mdelay(1000);
+
+ WARN_ONCE(1, "Unable to poweroff system\n");
+}
+
+static int mt6397_rtc_poweroff_probe(struct platform_device *pdev)
+{
+ struct mt6397_rtc *rtc = dev_get_drvdata(pdev->dev.parent);
+ struct mt6397_rtc_powercon *powercon;
+
+ if (!rtc) {
+ dev_err(&pdev->dev, "Can't find RTC as the parent\n");
+ return -ENODEV;
+ }
+
+ powercon = devm_kzalloc(&pdev->dev, sizeof(*powercon), GFP_KERNEL);
+ if (!powercon)
+ return -ENOMEM;
+
+ powercon->dev = &pdev->dev;
+ powercon->rtc = rtc;
+ mt_powercon = powercon;
+
+ pm_power_off = &mt6397_rtc_do_poweroff;
+
+ return 0;
+}
+
+static int mt6397_rtc_poweroff_remove(struct platform_device *pdev)
+{
+ if (pm_power_off == &mt6397_rtc_do_poweroff)
+ pm_power_off = NULL;
+
+ return 0;
+}
+
+static const struct of_device_id mt6397_rtc_poweroff_dt_match[] = {
+ { .compatible = "mediatek,mt6323-rtc-poweroff" },
+ { .compatible = "mediatek,mt6397-rtc-poweroff" },
+ {},
+};
+MODULE_DEVICE_TABLE(of, mt6397_rtc_poweroff_dt_match);
+
+static struct platform_driver mt6397_rtc_poweroff_driver = {
+ .probe = mt6397_rtc_poweroff_probe,
+ .remove = mt6397_rtc_poweroff_remove,
+ .driver = {
+ .name = "mt6397-rtc-poweroff",
+ .of_match_table = mt6397_rtc_poweroff_dt_match,
+ },
+};
+
+module_platform_driver(mt6397_rtc_poweroff_driver);
+
+MODULE_DESCRIPTION("Poweroff driver using MediaTek PMIC RTC");
+MODULE_AUTHOR("Sean Wang <[email protected]>");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:mt6397-rtc-poweroff");
diff --git a/include/linux/rtc/mt6397.h b/include/linux/rtc/mt6397.h
index 4b19f51..e618974 100644
--- a/include/linux/rtc/mt6397.h
+++ b/include/linux/rtc/mt6397.h
@@ -17,6 +17,7 @@
#define RTC_BBPU 0x0000
#define RTC_BBPU_CBUSY BIT(6)
+#define RTC_BBPU_KEY (0x43 << 8)
#define RTC_WRTGR 0x003c
--
2.7.4
From: Sean Wang <[email protected]>
Add devm_of_platform_populate to populate all child nodes according to
the dt-binding.
Signed-off-by: Sean Wang <[email protected]>
---
drivers/rtc/rtc-mt6397.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/rtc/rtc-mt6397.c b/drivers/rtc/rtc-mt6397.c
index bfc5d6f..d133d1f 100644
--- a/drivers/rtc/rtc-mt6397.c
+++ b/drivers/rtc/rtc-mt6397.c
@@ -16,6 +16,7 @@
#include <linux/init.h>
#include <linux/interrupt.h>
#include <linux/module.h>
+#include <linux/of_platform.h>
#include <linux/regmap.h>
#include <linux/rtc.h>
#include <linux/jiffies.h>
@@ -349,7 +350,7 @@ static int mtk_rtc_probe(struct platform_device *pdev)
return ret;
}
- return 0;
+ return devm_of_platform_populate(&pdev->dev);
}
#ifdef CONFIG_PM_SLEEP
--
2.7.4
From: Sean Wang <[email protected]>
Use device managed operation to simplify error handling, reduce source
code size, and reduce the likelyhood of bugs, and remove our removal
callback which contains anything already done by device managed functions.
Signed-off-by: Sean Wang <[email protected]>
---
drivers/rtc/rtc-mt6397.c | 31 ++++++++-----------------------
1 file changed, 8 insertions(+), 23 deletions(-)
diff --git a/drivers/rtc/rtc-mt6397.c b/drivers/rtc/rtc-mt6397.c
index cefb83b..bfc5d6f 100644
--- a/drivers/rtc/rtc-mt6397.c
+++ b/drivers/rtc/rtc-mt6397.c
@@ -14,6 +14,7 @@
#include <linux/delay.h>
#include <linux/init.h>
+#include <linux/interrupt.h>
#include <linux/module.h>
#include <linux/regmap.h>
#include <linux/rtc.h>
@@ -328,10 +329,10 @@ static int mtk_rtc_probe(struct platform_device *pdev)
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);
+ ret = devm_request_threaded_irq(&pdev->dev, 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);
@@ -340,30 +341,15 @@ static int mtk_rtc_probe(struct platform_device *pdev)
device_init_wakeup(&pdev->dev, 1);
- rtc->rtc_dev = rtc_device_register("mt6397-rtc", &pdev->dev,
- &mtk_rtc_ops, THIS_MODULE);
+ rtc->rtc_dev = devm_rtc_device_register(&pdev->dev, "mt6397-rtc",
+ &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;
+ return ret;
}
return 0;
-
-out_free_irq:
- free_irq(rtc->irq, rtc->rtc_dev);
-
- 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);
-
- return 0;
}
#ifdef CONFIG_PM_SLEEP
@@ -405,7 +391,6 @@ static struct platform_driver mtk_rtc_driver = {
.pm = &mt6397_pm_ops,
},
.probe = mtk_rtc_probe,
- .remove = mtk_rtc_remove,
};
module_platform_driver(mtk_rtc_driver);
--
2.7.4
From: Sean Wang <[email protected]>
This is in preparation for allowing other drivers can share the
declaration, so move the declaration into a globally visible header file.
Signed-off-by: Sean Wang <[email protected]>
---
drivers/rtc/rtc-mt6397.c | 53 +---------------------------------
include/linux/rtc/mt6397.h | 72 ++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 73 insertions(+), 52 deletions(-)
create mode 100644 include/linux/rtc/mt6397.h
diff --git a/drivers/rtc/rtc-mt6397.c b/drivers/rtc/rtc-mt6397.c
index d133d1f..015609d 100644
--- a/drivers/rtc/rtc-mt6397.c
+++ b/drivers/rtc/rtc-mt6397.c
@@ -19,63 +19,12 @@
#include <linux/of_platform.h>
#include <linux/regmap.h>
#include <linux/rtc.h>
-#include <linux/jiffies.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)
-
-#define MTK_RTC_POLL_DELAY_US 10
-#define MTK_RTC_POLL_TIMEOUT (jiffies_to_usecs(HZ))
-
-struct mt6397_rtc {
- struct device *dev;
- struct rtc_device *rtc_dev;
- struct mutex lock;
- struct regmap *regmap;
- int irq;
- u32 addr_base;
-};
+#include <linux/rtc/mt6397.h>
static int mtk_rtc_write_trigger(struct mt6397_rtc *rtc)
{
diff --git a/include/linux/rtc/mt6397.h b/include/linux/rtc/mt6397.h
new file mode 100644
index 0000000..4b19f51
--- /dev/null
+++ b/include/linux/rtc/mt6397.h
@@ -0,0 +1,72 @@
+
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2014-2018 MediaTek Inc.
+ *
+ * Author: Tianping.Fang <[email protected]>
+ * Sean Wang <[email protected]>
+ */
+
+#ifndef _LINUX_RTC_MT6397_H_
+#define _LINUX_RTC_MT6397_H_
+
+#include <linux/jiffies.h>
+#include <linux/mutex.h>
+#include <linux/regmap.h>
+#include <linux/rtc.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)
+
+#define MTK_RTC_POLL_DELAY_US 10
+#define MTK_RTC_POLL_TIMEOUT (jiffies_to_usecs(HZ))
+
+struct mt6397_rtc {
+ struct device *dev;
+ struct rtc_device *rtc_dev;
+
+ /* protect registers accessing */
+ struct mutex lock;
+ struct regmap *regmap;
+ int irq;
+ u32 addr_base;
+};
+
+#endif /* _LINUX_RTC_MT6397_H_ */
+
--
2.7.4
From: Sean Wang <[email protected]>
Add an entry for the MediaTek board level shutdown driver.
Signed-off-by: Sean Wang <[email protected]>
---
MAINTAINERS | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 1c13f29..863d1e3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8818,6 +8818,13 @@ S: Maintained
F: drivers/net/dsa/mt7530.*
F: net/dsa/tag_mtk.c
+MEDIATEK BOARD LEVEL SHUTDOWN DRIVERS
+M: Sean Wang <[email protected]>
+L: [email protected]
+S: Maintained
+F: Documentation/devicetree/bindings/power/reset/mt6397-rtc-poweroff.txt
+F: drivers/power/reset/mt6397-rtc-poweroff.c
+
MEDIATEK JPEG DRIVER
M: Rick Chang <[email protected]>
M: Bin Liu <[email protected]>
--
2.7.4
From: Sean Wang <[email protected]>
Remove unnecessary parentheses due to explicit C operator precedence.
Signed-off-by: Sean Wang <[email protected]>
---
drivers/rtc/rtc-mt6397.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/rtc/rtc-mt6397.c b/drivers/rtc/rtc-mt6397.c
index 0df7ccd..4411c08 100644
--- a/drivers/rtc/rtc-mt6397.c
+++ b/drivers/rtc/rtc-mt6397.c
@@ -106,7 +106,7 @@ static irqreturn_t mtk_rtc_irq_handler_thread(int irq, void *data)
int ret;
ret = regmap_read(rtc->regmap, rtc->addr_base + RTC_IRQ_STA, &irqsta);
- if ((ret >= 0) && (irqsta & RTC_IRQ_STA_AL)) {
+ 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);
--
2.7.4
From: Sean Wang <[email protected]>
Add a description for MT6323 RTC and link it to the detailed binding
documentation.
Signed-off-by: Sean Wang <[email protected]>
---
Documentation/devicetree/bindings/mfd/mt6397.txt | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/mfd/mt6397.txt b/Documentation/devicetree/bindings/mfd/mt6397.txt
index d1df77f..11a748d 100644
--- a/Documentation/devicetree/bindings/mfd/mt6397.txt
+++ b/Documentation/devicetree/bindings/mfd/mt6397.txt
@@ -22,8 +22,10 @@ compatible: "mediatek,mt6397" or "mediatek,mt6323"
Optional subnodes:
- rtc
- Required properties:
+ Required properties: Should be one of follows
+ - compatible: "mediatek,mt6323-rtc"
- compatible: "mediatek,mt6397-rtc"
+ For details, see Documentation/devicetree/bindings/rtc/rtc-mt6397.txt
- regulators
Required properties:
- compatible: "mediatek,mt6397-regulator"
--
2.7.4
From: Sean Wang <[email protected]>
Reuse the helper regmap_read_poll_timeout instead to simpify the logic.
Furthermore, the time for a wait in each iteration changed from cpu_relax
to 20us is for matching the usage of the helper, but it wouldn't acctually
break any the existent functionality according to a good test on MT6323
PMIC.
Signed-off-by: Sean Wang <[email protected]>
---
drivers/rtc/rtc-mt6397.c | 25 +++++++++++--------------
1 file changed, 11 insertions(+), 14 deletions(-)
diff --git a/drivers/rtc/rtc-mt6397.c b/drivers/rtc/rtc-mt6397.c
index 4411c08..b62eaa8 100644
--- a/drivers/rtc/rtc-mt6397.c
+++ b/drivers/rtc/rtc-mt6397.c
@@ -18,6 +18,7 @@
#include <linux/regmap.h>
#include <linux/rtc.h>
#include <linux/irqdomain.h>
+#include <linux/jiffies.h>
#include <linux/platform_device.h>
#include <linux/of_address.h>
#include <linux/of_irq.h>
@@ -63,6 +64,9 @@
#define RTC_NUM_YEARS 128
#define RTC_MIN_YEAR_OFFSET (RTC_MIN_YEAR - RTC_BASE_YEAR)
+#define MTK_RTC_POLL_DELAY_US 10
+#define MTK_RTC_POLL_TIMEOUT (jiffies_to_usecs(HZ))
+
struct mt6397_rtc {
struct device *dev;
struct rtc_device *rtc_dev;
@@ -74,7 +78,6 @@ struct mt6397_rtc {
static int mtk_rtc_write_trigger(struct mt6397_rtc *rtc)
{
- unsigned long timeout = jiffies + HZ;
int ret;
u32 data;
@@ -82,19 +85,13 @@ static int mtk_rtc_write_trigger(struct mt6397_rtc *rtc)
if (ret < 0)
return ret;
- 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();
- }
+ ret = regmap_read_poll_timeout(rtc->regmap,
+ rtc->addr_base + RTC_BBPU, data,
+ !(data & RTC_BBPU_CBUSY),
+ MTK_RTC_POLL_DELAY_US,
+ MTK_RTC_POLL_TIMEOUT);
+ if (ret)
+ dev_err(rtc->dev, "failed to write WRTGE: %d\n", ret);
return ret;
}
--
2.7.4
From: Sean Wang <[email protected]>
Add new files for new dt-bindings and a globally visible header file.
Signed-off-by: Sean Wang <[email protected]>
---
MAINTAINERS | 2 ++
1 file changed, 2 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index d3c33d7..1c13f29 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1626,9 +1626,11 @@ M: Sean Wang <[email protected]>
L: [email protected] (moderated for non-subscribers)
L: [email protected] (moderated for non-subscribers)
S: Maintained
+F: Documentation/devicetree/bindings/rtc/rtc-mt6397.txt
F: Documentation/devicetree/bindings/rtc/rtc-mt7622.txt
F: drivers/rtc/rtc-mt6397.c
F: drivers/rtc/rtc-mt7622.c
+F: include/linux/rtc/mt6397.h
ARM/Mediatek SoC support
M: Matthias Brugger <[email protected]>
--
2.7.4
From: Sean Wang <[email protected]>
Update the license and as well convert to using SPDX identifiers.
Signed-off-by: Sean Wang <[email protected]>
---
drivers/rtc/rtc-mt6397.c | 20 ++++++++------------
1 file changed, 8 insertions(+), 12 deletions(-)
diff --git a/drivers/rtc/rtc-mt6397.c b/drivers/rtc/rtc-mt6397.c
index f5dc70d..ad336a4 100644
--- a/drivers/rtc/rtc-mt6397.c
+++ b/drivers/rtc/rtc-mt6397.c
@@ -1,16 +1,12 @@
+// SPDX-License-Identifier: GPL-2.0
/*
-* 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.
-*/
+ * MediaTek PMIC RTC driver
+ *
+ * Copyright (C) 2014-2018 MediaTek Inc.
+ *
+ * Author: Tianping.Fang <[email protected]>
+ * Sean Wang <[email protected]>
+ */
#include <linux/err.h>
#include <linux/interrupt.h>
--
2.7.4
From: Sean Wang <[email protected]>
Just to add MT6323 support to the existent RTC driver.
Signed-off-by: Sean Wang <[email protected]>
---
drivers/rtc/rtc-mt6397.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/rtc/rtc-mt6397.c b/drivers/rtc/rtc-mt6397.c
index 385f830..0df7ccd 100644
--- a/drivers/rtc/rtc-mt6397.c
+++ b/drivers/rtc/rtc-mt6397.c
@@ -398,6 +398,7 @@ static SIMPLE_DEV_PM_OPS(mt6397_pm_ops, mt6397_rtc_suspend,
mt6397_rtc_resume);
static const struct of_device_id mt6397_rtc_of_match[] = {
+ { .compatible = "mediatek,mt6323-rtc", },
{ .compatible = "mediatek,mt6397-rtc", },
{ }
};
--
2.7.4
From: Sean Wang <[email protected]>
Add compatible string as "mt6323-rtc" that will make the OF core spawn
child devices for the RTC subnode of that MT6323 MFD node.
Signed-off-by: Sean Wang <[email protected]>
---
drivers/mfd/mt6397-core.c | 23 ++++++++++++++++++++++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/drivers/mfd/mt6397-core.c b/drivers/mfd/mt6397-core.c
index 77b64bd..f71874a 100644
--- a/drivers/mfd/mt6397-core.c
+++ b/drivers/mfd/mt6397-core.c
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2014 MediaTek Inc.
+ * Copyright (c) 2014-2018 MediaTek Inc.
* Author: Flora Fu, MediaTek
*
* This program is free software; you can redistribute it and/or modify
@@ -23,6 +23,9 @@
#include <linux/mfd/mt6397/registers.h>
#include <linux/mfd/mt6323/registers.h>
+#define MT6323_RTC_BASE 0x8000
+#define MT6323_RTC_SIZE 0x3e
+
#define MT6397_RTC_BASE 0xe000
#define MT6397_RTC_SIZE 0x3e
@@ -30,6 +33,19 @@
#define MT6391_CID_CODE 0x91
#define MT6397_CID_CODE 0x97
+static const struct resource mt6323_rtc_resources[] = {
+ {
+ .start = MT6323_RTC_BASE,
+ .end = MT6323_RTC_BASE + MT6323_RTC_SIZE,
+ .flags = IORESOURCE_MEM,
+ },
+ {
+ .start = MT6323_IRQ_STATUS_RTC,
+ .end = MT6323_IRQ_STATUS_RTC,
+ .flags = IORESOURCE_IRQ,
+ },
+};
+
static const struct resource mt6397_rtc_resources[] = {
{
.start = MT6397_RTC_BASE,
@@ -55,6 +71,11 @@ static const struct resource mt6397_keys_resources[] = {
static const struct mfd_cell mt6323_devs[] = {
{
+ .name = "mt6323-rtc",
+ .num_resources = ARRAY_SIZE(mt6323_rtc_resources),
+ .resources = mt6323_rtc_resources,
+ .of_compatible = "mediatek,mt6323-rtc",
+ }, {
.name = "mt6323-regulator",
.of_compatible = "mediatek,mt6323-regulator"
}, {
--
2.7.4
From: Sean Wang <[email protected]>
Add device-tree binding for MediaTek PMIC based RTC.
Signed-off-by: Sean Wang <[email protected]>
---
.../devicetree/bindings/rtc/rtc-mt6397.txt | 39 ++++++++++++++++++++++
1 file changed, 39 insertions(+)
create mode 100644 Documentation/devicetree/bindings/rtc/rtc-mt6397.txt
diff --git a/Documentation/devicetree/bindings/rtc/rtc-mt6397.txt b/Documentation/devicetree/bindings/rtc/rtc-mt6397.txt
new file mode 100644
index 0000000..83ff6be
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/rtc-mt6397.txt
@@ -0,0 +1,39 @@
+Device-Tree bindings for MediaTek PMIC based RTC
+
+MediaTek PMIC based RTC is an independent function of MediaTek PMIC which
+is working as a multi-function device (MFD). And the RTC can be configured and
+set up via PMIC wrapper bus. Which is also common resource shared among the
+other functions present on the PMIC.
+
+For MediaTek PMIC wrapper bus bindings, see:
+Documentation/devicetree/bindings/soc/mediatek/pwrap.txt
+
+Required parent node:
+- pmic
+ For MediaTek PMIC MFD bindings, see:
+ Documentation/devicetree/bindings/mfd/mt6397.txt
+
+Required properties:
+- compatible: Should be one of follows
+ "mediatek,mt6323-rtc": for MT6323 PMIC
+ "mediatek,mt6397-rtc": for MT6397 PMIC
+
+Optional child node:
+- power-off
+ For Power-Off Device for MediaTek PMIC RTC bindings, see:
+ Documentation/devicetree/bindings/power/reset/mt6397-rtc-poweroff.txt
+
+Example:
+
+ pmic {
+ compatible = "mediatek,mt6323";
+
+ ...
+ rtc {
+ compatible = "mediatek,mt6323-rtc";
+
+ power-off {
+ compatible = "mediatek,mt6323-rtc-poweroff";
+ };
+ };
+};
--
2.7.4
From: Sean Wang <[email protected]>
It's unnecessary doing irq_dispose_mapping as a reverse operation for
platform_get_irq.
Ususally, irq_dispose_mapping should be called in error path or module
removal to release the resources for irq_of_parse_and_map requested.
Signed-off-by: Sean Wang <[email protected]>
---
drivers/rtc/rtc-mt6397.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/rtc/rtc-mt6397.c b/drivers/rtc/rtc-mt6397.c
index b62eaa8..cefb83b 100644
--- a/drivers/rtc/rtc-mt6397.c
+++ b/drivers/rtc/rtc-mt6397.c
@@ -17,7 +17,6 @@
#include <linux/module.h>
#include <linux/regmap.h>
#include <linux/rtc.h>
-#include <linux/irqdomain.h>
#include <linux/jiffies.h>
#include <linux/platform_device.h>
#include <linux/of_address.h>
@@ -336,7 +335,7 @@ static int mtk_rtc_probe(struct platform_device *pdev)
if (ret) {
dev_err(&pdev->dev, "Failed to request alarm IRQ: %d: %d\n",
rtc->irq, ret);
- goto out_dispose_irq;
+ return ret;
}
device_init_wakeup(&pdev->dev, 1);
@@ -353,8 +352,7 @@ static int mtk_rtc_probe(struct platform_device *pdev)
out_free_irq:
free_irq(rtc->irq, rtc->rtc_dev);
-out_dispose_irq:
- irq_dispose_mapping(rtc->irq);
+
return ret;
}
@@ -364,7 +362,6 @@ static int mtk_rtc_remove(struct platform_device *pdev)
rtc_device_unregister(rtc->rtc_dev);
free_irq(rtc->irq, rtc->rtc_dev);
- irq_dispose_mapping(rtc->irq);
return 0;
}
--
2.7.4
Hi,
On 23/03/2018 at 17:14:59 +0800, [email protected] wrote:
> From: Sean Wang <[email protected]>
>
> Add device-tree binding for MediaTek PMIC based RTC.
>
> Signed-off-by: Sean Wang <[email protected]>
> ---
> .../devicetree/bindings/rtc/rtc-mt6397.txt | 39 ++++++++++++++++++++++
> 1 file changed, 39 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/rtc/rtc-mt6397.txt
>
> diff --git a/Documentation/devicetree/bindings/rtc/rtc-mt6397.txt b/Documentation/devicetree/bindings/rtc/rtc-mt6397.txt
> new file mode 100644
> index 0000000..83ff6be
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/rtc/rtc-mt6397.txt
> @@ -0,0 +1,39 @@
> +Device-Tree bindings for MediaTek PMIC based RTC
> +
> +MediaTek PMIC based RTC is an independent function of MediaTek PMIC which
> +is working as a multi-function device (MFD). And the RTC can be configured and
> +set up via PMIC wrapper bus. Which is also common resource shared among the
> +other functions present on the PMIC.
> +
> +For MediaTek PMIC wrapper bus bindings, see:
> +Documentation/devicetree/bindings/soc/mediatek/pwrap.txt
> +
> +Required parent node:
> +- pmic
> + For MediaTek PMIC MFD bindings, see:
> + Documentation/devicetree/bindings/mfd/mt6397.txt
> +
> +Required properties:
> +- compatible: Should be one of follows
> + "mediatek,mt6323-rtc": for MT6323 PMIC
> + "mediatek,mt6397-rtc": for MT6397 PMIC
> +
> +Optional child node:
> +- power-off
> + For Power-Off Device for MediaTek PMIC RTC bindings, see:
> + Documentation/devicetree/bindings/power/reset/mt6397-rtc-poweroff.txt
> +
> +Example:
> +
> + pmic {
> + compatible = "mediatek,mt6323";
> +
> + ...
> + rtc {
> + compatible = "mediatek,mt6323-rtc";
> +
> + power-off {
> + compatible = "mediatek,mt6323-rtc-poweroff";
> + };
I'm pretty sure the whole point of mfd is to avoid having the poweroff
controller under the rtc
--
Alexandre Belloni, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
On 23/03/2018 at 17:15:03 +0800, [email protected] wrote:
> From: Sean Wang <[email protected]>
>
> Remove unnecessary parentheses due to explicit C operator precedence.
>
> Signed-off-by: Sean Wang <[email protected]>
> ---
> drivers/rtc/rtc-mt6397.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/rtc/rtc-mt6397.c b/drivers/rtc/rtc-mt6397.c
> index 0df7ccd..4411c08 100644
> --- a/drivers/rtc/rtc-mt6397.c
> +++ b/drivers/rtc/rtc-mt6397.c
> @@ -106,7 +106,7 @@ static irqreturn_t mtk_rtc_irq_handler_thread(int irq, void *data)
> int ret;
>
> ret = regmap_read(rtc->regmap, rtc->addr_base + RTC_IRQ_STA, &irqsta);
> - if ((ret >= 0) && (irqsta & RTC_IRQ_STA_AL)) {
> + if (ret >= 0 && irqsta & RTC_IRQ_STA_AL) {
I don't think this makes the code particularly clearer.
> rtc_update_irq(rtc->rtc_dev, 1, RTC_IRQF | RTC_AF);
> irqen = irqsta & ~RTC_IRQ_EN_AL;
> mutex_lock(&rtc->lock);
> --
> 2.7.4
>
--
Alexandre Belloni, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
On 23/03/2018 at 17:15:05 +0800, [email protected] wrote:
> From: Sean Wang <[email protected]>
>
> It's unnecessary doing irq_dispose_mapping as a reverse operation for
> platform_get_irq.
>
> Ususally, irq_dispose_mapping should be called in error path or module
> removal to release the resources for irq_of_parse_and_map requested.
>
> Signed-off-by: Sean Wang <[email protected]>
> ---
> drivers/rtc/rtc-mt6397.c | 7 ++-----
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/rtc/rtc-mt6397.c b/drivers/rtc/rtc-mt6397.c
> index b62eaa8..cefb83b 100644
> --- a/drivers/rtc/rtc-mt6397.c
> +++ b/drivers/rtc/rtc-mt6397.c
> @@ -17,7 +17,6 @@
> #include <linux/module.h>
> #include <linux/regmap.h>
> #include <linux/rtc.h>
> -#include <linux/irqdomain.h>
> #include <linux/jiffies.h>
> #include <linux/platform_device.h>
> #include <linux/of_address.h>
> @@ -336,7 +335,7 @@ static int mtk_rtc_probe(struct platform_device *pdev)
> if (ret) {
> dev_err(&pdev->dev, "Failed to request alarm IRQ: %d: %d\n",
> rtc->irq, ret);
> - goto out_dispose_irq;
> + return ret;
> }
>
> device_init_wakeup(&pdev->dev, 1);
> @@ -353,8 +352,7 @@ static int mtk_rtc_probe(struct platform_device *pdev)
>
> out_free_irq:
> free_irq(rtc->irq, rtc->rtc_dev);
> -out_dispose_irq:
> - irq_dispose_mapping(rtc->irq);
> +
Don't you still have a irq_create_mapping?
> return ret;
> }
>
> @@ -364,7 +362,6 @@ static int mtk_rtc_remove(struct platform_device *pdev)
>
> rtc_device_unregister(rtc->rtc_dev);
> free_irq(rtc->irq, rtc->rtc_dev);
> - irq_dispose_mapping(rtc->irq);
>
> return 0;
> }
> --
> 2.7.4
>
--
Alexandre Belloni, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
On 23/03/2018 at 10:41:18 +0100, Alexandre Belloni wrote:
> Hi,
>
> On 23/03/2018 at 17:14:59 +0800, [email protected] wrote:
> > From: Sean Wang <[email protected]>
> >
> > Add device-tree binding for MediaTek PMIC based RTC.
> >
> > Signed-off-by: Sean Wang <[email protected]>
> > ---
> > .../devicetree/bindings/rtc/rtc-mt6397.txt | 39 ++++++++++++++++++++++
> > 1 file changed, 39 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/rtc/rtc-mt6397.txt
> >
> > diff --git a/Documentation/devicetree/bindings/rtc/rtc-mt6397.txt b/Documentation/devicetree/bindings/rtc/rtc-mt6397.txt
> > new file mode 100644
> > index 0000000..83ff6be
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/rtc/rtc-mt6397.txt
> > @@ -0,0 +1,39 @@
> > +Device-Tree bindings for MediaTek PMIC based RTC
> > +
> > +MediaTek PMIC based RTC is an independent function of MediaTek PMIC which
> > +is working as a multi-function device (MFD). And the RTC can be configured and
> > +set up via PMIC wrapper bus. Which is also common resource shared among the
> > +other functions present on the PMIC.
> > +
> > +For MediaTek PMIC wrapper bus bindings, see:
> > +Documentation/devicetree/bindings/soc/mediatek/pwrap.txt
> > +
> > +Required parent node:
> > +- pmic
> > + For MediaTek PMIC MFD bindings, see:
> > + Documentation/devicetree/bindings/mfd/mt6397.txt
> > +
> > +Required properties:
> > +- compatible: Should be one of follows
> > + "mediatek,mt6323-rtc": for MT6323 PMIC
> > + "mediatek,mt6397-rtc": for MT6397 PMIC
> > +
> > +Optional child node:
> > +- power-off
> > + For Power-Off Device for MediaTek PMIC RTC bindings, see:
> > + Documentation/devicetree/bindings/power/reset/mt6397-rtc-poweroff.txt
> > +
> > +Example:
> > +
> > + pmic {
> > + compatible = "mediatek,mt6323";
> > +
> > + ...
> > + rtc {
> > + compatible = "mediatek,mt6323-rtc";
> > +
> > + power-off {
> > + compatible = "mediatek,mt6323-rtc-poweroff";
> > + };
>
> I'm pretty sure the whole point of mfd is to avoid having the poweroff
> controller under the rtc
>
BTW, I think it is enough to have that documented in only one file (the
MFD one is enough)
>
> --
> Alexandre Belloni, Bootlin (formerly Free Electrons)
> Embedded Linux and Kernel engineering
> https://bootlin.com
--
Alexandre Belloni, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
On 23/03/2018 at 17:15:06 +0800, [email protected] wrote:
> From: Sean Wang <[email protected]>
>
> Use device managed operation to simplify error handling, reduce source
> code size, and reduce the likelyhood of bugs, and remove our removal
> callback which contains anything already done by device managed functions.
>
> Signed-off-by: Sean Wang <[email protected]>
> ---
> drivers/rtc/rtc-mt6397.c | 31 ++++++++-----------------------
> 1 file changed, 8 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/rtc/rtc-mt6397.c b/drivers/rtc/rtc-mt6397.c
> index cefb83b..bfc5d6f 100644
> --- a/drivers/rtc/rtc-mt6397.c
> +++ b/drivers/rtc/rtc-mt6397.c
> @@ -14,6 +14,7 @@
>
> #include <linux/delay.h>
> #include <linux/init.h>
> +#include <linux/interrupt.h>
> #include <linux/module.h>
> #include <linux/regmap.h>
> #include <linux/rtc.h>
> @@ -328,10 +329,10 @@ static int mtk_rtc_probe(struct platform_device *pdev)
>
> 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);
> + ret = devm_request_threaded_irq(&pdev->dev, 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);
> @@ -340,30 +341,15 @@ static int mtk_rtc_probe(struct platform_device *pdev)
>
> device_init_wakeup(&pdev->dev, 1);
>
> - rtc->rtc_dev = rtc_device_register("mt6397-rtc", &pdev->dev,
> - &mtk_rtc_ops, THIS_MODULE);
> + rtc->rtc_dev = devm_rtc_device_register(&pdev->dev, "mt6397-rtc",
> + &mtk_rtc_ops, THIS_MODULE);
You should probably switch to devm_rtc_allocate_device() and
rtc_register_device instead of devm_rtc_device_register.
> 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;
> + return ret;
ret doesn't seem necessary anymore here.
--
Alexandre Belloni, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
Hi,
The subject line should be rtc: mt6397: (to differentiate with rtc-mt7622)
On 23/03/2018 at 17:15:02 +0800, [email protected] wrote:
> From: Sean Wang <[email protected]>
>
> Just to add MT6323 support to the existent RTC driver.
>
> Signed-off-by: Sean Wang <[email protected]>
> ---
> drivers/rtc/rtc-mt6397.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/rtc/rtc-mt6397.c b/drivers/rtc/rtc-mt6397.c
> index 385f830..0df7ccd 100644
> --- a/drivers/rtc/rtc-mt6397.c
> +++ b/drivers/rtc/rtc-mt6397.c
> @@ -398,6 +398,7 @@ static SIMPLE_DEV_PM_OPS(mt6397_pm_ops, mt6397_rtc_suspend,
> mt6397_rtc_resume);
>
> static const struct of_device_id mt6397_rtc_of_match[] = {
> + { .compatible = "mediatek,mt6323-rtc", },
> { .compatible = "mediatek,mt6397-rtc", },
> { }
> };
> --
> 2.7.4
>
--
Alexandre Belloni, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
On 23/03/2018 at 17:15:08 +0800, [email protected] wrote:
> From: Sean Wang <[email protected]>
>
> This is in preparation for allowing other drivers can share the
> declaration, so move the declaration into a globally visible header file.
>
> Signed-off-by: Sean Wang <[email protected]>
> ---
> drivers/rtc/rtc-mt6397.c | 53 +---------------------------------
> include/linux/rtc/mt6397.h | 72 ++++++++++++++++++++++++++++++++++++++++++++++
This should go in include/linux/mfd/
> 2 files changed, 73 insertions(+), 52 deletions(-)
> create mode 100644 include/linux/rtc/mt6397.h
>
> diff --git a/drivers/rtc/rtc-mt6397.c b/drivers/rtc/rtc-mt6397.c
> index d133d1f..015609d 100644
> --- a/drivers/rtc/rtc-mt6397.c
> +++ b/drivers/rtc/rtc-mt6397.c
> @@ -19,63 +19,12 @@
> #include <linux/of_platform.h>
> #include <linux/regmap.h>
> #include <linux/rtc.h>
> -#include <linux/jiffies.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)
> -
> -#define MTK_RTC_POLL_DELAY_US 10
> -#define MTK_RTC_POLL_TIMEOUT (jiffies_to_usecs(HZ))
> -
> -struct mt6397_rtc {
> - struct device *dev;
> - struct rtc_device *rtc_dev;
> - struct mutex lock;
> - struct regmap *regmap;
> - int irq;
> - u32 addr_base;
> -};
> +#include <linux/rtc/mt6397.h>
>
> static int mtk_rtc_write_trigger(struct mt6397_rtc *rtc)
> {
> diff --git a/include/linux/rtc/mt6397.h b/include/linux/rtc/mt6397.h
> new file mode 100644
> index 0000000..4b19f51
> --- /dev/null
> +++ b/include/linux/rtc/mt6397.h
> @@ -0,0 +1,72 @@
> +
Unnecessary empty line
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2014-2018 MediaTek Inc.
> + *
> + * Author: Tianping.Fang <[email protected]>
> + * Sean Wang <[email protected]>
> + */
> +
> +#ifndef _LINUX_RTC_MT6397_H_
> +#define _LINUX_RTC_MT6397_H_
> +
> +#include <linux/jiffies.h>
> +#include <linux/mutex.h>
> +#include <linux/regmap.h>
> +#include <linux/rtc.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)
> +
> +#define MTK_RTC_POLL_DELAY_US 10
> +#define MTK_RTC_POLL_TIMEOUT (jiffies_to_usecs(HZ))
> +
> +struct mt6397_rtc {
> + struct device *dev;
> + struct rtc_device *rtc_dev;
> +
> + /* protect registers accessing */
> + struct mutex lock;
> + struct regmap *regmap;
> + int irq;
> + u32 addr_base;
> +};
> +
> +#endif /* _LINUX_RTC_MT6397_H_ */
> +
> --
> 2.7.4
>
--
Alexandre Belloni, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
On Fri, 2018-03-23 at 11:01 +0100, Alexandre Belloni wrote:
> Hi,
>
> The subject line should be rtc: mt6397: (to differentiate with rtc-mt7622)
>
Sure, I will change subject line into rtc: mt6397: along with the other
related patches.
> On 23/03/2018 at 17:15:02 +0800, [email protected] wrote:
> > From: Sean Wang <[email protected]>
> >
> > Just to add MT6323 support to the existent RTC driver.
> >
> > Signed-off-by: Sean Wang <[email protected]>
> > ---
> > drivers/rtc/rtc-mt6397.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/rtc/rtc-mt6397.c b/drivers/rtc/rtc-mt6397.c
> > index 385f830..0df7ccd 100644
> > --- a/drivers/rtc/rtc-mt6397.c
> > +++ b/drivers/rtc/rtc-mt6397.c
> > @@ -398,6 +398,7 @@ static SIMPLE_DEV_PM_OPS(mt6397_pm_ops, mt6397_rtc_suspend,
> > mt6397_rtc_resume);
> >
> > static const struct of_device_id mt6397_rtc_of_match[] = {
> > + { .compatible = "mediatek,mt6323-rtc", },
> > { .compatible = "mediatek,mt6397-rtc", },
> > { }
> > };
> > --
> > 2.7.4
> >
>
On Fri, 2018-03-23 at 11:21 +0100, Alexandre Belloni wrote:
> On 23/03/2018 at 17:15:03 +0800, [email protected] wrote:
> > From: Sean Wang <[email protected]>
> >
> > Remove unnecessary parentheses due to explicit C operator precedence.
> >
> > Signed-off-by: Sean Wang <[email protected]>
> > ---
> > drivers/rtc/rtc-mt6397.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/rtc/rtc-mt6397.c b/drivers/rtc/rtc-mt6397.c
> > index 0df7ccd..4411c08 100644
> > --- a/drivers/rtc/rtc-mt6397.c
> > +++ b/drivers/rtc/rtc-mt6397.c
> > @@ -106,7 +106,7 @@ static irqreturn_t mtk_rtc_irq_handler_thread(int irq, void *data)
> > int ret;
> >
> > ret = regmap_read(rtc->regmap, rtc->addr_base + RTC_IRQ_STA, &irqsta);
> > - if ((ret >= 0) && (irqsta & RTC_IRQ_STA_AL)) {
> > + if (ret >= 0 && irqsta & RTC_IRQ_STA_AL) {
>
> I don't think this makes the code particularly clearer.
>
But it is still a one of check items in checkpatch
CHECK:UNNECESSARY_PARENTHESES: Unnecessary parentheses around 'ret >= 0'
#126: FILE: drivers/rtc/rtc-xxx.c:109:
+ if ((ret >= 0) && (irqsta & RTC_IRQ_STA_AL)) {
or we still want to keep it in parentheses around here?
> > rtc_update_irq(rtc->rtc_dev, 1, RTC_IRQF | RTC_AF);
> > irqen = irqsta & ~RTC_IRQ_EN_AL;
> > mutex_lock(&rtc->lock);
> > --
> > 2.7.4
> >
>
On Fri, 2018-03-23 at 10:57 +0100, Alexandre Belloni wrote:
> On 23/03/2018 at 17:15:08 +0800, [email protected] wrote:
> > From: Sean Wang <[email protected]>
> >
> > This is in preparation for allowing other drivers can share the
> > declaration, so move the declaration into a globally visible header file.
> >
> > Signed-off-by: Sean Wang <[email protected]>
> > ---
> > drivers/rtc/rtc-mt6397.c | 53 +---------------------------------
> > include/linux/rtc/mt6397.h | 72 ++++++++++++++++++++++++++++++++++++++++++++++
>
> This should go in include/linux/mfd/
include/linux/mfd/mt6397 is present, so include/linux/mfd/mt6397/rtc.h
seems a nice place to keep these declarations for the pmic rtc.
> > 2 files changed, 73 insertions(+), 52 deletions(-)
> > create mode 100644 include/linux/rtc/mt6397.h
> >
> > diff --git a/drivers/rtc/rtc-mt6397.c b/drivers/rtc/rtc-mt6397.c
> > index d133d1f..015609d 100644
> > --- a/drivers/rtc/rtc-mt6397.c
> > +++ b/drivers/rtc/rtc-mt6397.c
> > @@ -19,63 +19,12 @@
> > #include <linux/of_platform.h>
> > #include <linux/regmap.h>
> > #include <linux/rtc.h>
> > -#include <linux/jiffies.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)
> > -
> > -#define MTK_RTC_POLL_DELAY_US 10
> > -#define MTK_RTC_POLL_TIMEOUT (jiffies_to_usecs(HZ))
> > -
> > -struct mt6397_rtc {
> > - struct device *dev;
> > - struct rtc_device *rtc_dev;
> > - struct mutex lock;
> > - struct regmap *regmap;
> > - int irq;
> > - u32 addr_base;
> > -};
> > +#include <linux/rtc/mt6397.h>
> >
> > static int mtk_rtc_write_trigger(struct mt6397_rtc *rtc)
> > {
> > diff --git a/include/linux/rtc/mt6397.h b/include/linux/rtc/mt6397.h
> > new file mode 100644
> > index 0000000..4b19f51
> > --- /dev/null
> > +++ b/include/linux/rtc/mt6397.h
> > @@ -0,0 +1,72 @@
> > +
>
> Unnecessary empty line
>
will remove the unnecessary empty line
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2014-2018 MediaTek Inc.
> > + *
> > + * Author: Tianping.Fang <[email protected]>
> > + * Sean Wang <[email protected]>
> > + */
> > +
> > +#ifndef _LINUX_RTC_MT6397_H_
> > +#define _LINUX_RTC_MT6397_H_
> > +
> > +#include <linux/jiffies.h>
> > +#include <linux/mutex.h>
> > +#include <linux/regmap.h>
> > +#include <linux/rtc.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)
> > +
> > +#define MTK_RTC_POLL_DELAY_US 10
> > +#define MTK_RTC_POLL_TIMEOUT (jiffies_to_usecs(HZ))
> > +
> > +struct mt6397_rtc {
> > + struct device *dev;
> > + struct rtc_device *rtc_dev;
> > +
> > + /* protect registers accessing */
> > + struct mutex lock;
> > + struct regmap *regmap;
> > + int irq;
> > + u32 addr_base;
> > +};
> > +
> > +#endif /* _LINUX_RTC_MT6397_H_ */
> > +
> > --
> > 2.7.4
> >
>
On 24/03/2018 at 15:14:12 +0800, Sean Wang wrote:
> On Fri, 2018-03-23 at 11:21 +0100, Alexandre Belloni wrote:
> > On 23/03/2018 at 17:15:03 +0800, [email protected] wrote:
> > > From: Sean Wang <[email protected]>
> > >
> > > Remove unnecessary parentheses due to explicit C operator precedence.
> > >
> > > Signed-off-by: Sean Wang <[email protected]>
> > > ---
> > > drivers/rtc/rtc-mt6397.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/rtc/rtc-mt6397.c b/drivers/rtc/rtc-mt6397.c
> > > index 0df7ccd..4411c08 100644
> > > --- a/drivers/rtc/rtc-mt6397.c
> > > +++ b/drivers/rtc/rtc-mt6397.c
> > > @@ -106,7 +106,7 @@ static irqreturn_t mtk_rtc_irq_handler_thread(int irq, void *data)
> > > int ret;
> > >
> > > ret = regmap_read(rtc->regmap, rtc->addr_base + RTC_IRQ_STA, &irqsta);
> > > - if ((ret >= 0) && (irqsta & RTC_IRQ_STA_AL)) {
> > > + if (ret >= 0 && irqsta & RTC_IRQ_STA_AL) {
> >
> > I don't think this makes the code particularly clearer.
> >
>
> But it is still a one of check items in checkpatch
>
> CHECK:UNNECESSARY_PARENTHESES: Unnecessary parentheses around 'ret >= 0'
> #126: FILE: drivers/rtc/rtc-xxx.c:109:
> + if ((ret >= 0) && (irqsta & RTC_IRQ_STA_AL)) {
>
>
> or we still want to keep it in parentheses around here?
>
Yeah, this is a matter of taste, I would keep the parentheses.
--
Alexandre Belloni, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
On 24/03/2018 at 15:31:49 +0800, Sean Wang wrote:
> On Fri, 2018-03-23 at 10:57 +0100, Alexandre Belloni wrote:
> > On 23/03/2018 at 17:15:08 +0800, [email protected] wrote:
> > > From: Sean Wang <[email protected]>
> > >
> > > This is in preparation for allowing other drivers can share the
> > > declaration, so move the declaration into a globally visible header file.
> > >
> > > Signed-off-by: Sean Wang <[email protected]>
> > > ---
> > > drivers/rtc/rtc-mt6397.c | 53 +---------------------------------
> > > include/linux/rtc/mt6397.h | 72 ++++++++++++++++++++++++++++++++++++++++++++++
> >
> > This should go in include/linux/mfd/
>
> include/linux/mfd/mt6397 is present, so include/linux/mfd/mt6397/rtc.h
> seems a nice place to keep these declarations for the pmic rtc.
>
Yes, this seems good to me.
> > > 2 files changed, 73 insertions(+), 52 deletions(-)
> > > create mode 100644 include/linux/rtc/mt6397.h
> > >
> > > diff --git a/drivers/rtc/rtc-mt6397.c b/drivers/rtc/rtc-mt6397.c
> > > index d133d1f..015609d 100644
> > > --- a/drivers/rtc/rtc-mt6397.c
> > > +++ b/drivers/rtc/rtc-mt6397.c
> > > @@ -19,63 +19,12 @@
> > > #include <linux/of_platform.h>
> > > #include <linux/regmap.h>
> > > #include <linux/rtc.h>
> > > -#include <linux/jiffies.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)
> > > -
> > > -#define MTK_RTC_POLL_DELAY_US 10
> > > -#define MTK_RTC_POLL_TIMEOUT (jiffies_to_usecs(HZ))
> > > -
> > > -struct mt6397_rtc {
> > > - struct device *dev;
> > > - struct rtc_device *rtc_dev;
> > > - struct mutex lock;
> > > - struct regmap *regmap;
> > > - int irq;
> > > - u32 addr_base;
> > > -};
> > > +#include <linux/rtc/mt6397.h>
> > >
> > > static int mtk_rtc_write_trigger(struct mt6397_rtc *rtc)
> > > {
> > > diff --git a/include/linux/rtc/mt6397.h b/include/linux/rtc/mt6397.h
> > > new file mode 100644
> > > index 0000000..4b19f51
> > > --- /dev/null
> > > +++ b/include/linux/rtc/mt6397.h
> > > @@ -0,0 +1,72 @@
> > > +
> >
> > Unnecessary empty line
> >
>
> will remove the unnecessary empty line
>
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Copyright (C) 2014-2018 MediaTek Inc.
> > > + *
> > > + * Author: Tianping.Fang <[email protected]>
> > > + * Sean Wang <[email protected]>
> > > + */
> > > +
> > > +#ifndef _LINUX_RTC_MT6397_H_
> > > +#define _LINUX_RTC_MT6397_H_
> > > +
> > > +#include <linux/jiffies.h>
> > > +#include <linux/mutex.h>
> > > +#include <linux/regmap.h>
> > > +#include <linux/rtc.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)
> > > +
> > > +#define MTK_RTC_POLL_DELAY_US 10
> > > +#define MTK_RTC_POLL_TIMEOUT (jiffies_to_usecs(HZ))
> > > +
> > > +struct mt6397_rtc {
> > > + struct device *dev;
> > > + struct rtc_device *rtc_dev;
> > > +
> > > + /* protect registers accessing */
> > > + struct mutex lock;
> > > + struct regmap *regmap;
> > > + int irq;
> > > + u32 addr_base;
> > > +};
> > > +
> > > +#endif /* _LINUX_RTC_MT6397_H_ */
> > > +
> > > --
> > > 2.7.4
> > >
> >
>
>
--
Alexandre Belloni, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
On Sat, 2018-03-24 at 19:53 +0100, Alexandre Belloni wrote:
> On 24/03/2018 at 15:14:12 +0800, Sean Wang wrote:
> > On Fri, 2018-03-23 at 11:21 +0100, Alexandre Belloni wrote:
> > > On 23/03/2018 at 17:15:03 +0800, [email protected] wrote:
> > > > From: Sean Wang <[email protected]>
> > > >
> > > > Remove unnecessary parentheses due to explicit C operator precedence.
> > > >
> > > > Signed-off-by: Sean Wang <[email protected]>
> > > > ---
> > > > drivers/rtc/rtc-mt6397.c | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/rtc/rtc-mt6397.c b/drivers/rtc/rtc-mt6397.c
> > > > index 0df7ccd..4411c08 100644
> > > > --- a/drivers/rtc/rtc-mt6397.c
> > > > +++ b/drivers/rtc/rtc-mt6397.c
> > > > @@ -106,7 +106,7 @@ static irqreturn_t mtk_rtc_irq_handler_thread(int irq, void *data)
> > > > int ret;
> > > >
> > > > ret = regmap_read(rtc->regmap, rtc->addr_base + RTC_IRQ_STA, &irqsta);
> > > > - if ((ret >= 0) && (irqsta & RTC_IRQ_STA_AL)) {
> > > > + if (ret >= 0 && irqsta & RTC_IRQ_STA_AL) {
> > >
> > > I don't think this makes the code particularly clearer.
> > >
> >
> > But it is still a one of check items in checkpatch
> >
> > CHECK:UNNECESSARY_PARENTHESES: Unnecessary parentheses around 'ret >= 0'
> > #126: FILE: drivers/rtc/rtc-xxx.c:109:
> > + if ((ret >= 0) && (irqsta & RTC_IRQ_STA_AL)) {
> >
> >
> > or we still want to keep it in parentheses around here?
> >
>
> Yeah, this is a matter of taste, I would keep the parentheses.
okay, lets keep the parentheses
>
>
On Fri, 2018-03-23 at 11:15 +0100, Alexandre Belloni wrote:
> On 23/03/2018 at 10:41:18 +0100, Alexandre Belloni wrote:
> > Hi,
> >
> > On 23/03/2018 at 17:14:59 +0800, [email protected] wrote:
> > > From: Sean Wang <[email protected]>
> > >
> > > Add device-tree binding for MediaTek PMIC based RTC.
> > >
> > > Signed-off-by: Sean Wang <[email protected]>
> > > ---
> > > .../devicetree/bindings/rtc/rtc-mt6397.txt | 39 ++++++++++++++++++++++
> > > 1 file changed, 39 insertions(+)
> > > create mode 100644 Documentation/devicetree/bindings/rtc/rtc-mt6397.txt
> > >
> > > diff --git a/Documentation/devicetree/bindings/rtc/rtc-mt6397.txt b/Documentation/devicetree/bindings/rtc/rtc-mt6397.txt
> > > new file mode 100644
> > > index 0000000..83ff6be
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/rtc/rtc-mt6397.txt
> > > @@ -0,0 +1,39 @@
> > > +Device-Tree bindings for MediaTek PMIC based RTC
> > > +
> > > +MediaTek PMIC based RTC is an independent function of MediaTek PMIC which
> > > +is working as a multi-function device (MFD). And the RTC can be configured and
> > > +set up via PMIC wrapper bus. Which is also common resource shared among the
> > > +other functions present on the PMIC.
> > > +
> > > +For MediaTek PMIC wrapper bus bindings, see:
> > > +Documentation/devicetree/bindings/soc/mediatek/pwrap.txt
> > > +
> > > +Required parent node:
> > > +- pmic
> > > + For MediaTek PMIC MFD bindings, see:
> > > + Documentation/devicetree/bindings/mfd/mt6397.txt
> > > +
> > > +Required properties:
> > > +- compatible: Should be one of follows
> > > + "mediatek,mt6323-rtc": for MT6323 PMIC
> > > + "mediatek,mt6397-rtc": for MT6397 PMIC
> > > +
> > > +Optional child node:
> > > +- power-off
> > > + For Power-Off Device for MediaTek PMIC RTC bindings, see:
> > > + Documentation/devicetree/bindings/power/reset/mt6397-rtc-poweroff.txt
> > > +
> > > +Example:
> > > +
> > > + pmic {
> > > + compatible = "mediatek,mt6323";
> > > +
> > > + ...
> > > + rtc {
> > > + compatible = "mediatek,mt6323-rtc";
> > > +
> > > + power-off {
> > > + compatible = "mediatek,mt6323-rtc-poweroff";
> > > + };
> >
> > I'm pretty sure the whole point of mfd is to avoid having the poweroff
> > controller under the rtc
> >
>
> BTW, I think it is enough to have that documented in only one file (the
> MFD one is enough)
>
just reply both replies in the same mail
1.) the power-off device is a part of rtc, use the same registers rtc
has and thus it is put as child nodes under the node rtc to reflect the
reality of characteristics the rtc has.
Or am I wrong for a certain aspect in these opinions?
2) the other sub-functions for the same pmic already created its own
dt-binding document belonged to its corresponding subsystem. Don't we
really want to follow it them all?
> >
> > --
> > Alexandre Belloni, Bootlin (formerly Free Electrons)
> > Embedded Linux and Kernel engineering
> > https://bootlin.com
>
On Fri, Mar 23, 2018 at 6:15 AM, <[email protected]> wrote:
> --- /dev/null
> +++ b/include/linux/rtc/mt6397.h
> @@ -0,0 +1,72 @@
> +
> +// SPDX-License-Identifier: GPL-2.0
According to Documentation/process/license-rules.rst the SPDX notation
for C header file should be:
/* SPDX-License-Identifier: GPL-2.0 */
On Sat, 2018-03-24 at 17:00 -0300, Fabio Estevam wrote:
> On Fri, Mar 23, 2018 at 6:15 AM, <[email protected]> wrote:
>
> > --- /dev/null
> > +++ b/include/linux/rtc/mt6397.h
> > @@ -0,0 +1,72 @@
> > +
> > +// SPDX-License-Identifier: GPL-2.0
>
> According to Documentation/process/license-rules.rst the SPDX notation
> for C header file should be:
>
> /* SPDX-License-Identifier: GPL-2.0 */
Thank you, I'll have an improvement according to the license rule.
Hi Sean,
I love your patch! Yet something to improve:
[auto build test ERROR on abelloni/rtc-next]
[also build test ERROR on v4.16-rc6 next-20180323]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/sean-wang-mediatek-com/Add-support-to-MT6323-RTC-and-its-power-device/20180325-104529
base: https://git.kernel.org/pub/scm/linux/kernel/git/abelloni/linux.git rtc-next
config: x86_64-randconfig-x004-201812 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
drivers//rtc/rtc-mt6397.c: In function 'mtk_rtc_probe':
>> drivers//rtc/rtc-mt6397.c:270:13: error: implicit declaration of function 'irq_create_mapping'; did you mean 'qid_has_mapping'? [-Werror=implicit-function-declaration]
rtc->irq = irq_create_mapping(mt6397_chip->irq_domain, res->start);
^~~~~~~~~~~~~~~~~~
qid_has_mapping
Cyclomatic Complexity 1 include/linux/err.h:PTR_ERR
Cyclomatic Complexity 1 include/linux/err.h:IS_ERR
Cyclomatic Complexity 1 include/linux/math64.h:div_s64_rem
Cyclomatic Complexity 1 include/linux/math64.h:div_s64
Cyclomatic Complexity 3 include/linux/ktime.h:ktime_compare
Cyclomatic Complexity 1 include/linux/ktime.h:ktime_add_us
Cyclomatic Complexity 1 include/linux/device.h:devm_kzalloc
Cyclomatic Complexity 1 include/linux/pm_wakeup.h:device_set_wakeup_capable
Cyclomatic Complexity 1 include/linux/pm_wakeup.h:device_set_wakeup_enable
Cyclomatic Complexity 1 include/linux/pm_wakeup.h:device_init_wakeup
Cyclomatic Complexity 1 include/linux/device.h:dev_get_drvdata
Cyclomatic Complexity 1 include/linux/device.h:dev_set_drvdata
Cyclomatic Complexity 1 include/linux/platform_device.h:platform_set_drvdata
Cyclomatic Complexity 1 include/linux/of_platform.h:devm_of_platform_populate
Cyclomatic Complexity 1 drivers//rtc/rtc-mt6397.c:mtk_rtc_driver_init
Cyclomatic Complexity 5 drivers//rtc/rtc-mt6397.c:mtk_rtc_probe
Cyclomatic Complexity 11 drivers//rtc/rtc-mt6397.c:mtk_rtc_write_trigger
Cyclomatic Complexity 6 drivers//rtc/rtc-mt6397.c:mtk_rtc_set_alarm
Cyclomatic Complexity 2 drivers//rtc/rtc-mt6397.c:mtk_rtc_set_time
Cyclomatic Complexity 4 drivers//rtc/rtc-mt6397.c:mtk_rtc_read_alarm
Cyclomatic Complexity 2 drivers//rtc/rtc-mt6397.c:__mtk_rtc_read_time
Cyclomatic Complexity 3 drivers//rtc/rtc-mt6397.c:mtk_rtc_read_time
Cyclomatic Complexity 4 drivers//rtc/rtc-mt6397.c:mtk_rtc_irq_handler_thread
Cyclomatic Complexity 1 drivers//rtc/rtc-mt6397.c:mtk_rtc_driver_exit
cc1: some warnings being treated as errors
vim +270 drivers//rtc/rtc-mt6397.c
fc2979118 Tianping Fang 2015-05-06 254
fc2979118 Tianping Fang 2015-05-06 255 static int mtk_rtc_probe(struct platform_device *pdev)
fc2979118 Tianping Fang 2015-05-06 256 {
fc2979118 Tianping Fang 2015-05-06 257 struct resource *res;
fc2979118 Tianping Fang 2015-05-06 258 struct mt6397_chip *mt6397_chip = dev_get_drvdata(pdev->dev.parent);
fc2979118 Tianping Fang 2015-05-06 259 struct mt6397_rtc *rtc;
fc2979118 Tianping Fang 2015-05-06 260 int ret;
fc2979118 Tianping Fang 2015-05-06 261
fc2979118 Tianping Fang 2015-05-06 262 rtc = devm_kzalloc(&pdev->dev, sizeof(struct mt6397_rtc), GFP_KERNEL);
fc2979118 Tianping Fang 2015-05-06 263 if (!rtc)
fc2979118 Tianping Fang 2015-05-06 264 return -ENOMEM;
fc2979118 Tianping Fang 2015-05-06 265
fc2979118 Tianping Fang 2015-05-06 266 res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
fc2979118 Tianping Fang 2015-05-06 267 rtc->addr_base = res->start;
fc2979118 Tianping Fang 2015-05-06 268
fc2979118 Tianping Fang 2015-05-06 269 res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
fc2979118 Tianping Fang 2015-05-06 @270 rtc->irq = irq_create_mapping(mt6397_chip->irq_domain, res->start);
fc2979118 Tianping Fang 2015-05-06 271 if (rtc->irq <= 0)
fc2979118 Tianping Fang 2015-05-06 272 return -EINVAL;
fc2979118 Tianping Fang 2015-05-06 273
fc2979118 Tianping Fang 2015-05-06 274 rtc->regmap = mt6397_chip->regmap;
fc2979118 Tianping Fang 2015-05-06 275 rtc->dev = &pdev->dev;
fc2979118 Tianping Fang 2015-05-06 276 mutex_init(&rtc->lock);
fc2979118 Tianping Fang 2015-05-06 277
fc2979118 Tianping Fang 2015-05-06 278 platform_set_drvdata(pdev, rtc);
fc2979118 Tianping Fang 2015-05-06 279
66c231b40 Sean Wang 2018-03-23 280 ret = devm_request_threaded_irq(&pdev->dev, rtc->irq, NULL,
fc2979118 Tianping Fang 2015-05-06 281 mtk_rtc_irq_handler_thread,
fc2979118 Tianping Fang 2015-05-06 282 IRQF_ONESHOT | IRQF_TRIGGER_HIGH,
fc2979118 Tianping Fang 2015-05-06 283 "mt6397-rtc", rtc);
fc2979118 Tianping Fang 2015-05-06 284 if (ret) {
fc2979118 Tianping Fang 2015-05-06 285 dev_err(&pdev->dev, "Failed to request alarm IRQ: %d: %d\n",
fc2979118 Tianping Fang 2015-05-06 286 rtc->irq, ret);
63044753b Sean Wang 2018-03-23 287 return ret;
fc2979118 Tianping Fang 2015-05-06 288 }
fc2979118 Tianping Fang 2015-05-06 289
baeca4495 Wei-Ning Huang 2015-07-02 290 device_init_wakeup(&pdev->dev, 1);
baeca4495 Wei-Ning Huang 2015-07-02 291
66c231b40 Sean Wang 2018-03-23 292 rtc->rtc_dev = devm_rtc_device_register(&pdev->dev, "mt6397-rtc",
fc2979118 Tianping Fang 2015-05-06 293 &mtk_rtc_ops, THIS_MODULE);
fc2979118 Tianping Fang 2015-05-06 294 if (IS_ERR(rtc->rtc_dev)) {
fc2979118 Tianping Fang 2015-05-06 295 dev_err(&pdev->dev, "register rtc device failed\n");
fc2979118 Tianping Fang 2015-05-06 296 ret = PTR_ERR(rtc->rtc_dev);
fc2979118 Tianping Fang 2015-05-06 297 return ret;
fc2979118 Tianping Fang 2015-05-06 298 }
fc2979118 Tianping Fang 2015-05-06 299
89a68f3c0 Sean Wang 2018-03-23 300 return devm_of_platform_populate(&pdev->dev);
fc2979118 Tianping Fang 2015-05-06 301 }
fc2979118 Tianping Fang 2015-05-06 302
:::::: The code at line 270 was first introduced by commit
:::::: fc2979118f3f5193475cb53d5df7bdaa7e358a42 rtc: mediatek: Add MT6397 RTC driver
:::::: TO: Tianping Fang <[email protected]>
:::::: CC: Alexandre Belloni <[email protected]>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Hi Sean,
I love your patch! Yet something to improve:
[auto build test ERROR on abelloni/rtc-next]
[also build test ERROR on v4.16-rc6]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/sean-wang-mediatek-com/Add-support-to-MT6323-RTC-and-its-power-device/20180325-104529
base: https://git.kernel.org/pub/scm/linux/kernel/git/abelloni/linux.git rtc-next
config: tile-allmodconfig (attached as .config)
compiler: tilegx-linux-gcc (GCC) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=tile
All errors (new ones prefixed by >>):
drivers//rtc/rtc-mt6397.c: In function 'mtk_rtc_probe':
>> drivers//rtc/rtc-mt6397.c:270:13: error: implicit declaration of function 'irq_create_mapping'; did you mean 'page_mapping'? [-Werror=implicit-function-declaration]
rtc->irq = irq_create_mapping(mt6397_chip->irq_domain, res->start);
^~~~~~~~~~~~~~~~~~
page_mapping
cc1: some warnings being treated as errors
vim +270 drivers//rtc/rtc-mt6397.c
fc2979118 Tianping Fang 2015-05-06 254
fc2979118 Tianping Fang 2015-05-06 255 static int mtk_rtc_probe(struct platform_device *pdev)
fc2979118 Tianping Fang 2015-05-06 256 {
fc2979118 Tianping Fang 2015-05-06 257 struct resource *res;
fc2979118 Tianping Fang 2015-05-06 258 struct mt6397_chip *mt6397_chip = dev_get_drvdata(pdev->dev.parent);
fc2979118 Tianping Fang 2015-05-06 259 struct mt6397_rtc *rtc;
fc2979118 Tianping Fang 2015-05-06 260 int ret;
fc2979118 Tianping Fang 2015-05-06 261
fc2979118 Tianping Fang 2015-05-06 262 rtc = devm_kzalloc(&pdev->dev, sizeof(struct mt6397_rtc), GFP_KERNEL);
fc2979118 Tianping Fang 2015-05-06 263 if (!rtc)
fc2979118 Tianping Fang 2015-05-06 264 return -ENOMEM;
fc2979118 Tianping Fang 2015-05-06 265
fc2979118 Tianping Fang 2015-05-06 266 res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
fc2979118 Tianping Fang 2015-05-06 267 rtc->addr_base = res->start;
fc2979118 Tianping Fang 2015-05-06 268
fc2979118 Tianping Fang 2015-05-06 269 res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
fc2979118 Tianping Fang 2015-05-06 @270 rtc->irq = irq_create_mapping(mt6397_chip->irq_domain, res->start);
fc2979118 Tianping Fang 2015-05-06 271 if (rtc->irq <= 0)
fc2979118 Tianping Fang 2015-05-06 272 return -EINVAL;
fc2979118 Tianping Fang 2015-05-06 273
fc2979118 Tianping Fang 2015-05-06 274 rtc->regmap = mt6397_chip->regmap;
fc2979118 Tianping Fang 2015-05-06 275 rtc->dev = &pdev->dev;
fc2979118 Tianping Fang 2015-05-06 276 mutex_init(&rtc->lock);
fc2979118 Tianping Fang 2015-05-06 277
fc2979118 Tianping Fang 2015-05-06 278 platform_set_drvdata(pdev, rtc);
fc2979118 Tianping Fang 2015-05-06 279
66c231b40 Sean Wang 2018-03-23 280 ret = devm_request_threaded_irq(&pdev->dev, rtc->irq, NULL,
fc2979118 Tianping Fang 2015-05-06 281 mtk_rtc_irq_handler_thread,
fc2979118 Tianping Fang 2015-05-06 282 IRQF_ONESHOT | IRQF_TRIGGER_HIGH,
fc2979118 Tianping Fang 2015-05-06 283 "mt6397-rtc", rtc);
fc2979118 Tianping Fang 2015-05-06 284 if (ret) {
fc2979118 Tianping Fang 2015-05-06 285 dev_err(&pdev->dev, "Failed to request alarm IRQ: %d: %d\n",
fc2979118 Tianping Fang 2015-05-06 286 rtc->irq, ret);
63044753b Sean Wang 2018-03-23 287 return ret;
fc2979118 Tianping Fang 2015-05-06 288 }
fc2979118 Tianping Fang 2015-05-06 289
baeca4495 Wei-Ning Huang 2015-07-02 290 device_init_wakeup(&pdev->dev, 1);
baeca4495 Wei-Ning Huang 2015-07-02 291
66c231b40 Sean Wang 2018-03-23 292 rtc->rtc_dev = devm_rtc_device_register(&pdev->dev, "mt6397-rtc",
fc2979118 Tianping Fang 2015-05-06 293 &mtk_rtc_ops, THIS_MODULE);
fc2979118 Tianping Fang 2015-05-06 294 if (IS_ERR(rtc->rtc_dev)) {
fc2979118 Tianping Fang 2015-05-06 295 dev_err(&pdev->dev, "register rtc device failed\n");
fc2979118 Tianping Fang 2015-05-06 296 ret = PTR_ERR(rtc->rtc_dev);
fc2979118 Tianping Fang 2015-05-06 297 return ret;
fc2979118 Tianping Fang 2015-05-06 298 }
fc2979118 Tianping Fang 2015-05-06 299
89a68f3c0 Sean Wang 2018-03-23 300 return devm_of_platform_populate(&pdev->dev);
fc2979118 Tianping Fang 2015-05-06 301 }
fc2979118 Tianping Fang 2015-05-06 302
:::::: The code at line 270 was first introduced by commit
:::::: fc2979118f3f5193475cb53d5df7bdaa7e358a42 rtc: mediatek: Add MT6397 RTC driver
:::::: TO: Tianping Fang <[email protected]>
:::::: CC: Alexandre Belloni <[email protected]>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
On Fri, 2018-03-23 at 11:38 +0100, Alexandre Belloni wrote:
> On 23/03/2018 at 17:15:05 +0800, [email protected] wrote:
> > From: Sean Wang <[email protected]>
> >
> > It's unnecessary doing irq_dispose_mapping as a reverse operation for
> > platform_get_irq.
> >
> > Ususally, irq_dispose_mapping should be called in error path or module
> > removal to release the resources for irq_of_parse_and_map requested.
> >
> > Signed-off-by: Sean Wang <[email protected]>
> > ---
> > drivers/rtc/rtc-mt6397.c | 7 ++-----
> > 1 file changed, 2 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/rtc/rtc-mt6397.c b/drivers/rtc/rtc-mt6397.c
> > index b62eaa8..cefb83b 100644
> > --- a/drivers/rtc/rtc-mt6397.c
> > +++ b/drivers/rtc/rtc-mt6397.c
> > @@ -17,7 +17,6 @@
> > #include <linux/module.h>
> > #include <linux/regmap.h>
> > #include <linux/rtc.h>
> > -#include <linux/irqdomain.h>
> > #include <linux/jiffies.h>
> > #include <linux/platform_device.h>
> > #include <linux/of_address.h>
> > @@ -336,7 +335,7 @@ static int mtk_rtc_probe(struct platform_device *pdev)
> > if (ret) {
> > dev_err(&pdev->dev, "Failed to request alarm IRQ: %d: %d\n",
> > rtc->irq, ret);
> > - goto out_dispose_irq;
> > + return ret;
> > }
> >
> > device_init_wakeup(&pdev->dev, 1);
> > @@ -353,8 +352,7 @@ static int mtk_rtc_probe(struct platform_device *pdev)
> >
> > out_free_irq:
> > free_irq(rtc->irq, rtc->rtc_dev);
> > -out_dispose_irq:
> > - irq_dispose_mapping(rtc->irq);
> > +
>
> Don't you still have a irq_create_mapping?
>
Sorry for that I didn't mention in the beginning that the series must
depend on another patch [1]. With the patch, the job irq_create_mapping
had been moved from rtc to mfd, so here it should be better to cleanup
up irq_dispose_mapping in all paths.
[1] https://patchwork.kernel.org/patch/9954643/
> > return ret;
> > }
> >
> > @@ -364,7 +362,6 @@ static int mtk_rtc_remove(struct platform_device *pdev)
> >
> > rtc_device_unregister(rtc->rtc_dev);
> > free_irq(rtc->irq, rtc->rtc_dev);
> > - irq_dispose_mapping(rtc->irq);
> >
> > return 0;
> > }
> > --
> > 2.7.4
> >
>
On Fri, 2018-03-23 at 11:50 +0100, Alexandre Belloni wrote:
> On 23/03/2018 at 17:15:06 +0800, [email protected] wrote:
> > From: Sean Wang <[email protected]>
> >
> > Use device managed operation to simplify error handling, reduce source
> > code size, and reduce the likelyhood of bugs, and remove our removal
> > callback which contains anything already done by device managed functions.
> >
> > Signed-off-by: Sean Wang <[email protected]>
> > ---
> > drivers/rtc/rtc-mt6397.c | 31 ++++++++-----------------------
> > 1 file changed, 8 insertions(+), 23 deletions(-)
> >
> > diff --git a/drivers/rtc/rtc-mt6397.c b/drivers/rtc/rtc-mt6397.c
> > index cefb83b..bfc5d6f 100644
> > --- a/drivers/rtc/rtc-mt6397.c
> > +++ b/drivers/rtc/rtc-mt6397.c
> > @@ -14,6 +14,7 @@
> >
> > #include <linux/delay.h>
> > #include <linux/init.h>
> > +#include <linux/interrupt.h>
> > #include <linux/module.h>
> > #include <linux/regmap.h>
> > #include <linux/rtc.h>
> > @@ -328,10 +329,10 @@ static int mtk_rtc_probe(struct platform_device *pdev)
> >
> > 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);
> > + ret = devm_request_threaded_irq(&pdev->dev, 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);
> > @@ -340,30 +341,15 @@ static int mtk_rtc_probe(struct platform_device *pdev)
> >
> > device_init_wakeup(&pdev->dev, 1);
> >
> > - rtc->rtc_dev = rtc_device_register("mt6397-rtc", &pdev->dev,
> > - &mtk_rtc_ops, THIS_MODULE);
> > + rtc->rtc_dev = devm_rtc_device_register(&pdev->dev, "mt6397-rtc",
> > + &mtk_rtc_ops, THIS_MODULE);
>
> You should probably switch to devm_rtc_allocate_device() and
> rtc_register_device instead of devm_rtc_device_register.
>
Just would like to know something details
It seems you just encourage me to switch into the new registration
method and currently devm_rtc_device_register I used for the driver
shouldn't cause any harm. right?
> > 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;
> > + return ret;
>
> ret doesn't seem necessary anymore here.
okay, it'll be removed
>
>
On Fri, Mar 23, 2018 at 05:15:00PM +0800, [email protected] wrote:
> From: Sean Wang <[email protected]>
>
> Add a description for MT6323 RTC and link it to the detailed binding
> documentation.
>
> Signed-off-by: Sean Wang <[email protected]>
> ---
> Documentation/devicetree/bindings/mfd/mt6397.txt | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
Reviewed-by: Rob Herring <[email protected]>
On Fri, Mar 23, 2018 at 05:14:58PM +0800, [email protected] wrote:
> From: Sean Wang <[email protected]>
>
> Add device-tree binding for the power device responsible for shutdown a
> remote SoC, which is a tiny circuit block present on MediaTek PMIC based
> RTC.
>
> Signed-off-by: Sean Wang <[email protected]>
> ---
> .../bindings/power/reset/mt6397-rtc-poweroff.txt | 24 ++++++++++++++++++++++
> 1 file changed, 24 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/power/reset/mt6397-rtc-poweroff.txt
>
> diff --git a/Documentation/devicetree/bindings/power/reset/mt6397-rtc-poweroff.txt b/Documentation/devicetree/bindings/power/reset/mt6397-rtc-poweroff.txt
> new file mode 100644
> index 0000000..75a9d7d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/reset/mt6397-rtc-poweroff.txt
> @@ -0,0 +1,24 @@
> +Device-Tree bindings for Power Device on MediaTek PMIC RTC
> +
> +The power device is responsible for externally power off or on the remote
> +MediaTek SoC through the a tiny circuit block BBPU inside PMIC RTC.
> +
> +Required parent node:
> +- rtc
> + For MediaTek PMIC RTC bindings, see:
> + Documentation/devicetree/bindings/mfd/mt6397.txt
> +
> +Required properties:
> +- compatible: Should be one of follows
> + "mediatek,mt6323-rtc-poweroff": for MT6323 PMIC
> + "mediatek,mt6397-rtc-poweroff": for MT6397 PMIC
> +
> +Example:
> +
> + rtc {
> + compatible = "mediatek,mt6323-rtc";
> +
> + power-off {
> + compatible = "mediatek,mt6323-rtc-poweroff";
> + };
There's no need for this node. The OS can register a device in the rtc
driver.
> + };
> --
> 2.7.4
>
On Mon, 2018-03-26 at 17:24 -0500, Rob Herring wrote:
> On Fri, Mar 23, 2018 at 05:14:58PM +0800, [email protected] wrote:
> > From: Sean Wang <[email protected]>
> >
> > Add device-tree binding for the power device responsible for shutdown a
> > remote SoC, which is a tiny circuit block present on MediaTek PMIC based
> > RTC.
> >
> > Signed-off-by: Sean Wang <[email protected]>
> > ---
> > .../bindings/power/reset/mt6397-rtc-poweroff.txt | 24 ++++++++++++++++++++++
> > 1 file changed, 24 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/power/reset/mt6397-rtc-poweroff.txt
> >
> > diff --git a/Documentation/devicetree/bindings/power/reset/mt6397-rtc-poweroff.txt b/Documentation/devicetree/bindings/power/reset/mt6397-rtc-poweroff.txt
> > new file mode 100644
> > index 0000000..75a9d7d
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/power/reset/mt6397-rtc-poweroff.txt
> > @@ -0,0 +1,24 @@
> > +Device-Tree bindings for Power Device on MediaTek PMIC RTC
> > +
> > +The power device is responsible for externally power off or on the remote
> > +MediaTek SoC through the a tiny circuit block BBPU inside PMIC RTC.
> > +
> > +Required parent node:
> > +- rtc
> > + For MediaTek PMIC RTC bindings, see:
> > + Documentation/devicetree/bindings/mfd/mt6397.txt
> > +
> > +Required properties:
> > +- compatible: Should be one of follows
> > + "mediatek,mt6323-rtc-poweroff": for MT6323 PMIC
> > + "mediatek,mt6397-rtc-poweroff": for MT6397 PMIC
> > +
> > +Example:
> > +
> > + rtc {
> > + compatible = "mediatek,mt6323-rtc";
> > +
> > + power-off {
> > + compatible = "mediatek,mt6323-rtc-poweroff";
> > + };
>
> There's no need for this node. The OS can register a device in the rtc
> driver.
>
It seems a good way.
I will remove the really simple dt-binding and use a
platform_device_register_simple api embedded in rtc driver to
register the power-off device.
Hope I do not misconception your points addressed here.
> > + };
> > --
> > 2.7.4
> >
On 26/03/2018 at 12:07:45 +0800, Sean Wang wrote:
> > > - rtc->rtc_dev = rtc_device_register("mt6397-rtc", &pdev->dev,
> > > - &mtk_rtc_ops, THIS_MODULE);
> > > + rtc->rtc_dev = devm_rtc_device_register(&pdev->dev, "mt6397-rtc",
> > > + &mtk_rtc_ops, THIS_MODULE);
> >
> > You should probably switch to devm_rtc_allocate_device() and
> > rtc_register_device instead of devm_rtc_device_register.
> >
>
> Just would like to know something details
>
> It seems you just encourage me to switch into the new registration
> method and currently devm_rtc_device_register I used for the driver
> shouldn't cause any harm. right?
>
It will work but it will have to be converted to rtc_register_device
later anyway.
--
Alexandre Belloni, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
On 25/03/2018 at 03:36:28 +0800, Sean Wang wrote:
> just reply both replies in the same mail
>
> 1.) the power-off device is a part of rtc, use the same registers rtc
> has and thus it is put as child nodes under the node rtc to reflect the
> reality of characteristics the rtc has.
>
> Or am I wrong for a certain aspect in these opinions?
>
My point is that it is also part of the PMIC so it may as well be
registers from the mfd driver which already registers a bunch of devices
instead of doing unusual stuff from the rtc driver.
mt6397_rtc->regmap is mt6397_chip->regmap anyway. You have the added
benefit that if the RTC driver probe fails for some reason, you may
still be able to probe the reset driver.
I don't tink there is any benefit having it as a child of the rtc
device.
> 2) the other sub-functions for the same pmic already created its own
> dt-binding document belonged to its corresponding subsystem. Don't we
> really want to follow it them all?
>
Ok, that's fine.
--
Alexandre Belloni, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
On Tue, 2018-03-27 at 17:18 +0200, Alexandre Belloni wrote:
> On 25/03/2018 at 03:36:28 +0800, Sean Wang wrote:
> > just reply both replies in the same mail
> >
> > 1.) the power-off device is a part of rtc, use the same registers rtc
> > has and thus it is put as child nodes under the node rtc to reflect the
> > reality of characteristics the rtc has.
> >
> > Or am I wrong for a certain aspect in these opinions?
> >
>
> My point is that it is also part of the PMIC so it may as well be
> registers from the mfd driver which already registers a bunch of devices
> instead of doing unusual stuff from the rtc driver.
>
> mt6397_rtc->regmap is mt6397_chip->regmap anyway. You have the added
> benefit that if the RTC driver probe fails for some reason, you may
> still be able to probe the reset driver.
>
> I don't tink there is any benefit having it as a child of the rtc
> device.
>
really thanks! it's an optional solution I thought it 's fine and worth
doing
but so far I cannot fully make sure of whether mfd can accept two
devices holding overlay IORESOURCE_MEM.
Or do you like Rob's suggestion in [1] ? By which, I tend to embed a
sub-device with platform_device_register_data api in the rtc probe()
instead of treating it as a dt node under rtc node, but which seems
something a bit violates your preferences :(
Just confirm to know which way I should step into before I produce next
version.
[1]
http://lists.infradead.org/pipermail/linux-mediatek/2018-March/012576.html
> > 2) the other sub-functions for the same pmic already created its own
> > dt-binding document belonged to its corresponding subsystem. Don't we
> > really want to follow it them all?
> >
>
> Ok, that's fine.
>
On 28/03/2018 at 11:53:17 +0800, Sean Wang wrote:
> On Tue, 2018-03-27 at 17:18 +0200, Alexandre Belloni wrote:
> > On 25/03/2018 at 03:36:28 +0800, Sean Wang wrote:
> > > just reply both replies in the same mail
> > >
> > > 1.) the power-off device is a part of rtc, use the same registers rtc
> > > has and thus it is put as child nodes under the node rtc to reflect the
> > > reality of characteristics the rtc has.
> > >
> > > Or am I wrong for a certain aspect in these opinions?
> > >
> >
> > My point is that it is also part of the PMIC so it may as well be
> > registers from the mfd driver which already registers a bunch of devices
> > instead of doing unusual stuff from the rtc driver.
> >
> > mt6397_rtc->regmap is mt6397_chip->regmap anyway. You have the added
> > benefit that if the RTC driver probe fails for some reason, you may
> > still be able to probe the reset driver.
> >
> > I don't tink there is any benefit having it as a child of the rtc
> > device.
> >
>
>
> really thanks! it's an optional solution I thought it 's fine and worth
> doing
>
> but so far I cannot fully make sure of whether mfd can accept two
> devices holding overlay IORESOURCE_MEM.
>
There is no overlay because you are using a regmap which handles
concurrency for you.
What your patch is doing is:
struct mt6397_rtc *rtc = dev_get_drvdata(pdev->dev.parent);
then you use rtc->regmap
But in the rtc driver, you have:
struct mt6397_chip *mt6397_chip = dev_get_drvdata(pdev->dev.parent);
struct mt6397_rtc *rtc;
rtc->regmap = mt6397_chip->regmap;
So there is no benefit from being the child of the rtc, you could just
do the following in your reset driver:
struct mt6397_chip *mt6397_chip = dev_get_drvdata(pdev->dev.parent);
and then use mt6397_chip->regmap.
--
Alexandre Belloni, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
On Fri, 23 Mar 2018, [email protected] wrote:
> From: Sean Wang <[email protected]>
>
> Add a description for MT6323 RTC and link it to the detailed binding
> documentation.
>
> Signed-off-by: Sean Wang <[email protected]>
> ---
> Documentation/devicetree/bindings/mfd/mt6397.txt | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
Acked-by: Lee Jones <[email protected]>
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
On Fri, 23 Mar 2018, [email protected] wrote:
> From: Sean Wang <[email protected]>
>
> Add compatible string as "mt6323-rtc" that will make the OF core spawn
> child devices for the RTC subnode of that MT6323 MFD node.
>
> Signed-off-by: Sean Wang <[email protected]>
> ---
> drivers/mfd/mt6397-core.c | 23 ++++++++++++++++++++++-
> 1 file changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mfd/mt6397-core.c b/drivers/mfd/mt6397-core.c
> index 77b64bd..f71874a 100644
> --- a/drivers/mfd/mt6397-core.c
> +++ b/drivers/mfd/mt6397-core.c
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2014 MediaTek Inc.
> + * Copyright (c) 2014-2018 MediaTek Inc.
> * Author: Flora Fu, MediaTek
> *
> * This program is free software; you can redistribute it and/or modify
> @@ -23,6 +23,9 @@
> #include <linux/mfd/mt6397/registers.h>
> #include <linux/mfd/mt6323/registers.h>
>
> +#define MT6323_RTC_BASE 0x8000
> +#define MT6323_RTC_SIZE 0x3e
> +
> #define MT6397_RTC_BASE 0xe000
> #define MT6397_RTC_SIZE 0x3e
>
> @@ -30,6 +33,19 @@
> #define MT6391_CID_CODE 0x91
> #define MT6397_CID_CODE 0x97
>
> +static const struct resource mt6323_rtc_resources[] = {
> + {
> + .start = MT6323_RTC_BASE,
> + .end = MT6323_RTC_BASE + MT6323_RTC_SIZE,
> + .flags = IORESOURCE_MEM,
> + },
> + {
> + .start = MT6323_IRQ_STATUS_RTC,
> + .end = MT6323_IRQ_STATUS_RTC,
> + .flags = IORESOURCE_IRQ,
> + },
> +};
Please use the DEFINE_RES_* helpers instead.
Defined in: include/linux/ioport.h
> static const struct resource mt6397_rtc_resources[] = {
> {
> .start = MT6397_RTC_BASE,
> @@ -55,6 +71,11 @@ static const struct resource mt6397_keys_resources[] = {
>
> static const struct mfd_cell mt6323_devs[] = {
> {
> + .name = "mt6323-rtc",
> + .num_resources = ARRAY_SIZE(mt6323_rtc_resources),
> + .resources = mt6323_rtc_resources,
> + .of_compatible = "mediatek,mt6323-rtc",
> + }, {
> .name = "mt6323-regulator",
> .of_compatible = "mediatek,mt6323-regulator"
> }, {
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog