[PATCH v2 0/8] Add support for i.MX8ULP's SIM LPAV

[PATCH v2 0/8] Add support for i.MX8ULP's SIM LPAV

[Laurentiu Mihalcea]

From: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>

The LPAV System Integration Module (SIM) is an IP found inside i.MX8ULP's
LPAV subsystem, which offers clock gating, reset line
assertion/de-assertion, and various other misc. options.

This series adds support for the IP by introducing a new clock HW provider
driver and by modifying i.MX8MP's AUDIOMIX block control reset driver to
allow it to be used for i.MX8ULP's SIM LPAV as well.

This series is a spin-off from [1].

[1]: https://lore.kernel.org/lkml/20240922174225.75948-1-laurentiumihalcea111@gmail.com/

---
Changes in v2:

* drop redundant description for "#clock-cells"/"#reset-cells" properties
  from binding
* make "mux-controller" and "#reset-cells" properties mandatory
* add "mux-controller" child to binding example node
* drop the compatiblity with syscon - this was never actually needed and
  it was done to allow using "mmio-mux", which requires a syscon parent
* modify the auxiliary reset driver to use regmap - this will allow the
  mux controller, clock control and reset control APIs to use the same
  spinlock
* rename "imx8ulp-reset-sim-lpav.h" to "fsl,imx8ulp-sim-lpav.h"
* drop the "IMX8ULP_CLK_SIM_LPAV_END" macro, which doesn't belong to the
  binding headers
* fix the values of "IMX8MP_AUDIOMIX_EARC_RESET_MASK" and
  "IMX8MP_AUDIOMIX_EARC_PHY_RESET_MASK", which were previously incorrect
* drop redundant macros from auxiliary reset driver - replace their
  occurrences with their values
* squash the binding-related macro additions into the binding patch
* add mux-controller child node to the sim_lpav node
* make the "gates" array static
* link to v1: https://lore.kernel.org/lkml/20250804155407.285353-1-laurentiumihalcea111@gmail.com/
---

Laurentiu Mihalcea (8):
  reset: imx8mp-audiomix: Fix bad mask values
  dt-bindings: clock: document 8ULP's SIM LPAV
  clk: imx: add driver for imx8ulp's sim lpav
  reset: imx8mp-audiomix: Drop unneeded macros
  reset: imx8mp-audiomix: Switch to using regmap API
  reset: imx8mp-audiomix: Extend the driver usage
  reset: imx8mp-audiomix: Support i.MX8ULP SIM LPAV
  arm64: dts: imx8ulp: add sim lpav node

 .../bindings/clock/fsl,imx8ulp-sim-lpav.yaml  |  72 ++++++
 arch/arm64/boot/dts/freescale/imx8ulp.dtsi    |  17 ++
 drivers/clk/imx/Makefile                      |   1 +
 drivers/clk/imx/clk-imx8ulp-sim-lpav.c        | 211 ++++++++++++++++++
 drivers/reset/reset-imx8mp-audiomix.c         | 174 +++++++++++----
 include/dt-bindings/clock/imx8ulp-clock.h     |   5 +
 .../dt-bindings/reset/fsl,imx8ulp-sim-lpav.h  |  16 ++
 7 files changed, 453 insertions(+), 43 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/clock/fsl,imx8ulp-sim-lpav.yaml
 create mode 100644 drivers/clk/imx/clk-imx8ulp-sim-lpav.c
 create mode 100644 include/dt-bindings/reset/fsl,imx8ulp-sim-lpav.h

-- 
2.43.0

[PATCH v2 1/8] reset: imx8mp-audiomix: Fix bad mask values

[Laurentiu Mihalcea]

From: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>

As per the i.MX8MP TRM, section 14.2 "AUDIO_BLK_CTRL", table 14.2.3.1.1
"memory map", the definition of the EARC control register shows that the
EARC controller software reset is controlled via bit 0, while the EARC PHY
software reset is controlled via bit 1.

This means that the current definitions of IMX8MP_AUDIOMIX_EARC_RESET_MASK
and IMX8MP_AUDIOMIX_EARC_PHY_RESET_MASK are wrong since their values would
imply that the EARC controller software reset is controlled via bit 1 and
the EARC PHY software reset is controlled via bit 2. Fix them.

Fixes: a83bc87cd30a ("reset: imx8mp-audiomix: Prepare the code for more reset bits")
Cc: stable@vger.kernel.org
Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
---
 drivers/reset/reset-imx8mp-audiomix.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/reset/reset-imx8mp-audiomix.c b/drivers/reset/reset-imx8mp-audiomix.c
index 6b357adfe646..eceb37ff5dc5 100644
--- a/drivers/reset/reset-imx8mp-audiomix.c
+++ b/drivers/reset/reset-imx8mp-audiomix.c
@@ -14,8 +14,8 @@
 #include <linux/reset-controller.h>
 
 #define IMX8MP_AUDIOMIX_EARC_RESET_OFFSET	0x200
-#define IMX8MP_AUDIOMIX_EARC_RESET_MASK		BIT(1)
-#define IMX8MP_AUDIOMIX_EARC_PHY_RESET_MASK	BIT(2)
+#define IMX8MP_AUDIOMIX_EARC_RESET_MASK		BIT(0)
+#define IMX8MP_AUDIOMIX_EARC_PHY_RESET_MASK	BIT(1)
 
 #define IMX8MP_AUDIOMIX_DSP_RUNSTALL_OFFSET	0x108
 #define IMX8MP_AUDIOMIX_DSP_RUNSTALL_MASK	BIT(5)
-- 
2.43.0

Re: [PATCH v2 1/8] reset: imx8mp-audiomix: Fix bad mask values

[Frank Li]

On Fri, Oct 17, 2025 at 04:20:18AM -0700, Laurentiu Mihalcea wrote:
> From: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
>
> As per the i.MX8MP TRM, section 14.2 "AUDIO_BLK_CTRL", table 14.2.3.1.1
> "memory map", the definition of the EARC control register shows that the
> EARC controller software reset is controlled via bit 0, while the EARC PHY
> software reset is controlled via bit 1.
>
> This means that the current definitions of IMX8MP_AUDIOMIX_EARC_RESET_MASK
> and IMX8MP_AUDIOMIX_EARC_PHY_RESET_MASK are wrong since their values would
> imply that the EARC controller software reset is controlled via bit 1 and
> the EARC PHY software reset is controlled via bit 2. Fix them.
>
> Fixes: a83bc87cd30a ("reset: imx8mp-audiomix: Prepare the code for more reset bits")
> Cc: stable@vger.kernel.org
> Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>

Just curious, why previous version can work?

Reviewed-by: Frank Li <Frank.Li@nxp.com>
> ---
>  drivers/reset/reset-imx8mp-audiomix.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/reset/reset-imx8mp-audiomix.c b/drivers/reset/reset-imx8mp-audiomix.c
> index 6b357adfe646..eceb37ff5dc5 100644
> --- a/drivers/reset/reset-imx8mp-audiomix.c
> +++ b/drivers/reset/reset-imx8mp-audiomix.c
> @@ -14,8 +14,8 @@
>  #include <linux/reset-controller.h>
>
>  #define IMX8MP_AUDIOMIX_EARC_RESET_OFFSET	0x200
> -#define IMX8MP_AUDIOMIX_EARC_RESET_MASK		BIT(1)
> -#define IMX8MP_AUDIOMIX_EARC_PHY_RESET_MASK	BIT(2)
> +#define IMX8MP_AUDIOMIX_EARC_RESET_MASK		BIT(0)
> +#define IMX8MP_AUDIOMIX_EARC_PHY_RESET_MASK	BIT(1)
>
>  #define IMX8MP_AUDIOMIX_DSP_RUNSTALL_OFFSET	0x108
>  #define IMX8MP_AUDIOMIX_DSP_RUNSTALL_MASK	BIT(5)
> --
> 2.43.0
>

Re: [PATCH v2 1/8] reset: imx8mp-audiomix: Fix bad mask values

[Laurentiu Mihalcea]

On 10/17/2025 7:28 AM, Frank Li wrote:
> On Fri, Oct 17, 2025 at 04:20:18AM -0700, Laurentiu Mihalcea wrote:
>> From: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
>>
>> As per the i.MX8MP TRM, section 14.2 "AUDIO_BLK_CTRL", table 14.2.3.1.1
>> "memory map", the definition of the EARC control register shows that the
>> EARC controller software reset is controlled via bit 0, while the EARC PHY
>> software reset is controlled via bit 1.
>>
>> This means that the current definitions of IMX8MP_AUDIOMIX_EARC_RESET_MASK
>> and IMX8MP_AUDIOMIX_EARC_PHY_RESET_MASK are wrong since their values would
>> imply that the EARC controller software reset is controlled via bit 1 and
>> the EARC PHY software reset is controlled via bit 2. Fix them.
>>
>> Fixes: a83bc87cd30a ("reset: imx8mp-audiomix: Prepare the code for more reset bits")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
> Just curious, why previous version can work?


good question. I assume this was never tested so I don't think the question

of this working in previous versions or not is applicable here. I also looked for

the usages of these macros in the DTS and there seems to be no consumer.


I discovered the issue while testing the patches from this series. As for testing,

I used a dummy consumer driver/node and manually checked the register values

after each reset assert/de-assert operation.


>
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
>> ---
>>  drivers/reset/reset-imx8mp-audiomix.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/reset/reset-imx8mp-audiomix.c b/drivers/reset/reset-imx8mp-audiomix.c
>> index 6b357adfe646..eceb37ff5dc5 100644
>> --- a/drivers/reset/reset-imx8mp-audiomix.c
>> +++ b/drivers/reset/reset-imx8mp-audiomix.c
>> @@ -14,8 +14,8 @@
>>  #include <linux/reset-controller.h>
>>
>>  #define IMX8MP_AUDIOMIX_EARC_RESET_OFFSET	0x200
>> -#define IMX8MP_AUDIOMIX_EARC_RESET_MASK		BIT(1)
>> -#define IMX8MP_AUDIOMIX_EARC_PHY_RESET_MASK	BIT(2)
>> +#define IMX8MP_AUDIOMIX_EARC_RESET_MASK		BIT(0)
>> +#define IMX8MP_AUDIOMIX_EARC_PHY_RESET_MASK	BIT(1)
>>
>>  #define IMX8MP_AUDIOMIX_DSP_RUNSTALL_OFFSET	0x108
>>  #define IMX8MP_AUDIOMIX_DSP_RUNSTALL_MASK	BIT(5)
>> --
>> 2.43.0
>>

Re: [PATCH v2 1/8] reset: imx8mp-audiomix: Fix bad mask values

[Shengjiu Wang]

On Fri, Oct 17, 2025 at 7:22 PM Laurentiu Mihalcea
<laurentiumihalcea111@gmail.com> wrote:
>
> From: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
>
> As per the i.MX8MP TRM, section 14.2 "AUDIO_BLK_CTRL", table 14.2.3.1.1
> "memory map", the definition of the EARC control register shows that the
> EARC controller software reset is controlled via bit 0, while the EARC PHY
> software reset is controlled via bit 1.
>
> This means that the current definitions of IMX8MP_AUDIOMIX_EARC_RESET_MASK
> and IMX8MP_AUDIOMIX_EARC_PHY_RESET_MASK are wrong since their values would
> imply that the EARC controller software reset is controlled via bit 1 and
> the EARC PHY software reset is controlled via bit 2. Fix them.
>
> Fixes: a83bc87cd30a ("reset: imx8mp-audiomix: Prepare the code for more reset bits")
> Cc: stable@vger.kernel.org
> Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>

Reviewed-by: Shengjiu Wang <shengjiu.wang@gmail.com>

best regards
shengjiu wang
> ---
>  drivers/reset/reset-imx8mp-audiomix.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/reset/reset-imx8mp-audiomix.c b/drivers/reset/reset-imx8mp-audiomix.c
> index 6b357adfe646..eceb37ff5dc5 100644
> --- a/drivers/reset/reset-imx8mp-audiomix.c
> +++ b/drivers/reset/reset-imx8mp-audiomix.c
> @@ -14,8 +14,8 @@
>  #include <linux/reset-controller.h>
>
>  #define IMX8MP_AUDIOMIX_EARC_RESET_OFFSET      0x200
> -#define IMX8MP_AUDIOMIX_EARC_RESET_MASK                BIT(1)
> -#define IMX8MP_AUDIOMIX_EARC_PHY_RESET_MASK    BIT(2)
> +#define IMX8MP_AUDIOMIX_EARC_RESET_MASK                BIT(0)
> +#define IMX8MP_AUDIOMIX_EARC_PHY_RESET_MASK    BIT(1)
>
>  #define IMX8MP_AUDIOMIX_DSP_RUNSTALL_OFFSET    0x108
>  #define IMX8MP_AUDIOMIX_DSP_RUNSTALL_MASK      BIT(5)
> --
> 2.43.0
>
>

Re: [PATCH v2 1/8] reset: imx8mp-audiomix: Fix bad mask values

[Daniel Baluta]

On Fri, Oct 17, 2025 at 2:22 PM Laurentiu Mihalcea
<laurentiumihalcea111@gmail.com> wrote:
>
> From: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
>
> As per the i.MX8MP TRM, section 14.2 "AUDIO_BLK_CTRL", table 14.2.3.1.1
> "memory map", the definition of the EARC control register shows that the
> EARC controller software reset is controlled via bit 0, while the EARC PHY
> software reset is controlled via bit 1.
>
> This means that the current definitions of IMX8MP_AUDIOMIX_EARC_RESET_MASK
> and IMX8MP_AUDIOMIX_EARC_PHY_RESET_MASK are wrong since their values would
> imply that the EARC controller software reset is controlled via bit 1 and
> the EARC PHY software reset is controlled via bit 2. Fix them.
>
> Fixes: a83bc87cd30a ("reset: imx8mp-audiomix: Prepare the code for more reset bits")
> Cc: stable@vger.kernel.org
> Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>

Reviewed-by: Daniel Baluta <daniel.baluta@nxp.com>

[PATCH v2 2/8] dt-bindings: clock: document 8ULP's SIM LPAV

[Laurentiu Mihalcea]

From: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>

Add documentation for i.MX8ULP's SIM LPAV module.

Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
---
 .../bindings/clock/fsl,imx8ulp-sim-lpav.yaml  | 72 +++++++++++++++++++
 include/dt-bindings/clock/imx8ulp-clock.h     |  5 ++
 .../dt-bindings/reset/fsl,imx8ulp-sim-lpav.h  | 16 +++++
 3 files changed, 93 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/fsl,imx8ulp-sim-lpav.yaml
 create mode 100644 include/dt-bindings/reset/fsl,imx8ulp-sim-lpav.h

diff --git a/Documentation/devicetree/bindings/clock/fsl,imx8ulp-sim-lpav.yaml b/Documentation/devicetree/bindings/clock/fsl,imx8ulp-sim-lpav.yaml
new file mode 100644
index 000000000000..fb3b9028a4c3
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/fsl,imx8ulp-sim-lpav.yaml
@@ -0,0 +1,72 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/fsl,imx8ulp-sim-lpav.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NXP i.MX8ULP LPAV System Integration Module (SIM)
+
+maintainers:
+  - Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
+
+description:
+  The i.MX8ULP LPAV subsystem contains a block control module known as
+  SIM LPAV, which offers functionalities such as clock gating or reset
+  line assertion/de-assertion.
+
+properties:
+  compatible:
+    const: fsl,imx8ulp-sim-lpav
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 3
+
+  clock-names:
+    items:
+      - const: lpav_bus
+      - const: hifi_core
+      - const: hifi_plat
+
+  '#clock-cells':
+    const: 1
+
+  '#reset-cells':
+    const: 1
+
+  mux-controller:
+    $ref: /schemas/mux/reg-mux.yaml#
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - '#clock-cells'
+  - '#reset-cells'
+  - mux-controller
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/imx8ulp-clock.h>
+
+    clock-controller@2da50000 {
+        compatible = "fsl,imx8ulp-sim-lpav";
+        reg = <0x2da50000 0x10000>;
+        clocks = <&cgc2 IMX8ULP_CLK_LPAV_BUS_DIV>,
+                 <&cgc2 IMX8ULP_CLK_HIFI_DIVCORE>,
+                 <&cgc2 IMX8ULP_CLK_HIFI_DIVPLAT>;
+        clock-names = "lpav_bus", "hifi_core", "hifi_plat";
+        #clock-cells = <1>;
+        #reset-cells = <1>;
+
+        mux-controller {
+            compatible = "reg-mux";
+            #mux-control-cells = <1>;
+            mux-reg-masks = <0x8 0x00000200>;
+        };
+    };
diff --git a/include/dt-bindings/clock/imx8ulp-clock.h b/include/dt-bindings/clock/imx8ulp-clock.h
index 827404fadf5c..c62d84d093a9 100644
--- a/include/dt-bindings/clock/imx8ulp-clock.h
+++ b/include/dt-bindings/clock/imx8ulp-clock.h
@@ -255,4 +255,9 @@
 
 #define IMX8ULP_CLK_PCC5_END		56
 
+/* LPAV SIM */
+#define IMX8ULP_CLK_SIM_LPAV_HIFI_CORE		0
+#define IMX8ULP_CLK_SIM_LPAV_HIFI_PBCLK		1
+#define IMX8ULP_CLK_SIM_LPAV_HIFI_PLAT		2
+
 #endif
diff --git a/include/dt-bindings/reset/fsl,imx8ulp-sim-lpav.h b/include/dt-bindings/reset/fsl,imx8ulp-sim-lpav.h
new file mode 100644
index 000000000000..adf95bb26d21
--- /dev/null
+++ b/include/dt-bindings/reset/fsl,imx8ulp-sim-lpav.h
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+/*
+ * Copyright 2025 NXP
+ */
+
+#ifndef DT_BINDING_RESET_IMX8ULP_SIM_LPAV_H
+#define DT_BINDING_RESET_IMX8ULP_SIM_LPAV_H
+
+#define IMX8ULP_SIM_LPAV_HIFI4_DSP_DBG_RST	0
+#define IMX8ULP_SIM_LPAV_HIFI4_DSP_RST		1
+#define IMX8ULP_SIM_LPAV_HIFI4_DSP_STALL	2
+#define IMX8ULP_SIM_LPAV_DSI_RST_BYTE_N		3
+#define IMX8ULP_SIM_LPAV_DSI_RST_ESC_N		4
+#define IMX8ULP_SIM_LPAV_DSI_RST_DPI_N		5
+
+#endif /* DT_BINDING_RESET_IMX8ULP_SIM_LPAV_H */
-- 
2.43.0

Re: [PATCH v2 2/8] dt-bindings: clock: document 8ULP's SIM LPAV

[Frank Li]

On Fri, Oct 17, 2025 at 04:20:19AM -0700, Laurentiu Mihalcea wrote:
> From: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
>
> Add documentation for i.MX8ULP's SIM LPAV module.
>
> Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
> ---
>  .../bindings/clock/fsl,imx8ulp-sim-lpav.yaml  | 72 +++++++++++++++++++
>  include/dt-bindings/clock/imx8ulp-clock.h     |  5 ++
>  .../dt-bindings/reset/fsl,imx8ulp-sim-lpav.h  | 16 +++++
>  3 files changed, 93 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/fsl,imx8ulp-sim-lpav.yaml
>  create mode 100644 include/dt-bindings/reset/fsl,imx8ulp-sim-lpav.h
>
> diff --git a/Documentation/devicetree/bindings/clock/fsl,imx8ulp-sim-lpav.yaml b/Documentation/devicetree/bindings/clock/fsl,imx8ulp-sim-lpav.yaml
> new file mode 100644
> index 000000000000..fb3b9028a4c3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/fsl,imx8ulp-sim-lpav.yaml
> @@ -0,0 +1,72 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/fsl,imx8ulp-sim-lpav.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NXP i.MX8ULP LPAV System Integration Module (SIM)
> +
> +maintainers:
> +  - Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
> +
> +description:
> +  The i.MX8ULP LPAV subsystem contains a block control module known as
> +  SIM LPAV, which offers functionalities such as clock gating or reset
> +  line assertion/de-assertion.
> +
> +properties:
> +  compatible:
> +    const: fsl,imx8ulp-sim-lpav
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 3
> +
> +  clock-names:
> +    items:
> +      - const: lpav_bus
> +      - const: hifi_core
> +      - const: hifi_plat
> +
> +  '#clock-cells':
> +    const: 1
> +
> +  '#reset-cells':
> +    const: 1
> +
> +  mux-controller:
> +    $ref: /schemas/mux/reg-mux.yaml#
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - '#clock-cells'
> +  - '#reset-cells'
> +  - mux-controller
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/imx8ulp-clock.h>
> +
> +    clock-controller@2da50000 {

Maybe mfd is better, this is clock, reset and mux controller actually.

Frank Li

> +        compatible = "fsl,imx8ulp-sim-lpav";
> +        reg = <0x2da50000 0x10000>;
> +        clocks = <&cgc2 IMX8ULP_CLK_LPAV_BUS_DIV>,
> +                 <&cgc2 IMX8ULP_CLK_HIFI_DIVCORE>,
> +                 <&cgc2 IMX8ULP_CLK_HIFI_DIVPLAT>;
> +        clock-names = "lpav_bus", "hifi_core", "hifi_plat";
> +        #clock-cells = <1>;
> +        #reset-cells = <1>;
> +
> +        mux-controller {
> +            compatible = "reg-mux";
> +            #mux-control-cells = <1>;
> +            mux-reg-masks = <0x8 0x00000200>;
> +        };
> +    };
> diff --git a/include/dt-bindings/clock/imx8ulp-clock.h b/include/dt-bindings/clock/imx8ulp-clock.h
> index 827404fadf5c..c62d84d093a9 100644
> --- a/include/dt-bindings/clock/imx8ulp-clock.h
> +++ b/include/dt-bindings/clock/imx8ulp-clock.h
> @@ -255,4 +255,9 @@
>
>  #define IMX8ULP_CLK_PCC5_END		56
>
> +/* LPAV SIM */
> +#define IMX8ULP_CLK_SIM_LPAV_HIFI_CORE		0
> +#define IMX8ULP_CLK_SIM_LPAV_HIFI_PBCLK		1
> +#define IMX8ULP_CLK_SIM_LPAV_HIFI_PLAT		2
> +
>  #endif
> diff --git a/include/dt-bindings/reset/fsl,imx8ulp-sim-lpav.h b/include/dt-bindings/reset/fsl,imx8ulp-sim-lpav.h
> new file mode 100644
> index 000000000000..adf95bb26d21
> --- /dev/null
> +++ b/include/dt-bindings/reset/fsl,imx8ulp-sim-lpav.h
> @@ -0,0 +1,16 @@
> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
> +/*
> + * Copyright 2025 NXP
> + */
> +
> +#ifndef DT_BINDING_RESET_IMX8ULP_SIM_LPAV_H
> +#define DT_BINDING_RESET_IMX8ULP_SIM_LPAV_H
> +
> +#define IMX8ULP_SIM_LPAV_HIFI4_DSP_DBG_RST	0
> +#define IMX8ULP_SIM_LPAV_HIFI4_DSP_RST		1
> +#define IMX8ULP_SIM_LPAV_HIFI4_DSP_STALL	2
> +#define IMX8ULP_SIM_LPAV_DSI_RST_BYTE_N		3
> +#define IMX8ULP_SIM_LPAV_DSI_RST_ESC_N		4
> +#define IMX8ULP_SIM_LPAV_DSI_RST_DPI_N		5
> +
> +#endif /* DT_BINDING_RESET_IMX8ULP_SIM_LPAV_H */
> --
> 2.43.0
>

Re: [PATCH v2 2/8] dt-bindings: clock: document 8ULP's SIM LPAV

[Krzysztof Kozlowski]

On 17/10/2025 16:33, Frank Li wrote:
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/clock/imx8ulp-clock.h>
>> +
>> +    clock-controller@2da50000 {
> 
> Maybe mfd is better, this is clock, reset and mux controller actually.

There is no such hardware term as mfd.

Best regards,
Krzysztof

Re: [PATCH v2 2/8] dt-bindings: clock: document 8ULP's SIM LPAV

[Frank Li]

On Fri, Oct 17, 2025 at 04:20:19AM -0700, Laurentiu Mihalcea wrote:
> From: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
>
> Add documentation for i.MX8ULP's SIM LPAV module.
>
> Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
> ---
>  .../bindings/clock/fsl,imx8ulp-sim-lpav.yaml  | 72 +++++++++++++++++++
>  include/dt-bindings/clock/imx8ulp-clock.h     |  5 ++
>  .../dt-bindings/reset/fsl,imx8ulp-sim-lpav.h  | 16 +++++
>  3 files changed, 93 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/fsl,imx8ulp-sim-lpav.yaml
>  create mode 100644 include/dt-bindings/reset/fsl,imx8ulp-sim-lpav.h
>
> diff --git a/Documentation/devicetree/bindings/clock/fsl,imx8ulp-sim-lpav.yaml b/Documentation/devicetree/bindings/clock/fsl,imx8ulp-sim-lpav.yaml
> new file mode 100644
> index 000000000000..fb3b9028a4c3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/fsl,imx8ulp-sim-lpav.yaml
> @@ -0,0 +1,72 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/fsl,imx8ulp-sim-lpav.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NXP i.MX8ULP LPAV System Integration Module (SIM)
> +
> +maintainers:
> +  - Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
> +
> +description:
> +  The i.MX8ULP LPAV subsystem contains a block control module known as
> +  SIM LPAV, which offers functionalities such as clock gating or reset
> +  line assertion/de-assertion.
> +
> +properties:
> +  compatible:
> +    const: fsl,imx8ulp-sim-lpav
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 3
> +
> +  clock-names:
> +    items:
> +      - const: lpav_bus
> +      - const: hifi_core
> +      - const: hifi_plat

dt prefer use -
lpav-bus, ...

> +
> +  '#clock-cells':
> +    const: 1
> +
> +  '#reset-cells':
> +    const: 1
> +
> +  mux-controller:
> +    $ref: /schemas/mux/reg-mux.yaml#
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - '#clock-cells'
> +  - '#reset-cells'
> +  - mux-controller
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/imx8ulp-clock.h>
> +
> +    clock-controller@2da50000 {
> +        compatible = "fsl,imx8ulp-sim-lpav";
> +        reg = <0x2da50000 0x10000>;
> +        clocks = <&cgc2 IMX8ULP_CLK_LPAV_BUS_DIV>,
> +                 <&cgc2 IMX8ULP_CLK_HIFI_DIVCORE>,
> +                 <&cgc2 IMX8ULP_CLK_HIFI_DIVPLAT>;
> +        clock-names = "lpav_bus", "hifi_core", "hifi_plat";
> +        #clock-cells = <1>;
> +        #reset-cells = <1>;
> +
> +        mux-controller {
> +            compatible = "reg-mux";
> +            #mux-control-cells = <1>;
> +            mux-reg-masks = <0x8 0x00000200>;
> +        };
> +    };
> diff --git a/include/dt-bindings/clock/imx8ulp-clock.h b/include/dt-bindings/clock/imx8ulp-clock.h
> index 827404fadf5c..c62d84d093a9 100644
> --- a/include/dt-bindings/clock/imx8ulp-clock.h
> +++ b/include/dt-bindings/clock/imx8ulp-clock.h
> @@ -255,4 +255,9 @@
>
>  #define IMX8ULP_CLK_PCC5_END		56
>
> +/* LPAV SIM */
> +#define IMX8ULP_CLK_SIM_LPAV_HIFI_CORE		0
> +#define IMX8ULP_CLK_SIM_LPAV_HIFI_PBCLK		1
> +#define IMX8ULP_CLK_SIM_LPAV_HIFI_PLAT		2
> +
>  #endif
> diff --git a/include/dt-bindings/reset/fsl,imx8ulp-sim-lpav.h b/include/dt-bindings/reset/fsl,imx8ulp-sim-lpav.h
> new file mode 100644
> index 000000000000..adf95bb26d21
> --- /dev/null
> +++ b/include/dt-bindings/reset/fsl,imx8ulp-sim-lpav.h
> @@ -0,0 +1,16 @@
> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
> +/*
> + * Copyright 2025 NXP
> + */
> +
> +#ifndef DT_BINDING_RESET_IMX8ULP_SIM_LPAV_H
> +#define DT_BINDING_RESET_IMX8ULP_SIM_LPAV_H
> +
> +#define IMX8ULP_SIM_LPAV_HIFI4_DSP_DBG_RST	0
> +#define IMX8ULP_SIM_LPAV_HIFI4_DSP_RST		1
> +#define IMX8ULP_SIM_LPAV_HIFI4_DSP_STALL	2
> +#define IMX8ULP_SIM_LPAV_DSI_RST_BYTE_N		3
> +#define IMX8ULP_SIM_LPAV_DSI_RST_ESC_N		4
> +#define IMX8ULP_SIM_LPAV_DSI_RST_DPI_N		5
> +
> +#endif /* DT_BINDING_RESET_IMX8ULP_SIM_LPAV_H */
> --
> 2.43.0
>

Re: [PATCH v2 2/8] dt-bindings: clock: document 8ULP's SIM LPAV

[Krzysztof Kozlowski]

On 17/10/2025 16:59, Frank Li wrote:
> On Fri, Oct 17, 2025 at 04:20:19AM -0700, Laurentiu Mihalcea wrote:
>> From: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
>>
>> Add documentation for i.MX8ULP's SIM LPAV module.
>>
>> Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
>> ---
>>  .../bindings/clock/fsl,imx8ulp-sim-lpav.yaml  | 72 +++++++++++++++++++
>>  include/dt-bindings/clock/imx8ulp-clock.h     |  5 ++
>>  .../dt-bindings/reset/fsl,imx8ulp-sim-lpav.h  | 16 +++++
>>  3 files changed, 93 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/clock/fsl,imx8ulp-sim-lpav.yaml
>>  create mode 100644 include/dt-bindings/reset/fsl,imx8ulp-sim-lpav.h
>>
>> diff --git a/Documentation/devicetree/bindings/clock/fsl,imx8ulp-sim-lpav.yaml b/Documentation/devicetree/bindings/clock/fsl,imx8ulp-sim-lpav.yaml
>> new file mode 100644
>> index 000000000000..fb3b9028a4c3
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/clock/fsl,imx8ulp-sim-lpav.yaml
>> @@ -0,0 +1,72 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/clock/fsl,imx8ulp-sim-lpav.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: NXP i.MX8ULP LPAV System Integration Module (SIM)
>> +
>> +maintainers:
>> +  - Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
>> +
>> +description:
>> +  The i.MX8ULP LPAV subsystem contains a block control module known as
>> +  SIM LPAV, which offers functionalities such as clock gating or reset
>> +  line assertion/de-assertion.
>> +
>> +properties:
>> +  compatible:
>> +    const: fsl,imx8ulp-sim-lpav
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  clocks:
>> +    maxItems: 3
>> +
>> +  clock-names:
>> +    items:
>> +      - const: lpav_bus
>> +      - const: hifi_core
>> +      - const: hifi_plat
> 
> dt prefer use -


I don't think we ever expressed such preference. Where did you find it?


> lpav-bus, ...
Then just "bus" or "apb".



Best regards,
Krzysztof

Re: [PATCH v2 2/8] dt-bindings: clock: document 8ULP's SIM LPAV

[Frank Li]

On Sun, Oct 19, 2025 at 12:05:27PM +0200, Krzysztof Kozlowski wrote:
> On 17/10/2025 16:59, Frank Li wrote:
> > On Fri, Oct 17, 2025 at 04:20:19AM -0700, Laurentiu Mihalcea wrote:
> >> From: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
> >>
> >> Add documentation for i.MX8ULP's SIM LPAV module.
> >>
> >> Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
> >> ---
> >>  .../bindings/clock/fsl,imx8ulp-sim-lpav.yaml  | 72 +++++++++++++++++++
> >>  include/dt-bindings/clock/imx8ulp-clock.h     |  5 ++
> >>  .../dt-bindings/reset/fsl,imx8ulp-sim-lpav.h  | 16 +++++
> >>  3 files changed, 93 insertions(+)
> >>  create mode 100644 Documentation/devicetree/bindings/clock/fsl,imx8ulp-sim-lpav.yaml
> >>  create mode 100644 include/dt-bindings/reset/fsl,imx8ulp-sim-lpav.h
> >>
> >> diff --git a/Documentation/devicetree/bindings/clock/fsl,imx8ulp-sim-lpav.yaml b/Documentation/devicetree/bindings/clock/fsl,imx8ulp-sim-lpav.yaml
> >> new file mode 100644
> >> index 000000000000..fb3b9028a4c3
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/clock/fsl,imx8ulp-sim-lpav.yaml
> >> @@ -0,0 +1,72 @@
> >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> >> +%YAML 1.2
> >> +---
> >> +$id: http://devicetree.org/schemas/clock/fsl,imx8ulp-sim-lpav.yaml#
> >> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >> +
> >> +title: NXP i.MX8ULP LPAV System Integration Module (SIM)
> >> +
> >> +maintainers:
> >> +  - Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
> >> +
> >> +description:
> >> +  The i.MX8ULP LPAV subsystem contains a block control module known as
> >> +  SIM LPAV, which offers functionalities such as clock gating or reset
> >> +  line assertion/de-assertion.
> >> +
> >> +properties:
> >> +  compatible:
> >> +    const: fsl,imx8ulp-sim-lpav
> >> +
> >> +  reg:
> >> +    maxItems: 1
> >> +
> >> +  clocks:
> >> +    maxItems: 3
> >> +
> >> +  clock-names:
> >> +    items:
> >> +      - const: lpav_bus
> >> +      - const: hifi_core
> >> +      - const: hifi_plat
> >
> > dt prefer use -
>
>
> I don't think we ever expressed such preference. Where did you find it?

It should come from review message when submit binding-doc patch.  but I
can't find it now. But at least, compatible string and node-name use "-".

It'd better to add to writing-bindings.rst. It is hard to search whole
linux-devicetree mail list or brain may cheat me. It is good to keep
everything consistent.

like:
     " vs '
     VCC-supply vs vcc-supply
     ...

>
>
> > lpav-bus, ...
> Then just "bus" or "apb".

core, plat is also better than hifi_core, hifi_plat

Frank
>
>
>
> Best regards,
> Krzysztof

Re: [PATCH v2 2/8] dt-bindings: clock: document 8ULP's SIM LPAV

[Krzysztof Kozlowski]

On 20/10/2025 17:22, Frank Li wrote:
> On Sun, Oct 19, 2025 at 12:05:27PM +0200, Krzysztof Kozlowski wrote:
>> On 17/10/2025 16:59, Frank Li wrote:
>>> On Fri, Oct 17, 2025 at 04:20:19AM -0700, Laurentiu Mihalcea wrote:
>>>> From: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
>>>>
>>>> Add documentation for i.MX8ULP's SIM LPAV module.
>>>>
>>>> Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
>>>> ---
>>>>  .../bindings/clock/fsl,imx8ulp-sim-lpav.yaml  | 72 +++++++++++++++++++
>>>>  include/dt-bindings/clock/imx8ulp-clock.h     |  5 ++
>>>>  .../dt-bindings/reset/fsl,imx8ulp-sim-lpav.h  | 16 +++++
>>>>  3 files changed, 93 insertions(+)
>>>>  create mode 100644 Documentation/devicetree/bindings/clock/fsl,imx8ulp-sim-lpav.yaml
>>>>  create mode 100644 include/dt-bindings/reset/fsl,imx8ulp-sim-lpav.h
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/clock/fsl,imx8ulp-sim-lpav.yaml b/Documentation/devicetree/bindings/clock/fsl,imx8ulp-sim-lpav.yaml
>>>> new file mode 100644
>>>> index 000000000000..fb3b9028a4c3
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/clock/fsl,imx8ulp-sim-lpav.yaml
>>>> @@ -0,0 +1,72 @@
>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>>> +%YAML 1.2
>>>> +---
>>>> +$id: http://devicetree.org/schemas/clock/fsl,imx8ulp-sim-lpav.yaml#
>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>> +
>>>> +title: NXP i.MX8ULP LPAV System Integration Module (SIM)
>>>> +
>>>> +maintainers:
>>>> +  - Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
>>>> +
>>>> +description:
>>>> +  The i.MX8ULP LPAV subsystem contains a block control module known as
>>>> +  SIM LPAV, which offers functionalities such as clock gating or reset
>>>> +  line assertion/de-assertion.
>>>> +
>>>> +properties:
>>>> +  compatible:
>>>> +    const: fsl,imx8ulp-sim-lpav
>>>> +
>>>> +  reg:
>>>> +    maxItems: 1
>>>> +
>>>> +  clocks:
>>>> +    maxItems: 3
>>>> +
>>>> +  clock-names:
>>>> +    items:
>>>> +      - const: lpav_bus
>>>> +      - const: hifi_core
>>>> +      - const: hifi_plat
>>>
>>> dt prefer use -
>>
>>
>> I don't think we ever expressed such preference. Where did you find it?
> 
> It should come from review message when submit binding-doc patch.  but I
> can't find it now. But at least, compatible string and node-name use "-".
> 
> It'd better to add to writing-bindings.rst. It is hard to search whole
> linux-devicetree mail list or brain may cheat me. It is good to keep
> everything consistent.


Yeah, but please don't make up rules and suggest them if you cannot
justify them. I don't recall such rule and I don't have it in my notes,
so that's why I asked.

Best regards,
Krzysztof

Re: [PATCH v2 2/8] dt-bindings: clock: document 8ULP's SIM LPAV

[Laurentiu Mihalcea]

On 10/20/2025 8:22 AM, Frank Li wrote:
> On Sun, Oct 19, 2025 at 12:05:27PM +0200, Krzysztof Kozlowski wrote:
>> On 17/10/2025 16:59, Frank Li wrote:
>>> On Fri, Oct 17, 2025 at 04:20:19AM -0700, Laurentiu Mihalcea wrote:
>>>> From: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
>>>>
>>>> Add documentation for i.MX8ULP's SIM LPAV module.
>>>>
>>>> Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
>>>> ---
>>>>  .../bindings/clock/fsl,imx8ulp-sim-lpav.yaml  | 72 +++++++++++++++++++
>>>>  include/dt-bindings/clock/imx8ulp-clock.h     |  5 ++
>>>>  .../dt-bindings/reset/fsl,imx8ulp-sim-lpav.h  | 16 +++++
>>>>  3 files changed, 93 insertions(+)
>>>>  create mode 100644 Documentation/devicetree/bindings/clock/fsl,imx8ulp-sim-lpav.yaml
>>>>  create mode 100644 include/dt-bindings/reset/fsl,imx8ulp-sim-lpav.h
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/clock/fsl,imx8ulp-sim-lpav.yaml b/Documentation/devicetree/bindings/clock/fsl,imx8ulp-sim-lpav.yaml
>>>> new file mode 100644
>>>> index 000000000000..fb3b9028a4c3
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/clock/fsl,imx8ulp-sim-lpav.yaml
>>>> @@ -0,0 +1,72 @@
>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>>> +%YAML 1.2
>>>> +---
>>>> +$id: http://devicetree.org/schemas/clock/fsl,imx8ulp-sim-lpav.yaml#
>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>> +
>>>> +title: NXP i.MX8ULP LPAV System Integration Module (SIM)
>>>> +
>>>> +maintainers:
>>>> +  - Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
>>>> +
>>>> +description:
>>>> +  The i.MX8ULP LPAV subsystem contains a block control module known as
>>>> +  SIM LPAV, which offers functionalities such as clock gating or reset
>>>> +  line assertion/de-assertion.
>>>> +
>>>> +properties:
>>>> +  compatible:
>>>> +    const: fsl,imx8ulp-sim-lpav
>>>> +
>>>> +  reg:
>>>> +    maxItems: 1
>>>> +
>>>> +  clocks:
>>>> +    maxItems: 3
>>>> +
>>>> +  clock-names:
>>>> +    items:
>>>> +      - const: lpav_bus
>>>> +      - const: hifi_core
>>>> +      - const: hifi_plat
>>> dt prefer use -
>>
>> I don't think we ever expressed such preference. Where did you find it?
> It should come from review message when submit binding-doc patch.  but I
> can't find it now. But at least, compatible string and node-name use "-".
>
> It'd better to add to writing-bindings.rst. It is hard to search whole
> linux-devicetree mail list or brain may cheat me. It is good to keep
> everything consistent.
>
> like:
>      " vs '
>      VCC-supply vs vcc-supply
>      ...
>
>>
>>> lpav-bus, ...
>> Then just "bus" or "apb".
> core, plat is also better than hifi_core, hifi_plat


sure, let's go with "bus", "core", "plat" then

Re: [PATCH v2 2/8] dt-bindings: clock: document 8ULP's SIM LPAV

[Krzysztof Kozlowski]

On 17/10/2025 13:20, Laurentiu Mihalcea wrote:
> From: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
> 
> Add documentation for i.MX8ULP's SIM LPAV module.
> 
> Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
> ---

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH v2 2/8] dt-bindings: clock: document 8ULP's SIM LPAV

[Peng Fan]

Hi Laurentiu,

On Fri, Oct 17, 2025 at 04:20:19AM -0700, Laurentiu Mihalcea wrote:
>From: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
>
>Add documentation for i.MX8ULP's SIM LPAV module.
>
>Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
>---
>+description:
>+  The i.MX8ULP LPAV subsystem contains a block control module known as
>+  SIM LPAV, which offers functionalities such as clock gating or reset
>+  line assertion/de-assertion.
>+
>+properties:
>+  compatible:
>+    const: fsl,imx8ulp-sim-lpav

This block also contains QoS registers, General purpose registers, HIFI
general purpose registers, and others.

I am not sure whether need to add a syscon fallback here. dt maintainer may
help comment.

Regards,
Peng

Re: [PATCH v2 2/8] dt-bindings: clock: document 8ULP's SIM LPAV

[Frank Li]

On Wed, Oct 22, 2025 at 10:08:41PM +0800, Peng Fan wrote:
> Hi Laurentiu,
>
> On Fri, Oct 17, 2025 at 04:20:19AM -0700, Laurentiu Mihalcea wrote:
> >From: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
> >
> >Add documentation for i.MX8ULP's SIM LPAV module.
> >
> >Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
> >---
> >+description:
> >+  The i.MX8ULP LPAV subsystem contains a block control module known as
> >+  SIM LPAV, which offers functionalities such as clock gating or reset
> >+  line assertion/de-assertion.
> >+
> >+properties:
> >+  compatible:
> >+    const: fsl,imx8ulp-sim-lpav
>
> This block also contains QoS registers, General purpose registers, HIFI

Qos should go though interconnect interface.

'#interconnect-cells' = <n>;

Frank

> general purpose registers, and others.
>
> I am not sure whether need to add a syscon fallback here. dt maintainer may
> help comment.
>
> Regards,
> Peng

Re: [PATCH v2 2/8] dt-bindings: clock: document 8ULP's SIM LPAV

[Laurentiu Mihalcea]

On 10/22/2025 7:08 AM, Peng Fan wrote:
> Hi Laurentiu,
>
> On Fri, Oct 17, 2025 at 04:20:19AM -0700, Laurentiu Mihalcea wrote:
>> From: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
>>
>> Add documentation for i.MX8ULP's SIM LPAV module.
>>
>> Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
>> ---
>> +description:
>> +  The i.MX8ULP LPAV subsystem contains a block control module known as
>> +  SIM LPAV, which offers functionalities such as clock gating or reset
>> +  line assertion/de-assertion.
>> +
>> +properties:
>> +  compatible:
>> +    const: fsl,imx8ulp-sim-lpav
> This block also contains QoS registers, General purpose registers, HIFI
> general purpose registers, and others.
>
> I am not sure whether need to add a syscon fallback here. dt maintainer may
> help comment.


syscon programming model is NOT compatible with this programming model.


If you need access to other registers (not covered by reset/MUX/clock APIs), you're going to have to either go

through a subsystem API or manually create a device link between SIM LPAV and your consumer and then use

something like dev_get_regmap().


Either way, you need to make sure that you're using the same lock for register access.


as for the interconnect QoS-related stuff: can't really comment on this as I haven't worked

with it, nor do I have an use case for it ATM. However, the binding does need to be complete so

suggestions in this regard would be much appreciated.


>
> Regards,
> Peng

Re: [PATCH v2 2/8] dt-bindings: clock: document 8ULP's SIM LPAV

[Daniel Baluta]

On Fri, Oct 17, 2025 at 2:22 PM Laurentiu Mihalcea
<laurentiumihalcea111@gmail.com> wrote:
>
> From: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
>
> Add documentation for i.MX8ULP's SIM LPAV module.
>
> Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>

Reviewed-by: Daniel Baluta <daniel.baluta@nxp.com>

[PATCH v2 3/8] clk: imx: add driver for imx8ulp's sim lpav

[Laurentiu Mihalcea]

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(&regmap_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, &regmap_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

Re: [PATCH v2 3/8] clk: imx: add driver for imx8ulp's sim lpav

[Frank Li]

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(&regmap_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, &regmap_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
>

Re: [PATCH v2 3/8] clk: imx: add driver for imx8ulp's sim lpav

[Laurentiu Mihalcea]

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 "&regmap->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 "&regmap->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(&regmap_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, &regmap_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
>>

Re: [PATCH v2 3/8] clk: imx: add driver for imx8ulp's sim lpav

[kernel test robot]

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

Re: [PATCH v2 3/8] clk: imx: add driver for imx8ulp's sim lpav

[kernel test robot]

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

Re: [PATCH v2 3/8] clk: imx: add driver for imx8ulp's sim lpav

[kernel test robot]

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

Re: [PATCH v2 3/8] clk: imx: add driver for imx8ulp's sim lpav

[Peng Fan]

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?

>+};
>+
>+#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

RE: [PATCH v2 3/8] clk: imx: add driver for imx8ulp's sim lpav

[Peng Fan]

> Subject: Re: [PATCH v2 3/8] clk: imx: add driver for imx8ulp's sim lpav
> 
> 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.

Ignore this comment. It should be in the structure.

Thanks,
Peng.

> 
> >+	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

Re: [PATCH v2 3/8] clk: imx: add driver for imx8ulp's sim lpav

[Laurentiu Mihalcea]

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

[PATCH v2 4/8] reset: imx8mp-audiomix: Drop unneeded macros

[Laurentiu Mihalcea]

From: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>

The macros defining the mask values for the EARC, EARC PHY resets,
and the DSP RUN_STALL signal can be dropped as they are not and will
not be used anywhere else except to set the value of the "mask" field
from "struct imx8mp_reset_map". In this particular case, based on the
name of the "mask" field, you can already deduce what these values are
for, which is why defining macros for them doesn't offer any new
information, nor does it help with the code readability.

Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
---
 drivers/reset/reset-imx8mp-audiomix.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/reset/reset-imx8mp-audiomix.c b/drivers/reset/reset-imx8mp-audiomix.c
index eceb37ff5dc5..e9643365a62c 100644
--- a/drivers/reset/reset-imx8mp-audiomix.c
+++ b/drivers/reset/reset-imx8mp-audiomix.c
@@ -14,11 +14,7 @@
 #include <linux/reset-controller.h>
 
 #define IMX8MP_AUDIOMIX_EARC_RESET_OFFSET	0x200
-#define IMX8MP_AUDIOMIX_EARC_RESET_MASK		BIT(0)
-#define IMX8MP_AUDIOMIX_EARC_PHY_RESET_MASK	BIT(1)
-
 #define IMX8MP_AUDIOMIX_DSP_RUNSTALL_OFFSET	0x108
-#define IMX8MP_AUDIOMIX_DSP_RUNSTALL_MASK	BIT(5)
 
 struct imx8mp_reset_map {
 	unsigned int offset;
@@ -29,17 +25,17 @@ struct imx8mp_reset_map {
 static const struct imx8mp_reset_map reset_map[] = {
 	[IMX8MP_AUDIOMIX_EARC_RESET] = {
 		.offset	= IMX8MP_AUDIOMIX_EARC_RESET_OFFSET,
-		.mask	= IMX8MP_AUDIOMIX_EARC_RESET_MASK,
+		.mask = BIT(0),
 		.active_low = true,
 	},
 	[IMX8MP_AUDIOMIX_EARC_PHY_RESET] = {
 		.offset	= IMX8MP_AUDIOMIX_EARC_RESET_OFFSET,
-		.mask	= IMX8MP_AUDIOMIX_EARC_PHY_RESET_MASK,
+		.mask = BIT(1),
 		.active_low = true,
 	},
 	[IMX8MP_AUDIOMIX_DSP_RUNSTALL] = {
 		.offset	= IMX8MP_AUDIOMIX_DSP_RUNSTALL_OFFSET,
-		.mask	= IMX8MP_AUDIOMIX_DSP_RUNSTALL_MASK,
+		.mask = BIT(5),
 		.active_low = false,
 	},
 };
-- 
2.43.0

Re: [PATCH v2 4/8] reset: imx8mp-audiomix: Drop unneeded macros

[Frank Li]

On Fri, Oct 17, 2025 at 04:20:21AM -0700, Laurentiu Mihalcea wrote:
> From: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
>
> The macros defining the mask values for the EARC, EARC PHY resets,
> and the DSP RUN_STALL signal can be dropped as they are not and will
> not be used anywhere else except to set the value of the "mask" field
> from "struct imx8mp_reset_map". In this particular case, based on the
> name of the "mask" field, you can already deduce what these values are
> for, which is why defining macros for them doesn't offer any new
> information, nor does it help with the code readability.
>
> Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
> ---

Register define still prefer use macro.

So far const string, hexvalue prefer use value if only use once.

Frank

>  drivers/reset/reset-imx8mp-audiomix.c | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/reset/reset-imx8mp-audiomix.c b/drivers/reset/reset-imx8mp-audiomix.c
> index eceb37ff5dc5..e9643365a62c 100644
> --- a/drivers/reset/reset-imx8mp-audiomix.c
> +++ b/drivers/reset/reset-imx8mp-audiomix.c
> @@ -14,11 +14,7 @@
>  #include <linux/reset-controller.h>
>
>  #define IMX8MP_AUDIOMIX_EARC_RESET_OFFSET	0x200
> -#define IMX8MP_AUDIOMIX_EARC_RESET_MASK		BIT(0)
> -#define IMX8MP_AUDIOMIX_EARC_PHY_RESET_MASK	BIT(1)
> -
>  #define IMX8MP_AUDIOMIX_DSP_RUNSTALL_OFFSET	0x108
> -#define IMX8MP_AUDIOMIX_DSP_RUNSTALL_MASK	BIT(5)
>
>  struct imx8mp_reset_map {
>  	unsigned int offset;
> @@ -29,17 +25,17 @@ struct imx8mp_reset_map {
>  static const struct imx8mp_reset_map reset_map[] = {
>  	[IMX8MP_AUDIOMIX_EARC_RESET] = {
>  		.offset	= IMX8MP_AUDIOMIX_EARC_RESET_OFFSET,
> -		.mask	= IMX8MP_AUDIOMIX_EARC_RESET_MASK,
> +		.mask = BIT(0),
>  		.active_low = true,
>  	},
>  	[IMX8MP_AUDIOMIX_EARC_PHY_RESET] = {
>  		.offset	= IMX8MP_AUDIOMIX_EARC_RESET_OFFSET,
> -		.mask	= IMX8MP_AUDIOMIX_EARC_PHY_RESET_MASK,
> +		.mask = BIT(1),
>  		.active_low = true,
>  	},
>  	[IMX8MP_AUDIOMIX_DSP_RUNSTALL] = {
>  		.offset	= IMX8MP_AUDIOMIX_DSP_RUNSTALL_OFFSET,
> -		.mask	= IMX8MP_AUDIOMIX_DSP_RUNSTALL_MASK,
> +		.mask = BIT(5),
>  		.active_low = false,
>  	},
>  };
> --
> 2.43.0
>

Re: [PATCH v2 4/8] reset: imx8mp-audiomix: Drop unneeded macros

[Daniel Baluta]

On Fri, Oct 17, 2025 at 5:47 PM Frank Li <Frank.li@nxp.com> wrote:
>
> On Fri, Oct 17, 2025 at 04:20:21AM -0700, Laurentiu Mihalcea wrote:
> > From: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
> >
> > The macros defining the mask values for the EARC, EARC PHY resets,
> > and the DSP RUN_STALL signal can be dropped as they are not and will
> > not be used anywhere else except to set the value of the "mask" field
> > from "struct imx8mp_reset_map". In this particular case, based on the
> > name of the "mask" field, you can already deduce what these values are
> > for, which is why defining macros for them doesn't offer any new
> > information, nor does it help with the code readability.
> >
> > Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
> > ---
>
> Register define still prefer use macro.
>
> So far const string, hexvalue prefer use value if only use once.

This is simple enough that we can use the BIT() macro directly to express
the masks.

As you said you can use the const value (including BIT()) when the value is used
only once as it is this case.

So I think this patch is fine:

Reviewed-by: Daniel Baluta <daniel.baluta@nxp.com>

[PATCH v2 5/8] reset: imx8mp-audiomix: Switch to using regmap API

[Laurentiu Mihalcea]

From: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>

As far as the Linux kernel is concerned, block devices such as i.MX8MP's
AUDIOMIX block control or i.MX8ULP's SIM LPAV can simultaneously act as
clock controllers, reset controllers or mux controllers. Since these IPs
offer different functionalities through different subsystem APIs, it's
important to make sure that the register R-M-W cycles are performed under
the same lock across all subsystem APIs. This will ensure that registers
will not end up with the wrong values because of race conditions (e.g.
clock consumer tries to update block control register A, while, at the
same time, reset consumer tries to update the same block control register).

However, the aforementioned race conditions will only impact block control
IPs which use the same register for multiple functionalities. For example,
i.MX8MP's AUDIOMIX block control IP provides clock gating functionalities
and reset control functionalities through different registers. This is why
the current approach (i.e. clock control and reset control work using
different locks) has worked well so far.

Since we want to extend this driver to be usable for i.MX8ULP's SIM LPAV
block control IP, we need to make sure that clock control, reset control,
and mux control APIs use the same lock since all of these functionalities
are performed using the SYSCTRL0 register.

To do so, we need to switch to the regmap API and, if possible, use the
parent device's regmap, which, in the case of i.MX8ULP, will be the clock
controller. This way, we can make sure that the clock gates and the reset
controller will use the same lock to perform the register R-M-W cycles.

This change will also work fine for cases where we don't really need to
share the lock across multiple APIs (e.g. i.MX8MP's AUDIOMIX block
control) since regmap will take care of the locking we were previously
explicitly performing in the driver.

The transition to the regmap API also involves some cleanup. Specifically,
we can make use of devres to unmap the device's memory and get rid of the
memory mapping-related error paths and the remove() function altogether.

Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
---
 drivers/reset/reset-imx8mp-audiomix.c | 95 +++++++++++++++++----------
 1 file changed, 61 insertions(+), 34 deletions(-)

diff --git a/drivers/reset/reset-imx8mp-audiomix.c b/drivers/reset/reset-imx8mp-audiomix.c
index e9643365a62c..c74ce6e04177 100644
--- a/drivers/reset/reset-imx8mp-audiomix.c
+++ b/drivers/reset/reset-imx8mp-audiomix.c
@@ -11,6 +11,7 @@
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
+#include <linux/regmap.h>
 #include <linux/reset-controller.h>
 
 #define IMX8MP_AUDIOMIX_EARC_RESET_OFFSET	0x200
@@ -19,6 +20,7 @@
 struct imx8mp_reset_map {
 	unsigned int offset;
 	unsigned int mask;
+	unsigned int shift;
 	bool active_low;
 };
 
@@ -26,24 +28,27 @@ static const struct imx8mp_reset_map reset_map[] = {
 	[IMX8MP_AUDIOMIX_EARC_RESET] = {
 		.offset	= IMX8MP_AUDIOMIX_EARC_RESET_OFFSET,
 		.mask = BIT(0),
+		.shift = 0,
 		.active_low = true,
 	},
 	[IMX8MP_AUDIOMIX_EARC_PHY_RESET] = {
 		.offset	= IMX8MP_AUDIOMIX_EARC_RESET_OFFSET,
 		.mask = BIT(1),
+		.shift = 1,
 		.active_low = true,
 	},
 	[IMX8MP_AUDIOMIX_DSP_RUNSTALL] = {
 		.offset	= IMX8MP_AUDIOMIX_DSP_RUNSTALL_OFFSET,
 		.mask = BIT(5),
+		.shift = 5,
 		.active_low = false,
 	},
 };
 
 struct imx8mp_audiomix_reset {
 	struct reset_controller_dev rcdev;
-	spinlock_t lock; /* protect register read-modify-write cycle */
 	void __iomem *base;
+	struct regmap *regmap;
 };
 
 static struct imx8mp_audiomix_reset *to_imx8mp_audiomix_reset(struct reset_controller_dev *rcdev)
@@ -55,26 +60,15 @@ static int imx8mp_audiomix_update(struct reset_controller_dev *rcdev,
 				  unsigned long id, bool assert)
 {
 	struct imx8mp_audiomix_reset *priv = to_imx8mp_audiomix_reset(rcdev);
-	void __iomem *reg_addr = priv->base;
-	unsigned int mask, offset, active_low;
-	unsigned long reg, flags;
+	unsigned int mask, offset, active_low, shift, val;
 
 	mask = reset_map[id].mask;
 	offset = reset_map[id].offset;
 	active_low = reset_map[id].active_low;
+	shift = reset_map[id].shift;
+	val = (active_low ^ assert) << shift;
 
-	spin_lock_irqsave(&priv->lock, flags);
-
-	reg = readl(reg_addr + offset);
-	if (active_low ^ assert)
-		reg |= mask;
-	else
-		reg &= ~mask;
-	writel(reg, reg_addr + offset);
-
-	spin_unlock_irqrestore(&priv->lock, flags);
-
-	return 0;
+	return regmap_update_bits(priv->regmap, offset, mask, val);
 }
 
 static int imx8mp_audiomix_reset_assert(struct reset_controller_dev *rcdev,
@@ -94,6 +88,50 @@ static const struct reset_control_ops imx8mp_audiomix_reset_ops = {
 	.deassert = imx8mp_audiomix_reset_deassert,
 };
 
+static const struct regmap_config regmap_config = {
+	.reg_bits = 32,
+	.val_bits = 32,
+	.reg_stride = 4,
+};
+
+/* assumption: registered only if not using parent regmap */
+static void imx8mp_audiomix_reset_iounmap(void *data)
+{
+	struct imx8mp_audiomix_reset *priv = dev_get_drvdata(data);
+
+	iounmap(priv->base);
+}
+
+/* assumption: dev_set_drvdata() is called before this */
+static int imx8mp_audiomix_reset_get_regmap(struct device *dev)
+{
+	struct imx8mp_audiomix_reset *priv;
+	int ret;
+
+	priv = dev_get_drvdata(dev);
+
+	/* try to use the parent's regmap */
+	priv->regmap = dev_get_regmap(dev->parent, NULL);
+	if (priv->regmap)
+		return 0;
+
+	/* ... if that's not possible then initialize the regmap right now */
+	priv->base = of_iomap(dev->parent->of_node, 0);
+	if (!priv->base)
+		return dev_err_probe(dev, -ENOMEM, "failed to iomap address space\n");
+
+	ret = devm_add_action_or_reset(dev, imx8mp_audiomix_reset_iounmap, dev);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to register action\n");
+
+	priv->regmap = devm_regmap_init_mmio(dev, priv->base, &regmap_config);
+	if (IS_ERR(priv->regmap))
+		return dev_err_probe(dev, PTR_ERR(priv->regmap),
+				     "failed to initialize regmap\n");
+
+	return 0;
+}
+
 static int imx8mp_audiomix_reset_probe(struct auxiliary_device *adev,
 				       const struct auxiliary_device_id *id)
 {
@@ -105,36 +143,26 @@ static int imx8mp_audiomix_reset_probe(struct auxiliary_device *adev,
 	if (!priv)
 		return -ENOMEM;
 
-	spin_lock_init(&priv->lock);
-
 	priv->rcdev.owner     = THIS_MODULE;
 	priv->rcdev.nr_resets = ARRAY_SIZE(reset_map);
 	priv->rcdev.ops       = &imx8mp_audiomix_reset_ops;
 	priv->rcdev.of_node   = dev->parent->of_node;
 	priv->rcdev.dev	      = dev;
 	priv->rcdev.of_reset_n_cells = 1;
-	priv->base            = of_iomap(dev->parent->of_node, 0);
-	if (!priv->base)
-		return -ENOMEM;
 
+	/* keep before call to imx8mp_audiomix_reset_init_regmap() */
 	dev_set_drvdata(dev, priv);
 
+	ret = imx8mp_audiomix_reset_get_regmap(dev);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to get regmap\n");
+
 	ret = devm_reset_controller_register(dev, &priv->rcdev);
 	if (ret)
-		goto out_unmap;
+		return dev_err_probe(dev, ret,
+				     "failed to register reset controller\n");
 
 	return 0;
-
-out_unmap:
-	iounmap(priv->base);
-	return ret;
-}
-
-static void imx8mp_audiomix_reset_remove(struct auxiliary_device *adev)
-{
-	struct imx8mp_audiomix_reset *priv = dev_get_drvdata(&adev->dev);
-
-	iounmap(priv->base);
 }
 
 static const struct auxiliary_device_id imx8mp_audiomix_reset_ids[] = {
@@ -147,7 +175,6 @@ MODULE_DEVICE_TABLE(auxiliary, imx8mp_audiomix_reset_ids);
 
 static struct auxiliary_driver imx8mp_audiomix_reset_driver = {
 	.probe		= imx8mp_audiomix_reset_probe,
-	.remove		= imx8mp_audiomix_reset_remove,
 	.id_table	= imx8mp_audiomix_reset_ids,
 };
 
-- 
2.43.0

Re: [PATCH v2 5/8] reset: imx8mp-audiomix: Switch to using regmap API

[Frank Li]

On Fri, Oct 17, 2025 at 04:20:22AM -0700, Laurentiu Mihalcea wrote:
> From: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
>
> As far as the Linux kernel is concerned, block devices such as i.MX8MP's
> AUDIOMIX block control or i.MX8ULP's SIM LPAV can simultaneously act as
> clock controllers, reset controllers or mux controllers. Since these IPs
> offer different functionalities through different subsystem APIs, it's
> important to make sure that the register R-M-W cycles are performed under
> the same lock across all subsystem APIs. This will ensure that registers
> will not end up with the wrong values because of race conditions (e.g.
> clock consumer tries to update block control register A, while, at the
> same time, reset consumer tries to update the same block control register).
>
> However, the aforementioned race conditions will only impact block control
> IPs which use the same register for multiple functionalities. For example,
> i.MX8MP's AUDIOMIX block control IP provides clock gating functionalities
> and reset control functionalities through different registers. This is why
> the current approach (i.e. clock control and reset control work using
> different locks) has worked well so far.
>
> Since we want to extend this driver to be usable for i.MX8ULP's SIM LPAV
> block control IP, we need to make sure that clock control, reset control,
> and mux control APIs use the same lock since all of these functionalities
> are performed using the SYSCTRL0 register.
>
> To do so, we need to switch to the regmap API and, if possible, use the
> parent device's regmap, which, in the case of i.MX8ULP, will be the clock
> controller. This way, we can make sure that the clock gates and the reset
> controller will use the same lock to perform the register R-M-W cycles.
>
> This change will also work fine for cases where we don't really need to
> share the lock across multiple APIs (e.g. i.MX8MP's AUDIOMIX block
> control) since regmap will take care of the locking we were previously
> explicitly performing in the driver.
>
> The transition to the regmap API also involves some cleanup. Specifically,
> we can make use of devres to unmap the device's memory and get rid of the
> memory mapping-related error paths and the remove() function altogether.
>
> Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
> ---
>  drivers/reset/reset-imx8mp-audiomix.c | 95 +++++++++++++++++----------
>  1 file changed, 61 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/reset/reset-imx8mp-audiomix.c b/drivers/reset/reset-imx8mp-audiomix.c
> index e9643365a62c..c74ce6e04177 100644
> --- a/drivers/reset/reset-imx8mp-audiomix.c
> +++ b/drivers/reset/reset-imx8mp-audiomix.c
> @@ -11,6 +11,7 @@
>  #include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/of_address.h>
> +#include <linux/regmap.h>
>  #include <linux/reset-controller.h>
>
>  #define IMX8MP_AUDIOMIX_EARC_RESET_OFFSET	0x200
> @@ -19,6 +20,7 @@
>  struct imx8mp_reset_map {
>  	unsigned int offset;
>  	unsigned int mask;
> +	unsigned int shift;
>  	bool active_low;
>  };
>
> @@ -26,24 +28,27 @@ static const struct imx8mp_reset_map reset_map[] = {
>  	[IMX8MP_AUDIOMIX_EARC_RESET] = {
>  		.offset	= IMX8MP_AUDIOMIX_EARC_RESET_OFFSET,
>  		.mask = BIT(0),
> +		.shift = 0,
>  		.active_low = true,
>  	},
>  	[IMX8MP_AUDIOMIX_EARC_PHY_RESET] = {
>  		.offset	= IMX8MP_AUDIOMIX_EARC_RESET_OFFSET,
>  		.mask = BIT(1),
> +		.shift = 1,
>  		.active_low = true,
>  	},
>  	[IMX8MP_AUDIOMIX_DSP_RUNSTALL] = {
>  		.offset	= IMX8MP_AUDIOMIX_DSP_RUNSTALL_OFFSET,
>  		.mask = BIT(5),
> +		.shift = 5,

why need shift?  you can use ffs(mask) to get shift.

>  		.active_low = false,
>  	},
>  };
>
>  struct imx8mp_audiomix_reset {
>  	struct reset_controller_dev rcdev;
> -	spinlock_t lock; /* protect register read-modify-write cycle */
>  	void __iomem *base;
> +	struct regmap *regmap;
>  };
>
>  static struct imx8mp_audiomix_reset *to_imx8mp_audiomix_reset(struct reset_controller_dev *rcdev)
> @@ -55,26 +60,15 @@ static int imx8mp_audiomix_update(struct reset_controller_dev *rcdev,
>  				  unsigned long id, bool assert)
>  {
>  	struct imx8mp_audiomix_reset *priv = to_imx8mp_audiomix_reset(rcdev);
> -	void __iomem *reg_addr = priv->base;
> -	unsigned int mask, offset, active_low;
> -	unsigned long reg, flags;
> +	unsigned int mask, offset, active_low, shift, val;
>
>  	mask = reset_map[id].mask;
>  	offset = reset_map[id].offset;
>  	active_low = reset_map[id].active_low;
> +	shift = reset_map[id].shift;
> +	val = (active_low ^ assert) << shift;
>
> -	spin_lock_irqsave(&priv->lock, flags);
> -
> -	reg = readl(reg_addr + offset);
> -	if (active_low ^ assert)
> -		reg |= mask;
> -	else
> -		reg &= ~mask;
> -	writel(reg, reg_addr + offset);
> -
> -	spin_unlock_irqrestore(&priv->lock, flags);
> -
> -	return 0;
> +	return regmap_update_bits(priv->regmap, offset, mask, val);
>  }
>
>  static int imx8mp_audiomix_reset_assert(struct reset_controller_dev *rcdev,
> @@ -94,6 +88,50 @@ static const struct reset_control_ops imx8mp_audiomix_reset_ops = {
>  	.deassert = imx8mp_audiomix_reset_deassert,
>  };
>
> +static const struct regmap_config regmap_config = {
> +	.reg_bits = 32,
> +	.val_bits = 32,
> +	.reg_stride = 4,
> +};
> +
> +/* assumption: registered only if not using parent regmap */
> +static void imx8mp_audiomix_reset_iounmap(void *data)
> +{
> +	struct imx8mp_audiomix_reset *priv = dev_get_drvdata(data);
> +
> +	iounmap(priv->base);
> +}
> +
> +/* assumption: dev_set_drvdata() is called before this */
> +static int imx8mp_audiomix_reset_get_regmap(struct device *dev)
> +{
> +	struct imx8mp_audiomix_reset *priv;
> +	int ret;
> +
> +	priv = dev_get_drvdata(dev);
> +
> +	/* try to use the parent's regmap */
> +	priv->regmap = dev_get_regmap(dev->parent, NULL);
> +	if (priv->regmap)
> +		return 0;
> +
> +	/* ... if that's not possible then initialize the regmap right now */
> +	priv->base = of_iomap(dev->parent->of_node, 0);
> +	if (!priv->base)
> +		return dev_err_probe(dev, -ENOMEM, "failed to iomap address space\n");
> +
> +	ret = devm_add_action_or_reset(dev, imx8mp_audiomix_reset_iounmap, dev);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to register action\n");
> +
> +	priv->regmap = devm_regmap_init_mmio(dev, priv->base, &regmap_config);
> +	if (IS_ERR(priv->regmap))
> +		return dev_err_probe(dev, PTR_ERR(priv->regmap),
> +				     "failed to initialize regmap\n");
> +
> +	return 0;
> +}
> +
>  static int imx8mp_audiomix_reset_probe(struct auxiliary_device *adev,
>  				       const struct auxiliary_device_id *id)
>  {
> @@ -105,36 +143,26 @@ static int imx8mp_audiomix_reset_probe(struct auxiliary_device *adev,
>  	if (!priv)
>  		return -ENOMEM;
>
> -	spin_lock_init(&priv->lock);
> -
>  	priv->rcdev.owner     = THIS_MODULE;
>  	priv->rcdev.nr_resets = ARRAY_SIZE(reset_map);
>  	priv->rcdev.ops       = &imx8mp_audiomix_reset_ops;
>  	priv->rcdev.of_node   = dev->parent->of_node;
>  	priv->rcdev.dev	      = dev;
>  	priv->rcdev.of_reset_n_cells = 1;
> -	priv->base            = of_iomap(dev->parent->of_node, 0);
> -	if (!priv->base)
> -		return -ENOMEM;
>
> +	/* keep before call to imx8mp_audiomix_reset_init_regmap() */
>  	dev_set_drvdata(dev, priv);
>
> +	ret = imx8mp_audiomix_reset_get_regmap(dev);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to get regmap\n");
> +
>  	ret = devm_reset_controller_register(dev, &priv->rcdev);
>  	if (ret)
> -		goto out_unmap;
> +		return dev_err_probe(dev, ret,
> +				     "failed to register reset controller\n");
>
>  	return 0;
> -
> -out_unmap:
> -	iounmap(priv->base);
> -	return ret;
> -}
> -
> -static void imx8mp_audiomix_reset_remove(struct auxiliary_device *adev)
> -{
> -	struct imx8mp_audiomix_reset *priv = dev_get_drvdata(&adev->dev);
> -
> -	iounmap(priv->base);
>  }
>
>  static const struct auxiliary_device_id imx8mp_audiomix_reset_ids[] = {
> @@ -147,7 +175,6 @@ MODULE_DEVICE_TABLE(auxiliary, imx8mp_audiomix_reset_ids);
>
>  static struct auxiliary_driver imx8mp_audiomix_reset_driver = {
>  	.probe		= imx8mp_audiomix_reset_probe,
> -	.remove		= imx8mp_audiomix_reset_remove,

cleanup imx8mp_audiomix_reset_remove need seperate patch.

Frank

>  	.id_table	= imx8mp_audiomix_reset_ids,
>  };
>
> --
> 2.43.0
>

Re: [PATCH v2 5/8] reset: imx8mp-audiomix: Switch to using regmap API

[Laurentiu Mihalcea]

On 10/17/2025 7:49 AM, Frank Li wrote:
> On Fri, Oct 17, 2025 at 04:20:22AM -0700, Laurentiu Mihalcea wrote:
>> From: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
>>
>> As far as the Linux kernel is concerned, block devices such as i.MX8MP's
>> AUDIOMIX block control or i.MX8ULP's SIM LPAV can simultaneously act as
>> clock controllers, reset controllers or mux controllers. Since these IPs
>> offer different functionalities through different subsystem APIs, it's
>> important to make sure that the register R-M-W cycles are performed under
>> the same lock across all subsystem APIs. This will ensure that registers
>> will not end up with the wrong values because of race conditions (e.g.
>> clock consumer tries to update block control register A, while, at the
>> same time, reset consumer tries to update the same block control register).
>>
>> However, the aforementioned race conditions will only impact block control
>> IPs which use the same register for multiple functionalities. For example,
>> i.MX8MP's AUDIOMIX block control IP provides clock gating functionalities
>> and reset control functionalities through different registers. This is why
>> the current approach (i.e. clock control and reset control work using
>> different locks) has worked well so far.
>>
>> Since we want to extend this driver to be usable for i.MX8ULP's SIM LPAV
>> block control IP, we need to make sure that clock control, reset control,
>> and mux control APIs use the same lock since all of these functionalities
>> are performed using the SYSCTRL0 register.
>>
>> To do so, we need to switch to the regmap API and, if possible, use the
>> parent device's regmap, which, in the case of i.MX8ULP, will be the clock
>> controller. This way, we can make sure that the clock gates and the reset
>> controller will use the same lock to perform the register R-M-W cycles.
>>
>> This change will also work fine for cases where we don't really need to
>> share the lock across multiple APIs (e.g. i.MX8MP's AUDIOMIX block
>> control) since regmap will take care of the locking we were previously
>> explicitly performing in the driver.
>>
>> The transition to the regmap API also involves some cleanup. Specifically,
>> we can make use of devres to unmap the device's memory and get rid of the
>> memory mapping-related error paths and the remove() function altogether.
>>
>> Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
>> ---
>>  drivers/reset/reset-imx8mp-audiomix.c | 95 +++++++++++++++++----------
>>  1 file changed, 61 insertions(+), 34 deletions(-)
>>
>> diff --git a/drivers/reset/reset-imx8mp-audiomix.c b/drivers/reset/reset-imx8mp-audiomix.c
>> index e9643365a62c..c74ce6e04177 100644
>> --- a/drivers/reset/reset-imx8mp-audiomix.c
>> +++ b/drivers/reset/reset-imx8mp-audiomix.c
>> @@ -11,6 +11,7 @@
>>  #include <linux/module.h>
>>  #include <linux/of.h>
>>  #include <linux/of_address.h>
>> +#include <linux/regmap.h>
>>  #include <linux/reset-controller.h>
>>
>>  #define IMX8MP_AUDIOMIX_EARC_RESET_OFFSET	0x200
>> @@ -19,6 +20,7 @@
>>  struct imx8mp_reset_map {
>>  	unsigned int offset;
>>  	unsigned int mask;
>> +	unsigned int shift;
>>  	bool active_low;
>>  };
>>
>> @@ -26,24 +28,27 @@ static const struct imx8mp_reset_map reset_map[] = {
>>  	[IMX8MP_AUDIOMIX_EARC_RESET] = {
>>  		.offset	= IMX8MP_AUDIOMIX_EARC_RESET_OFFSET,
>>  		.mask = BIT(0),
>> +		.shift = 0,
>>  		.active_low = true,
>>  	},
>>  	[IMX8MP_AUDIOMIX_EARC_PHY_RESET] = {
>>  		.offset	= IMX8MP_AUDIOMIX_EARC_RESET_OFFSET,
>>  		.mask = BIT(1),
>> +		.shift = 1,
>>  		.active_low = true,
>>  	},
>>  	[IMX8MP_AUDIOMIX_DSP_RUNSTALL] = {
>>  		.offset	= IMX8MP_AUDIOMIX_DSP_RUNSTALL_OFFSET,
>>  		.mask = BIT(5),
>> +		.shift = 5,
> why need shift?  you can use ffs(mask) to get shift.
>
>>  		.active_low = false,
>>  	},
>>  };
>>
>>  struct imx8mp_audiomix_reset {
>>  	struct reset_controller_dev rcdev;
>> -	spinlock_t lock; /* protect register read-modify-write cycle */
>>  	void __iomem *base;
>> +	struct regmap *regmap;
>>  };
>>
>>  static struct imx8mp_audiomix_reset *to_imx8mp_audiomix_reset(struct reset_controller_dev *rcdev)
>> @@ -55,26 +60,15 @@ static int imx8mp_audiomix_update(struct reset_controller_dev *rcdev,
>>  				  unsigned long id, bool assert)
>>  {
>>  	struct imx8mp_audiomix_reset *priv = to_imx8mp_audiomix_reset(rcdev);
>> -	void __iomem *reg_addr = priv->base;
>> -	unsigned int mask, offset, active_low;
>> -	unsigned long reg, flags;
>> +	unsigned int mask, offset, active_low, shift, val;
>>
>>  	mask = reset_map[id].mask;
>>  	offset = reset_map[id].offset;
>>  	active_low = reset_map[id].active_low;
>> +	shift = reset_map[id].shift;
>> +	val = (active_low ^ assert) << shift;
>>
>> -	spin_lock_irqsave(&priv->lock, flags);
>> -
>> -	reg = readl(reg_addr + offset);
>> -	if (active_low ^ assert)
>> -		reg |= mask;
>> -	else
>> -		reg &= ~mask;
>> -	writel(reg, reg_addr + offset);
>> -
>> -	spin_unlock_irqrestore(&priv->lock, flags);
>> -
>> -	return 0;
>> +	return regmap_update_bits(priv->regmap, offset, mask, val);
>>  }
>>
>>  static int imx8mp_audiomix_reset_assert(struct reset_controller_dev *rcdev,
>> @@ -94,6 +88,50 @@ static const struct reset_control_ops imx8mp_audiomix_reset_ops = {
>>  	.deassert = imx8mp_audiomix_reset_deassert,
>>  };
>>
>> +static const struct regmap_config regmap_config = {
>> +	.reg_bits = 32,
>> +	.val_bits = 32,
>> +	.reg_stride = 4,
>> +};
>> +
>> +/* assumption: registered only if not using parent regmap */
>> +static void imx8mp_audiomix_reset_iounmap(void *data)
>> +{
>> +	struct imx8mp_audiomix_reset *priv = dev_get_drvdata(data);
>> +
>> +	iounmap(priv->base);
>> +}
>> +
>> +/* assumption: dev_set_drvdata() is called before this */
>> +static int imx8mp_audiomix_reset_get_regmap(struct device *dev)
>> +{
>> +	struct imx8mp_audiomix_reset *priv;
>> +	int ret;
>> +
>> +	priv = dev_get_drvdata(dev);
>> +
>> +	/* try to use the parent's regmap */
>> +	priv->regmap = dev_get_regmap(dev->parent, NULL);
>> +	if (priv->regmap)
>> +		return 0;
>> +
>> +	/* ... if that's not possible then initialize the regmap right now */
>> +	priv->base = of_iomap(dev->parent->of_node, 0);
>> +	if (!priv->base)
>> +		return dev_err_probe(dev, -ENOMEM, "failed to iomap address space\n");
>> +
>> +	ret = devm_add_action_or_reset(dev, imx8mp_audiomix_reset_iounmap, dev);
>> +	if (ret)
>> +		return dev_err_probe(dev, ret, "failed to register action\n");
>> +
>> +	priv->regmap = devm_regmap_init_mmio(dev, priv->base, &regmap_config);
>> +	if (IS_ERR(priv->regmap))
>> +		return dev_err_probe(dev, PTR_ERR(priv->regmap),
>> +				     "failed to initialize regmap\n");
>> +
>> +	return 0;
>> +}
>> +
>>  static int imx8mp_audiomix_reset_probe(struct auxiliary_device *adev,
>>  				       const struct auxiliary_device_id *id)
>>  {
>> @@ -105,36 +143,26 @@ static int imx8mp_audiomix_reset_probe(struct auxiliary_device *adev,
>>  	if (!priv)
>>  		return -ENOMEM;
>>
>> -	spin_lock_init(&priv->lock);
>> -
>>  	priv->rcdev.owner     = THIS_MODULE;
>>  	priv->rcdev.nr_resets = ARRAY_SIZE(reset_map);
>>  	priv->rcdev.ops       = &imx8mp_audiomix_reset_ops;
>>  	priv->rcdev.of_node   = dev->parent->of_node;
>>  	priv->rcdev.dev	      = dev;
>>  	priv->rcdev.of_reset_n_cells = 1;
>> -	priv->base            = of_iomap(dev->parent->of_node, 0);
>> -	if (!priv->base)
>> -		return -ENOMEM;
>>
>> +	/* keep before call to imx8mp_audiomix_reset_init_regmap() */
>>  	dev_set_drvdata(dev, priv);
>>
>> +	ret = imx8mp_audiomix_reset_get_regmap(dev);
>> +	if (ret)
>> +		return dev_err_probe(dev, ret, "failed to get regmap\n");
>> +
>>  	ret = devm_reset_controller_register(dev, &priv->rcdev);
>>  	if (ret)
>> -		goto out_unmap;
>> +		return dev_err_probe(dev, ret,
>> +				     "failed to register reset controller\n");
>>
>>  	return 0;
>> -
>> -out_unmap:
>> -	iounmap(priv->base);
>> -	return ret;
>> -}
>> -
>> -static void imx8mp_audiomix_reset_remove(struct auxiliary_device *adev)
>> -{
>> -	struct imx8mp_audiomix_reset *priv = dev_get_drvdata(&adev->dev);
>> -
>> -	iounmap(priv->base);
>>  }
>>
>>  static const struct auxiliary_device_id imx8mp_audiomix_reset_ids[] = {
>> @@ -147,7 +175,6 @@ MODULE_DEVICE_TABLE(auxiliary, imx8mp_audiomix_reset_ids);
>>
>>  static struct auxiliary_driver imx8mp_audiomix_reset_driver = {
>>  	.probe		= imx8mp_audiomix_reset_probe,
>> -	.remove		= imx8mp_audiomix_reset_remove,
> cleanup imx8mp_audiomix_reset_remove need seperate patch.


as things stand now, this bit belongs in the same patch as the switch to the regmap

API because of the introduction of imx8mp_audiomix_reset_get_regmap(), which

uses devres. This makes the remove() function incorrect and, thus, needs to be dropped.


since the cleanup is quite trivial, I don't think this would justify purposely

making imx8mp_audiomix_reset_get_regmap() not use devres in this patch

and then transitioning to it in a subsequent patch.


>
> Frank
>
>>  	.id_table	= imx8mp_audiomix_reset_ids,
>>  };
>>
>> --
>> 2.43.0
>>

[PATCH v2 6/8] reset: imx8mp-audiomix: Extend the driver usage

[Laurentiu Mihalcea]

From: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>

Some NXP SoCs integrate one or more, per-subsystem, block control IPs,
which allow users to control the assertion and de-assertion of the
reset lines tied to some peripherals present in said subsystems. Some
examples of such SoCs include i.MX8MP with AUDIOMIX block control and
i.MX8ULP with SIM LPAV.

Some of the aformentioned block control IPs exhibit a common pattern with
respect to the assertion and de-assertion of the reset lines. Namely, the
user is able to control the state of the reset line by toggling a bit from
one of the IP's registers.

Linux can take advantage of this pattern and, instead of having one driver
for each block control IP, a single, more generic driver could be used.

To allow this to happen, the previous approach, in which a single reset
map is used, is replaced by a per-driver approach, in which each auxiliary
device driver holds a reference to a certain reset map.

Reviewed-by: Daniel Baluta <daniel.baluta@nxp.com>
Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
---
 drivers/reset/reset-imx8mp-audiomix.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/reset/reset-imx8mp-audiomix.c b/drivers/reset/reset-imx8mp-audiomix.c
index c74ce6e04177..c370913107f5 100644
--- a/drivers/reset/reset-imx8mp-audiomix.c
+++ b/drivers/reset/reset-imx8mp-audiomix.c
@@ -24,7 +24,12 @@ struct imx8mp_reset_map {
 	bool active_low;
 };
 
-static const struct imx8mp_reset_map reset_map[] = {
+struct imx8mp_reset_info {
+	const struct imx8mp_reset_map *map;
+	int num_lines;
+};
+
+static const struct imx8mp_reset_map imx8mp_reset_map[] = {
 	[IMX8MP_AUDIOMIX_EARC_RESET] = {
 		.offset	= IMX8MP_AUDIOMIX_EARC_RESET_OFFSET,
 		.mask = BIT(0),
@@ -45,10 +50,16 @@ static const struct imx8mp_reset_map reset_map[] = {
 	},
 };
 
+static const struct imx8mp_reset_info imx8mp_reset_info = {
+	.map = imx8mp_reset_map,
+	.num_lines = ARRAY_SIZE(imx8mp_reset_map),
+};
+
 struct imx8mp_audiomix_reset {
 	struct reset_controller_dev rcdev;
 	void __iomem *base;
 	struct regmap *regmap;
+	const struct imx8mp_reset_info *rinfo;
 };
 
 static struct imx8mp_audiomix_reset *to_imx8mp_audiomix_reset(struct reset_controller_dev *rcdev)
@@ -60,6 +71,7 @@ static int imx8mp_audiomix_update(struct reset_controller_dev *rcdev,
 				  unsigned long id, bool assert)
 {
 	struct imx8mp_audiomix_reset *priv = to_imx8mp_audiomix_reset(rcdev);
+	const struct imx8mp_reset_map *reset_map = priv->rinfo->map;
 	unsigned int mask, offset, active_low, shift, val;
 
 	mask = reset_map[id].mask;
@@ -144,7 +156,8 @@ static int imx8mp_audiomix_reset_probe(struct auxiliary_device *adev,
 		return -ENOMEM;
 
 	priv->rcdev.owner     = THIS_MODULE;
-	priv->rcdev.nr_resets = ARRAY_SIZE(reset_map);
+	priv->rinfo           = (const struct imx8mp_reset_info *)id->driver_data;
+	priv->rcdev.nr_resets = priv->rinfo->num_lines;
 	priv->rcdev.ops       = &imx8mp_audiomix_reset_ops;
 	priv->rcdev.of_node   = dev->parent->of_node;
 	priv->rcdev.dev	      = dev;
@@ -168,6 +181,7 @@ static int imx8mp_audiomix_reset_probe(struct auxiliary_device *adev,
 static const struct auxiliary_device_id imx8mp_audiomix_reset_ids[] = {
 	{
 		.name = "clk_imx8mp_audiomix.reset",
+		.driver_data = (kernel_ulong_t)&imx8mp_reset_info,
 	},
 	{ }
 };
-- 
2.43.0

Re: [PATCH v2 6/8] reset: imx8mp-audiomix: Extend the driver usage

[Frank Li]

On Fri, Oct 17, 2025 at 04:20:23AM -0700, Laurentiu Mihalcea wrote:
> From: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
>
> Some NXP SoCs integrate one or more, per-subsystem, block control IPs,
> which allow users to control the assertion and de-assertion of the
> reset lines tied to some peripherals present in said subsystems. Some
> examples of such SoCs include i.MX8MP with AUDIOMIX block control and
> i.MX8ULP with SIM LPAV.
>
> Some of the aformentioned block control IPs exhibit a common pattern with
> respect to the assertion and de-assertion of the reset lines. Namely, the
> user is able to control the state of the reset line by toggling a bit from
> one of the IP's registers.
>
> Linux can take advantage of this pattern and, instead of having one driver
> for each block control IP, a single, more generic driver could be used.
>
> To allow this to happen, the previous approach, in which a single reset
> map is used, is replaced by a per-driver approach, in which each auxiliary
> device driver holds a reference to a certain reset map.

Can you shorter your commit message?, basically, you just add
imx8mp_reset_info for difference auxiliary_device_id.

>
> Reviewed-by: Daniel Baluta <daniel.baluta@nxp.com>
> Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
> ---
>  drivers/reset/reset-imx8mp-audiomix.c | 18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/reset/reset-imx8mp-audiomix.c b/drivers/reset/reset-imx8mp-audiomix.c
> index c74ce6e04177..c370913107f5 100644
> --- a/drivers/reset/reset-imx8mp-audiomix.c
> +++ b/drivers/reset/reset-imx8mp-audiomix.c
> @@ -24,7 +24,12 @@ struct imx8mp_reset_map {
>  	bool active_low;
>  };
>
> -static const struct imx8mp_reset_map reset_map[] = {
> +struct imx8mp_reset_info {
> +	const struct imx8mp_reset_map *map;
> +	int num_lines;
> +};
> +
> +static const struct imx8mp_reset_map imx8mp_reset_map[] = {
>  	[IMX8MP_AUDIOMIX_EARC_RESET] = {
>  		.offset	= IMX8MP_AUDIOMIX_EARC_RESET_OFFSET,
>  		.mask = BIT(0),
> @@ -45,10 +50,16 @@ static const struct imx8mp_reset_map reset_map[] = {
>  	},
>  };
>
> +static const struct imx8mp_reset_info imx8mp_reset_info = {
> +	.map = imx8mp_reset_map,
> +	.num_lines = ARRAY_SIZE(imx8mp_reset_map),
> +};
> +
>  struct imx8mp_audiomix_reset {
>  	struct reset_controller_dev rcdev;
>  	void __iomem *base;
>  	struct regmap *regmap;
> +	const struct imx8mp_reset_info *rinfo;
>  };
>
>  static struct imx8mp_audiomix_reset *to_imx8mp_audiomix_reset(struct reset_controller_dev *rcdev)
> @@ -60,6 +71,7 @@ static int imx8mp_audiomix_update(struct reset_controller_dev *rcdev,
>  				  unsigned long id, bool assert)
>  {
>  	struct imx8mp_audiomix_reset *priv = to_imx8mp_audiomix_reset(rcdev);
> +	const struct imx8mp_reset_map *reset_map = priv->rinfo->map;
>  	unsigned int mask, offset, active_low, shift, val;
>
>  	mask = reset_map[id].mask;
> @@ -144,7 +156,8 @@ static int imx8mp_audiomix_reset_probe(struct auxiliary_device *adev,
>  		return -ENOMEM;
>
>  	priv->rcdev.owner     = THIS_MODULE;
> -	priv->rcdev.nr_resets = ARRAY_SIZE(reset_map);
> +	priv->rinfo           = (const struct imx8mp_reset_info *)id->driver_data;

needn't force convert from void*

Frank

> +	priv->rcdev.nr_resets = priv->rinfo->num_lines;
>  	priv->rcdev.ops       = &imx8mp_audiomix_reset_ops;
>  	priv->rcdev.of_node   = dev->parent->of_node;
>  	priv->rcdev.dev	      = dev;
> @@ -168,6 +181,7 @@ static int imx8mp_audiomix_reset_probe(struct auxiliary_device *adev,
>  static const struct auxiliary_device_id imx8mp_audiomix_reset_ids[] = {
>  	{
>  		.name = "clk_imx8mp_audiomix.reset",
> +		.driver_data = (kernel_ulong_t)&imx8mp_reset_info,
>  	},
>  	{ }
>  };
> --
> 2.43.0
>

Re: [PATCH v2 6/8] reset: imx8mp-audiomix: Extend the driver usage

[Laurentiu Mihalcea]

On 10/17/2025 7:56 AM, Frank Li wrote:
> On Fri, Oct 17, 2025 at 04:20:23AM -0700, Laurentiu Mihalcea wrote:
>> From: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
>>
>> Some NXP SoCs integrate one or more, per-subsystem, block control IPs,
>> which allow users to control the assertion and de-assertion of the
>> reset lines tied to some peripherals present in said subsystems. Some
>> examples of such SoCs include i.MX8MP with AUDIOMIX block control and
>> i.MX8ULP with SIM LPAV.
>>
>> Some of the aformentioned block control IPs exhibit a common pattern with
>> respect to the assertion and de-assertion of the reset lines. Namely, the
>> user is able to control the state of the reset line by toggling a bit from
>> one of the IP's registers.
>>
>> Linux can take advantage of this pattern and, instead of having one driver
>> for each block control IP, a single, more generic driver could be used.
>>
>> To allow this to happen, the previous approach, in which a single reset
>> map is used, is replaced by a per-driver approach, in which each auxiliary
>> device driver holds a reference to a certain reset map.
> Can you shorter your commit message?, basically, you just add
> imx8mp_reset_info for difference auxiliary_device_id.


yeah, but the commit message is not trying to explain the code, but, rather,

offer a bit more context to understand WHY we're doing this. However, I'm not

fixated on this so if people don't think it's useful then I'll just shorten it.


>
>> Reviewed-by: Daniel Baluta <daniel.baluta@nxp.com>
>> Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
>> ---
>>  drivers/reset/reset-imx8mp-audiomix.c | 18 ++++++++++++++++--
>>  1 file changed, 16 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/reset/reset-imx8mp-audiomix.c b/drivers/reset/reset-imx8mp-audiomix.c
>> index c74ce6e04177..c370913107f5 100644
>> --- a/drivers/reset/reset-imx8mp-audiomix.c
>> +++ b/drivers/reset/reset-imx8mp-audiomix.c
>> @@ -24,7 +24,12 @@ struct imx8mp_reset_map {
>>  	bool active_low;
>>  };
>>
>> -static const struct imx8mp_reset_map reset_map[] = {
>> +struct imx8mp_reset_info {
>> +	const struct imx8mp_reset_map *map;
>> +	int num_lines;
>> +};
>> +
>> +static const struct imx8mp_reset_map imx8mp_reset_map[] = {
>>  	[IMX8MP_AUDIOMIX_EARC_RESET] = {
>>  		.offset	= IMX8MP_AUDIOMIX_EARC_RESET_OFFSET,
>>  		.mask = BIT(0),
>> @@ -45,10 +50,16 @@ static const struct imx8mp_reset_map reset_map[] = {
>>  	},
>>  };
>>
>> +static const struct imx8mp_reset_info imx8mp_reset_info = {
>> +	.map = imx8mp_reset_map,
>> +	.num_lines = ARRAY_SIZE(imx8mp_reset_map),
>> +};
>> +
>>  struct imx8mp_audiomix_reset {
>>  	struct reset_controller_dev rcdev;
>>  	void __iomem *base;
>>  	struct regmap *regmap;
>> +	const struct imx8mp_reset_info *rinfo;
>>  };
>>
>>  static struct imx8mp_audiomix_reset *to_imx8mp_audiomix_reset(struct reset_controller_dev *rcdev)
>> @@ -60,6 +71,7 @@ static int imx8mp_audiomix_update(struct reset_controller_dev *rcdev,
>>  				  unsigned long id, bool assert)
>>  {
>>  	struct imx8mp_audiomix_reset *priv = to_imx8mp_audiomix_reset(rcdev);
>> +	const struct imx8mp_reset_map *reset_map = priv->rinfo->map;
>>  	unsigned int mask, offset, active_low, shift, val;
>>
>>  	mask = reset_map[id].mask;
>> @@ -144,7 +156,8 @@ static int imx8mp_audiomix_reset_probe(struct auxiliary_device *adev,
>>  		return -ENOMEM;
>>
>>  	priv->rcdev.owner     = THIS_MODULE;
>> -	priv->rcdev.nr_resets = ARRAY_SIZE(reset_map);
>> +	priv->rinfo           = (const struct imx8mp_reset_info *)id->driver_data;
> needn't force convert from void*
>
> Frank
>
>> +	priv->rcdev.nr_resets = priv->rinfo->num_lines;
>>  	priv->rcdev.ops       = &imx8mp_audiomix_reset_ops;
>>  	priv->rcdev.of_node   = dev->parent->of_node;
>>  	priv->rcdev.dev	      = dev;
>> @@ -168,6 +181,7 @@ static int imx8mp_audiomix_reset_probe(struct auxiliary_device *adev,
>>  static const struct auxiliary_device_id imx8mp_audiomix_reset_ids[] = {
>>  	{
>>  		.name = "clk_imx8mp_audiomix.reset",
>> +		.driver_data = (kernel_ulong_t)&imx8mp_reset_info,
>>  	},
>>  	{ }
>>  };
>> --
>> 2.43.0
>>

Re: [PATCH v2 6/8] reset: imx8mp-audiomix: Extend the driver usage

[Frank Li]

On Mon, Oct 20, 2025 at 04:59:52AM -0700, Laurentiu Mihalcea wrote:
>
> On 10/17/2025 7:56 AM, Frank Li wrote:
> > On Fri, Oct 17, 2025 at 04:20:23AM -0700, Laurentiu Mihalcea wrote:
> >> From: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
> >>
> >> Some NXP SoCs integrate one or more, per-subsystem, block control IPs,
> >> which allow users to control the assertion and de-assertion of the
> >> reset lines tied to some peripherals present in said subsystems. Some
> >> examples of such SoCs include i.MX8MP with AUDIOMIX block control and
> >> i.MX8ULP with SIM LPAV.
> >>
> >> Some of the aformentioned block control IPs exhibit a common pattern with
> >> respect to the assertion and de-assertion of the reset lines. Namely, the
> >> user is able to control the state of the reset line by toggling a bit from
> >> one of the IP's registers.
> >>
> >> Linux can take advantage of this pattern and, instead of having one driver
> >> for each block control IP, a single, more generic driver could be used.
> >>
> >> To allow this to happen, the previous approach, in which a single reset
> >> map is used, is replaced by a per-driver approach, in which each auxiliary
> >> device driver holds a reference to a certain reset map.
> > Can you shorter your commit message?, basically, you just add
> > imx8mp_reset_info for difference auxiliary_device_id.
>
>
> yeah, but the commit message is not trying to explain the code, but, rather,
>
> offer a bit more context to understand WHY we're doing this. However, I'm not
>
> fixated on this so if people don't think it's useful then I'll just shorten it.

Not necessary to mention obvious facts. A commit message should summarize
meaningful information — it’s meant to highlight what’s important, not what
everyone already knows.

Using driver_data to distinguish between different chips or blocks is a
common and well-established approach used by many drivers.

For example: below commit message is enough to bring necesary informaiton
to reviewer.

"Managing peripheral reset lines by toggling a register bit is widely used
on NXP SoCs such as i.MX8MP and i.MX8ULP.

Add imx8mp_reset_map to the auxiliary_device_id's driver data to allow
reuse of the same driver for different reset controller blocks."


Frank

>
>
> >
> >> Reviewed-by: Daniel Baluta <daniel.baluta@nxp.com>
> >> Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
> >> ---
> >>  drivers/reset/reset-imx8mp-audiomix.c | 18 ++++++++++++++++--
> >>  1 file changed, 16 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/reset/reset-imx8mp-audiomix.c b/drivers/reset/reset-imx8mp-audiomix.c
> >> index c74ce6e04177..c370913107f5 100644
> >> --- a/drivers/reset/reset-imx8mp-audiomix.c
> >> +++ b/drivers/reset/reset-imx8mp-audiomix.c
> >> @@ -24,7 +24,12 @@ struct imx8mp_reset_map {
> >>  	bool active_low;
> >>  };
> >>
> >> -static const struct imx8mp_reset_map reset_map[] = {
> >> +struct imx8mp_reset_info {
> >> +	const struct imx8mp_reset_map *map;
> >> +	int num_lines;
> >> +};
> >> +
> >> +static const struct imx8mp_reset_map imx8mp_reset_map[] = {
> >>  	[IMX8MP_AUDIOMIX_EARC_RESET] = {
> >>  		.offset	= IMX8MP_AUDIOMIX_EARC_RESET_OFFSET,
> >>  		.mask = BIT(0),
> >> @@ -45,10 +50,16 @@ static const struct imx8mp_reset_map reset_map[] = {
> >>  	},
> >>  };
> >>
> >> +static const struct imx8mp_reset_info imx8mp_reset_info = {
> >> +	.map = imx8mp_reset_map,
> >> +	.num_lines = ARRAY_SIZE(imx8mp_reset_map),
> >> +};
> >> +
> >>  struct imx8mp_audiomix_reset {
> >>  	struct reset_controller_dev rcdev;
> >>  	void __iomem *base;
> >>  	struct regmap *regmap;
> >> +	const struct imx8mp_reset_info *rinfo;
> >>  };
> >>
> >>  static struct imx8mp_audiomix_reset *to_imx8mp_audiomix_reset(struct reset_controller_dev *rcdev)
> >> @@ -60,6 +71,7 @@ static int imx8mp_audiomix_update(struct reset_controller_dev *rcdev,
> >>  				  unsigned long id, bool assert)
> >>  {
> >>  	struct imx8mp_audiomix_reset *priv = to_imx8mp_audiomix_reset(rcdev);
> >> +	const struct imx8mp_reset_map *reset_map = priv->rinfo->map;
> >>  	unsigned int mask, offset, active_low, shift, val;
> >>
> >>  	mask = reset_map[id].mask;
> >> @@ -144,7 +156,8 @@ static int imx8mp_audiomix_reset_probe(struct auxiliary_device *adev,
> >>  		return -ENOMEM;
> >>
> >>  	priv->rcdev.owner     = THIS_MODULE;
> >> -	priv->rcdev.nr_resets = ARRAY_SIZE(reset_map);
> >> +	priv->rinfo           = (const struct imx8mp_reset_info *)id->driver_data;
> > needn't force convert from void*
> >
> > Frank
> >
> >> +	priv->rcdev.nr_resets = priv->rinfo->num_lines;
> >>  	priv->rcdev.ops       = &imx8mp_audiomix_reset_ops;
> >>  	priv->rcdev.of_node   = dev->parent->of_node;
> >>  	priv->rcdev.dev	      = dev;
> >> @@ -168,6 +181,7 @@ static int imx8mp_audiomix_reset_probe(struct auxiliary_device *adev,
> >>  static const struct auxiliary_device_id imx8mp_audiomix_reset_ids[] = {
> >>  	{
> >>  		.name = "clk_imx8mp_audiomix.reset",
> >> +		.driver_data = (kernel_ulong_t)&imx8mp_reset_info,
> >>  	},
> >>  	{ }
> >>  };
> >> --
> >> 2.43.0
> >>

Re: [PATCH v2 6/8] reset: imx8mp-audiomix: Extend the driver usage

[Laurentiu Mihalcea]

On 10/17/2025 7:56 AM, Frank Li wrote:
> On Fri, Oct 17, 2025 at 04:20:23AM -0700, Laurentiu Mihalcea wrote:
>> From: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
>>
>> Some NXP SoCs integrate one or more, per-subsystem, block control IPs,
>> which allow users to control the assertion and de-assertion of the
>> reset lines tied to some peripherals present in said subsystems. Some
>> examples of such SoCs include i.MX8MP with AUDIOMIX block control and
>> i.MX8ULP with SIM LPAV.
>>
>> Some of the aformentioned block control IPs exhibit a common pattern with
>> respect to the assertion and de-assertion of the reset lines. Namely, the
>> user is able to control the state of the reset line by toggling a bit from
>> one of the IP's registers.
>>
>> Linux can take advantage of this pattern and, instead of having one driver
>> for each block control IP, a single, more generic driver could be used.
>>
>> To allow this to happen, the previous approach, in which a single reset
>> map is used, is replaced by a per-driver approach, in which each auxiliary
>> device driver holds a reference to a certain reset map.
> Can you shorter your commit message?, basically, you just add
> imx8mp_reset_info for difference auxiliary_device_id.
>
>> Reviewed-by: Daniel Baluta <daniel.baluta@nxp.com>
>> Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
>> ---
>>  drivers/reset/reset-imx8mp-audiomix.c | 18 ++++++++++++++++--
>>  1 file changed, 16 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/reset/reset-imx8mp-audiomix.c b/drivers/reset/reset-imx8mp-audiomix.c
>> index c74ce6e04177..c370913107f5 100644
>> --- a/drivers/reset/reset-imx8mp-audiomix.c
>> +++ b/drivers/reset/reset-imx8mp-audiomix.c
>> @@ -24,7 +24,12 @@ struct imx8mp_reset_map {
>>  	bool active_low;
>>  };
>>
>> -static const struct imx8mp_reset_map reset_map[] = {
>> +struct imx8mp_reset_info {
>> +	const struct imx8mp_reset_map *map;
>> +	int num_lines;
>> +};
>> +
>> +static const struct imx8mp_reset_map imx8mp_reset_map[] = {
>>  	[IMX8MP_AUDIOMIX_EARC_RESET] = {
>>  		.offset	= IMX8MP_AUDIOMIX_EARC_RESET_OFFSET,
>>  		.mask = BIT(0),
>> @@ -45,10 +50,16 @@ static const struct imx8mp_reset_map reset_map[] = {
>>  	},
>>  };
>>
>> +static const struct imx8mp_reset_info imx8mp_reset_info = {
>> +	.map = imx8mp_reset_map,
>> +	.num_lines = ARRAY_SIZE(imx8mp_reset_map),
>> +};
>> +
>>  struct imx8mp_audiomix_reset {
>>  	struct reset_controller_dev rcdev;
>>  	void __iomem *base;
>>  	struct regmap *regmap;
>> +	const struct imx8mp_reset_info *rinfo;
>>  };
>>
>>  static struct imx8mp_audiomix_reset *to_imx8mp_audiomix_reset(struct reset_controller_dev *rcdev)
>> @@ -60,6 +71,7 @@ static int imx8mp_audiomix_update(struct reset_controller_dev *rcdev,
>>  				  unsigned long id, bool assert)
>>  {
>>  	struct imx8mp_audiomix_reset *priv = to_imx8mp_audiomix_reset(rcdev);
>> +	const struct imx8mp_reset_map *reset_map = priv->rinfo->map;
>>  	unsigned int mask, offset, active_low, shift, val;
>>
>>  	mask = reset_map[id].mask;
>> @@ -144,7 +156,8 @@ static int imx8mp_audiomix_reset_probe(struct auxiliary_device *adev,
>>  		return -ENOMEM;
>>
>>  	priv->rcdev.owner     = THIS_MODULE;
>> -	priv->rcdev.nr_resets = ARRAY_SIZE(reset_map);
>> +	priv->rinfo           = (const struct imx8mp_reset_info *)id->driver_data;
> needn't force convert from void*


not a void *, "id->driver_data" is kernel_ulong_t


>
> Frank
>
>> +	priv->rcdev.nr_resets = priv->rinfo->num_lines;
>>  	priv->rcdev.ops       = &imx8mp_audiomix_reset_ops;
>>  	priv->rcdev.of_node   = dev->parent->of_node;
>>  	priv->rcdev.dev	      = dev;
>> @@ -168,6 +181,7 @@ static int imx8mp_audiomix_reset_probe(struct auxiliary_device *adev,
>>  static const struct auxiliary_device_id imx8mp_audiomix_reset_ids[] = {
>>  	{
>>  		.name = "clk_imx8mp_audiomix.reset",
>> +		.driver_data = (kernel_ulong_t)&imx8mp_reset_info,
>>  	},
>>  	{ }
>>  };
>> --
>> 2.43.0
>>

Re: [PATCH v2 6/8] reset: imx8mp-audiomix: Extend the driver usage

[Frank Li]

On Mon, Oct 20, 2025 at 07:06:55AM -0700, Laurentiu Mihalcea wrote:
>
> On 10/17/2025 7:56 AM, Frank Li wrote:
> > On Fri, Oct 17, 2025 at 04:20:23AM -0700, Laurentiu Mihalcea wrote:
> >> From: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
> >>
> >> Some NXP SoCs integrate one or more, per-subsystem, block control IPs,
> >> which allow users to control the assertion and de-assertion of the
> >> reset lines tied to some peripherals present in said subsystems. Some
> >> examples of such SoCs include i.MX8MP with AUDIOMIX block control and
> >> i.MX8ULP with SIM LPAV.
> >>
> >> Some of the aformentioned block control IPs exhibit a common pattern with
> >> respect to the assertion and de-assertion of the reset lines. Namely, the
> >> user is able to control the state of the reset line by toggling a bit from
> >> one of the IP's registers.
> >>
> >> Linux can take advantage of this pattern and, instead of having one driver
> >> for each block control IP, a single, more generic driver could be used.
> >>
> >> To allow this to happen, the previous approach, in which a single reset
> >> map is used, is replaced by a per-driver approach, in which each auxiliary
> >> device driver holds a reference to a certain reset map.
> > Can you shorter your commit message?, basically, you just add
> > imx8mp_reset_info for difference auxiliary_device_id.
> >
> >> Reviewed-by: Daniel Baluta <daniel.baluta@nxp.com>
> >> Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
> >> ---
> >>  drivers/reset/reset-imx8mp-audiomix.c | 18 ++++++++++++++++--
> >>  1 file changed, 16 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/reset/reset-imx8mp-audiomix.c b/drivers/reset/reset-imx8mp-audiomix.c
> >> index c74ce6e04177..c370913107f5 100644
> >> --- a/drivers/reset/reset-imx8mp-audiomix.c
> >> +++ b/drivers/reset/reset-imx8mp-audiomix.c
> >> @@ -24,7 +24,12 @@ struct imx8mp_reset_map {
> >>  	bool active_low;
> >>  };
> >>
> >> -static const struct imx8mp_reset_map reset_map[] = {
> >> +struct imx8mp_reset_info {
> >> +	const struct imx8mp_reset_map *map;
> >> +	int num_lines;
> >> +};
> >> +
> >> +static const struct imx8mp_reset_map imx8mp_reset_map[] = {
> >>  	[IMX8MP_AUDIOMIX_EARC_RESET] = {
> >>  		.offset	= IMX8MP_AUDIOMIX_EARC_RESET_OFFSET,
> >>  		.mask = BIT(0),
> >> @@ -45,10 +50,16 @@ static const struct imx8mp_reset_map reset_map[] = {
> >>  	},
> >>  };
> >>
> >> +static const struct imx8mp_reset_info imx8mp_reset_info = {
> >> +	.map = imx8mp_reset_map,
> >> +	.num_lines = ARRAY_SIZE(imx8mp_reset_map),
> >> +};
> >> +
> >>  struct imx8mp_audiomix_reset {
> >>  	struct reset_controller_dev rcdev;
> >>  	void __iomem *base;
> >>  	struct regmap *regmap;
> >> +	const struct imx8mp_reset_info *rinfo;
> >>  };
> >>
> >>  static struct imx8mp_audiomix_reset *to_imx8mp_audiomix_reset(struct reset_controller_dev *rcdev)
> >> @@ -60,6 +71,7 @@ static int imx8mp_audiomix_update(struct reset_controller_dev *rcdev,
> >>  				  unsigned long id, bool assert)
> >>  {
> >>  	struct imx8mp_audiomix_reset *priv = to_imx8mp_audiomix_reset(rcdev);
> >> +	const struct imx8mp_reset_map *reset_map = priv->rinfo->map;
> >>  	unsigned int mask, offset, active_low, shift, val;
> >>
> >>  	mask = reset_map[id].mask;
> >> @@ -144,7 +156,8 @@ static int imx8mp_audiomix_reset_probe(struct auxiliary_device *adev,
> >>  		return -ENOMEM;
> >>
> >>  	priv->rcdev.owner     = THIS_MODULE;
> >> -	priv->rcdev.nr_resets = ARRAY_SIZE(reset_map);
> >> +	priv->rinfo           = (const struct imx8mp_reset_info *)id->driver_data;
> > needn't force convert from void*
>
>
> not a void *, "id->driver_data" is kernel_ulong_t

why not use void *?

Frank
>
>
> >
> > Frank
> >
> >> +	priv->rcdev.nr_resets = priv->rinfo->num_lines;
> >>  	priv->rcdev.ops       = &imx8mp_audiomix_reset_ops;
> >>  	priv->rcdev.of_node   = dev->parent->of_node;
> >>  	priv->rcdev.dev	      = dev;
> >> @@ -168,6 +181,7 @@ static int imx8mp_audiomix_reset_probe(struct auxiliary_device *adev,
> >>  static const struct auxiliary_device_id imx8mp_audiomix_reset_ids[] = {
> >>  	{
> >>  		.name = "clk_imx8mp_audiomix.reset",
> >> +		.driver_data = (kernel_ulong_t)&imx8mp_reset_info,
> >>  	},
> >>  	{ }
> >>  };
> >> --
> >> 2.43.0
> >>

Re: [PATCH v2 6/8] reset: imx8mp-audiomix: Extend the driver usage

[Daniel Baluta]

On Fri, Oct 17, 2025 at 5:57 PM Frank Li <Frank.li@nxp.com> wrote:
>
> On Fri, Oct 17, 2025 at 04:20:23AM -0700, Laurentiu Mihalcea wrote:
> > From: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
> >
> > Some NXP SoCs integrate one or more, per-subsystem, block control IPs,
> > which allow users to control the assertion and de-assertion of the
> > reset lines tied to some peripherals present in said subsystems. Some
> > examples of such SoCs include i.MX8MP with AUDIOMIX block control and
> > i.MX8ULP with SIM LPAV.
> >
> > Some of the aformentioned block control IPs exhibit a common pattern with
> > respect to the assertion and de-assertion of the reset lines. Namely, the
> > user is able to control the state of the reset line by toggling a bit from
> > one of the IP's registers.
> >
> > Linux can take advantage of this pattern and, instead of having one driver
> > for each block control IP, a single, more generic driver could be used.
> >
> > To allow this to happen, the previous approach, in which a single reset
> > map is used, is replaced by a per-driver approach, in which each auxiliary
> > device driver holds a reference to a certain reset map.
>
> Can you shorter your commit message?, basically, you just add
> imx8mp_reset_info for difference auxiliary_device_id.

It is always preferred to have commit messages trying to explain the context.
I have never heard anyone complaining that the commit message is too long :D.

Daniel.

[PATCH v2 7/8] reset: imx8mp-audiomix: Support i.MX8ULP SIM LPAV

[Laurentiu Mihalcea]

From: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>

Support i.MX8ULP's SIM LPAV by adding its reset map definition.

Reviewed-by: Daniel Baluta <daniel.baluta@nxp.com>
Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
---
 drivers/reset/reset-imx8mp-audiomix.c | 51 +++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/drivers/reset/reset-imx8mp-audiomix.c b/drivers/reset/reset-imx8mp-audiomix.c
index c370913107f5..b333d7c1442a 100644
--- a/drivers/reset/reset-imx8mp-audiomix.c
+++ b/drivers/reset/reset-imx8mp-audiomix.c
@@ -3,6 +3,7 @@
  * Copyright 2024 NXP
  */
 
+#include <dt-bindings/reset/fsl,imx8ulp-sim-lpav.h>
 #include <dt-bindings/reset/imx8mp-reset-audiomix.h>
 
 #include <linux/auxiliary_bus.h>
@@ -17,6 +18,8 @@
 #define IMX8MP_AUDIOMIX_EARC_RESET_OFFSET	0x200
 #define IMX8MP_AUDIOMIX_DSP_RUNSTALL_OFFSET	0x108
 
+#define IMX8ULP_SIM_LPAV_SYSCTRL0_OFFSET	0x8
+
 struct imx8mp_reset_map {
 	unsigned int offset;
 	unsigned int mask;
@@ -55,6 +58,50 @@ static const struct imx8mp_reset_info imx8mp_reset_info = {
 	.num_lines = ARRAY_SIZE(imx8mp_reset_map),
 };
 
+static const struct imx8mp_reset_map imx8ulp_reset_map[] = {
+	[IMX8ULP_SIM_LPAV_HIFI4_DSP_DBG_RST] = {
+		.offset = IMX8ULP_SIM_LPAV_SYSCTRL0_OFFSET,
+		.mask = BIT(25),
+		.shift = 25,
+		.active_low = false,
+	},
+	[IMX8ULP_SIM_LPAV_HIFI4_DSP_RST] = {
+		.offset = IMX8ULP_SIM_LPAV_SYSCTRL0_OFFSET,
+		.mask = BIT(16),
+		.shift = 16,
+		.active_low = false,
+	},
+	[IMX8ULP_SIM_LPAV_HIFI4_DSP_STALL] = {
+		.offset = IMX8ULP_SIM_LPAV_SYSCTRL0_OFFSET,
+		.mask = BIT(13),
+		.shift = 13,
+		.active_low = false,
+	},
+	[IMX8ULP_SIM_LPAV_DSI_RST_BYTE_N] = {
+		.offset = IMX8ULP_SIM_LPAV_SYSCTRL0_OFFSET,
+		.mask = BIT(5),
+		.shift = 5,
+		.active_low = true,
+	},
+	[IMX8ULP_SIM_LPAV_DSI_RST_ESC_N] = {
+		.offset = IMX8ULP_SIM_LPAV_SYSCTRL0_OFFSET,
+		.mask = BIT(4),
+		.shift = 4,
+		.active_low = true,
+	},
+	[IMX8ULP_SIM_LPAV_DSI_RST_DPI_N] = {
+		.offset = IMX8ULP_SIM_LPAV_SYSCTRL0_OFFSET,
+		.mask = BIT(3),
+		.shift = 3,
+		.active_low = true,
+	},
+};
+
+static const struct imx8mp_reset_info imx8ulp_reset_info = {
+	.map = imx8ulp_reset_map,
+	.num_lines = ARRAY_SIZE(imx8ulp_reset_map),
+};
+
 struct imx8mp_audiomix_reset {
 	struct reset_controller_dev rcdev;
 	void __iomem *base;
@@ -183,6 +230,10 @@ static const struct auxiliary_device_id imx8mp_audiomix_reset_ids[] = {
 		.name = "clk_imx8mp_audiomix.reset",
 		.driver_data = (kernel_ulong_t)&imx8mp_reset_info,
 	},
+	{
+		.name = "clk_imx8ulp_sim_lpav.reset",
+		.driver_data = (kernel_ulong_t)&imx8ulp_reset_info,
+	},
 	{ }
 };
 MODULE_DEVICE_TABLE(auxiliary, imx8mp_audiomix_reset_ids);
-- 
2.43.0

Re: [PATCH v2 7/8] reset: imx8mp-audiomix: Support i.MX8ULP SIM LPAV

[Frank Li]

On Fri, Oct 17, 2025 at 04:20:24AM -0700, Laurentiu Mihalcea wrote:
> From: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
>
> Support i.MX8ULP's SIM LPAV by adding its reset map definition.
>
> Reviewed-by: Daniel Baluta <daniel.baluta@nxp.com>
> Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
> ---
>  drivers/reset/reset-imx8mp-audiomix.c | 51 +++++++++++++++++++++++++++
>  1 file changed, 51 insertions(+)
>
> diff --git a/drivers/reset/reset-imx8mp-audiomix.c b/drivers/reset/reset-imx8mp-audiomix.c
> index c370913107f5..b333d7c1442a 100644
> --- a/drivers/reset/reset-imx8mp-audiomix.c
> +++ b/drivers/reset/reset-imx8mp-audiomix.c
> @@ -3,6 +3,7 @@
>   * Copyright 2024 NXP
>   */
>
> +#include <dt-bindings/reset/fsl,imx8ulp-sim-lpav.h>
>  #include <dt-bindings/reset/imx8mp-reset-audiomix.h>
>
>  #include <linux/auxiliary_bus.h>
> @@ -17,6 +18,8 @@
>  #define IMX8MP_AUDIOMIX_EARC_RESET_OFFSET	0x200
>  #define IMX8MP_AUDIOMIX_DSP_RUNSTALL_OFFSET	0x108
>
> +#define IMX8ULP_SIM_LPAV_SYSCTRL0_OFFSET	0x8
> +
>  struct imx8mp_reset_map {
>  	unsigned int offset;
>  	unsigned int mask;
> @@ -55,6 +58,50 @@ static const struct imx8mp_reset_info imx8mp_reset_info = {
>  	.num_lines = ARRAY_SIZE(imx8mp_reset_map),
>  };
>
> +static const struct imx8mp_reset_map imx8ulp_reset_map[] = {
> +	[IMX8ULP_SIM_LPAV_HIFI4_DSP_DBG_RST] = {
> +		.offset = IMX8ULP_SIM_LPAV_SYSCTRL0_OFFSET,
> +		.mask = BIT(25),

Register defination still perfer use macro. If not, let me know.

Frank
> +		.shift = 25,
> +		.active_low = false,
> +	},
> +	[IMX8ULP_SIM_LPAV_HIFI4_DSP_RST] = {
> +		.offset = IMX8ULP_SIM_LPAV_SYSCTRL0_OFFSET,
> +		.mask = BIT(16),
> +		.shift = 16,
> +		.active_low = false,
> +	},
> +	[IMX8ULP_SIM_LPAV_HIFI4_DSP_STALL] = {
> +		.offset = IMX8ULP_SIM_LPAV_SYSCTRL0_OFFSET,
> +		.mask = BIT(13),
> +		.shift = 13,
> +		.active_low = false,
> +	},
> +	[IMX8ULP_SIM_LPAV_DSI_RST_BYTE_N] = {
> +		.offset = IMX8ULP_SIM_LPAV_SYSCTRL0_OFFSET,
> +		.mask = BIT(5),
> +		.shift = 5,
> +		.active_low = true,
> +	},
> +	[IMX8ULP_SIM_LPAV_DSI_RST_ESC_N] = {
> +		.offset = IMX8ULP_SIM_LPAV_SYSCTRL0_OFFSET,
> +		.mask = BIT(4),
> +		.shift = 4,
> +		.active_low = true,
> +	},
> +	[IMX8ULP_SIM_LPAV_DSI_RST_DPI_N] = {
> +		.offset = IMX8ULP_SIM_LPAV_SYSCTRL0_OFFSET,
> +		.mask = BIT(3),
> +		.shift = 3,
> +		.active_low = true,
> +	},
> +};
> +
> +static const struct imx8mp_reset_info imx8ulp_reset_info = {
> +	.map = imx8ulp_reset_map,
> +	.num_lines = ARRAY_SIZE(imx8ulp_reset_map),
> +};
> +
>  struct imx8mp_audiomix_reset {
>  	struct reset_controller_dev rcdev;
>  	void __iomem *base;
> @@ -183,6 +230,10 @@ static const struct auxiliary_device_id imx8mp_audiomix_reset_ids[] = {
>  		.name = "clk_imx8mp_audiomix.reset",
>  		.driver_data = (kernel_ulong_t)&imx8mp_reset_info,
>  	},
> +	{
> +		.name = "clk_imx8ulp_sim_lpav.reset",
> +		.driver_data = (kernel_ulong_t)&imx8ulp_reset_info,
> +	},
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(auxiliary, imx8mp_audiomix_reset_ids);
> --
> 2.43.0
>

Re: [PATCH v2 7/8] reset: imx8mp-audiomix: Support i.MX8ULP SIM LPAV

[Laurentiu Mihalcea]

On 10/17/2025 7:57 AM, Frank Li wrote:
> On Fri, Oct 17, 2025 at 04:20:24AM -0700, Laurentiu Mihalcea wrote:
>> From: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
>>
>> Support i.MX8ULP's SIM LPAV by adding its reset map definition.
>>
>> Reviewed-by: Daniel Baluta <daniel.baluta@nxp.com>
>> Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
>> ---
>>  drivers/reset/reset-imx8mp-audiomix.c | 51 +++++++++++++++++++++++++++
>>  1 file changed, 51 insertions(+)
>>
>> diff --git a/drivers/reset/reset-imx8mp-audiomix.c b/drivers/reset/reset-imx8mp-audiomix.c
>> index c370913107f5..b333d7c1442a 100644
>> --- a/drivers/reset/reset-imx8mp-audiomix.c
>> +++ b/drivers/reset/reset-imx8mp-audiomix.c
>> @@ -3,6 +3,7 @@
>>   * Copyright 2024 NXP
>>   */
>>
>> +#include <dt-bindings/reset/fsl,imx8ulp-sim-lpav.h>
>>  #include <dt-bindings/reset/imx8mp-reset-audiomix.h>
>>
>>  #include <linux/auxiliary_bus.h>
>> @@ -17,6 +18,8 @@
>>  #define IMX8MP_AUDIOMIX_EARC_RESET_OFFSET	0x200
>>  #define IMX8MP_AUDIOMIX_DSP_RUNSTALL_OFFSET	0x108
>>
>> +#define IMX8ULP_SIM_LPAV_SYSCTRL0_OFFSET	0x8
>> +
>>  struct imx8mp_reset_map {
>>  	unsigned int offset;
>>  	unsigned int mask;
>> @@ -55,6 +58,50 @@ static const struct imx8mp_reset_info imx8mp_reset_info = {
>>  	.num_lines = ARRAY_SIZE(imx8mp_reset_map),
>>  };
>>
>> +static const struct imx8mp_reset_map imx8ulp_reset_map[] = {
>> +	[IMX8ULP_SIM_LPAV_HIFI4_DSP_DBG_RST] = {
>> +		.offset = IMX8ULP_SIM_LPAV_SYSCTRL0_OFFSET,
>> +		.mask = BIT(25),
> Register defination still perfer use macro. If not, let me know.

I see no value in adding defines for the masks (see patch 4 commit message)

in this particular scenario.


Is the assignment of the "mask" field for the "struct imx8mp_reset_map" item found

at index  IMX8ULP_SIM_LPAV_HIFI4_DSP_DBG_RST not enough to deduce that the

constant we're using is the mask for the DSP_DBG_RST bit?


>
> Frank
>> +		.shift = 25,
>> +		.active_low = false,
>> +	},
>> +	[IMX8ULP_SIM_LPAV_HIFI4_DSP_RST] = {
>> +		.offset = IMX8ULP_SIM_LPAV_SYSCTRL0_OFFSET,
>> +		.mask = BIT(16),
>> +		.shift = 16,
>> +		.active_low = false,
>> +	},
>> +	[IMX8ULP_SIM_LPAV_HIFI4_DSP_STALL] = {
>> +		.offset = IMX8ULP_SIM_LPAV_SYSCTRL0_OFFSET,
>> +		.mask = BIT(13),
>> +		.shift = 13,
>> +		.active_low = false,
>> +	},
>> +	[IMX8ULP_SIM_LPAV_DSI_RST_BYTE_N] = {
>> +		.offset = IMX8ULP_SIM_LPAV_SYSCTRL0_OFFSET,
>> +		.mask = BIT(5),
>> +		.shift = 5,
>> +		.active_low = true,
>> +	},
>> +	[IMX8ULP_SIM_LPAV_DSI_RST_ESC_N] = {
>> +		.offset = IMX8ULP_SIM_LPAV_SYSCTRL0_OFFSET,
>> +		.mask = BIT(4),
>> +		.shift = 4,
>> +		.active_low = true,
>> +	},
>> +	[IMX8ULP_SIM_LPAV_DSI_RST_DPI_N] = {
>> +		.offset = IMX8ULP_SIM_LPAV_SYSCTRL0_OFFSET,
>> +		.mask = BIT(3),
>> +		.shift = 3,
>> +		.active_low = true,
>> +	},
>> +};
>> +
>> +static const struct imx8mp_reset_info imx8ulp_reset_info = {
>> +	.map = imx8ulp_reset_map,
>> +	.num_lines = ARRAY_SIZE(imx8ulp_reset_map),
>> +};
>> +
>>  struct imx8mp_audiomix_reset {
>>  	struct reset_controller_dev rcdev;
>>  	void __iomem *base;
>> @@ -183,6 +230,10 @@ static const struct auxiliary_device_id imx8mp_audiomix_reset_ids[] = {
>>  		.name = "clk_imx8mp_audiomix.reset",
>>  		.driver_data = (kernel_ulong_t)&imx8mp_reset_info,
>>  	},
>> +	{
>> +		.name = "clk_imx8ulp_sim_lpav.reset",
>> +		.driver_data = (kernel_ulong_t)&imx8ulp_reset_info,
>> +	},
>>  	{ }
>>  };
>>  MODULE_DEVICE_TABLE(auxiliary, imx8mp_audiomix_reset_ids);
>> --
>> 2.43.0
>>

Re: [PATCH v2 7/8] reset: imx8mp-audiomix: Support i.MX8ULP SIM LPAV

[Frank Li]

On Mon, Oct 20, 2025 at 07:29:28AM -0700, Laurentiu Mihalcea wrote:
>
> On 10/17/2025 7:57 AM, Frank Li wrote:
> > On Fri, Oct 17, 2025 at 04:20:24AM -0700, Laurentiu Mihalcea wrote:
> >> From: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
> >>
> >> Support i.MX8ULP's SIM LPAV by adding its reset map definition.
> >>
> >> Reviewed-by: Daniel Baluta <daniel.baluta@nxp.com>
> >> Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
> >> ---
> >>  drivers/reset/reset-imx8mp-audiomix.c | 51 +++++++++++++++++++++++++++
> >>  1 file changed, 51 insertions(+)
> >>
> >> diff --git a/drivers/reset/reset-imx8mp-audiomix.c b/drivers/reset/reset-imx8mp-audiomix.c
> >> index c370913107f5..b333d7c1442a 100644
> >> --- a/drivers/reset/reset-imx8mp-audiomix.c
> >> +++ b/drivers/reset/reset-imx8mp-audiomix.c
> >> @@ -3,6 +3,7 @@
> >>   * Copyright 2024 NXP
> >>   */
> >>
> >> +#include <dt-bindings/reset/fsl,imx8ulp-sim-lpav.h>
> >>  #include <dt-bindings/reset/imx8mp-reset-audiomix.h>
> >>
> >>  #include <linux/auxiliary_bus.h>
> >> @@ -17,6 +18,8 @@
> >>  #define IMX8MP_AUDIOMIX_EARC_RESET_OFFSET	0x200
> >>  #define IMX8MP_AUDIOMIX_DSP_RUNSTALL_OFFSET	0x108
> >>
> >> +#define IMX8ULP_SIM_LPAV_SYSCTRL0_OFFSET	0x8
> >> +
> >>  struct imx8mp_reset_map {
> >>  	unsigned int offset;
> >>  	unsigned int mask;
> >> @@ -55,6 +58,50 @@ static const struct imx8mp_reset_info imx8mp_reset_info = {
> >>  	.num_lines = ARRAY_SIZE(imx8mp_reset_map),
> >>  };
> >>
> >> +static const struct imx8mp_reset_map imx8ulp_reset_map[] = {
> >> +	[IMX8ULP_SIM_LPAV_HIFI4_DSP_DBG_RST] = {
> >> +		.offset = IMX8ULP_SIM_LPAV_SYSCTRL0_OFFSET,
> >> +		.mask = BIT(25),
> > Register defination still perfer use macro. If not, let me know.
>
> I see no value in adding defines for the masks (see patch 4 commit message)
>
> in this particular scenario.
>
>
> Is the assignment of the "mask" field for the "struct imx8mp_reset_map" item found
>
> at index  IMX8ULP_SIM_LPAV_HIFI4_DSP_DBG_RST not enough to deduce that the
>
> constant we're using is the mask for the DSP_DBG_RST bit?

This bit is NOT software choose bit, which must be align hardware spec.
Define macro help map name to spec and easy to look for spec by use macro.

There are over thousand result to seach bit 25.

eventhough search SYSCTRL0, may have many SYSCTRL0 in RM.

Frank
>
>
> >
> > Frank
> >> +		.shift = 25,
> >> +		.active_low = false,
> >> +	},
> >> +	[IMX8ULP_SIM_LPAV_HIFI4_DSP_RST] = {
> >> +		.offset = IMX8ULP_SIM_LPAV_SYSCTRL0_OFFSET,
> >> +		.mask = BIT(16),
> >> +		.shift = 16,
> >> +		.active_low = false,
> >> +	},
> >> +	[IMX8ULP_SIM_LPAV_HIFI4_DSP_STALL] = {
> >> +		.offset = IMX8ULP_SIM_LPAV_SYSCTRL0_OFFSET,
> >> +		.mask = BIT(13),
> >> +		.shift = 13,
> >> +		.active_low = false,
> >> +	},
> >> +	[IMX8ULP_SIM_LPAV_DSI_RST_BYTE_N] = {
> >> +		.offset = IMX8ULP_SIM_LPAV_SYSCTRL0_OFFSET,
> >> +		.mask = BIT(5),
> >> +		.shift = 5,
> >> +		.active_low = true,
> >> +	},
> >> +	[IMX8ULP_SIM_LPAV_DSI_RST_ESC_N] = {
> >> +		.offset = IMX8ULP_SIM_LPAV_SYSCTRL0_OFFSET,
> >> +		.mask = BIT(4),
> >> +		.shift = 4,
> >> +		.active_low = true,
> >> +	},
> >> +	[IMX8ULP_SIM_LPAV_DSI_RST_DPI_N] = {
> >> +		.offset = IMX8ULP_SIM_LPAV_SYSCTRL0_OFFSET,
> >> +		.mask = BIT(3),
> >> +		.shift = 3,
> >> +		.active_low = true,
> >> +	},
> >> +};
> >> +
> >> +static const struct imx8mp_reset_info imx8ulp_reset_info = {
> >> +	.map = imx8ulp_reset_map,
> >> +	.num_lines = ARRAY_SIZE(imx8ulp_reset_map),
> >> +};
> >> +
> >>  struct imx8mp_audiomix_reset {
> >>  	struct reset_controller_dev rcdev;
> >>  	void __iomem *base;
> >> @@ -183,6 +230,10 @@ static const struct auxiliary_device_id imx8mp_audiomix_reset_ids[] = {
> >>  		.name = "clk_imx8mp_audiomix.reset",
> >>  		.driver_data = (kernel_ulong_t)&imx8mp_reset_info,
> >>  	},
> >> +	{
> >> +		.name = "clk_imx8ulp_sim_lpav.reset",
> >> +		.driver_data = (kernel_ulong_t)&imx8ulp_reset_info,
> >> +	},
> >>  	{ }
> >>  };
> >>  MODULE_DEVICE_TABLE(auxiliary, imx8mp_audiomix_reset_ids);
> >> --
> >> 2.43.0
> >>

Re: [PATCH v2 7/8] reset: imx8mp-audiomix: Support i.MX8ULP SIM LPAV

[Laurentiu Mihalcea]

On 10/20/2025 7:50 AM, Frank Li wrote:
> On Mon, Oct 20, 2025 at 07:29:28AM -0700, Laurentiu Mihalcea wrote:
>> On 10/17/2025 7:57 AM, Frank Li wrote:
>>> On Fri, Oct 17, 2025 at 04:20:24AM -0700, Laurentiu Mihalcea wrote:
>>>> From: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
>>>>
>>>> Support i.MX8ULP's SIM LPAV by adding its reset map definition.
>>>>
>>>> Reviewed-by: Daniel Baluta <daniel.baluta@nxp.com>
>>>> Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
>>>> ---
>>>>  drivers/reset/reset-imx8mp-audiomix.c | 51 +++++++++++++++++++++++++++
>>>>  1 file changed, 51 insertions(+)
>>>>
>>>> diff --git a/drivers/reset/reset-imx8mp-audiomix.c b/drivers/reset/reset-imx8mp-audiomix.c
>>>> index c370913107f5..b333d7c1442a 100644
>>>> --- a/drivers/reset/reset-imx8mp-audiomix.c
>>>> +++ b/drivers/reset/reset-imx8mp-audiomix.c
>>>> @@ -3,6 +3,7 @@
>>>>   * Copyright 2024 NXP
>>>>   */
>>>>
>>>> +#include <dt-bindings/reset/fsl,imx8ulp-sim-lpav.h>
>>>>  #include <dt-bindings/reset/imx8mp-reset-audiomix.h>
>>>>
>>>>  #include <linux/auxiliary_bus.h>
>>>> @@ -17,6 +18,8 @@
>>>>  #define IMX8MP_AUDIOMIX_EARC_RESET_OFFSET	0x200
>>>>  #define IMX8MP_AUDIOMIX_DSP_RUNSTALL_OFFSET	0x108
>>>>
>>>> +#define IMX8ULP_SIM_LPAV_SYSCTRL0_OFFSET	0x8
>>>> +
>>>>  struct imx8mp_reset_map {
>>>>  	unsigned int offset;
>>>>  	unsigned int mask;
>>>> @@ -55,6 +58,50 @@ static const struct imx8mp_reset_info imx8mp_reset_info = {
>>>>  	.num_lines = ARRAY_SIZE(imx8mp_reset_map),
>>>>  };
>>>>
>>>> +static const struct imx8mp_reset_map imx8ulp_reset_map[] = {
>>>> +	[IMX8ULP_SIM_LPAV_HIFI4_DSP_DBG_RST] = {
>>>> +		.offset = IMX8ULP_SIM_LPAV_SYSCTRL0_OFFSET,
>>>> +		.mask = BIT(25),
>>> Register defination still perfer use macro. If not, let me know.
>> I see no value in adding defines for the masks (see patch 4 commit message)
>>
>> in this particular scenario.
>>
>>
>> Is the assignment of the "mask" field for the "struct imx8mp_reset_map" item found
>>
>> at index  IMX8ULP_SIM_LPAV_HIFI4_DSP_DBG_RST not enough to deduce that the
>>
>> constant we're using is the mask for the DSP_DBG_RST bit?
> This bit is NOT software choose bit, which must be align hardware spec.
> Define macro help map name to spec and easy to look for spec by use macro.


yeah, we already have the DT binding macros for that which perfectly match the name

of the corresponding bit in the SYSCTRL0 register. I don't see how adding 6 more macros

with the SAME name as the DT binding macros and the "_MASK" suffix would help you in

this regard?


>
> There are over thousand result to seach bit 25.
>
> eventhough search SYSCTRL0, may have many SYSCTRL0 in RM.
>
> Frank
>>
>>> Frank
>>>> +		.shift = 25,
>>>> +		.active_low = false,
>>>> +	},
>>>> +	[IMX8ULP_SIM_LPAV_HIFI4_DSP_RST] = {
>>>> +		.offset = IMX8ULP_SIM_LPAV_SYSCTRL0_OFFSET,
>>>> +		.mask = BIT(16),
>>>> +		.shift = 16,
>>>> +		.active_low = false,
>>>> +	},
>>>> +	[IMX8ULP_SIM_LPAV_HIFI4_DSP_STALL] = {
>>>> +		.offset = IMX8ULP_SIM_LPAV_SYSCTRL0_OFFSET,
>>>> +		.mask = BIT(13),
>>>> +		.shift = 13,
>>>> +		.active_low = false,
>>>> +	},
>>>> +	[IMX8ULP_SIM_LPAV_DSI_RST_BYTE_N] = {
>>>> +		.offset = IMX8ULP_SIM_LPAV_SYSCTRL0_OFFSET,
>>>> +		.mask = BIT(5),
>>>> +		.shift = 5,
>>>> +		.active_low = true,
>>>> +	},
>>>> +	[IMX8ULP_SIM_LPAV_DSI_RST_ESC_N] = {
>>>> +		.offset = IMX8ULP_SIM_LPAV_SYSCTRL0_OFFSET,
>>>> +		.mask = BIT(4),
>>>> +		.shift = 4,
>>>> +		.active_low = true,
>>>> +	},
>>>> +	[IMX8ULP_SIM_LPAV_DSI_RST_DPI_N] = {
>>>> +		.offset = IMX8ULP_SIM_LPAV_SYSCTRL0_OFFSET,
>>>> +		.mask = BIT(3),
>>>> +		.shift = 3,
>>>> +		.active_low = true,
>>>> +	},
>>>> +};
>>>> +
>>>> +static const struct imx8mp_reset_info imx8ulp_reset_info = {
>>>> +	.map = imx8ulp_reset_map,
>>>> +	.num_lines = ARRAY_SIZE(imx8ulp_reset_map),
>>>> +};
>>>> +
>>>>  struct imx8mp_audiomix_reset {
>>>>  	struct reset_controller_dev rcdev;
>>>>  	void __iomem *base;
>>>> @@ -183,6 +230,10 @@ static const struct auxiliary_device_id imx8mp_audiomix_reset_ids[] = {
>>>>  		.name = "clk_imx8mp_audiomix.reset",
>>>>  		.driver_data = (kernel_ulong_t)&imx8mp_reset_info,
>>>>  	},
>>>> +	{
>>>> +		.name = "clk_imx8ulp_sim_lpav.reset",
>>>> +		.driver_data = (kernel_ulong_t)&imx8ulp_reset_info,
>>>> +	},
>>>>  	{ }
>>>>  };
>>>>  MODULE_DEVICE_TABLE(auxiliary, imx8mp_audiomix_reset_ids);
>>>> --
>>>> 2.43.0
>>>>

Re: [PATCH v2 7/8] reset: imx8mp-audiomix: Support i.MX8ULP SIM LPAV

[Frank Li]

On Tue, Oct 21, 2025 at 06:16:41AM -0700, Laurentiu Mihalcea wrote:
>
> On 10/20/2025 7:50 AM, Frank Li wrote:
> > On Mon, Oct 20, 2025 at 07:29:28AM -0700, Laurentiu Mihalcea wrote:
> >> On 10/17/2025 7:57 AM, Frank Li wrote:
> >>> On Fri, Oct 17, 2025 at 04:20:24AM -0700, Laurentiu Mihalcea wrote:
> >>>> From: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
> >>>>
> >>>> Support i.MX8ULP's SIM LPAV by adding its reset map definition.
> >>>>
> >>>> Reviewed-by: Daniel Baluta <daniel.baluta@nxp.com>
> >>>> Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
> >>>> ---
> >>>>  drivers/reset/reset-imx8mp-audiomix.c | 51 +++++++++++++++++++++++++++
> >>>>  1 file changed, 51 insertions(+)
> >>>>
> >>>> diff --git a/drivers/reset/reset-imx8mp-audiomix.c b/drivers/reset/reset-imx8mp-audiomix.c
> >>>> index c370913107f5..b333d7c1442a 100644
> >>>> --- a/drivers/reset/reset-imx8mp-audiomix.c
> >>>> +++ b/drivers/reset/reset-imx8mp-audiomix.c
> >>>> @@ -3,6 +3,7 @@
> >>>>   * Copyright 2024 NXP
> >>>>   */
> >>>>
> >>>> +#include <dt-bindings/reset/fsl,imx8ulp-sim-lpav.h>
> >>>>  #include <dt-bindings/reset/imx8mp-reset-audiomix.h>
> >>>>
> >>>>  #include <linux/auxiliary_bus.h>
> >>>> @@ -17,6 +18,8 @@
> >>>>  #define IMX8MP_AUDIOMIX_EARC_RESET_OFFSET	0x200
> >>>>  #define IMX8MP_AUDIOMIX_DSP_RUNSTALL_OFFSET	0x108
> >>>>
> >>>> +#define IMX8ULP_SIM_LPAV_SYSCTRL0_OFFSET	0x8
> >>>> +
> >>>>  struct imx8mp_reset_map {
> >>>>  	unsigned int offset;
> >>>>  	unsigned int mask;
> >>>> @@ -55,6 +58,50 @@ static const struct imx8mp_reset_info imx8mp_reset_info = {
> >>>>  	.num_lines = ARRAY_SIZE(imx8mp_reset_map),
> >>>>  };
> >>>>
> >>>> +static const struct imx8mp_reset_map imx8ulp_reset_map[] = {
> >>>> +	[IMX8ULP_SIM_LPAV_HIFI4_DSP_DBG_RST] = {
> >>>> +		.offset = IMX8ULP_SIM_LPAV_SYSCTRL0_OFFSET,
> >>>> +		.mask = BIT(25),
> >>> Register defination still perfer use macro. If not, let me know.
> >> I see no value in adding defines for the masks (see patch 4 commit message)
> >>
> >> in this particular scenario.
> >>
> >>
> >> Is the assignment of the "mask" field for the "struct imx8mp_reset_map" item found
> >>
> >> at index  IMX8ULP_SIM_LPAV_HIFI4_DSP_DBG_RST not enough to deduce that the
> >>
> >> constant we're using is the mask for the DSP_DBG_RST bit?
> > This bit is NOT software choose bit, which must be align hardware spec.
> > Define macro help map name to spec and easy to look for spec by use macro.
>
>
> yeah, we already have the DT binding macros for that which perfectly match the name
>
> of the corresponding bit in the SYSCTRL0 register. I don't see how adding 6 more macros
>
> with the SAME name as the DT binding macros and the "_MASK" suffix would help you in
>
> this regard?

Okay, make sense.

Frank
>
>
> >
> > There are over thousand result to seach bit 25.
> >
> > eventhough search SYSCTRL0, may have many SYSCTRL0 in RM.
> >
> > Frank
> >>
> >>> Frank
> >>>> +		.shift = 25,
> >>>> +		.active_low = false,
> >>>> +	},
> >>>> +	[IMX8ULP_SIM_LPAV_HIFI4_DSP_RST] = {
> >>>> +		.offset = IMX8ULP_SIM_LPAV_SYSCTRL0_OFFSET,
> >>>> +		.mask = BIT(16),
> >>>> +		.shift = 16,
> >>>> +		.active_low = false,
> >>>> +	},
> >>>> +	[IMX8ULP_SIM_LPAV_HIFI4_DSP_STALL] = {
> >>>> +		.offset = IMX8ULP_SIM_LPAV_SYSCTRL0_OFFSET,
> >>>> +		.mask = BIT(13),
> >>>> +		.shift = 13,
> >>>> +		.active_low = false,
> >>>> +	},
> >>>> +	[IMX8ULP_SIM_LPAV_DSI_RST_BYTE_N] = {
> >>>> +		.offset = IMX8ULP_SIM_LPAV_SYSCTRL0_OFFSET,
> >>>> +		.mask = BIT(5),
> >>>> +		.shift = 5,
> >>>> +		.active_low = true,
> >>>> +	},
> >>>> +	[IMX8ULP_SIM_LPAV_DSI_RST_ESC_N] = {
> >>>> +		.offset = IMX8ULP_SIM_LPAV_SYSCTRL0_OFFSET,
> >>>> +		.mask = BIT(4),
> >>>> +		.shift = 4,
> >>>> +		.active_low = true,
> >>>> +	},
> >>>> +	[IMX8ULP_SIM_LPAV_DSI_RST_DPI_N] = {
> >>>> +		.offset = IMX8ULP_SIM_LPAV_SYSCTRL0_OFFSET,
> >>>> +		.mask = BIT(3),
> >>>> +		.shift = 3,
> >>>> +		.active_low = true,
> >>>> +	},
> >>>> +};
> >>>> +
> >>>> +static const struct imx8mp_reset_info imx8ulp_reset_info = {
> >>>> +	.map = imx8ulp_reset_map,
> >>>> +	.num_lines = ARRAY_SIZE(imx8ulp_reset_map),
> >>>> +};
> >>>> +
> >>>>  struct imx8mp_audiomix_reset {
> >>>>  	struct reset_controller_dev rcdev;
> >>>>  	void __iomem *base;
> >>>> @@ -183,6 +230,10 @@ static const struct auxiliary_device_id imx8mp_audiomix_reset_ids[] = {
> >>>>  		.name = "clk_imx8mp_audiomix.reset",
> >>>>  		.driver_data = (kernel_ulong_t)&imx8mp_reset_info,
> >>>>  	},
> >>>> +	{
> >>>> +		.name = "clk_imx8ulp_sim_lpav.reset",
> >>>> +		.driver_data = (kernel_ulong_t)&imx8ulp_reset_info,
> >>>> +	},
> >>>>  	{ }
> >>>>  };
> >>>>  MODULE_DEVICE_TABLE(auxiliary, imx8mp_audiomix_reset_ids);
> >>>> --
> >>>> 2.43.0
> >>>>

[PATCH v2 8/8] arm64: dts: imx8ulp: add sim lpav node

[Laurentiu Mihalcea]

From: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>

Add DT node for the SIM LPAV module.

Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
---
 arch/arm64/boot/dts/freescale/imx8ulp.dtsi | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8ulp.dtsi b/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
index 13b01f3aa2a4..676535c3fc84 100644
--- a/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
@@ -776,6 +776,23 @@ edma2: dma-controller@2d800000 {
 						"ch28", "ch29", "ch30", "ch31";
 			};
 
+			sim_lpav: clock-controller@2da50000 {
+				compatible = "fsl,imx8ulp-sim-lpav";
+				reg = <0x2da50000 0x10000>;
+				clocks = <&cgc2 IMX8ULP_CLK_LPAV_BUS_DIV>,
+					 <&cgc2 IMX8ULP_CLK_HIFI_DIVCORE>,
+					 <&cgc2 IMX8ULP_CLK_HIFI_DIVPLAT>;
+				clock-names = "lpav_bus", "hifi_core", "hifi_plat";
+				#clock-cells = <1>;
+				#reset-cells = <1>;
+
+				sim_lpav_mux: mux-controller {
+					compatible = "reg-mux";
+					#mux-control-cells = <1>;
+					mux-reg-masks = <0x8 0x00000200>;
+				};
+			};
+
 			cgc2: clock-controller@2da60000 {
 				compatible = "fsl,imx8ulp-cgc2";
 				reg = <0x2da60000 0x10000>;
-- 
2.43.0

Re: [PATCH v2 8/8] arm64: dts: imx8ulp: add sim lpav node

[Daniel Baluta]

On Fri, Oct 17, 2025 at 2:24 PM Laurentiu Mihalcea
<laurentiumihalcea111@gmail.com> wrote:
>
> From: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
>
> Add DT node for the SIM LPAV module.
>
> Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>

Reviewed-by: Daniel Baluta <daniel.baluta@nxp.com>