Hi Tommaso,
On Mon, Nov 27, 2023 at 12:27:37PM +0100, Tommaso Merciai wrote:
> Hi Laurent,
> Patch looks good to me.
>
> Some clarification:
>
> I'm working as you know on alvium driver actually v14 sent.
> Need to wait for a feedback on this or I can send v15 with the following
> missing stuff:
>
> diff --git a/drivers/media/i2c/alvium-csi2.c b/drivers/media/i2c/alvium-csi2.c
> index 1c6450c9b994..3f4104ab3094 100644
> --- a/drivers/media/i2c/alvium-csi2.c
> +++ b/drivers/media/i2c/alvium-csi2.c
> @@ -1850,8 +1850,8 @@ static int alvium_s_stream(struct v4l2_subdev *sd, int enable)
> return ret;
> }
>
> -static int alvium_init_cfg(struct v4l2_subdev *sd,
> - struct v4l2_subdev_state *state)
> +static int alvium_init_state(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state)
> {
> struct alvium_dev *alvium = sd_to_alvium(sd);
> struct alvium_mode *mode = &alvium->mode;
> @@ -2233,7 +2233,6 @@ static const struct v4l2_subdev_video_ops alvium_video_ops = {
> };
>
> static const struct v4l2_subdev_pad_ops alvium_pad_ops = {
> - .init_cfg = alvium_init_cfg,
> .enum_mbus_code = alvium_enum_mbus_code,
> .get_fmt = v4l2_subdev_get_fmt,
> .set_fmt = alvium_set_fmt,
> @@ -2241,6 +2240,10 @@ static const struct v4l2_subdev_pad_ops alvium_pad_ops = {
> .set_selection = alvium_set_selection,
> };
>
> +static const struct v4l2_subdev_internal_ops alvium_internal_ops = {
> + .init_state = alvium_init_state,
> +};
> +
> static const struct v4l2_subdev_ops alvium_subdev_ops = {
> .core = &alvium_core_ops,
> .pad = &alvium_pad_ops,
> @@ -2271,6 +2274,7 @@ static int alvium_subdev_init(struct alvium_dev *alvium)
> /* init alvium sd */
> v4l2_i2c_subdev_init(sd, client, &alvium_subdev_ops);
>
> + sd->internal_ops = &alvium_internal_ops;
> sd->flags |= V4L2_SUBDEV_FL_HAS_EVENTS | V4L2_SUBDEV_FL_HAS_DEVNODE;
> alvium->pad.flags = MEDIA_PAD_FL_SOURCE;
> sd->entity.function = MEDIA_ENT_F_CAM_SENSOR;
>
> Rebased this morning on top of Sakari master branch :)
As far as I understand, Sakari will send a pull request that will
include this patch in a few days. I think you can already base the next
version of the Alvium driver on top of his master branch. Sakari, please
let us know if this is not correct.
By the way, I forgot to CC you, but I have posted the next version of
the .[gs]_frame_interval rework ([1]). I don't mind rebasing on top of
your driver in case it gets merged first.
[1] https://lore.kernel.org/linux-media/20231127111359.30315-1-laurent.pinchart@xxxxxxxxxxxxxxxx/T/#t
> On Mon, Nov 27, 2023 at 11:07:44AM +0200, Laurent Pinchart wrote:
> > The subdev .init_cfg() operation is affected by two issues:
> >
> > - It has long been extended to initialize a whole v4l2_subdev_state
> > instead of just a v4l2_subdev_pad_config, but its name has stuck
> > around.
> >
> > - Despite operating on a whole subdev state and not being directly
> > exposed to the subdev users (either in-kernel or through the userspace
> > API), .init_cfg() is categorized as a subdev pad operation.
> >
> > This participates in making the subdev API confusing for new developers.
> > Fix it by renaming the operation to .init_state(), and make it a subdev
> > internal operation.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> > Acked-by: Michael Riesch <michael.riesch@xxxxxxxxxxxxxx> # for imx415
> > Acked-by: Shuah Khan <skhan@xxxxxxxxxxxxxxxxxxx> # for vimc
> > ---
> > Changes since v3:
> >
> > - Rebase on top of the new stm32-dcmipp driver
> > - Fix uninitialized variable in __v4l2_subdev_state_alloc() due to bad
> > rebase
> >
> > Changes since v2:
> >
> > - Rebase on top of the latest media tree
> >
> > Changes since v1:
> >
> > - Fix compilation of the vsp1 driver
> > ---
> > drivers/media/i2c/adv7180.c | 10 ++--
> > drivers/media/i2c/ccs/ccs-core.c | 6 +--
> > drivers/media/i2c/ds90ub913.c | 10 ++--
> > drivers/media/i2c/ds90ub953.c | 10 ++--
> > drivers/media/i2c/ds90ub960.c | 11 ++--
> > drivers/media/i2c/gc2145.c | 10 ++--
> > drivers/media/i2c/hi846.c | 10 ++--
> > drivers/media/i2c/imx214.c | 12 +++--
> > drivers/media/i2c/imx219.c | 9 ++--
> > drivers/media/i2c/imx290.c | 10 ++--
> > drivers/media/i2c/imx296.c | 10 ++--
> > drivers/media/i2c/imx334.c | 12 +++--
> > drivers/media/i2c/imx335.c | 12 +++--
> > drivers/media/i2c/imx412.c | 12 +++--
> > drivers/media/i2c/imx415.c | 10 ++--
> > drivers/media/i2c/mt9m001.c | 10 ++--
> > drivers/media/i2c/mt9m111.c | 10 ++--
> > drivers/media/i2c/mt9m114.c | 16 +++---
> > drivers/media/i2c/mt9p031.c | 8 +--
> > drivers/media/i2c/mt9v111.c | 10 ++--
> > drivers/media/i2c/ov01a10.c | 10 ++--
> > drivers/media/i2c/ov02a10.c | 10 ++--
> > drivers/media/i2c/ov2640.c | 10 ++--
> > drivers/media/i2c/ov2680.c | 10 ++--
> > drivers/media/i2c/ov2740.c | 10 ++--
> > drivers/media/i2c/ov5640.c | 10 ++--
> > drivers/media/i2c/ov5645.c | 12 +++--
> > drivers/media/i2c/ov5670.c | 10 ++--
> > drivers/media/i2c/ov7251.c | 12 +++--
> > drivers/media/i2c/ov8858.c | 10 ++--
> > drivers/media/i2c/ov9282.c | 12 +++--
> > drivers/media/i2c/st-vgxy61.c | 10 ++--
> > drivers/media/i2c/tc358746.c | 10 ++--
> > drivers/media/i2c/tda1997x.c | 10 ++--
> > drivers/media/i2c/thp7312.c | 10 ++--
> > drivers/media/i2c/tvp5150.c | 6 +--
> > drivers/media/pci/intel/ivsc/mei_csi.c | 10 ++--
> > drivers/media/platform/cadence/cdns-csi2rx.c | 10 ++--
> > .../platform/microchip/microchip-csi2dc.c | 10 ++--
> > .../platform/microchip/microchip-isc-scaler.c | 10 ++--
> > drivers/media/platform/nxp/imx-mipi-csis.c | 10 ++--
> > drivers/media/platform/nxp/imx7-media-csi.c | 6 +--
> > .../platform/nxp/imx8-isi/imx8-isi-crossbar.c | 10 ++--
> > .../platform/nxp/imx8-isi/imx8-isi-pipe.c | 10 ++--
> > drivers/media/platform/nxp/imx8mq-mipi-csi2.c | 10 ++--
> > .../platform/renesas/rzg2l-cru/rzg2l-csi2.c | 10 ++--
> > .../platform/renesas/rzg2l-cru/rzg2l-ip.c | 10 ++--
> > .../media/platform/renesas/vsp1/vsp1_brx.c | 1 -
> > .../media/platform/renesas/vsp1/vsp1_clu.c | 1 -
> > .../media/platform/renesas/vsp1/vsp1_entity.c | 53 +++++++++----------
> > .../media/platform/renesas/vsp1/vsp1_entity.h | 2 -
> > .../media/platform/renesas/vsp1/vsp1_hsit.c | 1 -
> > .../media/platform/renesas/vsp1/vsp1_lif.c | 1 -
> > .../media/platform/renesas/vsp1/vsp1_lut.c | 1 -
> > .../media/platform/renesas/vsp1/vsp1_rwpf.c | 1 -
> > .../media/platform/renesas/vsp1/vsp1_sru.c | 1 -
> > .../media/platform/renesas/vsp1/vsp1_uds.c | 1 -
> > .../media/platform/renesas/vsp1/vsp1_uif.c | 1 -
> > .../platform/rockchip/rkisp1/rkisp1-csi.c | 10 ++--
> > .../platform/rockchip/rkisp1/rkisp1-isp.c | 10 ++--
> > .../platform/rockchip/rkisp1/rkisp1-resizer.c | 10 ++--
> > .../st/stm32/stm32-dcmipp/dcmipp-byteproc.c | 6 +--
> > .../st/stm32/stm32-dcmipp/dcmipp-parallel.c | 6 +--
> > .../platform/sunxi/sun4i-csi/sun4i_csi.c | 1 +
> > .../platform/sunxi/sun4i-csi/sun4i_csi.h | 1 +
> > .../platform/sunxi/sun4i-csi/sun4i_v4l2.c | 9 ++--
> > .../sunxi/sun6i-csi/sun6i_csi_bridge.c | 10 ++--
> > .../sunxi/sun6i-mipi-csi2/sun6i_mipi_csi2.c | 10 ++--
> > .../sun8i_a83t_mipi_csi2.c | 10 ++--
> > drivers/media/platform/ti/cal/cal-camerarx.c | 10 ++--
> > drivers/media/platform/video-mux.c | 10 ++--
> > .../media/platform/xilinx/xilinx-csi2rxss.c | 10 ++--
> > .../media/test-drivers/vimc/vimc-debayer.c | 11 ++--
> > drivers/media/test-drivers/vimc/vimc-scaler.c | 11 ++--
> > drivers/media/test-drivers/vimc/vimc-sensor.c | 11 ++--
> > drivers/media/v4l2-core/v4l2-subdev.c | 20 +++----
> > drivers/staging/media/imx/imx-ic-prp.c | 2 +-
> > drivers/staging/media/imx/imx-ic-prpencvf.c | 2 +-
> > drivers/staging/media/imx/imx-media-csi.c | 2 +-
> > drivers/staging/media/imx/imx-media-utils.c | 8 +--
> > drivers/staging/media/imx/imx-media-vdic.c | 2 +-
> > drivers/staging/media/imx/imx-media.h | 4 +-
> > drivers/staging/media/imx/imx6-mipi-csi2.c | 2 +-
> > .../staging/media/starfive/camss/stf-isp.c | 6 ++-
> > .../media/sunxi/sun6i-isp/sun6i_isp_proc.c | 10 ++--
> > include/media/v4l2-subdev.h | 7 +--
> > 86 files changed, 487 insertions(+), 264 deletions(-)
[snip]
--
Regards,
Laurent Pinchart
