| Message ID | 20200313114121.32182-16-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:21PM +0200, Tomi Valkeinen wrote: > The stop-state timeout needs to be over 100us as per CSI spec. With the > CAL fclk of 266 MHZ on DRA76, with the current value the driver uses, > the timeout is 24us. Too small timeout will cause failure to enable the > streaming. > > Also, the fclk can be different on other SoCs, as is the case with AM65x > where the fclk is 250 MHz. > > This patch fixes the timeout by calculating it correctly based on the > fclk rate. > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> > --- > drivers/media/platform/ti-vpe/cal.c | 16 ++++++++++++++-- > 1 file changed, 14 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/platform/ti-vpe/cal.c b/drivers/media/platform/ti-vpe/cal.c > index df5a4281838b..e9dd405b8eb1 100644 > --- a/drivers/media/platform/ti-vpe/cal.c > +++ b/drivers/media/platform/ti-vpe/cal.c > @@ -6,6 +6,7 @@ > * Benoit Parrot, <bparrot@ti.com> > */ > > +#include <linux/clk.h> > #include <linux/interrupt.h> > #include <linux/io.h> > #include <linux/ioctl.h> > @@ -340,6 +341,7 @@ static const struct cal_data am654_cal_data = { > * all instances. > */ > struct cal_dev { > + struct clk *fclk; > int irq; > void __iomem *base; > struct resource *res; > @@ -766,6 +768,7 @@ static void csi2_phy_config(struct cal_ctx *ctx); > static void csi2_phy_init(struct cal_ctx *ctx) > { > u32 val; > + u32 sscounter; > > /* Steps > * 1. Configure D-PHY mode and enable required lanes > @@ -802,10 +805,13 @@ static void csi2_phy_init(struct cal_ctx *ctx) > csi2_phy_config(ctx); > > /* 3.B. Program Stop States */ > + /* Must be more than 100us */ You may want to expand this comment to explain the calculation (especially the * 16 * 4). Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + sscounter = DIV_ROUND_UP(clk_get_rate(ctx->dev->fclk), 10000 * 16 * 4); > + > val = reg_read(ctx->dev, CAL_CSI2_TIMING(ctx->csi2_port)); > set_field(&val, 1, CAL_CSI2_TIMING_STOP_STATE_X16_IO1_MASK); > - set_field(&val, 0, CAL_CSI2_TIMING_STOP_STATE_X4_IO1_MASK); > - set_field(&val, 407, CAL_CSI2_TIMING_STOP_STATE_COUNTER_IO1_MASK); > + set_field(&val, 1, CAL_CSI2_TIMING_STOP_STATE_X4_IO1_MASK); > + set_field(&val, sscounter, CAL_CSI2_TIMING_STOP_STATE_COUNTER_IO1_MASK); > reg_write(ctx->dev, CAL_CSI2_TIMING(ctx->csi2_port), val); > ctx_dbg(3, ctx, "CAL_CSI2_TIMING(%d) = 0x%08x Stop States\n", > ctx->csi2_port, > @@ -2263,6 +2269,12 @@ static int cal_probe(struct platform_device *pdev) > /* save pdev pointer */ > dev->pdev = pdev; > > + dev->fclk = devm_clk_get(&pdev->dev, "fck"); > + if (IS_ERR(dev->fclk)) { > + dev_err(&pdev->dev, "cannot get CAL fclk\n"); > + return PTR_ERR(dev->fclk); > + } > + > syscon_camerrx = syscon_regmap_lookup_by_phandle(parent, > "ti,camerrx-control"); > ret = of_property_read_u32_index(parent, "ti,camerrx-control", 1,
diff --git a/drivers/media/platform/ti-vpe/cal.c b/drivers/media/platform/ti-vpe/cal.c index df5a4281838b..e9dd405b8eb1 100644 --- a/drivers/media/platform/ti-vpe/cal.c +++ b/drivers/media/platform/ti-vpe/cal.c @@ -6,6 +6,7 @@ * Benoit Parrot, <bparrot@ti.com> */ +#include <linux/clk.h> #include <linux/interrupt.h> #include <linux/io.h> #include <linux/ioctl.h> @@ -340,6 +341,7 @@ static const struct cal_data am654_cal_data = { * all instances. */ struct cal_dev { + struct clk *fclk; int irq; void __iomem *base; struct resource *res; @@ -766,6 +768,7 @@ static void csi2_phy_config(struct cal_ctx *ctx); static void csi2_phy_init(struct cal_ctx *ctx) { u32 val; + u32 sscounter; /* Steps * 1. Configure D-PHY mode and enable required lanes @@ -802,10 +805,13 @@ static void csi2_phy_init(struct cal_ctx *ctx) csi2_phy_config(ctx); /* 3.B. Program Stop States */ + /* Must be more than 100us */ + sscounter = DIV_ROUND_UP(clk_get_rate(ctx->dev->fclk), 10000 * 16 * 4); + val = reg_read(ctx->dev, CAL_CSI2_TIMING(ctx->csi2_port)); set_field(&val, 1, CAL_CSI2_TIMING_STOP_STATE_X16_IO1_MASK); - set_field(&val, 0, CAL_CSI2_TIMING_STOP_STATE_X4_IO1_MASK); - set_field(&val, 407, CAL_CSI2_TIMING_STOP_STATE_COUNTER_IO1_MASK); + set_field(&val, 1, CAL_CSI2_TIMING_STOP_STATE_X4_IO1_MASK); + set_field(&val, sscounter, CAL_CSI2_TIMING_STOP_STATE_COUNTER_IO1_MASK); reg_write(ctx->dev, CAL_CSI2_TIMING(ctx->csi2_port), val); ctx_dbg(3, ctx, "CAL_CSI2_TIMING(%d) = 0x%08x Stop States\n", ctx->csi2_port, @@ -2263,6 +2269,12 @@ static int cal_probe(struct platform_device *pdev) /* save pdev pointer */ dev->pdev = pdev; + dev->fclk = devm_clk_get(&pdev->dev, "fck"); + if (IS_ERR(dev->fclk)) { + dev_err(&pdev->dev, "cannot get CAL fclk\n"); + return PTR_ERR(dev->fclk); + } + syscon_camerrx = syscon_regmap_lookup_by_phandle(parent, "ti,camerrx-control"); ret = of_property_read_u32_index(parent, "ti,camerrx-control", 1,
The stop-state timeout needs to be over 100us as per CSI spec. With the CAL fclk of 266 MHZ on DRA76, with the current value the driver uses, the timeout is 24us. Too small timeout will cause failure to enable the streaming. Also, the fclk can be different on other SoCs, as is the case with AM65x where the fclk is 250 MHz. This patch fixes the timeout by calculating it correctly based on the fclk rate. Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> --- drivers/media/platform/ti-vpe/cal.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-)