* [PATCH v4 01/13] media: dt-bindings: Add binding doc for i.MX8QXP and i.MX8QM ISI
2025-04-08 21:52 [PATCH v4 00/13] media: imx8: add camera support Frank Li
@ 2025-04-08 21:52 ` Frank Li
2025-04-08 21:53 ` [PATCH v4 02/13] media: nxp: imx8-isi: Allow num_sources to be greater than num_sink Frank Li
` (12 subsequent siblings)
13 siblings, 0 replies; 30+ messages in thread
From: Frank Li @ 2025-04-08 21:52 UTC (permalink / raw)
To: Laurent Pinchart, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Frank Li,
Rui Miguel Silva, Martin Kepplinger, Purism Kernel Team
Cc: linux-media, devicetree, imx, linux-arm-kernel, linux-kernel,
Robert Chiras, Guoniu.zhou
Add binding documentation for i.MX8QXP and i.MX8QM ISI. The clock-names,
power-domains, and ports differ significantly from the existing
nxp,imx8-isi.yaml. Create a new file to avoid complex if-else branches.
Add new file to MAINTAINERS.
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
change from v3 to v4
- Add Rob's R-o-b
- change qxp clock/irq/power-domain to 6. (QXP C0 change 0, previous chip
have not production).
- fix register size to 0x80000 for qm, 0x60000 for qxp.
- fix qxp's irq number and clock
change from v2 to v3
- none
change from v1 to v2
- create new file for 8qm and 8qxp accroding rob's suggestion.
---
.../devicetree/bindings/media/fsl,imx8qm-isi.yaml | 117 +++++++++++++++++++++
.../devicetree/bindings/media/fsl,imx8qxp-isi.yaml | 106 +++++++++++++++++++
MAINTAINERS | 1 +
3 files changed, 224 insertions(+)
diff --git a/Documentation/devicetree/bindings/media/fsl,imx8qm-isi.yaml b/Documentation/devicetree/bindings/media/fsl,imx8qm-isi.yaml
new file mode 100644
index 0000000000000..93f527e223aff
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/fsl,imx8qm-isi.yaml
@@ -0,0 +1,117 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/fsl,imx8qm-isi.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: i.MX8QM Image Sensing Interface
+
+maintainers:
+ - Frank Li <Frank.Li@nxp.com>
+
+description:
+ The Image Sensing Interface (ISI) combines image processing pipelines with
+ DMA engines to process and capture frames originating from a variety of
+ sources. The inputs to the ISI go through Pixel Link interfaces, and their
+ number and nature is SoC-dependent. They cover both capture interfaces (MIPI
+ CSI-2 RX, HDMI RX, ...) and display engine outputs for writeback support.
+
+properties:
+ compatible:
+ enum:
+ - fsl,imx8qm-isi
+
+ reg:
+ maxItems: 1
+
+ clocks:
+ maxItems: 8
+
+ clock-names:
+ items:
+ - const: per0
+ - const: per1
+ - const: per2
+ - const: per3
+ - const: per4
+ - const: per5
+ - const: per6
+ - const: per7
+
+ interrupts:
+ maxItems: 8
+
+ power-domains:
+ maxItems: 8
+
+ ports:
+ $ref: /schemas/graph.yaml#/properties/ports
+ properties:
+ port@2:
+ $ref: /schemas/graph.yaml#/properties/port
+ description: MIPI CSI-2 RX 0
+ port@3:
+ $ref: /schemas/graph.yaml#/properties/port
+ description: MIPI CSI-2 RX 1
+ port@4:
+ $ref: /schemas/graph.yaml#/properties/port
+ description: HDMI RX
+
+required:
+ - compatible
+ - reg
+ - clocks
+ - clock-names
+ - interrupts
+ - power-domains
+ - ports
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/interrupt-controller/irq.h>
+ #include <dt-bindings/clock/imx8-clock.h>
+ #include <dt-bindings/clock/imx8-lpcg.h>
+ #include <dt-bindings/firmware/imx/rsrc.h>
+
+ image-controller@58100000 {
+ compatible = "fsl,imx8qm-isi";
+ reg = <0x58100000 0x80000>;
+ interrupts = <GIC_SPI 297 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 298 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 299 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 300 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 301 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 302 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 303 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 304 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&pdma0_lpcg IMX_LPCG_CLK_0>,
+ <&pdma1_lpcg IMX_LPCG_CLK_0>,
+ <&pdma2_lpcg IMX_LPCG_CLK_0>,
+ <&pdma3_lpcg IMX_LPCG_CLK_0>,
+ <&pdma4_lpcg IMX_LPCG_CLK_0>,
+ <&pdma5_lpcg IMX_LPCG_CLK_0>,
+ <&pdma6_lpcg IMX_LPCG_CLK_0>,
+ <&pdma7_lpcg IMX_LPCG_CLK_0>;
+ clock-names = "per0", "per1", "per2", "per3",
+ "per4", "per5", "per6", "per7";
+ power-domains = <&pd IMX_SC_R_ISI_CH0>, <&pd IMX_SC_R_ISI_CH1>,
+ <&pd IMX_SC_R_ISI_CH2>, <&pd IMX_SC_R_ISI_CH3>,
+ <&pd IMX_SC_R_ISI_CH4>, <&pd IMX_SC_R_ISI_CH5>,
+ <&pd IMX_SC_R_ISI_CH6>, <&pd IMX_SC_R_ISI_CH7>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@2 {
+ reg = <2>;
+ endpoint {
+ remote-endpoint = <&mipi_csi0_out>;
+ };
+ };
+ };
+ };
+...
diff --git a/Documentation/devicetree/bindings/media/fsl,imx8qxp-isi.yaml b/Documentation/devicetree/bindings/media/fsl,imx8qxp-isi.yaml
new file mode 100644
index 0000000000000..bb41996bd2e36
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/fsl,imx8qxp-isi.yaml
@@ -0,0 +1,106 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/fsl,imx8qxp-isi.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: i.MX8QXP Image Sensing Interface
+
+maintainers:
+ - Frank Li <Frank.Li@nxp.com>
+
+description:
+ The Image Sensing Interface (ISI) combines image processing pipelines with
+ DMA engines to process and capture frames originating from a variety of
+ sources. The inputs to the ISI go through Pixel Link interfaces, and their
+ number and nature is SoC-dependent. They cover both capture interfaces (MIPI
+ CSI-2 RX, HDMI RX, ...) and display engine outputs for writeback support.
+
+properties:
+ compatible:
+ enum:
+ - fsl,imx8qxp-isi
+
+ reg:
+ maxItems: 1
+
+ clocks:
+ maxItems: 6
+
+ clock-names:
+ items:
+ - const: per0
+ - const: per1
+ - const: per2
+ - const: per3
+ - const: per4
+ - const: per5
+
+ interrupts:
+ maxItems: 6
+
+ power-domains:
+ maxItems: 6
+
+ ports:
+ $ref: /schemas/graph.yaml#/properties/ports
+ properties:
+ port@2:
+ $ref: /schemas/graph.yaml#/properties/port
+ description: MIPI CSI-2 RX 0
+ port@6:
+ $ref: /schemas/graph.yaml#/properties/port
+ description: CSI-2 Parallel RX
+
+required:
+ - compatible
+ - reg
+ - clocks
+ - clock-names
+ - interrupts
+ - power-domains
+ - ports
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/interrupt-controller/irq.h>
+ #include <dt-bindings/clock/imx8-clock.h>
+ #include <dt-bindings/clock/imx8-lpcg.h>
+ #include <dt-bindings/firmware/imx/rsrc.h>
+
+ image-controller@58100000 {
+ compatible = "fsl,imx8qxp-isi";
+ reg = <0x58100000 0x60000>;
+ interrupts = <GIC_SPI 297 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 298 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 299 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 300 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 301 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 302 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&pdma0_lpcg IMX_LPCG_CLK_0>,
+ <&pdma1_lpcg IMX_LPCG_CLK_0>,
+ <&pdma2_lpcg IMX_LPCG_CLK_0>,
+ <&pdma3_lpcg IMX_LPCG_CLK_0>,
+ <&pdma4_lpcg IMX_LPCG_CLK_0>,
+ <&pdma5_lpcg IMX_LPCG_CLK_0>;
+ clock-names = "per0", "per1", "per2", "per3", "per4", "per5";
+ power-domains = <&pd IMX_SC_R_ISI_CH0>, <&pd IMX_SC_R_ISI_CH1>,
+ <&pd IMX_SC_R_ISI_CH2>, <&pd IMX_SC_R_ISI_CH3>,
+ <&pd IMX_SC_R_ISI_CH4>, <&pd IMX_SC_R_ISI_CH5>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@2 {
+ reg = <2>;
+ endpoint {
+ remote-endpoint = <&mipi_csi0_out>;
+ };
+ };
+ };
+ };
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 78467ad7a8fef..977f338bec04c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -17428,6 +17428,7 @@ NXP i.MX 8M ISI DRIVER
M: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
L: linux-media@vger.kernel.org
S: Maintained
+F: Documentation/devicetree/bindings/media/fsl,imx8*-isi.yaml
F: Documentation/devicetree/bindings/media/nxp,imx8-isi.yaml
F: drivers/media/platform/nxp/imx8-isi/
--
2.34.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH v4 02/13] media: nxp: imx8-isi: Allow num_sources to be greater than num_sink
2025-04-08 21:52 [PATCH v4 00/13] media: imx8: add camera support Frank Li
2025-04-08 21:52 ` [PATCH v4 01/13] media: dt-bindings: Add binding doc for i.MX8QXP and i.MX8QM ISI Frank Li
@ 2025-04-08 21:53 ` Frank Li
2025-04-08 21:53 ` [PATCH v4 03/13] media: nxp: imx8-isi: Remove unused offset in mxc_isi_reg and use BIT() macro for mask Frank Li
` (11 subsequent siblings)
13 siblings, 0 replies; 30+ messages in thread
From: Frank Li @ 2025-04-08 21:53 UTC (permalink / raw)
To: Laurent Pinchart, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Frank Li,
Rui Miguel Silva, Martin Kepplinger, Purism Kernel Team
Cc: linux-media, devicetree, imx, linux-arm-kernel, linux-kernel,
Robert Chiras, Guoniu.zhou
Allow num_sources (drvdata: num_channels) to be greater than num_sink
(drvdata: num_ports + 1).
ISI support stream multiplexing, such as differentiates multiple cameras
from a single 2-lane MIPI input, or duplicates input stream into multiple
outputs. So num_channels may be greater than num_ports at some platform.
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
change from v3 to v4
- use routing.num_routes = min(xbar->num_sinks - 1, xbar->num_sources)
- replace xbar->num_sinks - 1 with routing.num_routes
change from v1 to v3
- none
---
drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
index 93a55c97cd173..55454445359f4 100644
--- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
+++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
@@ -188,11 +188,12 @@ static int mxc_isi_crossbar_init_state(struct v4l2_subdev *sd,
* Create a 1:1 mapping between pixel link inputs and outputs to
* pipelines by default.
*/
- routes = kcalloc(xbar->num_sources, sizeof(*routes), GFP_KERNEL);
+ routing.num_routes = min(xbar->num_sinks - 1, xbar->num_sources);
+ routes = kcalloc(routing.num_routes, sizeof(*routes), GFP_KERNEL);
if (!routes)
return -ENOMEM;
- for (i = 0; i < xbar->num_sources; ++i) {
+ for (i = 0; i < routing.num_routes; ++i) {
struct v4l2_subdev_route *route = &routes[i];
route->sink_pad = i;
@@ -200,7 +201,6 @@ static int mxc_isi_crossbar_init_state(struct v4l2_subdev *sd,
route->flags = V4L2_SUBDEV_ROUTE_FL_ACTIVE;
}
- routing.num_routes = xbar->num_sources;
routing.routes = routes;
ret = __mxc_isi_crossbar_set_routing(sd, state, &routing);
@@ -453,7 +453,7 @@ int mxc_isi_crossbar_init(struct mxc_isi_dev *isi)
* the memory input.
*/
xbar->num_sinks = isi->pdata->num_ports + 1;
- xbar->num_sources = isi->pdata->num_ports;
+ xbar->num_sources = isi->pdata->num_channels;
num_pads = xbar->num_sinks + xbar->num_sources;
xbar->pads = kcalloc(num_pads, sizeof(*xbar->pads), GFP_KERNEL);
--
2.34.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH v4 03/13] media: nxp: imx8-isi: Remove unused offset in mxc_isi_reg and use BIT() macro for mask
2025-04-08 21:52 [PATCH v4 00/13] media: imx8: add camera support Frank Li
2025-04-08 21:52 ` [PATCH v4 01/13] media: dt-bindings: Add binding doc for i.MX8QXP and i.MX8QM ISI Frank Li
2025-04-08 21:53 ` [PATCH v4 02/13] media: nxp: imx8-isi: Allow num_sources to be greater than num_sink Frank Li
@ 2025-04-08 21:53 ` Frank Li
2025-04-21 21:06 ` Laurent Pinchart
2025-04-08 21:53 ` [PATCH v4 04/13] media: nxp: imx8-isi: Use devm_clk_bulk_get_all() to fetch clocks Frank Li
` (10 subsequent siblings)
13 siblings, 1 reply; 30+ messages in thread
From: Frank Li @ 2025-04-08 21:53 UTC (permalink / raw)
To: Laurent Pinchart, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Frank Li,
Rui Miguel Silva, Martin Kepplinger, Purism Kernel Team
Cc: linux-media, devicetree, imx, linux-arm-kernel, linux-kernel,
Robert Chiras, Guoniu.zhou
Preserve clarity by removing the unused 'offset' field in struct mxc_isi_reg,
as it duplicates information already indicated by the mask and remains unused.
Improve readability by replacing hex value masks with the BIT() macro.
No functional change.
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
.../media/platform/nxp/imx8-isi/imx8-isi-core.c | 25 +++++++++++-----------
.../media/platform/nxp/imx8-isi/imx8-isi-core.h | 1 -
2 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
index 1e79b1211b603..ecfc95882f903 100644
--- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
+++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
@@ -3,6 +3,7 @@
* Copyright 2019-2020 NXP
*/
+#include <linux/bits.h>
#include <linux/clk.h>
#include <linux/device.h>
#include <linux/errno.h>
@@ -247,24 +248,24 @@ static void mxc_isi_v4l2_cleanup(struct mxc_isi_dev *isi)
/* For i.MX8QXP C0 and i.MX8MN ISI IER version */
static const struct mxc_isi_ier_reg mxc_imx8_isi_ier_v1 = {
- .oflw_y_buf_en = { .offset = 19, .mask = 0x80000 },
- .oflw_u_buf_en = { .offset = 21, .mask = 0x200000 },
- .oflw_v_buf_en = { .offset = 23, .mask = 0x800000 },
+ .oflw_y_buf_en = { .mask = BIT(19) },
+ .oflw_u_buf_en = { .mask = BIT(21) },
+ .oflw_v_buf_en = { .mask = BIT(23) },
- .panic_y_buf_en = {.offset = 20, .mask = 0x100000 },
- .panic_u_buf_en = {.offset = 22, .mask = 0x400000 },
- .panic_v_buf_en = {.offset = 24, .mask = 0x1000000 },
+ .panic_y_buf_en = { .mask = BIT(20) },
+ .panic_u_buf_en = { .mask = BIT(22) },
+ .panic_v_buf_en = { .mask = BIT(24) },
};
/* For i.MX8MP ISI IER version */
static const struct mxc_isi_ier_reg mxc_imx8_isi_ier_v2 = {
- .oflw_y_buf_en = { .offset = 18, .mask = 0x40000 },
- .oflw_u_buf_en = { .offset = 20, .mask = 0x100000 },
- .oflw_v_buf_en = { .offset = 22, .mask = 0x400000 },
+ .oflw_y_buf_en = { .mask = BIT(18) },
+ .oflw_u_buf_en = { .mask = BIT(20) },
+ .oflw_v_buf_en = { .mask = BIT(22) },
- .panic_y_buf_en = {.offset = 19, .mask = 0x80000 },
- .panic_u_buf_en = {.offset = 21, .mask = 0x200000 },
- .panic_v_buf_en = {.offset = 23, .mask = 0x800000 },
+ .panic_y_buf_en = { .mask = BIT(19) },
+ .panic_u_buf_en = { .mask = BIT(21) },
+ .panic_v_buf_en = { .mask = BIT(23) },
};
/* Panic will assert when the buffers are 50% full */
diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
index 9c7fe9e5f941f..e7534a80af7b4 100644
--- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
+++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
@@ -114,7 +114,6 @@ struct mxc_isi_buffer {
};
struct mxc_isi_reg {
- u32 offset;
u32 mask;
};
--
2.34.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH v4 03/13] media: nxp: imx8-isi: Remove unused offset in mxc_isi_reg and use BIT() macro for mask
2025-04-08 21:53 ` [PATCH v4 03/13] media: nxp: imx8-isi: Remove unused offset in mxc_isi_reg and use BIT() macro for mask Frank Li
@ 2025-04-21 21:06 ` Laurent Pinchart
2025-05-02 0:53 ` Frank Li
0 siblings, 1 reply; 30+ messages in thread
From: Laurent Pinchart @ 2025-04-21 21:06 UTC (permalink / raw)
To: Frank Li
Cc: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Rui Miguel Silva, Martin Kepplinger,
Purism Kernel Team, linux-media, devicetree, imx,
linux-arm-kernel, linux-kernel, Robert Chiras, Guoniu.zhou
Hi Frank,
Thank you for the patch.
On Tue, Apr 08, 2025 at 05:53:01PM -0400, Frank Li wrote:
> Preserve clarity by removing the unused 'offset' field in struct mxc_isi_reg,
> as it duplicates information already indicated by the mask and remains unused.
The commit message line length limit is normally 72 characters. I can
reflow when applying if no other change to the series is needed.
>
> Improve readability by replacing hex value masks with the BIT() macro.
>
> No functional change.
>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> .../media/platform/nxp/imx8-isi/imx8-isi-core.c | 25 +++++++++++-----------
> .../media/platform/nxp/imx8-isi/imx8-isi-core.h | 1 -
> 2 files changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> index 1e79b1211b603..ecfc95882f903 100644
> --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> @@ -3,6 +3,7 @@
> * Copyright 2019-2020 NXP
> */
>
> +#include <linux/bits.h>
> #include <linux/clk.h>
> #include <linux/device.h>
> #include <linux/errno.h>
> @@ -247,24 +248,24 @@ static void mxc_isi_v4l2_cleanup(struct mxc_isi_dev *isi)
>
> /* For i.MX8QXP C0 and i.MX8MN ISI IER version */
> static const struct mxc_isi_ier_reg mxc_imx8_isi_ier_v1 = {
> - .oflw_y_buf_en = { .offset = 19, .mask = 0x80000 },
> - .oflw_u_buf_en = { .offset = 21, .mask = 0x200000 },
> - .oflw_v_buf_en = { .offset = 23, .mask = 0x800000 },
> + .oflw_y_buf_en = { .mask = BIT(19) },
> + .oflw_u_buf_en = { .mask = BIT(21) },
> + .oflw_v_buf_en = { .mask = BIT(23) },
>
> - .panic_y_buf_en = {.offset = 20, .mask = 0x100000 },
> - .panic_u_buf_en = {.offset = 22, .mask = 0x400000 },
> - .panic_v_buf_en = {.offset = 24, .mask = 0x1000000 },
> + .panic_y_buf_en = { .mask = BIT(20) },
> + .panic_u_buf_en = { .mask = BIT(22) },
> + .panic_v_buf_en = { .mask = BIT(24) },
> };
>
> /* For i.MX8MP ISI IER version */
> static const struct mxc_isi_ier_reg mxc_imx8_isi_ier_v2 = {
> - .oflw_y_buf_en = { .offset = 18, .mask = 0x40000 },
> - .oflw_u_buf_en = { .offset = 20, .mask = 0x100000 },
> - .oflw_v_buf_en = { .offset = 22, .mask = 0x400000 },
> + .oflw_y_buf_en = { .mask = BIT(18) },
> + .oflw_u_buf_en = { .mask = BIT(20) },
> + .oflw_v_buf_en = { .mask = BIT(22) },
>
> - .panic_y_buf_en = {.offset = 19, .mask = 0x80000 },
> - .panic_u_buf_en = {.offset = 21, .mask = 0x200000 },
> - .panic_v_buf_en = {.offset = 23, .mask = 0x800000 },
> + .panic_y_buf_en = { .mask = BIT(19) },
> + .panic_u_buf_en = { .mask = BIT(21) },
> + .panic_v_buf_en = { .mask = BIT(23) },
> };
>
> /* Panic will assert when the buffers are 50% full */
> diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
> index 9c7fe9e5f941f..e7534a80af7b4 100644
> --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
> +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
> @@ -114,7 +114,6 @@ struct mxc_isi_buffer {
> };
>
> struct mxc_isi_reg {
> - u32 offset;
> u32 mask;
> };
>
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v4 03/13] media: nxp: imx8-isi: Remove unused offset in mxc_isi_reg and use BIT() macro for mask
2025-04-21 21:06 ` Laurent Pinchart
@ 2025-05-02 0:53 ` Frank Li
2025-05-02 15:25 ` Laurent Pinchart
0 siblings, 1 reply; 30+ messages in thread
From: Frank Li @ 2025-05-02 0:53 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Rui Miguel Silva, Martin Kepplinger,
Purism Kernel Team, linux-media, devicetree, imx,
linux-arm-kernel, linux-kernel, Robert Chiras, Guoniu.zhou
On Tue, Apr 22, 2025 at 12:06:55AM +0300, Laurent Pinchart wrote:
> Hi Frank,
>
> Thank you for the patch.
>
> On Tue, Apr 08, 2025 at 05:53:01PM -0400, Frank Li wrote:
> > Preserve clarity by removing the unused 'offset' field in struct mxc_isi_reg,
> > as it duplicates information already indicated by the mask and remains unused.
>
> The commit message line length limit is normally 72 characters. I can
> reflow when applying if no other change to the series is needed.
I remember it is 75 chars. Any document show it is 72. I need correct for
the other patches also.
Frank
>
> >
> > Improve readability by replacing hex value masks with the BIT() macro.
> >
> > No functional change.
> >
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> > ---
> > .../media/platform/nxp/imx8-isi/imx8-isi-core.c | 25 +++++++++++-----------
> > .../media/platform/nxp/imx8-isi/imx8-isi-core.h | 1 -
> > 2 files changed, 13 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> > index 1e79b1211b603..ecfc95882f903 100644
> > --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> > +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> > @@ -3,6 +3,7 @@
> > * Copyright 2019-2020 NXP
> > */
> >
> > +#include <linux/bits.h>
> > #include <linux/clk.h>
> > #include <linux/device.h>
> > #include <linux/errno.h>
> > @@ -247,24 +248,24 @@ static void mxc_isi_v4l2_cleanup(struct mxc_isi_dev *isi)
> >
> > /* For i.MX8QXP C0 and i.MX8MN ISI IER version */
> > static const struct mxc_isi_ier_reg mxc_imx8_isi_ier_v1 = {
> > - .oflw_y_buf_en = { .offset = 19, .mask = 0x80000 },
> > - .oflw_u_buf_en = { .offset = 21, .mask = 0x200000 },
> > - .oflw_v_buf_en = { .offset = 23, .mask = 0x800000 },
> > + .oflw_y_buf_en = { .mask = BIT(19) },
> > + .oflw_u_buf_en = { .mask = BIT(21) },
> > + .oflw_v_buf_en = { .mask = BIT(23) },
> >
> > - .panic_y_buf_en = {.offset = 20, .mask = 0x100000 },
> > - .panic_u_buf_en = {.offset = 22, .mask = 0x400000 },
> > - .panic_v_buf_en = {.offset = 24, .mask = 0x1000000 },
> > + .panic_y_buf_en = { .mask = BIT(20) },
> > + .panic_u_buf_en = { .mask = BIT(22) },
> > + .panic_v_buf_en = { .mask = BIT(24) },
> > };
> >
> > /* For i.MX8MP ISI IER version */
> > static const struct mxc_isi_ier_reg mxc_imx8_isi_ier_v2 = {
> > - .oflw_y_buf_en = { .offset = 18, .mask = 0x40000 },
> > - .oflw_u_buf_en = { .offset = 20, .mask = 0x100000 },
> > - .oflw_v_buf_en = { .offset = 22, .mask = 0x400000 },
> > + .oflw_y_buf_en = { .mask = BIT(18) },
> > + .oflw_u_buf_en = { .mask = BIT(20) },
> > + .oflw_v_buf_en = { .mask = BIT(22) },
> >
> > - .panic_y_buf_en = {.offset = 19, .mask = 0x80000 },
> > - .panic_u_buf_en = {.offset = 21, .mask = 0x200000 },
> > - .panic_v_buf_en = {.offset = 23, .mask = 0x800000 },
> > + .panic_y_buf_en = { .mask = BIT(19) },
> > + .panic_u_buf_en = { .mask = BIT(21) },
> > + .panic_v_buf_en = { .mask = BIT(23) },
> > };
> >
> > /* Panic will assert when the buffers are 50% full */
> > diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
> > index 9c7fe9e5f941f..e7534a80af7b4 100644
> > --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
> > +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
> > @@ -114,7 +114,6 @@ struct mxc_isi_buffer {
> > };
> >
> > struct mxc_isi_reg {
> > - u32 offset;
> > u32 mask;
> > };
> >
>
> --
> Regards,
>
> Laurent Pinchart
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v4 03/13] media: nxp: imx8-isi: Remove unused offset in mxc_isi_reg and use BIT() macro for mask
2025-05-02 0:53 ` Frank Li
@ 2025-05-02 15:25 ` Laurent Pinchart
0 siblings, 0 replies; 30+ messages in thread
From: Laurent Pinchart @ 2025-05-02 15:25 UTC (permalink / raw)
To: Frank Li
Cc: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Rui Miguel Silva, Martin Kepplinger,
Purism Kernel Team, linux-media, devicetree, imx,
linux-arm-kernel, linux-kernel, Robert Chiras, Guoniu.zhou
On Thu, May 01, 2025 at 08:53:17PM -0400, Frank Li wrote:
> On Tue, Apr 22, 2025 at 12:06:55AM +0300, Laurent Pinchart wrote:
> > Hi Frank,
> >
> > Thank you for the patch.
> >
> > On Tue, Apr 08, 2025 at 05:53:01PM -0400, Frank Li wrote:
> > > Preserve clarity by removing the unused 'offset' field in struct mxc_isi_reg,
> > > as it duplicates information already indicated by the mask and remains unused.
> >
> > The commit message line length limit is normally 72 characters. I can
> > reflow when applying if no other change to the series is needed.
>
> I remember it is 75 chars. Any document show it is 72. I need correct for
> the other patches also.
You're absolutely right, I don't know where I got the 72 columns limit
from. Please ignore my comment.
> > >
> > > Improve readability by replacing hex value masks with the BIT() macro.
> > >
> > > No functional change.
> > >
> > > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> >
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >
> > > ---
> > > .../media/platform/nxp/imx8-isi/imx8-isi-core.c | 25 +++++++++++-----------
> > > .../media/platform/nxp/imx8-isi/imx8-isi-core.h | 1 -
> > > 2 files changed, 13 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> > > index 1e79b1211b603..ecfc95882f903 100644
> > > --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> > > +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> > > @@ -3,6 +3,7 @@
> > > * Copyright 2019-2020 NXP
> > > */
> > >
> > > +#include <linux/bits.h>
> > > #include <linux/clk.h>
> > > #include <linux/device.h>
> > > #include <linux/errno.h>
> > > @@ -247,24 +248,24 @@ static void mxc_isi_v4l2_cleanup(struct mxc_isi_dev *isi)
> > >
> > > /* For i.MX8QXP C0 and i.MX8MN ISI IER version */
> > > static const struct mxc_isi_ier_reg mxc_imx8_isi_ier_v1 = {
> > > - .oflw_y_buf_en = { .offset = 19, .mask = 0x80000 },
> > > - .oflw_u_buf_en = { .offset = 21, .mask = 0x200000 },
> > > - .oflw_v_buf_en = { .offset = 23, .mask = 0x800000 },
> > > + .oflw_y_buf_en = { .mask = BIT(19) },
> > > + .oflw_u_buf_en = { .mask = BIT(21) },
> > > + .oflw_v_buf_en = { .mask = BIT(23) },
> > >
> > > - .panic_y_buf_en = {.offset = 20, .mask = 0x100000 },
> > > - .panic_u_buf_en = {.offset = 22, .mask = 0x400000 },
> > > - .panic_v_buf_en = {.offset = 24, .mask = 0x1000000 },
> > > + .panic_y_buf_en = { .mask = BIT(20) },
> > > + .panic_u_buf_en = { .mask = BIT(22) },
> > > + .panic_v_buf_en = { .mask = BIT(24) },
> > > };
> > >
> > > /* For i.MX8MP ISI IER version */
> > > static const struct mxc_isi_ier_reg mxc_imx8_isi_ier_v2 = {
> > > - .oflw_y_buf_en = { .offset = 18, .mask = 0x40000 },
> > > - .oflw_u_buf_en = { .offset = 20, .mask = 0x100000 },
> > > - .oflw_v_buf_en = { .offset = 22, .mask = 0x400000 },
> > > + .oflw_y_buf_en = { .mask = BIT(18) },
> > > + .oflw_u_buf_en = { .mask = BIT(20) },
> > > + .oflw_v_buf_en = { .mask = BIT(22) },
> > >
> > > - .panic_y_buf_en = {.offset = 19, .mask = 0x80000 },
> > > - .panic_u_buf_en = {.offset = 21, .mask = 0x200000 },
> > > - .panic_v_buf_en = {.offset = 23, .mask = 0x800000 },
> > > + .panic_y_buf_en = { .mask = BIT(19) },
> > > + .panic_u_buf_en = { .mask = BIT(21) },
> > > + .panic_v_buf_en = { .mask = BIT(23) },
> > > };
> > >
> > > /* Panic will assert when the buffers are 50% full */
> > > diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
> > > index 9c7fe9e5f941f..e7534a80af7b4 100644
> > > --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
> > > +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
> > > @@ -114,7 +114,6 @@ struct mxc_isi_buffer {
> > > };
> > >
> > > struct mxc_isi_reg {
> > > - u32 offset;
> > > u32 mask;
> > > };
> > >
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v4 04/13] media: nxp: imx8-isi: Use devm_clk_bulk_get_all() to fetch clocks
2025-04-08 21:52 [PATCH v4 00/13] media: imx8: add camera support Frank Li
` (2 preceding siblings ...)
2025-04-08 21:53 ` [PATCH v4 03/13] media: nxp: imx8-isi: Remove unused offset in mxc_isi_reg and use BIT() macro for mask Frank Li
@ 2025-04-08 21:53 ` Frank Li
2025-04-21 21:14 ` Laurent Pinchart
2025-04-08 21:53 ` [PATCH v4 05/13] media: nxp: imx8-isi: Remove redundant check for dma_set_mask_and_coherent() Frank Li
` (9 subsequent siblings)
13 siblings, 1 reply; 30+ messages in thread
From: Frank Li @ 2025-04-08 21:53 UTC (permalink / raw)
To: Laurent Pinchart, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Frank Li,
Rui Miguel Silva, Martin Kepplinger, Purism Kernel Team
Cc: linux-media, devicetree, imx, linux-arm-kernel, linux-kernel,
Robert Chiras, Guoniu.zhou
Use devm_clk_bulk_get_all() helper to simplify clock handle code.
No functional changes intended.
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
.../media/platform/nxp/imx8-isi/imx8-isi-core.c | 46 +++-------------------
.../media/platform/nxp/imx8-isi/imx8-isi-core.h | 3 +-
2 files changed, 6 insertions(+), 43 deletions(-)
diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
index ecfc95882f903..015350c6f2784 100644
--- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
+++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
@@ -275,11 +275,6 @@ static const struct mxc_isi_set_thd mxc_imx8_isi_thd_v1 = {
.panic_set_thd_v = { .mask = 0xf0000, .offset = 16, .threshold = 0x7 },
};
-static const struct clk_bulk_data mxc_imx8mn_clks[] = {
- { .id = "axi" },
- { .id = "apb" },
-};
-
static const struct mxc_isi_plat_data mxc_imx8mn_data = {
.model = MXC_ISI_IMX8MN,
.num_ports = 1,
@@ -287,8 +282,6 @@ static const struct mxc_isi_plat_data mxc_imx8mn_data = {
.reg_offset = 0,
.ier_reg = &mxc_imx8_isi_ier_v1,
.set_thd = &mxc_imx8_isi_thd_v1,
- .clks = mxc_imx8mn_clks,
- .num_clks = ARRAY_SIZE(mxc_imx8mn_clks),
.buf_active_reverse = false,
.gasket_ops = &mxc_imx8_gasket_ops,
.has_36bit_dma = false,
@@ -301,8 +294,6 @@ static const struct mxc_isi_plat_data mxc_imx8mp_data = {
.reg_offset = 0x2000,
.ier_reg = &mxc_imx8_isi_ier_v2,
.set_thd = &mxc_imx8_isi_thd_v1,
- .clks = mxc_imx8mn_clks,
- .num_clks = ARRAY_SIZE(mxc_imx8mn_clks),
.buf_active_reverse = true,
.gasket_ops = &mxc_imx8_gasket_ops,
.has_36bit_dma = true,
@@ -315,8 +306,6 @@ static const struct mxc_isi_plat_data mxc_imx8ulp_data = {
.reg_offset = 0x0,
.ier_reg = &mxc_imx8_isi_ier_v2,
.set_thd = &mxc_imx8_isi_thd_v1,
- .clks = mxc_imx8mn_clks,
- .num_clks = ARRAY_SIZE(mxc_imx8mn_clks),
.buf_active_reverse = true,
.has_36bit_dma = false,
};
@@ -328,8 +317,6 @@ static const struct mxc_isi_plat_data mxc_imx93_data = {
.reg_offset = 0,
.ier_reg = &mxc_imx8_isi_ier_v2,
.set_thd = &mxc_imx8_isi_thd_v1,
- .clks = mxc_imx8mn_clks,
- .num_clks = ARRAY_SIZE(mxc_imx8mn_clks),
.buf_active_reverse = true,
.gasket_ops = &mxc_imx93_gasket_ops,
.has_36bit_dma = false,
@@ -386,7 +373,7 @@ static int mxc_isi_runtime_suspend(struct device *dev)
{
struct mxc_isi_dev *isi = dev_get_drvdata(dev);
- clk_bulk_disable_unprepare(isi->pdata->num_clks, isi->clks);
+ clk_bulk_disable_unprepare(isi->num_clks, isi->clks);
return 0;
}
@@ -396,7 +383,7 @@ static int mxc_isi_runtime_resume(struct device *dev)
struct mxc_isi_dev *isi = dev_get_drvdata(dev);
int ret;
- ret = clk_bulk_prepare_enable(isi->pdata->num_clks, isi->clks);
+ ret = clk_bulk_prepare_enable(isi->num_clks, isi->clks);
if (ret) {
dev_err(dev, "Failed to enable clocks (%d)\n", ret);
return ret;
@@ -414,27 +401,6 @@ static const struct dev_pm_ops mxc_isi_pm_ops = {
* Probe, remove & driver
*/
-static int mxc_isi_clk_get(struct mxc_isi_dev *isi)
-{
- unsigned int size = isi->pdata->num_clks
- * sizeof(*isi->clks);
- int ret;
-
- isi->clks = devm_kmemdup(isi->dev, isi->pdata->clks, size, GFP_KERNEL);
- if (!isi->clks)
- return -ENOMEM;
-
- ret = devm_clk_bulk_get(isi->dev, isi->pdata->num_clks,
- isi->clks);
- if (ret < 0) {
- dev_err(isi->dev, "Failed to acquire clocks: %d\n",
- ret);
- return ret;
- }
-
- return 0;
-}
-
static int mxc_isi_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
@@ -457,11 +423,9 @@ static int mxc_isi_probe(struct platform_device *pdev)
if (!isi->pipes)
return -ENOMEM;
- ret = mxc_isi_clk_get(isi);
- if (ret < 0) {
- dev_err(dev, "Failed to get clocks\n");
- return ret;
- }
+ isi->num_clks = devm_clk_bulk_get_all(dev, &isi->clks);
+ if (isi->num_clks < 0)
+ return dev_err_probe(dev, isi->num_clks, "Failed to get clocks\n");
isi->regs = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(isi->regs)) {
diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
index e7534a80af7b4..bd3cfe5fbe063 100644
--- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
+++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
@@ -169,8 +169,6 @@ struct mxc_isi_plat_data {
const struct mxc_isi_ier_reg *ier_reg;
const struct mxc_isi_set_thd *set_thd;
const struct mxc_gasket_ops *gasket_ops;
- const struct clk_bulk_data *clks;
- unsigned int num_clks;
bool buf_active_reverse;
bool has_36bit_dma;
};
@@ -282,6 +280,7 @@ struct mxc_isi_dev {
void __iomem *regs;
struct clk_bulk_data *clks;
+ int num_clks;
struct regmap *gasket;
struct mxc_isi_crossbar crossbar;
--
2.34.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH v4 04/13] media: nxp: imx8-isi: Use devm_clk_bulk_get_all() to fetch clocks
2025-04-08 21:53 ` [PATCH v4 04/13] media: nxp: imx8-isi: Use devm_clk_bulk_get_all() to fetch clocks Frank Li
@ 2025-04-21 21:14 ` Laurent Pinchart
2025-05-02 1:02 ` Frank Li
0 siblings, 1 reply; 30+ messages in thread
From: Laurent Pinchart @ 2025-04-21 21:14 UTC (permalink / raw)
To: Frank Li
Cc: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Rui Miguel Silva, Martin Kepplinger,
Purism Kernel Team, linux-media, devicetree, imx,
linux-arm-kernel, linux-kernel, Robert Chiras, Guoniu.zhou
Hi Frank,
Thank you for the patch.
On Tue, Apr 08, 2025 at 05:53:02PM -0400, Frank Li wrote:
> Use devm_clk_bulk_get_all() helper to simplify clock handle code.
>
> No functional changes intended.
>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> .../media/platform/nxp/imx8-isi/imx8-isi-core.c | 46 +++-------------------
> .../media/platform/nxp/imx8-isi/imx8-isi-core.h | 3 +-
> 2 files changed, 6 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> index ecfc95882f903..015350c6f2784 100644
> --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> @@ -275,11 +275,6 @@ static const struct mxc_isi_set_thd mxc_imx8_isi_thd_v1 = {
> .panic_set_thd_v = { .mask = 0xf0000, .offset = 16, .threshold = 0x7 },
> };
>
> -static const struct clk_bulk_data mxc_imx8mn_clks[] = {
> - { .id = "axi" },
> - { .id = "apb" },
> -};
> -
> static const struct mxc_isi_plat_data mxc_imx8mn_data = {
> .model = MXC_ISI_IMX8MN,
> .num_ports = 1,
> @@ -287,8 +282,6 @@ static const struct mxc_isi_plat_data mxc_imx8mn_data = {
> .reg_offset = 0,
> .ier_reg = &mxc_imx8_isi_ier_v1,
> .set_thd = &mxc_imx8_isi_thd_v1,
> - .clks = mxc_imx8mn_clks,
> - .num_clks = ARRAY_SIZE(mxc_imx8mn_clks),
> .buf_active_reverse = false,
> .gasket_ops = &mxc_imx8_gasket_ops,
> .has_36bit_dma = false,
> @@ -301,8 +294,6 @@ static const struct mxc_isi_plat_data mxc_imx8mp_data = {
> .reg_offset = 0x2000,
> .ier_reg = &mxc_imx8_isi_ier_v2,
> .set_thd = &mxc_imx8_isi_thd_v1,
> - .clks = mxc_imx8mn_clks,
> - .num_clks = ARRAY_SIZE(mxc_imx8mn_clks),
> .buf_active_reverse = true,
> .gasket_ops = &mxc_imx8_gasket_ops,
> .has_36bit_dma = true,
> @@ -315,8 +306,6 @@ static const struct mxc_isi_plat_data mxc_imx8ulp_data = {
> .reg_offset = 0x0,
> .ier_reg = &mxc_imx8_isi_ier_v2,
> .set_thd = &mxc_imx8_isi_thd_v1,
> - .clks = mxc_imx8mn_clks,
> - .num_clks = ARRAY_SIZE(mxc_imx8mn_clks),
> .buf_active_reverse = true,
> .has_36bit_dma = false,
> };
> @@ -328,8 +317,6 @@ static const struct mxc_isi_plat_data mxc_imx93_data = {
> .reg_offset = 0,
> .ier_reg = &mxc_imx8_isi_ier_v2,
> .set_thd = &mxc_imx8_isi_thd_v1,
> - .clks = mxc_imx8mn_clks,
> - .num_clks = ARRAY_SIZE(mxc_imx8mn_clks),
> .buf_active_reverse = true,
> .gasket_ops = &mxc_imx93_gasket_ops,
> .has_36bit_dma = false,
> @@ -386,7 +373,7 @@ static int mxc_isi_runtime_suspend(struct device *dev)
> {
> struct mxc_isi_dev *isi = dev_get_drvdata(dev);
>
> - clk_bulk_disable_unprepare(isi->pdata->num_clks, isi->clks);
> + clk_bulk_disable_unprepare(isi->num_clks, isi->clks);
>
> return 0;
> }
> @@ -396,7 +383,7 @@ static int mxc_isi_runtime_resume(struct device *dev)
> struct mxc_isi_dev *isi = dev_get_drvdata(dev);
> int ret;
>
> - ret = clk_bulk_prepare_enable(isi->pdata->num_clks, isi->clks);
> + ret = clk_bulk_prepare_enable(isi->num_clks, isi->clks);
> if (ret) {
> dev_err(dev, "Failed to enable clocks (%d)\n", ret);
> return ret;
> @@ -414,27 +401,6 @@ static const struct dev_pm_ops mxc_isi_pm_ops = {
> * Probe, remove & driver
> */
>
> -static int mxc_isi_clk_get(struct mxc_isi_dev *isi)
> -{
> - unsigned int size = isi->pdata->num_clks
> - * sizeof(*isi->clks);
> - int ret;
> -
> - isi->clks = devm_kmemdup(isi->dev, isi->pdata->clks, size, GFP_KERNEL);
> - if (!isi->clks)
> - return -ENOMEM;
> -
> - ret = devm_clk_bulk_get(isi->dev, isi->pdata->num_clks,
> - isi->clks);
> - if (ret < 0) {
> - dev_err(isi->dev, "Failed to acquire clocks: %d\n",
> - ret);
> - return ret;
> - }
> -
> - return 0;
> -}
> -
> static int mxc_isi_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> @@ -457,11 +423,9 @@ static int mxc_isi_probe(struct platform_device *pdev)
> if (!isi->pipes)
> return -ENOMEM;
>
> - ret = mxc_isi_clk_get(isi);
> - if (ret < 0) {
> - dev_err(dev, "Failed to get clocks\n");
> - return ret;
> - }
> + isi->num_clks = devm_clk_bulk_get_all(dev, &isi->clks);
This prevents validating that the DT contains the expected clocks, which
could cause hard to debug issues. Isn't it a problem ?
> + if (isi->num_clks < 0)
> + return dev_err_probe(dev, isi->num_clks, "Failed to get clocks\n");
>
> isi->regs = devm_platform_ioremap_resource(pdev, 0);
> if (IS_ERR(isi->regs)) {
> diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
> index e7534a80af7b4..bd3cfe5fbe063 100644
> --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
> +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
> @@ -169,8 +169,6 @@ struct mxc_isi_plat_data {
> const struct mxc_isi_ier_reg *ier_reg;
> const struct mxc_isi_set_thd *set_thd;
> const struct mxc_gasket_ops *gasket_ops;
> - const struct clk_bulk_data *clks;
> - unsigned int num_clks;
> bool buf_active_reverse;
> bool has_36bit_dma;
> };
> @@ -282,6 +280,7 @@ struct mxc_isi_dev {
>
> void __iomem *regs;
> struct clk_bulk_data *clks;
> + int num_clks;
> struct regmap *gasket;
>
> struct mxc_isi_crossbar crossbar;
>
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v4 04/13] media: nxp: imx8-isi: Use devm_clk_bulk_get_all() to fetch clocks
2025-04-21 21:14 ` Laurent Pinchart
@ 2025-05-02 1:02 ` Frank Li
2025-05-02 15:57 ` Laurent Pinchart
0 siblings, 1 reply; 30+ messages in thread
From: Frank Li @ 2025-05-02 1:02 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Rui Miguel Silva, Martin Kepplinger,
Purism Kernel Team, linux-media, devicetree, imx,
linux-arm-kernel, linux-kernel, Robert Chiras, Guoniu.zhou
On Tue, Apr 22, 2025 at 12:14:38AM +0300, Laurent Pinchart wrote:
> Hi Frank,
>
> Thank you for the patch.
>
> On Tue, Apr 08, 2025 at 05:53:02PM -0400, Frank Li wrote:
> > Use devm_clk_bulk_get_all() helper to simplify clock handle code.
> >
> > No functional changes intended.
> >
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > ---
> > .../media/platform/nxp/imx8-isi/imx8-isi-core.c | 46 +++-------------------
> > .../media/platform/nxp/imx8-isi/imx8-isi-core.h | 3 +-
> > 2 files changed, 6 insertions(+), 43 deletions(-)
> >
> > diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> > index ecfc95882f903..015350c6f2784 100644
> > --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> > +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> > @@ -275,11 +275,6 @@ static const struct mxc_isi_set_thd mxc_imx8_isi_thd_v1 = {
> > .panic_set_thd_v = { .mask = 0xf0000, .offset = 16, .threshold = 0x7 },
> > };
> >
> > -static const struct clk_bulk_data mxc_imx8mn_clks[] = {
> > - { .id = "axi" },
> > - { .id = "apb" },
> > -};
> > -
> > static const struct mxc_isi_plat_data mxc_imx8mn_data = {
> > .model = MXC_ISI_IMX8MN,
> > .num_ports = 1,
> > @@ -287,8 +282,6 @@ static const struct mxc_isi_plat_data mxc_imx8mn_data = {
> > .reg_offset = 0,
> > .ier_reg = &mxc_imx8_isi_ier_v1,
> > .set_thd = &mxc_imx8_isi_thd_v1,
> > - .clks = mxc_imx8mn_clks,
> > - .num_clks = ARRAY_SIZE(mxc_imx8mn_clks),
> > .buf_active_reverse = false,
> > .gasket_ops = &mxc_imx8_gasket_ops,
> > .has_36bit_dma = false,
> > @@ -301,8 +294,6 @@ static const struct mxc_isi_plat_data mxc_imx8mp_data = {
> > .reg_offset = 0x2000,
> > .ier_reg = &mxc_imx8_isi_ier_v2,
> > .set_thd = &mxc_imx8_isi_thd_v1,
> > - .clks = mxc_imx8mn_clks,
> > - .num_clks = ARRAY_SIZE(mxc_imx8mn_clks),
> > .buf_active_reverse = true,
> > .gasket_ops = &mxc_imx8_gasket_ops,
> > .has_36bit_dma = true,
> > @@ -315,8 +306,6 @@ static const struct mxc_isi_plat_data mxc_imx8ulp_data = {
> > .reg_offset = 0x0,
> > .ier_reg = &mxc_imx8_isi_ier_v2,
> > .set_thd = &mxc_imx8_isi_thd_v1,
> > - .clks = mxc_imx8mn_clks,
> > - .num_clks = ARRAY_SIZE(mxc_imx8mn_clks),
> > .buf_active_reverse = true,
> > .has_36bit_dma = false,
> > };
> > @@ -328,8 +317,6 @@ static const struct mxc_isi_plat_data mxc_imx93_data = {
> > .reg_offset = 0,
> > .ier_reg = &mxc_imx8_isi_ier_v2,
> > .set_thd = &mxc_imx8_isi_thd_v1,
> > - .clks = mxc_imx8mn_clks,
> > - .num_clks = ARRAY_SIZE(mxc_imx8mn_clks),
> > .buf_active_reverse = true,
> > .gasket_ops = &mxc_imx93_gasket_ops,
> > .has_36bit_dma = false,
> > @@ -386,7 +373,7 @@ static int mxc_isi_runtime_suspend(struct device *dev)
> > {
> > struct mxc_isi_dev *isi = dev_get_drvdata(dev);
> >
> > - clk_bulk_disable_unprepare(isi->pdata->num_clks, isi->clks);
> > + clk_bulk_disable_unprepare(isi->num_clks, isi->clks);
> >
> > return 0;
> > }
> > @@ -396,7 +383,7 @@ static int mxc_isi_runtime_resume(struct device *dev)
> > struct mxc_isi_dev *isi = dev_get_drvdata(dev);
> > int ret;
> >
> > - ret = clk_bulk_prepare_enable(isi->pdata->num_clks, isi->clks);
> > + ret = clk_bulk_prepare_enable(isi->num_clks, isi->clks);
> > if (ret) {
> > dev_err(dev, "Failed to enable clocks (%d)\n", ret);
> > return ret;
> > @@ -414,27 +401,6 @@ static const struct dev_pm_ops mxc_isi_pm_ops = {
> > * Probe, remove & driver
> > */
> >
> > -static int mxc_isi_clk_get(struct mxc_isi_dev *isi)
> > -{
> > - unsigned int size = isi->pdata->num_clks
> > - * sizeof(*isi->clks);
> > - int ret;
> > -
> > - isi->clks = devm_kmemdup(isi->dev, isi->pdata->clks, size, GFP_KERNEL);
> > - if (!isi->clks)
> > - return -ENOMEM;
> > -
> > - ret = devm_clk_bulk_get(isi->dev, isi->pdata->num_clks,
> > - isi->clks);
> > - if (ret < 0) {
> > - dev_err(isi->dev, "Failed to acquire clocks: %d\n",
> > - ret);
> > - return ret;
> > - }
> > -
> > - return 0;
> > -}
> > -
> > static int mxc_isi_probe(struct platform_device *pdev)
> > {
> > struct device *dev = &pdev->dev;
> > @@ -457,11 +423,9 @@ static int mxc_isi_probe(struct platform_device *pdev)
> > if (!isi->pipes)
> > return -ENOMEM;
> >
> > - ret = mxc_isi_clk_get(isi);
> > - if (ret < 0) {
> > - dev_err(dev, "Failed to get clocks\n");
> > - return ret;
> > - }
> > + isi->num_clks = devm_clk_bulk_get_all(dev, &isi->clks);
>
> This prevents validating that the DT contains the expected clocks, which
> could cause hard to debug issues. Isn't it a problem ?
It is checked by dt-binding. Now no warning by DTB_CHECK under arm64 freecale.
CHECK_DTB should be enough to find expected clocks.
Frank Li
>
> > + if (isi->num_clks < 0)
> > + return dev_err_probe(dev, isi->num_clks, "Failed to get clocks\n");
> >
> > isi->regs = devm_platform_ioremap_resource(pdev, 0);
> > if (IS_ERR(isi->regs)) {
> > diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
> > index e7534a80af7b4..bd3cfe5fbe063 100644
> > --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
> > +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
> > @@ -169,8 +169,6 @@ struct mxc_isi_plat_data {
> > const struct mxc_isi_ier_reg *ier_reg;
> > const struct mxc_isi_set_thd *set_thd;
> > const struct mxc_gasket_ops *gasket_ops;
> > - const struct clk_bulk_data *clks;
> > - unsigned int num_clks;
> > bool buf_active_reverse;
> > bool has_36bit_dma;
> > };
> > @@ -282,6 +280,7 @@ struct mxc_isi_dev {
> >
> > void __iomem *regs;
> > struct clk_bulk_data *clks;
> > + int num_clks;
> > struct regmap *gasket;
> >
> > struct mxc_isi_crossbar crossbar;
> >
>
> --
> Regards,
>
> Laurent Pinchart
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v4 04/13] media: nxp: imx8-isi: Use devm_clk_bulk_get_all() to fetch clocks
2025-05-02 1:02 ` Frank Li
@ 2025-05-02 15:57 ` Laurent Pinchart
2025-05-08 12:35 ` Frank Li
0 siblings, 1 reply; 30+ messages in thread
From: Laurent Pinchart @ 2025-05-02 15:57 UTC (permalink / raw)
To: Frank Li
Cc: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Rui Miguel Silva, Martin Kepplinger,
Purism Kernel Team, linux-media, devicetree, imx,
linux-arm-kernel, linux-kernel, Robert Chiras, Guoniu.zhou
Hi Frank,
On Thu, May 01, 2025 at 09:02:04PM -0400, Frank Li wrote:
> On Tue, Apr 22, 2025 at 12:14:38AM +0300, Laurent Pinchart wrote:
> > On Tue, Apr 08, 2025 at 05:53:02PM -0400, Frank Li wrote:
> > > Use devm_clk_bulk_get_all() helper to simplify clock handle code.
> > >
> > > No functional changes intended.
> > >
> > > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > > ---
> > > .../media/platform/nxp/imx8-isi/imx8-isi-core.c | 46 +++-------------------
> > > .../media/platform/nxp/imx8-isi/imx8-isi-core.h | 3 +-
> > > 2 files changed, 6 insertions(+), 43 deletions(-)
> > >
> > > diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> > > index ecfc95882f903..015350c6f2784 100644
> > > --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> > > +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> > > @@ -275,11 +275,6 @@ static const struct mxc_isi_set_thd mxc_imx8_isi_thd_v1 = {
> > > .panic_set_thd_v = { .mask = 0xf0000, .offset = 16, .threshold = 0x7 },
> > > };
> > >
> > > -static const struct clk_bulk_data mxc_imx8mn_clks[] = {
> > > - { .id = "axi" },
> > > - { .id = "apb" },
> > > -};
> > > -
> > > static const struct mxc_isi_plat_data mxc_imx8mn_data = {
> > > .model = MXC_ISI_IMX8MN,
> > > .num_ports = 1,
> > > @@ -287,8 +282,6 @@ static const struct mxc_isi_plat_data mxc_imx8mn_data = {
> > > .reg_offset = 0,
> > > .ier_reg = &mxc_imx8_isi_ier_v1,
> > > .set_thd = &mxc_imx8_isi_thd_v1,
> > > - .clks = mxc_imx8mn_clks,
> > > - .num_clks = ARRAY_SIZE(mxc_imx8mn_clks),
> > > .buf_active_reverse = false,
> > > .gasket_ops = &mxc_imx8_gasket_ops,
> > > .has_36bit_dma = false,
> > > @@ -301,8 +294,6 @@ static const struct mxc_isi_plat_data mxc_imx8mp_data = {
> > > .reg_offset = 0x2000,
> > > .ier_reg = &mxc_imx8_isi_ier_v2,
> > > .set_thd = &mxc_imx8_isi_thd_v1,
> > > - .clks = mxc_imx8mn_clks,
> > > - .num_clks = ARRAY_SIZE(mxc_imx8mn_clks),
> > > .buf_active_reverse = true,
> > > .gasket_ops = &mxc_imx8_gasket_ops,
> > > .has_36bit_dma = true,
> > > @@ -315,8 +306,6 @@ static const struct mxc_isi_plat_data mxc_imx8ulp_data = {
> > > .reg_offset = 0x0,
> > > .ier_reg = &mxc_imx8_isi_ier_v2,
> > > .set_thd = &mxc_imx8_isi_thd_v1,
> > > - .clks = mxc_imx8mn_clks,
> > > - .num_clks = ARRAY_SIZE(mxc_imx8mn_clks),
> > > .buf_active_reverse = true,
> > > .has_36bit_dma = false,
> > > };
> > > @@ -328,8 +317,6 @@ static const struct mxc_isi_plat_data mxc_imx93_data = {
> > > .reg_offset = 0,
> > > .ier_reg = &mxc_imx8_isi_ier_v2,
> > > .set_thd = &mxc_imx8_isi_thd_v1,
> > > - .clks = mxc_imx8mn_clks,
> > > - .num_clks = ARRAY_SIZE(mxc_imx8mn_clks),
> > > .buf_active_reverse = true,
> > > .gasket_ops = &mxc_imx93_gasket_ops,
> > > .has_36bit_dma = false,
> > > @@ -386,7 +373,7 @@ static int mxc_isi_runtime_suspend(struct device *dev)
> > > {
> > > struct mxc_isi_dev *isi = dev_get_drvdata(dev);
> > >
> > > - clk_bulk_disable_unprepare(isi->pdata->num_clks, isi->clks);
> > > + clk_bulk_disable_unprepare(isi->num_clks, isi->clks);
> > >
> > > return 0;
> > > }
> > > @@ -396,7 +383,7 @@ static int mxc_isi_runtime_resume(struct device *dev)
> > > struct mxc_isi_dev *isi = dev_get_drvdata(dev);
> > > int ret;
> > >
> > > - ret = clk_bulk_prepare_enable(isi->pdata->num_clks, isi->clks);
> > > + ret = clk_bulk_prepare_enable(isi->num_clks, isi->clks);
> > > if (ret) {
> > > dev_err(dev, "Failed to enable clocks (%d)\n", ret);
> > > return ret;
> > > @@ -414,27 +401,6 @@ static const struct dev_pm_ops mxc_isi_pm_ops = {
> > > * Probe, remove & driver
> > > */
> > >
> > > -static int mxc_isi_clk_get(struct mxc_isi_dev *isi)
> > > -{
> > > - unsigned int size = isi->pdata->num_clks
> > > - * sizeof(*isi->clks);
> > > - int ret;
> > > -
> > > - isi->clks = devm_kmemdup(isi->dev, isi->pdata->clks, size, GFP_KERNEL);
> > > - if (!isi->clks)
> > > - return -ENOMEM;
> > > -
> > > - ret = devm_clk_bulk_get(isi->dev, isi->pdata->num_clks,
> > > - isi->clks);
> > > - if (ret < 0) {
> > > - dev_err(isi->dev, "Failed to acquire clocks: %d\n",
> > > - ret);
> > > - return ret;
> > > - }
> > > -
> > > - return 0;
> > > -}
> > > -
> > > static int mxc_isi_probe(struct platform_device *pdev)
> > > {
> > > struct device *dev = &pdev->dev;
> > > @@ -457,11 +423,9 @@ static int mxc_isi_probe(struct platform_device *pdev)
> > > if (!isi->pipes)
> > > return -ENOMEM;
> > >
> > > - ret = mxc_isi_clk_get(isi);
> > > - if (ret < 0) {
> > > - dev_err(dev, "Failed to get clocks\n");
> > > - return ret;
> > > - }
> > > + isi->num_clks = devm_clk_bulk_get_all(dev, &isi->clks);
> >
> > This prevents validating that the DT contains the expected clocks, which
> > could cause hard to debug issues. Isn't it a problem ?
>
> It is checked by dt-binding. Now no warning by DTB_CHECK under arm64 freecale.
> CHECK_DTB should be enough to find expected clocks.
Yes, the DTB check will catch issues at build time, but the driver will
not enforce that. I'm not sure if there's a clear policy here, and if
ensuring at runtime in drivers that the expected clocks are present is
considered as a good practice by the DT maintainers. Rob, Krzysztof,
Conor, do you have an opinion ?
> > > + if (isi->num_clks < 0)
> > > + return dev_err_probe(dev, isi->num_clks, "Failed to get clocks\n");
> > >
> > > isi->regs = devm_platform_ioremap_resource(pdev, 0);
> > > if (IS_ERR(isi->regs)) {
> > > diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
> > > index e7534a80af7b4..bd3cfe5fbe063 100644
> > > --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
> > > +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
> > > @@ -169,8 +169,6 @@ struct mxc_isi_plat_data {
> > > const struct mxc_isi_ier_reg *ier_reg;
> > > const struct mxc_isi_set_thd *set_thd;
> > > const struct mxc_gasket_ops *gasket_ops;
> > > - const struct clk_bulk_data *clks;
> > > - unsigned int num_clks;
> > > bool buf_active_reverse;
> > > bool has_36bit_dma;
> > > };
> > > @@ -282,6 +280,7 @@ struct mxc_isi_dev {
> > >
> > > void __iomem *regs;
> > > struct clk_bulk_data *clks;
> > > + int num_clks;
> > > struct regmap *gasket;
> > >
> > > struct mxc_isi_crossbar crossbar;
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v4 04/13] media: nxp: imx8-isi: Use devm_clk_bulk_get_all() to fetch clocks
2025-05-02 15:57 ` Laurent Pinchart
@ 2025-05-08 12:35 ` Frank Li
2025-06-11 14:14 ` Laurent Pinchart
0 siblings, 1 reply; 30+ messages in thread
From: Frank Li @ 2025-05-08 12:35 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Rui Miguel Silva, Martin Kepplinger,
Purism Kernel Team, linux-media, devicetree, imx,
linux-arm-kernel, linux-kernel, Robert Chiras, Guoniu.zhou
On Fri, May 02, 2025 at 06:57:47PM +0300, Laurent Pinchart wrote:
> Hi Frank,
>
> On Thu, May 01, 2025 at 09:02:04PM -0400, Frank Li wrote:
> > On Tue, Apr 22, 2025 at 12:14:38AM +0300, Laurent Pinchart wrote:
> > > On Tue, Apr 08, 2025 at 05:53:02PM -0400, Frank Li wrote:
> > > > Use devm_clk_bulk_get_all() helper to simplify clock handle code.
> > > >
> > > > No functional changes intended.
> > > >
> > > > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > > > ---
> > > > .../media/platform/nxp/imx8-isi/imx8-isi-core.c | 46 +++-------------------
> > > > .../media/platform/nxp/imx8-isi/imx8-isi-core.h | 3 +-
> > > > 2 files changed, 6 insertions(+), 43 deletions(-)
> > > >
> > > > diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> > > > index ecfc95882f903..015350c6f2784 100644
> > > > --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> > > > +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> > > > @@ -275,11 +275,6 @@ static const struct mxc_isi_set_thd mxc_imx8_isi_thd_v1 = {
> > > > .panic_set_thd_v = { .mask = 0xf0000, .offset = 16, .threshold = 0x7 },
> > > > };
> > > >
> > > > -static const struct clk_bulk_data mxc_imx8mn_clks[] = {
> > > > - { .id = "axi" },
> > > > - { .id = "apb" },
> > > > -};
> > > > -
> > > > static const struct mxc_isi_plat_data mxc_imx8mn_data = {
> > > > .model = MXC_ISI_IMX8MN,
> > > > .num_ports = 1,
> > > > @@ -287,8 +282,6 @@ static const struct mxc_isi_plat_data mxc_imx8mn_data = {
> > > > .reg_offset = 0,
> > > > .ier_reg = &mxc_imx8_isi_ier_v1,
> > > > .set_thd = &mxc_imx8_isi_thd_v1,
> > > > - .clks = mxc_imx8mn_clks,
> > > > - .num_clks = ARRAY_SIZE(mxc_imx8mn_clks),
> > > > .buf_active_reverse = false,
> > > > .gasket_ops = &mxc_imx8_gasket_ops,
> > > > .has_36bit_dma = false,
> > > > @@ -301,8 +294,6 @@ static const struct mxc_isi_plat_data mxc_imx8mp_data = {
> > > > .reg_offset = 0x2000,
> > > > .ier_reg = &mxc_imx8_isi_ier_v2,
> > > > .set_thd = &mxc_imx8_isi_thd_v1,
> > > > - .clks = mxc_imx8mn_clks,
> > > > - .num_clks = ARRAY_SIZE(mxc_imx8mn_clks),
> > > > .buf_active_reverse = true,
> > > > .gasket_ops = &mxc_imx8_gasket_ops,
> > > > .has_36bit_dma = true,
> > > > @@ -315,8 +306,6 @@ static const struct mxc_isi_plat_data mxc_imx8ulp_data = {
> > > > .reg_offset = 0x0,
> > > > .ier_reg = &mxc_imx8_isi_ier_v2,
> > > > .set_thd = &mxc_imx8_isi_thd_v1,
> > > > - .clks = mxc_imx8mn_clks,
> > > > - .num_clks = ARRAY_SIZE(mxc_imx8mn_clks),
> > > > .buf_active_reverse = true,
> > > > .has_36bit_dma = false,
> > > > };
> > > > @@ -328,8 +317,6 @@ static const struct mxc_isi_plat_data mxc_imx93_data = {
> > > > .reg_offset = 0,
> > > > .ier_reg = &mxc_imx8_isi_ier_v2,
> > > > .set_thd = &mxc_imx8_isi_thd_v1,
> > > > - .clks = mxc_imx8mn_clks,
> > > > - .num_clks = ARRAY_SIZE(mxc_imx8mn_clks),
> > > > .buf_active_reverse = true,
> > > > .gasket_ops = &mxc_imx93_gasket_ops,
> > > > .has_36bit_dma = false,
> > > > @@ -386,7 +373,7 @@ static int mxc_isi_runtime_suspend(struct device *dev)
> > > > {
> > > > struct mxc_isi_dev *isi = dev_get_drvdata(dev);
> > > >
> > > > - clk_bulk_disable_unprepare(isi->pdata->num_clks, isi->clks);
> > > > + clk_bulk_disable_unprepare(isi->num_clks, isi->clks);
> > > >
> > > > return 0;
> > > > }
> > > > @@ -396,7 +383,7 @@ static int mxc_isi_runtime_resume(struct device *dev)
> > > > struct mxc_isi_dev *isi = dev_get_drvdata(dev);
> > > > int ret;
> > > >
> > > > - ret = clk_bulk_prepare_enable(isi->pdata->num_clks, isi->clks);
> > > > + ret = clk_bulk_prepare_enable(isi->num_clks, isi->clks);
> > > > if (ret) {
> > > > dev_err(dev, "Failed to enable clocks (%d)\n", ret);
> > > > return ret;
> > > > @@ -414,27 +401,6 @@ static const struct dev_pm_ops mxc_isi_pm_ops = {
> > > > * Probe, remove & driver
> > > > */
> > > >
> > > > -static int mxc_isi_clk_get(struct mxc_isi_dev *isi)
> > > > -{
> > > > - unsigned int size = isi->pdata->num_clks
> > > > - * sizeof(*isi->clks);
> > > > - int ret;
> > > > -
> > > > - isi->clks = devm_kmemdup(isi->dev, isi->pdata->clks, size, GFP_KERNEL);
> > > > - if (!isi->clks)
> > > > - return -ENOMEM;
> > > > -
> > > > - ret = devm_clk_bulk_get(isi->dev, isi->pdata->num_clks,
> > > > - isi->clks);
> > > > - if (ret < 0) {
> > > > - dev_err(isi->dev, "Failed to acquire clocks: %d\n",
> > > > - ret);
> > > > - return ret;
> > > > - }
> > > > -
> > > > - return 0;
> > > > -}
> > > > -
> > > > static int mxc_isi_probe(struct platform_device *pdev)
> > > > {
> > > > struct device *dev = &pdev->dev;
> > > > @@ -457,11 +423,9 @@ static int mxc_isi_probe(struct platform_device *pdev)
> > > > if (!isi->pipes)
> > > > return -ENOMEM;
> > > >
> > > > - ret = mxc_isi_clk_get(isi);
> > > > - if (ret < 0) {
> > > > - dev_err(dev, "Failed to get clocks\n");
> > > > - return ret;
> > > > - }
> > > > + isi->num_clks = devm_clk_bulk_get_all(dev, &isi->clks);
> > >
> > > This prevents validating that the DT contains the expected clocks, which
> > > could cause hard to debug issues. Isn't it a problem ?
> >
> > It is checked by dt-binding. Now no warning by DTB_CHECK under arm64 freecale.
> > CHECK_DTB should be enough to find expected clocks.
>
> Yes, the DTB check will catch issues at build time, but the driver will
> not enforce that. I'm not sure if there's a clear policy here, and if
> ensuring at runtime in drivers that the expected clocks are present is
> considered as a good practice by the DT maintainers. Rob, Krzysztof,
> Conor, do you have an opinion ?
Rob:
can you comment this?
Laurent:
Many driver already switch to devm_clk_bulk_get_all() such as qcom/imx6
pci drivers recently.
Frank
>
> > > > + if (isi->num_clks < 0)
> > > > + return dev_err_probe(dev, isi->num_clks, "Failed to get clocks\n");
> > > >
> > > > isi->regs = devm_platform_ioremap_resource(pdev, 0);
> > > > if (IS_ERR(isi->regs)) {
> > > > diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
> > > > index e7534a80af7b4..bd3cfe5fbe063 100644
> > > > --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
> > > > +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
> > > > @@ -169,8 +169,6 @@ struct mxc_isi_plat_data {
> > > > const struct mxc_isi_ier_reg *ier_reg;
> > > > const struct mxc_isi_set_thd *set_thd;
> > > > const struct mxc_gasket_ops *gasket_ops;
> > > > - const struct clk_bulk_data *clks;
> > > > - unsigned int num_clks;
> > > > bool buf_active_reverse;
> > > > bool has_36bit_dma;
> > > > };
> > > > @@ -282,6 +280,7 @@ struct mxc_isi_dev {
> > > >
> > > > void __iomem *regs;
> > > > struct clk_bulk_data *clks;
> > > > + int num_clks;
> > > > struct regmap *gasket;
> > > >
> > > > struct mxc_isi_crossbar crossbar;
>
> --
> Regards,
>
> Laurent Pinchart
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v4 04/13] media: nxp: imx8-isi: Use devm_clk_bulk_get_all() to fetch clocks
2025-05-08 12:35 ` Frank Li
@ 2025-06-11 14:14 ` Laurent Pinchart
2025-06-17 14:11 ` Rob Herring
0 siblings, 1 reply; 30+ messages in thread
From: Laurent Pinchart @ 2025-06-11 14:14 UTC (permalink / raw)
To: Rob Herring
Cc: Frank Li, Mauro Carvalho Chehab, Krzysztof Kozlowski,
Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Rui Miguel Silva, Martin Kepplinger,
Purism Kernel Team, linux-media, devicetree, imx,
linux-arm-kernel, linux-kernel, Robert Chiras, Guoniu.zhou
Hi Rob,
On Thu, May 08, 2025 at 08:35:53AM -0400, Frank Li wrote:
> On Fri, May 02, 2025 at 06:57:47PM +0300, Laurent Pinchart wrote:
> > On Thu, May 01, 2025 at 09:02:04PM -0400, Frank Li wrote:
> > > On Tue, Apr 22, 2025 at 12:14:38AM +0300, Laurent Pinchart wrote:
> > > > On Tue, Apr 08, 2025 at 05:53:02PM -0400, Frank Li wrote:
> > > > > Use devm_clk_bulk_get_all() helper to simplify clock handle code.
> > > > >
> > > > > No functional changes intended.
> > > > >
> > > > > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > > > > ---
> > > > > .../media/platform/nxp/imx8-isi/imx8-isi-core.c | 46 +++-------------------
> > > > > .../media/platform/nxp/imx8-isi/imx8-isi-core.h | 3 +-
> > > > > 2 files changed, 6 insertions(+), 43 deletions(-)
> > > > >
> > > > > diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> > > > > index ecfc95882f903..015350c6f2784 100644
> > > > > --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> > > > > +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> > > > > @@ -275,11 +275,6 @@ static const struct mxc_isi_set_thd mxc_imx8_isi_thd_v1 = {
> > > > > .panic_set_thd_v = { .mask = 0xf0000, .offset = 16, .threshold = 0x7 },
> > > > > };
> > > > >
> > > > > -static const struct clk_bulk_data mxc_imx8mn_clks[] = {
> > > > > - { .id = "axi" },
> > > > > - { .id = "apb" },
> > > > > -};
> > > > > -
> > > > > static const struct mxc_isi_plat_data mxc_imx8mn_data = {
> > > > > .model = MXC_ISI_IMX8MN,
> > > > > .num_ports = 1,
> > > > > @@ -287,8 +282,6 @@ static const struct mxc_isi_plat_data mxc_imx8mn_data = {
> > > > > .reg_offset = 0,
> > > > > .ier_reg = &mxc_imx8_isi_ier_v1,
> > > > > .set_thd = &mxc_imx8_isi_thd_v1,
> > > > > - .clks = mxc_imx8mn_clks,
> > > > > - .num_clks = ARRAY_SIZE(mxc_imx8mn_clks),
> > > > > .buf_active_reverse = false,
> > > > > .gasket_ops = &mxc_imx8_gasket_ops,
> > > > > .has_36bit_dma = false,
> > > > > @@ -301,8 +294,6 @@ static const struct mxc_isi_plat_data mxc_imx8mp_data = {
> > > > > .reg_offset = 0x2000,
> > > > > .ier_reg = &mxc_imx8_isi_ier_v2,
> > > > > .set_thd = &mxc_imx8_isi_thd_v1,
> > > > > - .clks = mxc_imx8mn_clks,
> > > > > - .num_clks = ARRAY_SIZE(mxc_imx8mn_clks),
> > > > > .buf_active_reverse = true,
> > > > > .gasket_ops = &mxc_imx8_gasket_ops,
> > > > > .has_36bit_dma = true,
> > > > > @@ -315,8 +306,6 @@ static const struct mxc_isi_plat_data mxc_imx8ulp_data = {
> > > > > .reg_offset = 0x0,
> > > > > .ier_reg = &mxc_imx8_isi_ier_v2,
> > > > > .set_thd = &mxc_imx8_isi_thd_v1,
> > > > > - .clks = mxc_imx8mn_clks,
> > > > > - .num_clks = ARRAY_SIZE(mxc_imx8mn_clks),
> > > > > .buf_active_reverse = true,
> > > > > .has_36bit_dma = false,
> > > > > };
> > > > > @@ -328,8 +317,6 @@ static const struct mxc_isi_plat_data mxc_imx93_data = {
> > > > > .reg_offset = 0,
> > > > > .ier_reg = &mxc_imx8_isi_ier_v2,
> > > > > .set_thd = &mxc_imx8_isi_thd_v1,
> > > > > - .clks = mxc_imx8mn_clks,
> > > > > - .num_clks = ARRAY_SIZE(mxc_imx8mn_clks),
> > > > > .buf_active_reverse = true,
> > > > > .gasket_ops = &mxc_imx93_gasket_ops,
> > > > > .has_36bit_dma = false,
> > > > > @@ -386,7 +373,7 @@ static int mxc_isi_runtime_suspend(struct device *dev)
> > > > > {
> > > > > struct mxc_isi_dev *isi = dev_get_drvdata(dev);
> > > > >
> > > > > - clk_bulk_disable_unprepare(isi->pdata->num_clks, isi->clks);
> > > > > + clk_bulk_disable_unprepare(isi->num_clks, isi->clks);
> > > > >
> > > > > return 0;
> > > > > }
> > > > > @@ -396,7 +383,7 @@ static int mxc_isi_runtime_resume(struct device *dev)
> > > > > struct mxc_isi_dev *isi = dev_get_drvdata(dev);
> > > > > int ret;
> > > > >
> > > > > - ret = clk_bulk_prepare_enable(isi->pdata->num_clks, isi->clks);
> > > > > + ret = clk_bulk_prepare_enable(isi->num_clks, isi->clks);
> > > > > if (ret) {
> > > > > dev_err(dev, "Failed to enable clocks (%d)\n", ret);
> > > > > return ret;
> > > > > @@ -414,27 +401,6 @@ static const struct dev_pm_ops mxc_isi_pm_ops = {
> > > > > * Probe, remove & driver
> > > > > */
> > > > >
> > > > > -static int mxc_isi_clk_get(struct mxc_isi_dev *isi)
> > > > > -{
> > > > > - unsigned int size = isi->pdata->num_clks
> > > > > - * sizeof(*isi->clks);
> > > > > - int ret;
> > > > > -
> > > > > - isi->clks = devm_kmemdup(isi->dev, isi->pdata->clks, size, GFP_KERNEL);
> > > > > - if (!isi->clks)
> > > > > - return -ENOMEM;
> > > > > -
> > > > > - ret = devm_clk_bulk_get(isi->dev, isi->pdata->num_clks,
> > > > > - isi->clks);
> > > > > - if (ret < 0) {
> > > > > - dev_err(isi->dev, "Failed to acquire clocks: %d\n",
> > > > > - ret);
> > > > > - return ret;
> > > > > - }
> > > > > -
> > > > > - return 0;
> > > > > -}
> > > > > -
> > > > > static int mxc_isi_probe(struct platform_device *pdev)
> > > > > {
> > > > > struct device *dev = &pdev->dev;
> > > > > @@ -457,11 +423,9 @@ static int mxc_isi_probe(struct platform_device *pdev)
> > > > > if (!isi->pipes)
> > > > > return -ENOMEM;
> > > > >
> > > > > - ret = mxc_isi_clk_get(isi);
> > > > > - if (ret < 0) {
> > > > > - dev_err(dev, "Failed to get clocks\n");
> > > > > - return ret;
> > > > > - }
> > > > > + isi->num_clks = devm_clk_bulk_get_all(dev, &isi->clks);
> > > >
> > > > This prevents validating that the DT contains the expected clocks, which
> > > > could cause hard to debug issues. Isn't it a problem ?
> > >
> > > It is checked by dt-binding. Now no warning by DTB_CHECK under arm64 freecale.
> > > CHECK_DTB should be enough to find expected clocks.
> >
> > Yes, the DTB check will catch issues at build time, but the driver will
> > not enforce that. I'm not sure if there's a clear policy here, and if
> > ensuring at runtime in drivers that the expected clocks are present is
> > considered as a good practice by the DT maintainers. Rob, Krzysztof,
> > Conor, do you have an opinion ?
>
> Rob:
> can you comment this?
Rob, any comment on this ?
> Laurent:
> Many driver already switch to devm_clk_bulk_get_all() such as qcom/imx6
> pci drivers recently.
>
> > > > > + if (isi->num_clks < 0)
> > > > > + return dev_err_probe(dev, isi->num_clks, "Failed to get clocks\n");
> > > > >
> > > > > isi->regs = devm_platform_ioremap_resource(pdev, 0);
> > > > > if (IS_ERR(isi->regs)) {
> > > > > diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
> > > > > index e7534a80af7b4..bd3cfe5fbe063 100644
> > > > > --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
> > > > > +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
> > > > > @@ -169,8 +169,6 @@ struct mxc_isi_plat_data {
> > > > > const struct mxc_isi_ier_reg *ier_reg;
> > > > > const struct mxc_isi_set_thd *set_thd;
> > > > > const struct mxc_gasket_ops *gasket_ops;
> > > > > - const struct clk_bulk_data *clks;
> > > > > - unsigned int num_clks;
> > > > > bool buf_active_reverse;
> > > > > bool has_36bit_dma;
> > > > > };
> > > > > @@ -282,6 +280,7 @@ struct mxc_isi_dev {
> > > > >
> > > > > void __iomem *regs;
> > > > > struct clk_bulk_data *clks;
> > > > > + int num_clks;
> > > > > struct regmap *gasket;
> > > > >
> > > > > struct mxc_isi_crossbar crossbar;
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v4 04/13] media: nxp: imx8-isi: Use devm_clk_bulk_get_all() to fetch clocks
2025-06-11 14:14 ` Laurent Pinchart
@ 2025-06-17 14:11 ` Rob Herring
2025-06-17 14:29 ` Laurent Pinchart
0 siblings, 1 reply; 30+ messages in thread
From: Rob Herring @ 2025-06-17 14:11 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Frank Li, Mauro Carvalho Chehab, Krzysztof Kozlowski,
Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Rui Miguel Silva, Martin Kepplinger,
Purism Kernel Team, linux-media, devicetree, imx,
linux-arm-kernel, linux-kernel, Robert Chiras, Guoniu.zhou
On Wed, Jun 11, 2025 at 05:14:49PM +0300, Laurent Pinchart wrote:
> Hi Rob,
>
> On Thu, May 08, 2025 at 08:35:53AM -0400, Frank Li wrote:
> > On Fri, May 02, 2025 at 06:57:47PM +0300, Laurent Pinchart wrote:
> > > On Thu, May 01, 2025 at 09:02:04PM -0400, Frank Li wrote:
> > > > On Tue, Apr 22, 2025 at 12:14:38AM +0300, Laurent Pinchart wrote:
> > > > > On Tue, Apr 08, 2025 at 05:53:02PM -0400, Frank Li wrote:
> > > > > > Use devm_clk_bulk_get_all() helper to simplify clock handle code.
> > > > > >
> > > > > > No functional changes intended.
> > > > > >
> > > > > > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > > > > > ---
> > > > > > .../media/platform/nxp/imx8-isi/imx8-isi-core.c | 46 +++-------------------
> > > > > > .../media/platform/nxp/imx8-isi/imx8-isi-core.h | 3 +-
> > > > > > 2 files changed, 6 insertions(+), 43 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> > > > > > index ecfc95882f903..015350c6f2784 100644
> > > > > > --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> > > > > > +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> > > > > > @@ -275,11 +275,6 @@ static const struct mxc_isi_set_thd mxc_imx8_isi_thd_v1 = {
> > > > > > .panic_set_thd_v = { .mask = 0xf0000, .offset = 16, .threshold = 0x7 },
> > > > > > };
> > > > > >
> > > > > > -static const struct clk_bulk_data mxc_imx8mn_clks[] = {
> > > > > > - { .id = "axi" },
> > > > > > - { .id = "apb" },
> > > > > > -};
> > > > > > -
> > > > > > static const struct mxc_isi_plat_data mxc_imx8mn_data = {
> > > > > > .model = MXC_ISI_IMX8MN,
> > > > > > .num_ports = 1,
> > > > > > @@ -287,8 +282,6 @@ static const struct mxc_isi_plat_data mxc_imx8mn_data = {
> > > > > > .reg_offset = 0,
> > > > > > .ier_reg = &mxc_imx8_isi_ier_v1,
> > > > > > .set_thd = &mxc_imx8_isi_thd_v1,
> > > > > > - .clks = mxc_imx8mn_clks,
> > > > > > - .num_clks = ARRAY_SIZE(mxc_imx8mn_clks),
> > > > > > .buf_active_reverse = false,
> > > > > > .gasket_ops = &mxc_imx8_gasket_ops,
> > > > > > .has_36bit_dma = false,
> > > > > > @@ -301,8 +294,6 @@ static const struct mxc_isi_plat_data mxc_imx8mp_data = {
> > > > > > .reg_offset = 0x2000,
> > > > > > .ier_reg = &mxc_imx8_isi_ier_v2,
> > > > > > .set_thd = &mxc_imx8_isi_thd_v1,
> > > > > > - .clks = mxc_imx8mn_clks,
> > > > > > - .num_clks = ARRAY_SIZE(mxc_imx8mn_clks),
> > > > > > .buf_active_reverse = true,
> > > > > > .gasket_ops = &mxc_imx8_gasket_ops,
> > > > > > .has_36bit_dma = true,
> > > > > > @@ -315,8 +306,6 @@ static const struct mxc_isi_plat_data mxc_imx8ulp_data = {
> > > > > > .reg_offset = 0x0,
> > > > > > .ier_reg = &mxc_imx8_isi_ier_v2,
> > > > > > .set_thd = &mxc_imx8_isi_thd_v1,
> > > > > > - .clks = mxc_imx8mn_clks,
> > > > > > - .num_clks = ARRAY_SIZE(mxc_imx8mn_clks),
> > > > > > .buf_active_reverse = true,
> > > > > > .has_36bit_dma = false,
> > > > > > };
> > > > > > @@ -328,8 +317,6 @@ static const struct mxc_isi_plat_data mxc_imx93_data = {
> > > > > > .reg_offset = 0,
> > > > > > .ier_reg = &mxc_imx8_isi_ier_v2,
> > > > > > .set_thd = &mxc_imx8_isi_thd_v1,
> > > > > > - .clks = mxc_imx8mn_clks,
> > > > > > - .num_clks = ARRAY_SIZE(mxc_imx8mn_clks),
> > > > > > .buf_active_reverse = true,
> > > > > > .gasket_ops = &mxc_imx93_gasket_ops,
> > > > > > .has_36bit_dma = false,
> > > > > > @@ -386,7 +373,7 @@ static int mxc_isi_runtime_suspend(struct device *dev)
> > > > > > {
> > > > > > struct mxc_isi_dev *isi = dev_get_drvdata(dev);
> > > > > >
> > > > > > - clk_bulk_disable_unprepare(isi->pdata->num_clks, isi->clks);
> > > > > > + clk_bulk_disable_unprepare(isi->num_clks, isi->clks);
> > > > > >
> > > > > > return 0;
> > > > > > }
> > > > > > @@ -396,7 +383,7 @@ static int mxc_isi_runtime_resume(struct device *dev)
> > > > > > struct mxc_isi_dev *isi = dev_get_drvdata(dev);
> > > > > > int ret;
> > > > > >
> > > > > > - ret = clk_bulk_prepare_enable(isi->pdata->num_clks, isi->clks);
> > > > > > + ret = clk_bulk_prepare_enable(isi->num_clks, isi->clks);
> > > > > > if (ret) {
> > > > > > dev_err(dev, "Failed to enable clocks (%d)\n", ret);
> > > > > > return ret;
> > > > > > @@ -414,27 +401,6 @@ static const struct dev_pm_ops mxc_isi_pm_ops = {
> > > > > > * Probe, remove & driver
> > > > > > */
> > > > > >
> > > > > > -static int mxc_isi_clk_get(struct mxc_isi_dev *isi)
> > > > > > -{
> > > > > > - unsigned int size = isi->pdata->num_clks
> > > > > > - * sizeof(*isi->clks);
> > > > > > - int ret;
> > > > > > -
> > > > > > - isi->clks = devm_kmemdup(isi->dev, isi->pdata->clks, size, GFP_KERNEL);
> > > > > > - if (!isi->clks)
> > > > > > - return -ENOMEM;
> > > > > > -
> > > > > > - ret = devm_clk_bulk_get(isi->dev, isi->pdata->num_clks,
> > > > > > - isi->clks);
> > > > > > - if (ret < 0) {
> > > > > > - dev_err(isi->dev, "Failed to acquire clocks: %d\n",
> > > > > > - ret);
> > > > > > - return ret;
> > > > > > - }
> > > > > > -
> > > > > > - return 0;
> > > > > > -}
> > > > > > -
> > > > > > static int mxc_isi_probe(struct platform_device *pdev)
> > > > > > {
> > > > > > struct device *dev = &pdev->dev;
> > > > > > @@ -457,11 +423,9 @@ static int mxc_isi_probe(struct platform_device *pdev)
> > > > > > if (!isi->pipes)
> > > > > > return -ENOMEM;
> > > > > >
> > > > > > - ret = mxc_isi_clk_get(isi);
> > > > > > - if (ret < 0) {
> > > > > > - dev_err(dev, "Failed to get clocks\n");
> > > > > > - return ret;
> > > > > > - }
> > > > > > + isi->num_clks = devm_clk_bulk_get_all(dev, &isi->clks);
> > > > >
> > > > > This prevents validating that the DT contains the expected clocks, which
> > > > > could cause hard to debug issues. Isn't it a problem ?
> > > >
> > > > It is checked by dt-binding. Now no warning by DTB_CHECK under arm64 freecale.
> > > > CHECK_DTB should be enough to find expected clocks.
> > >
> > > Yes, the DTB check will catch issues at build time, but the driver will
> > > not enforce that. I'm not sure if there's a clear policy here, and if
> > > ensuring at runtime in drivers that the expected clocks are present is
> > > considered as a good practice by the DT maintainers. Rob, Krzysztof,
> > > Conor, do you have an opinion ?
> >
> > Rob:
> > can you comment this?
>
> Rob, any comment on this ?
My preference is to not do validation of the DT in the kernel. Unless
you need greater control of the clocks, I would use
devm_clk_bulk_get_all().
Rob
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v4 04/13] media: nxp: imx8-isi: Use devm_clk_bulk_get_all() to fetch clocks
2025-06-17 14:11 ` Rob Herring
@ 2025-06-17 14:29 ` Laurent Pinchart
0 siblings, 0 replies; 30+ messages in thread
From: Laurent Pinchart @ 2025-06-17 14:29 UTC (permalink / raw)
To: Rob Herring
Cc: Frank Li, Mauro Carvalho Chehab, Krzysztof Kozlowski,
Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Rui Miguel Silva, Martin Kepplinger,
Purism Kernel Team, linux-media, devicetree, imx,
linux-arm-kernel, linux-kernel, Robert Chiras, Guoniu.zhou
Hi Rob,
On Tue, Jun 17, 2025 at 09:11:12AM -0500, Rob Herring wrote:
> On Wed, Jun 11, 2025 at 05:14:49PM +0300, Laurent Pinchart wrote:
> > On Thu, May 08, 2025 at 08:35:53AM -0400, Frank Li wrote:
> > > On Fri, May 02, 2025 at 06:57:47PM +0300, Laurent Pinchart wrote:
> > > > On Thu, May 01, 2025 at 09:02:04PM -0400, Frank Li wrote:
> > > > > On Tue, Apr 22, 2025 at 12:14:38AM +0300, Laurent Pinchart wrote:
> > > > > > On Tue, Apr 08, 2025 at 05:53:02PM -0400, Frank Li wrote:
> > > > > > > Use devm_clk_bulk_get_all() helper to simplify clock handle code.
> > > > > > >
> > > > > > > No functional changes intended.
> > > > > > >
> > > > > > > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > > > > > > ---
> > > > > > > .../media/platform/nxp/imx8-isi/imx8-isi-core.c | 46 +++-------------------
> > > > > > > .../media/platform/nxp/imx8-isi/imx8-isi-core.h | 3 +-
> > > > > > > 2 files changed, 6 insertions(+), 43 deletions(-)
> > > > > > >
> > > > > > > diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> > > > > > > index ecfc95882f903..015350c6f2784 100644
> > > > > > > --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> > > > > > > +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> > > > > > > @@ -275,11 +275,6 @@ static const struct mxc_isi_set_thd mxc_imx8_isi_thd_v1 = {
> > > > > > > .panic_set_thd_v = { .mask = 0xf0000, .offset = 16, .threshold = 0x7 },
> > > > > > > };
> > > > > > >
> > > > > > > -static const struct clk_bulk_data mxc_imx8mn_clks[] = {
> > > > > > > - { .id = "axi" },
> > > > > > > - { .id = "apb" },
> > > > > > > -};
> > > > > > > -
> > > > > > > static const struct mxc_isi_plat_data mxc_imx8mn_data = {
> > > > > > > .model = MXC_ISI_IMX8MN,
> > > > > > > .num_ports = 1,
> > > > > > > @@ -287,8 +282,6 @@ static const struct mxc_isi_plat_data mxc_imx8mn_data = {
> > > > > > > .reg_offset = 0,
> > > > > > > .ier_reg = &mxc_imx8_isi_ier_v1,
> > > > > > > .set_thd = &mxc_imx8_isi_thd_v1,
> > > > > > > - .clks = mxc_imx8mn_clks,
> > > > > > > - .num_clks = ARRAY_SIZE(mxc_imx8mn_clks),
> > > > > > > .buf_active_reverse = false,
> > > > > > > .gasket_ops = &mxc_imx8_gasket_ops,
> > > > > > > .has_36bit_dma = false,
> > > > > > > @@ -301,8 +294,6 @@ static const struct mxc_isi_plat_data mxc_imx8mp_data = {
> > > > > > > .reg_offset = 0x2000,
> > > > > > > .ier_reg = &mxc_imx8_isi_ier_v2,
> > > > > > > .set_thd = &mxc_imx8_isi_thd_v1,
> > > > > > > - .clks = mxc_imx8mn_clks,
> > > > > > > - .num_clks = ARRAY_SIZE(mxc_imx8mn_clks),
> > > > > > > .buf_active_reverse = true,
> > > > > > > .gasket_ops = &mxc_imx8_gasket_ops,
> > > > > > > .has_36bit_dma = true,
> > > > > > > @@ -315,8 +306,6 @@ static const struct mxc_isi_plat_data mxc_imx8ulp_data = {
> > > > > > > .reg_offset = 0x0,
> > > > > > > .ier_reg = &mxc_imx8_isi_ier_v2,
> > > > > > > .set_thd = &mxc_imx8_isi_thd_v1,
> > > > > > > - .clks = mxc_imx8mn_clks,
> > > > > > > - .num_clks = ARRAY_SIZE(mxc_imx8mn_clks),
> > > > > > > .buf_active_reverse = true,
> > > > > > > .has_36bit_dma = false,
> > > > > > > };
> > > > > > > @@ -328,8 +317,6 @@ static const struct mxc_isi_plat_data mxc_imx93_data = {
> > > > > > > .reg_offset = 0,
> > > > > > > .ier_reg = &mxc_imx8_isi_ier_v2,
> > > > > > > .set_thd = &mxc_imx8_isi_thd_v1,
> > > > > > > - .clks = mxc_imx8mn_clks,
> > > > > > > - .num_clks = ARRAY_SIZE(mxc_imx8mn_clks),
> > > > > > > .buf_active_reverse = true,
> > > > > > > .gasket_ops = &mxc_imx93_gasket_ops,
> > > > > > > .has_36bit_dma = false,
> > > > > > > @@ -386,7 +373,7 @@ static int mxc_isi_runtime_suspend(struct device *dev)
> > > > > > > {
> > > > > > > struct mxc_isi_dev *isi = dev_get_drvdata(dev);
> > > > > > >
> > > > > > > - clk_bulk_disable_unprepare(isi->pdata->num_clks, isi->clks);
> > > > > > > + clk_bulk_disable_unprepare(isi->num_clks, isi->clks);
> > > > > > >
> > > > > > > return 0;
> > > > > > > }
> > > > > > > @@ -396,7 +383,7 @@ static int mxc_isi_runtime_resume(struct device *dev)
> > > > > > > struct mxc_isi_dev *isi = dev_get_drvdata(dev);
> > > > > > > int ret;
> > > > > > >
> > > > > > > - ret = clk_bulk_prepare_enable(isi->pdata->num_clks, isi->clks);
> > > > > > > + ret = clk_bulk_prepare_enable(isi->num_clks, isi->clks);
> > > > > > > if (ret) {
> > > > > > > dev_err(dev, "Failed to enable clocks (%d)\n", ret);
> > > > > > > return ret;
> > > > > > > @@ -414,27 +401,6 @@ static const struct dev_pm_ops mxc_isi_pm_ops = {
> > > > > > > * Probe, remove & driver
> > > > > > > */
> > > > > > >
> > > > > > > -static int mxc_isi_clk_get(struct mxc_isi_dev *isi)
> > > > > > > -{
> > > > > > > - unsigned int size = isi->pdata->num_clks
> > > > > > > - * sizeof(*isi->clks);
> > > > > > > - int ret;
> > > > > > > -
> > > > > > > - isi->clks = devm_kmemdup(isi->dev, isi->pdata->clks, size, GFP_KERNEL);
> > > > > > > - if (!isi->clks)
> > > > > > > - return -ENOMEM;
> > > > > > > -
> > > > > > > - ret = devm_clk_bulk_get(isi->dev, isi->pdata->num_clks,
> > > > > > > - isi->clks);
> > > > > > > - if (ret < 0) {
> > > > > > > - dev_err(isi->dev, "Failed to acquire clocks: %d\n",
> > > > > > > - ret);
> > > > > > > - return ret;
> > > > > > > - }
> > > > > > > -
> > > > > > > - return 0;
> > > > > > > -}
> > > > > > > -
> > > > > > > static int mxc_isi_probe(struct platform_device *pdev)
> > > > > > > {
> > > > > > > struct device *dev = &pdev->dev;
> > > > > > > @@ -457,11 +423,9 @@ static int mxc_isi_probe(struct platform_device *pdev)
> > > > > > > if (!isi->pipes)
> > > > > > > return -ENOMEM;
> > > > > > >
> > > > > > > - ret = mxc_isi_clk_get(isi);
> > > > > > > - if (ret < 0) {
> > > > > > > - dev_err(dev, "Failed to get clocks\n");
> > > > > > > - return ret;
> > > > > > > - }
> > > > > > > + isi->num_clks = devm_clk_bulk_get_all(dev, &isi->clks);
> > > > > >
> > > > > > This prevents validating that the DT contains the expected clocks, which
> > > > > > could cause hard to debug issues. Isn't it a problem ?
> > > > >
> > > > > It is checked by dt-binding. Now no warning by DTB_CHECK under arm64 freecale.
> > > > > CHECK_DTB should be enough to find expected clocks.
> > > >
> > > > Yes, the DTB check will catch issues at build time, but the driver will
> > > > not enforce that. I'm not sure if there's a clear policy here, and if
> > > > ensuring at runtime in drivers that the expected clocks are present is
> > > > considered as a good practice by the DT maintainers. Rob, Krzysztof,
> > > > Conor, do you have an opinion ?
> > >
> > > Rob:
> > > can you comment this?
> >
> > Rob, any comment on this ?
>
> My preference is to not do validation of the DT in the kernel. Unless
> you need greater control of the clocks, I would use
> devm_clk_bulk_get_all().
Thank you for your feedback. We'll go forward with this patch then.
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v4 05/13] media: nxp: imx8-isi: Remove redundant check for dma_set_mask_and_coherent()
2025-04-08 21:52 [PATCH v4 00/13] media: imx8: add camera support Frank Li
` (3 preceding siblings ...)
2025-04-08 21:53 ` [PATCH v4 04/13] media: nxp: imx8-isi: Use devm_clk_bulk_get_all() to fetch clocks Frank Li
@ 2025-04-08 21:53 ` Frank Li
2025-04-21 21:19 ` Laurent Pinchart
2025-04-08 21:53 ` [PATCH v4 06/13] media: nxp: imx8-isi: Use dev_err_probe() simplify code Frank Li
` (8 subsequent siblings)
13 siblings, 1 reply; 30+ messages in thread
From: Frank Li @ 2025-04-08 21:53 UTC (permalink / raw)
To: Laurent Pinchart, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Frank Li,
Rui Miguel Silva, Martin Kepplinger, Purism Kernel Team
Cc: linux-media, devicetree, imx, linux-arm-kernel, linux-kernel,
Robert Chiras, Guoniu.zhou
dma_set_mask_and_coherent() never return failure when mask bigger than
32bit.
See commit f7ae20f2fc4e ("docs: dma: correct dma_set_mask() sample code")
So remove return value check for dma_set_mask_and_coherent().
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
index 015350c6f2784..073ea5912de3b 100644
--- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
+++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
@@ -444,11 +444,7 @@ static int mxc_isi_probe(struct platform_device *pdev)
}
dma_size = isi->pdata->has_36bit_dma ? 36 : 32;
- ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(dma_size));
- if (ret) {
- dev_err(dev, "failed to set DMA mask\n");
- return ret;
- }
+ dma_set_mask_and_coherent(dev, DMA_BIT_MASK(dma_size));
pm_runtime_enable(dev);
--
2.34.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH v4 05/13] media: nxp: imx8-isi: Remove redundant check for dma_set_mask_and_coherent()
2025-04-08 21:53 ` [PATCH v4 05/13] media: nxp: imx8-isi: Remove redundant check for dma_set_mask_and_coherent() Frank Li
@ 2025-04-21 21:19 ` Laurent Pinchart
0 siblings, 0 replies; 30+ messages in thread
From: Laurent Pinchart @ 2025-04-21 21:19 UTC (permalink / raw)
To: Frank Li
Cc: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Rui Miguel Silva, Martin Kepplinger,
Purism Kernel Team, linux-media, devicetree, imx,
linux-arm-kernel, linux-kernel, Robert Chiras, Guoniu.zhou
Hi Frank,
Thank you for the patch.
On Tue, Apr 08, 2025 at 05:53:03PM -0400, Frank Li wrote:
> dma_set_mask_and_coherent() never return failure when mask bigger than
> 32bit.
>
> See commit f7ae20f2fc4e ("docs: dma: correct dma_set_mask() sample code")
>
> So remove return value check for dma_set_mask_and_coherent().
>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c | 6 +-----
> 1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> index 015350c6f2784..073ea5912de3b 100644
> --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> @@ -444,11 +444,7 @@ static int mxc_isi_probe(struct platform_device *pdev)
> }
>
> dma_size = isi->pdata->has_36bit_dma ? 36 : 32;
> - ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(dma_size));
> - if (ret) {
> - dev_err(dev, "failed to set DMA mask\n");
> - return ret;
> - }
> + dma_set_mask_and_coherent(dev, DMA_BIT_MASK(dma_size));
>
> pm_runtime_enable(dev);
>
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v4 06/13] media: nxp: imx8-isi: Use dev_err_probe() simplify code
2025-04-08 21:52 [PATCH v4 00/13] media: imx8: add camera support Frank Li
` (4 preceding siblings ...)
2025-04-08 21:53 ` [PATCH v4 05/13] media: nxp: imx8-isi: Remove redundant check for dma_set_mask_and_coherent() Frank Li
@ 2025-04-08 21:53 ` Frank Li
2025-04-21 21:21 ` Laurent Pinchart
2025-04-08 21:53 ` [PATCH v4 07/13] media: imx8-isi: Add support for i.MX8QM and i.MX8QXP Frank Li
` (7 subsequent siblings)
13 siblings, 1 reply; 30+ messages in thread
From: Frank Li @ 2025-04-08 21:53 UTC (permalink / raw)
To: Laurent Pinchart, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Frank Li,
Rui Miguel Silva, Martin Kepplinger, Purism Kernel Team
Cc: linux-media, devicetree, imx, linux-arm-kernel, linux-kernel,
Robert Chiras, Guoniu.zhou
Use dev_err_probe() simplify code. No functional changes intended.
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c | 13 ++++---------
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
index 073ea5912de3b..398cc03443be3 100644
--- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
+++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
@@ -428,19 +428,14 @@ static int mxc_isi_probe(struct platform_device *pdev)
return dev_err_probe(dev, isi->num_clks, "Failed to get clocks\n");
isi->regs = devm_platform_ioremap_resource(pdev, 0);
- if (IS_ERR(isi->regs)) {
- dev_err(dev, "Failed to get ISI register map\n");
- return PTR_ERR(isi->regs);
- }
+ if (IS_ERR(isi->regs))
+ return dev_err_probe(dev, PTR_ERR(isi->regs), "Failed to get ISI register map\n");
if (isi->pdata->gasket_ops) {
isi->gasket = syscon_regmap_lookup_by_phandle(dev->of_node,
"fsl,blk-ctrl");
- if (IS_ERR(isi->gasket)) {
- ret = PTR_ERR(isi->gasket);
- dev_err(dev, "failed to get gasket: %d\n", ret);
- return ret;
- }
+ if (IS_ERR(isi->gasket))
+ return dev_err_probe(dev, PTR_ERR(isi->gasket), "failed to get gasket\n");
}
dma_size = isi->pdata->has_36bit_dma ? 36 : 32;
--
2.34.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH v4 06/13] media: nxp: imx8-isi: Use dev_err_probe() simplify code
2025-04-08 21:53 ` [PATCH v4 06/13] media: nxp: imx8-isi: Use dev_err_probe() simplify code Frank Li
@ 2025-04-21 21:21 ` Laurent Pinchart
0 siblings, 0 replies; 30+ messages in thread
From: Laurent Pinchart @ 2025-04-21 21:21 UTC (permalink / raw)
To: Frank Li
Cc: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Rui Miguel Silva, Martin Kepplinger,
Purism Kernel Team, linux-media, devicetree, imx,
linux-arm-kernel, linux-kernel, Robert Chiras, Guoniu.zhou
Hi Frank,
Thank you for the patch.
On Tue, Apr 08, 2025 at 05:53:04PM -0400, Frank Li wrote:
> Use dev_err_probe() simplify code. No functional changes intended.
In the subject line and here, s/simplify/to simplify/
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c | 13 ++++---------
> 1 file changed, 4 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> index 073ea5912de3b..398cc03443be3 100644
> --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> @@ -428,19 +428,14 @@ static int mxc_isi_probe(struct platform_device *pdev)
> return dev_err_probe(dev, isi->num_clks, "Failed to get clocks\n");
>
> isi->regs = devm_platform_ioremap_resource(pdev, 0);
> - if (IS_ERR(isi->regs)) {
> - dev_err(dev, "Failed to get ISI register map\n");
> - return PTR_ERR(isi->regs);
> - }
> + if (IS_ERR(isi->regs))
> + return dev_err_probe(dev, PTR_ERR(isi->regs), "Failed to get ISI register map\n");
return dev_err_probe(dev, PTR_ERR(isi->regs),
"Failed to get ISI register map\n");
>
> if (isi->pdata->gasket_ops) {
> isi->gasket = syscon_regmap_lookup_by_phandle(dev->of_node,
> "fsl,blk-ctrl");
> - if (IS_ERR(isi->gasket)) {
> - ret = PTR_ERR(isi->gasket);
> - dev_err(dev, "failed to get gasket: %d\n", ret);
> - return ret;
> - }
> + if (IS_ERR(isi->gasket))
> + return dev_err_probe(dev, PTR_ERR(isi->gasket), "failed to get gasket\n");
return dev_err_probe(dev, PTR_ERR(isi->gasket),
"failed to get gasket\n");
With that,
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> }
>
> dma_size = isi->pdata->has_36bit_dma ? 36 : 32;
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v4 07/13] media: imx8-isi: Add support for i.MX8QM and i.MX8QXP
2025-04-08 21:52 [PATCH v4 00/13] media: imx8: add camera support Frank Li
` (5 preceding siblings ...)
2025-04-08 21:53 ` [PATCH v4 06/13] media: nxp: imx8-isi: Use dev_err_probe() simplify code Frank Li
@ 2025-04-08 21:53 ` Frank Li
2025-04-21 21:32 ` Laurent Pinchart
2025-04-08 21:53 ` [PATCH v4 08/13] media: dt-bindings: nxp,imx8mq-mipi-csi2: Add i.MX8QM(QXP) compatible strings Frank Li
` (6 subsequent siblings)
13 siblings, 1 reply; 30+ messages in thread
From: Frank Li @ 2025-04-08 21:53 UTC (permalink / raw)
To: Laurent Pinchart, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Frank Li,
Rui Miguel Silva, Martin Kepplinger, Purism Kernel Team
Cc: linux-media, devicetree, imx, linux-arm-kernel, linux-kernel,
Robert Chiras, Guoniu.zhou
From: Robert Chiras <robert.chiras@nxp.com>
Add compatibles and platform data for i.MX8QM and i.MX8QXP platforms.
i.MX8QM's IER register layout is difference with i.MX8QXP.
Signed-off-by: Robert Chiras <robert.chiras@nxp.com>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Change from v3 to v4
- fix i.MX8QMP ier register layout
- Remove clk-names array
- Change 8QXP channel number to 6
change from v2 to v3
- none
change from v1 to v2
- remove intenal review tags
---
.../media/platform/nxp/imx8-isi/imx8-isi-core.c | 43 +++++++++++++++++++++-
.../media/platform/nxp/imx8-isi/imx8-isi-core.h | 2 +
2 files changed, 43 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
index 398cc03443be3..568d7b24466aa 100644
--- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
+++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
@@ -246,7 +246,7 @@ static void mxc_isi_v4l2_cleanup(struct mxc_isi_dev *isi)
/* Panic will assert when the buffers are 50% full */
-/* For i.MX8QXP C0 and i.MX8MN ISI IER version */
+/* For i.MX8MN ISI IER version */
static const struct mxc_isi_ier_reg mxc_imx8_isi_ier_v1 = {
.oflw_y_buf_en = { .mask = BIT(19) },
.oflw_u_buf_en = { .mask = BIT(21) },
@@ -257,7 +257,7 @@ static const struct mxc_isi_ier_reg mxc_imx8_isi_ier_v1 = {
.panic_v_buf_en = { .mask = BIT(24) },
};
-/* For i.MX8MP ISI IER version */
+/* For i.MX8QXP C0 and i.MX8MP ISI IER version */
static const struct mxc_isi_ier_reg mxc_imx8_isi_ier_v2 = {
.oflw_y_buf_en = { .mask = BIT(18) },
.oflw_u_buf_en = { .mask = BIT(20) },
@@ -268,6 +268,21 @@ static const struct mxc_isi_ier_reg mxc_imx8_isi_ier_v2 = {
.panic_v_buf_en = { .mask = BIT(23) },
};
+/* For i.MX8QM ISI IER version */
+static const struct mxc_isi_ier_reg mxc_imx8_isi_ier_qm = {
+ .oflw_y_buf_en = { .mask = BIT(16) },
+ .oflw_u_buf_en = { .mask = BIT(19) },
+ .oflw_v_buf_en = { .mask = BIT(22) },
+
+ .excs_oflw_y_buf_en = { .mask = BIT(17) },
+ .excs_oflw_u_buf_en = { .mask = BIT(20) },
+ .excs_oflw_v_buf_en = { .mask = BIT(23) },
+
+ .panic_y_buf_en = { .mask = BIT(18) },
+ .panic_u_buf_en = { .mask = BIT(21) },
+ .panic_v_buf_en = { .mask = BIT(24) },
+};
+
/* Panic will assert when the buffers are 50% full */
static const struct mxc_isi_set_thd mxc_imx8_isi_thd_v1 = {
.panic_set_thd_y = { .mask = 0x0000f, .offset = 0, .threshold = 0x7 },
@@ -322,6 +337,28 @@ static const struct mxc_isi_plat_data mxc_imx93_data = {
.has_36bit_dma = false,
};
+static const struct mxc_isi_plat_data mxc_imx8qm_data = {
+ .model = MXC_ISI_IMX8QM,
+ .num_ports = 5,
+ .num_channels = 8,
+ .reg_offset = 0x10000,
+ .ier_reg = &mxc_imx8_isi_ier_qm,
+ .set_thd = &mxc_imx8_isi_thd_v1,
+ .buf_active_reverse = true,
+ .has_36bit_dma = false,
+};
+
+static const struct mxc_isi_plat_data mxc_imx8qxp_data = {
+ .model = MXC_ISI_IMX8QXP,
+ .num_ports = 5,
+ .num_channels = 6,
+ .reg_offset = 0x10000,
+ .ier_reg = &mxc_imx8_isi_ier_v2,
+ .set_thd = &mxc_imx8_isi_thd_v1,
+ .buf_active_reverse = true,
+ .has_36bit_dma = false,
+};
+
/* -----------------------------------------------------------------------------
* Power management
*/
@@ -497,6 +534,8 @@ static void mxc_isi_remove(struct platform_device *pdev)
static const struct of_device_id mxc_isi_of_match[] = {
{ .compatible = "fsl,imx8mn-isi", .data = &mxc_imx8mn_data },
{ .compatible = "fsl,imx8mp-isi", .data = &mxc_imx8mp_data },
+ { .compatible = "fsl,imx8qm-isi", .data = &mxc_imx8qm_data },
+ { .compatible = "fsl,imx8qxp-isi", .data = &mxc_imx8qxp_data },
{ .compatible = "fsl,imx8ulp-isi", .data = &mxc_imx8ulp_data },
{ .compatible = "fsl,imx93-isi", .data = &mxc_imx93_data },
{ /* sentinel */ },
diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
index bd3cfe5fbe063..206995bedca4a 100644
--- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
+++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
@@ -157,6 +157,8 @@ struct mxc_gasket_ops {
enum model {
MXC_ISI_IMX8MN,
MXC_ISI_IMX8MP,
+ MXC_ISI_IMX8QM,
+ MXC_ISI_IMX8QXP,
MXC_ISI_IMX8ULP,
MXC_ISI_IMX93,
};
--
2.34.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH v4 07/13] media: imx8-isi: Add support for i.MX8QM and i.MX8QXP
2025-04-08 21:53 ` [PATCH v4 07/13] media: imx8-isi: Add support for i.MX8QM and i.MX8QXP Frank Li
@ 2025-04-21 21:32 ` Laurent Pinchart
0 siblings, 0 replies; 30+ messages in thread
From: Laurent Pinchart @ 2025-04-21 21:32 UTC (permalink / raw)
To: Frank Li
Cc: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Rui Miguel Silva, Martin Kepplinger,
Purism Kernel Team, linux-media, devicetree, imx,
linux-arm-kernel, linux-kernel, Robert Chiras, Guoniu.zhou
Hi Frank,
Thank you for the patch.
On Tue, Apr 08, 2025 at 05:53:05PM -0400, Frank Li wrote:
> From: Robert Chiras <robert.chiras@nxp.com>
>
> Add compatibles and platform data for i.MX8QM and i.MX8QXP platforms.
> i.MX8QM's IER register layout is difference with i.MX8QXP.
>
> Signed-off-by: Robert Chiras <robert.chiras@nxp.com>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> Change from v3 to v4
> - fix i.MX8QMP ier register layout
> - Remove clk-names array
> - Change 8QXP channel number to 6
>
> change from v2 to v3
> - none
>
> change from v1 to v2
> - remove intenal review tags
> ---
> .../media/platform/nxp/imx8-isi/imx8-isi-core.c | 43 +++++++++++++++++++++-
> .../media/platform/nxp/imx8-isi/imx8-isi-core.h | 2 +
> 2 files changed, 43 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> index 398cc03443be3..568d7b24466aa 100644
> --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> @@ -246,7 +246,7 @@ static void mxc_isi_v4l2_cleanup(struct mxc_isi_dev *isi)
>
> /* Panic will assert when the buffers are 50% full */
>
> -/* For i.MX8QXP C0 and i.MX8MN ISI IER version */
> +/* For i.MX8MN ISI IER version */
> static const struct mxc_isi_ier_reg mxc_imx8_isi_ier_v1 = {
> .oflw_y_buf_en = { .mask = BIT(19) },
> .oflw_u_buf_en = { .mask = BIT(21) },
> @@ -257,7 +257,7 @@ static const struct mxc_isi_ier_reg mxc_imx8_isi_ier_v1 = {
> .panic_v_buf_en = { .mask = BIT(24) },
> };
>
> -/* For i.MX8MP ISI IER version */
> +/* For i.MX8QXP C0 and i.MX8MP ISI IER version */
> static const struct mxc_isi_ier_reg mxc_imx8_isi_ier_v2 = {
> .oflw_y_buf_en = { .mask = BIT(18) },
> .oflw_u_buf_en = { .mask = BIT(20) },
> @@ -268,6 +268,21 @@ static const struct mxc_isi_ier_reg mxc_imx8_isi_ier_v2 = {
> .panic_v_buf_en = { .mask = BIT(23) },
> };
>
> +/* For i.MX8QM ISI IER version */
> +static const struct mxc_isi_ier_reg mxc_imx8_isi_ier_qm = {
> + .oflw_y_buf_en = { .mask = BIT(16) },
> + .oflw_u_buf_en = { .mask = BIT(19) },
> + .oflw_v_buf_en = { .mask = BIT(22) },
> +
> + .excs_oflw_y_buf_en = { .mask = BIT(17) },
> + .excs_oflw_u_buf_en = { .mask = BIT(20) },
> + .excs_oflw_v_buf_en = { .mask = BIT(23) },
> +
> + .panic_y_buf_en = { .mask = BIT(18) },
> + .panic_u_buf_en = { .mask = BIT(21) },
> + .panic_v_buf_en = { .mask = BIT(24) },
> +};
> +
> /* Panic will assert when the buffers are 50% full */
> static const struct mxc_isi_set_thd mxc_imx8_isi_thd_v1 = {
> .panic_set_thd_y = { .mask = 0x0000f, .offset = 0, .threshold = 0x7 },
> @@ -322,6 +337,28 @@ static const struct mxc_isi_plat_data mxc_imx93_data = {
> .has_36bit_dma = false,
> };
>
> +static const struct mxc_isi_plat_data mxc_imx8qm_data = {
> + .model = MXC_ISI_IMX8QM,
> + .num_ports = 5,
> + .num_channels = 8,
> + .reg_offset = 0x10000,
> + .ier_reg = &mxc_imx8_isi_ier_qm,
> + .set_thd = &mxc_imx8_isi_thd_v1,
> + .buf_active_reverse = true,
> + .has_36bit_dma = false,
> +};
> +
> +static const struct mxc_isi_plat_data mxc_imx8qxp_data = {
> + .model = MXC_ISI_IMX8QXP,
> + .num_ports = 5,
> + .num_channels = 6,
> + .reg_offset = 0x10000,
> + .ier_reg = &mxc_imx8_isi_ier_v2,
> + .set_thd = &mxc_imx8_isi_thd_v1,
> + .buf_active_reverse = true,
> + .has_36bit_dma = false,
> +};
> +
> /* -----------------------------------------------------------------------------
> * Power management
> */
> @@ -497,6 +534,8 @@ static void mxc_isi_remove(struct platform_device *pdev)
> static const struct of_device_id mxc_isi_of_match[] = {
> { .compatible = "fsl,imx8mn-isi", .data = &mxc_imx8mn_data },
> { .compatible = "fsl,imx8mp-isi", .data = &mxc_imx8mp_data },
> + { .compatible = "fsl,imx8qm-isi", .data = &mxc_imx8qm_data },
> + { .compatible = "fsl,imx8qxp-isi", .data = &mxc_imx8qxp_data },
> { .compatible = "fsl,imx8ulp-isi", .data = &mxc_imx8ulp_data },
> { .compatible = "fsl,imx93-isi", .data = &mxc_imx93_data },
> { /* sentinel */ },
> diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
> index bd3cfe5fbe063..206995bedca4a 100644
> --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
> +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
> @@ -157,6 +157,8 @@ struct mxc_gasket_ops {
> enum model {
> MXC_ISI_IMX8MN,
> MXC_ISI_IMX8MP,
> + MXC_ISI_IMX8QM,
> + MXC_ISI_IMX8QXP,
> MXC_ISI_IMX8ULP,
> MXC_ISI_IMX93,
> };
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v4 08/13] media: dt-bindings: nxp,imx8mq-mipi-csi2: Add i.MX8QM(QXP) compatible strings
2025-04-08 21:52 [PATCH v4 00/13] media: imx8: add camera support Frank Li
` (6 preceding siblings ...)
2025-04-08 21:53 ` [PATCH v4 07/13] media: imx8-isi: Add support for i.MX8QM and i.MX8QXP Frank Li
@ 2025-04-08 21:53 ` Frank Li
2025-04-08 21:53 ` [PATCH v4 09/13] media: imx8mq-mipi-csi2: Add imx8mq_plat_data for different " Frank Li
` (5 subsequent siblings)
13 siblings, 0 replies; 30+ messages in thread
From: Frank Li @ 2025-04-08 21:53 UTC (permalink / raw)
To: Laurent Pinchart, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Frank Li,
Rui Miguel Silva, Martin Kepplinger, Purism Kernel Team
Cc: linux-media, devicetree, imx, linux-arm-kernel, linux-kernel,
Robert Chiras, Guoniu.zhou
From: Robert Chiras <robert.chiras@nxp.com>
Add compatible strings for i.MX8QM/i.MX8QXP platform. Remove
fsl,mipi-phy-gpr from required properties and add new reg space, since
i.MX8QM and i.MX8QXP use dedicate control and status register(csr) space.
Keep the same restriction for other compatible strings.
Signed-off-by: Robert Chiras <robert.chiras@nxp.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Change from v3 to v4
- use MIPI CSI-2
- Add Laurent Pinchart reviewed-by tags. Reset description is not very
accurate, but it should be good enough since use SCU reset. SCU reset do
reset for PHY and controller.
change from v2 to v3
- use dedicate csr register space
change from v1 to v2
- remove internal review tags
- remove reg maxitems:1
- remove 8ulp part
- add 8qxp compatible string and make 8qm failback to 8qxp
- limit reset and power domain number to 1 for 8qxp and 8qm
- remove power-domains change because 8qm/8qxp only need 1 power domain
---
.../bindings/media/nxp,imx8mq-mipi-csi2.yaml | 38 +++++++++++++++++++---
1 file changed, 34 insertions(+), 4 deletions(-)
diff --git a/Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.yaml b/Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.yaml
index 2a14e3b0e0040..3389bab266a9a 100644
--- a/Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.yaml
+++ b/Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.yaml
@@ -16,11 +16,19 @@ description: |-
properties:
compatible:
- enum:
- - fsl,imx8mq-mipi-csi2
+ oneOf:
+ - enum:
+ - fsl,imx8mq-mipi-csi2
+ - fsl,imx8qxp-mipi-csi2
+ - items:
+ - const: fsl,imx8qm-mipi-csi2
+ - const: fsl,imx8qxp-mipi-csi2
reg:
- maxItems: 1
+ items:
+ - description: MIPI CSI-2 RX host controller register.
+ - description: MIPI CSI-2 control and status register (csr).
+ minItems: 1
clocks:
items:
@@ -46,6 +54,7 @@ properties:
- description: CORE_RESET reset register bit definition
- description: PHY_REF_RESET reset register bit definition
- description: ESC_RESET reset register bit definition
+ minItems: 1
fsl,mipi-phy-gpr:
description: |
@@ -113,9 +122,30 @@ required:
- clock-names
- power-domains
- resets
- - fsl,mipi-phy-gpr
- ports
+allOf:
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - fsl,imx8qxp-mipi-csi2
+ then:
+ properties:
+ reg:
+ minItems: 2
+ resets:
+ maxItems: 1
+ else:
+ properties:
+ reg:
+ maxItems: 1
+ resets:
+ minItems: 3
+ required:
+ - fsl,mipi-phy-gpr
+
additionalProperties: false
examples:
--
2.34.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH v4 09/13] media: imx8mq-mipi-csi2: Add imx8mq_plat_data for different compatible strings
2025-04-08 21:52 [PATCH v4 00/13] media: imx8: add camera support Frank Li
` (7 preceding siblings ...)
2025-04-08 21:53 ` [PATCH v4 08/13] media: dt-bindings: nxp,imx8mq-mipi-csi2: Add i.MX8QM(QXP) compatible strings Frank Li
@ 2025-04-08 21:53 ` Frank Li
2025-04-08 21:53 ` [PATCH v4 10/13] media: imx8mq-mipi-csi2: Add support for i.MX8QXP Frank Li
` (4 subsequent siblings)
13 siblings, 0 replies; 30+ messages in thread
From: Frank Li @ 2025-04-08 21:53 UTC (permalink / raw)
To: Laurent Pinchart, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Frank Li,
Rui Miguel Silva, Martin Kepplinger, Purism Kernel Team
Cc: linux-media, devicetree, imx, linux-arm-kernel, linux-kernel,
Robert Chiras, Guoniu.zhou
From: "Guoniu.zhou" <guoniu.zhou@nxp.com>
Introduce `imx8mq_plat_data` along with enable/disable callback operations
to facilitate support for new chips. No functional changes.
Signed-off-by: Guoniu.zhou <guoniu.zhou@nxp.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Change from v3 to v4
- Add Laurent Pinchart review tags
- remove unused 'names'
Change from v2 to v3
- none
change from v1 to v2
- remove internal review tags
---
drivers/media/platform/nxp/imx8mq-mipi-csi2.c | 58 ++++++++++++++++++++-------
1 file changed, 44 insertions(+), 14 deletions(-)
diff --git a/drivers/media/platform/nxp/imx8mq-mipi-csi2.c b/drivers/media/platform/nxp/imx8mq-mipi-csi2.c
index a8bcf60e2f37d..59ec7107b4508 100644
--- a/drivers/media/platform/nxp/imx8mq-mipi-csi2.c
+++ b/drivers/media/platform/nxp/imx8mq-mipi-csi2.c
@@ -62,6 +62,8 @@
#define CSI2RX_CFG_VID_P_FIFO_SEND_LEVEL 0x188
#define CSI2RX_CFG_DISABLE_PAYLOAD_1 0x130
+struct csi_state;
+
enum {
ST_POWERED = 1,
ST_STREAMING = 2,
@@ -83,11 +85,10 @@ static const char * const imx8mq_mipi_csi_clk_id[CSI2_NUM_CLKS] = {
#define CSI2_NUM_CLKS ARRAY_SIZE(imx8mq_mipi_csi_clk_id)
-#define GPR_CSI2_1_RX_ENABLE BIT(13)
-#define GPR_CSI2_1_VID_INTFC_ENB BIT(12)
-#define GPR_CSI2_1_HSEL BIT(10)
-#define GPR_CSI2_1_CONT_CLK_MODE BIT(8)
-#define GPR_CSI2_1_S_PRG_RXHS_SETTLE(x) (((x) & 0x3f) << 2)
+struct imx8mq_plat_data {
+ int (*enable)(struct csi_state *state, u32 hs_settle);
+ void (*disable)(struct csi_state *state);
+};
/*
* The send level configures the number of entries that must accumulate in
@@ -106,6 +107,7 @@ static const char * const imx8mq_mipi_csi_clk_id[CSI2_NUM_CLKS] = {
struct csi_state {
struct device *dev;
+ const struct imx8mq_plat_data *pdata;
void __iomem *regs;
struct clk_bulk_data clks[CSI2_NUM_CLKS];
struct reset_control *rst;
@@ -137,6 +139,34 @@ struct csi2_pix_format {
u8 width;
};
+/* -----------------------------------------------------------------------------
+ * i.MX8MQ GPR
+ */
+
+#define GPR_CSI2_1_RX_ENABLE BIT(13)
+#define GPR_CSI2_1_VID_INTFC_ENB BIT(12)
+#define GPR_CSI2_1_HSEL BIT(10)
+#define GPR_CSI2_1_CONT_CLK_MODE BIT(8)
+#define GPR_CSI2_1_S_PRG_RXHS_SETTLE(x) (((x) & 0x3f) << 2)
+
+static int imx8mq_gpr_enable(struct csi_state *state, u32 hs_settle)
+{
+ regmap_update_bits(state->phy_gpr,
+ state->phy_gpr_reg,
+ 0x3fff,
+ GPR_CSI2_1_RX_ENABLE |
+ GPR_CSI2_1_VID_INTFC_ENB |
+ GPR_CSI2_1_HSEL |
+ GPR_CSI2_1_CONT_CLK_MODE |
+ GPR_CSI2_1_S_PRG_RXHS_SETTLE(hs_settle));
+
+ return 0;
+}
+
+static const struct imx8mq_plat_data imx8mq_data = {
+ .enable = imx8mq_gpr_enable,
+};
+
static const struct csi2_pix_format imx8mq_mipi_csi_formats[] = {
/* RAW (Bayer and greyscale) formats. */
{
@@ -371,14 +401,9 @@ static int imx8mq_mipi_csi_start_stream(struct csi_state *state,
if (ret)
return ret;
- regmap_update_bits(state->phy_gpr,
- state->phy_gpr_reg,
- 0x3fff,
- GPR_CSI2_1_RX_ENABLE |
- GPR_CSI2_1_VID_INTFC_ENB |
- GPR_CSI2_1_HSEL |
- GPR_CSI2_1_CONT_CLK_MODE |
- GPR_CSI2_1_S_PRG_RXHS_SETTLE(hs_settle));
+ ret = state->pdata->enable(state, hs_settle);
+ if (ret)
+ return ret;
return 0;
}
@@ -386,6 +411,9 @@ static int imx8mq_mipi_csi_start_stream(struct csi_state *state,
static void imx8mq_mipi_csi_stop_stream(struct csi_state *state)
{
imx8mq_mipi_csi_write(state, CSI2RX_CFG_DISABLE_DATA_LANES, 0xf);
+
+ if (state->pdata->disable)
+ state->pdata->disable(state);
}
/* -----------------------------------------------------------------------------
@@ -876,6 +904,8 @@ static int imx8mq_mipi_csi_probe(struct platform_device *pdev)
state->dev = dev;
+ state->pdata = of_device_get_match_data(dev);
+
ret = imx8mq_mipi_csi_parse_dt(state);
if (ret < 0) {
dev_err(dev, "Failed to parse device tree: %d\n", ret);
@@ -953,7 +983,7 @@ static void imx8mq_mipi_csi_remove(struct platform_device *pdev)
}
static const struct of_device_id imx8mq_mipi_csi_of_match[] = {
- { .compatible = "fsl,imx8mq-mipi-csi2", },
+ { .compatible = "fsl,imx8mq-mipi-csi2", .data = &imx8mq_data },
{ /* sentinel */ },
};
MODULE_DEVICE_TABLE(of, imx8mq_mipi_csi_of_match);
--
2.34.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH v4 10/13] media: imx8mq-mipi-csi2: Add support for i.MX8QXP
2025-04-08 21:52 [PATCH v4 00/13] media: imx8: add camera support Frank Li
` (8 preceding siblings ...)
2025-04-08 21:53 ` [PATCH v4 09/13] media: imx8mq-mipi-csi2: Add imx8mq_plat_data for different " Frank Li
@ 2025-04-08 21:53 ` Frank Li
2025-04-21 21:41 ` Laurent Pinchart
2025-04-08 21:53 ` [PATCH v4 11/13] arm64: dts: imx8: add capture controller for i.MX8's img subsystem Frank Li
` (3 subsequent siblings)
13 siblings, 1 reply; 30+ messages in thread
From: Frank Li @ 2025-04-08 21:53 UTC (permalink / raw)
To: Laurent Pinchart, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Frank Li,
Rui Miguel Silva, Martin Kepplinger, Purism Kernel Team
Cc: linux-media, devicetree, imx, linux-arm-kernel, linux-kernel,
Robert Chiras, Guoniu.zhou
Add support for i.MX8QXP, which has a dedicated control and status register
(CSR) space. Enable obtaining the second register space and initializing
PHY and link settings accordingly.
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
change from v3 to v4
- remove reset_delay
- sort register field defination
- fix error messag in dev_err_probe
- fix comments
- use true for 1
- regmap_clear_bits(state->phy_gpr, CSI2SS_CTRL_CLK_RESET, CSI2SS_CTRL_CLK_RESET_EN);
in imx8qxp_gpr_disable()
- use regmap_write to clean register at imx8qxp_gpr_enable()
- remove reduntant CSI2SS_PLM_CTRL_POLARITY
- rename register DATA_TYPE to DATA_TYPE_DISABLE_BF
change from v2 to v3
- use dedicate csr reg to control phy and link settings.
Change from v1 to v2
- change 8QM go 8QXP, 8QM will failback to 8QXP to keep consisense with
phy drivers
---
drivers/media/platform/nxp/imx8mq-mipi-csi2.c | 111 ++++++++++++++++++++++++++
1 file changed, 111 insertions(+)
diff --git a/drivers/media/platform/nxp/imx8mq-mipi-csi2.c b/drivers/media/platform/nxp/imx8mq-mipi-csi2.c
index 59ec7107b4508..c6eb6dd0d9e5a 100644
--- a/drivers/media/platform/nxp/imx8mq-mipi-csi2.c
+++ b/drivers/media/platform/nxp/imx8mq-mipi-csi2.c
@@ -5,6 +5,7 @@
* Copyright (C) 2021 Purism SPC
*/
+#include <linux/bitfield.h>
#include <linux/clk.h>
#include <linux/clk-provider.h>
#include <linux/delay.h>
@@ -88,6 +89,7 @@ static const char * const imx8mq_mipi_csi_clk_id[CSI2_NUM_CLKS] = {
struct imx8mq_plat_data {
int (*enable)(struct csi_state *state, u32 hs_settle);
void (*disable)(struct csi_state *state);
+ bool use_reg_csr;
};
/*
@@ -167,6 +169,95 @@ static const struct imx8mq_plat_data imx8mq_data = {
.enable = imx8mq_gpr_enable,
};
+/* -----------------------------------------------------------------------------
+ * i.MX8QXP
+ */
+
+#define CSI2SS_PL_CLK_INTERVAL_US 100
+#define CSI2SS_PL_CLK_TIMEOUT_US 100000
+
+#define CSI2SS_PLM_CTRL 0x0
+#define CSI2SS_PLM_CTRL_ENABLE_PL BIT(0)
+#define CSI2SS_PLM_CTRL_VSYNC_OVERRIDE BIT(9)
+#define CSI2SS_PLM_CTRL_HSYNC_OVERRIDE BIT(10)
+#define CSI2SS_PLM_CTRL_VALID_OVERRIDE BIT(11)
+#define CSI2SS_PLM_CTRL_POLARITY_HIGH BIT(12)
+#define CSI2SS_PLM_CTRL_PL_CLK_RUN BIT(31)
+
+#define CSI2SS_PHY_CTRL 0x4
+#define CSI2SS_PHY_CTRL_RX_ENABLE BIT(0)
+#define CSI2SS_PHY_CTRL_AUTO_PD_EN BIT(1)
+#define CSI2SS_PHY_CTRL_DDRCLK_EN BIT(2)
+#define CSI2SS_PHY_CTRL_CONT_CLK_MODE BIT(3)
+#define CSI2SS_PHY_CTRL_RX_HS_SETTLE_MASK GENMASK(9, 4)
+#define CSI2SS_PHY_CTRL_RTERM_SEL BIT(21)
+#define CSI2SS_PHY_CTRL_PD BIT(22)
+
+#define CSI2SS_DATA_TYPE_DISABLE_BF 0x38
+#define CSI2SS_DATA_TYPE_DISABLE_BF_MASK GENMASK(23, 0)
+
+#define CSI2SS_CTRL_CLK_RESET 0x44
+#define CSI2SS_CTRL_CLK_RESET_EN BIT(0)
+
+static int imx8qxp_gpr_enable(struct csi_state *state, u32 hs_settle)
+{
+ int ret;
+ u32 val;
+
+ /* Clear format */
+ regmap_clear_bits(state->phy_gpr,
+ CSI2SS_DATA_TYPE_DISABLE_BF, CSI2SS_DATA_TYPE_DISABLE_BF_MASK);
+
+ regmap_write(state->phy_gpr, CSI2SS_PLM_CTRL, 0x0);
+ regmap_write(state->phy_gpr, CSI2SS_PHY_CTRL, 0x0);
+
+ regmap_write(state->phy_gpr, CSI2SS_PHY_CTRL,
+ FIELD_PREP(CSI2SS_PHY_CTRL_RX_HS_SETTLE_MASK, hs_settle) |
+ CSI2SS_PHY_CTRL_RX_ENABLE | CSI2SS_PHY_CTRL_DDRCLK_EN |
+ CSI2SS_PHY_CTRL_CONT_CLK_MODE | CSI2SS_PHY_CTRL_PD |
+ CSI2SS_PHY_CTRL_RTERM_SEL | CSI2SS_PHY_CTRL_AUTO_PD_EN);
+
+ ret = regmap_read_poll_timeout(state->phy_gpr, CSI2SS_PLM_CTRL,
+ val, !(val & CSI2SS_PLM_CTRL_PL_CLK_RUN),
+ CSI2SS_PL_CLK_INTERVAL_US,
+ CSI2SS_PL_CLK_TIMEOUT_US);
+
+ if (ret) {
+ dev_err(state->dev, "Timeout waiting for Pixel-Link clock");
+ return ret;
+ }
+
+ /* Enable Pixel link Master */
+ regmap_set_bits(state->phy_gpr, CSI2SS_PLM_CTRL,
+ CSI2SS_PLM_CTRL_ENABLE_PL | CSI2SS_PLM_CTRL_VALID_OVERRIDE);
+
+ /* PHY Enable */
+ regmap_clear_bits(state->phy_gpr, CSI2SS_PHY_CTRL,
+ CSI2SS_PHY_CTRL_PD | CSI2SS_PLM_CTRL_POLARITY_HIGH);
+
+ /* Release Reset */
+ regmap_set_bits(state->phy_gpr, CSI2SS_CTRL_CLK_RESET, CSI2SS_CTRL_CLK_RESET_EN);
+
+ return ret;
+}
+
+static void imx8qxp_gpr_disable(struct csi_state *state)
+{
+ /* Disable Pixel Link */
+ regmap_write(state->phy_gpr, CSI2SS_PLM_CTRL, 0x0);
+
+ /* Disable PHY */
+ regmap_write(state->phy_gpr, CSI2SS_PHY_CTRL, 0x0);
+
+ regmap_clear_bits(state->phy_gpr, CSI2SS_CTRL_CLK_RESET, CSI2SS_CTRL_CLK_RESET_EN);
+};
+
+static const struct imx8mq_plat_data imx8qxp_data = {
+ .enable = imx8qxp_gpr_enable,
+ .disable = imx8qxp_gpr_disable,
+ .use_reg_csr = true,
+};
+
static const struct csi2_pix_format imx8mq_mipi_csi_formats[] = {
/* RAW (Bayer and greyscale) formats. */
{
@@ -865,6 +956,25 @@ static int imx8mq_mipi_csi_parse_dt(struct csi_state *state)
return PTR_ERR(state->rst);
}
+ if (state->pdata->use_reg_csr) {
+ const struct regmap_config regmap_config = {
+ .reg_bits = 32,
+ .val_bits = 32,
+ .reg_stride = 4,
+ };
+ void __iomem *base;
+
+ base = devm_platform_ioremap_resource(to_platform_device(dev), 1);
+ if (IS_ERR(base))
+ return dev_err_probe(dev, IS_ERR(base), "Missing CSR register\n");
+
+ state->phy_gpr = devm_regmap_init_mmio(dev, base, ®map_config);
+ if (IS_ERR(state->phy_gpr))
+ return dev_err_probe(dev, PTR_ERR(state->phy_gpr),
+ "Failed to init CSI MMIO regmap\n");
+ return 0;
+ }
+
ret = of_property_read_u32_array(np, "fsl,mipi-phy-gpr", out_val,
ARRAY_SIZE(out_val));
if (ret) {
@@ -984,6 +1094,7 @@ static void imx8mq_mipi_csi_remove(struct platform_device *pdev)
static const struct of_device_id imx8mq_mipi_csi_of_match[] = {
{ .compatible = "fsl,imx8mq-mipi-csi2", .data = &imx8mq_data },
+ { .compatible = "fsl,imx8qxp-mipi-csi2", .data = &imx8qxp_data },
{ /* sentinel */ },
};
MODULE_DEVICE_TABLE(of, imx8mq_mipi_csi_of_match);
--
2.34.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH v4 10/13] media: imx8mq-mipi-csi2: Add support for i.MX8QXP
2025-04-08 21:53 ` [PATCH v4 10/13] media: imx8mq-mipi-csi2: Add support for i.MX8QXP Frank Li
@ 2025-04-21 21:41 ` Laurent Pinchart
0 siblings, 0 replies; 30+ messages in thread
From: Laurent Pinchart @ 2025-04-21 21:41 UTC (permalink / raw)
To: Frank Li
Cc: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Rui Miguel Silva, Martin Kepplinger,
Purism Kernel Team, linux-media, devicetree, imx,
linux-arm-kernel, linux-kernel, Robert Chiras, Guoniu.zhou
Hi Frank,
Thank you for the patch.
On Tue, Apr 08, 2025 at 05:53:08PM -0400, Frank Li wrote:
> Add support for i.MX8QXP, which has a dedicated control and status register
> (CSR) space. Enable obtaining the second register space and initializing
> PHY and link settings accordingly.
>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> change from v3 to v4
> - remove reset_delay
> - sort register field defination
> - fix error messag in dev_err_probe
> - fix comments
> - use true for 1
> - regmap_clear_bits(state->phy_gpr, CSI2SS_CTRL_CLK_RESET, CSI2SS_CTRL_CLK_RESET_EN);
> in imx8qxp_gpr_disable()
> - use regmap_write to clean register at imx8qxp_gpr_enable()
> - remove reduntant CSI2SS_PLM_CTRL_POLARITY
> - rename register DATA_TYPE to DATA_TYPE_DISABLE_BF
>
> change from v2 to v3
> - use dedicate csr reg to control phy and link settings.
>
> Change from v1 to v2
> - change 8QM go 8QXP, 8QM will failback to 8QXP to keep consisense with
> phy drivers
> ---
> drivers/media/platform/nxp/imx8mq-mipi-csi2.c | 111 ++++++++++++++++++++++++++
> 1 file changed, 111 insertions(+)
>
> diff --git a/drivers/media/platform/nxp/imx8mq-mipi-csi2.c b/drivers/media/platform/nxp/imx8mq-mipi-csi2.c
> index 59ec7107b4508..c6eb6dd0d9e5a 100644
> --- a/drivers/media/platform/nxp/imx8mq-mipi-csi2.c
> +++ b/drivers/media/platform/nxp/imx8mq-mipi-csi2.c
> @@ -5,6 +5,7 @@
> * Copyright (C) 2021 Purism SPC
> */
>
> +#include <linux/bitfield.h>
> #include <linux/clk.h>
> #include <linux/clk-provider.h>
> #include <linux/delay.h>
> @@ -88,6 +89,7 @@ static const char * const imx8mq_mipi_csi_clk_id[CSI2_NUM_CLKS] = {
> struct imx8mq_plat_data {
> int (*enable)(struct csi_state *state, u32 hs_settle);
> void (*disable)(struct csi_state *state);
> + bool use_reg_csr;
> };
>
> /*
> @@ -167,6 +169,95 @@ static const struct imx8mq_plat_data imx8mq_data = {
> .enable = imx8mq_gpr_enable,
> };
>
> +/* -----------------------------------------------------------------------------
> + * i.MX8QXP
> + */
> +
> +#define CSI2SS_PL_CLK_INTERVAL_US 100
> +#define CSI2SS_PL_CLK_TIMEOUT_US 100000
> +
> +#define CSI2SS_PLM_CTRL 0x0
> +#define CSI2SS_PLM_CTRL_ENABLE_PL BIT(0)
> +#define CSI2SS_PLM_CTRL_VSYNC_OVERRIDE BIT(9)
> +#define CSI2SS_PLM_CTRL_HSYNC_OVERRIDE BIT(10)
> +#define CSI2SS_PLM_CTRL_VALID_OVERRIDE BIT(11)
> +#define CSI2SS_PLM_CTRL_POLARITY_HIGH BIT(12)
> +#define CSI2SS_PLM_CTRL_PL_CLK_RUN BIT(31)
> +
> +#define CSI2SS_PHY_CTRL 0x4
> +#define CSI2SS_PHY_CTRL_RX_ENABLE BIT(0)
> +#define CSI2SS_PHY_CTRL_AUTO_PD_EN BIT(1)
> +#define CSI2SS_PHY_CTRL_DDRCLK_EN BIT(2)
> +#define CSI2SS_PHY_CTRL_CONT_CLK_MODE BIT(3)
> +#define CSI2SS_PHY_CTRL_RX_HS_SETTLE_MASK GENMASK(9, 4)
> +#define CSI2SS_PHY_CTRL_RTERM_SEL BIT(21)
> +#define CSI2SS_PHY_CTRL_PD BIT(22)
> +
> +#define CSI2SS_DATA_TYPE_DISABLE_BF 0x38
> +#define CSI2SS_DATA_TYPE_DISABLE_BF_MASK GENMASK(23, 0)
> +
> +#define CSI2SS_CTRL_CLK_RESET 0x44
> +#define CSI2SS_CTRL_CLK_RESET_EN BIT(0)
> +
> +static int imx8qxp_gpr_enable(struct csi_state *state, u32 hs_settle)
> +{
> + int ret;
> + u32 val;
> +
> + /* Clear format */
> + regmap_clear_bits(state->phy_gpr,
> + CSI2SS_DATA_TYPE_DISABLE_BF, CSI2SS_DATA_TYPE_DISABLE_BF_MASK);
regmap_clear_bits(state->phy_gpr, CSI2SS_DATA_TYPE_DISABLE_BF,
CSI2SS_DATA_TYPE_DISABLE_BF_MASK);
> +
> + regmap_write(state->phy_gpr, CSI2SS_PLM_CTRL, 0x0);
> + regmap_write(state->phy_gpr, CSI2SS_PHY_CTRL, 0x0);
This write to CSI2SS_PHY_CTRL is overridden by the next line. Is it
needed ?
> +
> + regmap_write(state->phy_gpr, CSI2SS_PHY_CTRL,
> + FIELD_PREP(CSI2SS_PHY_CTRL_RX_HS_SETTLE_MASK, hs_settle) |
> + CSI2SS_PHY_CTRL_RX_ENABLE | CSI2SS_PHY_CTRL_DDRCLK_EN |
> + CSI2SS_PHY_CTRL_CONT_CLK_MODE | CSI2SS_PHY_CTRL_PD |
> + CSI2SS_PHY_CTRL_RTERM_SEL | CSI2SS_PHY_CTRL_AUTO_PD_EN);
> +
> + ret = regmap_read_poll_timeout(state->phy_gpr, CSI2SS_PLM_CTRL,
> + val, !(val & CSI2SS_PLM_CTRL_PL_CLK_RUN),
> + CSI2SS_PL_CLK_INTERVAL_US,
> + CSI2SS_PL_CLK_TIMEOUT_US);
> +
> + if (ret) {
> + dev_err(state->dev, "Timeout waiting for Pixel-Link clock");
Missing \n at end of string.
> + return ret;
> + }
> +
> + /* Enable Pixel link Master */
> + regmap_set_bits(state->phy_gpr, CSI2SS_PLM_CTRL,
> + CSI2SS_PLM_CTRL_ENABLE_PL | CSI2SS_PLM_CTRL_VALID_OVERRIDE);
> +
> + /* PHY Enable */
> + regmap_clear_bits(state->phy_gpr, CSI2SS_PHY_CTRL,
> + CSI2SS_PHY_CTRL_PD | CSI2SS_PLM_CTRL_POLARITY_HIGH);
> +
> + /* Release Reset */
> + regmap_set_bits(state->phy_gpr, CSI2SS_CTRL_CLK_RESET, CSI2SS_CTRL_CLK_RESET_EN);
> +
> + return ret;
> +}
> +
> +static void imx8qxp_gpr_disable(struct csi_state *state)
> +{
> + /* Disable Pixel Link */
> + regmap_write(state->phy_gpr, CSI2SS_PLM_CTRL, 0x0);
> +
> + /* Disable PHY */
> + regmap_write(state->phy_gpr, CSI2SS_PHY_CTRL, 0x0);
> +
> + regmap_clear_bits(state->phy_gpr, CSI2SS_CTRL_CLK_RESET, CSI2SS_CTRL_CLK_RESET_EN);
regmap_clear_bits(state->phy_gpr, CSI2SS_CTRL_CLK_RESET,
CSI2SS_CTRL_CLK_RESET_EN);
With those small issues addressed,
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> +};
> +
> +static const struct imx8mq_plat_data imx8qxp_data = {
> + .enable = imx8qxp_gpr_enable,
> + .disable = imx8qxp_gpr_disable,
> + .use_reg_csr = true,
> +};
> +
> static const struct csi2_pix_format imx8mq_mipi_csi_formats[] = {
> /* RAW (Bayer and greyscale) formats. */
> {
> @@ -865,6 +956,25 @@ static int imx8mq_mipi_csi_parse_dt(struct csi_state *state)
> return PTR_ERR(state->rst);
> }
>
> + if (state->pdata->use_reg_csr) {
> + const struct regmap_config regmap_config = {
> + .reg_bits = 32,
> + .val_bits = 32,
> + .reg_stride = 4,
> + };
> + void __iomem *base;
> +
> + base = devm_platform_ioremap_resource(to_platform_device(dev), 1);
> + if (IS_ERR(base))
> + return dev_err_probe(dev, IS_ERR(base), "Missing CSR register\n");
> +
> + state->phy_gpr = devm_regmap_init_mmio(dev, base, ®map_config);
> + if (IS_ERR(state->phy_gpr))
> + return dev_err_probe(dev, PTR_ERR(state->phy_gpr),
> + "Failed to init CSI MMIO regmap\n");
> + return 0;
> + }
> +
> ret = of_property_read_u32_array(np, "fsl,mipi-phy-gpr", out_val,
> ARRAY_SIZE(out_val));
> if (ret) {
> @@ -984,6 +1094,7 @@ static void imx8mq_mipi_csi_remove(struct platform_device *pdev)
>
> static const struct of_device_id imx8mq_mipi_csi_of_match[] = {
> { .compatible = "fsl,imx8mq-mipi-csi2", .data = &imx8mq_data },
> + { .compatible = "fsl,imx8qxp-mipi-csi2", .data = &imx8qxp_data },
> { /* sentinel */ },
> };
> MODULE_DEVICE_TABLE(of, imx8mq_mipi_csi_of_match);
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v4 11/13] arm64: dts: imx8: add capture controller for i.MX8's img subsystem
2025-04-08 21:52 [PATCH v4 00/13] media: imx8: add camera support Frank Li
` (9 preceding siblings ...)
2025-04-08 21:53 ` [PATCH v4 10/13] media: imx8mq-mipi-csi2: Add support for i.MX8QXP Frank Li
@ 2025-04-08 21:53 ` Frank Li
2025-04-21 21:46 ` Laurent Pinchart
2025-04-08 21:53 ` [PATCH v4 12/13] arm64: dts: imx8q: add linux,cma node for imx8qm-mek and imx8qxp-mek Frank Li
` (2 subsequent siblings)
13 siblings, 1 reply; 30+ messages in thread
From: Frank Li @ 2025-04-08 21:53 UTC (permalink / raw)
To: Laurent Pinchart, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Frank Li,
Rui Miguel Silva, Martin Kepplinger, Purism Kernel Team
Cc: linux-media, devicetree, imx, linux-arm-kernel, linux-kernel,
Robert Chiras, Guoniu.zhou
Add CSI related nodes (i2c, irqsteer, csi, lpcg) for i.MX8 img subsystem.
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Change from v3 to v4
- remove unused clock clock-img-axi
- add ports information for isi and csi
- Fix 8qxp clock, irq and power domain
- Fix reg size
Change from v2 to v3
- remove phy and put csr register space under mipi csi2
change from v1 to v2
- move scu reset under scu node
- add 8qm comaptible string for mipi csi2 and mipi csi phys.
---
arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi | 362 ++++++++++++++++++++++
arch/arm64/boot/dts/freescale/imx8qm-ss-img.dtsi | 85 +++++
arch/arm64/boot/dts/freescale/imx8qm.dtsi | 5 +
arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi | 86 +++++
arch/arm64/boot/dts/freescale/imx8qxp.dtsi | 5 +
5 files changed, 543 insertions(+)
diff --git a/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi b/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
index d39242c1b9f79..2cf0f7208350a 100644
--- a/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
@@ -10,12 +10,264 @@ img_ipg_clk: clock-img-ipg {
clock-output-names = "img_ipg_clk";
};
+img_pxl_clk: clock-img-pxl {
+ compatible = "fixed-clock";
+ #clock-cells = <0>;
+ clock-frequency = <600000000>;
+ clock-output-names = "img_pxl_clk";
+};
+
img_subsys: bus@58000000 {
compatible = "simple-bus";
#address-cells = <1>;
#size-cells = <1>;
ranges = <0x58000000 0x0 0x58000000 0x1000000>;
+ isi: isi@58100000 {
+ reg = <0x58100000 0x80000>;
+ interrupts = <GIC_SPI 297 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 298 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 299 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 300 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 301 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 302 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 303 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 304 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&pdma0_lpcg IMX_LPCG_CLK_0>,
+ <&pdma1_lpcg IMX_LPCG_CLK_0>,
+ <&pdma2_lpcg IMX_LPCG_CLK_0>,
+ <&pdma3_lpcg IMX_LPCG_CLK_0>,
+ <&pdma4_lpcg IMX_LPCG_CLK_0>,
+ <&pdma5_lpcg IMX_LPCG_CLK_0>,
+ <&pdma6_lpcg IMX_LPCG_CLK_0>,
+ <&pdma7_lpcg IMX_LPCG_CLK_0>;
+ clock-names = "per0", "per1", "per2", "per3",
+ "per4", "per5", "per6", "per7";
+ interrupt-parent = <&gic>;
+ power-domains = <&pd IMX_SC_R_ISI_CH0>,
+ <&pd IMX_SC_R_ISI_CH1>,
+ <&pd IMX_SC_R_ISI_CH2>,
+ <&pd IMX_SC_R_ISI_CH3>,
+ <&pd IMX_SC_R_ISI_CH4>,
+ <&pd IMX_SC_R_ISI_CH5>,
+ <&pd IMX_SC_R_ISI_CH6>,
+ <&pd IMX_SC_R_ISI_CH7>;
+ status = "disabled";
+ };
+
+ irqsteer_csi0: irqsteer@58220000 {
+ compatible = "fsl,imx8qm-irqsteer", "fsl,imx-irqsteer";
+ reg = <0x58220000 0x1000>;
+ #interrupt-cells = <1>;
+ interrupt-controller;
+ interrupts = <GIC_SPI 320 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&img_ipg_clk>;
+ clock-names = "ipg";
+ interrupt-parent = <&gic>;
+ power-domains = <&pd IMX_SC_R_CSI_0>;
+ fsl,channel = <0>;
+ fsl,num-irqs = <32>;
+ status = "disabled";
+ };
+
+ gpio0_mipi_csi0: gpio@58222000 {
+ compatible = "fsl,imx8qm-gpio", "fsl,imx35-gpio";
+ reg = <0x58222000 0x1000>;
+ #interrupt-cells = <2>;
+ interrupt-controller;
+ interrupts = <0>;
+ #gpio-cells = <2>;
+ gpio-controller;
+ interrupt-parent = <&irqsteer_csi0>;
+ power-domains = <&pd IMX_SC_R_CSI_0>;
+ };
+
+ csi0_core_lpcg: clock-controller@58223018 {
+ compatible = "fsl,imx8qxp-lpcg";
+ reg = <0x58223018 0x4>;
+ clocks = <&clk IMX_SC_R_CSI_0 IMX_SC_PM_CLK_PER>;
+ #clock-cells = <1>;
+ clock-indices = <IMX_LPCG_CLK_4>;
+ clock-output-names = "csi0_lpcg_core_clk";
+ power-domains = <&pd IMX_SC_R_ISI_CH0>;
+ };
+
+ csi0_esc_lpcg: clock-controller@5822301c {
+ compatible = "fsl,imx8qxp-lpcg";
+ reg = <0x5822301c 0x4>;
+ clocks = <&clk IMX_SC_R_CSI_0 IMX_SC_PM_CLK_MISC>;
+ #clock-cells = <1>;
+ clock-indices = <IMX_LPCG_CLK_4>;
+ clock-output-names = "csi0_lpcg_esc_clk";
+ power-domains = <&pd IMX_SC_R_ISI_CH0>;
+ };
+
+ i2c_mipi_csi0: i2c@58226000 {
+ compatible = "fsl,imx8qxp-lpi2c", "fsl,imx7ulp-lpi2c";
+ reg = <0x58226000 0x1000>;
+ interrupts = <8>;
+ clocks = <&clk IMX_SC_R_CSI_0_I2C_0 IMX_SC_PM_CLK_PER>,
+ <&img_ipg_clk>;
+ clock-names = "per", "ipg";
+ assigned-clocks = <&clk IMX_SC_R_CSI_0_I2C_0 IMX_SC_PM_CLK_PER>;
+ assigned-clock-rates = <24000000>;
+ interrupt-parent = <&irqsteer_csi0>;
+ power-domains = <&pd IMX_SC_R_CSI_0_I2C_0>;
+ status = "disabled";
+ };
+
+ mipi_csi_0: csi@58227000 {
+ compatible = "fsl,imx8qxp-mipi-csi2";
+ reg = <0x58227000 0x1000>,
+ <0x58221000 0x1000>;
+ clocks = <&csi0_core_lpcg IMX_LPCG_CLK_4>,
+ <&csi0_esc_lpcg IMX_LPCG_CLK_4>,
+ <&csi0_pxl_lpcg IMX_LPCG_CLK_0>;
+ clock-names = "core", "esc", "ui";
+ assigned-clocks = <&csi0_core_lpcg IMX_LPCG_CLK_4>,
+ <&csi0_esc_lpcg IMX_LPCG_CLK_4>;
+ assigned-clock-rates = <360000000>, <72000000>;
+ power-domains = <&pd IMX_SC_R_ISI_CH0>;
+ resets = <&scu_reset IMX_SC_R_CSI_0>;
+ status = "disabled";
+ };
+
+ irqsteer_csi1: irqsteer@58240000 {
+ compatible = "fsl,imx8qm-irqsteer", "fsl,imx-irqsteer";
+ reg = <0x58240000 0x1000>;
+ #interrupt-cells = <1>;
+ interrupt-controller;
+ interrupts = <GIC_SPI 321 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&img_ipg_clk>;
+ clock-names = "ipg";
+ interrupt-parent = <&gic>;
+ power-domains = <&pd IMX_SC_R_CSI_1>;
+ fsl,channel = <0>;
+ fsl,num-irqs = <32>;
+ status = "disabled";
+ };
+
+ gpio0_mipi_csi1: gpio@58242000 {
+ compatible = "fsl,imx8qm-gpio", "fsl,imx35-gpio";
+ reg = <0x58242000 0x1000>;
+ #interrupt-cells = <2>;
+ interrupt-controller;
+ interrupts = <0>;
+ #gpio-cells = <2>;
+ gpio-controller;
+ interrupt-parent = <&irqsteer_csi1>;
+ power-domains = <&pd IMX_SC_R_CSI_1>;
+ };
+
+ csi1_core_lpcg: clock-controller@58243018 {
+ compatible = "fsl,imx8qxp-lpcg";
+ reg = <0x58243018 0x4>;
+ clocks = <&clk IMX_SC_R_CSI_1 IMX_SC_PM_CLK_PER>;
+ #clock-cells = <1>;
+ clock-indices = <IMX_LPCG_CLK_4>;
+ clock-output-names = "csi1_lpcg_core_clk";
+ power-domains = <&pd IMX_SC_R_ISI_CH0>;
+ };
+
+ csi1_esc_lpcg: clock-controller@5824301c {
+ compatible = "fsl,imx8qxp-lpcg";
+ reg = <0x5824301c 0x4>;
+ clocks = <&clk IMX_SC_R_CSI_1 IMX_SC_PM_CLK_MISC>;
+ #clock-cells = <1>;
+ clock-indices = <IMX_LPCG_CLK_4>;
+ clock-output-names = "csi1_lpcg_esc_clk";
+ power-domains = <&pd IMX_SC_R_ISI_CH0>;
+ };
+
+ i2c_mipi_csi1: i2c@58246000 {
+ compatible = "fsl,imx8qxp-lpi2c", "fsl,imx7ulp-lpi2c";
+ reg = <0x58246000 0x1000>;
+ interrupts = <8>;
+ clocks = <&clk IMX_SC_R_CSI_1_I2C_0 IMX_SC_PM_CLK_PER>,
+ <&img_ipg_clk>;
+ clock-names = "per", "ipg";
+ assigned-clocks = <&clk IMX_SC_R_CSI_1_I2C_0 IMX_SC_PM_CLK_PER>;
+ assigned-clock-rates = <24000000>;
+ interrupt-parent = <&irqsteer_csi1>;
+ power-domains = <&pd IMX_SC_R_CSI_1_I2C_0>;
+ status = "disabled";
+ };
+
+ mipi_csi_1: csi@58247000 {
+ compatible = "fsl,imx8qxp-mipi-csi2";
+ reg = <0x58247000 0x1000>,
+ <0x58241000 0x1000>;
+ clocks = <&csi1_core_lpcg IMX_LPCG_CLK_4>,
+ <&csi1_esc_lpcg IMX_LPCG_CLK_4>,
+ <&csi1_pxl_lpcg IMX_LPCG_CLK_0>;
+ clock-names = "core", "esc", "ui";
+ assigned-clocks = <&csi1_core_lpcg IMX_LPCG_CLK_4>,
+ <&csi1_esc_lpcg IMX_LPCG_CLK_4>;
+ assigned-clock-rates = <360000000>, <72000000>;
+ power-domains = <&pd IMX_SC_R_ISI_CH0>;
+ resets = <&scu_reset IMX_SC_R_CSI_1>;
+ status = "disabled";
+ };
+
+ irqsteer_parallel: irqsteer@58260000 {
+ compatible = "fsl,imx8qm-irqsteer", "fsl,imx-irqsteer";
+ reg = <0x58260000 0x1000>;
+ #interrupt-cells = <1>;
+ interrupt-controller;
+ interrupts = <GIC_SPI 322 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clk_dummy>;
+ clock-names = "ipg";
+ interrupt-parent = <&gic>;
+ power-domains = <&pd IMX_SC_R_PI_0>;
+ fsl,channel = <0>;
+ fsl,num-irqs = <32>;
+ status = "disabled";
+ };
+
+ pi0_ipg_lpcg: clock-controller@58263004 {
+ compatible = "fsl,imx8qxp-lpcg";
+ reg = <0x58263004 0x4>;
+ clocks = <&clk IMX_SC_R_PI_0 IMX_SC_PM_CLK_PER>;
+ #clock-cells = <1>;
+ clock-indices = <IMX_LPCG_CLK_4>;
+ clock-output-names = "pi0_lpcg_ipg_clk";
+ power-domains = <&pd IMX_SC_R_ISI_CH0>;
+ };
+
+ pi0_pxl_lpcg: clock-controller@58263018 {
+ compatible = "fsl,imx8qxp-lpcg";
+ reg = <0x58263018 0x4>;
+ clocks = <&clk IMX_SC_R_PI_0 IMX_SC_PM_CLK_PER>;
+ #clock-cells = <1>;
+ clock-indices = <IMX_LPCG_CLK_0>;
+ clock-output-names = "pi0_lpcg_pxl_clk";
+ power-domains = <&pd IMX_SC_R_ISI_CH0>;
+ };
+
+ pi0_misc_lpcg: clock-controller@5826301c {
+ compatible = "fsl,imx8qxp-lpcg";
+ reg = <0x5826301c 0x4>;
+ clocks = <&clk IMX_SC_R_PI_0 IMX_SC_PM_CLK_MISC0>;
+ #clock-cells = <1>;
+ clock-indices = <IMX_LPCG_CLK_0>;
+ clock-output-names = "pi0_lpcg_misc_clk";
+ power-domains = <&pd IMX_SC_R_ISI_CH0>;
+ };
+
+ i2c0_parallel: i2c@58266000 {
+ compatible = "fsl,imx8qxp-lpi2c", "fsl,imx7ulp-lpi2c";
+ reg = <0x58266000 0x1000>;
+ interrupts = <8>;
+ clocks = <&clk IMX_SC_R_PI_0_I2C_0 IMX_SC_PM_CLK_PER>,
+ <&img_ipg_clk>;
+ clock-names = "per", "ipg";
+ assigned-clocks = <&clk IMX_SC_R_PI_0_I2C_0 IMX_SC_PM_CLK_PER>;
+ assigned-clock-rates = <24000000>;
+ interrupt-parent = <&irqsteer_parallel>;
+ power-domains = <&pd IMX_SC_R_PI_0_I2C_0>;
+ status = "disabled";
+ };
+
jpegdec: jpegdec@58400000 {
reg = <0x58400000 0x00050000>;
interrupts = <GIC_SPI 309 IRQ_TYPE_LEVEL_HIGH>;
@@ -40,6 +292,116 @@ jpegenc: jpegenc@58450000 {
<&pd IMX_SC_R_MJPEG_ENC_S0>;
};
+ pdma0_lpcg: clock-controller@58500000 {
+ compatible = "fsl,imx8qxp-lpcg";
+ reg = <0x58500000 0x10000>;
+ clocks = <&img_pxl_clk>;
+ #clock-cells = <1>;
+ clock-indices = <IMX_LPCG_CLK_0>;
+ clock-output-names = "pdma0_lpcg_clk";
+ power-domains = <&pd IMX_SC_R_ISI_CH0>;
+ };
+
+ pdma1_lpcg: clock-controller@58510000 {
+ compatible = "fsl,imx8qxp-lpcg";
+ reg = <0x58510000 0x10000>;
+ clocks = <&img_pxl_clk>;
+ #clock-cells = <1>;
+ clock-indices = <IMX_LPCG_CLK_0>;
+ clock-output-names = "pdma1_lpcg_clk";
+ power-domains = <&pd IMX_SC_R_ISI_CH1>;
+ };
+
+ pdma2_lpcg: clock-controller@58520000 {
+ compatible = "fsl,imx8qxp-lpcg";
+ reg = <0x58520000 0x10000>;
+ clocks = <&img_pxl_clk>;
+ #clock-cells = <1>;
+ clock-indices = <IMX_LPCG_CLK_0>;
+ clock-output-names = "pdma2_lpcg_clk";
+ power-domains = <&pd IMX_SC_R_ISI_CH2>;
+ };
+
+ pdma3_lpcg: clock-controller@58530000 {
+ compatible = "fsl,imx8qxp-lpcg";
+ reg = <0x58530000 0x10000>;
+ clocks = <&img_pxl_clk>;
+ #clock-cells = <1>;
+ clock-indices = <IMX_LPCG_CLK_0>;
+ clock-output-names = "pdma3_lpcg_clk";
+ power-domains = <&pd IMX_SC_R_ISI_CH3>;
+ };
+
+ pdma4_lpcg: clock-controller@58540000 {
+ compatible = "fsl,imx8qxp-lpcg";
+ reg = <0x58540000 0x10000>;
+ clocks = <&img_pxl_clk>;
+ #clock-cells = <1>;
+ clock-indices = <IMX_LPCG_CLK_0>;
+ clock-output-names = "pdma4_lpcg_clk";
+ power-domains = <&pd IMX_SC_R_ISI_CH4>;
+ };
+
+ pdma5_lpcg: clock-controller@58550000 {
+ compatible = "fsl,imx8qxp-lpcg";
+ reg = <0x58550000 0x10000>;
+ clocks = <&img_pxl_clk>;
+ #clock-cells = <1>;
+ clock-indices = <IMX_LPCG_CLK_0>;
+ clock-output-names = "pdma5_lpcg_clk";
+ power-domains = <&pd IMX_SC_R_ISI_CH5>;
+ };
+
+ pdma6_lpcg: clock-controller@58560000 {
+ compatible = "fsl,imx8qxp-lpcg";
+ reg = <0x58560000 0x10000>;
+ clocks = <&img_pxl_clk>;
+ #clock-cells = <1>;
+ clock-indices = <IMX_LPCG_CLK_0>;
+ clock-output-names = "pdma6_lpcg_clk";
+ power-domains = <&pd IMX_SC_R_ISI_CH6>;
+ };
+
+ pdma7_lpcg: clock-controller@58570000 {
+ compatible = "fsl,imx8qxp-lpcg";
+ reg = <0x58570000 0x10000>;
+ clocks = <&img_pxl_clk>;
+ #clock-cells = <1>;
+ clock-indices = <IMX_LPCG_CLK_0>;
+ clock-output-names = "pdma7_lpcg_clk";
+ power-domains = <&pd IMX_SC_R_ISI_CH7>;
+ };
+
+ csi0_pxl_lpcg: clock-controller@58580000 {
+ compatible = "fsl,imx8qxp-lpcg";
+ reg = <0x58580000 0x10000>;
+ clocks = <&img_pxl_clk>;
+ #clock-cells = <1>;
+ clock-indices = <IMX_LPCG_CLK_0>;
+ clock-output-names = "csi0_lpcg_pxl_clk";
+ power-domains = <&pd IMX_SC_R_CSI_0>;
+ };
+
+ csi1_pxl_lpcg: clock-controller@58590000 {
+ compatible = "fsl,imx8qxp-lpcg";
+ reg = <0x58590000 0x10000>;
+ clocks = <&img_pxl_clk>;
+ #clock-cells = <1>;
+ clock-indices = <IMX_LPCG_CLK_0>;
+ clock-output-names = "csi1_lpcg_pxl_clk";
+ power-domains = <&pd IMX_SC_R_CSI_1>;
+ };
+
+ hdmi_rx_pxl_link_lpcg: clock-controller@585a0000 {
+ compatible = "fsl,imx8qxp-lpcg";
+ reg = <0x585a0000 0x10000>;
+ clocks = <&img_pxl_clk>;
+ #clock-cells = <1>;
+ clock-indices = <IMX_LPCG_CLK_0>;
+ clock-output-names = "hdmi_rx_lpcg_pxl_link_clk";
+ power-domains = <&pd IMX_SC_R_HDMI_RX>;
+ };
+
img_jpeg_dec_lpcg: clock-controller@585d0000 {
compatible = "fsl,imx8qxp-lpcg";
reg = <0x585d0000 0x10000>;
diff --git a/arch/arm64/boot/dts/freescale/imx8qm-ss-img.dtsi b/arch/arm64/boot/dts/freescale/imx8qm-ss-img.dtsi
index 2bbdacb1313f9..ce2ab67c6cba3 100644
--- a/arch/arm64/boot/dts/freescale/imx8qm-ss-img.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8qm-ss-img.dtsi
@@ -3,6 +3,31 @@
* Copyright 2021 NXP
*/
+&isi {
+ compatible = "fsl,imx8qm-isi";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@2 {
+ reg = <2>;
+
+ isi_in_2: endpoint {
+ remote-endpoint = <&mipi_csi0_out>;
+ };
+ };
+
+ port@3 {
+ reg = <3>;
+
+ isi_in_3: endpoint {
+ remote-endpoint = <&mipi_csi1_out>;
+ };
+ };
+ };
+};
+
&jpegdec {
compatible = "nxp,imx8qm-jpgdec", "nxp,imx8qxp-jpgdec";
};
@@ -10,3 +35,63 @@ &jpegdec {
&jpegenc {
compatible = "nxp,imx8qm-jpgenc", "nxp,imx8qxp-jpgenc";
};
+
+&mipi_csi_0 {
+ compatible = "fsl,imx8qm-mipi-csi2", "fsl,imx8qxp-mipi-csi2";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+
+ mipi_csi0_in: endpoint {
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+
+ mipi_csi0_out: endpoint {
+ remote-endpoint = <&isi_in_2>;
+ };
+ };
+ };
+};
+
+&mipi_csi_1 {
+ compatible = "fsl,imx8qm-mipi-csi2", "fsl,imx8qxp-mipi-csi2";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+
+ mipi_csi1_in: endpoint {
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+
+ mipi_csi1_out: endpoint {
+ remote-endpoint = <&isi_in_3>;
+ };
+ };
+ };
+};
+
+&pi0_ipg_lpcg {
+ status = "disabled";
+};
+
+&pi0_misc_lpcg {
+ status = "disabled";
+};
+
+&pi0_pxl_lpcg {
+ status = "disabled";
+};
diff --git a/arch/arm64/boot/dts/freescale/imx8qm.dtsi b/arch/arm64/boot/dts/freescale/imx8qm.dtsi
index 6fa31bc9ece8f..c6a17a0d739c5 100644
--- a/arch/arm64/boot/dts/freescale/imx8qm.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8qm.dtsi
@@ -333,6 +333,11 @@ iomuxc: pinctrl {
compatible = "fsl,imx8qm-iomuxc";
};
+ scu_reset: reset-controller {
+ compatible = "fsl,imx-scu-reset";
+ #reset-cells = <1>;
+ };
+
rtc: rtc {
compatible = "fsl,imx8qxp-sc-rtc";
};
diff --git a/arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi b/arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi
index 3a087317591d8..9d9d933148f1b 100644
--- a/arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi
@@ -4,6 +4,88 @@
* Dong Aisheng <aisheng.dong@nxp.com>
*/
+&csi1_pxl_lpcg {
+ status = "disabled";
+};
+
+&csi1_core_lpcg {
+ status = "disabled";
+};
+
+&csi1_esc_lpcg {
+ status = "disabled";
+};
+
+&gpio0_mipi_csi1 {
+ status = "disabled";
+};
+
+&i2c_mipi_csi1 {
+ status = "disabled";
+};
+
+&irqsteer_csi1 {
+ status = "disabled";
+};
+
+&isi {
+ compatible = "fsl,imx8qxp-isi";
+ reg = <0x58100000 0x60000>;
+ interrupts = <GIC_SPI 297 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 298 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 299 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 300 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 301 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 302 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&pdma0_lpcg IMX_LPCG_CLK_0>,
+ <&pdma1_lpcg IMX_LPCG_CLK_0>,
+ <&pdma2_lpcg IMX_LPCG_CLK_0>,
+ <&pdma3_lpcg IMX_LPCG_CLK_0>,
+ <&pdma4_lpcg IMX_LPCG_CLK_0>,
+ <&pdma5_lpcg IMX_LPCG_CLK_0>;
+ clock-names = "per0", "per1", "per2", "per3", "per4", "per5";
+ power-domains = <&pd IMX_SC_R_ISI_CH0>,
+ <&pd IMX_SC_R_ISI_CH1>,
+ <&pd IMX_SC_R_ISI_CH2>,
+ <&pd IMX_SC_R_ISI_CH3>,
+ <&pd IMX_SC_R_ISI_CH4>,
+ <&pd IMX_SC_R_ISI_CH5>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@2 {
+ reg = <2>;
+
+ isi_in_2: endpoint {
+ remote-endpoint = <&mipi_csi0_out>;
+ };
+ };
+ };
+};
+
+&mipi_csi_0 {
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+
+ mipi_csi0_in: endpoint {
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+ mipi_csi0_out: endpoint {
+ remote-endpoint = <&isi_in_2>;
+ };
+ };
+ };
+};
+
&jpegdec {
compatible = "nxp,imx8qxp-jpgdec";
};
@@ -11,3 +93,7 @@ &jpegdec {
&jpegenc {
compatible = "nxp,imx8qxp-jpgenc";
};
+
+&mipi_csi_1 {
+ status = "disabled";
+};
diff --git a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
index 05138326f0a57..c078d92f76c0e 100644
--- a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
@@ -241,6 +241,11 @@ scu_key: keys {
status = "disabled";
};
+ scu_reset: reset-controller {
+ compatible = "fsl,imx-scu-reset";
+ #reset-cells = <1>;
+ };
+
rtc: rtc {
compatible = "fsl,imx8qxp-sc-rtc";
};
--
2.34.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH v4 11/13] arm64: dts: imx8: add capture controller for i.MX8's img subsystem
2025-04-08 21:53 ` [PATCH v4 11/13] arm64: dts: imx8: add capture controller for i.MX8's img subsystem Frank Li
@ 2025-04-21 21:46 ` Laurent Pinchart
0 siblings, 0 replies; 30+ messages in thread
From: Laurent Pinchart @ 2025-04-21 21:46 UTC (permalink / raw)
To: Frank Li
Cc: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Rui Miguel Silva, Martin Kepplinger,
Purism Kernel Team, linux-media, devicetree, imx,
linux-arm-kernel, linux-kernel, Robert Chiras, Guoniu.zhou
Hi Frank,
Thank you for the patch.
On Tue, Apr 08, 2025 at 05:53:09PM -0400, Frank Li wrote:
> Add CSI related nodes (i2c, irqsteer, csi, lpcg) for i.MX8 img subsystem.
I won't have time to review this patch in details, as the number of
nodes is large and they need to be matched with the documentation. I've
left a few comments below, other than those, it would be nice if you
could get someone from NXP to do the detailed review.
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> Change from v3 to v4
> - remove unused clock clock-img-axi
> - add ports information for isi and csi
> - Fix 8qxp clock, irq and power domain
> - Fix reg size
>
> Change from v2 to v3
> - remove phy and put csr register space under mipi csi2
>
> change from v1 to v2
> - move scu reset under scu node
> - add 8qm comaptible string for mipi csi2 and mipi csi phys.
> ---
> arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi | 362 ++++++++++++++++++++++
> arch/arm64/boot/dts/freescale/imx8qm-ss-img.dtsi | 85 +++++
> arch/arm64/boot/dts/freescale/imx8qm.dtsi | 5 +
> arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi | 86 +++++
> arch/arm64/boot/dts/freescale/imx8qxp.dtsi | 5 +
> 5 files changed, 543 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi b/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
> index d39242c1b9f79..2cf0f7208350a 100644
> --- a/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
> @@ -10,12 +10,264 @@ img_ipg_clk: clock-img-ipg {
> clock-output-names = "img_ipg_clk";
> };
>
> +img_pxl_clk: clock-img-pxl {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <600000000>;
> + clock-output-names = "img_pxl_clk";
> +};
> +
> img_subsys: bus@58000000 {
> compatible = "simple-bus";
> #address-cells = <1>;
> #size-cells = <1>;
> ranges = <0x58000000 0x0 0x58000000 0x1000000>;
>
> + isi: isi@58100000 {
> + reg = <0x58100000 0x80000>;
> + interrupts = <GIC_SPI 297 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 298 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 299 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 300 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 301 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 302 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 303 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 304 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&pdma0_lpcg IMX_LPCG_CLK_0>,
> + <&pdma1_lpcg IMX_LPCG_CLK_0>,
> + <&pdma2_lpcg IMX_LPCG_CLK_0>,
> + <&pdma3_lpcg IMX_LPCG_CLK_0>,
> + <&pdma4_lpcg IMX_LPCG_CLK_0>,
> + <&pdma5_lpcg IMX_LPCG_CLK_0>,
> + <&pdma6_lpcg IMX_LPCG_CLK_0>,
> + <&pdma7_lpcg IMX_LPCG_CLK_0>;
> + clock-names = "per0", "per1", "per2", "per3",
> + "per4", "per5", "per6", "per7";
> + interrupt-parent = <&gic>;
> + power-domains = <&pd IMX_SC_R_ISI_CH0>,
> + <&pd IMX_SC_R_ISI_CH1>,
> + <&pd IMX_SC_R_ISI_CH2>,
> + <&pd IMX_SC_R_ISI_CH3>,
> + <&pd IMX_SC_R_ISI_CH4>,
> + <&pd IMX_SC_R_ISI_CH5>,
> + <&pd IMX_SC_R_ISI_CH6>,
> + <&pd IMX_SC_R_ISI_CH7>;
> + status = "disabled";
> + };
> +
> + irqsteer_csi0: irqsteer@58220000 {
> + compatible = "fsl,imx8qm-irqsteer", "fsl,imx-irqsteer";
> + reg = <0x58220000 0x1000>;
> + #interrupt-cells = <1>;
> + interrupt-controller;
> + interrupts = <GIC_SPI 320 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&img_ipg_clk>;
> + clock-names = "ipg";
> + interrupt-parent = <&gic>;
> + power-domains = <&pd IMX_SC_R_CSI_0>;
> + fsl,channel = <0>;
> + fsl,num-irqs = <32>;
> + status = "disabled";
> + };
> +
> + gpio0_mipi_csi0: gpio@58222000 {
> + compatible = "fsl,imx8qm-gpio", "fsl,imx35-gpio";
> + reg = <0x58222000 0x1000>;
> + #interrupt-cells = <2>;
> + interrupt-controller;
> + interrupts = <0>;
> + #gpio-cells = <2>;
> + gpio-controller;
> + interrupt-parent = <&irqsteer_csi0>;
> + power-domains = <&pd IMX_SC_R_CSI_0>;
> + };
> +
> + csi0_core_lpcg: clock-controller@58223018 {
> + compatible = "fsl,imx8qxp-lpcg";
> + reg = <0x58223018 0x4>;
> + clocks = <&clk IMX_SC_R_CSI_0 IMX_SC_PM_CLK_PER>;
> + #clock-cells = <1>;
> + clock-indices = <IMX_LPCG_CLK_4>;
> + clock-output-names = "csi0_lpcg_core_clk";
> + power-domains = <&pd IMX_SC_R_ISI_CH0>;
> + };
> +
> + csi0_esc_lpcg: clock-controller@5822301c {
> + compatible = "fsl,imx8qxp-lpcg";
> + reg = <0x5822301c 0x4>;
> + clocks = <&clk IMX_SC_R_CSI_0 IMX_SC_PM_CLK_MISC>;
> + #clock-cells = <1>;
> + clock-indices = <IMX_LPCG_CLK_4>;
> + clock-output-names = "csi0_lpcg_esc_clk";
> + power-domains = <&pd IMX_SC_R_ISI_CH0>;
> + };
> +
> + i2c_mipi_csi0: i2c@58226000 {
> + compatible = "fsl,imx8qxp-lpi2c", "fsl,imx7ulp-lpi2c";
> + reg = <0x58226000 0x1000>;
> + interrupts = <8>;
> + clocks = <&clk IMX_SC_R_CSI_0_I2C_0 IMX_SC_PM_CLK_PER>,
> + <&img_ipg_clk>;
> + clock-names = "per", "ipg";
> + assigned-clocks = <&clk IMX_SC_R_CSI_0_I2C_0 IMX_SC_PM_CLK_PER>;
> + assigned-clock-rates = <24000000>;
> + interrupt-parent = <&irqsteer_csi0>;
> + power-domains = <&pd IMX_SC_R_CSI_0_I2C_0>;
> + status = "disabled";
> + };
> +
> + mipi_csi_0: csi@58227000 {
> + compatible = "fsl,imx8qxp-mipi-csi2";
> + reg = <0x58227000 0x1000>,
> + <0x58221000 0x1000>;
> + clocks = <&csi0_core_lpcg IMX_LPCG_CLK_4>,
> + <&csi0_esc_lpcg IMX_LPCG_CLK_4>,
> + <&csi0_pxl_lpcg IMX_LPCG_CLK_0>;
> + clock-names = "core", "esc", "ui";
> + assigned-clocks = <&csi0_core_lpcg IMX_LPCG_CLK_4>,
> + <&csi0_esc_lpcg IMX_LPCG_CLK_4>;
> + assigned-clock-rates = <360000000>, <72000000>;
> + power-domains = <&pd IMX_SC_R_ISI_CH0>;
> + resets = <&scu_reset IMX_SC_R_CSI_0>;
> + status = "disabled";
> + };
> +
> + irqsteer_csi1: irqsteer@58240000 {
> + compatible = "fsl,imx8qm-irqsteer", "fsl,imx-irqsteer";
> + reg = <0x58240000 0x1000>;
> + #interrupt-cells = <1>;
> + interrupt-controller;
> + interrupts = <GIC_SPI 321 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&img_ipg_clk>;
> + clock-names = "ipg";
> + interrupt-parent = <&gic>;
> + power-domains = <&pd IMX_SC_R_CSI_1>;
> + fsl,channel = <0>;
> + fsl,num-irqs = <32>;
> + status = "disabled";
> + };
> +
> + gpio0_mipi_csi1: gpio@58242000 {
> + compatible = "fsl,imx8qm-gpio", "fsl,imx35-gpio";
> + reg = <0x58242000 0x1000>;
> + #interrupt-cells = <2>;
> + interrupt-controller;
> + interrupts = <0>;
> + #gpio-cells = <2>;
> + gpio-controller;
> + interrupt-parent = <&irqsteer_csi1>;
> + power-domains = <&pd IMX_SC_R_CSI_1>;
> + };
> +
> + csi1_core_lpcg: clock-controller@58243018 {
> + compatible = "fsl,imx8qxp-lpcg";
> + reg = <0x58243018 0x4>;
> + clocks = <&clk IMX_SC_R_CSI_1 IMX_SC_PM_CLK_PER>;
> + #clock-cells = <1>;
> + clock-indices = <IMX_LPCG_CLK_4>;
> + clock-output-names = "csi1_lpcg_core_clk";
> + power-domains = <&pd IMX_SC_R_ISI_CH0>;
> + };
> +
> + csi1_esc_lpcg: clock-controller@5824301c {
> + compatible = "fsl,imx8qxp-lpcg";
> + reg = <0x5824301c 0x4>;
> + clocks = <&clk IMX_SC_R_CSI_1 IMX_SC_PM_CLK_MISC>;
> + #clock-cells = <1>;
> + clock-indices = <IMX_LPCG_CLK_4>;
> + clock-output-names = "csi1_lpcg_esc_clk";
> + power-domains = <&pd IMX_SC_R_ISI_CH0>;
> + };
> +
> + i2c_mipi_csi1: i2c@58246000 {
> + compatible = "fsl,imx8qxp-lpi2c", "fsl,imx7ulp-lpi2c";
> + reg = <0x58246000 0x1000>;
> + interrupts = <8>;
> + clocks = <&clk IMX_SC_R_CSI_1_I2C_0 IMX_SC_PM_CLK_PER>,
> + <&img_ipg_clk>;
> + clock-names = "per", "ipg";
> + assigned-clocks = <&clk IMX_SC_R_CSI_1_I2C_0 IMX_SC_PM_CLK_PER>;
> + assigned-clock-rates = <24000000>;
> + interrupt-parent = <&irqsteer_csi1>;
> + power-domains = <&pd IMX_SC_R_CSI_1_I2C_0>;
> + status = "disabled";
> + };
> +
> + mipi_csi_1: csi@58247000 {
> + compatible = "fsl,imx8qxp-mipi-csi2";
> + reg = <0x58247000 0x1000>,
> + <0x58241000 0x1000>;
> + clocks = <&csi1_core_lpcg IMX_LPCG_CLK_4>,
> + <&csi1_esc_lpcg IMX_LPCG_CLK_4>,
> + <&csi1_pxl_lpcg IMX_LPCG_CLK_0>;
> + clock-names = "core", "esc", "ui";
> + assigned-clocks = <&csi1_core_lpcg IMX_LPCG_CLK_4>,
> + <&csi1_esc_lpcg IMX_LPCG_CLK_4>;
> + assigned-clock-rates = <360000000>, <72000000>;
> + power-domains = <&pd IMX_SC_R_ISI_CH0>;
> + resets = <&scu_reset IMX_SC_R_CSI_1>;
> + status = "disabled";
> + };
> +
> + irqsteer_parallel: irqsteer@58260000 {
> + compatible = "fsl,imx8qm-irqsteer", "fsl,imx-irqsteer";
> + reg = <0x58260000 0x1000>;
> + #interrupt-cells = <1>;
> + interrupt-controller;
> + interrupts = <GIC_SPI 322 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&clk_dummy>;
> + clock-names = "ipg";
> + interrupt-parent = <&gic>;
> + power-domains = <&pd IMX_SC_R_PI_0>;
> + fsl,channel = <0>;
> + fsl,num-irqs = <32>;
> + status = "disabled";
> + };
> +
> + pi0_ipg_lpcg: clock-controller@58263004 {
> + compatible = "fsl,imx8qxp-lpcg";
> + reg = <0x58263004 0x4>;
> + clocks = <&clk IMX_SC_R_PI_0 IMX_SC_PM_CLK_PER>;
> + #clock-cells = <1>;
> + clock-indices = <IMX_LPCG_CLK_4>;
> + clock-output-names = "pi0_lpcg_ipg_clk";
> + power-domains = <&pd IMX_SC_R_ISI_CH0>;
> + };
> +
> + pi0_pxl_lpcg: clock-controller@58263018 {
> + compatible = "fsl,imx8qxp-lpcg";
> + reg = <0x58263018 0x4>;
> + clocks = <&clk IMX_SC_R_PI_0 IMX_SC_PM_CLK_PER>;
> + #clock-cells = <1>;
> + clock-indices = <IMX_LPCG_CLK_0>;
> + clock-output-names = "pi0_lpcg_pxl_clk";
> + power-domains = <&pd IMX_SC_R_ISI_CH0>;
> + };
> +
> + pi0_misc_lpcg: clock-controller@5826301c {
> + compatible = "fsl,imx8qxp-lpcg";
> + reg = <0x5826301c 0x4>;
> + clocks = <&clk IMX_SC_R_PI_0 IMX_SC_PM_CLK_MISC0>;
> + #clock-cells = <1>;
> + clock-indices = <IMX_LPCG_CLK_0>;
> + clock-output-names = "pi0_lpcg_misc_clk";
> + power-domains = <&pd IMX_SC_R_ISI_CH0>;
> + };
> +
> + i2c0_parallel: i2c@58266000 {
> + compatible = "fsl,imx8qxp-lpi2c", "fsl,imx7ulp-lpi2c";
> + reg = <0x58266000 0x1000>;
> + interrupts = <8>;
> + clocks = <&clk IMX_SC_R_PI_0_I2C_0 IMX_SC_PM_CLK_PER>,
> + <&img_ipg_clk>;
> + clock-names = "per", "ipg";
> + assigned-clocks = <&clk IMX_SC_R_PI_0_I2C_0 IMX_SC_PM_CLK_PER>;
> + assigned-clock-rates = <24000000>;
> + interrupt-parent = <&irqsteer_parallel>;
> + power-domains = <&pd IMX_SC_R_PI_0_I2C_0>;
> + status = "disabled";
> + };
> +
> jpegdec: jpegdec@58400000 {
> reg = <0x58400000 0x00050000>;
> interrupts = <GIC_SPI 309 IRQ_TYPE_LEVEL_HIGH>;
> @@ -40,6 +292,116 @@ jpegenc: jpegenc@58450000 {
> <&pd IMX_SC_R_MJPEG_ENC_S0>;
> };
>
> + pdma0_lpcg: clock-controller@58500000 {
> + compatible = "fsl,imx8qxp-lpcg";
> + reg = <0x58500000 0x10000>;
> + clocks = <&img_pxl_clk>;
> + #clock-cells = <1>;
> + clock-indices = <IMX_LPCG_CLK_0>;
> + clock-output-names = "pdma0_lpcg_clk";
> + power-domains = <&pd IMX_SC_R_ISI_CH0>;
> + };
> +
> + pdma1_lpcg: clock-controller@58510000 {
> + compatible = "fsl,imx8qxp-lpcg";
> + reg = <0x58510000 0x10000>;
> + clocks = <&img_pxl_clk>;
> + #clock-cells = <1>;
> + clock-indices = <IMX_LPCG_CLK_0>;
> + clock-output-names = "pdma1_lpcg_clk";
> + power-domains = <&pd IMX_SC_R_ISI_CH1>;
> + };
> +
> + pdma2_lpcg: clock-controller@58520000 {
> + compatible = "fsl,imx8qxp-lpcg";
> + reg = <0x58520000 0x10000>;
> + clocks = <&img_pxl_clk>;
> + #clock-cells = <1>;
> + clock-indices = <IMX_LPCG_CLK_0>;
> + clock-output-names = "pdma2_lpcg_clk";
> + power-domains = <&pd IMX_SC_R_ISI_CH2>;
> + };
> +
> + pdma3_lpcg: clock-controller@58530000 {
> + compatible = "fsl,imx8qxp-lpcg";
> + reg = <0x58530000 0x10000>;
> + clocks = <&img_pxl_clk>;
> + #clock-cells = <1>;
> + clock-indices = <IMX_LPCG_CLK_0>;
> + clock-output-names = "pdma3_lpcg_clk";
> + power-domains = <&pd IMX_SC_R_ISI_CH3>;
> + };
> +
> + pdma4_lpcg: clock-controller@58540000 {
> + compatible = "fsl,imx8qxp-lpcg";
> + reg = <0x58540000 0x10000>;
> + clocks = <&img_pxl_clk>;
> + #clock-cells = <1>;
> + clock-indices = <IMX_LPCG_CLK_0>;
> + clock-output-names = "pdma4_lpcg_clk";
> + power-domains = <&pd IMX_SC_R_ISI_CH4>;
> + };
> +
> + pdma5_lpcg: clock-controller@58550000 {
> + compatible = "fsl,imx8qxp-lpcg";
> + reg = <0x58550000 0x10000>;
> + clocks = <&img_pxl_clk>;
> + #clock-cells = <1>;
> + clock-indices = <IMX_LPCG_CLK_0>;
> + clock-output-names = "pdma5_lpcg_clk";
> + power-domains = <&pd IMX_SC_R_ISI_CH5>;
> + };
> +
> + pdma6_lpcg: clock-controller@58560000 {
> + compatible = "fsl,imx8qxp-lpcg";
> + reg = <0x58560000 0x10000>;
> + clocks = <&img_pxl_clk>;
> + #clock-cells = <1>;
> + clock-indices = <IMX_LPCG_CLK_0>;
> + clock-output-names = "pdma6_lpcg_clk";
> + power-domains = <&pd IMX_SC_R_ISI_CH6>;
> + };
> +
> + pdma7_lpcg: clock-controller@58570000 {
> + compatible = "fsl,imx8qxp-lpcg";
> + reg = <0x58570000 0x10000>;
> + clocks = <&img_pxl_clk>;
> + #clock-cells = <1>;
> + clock-indices = <IMX_LPCG_CLK_0>;
> + clock-output-names = "pdma7_lpcg_clk";
> + power-domains = <&pd IMX_SC_R_ISI_CH7>;
> + };
> +
> + csi0_pxl_lpcg: clock-controller@58580000 {
> + compatible = "fsl,imx8qxp-lpcg";
> + reg = <0x58580000 0x10000>;
> + clocks = <&img_pxl_clk>;
> + #clock-cells = <1>;
> + clock-indices = <IMX_LPCG_CLK_0>;
> + clock-output-names = "csi0_lpcg_pxl_clk";
> + power-domains = <&pd IMX_SC_R_CSI_0>;
> + };
> +
> + csi1_pxl_lpcg: clock-controller@58590000 {
> + compatible = "fsl,imx8qxp-lpcg";
> + reg = <0x58590000 0x10000>;
> + clocks = <&img_pxl_clk>;
> + #clock-cells = <1>;
> + clock-indices = <IMX_LPCG_CLK_0>;
> + clock-output-names = "csi1_lpcg_pxl_clk";
> + power-domains = <&pd IMX_SC_R_CSI_1>;
> + };
> +
> + hdmi_rx_pxl_link_lpcg: clock-controller@585a0000 {
> + compatible = "fsl,imx8qxp-lpcg";
> + reg = <0x585a0000 0x10000>;
> + clocks = <&img_pxl_clk>;
> + #clock-cells = <1>;
> + clock-indices = <IMX_LPCG_CLK_0>;
> + clock-output-names = "hdmi_rx_lpcg_pxl_link_clk";
> + power-domains = <&pd IMX_SC_R_HDMI_RX>;
> + };
> +
> img_jpeg_dec_lpcg: clock-controller@585d0000 {
> compatible = "fsl,imx8qxp-lpcg";
> reg = <0x585d0000 0x10000>;
> diff --git a/arch/arm64/boot/dts/freescale/imx8qm-ss-img.dtsi b/arch/arm64/boot/dts/freescale/imx8qm-ss-img.dtsi
> index 2bbdacb1313f9..ce2ab67c6cba3 100644
> --- a/arch/arm64/boot/dts/freescale/imx8qm-ss-img.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8qm-ss-img.dtsi
> @@ -3,6 +3,31 @@
> * Copyright 2021 NXP
> */
>
> +&isi {
> + compatible = "fsl,imx8qm-isi";
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@2 {
> + reg = <2>;
> +
> + isi_in_2: endpoint {
> + remote-endpoint = <&mipi_csi0_out>;
> + };
> + };
> +
> + port@3 {
> + reg = <3>;
> +
> + isi_in_3: endpoint {
> + remote-endpoint = <&mipi_csi1_out>;
> + };
> + };
> + };
> +};
> +
> &jpegdec {
> compatible = "nxp,imx8qm-jpgdec", "nxp,imx8qxp-jpgdec";
> };
> @@ -10,3 +35,63 @@ &jpegdec {
> &jpegenc {
> compatible = "nxp,imx8qm-jpgenc", "nxp,imx8qxp-jpgenc";
> };
> +
> +&mipi_csi_0 {
> + compatible = "fsl,imx8qm-mipi-csi2", "fsl,imx8qxp-mipi-csi2";
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> +
> + mipi_csi0_in: endpoint {
> + };
Ports model connection points, and exist even if they are not connected.
That's why declaring empty ports in a .dtsi file is fine. Endpoints, on
the other end, model connection, so an unconnected endpoint is not
allowed. You should drop the endpoint here.
> + };
> +
> + port@1 {
> + reg = <1>;
> +
> + mipi_csi0_out: endpoint {
> + remote-endpoint = <&isi_in_2>;
> + };
> + };
> + };
> +};
> +
> +&mipi_csi_1 {
> + compatible = "fsl,imx8qm-mipi-csi2", "fsl,imx8qxp-mipi-csi2";
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> +
> + mipi_csi1_in: endpoint {
> + };
Same here.
> + };
> +
> + port@1 {
> + reg = <1>;
> +
> + mipi_csi1_out: endpoint {
> + remote-endpoint = <&isi_in_3>;
> + };
> + };
> + };
> +};
> +
> +&pi0_ipg_lpcg {
> + status = "disabled";
> +};
> +
> +&pi0_misc_lpcg {
> + status = "disabled";
> +};
> +
> +&pi0_pxl_lpcg {
> + status = "disabled";
> +};
> diff --git a/arch/arm64/boot/dts/freescale/imx8qm.dtsi b/arch/arm64/boot/dts/freescale/imx8qm.dtsi
> index 6fa31bc9ece8f..c6a17a0d739c5 100644
> --- a/arch/arm64/boot/dts/freescale/imx8qm.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8qm.dtsi
> @@ -333,6 +333,11 @@ iomuxc: pinctrl {
> compatible = "fsl,imx8qm-iomuxc";
> };
>
> + scu_reset: reset-controller {
> + compatible = "fsl,imx-scu-reset";
> + #reset-cells = <1>;
> + };
> +
> rtc: rtc {
> compatible = "fsl,imx8qxp-sc-rtc";
> };
> diff --git a/arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi b/arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi
> index 3a087317591d8..9d9d933148f1b 100644
> --- a/arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi
> @@ -4,6 +4,88 @@
> * Dong Aisheng <aisheng.dong@nxp.com>
> */
>
> +&csi1_pxl_lpcg {
> + status = "disabled";
> +};
> +
> +&csi1_core_lpcg {
> + status = "disabled";
> +};
> +
> +&csi1_esc_lpcg {
> + status = "disabled";
> +};
> +
> +&gpio0_mipi_csi1 {
> + status = "disabled";
> +};
> +
> +&i2c_mipi_csi1 {
> + status = "disabled";
> +};
> +
> +&irqsteer_csi1 {
> + status = "disabled";
> +};
> +
> +&isi {
> + compatible = "fsl,imx8qxp-isi";
> + reg = <0x58100000 0x60000>;
> + interrupts = <GIC_SPI 297 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 298 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 299 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 300 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 301 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 302 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&pdma0_lpcg IMX_LPCG_CLK_0>,
> + <&pdma1_lpcg IMX_LPCG_CLK_0>,
> + <&pdma2_lpcg IMX_LPCG_CLK_0>,
> + <&pdma3_lpcg IMX_LPCG_CLK_0>,
> + <&pdma4_lpcg IMX_LPCG_CLK_0>,
> + <&pdma5_lpcg IMX_LPCG_CLK_0>;
> + clock-names = "per0", "per1", "per2", "per3", "per4", "per5";
> + power-domains = <&pd IMX_SC_R_ISI_CH0>,
> + <&pd IMX_SC_R_ISI_CH1>,
> + <&pd IMX_SC_R_ISI_CH2>,
> + <&pd IMX_SC_R_ISI_CH3>,
> + <&pd IMX_SC_R_ISI_CH4>,
> + <&pd IMX_SC_R_ISI_CH5>;
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@2 {
> + reg = <2>;
> +
> + isi_in_2: endpoint {
> + remote-endpoint = <&mipi_csi0_out>;
> + };
> + };
> + };
> +};
> +
> +&mipi_csi_0 {
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> +
> + mipi_csi0_in: endpoint {
> + };
> + };
> +
> + port@1 {
> + reg = <1>;
> + mipi_csi0_out: endpoint {
> + remote-endpoint = <&isi_in_2>;
> + };
> + };
> + };
> +};
> +
> &jpegdec {
> compatible = "nxp,imx8qxp-jpgdec";
> };
> @@ -11,3 +93,7 @@ &jpegdec {
> &jpegenc {
> compatible = "nxp,imx8qxp-jpgenc";
> };
> +
> +&mipi_csi_1 {
> + status = "disabled";
> +};
> diff --git a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> index 05138326f0a57..c078d92f76c0e 100644
> --- a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> @@ -241,6 +241,11 @@ scu_key: keys {
> status = "disabled";
> };
>
> + scu_reset: reset-controller {
> + compatible = "fsl,imx-scu-reset";
> + #reset-cells = <1>;
> + };
> +
> rtc: rtc {
> compatible = "fsl,imx8qxp-sc-rtc";
> };
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v4 12/13] arm64: dts: imx8q: add linux,cma node for imx8qm-mek and imx8qxp-mek
2025-04-08 21:52 [PATCH v4 00/13] media: imx8: add camera support Frank Li
` (10 preceding siblings ...)
2025-04-08 21:53 ` [PATCH v4 11/13] arm64: dts: imx8: add capture controller for i.MX8's img subsystem Frank Li
@ 2025-04-08 21:53 ` Frank Li
2025-04-08 21:53 ` [PATCH v4 13/13] arm64: dts: imx8q: add camera ov5640 support " Frank Li
2025-04-21 18:44 ` [PATCH v4 00/13] media: imx8: add camera support Frank Li
13 siblings, 0 replies; 30+ messages in thread
From: Frank Li @ 2025-04-08 21:53 UTC (permalink / raw)
To: Laurent Pinchart, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Frank Li,
Rui Miguel Silva, Martin Kepplinger, Purism Kernel Team
Cc: linux-media, devicetree, imx, linux-arm-kernel, linux-kernel,
Robert Chiras, Guoniu.zhou
Add linux,cma node because some devices, such as camera, need big continue
physical memory.
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
change from v1 to v3
- none
---
arch/arm64/boot/dts/freescale/imx8qm-mek.dts | 9 +++++++++
arch/arm64/boot/dts/freescale/imx8qxp-mek.dts | 8 ++++++++
2 files changed, 17 insertions(+)
diff --git a/arch/arm64/boot/dts/freescale/imx8qm-mek.dts b/arch/arm64/boot/dts/freescale/imx8qm-mek.dts
index 353f825a8ac5d..68442c8575f3f 100644
--- a/arch/arm64/boot/dts/freescale/imx8qm-mek.dts
+++ b/arch/arm64/boot/dts/freescale/imx8qm-mek.dts
@@ -113,6 +113,15 @@ dsp_vdev0buffer: memory@94300000 {
reg = <0 0x94300000 0 0x100000>;
no-map;
};
+
+ /* global autoconfigured region for contiguous allocations */
+ linux,cma {
+ compatible = "shared-dma-pool";
+ alloc-ranges = <0 0xc0000000 0 0x3c000000>;
+ size = <0 0x3c000000>;
+ linux,cma-default;
+ reusable;
+ };
};
lvds_backlight0: backlight-lvds0 {
diff --git a/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts b/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts
index a669a5d500d32..a378f462a283b 100644
--- a/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts
+++ b/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts
@@ -56,6 +56,14 @@ dsp_vdev0buffer: memory@94300000 {
reg = <0 0x94300000 0 0x100000>;
no-map;
};
+
+ linux,cma {
+ compatible = "shared-dma-pool";
+ alloc-ranges = <0 0xc0000000 0 0x3c000000>;
+ size = <0 0x3c000000>;
+ linux,cma-default;
+ reusable;
+ };
};
reg_usdhc2_vmmc: usdhc2-vmmc {
--
2.34.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH v4 13/13] arm64: dts: imx8q: add camera ov5640 support for imx8qm-mek and imx8qxp-mek
2025-04-08 21:52 [PATCH v4 00/13] media: imx8: add camera support Frank Li
` (11 preceding siblings ...)
2025-04-08 21:53 ` [PATCH v4 12/13] arm64: dts: imx8q: add linux,cma node for imx8qm-mek and imx8qxp-mek Frank Li
@ 2025-04-08 21:53 ` Frank Li
2025-04-21 18:44 ` [PATCH v4 00/13] media: imx8: add camera support Frank Li
13 siblings, 0 replies; 30+ messages in thread
From: Frank Li @ 2025-04-08 21:53 UTC (permalink / raw)
To: Laurent Pinchart, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Frank Li,
Rui Miguel Silva, Martin Kepplinger, Purism Kernel Team
Cc: linux-media, devicetree, imx, linux-arm-kernel, linux-kernel,
Robert Chiras, Guoniu.zhou
Add ov5640 overlay file for imx8qm-mek and imx8qxp-mek board. Camera can
connect different CSI port. So use dts overlay file to handle these
difference connect options.
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
change from v3 to v4
- add board level xtal24m
- remove reduntant ports information at dtso because chip leave already add
it.
change from v2 to v3
- remove phy nodes
change from v1 to v2
- none
---
arch/arm64/boot/dts/freescale/Makefile | 11 ++++
.../boot/dts/freescale/imx8qm-mek-ov5640-csi0.dtso | 60 ++++++++++++++++++++++
.../boot/dts/freescale/imx8qm-mek-ov5640-csi1.dtso | 60 ++++++++++++++++++++++
arch/arm64/boot/dts/freescale/imx8qm-mek.dts | 58 +++++++++++++++++++++
.../boot/dts/freescale/imx8qxp-mek-ov5640-csi.dtso | 59 +++++++++++++++++++++
arch/arm64/boot/dts/freescale/imx8qxp-mek.dts | 36 +++++++++++++
6 files changed, 284 insertions(+)
diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile
index b6d3fe26d6212..4101ef6ed520d 100644
--- a/arch/arm64/boot/dts/freescale/Makefile
+++ b/arch/arm64/boot/dts/freescale/Makefile
@@ -277,6 +277,14 @@ dtb-$(CONFIG_ARCH_MXC) += imx8qm-apalis-v1.1-eval-v1.2.dtb
dtb-$(CONFIG_ARCH_MXC) += imx8qm-apalis-v1.1-ixora-v1.1.dtb
dtb-$(CONFIG_ARCH_MXC) += imx8qm-apalis-v1.1-ixora-v1.2.dtb
dtb-$(CONFIG_ARCH_MXC) += imx8qm-mek.dtb
+
+imx8qm-mek-ov5640-csi0-dtbs := imx8qm-mek.dtb imx8qm-mek-ov5640-csi0.dtbo
+dtb-${CONFIG_ARCH_MXC} += imx8qm-mek-ov5640-csi0.dtb
+imx8qm-mek-ov5640-csi1-dtbs := imx8qm-mek.dtb imx8qm-mek-ov5640-csi1.dtbo
+dtb-${CONFIG_ARCH_MXC} += imx8qm-mek-ov5640-csi1.dtb
+imx8qm-mek-ov5640-dual-dtbs := imx8qm-mek.dtb imx8qm-mek-ov5640-csi0.dtbo imx8qm-mek-ov5640-csi1.dtbo
+dtb-${CONFIG_ARCH_MXC} += imx8qm-mek-ov5640-dual.dtb
+
dtb-$(CONFIG_ARCH_MXC) += imx8qxp-ai_ml.dtb
dtb-$(CONFIG_ARCH_MXC) += imx8qxp-colibri-aster.dtb
dtb-$(CONFIG_ARCH_MXC) += imx8qxp-colibri-eval-v3.dtb
@@ -287,6 +295,9 @@ dtb-$(CONFIG_ARCH_MXC) += imx8qxp-mek.dtb
imx8qxp-mek-pcie-ep-dtbs += imx8qxp-mek.dtb imx8qxp-mek-pcie-ep.dtbo
dtb-$(CONFIG_ARCH_MXC) += imx8qxp-mek-pcie-ep.dtb
+imx8qxp-mek-ov5640-csi-dtbs := imx8qxp-mek.dtb imx8qxp-mek-ov5640-csi.dtbo
+dtb-${CONFIG_ARCH_MXC} += imx8qxp-mek-ov5640-csi.dtb
+
dtb-$(CONFIG_ARCH_MXC) += imx8qxp-tqma8xqp-mba8xx.dtb
dtb-$(CONFIG_ARCH_MXC) += imx8ulp-evk.dtb
dtb-$(CONFIG_ARCH_MXC) += imx93-9x9-qsb.dtb
diff --git a/arch/arm64/boot/dts/freescale/imx8qm-mek-ov5640-csi0.dtso b/arch/arm64/boot/dts/freescale/imx8qm-mek-ov5640-csi0.dtso
new file mode 100644
index 0000000000000..ba8ceee41db6f
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/imx8qm-mek-ov5640-csi0.dtso
@@ -0,0 +1,60 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright 2025 NXP
+ */
+
+/dts-v1/;
+/plugin/;
+
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/media/video-interfaces.h>
+
+&i2c_mipi_csi0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ clock-frequency = <100000>;
+ pinctrl-0 = <&pinctrl_i2c_mipi_csi0>;
+ pinctrl-names = "default";
+ status = "okay";
+
+ ov5640_mipi_0: camera@3c {
+ compatible = "ovti,ov5640";
+ reg = <0x3c>;
+ clocks = <&xtal24m>;
+ clock-names = "xclk";
+ pinctrl-0 = <&pinctrl_mipi_csi0>;
+ pinctrl-names = "default";
+ powerdown-gpios = <&lsio_gpio1 28 GPIO_ACTIVE_HIGH>;
+ reset-gpios = <&lsio_gpio1 27 GPIO_ACTIVE_LOW>;
+ AVDD-supply = <®_2v8>;
+ DVDD-supply = <®_1v5>;
+ DOVDD-supply = <®_1v8>;
+ status = "okay";
+
+ port {
+ ov5640_mipi_0_ep: endpoint {
+ bus-type = <MEDIA_BUS_TYPE_CSI2_DPHY>;
+ data-lanes = <1 2>;
+ remote-endpoint = <&mipi_csi0_in>;
+ };
+ };
+ };
+};
+
+&irqsteer_csi0 {
+ status = "okay";
+};
+
+&isi {
+ status = "okay";
+};
+
+&mipi_csi_0 {
+ status = "okay";
+};
+
+&mipi_csi0_in {
+ data-lanes = <1 2>;
+ remote-endpoint = <&ov5640_mipi_0_ep>;
+};
+
diff --git a/arch/arm64/boot/dts/freescale/imx8qm-mek-ov5640-csi1.dtso b/arch/arm64/boot/dts/freescale/imx8qm-mek-ov5640-csi1.dtso
new file mode 100644
index 0000000000000..549633f37db53
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/imx8qm-mek-ov5640-csi1.dtso
@@ -0,0 +1,60 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright 2025 NXP
+ */
+
+/dts-v1/;
+/plugin/;
+
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/media/video-interfaces.h>
+
+&i2c_mipi_csi1 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ clock-frequency = <100000>;
+ pinctrl-0 = <&pinctrl_i2c_mipi_csi1>;
+ pinctrl-names = "default";
+ status = "okay";
+
+ ov5640_mipi_1: camera@3c {
+ compatible = "ovti,ov5640";
+ reg = <0x3c>;
+ clocks = <&xtal24m>;
+ clock-names = "xclk";
+ pinctrl-0 = <&pinctrl_mipi_csi1>;
+ pinctrl-names = "default";
+ powerdown-gpios = <&lsio_gpio1 31 GPIO_ACTIVE_HIGH>;
+ reset-gpios = <&lsio_gpio1 30 GPIO_ACTIVE_LOW>;
+ AVDD-supply = <®_2v8>;
+ DVDD-supply = <®_1v5>;
+ DOVDD-supply = <®_1v8>;
+ status = "okay";
+
+ port {
+ ov5640_mipi_1_ep: endpoint {
+ bus-type = <MEDIA_BUS_TYPE_CSI2_DPHY>;
+ data-lanes = <1 2>;
+ remote-endpoint = <&mipi_csi1_in>;
+ };
+ };
+ };
+};
+
+&irqsteer_csi1 {
+ status = "okay";
+};
+
+&isi {
+ status = "okay";
+};
+
+&mipi_csi_1 {
+ status = "okay";
+};
+
+&mipi_csi1_in {
+ data-lanes = <1 2>;
+ remote-endpoint = <&ov5640_mipi_1_ep>;
+};
+
diff --git a/arch/arm64/boot/dts/freescale/imx8qm-mek.dts b/arch/arm64/boot/dts/freescale/imx8qm-mek.dts
index 68442c8575f3f..503e0acd7963d 100644
--- a/arch/arm64/boot/dts/freescale/imx8qm-mek.dts
+++ b/arch/arm64/boot/dts/freescale/imx8qm-mek.dts
@@ -32,6 +32,13 @@ memory@80000000 {
reg = <0x00000000 0x80000000 0 0x40000000>;
};
+ xtal24m: clock-xtal24m {
+ compatible = "fixed-clock";
+ #clock-cells = <0>;
+ clock-frequency = <24000000>;
+ clock-output-names = "xtal_24MHz";
+ };
+
reserved-memory {
#address-cells = <2>;
#size-cells = <2>;
@@ -155,6 +162,27 @@ usb3_data_ss: endpoint {
};
};
+ reg_1v5: regulator-1v5 {
+ compatible = "regulator-fixed";
+ regulator-name = "1v5";
+ regulator-min-microvolt = <1500000>;
+ regulator-max-microvolt = <1500000>;
+ };
+
+ reg_1v8: regulator-1v8 {
+ compatible = "regulator-fixed";
+ regulator-name = "1v8";
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ };
+
+ reg_2v8: regulator-2v8 {
+ compatible = "regulator-fixed";
+ regulator-name = "2v8";
+ regulator-min-microvolt = <2800000>;
+ regulator-max-microvolt = <2800000>;
+ };
+
reg_usdhc2_vmmc: usdhc2-vmmc {
compatible = "regulator-fixed";
regulator-name = "SD1_SPWR";
@@ -824,6 +852,20 @@ IMX8QM_QSPI1A_DATA1_LSIO_GPIO4_IO25 0x0600004c
>;
};
+ pinctrl_i2c_mipi_csi0: i2c-mipi-csi0grp {
+ fsl,pins = <
+ IMX8QM_MIPI_CSI0_I2C0_SCL_MIPI_CSI0_I2C0_SCL 0xc2000020
+ IMX8QM_MIPI_CSI0_I2C0_SDA_MIPI_CSI0_I2C0_SDA 0xc2000020
+ >;
+ };
+
+ pinctrl_i2c_mipi_csi1: i2c-mipi-csi1grp {
+ fsl,pins = <
+ IMX8QM_MIPI_CSI1_I2C0_SCL_MIPI_CSI1_I2C0_SCL 0xc2000020
+ IMX8QM_MIPI_CSI1_I2C0_SDA_MIPI_CSI1_I2C0_SDA 0xc2000020
+ >;
+ };
+
pinctrl_i2c0: i2c0grp {
fsl,pins = <
IMX8QM_HDMI_TX0_TS_SCL_DMA_I2C0_SCL 0x06000021
@@ -1017,6 +1059,22 @@ IMX8QM_LVDS1_I2C1_SDA_LVDS1_I2C1_SDA 0xc600004c
>;
};
+ pinctrl_mipi_csi0: mipi-csi0grp {
+ fsl,pins = <
+ IMX8QM_MIPI_CSI0_GPIO0_00_LSIO_GPIO1_IO27 0xC0000041
+ IMX8QM_MIPI_CSI0_GPIO0_01_LSIO_GPIO1_IO28 0xC0000041
+ IMX8QM_MIPI_CSI0_MCLK_OUT_MIPI_CSI0_ACM_MCLK_OUT 0xC0000041
+ >;
+ };
+
+ pinctrl_mipi_csi1: mipi-csi1grp {
+ fsl,pins = <
+ IMX8QM_MIPI_CSI1_GPIO0_00_LSIO_GPIO1_IO30 0xC0000041
+ IMX8QM_MIPI_CSI1_GPIO0_01_LSIO_GPIO1_IO31 0xC0000041
+ IMX8QM_MIPI_CSI1_MCLK_OUT_MIPI_CSI1_ACM_MCLK_OUT 0xC0000041
+ >;
+ };
+
pinctrl_pciea: pcieagrp {
fsl,pins = <
IMX8QM_PCIE_CTRL0_WAKE_B_LSIO_GPIO4_IO28 0x04000021
diff --git a/arch/arm64/boot/dts/freescale/imx8qxp-mek-ov5640-csi.dtso b/arch/arm64/boot/dts/freescale/imx8qxp-mek-ov5640-csi.dtso
new file mode 100644
index 0000000000000..5500c4846f031
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/imx8qxp-mek-ov5640-csi.dtso
@@ -0,0 +1,59 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright 2024 NXP
+ */
+/dts-v1/;
+/plugin/;
+
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/media/video-interfaces.h>
+
+&i2c_mipi_csi0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ clock-frequency = <100000>;
+ pinctrl-0 = <&pinctrl_i2c_mipi_csi0>;
+ pinctrl-names = "default";
+ status = "okay";
+
+ ov5640_mipi: camera@3c {
+ compatible = "ovti,ov5640";
+ reg = <0x3c>;
+ clocks = <&xtal24m>;
+ clock-names = "xclk";
+ pinctrl-0 = <&pinctrl_mipi_csi0>;
+ pinctrl-names = "default";
+ powerdown-gpios = <&lsio_gpio3 7 GPIO_ACTIVE_HIGH>;
+ reset-gpios = <&lsio_gpio3 8 GPIO_ACTIVE_LOW>;
+ AVDD-supply = <®_2v8>;
+ DVDD-supply = <®_1v5>;
+ DOVDD-supply = <®_1v8>;
+ status = "okay";
+
+ port {
+ ov5640_mipi_ep: endpoint {
+ bus-type = <MEDIA_BUS_TYPE_CSI2_DPHY>;
+ data-lanes = <1 2>;
+ remote-endpoint = <&mipi_csi0_in>;
+ };
+ };
+ };
+};
+
+&irqsteer_csi0 {
+ status = "okay";
+};
+
+&isi {
+ status = "okay";
+};
+
+&mipi_csi_0 {
+ status = "okay";
+};
+
+&mipi_csi0_in {
+ data-lanes = <1 2>;
+ remote-endpoint = <&ov5640_mipi_ep>;
+};
+
diff --git a/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts b/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts
index a378f462a283b..d7ab042e0e72b 100644
--- a/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts
+++ b/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts
@@ -90,6 +90,27 @@ usb3_data_ss: endpoint {
};
};
+ reg_1v5: regulator-1v5 {
+ compatible = "regulator-fixed";
+ regulator-name = "1v5";
+ regulator-min-microvolt = <1500000>;
+ regulator-max-microvolt = <1500000>;
+ };
+
+ reg_1v8: regulator-1v8 {
+ compatible = "regulator-fixed";
+ regulator-name = "1v8";
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ };
+
+ reg_2v8: regulator-2v8 {
+ compatible = "regulator-fixed";
+ regulator-name = "2v8";
+ regulator-min-microvolt = <2800000>;
+ regulator-max-microvolt = <2800000>;
+ };
+
reg_pcieb: regulator-pcie {
compatible = "regulator-fixed";
regulator-max-microvolt = <3300000>;
@@ -781,6 +802,13 @@ IMX8QXP_FLEXCAN1_RX_ADMA_FLEXCAN1_RX 0x21
>;
};
+ pinctrl_i2c_mipi_csi0: i2c-mipi-csi0grp {
+ fsl,pins = <
+ IMX8QXP_MIPI_CSI0_I2C0_SCL_MIPI_CSI0_I2C0_SCL 0xc2000020
+ IMX8QXP_MIPI_CSI0_I2C0_SDA_MIPI_CSI0_I2C0_SDA 0xc2000020
+ >;
+ };
+
pinctrl_ioexp_rst: ioexprstgrp {
fsl,pins = <
IMX8QXP_SPI2_SDO_LSIO_GPIO1_IO01 0x06000021
@@ -821,6 +849,14 @@ IMX8QXP_FLEXCAN2_RX_ADMA_UART3_RX 0x06000020
>;
};
+ pinctrl_mipi_csi0: mipi-csi0grp {
+ fsl,pins = <
+ IMX8QXP_MIPI_CSI0_GPIO0_01_LSIO_GPIO3_IO07 0xC0000041
+ IMX8QXP_MIPI_CSI0_GPIO0_00_LSIO_GPIO3_IO08 0xC0000041
+ IMX8QXP_MIPI_CSI0_MCLK_OUT_MIPI_CSI0_ACM_MCLK_OUT 0xC0000041
+ >;
+ };
+
pinctrl_pcieb: pcieagrp {
fsl,pins = <
IMX8QXP_PCIE_CTRL0_PERST_B_LSIO_GPIO4_IO00 0x06000021
--
2.34.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH v4 00/13] media: imx8: add camera support
2025-04-08 21:52 [PATCH v4 00/13] media: imx8: add camera support Frank Li
` (12 preceding siblings ...)
2025-04-08 21:53 ` [PATCH v4 13/13] arm64: dts: imx8q: add camera ov5640 support " Frank Li
@ 2025-04-21 18:44 ` Frank Li
13 siblings, 0 replies; 30+ messages in thread
From: Frank Li @ 2025-04-21 18:44 UTC (permalink / raw)
To: Laurent Pinchart, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Rui Miguel Silva,
Martin Kepplinger, Purism Kernel Team
Cc: linux-media, devicetree, imx, linux-arm-kernel, linux-kernel,
Robert Chiras, Guoniu.zhou
On Tue, Apr 08, 2025 at 05:52:58PM -0400, Frank Li wrote:
> Add SCU reset driver for i.MX8QM/i.MX8QXP.
> Update binding doc.
> Update driver for imx8qxp and imx8qm.
> Add dts files for it.
>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
Laurent Pinchart:
Do you have chance to check this version?
Frank Li
>
> Changes in v4:
> - Add 4 clean up patches
> media: nxp: imx8-isi: Remove unused offset in mxc_isi_reg and use BIT() macro for mask
> media: nxp: imx8-isi: Use dev_err_probe() simplify code
> media: nxp: imx8-isi: Remove redundant check for dma_set_mask_and_coherent()
> media: nxp: imx8-isi: Use devm_clk_bulk_get_all() to fetch clocks
> - rebase to v6.15-rc1.
> - Remove scu reset patches, which already in linux-next
> - Remove patch
> Add fixed clock node clock-xtal24m to prepare to add camera support.
> - other detail change log see each patch's change log
> - Link to v3: https://lore.kernel.org/r/20250210-8qxp_camera-v3-0-324f5105accc@nxp.com
>
> Changes in v3:
> - Remove phy driver parts.
> - csr is dedicate for mipi csi2, so add it as second register space. csr is
> mixed with PHY and link control with csi2.
> - Link to v2: https://lore.kernel.org/r/20250205-8qxp_camera-v2-0-731a3edf2744@nxp.com
>
> Changes in v2:
> - move scu reset binding doc to top scu doc.
> - isi use seperate binding doc for imx8qxp and imx8qm.
> - phy and csi2, compatible string 8qm fallback to qxp
> - remove internal review tags
> - Link to v1: https://lore.kernel.org/r/20250131-8qxp_camera-v1-0-319402ab606a@nxp.com
>
> ---
> Frank Li (10):
> media: dt-bindings: Add binding doc for i.MX8QXP and i.MX8QM ISI
> media: nxp: imx8-isi: Allow num_sources to be greater than num_sink
> media: nxp: imx8-isi: Remove unused offset in mxc_isi_reg and use BIT() macro for mask
> media: nxp: imx8-isi: Use devm_clk_bulk_get_all() to fetch clocks
> media: nxp: imx8-isi: Remove redundant check for dma_set_mask_and_coherent()
> media: nxp: imx8-isi: Use dev_err_probe() simplify code
> media: imx8mq-mipi-csi2: Add support for i.MX8QXP
> arm64: dts: imx8: add capture controller for i.MX8's img subsystem
> arm64: dts: imx8q: add linux,cma node for imx8qm-mek and imx8qxp-mek
> arm64: dts: imx8q: add camera ov5640 support for imx8qm-mek and imx8qxp-mek
>
> Guoniu.zhou (1):
> media: imx8mq-mipi-csi2: Add imx8mq_plat_data for different compatible strings
>
> Robert Chiras (2):
> media: imx8-isi: Add support for i.MX8QM and i.MX8QXP
> media: dt-bindings: nxp,imx8mq-mipi-csi2: Add i.MX8QM(QXP) compatible strings
>
> .../devicetree/bindings/media/fsl,imx8qm-isi.yaml | 117 +++++++
> .../devicetree/bindings/media/fsl,imx8qxp-isi.yaml | 106 ++++++
> .../bindings/media/nxp,imx8mq-mipi-csi2.yaml | 38 ++-
> MAINTAINERS | 1 +
> arch/arm64/boot/dts/freescale/Makefile | 11 +
> arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi | 362 +++++++++++++++++++++
> .../boot/dts/freescale/imx8qm-mek-ov5640-csi0.dtso | 60 ++++
> .../boot/dts/freescale/imx8qm-mek-ov5640-csi1.dtso | 60 ++++
> arch/arm64/boot/dts/freescale/imx8qm-mek.dts | 67 ++++
> arch/arm64/boot/dts/freescale/imx8qm-ss-img.dtsi | 85 +++++
> arch/arm64/boot/dts/freescale/imx8qm.dtsi | 5 +
> .../boot/dts/freescale/imx8qxp-mek-ov5640-csi.dtso | 59 ++++
> arch/arm64/boot/dts/freescale/imx8qxp-mek.dts | 44 +++
> arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi | 86 +++++
> arch/arm64/boot/dts/freescale/imx8qxp.dtsi | 5 +
> .../media/platform/nxp/imx8-isi/imx8-isi-core.c | 133 ++++----
> .../media/platform/nxp/imx8-isi/imx8-isi-core.h | 6 +-
> .../platform/nxp/imx8-isi/imx8-isi-crossbar.c | 8 +-
> drivers/media/platform/nxp/imx8mq-mipi-csi2.c | 169 +++++++++-
> 19 files changed, 1328 insertions(+), 94 deletions(-)
> ---
> base-commit: 1f665976a7c4e8779566e153b8854d7829ce33ac
> change-id: 20250114-8qxp_camera-c1af5749d304
>
> Best regards,
> ---
> Frank Li <Frank.Li@nxp.com>
>
^ permalink raw reply [flat|nested] 30+ messages in thread