* [PATCH] v4l2: i2c: ov7670: Fix PLL bypass register values
@ 2017-12-29 12:22 Jacopo Mondi
2018-12-09 23:39 ` sakari.ailus
0 siblings, 1 reply; 4+ messages in thread
From: Jacopo Mondi @ 2017-12-29 12:22 UTC (permalink / raw)
To: corbet, mchehab; +Cc: Jacopo Mondi, linux-media, linux-kernel
The following commits:
commit f6dd927f34d6 ("[media] media: ov7670: calculate framerate properly for ov7675")
commit 04ee6d92047e ("[media] media: ov7670: add possibility to bypass pll for ov7675")
introduced the ability to bypass PLL multiplier and use input clock (xvclk)
as pixel clock output frequency for ov7675 sensor.
PLL is bypassed using register DBLV[7:6], according to ov7670 and ov7675
sensor manuals. Macros used to set DBLV register seem wrong in the
driver, as their values do not match what reported in the datasheet.
Fix by changing DBLV_* macros to use bits [7:6] and set bits [3:0] to
default 0x0a reserved value (according to datasheets).
While at there, remove a write to DBLV register in
"ov7675_set_framerate()" that over-writes the previous one to the same
register that takes "info->pll_bypass" flag into account instead of setting PLL
multiplier to 4x unconditionally.
And, while at there, since "info->pll_bypass" is only used in
set/get_framerate() functions used by ov7675 only, it is not necessary
to check for the device id at probe time to make sure that when using
ov7670 "info->pll_bypass" is set to false.
Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
drivers/media/i2c/ov7670.c | 16 ++++++----------
1 file changed, 6 insertions(+), 10 deletions(-)
---
Javier, as you're the author of commit 04ee6d92047e1ac68d4eb615119343f4f0fc57db
that introduced DBLV_ macros, have you ever seen it working with those macros
values? Where did those value come from? I have checked both ov7670 and ov7675
datasheet and they seems wrong, but I don't have an ov7675 sensor, so I can
only compile test this...
Thanks
j
EDIT: seems like Javier email address is not valid anymore.. I'm dropping
it from receiver list, hoping he reads the media mailing list.
diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index 950a0ac..0a5fa33 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -158,10 +158,10 @@ MODULE_PARM_DESC(debug, "Debug level (0-1)");
#define REG_GFIX 0x69 /* Fix gain control */
#define REG_DBLV 0x6b /* PLL control an debugging */
-#define DBLV_BYPASS 0x00 /* Bypass PLL */
-#define DBLV_X4 0x01 /* clock x4 */
-#define DBLV_X6 0x10 /* clock x6 */
-#define DBLV_X8 0x11 /* clock x8 */
+#define DBLV_BYPASS 0x0a /* Bypass PLL */
+#define DBLV_X4 0x4a /* clock x4 */
+#define DBLV_X6 0x8a /* clock x6 */
+#define DBLV_X8 0xca /* clock x8 */
#define REG_REG76 0x76 /* OV's name */
#define R76_BLKPCOR 0x80 /* Black pixel correction enable */
@@ -841,7 +841,7 @@ static int ov7675_set_framerate(struct v4l2_subdev *sd,
if (ret < 0)
return ret;
- return ov7670_write(sd, REG_DBLV, DBLV_X4);
+ return 0;
}
static void ov7670_get_framerate_legacy(struct v4l2_subdev *sd,
@@ -1692,11 +1692,7 @@ static int ov7670_probe(struct i2c_client *client,
if (config->clock_speed)
info->clock_speed = config->clock_speed;
- /*
- * It should be allowed for ov7670 too when it is migrated to
- * the new frame rate formula.
- */
- if (config->pll_bypass && id->driver_data != MODEL_OV7670)
+ if (config->pll_bypass)
info->pll_bypass = true;
if (config->pclk_hb_disable)
--
2.7.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] v4l2: i2c: ov7670: Fix PLL bypass register values
2017-12-29 12:22 [PATCH] v4l2: i2c: ov7670: Fix PLL bypass register values Jacopo Mondi
@ 2018-12-09 23:39 ` sakari.ailus
2018-12-10 7:59 ` jacopo mondi
0 siblings, 1 reply; 4+ messages in thread
From: sakari.ailus @ 2018-12-09 23:39 UTC (permalink / raw)
To: Jacopo Mondi; +Cc: corbet, mchehab, linux-media, linux-kernel
Hi Jacopo,
On Fri, Dec 29, 2017 at 01:22:26PM +0100, Jacopo Mondi wrote:
> The following commits:
> commit f6dd927f34d6 ("[media] media: ov7670: calculate framerate properly for ov7675")
> commit 04ee6d92047e ("[media] media: ov7670: add possibility to bypass pll for ov7675")
> introduced the ability to bypass PLL multiplier and use input clock (xvclk)
> as pixel clock output frequency for ov7675 sensor.
>
> PLL is bypassed using register DBLV[7:6], according to ov7670 and ov7675
> sensor manuals. Macros used to set DBLV register seem wrong in the
> driver, as their values do not match what reported in the datasheet.
>
> Fix by changing DBLV_* macros to use bits [7:6] and set bits [3:0] to
> default 0x0a reserved value (according to datasheets).
>
> While at there, remove a write to DBLV register in
> "ov7675_set_framerate()" that over-writes the previous one to the same
> register that takes "info->pll_bypass" flag into account instead of setting PLL
> multiplier to 4x unconditionally.
>
> And, while at there, since "info->pll_bypass" is only used in
> set/get_framerate() functions used by ov7675 only, it is not necessary
> to check for the device id at probe time to make sure that when using
> ov7670 "info->pll_bypass" is set to false.
>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
I assume this is still valid and long overdue for merging. :-) No other
work in the area seem to have been done so I'm picking it up...
--
Regards,
Sakari Ailus
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] v4l2: i2c: ov7670: Fix PLL bypass register values
2018-12-09 23:39 ` sakari.ailus
@ 2018-12-10 7:59 ` jacopo mondi
2018-12-10 8:14 ` sakari.ailus
0 siblings, 1 reply; 4+ messages in thread
From: jacopo mondi @ 2018-12-10 7:59 UTC (permalink / raw)
To: sakari.ailus; +Cc: Jacopo Mondi, corbet, mchehab, linux-media, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1871 bytes --]
Hi Sakari,
thanks for digging this out
On Mon, Dec 10, 2018 at 01:39:17AM +0200, sakari.ailus@iki.fi wrote:
> Hi Jacopo,
>
> On Fri, Dec 29, 2017 at 01:22:26PM +0100, Jacopo Mondi wrote:
> > The following commits:
> > commit f6dd927f34d6 ("[media] media: ov7670: calculate framerate properly for ov7675")
> > commit 04ee6d92047e ("[media] media: ov7670: add possibility to bypass pll for ov7675")
> > introduced the ability to bypass PLL multiplier and use input clock (xvclk)
> > as pixel clock output frequency for ov7675 sensor.
> >
> > PLL is bypassed using register DBLV[7:6], according to ov7670 and ov7675
> > sensor manuals. Macros used to set DBLV register seem wrong in the
> > driver, as their values do not match what reported in the datasheet.
> >
> > Fix by changing DBLV_* macros to use bits [7:6] and set bits [3:0] to
> > default 0x0a reserved value (according to datasheets).
> >
> > While at there, remove a write to DBLV register in
> > "ov7675_set_framerate()" that over-writes the previous one to the same
> > register that takes "info->pll_bypass" flag into account instead of setting PLL
> > multiplier to 4x unconditionally.
> >
> > And, while at there, since "info->pll_bypass" is only used in
> > set/get_framerate() functions used by ov7675 only, it is not necessary
> > to check for the device id at probe time to make sure that when using
> > ov7670 "info->pll_bypass" is set to false.
> >
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
>
> I assume this is still valid and long overdue for merging. :-) No other
> work in the area seem to have been done so I'm picking it up...
>
It should still be valid, and should still apply regardless of its
age :) Is it worth a proper 'Fixes' tag?
Fixes: f6dd927f34d6 ("[media] media: ov7670: calculate framerate properly for ov7675")
Thanks
j
> --
> Regards,
>
> Sakari Ailus
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] v4l2: i2c: ov7670: Fix PLL bypass register values
2018-12-10 7:59 ` jacopo mondi
@ 2018-12-10 8:14 ` sakari.ailus
0 siblings, 0 replies; 4+ messages in thread
From: sakari.ailus @ 2018-12-10 8:14 UTC (permalink / raw)
To: jacopo mondi; +Cc: Jacopo Mondi, corbet, mchehab, linux-media, linux-kernel
On Mon, Dec 10, 2018 at 08:59:02AM +0100, jacopo mondi wrote:
> Hi Sakari,
> thanks for digging this out
>
> On Mon, Dec 10, 2018 at 01:39:17AM +0200, sakari.ailus@iki.fi wrote:
> > Hi Jacopo,
> >
> > On Fri, Dec 29, 2017 at 01:22:26PM +0100, Jacopo Mondi wrote:
> > > The following commits:
> > > commit f6dd927f34d6 ("[media] media: ov7670: calculate framerate properly for ov7675")
> > > commit 04ee6d92047e ("[media] media: ov7670: add possibility to bypass pll for ov7675")
> > > introduced the ability to bypass PLL multiplier and use input clock (xvclk)
> > > as pixel clock output frequency for ov7675 sensor.
> > >
> > > PLL is bypassed using register DBLV[7:6], according to ov7670 and ov7675
> > > sensor manuals. Macros used to set DBLV register seem wrong in the
> > > driver, as their values do not match what reported in the datasheet.
> > >
> > > Fix by changing DBLV_* macros to use bits [7:6] and set bits [3:0] to
> > > default 0x0a reserved value (according to datasheets).
> > >
> > > While at there, remove a write to DBLV register in
> > > "ov7675_set_framerate()" that over-writes the previous one to the same
> > > register that takes "info->pll_bypass" flag into account instead of setting PLL
> > > multiplier to 4x unconditionally.
> > >
> > > And, while at there, since "info->pll_bypass" is only used in
> > > set/get_framerate() functions used by ov7675 only, it is not necessary
> > > to check for the device id at probe time to make sure that when using
> > > ov7670 "info->pll_bypass" is set to false.
> > >
> > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> >
> > I assume this is still valid and long overdue for merging. :-) No other
> > work in the area seem to have been done so I'm picking it up...
> >
>
> It should still be valid, and should still apply regardless of its
> age :) Is it worth a proper 'Fixes' tag?
>
> Fixes: f6dd927f34d6 ("[media] media: ov7670: calculate framerate properly for ov7675")
Thanks; I added that to the commit message.
--
Sakari Ailus
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-12-10 8:14 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-29 12:22 [PATCH] v4l2: i2c: ov7670: Fix PLL bypass register values Jacopo Mondi
2018-12-09 23:39 ` sakari.ailus
2018-12-10 7:59 ` jacopo mondi
2018-12-10 8:14 ` sakari.ailus
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).