diff mbox series

[1/1] media: nxp: imx8-isi-debug: Add missing registers to debugfs output

Message ID 20231206094401.491100-1-alexander.stein@ew.tq-group.com (mailing list archive)
State New
Headers show
Series [1/1] media: nxp: imx8-isi-debug: Add missing registers to debugfs output | expand

Commit Message

Alexander Stein Dec. 6, 2023, 9:44 a.m. UTC 
The extended address registers are missing in the debug output register
list. Add them.

Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
 drivers/media/platform/nxp/imx8-isi/imx8-isi-debug.c | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Laurent Pinchart Dec. 6, 2023, 10:36 p.m. UTC | #1 
Hi Alexander,

On Wed, Dec 06, 2023 at 10:44:01AM +0100, Alexander Stein wrote:
> The extended address registers are missing in the debug output register
> list. Add them.
> 
> Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> ---
>  drivers/media/platform/nxp/imx8-isi/imx8-isi-debug.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-debug.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-debug.c
> index 6709ab7ea1f3..3ac5685d6cc1 100644
> --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-debug.c
> +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-debug.c
> @@ -66,6 +66,13 @@ static int mxc_isi_debug_dump_regs_show(struct seq_file *m, void *p)
>  		MXC_ISI_DEBUG_REG(CHNL_OUT_BUF2_ADDR_V),
>  		MXC_ISI_DEBUG_REG(CHNL_SCL_IMG_CFG),
>  		MXC_ISI_DEBUG_REG(CHNL_FLOW_CTRL),
> +		MXC_ISI_DEBUG_REG(CHNL_Y_BUF1_XTND_ADDR),
> +		MXC_ISI_DEBUG_REG(CHNL_U_BUF1_XTND_ADDR),
> +		MXC_ISI_DEBUG_REG(CHNL_V_BUF1_XTND_ADDR),
> +		MXC_ISI_DEBUG_REG(CHNL_Y_BUF2_XTND_ADDR),
> +		MXC_ISI_DEBUG_REG(CHNL_U_BUF2_XTND_ADDR),
> +		MXC_ISI_DEBUG_REG(CHNL_V_BUF2_XTND_ADDR),
> +		MXC_ISI_DEBUG_REG(CHNL_IN_BUF_XTND_ADDR),

Those registers only exist in the i.MX8MP. The i.MX8MN referenece manual
lists them as reserved and indicates they read as 0's, so it should be
safe.  The i.MX93 reference manual, however, doesn't list those
registers at all, so accessing them may lead to issues.

How what platform(s) have you tested this patch ?

>  	};
>  
>  	struct mxc_isi_pipe *pipe = m->private;
Alexander Stein Dec. 7, 2023, 7:52 a.m. UTC | #2 
Hi Laurent,

Am Mittwoch, 6. Dezember 2023, 23:36:36 CET schrieb Laurent Pinchart:
> Hi Alexander,
> 
> On Wed, Dec 06, 2023 at 10:44:01AM +0100, Alexander Stein wrote:
> > The extended address registers are missing in the debug output register
> > list. Add them.
> > 
> > Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> > ---
> > 
> >  drivers/media/platform/nxp/imx8-isi/imx8-isi-debug.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-debug.c
> > b/drivers/media/platform/nxp/imx8-isi/imx8-isi-debug.c index
> > 6709ab7ea1f3..3ac5685d6cc1 100644
> > --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-debug.c
> > +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-debug.c
> > @@ -66,6 +66,13 @@ static int mxc_isi_debug_dump_regs_show(struct seq_file
> > *m, void *p)> 
> >  		MXC_ISI_DEBUG_REG(CHNL_OUT_BUF2_ADDR_V),
> >  		MXC_ISI_DEBUG_REG(CHNL_SCL_IMG_CFG),
> >  		MXC_ISI_DEBUG_REG(CHNL_FLOW_CTRL),
> > 
> > +		MXC_ISI_DEBUG_REG(CHNL_Y_BUF1_XTND_ADDR),
> > +		MXC_ISI_DEBUG_REG(CHNL_U_BUF1_XTND_ADDR),
> > +		MXC_ISI_DEBUG_REG(CHNL_V_BUF1_XTND_ADDR),
> > +		MXC_ISI_DEBUG_REG(CHNL_Y_BUF2_XTND_ADDR),
> > +		MXC_ISI_DEBUG_REG(CHNL_U_BUF2_XTND_ADDR),
> > +		MXC_ISI_DEBUG_REG(CHNL_V_BUF2_XTND_ADDR),
> > +		MXC_ISI_DEBUG_REG(CHNL_IN_BUF_XTND_ADDR),
> 
> Those registers only exist in the i.MX8MP. The i.MX8MN referenece manual
> lists them as reserved and indicates they read as 0's, so it should be
> safe.  The i.MX93 reference manual, however, doesn't list those
> registers at all, so accessing them may lead to issues.
> 
> How what platform(s) have you tested this patch ?

Ah, you are right. That's something I didn't take into account. Up until now I 
only tested this on i.MX8MP. I hope I can test this on i.MX8MN and i.MX93 
soon.
Otherwise dumping these registers can depend on pdata->has_36bit_dma just to 
be safe.

Best regards,
Alexander

> >  	};
> >  	
> >  	struct mxc_isi_pipe *pipe = m->private;
Laurent Pinchart Dec. 7, 2023, 10:14 a.m. UTC | #3 
Hi Alexander,

On Thu, Dec 07, 2023 at 08:52:46AM +0100, Alexander Stein wrote:
> Am Mittwoch, 6. Dezember 2023, 23:36:36 CET schrieb Laurent Pinchart:
> > On Wed, Dec 06, 2023 at 10:44:01AM +0100, Alexander Stein wrote:
> > > The extended address registers are missing in the debug output register
> > > list. Add them.
> > > 
> > > Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> > > ---
> > > 
> > >  drivers/media/platform/nxp/imx8-isi/imx8-isi-debug.c | 7 +++++++
> > >  1 file changed, 7 insertions(+)
> > > 
> > > diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-debug.c
> > > b/drivers/media/platform/nxp/imx8-isi/imx8-isi-debug.c index
> > > 6709ab7ea1f3..3ac5685d6cc1 100644
> > > --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-debug.c
> > > +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-debug.c
> > > @@ -66,6 +66,13 @@ static int mxc_isi_debug_dump_regs_show(struct seq_file *m, void *p)
> > >  		MXC_ISI_DEBUG_REG(CHNL_OUT_BUF2_ADDR_V),
> > >  		MXC_ISI_DEBUG_REG(CHNL_SCL_IMG_CFG),
> > >  		MXC_ISI_DEBUG_REG(CHNL_FLOW_CTRL),
> > > 
> > > +		MXC_ISI_DEBUG_REG(CHNL_Y_BUF1_XTND_ADDR),
> > > +		MXC_ISI_DEBUG_REG(CHNL_U_BUF1_XTND_ADDR),
> > > +		MXC_ISI_DEBUG_REG(CHNL_V_BUF1_XTND_ADDR),
> > > +		MXC_ISI_DEBUG_REG(CHNL_Y_BUF2_XTND_ADDR),
> > > +		MXC_ISI_DEBUG_REG(CHNL_U_BUF2_XTND_ADDR),
> > > +		MXC_ISI_DEBUG_REG(CHNL_V_BUF2_XTND_ADDR),
> > > +		MXC_ISI_DEBUG_REG(CHNL_IN_BUF_XTND_ADDR),
> > 
> > Those registers only exist in the i.MX8MP. The i.MX8MN referenece manual
> > lists them as reserved and indicates they read as 0's, so it should be
> > safe.  The i.MX93 reference manual, however, doesn't list those
> > registers at all, so accessing them may lead to issues.
> > 
> > How what platform(s) have you tested this patch ?
> 
> Ah, you are right. That's something I didn't take into account. Up until now I 
> only tested this on i.MX8MP. I hope I can test this on i.MX8MN and i.MX93 
> soon.
> Otherwise dumping these registers can depend on pdata->has_36bit_dma just to 
> be safe.

That sounds like a good solution to me. Will you do so in a v2 ?

> > >  	};
> > >  	
> > >  	struct mxc_isi_pipe *pipe = m->private;
Alexander Stein Dec. 7, 2023, 11:09 a.m. UTC | #4 
Hi Laurent,

Am Donnerstag, 7. Dezember 2023, 11:14:00 CET schrieb Laurent Pinchart:
> Hi Alexander,
> 
> On Thu, Dec 07, 2023 at 08:52:46AM +0100, Alexander Stein wrote:
> > Am Mittwoch, 6. Dezember 2023, 23:36:36 CET schrieb Laurent Pinchart:
> > > On Wed, Dec 06, 2023 at 10:44:01AM +0100, Alexander Stein wrote:
> > > > The extended address registers are missing in the debug output
> > > > register
> > > > list. Add them.
> > > > 
> > > > Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> > > > ---
> > > > 
> > > >  drivers/media/platform/nxp/imx8-isi/imx8-isi-debug.c | 7 +++++++
> > > >  1 file changed, 7 insertions(+)
> > > > 
> > > > diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-debug.c
> > > > b/drivers/media/platform/nxp/imx8-isi/imx8-isi-debug.c index
> > > > 6709ab7ea1f3..3ac5685d6cc1 100644
> > > > --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-debug.c
> > > > +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-debug.c
> > > > @@ -66,6 +66,13 @@ static int mxc_isi_debug_dump_regs_show(struct
> > > > seq_file *m, void *p)> > > 
> > > >  		MXC_ISI_DEBUG_REG(CHNL_OUT_BUF2_ADDR_V),
> > > >  		MXC_ISI_DEBUG_REG(CHNL_SCL_IMG_CFG),
> > > >  		MXC_ISI_DEBUG_REG(CHNL_FLOW_CTRL),
> > > > 
> > > > +		MXC_ISI_DEBUG_REG(CHNL_Y_BUF1_XTND_ADDR),
> > > > +		MXC_ISI_DEBUG_REG(CHNL_U_BUF1_XTND_ADDR),
> > > > +		MXC_ISI_DEBUG_REG(CHNL_V_BUF1_XTND_ADDR),
> > > > +		MXC_ISI_DEBUG_REG(CHNL_Y_BUF2_XTND_ADDR),
> > > > +		MXC_ISI_DEBUG_REG(CHNL_U_BUF2_XTND_ADDR),
> > > > +		MXC_ISI_DEBUG_REG(CHNL_V_BUF2_XTND_ADDR),
> > > > +		MXC_ISI_DEBUG_REG(CHNL_IN_BUF_XTND_ADDR),
> > > 
> > > Those registers only exist in the i.MX8MP. The i.MX8MN referenece manual
> > > lists them as reserved and indicates they read as 0's, so it should be
> > > safe.  The i.MX93 reference manual, however, doesn't list those
> > > registers at all, so accessing them may lead to issues.
> > > 
> > > How what platform(s) have you tested this patch ?
> > 
> > Ah, you are right. That's something I didn't take into account. Up until
> > now I only tested this on i.MX8MP. I hope I can test this on i.MX8MN and
> > i.MX93 soon.
> > Otherwise dumping these registers can depend on pdata->has_36bit_dma just
> > to be safe.
> 
> That sounds like a good solution to me. Will you do so in a v2 ?

Yep, yep just sent it.

Best regards
Alexander

> 
> > > >  	};
> > > >  	
> > > >  	struct mxc_isi_pipe *pipe = m->private;
diff mbox series

Patch

diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-debug.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-debug.c
index 6709ab7ea1f3..3ac5685d6cc1 100644
--- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-debug.c
+++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-debug.c
@@ -66,6 +66,13 @@  static int mxc_isi_debug_dump_regs_show(struct seq_file *m, void *p)
 		MXC_ISI_DEBUG_REG(CHNL_OUT_BUF2_ADDR_V),
 		MXC_ISI_DEBUG_REG(CHNL_SCL_IMG_CFG),
 		MXC_ISI_DEBUG_REG(CHNL_FLOW_CTRL),
+		MXC_ISI_DEBUG_REG(CHNL_Y_BUF1_XTND_ADDR),
+		MXC_ISI_DEBUG_REG(CHNL_U_BUF1_XTND_ADDR),
+		MXC_ISI_DEBUG_REG(CHNL_V_BUF1_XTND_ADDR),
+		MXC_ISI_DEBUG_REG(CHNL_Y_BUF2_XTND_ADDR),
+		MXC_ISI_DEBUG_REG(CHNL_U_BUF2_XTND_ADDR),
+		MXC_ISI_DEBUG_REG(CHNL_V_BUF2_XTND_ADDR),
+		MXC_ISI_DEBUG_REG(CHNL_IN_BUF_XTND_ADDR),
 	};
 
 	struct mxc_isi_pipe *pipe = m->private;