| Message ID | 20200310134603.30260-4-robert.foss@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: ov8856: Add sensor modes & devicetree support | expand |
On Tue, Mar 10, 2020 at 02:46:03PM +0100, Robert Foss wrote: > Query the sensor for its module revision, and compare it > to known revisions. > Currently only the '1B' revision has been added. Are you sure you send latest version? I have a déjà vu that I have seen this already and this one doesn't address any comment given. ... > + dev_info(&client->dev, "OV8856 revision %x (%s) at address 0x%02x\n", > + val, > + val == OV8856_1B_MODULE ? "1B" : "unknown revision", This is weird. Can you add a bit more general way of showing revision? > + client->addr);
Hey Andy, On Tue, 10 Mar 2020 at 15:30, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > On Tue, Mar 10, 2020 at 02:46:03PM +0100, Robert Foss wrote: > > Query the sensor for its module revision, and compare it > > to known revisions. > > Currently only the '1B' revision has been added. > > Are you sure you send latest version? > > I have a déją vu that I have seen this already and this one doesn't address any > comment given. I think pulled a series Dongchun Zhus earlier series apart and used some of it, I may have missed some of the feedback given to his v3. Sorry about that. > > ... > > > + dev_info(&client->dev, "OV8856 revision %x (%s) at address 0x%02x\n", > > + val, > > > + val == OV8856_1B_MODULE ? "1B" : "unknown revision", > > This is weird. Can you add a bit more general way of showing revision? This is modeled after the ov7251 driver, since that output came in handy during bringup. dev_info(dev, "OV7251 revision %x (%s) detected at address 0x%02x\n", chip_rev, chip_rev == 0x4 ? "1A / 1B" : chip_rev == 0x5 ? "1C / 1D" : chip_rev == 0x6 ? "1E" : chip_rev == 0x7 ? "1F" : "unknown", client->addr); To me this is pretty general approach, at least until this revision information is used in other places. I'm not quite sure what you had in mind. Maybe the current implementation is a little bit clunky in the case of ov8856 since there's only one revision number known currently. Either way, I'll happily change it. But I don't quite know what you have in mind. > > > + client->addr); > > -- > With Best Regards, > Andy Shevchenko > >
diff --git a/drivers/media/i2c/ov8856.c b/drivers/media/i2c/ov8856.c index 1769acdfaa44..48e8f4b997d6 100644 --- a/drivers/media/i2c/ov8856.c +++ b/drivers/media/i2c/ov8856.c @@ -34,6 +34,18 @@ #define OV8856_MODE_STANDBY 0x00 #define OV8856_MODE_STREAMING 0x01 +/* define 1B module revision */ +#define OV8856_1B_MODULE 0x02 + +/* the OTP read-out buffer is at 0x7000 and 0xf is the offset + * of the byte in the OTP that means the module revision + */ +#define OV8856_MODULE_REVISION 0x700f +#define OV8856_OTP_MODE_CTRL 0x3d84 +#define OV8856_OTP_LOAD_CTRL 0x3d81 +#define OV8856_OTP_MODE_AUTO 0x00 +#define OV8856_OTP_LOAD_CTRL_ENABLE BIT(0) + /* vertical-timings from sensor */ #define OV8856_REG_VTS 0x380e #define OV8856_VTS_MAX 0x7fff @@ -713,6 +725,25 @@ static int ov8856_test_pattern(struct ov8856 *ov8856, u32 pattern) OV8856_REG_VALUE_08BIT, pattern); } +static int ov8856_check_revision(struct ov8856 *ov8856) +{ + int ret; + + ret = ov8856_write_reg(ov8856, OV8856_REG_MODE_SELECT, + OV8856_REG_VALUE_08BIT, OV8856_MODE_STREAMING); + if (ret) + return ret; + + ret = ov8856_write_reg(ov8856, OV8856_OTP_MODE_CTRL, + OV8856_REG_VALUE_08BIT, OV8856_OTP_MODE_AUTO); + if (ret) + return ret; + + return ov8856_write_reg(ov8856, OV8856_OTP_LOAD_CTRL, + OV8856_REG_VALUE_08BIT, + OV8856_OTP_LOAD_CTRL_ENABLE); +} + static int ov8856_set_ctrl(struct v4l2_ctrl *ctrl) { struct ov8856 *ov8856 = container_of(ctrl->handler, @@ -1145,6 +1176,23 @@ static int ov8856_identify_module(struct ov8856 *ov8856) return -ENXIO; } + /* check sensor hardware revision */ + ret = ov8856_check_revision(ov8856); + if (ret) { + dev_err(&client->dev, "failed to check sensor revision"); + return ret; + } + + ret = ov8856_read_reg(ov8856, OV8856_MODULE_REVISION, + OV8856_REG_VALUE_08BIT, &val); + if (ret) + return ret; + + dev_info(&client->dev, "OV8856 revision %x (%s) at address 0x%02x\n", + val, + val == OV8856_1B_MODULE ? "1B" : "unknown revision", + client->addr); + return 0; }
Query the sensor for its module revision, and compare it to known revisions. Currently only the '1B' revision has been added. Signed-off-by: Robert Foss <robert.foss@linaro.org> --- drivers/media/i2c/ov8856.c | 48 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+)