On Fri, Jul 17, 2020 at 4:21 PM Chun-Kuang Hu <chunkuang.hu@xxxxxxxxxx> wrote: > > Hi, Louis: > > Louis Kuo (郭德寧) <louis.kuo@xxxxxxxxxxxx> 於 2020年7月17日 週五 上午10:56寫道： > > > > 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. Best regards, Tomasz > > Regards, > Chun-Kuang. > > > > > BRs > > Louis > > > > -----Original Message----- > > From: Chun-Kuang Hu [mailto:chunkuang.hu@xxxxxxxxxx] > > Sent: Thursday, July 9, 2020 9:13 PM > > To: Louis Kuo (郭德寧) > > Cc: hans.verkuil@xxxxxxxxx; laurent.pinchart+renesas@xxxxxxxxxxxxxxxx; Tomasz Figa; keiichiw@xxxxxxxxxxxx; Matthias Brugger; Mauro Carvalho Chehab; devicetree@xxxxxxxxxxxxxxx; Sean Cheng (鄭昇弘); srv_heupstream; Jerry-ch Chen (陳敬憲); Jungo Lin (林明俊); Sj Huang (黃信璋); yuzhao@xxxxxxxxxxxx; moderated list:ARM/Mediatek SoC support; zwisler@xxxxxxxxxxxx; Christie Yu (游雅惠); Frederic Chen (陳俊元); Linux ARM; linux-media@xxxxxxxxxxxxxxx > > Subject: Re: [RFC PATCH V7 1/3] media: platform: mtk-isp: Add Mediatek sensor interface driver > > > > Hi, Louis: > > > > Louis Kuo <louis.kuo@xxxxxxxxxxxx> 於 2020年7月8日 週三 下午6:41寫道： > > > > > > 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 <louis.kuo@xxxxxxxxxxxx> > > > --- > > > 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. > > > > > .../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] > > > > > + > > > +#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. > > > > > +#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] > > > > > + > > > +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. > > > > > + else if (ctrl->val == TEST_DUMP_DEBUG_INFO) > > > + return seninf_dump_debug_info(priv); > > > > Ditto. > > > > > + else > > > + return -EINVAL; > > > + } > > > + > > > + return 0; > > > +} > > > + > > > > [snip] > > > > > + > > > +#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. > > > > > + {}, > > > +}; > > > +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); > > > +
