2018-04-25 09:36:22

by Sean Wang

[permalink] [raw]
Subject: [PATCH v2 00/17] Add support to MT6323 RTC and its power device

From: Sean Wang <[email protected]>

v2:

- The changes must be relative to the below tree since one critial patch
for the rtc-mt6397.c the series depends on was being applied into
the tree. Otherwise, for example, if the series are applied on the top
of [1], a build error must happen due to an implicit declaration of
function 'irq_create_mapping' as kbuild test robot was reporting in [2].

tree : https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git
branch : mfd/ib-mfd-input-rtc-4.18

- changes since v1:
* Define the power controller as a function of MT6323 MFD.
* Refine the whole series commit message and title.
* Add tag Reviewed-by and Acked-by got from v1 result.
* Reuse use the DEFINE_RES_* helpers in mfd driver.
* Remove an overdone patch rtc: mediatek: remove unnecessary parentheses.
* Use devm_rtc_allocate_device() and rtc_register_device instead of
devm_rtc_device_register.
* Have an improvement according to the license rule the SPDX notation
for C header file.
* Move a file from include/linux/rtc/mt6397.h to include/linux/mfd/mt6397/rtc.h
* Refine include/linux/mfd/mt6397/rtc.h

[1] https://git.kernel.org/pub/scm/linux/kernel/git/abelloni/linux.git
[2] http://lists.infradead.org/pipermail/linux-mediatek/2018-March/012548.html

v1:

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 (17):
dt-bindings: power: reset: mt6323: add bindings for MT6323 power
controller
dt-bindings: rtc: mt6397: add bindings for MediaTek PMIC based RTC
dt-bindings: mfd: mt6397: add a description for MT6323 RTC
dt-bindings: mfd: mt6397: add a description for MT6323 power
controller
mfd: mt6397: add MT6323 RTC support into MT6397 driver
mfd: mt6397: add support for MT6323 power controller into MT6397
driver
mfd: mt6397: reuse DEFINE_RES_* helpers
rtc: mt6397: add MT6323 support to RTC driver
rtc: mt6397: replace a poll with regmap_read_poll_timeout
rtc: mt6397: remove unnecessary irq_dispose_mapping
rtc: mt6397: convert to use device managed functions
rtc: mt6397: move the declaration into a globally visible header file
rtc: mt6397: cleanup header files to include
rtc: mt6397: update license converting to using SPDX identifiers
power: reset: mt6323: add a driver for MT6323 power controller
MAINTAINERS: update entry for ARM/Mediatek RTC DRIVER
MAINTAINERS: add an entry for MediaTek board level shutdown driver

Documentation/devicetree/bindings/mfd/mt6397.txt | 10 +-
.../bindings/power/reset/mt6323-poweroff.txt | 20 +++
.../devicetree/bindings/rtc/rtc-mt6397.txt | 29 ++++
MAINTAINERS | 9 ++
drivers/mfd/mt6397-core.c | 40 ++++--
drivers/power/reset/Kconfig | 10 ++
drivers/power/reset/Makefile | 1 +
drivers/power/reset/mt6323-poweroff.c | 97 ++++++++++++++
drivers/rtc/rtc-mt6397.c | 146 +++++----------------
include/linux/mfd/mt6397/rtc.h | 71 ++++++++++
10 files changed, 308 insertions(+), 125 deletions(-)
create mode 100644 Documentation/devicetree/bindings/power/reset/mt6323-poweroff.txt
create mode 100644 Documentation/devicetree/bindings/rtc/rtc-mt6397.txt
create mode 100644 drivers/power/reset/mt6323-poweroff.c
create mode 100644 include/linux/mfd/mt6397/rtc.h

--
2.7.4



2018-04-25 09:34:47

by Sean Wang

[permalink] [raw]
Subject: [PATCH v2 02/17] dt-bindings: rtc: mt6397: add bindings for MediaTek PMIC based RTC

From: Sean Wang <[email protected]>

Add device-tree binding for MediaTek PMIC based RTC

Cc: [email protected]
Signed-off-by: Sean Wang <[email protected]>
---
.../devicetree/bindings/rtc/rtc-mt6397.txt | 29 ++++++++++++++++++++++
1 file changed, 29 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..6e97248
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/rtc-mt6397.txt
@@ -0,0 +1,29 @@
+Device-Tree bindings for MediaTek PMIC based RTC
+
+MediaTek PMIC based RTC is an independent function of MediaTek PMIC that works
+as a type of multi-function device (MFD). The RTC can be configured and set up
+with PMIC wrapper bus which is a common resource shared with the other
+functions found on the same PMIC.
+
+For MediaTek PMIC MFD bindings, see:
+Documentation/devicetree/bindings/mfd/mt6397.txt
+
+For MediaTek PMIC wrapper bus bindings, see:
+Documentation/devicetree/bindings/soc/mediatek/pwrap.txt
+
+Required properties:
+- compatible: Should be one of follows
+ "mediatek,mt6323-rtc": for MT6323 PMIC
+ "mediatek,mt6397-rtc": for MT6397 PMIC
+
+Example:
+
+ pmic {
+ compatible = "mediatek,mt6323";
+
+ ...
+
+ rtc {
+ compatible = "mediatek,mt6323-rtc";
+ };
+ };
--
2.7.4


2018-04-25 09:35:01

by Sean Wang

[permalink] [raw]
Subject: [PATCH v2 06/17] mfd: mt6397: add support for MT6323 power controller into MT6397 driver

From: Sean Wang <[email protected]>

Add compatible string as "mt6323-pwrc" that will make the OF core spawn
child devices for the power-controller subnode of that MT6323 MFD device.

Signed-off-by: Sean Wang <[email protected]>
---
drivers/mfd/mt6397-core.c | 12 ++++++++++++
1 file changed, 12 insertions(+)

diff --git a/drivers/mfd/mt6397-core.c b/drivers/mfd/mt6397-core.c
index c70d5b2..c4ad5c3 100644
--- a/drivers/mfd/mt6397-core.c
+++ b/drivers/mfd/mt6397-core.c
@@ -30,6 +30,9 @@
#define MT6397_RTC_BASE 0xe000
#define MT6397_RTC_SIZE 0x3e

+#define MT6323_PWRC_BASE 0x8000
+#define MT6323_PWRC_SIZE 0x40
+
#define MT6323_CID_CODE 0x23
#define MT6391_CID_CODE 0x91
#define MT6397_CID_CODE 0x97
@@ -62,6 +65,10 @@ static const struct resource mt6397_keys_resources[] = {
DEFINE_RES_IRQ(MT6397_IRQ_HOMEKEY),
};

+static const struct resource mt6323_pwrc_resources[] = {
+ DEFINE_RES_MEM(MT6323_PWRC_BASE, MT6323_PWRC_SIZE),
+};
+
static const struct mfd_cell mt6323_devs[] = {
{
.name = "mt6323-rtc",
@@ -79,6 +86,11 @@ static const struct mfd_cell mt6323_devs[] = {
.num_resources = ARRAY_SIZE(mt6323_keys_resources),
.resources = mt6323_keys_resources,
.of_compatible = "mediatek,mt6323-keys"
+ }, {
+ .name = "mt6323-pwrc",
+ .num_resources = ARRAY_SIZE(mt6323_pwrc_resources),
+ .resources = mt6323_pwrc_resources,
+ .of_compatible = "mediatek,mt6323-pwrc"
},
};

--
2.7.4


2018-04-25 09:35:18

by Sean Wang

[permalink] [raw]
Subject: [PATCH v2 16/17] MAINTAINERS: update entry for ARM/Mediatek RTC DRIVER

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 0a1410d..a7ef37f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1607,9 +1607,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/mfd/mt6397/rtc.h

ARM/Mediatek SoC support
M: Matthias Brugger <[email protected]>
--
2.7.4


2018-04-25 09:35:28

by Sean Wang

[permalink] [raw]
Subject: [PATCH v2 01/17] dt-bindings: power: reset: mt6323: add bindings for MT6323 power controller

From: Sean Wang <[email protected]>

Adding device-tree binding for the power controller which is a tiny
circuit block present as a part of MT6323 PMIC and is responsible for
externally powering off or on a remote SoC the PMIC is connected to.

Cc: [email protected]
Signed-off-by: Sean Wang <[email protected]>
---
.../bindings/power/reset/mt6323-poweroff.txt | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
create mode 100644 Documentation/devicetree/bindings/power/reset/mt6323-poweroff.txt

diff --git a/Documentation/devicetree/bindings/power/reset/mt6323-poweroff.txt b/Documentation/devicetree/bindings/power/reset/mt6323-poweroff.txt
new file mode 100644
index 0000000..6f7c590
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/reset/mt6323-poweroff.txt
@@ -0,0 +1,20 @@
+Device Tree Bindings for Power Controller on MediaTek PMIC
+
+The power controller which could be found on PMIC is responsible for externally
+powering off or on the remote MediaTek SoC through the circuit BBPU.
+
+Required properties:
+- compatible: Should be one of follows
+ "mediatek,mt6323-pwrc": for MT6323 PMIC
+
+Example:
+
+ pmic {
+ compatible = "mediatek,mt6323";
+
+ ...
+
+ power-controller {
+ compatible = "mediatek,mt6323-pwrc";
+ };
+ }
--
2.7.4


2018-04-25 09:36:10

by Sean Wang

[permalink] [raw]
Subject: [PATCH v2 14/17] rtc: mt6397: update license converting to using SPDX identifiers

From: Sean Wang <[email protected]>

update the license and 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 3672d622..1ebeaaf 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


2018-04-25 09:36:15

by Sean Wang

[permalink] [raw]
Subject: [PATCH v2 17/17] MAINTAINERS: add an entry for MediaTek board level shutdown driver

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 a7ef37f..816554b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8912,6 +8912,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/mt6323-poweroff.txt
+F: drivers/power/reset/mt6323-poweroff.c
+
MEDIATEK JPEG DRIVER
M: Rick Chang <[email protected]>
M: Bin Liu <[email protected]>
--
2.7.4


2018-04-25 09:36:52

by Sean Wang

[permalink] [raw]
Subject: [PATCH v2 12/17] rtc: mt6397: move the declaration into a globally visible header file

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/mfd/mt6397/rtc.h | 70 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 71 insertions(+), 52 deletions(-)
create mode 100644 include/linux/mfd/mt6397/rtc.h

diff --git a/drivers/rtc/rtc-mt6397.c b/drivers/rtc/rtc-mt6397.c
index 91d584a..4c5cdaa 100644
--- a/drivers/rtc/rtc-mt6397.c
+++ b/drivers/rtc/rtc-mt6397.c
@@ -18,63 +18,12 @@
#include <linux/module.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/mfd/mt6397/rtc.h>

static int mtk_rtc_write_trigger(struct mt6397_rtc *rtc)
{
diff --git a/include/linux/mfd/mt6397/rtc.h b/include/linux/mfd/mt6397/rtc.h
new file mode 100644
index 0000000..480977c
--- /dev/null
+++ b/include/linux/mfd/mt6397/rtc.h
@@ -0,0 +1,70 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2014-2018 MediaTek Inc.
+ *
+ * Author: Tianping.Fang <[email protected]>
+ * Sean Wang <[email protected]>
+ */
+
+#ifndef _LINUX_MFD_MT6397_RTC_H_
+#define _LINUX_MFD_MT6397_RTC_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 register access from multiple tasks */
+ struct mutex lock;
+ struct regmap *regmap;
+ int irq;
+ u32 addr_base;
+};
+
+#endif /* _LINUX_MFD_MT6397_RTC_H_ */
--
2.7.4


2018-04-25 09:37:03

by Sean Wang

[permalink] [raw]
Subject: [PATCH v2 10/17] rtc: mt6397: remove unnecessary irq_dispose_mapping

From: Sean Wang <[email protected]>

With the patch [1], the job irq_create_mapping had been moved from the
rtc driver to mfd core, so it should be unnecessary to keep
irq_dispose_mapping in all paths of the leaf driver.

[1] http://lists.infradead.org/pipermail/linux-mediatek/2017-September/010455.html

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 efb9370..398bae5 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


2018-04-25 09:37:14

by Sean Wang

[permalink] [raw]
Subject: [PATCH v2 08/17] rtc: mt6397: add MT6323 support to RTC driver

From: Sean Wang <[email protected]>

Just to add MT6323 support to existing 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


2018-04-25 09:37:22

by Sean Wang

[permalink] [raw]
Subject: [PATCH v2 13/17] rtc: mt6397: cleanup header files to include

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 4c5cdaa..3672d622 100644
--- a/drivers/rtc/rtc-mt6397.c
+++ b/drivers/rtc/rtc-mt6397.c
@@ -12,17 +12,14 @@
* 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/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/mfd/mt6397/rtc.h>

static int mtk_rtc_write_trigger(struct mt6397_rtc *rtc)
--
2.7.4


2018-04-25 09:37:41

by Sean Wang

[permalink] [raw]
Subject: [PATCH v2 07/17] mfd: mt6397: reuse DEFINE_RES_* helpers

From: Sean Wang <[email protected]>

Reuse DEFINE_RES_* helpers instead of an open-coded handling.

Signed-off-by: Sean Wang <[email protected]>
---
drivers/mfd/mt6397-core.c | 12 ++----------
1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/mfd/mt6397-core.c b/drivers/mfd/mt6397-core.c
index c4ad5c3..e0012e2 100644
--- a/drivers/mfd/mt6397-core.c
+++ b/drivers/mfd/mt6397-core.c
@@ -43,16 +43,8 @@ static const struct resource mt6323_rtc_resources[] = {
};

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,
- },
+ DEFINE_RES_MEM(MT6397_RTC_BASE, MT6397_RTC_SIZE),
+ DEFINE_RES_IRQ(MT6397_IRQ_RTC),
};

static const struct resource mt6323_keys_resources[] = {
--
2.7.4


2018-04-25 09:37:49

by Sean Wang

[permalink] [raw]
Subject: [PATCH v2 11/17] rtc: mt6397: convert to use device managed functions

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.

v1 -> v2:
- Use devm_rtc_allocate_device() and rtc_register_device instead of
devm_rtc_device_register.

Signed-off-by: Sean Wang <[email protected]>
---
drivers/rtc/rtc-mt6397.c | 37 ++++++++++---------------------------
1 file changed, 10 insertions(+), 27 deletions(-)

diff --git a/drivers/rtc/rtc-mt6397.c b/drivers/rtc/rtc-mt6397.c
index 398bae5..91d584a 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,13 @@ 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);
- 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 0;
-
-out_free_irq:
- free_irq(rtc->irq, rtc->rtc_dev);
-
- return ret;
-}
+ rtc->rtc_dev = devm_rtc_allocate_device(&pdev->dev);
+ if (IS_ERR(rtc->rtc_dev))
+ return PTR_ERR(rtc->rtc_dev);

-static int mtk_rtc_remove(struct platform_device *pdev)
-{
- struct mt6397_rtc *rtc = platform_get_drvdata(pdev);
+ rtc->rtc_dev->ops = &mtk_rtc_ops;

- rtc_device_unregister(rtc->rtc_dev);
- free_irq(rtc->irq, rtc->rtc_dev);
-
- return 0;
+ return rtc_register_device(rtc->rtc_dev);
}

#ifdef CONFIG_PM_SLEEP
@@ -405,7 +389,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


2018-04-25 09:37:56

by Sean Wang

[permalink] [raw]
Subject: [PATCH v2 03/17] dt-bindings: mfd: mt6397: add a description for MT6323 RTC

From: Sean Wang <[email protected]>

Add a description for MT6323 RTC and link it to the detailed binding
documentation.

Cc: [email protected]
Signed-off-by: Sean Wang <[email protected]>
Reviewed-by: Rob Herring <[email protected]>
Acked-by: Lee Jones <[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


2018-04-25 09:38:25

by Sean Wang

[permalink] [raw]
Subject: [PATCH v2 04/17] dt-bindings: mfd: mt6397: add a description for MT6323 power controller

From: Sean Wang <[email protected]>

Add a description for MT6323 power controller on MT6323 and link it to the
detailed binding documentation.

Cc: [email protected]
Signed-off-by: Sean Wang <[email protected]>
---
Documentation/devicetree/bindings/mfd/mt6397.txt | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/mt6397.txt b/Documentation/devicetree/bindings/mfd/mt6397.txt
index 11a748d..61115ea 100644
--- a/Documentation/devicetree/bindings/mfd/mt6397.txt
+++ b/Documentation/devicetree/bindings/mfd/mt6397.txt
@@ -8,6 +8,7 @@ MT6397/MT6323 is a multifunction device with the following sub modules:
- Clock
- LED
- Keys
+- Power controller

It is interfaced to host controller using SPI interface by a proprietary hardware
called PMIC wrapper or pwrap. MT6397/MT6323 MFD is a child device of pwrap.
@@ -48,6 +49,11 @@ Optional subnodes:
- compatible: "mediatek,mt6397-keys" or "mediatek,mt6323-keys"
see Documentation/devicetree/bindings/input/mtk-pmic-keys.txt

+- power-controller
+ Required properties:
+ - compatible: "mediatek,mt6323-pwrc"
+ For details, see Documentation/devicetree/bindings/power/reset/mt6323-poweroff.txt
+
Example:
pwrap: pwrap@1000f000 {
compatible = "mediatek,mt8135-pwrap";
--
2.7.4


2018-04-25 09:38:55

by Sean Wang

[permalink] [raw]
Subject: [PATCH v2 15/17] power: reset: mt6323: add a driver for MT6323 power controller

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 than to be a part of existent RTC driver so as to let us make
concentration on works about power-controlling topic and to obtain
improvements while the subsystem power/reset 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 | 10 ++++
drivers/power/reset/Makefile | 1 +
drivers/power/reset/mt6323-poweroff.c | 97 +++++++++++++++++++++++++++++++++++
include/linux/mfd/mt6397/rtc.h | 1 +
4 files changed, 109 insertions(+)
create mode 100644 drivers/power/reset/mt6323-poweroff.c

diff --git a/drivers/power/reset/Kconfig b/drivers/power/reset/Kconfig
index df58fc8..7b49d71 100644
--- a/drivers/power/reset/Kconfig
+++ b/drivers/power/reset/Kconfig
@@ -128,6 +128,16 @@ 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_MT6323
+ bool "MediaTek MT6323 power-off driver"
+ depends on MFD_MT6397
+ help
+ The power-off driver is responsible for externally shutdown down
+ the power of a remote MediaTek SoC MT6323 is connected to through
+ controlling a tiny circuit BBPU inside MT6323 RTC.
+
+ Say Y if you have a board where MT6323 could be found.
+
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 7778c74..8836172 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_MT6323) += mt6323-poweroff.o
obj-$(CONFIG_POWER_RESET_OCELOT_RESET) += ocelot-reset.o
obj-$(CONFIG_POWER_RESET_PIIX4_POWEROFF) += piix4-poweroff.o
obj-$(CONFIG_POWER_RESET_LTC2952) += ltc2952-poweroff.o
diff --git a/drivers/power/reset/mt6323-poweroff.c b/drivers/power/reset/mt6323-poweroff.c
new file mode 100644
index 0000000..c195766
--- /dev/null
+++ b/drivers/power/reset/mt6323-poweroff.c
@@ -0,0 +1,97 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Power off through MediaTek PMIC
+ *
+ * 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/mfd/mt6397/core.h>
+#include <linux/mfd/mt6397/rtc.h>
+
+struct mt6323_pwrc {
+ struct device *dev;
+ struct regmap *regmap;
+ u32 base;
+};
+
+static struct mt6323_pwrc *mt_pwrc;
+
+static void mt6323_do_pwroff(void)
+{
+ struct mt6323_pwrc *pwrc = mt_pwrc;
+ unsigned int val;
+ int ret;
+
+ regmap_write(pwrc->regmap, pwrc->base + RTC_BBPU, RTC_BBPU_KEY);
+ regmap_write(pwrc->regmap, pwrc->base + RTC_WRTGR, 1);
+
+ ret = regmap_read_poll_timeout(pwrc->regmap,
+ pwrc->base + RTC_BBPU, val,
+ !(val & RTC_BBPU_CBUSY),
+ MTK_RTC_POLL_DELAY_US,
+ MTK_RTC_POLL_TIMEOUT);
+ if (ret)
+ dev_err(pwrc->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 power off system\n");
+}
+
+static int mt6323_pwrc_probe(struct platform_device *pdev)
+{
+ struct mt6397_chip *mt6397_chip = dev_get_drvdata(pdev->dev.parent);
+ struct mt6323_pwrc *pwrc;
+ struct resource *res;
+
+ pwrc = devm_kzalloc(&pdev->dev, sizeof(*pwrc), GFP_KERNEL);
+ if (!pwrc)
+ return -ENOMEM;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ pwrc->base = res->start;
+ pwrc->regmap = mt6397_chip->regmap;
+ pwrc->dev = &pdev->dev;
+ mt_pwrc = pwrc;
+
+ pm_power_off = &mt6323_do_pwroff;
+
+ return 0;
+}
+
+static int mt6323_pwrc_remove(struct platform_device *pdev)
+{
+ if (pm_power_off == &mt6323_do_pwroff)
+ pm_power_off = NULL;
+
+ return 0;
+}
+
+static const struct of_device_id mt6323_pwrc_dt_match[] = {
+ { .compatible = "mediatek,mt6323-pwrc" },
+ {},
+};
+MODULE_DEVICE_TABLE(of, mt6323_pwrc_dt_match);
+
+static struct platform_driver mt6323_pwrc_driver = {
+ .probe = mt6323_pwrc_probe,
+ .remove = mt6323_pwrc_remove,
+ .driver = {
+ .name = "mt6323-pwrc",
+ .of_match_table = mt6323_pwrc_dt_match,
+ },
+};
+
+module_platform_driver(mt6323_pwrc_driver);
+
+MODULE_DESCRIPTION("Poweroff driver for MT6323 PMIC");
+MODULE_AUTHOR("Sean Wang <[email protected]>");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/mfd/mt6397/rtc.h b/include/linux/mfd/mt6397/rtc.h
index 480977c..ac932c9 100644
--- a/include/linux/mfd/mt6397/rtc.h
+++ b/include/linux/mfd/mt6397/rtc.h
@@ -16,6 +16,7 @@

#define RTC_BBPU 0x0000
#define RTC_BBPU_CBUSY BIT(6)
+#define RTC_BBPU_KEY (0x43 << 8)

#define RTC_WRTGR 0x003c

--
2.7.4


2018-04-25 09:38:56

by Sean Wang

[permalink] [raw]
Subject: [PATCH v2 05/17] mfd: mt6397: add MT6323 RTC support into MT6397 driver

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 device.

Signed-off-by: Sean Wang <[email protected]>
---
drivers/mfd/mt6397-core.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/mfd/mt6397-core.c b/drivers/mfd/mt6397-core.c
index 77b64bd..c70d5b2 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
@@ -13,6 +13,7 @@
*/

#include <linux/interrupt.h>
+#include <linux/ioport.h>
#include <linux/module.h>
#include <linux/of_device.h>
#include <linux/of_irq.h>
@@ -23,6 +24,9 @@
#include <linux/mfd/mt6397/registers.h>
#include <linux/mfd/mt6323/registers.h>

+#define MT6323_RTC_BASE 0x8000
+#define MT6323_RTC_SIZE 0x40
+
#define MT6397_RTC_BASE 0xe000
#define MT6397_RTC_SIZE 0x3e

@@ -30,6 +34,11 @@
#define MT6391_CID_CODE 0x91
#define MT6397_CID_CODE 0x97

+static const struct resource mt6323_rtc_resources[] = {
+ DEFINE_RES_MEM(MT6323_RTC_BASE, MT6323_RTC_SIZE),
+ DEFINE_RES_IRQ(MT6323_IRQ_STATUS_RTC),
+};
+
static const struct resource mt6397_rtc_resources[] = {
{
.start = MT6397_RTC_BASE,
@@ -55,6 +64,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


2018-04-25 09:39:05

by Sean Wang

[permalink] [raw]
Subject: [PATCH v2 09/17] rtc: mt6397: replace a poll with regmap_read_poll_timeout

From: Sean Wang <[email protected]>

Reuse the helper regmap_read_poll_timeout instead of an open-coding
handling 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 0df7ccd..efb9370 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 < 0)
+ dev_err(rtc->dev, "failed to write WRTGE: %d\n", ret);

return ret;
}
--
2.7.4


2018-04-27 20:03:49

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v2 01/17] dt-bindings: power: reset: mt6323: add bindings for MT6323 power controller

On Wed, Apr 25, 2018 at 05:32:27PM +0800, [email protected] wrote:
> From: Sean Wang <[email protected]>
>
> Adding device-tree binding for the power controller which is a tiny
> circuit block present as a part of MT6323 PMIC and is responsible for
> externally powering off or on a remote SoC the PMIC is connected to.
>
> Cc: [email protected]
> Signed-off-by: Sean Wang <[email protected]>
> ---
> .../bindings/power/reset/mt6323-poweroff.txt | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/power/reset/mt6323-poweroff.txt
>
> diff --git a/Documentation/devicetree/bindings/power/reset/mt6323-poweroff.txt b/Documentation/devicetree/bindings/power/reset/mt6323-poweroff.txt
> new file mode 100644
> index 0000000..6f7c590
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/reset/mt6323-poweroff.txt
> @@ -0,0 +1,20 @@
> +Device Tree Bindings for Power Controller on MediaTek PMIC
> +
> +The power controller which could be found on PMIC is responsible for externally
> +powering off or on the remote MediaTek SoC through the circuit BBPU.
> +
> +Required properties:
> +- compatible: Should be one of follows
> + "mediatek,mt6323-pwrc": for MT6323 PMIC
> +
> +Example:
> +
> + pmic {
> + compatible = "mediatek,mt6323";
> +
> + ...
> +
> + power-controller {
> + compatible = "mediatek,mt6323-pwrc";

Why do you need this in DT? It doesn't define any resources. The parent
can just as well register a reset or poweroff handler.

Rob

> + };
> + }
> --
> 2.7.4
>

2018-04-30 07:36:26

by Sean Wang

[permalink] [raw]
Subject: Re: [PATCH v2 01/17] dt-bindings: power: reset: mt6323: add bindings for MT6323 power controller

On Fri, 2018-04-27 at 15:02 -0500, Rob Herring wrote:
> On Wed, Apr 25, 2018 at 05:32:27PM +0800, [email protected] wrote:
> > From: Sean Wang <[email protected]>
> >
> > Adding device-tree binding for the power controller which is a tiny
> > circuit block present as a part of MT6323 PMIC and is responsible for
> > externally powering off or on a remote SoC the PMIC is connected to.
> >
> > Cc: [email protected]
> > Signed-off-by: Sean Wang <[email protected]>
> > ---
> > .../bindings/power/reset/mt6323-poweroff.txt | 20 ++++++++++++++++++++
> > 1 file changed, 20 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/power/reset/mt6323-poweroff.txt
> >
> > diff --git a/Documentation/devicetree/bindings/power/reset/mt6323-poweroff.txt b/Documentation/devicetree/bindings/power/reset/mt6323-poweroff.txt
> > new file mode 100644
> > index 0000000..6f7c590
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/power/reset/mt6323-poweroff.txt
> > @@ -0,0 +1,20 @@
> > +Device Tree Bindings for Power Controller on MediaTek PMIC
> > +
> > +The power controller which could be found on PMIC is responsible for externally
> > +powering off or on the remote MediaTek SoC through the circuit BBPU.
> > +
> > +Required properties:
> > +- compatible: Should be one of follows
> > + "mediatek,mt6323-pwrc": for MT6323 PMIC
> > +
> > +Example:
> > +
> > + pmic {
> > + compatible = "mediatek,mt6323";
> > +
> > + ...
> > +
> > + power-controller {
> > + compatible = "mediatek,mt6323-pwrc";
>
> Why do you need this in DT? It doesn't define any resources. The parent
> can just as well register a reset or poweroff handler.
>
> Rob
>

Yes, those binding can be removed.

I tend to use platform_device_register_data embedded in mfd driver
to probe the existent poweroff driver in the next version.

> > + };
> > + }
> > --
> > 2.7.4
> >



2018-05-01 12:57:43

by Sebastian Reichel

[permalink] [raw]
Subject: Re: [PATCH v2 15/17] power: reset: mt6323: add a driver for MT6323 power controller

Hi,

On Wed, Apr 25, 2018 at 05:32:41PM +0800, [email protected] wrote:
> 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 than to be a part of existent RTC driver so as to let us make
> concentration on works about power-controlling topic and to obtain
> improvements while the subsystem power/reset constantly growing.

I don't understand the above sentence. (I'm fine with adding the
driver)

> 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 | 10 ++++
> drivers/power/reset/Makefile | 1 +
> drivers/power/reset/mt6323-poweroff.c | 97 +++++++++++++++++++++++++++++++++++
> include/linux/mfd/mt6397/rtc.h | 1 +
> 4 files changed, 109 insertions(+)
> create mode 100644 drivers/power/reset/mt6323-poweroff.c
>
> diff --git a/drivers/power/reset/Kconfig b/drivers/power/reset/Kconfig
> index df58fc8..7b49d71 100644
> --- a/drivers/power/reset/Kconfig
> +++ b/drivers/power/reset/Kconfig
> @@ -128,6 +128,16 @@ 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_MT6323
> + bool "MediaTek MT6323 power-off driver"
> + depends on MFD_MT6397
> + help
> + The power-off driver is responsible for externally shutdown down
> + the power of a remote MediaTek SoC MT6323 is connected to through
> + controlling a tiny circuit BBPU inside MT6323 RTC.
> +
> + Say Y if you have a board where MT6323 could be found.
> +
> 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 7778c74..8836172 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_MT6323) += mt6323-poweroff.o
> obj-$(CONFIG_POWER_RESET_OCELOT_RESET) += ocelot-reset.o
> obj-$(CONFIG_POWER_RESET_PIIX4_POWEROFF) += piix4-poweroff.o
> obj-$(CONFIG_POWER_RESET_LTC2952) += ltc2952-poweroff.o
> diff --git a/drivers/power/reset/mt6323-poweroff.c b/drivers/power/reset/mt6323-poweroff.c
> new file mode 100644
> index 0000000..c195766
> --- /dev/null
> +++ b/drivers/power/reset/mt6323-poweroff.c
> @@ -0,0 +1,97 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Power off through MediaTek PMIC
> + *
> + * Copyright (C) 2018 MediaTek Inc.
> + *
> + * Author: Sean Wang <[email protected]>
> + *
> + */
> +
> +#include <linux/err.h>
> +#include <linux/module.h>
> +#include <linux/of.h>

Not needed

> +#include <linux/platform_device.h>
> +#include <linux/mfd/mt6397/core.h>
> +#include <linux/mfd/mt6397/rtc.h>
> +
> +struct mt6323_pwrc {
> + struct device *dev;
> + struct regmap *regmap;
> + u32 base;
> +};
> +
> +static struct mt6323_pwrc *mt_pwrc;
> +
> +static void mt6323_do_pwroff(void)
> +{
> + struct mt6323_pwrc *pwrc = mt_pwrc;
> + unsigned int val;
> + int ret;
> +
> + regmap_write(pwrc->regmap, pwrc->base + RTC_BBPU, RTC_BBPU_KEY);
> + regmap_write(pwrc->regmap, pwrc->base + RTC_WRTGR, 1);
> +
> + ret = regmap_read_poll_timeout(pwrc->regmap,
> + pwrc->base + RTC_BBPU, val,
> + !(val & RTC_BBPU_CBUSY),
> + MTK_RTC_POLL_DELAY_US,
> + MTK_RTC_POLL_TIMEOUT);
> + if (ret)
> + dev_err(pwrc->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 power off system\n");
> +}
> +
> +static int mt6323_pwrc_probe(struct platform_device *pdev)
> +{
> + struct mt6397_chip *mt6397_chip = dev_get_drvdata(pdev->dev.parent);
> + struct mt6323_pwrc *pwrc;
> + struct resource *res;
> +
> + pwrc = devm_kzalloc(&pdev->dev, sizeof(*pwrc), GFP_KERNEL);
> + if (!pwrc)
> + return -ENOMEM;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + pwrc->base = res->start;
> + pwrc->regmap = mt6397_chip->regmap;
> + pwrc->dev = &pdev->dev;
> + mt_pwrc = pwrc;
> +
> + pm_power_off = &mt6323_do_pwroff;
> +
> + return 0;
> +}
> +
> +static int mt6323_pwrc_remove(struct platform_device *pdev)
> +{
> + if (pm_power_off == &mt6323_do_pwroff)
> + pm_power_off = NULL;
> +
> + return 0;
> +}
> +
> +static const struct of_device_id mt6323_pwrc_dt_match[] = {
> + { .compatible = "mediatek,mt6323-pwrc" },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, mt6323_pwrc_dt_match);

Don't forget to remove the DT table when you respin the series for
the requested binding changes.

> +static struct platform_driver mt6323_pwrc_driver = {
> + .probe = mt6323_pwrc_probe,
> + .remove = mt6323_pwrc_remove,
> + .driver = {
> + .name = "mt6323-pwrc",
> + .of_match_table = mt6323_pwrc_dt_match,
> + },
> +};
> +
> +module_platform_driver(mt6323_pwrc_driver);
> +
> +MODULE_DESCRIPTION("Poweroff driver for MT6323 PMIC");
> +MODULE_AUTHOR("Sean Wang <[email protected]>");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/mfd/mt6397/rtc.h b/include/linux/mfd/mt6397/rtc.h
> index 480977c..ac932c9 100644
> --- a/include/linux/mfd/mt6397/rtc.h
> +++ b/include/linux/mfd/mt6397/rtc.h
> @@ -16,6 +16,7 @@
>
> #define RTC_BBPU 0x0000
> #define RTC_BBPU_CBUSY BIT(6)
> +#define RTC_BBPU_KEY (0x43 << 8)
>
> #define RTC_WRTGR 0x003c

The driver looks fine otherwise.

-- Sebastian


Attachments:
(No filename) (6.02 kB)
signature.asc (849.00 B)
Download all attachments

2018-05-02 02:31:49

by Sean Wang

[permalink] [raw]
Subject: Re: Re: [PATCH v2 15/17] power: reset: mt6323: add a driver for MT6323 power controller

On Tue, 2018-05-01 at 14:56 +0200, Sebastian Reichel wrote:
> Hi,
>
> On Wed, Apr 25, 2018 at 05:32:41PM +0800, [email protected] wrote:
> > 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 than to be a part of existent RTC driver so as to let us make
> > concentration on works about power-controlling topic and to obtain
> > improvements while the subsystem power/reset constantly growing.
>
> I don't understand the above sentence. (I'm fine with adding the
> driver)

Which I was actually meaning is that taking the poweroff driver as the
standalone one and placing it under driver/reset/power seems can simply
get a help from its subsystem either for function extension or bugfix in
the future. I thought that way is better than just an embedded one
somewhere in rtc driver or other drivers.

> > 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 | 10 ++++
> > drivers/power/reset/Makefile | 1 +
> > drivers/power/reset/mt6323-poweroff.c | 97 +++++++++++++++++++++++++++++++++++
> > include/linux/mfd/mt6397/rtc.h | 1 +
> > 4 files changed, 109 insertions(+)
> > create mode 100644 drivers/power/reset/mt6323-poweroff.c
> >
> > diff --git a/drivers/power/reset/Kconfig b/drivers/power/reset/Kconfig
> > index df58fc8..7b49d71 100644
> > --- a/drivers/power/reset/Kconfig
> > +++ b/drivers/power/reset/Kconfig
> > @@ -128,6 +128,16 @@ 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_MT6323
> > + bool "MediaTek MT6323 power-off driver"
> > + depends on MFD_MT6397
> > + help
> > + The power-off driver is responsible for externally shutdown down
> > + the power of a remote MediaTek SoC MT6323 is connected to through
> > + controlling a tiny circuit BBPU inside MT6323 RTC.
> > +
> > + Say Y if you have a board where MT6323 could be found.
> > +
> > 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 7778c74..8836172 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_MT6323) += mt6323-poweroff.o
> > obj-$(CONFIG_POWER_RESET_OCELOT_RESET) += ocelot-reset.o
> > obj-$(CONFIG_POWER_RESET_PIIX4_POWEROFF) += piix4-poweroff.o
> > obj-$(CONFIG_POWER_RESET_LTC2952) += ltc2952-poweroff.o
> > diff --git a/drivers/power/reset/mt6323-poweroff.c b/drivers/power/reset/mt6323-poweroff.c
> > new file mode 100644
> > index 0000000..c195766
> > --- /dev/null
> > +++ b/drivers/power/reset/mt6323-poweroff.c
> > @@ -0,0 +1,97 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Power off through MediaTek PMIC
> > + *
> > + * Copyright (C) 2018 MediaTek Inc.
> > + *
> > + * Author: Sean Wang <[email protected]>
> > + *
> > + */
> > +
> > +#include <linux/err.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
>
> Not needed

Will be removed.

>
> > +#include <linux/platform_device.h>
> > +#include <linux/mfd/mt6397/core.h>
> > +#include <linux/mfd/mt6397/rtc.h>
> > +
> > +struct mt6323_pwrc {
> > + struct device *dev;
> > + struct regmap *regmap;
> > + u32 base;
> > +};
> > +
> > +static struct mt6323_pwrc *mt_pwrc;
> > +
> > +static void mt6323_do_pwroff(void)
> > +{
> > + struct mt6323_pwrc *pwrc = mt_pwrc;
> > + unsigned int val;
> > + int ret;
> > +
> > + regmap_write(pwrc->regmap, pwrc->base + RTC_BBPU, RTC_BBPU_KEY);
> > + regmap_write(pwrc->regmap, pwrc->base + RTC_WRTGR, 1);
> > +
> > + ret = regmap_read_poll_timeout(pwrc->regmap,
> > + pwrc->base + RTC_BBPU, val,
> > + !(val & RTC_BBPU_CBUSY),
> > + MTK_RTC_POLL_DELAY_US,
> > + MTK_RTC_POLL_TIMEOUT);
> > + if (ret)
> > + dev_err(pwrc->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 power off system\n");
> > +}
> > +
> > +static int mt6323_pwrc_probe(struct platform_device *pdev)
> > +{
> > + struct mt6397_chip *mt6397_chip = dev_get_drvdata(pdev->dev.parent);
> > + struct mt6323_pwrc *pwrc;
> > + struct resource *res;
> > +
> > + pwrc = devm_kzalloc(&pdev->dev, sizeof(*pwrc), GFP_KERNEL);
> > + if (!pwrc)
> > + return -ENOMEM;
> > +
> > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > + pwrc->base = res->start;
> > + pwrc->regmap = mt6397_chip->regmap;
> > + pwrc->dev = &pdev->dev;
> > + mt_pwrc = pwrc;
> > +
> > + pm_power_off = &mt6323_do_pwroff;
> > +
> > + return 0;
> > +}
> > +
> > +static int mt6323_pwrc_remove(struct platform_device *pdev)
> > +{
> > + if (pm_power_off == &mt6323_do_pwroff)
> > + pm_power_off = NULL;
> > +
> > + return 0;
> > +}
> > +
> > +static const struct of_device_id mt6323_pwrc_dt_match[] = {
> > + { .compatible = "mediatek,mt6323-pwrc" },
> > + {},
> > +};
> > +MODULE_DEVICE_TABLE(of, mt6323_pwrc_dt_match);
>
> Don't forget to remove the DT table when you respin the series for
> the requested binding changes.
>

Thanks. This table will be removed in the next version.


> > +static struct platform_driver mt6323_pwrc_driver = {
> > + .probe = mt6323_pwrc_probe,
> > + .remove = mt6323_pwrc_remove,
> > + .driver = {
> > + .name = "mt6323-pwrc",
> > + .of_match_table = mt6323_pwrc_dt_match,
> > + },
> > +};
> > +
> > +module_platform_driver(mt6323_pwrc_driver);
> > +
> > +MODULE_DESCRIPTION("Poweroff driver for MT6323 PMIC");
> > +MODULE_AUTHOR("Sean Wang <[email protected]>");
> > +MODULE_LICENSE("GPL v2");
> > diff --git a/include/linux/mfd/mt6397/rtc.h b/include/linux/mfd/mt6397/rtc.h
> > index 480977c..ac932c9 100644
> > --- a/include/linux/mfd/mt6397/rtc.h
> > +++ b/include/linux/mfd/mt6397/rtc.h
> > @@ -16,6 +16,7 @@
> >
> > #define RTC_BBPU 0x0000
> > #define RTC_BBPU_CBUSY BIT(6)
> > +#define RTC_BBPU_KEY (0x43 << 8)
> >
> > #define RTC_WRTGR 0x003c
>
> The driver looks fine otherwise.
>

Thanks.

> -- Sebastian
> _______________________________________________
> Linux-mediatek mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-mediatek



2018-05-08 15:40:23

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v2 02/17] dt-bindings: rtc: mt6397: add bindings for MediaTek PMIC based RTC

On Wed, Apr 25, 2018 at 05:32:28PM +0800, [email protected] wrote:
> From: Sean Wang <[email protected]>
>
> Add device-tree binding for MediaTek PMIC based RTC
>
> Cc: [email protected]
> Signed-off-by: Sean Wang <[email protected]>
> ---
> .../devicetree/bindings/rtc/rtc-mt6397.txt | 29 ++++++++++++++++++++++
> 1 file changed, 29 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/rtc/rtc-mt6397.txt

Reviewed-by: Rob Herring <[email protected]>