linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* drivers/media/i2c/adp1653.c: does not show as /dev/video* or v4l-subdev*
@ 2016-04-28  8:45 Pavel Machek
  2016-04-29  7:15 ` v4l subdevs without big device was " Pavel Machek
  0 siblings, 1 reply; 19+ messages in thread
From: Pavel Machek @ 2016-04-28  8:45 UTC (permalink / raw)
  To: pali.rohar, sre, kernel list, linux-arm-kernel, linux-omap, tony,
	khilman, aaro.koskinen, ivo.g.dimitrov.75, patrikbachan, serge,
	sakari.ailus, tuukkat76

Hi!

On n900, probe finishes ok (verified by adding printks), and the
device shows up in /sys, but I  don't get /dev/video* or
/dev/v4l-subdev*.

Other drivers (back and front camera) load ok, and actually work. Any
idea what could be wrong?

Thanks,
								Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply	[flat|nested] 19+ messages in thread

* v4l subdevs without big device was Re: drivers/media/i2c/adp1653.c: does not show as /dev/video* or v4l-subdev*
  2016-04-28  8:45 drivers/media/i2c/adp1653.c: does not show as /dev/video* or v4l-subdev* Pavel Machek
@ 2016-04-29  7:15 ` Pavel Machek
  2016-04-29  7:31   ` Hans Verkuil
  0 siblings, 1 reply; 19+ messages in thread
From: Pavel Machek @ 2016-04-29  7:15 UTC (permalink / raw)
  To: pali.rohar, sre, kernel list, linux-arm-kernel, linux-omap, tony,
	khilman, aaro.koskinen, ivo.g.dimitrov.75, patrikbachan, serge,
	sakari.ailus, tuukkat76
  Cc: mchehab, linux-media

Hi!

> On n900, probe finishes ok (verified by adding printks), and the
> device shows up in /sys, but I  don't get /dev/video* or
> /dev/v4l-subdev*.
> 
> Other drivers (back and front camera) load ok, and actually work. Any
> idea what could be wrong?

Ok, so I guess I realized what is the problem:

adp1653 registers itself as a subdev, but there's no device that
register it as its part.

(ad5820 driver seems to have the same problem).

Is there example "dummy" device I could use, for sole purpose of
having these devices appear in /dev? They are on i2c, so both can work
on their own.

Thanks,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: v4l subdevs without big device was Re: drivers/media/i2c/adp1653.c: does not show as /dev/video* or v4l-subdev*
  2016-04-29  7:15 ` v4l subdevs without big device was " Pavel Machek
@ 2016-04-29  7:31   ` Hans Verkuil
  2016-04-29  7:56     ` Sakari Ailus
                       ` (2 more replies)
  0 siblings, 3 replies; 19+ messages in thread
From: Hans Verkuil @ 2016-04-29  7:31 UTC (permalink / raw)
  To: Pavel Machek, pali.rohar, sre, kernel list, linux-arm-kernel,
	linux-omap, tony, khilman, aaro.koskinen, ivo.g.dimitrov.75,
	patrikbachan, serge, sakari.ailus, tuukkat76
  Cc: mchehab, linux-media

On 04/29/2016 09:15 AM, Pavel Machek wrote:
> Hi!
> 
>> On n900, probe finishes ok (verified by adding printks), and the
>> device shows up in /sys, but I  don't get /dev/video* or
>> /dev/v4l-subdev*.
>>
>> Other drivers (back and front camera) load ok, and actually work. Any
>> idea what could be wrong?
> 
> Ok, so I guess I realized what is the problem:
> 
> adp1653 registers itself as a subdev, but there's no device that
> register it as its part.
> 
> (ad5820 driver seems to have the same problem).
> 
> Is there example "dummy" device I could use, for sole purpose of
> having these devices appear in /dev? They are on i2c, so both can work
> on their own.

Ah, interesting. This was discussed a little bit during the Media Summit
a few weeks back:

http://linuxtv.org/news.php?entry=2016-04-20.mchehab

See section 5:

"5. DT Bindings for flash & lens controllers

There are drivers that create their MC topology using the device tree information,
which works great for entities that transport data, but how to detect entities
that don’t transport data such as flash devices, focusers, etc.? How can those be
deduced using the device tree?

Sensor DT node add phandle to focus controller: add generic v4l binding properties
to reference such devices."

This wasn't a problem with the original N900 since that didn't use DT AFAIK and
these devices were loaded explicitly through board code.

But now you run into the same problem that I have.

The solution is that sensor devices have to provide phandles to those controller
devices. And to do that you need to define bindings which is always the hard part.

Look in Documentation/devicetree/bindings/media/video-interfaces.txt, section
"Optional endpoint properties".

Something like:

controllers: an array of phandles to controller devices associated with this
endpoint such as flash and lens controllers.

Warning: I'm no DT expert, so this is just a first attempt.

Platform drivers (omap3isp) will have to add these controller devices to the list
of subdevs to load asynchronously.

Regards,

	Hans

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: v4l subdevs without big device was Re: drivers/media/i2c/adp1653.c: does not show as /dev/video* or v4l-subdev*
  2016-04-29  7:31   ` Hans Verkuil
@ 2016-04-29  7:56     ` Sakari Ailus
  2016-04-29  9:50       ` Pavel Machek
  2016-04-29 10:15       ` Pavel Machek
  2016-04-29 21:30     ` [pre-rfc] focus and flash for Nokia N900 (was Re: v4l subdevs without big device was Re: drivers/media/i2c/adp1653.c: does not show as /dev/video* or v4l-subdev*) Pavel Machek
  2016-04-29 22:13     ` camera application for testing (was Re: v4l subdevs without big device) Pavel Machek
  2 siblings, 2 replies; 19+ messages in thread
From: Sakari Ailus @ 2016-04-29  7:56 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Pavel Machek, pali.rohar, sre, kernel list, linux-arm-kernel,
	linux-omap, tony, khilman, aaro.koskinen, ivo.g.dimitrov.75,
	patrikbachan, serge, tuukkat76, mchehab, linux-media

Hi Hans and Pavel,

On Fri, Apr 29, 2016 at 09:31:51AM +0200, Hans Verkuil wrote:
> On 04/29/2016 09:15 AM, Pavel Machek wrote:
> > Hi!
> > 
> >> On n900, probe finishes ok (verified by adding printks), and the
> >> device shows up in /sys, but I  don't get /dev/video* or
> >> /dev/v4l-subdev*.
> >>
> >> Other drivers (back and front camera) load ok, and actually work. Any
> >> idea what could be wrong?
> > 
> > Ok, so I guess I realized what is the problem:
> > 
> > adp1653 registers itself as a subdev, but there's no device that
> > register it as its part.
> > 
> > (ad5820 driver seems to have the same problem).
> > 
> > Is there example "dummy" device I could use, for sole purpose of
> > having these devices appear in /dev? They are on i2c, so both can work
> > on their own.
> 
> Ah, interesting. This was discussed a little bit during the Media Summit
> a few weeks back:
> 
> http://linuxtv.org/news.php?entry=2016-04-20.mchehab
> 
> See section 5:
> 
> "5. DT Bindings for flash & lens controllers
> 
> There are drivers that create their MC topology using the device tree information,
> which works great for entities that transport data, but how to detect entities
> that don’t transport data such as flash devices, focusers, etc.? How can those be
> deduced using the device tree?
> 
> Sensor DT node add phandle to focus controller: add generic v4l binding properties
> to reference such devices."
> 
> This wasn't a problem with the original N900 since that didn't use DT AFAIK and
> these devices were loaded explicitly through board code.
> 
> But now you run into the same problem that I have.
> 
> The solution is that sensor devices have to provide phandles to those controller
> devices. And to do that you need to define bindings which is always the hard part.
> 
> Look in Documentation/devicetree/bindings/media/video-interfaces.txt, section
> "Optional endpoint properties".
> 
> Something like:
> 
> controllers: an array of phandles to controller devices associated with this
> endpoint such as flash and lens controllers.
> 
> Warning: I'm no DT expert, so this is just a first attempt.
> 
> Platform drivers (omap3isp) will have to add these controller devices to the list
> of subdevs to load asynchronously.

I seem to have patches I haven't had time to push back then:

<URL:http://salottisipuli.retiisi.org.uk/cgi-bin/gitweb.cgi?p=~sailus/linux.git;a=shortlog;h=refs/heads/leds-as3645a>

This seems to be mostly in line with what has been discussed in the meeting,
except that the patches add a device specific property. Please ignore the
led patches in that tree for now (i.e. four patches on the top are the
relevant ones here).

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: v4l subdevs without big device was Re: drivers/media/i2c/adp1653.c: does not show as /dev/video* or v4l-subdev*
  2016-04-29  7:56     ` Sakari Ailus
@ 2016-04-29  9:50       ` Pavel Machek
  2016-04-29 10:59         ` Sakari Ailus
  2016-04-29 10:15       ` Pavel Machek
  1 sibling, 1 reply; 19+ messages in thread
From: Pavel Machek @ 2016-04-29  9:50 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: Hans Verkuil, pali.rohar, sre, kernel list, linux-arm-kernel,
	linux-omap, tony, khilman, aaro.koskinen, ivo.g.dimitrov.75,
	patrikbachan, serge, tuukkat76, mchehab, linux-media

Hi!

> > > adp1653 registers itself as a subdev, but there's no device that
> > > register it as its part.
> > > 
> > > (ad5820 driver seems to have the same problem).
> > > 
> > > Is there example "dummy" device I could use, for sole purpose of
> > > having these devices appear in /dev? They are on i2c, so both can work
> > > on their own.
> > 
> > Ah, interesting. This was discussed a little bit during the Media Summit
> > a few weeks back:
> > 
> > http://linuxtv.org/news.php?entry=2016-04-20.mchehab
> > 
> > See section 5:
> > 
> > "5. DT Bindings for flash & lens controllers
> > 
> > There are drivers that create their MC topology using the device tree information,
> > which works great for entities that transport data, but how to detect entities
> > that don’t transport data such as flash devices, focusers, etc.? How can those be
> > deduced using the device tree?
> > 
> > Sensor DT node add phandle to focus controller: add generic v4l binding properties
> > to reference such devices."
> > 
> > This wasn't a problem with the original N900 since that didn't use DT AFAIK and
> > these devices were loaded explicitly through board code.

> > But now you run into the same problem that I have.

Actually... being able to do board-code solution for testing for now
would be nice...

> > 
> > The solution is that sensor devices have to provide phandles to those controller
> > devices. And to do that you need to define bindings which is always the hard part.
> > 
> > Look in Documentation/devicetree/bindings/media/video-interfaces.txt, section
> > "Optional endpoint properties".
> > 
> > Something like:
> > 
> > controllers: an array of phandles to controller devices associated with this
> > endpoint such as flash and lens controllers.
> > 
> > Warning: I'm no DT expert, so this is just a first attempt.
> > 
> > Platform drivers (omap3isp) will have to add these controller devices to the list
> > of subdevs to load asynchronously.
> 
> I seem to have patches I haven't had time to push back then:
> 
> <URL:http://salottisipuli.retiisi.org.uk/cgi-bin/gitweb.cgi?p=~sailus/linux.git;a=shortlog;h=refs/heads/leds-as3645a>
>

That gitweb is a bit confused about its own address, but I figured it
out. Let me check...

pavel@amd:/data/l/linux-n900$ git fetch
git://git.retiisi.org.uk/~sailus/linux.git leds-as3645a:leds-as3645a
fatal: unable to connect to git.retiisi.org.uk:
git.retiisi.org.uk: Name or service not known

pavel@amd:/data/l/linux-n900$ git fetch
git://salottisipuli.retiisi.org.uk/~sailus/linux.git
leds-as3645a:leds-as3645a
remote: Counting objects: 132, done.
remote: Compressing objects: 100% (46/46), done.
remote: Total 132 (delta 111), reused 107 (delta 86)
Receiving objects: 100% (132/132), 22.80 KiB | 0 bytes/s, done.
Resolving deltas: 100% (111/111), completed with 34 local objects.
>From git://salottisipuli.retiisi.org.uk/~sailus/linux
 * [new branch]      leds-as3645a -> leds-as3645a
 

> This seems to be mostly in line with what has been discussed in the meeting,
> except that the patches add a device specific property. Please ignore the
> led patches in that tree for now (i.e. four patches on the top are the
> relevant ones here).
> 

I'm currently trying to apply them to v4.6, but am getting rather ugly
rejects :-(.

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: v4l subdevs without big device was Re: drivers/media/i2c/adp1653.c: does not show as /dev/video* or v4l-subdev*
  2016-04-29  7:56     ` Sakari Ailus
  2016-04-29  9:50       ` Pavel Machek
@ 2016-04-29 10:15       ` Pavel Machek
  1 sibling, 0 replies; 19+ messages in thread
From: Pavel Machek @ 2016-04-29 10:15 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: Hans Verkuil, pali.rohar, sre, kernel list, linux-arm-kernel,
	linux-omap, tony, khilman, aaro.koskinen, ivo.g.dimitrov.75,
	patrikbachan, serge, tuukkat76, mchehab, linux-media

[-- Attachment #1: Type: text/plain, Size: 863 bytes --]

Hi!

> > Warning: I'm no DT expert, so this is just a first attempt.
> > 
> > Platform drivers (omap3isp) will have to add these controller devices to the list
> > of subdevs to load asynchronously.
> 
> I seem to have patches I haven't had time to push back then:
> 
> <URL:http://salottisipuli.retiisi.org.uk/cgi-bin/gitweb.cgi?p=~sailus/linux.git;a=shortlog;h=refs/heads/leds-as3645a>
> 
> This seems to be mostly in line with what has been discussed in the meeting,
> except that the patches add a device specific property. Please ignore the
> led patches in that tree for now (i.e. four patches on the top are the
> relevant ones here).

Ok, I attempted to forward-port the patches to v4.6. Not sure if I was successful.

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: 0001-omap3isp-Fix-async-notifier-registration-order.patch --]
[-- Type: text/x-diff, Size: 4550 bytes --]

>From fad952ee3d1e888401b047c9657f04afeaf40fc5 Mon Sep 17 00:00:00 2001
From: Sakari Ailus <sakari.ailus@iki.fi>
Date: Sat, 16 May 2015 23:34:34 +0300
Subject: [PATCH 1/5] omap3isp: Fix async notifier registration order

The async notifier was registered before the v4l2_device was registered and
before the notifier callbacks were set. This could lead to missing the
bound() and complete() callbacks and to attempting to spin_lock() and
uninitialised spin lock.

Also fix unregistering the async notifier in the case of an error --- the
function may not fail anymore after the notifier is registered.

Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>

Conflicts:
	drivers/media/platform/omap3isp/isp.c
---
 arch/arm/boot/dts/omap3-n900.dts      | 24 ++++++++++++++++++++++++
 drivers/media/i2c/ad5820.c            |  2 +-
 drivers/media/i2c/adp1653.c           |  9 ++++++++-
 drivers/media/platform/omap3isp/isp.c | 12 ++++++++++++
 4 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/omap3-n900.dts b/arch/arm/boot/dts/omap3-n900.dts
index 640d409..0729c69 100644
--- a/arch/arm/boot/dts/omap3-n900.dts
+++ b/arch/arm/boot/dts/omap3-n900.dts
@@ -186,6 +186,7 @@
 		module {
 			model = "TCM8341MD";
 			sensor = <&cam1>;
+			focus = <&autofocus>;			
 		};
 	};
 
@@ -198,6 +199,11 @@
 		};
 	};
 
+	autofocus: dac-camera-autofocus {
+		compatible = "dac-camera-autofocus";
+		io-channels = <&ad5820>;
+	};
+
 	video-bus-switch {
 		compatible = "video-bus-switch";
 
@@ -255,6 +261,11 @@
 				crc = <0>;
 			};
 		};
+		port@99 {
+			adp1653_link: endpoint {
+				      remote-endpoint = <&adp1653_ep>;
+			};
+		};
 	};
 };
 
@@ -696,6 +707,11 @@
 		indicator {
 			led-max-microamp = <17500>;
 		};
+		port {
+			adp1653_ep: endpoint {
+				   remote-endpoint = <&adp1653_link>;
+ 			};
+ 		};
 	};
 
 	lp5523: lp5523@32 {
@@ -879,6 +895,14 @@
 			};
 		};
 	};
+
+	/* D/A converter for auto-focus */
+	ad5820: dac@0c {
+		compatible = "adi,ad5820";
+		reg = <0x0c>;
+
+		#io-channel-cells = <0>;
+	};
 };
 
 &mmc1 {
diff --git a/drivers/media/i2c/ad5820.c b/drivers/media/i2c/ad5820.c
index 58a44f1..400200c 100644
--- a/drivers/media/i2c/ad5820.c
+++ b/drivers/media/i2c/ad5820.c
@@ -421,7 +421,7 @@ static int ad5820_probe(struct i2c_client *client,
 	coil->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
 	coil->subdev.internal_ops = &ad5820_internal_ops;
 
-	ret = media_entity_init(&coil->subdev.entity, 0, NULL, 0);
+	ret = media_entity_pads_init(&coil->subdev.entity, 0, NULL);
 	if (ret < 0)
 		kfree(coil);
 
diff --git a/drivers/media/i2c/adp1653.c b/drivers/media/i2c/adp1653.c
index b90e15b..6dd5d6a 100644
--- a/drivers/media/i2c/adp1653.c
+++ b/drivers/media/i2c/adp1653.c
@@ -515,6 +515,7 @@ static int adp1653_probe(struct i2c_client *client,
 	v4l2_i2c_subdev_init(&flash->subdev, client, &adp1653_ops);
 	flash->subdev.internal_ops = &adp1653_internal_ops;
 	flash->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
+	strcpy(flash->subdev.name, "adp1653 flash");
 
 	ret = adp1653_init_controls(flash);
 	if (ret)
@@ -524,7 +525,13 @@ static int adp1653_probe(struct i2c_client *client,
 	if (ret < 0)
 		goto free_and_quit;
 
-	dev_info(&client->dev, "adp1653 probe: should be ok\n");		
+	dev_info(&client->dev, "adp1653 probe: should be ok\n");
+
+	ret = v4l2_async_register_subdev(&flash->subdev);
+	if (ret < 0)
+		goto free_and_quit;
+
+	dev_info(&client->dev, "adp1653 probe: async register subdev ok\n");	
 
 	flash->subdev.entity.function = MEDIA_ENT_F_FLASH;
 
diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c
index 6361fde..9f51127 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -2392,6 +2392,7 @@ static int isp_probe(struct platform_device *pdev)
 	if (ret < 0)
 		goto error_modules;
 
+<<<<<<< HEAD
 	ret = isp_create_links(isp);
 	if (ret < 0)
 		goto error_register_entities;
@@ -2402,6 +2403,17 @@ static int isp_probe(struct platform_device *pdev)
 	ret = v4l2_async_notifier_register(&isp->v4l2_dev, &isp->notifier);
 	if (ret)
 		goto error_register_entities;
+=======
+	if (IS_ENABLED(CONFIG_OF) && pdev->dev.of_node) {
+		isp->notifier.bound = isp_subdev_notifier_bound;
+		isp->notifier.complete = isp_subdev_notifier_complete;
+
+		ret = v4l2_async_notifier_register(&isp->v4l2_dev,
+						   &isp->notifier);
+		if (ret)
+			goto error_register_entities;
+	}
+>>>>>>> a035633... omap3isp: Fix async notifier registration order
 
 	isp_core_init(isp, 1);
 	omap3isp_put(isp);
-- 
2.1.4


[-- Attachment #3: 0002-Solve-conflict-I-missed.patch --]
[-- Type: text/x-diff, Size: 1354 bytes --]

>From 4f8286dfb13a14c1e166a61837ec20f5a096ed7b Mon Sep 17 00:00:00 2001
From: Pavel <pavel@ucw.cz>
Date: Fri, 29 Apr 2016 12:02:07 +0200
Subject: [PATCH 2/5] Solve conflict I missed.

---
 drivers/media/platform/omap3isp/isp.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c
index 9f51127..7419404 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -2392,18 +2392,14 @@ static int isp_probe(struct platform_device *pdev)
 	if (ret < 0)
 		goto error_modules;
 
-<<<<<<< HEAD
 	ret = isp_create_links(isp);
 	if (ret < 0)
 		goto error_register_entities;
 
-	isp->notifier.bound = isp_subdev_notifier_bound;
-	isp->notifier.complete = isp_subdev_notifier_complete;
-
 	ret = v4l2_async_notifier_register(&isp->v4l2_dev, &isp->notifier);
 	if (ret)
 		goto error_register_entities;
-=======
+
 	if (IS_ENABLED(CONFIG_OF) && pdev->dev.of_node) {
 		isp->notifier.bound = isp_subdev_notifier_bound;
 		isp->notifier.complete = isp_subdev_notifier_complete;
@@ -2413,7 +2409,6 @@ static int isp_probe(struct platform_device *pdev)
 		if (ret)
 			goto error_register_entities;
 	}
->>>>>>> a035633... omap3isp: Fix async notifier registration order
 
 	isp_core_init(isp, 1);
 	omap3isp_put(isp);
-- 
2.1.4


[-- Attachment #4: 0003-Cherry-pick-manually.patch --]
[-- Type: text/x-diff, Size: 5662 bytes --]

>From b402e5a19c46a9c264a311b7656c0e7e0d65322f Mon Sep 17 00:00:00 2001
From: Pavel <pavel@ucw.cz>
Date: Fri, 29 Apr 2016 12:06:32 +0200
Subject: [PATCH 3/5] Cherry-pick (manually)

commit b3691ff97a99bcf28790c4da906ffc17c822844a
Author: Sakari Ailus <sakari.ailus@iki.fi>
Date:   Sat May 16 23:56:03 2015 +0300

    omap3isp: Support the ti,camera-flashes property

    Before this patch the omap3isp driver only supported external
    devices that
        are connected to it by a data bus. This patch adds support for
    flash
        devices, in form of an array of phandles to sensor and
    associated flash
        devices, respectively, in the "ti,camera-flashes" property.

    The non image source devices are recognised by empty bus field in
    struct
        isp_async_subdev, which is made a pointer by the patch.

    Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
---
 drivers/media/platform/omap3isp/isp.c       | 90 +++++++++++++++++++++--------
 drivers/media/platform/omap3isp/isp.h       |  2 +-
 drivers/media/platform/omap3isp/ispcsiphy.c |  2 +-
 3 files changed, 67 insertions(+), 27 deletions(-)

diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c
index 7419404..a1c3bdb 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -2095,13 +2095,20 @@ static void isp_of_parse_node_csi2(struct device *dev,
 	buscfg->bus.csi2.crc = 1;
 }
 
-static int isp_of_parse_node(struct device *dev, struct device_node *node,
-			     struct isp_async_subdev *isd)
+static int isp_of_parse_node_endpoint(struct device *dev,
+				      struct device_node *node,
+				      struct isp_async_subdev *isd)
 {
-	struct isp_bus_cfg *buscfg = &isd->bus;
+	struct isp_bus_cfg *buscfg;
 	struct v4l2_of_endpoint vep;
 	int ret;
 
+	isd->bus = devm_kzalloc(dev, sizeof(*isd->bus), GFP_KERNEL);
+	if (!isd->bus)
+		return -ENOMEM;
+
+	buscfg = isd->bus;
+
 	ret = v4l2_of_parse_endpoint(node, &vep);
 	if (ret)
 		return ret;
@@ -2144,10 +2151,48 @@ static int isp_of_parse_node(struct device *dev, struct device_node *node,
 	return 0;
 }
 
+static int isp_of_parse_node(struct device *dev, struct device_node *node,
+			     struct v4l2_async_notifier *notifier, bool link)
+{
+	struct isp_async_subdev *isd;
+
+	isd = devm_kzalloc(dev, sizeof(*isd), GFP_KERNEL);
+	if (!isd) {
+		of_node_put(node);
+		return -ENOMEM;
+	}
+
+	notifier->subdevs[notifier->num_subdevs] = &isd->asd;
+
+	if (link) {
+		if (isp_of_parse_node_endpoint(dev, node, isd)) {
+			of_node_put(node);
+			return -EINVAL;
+		}
+
+		isd->asd.match.of.node = of_graph_get_remote_port_parent(node);
+		of_node_put(node);
+	} else {
+		isd->asd.match.of.node = node;
+	}
+
+	if (!isd->asd.match.of.node) {
+		dev_warn(dev, "bad remote port parent\n");
+		return -EINVAL;
+	}
+
+	isd->asd.match_type = V4L2_ASYNC_MATCH_OF;
+	notifier->num_subdevs++;
+
+	return 0;
+}
+
 static int isp_of_parse_nodes(struct device *dev,
 			      struct v4l2_async_notifier *notifier)
 {
 	struct device_node *node = NULL;
+	int ret;
+	unsigned int flash = 0;
 
 	notifier->subdevs = devm_kcalloc(
 		dev, ISP_MAX_SUBDEVS, sizeof(*notifier->subdevs), GFP_KERNEL);
@@ -2156,30 +2201,25 @@ static int isp_of_parse_nodes(struct device *dev,
 
 	while (notifier->num_subdevs < ISP_MAX_SUBDEVS &&
 	       (node = of_graph_get_next_endpoint(dev->of_node, node))) {
-		struct isp_async_subdev *isd;
-
-		isd = devm_kzalloc(dev, sizeof(*isd), GFP_KERNEL);
-		if (!isd) {
-			of_node_put(node);
-			return -ENOMEM;
-		}
-
-		notifier->subdevs[notifier->num_subdevs] = &isd->asd;
-
-		if (isp_of_parse_node(dev, node, isd)) {
-			of_node_put(node);
-			return -EINVAL;
-		}
+		ret = isp_of_parse_node(dev, node, notifier, true);
+		if (ret)
+			return ret;
+	}
 
-		isd->asd.match.of.node = of_graph_get_remote_port_parent(node);
-		of_node_put(node);
-		if (!isd->asd.match.of.node) {
-			dev_warn(dev, "bad remote port parent\n");
+	while (notifier->num_subdevs < ISP_MAX_SUBDEVS &&
+	       (node = of_parse_phandle(dev->of_node, "ti,camera-flashes",
+					flash++))) {
+		struct device_node *sensor_node =
+			of_parse_phandle(dev->of_node, "ti,camera-flashes",
+					 flash++);
+		unsigned int i;
+
+		if (!sensor_node)
 			return -EINVAL;
-		}
 
-		isd->asd.match_type = V4L2_ASYNC_MATCH_OF;
-		notifier->num_subdevs++;
+		ret = isp_of_parse_node(dev, node, notifier, false);
+		if (ret)
+			return ret;
 	}
 
 	return notifier->num_subdevs;
@@ -2193,7 +2233,7 @@ static int isp_subdev_notifier_bound(struct v4l2_async_notifier *async,
 		container_of(asd, struct isp_async_subdev, asd);
 
 	isd->sd = subdev;
-	isd->sd->host_priv = &isd->bus;
+	isd->sd->host_priv = isd->bus;
 
 	return 0;
 }
diff --git a/drivers/media/platform/omap3isp/isp.h b/drivers/media/platform/omap3isp/isp.h
index 7e6f663..c0b9d1d 100644
--- a/drivers/media/platform/omap3isp/isp.h
+++ b/drivers/media/platform/omap3isp/isp.h
@@ -228,7 +228,7 @@ struct isp_device {
 
 struct isp_async_subdev {
 	struct v4l2_subdev *sd;
-	struct isp_bus_cfg bus;
+	struct isp_bus_cfg *bus;
 	struct v4l2_async_subdev asd;
 };
 
diff --git a/drivers/media/platform/omap3isp/ispcsiphy.c b/drivers/media/platform/omap3isp/ispcsiphy.c
index 495447d..750ce93 100644
--- a/drivers/media/platform/omap3isp/ispcsiphy.c
+++ b/drivers/media/platform/omap3isp/ispcsiphy.c
@@ -177,7 +177,7 @@ static int omap3isp_csiphy_config(struct isp_csiphy *phy)
 		struct isp_async_subdev *isd =
 			container_of(pipe->external->asd,
 				     struct isp_async_subdev, asd);
-		buscfg = &isd->bus;
+		buscfg = isd->bus;
 	}
 
 	if (buscfg->interface == ISP_INTERFACE_CCP2B_PHY1
-- 
2.1.4


[-- Attachment #5: 0004-omap3isp-Assign-a-group-ID-for-sensor-and-flash-enti.patch --]
[-- Type: text/x-diff, Size: 3782 bytes --]

>From 568ea9c5c8aa61ee44e30aa4134d2b59e55a5b45 Mon Sep 17 00:00:00 2001
From: Sakari Ailus <sakari.ailus@iki.fi>
Date: Sat, 23 May 2015 15:24:55 +0300
Subject: [PATCH 4/5] omap3isp: Assign a group ID for sensor and flash entities

Starting from zero, assign a group ID for each sensor. Look for the
associated flash device node and based on nodes found, assign the same group
ID for them.

Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
---
 drivers/media/platform/omap3isp/isp.c | 42 ++++++++++++++++++++++++++++++++---
 drivers/media/platform/omap3isp/isp.h |  1 +
 2 files changed, 40 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c
index a1c3bdb..f89abea 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -2152,7 +2152,8 @@ static int isp_of_parse_node_endpoint(struct device *dev,
 }
 
 static int isp_of_parse_node(struct device *dev, struct device_node *node,
-			     struct v4l2_async_notifier *notifier, bool link)
+			     struct v4l2_async_notifier *notifier,
+			     u32 group_id, bool link)
 {
 	struct isp_async_subdev *isd;
 
@@ -2182,6 +2183,7 @@ static int isp_of_parse_node(struct device *dev, struct device_node *node,
 	}
 
 	isd->asd.match_type = V4L2_ASYNC_MATCH_OF;
+	isd->group_id = group_id;
 	notifier->num_subdevs++;
 
 	return 0;
@@ -2193,6 +2195,7 @@ static int isp_of_parse_nodes(struct device *dev,
 	struct device_node *node = NULL;
 	int ret;
 	unsigned int flash = 0;
+	u32 group_id = 0;
 
 	notifier->subdevs = devm_kcalloc(
 		dev, ISP_MAX_SUBDEVS, sizeof(*notifier->subdevs), GFP_KERNEL);
@@ -2201,7 +2204,7 @@ static int isp_of_parse_nodes(struct device *dev,
 
 	while (notifier->num_subdevs < ISP_MAX_SUBDEVS &&
 	       (node = of_graph_get_next_endpoint(dev->of_node, node))) {
-		ret = isp_of_parse_node(dev, node, notifier, true);
+		ret = isp_of_parse_node(dev, node, notifier, group_id++, true);
 		if (ret)
 			return ret;
 	}
@@ -2213,11 +2216,43 @@ static int isp_of_parse_nodes(struct device *dev,
 			of_parse_phandle(dev->of_node, "ti,camera-flashes",
 					 flash++);
 		unsigned int i;
+		u32 flash_group_id;
 
 		if (!sensor_node)
 			return -EINVAL;
 
-		ret = isp_of_parse_node(dev, node, notifier, false);
+		for (i = 0; i < notifier->num_subdevs; i++) {
+			struct isp_async_subdev *isd = container_of(
+				notifier->subdevs[i], struct isp_async_subdev,
+				asd);
+
+			if (!isd->bus)
+				continue;
+
+			dev_dbg(dev, "match \"%s\", \"%s\"\n",sensor_node->name,
+				isd->asd.match.of.node->name);
+
+			if (sensor_node != isd->asd.match.of.node)
+				continue;
+
+			dev_dbg(dev, "found\n");
+
+			flash_group_id = isd->group_id;
+			break;
+		}
+
+		/*
+		 * No sensor was found --- complain and allocate a new
+		 * group ID.
+		 */
+		if (i == notifier->num_subdevs) {
+			dev_warn(dev, "no device node \"%s\" was found",
+				 sensor_node->name);
+			flash_group_id = group_id++;
+		}
+
+		ret = isp_of_parse_node(dev, node, notifier, flash_group_id,
+					false);
 		if (ret)
 			return ret;
 	}
@@ -2232,6 +2267,7 @@ static int isp_subdev_notifier_bound(struct v4l2_async_notifier *async,
 	struct isp_async_subdev *isd =
 		container_of(asd, struct isp_async_subdev, asd);
 
+	subdev->entity.group_id = isd->group_id;
 	isd->sd = subdev;
 	isd->sd->host_priv = isd->bus;
 
diff --git a/drivers/media/platform/omap3isp/isp.h b/drivers/media/platform/omap3isp/isp.h
index c0b9d1d..639b3ca 100644
--- a/drivers/media/platform/omap3isp/isp.h
+++ b/drivers/media/platform/omap3isp/isp.h
@@ -230,6 +230,7 @@ struct isp_async_subdev {
 	struct v4l2_subdev *sd;
 	struct isp_bus_cfg *bus;
 	struct v4l2_async_subdev asd;
+	u32 group_id;
 };
 
 #define v4l2_dev_to_isp_device(dev) \
-- 
2.1.4


[-- Attachment #6: 0005-Fix-work-around-compilation.patch --]
[-- Type: text/x-diff, Size: 833 bytes --]

>From 70e7eada67a5a4170f2a9e268d8134a7c773e225 Mon Sep 17 00:00:00 2001
From: Pavel <pavel@ucw.cz>
Date: Fri, 29 Apr 2016 12:13:24 +0200
Subject: [PATCH 5/5] Fix (work around?) compilation.

---
 drivers/media/platform/omap3isp/isp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c
index f89abea..dd96a4b 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -2267,7 +2267,7 @@ static int isp_subdev_notifier_bound(struct v4l2_async_notifier *async,
 	struct isp_async_subdev *isd =
 		container_of(asd, struct isp_async_subdev, asd);
 
-	subdev->entity.group_id = isd->group_id;
+//	subdev->entity.group_id = isd->group_id;
 	isd->sd = subdev;
 	isd->sd->host_priv = isd->bus;
 
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: v4l subdevs without big device was Re: drivers/media/i2c/adp1653.c: does not show as /dev/video* or v4l-subdev*
  2016-04-29  9:50       ` Pavel Machek
@ 2016-04-29 10:59         ` Sakari Ailus
  2016-04-29 11:05           ` Pali Rohár
  2016-04-29 14:06           ` Pavel Machek
  0 siblings, 2 replies; 19+ messages in thread
From: Sakari Ailus @ 2016-04-29 10:59 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Hans Verkuil, pali.rohar, sre, kernel list, linux-arm-kernel,
	linux-omap, tony, khilman, aaro.koskinen, ivo.g.dimitrov.75,
	patrikbachan, serge, tuukkat76, mchehab, linux-media

Hi Pavel,

On Fri, Apr 29, 2016 at 11:50:02AM +0200, Pavel Machek wrote:
> Hi!
> 
> > > > adp1653 registers itself as a subdev, but there's no device that
> > > > register it as its part.
> > > > 
> > > > (ad5820 driver seems to have the same problem).
> > > > 
> > > > Is there example "dummy" device I could use, for sole purpose of
> > > > having these devices appear in /dev? They are on i2c, so both can work
> > > > on their own.
> > > 
> > > Ah, interesting. This was discussed a little bit during the Media Summit
> > > a few weeks back:
> > > 
> > > http://linuxtv.org/news.php?entry=2016-04-20.mchehab
> > > 
> > > See section 5:
> > > 
> > > "5. DT Bindings for flash & lens controllers
> > > 
> > > There are drivers that create their MC topology using the device tree information,
> > > which works great for entities that transport data, but how to detect entities
> > > that don’t transport data such as flash devices, focusers, etc.? How can those be
> > > deduced using the device tree?
> > > 
> > > Sensor DT node add phandle to focus controller: add generic v4l binding properties
> > > to reference such devices."
> > > 
> > > This wasn't a problem with the original N900 since that didn't use DT AFAIK and
> > > these devices were loaded explicitly through board code.
> 
> > > But now you run into the same problem that I have.
> 
> Actually... being able to do board-code solution for testing for now
> would be nice...
> 
> > > 
> > > The solution is that sensor devices have to provide phandles to those controller
> > > devices. And to do that you need to define bindings which is always the hard part.
> > > 
> > > Look in Documentation/devicetree/bindings/media/video-interfaces.txt, section
> > > "Optional endpoint properties".
> > > 
> > > Something like:
> > > 
> > > controllers: an array of phandles to controller devices associated with this
> > > endpoint such as flash and lens controllers.
> > > 
> > > Warning: I'm no DT expert, so this is just a first attempt.
> > > 
> > > Platform drivers (omap3isp) will have to add these controller devices to the list
> > > of subdevs to load asynchronously.
> > 
> > I seem to have patches I haven't had time to push back then:
> > 
> > <URL:http://salottisipuli.retiisi.org.uk/cgi-bin/gitweb.cgi?p=~sailus/linux.git;a=shortlog;h=refs/heads/leds-as3645a>
> >
> 
> That gitweb is a bit confused about its own address, but I figured it
> out. Let me check...
> 
> pavel@amd:/data/l/linux-n900$ git fetch
> git://git.retiisi.org.uk/~sailus/linux.git leds-as3645a:leds-as3645a
> fatal: unable to connect to git.retiisi.org.uk:
> git.retiisi.org.uk: Name or service not known
> 
> pavel@amd:/data/l/linux-n900$ git fetch
> git://salottisipuli.retiisi.org.uk/~sailus/linux.git
> leds-as3645a:leds-as3645a
> remote: Counting objects: 132, done.
> remote: Compressing objects: 100% (46/46), done.
> remote: Total 132 (delta 111), reused 107 (delta 86)
> Receiving objects: 100% (132/132), 22.80 KiB | 0 bytes/s, done.
> Resolving deltas: 100% (111/111), completed with 34 local objects.
> From git://salottisipuli.retiisi.org.uk/~sailus/linux
>  * [new branch]      leds-as3645a -> leds-as3645a

Yeah, that works, too. git alias has been added some three weeks ago so
there seem to be something strange going on with DNS.

>  
> 
> > This seems to be mostly in line with what has been discussed in the meeting,
> > except that the patches add a device specific property. Please ignore the
> > led patches in that tree for now (i.e. four patches on the top are the
> > relevant ones here).
> > 
> 
> I'm currently trying to apply them to v4.6, but am getting rather ugly
> rejects :-(.

:-\

There have been patches applied to the omap3isp driver since that I suppose.
These aren't overly complex, feel free to take the patches if they're still
useful.

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: v4l subdevs without big device was Re: drivers/media/i2c/adp1653.c: does not show as /dev/video* or v4l-subdev*
  2016-04-29 10:59         ` Sakari Ailus
@ 2016-04-29 11:05           ` Pali Rohár
  2016-04-29 11:23             ` Sakari Ailus
  2016-04-29 14:06           ` Pavel Machek
  1 sibling, 1 reply; 19+ messages in thread
From: Pali Rohár @ 2016-04-29 11:05 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: Pavel Machek, Hans Verkuil, sre, kernel list, linux-arm-kernel,
	linux-omap, tony, khilman, aaro.koskinen, ivo.g.dimitrov.75,
	patrikbachan, serge, tuukkat76, mchehab, linux-media

On Friday 29 April 2016 13:59:44 Sakari Ailus wrote:
> > pavel@amd:/data/l/linux-n900$ git fetch
> > git://git.retiisi.org.uk/~sailus/linux.git leds-as3645a:leds-as3645a
> > fatal: unable to connect to git.retiisi.org.uk:
> > git.retiisi.org.uk: Name or service not known
> > 
> > pavel@amd:/data/l/linux-n900$ git fetch
> > git://salottisipuli.retiisi.org.uk/~sailus/linux.git
> > leds-as3645a:leds-as3645a
> > remote: Counting objects: 132, done.
> > remote: Compressing objects: 100% (46/46), done.
> > remote: Total 132 (delta 111), reused 107 (delta 86)
> > Receiving objects: 100% (132/132), 22.80 KiB | 0 bytes/s, done.
> > Resolving deltas: 100% (111/111), completed with 34 local objects.
> > From git://salottisipuli.retiisi.org.uk/~sailus/linux
> >  * [new branch]      leds-as3645a -> leds-as3645a
> 
> Yeah, that works, too. git alias has been added some three weeks ago so
> there seem to be something strange going on with DNS.

Maybe update SOA record?

-- 
Pali Rohár
pali.rohar@gmail.com

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: v4l subdevs without big device was Re: drivers/media/i2c/adp1653.c: does not show as /dev/video* or v4l-subdev*
  2016-04-29 11:05           ` Pali Rohár
@ 2016-04-29 11:23             ` Sakari Ailus
  0 siblings, 0 replies; 19+ messages in thread
From: Sakari Ailus @ 2016-04-29 11:23 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Pavel Machek, Hans Verkuil, sre, kernel list, linux-arm-kernel,
	linux-omap, tony, khilman, aaro.koskinen, ivo.g.dimitrov.75,
	patrikbachan, serge, tuukkat76, mchehab, linux-media

On Fri, Apr 29, 2016 at 01:05:46PM +0200, Pali Rohár wrote:
> On Friday 29 April 2016 13:59:44 Sakari Ailus wrote:
> > > pavel@amd:/data/l/linux-n900$ git fetch
> > > git://git.retiisi.org.uk/~sailus/linux.git leds-as3645a:leds-as3645a
> > > fatal: unable to connect to git.retiisi.org.uk:
> > > git.retiisi.org.uk: Name or service not known
> > > 
> > > pavel@amd:/data/l/linux-n900$ git fetch
> > > git://salottisipuli.retiisi.org.uk/~sailus/linux.git
> > > leds-as3645a:leds-as3645a
> > > remote: Counting objects: 132, done.
> > > remote: Compressing objects: 100% (46/46), done.
> > > remote: Total 132 (delta 111), reused 107 (delta 86)
> > > Receiving objects: 100% (132/132), 22.80 KiB | 0 bytes/s, done.
> > > Resolving deltas: 100% (111/111), completed with 34 local objects.
> > > From git://salottisipuli.retiisi.org.uk/~sailus/linux
> > >  * [new branch]      leds-as3645a -> leds-as3645a
> > 
> > Yeah, that works, too. git alias has been added some three weeks ago so
> > there seem to be something strange going on with DNS.
> 
> Maybe update SOA record?

The host has been added before that. It looks like the slaves are performing
the zone transfer nicely but for some reason they don't seem to correctly
respond when the newly added name is queried.

-- 
Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: v4l subdevs without big device was Re: drivers/media/i2c/adp1653.c: does not show as /dev/video* or v4l-subdev*
  2016-04-29 10:59         ` Sakari Ailus
  2016-04-29 11:05           ` Pali Rohár
@ 2016-04-29 14:06           ` Pavel Machek
  1 sibling, 0 replies; 19+ messages in thread
From: Pavel Machek @ 2016-04-29 14:06 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: Hans Verkuil, pali.rohar, sre, kernel list, linux-arm-kernel,
	linux-omap, tony, khilman, aaro.koskinen, ivo.g.dimitrov.75,
	patrikbachan, serge, tuukkat76, mchehab, linux-media

Hi!

> > > This seems to be mostly in line with what has been discussed in the meeting,
> > > except that the patches add a device specific property. Please ignore the
> > > led patches in that tree for now (i.e. four patches on the top are the
> > > relevant ones here).
> > > 
> > 
> > I'm currently trying to apply them to v4.6, but am getting rather ugly
> > rejects :-(.
> 
> :-\
> 
> There have been patches applied to the omap3isp driver since that I suppose.
> These aren't overly complex, feel free to take the patches if they're still
> useful.

Ok, I got it to work. I can split it back, if needed. I've got patches
on camera-fm3 branch. And yes, that gets flash to work.

(I don't know how to turn flash into torch, which is what I really
wanted, but I guess I'll figure it out.)

									Pavel

diff --git a/arch/arm/boot/dts/omap3-n900.dts b/arch/arm/boot/dts/omap3-n900.dts
index 640d409..a6b9fac 100644
--- a/arch/arm/boot/dts/omap3-n900.dts
+++ b/arch/arm/boot/dts/omap3-n900.dts
@@ -239,6 +239,7 @@
 
 	pinctrl-names = "default";
 	pinctrl-0 = <&camera_pins>;
+	ti,camera-flashes = <&adp1653 &cam1>;
 
 	ports {
 		port@1 {
diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c
index 6361fde..23d484c 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -2095,13 +2095,20 @@ static void isp_of_parse_node_csi2(struct device *dev,
 	buscfg->bus.csi2.crc = 1;
 }
 
-static int isp_of_parse_node(struct device *dev, struct device_node *node,
-			     struct isp_async_subdev *isd)
+static int isp_of_parse_node_endpoint(struct device *dev,
+				      struct device_node *node,
+				      struct isp_async_subdev *isd)
 {
-	struct isp_bus_cfg *buscfg = &isd->bus;
+	struct isp_bus_cfg *buscfg;
 	struct v4l2_of_endpoint vep;
 	int ret;
 
+	isd->bus = devm_kzalloc(dev, sizeof(*isd->bus), GFP_KERNEL);
+	if (!isd->bus)
+		return -ENOMEM;
+
+	buscfg = isd->bus;
+
 	ret = v4l2_of_parse_endpoint(node, &vep);
 	if (ret)
 		return ret;
@@ -2144,10 +2151,51 @@ static int isp_of_parse_node(struct device *dev, struct device_node *node,
 	return 0;
 }
 
+static int isp_of_parse_node(struct device *dev, struct device_node *node,
+			     struct v4l2_async_notifier *notifier,
+			     u32 group_id, bool link)
+{
+	struct isp_async_subdev *isd;
+
+	isd = devm_kzalloc(dev, sizeof(*isd), GFP_KERNEL);
+	if (!isd) {
+		of_node_put(node);
+		return -ENOMEM;
+	}
+
+	notifier->subdevs[notifier->num_subdevs] = &isd->asd;
+
+	if (link) {
+		if (isp_of_parse_node_endpoint(dev, node, isd)) {
+			of_node_put(node);
+			return -EINVAL;
+		}
+
+		isd->asd.match.of.node = of_graph_get_remote_port_parent(node);
+		of_node_put(node);
+	} else {
+		isd->asd.match.of.node = node;
+	}
+
+	if (!isd->asd.match.of.node) {
+		dev_warn(dev, "bad remote port parent\n");
+		return -EINVAL;
+	}
+
+	isd->asd.match_type = V4L2_ASYNC_MATCH_OF;
+	isd->group_id = group_id;
+	notifier->num_subdevs++;
+
+	return 0;
+}
+
 static int isp_of_parse_nodes(struct device *dev,
 			      struct v4l2_async_notifier *notifier)
 {
 	struct device_node *node = NULL;
+	int ret;
+	unsigned int flash = 0;
+	u32 group_id = 0;
 
 	notifier->subdevs = devm_kcalloc(
 		dev, ISP_MAX_SUBDEVS, sizeof(*notifier->subdevs), GFP_KERNEL);
@@ -2156,30 +2204,57 @@ static int isp_of_parse_nodes(struct device *dev,
 
 	while (notifier->num_subdevs < ISP_MAX_SUBDEVS &&
 	       (node = of_graph_get_next_endpoint(dev->of_node, node))) {
-		struct isp_async_subdev *isd;
+		ret = isp_of_parse_node(dev, node, notifier, group_id++, true);
+		if (ret)
+			return ret;
+	}
 
-		isd = devm_kzalloc(dev, sizeof(*isd), GFP_KERNEL);
-		if (!isd) {
-			of_node_put(node);
-			return -ENOMEM;
-		}
+	while (notifier->num_subdevs < ISP_MAX_SUBDEVS &&
+	       (node = of_parse_phandle(dev->of_node, "ti,camera-flashes",
+					flash++))) {
+		struct device_node *sensor_node =
+			of_parse_phandle(dev->of_node, "ti,camera-flashes",
+					 flash++);
+		unsigned int i;
+		u32 flash_group_id;
+
+		if (!sensor_node)
+			return -EINVAL;
 
-		notifier->subdevs[notifier->num_subdevs] = &isd->asd;
+		for (i = 0; i < notifier->num_subdevs; i++) {
+			struct isp_async_subdev *isd = container_of(
+				notifier->subdevs[i], struct isp_async_subdev,
+				asd);
 
-		if (isp_of_parse_node(dev, node, isd)) {
-			of_node_put(node);
-			return -EINVAL;
+			if (!isd->bus)
+				continue;
+
+			dev_dbg(dev, "match \"%s\", \"%s\"\n",sensor_node->name,
+				isd->asd.match.of.node->name);
+
+			if (sensor_node != isd->asd.match.of.node)
+				continue;
+
+			dev_dbg(dev, "found\n");
+
+			flash_group_id = isd->group_id;
+			break;
 		}
 
-		isd->asd.match.of.node = of_graph_get_remote_port_parent(node);
-		of_node_put(node);
-		if (!isd->asd.match.of.node) {
-			dev_warn(dev, "bad remote port parent\n");
-			return -EINVAL;
+		/*
+		 * No sensor was found --- complain and allocate a new
+		 * group ID.
+		 */
+		if (i == notifier->num_subdevs) {
+			dev_warn(dev, "no device node \"%s\" was found",
+				 sensor_node->name);
+			flash_group_id = group_id++;
 		}
 
-		isd->asd.match_type = V4L2_ASYNC_MATCH_OF;
-		notifier->num_subdevs++;
+		ret = isp_of_parse_node(dev, node, notifier, flash_group_id,
+					false);
+		if (ret)
+			return ret;
 	}
 
 	return notifier->num_subdevs;
@@ -2192,8 +2267,9 @@ static int isp_subdev_notifier_bound(struct v4l2_async_notifier *async,
 	struct isp_async_subdev *isd =
 		container_of(asd, struct isp_async_subdev, asd);
 
+//	subdev->entity.group_id = isd->group_id;
 	isd->sd = subdev;
-	isd->sd->host_priv = &isd->bus;
+	isd->sd->host_priv = isd->bus;
 
 	return 0;
 }
@@ -2396,12 +2472,15 @@ static int isp_probe(struct platform_device *pdev)
 	if (ret < 0)
 		goto error_register_entities;
 
-	isp->notifier.bound = isp_subdev_notifier_bound;
-	isp->notifier.complete = isp_subdev_notifier_complete;
+	if (IS_ENABLED(CONFIG_OF) && pdev->dev.of_node) {
+		isp->notifier.bound = isp_subdev_notifier_bound;
+		isp->notifier.complete = isp_subdev_notifier_complete;
 
-	ret = v4l2_async_notifier_register(&isp->v4l2_dev, &isp->notifier);
-	if (ret)
-		goto error_register_entities;
+		ret = v4l2_async_notifier_register(&isp->v4l2_dev,
+						   &isp->notifier);
+		if (ret)
+			goto error_register_entities;
+	}
 
 	isp_core_init(isp, 1);
 	omap3isp_put(isp);
diff --git a/drivers/media/platform/omap3isp/isp.h b/drivers/media/platform/omap3isp/isp.h
index 7e6f663..639b3ca 100644
--- a/drivers/media/platform/omap3isp/isp.h
+++ b/drivers/media/platform/omap3isp/isp.h
@@ -228,8 +228,9 @@ struct isp_device {
 
 struct isp_async_subdev {
 	struct v4l2_subdev *sd;
-	struct isp_bus_cfg bus;
+	struct isp_bus_cfg *bus;
 	struct v4l2_async_subdev asd;
+	u32 group_id;
 };
 
 #define v4l2_dev_to_isp_device(dev) \
diff --git a/drivers/media/platform/omap3isp/ispcsiphy.c b/drivers/media/platform/omap3isp/ispcsiphy.c
index 495447d..750ce93 100644
--- a/drivers/media/platform/omap3isp/ispcsiphy.c
+++ b/drivers/media/platform/omap3isp/ispcsiphy.c
@@ -177,7 +177,7 @@ static int omap3isp_csiphy_config(struct isp_csiphy *phy)
 		struct isp_async_subdev *isd =
 			container_of(pipe->external->asd,
 				     struct isp_async_subdev, asd);
-		buscfg = &isd->bus;
+		buscfg = isd->bus;
 	}
 
 	if (buscfg->interface == ISP_INTERFACE_CCP2B_PHY1


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [pre-rfc] focus and flash for Nokia N900 (was Re: v4l subdevs without big device was Re: drivers/media/i2c/adp1653.c: does not show as /dev/video* or v4l-subdev*)
  2016-04-29  7:31   ` Hans Verkuil
  2016-04-29  7:56     ` Sakari Ailus
@ 2016-04-29 21:30     ` Pavel Machek
  2016-04-29 22:13     ` camera application for testing (was Re: v4l subdevs without big device) Pavel Machek
  2 siblings, 0 replies; 19+ messages in thread
From: Pavel Machek @ 2016-04-29 21:30 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: pali.rohar, sre, kernel list, linux-arm-kernel, linux-omap, tony,
	khilman, aaro.koskinen, ivo.g.dimitrov.75, patrikbachan, serge,
	sakari.ailus, tuukkat76, mchehab, linux-media

Hi!

> "5. DT Bindings for flash & lens controllers
> 
> There are drivers that create their MC topology using the device tree information,
> which works great for entities that transport data, but how to detect entities
> that don’t transport data such as flash devices, focusers, etc.? How can those be
> deduced using the device tree?
> 
> Sensor DT node add phandle to focus controller: add generic v4l binding properties
> to reference such devices."
> 
> This wasn't a problem with the original N900 since that didn't use DT AFAIK and
> these devices were loaded explicitly through board code.
> 
> But now you run into the same problem that I have.
> 
> The solution is that sensor devices have to provide phandles to those controller
> devices. And to do that you need to define bindings which is always the hard part.
> 
> Look in Documentation/devicetree/bindings/media/video-interfaces.txt, section
> "Optional endpoint properties".
> 
> Something like:
> 
> controllers: an array of phandles to controller devices associated with this
> endpoint such as flash and lens controllers.

Ok, so after a big fight, I got both auto focus and flash to work on
n900. Relative to N900 camera trees recently posted.

Subdevs behave rather funny, and --sleep-forever is needed for useful
operation.

YA=/my/tui/yavta/yavta
# torch
sudo $YA --sleep-forever --set-control '0x009c0901 2'  /dev/v4l-subdev11

# focus -- near
sudo $YA --sleep-forever --set-control '0x009a090a 1023' /dev/l-subdev12

Signed-off-by: Pavel Machek <pavel@ucw.cz>

diff --git a/arch/arm/boot/dts/omap3-n900.dts b/arch/arm/boot/dts/omap3-n900.dts
index 9c9c1e8..acf1457 100644
--- a/arch/arm/boot/dts/omap3-n900.dts
+++ b/arch/arm/boot/dts/omap3-n900.dts
@@ -239,6 +239,7 @@
 
 	pinctrl-names = "default";
 	pinctrl-0 = <&camera_pins>;
+	ti,camera-flashes = <&adp1653 &cam1 &ad5820 &cam1>;
 
 	ports {
 		port@1 {
@@ -251,7 +252,7 @@
 				data-lanes = <1>;
 				lane-polarity = <0 0>;
 				clock-inv = <0>;
-				strobe = <0>;
+				strobe = <1>;
 				crc = <0>;
 			};
 		};
@@ -879,6 +880,16 @@
 			};
 		};
 	};
+
+	/* D/A converter for auto-focus */
+	ad5820: dac@0c {
+		compatible = "adi,ad5820";
+		reg = <0x0c>;
+
+		VANA-supply = <&vaux4>;
+
+		#io-channel-cells = <0>;
+	};
 };
 
 &mmc1 {
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 254c106..77313a1 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -279,6 +279,13 @@ config VIDEO_ML86V7667
 	  To compile this driver as a module, choose M here: the
 	  module will be called ml86v7667.
 
+config VIDEO_AD5820
+	tristate "AD5820 lens voice coil support"
+	depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
+	---help---
+	  This is a driver for the AD5820 camera lens voice coil.
+	  It is used for example in Nokia N900 (RX-51).
+
 config VIDEO_SAA7110
 	tristate "Philips SAA7110 video decoder"
 	depends on VIDEO_V4L2 && I2C
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index 05e79aa..34434ae 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -20,6 +20,7 @@ obj-$(CONFIG_VIDEO_SAA717X) += saa717x.o
 obj-$(CONFIG_VIDEO_SAA7127) += saa7127.o
 obj-$(CONFIG_VIDEO_SAA7185) += saa7185.o
 obj-$(CONFIG_VIDEO_SAA6752HS) += saa6752hs.o
+obj-$(CONFIG_VIDEO_AD5820)  += ad5820.o
 obj-$(CONFIG_VIDEO_ADV7170) += adv7170.o
 obj-$(CONFIG_VIDEO_ADV7175) += adv7175.o
 obj-$(CONFIG_VIDEO_ADV7180) += adv7180.o
diff --git a/drivers/media/i2c/ad5820.c b/drivers/media/i2c/ad5820.c
new file mode 100644
index 0000000..5aee185
--- /dev/null
+++ b/drivers/media/i2c/ad5820.c
@@ -0,0 +1,526 @@
+/*
+ * drivers/media/i2c/ad5820.c
+ *
+ * AD5820 DAC driver for camera voice coil focus.
+ *
+ * Copyright (C) 2008 Nokia Corporation
+ * Copyright (C) 2007 Texas Instruments
+ *
+ * Contact: Tuukka Toivonen <tuukka.o.toivonen@nokia.com>
+ *          Sakari Ailus <sakari.ailus@nokia.com>
+ *
+ * Based on af_d88.c by Texas Instruments.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
+ * 02110-1301 USA
+ */
+
+#include <linux/module.h>
+#include <linux/errno.h>
+#include <linux/i2c.h>
+#include <linux/slab.h>
+#include <linux/sched.h>
+#include <linux/delay.h>
+#include <linux/bitops.h>
+#include <linux/kernel.h>
+#include <linux/regulator/consumer.h>
+
+#include <media/ad5820.h>
+#include <media/v4l2-device.h>
+
+#define CODE_TO_RAMP_US(s)	((s) == 0 ? 0 : (1 << ((s) - 1)) * 50)
+#define RAMP_US_TO_CODE(c)	fls(((c) + ((c)>>1)) / 50)
+
+/**
+ * @brief I2C write using i2c_transfer().
+ * @param coil - the driver data structure
+ * @param data - register value to be written
+ * @returns nonnegative on success, negative if failed
+ */
+static int ad5820_write(struct ad5820_device *coil, u16 data)
+{
+	struct i2c_client *client = v4l2_get_subdevdata(&coil->subdev);
+	struct i2c_msg msg;
+	int r;
+
+	if (!client->adapter)
+		return -ENODEV;
+
+	data = cpu_to_be16(data);
+	msg.addr  = client->addr;
+	msg.flags = 0;
+	msg.len   = 2;
+	msg.buf   = (u8 *)&data;
+
+	r = i2c_transfer(client->adapter, &msg, 1);
+	if (r < 0) {
+		dev_err(&client->dev, "write failed, error %d\n", r);
+		return r;
+	}
+
+	return 0;
+}
+
+/**
+ * @brief I2C read using i2c_transfer().
+ * @param coil - the driver data structure
+ * @returns unsigned 16-bit register value on success, negative if failed
+ */
+static int ad5820_read(struct ad5820_device *coil)
+{
+	struct i2c_client *client = v4l2_get_subdevdata(&coil->subdev);
+	struct i2c_msg msg;
+	int r;
+	u16 data = 0;
+
+	if (!client->adapter)
+		return -ENODEV;
+
+	msg.addr  = client->addr;
+	msg.flags = I2C_M_RD;
+	msg.len   = 2;
+	msg.buf   = (u8 *)&data;
+
+	r = i2c_transfer(client->adapter, &msg, 1);
+	if (r < 0) {
+		dev_err(&client->dev, "read failed, error %d\n", r);
+		return r;
+	}
+
+	return be16_to_cpu(data);
+}
+
+/*
+ * Calculate status word and write it to the device based on current
+ * values of V4L2 controls. It is assumed that the stored V4L2 control
+ * values are properly limited and rounded.
+ */
+static int ad5820_update_hw(struct ad5820_device *coil)
+{
+	u16 status;
+
+	status = RAMP_US_TO_CODE(coil->focus_ramp_time);
+	status |= coil->focus_ramp_mode
+		? AD5820_RAMP_MODE_64_16 : AD5820_RAMP_MODE_LINEAR;
+	status |= coil->focus_absolute << AD5820_DAC_SHIFT;
+
+	if (coil->standby)
+		status |= AD5820_POWER_DOWN;
+
+	return ad5820_write(coil, status);
+}
+
+/*
+ * Power handling
+ */
+static int ad5820_power_off(struct ad5820_device *coil, int standby)
+{
+	int ret = 0;
+
+	/*
+	 * Go to standby first as real power off my be denied by the hardware
+	 * (single power line control for both coil and sensor).
+	 */
+	if (standby) {
+		coil->standby = 1;
+		ret = ad5820_update_hw(coil);
+	}
+
+//	ret |= coil->platform_data->set_xshutdown(&coil->subdev, 0);
+	ret |= regulator_disable(coil->vana);
+
+	return ret;
+}
+
+static int ad5820_power_on(struct ad5820_device *coil, int restore)
+{
+	int ret;
+
+	printk("ad5820_power_on: 1\n");
+	ret = regulator_enable(coil->vana);
+	if (ret < 0)
+		return ret;
+
+	printk("ad5820_power_on: 2\n");
+#if 0	
+	printk("ad5820_power_on: pd %lx\n", coil->platform_data);
+	printk("ad5820_power_on: xs %lx\n", coil->platform_data->set_xshutdown);
+	ret = coil->platform_data->set_xshutdown(&coil->subdev, 1);
+	if (ret)
+		goto fail;
+#endif
+
+	printk("ad5820_power_on: 3\n");
+	if (restore) {
+		/* Restore the hardware settings. */
+		coil->standby = 0;
+		printk("ad5820_power_on: 4\n");		
+		ret = ad5820_update_hw(coil);
+		if (ret)
+			goto fail;
+	}
+	printk("ad5820_power_on: 5\n"); 
+	return 0;
+
+fail:
+	coil->standby = 1;
+
+#if 0
+	coil->platform_data->set_xshutdown(&coil->subdev, 0);
+#endif
+	regulator_disable(coil->vana);
+
+	return ret;
+}
+
+/*
+ * V4L2 controls
+ */
+static int ad5820_set_ctrl(struct v4l2_ctrl *ctrl)
+{
+	struct ad5820_device *coil =
+		container_of(ctrl->handler, struct ad5820_device, ctrls);
+	u32 code;
+	int r = 0;
+
+	switch (ctrl->id) {
+	case V4L2_CID_FOCUS_ABSOLUTE:
+		coil->focus_absolute = ctrl->val;
+		return ad5820_update_hw(coil);
+
+	case V4L2_CID_FOCUS_AD5820_RAMP_TIME:
+		code = RAMP_US_TO_CODE(ctrl->val);
+		ctrl->val = CODE_TO_RAMP_US(code);
+		coil->focus_ramp_time = ctrl->val;
+		break;
+
+	case V4L2_CID_FOCUS_AD5820_RAMP_MODE:
+		coil->focus_ramp_mode = ctrl->val;
+		break;
+	}
+
+	return r;
+}
+
+static const struct v4l2_ctrl_ops ad5820_ctrl_ops = {
+	.s_ctrl = ad5820_set_ctrl,
+};
+
+static const char *ad5820_focus_menu[] = {
+	"Linear ramp",
+	"64/16 ramp",
+};
+
+static const struct v4l2_ctrl_config ad5820_ctrls[] = {
+	{
+		.ops		= &ad5820_ctrl_ops,
+		.id		= V4L2_CID_FOCUS_AD5820_RAMP_TIME,
+		.type		= V4L2_CTRL_TYPE_INTEGER,
+		.name		= "Focus ramping time [us]",
+		.min		= 0,
+		.max		= 3200,
+		.step		= 50,
+		.def		= 0,
+		.flags		= 0,
+	},
+	{
+		.ops		= &ad5820_ctrl_ops,
+		.id		= V4L2_CID_FOCUS_AD5820_RAMP_MODE,
+		.type		= V4L2_CTRL_TYPE_MENU,
+		.name		= "Focus ramping mode",
+		.min		= 0,
+		.max		= ARRAY_SIZE(ad5820_focus_menu),
+		.step		= 0,
+		.def		= 0,
+		.flags		= 0,
+		.qmenu		= ad5820_focus_menu,
+	},
+};
+
+
+static int ad5820_init_controls(struct ad5820_device *coil)
+{
+	unsigned int i;
+
+	v4l2_ctrl_handler_init(&coil->ctrls, ARRAY_SIZE(ad5820_ctrls) + 1);
+
+	/*
+	 * V4L2_CID_FOCUS_ABSOLUTE
+	 *
+	 * Minimum current is 0 mA, maximum is 100 mA. Thus, 1 code is
+	 * equivalent to 100/1023 = 0.0978 mA. Nevertheless, we do not use [mA]
+	 * for focus position, because it is meaningless for user. Meaningful
+	 * would be to use focus distance or even its inverse, but since the
+	 * driver doesn't have sufficiently knowledge to do the conversion, we
+	 * will just use abstract codes here. In any case, smaller value = focus
+	 * position farther from camera. The default zero value means focus at
+	 * infinity, and also least current consumption.
+	 */
+	v4l2_ctrl_new_std(&coil->ctrls, &ad5820_ctrl_ops,
+			  V4L2_CID_FOCUS_ABSOLUTE, 0, 1023, 1, 0);
+
+	/* V4L2_CID_TEST_PATTERN and V4L2_CID_MODE_* */
+	for (i = 0; i < ARRAY_SIZE(ad5820_ctrls); ++i)
+		v4l2_ctrl_new_custom(&coil->ctrls, &ad5820_ctrls[i], NULL);
+
+	if (coil->ctrls.error)
+		return coil->ctrls.error;
+
+	coil->focus_absolute = 0;
+	coil->focus_ramp_time = 0;
+	coil->focus_ramp_mode = 0;
+
+	coil->subdev.ctrl_handler = &coil->ctrls;
+	return 0;
+}
+
+/*
+ * V4L2 subdev operations
+ */
+static int
+ad5820_registered(struct v4l2_subdev *subdev)
+{
+	static const int CHECK_VALUE = 0x3FF0;
+
+	struct ad5820_device *coil = to_ad5820_device(subdev);
+	struct i2c_client *client = v4l2_get_subdevdata(subdev);
+	u16 status = AD5820_POWER_DOWN | CHECK_VALUE;
+	int rval;
+
+	printk("registered\n");
+	coil->vana = regulator_get(&client->dev, "VANA");
+	if (IS_ERR(coil->vana)) {
+		dev_err(&client->dev, "could not get regulator for vana\n");
+		return -ENODEV;
+	}
+#if 0
+	printk("detect\n");
+	/* Detect that the chip is there */
+	rval = ad5820_power_on(coil, 0);
+	if (rval)
+		goto not_detected;
+	rval = ad5820_write(coil, status);
+	if (rval)
+		goto not_detected;
+	rval = ad5820_read(coil);
+	if (rval != status)
+		goto not_detected;
+
+
+	{
+		int i, j;
+		for (j = 0; j<5; j++) {
+			printk("hwtest: phase %d\n", j);
+			for (i=0; i<1023; i++) {
+				coil->focus_absolute = i;
+				msleep(1);
+				ad5820_update_hw(coil);
+			}
+		}
+	}	
+
+	printk("detect ok, poweroff\n");	
+	ad5820_power_off(coil, 1);
+#endif
+	printk("controls\n");	
+	return ad5820_init_controls(coil);
+
+not_detected:
+	dev_err(&client->dev, "not detected\n");
+	ad5820_power_off(coil, 0);
+	regulator_put(coil->vana);
+	return -ENODEV;
+}
+
+static int
+ad5820_set_power(struct v4l2_subdev *subdev, int on)
+{
+	struct ad5820_device *coil = to_ad5820_device(subdev);
+	int ret = 0;
+
+	mutex_lock(&coil->power_lock);
+
+	/*
+	 * If the power count is modified from 0 to != 0 or from != 0 to 0,
+	 * update the power state.
+	 */
+	if (coil->power_count == !on) {
+		ret = on ? ad5820_power_on(coil, 1) : ad5820_power_off(coil, 1);
+		if (ret < 0)
+			goto done;
+	}
+
+	/* Update the power count. */
+	coil->power_count += on ? 1 : -1;
+	WARN_ON(coil->power_count < 0);
+
+done:
+	mutex_unlock(&coil->power_lock);
+	return ret;
+}
+
+static int ad5820_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
+{
+	return ad5820_set_power(sd, 1);
+}
+
+static int ad5820_close(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
+{
+	return ad5820_set_power(sd, 0);
+}
+
+static const struct v4l2_subdev_core_ops ad5820_core_ops = {
+	.s_power = ad5820_set_power,
+};
+
+static const struct v4l2_subdev_ops ad5820_ops = {
+	.core = &ad5820_core_ops,
+};
+
+static const struct v4l2_subdev_internal_ops ad5820_internal_ops = {
+	.registered = ad5820_registered,
+	.open = ad5820_open,
+	.close = ad5820_close,
+};
+
+/*
+ * I2C driver
+ */
+#ifdef CONFIG_PM
+
+static int ad5820_suspend(struct device *dev)
+{
+	struct i2c_client *client = container_of(dev, struct i2c_client, dev);
+	struct v4l2_subdev *subdev = i2c_get_clientdata(client);
+	struct ad5820_device *coil = to_ad5820_device(subdev);
+
+	if (!coil->power_count)
+		return 0;
+
+	return ad5820_power_off(coil, 0);
+}
+
+static int ad5820_resume(struct device *dev)
+{
+	struct i2c_client *client = container_of(dev, struct i2c_client, dev);
+	struct v4l2_subdev *subdev = i2c_get_clientdata(client);
+	struct ad5820_device *coil = to_ad5820_device(subdev);
+
+	if (!coil->power_count)
+		return 0;
+
+	return ad5820_power_on(coil, 1);
+}
+
+#else
+
+#define ad5820_suspend	NULL
+#define ad5820_resume	NULL
+
+#endif /* CONFIG_PM */
+
+static int ad5820_probe(struct i2c_client *client,
+			const struct i2c_device_id *devid)
+{
+	struct ad5820_device *coil;
+	int ret = 0;
+
+	coil = kzalloc(sizeof(*coil), GFP_KERNEL);
+	if (coil == NULL)
+		return -ENOMEM;
+
+	coil->platform_data = NULL; // client->dev.platform_data;
+
+	mutex_init(&coil->power_lock);
+
+	v4l2_i2c_subdev_init(&coil->subdev, client, &ad5820_ops);
+	coil->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
+	coil->subdev.internal_ops = &ad5820_internal_ops;
+	strcpy(coil->subdev.name, "ad5820 focus");
+
+	ret = media_entity_pads_init(&coil->subdev.entity, 0, NULL);
+	if (ret < 0) {
+		kfree(coil);
+		return ret;
+	}
+
+	ret = v4l2_async_register_subdev(&coil->subdev);
+	if (ret < 0)
+		kfree(coil);
+
+	printk("Hack -- testing hw\n");
+	ad5820_registered(coil);
+
+	printk("hw test done\n");
+
+	return ret;
+}
+
+static int __exit ad5820_remove(struct i2c_client *client)
+{
+	struct v4l2_subdev *subdev = i2c_get_clientdata(client);
+	struct ad5820_device *coil = to_ad5820_device(subdev);
+
+	v4l2_device_unregister_subdev(&coil->subdev);
+	v4l2_ctrl_handler_free(&coil->ctrls);
+	media_entity_cleanup(&coil->subdev.entity);
+	if (coil->vana)
+		regulator_put(coil->vana);
+
+	kfree(coil);
+	return 0;
+}
+
+static const struct i2c_device_id ad5820_id_table[] = {
+	{ AD5820_NAME, 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, ad5820_id_table);
+
+static SIMPLE_DEV_PM_OPS(ad5820_pm, ad5820_suspend, ad5820_resume);
+
+static struct i2c_driver ad5820_i2c_driver = {
+	.driver		= {
+		.name	= AD5820_NAME,
+		.pm	= &ad5820_pm,
+	},
+	.probe		= ad5820_probe,
+	.remove		= __exit_p(ad5820_remove),
+	.id_table	= ad5820_id_table,
+};
+
+static int __init ad5820_init(void)
+{
+	int rval;
+
+	rval = i2c_add_driver(&ad5820_i2c_driver);
+	if (rval)
+		printk(KERN_INFO "%s: failed registering " AD5820_NAME "\n",
+		       __func__);
+
+	return rval;
+}
+
+static void __exit ad5820_exit(void)
+{
+	i2c_del_driver(&ad5820_i2c_driver);
+}
+
+
+module_init(ad5820_init);
+module_exit(ad5820_exit);
+
+MODULE_AUTHOR("Tuukka Toivonen <tuukka.o.toivonen@nokia.com>");
+MODULE_DESCRIPTION("AD5820 camera lens driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/media/i2c/adp1653.c b/drivers/media/i2c/adp1653.c
index 0cd494e..6dd5d6a 100644
--- a/drivers/media/i2c/adp1653.c
+++ b/drivers/media/i2c/adp1653.c
@@ -306,6 +306,8 @@ adp1653_init_device(struct adp1653_flash *flash)
 		return -EIO;
 	}
 
+	dev_info(&client->dev, "adp1653 device ok\n", __func__);
+
 	return 0;
 }
 
@@ -492,6 +494,8 @@ static int adp1653_probe(struct i2c_client *client,
 	if (flash == NULL)
 		return -ENOMEM;
 
+	dev_info(&client->dev, "adp1653 probe\n");
+	
 	if (client->dev.of_node) {
 		ret = adp1653_of_init(client, flash, client->dev.of_node);
 		if (ret)
@@ -505,11 +509,13 @@ static int adp1653_probe(struct i2c_client *client,
 		flash->platform_data = client->dev.platform_data;
 	}
 
+	dev_info(&client->dev, "adp1653 probe: subdev\n", __func__);	
 	mutex_init(&flash->power_lock);
 
 	v4l2_i2c_subdev_init(&flash->subdev, client, &adp1653_ops);
 	flash->subdev.internal_ops = &adp1653_internal_ops;
 	flash->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
+	strcpy(flash->subdev.name, "adp1653 flash");
 
 	ret = adp1653_init_controls(flash);
 	if (ret)
@@ -519,6 +525,14 @@ static int adp1653_probe(struct i2c_client *client,
 	if (ret < 0)
 		goto free_and_quit;
 
+	dev_info(&client->dev, "adp1653 probe: should be ok\n");
+
+	ret = v4l2_async_register_subdev(&flash->subdev);
+	if (ret < 0)
+		goto free_and_quit;
+
+	dev_info(&client->dev, "adp1653 probe: async register subdev ok\n");	
+
 	flash->subdev.entity.function = MEDIA_ENT_F_FLASH;
 
 	return 0;
@@ -538,6 +552,8 @@ static int adp1653_remove(struct i2c_client *client)
 	v4l2_ctrl_handler_free(&flash->ctrls);
 	media_entity_cleanup(&flash->subdev.entity);
 
+	dev_info(&client->dev, "adp1653 remove\n");			
+
 	return 0;
 }
 
diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c
index 6361fde..23d484c 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -2095,13 +2095,20 @@ static void isp_of_parse_node_csi2(struct device *dev,
 	buscfg->bus.csi2.crc = 1;
 }
 
-static int isp_of_parse_node(struct device *dev, struct device_node *node,
-			     struct isp_async_subdev *isd)
+static int isp_of_parse_node_endpoint(struct device *dev,
+				      struct device_node *node,
+				      struct isp_async_subdev *isd)
 {
-	struct isp_bus_cfg *buscfg = &isd->bus;
+	struct isp_bus_cfg *buscfg;
 	struct v4l2_of_endpoint vep;
 	int ret;
 
+	isd->bus = devm_kzalloc(dev, sizeof(*isd->bus), GFP_KERNEL);
+	if (!isd->bus)
+		return -ENOMEM;
+
+	buscfg = isd->bus;
+
 	ret = v4l2_of_parse_endpoint(node, &vep);
 	if (ret)
 		return ret;
@@ -2144,10 +2151,51 @@ static int isp_of_parse_node(struct device *dev, struct device_node *node,
 	return 0;
 }
 
+static int isp_of_parse_node(struct device *dev, struct device_node *node,
+			     struct v4l2_async_notifier *notifier,
+			     u32 group_id, bool link)
+{
+	struct isp_async_subdev *isd;
+
+	isd = devm_kzalloc(dev, sizeof(*isd), GFP_KERNEL);
+	if (!isd) {
+		of_node_put(node);
+		return -ENOMEM;
+	}
+
+	notifier->subdevs[notifier->num_subdevs] = &isd->asd;
+
+	if (link) {
+		if (isp_of_parse_node_endpoint(dev, node, isd)) {
+			of_node_put(node);
+			return -EINVAL;
+		}
+
+		isd->asd.match.of.node = of_graph_get_remote_port_parent(node);
+		of_node_put(node);
+	} else {
+		isd->asd.match.of.node = node;
+	}
+
+	if (!isd->asd.match.of.node) {
+		dev_warn(dev, "bad remote port parent\n");
+		return -EINVAL;
+	}
+
+	isd->asd.match_type = V4L2_ASYNC_MATCH_OF;
+	isd->group_id = group_id;
+	notifier->num_subdevs++;
+
+	return 0;
+}
+
 static int isp_of_parse_nodes(struct device *dev,
 			      struct v4l2_async_notifier *notifier)
 {
 	struct device_node *node = NULL;
+	int ret;
+	unsigned int flash = 0;
+	u32 group_id = 0;
 
 	notifier->subdevs = devm_kcalloc(
 		dev, ISP_MAX_SUBDEVS, sizeof(*notifier->subdevs), GFP_KERNEL);
@@ -2156,30 +2204,57 @@ static int isp_of_parse_nodes(struct device *dev,
 
 	while (notifier->num_subdevs < ISP_MAX_SUBDEVS &&
 	       (node = of_graph_get_next_endpoint(dev->of_node, node))) {
-		struct isp_async_subdev *isd;
+		ret = isp_of_parse_node(dev, node, notifier, group_id++, true);
+		if (ret)
+			return ret;
+	}
 
-		isd = devm_kzalloc(dev, sizeof(*isd), GFP_KERNEL);
-		if (!isd) {
-			of_node_put(node);
-			return -ENOMEM;
-		}
+	while (notifier->num_subdevs < ISP_MAX_SUBDEVS &&
+	       (node = of_parse_phandle(dev->of_node, "ti,camera-flashes",
+					flash++))) {
+		struct device_node *sensor_node =
+			of_parse_phandle(dev->of_node, "ti,camera-flashes",
+					 flash++);
+		unsigned int i;
+		u32 flash_group_id;
+
+		if (!sensor_node)
+			return -EINVAL;
 
-		notifier->subdevs[notifier->num_subdevs] = &isd->asd;
+		for (i = 0; i < notifier->num_subdevs; i++) {
+			struct isp_async_subdev *isd = container_of(
+				notifier->subdevs[i], struct isp_async_subdev,
+				asd);
 
-		if (isp_of_parse_node(dev, node, isd)) {
-			of_node_put(node);
-			return -EINVAL;
+			if (!isd->bus)
+				continue;
+
+			dev_dbg(dev, "match \"%s\", \"%s\"\n",sensor_node->name,
+				isd->asd.match.of.node->name);
+
+			if (sensor_node != isd->asd.match.of.node)
+				continue;
+
+			dev_dbg(dev, "found\n");
+
+			flash_group_id = isd->group_id;
+			break;
 		}
 
-		isd->asd.match.of.node = of_graph_get_remote_port_parent(node);
-		of_node_put(node);
-		if (!isd->asd.match.of.node) {
-			dev_warn(dev, "bad remote port parent\n");
-			return -EINVAL;
+		/*
+		 * No sensor was found --- complain and allocate a new
+		 * group ID.
+		 */
+		if (i == notifier->num_subdevs) {
+			dev_warn(dev, "no device node \"%s\" was found",
+				 sensor_node->name);
+			flash_group_id = group_id++;
 		}
 
-		isd->asd.match_type = V4L2_ASYNC_MATCH_OF;
-		notifier->num_subdevs++;
+		ret = isp_of_parse_node(dev, node, notifier, flash_group_id,
+					false);
+		if (ret)
+			return ret;
 	}
 
 	return notifier->num_subdevs;
@@ -2192,8 +2267,9 @@ static int isp_subdev_notifier_bound(struct v4l2_async_notifier *async,
 	struct isp_async_subdev *isd =
 		container_of(asd, struct isp_async_subdev, asd);
 
+//	subdev->entity.group_id = isd->group_id;
 	isd->sd = subdev;
-	isd->sd->host_priv = &isd->bus;
+	isd->sd->host_priv = isd->bus;
 
 	return 0;
 }
@@ -2396,12 +2472,15 @@ static int isp_probe(struct platform_device *pdev)
 	if (ret < 0)
 		goto error_register_entities;
 
-	isp->notifier.bound = isp_subdev_notifier_bound;
-	isp->notifier.complete = isp_subdev_notifier_complete;
+	if (IS_ENABLED(CONFIG_OF) && pdev->dev.of_node) {
+		isp->notifier.bound = isp_subdev_notifier_bound;
+		isp->notifier.complete = isp_subdev_notifier_complete;
 
-	ret = v4l2_async_notifier_register(&isp->v4l2_dev, &isp->notifier);
-	if (ret)
-		goto error_register_entities;
+		ret = v4l2_async_notifier_register(&isp->v4l2_dev,
+						   &isp->notifier);
+		if (ret)
+			goto error_register_entities;
+	}
 
 	isp_core_init(isp, 1);
 	omap3isp_put(isp);
diff --git a/drivers/media/platform/omap3isp/isp.h b/drivers/media/platform/omap3isp/isp.h
index 7e6f663..639b3ca 100644
--- a/drivers/media/platform/omap3isp/isp.h
+++ b/drivers/media/platform/omap3isp/isp.h
@@ -228,8 +228,9 @@ struct isp_device {
 
 struct isp_async_subdev {
 	struct v4l2_subdev *sd;
-	struct isp_bus_cfg bus;
+	struct isp_bus_cfg *bus;
 	struct v4l2_async_subdev asd;
+	u32 group_id;
 };
 
 #define v4l2_dev_to_isp_device(dev) \
diff --git a/drivers/media/platform/omap3isp/ispcsiphy.c b/drivers/media/platform/omap3isp/ispcsiphy.c
index 495447d..750ce93 100644
--- a/drivers/media/platform/omap3isp/ispcsiphy.c
+++ b/drivers/media/platform/omap3isp/ispcsiphy.c
@@ -177,7 +177,7 @@ static int omap3isp_csiphy_config(struct isp_csiphy *phy)
 		struct isp_async_subdev *isd =
 			container_of(pipe->external->asd,
 				     struct isp_async_subdev, asd);
-		buscfg = &isd->bus;
+		buscfg = isd->bus;
 	}
 
 	if (buscfg->interface == ISP_INTERFACE_CCP2B_PHY1
diff --git a/include/media/ad5820.h b/include/media/ad5820.h
new file mode 100644
index 0000000..f5a1565
--- /dev/null
+++ b/include/media/ad5820.h
@@ -0,0 +1,70 @@
+/*
+ * include/media/ad5820.h
+ *
+ * Copyright (C) 2008 Nokia Corporation
+ * Copyright (C) 2007 Texas Instruments
+ *
+ * Contact: Tuukka Toivonen <tuukka.o.toivonen@nokia.com>
+ *          Sakari Ailus <sakari.ailus@nokia.com>
+ *
+ * Based on af_d88.c by Texas Instruments.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
+ * 02110-1301 USA
+ */
+
+#ifndef AD5820_H
+#define AD5820_H
+
+#include <linux/i2c.h>
+#include <linux/mutex.h>
+#include <linux/videodev2.h>
+
+#include <media/v4l2-ctrls.h>
+#include <media/v4l2-subdev.h>
+
+struct regulator;
+
+#define AD5820_NAME		"ad5820"
+#define AD5820_I2C_ADDR		(0x18 >> 1)
+
+/* Register definitions */
+#define AD5820_POWER_DOWN		(1 << 15)
+#define AD5820_DAC_SHIFT		4
+#define AD5820_RAMP_MODE_LINEAR		(0 << 3)
+#define AD5820_RAMP_MODE_64_16		(1 << 3)
+
+struct ad5820_platform_data {
+	int (*set_xshutdown)(struct v4l2_subdev *subdev, int set);
+};
+
+#define to_ad5820_device(sd)	container_of(sd, struct ad5820_device, subdev)
+
+struct ad5820_device {
+	struct v4l2_subdev subdev;
+	struct ad5820_platform_data *platform_data;
+	struct regulator *vana;
+
+	struct v4l2_ctrl_handler ctrls;
+	u32 focus_absolute;
+	u32 focus_ramp_time;
+	u32 focus_ramp_mode;
+
+	struct mutex power_lock;
+	int power_count;
+
+	int standby : 1;
+};
+
+#endif /* AD5820_H */


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* camera application for testing (was Re: v4l subdevs without big device)
  2016-04-29  7:31   ` Hans Verkuil
  2016-04-29  7:56     ` Sakari Ailus
  2016-04-29 21:30     ` [pre-rfc] focus and flash for Nokia N900 (was Re: v4l subdevs without big device was Re: drivers/media/i2c/adp1653.c: does not show as /dev/video* or v4l-subdev*) Pavel Machek
@ 2016-04-29 22:13     ` Pavel Machek
  2016-04-29 22:20       ` Pali Rohár
  2016-05-01 14:08       ` Sakari Ailus
  2 siblings, 2 replies; 19+ messages in thread
From: Pavel Machek @ 2016-04-29 22:13 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: pali.rohar, sre, kernel list, linux-arm-kernel, linux-omap, tony,
	khilman, aaro.koskinen, ivo.g.dimitrov.75, patrikbachan, serge,
	sakari.ailus, tuukkat76, mchehab, linux-media

Hi!

What is reasonable camera application for testing?

N900 looks like a low-end digital camera. I have now have the hardware
working (can set focus to X cm using command line), but that's not
going to be useful for taking photos.

In particular, who is going to do computation neccessary for
autofocus, whitebalance and exposure/gain?

There's http://fcam.garage.maemo.org/gettingStarted.html that should
work on maemo, but a) it is not in Debian, b) it has non-trivial
dependencies and c) will be a lot of fun to get working...

(and d), will not be too useful, anyway, due to 1sec shutter lag:

Fast resolution switching (less shutter lag)
FCam is built on top of V4L2, which doesn't handle rapidly varying
resolutions. When a Shot with a different resolution to the previous
one comes down the pipeline, FCam currently flushes the entire V4L2
pipeline, shuts down and restarts the whole camera subsystem, then
starts streaming at the new resolution. This takes a long time (nearly
a second), and is the cause of the horrible shutter lag on the N900. A
brave kernel hacker may be able to reduce this time by fiddling with
the FCam ISP kernel modules and the guts of the FCam library source
(primarily Daemon.cpp).
Anyone who solves this one will have our undying gratitude. An ideal
solution would be able to insert a 5MP capture into a stream of
640x480 frames running at 30fps, without skipping more than the frame
time of the 5MP capture. That is, the viewfinder would effectively
stay live while taking a photograph.

)

Any other application I should look at? Thanks,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: camera application for testing (was Re: v4l subdevs without big device)
  2016-04-29 22:13     ` camera application for testing (was Re: v4l subdevs without big device) Pavel Machek
@ 2016-04-29 22:20       ` Pali Rohár
  2016-04-29 22:47         ` Ivaylo Dimitrov
  2016-05-01 14:08       ` Sakari Ailus
  1 sibling, 1 reply; 19+ messages in thread
From: Pali Rohár @ 2016-04-29 22:20 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Hans Verkuil, sre, kernel list, linux-arm-kernel, linux-omap,
	tony, khilman, aaro.koskinen, ivo.g.dimitrov.75, patrikbachan,
	serge, sakari.ailus, tuukkat76, mchehab, linux-media

[-- Attachment #1: Type: Text/Plain, Size: 238 bytes --]

On Saturday 30 April 2016 00:13:59 Pavel Machek wrote:
> Any other application I should look at? Thanks,

Maybe camera-ui, which is part of CSSU?

https://github.com/community-ssu/camera-ui

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: camera application for testing (was Re: v4l subdevs without big device)
  2016-04-29 22:20       ` Pali Rohár
@ 2016-04-29 22:47         ` Ivaylo Dimitrov
  0 siblings, 0 replies; 19+ messages in thread
From: Ivaylo Dimitrov @ 2016-04-29 22:47 UTC (permalink / raw)
  To: Pali Rohár, Pavel Machek
  Cc: Hans Verkuil, sre, kernel list, linux-arm-kernel, linux-omap,
	tony, khilman, aaro.koskinen, patrikbachan, serge, sakari.ailus,
	tuukkat76, mchehab, linux-media

Hi,

On 30.04.2016 01:20, Pali Rohár wrote:
> On Saturday 30 April 2016 00:13:59 Pavel Machek wrote:
>> Any other application I should look at? Thanks,
>
> Maybe camera-ui, which is part of CSSU?
>
> https://github.com/community-ssu/camera-ui
>

This is based on gdigicam, are you sure it is compatible with recent 
kernel/userspace? Also, iirc gdigicam needs omap3camd working, but we 
still don't have the needed compat layer.

Ivo

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: camera application for testing (was Re: v4l subdevs without big device)
  2016-04-29 22:13     ` camera application for testing (was Re: v4l subdevs without big device) Pavel Machek
  2016-04-29 22:20       ` Pali Rohár
@ 2016-05-01 14:08       ` Sakari Ailus
  2016-05-01 19:21         ` Pavel Machek
                           ` (2 more replies)
  1 sibling, 3 replies; 19+ messages in thread
From: Sakari Ailus @ 2016-05-01 14:08 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Hans Verkuil, pali.rohar, sre, kernel list, linux-arm-kernel,
	linux-omap, tony, khilman, aaro.koskinen, ivo.g.dimitrov.75,
	patrikbachan, serge, tuukkat76, mchehab, linux-media,
	laurent.pinchart

Hi Pavel,

On Sat, Apr 30, 2016 at 12:13:59AM +0200, Pavel Machek wrote:
> Hi!
> 
> What is reasonable camera application for testing?
> 
> N900 looks like a low-end digital camera. I have now have the hardware
> working (can set focus to X cm using command line), but that's not
> going to be useful for taking photos.

I guess you already knew about omap3camd; it's proprietary but from purely
practical point of view it'd be an option to support taking photos on the
N900. That would not be extensible any way, the best possible functionality
is limited what the daemon implements.

I'm just mentioning the option of implementing wrapper for the omap3camd so
that it can work with upsteam APIs, I don't propose that however.

> 
> In particular, who is going to do computation neccessary for
> autofocus, whitebalance and exposure/gain?

I think libv4l itself has algorithms to control at least some of these. It
relies on the image data so the CPU time consumption will be high.

AFAIR Laurent has also worked on implementing some algorithms that use the
histogram and some of the statistics. Add him to cc list.

> 
> There's http://fcam.garage.maemo.org/gettingStarted.html that should
> work on maemo, but a) it is not in Debian, b) it has non-trivial
> dependencies and c) will be a lot of fun to get working...
> 
> (and d), will not be too useful, anyway, due to 1sec shutter lag:

I believe this will be shorter nowadays. I don't remember the exact
technical solution which the text below refers to but I'm pretty sure it'll
be better with the current upstream. API-wise, there's work to be done there
(to port FCAM to upsteram APIs) but it's a possibility.

> 
> Fast resolution switching (less shutter lag)
> FCam is built on top of V4L2, which doesn't handle rapidly varying
> resolutions. When a Shot with a different resolution to the previous
> one comes down the pipeline, FCam currently flushes the entire V4L2
> pipeline, shuts down and restarts the whole camera subsystem, then
> starts streaming at the new resolution. This takes a long time (nearly
> a second), and is the cause of the horrible shutter lag on the N900. A
> brave kernel hacker may be able to reduce this time by fiddling with
> the FCam ISP kernel modules and the guts of the FCam library source
> (primarily Daemon.cpp).
> Anyone who solves this one will have our undying gratitude. An ideal
> solution would be able to insert a 5MP capture into a stream of
> 640x480 frames running at 30fps, without skipping more than the frame
> time of the 5MP capture. That is, the viewfinder would effectively
> stay live while taking a photograph.
> 
> )
> 
> Any other application I should look at? Thanks,

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: camera application for testing (was Re: v4l subdevs without big device)
  2016-05-01 14:08       ` Sakari Ailus
@ 2016-05-01 19:21         ` Pavel Machek
  2016-06-21 22:32         ` Pavel Machek
  2016-06-21 23:59         ` Laurent Pinchart
  2 siblings, 0 replies; 19+ messages in thread
From: Pavel Machek @ 2016-05-01 19:21 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: Hans Verkuil, pali.rohar, sre, kernel list, linux-arm-kernel,
	linux-omap, tony, khilman, aaro.koskinen, ivo.g.dimitrov.75,
	patrikbachan, serge, tuukkat76, mchehab, linux-media,
	laurent.pinchart

Hi!
> > 
> > What is reasonable camera application for testing?
> > 
> > N900 looks like a low-end digital camera. I have now have the hardware
> > working (can set focus to X cm using command line), but that's not
> > going to be useful for taking photos.
> 
> I guess you already knew about omap3camd; it's proprietary but from purely
> practical point of view it'd be an option to support taking photos on the
> N900. That would not be extensible any way, the best possible functionality
> is limited what the daemon implements.
> 
> I'm just mentioning the option of implementing wrapper for the omap3camd so
> that it can work with upsteam APIs, I don't propose that however.

I knew it existed, but I'd prefer not to touch proprietary code.

> > In particular, who is going to do computation neccessary for
> > autofocus, whitebalance and exposure/gain?
> 
> I think libv4l itself has algorithms to control at least some of these. It
> relies on the image data so the CPU time consumption will be high.
> 
> AFAIR Laurent has also worked on implementing some algorithms that use the
> histogram and some of the statistics. Add him to cc list.

Aha, good, let me know.

> > There's http://fcam.garage.maemo.org/gettingStarted.html that should
> > work on maemo, but a) it is not in Debian, b) it has non-trivial
> > dependencies and c) will be a lot of fun to get working...
> > 
> > (and d), will not be too useful, anyway, due to 1sec shutter lag:
> 
> I believe this will be shorter nowadays. I don't remember the exact
> technical solution which the text below refers to but I'm pretty sure it'll
> be better with the current upstream. API-wise, there's work to be done there
> (to port FCAM to upsteram APIs) but it's a possibility.

I took a look at fcam-dev in the meantime... and it does not look too
bad. It is quite n900-specific -- it needs hardware support for
histograms and sharpness maps -- but it should not be too hard to
modify.

Relying on hardware support without having fallback software
implementation feels wrong, but... it should actually be ok as I
already have the hardware...

(Is there accepted "upstream" for it?)
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: camera application for testing (was Re: v4l subdevs without big device)
  2016-05-01 14:08       ` Sakari Ailus
  2016-05-01 19:21         ` Pavel Machek
@ 2016-06-21 22:32         ` Pavel Machek
  2016-06-21 23:59         ` Laurent Pinchart
  2 siblings, 0 replies; 19+ messages in thread
From: Pavel Machek @ 2016-06-21 22:32 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: Hans Verkuil, pali.rohar, sre, kernel list, linux-arm-kernel,
	linux-omap, tony, khilman, aaro.koskinen, ivo.g.dimitrov.75,
	patrikbachan, serge, tuukkat76, mchehab, linux-media,
	laurent.pinchart

Hi!

> > What is reasonable camera application for testing?
> > 
> > N900 looks like a low-end digital camera. I have now have the hardware
> > working (can set focus to X cm using command line), but that's not
> > going to be useful for taking photos.
> 
> I guess you already knew about omap3camd; it's proprietary but from purely
> practical point of view it'd be an option to support taking photos on the
> N900. That would not be extensible any way, the best possible functionality
> is limited what the daemon implements.
> 
> I'm just mentioning the option of implementing wrapper for the omap3camd so
> that it can work with upsteam APIs, I don't propose that however.
> 
> > 
> > In particular, who is going to do computation neccessary for
> > autofocus, whitebalance and exposure/gain?
> 
> I think libv4l itself has algorithms to control at least some of these. It
> relies on the image data so the CPU time consumption will be high.
> 
> AFAIR Laurent has also worked on implementing some algorithms that use the
> histogram and some of the statistics. Add him to cc list.

Ok, so I played a bit... and have something working. I modified
fcam-dev to work with the current kernel, and removed most
functionality in the process. Then I re-did sharpness and autogain
computations in software, and it seems to work somehow... but its
rather slow. (But good news is that's because code is stupid; the
computations should be fast enough.)

In particular, it takes cca 10 seconds to take a picture. I guess
there's a room for improvement :-).

> > There's http://fcam.garage.maemo.org/gettingStarted.html that should
> > work on maemo, but a) it is not in Debian, b) it has non-trivial
> > dependencies and c) will be a lot of fun to get working...
> > 
> > (and d), will not be too useful, anyway, due to 1sec shutter lag:
> 
> I believe this will be shorter nowadays. I don't remember the exact
> technical solution which the text below refers to but I'm pretty sure it'll
> be better with the current upstream. API-wise, there's work to be done there
> (to port FCAM to upsteram APIs) but it's a possibility.

So far I have really horrible hacks.

Best regards,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: camera application for testing (was Re: v4l subdevs without big device)
  2016-05-01 14:08       ` Sakari Ailus
  2016-05-01 19:21         ` Pavel Machek
  2016-06-21 22:32         ` Pavel Machek
@ 2016-06-21 23:59         ` Laurent Pinchart
  2016-06-22 11:52           ` Pavel Machek
  2 siblings, 1 reply; 19+ messages in thread
From: Laurent Pinchart @ 2016-06-21 23:59 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: Pavel Machek, Hans Verkuil, pali.rohar, sre, kernel list,
	linux-arm-kernel, linux-omap, tony, khilman, aaro.koskinen,
	ivo.g.dimitrov.75, patrikbachan, serge, tuukkat76, mchehab,
	linux-media

Hi Sakari,

On Sunday 01 May 2016 17:08:31 Sakari Ailus wrote:
> On Sat, Apr 30, 2016 at 12:13:59AM +0200, Pavel Machek wrote:
> > Hi!
> > 
> > What is reasonable camera application for testing?
> > 
> > N900 looks like a low-end digital camera. I have now have the hardware
> > working (can set focus to X cm using command line), but that's not
> > going to be useful for taking photos.
> 
> I guess you already knew about omap3camd; it's proprietary but from purely
> practical point of view it'd be an option to support taking photos on the
> N900. That would not be extensible any way, the best possible functionality
> is limited what the daemon implements.
> 
> I'm just mentioning the option of implementing wrapper for the omap3camd so
> that it can work with upsteam APIs, I don't propose that however.
> 
> > In particular, who is going to do computation neccessary for
> > autofocus, whitebalance and exposure/gain?
> 
> I think libv4l itself has algorithms to control at least some of these. It
> relies on the image data so the CPU time consumption will be high.
> 
> AFAIR Laurent has also worked on implementing some algorithms that use the
> histogram and some of the statistics. Add him to cc list.

http://git.ideasonboard.org/omap3-isp-live.git

That's outdated and might not run or compile anymore. The code is more of a 
proof of concept implementation, but it could be used as a starting point. 
With an infinite amount of free time I'd love to work on an open-source 
project for computational cameras, integrating it with libv4l.

> > There's http://fcam.garage.maemo.org/gettingStarted.html that should
> > work on maemo, but a) it is not in Debian, b) it has non-trivial
> > dependencies and c) will be a lot of fun to get working...
> > (and d), will not be too useful, anyway, due to 1sec shutter lag:
>
> I believe this will be shorter nowadays. I don't remember the exact
> technical solution which the text below refers to but I'm pretty sure it'll
> be better with the current upstream. API-wise, there's work to be done there
> (to port FCAM to upsteram APIs) but it's a possibility.
> 
> > Fast resolution switching (less shutter lag)
> > FCam is built on top of V4L2, which doesn't handle rapidly varying
> > resolutions. When a Shot with a different resolution to the previous
> > one comes down the pipeline, FCam currently flushes the entire V4L2
> > pipeline, shuts down and restarts the whole camera subsystem, then
> > starts streaming at the new resolution. This takes a long time (nearly
> > a second), and is the cause of the horrible shutter lag on the N900. A
> > brave kernel hacker may be able to reduce this time by fiddling with
> > the FCam ISP kernel modules and the guts of the FCam library source
> > (primarily Daemon.cpp).
> > Anyone who solves this one will have our undying gratitude. An ideal
> > solution would be able to insert a 5MP capture into a stream of
> > 640x480 frames running at 30fps, without skipping more than the frame
> > time of the 5MP capture. That is, the viewfinder would effectively
> > stay live while taking a photograph.
> > 
> > )
> > 
> > Any other application I should look at? Thanks,

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: camera application for testing (was Re: v4l subdevs without big device)
  2016-06-21 23:59         ` Laurent Pinchart
@ 2016-06-22 11:52           ` Pavel Machek
  0 siblings, 0 replies; 19+ messages in thread
From: Pavel Machek @ 2016-06-22 11:52 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Sakari Ailus, Hans Verkuil, pali.rohar, sre, kernel list,
	linux-arm-kernel, linux-omap, tony, khilman, aaro.koskinen,
	ivo.g.dimitrov.75, patrikbachan, serge, tuukkat76, mchehab,
	linux-media

Hi!

> > I think libv4l itself has algorithms to control at least some of these. It
> > relies on the image data so the CPU time consumption will be high.
> > 
> > AFAIR Laurent has also worked on implementing some algorithms that use the
> > histogram and some of the statistics. Add him to cc list.
> 
> http://git.ideasonboard.org/omap3-isp-live.git
> 
> That's outdated and might not run or compile anymore. The code is
> more of a

Lets see, it compiles with this hack:

index 6f3ffbe..935f41d 100644
--- a/isp/v4l2.c
+++ b/isp/v4l2.c
@@ -292,7 +292,7 @@ struct v4l2_device *v4l2_open(const char *devname)
         * driver (>= v3.19) will set both CAPTURE and OUTPUT in the
         * capabilities field.
         */
-       capabilities = cap.device_caps ? : cap.capabilities;
+       capabilities = /* cap.device_caps ? : */ cap.capabilities;
 
        if (capabilities & V4L2_CAP_VIDEO_CAPTURE)
                dev->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;


I can try to run it, but I guess I'll need kernel with camera support.

pavel@n900:/my/omap3-isp-live$ LD_LIBRARY_PATH=isp ./snapshot
media_open: Can't open media device /dev/media0
error: unable to open media device /dev/media0
Segmentation fault (core dumped)

I tried again on kernel with camera:

pavel@n900:/my/omap3-isp-live$ LD_LIBRARY_PATH=isp ./snapshot
error: unable to locate sensor.
Segmentation fault (core dumped)
pavel@n900:/my/omap3-isp-live$

Here's the fix for coredump:

diff --git a/isp/subdev.c b/isp/subdev.c
index 9b36234..c74514e 100644
--- a/isp/subdev.c
+++ b/isp/subdev.c
@@ -75,6 +75,8 @@ int v4l2_subdev_open(struct media_entity *entity)
 
 void v4l2_subdev_close(struct media_entity *entity)
 {
+  if (!entity)
+    return;
        if (entity->fd == -1)
                return;

Let me investigate some more.

> proof of concept implementation, but it could be used as a starting point. 
> With an infinite amount of free time I'd love to work on an open-source 
> project for computational cameras, integrating it with libv4l.

For the record, I pushed my code to

https://gitlab.com/pavelm/fcam-dev

Best regards,

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply related	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2016-06-22 12:00 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-28  8:45 drivers/media/i2c/adp1653.c: does not show as /dev/video* or v4l-subdev* Pavel Machek
2016-04-29  7:15 ` v4l subdevs without big device was " Pavel Machek
2016-04-29  7:31   ` Hans Verkuil
2016-04-29  7:56     ` Sakari Ailus
2016-04-29  9:50       ` Pavel Machek
2016-04-29 10:59         ` Sakari Ailus
2016-04-29 11:05           ` Pali Rohár
2016-04-29 11:23             ` Sakari Ailus
2016-04-29 14:06           ` Pavel Machek
2016-04-29 10:15       ` Pavel Machek
2016-04-29 21:30     ` [pre-rfc] focus and flash for Nokia N900 (was Re: v4l subdevs without big device was Re: drivers/media/i2c/adp1653.c: does not show as /dev/video* or v4l-subdev*) Pavel Machek
2016-04-29 22:13     ` camera application for testing (was Re: v4l subdevs without big device) Pavel Machek
2016-04-29 22:20       ` Pali Rohár
2016-04-29 22:47         ` Ivaylo Dimitrov
2016-05-01 14:08       ` Sakari Ailus
2016-05-01 19:21         ` Pavel Machek
2016-06-21 22:32         ` Pavel Machek
2016-06-21 23:59         ` Laurent Pinchart
2016-06-22 11:52           ` Pavel Machek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).