Hi,
Here's a revised patchset that introduces a few updates to the
jz4740-rtc driver.
Patch [1/4] used to break ABI, it does not anymore.
Patch [2/4] did not change, patch [3/4] is new.
Patch [3/4] has been updated to use dev_err_probe(), use __clk_hw_get()
instead of looking up the parent's clock by name, and will now register
the CLK32K clock when the #clock-cells device property is present
instead of doing it based on the compatible string.
V2 had an extra patch to add support for fine-tuning the RTC; but since
it was not tested enough I decided to drop it from the V3 until it's
ready for prime time.
Cheers,
-Paul
Paul Cercueil (4):
dt-bindings: rtc: Add #clock-cells property
rtc: jz4740: Use readl_poll_timeout
rtc: jz4740: Use dev_err_probe()
rtc: jz4740: Register clock provider for the CLK32K pin
.../devicetree/bindings/rtc/ingenic,rtc.yaml | 29 ++++++
drivers/rtc/Kconfig | 2 +-
drivers/rtc/rtc-jz4740.c | 94 ++++++++++++++-----
3 files changed, 99 insertions(+), 26 deletions(-)
--
2.39.0
The RTC in the JZ4770 is compatible with the JZ4760, but has an extra
register that permits to configure the behaviour of the CLK32K pin. The
same goes for the RTC in the JZ4780.
With this change, the RTC node is now also a clock provider on these
SoCs, so a #clock-cells property is added.
Signed-off-by: Paul Cercueil <[email protected]>
---
v2: - add constraint on which SoCs can have the #clock-cells property
- add JZ4780 example which has a #clock-cells
v3: Don't break ABI anymore.
---
.../devicetree/bindings/rtc/ingenic,rtc.yaml | 29 +++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/Documentation/devicetree/bindings/rtc/ingenic,rtc.yaml b/Documentation/devicetree/bindings/rtc/ingenic,rtc.yaml
index af78b67b3da4..de9879bdb317 100644
--- a/Documentation/devicetree/bindings/rtc/ingenic,rtc.yaml
+++ b/Documentation/devicetree/bindings/rtc/ingenic,rtc.yaml
@@ -11,6 +11,17 @@ maintainers:
allOf:
- $ref: rtc.yaml#
+ - if:
+ not:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - ingenic,jz4770-rtc
+ - ingenic,jz4780-rtc
+ then:
+ properties:
+ "#clock-cells": false
properties:
compatible:
@@ -39,6 +50,9 @@ properties:
clock-names:
const: rtc
+ "#clock-cells":
+ const: 0
+
system-power-controller:
description: |
Indicates that the RTC is responsible for powering OFF
@@ -83,3 +97,18 @@ examples:
clocks = <&cgu JZ4740_CLK_RTC>;
clock-names = "rtc";
};
+
+ - |
+ #include <dt-bindings/clock/ingenic,jz4780-cgu.h>
+ rtc: rtc@10003000 {
+ compatible = "ingenic,jz4780-rtc", "ingenic,jz4760-rtc";
+ reg = <0x10003000 0x4c>;
+
+ interrupt-parent = <&intc>;
+ interrupts = <32>;
+
+ clocks = <&cgu JZ4780_CLK_RTCLK>;
+ clock-names = "rtc";
+
+ #clock-cells = <0>;
+ };
--
2.39.0
Use readl_poll_timeout() from <iopoll.h> instead of using custom poll
loops.
The timeout settings are different, but that shouldn't be much of a
problem. Instead of polling 10000 times in a close loop, it polls for
one millisecond.
Signed-off-by: Paul Cercueil <[email protected]>
---
drivers/rtc/rtc-jz4740.c | 18 ++++++------------
1 file changed, 6 insertions(+), 12 deletions(-)
diff --git a/drivers/rtc/rtc-jz4740.c b/drivers/rtc/rtc-jz4740.c
index c383719292c7..4ee6e5ee09b1 100644
--- a/drivers/rtc/rtc-jz4740.c
+++ b/drivers/rtc/rtc-jz4740.c
@@ -7,6 +7,7 @@
#include <linux/clk.h>
#include <linux/io.h>
+#include <linux/iopoll.h>
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/of_device.h>
@@ -69,19 +70,15 @@ static inline uint32_t jz4740_rtc_reg_read(struct jz4740_rtc *rtc, size_t reg)
static int jz4740_rtc_wait_write_ready(struct jz4740_rtc *rtc)
{
uint32_t ctrl;
- int timeout = 10000;
- do {
- ctrl = jz4740_rtc_reg_read(rtc, JZ_REG_RTC_CTRL);
- } while (!(ctrl & JZ_RTC_CTRL_WRDY) && --timeout);
-
- return timeout ? 0 : -EIO;
+ return readl_poll_timeout(rtc->base + JZ_REG_RTC_CTRL, ctrl,
+ ctrl & JZ_RTC_CTRL_WRDY, 0, 1000);
}
static inline int jz4780_rtc_enable_write(struct jz4740_rtc *rtc)
{
uint32_t ctrl;
- int ret, timeout = 10000;
+ int ret;
ret = jz4740_rtc_wait_write_ready(rtc);
if (ret != 0)
@@ -89,11 +86,8 @@ static inline int jz4780_rtc_enable_write(struct jz4740_rtc *rtc)
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;
+ return readl_poll_timeout(rtc->base + JZ_REG_RTC_WENR, ctrl,
+ ctrl & JZ_RTC_WENR_WEN, 0, 1000);
}
static inline int jz4740_rtc_reg_write(struct jz4740_rtc *rtc, size_t reg,
--
2.39.0
Use dev_err_probe() where it makes sense to simplify a bit the code.
Signed-off-by: Paul Cercueil <[email protected]>
---
drivers/rtc/rtc-jz4740.c | 20 +++++++-------------
1 file changed, 7 insertions(+), 13 deletions(-)
diff --git a/drivers/rtc/rtc-jz4740.c b/drivers/rtc/rtc-jz4740.c
index 4ee6e5ee09b1..9ffa764aa71e 100644
--- a/drivers/rtc/rtc-jz4740.c
+++ b/drivers/rtc/rtc-jz4740.c
@@ -329,17 +329,13 @@ static int jz4740_rtc_probe(struct platform_device *pdev)
device_init_wakeup(dev, 1);
ret = dev_pm_set_wake_irq(dev, irq);
- if (ret) {
- dev_err(dev, "Failed to set wake irq: %d\n", ret);
- return ret;
- }
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to set wake irq\n");
rtc->rtc = devm_rtc_allocate_device(dev);
- if (IS_ERR(rtc->rtc)) {
- ret = PTR_ERR(rtc->rtc);
- dev_err(dev, "Failed to allocate rtc device: %d\n", ret);
- return ret;
- }
+ if (IS_ERR(rtc->rtc))
+ return dev_err_probe(dev, PTR_ERR(rtc->rtc),
+ "Failed to allocate rtc device\n");
rtc->rtc->ops = &jz4740_rtc_ops;
rtc->rtc->range_max = U32_MAX;
@@ -356,10 +352,8 @@ static int jz4740_rtc_probe(struct platform_device *pdev)
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);
- return ret;
- }
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to request rtc irq\n");
if (of_device_is_system_power_controller(np)) {
dev_for_power_off = dev;
--
2.39.0
On JZ4770 and JZ4780, the CLK32K pin is configurable. By default, it is
configured as a GPIO in input mode, and its value can be read through
GPIO PD14.
With this change, clients can now request the 32 kHz clock on the CLK32K
pin, through Device Tree. This clock is simply a pass-through of the
input oscillator's clock with enable/disable operations.
This will permit the WiFi/Bluetooth chip to work on the MIPS CI20 board,
which does source one of its clocks from the CLK32K pin.
Signed-off-by: Paul Cercueil <[email protected]>
---
v3: - Use dev_err_probe()
- Use __clk_hw_get() to get a pointer to the parent clock, instead
of doing it by name.
- Add Kconfig dependency on CONFIG_COMMON_CLK
- Register CLK32K clock if the #clock-cells device property is
present, instead of doing it based on the compatible string
---
drivers/rtc/Kconfig | 2 +-
drivers/rtc/rtc-jz4740.c | 56 ++++++++++++++++++++++++++++++++++++++++
2 files changed, 57 insertions(+), 1 deletion(-)
diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 677d2601d305..d2b6d20a6745 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -1690,7 +1690,7 @@ config RTC_DRV_MPC5121
config RTC_DRV_JZ4740
tristate "Ingenic JZ4740 SoC"
depends on MIPS || COMPILE_TEST
- depends on OF
+ depends on OF && COMMON_CLK
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 9ffa764aa71e..59d279e3e6f5 100644
--- a/drivers/rtc/rtc-jz4740.c
+++ b/drivers/rtc/rtc-jz4740.c
@@ -6,6 +6,7 @@
*/
#include <linux/clk.h>
+#include <linux/clk-provider.h>
#include <linux/io.h>
#include <linux/iopoll.h>
#include <linux/kernel.h>
@@ -13,6 +14,7 @@
#include <linux/of_device.h>
#include <linux/platform_device.h>
#include <linux/pm_wakeirq.h>
+#include <linux/property.h>
#include <linux/reboot.h>
#include <linux/rtc.h>
#include <linux/slab.h>
@@ -26,6 +28,7 @@
#define JZ_REG_RTC_WAKEUP_FILTER 0x24
#define JZ_REG_RTC_RESET_COUNTER 0x28
#define JZ_REG_RTC_SCRATCHPAD 0x34
+#define JZ_REG_RTC_CKPCR 0x40
/* The following are present on the jz4780 */
#define JZ_REG_RTC_WENR 0x3C
@@ -45,6 +48,9 @@
#define JZ_RTC_WAKEUP_FILTER_MASK 0x0000FFE0
#define JZ_RTC_RESET_COUNTER_MASK 0x00000FE0
+#define JZ_RTC_CKPCR_CK32PULL_DIS BIT(4)
+#define JZ_RTC_CKPCR_CK32CTL_EN (BIT(2) | BIT(1))
+
enum jz4740_rtc_type {
ID_JZ4740,
ID_JZ4760,
@@ -57,6 +63,8 @@ struct jz4740_rtc {
struct rtc_device *rtc;
+ struct clk_hw clk32k;
+
spinlock_t lock;
};
@@ -254,6 +262,7 @@ static void jz4740_rtc_power_off(void)
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 },
+ { .compatible = "ingenic,jz4770-rtc", .data = (void *)ID_JZ4780 },
{ .compatible = "ingenic,jz4780-rtc", .data = (void *)ID_JZ4780 },
{},
};
@@ -295,6 +304,38 @@ static void jz4740_rtc_set_wakeup_params(struct jz4740_rtc *rtc,
jz4740_rtc_reg_write(rtc, JZ_REG_RTC_RESET_COUNTER, reset_ticks);
}
+static int jz4740_rtc_clk32k_enable(struct clk_hw *hw)
+{
+ struct jz4740_rtc *rtc = container_of(hw, struct jz4740_rtc, clk32k);
+
+ return jz4740_rtc_reg_write(rtc, JZ_REG_RTC_CKPCR,
+ JZ_RTC_CKPCR_CK32PULL_DIS |
+ JZ_RTC_CKPCR_CK32CTL_EN);
+}
+
+static void jz4740_rtc_clk32k_disable(struct clk_hw *hw)
+{
+ struct jz4740_rtc *rtc = container_of(hw, struct jz4740_rtc, clk32k);
+
+ jz4740_rtc_reg_write(rtc, JZ_REG_RTC_CKPCR, 0);
+}
+
+static int jz4740_rtc_clk32k_is_enabled(struct clk_hw *hw)
+{
+ struct jz4740_rtc *rtc = container_of(hw, struct jz4740_rtc, clk32k);
+ u32 ckpcr;
+
+ ckpcr = jz4740_rtc_reg_read(rtc, JZ_REG_RTC_CKPCR);
+
+ return !!(ckpcr & JZ_RTC_CKPCR_CK32CTL_EN);
+}
+
+static const struct clk_ops jz4740_rtc_clk32k_ops = {
+ .enable = jz4740_rtc_clk32k_enable,
+ .disable = jz4740_rtc_clk32k_disable,
+ .is_enabled = jz4740_rtc_clk32k_is_enabled,
+};
+
static int jz4740_rtc_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
@@ -364,6 +405,21 @@ static int jz4740_rtc_probe(struct platform_device *pdev)
dev_warn(dev, "Poweroff handler already present!\n");
}
+ if (device_property_present(dev, "#clock-cells")) {
+ rtc->clk32k.init = CLK_HW_INIT_HW("clk32k", __clk_get_hw(clk),
+ &jz4740_rtc_clk32k_ops, 0);
+
+ ret = devm_clk_hw_register(dev, &rtc->clk32k);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "Unable to register clk32k clock\n");
+
+ ret = of_clk_add_hw_provider(np, of_clk_hw_simple_get, &rtc->clk32k);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "Unable to register clk32k clock provider\n");
+ }
+
return 0;
}
--
2.39.0
On Sun, 29 Jan 2023 12:04:39 +0000, Paul Cercueil wrote:
> The RTC in the JZ4770 is compatible with the JZ4760, but has an extra
> register that permits to configure the behaviour of the CLK32K pin. The
> same goes for the RTC in the JZ4780.
>
> With this change, the RTC node is now also a clock provider on these
> SoCs, so a #clock-cells property is added.
>
> Signed-off-by: Paul Cercueil <[email protected]>
>
> ---
> v2: - add constraint on which SoCs can have the #clock-cells property
> - add JZ4780 example which has a #clock-cells
> v3: Don't break ABI anymore.
> ---
> .../devicetree/bindings/rtc/ingenic,rtc.yaml | 29 +++++++++++++++++++
> 1 file changed, 29 insertions(+)
>
Reviewed-by: Rob Herring <[email protected]>
On Sun, 29 Jan 2023 12:04:38 +0000, Paul Cercueil wrote:
> Here's a revised patchset that introduces a few updates to the
> jz4740-rtc driver.
>
> Patch [1/4] used to break ABI, it does not anymore.
> Patch [2/4] did not change, patch [3/4] is new.
>
> Patch [3/4] has been updated to use dev_err_probe(), use __clk_hw_get()
> instead of looking up the parent's clock by name, and will now register
> the CLK32K clock when the #clock-cells device property is present
> instead of doing it based on the compatible string.
>
> [...]
Applied, thanks!
[1/4] dt-bindings: rtc: Add #clock-cells property
commit: 4737a703528c769c4fde6b68462f656f91f4ad99
[2/4] rtc: jz4740: Use readl_poll_timeout
commit: d644b133f78d6d8efd36f7b1703bebca09036f0b
[3/4] rtc: jz4740: Use dev_err_probe()
commit: ff6fd3770e9687d7b849a0e826a32563bfcb98da
[4/4] rtc: jz4740: Register clock provider for the CLK32K pin
commit: 5ddfa148de8cf5491fd1c89522c7cad859db8c88
Best regards,
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com