Hi Mauro, On Tuesday 18 August 2015 07:06:36 Mauro Carvalho Chehab wrote: > Em Tue, 18 Aug 2015 09:35:14 +0300 Laurent Pinchart escreveu: > > On Friday 14 August 2015 12:54:44 Hans Verkuil wrote: [snip] > > I think this is becoming too complex. How about considering "deploy" as a > > commit instead ? There would then be no need to undeploy, any modification > > will start a new transaction that will be applied in one go when > > committed. This includes removal of entities by removing the corresponding > > directories. > > Agreed. I would implement just a /configfs/vimc/commit file, instead of > /configfs/vimc/vimc1/build_topology. > > any write to the "commit" configfs file will make all changes to all vimc > instances to be applied, or return an error if committing is not possible. Wouldn't it be better to have a commit file inside each vimc[0-9]+ directory ? vimc device instances are completely independent, I'd prefer having the configuration API operate per-instance as well. > A read to it would return a message saying if the changes were committed or > not. > > This way, an entire vimc instance could be removed with just: > > rm -rf /configfs/vimc/vimc1 > > As we won't have unremoved files there anymore. Some files will be automatically created by the kernel, such as the flags file in link directories, or the name file in entity directories. rm -rf might thus not work. That's a technical detail though, I haven't checked how configfs operates. > In order to remove a > group of objects: > rm -rf /configfs/vimc/vimc1/[files that belong to the group] > > The API also become simpler and clearer, IMHO. > > Btw, as we discussed at the userspace API RFC, if we end by having a new > type of graph object that represents a group of objects (MEDIA_ID_T_GROUP), Let's see about that when the userspace API will be agreed on. > that could be used, for example, to represent a project ARA hardware module, > it would be easier to remove an entire group by doing something like: > > rm -rf /configfs/vimc/vimc1/obj_group_1 [snip] > >> I misunderstood your original proposal, I thought the name of the > >> link_to_raw_capture_0 directory was interpreted by the driver to mean > >> that a link between the pad and pad 0 of the raw_capture entity should > >> be created. But you don't interpret the name at all. > >> > >> I think this is confusing. Wouldn't it be easier to interpret the name > >> of the link directory? I.e. it has to be of the form: link_to_<entity > >> name>_<pad name>. > > > > I'd rather use symlinks and no link directory at all, but then we'd have > > no place to specify link flags :-/ I believe that's the reason why a link > > directory is needed. > > > > Maybe I worry for no reason, but interpreting the name seems to me more > > error- prone than using a symlink inside the link directory. > > Yeah, using symlinks makes perfect sense to me, although I'm not so sure > of adding them inside the pads (/configfs/vimc/vimc0/sensor_a/pad_0/). > If we do that, we'll need to represent both links and backlinks, with > makes harder to remove them. I don't think we need to specify both, the forward link should be enough. > I would, instead, have a separate part of the configfs for the links: > > /configfs/vimc/vimc0/links > > and a link from sensor_a/pad_0 to raw_capture_1/pad_0/ would > be represented as: > > ../../sensor_a/pad_0 -> /configfs/vimc/vimc0/links/link0/source > ../../raw_capture_1/pad_0 -> /configfs/vimc/vimc0/links/link0/sink > > This way, if one wants to remove the link, he can do just: > > rm -rf /configfs/vimc/vimc0/links/link0 > > Also, the direction of the link is properly expressed by using the > names "source" and "sink" there. That's an interesting option. The drawback is that you can't easily see links in the configfs entities directory tree. I'll think about it. -- 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
