Re: [PATCH 05/14] media: add a V4L2 OF parser
Hi Bearing in mind the local time, I hope my brevity will be excused:-) Just wanted to give a sign, that I just finished a first successful run of a fully asynchronous and uniform multi-component non-DT video (soc-camera) pipeline probing, verified by a sample capture. What this means: The pipeline consists of 3 components: a sensor, a CSI-2 interface, and a bridge. The platform code registers 3 devices: 2 platform devices for the bridge and CSI-2 and an I2C device for the sensor. The bridge driver gets a list of device descriptors, which it passes to the (soc-camera) generic code. That is used to initialise internal data and install bus (platform- and i2c-) notifiers. After all components have been collected the normal probing continues. Next week I'll (hopefully) be cleaning all this up and converting from soc-camera to V4L2 core... After that I'll start posting, beginning with v2 of this DT series, taking into account possibly many comments:-) I think, it might make sense to first post and have a look at the purely asynchronous code, first without DT additions, that we planned to implement for bus notifiers and whatever is related to them. If that looks reasonable, we can move on with adding DT. (One of) the ugly part(s) is going to be, that with this we'll be supporting 3 (!) pipeline initialisation modes: current (legacy) mode, generating I2C devices on the fly, non-DT asynchronous with all devices probing independently, and DT... Let's see if all this actually flies. 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 linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/14] media: add a V4L2 OF parser
On 10/10/2012 04:51 PM, Laurent Pinchart wrote: On Wednesday 10 October 2012 10:50:20 Stephen Warren wrote: On 10/10/2012 07:18 AM, Laurent Pinchart wrote: On Monday 08 October 2012 17:15:53 Guennadi Liakhovetski wrote: ... But how do you get the subdev pointer? With the notifier I get it from i2c_get_clientdata(client) and what do you do without it? How do you get to the client? And can't it get that from DT as well? No, I don't think there is a way to get a device pointer from a DT node. I don't believe there's a generic API for this (although perhaps there could be), but it can be implemented quite easily. For example, on Tegra, the SMMU needs to flip a bit in the AHB register space in order to enable itself. The SMMU DT node contains a phandle that points at the AHB DT node. The SMMU driver parses the phandle and passes the DT node pointer to the AHB driver. The AHB driver looks up the struct device that was instantiated for that node. See drivers/amba/tegra-ahb.c:tegra_ahb_enable_smmu(). There are a few other cases of similar code in the kernel, although I can't remember the others! That's a very naive approach, but what about storing the struct device in struct device_node when the device is instantiated ? It's so simple that there's probably a good reason why that hasn't been implemented though. It sounds like that would work. The advantage of calling a function in the driver for the node, rather than just grabbing something from the node directly in code unrelated to the driver for the node, is that it any knowledge of what kind of device/driver is handling that node is embedded into a function in the driver for the node, not the driver using the node, which makes everything a bit more type-safe. Now, perhaps the implementation of that function could just pull a field out of struct of_node rather than calling driver_find_device(), but it'd then have to validate that the struct device that was found was truly managed by the expected driver, for safety. I assume that's pretty simple, but haven't checked. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/14] media: add a V4L2 OF parser
Hi Guennadi, On Mon, Oct 08, 2012 at 02:23:25PM +0200, Guennadi Liakhovetski wrote: ... If we do it from the bridge driver, we could install an I2C bus-notifier, _before_ the subdevice driver is probed, i.e. upon the BUS_NOTIFY_BIND_DRIVER event we could turn on the clock. If subdevice probing was successful, we can then wait for the BUS_NOTIFY_BOUND_DRIVER event to switch the clock back off. BUT - if the subdevice fails probing? How do we find out about that and turn the clock back off? There is no notification event for that... Possible solutions: 1. timer - ugly and unreliable. 2. add a probing failed notifier event to the device core - would this be accepted? 3. let the subdevice turn the master clock on and off for the duration of probing. My vote goes for (3). Ideally this should be done using the generic clock framework. But can we really expect all drivers and platforms to switch to it quickly enough? If not, we need a V4L2-specific callback from subdevice drivers to bridge drivers to turn the clock on and off. That's what I've done temporarily in this patch series for soc-camera. I'd say the clock has to be controlled by the sub-device driver. Sensors also have a power-up (and power-down) sequences that should be followed. Usually they also involve switching the external clock on (and off) at a given point of time. Also the OMAP 3 provides that clock through ISP so it, too, requires the generic clock framework to function with DT. Suppose we decide to do the same for V4L2 centrally - add call-backs. Then I'd prefer to use the generic clock framework, albeit I admit it may take some time till all the relevant platforms support it. Nevertheless, if there's going to be a temporary solution it should be removed once the clock framework support is there. Kind regards, -- Sakari Ailus e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/14] media: add a V4L2 OF parser
Hi Guennadi, On Monday 08 October 2012 14:23:25 Guennadi Liakhovetski wrote: On Fri, 5 Oct 2012, Hans Verkuil wrote: [snip] I think the soc_camera patches should be left out for now. I suspect that by adding core support for async i2c handling first, Ok, let's think, what this meacs - async I2C in media / V4L2 core. The main reason for our probing order problem is the master clock, typically supplied from the camera bridge to I2C subdevices, which we only want to start when necessary, i.e. when accessing the subdevice. And the subdevice driver needs that clock running during its .probe() to be able to access and verify or configure the hardware. Our current solution is to not register I2C subdevices from the platform data, as is usual for all I2C devices, but from the bridge driver and only after it has switched on the master clock. After the subdevice driver has completed its probing we switch the clock back off until the subdevice has to be activated, e.g. for video capture. Also important - when we want to unregister the bridge driver we just also unregister the I2C device. Now, to reverse the whole thing and to allow I2C devices be registered as usual - via platform data or OF, first of all we have to teach I2C subdevice drivers to recognise the too early situation and request deferred probing in such a case. Then it will be reprobed after each new successful probe or unregister on the system. After the bridge driver has successfully probed the subdevice driver will be re-probed and at that time it should succeed. Now, there is a problem here too: who should switch on and off the master clock? If we do it from the bridge driver, we could install an I2C bus-notifier, _before_ the subdevice driver is probed, i.e. upon the BUS_NOTIFY_BIND_DRIVER event we could turn on the clock. If subdevice probing was successful, we can then wait for the BUS_NOTIFY_BOUND_DRIVER event to switch the clock back off. BUT - if the subdevice fails probing? How do we find out about that and turn the clock back off? There is no notification event for that... Possible solutions: 1. timer - ugly and unreliable. 2. add a probing failed notifier event to the device core - would this be accepted? 3. let the subdevice turn the master clock on and off for the duration of probing. My vote goes for (3). Ideally this should be done using the generic clock framework. But can we really expect all drivers and platforms to switch to it quickly enough? If not, we need a V4L2-specific callback from subdevice drivers to bridge drivers to turn the clock on and off. That's what I've done temporarily in this patch series for soc-camera. I vote for (3) as well, using the generic clock framework. You're right that we need an interim solution, as not all platforms will switch quickly enough. I'm fine with that interim solution being a bit hackish, as long as we push new drivers towards the right solution. Suppose we decide to do the same for V4L2 centrally - add call-backs. Then we can think what else we need to add to V4L2 to support asynchronous subdevice driver probing. 1. We'll have to create these V4L2 clock start and stop functions, that, supplied (in case of I2C) with client address and adapter number will find the correct v4l2_device instance and call its callbacks. 2. The I2C notifier. I'm not sure, whether this one should be common. Of common tasks we have to refcount the I2C adapter and register the subdevice. Then we'd have to call the bridge driver's callback. Is it worth it doing this centrally or rather allow individual drivers to do that themselves? What about going through board code for platforms that don't support the generic clock framework yet ? That's what the OMAP3 ISP driver does. DT support would then require the generic clock framework. Also, ideally OF-compatible (I2C) drivers should run with no platform data, but soc-camera is using I2C device platform data intensively. To avoid modifying the soc-camera core and all drivers, I also trigger on the BUS_NOTIFY_BIND_DRIVER event and assign a reference to the dynamically created platform data to the I2C device. Would we also want to do this for all V4L2 bridge drivers? We could call this a prepare callback or something similar... If that's going to be an interim solution only I'm fine with keeping it in soc-camera. 3. Bridge driver unregistering. Here we have to put the subdevice driver back into the deferred-probe state... Ugliness alert: I'm doing this by unregistering and re-registering the I2C device... For that I also have to create a copy of devices I2C board-info data. Lovely, ain't it? This I'd be happy to move to the V4L2 core;-) That's the ugly part. As soon as the I2C device starts using resources provided by the bridge, those resources can't disappear behind the scene. Reference counting must ensure that the bridge driver doesn't get removed. And that's
Re: [PATCH 05/14] media: add a V4L2 OF parser
Hi Hans, On Monday 08 October 2012 16:53:55 Hans Verkuil wrote: On Mon October 8 2012 16:30:53 Guennadi Liakhovetski wrote: On Mon, 8 Oct 2012, Hans Verkuil wrote: On Mon October 8 2012 14:23:25 Guennadi Liakhovetski wrote: On Fri, 5 Oct 2012, Hans Verkuil wrote: [snip] I think the soc_camera patches should be left out for now. I suspect that by adding core support for async i2c handling first, Ok, let's think, what this meacs - async I2C in media / V4L2 core. The main reason for our probing order problem is the master clock, typically supplied from the camera bridge to I2C subdevices, which we only want to start when necessary, i.e. when accessing the subdevice. And the subdevice driver needs that clock running during its .probe() to be able to access and verify or configure the hardware. Our current solution is to not register I2C subdevices from the platform data, as is usual for all I2C devices, but from the bridge driver and only after it has switched on the master clock. After the subdevice driver has completed its probing we switch the clock back off until the subdevice has to be activated, e.g. for video capture. Also important - when we want to unregister the bridge driver we just also unregister the I2C device. Now, to reverse the whole thing and to allow I2C devices be registered as usual - via platform data or OF, first of all we have to teach I2C subdevice drivers to recognise the too early situation and request deferred probing in such a case. Then it will be reprobed after each new successful probe or unregister on the system. After the bridge driver has successfully probed the subdevice driver will be re-probed and at that time it should succeed. Now, there is a problem here too: who should switch on and off the master clock? If we do it from the bridge driver, we could install an I2C bus- notifier, _before_ the subdevice driver is probed, i.e. upon the BUS_NOTIFY_BIND_DRIVER event we could turn on the clock. If subdevice probing was successful, we can then wait for the BUS_NOTIFY_BOUND_DRIVER event to switch the clock back off. BUT - if the subdevice fails probing? How do we find out about that and turn the clock back off? There is no notification event for that... Possible solutions: 1. timer - ugly and unreliable. 2. add a probing failed notifier event to the device core - would this be accepted? 3. let the subdevice turn the master clock on and off for the duration of probing. My vote goes for (3). Ideally this should be done using the generic clock framework. But can we really expect all drivers and platforms to switch to it quickly enough? If not, we need a V4L2-specific callback from subdevice drivers to bridge drivers to turn the clock on and off. That's what I've done temporarily in this patch series for soc- camera. Suppose we decide to do the same for V4L2 centrally - add call-backs. Then we can think what else we need to add to V4L2 to support asynchronous subdevice driver probing. I wonder, don't we have the necessary code already? V4L2 subdev drivers can have internal_ops with register/unregister ops. These are called by v4l2_device_register_subdev. This happens during the bridge driver's probe. Suppose the subdev's probe does not actually access the i2c device, but instead defers that to the register callback? The bridge driver will turn on the clock before calling v4l2_device_register_subdev to ensure that the register callback can access the i2c registers. The register callback will do any initialization and can return an error. In case of an error the i2c client is automatically unregistered as well. Yes, if v4l2_i2c_new_subdev_board() is used. This has been discussed several times before and always what I didn't like in this is, that I2C device probe() in this case succeeds without even trying to access the hardware. And think about DT. In this case we don't instantiate the I2C device, OF code does it for us. What do you do then? If you let probe() succeed, then you will have to somehow remember the subdevice to later match it against bridges... Yes, but you need that information anyway. The bridge still needs to call v4l2_device_register_subdev so it needs to know which subdevs are loaded. And can't it get that from DT as well? That information will definitely come from the DT, but the bridge won't instantiate the I2C devices. They will be instantiated asynchronously by the I2C bus master driver when parsing the DT. The bridge will then need to be notified or I2C devices registration and finish its initialization when all needed I2C (or SPI, ...) devices will be available. That should in my opinion be handled by the V4L2 core : the bridge would pass a list of
Re: [PATCH 05/14] media: add a V4L2 OF parser
Hi Guennadi, On Monday 08 October 2012 17:15:53 Guennadi Liakhovetski wrote: On Mon, 8 Oct 2012, Hans Verkuil wrote: On Mon October 8 2012 16:30:53 Guennadi Liakhovetski wrote: On Mon, 8 Oct 2012, Hans Verkuil wrote: On Mon October 8 2012 14:23:25 Guennadi Liakhovetski wrote: On Fri, 5 Oct 2012, Hans Verkuil wrote: [snip] I think the soc_camera patches should be left out for now. I suspect that by adding core support for async i2c handling first, Ok, let's think, what this meacs - async I2C in media / V4L2 core. The main reason for our probing order problem is the master clock, typically supplied from the camera bridge to I2C subdevices, which we only want to start when necessary, i.e. when accessing the subdevice. And the subdevice driver needs that clock running during its .probe() to be able to access and verify or configure the hardware. Our current solution is to not register I2C subdevices from the platform data, as is usual for all I2C devices, but from the bridge driver and only after it has switched on the master clock. After the subdevice driver has completed its probing we switch the clock back off until the subdevice has to be activated, e.g. for video capture. Also important - when we want to unregister the bridge driver we just also unregister the I2C device. Now, to reverse the whole thing and to allow I2C devices be registered as usual - via platform data or OF, first of all we have to teach I2C subdevice drivers to recognise the too early situation and request deferred probing in such a case. Then it will be reprobed after each new successful probe or unregister on the system. After the bridge driver has successfully probed the subdevice driver will be re-probed and at that time it should succeed. Now, there is a problem here too: who should switch on and off the master clock? If we do it from the bridge driver, we could install an I2C bus-notifier, _before_ the subdevice driver is probed, i.e. upon the BUS_NOTIFY_BIND_DRIVER event we could turn on the clock. If subdevice probing was successful, we can then wait for the BUS_NOTIFY_BOUND_DRIVER event to switch the clock back off. BUT - if the subdevice fails probing? How do we find out about that and turn the clock back off? There is no notification event for that... Possible solutions: 1. timer - ugly and unreliable. 2. add a probing failed notifier event to the device core - would this be accepted? 3. let the subdevice turn the master clock on and off for the duration of probing. My vote goes for (3). Ideally this should be done using the generic clock framework. But can we really expect all drivers and platforms to switch to it quickly enough? If not, we need a V4L2-specific callback from subdevice drivers to bridge drivers to turn the clock on and off. That's what I've done temporarily in this patch series for soc-camera. Suppose we decide to do the same for V4L2 centrally - add call-backs. Then we can think what else we need to add to V4L2 to support asynchronous subdevice driver probing. I wonder, don't we have the necessary code already? V4L2 subdev drivers can have internal_ops with register/unregister ops. These are called by v4l2_device_register_subdev. This happens during the bridge driver's probe. Suppose the subdev's probe does not actually access the i2c device, but instead defers that to the register callback? The bridge driver will turn on the clock before calling v4l2_device_register_subdev to ensure that the register callback can access the i2c registers. The register callback will do any initialization and can return an error. In case of an error the i2c client is automatically unregistered as well. Yes, if v4l2_i2c_new_subdev_board() is used. This has been discussed several times before and always what I didn't like in this is, that I2C device probe() in this case succeeds without even trying to access the hardware. And think about DT. In this case we don't instantiate the I2C device, OF code does it for us. What do you do then? If you let probe() succeed, then you will have to somehow remember the subdevice to later match it against bridges... Yes, but you need that information anyway. The bridge still needs to call v4l2_device_register_subdev so it needs to know which subdevs are loaded. But how do you get the subdev pointer? With the notifier I get it from i2c_get_clientdata(client) and what do you do without it? How do you get to the client? And can't it get that from DT as well? No, I don't think there is a way to get a device pointer from a DT node. But we'll need a way. The bridge driver will get sensor
Re: [PATCH 05/14] media: add a V4L2 OF parser
Hi Hans, On Monday 08 October 2012 17:41:43 Hans Verkuil wrote: On Mon October 8 2012 17:15:53 Guennadi Liakhovetski wrote: On Mon, 8 Oct 2012, Hans Verkuil wrote: On Mon October 8 2012 16:30:53 Guennadi Liakhovetski wrote: On Mon, 8 Oct 2012, Hans Verkuil wrote: [snip] I wonder, don't we have the necessary code already? V4L2 subdev drivers can have internal_ops with register/unregister ops. These are called by v4l2_device_register_subdev. This happens during the bridge driver's probe. Suppose the subdev's probe does not actually access the i2c device, but instead defers that to the register callback? The bridge driver will turn on the clock before calling v4l2_device_register_subdev to ensure that the register callback can access the i2c registers. The register callback will do any initialization and can return an error. In case of an error the i2c client is automatically unregistered as well. Yes, if v4l2_i2c_new_subdev_board() is used. This has been discussed several times before and always what I didn't like in this is, that I2C device probe() in this case succeeds without even trying to access the hardware. And think about DT. In this case we don't instantiate the I2C device, OF code does it for us. What do you do then? If you let probe() succeed, then you will have to somehow remember the subdevice to later match it against bridges... Yes, but you need that information anyway. The bridge still needs to call v4l2_device_register_subdev so it needs to know which subdevs are loaded. But how do you get the subdev pointer? With the notifier I get it from i2c_get_clientdata(client) and what do you do without it? How do you get to the client? And can't it get that from DT as well? No, I don't think there is a way to get a device pointer from a DT node. Not a device pointer, but the i2c bus and i2c address. With that information you can get the i2c_client, and with that you can get the subdev pointer. That could work as well, but it might be easier to keep a mapping from the DT node to struct device or struct v4l2_subdev instead. I have no strong opinion on this at the moment. If there is no way to get that information from the proposed V4L2 DT, then it needs to be modified since a bridge driver really needs to know which subdevs it has to register with the v4l2_device struct. That information is also board specific so it should be part of the DT. In my view you cannot do a proper initialization unless you have both the bridge driver and all subdev drivers loaded and instantiated. They need one another. So I am perfectly fine with letting the probe function do next to nothing and postponing that until register() is called. I2C and actual probing to check if it's the right device is a bad idea in general since you have no idea what a hardware access to an unknown i2c device will do. There are still some corner cases where that is needed, but I do not think that that is an issue here. It would simplify things a lot IMHO. Also note that the register() op will work with any device, not just i2c. That may be a useful property as well. And what if the subdevice device is not yet instantiated by OF by the time your bridge driver probes? That is where you still need to have a bus notifier mechanism. You have to be able to wait until all dependent drivers are loaded/instantiated, or alternatively you have to be able to load them explicitly. But this should be relatively easy to implement in a generic manner. I still think this sucks (excuse my language), but I see no way around it as long as there is no explicit probe order one can rely on. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/14] media: add a V4L2 OF parser
Hi Hans, On Tuesday 09 October 2012 13:00:24 Hans Verkuil wrote: On Tue 9 October 2012 12:34:48 Sylwester Nawrocki wrote: On 10/08/2012 11:40 AM, Guennadi Liakhovetski wrote: On Fri, 5 Oct 2012, Sylwester Nawrocki wrote: I would really like to see more than one user until we add any core code. No that it couldn't be changed afterwards, but it would be nice to ensure the concepts are right and proven in real life. Unfortunately I don't have any more systems on which I could easily enough try this. I've got a beagleboard with a camera, but I don't think I'm a particularly good candidate for implementing DT support for OMAP3 camera drivers;-) Apart from that I've only got soc-camera based systems, of which none are _really_ DT-ready... At best I could try an i.MX31 based board, but that doesn't have a very well developed .dts and that would be soc-camera too anyway. I certainly wouldn't expect you would do all the job. I mean it would be good to possibly have some other developers adding device tree support based on that new bindings and new infrastructure related to them. As I mentioned in another e-mail, I plan to work on DT support for the OMAP3 ISP, but I first need generic clock framework support for OMAP3. There have been recently some progress in device tree support for Exynos SoCs, including common clock framework support and we hope to add FDT support to the Samsung SoC camera devices during this kernel cycle, based on the newly designed media bindings. This is going to be a second attempt, after our initial RFC from May [1]. It would still be SoC specific implementation, but not soc-camera based. I wasn't a big fan of this asynchronous sub-devices probing, but it now seems to be a most complete solution to me. I think it just need to be done right at the v4l2-core so individual drivers don't get complicated too much. After investigating this some more I think I agree with that. There are some things where we should probably ask for advice from the i2c subsystem devs, I'm thinking of putting the driver back into the deferred-probe state in particular. We might actually not need that, it might be easier to handle the circular dependency problem from the other end. We could add a way (ioctl, sysfs, ...) to force a V4L2 bridge driver to release its subdevs. Once done, the subdev driver could be unloaded and/or the subdev device unregistered, which would release the resources used by the subdev, such as clocks. The bridge driver could then be unregistered. Creating v4l2-core support for this is crucial as it is quite complex and without core support this is going to be a nightmare for drivers. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/14] media: add a V4L2 OF parser
Em Wed, 10 Oct 2012 14:54 +0200 Laurent Pinchart laurent.pinch...@ideasonboard.com escreveu: Also, ideally OF-compatible (I2C) drivers should run with no platform data, but soc-camera is using I2C device platform data intensively. To avoid modifying the soc-camera core and all drivers, I also trigger on the BUS_NOTIFY_BIND_DRIVER event and assign a reference to the dynamically created platform data to the I2C device. Would we also want to do this for all V4L2 bridge drivers? We could call this a prepare callback or something similar... If that's going to be an interim solution only I'm fine with keeping it in soc-camera. I'm far from being an OF expert, but why do we need to export I2C devices to DT/OF? On my understanding, it is the bridge code that should be responsible for detecting, binding and initializing the proper I2C devices. On several cases, it is impossible or it would cause lots of ugly hacks if we ever try to move away from platform data stuff, as only the bridge driver knows what initialization is needed for an specific I2C driver. To make things more complex, it is expected that most I2C drivers to be arch independent, and they should be allowed to be used by a personal computer or by an embedded device. Let me give 2 such examples: - ir-i2c-kbd driver supports lots of IR devices. Platform_data is needed to specify what hardware will actually be used, and what should be the default Remote Controller keymap; - Sensor drivers like ov2940 is needed by soc_camera and by other webcam drivers like em28xx. The setup for em28xx should be completely different than the one for soc_camera: the initial registers init sequence is different for both. As several registers there aren't properly documented, there's no easy way to parametrize the configuration. So, for me, we should not expose the I2C devices directly on OF; it should, instead, see just the bridge, and let the bridge to map the needed I2C devices using the needed platform_data. -- Regards, Mauro -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/14] media: add a V4L2 OF parser
Hi Mauro, On Wednesday 10 October 2012 10:45:22 Mauro Carvalho Chehab wrote: Em Wed, 10 Oct 2012 14:54 +0200 Laurent Pinchart escreveu: Also, ideally OF-compatible (I2C) drivers should run with no platform data, but soc-camera is using I2C device platform data intensively. To avoid modifying the soc-camera core and all drivers, I also trigger on the BUS_NOTIFY_BIND_DRIVER event and assign a reference to the dynamically created platform data to the I2C device. Would we also want to do this for all V4L2 bridge drivers? We could call this a prepare callback or something similar... If that's going to be an interim solution only I'm fine with keeping it in soc-camera. I'm far from being an OF expert, but why do we need to export I2C devices to DT/OF? On my understanding, it is the bridge code that should be responsible for detecting, binding and initializing the proper I2C devices. On several cases, it is impossible or it would cause lots of ugly hacks if we ever try to move away from platform data stuff, as only the bridge driver knows what initialization is needed for an specific I2C driver. In a nutshell, a DT-based platform doesn't have any board code (except in rare cases, but let's not get into that), and thus doesn't pass any platform data structure to drivers. However, drivers receive a DT node, which contains a hierarchical description of the hardware, and parse those to extract information necessary to configure the device. One very important point to keep in mind here is that the DT is not Linux- specific. DT bindings are designed to be portable, and thus must only contain hardware descriptions, without any OS-specific information or policy information. For instance information such as the maximum video buffers size is not allowed in the DT. The DT is used both to provide platform data to drivers and to instantiate devices. I2C device DT nodes are stored as children of the I2C bus master DT node, and are automatically instantiated by the I2C bus master. This is a significant change compared to our current situation where the V4L2 bridge driver receives an array of I2C board information structures and instatiates the devices itself. Most of the DT support work will go in supporting that new instantiation mechanism. The bridge driver doesn't control instantiation of the I2C devices anymore, but need to bind with them at runtime. To make things more complex, it is expected that most I2C drivers to be arch independent, and they should be allowed to be used by a personal computer or by an embedded device. Agreed. That's why platform data structures won't go away anytime soon, a PCI bridge driver for hardware that contain I2C devices will still instantiate the I2C devices itself. We don't plan to or even want to get rid of that mechanism, as there are perfectly valid use cases. However, for DT-based embedded platforms, we need to support a new instantiation mechanism. Let me give 2 such examples: - ir-i2c-kbd driver supports lots of IR devices. Platform_data is needed to specify what hardware will actually be used, and what should be the default Remote Controller keymap; That driver isn't used on embedded platforms so it doesn't need to be changed now. - Sensor drivers like ov2940 is needed by soc_camera and by other webcam drivers like em28xx. The setup for em28xx should be completely different than the one for soc_camera: the initial registers init sequence is different for both. As several registers there aren't properly documented, there's no easy way to parametrize the configuration. Such drivers will need to support both DT-based platform data and structure- based platform data. They will likely parse the DT node and fill a platform data structure, to avoid duplicating initialization code. Please note that the new subdevs instantiation mechanism needed for DT will need to handle any instantiation order, as we can't guarantee the I2C device and bridge device instantiation order with DT. It should thus then support the current instantiation order we use for PCI and USB platforms. So, for me, we should not expose the I2C devices directly on OF; it should, instead, see just the bridge, and let the bridge to map the needed I2C devices using the needed platform_data. We can't do that, there will be no platform data anymore with DT-based platforms. As a summary, platform data structures won't go away, I2C drivers that need to work both on DT-based and non-DT-based platforms will need to support both the DT and platform data structures to get platform data. PCI and USB bridges will still instantiate their I2C devices, and binding between the I2C devices and the bridge can either be handled with two different instantiation mechanisms (the new one for DT platforms, with runtime bindings, and the existing one for non-DT platforms, with binding at instantiation time) or move to a single runtime
Re: [PATCH 05/14] media: add a V4L2 OF parser
Em Wed, 10 Oct 2012 16:48 +0200 Laurent Pinchart laurent.pinch...@ideasonboard.com escreveu: Hi Mauro, On Wednesday 10 October 2012 10:45:22 Mauro Carvalho Chehab wrote: Em Wed, 10 Oct 2012 14:54 +0200 Laurent Pinchart escreveu: Also, ideally OF-compatible (I2C) drivers should run with no platform data, but soc-camera is using I2C device platform data intensively. To avoid modifying the soc-camera core and all drivers, I also trigger on the BUS_NOTIFY_BIND_DRIVER event and assign a reference to the dynamically created platform data to the I2C device. Would we also want to do this for all V4L2 bridge drivers? We could call this a prepare callback or something similar... If that's going to be an interim solution only I'm fine with keeping it in soc-camera. I'm far from being an OF expert, but why do we need to export I2C devices to DT/OF? On my understanding, it is the bridge code that should be responsible for detecting, binding and initializing the proper I2C devices. On several cases, it is impossible or it would cause lots of ugly hacks if we ever try to move away from platform data stuff, as only the bridge driver knows what initialization is needed for an specific I2C driver. In a nutshell, a DT-based platform doesn't have any board code (except in rare cases, but let's not get into that), and thus doesn't pass any platform data structure to drivers. However, drivers receive a DT node, which contains a hierarchical description of the hardware, and parse those to extract information necessary to configure the device. One very important point to keep in mind here is that the DT is not Linux- specific. DT bindings are designed to be portable, and thus must only contain hardware descriptions, without any OS-specific information or policy information. For instance information such as the maximum video buffers size is not allowed in the DT. The DT is used both to provide platform data to drivers and to instantiate devices. I2C device DT nodes are stored as children of the I2C bus master DT node, and are automatically instantiated by the I2C bus master. This is a significant change compared to our current situation where the V4L2 bridge driver receives an array of I2C board information structures and instatiates the devices itself. Most of the DT support work will go in supporting that new instantiation mechanism. The bridge driver doesn't control instantiation of the I2C devices anymore, but need to bind with them at runtime. To make things more complex, it is expected that most I2C drivers to be arch independent, and they should be allowed to be used by a personal computer or by an embedded device. Agreed. That's why platform data structures won't go away anytime soon, a PCI bridge driver for hardware that contain I2C devices will still instantiate the I2C devices itself. We don't plan to or even want to get rid of that mechanism, as there are perfectly valid use cases. However, for DT-based embedded platforms, we need to support a new instantiation mechanism. Let me give 2 such examples: - ir-i2c-kbd driver supports lots of IR devices. Platform_data is needed to specify what hardware will actually be used, and what should be the default Remote Controller keymap; That driver isn't used on embedded platforms so it doesn't need to be changed now. - Sensor drivers like ov2940 is needed by soc_camera and by other webcam drivers like em28xx. The setup for em28xx should be completely different than the one for soc_camera: the initial registers init sequence is different for both. As several registers there aren't properly documented, there's no easy way to parametrize the configuration. Such drivers will need to support both DT-based platform data and structure- based platform data. They will likely parse the DT node and fill a platform data structure, to avoid duplicating initialization code. Please note that the new subdevs instantiation mechanism needed for DT will need to handle any instantiation order, as we can't guarantee the I2C device and bridge device instantiation order with DT. It should thus then support the current instantiation order we use for PCI and USB platforms. So, for me, we should not expose the I2C devices directly on OF; it should, instead, see just the bridge, and let the bridge to map the needed I2C devices using the needed platform_data. We can't do that, there will be no platform data anymore with DT-based platforms. As a summary, platform data structures won't go away, I2C drivers that need to work both on DT-based and non-DT-based platforms will need to support both the DT and platform data structures to get platform data. PCI and USB bridges will still instantiate their I2C devices, and binding between the I2C devices and the bridge can either be handled with two
Re: [PATCH 05/14] media: add a V4L2 OF parser
Hi Mauro, On Wednesday 10 October 2012 11:57:03 Mauro Carvalho Chehab wrote: Em Wed, 10 Oct 2012 16:48 +0200 Laurent Pinchart escreveu: On Wednesday 10 October 2012 10:45:22 Mauro Carvalho Chehab wrote: Em Wed, 10 Oct 2012 14:54 +0200 Laurent Pinchart escreveu: Also, ideally OF-compatible (I2C) drivers should run with no platform data, but soc-camera is using I2C device platform data intensively. To avoid modifying the soc-camera core and all drivers, I also trigger on the BUS_NOTIFY_BIND_DRIVER event and assign a reference to the dynamically created platform data to the I2C device. Would we also want to do this for all V4L2 bridge drivers? We could call this a prepare callback or something similar... If that's going to be an interim solution only I'm fine with keeping it in soc-camera. I'm far from being an OF expert, but why do we need to export I2C devices to DT/OF? On my understanding, it is the bridge code that should be responsible for detecting, binding and initializing the proper I2C devices. On several cases, it is impossible or it would cause lots of ugly hacks if we ever try to move away from platform data stuff, as only the bridge driver knows what initialization is needed for an specific I2C driver. In a nutshell, a DT-based platform doesn't have any board code (except in rare cases, but let's not get into that), and thus doesn't pass any platform data structure to drivers. However, drivers receive a DT node, which contains a hierarchical description of the hardware, and parse those to extract information necessary to configure the device. One very important point to keep in mind here is that the DT is not Linux- specific. DT bindings are designed to be portable, and thus must only contain hardware descriptions, without any OS-specific information or policy information. For instance information such as the maximum video buffers size is not allowed in the DT. The DT is used both to provide platform data to drivers and to instantiate devices. I2C device DT nodes are stored as children of the I2C bus master DT node, and are automatically instantiated by the I2C bus master. This is a significant change compared to our current situation where the V4L2 bridge driver receives an array of I2C board information structures and instatiates the devices itself. Most of the DT support work will go in supporting that new instantiation mechanism. The bridge driver doesn't control instantiation of the I2C devices anymore, but need to bind with them at runtime. To make things more complex, it is expected that most I2C drivers to be arch independent, and they should be allowed to be used by a personal computer or by an embedded device. Agreed. That's why platform data structures won't go away anytime soon, a PCI bridge driver for hardware that contain I2C devices will still instantiate the I2C devices itself. We don't plan to or even want to get rid of that mechanism, as there are perfectly valid use cases. However, for DT-based embedded platforms, we need to support a new instantiation mechanism. Let me give 2 such examples: - ir-i2c-kbd driver supports lots of IR devices. Platform_data is needed to specify what hardware will actually be used, and what should be the default Remote Controller keymap; That driver isn't used on embedded platforms so it doesn't need to be changed now. - Sensor drivers like ov2940 is needed by soc_camera and by other webcam drivers like em28xx. The setup for em28xx should be completely different than the one for soc_camera: the initial registers init sequence is different for both. As several registers there aren't properly documented, there's no easy way to parametrize the configuration. Such drivers will need to support both DT-based platform data and structure- based platform data. They will likely parse the DT node and fill a platform data structure, to avoid duplicating initialization code. Please note that the new subdevs instantiation mechanism needed for DT will need to handle any instantiation order, as we can't guarantee the I2C device and bridge device instantiation order with DT. It should thus then support the current instantiation order we use for PCI and USB platforms. So, for me, we should not expose the I2C devices directly on OF; it should, instead, see just the bridge, and let the bridge to map the needed I2C devices using the needed platform_data. We can't do that, there will be no platform data anymore with DT-based platforms. As a summary, platform data structures won't go away, I2C drivers that need to work both on DT-based and non-DT-based platforms will need to support both the DT and platform data structures to get platform data. PCI and USB bridges will still instantiate their I2C devices, and
Re: [PATCH 05/14] media: add a V4L2 OF parser
On 10/10/2012 07:18 AM, Laurent Pinchart wrote: On Monday 08 October 2012 17:15:53 Guennadi Liakhovetski wrote: ... But how do you get the subdev pointer? With the notifier I get it from i2c_get_clientdata(client) and what do you do without it? How do you get to the client? And can't it get that from DT as well? No, I don't think there is a way to get a device pointer from a DT node. I don't believe there's a generic API for this (although perhaps there could be), but it can be implemented quite easily. For example, on Tegra, the SMMU needs to flip a bit in the AHB register space in order to enable itself. The SMMU DT node contains a phandle that points at the AHB DT node. The SMMU driver parses the phandle and passes the DT node pointer to the AHB driver. The AHB driver looks up the struct device that was instantiated for that node. See drivers/amba/tegra-ahb.c:tegra_ahb_enable_smmu(). There are a few other cases of similar code in the kernel, although I can't remember the others! -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/14] media: add a V4L2 OF parser
Hi Laurent, On 10/10/2012 03:25 PM, Laurent Pinchart wrote: On Tuesday 09 October 2012 13:00:24 Hans Verkuil wrote: On Tue 9 October 2012 12:34:48 Sylwester Nawrocki wrote: On 10/08/2012 11:40 AM, Guennadi Liakhovetski wrote: On Fri, 5 Oct 2012, Sylwester Nawrocki wrote: I would really like to see more than one user until we add any core code. No that it couldn't be changed afterwards, but it would be nice to ensure the concepts are right and proven in real life. Unfortunately I don't have any more systems on which I could easily enough try this. I've got a beagleboard with a camera, but I don't think I'm a particularly good candidate for implementing DT support for OMAP3 camera drivers;-) Apart from that I've only got soc-camera based systems, of which none are _really_ DT-ready... At best I could try an i.MX31 based board, but that doesn't have a very well developed .dts and that would be soc-camera too anyway. I certainly wouldn't expect you would do all the job. I mean it would be good to possibly have some other developers adding device tree support based on that new bindings and new infrastructure related to them. As I mentioned in another e-mail, I plan to work on DT support for the OMAP3 ISP, but I first need generic clock framework support for OMAP3. OK, let's hope it's available soon. There have been recently some progress in device tree support for Exynos SoCs, including common clock framework support and we hope to add FDT support to the Samsung SoC camera devices during this kernel cycle, based on the newly designed media bindings. This is going to be a second attempt, after our initial RFC from May [1]. It would still be SoC specific implementation, but not soc-camera based. I wasn't a big fan of this asynchronous sub-devices probing, but it now seems to be a most complete solution to me. I think it just need to be done right at the v4l2-core so individual drivers don't get complicated too much. After investigating this some more I think I agree with that. There are some things where we should probably ask for advice from the i2c subsystem devs, I'm thinking of putting the driver back into the deferred-probe state in particular. We might actually not need that, it might be easier to handle the circular dependency problem from the other end. We could add a way (ioctl, sysfs, ...) to force a V4L2 bridge driver to release its subdevs. Once done, the subdev driver could be unloaded and/or the subdev device unregistered, which would release the resources used by the subdev, such as clocks. The bridge driver could then be unregistered. That sounds like an option. Perhaps it could be done by v4l2-core, e.g. a sysfs entry could be registered for a media or video device if driver requests it. I'm not sure if we should allow subdevs in released state, perhaps it's better to just unregister subdevs entirely right away ? Creating v4l2-core support for this is crucial as it is quite complex and without core support this is going to be a nightmare for drivers. -- Regards, Sylwester -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/14] media: add a V4L2 OF parser
On Wed, 10 Oct 2012, Sylwester Nawrocki wrote: Hi Laurent, On 10/10/2012 03:25 PM, Laurent Pinchart wrote: On Tuesday 09 October 2012 13:00:24 Hans Verkuil wrote: On Tue 9 October 2012 12:34:48 Sylwester Nawrocki wrote: On 10/08/2012 11:40 AM, Guennadi Liakhovetski wrote: On Fri, 5 Oct 2012, Sylwester Nawrocki wrote: I would really like to see more than one user until we add any core code. No that it couldn't be changed afterwards, but it would be nice to ensure the concepts are right and proven in real life. Unfortunately I don't have any more systems on which I could easily enough try this. I've got a beagleboard with a camera, but I don't think I'm a particularly good candidate for implementing DT support for OMAP3 camera drivers;-) Apart from that I've only got soc-camera based systems, of which none are _really_ DT-ready... At best I could try an i.MX31 based board, but that doesn't have a very well developed .dts and that would be soc-camera too anyway. I certainly wouldn't expect you would do all the job. I mean it would be good to possibly have some other developers adding device tree support based on that new bindings and new infrastructure related to them. As I mentioned in another e-mail, I plan to work on DT support for the OMAP3 ISP, but I first need generic clock framework support for OMAP3. OK, let's hope it's available soon. There have been recently some progress in device tree support for Exynos SoCs, including common clock framework support and we hope to add FDT support to the Samsung SoC camera devices during this kernel cycle, based on the newly designed media bindings. This is going to be a second attempt, after our initial RFC from May [1]. It would still be SoC specific implementation, but not soc-camera based. I wasn't a big fan of this asynchronous sub-devices probing, but it now seems to be a most complete solution to me. I think it just need to be done right at the v4l2-core so individual drivers don't get complicated too much. After investigating this some more I think I agree with that. There are some things where we should probably ask for advice from the i2c subsystem devs, I'm thinking of putting the driver back into the deferred-probe state in particular. We might actually not need that, it might be easier to handle the circular dependency problem from the other end. We could add a way (ioctl, sysfs, ...) to force a V4L2 bridge driver to release its subdevs. Once done, the subdev driver could be unloaded and/or the subdev device unregistered, which would release the resources used by the subdev, such as clocks. The bridge driver could then be unregistered. That sounds like an option. Perhaps it could be done by v4l2-core, e.g. a sysfs entry could be registered for a media or video device if driver requests it. I'm not sure if we should allow subdevs in released state, perhaps it's better to just unregister subdevs entirely right away ? What speaks against holding a clock reference only during streaming, or at least between open and close? Wouldn't that solve the problem naturally? Yes, after giving up your reference to a clock at close() and re-acquiring it at open() you will have to make sure the frequency hasn't change, resp. adjust it, but I don't see it as a huge problem, I don't think many users on embedded systems will compete for your camera master clock. And if they do, you have a different problem, IMHO;-) Thanks Guennadi Creating v4l2-core support for this is crucial as it is quite complex and without core support this is going to be a nightmare for drivers. -- Regards, Sylwester --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/14] media: add a V4L2 OF parser
On 10/10/2012 10:32 PM, Guennadi Liakhovetski wrote: We might actually not need that, it might be easier to handle the circular dependency problem from the other end. We could add a way (ioctl, sysfs, ...) to force a V4L2 bridge driver to release its subdevs. Once done, the subdev driver could be unloaded and/or the subdev device unregistered, which would release the resources used by the subdev, such as clocks. The bridge driver could then be unregistered. That sounds like an option. Perhaps it could be done by v4l2-core, e.g. a sysfs entry could be registered for a media or video device if driver requests it. I'm not sure if we should allow subdevs in released state, perhaps it's better to just unregister subdevs entirely right away ? What speaks against holding a clock reference only during streaming, or at least between open and close? Wouldn't that solve the problem naturally? Yes, after giving up your reference to a clock at close() and re-acquiring it at open() you will have to make sure the frequency hasn't change, resp. adjust it, but I don't see it as a huge problem, I don't think many users on embedded systems will compete for your camera master clock. And if they do, you have a different problem, IMHO;-) I agree, normally nobody should touch these clocks except the subdev (or as of now the host) drivers. It depends on a sensor, video encoder, etc. how much it tolerates switching the clock on/off. I suppose it's best to acquire/release it in .s_power callback, since only then the proper voltage supply, GPIO, clock enable/disable sequences could be ensured. I know those things are currently mostly ignored, but some sensors might be picky WRT their initialization/shutdown sequences and it would be good to ensure these sequences are fully controllable by the sensor driver itsels, where the host's architecture allows that. To summarize, I can't see how holding a clock only when a device is active could cause any problems, in case of camera sensors. I'm not sure about other devices, like e.g. tuners. -- Regards, Sylwester -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/14] media: add a V4L2 OF parser
On Wednesday 10 October 2012 10:50:20 Stephen Warren wrote: On 10/10/2012 07:18 AM, Laurent Pinchart wrote: On Monday 08 October 2012 17:15:53 Guennadi Liakhovetski wrote: ... But how do you get the subdev pointer? With the notifier I get it from i2c_get_clientdata(client) and what do you do without it? How do you get to the client? And can't it get that from DT as well? No, I don't think there is a way to get a device pointer from a DT node. I don't believe there's a generic API for this (although perhaps there could be), but it can be implemented quite easily. For example, on Tegra, the SMMU needs to flip a bit in the AHB register space in order to enable itself. The SMMU DT node contains a phandle that points at the AHB DT node. The SMMU driver parses the phandle and passes the DT node pointer to the AHB driver. The AHB driver looks up the struct device that was instantiated for that node. See drivers/amba/tegra-ahb.c:tegra_ahb_enable_smmu(). There are a few other cases of similar code in the kernel, although I can't remember the others! That's a very naive approach, but what about storing the struct device in struct device_node when the device is instantiated ? It's so simple that there's probably a good reason why that hasn't been implemented though. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/14] media: add a V4L2 OF parser
Hi Sylwester, On Wednesday 10 October 2012 22:23:35 Sylwester Nawrocki wrote: On 10/10/2012 03:25 PM, Laurent Pinchart wrote: On Tuesday 09 October 2012 13:00:24 Hans Verkuil wrote: On Tue 9 October 2012 12:34:48 Sylwester Nawrocki wrote: On 10/08/2012 11:40 AM, Guennadi Liakhovetski wrote: On Fri, 5 Oct 2012, Sylwester Nawrocki wrote: I would really like to see more than one user until we add any core code. No that it couldn't be changed afterwards, but it would be nice to ensure the concepts are right and proven in real life. Unfortunately I don't have any more systems on which I could easily enough try this. I've got a beagleboard with a camera, but I don't think I'm a particularly good candidate for implementing DT support for OMAP3 camera drivers;-) Apart from that I've only got soc-camera based systems, of which none are _really_ DT-ready... At best I could try an i.MX31 based board, but that doesn't have a very well developed .dts and that would be soc-camera too anyway. I certainly wouldn't expect you would do all the job. I mean it would be good to possibly have some other developers adding device tree support based on that new bindings and new infrastructure related to them. As I mentioned in another e-mail, I plan to work on DT support for the OMAP3 ISP, but I first need generic clock framework support for OMAP3. OK, let's hope it's available soon. I've been told a month and a half ago that v3.7 was a plausible target, let's see. There have been recently some progress in device tree support for Exynos SoCs, including common clock framework support and we hope to add FDT support to the Samsung SoC camera devices during this kernel cycle, based on the newly designed media bindings. This is going to be a second attempt, after our initial RFC from May [1]. It would still be SoC specific implementation, but not soc-camera based. I wasn't a big fan of this asynchronous sub-devices probing, but it now seems to be a most complete solution to me. I think it just need to be done right at the v4l2-core so individual drivers don't get complicated too much. After investigating this some more I think I agree with that. There are some things where we should probably ask for advice from the i2c subsystem devs, I'm thinking of putting the driver back into the deferred-probe state in particular. We might actually not need that, it might be easier to handle the circular dependency problem from the other end. We could add a way (ioctl, sysfs, ...) to force a V4L2 bridge driver to release its subdevs. Once done, the subdev driver could be unloaded and/or the subdev device unregistered, which would release the resources used by the subdev, such as clocks. The bridge driver could then be unregistered. That sounds like an option. Perhaps it could be done by v4l2-core, e.g. a sysfs entry could be registered for a media or video device if driver requests it. That's what I was thinking about. I'm not sure if we should allow subdevs in released state, perhaps it's better to just unregister subdevs entirely right away ? I think we need three states: not created, unbound and bound (names are not set in stone). The not created state corresponds to a subdev that hasn't been created yet by its I2C/SPI/whatever driver (agreed, it's not really a state technically). Upon creation the subdev is not bound to any bridge driver. It later gets bound when the bridge driver requests the subdev through the API the V4L2 core will provide (most probably using notifiers). The new sysfs entry would be used to unbind subdevs (either selectively, or all in one go) from the bridge, in which case they will go back to the unbound state, allowing driver removal or device release. Creating v4l2-core support for this is crucial as it is quite complex and without core support this is going to be a nightmare for drivers. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/14] media: add a V4L2 OF parser
Hi Guennadi, On Wednesday 10 October 2012 22:32:22 Guennadi Liakhovetski wrote: On Wed, 10 Oct 2012, Sylwester Nawrocki wrote: On 10/10/2012 03:25 PM, Laurent Pinchart wrote: On Tuesday 09 October 2012 13:00:24 Hans Verkuil wrote: On Tue 9 October 2012 12:34:48 Sylwester Nawrocki wrote: On 10/08/2012 11:40 AM, Guennadi Liakhovetski wrote: On Fri, 5 Oct 2012, Sylwester Nawrocki wrote: I would really like to see more than one user until we add any core code. No that it couldn't be changed afterwards, but it would be nice to ensure the concepts are right and proven in real life. Unfortunately I don't have any more systems on which I could easily enough try this. I've got a beagleboard with a camera, but I don't think I'm a particularly good candidate for implementing DT support for OMAP3 camera drivers;-) Apart from that I've only got soc-camera based systems, of which none are _really_ DT-ready... At best I could try an i.MX31 based board, but that doesn't have a very well developed .dts and that would be soc-camera too anyway. I certainly wouldn't expect you would do all the job. I mean it would be good to possibly have some other developers adding device tree support based on that new bindings and new infrastructure related to them. As I mentioned in another e-mail, I plan to work on DT support for the OMAP3 ISP, but I first need generic clock framework support for OMAP3. OK, let's hope it's available soon. There have been recently some progress in device tree support for Exynos SoCs, including common clock framework support and we hope to add FDT support to the Samsung SoC camera devices during this kernel cycle, based on the newly designed media bindings. This is going to be a second attempt, after our initial RFC from May [1]. It would still be SoC specific implementation, but not soc-camera based. I wasn't a big fan of this asynchronous sub-devices probing, but it now seems to be a most complete solution to me. I think it just need to be done right at the v4l2-core so individual drivers don't get complicated too much. After investigating this some more I think I agree with that. There are some things where we should probably ask for advice from the i2c subsystem devs, I'm thinking of putting the driver back into the deferred-probe state in particular. We might actually not need that, it might be easier to handle the circular dependency problem from the other end. We could add a way (ioctl, sysfs, ...) to force a V4L2 bridge driver to release its subdevs. Once done, the subdev driver could be unloaded and/or the subdev device unregistered, which would release the resources used by the subdev, such as clocks. The bridge driver could then be unregistered. That sounds like an option. Perhaps it could be done by v4l2-core, e.g. a sysfs entry could be registered for a media or video device if driver requests it. I'm not sure if we should allow subdevs in released state, perhaps it's better to just unregister subdevs entirely right away ? What speaks against holding a clock reference only during streaming, or at least between open and close? Wouldn't that solve the problem naturally? Yes, after giving up your reference to a clock at close() and re-acquiring it at open() you will have to make sure the frequency hasn't change, resp. adjust it, but I don't see it as a huge problem, I don't think many users on embedded systems will compete for your camera master clock. And if they do, you have a different problem, IMHO;-) That's an option as well. I'm a bit worried that it would make subdev drivers more complex, as they would need to acquire/release resources in a more fine- grained fashion at runtime, and handle failures dynamically there instead of doing it all at probe time. It could work though, I think we need to experiment the possible solutions to find out which one is the best. Regardless of how we solve the circular dependencies issue at unregistration time we will need an easy way for bridge drivers to bind to subdevs. I believe that's orthogonal to the unregistration problem, so we can start working on registration before knowing exactly how unregistration will be handled. Creating v4l2-core support for this is crucial as it is quite complex and without core support this is going to be a nightmare for drivers. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/14] media: add a V4L2 OF parser
Hi Guennadi, On 10/08/2012 11:40 AM, Guennadi Liakhovetski wrote: On Fri, 5 Oct 2012, Sylwester Nawrocki wrote: I would really like to see more than one user until we add any core code. No that it couldn't be changed afterwards, but it would be nice to ensure the concepts are right and proven in real life. Unfortunately I don't have any more systems on which I could easily enough try this. I've got a beagleboard with a camera, but I don't think I'm a particularly good candidate for implementing DT support for OMAP3 camera drivers;-) Apart from that I've only got soc-camera based systems, of which none are _really_ DT-ready... At best I could try an i.MX31 based board, but that doesn't have a very well developed .dts and that would be soc-camera too anyway. I certainly wouldn't expect you would do all the job. I mean it would be good to possibly have some other developers adding device tree support based on that new bindings and new infrastructure related to them. There have been recently some progress in device tree support for Exynos SoCs, including common clock framework support and we hope to add FDT support to the Samsung SoC camera devices during this kernel cycle, based on the newly designed media bindings. This is going to be a second attempt, after our initial RFC from May [1]. It would still be SoC specific implementation, but not soc-camera based. I wasn't a big fan of this asynchronous sub-devices probing, but it now seems to be a most complete solution to me. I think it just need to be done right at the v4l2-core so individual drivers don't get complicated too much. -- Regards, Sylwester [1] http://www.spinics.net/lists/linux-media/msg48341.html -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/14] media: add a V4L2 OF parser
On Tue 9 October 2012 12:34:48 Sylwester Nawrocki wrote: Hi Guennadi, On 10/08/2012 11:40 AM, Guennadi Liakhovetski wrote: On Fri, 5 Oct 2012, Sylwester Nawrocki wrote: I would really like to see more than one user until we add any core code. No that it couldn't be changed afterwards, but it would be nice to ensure the concepts are right and proven in real life. Unfortunately I don't have any more systems on which I could easily enough try this. I've got a beagleboard with a camera, but I don't think I'm a particularly good candidate for implementing DT support for OMAP3 camera drivers;-) Apart from that I've only got soc-camera based systems, of which none are _really_ DT-ready... At best I could try an i.MX31 based board, but that doesn't have a very well developed .dts and that would be soc-camera too anyway. I certainly wouldn't expect you would do all the job. I mean it would be good to possibly have some other developers adding device tree support based on that new bindings and new infrastructure related to them. There have been recently some progress in device tree support for Exynos SoCs, including common clock framework support and we hope to add FDT support to the Samsung SoC camera devices during this kernel cycle, based on the newly designed media bindings. This is going to be a second attempt, after our initial RFC from May [1]. It would still be SoC specific implementation, but not soc-camera based. I wasn't a big fan of this asynchronous sub-devices probing, but it now seems to be a most complete solution to me. I think it just need to be done right at the v4l2-core so individual drivers don't get complicated too much. After investigating this some more I think I agree with that. There are some things where we should probably ask for advice from the i2c subsystem devs, I'm thinking of putting the driver back into the deferred-probe state in particular. Creating v4l2-core support for this is crucial as it is quite complex and without core support this is going to be a nightmare for drivers. Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/14] media: add a V4L2 OF parser
Hi Sylwester On Fri, 5 Oct 2012, Sylwester Nawrocki wrote: On 10/05/2012 12:58 PM, Guennadi Liakhovetski wrote: One area that I do not yet completely understand is the i2c bus notifications (or asynchronous loading of i2c modules). I would have expected that using OF the i2c devices are still initialized before the host/bridge driver is initialized. But I gather that's not the case? No, it's not. I'm not sure, whether it depends on the order of devices in the .dts, but, I think, it's better to not have to mandate a certain order and I also seem to have seen devices being registered in different order with the same DT, but I'm not 100% sure about that. The device instantiation (and initialization) order is not something that is supposed to be ensured by a specific device tree source layout, IIUC. The initialization sequence is probably something that is specific to a particular operating system. I checked the device tree compiler but couldn't find if it is free to reorder anything when converting from the human readable device tree to its binary representation. The deferred probing was introduced in Linux to resolve resource inter-dependencies in case of FDT based booting AFAIK. If this deferred probing is a general problem, then I think we need a general solution as well that's part of the v4l2 core. That can be done, perhaps. But we can do it as a next step. As soon as we're happy with the OF implementation as such, we can commit that, possibly leaving soc-camera patches out for now, then we can think where to put async I2C handling. I would really like to see more than one user until we add any core code. No that it couldn't be changed afterwards, but it would be nice to ensure the concepts are right and proven in real life. Unfortunately I don't have any more systems on which I could easily enough try this. I've got a beagleboard with a camera, but I don't think I'm a particularly good candidate for implementing DT support for OMAP3 camera drivers;-) Apart from that I've only got soc-camera based systems, of which none are _really_ DT-ready... At best I could try an i.MX31 based board, but that doesn't have a very well developed .dts and that would be soc-camera too anyway. 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 linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/14] media: add a V4L2 OF parser
Hi Guennadi,
On 09/27/2012 04:07 PM, Guennadi Liakhovetski wrote:
Add a V4L2 OF parser, implementing bindings, documented in
Documentation/devicetree/bindings/media/v4l2.txt.
Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de
---
drivers/media/v4l2-core/Makefile |3 +
drivers/media/v4l2-core/v4l2-of.c | 190
+
include/media/v4l2-of.h | 62
3 files changed, 255 insertions(+), 0 deletions(-)
create mode 100644 drivers/media/v4l2-core/v4l2-of.c
create mode 100644 include/media/v4l2-of.h
diff --git a/drivers/media/v4l2-core/Makefile
b/drivers/media/v4l2-core/Makefile
index c2d61d4..00f64d6 100644
--- a/drivers/media/v4l2-core/Makefile
+++ b/drivers/media/v4l2-core/Makefile
@@ -9,6 +9,9 @@ videodev-objs := v4l2-dev.o v4l2-ioctl.o v4l2-device.o
v4l2-fh.o \
ifeq ($(CONFIG_COMPAT),y)
videodev-objs += v4l2-compat-ioctl32.o
endif
+ifeq ($(CONFIG_OF),y)
+ videodev-objs += v4l2-of.o
+endif
obj-$(CONFIG_VIDEO_DEV) += videodev.o v4l2-int-device.o
obj-$(CONFIG_VIDEO_V4L2) += v4l2-common.o
diff --git a/drivers/media/v4l2-core/v4l2-of.c
b/drivers/media/v4l2-core/v4l2-of.c
new file mode 100644
index 000..f45d64b
--- /dev/null
+++ b/drivers/media/v4l2-core/v4l2-of.c
@@ -0,0 +1,190 @@
+/*
+ * V4L2 OF binding parsing library
+ *
+ * Copyright (C) 2012 Renesas Electronics Corp.
+ * Author: Guennadi Liakhovetski g.liakhovet...@gmx.de
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of version 2 of the GNU General Public License as
+ * published by the Free Software Foundation.
+ */
+#include linux/kernel.h
+#include linux/module.h
+#include linux/of.h
+#include linux/types.h
+
+#include media/v4l2-of.h
+
+/*
+ * All properties are optional. If none are found, we don't set any flags.
This
+ * means, the port has a static configuration and no properties have to be
+ * specified explicitly.
+ * If any properties are found, that identify the bus as parallel, and
+ * slave-mode isn't set, we set V4L2_MBUS_MASTER. Similarly, if we recognise
the
+ * bus as serial CSI-2 and clock-noncontinuous isn't set, we set the
+ * V4L2_MBUS_CSI2_CONTINUOUS_CLOCK flag.
+ * The caller should hold a reference to node.
+ */
Since this is a library function, how about converting this description
to kernel-doc ?
+void v4l2_of_parse_link(const struct device_node *node,
+ struct v4l2_of_link *link)
+{
+ const struct device_node *port_node = of_get_parent(node);
+ int size;
+ unsigned int v;
+ u32 data_lanes[ARRAY_SIZE(link-mipi_csi_2.data_lanes)];
+ bool data_lanes_present;
+
+ memset(link, 0, sizeof(*link));
+
+ link-local_node = node;
+
+ /* Doesn't matter, whether the below two calls succeed */
+ of_property_read_u32(port_node, reg, link-port);
+ of_property_read_u32(node, reg, link-addr);
+
+ if (!of_property_read_u32(node, bus-width, v))
+ link-parallel.bus_width = v;
+
+ if (!of_property_read_u32(node, data-shift, v))
+ link-parallel.data_shift = v;
+
+ if (!of_property_read_u32(node, hsync-active, v))
+ link-mbus_flags |= v ? V4L2_MBUS_HSYNC_ACTIVE_HIGH :
+ V4L2_MBUS_HSYNC_ACTIVE_LOW;
+
+ if (!of_property_read_u32(node, vsync-active, v))
+ link-mbus_flags |= v ? V4L2_MBUS_VSYNC_ACTIVE_HIGH :
+ V4L2_MBUS_VSYNC_ACTIVE_LOW;
+
+ if (!of_property_read_u32(node, data-active, v))
+ link-mbus_flags |= v ? V4L2_MBUS_DATA_ACTIVE_HIGH :
+ V4L2_MBUS_DATA_ACTIVE_LOW;
+
+ if (!of_property_read_u32(node, pclk-sample, v))
+ link-mbus_flags |= v ? V4L2_MBUS_PCLK_SAMPLE_RISING :
+ V4L2_MBUS_PCLK_SAMPLE_FALLING;
+
+ if (!of_property_read_u32(node, field-even-active, v))
+ link-mbus_flags |= v ? V4L2_MBUS_FIELD_EVEN_HIGH :
+ V4L2_MBUS_FIELD_EVEN_LOW;
+
+ if (of_get_property(node, slave-mode, size))
+ link-mbus_flags |= V4L2_MBUS_SLAVE;
+
+ /* If any parallel-bus properties have been found, skip serial ones */
+ if (link-parallel.bus_width || link-parallel.data_shift ||
+ link-mbus_flags) {
+ /* Default parallel bus-master */
+ if (!(link-mbus_flags V4L2_MBUS_SLAVE))
+ link-mbus_flags |= V4L2_MBUS_MASTER;
+ return;
+ }
+
+ if (!of_property_read_u32(node, clock-lanes, v))
+ link-mipi_csi_2.clock_lane = v;
+
+ if (!of_property_read_u32_array(node, data-lanes, data_lanes,
+ ARRAY_SIZE(data_lanes))) {
+ int i;
+ for (i = 0; i ARRAY_SIZE(data_lanes); i++)
+ link-mipi_csi_2.data_lanes[i] = data_lanes[i];
+
Re: [PATCH 05/14] media: add a V4L2 OF parser
Hi Hans On Fri, 5 Oct 2012, Hans Verkuil wrote: [snip] I think the soc_camera patches should be left out for now. I suspect that by adding core support for async i2c handling first, Ok, let's think, what this meacs - async I2C in media / V4L2 core. The main reason for our probing order problem is the master clock, typically supplied from the camera bridge to I2C subdevices, which we only want to start when necessary, i.e. when accessing the subdevice. And the subdevice driver needs that clock running during its .probe() to be able to access and verify or configure the hardware. Our current solution is to not register I2C subdevices from the platform data, as is usual for all I2C devices, but from the bridge driver and only after it has switched on the master clock. After the subdevice driver has completed its probing we switch the clock back off until the subdevice has to be activated, e.g. for video capture. Also important - when we want to unregister the bridge driver we just also unregister the I2C device. Now, to reverse the whole thing and to allow I2C devices be registered as usual - via platform data or OF, first of all we have to teach I2C subdevice drivers to recognise the too early situation and request deferred probing in such a case. Then it will be reprobed after each new successful probe or unregister on the system. After the bridge driver has successfully probed the subdevice driver will be re-probed and at that time it should succeed. Now, there is a problem here too: who should switch on and off the master clock? If we do it from the bridge driver, we could install an I2C bus-notifier, _before_ the subdevice driver is probed, i.e. upon the BUS_NOTIFY_BIND_DRIVER event we could turn on the clock. If subdevice probing was successful, we can then wait for the BUS_NOTIFY_BOUND_DRIVER event to switch the clock back off. BUT - if the subdevice fails probing? How do we find out about that and turn the clock back off? There is no notification event for that... Possible solutions: 1. timer - ugly and unreliable. 2. add a probing failed notifier event to the device core - would this be accepted? 3. let the subdevice turn the master clock on and off for the duration of probing. My vote goes for (3). Ideally this should be done using the generic clock framework. But can we really expect all drivers and platforms to switch to it quickly enough? If not, we need a V4L2-specific callback from subdevice drivers to bridge drivers to turn the clock on and off. That's what I've done temporarily in this patch series for soc-camera. Suppose we decide to do the same for V4L2 centrally - add call-backs. Then we can think what else we need to add to V4L2 to support asynchronous subdevice driver probing. 1. We'll have to create these V4L2 clock start and stop functions, that, supplied (in case of I2C) with client address and adapter number will find the correct v4l2_device instance and call its callbacks. 2. The I2C notifier. I'm not sure, whether this one should be common. Of common tasks we have to refcount the I2C adapter and register the subdevice. Then we'd have to call the bridge driver's callback. Is it worth it doing this centrally or rather allow individual drivers to do that themselves? Also, ideally OF-compatible (I2C) drivers should run with no platform data, but soc-camera is using I2C device platform data intensively. To avoid modifying the soc-camera core and all drivers, I also trigger on the BUS_NOTIFY_BIND_DRIVER event and assign a reference to the dynamically created platform data to the I2C device. Would we also want to do this for all V4L2 bridge drivers? We could call this a prepare callback or something similar... 3. Bridge driver unregistering. Here we have to put the subdevice driver back into the deferred-probe state... Ugliness alert: I'm doing this by unregistering and re-registering the I2C device... For that I also have to create a copy of devices I2C board-info data. Lovely, ain't it? This I'd be happy to move to the V4L2 core;-) Thanks Guennadi the soc_camera patches will become a lot easier to understand. Regards, Hans 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 linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/14] media: add a V4L2 OF parser
On Mon October 8 2012 14:23:25 Guennadi Liakhovetski wrote: Hi Hans On Fri, 5 Oct 2012, Hans Verkuil wrote: [snip] I think the soc_camera patches should be left out for now. I suspect that by adding core support for async i2c handling first, Ok, let's think, what this meacs - async I2C in media / V4L2 core. The main reason for our probing order problem is the master clock, typically supplied from the camera bridge to I2C subdevices, which we only want to start when necessary, i.e. when accessing the subdevice. And the subdevice driver needs that clock running during its .probe() to be able to access and verify or configure the hardware. Our current solution is to not register I2C subdevices from the platform data, as is usual for all I2C devices, but from the bridge driver and only after it has switched on the master clock. After the subdevice driver has completed its probing we switch the clock back off until the subdevice has to be activated, e.g. for video capture. Also important - when we want to unregister the bridge driver we just also unregister the I2C device. Now, to reverse the whole thing and to allow I2C devices be registered as usual - via platform data or OF, first of all we have to teach I2C subdevice drivers to recognise the too early situation and request deferred probing in such a case. Then it will be reprobed after each new successful probe or unregister on the system. After the bridge driver has successfully probed the subdevice driver will be re-probed and at that time it should succeed. Now, there is a problem here too: who should switch on and off the master clock? If we do it from the bridge driver, we could install an I2C bus-notifier, _before_ the subdevice driver is probed, i.e. upon the BUS_NOTIFY_BIND_DRIVER event we could turn on the clock. If subdevice probing was successful, we can then wait for the BUS_NOTIFY_BOUND_DRIVER event to switch the clock back off. BUT - if the subdevice fails probing? How do we find out about that and turn the clock back off? There is no notification event for that... Possible solutions: 1. timer - ugly and unreliable. 2. add a probing failed notifier event to the device core - would this be accepted? 3. let the subdevice turn the master clock on and off for the duration of probing. My vote goes for (3). Ideally this should be done using the generic clock framework. But can we really expect all drivers and platforms to switch to it quickly enough? If not, we need a V4L2-specific callback from subdevice drivers to bridge drivers to turn the clock on and off. That's what I've done temporarily in this patch series for soc-camera. Suppose we decide to do the same for V4L2 centrally - add call-backs. Then we can think what else we need to add to V4L2 to support asynchronous subdevice driver probing. I wonder, don't we have the necessary code already? V4L2 subdev drivers can have internal_ops with register/unregister ops. These are called by v4l2_device_register_subdev. This happens during the bridge driver's probe. Suppose the subdev's probe does not actually access the i2c device, but instead defers that to the register callback? The bridge driver will turn on the clock before calling v4l2_device_register_subdev to ensure that the register callback can access the i2c registers. The register callback will do any initialization and can return an error. In case of an error the i2c client is automatically unregistered as well. In addition, during the register op the subdev driver can call into the bridge driver since it knows the v4l2_device struct. This has also the advantage that subdev drivers can change to this model gradually. Only drivers that need master clocks, etc. need to move any probe code that actually accesses hardware to the register op. Others can remain as. Nor should this change behavior of existing drivers as this happens all in the V4L2 core. The bridge driver may still have to wait until all i2c drivers are loaded, though. But that can definitely be handled centrally (i.e.: 'I need these drivers, wait until all are loaded'). 1. We'll have to create these V4L2 clock start and stop functions, that, supplied (in case of I2C) with client address and adapter number will find the correct v4l2_device instance and call its callbacks. 2. The I2C notifier. I'm not sure, whether this one should be common. Of common tasks we have to refcount the I2C adapter and register the subdevice. Then we'd have to call the bridge driver's callback. Is it worth it doing this centrally or rather allow individual drivers to do that themselves? Also, ideally OF-compatible (I2C) drivers should run with no platform data, but soc-camera is using I2C device platform data intensively. To avoid modifying the soc-camera core and all drivers, I also trigger on the BUS_NOTIFY_BIND_DRIVER event and assign a reference to the
Re: [PATCH 05/14] media: add a V4L2 OF parser
On Mon, 8 Oct 2012, Hans Verkuil wrote: On Mon October 8 2012 14:23:25 Guennadi Liakhovetski wrote: Hi Hans On Fri, 5 Oct 2012, Hans Verkuil wrote: [snip] I think the soc_camera patches should be left out for now. I suspect that by adding core support for async i2c handling first, Ok, let's think, what this meacs - async I2C in media / V4L2 core. The main reason for our probing order problem is the master clock, typically supplied from the camera bridge to I2C subdevices, which we only want to start when necessary, i.e. when accessing the subdevice. And the subdevice driver needs that clock running during its .probe() to be able to access and verify or configure the hardware. Our current solution is to not register I2C subdevices from the platform data, as is usual for all I2C devices, but from the bridge driver and only after it has switched on the master clock. After the subdevice driver has completed its probing we switch the clock back off until the subdevice has to be activated, e.g. for video capture. Also important - when we want to unregister the bridge driver we just also unregister the I2C device. Now, to reverse the whole thing and to allow I2C devices be registered as usual - via platform data or OF, first of all we have to teach I2C subdevice drivers to recognise the too early situation and request deferred probing in such a case. Then it will be reprobed after each new successful probe or unregister on the system. After the bridge driver has successfully probed the subdevice driver will be re-probed and at that time it should succeed. Now, there is a problem here too: who should switch on and off the master clock? If we do it from the bridge driver, we could install an I2C bus-notifier, _before_ the subdevice driver is probed, i.e. upon the BUS_NOTIFY_BIND_DRIVER event we could turn on the clock. If subdevice probing was successful, we can then wait for the BUS_NOTIFY_BOUND_DRIVER event to switch the clock back off. BUT - if the subdevice fails probing? How do we find out about that and turn the clock back off? There is no notification event for that... Possible solutions: 1. timer - ugly and unreliable. 2. add a probing failed notifier event to the device core - would this be accepted? 3. let the subdevice turn the master clock on and off for the duration of probing. My vote goes for (3). Ideally this should be done using the generic clock framework. But can we really expect all drivers and platforms to switch to it quickly enough? If not, we need a V4L2-specific callback from subdevice drivers to bridge drivers to turn the clock on and off. That's what I've done temporarily in this patch series for soc-camera. Suppose we decide to do the same for V4L2 centrally - add call-backs. Then we can think what else we need to add to V4L2 to support asynchronous subdevice driver probing. I wonder, don't we have the necessary code already? V4L2 subdev drivers can have internal_ops with register/unregister ops. These are called by v4l2_device_register_subdev. This happens during the bridge driver's probe. Suppose the subdev's probe does not actually access the i2c device, but instead defers that to the register callback? The bridge driver will turn on the clock before calling v4l2_device_register_subdev to ensure that the register callback can access the i2c registers. The register callback will do any initialization and can return an error. In case of an error the i2c client is automatically unregistered as well. Yes, if v4l2_i2c_new_subdev_board() is used. This has been discussed several times before and always what I didn't like in this is, that I2C device probe() in this case succeeds without even trying to access the hardware. And think about DT. In this case we don't instantiate the I2C device, OF code does it for us. What do you do then? If you let probe() succeed, then you will have to somehow remember the subdevice to later match it against bridges... In addition, during the register op the subdev driver can call into the bridge driver since it knows the v4l2_device struct. This has also the advantage that subdev drivers can change to this model gradually. Only drivers that need master clocks, etc. need to move any probe code that actually accesses hardware to the register op. Others can remain as. Nor should this change behavior of existing drivers as this happens all in the V4L2 core. The bridge driver may still have to wait until all i2c drivers are loaded, though. But that can definitely be handled centrally (i.e.: 'I need these drivers, wait until all are loaded'). 1. We'll have to create these V4L2 clock start and stop functions, that, supplied (in case of I2C) with client address and adapter number will find the correct v4l2_device instance and call its callbacks. 2. The
Re: [PATCH 05/14] media: add a V4L2 OF parser
On Mon October 8 2012 16:30:53 Guennadi Liakhovetski wrote: On Mon, 8 Oct 2012, Hans Verkuil wrote: On Mon October 8 2012 14:23:25 Guennadi Liakhovetski wrote: Hi Hans On Fri, 5 Oct 2012, Hans Verkuil wrote: [snip] I think the soc_camera patches should be left out for now. I suspect that by adding core support for async i2c handling first, Ok, let's think, what this meacs - async I2C in media / V4L2 core. The main reason for our probing order problem is the master clock, typically supplied from the camera bridge to I2C subdevices, which we only want to start when necessary, i.e. when accessing the subdevice. And the subdevice driver needs that clock running during its .probe() to be able to access and verify or configure the hardware. Our current solution is to not register I2C subdevices from the platform data, as is usual for all I2C devices, but from the bridge driver and only after it has switched on the master clock. After the subdevice driver has completed its probing we switch the clock back off until the subdevice has to be activated, e.g. for video capture. Also important - when we want to unregister the bridge driver we just also unregister the I2C device. Now, to reverse the whole thing and to allow I2C devices be registered as usual - via platform data or OF, first of all we have to teach I2C subdevice drivers to recognise the too early situation and request deferred probing in such a case. Then it will be reprobed after each new successful probe or unregister on the system. After the bridge driver has successfully probed the subdevice driver will be re-probed and at that time it should succeed. Now, there is a problem here too: who should switch on and off the master clock? If we do it from the bridge driver, we could install an I2C bus-notifier, _before_ the subdevice driver is probed, i.e. upon the BUS_NOTIFY_BIND_DRIVER event we could turn on the clock. If subdevice probing was successful, we can then wait for the BUS_NOTIFY_BOUND_DRIVER event to switch the clock back off. BUT - if the subdevice fails probing? How do we find out about that and turn the clock back off? There is no notification event for that... Possible solutions: 1. timer - ugly and unreliable. 2. add a probing failed notifier event to the device core - would this be accepted? 3. let the subdevice turn the master clock on and off for the duration of probing. My vote goes for (3). Ideally this should be done using the generic clock framework. But can we really expect all drivers and platforms to switch to it quickly enough? If not, we need a V4L2-specific callback from subdevice drivers to bridge drivers to turn the clock on and off. That's what I've done temporarily in this patch series for soc-camera. Suppose we decide to do the same for V4L2 centrally - add call-backs. Then we can think what else we need to add to V4L2 to support asynchronous subdevice driver probing. I wonder, don't we have the necessary code already? V4L2 subdev drivers can have internal_ops with register/unregister ops. These are called by v4l2_device_register_subdev. This happens during the bridge driver's probe. Suppose the subdev's probe does not actually access the i2c device, but instead defers that to the register callback? The bridge driver will turn on the clock before calling v4l2_device_register_subdev to ensure that the register callback can access the i2c registers. The register callback will do any initialization and can return an error. In case of an error the i2c client is automatically unregistered as well. Yes, if v4l2_i2c_new_subdev_board() is used. This has been discussed several times before and always what I didn't like in this is, that I2C device probe() in this case succeeds without even trying to access the hardware. And think about DT. In this case we don't instantiate the I2C device, OF code does it for us. What do you do then? If you let probe() succeed, then you will have to somehow remember the subdevice to later match it against bridges... Yes, but you need that information anyway. The bridge still needs to call v4l2_device_register_subdev so it needs to know which subdevs are loaded. And can't it get that from DT as well? In my view you cannot do a proper initialization unless you have both the bridge driver and all subdev drivers loaded and instantiated. They need one another. So I am perfectly fine with letting the probe function do next to nothing and postponing that until register() is called. I2C and actual probing to check if it's the right device is a bad idea in general since you have no idea what a hardware access to an unknown i2c device will do. There are still some corner cases where that is needed, but I do not think
Re: [PATCH 05/14] media: add a V4L2 OF parser
On Mon, 8 Oct 2012, Hans Verkuil wrote: On Mon October 8 2012 16:30:53 Guennadi Liakhovetski wrote: On Mon, 8 Oct 2012, Hans Verkuil wrote: On Mon October 8 2012 14:23:25 Guennadi Liakhovetski wrote: Hi Hans On Fri, 5 Oct 2012, Hans Verkuil wrote: [snip] I think the soc_camera patches should be left out for now. I suspect that by adding core support for async i2c handling first, Ok, let's think, what this meacs - async I2C in media / V4L2 core. The main reason for our probing order problem is the master clock, typically supplied from the camera bridge to I2C subdevices, which we only want to start when necessary, i.e. when accessing the subdevice. And the subdevice driver needs that clock running during its .probe() to be able to access and verify or configure the hardware. Our current solution is to not register I2C subdevices from the platform data, as is usual for all I2C devices, but from the bridge driver and only after it has switched on the master clock. After the subdevice driver has completed its probing we switch the clock back off until the subdevice has to be activated, e.g. for video capture. Also important - when we want to unregister the bridge driver we just also unregister the I2C device. Now, to reverse the whole thing and to allow I2C devices be registered as usual - via platform data or OF, first of all we have to teach I2C subdevice drivers to recognise the too early situation and request deferred probing in such a case. Then it will be reprobed after each new successful probe or unregister on the system. After the bridge driver has successfully probed the subdevice driver will be re-probed and at that time it should succeed. Now, there is a problem here too: who should switch on and off the master clock? If we do it from the bridge driver, we could install an I2C bus-notifier, _before_ the subdevice driver is probed, i.e. upon the BUS_NOTIFY_BIND_DRIVER event we could turn on the clock. If subdevice probing was successful, we can then wait for the BUS_NOTIFY_BOUND_DRIVER event to switch the clock back off. BUT - if the subdevice fails probing? How do we find out about that and turn the clock back off? There is no notification event for that... Possible solutions: 1. timer - ugly and unreliable. 2. add a probing failed notifier event to the device core - would this be accepted? 3. let the subdevice turn the master clock on and off for the duration of probing. My vote goes for (3). Ideally this should be done using the generic clock framework. But can we really expect all drivers and platforms to switch to it quickly enough? If not, we need a V4L2-specific callback from subdevice drivers to bridge drivers to turn the clock on and off. That's what I've done temporarily in this patch series for soc-camera. Suppose we decide to do the same for V4L2 centrally - add call-backs. Then we can think what else we need to add to V4L2 to support asynchronous subdevice driver probing. I wonder, don't we have the necessary code already? V4L2 subdev drivers can have internal_ops with register/unregister ops. These are called by v4l2_device_register_subdev. This happens during the bridge driver's probe. Suppose the subdev's probe does not actually access the i2c device, but instead defers that to the register callback? The bridge driver will turn on the clock before calling v4l2_device_register_subdev to ensure that the register callback can access the i2c registers. The register callback will do any initialization and can return an error. In case of an error the i2c client is automatically unregistered as well. Yes, if v4l2_i2c_new_subdev_board() is used. This has been discussed several times before and always what I didn't like in this is, that I2C device probe() in this case succeeds without even trying to access the hardware. And think about DT. In this case we don't instantiate the I2C device, OF code does it for us. What do you do then? If you let probe() succeed, then you will have to somehow remember the subdevice to later match it against bridges... Yes, but you need that information anyway. The bridge still needs to call v4l2_device_register_subdev so it needs to know which subdevs are loaded. But how do you get the subdev pointer? With the notifier I get it from i2c_get_clientdata(client) and what do you do without it? How do you get to the client? And can't it get that from DT as well? No, I don't think there is a way to get a device pointer from a DT node. In my view you cannot do a proper initialization unless you have
Re: [PATCH 05/14] media: add a V4L2 OF parser
On Mon October 8 2012 17:15:53 Guennadi Liakhovetski wrote: On Mon, 8 Oct 2012, Hans Verkuil wrote: On Mon October 8 2012 16:30:53 Guennadi Liakhovetski wrote: On Mon, 8 Oct 2012, Hans Verkuil wrote: On Mon October 8 2012 14:23:25 Guennadi Liakhovetski wrote: Hi Hans On Fri, 5 Oct 2012, Hans Verkuil wrote: [snip] I think the soc_camera patches should be left out for now. I suspect that by adding core support for async i2c handling first, Ok, let's think, what this meacs - async I2C in media / V4L2 core. The main reason for our probing order problem is the master clock, typically supplied from the camera bridge to I2C subdevices, which we only want to start when necessary, i.e. when accessing the subdevice. And the subdevice driver needs that clock running during its .probe() to be able to access and verify or configure the hardware. Our current solution is to not register I2C subdevices from the platform data, as is usual for all I2C devices, but from the bridge driver and only after it has switched on the master clock. After the subdevice driver has completed its probing we switch the clock back off until the subdevice has to be activated, e.g. for video capture. Also important - when we want to unregister the bridge driver we just also unregister the I2C device. Now, to reverse the whole thing and to allow I2C devices be registered as usual - via platform data or OF, first of all we have to teach I2C subdevice drivers to recognise the too early situation and request deferred probing in such a case. Then it will be reprobed after each new successful probe or unregister on the system. After the bridge driver has successfully probed the subdevice driver will be re-probed and at that time it should succeed. Now, there is a problem here too: who should switch on and off the master clock? If we do it from the bridge driver, we could install an I2C bus-notifier, _before_ the subdevice driver is probed, i.e. upon the BUS_NOTIFY_BIND_DRIVER event we could turn on the clock. If subdevice probing was successful, we can then wait for the BUS_NOTIFY_BOUND_DRIVER event to switch the clock back off. BUT - if the subdevice fails probing? How do we find out about that and turn the clock back off? There is no notification event for that... Possible solutions: 1. timer - ugly and unreliable. 2. add a probing failed notifier event to the device core - would this be accepted? 3. let the subdevice turn the master clock on and off for the duration of probing. My vote goes for (3). Ideally this should be done using the generic clock framework. But can we really expect all drivers and platforms to switch to it quickly enough? If not, we need a V4L2-specific callback from subdevice drivers to bridge drivers to turn the clock on and off. That's what I've done temporarily in this patch series for soc-camera. Suppose we decide to do the same for V4L2 centrally - add call-backs. Then we can think what else we need to add to V4L2 to support asynchronous subdevice driver probing. I wonder, don't we have the necessary code already? V4L2 subdev drivers can have internal_ops with register/unregister ops. These are called by v4l2_device_register_subdev. This happens during the bridge driver's probe. Suppose the subdev's probe does not actually access the i2c device, but instead defers that to the register callback? The bridge driver will turn on the clock before calling v4l2_device_register_subdev to ensure that the register callback can access the i2c registers. The register callback will do any initialization and can return an error. In case of an error the i2c client is automatically unregistered as well. Yes, if v4l2_i2c_new_subdev_board() is used. This has been discussed several times before and always what I didn't like in this is, that I2C device probe() in this case succeeds without even trying to access the hardware. And think about DT. In this case we don't instantiate the I2C device, OF code does it for us. What do you do then? If you let probe() succeed, then you will have to somehow remember the subdevice to later match it against bridges... Yes, but you need that information anyway. The bridge still needs to call v4l2_device_register_subdev so it needs to know which subdevs are loaded. But how do you get the subdev pointer? With the notifier I get it from i2c_get_clientdata(client) and what do you do without it? How do
Re: [PATCH 05/14] media: add a V4L2 OF parser
On Mon, 8 Oct 2012, Hans Verkuil wrote: On Mon October 8 2012 17:15:53 Guennadi Liakhovetski wrote: On Mon, 8 Oct 2012, Hans Verkuil wrote: On Mon October 8 2012 16:30:53 Guennadi Liakhovetski wrote: On Mon, 8 Oct 2012, Hans Verkuil wrote: On Mon October 8 2012 14:23:25 Guennadi Liakhovetski wrote: Hi Hans On Fri, 5 Oct 2012, Hans Verkuil wrote: [snip] I think the soc_camera patches should be left out for now. I suspect that by adding core support for async i2c handling first, Ok, let's think, what this meacs - async I2C in media / V4L2 core. The main reason for our probing order problem is the master clock, typically supplied from the camera bridge to I2C subdevices, which we only want to start when necessary, i.e. when accessing the subdevice. And the subdevice driver needs that clock running during its .probe() to be able to access and verify or configure the hardware. Our current solution is to not register I2C subdevices from the platform data, as is usual for all I2C devices, but from the bridge driver and only after it has switched on the master clock. After the subdevice driver has completed its probing we switch the clock back off until the subdevice has to be activated, e.g. for video capture. Also important - when we want to unregister the bridge driver we just also unregister the I2C device. Now, to reverse the whole thing and to allow I2C devices be registered as usual - via platform data or OF, first of all we have to teach I2C subdevice drivers to recognise the too early situation and request deferred probing in such a case. Then it will be reprobed after each new successful probe or unregister on the system. After the bridge driver has successfully probed the subdevice driver will be re-probed and at that time it should succeed. Now, there is a problem here too: who should switch on and off the master clock? If we do it from the bridge driver, we could install an I2C bus-notifier, _before_ the subdevice driver is probed, i.e. upon the BUS_NOTIFY_BIND_DRIVER event we could turn on the clock. If subdevice probing was successful, we can then wait for the BUS_NOTIFY_BOUND_DRIVER event to switch the clock back off. BUT - if the subdevice fails probing? How do we find out about that and turn the clock back off? There is no notification event for that... Possible solutions: 1. timer - ugly and unreliable. 2. add a probing failed notifier event to the device core - would this be accepted? 3. let the subdevice turn the master clock on and off for the duration of probing. My vote goes for (3). Ideally this should be done using the generic clock framework. But can we really expect all drivers and platforms to switch to it quickly enough? If not, we need a V4L2-specific callback from subdevice drivers to bridge drivers to turn the clock on and off. That's what I've done temporarily in this patch series for soc-camera. Suppose we decide to do the same for V4L2 centrally - add call-backs. Then we can think what else we need to add to V4L2 to support asynchronous subdevice driver probing. I wonder, don't we have the necessary code already? V4L2 subdev drivers can have internal_ops with register/unregister ops. These are called by v4l2_device_register_subdev. This happens during the bridge driver's probe. Suppose the subdev's probe does not actually access the i2c device, but instead defers that to the register callback? The bridge driver will turn on the clock before calling v4l2_device_register_subdev to ensure that the register callback can access the i2c registers. The register callback will do any initialization and can return an error. In case of an error the i2c client is automatically unregistered as well. Yes, if v4l2_i2c_new_subdev_board() is used. This has been discussed several times before and always what I didn't like in this is, that I2C device probe() in this case succeeds without even trying to access the hardware. And think about DT. In this case we don't instantiate the I2C device, OF code does it for us. What do you do then? If you let probe() succeed, then you will have to somehow remember the subdevice to later match it against bridges... Yes, but you need that information anyway. The bridge still needs to call
Re: [PATCH 05/14] media: add a V4L2 OF parser
On Mon, 8 Oct 2012, Guennadi Liakhovetski wrote: On Mon, 8 Oct 2012, Hans Verkuil wrote: On Mon October 8 2012 17:15:53 Guennadi Liakhovetski wrote: [snip] No, I don't think there is a way to get a device pointer from a DT node. Not a device pointer, but the i2c bus and i2c address. With that information you can get the i2c_client, and with that you can get the subdev pointer. How? How can you get a client pointer from adapter and i2c device address? I haven't found anything suitable in i2c.h. Ok, you could use of_find_i2c_device_by_node(), but the second argument remains - if you need notifiers in one case, I don't think it makes sense to implement multiple methods. Thanks Guennadi If there is no way to get that information from the proposed V4L2 DT, then it needs to be modified since a bridge driver really needs to know which subdevs it has to register with the v4l2_device struct. That information is also board specific so it should be part of the DT. In my view you cannot do a proper initialization unless you have both the bridge driver and all subdev drivers loaded and instantiated. They need one another. So I am perfectly fine with letting the probe function do next to nothing and postponing that until register() is called. I2C and actual probing to check if it's the right device is a bad idea in general since you have no idea what a hardware access to an unknown i2c device will do. There are still some corner cases where that is needed, but I do not think that that is an issue here. It would simplify things a lot IMHO. Also note that the register() op will work with any device, not just i2c. That may be a useful property as well. And what if the subdevice device is not yet instantiated by OF by the time your bridge driver probes? That is where you still need to have a bus notifier mechanism. You have to be able to wait until all dependent drivers are loaded/instantiated, No, drivers are not the problem, as you say, you can load them. Devices are the problem. You don't know when they will be registered. or alternatively you have to be able to load them explicitly. But this should be relatively easy to implement in a generic manner. So, if you need notifiers anyway, why not use them in all cases instead of mixing multiple methods? Thanks Guennadi I still think this sucks (excuse my language), but I see no way around it as long as there is no explicit probe order one can rely on. Regards, Hans --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/14] media: add a V4L2 OF parser
On Friday 05 October 2012 12:58:21 Guennadi Liakhovetski wrote:
On Fri, 5 Oct 2012, Hans Verkuil wrote:
On Tue October 2 2012 12:13:20 Sylwester Nawrocki wrote:
On 10/02/2012 11:49 AM, Guennadi Liakhovetski wrote:
+ if (!of_property_read_u32_array(node, data-lanes, data_lanes,
+ ARRAY_SIZE(data_lanes))) {
+ int i;
+ for (i = 0; i ARRAY_SIZE(data_lanes); i++)
+ link-mipi_csi_2.data_lanes[i] = data_lanes[i];
It doesn't look like what we aimed for. The data-lanes array is
supposed to be of variable length, thus I don't think it can be
parsed like that. Or am I missing something ? I think we need more
something like below
(based on of_property_read_u32_array(), not tested):
Ok, you're right, that my version only was suitable for fixed-size
arrays, which wasn't our goal. But I don't think we should go down to
manually parsing property data. How about (tested;-))
data = of_find_property(node, data-lanes, NULL);
if (data) {
int i = 0;
const __be32 *lane = NULL;
do {
lane = of_prop_next_u32(data, lane,
data_lanes[i]);
} while (lane i++ ARRAY_SIZE(data_lanes));
link-mipi_csi_2.num_data_lanes = i;
while (i--)
link-mipi_csi_2.data_lanes[i] = data_lanes[i];
}
Yes, that looks neat and does what it is supposed to do. :) Thanks!
For now, I'll trust you it works ;)
With regards to the remaining patches, it looks a bit scary to me how
complicated it got, perhaps mostly because of requirement to reference
host devices from subdevs. And it seems to rely on the existing SoC
camera infrastructure, which might imply lot's of work need to be done
for non soc-camera drivers. But I'm going to take a closer look and
comment more on the details at the corresponding patches.
I have to say that I agree with Sylwester here. It seems awfully
complicated, but I can't really put my finger on the actual cause.
Well, which exactly part? The V4L2 OF part is quite simple.
It would be really interesting to see this implemented for a non-SoC
device and to compare the two.
Sure, volunteers? ;-) In principle, if I find time, I could try to convert
sh_vou, which is also interesting, because it's an output driver.
The OMAP3 ISP is on my to-do list, but that depends on the generic clock
availability on the OMAP3, so I have to wait.
One area that I do not yet completely understand is the i2c bus
notifications (or asynchronous loading or i2c modules).
I would have expected that using OF the i2c devices are still initialized
before the host/bridge driver is initialized. But I gather that's not the
case?
No, it's not. I'm not sure, whether it depends on the order of devices in
the .dts, but, I think, it's better to not have to mandate a certain order
and I also seem to have seen devices being registered in different order
with the same DT, but I'm not 100% sure about that.
If this deferred probing is a general problem, then I think we need a
general solution as well that's part of the v4l2 core.
That can be done, perhaps. But we can do it as a next step. As soon as
we're happy with the OF implementation as such, we can commit that,
possibly leaving soc-camera patches out for now, then we can think where
to put async I2C handling.
I agree that async I2C handling should have V4L2 core helpers, otherwise it's
going to be pretty complex for ISP drivers.
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/14] media: add a V4L2 OF parser
On Tue October 2 2012 12:13:20 Sylwester Nawrocki wrote:
Hi Guennadi,
On 10/02/2012 11:49 AM, Guennadi Liakhovetski wrote:
+ if (!of_property_read_u32_array(node, data-lanes, data_lanes,
+ ARRAY_SIZE(data_lanes))) {
+ int i;
+ for (i = 0; i ARRAY_SIZE(data_lanes); i++)
+ link-mipi_csi_2.data_lanes[i] = data_lanes[i];
It doesn't look like what we aimed for. The data-lanes array is supposed
to be of variable length, thus I don't think it can be parsed like that.
Or am I missing something ? I think we need more something like below
(based on of_property_read_u32_array(), not tested):
Ok, you're right, that my version only was suitable for fixed-size arrays,
which wasn't our goal. But I don't think we should go down to manually
parsing property data. How about (tested;-))
data = of_find_property(node, data-lanes, NULL);
if (data) {
int i = 0;
const __be32 *lane = NULL;
do {
lane = of_prop_next_u32(data, lane, data_lanes[i]);
} while (lane i++ ARRAY_SIZE(data_lanes));
link-mipi_csi_2.num_data_lanes = i;
while (i--)
link-mipi_csi_2.data_lanes[i] = data_lanes[i];
}
Yes, that looks neat and does what it is supposed to do. :) Thanks!
For now, I'll trust you it works ;)
With regards to the remaining patches, it looks a bit scary to me how
complicated it got, perhaps mostly because of requirement to reference
host devices from subdevs. And it seems to rely on the existing SoC
camera infrastructure, which might imply lot's of work need to be done
for non soc-camera drivers. But I'm going to take a closer look and
comment more on the details at the corresponding patches.
I have to say that I agree with Sylwester here. It seems awfully complicated,
but I can't really put my finger on the actual cause. It would be really
interesting to see this implemented for a non-SoC device and to compare the
two.
One area that I do not yet completely understand is the i2c bus notifications
(or asynchronous loading or i2c modules).
I would have expected that using OF the i2c devices are still initialized
before the host/bridge driver is initialized. But I gather that's not the
case?
If this deferred probing is a general problem, then I think we need a general
solution as well that's part of the v4l2 core.
Regards,
Hans
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/14] media: add a V4L2 OF parser
On Fri, 5 Oct 2012, Hans Verkuil wrote:
On Tue October 2 2012 12:13:20 Sylwester Nawrocki wrote:
Hi Guennadi,
On 10/02/2012 11:49 AM, Guennadi Liakhovetski wrote:
+ if (!of_property_read_u32_array(node, data-lanes, data_lanes,
+ ARRAY_SIZE(data_lanes))) {
+ int i;
+ for (i = 0; i ARRAY_SIZE(data_lanes); i++)
+ link-mipi_csi_2.data_lanes[i] = data_lanes[i];
It doesn't look like what we aimed for. The data-lanes array is supposed
to be of variable length, thus I don't think it can be parsed like that.
Or am I missing something ? I think we need more something like below
(based on of_property_read_u32_array(), not tested):
Ok, you're right, that my version only was suitable for fixed-size
arrays,
which wasn't our goal. But I don't think we should go down to manually
parsing property data. How about (tested;-))
data = of_find_property(node, data-lanes, NULL);
if (data) {
int i = 0;
const __be32 *lane = NULL;
do {
lane = of_prop_next_u32(data, lane, data_lanes[i]);
} while (lane i++ ARRAY_SIZE(data_lanes));
link-mipi_csi_2.num_data_lanes = i;
while (i--)
link-mipi_csi_2.data_lanes[i] = data_lanes[i];
}
Yes, that looks neat and does what it is supposed to do. :) Thanks!
For now, I'll trust you it works ;)
With regards to the remaining patches, it looks a bit scary to me how
complicated it got, perhaps mostly because of requirement to reference
host devices from subdevs. And it seems to rely on the existing SoC
camera infrastructure, which might imply lot's of work need to be done
for non soc-camera drivers. But I'm going to take a closer look and
comment more on the details at the corresponding patches.
I have to say that I agree with Sylwester here. It seems awfully complicated,
but I can't really put my finger on the actual cause.
Well, which exactly part? The V4L2 OF part is quite simple.
It would be really
interesting to see this implemented for a non-SoC device and to compare the
two.
Sure, volunteers? ;-) In principle, if I find time, I could try to convert
sh_vou, which is also interesting, because it's an output driver.
One area that I do not yet completely understand is the i2c bus notifications
(or asynchronous loading or i2c modules).
I would have expected that using OF the i2c devices are still initialized
before the host/bridge driver is initialized. But I gather that's not the
case?
No, it's not. I'm not sure, whether it depends on the order of devices in
the .dts, but, I think, it's better to not have to mandate a certain order
and I also seem to have seen devices being registered in different order
with the same DT, but I'm not 100% sure about that.
If this deferred probing is a general problem, then I think we need a general
solution as well that's part of the v4l2 core.
That can be done, perhaps. But we can do it as a next step. As soon as
we're happy with the OF implementation as such, we can commit that,
possibly leaving soc-camera patches out for now, then we can think where
to put async I2C handling.
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 linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/14] media: add a V4L2 OF parser
On Fri October 5 2012 12:58:21 Guennadi Liakhovetski wrote:
On Fri, 5 Oct 2012, Hans Verkuil wrote:
On Tue October 2 2012 12:13:20 Sylwester Nawrocki wrote:
Hi Guennadi,
On 10/02/2012 11:49 AM, Guennadi Liakhovetski wrote:
+ if (!of_property_read_u32_array(node, data-lanes, data_lanes,
+ ARRAY_SIZE(data_lanes))) {
+ int i;
+ for (i = 0; i ARRAY_SIZE(data_lanes); i++)
+ link-mipi_csi_2.data_lanes[i] = data_lanes[i];
It doesn't look like what we aimed for. The data-lanes array is
supposed
to be of variable length, thus I don't think it can be parsed like
that.
Or am I missing something ? I think we need more something like below
(based on of_property_read_u32_array(), not tested):
Ok, you're right, that my version only was suitable for fixed-size
arrays,
which wasn't our goal. But I don't think we should go down to manually
parsing property data. How about (tested;-))
data = of_find_property(node, data-lanes, NULL);
if (data) {
int i = 0;
const __be32 *lane = NULL;
do {
lane = of_prop_next_u32(data, lane,
data_lanes[i]);
} while (lane i++ ARRAY_SIZE(data_lanes));
link-mipi_csi_2.num_data_lanes = i;
while (i--)
link-mipi_csi_2.data_lanes[i] = data_lanes[i];
}
Yes, that looks neat and does what it is supposed to do. :) Thanks!
For now, I'll trust you it works ;)
With regards to the remaining patches, it looks a bit scary to me how
complicated it got, perhaps mostly because of requirement to reference
host devices from subdevs. And it seems to rely on the existing SoC
camera infrastructure, which might imply lot's of work need to be done
for non soc-camera drivers. But I'm going to take a closer look and
comment more on the details at the corresponding patches.
I have to say that I agree with Sylwester here. It seems awfully
complicated,
but I can't really put my finger on the actual cause.
Well, which exactly part? The V4L2 OF part is quite simple.
No, the soc_camera part. The V4L2 OF part looks OK. Sorry, I should have
mentioned that!
It would be really
interesting to see this implemented for a non-SoC device and to compare the
two.
Sure, volunteers? ;-) In principle, if I find time, I could try to convert
sh_vou, which is also interesting, because it's an output driver.
One area that I do not yet completely understand is the i2c bus
notifications
(or asynchronous loading or i2c modules).
I would have expected that using OF the i2c devices are still initialized
before the host/bridge driver is initialized. But I gather that's not the
case?
No, it's not. I'm not sure, whether it depends on the order of devices in
the .dts, but, I think, it's better to not have to mandate a certain order
and I also seem to have seen devices being registered in different order
with the same DT, but I'm not 100% sure about that.
If this deferred probing is a general problem, then I think we need a
general
solution as well that's part of the v4l2 core.
That can be done, perhaps. But we can do it as a next step. As soon as
we're happy with the OF implementation as such, we can commit that,
possibly leaving soc-camera patches out for now, then we can think where
to put async I2C handling.
It would be good to have a number of 'Reviewed-by's or 'Acked-by's for the
DT binding documentation at least before it is merged.
I think the soc_camera patches should be left out for now. I suspect that
by adding core support for async i2c handling first, the soc_camera patches
will become a lot easier to understand.
Regards,
Hans
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 linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/14] media: add a V4L2 OF parser
On Fri, 5 Oct 2012, Hans Verkuil wrote: On Fri October 5 2012 12:58:21 Guennadi Liakhovetski wrote: On Fri, 5 Oct 2012, Hans Verkuil wrote: [snip] One area that I do not yet completely understand is the i2c bus notifications (or asynchronous loading or i2c modules). I would have expected that using OF the i2c devices are still initialized before the host/bridge driver is initialized. But I gather that's not the case? No, it's not. I'm not sure, whether it depends on the order of devices in the .dts, but, I think, it's better to not have to mandate a certain order and I also seem to have seen devices being registered in different order with the same DT, but I'm not 100% sure about that. If this deferred probing is a general problem, then I think we need a general solution as well that's part of the v4l2 core. That can be done, perhaps. But we can do it as a next step. As soon as we're happy with the OF implementation as such, we can commit that, possibly leaving soc-camera patches out for now, then we can think where to put async I2C handling. It would be good to have a number of 'Reviewed-by's or 'Acked-by's for the DT binding documentation at least before it is merged. Definitely, I'm sure you'll be honoured to be the first one in the list;-) I think the soc_camera patches should be left out for now. I suspect that by adding core support for async i2c handling first, the soc_camera patches will become a lot easier to understand. Ok, we can do this. 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 linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/14] media: add a V4L2 OF parser
On 10/05/2012 12:58 PM, Guennadi Liakhovetski wrote: One area that I do not yet completely understand is the i2c bus notifications (or asynchronous loading of i2c modules). I would have expected that using OF the i2c devices are still initialized before the host/bridge driver is initialized. But I gather that's not the case? No, it's not. I'm not sure, whether it depends on the order of devices in the .dts, but, I think, it's better to not have to mandate a certain order and I also seem to have seen devices being registered in different order with the same DT, but I'm not 100% sure about that. The device instantiation (and initialization) order is not something that is supposed to be ensured by a specific device tree source layout, IIUC. The initialization sequence is probably something that is specific to a particular operating system. I checked the device tree compiler but couldn't find if it is free to reorder anything when converting from the human readable device tree to its binary representation. The deferred probing was introduced in Linux to resolve resource inter-dependencies in case of FDT based booting AFAIK. If this deferred probing is a general problem, then I think we need a general solution as well that's part of the v4l2 core. That can be done, perhaps. But we can do it as a next step. As soon as we're happy with the OF implementation as such, we can commit that, possibly leaving soc-camera patches out for now, then we can think where to put async I2C handling. I would really like to see more than one user until we add any core code. No that it couldn't be changed afterwards, but it would be nice to ensure the concepts are right and proven in real life. -- Regards, Sylwester -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/14] media: add a V4L2 OF parser
On Fri, Oct 05, 2012 at 08:30:59PM +0200, Sylwester Nawrocki wrote: The deferred probing was introduced in Linux to resolve resource inter-dependencies in case of FDT based booting AFAIK. It's completely unrelated to FDT, it's a general issue. There's no sane way to use hacks like link ordering to resolve boot time dependencies - start getting things like regulators connected to I2C or SPI controllers which may also use GPIOs for some signals and may be parents for other regulators and mapping out the dependency graph becomes unreasonably complicated. Deferred probing is designed to solve this problem by working things out dynamically at runtime. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/14] media: add a V4L2 OF parser
Hi Sylwester
On Mon, 1 Oct 2012, Sylwester Nawrocki wrote:
On 09/27/2012 04:07 PM, Guennadi Liakhovetski wrote:
Add a V4L2 OF parser, implementing bindings, documented in
Documentation/devicetree/bindings/media/v4l2.txt.
Signed-off-by: Guennadi Liakhovetskig.liakhovet...@gmx.de
---
drivers/media/v4l2-core/Makefile |3 +
drivers/media/v4l2-core/v4l2-of.c | 190
+
include/media/v4l2-of.h | 62
3 files changed, 255 insertions(+), 0 deletions(-)
create mode 100644 drivers/media/v4l2-core/v4l2-of.c
create mode 100644 include/media/v4l2-of.h
diff --git a/drivers/media/v4l2-core/Makefile
b/drivers/media/v4l2-core/Makefile
index c2d61d4..00f64d6 100644
--- a/drivers/media/v4l2-core/Makefile
+++ b/drivers/media/v4l2-core/Makefile
@@ -9,6 +9,9 @@ videodev-objs := v4l2-dev.o v4l2-ioctl.o
v4l2-device.o v4l2-fh.o \
ifeq ($(CONFIG_COMPAT),y)
videodev-objs += v4l2-compat-ioctl32.o
endif
+ifeq ($(CONFIG_OF),y)
+ videodev-objs += v4l2-of.o
+endif
obj-$(CONFIG_VIDEO_DEV) += videodev.o v4l2-int-device.o
obj-$(CONFIG_VIDEO_V4L2) += v4l2-common.o
diff --git a/drivers/media/v4l2-core/v4l2-of.c
b/drivers/media/v4l2-core/v4l2-of.c
new file mode 100644
index 000..f45d64b
--- /dev/null
+++ b/drivers/media/v4l2-core/v4l2-of.c
@@ -0,0 +1,190 @@
+/*
+ * V4L2 OF binding parsing library
+ *
+ * Copyright (C) 2012 Renesas Electronics Corp.
+ * Author: Guennadi Liakhovetskig.liakhovet...@gmx.de
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of version 2 of the GNU General Public License as
+ * published by the Free Software Foundation.
+ */
+#includelinux/kernel.h
+#includelinux/module.h
+#includelinux/of.h
+#includelinux/types.h
+
+#includemedia/v4l2-of.h
+
+/*
+ * All properties are optional. If none are found, we don't set any flags.
This
+ * means, the port has a static configuration and no properties have to be
+ * specified explicitly.
+ * If any properties are found, that identify the bus as parallel, and
+ * slave-mode isn't set, we set V4L2_MBUS_MASTER. Similarly, if we
recognise the
+ * bus as serial CSI-2 and clock-noncontinuous isn't set, we set the
+ * V4L2_MBUS_CSI2_CONTINUOUS_CLOCK flag.
+ * The caller should hold a reference to node.
+ */
+void v4l2_of_parse_link(const struct device_node *node,
+ struct v4l2_of_link *link)
+{
+ const struct device_node *port_node = of_get_parent(node);
+ int size;
+ unsigned int v;
+ u32 data_lanes[ARRAY_SIZE(link-mipi_csi_2.data_lanes)];
+ bool data_lanes_present;
+
+ memset(link, 0, sizeof(*link));
+
+ link-local_node = node;
+
+ /* Doesn't matter, whether the below two calls succeed */
+ of_property_read_u32(port_node, reg,link-port);
+ of_property_read_u32(node, reg,link-addr);
+
+ if (!of_property_read_u32(node, bus-width,v))
+ link-parallel.bus_width = v;
+
+ if (!of_property_read_u32(node, data-shift,v))
+ link-parallel.data_shift = v;
+
+ if (!of_property_read_u32(node, hsync-active,v))
+ link-mbus_flags |= v ? V4L2_MBUS_HSYNC_ACTIVE_HIGH :
+ V4L2_MBUS_HSYNC_ACTIVE_LOW;
+
+ if (!of_property_read_u32(node, vsync-active,v))
+ link-mbus_flags |= v ? V4L2_MBUS_VSYNC_ACTIVE_HIGH :
+ V4L2_MBUS_VSYNC_ACTIVE_LOW;
+
+ if (!of_property_read_u32(node, data-active,v))
+ link-mbus_flags |= v ? V4L2_MBUS_DATA_ACTIVE_HIGH :
+ V4L2_MBUS_DATA_ACTIVE_LOW;
+
+ if (!of_property_read_u32(node, pclk-sample,v))
+ link-mbus_flags |= v ? V4L2_MBUS_PCLK_SAMPLE_RISING :
+ V4L2_MBUS_PCLK_SAMPLE_FALLING;
+
+ if (!of_property_read_u32(node, field-even-active,v))
+ link-mbus_flags |= v ? V4L2_MBUS_FIELD_EVEN_HIGH :
+ V4L2_MBUS_FIELD_EVEN_LOW;
+
+ if (of_get_property(node, slave-mode,size))
+ link-mbus_flags |= V4L2_MBUS_SLAVE;
+
+ /* If any parallel-bus properties have been found, skip serial ones */
+ if (link-parallel.bus_width || link-parallel.data_shift ||
+ link-mbus_flags) {
+ /* Default parallel bus-master */
+ if (!(link-mbus_flags V4L2_MBUS_SLAVE))
+ link-mbus_flags |= V4L2_MBUS_MASTER;
+ return;
+ }
+
+ if (!of_property_read_u32(node, clock-lanes,v))
+ link-mipi_csi_2.clock_lane = v;
+
+ if (!of_property_read_u32_array(node, data-lanes, data_lanes,
+ ARRAY_SIZE(data_lanes))) {
+ int i;
+ for (i = 0; i ARRAY_SIZE(data_lanes); i++)
+ link-mipi_csi_2.data_lanes[i] = data_lanes[i];
It doesn't look like what we aimed for.
Re: [PATCH 05/14] media: add a V4L2 OF parser
Hi Guennadi,
On 10/02/2012 11:49 AM, Guennadi Liakhovetski wrote:
+ if (!of_property_read_u32_array(node, data-lanes, data_lanes,
+ ARRAY_SIZE(data_lanes))) {
+ int i;
+ for (i = 0; i ARRAY_SIZE(data_lanes); i++)
+ link-mipi_csi_2.data_lanes[i] = data_lanes[i];
It doesn't look like what we aimed for. The data-lanes array is supposed
to be of variable length, thus I don't think it can be parsed like that.
Or am I missing something ? I think we need more something like below
(based on of_property_read_u32_array(), not tested):
Ok, you're right, that my version only was suitable for fixed-size arrays,
which wasn't our goal. But I don't think we should go down to manually
parsing property data. How about (tested;-))
data = of_find_property(node, data-lanes, NULL);
if (data) {
int i = 0;
const __be32 *lane = NULL;
do {
lane = of_prop_next_u32(data, lane, data_lanes[i]);
} while (lane i++ ARRAY_SIZE(data_lanes));
link-mipi_csi_2.num_data_lanes = i;
while (i--)
link-mipi_csi_2.data_lanes[i] = data_lanes[i];
}
Yes, that looks neat and does what it is supposed to do. :) Thanks!
For now, I'll trust you it works ;)
With regards to the remaining patches, it looks a bit scary to me how
complicated it got, perhaps mostly because of requirement to reference
host devices from subdevs. And it seems to rely on the existing SoC
camera infrastructure, which might imply lot's of work need to be done
for non soc-camera drivers. But I'm going to take a closer look and
comment more on the details at the corresponding patches.
--
Regards,
Sylwester
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/14] media: add a V4L2 OF parser
On Tue, 2 Oct 2012, Sylwester Nawrocki wrote:
Hi Guennadi,
On 10/02/2012 11:49 AM, Guennadi Liakhovetski wrote:
+ if (!of_property_read_u32_array(node, data-lanes, data_lanes,
+ ARRAY_SIZE(data_lanes))) {
+ int i;
+ for (i = 0; i ARRAY_SIZE(data_lanes); i++)
+ link-mipi_csi_2.data_lanes[i] = data_lanes[i];
It doesn't look like what we aimed for. The data-lanes array is supposed
to be of variable length, thus I don't think it can be parsed like that.
Or am I missing something ? I think we need more something like below
(based on of_property_read_u32_array(), not tested):
Ok, you're right, that my version only was suitable for fixed-size arrays,
which wasn't our goal. But I don't think we should go down to manually
parsing property data. How about (tested;-))
data = of_find_property(node, data-lanes, NULL);
if (data) {
int i = 0;
const __be32 *lane = NULL;
do {
lane = of_prop_next_u32(data, lane, data_lanes[i]);
} while (lane i++ ARRAY_SIZE(data_lanes));
link-mipi_csi_2.num_data_lanes = i;
while (i--)
link-mipi_csi_2.data_lanes[i] = data_lanes[i];
}
Yes, that looks neat and does what it is supposed to do. :) Thanks!
For now, I'll trust you it works ;)
With regards to the remaining patches, it looks a bit scary to me how
complicated it got, perhaps mostly because of requirement to reference
host devices from subdevs.
If you mean uses of the v4l2_of_get_remote() function, then it's the other
way round: I'm accessing subdevices from bridges, which is also
understandable - you need the subdevice.
Or do you mean the need to turn the master clock on and off from the
subdevice driver? This shall be eliminated, once we switch to using the
generic clock framework.
And it seems to rely on the existing SoC
camera infrastructure, which might imply lot's of work need to be done
for non soc-camera drivers.
Sorry, what it is relying on soc-camera? The patch series consists of
several generic patches, which have nothing to do with soc-camera, and
patches, porting soc-camera to use OF with cameras. I think, complexity
with soc-camera is also higher, than what you'd need with specific single
bridge drivers, because it is generic.
A big part of the complexity is supporting deferred subdevice (sensor)
probing. Beginning with what most bridge drivers currently use - static
subdevice lists in platform data, for which they then register I2C
devices, it is natural to implement that in 2 steps: (1) support directly
registered I2C sensors from platform data, that request deferred probing
until the bridge driver has been probed and is ready to handle them; (2)
support I2C subdevices in DT. After this your bridge driver will support 3
(!) modes in which subdevices can be initialised. In principle you could
drop step (1) - supporting that isn't really required, but then the jump
to (2) will be larger.
Another part of the complexity is specific to soc-camera, it comes from
the way, how subdevices are represented in platform data - as platform
devices with a bus ID, used to link bridges and subdevices. With DT those
platform devices and bus ID have to be generated dynamically.
And you're right - soc-camera bridge drivers have the advantage, that the
soc-camera core has taken the most work on supporting DT on itself, so, DT
support will come to them at only a fraction of the cost;-)
Thanks
Guennadi
But I'm going to take a closer look and
comment more on the details at the corresponding patches.
--
Regards,
Sylwester
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/14] media: add a V4L2 OF parser
On 09/27/2012 04:07 PM, Guennadi Liakhovetski wrote:
Add a V4L2 OF parser, implementing bindings, documented in
Documentation/devicetree/bindings/media/v4l2.txt.
Signed-off-by: Guennadi Liakhovetskig.liakhovet...@gmx.de
---
drivers/media/v4l2-core/Makefile |3 +
drivers/media/v4l2-core/v4l2-of.c | 190
+
include/media/v4l2-of.h | 62
3 files changed, 255 insertions(+), 0 deletions(-)
create mode 100644 drivers/media/v4l2-core/v4l2-of.c
create mode 100644 include/media/v4l2-of.h
diff --git a/drivers/media/v4l2-core/Makefile
b/drivers/media/v4l2-core/Makefile
index c2d61d4..00f64d6 100644
--- a/drivers/media/v4l2-core/Makefile
+++ b/drivers/media/v4l2-core/Makefile
@@ -9,6 +9,9 @@ videodev-objs := v4l2-dev.o v4l2-ioctl.o v4l2-device.o
v4l2-fh.o \
ifeq ($(CONFIG_COMPAT),y)
videodev-objs += v4l2-compat-ioctl32.o
endif
+ifeq ($(CONFIG_OF),y)
+ videodev-objs += v4l2-of.o
+endif
obj-$(CONFIG_VIDEO_DEV) += videodev.o v4l2-int-device.o
obj-$(CONFIG_VIDEO_V4L2) += v4l2-common.o
diff --git a/drivers/media/v4l2-core/v4l2-of.c
b/drivers/media/v4l2-core/v4l2-of.c
new file mode 100644
index 000..f45d64b
--- /dev/null
+++ b/drivers/media/v4l2-core/v4l2-of.c
@@ -0,0 +1,190 @@
+/*
+ * V4L2 OF binding parsing library
+ *
+ * Copyright (C) 2012 Renesas Electronics Corp.
+ * Author: Guennadi Liakhovetskig.liakhovet...@gmx.de
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of version 2 of the GNU General Public License as
+ * published by the Free Software Foundation.
+ */
+#includelinux/kernel.h
+#includelinux/module.h
+#includelinux/of.h
+#includelinux/types.h
+
+#includemedia/v4l2-of.h
+
+/*
+ * All properties are optional. If none are found, we don't set any flags.
This
+ * means, the port has a static configuration and no properties have to be
+ * specified explicitly.
+ * If any properties are found, that identify the bus as parallel, and
+ * slave-mode isn't set, we set V4L2_MBUS_MASTER. Similarly, if we recognise
the
+ * bus as serial CSI-2 and clock-noncontinuous isn't set, we set the
+ * V4L2_MBUS_CSI2_CONTINUOUS_CLOCK flag.
+ * The caller should hold a reference to node.
+ */
+void v4l2_of_parse_link(const struct device_node *node,
+ struct v4l2_of_link *link)
+{
+ const struct device_node *port_node = of_get_parent(node);
+ int size;
+ unsigned int v;
+ u32 data_lanes[ARRAY_SIZE(link-mipi_csi_2.data_lanes)];
+ bool data_lanes_present;
+
+ memset(link, 0, sizeof(*link));
+
+ link-local_node = node;
+
+ /* Doesn't matter, whether the below two calls succeed */
+ of_property_read_u32(port_node, reg,link-port);
+ of_property_read_u32(node, reg,link-addr);
+
+ if (!of_property_read_u32(node, bus-width,v))
+ link-parallel.bus_width = v;
+
+ if (!of_property_read_u32(node, data-shift,v))
+ link-parallel.data_shift = v;
+
+ if (!of_property_read_u32(node, hsync-active,v))
+ link-mbus_flags |= v ? V4L2_MBUS_HSYNC_ACTIVE_HIGH :
+ V4L2_MBUS_HSYNC_ACTIVE_LOW;
+
+ if (!of_property_read_u32(node, vsync-active,v))
+ link-mbus_flags |= v ? V4L2_MBUS_VSYNC_ACTIVE_HIGH :
+ V4L2_MBUS_VSYNC_ACTIVE_LOW;
+
+ if (!of_property_read_u32(node, data-active,v))
+ link-mbus_flags |= v ? V4L2_MBUS_DATA_ACTIVE_HIGH :
+ V4L2_MBUS_DATA_ACTIVE_LOW;
+
+ if (!of_property_read_u32(node, pclk-sample,v))
+ link-mbus_flags |= v ? V4L2_MBUS_PCLK_SAMPLE_RISING :
+ V4L2_MBUS_PCLK_SAMPLE_FALLING;
+
+ if (!of_property_read_u32(node, field-even-active,v))
+ link-mbus_flags |= v ? V4L2_MBUS_FIELD_EVEN_HIGH :
+ V4L2_MBUS_FIELD_EVEN_LOW;
+
+ if (of_get_property(node, slave-mode,size))
+ link-mbus_flags |= V4L2_MBUS_SLAVE;
+
+ /* If any parallel-bus properties have been found, skip serial ones */
+ if (link-parallel.bus_width || link-parallel.data_shift ||
+ link-mbus_flags) {
+ /* Default parallel bus-master */
+ if (!(link-mbus_flags V4L2_MBUS_SLAVE))
+ link-mbus_flags |= V4L2_MBUS_MASTER;
+ return;
+ }
+
+ if (!of_property_read_u32(node, clock-lanes,v))
+ link-mipi_csi_2.clock_lane = v;
+
+ if (!of_property_read_u32_array(node, data-lanes, data_lanes,
+ ARRAY_SIZE(data_lanes))) {
+ int i;
+ for (i = 0; i ARRAY_SIZE(data_lanes); i++)
+ link-mipi_csi_2.data_lanes[i] = data_lanes[i];
It doesn't look like what we aimed for. The data-lanes array is supposed
to be of variable length, thus I don't think it can be parsed like
[PATCH 05/14] media: add a V4L2 OF parser
Add a V4L2 OF parser, implementing bindings, documented in
Documentation/devicetree/bindings/media/v4l2.txt.
Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de
---
drivers/media/v4l2-core/Makefile |3 +
drivers/media/v4l2-core/v4l2-of.c | 190 +
include/media/v4l2-of.h | 62
3 files changed, 255 insertions(+), 0 deletions(-)
create mode 100644 drivers/media/v4l2-core/v4l2-of.c
create mode 100644 include/media/v4l2-of.h
diff --git a/drivers/media/v4l2-core/Makefile b/drivers/media/v4l2-core/Makefile
index c2d61d4..00f64d6 100644
--- a/drivers/media/v4l2-core/Makefile
+++ b/drivers/media/v4l2-core/Makefile
@@ -9,6 +9,9 @@ videodev-objs := v4l2-dev.o v4l2-ioctl.o v4l2-device.o
v4l2-fh.o \
ifeq ($(CONFIG_COMPAT),y)
videodev-objs += v4l2-compat-ioctl32.o
endif
+ifeq ($(CONFIG_OF),y)
+ videodev-objs += v4l2-of.o
+endif
obj-$(CONFIG_VIDEO_DEV) += videodev.o v4l2-int-device.o
obj-$(CONFIG_VIDEO_V4L2) += v4l2-common.o
diff --git a/drivers/media/v4l2-core/v4l2-of.c
b/drivers/media/v4l2-core/v4l2-of.c
new file mode 100644
index 000..f45d64b
--- /dev/null
+++ b/drivers/media/v4l2-core/v4l2-of.c
@@ -0,0 +1,190 @@
+/*
+ * V4L2 OF binding parsing library
+ *
+ * Copyright (C) 2012 Renesas Electronics Corp.
+ * Author: Guennadi Liakhovetski g.liakhovet...@gmx.de
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of version 2 of the GNU General Public License as
+ * published by the Free Software Foundation.
+ */
+#include linux/kernel.h
+#include linux/module.h
+#include linux/of.h
+#include linux/types.h
+
+#include media/v4l2-of.h
+
+/*
+ * All properties are optional. If none are found, we don't set any flags. This
+ * means, the port has a static configuration and no properties have to be
+ * specified explicitly.
+ * If any properties are found, that identify the bus as parallel, and
+ * slave-mode isn't set, we set V4L2_MBUS_MASTER. Similarly, if we recognise
the
+ * bus as serial CSI-2 and clock-noncontinuous isn't set, we set the
+ * V4L2_MBUS_CSI2_CONTINUOUS_CLOCK flag.
+ * The caller should hold a reference to node.
+ */
+void v4l2_of_parse_link(const struct device_node *node,
+ struct v4l2_of_link *link)
+{
+ const struct device_node *port_node = of_get_parent(node);
+ int size;
+ unsigned int v;
+ u32 data_lanes[ARRAY_SIZE(link-mipi_csi_2.data_lanes)];
+ bool data_lanes_present;
+
+ memset(link, 0, sizeof(*link));
+
+ link-local_node = node;
+
+ /* Doesn't matter, whether the below two calls succeed */
+ of_property_read_u32(port_node, reg, link-port);
+ of_property_read_u32(node, reg, link-addr);
+
+ if (!of_property_read_u32(node, bus-width, v))
+ link-parallel.bus_width = v;
+
+ if (!of_property_read_u32(node, data-shift, v))
+ link-parallel.data_shift = v;
+
+ if (!of_property_read_u32(node, hsync-active, v))
+ link-mbus_flags |= v ? V4L2_MBUS_HSYNC_ACTIVE_HIGH :
+ V4L2_MBUS_HSYNC_ACTIVE_LOW;
+
+ if (!of_property_read_u32(node, vsync-active, v))
+ link-mbus_flags |= v ? V4L2_MBUS_VSYNC_ACTIVE_HIGH :
+ V4L2_MBUS_VSYNC_ACTIVE_LOW;
+
+ if (!of_property_read_u32(node, data-active, v))
+ link-mbus_flags |= v ? V4L2_MBUS_DATA_ACTIVE_HIGH :
+ V4L2_MBUS_DATA_ACTIVE_LOW;
+
+ if (!of_property_read_u32(node, pclk-sample, v))
+ link-mbus_flags |= v ? V4L2_MBUS_PCLK_SAMPLE_RISING :
+ V4L2_MBUS_PCLK_SAMPLE_FALLING;
+
+ if (!of_property_read_u32(node, field-even-active, v))
+ link-mbus_flags |= v ? V4L2_MBUS_FIELD_EVEN_HIGH :
+ V4L2_MBUS_FIELD_EVEN_LOW;
+
+ if (of_get_property(node, slave-mode, size))
+ link-mbus_flags |= V4L2_MBUS_SLAVE;
+
+ /* If any parallel-bus properties have been found, skip serial ones */
+ if (link-parallel.bus_width || link-parallel.data_shift ||
+ link-mbus_flags) {
+ /* Default parallel bus-master */
+ if (!(link-mbus_flags V4L2_MBUS_SLAVE))
+ link-mbus_flags |= V4L2_MBUS_MASTER;
+ return;
+ }
+
+ if (!of_property_read_u32(node, clock-lanes, v))
+ link-mipi_csi_2.clock_lane = v;
+
+ if (!of_property_read_u32_array(node, data-lanes, data_lanes,
+ ARRAY_SIZE(data_lanes))) {
+ int i;
+ for (i = 0; i ARRAY_SIZE(data_lanes); i++)
+ link-mipi_csi_2.data_lanes[i] = data_lanes[i];
+ data_lanes_present = true;
+ } else {
+ data_lanes_present = false;
+ }
+
+ if (of_get_property(node, clock-noncontinuous, size))
+ link-mbus_flags |=
