Em Thu, 5 Nov 2020 08:43:38 +0100
Mauro Carvalho Chehab <mchehab+huawei@xxxxxxxxxx> escreveu:
> Em Wed, 7 Oct 2020 11:44:23 +0300
> Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx> escreveu:
>
> > Calculate the limit offsets and the size of the limit buffer. CCS limits
> > are read into this buffer, and the offsets are helpful in accessing the
> > information in it.
> >
> > Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
> > ---
> > drivers/media/i2c/smiapp/smiapp-core.c | 40 +++++++++++++++++++++++++-
> > 1 file changed, 39 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/media/i2c/smiapp/smiapp-core.c b/drivers/media/i2c/smiapp/smiapp-core.c
> > index 105ef29152e8..47e983e9cd87 100644
> > --- a/drivers/media/i2c/smiapp/smiapp-core.c
> > +++ b/drivers/media/i2c/smiapp/smiapp-core.c
> > @@ -27,6 +27,7 @@
> > #include <media/v4l2-fwnode.h>
> > #include <media/v4l2-device.h>
> >
> > +#include "ccs-limits.h"
> > #include "smiapp.h"
> >
> > #define SMIAPP_ALIGN_DIM(dim, flags) \
> > @@ -34,6 +35,11 @@
> > ? ALIGN((dim), 2) \
> > : (dim) & ~1)
> >
> > +struct ccs_limit_offset {
> > + u16 lim;
> > + u16 info;
> > +} ccs_limit_offsets[CCS_L_LAST + 1];
> > +
>
> Hmm... that sounds weird.
>
> As you're declaring the struct inside smiapp-core.c, this
> should be static...
>
> > /*
> > * smiapp_module_idents - supported camera modules
> > */
> > @@ -3166,7 +3172,39 @@ static struct i2c_driver smiapp_i2c_driver = {
> > .id_table = smiapp_id_table,
> > };
> >
> > -module_i2c_driver(smiapp_i2c_driver);
> > +static int smiapp_module_init(void)
> > +{
> > + unsigned int i, l;
> > +
> > + for (i = 0, l = 0; ccs_limits[i].size && l < CCS_L_LAST; i++) {
> > + if (!(ccs_limits[i].flags & CCS_L_FL_SAME_REG)) {
> > + ccs_limit_offsets[l + 1].lim =
> > + ALIGN(ccs_limit_offsets[l].lim +
> > + ccs_limits[i].size,
> > + ccs_reg_width(ccs_limits[i + 1].reg));
> > + ccs_limit_offsets[l].info = i;
> > + l++;
> > + } else {
> > + ccs_limit_offsets[l].lim += ccs_limits[i].size;
> > + }
> > + }
> > +
> > + if (WARN_ON(ccs_limits[i].size))
> > + return -EINVAL;
>
> ... yet, this is the only place where this is used.
>
> It sounds to me that you should move the var to be inside this function,
>
> e. g. changing the above to:
>
> struct ccs_limit_offset {
> u16 lim;
> u16 info;
> };
>
> static int smiapp_module_init(void)
> {
> struct ccs_limit_offset ccs_limit_offsets[CCS_L_LAST + 1];
> unsigned int i, l;
Ok, it sounds that you start using this on patch 007/106.
So, it sounds that you should just need to add "static" to its definition.
>
>
> > +
> > + if (WARN_ON(l != CCS_L_LAST))
> > + return -EINVAL;
> > +
> > + return i2c_register_driver(THIS_MODULE, &smiapp_i2c_driver);
> > +}
> > +
> > +static void smiapp_module_cleanup(void)
> > +{
> > + i2c_del_driver(&smiapp_i2c_driver);
> > +}
> > +
> > +module_init(smiapp_module_init);
> > +module_exit(smiapp_module_cleanup);
> >
> > MODULE_AUTHOR("Sakari Ailus <sakari.ailus@xxxxxx>");
> > MODULE_DESCRIPTION("Generic SMIA/SMIA++ camera module driver");
>
>
>
> Thanks,
> Mauro
Thanks,
Mauro
