[PATCH v2] media: atomisp: pci: sh_css: Inline single invocation of macro STATS_ENABLED()

Brent Pappas posted 1 patch 2 years, 7 months ago
There is a newer version of this series
drivers/staging/media/atomisp/pci/sh_css.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
[PATCH v2] media: atomisp: pci: sh_css: Inline single invocation of macro STATS_ENABLED()
Posted by Brent Pappas 2 years, 7 months ago
Inline the single invocation of the macro STATS_ENABLED().
The macro abstraction is not necessary because the logic behind it is only
used once.

Signed-off-by: Brent Pappas <bpappas@pappasbrent.com>
---
 drivers/staging/media/atomisp/pci/sh_css.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/sh_css.c b/drivers/staging/media/atomisp/pci/sh_css.c
index 726cb7aa4ecd..93789500416f 100644
--- a/drivers/staging/media/atomisp/pci/sh_css.c
+++ b/drivers/staging/media/atomisp/pci/sh_css.c
@@ -97,9 +97,6 @@
  */
 #define JPEG_BYTES (16 * 1024 * 1024)
 
-#define STATS_ENABLED(stage) (stage && stage->binary && stage->binary->info && \
-	(stage->binary->info->sp.enable.s3a || stage->binary->info->sp.enable.dis))
-
 struct sh_css my_css;
 
 int  __printf(1, 0) (*sh_css_printf)(const char *fmt, va_list args) = NULL;
@@ -3743,7 +3740,9 @@ ia_css_pipe_enqueue_buffer(struct ia_css_pipe *pipe,
 			 * The SP will read the params after it got
 			 * empty 3a and dis
 			 */
-			if (STATS_ENABLED(stage)) {
+			if (stage && stage->binary && stage->binary->info &&
+			    (stage->binary->info->sp.enable.s3a ||
+			     stage->binary->info->sp.enable.dis)) {
 				/* there is a stage that needs it */
 				return_err = ia_css_bufq_enqueue_buffer(thread_id,
 									queue_id,
-- 
2.34.1
Re: [PATCH v2] media: atomisp: pci: sh_css: Inline single invocation of macro STATS_ENABLED()
Posted by Dan Carpenter 2 years, 7 months ago
On Thu, Jan 19, 2023 at 09:42:03AM -0500, Brent Pappas wrote:
> Inline the single invocation of the macro STATS_ENABLED().
> The macro abstraction is not necessary because the logic behind it is only
> used once.
> 
> Signed-off-by: Brent Pappas <bpappas@pappasbrent.com>
> ---

Needs a V2: note which says what changed.

>  drivers/staging/media/atomisp/pci/sh_css.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/media/atomisp/pci/sh_css.c b/drivers/staging/media/atomisp/pci/sh_css.c
> index 726cb7aa4ecd..93789500416f 100644
> --- a/drivers/staging/media/atomisp/pci/sh_css.c
> +++ b/drivers/staging/media/atomisp/pci/sh_css.c
> @@ -97,9 +97,6 @@
>   */
>  #define JPEG_BYTES (16 * 1024 * 1024)
>  
> -#define STATS_ENABLED(stage) (stage && stage->binary && stage->binary->info && \
> -	(stage->binary->info->sp.enable.s3a || stage->binary->info->sp.enable.dis))
> -
>  struct sh_css my_css;
>  
>  int  __printf(1, 0) (*sh_css_printf)(const char *fmt, va_list args) = NULL;
> @@ -3743,7 +3740,9 @@ ia_css_pipe_enqueue_buffer(struct ia_css_pipe *pipe,
>  			 * The SP will read the params after it got
>  			 * empty 3a and dis
>  			 */
> -			if (STATS_ENABLED(stage)) {
> +			if (stage && stage->binary && stage->binary->info &&
                            ^^^^^^^^
We know that "stage" must be non-NULL.  You should wait over night
before resending v2 patches so that we think "This guy spend overnight
running tests.  He must be very serious."  There is no rush.

regards,
dan carpenter
[PATCH v3] media: atomisp: pci: sh_css: Inline single invocation of macro STATS_ENABLED()
Posted by Brent Pappas 2 years, 7 months ago
Inline the single invocation of the macro STATS_ENABLED().
The macro abstraction is not necessary because the logic behind it is only
used once.

Signed-off-by: Brent Pappas <bpappas@pappasbrent.com>
---
Changelog:
V1 -> V2: Inline macro instead of replacing it with a function.

V2 -> V3: Remove unnecessary check that stage is non-null.

 drivers/staging/media/atomisp/pci/sh_css.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/sh_css.c b/drivers/staging/media/atomisp/pci/sh_css.c
index 726cb7aa4ecd..93789500416f 100644
--- a/drivers/staging/media/atomisp/pci/sh_css.c
+++ b/drivers/staging/media/atomisp/pci/sh_css.c
@@ -97,9 +97,6 @@
  */
 #define JPEG_BYTES (16 * 1024 * 1024)
 
-#define STATS_ENABLED(stage) (stage && stage->binary && stage->binary->info && \
-	(stage->binary->info->sp.enable.s3a || stage->binary->info->sp.enable.dis))
-
 struct sh_css my_css;
 
 int  __printf(1, 0) (*sh_css_printf)(const char *fmt, va_list args) = NULL;
@@ -3743,7 +3740,9 @@ ia_css_pipe_enqueue_buffer(struct ia_css_pipe *pipe,
 			 * The SP will read the params after it got
 			 * empty 3a and dis
 			 */
-			if (STATS_ENABLED(stage)) {
+			if (stage->binary && stage->binary->info &&
+			    (stage->binary->info->sp.enable.s3a ||
+			     stage->binary->info->sp.enable.dis)) {
 				/* there is a stage that needs it */
 				return_err = ia_css_bufq_enqueue_buffer(thread_id,
 									queue_id,
-- 
2.34.1
Re: [PATCH v3] media: atomisp: pci: sh_css: Inline single invocation of macro STATS_ENABLED()
Posted by Hans de Goede 2 years, 7 months ago
Hi,

On 1/20/23 18:14, Brent Pappas wrote:
> Inline the single invocation of the macro STATS_ENABLED().
> The macro abstraction is not necessary because the logic behind it is only
> used once.
> 
> Signed-off-by: Brent Pappas <bpappas@pappasbrent.com>


Thank you.

I have added this to my personal git tree now and I will include
this in the atomisp driver pull-req which I will send to the
media-subsystem maintainer in a couple of weeks.

Regards,

Hans



> ---
> Changelog:
> V1 -> V2: Inline macro instead of replacing it with a function.
> 
> V2 -> V3: Remove unnecessary check that stage is non-null.
> 
>  drivers/staging/media/atomisp/pci/sh_css.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/media/atomisp/pci/sh_css.c b/drivers/staging/media/atomisp/pci/sh_css.c
> index 726cb7aa4ecd..93789500416f 100644
> --- a/drivers/staging/media/atomisp/pci/sh_css.c
> +++ b/drivers/staging/media/atomisp/pci/sh_css.c
> @@ -97,9 +97,6 @@
>   */
>  #define JPEG_BYTES (16 * 1024 * 1024)
>  
> -#define STATS_ENABLED(stage) (stage && stage->binary && stage->binary->info && \
> -	(stage->binary->info->sp.enable.s3a || stage->binary->info->sp.enable.dis))
> -
>  struct sh_css my_css;
>  
>  int  __printf(1, 0) (*sh_css_printf)(const char *fmt, va_list args) = NULL;
> @@ -3743,7 +3740,9 @@ ia_css_pipe_enqueue_buffer(struct ia_css_pipe *pipe,
>  			 * The SP will read the params after it got
>  			 * empty 3a and dis
>  			 */
> -			if (STATS_ENABLED(stage)) {
> +			if (stage->binary && stage->binary->info &&
> +			    (stage->binary->info->sp.enable.s3a ||
> +			     stage->binary->info->sp.enable.dis)) {
>  				/* there is a stage that needs it */
>  				return_err = ia_css_bufq_enqueue_buffer(thread_id,
>  									queue_id,
Re: [PATCH v3] media: atomisp: pci: sh_css: Inline single invocation of macro STATS_ENABLED()
Posted by Dan Carpenter 2 years, 7 months ago
On Fri, Jan 20, 2023 at 12:14:08PM -0500, Brent Pappas wrote:
> Inline the single invocation of the macro STATS_ENABLED().
> The macro abstraction is not necessary because the logic behind it is only
> used once.
> 
> Signed-off-by: Brent Pappas <bpappas@pappasbrent.com>
> ---
> Changelog:
> V1 -> V2: Inline macro instead of replacing it with a function.
> 
> V2 -> V3: Remove unnecessary check that stage is non-null.
> 

Thanks!

Reviewed-by: Dan Carpenter <error27@gmail.com>

regards,
dan carpenter
Re: [PATCH v3] media: atomisp: pci: sh_css: Inline single invocation of macro STATS_ENABLED()
Posted by Andy Shevchenko 2 years, 7 months ago
On Fri, Jan 20, 2023 at 7:14 PM Brent Pappas <bpappas@pappasbrent.com> wrote:
>
> Inline the single invocation of the macro STATS_ENABLED().
> The macro abstraction is not necessary because the logic behind it is only
> used once.

LGTM,
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Signed-off-by: Brent Pappas <bpappas@pappasbrent.com>
> ---
> Changelog:
> V1 -> V2: Inline macro instead of replacing it with a function.
>
> V2 -> V3: Remove unnecessary check that stage is non-null.
>
>  drivers/staging/media/atomisp/pci/sh_css.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/media/atomisp/pci/sh_css.c b/drivers/staging/media/atomisp/pci/sh_css.c
> index 726cb7aa4ecd..93789500416f 100644
> --- a/drivers/staging/media/atomisp/pci/sh_css.c
> +++ b/drivers/staging/media/atomisp/pci/sh_css.c
> @@ -97,9 +97,6 @@
>   */
>  #define JPEG_BYTES (16 * 1024 * 1024)
>
> -#define STATS_ENABLED(stage) (stage && stage->binary && stage->binary->info && \
> -       (stage->binary->info->sp.enable.s3a || stage->binary->info->sp.enable.dis))
> -
>  struct sh_css my_css;
>
>  int  __printf(1, 0) (*sh_css_printf)(const char *fmt, va_list args) = NULL;
> @@ -3743,7 +3740,9 @@ ia_css_pipe_enqueue_buffer(struct ia_css_pipe *pipe,
>                          * The SP will read the params after it got
>                          * empty 3a and dis
>                          */
> -                       if (STATS_ENABLED(stage)) {
> +                       if (stage->binary && stage->binary->info &&
> +                           (stage->binary->info->sp.enable.s3a ||
> +                            stage->binary->info->sp.enable.dis)) {
>                                 /* there is a stage that needs it */
>                                 return_err = ia_css_bufq_enqueue_buffer(thread_id,
>                                                                         queue_id,
> --
> 2.34.1
>


-- 
With Best Regards,
Andy Shevchenko