2016-03-05 22:46:29

by Paul Cercueil

[permalink] [raw]
Subject: [PATCH 1/5] rtc: rtc-jz4740: Add support for the RTC in the jz4780 SoC

Signed-off-by: Paul Cercueil <[email protected]>
---
drivers/rtc/Kconfig | 6 +++---
drivers/rtc/rtc-jz4740.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++--
2 files changed, 51 insertions(+), 5 deletions(-)

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index e593c55..b322f08 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -1494,10 +1494,10 @@ config RTC_DRV_MPC5121

config RTC_DRV_JZ4740
tristate "Ingenic JZ4740 SoC"
- depends on MACH_JZ4740 || COMPILE_TEST
+ depends on MACH_INGENIC || COMPILE_TEST
help
- If you say yes here you get support for the Ingenic JZ4740 SoC RTC
- controller.
+ If you say yes here you get support for the Ingenic JZ47xx SoCs RTC
+ controllers.

This driver can also be buillt as a module. If so, the module
will be called rtc-jz4740.
diff --git a/drivers/rtc/rtc-jz4740.c b/drivers/rtc/rtc-jz4740.c
index b2bcfc0..47617bd 100644
--- a/drivers/rtc/rtc-jz4740.c
+++ b/drivers/rtc/rtc-jz4740.c
@@ -29,6 +29,10 @@
#define JZ_REG_RTC_HIBERNATE 0x20
#define JZ_REG_RTC_SCRATCHPAD 0x34

+/* The following are present on the jz4780 */
+#define JZ_REG_RTC_WENR 0x3C
+#define JZ_RTC_WENR_WEN BIT(31)
+
#define JZ_RTC_CTRL_WRDY BIT(7)
#define JZ_RTC_CTRL_1HZ BIT(6)
#define JZ_RTC_CTRL_1HZ_IRQ BIT(5)
@@ -37,8 +41,17 @@
#define JZ_RTC_CTRL_AE BIT(2)
#define JZ_RTC_CTRL_ENABLE BIT(0)

+/* Magic value to enable writes on jz4780 */
+#define JZ_RTC_WENR_MAGIC 0xA55A
+
+enum jz4740_rtc_type {
+ ID_JZ4740,
+ ID_JZ4780,
+};
+
struct jz4740_rtc {
void __iomem *base;
+ enum jz4740_rtc_type type;

struct rtc_device *rtc;

@@ -64,11 +77,33 @@ static int jz4740_rtc_wait_write_ready(struct jz4740_rtc *rtc)
return timeout ? 0 : -EIO;
}

+static inline int jz4780_rtc_enable_write(struct jz4740_rtc *rtc)
+{
+ uint32_t ctrl;
+ int ret, timeout = 1000;
+
+ ret = jz4740_rtc_wait_write_ready(rtc);
+ if (ret != 0)
+ return ret;
+
+ writel(JZ_RTC_WENR_MAGIC, rtc->base + JZ_REG_RTC_WENR);
+
+ do {
+ ctrl = readl(rtc->base + JZ_REG_RTC_WENR);
+ } while (!(ctrl & JZ_RTC_WENR_WEN) && --timeout);
+
+ return timeout ? 0 : -EIO;
+}
+
static inline int jz4740_rtc_reg_write(struct jz4740_rtc *rtc, size_t reg,
uint32_t val)
{
- int ret;
- ret = jz4740_rtc_wait_write_ready(rtc);
+ int ret = 0;
+
+ if (rtc->type >= ID_JZ4780)
+ ret = jz4780_rtc_enable_write(rtc);
+ if (ret == 0)
+ ret = jz4740_rtc_wait_write_ready(rtc);
if (ret == 0)
writel(val, rtc->base + reg);

@@ -216,11 +251,14 @@ static int jz4740_rtc_probe(struct platform_device *pdev)
struct jz4740_rtc *rtc;
uint32_t scratchpad;
struct resource *mem;
+ const struct platform_device_id *id = platform_get_device_id(pdev);

rtc = devm_kzalloc(&pdev->dev, sizeof(*rtc), GFP_KERNEL);
if (!rtc)
return -ENOMEM;

+ rtc->type = id->driver_data;
+
rtc->irq = platform_get_irq(pdev, 0);
if (rtc->irq < 0) {
dev_err(&pdev->dev, "Failed to get platform irq\n");
@@ -295,12 +333,20 @@ static const struct dev_pm_ops jz4740_pm_ops = {
#define JZ4740_RTC_PM_OPS NULL
#endif /* CONFIG_PM */

+static const struct platform_device_id jz4740_rtc_ids[] = {
+ {"jz4740-rtc", ID_JZ4740},
+ {"jz4780-rtc", ID_JZ4780},
+ {}
+};
+MODULE_DEVICE_TABLE(platform, jz4740_rtc_ids);
+
static struct platform_driver jz4740_rtc_driver = {
.probe = jz4740_rtc_probe,
.driver = {
.name = "jz4740-rtc",
.pm = JZ4740_RTC_PM_OPS,
},
+ .id_table = jz4740_rtc_ids,
};

module_platform_driver(jz4740_rtc_driver);
--
2.7.0


2016-03-05 22:45:43

by Paul Cercueil

[permalink] [raw]
Subject: [PATCH 3/5] rtc: rtc-jz4740: Add support for devicetree

Signed-off-by: Paul Cercueil <[email protected]>
---
drivers/rtc/rtc-jz4740.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-jz4740.c b/drivers/rtc/rtc-jz4740.c
index 47617bd..3914b1c 100644
--- a/drivers/rtc/rtc-jz4740.c
+++ b/drivers/rtc/rtc-jz4740.c
@@ -17,6 +17,7 @@
#include <linux/io.h>
#include <linux/kernel.h>
#include <linux/module.h>
+#include <linux/of_device.h>
#include <linux/platform_device.h>
#include <linux/rtc.h>
#include <linux/slab.h>
@@ -245,6 +246,13 @@ void jz4740_rtc_poweroff(struct device *dev)
}
EXPORT_SYMBOL_GPL(jz4740_rtc_poweroff);

+static const struct of_device_id jz4740_rtc_of_match[] = {
+ { .compatible = "ingenic,jz4740-rtc", .data = (void *) ID_JZ4740 },
+ { .compatible = "ingenic,jz4780-rtc", .data = (void *) ID_JZ4780 },
+ {},
+};
+MODULE_DEVICE_TABLE(of, jz4740_rtc_of_match);
+
static int jz4740_rtc_probe(struct platform_device *pdev)
{
int ret;
@@ -252,12 +260,17 @@ static int jz4740_rtc_probe(struct platform_device *pdev)
uint32_t scratchpad;
struct resource *mem;
const struct platform_device_id *id = platform_get_device_id(pdev);
+ const struct of_device_id *of_id = of_match_device(
+ jz4740_rtc_of_match, &pdev->dev);

rtc = devm_kzalloc(&pdev->dev, sizeof(*rtc), GFP_KERNEL);
if (!rtc)
return -ENOMEM;

- rtc->type = id->driver_data;
+ if (of_id)
+ rtc->type = (enum jz4740_rtc_type) of_id->data;
+ else
+ rtc->type = id->driver_data;

rtc->irq = platform_get_irq(pdev, 0);
if (rtc->irq < 0) {
@@ -345,6 +358,7 @@ static struct platform_driver jz4740_rtc_driver = {
.driver = {
.name = "jz4740-rtc",
.pm = JZ4740_RTC_PM_OPS,
+ .of_match_table = of_match_ptr(jz4740_rtc_of_match),
},
.id_table = jz4740_rtc_ids,
};
--
2.7.0

2016-03-05 22:45:52

by Paul Cercueil

[permalink] [raw]
Subject: [PATCH 2/5] Documentation: dt: Add binding info for jz4740-rtc driver

Signed-off-by: Paul Cercueil <[email protected]>
---
.../devicetree/bindings/rtc/ingenic,jz4740-rtc.txt | 38 ++++++++++++++++++++++
1 file changed, 38 insertions(+)
create mode 100644 Documentation/devicetree/bindings/rtc/ingenic,jz4740-rtc.txt

diff --git a/Documentation/devicetree/bindings/rtc/ingenic,jz4740-rtc.txt b/Documentation/devicetree/bindings/rtc/ingenic,jz4740-rtc.txt
new file mode 100644
index 0000000..71e4ad0
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/ingenic,jz4740-rtc.txt
@@ -0,0 +1,38 @@
+JZ4740 and similar SoCs real-time clock driver
+
+Required properties:
+
+- compatible: One of:
+ - "ingenic,jz4740-rtc" - for use with the JZ4740 SoC
+ - "ingenic,jz4780-rtc" - for use with the JZ4780 SoC
+- reg: Address range of rtc register set
+- interrupts: IRQ number for the alarm interrupt
+- interrupt-parent: phandle of the interrupt controller
+- clocks: phandle to the "rtc" clock
+- clock-names: must be "rtc"
+
+Optional properties:
+- system-power-controller: To use this component as the
+ system power controller
+- reset-pin-assert-time: Reset pin low-level assertion time
+ after wakeup (default 60ms; range 0-125ms if RTC clock at
+ 32 kHz)
+- min-wakeup-pin-assert-time: Minimum wakeup pin assertion time
+ (default 100ms; range 0-2s if RTC clock at 32 kHz)
+
+Example:
+
+rtc@10003000 {
+ compatible = "ingenic,jz4740-rtc";
+ reg = <0x10003000 0x3F>;
+
+ interrupt-parent = <&intc>;
+ interrupts = <32>;
+
+ clocks = <&rtc_clock>;
+ clock-names = "rtc";
+
+ system-power-controller;
+ reset-pin-assert-time = <60>;
+ min-wakeup-pin-assert-time = <100>;
+};
--
2.7.0

2016-03-05 22:46:01

by Paul Cercueil

[permalink] [raw]
Subject: [PATCH 5/5] MIPS: jz4740: Use the jz4740-rtc driver as the power controller

Signed-off-by: Paul Cercueil <[email protected]>
---
arch/mips/boot/dts/ingenic/jz4740.dtsi | 2 ++
arch/mips/jz4740/reset.c | 64 ----------------------------------
2 files changed, 2 insertions(+), 64 deletions(-)

diff --git a/arch/mips/boot/dts/ingenic/jz4740.dtsi b/arch/mips/boot/dts/ingenic/jz4740.dtsi
index 8b2437c..f2ddacb 100644
--- a/arch/mips/boot/dts/ingenic/jz4740.dtsi
+++ b/arch/mips/boot/dts/ingenic/jz4740.dtsi
@@ -32,6 +32,8 @@
compatible = "fixed-clock";
#clock-cells = <0>;
clock-frequency = <32768>;
+
+ system-power-controller;
};

cgu: jz4740-cgu@10000000 {
diff --git a/arch/mips/jz4740/reset.c b/arch/mips/jz4740/reset.c
index 954e669..0a88b17 100644
--- a/arch/mips/jz4740/reset.c
+++ b/arch/mips/jz4740/reset.c
@@ -12,7 +12,6 @@
*
*/

-#include <linux/clk.h>
#include <linux/io.h>
#include <linux/kernel.h>
#include <linux/pm.h>
@@ -57,71 +56,8 @@ static void jz4740_restart(char *command)
jz4740_halt();
}

-#define JZ_REG_RTC_CTRL 0x00
-#define JZ_REG_RTC_HIBERNATE 0x20
-#define JZ_REG_RTC_WAKEUP_FILTER 0x24
-#define JZ_REG_RTC_RESET_COUNTER 0x28
-
-#define JZ_RTC_CTRL_WRDY BIT(7)
-#define JZ_RTC_WAKEUP_FILTER_MASK 0x0000FFE0
-#define JZ_RTC_RESET_COUNTER_MASK 0x00000FE0
-
-static inline void jz4740_rtc_wait_ready(void __iomem *rtc_base)
-{
- uint32_t ctrl;
-
- do {
- ctrl = readl(rtc_base + JZ_REG_RTC_CTRL);
- } while (!(ctrl & JZ_RTC_CTRL_WRDY));
-}
-
-static void jz4740_power_off(void)
-{
- void __iomem *rtc_base = ioremap(JZ4740_RTC_BASE_ADDR, 0x38);
- unsigned long wakeup_filter_ticks;
- unsigned long reset_counter_ticks;
- struct clk *rtc_clk;
- unsigned long rtc_rate;
-
- rtc_clk = clk_get(NULL, "rtc");
- if (IS_ERR(rtc_clk))
- panic("unable to get RTC clock");
- rtc_rate = clk_get_rate(rtc_clk);
- clk_put(rtc_clk);
-
- /*
- * Set minimum wakeup pin assertion time: 100 ms.
- * Range is 0 to 2 sec if RTC is clocked at 32 kHz.
- */
- wakeup_filter_ticks = (100 * rtc_rate) / 1000;
- if (wakeup_filter_ticks < JZ_RTC_WAKEUP_FILTER_MASK)
- wakeup_filter_ticks &= JZ_RTC_WAKEUP_FILTER_MASK;
- else
- wakeup_filter_ticks = JZ_RTC_WAKEUP_FILTER_MASK;
- jz4740_rtc_wait_ready(rtc_base);
- writel(wakeup_filter_ticks, rtc_base + JZ_REG_RTC_WAKEUP_FILTER);
-
- /*
- * Set reset pin low-level assertion time after wakeup: 60 ms.
- * Range is 0 to 125 ms if RTC is clocked at 32 kHz.
- */
- reset_counter_ticks = (60 * rtc_rate) / 1000;
- if (reset_counter_ticks < JZ_RTC_RESET_COUNTER_MASK)
- reset_counter_ticks &= JZ_RTC_RESET_COUNTER_MASK;
- else
- reset_counter_ticks = JZ_RTC_RESET_COUNTER_MASK;
- jz4740_rtc_wait_ready(rtc_base);
- writel(reset_counter_ticks, rtc_base + JZ_REG_RTC_RESET_COUNTER);
-
- jz4740_rtc_wait_ready(rtc_base);
- writel(1, rtc_base + JZ_REG_RTC_HIBERNATE);
-
- jz4740_halt();
-}
-
void jz4740_reset_init(void)
{
_machine_restart = jz4740_restart;
_machine_halt = jz4740_halt;
- pm_power_off = jz4740_power_off;
}
--
2.7.0

2016-03-05 22:46:23

by Paul Cercueil

[permalink] [raw]
Subject: [PATCH 4/5] rtc: jz4740_rtc: Add support for acting as the system power controller

Signed-off-by: Paul Cercueil <[email protected]>
---
drivers/rtc/rtc-jz4740.c | 77 ++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 77 insertions(+)

diff --git a/drivers/rtc/rtc-jz4740.c b/drivers/rtc/rtc-jz4740.c
index 3914b1c..f53cfd6 100644
--- a/drivers/rtc/rtc-jz4740.c
+++ b/drivers/rtc/rtc-jz4740.c
@@ -14,11 +14,13 @@
*
*/

+#include <linux/clk.h>
#include <linux/io.h>
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/of_device.h>
#include <linux/platform_device.h>
+#include <linux/reboot.h>
#include <linux/rtc.h>
#include <linux/slab.h>
#include <linux/spinlock.h>
@@ -28,6 +30,8 @@
#define JZ_REG_RTC_SEC_ALARM 0x08
#define JZ_REG_RTC_REGULATOR 0x0C
#define JZ_REG_RTC_HIBERNATE 0x20
+#define JZ_REG_RTC_WAKEUP_FILTER 0x24
+#define JZ_REG_RTC_RESET_COUNTER 0x28
#define JZ_REG_RTC_SCRATCHPAD 0x34

/* The following are present on the jz4780 */
@@ -45,6 +49,9 @@
/* Magic value to enable writes on jz4780 */
#define JZ_RTC_WENR_MAGIC 0xA55A

+#define JZ_RTC_WAKEUP_FILTER_MASK 0x0000FFE0
+#define JZ_RTC_RESET_COUNTER_MASK 0x00000FE0
+
enum jz4740_rtc_type {
ID_JZ4740,
ID_JZ4780,
@@ -59,8 +66,13 @@ struct jz4740_rtc {
int irq;

spinlock_t lock;
+
+ unsigned int min_wakeup_pin_assert_time;
+ unsigned int reset_pin_assert_time;
};

+static struct device *dev_for_power_off;
+
static inline uint32_t jz4740_rtc_reg_read(struct jz4740_rtc *rtc, size_t reg)
{
return readl(rtc->base + reg);
@@ -246,6 +258,49 @@ void jz4740_rtc_poweroff(struct device *dev)
}
EXPORT_SYMBOL_GPL(jz4740_rtc_poweroff);

+static void jz4740_rtc_power_off(void)
+{
+ struct jz4740_rtc *rtc = dev_get_drvdata(dev_for_power_off);
+ unsigned long wakeup_filter_ticks;
+ unsigned long reset_counter_ticks;
+ struct clk *rtc_clk;
+ unsigned long rtc_rate;
+
+ rtc_clk = clk_get(dev_for_power_off, "rtc");
+ if (IS_ERR(rtc_clk))
+ panic("unable to get RTC clock");
+ rtc_rate = clk_get_rate(rtc_clk);
+ clk_put(rtc_clk);
+
+ /*
+ * Set minimum wakeup pin assertion time: 100 ms.
+ * Range is 0 to 2 sec if RTC is clocked at 32 kHz.
+ */
+ wakeup_filter_ticks =
+ (rtc->min_wakeup_pin_assert_time * rtc_rate) / 1000;
+ if (wakeup_filter_ticks < JZ_RTC_WAKEUP_FILTER_MASK)
+ wakeup_filter_ticks &= JZ_RTC_WAKEUP_FILTER_MASK;
+ else
+ wakeup_filter_ticks = JZ_RTC_WAKEUP_FILTER_MASK;
+ jz4740_rtc_reg_write(rtc,
+ JZ_REG_RTC_WAKEUP_FILTER, wakeup_filter_ticks);
+
+ /*
+ * Set reset pin low-level assertion time after wakeup: 60 ms.
+ * Range is 0 to 125 ms if RTC is clocked at 32 kHz.
+ */
+ reset_counter_ticks = (rtc->reset_pin_assert_time * rtc_rate) / 1000;
+ if (reset_counter_ticks < JZ_RTC_RESET_COUNTER_MASK)
+ reset_counter_ticks &= JZ_RTC_RESET_COUNTER_MASK;
+ else
+ reset_counter_ticks = JZ_RTC_RESET_COUNTER_MASK;
+ jz4740_rtc_reg_write(rtc,
+ JZ_REG_RTC_RESET_COUNTER, reset_counter_ticks);
+
+ jz4740_rtc_poweroff(dev_for_power_off);
+ machine_halt();
+}
+
static const struct of_device_id jz4740_rtc_of_match[] = {
{ .compatible = "ingenic,jz4740-rtc", .data = (void *) ID_JZ4740 },
{ .compatible = "ingenic,jz4780-rtc", .data = (void *) ID_JZ4780 },
@@ -314,6 +369,28 @@ static int jz4740_rtc_probe(struct platform_device *pdev)
}
}

+ if (of_device_is_system_power_controller(pdev->dev.of_node)) {
+ if (!pm_power_off) {
+ /* Default: 60ms */
+ rtc->reset_pin_assert_time = 60;
+ device_property_read_u32(&pdev->dev,
+ "reset-pin-assert-time",
+ &rtc->reset_pin_assert_time);
+
+ /* Default: 100ms */
+ rtc->min_wakeup_pin_assert_time = 100;
+ device_property_read_u32(&pdev->dev,
+ "min-wakeup-pin-assert-time",
+ &rtc->min_wakeup_pin_assert_time);
+
+ dev_for_power_off = &pdev->dev;
+ pm_power_off = jz4740_rtc_power_off;
+ } else {
+ dev_err(&pdev->dev,
+ "Poweroff handler already present!\n");
+ }
+ }
+
return 0;
}

--
2.7.0

2016-03-06 12:14:14

by Sergei Shtylyov

[permalink] [raw]
Subject: Re: [PATCH 2/5] Documentation: dt: Add binding info for jz4740-rtc driver

Hello.

On 3/6/2016 1:38 AM, Paul Cercueil wrote:

> Signed-off-by: Paul Cercueil <[email protected]>
> ---
> .../devicetree/bindings/rtc/ingenic,jz4740-rtc.txt | 38 ++++++++++++++++++++++
> 1 file changed, 38 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/rtc/ingenic,jz4740-rtc.txt
>
> diff --git a/Documentation/devicetree/bindings/rtc/ingenic,jz4740-rtc.txt b/Documentation/devicetree/bindings/rtc/ingenic,jz4740-rtc.txt
> new file mode 100644
> index 0000000..71e4ad0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/rtc/ingenic,jz4740-rtc.txt
> @@ -0,0 +1,38 @@
> +JZ4740 and similar SoCs real-time clock driver
> +
> +Required properties:
> +
> +- compatible: One of:
> + - "ingenic,jz4740-rtc" - for use with the JZ4740 SoC
> + - "ingenic,jz4780-rtc" - for use with the JZ4780 SoC
> +- reg: Address range of rtc register set
> +- interrupts: IRQ number for the alarm interrupt
> +- interrupt-parent: phandle of the interrupt controller

This is never a required property, it can be inherited from the parent node.

[...]

MBR, Sergei

2016-03-14 01:54:11

by Ralf Baechle

[permalink] [raw]
Subject: Re: [PATCH 5/5] MIPS: jz4740: Use the jz4740-rtc driver as the power controller

On Sat, Mar 05, 2016 at 11:38:51PM +0100, Paul Cercueil wrote:

> Signed-off-by: Paul Cercueil <[email protected]>
> ---
> arch/mips/boot/dts/ingenic/jz4740.dtsi | 2 ++
> arch/mips/jz4740/reset.c | 64 ----------------------------------

Acked-by: Ralf Baechle <[email protected]>

Feel free to funnel this patch upstream through the RTC tree. Or I can
carry the whole series, just lemme know.

Ralf

2016-03-17 11:52:33

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH 2/5] Documentation: dt: Add binding info for jz4740-rtc driver

On Sat, Mar 05, 2016 at 11:38:48PM +0100, Paul Cercueil wrote:
> Signed-off-by: Paul Cercueil <[email protected]>
> ---
> .../devicetree/bindings/rtc/ingenic,jz4740-rtc.txt | 38 ++++++++++++++++++++++
> 1 file changed, 38 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/rtc/ingenic,jz4740-rtc.txt
>
> diff --git a/Documentation/devicetree/bindings/rtc/ingenic,jz4740-rtc.txt b/Documentation/devicetree/bindings/rtc/ingenic,jz4740-rtc.txt
> new file mode 100644
> index 0000000..71e4ad0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/rtc/ingenic,jz4740-rtc.txt
> @@ -0,0 +1,38 @@
> +JZ4740 and similar SoCs real-time clock driver
> +
> +Required properties:
> +
> +- compatible: One of:
> + - "ingenic,jz4740-rtc" - for use with the JZ4740 SoC
> + - "ingenic,jz4780-rtc" - for use with the JZ4780 SoC
> +- reg: Address range of rtc register set
> +- interrupts: IRQ number for the alarm interrupt
> +- interrupt-parent: phandle of the interrupt controller
> +- clocks: phandle to the "rtc" clock
> +- clock-names: must be "rtc"
> +
> +Optional properties:
> +- system-power-controller: To use this component as the
> + system power controller

> +- reset-pin-assert-time: Reset pin low-level assertion time
> + after wakeup (default 60ms; range 0-125ms if RTC clock at
> + 32 kHz)
> +- min-wakeup-pin-assert-time: Minimum wakeup pin assertion time
> + (default 100ms; range 0-2s if RTC clock at 32 kHz)

Please append units on these (-msec).

> +
> +Example:
> +
> +rtc@10003000 {
> + compatible = "ingenic,jz4740-rtc";
> + reg = <0x10003000 0x3F>;
> +
> + interrupt-parent = <&intc>;
> + interrupts = <32>;
> +
> + clocks = <&rtc_clock>;
> + clock-names = "rtc";
> +
> + system-power-controller;
> + reset-pin-assert-time = <60>;
> + min-wakeup-pin-assert-time = <100>;
> +};
> --
> 2.7.0
>

2016-03-17 12:04:24

by Alexandre Belloni

[permalink] [raw]
Subject: Re: [PATCH 1/5] rtc: rtc-jz4740: Add support for the RTC in the jz4780 SoC

Please, always include a commit message, even if it is short.

On 05/03/2016 at 23:38:47 +0100, Paul Cercueil wrote :
> Signed-off-by: Paul Cercueil <[email protected]>
> ---
> drivers/rtc/Kconfig | 6 +++---
> drivers/rtc/rtc-jz4740.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++--
> 2 files changed, 51 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
> index e593c55..b322f08 100644
> --- a/drivers/rtc/Kconfig
> +++ b/drivers/rtc/Kconfig
> @@ -1494,10 +1494,10 @@ config RTC_DRV_MPC5121
>
> config RTC_DRV_JZ4740
> tristate "Ingenic JZ4740 SoC"
> - depends on MACH_JZ4740 || COMPILE_TEST
> + depends on MACH_INGENIC || COMPILE_TEST
> help
> - If you say yes here you get support for the Ingenic JZ4740 SoC RTC
> - controller.
> + If you say yes here you get support for the Ingenic JZ47xx SoCs RTC
> + controllers.
>
> This driver can also be buillt as a module. If so, the module
> will be called rtc-jz4740.
> diff --git a/drivers/rtc/rtc-jz4740.c b/drivers/rtc/rtc-jz4740.c
> index b2bcfc0..47617bd 100644
> --- a/drivers/rtc/rtc-jz4740.c
> +++ b/drivers/rtc/rtc-jz4740.c
> @@ -29,6 +29,10 @@
> #define JZ_REG_RTC_HIBERNATE 0x20
> #define JZ_REG_RTC_SCRATCHPAD 0x34
>
> +/* The following are present on the jz4780 */
> +#define JZ_REG_RTC_WENR 0x3C
> +#define JZ_RTC_WENR_WEN BIT(31)
> +
> #define JZ_RTC_CTRL_WRDY BIT(7)
> #define JZ_RTC_CTRL_1HZ BIT(6)
> #define JZ_RTC_CTRL_1HZ_IRQ BIT(5)
> @@ -37,8 +41,17 @@
> #define JZ_RTC_CTRL_AE BIT(2)
> #define JZ_RTC_CTRL_ENABLE BIT(0)
>
> +/* Magic value to enable writes on jz4780 */
> +#define JZ_RTC_WENR_MAGIC 0xA55A
> +
> +enum jz4740_rtc_type {
> + ID_JZ4740,
> + ID_JZ4780,
> +};
> +
> struct jz4740_rtc {
> void __iomem *base;
> + enum jz4740_rtc_type type;
>
> struct rtc_device *rtc;
>
> @@ -64,11 +77,33 @@ static int jz4740_rtc_wait_write_ready(struct jz4740_rtc *rtc)
> return timeout ? 0 : -EIO;
> }
>
> +static inline int jz4780_rtc_enable_write(struct jz4740_rtc *rtc)
> +{
> + uint32_t ctrl;
> + int ret, timeout = 1000;
> +
> + ret = jz4740_rtc_wait_write_ready(rtc);
> + if (ret != 0)
> + return ret;
> +
> + writel(JZ_RTC_WENR_MAGIC, rtc->base + JZ_REG_RTC_WENR);
> +
> + do {
> + ctrl = readl(rtc->base + JZ_REG_RTC_WENR);
> + } while (!(ctrl & JZ_RTC_WENR_WEN) && --timeout);
> +
> + return timeout ? 0 : -EIO;
> +}
> +
> static inline int jz4740_rtc_reg_write(struct jz4740_rtc *rtc, size_t reg,
> uint32_t val)
> {
> - int ret;
> - ret = jz4740_rtc_wait_write_ready(rtc);
> + int ret = 0;
> +
> + if (rtc->type >= ID_JZ4780)
> + ret = jz4780_rtc_enable_write(rtc);
> + if (ret == 0)
> + ret = jz4740_rtc_wait_write_ready(rtc);
> if (ret == 0)
> writel(val, rtc->base + reg);
>
> @@ -216,11 +251,14 @@ static int jz4740_rtc_probe(struct platform_device *pdev)
> struct jz4740_rtc *rtc;
> uint32_t scratchpad;
> struct resource *mem;
> + const struct platform_device_id *id = platform_get_device_id(pdev);
>
> rtc = devm_kzalloc(&pdev->dev, sizeof(*rtc), GFP_KERNEL);
> if (!rtc)
> return -ENOMEM;
>
> + rtc->type = id->driver_data;
> +
> rtc->irq = platform_get_irq(pdev, 0);
> if (rtc->irq < 0) {
> dev_err(&pdev->dev, "Failed to get platform irq\n");
> @@ -295,12 +333,20 @@ static const struct dev_pm_ops jz4740_pm_ops = {
> #define JZ4740_RTC_PM_OPS NULL
> #endif /* CONFIG_PM */
>
> +static const struct platform_device_id jz4740_rtc_ids[] = {
> + {"jz4740-rtc", ID_JZ4740},
> + {"jz4780-rtc", ID_JZ4780},
> + {}
> +};
> +MODULE_DEVICE_TABLE(platform, jz4740_rtc_ids);
> +
> static struct platform_driver jz4740_rtc_driver = {
> .probe = jz4740_rtc_probe,
> .driver = {
> .name = "jz4740-rtc",
> .pm = JZ4740_RTC_PM_OPS,
> },
> + .id_table = jz4740_rtc_ids,
> };
>
> module_platform_driver(jz4740_rtc_driver);
> --
> 2.7.0
>

--
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

2016-03-17 12:05:04

by Alexandre Belloni

[permalink] [raw]
Subject: Re: [PATCH 2/5] Documentation: dt: Add binding info for jz4740-rtc driver

On 05/03/2016 at 23:38:48 +0100, Paul Cercueil wrote :
> Signed-off-by: Paul Cercueil <[email protected]>
> ---
> .../devicetree/bindings/rtc/ingenic,jz4740-rtc.txt | 38 ++++++++++++++++++++++
> 1 file changed, 38 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/rtc/ingenic,jz4740-rtc.txt
>
> diff --git a/Documentation/devicetree/bindings/rtc/ingenic,jz4740-rtc.txt b/Documentation/devicetree/bindings/rtc/ingenic,jz4740-rtc.txt
> new file mode 100644
> index 0000000..71e4ad0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/rtc/ingenic,jz4740-rtc.txt
> @@ -0,0 +1,38 @@
> +JZ4740 and similar SoCs real-time clock driver
> +
> +Required properties:
> +
> +- compatible: One of:
> + - "ingenic,jz4740-rtc" - for use with the JZ4740 SoC
> + - "ingenic,jz4780-rtc" - for use with the JZ4780 SoC
> +- reg: Address range of rtc register set
> +- interrupts: IRQ number for the alarm interrupt
> +- interrupt-parent: phandle of the interrupt controller
> +- clocks: phandle to the "rtc" clock
> +- clock-names: must be "rtc"
> +
> +Optional properties:
> +- system-power-controller: To use this component as the
> + system power controller
> +- reset-pin-assert-time: Reset pin low-level assertion time
> + after wakeup (default 60ms; range 0-125ms if RTC clock at

Trailing whitespace on that line.


--
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

2016-03-17 12:08:09

by Alexandre Belloni

[permalink] [raw]
Subject: Re: [PATCH 3/5] rtc: rtc-jz4740: Add support for devicetree

On 05/03/2016 at 23:38:49 +0100, Paul Cercueil wrote :
> Signed-off-by: Paul Cercueil <[email protected]>
> ---
> drivers/rtc/rtc-jz4740.c | 16 +++++++++++++++-
> 1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/rtc/rtc-jz4740.c b/drivers/rtc/rtc-jz4740.c
> index 47617bd..3914b1c 100644
> --- a/drivers/rtc/rtc-jz4740.c
> +++ b/drivers/rtc/rtc-jz4740.c
> @@ -17,6 +17,7 @@
> #include <linux/io.h>
> #include <linux/kernel.h>
> #include <linux/module.h>
> +#include <linux/of_device.h>
> #include <linux/platform_device.h>
> #include <linux/rtc.h>
> #include <linux/slab.h>
> @@ -245,6 +246,13 @@ void jz4740_rtc_poweroff(struct device *dev)
> }
> EXPORT_SYMBOL_GPL(jz4740_rtc_poweroff);
>
> +static const struct of_device_id jz4740_rtc_of_match[] = {
> + { .compatible = "ingenic,jz4740-rtc", .data = (void *) ID_JZ4740 },
> + { .compatible = "ingenic,jz4780-rtc", .data = (void *) ID_JZ4780 },

ingenic is not in Documentation/devicetree/bindings/vendor-prefixes.txt,
you have to add it there before using it.

Also, no space is necessary after the "(void *)" cast.

> + {},
> +};
> +MODULE_DEVICE_TABLE(of, jz4740_rtc_of_match);
> +
> static int jz4740_rtc_probe(struct platform_device *pdev)
> {
> int ret;
> @@ -252,12 +260,17 @@ static int jz4740_rtc_probe(struct platform_device *pdev)
> uint32_t scratchpad;
> struct resource *mem;
> const struct platform_device_id *id = platform_get_device_id(pdev);
> + const struct of_device_id *of_id = of_match_device(
> + jz4740_rtc_of_match, &pdev->dev);
>
> rtc = devm_kzalloc(&pdev->dev, sizeof(*rtc), GFP_KERNEL);
> if (!rtc)
> return -ENOMEM;
>
> - rtc->type = id->driver_data;
> + if (of_id)
> + rtc->type = (enum jz4740_rtc_type) of_id->data;

No space after that cast either.

> + else
> + rtc->type = id->driver_data;
>
> rtc->irq = platform_get_irq(pdev, 0);
> if (rtc->irq < 0) {
> @@ -345,6 +358,7 @@ static struct platform_driver jz4740_rtc_driver = {
> .driver = {
> .name = "jz4740-rtc",
> .pm = JZ4740_RTC_PM_OPS,
> + .of_match_table = of_match_ptr(jz4740_rtc_of_match),
> },
> .id_table = jz4740_rtc_ids,
> };
> --
> 2.7.0
>

--
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

2016-03-17 12:21:04

by Alexandre Belloni

[permalink] [raw]
Subject: Re: [PATCH 4/5] rtc: jz4740_rtc: Add support for acting as the system power controller

On 05/03/2016 at 23:38:50 +0100, Paul Cercueil wrote :
> + if (of_device_is_system_power_controller(pdev->dev.of_node)) {
> + if (!pm_power_off) {
> + /* Default: 60ms */
> + rtc->reset_pin_assert_time = 60;
> + device_property_read_u32(&pdev->dev,

It is probably more efficient to use of_property_read_u32 directly. Any
particular reason to use device_property_read_u32?

> + } else {
> + dev_err(&pdev->dev,
> + "Poweroff handler already present!\n");

I'm not found of that alignement, it is probably better to let the
string go over 80 chars. checkpatch will not complain.

--
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

2016-03-17 12:22:29

by Alexandre Belloni

[permalink] [raw]
Subject: Re: [PATCH 5/5] MIPS: jz4740: Use the jz4740-rtc driver as the power controller

Hi,

On 14/03/2016 at 02:53:53 +0100, Ralf Baechle wrote :
> On Sat, Mar 05, 2016 at 11:38:51PM +0100, Paul Cercueil wrote:
>
> > Signed-off-by: Paul Cercueil <[email protected]>
> > ---
> > arch/mips/boot/dts/ingenic/jz4740.dtsi | 2 ++
> > arch/mips/jz4740/reset.c | 64 ----------------------------------
>
> Acked-by: Ralf Baechle <[email protected]>
>
> Feel free to funnel this patch upstream through the RTC tree. Or I can
> carry the whole series, just lemme know.
>

I made a few minor comments. I'll take the series once it has been
resent.

--
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

2016-03-17 13:33:11

by Harvey Hunt

[permalink] [raw]
Subject: Re: [PATCH 3/5] rtc: rtc-jz4740: Add support for devicetree

Hi Alexandre,

On 17/03/16 12:08, Alexandre Belloni wrote:
> On 05/03/2016 at 23:38:49 +0100, Paul Cercueil wrote :
>> Signed-off-by: Paul Cercueil <[email protected]>
>> ---
>> drivers/rtc/rtc-jz4740.c | 16 +++++++++++++++-
>> 1 file changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/rtc/rtc-jz4740.c b/drivers/rtc/rtc-jz4740.c
>> index 47617bd..3914b1c 100644
>> --- a/drivers/rtc/rtc-jz4740.c
>> +++ b/drivers/rtc/rtc-jz4740.c
>> @@ -17,6 +17,7 @@
>> #include <linux/io.h>
>> #include <linux/kernel.h>
>> #include <linux/module.h>
>> +#include <linux/of_device.h>
>> #include <linux/platform_device.h>
>> #include <linux/rtc.h>
>> #include <linux/slab.h>
>> @@ -245,6 +246,13 @@ void jz4740_rtc_poweroff(struct device *dev)
>> }
>> EXPORT_SYMBOL_GPL(jz4740_rtc_poweroff);
>>
>> +static const struct of_device_id jz4740_rtc_of_match[] = {
>> + { .compatible = "ingenic,jz4740-rtc", .data = (void *) ID_JZ4740 },
>> + { .compatible = "ingenic,jz4780-rtc", .data = (void *) ID_JZ4780 },
>
> ingenic is not in Documentation/devicetree/bindings/vendor-prefixes.txt,
> you have to add it there before using it.

Ingenic is in vendor-prefixes.txt - it was added by Commit f289cc7
("devicetree/bindings: add Ingenic Semiconductor vendor prefix").

Thanks,

Harvey

2016-03-17 13:56:47

by Alexandre Belloni

[permalink] [raw]
Subject: Re: [PATCH 3/5] rtc: rtc-jz4740: Add support for devicetree

On 17/03/2016 at 13:33:04 +0000, Harvey Hunt wrote :
> On 17/03/16 12:08, Alexandre Belloni wrote:
> >On 05/03/2016 at 23:38:49 +0100, Paul Cercueil wrote :
> >>Signed-off-by: Paul Cercueil <[email protected]>
> >>---
> >> drivers/rtc/rtc-jz4740.c | 16 +++++++++++++++-
> >> 1 file changed, 15 insertions(+), 1 deletion(-)
> >>
> >>diff --git a/drivers/rtc/rtc-jz4740.c b/drivers/rtc/rtc-jz4740.c
> >>index 47617bd..3914b1c 100644
> >>--- a/drivers/rtc/rtc-jz4740.c
> >>+++ b/drivers/rtc/rtc-jz4740.c
> >>@@ -17,6 +17,7 @@
> >> #include <linux/io.h>
> >> #include <linux/kernel.h>
> >> #include <linux/module.h>
> >>+#include <linux/of_device.h>
> >> #include <linux/platform_device.h>
> >> #include <linux/rtc.h>
> >> #include <linux/slab.h>
> >>@@ -245,6 +246,13 @@ void jz4740_rtc_poweroff(struct device *dev)
> >> }
> >> EXPORT_SYMBOL_GPL(jz4740_rtc_poweroff);
> >>
> >>+static const struct of_device_id jz4740_rtc_of_match[] = {
> >>+ { .compatible = "ingenic,jz4740-rtc", .data = (void *) ID_JZ4740 },
> >>+ { .compatible = "ingenic,jz4780-rtc", .data = (void *) ID_JZ4780 },
> >
> >ingenic is not in Documentation/devicetree/bindings/vendor-prefixes.txt,
> >you have to add it there before using it.
>
> Ingenic is in vendor-prefixes.txt - it was added by Commit f289cc7
> ("devicetree/bindings: add Ingenic Semiconductor vendor prefix").
>

Indeed, I was looking at an old v4.1 based branch instead of master.

You can forget that comment :)


--
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com