[PATCH 1/7] rtc: ingenic: Only support probing from devicetree

[PATCH 1/7] rtc: ingenic: Only support probing from devicetree

[Paul Cercueil]

With the recent work on supporting Device Tree on Ingenic SoCs, no
driver ever probes from platform code anymore, so we can clean a bit
this driver by removing the non-devicetree paths.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 drivers/rtc/Kconfig      |  1 +
 drivers/rtc/rtc-jz4740.c | 20 +++-----------------
 2 files changed, 4 insertions(+), 17 deletions(-)

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index ec873f09c763..82a210920c1d 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -1680,6 +1680,7 @@ config RTC_DRV_MPC5121
 config RTC_DRV_JZ4740
 	tristate "Ingenic JZ4740 SoC"
 	depends on MIPS || COMPILE_TEST
+	depends on OF
 	help
 	  If you say yes here you get support for the Ingenic JZ47xx SoCs RTC
 	  controllers.
diff --git a/drivers/rtc/rtc-jz4740.c b/drivers/rtc/rtc-jz4740.c
index e4c719085c31..949d395066e2 100644
--- a/drivers/rtc/rtc-jz4740.c
+++ b/drivers/rtc/rtc-jz4740.c
@@ -309,19 +309,13 @@ static int jz4740_rtc_probe(struct platform_device *pdev)
 {
 	int ret;
 	struct jz4740_rtc *rtc;
-	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);
 	struct device_node *np = pdev->dev.of_node;
 
 	rtc = devm_kzalloc(&pdev->dev, sizeof(*rtc), GFP_KERNEL);
 	if (!rtc)
 		return -ENOMEM;
 
-	if (of_id)
-		rtc->type = (enum jz4740_rtc_type)of_id->data;
-	else
-		rtc->type = id->driver_data;
+	rtc->type = (enum jz4740_rtc_type)device_get_match_data(dev);
 
 	rtc->irq = platform_get_irq(pdev, 0);
 	if (rtc->irq < 0)
@@ -370,7 +364,7 @@ static int jz4740_rtc_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	if (np && of_device_is_system_power_controller(np)) {
+	if (of_device_is_system_power_controller(np)) {
 		if (!pm_power_off) {
 			/* Default: 60ms */
 			rtc->reset_pin_assert_time = 60;
@@ -395,20 +389,12 @@ static int jz4740_rtc_probe(struct platform_device *pdev)
 	return 0;
 }
 
-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",
-		.of_match_table = of_match_ptr(jz4740_rtc_of_match),
+		.of_match_table = jz4740_rtc_of_match,
 	},
-	.id_table = jz4740_rtc_ids,
 };
 
 module_platform_driver(jz4740_rtc_driver);
-- 
2.26.2

[PATCH 2/7] rtc: ingenic: Use local 'dev' variable in probe

[Paul Cercueil]

Clean a bit the probe function by adding a local struct device *dev
variable.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 drivers/rtc/rtc-jz4740.c | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/rtc/rtc-jz4740.c b/drivers/rtc/rtc-jz4740.c
index 949d395066e2..06ee08089815 100644
--- a/drivers/rtc/rtc-jz4740.c
+++ b/drivers/rtc/rtc-jz4740.c
@@ -307,11 +307,12 @@ MODULE_DEVICE_TABLE(of, jz4740_rtc_of_match);
 
 static int jz4740_rtc_probe(struct platform_device *pdev)
 {
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
 	int ret;
 	struct jz4740_rtc *rtc;
-	struct device_node *np = pdev->dev.of_node;
 
-	rtc = devm_kzalloc(&pdev->dev, sizeof(*rtc), GFP_KERNEL);
+	rtc = devm_kzalloc(dev, sizeof(*rtc), GFP_KERNEL);
 	if (!rtc)
 		return -ENOMEM;
 
@@ -325,9 +326,9 @@ static int jz4740_rtc_probe(struct platform_device *pdev)
 	if (IS_ERR(rtc->base))
 		return PTR_ERR(rtc->base);
 
-	rtc->clk = devm_clk_get(&pdev->dev, "rtc");
+	rtc->clk = devm_clk_get(dev, "rtc");
 	if (IS_ERR(rtc->clk)) {
-		dev_err(&pdev->dev, "Failed to get RTC clock\n");
+		dev_err(dev, "Failed to get RTC clock\n");
 		return PTR_ERR(rtc->clk);
 	}
 
@@ -335,18 +336,18 @@ static int jz4740_rtc_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, rtc);
 
-	device_init_wakeup(&pdev->dev, 1);
+	device_init_wakeup(dev, 1);
 
-	ret = dev_pm_set_wake_irq(&pdev->dev, rtc->irq);
+	ret = dev_pm_set_wake_irq(dev, rtc->irq);
 	if (ret) {
-		dev_err(&pdev->dev, "Failed to set wake irq: %d\n", ret);
+		dev_err(dev, "Failed to set wake irq: %d\n", ret);
 		return ret;
 	}
 
-	rtc->rtc = devm_rtc_allocate_device(&pdev->dev);
+	rtc->rtc = devm_rtc_allocate_device(dev);
 	if (IS_ERR(rtc->rtc)) {
 		ret = PTR_ERR(rtc->rtc);
-		dev_err(&pdev->dev, "Failed to allocate rtc device: %d\n", ret);
+		dev_err(dev, "Failed to allocate rtc device: %d\n", ret);
 		return ret;
 	}
 
@@ -357,10 +358,10 @@ static int jz4740_rtc_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	ret = devm_request_irq(&pdev->dev, rtc->irq, jz4740_rtc_irq, 0,
-				pdev->name, rtc);
+	ret = devm_request_irq(dev, rtc->irq, jz4740_rtc_irq, 0,
+			       pdev->name, rtc);
 	if (ret) {
-		dev_err(&pdev->dev, "Failed to request rtc irq: %d\n", ret);
+		dev_err(dev, "Failed to request rtc irq: %d\n", ret);
 		return ret;
 	}
 
@@ -378,11 +379,10 @@ static int jz4740_rtc_probe(struct platform_device *pdev)
 					     "ingenic,min-wakeup-pin-assert-time-ms",
 					     &rtc->min_wakeup_pin_assert_time);
 
-			dev_for_power_off = &pdev->dev;
+			dev_for_power_off = dev;
 			pm_power_off = jz4740_rtc_power_off;
 		} else {
-			dev_warn(&pdev->dev,
-				 "Poweroff handler already present!\n");
+			dev_warn(dev, "Poweroff handler already present!\n");
 		}
 	}
 
-- 
2.26.2

[PATCH 3/7] rtc: ingenic: Enable clock in probe

[Paul Cercueil]

It makes no sense to request a clock and not enable it even though the
hardware is being used. So the driver now enables the clock in the
probe. Besides, now we can properly handle errors.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 drivers/rtc/rtc-jz4740.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/rtc/rtc-jz4740.c b/drivers/rtc/rtc-jz4740.c
index 06ee08089815..129c68cebb92 100644
--- a/drivers/rtc/rtc-jz4740.c
+++ b/drivers/rtc/rtc-jz4740.c
@@ -264,8 +264,6 @@ static void jz4740_rtc_power_off(void)
 	unsigned long wakeup_filter_ticks;
 	unsigned long reset_counter_ticks;
 
-	clk_prepare_enable(rtc->clk);
-
 	rtc_rate = clk_get_rate(rtc->clk);
 
 	/*
@@ -297,6 +295,11 @@ static void jz4740_rtc_power_off(void)
 	kernel_halt();
 }
 
+static void jz4740_rtc_clk_disable(void *data)
+{
+	clk_disable_unprepare(data);
+}
+
 static const struct of_device_id jz4740_rtc_of_match[] = {
 	{ .compatible = "ingenic,jz4740-rtc", .data = (void *)ID_JZ4740 },
 	{ .compatible = "ingenic,jz4760-rtc", .data = (void *)ID_JZ4760 },
@@ -332,6 +335,18 @@ static int jz4740_rtc_probe(struct platform_device *pdev)
 		return PTR_ERR(rtc->clk);
 	}
 
+	ret = clk_prepare_enable(rtc->clk);
+	if (ret) {
+		dev_err(dev, "Failed to enable clock\n");
+		return ret;
+	}
+
+	ret = devm_add_action_or_reset(dev, jz4740_rtc_clk_disable, rtc->clk);
+	if (ret) {
+		dev_err(dev, "Failed to register devm action\n");
+		return ret;
+	}
+
 	spin_lock_init(&rtc->lock);
 
 	platform_set_drvdata(pdev, rtc);
-- 
2.26.2

[PATCH 4/7] rtc: ingenic: Set wakeup params in probe

[Paul Cercueil]

We can write the wakeup timing parameters as soon as the driver probes,
there's no need to wait the very last moment.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 drivers/rtc/rtc-jz4740.c | 95 +++++++++++++++++++---------------------
 1 file changed, 44 insertions(+), 51 deletions(-)

diff --git a/drivers/rtc/rtc-jz4740.c b/drivers/rtc/rtc-jz4740.c
index 129c68cebb92..8927fd0fb086 100644
--- a/drivers/rtc/rtc-jz4740.c
+++ b/drivers/rtc/rtc-jz4740.c
@@ -60,9 +60,6 @@ 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;
@@ -259,38 +256,6 @@ static void jz4740_rtc_poweroff(struct device *dev)
 
 static void jz4740_rtc_power_off(void)
 {
-	struct jz4740_rtc *rtc = dev_get_drvdata(dev_for_power_off);
-	unsigned long rtc_rate;
-	unsigned long wakeup_filter_ticks;
-	unsigned long reset_counter_ticks;
-
-	rtc_rate = clk_get_rate(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);
 	kernel_halt();
 }
@@ -308,12 +273,49 @@ static const struct of_device_id jz4740_rtc_of_match[] = {
 };
 MODULE_DEVICE_TABLE(of, jz4740_rtc_of_match);
 
+static void jz4740_rtc_set_wakeup_params(struct jz4740_rtc *rtc,
+					 struct device_node *np,
+					 unsigned long rate)
+{
+	unsigned long wakeup_ticks, reset_ticks;
+	unsigned int min_wakeup_pin_assert_time = 60; /* Default: 60ms */
+	unsigned int reset_pin_assert_time = 100; /* Default: 100ms */
+
+	of_property_read_u32(np, "ingenic,reset-pin-assert-time-ms",
+			     &reset_pin_assert_time);
+	of_property_read_u32(np, "ingenic,min-wakeup-pin-assert-time-ms",
+			     &min_wakeup_pin_assert_time);
+
+	/*
+	 * Set minimum wakeup pin assertion time: 100 ms.
+	 * Range is 0 to 2 sec if RTC is clocked at 32 kHz.
+	 */
+	wakeup_ticks = (min_wakeup_pin_assert_time * rate) / 1000;
+	if (wakeup_ticks < JZ_RTC_WAKEUP_FILTER_MASK)
+		wakeup_ticks &= JZ_RTC_WAKEUP_FILTER_MASK;
+	else
+		wakeup_ticks = JZ_RTC_WAKEUP_FILTER_MASK;
+	jz4740_rtc_reg_write(rtc, JZ_REG_RTC_WAKEUP_FILTER, wakeup_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_ticks = (reset_pin_assert_time * rate) / 1000;
+	if (reset_ticks < JZ_RTC_RESET_COUNTER_MASK)
+		reset_ticks &= JZ_RTC_RESET_COUNTER_MASK;
+	else
+		reset_ticks = JZ_RTC_RESET_COUNTER_MASK;
+	jz4740_rtc_reg_write(rtc, JZ_REG_RTC_RESET_COUNTER, reset_ticks);
+}
+
 static int jz4740_rtc_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct device_node *np = dev->of_node;
 	int ret;
 	struct jz4740_rtc *rtc;
+	unsigned long rate;
 
 	rtc = devm_kzalloc(dev, sizeof(*rtc), GFP_KERNEL);
 	if (!rtc)
@@ -369,6 +371,9 @@ static int jz4740_rtc_probe(struct platform_device *pdev)
 	rtc->rtc->ops = &jz4740_rtc_ops;
 	rtc->rtc->range_max = U32_MAX;
 
+	rate = clk_get_rate(rtc->clk);
+	jz4740_rtc_set_wakeup_params(rtc, np, rate);
+
 	ret = rtc_register_device(rtc->rtc);
 	if (ret)
 		return ret;
@@ -381,24 +386,12 @@ static int jz4740_rtc_probe(struct platform_device *pdev)
 	}
 
 	if (of_device_is_system_power_controller(np)) {
-		if (!pm_power_off) {
-			/* Default: 60ms */
-			rtc->reset_pin_assert_time = 60;
-			of_property_read_u32(np,
-					     "ingenic,reset-pin-assert-time-ms",
-					     &rtc->reset_pin_assert_time);
-
-			/* Default: 100ms */
-			rtc->min_wakeup_pin_assert_time = 100;
-			of_property_read_u32(np,
-					     "ingenic,min-wakeup-pin-assert-time-ms",
-					     &rtc->min_wakeup_pin_assert_time);
-
-			dev_for_power_off = dev;
+		dev_for_power_off = dev;
+
+		if (!pm_power_off)
 			pm_power_off = jz4740_rtc_power_off;
-		} else {
+		else
 			dev_warn(dev, "Poweroff handler already present!\n");
-		}
 	}
 
 	return 0;
-- 
2.26.2

[PATCH 5/7] rtc: ingenic: Remove unused fields from private structure

[Paul Cercueil]

The 'clk' and 'irq' fields were only ever used in the probe function.
Therefore they can be moved to be simple local variables of the probe
function.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 drivers/rtc/rtc-jz4740.c | 26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/drivers/rtc/rtc-jz4740.c b/drivers/rtc/rtc-jz4740.c
index 8927fd0fb086..3193eb8bd131 100644
--- a/drivers/rtc/rtc-jz4740.c
+++ b/drivers/rtc/rtc-jz4740.c
@@ -55,9 +55,6 @@ struct jz4740_rtc {
 	enum jz4740_rtc_type type;
 
 	struct rtc_device *rtc;
-	struct clk *clk;
-
-	int irq;
 
 	spinlock_t lock;
 };
@@ -313,9 +310,10 @@ static int jz4740_rtc_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct device_node *np = dev->of_node;
-	int ret;
 	struct jz4740_rtc *rtc;
 	unsigned long rate;
+	struct clk *clk;
+	int ret, irq;
 
 	rtc = devm_kzalloc(dev, sizeof(*rtc), GFP_KERNEL);
 	if (!rtc)
@@ -323,27 +321,27 @@ static int jz4740_rtc_probe(struct platform_device *pdev)
 
 	rtc->type = (enum jz4740_rtc_type)device_get_match_data(dev);
 
-	rtc->irq = platform_get_irq(pdev, 0);
-	if (rtc->irq < 0)
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0)
 		return -ENOENT;
 
 	rtc->base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(rtc->base))
 		return PTR_ERR(rtc->base);
 
-	rtc->clk = devm_clk_get(dev, "rtc");
-	if (IS_ERR(rtc->clk)) {
+	clk = devm_clk_get(dev, "rtc");
+	if (IS_ERR(clk)) {
 		dev_err(dev, "Failed to get RTC clock\n");
-		return PTR_ERR(rtc->clk);
+		return PTR_ERR(clk);
 	}
 
-	ret = clk_prepare_enable(rtc->clk);
+	ret = clk_prepare_enable(clk);
 	if (ret) {
 		dev_err(dev, "Failed to enable clock\n");
 		return ret;
 	}
 
-	ret = devm_add_action_or_reset(dev, jz4740_rtc_clk_disable, rtc->clk);
+	ret = devm_add_action_or_reset(dev, jz4740_rtc_clk_disable, clk);
 	if (ret) {
 		dev_err(dev, "Failed to register devm action\n");
 		return ret;
@@ -355,7 +353,7 @@ static int jz4740_rtc_probe(struct platform_device *pdev)
 
 	device_init_wakeup(dev, 1);
 
-	ret = dev_pm_set_wake_irq(dev, rtc->irq);
+	ret = dev_pm_set_wake_irq(dev, irq);
 	if (ret) {
 		dev_err(dev, "Failed to set wake irq: %d\n", ret);
 		return ret;
@@ -371,14 +369,14 @@ static int jz4740_rtc_probe(struct platform_device *pdev)
 	rtc->rtc->ops = &jz4740_rtc_ops;
 	rtc->rtc->range_max = U32_MAX;
 
-	rate = clk_get_rate(rtc->clk);
+	rate = clk_get_rate(clk);
 	jz4740_rtc_set_wakeup_params(rtc, np, rate);
 
 	ret = rtc_register_device(rtc->rtc);
 	if (ret)
 		return ret;
 
-	ret = devm_request_irq(dev, rtc->irq, jz4740_rtc_irq, 0,
+	ret = devm_request_irq(dev, irq, jz4740_rtc_irq, 0,
 			       pdev->name, rtc);
 	if (ret) {
 		dev_err(dev, "Failed to request rtc irq: %d\n", ret);
-- 
2.26.2

[PATCH 6/7] rtc: ingenic: Fix masking of error code

[Paul Cercueil]

The code was returning -ENOENT on any error of platform_get_irq(), even
if it returned a different error.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 drivers/rtc/rtc-jz4740.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-jz4740.c b/drivers/rtc/rtc-jz4740.c
index 3193eb8bd131..65e130726fc6 100644
--- a/drivers/rtc/rtc-jz4740.c
+++ b/drivers/rtc/rtc-jz4740.c
@@ -323,7 +323,7 @@ static int jz4740_rtc_probe(struct platform_device *pdev)
 
 	irq = platform_get_irq(pdev, 0);
 	if (irq < 0)
-		return -ENOENT;
+		return irq;
 
 	rtc->base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(rtc->base))
-- 
2.26.2

[PATCH 7/7] rtc: ingenic: Reset regulator register in probe

[Paul Cercueil]

The regulator register specifies how many input clock cycles (minus one)
are contained in one tick of the 1 Hz clock.

Since this register can contain bogus values after the system boots, it
needs to be reset in the probe register, otherwise the RTC may count way
to slow or way too fast.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 drivers/rtc/rtc-jz4740.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/rtc/rtc-jz4740.c b/drivers/rtc/rtc-jz4740.c
index 65e130726fc6..9607e6b6e0b3 100644
--- a/drivers/rtc/rtc-jz4740.c
+++ b/drivers/rtc/rtc-jz4740.c
@@ -372,6 +372,9 @@ static int jz4740_rtc_probe(struct platform_device *pdev)
 	rate = clk_get_rate(clk);
 	jz4740_rtc_set_wakeup_params(rtc, np, rate);
 
+	/* Each 1 Hz pulse should happen after (rate) ticks */
+	jz4740_rtc_reg_write(rtc, JZ_REG_RTC_REGULATOR, rate - 1);
+
 	ret = rtc_register_device(rtc->rtc);
 	if (ret)
 		return ret;
-- 
2.26.2

Re: [PATCH 1/7] rtc: ingenic: Only support probing from devicetree

[Alexandre Belloni]

On 06/05/2020 00:13:30+0200, Paul Cercueil wrote:
> With the recent work on supporting Device Tree on Ingenic SoCs, no
> driver ever probes from platform code anymore, so we can clean a bit
> this driver by removing the non-devicetree paths.
> 
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---
>  drivers/rtc/Kconfig      |  1 +
>  drivers/rtc/rtc-jz4740.c | 20 +++-----------------
>  2 files changed, 4 insertions(+), 17 deletions(-)
> 

All applied, thanks.

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com