From: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
The i.MX8ULP System Integration Module (SIM) LPAV module is a block
control module found inside the LPAV subsystem, which offers some clock
gating options and reset line assertion/de-assertion capabilities.
Therefore, the clock gate management is supported by registering the
module's driver as a clock provider, while the reset capabilities are
managed via the auxiliary device API to allow the DT node to act as a
reset and clock provider.
Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
---
drivers/clk/imx/Makefile | 1 +
drivers/clk/imx/clk-imx8ulp-sim-lpav.c | 211 +++++++++++++++++++++++++
2 files changed, 212 insertions(+)
create mode 100644 drivers/clk/imx/clk-imx8ulp-sim-lpav.c
diff --git a/drivers/clk/imx/Makefile b/drivers/clk/imx/Makefile
index 03f2b2a1ab63..208b46873a18 100644
--- a/drivers/clk/imx/Makefile
+++ b/drivers/clk/imx/Makefile
@@ -41,6 +41,7 @@ clk-imx-lpcg-scu-$(CONFIG_CLK_IMX8QXP) += clk-lpcg-scu.o clk-imx8qxp-lpcg.o
clk-imx-acm-$(CONFIG_CLK_IMX8QXP) = clk-imx8-acm.o
obj-$(CONFIG_CLK_IMX8ULP) += clk-imx8ulp.o
+obj-$(CONFIG_CLK_IMX8ULP) += clk-imx8ulp-sim-lpav.o
obj-$(CONFIG_CLK_IMX1) += clk-imx1.o
obj-$(CONFIG_CLK_IMX25) += clk-imx25.o
diff --git a/drivers/clk/imx/clk-imx8ulp-sim-lpav.c b/drivers/clk/imx/clk-imx8ulp-sim-lpav.c
new file mode 100644
index 000000000000..a67a0e50e1ce
--- /dev/null
+++ b/drivers/clk/imx/clk-imx8ulp-sim-lpav.c
@@ -0,0 +1,211 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2025 NXP
+ */
+
+#include <dt-bindings/clock/imx8ulp-clock.h>
+
+#include <linux/auxiliary_bus.h>
+#include <linux/clk-provider.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#define SYSCTRL0 0x8
+
+#define IMX8ULP_HIFI_CLK_GATE(gname, cname, pname, bidx) \
+ { \
+ .name = gname "_cg", \
+ .id = IMX8ULP_CLK_SIM_LPAV_HIFI_##cname, \
+ .parent = { .fw_name = pname, .name = pname }, \
+ .bit = bidx, \
+ }
+
+struct clk_imx8ulp_sim_lpav_data {
+ void __iomem *base;
+ struct regmap *regmap;
+ spinlock_t lock; /* shared by MUX, clock gate and reset */
+ unsigned long flags; /* for spinlock usage */
+ struct clk_hw_onecell_data clk_data; /* keep last */
+};
+
+struct clk_imx8ulp_sim_lpav_gate {
+ const char *name;
+ int id;
+ const struct clk_parent_data parent;
+ u8 bit;
+};
+
+static struct clk_imx8ulp_sim_lpav_gate gates[] = {
+ IMX8ULP_HIFI_CLK_GATE("hifi_core", CORE, "hifi_core", 17),
+ IMX8ULP_HIFI_CLK_GATE("hifi_pbclk", PBCLK, "lpav_bus", 18),
+ IMX8ULP_HIFI_CLK_GATE("hifi_plat", PLAT, "hifi_plat", 19)
+};
+
+#ifdef CONFIG_RESET_CONTROLLER
+static void clk_imx8ulp_sim_lpav_aux_reset_release(struct device *dev)
+{
+ struct auxiliary_device *adev = to_auxiliary_dev(dev);
+
+ kfree(adev);
+}
+
+static void clk_imx8ulp_sim_lpav_unregister_aux_reset(void *data)
+{
+ struct auxiliary_device *adev = data;
+
+ auxiliary_device_delete(adev);
+ auxiliary_device_uninit(adev);
+}
+
+static int clk_imx8ulp_sim_lpav_register_aux_reset(struct platform_device *pdev)
+{
+ struct auxiliary_device *adev __free(kfree) = NULL;
+ int ret;
+
+ adev = kzalloc(sizeof(*adev), GFP_KERNEL);
+ if (!adev)
+ return -ENOMEM;
+
+ adev->name = "reset";
+ adev->dev.parent = &pdev->dev;
+ adev->dev.release = clk_imx8ulp_sim_lpav_aux_reset_release;
+
+ ret = auxiliary_device_init(adev);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to initialize aux dev\n");
+ return ret;
+ }
+
+ ret = auxiliary_device_add(adev);
+ if (ret) {
+ auxiliary_device_uninit(adev);
+ dev_err(&pdev->dev, "failed to add aux dev\n");
+ return ret;
+ }
+
+ return devm_add_action_or_reset(&pdev->dev,
+ clk_imx8ulp_sim_lpav_unregister_aux_reset,
+ no_free_ptr(adev));
+}
+#else
+static int clk_imx8ulp_sim_lpav_register_aux_reset(struct platform_device *pdev)
+{
+ return 0;
+}
+#endif /* CONFIG_RESET_CONTROLLER */
+
+static void clk_imx8ulp_sim_lpav_lock(void *arg) __acquires(&data->lock)
+{
+ struct clk_imx8ulp_sim_lpav_data *data = dev_get_drvdata(arg);
+
+ spin_lock_irqsave(&data->lock, data->flags);
+}
+
+static void clk_imx8ulp_sim_lpav_unlock(void *arg) __releases(&data->lock)
+{
+ struct clk_imx8ulp_sim_lpav_data *data = dev_get_drvdata(arg);
+
+ spin_unlock_irqrestore(&data->lock, data->flags);
+}
+
+static const struct regmap_config clk_imx8ulp_sim_lpav_regmap_cfg = {
+ .reg_bits = 32,
+ .val_bits = 32,
+ .reg_stride = 4,
+ .lock = clk_imx8ulp_sim_lpav_lock,
+ .unlock = clk_imx8ulp_sim_lpav_unlock,
+};
+
+static int clk_imx8ulp_sim_lpav_probe(struct platform_device *pdev)
+{
+ struct clk_imx8ulp_sim_lpav_data *data;
+ struct regmap_config regmap_config;
+ struct clk_hw *hw;
+ int i, ret;
+
+ data = devm_kzalloc(&pdev->dev,
+ struct_size(data, clk_data.hws, ARRAY_SIZE(gates)),
+ GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ dev_set_drvdata(&pdev->dev, data);
+
+ memcpy(®map_config, &clk_imx8ulp_sim_lpav_regmap_cfg, sizeof(regmap_config));
+ regmap_config.lock_arg = &pdev->dev;
+
+ /*
+ * this lock is used directly by the clock gate and indirectly
+ * by the reset and mux controller via the regmap API
+ */
+ spin_lock_init(&data->lock);
+
+ data->base = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(data->base))
+ return dev_err_probe(&pdev->dev, PTR_ERR(data->base),
+ "failed to ioremap base\n");
+ /*
+ * although the clock gate doesn't use the regmap API to modify the
+ * registers, we still need the regmap because of the reset auxiliary
+ * driver and the MUX drivers, which use the parent device's regmap
+ */
+ data->regmap = devm_regmap_init_mmio(&pdev->dev, data->base, ®map_config);
+ if (IS_ERR(data->regmap))
+ return dev_err_probe(&pdev->dev, PTR_ERR(data->regmap),
+ "failed to initialize regmap\n");
+
+ data->clk_data.num = ARRAY_SIZE(gates);
+
+ for (i = 0; i < ARRAY_SIZE(gates); i++) {
+ hw = devm_clk_hw_register_gate_parent_data(&pdev->dev,
+ gates[i].name,
+ &gates[i].parent,
+ CLK_SET_RATE_PARENT,
+ data->base + SYSCTRL0,
+ gates[i].bit,
+ 0x0, &data->lock);
+ if (IS_ERR(hw))
+ return dev_err_probe(&pdev->dev, PTR_ERR(hw),
+ "failed to register %s gate\n",
+ gates[i].name);
+
+ data->clk_data.hws[i] = hw;
+ }
+
+ ret = clk_imx8ulp_sim_lpav_register_aux_reset(pdev);
+ if (ret)
+ return dev_err_probe(&pdev->dev, ret,
+ "failed to register aux reset\n");
+
+ ret = devm_of_clk_add_hw_provider(&pdev->dev,
+ of_clk_hw_onecell_get,
+ &data->clk_data);
+ if (ret)
+ return dev_err_probe(&pdev->dev, ret,
+ "failed to register clk hw provider\n");
+
+ /* used to probe MUX child device */
+ return devm_of_platform_populate(&pdev->dev);
+}
+
+static const struct of_device_id clk_imx8ulp_sim_lpav_of_match[] = {
+ { .compatible = "fsl,imx8ulp-sim-lpav" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, clk_imx8ulp_sim_lpav_of_match);
+
+static struct platform_driver clk_imx8ulp_sim_lpav_driver = {
+ .probe = clk_imx8ulp_sim_lpav_probe,
+ .driver = {
+ .name = "clk-imx8ulp-sim-lpav",
+ .of_match_table = clk_imx8ulp_sim_lpav_of_match,
+ },
+};
+module_platform_driver(clk_imx8ulp_sim_lpav_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("i.MX8ULP LPAV System Integration Module (SIM) clock driver");
+MODULE_AUTHOR("Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>");
--
2.43.0Hi Laurentiu,
On Fri, Oct 17, 2025 at 04:20:20AM -0700, Laurentiu Mihalcea wrote:
>From: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
>
>The i.MX8ULP System Integration Module (SIM) LPAV module is a block
>control module found inside the LPAV subsystem, which offers some clock
>gating options and reset line assertion/de-assertion capabilities.
>
>Therefore, the clock gate management is supported by registering the
>module's driver as a clock provider, while the reset capabilities are
>managed via the auxiliary device API to allow the DT node to act as a
>reset and clock provider.
>
>Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
>---
....
>+struct clk_imx8ulp_sim_lpav_data {
>+ void __iomem *base;
>+ struct regmap *regmap;
>+ spinlock_t lock; /* shared by MUX, clock gate and reset */
>+ unsigned long flags; /* for spinlock usage */
This does not need to be here, put it as function local variable should
be fine.
>+ struct clk_hw_onecell_data clk_data; /* keep last */
>+};
>+
>+struct clk_imx8ulp_sim_lpav_gate {
>+ const char *name;
>+ int id;
>+ const struct clk_parent_data parent;
>+ u8 bit;
>+};
>+
>+static struct clk_imx8ulp_sim_lpav_gate gates[] = {
>+ IMX8ULP_HIFI_CLK_GATE("hifi_core", CORE, "hifi_core", 17),
>+ IMX8ULP_HIFI_CLK_GATE("hifi_pbclk", PBCLK, "lpav_bus", 18),
>+ IMX8ULP_HIFI_CLK_GATE("hifi_plat", PLAT, "hifi_plat", 19)
For the parent name, my understanding is they should be the one
from clk-imx8ulp.c, but I not find them, or may I miss something?
>+};
>+
>+#ifdef CONFIG_RESET_CONTROLLER
>+static void clk_imx8ulp_sim_lpav_aux_reset_release(struct device *dev)
>+{
>+ struct auxiliary_device *adev = to_auxiliary_dev(dev);
>+
>+ kfree(adev);
>+}
>+
>+static void clk_imx8ulp_sim_lpav_unregister_aux_reset(void *data)
>+{
>+ struct auxiliary_device *adev = data;
>+
>+ auxiliary_device_delete(adev);
>+ auxiliary_device_uninit(adev);
>+}
>+
>+static int clk_imx8ulp_sim_lpav_register_aux_reset(struct platform_device *pdev)
>+{
>+ struct auxiliary_device *adev __free(kfree) = NULL;
>+ int ret;
>+
>+ adev = kzalloc(sizeof(*adev), GFP_KERNEL);
>+ if (!adev)
>+ return -ENOMEM;
>+
>+ adev->name = "reset";
>+ adev->dev.parent = &pdev->dev;
>+ adev->dev.release = clk_imx8ulp_sim_lpav_aux_reset_release;
>+
>+ ret = auxiliary_device_init(adev);
>+ if (ret) {
>+ dev_err(&pdev->dev, "failed to initialize aux dev\n");
>+ return ret;
>+ }
>+
>+ ret = auxiliary_device_add(adev);
>+ if (ret) {
>+ auxiliary_device_uninit(adev);
>+ dev_err(&pdev->dev, "failed to add aux dev\n");
>+ return ret;
>+ }
>+
>+ return devm_add_action_or_reset(&pdev->dev,
>+ clk_imx8ulp_sim_lpav_unregister_aux_reset,
>+ no_free_ptr(adev));
clk_imx8ulp_sim_lpav_unregister_aux_reset() clean up the resources, if
moving this before auxiliary_device_add(), then no need
auxiliary_device_uninit() when add fails?
>+}
Regards
Peng
On 10/22/2025 7:03 AM, Peng Fan wrote:
> Hi Laurentiu,
>
> On Fri, Oct 17, 2025 at 04:20:20AM -0700, Laurentiu Mihalcea wrote:
>> From: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
>>
>> The i.MX8ULP System Integration Module (SIM) LPAV module is a block
>> control module found inside the LPAV subsystem, which offers some clock
>> gating options and reset line assertion/de-assertion capabilities.
>>
>> Therefore, the clock gate management is supported by registering the
>> module's driver as a clock provider, while the reset capabilities are
>> managed via the auxiliary device API to allow the DT node to act as a
>> reset and clock provider.
>>
>> Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
>> ---
> ....
>> +struct clk_imx8ulp_sim_lpav_data {
>> + void __iomem *base;
>> + struct regmap *regmap;
>> + spinlock_t lock; /* shared by MUX, clock gate and reset */
>> + unsigned long flags; /* for spinlock usage */
> This does not need to be here, put it as function local variable should
> be fine.
>
>> + struct clk_hw_onecell_data clk_data; /* keep last */
>> +};
>> +
>> +struct clk_imx8ulp_sim_lpav_gate {
>> + const char *name;
>> + int id;
>> + const struct clk_parent_data parent;
>> + u8 bit;
>> +};
>> +
>> +static struct clk_imx8ulp_sim_lpav_gate gates[] = {
>> + IMX8ULP_HIFI_CLK_GATE("hifi_core", CORE, "hifi_core", 17),
>> + IMX8ULP_HIFI_CLK_GATE("hifi_pbclk", PBCLK, "lpav_bus", 18),
>> + IMX8ULP_HIFI_CLK_GATE("hifi_plat", PLAT, "hifi_plat", 19)
> For the parent name, my understanding is they should be the one
> from clk-imx8ulp.c, but I not find them, or may I miss something?
we want to use parent names local to the SIM LPAV provider (i.e. names specified via clock-names)
instead of the global ones. See definition of "struct clk_parent_data".
while we're at it, maybe it would be a good idea to drop the ".name = pname" assignment bit
as that won't work anyways since we're relying on local parent names?
>
>> +};
>> +
>> +#ifdef CONFIG_RESET_CONTROLLER
>> +static void clk_imx8ulp_sim_lpav_aux_reset_release(struct device *dev)
>> +{
>> + struct auxiliary_device *adev = to_auxiliary_dev(dev);
>> +
>> + kfree(adev);
>> +}
>> +
>> +static void clk_imx8ulp_sim_lpav_unregister_aux_reset(void *data)
>> +{
>> + struct auxiliary_device *adev = data;
>> +
>> + auxiliary_device_delete(adev);
>> + auxiliary_device_uninit(adev);
>> +}
>> +
>> +static int clk_imx8ulp_sim_lpav_register_aux_reset(struct platform_device *pdev)
>> +{
>> + struct auxiliary_device *adev __free(kfree) = NULL;
>> + int ret;
>> +
>> + adev = kzalloc(sizeof(*adev), GFP_KERNEL);
>> + if (!adev)
>> + return -ENOMEM;
>> +
>> + adev->name = "reset";
>> + adev->dev.parent = &pdev->dev;
>> + adev->dev.release = clk_imx8ulp_sim_lpav_aux_reset_release;
>> +
>> + ret = auxiliary_device_init(adev);
>> + if (ret) {
>> + dev_err(&pdev->dev, "failed to initialize aux dev\n");
>> + return ret;
>> + }
>> +
>> + ret = auxiliary_device_add(adev);
>> + if (ret) {
>> + auxiliary_device_uninit(adev);
>> + dev_err(&pdev->dev, "failed to add aux dev\n");
>> + return ret;
>> + }
>> +
>> + return devm_add_action_or_reset(&pdev->dev,
>> + clk_imx8ulp_sim_lpav_unregister_aux_reset,
>> + no_free_ptr(adev));
> clk_imx8ulp_sim_lpav_unregister_aux_reset() clean up the resources, if
> moving this before auxiliary_device_add(), then no need
> auxiliary_device_uninit() when add fails?
the whole chunk guarded by the #ifdef CONFIG_RESET_CONTROLLER should go away
anyways once I switch to devres as Frank suggested
>
>> +}
> Regards
> Peng
Hi Laurentiu,
kernel test robot noticed the following build errors:
[auto build test ERROR on pza/reset/next]
[also build test ERROR on abelvesa/clk/imx abelvesa/for-next linus/master v6.18-rc1 next-20251017]
[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/Laurentiu-Mihalcea/reset-imx8mp-audiomix-Fix-bad-mask-values/20251017-192620
base: https://git.pengutronix.de/git/pza/linux reset/next
patch link: https://lore.kernel.org/r/20251017112025.11997-4-laurentiumihalcea111%40gmail.com
patch subject: [PATCH v2 3/8] clk: imx: add driver for imx8ulp's sim lpav
config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20251018/202510182350.57sb54Rm-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 15.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251018/202510182350.57sb54Rm-lkp@intel.com/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 <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202510182350.57sb54Rm-lkp@intel.com/
All errors (new ones prefixed by >>):
drivers/clk/imx/clk-imx8ulp-sim-lpav.c: In function 'clk_imx8ulp_sim_lpav_aux_reset_release':
>> drivers/clk/imx/clk-imx8ulp-sim-lpav.c:52:9: error: implicit declaration of function 'kfree' [-Wimplicit-function-declaration]
52 | kfree(adev);
| ^~~~~
drivers/clk/imx/clk-imx8ulp-sim-lpav.c: In function 'clk_imx8ulp_sim_lpav_register_aux_reset':
>> drivers/clk/imx/clk-imx8ulp-sim-lpav.c:65:16: error: cleanup argument not a function
65 | struct auxiliary_device *adev __free(kfree) = NULL;
| ^~~~~~~~~~~~~~~~
>> drivers/clk/imx/clk-imx8ulp-sim-lpav.c:68:16: error: implicit declaration of function 'kzalloc' [-Wimplicit-function-declaration]
68 | adev = kzalloc(sizeof(*adev), GFP_KERNEL);
| ^~~~~~~
>> drivers/clk/imx/clk-imx8ulp-sim-lpav.c:68:14: error: assignment to 'struct auxiliary_device *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
68 | adev = kzalloc(sizeof(*adev), GFP_KERNEL);
| ^
vim +/kfree +52 drivers/clk/imx/clk-imx8ulp-sim-lpav.c
46
47 #ifdef CONFIG_RESET_CONTROLLER
48 static void clk_imx8ulp_sim_lpav_aux_reset_release(struct device *dev)
49 {
50 struct auxiliary_device *adev = to_auxiliary_dev(dev);
51
> 52 kfree(adev);
53 }
54
55 static void clk_imx8ulp_sim_lpav_unregister_aux_reset(void *data)
56 {
57 struct auxiliary_device *adev = data;
58
59 auxiliary_device_delete(adev);
60 auxiliary_device_uninit(adev);
61 }
62
63 static int clk_imx8ulp_sim_lpav_register_aux_reset(struct platform_device *pdev)
64 {
> 65 struct auxiliary_device *adev __free(kfree) = NULL;
66 int ret;
67
> 68 adev = kzalloc(sizeof(*adev), GFP_KERNEL);
69 if (!adev)
70 return -ENOMEM;
71
72 adev->name = "reset";
73 adev->dev.parent = &pdev->dev;
74 adev->dev.release = clk_imx8ulp_sim_lpav_aux_reset_release;
75
76 ret = auxiliary_device_init(adev);
77 if (ret) {
78 dev_err(&pdev->dev, "failed to initialize aux dev\n");
79 return ret;
80 }
81
82 ret = auxiliary_device_add(adev);
83 if (ret) {
84 auxiliary_device_uninit(adev);
85 dev_err(&pdev->dev, "failed to add aux dev\n");
86 return ret;
87 }
88
89 return devm_add_action_or_reset(&pdev->dev,
90 clk_imx8ulp_sim_lpav_unregister_aux_reset,
91 no_free_ptr(adev));
92 }
93 #else
94 static int clk_imx8ulp_sim_lpav_register_aux_reset(struct platform_device *pdev)
95 {
96 return 0;
97 }
98 #endif /* CONFIG_RESET_CONTROLLER */
99
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Hi Laurentiu,
kernel test robot noticed the following build warnings:
[auto build test WARNING on pza/reset/next]
[also build test WARNING on abelvesa/clk/imx abelvesa/for-next linus/master v6.18-rc1 next-20251017]
[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/Laurentiu-Mihalcea/reset-imx8mp-audiomix-Fix-bad-mask-values/20251017-192620
base: https://git.pengutronix.de/git/pza/linux reset/next
patch link: https://lore.kernel.org/r/20251017112025.11997-4-laurentiumihalcea111%40gmail.com
patch subject: [PATCH v2 3/8] clk: imx: add driver for imx8ulp's sim lpav
config: xtensa-randconfig-r132-20251018 (https://download.01.org/0day-ci/archive/20251018/202510181949.IazLEB6V-lkp@intel.com/config)
compiler: xtensa-linux-gcc (GCC) 11.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251018/202510181949.IazLEB6V-lkp@intel.com/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 <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202510181949.IazLEB6V-lkp@intel.com/
All warnings (new ones prefixed by >>):
drivers/clk/imx/clk-imx8ulp-sim-lpav.c: In function 'clk_imx8ulp_sim_lpav_aux_reset_release':
drivers/clk/imx/clk-imx8ulp-sim-lpav.c:52:9: error: implicit declaration of function 'kfree' [-Werror=implicit-function-declaration]
52 | kfree(adev);
| ^~~~~
drivers/clk/imx/clk-imx8ulp-sim-lpav.c: In function 'clk_imx8ulp_sim_lpav_register_aux_reset':
drivers/clk/imx/clk-imx8ulp-sim-lpav.c:65:16: error: cleanup argument not a function
65 | struct auxiliary_device *adev __free(kfree) = NULL;
| ^~~~~~~~~~~~~~~~
drivers/clk/imx/clk-imx8ulp-sim-lpav.c:68:16: error: implicit declaration of function 'kzalloc' [-Werror=implicit-function-declaration]
68 | adev = kzalloc(sizeof(*adev), GFP_KERNEL);
| ^~~~~~~
>> drivers/clk/imx/clk-imx8ulp-sim-lpav.c:68:14: warning: assignment to 'struct auxiliary_device *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
68 | adev = kzalloc(sizeof(*adev), GFP_KERNEL);
| ^
cc1: some warnings being treated as errors
vim +68 drivers/clk/imx/clk-imx8ulp-sim-lpav.c
62
63 static int clk_imx8ulp_sim_lpav_register_aux_reset(struct platform_device *pdev)
64 {
65 struct auxiliary_device *adev __free(kfree) = NULL;
66 int ret;
67
> 68 adev = kzalloc(sizeof(*adev), GFP_KERNEL);
69 if (!adev)
70 return -ENOMEM;
71
72 adev->name = "reset";
73 adev->dev.parent = &pdev->dev;
74 adev->dev.release = clk_imx8ulp_sim_lpav_aux_reset_release;
75
76 ret = auxiliary_device_init(adev);
77 if (ret) {
78 dev_err(&pdev->dev, "failed to initialize aux dev\n");
79 return ret;
80 }
81
82 ret = auxiliary_device_add(adev);
83 if (ret) {
84 auxiliary_device_uninit(adev);
85 dev_err(&pdev->dev, "failed to add aux dev\n");
86 return ret;
87 }
88
89 return devm_add_action_or_reset(&pdev->dev,
90 clk_imx8ulp_sim_lpav_unregister_aux_reset,
91 no_free_ptr(adev));
92 }
93 #else
94 static int clk_imx8ulp_sim_lpav_register_aux_reset(struct platform_device *pdev)
95 {
96 return 0;
97 }
98 #endif /* CONFIG_RESET_CONTROLLER */
99
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Hi Laurentiu,
kernel test robot noticed the following build errors:
[auto build test ERROR on pza/reset/next]
[also build test ERROR on abelvesa/clk/imx abelvesa/for-next linus/master v6.18-rc1 next-20251017]
[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/Laurentiu-Mihalcea/reset-imx8mp-audiomix-Fix-bad-mask-values/20251017-192620
base: https://git.pengutronix.de/git/pza/linux reset/next
patch link: https://lore.kernel.org/r/20251017112025.11997-4-laurentiumihalcea111%40gmail.com
patch subject: [PATCH v2 3/8] clk: imx: add driver for imx8ulp's sim lpav
config: hexagon-allmodconfig (https://download.01.org/0day-ci/archive/20251018/202510181121.D3XVfMFh-lkp@intel.com/config)
compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251018/202510181121.D3XVfMFh-lkp@intel.com/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 <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202510181121.D3XVfMFh-lkp@intel.com/
All errors (new ones prefixed by >>):
>> drivers/clk/imx/clk-imx8ulp-sim-lpav.c:52:2: error: call to undeclared function 'kfree'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
52 | kfree(adev);
| ^
>> drivers/clk/imx/clk-imx8ulp-sim-lpav.c:65:32: error: use of undeclared identifier '__free_kfree'; did you mean '__free_pages'?
65 | struct auxiliary_device *adev __free(kfree) = NULL;
| ^
include/linux/cleanup.h:213:33: note: expanded from macro '__free'
213 | #define __free(_name) __cleanup(__free_##_name)
| ^
<scratch space>:30:1: note: expanded from here
30 | __free_kfree
| ^
include/linux/gfp.h:381:13: note: '__free_pages' declared here
381 | extern void __free_pages(struct page *page, unsigned int order);
| ^
>> drivers/clk/imx/clk-imx8ulp-sim-lpav.c:65:32: error: 'cleanup' function '__free_pages' must take 1 parameter
65 | struct auxiliary_device *adev __free(kfree) = NULL;
| ^
include/linux/cleanup.h:213:33: note: expanded from macro '__free'
213 | #define __free(_name) __cleanup(__free_##_name)
| ^
<scratch space>:30:1: note: expanded from here
30 | __free_kfree
| ^
>> drivers/clk/imx/clk-imx8ulp-sim-lpav.c:68:9: error: call to undeclared function 'kzalloc'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
68 | adev = kzalloc(sizeof(*adev), GFP_KERNEL);
| ^
>> drivers/clk/imx/clk-imx8ulp-sim-lpav.c:68:7: error: incompatible integer to pointer conversion assigning to 'struct auxiliary_device *' from 'int' [-Wint-conversion]
68 | adev = kzalloc(sizeof(*adev), GFP_KERNEL);
| ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
5 errors generated.
vim +/kfree +52 drivers/clk/imx/clk-imx8ulp-sim-lpav.c
46
47 #ifdef CONFIG_RESET_CONTROLLER
48 static void clk_imx8ulp_sim_lpav_aux_reset_release(struct device *dev)
49 {
50 struct auxiliary_device *adev = to_auxiliary_dev(dev);
51
> 52 kfree(adev);
53 }
54
55 static void clk_imx8ulp_sim_lpav_unregister_aux_reset(void *data)
56 {
57 struct auxiliary_device *adev = data;
58
59 auxiliary_device_delete(adev);
60 auxiliary_device_uninit(adev);
61 }
62
63 static int clk_imx8ulp_sim_lpav_register_aux_reset(struct platform_device *pdev)
64 {
> 65 struct auxiliary_device *adev __free(kfree) = NULL;
66 int ret;
67
> 68 adev = kzalloc(sizeof(*adev), GFP_KERNEL);
69 if (!adev)
70 return -ENOMEM;
71
72 adev->name = "reset";
73 adev->dev.parent = &pdev->dev;
74 adev->dev.release = clk_imx8ulp_sim_lpav_aux_reset_release;
75
76 ret = auxiliary_device_init(adev);
77 if (ret) {
78 dev_err(&pdev->dev, "failed to initialize aux dev\n");
79 return ret;
80 }
81
82 ret = auxiliary_device_add(adev);
83 if (ret) {
84 auxiliary_device_uninit(adev);
85 dev_err(&pdev->dev, "failed to add aux dev\n");
86 return ret;
87 }
88
89 return devm_add_action_or_reset(&pdev->dev,
90 clk_imx8ulp_sim_lpav_unregister_aux_reset,
91 no_free_ptr(adev));
92 }
93 #else
94 static int clk_imx8ulp_sim_lpav_register_aux_reset(struct platform_device *pdev)
95 {
96 return 0;
97 }
98 #endif /* CONFIG_RESET_CONTROLLER */
99
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
On Fri, Oct 17, 2025 at 04:20:20AM -0700, Laurentiu Mihalcea wrote:
> From: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
>
> The i.MX8ULP System Integration Module (SIM) LPAV module is a block
> control module found inside the LPAV subsystem, which offers some clock
> gating options and reset line assertion/de-assertion capabilities.
>
> Therefore, the clock gate management is supported by registering the
> module's driver as a clock provider, while the reset capabilities are
> managed via the auxiliary device API to allow the DT node to act as a
> reset and clock provider.
>
> Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
> ---
> drivers/clk/imx/Makefile | 1 +
> drivers/clk/imx/clk-imx8ulp-sim-lpav.c | 211 +++++++++++++++++++++++++
> 2 files changed, 212 insertions(+)
> create mode 100644 drivers/clk/imx/clk-imx8ulp-sim-lpav.c
>
> diff --git a/drivers/clk/imx/Makefile b/drivers/clk/imx/Makefile
> index 03f2b2a1ab63..208b46873a18 100644
> --- a/drivers/clk/imx/Makefile
> +++ b/drivers/clk/imx/Makefile
> @@ -41,6 +41,7 @@ clk-imx-lpcg-scu-$(CONFIG_CLK_IMX8QXP) += clk-lpcg-scu.o clk-imx8qxp-lpcg.o
> clk-imx-acm-$(CONFIG_CLK_IMX8QXP) = clk-imx8-acm.o
>
> obj-$(CONFIG_CLK_IMX8ULP) += clk-imx8ulp.o
> +obj-$(CONFIG_CLK_IMX8ULP) += clk-imx8ulp-sim-lpav.o
>
> obj-$(CONFIG_CLK_IMX1) += clk-imx1.o
> obj-$(CONFIG_CLK_IMX25) += clk-imx25.o
> diff --git a/drivers/clk/imx/clk-imx8ulp-sim-lpav.c b/drivers/clk/imx/clk-imx8ulp-sim-lpav.c
> new file mode 100644
> index 000000000000..a67a0e50e1ce
> --- /dev/null
> +++ b/drivers/clk/imx/clk-imx8ulp-sim-lpav.c
> @@ -0,0 +1,211 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright 2025 NXP
> + */
> +
> +#include <dt-bindings/clock/imx8ulp-clock.h>
> +
> +#include <linux/auxiliary_bus.h>
> +#include <linux/clk-provider.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +
> +#define SYSCTRL0 0x8
> +
> +#define IMX8ULP_HIFI_CLK_GATE(gname, cname, pname, bidx) \
> + { \
> + .name = gname "_cg", \
> + .id = IMX8ULP_CLK_SIM_LPAV_HIFI_##cname, \
> + .parent = { .fw_name = pname, .name = pname }, \
> + .bit = bidx, \
> + }
> +
> +struct clk_imx8ulp_sim_lpav_data {
> + void __iomem *base;
> + struct regmap *regmap;
> + spinlock_t lock; /* shared by MUX, clock gate and reset */
> + unsigned long flags; /* for spinlock usage */
> + struct clk_hw_onecell_data clk_data; /* keep last */
> +};
> +
> +struct clk_imx8ulp_sim_lpav_gate {
> + const char *name;
> + int id;
> + const struct clk_parent_data parent;
> + u8 bit;
> +};
> +
> +static struct clk_imx8ulp_sim_lpav_gate gates[] = {
> + IMX8ULP_HIFI_CLK_GATE("hifi_core", CORE, "hifi_core", 17),
> + IMX8ULP_HIFI_CLK_GATE("hifi_pbclk", PBCLK, "lpav_bus", 18),
> + IMX8ULP_HIFI_CLK_GATE("hifi_plat", PLAT, "hifi_plat", 19)
> +};
> +
> +#ifdef CONFIG_RESET_CONTROLLER
Needn't this ifdef because if CONFIG_RESET_CONTROLLER, reset driver will
not probe, just register aux device is not harmful.
> +static void clk_imx8ulp_sim_lpav_aux_reset_release(struct device *dev)
> +{
> + struct auxiliary_device *adev = to_auxiliary_dev(dev);
> +
> + kfree(adev);
> +}
> +
> +static void clk_imx8ulp_sim_lpav_unregister_aux_reset(void *data)
> +{
> + struct auxiliary_device *adev = data;
> +
> + auxiliary_device_delete(adev);
> + auxiliary_device_uninit(adev);
> +}
> +
> +static int clk_imx8ulp_sim_lpav_register_aux_reset(struct platform_device *pdev)
> +{
> + struct auxiliary_device *adev __free(kfree) = NULL;
> + int ret;
> +
> + adev = kzalloc(sizeof(*adev), GFP_KERNEL);
> + if (!adev)
> + return -ENOMEM;
> +
> + adev->name = "reset";
> + adev->dev.parent = &pdev->dev;
> + adev->dev.release = clk_imx8ulp_sim_lpav_aux_reset_release;
> +
> + ret = auxiliary_device_init(adev);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to initialize aux dev\n");
> + return ret;
> + }
> +
> + ret = auxiliary_device_add(adev);
> + if (ret) {
> + auxiliary_device_uninit(adev);
> + dev_err(&pdev->dev, "failed to add aux dev\n");
> + return ret;
> + }
> +
> + return devm_add_action_or_reset(&pdev->dev,
> + clk_imx8ulp_sim_lpav_unregister_aux_reset,
> + no_free_ptr(adev));
use devm_auxiliary_device_create() to simple whole code.
> +}
> +#else
> +static int clk_imx8ulp_sim_lpav_register_aux_reset(struct platform_device *pdev)
> +{
> + return 0;
> +}
> +#endif /* CONFIG_RESET_CONTROLLER */
> +
> +static void clk_imx8ulp_sim_lpav_lock(void *arg) __acquires(&data->lock)
> +{
> + struct clk_imx8ulp_sim_lpav_data *data = dev_get_drvdata(arg);
> +
> + spin_lock_irqsave(&data->lock, data->flags);
> +}
> +
> +static void clk_imx8ulp_sim_lpav_unlock(void *arg) __releases(&data->lock)
> +{
> + struct clk_imx8ulp_sim_lpav_data *data = dev_get_drvdata(arg);
> +
> + spin_unlock_irqrestore(&data->lock, data->flags);
> +}
> +
> +static const struct regmap_config clk_imx8ulp_sim_lpav_regmap_cfg = {
> + .reg_bits = 32,
> + .val_bits = 32,
> + .reg_stride = 4,
> + .lock = clk_imx8ulp_sim_lpav_lock,
> + .unlock = clk_imx8ulp_sim_lpav_unlock,
why need lock/unlock here, regmap already have lock to access one register.
Frank
> +};
> +
> +static int clk_imx8ulp_sim_lpav_probe(struct platform_device *pdev)
> +{
> + struct clk_imx8ulp_sim_lpav_data *data;
> + struct regmap_config regmap_config;
> + struct clk_hw *hw;
> + int i, ret;
> +
> + data = devm_kzalloc(&pdev->dev,
> + struct_size(data, clk_data.hws, ARRAY_SIZE(gates)),
> + GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + dev_set_drvdata(&pdev->dev, data);
> +
> + memcpy(®map_config, &clk_imx8ulp_sim_lpav_regmap_cfg, sizeof(regmap_config));
> + regmap_config.lock_arg = &pdev->dev;
> +
> + /*
> + * this lock is used directly by the clock gate and indirectly
> + * by the reset and mux controller via the regmap API
> + */
> + spin_lock_init(&data->lock);
> +
> + data->base = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(data->base))
> + return dev_err_probe(&pdev->dev, PTR_ERR(data->base),
> + "failed to ioremap base\n");
> + /*
> + * although the clock gate doesn't use the regmap API to modify the
> + * registers, we still need the regmap because of the reset auxiliary
> + * driver and the MUX drivers, which use the parent device's regmap
> + */
> + data->regmap = devm_regmap_init_mmio(&pdev->dev, data->base, ®map_config);
> + if (IS_ERR(data->regmap))
> + return dev_err_probe(&pdev->dev, PTR_ERR(data->regmap),
> + "failed to initialize regmap\n");
> +
> + data->clk_data.num = ARRAY_SIZE(gates);
> +
> + for (i = 0; i < ARRAY_SIZE(gates); i++) {
> + hw = devm_clk_hw_register_gate_parent_data(&pdev->dev,
> + gates[i].name,
> + &gates[i].parent,
> + CLK_SET_RATE_PARENT,
> + data->base + SYSCTRL0,
> + gates[i].bit,
> + 0x0, &data->lock);
> + if (IS_ERR(hw))
> + return dev_err_probe(&pdev->dev, PTR_ERR(hw),
> + "failed to register %s gate\n",
> + gates[i].name);
> +
> + data->clk_data.hws[i] = hw;
> + }
> +
> + ret = clk_imx8ulp_sim_lpav_register_aux_reset(pdev);
> + if (ret)
> + return dev_err_probe(&pdev->dev, ret,
> + "failed to register aux reset\n");
> +
> + ret = devm_of_clk_add_hw_provider(&pdev->dev,
> + of_clk_hw_onecell_get,
> + &data->clk_data);
> + if (ret)
> + return dev_err_probe(&pdev->dev, ret,
> + "failed to register clk hw provider\n");
> +
> + /* used to probe MUX child device */
> + return devm_of_platform_populate(&pdev->dev);
> +}
> +
> +static const struct of_device_id clk_imx8ulp_sim_lpav_of_match[] = {
> + { .compatible = "fsl,imx8ulp-sim-lpav" },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, clk_imx8ulp_sim_lpav_of_match);
> +
> +static struct platform_driver clk_imx8ulp_sim_lpav_driver = {
> + .probe = clk_imx8ulp_sim_lpav_probe,
> + .driver = {
> + .name = "clk-imx8ulp-sim-lpav",
> + .of_match_table = clk_imx8ulp_sim_lpav_of_match,
> + },
> +};
> +module_platform_driver(clk_imx8ulp_sim_lpav_driver);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("i.MX8ULP LPAV System Integration Module (SIM) clock driver");
> +MODULE_AUTHOR("Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>");
> --
> 2.43.0
>
On 10/17/2025 7:41 AM, Frank Li wrote:
> On Fri, Oct 17, 2025 at 04:20:20AM -0700, Laurentiu Mihalcea wrote:
>> From: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
>>
>> The i.MX8ULP System Integration Module (SIM) LPAV module is a block
>> control module found inside the LPAV subsystem, which offers some clock
>> gating options and reset line assertion/de-assertion capabilities.
>>
>> Therefore, the clock gate management is supported by registering the
>> module's driver as a clock provider, while the reset capabilities are
>> managed via the auxiliary device API to allow the DT node to act as a
>> reset and clock provider.
>>
>> Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
>> ---
>> drivers/clk/imx/Makefile | 1 +
>> drivers/clk/imx/clk-imx8ulp-sim-lpav.c | 211 +++++++++++++++++++++++++
>> 2 files changed, 212 insertions(+)
>> create mode 100644 drivers/clk/imx/clk-imx8ulp-sim-lpav.c
>>
>> diff --git a/drivers/clk/imx/Makefile b/drivers/clk/imx/Makefile
>> index 03f2b2a1ab63..208b46873a18 100644
>> --- a/drivers/clk/imx/Makefile
>> +++ b/drivers/clk/imx/Makefile
>> @@ -41,6 +41,7 @@ clk-imx-lpcg-scu-$(CONFIG_CLK_IMX8QXP) += clk-lpcg-scu.o clk-imx8qxp-lpcg.o
>> clk-imx-acm-$(CONFIG_CLK_IMX8QXP) = clk-imx8-acm.o
>>
>> obj-$(CONFIG_CLK_IMX8ULP) += clk-imx8ulp.o
>> +obj-$(CONFIG_CLK_IMX8ULP) += clk-imx8ulp-sim-lpav.o
>>
>> obj-$(CONFIG_CLK_IMX1) += clk-imx1.o
>> obj-$(CONFIG_CLK_IMX25) += clk-imx25.o
>> diff --git a/drivers/clk/imx/clk-imx8ulp-sim-lpav.c b/drivers/clk/imx/clk-imx8ulp-sim-lpav.c
>> new file mode 100644
>> index 000000000000..a67a0e50e1ce
>> --- /dev/null
>> +++ b/drivers/clk/imx/clk-imx8ulp-sim-lpav.c
>> @@ -0,0 +1,211 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright 2025 NXP
>> + */
>> +
>> +#include <dt-bindings/clock/imx8ulp-clock.h>
>> +
>> +#include <linux/auxiliary_bus.h>
>> +#include <linux/clk-provider.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/regmap.h>
>> +
>> +#define SYSCTRL0 0x8
>> +
>> +#define IMX8ULP_HIFI_CLK_GATE(gname, cname, pname, bidx) \
>> + { \
>> + .name = gname "_cg", \
>> + .id = IMX8ULP_CLK_SIM_LPAV_HIFI_##cname, \
>> + .parent = { .fw_name = pname, .name = pname }, \
>> + .bit = bidx, \
>> + }
>> +
>> +struct clk_imx8ulp_sim_lpav_data {
>> + void __iomem *base;
>> + struct regmap *regmap;
>> + spinlock_t lock; /* shared by MUX, clock gate and reset */
>> + unsigned long flags; /* for spinlock usage */
>> + struct clk_hw_onecell_data clk_data; /* keep last */
>> +};
>> +
>> +struct clk_imx8ulp_sim_lpav_gate {
>> + const char *name;
>> + int id;
>> + const struct clk_parent_data parent;
>> + u8 bit;
>> +};
>> +
>> +static struct clk_imx8ulp_sim_lpav_gate gates[] = {
>> + IMX8ULP_HIFI_CLK_GATE("hifi_core", CORE, "hifi_core", 17),
>> + IMX8ULP_HIFI_CLK_GATE("hifi_pbclk", PBCLK, "lpav_bus", 18),
>> + IMX8ULP_HIFI_CLK_GATE("hifi_plat", PLAT, "hifi_plat", 19)
>> +};
>> +
>> +#ifdef CONFIG_RESET_CONTROLLER
> Needn't this ifdef because if CONFIG_RESET_CONTROLLER, reset driver will
> not probe, just register aux device is not harmful.
ah, yes, was meant to drop this. Will give it a try and do the change for V3 if everything
works well.
>
>> +static void clk_imx8ulp_sim_lpav_aux_reset_release(struct device *dev)
>> +{
>> + struct auxiliary_device *adev = to_auxiliary_dev(dev);
>> +
>> + kfree(adev);
>> +}
>> +
>> +static void clk_imx8ulp_sim_lpav_unregister_aux_reset(void *data)
>> +{
>> + struct auxiliary_device *adev = data;
>> +
>> + auxiliary_device_delete(adev);
>> + auxiliary_device_uninit(adev);
>> +}
>> +
>> +static int clk_imx8ulp_sim_lpav_register_aux_reset(struct platform_device *pdev)
>> +{
>> + struct auxiliary_device *adev __free(kfree) = NULL;
>> + int ret;
>> +
>> + adev = kzalloc(sizeof(*adev), GFP_KERNEL);
>> + if (!adev)
>> + return -ENOMEM;
>> +
>> + adev->name = "reset";
>> + adev->dev.parent = &pdev->dev;
>> + adev->dev.release = clk_imx8ulp_sim_lpav_aux_reset_release;
>> +
>> + ret = auxiliary_device_init(adev);
>> + if (ret) {
>> + dev_err(&pdev->dev, "failed to initialize aux dev\n");
>> + return ret;
>> + }
>> +
>> + ret = auxiliary_device_add(adev);
>> + if (ret) {
>> + auxiliary_device_uninit(adev);
>> + dev_err(&pdev->dev, "failed to add aux dev\n");
>> + return ret;
>> + }
>> +
>> + return devm_add_action_or_reset(&pdev->dev,
>> + clk_imx8ulp_sim_lpav_unregister_aux_reset,
>> + no_free_ptr(adev));
> use devm_auxiliary_device_create() to simple whole code.
good catch. Will give this a try and switch to it in next version if there's no issues.
>
>> +}
>> +#else
>> +static int clk_imx8ulp_sim_lpav_register_aux_reset(struct platform_device *pdev)
>> +{
>> + return 0;
>> +}
>> +#endif /* CONFIG_RESET_CONTROLLER */
>> +
>> +static void clk_imx8ulp_sim_lpav_lock(void *arg) __acquires(&data->lock)
>> +{
>> + struct clk_imx8ulp_sim_lpav_data *data = dev_get_drvdata(arg);
>> +
>> + spin_lock_irqsave(&data->lock, data->flags);
>> +}
>> +
>> +static void clk_imx8ulp_sim_lpav_unlock(void *arg) __releases(&data->lock)
>> +{
>> + struct clk_imx8ulp_sim_lpav_data *data = dev_get_drvdata(arg);
>> +
>> + spin_unlock_irqrestore(&data->lock, data->flags);
>> +}
>> +
>> +static const struct regmap_config clk_imx8ulp_sim_lpav_regmap_cfg = {
>> + .reg_bits = 32,
>> + .val_bits = 32,
>> + .reg_stride = 4,
>> + .lock = clk_imx8ulp_sim_lpav_lock,
>> + .unlock = clk_imx8ulp_sim_lpav_unlock,
> why need lock/unlock here, regmap already have lock to access one register.
yeah but we want to share the lock with the clock gate. I know we can theoretically do
something like "®map->spinlock" but the lock initialization (i.e. which lock we initialize)
depends on how we configure "struct regmap_config", which is why I think my proposal is
more robust since we don't depend on how we fill in the fields of "struct regmap_config"
(other than lock/unlock/lock_arg).
But then again I'm not fixated on current approach so if more people think it would
be better to just use "®map->spinlock" then I'll give that a try.
>
> Frank
>> +};
>> +
>> +static int clk_imx8ulp_sim_lpav_probe(struct platform_device *pdev)
>> +{
>> + struct clk_imx8ulp_sim_lpav_data *data;
>> + struct regmap_config regmap_config;
>> + struct clk_hw *hw;
>> + int i, ret;
>> +
>> + data = devm_kzalloc(&pdev->dev,
>> + struct_size(data, clk_data.hws, ARRAY_SIZE(gates)),
>> + GFP_KERNEL);
>> + if (!data)
>> + return -ENOMEM;
>> +
>> + dev_set_drvdata(&pdev->dev, data);
>> +
>> + memcpy(®map_config, &clk_imx8ulp_sim_lpav_regmap_cfg, sizeof(regmap_config));
>> + regmap_config.lock_arg = &pdev->dev;
>> +
>> + /*
>> + * this lock is used directly by the clock gate and indirectly
>> + * by the reset and mux controller via the regmap API
>> + */
>> + spin_lock_init(&data->lock);
>> +
>> + data->base = devm_platform_ioremap_resource(pdev, 0);
>> + if (IS_ERR(data->base))
>> + return dev_err_probe(&pdev->dev, PTR_ERR(data->base),
>> + "failed to ioremap base\n");
>> + /*
>> + * although the clock gate doesn't use the regmap API to modify the
>> + * registers, we still need the regmap because of the reset auxiliary
>> + * driver and the MUX drivers, which use the parent device's regmap
>> + */
>> + data->regmap = devm_regmap_init_mmio(&pdev->dev, data->base, ®map_config);
>> + if (IS_ERR(data->regmap))
>> + return dev_err_probe(&pdev->dev, PTR_ERR(data->regmap),
>> + "failed to initialize regmap\n");
>> +
>> + data->clk_data.num = ARRAY_SIZE(gates);
>> +
>> + for (i = 0; i < ARRAY_SIZE(gates); i++) {
>> + hw = devm_clk_hw_register_gate_parent_data(&pdev->dev,
>> + gates[i].name,
>> + &gates[i].parent,
>> + CLK_SET_RATE_PARENT,
>> + data->base + SYSCTRL0,
>> + gates[i].bit,
>> + 0x0, &data->lock);
>> + if (IS_ERR(hw))
>> + return dev_err_probe(&pdev->dev, PTR_ERR(hw),
>> + "failed to register %s gate\n",
>> + gates[i].name);
>> +
>> + data->clk_data.hws[i] = hw;
>> + }
>> +
>> + ret = clk_imx8ulp_sim_lpav_register_aux_reset(pdev);
>> + if (ret)
>> + return dev_err_probe(&pdev->dev, ret,
>> + "failed to register aux reset\n");
>> +
>> + ret = devm_of_clk_add_hw_provider(&pdev->dev,
>> + of_clk_hw_onecell_get,
>> + &data->clk_data);
>> + if (ret)
>> + return dev_err_probe(&pdev->dev, ret,
>> + "failed to register clk hw provider\n");
>> +
>> + /* used to probe MUX child device */
>> + return devm_of_platform_populate(&pdev->dev);
>> +}
>> +
>> +static const struct of_device_id clk_imx8ulp_sim_lpav_of_match[] = {
>> + { .compatible = "fsl,imx8ulp-sim-lpav" },
>> + { }
>> +};
>> +MODULE_DEVICE_TABLE(of, clk_imx8ulp_sim_lpav_of_match);
>> +
>> +static struct platform_driver clk_imx8ulp_sim_lpav_driver = {
>> + .probe = clk_imx8ulp_sim_lpav_probe,
>> + .driver = {
>> + .name = "clk-imx8ulp-sim-lpav",
>> + .of_match_table = clk_imx8ulp_sim_lpav_of_match,
>> + },
>> +};
>> +module_platform_driver(clk_imx8ulp_sim_lpav_driver);
>> +
>> +MODULE_LICENSE("GPL");
>> +MODULE_DESCRIPTION("i.MX8ULP LPAV System Integration Module (SIM) clock driver");
>> +MODULE_AUTHOR("Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>");
>> --
>> 2.43.0
>>
© 2016 - 2026 Red Hat, Inc.