From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from smtp-vbr8.xs4all.nl ([194.109.24.28]:1060 "EHLO smtp-vbr8.xs4all.nl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755117Ab0GHMeu (ORCPT ); Thu, 8 Jul 2010 08:34:50 -0400 Message-ID: In-Reply-To: <201007081401.53023.laurent.pinchart@ideasonboard.com> References: <1278503608-9126-1-git-send-email-laurent.pinchart@ideasonboard.com> <201007071453.22261.laurent.pinchart@ideasonboard.com> <72c0021199a577840a434d9195cb9e61.squirrel@webmail.xs4all.nl> <201007081401.53023.laurent.pinchart@ideasonboard.com> Date: Thu, 8 Jul 2010 14:34:46 +0200 Subject: Re: [RFC/PATCH 2/6] v4l: subdev: Add device node support From: "Hans Verkuil" To: "Laurent Pinchart" Cc: linux-media@vger.kernel.org, sakari.ailus@maxwell.research.nokia.com MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7BIT Sender: linux-media-owner@vger.kernel.org List-ID: > Hi Hans, > > On Wednesday 07 July 2010 15:04:17 Hans Verkuil wrote: >> > On Wednesday 07 July 2010 14:30:45 Hans Verkuil wrote: > > [snip] > >> > Some (most ?) I2C sensors need to be accessed during probe. This >> involves >> > powering the sensor up, which is handled by a board code function >> called >> > through a platform data function pointer. >> > >> > On the OMAP3 ISP the sensor clock can be generated by the ISP. This >> means >> > that board code needs to call an ISP (exported) function to turn the >> clock >> > on. To do so we currently retrieve a pointer to the ISP device through >> the >> > subdev's parent v4l2_device, embedded in the ISP device structure. >> This >> > requires the subdev to be registered, otherwise its parent will be >> NULL. >> > For that reason we're still using the core::s_config operation. >> > >> > This is obviously not an ideal situation, and I'm open to suggestions >> on >> > how to solve it without core::s_config. One possibility would be to >> use a >> > global ISP device pointer in the board code, as there's only one ISP >> > device in the system. It feels a bit hackish though. >> >> Or make the v4l2_device pointer part of the platform data? That way the >> subdev driver has access to the v4l2_device pointer in a fairly clean >> manner. > > As we've discussed on IRC, the platform data should really store hardware > properties, not software/driver information. Beside, storing the > v4l2_device > pointer in the platform data would be quite difficult, as > v4l2_device_register_subdev only sees a void * pointer for platform_data. > It > has no knowledge of the device-specific structure. I think my main problem is the use of s_config. This op was designed to pass irq and platform data to i2c drivers in pre-2.6.26 kernels. Looking at the current tree I see that the only driver using it is mt9v011.c. I think s_config should be removed and mt9v011.c/em28xx-cards.c be changed to use the platform_data directly. Currently s_config is used there to set the sensor xtal, which seems very much hardware specific to me. Instead of s_config I would probably add a 'registered' op that is called automatically by v4l2_device_register_subdev once the subdev has been registered successfully. Subdev drivers can then execute code that can only be run when registered. Later we might add an 'unregistered' op as well should we need it. This seems much more reasonable to me. Comments? Hans -- Hans Verkuil - video4linux developer - sponsored by TANDBERG, part of Cisco