* [PATCH] staging: media: omap4iss: Use list_entry instead of container_of @ 2019-03-24 16:54 Himadri Pandya 2019-03-24 17:18 ` [Outreachy kernel] " Julia Lawall 0 siblings, 1 reply; 9+ messages in thread From: Himadri Pandya @ 2019-03-24 16:54 UTC (permalink / raw) To: laurent.pinchart, mchehab, gregkh; +Cc: outreachy-kernel, Himadri Pandya 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) 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) struct iss_video_fh { struct v4l2_fh vfh; -- 2.17.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [Outreachy kernel] [PATCH] staging: media: omap4iss: Use list_entry instead of container_of 2019-03-24 16:54 [PATCH] staging: media: omap4iss: Use list_entry instead of container_of Himadri Pandya @ 2019-03-24 17:18 ` Julia Lawall 2019-03-24 17:49 ` Himadri Pandya 0 siblings, 1 reply; 9+ messages in thread From: Julia Lawall @ 2019-03-24 17:18 UTC (permalink / raw) To: Himadri Pandya; +Cc: laurent.pinchart, mchehab, gregkh, outreachy-kernel 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. > > 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. julia > struct iss_video_fh { > struct v4l2_fh vfh; > -- > 2.17.1 > > -- > You received this message because you are subscribed to the Google Groups "outreachy-kernel" group. > To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com. > To post to this group, send email to outreachy-kernel@googlegroups.com. > To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20190324165433.95833-1-himadri18.07%40gmail.com. > For more options, visit https://groups.google.com/d/optout. > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Outreachy kernel] [PATCH] staging: media: omap4iss: Use list_entry instead of container_of 2019-03-24 17:18 ` [Outreachy kernel] " Julia Lawall @ 2019-03-24 17:49 ` Himadri Pandya 2019-03-24 17:57 ` Julia Lawall 2019-03-25 1:46 ` Laurent Pinchart 0 siblings, 2 replies; 9+ messages in thread From: Himadri Pandya @ 2019-03-24 17:49 UTC (permalink / raw) To: Julia Lawall; +Cc: laurent.pinchart, mchehab, Greg KH, outreachy-kernel [-- Attachment #1: Type: text/plain, Size: 2853 bytes --] 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? > > > > > 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. - Himadri > > julia > > > struct iss_video_fh { > > struct v4l2_fh vfh; > > -- > > 2.17.1 > > > > -- > > You received this message because you are subscribed to the Google > Groups "outreachy-kernel" group. > > To unsubscribe from this group and stop receiving emails from it, send > an email to outreachy-kernel+unsubscribe@googlegroups.com. > > To post to this group, send email to outreachy-kernel@googlegroups.com. > > To view this discussion on the web visit > https://groups.google.com/d/msgid/outreachy-kernel/20190324165433.95833-1-himadri18.07%40gmail.com > . > > For more options, visit https://groups.google.com/d/optout. > > > > -- > You received this message because you are subscribed to the Google Groups > "outreachy-kernel" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to outreachy-kernel+unsubscribe@googlegroups.com. > To post to this group, send email to outreachy-kernel@googlegroups.com. > To view this discussion on the web visit > https://groups.google.com/d/msgid/outreachy-kernel/alpine.DEB.2.21.1903241812520.2526%40hadrien > . > For more options, visit https://groups.google.com/d/optout. > [-- Attachment #2: Type: text/html, Size: 4697 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Outreachy kernel] [PATCH] staging: media: omap4iss: Use list_entry instead of container_of 2019-03-24 17:49 ` Himadri Pandya @ 2019-03-24 17:57 ` Julia Lawall 2019-03-25 1:46 ` Laurent Pinchart 1 sibling, 0 replies; 9+ messages in thread From: Julia Lawall @ 2019-03-24 17:57 UTC (permalink / raw) To: Himadri Pandya; +Cc: laurent.pinchart, mchehab, Greg KH, outreachy-kernel [-- Attachment #1: Type: text/plain, Size: 4160 bytes --] On Sun, 24 Mar 2019, 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? One of the macros is used and one is not. You can send a patch to drop the one that is not used. The other one is used only once. Still it looks more readable than a call to container_of. julia > > > > > > 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. > > - Himadri > > > julia > > > struct iss_video_fh { > > struct v4l2_fh vfh; > > -- > > 2.17.1 > > > > -- > > You received this message because you are subscribed to the > Google Groups "outreachy-kernel" group. > > To unsubscribe from this group and stop receiving emails from > it, send an email to > outreachy-kernel+unsubscribe@googlegroups.com. > > To post to this group, send email to > outreachy-kernel@googlegroups.com. > > To view this discussion on the web visithttps://groups.google.com/d/msgid/outreachy-kernel/20190324165433.95833-1-h > imadri18.07%40gmail.com. > > For more options, visit https://groups.google.com/d/optout. > > > > -- > You received this message because you are subscribed to the > Google Groups "outreachy-kernel" group. > To unsubscribe from this group and stop receiving emails from > it, send an email to > outreachy-kernel+unsubscribe@googlegroups.com. > To post to this group, send email to > outreachy-kernel@googlegroups.com. > To view this discussion on the web visithttps://groups.google.com/d/msgid/outreachy-kernel/alpine.DEB.2.21.19032418 > 12520.2526%40hadrien. > For more options, visit https://groups.google.com/d/optout. > > -- > You received this message because you are subscribed to the Google Groups > "outreachy-kernel" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to outreachy-kernel+unsubscribe@googlegroups.com. > To post to this group, send email to outreachy-kernel@googlegroups.com. > To view this discussion on the web visithttps://groups.google.com/d/msgid/outreachy-kernel/CAPnhUpYSAaY0J6dRsQf2hMv > MfUhvaSXqu%2BVgt3aQ7g_mjGX8Hw%40mail.gmail.com. > For more options, visit https://groups.google.com/d/optout. > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Outreachy kernel] [PATCH] staging: media: omap4iss: Use list_entry instead of container_of 2019-03-24 17:49 ` Himadri Pandya 2019-03-24 17:57 ` Julia Lawall @ 2019-03-25 1:46 ` Laurent Pinchart 2019-03-26 19:43 ` Himadri Pandya 1 sibling, 1 reply; 9+ messages in thread From: Laurent Pinchart @ 2019-03-25 1:46 UTC (permalink / raw) To: Himadri Pandya; +Cc: Julia Lawall, mchehab, Greg KH, outreachy-kernel 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. > >> 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Outreachy kernel] [PATCH] staging: media: omap4iss: Use list_entry instead of container_of 2019-03-25 1:46 ` Laurent Pinchart @ 2019-03-26 19:43 ` Himadri Pandya 2019-03-30 15:40 ` Himadri Pandya 2019-03-31 10:08 ` Laurent Pinchart 0 siblings, 2 replies; 9+ messages in thread From: Himadri Pandya @ 2019-03-26 19:43 UTC (permalink / raw) To: Laurent Pinchart; +Cc: Julia Lawall, mchehab, Greg KH, outreachy-kernel [-- Attachment #1: Type: text/plain, Size: 3553 bytes --] 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 > [-- Attachment #2: Type: text/html, Size: 6395 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Outreachy kernel] [PATCH] staging: media: omap4iss: Use list_entry instead of container_of 2019-03-26 19:43 ` Himadri Pandya @ 2019-03-30 15:40 ` Himadri Pandya 2019-03-31 10:08 ` Laurent Pinchart 1 sibling, 0 replies; 9+ messages in thread From: Himadri Pandya @ 2019-03-30 15:40 UTC (permalink / raw) To: Laurent Pinchart; +Cc: Julia Lawall, mchehab, Greg KH, outreachy-kernel [-- Attachment #1: Type: text/plain, Size: 4028 bytes --] 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 [-- Attachment #2: Type: text/html, Size: 6983 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Outreachy kernel] [PATCH] staging: media: omap4iss: Use list_entry instead of container_of 2019-03-26 19:43 ` Himadri Pandya 2019-03-30 15:40 ` Himadri Pandya @ 2019-03-31 10:08 ` Laurent Pinchart 2019-04-01 18:54 ` Himadri Pandya 1 sibling, 1 reply; 9+ messages in thread From: Laurent Pinchart @ 2019-03-31 10:08 UTC (permalink / raw) To: Himadri Pandya; +Cc: Julia Lawall, mchehab, Greg KH, outreachy-kernel Hello Himadri, On Wed, Mar 27, 2019 at 01:13:27AM +0530, Himadri Pandya wrote: > On Mon, Mar 25, 2019 at 7:17 AM Laurent Pinchart wrote: > > On Sun, Mar 23, 2019 at 11:19:54PM +0530, Himadri Pandya wrote: > >> On Sun, Mar 24, 2019 at 10:48 PM Julia Lawall 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) to_iss_buffer() casts a vb2_v4l2_buffer pointer to a iss_buffer pointer. > 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); > ... > } These three functions need to cast a vb2_buffer pointer to a iss_buffer pointer. They do so by first casting to vb2_v4l2_buffer, and then to iss_buffer with a direct use of container_of. It would be best to combine the two casts in a single to_iss_buffer macro and use it through the code. > 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? > > >>>> 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Outreachy kernel] [PATCH] staging: media: omap4iss: Use list_entry instead of container_of 2019-03-31 10:08 ` Laurent Pinchart @ 2019-04-01 18:54 ` Himadri Pandya 0 siblings, 0 replies; 9+ messages in thread From: Himadri Pandya @ 2019-04-01 18:54 UTC (permalink / raw) To: Laurent Pinchart; +Cc: Julia Lawall, mchehab, Greg KH, outreachy-kernel [-- Attachment #1: Type: text/plain, Size: 4294 bytes --] On Sun, Mar 31, 2019 at 3:38 PM Laurent Pinchart < laurent.pinchart@ideasonboard.com> wrote: > Hello Himadri, > > On Wed, Mar 27, 2019 at 01:13:27AM +0530, Himadri Pandya wrote: > > On Mon, Mar 25, 2019 at 7:17 AM Laurent Pinchart wrote: > > > On Sun, Mar 23, 2019 at 11:19:54PM +0530, Himadri Pandya wrote: > > >> On Sun, Mar 24, 2019 at 10:48 PM Julia Lawall 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) > > to_iss_buffer() casts a vb2_v4l2_buffer pointer to a iss_buffer pointer. > > > 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); > > ... > > } > > These three functions need to cast a vb2_buffer pointer to a iss_buffer > pointer. They do so by first casting to vb2_v4l2_buffer, and then to > iss_buffer with a direct use of container_of. It would be best to > combine the two casts in a single to_iss_buffer macro and use it through > the code. > Got it. Thank you. - Himadri > > 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? > > > > >>>> 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 > [-- Attachment #2: Type: text/html, Size: 5774 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-04-01 18:54 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-03-24 16:54 [PATCH] staging: media: omap4iss: Use list_entry instead of container_of Himadri Pandya 2019-03-24 17:18 ` [Outreachy kernel] " Julia Lawall 2019-03-24 17:49 ` Himadri Pandya 2019-03-24 17:57 ` Julia Lawall 2019-03-25 1:46 ` Laurent Pinchart 2019-03-26 19:43 ` Himadri Pandya 2019-03-30 15:40 ` Himadri Pandya 2019-03-31 10:08 ` Laurent Pinchart 2019-04-01 18:54 ` Himadri Pandya
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.