| Message ID | 20250428161643.321617-1-stanislaw.gruszka@linux.intel.com |
|---|---|
| Headers | show |
| Series | media: intel/ipu6: initial ipu7 code sharing preparation | expand |
Hi Stanislaw, On Mon, Apr 28, 2025 at 06:16:40PM +0200, Stanislaw Gruszka wrote: > isys back pointer of ipu6_isys_subdev structure is only used to get > pointer to struct device. We can use device pointer directly, what > would allow to refactor ipu6 subdev code to make it more independent. > > Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com> > --- > drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c | 15 ++++----------- > drivers/media/pci/intel/ipu6/ipu6-isys-subdev.c | 10 ++++------ > drivers/media/pci/intel/ipu6/ipu6-isys-subdev.h | 8 ++------ > 3 files changed, 10 insertions(+), 23 deletions(-) > > diff --git a/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c b/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c > index 685ef81969ac..543f81b1899f 100644 > --- a/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c > +++ b/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c > @@ -98,12 +98,8 @@ s64 ipu6_isys_csi2_get_link_freq(struct ipu6_isys_csi2 *csi2) > static int csi2_subscribe_event(struct v4l2_subdev *sd, struct v4l2_fh *fh, > struct v4l2_event_subscription *sub) > { > - struct ipu6_isys_subdev *asd = to_ipu6_isys_subdev(sd); > - struct ipu6_isys_csi2 *csi2 = to_ipu6_isys_csi2(asd); > - struct device *dev = &csi2->isys->adev->auxdev.dev; > - > - dev_dbg(dev, "csi2 subscribe event(type %u id %u)\n", > - sub->type, sub->id); > + dev_dbg(sd->dev, "csi2 subscribe event(type %u id %u)\n", sub->type, > + sub->id); > > switch (sub->type) { > case V4L2_EVENT_FRAME_SYNC: > @@ -402,8 +398,6 @@ static int ipu6_isys_csi2_set_sel(struct v4l2_subdev *sd, > struct v4l2_subdev_state *state, > struct v4l2_subdev_selection *sel) > { > - struct ipu6_isys_subdev *asd = to_ipu6_isys_subdev(sd); > - struct device *dev = &asd->isys->adev->auxdev.dev; > struct v4l2_mbus_framefmt *sink_ffmt; > struct v4l2_mbus_framefmt *src_ffmt; > struct v4l2_rect *crop; > @@ -442,7 +436,7 @@ static int ipu6_isys_csi2_set_sel(struct v4l2_subdev *sd, > src_ffmt->code = ipu6_isys_convert_bayer_order(sink_ffmt->code, > sel->r.left, > sel->r.top); > - dev_dbg(dev, "set crop for %s sel: %d,%d,%d,%d code: 0x%x\n", > + dev_dbg(sd->dev, "set crop for %s sel: %d,%d,%d,%d code: 0x%x\n", > sd->name, sel->r.left, sel->r.top, sel->r.width, sel->r.height, > src_ffmt->code); > > @@ -532,8 +526,7 @@ int ipu6_isys_csi2_init(struct ipu6_isys_csi2 *csi2, > csi2->port = index; > > csi2->asd.sd.entity.ops = &csi2_entity_ops; > - csi2->asd.isys = isys; > - ret = ipu6_isys_subdev_init(&csi2->asd, &csi2_sd_ops, 0, > + ret = ipu6_isys_subdev_init(&csi2->asd, dev, &csi2_sd_ops, 0, > NR_OF_CSI2_SINK_PADS, NR_OF_CSI2_SRC_PADS); > if (ret) > goto fail; > diff --git a/drivers/media/pci/intel/ipu6/ipu6-isys-subdev.c b/drivers/media/pci/intel/ipu6/ipu6-isys-subdev.c > index e5ce76c17ca3..7c6125dc4af4 100644 > --- a/drivers/media/pci/intel/ipu6/ipu6-isys-subdev.c > +++ b/drivers/media/pci/intel/ipu6/ipu6-isys-subdev.c > @@ -265,10 +265,9 @@ static const struct v4l2_subdev_internal_ops ipu6_isys_subdev_internal_ops = { > .init_state = ipu6_isys_subdev_init_state, > }; > > -int ipu6_isys_subdev_init(struct ipu6_isys_subdev *asd, > +int ipu6_isys_subdev_init(struct ipu6_isys_subdev *asd, struct device *dev, I wouldn't pass a function arguments that it already has access to through other means, i.e. dev in this case. That being said, having a macro to obtain a struct device pointer based on sub-device or isys could be nice, it could be called e.g. ipu_isys_to_dev(). As this patch is simply moving the code around I'd add this in a separate patch. > const struct v4l2_subdev_ops *ops, > - unsigned int nr_ctrls, > - unsigned int num_sink_pads, > + unsigned int nr_ctrls, unsigned int num_sink_pads, > unsigned int num_source_pads) > { > unsigned int num_pads = num_sink_pads + num_source_pads; > @@ -281,12 +280,11 @@ int ipu6_isys_subdev_init(struct ipu6_isys_subdev *asd, > V4L2_SUBDEV_FL_HAS_EVENTS | > V4L2_SUBDEV_FL_STREAMS; > asd->sd.owner = THIS_MODULE; > - asd->sd.dev = &asd->isys->adev->auxdev.dev; > + asd->sd.dev = dev; > asd->sd.entity.function = MEDIA_ENT_F_VID_IF_BRIDGE; > asd->sd.internal_ops = &ipu6_isys_subdev_internal_ops; > > - asd->pad = devm_kcalloc(&asd->isys->adev->auxdev.dev, num_pads, > - sizeof(*asd->pad), GFP_KERNEL); > + asd->pad = devm_kcalloc(dev, num_pads, sizeof(*asd->pad), GFP_KERNEL); > if (!asd->pad) > return -ENOMEM; > > diff --git a/drivers/media/pci/intel/ipu6/ipu6-isys-subdev.h b/drivers/media/pci/intel/ipu6/ipu6-isys-subdev.h > index 46a2ede867f0..732e6ecc927a 100644 > --- a/drivers/media/pci/intel/ipu6/ipu6-isys-subdev.h > +++ b/drivers/media/pci/intel/ipu6/ipu6-isys-subdev.h > @@ -12,11 +12,8 @@ > > #include "ipu-isys.h" > > -struct ipu6_isys; > - > struct ipu6_isys_subdev { > struct v4l2_subdev sd; > - struct ipu6_isys *isys; > u32 const *supported_codes; > struct media_pad *pad; > struct v4l2_ctrl_handler ctrl_handler; > @@ -39,10 +36,9 @@ int ipu6_isys_subdev_enum_mbus_code(struct v4l2_subdev *sd, > struct v4l2_subdev_state *state, > struct v4l2_subdev_mbus_code_enum > *code); > -int ipu6_isys_subdev_init(struct ipu6_isys_subdev *asd, > +int ipu6_isys_subdev_init(struct ipu6_isys_subdev *asd, struct device *dev, > const struct v4l2_subdev_ops *ops, > - unsigned int nr_ctrls, > - unsigned int num_sink_pads, > + unsigned int nr_ctrls, unsigned int num_sink_pads, > unsigned int num_source_pads); > void ipu6_isys_subdev_cleanup(struct ipu6_isys_subdev *asd); > #endif /* IPU6_ISYS_SUBDEV_H */