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

[Laurent Pinchart]

Hi Ricardo,

On Mon, Apr 25, 2022 at 01:49:57PM +0200, Ricardo Ribalda wrote:
> 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.

That's a good point. I wrote this patch to avoid maintaining the maximum
register name length manually, but I'm fine moving it to a macro instead
if that's preferred. It's debug code, so I don't mind much.

> 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