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

[Dan Carpenter <dan.carpenter@linaro.org>]

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