2024-05-16 20:41:35

by Laurentiu Mihalcea

[permalink] [raw]
Subject: [PATCH 0/4] Add support for imx8ulp's SIM

From: Laurentiu Mihalcea <[email protected]>

i.MX8ULP's SIM (System Integration Module) allows
control and configuration of certain components
from the domain it's assigned to. Add DT node
and schema for it. The children shall also be
included. For the reset controller child, this
also includes a schema and a driver.

Laurentiu Mihalcea (4):
dt-bindings: reset: add schema for imx8ulp SIM reset
reset: add driver for imx8ulp SIM reset controller
dt-bindings: mfd: add schema for 8ulp's SIM
arm64: dts: imx8ulp: add AVD-SIM node

.../bindings/mfd/nxp,imx8ulp-sim.yaml | 71 ++++++++++++++
.../bindings/reset/nxp,imx8ulp-sim-reset.yaml | 43 ++++++++
arch/arm64/boot/dts/freescale/imx8ulp.dtsi | 17 ++++
drivers/reset/Kconfig | 7 ++
drivers/reset/Makefile | 1 +
drivers/reset/reset-imx8ulp-sim.c | 98 +++++++++++++++++++
include/dt-bindings/reset/imx8ulp-sim-reset.h | 16 +++
7 files changed, 253 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mfd/nxp,imx8ulp-sim.yaml
create mode 100644 Documentation/devicetree/bindings/reset/nxp,imx8ulp-sim-reset.yaml
create mode 100644 drivers/reset/reset-imx8ulp-sim.c
create mode 100644 include/dt-bindings/reset/imx8ulp-sim-reset.h

--
2.34.1



2024-05-16 20:41:50

by Laurentiu Mihalcea

[permalink] [raw]
Subject: [PATCH 1/4] dt-bindings: reset: add schema for imx8ulp SIM reset

From: Laurentiu Mihalcea <[email protected]>

Add schema for imx8ulp's SIM reset controller.

Signed-off-by: Liu Ying <[email protected]>
Signed-off-by: Laurentiu Mihalcea <[email protected]>
---
.../bindings/reset/nxp,imx8ulp-sim-reset.yaml | 43 +++++++++++++++++++
1 file changed, 43 insertions(+)
create mode 100644 Documentation/devicetree/bindings/reset/nxp,imx8ulp-sim-reset.yaml

diff --git a/Documentation/devicetree/bindings/reset/nxp,imx8ulp-sim-reset.yaml b/Documentation/devicetree/bindings/reset/nxp,imx8ulp-sim-reset.yaml
new file mode 100644
index 000000000000..ec9a5c73e83c
--- /dev/null
+++ b/Documentation/devicetree/bindings/reset/nxp,imx8ulp-sim-reset.yaml
@@ -0,0 +1,43 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/reset/nxp,imx8ulp-sim-reset.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NXP i.MX8ULP System Integration Module Reset Controller
+
+maintainers:
+ - Liu Ying <[email protected]>
+
+description: |
+ Some instances of i.MX8ULP's SIM may offer control over the
+ reset of some components of a certain domain (e.g: AVD-SIM).
+ As far as the DT is concerned, this means that the reset
+ controller needs to be a child of the SIM node.
+
+properties:
+ compatible:
+ const: nxp,imx8ulp-avd-sim-reset
+
+ '#reset-cells':
+ const: 1
+
+required:
+ - compatible
+ - '#reset-cells'
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/imx8ulp-clock.h>
+ syscon@2da50000 {
+ compatible = "nxp,imx8ulp-avd-sim", "syscon";
+ reg = <0x2da50000 0x38>;
+ clocks = <&pcc5 IMX8ULP_CLK_AVD_SIM>;
+
+ reset-controller {
+ compatible = "nxp,imx8ulp-avd-sim-reset";
+ #reset-cells = <1>;
+ };
+ };
--
2.34.1


2024-05-16 20:42:05

by Laurentiu Mihalcea

[permalink] [raw]
Subject: [PATCH 2/4] reset: add driver for imx8ulp SIM reset controller

From: Laurentiu Mihalcea <[email protected]>

Certain components can be reset via the SIM module.
Add reset controller driver for the SIM module to
allow drivers for said components to control the
reset signal(s).

Signed-off-by: Liu Ying <[email protected]>
Signed-off-by: Laurentiu Mihalcea <[email protected]>
---
drivers/reset/Kconfig | 7 ++
drivers/reset/Makefile | 1 +
drivers/reset/reset-imx8ulp-sim.c | 98 +++++++++++++++++++
include/dt-bindings/reset/imx8ulp-sim-reset.h | 16 +++
4 files changed, 122 insertions(+)
create mode 100644 drivers/reset/reset-imx8ulp-sim.c
create mode 100644 include/dt-bindings/reset/imx8ulp-sim-reset.h

diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index 85b27c42cf65..c1f4d9ebd0fd 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -91,6 +91,13 @@ config RESET_IMX7
help
This enables the reset controller driver for i.MX7 SoCs.

+config RESET_IMX8ULP_SIM
+ tristate "i.MX8ULP SIM Reset Driver"
+ depends on ARCH_MXC
+ help
+ This enables the SIM (System Integration Module) reset driver
+ for the i.MX8ULP SoC.
+
config RESET_INTEL_GW
bool "Intel Reset Controller Driver"
depends on X86 || COMPILE_TEST
diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
index fd8b49fa46fc..f257d6a41f1e 100644
--- a/drivers/reset/Makefile
+++ b/drivers/reset/Makefile
@@ -42,3 +42,4 @@ obj-$(CONFIG_RESET_UNIPHIER) += reset-uniphier.o
obj-$(CONFIG_RESET_UNIPHIER_GLUE) += reset-uniphier-glue.o
obj-$(CONFIG_RESET_ZYNQ) += reset-zynq.o
obj-$(CONFIG_ARCH_ZYNQMP) += reset-zynqmp.o
+obj-$(CONFIG_RESET_IMX8ULP_SIM) += reset-imx8ulp-sim.o
diff --git a/drivers/reset/reset-imx8ulp-sim.c b/drivers/reset/reset-imx8ulp-sim.c
new file mode 100644
index 000000000000..27923b9cd454
--- /dev/null
+++ b/drivers/reset/reset-imx8ulp-sim.c
@@ -0,0 +1,98 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/*
+ * Copyright 2024 NXP
+ */
+
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/reset-controller.h>
+#include <dt-bindings/reset/imx8ulp-sim-reset.h>
+
+#define AVD_SIM_SYSCTRL0 0x8
+
+struct imx8ulp_sim_reset {
+ struct reset_controller_dev rcdev;
+ struct regmap *regmap;
+};
+
+static const u32 imx8ulp_sim_reset_bits[IMX8ULP_SIM_RESET_NUM] = {
+ [IMX8ULP_SIM_RESET_MIPI_DSI_RST_DPI_N] = BIT(3),
+ [IMX8ULP_SIM_RESET_MIPI_DSI_RST_ESC_N] = BIT(4),
+ [IMX8ULP_SIM_RESET_MIPI_DSI_RST_BYTE_N] = BIT(5),
+};
+
+static inline struct imx8ulp_sim_reset *
+to_imx8ulp_sim_reset(struct reset_controller_dev *rcdev)
+{
+ return container_of(rcdev, struct imx8ulp_sim_reset, rcdev);
+}
+
+static int imx8ulp_sim_reset_assert(struct reset_controller_dev *rcdev,
+ unsigned long id)
+{
+ struct imx8ulp_sim_reset *simr = to_imx8ulp_sim_reset(rcdev);
+ const u32 bit = imx8ulp_sim_reset_bits[id];
+
+ return regmap_update_bits(simr->regmap, AVD_SIM_SYSCTRL0, bit, 0);
+}
+
+static int imx8ulp_sim_reset_deassert(struct reset_controller_dev *rcdev,
+ unsigned long id)
+{
+ struct imx8ulp_sim_reset *simr = to_imx8ulp_sim_reset(rcdev);
+ const u32 bit = imx8ulp_sim_reset_bits[id];
+
+ return regmap_update_bits(simr->regmap, AVD_SIM_SYSCTRL0, bit, bit);
+}
+
+static const struct reset_control_ops imx8ulp_sim_reset_ops = {
+ .assert = imx8ulp_sim_reset_assert,
+ .deassert = imx8ulp_sim_reset_deassert,
+};
+
+static const struct of_device_id imx8ulp_sim_reset_dt_ids[] = {
+ { .compatible = "nxp,imx8ulp-avd-sim-reset", },
+ { /* sentinel */ },
+};
+
+static int imx8ulp_sim_reset_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct imx8ulp_sim_reset *simr;
+ int ret;
+
+ simr = devm_kzalloc(dev, sizeof(*simr), GFP_KERNEL);
+ if (!simr)
+ return -ENOMEM;
+
+ simr->regmap = syscon_node_to_regmap(dev->of_node->parent);
+ if (IS_ERR(simr->regmap)) {
+ ret = PTR_ERR(simr->regmap);
+ dev_err(dev, "failed to get regmap: %d\n", ret);
+ return ret;
+ }
+
+ simr->rcdev.owner = THIS_MODULE;
+ simr->rcdev.nr_resets = IMX8ULP_SIM_RESET_NUM;
+ simr->rcdev.ops = &imx8ulp_sim_reset_ops;
+ simr->rcdev.of_node = dev->of_node;
+
+ return devm_reset_controller_register(dev, &simr->rcdev);
+}
+
+static struct platform_driver imx8ulp_sim_reset_driver = {
+ .probe = imx8ulp_sim_reset_probe,
+ .driver = {
+ .name = KBUILD_MODNAME,
+ .of_match_table = imx8ulp_sim_reset_dt_ids,
+ },
+};
+module_platform_driver(imx8ulp_sim_reset_driver);
+
+MODULE_AUTHOR("Liu Ying <[email protected]>");
+MODULE_DESCRIPTION("NXP i.MX8ULP System Integration Module Reset driver");
+MODULE_LICENSE("GPL");
diff --git a/include/dt-bindings/reset/imx8ulp-sim-reset.h b/include/dt-bindings/reset/imx8ulp-sim-reset.h
new file mode 100644
index 000000000000..a3cee0d60773
--- /dev/null
+++ b/include/dt-bindings/reset/imx8ulp-sim-reset.h
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0-only OR BSD-3-Clause */
+
+/*
+ * Copyright 2024 NXP
+ */
+
+#ifndef DT_BINDINGS_RESET_IMX8ULP_SIM_RESET_H
+#define DT_BINDINGS_RESET_IMX8ULP_SIM_RESET_H
+
+#define IMX8ULP_SIM_RESET_MIPI_DSI_RST_DPI_N 0
+#define IMX8ULP_SIM_RESET_MIPI_DSI_RST_ESC_N 1
+#define IMX8ULP_SIM_RESET_MIPI_DSI_RST_BYTE_N 2
+
+#define IMX8ULP_SIM_RESET_NUM 3
+
+#endif /* DT_BINDINGS_RESET_IMX8ULP_SIM_RESET_H */
--
2.34.1


2024-05-16 20:42:17

by Laurentiu Mihalcea

[permalink] [raw]
Subject: [PATCH 3/4] dt-bindings: mfd: add schema for 8ulp's SIM

From: Laurentiu Mihalcea <[email protected]>

Add schema for i.MX8ULP's SIM.

Signed-off-by: Liu Ying <[email protected]>
Signed-off-by: Laurentiu Mihalcea <[email protected]>
---
.../bindings/mfd/nxp,imx8ulp-sim.yaml | 71 +++++++++++++++++++
1 file changed, 71 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mfd/nxp,imx8ulp-sim.yaml

diff --git a/Documentation/devicetree/bindings/mfd/nxp,imx8ulp-sim.yaml b/Documentation/devicetree/bindings/mfd/nxp,imx8ulp-sim.yaml
new file mode 100644
index 000000000000..fbb17c06e3c0
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/nxp,imx8ulp-sim.yaml
@@ -0,0 +1,71 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/nxp,imx8ulp-sim.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NXP i.MX8ULP System Integration Module
+
+maintainers:
+ - Liu Ying <[email protected]>
+
+description: |
+ 8ULP's SIM provides control and configuration options for
+ some of the chip's components.
+
+properties:
+ compatible:
+ items:
+ - enum:
+ - nxp,imx8ulp-avd-sim
+ - const: syscon
+
+ reg:
+ maxItems: 1
+
+ clocks:
+ maxItems: 1
+
+ mux-controller:
+ $ref: ../mux/reg-mux.yaml
+
+ reset-controller:
+ $ref: ../reset/nxp,imx8ulp-sim-reset.yaml
+
+required:
+ - compatible
+ - reg
+ - clocks
+
+allOf:
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: nxp,imx8ulp-avd-sim
+ then:
+ required:
+ - reset-controller
+ - mux-controller
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/imx8ulp-clock.h>
+ syscon@2da50000 {
+ compatible = "nxp,imx8ulp-avd-sim", "syscon";
+ reg = <0x2da50000 0x38>;
+ clocks = <&pcc5 IMX8ULP_CLK_AVD_SIM>;
+
+ mux-controller {
+ compatible = "mmio-mux";
+ #mux-control-cells = <1>;
+ mux-reg-masks = <0x8 0x00000200>;
+ };
+
+ reset-controller {
+ compatible = "nxp,imx8ulp-avd-sim-reset";
+ #reset-cells = <1>;
+ };
+ };
--
2.34.1


2024-05-16 20:42:33

by Laurentiu Mihalcea

[permalink] [raw]
Subject: [PATCH 4/4] arm64: dts: imx8ulp: add AVD-SIM node

From: Laurentiu Mihalcea <[email protected]>

Add node for imx8ulp's AVD-SIM. This also includes its children.

Signed-off-by: Liu Ying <[email protected]>
Signed-off-by: Laurentiu Mihalcea <[email protected]>
---
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 c4a0082f30d3..5135d98dc6f2 100644
--- a/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
@@ -520,6 +520,23 @@ per_bridge5: bus@2d800000 {
#size-cells = <1>;
ranges;

+ avd_sim: syscon@2da50000 {
+ compatible = "nxp,imx8ulp-avd-sim", "syscon";
+ reg = <0x2da50000 0x38>;
+ clocks = <&pcc5 IMX8ULP_CLK_AVD_SIM>;
+
+ mux: mux-controller {
+ compatible = "mmio-mux";
+ #mux-control-cells = <1>;
+ mux-reg-masks = <0x8 0x00000200>;
+ };
+
+ avd_sim_rst: reset-controller {
+ compatible = "nxp,imx8ulp-avd-sim-reset";
+ #reset-cells = <1>;
+ };
+ };
+
cgc2: clock-controller@2da60000 {
compatible = "fsl,imx8ulp-cgc2";
reg = <0x2da60000 0x10000>;
--
2.34.1


2024-05-16 21:22:31

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH 1/4] dt-bindings: reset: add schema for imx8ulp SIM reset


On Thu, 16 May 2024 23:40:28 +0300, Laurentiu Mihalcea wrote:
> From: Laurentiu Mihalcea <[email protected]>
>
> Add schema for imx8ulp's SIM reset controller.
>
> Signed-off-by: Liu Ying <[email protected]>
> Signed-off-by: Laurentiu Mihalcea <[email protected]>
> ---
> .../bindings/reset/nxp,imx8ulp-sim-reset.yaml | 43 +++++++++++++++++++
> 1 file changed, 43 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/reset/nxp,imx8ulp-sim-reset.yaml
>

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/reset/nxp,imx8ulp-sim-reset.example.dtb: /example-0/syscon@2da50000: failed to match any schema with compatible: ['nxp,imx8ulp-avd-sim', 'syscon']

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[email protected]

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.


2024-05-16 21:22:40

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH 3/4] dt-bindings: mfd: add schema for 8ulp's SIM


On Thu, 16 May 2024 23:40:30 +0300, Laurentiu Mihalcea wrote:
> From: Laurentiu Mihalcea <[email protected]>
>
> Add schema for i.MX8ULP's SIM.
>
> Signed-off-by: Liu Ying <[email protected]>
> Signed-off-by: Laurentiu Mihalcea <[email protected]>
> ---
> .../bindings/mfd/nxp,imx8ulp-sim.yaml | 71 +++++++++++++++++++
> 1 file changed, 71 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mfd/nxp,imx8ulp-sim.yaml
>

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/reset/nxp,imx8ulp-sim-reset.example.dtb: syscon@2da50000: 'mux-controller' is a required property
from schema $id: http://devicetree.org/schemas/mfd/nxp,imx8ulp-sim.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[email protected]

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.


2024-05-17 06:22:15

by Amit Singh Tomar

[permalink] [raw]
Subject: [PATCH 2/4] reset: add driver for imx8ulp SIM reset controller

Hi,

> ----------------------------------------------------------------------
> From: Laurentiu Mihalcea <[email protected]>
>
> Certain components can be reset via the SIM module.
> Add reset controller driver for the SIM module to
> allow drivers for said components to control the
> reset signal(s).
>
> Signed-off-by: Liu Ying <[email protected]>
> Signed-off-by: Laurentiu Mihalcea <[email protected]>
> ---
> drivers/reset/Kconfig | 7 ++
> drivers/reset/Makefile | 1 +
> drivers/reset/reset-imx8ulp-sim.c | 98 +++++++++++++++++++

Just out of curiosity, can't this be accomplished using a generic reset
driver?

https://elixir.bootlin.com/linux/latest/source/drivers/reset/reset-simple.c

> include/dt-bindings/reset/imx8ulp-sim-reset.h | 16 +++
> 4 files changed, 122 insertions(+)
> create mode 100644 drivers/reset/reset-imx8ulp-sim.c
> create mode 100644 include/dt-bindings/reset/imx8ulp-sim-reset.h
>
> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> index 85b27c42cf65..c1f4d9ebd0fd 100644
> --- a/drivers/reset/Kconfig
> +++ b/drivers/reset/Kconfig
> @@ -91,6 +91,13 @@ config RESET_IMX7
> help
> This enables the reset controller driver for i.MX7 SoCs.
>
> +config RESET_IMX8ULP_SIM
> + tristate "i.MX8ULP SIM Reset Driver"
> + depends on ARCH_MXC
> + help
> + This enables the SIM (System Integration Module) reset driver
> + for the i.MX8ULP SoC.
> +
> config RESET_INTEL_GW
> bool "Intel Reset Controller Driver"
> depends on X86 || COMPILE_TEST
> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
> index fd8b49fa46fc..f257d6a41f1e 100644
> --- a/drivers/reset/Makefile
> +++ b/drivers/reset/Makefile
> @@ -42,3 +42,4 @@ obj-$(CONFIG_RESET_UNIPHIER) += reset-uniphier.o
> obj-$(CONFIG_RESET_UNIPHIER_GLUE) += reset-uniphier-glue.o
> obj-$(CONFIG_RESET_ZYNQ) += reset-zynq.o
> obj-$(CONFIG_ARCH_ZYNQMP) += reset-zynqmp.o
> +obj-$(CONFIG_RESET_IMX8ULP_SIM) += reset-imx8ulp-sim.o
> diff --git a/drivers/reset/reset-imx8ulp-sim.c b/drivers/reset/reset-imx8ulp-sim.c
> new file mode 100644
> index 000000000000..27923b9cd454
> --- /dev/null
> +++ b/drivers/reset/reset-imx8ulp-sim.c
> @@ -0,0 +1,98 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +
> +/*
> + * Copyright 2024 NXP
> + */
> +
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/reset-controller.h>
> +#include <dt-bindings/reset/imx8ulp-sim-reset.h>
> +
> +#define AVD_SIM_SYSCTRL0 0x8
> +
> +struct imx8ulp_sim_reset {
> + struct reset_controller_dev rcdev;
> + struct regmap *regmap;
> +};
> +
> +static const u32 imx8ulp_sim_reset_bits[IMX8ULP_SIM_RESET_NUM] = {
> + [IMX8ULP_SIM_RESET_MIPI_DSI_RST_DPI_N] = BIT(3),
> + [IMX8ULP_SIM_RESET_MIPI_DSI_RST_ESC_N] = BIT(4),
> + [IMX8ULP_SIM_RESET_MIPI_DSI_RST_BYTE_N] = BIT(5),
> +};
> +
> +static inline struct imx8ulp_sim_reset *
> +to_imx8ulp_sim_reset(struct reset_controller_dev *rcdev)
> +{
> + return container_of(rcdev, struct imx8ulp_sim_reset, rcdev);
> +}
> +
> +static int imx8ulp_sim_reset_assert(struct reset_controller_dev *rcdev,
> + unsigned long id)
> +{
> + struct imx8ulp_sim_reset *simr = to_imx8ulp_sim_reset(rcdev);
> + const u32 bit = imx8ulp_sim_reset_bits[id];
> +
> + return regmap_update_bits(simr->regmap, AVD_SIM_SYSCTRL0, bit, 0);
> +}
> +
> +static int imx8ulp_sim_reset_deassert(struct reset_controller_dev *rcdev,
> + unsigned long id)
> +{
> + struct imx8ulp_sim_reset *simr = to_imx8ulp_sim_reset(rcdev);
> + const u32 bit = imx8ulp_sim_reset_bits[id];
> +
> + return regmap_update_bits(simr->regmap, AVD_SIM_SYSCTRL0, bit, bit);
> +}
> +
> +static const struct reset_control_ops imx8ulp_sim_reset_ops = {
> + .assert = imx8ulp_sim_reset_assert,
> + .deassert = imx8ulp_sim_reset_deassert,
> +};
> +
> +static const struct of_device_id imx8ulp_sim_reset_dt_ids[] = {
> + { .compatible = "nxp,imx8ulp-avd-sim-reset", },
> + { /* sentinel */ },
> +};
> +
> +static int imx8ulp_sim_reset_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct imx8ulp_sim_reset *simr;
> + int ret;
> +
> + simr = devm_kzalloc(dev, sizeof(*simr), GFP_KERNEL);
> + if (!simr)
> + return -ENOMEM;
> +
> + simr->regmap = syscon_node_to_regmap(dev->of_node->parent);
> + if (IS_ERR(simr->regmap)) {
> + ret = PTR_ERR(simr->regmap);
> + dev_err(dev, "failed to get regmap: %d\n", ret);
> + return ret;
> + }
> +
> + simr->rcdev.owner = THIS_MODULE;
> + simr->rcdev.nr_resets = IMX8ULP_SIM_RESET_NUM;
> + simr->rcdev.ops = &imx8ulp_sim_reset_ops;
> + simr->rcdev.of_node = dev->of_node;
> +
> + return devm_reset_controller_register(dev, &simr->rcdev);
> +}
> +
> +static struct platform_driver imx8ulp_sim_reset_driver = {
> + .probe = imx8ulp_sim_reset_probe,
> + .driver = {
> + .name = KBUILD_MODNAME,
> + .of_match_table = imx8ulp_sim_reset_dt_ids,
> + },
> +};
> +module_platform_driver(imx8ulp_sim_reset_driver);
> +
> +MODULE_AUTHOR("Liu Ying <[email protected]>");
> +MODULE_DESCRIPTION("NXP i.MX8ULP System Integration Module Reset driver");
> +MODULE_LICENSE("GPL");
> diff --git a/include/dt-bindings/reset/imx8ulp-sim-reset.h b/include/dt-bindings/reset/imx8ulp-sim-reset.h
> new file mode 100644
> index 000000000000..a3cee0d60773
> --- /dev/null
> +++ b/include/dt-bindings/reset/imx8ulp-sim-reset.h
> @@ -0,0 +1,16 @@
> +/* SPDX-License-Identifier: GPL-2.0-only OR BSD-3-Clause */
> +
> +/*
> + * Copyright 2024 NXP
> + */
> +
> +#ifndef DT_BINDINGS_RESET_IMX8ULP_SIM_RESET_H
> +#define DT_BINDINGS_RESET_IMX8ULP_SIM_RESET_H
> +
> +#define IMX8ULP_SIM_RESET_MIPI_DSI_RST_DPI_N 0
> +#define IMX8ULP_SIM_RESET_MIPI_DSI_RST_ESC_N 1
> +#define IMX8ULP_SIM_RESET_MIPI_DSI_RST_BYTE_N 2
> +
> +#define IMX8ULP_SIM_RESET_NUM 3
> +
> +#endif /* DT_BINDINGS_RESET_IMX8ULP_SIM_RESET_H */


2024-05-17 15:18:39

by Frank Li

[permalink] [raw]
Subject: Re: [PATCH 2/4] reset: add driver for imx8ulp SIM reset controller

On Fri, May 17, 2024 at 11:51:30AM +0530, Amit Singh Tomar wrote:
> Hi,
>
> > ----------------------------------------------------------------------
> > From: Laurentiu Mihalcea <[email protected]>
> >
> > Certain components can be reset via the SIM module.
> > Add reset controller driver for the SIM module to
> > allow drivers for said components to control the
> > reset signal(s).
> >
> > Signed-off-by: Liu Ying <[email protected]>
> > Signed-off-by: Laurentiu Mihalcea <[email protected]>
> > ---
> > drivers/reset/Kconfig | 7 ++
> > drivers/reset/Makefile | 1 +
> > drivers/reset/reset-imx8ulp-sim.c | 98 +++++++++++++++++++
>
> Just out of curiosity, can't this be accomplished using a generic reset
> driver?
>
> https://elixir.bootlin.com/linux/latest/source/drivers/reset/reset-simple.c

reset-simple have not use regmap. I think it can use ti's
https://elixir.bootlin.com/linux/latest/source/drivers/reset/reset-ti-syscon.c

Or should change ti to reset-simple-syscon? or add regmap support for
reset-simple.c

Frank Li


>
> > include/dt-bindings/reset/imx8ulp-sim-reset.h | 16 +++
> > 4 files changed, 122 insertions(+)
> > create mode 100644 drivers/reset/reset-imx8ulp-sim.c
> > create mode 100644 include/dt-bindings/reset/imx8ulp-sim-reset.h
> >
> > diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> > index 85b27c42cf65..c1f4d9ebd0fd 100644
> > --- a/drivers/reset/Kconfig
> > +++ b/drivers/reset/Kconfig
> > @@ -91,6 +91,13 @@ config RESET_IMX7
> > help
> > This enables the reset controller driver for i.MX7 SoCs.
> > +config RESET_IMX8ULP_SIM
> > + tristate "i.MX8ULP SIM Reset Driver"
> > + depends on ARCH_MXC
> > + help
> > + This enables the SIM (System Integration Module) reset driver
> > + for the i.MX8ULP SoC.
> > +
> > config RESET_INTEL_GW
> > bool "Intel Reset Controller Driver"
> > depends on X86 || COMPILE_TEST
> > diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
> > index fd8b49fa46fc..f257d6a41f1e 100644
> > --- a/drivers/reset/Makefile
> > +++ b/drivers/reset/Makefile
> > @@ -42,3 +42,4 @@ obj-$(CONFIG_RESET_UNIPHIER) += reset-uniphier.o
> > obj-$(CONFIG_RESET_UNIPHIER_GLUE) += reset-uniphier-glue.o
> > obj-$(CONFIG_RESET_ZYNQ) += reset-zynq.o
> > obj-$(CONFIG_ARCH_ZYNQMP) += reset-zynqmp.o
> > +obj-$(CONFIG_RESET_IMX8ULP_SIM) += reset-imx8ulp-sim.o
> > diff --git a/drivers/reset/reset-imx8ulp-sim.c b/drivers/reset/reset-imx8ulp-sim.c
> > new file mode 100644
> > index 000000000000..27923b9cd454
> > --- /dev/null
> > +++ b/drivers/reset/reset-imx8ulp-sim.c
> > @@ -0,0 +1,98 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +
> > +/*
> > + * Copyright 2024 NXP
> > + */
> > +
> > +#include <linux/mfd/syscon.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/regmap.h>
> > +#include <linux/reset-controller.h>
> > +#include <dt-bindings/reset/imx8ulp-sim-reset.h>
> > +
> > +#define AVD_SIM_SYSCTRL0 0x8
> > +
> > +struct imx8ulp_sim_reset {
> > + struct reset_controller_dev rcdev;
> > + struct regmap *regmap;
> > +};
> > +
> > +static const u32 imx8ulp_sim_reset_bits[IMX8ULP_SIM_RESET_NUM] = {
> > + [IMX8ULP_SIM_RESET_MIPI_DSI_RST_DPI_N] = BIT(3),
> > + [IMX8ULP_SIM_RESET_MIPI_DSI_RST_ESC_N] = BIT(4),
> > + [IMX8ULP_SIM_RESET_MIPI_DSI_RST_BYTE_N] = BIT(5),
> > +};
> > +
> > +static inline struct imx8ulp_sim_reset *
> > +to_imx8ulp_sim_reset(struct reset_controller_dev *rcdev)
> > +{
> > + return container_of(rcdev, struct imx8ulp_sim_reset, rcdev);
> > +}
> > +
> > +static int imx8ulp_sim_reset_assert(struct reset_controller_dev *rcdev,
> > + unsigned long id)
> > +{
> > + struct imx8ulp_sim_reset *simr = to_imx8ulp_sim_reset(rcdev);
> > + const u32 bit = imx8ulp_sim_reset_bits[id];
> > +
> > + return regmap_update_bits(simr->regmap, AVD_SIM_SYSCTRL0, bit, 0);
> > +}
> > +
> > +static int imx8ulp_sim_reset_deassert(struct reset_controller_dev *rcdev,
> > + unsigned long id)
> > +{
> > + struct imx8ulp_sim_reset *simr = to_imx8ulp_sim_reset(rcdev);
> > + const u32 bit = imx8ulp_sim_reset_bits[id];
> > +
> > + return regmap_update_bits(simr->regmap, AVD_SIM_SYSCTRL0, bit, bit);
> > +}
> > +
> > +static const struct reset_control_ops imx8ulp_sim_reset_ops = {
> > + .assert = imx8ulp_sim_reset_assert,
> > + .deassert = imx8ulp_sim_reset_deassert,
> > +};
> > +
> > +static const struct of_device_id imx8ulp_sim_reset_dt_ids[] = {
> > + { .compatible = "nxp,imx8ulp-avd-sim-reset", },
> > + { /* sentinel */ },
> > +};
> > +
> > +static int imx8ulp_sim_reset_probe(struct platform_device *pdev)
> > +{
> > + struct device *dev = &pdev->dev;
> > + struct imx8ulp_sim_reset *simr;
> > + int ret;
> > +
> > + simr = devm_kzalloc(dev, sizeof(*simr), GFP_KERNEL);
> > + if (!simr)
> > + return -ENOMEM;
> > +
> > + simr->regmap = syscon_node_to_regmap(dev->of_node->parent);
> > + if (IS_ERR(simr->regmap)) {
> > + ret = PTR_ERR(simr->regmap);
> > + dev_err(dev, "failed to get regmap: %d\n", ret);
> > + return ret;
> > + }
> > +
> > + simr->rcdev.owner = THIS_MODULE;
> > + simr->rcdev.nr_resets = IMX8ULP_SIM_RESET_NUM;
> > + simr->rcdev.ops = &imx8ulp_sim_reset_ops;
> > + simr->rcdev.of_node = dev->of_node;
> > +
> > + return devm_reset_controller_register(dev, &simr->rcdev);
> > +}
> > +
> > +static struct platform_driver imx8ulp_sim_reset_driver = {
> > + .probe = imx8ulp_sim_reset_probe,
> > + .driver = {
> > + .name = KBUILD_MODNAME,
> > + .of_match_table = imx8ulp_sim_reset_dt_ids,
> > + },
> > +};
> > +module_platform_driver(imx8ulp_sim_reset_driver);
> > +
> > +MODULE_AUTHOR("Liu Ying <[email protected]>");
> > +MODULE_DESCRIPTION("NXP i.MX8ULP System Integration Module Reset driver");
> > +MODULE_LICENSE("GPL");
> > diff --git a/include/dt-bindings/reset/imx8ulp-sim-reset.h b/include/dt-bindings/reset/imx8ulp-sim-reset.h
> > new file mode 100644
> > index 000000000000..a3cee0d60773
> > --- /dev/null
> > +++ b/include/dt-bindings/reset/imx8ulp-sim-reset.h
> > @@ -0,0 +1,16 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only OR BSD-3-Clause */
> > +
> > +/*
> > + * Copyright 2024 NXP
> > + */
> > +
> > +#ifndef DT_BINDINGS_RESET_IMX8ULP_SIM_RESET_H
> > +#define DT_BINDINGS_RESET_IMX8ULP_SIM_RESET_H
> > +
> > +#define IMX8ULP_SIM_RESET_MIPI_DSI_RST_DPI_N 0
> > +#define IMX8ULP_SIM_RESET_MIPI_DSI_RST_ESC_N 1
> > +#define IMX8ULP_SIM_RESET_MIPI_DSI_RST_BYTE_N 2
> > +
> > +#define IMX8ULP_SIM_RESET_NUM 3
> > +
> > +#endif /* DT_BINDINGS_RESET_IMX8ULP_SIM_RESET_H */
>

2024-05-17 15:25:20

by Frank Li

[permalink] [raw]
Subject: Re: [PATCH 3/4] dt-bindings: mfd: add schema for 8ulp's SIM

On Thu, May 16, 2024 at 11:40:30PM +0300, Laurentiu Mihalcea wrote:
> From: Laurentiu Mihalcea <[email protected]>
>
> Add schema for i.MX8ULP's SIM.
>
> Signed-off-by: Liu Ying <[email protected]>
> Signed-off-by: Laurentiu Mihalcea <[email protected]>
> ---
> .../bindings/mfd/nxp,imx8ulp-sim.yaml | 71 +++++++++++++++++++
> 1 file changed, 71 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mfd/nxp,imx8ulp-sim.yaml
>
> diff --git a/Documentation/devicetree/bindings/mfd/nxp,imx8ulp-sim.yaml b/Documentation/devicetree/bindings/mfd/nxp,imx8ulp-sim.yaml
> new file mode 100644
> index 000000000000..fbb17c06e3c0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/nxp,imx8ulp-sim.yaml
> @@ -0,0 +1,71 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/nxp,imx8ulp-sim.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NXP i.MX8ULP System Integration Module
> +
> +maintainers:
> + - Liu Ying <[email protected]>
> +
> +description: |

Needn't "|"

> + 8ULP's SIM provides control and configuration options for
> + some of the chip's components.
> +
> +properties:
> + compatible:
> + items:
> + - enum:
> + - nxp,imx8ulp-avd-sim
> + - const: syscon
> +
> + reg:
> + maxItems: 1
> +
> + clocks:
> + maxItems: 1
> +
> + mux-controller:
> + $ref: ../mux/reg-mux.yaml
> +
> + reset-controller:
> + $ref: ../reset/nxp,imx8ulp-sim-reset.yaml

I think we directly use reset-ti-syscon.c

> +
> +required:
> + - compatible
> + - reg
> + - clocks
> +
> +allOf:
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: nxp,imx8ulp-avd-sim
> + then:
> + required:
> + - reset-controller
> + - mux-controller
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/clock/imx8ulp-clock.h>
> + syscon@2da50000 {
> + compatible = "nxp,imx8ulp-avd-sim", "syscon";
> + reg = <0x2da50000 0x38>;
> + clocks = <&pcc5 IMX8ULP_CLK_AVD_SIM>;
> +
> + mux-controller {
> + compatible = "mmio-mux";
> + #mux-control-cells = <1>;
> + mux-reg-masks = <0x8 0x00000200>;
> + };
> +
> + reset-controller {
> + compatible = "nxp,imx8ulp-avd-sim-reset";
> + #reset-cells = <1>;
> + };
> + };
> --
> 2.34.1
>

2024-05-17 19:59:52

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH 1/4] dt-bindings: reset: add schema for imx8ulp SIM reset

On Thu, May 16, 2024 at 11:40:28PM +0300, Laurentiu Mihalcea wrote:
> From: Laurentiu Mihalcea <[email protected]>
>
> Add schema for imx8ulp's SIM reset controller.
>
> Signed-off-by: Liu Ying <[email protected]>
> Signed-off-by: Laurentiu Mihalcea <[email protected]>
> ---
> .../bindings/reset/nxp,imx8ulp-sim-reset.yaml | 43 +++++++++++++++++++
> 1 file changed, 43 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/reset/nxp,imx8ulp-sim-reset.yaml
>
> diff --git a/Documentation/devicetree/bindings/reset/nxp,imx8ulp-sim-reset.yaml b/Documentation/devicetree/bindings/reset/nxp,imx8ulp-sim-reset.yaml
> new file mode 100644
> index 000000000000..ec9a5c73e83c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/reset/nxp,imx8ulp-sim-reset.yaml
> @@ -0,0 +1,43 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/reset/nxp,imx8ulp-sim-reset.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NXP i.MX8ULP System Integration Module Reset Controller
> +
> +maintainers:
> + - Liu Ying <[email protected]>
> +
> +description: |
> + Some instances of i.MX8ULP's SIM may offer control over the
> + reset of some components of a certain domain (e.g: AVD-SIM).
> + As far as the DT is concerned, this means that the reset
> + controller needs to be a child of the SIM node.
> +
> +properties:
> + compatible:
> + const: nxp,imx8ulp-avd-sim-reset
> +
> + '#reset-cells':
> + const: 1
> +
> +required:
> + - compatible
> + - '#reset-cells'
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/clock/imx8ulp-clock.h>
> + syscon@2da50000 {
> + compatible = "nxp,imx8ulp-avd-sim", "syscon";
> + reg = <0x2da50000 0x38>;
> + clocks = <&pcc5 IMX8ULP_CLK_AVD_SIM>;
> +
> + reset-controller {
> + compatible = "nxp,imx8ulp-avd-sim-reset";
> + #reset-cells = <1>;
> + };
> + };

Why do you need a child node here? No DT resources or anything for this
'sub-block'. Just put "#reset-cells" in the parent node.

(Note that examples for MFDs like this go in the MFD binding rather than
having incomplete examples here.)

Rob

2024-05-20 14:42:13

by Amit Singh Tomar

[permalink] [raw]
Subject: Re: [PATCH 2/4] reset: add driver for imx8ulp SIM reset controller


>
> ----------------------------------------------------------------------
> On Fri, May 17, 2024 at 11:51:30AM +0530, Amit Singh Tomar wrote:
>> Hi,
>>
>>> ----------------------------------------------------------------------
>>> From: Laurentiu Mihalcea <[email protected]>
>>>
>>> Certain components can be reset via the SIM module.
>>> Add reset controller driver for the SIM module to
>>> allow drivers for said components to control the
>>> reset signal(s).
>>>
>>> Signed-off-by: Liu Ying <[email protected]>
>>> Signed-off-by: Laurentiu Mihalcea <[email protected]>
>>> ---
>>> drivers/reset/Kconfig | 7 ++
>>> drivers/reset/Makefile | 1 +
>>> drivers/reset/reset-imx8ulp-sim.c | 98 +++++++++++++++++++
>>
>> Just out of curiosity, can't this be accomplished using a generic reset
>> driver?
>>
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__elixir.bootlin.com_linux_latest_source_drivers_reset_reset-2Dsimple.c&d=DwIBAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=V_GK7jRuCHDErm6txmgDK1-MbUihtnSQ3gPgB-A-JKU&m=MnEcoegPnKc-F7TNf8PB3icMf8uCGrxsh6mEDWgNq3YA-OugL6Y53LxytlBGcsKp&s=6mBPHYYYXoyyxPPbUeOiIpwR3CXdnui1W3nkUJQ76Vo&e=
>
> reset-simple have not use regmap. I think it can use ti's
> https://urldefense.proofpoint.com/v2/url?u=https-3A__elixir.bootlin.com_linux_latest_source_drivers_reset_reset-2Dti-2Dsyscon.c&d=DwIBAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=V_GK7jRuCHDErm6txmgDK1-MbUihtnSQ3gPgB-A-JKU&m=MnEcoegPnKc-F7TNf8PB3icMf8uCGrxsh6mEDWgNq3YA-OugL6Y53LxytlBGcsKp&s=D6tqLNYrUbsdG_gYg4G-ORlC9YKz4XRAlVeo1hyoYS4&e=
>
> Or should change ti to reset-simple-syscon? or add regmap support for
> reset-simple.c
>

Quickly looked into U-Boot (not sure if it's the right reference), and
it has a separate reset-syscon.c file:

https://elixir.bootlin.com/u-boot/latest/source/drivers/reset/reset-syscon.c

So, converting reset-ti-syscon.c to reset-syscon.c might be the way to go.

> Frank Li
>
>
>>
>>> include/dt-bindings/reset/imx8ulp-sim-reset.h | 16 +++
>>> 4 files changed, 122 insertions(+)
>>> create mode 100644 drivers/reset/reset-imx8ulp-sim.c
>>> create mode 100644 include/dt-bindings/reset/imx8ulp-sim-reset.h
>>>
>>> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
>>> index 85b27c42cf65..c1f4d9ebd0fd 100644
>>> --- a/drivers/reset/Kconfig
>>> +++ b/drivers/reset/Kconfig
>>> @@ -91,6 +91,13 @@ config RESET_IMX7
>>> help
>>> This enables the reset controller driver for i.MX7 SoCs.
>>> +config RESET_IMX8ULP_SIM
>>> + tristate "i.MX8ULP SIM Reset Driver"
>>> + depends on ARCH_MXC
>>> + help
>>> + This enables the SIM (System Integration Module) reset driver
>>> + for the i.MX8ULP SoC.
>>> +
>>> config RESET_INTEL_GW
>>> bool "Intel Reset Controller Driver"
>>> depends on X86 || COMPILE_TEST
>>> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
>>> index fd8b49fa46fc..f257d6a41f1e 100644
>>> --- a/drivers/reset/Makefile
>>> +++ b/drivers/reset/Makefile
>>> @@ -42,3 +42,4 @@ obj-$(CONFIG_RESET_UNIPHIER) += reset-uniphier.o
>>> obj-$(CONFIG_RESET_UNIPHIER_GLUE) += reset-uniphier-glue.o
>>> obj-$(CONFIG_RESET_ZYNQ) += reset-zynq.o
>>> obj-$(CONFIG_ARCH_ZYNQMP) += reset-zynqmp.o
>>> +obj-$(CONFIG_RESET_IMX8ULP_SIM) += reset-imx8ulp-sim.o
>>> diff --git a/drivers/reset/reset-imx8ulp-sim.c b/drivers/reset/reset-imx8ulp-sim.c
>>> new file mode 100644
>>> index 000000000000..27923b9cd454
>>> --- /dev/null
>>> +++ b/drivers/reset/reset-imx8ulp-sim.c
>>> @@ -0,0 +1,98 @@
>>> +// SPDX-License-Identifier: GPL-2.0+
>>> +
>>> +/*
>>> + * Copyright 2024 NXP
>>> + */
>>> +
>>> +#include <linux/mfd/syscon.h>
>>> +#include <linux/module.h>
>>> +#include <linux/of.h>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/regmap.h>
>>> +#include <linux/reset-controller.h>
>>> +#include <dt-bindings/reset/imx8ulp-sim-reset.h>
>>> +
>>> +#define AVD_SIM_SYSCTRL0 0x8
>>> +
>>> +struct imx8ulp_sim_reset {
>>> + struct reset_controller_dev rcdev;
>>> + struct regmap *regmap;
>>> +};
>>> +
>>> +static const u32 imx8ulp_sim_reset_bits[IMX8ULP_SIM_RESET_NUM] = {
>>> + [IMX8ULP_SIM_RESET_MIPI_DSI_RST_DPI_N] = BIT(3),
>>> + [IMX8ULP_SIM_RESET_MIPI_DSI_RST_ESC_N] = BIT(4),
>>> + [IMX8ULP_SIM_RESET_MIPI_DSI_RST_BYTE_N] = BIT(5),
>>> +};
>>> +
>>> +static inline struct imx8ulp_sim_reset *
>>> +to_imx8ulp_sim_reset(struct reset_controller_dev *rcdev)
>>> +{
>>> + return container_of(rcdev, struct imx8ulp_sim_reset, rcdev);
>>> +}
>>> +
>>> +static int imx8ulp_sim_reset_assert(struct reset_controller_dev *rcdev,
>>> + unsigned long id)
>>> +{
>>> + struct imx8ulp_sim_reset *simr = to_imx8ulp_sim_reset(rcdev);
>>> + const u32 bit = imx8ulp_sim_reset_bits[id];
>>> +
>>> + return regmap_update_bits(simr->regmap, AVD_SIM_SYSCTRL0, bit, 0);
>>> +}
>>> +
>>> +static int imx8ulp_sim_reset_deassert(struct reset_controller_dev *rcdev,
>>> + unsigned long id)
>>> +{
>>> + struct imx8ulp_sim_reset *simr = to_imx8ulp_sim_reset(rcdev);
>>> + const u32 bit = imx8ulp_sim_reset_bits[id];
>>> +
>>> + return regmap_update_bits(simr->regmap, AVD_SIM_SYSCTRL0, bit, bit);
>>> +}
>>> +
>>> +static const struct reset_control_ops imx8ulp_sim_reset_ops = {
>>> + .assert = imx8ulp_sim_reset_assert,
>>> + .deassert = imx8ulp_sim_reset_deassert,
>>> +};
>>> +
>>> +static const struct of_device_id imx8ulp_sim_reset_dt_ids[] = {
>>> + { .compatible = "nxp,imx8ulp-avd-sim-reset", },
>>> + { /* sentinel */ },
>>> +};
>>> +
>>> +static int imx8ulp_sim_reset_probe(struct platform_device *pdev)
>>> +{
>>> + struct device *dev = &pdev->dev;
>>> + struct imx8ulp_sim_reset *simr;
>>> + int ret;
>>> +
>>> + simr = devm_kzalloc(dev, sizeof(*simr), GFP_KERNEL);
>>> + if (!simr)
>>> + return -ENOMEM;
>>> +
>>> + simr->regmap = syscon_node_to_regmap(dev->of_node->parent);
>>> + if (IS_ERR(simr->regmap)) {
>>> + ret = PTR_ERR(simr->regmap);
>>> + dev_err(dev, "failed to get regmap: %d\n", ret);
>>> + return ret;
>>> + }
>>> +
>>> + simr->rcdev.owner = THIS_MODULE;
>>> + simr->rcdev.nr_resets = IMX8ULP_SIM_RESET_NUM;
>>> + simr->rcdev.ops = &imx8ulp_sim_reset_ops;
>>> + simr->rcdev.of_node = dev->of_node;
>>> +
>>> + return devm_reset_controller_register(dev, &simr->rcdev);
>>> +}
>>> +
>>> +static struct platform_driver imx8ulp_sim_reset_driver = {
>>> + .probe = imx8ulp_sim_reset_probe,
>>> + .driver = {
>>> + .name = KBUILD_MODNAME,
>>> + .of_match_table = imx8ulp_sim_reset_dt_ids,
>>> + },
>>> +};
>>> +module_platform_driver(imx8ulp_sim_reset_driver);
>>> +
>>> +MODULE_AUTHOR("Liu Ying <[email protected]>");
>>> +MODULE_DESCRIPTION("NXP i.MX8ULP System Integration Module Reset driver");
>>> +MODULE_LICENSE("GPL");
>>> diff --git a/include/dt-bindings/reset/imx8ulp-sim-reset.h b/include/dt-bindings/reset/imx8ulp-sim-reset.h
>>> new file mode 100644
>>> index 000000000000..a3cee0d60773
>>> --- /dev/null
>>> +++ b/include/dt-bindings/reset/imx8ulp-sim-reset.h
>>> @@ -0,0 +1,16 @@
>>> +/* SPDX-License-Identifier: GPL-2.0-only OR BSD-3-Clause */
>>> +
>>> +/*
>>> + * Copyright 2024 NXP
>>> + */
>>> +
>>> +#ifndef DT_BINDINGS_RESET_IMX8ULP_SIM_RESET_H
>>> +#define DT_BINDINGS_RESET_IMX8ULP_SIM_RESET_H
>>> +
>>> +#define IMX8ULP_SIM_RESET_MIPI_DSI_RST_DPI_N 0
>>> +#define IMX8ULP_SIM_RESET_MIPI_DSI_RST_ESC_N 1
>>> +#define IMX8ULP_SIM_RESET_MIPI_DSI_RST_BYTE_N 2
>>> +
>>> +#define IMX8ULP_SIM_RESET_NUM 3
>>> +
>>> +#endif /* DT_BINDINGS_RESET_IMX8ULP_SIM_RESET_H */
>>


2024-05-22 18:43:57

by Laurentiu Mihalcea

[permalink] [raw]
Subject: Re: [PATCH 1/4] dt-bindings: reset: add schema for imx8ulp SIM reset



On 5/17/2024 10:59 PM, Rob Herring wrote:
> On Thu, May 16, 2024 at 11:40:28PM +0300, Laurentiu Mihalcea wrote:
>> From: Laurentiu Mihalcea <[email protected]>
>>
>> Add schema for imx8ulp's SIM reset controller.
>>
>> Signed-off-by: Liu Ying <[email protected]>
>> Signed-off-by: Laurentiu Mihalcea <[email protected]>
>> ---
>> .../bindings/reset/nxp,imx8ulp-sim-reset.yaml | 43 +++++++++++++++++++
>> 1 file changed, 43 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/reset/nxp,imx8ulp-sim-reset.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/reset/nxp,imx8ulp-sim-reset.yaml b/Documentation/devicetree/bindings/reset/nxp,imx8ulp-sim-reset.yaml
>> new file mode 100644
>> index 000000000000..ec9a5c73e83c
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/reset/nxp,imx8ulp-sim-reset.yaml
>> @@ -0,0 +1,43 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/reset/nxp,imx8ulp-sim-reset.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: NXP i.MX8ULP System Integration Module Reset Controller
>> +
>> +maintainers:
>> + - Liu Ying <[email protected]>
>> +
>> +description: |
>> + Some instances of i.MX8ULP's SIM may offer control over the
>> + reset of some components of a certain domain (e.g: AVD-SIM).
>> + As far as the DT is concerned, this means that the reset
>> + controller needs to be a child of the SIM node.
>> +
>> +properties:
>> + compatible:
>> + const: nxp,imx8ulp-avd-sim-reset
>> +
>> + '#reset-cells':
>> + const: 1
>> +
>> +required:
>> + - compatible
>> + - '#reset-cells'
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> + - |
>> + #include <dt-bindings/clock/imx8ulp-clock.h>
>> + syscon@2da50000 {
>> + compatible = "nxp,imx8ulp-avd-sim", "syscon";
>> + reg = <0x2da50000 0x38>;
>> + clocks = <&pcc5 IMX8ULP_CLK_AVD_SIM>;
>> +
>> + reset-controller {
>> + compatible = "nxp,imx8ulp-avd-sim-reset";
>> + #reset-cells = <1>;
>> + };
>> + };
> Why do you need a child node here? No DT resources or anything for this
> 'sub-block'. Just put "#reset-cells" in the parent node.
You're right, we don't need the child here. In fact, the reset controller will never
get probed anyways since "simple-mfd" was removed from the compatible
list of the parent. Will remove it and turn the parent into a syscon + reset controller
in V2.
>
> (Note that examples for MFDs like this go in the MFD binding rather than
> having incomplete examples here.)
>
> Rob
Noted, thank you!

2024-05-22 19:20:24

by Laurentiu Mihalcea

[permalink] [raw]
Subject: Re: [PATCH 2/4] reset: add driver for imx8ulp SIM reset controller



On 5/20/2024 5:41 PM, Amit Singh Tomar wrote:
>
>>
>> ----------------------------------------------------------------------
>> On Fri, May 17, 2024 at 11:51:30AM +0530, Amit Singh Tomar wrote:
>>> Hi,
>>>
>>>> ----------------------------------------------------------------------
>>>> From: Laurentiu Mihalcea <[email protected]>
>>>>
>>>> Certain components can be reset via the SIM module.
>>>> Add reset controller driver for the SIM module to
>>>> allow drivers for said components to control the
>>>> reset signal(s).
>>>>
>>>> Signed-off-by: Liu Ying <[email protected]>
>>>> Signed-off-by: Laurentiu Mihalcea <[email protected]>
>>>> ---
>>>>    drivers/reset/Kconfig                         |  7 ++
>>>>    drivers/reset/Makefile                        |  1 +
>>>>    drivers/reset/reset-imx8ulp-sim.c             | 98 +++++++++++++++++++
>>>
>>> Just out of curiosity, can't this be accomplished using a generic reset
>>> driver?
>>>
>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__elixir.bootlin.com_linux_latest_source_drivers_reset_reset-2Dsimple.c&d=DwIBAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=V_GK7jRuCHDErm6txmgDK1-MbUihtnSQ3gPgB-A-JKU&m=MnEcoegPnKc-F7TNf8PB3icMf8uCGrxsh6mEDWgNq3YA-OugL6Y53LxytlBGcsKp&s=6mBPHYYYXoyyxPPbUeOiIpwR3CXdnui1W3nkUJQ76Vo&e=
>>
>> reset-simple have not use regmap. I think it can use ti's
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__elixir.bootlin.com_linux_latest_source_drivers_reset_reset-2Dti-2Dsyscon.c&d=DwIBAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=V_GK7jRuCHDErm6txmgDK1-MbUihtnSQ3gPgB-A-JKU&m=MnEcoegPnKc-F7TNf8PB3icMf8uCGrxsh6mEDWgNq3YA-OugL6Y53LxytlBGcsKp&s=D6tqLNYrUbsdG_gYg4G-ORlC9YKz4XRAlVeo1hyoYS4&e=
>>
>> Or should change ti to reset-simple-syscon?  or add regmap support for
>> reset-simple.c
>>
>
> Quickly looked into U-Boot (not sure if it's the right reference), and it has a separate reset-syscon.c file:
>
> https://elixir.bootlin.com/u-boot/latest/source/drivers/reset/reset-syscon.c
>
> So, converting reset-ti-syscon.c to reset-syscon.c might be the way to go.

A few comments here: current version of the patch is wrong because
the reset controller driver doesn't get probed. If my understanding
is correct this is because the syscon driver doesn't create devices
for its children (via of_platform_populate() or its variants).

I plan to fix this in V2 by transforming the syscon parent node into
a syscon + reset controller node (+ the of_platform_populate() call
in the driver for the mux controller probe).

In this case, I think TI's reset driver will no longer be an option.

>
>> Frank Li
>>
>>
>>>
>>>>    include/dt-bindings/reset/imx8ulp-sim-reset.h | 16 +++
>>>>    4 files changed, 122 insertions(+)
>>>>    create mode 100644 drivers/reset/reset-imx8ulp-sim.c
>>>>    create mode 100644 include/dt-bindings/reset/imx8ulp-sim-reset.h
>>>>
>>>> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
>>>> index 85b27c42cf65..c1f4d9ebd0fd 100644
>>>> --- a/drivers/reset/Kconfig
>>>> +++ b/drivers/reset/Kconfig
>>>> @@ -91,6 +91,13 @@ config RESET_IMX7
>>>>        help
>>>>          This enables the reset controller driver for i.MX7 SoCs.
>>>> +config RESET_IMX8ULP_SIM
>>>> +    tristate "i.MX8ULP SIM Reset Driver"
>>>> +    depends on ARCH_MXC
>>>> +    help
>>>> +      This enables the SIM (System Integration Module) reset driver
>>>> +      for the i.MX8ULP SoC.
>>>> +
>>>>    config RESET_INTEL_GW
>>>>        bool "Intel Reset Controller Driver"
>>>>        depends on X86 || COMPILE_TEST
>>>> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
>>>> index fd8b49fa46fc..f257d6a41f1e 100644
>>>> --- a/drivers/reset/Makefile
>>>> +++ b/drivers/reset/Makefile
>>>> @@ -42,3 +42,4 @@ obj-$(CONFIG_RESET_UNIPHIER) += reset-uniphier.o
>>>>    obj-$(CONFIG_RESET_UNIPHIER_GLUE) += reset-uniphier-glue.o
>>>>    obj-$(CONFIG_RESET_ZYNQ) += reset-zynq.o
>>>>    obj-$(CONFIG_ARCH_ZYNQMP) += reset-zynqmp.o
>>>> +obj-$(CONFIG_RESET_IMX8ULP_SIM) += reset-imx8ulp-sim.o
>>>> diff --git a/drivers/reset/reset-imx8ulp-sim.c b/drivers/reset/reset-imx8ulp-sim.c
>>>> new file mode 100644
>>>> index 000000000000..27923b9cd454
>>>> --- /dev/null
>>>> +++ b/drivers/reset/reset-imx8ulp-sim.c
>>>> @@ -0,0 +1,98 @@
>>>> +// SPDX-License-Identifier: GPL-2.0+
>>>> +
>>>> +/*
>>>> + * Copyright 2024 NXP
>>>> + */
>>>> +
>>>> +#include <linux/mfd/syscon.h>
>>>> +#include <linux/module.h>
>>>> +#include <linux/of.h>
>>>> +#include <linux/platform_device.h>
>>>> +#include <linux/regmap.h>
>>>> +#include <linux/reset-controller.h>
>>>> +#include <dt-bindings/reset/imx8ulp-sim-reset.h>
>>>> +
>>>> +#define AVD_SIM_SYSCTRL0        0x8
>>>> +
>>>> +struct imx8ulp_sim_reset {
>>>> +    struct reset_controller_dev     rcdev;
>>>> +    struct regmap                   *regmap;
>>>> +};
>>>> +
>>>> +static const u32 imx8ulp_sim_reset_bits[IMX8ULP_SIM_RESET_NUM] = {
>>>> +    [IMX8ULP_SIM_RESET_MIPI_DSI_RST_DPI_N] = BIT(3),
>>>> +    [IMX8ULP_SIM_RESET_MIPI_DSI_RST_ESC_N] = BIT(4),
>>>> +    [IMX8ULP_SIM_RESET_MIPI_DSI_RST_BYTE_N] = BIT(5),
>>>> +};
>>>> +
>>>> +static inline struct imx8ulp_sim_reset *
>>>> +to_imx8ulp_sim_reset(struct reset_controller_dev *rcdev)
>>>> +{
>>>> +    return container_of(rcdev, struct imx8ulp_sim_reset, rcdev);
>>>> +}
>>>> +
>>>> +static int imx8ulp_sim_reset_assert(struct reset_controller_dev *rcdev,
>>>> +                    unsigned long id)
>>>> +{
>>>> +    struct imx8ulp_sim_reset *simr = to_imx8ulp_sim_reset(rcdev);
>>>> +    const u32 bit = imx8ulp_sim_reset_bits[id];
>>>> +
>>>> +    return regmap_update_bits(simr->regmap, AVD_SIM_SYSCTRL0, bit, 0);
>>>> +}
>>>> +
>>>> +static int imx8ulp_sim_reset_deassert(struct reset_controller_dev *rcdev,
>>>> +                      unsigned long id)
>>>> +{
>>>> +    struct imx8ulp_sim_reset *simr = to_imx8ulp_sim_reset(rcdev);
>>>> +    const u32 bit = imx8ulp_sim_reset_bits[id];
>>>> +
>>>> +    return regmap_update_bits(simr->regmap, AVD_SIM_SYSCTRL0, bit, bit);
>>>> +}
>>>> +
>>>> +static const struct reset_control_ops imx8ulp_sim_reset_ops = {
>>>> +    .assert         = imx8ulp_sim_reset_assert,
>>>> +    .deassert       = imx8ulp_sim_reset_deassert,
>>>> +};
>>>> +
>>>> +static const struct of_device_id imx8ulp_sim_reset_dt_ids[] = {
>>>> +    { .compatible = "nxp,imx8ulp-avd-sim-reset", },
>>>> +    { /* sentinel */ },
>>>> +};
>>>> +
>>>> +static int imx8ulp_sim_reset_probe(struct platform_device *pdev)
>>>> +{
>>>> +    struct device *dev = &pdev->dev;
>>>> +    struct imx8ulp_sim_reset *simr;
>>>> +    int ret;
>>>> +
>>>> +    simr = devm_kzalloc(dev, sizeof(*simr), GFP_KERNEL);
>>>> +    if (!simr)
>>>> +        return -ENOMEM;
>>>> +
>>>> +    simr->regmap = syscon_node_to_regmap(dev->of_node->parent);
>>>> +    if (IS_ERR(simr->regmap)) {
>>>> +        ret = PTR_ERR(simr->regmap);
>>>> +        dev_err(dev, "failed to get regmap: %d\n", ret);
>>>> +        return ret;
>>>> +    }
>>>> +
>>>> +    simr->rcdev.owner = THIS_MODULE;
>>>> +    simr->rcdev.nr_resets = IMX8ULP_SIM_RESET_NUM;
>>>> +    simr->rcdev.ops = &imx8ulp_sim_reset_ops;
>>>> +    simr->rcdev.of_node = dev->of_node;
>>>> +
>>>> +    return devm_reset_controller_register(dev, &simr->rcdev);
>>>> +}
>>>> +
>>>> +static struct platform_driver imx8ulp_sim_reset_driver = {
>>>> +    .probe  = imx8ulp_sim_reset_probe,
>>>> +    .driver = {
>>>> +        .name           = KBUILD_MODNAME,
>>>> +        .of_match_table = imx8ulp_sim_reset_dt_ids,
>>>> +    },
>>>> +};
>>>> +module_platform_driver(imx8ulp_sim_reset_driver);
>>>> +
>>>> +MODULE_AUTHOR("Liu Ying <[email protected]>");
>>>> +MODULE_DESCRIPTION("NXP i.MX8ULP System Integration Module Reset driver");
>>>> +MODULE_LICENSE("GPL");
>>>> diff --git a/include/dt-bindings/reset/imx8ulp-sim-reset.h b/include/dt-bindings/reset/imx8ulp-sim-reset.h
>>>> new file mode 100644
>>>> index 000000000000..a3cee0d60773
>>>> --- /dev/null
>>>> +++ b/include/dt-bindings/reset/imx8ulp-sim-reset.h
>>>> @@ -0,0 +1,16 @@
>>>> +/* SPDX-License-Identifier: GPL-2.0-only OR BSD-3-Clause */
>>>> +
>>>> +/*
>>>> + * Copyright 2024 NXP
>>>> + */
>>>> +
>>>> +#ifndef DT_BINDINGS_RESET_IMX8ULP_SIM_RESET_H
>>>> +#define DT_BINDINGS_RESET_IMX8ULP_SIM_RESET_H
>>>> +
>>>> +#define IMX8ULP_SIM_RESET_MIPI_DSI_RST_DPI_N    0
>>>> +#define IMX8ULP_SIM_RESET_MIPI_DSI_RST_ESC_N    1
>>>> +#define IMX8ULP_SIM_RESET_MIPI_DSI_RST_BYTE_N   2
>>>> +
>>>> +#define IMX8ULP_SIM_RESET_NUM                   3
>>>> +
>>>> +#endif /* DT_BINDINGS_RESET_IMX8ULP_SIM_RESET_H */
>>>
>