This patchset adds clock support for the Nuvoton
Arbel NPCM8XX Board Management controller (BMC) SoC family.
The NPCM8xx clock controller is created using the auxiliary device framework
and set up in the npcm reset driver since the NPCM8xx clock is using the
same register region.
This patchset cover letter is based from the initial support for NPCM8xx BMC to
keep tracking the version history.
This patchset was tested on the Arbel NPCM8XX evaluation board.
Addressed comments from:
- Stephen Boyd: https://www.spinics.net/lists/linux-clk/msg96261.html
Changes since version 23:
- NPCM8xx clock controller using the auxiliary device framework.
- Add NPCM8xx clock controller aux device registration support in npcm reset driver.
- Remove unused nuvoton,npcm845 clk bindings.
- Remove all string #define
Changes since version 22:
- Modify commit message to explain broken ABI in dt-binding
- Using regmap parenet regmap memory therefore remove use of npcm8xx rst-clock patch.
- Leave npcm7xx rst node as is
Changes since version 21:
- Since using regmap instead of ioremap replace reg to syscon
property in dt-bindings and dts.
- Add reference clock property to the dt-bindings and dts.
- Using .index instead of .name in clk_parent_data structures.
- Using string where any macros are used once.
Changes since version 20:
- Using regmap instead of ioremap.
the clock and reset modules are sharing the same memory region
and cause failure when using devm_platform_ioremap_resource
function, this version uses regmap to handle shared
reset and clock memory region, in case it is approved I will
modify the reset driver to use the regmap as well.
- Using clk_hw instead of clk_parent_data structre.
- Divider clock definition to one line
Changes since version 19:
- Remove unnecessary free command.
- Defining pr_fmt().
- Using dev_err_probe.
- Return zero in the end of the probe function.
Changes since version 18:
- NPCM8XX clock driver did not changed from version 18 only build and tested under kernel 6.6-rc1.
Changes since version 17:
- NPCM8XX clock driver did not changed from version 17 only build and tested under kernel 6.5-rc3.
Changes since version 16:
- NPCM8XX clock driver
- Using devm_kzalloc instead kzalloc.
- Remove unnecessary parenthesis.
- Modify incorrect spelling.
Changes since version 15:
- NPCM8XX clock driver
- Remove unused regs parameter from npcm8xx_pll_data structure.
- Using index and clk_hw parameters to set the clock parent in the clock structures.
Changes since version 14:
- NPCM8XX clock driver
- Remove unnecessary register definitions.
- Remove the internal reference clock, instead use the external DT reference clock.
- rearrange the driver.
- using .names parameter in DT to define clock (refclk).
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 5:
- NPCM8XX clock driver
- Remove refclk if devm_of_clk_add_hw_provider function failed.
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.
Changes since version 2:
- NPCM8XX clock driver
- Add debug new line.
- Add 25M fixed rate clock.
- Remove unused clocks and clock name from dt-binding.
Changes since version 1:
- NPCM8XX clock driver
- Modify dt-binding.
- Remove unsed definition and include.
- Include alphabetically.
- Use clock devm.
Tomer Maimon (4):
dt-bindings: reset: npcm: add clock properties
reset: npcm: register npcm8xx clock auxiliary bus device
clk: npcm8xx: add clock controller
dt-binding: clock: remove nuvoton npcm845-clk bindings
.../bindings/clock/nuvoton,npcm845-clk.yaml | 49 --
.../bindings/reset/nuvoton,npcm750-reset.yaml | 18 +
drivers/clk/Kconfig | 8 +
drivers/clk/Makefile | 1 +
drivers/clk/clk-npcm8xx.c | 429 ++++++++++++++++++
drivers/reset/reset-npcm.c | 72 ++-
include/soc/nuvoton/clock-npcm8xx.h | 16 +
7 files changed, 543 insertions(+), 50 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml
create mode 100644 drivers/clk/clk-npcm8xx.c
create mode 100644 include/soc/nuvoton/clock-npcm8xx.h
--
2.34.1
Remove nuvoton,npcm845-clk binding since the NPCM8xx clock driver
using the auxiliary device framework and not the device tree framework.
Signed-off-by: Tomer Maimon <[email protected]>
---
.../bindings/clock/nuvoton,npcm845-clk.yaml | 49 -------------------
1 file changed, 49 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml
diff --git a/Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml b/Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml
deleted file mode 100644
index b901ca13cd25..000000000000
--- a/Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml
+++ /dev/null
@@ -1,49 +0,0 @@
-# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
-%YAML 1.2
----
-$id: http://devicetree.org/schemas/clock/nuvoton,npcm845-clk.yaml#
-$schema: http://devicetree.org/meta-schemas/core.yaml#
-
-title: Nuvoton NPCM8XX Clock Controller
-
-maintainers:
- - Tomer Maimon <[email protected]>
-
-description: |
- Nuvoton Arbel BMC NPCM8XX contains an integrated clock controller, which
- generates and supplies clocks to all modules within the BMC.
-
-properties:
- compatible:
- enum:
- - nuvoton,npcm845-clk
-
- reg:
- maxItems: 1
-
- '#clock-cells':
- const: 1
- description:
- See include/dt-bindings/clock/nuvoton,npcm8xx-clock.h for the full
- list of NPCM8XX clock IDs.
-
-required:
- - compatible
- - reg
- - '#clock-cells'
-
-additionalProperties: false
-
-examples:
- - |
- ahb {
- #address-cells = <2>;
- #size-cells = <2>;
-
- clock-controller@f0801000 {
- compatible = "nuvoton,npcm845-clk";
- reg = <0x0 0xf0801000 0x0 0x1000>;
- #clock-cells = <1>;
- };
- };
-...
--
2.34.1
Adding 25MHz reference clock and clock-cell properties to NPCM reset
document due to the registration of the npcm8xx clock auxiliary bus device
in the NPCM reset driver
The NPCM8xx clock auxiliary bus device has been registered in the NPCM
reset driver because the reset and the clock share the same register
region.
Signed-off-by: Tomer Maimon <[email protected]>
---
.../bindings/reset/nuvoton,npcm750-reset.yaml | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/Documentation/devicetree/bindings/reset/nuvoton,npcm750-reset.yaml b/Documentation/devicetree/bindings/reset/nuvoton,npcm750-reset.yaml
index d82e65e37cc0..18db4de13098 100644
--- a/Documentation/devicetree/bindings/reset/nuvoton,npcm750-reset.yaml
+++ b/Documentation/devicetree/bindings/reset/nuvoton,npcm750-reset.yaml
@@ -21,6 +21,13 @@ properties:
'#reset-cells':
const: 2
+ '#clock-cells':
+ const: 1
+
+ clocks:
+ items:
+ - description: specify external 25MHz referance clock.
+
nuvoton,sysgcr:
$ref: /schemas/types.yaml#/definitions/phandle
description: a phandle to access GCR registers.
@@ -39,6 +46,17 @@ required:
- '#reset-cells'
- nuvoton,sysgcr
+if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - nuvoton,npcm845-reset
+then:
+ required:
+ - '#clock-cells'
+ - clocks
+
additionalProperties: false
examples:
--
2.34.1
Add NPCM8xx clock controller auxiliary bus device registration.
The NPCM8xx clock controller is registered as an aux device because the
reset and the clock controller share the same register region.
Signed-off-by: Tomer Maimon <[email protected]>
---
drivers/reset/reset-npcm.c | 72 ++++++++++++++++++++++++++++-
include/soc/nuvoton/clock-npcm8xx.h | 16 +++++++
2 files changed, 87 insertions(+), 1 deletion(-)
create mode 100644 include/soc/nuvoton/clock-npcm8xx.h
diff --git a/drivers/reset/reset-npcm.c b/drivers/reset/reset-npcm.c
index 8935ef95a2d1..2db9e56485f6 100644
--- a/drivers/reset/reset-npcm.c
+++ b/drivers/reset/reset-npcm.c
@@ -15,6 +15,8 @@
#include <linux/regmap.h>
#include <linux/of_address.h>
+#include <soc/nuvoton/clock-npcm8xx.h>
+
/* NPCM7xx GCR registers */
#define NPCM_MDLR_OFFSET 0x7C
#define NPCM7XX_MDLR_USBD0 BIT(9)
@@ -89,6 +91,7 @@ struct npcm_rc_data {
const struct npcm_reset_info *info;
struct regmap *gcr_regmap;
u32 sw_reset_number;
+ struct device *dev;
void __iomem *base;
spinlock_t lock;
};
@@ -372,6 +375,67 @@ static const struct reset_control_ops npcm_rc_ops = {
.status = npcm_rc_status,
};
+static void npcm_clock_unregister_adev(void *_adev)
+{
+ struct auxiliary_device *adev = _adev;
+
+ auxiliary_device_delete(adev);
+ auxiliary_device_uninit(adev);
+}
+
+static void npcm_clock_adev_release(struct device *dev)
+{
+ struct auxiliary_device *adev = to_auxiliary_dev(dev);
+ struct npcm_clock_adev *rdev = to_npcm_clock_adev(adev);
+
+ kfree(rdev);
+}
+
+static struct auxiliary_device *npcm_clock_adev_alloc(struct npcm_rc_data *rst_data, char *clk_name)
+{
+ struct npcm_clock_adev *rdev;
+ struct auxiliary_device *adev;
+ int ret;
+
+ rdev = kzalloc(sizeof(*rdev), GFP_KERNEL);
+ if (!rdev)
+ return ERR_PTR(-ENOMEM);
+
+ rdev->base = rst_data->base;
+
+ adev = &rdev->adev;
+ adev->name = clk_name;
+ adev->dev.parent = rst_data->dev;
+ adev->dev.release = npcm_clock_adev_release;
+ adev->id = 555u;
+
+ ret = auxiliary_device_init(adev);
+ if (ret) {
+ kfree(adev);
+ return ERR_PTR(ret);
+ }
+
+ return adev;
+}
+
+static int npcm8xx_clock_controller_register(struct npcm_rc_data *rst_data, char *clk_name)
+{
+ struct auxiliary_device *adev;
+ int ret;
+
+ adev = npcm_clock_adev_alloc(rst_data, clk_name);
+ if (IS_ERR(adev))
+ return PTR_ERR(adev);
+
+ ret = auxiliary_device_add(adev);
+ if (ret) {
+ auxiliary_device_uninit(adev);
+ return ret;
+ }
+
+ return devm_add_action_or_reset(rst_data->dev, npcm_clock_unregister_adev, adev);
+}
+
static int npcm_rc_probe(struct platform_device *pdev)
{
struct npcm_rc_data *rc;
@@ -392,6 +456,7 @@ static int npcm_rc_probe(struct platform_device *pdev)
rc->rcdev.of_node = pdev->dev.of_node;
rc->rcdev.of_reset_n_cells = 2;
rc->rcdev.of_xlate = npcm_reset_xlate;
+ rc->dev = &pdev->dev;
ret = devm_reset_controller_register(&pdev->dev, &rc->rcdev);
if (ret) {
@@ -413,7 +478,12 @@ static int npcm_rc_probe(struct platform_device *pdev)
}
}
- return ret;
+ switch (rc->info->bmc_id) {
+ case BMC_NPCM8XX:
+ return npcm8xx_clock_controller_register(rc, "clk-npcm8xx");
+ default:
+ return ret;
+ }
}
static struct platform_driver npcm_rc_driver = {
diff --git a/include/soc/nuvoton/clock-npcm8xx.h b/include/soc/nuvoton/clock-npcm8xx.h
new file mode 100644
index 000000000000..139130e98c51
--- /dev/null
+++ b/include/soc/nuvoton/clock-npcm8xx.h
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __SOC_NPCM8XX_CLOCK_H
+#define __SOC_NPCM8XX_CLOCK_H
+
+#include <linux/auxiliary_bus.h>
+#include <linux/container_of.h>
+
+struct npcm_clock_adev {
+ void __iomem *base;
+ struct auxiliary_device adev;
+};
+
+#define to_npcm_clock_adev(_adev) \
+ container_of((_adev), struct npcm_clock_adev, adev)
+
+#endif
--
2.34.1
Hi Tomer,
kernel test robot noticed the following build warnings:
[auto build test WARNING on clk/clk-next]
[also build test WARNING on pza/reset/next linus/master v6.9-rc7 next-20240510]
[cannot apply to pza/imx-drm/next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Tomer-Maimon/dt-bindings-reset-npcm-add-clock-properties/20240510-072622
base: https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-next
patch link: https://lore.kernel.org/r/20240509192411.2432066-3-tmaimon77%40gmail.com
patch subject: [PATCH v24 2/4] reset: npcm: register npcm8xx clock auxiliary bus device
config: arm-wpcm450_defconfig (https://download.01.org/0day-ci/archive/20240510/[email protected]/config)
compiler: arm-linux-gnueabi-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240510/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All warnings (new ones prefixed by >>):
drivers/reset/reset-npcm.c: In function 'npcm_clock_adev_release':
drivers/reset/reset-npcm.c:391:9: error: implicit declaration of function 'kfree'; did you mean 'vfree'? [-Werror=implicit-function-declaration]
391 | kfree(rdev);
| ^~~~~
| vfree
drivers/reset/reset-npcm.c: In function 'npcm_clock_adev_alloc':
drivers/reset/reset-npcm.c:400:16: error: implicit declaration of function 'kzalloc'; did you mean 'vzalloc'? [-Werror=implicit-function-declaration]
400 | rdev = kzalloc(sizeof(*rdev), GFP_KERNEL);
| ^~~~~~~
| vzalloc
>> drivers/reset/reset-npcm.c:400:14: warning: assignment to 'struct npcm_clock_adev *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
400 | rdev = kzalloc(sizeof(*rdev), GFP_KERNEL);
| ^
cc1: some warnings being treated as errors
vim +400 drivers/reset/reset-npcm.c
393
394 static struct auxiliary_device *npcm_clock_adev_alloc(struct npcm_rc_data *rst_data, char *clk_name)
395 {
396 struct npcm_clock_adev *rdev;
397 struct auxiliary_device *adev;
398 int ret;
399
> 400 rdev = kzalloc(sizeof(*rdev), GFP_KERNEL);
401 if (!rdev)
402 return ERR_PTR(-ENOMEM);
403
404 rdev->base = rst_data->base;
405
406 adev = &rdev->adev;
407 adev->name = clk_name;
408 adev->dev.parent = rst_data->dev;
409 adev->dev.release = npcm_clock_adev_release;
410 adev->id = 555u;
411
412 ret = auxiliary_device_init(adev);
413 if (ret) {
414 kfree(adev);
415 return ERR_PTR(ret);
416 }
417
418 return adev;
419 }
420
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Hi Tomer,
kernel test robot noticed the following build warnings:
[auto build test WARNING on clk/clk-next]
[also build test WARNING on pza/reset/next linus/master v6.9-rc7 next-20240510]
[cannot apply to pza/imx-drm/next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Tomer-Maimon/dt-bindings-reset-npcm-add-clock-properties/20240510-072622
base: https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-next
patch link: https://lore.kernel.org/r/20240509192411.2432066-3-tmaimon77%40gmail.com
patch subject: [PATCH v24 2/4] reset: npcm: register npcm8xx clock auxiliary bus device
config: i386-buildonly-randconfig-002-20240510 (https://download.01.org/0day-ci/archive/20240510/[email protected]/config)
compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240510/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All warnings (new ones prefixed by >>):
drivers/reset/reset-npcm.c: In function 'npcm_clock_adev_release':
drivers/reset/reset-npcm.c:391:2: error: implicit declaration of function 'kfree'; did you mean 'vfree'? [-Werror=implicit-function-declaration]
kfree(rdev);
^~~~~
vfree
drivers/reset/reset-npcm.c: In function 'npcm_clock_adev_alloc':
drivers/reset/reset-npcm.c:400:9: error: implicit declaration of function 'kzalloc'; did you mean 'vzalloc'? [-Werror=implicit-function-declaration]
rdev = kzalloc(sizeof(*rdev), GFP_KERNEL);
^~~~~~~
vzalloc
>> drivers/reset/reset-npcm.c:400:7: warning: assignment makes pointer from integer without a cast [-Wint-conversion]
rdev = kzalloc(sizeof(*rdev), GFP_KERNEL);
^
cc1: some warnings being treated as errors
vim +400 drivers/reset/reset-npcm.c
385
386 static void npcm_clock_adev_release(struct device *dev)
387 {
388 struct auxiliary_device *adev = to_auxiliary_dev(dev);
389 struct npcm_clock_adev *rdev = to_npcm_clock_adev(adev);
390
> 391 kfree(rdev);
392 }
393
394 static struct auxiliary_device *npcm_clock_adev_alloc(struct npcm_rc_data *rst_data, char *clk_name)
395 {
396 struct npcm_clock_adev *rdev;
397 struct auxiliary_device *adev;
398 int ret;
399
> 400 rdev = kzalloc(sizeof(*rdev), GFP_KERNEL);
401 if (!rdev)
402 return ERR_PTR(-ENOMEM);
403
404 rdev->base = rst_data->base;
405
406 adev = &rdev->adev;
407 adev->name = clk_name;
408 adev->dev.parent = rst_data->dev;
409 adev->dev.release = npcm_clock_adev_release;
410 adev->id = 555u;
411
412 ret = auxiliary_device_init(adev);
413 if (ret) {
414 kfree(adev);
415 return ERR_PTR(ret);
416 }
417
418 return adev;
419 }
420
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Hi Tomer,
kernel test robot noticed the following build errors:
[auto build test ERROR on clk/clk-next]
[also build test ERROR on pza/reset/next linus/master v6.9-rc7 next-20240510]
[cannot apply to pza/imx-drm/next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Tomer-Maimon/dt-bindings-reset-npcm-add-clock-properties/20240510-072622
base: https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-next
patch link: https://lore.kernel.org/r/20240509192411.2432066-3-tmaimon77%40gmail.com
patch subject: [PATCH v24 2/4] reset: npcm: register npcm8xx clock auxiliary bus device
config: i386-buildonly-randconfig-004-20240510 (https://download.01.org/0day-ci/archive/20240510/[email protected]/config)
compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240510/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All errors (new ones prefixed by >>):
>> drivers/reset/reset-npcm.c:391:2: error: call to undeclared function 'kfree'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
391 | kfree(rdev);
| ^
drivers/reset/reset-npcm.c:391:2: note: did you mean 'vfree'?
include/linux/vmalloc.h:162:13: note: 'vfree' declared here
162 | extern void vfree(const void *addr);
| ^
>> drivers/reset/reset-npcm.c:400:9: error: call to undeclared function 'kzalloc'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
400 | rdev = kzalloc(sizeof(*rdev), GFP_KERNEL);
| ^
drivers/reset/reset-npcm.c:400:9: note: did you mean 'vzalloc'?
include/linux/vmalloc.h:142:14: note: 'vzalloc' declared here
142 | extern void *vzalloc(unsigned long size) __alloc_size(1);
| ^
>> drivers/reset/reset-npcm.c:400:7: error: incompatible integer to pointer conversion assigning to 'struct npcm_clock_adev *' from 'int' [-Wint-conversion]
400 | rdev = kzalloc(sizeof(*rdev), GFP_KERNEL);
| ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/reset/reset-npcm.c:414:3: error: call to undeclared function 'kfree'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
414 | kfree(adev);
| ^
4 errors generated.
vim +/kfree +391 drivers/reset/reset-npcm.c
385
386 static void npcm_clock_adev_release(struct device *dev)
387 {
388 struct auxiliary_device *adev = to_auxiliary_dev(dev);
389 struct npcm_clock_adev *rdev = to_npcm_clock_adev(adev);
390
> 391 kfree(rdev);
392 }
393
394 static struct auxiliary_device *npcm_clock_adev_alloc(struct npcm_rc_data *rst_data, char *clk_name)
395 {
396 struct npcm_clock_adev *rdev;
397 struct auxiliary_device *adev;
398 int ret;
399
> 400 rdev = kzalloc(sizeof(*rdev), GFP_KERNEL);
401 if (!rdev)
402 return ERR_PTR(-ENOMEM);
403
404 rdev->base = rst_data->base;
405
406 adev = &rdev->adev;
407 adev->name = clk_name;
408 adev->dev.parent = rst_data->dev;
409 adev->dev.release = npcm_clock_adev_release;
410 adev->id = 555u;
411
412 ret = auxiliary_device_init(adev);
413 if (ret) {
414 kfree(adev);
415 return ERR_PTR(ret);
416 }
417
418 return adev;
419 }
420
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
On Thu, May 09, 2024 at 10:24:08PM +0300, Tomer Maimon wrote:
> Adding 25MHz reference clock and clock-cell properties to NPCM reset
> document due to the registration of the npcm8xx clock auxiliary bus device
> in the NPCM reset driver
>
> The NPCM8xx clock auxiliary bus device has been registered in the NPCM
> reset driver because the reset and the clock share the same register
> region.
auxiliary bus is a Linux concept. The reasoning for this should be the
reset block also provides clocks.
> Signed-off-by: Tomer Maimon <[email protected]>
> ---
> .../bindings/reset/nuvoton,npcm750-reset.yaml | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/reset/nuvoton,npcm750-reset.yaml b/Documentation/devicetree/bindings/reset/nuvoton,npcm750-reset.yaml
> index d82e65e37cc0..18db4de13098 100644
> --- a/Documentation/devicetree/bindings/reset/nuvoton,npcm750-reset.yaml
> +++ b/Documentation/devicetree/bindings/reset/nuvoton,npcm750-reset.yaml
> @@ -21,6 +21,13 @@ properties:
> '#reset-cells':
> const: 2
>
> + '#clock-cells':
> + const: 1
> +
> + clocks:
> + items:
> + - description: specify external 25MHz referance clock.
s/referance/reference/
> +
> nuvoton,sysgcr:
> $ref: /schemas/types.yaml#/definitions/phandle
> description: a phandle to access GCR registers.
> @@ -39,6 +46,17 @@ required:
> - '#reset-cells'
> - nuvoton,sysgcr
>
> +if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - nuvoton,npcm845-reset
> +then:
> + required:
> + - '#clock-cells'
> + - clocks
New required properties are an ABI break. Please justify why that's okay
for this platform in the commit message (assuming that it is).
Rob
On Thu, May 09, 2024 at 10:24:11PM +0300, Tomer Maimon wrote:
> Remove nuvoton,npcm845-clk binding since the NPCM8xx clock driver
> using the auxiliary device framework and not the device tree framework.
Again, this is an ABI break. Changing driver architecture for 1 OS is
not a reason to change DT.
Rob
Hi Rob,
Thanks for your comment.
On Mon, 13 May 2024 at 18:53, Rob Herring <[email protected]> wrote:
>
> On Thu, May 09, 2024 at 10:24:11PM +0300, Tomer Maimon wrote:
> > Remove nuvoton,npcm845-clk binding since the NPCM8xx clock driver
> > using the auxiliary device framework and not the device tree framework.
>
> Again, this is an ABI break. Changing driver architecture for 1 OS is
> not a reason to change DT.
Is it an ABI break even if the NPCM8xx clock driver hasn't upstream
the kernel vanilla yet?
I thought that since the NPCM8xx clock driver hasn't upstream the
kernel vanilla yet and and in the latest NPCM8xx clock driver patch
the NPCM8xx clock driver.
using auxiliary device framework instead of DT we should remove the
nuvoton,npcm845-clk.yaml file.
https://patchwork.kernel.org/project/linux-clk/patch/[email protected]/
>
> Rob
Thanks,
Tomer
Hi Rob,
Thanks for your comments
On Mon, 13 May 2024 at 18:51, Rob Herring <[email protected]> wrote:
>
> On Thu, May 09, 2024 at 10:24:08PM +0300, Tomer Maimon wrote:
> > Adding 25MHz reference clock and clock-cell properties to NPCM reset
> > document due to the registration of the npcm8xx clock auxiliary bus device
> > in the NPCM reset driver
> >
> > The NPCM8xx clock auxiliary bus device has been registered in the NPCM
> > reset driver because the reset and the clock share the same register
> > region.
>
> auxiliary bus is a Linux concept. The reasoning for this should be the
> reset block also provides clocks.
>
>
> > Signed-off-by: Tomer Maimon <[email protected]>
> > ---
> > .../bindings/reset/nuvoton,npcm750-reset.yaml | 18 ++++++++++++++++++
> > 1 file changed, 18 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/reset/nuvoton,npcm750-reset.yaml b/Documentation/devicetree/bindings/reset/nuvoton,npcm750-reset.yaml
> > index d82e65e37cc0..18db4de13098 100644
> > --- a/Documentation/devicetree/bindings/reset/nuvoton,npcm750-reset.yaml
> > +++ b/Documentation/devicetree/bindings/reset/nuvoton,npcm750-reset.yaml
> > @@ -21,6 +21,13 @@ properties:
> > '#reset-cells':
> > const: 2
> >
> > + '#clock-cells':
> > + const: 1
> > +
> > + clocks:
> > + items:
> > + - description: specify external 25MHz referance clock.
>
> s/referance/reference/
>
> > +
> > nuvoton,sysgcr:
> > $ref: /schemas/types.yaml#/definitions/phandle
> > description: a phandle to access GCR registers.
> > @@ -39,6 +46,17 @@ required:
> > - '#reset-cells'
> > - nuvoton,sysgcr
> >
> > +if:
> > + properties:
> > + compatible:
> > + contains:
> > + enum:
> > + - nuvoton,npcm845-reset
> > +then:
> > + required:
> > + - '#clock-cells'
> > + - clocks
>
> New required properties are an ABI break. Please justify why that's okay
> for this platform in the commit message (assuming that it is).
will be done in next version
>
> Rob
Thanks,
Tomer
Hi Rob,
Kind reminder about the question in the mail thread below.
Your response would be greatly appreciated.
Thanks,
Tomer
On Thu, 16 May 2024 at 12:44, Tomer Maimon <[email protected]> wrote:
>
> Hi Rob,
>
> Thanks for your comment.
>
> On Mon, 13 May 2024 at 18:53, Rob Herring <[email protected]> wrote:
> >
> > On Thu, May 09, 2024 at 10:24:11PM +0300, Tomer Maimon wrote:
> > > Remove nuvoton,npcm845-clk binding since the NPCM8xx clock driver
> > > using the auxiliary device framework and not the device tree framework.
> >
> > Again, this is an ABI break. Changing driver architecture for 1 OS is
> > not a reason to change DT.
> Is it an ABI break even if the NPCM8xx clock driver hasn't upstream
> the kernel vanilla yet?
>
> I thought that since the NPCM8xx clock driver hasn't upstream the
> kernel vanilla yet and and in the latest NPCM8xx clock driver patch
> the NPCM8xx clock driver.
> using auxiliary device framework instead of DT we should remove the
> nuvoton,npcm845-clk.yaml file.
> https://patchwork.kernel.org/project/linux-clk/patch/[email protected]/
>
> >
> > Rob
>
> Thanks,
>
> Tomer
On 22/05/2024 18:34, Tomer Maimon wrote:
> Hi Rob,
>
> Kind reminder about the question in the mail thread below.
> Your response would be greatly appreciated.
>
> Thanks,
>
> Tomer
>
> On Thu, 16 May 2024 at 12:44, Tomer Maimon <[email protected]> wrote:
>>
>> Hi Rob,
>>
>> Thanks for your comment.
>>
>> On Mon, 13 May 2024 at 18:53, Rob Herring <[email protected]> wrote:
>>>
>>> On Thu, May 09, 2024 at 10:24:11PM +0300, Tomer Maimon wrote:
>>>> Remove nuvoton,npcm845-clk binding since the NPCM8xx clock driver
>>>> using the auxiliary device framework and not the device tree framework.
>>>
>>> Again, this is an ABI break. Changing driver architecture for 1 OS is
>>> not a reason to change DT.
>> Is it an ABI break even if the NPCM8xx clock driver hasn't upstream
>> the kernel vanilla yet?
>>
>> I thought that since the NPCM8xx clock driver hasn't upstream the
>> kernel vanilla yet and and in the latest NPCM8xx clock driver patch
>> the NPCM8xx clock driver.
>> using auxiliary device framework instead of DT we should remove the
>> nuvoton,npcm845-clk.yaml file.
>> https://patchwork.kernel.org/project/linux-clk/patch/[email protected]/
>>
Binding goes with the driver, so I wonder how did it happen that driver
is not in mainline but binding is?
Anyway, once binding is released other users might start to use it, so
yeah, could still be ABI break.
Best regards,
Krzysztof