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@xxxxxxxxxxxxxxxx>
Reviewed-by: Ricardo Ribalda <ribalda@xxxxxxxxxxxx>
> ---
> .../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
>
>
