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

CK Hu
2019-06-10T02:23:55Z

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";
> + }
> +