linux-staging.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
From: Hans de Goede <hdegoede@redhat.com>
To: Kate Hsuan <hpa@redhat.com>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Sakari Ailus <sakari.ailus@linux.intel.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-media@vger.kernel.org, linux-staging@lists.linux.dev
Subject: Re: [PATCH 3/3] media: atomisp: ia_css_debug: remove unused codes
Date: Tue, 6 Jun 2023 12:50:24 +0200	[thread overview]
Message-ID: <5e4e49d8-1e0d-709d-0a4a-576fa1212f01@redhat.com> (raw)
In-Reply-To: <20230605102903.924283-4-hpa@redhat.com>

Hi Kate,

On 6/5/23 12:29, Kate Hsuan wrote:
> This portion of the codes was not referenced so it can be completely
> removed.
> 
> Signed-off-by: Kate Hsuan <hpa@redhat.com>

So this patch fixes compile errors introduced by the first patch
and MAX_THREAD_NUM is still used after this by code in
"#ifdef HAS_WATCHDOG_SP_THREAD_DEBUG" blocks.

Note HAS_WATCHDOG_SP_THREAD_DEBUG is never set.

So IMHO the right thing to do here is:

1. Introduce a new patch which should be the first patch
   of the set which removes all code blocks protected by
   "#ifdef HAS_WATCHDOG_SP_THREAD_DEBUG" as well as
   the "uncomment this here" HAS_WATCHDOG_SP_THREAD_DEBUG bits
   in drivers/staging/media/atomisp/pci/hive_isp_css_common/debug_global.h

2.Make this patch the second (first patch after the
  HAS_WATCHDOG_SP_THREAD_DEBUG removal) patch of the series.

  And instead of renaming MAX_THREAD_NUM to
  MAX_THREAD_NUM_2400 / MAX_THREAD_NUM_2401 just entirely drop it
  and also drop the following now no longer used defines:
  SH_CSS_SP_INTERNAL_METADATA_THREAD
  SH_CSS_SP_INTERNAL_SERVICE_THREAD
  SH_CSS_MAX_SP_INTERNAL_THREADS

Regards,

Hans


SH_CSS_SP_INTERNAL_METADATA_THREAD and
  
the entire now no longer 

> ---
>  .../runtime/debug/interface/ia_css_debug.h    |  6 --
>  .../pci/runtime/debug/src/ia_css_debug.c      | 76 +------------------
>  2 files changed, 2 insertions(+), 80 deletions(-)
> 
> diff --git a/drivers/staging/media/atomisp/pci/runtime/debug/interface/ia_css_debug.h b/drivers/staging/media/atomisp/pci/runtime/debug/interface/ia_css_debug.h
> index fff89e9b4b01..3a3d72c6eaaa 100644
> --- a/drivers/staging/media/atomisp/pci/runtime/debug/interface/ia_css_debug.h
> +++ b/drivers/staging/media/atomisp/pci/runtime/debug/interface/ia_css_debug.h
> @@ -141,12 +141,6 @@ static inline void __printf(2, 0) ia_css_debug_vdtrace(unsigned int level,
>  __printf(2, 3) void ia_css_debug_dtrace(unsigned int level,
>  					const char *fmt, ...);
>  
> -/*! @brief Dump sp thread's stack contents
> - * SP thread's stack contents are set to 0xcafecafe. This function dumps the
> - * stack to inspect if the stack's boundaries are compromised.
> - * @return	None
> - */
> -void ia_css_debug_dump_sp_stack_info(void);
>  
>  /*! @brief Function to set the global dtrace verbosity level.
>   * @param[in]	trace_level	Maximum level of the messages to be traced.
> diff --git a/drivers/staging/media/atomisp/pci/runtime/debug/src/ia_css_debug.c b/drivers/staging/media/atomisp/pci/runtime/debug/src/ia_css_debug.c
> index bb6204cb42c5..bb30146c5fe7 100644
> --- a/drivers/staging/media/atomisp/pci/runtime/debug/src/ia_css_debug.c
> +++ b/drivers/staging/media/atomisp/pci/runtime/debug/src/ia_css_debug.c
> @@ -105,7 +105,8 @@
>   * TODO:SH_CSS_MAX_SP_THREADS is not the max number of sp threads
>   * future rework should fix this and remove the define MAX_THREAD_NUM
>   */
> -#define MAX_THREAD_NUM (SH_CSS_MAX_SP_THREADS + SH_CSS_MAX_SP_INTERNAL_THREADS)
> +#define MAX_THREAD_NUM_2400 (SH_CSS_MAX_SP_THREADS + SH_CSS_MAX_SP_INTERNAL_THREADS_2400)
> +#define MAX_THREAD_NUM_2401 (SH_CSS_MAX_SP_THREADS + SH_CSS_MAX_SP_INTERNAL_THREADS_2401)
>  
>  static struct pipe_graph_class {
>  	bool do_init;
> @@ -147,79 +148,6 @@ void ia_css_debug_dtrace(unsigned int level, const char *fmt, ...)
>  	va_end(ap);
>  }
>  
> -static void debug_dump_long_array_formatted(
> -    const sp_ID_t sp_id,
> -    hrt_address stack_sp_addr,
> -    unsigned int stack_size)
> -{
> -	unsigned int i;
> -	u32 val;
> -	u32 addr = (uint32_t)stack_sp_addr;
> -	u32 stack_size_words = CEIL_DIV(stack_size, sizeof(uint32_t));
> -
> -	/* When size is not multiple of four, last word is only relevant for
> -	 * remaining bytes */
> -	for (i = 0; i < stack_size_words; i++) {
> -		val = sp_dmem_load_uint32(sp_id, (hrt_address)addr);
> -		if ((i % 8) == 0)
> -			ia_css_debug_dtrace(IA_CSS_DEBUG_VERBOSE, "\n");
> -
> -		ia_css_debug_dtrace(IA_CSS_DEBUG_VERBOSE, "0x%08x ", val);
> -		addr += sizeof(uint32_t);
> -	}
> -
> -	ia_css_debug_dtrace(IA_CSS_DEBUG_VERBOSE, "\n");
> -}
> -
> -static void debug_dump_sp_stack_info(
> -    const sp_ID_t sp_id)
> -{
> -	const struct ia_css_fw_info *fw;
> -	unsigned int HIVE_ADDR_sp_threads_stack;
> -	unsigned int HIVE_ADDR_sp_threads_stack_size;
> -	u32 stack_sizes[MAX_THREAD_NUM];
> -	u32 stack_sp_addr[MAX_THREAD_NUM];
> -	unsigned int i;
> -
> -	fw = &sh_css_sp_fw;
> -
> -	ia_css_debug_dtrace(IA_CSS_DEBUG_VERBOSE, "sp_id(%u) stack info\n", sp_id);
> -	ia_css_debug_dtrace(IA_CSS_DEBUG_VERBOSE,
> -			    "from objects stack_addr_offset:0x%x stack_size_offset:0x%x\n",
> -			    fw->info.sp.threads_stack,
> -			    fw->info.sp.threads_stack_size);
> -
> -	HIVE_ADDR_sp_threads_stack = fw->info.sp.threads_stack;
> -	HIVE_ADDR_sp_threads_stack_size = fw->info.sp.threads_stack_size;
> -
> -	if (fw->info.sp.threads_stack == 0 ||
> -	    fw->info.sp.threads_stack_size == 0)
> -		return;
> -
> -	(void)HIVE_ADDR_sp_threads_stack;
> -	(void)HIVE_ADDR_sp_threads_stack_size;
> -
> -	sp_dmem_load(sp_id,
> -		     (unsigned int)sp_address_of(sp_threads_stack),
> -		     &stack_sp_addr, sizeof(stack_sp_addr));
> -	sp_dmem_load(sp_id,
> -		     (unsigned int)sp_address_of(sp_threads_stack_size),
> -		     &stack_sizes, sizeof(stack_sizes));
> -
> -	for (i = 0 ; i < MAX_THREAD_NUM; i++) {
> -		ia_css_debug_dtrace(IA_CSS_DEBUG_VERBOSE,
> -				    "thread: %u stack_addr: 0x%08x stack_size: %u\n",
> -				    i, stack_sp_addr[i], stack_sizes[i]);
> -		debug_dump_long_array_formatted(sp_id, (hrt_address)stack_sp_addr[i],
> -						stack_sizes[i]);
> -	}
> -}
> -
> -void ia_css_debug_dump_sp_stack_info(void)
> -{
> -	debug_dump_sp_stack_info(SP0_ID);
> -}
> -
>  void ia_css_debug_set_dtrace_level(const unsigned int trace_level)
>  {
>  	dbg_level = trace_level;


      reply	other threads:[~2023-06-06 10:50 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-05 10:29 [PATCH 0/3] Remove #ifdef ISP2401 and unifying sh_css_sp_group structure Kate Hsuan
2023-06-05 10:29 ` [PATCH 1/3] media: atomisp: sh_css_internal: Unifying sh_css_sp_group to remove #ifdef ISP2401 Kate Hsuan
2023-06-06 11:03   ` Hans de Goede
2023-06-05 10:29 ` [PATCH 2/3] atomisp: sh_css_params: write the sp_group config according to the ISP model Kate Hsuan
2023-06-05 11:10   ` Dan Carpenter
2023-06-06  2:34     ` Kate Hsuan
2023-06-06 11:02   ` Hans de Goede
2023-06-06 13:19     ` Hans de Goede
2023-06-07  3:20     ` Kate Hsuan
2023-06-05 10:29 ` [PATCH 3/3] media: atomisp: ia_css_debug: remove unused codes Kate Hsuan
2023-06-06 10:50   ` Hans de Goede [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5e4e49d8-1e0d-709d-0a4a-576fa1212f01@redhat.com \
    --to=hdegoede@redhat.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hpa@redhat.com \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-staging@lists.linux.dev \
    --cc=mchehab@kernel.org \
    --cc=sakari.ailus@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).