mbox series

[v5,0/6] Nuvoton WPCM450 clock and reset driver

Message ID 20221104161850.2889894-1-j.neuschaefer@gmx.net
Headers show
Series Nuvoton WPCM450 clock and reset driver | expand

Message

J. Neuschäfer Nov. 4, 2022, 4:18 p.m. UTC 
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 clock controller bindings and driver should go through the clk tree
- It probably makes 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.


v5:
- Dropped patch 2 (watchdog: npcm: Enable clock if provided), which
  was since merged upstream
- Added patch 2 (clocksource: timer-npcm7xx: Enable timer 1 clock before use) again,
  because I wasn't able to find it in linux-next
- Switched the driver to using struct clk_parent_data
- Rebased on 6.1-rc3

v4:
- https://lore.kernel.org/lkml/20220610072141.347795-1-j.neuschaefer@gmx.net/
- Leave WDT clock running during after restart handler
- Fix reset controller initialization
- Dropped patch 2/7 (clocksource: timer-npcm7xx: Enable timer 1 clock before use),
  as it was applied by Daniel Lezcano

v3:
- https://lore.kernel.org/lkml/20220508194333.2170161-1-j.neuschaefer@gmx.net/
- Changed "refclk" string to "ref"
- Fixed some dead code in the driver
- Added clk_prepare_enable call to the watchdog restart handler
- Added a few review tags

v2:
- https://lore.kernel.org/lkml/20220429172030.398011-1-j.neuschaefer@gmx.net/
- various small improvements

v1:
- https://lore.kernel.org/lkml/20220422183012.444674-1-j.neuschaefer@gmx.net/

Jonathan Neuschäfer (6):
  dt-bindings: timer: nuvoton,npcm7xx-timer: Allow specifying all clocks
  clocksource: timer-npcm7xx: Enable timer 1 clock before use
  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
  [NOT FOR MERGE] ARM: dts: wpcm450: Switch clocks to clock controller

 .../bindings/clock/nuvoton,wpcm450-clk.yaml   |  66 +++
 .../bindings/timer/nuvoton,npcm7xx-timer.yaml |   8 +-
 arch/arm/boot/dts/nuvoton-wpcm450.dtsi        |  29 +-
 drivers/clk/Makefile                          |   1 +
 drivers/clk/clk-wpcm450.c                     | 375 ++++++++++++++++++
 drivers/clocksource/timer-npcm7xx.c           |  10 +
 drivers/reset/Kconfig                         |   2 +-
 .../dt-bindings/clock/nuvoton,wpcm450-clk.h   |  67 ++++
 8 files changed, 549 insertions(+), 9 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

Comments

Joel Stanley Nov. 22, 2022, 1:40 a.m. UTC | #1 
On Fri, 4 Nov 2022 at 16:21, Jonathan Neuschäfer <j.neuschaefer@gmx.net> wrote:
>
> 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.

Reviewed-by: Joel Stanley <joel@jms.id.au>

>
>
> Upstreaming plan (although other suggestions are welcome):
>
> Once reviewed,
>
> - The ARM/dts changes should go through Joel Stanley's bmc tree

I've picked up the standalone patch ("Add clock controller node").

> - The clocksource/timer changes should probably go via Daniel Lezcano and TIP
> - The clock controller bindings and driver should go through the clk tree

Stephen, do you plan on merging this driver for v6.2?

Alternatively, could we get an ack from you and merge this entire
series through Arnd, where we have precedent for merging these initial
support cross-tree patch sets?



> - It probably makes 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.
>
>
> v5:
> - Dropped patch 2 (watchdog: npcm: Enable clock if provided), which
>   was since merged upstream
> - Added patch 2 (clocksource: timer-npcm7xx: Enable timer 1 clock before use) again,
>   because I wasn't able to find it in linux-next
> - Switched the driver to using struct clk_parent_data
> - Rebased on 6.1-rc3
>
> v4:
> - https://lore.kernel.org/lkml/20220610072141.347795-1-j.neuschaefer@gmx.net/
> - Leave WDT clock running during after restart handler
> - Fix reset controller initialization
> - Dropped patch 2/7 (clocksource: timer-npcm7xx: Enable timer 1 clock before use),
>   as it was applied by Daniel Lezcano
>
> v3:
> - https://lore.kernel.org/lkml/20220508194333.2170161-1-j.neuschaefer@gmx.net/
> - Changed "refclk" string to "ref"
> - Fixed some dead code in the driver
> - Added clk_prepare_enable call to the watchdog restart handler
> - Added a few review tags
>
> v2:
> - https://lore.kernel.org/lkml/20220429172030.398011-1-j.neuschaefer@gmx.net/
> - various small improvements
>
> v1:
> - https://lore.kernel.org/lkml/20220422183012.444674-1-j.neuschaefer@gmx.net/
>
> Jonathan Neuschäfer (6):
>   dt-bindings: timer: nuvoton,npcm7xx-timer: Allow specifying all clocks
>   clocksource: timer-npcm7xx: Enable timer 1 clock before use
>   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
>   [NOT FOR MERGE] ARM: dts: wpcm450: Switch clocks to clock controller
>
>  .../bindings/clock/nuvoton,wpcm450-clk.yaml   |  66 +++
>  .../bindings/timer/nuvoton,npcm7xx-timer.yaml |   8 +-
>  arch/arm/boot/dts/nuvoton-wpcm450.dtsi        |  29 +-
>  drivers/clk/Makefile                          |   1 +
>  drivers/clk/clk-wpcm450.c                     | 375 ++++++++++++++++++
>  drivers/clocksource/timer-npcm7xx.c           |  10 +
>  drivers/reset/Kconfig                         |   2 +-
>  .../dt-bindings/clock/nuvoton,wpcm450-clk.h   |  67 ++++
>  8 files changed, 549 insertions(+), 9 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
>
Daniel Lezcano Nov. 22, 2022, 8:09 a.m. UTC | #2 
On 22/11/2022 02:40, Joel Stanley wrote:
> On Fri, 4 Nov 2022 at 16:21, Jonathan Neuschäfer <j.neuschaefer@gmx.net> wrote:
>>
>> 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.
> 
> Reviewed-by: Joel Stanley <joel@jms.id.au>
> 
>>
>>
>> Upstreaming plan (although other suggestions are welcome):
>>
>> Once reviewed,
>>
>> - The ARM/dts changes should go through Joel Stanley's bmc tree
> 
> I've picked up the standalone patch ("Add clock controller node").
> 
>> - The clocksource/timer changes should probably go via Daniel Lezcano and TIP

I picked the timer change along with the binding
Stephen Boyd Dec. 9, 2022, 8:21 p.m. UTC | #3 
Quoting Jonathan Neuschäfer (2022-11-04 09:18:49)
> This driver implements the following features w.r.t. the clock and reset
> controller in the WPCM450 SoC:
> 
> - It calculates the rates for all clocks managed by the clock controller
> - It leaves the clock tree mostly unchanged, except that it enables/
>   disables clock gates based on usage.
> - It exposes the reset lines managed by the controller using the
>   Generic Reset Controller subsystem
> 
> NOTE: If the driver and the corresponding devicetree node are present,
>       the driver will disable "unused" clocks. This is problem until
>       the clock relations are properly declared in the devicetree (in a
>       later patch). Until then, the clk_ignore_unused kernel parameter
>       can be used as a workaround.
> 
> Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
> ---

Applied to clk-next
Tomer Maimon Dec. 12, 2022, 7:30 a.m. UTC | #4 
Hi Jonathan,

Thanks for your patch, and sorry for the late reply.

On Fri, 4 Nov 2022 at 18:19, Jonathan Neuschäfer <j.neuschaefer@gmx.net> wrote:
>
> This driver implements the following features w.r.t. the clock and reset
> controller in the WPCM450 SoC:
>
> - It calculates the rates for all clocks managed by the clock controller
> - It leaves the clock tree mostly unchanged, except that it enables/
>   disables clock gates based on usage.
> - It exposes the reset lines managed by the controller using the
>   Generic Reset Controller subsystem
>
> NOTE: If the driver and the corresponding devicetree node are present,
>       the driver will disable "unused" clocks. This is problem until
>       the clock relations are properly declared in the devicetree (in a
>       later patch). Until then, the clk_ignore_unused kernel parameter
>       can be used as a workaround.
>
> Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
> ---
>
> v5:
> - Switch to using clk_parent_data
>
> v4:
> - https://lore.kernel.org/lkml/20220610072141.347795-6-j.neuschaefer@gmx.net/
> - Fix reset controller initialization
>
> v3:
> - https://lore.kernel.org/lkml/20220508194333.2170161-7-j.neuschaefer@gmx.net/
> - Change reference clock name from "refclk" to "ref"
> - Remove unused variable in return path of wpcm450_clk_register_pll
> - Remove unused divisor tables
>
> v2:
> - https://lore.kernel.org/lkml/20220429172030.398011-7-j.neuschaefer@gmx.net/
> - no changes
> ---
>  drivers/clk/Makefile      |   1 +
>  drivers/clk/clk-wpcm450.c | 375 ++++++++++++++++++++++++++++++++++++++
>  drivers/reset/Kconfig     |   2 +-
>  3 files changed, 377 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/clk/clk-wpcm450.c
>
> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
> index e3ca0d058a256..b58352d4d615d 100644
> --- a/drivers/clk/Makefile
> +++ b/drivers/clk/Makefile
> @@ -75,6 +75,7 @@ obj-$(CONFIG_COMMON_CLK_RS9_PCIE)     += clk-renesas-pcie.o
>  obj-$(CONFIG_COMMON_CLK_VC5)           += clk-versaclock5.o
>  obj-$(CONFIG_COMMON_CLK_VC7)           += clk-versaclock7.o
>  obj-$(CONFIG_COMMON_CLK_WM831X)                += clk-wm831x.o
> +obj-$(CONFIG_ARCH_WPCM450)             += clk-wpcm450.o
>  obj-$(CONFIG_COMMON_CLK_XGENE)         += clk-xgene.o
>
>  # please keep this section sorted lexicographically by directory path name
> diff --git a/drivers/clk/clk-wpcm450.c b/drivers/clk/clk-wpcm450.c
> new file mode 100644
> index 0000000000000..b5e81b3b6b982
> --- /dev/null
> +++ b/drivers/clk/clk-wpcm450.c
> @@ -0,0 +1,375 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Nuvoton WPCM450 clock and reset controller driver.
> + *
> + * Copyright (C) 2022 Jonathan Neuschäfer <j.neuschaefer@gmx.net>
> + */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/bitfield.h>
> +#include <linux/clk-provider.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/reset-controller.h>
> +#include <linux/reset/reset-simple.h>
> +#include <linux/slab.h>
> +
> +#include <dt-bindings/clock/nuvoton,wpcm450-clk.h>
> +
> +struct wpcm450_clk_pll {
> +       struct clk_hw hw;
> +       void __iomem *pllcon;
> +       u8 flags;
> +};
> +
> +#define to_wpcm450_clk_pll(_hw) container_of(_hw, struct wpcm450_clk_pll, hw)
> +
> +#define PLLCON_FBDV    GENMASK(24, 16)
> +#define PLLCON_PRST    BIT(13)
> +#define PLLCON_PWDEN   BIT(12)
> +#define PLLCON_OTDV    GENMASK(10, 8)
> +#define PLLCON_INDV    GENMASK(5, 0)
> +
> +static unsigned long wpcm450_clk_pll_recalc_rate(struct clk_hw *hw,
> +                                                unsigned long parent_rate)
> +{
> +       struct wpcm450_clk_pll *pll = to_wpcm450_clk_pll(hw);
> +       unsigned long fbdv, indv, otdv;
> +       u64 rate;
> +       u32 pllcon;
> +
> +       if (parent_rate == 0) {
> +               pr_err("%s: parent rate is zero", __func__);
> +               return 0;
> +       }
> +
> +       pllcon = readl_relaxed(pll->pllcon);
> +
> +       indv = FIELD_GET(PLLCON_INDV, pllcon) + 1;
> +       fbdv = FIELD_GET(PLLCON_FBDV, pllcon) + 1;
> +       otdv = FIELD_GET(PLLCON_OTDV, pllcon) + 1;
> +
> +       rate = (u64)parent_rate * fbdv;
> +       do_div(rate, indv * otdv);
> +
> +       return rate;
> +}
> +
> +static int wpcm450_clk_pll_is_enabled(struct clk_hw *hw)
> +{
> +       struct wpcm450_clk_pll *pll = to_wpcm450_clk_pll(hw);
> +       u32 pllcon;
> +
> +       pllcon = readl_relaxed(pll->pllcon);
> +
> +       return !(pllcon & PLLCON_PRST);
> +}
> +
> +static void wpcm450_clk_pll_disable(struct clk_hw *hw)
> +{
> +       struct wpcm450_clk_pll *pll = to_wpcm450_clk_pll(hw);
> +       u32 pllcon;
> +
> +       pllcon = readl_relaxed(pll->pllcon);
> +       pllcon |= PLLCON_PRST | PLLCON_PWDEN;
> +       writel(pllcon, pll->pllcon);
> +}
> +
> +static const struct clk_ops wpcm450_clk_pll_ops = {
> +       .recalc_rate = wpcm450_clk_pll_recalc_rate,
> +       .is_enabled = wpcm450_clk_pll_is_enabled,
> +       .disable = wpcm450_clk_pll_disable
> +};
> +
> +static struct clk_hw *
> +wpcm450_clk_register_pll(void __iomem *pllcon, const char *name,
> +                        const struct clk_parent_data *parent, unsigned long flags)
> +{
> +       struct wpcm450_clk_pll *pll;
> +       struct clk_init_data init = {};
> +       int ret;
> +
> +       pll = kzalloc(sizeof(*pll), GFP_KERNEL);
> +       if (!pll)
> +               return ERR_PTR(-ENOMEM);
> +
> +       init.name = name;
> +       init.ops = &wpcm450_clk_pll_ops;
> +       init.parent_data = parent;
> +       init.num_parents = 1;
> +       init.flags = flags;
> +
> +       pll->pllcon = pllcon;
> +       pll->hw.init = &init;
> +
> +       ret = clk_hw_register(NULL, &pll->hw);
> +       if (ret) {
> +               kfree(pll);
> +               return ERR_PTR(ret);
> +       }
> +
> +       return &pll->hw;
> +}
> +
> +#define REG_CLKEN      0x00
> +#define REG_CLKSEL     0x04
> +#define REG_CLKDIV     0x08
> +#define REG_PLLCON0    0x0c
> +#define REG_PLLCON1    0x10
> +#define REG_PMCON      0x14
> +#define REG_IRQWAKECON 0x18
> +#define REG_IRQWAKEFLAG        0x1c
> +#define REG_IPSRST     0x20
> +
> +struct wpcm450_pll_data {
> +       const char *name;
> +       struct clk_parent_data parent;
> +       unsigned int reg;
> +       unsigned long flags;
> +};
> +
> +static const struct wpcm450_pll_data pll_data[] = {
> +       { "pll0", { .name = "ref" }, REG_PLLCON0, 0 },
> +       { "pll1", { .name = "ref" }, REG_PLLCON1, 0 },
> +};
> +
> +struct wpcm450_clksel_data {
> +       const char *name;
> +       const struct clk_parent_data *parents;
> +       unsigned int num_parents;
> +       const u32 *table;
> +       int shift;
> +       int width;
> +       int index;
> +       unsigned long flags;
> +};
> +
> +static const u32 parent_table[] = { 0, 1, 2 };
> +
> +static const struct clk_parent_data default_parents[] = {
> +       { .name = "pll0" },
> +       { .name = "pll1" },
> +       { .name = "ref" },
> +};
> +
> +static const struct clk_parent_data huart_parents[] = {
> +       { .name = "ref" },
> +       { .name = "refdiv2" },
> +};
> +
> +static const struct wpcm450_clksel_data clksel_data[] = {
> +       { "cpusel", default_parents, ARRAY_SIZE(default_parents),
> +               parent_table, 0, 2, -1, CLK_IS_CRITICAL },
> +       { "clkout", default_parents, ARRAY_SIZE(default_parents),
> +               parent_table, 2, 2, -1, 0 },
> +       { "usbphy", default_parents, ARRAY_SIZE(default_parents),
> +               parent_table, 6, 2, -1, 0 },
> +       { "uartsel", default_parents, ARRAY_SIZE(default_parents),
> +               parent_table, 8, 2, WPCM450_CLK_USBPHY, 0 },
> +       { "huartsel", huart_parents, ARRAY_SIZE(huart_parents),
> +               parent_table, 10, 1, -1, 0 },
> +};
> +
> +static const struct clk_div_table div_fixed2[] = {
> +       { .val = 0, .div = 2 },
> +       { }
> +};
> +
> +struct wpcm450_clkdiv_data {
> +       const char *name;
> +       struct clk_parent_data parent;
> +       int div_flags;
> +       const struct clk_div_table *table;
> +       int shift;
> +       int width;
> +       unsigned long flags;
> +};
> +
> +static struct wpcm450_clkdiv_data clkdiv_data_early[] = {
> +       { "refdiv2", { .name = "ref" }, 0, div_fixed2, 0, 0 },
> +};
> +
> +static const struct wpcm450_clkdiv_data clkdiv_data[] = {
> +       { "cpu", { .name = "cpusel" }, 0, div_fixed2, 0, 0, CLK_IS_CRITICAL },
> +       { "adcdiv", { .name = "ref" }, CLK_DIVIDER_POWER_OF_TWO, NULL, 28, 2, 0 },
> +       { "apb", { .name = "ahb" }, CLK_DIVIDER_POWER_OF_TWO, NULL, 26, 2, 0 },
> +       { "ahb", { .name = "cpu" }, CLK_DIVIDER_POWER_OF_TWO, NULL, 24, 2, 0 },
> +       { "uart", { .name = "uartsel" }, 0, NULL, 16, 4, 0 },
> +       { "ahb3", { .name = "ahb" }, CLK_DIVIDER_POWER_OF_TWO, NULL, 8, 2, 0 },
> +};
> +
> +struct wpcm450_clken_data {
> +       const char *name;
> +       struct clk_parent_data parent;
> +       int bitnum;
> +       unsigned long flags;
> +};
> +
> +static const struct wpcm450_clken_data clken_data[] = {
> +       { "fiu", { .name = "ahb3" }, WPCM450_CLK_FIU, 0 },
> +       { "xbus", { .name = "ahb3" }, WPCM450_CLK_XBUS, 0 },
> +       { "kcs", { .name = "apb" }, WPCM450_CLK_KCS, 0 },
> +       { "shm", { .name = "ahb3" }, WPCM450_CLK_SHM, 0 },
> +       { "usb1", { .name = "ahb" }, WPCM450_CLK_USB1, 0 },
> +       { "emc0", { .name = "ahb" }, WPCM450_CLK_EMC0, 0 },
> +       { "emc1", { .name = "ahb" }, WPCM450_CLK_EMC1, 0 },
> +       { "usb0", { .name = "ahb" }, WPCM450_CLK_USB0, 0 },
> +       { "peci", { .name = "apb" }, WPCM450_CLK_PECI, 0 },
> +       { "aes", { .name = "apb" }, WPCM450_CLK_AES, 0 },
> +       { "uart0", { .name = "uart" }, WPCM450_CLK_UART0, 0 },
> +       { "uart1", { .name = "uart" }, WPCM450_CLK_UART1, 0 },
> +       { "smb2", { .name = "apb" }, WPCM450_CLK_SMB2, 0 },
> +       { "smb3", { .name = "apb" }, WPCM450_CLK_SMB3, 0 },
> +       { "smb4", { .name = "apb" }, WPCM450_CLK_SMB4, 0 },
> +       { "smb5", { .name = "apb" }, WPCM450_CLK_SMB5, 0 },
> +       { "huart", { .name = "huartsel" }, WPCM450_CLK_HUART, 0 },
> +       { "pwm", { .name = "apb" }, WPCM450_CLK_PWM, 0 },
> +       { "timer0", { .name = "refdiv2" }, WPCM450_CLK_TIMER0, 0 },
> +       { "timer1", { .name = "refdiv2" }, WPCM450_CLK_TIMER1, 0 },
> +       { "timer2", { .name = "refdiv2" }, WPCM450_CLK_TIMER2, 0 },
> +       { "timer3", { .name = "refdiv2" }, WPCM450_CLK_TIMER3, 0 },
> +       { "timer4", { .name = "refdiv2" }, WPCM450_CLK_TIMER4, 0 },
> +       { "mft0", { .name = "apb" }, WPCM450_CLK_MFT0, 0 },
> +       { "mft1", { .name = "apb" }, WPCM450_CLK_MFT1, 0 },
> +       { "wdt", { .name = "refdiv2" }, WPCM450_CLK_WDT, 0 },
> +       { "adc", { .name = "adcdiv" }, WPCM450_CLK_ADC, 0 },
> +       { "sdio", { .name = "ahb" }, WPCM450_CLK_SDIO, 0 },
> +       { "sspi", { .name = "apb" }, WPCM450_CLK_SSPI, 0 },
> +       { "smb0", { .name = "apb" }, WPCM450_CLK_SMB0, 0 },
> +       { "smb1", { .name = "apb" }, WPCM450_CLK_SMB1, 0 },
> +};
> +
> +static DEFINE_SPINLOCK(wpcm450_clk_lock);
> +
> +static void __init wpcm450_clk_init(struct device_node *clk_np)
> +{
> +       struct clk_hw_onecell_data *clk_data;
> +       static struct clk_hw **hws;
> +       static struct clk_hw *hw;
> +       void __iomem *clk_base;
> +       int i, ret;
> +       struct reset_simple_data *reset;
> +
> +       clk_base = of_iomap(clk_np, 0);
> +       if (!clk_base) {
> +               pr_err("%pOFP: failed to map registers\n", clk_np);
> +               of_node_put(clk_np);
> +               return;
> +       }
> +       of_node_put(clk_np);
> +
> +       clk_data = kzalloc(struct_size(clk_data, hws, WPCM450_NUM_CLKS), GFP_KERNEL);
> +       if (!clk_data)
> +               goto err_unmap;
> +
> +       clk_data->num = WPCM450_NUM_CLKS;
> +       hws = clk_data->hws;
> +
> +       for (i = 0; i < WPCM450_NUM_CLKS; i++)
> +               hws[i] = ERR_PTR(-ENOENT);
> +
> +       // PLLs
> +       for (i = 0; i < ARRAY_SIZE(pll_data); i++) {
> +               const struct wpcm450_pll_data *data = &pll_data[i];
> +
> +               hw = wpcm450_clk_register_pll(clk_base + data->reg, data->name,
> +                                             &data->parent, data->flags);
> +               if (IS_ERR(hw)) {
> +                       pr_info("Failed to register PLL: %pe", hw);
> +                       goto err_free;
> +               }
> +       }
> +
> +       // Early divisors (REF/2)
> +       for (i = 0; i < ARRAY_SIZE(clkdiv_data_early); i++) {
> +               const struct wpcm450_clkdiv_data *data = &clkdiv_data_early[i];
> +
> +               hw = clk_hw_register_divider_table_parent_data(NULL, data->name, &data->parent,
> +                                                              data->flags, clk_base + REG_CLKDIV,
> +                                                              data->shift, data->width,
> +                                                              data->div_flags, data->table,
> +                                                              &wpcm450_clk_lock);
> +               if (IS_ERR(hw)) {
> +                       pr_err("Failed to register div table: %pe\n", hw);
> +                       goto err_free;
> +               }
> +       }
> +
> +       // Selects/muxes
> +       for (i = 0; i < ARRAY_SIZE(clksel_data); i++) {
> +               const struct wpcm450_clksel_data *data = &clksel_data[i];
> +
> +               hw = clk_hw_register_mux_parent_data(NULL, data->name, data->parents,
> +                                                    data->num_parents, data->flags,
> +                                                    clk_base + REG_CLKSEL, data->shift,
> +                                                    data->width, 0,
> +                                                    &wpcm450_clk_lock);
> +               if (IS_ERR(hw)) {
> +                       pr_err("Failed to register mux: %pe\n", hw);
> +                       goto err_free;
> +               }
> +               if (data->index >= 0)
> +                       clk_data->hws[data->index] = hw;
> +       }
> +
> +       // Divisors
> +       for (i = 0; i < ARRAY_SIZE(clkdiv_data); i++) {
> +               const struct wpcm450_clkdiv_data *data = &clkdiv_data[i];
> +
> +               hw = clk_hw_register_divider_table_parent_data(NULL, data->name, &data->parent,
> +                                                              data->flags, clk_base + REG_CLKDIV,
> +                                                              data->shift, data->width,
> +                                                              data->div_flags, data->table,
> +                                                              &wpcm450_clk_lock);
> +               if (IS_ERR(hw)) {
> +                       pr_err("Failed to register divider: %pe\n", hw);
> +                       goto err_free;
> +               }
> +       }
> +
> +       // Enables/gates
> +       for (i = 0; i < ARRAY_SIZE(clken_data); i++) {
> +               const struct wpcm450_clken_data *data = &clken_data[i];
> +
> +               hw = clk_hw_register_gate_parent_data(NULL, data->name, &data->parent, data->flags,
> +                                                     clk_base + REG_CLKEN, data->bitnum,
> +                                                     data->flags, &wpcm450_clk_lock);
> +               if (IS_ERR(hw)) {
> +                       pr_err("Failed to register gate: %pe\n", hw);
> +                       goto err_free;
> +               }
> +               clk_data->hws[data->bitnum] = hw;
> +       }
> +
> +       ret = of_clk_add_hw_provider(clk_np, of_clk_hw_onecell_get, clk_data);
> +       if (ret)
> +               pr_err("Failed to add DT provider: %d\n", ret);
> +
> +       // Reset controller
> +       reset = kzalloc(sizeof(*reset), GFP_KERNEL);
> +       if (!reset)
> +               goto err_free;
> +       reset->rcdev.owner = THIS_MODULE;
> +       reset->rcdev.nr_resets = WPCM450_NUM_RESETS;
> +       reset->rcdev.ops = &reset_simple_ops;
> +       reset->rcdev.of_node = clk_np;
> +       reset->membase = clk_base + REG_IPSRST;
> +       ret = reset_controller_register(&reset->rcdev);
> +       if (ret)
> +               pr_err("Failed to register reset controller: %d\n", ret);
> +
> +       of_node_put(clk_np);
> +       return;
> +
> +err_free:
> +       kfree(clk_data->hws);
> +err_unmap:
> +       iounmap(clk_base);
> +       of_node_put(clk_np);
> +}
> +
> +CLK_OF_DECLARE(wpcm450_clk_init, "nuvoton,wpcm450-clk", wpcm450_clk_init);
> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> index de176c2fbad96..de5662830fce8 100644
> --- a/drivers/reset/Kconfig
> +++ b/drivers/reset/Kconfig
> @@ -208,7 +208,7 @@ config RESET_SCMI
>
>  config RESET_SIMPLE
>         bool "Simple Reset Controller Driver" if COMPILE_TEST || EXPERT
> -       default ARCH_ASPEED || ARCH_BCMBCA || ARCH_BITMAIN || ARCH_REALTEK || ARCH_STM32 || (ARCH_INTEL_SOCFPGA && ARM64) || ARCH_SUNXI || ARC
> +       default ARCH_ASPEED || ARCH_BCMBCA || ARCH_BITMAIN || ARCH_REALTEK || ARCH_STM32 || (ARCH_INTEL_SOCFPGA && ARM64) || ARCH_SUNXI || ARC || ARCH_NPCM
Please modify the default ARCH_NPCM to the specific Hermon use,
ARCH_WPCM450, since NPCM7XX and NPCM8XX use a specific NPCM reset
driver.
https://elixir.bootlin.com/linux/v6.1-rc8/source/drivers/reset/reset-npcm.c
we prefer not to have two reset drivers when compiling ARCH_NPCM.
>         depends on HAS_IOMEM
>         help
>           This enables a simple reset controller driver for reset lines that
> --
> 2.35.1
>

Best regards,

Tomer
J. Neuschäfer Dec. 13, 2022, 2:53 p.m. UTC | #5 
On Mon, Dec 12, 2022 at 09:30:46AM +0200, Tomer Maimon wrote:
> Hi Jonathan,
> 
> Thanks for your patch, and sorry for the late reply.
> 
> On Fri, 4 Nov 2022 at 18:19, Jonathan Neuschäfer <j.neuschaefer@gmx.net> wrote:
[...]
> > diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> > index de176c2fbad96..de5662830fce8 100644
> > --- a/drivers/reset/Kconfig
> > +++ b/drivers/reset/Kconfig
> > @@ -208,7 +208,7 @@ config RESET_SCMI
> >
> >  config RESET_SIMPLE
> >         bool "Simple Reset Controller Driver" if COMPILE_TEST || EXPERT
> > -       default ARCH_ASPEED || ARCH_BCMBCA || ARCH_BITMAIN || ARCH_REALTEK || ARCH_STM32 || (ARCH_INTEL_SOCFPGA && ARM64) || ARCH_SUNXI || ARC
> > +       default ARCH_ASPEED || ARCH_BCMBCA || ARCH_BITMAIN || ARCH_REALTEK || ARCH_STM32 || (ARCH_INTEL_SOCFPGA && ARM64) || ARCH_SUNXI || ARC || ARCH_NPCM
> Please modify the default ARCH_NPCM to the specific Hermon use,
> ARCH_WPCM450, since NPCM7XX and NPCM8XX use a specific NPCM reset
> driver.
> https://elixir.bootlin.com/linux/v6.1-rc8/source/drivers/reset/reset-npcm.c
> we prefer not to have two reset drivers when compiling ARCH_NPCM.

Good point. I'll fix that.


Thanks,
J.