Thread (8 messages) 8 messages, 4 authors, 2015-12-15

Re: [Patch v5 1/2] media: v4l: ti-vpe: Add CAL v4l2 camera capture driver

From: Mauro Carvalho Chehab <hidden>
Date: 2015-12-15 15:52:54
Also in: linux-media, lkml

Em Fri, 11 Dec 2015 16:16:33 -0600
Benoit Parrot [off-list ref] escreveu:
Mauro Carvalho Chehab [off-list ref] wrote on Thu [2015-Dec-03 11:19:22 -0200]:
quoted
Em Wed, 18 Nov 2015 14:47:11 -0600
Benoit Parrot [off-list ref] escreveu:
quoted
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)
quoted
quoted
+/* 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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help