On Thu, Dec 21, 2017 at 5:24 PM, Ognjen Galić <smclt30p@xxxxxxxxx> wrote: > On 21 Dec 2017 2:53 pm, "Andy Shevchenko" <andy.shevchenko@xxxxxxxxx> wrote: > On Thu, Dec 21, 2017 at 12:00 PM, Ognjen Galic <smclt30p@xxxxxxxxx> wrote: You need stop using HTML as well. >> +DEVICE_ATTR(charge_start_threshold, 0644, >> + tpacpi_battery_show, tpacpi_battery_store); >> +DEVICE_ATTR(charge_stop_threshold, 0644, >> + tpacpi_battery_show, tpacpi_battery_store); > > DEVICE_ATTR_RW(). > I did not use DEVICE_ATTR_RW() because I can't use a common store and show > function for both attributes to minimize the code I need. If I used > DEVICE_ATTR_RW() I would need to define 4 more functions that do the same. > That seems redundant. _RW() variant still preferable since it makes much more easy to understand that this attribute doesn't need any *special* permissions. I think it's not big payment to make it so. >> +#define to_power_supply(device) container_of(device, struct power_supply, >> dev) >> This one sounds to me as a separate change. > At the same time you may convert the current user of it to make sense > of the change. > drivers/power/supply/power_supply_core.c:671: > > I think I pointed to this out once. > I wanted to minimize the changes in pm to avoid going through all the review > process hassle for a few simple changes, because I'm modifying a third > subsystem. But I will do it later tonight. Thanks! > This is my first patchset and I did not expect for the review process to be > this agonizingly slow, so I wanted to speed it up by not touching pm. I see, though you actually made an opposite by that decision :-) -- With Best Regards, Andy Shevchenko ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot _______________________________________________ ibm-acpi-devel mailing list ibm-acpi-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel