Re: [RFC PATCH V3 3/4] dt-bindings: mt8183: Add sensor interface dt-bindings

[CK Hu <ck.hu@mediatek.com>]

Hi, Louis:

It looks like that "mediatek,mt8183-seninf" has many common part with
"mediatek,mt2712-mipicsi" [1] and "mediatek,mt2712-mipicsi-common" [2].

The evidence comes from each driver.

The register definition of mipi_rx phy is

"mediatek,mt2712-mipicsi"

+#define MIPI_RX_ANA00_CSI				0x00
+#define MIPI_RX_ANA04_CSI				0x04
+#define MIPI_RX_ANA08_CSI				0x08
+#define MIPI_RX_ANA0C_CSI				0x0c
+#define MIPI_RX_ANA10_CSI				0x10
+#define MIPI_RX_ANA20_CSI				0x20
+#define MIPI_RX_ANA24_CSI				0x24

"mediatek,mt8183-seninf"

+#define MIPI_RX_ANA00_CSI0A           0x0000
+#define MIPI_RX_ANA04_CSI0A           0x0004
+#define MIPI_RX_ANA08_CSI0A           0x0008
+#define MIPI_RX_ANA0C_CSI0A           0x000C
+#define MIPI_RX_ANA10_CSI0A           0x0010
+#define MIPI_RX_ANA20_CSI0A           0x0020
+#define MIPI_RX_ANA24_CSI0A           0x0024

The register definition of sensor interface top is

"mediatek,mt2712-mipicsi-common"

+#define SENINF_TOP_CTRL					0x00
+#define SENINF_TOP_CMODEL_PAR				0x04
+#define SENINF_TOP_MUX					0x08
+
+#define SENINF_MUX_CTRL					0x00

"mediatek,mt8183-seninf"

+#define SENINF_TOP_CTRL                        0x0000
+#define SENINF_TOP_CMODEL_PAR                  0x0004
+#define SENINF_TOP_MUX_CTRL                    0x0008
+#define rsv_000C                               0x000C
+#define SENINF_TOP_CAM_MUX_CTRL                0x0010

The register definition of csi is

"mediatek,mt2712-mipicsi"

+#define SENINF_NCSI2_CTL				0xA0
+#define SENINF_NCSI2_LNRD_TIMING			0xA8
+#define SENINF_NCSI2_INT_EN				0xB0
+#define SENINF_NCSI2_INT_STATUS				0xB4
+#define SENINF_NCSI2_DBG_SEL				0xB8
+#define SENINF_NCSI2_HSRX_DBG				0xD8
+#define SENINF_NCSI2_DI					0xDC
+#define SENINF_NCSI2_DI_CTRL				0xE4

"mediatek,mt8183-seninf"

+#define SENINF1_CSI2_CTL                       0x0A00
+#define SENINF1_CSI2_LNRD_TIMING               0x0A08
+#define SENINF1_CSI2_INT_EN                    0x0A10
+#define SENINF1_CSI2_INT_STATUS                0x0A14
+#define SENINF1_CSI2_DGB_SEL                   0x0A18
+#define SENINF1_CSI2_HSRX_DBG                  0x0A38
+#define SENINF1_CSI2_DI                        0x0A3C
+#define SENINF1_CSI2_DI_CTRL                   0x0A44

Because they have many common part, the common part should be merged and
keep the different part for each SoC. Here is an example for common part
and different part.

Mtk_hdmi_phy (Common part)
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/mediatek/mtk_hdmi_phy.c?h=v5.2-rc4
Mtk_hdmi_phy (MT2701 part)
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/mediatek/mtk_mt2701_hdmi_phy.c?h=v5.2-rc4
Mtk_hdmi_phy (MT8173 part)
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/mediatek/mtk_mt8173_hdmi_phy.c?h=v5.2-rc4

So the first step is to common the binding document. Maybe you should
discuss with Stu to achieve this target.

[1] https://patchwork.kernel.org/patch/10974567/
[2] https://patchwork.kernel.org/patch/10974559/

Regards,
CK

On Thu, 2019-06-06 at 18:00 +0800, Louis Kuo wrote:
> This patch adds the DT binding documentation for the sensor interface
> module in Mediatek SoCs.
> 
> Signed-off-by: Louis Kuo <louis.kuo@mediatek.com>
> ---
>  .../devicetree/bindings/media/mediatek-seninf.txt  | 31 ++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/mediatek-seninf.txt
> 
> diff --git a/Documentation/devicetree/bindings/media/mediatek-seninf.txt b/Documentation/devicetree/bindings/media/mediatek-seninf.txt
> new file mode 100644
> index 0000000..979063a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/mediatek-seninf.txt
> @@ -0,0 +1,31 @@
> +* Mediatek seninf MIPI-CSI2 host driver
> +
> +Seninf MIPI-CSI2 host driver is a HW camera interface controller. It support a widely adopted,
> +simple, high-speed protocol primarily intended for point-to-point image and video
> +transmission between cameras and host devices.
> +
> +Required properties:
> +  - compatible: "mediatek,mt8183-seninf"
> +  - reg: Must contain an entry for each entry in reg-names.
> +  - reg-names: Must include the following entries:
> +    "base_reg": seninf registers base
> +    "rx_reg": Rx analog registers base
> +  - interrupts: interrupt number to the cpu.
> +  - clocks : clock name from clock manager
> +  - clock-names: must be CLK_CAM_SENINF and CLK_TOP_MUX_SENINF.
> +    It is the clocks of seninf
> +
> +Example:
> +	seninf: seninf@1a040000 {
> +		compatible = "mediatek,mt8183-seninf";
> +		reg = <0 0x1a040000 0 0x8000>,
> +		      <0 0x11C80000 0 0x6000>;
> +		reg-names = "base_reg", "rx_reg";
> +		interrupts = <GIC_SPI 251 IRQ_TYPE_LEVEL_LOW>;
> +		power-domains = <&scpsys MT8183_POWER_DOMAIN_CAM>;
> +		clocks =
> +				<&camsys CLK_CAM_SENINF>, <&topckgen CLK_TOP_MUX_SENINF>;
> +		clock-names =
> +			"CLK_CAM_SENINF", "CLK_TOP_MUX_SENINF";
> +	}
> +