Hi, Guennadi

On 5/19/2014 5:51 AM, Guennadi Liakhovetski wrote:

On Tue, 25 Mar 2014, Josh Wu wrote:

quoted

This patch add the DT support for Atmel ISI driver.
It use the same v4l2 DT interface that defined in video-interfaces.txt.

Signed-off-by: Josh Wu <redacted>
Cc: devicetree@vger.kernel.org
---
v1 --> v2:
  refine the binding document.
  add port node description.
  removed the optional property.

  .../devicetree/bindings/media/atmel-isi.txt        |   50 ++++++++++++++++++++
  drivers/media/platform/soc_camera/atmel-isi.c      |   31 +++++++++++-
  2 files changed, 79 insertions(+), 2 deletions(-)
  create mode 100644 Documentation/devicetree/bindings/media/atmel-isi.txt

[snip]

quoted

diff --git a/drivers/media/platform/soc_camera/atmel-isi.c b/drivers/media/platform/soc_camera/atmel-isi.c
index f4add0a..d6a1f7b 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c

[snip]

quoted

@@ -885,6 +887,20 @@ static int atmel_isi_remove(struct platform_device *pdev)
  	return 0;
  }
  
+static int atmel_isi_probe_dt(struct atmel_isi *isi,
+			struct platform_device *pdev)
+{
+	struct device_node *node = pdev->dev.of_node;
+
+	/* Default settings for ISI */
+	isi->pdata.full_mode = 1;
+	isi->pdata.mck_hz = ISI_DEFAULT_MCLK_FREQ;
+	isi->pdata.frate = ISI_CFG1_FRATE_CAPTURE_ALL;

The above flags eventually should probably partially be added as new
driver-specific DT properties, partially derived from DT clock bindings.
But I'm ok to have them fixed like this in the initial version.

quoted

+	isi->pdata.data_width_flags = ISI_DATAWIDTH_8 | ISI_DATAWIDTH_10;

Whereas these flags, I think, should already now be derived from the
bus-width standard property?

yes. I agree.

v4l2_of_parse_parallel_bus() will extract
them for you and I just asked Ben to add a call to
v4l2_of_parse_endpoint() to his patch.

Is it better to call v4l2_of_parse_endpoint() in the atmel-isi driver? I 
think the dt parsing stuff should be done by host driver and sensor 
driver itself. No need to call v4l2_of_parse_endpoint() in soc-camera.c.

Consequently you'll have to
rearrange bus-width interpretation in isi_camera_try_bus_param() a bit and
use OF parsing results there directly if available? Or maybe you find a
better way. It would certainly be better to extend your probing code and
just use OF results to initialise isi->width_flags, but that might be
impossible, because OF parsing would be performed inside
soc_camera_host_register() and your isi_camera_try_bus_param() can also be
called immediately from it if all required I2C devices are already
available?

I am little bit confuse here. I don't see any issue in above case. Since 
atmel_isi_probe_dt() will always be called earlier then 
soc_camera_host_register().
That means when soc_camera_host_register() called 
isi_camera_try_bus_param(), the isi->width_flags are already initialized 
in a valid value by atmel_isi_probe_dt().
Am I missing anything here?

If your I2C subdevice drivers defer probing until the host has
probed, then you could initialise .width_flags after
soc_camera_host_register(), but you cannot rely on that.

I tested these two cases without any issue:
1. In dtb, the i2c sensor dt node probe earlier than atmel-isi dt node.
     i2c sensor will do a defer probe here as mclk is not found until 
atmel-isi driver probed and call soc_camera_host_register().
2. In dtb, the atmel-isi dt node is probed earlier than i2c sensor.

Best Regards,
Josh Wu

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html