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

From: Oleg Verych
Date: Mon Jan 16 2023 - 05:05:08 EST


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@xxxxxxxxxxx/#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@xxxxxxxxxx/

> + 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 */
> + };
> + };