[PATCH v2 2/2] media: mediatek: amend vpu_get_plat_device() documentation

Johan Hovold posted 2 patches 1 month, 2 weeks ago
[PATCH v2 2/2] media: mediatek: amend vpu_get_plat_device() documentation
Posted by Johan Hovold 1 month, 2 weeks ago
Add a comment to the vpu_get_plat_device() documentation to make it
clear that the VPU platform device is returned with an incremented
reference count (which needs to be dropped after use).

Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/media/platform/mediatek/vpu/mtk_vpu.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/mediatek/vpu/mtk_vpu.h b/drivers/media/platform/mediatek/vpu/mtk_vpu.h
index da05f3e74081..5808311d2b15 100644
--- a/drivers/media/platform/mediatek/vpu/mtk_vpu.h
+++ b/drivers/media/platform/mediatek/vpu/mtk_vpu.h
@@ -120,7 +120,7 @@ int vpu_ipi_send(struct platform_device *pdev,
  *		device for using VPU API.
  *
  * Return: Return NULL if it is failed.
- * otherwise it is VPU's platform device
+ * otherwise it is VPU's platform device with incremented reference count
  **/
 struct platform_device *vpu_get_plat_device(struct platform_device *pdev);
 
-- 
2.51.0
Re: [PATCH v2 2/2] media: mediatek: amend vpu_get_plat_device() documentation
Posted by Nicolas Dufresne 1 week ago
Hi,

Le mardi 28 octobre 2025 à 11:04 +0100, Johan Hovold a écrit :
> Add a comment to the vpu_get_plat_device() documentation to make it
> clear that the VPU platform device is returned with an incremented
> reference count (which needs to be dropped after use).
> 
> Signed-off-by: Johan Hovold <johan@kernel.org>
> ---
>  drivers/media/platform/mediatek/vpu/mtk_vpu.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/mediatek/vpu/mtk_vpu.h b/drivers/media/platform/mediatek/vpu/mtk_vpu.h
> index da05f3e74081..5808311d2b15 100644
> --- a/drivers/media/platform/mediatek/vpu/mtk_vpu.h
> +++ b/drivers/media/platform/mediatek/vpu/mtk_vpu.h
> @@ -120,7 +120,7 @@ int vpu_ipi_send(struct platform_device *pdev,
>   *		device for using VPU API.
>   *
>   * Return: Return NULL if it is failed.
> - * otherwise it is VPU's platform device
> + * otherwise it is VPU's platform device with incremented reference count

I picked this patch but rewrote with what felt like better and dense.

- * Return: Return NULL if it is failed.
- * otherwise it is VPU's platform device
+ * Return: a reference to the VPU's platform device, or NULL on failure.

hope its ok with you,

cheers,
Nicolas

>   **/
>  struct platform_device *vpu_get_plat_device(struct platform_device *pdev);
>  
Re: [PATCH v2 2/2] media: mediatek: amend vpu_get_plat_device() documentation
Posted by Johan Hovold 1 week ago
On Tue, Dec 09, 2025 at 03:43:30PM -0500, Nicolas Dufresne wrote:
> Hi,
> 
> Le mardi 28 octobre 2025 à 11:04 +0100, Johan Hovold a écrit :
> > Add a comment to the vpu_get_plat_device() documentation to make it
> > clear that the VPU platform device is returned with an incremented
> > reference count (which needs to be dropped after use).
> > 
> > Signed-off-by: Johan Hovold <johan@kernel.org>
> > ---
> >  drivers/media/platform/mediatek/vpu/mtk_vpu.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/media/platform/mediatek/vpu/mtk_vpu.h b/drivers/media/platform/mediatek/vpu/mtk_vpu.h
> > index da05f3e74081..5808311d2b15 100644
> > --- a/drivers/media/platform/mediatek/vpu/mtk_vpu.h
> > +++ b/drivers/media/platform/mediatek/vpu/mtk_vpu.h
> > @@ -120,7 +120,7 @@ int vpu_ipi_send(struct platform_device *pdev,
> >   *		device for using VPU API.
> >   *
> >   * Return: Return NULL if it is failed.
> > - * otherwise it is VPU's platform device
> > + * otherwise it is VPU's platform device with incremented reference count
> 
> I picked this patch but rewrote with what felt like better and dense.
> 
> - * Return: Return NULL if it is failed.
> - * otherwise it is VPU's platform device
> + * Return: a reference to the VPU's platform device, or NULL on failure.
> 
> hope its ok with you,

Sure, my only concern is that just saying "reference" is too subtle,
that's why I explicitly mentioned the refcount.

Btw, why is patch 2/2 marked obsolete? That leak is still there both on
probe errors (which I saw someone else posted a fix for) and on driver
unbind.

Johan
Re: [PATCH v2 2/2] media: mediatek: amend vpu_get_plat_device() documentation
Posted by Nicolas Dufresne 6 days, 20 hours ago
Le mercredi 10 décembre 2025 à 12:21 +0900, Johan Hovold a écrit :
> On Tue, Dec 09, 2025 at 03:43:30PM -0500, Nicolas Dufresne wrote:
> > Hi,
> > 
> > Le mardi 28 octobre 2025 à 11:04 +0100, Johan Hovold a écrit :
> > > Add a comment to the vpu_get_plat_device() documentation to make it
> > > clear that the VPU platform device is returned with an incremented
> > > reference count (which needs to be dropped after use).
> > > 
> > > Signed-off-by: Johan Hovold <johan@kernel.org>
> > > ---
> > >  drivers/media/platform/mediatek/vpu/mtk_vpu.h | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/media/platform/mediatek/vpu/mtk_vpu.h
> > > b/drivers/media/platform/mediatek/vpu/mtk_vpu.h
> > > index da05f3e74081..5808311d2b15 100644
> > > --- a/drivers/media/platform/mediatek/vpu/mtk_vpu.h
> > > +++ b/drivers/media/platform/mediatek/vpu/mtk_vpu.h
> > > @@ -120,7 +120,7 @@ int vpu_ipi_send(struct platform_device *pdev,
> > >   *		device for using VPU API.
> > >   *
> > >   * Return: Return NULL if it is failed.
> > > - * otherwise it is VPU's platform device
> > > + * otherwise it is VPU's platform device with incremented reference count
> > 
> > I picked this patch but rewrote with what felt like better and dense.
> > 
> > - * Return: Return NULL if it is failed.
> > - * otherwise it is VPU's platform device
> > + * Return: a reference to the VPU's platform device, or NULL on failure.
> > 
> > hope its ok with you,
> 
> Sure, my only concern is that just saying "reference" is too subtle,
> that's why I explicitly mentioned the refcount.

For me everyone should read "a reference" as a kref based reference counted
structure.  A quick grep across out documentation, this is the vast majority of
the wording. Though, I spent limited time looking.

> 
> Btw, why is patch 2/2 marked obsolete? That leak is still there both on
> probe errors (which I saw someone else posted a fix for) and on driver
> unbind.

I had two patches fixing the same thing, it just happen that I ended up picking
the other one first, and liked you documentation fix, except it was replicating
obvious weird english such as "if it is failed", and documenting the error
before the expected outcome (opposite of my preference, not sure there is any
rules or guidelines).

https://lore.kernel.org/all/20251008090156.14224-1-haoxiang_li2024@163.com/

cheers,
Nicolas

> 
> Johan
Re: [PATCH v2 2/2] media: mediatek: amend vpu_get_plat_device() documentation
Posted by Johan Hovold 5 days, 10 hours ago
On Wed, Dec 10, 2025 at 10:58:41AM -0500, Nicolas Dufresne wrote:
> Le mercredi 10 décembre 2025 à 12:21 +0900, Johan Hovold a écrit :
> > On Tue, Dec 09, 2025 at 03:43:30PM -0500, Nicolas Dufresne wrote:
> > > Le mardi 28 octobre 2025 à 11:04 +0100, Johan Hovold a écrit :

> > > > Add a comment to the vpu_get_plat_device() documentation to make it
> > > > clear that the VPU platform device is returned with an incremented
> > > > reference count (which needs to be dropped after use).

> > > > @@ -120,7 +120,7 @@ int vpu_ipi_send(struct platform_device *pdev,
> > > >   *		device for using VPU API.
> > > >   *
> > > >   * Return: Return NULL if it is failed.
> > > > - * otherwise it is VPU's platform device
> > > > + * otherwise it is VPU's platform device with incremented reference count
> > > 
> > > I picked this patch but rewrote with what felt like better and dense.
> > > 
> > > - * Return: Return NULL if it is failed.
> > > - * otherwise it is VPU's platform device
> > > + * Return: a reference to the VPU's platform device, or NULL on failure.
> > > 
> > > hope its ok with you,
> > 
> > Sure, my only concern is that just saying "reference" is too subtle,
> > that's why I explicitly mentioned the refcount.
> 
> For me everyone should read "a reference" as a kref based reference counted
> structure.  A quick grep across out documentation, this is the vast majority of
> the wording. Though, I spent limited time looking.

It should be sufficient but given how many people miss this it may still
be worth being more explicit.

> > Btw, why is patch 2/2 marked obsolete? That leak is still there both on
> > probe errors (which I saw someone else posted a fix for) and on driver
> > unbind.
> 
> I had two patches fixing the same thing, it just happen that I ended up picking
> the other one first

Ah, I had missed that and the fix is not in linux-next yet either so
without a reply it wasn't obvious.

> and liked you documentation fix, except it was replicating
> obvious weird english such as "if it is failed", and documenting the error
> before the expected outcome (opposite of my preference, not sure there is any
> rules or guidelines).

Yeah, I considered rewriting the whole comment too but given that the
surrounding comments used similar language I just amended what was
there.

But I'm totally fine with the update you did.

> https://lore.kernel.org/all/20251008090156.14224-1-haoxiang_li2024@163.com/

Johan