Message ID | 20220106172441.7399-1-laurent.pinchart@ideasonboard.com |
---|---|
Headers | show |
Series | staging: media: imx7-mipi-csis: Small improvements | expand |
Hi Laurent, On Thu Jan 6, 2022 at 5:24 PM WET, Laurent Pinchart wrote: > The frame counter is useful debugging information, add it to the > register dump printed by mipi_csis_dump_regs(). > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> LGTM Reviewed-by: Rui Miguel Silva <rmfrfs@gmail.com> Cheers, Rui > --- > drivers/staging/media/imx/imx7-mipi-csis.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/staging/media/imx/imx7-mipi-csis.c > index 2b73fa55c938..c9c0089ad816 100644 > --- a/drivers/staging/media/imx/imx7-mipi-csis.c > +++ b/drivers/staging/media/imx/imx7-mipi-csis.c > @@ -211,6 +211,8 @@ > #define MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_FALL BIT(4) > #define MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_RISE BIT(0) > > +#define MIPI_CSIS_FRAME_COUNTER_CH(n) (0x0100 + (n) * 4) > + > /* Non-image packet data buffers */ > #define MIPI_CSIS_PKTDATA_ODD 0x2000 > #define MIPI_CSIS_PKTDATA_EVEN 0x3000 > @@ -773,6 +775,7 @@ static int mipi_csis_dump_regs(struct csi_state *state) > { MIPI_CSIS_SDW_CONFIG_CH(0), "SDW_CONFIG_CH0" }, > { MIPI_CSIS_SDW_RESOL_CH(0), "SDW_RESOL_CH0" }, > { MIPI_CSIS_DBG_CTRL, "DBG_CTRL" }, > + { MIPI_CSIS_FRAME_COUNTER_CH(0), "FRAME_COUNTER_CH0" }, > }; > > unsigned int i; > -- > Regards, > > Laurent Pinchart
Hi Laurent, Thanks for the patch. On Thu Jan 6, 2022 at 5:24 PM WET, Laurent Pinchart wrote: > When multiple CSIS instances are present in a single graph, they are > currently all named "imx7-mipi-csis.0", which breaks the entity name > uniqueness requirement. Fix it by using the device name to create the > subdev name. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Maybe a Fix tag here to make sure it is backported, since this look to me a legit bug fix for multiple instances. Other than that LGTM. Reviewed-by: Rui Miguel Silva <rmfrfs@gmail.com> Cheers, Rui > --- > drivers/staging/media/imx/imx7-mipi-csis.c | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/staging/media/imx/imx7-mipi-csis.c > index d7bcfb1a0c52..6443cca817fe 100644 > --- a/drivers/staging/media/imx/imx7-mipi-csis.c > +++ b/drivers/staging/media/imx/imx7-mipi-csis.c > @@ -32,7 +32,6 @@ > #include <media/v4l2-subdev.h> > > #define CSIS_DRIVER_NAME "imx7-mipi-csis" > -#define CSIS_SUBDEV_NAME CSIS_DRIVER_NAME > > #define CSIS_PAD_SINK 0 > #define CSIS_PAD_SOURCE 1 > @@ -313,7 +312,6 @@ struct csi_state { > struct reset_control *mrst; > struct regulator *mipi_phy_regulator; > const struct mipi_csis_info *info; > - u8 index; > > struct v4l2_subdev sd; > struct media_pad pads[CSIS_PADS_NUM]; > @@ -1329,8 +1327,8 @@ static int mipi_csis_subdev_init(struct csi_state *state) > > v4l2_subdev_init(sd, &mipi_csis_subdev_ops); > sd->owner = THIS_MODULE; > - snprintf(sd->name, sizeof(sd->name), "%s.%d", > - CSIS_SUBDEV_NAME, state->index); > + snprintf(sd->name, sizeof(sd->name), "csis-%s", > + dev_name(state->dev)); > > sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; > sd->ctrl_handler = NULL; > -- > Regards, > > Laurent Pinchart
On Thu, Jan 06, 2022 at 07:24:41PM +0200, Laurent Pinchart wrote: > When multiple CSIS instances are present in a single graph, they are > currently all named "imx7-mipi-csis.0", which breaks the entity name > uniqueness requirement. Fix it by using the device name to create the > subdev name. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > drivers/staging/media/imx/imx7-mipi-csis.c | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/staging/media/imx/imx7-mipi-csis.c > index d7bcfb1a0c52..6443cca817fe 100644 > --- a/drivers/staging/media/imx/imx7-mipi-csis.c > +++ b/drivers/staging/media/imx/imx7-mipi-csis.c > @@ -32,7 +32,6 @@ > #include <media/v4l2-subdev.h> > > #define CSIS_DRIVER_NAME "imx7-mipi-csis" > -#define CSIS_SUBDEV_NAME CSIS_DRIVER_NAME > > #define CSIS_PAD_SINK 0 > #define CSIS_PAD_SOURCE 1 > @@ -313,7 +312,6 @@ struct csi_state { > struct reset_control *mrst; > struct regulator *mipi_phy_regulator; > const struct mipi_csis_info *info; > - u8 index; > > struct v4l2_subdev sd; > struct media_pad pads[CSIS_PADS_NUM]; > @@ -1329,8 +1327,8 @@ static int mipi_csis_subdev_init(struct csi_state *state) > > v4l2_subdev_init(sd, &mipi_csis_subdev_ops); > sd->owner = THIS_MODULE; > - snprintf(sd->name, sizeof(sd->name), "%s.%d", > - CSIS_SUBDEV_NAME, state->index); > + snprintf(sd->name, sizeof(sd->name), "csis-%s", > + dev_name(state->dev)); > > sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; > sd->ctrl_handler = NULL; Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>