linux-kernel - Re: [PATCH v6 3/5] media: platform: mediatek: isp

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
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <90e13cea78a15ca85edce1ca6cdf2c972fe3aa01.camel@mediatek.com>
Date: Wed, 31 Jul 2024 06:19:49 +0000
From: CK Hu (胡俊光) <ck.hu@...iatek.com>
To: "mchehab@...nel.org" <mchehab@...nel.org>, "conor+dt@...nel.org"
	<conor+dt@...nel.org>, "robh@...nel.org" <robh@...nel.org>,
	Andy Hsieh (謝智皓) <Andy.Hsieh@...iatek.com>,
	"jstephan@...libre.com" <jstephan@...libre.com>, "matthias.bgg@...il.com"
	<matthias.bgg@...il.com>, "laurent.pinchart@...asonboard.com"
	<laurent.pinchart@...asonboard.com>, "krzk+dt@...nel.org"
	<krzk+dt@...nel.org>, "angelogioacchino.delregno@...labora.com"
	<angelogioacchino.delregno@...labora.com>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-mediatek@...ts.infradead.org" <linux-mediatek@...ts.infradead.org>,
	"linux-media@...r.kernel.org" <linux-media@...r.kernel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>,
	Louis Kuo (郭德寧) <louis.kuo@...iatek.com>,
	"fsylvestre@...libre.com" <fsylvestre@...libre.com>, "pnguyen@...libre.com"
	<pnguyen@...libre.com>
Subject: Re: [PATCH v6 3/5] media: platform: mediatek: isp_30: add mediatek
 ISP3.0 sensor interface

Hi, Julien:

On Mon, 2024-07-29 at 16:48 +0200, Julien Stephan wrote:
>  	 
> External email : Please do not click links or open attachments until you have verified the sender or the content.
>  From: Louis Kuo <louis.kuo@...iatek.com>
> 
> This will add the mediatek ISP3.0 seninf (sensor interface) driver found
> on several Mediatek SoCs such as the mt8365.
> 
> Then seninf module has 4 physical CSI-2 inputs. Depending on the soc they
> may not be all connected.
> 
> Signed-off-by: Louis Kuo <louis.kuo@...iatek.com>
> Signed-off-by: Phi-bang Nguyen <pnguyen@...libre.com>
> Signed-off-by: Florian Sylvestre <fsylvestre@...libre.com>
> Co-developed-by: Laurent Pinchart <laurent.pinchart@...asonboard.com>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@...asonboard.com>
> Co-developed-by: Julien Stephan <jstephan@...libre.com>
> Signed-off-by: Julien Stephan <jstephan@...libre.com>
> ---

[snip]

> +static int mtk_seninf_input_setup_csi2(struct mtk_seninf *priv,
> +struct mtk_seninf_input *input,
> +struct v4l2_subdev_state *state)
> +{
> +const struct mtk_seninf_format_info *fmtinfo;
> +const struct v4l2_mbus_framefmt *format;
> +unsigned int num_data_lanes = input->bus.num_data_lanes;
> +unsigned int val = 0;
> +s64 clock_count;
> +
> +format = v4l2_subdev_state_get_format(state, input->pad, 0);
> +fmtinfo = mtk_seninf_format_info(format->code);
> +
> +/* Configure timestamp */
> +writel(SENINF_TIMESTAMP_STEP, input->base + SENINF_TG1_TM_STP);
> +
> +/* HQ */
> +/*
> + * Configure phase counter. Zero means:
> + * - Sensor master clock: ISP_CLK
> + * - Sensor clock polarity: Rising edge
> + * - Sensor reset deasserted
> + * - Sensor powered up
> + * - Pixel clock inversion disabled
> + * - Sensor master clock polarity disabled
> + * - Phase counter disabled
> + */
> +writel(0x0, input->base + SENINF_TG1_PH_CNT);
> +
> +clock_count = mtk_seninf_get_clk_divider(priv, input->seninf_id, fmtinfo->bpp,
> + num_data_lanes);
> +if (clock_count < 0)
> +return clock_count;
> +
> +clock_count = FIELD_PREP(GENMASK(21, 16), clock_count) | 0x1;
> +writel(clock_count, input->base + SENINF_TG1_SEN_CK);
> +
> +/* First Enable Sensor interface and select pad (0x1a04_0200) */
> +mtk_seninf_input_update(input, SENINF_CTRL, SENINF_EN, 1);
> +mtk_seninf_input_update(input, SENINF_CTRL, PAD2CAM_DATA_SEL, SENINF_PAD_10BIT);
> +mtk_seninf_input_update(input, SENINF_CTRL, SENINF_SRC_SEL, 0);
> +mtk_seninf_input_update(input, SENINF_CTRL_EXT, SENINF_CSI2_IP_EN, 1);
> +mtk_seninf_input_update(input, SENINF_CTRL_EXT, SENINF_NCSI2_IP_EN, 0);
> +
> +/* DPCM Enable */
> +if (fmtinfo->flags & MTK_SENINF_FORMAT_DPCM)
> +val = SENINF_CSI2_DPCM_DI_2A_DPCM_EN;
> +else
> +val = SENINF_CSI2_DPCM_DI_30_DPCM_EN;
> +writel(val, input->base + SENINF_CSI2_DPCM);
> +
> +/* Settle delay */
> +mtk_seninf_input_update(input, SENINF_CSI2_LNRD_TIMING,
> +DATA_SETTLE_PARAMETER, SENINF_SETTLE_DELAY);
> +
> +/* CSI2 control */
> +val = readl(input->base + SENINF_CSI2_CTL)
> +      | (FIELD_PREP(SENINF_CSI2_CTL_ED_SEL, DATA_HEADER_ORDER_DI_WCL_WCH)
> +      | SENINF_CSI2_CTL_CLOCK_LANE_EN | (BIT(num_data_lanes) - 1));
> +writel(val, input->base + SENINF_CSI2_CTL);
> +
> +mtk_seninf_input_update(input, SENINF_CSI2_RESYNC_MERGE_CTL, BYPASS_LANE_RESYNC, 0);
> +mtk_seninf_input_update(input, SENINF_CSI2_RESYNC_MERGE_CTL, CDPHY_SEL, 0);
> +mtk_seninf_input_update(input, SENINF_CSI2_RESYNC_MERGE_CTL, CPHY_LANE_RESYNC_CNT, 3);
> +mtk_seninf_input_update(input, SENINF_CSI2_MODE, CSR_CSI2_MODE, 0);
> +mtk_seninf_input_update(input, SENINF_CSI2_MODE, CSR_CSI2_HEADER_LEN, 0);
> +mtk_seninf_input_update(input, SENINF_CSI2_DPHY_SYNC, SYNC_SEQ_MASK_0, 0xff00);
> +mtk_seninf_input_update(input, SENINF_CSI2_DPHY_SYNC, SYNC_SEQ_PAT_0, 0x001d);
> +
> +mtk_seninf_input_update(input, SENINF_CSI2_CTL, CLOCK_HS_OPTION, 0);
> +mtk_seninf_input_update(input, SENINF_CSI2_CTL, HSRX_DET_EN, 0);
> +mtk_seninf_input_update(input, SENINF_CSI2_CTL, HS_TRAIL_EN, 1);
> +mtk_seninf_input_update(input, SENINF_CSI2_HS_TRAIL, HS_TRAIL_PARAMETER,
> +SENINF_HS_TRAIL_PARAMETER);
> +
> +/* Set debug port to output packet number */
> +mtk_seninf_input_update(input, SENINF_CSI2_DGB_SEL, DEBUG_EN, 1);
> +mtk_seninf_input_update(input, SENINF_CSI2_DGB_SEL, DEBUG_SEL, 0x1a);
> +
> +/* HQ */
> +writel(0xfffffffe, input->base + SENINF_CSI2_SPARE0);
> +
> +/* Enable CSI2 IRQ mask */
> +/* Turn on all interrupt */
> +writel(0xffffffff, input->base + SENINF_CSI2_INT_EN);
> +/* Write clear CSI2 IRQ */
> +writel(0xffffffff, input->base + SENINF_CSI2_INT_STATUS);
> +writel(0xffffffff, input->base + SENINF_CSI2_INT_STATUS_EXT);
> +/* Enable CSI2 Extend IRQ mask */
> +writel(0xffffffff, input->base + SENINF_CSI2_INT_EN_EXT);

Does the IRQ means the CPU would receive an interrupt?
But this driver does not request interrupt handler, so enable IRQ seems redundant.

Regards,
CK

> +/* Reset the CSI2 to commit changes */
> +mtk_seninf_input_update(input, SENINF_CTRL, CSI2_SW_RST, 1);
> +udelay(1);
> +mtk_seninf_input_update(input, SENINF_CTRL, CSI2_SW_RST, 0);
> +
> +return 0;
> +}
> +
>