| Message ID | 20200313114121.32182-13-tomi.valkeinen@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/16] media: ti-vpe: cal: fix use of wrong macro | expand |
Hi Tomi, Thank you for the patch. On Fri, Mar 13, 2020 at 01:41:18PM +0200, Tomi Valkeinen wrote: > To make csi2_wait_for_phy() more readable, move code to separate > functions. > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> > --- > drivers/media/platform/ti-vpe/cal.c | 38 ++++++++++++++++++++--------- > 1 file changed, 26 insertions(+), 12 deletions(-) > > diff --git a/drivers/media/platform/ti-vpe/cal.c b/drivers/media/platform/ti-vpe/cal.c > index 64ea92dbfac5..319312770eea 100644 > --- a/drivers/media/platform/ti-vpe/cal.c > +++ b/drivers/media/platform/ti-vpe/cal.c > @@ -822,20 +822,10 @@ static void csi2_phy_init(struct cal_ctx *ctx) > csi2_cio_pwr(ctx, true); > } > > -static void csi2_wait_for_phy(struct cal_ctx *ctx) > +static void csi2_wait_complexio_reset(struct cal_ctx *ctx) > { > int i; unsigned int ? > > - /* Steps > - * 2. Wait for completion of reset > - * Note if the external sensor is not sending byte clock, > - * the reset will timeout > - * 4.Force FORCERXMODE > - * G. Wait for all enabled lane to reach stop state > - * H. Disable pull down using pad control > - */ > - > - /* 2. Wait for reset completion */ > for (i = 0; i < 250; i++) { > if (reg_read_field(ctx->dev, > CAL_CSI2_COMPLEXIO_CFG(ctx->csi2_port), > @@ -854,7 +844,12 @@ static void csi2_wait_for_phy(struct cal_ctx *ctx) > CAL_CSI2_COMPLEXIO_CFG_RESET_DONE_RESETCOMPLETED) > ctx_err(ctx, "Timeout waiting for Complex IO reset done\n"); > > - /* 4. G. Wait for all enabled lane to reach stop state */ > +} > + > +static void csi2_wait_stop_state(struct cal_ctx *ctx) > +{ > + int i; Ditto. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + > for (i = 0; i < 10; i++) { > if (reg_read_field(ctx->dev, > CAL_CSI2_TIMING(ctx->csi2_port), > @@ -866,9 +861,28 @@ static void csi2_wait_for_phy(struct cal_ctx *ctx) > ctx->csi2_port, > reg_read(ctx->dev, CAL_CSI2_TIMING(ctx->csi2_port)), > (i >= 10) ? "(timeout)" : ""); > + > if (reg_read_field(ctx->dev, CAL_CSI2_TIMING(ctx->csi2_port), > CAL_CSI2_TIMING_FORCE_RX_MODE_IO1_MASK) != 0) > ctx_err(ctx, "Timeout waiting for stop state\n"); > +} > + > +static void csi2_wait_for_phy(struct cal_ctx *ctx) > +{ > + /* Steps > + * 2. Wait for completion of reset > + * Note if the external sensor is not sending byte clock, > + * the reset will timeout > + * 4.Force FORCERXMODE > + * G. Wait for all enabled lane to reach stop state > + * H. Disable pull down using pad control > + */ > + > + /* 2. Wait for reset completion */ > + csi2_wait_complexio_reset(ctx); > + > + /* 4. G. Wait for all enabled lane to reach stop state */ > + csi2_wait_stop_state(ctx); > > ctx_dbg(1, ctx, "CSI2_%d_REG1 = 0x%08x (Bit(31,28) should be set!)\n", > (ctx->csi2_port - 1), reg_read(ctx->cc, CAL_CSI2_PHY_REG1));
On 16/03/2020 14:41, Laurent Pinchart wrote: > Hi Tomi, > > Thank you for the patch. > > On Fri, Mar 13, 2020 at 01:41:18PM +0200, Tomi Valkeinen wrote: >> To make csi2_wait_for_phy() more readable, move code to separate >> functions. >> >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> >> --- >> drivers/media/platform/ti-vpe/cal.c | 38 ++++++++++++++++++++--------- >> 1 file changed, 26 insertions(+), 12 deletions(-) >> >> diff --git a/drivers/media/platform/ti-vpe/cal.c b/drivers/media/platform/ti-vpe/cal.c >> index 64ea92dbfac5..319312770eea 100644 >> --- a/drivers/media/platform/ti-vpe/cal.c >> +++ b/drivers/media/platform/ti-vpe/cal.c >> @@ -822,20 +822,10 @@ static void csi2_phy_init(struct cal_ctx *ctx) >> csi2_cio_pwr(ctx, true); >> } >> >> -static void csi2_wait_for_phy(struct cal_ctx *ctx) >> +static void csi2_wait_complexio_reset(struct cal_ctx *ctx) >> { >> int i; > > unsigned int ? > >> >> - /* Steps >> - * 2. Wait for completion of reset >> - * Note if the external sensor is not sending byte clock, >> - * the reset will timeout >> - * 4.Force FORCERXMODE >> - * G. Wait for all enabled lane to reach stop state >> - * H. Disable pull down using pad control >> - */ >> - >> - /* 2. Wait for reset completion */ >> for (i = 0; i < 250; i++) { >> if (reg_read_field(ctx->dev, >> CAL_CSI2_COMPLEXIO_CFG(ctx->csi2_port), >> @@ -854,7 +844,12 @@ static void csi2_wait_for_phy(struct cal_ctx *ctx) >> CAL_CSI2_COMPLEXIO_CFG_RESET_DONE_RESETCOMPLETED) >> ctx_err(ctx, "Timeout waiting for Complex IO reset done\n"); >> >> - /* 4. G. Wait for all enabled lane to reach stop state */ >> +} >> + >> +static void csi2_wait_stop_state(struct cal_ctx *ctx) >> +{ >> + int i; > > Ditto. I'll leave these, as they were "int" also in the original code, and I remove these in the next patches. Tomi
diff --git a/drivers/media/platform/ti-vpe/cal.c b/drivers/media/platform/ti-vpe/cal.c index 64ea92dbfac5..319312770eea 100644 --- a/drivers/media/platform/ti-vpe/cal.c +++ b/drivers/media/platform/ti-vpe/cal.c @@ -822,20 +822,10 @@ static void csi2_phy_init(struct cal_ctx *ctx) csi2_cio_pwr(ctx, true); } -static void csi2_wait_for_phy(struct cal_ctx *ctx) +static void csi2_wait_complexio_reset(struct cal_ctx *ctx) { int i; - /* Steps - * 2. Wait for completion of reset - * Note if the external sensor is not sending byte clock, - * the reset will timeout - * 4.Force FORCERXMODE - * G. Wait for all enabled lane to reach stop state - * H. Disable pull down using pad control - */ - - /* 2. Wait for reset completion */ for (i = 0; i < 250; i++) { if (reg_read_field(ctx->dev, CAL_CSI2_COMPLEXIO_CFG(ctx->csi2_port), @@ -854,7 +844,12 @@ static void csi2_wait_for_phy(struct cal_ctx *ctx) CAL_CSI2_COMPLEXIO_CFG_RESET_DONE_RESETCOMPLETED) ctx_err(ctx, "Timeout waiting for Complex IO reset done\n"); - /* 4. G. Wait for all enabled lane to reach stop state */ +} + +static void csi2_wait_stop_state(struct cal_ctx *ctx) +{ + int i; + for (i = 0; i < 10; i++) { if (reg_read_field(ctx->dev, CAL_CSI2_TIMING(ctx->csi2_port), @@ -866,9 +861,28 @@ static void csi2_wait_for_phy(struct cal_ctx *ctx) ctx->csi2_port, reg_read(ctx->dev, CAL_CSI2_TIMING(ctx->csi2_port)), (i >= 10) ? "(timeout)" : ""); + if (reg_read_field(ctx->dev, CAL_CSI2_TIMING(ctx->csi2_port), CAL_CSI2_TIMING_FORCE_RX_MODE_IO1_MASK) != 0) ctx_err(ctx, "Timeout waiting for stop state\n"); +} + +static void csi2_wait_for_phy(struct cal_ctx *ctx) +{ + /* Steps + * 2. Wait for completion of reset + * Note if the external sensor is not sending byte clock, + * the reset will timeout + * 4.Force FORCERXMODE + * G. Wait for all enabled lane to reach stop state + * H. Disable pull down using pad control + */ + + /* 2. Wait for reset completion */ + csi2_wait_complexio_reset(ctx); + + /* 4. G. Wait for all enabled lane to reach stop state */ + csi2_wait_stop_state(ctx); ctx_dbg(1, ctx, "CSI2_%d_REG1 = 0x%08x (Bit(31,28) should be set!)\n", (ctx->csi2_port - 1), reg_read(ctx->cc, CAL_CSI2_PHY_REG1));
To make csi2_wait_for_phy() more readable, move code to separate functions. Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> --- drivers/media/platform/ti-vpe/cal.c | 38 ++++++++++++++++++++--------- 1 file changed, 26 insertions(+), 12 deletions(-)