Hello Himadri,
On Sun, Mar 24, 2019 at 11:19:54PM +0530, Himadri Pandya wrote:
> On Sun, Mar 24, 2019 at 10:48 PM Julia Lawall <julia.lawall@lip6.fr> wrote:
> > On Sun, 24 Mar 2019, Himadri Pandya wrote:
> >> Use list_entry instead of container_of for lists iss_buffer and
> >> iss_video.
> >>
> >> Signed-off-by: Himadri Pandya <himadri18.07@gmail.com>
> >> ---
> >> drivers/staging/media/omap4iss/iss_video.h | 4 ++--
> >> 1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/staging/media/omap4iss/iss_video.h b/drivers/staging/media/omap4iss/iss_video.h
> >> index f22489edb562..e8d27b7ac329 100644
> >> --- a/drivers/staging/media/omap4iss/iss_video.h
> >> +++ b/drivers/staging/media/omap4iss/iss_video.h
> >> @@ -118,7 +118,7 @@ struct iss_buffer {
> >> dma_addr_t iss_addr;
> >> };
> >>
> >> -#define to_iss_buffer(buf) container_of(buf, struct iss_buffer, vb)
> >> +#define to_iss_buffer(buf) list_entry(buf, struct iss_buffer, vb)
> >
> > Actually, this macro is no longer used. Perhaps someone else converted
> > the uses to use the proper list operations and forgot to drop the macro
> > definition.
>
> Then should I send a patch to drop these macros?
It would be better to modify the to_iss_buffer macro to convert from a
vb2_buffer to a iss_buffer, and use it where the driver currently uses
container_of directly.
I am sorry but I am not sure if I understand what you are suggesting correctly.
to_iss_buffer is defined as:
#define to_iss_buffer(buf) container_of(buf, struct iss_buffer, vb)
And it is not used. Instead, container_of is used directly
static void iss_video_buf_cleanup(struct vb2_buffer *vb)
{
struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
struct iss_buffer *buffer = container_of(vbuf, struct iss_buffer, vb);
...
}
static int iss_video_buf_prepare(struct vb2_buffer *vb)
{
struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
struct iss_video_fh *vfh = vb2_get_drv_priv(vb->vb2_queue);
struct iss_buffer *buffer = container_of(vbuf, struct iss_buffer, vb);
...
}
static void iss_video_buf_queue(struct vb2_buffer *vb)
{
struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
struct iss_video_fh *vfh = vb2_get_drv_priv(vb->vb2_queue);
struct iss_video *video = vfh->video;
struct iss_buffer *buffer = container_of(vbuf, struct iss_buffer, vb);
...
}
If I understand it correctly, these usages are converting vb2_buffer to iss_buffer. Which could be done by using the to_iss_buffer(vbuf) as well. Why is this macro no longer used? I also don't understand what modification you are suggesting. Can you please elaborate?
Thank you.
- Himadri
> >> enum iss_video_dmaqueue_flags {
> >> /* Set if DMA queue becomes empty when ISS_PIPELINE_STREAM_CONTINUOUS */
> >> @@ -170,7 +170,7 @@ struct iss_video {
> >> const struct iss_video_operations *ops;
> >> };
> >>
> >> -#define to_iss_video(vdev) container_of(vdev, struct iss_video, video)
> >> +#define to_iss_video(vdev) list_entry(vdev, struct iss_video, video)
> >
> > This doesn't involve a list. Nor does the previous one. To use
> > list_entry, the third argument should be the name of a field that has type
> > list_head.
>
> Understood. Thank you.
>
> >> struct iss_video_fh {
> >> struct v4l2_fh vfh;
--
Regards,
Laurent Pinchart