Hi Laurent,
On 07/05/2013 01:30 PM, Laurent Pinchart wrote:
On Thursday 04 July 2013 22:19:20 Sylwester Nawrocki wrote:
On 07/04/2013 01:13 PM, Hans Verkuil wrote:
On Thu 4 July 2013 00:49:36 Laurent Pinchart wrote:
On Thursday 27 June 2013 11:53:15 Sylwester Nawrocki wrote:
On 06/27/2013 08:43 AM, Hans Verkuil wrote:
On Wed June 26 2013 11:00:51 Sakari Ailus wrote:
On Tue, Jun 25, 2013 at 06:55:49PM +0200, Sylwester Nawrocki wrote:
On 06/24/2013 10:54 AM, Hans Verkuil wrote:
[snip]
Before we start messing with those drivers it would be nice to have
defined rules of the media entity naming. I2C bus number and address
is not something that's useful in the media entity name. And multiple
Isn't it?
Why not? As long as the format is strictly adhered to then I see no
reason not to use it. Not only does it make the name unique, it also
tells you where the device is in the hardware topology.
It's a shame that entities don't have a bus info field in addition to
their name, but we have to live with that.
Userspace needs a way to distinguish between multiple identical subdevs.
We can't rely on IDs only, as they're not guaranteed to be stable. We
thus need to use names and possibly connection information.
Two identical sensors connected to separate receivers could be
distinguished by checking which receiver they're connected to.
Unfortunately this breaks when the two sensors are connected to the same
receiver, in which case we can only rely on the name. Media entity names
thus need to be unique when connection information can't help
distinguishing otherwise identical subdevs, which implies that subdev
names must be unique.
We could make the simple rule that the driver name is the first word of
the name. So it would be easy to provide a function that matches just
the first word and ignores the bus info (if there is any).
Yes, and that's basically all I needed before "fixing" those affected
drivers. No matter what exact rules, if there are any, user space could
handle various hardware configurations without issues.
Besides, the drivers would need to strip/replace with something else any
spaces when initializing subddev name, as that character would be used
as the bus info delimiter ?
Or we could decide that the bus info can't contain any space, in which
case the last space would be the delimiter.
Sounds reasonable as well.
Frankly, I don't think either should contain a space :-) Today nobody is
using spaces anywhere to the best of my knowledge.
OK, then there would be spaces neither in<name> nor in<bus-info>. From
a quick grep I can't see any driver currently using spaces in its subdev
name.
In case of multi-subdev sensors (when the sensor includes a scaler for
instance) the subdev names will likely be made of the sensor name (or driver
name) and a subdev description. Something like "xxxxx pixel array" and "xxxxxx
scaler". We could use a dash or underscore to replace spaces though.
Yes, I guess dash or underscore could be well used instead of spaces.
But my feeling is that 32 characters might be often not enough to hold
longer names and bus info. Also it would be good to denote what sort of
bus we refer to, i2c, spi, usb, platform, etc. I doesn't look like we
can always fit that information in 32 characters.
[...]
How should bus info be retrieved if it's not part of the media entity
name ?
If that subdev name is also going to be used in the MC, then yes, it
should contain the i2c bus info. At the moment the v4l2 core makes no
assumptions on the subdev name, other than that it must be unique. which
is generally achieved by appending the i2c bus info. But some platform
subdevs (non-i2c) may not have any bus info since that doesn't apply in
all cases.
I would propose a guideline for the subdev naming like this:
<name> <bus-info>
where<bus-info> is optional and neither string contains spaces.
Hmm, it might be inconvenient for platform subdevs. E.g. it could mean
something like:
currently |<name> <bus-info>
----------------------+------------------------------------------
s5p-mipi-csis.0 | s5p-mipi-csis 11800000.csis
s5p-mipi-csis.1 | s5p-mipi-csis 11810000.csis
FIMC-LITE.0 | FIMC-LITE 12040000.fimc-lite
FIMC-LITE.0 | FIMC-LITE 12050000.fimc-lite
The register window addresses can vary across various SoCs and it doesn't
sound very clever to expose that to user space, when a device is exactly
same from the user point of view.
Presumably the ".<index>" part in the names in the above cases should be
kept, and user space could just ignore bus-info, e.g.
s5p-mipi-csis.0 | s5p-mipi-csis.0 11800000.csis
FIMC-LITE.0 | FIMC-LITE.0 12050000.fimc-lite
If the bus info is too long it would get truncated.
We're limited to 32 characters, which isn't much to store both the name and
bus info.
Indeed, it's a pretty serious limitation IMHO.
While we are at it, how about v4l2_i2c_subdev_init() ? It initializes
sd->name with SPI driver name. It doesn't look like it could be unique
then ?
Presumably we could have subdev name postfixed with I2C bus id/slave
address as it is done currently and the media core would be using
only a part of subdev's name up to ' ' character to initialize the
entity name ?
Yes, that's an option. But I would like Laurent's opinion on this. The
problem I see with that is that it would actually make it hard to map
an entity name to a subdev since there is no bus_info information
associated with the entity, just an ID.
Yes, without bus info in the entity structure this would likely not be a
good idea.
As explained above, userspace needs to know which entity corresponds to
which piece of hardware, so non-unique (in the context of a media
device, and when connection information doesn't provide the required
information) entity names are a bad idea in the general case.
So if you have two identical subdevs, e.g. "saa7115 6-0021" and
"saa7115 7-0021", and you name the corresponding entities "saa7115",
but with different IDs, then how do you know which ID maps to which
subdev? If you keep the i2c postfix, then that's unambiguous.
The I2C bus info in the subdev's name can be a completely random string.
Please note that I2C bus id can be assigned dynamically. So there is no
guarantee you get reproducible bus IDs assigned to each sensor in all
cases. That's said I2C bus info is not reliable means to identify
physical
device.
I'm afraid you're right :-) (I don't know whether I2C bus IDs will be
assigned dynamically in practice on systems where the information is
important though).
i2c devices on an embedded system (i.e. hooked up to the SoC i2c bus) will
always get the same bus number. Obviously, if the i2c device is on a
PCI(e) or USB board,
That has not always been true, before patch [1] most drivers used to
register I2C adapters with dynamically assigned IDs. Now there is a standard
way to specify the adapter's id in DT.
then it becomes dynamic (but still unique, and still it specifies exactly
where the device can be found in the hardware topology).
Presumably it allows to locate exactly a specific hardware device
indirectly, by e.g. parsing some additional data from sysfs. But it is not
very useful as an absolute identifier of a device.
Perhaps a sysfs link would have been a better way to expose the media
entity's underlying device, its placement in the hardware topology, etc. But
not all subdevs have struct device associated with them, not all have /dev
entry. Perhaps the entities could be listed in sysfs under corresponding
media device, with relevant bus information associated with them.
I'd rather not get started with the whole "media controller should have been
implemented in sysfs" discussion again :-)
Ok, I just wanted to point out some alternatives. ;-)
We need an ioctl to report additional information about media entities (it's
been on my to-do list for wayyyyyyyyy too long). It could be used to report
bus information as well.
Yes, that sounds much more interesting than using just subdev name to sqeeze
all the information in. Why we don't have such an ioctl yet anyway ? Were
there some arguments against it, or its been just a low priority issue ?
Regards,
Sylwester
--
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
