Hey Andy,
On Tue, 10 Mar 2020 at 15:30, Andy Shevchenko
<andriy.shevchenko@xxxxxxxxxxxxxxx> 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
>
>
