Re: [RFC] Per-subdev, host-specific data
On Friday 23 July 2010 22:52:48 Andy Walls wrote: > On Fri, 2010-07-23 at 16:31 +0200, Laurent Pinchart wrote: > > Hi Hans, > > > > On Friday 23 July 2010 15:46:29 Hans Verkuil wrote: > > > On Friday 23 July 2010 15:01:29 Laurent Pinchart wrote: > > > > Hi everybody, > > > > > > > > Trying to implement support for multiple sensors connected to the same > > > > OMAP3 ISP input (all but one of the sensors need to be kept in reset > > > > obviously), I need to associate host-specific data to the sensor > > > > subdevs. > > > > > > > > The terms host and bridge are considered as synonyms in the rest of this > > > > e- mail. > > > > > > > > The OMAP3 ISP platform data has interface configuration parameters for > > > > the two CSI2 (a and c), CCP2 and parallel interfaces. The parameters are > > > > used to configure the bus when a sensor is selected. To support multiple > > > > sensors on the same input, the parameters need to be specified > > > > per-sensor, and not ISP- wide. > > > > > > > > No issue in the platform data. Board codes declare an array of > > > > structures > > > > that embed a struct v4l2_subdev_i2c_board_info instance and an OMAP3 > > > > ISP-specific interface configuration structure. > > > > > > > > At runtime, when a sensor is selected, I need to access the OMAP3 > > > > ISP-specific interface configuration structure for the selected sensor. > > > > At that point all I have is a v4l2_subdev structure pointer, without a > > > > way to get back to the interface configuration structure. > > > > > > > > The only point in the code where the v4l2_subdev and the interface > > > > configuration data are both known and could be linked together is in the > > > > host driver's probe function, where the v4l2_subdev instances are > > > > created. I have two solutions there: > > > > > > > > - store the v4l2_subdev pointer and the interface configuration data > > > > pointer in a host-specific array, and perform a an array lookup > > > > operation at runtime with the v4l2_subdev pointer as a key > > > > > > > > - add a void *host_priv field to the v4l2_subdev structure, store the > > > > interface configuration data pointer in that field, and use the field at > > > > runtime > > > > > > > > The second solution seems cleaner but requires an additional field in > > > > v4l2_subdev. Opinions and other comments will be appreciated. > > Why isn't > > v4l2_set_subdevdata(sd, private_ptr); > > sufficient? > > The cx18-av-core.[ch] files use that to get a bridge instance pointer > from a v4l2_subdev. > > Or is it that the v4l2_subdev infrastructure help functions for I2C > connected devices already claim that? Yes, they do. It is used to store the struct i2c_client pointer. Regards, Hans > > Regards, > Andy > > > > There is a third option: the grp_id field is owned by the host driver, so > > > you could make that an index into a host specific array. > > > > > Good point. > > > > > That said, I think having a host_priv field isn't a bad idea. Although if > > > we do this, then I think the existing priv field should be renamed to > > > drv_priv to prevent confusion. > > > > As Guennadi, Sakari and you all agree that the host_priv field is a good > > idea, > > I'll submit a patch. > > > > Thanks. > > > > > -- Hans Verkuil - video4linux developer - sponsored by TANDBERG, part of Cisco -- 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: [RFC] Per-subdev, host-specific data
On Fri, 2010-07-23 at 16:31 +0200, Laurent Pinchart wrote: > Hi Hans, > > On Friday 23 July 2010 15:46:29 Hans Verkuil wrote: > > On Friday 23 July 2010 15:01:29 Laurent Pinchart wrote: > > > Hi everybody, > > > > > > Trying to implement support for multiple sensors connected to the same > > > OMAP3 ISP input (all but one of the sensors need to be kept in reset > > > obviously), I need to associate host-specific data to the sensor > > > subdevs. > > > > > > The terms host and bridge are considered as synonyms in the rest of this > > > e- mail. > > > > > > The OMAP3 ISP platform data has interface configuration parameters for > > > the two CSI2 (a and c), CCP2 and parallel interfaces. The parameters are > > > used to configure the bus when a sensor is selected. To support multiple > > > sensors on the same input, the parameters need to be specified > > > per-sensor, and not ISP- wide. > > > > > > No issue in the platform data. Board codes declare an array of structures > > > that embed a struct v4l2_subdev_i2c_board_info instance and an OMAP3 > > > ISP-specific interface configuration structure. > > > > > > At runtime, when a sensor is selected, I need to access the OMAP3 > > > ISP-specific interface configuration structure for the selected sensor. > > > At that point all I have is a v4l2_subdev structure pointer, without a > > > way to get back to the interface configuration structure. > > > > > > The only point in the code where the v4l2_subdev and the interface > > > configuration data are both known and could be linked together is in the > > > host driver's probe function, where the v4l2_subdev instances are > > > created. I have two solutions there: > > > > > > - store the v4l2_subdev pointer and the interface configuration data > > > pointer in a host-specific array, and perform a an array lookup > > > operation at runtime with the v4l2_subdev pointer as a key > > > > > > - add a void *host_priv field to the v4l2_subdev structure, store the > > > interface configuration data pointer in that field, and use the field at > > > runtime > > > > > > The second solution seems cleaner but requires an additional field in > > > v4l2_subdev. Opinions and other comments will be appreciated. Why isn't v4l2_set_subdevdata(sd, private_ptr); sufficient? The cx18-av-core.[ch] files use that to get a bridge instance pointer from a v4l2_subdev. Or is it that the v4l2_subdev infrastructure help functions for I2C connected devices already claim that? Regards, Andy > > There is a third option: the grp_id field is owned by the host driver, so > > you could make that an index into a host specific array. > Good point. > > > That said, I think having a host_priv field isn't a bad idea. Although if > > we do this, then I think the existing priv field should be renamed to > > drv_priv to prevent confusion. > > As Guennadi, Sakari and you all agree that the host_priv field is a good > idea, > I'll submit a patch. > > Thanks. > -- 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: [RFC] Per-subdev, host-specific data
Hello, > -Original Message- > From: linux-media-ow...@vger.kernel.org [mailto:linux-media- > ow...@vger.kernel.org] On Behalf Of Laurent Pinchart > Sent: Friday, July 23, 2010 3:01 PM > To: Linux Media Mailing List > Cc: Sakari Ailus; Guennadi Liakhovetski > Subject: [RFC] Per-subdev, host-specific data > > Hi everybody, > > Trying to implement support for multiple sensors connected to the same > OMAP3 > ISP input (all but one of the sensors need to be kept in reset > obviously), I > need to associate host-specific data to the sensor subdevs. > > The terms host and bridge are considered as synonyms in the rest of > this e- > mail. > > The OMAP3 ISP platform data has interface configuration parameters for > the two > CSI2 (a and c), CCP2 and parallel interfaces. The parameters are used > to > configure the bus when a sensor is selected. To support multiple > sensors on > the same input, the parameters need to be specified per-sensor, and not > ISP- > wide. > > No issue in the platform data. Board codes declare an array of > structures that > embed a struct v4l2_subdev_i2c_board_info instance and an OMAP3 ISP- > specific > interface configuration structure. > > At runtime, when a sensor is selected, I need to access the OMAP3 ISP- > specific > interface configuration structure for the selected sensor. At that > point all I > have is a v4l2_subdev structure pointer, without a way to get back to > the > interface configuration structure. > > The only point in the code where the v4l2_subdev and the interface > configuration data are both known and could be linked together is in > the host > driver's probe function, where the v4l2_subdev instances are created. I > have > two solutions there: > > - store the v4l2_subdev pointer and the interface configuration data > pointer > in a host-specific array, and perform a an array lookup operation at > runtime > with the v4l2_subdev pointer as a key > > - add a void *host_priv field to the v4l2_subdev structure, store the > interface configuration data pointer in that field, and use the field > at > runtime > > The second solution seems cleaner but requires an additional field in > v4l2_subdev. Opinions and other comments will be appreciated. > I like the solution with an additional void *host_priv field, it could also possibly be useful for the notify() callback to v4l2_subdev parent. On our SoCs we also need some camera host interface specific data to be attached to image sensor subdevice and later passed to host driver. So host_priv field in v4l2_subdev would be nice feature to have. > -- > 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 Regards, -- Sylwester Nawrocki Samsung Poland R&D Center -- 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: [RFC] Per-subdev, host-specific data
Hi Hans, On Friday 23 July 2010 15:46:29 Hans Verkuil wrote: > On Friday 23 July 2010 15:01:29 Laurent Pinchart wrote: > > Hi everybody, > > > > Trying to implement support for multiple sensors connected to the same > > OMAP3 ISP input (all but one of the sensors need to be kept in reset > > obviously), I need to associate host-specific data to the sensor > > subdevs. > > > > The terms host and bridge are considered as synonyms in the rest of this > > e- mail. > > > > The OMAP3 ISP platform data has interface configuration parameters for > > the two CSI2 (a and c), CCP2 and parallel interfaces. The parameters are > > used to configure the bus when a sensor is selected. To support multiple > > sensors on the same input, the parameters need to be specified > > per-sensor, and not ISP- wide. > > > > No issue in the platform data. Board codes declare an array of structures > > that embed a struct v4l2_subdev_i2c_board_info instance and an OMAP3 > > ISP-specific interface configuration structure. > > > > At runtime, when a sensor is selected, I need to access the OMAP3 > > ISP-specific interface configuration structure for the selected sensor. > > At that point all I have is a v4l2_subdev structure pointer, without a > > way to get back to the interface configuration structure. > > > > The only point in the code where the v4l2_subdev and the interface > > configuration data are both known and could be linked together is in the > > host driver's probe function, where the v4l2_subdev instances are > > created. I have two solutions there: > > > > - store the v4l2_subdev pointer and the interface configuration data > > pointer in a host-specific array, and perform a an array lookup > > operation at runtime with the v4l2_subdev pointer as a key > > > > - add a void *host_priv field to the v4l2_subdev structure, store the > > interface configuration data pointer in that field, and use the field at > > runtime > > > > The second solution seems cleaner but requires an additional field in > > v4l2_subdev. Opinions and other comments will be appreciated. > > There is a third option: the grp_id field is owned by the host driver, so > you could make that an index into a host specific array. Good point. > That said, I think having a host_priv field isn't a bad idea. Although if > we do this, then I think the existing priv field should be renamed to > drv_priv to prevent confusion. As Guennadi, Sakari and you all agree that the host_priv field is a good idea, I'll submit a patch. Thanks. -- 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: [RFC] Per-subdev, host-specific data
On Friday 23 July 2010 15:01:29 Laurent Pinchart wrote: > Hi everybody, > > Trying to implement support for multiple sensors connected to the same OMAP3 > ISP input (all but one of the sensors need to be kept in reset obviously), I > need to associate host-specific data to the sensor subdevs. > > The terms host and bridge are considered as synonyms in the rest of this e- > mail. > > The OMAP3 ISP platform data has interface configuration parameters for the > two > CSI2 (a and c), CCP2 and parallel interfaces. The parameters are used to > configure the bus when a sensor is selected. To support multiple sensors on > the same input, the parameters need to be specified per-sensor, and not ISP- > wide. > > No issue in the platform data. Board codes declare an array of structures > that > embed a struct v4l2_subdev_i2c_board_info instance and an OMAP3 ISP-specific > interface configuration structure. > > At runtime, when a sensor is selected, I need to access the OMAP3 > ISP-specific > interface configuration structure for the selected sensor. At that point all > I > have is a v4l2_subdev structure pointer, without a way to get back to the > interface configuration structure. > > The only point in the code where the v4l2_subdev and the interface > configuration data are both known and could be linked together is in the host > driver's probe function, where the v4l2_subdev instances are created. I have > two solutions there: > > - store the v4l2_subdev pointer and the interface configuration data pointer > in a host-specific array, and perform a an array lookup operation at runtime > with the v4l2_subdev pointer as a key > > - add a void *host_priv field to the v4l2_subdev structure, store the > interface configuration data pointer in that field, and use the field at > runtime > > The second solution seems cleaner but requires an additional field in > v4l2_subdev. Opinions and other comments will be appreciated. There is a third option: the grp_id field is owned by the host driver, so you could make that an index into a host specific array. That said, I think having a host_priv field isn't a bad idea. Although if we do this, then I think the existing priv field should be renamed to drv_priv to prevent confusion. Regards, Hans -- Hans Verkuil - video4linux developer - sponsored by TANDBERG, part of Cisco -- 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