| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20250421214128.GR17813@pendragon.ideasonboard.com>
Date: Tue, 22 Apr 2025 00:41:28 +0300
From: Laurent Pinchart <laurent.pinchart@...asonboard.com>
To: Frank Li <Frank.Li@....com>
Cc: Mauro Carvalho Chehab <mchehab@...nel.org>,
Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, Shawn Guo <shawnguo@...nel.org>,
Sascha Hauer <s.hauer@...gutronix.de>,
Pengutronix Kernel Team <kernel@...gutronix.de>,
Fabio Estevam <festevam@...il.com>,
Rui Miguel Silva <rmfrfs@...il.com>,
Martin Kepplinger <martink@...teo.de>,
Purism Kernel Team <kernel@...i.sm>, linux-media@...r.kernel.org,
devicetree@...r.kernel.org, imx@...ts.linux.dev,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
Robert Chiras <robert.chiras@....com>,
"Guoniu.zhou" <guoniu.zhou@....com>
Subject: Re: [PATCH v4 10/13] media: imx8mq-mipi-csi2: Add support for
i.MX8QXP
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@....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@...asonboard.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
Powered by blists - more mailing lists