On Wed, Mar 27, 2019 at 1:13 AM Himadri Pandya <himadri18.07@gmail.com>
wrote:

>
>
> On Mon, Mar 25, 2019 at 7:17 AM Laurent Pinchart <
> laurent.pinchart@ideasonboard.com> wrote:
>
>> 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
>>
>
I have been suggested to modify the macro to_iss_bufer(). But I do not
understand how should it be changed. Can anybody please explain it? I need
to understand this in order to send the next version of the patch.

Thank you.

- Himadri