Hi, Laurent: On Wed, 2024-07-31 at 11:29 +0300, Laurent Pinchart wrote: > > External email : Please do not click links or open attachments until you have verified the sender or the content. > Hi CK, > > On Wed, Jul 31, 2024 at 02:59:51AM +0000, CK Hu (胡俊光) wrote: > > On Mon, 2024-07-29 at 16:48 +0200, Julien Stephan wrote: > > > From: Phi-bang Nguyen <pnguyen@xxxxxxxxxxxx> > > > > > > This driver provides a path to bypass the SoC ISP so that image data > > > coming from the SENINF can go directly into memory without any image > > > processing. This allows the use of an external ISP. > > > > > > Signed-off-by: Phi-bang Nguyen <pnguyen@xxxxxxxxxxxx> > > > Signed-off-by: Florian Sylvestre <fsylvestre@xxxxxxxxxxxx> > > > [Paul Elder fix irq locking] > > > Signed-off-by: Paul Elder <paul.elder@xxxxxxxxxxxxxxxx> > > > Co-developed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> > > > Co-developed-by: Julien Stephan <jstephan@xxxxxxxxxxxx> > > > Signed-off-by: Julien Stephan <jstephan@xxxxxxxxxxxx> > > > --- > > > > [snip] > > > > > + > > > +static void mtk_cam_cmos_vf_enable(struct mtk_cam_dev *cam_dev, > > > + bool enable, bool pak_en) > > > +{ > > > +struct device *dev = cam_dev->dev; > > > + > > > +if (pm_runtime_get_sync(dev) < 0) { > > > +dev_err(dev, "failed to get pm_runtime\n"); > > > +goto out; > > > +} > > > + > > > +if (enable) > > > +cam_dev->hw_functions->mtk_cam_cmos_vf_hw_enable(cam_dev); > > > > Directly call mtk_camsv30_cmos_vf_hw_enable(). > > The goal, when this was developed, was to support multiple generations > of hardware with a single driver. I think it's a worthwhile goal, but at > the same time, I'm not sure that will ever happen as I'm not aware of > plans to upstream Genio 350 and 500 support (which is a bad sad, as it's > more or less working out-of-tree). I'm thus fine either way, and if we > think the most likely outcome is that this driver will only support > Genio 300, I'm fine dropping the abstraction layer. I know this goal. For the mtk_camsv_30_setup(), in new SoC, if only one line in this function is different, should we duplicate the whole function and modify only one line? I think we don't know what would happen in future, so we should not design for something which we have no any information. Regards, CK > > > > +else > > > +cam_dev->hw_functions->mtk_cam_cmos_vf_hw_disable(cam_dev); > > > > Directly call mtk_camsv30_cmos_vf_hw_disable(). > > > > > + > > > +out: > > > +pm_runtime_put_autosuspend(dev); > > > +} > > > + > > -- > Regards, > > Laurent Pinchart