Messages in this thread |  | | From | Wei Wang <> | Date | Mon, 16 Dec 2019 09:37:50 -0800 | Subject | Re: [PATCH v3 0/3] thermal: introduce by-name softlink |
| |
On Sun, Dec 15, 2019 at 8:34 AM Sandeep Patil <sspatil@android.com> wrote: > > Hi Wei, > > (resending because I accidentally replied only to you. Also adding everyone > else back in the thread, not sure what happened before). > > On Wed, Dec 11, 2019 at 02:19:03PM -0800, Wei Wang wrote: > > On Wed, Dec 11, 2019 at 1:11 PM Daniel Lezcano > > <daniel.lezcano@linaro.org> wrote: > > > > > > On 11/12/2019 21:11, Wei Wang wrote: > > > > On Wed, Dec 11, 2019 at 12:54 AM Greg Kroah-Hartman > > > > <gregkh@linuxfoundation.org> wrote: > > > >> > > > >> On Tue, Dec 10, 2019 at 09:54:11PM +0100, Daniel Lezcano wrote: > > > >>> On 10/12/2019 21:01, Wei Wang wrote: > > > >>>> On Tue, Dec 10, 2019 at 6:36 AM Daniel Lezcano > > > >>>> <daniel.lezcano@linaro.org> wrote: > > > >>>>> > > > >>>>> On 05/12/2019 08:19, Wei Wang wrote: > > > >>>>>> The paths thermal_zone%d and cooling_device%d are not intuitive and the > > > >>>>>> numbers are subject to change due to device tree change. This usually > > > >>>>>> leads to tree traversal in userspace code. > > > >> > > > >> tree traversal is supposed to be done in userspace code :) > > > >> > > > > Yes, that can be done in userspace, but given the amount of thermal > > > > zones we have in some mobile devices, this will bring a lot of > > > > convenience. > > > > > > But usually all these thermal zones are fixed and building the name<->tz > > > association is just a question of a few lines of code from userspace, > > > no? What would be the benefit of adding more ABI? > > > > > > code reuse as tz mapping is per device tree setup and we try to use > > the same _type_ in thermal zone in shared client code for Android > > thermal API. > > CMIIW, but the tz mapping today can still be done by parsing through the > directory the other way? (i.e. tz->type) instead of type->tz? > Parsing can be done in clients, one particular place is for init, but still possible. https://cs.android.com/search?q=tz-by-name%20file:%5C.rc Yeah, as said, we can definitely do pasing in every userspace daemon, but AFAICT, all userspace codes (open source / closed source) are using thermal zones / cooling devices by "name/type". So I am probling the possibility of having this kind of support for convenience. FYI, Chrome also feel this is useful https://lore.kernel.org/lkml/20191206191545.GO228856@google.com/
> Also, I understand the resistance to add a new ABI since it will have to be > kept around forever. What would be worth knowing is if these mappings are > static (built once) or you have to go through and parse the directories every > time? AFAIU, the parsing (map creation) seems to only happen once during > boot, so we (Android) should be ok? > Those mapping are usually static nowadays, but could be changed if modules are unloaded. Right now we only do UEVENT for userspace governor, https://github.com/torvalds/linux/blob/9c7db5004280767566e91a33445bf93aa479ef02/drivers/thermal/user_space.c#L37 Actually, I am about sending another RFC to add some more uevent to userspace. Without the UEVENT, I think we cannot guarantee the mapping is static.
> > > > > > > > If there is a thermal zone change, then a notification can be used, so > > > the userspace code rebuild the name<->tz, no? > > > > > > Assuming you meant for type<->tz, this mapping won't change in runtime > > in our case (not unloading things), but it could be changed due to > > device tree change, e.g. adding/removing thermal zones. > > I actually think we might need this after all. This assumes all tz devices > are registered when userspace (thermal hal/daemon) is coming up. However, > that may soon not be the case and we have to adjust for devices being added > in parallel with userspace? > > So, may be we need to start monitoring uevents like we do for power supplies > too? In that case, the mapping can also be done dynamically. > > Lastly, I think we can make a case further if there are benefits we can share > here other than convenience. Like startup time, handing hardware variations > etc? (Also point to the current userspace code that uses this) > I replied above before reading this part, and I guess we had the same thought on this.
> - ssp > > > > > > > > > > > e.g. this is on Pixel 4 XL: > > > > coral:/ # ls /sys/devices/virtual/thermal/ > > > > cdev-by-name cooling_device15 cooling_device22 cooling_device3 > > > > cooling_device9 thermal_zone15 thermal_zone22 thermal_zone3 > > > > thermal_zone37 thermal_zone44 thermal_zone51 thermal_zone59 > > > > thermal_zone66 thermal_zone73 thermal_zone80 thermal_zone88 > > > > cooling_device0 cooling_device16 cooling_device23 cooling_device30 > > > > thermal_zone0 thermal_zone16 thermal_zone23 thermal_zone30 > > > > thermal_zone38 thermal_zone45 thermal_zone52 thermal_zone6 > > > > thermal_zone67 thermal_zone74 thermal_zone81 thermal_zone9 > > > > cooling_device1 cooling_device17 cooling_device24 cooling_device31 > > > > thermal_zone1 thermal_zone17 thermal_zone24 thermal_zone31 > > > > thermal_zone39 thermal_zone46 thermal_zone53 thermal_zone60 > > > > thermal_zone68 thermal_zone75 thermal_zone82 tz-by-name > > > > cooling_device10 cooling_device18 cooling_device25 cooling_device4 > > > > thermal_zone10 thermal_zone18 thermal_zone25 thermal_zone32 > > > > thermal_zone4 thermal_zone47 thermal_zone54 thermal_zone61 > > > > thermal_zone69 thermal_zone76 thermal_zone83 > > > > cooling_device11 cooling_device19 cooling_device26 cooling_device5 > > > > thermal_zone11 thermal_zone19 thermal_zone26 thermal_zone33 > > > > thermal_zone40 thermal_zone48 thermal_zone55 thermal_zone62 > > > > thermal_zone7 thermal_zone77 thermal_zone84 > > > > cooling_device12 cooling_device2 cooling_device27 cooling_device6 > > > > thermal_zone12 thermal_zone2 thermal_zone27 thermal_zone34 > > > > thermal_zone41 thermal_zone49 thermal_zone56 thermal_zone63 > > > > thermal_zone70 thermal_zone78 thermal_zone85 > > > > cooling_device13 cooling_device20 cooling_device28 cooling_device7 > > > > thermal_zone13 thermal_zone20 thermal_zone28 thermal_zone35 > > > > thermal_zone42 thermal_zone5 thermal_zone57 thermal_zone64 > > > > thermal_zone71 thermal_zone79 thermal_zone86 > > > > cooling_device14 cooling_device21 cooling_device29 cooling_device8 > > > > thermal_zone14 thermal_zone21 thermal_zone29 thermal_zone36 > > > > thermal_zone43 thermal_zone50 thermal_zone58 thermal_zone65 > > > > thermal_zone72 thermal_zone8 thermal_zone87 > > > > > > > > > > > >> But what userspace code needs to do this, and for what? > > > > In Android, thermal daemon and thermal HAL as well as some init.rc > > > > script would use those thermal paths for managing and monitoring > > > > thermal. The daemon/HAL could have logic pipled in, however Android's > > > > init.rc script would be really tricky. > > > > On a related note, we also create /dev/block/by-name links from userspace. > > > > > > > > Thanks! > > > > -Wei > > > >> > > > >> thanks, > > > >> > > > >> greg k-h > > > > > > > > > -- > > > <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs > > > > > > Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | > > > <http://twitter.com/#!/linaroorg> Twitter | > > > <http://www.linaro.org/linaro-blog/> Blog > > >
|  |