Em Fri, 11 Dec 2015 16:16:33 -0600
Benoit Parrot <bparrot@xxxxxx> escreveu:
> Mauro Carvalho Chehab <mchehab@xxxxxxxxxxxxxxx> wrote on Thu [2015-Dec-03 11:19:22 -0200]:
> > Em Wed, 18 Nov 2015 14:47:11 -0600
> > Benoit Parrot <bparrot@xxxxxx> escreveu:
> >
> > > The Camera Adaptation Layer (CAL) is a block which consists of a dual
> > > port CSI2/MIPI camera capture engine.
> > > Port #0 can handle CSI2 camera connected to up to 4 data lanes.
> > > Port #1 can handle CSI2 camera connected to up to 2 data lanes.
> > > The driver implements the required API/ioctls to be V4L2 compliant.
> > > Driver supports the following:
> > > - V4L2 API using DMABUF/MMAP buffer access based on videobuf2 api
> > > - Asynchronous sensor sub device registration
> >
> > Please see the comments I did for the git pull request. Additionally,
> > see below.
>
> Yes I'll take care of the comments about the MAINTAINERS mods and patch order.
>
> However I do have a few question on your comments, see below.
(removed the code that was already agreed or that Hans commented)
> > >
> > > +/* register field read/write helpers */
> > > +static inline int get_field(u32 value, u32 mask, int shift)
> > > +{
> > > + return (value & (mask << shift)) >> shift;
> > > +}
> >
> > Please use the macros defined at bitmap.h instead of writing your own
> > version of it.
>
> Not exactly sure what you meant here.
>
> Did you mean bitops.h instead as in change read_field() and write_field()
> to use __ffs or something like that?
>
> If that is the case then I would have to change all of the bit mask and shift
> macros in cal_regs.h before I do that I want to make sure we are on the same
> page.
Yes, i mean include/linux/bitops.h.
There are several things that you dould use to simplify it using some
definitions there (and not only __ffs).
See those macros, for example:
#define BIT_MASK(nr) (1UL << ((nr) % BITS_PER_LONG))
/*
* Create a contiguous bitmask starting at bit position @l and ending at
* position @h. For example
* GENMASK_ULL(39, 21) gives us the 64bit vector 0x000000ffffe00000.
*/
#define GENMASK(h, l) \
(((~0UL) << (l)) & (~0UL >> (BITS_PER_LONG - 1 - (h))))
#define GENMASK_ULL(h, l) \
(((~0ULL) << (l)) & (~0ULL >> (BITS_PER_LONG_LONG - 1 - (h))))
Regards,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
