linux-kernel - Re: [PATCH 04/14] media: sun4i-csi: Fix [HV]sync polarity handling

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]

Date:   Mon, 16 Jan 2023 13:03:59 +0300
From:   Oleg Verych <olecom@...il.com>
To:     wens@...nel.org
Cc:     devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-kernel@...r.kernel.org, linux-media@...r.kernel.org,
        mark.rutland@....com, mchehab@...nel.org, mripard@...nel.org,
        robh+dt@...nel.org, sakari.ailus@...ux.intel.com, wens@...e.org,
        Oleg Verych <olecom@...il.com>
Subject: Re: [PATCH 04/14] media: sun4i-csi: Fix [HV]sync polarity handling

Hi, Chen-Yu Tsai

> -	hsync_pol = !!(bus->flags & V4L2_MBUS_HSYNC_ACTIVE_HIGH);
> -	vsync_pol = !!(bus->flags & V4L2_MBUS_VSYNC_ACTIVE_HIGH);
> +	/*
> +	 * This hardware uses [HV]REF instead of [HV]SYNC. Based on the
> +	 * provided timing diagrams in the manual, positive polarity
> +	 * equals active high [HV]REF.
> +	 *
> +	 * When the back porch is 0, [HV]REF is more or less equivalent
> +	 * to [HV]SYNC inverted.
> +	 */
> +	href_pol = !!(bus->flags & V4L2_MBUS_HSYNC_ACTIVE_LOW);
> +	vref_pol = !!(bus->flags & V4L2_MBUS_VSYNC_ACTIVE_LOW);

After this change has been made there is a need of explicit explanation
of what "Active high" / "Active low" in dts really mean.

Currently physical high/low voltage levels are like that:
(I'm not sure about vsync-active)

* hsync-active = <0>; /* HSYNC active 'low' => wire active is 'high' */
  CSI register setting: href_pol: 1,

That is confusing:

[PATCH v6 5/5] DO NOT MERGE: ARM: dts: bananapi: Add Camera support
https://lore.kernel.org/linux-arm-kernel/cf0e40b0bca9219d2bb023a5b7f23bad8baba1e5.1562847292.git-series.maxime.ripard@bootlin.com/#r

> +	port {
> +		csi_from_ov5640: endpoint {
> +                        remote-endpoint = <&ov5640_to_csi>;
> +                        bus-width = <8>;
> +                        hsync-active = <1>; /* Active high */

original CSI driver

> +                        vsync-active = <0>; /* Active low */
> +                        data-active = <1>;  /* Active high */
> +                        pclk-sample = <1>;  /* Rising */
> +                };
> +	};

[PATCH 13/14] [DO NOT MERGE] ARM: dts: sun7i: cubieboard2: Enable OV7670 camera on CSI1
https://lore.kernel.org/linux-arm-kernel/20191215165924.28314-14-wens@kernel.org/

> +	port {
> +		/* Parallel bus endpoint */
> +		csi_from_ov7670: endpoint {
> +			remote-endpoint = <&ov7670_to_csi>;
> +			bus-width = <8>;
> +			/* driver is broken */
> +			hsync-active = <0>; /* Active high */

this change patchset

> +			vsync-active = <1>; /* Active high */
> +			data-active = <1>;  /* Active high */
> +			pclk-sample = <1>;  /* Rising */
> +		};

> +			ov7670_to_csi: endpoint {
> +				remote-endpoint = <&csi_from_ov7670>;
> +				bus-width = <8>;
> +				hsync-active = <1>; /* Active high */

this patcheset

> +				vsync-active = <1>; /* Active high */
> +				data-active = <1>;  /* Active high */
> +				pclk-sample = <1>;  /* Rising */
> +			};
> +		};