diff mbox series

[10/55] media: rkisp1: cap: Print debug message on failed link validation

Message ID 20220614191127.3420492-11-paul.elder@ideasonboard.com (mailing list archive)
State Superseded
Headers show
Series media: rkisp1: Cleanups and add support for i.MX8MP | expand

Commit Message

Paul Elder June 14, 2022, 7:10 p.m. UTC 
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

When a link validation failure occurs, print a debug message to help
diagnosing the cause.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 .../media/platform/rockchip/rkisp1/rkisp1-capture.c    | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

Comments

Alexander Stein June 16, 2022, 7:32 a.m. UTC | #1 
Hello Paul,

Am Dienstag, 14. Juni 2022, 21:10:42 CEST schrieb Paul Elder:
> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> When a link validation failure occurs, print a debug message to help
> diagnosing the cause.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  .../media/platform/rockchip/rkisp1/rkisp1-capture.c    | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
> b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c index
> 94819e6c23e2..94a0d787a312 100644
> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
> @@ -1294,8 +1294,16 @@ static int rkisp1_capture_link_validate(struct
> media_link *link)
> 
>  	if (sd_fmt.format.height != cap->pix.fmt.height ||
>  	    sd_fmt.format.width != cap->pix.fmt.width ||
> -	    sd_fmt.format.code != fmt->mbus)
> +	    sd_fmt.format.code != fmt->mbus) {
> +		dev_dbg(cap->rkisp1->dev,

I wonder if a dev_warn is more suitable here.

Best regards,
Alexander

> +			"link '%s':%u -> '%s':%u not valid: 0x%04x/
%ux%u != 0x%04x/%ux%u",
> +			link->source->entity->name, link->source-
>index,
> +			link->sink->entity->name, link->sink->index,
> +			sd_fmt.format.code, sd_fmt.format.width,
> +			sd_fmt.format.height, fmt->mbus, cap-
>pix.fmt.width,
> +			cap->pix.fmt.height);
>  		return -EPIPE;
> +	}
> 
>  	return 0;
>  }
Laurent Pinchart June 16, 2022, 7:41 a.m. UTC | #2 
Hi Alexander,

On Thu, Jun 16, 2022 at 09:32:17AM +0200, Alexander Stein wrote:
> Am Dienstag, 14. Juni 2022, 21:10:42 CEST schrieb Paul Elder:
> > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > 
> > When a link validation failure occurs, print a debug message to help
> > diagnosing the cause.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  .../media/platform/rockchip/rkisp1/rkisp1-capture.c    | 10 +++++++++-
> >  1 file changed, 9 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
> > b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c index
> > 94819e6c23e2..94a0d787a312 100644
> > --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
> > +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
> > @@ -1294,8 +1294,16 @@ static int rkisp1_capture_link_validate(struct
> > media_link *link)
> > 
> >  	if (sd_fmt.format.height != cap->pix.fmt.height ||
> >  	    sd_fmt.format.width != cap->pix.fmt.width ||
> > -	    sd_fmt.format.code != fmt->mbus)
> > +	    sd_fmt.format.code != fmt->mbus) {
> > +		dev_dbg(cap->rkisp1->dev,
> 
> I wonder if a dev_warn is more suitable here.

I usually recommend dev_dbg() for conditions that are triggered directly
by userspace, to avoid giving unpriviledged applications an(other) easy
way to flood the kernel log.

> > +			"link '%s':%u -> '%s':%u not valid: 0x%04x/ %ux%u != 0x%04x/%ux%u",
> > +			link->source->entity->name, link->source->index,
> > +			link->sink->entity->name, link->sink->index,
> > +			sd_fmt.format.code, sd_fmt.format.width,
> > +			sd_fmt.format.height, fmt->mbus, cap->pix.fmt.width,
> > +			cap->pix.fmt.height);
> >  		return -EPIPE;
> > +	}
> > 
> >  	return 0;
> >  }
Alexander Stein June 16, 2022, 7:59 a.m. UTC | #3 
Hello Laurent,

Am Donnerstag, 16. Juni 2022, 09:41:22 CEST schrieb Laurent Pinchart:
> Hi Alexander,
> 
> On Thu, Jun 16, 2022 at 09:32:17AM +0200, Alexander Stein wrote:
> > Am Dienstag, 14. Juni 2022, 21:10:42 CEST schrieb Paul Elder:
> > > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > 
> > > When a link validation failure occurs, print a debug message to help
> > > diagnosing the cause.
> > > 
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > ---
> > > 
> > >  .../media/platform/rockchip/rkisp1/rkisp1-capture.c    | 10 +++++++++-
> > >  1 file changed, 9 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
> > > b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c index
> > > 94819e6c23e2..94a0d787a312 100644
> > > --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
> > > +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
> > > @@ -1294,8 +1294,16 @@ static int rkisp1_capture_link_validate(struct
> > > media_link *link)
> > > 
> > >  	if (sd_fmt.format.height != cap->pix.fmt.height ||
> > >  	
> > >  	    sd_fmt.format.width != cap->pix.fmt.width ||
> > > 
> > > -	    sd_fmt.format.code != fmt->mbus)
> > > +	    sd_fmt.format.code != fmt->mbus) {
> > > +		dev_dbg(cap->rkisp1->dev,
> > 
> > I wonder if a dev_warn is more suitable here.
> 
> I usually recommend dev_dbg() for conditions that are triggered directly
> by userspace, to avoid giving unpriviledged applications an(other) easy
> way to flood the kernel log.

Agreed, this is a sensible thing to do. I'm still wondering if this 
information might be missing, when having a build without DYNAMIC_DEBUG.
Nevertheless I'm fine to start with this debug output at least.

Regards,
Alexander

> > > +			"link '%s':%u -> '%s':%u not valid: 0x%04x/ 
%ux%u != 0x%04x/%ux%u",
> > > +			link->source->entity->name, link->source-
>index,
> > > +			link->sink->entity->name, link->sink->index,
> > > +			sd_fmt.format.code, sd_fmt.format.width,
> > > +			sd_fmt.format.height, fmt->mbus, cap-
>pix.fmt.width,
> > > +			cap->pix.fmt.height);
> > > 
> > >  		return -EPIPE;
> > > 
> > > +	}
> > > 
> > >  	return 0;
> > >  
> > >  }
Dafna Hirschfeld June 24, 2022, 3 p.m. UTC | #4 
On 15.06.2022 04:10, Paul Elder wrote:
>From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
>When a link validation failure occurs, print a debug message to help
>diagnosing the cause.
>
>Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>---
> .../media/platform/rockchip/rkisp1/rkisp1-capture.c    | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
>index 94819e6c23e2..94a0d787a312 100644
>--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
>+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
>@@ -1294,8 +1294,16 @@ static int rkisp1_capture_link_validate(struct media_link *link)
>
> 	if (sd_fmt.format.height != cap->pix.fmt.height ||
> 	    sd_fmt.format.width != cap->pix.fmt.width ||
>-	    sd_fmt.format.code != fmt->mbus)
>+	    sd_fmt.format.code != fmt->mbus) {
>+		dev_dbg(cap->rkisp1->dev,
>+			"link '%s':%u -> '%s':%u not valid: 0x%04x/%ux%u != 0x%04x/%ux%u",

missing '\n'

thanks,
Dafna

>+			link->source->entity->name, link->source->index,
>+			link->sink->entity->name, link->sink->index,
>+			sd_fmt.format.code, sd_fmt.format.width,
>+			sd_fmt.format.height, fmt->mbus, cap->pix.fmt.width,
>+			cap->pix.fmt.height);
> 		return -EPIPE;
>+	}
>
> 	return 0;
> }
>-- 
>2.30.2
>
diff mbox series

Patch

diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
index 94819e6c23e2..94a0d787a312 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
@@ -1294,8 +1294,16 @@  static int rkisp1_capture_link_validate(struct media_link *link)
 
 	if (sd_fmt.format.height != cap->pix.fmt.height ||
 	    sd_fmt.format.width != cap->pix.fmt.width ||
-	    sd_fmt.format.code != fmt->mbus)
+	    sd_fmt.format.code != fmt->mbus) {
+		dev_dbg(cap->rkisp1->dev,
+			"link '%s':%u -> '%s':%u not valid: 0x%04x/%ux%u != 0x%04x/%ux%u",
+			link->source->entity->name, link->source->index,
+			link->sink->entity->name, link->sink->index,
+			sd_fmt.format.code, sd_fmt.format.width,
+			sd_fmt.format.height, fmt->mbus, cap->pix.fmt.width,
+			cap->pix.fmt.height);
 		return -EPIPE;
+	}
 
 	return 0;
 }