2022-04-22 23:22:46

by Jonathan Neuschäfer

[permalink] [raw]
Subject: [PATCH 0/7] Nuvoton WPCM450 clock and reset driver

This series adds support for the clock and reset controller in the Nuvoton
WPCM450 SoC. This means that the clock rates for peripherals will be calculated
automatically based on the clock tree as it was preconfigured by the bootloader.
The 24 MHz dummy clock, that is currently in the devicetree, is no longer needed.
Somewhat unfortunately, this also means that there is a breaking change once
the devicetree starts relying on the clock driver, but I find it acceptable in
this case, because WPCM450 is still at a somewhat early stage.


Upstreaming plan (although other suggestions are welcome):

Once reviewed,

- The ARM/dts changes should go through Joel Stanley's bmc tree
- The clocksource/timer changes should probably go via Daniel Lezcano and TIP
- The watchdog patch should go via the watchdog tree
- The clock controller bindings and driver should go through the clk tree
- It might make sense to delay the final ARM/dts patch ("ARM: dts: wpcm450:
Switch clocks to clock controller") until next cycle to make sure it is
merged after the clock driver.

Jonathan Neuschäfer (7):
dt-bindings: timer: nuvoton,npcm7xx-timer: Allow specifying all clocks
clocksource: timer-npcm7xx: Enable timer 1 clock before use
watchdog: npcm: Enable clock if provided
dt-bindings: clock: Add Nuvoton WPCM450 clock/reset controller
ARM: dts: wpcm450: Add clock controller node
clk: wpcm450: Add Nuvoton WPCM450 clock/reset controller driver
ARM: dts: wpcm450: Switch clocks to clock controller

.../bindings/clock/nuvoton,wpcm450-clk.yaml | 74 ++++
.../bindings/timer/nuvoton,npcm7xx-timer.yaml | 8 +-
arch/arm/boot/dts/nuvoton-wpcm450.dtsi | 29 +-
drivers/clk/Makefile | 1 +
drivers/clk/clk-wpcm450.c | 378 ++++++++++++++++++
drivers/clocksource/timer-npcm7xx.c | 14 +-
drivers/reset/Kconfig | 2 +-
drivers/watchdog/npcm_wdt.c | 9 +
.../dt-bindings/clock/nuvoton,wpcm450-clk.h | 67 ++++
9 files changed, 572 insertions(+), 10 deletions(-)
create mode 100644 Documentation/devicetree/bindings/clock/nuvoton,wpcm450-clk.yaml
create mode 100644 drivers/clk/clk-wpcm450.c
create mode 100644 include/dt-bindings/clock/nuvoton,wpcm450-clk.h

--
2.35.1


2022-04-22 23:22:52

by Jonathan Neuschäfer

[permalink] [raw]
Subject: [PATCH 2/7] clocksource: timer-npcm7xx: Enable timer 1 clock before use

In the WPCM450 SoC, the clocks for each timer can be gated individually.
To prevent the timer 1 clock from being gated, enable it explicitly.

Signed-off-by: Jonathan Neuschäfer <[email protected]>
---
drivers/clocksource/timer-npcm7xx.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/clocksource/timer-npcm7xx.c b/drivers/clocksource/timer-npcm7xx.c
index a00520cbb660a..974269b6b0c36 100644
--- a/drivers/clocksource/timer-npcm7xx.c
+++ b/drivers/clocksource/timer-npcm7xx.c
@@ -188,17 +188,29 @@ static void __init npcm7xx_clocksource_init(void)

static int __init npcm7xx_timer_init(struct device_node *np)
{
+ struct clk *clk;
int ret;

ret = timer_of_init(np, &npcm7xx_to);
- if (ret)
+ if (ret) {
+ pr_warn("timer_of_init failed: %d\n", ret);
return ret;
+ }

/* Clock input is divided by PRESCALE + 1 before it is fed */
/* to the counter */
npcm7xx_to.of_clk.rate = npcm7xx_to.of_clk.rate /
(NPCM7XX_Tx_MIN_PRESCALE + 1);

+ /* Enable the clock for timer1, if it exists */
+ clk = of_clk_get(np, 1);
+ if (clk) {
+ if (!IS_ERR(clk))
+ clk_prepare_enable(clk);
+ else
+ pr_warn("Failed to get clock for timer1: %pe", clk);
+ }
+
npcm7xx_clocksource_init();
npcm7xx_clockevents_init();

--
2.35.1

2022-04-22 23:23:00

by Jonathan Neuschäfer

[permalink] [raw]
Subject: [PATCH 3/7] watchdog: npcm: Enable clock if provided

On the Nuvoton WPCM450 SoC, with its upcoming clock driver, peripheral
clocks are individually gated and ungated. Therefore, the watchdog
driver must be able to ungate the watchdog clock.

Signed-off-by: Jonathan Neuschäfer <[email protected]>
---
drivers/watchdog/npcm_wdt.c | 9 +++++++++
1 file changed, 9 insertions(+)

diff --git a/drivers/watchdog/npcm_wdt.c b/drivers/watchdog/npcm_wdt.c
index 28a24caa2627c..6d27f0e16188e 100644
--- a/drivers/watchdog/npcm_wdt.c
+++ b/drivers/watchdog/npcm_wdt.c
@@ -3,6 +3,7 @@
// Copyright (c) 2018 IBM Corp.

#include <linux/bitops.h>
+#include <linux/clk.h>
#include <linux/delay.h>
#include <linux/interrupt.h>
#include <linux/kernel.h>
@@ -180,6 +181,7 @@ static int npcm_wdt_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
struct npcm_wdt *wdt;
+ struct clk *clk;
int irq;
int ret;

@@ -191,6 +193,13 @@ static int npcm_wdt_probe(struct platform_device *pdev)
if (IS_ERR(wdt->reg))
return PTR_ERR(wdt->reg);

+ clk = devm_clk_get_optional(&pdev->dev, NULL);
+ if (IS_ERR(clk))
+ return PTR_ERR(clk);
+
+ if (clk)
+ clk_prepare_enable(clk);
+
irq = platform_get_irq(pdev, 0);
if (irq < 0)
return irq;
--
2.35.1

2022-04-29 11:53:30

by Zev Weiss

[permalink] [raw]
Subject: Re: [PATCH 2/7] clocksource: timer-npcm7xx: Enable timer 1 clock before use

On Fri, Apr 22, 2022 at 11:30:07AM PDT, Jonathan Neusch?fer wrote:
>In the WPCM450 SoC, the clocks for each timer can be gated individually.
>To prevent the timer 1 clock from being gated, enable it explicitly.
>
>Signed-off-by: Jonathan Neusch?fer <[email protected]>
>---
> drivers/clocksource/timer-npcm7xx.c | 14 +++++++++++++-
> 1 file changed, 13 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/clocksource/timer-npcm7xx.c b/drivers/clocksource/timer-npcm7xx.c
>index a00520cbb660a..974269b6b0c36 100644
>--- a/drivers/clocksource/timer-npcm7xx.c
>+++ b/drivers/clocksource/timer-npcm7xx.c
>@@ -188,17 +188,29 @@ static void __init npcm7xx_clocksource_init(void)
>
> static int __init npcm7xx_timer_init(struct device_node *np)
> {
>+ struct clk *clk;
> int ret;
>
> ret = timer_of_init(np, &npcm7xx_to);
>- if (ret)
>+ if (ret) {
>+ pr_warn("timer_of_init failed: %d\n", ret);

This seems like a somewhat opaque message to emit, especially given this
file's lack of a pr_fmt() definition -- maybe add a %pOF so it's
slightly easier to trace back to the device it stems from?

> return ret;
>+ }
>
> /* Clock input is divided by PRESCALE + 1 before it is fed */
> /* to the counter */
> npcm7xx_to.of_clk.rate = npcm7xx_to.of_clk.rate /
> (NPCM7XX_Tx_MIN_PRESCALE + 1);
>
>+ /* Enable the clock for timer1, if it exists */
>+ clk = of_clk_get(np, 1);
>+ if (clk) {
>+ if (!IS_ERR(clk))
>+ clk_prepare_enable(clk);
>+ else
>+ pr_warn("Failed to get clock for timer1: %pe", clk);

Likewise here (though to a slightly lesser extent).

>+ }
>+
> npcm7xx_clocksource_init();
> npcm7xx_clockevents_init();
>
>--
>2.35.1
>

2022-04-29 20:14:01

by Jonathan Neuschäfer

[permalink] [raw]
Subject: Re: [PATCH 2/7] clocksource: timer-npcm7xx: Enable timer 1 clock before use

On Thu, Apr 28, 2022 at 09:11:58AM +0000, Zev Weiss wrote:
> On Fri, Apr 22, 2022 at 11:30:07AM PDT, Jonathan Neuschäfer wrote:
> >In the WPCM450 SoC, the clocks for each timer can be gated individually.
> >To prevent the timer 1 clock from being gated, enable it explicitly.
> >
> >Signed-off-by: Jonathan Neuschäfer <[email protected]>
> >---
> > drivers/clocksource/timer-npcm7xx.c | 14 +++++++++++++-
> > 1 file changed, 13 insertions(+), 1 deletion(-)
> >
> >diff --git a/drivers/clocksource/timer-npcm7xx.c b/drivers/clocksource/timer-npcm7xx.c
> >index a00520cbb660a..974269b6b0c36 100644
> >--- a/drivers/clocksource/timer-npcm7xx.c
> >+++ b/drivers/clocksource/timer-npcm7xx.c
> >@@ -188,17 +188,29 @@ static void __init npcm7xx_clocksource_init(void)
> >
> > static int __init npcm7xx_timer_init(struct device_node *np)
> > {
> >+ struct clk *clk;
> > int ret;
> >
> > ret = timer_of_init(np, &npcm7xx_to);
> >- if (ret)
> >+ if (ret) {
> >+ pr_warn("timer_of_init failed: %d\n", ret);
>
> This seems like a somewhat opaque message to emit, especially given this
> file's lack of a pr_fmt() definition -- maybe add a %pOF so it's
> slightly easier to trace back to the device it stems from?

Now that I look at this code again, I think I should just drop the
pr_warn entirely, since I didn't mention it in the description, and it's
unrelated to enabling the clock.

> > return ret;
> >+ }
> >
> > /* Clock input is divided by PRESCALE + 1 before it is fed */
> > /* to the counter */
> > npcm7xx_to.of_clk.rate = npcm7xx_to.of_clk.rate /
> > (NPCM7XX_Tx_MIN_PRESCALE + 1);
> >
> >+ /* Enable the clock for timer1, if it exists */
> >+ clk = of_clk_get(np, 1);
> >+ if (clk) {
> >+ if (!IS_ERR(clk))
> >+ clk_prepare_enable(clk);
> >+ else
> >+ pr_warn("Failed to get clock for timer1: %pe", clk);
>
> Likewise here (though to a slightly lesser extent).

I'll add %pOF here.


Thanks,
Jonathan


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