On Mon, Nov 28, 2022 at 03:14:26PM +0200, Petko Manolov wrote:
> On 22-11-28 12:47:25, Kieran Bingham wrote:
> > Quoting Petko Manolov (2022-11-25 15:31:17)
> > > The header. For the moment only two modes are supported.
> > >
> > > Signed-off-by: Petko Manolov <petko.manolov@xxxxxxxxxxxx>
> > > ---
> > > drivers/media/i2c/imx492.h | 555 +++++++++++++++++++++++++++++++++++++
> > > 1 file changed, 555 insertions(+)
> > > create mode 100644 drivers/media/i2c/imx492.h
> > >
> > > diff --git a/drivers/media/i2c/imx492.h b/drivers/media/i2c/imx492.h
> > > new file mode 100644
> > > index 000000000000..301fd66c77d5
> > > --- /dev/null
> > > +++ b/drivers/media/i2c/imx492.h
> > > @@ -0,0 +1,555 @@
> > > +#ifndef __imx492_h__
> > > +#define __imx492_h__
> > > +
> > > +struct imx492_reg {
> > > + u16 address;
> > > + u8 val;
> > > +};
> > > +
> > > +static const struct imx492_reg mode_17to9_regs[] = {
> >
> > Why is this table in the header? Should it be available in multiple locations?
>
> Nope, but there are multiple modes that will eventually be in use and scrolling
> down a couple of seconds until one gets to the code started to get a bit boring.
Ideally we should get rid of those tables and use logic to compute
register values :-) That's a dream only at this point though.
I agree with Kieran that everything could be in the same file, and I
also agree with you that it's not nice to have a large list of registers
at the beginning of the driver. I'm thus fine with either option.
> > I think it is likely better in the driver itself.
>
> Will put register definitions in the .c file for the time being.
--
Regards,
Laurent Pinchart
