Re: [RFC PATCH V7 1/3] media: platform: mtk-isp: Add Mediatek sensor interface driver

On Fri, Jul 17, 2020 at 5:47 PM Chun-Kuang Hu [off-list ref] wrote:

Tomasz Figa [off-list ref] 於 2020年7月17日 週五 下午10:33寫道：

quoted

On Fri, Jul 17, 2020 at 4:21 PM Chun-Kuang Hu [off-list ref] wrote:

quoted

Hi, Louis:

Louis Kuo (郭德寧) [off-list ref] 於 2020年7月17日 週五 上午10:56寫道：

quoted

Hi Chun-Kuang,

Since phy driver is not belong to V4L2 scope

Should I need to upsteam 8183 mipi phy driver with new a patch other than this one ?

Yes, I think so. Maybe different series would be better.

Since both would not be usable without each other, I believe they
should be handled by the same series, although care should be taken to
have the patches in the series clearly separated between the two
subsystems.

One series is ok for me. For each maintainer, they could pick what
they want from this series, or get ack by another maintainer and pick
all patches into one tree.

Sounds good to me.

However, I'm currently reviewing the series, so would appreciate
holding on with the resend.

Best regards,
Tomasz

Regards,
Chun-Kuang.

quoted

Best regards,
Tomasz

quoted

Regards,
Chun-Kuang.

quoted

BRs
Louis

-----Original Message-----
From: Chun-Kuang Hu [mailto:chunkuang.hu@kernel.org]
Sent: Thursday, July 9, 2020 9:13 PM
To: Louis Kuo (郭德寧)
Cc: hans.verkuil@cisco.com; laurent.pinchart+renesas@ideasonboard.com; Tomasz Figa; keiichiw@chromium.org; Matthias Brugger; Mauro Carvalho Chehab; devicetree@vger.kernel.org; Sean Cheng (鄭昇弘); srv_heupstream; Jerry-ch Chen (陳敬憲); Jungo Lin (林明俊); Sj Huang (黃信璋); yuzhao@chromium.org; moderated list:ARM/Mediatek SoC support; zwisler@chromium.org; Christie Yu (游雅惠); Frederic Chen (陳俊元); Linux ARM; linux-media@vger.kernel.org
Subject: Re: [RFC PATCH V7 1/3] media: platform: mtk-isp: Add Mediatek sensor interface driver

Hi, Louis:

Louis Kuo [off-list ref] 於 2020年7月8日 週三 下午6:41寫道：

quoted

This patch adds Mediatek's sensor interface driver. Sensor interface
driver is a MIPI-CSI2 host driver, namely, 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. The mtk-isp directory will contain drivers
for multiple IP blocks found in Mediatek ISP system. It will include
ISP Pass 1 driver, sensor interface driver, DIP driver and face detection driver.

Signed-off-by: Louis Kuo <redacted>
---
 drivers/media/platform/Makefile               |    1 +
 drivers/media/platform/mtk-isp/Kconfig        |   18 +
 drivers/media/platform/mtk-isp/Makefile       |    3 +
 .../media/platform/mtk-isp/seninf/Makefile    |    7 +
 .../platform/mtk-isp/seninf/mtk_seninf.c      |  974 +++++++++++
 .../platform/mtk-isp/seninf/mtk_seninf_dphy.c |  353 ++++

I think phy driver should be placed in drivers/phy/mediatek and separate phy driver to an independent patch.

quoted

 .../platform/mtk-isp/seninf/mtk_seninf_reg.h  | 1491 +++++++++++++++++
 .../mtk-isp/seninf/mtk_seninf_rx_reg.h        |  515 ++++++
 8 files changed, 3362 insertions(+)
 create mode 100644 drivers/media/platform/mtk-isp/Kconfig
 create mode 100644 drivers/media/platform/mtk-isp/Makefile
 create mode 100644 drivers/media/platform/mtk-isp/seninf/Makefile
 create mode 100644 drivers/media/platform/mtk-isp/seninf/mtk_seninf.c
 create mode 100644
drivers/media/platform/mtk-isp/seninf/mtk_seninf_dphy.c
 create mode 100644
drivers/media/platform/mtk-isp/seninf/mtk_seninf_reg.h
 create mode 100644
drivers/media/platform/mtk-isp/seninf/mtk_seninf_rx_reg.h

[snip]

quoted

+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/of_graph.h>
+#include <linux/of_irq.h>

No irq handler, so remove this.

quoted

+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/slab.h>
+#include <linux/videodev2.h>
+#include <media/v4l2-async.h>
+#include <media/v4l2-ctrls.h>
+#include <media/v4l2-event.h>
+#include <media/v4l2-fwnode.h>
+#include <media/v4l2-subdev.h>
+#include <linux/phy/phy.h>
+#include "mtk_seninf_reg.h"
+

[snip]

quoted

+
+static int seninf_set_ctrl(struct v4l2_ctrl *ctrl) {
+       struct mtk_seninf *priv = container_of(ctrl->handler,
+                                            struct mtk_seninf,
+ctrl_handler);
+
+       switch (ctrl->id) {
+       case V4L2_CID_TEST_PATTERN:
+               if (ctrl->val == TEST_GEN_PATTERN)
+                       return seninf_enable_test_pattern(priv);

Without this, this driver still works, so move this to an independent patch.

quoted

+               else if (ctrl->val == TEST_DUMP_DEBUG_INFO)
+                       return seninf_dump_debug_info(priv);

Ditto.

quoted

+               else
+                       return -EINVAL;
+       }
+
+       return 0;
+}
+

[snip]

quoted

+
+#ifdef CONFIG_OF
+static const struct of_device_id mtk_mipi_dphy_of_match[] = {
+       {.compatible = "mediatek,mt8183-mipi_dphy"},

Where is the definition of "mediatek,mt8183-mipi_dphy"?

Regards,
Chun-Kuang.

quoted

+       {},
+};
+MODULE_DEVICE_TABLE(of, mtk_mipi_dphy_of_match); #endif
+
+static struct platform_driver mipi_dphy_pdrv = {
+       .probe  = mipi_dphy_probe,
+       .driver = {
+               .name   = "mipi_dphy",
+               .of_match_table = of_match_ptr(mtk_mipi_dphy_of_match),
+       },
+};
+
+module_platform_driver(mipi_dphy_pdrv);
+