| Message ID | 20210413023014.28797-2-laurent.pinchart@ideasonboard.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: imx: imx7-mipi-csis: Add i.MX8MM support | expand |
On 13.04.21 04:29, Laurent Pinchart wrote: > The mipi_csis_events array ends with 6 non-error events, not 4. Update > mipi_csis_log_counters() accordingly. While at it, log event counters in > forward order, as there's no reason to log them backward. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> As there is currently no user of mipi_csis_log_counters() with non_errors set to false, maybe we should just remove this path? If you think we should keep it, I'm fine with that, too: Reviewed-by: Frieder Schrempf <frieder.schrempf@kontron.de> > --- > drivers/staging/media/imx/imx7-mipi-csis.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/staging/media/imx/imx7-mipi-csis.c > index 025fdc488bd6..25d0f89b2e53 100644 > --- a/drivers/staging/media/imx/imx7-mipi-csis.c > +++ b/drivers/staging/media/imx/imx7-mipi-csis.c > @@ -666,13 +666,15 @@ static void mipi_csis_clear_counters(struct csi_state *state) > > static void mipi_csis_log_counters(struct csi_state *state, bool non_errors) > { > - int i = non_errors ? MIPI_CSIS_NUM_EVENTS : MIPI_CSIS_NUM_EVENTS - 4; > + unsigned int num_events = non_errors ? MIPI_CSIS_NUM_EVENTS > + : MIPI_CSIS_NUM_EVENTS - 6; > struct device *dev = &state->pdev->dev; > unsigned long flags; > + unsigned int i; > > spin_lock_irqsave(&state->slock, flags); > > - for (i--; i >= 0; i--) { > + for (i = 0; i < num_events; ++i) { > if (state->events[i].counter > 0 || state->debug) > dev_info(dev, "%s events: %d\n", state->events[i].name, > state->events[i].counter); >
Hi Frieder, On Mon, Apr 26, 2021 at 01:01:50PM +0200, Frieder Schrempf wrote: > On 13.04.21 04:29, Laurent Pinchart wrote: > > The mipi_csis_events array ends with 6 non-error events, not 4. Update > > mipi_csis_log_counters() accordingly. While at it, log event counters in > > forward order, as there's no reason to log them backward. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > As there is currently no user of mipi_csis_log_counters() with > non_errors set to false, maybe we should just remove this path? > > If you think we should keep it, I'm fine with that, too: I'd rather keep it, and add a patch that logs non-error counters only when debugging is enabled. It can get too verbose otherwise. > Reviewed-by: Frieder Schrempf <frieder.schrempf@kontron.de> > > > --- > > drivers/staging/media/imx/imx7-mipi-csis.c | 6 ++++-- > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/staging/media/imx/imx7-mipi-csis.c > > index 025fdc488bd6..25d0f89b2e53 100644 > > --- a/drivers/staging/media/imx/imx7-mipi-csis.c > > +++ b/drivers/staging/media/imx/imx7-mipi-csis.c > > @@ -666,13 +666,15 @@ static void mipi_csis_clear_counters(struct csi_state *state) > > > > static void mipi_csis_log_counters(struct csi_state *state, bool non_errors) > > { > > - int i = non_errors ? MIPI_CSIS_NUM_EVENTS : MIPI_CSIS_NUM_EVENTS - 4; > > + unsigned int num_events = non_errors ? MIPI_CSIS_NUM_EVENTS > > + : MIPI_CSIS_NUM_EVENTS - 6; > > struct device *dev = &state->pdev->dev; > > unsigned long flags; > > + unsigned int i; > > > > spin_lock_irqsave(&state->slock, flags); > > > > - for (i--; i >= 0; i--) { > > + for (i = 0; i < num_events; ++i) { > > if (state->events[i].counter > 0 || state->debug) > > dev_info(dev, "%s events: %d\n", state->events[i].name, > > state->events[i].counter); > >
diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/staging/media/imx/imx7-mipi-csis.c index 025fdc488bd6..25d0f89b2e53 100644 --- a/drivers/staging/media/imx/imx7-mipi-csis.c +++ b/drivers/staging/media/imx/imx7-mipi-csis.c @@ -666,13 +666,15 @@ static void mipi_csis_clear_counters(struct csi_state *state) static void mipi_csis_log_counters(struct csi_state *state, bool non_errors) { - int i = non_errors ? MIPI_CSIS_NUM_EVENTS : MIPI_CSIS_NUM_EVENTS - 4; + unsigned int num_events = non_errors ? MIPI_CSIS_NUM_EVENTS + : MIPI_CSIS_NUM_EVENTS - 6; struct device *dev = &state->pdev->dev; unsigned long flags; + unsigned int i; spin_lock_irqsave(&state->slock, flags); - for (i--; i >= 0; i--) { + for (i = 0; i < num_events; ++i) { if (state->events[i].counter > 0 || state->debug) dev_info(dev, "%s events: %d\n", state->events[i].name, state->events[i].counter);
The mipi_csis_events array ends with 6 non-error events, not 4. Update mipi_csis_log_counters() accordingly. While at it, log event counters in forward order, as there's no reason to log them backward. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- drivers/staging/media/imx/imx7-mipi-csis.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)