Hi Sylwester, On Saturday 06 July 2013 22:50:32 Sylwester Nawrocki wrote: > 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 ? It has just been a low-priority issue. All we need here is a way for userspace to pass a buffer pointer to the kernel that will be filled with typed key-value pairs. I'm not sure whether a hierarchical structured format such as ASN.1 would be useful or if a flat hierarchy would be enough. -- Regards, Laurent Pinchart -- 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
