On Wed, Jun 29, 2011 at 09:28:06PM +0200, Guennadi Liakhovetski wrote: > On Wed, 29 Jun 2011, Sakari Ailus wrote: > > > Guennadi Liakhovetski wrote: > > > On Mon, 27 Jun 2011, Guennadi Liakhovetski wrote: > > > > > > [snip] > > > > > >>> If the structures are expected to be generic I somehow feel that a field of > > >>> flags isn't the best way to describe the configuration of CSI-2 or other > > >>> busses. Why not to just use a structure with bus type and an union for > > >>> bus-specific configuration parameters? It'd be easier to access and also to > > >>> change as needed than flags in an unsigned long field. > > >> > > >> Well, yes, a union can be a good idea, thanks. > > > > > > ...on a second thought, we currently only have one field: flags, and it is > > > common for all 3 bus types: parallel, reduced parallel (bt.656, etc.), and > > > CSI-2. In the future, when we need more parameters for any of these busses > > > we'll just add such a union, shouldn't be a problem. > > > > What I meant above was that I would prefer to describe the capabilities > > in a structure which would contain appropriate data type for the field, > > not as flags or sets of flags in a bit field. > > > > This would allow e.g. just testing for > > v4l2_mbus_config.u.parallel.hsync_active_low instead of > > v4l2_mbus_config.flags & V4L2_MBUS_HSYNC_ACTIVE_LOW. This way the flags > > used for the bus also express the bus explicitly rather than implicitly. > > I don't think, using flags is any less explicit, than using struct > members. As I already explained, I'd like to use the same interface for > querying capabilities, the present configuration, and setting a > configuration. This is easily achieved with flags. See the > v4l2_mbus_config_compatible() function and try to rewrite all the bitwise > operations in it, using your struct. I still don't like the use of flags this way. I do admit that v4l2_mbus_config_compatible() might be more difficult to write. It's still a tiny piece of code. Speaking of which, should it be part of the latest patch? As far as I understand, v4l2_mbus_config_compatible() is meant for SoC camera compatibility only. I would like that the interface which is not SoC camera dependent would not get compatibility burden right from the beginning. That said, it's a nice property that e.g. sensor drivers written for SoC camera would work without it as well --- is this what the patch would achieve, or a part of it? Regards, -- Sakari Ailus sakari.ailus@xxxxxx -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
