Re: [PATCH v3 1/9] string: introduce memweight
2012/6/21 Tony Luck tony.l...@gmail.com: On Fri, Jun 8, 2012 at 5:50 PM, Akinobu Mita akinobu.m...@gmail.com wrote: lib/string.c | 36 Is lib/string.c the right place for this? I get a build error on the ia64 sim_defconfig: LD arch/ia64/hp/sim/boot/bootloader It fails because it pulls in lib/lib.a(string.o) to get some innocuous function like strcpy() ... but it also gets given memweight() which relies on __bitmap_weight() which it doesn't have, because it doesn't include lib/built-in.o (which is where bitmap.o, the definer of __bitmap_weight(), has been linked). Moving memweight() to lib/bitmap.c fixes the problem. But it isn't really clear that it belongs there either. Perhaps it should be its own file lib/memweight.c that gets included in lib/lib.a? I'll fix it by making lib/memweight.c as you suggested. Thanks for your report and suggestion. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 01/10] string: introduce memweight
memweight() is the function that counts the total number of bits set
in memory area. The memory area doesn't need to be aligned to
long-word boundary unlike bitmap_weight().
Signed-off-by: Akinobu Mita akinobu.m...@gmail.com
Cc: Anders Larsen a...@alarsen.net
Cc: Alasdair Kergon a...@redhat.com
Cc: dm-de...@redhat.com
Cc: linux-fsde...@vger.kernel.org
Cc: Laurent Pinchart laurent.pinch...@ideasonboard.com
Cc: linux-media@vger.kernel.org
Cc: Mark Fasheh mfas...@suse.com
Cc: Joel Becker jl...@evilplan.org
Cc: ocfs2-de...@oss.oracle.com
Cc: Jan Kara j...@suse.cz
Cc: linux-e...@vger.kernel.org
Cc: Andrew Morton a...@linux-foundation.org
Cc: Andreas Dilger adilger.ker...@dilger.ca
Cc: Theodore Ts'o ty...@mit.edu
---
include/linux/string.h |3 +++
lib/string.c | 37 +
2 files changed, 40 insertions(+), 0 deletions(-)
diff --git a/include/linux/string.h b/include/linux/string.h
index e033564..ffe0442 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -145,4 +145,7 @@ static inline bool strstarts(const char *str, const char
*prefix)
return strncmp(str, prefix, strlen(prefix)) == 0;
}
#endif
+
+extern size_t memweight(const void *ptr, size_t bytes);
+
#endif /* _LINUX_STRING_H_ */
diff --git a/lib/string.c b/lib/string.c
index e5878de..c8b92a0 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -26,6 +26,7 @@
#include linux/export.h
#include linux/bug.h
#include linux/errno.h
+#include linux/bitmap.h
#ifndef __HAVE_ARCH_STRNICMP
/**
@@ -824,3 +825,39 @@ void *memchr_inv(const void *start, int c, size_t bytes)
return check_bytes8(start, value, bytes % 8);
}
EXPORT_SYMBOL(memchr_inv);
+
+/**
+ * memweight - count the total number of bits set in memory area
+ * @ptr: pointer to the start of the area
+ * @bytes: the size of the area
+ */
+size_t memweight(const void *ptr, size_t bytes)
+{
+ size_t w = 0;
+ size_t longs;
+ union {
+ const void *ptr;
+ const unsigned char *b;
+ unsigned long address;
+ } bitmap;
+
+ for (bitmap.ptr = ptr; bytes 0 bitmap.address % sizeof(long);
+ bytes--, bitmap.address++)
+ w += hweight8(*bitmap.b);
+
+ for (longs = bytes / sizeof(long); longs 0; ) {
+ size_t bits = min_t(size_t, INT_MAX ~(BITS_PER_LONG - 1),
+ longs * BITS_PER_LONG);
+
+ w += bitmap_weight(bitmap.ptr, bits);
+ bytes -= bits / BITS_PER_BYTE;
+ bitmap.address += bits / BITS_PER_BYTE;
+ longs -= bits / BITS_PER_LONG;
+ }
+
+ for (; bytes 0; bytes--, bitmap.address++)
+ w += hweight8(*bitmap.b);
+
+ return w;
+}
+EXPORT_SYMBOL(memweight);
--
1.7.7.6
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 06/10] video/uvc: use memweight()
Use memweight() to count the total number of bits set in memory area.
Signed-off-by: Akinobu Mita akinobu.m...@gmail.com
Cc: Laurent Pinchart laurent.pinch...@ideasonboard.com
Cc: linux-media@vger.kernel.org
---
drivers/media/video/uvc/uvc_ctrl.c |5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/media/video/uvc/uvc_ctrl.c
b/drivers/media/video/uvc/uvc_ctrl.c
index 0efd3b1..8683be0 100644
--- a/drivers/media/video/uvc/uvc_ctrl.c
+++ b/drivers/media/video/uvc/uvc_ctrl.c
@@ -1851,7 +1851,7 @@ int uvc_ctrl_init_device(struct uvc_device *dev)
/* Walk the entities list and instantiate controls */
list_for_each_entry(entity, dev-entities, list) {
struct uvc_control *ctrl;
- unsigned int bControlSize = 0, ncontrols = 0;
+ unsigned int bControlSize = 0, ncontrols;
__u8 *bmControls = NULL;
if (UVC_ENTITY_TYPE(entity) == UVC_VC_EXTENSION_UNIT) {
@@ -1869,8 +1869,7 @@ int uvc_ctrl_init_device(struct uvc_device *dev)
uvc_ctrl_prune_entity(dev, entity);
/* Count supported controls and allocate the controls array */
- for (i = 0; i bControlSize; ++i)
- ncontrols += hweight8(bmControls[i]);
+ ncontrols = memweight(bmControls, bControlSize);
if (ncontrols == 0)
continue;
--
1.7.7.6
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 06/10] video/uvc: use memweight()
2012/5/21 Laurent Pinchart laurent.pinch...@ideasonboard.com: Hi Akinobu, Thank you for the patch. On Sunday 20 May 2012 22:23:19 Akinobu Mita wrote: Use memweight() to count the total number of bits set in memory area. Signed-off-by: Akinobu Mita akinobu.m...@gmail.com Cc: Laurent Pinchart laurent.pinch...@ideasonboard.com Cc: linux-media@vger.kernel.org Laurent Pinchart laurent.pinch...@ideasonboard.com You meant Acked-by, didn't you? -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/10] string: introduce memweight
2012/5/23 Jan Kara j...@suse.cz:
On Sun 20-05-12 22:23:14, Akinobu Mita wrote:
memweight() is the function that counts the total number of bits set
in memory area. The memory area doesn't need to be aligned to
long-word boundary unlike bitmap_weight().
Thanks for the patch. I have some comments below.
Thanks for the review.
@@ -824,3 +825,39 @@ void *memchr_inv(const void *start, int c, size_t bytes)
return check_bytes8(start, value, bytes % 8);
}
EXPORT_SYMBOL(memchr_inv);
+
+/**
+ * memweight - count the total number of bits set in memory area
+ * @ptr: pointer to the start of the area
+ * @bytes: the size of the area
+ */
+size_t memweight(const void *ptr, size_t bytes)
+{
+ size_t w = 0;
+ size_t longs;
+ union {
+ const void *ptr;
+ const unsigned char *b;
+ unsigned long address;
+ } bitmap;
Ugh, this is ugly and mostly unnecessary. Just use const unsigned char
*bitmap.
+
+ for (bitmap.ptr = ptr; bytes 0 bitmap.address % sizeof(long);
+ bytes--, bitmap.address++)
+ w += hweight8(*bitmap.b);
This can be:
count = ((unsigned long)bitmap) % sizeof(long);
The count should be the size of unaligned area and it can be greater than
bytes. So
count = min(bytes,
sizeof(long) - ((unsigned long)bitmap) % sizeof(long));
while (count--) {
w += hweight(*bitmap);
bitmap++;
bytes--;
}
+
+ for (longs = bytes / sizeof(long); longs 0; ) {
+ size_t bits = min_t(size_t, INT_MAX ~(BITS_PER_LONG - 1),
+ longs * BITS_PER_LONG);
I find it highly unlikely that someone would have such a large bitmap
(256 MB or more on 32-bit). Also the condition as you wrote it can just
overflow so it won't have the desired effect. Just do
BUG_ON(longs = ULONG_MAX / BITS_PER_LONG);
The bits argument of bitmap_weight() is int type. So this should be
BUG_ON(longs = INT_MAX / BITS_PER_LONG);
and remove the loop completely. If someone comes with such a huge bitmap,
the code can be modified easily (after really closely inspecting whether
such a huge bitmap is really well justified).
size_t memweight(const void *ptr, size_t bytes)
{
size_t w = 0;
size_t longs;
const unsigned char *bitmap = ptr;
for (; bytes 0 ((unsigned long)bitmap) % sizeof(long);
bytes--, bitmap++)
w += hweight8(*bitmap);
longs = bytes / sizeof(long);
BUG_ON(longs = INT_MAX / BITS_PER_LONG);
w += bitmap_weight((unsigned long *)bitmap, longs * BITS_PER_LONG);
bytes -= longs * sizeof(long);
bitmap += longs * sizeof(long);
for (; bytes 0; bytes--, bitmap++)
w += hweight8(*bitmap);
return w;
}
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/10] string: introduce memweight
2012/5/23 Matthew Wilcox matt...@wil.cx:
On Wed, May 23, 2012 at 09:12:18PM +0900, Akinobu Mita wrote:
size_t memweight(const void *ptr, size_t bytes)
Why should this return size_t instead of unsigned long?
I just use the same type as the bytes argument without mature
consideration. If unsigned long is better than size_t, I'll
change the return type.
{
size_t w = 0;
size_t longs;
const unsigned char *bitmap = ptr;
for (; bytes 0 ((unsigned long)bitmap) % sizeof(long);
bytes--, bitmap++)
w += hweight8(*bitmap);
longs = bytes / sizeof(long);
BUG_ON(longs = INT_MAX / BITS_PER_LONG);
w += bitmap_weight((unsigned long *)bitmap, longs * BITS_PER_LONG);
bytes -= longs * sizeof(long);
bitmap += longs * sizeof(long);
for (; bytes 0; bytes--, bitmap++)
w += hweight8(*bitmap);
return w;
}
bitmap_weight copes with a bitmask that isn't a multiple of BITS_PER_LONG
in size already. So I think this can be done as:
unsigned long memweight(const void *s, size_t n)
{
const unsigned char *ptr = s;
unsigned long r = 0;
while (n 0 (unsigned long)ptr % sizeof(long)) {
r += hweight8(*ptr);
n--;
ptr++;
}
BUG_ON(n = INT_MAX / 8)
return r + bitmap_weight((unsigned long *)ptr, n * 8);
}
This works perfectly on little-endian machines. But it doesn't work
on big-endian machines, if the bottom edge of memory area is not
aligned on long word boundary.
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 01/10] string: introduce memweight
memweight() is the function that counts the total number of bits set
in memory area. Unlike bitmap_weight(), memweight() takes pointer
and size in bytes to specify a memory area which does not need to be
aligned to long-word boundary.
Signed-off-by: Akinobu Mita akinobu.m...@gmail.com
Cc: Anders Larsen a...@alarsen.net
Cc: Alasdair Kergon a...@redhat.com
Cc: dm-de...@redhat.com
Cc: linux-fsde...@vger.kernel.org
Cc: Laurent Pinchart laurent.pinch...@ideasonboard.com
Cc: linux-media@vger.kernel.org
Cc: Mark Fasheh mfas...@suse.com
Cc: Joel Becker jl...@evilplan.org
Cc: ocfs2-de...@oss.oracle.com
Cc: Jan Kara j...@suse.cz
Cc: linux-e...@vger.kernel.org
Cc: Andrew Morton a...@linux-foundation.org
Cc: Andreas Dilger adilger.ker...@dilger.ca
Cc: Theodore Ts'o ty...@mit.edu
Cc: Matthew Wilcox matt...@wil.cx
---
v2: simplify memweight(), adviced by Jan Kara
include/linux/string.h |3 +++
lib/string.c | 32
2 files changed, 35 insertions(+), 0 deletions(-)
diff --git a/include/linux/string.h b/include/linux/string.h
index e033564..ffe0442 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -145,4 +145,7 @@ static inline bool strstarts(const char *str, const char
*prefix)
return strncmp(str, prefix, strlen(prefix)) == 0;
}
#endif
+
+extern size_t memweight(const void *ptr, size_t bytes);
+
#endif /* _LINUX_STRING_H_ */
diff --git a/lib/string.c b/lib/string.c
index e5878de..bf4d5a8 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -26,6 +26,7 @@
#include linux/export.h
#include linux/bug.h
#include linux/errno.h
+#include linux/bitmap.h
#ifndef __HAVE_ARCH_STRNICMP
/**
@@ -824,3 +825,34 @@ void *memchr_inv(const void *start, int c, size_t bytes)
return check_bytes8(start, value, bytes % 8);
}
EXPORT_SYMBOL(memchr_inv);
+
+/**
+ * memweight - count the total number of bits set in memory area
+ * @ptr: pointer to the start of the area
+ * @bytes: the size of the area
+ */
+size_t memweight(const void *ptr, size_t bytes)
+{
+ size_t w = 0;
+ size_t longs;
+ const unsigned char *bitmap = ptr;
+
+ for (; bytes 0 ((unsigned long)bitmap) % sizeof(long);
+ bytes--, bitmap++)
+ w += hweight8(*bitmap);
+
+ longs = bytes / sizeof(long);
+ if (longs) {
+ BUG_ON(longs = INT_MAX / BITS_PER_LONG);
+ w += bitmap_weight((unsigned long *)bitmap,
+ longs * BITS_PER_LONG);
+ bytes -= longs * sizeof(long);
+ bitmap += longs * sizeof(long);
+ }
+
+ for (; bytes 0; bytes--, bitmap++)
+ w += hweight8(*bitmap);
+
+ return w;
+}
+EXPORT_SYMBOL(memweight);
--
1.7.7.6
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 06/10] video/uvc: use memweight()
Use memweight() to count the total number of bits set in memory area.
Signed-off-by: Akinobu Mita akinobu.m...@gmail.com
Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
Cc: linux-media@vger.kernel.org
---
No changes from v1
drivers/media/video/uvc/uvc_ctrl.c |5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/media/video/uvc/uvc_ctrl.c
b/drivers/media/video/uvc/uvc_ctrl.c
index af26bbe..f7061a5 100644
--- a/drivers/media/video/uvc/uvc_ctrl.c
+++ b/drivers/media/video/uvc/uvc_ctrl.c
@@ -2083,7 +2083,7 @@ int uvc_ctrl_init_device(struct uvc_device *dev)
/* Walk the entities list and instantiate controls */
list_for_each_entry(entity, dev-entities, list) {
struct uvc_control *ctrl;
- unsigned int bControlSize = 0, ncontrols = 0;
+ unsigned int bControlSize = 0, ncontrols;
__u8 *bmControls = NULL;
if (UVC_ENTITY_TYPE(entity) == UVC_VC_EXTENSION_UNIT) {
@@ -2101,8 +2101,7 @@ int uvc_ctrl_init_device(struct uvc_device *dev)
uvc_ctrl_prune_entity(dev, entity);
/* Count supported controls and allocate the controls array */
- for (i = 0; i bControlSize; ++i)
- ncontrols += hweight8(bmControls[i]);
+ ncontrols = memweight(bmControls, bControlSize);
if (ncontrols == 0)
continue;
--
1.7.7.6
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 01/10] string: introduce memweight
2012/6/4 Jan Kara j...@suse.cz:
On Sat 02-06-12 22:40:07, Akinobu Mita wrote:
memweight() is the function that counts the total number of bits set
in memory area. Unlike bitmap_weight(), memweight() takes pointer
and size in bytes to specify a memory area which does not need to be
aligned to long-word boundary.
Signed-off-by: Akinobu Mita akinobu.m...@gmail.com
Cc: Anders Larsen a...@alarsen.net
Cc: Alasdair Kergon a...@redhat.com
Cc: dm-de...@redhat.com
Cc: linux-fsde...@vger.kernel.org
Cc: Laurent Pinchart laurent.pinch...@ideasonboard.com
Cc: linux-media@vger.kernel.org
Cc: Mark Fasheh mfas...@suse.com
Cc: Joel Becker jl...@evilplan.org
Cc: ocfs2-de...@oss.oracle.com
Cc: Jan Kara j...@suse.cz
Cc: linux-e...@vger.kernel.org
Cc: Andrew Morton a...@linux-foundation.org
Cc: Andreas Dilger adilger.ker...@dilger.ca
Cc: Theodore Ts'o ty...@mit.edu
Cc: Matthew Wilcox matt...@wil.cx
---
v2: simplify memweight(), adviced by Jan Kara
include/linux/string.h | 3 +++
lib/string.c | 32
2 files changed, 35 insertions(+), 0 deletions(-)
diff --git a/include/linux/string.h b/include/linux/string.h
index e033564..ffe0442 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -145,4 +145,7 @@ static inline bool strstarts(const char *str, const char
*prefix)
return strncmp(str, prefix, strlen(prefix)) == 0;
}
#endif
+
+extern size_t memweight(const void *ptr, size_t bytes);
+
#endif /* _LINUX_STRING_H_ */
diff --git a/lib/string.c b/lib/string.c
index e5878de..bf4d5a8 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -26,6 +26,7 @@
#include linux/export.h
#include linux/bug.h
#include linux/errno.h
+#include linux/bitmap.h
#ifndef __HAVE_ARCH_STRNICMP
/**
@@ -824,3 +825,34 @@ void *memchr_inv(const void *start, int c, size_t bytes)
return check_bytes8(start, value, bytes % 8);
}
EXPORT_SYMBOL(memchr_inv);
+
+/**
+ * memweight - count the total number of bits set in memory area
+ * @ptr: pointer to the start of the area
+ * @bytes: the size of the area
+ */
+size_t memweight(const void *ptr, size_t bytes)
+{
+ size_t w = 0;
+ size_t longs;
+ const unsigned char *bitmap = ptr;
+
+ for (; bytes 0 ((unsigned long)bitmap) % sizeof(long);
+ bytes--, bitmap++)
+ w += hweight8(*bitmap);
+
+ longs = bytes / sizeof(long);
+ if (longs) {
+ BUG_ON(longs = INT_MAX / BITS_PER_LONG);
+ w += bitmap_weight((unsigned long *)bitmap,
+ longs * BITS_PER_LONG);
+ bytes -= longs * sizeof(long);
+ bitmap += longs * sizeof(long);
+ }
+
+ for (; bytes 0; bytes--, bitmap++)
+ w += hweight8(*bitmap);
Looking at bitmap_weight() it seems this last loop is not needed. Just
pass to bitmap_weight() bytes*BITS_PER_BYTE. Also generally this function
doesn't seem necessary at all at least for ext2 ext3 (sorry for not
noticing this earlier...).
This last loop is necessary for big-endian architecture.
if bytes % sizeof(long) != 0, bitmap_weight() counts one-bits in wrong
byte-field
of the last long word.
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 01/10] string: introduce memweight
2012/6/4 Jan Kara j...@suse.cz:
On Mon 04-06-12 20:46:14, Akinobu Mita wrote:
2012/6/4 Jan Kara j...@suse.cz:
On Sat 02-06-12 22:40:07, Akinobu Mita wrote:
memweight() is the function that counts the total number of bits set
in memory area. áUnlike bitmap_weight(), memweight() takes pointer
and size in bytes to specify a memory area which does not need to be
aligned to long-word boundary.
Signed-off-by: Akinobu Mita akinobu.m...@gmail.com
Cc: Anders Larsen a...@alarsen.net
Cc: Alasdair Kergon a...@redhat.com
Cc: dm-de...@redhat.com
Cc: linux-fsde...@vger.kernel.org
Cc: Laurent Pinchart laurent.pinch...@ideasonboard.com
Cc: linux-media@vger.kernel.org
Cc: Mark Fasheh mfas...@suse.com
Cc: Joel Becker jl...@evilplan.org
Cc: ocfs2-de...@oss.oracle.com
Cc: Jan Kara j...@suse.cz
Cc: linux-e...@vger.kernel.org
Cc: Andrew Morton a...@linux-foundation.org
Cc: Andreas Dilger adilger.ker...@dilger.ca
Cc: Theodore Ts'o ty...@mit.edu
Cc: Matthew Wilcox matt...@wil.cx
---
v2: simplify memweight(), adviced by Jan Kara
áinclude/linux/string.h | á á3 +++
álib/string.c á á á á á | á 32
á2 files changed, 35 insertions(+), 0 deletions(-)
diff --git a/include/linux/string.h b/include/linux/string.h
index e033564..ffe0442 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -145,4 +145,7 @@ static inline bool strstarts(const char *str, const
char *prefix)
á á á return strncmp(str, prefix, strlen(prefix)) == 0;
á}
á#endif
+
+extern size_t memweight(const void *ptr, size_t bytes);
+
á#endif /* _LINUX_STRING_H_ */
diff --git a/lib/string.c b/lib/string.c
index e5878de..bf4d5a8 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -26,6 +26,7 @@
á#include linux/export.h
á#include linux/bug.h
á#include linux/errno.h
+#include linux/bitmap.h
á#ifndef __HAVE_ARCH_STRNICMP
á/**
@@ -824,3 +825,34 @@ void *memchr_inv(const void *start, int c, size_t
bytes)
á á á return check_bytes8(start, value, bytes % 8);
á}
áEXPORT_SYMBOL(memchr_inv);
+
+/**
+ * memweight - count the total number of bits set in memory area
+ * @ptr: pointer to the start of the area
+ * @bytes: the size of the area
+ */
+size_t memweight(const void *ptr, size_t bytes)
+{
+ á á size_t w = 0;
+ á á size_t longs;
+ á á const unsigned char *bitmap = ptr;
+
+ á á for (; bytes 0 ((unsigned long)bitmap) % sizeof(long);
+ á á á á á á á á á á bytes--, bitmap++)
+ á á á á á á w += hweight8(*bitmap);
+
+ á á longs = bytes / sizeof(long);
+ á á if (longs) {
+ á á á á á á BUG_ON(longs = INT_MAX / BITS_PER_LONG);
+ á á á á á á w += bitmap_weight((unsigned long *)bitmap,
+ á á á á á á á á á á á á á á longs * BITS_PER_LONG);
+ á á á á á á bytes -= longs * sizeof(long);
+ á á á á á á bitmap += longs * sizeof(long);
+ á á }
+
+ á á for (; bytes 0; bytes--, bitmap++)
+ á á á á á á w += hweight8(*bitmap);
áLooking at bitmap_weight() it seems this last loop is not needed. Just
pass to bitmap_weight() bytes*BITS_PER_BYTE. Also generally this function
doesn't seem necessary at all at least for ext2 ext3 (sorry for not
noticing this earlier...).
This last loop is necessary for big-endian architecture.
if bytes % sizeof(long) != 0, bitmap_weight() counts one-bits in wrong
byte-field
of the last long word.
Ah, right. OK. Add this to the comment before the loop please. You save
yourself some explanations :)
OK. Thanks for your advice again.
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 1/9] string: introduce memweight
memweight() is the function that counts the total number of bits set
in memory area. Unlike bitmap_weight(), memweight() takes pointer
and size in bytes to specify a memory area which does not need to be
aligned to long-word boundary.
Signed-off-by: Akinobu Mita akinobu.m...@gmail.com
Cc: Anders Larsen a...@alarsen.net
Cc: Alasdair Kergon a...@redhat.com
Cc: dm-de...@redhat.com
Cc: linux-fsde...@vger.kernel.org
Cc: Laurent Pinchart laurent.pinch...@ideasonboard.com
Cc: linux-media@vger.kernel.org
Cc: Mark Fasheh mfas...@suse.com
Cc: Joel Becker jl...@evilplan.org
Cc: ocfs2-de...@oss.oracle.com
Cc: Jan Kara j...@suse.cz
Cc: linux-e...@vger.kernel.org
Cc: Andrew Morton a...@linux-foundation.org
Cc: Andreas Dilger adilger.ker...@dilger.ca
Cc: Theodore Ts'o ty...@mit.edu
Cc: Matthew Wilcox matt...@wil.cx
---
v3: add comment for the last loop, adviced by Jan Kara
v2: simplify memweight(), adviced by Jan Kara
include/linux/string.h |3 +++
lib/string.c | 36
2 files changed, 39 insertions(+), 0 deletions(-)
diff --git a/include/linux/string.h b/include/linux/string.h
index e033564..ffe0442 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -145,4 +145,7 @@ static inline bool strstarts(const char *str, const char
*prefix)
return strncmp(str, prefix, strlen(prefix)) == 0;
}
#endif
+
+extern size_t memweight(const void *ptr, size_t bytes);
+
#endif /* _LINUX_STRING_H_ */
diff --git a/lib/string.c b/lib/string.c
index e5878de..e467186 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -26,6 +26,7 @@
#include linux/export.h
#include linux/bug.h
#include linux/errno.h
+#include linux/bitmap.h
#ifndef __HAVE_ARCH_STRNICMP
/**
@@ -824,3 +825,38 @@ void *memchr_inv(const void *start, int c, size_t bytes)
return check_bytes8(start, value, bytes % 8);
}
EXPORT_SYMBOL(memchr_inv);
+
+/**
+ * memweight - count the total number of bits set in memory area
+ * @ptr: pointer to the start of the area
+ * @bytes: the size of the area
+ */
+size_t memweight(const void *ptr, size_t bytes)
+{
+ size_t w = 0;
+ size_t longs;
+ const unsigned char *bitmap = ptr;
+
+ for (; bytes 0 ((unsigned long)bitmap) % sizeof(long);
+ bytes--, bitmap++)
+ w += hweight8(*bitmap);
+
+ longs = bytes / sizeof(long);
+ if (longs) {
+ BUG_ON(longs = INT_MAX / BITS_PER_LONG);
+ w += bitmap_weight((unsigned long *)bitmap,
+ longs * BITS_PER_LONG);
+ bytes -= longs * sizeof(long);
+ bitmap += longs * sizeof(long);
+ }
+ /*
+* The reason that this last loop is distinct from the preceding
+* bitmap_weight() call is to compute 1-bits in the last region smaller
+* than sizeof(long) properly on big-endian systems.
+*/
+ for (; bytes 0; bytes--, bitmap++)
+ w += hweight8(*bitmap);
+
+ return w;
+}
+EXPORT_SYMBOL(memweight);
--
1.7.7.6
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 5/9] video/uvc: use memweight()
Use memweight() to count the total number of bits set in memory area.
Signed-off-by: Akinobu Mita akinobu.m...@gmail.com
Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
Cc: linux-media@vger.kernel.org
---
No changes from v1
drivers/media/video/uvc/uvc_ctrl.c |5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/media/video/uvc/uvc_ctrl.c
b/drivers/media/video/uvc/uvc_ctrl.c
index af26bbe..f7061a5 100644
--- a/drivers/media/video/uvc/uvc_ctrl.c
+++ b/drivers/media/video/uvc/uvc_ctrl.c
@@ -2083,7 +2083,7 @@ int uvc_ctrl_init_device(struct uvc_device *dev)
/* Walk the entities list and instantiate controls */
list_for_each_entry(entity, dev-entities, list) {
struct uvc_control *ctrl;
- unsigned int bControlSize = 0, ncontrols = 0;
+ unsigned int bControlSize = 0, ncontrols;
__u8 *bmControls = NULL;
if (UVC_ENTITY_TYPE(entity) == UVC_VC_EXTENSION_UNIT) {
@@ -2101,8 +2101,7 @@ int uvc_ctrl_init_device(struct uvc_device *dev)
uvc_ctrl_prune_entity(dev, entity);
/* Count supported controls and allocate the controls array */
- for (i = 0; i bControlSize; ++i)
- ncontrols += hweight8(bmControls[i]);
+ ncontrols = memweight(bmControls, bControlSize);
if (ncontrols == 0)
continue;
--
1.7.7.6
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] dt-bindings: media: xilinx: fix typo in example
Fix typo s/:/;/
Cc: Rob Herring <robh...@kernel.org>
Cc: Hyun Kwon <hyun.k...@xilinx.com>
Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
---
Documentation/devicetree/bindings/media/xilinx/xlnx,v-tpg.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/media/xilinx/xlnx,v-tpg.txt
b/Documentation/devicetree/bindings/media/xilinx/xlnx,v-tpg.txt
index 9dd86b3..439351a 100644
--- a/Documentation/devicetree/bindings/media/xilinx/xlnx,v-tpg.txt
+++ b/Documentation/devicetree/bindings/media/xilinx/xlnx,v-tpg.txt
@@ -66,6 +66,6 @@ Example:
tpg1_out: endpoint {
remote-endpoint = <_in0>;
};
- }:
+ };
};
};
--
2.7.4
[PATCH] media: xilinx-video: fix bad of_node_put() on endpoint error
When iterating through all endpoints using of_graph_get_next_endpoint(),
the refcount of the returned endpoint node is incremented and the refcount
of the node which is passed as previous endpoint is decremented.
So the caller doesn't need to call of_node_put() for each iterated node
except for error exit paths. Otherwise we get "OF: ERROR: Bad
of_node_put() on ..." messages.
Cc: Hyun Kwon <hyun.k...@xilinx.com>
Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
---
drivers/media/platform/xilinx/xilinx-vipp.c | 16
1 file changed, 4 insertions(+), 12 deletions(-)
diff --git a/drivers/media/platform/xilinx/xilinx-vipp.c
b/drivers/media/platform/xilinx/xilinx-vipp.c
index ebfdf33..e5c80c9 100644
--- a/drivers/media/platform/xilinx/xilinx-vipp.c
+++ b/drivers/media/platform/xilinx/xilinx-vipp.c
@@ -76,20 +76,16 @@ static int xvip_graph_build_one(struct
xvip_composite_device *xdev,
struct xvip_graph_entity *ent;
struct v4l2_fwnode_link link;
struct device_node *ep = NULL;
- struct device_node *next;
int ret = 0;
dev_dbg(xdev->dev, "creating links for entity %s\n", local->name);
while (1) {
/* Get the next endpoint and parse its link. */
- next = of_graph_get_next_endpoint(entity->node, ep);
- if (next == NULL)
+ ep = of_graph_get_next_endpoint(entity->node, ep);
+ if (ep == NULL)
break;
- of_node_put(ep);
- ep = next;
-
dev_dbg(xdev->dev, "processing endpoint %pOF\n", ep);
ret = v4l2_fwnode_parse_link(of_fwnode_handle(ep), );
@@ -200,7 +196,6 @@ static int xvip_graph_build_dma(struct
xvip_composite_device *xdev)
struct xvip_graph_entity *ent;
struct v4l2_fwnode_link link;
struct device_node *ep = NULL;
- struct device_node *next;
struct xvip_dma *dma;
int ret = 0;
@@ -208,13 +203,10 @@ static int xvip_graph_build_dma(struct
xvip_composite_device *xdev)
while (1) {
/* Get the next endpoint and parse its link. */
- next = of_graph_get_next_endpoint(node, ep);
- if (next == NULL)
+ ep = of_graph_get_next_endpoint(node, ep);
+ if (ep == NULL)
break;
- of_node_put(ep);
- ep = next;
-
dev_dbg(xdev->dev, "processing endpoint %pOF\n", ep);
ret = v4l2_fwnode_parse_link(of_fwnode_handle(ep), );
--
2.7.4
[PATCH 0/4] media: ov7670: add media controller support
This series adds media controller support and other related changes to the OV7670 which is cheap and highly available CMOS image sensor for hobbyists. This enables to control a video pipeline system with the OV7670. I've tested this with the xilinx video IP pipeline. Akinobu Mita (4): media: ov7670: create subdevice device node media: ov7670: use v4l2_async_unregister_subdev() media: ov7670: add media controller support media: ov7670: add get_fmt() pad ops callback drivers/media/i2c/ov7670.c | 55 +- 1 file changed, 54 insertions(+), 1 deletion(-) Cc: Jonathan Corbet <cor...@lwn.net> Cc: Mauro Carvalho Chehab <mche...@s-opensource.com> -- 2.7.4
[PATCH 1/4] media: ov7670: create subdevice device node
Set the V4L2_SUBDEV_FL_HAS_DEVNODE flag for the subdevice so it is
possible to create a subdevice device node.
Cc: Jonathan Corbet <cor...@lwn.net>
Cc: Mauro Carvalho Chehab <mche...@s-opensource.com>
Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
---
drivers/media/i2c/ov7670.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index e88549f..d3f7d61 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -1585,6 +1585,7 @@ static int ov7670_probe(struct i2c_client *client,
return -ENOMEM;
sd = >sd;
v4l2_i2c_subdev_init(sd, client, _ops);
+ sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
info->clock_speed = 30; /* default: a guess */
if (client->dev.platform_data) {
--
2.7.4
[PATCH 2/4] media: ov7670: use v4l2_async_unregister_subdev()
The sub-device for ov7670 is registered by v4l2_async_register_subdev(). So it should be unregistered by v4l2_async_unregister_subdev() instead of v4l2_device_unregister_subdev(). Cc: Jonathan Corbet <cor...@lwn.net> Cc: Mauro Carvalho Chehab <mche...@s-opensource.com> Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com> --- drivers/media/i2c/ov7670.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c index d3f7d61..4f89a51 100644 --- a/drivers/media/i2c/ov7670.c +++ b/drivers/media/i2c/ov7670.c @@ -19,6 +19,7 @@ #include #include #include +#include #include #include #include @@ -1710,7 +1711,7 @@ static int ov7670_remove(struct i2c_client *client) struct v4l2_subdev *sd = i2c_get_clientdata(client); struct ov7670_info *info = to_state(sd); - v4l2_device_unregister_subdev(sd); + v4l2_async_unregister_subdev(sd); v4l2_ctrl_handler_free(>hdl); clk_disable_unprepare(info->clk); return 0; -- 2.7.4
[PATCH 4/4] media: ov7670: add get_fmt() pad ops callback
This enables to print the current format on the source pad of the ov7670
subdev by media-ctl.
Cc: Jonathan Corbet <cor...@lwn.net>
Cc: Mauro Carvalho Chehab <mche...@s-opensource.com>
Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
---
drivers/media/i2c/ov7670.c | 34 ++
1 file changed, 34 insertions(+)
diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index 38e1876..9fe99c5 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -234,6 +234,7 @@ struct ov7670_info {
struct v4l2_ctrl *hue;
};
struct ov7670_format_struct *fmt; /* Current format */
+ struct ov7670_win_size *wsize; /* Current format */
struct clk *clk;
struct gpio_desc *resetb_gpio;
struct gpio_desc *pwdn_gpio;
@@ -875,6 +876,36 @@ static int ov7670_set_framerate_legacy(struct v4l2_subdev
*sd,
return ov7670_write(sd, REG_CLKRC, info->clkrc);
}
+static int ov7670_get_fmt(struct v4l2_subdev *sd,
+ struct v4l2_subdev_pad_config *cfg,
+ struct v4l2_subdev_format *fmt)
+{
+ struct ov7670_info *info = to_state(sd);
+
+ if (fmt->pad)
+ return -EINVAL;
+
+ if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) {
+#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
+ struct v4l2_mbus_framefmt *mf;
+
+ mf = v4l2_subdev_get_try_format(sd, cfg, 0);
+ fmt->format = *mf;
+ return 0;
+#else
+ return -ENOTTY;
+#endif
+ }
+
+ fmt->format.colorspace = info->fmt->colorspace;
+ fmt->format.code = info->fmt->mbus_code;
+ fmt->format.field = V4L2_FIELD_NONE;
+ fmt->format.width = info->wsize->width;
+ fmt->format.height = info->wsize->height;
+
+ return 0;
+}
+
/*
* Store a set of start/stop values into the camera.
*/
@@ -1026,6 +1057,7 @@ static int ov7670_set_fmt(struct v4l2_subdev *sd,
if (wsize->regs)
ret = ov7670_write_array(sd, wsize->regs);
info->fmt = ovfmt;
+ info->wsize = wsize;
/*
* If we're running RGB565, we must rewrite clkrc after setting
@@ -1529,6 +1561,7 @@ static const struct v4l2_subdev_pad_ops ov7670_pad_ops = {
.enum_frame_interval = ov7670_enum_frame_interval,
.enum_frame_size = ov7670_enum_frame_size,
.enum_mbus_code = ov7670_enum_mbus_code,
+ .get_fmt = ov7670_get_fmt,
.set_fmt = ov7670_set_fmt,
};
@@ -1647,6 +1680,7 @@ static int ov7670_probe(struct i2c_client *client,
info->devtype = _devdata[id->driver_data];
info->fmt = _formats[0];
+ info->wsize = >devtype->win_sizes[0];
info->clkrc = 0;
/* Set default frame rate to 30 fps */
--
2.7.4
[PATCH 3/4] media: ov7670: add media controller support
Create a source pad and set the media controller type to the sensor.
Cc: Jonathan Corbet <cor...@lwn.net>
Cc: Mauro Carvalho Chehab <mche...@s-opensource.com>
Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
---
drivers/media/i2c/ov7670.c | 17 +
1 file changed, 17 insertions(+)
diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index 4f89a51..38e1876 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -214,6 +214,9 @@ struct ov7670_devtype {
struct ov7670_format_struct; /* coming later */
struct ov7670_info {
struct v4l2_subdev sd;
+#ifdef CONFIG_MEDIA_CONTROLLER
+ struct media_pad pad;
+#endif
struct v4l2_ctrl_handler hdl;
struct {
/* gain cluster */
@@ -1654,6 +1657,14 @@ static int ov7670_probe(struct i2c_client *client,
if (info->pclk_hb_disable)
ov7670_write(sd, REG_COM10, COM10_PCLK_HB);
+#ifdef CONFIG_MEDIA_CONTROLLER
+ info->pad.flags = MEDIA_PAD_FL_SOURCE;
+ sd->entity.function = MEDIA_ENT_F_CAM_SENSOR;
+ ret = media_entity_pads_init(>entity, 1, >pad);
+ if (ret)
+ goto clk_disable;
+#endif
+
v4l2_ctrl_handler_init(>hdl, 10);
v4l2_ctrl_new_std(>hdl, _ctrl_ops,
V4L2_CID_BRIGHTNESS, 0, 255, 1, 128);
@@ -1700,6 +1711,9 @@ static int ov7670_probe(struct i2c_client *client,
hdl_free:
v4l2_ctrl_handler_free(>hdl);
+#ifdef CONFIG_MEDIA_CONTROLLER
+ media_entity_cleanup(>entity);
+#endif
clk_disable:
clk_disable_unprepare(info->clk);
return ret;
@@ -1713,6 +1727,9 @@ static int ov7670_remove(struct i2c_client *client)
v4l2_async_unregister_subdev(sd);
v4l2_ctrl_handler_free(>hdl);
+#ifdef CONFIG_MEDIA_CONTROLLER
+ media_entity_cleanup(>entity);
+#endif
clk_disable_unprepare(info->clk);
return 0;
}
--
2.7.4
[PATCH] media: ov7670: add V4L2_CID_TEST_PATTERN control
The ov7670 has the test pattern generator features. This makes use of
it through V4L2_CID_TEST_PATTERN control.
Cc: Jonathan Corbet <cor...@lwn.net>
Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
Cc: Mauro Carvalho Chehab <mche...@s-opensource.com>
Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
---
drivers/media/i2c/ov7670.c | 46 +-
1 file changed, 45 insertions(+), 1 deletion(-)
diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index b61d88e..c6c32f6 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -163,6 +163,11 @@ MODULE_PARM_DESC(debug, "Debug level (0-1)");
#define DBLV_X60x10/* clock x6 */
#define DBLV_X80x11/* clock x8 */
+#define REG_SCALING_XSC0x70/* Test pattern and horizontal scale
factor */
+#define TEST_PATTTERN_0 0x80
+#define REG_SCALING_YSC0x71/* Test pattern and vertical scale
factor */
+#define TEST_PATTTERN_1 0x80
+
#define REG_REG76 0x76/* OV's name */
#define R76_BLKPCOR0x80/* Black pixel correction enable */
#define R76_WHTPCOR0x40/* White pixel correction enable */
@@ -292,7 +297,8 @@ static struct regval_list ov7670_default_regs[] = {
{ REG_COM3, 0 },{ REG_COM14, 0 },
/* Mystery scaling numbers */
- { 0x70, 0x3a }, { 0x71, 0x35 },
+ { REG_SCALING_XSC, 0x3a },
+ { REG_SCALING_YSC, 0x35 },
{ 0x72, 0x11 }, { 0x73, 0xf0 },
{ 0xa2, 0x02 }, { REG_COM10, 0x0 },
@@ -568,6 +574,19 @@ static int ov7670_write(struct v4l2_subdev *sd, unsigned
char reg,
return ov7670_write_i2c(sd, reg, value);
}
+static int ov7670_update_bits(struct v4l2_subdev *sd, unsigned char reg,
+ unsigned char mask, unsigned char value)
+{
+ unsigned char orig;
+ int ret;
+
+ ret = ov7670_read(sd, reg, );
+ if (ret)
+ return ret;
+
+ return ov7670_write(sd, reg, (orig & ~mask) | (value & mask));
+}
+
/*
* Write a list of register settings; ff/ff stops the process.
*/
@@ -1470,6 +1489,25 @@ static int ov7670_s_autoexp(struct v4l2_subdev *sd,
return ret;
}
+static const char * const ov7670_test_pattern_menu[] = {
+ "No test output",
+ "Shifting \"1\"",
+ "8-bar color bar",
+ "Fade to gray color bar",
+};
+
+static int ov7670_s_test_pattern(struct v4l2_subdev *sd, int value)
+{
+ int ret;
+
+ ret = ov7670_update_bits(sd, REG_SCALING_XSC, TEST_PATTTERN_0,
+ value & BIT(0) ? TEST_PATTTERN_0 : 0);
+ if (ret)
+ return ret;
+
+ return ov7670_update_bits(sd, REG_SCALING_YSC, TEST_PATTTERN_1,
+ value & BIT(1) ? TEST_PATTTERN_1 : 0);
+}
static int ov7670_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
{
@@ -1516,6 +1554,8 @@ static int ov7670_s_ctrl(struct v4l2_ctrl *ctrl)
return ov7670_s_exp(sd, info->exposure->val);
}
return ov7670_s_autoexp(sd, ctrl->val);
+ case V4L2_CID_TEST_PATTERN:
+ return ov7670_s_test_pattern(sd, ctrl->val);
}
return -EINVAL;
}
@@ -1770,6 +1810,10 @@ static int ov7670_probe(struct i2c_client *client,
info->auto_exposure = v4l2_ctrl_new_std_menu(>hdl,
_ctrl_ops,
V4L2_CID_EXPOSURE_AUTO, V4L2_EXPOSURE_MANUAL, 0,
V4L2_EXPOSURE_AUTO);
+ v4l2_ctrl_new_std_menu_items(>hdl, _ctrl_ops,
+ V4L2_CID_TEST_PATTERN,
+ ARRAY_SIZE(ov7670_test_pattern_menu) - 1, 0, 0,
+ ov7670_test_pattern_menu);
sd->ctrl_handler = >hdl;
if (info->hdl.error) {
ret = info->hdl.error;
--
2.7.4
[PATCH] media: ov7670: use v4l2_async_unregister_subdev()
The sub-device for ov7670 is registered by v4l2_async_register_subdev(). So it should be unregistered by v4l2_async_unregister_subdev() instead of v4l2_device_unregister_subdev(). Cc: Jonathan Corbet <cor...@lwn.net> Cc: Sakari Ailus <sakari.ai...@linux.intel.com> Cc: Mauro Carvalho Chehab <mche...@s-opensource.com> Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com> --- drivers/media/i2c/ov7670.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c index 950a0ac..b61d88e 100644 --- a/drivers/media/i2c/ov7670.c +++ b/drivers/media/i2c/ov7670.c @@ -1820,7 +1820,7 @@ static int ov7670_remove(struct i2c_client *client) struct v4l2_subdev *sd = i2c_get_clientdata(client); struct ov7670_info *info = to_state(sd); - v4l2_device_unregister_subdev(sd); + v4l2_async_unregister_subdev(sd); v4l2_ctrl_handler_free(>hdl); clk_disable_unprepare(info->clk); #if defined(CONFIG_MEDIA_CONTROLLER) -- 2.7.4
Re: [PATCH] media: ov9650: support VIDIOC_DBG_G/S_REGISTER ioctls
Hi Sakari,
2017-12-19 19:35 GMT+09:00 Sakari Ailus <sakari.ai...@iki.fi>:
> Hi Akinobu,
>
> On Thu, Dec 14, 2017 at 01:00:49AM +0900, Akinobu Mita wrote:
>> This adds support VIDIOC_DBG_G/S_REGISTER ioctls.
>>
>> There are many device control registers contained in the OV9650. So
>> this helps debugging the lower level issues by getting and setting the
>> registers.
>>
>> Cc: Sylwester Nawrocki <sylvester.nawro...@gmail.com>
>> Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
>> Cc: Mauro Carvalho Chehab <mche...@s-opensource.com>
>> Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
>
> Just wondering --- doesn't the I涎 user space interface offer essentially
> the same functionality?
You are right. I thought /dev/i2c-X interface is not allowed for the
i2c device that is currently in use by a driver. But I found that
there is I2C_SLAVE_FORCE ioctl to bypass the check and the i2cget and
i2cset with '-f' option use I2C_SLAVE_FORCE ioctls.
So I can live without the proposed patch.
> See: Documentation/i2c/dev-interface
>
> Cc Hans.
>
>> ---
>> drivers/media/i2c/ov9650.c | 36
>> 1 file changed, 36 insertions(+)
>>
>> diff --git a/drivers/media/i2c/ov9650.c b/drivers/media/i2c/ov9650.c
>> index 69433e1..c6462cf 100644
>> --- a/drivers/media/i2c/ov9650.c
>> +++ b/drivers/media/i2c/ov9650.c
>> @@ -1374,6 +1374,38 @@ static int ov965x_open(struct v4l2_subdev *sd, struct
>> v4l2_subdev_fh *fh)
>> return 0;
>> }
>>
>> +#ifdef CONFIG_VIDEO_ADV_DEBUG
>> +
>> +static int ov965x_g_register(struct v4l2_subdev *sd,
>> + struct v4l2_dbg_register *reg)
>> +{
>> + struct i2c_client *client = v4l2_get_subdevdata(sd);
>> + u8 val = 0;
>> + int ret;
>> +
>> + if (reg->reg > 0xff)
>> + return -EINVAL;
>> +
>> + ret = ov965x_read(client, reg->reg, );
>> + reg->val = val;
>> + reg->size = 1;
>> +
>> + return ret;
>> +}
>> +
>> +static int ov965x_s_register(struct v4l2_subdev *sd,
>> + const struct v4l2_dbg_register *reg)
>> +{
>> + struct i2c_client *client = v4l2_get_subdevdata(sd);
>> +
>> + if (reg->reg > 0xff || reg->val > 0xff)
>> + return -EINVAL;
>> +
>> + return ov965x_write(client, reg->reg, reg->val);
>> +}
>> +
>> +#endif
>> +
>> static const struct v4l2_subdev_pad_ops ov965x_pad_ops = {
>> .enum_mbus_code = ov965x_enum_mbus_code,
>> .enum_frame_size = ov965x_enum_frame_sizes,
>> @@ -1397,6 +1429,10 @@ static const struct v4l2_subdev_core_ops
>> ov965x_core_ops = {
>> .log_status = v4l2_ctrl_subdev_log_status,
>> .subscribe_event = v4l2_ctrl_subdev_subscribe_event,
>> .unsubscribe_event = v4l2_event_subdev_unsubscribe,
>> +#ifdef CONFIG_VIDEO_ADV_DEBUG
>> + .g_register = ov965x_g_register,
>> + .s_register = ov965x_s_register,
>> +#endif
>> };
>>
>> static const struct v4l2_subdev_ops ov965x_subdev_ops = {
>> --
>> 2.7.4
>>
>
> --
> Sakari Ailus
> e-mail: sakari.ai...@iki.fi
[PATCH] media: ov9650: support VIDIOC_DBG_G/S_REGISTER ioctls
This adds support VIDIOC_DBG_G/S_REGISTER ioctls.
There are many device control registers contained in the OV9650. So
this helps debugging the lower level issues by getting and setting the
registers.
Cc: Sylwester Nawrocki <sylvester.nawro...@gmail.com>
Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
Cc: Mauro Carvalho Chehab <mche...@s-opensource.com>
Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
---
drivers/media/i2c/ov9650.c | 36
1 file changed, 36 insertions(+)
diff --git a/drivers/media/i2c/ov9650.c b/drivers/media/i2c/ov9650.c
index 69433e1..c6462cf 100644
--- a/drivers/media/i2c/ov9650.c
+++ b/drivers/media/i2c/ov9650.c
@@ -1374,6 +1374,38 @@ static int ov965x_open(struct v4l2_subdev *sd, struct
v4l2_subdev_fh *fh)
return 0;
}
+#ifdef CONFIG_VIDEO_ADV_DEBUG
+
+static int ov965x_g_register(struct v4l2_subdev *sd,
+struct v4l2_dbg_register *reg)
+{
+ struct i2c_client *client = v4l2_get_subdevdata(sd);
+ u8 val = 0;
+ int ret;
+
+ if (reg->reg > 0xff)
+ return -EINVAL;
+
+ ret = ov965x_read(client, reg->reg, );
+ reg->val = val;
+ reg->size = 1;
+
+ return ret;
+}
+
+static int ov965x_s_register(struct v4l2_subdev *sd,
+const struct v4l2_dbg_register *reg)
+{
+ struct i2c_client *client = v4l2_get_subdevdata(sd);
+
+ if (reg->reg > 0xff || reg->val > 0xff)
+ return -EINVAL;
+
+ return ov965x_write(client, reg->reg, reg->val);
+}
+
+#endif
+
static const struct v4l2_subdev_pad_ops ov965x_pad_ops = {
.enum_mbus_code = ov965x_enum_mbus_code,
.enum_frame_size = ov965x_enum_frame_sizes,
@@ -1397,6 +1429,10 @@ static const struct v4l2_subdev_core_ops ov965x_core_ops
= {
.log_status = v4l2_ctrl_subdev_log_status,
.subscribe_event = v4l2_ctrl_subdev_subscribe_event,
.unsubscribe_event = v4l2_event_subdev_unsubscribe,
+#ifdef CONFIG_VIDEO_ADV_DEBUG
+ .g_register = ov965x_g_register,
+ .s_register = ov965x_s_register,
+#endif
};
static const struct v4l2_subdev_ops ov965x_subdev_ops = {
--
2.7.4
Re: [PATCH] media: ov9650: remove unnecessary terminated entry in menu items array
Hi Sakari,
2017-10-25 19:23 GMT+09:00 Sakari Ailus <sakari.ai...@iki.fi>:
> On Tue, Oct 24, 2017 at 02:30:26AM +0900, Akinobu Mita wrote:
>> The test_pattern_menu[] array has two valid items and a null terminated
>> item. So the control's maximum value which is passed to
>> v4l2_ctrl_new_std_menu_items() should be one. However,
>> 'ARRAY_SIZE(test_pattern_menu) - 1' is actually passed and it's not
>> correct.
>>
>> Fix it by removing unnecessary terminated entry and let the correct
>> control's maximum value be passed to v4l2_ctrl_new_std_menu_items().
>>
>> Cc: Sylwester Nawrocki <sylvester.nawro...@gmail.com>
>> Cc: Mauro Carvalho Chehab <mche...@s-opensource.com>
>> Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
>> ---
>> drivers/media/i2c/ov9650.c | 1 -
>> 1 file changed, 1 deletion(-)
>>
>> diff --git a/drivers/media/i2c/ov9650.c b/drivers/media/i2c/ov9650.c
>> index 6ffb460..69433e1 100644
>> --- a/drivers/media/i2c/ov9650.c
>> +++ b/drivers/media/i2c/ov9650.c
>> @@ -985,7 +985,6 @@ static const struct v4l2_ctrl_ops ov965x_ctrl_ops = {
>> static const char * const test_pattern_menu[] = {
>> "Disabled",
>> "Color bars",
>> - NULL
>
> The number of items in the menu changes; I fixed that while applying the
> patch:
>
> diff --git a/drivers/media/i2c/ov9650.c b/drivers/media/i2c/ov9650.c
> index 69433e1e2533..4f59da1f967b 100644
> --- a/drivers/media/i2c/ov9650.c
> +++ b/drivers/media/i2c/ov9650.c
> @@ -1039,7 +1039,7 @@ static int ov965x_initialize_controls(struct ov965x
> *ov965x)
>V4L2_CID_POWER_LINE_FREQUENCY_50HZ);
>
> v4l2_ctrl_new_std_menu_items(hdl, ops, V4L2_CID_TEST_PATTERN,
> -ARRAY_SIZE(test_pattern_menu) - 1, 0, 0,
> +ARRAY_SIZE(test_pattern_menu), 0, 0,
> test_pattern_menu);
> if (hdl->error) {
> ret = hdl->error;
>
>
> Let me know if you see issues with this.
This change actually causes an issue.
This problem was found when I got the list of available controls for
ov9650 subdev.
$ yavta -l /dev/v4l-subdev0
<...>
--- Image Processing Controls (class 0x009f0001) ---
control 0x009f0903 `Test Pattern' min 0 max 2 step 1 default 0 current 0.
0: Disabled (*)
1: Color bars
Strangely, the max control value is '2'. So I tried to set the control to '2'
for the fun and got a null pointer dereference.
$ yavta -w '0x009f0903 2' --no-query /dev/v4l-subdev0
Device /dev/v4l-subdev0 opened.
Segmentation fault
Then, I found that the root cause is unnecessary terminated entry.
So 'ARRAY_SIZE(test_pattern_menu) - 1' (=1) should be passed to
v4l2_ctrl_new_std_menu_items().
[PATCH 2/4] meida: mt9m111: add media controller support
Create a source pad and set the media controller type to the sensor.
Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
Cc: Mauro Carvalho Chehab <mche...@s-opensource.com>
Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
---
drivers/media/i2c/mt9m111.c | 22 --
1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/drivers/media/i2c/mt9m111.c b/drivers/media/i2c/mt9m111.c
index 4fa10df..e1d5032 100644
--- a/drivers/media/i2c/mt9m111.c
+++ b/drivers/media/i2c/mt9m111.c
@@ -215,6 +215,9 @@ struct mt9m111 {
int power_count;
const struct mt9m111_datafmt *fmt;
int lastpage; /* PageMap cache value */
+#ifdef CONFIG_MEDIA_CONTROLLER
+ struct media_pad pad;
+#endif
};
/* Find a data format by a pixel code */
@@ -971,6 +974,14 @@ static int mt9m111_probe(struct i2c_client *client,
goto out_clkput;
}
+#ifdef CONFIG_MEDIA_CONTROLLER
+ mt9m111->pad.flags = MEDIA_PAD_FL_SOURCE;
+ mt9m111->subdev.entity.function = MEDIA_ENT_F_CAM_SENSOR;
+ ret = media_entity_pads_init(>subdev.entity, 1, >pad);
+ if (ret < 0)
+ goto out_hdlfree;
+#endif
+
/* Second stage probe - when a capture adapter is there */
mt9m111->rect.left = MT9M111_MIN_DARK_COLS;
mt9m111->rect.top = MT9M111_MIN_DARK_ROWS;
@@ -982,16 +993,20 @@ static int mt9m111_probe(struct i2c_client *client,
ret = mt9m111_video_probe(client);
if (ret < 0)
- goto out_hdlfree;
+ goto out_entityclean;
mt9m111->subdev.dev = >dev;
ret = v4l2_async_register_subdev(>subdev);
if (ret < 0)
- goto out_hdlfree;
+ goto out_entityclean;
return 0;
+out_entityclean:
+#ifdef CONFIG_MEDIA_CONTROLLER
+ media_entity_cleanup(>subdev.entity);
out_hdlfree:
+#endif
v4l2_ctrl_handler_free(>hdl);
out_clkput:
v4l2_clk_put(mt9m111->clk);
@@ -1004,6 +1019,9 @@ static int mt9m111_remove(struct i2c_client *client)
struct mt9m111 *mt9m111 = to_mt9m111(client);
v4l2_async_unregister_subdev(>subdev);
+#ifdef CONFIG_MEDIA_CONTROLLER
+ media_entity_cleanup(>subdev.entity);
+#endif
v4l2_clk_put(mt9m111->clk);
v4l2_ctrl_handler_free(>hdl);
--
2.7.4
[PATCH 4/4] meida: mt9m111: add V4L2_CID_TEST_PATTERN control
The mt9m111 has the test pattern generator features. This makes use of
it through V4L2_CID_TEST_PATTERN control.
Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
Cc: Mauro Carvalho Chehab <mche...@s-opensource.com>
Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
---
drivers/media/i2c/mt9m111.c | 27 +++
1 file changed, 27 insertions(+)
diff --git a/drivers/media/i2c/mt9m111.c b/drivers/media/i2c/mt9m111.c
index e1d5032..d74f254 100644
--- a/drivers/media/i2c/mt9m111.c
+++ b/drivers/media/i2c/mt9m111.c
@@ -92,6 +92,7 @@
*/
#define MT9M111_OPER_MODE_CTRL 0x106
#define MT9M111_OUTPUT_FORMAT_CTRL 0x108
+#define MT9M111_TPG_CTRL 0x148
#define MT9M111_REDUCER_XZOOM_B0x1a0
#define MT9M111_REDUCER_XSIZE_B0x1a1
#define MT9M111_REDUCER_YZOOM_B0x1a3
@@ -124,6 +125,7 @@
#define MT9M111_OUTFMT_AVG_CHROMA (1 << 2)
#define MT9M111_OUTFMT_SWAP_YCbCr_C_Y_RGB_EVEN (1 << 1)
#define MT9M111_OUTFMT_SWAP_YCbCr_Cb_Cr_RGB_R_B(1 << 0)
+#define MT9M111_TPG_SEL_MASK GENMASK(2, 0)
/*
* Camera control register addresses (0x200..0x2ff not implemented)
@@ -706,6 +708,25 @@ static int mt9m111_set_autowhitebalance(struct mt9m111
*mt9m111, int on)
return reg_clear(OPER_MODE_CTRL, MT9M111_OPMODE_AUTOWHITEBAL_EN);
}
+static const char * const mt9m111_test_pattern_menu[] = {
+ "Disabled",
+ "Vertical monochrome gradient",
+ "Flat color type 1",
+ "Flat color type 2",
+ "Flat color type 3",
+ "Flat color type 4",
+ "Flat color type 5",
+ "Color bar",
+};
+
+static int mt9m111_set_test_pattern(struct mt9m111 *mt9m111, int val)
+{
+ struct i2c_client *client = v4l2_get_subdevdata(>subdev);
+
+ return mt9m111_reg_mask(client, MT9M111_TPG_CTRL, val,
+ MT9M111_TPG_SEL_MASK);
+}
+
static int mt9m111_s_ctrl(struct v4l2_ctrl *ctrl)
{
struct mt9m111 *mt9m111 = container_of(ctrl->handler,
@@ -724,6 +745,8 @@ static int mt9m111_s_ctrl(struct v4l2_ctrl *ctrl)
return mt9m111_set_autoexposure(mt9m111, ctrl->val);
case V4L2_CID_AUTO_WHITE_BALANCE:
return mt9m111_set_autowhitebalance(mt9m111, ctrl->val);
+ case V4L2_CID_TEST_PATTERN:
+ return mt9m111_set_test_pattern(mt9m111, ctrl->val);
}
return -EINVAL;
@@ -968,6 +991,10 @@ static int mt9m111_probe(struct i2c_client *client,
v4l2_ctrl_new_std_menu(>hdl,
_ctrl_ops, V4L2_CID_EXPOSURE_AUTO, 1, 0,
V4L2_EXPOSURE_AUTO);
+ v4l2_ctrl_new_std_menu_items(>hdl,
+ _ctrl_ops, V4L2_CID_TEST_PATTERN,
+ ARRAY_SIZE(mt9m111_test_pattern_menu) - 1, 0, 0,
+ mt9m111_test_pattern_menu);
mt9m111->subdev.ctrl_handler = >hdl;
if (mt9m111->hdl.error) {
ret = mt9m111->hdl.error;
--
2.7.4
[PATCH 0/4] media: mt9m111: media controller support and misc changes
This series adds media controller support and other miscellaneous changes to the mt9m111 driver. The MT9M111 camera modules are easily available for the hobbyists. Akinobu Mita (4): meida: mt9m111: create subdevice device node meida: mt9m111: add media controller support meida: mt9m111: document missing required clocks property meida: mt9m111: add V4L2_CID_TEST_PATTERN control .../devicetree/bindings/media/i2c/mt9m111.txt | 4 ++ drivers/media/i2c/mt9m111.c| 51 +- 2 files changed, 53 insertions(+), 2 deletions(-) Cc: Rob Herring <r...@kernel.org> Cc: Sakari Ailus <sakari.ai...@linux.intel.com> Cc: Mauro Carvalho Chehab <mche...@s-opensource.com> -- 2.7.4
[PATCH 1/4] meida: mt9m111: create subdevice device node
Set the V4L2_SUBDEV_FL_HAS_DEVNODE flag for the subdevice so that the subdevice device node is created. Cc: Sakari Ailus <sakari.ai...@linux.intel.com> Cc: Mauro Carvalho Chehab <mche...@s-opensource.com> Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com> --- drivers/media/i2c/mt9m111.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/media/i2c/mt9m111.c b/drivers/media/i2c/mt9m111.c index b1665d9..4fa10df 100644 --- a/drivers/media/i2c/mt9m111.c +++ b/drivers/media/i2c/mt9m111.c @@ -951,6 +951,8 @@ static int mt9m111_probe(struct i2c_client *client, mt9m111->ctx = _b; v4l2_i2c_subdev_init(>subdev, client, _subdev_ops); + mt9m111->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; + v4l2_ctrl_handler_init(>hdl, 5); v4l2_ctrl_new_std(>hdl, _ctrl_ops, V4L2_CID_VFLIP, 0, 1, 1, 0); -- 2.7.4
[PATCH 3/4] meida: mt9m111: document missing required clocks property
The mt9m111 driver requires clocks property for the master clock to the
sensor, but there is no description for that. This adds it.
Cc: Rob Herring <r...@kernel.org>
Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
Cc: Mauro Carvalho Chehab <mche...@s-opensource.com>
Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
---
Documentation/devicetree/bindings/media/i2c/mt9m111.txt | 4
1 file changed, 4 insertions(+)
diff --git a/Documentation/devicetree/bindings/media/i2c/mt9m111.txt
b/Documentation/devicetree/bindings/media/i2c/mt9m111.txt
index ed5a334..ffb57d1 100644
--- a/Documentation/devicetree/bindings/media/i2c/mt9m111.txt
+++ b/Documentation/devicetree/bindings/media/i2c/mt9m111.txt
@@ -6,6 +6,8 @@ interface.
Required Properties:
- compatible: value should be "micron,mt9m111"
+- clocks: reference to the master clock.
+- clock-names: should be "mclk".
For further reading on port node refer to
Documentation/devicetree/bindings/media/video-interfaces.txt.
@@ -16,6 +18,8 @@ Example:
mt9m111@5d {
compatible = "micron,mt9m111";
reg = <0x5d>;
+ clocks = <>;
+ clock-names = "mclk";
remote = <_camera>;
port {
--
2.7.4
Re: [PATCH v4 10/14] media: ov772x: reconstruct s_frame_interval()
2018-05-04 5:29 GMT+09:00 jacopo mondi <jac...@jmondi.org>:
> Hi Akinobu,
>thank you for the patch
>
> On Mon, Apr 30, 2018 at 02:13:09AM +0900, Akinobu Mita wrote:
>> This splits the s_frame_interval() in subdev video ops into selecting the
>> frame interval and setting up the registers.
>>
>> This is a preparatory change to avoid accessing registers under power
>> saving mode.
>>
>> Cc: Jacopo Mondi <jacopo+rene...@jmondi.org>
>> Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
>> Cc: Hans Verkuil <hans.verk...@cisco.com>
>> Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
>> Cc: Mauro Carvalho Chehab <mche...@s-opensource.com>
>> Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
>> ---
>> * v4
>> - No changes
>>
>> drivers/media/i2c/ov772x.c | 56
>> +-
>> 1 file changed, 35 insertions(+), 21 deletions(-)
>>
>> diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
>> index edc013d..7ea157e 100644
>> --- a/drivers/media/i2c/ov772x.c
>> +++ b/drivers/media/i2c/ov772x.c
>> @@ -617,25 +617,16 @@ static int ov772x_s_stream(struct v4l2_subdev *sd, int
>> enable)
>> return 0;
>> }
>>
>> -static int ov772x_set_frame_rate(struct ov772x_priv *priv,
>> - struct v4l2_fract *tpf,
>> - const struct ov772x_color_format *cfmt,
>> - const struct ov772x_win_size *win)
>> +static unsigned int ov772x_select_fps(struct ov772x_priv *priv,
>> + struct v4l2_fract *tpf)
>
> Please align to the open brace, as all the other functions in the
> driver.
OK.
> Also, is it worth making a function out of this? It is called from one
> place only... see below.
>
>> {
>> - struct i2c_client *client = v4l2_get_subdevdata(>subdev);
>> - unsigned long fin = clk_get_rate(priv->clk);
>> unsigned int fps = tpf->numerator ?
>> tpf->denominator / tpf->numerator :
>> tpf->denominator;
>> unsigned int best_diff;
>> - unsigned int fsize;
>> - unsigned int pclk;
>> unsigned int diff;
>> unsigned int idx;
>> unsigned int i;
>> - u8 clkrc = 0;
>> - u8 com4 = 0;
>> - int ret;
>>
>> /* Approximate to the closest supported frame interval. */
>> best_diff = ~0L;
>> @@ -646,7 +637,25 @@ static int ov772x_set_frame_rate(struct ov772x_priv
>> *priv,
>> best_diff = diff;
>> }
>> }
>> - fps = ov772x_frame_intervals[idx];
>> +
>> + return ov772x_frame_intervals[idx];
>> +}
>> +
>> +static int ov772x_set_frame_rate(struct ov772x_priv *priv,
>> + unsigned int fps,
>> + const struct ov772x_color_format *cfmt,
>> + const struct ov772x_win_size *win)
>> +{
>> + struct i2c_client *client = v4l2_get_subdevdata(>subdev);
>> + unsigned long fin = clk_get_rate(priv->clk);
>> + unsigned int fsize;
>> + unsigned int pclk;
>> + unsigned int best_diff;
>
> Please keep variable declarations sorted as in other functions in the
> driver.
OK.
>> + unsigned int diff;
>> + unsigned int i;
>> + u8 clkrc = 0;
>> + u8 com4 = 0;
>> + int ret;
>>
>> /* Use image size (with blankings) to calculate desired pixel clock. */
>> switch (cfmt->com7 & OFMT_MASK) {
>> @@ -711,10 +720,6 @@ static int ov772x_set_frame_rate(struct ov772x_priv
>> *priv,
>> if (ret < 0)
>> return ret;
>>
>> - tpf->numerator = 1;
>> - tpf->denominator = fps;
>> - priv->fps = tpf->denominator;
>> -
>> return 0;
>> }
>>
>> @@ -735,8 +740,20 @@ static int ov772x_s_frame_interval(struct v4l2_subdev
>> *sd,
>> {
>> struct ov772x_priv *priv = to_ov772x(sd);
>> struct v4l2_fract *tpf = >interval;
>> + unsigned int fps;
>> + int ret;
>> +
>> + fps = ov772x_select_fps(priv, tpf);
>
> That's the only caller of this function. I'm fine if you want to keep
> it as it is though.
I would like to keep ov772x_select_fps() because the function name is
self descriptive and imples it doesn't change HW registers. So I
[PATCH v5 10/14] media: ov772x: reconstruct s_frame_interval()
This splits the s_frame_interval() in subdev video ops into selecting the
frame interval and setting up the registers.
This is a preparatory change to avoid accessing registers under power
saving mode.
Cc: Jacopo Mondi <jacopo+rene...@jmondi.org>
Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
Cc: Hans Verkuil <hans.verk...@cisco.com>
Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
Cc: Mauro Carvalho Chehab <mche...@s-opensource.com>
Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
---
* v5
- Align arguments to open parenthesis
- Sort variable declarations
drivers/media/i2c/ov772x.c | 56 +-
1 file changed, 35 insertions(+), 21 deletions(-)
diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
index 6c0c792..92ad13f 100644
--- a/drivers/media/i2c/ov772x.c
+++ b/drivers/media/i2c/ov772x.c
@@ -617,25 +617,16 @@ static int ov772x_s_stream(struct v4l2_subdev *sd, int
enable)
return 0;
}
-static int ov772x_set_frame_rate(struct ov772x_priv *priv,
-struct v4l2_fract *tpf,
-const struct ov772x_color_format *cfmt,
-const struct ov772x_win_size *win)
+static unsigned int ov772x_select_fps(struct ov772x_priv *priv,
+ struct v4l2_fract *tpf)
{
- struct i2c_client *client = v4l2_get_subdevdata(>subdev);
- unsigned long fin = clk_get_rate(priv->clk);
unsigned int fps = tpf->numerator ?
tpf->denominator / tpf->numerator :
tpf->denominator;
unsigned int best_diff;
- unsigned int fsize;
- unsigned int pclk;
unsigned int diff;
unsigned int idx;
unsigned int i;
- u8 clkrc = 0;
- u8 com4 = 0;
- int ret;
/* Approximate to the closest supported frame interval. */
best_diff = ~0L;
@@ -646,7 +637,25 @@ static int ov772x_set_frame_rate(struct ov772x_priv *priv,
best_diff = diff;
}
}
- fps = ov772x_frame_intervals[idx];
+
+ return ov772x_frame_intervals[idx];
+}
+
+static int ov772x_set_frame_rate(struct ov772x_priv *priv,
+unsigned int fps,
+const struct ov772x_color_format *cfmt,
+const struct ov772x_win_size *win)
+{
+ struct i2c_client *client = v4l2_get_subdevdata(>subdev);
+ unsigned long fin = clk_get_rate(priv->clk);
+ unsigned int best_diff;
+ unsigned int fsize;
+ unsigned int pclk;
+ unsigned int diff;
+ unsigned int i;
+ u8 clkrc = 0;
+ u8 com4 = 0;
+ int ret;
/* Use image size (with blankings) to calculate desired pixel clock. */
switch (cfmt->com7 & OFMT_MASK) {
@@ -711,10 +720,6 @@ static int ov772x_set_frame_rate(struct ov772x_priv *priv,
if (ret < 0)
return ret;
- tpf->numerator = 1;
- tpf->denominator = fps;
- priv->fps = tpf->denominator;
-
return 0;
}
@@ -735,8 +740,20 @@ static int ov772x_s_frame_interval(struct v4l2_subdev *sd,
{
struct ov772x_priv *priv = to_ov772x(sd);
struct v4l2_fract *tpf = >interval;
+ unsigned int fps;
+ int ret;
+
+ fps = ov772x_select_fps(priv, tpf);
+
+ ret = ov772x_set_frame_rate(priv, fps, priv->cfmt, priv->win);
+ if (ret)
+ return ret;
- return ov772x_set_frame_rate(priv, tpf, priv->cfmt, priv->win);
+ tpf->numerator = 1;
+ tpf->denominator = fps;
+ priv->fps = fps;
+
+ return 0;
}
static int ov772x_s_ctrl(struct v4l2_ctrl *ctrl)
@@ -993,7 +1010,6 @@ static int ov772x_set_params(struct ov772x_priv *priv,
const struct ov772x_win_size *win)
{
struct i2c_client *client = v4l2_get_subdevdata(>subdev);
- struct v4l2_fract tpf;
int ret;
u8 val;
@@ -1075,9 +1091,7 @@ static int ov772x_set_params(struct ov772x_priv *priv,
goto ov772x_set_fmt_error;
/* COM4, CLKRC: Set pixel clock and framerate. */
- tpf.numerator = 1;
- tpf.denominator = priv->fps;
- ret = ov772x_set_frame_rate(priv, , cfmt, win);
+ ret = ov772x_set_frame_rate(priv, priv->fps, cfmt, win);
if (ret < 0)
goto ov772x_set_fmt_error;
--
2.7.4
[PATCH v5 08/14] media: ov772x: support device tree probing
The ov772x driver currently only supports legacy platform data probe.
This change enables device tree probing.
Note that the platform data probe can select auto or manual edge control
mode, but the device tree probling can only select auto edge control mode
for now.
Cc: Jacopo Mondi <jacopo+rene...@jmondi.org>
Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
Cc: Hans Verkuil <hans.verk...@cisco.com>
Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
Cc: Mauro Carvalho Chehab <mche...@s-opensource.com>
Reviewed-by: Jacopo Mondi <jac...@jmondi.org>
Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
---
* v5
- Remove unnecessary space
drivers/media/i2c/ov772x.c | 64 --
1 file changed, 45 insertions(+), 19 deletions(-)
diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
index f939e28..2b02411 100644
--- a/drivers/media/i2c/ov772x.c
+++ b/drivers/media/i2c/ov772x.c
@@ -749,13 +749,13 @@ static int ov772x_s_ctrl(struct v4l2_ctrl *ctrl)
case V4L2_CID_VFLIP:
val = ctrl->val ? VFLIP_IMG : 0x00;
priv->flag_vflip = ctrl->val;
- if (priv->info->flags & OV772X_FLAG_VFLIP)
+ if (priv->info && (priv->info->flags & OV772X_FLAG_VFLIP))
val ^= VFLIP_IMG;
return ov772x_mask_set(client, COM3, VFLIP_IMG, val);
case V4L2_CID_HFLIP:
val = ctrl->val ? HFLIP_IMG : 0x00;
priv->flag_hflip = ctrl->val;
- if (priv->info->flags & OV772X_FLAG_HFLIP)
+ if (priv->info && (priv->info->flags & OV772X_FLAG_HFLIP))
val ^= HFLIP_IMG;
return ov772x_mask_set(client, COM3, HFLIP_IMG, val);
case V4L2_CID_BAND_STOP_FILTER:
@@ -914,19 +914,14 @@ static void ov772x_select_params(const struct
v4l2_mbus_framefmt *mf,
*win = ov772x_select_win(mf->width, mf->height);
}
-static int ov772x_set_params(struct ov772x_priv *priv,
-const struct ov772x_color_format *cfmt,
-const struct ov772x_win_size *win)
+static int ov772x_edgectrl(struct ov772x_priv *priv)
{
struct i2c_client *client = v4l2_get_subdevdata(>subdev);
- struct v4l2_fract tpf;
int ret;
- u8 val;
- /* Reset hardware. */
- ov772x_reset(client);
+ if (!priv->info)
+ return 0;
- /* Edge Ctrl. */
if (priv->info->edgectrl.strength & OV772X_MANUAL_EDGE_CTRL) {
/*
* Manual Edge Control Mode.
@@ -937,19 +932,19 @@ static int ov772x_set_params(struct ov772x_priv *priv,
ret = ov772x_mask_set(client, DSPAUTO, EDGE_ACTRL, 0x00);
if (ret < 0)
- goto ov772x_set_fmt_error;
+ return ret;
ret = ov772x_mask_set(client,
EDGE_TRSHLD, OV772X_EDGE_THRESHOLD_MASK,
priv->info->edgectrl.threshold);
if (ret < 0)
- goto ov772x_set_fmt_error;
+ return ret;
ret = ov772x_mask_set(client,
EDGE_STRNGT, OV772X_EDGE_STRENGTH_MASK,
priv->info->edgectrl.strength);
if (ret < 0)
- goto ov772x_set_fmt_error;
+ return ret;
} else if (priv->info->edgectrl.upper > priv->info->edgectrl.lower) {
/*
@@ -961,15 +956,35 @@ static int ov772x_set_params(struct ov772x_priv *priv,
EDGE_UPPER, OV772X_EDGE_UPPER_MASK,
priv->info->edgectrl.upper);
if (ret < 0)
- goto ov772x_set_fmt_error;
+ return ret;
ret = ov772x_mask_set(client,
EDGE_LOWER, OV772X_EDGE_LOWER_MASK,
priv->info->edgectrl.lower);
if (ret < 0)
- goto ov772x_set_fmt_error;
+ return ret;
}
+ return 0;
+}
+
+static int ov772x_set_params(struct ov772x_priv *priv,
+const struct ov772x_color_format *cfmt,
+const struct ov772x_win_size *win)
+{
+ struct i2c_client *client = v4l2_get_subdevdata(>subdev);
+ struct v4l2_fract tpf;
+ int ret;
+ u8 val;
+
+ /* Reset hardware. */
+ ov772x_reset(client);
+
+ /* Edge Ctrl. */
+ ret = ov772x_edgectrl(priv);
+ if (ret < 0)
+ return ret;
+
/* Fo
[PATCH v5 11/14] media: ov772x: use v4l2_ctrl to get current control value
The ov772x driver provides three V4L2 controls and the current value of
each control is saved as a variable in the private data structure.
We don't need to keep track of the current value by ourself, if we use
v4l2_ctrl returned from v4l2_ctrl_new_std() instead.
This is a preparatory change to avoid accessing registers under power
saving mode. This simplifies s_ctrl() by making it just return without
saving the current control value in private area when it is called under
power saving mode.
Cc: Jacopo Mondi <jacopo+rene...@jmondi.org>
Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
Cc: Hans Verkuil <hans.verk...@cisco.com>
Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
Cc: Mauro Carvalho Chehab <mche...@s-opensource.com>
Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
---
* v5
- No changes
drivers/media/i2c/ov772x.c | 34 +-
1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
index 92ad13f..9292a18 100644
--- a/drivers/media/i2c/ov772x.c
+++ b/drivers/media/i2c/ov772x.c
@@ -419,10 +419,10 @@ struct ov772x_priv {
struct gpio_desc *rstb_gpio;
const struct ov772x_color_format *cfmt;
const struct ov772x_win_size *win;
- unsigned shortflag_vflip:1;
- unsigned shortflag_hflip:1;
+ struct v4l2_ctrl *vflip_ctrl;
+ struct v4l2_ctrl *hflip_ctrl;
/* band_filter = COM8[5] ? 256 - BDBASE : 0 */
- unsigned shortband_filter;
+ struct v4l2_ctrl *band_filter_ctrl;
unsigned int fps;
/* lock to protect power_count */
struct mutex lock;
@@ -768,13 +768,11 @@ static int ov772x_s_ctrl(struct v4l2_ctrl *ctrl)
switch (ctrl->id) {
case V4L2_CID_VFLIP:
val = ctrl->val ? VFLIP_IMG : 0x00;
- priv->flag_vflip = ctrl->val;
if (priv->info && (priv->info->flags & OV772X_FLAG_VFLIP))
val ^= VFLIP_IMG;
return ov772x_mask_set(client, COM3, VFLIP_IMG, val);
case V4L2_CID_HFLIP:
val = ctrl->val ? HFLIP_IMG : 0x00;
- priv->flag_hflip = ctrl->val;
if (priv->info && (priv->info->flags & OV772X_FLAG_HFLIP))
val ^= HFLIP_IMG;
return ov772x_mask_set(client, COM3, HFLIP_IMG, val);
@@ -794,8 +792,7 @@ static int ov772x_s_ctrl(struct v4l2_ctrl *ctrl)
ret = ov772x_mask_set(client, BDBASE,
0xff, val);
}
- if (!ret)
- priv->band_filter = ctrl->val;
+
return ret;
}
@@ -1075,9 +1072,9 @@ static int ov772x_set_params(struct ov772x_priv *priv,
val |= VFLIP_IMG;
if (priv->info && (priv->info->flags & OV772X_FLAG_HFLIP))
val |= HFLIP_IMG;
- if (priv->flag_vflip)
+ if (priv->vflip_ctrl->val)
val ^= VFLIP_IMG;
- if (priv->flag_hflip)
+ if (priv->hflip_ctrl->val)
val ^= HFLIP_IMG;
ret = ov772x_mask_set(client,
@@ -1096,11 +1093,13 @@ static int ov772x_set_params(struct ov772x_priv *priv,
goto ov772x_set_fmt_error;
/* Set COM8. */
- if (priv->band_filter) {
+ if (priv->band_filter_ctrl->val) {
+ unsigned short band_filter = priv->band_filter_ctrl->val;
+
ret = ov772x_mask_set(client, COM8, BNDF_ON_OFF, BNDF_ON_OFF);
if (!ret)
ret = ov772x_mask_set(client, BDBASE,
- 0xff, 256 - priv->band_filter);
+ 0xff, 256 - band_filter);
if (ret < 0)
goto ov772x_set_fmt_error;
}
@@ -1341,12 +1340,13 @@ static int ov772x_probe(struct i2c_client *client,
v4l2_i2c_subdev_init(>subdev, client, _subdev_ops);
v4l2_ctrl_handler_init(>hdl, 3);
- v4l2_ctrl_new_std(>hdl, _ctrl_ops,
- V4L2_CID_VFLIP, 0, 1, 1, 0);
- v4l2_ctrl_new_std(>hdl, _ctrl_ops,
- V4L2_CID_HFLIP, 0, 1, 1, 0);
- v4l2_ctrl_new_std(>hdl, _ctrl_ops,
- V4L2_CID_BAND_STOP_FILTER, 0, 256, 1, 0);
+ priv->vflip_ctrl = v4l2_ctrl_new_std(>hdl, _ctrl_ops,
+V4L2_CID_VFLIP, 0, 1, 1, 0);
+ priv->hflip_ctrl = v4l2_ctrl_new_std(>hdl, _ctrl_ops,
+
[PATCH v5 09/14] media: ov772x: handle nested s_power() calls
Depending on the v4l2 driver, calling s_power() could be nested. So the
actual transitions between power saving mode and normal operation mode
should only happen at the first power on and the last power off.
This adds an s_power() nesting counter and updates the power state if the
counter is modified from 0 to != 0 or from != 0 to 0.
Cc: Jacopo Mondi <jacopo+rene...@jmondi.org>
Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
Cc: Hans Verkuil <hans.verk...@cisco.com>
Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
Cc: Mauro Carvalho Chehab <mche...@s-opensource.com>
Reviewed-by: Jacopo Mondi <jac...@jmondi.org>
Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
---
* v5
- No changes
drivers/media/i2c/ov772x.c | 34 ++
1 file changed, 30 insertions(+), 4 deletions(-)
diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
index 2b02411..6c0c792 100644
--- a/drivers/media/i2c/ov772x.c
+++ b/drivers/media/i2c/ov772x.c
@@ -424,6 +424,9 @@ struct ov772x_priv {
/* band_filter = COM8[5] ? 256 - BDBASE : 0 */
unsigned shortband_filter;
unsigned int fps;
+ /* lock to protect power_count */
+ struct mutex lock;
+ int power_count;
#ifdef CONFIG_MEDIA_CONTROLLER
struct media_pad pad;
#endif
@@ -871,9 +874,26 @@ static int ov772x_power_off(struct ov772x_priv *priv)
static int ov772x_s_power(struct v4l2_subdev *sd, int on)
{
struct ov772x_priv *priv = to_ov772x(sd);
+ int ret = 0;
+
+ mutex_lock(>lock);
+
+ /* If the power count is modified from 0 to != 0 or from != 0 to 0,
+* update the power state.
+*/
+ if (priv->power_count == !on)
+ ret = on ? ov772x_power_on(priv) : ov772x_power_off(priv);
+
+ if (!ret) {
+ /* Update the power count. */
+ priv->power_count += on ? 1 : -1;
+ WARN(priv->power_count < 0, "Unbalanced power count\n");
+ WARN(priv->power_count > 1, "Duplicated s_power call\n");
+ }
+
+ mutex_unlock(>lock);
- return on ? ov772x_power_on(priv) :
- ov772x_power_off(priv);
+ return ret;
}
static const struct ov772x_win_size *ov772x_select_win(u32 width, u32 height)
@@ -1303,6 +1323,7 @@ static int ov772x_probe(struct i2c_client *client,
return -ENOMEM;
priv->info = client->dev.platform_data;
+ mutex_init(>lock);
v4l2_i2c_subdev_init(>subdev, client, _subdev_ops);
v4l2_ctrl_handler_init(>hdl, 3);
@@ -1313,8 +1334,10 @@ static int ov772x_probe(struct i2c_client *client,
v4l2_ctrl_new_std(>hdl, _ctrl_ops,
V4L2_CID_BAND_STOP_FILTER, 0, 256, 1, 0);
priv->subdev.ctrl_handler = >hdl;
- if (priv->hdl.error)
- return priv->hdl.error;
+ if (priv->hdl.error) {
+ ret = priv->hdl.error;
+ goto error_mutex_destroy;
+ }
priv->clk = clk_get(>dev, NULL);
if (IS_ERR(priv->clk)) {
@@ -1362,6 +1385,8 @@ static int ov772x_probe(struct i2c_client *client,
clk_put(priv->clk);
error_ctrl_free:
v4l2_ctrl_handler_free(>hdl);
+error_mutex_destroy:
+ mutex_destroy(>lock);
return ret;
}
@@ -1376,6 +1401,7 @@ static int ov772x_remove(struct i2c_client *client)
gpiod_put(priv->pwdn_gpio);
v4l2_async_unregister_subdev(>subdev);
v4l2_ctrl_handler_free(>hdl);
+ mutex_destroy(>lock);
return 0;
}
--
2.7.4
[PATCH v5 13/14] media: ov772x: make set_fmt() and s_frame_interval() return -EBUSY while streaming
The ov772x driver is going to offer a V4L2 sub-device interface, so
changing the output data format and the frame interval on this sub-device
can be made anytime. However, these requests are preferred to fail while
the video stream on the device is active.
Cc: Jacopo Mondi <jacopo+rene...@jmondi.org>
Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
Cc: Hans Verkuil <hans.verk...@cisco.com>
Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
Cc: Mauro Carvalho Chehab <mche...@s-opensource.com>
Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
---
* v5:
- Make s_frame_interval() return -EBUSY while streaming
drivers/media/i2c/ov772x.c | 43 +--
1 file changed, 33 insertions(+), 10 deletions(-)
diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
index 262a7e5..4b479f9 100644
--- a/drivers/media/i2c/ov772x.c
+++ b/drivers/media/i2c/ov772x.c
@@ -424,9 +424,10 @@ struct ov772x_priv {
/* band_filter = COM8[5] ? 256 - BDBASE : 0 */
struct v4l2_ctrl *band_filter_ctrl;
unsigned int fps;
- /* lock to protect power_count */
+ /* lock to protect power_count and streaming */
struct mutex lock;
int power_count;
+ int streaming;
#ifdef CONFIG_MEDIA_CONTROLLER
struct media_pad pad;
#endif
@@ -603,18 +604,28 @@ static int ov772x_s_stream(struct v4l2_subdev *sd, int
enable)
{
struct i2c_client *client = v4l2_get_subdevdata(sd);
struct ov772x_priv *priv = to_ov772x(sd);
+ int ret = 0;
- if (!enable) {
- ov772x_mask_set(client, COM2, SOFT_SLEEP_MODE, SOFT_SLEEP_MODE);
- return 0;
- }
+ mutex_lock(>lock);
- ov772x_mask_set(client, COM2, SOFT_SLEEP_MODE, 0);
+ if (priv->streaming == enable)
+ goto done;
- dev_dbg(>dev, "format %d, win %s\n",
- priv->cfmt->code, priv->win->name);
+ ret = ov772x_mask_set(client, COM2, SOFT_SLEEP_MODE,
+ enable ? 0 : SOFT_SLEEP_MODE);
+ if (ret)
+ goto done;
- return 0;
+ if (enable) {
+ dev_dbg(>dev, "format %d, win %s\n",
+ priv->cfmt->code, priv->win->name);
+ }
+ priv->streaming = enable;
+
+done:
+ mutex_unlock(>lock);
+
+ return ret;
}
static unsigned int ov772x_select_fps(struct ov772x_priv *priv,
@@ -743,9 +754,15 @@ static int ov772x_s_frame_interval(struct v4l2_subdev *sd,
unsigned int fps;
int ret = 0;
+ mutex_lock(>lock);
+
+ if (priv->streaming) {
+ ret = -EBUSY;
+ goto error;
+ }
+
fps = ov772x_select_fps(priv, tpf);
- mutex_lock(>lock);
/*
* If the device is not powered up by the host driver do
* not apply any changes to H/W at this time. Instead
@@ -1222,6 +1239,12 @@ static int ov772x_set_fmt(struct v4l2_subdev *sd,
}
mutex_lock(>lock);
+
+ if (priv->streaming) {
+ ret = -EBUSY;
+ goto error;
+ }
+
/*
* If the device is not powered up by the host driver do
* not apply any changes to H/W at this time. Instead
--
2.7.4
[PATCH v5 12/14] media: ov772x: avoid accessing registers under power saving mode
The set_fmt() in subdev pad ops, the s_ctrl() for subdev control handler,
and the s_frame_interval() in subdev video ops could be called when the
device is under power saving mode. These callbacks for ov772x driver
cause updating H/W registers that will fail under power saving mode.
This avoids it by not apply any changes to H/W if the device is not powered
up. Instead the changes will be restored right after power-up.
Cc: Jacopo Mondi <jacopo+rene...@jmondi.org>
Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
Cc: Hans Verkuil <hans.verk...@cisco.com>
Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
Cc: Mauro Carvalho Chehab <mche...@s-opensource.com>
Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
---
* v5
- No changes
drivers/media/i2c/ov772x.c | 79 +-
1 file changed, 64 insertions(+), 15 deletions(-)
diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
index 9292a18..262a7e5 100644
--- a/drivers/media/i2c/ov772x.c
+++ b/drivers/media/i2c/ov772x.c
@@ -741,19 +741,30 @@ static int ov772x_s_frame_interval(struct v4l2_subdev *sd,
struct ov772x_priv *priv = to_ov772x(sd);
struct v4l2_fract *tpf = >interval;
unsigned int fps;
- int ret;
+ int ret = 0;
fps = ov772x_select_fps(priv, tpf);
- ret = ov772x_set_frame_rate(priv, fps, priv->cfmt, priv->win);
- if (ret)
- return ret;
+ mutex_lock(>lock);
+ /*
+* If the device is not powered up by the host driver do
+* not apply any changes to H/W at this time. Instead
+* the frame rate will be restored right after power-up.
+*/
+ if (priv->power_count > 0) {
+ ret = ov772x_set_frame_rate(priv, fps, priv->cfmt, priv->win);
+ if (ret)
+ goto error;
+ }
tpf->numerator = 1;
tpf->denominator = fps;
priv->fps = fps;
- return 0;
+error:
+ mutex_unlock(>lock);
+
+ return ret;
}
static int ov772x_s_ctrl(struct v4l2_ctrl *ctrl)
@@ -765,6 +776,16 @@ static int ov772x_s_ctrl(struct v4l2_ctrl *ctrl)
int ret = 0;
u8 val;
+ /* v4l2_ctrl_lock() locks our own mutex */
+
+ /*
+* If the device is not powered up by the host driver do
+* not apply any controls to H/W at this time. Instead
+* the controls will be restored right after power-up.
+*/
+ if (priv->power_count == 0)
+ return 0;
+
switch (ctrl->id) {
case V4L2_CID_VFLIP:
val = ctrl->val ? VFLIP_IMG : 0x00;
@@ -885,6 +906,10 @@ static int ov772x_power_off(struct ov772x_priv *priv)
return 0;
}
+static int ov772x_set_params(struct ov772x_priv *priv,
+const struct ov772x_color_format *cfmt,
+const struct ov772x_win_size *win);
+
static int ov772x_s_power(struct v4l2_subdev *sd, int on)
{
struct ov772x_priv *priv = to_ov772x(sd);
@@ -895,8 +920,20 @@ static int ov772x_s_power(struct v4l2_subdev *sd, int on)
/* If the power count is modified from 0 to != 0 or from != 0 to 0,
* update the power state.
*/
- if (priv->power_count == !on)
- ret = on ? ov772x_power_on(priv) : ov772x_power_off(priv);
+ if (priv->power_count == !on) {
+ if (on) {
+ ret = ov772x_power_on(priv);
+ /*
+* Restore the format, the frame rate, and
+* the controls
+*/
+ if (!ret)
+ ret = ov772x_set_params(priv, priv->cfmt,
+ priv->win);
+ } else {
+ ret = ov772x_power_off(priv);
+ }
+ }
if (!ret) {
/* Update the power count. */
@@ -1163,7 +1200,7 @@ static int ov772x_set_fmt(struct v4l2_subdev *sd,
struct v4l2_mbus_framefmt *mf = >format;
const struct ov772x_color_format *cfmt;
const struct ov772x_win_size *win;
- int ret;
+ int ret = 0;
if (format->pad)
return -EINVAL;
@@ -1184,14 +1221,24 @@ static int ov772x_set_fmt(struct v4l2_subdev *sd,
return 0;
}
- ret = ov772x_set_params(priv, cfmt, win);
- if (ret < 0)
- return ret;
-
+ mutex_lock(>lock);
+ /*
+* If the device is not powered up by the host driver do
+* not apply any changes to H/W at this time. Instead
+* the format will be restored right after power-up.
+*/
+ if (priv->power_count > 0) {
+ ret = ov772x_set_params(priv, cfmt, win);
+ if (ret < 0)
+
[PATCH v5 14/14] media: ov772x: create subdevice device node
Set the V4L2_SUBDEV_FL_HAS_DEVNODE flag for the subdevice so that the subdevice device node is created. Cc: Jacopo Mondi <jacopo+rene...@jmondi.org> Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com> Cc: Hans Verkuil <hans.verk...@cisco.com> Cc: Sakari Ailus <sakari.ai...@linux.intel.com> Cc: Mauro Carvalho Chehab <mche...@s-opensource.com> Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com> --- * v5 - No changes drivers/media/i2c/ov772x.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c index 4b479f9..f7f4fe6 100644 --- a/drivers/media/i2c/ov772x.c +++ b/drivers/media/i2c/ov772x.c @@ -1409,6 +1409,7 @@ static int ov772x_probe(struct i2c_client *client, mutex_init(>lock); v4l2_i2c_subdev_init(>subdev, client, _subdev_ops); + priv->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; v4l2_ctrl_handler_init(>hdl, 3); /* Use our mutex for the controls */ priv->hdl.lock = >lock; -- 2.7.4
[PATCH v5 03/14] media: ov772x: allow i2c controllers without I2C_FUNC_PROTOCOL_MANGLING
The ov772x driver only works when the i2c controller have
I2C_FUNC_PROTOCOL_MANGLING. However, many i2c controller drivers don't
support it.
The reason that the ov772x requires I2C_FUNC_PROTOCOL_MANGLING is that
it doesn't support repeated starts.
This changes the reading ov772x register method so that it doesn't
require I2C_FUNC_PROTOCOL_MANGLING by calling two separated i2c messages.
Cc: Jacopo Mondi <jacopo+rene...@jmondi.org>
Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
Cc: Hans Verkuil <hans.verk...@cisco.com>
Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
Cc: Mauro Carvalho Chehab <mche...@s-opensource.com>
Cc: Wolfram Sang <w...@the-dreams.de>
Reviewed-by: Jacopo Mondi <jacopo+rene...@jmondi.org>
Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
---
* v5
- Add Reviewed-by: line
drivers/media/i2c/ov772x.c | 20 ++--
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
index e255070..b6223bf 100644
--- a/drivers/media/i2c/ov772x.c
+++ b/drivers/media/i2c/ov772x.c
@@ -542,9 +542,19 @@ static struct ov772x_priv *to_ov772x(struct v4l2_subdev
*sd)
return container_of(sd, struct ov772x_priv, subdev);
}
-static inline int ov772x_read(struct i2c_client *client, u8 addr)
+static int ov772x_read(struct i2c_client *client, u8 addr)
{
- return i2c_smbus_read_byte_data(client, addr);
+ int ret;
+ u8 val;
+
+ ret = i2c_master_send(client, , 1);
+ if (ret < 0)
+ return ret;
+ ret = i2c_master_recv(client, , 1);
+ if (ret < 0)
+ return ret;
+
+ return val;
}
static inline int ov772x_write(struct i2c_client *client, u8 addr, u8 value)
@@ -1255,13 +1265,11 @@ static int ov772x_probe(struct i2c_client *client,
return -EINVAL;
}
- if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA |
- I2C_FUNC_PROTOCOL_MANGLING)) {
+ if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA)) {
dev_err(>dev,
- "I2C-Adapter doesn't support SMBUS_BYTE_DATA or
PROTOCOL_MANGLING\n");
+ "I2C-Adapter doesn't support SMBUS_BYTE_DATA\n");
return -EIO;
}
- client->flags |= I2C_CLIENT_SCCB;
priv = devm_kzalloc(>dev, sizeof(*priv), GFP_KERNEL);
if (!priv)
--
2.7.4
[PATCH v5 01/14] media: dt-bindings: ov772x: add device tree binding
This adds a device tree binding documentation for OV7720/OV7725 sensor.
Cc: Jacopo Mondi <jacopo+rene...@jmondi.org>
Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
Cc: Hans Verkuil <hans.verk...@cisco.com>
Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
Cc: Mauro Carvalho Chehab <mche...@s-opensource.com>
Cc: Rob Herring <robh...@kernel.org>
Reviewed-by: Rob Herring <r...@kernel.org>
Reviewed-by: Jacopo Mondi <jac...@jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
---
* v5
- No changes
.../devicetree/bindings/media/i2c/ov772x.txt | 40 ++
MAINTAINERS| 1 +
2 files changed, 41 insertions(+)
create mode 100644 Documentation/devicetree/bindings/media/i2c/ov772x.txt
diff --git a/Documentation/devicetree/bindings/media/i2c/ov772x.txt
b/Documentation/devicetree/bindings/media/i2c/ov772x.txt
new file mode 100644
index 000..0b3ede5
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ov772x.txt
@@ -0,0 +1,40 @@
+* Omnivision OV7720/OV7725 CMOS sensor
+
+The Omnivision OV7720/OV7725 sensor supports multiple resolutions output,
+such as VGA, QVGA, and any size scaling down from CIF to 40x30. It also can
+support the YUV422, RGB565/555/444, GRB422 or raw RGB output formats.
+
+Required Properties:
+- compatible: shall be one of
+ "ovti,ov7720"
+ "ovti,ov7725"
+- clocks: reference to the xclk input clock.
+
+Optional Properties:
+- reset-gpios: reference to the GPIO connected to the RSTB pin which is
+ active low, if any.
+- powerdown-gpios: reference to the GPIO connected to the PWDN pin which is
+ active high, if any.
+
+The device node shall contain one 'port' child node with one child 'endpoint'
+subnode for its digital output video port, in accordance with the video
+interface bindings defined in Documentation/devicetree/bindings/media/
+video-interfaces.txt.
+
+Example:
+
+ {
+ ov772x: camera@21 {
+ compatible = "ovti,ov7725";
+ reg = <0x21>;
+ reset-gpios = <_gpio_0 0 GPIO_ACTIVE_LOW>;
+ powerdown-gpios = <_gpio_0 1 GPIO_ACTIVE_LOW>;
+ clocks = <>;
+
+ port {
+ ov772x_0: endpoint {
+ remote-endpoint = <_in0>;
+ };
+ };
+ };
+};
diff --git a/MAINTAINERS b/MAINTAINERS
index df6e9bb..cd4c270 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10356,6 +10356,7 @@ T: git git://linuxtv.org/media_tree.git
S: Odd fixes
F: drivers/media/i2c/ov772x.c
F: include/media/i2c/ov772x.h
+F: Documentation/devicetree/bindings/media/i2c/ov772x.txt
OMNIVISION OV7740 SENSOR DRIVER
M: Wenyou Yang <wenyou.y...@microchip.com>
--
2.7.4
[PATCH v5 04/14] media: ov772x: add checks for register read errors
This change adds checks for register read errors and returns correct
error code.
Cc: Jacopo Mondi <jacopo+rene...@jmondi.org>
Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
Cc: Hans Verkuil <hans.verk...@cisco.com>
Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
Cc: Mauro Carvalho Chehab <mche...@s-opensource.com>
Reviewed-by: Jacopo Mondi <jac...@jmondi.org>
Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
---
* v5
- No changes
drivers/media/i2c/ov772x.c | 20 ++--
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
index b6223bf..3fdbe64 100644
--- a/drivers/media/i2c/ov772x.c
+++ b/drivers/media/i2c/ov772x.c
@@ -1146,7 +1146,7 @@ static int ov772x_set_fmt(struct v4l2_subdev *sd,
static int ov772x_video_probe(struct ov772x_priv *priv)
{
struct i2c_client *client = v4l2_get_subdevdata(>subdev);
- u8 pid, ver;
+ int pid, ver, midh, midl;
const char *devname;
int ret;
@@ -1156,7 +1156,11 @@ static int ov772x_video_probe(struct ov772x_priv *priv)
/* Check and show product ID and manufacturer ID. */
pid = ov772x_read(client, PID);
+ if (pid < 0)
+ return pid;
ver = ov772x_read(client, VER);
+ if (ver < 0)
+ return ver;
switch (VERSION(pid, ver)) {
case OV7720:
@@ -1172,13 +1176,17 @@ static int ov772x_video_probe(struct ov772x_priv *priv)
goto done;
}
+ midh = ov772x_read(client, MIDH);
+ if (midh < 0)
+ return midh;
+ midl = ov772x_read(client, MIDL);
+ if (midl < 0)
+ return midl;
+
dev_info(>dev,
"%s Product ID %0x:%0x Manufacturer ID %x:%x\n",
-devname,
-pid,
-ver,
-ov772x_read(client, MIDH),
-ov772x_read(client, MIDL));
+devname, pid, ver, midh, midl);
+
ret = v4l2_ctrl_handler_setup(>hdl);
done:
--
2.7.4
[PATCH v5 00/14] media: ov772x: support media controller, device tree probing, etc.
This patchset includes support media controller, sub-device interface, device tree probing and other miscellanuous changes for ov772x driver. * v5 (thanks to Jacopo Mondi) - Add Acked-by: line - Add Reviewed-by: line - Remove unnecessary space - Align arguments to open parenthesis - Sort variable declarations - Make s_frame_interval() return -EBUSY while streaming * v4 (thanks to Laurent Pinchart) - Add Reviewed-by: lines - Correct setting of banding filter (New) - Omit consumer ID when getting clock reference (New) - Use v4l2_ctrl to get current control value (New) - Correctly restore the controls changed under power saving mode * v3 (thanks to Sakari Ailus and Jacopo Mondi) - Reorder the patches - Add Reviewed-by: lines - Remove I2C_CLIENT_SCCB flag set as it isn't needed anymore - Fix typo in the commit log - Return without resetting if ov772x_edgectrl() failed - Update the error message for missing platform data - Rename mutex name from power_lock to lock - Add warning for duplicated s_power call - Add newlines before labels - Remove __v4l2_ctrl_handler_setup in s_power() as it causes duplicated register settings - Make set_fmt() return -EBUSY while streaming (New) * v2 (thanks to Jacopo Mondi) - Replace the implementation of ov772x_read() instead of adding an alternative method - Assign the ov772x_read() return value to pid and ver directly - Do the same for MIDH and MIDL - Move video_probe() before the entity initialization and remove the #ifdef around the media_entity_cleanup() - Use generic names for reset and powerdown gpios (New) - Add "dt-bindings:" in the subject - Add a brief description of the sensor - Update the GPIO names - Indicate the GPIO active level - Add missing NULL checks for priv->info - Leave the check for the missing platform data if legacy platform data probe is used. - Handle nested s_power() calls (New) - Reconstruct s_frame_interval() (New) - Avoid accessing registers Akinobu Mita (14): media: dt-bindings: ov772x: add device tree binding media: ov772x: correct setting of banding filter media: ov772x: allow i2c controllers without I2C_FUNC_PROTOCOL_MANGLING media: ov772x: add checks for register read errors media: ov772x: add media controller support media: ov772x: use generic names for reset and powerdown gpios media: ov772x: omit consumer ID when getting clock reference media: ov772x: support device tree probing media: ov772x: handle nested s_power() calls media: ov772x: reconstruct s_frame_interval() media: ov772x: use v4l2_ctrl to get current control value media: ov772x: avoid accessing registers under power saving mode media: ov772x: make set_fmt() and s_frame_interval() return -EBUSY while streaming media: ov772x: create subdevice device node .../devicetree/bindings/media/i2c/ov772x.txt | 40 +++ MAINTAINERS| 1 + arch/sh/boards/mach-migor/setup.c | 7 +- drivers/media/i2c/ov772x.c | 357 +++-- 4 files changed, 308 insertions(+), 97 deletions(-) create mode 100644 Documentation/devicetree/bindings/media/i2c/ov772x.txt Cc: Jacopo Mondi <jacopo+rene...@jmondi.org> Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com> Cc: Hans Verkuil <hans.verk...@cisco.com> Cc: Sakari Ailus <sakari.ai...@linux.intel.com> Cc: Mauro Carvalho Chehab <mche...@s-opensource.com> Cc: Rob Herring <robh...@kernel.org> Cc: Wolfram Sang <w...@the-dreams.de> -- 2.7.4
[PATCH v5 02/14] media: ov772x: correct setting of banding filter
The banding filter ON/OFF is controlled via bit 5 of COM8 register. It
is attempted to be enabled in ov772x_set_params() by the following line.
ret = ov772x_mask_set(client, COM8, BNDF_ON_OFF, 1);
But this unexpectedly results disabling the banding filter, because the
mask and set bits are exclusive.
On the other hand, ov772x_s_ctrl() correctly sets the bit by:
ret = ov772x_mask_set(client, COM8, BNDF_ON_OFF, BNDF_ON_OFF);
Cc: Jacopo Mondi <jacopo+rene...@jmondi.org>
Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
Cc: Hans Verkuil <hans.verk...@cisco.com>
Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
Cc: Mauro Carvalho Chehab <mche...@s-opensource.com>
Acked-by: Jacopo Mondi <jacopo+rene...@jmondi.org>
Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
---
* v5
- Add Acked-by: line
drivers/media/i2c/ov772x.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
index b62860c..e255070 100644
--- a/drivers/media/i2c/ov772x.c
+++ b/drivers/media/i2c/ov772x.c
@@ -1035,7 +1035,7 @@ static int ov772x_set_params(struct ov772x_priv *priv,
/* Set COM8. */
if (priv->band_filter) {
- ret = ov772x_mask_set(client, COM8, BNDF_ON_OFF, 1);
+ ret = ov772x_mask_set(client, COM8, BNDF_ON_OFF, BNDF_ON_OFF);
if (!ret)
ret = ov772x_mask_set(client, BDBASE,
0xff, 256 - priv->band_filter);
--
2.7.4
[PATCH v5 06/14] media: ov772x: use generic names for reset and powerdown gpios
The ov772x driver uses "rstb-gpios" and "pwdn-gpios" for reset and
powerdown pins. However, using generic names for these gpios is
preferred. ("reset-gpios" and "powerdown-gpios" respectively)
There is only one mainline user for these gpios, so rename to generic
names.
Cc: Jacopo Mondi <jacopo+rene...@jmondi.org>
Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
Cc: Hans Verkuil <hans.verk...@cisco.com>
Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
Cc: Mauro Carvalho Chehab <mche...@s-opensource.com>
Reviewed-by: Jacopo Mondi <jac...@jmondi.org>
Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
---
* v5
- No changes
arch/sh/boards/mach-migor/setup.c | 5 +++--
drivers/media/i2c/ov772x.c| 8
2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/arch/sh/boards/mach-migor/setup.c
b/arch/sh/boards/mach-migor/setup.c
index 271dfc2..73b9ee4 100644
--- a/arch/sh/boards/mach-migor/setup.c
+++ b/arch/sh/boards/mach-migor/setup.c
@@ -351,8 +351,9 @@ static struct platform_device migor_ceu_device = {
static struct gpiod_lookup_table ov7725_gpios = {
.dev_id = "0-0021",
.table = {
- GPIO_LOOKUP("sh7722_pfc", GPIO_PTT0, "pwdn", GPIO_ACTIVE_HIGH),
- GPIO_LOOKUP("sh7722_pfc", GPIO_PTT3, "rstb", GPIO_ACTIVE_LOW),
+ GPIO_LOOKUP("sh7722_pfc", GPIO_PTT0, "powerdown",
+ GPIO_ACTIVE_HIGH),
+ GPIO_LOOKUP("sh7722_pfc", GPIO_PTT3, "reset", GPIO_ACTIVE_LOW),
},
};
diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
index bb5327f..97a65ce 100644
--- a/drivers/media/i2c/ov772x.c
+++ b/drivers/media/i2c/ov772x.c
@@ -837,10 +837,10 @@ static int ov772x_power_on(struct ov772x_priv *priv)
* available to handle this cleanly, request the GPIO temporarily
* to avoid conflicts.
*/
- priv->rstb_gpio = gpiod_get_optional(>dev, "rstb",
+ priv->rstb_gpio = gpiod_get_optional(>dev, "reset",
GPIOD_OUT_LOW);
if (IS_ERR(priv->rstb_gpio)) {
- dev_info(>dev, "Unable to get GPIO \"rstb\"");
+ dev_info(>dev, "Unable to get GPIO \"reset\"");
return PTR_ERR(priv->rstb_gpio);
}
@@ -1307,10 +1307,10 @@ static int ov772x_probe(struct i2c_client *client,
goto error_ctrl_free;
}
- priv->pwdn_gpio = gpiod_get_optional(>dev, "pwdn",
+ priv->pwdn_gpio = gpiod_get_optional(>dev, "powerdown",
GPIOD_OUT_LOW);
if (IS_ERR(priv->pwdn_gpio)) {
- dev_info(>dev, "Unable to get GPIO \"pwdn\"");
+ dev_info(>dev, "Unable to get GPIO \"powerdown\"");
ret = PTR_ERR(priv->pwdn_gpio);
goto error_clk_put;
}
--
2.7.4
[PATCH v5 07/14] media: ov772x: omit consumer ID when getting clock reference
Currently the ov772x driver obtains a clock with a specific consumer ID.
As there's a single clock for this driver, we could omit clock-names
property in device tree by passing NULL as a consumer ID to clk_get().
Cc: Jacopo Mondi <jacopo+rene...@jmondi.org>
Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
Cc: Hans Verkuil <hans.verk...@cisco.com>
Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
Cc: Mauro Carvalho Chehab <mche...@s-opensource.com>
Suggested-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
Tested-by: Jacopo Mondi <jacopo+rene...@jmondi.org>
Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
---
* v5
- No changes
arch/sh/boards/mach-migor/setup.c | 2 +-
drivers/media/i2c/ov772x.c| 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/sh/boards/mach-migor/setup.c
b/arch/sh/boards/mach-migor/setup.c
index 73b9ee4..11f8001 100644
--- a/arch/sh/boards/mach-migor/setup.c
+++ b/arch/sh/boards/mach-migor/setup.c
@@ -593,7 +593,7 @@ static int __init migor_devices_setup(void)
}
/* Add a clock alias for ov7725 xclk source. */
- clk_add_alias("xclk", "0-0021", "video_clk", NULL);
+ clk_add_alias(NULL, "0-0021", "video_clk", NULL);
/* Register GPIOs for video sources. */
gpiod_add_lookup_table(_gpios);
diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
index 97a65ce..f939e28 100644
--- a/drivers/media/i2c/ov772x.c
+++ b/drivers/media/i2c/ov772x.c
@@ -1300,7 +1300,7 @@ static int ov772x_probe(struct i2c_client *client,
if (priv->hdl.error)
return priv->hdl.error;
- priv->clk = clk_get(>dev, "xclk");
+ priv->clk = clk_get(>dev, NULL);
if (IS_ERR(priv->clk)) {
dev_err(>dev, "Unable to get xclk clock\n");
ret = PTR_ERR(priv->clk);
--
2.7.4
[PATCH v5 05/14] media: ov772x: add media controller support
Create a source pad and set the media controller type to the sensor.
Cc: Jacopo Mondi <jacopo+rene...@jmondi.org>
Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
Cc: Hans Verkuil <hans.verk...@cisco.com>
Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
Cc: Mauro Carvalho Chehab <mche...@s-opensource.com>
Reviewed-by: Jacopo Mondi <jac...@jmondi.org>
Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
---
* v5
- No changes
drivers/media/i2c/ov772x.c | 16 +++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
index 3fdbe64..bb5327f 100644
--- a/drivers/media/i2c/ov772x.c
+++ b/drivers/media/i2c/ov772x.c
@@ -424,6 +424,9 @@ struct ov772x_priv {
/* band_filter = COM8[5] ? 256 - BDBASE : 0 */
unsigned shortband_filter;
unsigned int fps;
+#ifdef CONFIG_MEDIA_CONTROLLER
+ struct media_pad pad;
+#endif
};
/*
@@ -1316,16 +1319,26 @@ static int ov772x_probe(struct i2c_client *client,
if (ret < 0)
goto error_gpio_put;
+#ifdef CONFIG_MEDIA_CONTROLLER
+ priv->pad.flags = MEDIA_PAD_FL_SOURCE;
+ priv->subdev.entity.function = MEDIA_ENT_F_CAM_SENSOR;
+ ret = media_entity_pads_init(>subdev.entity, 1, >pad);
+ if (ret < 0)
+ goto error_gpio_put;
+#endif
+
priv->cfmt = _cfmts[0];
priv->win = _win_sizes[0];
priv->fps = 15;
ret = v4l2_async_register_subdev(>subdev);
if (ret)
- goto error_gpio_put;
+ goto error_entity_cleanup;
return 0;
+error_entity_cleanup:
+ media_entity_cleanup(>subdev.entity);
error_gpio_put:
if (priv->pwdn_gpio)
gpiod_put(priv->pwdn_gpio);
@@ -1341,6 +1354,7 @@ static int ov772x_remove(struct i2c_client *client)
{
struct ov772x_priv *priv = to_ov772x(i2c_get_clientdata(client));
+ media_entity_cleanup(>subdev.entity);
clk_put(priv->clk);
if (priv->pwdn_gpio)
gpiod_put(priv->pwdn_gpio);
--
2.7.4
Re: [PATCH v5 08/14] media: ov772x: support device tree probing
2018-05-07 18:26 GMT+09:00 Sakari Ailus <sakari.ai...@linux.intel.com>:
> Dear Mita-san,
>
> On Sun, May 06, 2018 at 11:19:23PM +0900, Akinobu Mita wrote:
>> The ov772x driver currently only supports legacy platform data probe.
>> This change enables device tree probing.
>>
>> Note that the platform data probe can select auto or manual edge control
>> mode, but the device tree probling can only select auto edge control mode
>> for now.
>>
>> Cc: Jacopo Mondi <jacopo+rene...@jmondi.org>
>> Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
>> Cc: Hans Verkuil <hans.verk...@cisco.com>
>> Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
>> Cc: Mauro Carvalho Chehab <mche...@s-opensource.com>
>> Reviewed-by: Jacopo Mondi <jac...@jmondi.org>
>> Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
>> ---
>> * v5
>> - Remove unnecessary space
>>
>> drivers/media/i2c/ov772x.c | 64
>> --
>> 1 file changed, 45 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
>> index f939e28..2b02411 100644
>> --- a/drivers/media/i2c/ov772x.c
>> +++ b/drivers/media/i2c/ov772x.c
>> @@ -749,13 +749,13 @@ static int ov772x_s_ctrl(struct v4l2_ctrl *ctrl)
>> case V4L2_CID_VFLIP:
>> val = ctrl->val ? VFLIP_IMG : 0x00;
>> priv->flag_vflip = ctrl->val;
>> - if (priv->info->flags & OV772X_FLAG_VFLIP)
>> + if (priv->info && (priv->info->flags & OV772X_FLAG_VFLIP))
>> val ^= VFLIP_IMG;
>> return ov772x_mask_set(client, COM3, VFLIP_IMG, val);
>> case V4L2_CID_HFLIP:
>> val = ctrl->val ? HFLIP_IMG : 0x00;
>> priv->flag_hflip = ctrl->val;
>> - if (priv->info->flags & OV772X_FLAG_HFLIP)
>> + if (priv->info && (priv->info->flags & OV772X_FLAG_HFLIP))
>> val ^= HFLIP_IMG;
>> return ov772x_mask_set(client, COM3, HFLIP_IMG, val);
>> case V4L2_CID_BAND_STOP_FILTER:
>> @@ -914,19 +914,14 @@ static void ov772x_select_params(const struct
>> v4l2_mbus_framefmt *mf,
>> *win = ov772x_select_win(mf->width, mf->height);
>> }
>>
>> -static int ov772x_set_params(struct ov772x_priv *priv,
>> - const struct ov772x_color_format *cfmt,
>> - const struct ov772x_win_size *win)
>> +static int ov772x_edgectrl(struct ov772x_priv *priv)
>> {
>> struct i2c_client *client = v4l2_get_subdevdata(>subdev);
>> - struct v4l2_fract tpf;
>> int ret;
>> - u8 val;
>>
>> - /* Reset hardware. */
>> - ov772x_reset(client);
>> + if (!priv->info)
>> + return 0;
>>
>> - /* Edge Ctrl. */
>> if (priv->info->edgectrl.strength & OV772X_MANUAL_EDGE_CTRL) {
>> /*
>>* Manual Edge Control Mode.
>> @@ -937,19 +932,19 @@ static int ov772x_set_params(struct ov772x_priv *priv,
>>
>> ret = ov772x_mask_set(client, DSPAUTO, EDGE_ACTRL, 0x00);
>> if (ret < 0)
>> - goto ov772x_set_fmt_error;
>> + return ret;
>>
>> ret = ov772x_mask_set(client,
>> EDGE_TRSHLD, OV772X_EDGE_THRESHOLD_MASK,
>> priv->info->edgectrl.threshold);
>> if (ret < 0)
>> - goto ov772x_set_fmt_error;
>> + return ret;
>>
>> ret = ov772x_mask_set(client,
>> EDGE_STRNGT, OV772X_EDGE_STRENGTH_MASK,
>> priv->info->edgectrl.strength);
>> if (ret < 0)
>> - goto ov772x_set_fmt_error;
>> + return ret;
>>
>> } else if (priv->info->edgectrl.upper > priv->info->edgectrl.lower) {
>> /*
>> @@ -961,15 +956,35 @@ static int ov772x_set_params(struct ov772x_priv *priv,
>> EDGE_UPPER, OV772X_EDGE_UPPER_MASK,
>> priv->info->edgectrl.upper);
>> if (ret < 0)
>> - goto ov772x_set_fm
[RFC PATCH] i2c: add I2C_M_FORCE_STOP
This adds a new I2C_M_FORCE_STOP flag that forces a stop condition after
the message in a combined transaction.
This flag is intended to be used by the devices that don't support
repeated starts like SCCB (Serial Camera Control Bus) devices.
Here is an example usage for ov772x driver that needs to issue two
separated I2C messages as the ov772x device doesn't support repeated
starts.
static int ov772x_read(struct i2c_client *client, u8 addr)
{
u8 val;
struct i2c_msg msg[] = {
{
.addr = client->addr,
.flags = I2C_M_FORCE_STOP,
.len = 1,
.buf = ,
},
{
.addr = client->addr,
.flags = I2C_M_RD,
.len = 1,
.buf = ,
},
};
int ret;
ret = i2c_transfer(client->adapter, msg, 2);
if (ret != 2)
return (ret < 0) ? ret : -EIO;
return val;
}
This is another approach based on Mauro's advise for the initial attempt
(http://patchwork.ozlabs.org/patch/905192/).
Cc: Sebastian Reichel <sebastian.reic...@collabora.co.uk>
Cc: Wolfram Sang <w...@the-dreams.de>
Cc: Jacopo Mondi <jacopo+rene...@jmondi.org>
Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
Cc: Hans Verkuil <hans.verk...@cisco.com>
Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
Cc: Mauro Carvalho Chehab <mche...@s-opensource.com>
Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
---
drivers/i2c/i2c-core-base.c | 46 ++---
include/uapi/linux/i2c.h| 1 +
2 files changed, 36 insertions(+), 11 deletions(-)
diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index 1ba40bb..6b73484 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -1828,6 +1828,25 @@ static int i2c_check_for_quirks(struct i2c_adapter
*adap, struct i2c_msg *msgs,
return 0;
}
+static int i2c_transfer_nolock(struct i2c_adapter *adap, struct i2c_msg *msgs,
+ int num)
+{
+ unsigned long orig_jiffies;
+ int ret, try;
+
+ /* Retry automatically on arbitration loss */
+ orig_jiffies = jiffies;
+ for (ret = 0, try = 0; try <= adap->retries; try++) {
+ ret = adap->algo->master_xfer(adap, msgs, num);
+ if (ret != -EAGAIN)
+ break;
+ if (time_after(jiffies, orig_jiffies + adap->timeout))
+ break;
+ }
+
+ return ret;
+}
+
/**
* __i2c_transfer - unlocked flavor of i2c_transfer
* @adap: Handle to I2C bus
@@ -1842,8 +1861,8 @@ static int i2c_check_for_quirks(struct i2c_adapter *adap,
struct i2c_msg *msgs,
*/
int __i2c_transfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
{
- unsigned long orig_jiffies;
- int ret, try;
+ int ret;
+ int i, n;
if (WARN_ON(!msgs || num < 1))
return -EINVAL;
@@ -1857,7 +1876,6 @@ int __i2c_transfer(struct i2c_adapter *adap, struct
i2c_msg *msgs, int num)
* being executed when not needed.
*/
if (static_branch_unlikely(_trace_msg_key)) {
- int i;
for (i = 0; i < num; i++)
if (msgs[i].flags & I2C_M_RD)
trace_i2c_read(adap, [i], i);
@@ -1865,18 +1883,24 @@ int __i2c_transfer(struct i2c_adapter *adap, struct
i2c_msg *msgs, int num)
trace_i2c_write(adap, [i], i);
}
- /* Retry automatically on arbitration loss */
- orig_jiffies = jiffies;
- for (ret = 0, try = 0; try <= adap->retries; try++) {
- ret = adap->algo->master_xfer(adap, msgs, num);
- if (ret != -EAGAIN)
- break;
- if (time_after(jiffies, orig_jiffies + adap->timeout))
+ for (i = 0; i < num; i += n) {
+ for (n = 0; i + n < num; n++) {
+ if (msgs[i + n].flags & I2C_M_FORCE_STOP) {
+ n++;
+ break;
+ }
+ }
+
+ ret = i2c_transfer_nolock(adap, [i], n);
+ if (ret != n) {
+ if (i > 0)
+ ret = (ret < 0) ? i : i + ret;
break;
+ }
+ ret = i + n;
}
if (static_branch_unlikely(_trace_msg_key)) {
- int i;
for (i = 0; i < ret; i++)
if (msgs[i].flags & I2C_M_RD)
trace_i2c_reply(adap, [i], i);
diff --git a/include/uapi/linux/i2c.h b/include/uapi/linux/i2c.h
index f71a17
Re: [RFC PATCH] media: i2c: add SCCB helpers
2018-05-05 23:51 GMT+09:00 Mauro Carvalho Chehab <mchehab+sams...@kernel.org>:
> Em Fri, 27 Apr 2018 01:13:32 +0900
> Akinobu Mita <akinobu.m...@gmail.com> escreveu:
>
>> (This patch is in prototype stage)
>>
>> This adds SCCB helper functions (sccb_read_byte and sccb_write_byte) that
>> are intended to be used by some of Omnivision sensor drivers.
>
> What do you mean by "SCCB"?
Serial Camera Control Bus (SCCB). I'll write SCCB and the non
abbreviation together in the comment block and commit log.
>>
>> The ov772x driver is going to use these functions in order to make it work
>> with most i2c controllers.
>>
>> As the ov772x device doesn't support repeated starts, this driver currently
>> requires I2C_FUNC_PROTOCOL_MANGLING that is not supported by many i2c
>> controller drivers.
>>
>> With the sccb_read_byte() that issues two separated requests in order to
>> avoid repeated start, the driver doesn't require I2C_FUNC_PROTOCOL_MANGLING.
>>
>> Cc: Wolfram Sang <w...@the-dreams.de>
>> Cc: Jacopo Mondi <jacopo+rene...@jmondi.org>
>> Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
>> Cc: Hans Verkuil <hans.verk...@cisco.com>
>> Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
>> Cc: Mauro Carvalho Chehab <mche...@s-opensource.com>
>> Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
>> ---
>> drivers/media/i2c/Kconfig | 4
>> drivers/media/i2c/Makefile | 1 +
>> drivers/media/i2c/sccb.c | 35 +++
>> drivers/media/i2c/sccb.h | 14 ++
>> 4 files changed, 54 insertions(+)
>> create mode 100644 drivers/media/i2c/sccb.c
>> create mode 100644 drivers/media/i2c/sccb.h
>>
>> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
>> index 541f0d28..19e5601 100644
>> --- a/drivers/media/i2c/Kconfig
>> +++ b/drivers/media/i2c/Kconfig
>> @@ -569,6 +569,9 @@ config VIDEO_THS8200
>>
>> comment "Camera sensor devices"
>>
>> +config SCCB
>> + bool
>> +
>> config VIDEO_APTINA_PLL
>> tristate
>>
>> @@ -692,6 +695,7 @@ config VIDEO_OV772X
>> tristate "OmniVision OV772x sensor support"
>> depends on I2C && VIDEO_V4L2
>> depends on MEDIA_CAMERA_SUPPORT
>> + select SCCB
>> ---help---
>> This is a Video4Linux2 sensor-level driver for the OmniVision
>> OV772x camera.
>> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
>> index ea34aee..82fbd78 100644
>> --- a/drivers/media/i2c/Makefile
>> +++ b/drivers/media/i2c/Makefile
>> @@ -62,6 +62,7 @@ obj-$(CONFIG_VIDEO_VP27SMPX) += vp27smpx.o
>> obj-$(CONFIG_VIDEO_SONY_BTF_MPX) += sony-btf-mpx.o
>> obj-$(CONFIG_VIDEO_UPD64031A) += upd64031a.o
>> obj-$(CONFIG_VIDEO_UPD64083) += upd64083.o
>> +obj-$(CONFIG_SCCB) += sccb.o
>> obj-$(CONFIG_VIDEO_OV2640) += ov2640.o
>> obj-$(CONFIG_VIDEO_OV2685) += ov2685.o
>> obj-$(CONFIG_VIDEO_OV5640) += ov5640.o
>> diff --git a/drivers/media/i2c/sccb.c b/drivers/media/i2c/sccb.c
>> new file mode 100644
>> index 000..80a3fb7
>> --- /dev/null
>> +++ b/drivers/media/i2c/sccb.c
>> @@ -0,0 +1,35 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +#include
>> +
>> +int sccb_read_byte(struct i2c_client *client, u8 addr)
>> +{
>> + int ret;
>> + u8 val;
>> +
>> + /* Issue two separated requests in order to avoid repeated start */
>> +
>> + ret = i2c_master_send(client, , 1);
>> + if (ret < 0)
>> + return ret;
>> +
>> + ret = i2c_master_recv(client, , 1);
>> + if (ret < 0)
>> + return ret;
>
> Handling it this way is a very bad idea, as you may have an operation
> between those two, as you're locking/unlocking for each i2c_master
> call, e. g. the code should be, instead:
>
> i2c_lock_adapter();
> __i2c_transfer(); /* Send */
> __i2c_transfer(); /* Receive */
> i2c_unlock_adapter();
>
> Also, if the problem is just due to I2C not supporting REPEAT, IMHO,
> the best would be to add some IRC flag to indicate that.
I sent a patch using this approach.
> Btw, this is not the first device that doesn't support repeats.
> A good hint of drivers with similar issues is:
>
> $ git grep i2c_lock_adapter drivers/media/
> drivers/media/dvb-frontends/af9013.c:
> i2c_lock_adapter(client->adapter);
> drivers/media/dvb-frontends/af9013.c:
> i2c_lock_adapter(client->adapter);
> drivers/media/dvb-frontends/drxk_hard.c:i2c_lock_adapter(state->i2c);
> drivers/media/dvb-frontends/rtl2830.c: i2c_lock_adapter(client->adapter);
> drivers/media/dvb-frontends/rtl2830.c: i2c_lock_adapter(client->adapter);
> drivers/media/dvb-frontends/rtl2830.c: i2c_lock_adapter(client->adapter);
> drivers/media/dvb-frontends/tda1004x.c: i2c_lock_adapter(state->i2c);
> drivers/media/tuners/tda18271-common.c:
> i2c_lock_adapter(priv->i2c_props.adap);
> drivers/media/tuners/tda18271-common.c:
> i2c_lock_adapter(priv->i2c_props.adap);
>
> Regards,
> Mauro
Re: [PATCH v3 07/11] media: ov772x: handle nested s_power() calls
2018-04-23 17:35 GMT+09:00 jacopo mondi <jac...@jmondi.org>:
> Hi Akinobu,
>thanks for v3
>
> On Mon, Apr 23, 2018 at 12:56:13AM +0900, Akinobu Mita wrote:
>> Depending on the v4l2 driver, calling s_power() could be nested. So the
>> actual transitions between power saving mode and normal operation mode
>> should only happen at the first power on and the last power off.
>>
>> This adds an s_power() nesting counter and updates the power state if the
>> counter is modified from 0 to != 0 or from != 0 to 0.
>>
>> Cc: Jacopo Mondi <jacopo+rene...@jmondi.org>
>> Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
>> Cc: Hans Verkuil <hans.verk...@cisco.com>
>> Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
>> Cc: Mauro Carvalho Chehab <mche...@s-opensource.com>
>> Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
>> ---
>> * v3
>> - Rename mutex name from power_lock to lock
>> - Add warning for duplicated s_power call
>>
>> drivers/media/i2c/ov772x.c | 34 ++
>> 1 file changed, 30 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
>> index 95c1c95d..8c0b850 100644
>> --- a/drivers/media/i2c/ov772x.c
>> +++ b/drivers/media/i2c/ov772x.c
>> @@ -424,6 +424,9 @@ struct ov772x_priv {
>> /* band_filter = COM8[5] ? 256 - BDBASE : 0 */
>> unsigned shortband_filter;
>> unsigned int fps;
>> + /* lock to protect power_count */
>> + struct mutex lock;
>> + int power_count;
>> #ifdef CONFIG_MEDIA_CONTROLLER
>> struct media_pad pad;
>> #endif
>> @@ -871,9 +874,26 @@ static int ov772x_power_off(struct ov772x_priv *priv)
>> static int ov772x_s_power(struct v4l2_subdev *sd, int on)
>> {
>> struct ov772x_priv *priv = to_ov772x(sd);
>> + int ret = 0;
>> +
>> + mutex_lock(>lock);
>> +
>> + /* If the power count is modified from 0 to != 0 or from != 0 to 0,
>> + * update the power state.
>> + */
>> + if (priv->power_count == !on)
>> + ret = on ? ov772x_power_on(priv) : ov772x_power_off(priv);
>> +
>> + if (!ret) {
>> + /* Update the power count. */
>> + priv->power_count += on ? 1 : -1;
>> + WARN(priv->power_count < 0, "Unbalanced power count\n");
>> + WARN(priv->power_count > 1, "Duplicated s_power call\n");
>
>
> The first time you receive a power on (on == 1, power_count == 0)
> you'll have:
>
> if (0 == !1)
> ret = ov772x_power_on()
> if (!0)
> power_count += 1;
>
> If power management is 'nested' you could receive a second,
> unexpected, power on (on == 1, power_count == 1)
> if (1 == !1)
> if (!0)
> power_count += 1;
>
> Now power_count == 2, if you now receive a power off, you're going to
> ignore it:
>
> if (2 == !0)
> ret = ov772x_power_off();
> if (!0)
> power_count += -1;
>
> Again you now receive a new power off, and you're going to issue that
> one.
>
> if (1 == !0)
> ret = ov772x_power_off();
> if (!0)
> power_count += -1;
>
> So this seems correct to me!
>
> If I were you, I would simplify this as:
>
> power_count += on ? 1 : -1;
> if (power_count == !!on)
> return on ? ov772x_power_on(priv) :
> ov772x_power_off(priv);
> WARN_ON(power_count < 0 || power_count > 1);
>
> return 0;
The only drawback is that the power_count is updated even when the
ov772x_power_on/off failed.
> which, if I'm not wrong expands to:
>
> power_count = 0 ; on = 1
> power_count += 1;
> if (1 == !!1)
> return ov772x_power_on(priv);
>
> power_count = 1 ; on = 1
> power_count += 1;
> if (2 == !!1)
> WARN_ON(2 > 1)
>
> power_count = 2 ; on = 0
> power_count -= 1;
> if (1 == !!0)
> if (1 > 1 || 1 < 0)
>
> power_count = 1; on = 0
> power_count -= 1;
> if (0 == !!0)
> return ov772_power_off(priv);
>
> power_count = 0 ; on = 0
> power_count -= 1;
> if (-1 == !!0)
> WARN_ON(-1 < 0)
>
> But if you have see
Re: [PATCH v3 01/11] media: dt-bindings: ov772x: add device tree binding
2018-04-27 6:34 GMT+09:00 Laurent Pinchart <laurent.pinch...@ideasonboard.com>:
> Hi Mita-san,
>
> On Thursday, 26 April 2018 19:17:55 EEST Akinobu Mita wrote:
>> 2018-04-26 7:40 GMT+09:00 Laurent Pinchart:
>> > On Wednesday, 25 April 2018 19:19:11 EEST Akinobu Mita wrote:
>> >> 2018-04-24 0:54 GMT+09:00 Akinobu Mita <akinobu.m...@gmail.com>:
>> >> > 2018-04-23 18:17 GMT+09:00 Laurent Pinchart:
>> >> >> On Sunday, 22 April 2018 18:56:07 EEST Akinobu Mita wrote:
>> >> >>> This adds a device tree binding documentation for OV7720/OV7725
>> >> >>> sensor.
>> >> >>>
>> >> >>> Cc: Jacopo Mondi <jacopo+rene...@jmondi.org>
>> >> >>> Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
>> >> >>> Cc: Hans Verkuil <hans.verk...@cisco.com>
>> >> >>> Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
>> >> >>> Cc: Mauro Carvalho Chehab <mche...@s-opensource.com>
>> >> >>> Cc: Rob Herring <robh...@kernel.org>
>> >> >>> Reviewed-by: Rob Herring <r...@kernel.org>
>> >> >>> Reviewed-by: Jacopo Mondi <jac...@jmondi.org>
>> >> >>> Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
>> >> >>> ---
>> >> >>> * v3
>> >> >>> - Add Reviewed-by: lines
>> >> >>>
>> >> >>> .../devicetree/bindings/media/i2c/ov772x.txt | 42
>> >> >>> +++
>> >> >>> MAINTAINERS| 1 +
>> >> >>> 2 files changed, 43 insertions(+)
>> >> >>> create mode 100644
>> >> >>> Documentation/devicetree/bindings/media/i2c/ov772x.txt
>> >> >>>
>> >> >>> diff --git a/Documentation/devicetree/bindings/media/i2c/ov772x.txt
>> >> >>> b/Documentation/devicetree/bindings/media/i2c/ov772x.txt new file
>> >> >>> mode
>> >> >>> 100644
>> >> >>> index 000..b045503
>> >> >>> --- /dev/null
>> >> >>> +++ b/Documentation/devicetree/bindings/media/i2c/ov772x.txt
>> >> >>> @@ -0,0 +1,42 @@
>> >> >>> +* Omnivision OV7720/OV7725 CMOS sensor
>> >> >>> +
>> >> >>> +The Omnivision OV7720/OV7725 sensor supports multiple resolutions
>> >> >>> output,
>> >> >>> +such as VGA, QVGA, and any size scaling down from CIF to 40x30. It
>> >> >>> also
>> >> >>> can +support the YUV422, RGB565/555/444, GRB422 or raw RGB output
>> >> >>> formats. +
>> >> >>> +Required Properties:
>> >> >>> +- compatible: shall be one of
>> >> >>> + "ovti,ov7720"
>> >> >>> + "ovti,ov7725"
>> >> >>> +- clocks: reference to the xclk input clock.
>> >> >>> +- clock-names: shall be "xclk".
>> >> >>
>> >> >> As there's a single clock we could omit clock-names, couldn't we ?
>> >> >
>> >> > Sounds good.
>> >> >
>> >> > I'll prepare another patch that replaces the clock consumer ID argument
>> >> > of clk_get() from "xclk" to NULL, and remove the above line in this
>> >> > bindings.
>> >>
>> >> I thought it's easy to do. However, there is a non-DT user
>> >> (arch/sh/boards/mach-migor/setup.c) that defines a clock with "xclk" ID.
>> >>
>> >> This can be resolved by retrying clk_get() with NULL if no entry
>> >> with "xclk". But should we do so or leave as is?
>> >
>> > How about patching the board code to register the clock alias with
>> >
>> > clk_add_alias(NULL, "0-0021", "video_clk", NULL);
>>
>> Sounds good.
>>
>> But I'm a bit worried about whether clk_add_alias() can be called with
>> alias == NULL. I couldn't find such use case.
>
> There aren't many occurrences, but
>
> $ find . -type f -exec grep -l 'clk_add_alias(NULL' {} \;
> /drivers/clk/ti/clk.c
> /drivers/clk/ti/fixed-factor.c
> /drivers/clk/ti/clk-dra7-atl.c
> /drivers/clk/ti/composite.c
>
> A quick code analysis also shows me that this should be supported.
This hits ti_clk_add_alias() only. The function name is very similar,
but the first argument is struct device *.
Anyway, I'll add the change you suggested as Jacopo tested it.
Re: [PATCH v4 12/14] media: ov772x: avoid accessing registers under power saving mode
2018-05-04 6:03 GMT+09:00 jacopo mondi <jac...@jmondi.org>:
> Hi Akinobu,
> let me see if I got this right...
>
> On Mon, Apr 30, 2018 at 02:13:11AM +0900, Akinobu Mita wrote:
>> The set_fmt() in subdev pad ops, the s_ctrl() for subdev control handler,
>> and the s_frame_interval() in subdev video ops could be called when the
>> device is under power saving mode. These callbacks for ov772x driver
>> cause updating H/W registers that will fail under power saving mode.
>>
>> This avoids it by not apply any changes to H/W if the device is not powered
>> up. Instead the changes will be restored right after power-up.
>>
>> Cc: Jacopo Mondi <jacopo+rene...@jmondi.org>
>> Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
>> Cc: Hans Verkuil <hans.verk...@cisco.com>
>> Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
>> Cc: Mauro Carvalho Chehab <mche...@s-opensource.com>
>> Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
>> ---
>> * v4
>> - No changes
>>
>> drivers/media/i2c/ov772x.c | 79
>> +-
>> 1 file changed, 64 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
>> index 3e6ca98..bd37169 100644
>> --- a/drivers/media/i2c/ov772x.c
>> +++ b/drivers/media/i2c/ov772x.c
>> @@ -741,19 +741,30 @@ static int ov772x_s_frame_interval(struct v4l2_subdev
>> *sd,
>> struct ov772x_priv *priv = to_ov772x(sd);
>> struct v4l2_fract *tpf = >interval;
>> unsigned int fps;
>> - int ret;
>> + int ret = 0;
>>
>> fps = ov772x_select_fps(priv, tpf);
>>
>> - ret = ov772x_set_frame_rate(priv, fps, priv->cfmt, priv->win);
>> - if (ret)
>> - return ret;
>> + mutex_lock(>lock);
>> + /*
>> + * If the device is not powered up by the host driver do
>> + * not apply any changes to H/W at this time. Instead
>> + * the frame rate will be restored right after power-up.
>> + */
>> + if (priv->power_count > 0) {
>
> If I'm not wrong this is not protected when the device is
> streaming.
>
> Since Hans' series frame control is called by g/s_parm (until a new
> ioctl is not introduced) and I've looked at the call stack and it
> seems to me it does not check for active streaming[1]. I -think-
> this is even worse when this is called on the subdev, as there is
> no shared notion of active streaming. Am I wrong?
>
> If you have to check for active streaming here (I see some other
> drivers doing that, eg ov5640), then I see two ways, or you just
> return -EBUSY, or you save the desired FPS for later, but then it gets
> messy as you won't be able to just restore paramters at power_on()
> time, but you would need to do that also at start streaming time.
>
> My opinion is that you should check for streaming active (as you're
> doing for the set_fmt() function in [13/14], do you agree?
I agree. I would like to make ov772x_s_frame_interval() return -EBUSY
without saving the desired FPS for later when the device is streaming.
I'm going to prepare v5 patches that address the above and other issues
you found in v4 unless you prefer the incremental patch series.
> [1] This calls for a device wise 'streaming' state handler. Maybe it's
> there but I failed to find checks for that.
[PATCH] media: pxa_camera: avoid duplicate s_power calls
The open() operation for the pxa_camera driver always calls s_power()
operation to put its subdevice sensor in normal operation mode, and the
release() operation always call s_power() operation to put the subdevice
in power saving mode.
This requires the subdevice sensor driver to keep track of its power
state in order to avoid putting the subdevice in power saving mode while
the device is still opened by some users.
Many subdevice drivers handle it by the boilerplate code that increments
and decrements an internal counter in s_power() like below:
/*
* If the power count is modified from 0 to != 0 or from != 0 to 0,
* update the power state.
*/
if (sensor->power_count == !on) {
ret = ov5640_set_power(sensor, !!on);
if (ret)
goto out;
}
/* Update the power count. */
sensor->power_count += on ? 1 : -1;
However, some subdevice drivers don't handle it and may cause a problem
with the pxa_camera driver if the video device is opened by more than
two users at the same time.
Instead of propagating the boilerplate code for each subdevice driver
that implement s_power, this introduces an trick that many V4L2 drivers
are using with v4l2_fh_is_singular_file().
Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
Cc: Mauro Carvalho Chehab <mche...@kernel.org>
Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
---
drivers/media/platform/pxa_camera.c | 17 -
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/drivers/media/platform/pxa_camera.c
b/drivers/media/platform/pxa_camera.c
index c71a007..c792cb1 100644
--- a/drivers/media/platform/pxa_camera.c
+++ b/drivers/media/platform/pxa_camera.c
@@ -2040,6 +2040,9 @@ static int pxac_fops_camera_open(struct file *filp)
if (ret < 0)
goto out;
+ if (!v4l2_fh_is_singular_file(filp))
+ goto out;
+
ret = sensor_call(pcdev, core, s_power, 1);
if (ret)
v4l2_fh_release(filp);
@@ -2052,13 +2055,17 @@ static int pxac_fops_camera_release(struct file *filp)
{
struct pxa_camera_dev *pcdev = video_drvdata(filp);
int ret;
-
- ret = vb2_fop_release(filp);
- if (ret < 0)
- return ret;
+ bool fh_singular;
mutex_lock(>mlock);
- ret = sensor_call(pcdev, core, s_power, 0);
+
+ fh_singular = v4l2_fh_is_singular_file(filp);
+
+ ret = _vb2_fop_release(filp, NULL);
+
+ if (fh_singular)
+ ret = sensor_call(pcdev, core, s_power, 0);
+
mutex_unlock(>mlock);
return ret;
--
2.7.4
Re: [PATCH] media: pxa_camera: avoid duplicate s_power calls
2018-05-22 22:59 GMT+09:00 Sakari Ailus <sakari.ai...@linux.intel.com>:
> Dear Mita-san,
>
> On Mon, May 21, 2018 at 12:40:38AM +0900, Akinobu Mita wrote:
>> The open() operation for the pxa_camera driver always calls s_power()
>> operation to put its subdevice sensor in normal operation mode, and the
>> release() operation always call s_power() operation to put the subdevice
>> in power saving mode.
>>
>> This requires the subdevice sensor driver to keep track of its power
>> state in order to avoid putting the subdevice in power saving mode while
>> the device is still opened by some users.
>>
>> Many subdevice drivers handle it by the boilerplate code that increments
>> and decrements an internal counter in s_power() like below:
>>
>> /*
>>* If the power count is modified from 0 to != 0 or from != 0 to 0,
>>* update the power state.
>>*/
>> if (sensor->power_count == !on) {
>> ret = ov5640_set_power(sensor, !!on);
>> if (ret)
>> goto out;
>> }
>>
>> /* Update the power count. */
>> sensor->power_count += on ? 1 : -1;
>>
>> However, some subdevice drivers don't handle it and may cause a problem
>> with the pxa_camera driver if the video device is opened by more than
>> two users at the same time.
>>
>> Instead of propagating the boilerplate code for each subdevice driver
>> that implement s_power, this introduces an trick that many V4L2 drivers
>> are using with v4l2_fh_is_singular_file().
>
> I'd rather like that the sub-device drivers would move to use runtime PM
> instead of depending on the s_power() callback. It's much cleaner that way.
Sounds good.
I'll look into whether some sensor drivers can be converted to use it.
> It's not a near-term solution though. The approach seems fine, please see
> comments below though.
>
>>
>> Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
>> Cc: Mauro Carvalho Chehab <mche...@kernel.org>
>> Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
>> ---
>> drivers/media/platform/pxa_camera.c | 17 -
>> 1 file changed, 12 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/media/platform/pxa_camera.c
>> b/drivers/media/platform/pxa_camera.c
>> index c71a007..c792cb1 100644
>> --- a/drivers/media/platform/pxa_camera.c
>> +++ b/drivers/media/platform/pxa_camera.c
>> @@ -2040,6 +2040,9 @@ static int pxac_fops_camera_open(struct file *filp)
>> if (ret < 0)
>> goto out;
>>
>> + if (!v4l2_fh_is_singular_file(filp))
>> + goto out;
>> +
>> ret = sensor_call(pcdev, core, s_power, 1);
>> if (ret)
>> v4l2_fh_release(filp);
>> @@ -2052,13 +2055,17 @@ static int pxac_fops_camera_release(struct file
>> *filp)
>> {
>> struct pxa_camera_dev *pcdev = video_drvdata(filp);
>> int ret;
>> -
>> - ret = vb2_fop_release(filp);
>> - if (ret < 0)
>> - return ret;
>> + bool fh_singular;
>>
>> mutex_lock(>mlock);
>> - ret = sensor_call(pcdev, core, s_power, 0);
>> +
>> + fh_singular = v4l2_fh_is_singular_file(filp);
>> +
>> + ret = _vb2_fop_release(filp, NULL);
>
> I'm not sure whether using the return value to return an error from release
> is really useful. If you want to use it, I'd shout loud instead.
What is the best way to handle these errors in release?
AFAICS, vb2_fop_release() always returns zero for now and most platform
drivers don't use return value from s_power() calling with on == 0.
So ignoring both of vb2_fop_release error and s_power error makes sense?
>> +
>> + if (fh_singular)
>
> ret assigned previously is overwritten here without checking.
>
>> + ret = sensor_call(pcdev, core, s_power, 0);
>> +
>> mutex_unlock(>mlock);
>>
>> return ret;
>
> --
> Sakari Ailus
> sakari.ai...@linux.intel.com
[PATCH] media: s3c-camif: ignore -ENOIOCTLCMD from v4l2_subdev_call for s_power
When the subdevice doesn't provide s_power core ops callback, the v4l2_subdev_call for s_power returns -ENOIOCTLCMD. If the subdevice doesn't have the special handling for its power saving mode, the s_power isn't required. So -ENOIOCTLCMD from the v4l2_subdev_call should be ignored. Cc: Sylwester Nawrocki Cc: Hans Verkuil Cc: Sakari Ailus Cc: Mauro Carvalho Chehab Signed-off-by: Akinobu Mita --- drivers/media/platform/s3c-camif/camif-capture.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/media/platform/s3c-camif/camif-capture.c b/drivers/media/platform/s3c-camif/camif-capture.c index 9ab8e7e..b1d9f38 100644 --- a/drivers/media/platform/s3c-camif/camif-capture.c +++ b/drivers/media/platform/s3c-camif/camif-capture.c @@ -117,6 +117,8 @@ static int sensor_set_power(struct camif_dev *camif, int on) if (camif->sensor.power_count == !on) err = v4l2_subdev_call(sensor->sd, core, s_power, on); + if (err == -ENOIOCTLCMD) + err = 0; if (!err) sensor->power_count += on ? 1 : -1; -- 2.7.4
[PATCH] media: soc_camera: ov772x: correct setting of banding filter
The banding filter ON/OFF is controlled via bit 5 of COM8 register. It
is attempted to be enabled in ov772x_set_params() by the following line.
ret = ov772x_mask_set(client, COM8, BNDF_ON_OFF, 1);
But this unexpectedly results disabling the banding filter, because the
mask and set bits are exclusive.
On the other hand, ov772x_s_ctrl() correctly sets the bit by:
ret = ov772x_mask_set(client, COM8, BNDF_ON_OFF, BNDF_ON_OFF);
The same fix was already applied to non-soc_camera version of ov772x
driver in the commit commit a024ee14cd36 ("media: ov772x: correct setting
of banding filter")
Cc: Jacopo Mondi
Cc: Laurent Pinchart
Cc: Hans Verkuil
Cc: Sakari Ailus
Cc: Mauro Carvalho Chehab
Signed-off-by: Akinobu Mita
---
drivers/media/i2c/soc_camera/ov772x.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/i2c/soc_camera/ov772x.c
b/drivers/media/i2c/soc_camera/ov772x.c
index 8063835..14377af 100644
--- a/drivers/media/i2c/soc_camera/ov772x.c
+++ b/drivers/media/i2c/soc_camera/ov772x.c
@@ -834,7 +834,7 @@ static int ov772x_set_params(struct ov772x_priv *priv,
* set COM8
*/
if (priv->band_filter) {
- ret = ov772x_mask_set(client, COM8, BNDF_ON_OFF, 1);
+ ret = ov772x_mask_set(client, COM8, BNDF_ON_OFF, BNDF_ON_OFF);
if (!ret)
ret = ov772x_mask_set(client, BDBASE,
0xff, 256 - priv->band_filter);
--
2.7.4
[RFC PATCH v2] media: i2c: add SCCB helpers
(This is 2nd version of SCCB helpers patch. After 1st version was
submitted, I sent alternative patch titled "i2c: add I2C_M_FORCE_STOP".
But it wasn't accepted because it makes the I2C core code unreadable.
I couldn't find out a way to untangle it, so I returned to the original
approach.)
This adds Serial Camera Control Bus (SCCB) helper functions (sccb_read_byte
and sccb_write_byte) that are intended to be used by some of Omnivision
sensor drivers.
The ov772x driver is going to use these functions in order to make it work
with most i2c controllers.
As the ov772x device doesn't support repeated starts, this driver currently
requires I2C_FUNC_PROTOCOL_MANGLING that is not supported by many i2c
controller drivers.
With the sccb_read_byte() that issues two separated requests in order to
avoid repeated start, the driver doesn't require I2C_FUNC_PROTOCOL_MANGLING.
Cc: Sebastian Reichel
Cc: Wolfram Sang
Cc: Jacopo Mondi
Cc: Laurent Pinchart
Cc: Hans Verkuil
Cc: Sakari Ailus
Cc: Mauro Carvalho Chehab
Signed-off-by: Akinobu Mita
---
* v2
- Convert all helpers into static inline functions, and remove C source
and Kconfig option.
- Acquire i2c adapter lock while issuing two requests for sccb_read_byte
drivers/media/i2c/sccb.h | 74
1 file changed, 74 insertions(+)
create mode 100644 drivers/media/i2c/sccb.h
diff --git a/drivers/media/i2c/sccb.h b/drivers/media/i2c/sccb.h
new file mode 100644
index 000..a531fdc
--- /dev/null
+++ b/drivers/media/i2c/sccb.h
@@ -0,0 +1,74 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Serial Camera Control Bus (SCCB) helper functions
+ */
+
+#ifndef __SCCB_H__
+#define __SCCB_H__
+
+#include
+
+/**
+ * sccb_read_byte - Read data from SCCB slave device
+ * @client: Handle to slave device
+ * @addr: Register to be read from
+ *
+ * This executes the 2-phase write transmission cycle that is followed by a
+ * 2-phase read transmission cycle, returning negative errno else a data byte
+ * received from the device.
+ */
+static inline int sccb_read_byte(struct i2c_client *client, u8 addr)
+{
+ u8 val;
+ struct i2c_msg msg[] = {
+ {
+ .addr = client->addr,
+ .len = 1,
+ .buf = ,
+ },
+ {
+ .addr = client->addr,
+ .flags = I2C_M_RD,
+ .len = 1,
+ .buf = ,
+ },
+ };
+ int ret;
+ int i;
+
+ i2c_lock_adapter(client->adapter);
+
+ /* Issue two separated requests in order to avoid repeated start */
+ for (i = 0; i < 2; i++) {
+ ret = __i2c_transfer(client->adapter, [i], 1);
+ if (ret != 1)
+ break;
+ }
+
+ i2c_unlock_adapter(client->adapter);
+
+ return i == 2 ? val : ret;
+}
+
+/**
+ * sccb_write_byte - Write data to SCCB slave device
+ * @client: Handle to slave device
+ * @addr: Register to write to
+ * @data: Value to be written
+ *
+ * This executes the SCCB 3-phase write transmission cycle, returning negative
+ * errno else zero on success.
+ */
+static inline int sccb_write_byte(struct i2c_client *client, u8 addr, u8 data)
+{
+ int ret;
+ unsigned char msgbuf[] = { addr, data };
+
+ ret = i2c_master_send(client, msgbuf, 2);
+ if (ret < 0)
+ return ret;
+
+ return 0;
+}
+
+#endif /* __SCCB_H__ */
--
2.7.4
[PATCH] media: pxa_camera: ignore -ENOIOCTLCMD from v4l2_subdev_call for s_power
When the subdevice doesn't provide s_power core ops callback, the
v4l2_subdev_call for s_power returns -ENOIOCTLCMD. If the subdevice
doesn't have the special handling for its power saving mode, the s_power
isn't required. So -ENOIOCTLCMD from the v4l2_subdev_call should be
ignored.
Actually the -ENOIOCTLCMD is ignored in this driver's suspend/resume,
but the others treat the -ENOIOCTLCMD as an error.
This prepares a wrapper function to ignore -ENOIOCTLCMD and replaces
all s_power calls with it.
This also adds warning message when s_power() is failed.
Cc: Hans Verkuil
Cc: Sakari Ailus
Cc: Mauro Carvalho Chehab
Signed-off-by: Akinobu Mita
---
drivers/media/platform/pxa_camera.c | 35 +++
1 file changed, 23 insertions(+), 12 deletions(-)
diff --git a/drivers/media/platform/pxa_camera.c
b/drivers/media/platform/pxa_camera.c
index c792cb1..4d5a26b 100644
--- a/drivers/media/platform/pxa_camera.c
+++ b/drivers/media/platform/pxa_camera.c
@@ -2030,6 +2030,22 @@ static int pxac_vidioc_s_input(struct file *file, void
*priv, unsigned int i)
return 0;
}
+static int pxac_sensor_set_power(struct pxa_camera_dev *pcdev, int on)
+{
+ int ret;
+
+ ret = sensor_call(pcdev, core, s_power, on);
+ if (ret == -ENOIOCTLCMD)
+ ret = 0;
+ if (ret) {
+ dev_warn(pcdev_to_dev(pcdev),
+"Failed to put subdevice in %s mode: %d\n",
+on ? "normal operation" : "power saving", ret);
+ }
+
+ return ret;
+}
+
static int pxac_fops_camera_open(struct file *filp)
{
struct pxa_camera_dev *pcdev = video_drvdata(filp);
@@ -2043,7 +2059,7 @@ static int pxac_fops_camera_open(struct file *filp)
if (!v4l2_fh_is_singular_file(filp))
goto out;
- ret = sensor_call(pcdev, core, s_power, 1);
+ ret = pxac_sensor_set_power(pcdev, 1);
if (ret)
v4l2_fh_release(filp);
out:
@@ -2064,7 +2080,7 @@ static int pxac_fops_camera_release(struct file *filp)
ret = _vb2_fop_release(filp, NULL);
if (fh_singular)
- ret = sensor_call(pcdev, core, s_power, 0);
+ ret = pxac_sensor_set_power(pcdev, 0);
mutex_unlock(>mlock);
@@ -2167,7 +2183,7 @@ static int pxa_camera_sensor_bound(struct
v4l2_async_notifier *notifier,
pix->pixelformat = pcdev->current_fmt->host_fmt->fourcc;
v4l2_fill_mbus_format(mf, pix, pcdev->current_fmt->code);
- err = sensor_call(pcdev, core, s_power, 1);
+ err = pxac_sensor_set_power(pcdev, 1);
if (err)
goto out;
@@ -2194,7 +2210,7 @@ static int pxa_camera_sensor_bound(struct
v4l2_async_notifier *notifier,
}
out_sensor_poweroff:
- err = sensor_call(pcdev, core, s_power, 0);
+ err = pxac_sensor_set_power(pcdev, 0);
out:
mutex_unlock(>mlock);
return err;
@@ -2249,11 +2265,8 @@ static int pxa_camera_suspend(struct device *dev)
pcdev->save_cicr[i++] = __raw_readl(pcdev->base + CICR3);
pcdev->save_cicr[i++] = __raw_readl(pcdev->base + CICR4);
- if (pcdev->sensor) {
- ret = sensor_call(pcdev, core, s_power, 0);
- if (ret == -ENOIOCTLCMD)
- ret = 0;
- }
+ if (pcdev->sensor)
+ ret = pxac_sensor_set_power(pcdev, 0);
return ret;
}
@@ -2270,9 +2283,7 @@ static int pxa_camera_resume(struct device *dev)
__raw_writel(pcdev->save_cicr[i++], pcdev->base + CICR4);
if (pcdev->sensor) {
- ret = sensor_call(pcdev, core, s_power, 1);
- if (ret == -ENOIOCTLCMD)
- ret = 0;
+ ret = pxac_sensor_set_power(pcdev, 1);
}
/* Restart frame capture if active buffer exists */
--
2.7.4
Re: [PATCH v2] media: pxa_camera: avoid duplicate s_power calls
2018-05-29 15:17 GMT+09:00 Hans Verkuil :
> Hi Akinobu,
>
> On 05/27/2018 05:30 PM, Akinobu Mita wrote:
>> The open() operation for the pxa_camera driver always calls s_power()
>> operation to put its subdevice sensor in normal operation mode, and the
>> release() operation always call s_power() operation to put the subdevice
>> in power saving mode.
>>
>> This requires the subdevice sensor driver to keep track of its power
>> state in order to avoid putting the subdevice in power saving mode while
>> the device is still opened by some users.
>>
>> Many subdevice drivers handle it by the boilerplate code that increments
>> and decrements an internal counter in s_power() like below:
>>
>> /*
>>* If the power count is modified from 0 to != 0 or from != 0 to 0,
>>* update the power state.
>>*/
>> if (sensor->power_count == !on) {
>> ret = ov5640_set_power(sensor, !!on);
>> if (ret)
>> goto out;
>> }
>>
>> /* Update the power count. */
>> sensor->power_count += on ? 1 : -1;
>>
>> However, some subdevice drivers don't handle it and may cause a problem
>> with the pxa_camera driver if the video device is opened by more than
>> two users at the same time.
>>
>> Instead of propagating the boilerplate code for each subdevice driver
>> that implement s_power, this introduces an trick that many V4L2 drivers
>> are using with v4l2_fh_is_singular_file().
>>
>> Cc: Sakari Ailus
>> Cc: Mauro Carvalho Chehab
>> Signed-off-by: Akinobu Mita
>> ---
>> * v2
>> - Print warning message when s_power() is failed. (not printing warning
>> when _vb2_fop_release() is failed as it always returns zero for now)
>
> Please note that v1 has already been merged, so if you can make a v3 rebased
> on top of the latest media_tree master branch, then I'll queue that up for
> 4.18.
OK. There are several calls to s_power in this driver, so I'll make
a patch to add a wrapper function that prints warning message and
replace s_power calls with it.
I realized that s_power calls in suspend/resume ignore -ENOIOCTLCMD
error and other s_power calls also should ignore it. So I'll include
the check in the wrapper function.
[PATCH v2] media: pxa_camera: avoid duplicate s_power calls
The open() operation for the pxa_camera driver always calls s_power()
operation to put its subdevice sensor in normal operation mode, and the
release() operation always call s_power() operation to put the subdevice
in power saving mode.
This requires the subdevice sensor driver to keep track of its power
state in order to avoid putting the subdevice in power saving mode while
the device is still opened by some users.
Many subdevice drivers handle it by the boilerplate code that increments
and decrements an internal counter in s_power() like below:
/*
* If the power count is modified from 0 to != 0 or from != 0 to 0,
* update the power state.
*/
if (sensor->power_count == !on) {
ret = ov5640_set_power(sensor, !!on);
if (ret)
goto out;
}
/* Update the power count. */
sensor->power_count += on ? 1 : -1;
However, some subdevice drivers don't handle it and may cause a problem
with the pxa_camera driver if the video device is opened by more than
two users at the same time.
Instead of propagating the boilerplate code for each subdevice driver
that implement s_power, this introduces an trick that many V4L2 drivers
are using with v4l2_fh_is_singular_file().
Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
Cc: Mauro Carvalho Chehab <mche...@kernel.org>
Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
---
* v2
- Print warning message when s_power() is failed. (not printing warning
when _vb2_fop_release() is failed as it always returns zero for now)
drivers/media/platform/pxa_camera.c | 23 ++-
1 file changed, 18 insertions(+), 5 deletions(-)
diff --git a/drivers/media/platform/pxa_camera.c
b/drivers/media/platform/pxa_camera.c
index c71a007..a35f461 100644
--- a/drivers/media/platform/pxa_camera.c
+++ b/drivers/media/platform/pxa_camera.c
@@ -2040,6 +2040,9 @@ static int pxac_fops_camera_open(struct file *filp)
if (ret < 0)
goto out;
+ if (!v4l2_fh_is_singular_file(filp))
+ goto out;
+
ret = sensor_call(pcdev, core, s_power, 1);
if (ret)
v4l2_fh_release(filp);
@@ -2052,13 +2055,23 @@ static int pxac_fops_camera_release(struct file *filp)
{
struct pxa_camera_dev *pcdev = video_drvdata(filp);
int ret;
-
- ret = vb2_fop_release(filp);
- if (ret < 0)
- return ret;
+ bool fh_singular;
mutex_lock(>mlock);
- ret = sensor_call(pcdev, core, s_power, 0);
+
+ fh_singular = v4l2_fh_is_singular_file(filp);
+
+ ret = _vb2_fop_release(filp, NULL);
+
+ if (fh_singular) {
+ ret = sensor_call(pcdev, core, s_power, 0);
+ if (ret) {
+ dev_warn(pcdev_to_dev(pcdev),
+"Failed to put subdevice in power saving mode:
%d\n",
+ret);
+ }
+ }
+
mutex_unlock(>mlock);
return ret;
--
2.7.4
[RFC PATCH] media: i2c: add SCCB helpers
(This patch is in prototype stage)
This adds SCCB helper functions (sccb_read_byte and sccb_write_byte) that
are intended to be used by some of Omnivision sensor drivers.
The ov772x driver is going to use these functions in order to make it work
with most i2c controllers.
As the ov772x device doesn't support repeated starts, this driver currently
requires I2C_FUNC_PROTOCOL_MANGLING that is not supported by many i2c
controller drivers.
With the sccb_read_byte() that issues two separated requests in order to
avoid repeated start, the driver doesn't require I2C_FUNC_PROTOCOL_MANGLING.
Cc: Wolfram Sang <w...@the-dreams.de>
Cc: Jacopo Mondi <jacopo+rene...@jmondi.org>
Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
Cc: Hans Verkuil <hans.verk...@cisco.com>
Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
Cc: Mauro Carvalho Chehab <mche...@s-opensource.com>
Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
---
drivers/media/i2c/Kconfig | 4
drivers/media/i2c/Makefile | 1 +
drivers/media/i2c/sccb.c | 35 +++
drivers/media/i2c/sccb.h | 14 ++
4 files changed, 54 insertions(+)
create mode 100644 drivers/media/i2c/sccb.c
create mode 100644 drivers/media/i2c/sccb.h
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 541f0d28..19e5601 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -569,6 +569,9 @@ config VIDEO_THS8200
comment "Camera sensor devices"
+config SCCB
+ bool
+
config VIDEO_APTINA_PLL
tristate
@@ -692,6 +695,7 @@ config VIDEO_OV772X
tristate "OmniVision OV772x sensor support"
depends on I2C && VIDEO_V4L2
depends on MEDIA_CAMERA_SUPPORT
+ select SCCB
---help---
This is a Video4Linux2 sensor-level driver for the OmniVision
OV772x camera.
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index ea34aee..82fbd78 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -62,6 +62,7 @@ obj-$(CONFIG_VIDEO_VP27SMPX) += vp27smpx.o
obj-$(CONFIG_VIDEO_SONY_BTF_MPX) += sony-btf-mpx.o
obj-$(CONFIG_VIDEO_UPD64031A) += upd64031a.o
obj-$(CONFIG_VIDEO_UPD64083) += upd64083.o
+obj-$(CONFIG_SCCB) += sccb.o
obj-$(CONFIG_VIDEO_OV2640) += ov2640.o
obj-$(CONFIG_VIDEO_OV2685) += ov2685.o
obj-$(CONFIG_VIDEO_OV5640) += ov5640.o
diff --git a/drivers/media/i2c/sccb.c b/drivers/media/i2c/sccb.c
new file mode 100644
index 000..80a3fb7
--- /dev/null
+++ b/drivers/media/i2c/sccb.c
@@ -0,0 +1,35 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include
+
+int sccb_read_byte(struct i2c_client *client, u8 addr)
+{
+ int ret;
+ u8 val;
+
+ /* Issue two separated requests in order to avoid repeated start */
+
+ ret = i2c_master_send(client, , 1);
+ if (ret < 0)
+ return ret;
+
+ ret = i2c_master_recv(client, , 1);
+ if (ret < 0)
+ return ret;
+
+ return val;
+}
+EXPORT_SYMBOL_GPL(sccb_read_byte);
+
+int sccb_write_byte(struct i2c_client *client, u8 addr, u8 data)
+{
+ int ret;
+ unsigned char msgbuf[] = { addr, data };
+
+ ret = i2c_master_send(client, msgbuf, 2);
+ if (ret < 0)
+ return ret;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(sccb_write_byte);
diff --git a/drivers/media/i2c/sccb.h b/drivers/media/i2c/sccb.h
new file mode 100644
index 000..68da0e9
--- /dev/null
+++ b/drivers/media/i2c/sccb.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * SCCB helper functions
+ */
+
+#ifndef __SCCB_H__
+#define __SCCB_H__
+
+#include
+
+int sccb_read_byte(struct i2c_client *client, u8 addr);
+int sccb_write_byte(struct i2c_client *client, u8 addr, u8 data);
+
+#endif /* __SCCB_H__ */
--
2.7.4
Re: [PATCH v3 01/11] media: dt-bindings: ov772x: add device tree binding
2018-04-26 7:40 GMT+09:00 Laurent Pinchart <laurent.pinch...@ideasonboard.com>: > Hi Mita-san, > > On Wednesday, 25 April 2018 19:19:11 EEST Akinobu Mita wrote: >> 2018-04-24 0:54 GMT+09:00 Akinobu Mita <akinobu.m...@gmail.com>: >> > 2018-04-23 18:17 GMT+09:00 Laurent Pinchart: >> >> On Sunday, 22 April 2018 18:56:07 EEST Akinobu Mita wrote: >> >>> This adds a device tree binding documentation for OV7720/OV7725 sensor. >> >>> >> >>> Cc: Jacopo Mondi <jacopo+rene...@jmondi.org> >> >>> Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com> >> >>> Cc: Hans Verkuil <hans.verk...@cisco.com> >> >>> Cc: Sakari Ailus <sakari.ai...@linux.intel.com> >> >>> Cc: Mauro Carvalho Chehab <mche...@s-opensource.com> >> >>> Cc: Rob Herring <robh...@kernel.org> >> >>> Reviewed-by: Rob Herring <r...@kernel.org> >> >>> Reviewed-by: Jacopo Mondi <jac...@jmondi.org> >> >>> Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com> >> >>> --- >> >>> * v3 >> >>> - Add Reviewed-by: lines >> >>> >> >>> .../devicetree/bindings/media/i2c/ov772x.txt | 42 +++ >> >>> MAINTAINERS| 1 + >> >>> 2 files changed, 43 insertions(+) >> >>> create mode 100644 >> >>> Documentation/devicetree/bindings/media/i2c/ov772x.txt >> >>> >> >>> diff --git a/Documentation/devicetree/bindings/media/i2c/ov772x.txt >> >>> b/Documentation/devicetree/bindings/media/i2c/ov772x.txt new file mode >> >>> 100644 >> >>> index 000..b045503 >> >>> --- /dev/null >> >>> +++ b/Documentation/devicetree/bindings/media/i2c/ov772x.txt >> >>> @@ -0,0 +1,42 @@ >> >>> +* Omnivision OV7720/OV7725 CMOS sensor >> >>> + >> >>> +The Omnivision OV7720/OV7725 sensor supports multiple resolutions >> >>> output, >> >>> +such as VGA, QVGA, and any size scaling down from CIF to 40x30. It also >> >>> can +support the YUV422, RGB565/555/444, GRB422 or raw RGB output >> >>> formats. + >> >>> +Required Properties: >> >>> +- compatible: shall be one of >> >>> + "ovti,ov7720" >> >>> + "ovti,ov7725" >> >>> +- clocks: reference to the xclk input clock. >> >>> +- clock-names: shall be "xclk". >> >> >> >> As there's a single clock we could omit clock-names, couldn't we ? >> > >> > Sounds good. >> > >> > I'll prepare another patch that replaces the clock consumer ID argument >> > of clk_get() from "xclk" to NULL, and remove the above line in this >> > bindings. >> >> I thought it's easy to do. However, there is a non-DT user >> (arch/sh/boards/mach-migor/setup.c) that defines a clock with "xclk" ID. >> >> This can be resolved by retrying clk_get() with NULL if no entry >> with "xclk". But should we do so or leave as is? > > How about patching the board code to register the clock alias with > > clk_add_alias(NULL, "0-0021", "video_clk", NULL); Sounds good. But I'm a bit worried about whether clk_add_alias() can be called with alias == NULL. I couldn't find such use case. Probably Jacopo can verify whether it works or not with v4 patchset.
[PATCH v4 06/14] media: ov772x: use generic names for reset and powerdown gpios
The ov772x driver uses "rstb-gpios" and "pwdn-gpios" for reset and
powerdown pins. However, using generic names for these gpios is
preferred. ("reset-gpios" and "powerdown-gpios" respectively)
There is only one mainline user for these gpios, so rename to generic
names.
Cc: Jacopo Mondi <jacopo+rene...@jmondi.org>
Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
Cc: Hans Verkuil <hans.verk...@cisco.com>
Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
Cc: Mauro Carvalho Chehab <mche...@s-opensource.com>
Reviewed-by: Jacopo Mondi <jac...@jmondi.org>
Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
---
* v4
- No changes
arch/sh/boards/mach-migor/setup.c | 5 +++--
drivers/media/i2c/ov772x.c| 8
2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/arch/sh/boards/mach-migor/setup.c
b/arch/sh/boards/mach-migor/setup.c
index 271dfc2..73b9ee4 100644
--- a/arch/sh/boards/mach-migor/setup.c
+++ b/arch/sh/boards/mach-migor/setup.c
@@ -351,8 +351,9 @@ static struct platform_device migor_ceu_device = {
static struct gpiod_lookup_table ov7725_gpios = {
.dev_id = "0-0021",
.table = {
- GPIO_LOOKUP("sh7722_pfc", GPIO_PTT0, "pwdn", GPIO_ACTIVE_HIGH),
- GPIO_LOOKUP("sh7722_pfc", GPIO_PTT3, "rstb", GPIO_ACTIVE_LOW),
+ GPIO_LOOKUP("sh7722_pfc", GPIO_PTT0, "powerdown",
+ GPIO_ACTIVE_HIGH),
+ GPIO_LOOKUP("sh7722_pfc", GPIO_PTT3, "reset", GPIO_ACTIVE_LOW),
},
};
diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
index bb5327f..97a65ce 100644
--- a/drivers/media/i2c/ov772x.c
+++ b/drivers/media/i2c/ov772x.c
@@ -837,10 +837,10 @@ static int ov772x_power_on(struct ov772x_priv *priv)
* available to handle this cleanly, request the GPIO temporarily
* to avoid conflicts.
*/
- priv->rstb_gpio = gpiod_get_optional(>dev, "rstb",
+ priv->rstb_gpio = gpiod_get_optional(>dev, "reset",
GPIOD_OUT_LOW);
if (IS_ERR(priv->rstb_gpio)) {
- dev_info(>dev, "Unable to get GPIO \"rstb\"");
+ dev_info(>dev, "Unable to get GPIO \"reset\"");
return PTR_ERR(priv->rstb_gpio);
}
@@ -1307,10 +1307,10 @@ static int ov772x_probe(struct i2c_client *client,
goto error_ctrl_free;
}
- priv->pwdn_gpio = gpiod_get_optional(>dev, "pwdn",
+ priv->pwdn_gpio = gpiod_get_optional(>dev, "powerdown",
GPIOD_OUT_LOW);
if (IS_ERR(priv->pwdn_gpio)) {
- dev_info(>dev, "Unable to get GPIO \"pwdn\"");
+ dev_info(>dev, "Unable to get GPIO \"powerdown\"");
ret = PTR_ERR(priv->pwdn_gpio);
goto error_clk_put;
}
--
2.7.4
[PATCH v4 12/14] media: ov772x: avoid accessing registers under power saving mode
The set_fmt() in subdev pad ops, the s_ctrl() for subdev control handler,
and the s_frame_interval() in subdev video ops could be called when the
device is under power saving mode. These callbacks for ov772x driver
cause updating H/W registers that will fail under power saving mode.
This avoids it by not apply any changes to H/W if the device is not powered
up. Instead the changes will be restored right after power-up.
Cc: Jacopo Mondi <jacopo+rene...@jmondi.org>
Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
Cc: Hans Verkuil <hans.verk...@cisco.com>
Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
Cc: Mauro Carvalho Chehab <mche...@s-opensource.com>
Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
---
* v4
- No changes
drivers/media/i2c/ov772x.c | 79 +-
1 file changed, 64 insertions(+), 15 deletions(-)
diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
index 3e6ca98..bd37169 100644
--- a/drivers/media/i2c/ov772x.c
+++ b/drivers/media/i2c/ov772x.c
@@ -741,19 +741,30 @@ static int ov772x_s_frame_interval(struct v4l2_subdev *sd,
struct ov772x_priv *priv = to_ov772x(sd);
struct v4l2_fract *tpf = >interval;
unsigned int fps;
- int ret;
+ int ret = 0;
fps = ov772x_select_fps(priv, tpf);
- ret = ov772x_set_frame_rate(priv, fps, priv->cfmt, priv->win);
- if (ret)
- return ret;
+ mutex_lock(>lock);
+ /*
+* If the device is not powered up by the host driver do
+* not apply any changes to H/W at this time. Instead
+* the frame rate will be restored right after power-up.
+*/
+ if (priv->power_count > 0) {
+ ret = ov772x_set_frame_rate(priv, fps, priv->cfmt, priv->win);
+ if (ret)
+ goto error;
+ }
tpf->numerator = 1;
tpf->denominator = fps;
priv->fps = fps;
- return 0;
+error:
+ mutex_unlock(>lock);
+
+ return ret;
}
static int ov772x_s_ctrl(struct v4l2_ctrl *ctrl)
@@ -765,6 +776,16 @@ static int ov772x_s_ctrl(struct v4l2_ctrl *ctrl)
int ret = 0;
u8 val;
+ /* v4l2_ctrl_lock() locks our own mutex */
+
+ /*
+* If the device is not powered up by the host driver do
+* not apply any controls to H/W at this time. Instead
+* the controls will be restored right after power-up.
+*/
+ if (priv->power_count == 0)
+ return 0;
+
switch (ctrl->id) {
case V4L2_CID_VFLIP:
val = ctrl->val ? VFLIP_IMG : 0x00;
@@ -885,6 +906,10 @@ static int ov772x_power_off(struct ov772x_priv *priv)
return 0;
}
+static int ov772x_set_params(struct ov772x_priv *priv,
+const struct ov772x_color_format *cfmt,
+const struct ov772x_win_size *win);
+
static int ov772x_s_power(struct v4l2_subdev *sd, int on)
{
struct ov772x_priv *priv = to_ov772x(sd);
@@ -895,8 +920,20 @@ static int ov772x_s_power(struct v4l2_subdev *sd, int on)
/* If the power count is modified from 0 to != 0 or from != 0 to 0,
* update the power state.
*/
- if (priv->power_count == !on)
- ret = on ? ov772x_power_on(priv) : ov772x_power_off(priv);
+ if (priv->power_count == !on) {
+ if (on) {
+ ret = ov772x_power_on(priv);
+ /*
+* Restore the format, the frame rate, and
+* the controls
+*/
+ if (!ret)
+ ret = ov772x_set_params(priv, priv->cfmt,
+ priv->win);
+ } else {
+ ret = ov772x_power_off(priv);
+ }
+ }
if (!ret) {
/* Update the power count. */
@@ -1163,7 +1200,7 @@ static int ov772x_set_fmt(struct v4l2_subdev *sd,
struct v4l2_mbus_framefmt *mf = >format;
const struct ov772x_color_format *cfmt;
const struct ov772x_win_size *win;
- int ret;
+ int ret = 0;
if (format->pad)
return -EINVAL;
@@ -1184,14 +1221,24 @@ static int ov772x_set_fmt(struct v4l2_subdev *sd,
return 0;
}
- ret = ov772x_set_params(priv, cfmt, win);
- if (ret < 0)
- return ret;
-
+ mutex_lock(>lock);
+ /*
+* If the device is not powered up by the host driver do
+* not apply any changes to H/W at this time. Instead
+* the format will be restored right after power-up.
+*/
+ if (priv->power_count > 0) {
+ ret = ov772x_set_params(priv, cfmt, win);
+ if (ret < 0)
+
[PATCH v4 11/14] media: ov772x: use v4l2_ctrl to get current control value
The ov772x driver provides three V4L2 controls and the current value of
each control is saved as a variable in the private data structure.
We don't need to keep track of the current value by ourself, if we use
v4l2_ctrl returned from v4l2_ctrl_new_std() instead.
This is a preparatory change to avoid accessing registers under power
saving mode. This simplifies s_ctrl() by making it just return without
saving the current control value in private area when it is called under
power saving mode.
Cc: Jacopo Mondi <jacopo+rene...@jmondi.org>
Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
Cc: Hans Verkuil <hans.verk...@cisco.com>
Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
Cc: Mauro Carvalho Chehab <mche...@s-opensource.com>
Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
---
* v4
- New patch
drivers/media/i2c/ov772x.c | 34 +-
1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
index 7ea157e..3e6ca98 100644
--- a/drivers/media/i2c/ov772x.c
+++ b/drivers/media/i2c/ov772x.c
@@ -419,10 +419,10 @@ struct ov772x_priv {
struct gpio_desc *rstb_gpio;
const struct ov772x_color_format *cfmt;
const struct ov772x_win_size *win;
- unsigned shortflag_vflip:1;
- unsigned shortflag_hflip:1;
+ struct v4l2_ctrl *vflip_ctrl;
+ struct v4l2_ctrl *hflip_ctrl;
/* band_filter = COM8[5] ? 256 - BDBASE : 0 */
- unsigned shortband_filter;
+ struct v4l2_ctrl *band_filter_ctrl;
unsigned int fps;
/* lock to protect power_count */
struct mutex lock;
@@ -768,13 +768,11 @@ static int ov772x_s_ctrl(struct v4l2_ctrl *ctrl)
switch (ctrl->id) {
case V4L2_CID_VFLIP:
val = ctrl->val ? VFLIP_IMG : 0x00;
- priv->flag_vflip = ctrl->val;
if (priv->info && (priv->info->flags & OV772X_FLAG_VFLIP))
val ^= VFLIP_IMG;
return ov772x_mask_set(client, COM3, VFLIP_IMG, val);
case V4L2_CID_HFLIP:
val = ctrl->val ? HFLIP_IMG : 0x00;
- priv->flag_hflip = ctrl->val;
if (priv->info && (priv->info->flags & OV772X_FLAG_HFLIP))
val ^= HFLIP_IMG;
return ov772x_mask_set(client, COM3, HFLIP_IMG, val);
@@ -794,8 +792,7 @@ static int ov772x_s_ctrl(struct v4l2_ctrl *ctrl)
ret = ov772x_mask_set(client, BDBASE,
0xff, val);
}
- if (!ret)
- priv->band_filter = ctrl->val;
+
return ret;
}
@@ -1075,9 +1072,9 @@ static int ov772x_set_params(struct ov772x_priv *priv,
val |= VFLIP_IMG;
if (priv->info && (priv->info->flags & OV772X_FLAG_HFLIP))
val |= HFLIP_IMG;
- if (priv->flag_vflip)
+ if (priv->vflip_ctrl->val)
val ^= VFLIP_IMG;
- if (priv->flag_hflip)
+ if (priv->hflip_ctrl->val)
val ^= HFLIP_IMG;
ret = ov772x_mask_set(client,
@@ -1096,11 +1093,13 @@ static int ov772x_set_params(struct ov772x_priv *priv,
goto ov772x_set_fmt_error;
/* Set COM8. */
- if (priv->band_filter) {
+ if (priv->band_filter_ctrl->val) {
+ unsigned short band_filter = priv->band_filter_ctrl->val;
+
ret = ov772x_mask_set(client, COM8, BNDF_ON_OFF, BNDF_ON_OFF);
if (!ret)
ret = ov772x_mask_set(client, BDBASE,
- 0xff, 256 - priv->band_filter);
+ 0xff, 256 - band_filter);
if (ret < 0)
goto ov772x_set_fmt_error;
}
@@ -1341,12 +1340,13 @@ static int ov772x_probe(struct i2c_client *client,
v4l2_i2c_subdev_init(>subdev, client, _subdev_ops);
v4l2_ctrl_handler_init(>hdl, 3);
- v4l2_ctrl_new_std(>hdl, _ctrl_ops,
- V4L2_CID_VFLIP, 0, 1, 1, 0);
- v4l2_ctrl_new_std(>hdl, _ctrl_ops,
- V4L2_CID_HFLIP, 0, 1, 1, 0);
- v4l2_ctrl_new_std(>hdl, _ctrl_ops,
- V4L2_CID_BAND_STOP_FILTER, 0, 256, 1, 0);
+ priv->vflip_ctrl = v4l2_ctrl_new_std(>hdl, _ctrl_ops,
+V4L2_CID_VFLIP, 0, 1, 1, 0);
+ priv->hflip_ctrl = v4l2_ctrl_new_std(>hdl, _ctrl_ops,
+
[PATCH v4 04/14] media: ov772x: add checks for register read errors
This change adds checks for register read errors and returns correct
error code.
Cc: Jacopo Mondi <jacopo+rene...@jmondi.org>
Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
Cc: Hans Verkuil <hans.verk...@cisco.com>
Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
Cc: Mauro Carvalho Chehab <mche...@s-opensource.com>
Reviewed-by: Jacopo Mondi <jac...@jmondi.org>
Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
---
* v4
- No changes
drivers/media/i2c/ov772x.c | 20 ++--
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
index b6223bf..3fdbe64 100644
--- a/drivers/media/i2c/ov772x.c
+++ b/drivers/media/i2c/ov772x.c
@@ -1146,7 +1146,7 @@ static int ov772x_set_fmt(struct v4l2_subdev *sd,
static int ov772x_video_probe(struct ov772x_priv *priv)
{
struct i2c_client *client = v4l2_get_subdevdata(>subdev);
- u8 pid, ver;
+ int pid, ver, midh, midl;
const char *devname;
int ret;
@@ -1156,7 +1156,11 @@ static int ov772x_video_probe(struct ov772x_priv *priv)
/* Check and show product ID and manufacturer ID. */
pid = ov772x_read(client, PID);
+ if (pid < 0)
+ return pid;
ver = ov772x_read(client, VER);
+ if (ver < 0)
+ return ver;
switch (VERSION(pid, ver)) {
case OV7720:
@@ -1172,13 +1176,17 @@ static int ov772x_video_probe(struct ov772x_priv *priv)
goto done;
}
+ midh = ov772x_read(client, MIDH);
+ if (midh < 0)
+ return midh;
+ midl = ov772x_read(client, MIDL);
+ if (midl < 0)
+ return midl;
+
dev_info(>dev,
"%s Product ID %0x:%0x Manufacturer ID %x:%x\n",
-devname,
-pid,
-ver,
-ov772x_read(client, MIDH),
-ov772x_read(client, MIDL));
+devname, pid, ver, midh, midl);
+
ret = v4l2_ctrl_handler_setup(>hdl);
done:
--
2.7.4
[PATCH v4 10/14] media: ov772x: reconstruct s_frame_interval()
This splits the s_frame_interval() in subdev video ops into selecting the
frame interval and setting up the registers.
This is a preparatory change to avoid accessing registers under power
saving mode.
Cc: Jacopo Mondi <jacopo+rene...@jmondi.org>
Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
Cc: Hans Verkuil <hans.verk...@cisco.com>
Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
Cc: Mauro Carvalho Chehab <mche...@s-opensource.com>
Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
---
* v4
- No changes
drivers/media/i2c/ov772x.c | 56 +-
1 file changed, 35 insertions(+), 21 deletions(-)
diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
index edc013d..7ea157e 100644
--- a/drivers/media/i2c/ov772x.c
+++ b/drivers/media/i2c/ov772x.c
@@ -617,25 +617,16 @@ static int ov772x_s_stream(struct v4l2_subdev *sd, int
enable)
return 0;
}
-static int ov772x_set_frame_rate(struct ov772x_priv *priv,
-struct v4l2_fract *tpf,
-const struct ov772x_color_format *cfmt,
-const struct ov772x_win_size *win)
+static unsigned int ov772x_select_fps(struct ov772x_priv *priv,
+struct v4l2_fract *tpf)
{
- struct i2c_client *client = v4l2_get_subdevdata(>subdev);
- unsigned long fin = clk_get_rate(priv->clk);
unsigned int fps = tpf->numerator ?
tpf->denominator / tpf->numerator :
tpf->denominator;
unsigned int best_diff;
- unsigned int fsize;
- unsigned int pclk;
unsigned int diff;
unsigned int idx;
unsigned int i;
- u8 clkrc = 0;
- u8 com4 = 0;
- int ret;
/* Approximate to the closest supported frame interval. */
best_diff = ~0L;
@@ -646,7 +637,25 @@ static int ov772x_set_frame_rate(struct ov772x_priv *priv,
best_diff = diff;
}
}
- fps = ov772x_frame_intervals[idx];
+
+ return ov772x_frame_intervals[idx];
+}
+
+static int ov772x_set_frame_rate(struct ov772x_priv *priv,
+unsigned int fps,
+const struct ov772x_color_format *cfmt,
+const struct ov772x_win_size *win)
+{
+ struct i2c_client *client = v4l2_get_subdevdata(>subdev);
+ unsigned long fin = clk_get_rate(priv->clk);
+ unsigned int fsize;
+ unsigned int pclk;
+ unsigned int best_diff;
+ unsigned int diff;
+ unsigned int i;
+ u8 clkrc = 0;
+ u8 com4 = 0;
+ int ret;
/* Use image size (with blankings) to calculate desired pixel clock. */
switch (cfmt->com7 & OFMT_MASK) {
@@ -711,10 +720,6 @@ static int ov772x_set_frame_rate(struct ov772x_priv *priv,
if (ret < 0)
return ret;
- tpf->numerator = 1;
- tpf->denominator = fps;
- priv->fps = tpf->denominator;
-
return 0;
}
@@ -735,8 +740,20 @@ static int ov772x_s_frame_interval(struct v4l2_subdev *sd,
{
struct ov772x_priv *priv = to_ov772x(sd);
struct v4l2_fract *tpf = >interval;
+ unsigned int fps;
+ int ret;
+
+ fps = ov772x_select_fps(priv, tpf);
+
+ ret = ov772x_set_frame_rate(priv, fps, priv->cfmt, priv->win);
+ if (ret)
+ return ret;
- return ov772x_set_frame_rate(priv, tpf, priv->cfmt, priv->win);
+ tpf->numerator = 1;
+ tpf->denominator = fps;
+ priv->fps = fps;
+
+ return 0;
}
static int ov772x_s_ctrl(struct v4l2_ctrl *ctrl)
@@ -993,7 +1010,6 @@ static int ov772x_set_params(struct ov772x_priv *priv,
const struct ov772x_win_size *win)
{
struct i2c_client *client = v4l2_get_subdevdata(>subdev);
- struct v4l2_fract tpf;
int ret;
u8 val;
@@ -1075,9 +1091,7 @@ static int ov772x_set_params(struct ov772x_priv *priv,
goto ov772x_set_fmt_error;
/* COM4, CLKRC: Set pixel clock and framerate. */
- tpf.numerator = 1;
- tpf.denominator = priv->fps;
- ret = ov772x_set_frame_rate(priv, , cfmt, win);
+ ret = ov772x_set_frame_rate(priv, priv->fps, cfmt, win);
if (ret < 0)
goto ov772x_set_fmt_error;
--
2.7.4
[PATCH v4 00/14] media: ov772x: support media controller, device tree probing, etc.
This patchset includes support media controller, sub-device interface, device tree probing and other miscellanuous changes for ov772x driver. * v4 (thanks to Laurent Pinchart) - Add Reviewed-by: lines - Correct setting of banding filter (New) - Omit consumer ID when getting clock reference (New) - Use v4l2_ctrl to get current control value (New) - Correctly restore the controls changed under power saving mode * v3 (thanks to Sakari Ailus and Jacopo Mondi) - Reorder the patches - Add Reviewed-by: lines - Remove I2C_CLIENT_SCCB flag set as it isn't needed anymore - Fix typo in the commit log - Return without resetting if ov772x_edgectrl() failed - Update the error message for missing platform data - Rename mutex name from power_lock to lock - Add warning for duplicated s_power call - Add newlines before labels - Remove __v4l2_ctrl_handler_setup in s_power() as it causes duplicated register settings - Make set_fmt() return -EBUSY while streaming (New) * v2 (thanks to Jacopo Mondi) - Replace the implementation of ov772x_read() instead of adding an alternative method - Assign the ov772x_read() return value to pid and ver directly - Do the same for MIDH and MIDL - Move video_probe() before the entity initialization and remove the #ifdef around the media_entity_cleanup() - Use generic names for reset and powerdown gpios (New) - Add "dt-bindings:" in the subject - Add a brief description of the sensor - Update the GPIO names - Indicate the GPIO active level - Add missing NULL checks for priv->info - Leave the check for the missing platform data if legacy platform data probe is used. - Handle nested s_power() calls (New) - Reconstruct s_frame_interval() (New) - Avoid accessing registers Akinobu Mita (14): media: dt-bindings: ov772x: add device tree binding media: ov772x: correct setting of banding filter media: ov772x: allow i2c controllers without I2C_FUNC_PROTOCOL_MANGLING media: ov772x: add checks for register read errors media: ov772x: add media controller support media: ov772x: use generic names for reset and powerdown gpios media: ov772x: omit consumer ID when getting clock reference media: ov772x: support device tree probing media: ov772x: handle nested s_power() calls media: ov772x: reconstruct s_frame_interval() media: ov772x: use v4l2_ctrl to get current control value media: ov772x: avoid accessing registers under power saving mode media: ov772x: make set_fmt() return -EBUSY while streaming media: ov772x: create subdevice device node .../devicetree/bindings/media/i2c/ov772x.txt | 40 +++ MAINTAINERS| 1 + arch/sh/boards/mach-migor/setup.c | 7 +- drivers/media/i2c/ov772x.c | 351 +++-- 4 files changed, 302 insertions(+), 97 deletions(-) create mode 100644 Documentation/devicetree/bindings/media/i2c/ov772x.txt Cc: Jacopo Mondi <jacopo+rene...@jmondi.org> Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com> Cc: Hans Verkuil <hans.verk...@cisco.com> Cc: Sakari Ailus <sakari.ai...@linux.intel.com> Cc: Mauro Carvalho Chehab <mche...@s-opensource.com> Cc: Rob Herring <robh...@kernel.org> Cc: Wolfram Sang <w...@the-dreams.de> -- 2.7.4
[PATCH v4 01/14] media: dt-bindings: ov772x: add device tree binding
This adds a device tree binding documentation for OV7720/OV7725 sensor.
Cc: Jacopo Mondi <jacopo+rene...@jmondi.org>
Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
Cc: Hans Verkuil <hans.verk...@cisco.com>
Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
Cc: Mauro Carvalho Chehab <mche...@s-opensource.com>
Cc: Rob Herring <robh...@kernel.org>
Reviewed-by: Rob Herring <r...@kernel.org>
Reviewed-by: Jacopo Mondi <jac...@jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
---
* v4
- Add Reviewed-by: line
- Omit clock-names property
.../devicetree/bindings/media/i2c/ov772x.txt | 40 ++
MAINTAINERS| 1 +
2 files changed, 41 insertions(+)
create mode 100644 Documentation/devicetree/bindings/media/i2c/ov772x.txt
diff --git a/Documentation/devicetree/bindings/media/i2c/ov772x.txt
b/Documentation/devicetree/bindings/media/i2c/ov772x.txt
new file mode 100644
index 000..0b3ede5
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ov772x.txt
@@ -0,0 +1,40 @@
+* Omnivision OV7720/OV7725 CMOS sensor
+
+The Omnivision OV7720/OV7725 sensor supports multiple resolutions output,
+such as VGA, QVGA, and any size scaling down from CIF to 40x30. It also can
+support the YUV422, RGB565/555/444, GRB422 or raw RGB output formats.
+
+Required Properties:
+- compatible: shall be one of
+ "ovti,ov7720"
+ "ovti,ov7725"
+- clocks: reference to the xclk input clock.
+
+Optional Properties:
+- reset-gpios: reference to the GPIO connected to the RSTB pin which is
+ active low, if any.
+- powerdown-gpios: reference to the GPIO connected to the PWDN pin which is
+ active high, if any.
+
+The device node shall contain one 'port' child node with one child 'endpoint'
+subnode for its digital output video port, in accordance with the video
+interface bindings defined in Documentation/devicetree/bindings/media/
+video-interfaces.txt.
+
+Example:
+
+ {
+ ov772x: camera@21 {
+ compatible = "ovti,ov7725";
+ reg = <0x21>;
+ reset-gpios = <_gpio_0 0 GPIO_ACTIVE_LOW>;
+ powerdown-gpios = <_gpio_0 1 GPIO_ACTIVE_LOW>;
+ clocks = <>;
+
+ port {
+ ov772x_0: endpoint {
+ remote-endpoint = <_in0>;
+ };
+ };
+ };
+};
diff --git a/MAINTAINERS b/MAINTAINERS
index db2bc3f..f39d78b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10363,6 +10363,7 @@ T: git git://linuxtv.org/media_tree.git
S: Odd fixes
F: drivers/media/i2c/ov772x.c
F: include/media/i2c/ov772x.h
+F: Documentation/devicetree/bindings/media/i2c/ov772x.txt
OMNIVISION OV7740 SENSOR DRIVER
M: Wenyou Yang <wenyou.y...@microchip.com>
--
2.7.4
[PATCH v4 08/14] media: ov772x: support device tree probing
The ov772x driver currently only supports legacy platform data probe.
This change enables device tree probing.
Note that the platform data probe can select auto or manual edge control
mode, but the device tree probling can only select auto edge control mode
for now.
Cc: Jacopo Mondi <jacopo+rene...@jmondi.org>
Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
Cc: Hans Verkuil <hans.verk...@cisco.com>
Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
Cc: Mauro Carvalho Chehab <mche...@s-opensource.com>
Reviewed-by: Jacopo Mondi <jac...@jmondi.org>
Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
---
* v4
- No changes
drivers/media/i2c/ov772x.c | 64 --
1 file changed, 45 insertions(+), 19 deletions(-)
diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
index f939e28..621149a 100644
--- a/drivers/media/i2c/ov772x.c
+++ b/drivers/media/i2c/ov772x.c
@@ -749,13 +749,13 @@ static int ov772x_s_ctrl(struct v4l2_ctrl *ctrl)
case V4L2_CID_VFLIP:
val = ctrl->val ? VFLIP_IMG : 0x00;
priv->flag_vflip = ctrl->val;
- if (priv->info->flags & OV772X_FLAG_VFLIP)
+ if (priv->info && (priv->info->flags & OV772X_FLAG_VFLIP))
val ^= VFLIP_IMG;
return ov772x_mask_set(client, COM3, VFLIP_IMG, val);
case V4L2_CID_HFLIP:
val = ctrl->val ? HFLIP_IMG : 0x00;
priv->flag_hflip = ctrl->val;
- if (priv->info->flags & OV772X_FLAG_HFLIP)
+ if (priv->info && (priv->info->flags & OV772X_FLAG_HFLIP))
val ^= HFLIP_IMG;
return ov772x_mask_set(client, COM3, HFLIP_IMG, val);
case V4L2_CID_BAND_STOP_FILTER:
@@ -914,19 +914,14 @@ static void ov772x_select_params(const struct
v4l2_mbus_framefmt *mf,
*win = ov772x_select_win(mf->width, mf->height);
}
-static int ov772x_set_params(struct ov772x_priv *priv,
-const struct ov772x_color_format *cfmt,
-const struct ov772x_win_size *win)
+static int ov772x_edgectrl(struct ov772x_priv *priv)
{
struct i2c_client *client = v4l2_get_subdevdata(>subdev);
- struct v4l2_fract tpf;
int ret;
- u8 val;
- /* Reset hardware. */
- ov772x_reset(client);
+ if (!priv->info)
+ return 0;
- /* Edge Ctrl. */
if (priv->info->edgectrl.strength & OV772X_MANUAL_EDGE_CTRL) {
/*
* Manual Edge Control Mode.
@@ -937,19 +932,19 @@ static int ov772x_set_params(struct ov772x_priv *priv,
ret = ov772x_mask_set(client, DSPAUTO, EDGE_ACTRL, 0x00);
if (ret < 0)
- goto ov772x_set_fmt_error;
+ return ret;
ret = ov772x_mask_set(client,
EDGE_TRSHLD, OV772X_EDGE_THRESHOLD_MASK,
priv->info->edgectrl.threshold);
if (ret < 0)
- goto ov772x_set_fmt_error;
+ return ret;
ret = ov772x_mask_set(client,
EDGE_STRNGT, OV772X_EDGE_STRENGTH_MASK,
priv->info->edgectrl.strength);
if (ret < 0)
- goto ov772x_set_fmt_error;
+ return ret;
} else if (priv->info->edgectrl.upper > priv->info->edgectrl.lower) {
/*
@@ -961,15 +956,35 @@ static int ov772x_set_params(struct ov772x_priv *priv,
EDGE_UPPER, OV772X_EDGE_UPPER_MASK,
priv->info->edgectrl.upper);
if (ret < 0)
- goto ov772x_set_fmt_error;
+ return ret;
ret = ov772x_mask_set(client,
EDGE_LOWER, OV772X_EDGE_LOWER_MASK,
priv->info->edgectrl.lower);
if (ret < 0)
- goto ov772x_set_fmt_error;
+ return ret;
}
+ return 0;
+}
+
+static int ov772x_set_params(struct ov772x_priv *priv,
+const struct ov772x_color_format *cfmt,
+const struct ov772x_win_size *win)
+{
+ struct i2c_client *client = v4l2_get_subdevdata(>subdev);
+ struct v4l2_fract tpf;
+ int ret;
+ u8 val;
+
+ /* Reset hardware. */
+ ov772x_reset(client);
+
+ /* Edge Ctrl. */
+ ret = ov772x_edgectrl(priv);
+ if (ret < 0)
+ return ret;
+
/* Format
[PATCH v4 05/14] media: ov772x: add media controller support
Create a source pad and set the media controller type to the sensor.
Cc: Jacopo Mondi <jacopo+rene...@jmondi.org>
Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
Cc: Hans Verkuil <hans.verk...@cisco.com>
Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
Cc: Mauro Carvalho Chehab <mche...@s-opensource.com>
Reviewed-by: Jacopo Mondi <jac...@jmondi.org>
Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
---
* v4
- No changes
drivers/media/i2c/ov772x.c | 16 +++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
index 3fdbe64..bb5327f 100644
--- a/drivers/media/i2c/ov772x.c
+++ b/drivers/media/i2c/ov772x.c
@@ -424,6 +424,9 @@ struct ov772x_priv {
/* band_filter = COM8[5] ? 256 - BDBASE : 0 */
unsigned shortband_filter;
unsigned int fps;
+#ifdef CONFIG_MEDIA_CONTROLLER
+ struct media_pad pad;
+#endif
};
/*
@@ -1316,16 +1319,26 @@ static int ov772x_probe(struct i2c_client *client,
if (ret < 0)
goto error_gpio_put;
+#ifdef CONFIG_MEDIA_CONTROLLER
+ priv->pad.flags = MEDIA_PAD_FL_SOURCE;
+ priv->subdev.entity.function = MEDIA_ENT_F_CAM_SENSOR;
+ ret = media_entity_pads_init(>subdev.entity, 1, >pad);
+ if (ret < 0)
+ goto error_gpio_put;
+#endif
+
priv->cfmt = _cfmts[0];
priv->win = _win_sizes[0];
priv->fps = 15;
ret = v4l2_async_register_subdev(>subdev);
if (ret)
- goto error_gpio_put;
+ goto error_entity_cleanup;
return 0;
+error_entity_cleanup:
+ media_entity_cleanup(>subdev.entity);
error_gpio_put:
if (priv->pwdn_gpio)
gpiod_put(priv->pwdn_gpio);
@@ -1341,6 +1354,7 @@ static int ov772x_remove(struct i2c_client *client)
{
struct ov772x_priv *priv = to_ov772x(i2c_get_clientdata(client));
+ media_entity_cleanup(>subdev.entity);
clk_put(priv->clk);
if (priv->pwdn_gpio)
gpiod_put(priv->pwdn_gpio);
--
2.7.4
[PATCH v4 07/14] media: ov772x: omit consumer ID when getting clock reference
Currently the ov772x driver obtains a clock with a specific consumer ID.
As there's a single clock for this driver, we could omit clock-names
property in device tree by passing NULL as a consumer ID to clk_get().
Cc: Jacopo Mondi <jacopo+rene...@jmondi.org>
Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
Cc: Hans Verkuil <hans.verk...@cisco.com>
Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
Cc: Mauro Carvalho Chehab <mche...@s-opensource.com>
Suggested-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
Tested-by: Jacopo Mondi <jacopo+rene...@jmondi.org>
Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
---
* v4
- New patch
arch/sh/boards/mach-migor/setup.c | 2 +-
drivers/media/i2c/ov772x.c| 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/sh/boards/mach-migor/setup.c
b/arch/sh/boards/mach-migor/setup.c
index 73b9ee4..11f8001 100644
--- a/arch/sh/boards/mach-migor/setup.c
+++ b/arch/sh/boards/mach-migor/setup.c
@@ -593,7 +593,7 @@ static int __init migor_devices_setup(void)
}
/* Add a clock alias for ov7725 xclk source. */
- clk_add_alias("xclk", "0-0021", "video_clk", NULL);
+ clk_add_alias(NULL, "0-0021", "video_clk", NULL);
/* Register GPIOs for video sources. */
gpiod_add_lookup_table(_gpios);
diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
index 97a65ce..f939e28 100644
--- a/drivers/media/i2c/ov772x.c
+++ b/drivers/media/i2c/ov772x.c
@@ -1300,7 +1300,7 @@ static int ov772x_probe(struct i2c_client *client,
if (priv->hdl.error)
return priv->hdl.error;
- priv->clk = clk_get(>dev, "xclk");
+ priv->clk = clk_get(>dev, NULL);
if (IS_ERR(priv->clk)) {
dev_err(>dev, "Unable to get xclk clock\n");
ret = PTR_ERR(priv->clk);
--
2.7.4
[PATCH v4 03/14] media: ov772x: allow i2c controllers without I2C_FUNC_PROTOCOL_MANGLING
The ov772x driver only works when the i2c controller have
I2C_FUNC_PROTOCOL_MANGLING. However, many i2c controller drivers don't
support it.
The reason that the ov772x requires I2C_FUNC_PROTOCOL_MANGLING is that
it doesn't support repeated starts.
This changes the reading ov772x register method so that it doesn't
require I2C_FUNC_PROTOCOL_MANGLING by calling two separated i2c messages.
Cc: Jacopo Mondi <jacopo+rene...@jmondi.org>
Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
Cc: Hans Verkuil <hans.verk...@cisco.com>
Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
Cc: Mauro Carvalho Chehab <mche...@s-opensource.com>
Cc: Wolfram Sang <w...@the-dreams.de>
Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
---
* v4
- No changes
drivers/media/i2c/ov772x.c | 20 ++--
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
index e255070..b6223bf 100644
--- a/drivers/media/i2c/ov772x.c
+++ b/drivers/media/i2c/ov772x.c
@@ -542,9 +542,19 @@ static struct ov772x_priv *to_ov772x(struct v4l2_subdev
*sd)
return container_of(sd, struct ov772x_priv, subdev);
}
-static inline int ov772x_read(struct i2c_client *client, u8 addr)
+static int ov772x_read(struct i2c_client *client, u8 addr)
{
- return i2c_smbus_read_byte_data(client, addr);
+ int ret;
+ u8 val;
+
+ ret = i2c_master_send(client, , 1);
+ if (ret < 0)
+ return ret;
+ ret = i2c_master_recv(client, , 1);
+ if (ret < 0)
+ return ret;
+
+ return val;
}
static inline int ov772x_write(struct i2c_client *client, u8 addr, u8 value)
@@ -1255,13 +1265,11 @@ static int ov772x_probe(struct i2c_client *client,
return -EINVAL;
}
- if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA |
- I2C_FUNC_PROTOCOL_MANGLING)) {
+ if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA)) {
dev_err(>dev,
- "I2C-Adapter doesn't support SMBUS_BYTE_DATA or
PROTOCOL_MANGLING\n");
+ "I2C-Adapter doesn't support SMBUS_BYTE_DATA\n");
return -EIO;
}
- client->flags |= I2C_CLIENT_SCCB;
priv = devm_kzalloc(>dev, sizeof(*priv), GFP_KERNEL);
if (!priv)
--
2.7.4
[PATCH v4 13/14] media: ov772x: make set_fmt() return -EBUSY while streaming
The ov772x driver is going to offer a V4L2 sub-device interface, so
changing the output data format on this sub-device can be made anytime.
However, the request is preferred to fail while the video stream on the
device is active.
Cc: Jacopo Mondi <jacopo+rene...@jmondi.org>
Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
Cc: Hans Verkuil <hans.verk...@cisco.com>
Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
Cc: Mauro Carvalho Chehab <mche...@s-opensource.com>
Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
---
* v4
- No changes
drivers/media/i2c/ov772x.c | 35 ++-
1 file changed, 26 insertions(+), 9 deletions(-)
diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
index bd37169..f3c4f78 100644
--- a/drivers/media/i2c/ov772x.c
+++ b/drivers/media/i2c/ov772x.c
@@ -424,9 +424,10 @@ struct ov772x_priv {
/* band_filter = COM8[5] ? 256 - BDBASE : 0 */
struct v4l2_ctrl *band_filter_ctrl;
unsigned int fps;
- /* lock to protect power_count */
+ /* lock to protect power_count and streaming */
struct mutex lock;
int power_count;
+ int streaming;
#ifdef CONFIG_MEDIA_CONTROLLER
struct media_pad pad;
#endif
@@ -603,18 +604,28 @@ static int ov772x_s_stream(struct v4l2_subdev *sd, int
enable)
{
struct i2c_client *client = v4l2_get_subdevdata(sd);
struct ov772x_priv *priv = to_ov772x(sd);
+ int ret = 0;
- if (!enable) {
- ov772x_mask_set(client, COM2, SOFT_SLEEP_MODE, SOFT_SLEEP_MODE);
- return 0;
- }
+ mutex_lock(>lock);
- ov772x_mask_set(client, COM2, SOFT_SLEEP_MODE, 0);
+ if (priv->streaming == enable)
+ goto done;
- dev_dbg(>dev, "format %d, win %s\n",
- priv->cfmt->code, priv->win->name);
+ ret = ov772x_mask_set(client, COM2, SOFT_SLEEP_MODE,
+ enable ? 0 : SOFT_SLEEP_MODE);
+ if (ret)
+ goto done;
- return 0;
+ if (enable) {
+ dev_dbg(>dev, "format %d, win %s\n",
+ priv->cfmt->code, priv->win->name);
+ }
+ priv->streaming = enable;
+
+done:
+ mutex_unlock(>lock);
+
+ return ret;
}
static unsigned int ov772x_select_fps(struct ov772x_priv *priv,
@@ -1222,6 +1233,12 @@ static int ov772x_set_fmt(struct v4l2_subdev *sd,
}
mutex_lock(>lock);
+
+ if (priv->streaming) {
+ ret = -EBUSY;
+ goto error;
+ }
+
/*
* If the device is not powered up by the host driver do
* not apply any changes to H/W at this time. Instead
--
2.7.4
[PATCH v4 02/14] media: ov772x: correct setting of banding filter
The banding filter ON/OFF is controlled via bit 5 of COM8 register. It
is attempted to be enabled in ov772x_set_params() by the following line.
ret = ov772x_mask_set(client, COM8, BNDF_ON_OFF, 1);
But this unexpectedly results disabling the banding filter, because the
mask and set bits are exclusive.
On the other hand, ov772x_s_ctrl() correctly sets the bit by:
ret = ov772x_mask_set(client, COM8, BNDF_ON_OFF, BNDF_ON_OFF);
Cc: Jacopo Mondi <jacopo+rene...@jmondi.org>
Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
Cc: Hans Verkuil <hans.verk...@cisco.com>
Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
Cc: Mauro Carvalho Chehab <mche...@s-opensource.com>
Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
---
* v4
- New patch
drivers/media/i2c/ov772x.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
index b62860c..e255070 100644
--- a/drivers/media/i2c/ov772x.c
+++ b/drivers/media/i2c/ov772x.c
@@ -1035,7 +1035,7 @@ static int ov772x_set_params(struct ov772x_priv *priv,
/* Set COM8. */
if (priv->band_filter) {
- ret = ov772x_mask_set(client, COM8, BNDF_ON_OFF, 1);
+ ret = ov772x_mask_set(client, COM8, BNDF_ON_OFF, BNDF_ON_OFF);
if (!ret)
ret = ov772x_mask_set(client, BDBASE,
0xff, 256 - priv->band_filter);
--
2.7.4
[PATCH v4 14/14] media: ov772x: create subdevice device node
Set the V4L2_SUBDEV_FL_HAS_DEVNODE flag for the subdevice so that the subdevice device node is created. Cc: Jacopo Mondi <jacopo+rene...@jmondi.org> Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com> Cc: Hans Verkuil <hans.verk...@cisco.com> Cc: Sakari Ailus <sakari.ai...@linux.intel.com> Cc: Mauro Carvalho Chehab <mche...@s-opensource.com> Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com> --- * v4 - No changes drivers/media/i2c/ov772x.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c index f3c4f78..ec45eed 100644 --- a/drivers/media/i2c/ov772x.c +++ b/drivers/media/i2c/ov772x.c @@ -1403,6 +1403,7 @@ static int ov772x_probe(struct i2c_client *client, mutex_init(>lock); v4l2_i2c_subdev_init(>subdev, client, _subdev_ops); + priv->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; v4l2_ctrl_handler_init(>hdl, 3); /* Use our mutex for the controls */ priv->hdl.lock = >lock; -- 2.7.4
[PATCH v4 09/14] media: ov772x: handle nested s_power() calls
Depending on the v4l2 driver, calling s_power() could be nested. So the
actual transitions between power saving mode and normal operation mode
should only happen at the first power on and the last power off.
This adds an s_power() nesting counter and updates the power state if the
counter is modified from 0 to != 0 or from != 0 to 0.
Cc: Jacopo Mondi <jacopo+rene...@jmondi.org>
Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
Cc: Hans Verkuil <hans.verk...@cisco.com>
Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
Cc: Mauro Carvalho Chehab <mche...@s-opensource.com>
Reviewed-by: Jacopo Mondi <jac...@jmondi.org>
Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
---
* v4
- Add Reviewed-by: line
drivers/media/i2c/ov772x.c | 34 ++
1 file changed, 30 insertions(+), 4 deletions(-)
diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
index 621149a..edc013d 100644
--- a/drivers/media/i2c/ov772x.c
+++ b/drivers/media/i2c/ov772x.c
@@ -424,6 +424,9 @@ struct ov772x_priv {
/* band_filter = COM8[5] ? 256 - BDBASE : 0 */
unsigned shortband_filter;
unsigned int fps;
+ /* lock to protect power_count */
+ struct mutex lock;
+ int power_count;
#ifdef CONFIG_MEDIA_CONTROLLER
struct media_pad pad;
#endif
@@ -871,9 +874,26 @@ static int ov772x_power_off(struct ov772x_priv *priv)
static int ov772x_s_power(struct v4l2_subdev *sd, int on)
{
struct ov772x_priv *priv = to_ov772x(sd);
+ int ret = 0;
+
+ mutex_lock(>lock);
+
+ /* If the power count is modified from 0 to != 0 or from != 0 to 0,
+* update the power state.
+*/
+ if (priv->power_count == !on)
+ ret = on ? ov772x_power_on(priv) : ov772x_power_off(priv);
+
+ if (!ret) {
+ /* Update the power count. */
+ priv->power_count += on ? 1 : -1;
+ WARN(priv->power_count < 0, "Unbalanced power count\n");
+ WARN(priv->power_count > 1, "Duplicated s_power call\n");
+ }
+
+ mutex_unlock(>lock);
- return on ? ov772x_power_on(priv) :
- ov772x_power_off(priv);
+ return ret;
}
static const struct ov772x_win_size *ov772x_select_win(u32 width, u32 height)
@@ -1303,6 +1323,7 @@ static int ov772x_probe(struct i2c_client *client,
return -ENOMEM;
priv->info = client->dev.platform_data;
+ mutex_init(>lock);
v4l2_i2c_subdev_init(>subdev, client, _subdev_ops);
v4l2_ctrl_handler_init(>hdl, 3);
@@ -1313,8 +1334,10 @@ static int ov772x_probe(struct i2c_client *client,
v4l2_ctrl_new_std(>hdl, _ctrl_ops,
V4L2_CID_BAND_STOP_FILTER, 0, 256, 1, 0);
priv->subdev.ctrl_handler = >hdl;
- if (priv->hdl.error)
- return priv->hdl.error;
+ if (priv->hdl.error) {
+ ret = priv->hdl.error;
+ goto error_mutex_destroy;
+ }
priv->clk = clk_get(>dev, NULL);
if (IS_ERR(priv->clk)) {
@@ -1362,6 +1385,8 @@ static int ov772x_probe(struct i2c_client *client,
clk_put(priv->clk);
error_ctrl_free:
v4l2_ctrl_handler_free(>hdl);
+error_mutex_destroy:
+ mutex_destroy(>lock);
return ret;
}
@@ -1376,6 +1401,7 @@ static int ov772x_remove(struct i2c_client *client)
gpiod_put(priv->pwdn_gpio);
v4l2_async_unregister_subdev(>subdev);
v4l2_ctrl_handler_free(>hdl);
+ mutex_destroy(>lock);
return 0;
}
--
2.7.4
Re: [PATCH] dt-bindings: media: xilinx: fix typo in example
2017-10-19 1:08 GMT+09:00 Laurent Pinchart <laurent.pinch...@ideasonboard.com>: > Hi Akinobu, > > Thank you for the patch. > > On Thursday, 12 October 2017 19:03:34 EEST Akinobu Mita wrote: >> Fix typo s/:/;/ >> >> Cc: Rob Herring <robh...@kernel.org> >> Cc: Hyun Kwon <hyun.k...@xilinx.com> >> Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com> >> Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com> > > Good catch. > > Reviewed-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com> > > Should I take this patch in my tree ? Yes, please do. Could you also take a look at another patch that I sent at the same time "media: xilinx-video: fix bad of_node_put() on endpoint error" ?
[PATCH 2/4] media: max2175: don't clear V4L2_SUBDEV_FL_IS_I2C
The v4l2_i2c_subdev_init() sets V4L2_SUBDEV_FL_IS_I2C flag in the subdev->flags. But this driver overwrites subdev->flags immediately after calling v4l2_i2c_subdev_init(). So V4L2_SUBDEV_FL_IS_I2C is not set after all. This stops breaking subdev->flags and preserves V4L2_SUBDEV_FL_IS_I2C. Side note: According to the comment in v4l2_device_unregister(), this is problematic only if the device is platform bus device. Device tree or ACPI based devices are not affected. Cc: Ramesh Shanmugasundaram <ramesh.shanmugasunda...@bp.renesas.com> Cc: Mauro Carvalho Chehab <mche...@s-opensource.com> Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com> --- drivers/media/i2c/max2175.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/i2c/max2175.c b/drivers/media/i2c/max2175.c index bf0e821..2f1966b 100644 --- a/drivers/media/i2c/max2175.c +++ b/drivers/media/i2c/max2175.c @@ -1345,7 +1345,7 @@ static int max2175_probe(struct i2c_client *client, v4l2_i2c_subdev_init(sd, client, _ops); ctx->client = client; - sd->flags = V4L2_SUBDEV_FL_HAS_DEVNODE; + sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; /* Controls */ hdl = >ctrl_hdl; -- 2.7.4
[PATCH 4/4] media: ov5640: don't clear V4L2_SUBDEV_FL_IS_I2C
The v4l2_i2c_subdev_init() sets V4L2_SUBDEV_FL_IS_I2C flag in the subdev->flags. But this driver overwrites subdev->flags immediately after calling v4l2_i2c_subdev_init(). So V4L2_SUBDEV_FL_IS_I2C is not set after all. This stops breaking subdev->flags and preserves V4L2_SUBDEV_FL_IS_I2C. Side note: According to the comment in v4l2_device_unregister(), this is problematic only if the device is platform bus device. Device tree or ACPI based devices are not affected. Cc: Sakari Ailus <sakari.ai...@linux.intel.com> Cc: Mauro Carvalho Chehab <mche...@s-opensource.com> Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com> --- drivers/media/i2c/ov5640.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c index 39a2269..c89ed66 100644 --- a/drivers/media/i2c/ov5640.c +++ b/drivers/media/i2c/ov5640.c @@ -2271,7 +2271,7 @@ static int ov5640_probe(struct i2c_client *client, v4l2_i2c_subdev_init(>sd, client, _subdev_ops); - sensor->sd.flags = V4L2_SUBDEV_FL_HAS_DEVNODE; + sensor->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; sensor->pad.flags = MEDIA_PAD_FL_SOURCE; sensor->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR; ret = media_entity_pads_init(>sd.entity, 1, >pad); -- 2.7.4
[PATCH 3/4] media: ov2640: don't clear V4L2_SUBDEV_FL_IS_I2C
The v4l2_i2c_subdev_init() sets V4L2_SUBDEV_FL_IS_I2C flag in the subdev->flags. But this driver overwrites subdev->flags immediately after calling v4l2_i2c_subdev_init(). So V4L2_SUBDEV_FL_IS_I2C is not set after all. This stops breaking subdev->flags and preserves V4L2_SUBDEV_FL_IS_I2C. Side note: According to the comment in v4l2_device_unregister(), this is problematic only if the device is platform bus device. Device tree or ACPI based devices are not affected. Cc: Mauro Carvalho Chehab <mche...@s-opensource.com> Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com> --- drivers/media/i2c/ov2640.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/i2c/ov2640.c b/drivers/media/i2c/ov2640.c index e6d0c1f..5b035d1 100644 --- a/drivers/media/i2c/ov2640.c +++ b/drivers/media/i2c/ov2640.c @@ -1116,7 +1116,7 @@ static int ov2640_probe(struct i2c_client *client, goto err_clk; v4l2_i2c_subdev_init(>subdev, client, _subdev_ops); - priv->subdev.flags = V4L2_SUBDEV_FL_HAS_DEVNODE; + priv->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; v4l2_ctrl_handler_init(>hdl, 2); v4l2_ctrl_new_std(>hdl, _ctrl_ops, V4L2_CID_VFLIP, 0, 1, 1, 0); -- 2.7.4
[PATCH 1/4] media: adv7180: don't clear V4L2_SUBDEV_FL_IS_I2C
The v4l2_i2c_subdev_init() sets V4L2_SUBDEV_FL_IS_I2C flag in the subdev->flags. But this driver overwrites subdev->flags immediately after calling v4l2_i2c_subdev_init(). So V4L2_SUBDEV_FL_IS_I2C is not set after all. This stops breaking subdev->flags and preserves V4L2_SUBDEV_FL_IS_I2C. Side note: According to the comment in v4l2_device_unregister(), this is problematic only if the device is platform bus device. Device tree or ACPI based devices are not affected. Cc: Lars-Peter Clausen <l...@metafoo.de> Cc: Mauro Carvalho Chehab <mche...@s-opensource.com> Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com> --- drivers/media/i2c/adv7180.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c index 3df28f2..6fb818a 100644 --- a/drivers/media/i2c/adv7180.c +++ b/drivers/media/i2c/adv7180.c @@ -1328,7 +1328,7 @@ static int adv7180_probe(struct i2c_client *client, state->input = 0; sd = >sd; v4l2_i2c_subdev_init(sd, client, _ops); - sd->flags = V4L2_SUBDEV_FL_HAS_DEVNODE | V4L2_SUBDEV_FL_HAS_EVENTS; + sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE | V4L2_SUBDEV_FL_HAS_EVENTS; ret = adv7180_init_controls(state); if (ret) -- 2.7.4
[PATCH 0/4] media: don't clear V4L2_SUBDEV_FL_IS_I2C
The v4l2_i2c_subdev_init() sets V4L2_SUBDEV_FL_IS_I2C flag in the subdev->flags. But some drivers overwrite subdev->flags immediately after calling v4l2_i2c_subdev_init(). So V4L2_SUBDEV_FL_IS_I2C is not set after all. This patch series fixes the problem for each driver. Side note: According to the comment in v4l2_device_unregister(), this is problematic only if the device is platform bus device. Device tree or ACPI based devices are not affected. Akinobu Mita (4): media: adv7180: don't clear V4L2_SUBDEV_FL_IS_I2C media: max2175: don't clear V4L2_SUBDEV_FL_IS_I2C media: ov2640: don't clear V4L2_SUBDEV_FL_IS_I2C media: ov5640: don't clear V4L2_SUBDEV_FL_IS_I2C drivers/media/i2c/adv7180.c | 2 +- drivers/media/i2c/max2175.c | 2 +- drivers/media/i2c/ov2640.c | 2 +- drivers/media/i2c/ov5640.c | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) Cc: Lars-Peter Clausen <l...@metafoo.de> Cc: Mauro Carvalho Chehab <mche...@s-opensource.com> Cc: Ramesh Shanmugasundaram <ramesh.shanmugasunda...@bp.renesas.com> Cc: Sakari Ailus <sakari.ai...@linux.intel.com> -- 2.7.4
[PATCH] media: ov9650: remove unnecessary terminated entry in menu items array
The test_pattern_menu[] array has two valid items and a null terminated
item. So the control's maximum value which is passed to
v4l2_ctrl_new_std_menu_items() should be one. However,
'ARRAY_SIZE(test_pattern_menu) - 1' is actually passed and it's not
correct.
Fix it by removing unnecessary terminated entry and let the correct
control's maximum value be passed to v4l2_ctrl_new_std_menu_items().
Cc: Sylwester Nawrocki <sylvester.nawro...@gmail.com>
Cc: Mauro Carvalho Chehab <mche...@s-opensource.com>
Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
---
drivers/media/i2c/ov9650.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/media/i2c/ov9650.c b/drivers/media/i2c/ov9650.c
index 6ffb460..69433e1 100644
--- a/drivers/media/i2c/ov9650.c
+++ b/drivers/media/i2c/ov9650.c
@@ -985,7 +985,6 @@ static const struct v4l2_ctrl_ops ov965x_ctrl_ops = {
static const char * const test_pattern_menu[] = {
"Disabled",
"Color bars",
- NULL
};
static int ov965x_initialize_controls(struct ov965x *ov965x)
--
2.7.4
[PATCH] media: ov9650: support device tree probing
The ov9650 driver currently only supports legacy platform data probe.
This change adds device tree probing.
There has been an attempt to add device tree support for ov9650 driver
by Hugues Fruchet as a part of the patchset that adds support of OV9655
camera (http://www.spinics.net/lists/linux-media/msg117903.html), but
it wasn't merged into mainline because creating a separate driver for
OV9655 is preferred.
This is very similar to Hugues's patch, but not supporting new device.
Cc: H. Nikolaus Schaller <h...@goldelico.com>
Cc: Hugues Fruchet <hugues.fruc...@st.com>
Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
Cc: Mauro Carvalho Chehab <mche...@s-opensource.com>
Cc: Rob Herring <r...@kernel.org>
Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
---
.../devicetree/bindings/media/i2c/ov9650.txt | 35 +++
drivers/media/i2c/ov9650.c | 107 -
2 files changed, 117 insertions(+), 25 deletions(-)
create mode 100644 Documentation/devicetree/bindings/media/i2c/ov9650.txt
diff --git a/Documentation/devicetree/bindings/media/i2c/ov9650.txt
b/Documentation/devicetree/bindings/media/i2c/ov9650.txt
new file mode 100644
index 000..aa5024d
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ov9650.txt
@@ -0,0 +1,35 @@
+* Omnivision OV9650/OV9652 CMOS sensor
+
+Required Properties:
+- compatible: should be one of
+ "ovti,ov9650"
+ "ovti,ov9652"
+- clocks: reference to the xvclk input clock.
+
+Optional Properties:
+- reset-gpios: reference to the GPIO connected to the resetb pin, if any.
+ Active is high.
+- powerdown-gpios: reference to the GPIO connected to the pwdn pin, if any.
+ Active is high.
+
+The device node must contain one 'port' child node for its digital output
+video port, in accordance with the video interface bindings defined in
+Documentation/devicetree/bindings/media/video-interfaces.txt.
+
+Example:
+
+ {
+ ov9650: camera@30 {
+ compatible = "ovti,ov9650";
+ reg = <0x30>;
+ reset-gpios = <_gpio_0 0 GPIO_ACTIVE_HIGH>;
+ powerdown-gpios = <_gpio_0 1 GPIO_ACTIVE_HIGH>;
+ clocks = <>;
+
+ port {
+ ov9650_0: endpoint {
+ remote-endpoint = <_in0>;
+ };
+ };
+ };
+};
diff --git a/drivers/media/i2c/ov9650.c b/drivers/media/i2c/ov9650.c
index 69433e1..1affdc0 100644
--- a/drivers/media/i2c/ov9650.c
+++ b/drivers/media/i2c/ov9650.c
@@ -11,8 +11,10 @@
* it under the terms of the GNU General Public License version 2 as
* published by the Free Software Foundation.
*/
+#include
#include
#include
+#include
#include
#include
#include
@@ -249,9 +251,10 @@ struct ov965x {
struct v4l2_subdev sd;
struct media_pad pad;
enum v4l2_mbus_type bus_type;
- int gpios[NUM_GPIOS];
+ struct gpio_desc *gpios[NUM_GPIOS];
/* External master clock frequency */
unsigned long mclk_frequency;
+ struct clk *clk;
/* Protects the struct fields below */
struct mutex lock;
@@ -513,10 +516,9 @@ static int ov965x_set_color_matrix(struct ov965x *ov965x)
return 0;
}
-static void ov965x_gpio_set(int gpio, int val)
+static void ov965x_gpio_set(struct gpio_desc *gpio, int val)
{
- if (gpio_is_valid(gpio))
- gpio_set_value(gpio, val);
+ gpiod_set_value_cansleep(gpio, val);
}
static void __ov965x_set_power(struct ov965x *ov965x, int on)
@@ -1408,16 +1410,17 @@ static const struct v4l2_subdev_ops ov965x_subdev_ops =
{
/*
* Reset and power down GPIOs configuration
*/
-static int ov965x_configure_gpios(struct ov965x *ov965x,
- const struct ov9650_platform_data *pdata)
+static int ov965x_configure_gpios_pdata(struct ov965x *ov965x,
+ const struct ov9650_platform_data *pdata)
{
int ret, i;
+ int gpios[NUM_GPIOS];
- ov965x->gpios[GPIO_PWDN] = pdata->gpio_pwdn;
- ov965x->gpios[GPIO_RST] = pdata->gpio_reset;
+ gpios[GPIO_PWDN] = pdata->gpio_pwdn;
+ gpios[GPIO_RST] = pdata->gpio_reset;
for (i = 0; i < ARRAY_SIZE(ov965x->gpios); i++) {
- int gpio = ov965x->gpios[i];
+ int gpio = gpios[i];
if (!gpio_is_valid(gpio))
continue;
@@ -1427,14 +1430,52 @@ static int ov965x_configure_gpios(struct ov965x *ov965x,
return ret;
v4l2_dbg(1, debug, >sd, "set gpio %d to 1\n", gpio);
- gpio_set_value(gpio, 1);
+ gpio_set_value_cansleep(gpio, 1);
gpio_export(gpio, 0);
- ov965x->gpios[i] = gpio;
+ ov965x->gpios[i] = gpio_to_desc(gp
[PATCH] media: xilinx-video: support pipeline power management
This enables pipeline power management for Xilinx Video IP driver.
Some V4L2 subdevices are put their power status into power down mode
after their probe function, and require to be powered up by calling
s_power() operation when the pipeline is in use.
So this change is necessary if the video pipeline contains such a V4L2
subdevice.
Cc: Hyun Kwon <hyun.k...@xilinx.com>
Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
---
drivers/media/platform/xilinx/xilinx-dma.c | 35 +++--
drivers/media/platform/xilinx/xilinx-vipp.c | 7 +-
2 files changed, 39 insertions(+), 3 deletions(-)
diff --git a/drivers/media/platform/xilinx/xilinx-dma.c
b/drivers/media/platform/xilinx/xilinx-dma.c
index 522cdfd..cd0b846 100644
--- a/drivers/media/platform/xilinx/xilinx-dma.c
+++ b/drivers/media/platform/xilinx/xilinx-dma.c
@@ -22,6 +22,7 @@
#include
#include
#include
+#include
#include
#include
@@ -644,11 +645,41 @@ static const struct v4l2_ioctl_ops xvip_dma_ioctl_ops = {
* V4L2 file operations
*/
+static int xvip_dma_open(struct file *file)
+{
+ struct video_device *vdev = video_devdata(file);
+ int ret;
+
+ ret = v4l2_pipeline_pm_use(>entity, 1);
+ if (ret)
+ return ret;
+
+ ret = v4l2_fh_open(file);
+ if (ret)
+ v4l2_pipeline_pm_use(>entity, 0);
+
+ return ret;
+}
+
+static int xvip_dma_release(struct file *file)
+{
+ struct video_device *vdev = video_devdata(file);
+ int ret;
+
+ ret = vb2_fop_release(file);
+ if (ret)
+ return ret;
+
+ v4l2_pipeline_pm_use(>entity, 0);
+
+ return 0;
+}
+
static const struct v4l2_file_operations xvip_dma_fops = {
.owner = THIS_MODULE,
.unlocked_ioctl = video_ioctl2,
- .open = v4l2_fh_open,
- .release= vb2_fop_release,
+ .open = xvip_dma_open,
+ .release= xvip_dma_release,
.poll = vb2_fop_poll,
.mmap = vb2_fop_mmap,
};
diff --git a/drivers/media/platform/xilinx/xilinx-vipp.c
b/drivers/media/platform/xilinx/xilinx-vipp.c
index f4c3e48..6d098e3 100644
--- a/drivers/media/platform/xilinx/xilinx-vipp.c
+++ b/drivers/media/platform/xilinx/xilinx-vipp.c
@@ -23,6 +23,7 @@
#include
#include
#include
+#include
#include "xilinx-dma.h"
#include "xilinx-vipp.h"
@@ -573,6 +574,10 @@ static void xvip_composite_v4l2_cleanup(struct
xvip_composite_device *xdev)
media_device_cleanup(>media_dev);
}
+static const struct media_device_ops xvip_media_ops = {
+ .link_notify = v4l2_pipeline_link_notify,
+};
+
static int xvip_composite_v4l2_init(struct xvip_composite_device *xdev)
{
int ret;
@@ -581,7 +586,7 @@ static int xvip_composite_v4l2_init(struct
xvip_composite_device *xdev)
strlcpy(xdev->media_dev.model, "Xilinx Video Composite Device",
sizeof(xdev->media_dev.model));
xdev->media_dev.hw_revision = 0;
-
+ xdev->media_dev.ops = _media_ops;
media_device_init(>media_dev);
xdev->v4l2_dev.mdev = >media_dev;
--
2.7.4
[PATCH v2 0/2] media: ov9650: support device tree probing
This patchset adds device tree probing for ov9650 driver. This contains an actual driver change and a newly added binding documentation part. * Changelog v2 - Split binding documentation, suggested by Rob Herring and Jacopo Mondi - Improve the wording for compatible property in the binding documentation, suggested by Jacopo Mondi - Improve the description for the device node in the binding documentation, suggested by Sakari Ailus - Remove ov965x_gpio_set() helper and open-code it, suggested by Jacopo Mondi and Sakari Ailus - Call clk_prepare_enable() in s_power callback instead of probe, suggested by Sakari Ailus - Unify clk and gpio configuration in a single if-else block and, also add a check either platform data or fwnode is actually specified, suggested by Jacopo Mondi - Add CONFIG_OF guards, suggested by Jacopo Mondi Akinobu Mita (2): media: ov9650: support device tree probing media: ov9650: add device tree binding .../devicetree/bindings/media/i2c/ov9650.txt | 36 ++ drivers/media/i2c/ov9650.c | 130 +++-- 2 files changed, 128 insertions(+), 38 deletions(-) create mode 100644 Documentation/devicetree/bindings/media/i2c/ov9650.txt Cc: Jacopo Mondi <jac...@jmondi.org> Cc: H. Nikolaus Schaller <h...@goldelico.com> Cc: Hugues Fruchet <hugues.fruc...@st.com> Cc: Sakari Ailus <sakari.ai...@linux.intel.com> Cc: Mauro Carvalho Chehab <mche...@s-opensource.com> Cc: Rob Herring <r...@kernel.org> -- 2.7.4
[PATCH v2 1/2] media: ov9650: support device tree probing
The ov9650 driver currently only supports legacy platform data probe.
This change adds device tree probing.
There has been an attempt to add device tree support for ov9650 driver
by Hugues Fruchet as a part of the patchset that adds support of OV9655
camera (http://www.spinics.net/lists/linux-media/msg117903.html), but
it wasn't merged into mainline because creating a separate driver for
OV9655 is preferred.
This is very similar to Hugues's patch, but not supporting new device.
Cc: Jacopo Mondi <jac...@jmondi.org>
Cc: H. Nikolaus Schaller <h...@goldelico.com>
Cc: Hugues Fruchet <hugues.fruc...@st.com>
Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
Cc: Mauro Carvalho Chehab <mche...@s-opensource.com>
Cc: Rob Herring <r...@kernel.org>
Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
---
* Changelog v2
- Split binding documentation, suggested by Rob Herring and Jacopo Mondi
- Remove ov965x_gpio_set() helper and open-code it, suggested by Jacopo Mondi
and Sakari Ailus
- Call clk_prepare_enable() in s_power callback instead of probe, suggested
by Sakari Ailus
- Unify clk and gpio configuration in a single if-else block and, also add
a check either platform data or fwnode is actually specified, suggested
by Jacopo Mondi
- Add CONFIG_OF guards, suggested by Jacopo Mondi
drivers/media/i2c/ov9650.c | 130 -
1 file changed, 92 insertions(+), 38 deletions(-)
diff --git a/drivers/media/i2c/ov9650.c b/drivers/media/i2c/ov9650.c
index 69433e1..99a3eab 100644
--- a/drivers/media/i2c/ov9650.c
+++ b/drivers/media/i2c/ov9650.c
@@ -11,8 +11,10 @@
* it under the terms of the GNU General Public License version 2 as
* published by the Free Software Foundation.
*/
+#include
#include
#include
+#include
#include
#include
#include
@@ -249,9 +251,10 @@ struct ov965x {
struct v4l2_subdev sd;
struct media_pad pad;
enum v4l2_mbus_type bus_type;
- int gpios[NUM_GPIOS];
+ struct gpio_desc *gpios[NUM_GPIOS];
/* External master clock frequency */
unsigned long mclk_frequency;
+ struct clk *clk;
/* Protects the struct fields below */
struct mutex lock;
@@ -513,24 +516,27 @@ static int ov965x_set_color_matrix(struct ov965x *ov965x)
return 0;
}
-static void ov965x_gpio_set(int gpio, int val)
-{
- if (gpio_is_valid(gpio))
- gpio_set_value(gpio, val);
-}
-
-static void __ov965x_set_power(struct ov965x *ov965x, int on)
+static int __ov965x_set_power(struct ov965x *ov965x, int on)
{
if (on) {
- ov965x_gpio_set(ov965x->gpios[GPIO_PWDN], 0);
- ov965x_gpio_set(ov965x->gpios[GPIO_RST], 0);
+ int ret = clk_prepare_enable(ov965x->clk);
+
+ if (ret)
+ return ret;
+
+ gpiod_set_value_cansleep(ov965x->gpios[GPIO_PWDN], 0);
+ gpiod_set_value_cansleep(ov965x->gpios[GPIO_RST], 0);
msleep(25);
} else {
- ov965x_gpio_set(ov965x->gpios[GPIO_RST], 1);
- ov965x_gpio_set(ov965x->gpios[GPIO_PWDN], 1);
+ gpiod_set_value_cansleep(ov965x->gpios[GPIO_RST], 1);
+ gpiod_set_value_cansleep(ov965x->gpios[GPIO_PWDN], 1);
+
+ clk_disable_unprepare(ov965x->clk);
}
ov965x->streaming = 0;
+
+ return 0;
}
static int ov965x_s_power(struct v4l2_subdev *sd, int on)
@@ -543,8 +549,8 @@ static int ov965x_s_power(struct v4l2_subdev *sd, int on)
mutex_lock(>lock);
if (ov965x->power == !on) {
- __ov965x_set_power(ov965x, on);
- if (on) {
+ ret = __ov965x_set_power(ov965x, on);
+ if (!ret && on) {
ret = ov965x_write_array(client,
ov965x_init_regs);
ov965x->apply_frame_fmt = 1;
@@ -1408,16 +1414,17 @@ static const struct v4l2_subdev_ops ov965x_subdev_ops =
{
/*
* Reset and power down GPIOs configuration
*/
-static int ov965x_configure_gpios(struct ov965x *ov965x,
- const struct ov9650_platform_data *pdata)
+static int ov965x_configure_gpios_pdata(struct ov965x *ov965x,
+ const struct ov9650_platform_data *pdata)
{
int ret, i;
+ int gpios[NUM_GPIOS];
- ov965x->gpios[GPIO_PWDN] = pdata->gpio_pwdn;
- ov965x->gpios[GPIO_RST] = pdata->gpio_reset;
+ gpios[GPIO_PWDN] = pdata->gpio_pwdn;
+ gpios[GPIO_RST] = pdata->gpio_reset;
for (i = 0; i < ARRAY_SIZE(ov965x->gpios); i++) {
- int gpio = ov965x->gpios[i];
+ int gpio = gpios[i];
if (!gpio_is_valid(gpio))
continue;
@@ -1427,9 +1434,30 @@ static int ov965x_
[PATCH v2 2/2] media: ov9650: add device tree binding
Now the ov9650 driver supports device tree probing. So this adds a
device tree binding documentation.
Cc: Jacopo Mondi <jac...@jmondi.org>
Cc: H. Nikolaus Schaller <h...@goldelico.com>
Cc: Hugues Fruchet <hugues.fruc...@st.com>
Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
Cc: Mauro Carvalho Chehab <mche...@s-opensource.com>
Cc: Rob Herring <r...@kernel.org>
Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
---
* Changelog v2
- Split binding documentation, suggested by Rob Herring and Jacopo Mondi
- Improve the wording for compatible property in the binding documentation,
suggested by Jacopo Mondi
- Improve the description for the device node in the binding documentation,
suggested by Sakari Ailus
.../devicetree/bindings/media/i2c/ov9650.txt | 36 ++
1 file changed, 36 insertions(+)
create mode 100644 Documentation/devicetree/bindings/media/i2c/ov9650.txt
diff --git a/Documentation/devicetree/bindings/media/i2c/ov9650.txt
b/Documentation/devicetree/bindings/media/i2c/ov9650.txt
new file mode 100644
index 000..506dfc5
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ov9650.txt
@@ -0,0 +1,36 @@
+* Omnivision OV9650/OV9652 CMOS sensor
+
+Required Properties:
+- compatible: shall be one of
+ "ovti,ov9650"
+ "ovti,ov9652"
+- clocks: reference to the xvclk input clock.
+
+Optional Properties:
+- reset-gpios: reference to the GPIO connected to the resetb pin, if any.
+ Active is high.
+- powerdown-gpios: reference to the GPIO connected to the pwdn pin, if any.
+ Active is high.
+
+The device node shall contain one 'port' child node with one child 'endpoint'
+subnode for its digital output video port, in accordance with the video
+interface bindings defined in Documentation/devicetree/bindings/media/
+video-interfaces.txt.
+
+Example:
+
+ {
+ ov9650: camera@30 {
+ compatible = "ovti,ov9650";
+ reg = <0x30>;
+ reset-gpios = <_gpio_0 0 GPIO_ACTIVE_HIGH>;
+ powerdown-gpios = <_gpio_0 1 GPIO_ACTIVE_HIGH>;
+ clocks = <>;
+
+ port {
+ ov9650_0: endpoint {
+ remote-endpoint = <_in0>;
+ };
+ };
+ };
+};
--
2.7.4
[PATCH v3 0/3] media: ov9650: support device tree probing
This patchset adds device tree probing for ov9650 driver. This contains an actual driver change and a newly added binding documentation part. * Changelog v3 - Add Reviewed-by: tags - Add MAINTAINERS entry * Changelog v2 - Split binding documentation, suggested by Rob Herring and Jacopo Mondi - Improve the wording for compatible property in the binding documentation, suggested by Jacopo Mondi - Improve the description for the device node in the binding documentation, suggested by Sakari Ailus - Remove ov965x_gpio_set() helper and open-code it, suggested by Jacopo Mondi and Sakari Ailus - Call clk_prepare_enable() in s_power callback instead of probe, suggested by Sakari Ailus - Unify clk and gpio configuration in a single if-else block and, also add a check either platform data or fwnode is actually specified, suggested by Jacopo Mondi - Add CONFIG_OF guards, suggested by Jacopo Mondi Akinobu Mita (3): media: ov9650: support device tree probing media: MAINTAINERS: add entry for ov9650 driver media: ov9650: add device tree binding .../devicetree/bindings/media/i2c/ov9650.txt | 36 ++ MAINTAINERS| 10 ++ drivers/media/i2c/ov9650.c | 130 +++-- 3 files changed, 138 insertions(+), 38 deletions(-) create mode 100644 Documentation/devicetree/bindings/media/i2c/ov9650.txt Cc: Sylwester Nawrocki <s.nawro...@samsung.com> Cc: Jacopo Mondi <jac...@jmondi.org> Cc: H. Nikolaus Schaller <h...@goldelico.com> Cc: Hugues Fruchet <hugues.fruc...@st.com> Cc: Sakari Ailus <sakari.ai...@linux.intel.com> Cc: Mauro Carvalho Chehab <mche...@s-opensource.com> Cc: Rob Herring <r...@kernel.org> -- 2.7.4
[PATCH v3 1/3] media: ov9650: support device tree probing
The ov9650 driver currently only supports legacy platform data probe.
This change adds device tree probing.
There has been an attempt to add device tree support for ov9650 driver
by Hugues Fruchet as a part of the patchset that adds support of OV9655
camera (http://www.spinics.net/lists/linux-media/msg117903.html), but
it wasn't merged into mainline because creating a separate driver for
OV9655 is preferred.
This is very similar to Hugues's patch, but not supporting new device.
Cc: Sylwester Nawrocki <s.nawro...@samsung.com>
Cc: Jacopo Mondi <jac...@jmondi.org>
Cc: H. Nikolaus Schaller <h...@goldelico.com>
Cc: Hugues Fruchet <hugues.fruc...@st.com>
Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
Cc: Mauro Carvalho Chehab <mche...@s-opensource.com>
Cc: Rob Herring <r...@kernel.org>
Reviewed-by: Sylwester Nawrocki <s.nawro...@samsung.com>
Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
---
* Changelog v3
- Add Reviewed-by: tag
* Changelog v2
- Split binding documentation, suggested by Rob Herring and Jacopo Mondi
- Remove ov965x_gpio_set() helper and open-code it, suggested by Jacopo Mondi
and Sakari Ailus
- Call clk_prepare_enable() in s_power callback instead of probe, suggested
by Sakari Ailus
- Unify clk and gpio configuration in a single if-else block and, also add
a check either platform data or fwnode is actually specified, suggested
by Jacopo Mondi
- Add CONFIG_OF guards, suggested by Jacopo Mondi
drivers/media/i2c/ov9650.c | 130 -
1 file changed, 92 insertions(+), 38 deletions(-)
diff --git a/drivers/media/i2c/ov9650.c b/drivers/media/i2c/ov9650.c
index 69433e1..99a3eab 100644
--- a/drivers/media/i2c/ov9650.c
+++ b/drivers/media/i2c/ov9650.c
@@ -11,8 +11,10 @@
* it under the terms of the GNU General Public License version 2 as
* published by the Free Software Foundation.
*/
+#include
#include
#include
+#include
#include
#include
#include
@@ -249,9 +251,10 @@ struct ov965x {
struct v4l2_subdev sd;
struct media_pad pad;
enum v4l2_mbus_type bus_type;
- int gpios[NUM_GPIOS];
+ struct gpio_desc *gpios[NUM_GPIOS];
/* External master clock frequency */
unsigned long mclk_frequency;
+ struct clk *clk;
/* Protects the struct fields below */
struct mutex lock;
@@ -513,24 +516,27 @@ static int ov965x_set_color_matrix(struct ov965x *ov965x)
return 0;
}
-static void ov965x_gpio_set(int gpio, int val)
-{
- if (gpio_is_valid(gpio))
- gpio_set_value(gpio, val);
-}
-
-static void __ov965x_set_power(struct ov965x *ov965x, int on)
+static int __ov965x_set_power(struct ov965x *ov965x, int on)
{
if (on) {
- ov965x_gpio_set(ov965x->gpios[GPIO_PWDN], 0);
- ov965x_gpio_set(ov965x->gpios[GPIO_RST], 0);
+ int ret = clk_prepare_enable(ov965x->clk);
+
+ if (ret)
+ return ret;
+
+ gpiod_set_value_cansleep(ov965x->gpios[GPIO_PWDN], 0);
+ gpiod_set_value_cansleep(ov965x->gpios[GPIO_RST], 0);
msleep(25);
} else {
- ov965x_gpio_set(ov965x->gpios[GPIO_RST], 1);
- ov965x_gpio_set(ov965x->gpios[GPIO_PWDN], 1);
+ gpiod_set_value_cansleep(ov965x->gpios[GPIO_RST], 1);
+ gpiod_set_value_cansleep(ov965x->gpios[GPIO_PWDN], 1);
+
+ clk_disable_unprepare(ov965x->clk);
}
ov965x->streaming = 0;
+
+ return 0;
}
static int ov965x_s_power(struct v4l2_subdev *sd, int on)
@@ -543,8 +549,8 @@ static int ov965x_s_power(struct v4l2_subdev *sd, int on)
mutex_lock(>lock);
if (ov965x->power == !on) {
- __ov965x_set_power(ov965x, on);
- if (on) {
+ ret = __ov965x_set_power(ov965x, on);
+ if (!ret && on) {
ret = ov965x_write_array(client,
ov965x_init_regs);
ov965x->apply_frame_fmt = 1;
@@ -1408,16 +1414,17 @@ static const struct v4l2_subdev_ops ov965x_subdev_ops =
{
/*
* Reset and power down GPIOs configuration
*/
-static int ov965x_configure_gpios(struct ov965x *ov965x,
- const struct ov9650_platform_data *pdata)
+static int ov965x_configure_gpios_pdata(struct ov965x *ov965x,
+ const struct ov9650_platform_data *pdata)
{
int ret, i;
+ int gpios[NUM_GPIOS];
- ov965x->gpios[GPIO_PWDN] = pdata->gpio_pwdn;
- ov965x->gpios[GPIO_RST] = pdata->gpio_reset;
+ gpios[GPIO_PWDN] = pdata->gpio_pwdn;
+ gpios[GPIO_RST] = pdata->gpio_reset;
for (i = 0; i < ARRAY_SIZE(ov965x->gpios); i++) {
- int gpio = ov965x->gpios[i];
+
[PATCH v3 2/3] media: MAINTAINERS: add entry for ov9650 driver
This adds an entry to the MAINTAINERS file for ov9650 driver. The following persons are added in this entry. * Sakari as a person who looks after media sensor driver patches * Sylwester as a module author * Myself as a person who has the hardware and can test the patches Cc: Sylwester Nawrocki <s.nawro...@samsung.com> Cc: Jacopo Mondi <jac...@jmondi.org> Cc: H. Nikolaus Schaller <h...@goldelico.com> Cc: Hugues Fruchet <hugues.fruc...@st.com> Cc: Sakari Ailus <sakari.ai...@linux.intel.com> Cc: Mauro Carvalho Chehab <mche...@s-opensource.com> Cc: Rob Herring <r...@kernel.org> Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com> --- MAINTAINERS | 9 + 1 file changed, 9 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index e358141..8924e39 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -10052,6 +10052,15 @@ S: Maintained F: drivers/media/i2c/ov7670.c F: Documentation/devicetree/bindings/media/i2c/ov7670.txt +OMNIVISION OV9650 SENSOR DRIVER +M: Sakari Ailus <sakari.ai...@linux.intel.com> +R: Akinobu Mita <akinobu.m...@gmail.com> +R: Sylwester Nawrocki <s.nawro...@samsung.com> +L: linux-media@vger.kernel.org +T: git git://linuxtv.org/media_tree.git +S: Maintained +F: drivers/media/i2c/ov9650.c + ONENAND FLASH DRIVER M: Kyungmin Park <kyungmin.p...@samsung.com> L: linux-...@lists.infradead.org -- 2.7.4
[PATCH v3 3/3] media: ov9650: add device tree binding
Now the ov9650 driver supports device tree probing. So this adds a
device tree binding documentation.
Cc: Sylwester Nawrocki <s.nawro...@samsung.com>
Cc: Jacopo Mondi <jac...@jmondi.org>
Cc: H. Nikolaus Schaller <h...@goldelico.com>
Cc: Hugues Fruchet <hugues.fruc...@st.com>
Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
Cc: Mauro Carvalho Chehab <mche...@s-opensource.com>
Cc: Rob Herring <r...@kernel.org>
Reviewed-by: Rob Herring <r...@kernel.org>
Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
---
* Changelog v3
- Add Reviewed-by: tag
- Add F: line in MAINTAINERS
* Changelog v2
- Split binding documentation, suggested by Rob Herring and Jacopo Mondi
- Improve the wording for compatible property in the binding documentation,
suggested by Jacopo Mondi
- Improve the description for the device node in the binding documentation,
suggested by Sakari Ailus
.../devicetree/bindings/media/i2c/ov9650.txt | 36 ++
MAINTAINERS| 1 +
2 files changed, 37 insertions(+)
create mode 100644 Documentation/devicetree/bindings/media/i2c/ov9650.txt
diff --git a/Documentation/devicetree/bindings/media/i2c/ov9650.txt
b/Documentation/devicetree/bindings/media/i2c/ov9650.txt
new file mode 100644
index 000..506dfc5
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ov9650.txt
@@ -0,0 +1,36 @@
+* Omnivision OV9650/OV9652 CMOS sensor
+
+Required Properties:
+- compatible: shall be one of
+ "ovti,ov9650"
+ "ovti,ov9652"
+- clocks: reference to the xvclk input clock.
+
+Optional Properties:
+- reset-gpios: reference to the GPIO connected to the resetb pin, if any.
+ Active is high.
+- powerdown-gpios: reference to the GPIO connected to the pwdn pin, if any.
+ Active is high.
+
+The device node shall contain one 'port' child node with one child 'endpoint'
+subnode for its digital output video port, in accordance with the video
+interface bindings defined in Documentation/devicetree/bindings/media/
+video-interfaces.txt.
+
+Example:
+
+ {
+ ov9650: camera@30 {
+ compatible = "ovti,ov9650";
+ reg = <0x30>;
+ reset-gpios = <_gpio_0 0 GPIO_ACTIVE_HIGH>;
+ powerdown-gpios = <_gpio_0 1 GPIO_ACTIVE_HIGH>;
+ clocks = <>;
+
+ port {
+ ov9650_0: endpoint {
+ remote-endpoint = <_in0>;
+ };
+ };
+ };
+};
diff --git a/MAINTAINERS b/MAINTAINERS
index 8924e39..876c0f9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10060,6 +10060,7 @@ L: linux-media@vger.kernel.org
T: git git://linuxtv.org/media_tree.git
S: Maintained
F: drivers/media/i2c/ov9650.c
+F: Documentation/devicetree/bindings/media/i2c/ov9650.txt
ONENAND FLASH DRIVER
M: Kyungmin Park <kyungmin.p...@samsung.com>
--
2.7.4
Re: [PATCH v2 1/2] media: ov9650: support device tree probing
2018-01-12 23:14 GMT+09:00 Sylwester Nawrocki <s.nawro...@samsung.com>:
> On 01/07/2018 05:54 PM, Akinobu Mita wrote:
>> The ov9650 driver currently only supports legacy platform data probe.
>> This change adds device tree probing.
>
>> Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
>> ---
>
>> drivers/media/i2c/ov9650.c | 130
>> -
>> 1 file changed, 92 insertions(+), 38 deletions(-)
>>
>> diff --git a/drivers/media/i2c/ov9650.c b/drivers/media/i2c/ov9650.c
>> index 69433e1..99a3eab 100644
>> --- a/drivers/media/i2c/ov9650.c
>> +++ b/drivers/media/i2c/ov9650.c
>
>> -static void __ov965x_set_power(struct ov965x *ov965x, int on)
>> +static int __ov965x_set_power(struct ov965x *ov965x, int on)
>> {
>> if (on) {
>> - ov965x_gpio_set(ov965x->gpios[GPIO_PWDN], 0);
>> - ov965x_gpio_set(ov965x->gpios[GPIO_RST], 0);
>> + int ret = clk_prepare_enable(ov965x->clk);
>
> It seems you rely on the fact clk_prepare_enable() is a nop when passed
> argument is NULL, which happens in non-DT case.
So this works fine as before in non-DT case, doesn't it?
>> + if (ret)
>> + return ret;
>> +
>> + gpiod_set_value_cansleep(ov965x->gpios[GPIO_PWDN], 0);
>> + gpiod_set_value_cansleep(ov965x->gpios[GPIO_RST], 0);
>> msleep(25);
>> } else {
>> - ov965x_gpio_set(ov965x->gpios[GPIO_RST], 1);
>> - ov965x_gpio_set(ov965x->gpios[GPIO_PWDN], 1);
>> + gpiod_set_value_cansleep(ov965x->gpios[GPIO_RST], 1);
>> + gpiod_set_value_cansleep(ov965x->gpios[GPIO_PWDN], 1);
>> +
>> + clk_disable_unprepare(ov965x->clk);
>> }
>>
>> ov965x->streaming = 0;
>> +
>> + return 0;
>> }
>
>> @@ -1408,16 +1414,17 @@ static const struct v4l2_subdev_ops
>> ov965x_subdev_ops = {
>> /*
>> * Reset and power down GPIOs configuration
>> */
>> -static int ov965x_configure_gpios(struct ov965x *ov965x,
>> - const struct ov9650_platform_data *pdata)
>> +static int ov965x_configure_gpios_pdata(struct ov965x *ov965x,
>> + const struct ov9650_platform_data *pdata)
>> {
>> int ret, i;
>> + int gpios[NUM_GPIOS];
>>
>> - ov965x->gpios[GPIO_PWDN] = pdata->gpio_pwdn;
>> - ov965x->gpios[GPIO_RST] = pdata->gpio_reset;
>> + gpios[GPIO_PWDN] = pdata->gpio_pwdn;
>> + gpios[GPIO_RST] = pdata->gpio_reset;
>>
>> for (i = 0; i < ARRAY_SIZE(ov965x->gpios); i++) {
>> - int gpio = ov965x->gpios[i];
>> + int gpio = gpios[i];
>>
>> if (!gpio_is_valid(gpio))
>> continue;
>> @@ -1427,9 +1434,30 @@ static int ov965x_configure_gpios(struct ov965x
>> *ov965x,
>> return ret;
>> v4l2_dbg(1, debug, >sd, "set gpio %d to 1\n", gpio);
>>
>> - gpio_set_value(gpio, 1);
>> + gpio_set_value_cansleep(gpio, 1);
>> gpio_export(gpio, 0);
>> - ov965x->gpios[i] = gpio;
>> + ov965x->gpios[i] = gpio_to_desc(gpio);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int ov965x_configure_gpios(struct ov965x *ov965x)
>> +{
>> + struct device *dev = >client->dev;
>> +
>> + ov965x->gpios[GPIO_PWDN] = devm_gpiod_get_optional(dev, "powerdown",
>> + GPIOD_OUT_HIGH);
>> + if (IS_ERR(ov965x->gpios[GPIO_PWDN])) {
>> + dev_info(dev, "can't get %s GPIO\n", "powerdown");
>> + return PTR_ERR(ov965x->gpios[GPIO_PWDN]);
>> + }
>> +
>> + ov965x->gpios[GPIO_RST] = devm_gpiod_get_optional(dev, "reset",
>> + GPIOD_OUT_HIGH);
>> + if (IS_ERR(ov965x->gpios[GPIO_RST])) {
>> + dev_info(dev, "can't get %s GPIO\n", "reset");
>> + return PTR_ERR(ov965x->gpios[GPIO_RST]);
>> }
>>
>> return 0;
>> @@ -1443,7 +1471,10 @@ static int ov965x_detect_sensor(struct v4l2_subdev
>> *sd)
>> int ret;
>>
>> mutex_lock(>lock);
>> -
Re: [PATCH v2 2/2] media: ov9650: add device tree binding
2018-01-12 23:15 GMT+09:00 Sylwester Nawrocki <s.nawro...@samsung.com>: > On 01/08/2018 10:35 AM, Sakari Ailus wrote: > >> I was going to say you're missing the MAINTAINERS entry for this newly >> added file but then I noticed that the entire driver is missing an entry. >> Still this file should have a MAINTAINERS entry added for it independently >> of that, in the same patch. >> >> Cc Sylwester. > > I don't the hardware and I can't test the patches so Mita-san if you wish > so please add yourself as a maintainer of whole driver. Even if you don't have the hardware, you can help reviwing. So if you don't mind, I would like to add the following maintainer entry for this driver. OMNIVISION OV9650 SENSOR DRIVER M: Sakari Ailus <sakari.ai...@iki.fi> R: Akinobu Mita <akinobu.m...@gmail.com> R: Sylwester Nawrocki <s.nawro...@samsung.com> L: linux-media@vger.kernel.org T: git git://linuxtv.org/media_tree.git S: Maintained F: drivers/media/i2c/ov9650.c F: Documentation/devicetree/bindings/media/i2c/ov9650.txt
[PATCH 1/2] media: ov2640: make set_fmt() work in power-down mode
The set_fmt() subdev pad operation for this driver currently does not
only do the driver internal format selection but also do the actual
register setup.
This doesn't work if the device power control via GPIO lines is enabled.
Because the set_fmt() can be called when the device is placed into power
down mode.
First of all, this fix adds flag to keep track of whether the device starts
streaming or not. Then, the set_fmt() postpones applying the actual
register setup at this time. Instead the setup will be applied when the
streaming is started.
Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
Cc: Mauro Carvalho Chehab <mche...@s-opensource.com>
Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
---
drivers/media/i2c/ov2640.c | 71 +-
1 file changed, 57 insertions(+), 14 deletions(-)
diff --git a/drivers/media/i2c/ov2640.c b/drivers/media/i2c/ov2640.c
index 4c3b927..68a356d 100644
--- a/drivers/media/i2c/ov2640.c
+++ b/drivers/media/i2c/ov2640.c
@@ -307,6 +307,9 @@ struct ov2640_priv {
struct gpio_desc *resetb_gpio;
struct gpio_desc *pwdn_gpio;
+
+ struct mutex lock; /* lock to protect streaming */
+ bool streaming;
};
/*
@@ -798,16 +801,13 @@ static const struct ov2640_win_size
*ov2640_select_win(u32 width, u32 height)
static int ov2640_set_params(struct i2c_client *client,
const struct ov2640_win_size *win, u32 code)
{
- struct ov2640_priv *priv = to_ov2640(client);
const struct regval_list *selected_cfmt_regs;
u8 val;
int ret;
- /* select win */
- priv->win = win;
+ if (!win)
+ return -EINVAL;
- /* select format */
- priv->cfmt_code = 0;
switch (code) {
case MEDIA_BUS_FMT_RGB565_2X8_BE:
dev_dbg(>dev, "%s: Selected cfmt RGB565 BE", __func__);
@@ -846,13 +846,13 @@ static int ov2640_set_params(struct i2c_client *client,
goto err;
/* select preamble */
- dev_dbg(>dev, "%s: Set size to %s", __func__, priv->win->name);
+ dev_dbg(>dev, "%s: Set size to %s", __func__, win->name);
ret = ov2640_write_array(client, ov2640_size_change_preamble_regs);
if (ret < 0)
goto err;
/* set size win */
- ret = ov2640_write_array(client, priv->win->regs);
+ ret = ov2640_write_array(client, win->regs);
if (ret < 0)
goto err;
@@ -872,14 +872,11 @@ static int ov2640_set_params(struct i2c_client *client,
if (ret < 0)
goto err;
- priv->cfmt_code = code;
-
return 0;
err:
dev_err(>dev, "%s: Error %d", __func__, ret);
ov2640_reset(client);
- priv->win = NULL;
return ret;
}
@@ -915,11 +912,15 @@ static int ov2640_set_fmt(struct v4l2_subdev *sd,
{
struct v4l2_mbus_framefmt *mf = >format;
struct i2c_client *client = v4l2_get_subdevdata(sd);
+ struct ov2640_priv *priv = to_ov2640(client);
const struct ov2640_win_size *win;
+ int ret = 0;
if (format->pad)
return -EINVAL;
+ mutex_lock(>lock);
+
/* select suitable win */
win = ov2640_select_win(mf->width, mf->height);
mf->width = win->width;
@@ -941,10 +942,24 @@ static int ov2640_set_fmt(struct v4l2_subdev *sd,
break;
}
- if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE)
- return ov2640_set_params(client, win, mf->code);
- cfg->try_fmt = *mf;
- return 0;
+ if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
+ struct ov2640_priv *priv = to_ov2640(client);
+
+ if (priv->streaming) {
+ ret = -EBUSY;
+ goto out;
+ }
+ /* select win */
+ priv->win = win;
+ /* select format */
+ priv->cfmt_code = mf->code;
+ } else {
+ cfg->try_fmt = *mf;
+ }
+out:
+ mutex_unlock(>lock);
+
+ return ret;
}
static int ov2640_enum_mbus_code(struct v4l2_subdev *sd,
@@ -979,6 +994,26 @@ static int ov2640_get_selection(struct v4l2_subdev *sd,
}
}
+static int ov2640_s_stream(struct v4l2_subdev *sd, int on)
+{
+ struct i2c_client *client = v4l2_get_subdevdata(sd);
+ struct ov2640_priv *priv = to_ov2640(client);
+ int ret = 0;
+
+ mutex_lock(>lock);
+ if (priv->streaming == !on) {
+ if (on) {
+ ret = ov2640_set_params(client, priv->win,
+ priv->cfmt_code);
+ }
+ }
+ if (!ret)
+ priv->streaming = on;
+ mutex_unlock(>lock);
[PATCH 0/2] media: ov2640: fix issues in power-down mode
The ov2640 driver has support for controlling the reset and power-down pins via GPIO lines. But enabling these GPIO controls will prevent the device from working correctly due to the lack of considering the current device power state. This series contains the fixes based on how other sensor drivers handle it correctly. Akinobu Mita (2): media: ov2640: make set_fmt() work in power-down mode media: ov2640: make s_ctrl() work in power-down mode drivers/media/i2c/ov2640.c | 112 + 1 file changed, 92 insertions(+), 20 deletions(-) Cc: Sakari Ailus <sakari.ai...@linux.intel.com> Cc: Mauro Carvalho Chehab <mche...@s-opensource.com> -- 2.7.4
[PATCH 2/2] media: ov2640: make s_ctrl() work in power-down mode
The s_ctrl() operation can be called when the device is placed into
power down mode. Then, applying controls to H/W should be postponed at
this time. Instead the controls will be restored when the streaming is
started.
Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
Cc: Mauro Carvalho Chehab <mche...@s-opensource.com>
Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
---
drivers/media/i2c/ov2640.c | 43 ---
1 file changed, 36 insertions(+), 7 deletions(-)
diff --git a/drivers/media/i2c/ov2640.c b/drivers/media/i2c/ov2640.c
index 68a356d..beb7220 100644
--- a/drivers/media/i2c/ov2640.c
+++ b/drivers/media/i2c/ov2640.c
@@ -308,8 +308,9 @@ struct ov2640_priv {
struct gpio_desc *resetb_gpio;
struct gpio_desc *pwdn_gpio;
- struct mutex lock; /* lock to protect streaming */
+ struct mutex lock; /* lock to protect streaming and power_count */
bool streaming;
+ int power_count;
};
/*
@@ -712,9 +713,20 @@ static int ov2640_s_ctrl(struct v4l2_ctrl *ctrl)
struct v4l2_subdev *sd =
_of(ctrl->handler, struct ov2640_priv, hdl)->subdev;
struct i2c_client *client = v4l2_get_subdevdata(sd);
+ struct ov2640_priv *priv = to_ov2640(client);
u8 val;
int ret;
+ /* v4l2_ctrl_lock() locks our own mutex */
+
+ /*
+* If the device is not powered up by the host driver, do not apply any
+* controls to H/W at this time. Instead the controls will be restored
+* when the streaming is started.
+*/
+ if (!priv->power_count)
+ return 0;
+
ret = i2c_smbus_write_byte_data(client, BANK_SEL, BANK_SEL_SENS);
if (ret < 0)
return ret;
@@ -766,12 +778,9 @@ static int ov2640_s_register(struct v4l2_subdev *sd,
}
#endif
-static int ov2640_s_power(struct v4l2_subdev *sd, int on)
+static void ov2640_set_power(struct ov2640_priv *priv, int on)
{
#ifdef CONFIG_GPIOLIB
- struct i2c_client *client = v4l2_get_subdevdata(sd);
- struct ov2640_priv *priv = to_ov2640(client);
-
if (priv->pwdn_gpio)
gpiod_direction_output(priv->pwdn_gpio, !on);
if (on && priv->resetb_gpio) {
@@ -781,6 +790,25 @@ static int ov2640_s_power(struct v4l2_subdev *sd, int on)
gpiod_set_value(priv->resetb_gpio, 0);
}
#endif
+}
+
+static int ov2640_s_power(struct v4l2_subdev *sd, int on)
+{
+ struct i2c_client *client = v4l2_get_subdevdata(sd);
+ struct ov2640_priv *priv = to_ov2640(client);
+
+ mutex_lock(>lock);
+
+ /*
+* If the power count is modified from 0 to != 0 or from != 0 to 0,
+* update the power state.
+*/
+ if (priv->power_count == !on)
+ ov2640_set_power(priv, on);
+ priv->power_count += on ? 1 : -1;
+ WARN_ON(priv->power_count < 0);
+ mutex_unlock(>lock);
+
return 0;
}
@@ -1005,6 +1033,8 @@ static int ov2640_s_stream(struct v4l2_subdev *sd, int on)
if (on) {
ret = ov2640_set_params(client, priv->win,
priv->cfmt_code);
+ if (!ret)
+ ret = __v4l2_ctrl_handler_setup(>hdl);
}
}
if (!ret)
@@ -1049,8 +1079,6 @@ static int ov2640_video_probe(struct i2c_client *client)
"%s Product ID %0x:%0x Manufacturer ID %x:%x\n",
devname, pid, ver, midh, midl);
- ret = v4l2_ctrl_handler_setup(>hdl);
-
done:
ov2640_s_power(>subdev, 0);
return ret;
@@ -1158,6 +1186,7 @@ static int ov2640_probe(struct i2c_client *client,
priv->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
mutex_init(>lock);
v4l2_ctrl_handler_init(>hdl, 2);
+ priv->hdl.lock = >lock;
v4l2_ctrl_new_std(>hdl, _ctrl_ops,
V4L2_CID_VFLIP, 0, 1, 1, 0);
v4l2_ctrl_new_std(>hdl, _ctrl_ops,
--
2.7.4
[PATCH v2 4/4] media: mt9m111: add V4L2_CID_TEST_PATTERN control
The mt9m111 has the test pattern generator features. This makes use of
it through V4L2_CID_TEST_PATTERN control.
Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
Cc: Mauro Carvalho Chehab <mche...@s-opensource.com>
Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
---
* Changelog v2
- Fix typo s/meida/media/ in the patch title, noticed by Sakari Ailus
drivers/media/i2c/mt9m111.c | 27 +++
1 file changed, 27 insertions(+)
diff --git a/drivers/media/i2c/mt9m111.c b/drivers/media/i2c/mt9m111.c
index e1d5032..d74f254 100644
--- a/drivers/media/i2c/mt9m111.c
+++ b/drivers/media/i2c/mt9m111.c
@@ -92,6 +92,7 @@
*/
#define MT9M111_OPER_MODE_CTRL 0x106
#define MT9M111_OUTPUT_FORMAT_CTRL 0x108
+#define MT9M111_TPG_CTRL 0x148
#define MT9M111_REDUCER_XZOOM_B0x1a0
#define MT9M111_REDUCER_XSIZE_B0x1a1
#define MT9M111_REDUCER_YZOOM_B0x1a3
@@ -124,6 +125,7 @@
#define MT9M111_OUTFMT_AVG_CHROMA (1 << 2)
#define MT9M111_OUTFMT_SWAP_YCbCr_C_Y_RGB_EVEN (1 << 1)
#define MT9M111_OUTFMT_SWAP_YCbCr_Cb_Cr_RGB_R_B(1 << 0)
+#define MT9M111_TPG_SEL_MASK GENMASK(2, 0)
/*
* Camera control register addresses (0x200..0x2ff not implemented)
@@ -706,6 +708,25 @@ static int mt9m111_set_autowhitebalance(struct mt9m111
*mt9m111, int on)
return reg_clear(OPER_MODE_CTRL, MT9M111_OPMODE_AUTOWHITEBAL_EN);
}
+static const char * const mt9m111_test_pattern_menu[] = {
+ "Disabled",
+ "Vertical monochrome gradient",
+ "Flat color type 1",
+ "Flat color type 2",
+ "Flat color type 3",
+ "Flat color type 4",
+ "Flat color type 5",
+ "Color bar",
+};
+
+static int mt9m111_set_test_pattern(struct mt9m111 *mt9m111, int val)
+{
+ struct i2c_client *client = v4l2_get_subdevdata(>subdev);
+
+ return mt9m111_reg_mask(client, MT9M111_TPG_CTRL, val,
+ MT9M111_TPG_SEL_MASK);
+}
+
static int mt9m111_s_ctrl(struct v4l2_ctrl *ctrl)
{
struct mt9m111 *mt9m111 = container_of(ctrl->handler,
@@ -724,6 +745,8 @@ static int mt9m111_s_ctrl(struct v4l2_ctrl *ctrl)
return mt9m111_set_autoexposure(mt9m111, ctrl->val);
case V4L2_CID_AUTO_WHITE_BALANCE:
return mt9m111_set_autowhitebalance(mt9m111, ctrl->val);
+ case V4L2_CID_TEST_PATTERN:
+ return mt9m111_set_test_pattern(mt9m111, ctrl->val);
}
return -EINVAL;
@@ -968,6 +991,10 @@ static int mt9m111_probe(struct i2c_client *client,
v4l2_ctrl_new_std_menu(>hdl,
_ctrl_ops, V4L2_CID_EXPOSURE_AUTO, 1, 0,
V4L2_EXPOSURE_AUTO);
+ v4l2_ctrl_new_std_menu_items(>hdl,
+ _ctrl_ops, V4L2_CID_TEST_PATTERN,
+ ARRAY_SIZE(mt9m111_test_pattern_menu) - 1, 0, 0,
+ mt9m111_test_pattern_menu);
mt9m111->subdev.ctrl_handler = >hdl;
if (mt9m111->hdl.error) {
ret = mt9m111->hdl.error;
--
2.7.4
[PATCH v2 2/4] media: mt9m111: add media controller support
Create a source pad and set the media controller type to the sensor.
Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
Cc: Mauro Carvalho Chehab <mche...@s-opensource.com>
Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
---
* Changelog v2
- Fix typo s/meida/media/ in the patch title, noticed by Sakari Ailus
drivers/media/i2c/mt9m111.c | 22 --
1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/drivers/media/i2c/mt9m111.c b/drivers/media/i2c/mt9m111.c
index 4fa10df..e1d5032 100644
--- a/drivers/media/i2c/mt9m111.c
+++ b/drivers/media/i2c/mt9m111.c
@@ -215,6 +215,9 @@ struct mt9m111 {
int power_count;
const struct mt9m111_datafmt *fmt;
int lastpage; /* PageMap cache value */
+#ifdef CONFIG_MEDIA_CONTROLLER
+ struct media_pad pad;
+#endif
};
/* Find a data format by a pixel code */
@@ -971,6 +974,14 @@ static int mt9m111_probe(struct i2c_client *client,
goto out_clkput;
}
+#ifdef CONFIG_MEDIA_CONTROLLER
+ mt9m111->pad.flags = MEDIA_PAD_FL_SOURCE;
+ mt9m111->subdev.entity.function = MEDIA_ENT_F_CAM_SENSOR;
+ ret = media_entity_pads_init(>subdev.entity, 1, >pad);
+ if (ret < 0)
+ goto out_hdlfree;
+#endif
+
/* Second stage probe - when a capture adapter is there */
mt9m111->rect.left = MT9M111_MIN_DARK_COLS;
mt9m111->rect.top = MT9M111_MIN_DARK_ROWS;
@@ -982,16 +993,20 @@ static int mt9m111_probe(struct i2c_client *client,
ret = mt9m111_video_probe(client);
if (ret < 0)
- goto out_hdlfree;
+ goto out_entityclean;
mt9m111->subdev.dev = >dev;
ret = v4l2_async_register_subdev(>subdev);
if (ret < 0)
- goto out_hdlfree;
+ goto out_entityclean;
return 0;
+out_entityclean:
+#ifdef CONFIG_MEDIA_CONTROLLER
+ media_entity_cleanup(>subdev.entity);
out_hdlfree:
+#endif
v4l2_ctrl_handler_free(>hdl);
out_clkput:
v4l2_clk_put(mt9m111->clk);
@@ -1004,6 +1019,9 @@ static int mt9m111_remove(struct i2c_client *client)
struct mt9m111 *mt9m111 = to_mt9m111(client);
v4l2_async_unregister_subdev(>subdev);
+#ifdef CONFIG_MEDIA_CONTROLLER
+ media_entity_cleanup(>subdev.entity);
+#endif
v4l2_clk_put(mt9m111->clk);
v4l2_ctrl_handler_free(>hdl);
--
2.7.4
[PATCH v2 3/4] media: mt9m111: document missing required clocks property
The mt9m111 driver requires clocks property for the master clock to the
sensor, but there is no description for that. This adds it.
Cc: Rob Herring <r...@kernel.org>
Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
Cc: Mauro Carvalho Chehab <mche...@s-opensource.com>
Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
---
* Changelog v2
- Fix typo s/meida/media/ in the patch title, noticed by Sakari Ailus
- Improve the wording for "clock-names" property, suggested by Sakari Ailus
Documentation/devicetree/bindings/media/i2c/mt9m111.txt | 4
1 file changed, 4 insertions(+)
diff --git a/Documentation/devicetree/bindings/media/i2c/mt9m111.txt
b/Documentation/devicetree/bindings/media/i2c/mt9m111.txt
index ed5a334..6b91003 100644
--- a/Documentation/devicetree/bindings/media/i2c/mt9m111.txt
+++ b/Documentation/devicetree/bindings/media/i2c/mt9m111.txt
@@ -6,6 +6,8 @@ interface.
Required Properties:
- compatible: value should be "micron,mt9m111"
+- clocks: reference to the master clock.
+- clock-names: shall be "mclk".
For further reading on port node refer to
Documentation/devicetree/bindings/media/video-interfaces.txt.
@@ -16,6 +18,8 @@ Example:
mt9m111@5d {
compatible = "micron,mt9m111";
reg = <0x5d>;
+ clocks = <>;
+ clock-names = "mclk";
remote = <_camera>;
port {
--
2.7.4
