Introduce Nuvoton Arbel NPCM8XX BMC SoC
Subsystems (2)
Tomer Maimon
maintainerv14 cover Introduce Nuvoton Arbel NPCM8XX BMC SoC
This patchset adds clock support for the Nuvoton Arbel NPCM8XX Board Management controller (BMC) SoC family.
This patchset cover letter is based from the initial support for NPCM8xx BMC to keep tracking the version history.
all the other initial support patches had been applied to Linux kernel 6.0.
This patchset was tested on the Arbel NPCM8XX evaluation board.
Addressed comments from:
- Stephen Boyd: https://www.spinics.net/lists/kernel/msg4620417.html
Changes since version 13:
- NPCM8XX clock driver
- Remove unnecessary definitions and add module.h define
- Use in clk_parent_data struct.fw_name and .name.
- Add module_exit function.
- Add const to divider clock names.
- Add MODULE_DESCRIPTION and MODULE_LICENSE
Changes since version 12:
- NPCM8XX clock driver
- Use clk_parent_data in mux and div clock structure.
- Add const to mux tables.
- Using devm_clk_hw_register_fixed_rate function.
- use only .name clk_parent_data instead .name and .fw_name.
- Modify mask values in mux clocks.
Changes since version 11:
- NPCM8XX clock driver
- Modify Kconfig help.
- Modify loop variable to unsigned int.
Changes since version 11:
- NPCM8XX clock driver
- Modify Kconfig help.
- Modify loop variable to unsigned int.
Changes since version 10:
- NPCM8XX clock driver
- Fix const warning.
Changes since version 9:
- NPCM8XX clock driver
- Move configuration place.
- Using clk_parent_data instead of parent_name
- using devm_ioremap instead of ioremap. deeply sorry, I know we had a long discussion on what should the driver use, from other examples (also in other clock drivers) I see the combination of platform_get_resource and devm_ioremap are commonly used and it answer the reset and clock needs.
Changes since version 8:
- NPCM8XX clock driver
- Move configuration place.
- Add space before and aftre '{' '}'.
- Handle devm_of_clk_add_hw_provider function error.
Changes since version 7:
- NPCM8XX clock driver
- The clock and reset registers using the same memory region, due to it the clock driver should claim the ioremap directly without checking the memory region.
Changes since version 6:
- NPCM reset driver
- Modify warning message.
- dt-bindings: serial: 8250: Add npcm845 compatible string patch accepted, due to it the patch removed from the patchset.
Changes since version 5:
- NPCM8XX clock driver
- Remove refclk if devm_of_clk_add_hw_provider function failed.
- NPCM8XX clock source driver
- Remove NPCM8XX TIMER_OF_DECLARE support, using the same as NPCM7XX.
Changes since version 4:
- NPCM8XX clock driver
- Use the same quote in the dt-binding file.
Changes since version 3:
- NPCM8XX clock driver
- Rename NPCM8xx clock dt-binding header file.
- Remove unused structures.
- Improve Handling the clocks registration.
- NPCM reset driver
- Add ref phandle to dt-binding.
Changes since version 2:
- Remove NPCM8xx WDT compatible patch.
- Remove NPCM8xx UART compatible patch.
- NPCM8XX clock driver
- Add debug new line.
- Add 25M fixed rate clock.
- Remove unused clocks and clock name from dt-binding.
- NPCM reset driver
- Revert to npcm7xx dt-binding.
- Skip dt binding quotes.
- Adding DTS backward compatibility.
- Remove NPCM8xx binding include file.
- Warp commit message.
- NPCM8XX device tree:
- Remove unused clock nodes (used in the clock driver)
- Modify gcr and rst node names.
Changes since version 1:
- NPCM8XX clock driver
- Modify dt-binding.
- Remove unsed definition and include.
- Include alphabetically.
- Use clock devm.
- NPCM reset driver
- Modify dt-binding.
- Modify syscon name.
- Add syscon support to NPCM7XX dts reset node.
- use data structure.
- NPCM8XX device tree:
- Modify evb compatible name.
- Add NPCM7xx compatible.
- Remove disable nodes from the EVB DTS.
Tomer Maimon (1):
clk: npcm8xx: add clock controller
drivers/clk/Kconfig | 8 + drivers/clk/Makefile | 1 + drivers/clk/clk-npcm8xx.c | 650 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 659 insertions(+) create mode 100644 drivers/clk/clk-npcm8xx.c
Signature · 1 line
2.33.0
Tomer Maimon
maintainerPATCH v14 1/1 clk: npcm8xx: add clock controller
Nuvoton Arbel BMC NPCM8XX contains an integrated clock controller which generates and supplies clocks to all modules within the BMC.
--- drivers/clk/Kconfig | 8 + drivers/clk/Makefile | 1 + drivers/clk/clk-npcm8xx.c | 650 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 659 insertions(+) create mode 100644 drivers/clk/clk-npcm8xx.c
drivers/clk/Kconfig +8 -0
@@ -326,6 +326,14 @@ config COMMON_CLK_LOCHNAGAR This driver supports the clocking features of the Cirrus Logic Lochnagar audio development board. +config COMMON_CLK_NPCM8XX + tristate "Clock driver for the NPCM8XX SoC Family" + depends on ARCH_NPCM || COMPILE_TEST + help + This driver supports the clocks on the Nuvoton BMC NPCM8XX SoC Family, + all the clocks are initialized by the bootloader, so this driver + allows only reading of current settings directly from the hardware. + config COMMON_CLK_NXP def_bool COMMON_CLK && (ARCH_LPC18XX || ARCH_LPC32XX) select REGMAP_MMIO if ARCH_LPC32XX
drivers/clk/Makefile +1 -0
@@ -49,6 +49,7 @@ obj-$(CONFIG_ARCH_MILBEAUT_M10V) += clk-milbeaut.o obj-$(CONFIG_ARCH_MOXART) += clk-moxart.o obj-$(CONFIG_ARCH_NOMADIK) += clk-nomadik.o obj-$(CONFIG_ARCH_NPCM7XX) += clk-npcm7xx.o +obj-$(CONFIG_COMMON_CLK_NPCM8XX) += clk-npcm8xx.o obj-$(CONFIG_ARCH_NSPIRE) += clk-nspire.o obj-$(CONFIG_COMMON_CLK_OXNAS) += clk-oxnas.o obj-$(CONFIG_COMMON_CLK_PALMAS) += clk-palmas.o
drivers/clk/clk-npcm8xx.c new file +650 -0
650 lines not on this page · raw message
Signature · 1 line
2.33.0
Stephen Boyd
maintainerRe: [PATCH v14 1/1] clk: npcm8xx: add clock controller
Quoting Tomer Maimon (2022-12-11 12:43:24)
Quoted 8 lines
drivers/clk/clk-npcm8xx.c new file +2
-0@@ -0,0 +1,650 @@ +// SPDX-License-Identifier: GPL-2.0 +/*
[...]
drivers/clk/clk-npcm8xx.c line 221
Quoted 5 lines
drivers/clk/clk-npcm8xx.c new file +5
-0+#define NPCM8XX_CLK_S_RCP "rcp" + +static const u32 pll_mux_table[] = {0 ,1 ,2 ,3 }; +static const struct clk_parent_data pll_mux_parents[] = { + { .fw_name =NPCM8XX_CLK_S_PLL0 , .name =NPCM8XX_CLK_S_PLL0 },
As this is a new driver either you should only have .fw_name here. The .name field is a backup to migrate code over to a new binding. When .fw_name is used there should be an associated DT binding update. I doubt the usage of .fw_name is correct though, because aren't these clks internal to the controller? The .fw_name field is about describing parents that are an input to the clk controller node in DT (because the controller is a consumer of these clks that are external to the device).
So can you use the .hw field for these internal clks? Check out CLK_HW_INIT_HWS() macro and friends for a possible way to initialize this.
drivers/clk/clk-npcm8xx.c
Quoted 2 lines
drivers/clk/clk-npcm8xx.c new file +2
-0+ { .fw_name = NPCM8XX_CLK_S_PLL1, .name = NPCM8XX_CLK_S_PLL1 }, + { .fw_name = NPCM8XX_CLK_S_REFCLK, .name = NPCM8XX_CLK_S_REFCLK },
Maybe this is external? If so, it would be great to have this in the binding as a clocks property.
Tomer Maimon
maintainerRe: [PATCH v14 1/1] clk: npcm8xx: add clock controller
Hi Stephen,
Very sorry for the late reply.
Quoted 11 lines · Stephen Boyd
Quoting Tomer Maimon (2022-12-11 12:43:24)
Quoted 8 lines
drivers/clk/clk-npcm8xx.c new file +2
-0@@ -0,0 +1,650 @@ +// SPDX-License-Identifier: GPL-2.0 +/* [...]
Quoted 9 lines
drivers/clk/clk-npcm8xx.c line 221
Quoted 5 lines
drivers/clk/clk-npcm8xx.c new file +5
-0+#define NPCM8XX_CLK_S_RCP "rcp" + +static const u32 pll_mux_table[] = {0 ,1 ,2 ,3 }; +static const struct clk_parent_data pll_mux_parents[] = { + { .fw_name =NPCM8XX_CLK_S_PLL0 , .name =NPCM8XX_CLK_S_PLL0 },As this is a new driver either you should only have .fw_name here. The .name field is a backup to migrate code over to a new binding. When .fw_name is used there should be an associated DT binding update. I
What do you mean by associated DT binding? does the.fw_name, for example, NPCM8XX_CLK_S_PLL0 need to represent in the device tree?
Quoted 2 lines
doubt the usage of .fw_name is correct though, because aren't these clks internal to the controller? The .fw_name field is about describing does the
yes the PLL clocks are internal.
Quoted 6 lines
parents that are an input to the clk controller node in DT (because the controller is a consumer of these clks that are external to the device).
So can you use the .hw field for these internal clks? Check out
CLK_HW_INIT_HWS()macro and friends for a possible way to initialize this.
but still, I have used devm_clk_hw_register_mux_parent_data_table function to register the clock mux,
should I use devm_clk_hw_register_mux_parent_hws function instead? Does this modification need to be done in all the mux parent struct? could you point me to some example in the Linux kernel how do you think that I should represent the mux clock in the NPCM8XX clock driver?
Quoted 6 lines
drivers/clk/clk-npcm8xx.c
Quoted 3 lines
drivers/clk/clk-npcm8xx.c new file +2
-0+ { .fw_name = NPCM8XX_CLK_S_PLL1, .name = NPCM8XX_CLK_S_PLL1 }, + { .fw_name = NPCM8XX_CLK_S_REFCLK, .name = NPCM8XX_CLK_S_REFCLK },Maybe this is external? If so, it would be great to have this in the binding as a
clocksproperty.
O.K.
Thanks,
Tomer
Tomer Maimon
maintainerRe: [PATCH v14 1/1] clk: npcm8xx: add clock controller
Hi Stephen,
Kind reminder regarding the NPCM clock questions
Thanks,
Tomer
Quoted 55 lines · Tomer Maimon
Hi Stephen,
Very sorry for the late reply.
Quoted 11 lines · Stephen Boyd
Quoting Tomer Maimon (2022-12-11 12:43:24)
Quoted 8 lines
drivers/clk/clk-npcm8xx.c new file +2
-0@@ -0,0 +1,650 @@ +// SPDX-License-Identifier: GPL-2.0 +/* [...]
Quoted 9 lines
drivers/clk/clk-npcm8xx.c line 221
Quoted 5 lines
drivers/clk/clk-npcm8xx.c new file +5
-0+#define NPCM8XX_CLK_S_RCP "rcp" + +static const u32 pll_mux_table[] = {0 ,1 ,2 ,3 }; +static const struct clk_parent_data pll_mux_parents[] = { + { .fw_name =NPCM8XX_CLK_S_PLL0 , .name =NPCM8XX_CLK_S_PLL0 },As this is a new driver either you should only have .fw_name here. The .name field is a backup to migrate code over to a new binding. When .fw_name is used there should be an associated DT binding update. I
What do you mean by associated DT binding? does the.fw_name, for example,
NPCM8XX_CLK_S_PLL0need to represent in the device tree?Quoted 2 lines
doubt the usage of .fw_name is correct though, because aren't these clks internal to the controller? The .fw_name field is about describing does the
yes the PLL clocks are internal.
Quoted 6 lines
parents that are an input to the clk controller node in DT (because the controller is a consumer of these clks that are external to the device).
So can you use the .hw field for these internal clks? Check out
CLK_HW_INIT_HWS()macro and friends for a possible way to initialize this.but still, I have used
devm_clk_hw_register_mux_parent_data_tablefunction to register the clock mux,
should I usedevm_clk_hw_register_mux_parent_hwsfunction instead? Does this modification need to be done in all the mux parent struct? could you point me to some example in the Linux kernel how do you think that I should represent the mux clock in the NPCM8XX clock driver?Quoted 6 lines
drivers/clk/clk-npcm8xx.c
Quoted 3 lines
drivers/clk/clk-npcm8xx.c new file +2
-0+ { .fw_name = NPCM8XX_CLK_S_PLL1, .name = NPCM8XX_CLK_S_PLL1 }, + { .fw_name = NPCM8XX_CLK_S_REFCLK, .name = NPCM8XX_CLK_S_REFCLK },Maybe this is external? If so, it would be great to have this in the binding as a
clocksproperty.O.K.
Thanks,
Tomer
Stephen Boyd
maintainerRe: [PATCH v14 1/1] clk: npcm8xx: add clock controller
In reply to Tomer MaimonQuoting Tomer Maimon (2023-01-17 09:35:33)
Quoted 25 lines
Hi Stephen,
Very sorry for the late reply.
Quoted 11 lines · Stephen Boyd
Quoting Tomer Maimon (2022-12-11 12:43:24)
Quoted 8 lines
drivers/clk/clk-npcm8xx.c new file +2
-0@@ -0,0 +1,650 @@ +// SPDX-License-Identifier: GPL-2.0 +/* [...]
Quoted 9 lines
drivers/clk/clk-npcm8xx.c line 221
Quoted 5 lines
drivers/clk/clk-npcm8xx.c new file +5
-0+#define NPCM8XX_CLK_S_RCP "rcp" + +static const u32 pll_mux_table[] = {0 ,1 ,2 ,3 }; +static const struct clk_parent_data pll_mux_parents[] = { + { .fw_name =NPCM8XX_CLK_S_PLL0 , .name =NPCM8XX_CLK_S_PLL0 },As this is a new driver either you should only have .fw_name here. The .name field is a backup to migrate code over to a new binding. When .fw_name is used there should be an associated DT binding update. I
Quoted 2 lines
What do you mean by associated DT binding? does the.fw_name, for example,
NPCM8XX_CLK_S_PLL0need to represent in the device tree?
Yes it should match a string in the "clock-names" property for this clk provider's device node.
Quoted 3 lines
Quoted 2 lines
doubt the usage of .fw_name is correct though, because aren't these clks internal to the controller? The .fw_name field is about describing does the
yes the PLL clocks are internal.
Ok.
Quoted 9 lines
Quoted 6 lines
parents that are an input to the clk controller node in DT (because the controller is a consumer of these clks that are external to the device).
So can you use the .hw field for these internal clks? Check out
CLK_HW_INIT_HWS()macro and friends for a possible way to initialize this.but still, I have used
devm_clk_hw_register_mux_parent_data_tablefunction to register the clock mux,
should I usedevm_clk_hw_register_mux_parent_hwsfunction instead?
Probably, yes.
Quoted 4 lines
Does this modification need to be done in all the mux parent struct? could you point me to some example in the Linux kernel how do you think that I should represent the mux clock in the NPCM8XX clock driver?
I don't know. If the clk is external to the provider, then it should be in .fw_name or .index and be provided through DT. Otherwise, if the clk is internal to the clk provider use direct pointers.
Quoted 8 lines
Quoted 6 lines
drivers/clk/clk-npcm8xx.c
Quoted 3 lines
drivers/clk/clk-npcm8xx.c new file +2
-0+ { .fw_name = NPCM8XX_CLK_S_PLL1, .name = NPCM8XX_CLK_S_PLL1 }, + { .fw_name = NPCM8XX_CLK_S_REFCLK, .name = NPCM8XX_CLK_S_REFCLK },Maybe this is external? If so, it would be great to have this in the binding as a
clocksproperty.O.K.
Is it external? If so, then fw_name would be correct. You can look at the kernel-doc above clk_core_get(), but I really just need to spend a few hours and write a proper kernel-doc for this stuff.