Now that the VIDIOC_DBG_G_CHIP_NAME ioctl has been added to the v4l2 API I
started work on removing the VIDIOC_DBG_G_CHIP_IDENT support in existing
drivers. Based on that effort I realized that there are a few things that
could be improved.
One thing that Laurent pointed out is that this ioctl should be available
only if CONFIG_VIDEO_ADV_DEBUG is set to prevent abuse by either userspace
or kernelspace. I agree with that, especially since g_chip_ident is being
abused today by some bridge drivers. That should be avoided in the future.
I am also unhappy with the name. G_CHIP_INFO would certainly be more descriptive,
but perhaps we should move a bit more into the direction of the Media Controller
and call it G_ENTITY_INFO. Opinions are welcome.
What surprised me when digging into the existing uses of G_CHIP_IDENT was that
there are more devices than expected that have multiple register blocks. I.e.
rather than a single set of registers they have multiple blocks of registers,
say one block at address 0x1000, another at 0x2000, etc.
Usually such register blocks represent IP blocks inside the chip, each doing
a specific task. In other cases (adv7604) each block corresponds to an i2c
address, each again representing an IP block inside the chip.
In the case of adv7604 it has been implementing by mapping register offsets
to specific i2c addresses, in the case of the cx231xx it has been implemented
by exposing different bridge chips, unfortunately that's done in such a way
that it can't be enumerated.
The existing debug API has no support for discovering such ranges, but having
worked with such a chip I think that having support for this is very desirable.
Since we added a new ioctl anyway, I thought that this is a good time to
extend it a bit and allow range discovery to be implemented:
/**
* struct v4l2_dbg_chip_name - VIDIOC_DBG_G_CHIP_NAME argument
* @match: which chip to match
* @flags: flags that tell whether this range is readable/writable
* @name: unique name of the chip
* @range_name: name of the register range
* @range_min: minimum register of the register range
* @range_max: maximum register of the register range
* @reserved: future extensions
*/
struct v4l2_dbg_chip_name {
struct v4l2_dbg_match match;
__u32 range;
__u32 flags;
char name[32];
char range_name[32];
__u64 range_start;
__u64 range_size;
__u32 reserved[8];
} __attribute__ ((packed));
range is the range index, range_name describes the purpose of the register
range, range_start and size are the start register address and the size of
this register range.
This extension allows you to enumerate the available register ranges for each
device. If there is only one range, then range_size may be 0. This is mostly
for backwards compatibility as otherwise I would have to modify all existing
drivers for this, and also because this is not really necessary for simple
devices with just one range. These are mostly i2c devices with start address
0 and a size of 256 bytes at most.
Comments?
Regards,
Hans
--
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
