* [PATCH 2/5] arm64: dts: imx8mm: Add CSI nodes
2021-11-05 13:42 [PATCH 1/5] soc: imx: imx8m-blk-ctrl: Fix imx8mm mipi reset Adam Ford
@ 2021-11-05 13:42 ` Adam Ford
2021-11-05 19:32 ` Tim Harvey
2021-11-05 13:42 ` [PATCH 3/5] arm64: defconfig: Enable VIDEO_IMX_MEDIA Adam Ford
` (3 subsequent siblings)
4 siblings, 1 reply; 9+ messages in thread
From: Adam Ford @ 2021-11-05 13:42 UTC (permalink / raw)
To: linux-arm-kernel
Cc: tharvey, frieder.schrempf, linux-media, laurent.pinchart, aford,
cstevens, Adam Ford, Rob Herring, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
Catalin Marinas, Will Deacon, Lucas Stach, Peng Fan, devicetree,
linux-kernel
There is a csi bridge and csis interface that tie together
to allow csi2 capture.
Signed-off-by: Adam Ford <aford173@gmail.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
arch/arm64/boot/dts/freescale/imx8mm.dtsi | 51 +++++++++++++++++++++++
1 file changed, 51 insertions(+)
diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
index c2f3f118f82e..1f69c14d953f 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
@@ -1068,6 +1068,22 @@ aips4: bus@32c00000 {
#size-cells = <1>;
ranges = <0x32c00000 0x32c00000 0x400000>;
+ csi: csi@32e20000 {
+ compatible = "fsl,imx8mm-csi", "fsl,imx7-csi";
+ reg = <0x32e20000 0x1000>;
+ interrupts = <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clk IMX8MM_CLK_CSI1_ROOT>;
+ clock-names = "mclk";
+ power-domains = <&disp_blk_ctrl IMX8MM_DISPBLK_PD_CSI_BRIDGE>;
+ status = "disabled";
+
+ port {
+ csi_in: endpoint {
+ remote-endpoint = <&imx8mm_mipi_csi_out>;
+ };
+ };
+ };
+
disp_blk_ctrl: blk-ctrl@32e28000 {
compatible = "fsl,imx8mm-disp-blk-ctrl", "syscon";
reg = <0x32e28000 0x100>;
@@ -1095,6 +1111,41 @@ disp_blk_ctrl: blk-ctrl@32e28000 {
#power-domain-cells = <1>;
};
+ mipi_csi: mipi-csi@32e30000 {
+ compatible = "fsl,imx8mm-mipi-csi2";
+ reg = <0x32e30000 0x1000>;
+ interrupts = <GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>;
+ assigned-clocks = <&clk IMX8MM_CLK_CSI1_CORE>,
+ <&clk IMX8MM_CLK_CSI1_PHY_REF>;
+ assigned-clock-parents = <&clk IMX8MM_SYS_PLL2_1000M>,
+ <&clk IMX8MM_SYS_PLL2_1000M>;
+ clock-frequency = <333000000>;
+ clocks = <&clk IMX8MM_CLK_DISP_APB_ROOT>,
+ <&clk IMX8MM_CLK_CSI1_ROOT>,
+ <&clk IMX8MM_CLK_CSI1_PHY_REF>,
+ <&clk IMX8MM_CLK_DISP_AXI_ROOT>;
+ clock-names = "pclk", "wrap", "phy", "axi";
+ power-domains = <&disp_blk_ctrl IMX8MM_DISPBLK_PD_MIPI_CSI>;
+ status = "disabled";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ };
+
+ port@1 {
+ reg = <1>;
+
+ imx8mm_mipi_csi_out: endpoint {
+ remote-endpoint = <&csi_in>;
+ };
+ };
+ };
+ };
+
usbotg1: usb@32e40000 {
compatible = "fsl,imx8mm-usb", "fsl,imx7d-usb";
reg = <0x32e40000 0x200>;
--
2.32.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 2/5] arm64: dts: imx8mm: Add CSI nodes
2021-11-05 13:42 ` [PATCH 2/5] arm64: dts: imx8mm: Add CSI nodes Adam Ford
@ 2021-11-05 19:32 ` Tim Harvey
0 siblings, 0 replies; 9+ messages in thread
From: Tim Harvey @ 2021-11-05 19:32 UTC (permalink / raw)
To: Adam Ford
Cc: Linux ARM Mailing List, Schrempf Frieder, linux-media,
Laurent Pinchart, Adam Ford-BE, cstevens, Rob Herring, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
NXP Linux Team, Catalin Marinas, Will Deacon, Lucas Stach,
Peng Fan, Device Tree Mailing List, open list
On Fri, Nov 5, 2021 at 6:42 AM Adam Ford <aford173@gmail.com> wrote:
>
> There is a csi bridge and csis interface that tie together
> to allow csi2 capture.
>
> Signed-off-by: Adam Ford <aford173@gmail.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> arch/arm64/boot/dts/freescale/imx8mm.dtsi | 51 +++++++++++++++++++++++
> 1 file changed, 51 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> index c2f3f118f82e..1f69c14d953f 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> @@ -1068,6 +1068,22 @@ aips4: bus@32c00000 {
> #size-cells = <1>;
> ranges = <0x32c00000 0x32c00000 0x400000>;
>
> + csi: csi@32e20000 {
> + compatible = "fsl,imx8mm-csi", "fsl,imx7-csi";
> + reg = <0x32e20000 0x1000>;
> + interrupts = <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&clk IMX8MM_CLK_CSI1_ROOT>;
> + clock-names = "mclk";
> + power-domains = <&disp_blk_ctrl IMX8MM_DISPBLK_PD_CSI_BRIDGE>;
> + status = "disabled";
> +
> + port {
> + csi_in: endpoint {
> + remote-endpoint = <&imx8mm_mipi_csi_out>;
> + };
> + };
> + };
> +
> disp_blk_ctrl: blk-ctrl@32e28000 {
> compatible = "fsl,imx8mm-disp-blk-ctrl", "syscon";
> reg = <0x32e28000 0x100>;
> @@ -1095,6 +1111,41 @@ disp_blk_ctrl: blk-ctrl@32e28000 {
> #power-domain-cells = <1>;
> };
>
> + mipi_csi: mipi-csi@32e30000 {
> + compatible = "fsl,imx8mm-mipi-csi2";
> + reg = <0x32e30000 0x1000>;
> + interrupts = <GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>;
> + assigned-clocks = <&clk IMX8MM_CLK_CSI1_CORE>,
> + <&clk IMX8MM_CLK_CSI1_PHY_REF>;
> + assigned-clock-parents = <&clk IMX8MM_SYS_PLL2_1000M>,
> + <&clk IMX8MM_SYS_PLL2_1000M>;
> + clock-frequency = <333000000>;
> + clocks = <&clk IMX8MM_CLK_DISP_APB_ROOT>,
> + <&clk IMX8MM_CLK_CSI1_ROOT>,
> + <&clk IMX8MM_CLK_CSI1_PHY_REF>,
> + <&clk IMX8MM_CLK_DISP_AXI_ROOT>;
> + clock-names = "pclk", "wrap", "phy", "axi";
> + power-domains = <&disp_blk_ctrl IMX8MM_DISPBLK_PD_MIPI_CSI>;
> + status = "disabled";
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> + };
> +
> + port@1 {
> + reg = <1>;
> +
> + imx8mm_mipi_csi_out: endpoint {
> + remote-endpoint = <&csi_in>;
> + };
> + };
> + };
> + };
> +
> usbotg1: usb@32e40000 {
> compatible = "fsl,imx8mm-usb", "fsl,imx7d-usb";
> reg = <0x32e40000 0x200>;
> --
> 2.32.0
>
Adam,
With your new series I can also now capture without hanging from
imx219 on imx8mm-venice-gw73xx-0x:
media-ctl --links "'imx219 2-0010':0->'imx7-mipi-csis.0':0[1]"
media-ctl -v -V "'imx219 2-0010':0 [fmt:SRGGB8/640x480 field:none]"
media-ctl -v -V "'csi':0 [fmt:SRGGB8/640x480 field:none]"
media-ctl --print-topology
gst-launch-1.0 -v v4l2src num-buffers=1 !
video/x-bayer,format=rggb,width=640,height=480,framerate=30/1 !
filesink location=test.raw # not sure how to view this
gst-launch-1.0 -v v4l2src num-buffers=1 !
video/x-bayer,format=rggb,width=640,height=480,framerate=30/1 !
bayer2rgb ! filesink location=test.rgb # not sure how to view this
gst-launch-1.0 -v v4l2src num-buffers=1 !
video/x-bayer,format=rggb,width=640,height=480,framerate=30/1 !
bayer2rgb ! jpegenc ! filesink location=test.jpg # works - image is
good
gst-launch-1.0 -v v4l2src num-buffers=300 !
video/x-bayer,format=rggb,width=640,height=480,framerate=30/1 !
bayer2rgb ! jpegenc ! avimux ! filesink location=test.avi # works
-image is good
Thanks for your work on this!
For the series:
Reviewed-By: Tim Harvey <tharvey@gateworks.com>
Tested-By: Tim Harvey <tharvey@gateworks.com>
(imx8mm-venice-gw73xx-0x + rpiv2 IMX219 camera)
Best regards,
Tim
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 3/5] arm64: defconfig: Enable VIDEO_IMX_MEDIA
2021-11-05 13:42 [PATCH 1/5] soc: imx: imx8m-blk-ctrl: Fix imx8mm mipi reset Adam Ford
2021-11-05 13:42 ` [PATCH 2/5] arm64: dts: imx8mm: Add CSI nodes Adam Ford
@ 2021-11-05 13:42 ` Adam Ford
2021-11-05 13:42 ` [PATCH 4/5] arm64: dts: imx8mm-beacon: Enable OV5640 Camera Adam Ford
` (2 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Adam Ford @ 2021-11-05 13:42 UTC (permalink / raw)
To: linux-arm-kernel
Cc: tharvey, frieder.schrempf, linux-media, laurent.pinchart, aford,
cstevens, Adam Ford, Rob Herring, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
Catalin Marinas, Will Deacon, Lucas Stach, Peng Fan, devicetree,
linux-kernel
To use a camera, the CSIS and CSI drivers need to be enabled with
VIDEO_IMX_MEDIA.
Signed-off-by: Adam Ford <aford173@gmail.com>
---
arch/arm64/configs/defconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index f2e2b9bdd702..bc261cf2ef5a 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -958,6 +958,7 @@ CONFIG_MFD_CROS_EC_DEV=y
CONFIG_STAGING=y
CONFIG_STAGING_MEDIA=y
CONFIG_VIDEO_HANTRO=m
+CONFIG_VIDEO_IMX_MEDIA=m
CONFIG_CHROME_PLATFORMS=y
CONFIG_CROS_EC=y
CONFIG_CROS_EC_I2C=y
--
2.32.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 4/5] arm64: dts: imx8mm-beacon: Enable OV5640 Camera
2021-11-05 13:42 [PATCH 1/5] soc: imx: imx8m-blk-ctrl: Fix imx8mm mipi reset Adam Ford
2021-11-05 13:42 ` [PATCH 2/5] arm64: dts: imx8mm: Add CSI nodes Adam Ford
2021-11-05 13:42 ` [PATCH 3/5] arm64: defconfig: Enable VIDEO_IMX_MEDIA Adam Ford
@ 2021-11-05 13:42 ` Adam Ford
2021-11-05 13:42 ` [PATCH 5/5] arm64: defconfig: Enable OV5640 Adam Ford
2021-11-05 14:01 ` [PATCH 1/5] soc: imx: imx8m-blk-ctrl: Fix imx8mm mipi reset Lucas Stach
4 siblings, 0 replies; 9+ messages in thread
From: Adam Ford @ 2021-11-05 13:42 UTC (permalink / raw)
To: linux-arm-kernel
Cc: tharvey, frieder.schrempf, linux-media, laurent.pinchart, aford,
cstevens, Adam Ford, Rob Herring, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
Catalin Marinas, Will Deacon, Lucas Stach, Peng Fan, devicetree,
linux-kernel
The baseboard has support for a TDNext 5640 Camera which
uses an OV5640 connected to a 2-lane CSI2 interface.
With the CSI and mipi_csi2 drivers pointing to an OV5640 camera, the media
pipeline can be configured with the following:
media-ctl --links "'ov5640 1-003c':0->'imx7-mipi-csis.0':0[1]"
The camera and various nodes in the pipeline can be configured for UYVY:
media-ctl -v -V "'ov5640 1-003c':0 [fmt:UYVY8_1X16/640x480 field:none]"
media-ctl -v -V "'csi':0 [fmt:UYVY8_1X16/640x480 field:none]"
Signed-off-by: Adam Ford <aford173@gmail.com>
---
.../freescale/imx8mm-beacon-baseboard.dtsi | 58 +++++++++++++++++++
1 file changed, 58 insertions(+)
diff --git a/arch/arm64/boot/dts/freescale/imx8mm-beacon-baseboard.dtsi b/arch/arm64/boot/dts/freescale/imx8mm-beacon-baseboard.dtsi
index 6f5e63696ec0..0fb95f4a5e78 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm-beacon-baseboard.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm-beacon-baseboard.dtsi
@@ -43,6 +43,16 @@ reg_audio: regulator-audio {
enable-active-high;
};
+ reg_camera: regulator-camera {
+ compatible = "regulator-fixed";
+ regulator-name = "mipi_pwr";
+ regulator-min-microvolt = <2800000>;
+ regulator-max-microvolt = <2800000>;
+ gpio = <&pca6416_1 0 GPIO_ACTIVE_HIGH>;
+ enable-active-high;
+ startup-delay-us = <100000>;
+ };
+
reg_usdhc2_vmmc: regulator-usdhc2 {
compatible = "regulator-fixed";
regulator-name = "VSD_3V3";
@@ -67,6 +77,10 @@ sound {
};
};
+&csi {
+ status = "okay";
+};
+
&ecspi2 {
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_espi2>;
@@ -90,6 +104,30 @@ &i2c2 {
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_i2c2>;
status = "okay";
+
+ camera@3c {
+ compatible = "ovti,ov5640";
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_ov5640>;
+ reg = <0x3c>;
+ clocks = <&clk IMX8MM_CLK_CLKO1>;
+ clock-names = "xclk";
+ assigned-clocks = <&clk IMX8MM_CLK_CLKO1>;
+ assigned-clock-parents = <&clk IMX8MM_CLK_24M>;
+ assigned-clock-rates = <24000000>;
+ AVDD-supply = <®_camera>; /* 2.8v */
+ powerdown-gpios = <&gpio1 7 GPIO_ACTIVE_HIGH>;
+ reset-gpios = <&gpio1 6 GPIO_ACTIVE_LOW>;
+
+ port {
+ /* MIPI CSI-2 bus endpoint */
+ ov5640_to_mipi_csi2: endpoint {
+ remote-endpoint = <&imx8mm_mipi_csi_in>;
+ clock-lanes = <0>;
+ data-lanes = <1 2>;
+ };
+ };
+ };
};
&i2c4 {
@@ -141,6 +179,18 @@ pca6416_1: gpio@21 {
};
};
+&mipi_csi {
+ status = "okay";
+ ports {
+ port@0 {
+ imx8mm_mipi_csi_in: endpoint {
+ remote-endpoint = <&ov5640_to_mipi_csi2>;
+ data-lanes = <1 2>;
+ };
+ };
+ };
+};
+
&sai3 {
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_sai3>;
@@ -209,6 +259,14 @@ MX8MM_IOMUXC_SAI3_RXFS_GPIO4_IO28 0x41
>;
};
+ pinctrl_ov5640: ov5640grp {
+ fsl,pins = <
+ MX8MM_IOMUXC_GPIO1_IO07_GPIO1_IO7 0x19
+ MX8MM_IOMUXC_GPIO1_IO06_GPIO1_IO6 0x19
+ MX8MM_IOMUXC_GPIO1_IO14_CCMSRCGPCMIX_CLKO1 0x59
+ >;
+ };
+
pinctrl_pcal6414: pcal6414-gpiogrp {
fsl,pins = <
MX8MM_IOMUXC_SAI2_MCLK_GPIO4_IO27 0x19
--
2.32.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 5/5] arm64: defconfig: Enable OV5640
2021-11-05 13:42 [PATCH 1/5] soc: imx: imx8m-blk-ctrl: Fix imx8mm mipi reset Adam Ford
` (2 preceding siblings ...)
2021-11-05 13:42 ` [PATCH 4/5] arm64: dts: imx8mm-beacon: Enable OV5640 Camera Adam Ford
@ 2021-11-05 13:42 ` Adam Ford
2021-11-05 14:01 ` [PATCH 1/5] soc: imx: imx8m-blk-ctrl: Fix imx8mm mipi reset Lucas Stach
4 siblings, 0 replies; 9+ messages in thread
From: Adam Ford @ 2021-11-05 13:42 UTC (permalink / raw)
To: linux-arm-kernel
Cc: tharvey, frieder.schrempf, linux-media, laurent.pinchart, aford,
cstevens, Adam Ford, Rob Herring, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
Catalin Marinas, Will Deacon, Lucas Stach, Peng Fan, devicetree,
linux-kernel
The Beacon EmbeddedWorks imx8mm development kit has a TD Next 5640
Camera. Enable the OV5640 driver to use the camera.
Signed-off-by: Adam Ford <aford173@gmail.com>
---
arch/arm64/configs/defconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index bc261cf2ef5a..4c1eb9aae5e5 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -668,6 +668,7 @@ CONFIG_VIDEO_QCOM_VENUS=m
CONFIG_SDR_PLATFORM_DRIVERS=y
CONFIG_VIDEO_RCAR_DRIF=m
CONFIG_VIDEO_IMX219=m
+CONFIG_VIDEO_OV5640=m
CONFIG_VIDEO_OV5645=m
CONFIG_VIDEO_QCOM_CAMSS=m
CONFIG_DRM=m
--
2.32.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 1/5] soc: imx: imx8m-blk-ctrl: Fix imx8mm mipi reset
2021-11-05 13:42 [PATCH 1/5] soc: imx: imx8m-blk-ctrl: Fix imx8mm mipi reset Adam Ford
` (3 preceding siblings ...)
2021-11-05 13:42 ` [PATCH 5/5] arm64: defconfig: Enable OV5640 Adam Ford
@ 2021-11-05 14:01 ` Lucas Stach
2021-11-05 14:21 ` Adam Ford
4 siblings, 1 reply; 9+ messages in thread
From: Lucas Stach @ 2021-11-05 14:01 UTC (permalink / raw)
To: Adam Ford, linux-arm-kernel
Cc: tharvey, frieder.schrempf, linux-media, laurent.pinchart, aford,
cstevens, Rob Herring, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
Catalin Marinas, Will Deacon, Peng Fan, devicetree, linux-kernel
Hi Adam,
Am Freitag, dem 05.11.2021 um 08:42 -0500 schrieb Adam Ford:
> Most of the blk-ctrl reset bits are found in one register, however
> there are two bits in offset 8 for pulling the MIPI DPHY out of reset
> and these need to be set when IMX8MM_DISPBLK_PD_MIPI_CSI is brought
> out of reset or the MIPI_CSI hangs.
>
I'm undecided yet if I like this solution, as register 0x8 has
different content on different instances of the blk-ctrl, so naming it
mipi_rst_mask is confusing for others.
My initial thought was that we should habe a MIPI PHY driver
controlling all the registers in the disp-blk-ctrl, other than SFT_RSTN
and CLK_EN, however I see how the reset handling for the PHY would then
be inconsistent with how we handle all the other devices in the disp-
blk domain. But then the HW guys seemed to think along the same lines,
as they placed the PHY resets into the PHY registers, instead of the
SFT_RSTN, which had more than enough spare bits to carry those
resets...
> Fixes: 926e57c065df ("soc: imx: imx8m-blk-ctrl: add DISP blk-ctrl")
> Signed-off-by: Adam Ford <aford173@gmail.com>
> ---
> drivers/soc/imx/imx8m-blk-ctrl.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/soc/imx/imx8m-blk-ctrl.c b/drivers/soc/imx/imx8m-blk-ctrl.c
> index 519b3651d1d9..5506bd075c35 100644
> --- a/drivers/soc/imx/imx8m-blk-ctrl.c
> +++ b/drivers/soc/imx/imx8m-blk-ctrl.c
> @@ -17,6 +17,7 @@
>
> #define BLK_SFT_RSTN 0x0
> #define BLK_CLK_EN 0x4
> +#define BLK_MIPI_RESET_DIV 0x8
>
> struct imx8m_blk_ctrl_domain;
>
> @@ -36,6 +37,7 @@ struct imx8m_blk_ctrl_domain_data {
> const char *gpc_name;
> u32 rst_mask;
> u32 clk_mask;
> + u32 mipi_rst_mask;
> };
>
> #define DOMAIN_MAX_CLKS 3
> @@ -78,6 +80,7 @@ static int imx8m_blk_ctrl_power_on(struct generic_pm_domain *genpd)
>
> /* put devices into reset */
> regmap_clear_bits(bc->regmap, BLK_SFT_RSTN, data->rst_mask);
> + regmap_clear_bits(bc->regmap, BLK_MIPI_RESET_DIV, data->mipi_rst_mask);
>
> /* enable upstream and blk-ctrl clocks to allow reset to propagate */
> ret = clk_bulk_prepare_enable(data->num_clks, domain->clks);
> @@ -99,6 +102,7 @@ static int imx8m_blk_ctrl_power_on(struct generic_pm_domain *genpd)
>
> /* release reset */
> regmap_set_bits(bc->regmap, BLK_SFT_RSTN, data->rst_mask);
> + regmap_set_bits(bc->regmap, BLK_MIPI_RESET_DIV, data->mipi_rst_mask);
>
> /* disable upstream clocks */
> clk_bulk_disable_unprepare(data->num_clks, domain->clks);
> @@ -122,6 +126,7 @@ static int imx8m_blk_ctrl_power_off(struct generic_pm_domain *genpd)
> /* put devices into reset and disable clocks */
> regmap_clear_bits(bc->regmap, BLK_SFT_RSTN, data->rst_mask);
> regmap_clear_bits(bc->regmap, BLK_CLK_EN, data->clk_mask);
> + regmap_clear_bits(bc->regmap, BLK_MIPI_RESET_DIV, data->mipi_rst_mask);
For consistency the reset assertion should be put before the clock
disable.
Regards,
Lucas
>
> /* power down upstream GPC domain */
> pm_runtime_put(domain->power_dev);
> @@ -488,6 +493,7 @@ static const struct imx8m_blk_ctrl_domain_data imx8mm_disp_blk_ctl_domain_data[]
> .gpc_name = "mipi-csi",
> .rst_mask = BIT(3) | BIT(4),
> .clk_mask = BIT(10) | BIT(11),
> + .mipi_rst_mask = BIT(16) | BIT(17),
> },
> };
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 1/5] soc: imx: imx8m-blk-ctrl: Fix imx8mm mipi reset
2021-11-05 14:01 ` [PATCH 1/5] soc: imx: imx8m-blk-ctrl: Fix imx8mm mipi reset Lucas Stach
@ 2021-11-05 14:21 ` Adam Ford
2021-11-05 14:36 ` Lucas Stach
0 siblings, 1 reply; 9+ messages in thread
From: Adam Ford @ 2021-11-05 14:21 UTC (permalink / raw)
To: Lucas Stach
Cc: arm-soc, Tim Harvey, Schrempf Frieder, linux-media,
Laurent Pinchart, Adam Ford-BE, cstevens, Rob Herring, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
NXP Linux Team, Catalin Marinas, Will Deacon, Peng Fan,
devicetree, Linux Kernel Mailing List
On Fri, Nov 5, 2021 at 9:01 AM Lucas Stach <l.stach@pengutronix.de> wrote:
>
> Hi Adam,
>
> Am Freitag, dem 05.11.2021 um 08:42 -0500 schrieb Adam Ford:
> > Most of the blk-ctrl reset bits are found in one register, however
> > there are two bits in offset 8 for pulling the MIPI DPHY out of reset
> > and these need to be set when IMX8MM_DISPBLK_PD_MIPI_CSI is brought
> > out of reset or the MIPI_CSI hangs.
> >
> I'm undecided yet if I like this solution, as register 0x8 has
> different content on different instances of the blk-ctrl, so naming it
> mipi_rst_mask is confusing for others.
Both Mini and Nano use it, so I could add a comment that this
register only applies to Mini and Nano. For all others, it would be
empty and do nothing anyway. If you have a name you'd prefer, I can
do that.
>
> My initial thought was that we should habe a MIPI PHY driver
> controlling all the registers in the disp-blk-ctrl, other than SFT_RSTN
> and CLK_EN, however I see how the reset handling for the PHY would then
> be inconsistent with how we handle all the other devices in the disp-
> blk domain. But then the HW guys seemed to think along the same lines,
> as they placed the PHY resets into the PHY registers, instead of the
> SFT_RSTN, which had more than enough spare bits to carry those
> resets...
The NXP ATF code sets these bits in the GPC code with the other power
domain stuff, so it seems like they intended it to be used as a power
domain thing and not MIPI PHY driver.
>
> > Fixes: 926e57c065df ("soc: imx: imx8m-blk-ctrl: add DISP blk-ctrl")
> > Signed-off-by: Adam Ford <aford173@gmail.com>
> > ---
> > drivers/soc/imx/imx8m-blk-ctrl.c | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/drivers/soc/imx/imx8m-blk-ctrl.c b/drivers/soc/imx/imx8m-blk-ctrl.c
> > index 519b3651d1d9..5506bd075c35 100644
> > --- a/drivers/soc/imx/imx8m-blk-ctrl.c
> > +++ b/drivers/soc/imx/imx8m-blk-ctrl.c
> > @@ -17,6 +17,7 @@
> >
> > #define BLK_SFT_RSTN 0x0
> > #define BLK_CLK_EN 0x4
> > +#define BLK_MIPI_RESET_DIV 0x8
> >
> > struct imx8m_blk_ctrl_domain;
> >
> > @@ -36,6 +37,7 @@ struct imx8m_blk_ctrl_domain_data {
> > const char *gpc_name;
> > u32 rst_mask;
> > u32 clk_mask;
> > + u32 mipi_rst_mask;
> > };
> >
> > #define DOMAIN_MAX_CLKS 3
> > @@ -78,6 +80,7 @@ static int imx8m_blk_ctrl_power_on(struct generic_pm_domain *genpd)
> >
> > /* put devices into reset */
> > regmap_clear_bits(bc->regmap, BLK_SFT_RSTN, data->rst_mask);
> > + regmap_clear_bits(bc->regmap, BLK_MIPI_RESET_DIV, data->mipi_rst_mask);
> >
> > /* enable upstream and blk-ctrl clocks to allow reset to propagate */
> > ret = clk_bulk_prepare_enable(data->num_clks, domain->clks);
> > @@ -99,6 +102,7 @@ static int imx8m_blk_ctrl_power_on(struct generic_pm_domain *genpd)
> >
> > /* release reset */
> > regmap_set_bits(bc->regmap, BLK_SFT_RSTN, data->rst_mask);
> > + regmap_set_bits(bc->regmap, BLK_MIPI_RESET_DIV, data->mipi_rst_mask);
> >
> > /* disable upstream clocks */
> > clk_bulk_disable_unprepare(data->num_clks, domain->clks);
> > @@ -122,6 +126,7 @@ static int imx8m_blk_ctrl_power_off(struct generic_pm_domain *genpd)
> > /* put devices into reset and disable clocks */
> > regmap_clear_bits(bc->regmap, BLK_SFT_RSTN, data->rst_mask);
> > regmap_clear_bits(bc->regmap, BLK_CLK_EN, data->clk_mask);
> > + regmap_clear_bits(bc->regmap, BLK_MIPI_RESET_DIV, data->mipi_rst_mask);
>
> For consistency the reset assertion should be put before the clock
> disable.
I can do that. How do you want me to proceed?
adam
>
> Regards,
> Lucas
>
> >
> > /* power down upstream GPC domain */
> > pm_runtime_put(domain->power_dev);
> > @@ -488,6 +493,7 @@ static const struct imx8m_blk_ctrl_domain_data imx8mm_disp_blk_ctl_domain_data[]
> > .gpc_name = "mipi-csi",
> > .rst_mask = BIT(3) | BIT(4),
> > .clk_mask = BIT(10) | BIT(11),
> > + .mipi_rst_mask = BIT(16) | BIT(17),
> > },
> > };
> >
>
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 1/5] soc: imx: imx8m-blk-ctrl: Fix imx8mm mipi reset
2021-11-05 14:21 ` Adam Ford
@ 2021-11-05 14:36 ` Lucas Stach
0 siblings, 0 replies; 9+ messages in thread
From: Lucas Stach @ 2021-11-05 14:36 UTC (permalink / raw)
To: Adam Ford
Cc: arm-soc, Tim Harvey, Schrempf Frieder, linux-media,
Laurent Pinchart, Adam Ford-BE, cstevens, Rob Herring, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
NXP Linux Team, Catalin Marinas, Will Deacon, Peng Fan,
devicetree, Linux Kernel Mailing List
Am Freitag, dem 05.11.2021 um 09:21 -0500 schrieb Adam Ford:
> On Fri, Nov 5, 2021 at 9:01 AM Lucas Stach <l.stach@pengutronix.de> wrote:
> >
> > Hi Adam,
> >
> > Am Freitag, dem 05.11.2021 um 08:42 -0500 schrieb Adam Ford:
> > > Most of the blk-ctrl reset bits are found in one register, however
> > > there are two bits in offset 8 for pulling the MIPI DPHY out of reset
> > > and these need to be set when IMX8MM_DISPBLK_PD_MIPI_CSI is brought
> > > out of reset or the MIPI_CSI hangs.
> > >
> > I'm undecided yet if I like this solution, as register 0x8 has
> > different content on different instances of the blk-ctrl, so naming it
> > mipi_rst_mask is confusing for others.
>
> Both Mini and Nano use it, so I could add a comment that this
> register only applies to Mini and Nano. For all others, it would be
> empty and do nothing anyway. If you have a name you'd prefer, I can
> do that.
>
Register 0x8 in the vpu-blk-ctrl also has a very different meaning. :)
I don't really have a better suggestion for the name. Maybe just keep
it or mipi_phy_rst_mask or something, but a comment would certainly be
welcome.
> >
> > My initial thought was that we should habe a MIPI PHY driver
> > controlling all the registers in the disp-blk-ctrl, other than SFT_RSTN
> > and CLK_EN, however I see how the reset handling for the PHY would then
> > be inconsistent with how we handle all the other devices in the disp-
> > blk domain. But then the HW guys seemed to think along the same lines,
> > as they placed the PHY resets into the PHY registers, instead of the
> > SFT_RSTN, which had more than enough spare bits to carry those
> > resets...
>
> The NXP ATF code sets these bits in the GPC code with the other power
> domain stuff, so it seems like they intended it to be used as a power
> domain thing and not MIPI PHY driver.
Yea, maybe the HW guys just misplaced those resets and it certainly
makes sense to handle the reset from the power-domain. While the MIPI
PHY has no direct connection to a SoC bus and should thus be unable to
corrupt SoC state it seems like a good idea to bring it into a clean
state after powering up the domain.
>
> >
> > > Fixes: 926e57c065df ("soc: imx: imx8m-blk-ctrl: add DISP blk-ctrl")
> > > Signed-off-by: Adam Ford <aford173@gmail.com>
> > > ---
> > > drivers/soc/imx/imx8m-blk-ctrl.c | 6 ++++++
> > > 1 file changed, 6 insertions(+)
> > >
> > > diff --git a/drivers/soc/imx/imx8m-blk-ctrl.c b/drivers/soc/imx/imx8m-blk-ctrl.c
> > > index 519b3651d1d9..5506bd075c35 100644
> > > --- a/drivers/soc/imx/imx8m-blk-ctrl.c
> > > +++ b/drivers/soc/imx/imx8m-blk-ctrl.c
> > > @@ -17,6 +17,7 @@
> > >
> > > #define BLK_SFT_RSTN 0x0
> > > #define BLK_CLK_EN 0x4
> > > +#define BLK_MIPI_RESET_DIV 0x8
> > >
> > > struct imx8m_blk_ctrl_domain;
> > >
> > > @@ -36,6 +37,7 @@ struct imx8m_blk_ctrl_domain_data {
> > > const char *gpc_name;
> > > u32 rst_mask;
> > > u32 clk_mask;
> > > + u32 mipi_rst_mask;
> > > };
> > >
> > > #define DOMAIN_MAX_CLKS 3
> > > @@ -78,6 +80,7 @@ static int imx8m_blk_ctrl_power_on(struct generic_pm_domain *genpd)
> > >
> > > /* put devices into reset */
> > > regmap_clear_bits(bc->regmap, BLK_SFT_RSTN, data->rst_mask);
> > > + regmap_clear_bits(bc->regmap, BLK_MIPI_RESET_DIV, data->mipi_rst_mask);
Please guard all of those in a if (data->mipi_rst_mask). Regmap doesn't
check if the mask is 0, so we can avoid some MMIO accesses with this
condition and at the same time make it clear in the code that this
additional reset doesn't apply to most of the blk-ctrl instances.
> > >
> > > /* enable upstream and blk-ctrl clocks to allow reset to propagate */
> > > ret = clk_bulk_prepare_enable(data->num_clks, domain->clks);
> > > @@ -99,6 +102,7 @@ static int imx8m_blk_ctrl_power_on(struct generic_pm_domain *genpd)
> > >
> > > /* release reset */
> > > regmap_set_bits(bc->regmap, BLK_SFT_RSTN, data->rst_mask);
> > > + regmap_set_bits(bc->regmap, BLK_MIPI_RESET_DIV, data->mipi_rst_mask);
> > >
> > > /* disable upstream clocks */
> > > clk_bulk_disable_unprepare(data->num_clks, domain->clks);
> > > @@ -122,6 +126,7 @@ static int imx8m_blk_ctrl_power_off(struct generic_pm_domain *genpd)
> > > /* put devices into reset and disable clocks */
> > > regmap_clear_bits(bc->regmap, BLK_SFT_RSTN, data->rst_mask);
> > > regmap_clear_bits(bc->regmap, BLK_CLK_EN, data->clk_mask);
> > > + regmap_clear_bits(bc->regmap, BLK_MIPI_RESET_DIV, data->mipi_rst_mask);
> >
> > For consistency the reset assertion should be put before the clock
> > disable.
>
> I can do that. How do you want me to proceed?
> > >
I agree that we should do this in the blk-ctrl driver.
Regards,
Lucas
> > > /* power down upstream GPC domain */
> > > pm_runtime_put(domain->power_dev);
> > > @@ -488,6 +493,7 @@ static const struct imx8m_blk_ctrl_domain_data imx8mm_disp_blk_ctl_domain_data[]
> > > .gpc_name = "mipi-csi",
> > > .rst_mask = BIT(3) | BIT(4),
> > > .clk_mask = BIT(10) | BIT(11),
> > > + .mipi_rst_mask = BIT(16) | BIT(17),
> > > },
> > > };
> > >
> >
> >
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 9+ messages in thread