| Message ID | 20230425074841.29063-3-hpa@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | staging: media: atomisp: Remove #ifdef 2401 | expand |
Hi Kate, On 4/25/23 09:48, Kate Hsuan wrote: > The actions of ISP2401 and 2400 are determined at the runtime. > > Signed-off-by: Kate Hsuan <hpa@redhat.com> > --- > .../media/atomisp/pci/runtime/frame/src/frame.c | 15 +++++++-------- > 1 file changed, 7 insertions(+), 8 deletions(-) > > diff --git a/drivers/staging/media/atomisp/pci/runtime/frame/src/frame.c b/drivers/staging/media/atomisp/pci/runtime/frame/src/frame.c > index 83bb42e05421..425e75f7dda7 100644 > --- a/drivers/staging/media/atomisp/pci/runtime/frame/src/frame.c > +++ b/drivers/staging/media/atomisp/pci/runtime/frame/src/frame.c > @@ -601,9 +601,9 @@ static void frame_init_qplane6_planes(struct ia_css_frame *frame) > > static int frame_allocate_buffer_data(struct ia_css_frame *frame) > { > -#ifdef ISP2401 > - IA_CSS_ENTER_LEAVE_PRIVATE("frame->data_bytes=%d\n", frame->data_bytes); > -#endif > + if (IS_ISP2401) > + IA_CSS_ENTER_LEAVE_PRIVATE("frame->data_bytes=%d\n", frame->data_bytes); > + > frame->data = hmm_alloc(frame->data_bytes); > if (frame->data == mmgr_NULL) > return -ENOMEM; This is just a debug log message, IMHO it is best to just completely remove the message for both ISP models. > @@ -635,11 +635,10 @@ static int frame_allocate_with_data(struct ia_css_frame **frame, > > if (err) { > kvfree(me); > -#ifndef ISP2401 > - return err; > -#else > - me = NULL; > -#endif > + if (IS_ISP2401) > + me = NULL; > + else > + return err; > } > > *frame = me; This one is also weird. I have checked the only caller of this and it does not matter what *frame is set to since as long as this returns non 0 the *frame is ignored and the functions always returns err (just outside of the context of the patch). So this can be simplified to just: if (err) { kvfree(me); me = NULL; } And then fall through to the original code of: *frame = me; return err; } The me = NULL is not strictly necessary but setting *frame = NULL on errors is a bit cleaner and may help catch future bugs. Regards, Hans
diff --git a/drivers/staging/media/atomisp/pci/runtime/frame/src/frame.c b/drivers/staging/media/atomisp/pci/runtime/frame/src/frame.c index 83bb42e05421..425e75f7dda7 100644 --- a/drivers/staging/media/atomisp/pci/runtime/frame/src/frame.c +++ b/drivers/staging/media/atomisp/pci/runtime/frame/src/frame.c @@ -601,9 +601,9 @@ static void frame_init_qplane6_planes(struct ia_css_frame *frame) static int frame_allocate_buffer_data(struct ia_css_frame *frame) { -#ifdef ISP2401 - IA_CSS_ENTER_LEAVE_PRIVATE("frame->data_bytes=%d\n", frame->data_bytes); -#endif + if (IS_ISP2401) + IA_CSS_ENTER_LEAVE_PRIVATE("frame->data_bytes=%d\n", frame->data_bytes); + frame->data = hmm_alloc(frame->data_bytes); if (frame->data == mmgr_NULL) return -ENOMEM; @@ -635,11 +635,10 @@ static int frame_allocate_with_data(struct ia_css_frame **frame, if (err) { kvfree(me); -#ifndef ISP2401 - return err; -#else - me = NULL; -#endif + if (IS_ISP2401) + me = NULL; + else + return err; } *frame = me;
The actions of ISP2401 and 2400 are determined at the runtime. Signed-off-by: Kate Hsuan <hpa@redhat.com> --- .../media/atomisp/pci/runtime/frame/src/frame.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-)