[PATCH v4 18/21] media: rkisp1: debug: Compute max register length name dynamically

Ricardo Ribalda ribalda at chromium.org
Mon Apr 25 04:49:57 PDT 2022


Laurent Pinchart wrote:

> Don't hardcode the register name maximum length (used for alignment
> purposes) to 14, but compute it dynamically. The small performance
> impact isn't an issue as this is debugging code.

Not sure if we want this. Different files will have different alignment,
which will look ugly if we cat them.

Besides that, the code looks correct.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
Reviewed-by: Ricardo Ribalda <ribalda at chromium.org>
> ---
>  .../platform/rockchip/rkisp1/rkisp1-debug.c   | 21 +++++++++++++------
>  1 file changed, 15 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-debug.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-debug.c
> index 2c226f20f525..28a69323cb38 100644
> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-debug.c
> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-debug.c
> @@ -11,8 +11,10 @@
>  #include <linux/debugfs.h>
>  #include <linux/delay.h>
>  #include <linux/device.h>
> +#include <linux/minmax.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/seq_file.h>
> +#include <linux/string.h>
>  
>  #include "rkisp1-common.h"
>  #include "rkisp1-regs.h"
> @@ -32,22 +34,29 @@ static int rkisp1_debug_dump_regs(struct rkisp1_device *rkisp1,
>  				  struct seq_file *m, unsigned int offset,
>  				  const struct rkisp1_debug_register *regs)
>  {
> +	const struct rkisp1_debug_register *reg;
> +	int width = 0;
>  	u32 val, shd;
>  	int ret;
>  
> +	for (reg = regs; reg->name; ++reg)
> +		width = max_t(int, width, strlen(reg->name));
> +
> +	width += 1;
> +
>  	ret = pm_runtime_get_if_in_use(rkisp1->dev);
>  	if (ret <= 0)
>  		return ret ? : -ENODATA;
>  
> -	for ( ; regs->name; ++regs) {
> -		val = rkisp1_read(rkisp1, offset + regs->reg);
> +	for (reg = regs; reg->name; ++reg) {
> +		val = rkisp1_read(rkisp1, offset + reg->reg);
>  
> -		if (regs->shd) {
> -			shd = rkisp1_read(rkisp1, offset + regs->shd);
> -			seq_printf(m, "%14s: 0x%08x/0x%08x\n", regs->name,
> +		if (reg->shd) {
> +			shd = rkisp1_read(rkisp1, offset + reg->shd);
> +			seq_printf(m, "%*s: 0x%08x/0x%08x\n", width, reg->name,
>  				   val, shd);
>  		} else {
> -			seq_printf(m, "%14s: 0x%08x\n", regs->name, val);
> +			seq_printf(m, "%*s: 0x%08x\n", width, reg->name, val);
>  		}
>  	}
>  
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> 



More information about the Linux-rockchip mailing list