Re: [PATCH v2 08/13] staging: media: starfive: Add for StarFive ISP 3A SC

Dan Carpenter
2024-01-11T11:04:59Z

On Thu, Jan 11, 2024 at 12:41:15AM -0800, Changhuang Liang wrote:
> Register ISP 3A "capture_scd" video device to receive statistics
> collection data.
>
> Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
> ---
> .../staging/media/starfive/camss/stf-buffer.h | 1 +
> .../staging/media/starfive/camss/stf-camss.c | 8 +
> .../media/starfive/camss/stf-capture.c | 21 ++-
> .../media/starfive/camss/stf-isp-hw-ops.c | 66 ++++++++
> .../staging/media/starfive/camss/stf-isp.h | 23 +++
> .../staging/media/starfive/camss/stf-video.c | 143 +++++++++++++++++-
> .../staging/media/starfive/camss/stf-video.h | 1 +
> 7 files changed, 254 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/staging/media/starfive/camss/stf-buffer.h b/drivers/staging/media/starfive/camss/stf-buffer.h
> index 9d1670fb05ed..727d00617448 100644
> --- a/drivers/staging/media/starfive/camss/stf-buffer.h
> +++ b/drivers/staging/media/starfive/camss/stf-buffer.h
> @@ -23,6 +23,7 @@ enum stf_v_state {
> struct stfcamss_buffer {
> struct vb2_v4l2_buffer vb;
> dma_addr_t addr[2];
> + void *vaddr;
> struct list_head queue;
> };
>
> diff --git a/drivers/staging/media/starfive/camss/stf-camss.c b/drivers/staging/media/starfive/camss/stf-camss.c
> index a587f860101a..3175d0d9a05c 100644
> --- a/drivers/staging/media/starfive/camss/stf-camss.c
> +++ b/drivers/staging/media/starfive/camss/stf-camss.c
> @@ -126,6 +126,7 @@ static int stfcamss_of_parse_ports(struct stfcamss *stfcamss)
> static int stfcamss_register_devs(struct stfcamss *stfcamss)
> {
> struct stf_capture *cap_yuv = &stfcamss->captures[STF_CAPTURE_YUV];
> + struct stf_capture *cap_scd = &stfcamss->captures[STF_CAPTURE_SCD];
> struct stf_isp_dev *isp_dev = &stfcamss->isp_dev;
> int ret;
>
> @@ -150,6 +151,13 @@ static int stfcamss_register_devs(struct stfcamss *stfcamss)
>
> cap_yuv->video.source_subdev = &isp_dev->subdev;
>
> + ret = media_create_pad_link(&isp_dev->subdev.entity, STF_ISP_PAD_SRC_SCD,
> + &cap_scd->video.vdev.entity, 0, 0);
> + if (ret)
> + goto err_cap_unregister;
> +

Don't we need to do something like

media_entity_remove_links(&cap_yuv->video.vdev.entity);

as part of the cleanup? Also where do we clean up this new call to
media_create_pad_link() in the unregister process?

> + cap_scd->video.source_subdev = &isp_dev->subdev;
> +
> return ret;
>
> err_cap_unregister:

regards,
dan carpenter