On Wed, Jul 31, 2019 at 4:45 PM Stephen Boyd <swb...@chromium.org> wrote: > > Quoting Rafael J. Wysocki (2019-07-31 16:10:38) > > On Thu, Aug 1, 2019 at 12:59 AM Tri Vo <tr...@android.com> wrote: > > > > > > On Wed, Jul 31, 2019 at 3:42 PM Rafael J. Wysocki <r...@rjwysocki.net> > > > wrote: > > > > > > > > That's not my point (see below). > > > > > > > > > > > > + if (id < 0) > > > > > > > > + return id; > > > > > > > > + ws->id = id; > > > > > > > > + > > > > > > > > + dev = device_create_with_groups(wakeup_class, parent, > > > > > > > > MKDEV(0, 0), ws, > > > > > > > > + wakeup_source_groups, > > > > > > > > "ws%d", > > > > > > > > > > > > > > I thought the name was going to still be 'wakeupN'? > > > > > > > > > > > > So can't we prefix the wakeup source name with something like > > > > > > "wakeup:" or similar here? > > > > > > > > > > "ws%d" here is the name in the sysfs path rather than the name of the > > > > > wakeup source. Wakeup source name is not altered in this patch. > > > > > > > > > > > > > So why wouldn't something like this suffice: > > > > > > > > dev = device_create_with_groups(wakeup_class, parent, MKDEV(0, 0), ws, > > > > wakeup_source_groups, "wakeup:%s", > > > > ws->name); > > > > > > > > ? > > > > > > ws->name is inherited from the device name. IIUC device names are not > > > guaranteed to be unique. So if different devices with the same name > > > register wakeup sources, there is an error. > > > > OK > > > > So I guess the names are retained for backwards compatibility with > > existing user space that may be using them? > > > > That's kind of fair enough, but having two different identification > > schemes for wakeup sources will end up confusing. > > I understand your concern about the IDA now. Thanks for clarifying. > > How about we name the devices 'wakeupN' with the IDA when they're > registered with a non-NULL device pointer and then name them whatever > the name argument is when the device pointer is NULL. If we have this, > we should be able to drop the name attribute in sysfs and figure out the > name either by looking at the device name in /sys/class/wakeup/ if it > isn't 'wakeupN', or follow the symlink to the device in /sys/devices/ > and look at the parent device name there.
This makes it difficult for userspace to query the name a wakeup source, as it now has to first figure out if a wakeup source is associated with a device or not. The criteria for that is also awkward, userspase has to check if directory path contains "wakeupN", then it's a virtual wakeup source. IMO it's cleaner to consistently have /sys/class/wakeup/wakeupN/name for every wakeup source.
