Mauro Carvalho Chehab wrote: > Em 01-07-2011 08:15, Sakari Ailus escreveu: >> On Fri, Jul 01, 2011 at 09:57:39AM +0200, Hans Verkuil wrote: >>> On Friday, July 01, 2011 03:27:10 Mauro Carvalho Chehab wrote: >>>> Em 10-06-2011 06:27, Sakari Ailus escreveu: >>>>> Hi Mauro, >>>>> >>>>> This pull request adds the bitmask controls, flash API and the adp1653 >>>>> driver. What has changed since the patches is: >>>>> >>>>> - Adp1653 flash faults control is volatile. Fix this. >>>>> - Flash interface marked as experimental. >>>>> - Moved the DocBook documentation to a new location. >>>>> - The target version is 3.1, not 2.6.41. >>>>> >>>>> The following changes since commit 75125b9d44456e0cf2d1fbb72ae33c13415299d1: >>>>> >>>>> [media] DocBook: Don't be noisy at make cleanmediadocs (2011-06-09 16:40:58 -0300) >>>>> >>>>> are available in the git repository at: >>>>> ssh://linuxtv.org/git/sailus/media_tree.git media-for-3.1 >>>>> >>>>> Hans Verkuil (3): >>>>> v4l2-ctrls: add new bitmask control type. >>>>> vivi: add bitmask test control. >>>>> DocBook: document V4L2_CTRL_TYPE_BITMASK. >>>> >>>> I'm sure I've already mentioned, but I think it was at the Hans pull request: >>>> the specs don't mention what endiannes is needed for the bitmask controls: >>>> machine endianess, little endian or big endian. IMO, we should stick with either >>>> LE or BE. >>> >>> Sorry Sakari, I should have fixed that. But since the patch was going through >>> your repository I forgot about it. Anyway, it should be machine endianess. You >>> have to be able to do (value & bit_define). The bit_defines for each bitmask >>> control should be part of the control's definition in videodev2.h. >>> >>> It makes no sense to require LE or BE. We don't do that for other control types, >>> so why should bitmask be any different? >>> >>> Can you add this clarification to DocBook? >> >> Thinking about this some more, if we're to say something about the >> endianness we should specify it for all controls, not just bitmasks. I >> really wonder if need this at all: why would you think the endianness in a >> bitmask would be some other than machine endianness, be it a V4L2 control or >> a flags field in, say, struct v4l2_buffer? It would make sense to document >> it if it differs from the norm, so in my opinion such statement would be >> redundant. > > Because someone might have the "bright" idea of exposing a hardware register via > V4L2_CTRL_TYPE_BITMASK directly without reminding about the endiannes, and do the > wrong thing. The same could be done to an integer control as well, the bitmask isn't special in that sense. Just my opinion. The spec says "When for example a hardware register accepts values 0-511 and the driver reports 0-65535, step should be 128." on step field in struct v4l2_queryctrl. This suggests that registers may be exposed to user space as integer controls. So endianness issues may exist on other types of controls as well if the programmer didn't remember that other endian than his existed. > There's not much problem on being redundant at the specs, but not specifying it > means that different implementations will still be valid. > > Btw, if you take a look at the descriptions for RGB format at the spec, you'll see > that endiannes problems already happened in the past: the specs had to explicitly > allow both endiannes for a few RGB formats due to that. > > I'm ok if we standardize that it should be the machine endiannes, but we should > be explicit on that. I'll add a few words on this, but should it be added to bitmask controls documentation or controls in general? 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
