Messages in this thread | | | Subject | Re: v4.10-rc4 to v4.10-rc5: battery regression on Nokia N900 | From | Guenter Roeck <> | Date | Thu, 26 Jan 2017 22:27:32 -0800 |
| |
On 01/26/2017 07:39 PM, Zhang Rui wrote: > On Thu, 2017-01-26 at 18:03 -0800, Guenter Roeck wrote: >> On 01/26/2017 05:37 PM, Zhang Rui wrote: >>> >>> On Wed, 2017-01-25 at 13:09 +0100, Pali Rohár wrote: >>>> >>>> On Wednesday 25 January 2017 12:12:33 Pavel Machek wrote: >>>>> >>>>> >>>>> Hi! >>>>> >>>>>> >>>>>> >>>>>>> >>>>>>> >>>>>>>> >>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> Right. >>>>>>>>>> >>>>>>>>>> Before reverting, can you please try if this patch >>>>>>>>>> works >>>>>>>>>> or not? >>>>>>>>> Not really. Revert now. Sorry. >>>>>>>>> >>>>>>>>> Are you sure? This does not look equivalent to me at >>>>>>>>> all. >>>>>>>>> >>>>>>>>> "name" file handling moved from drivers to the core, >>>>>>>>> which >>>>>>>>> added some >>>>>>>>> crazy checks what name can contain. Even if this >>>>>>>>> "works", >>>>>>>>> what is the >>>>>>>>> expected effect on the "name" file? >>>>>>>>> >>>>>>>> The hwmon name attribute must not include '-', as >>>>>>>> documented >>>>>>>> in >>>>>>>> Documentation/hwmon/sysfs-interface. Is enforcing that >>>>>>>> 'crazy' ? >>>>>>>> Maybe in your world, but not in mine. >>>>>>> Well, lets revert the patch and then we can discuss what to >>>>>>> do >>>>>>> with >>>>>>> the "name" problem. >>>>> Ok, so the patch is on the way in. What to do next? >>>>> >>>>> pavel@n900:/sys/class/hwmon$ cat hwmon0/name >>>>> bq27200-0 >>>>> pavel@n900:/sys/class/hwmon$ cat hwmon1/name >>>>> rx51-battery >>>>> >>>>>> >>>>>> >>>>>> To provide some detail: libsensors gets just as confused with >>>>>> wildcards >>>>>> and whitespace/newline as it does with '-' in the reported >>>>>> name, >>>>>> which >>>>>> is why those are blocked by the new API. >>>>> Ok... Question is "does someone actually use hwmon*/name on >>>>> N900"? >>>>> If >>>>> so, we can't change it, but it is well possible that noone is. >>>> IIRC hwmon is used on Nokia N900. >>>> >>>> But I have not seen hwmon devices for bq27200 and rx51-battery >>>> yet. >>>> Those are power supply driver and auto-exporting them also via >>>> hwmon >>>> is >>>> something new, right? If yes, then we can use any name for those >>>> new >>>> hwmon devices as they cannot break userspace... as there is no >>>> userspace >>>> application for them. >>>> >>> If this is the case, you'd better set >>> (struct thermal_zone_params)->no_hwmon when registering the thermal >>> zone device, in which case, the hwmon device will not be created. >>> >>> In fact, I'd prefer to change tzp->no_hwmon to tzp->hwmon to not >>> create >>> hwmon I/F by default, and see if there is anyone using it. If yes, >>> we >>> can set the flag in soc thermal driver, explicitly, at meantime, a >>> hwmon compatible name is required. >>> >>> But one foreseeable result is that we may get bug reports from end >>> user >>> that some sensors (acpitz, etc) are gone in 'sensors' output. And >>> TBH, >>> I'm not quite sure if this can be counted as a regression or not. >>> >> That sounds like fun. Changing bq27200-0 to bq27200_0 is Forbidden by >> the ABI Police, but taking the entire device away is ok. >> > No. IMO, it depends on if the interface is used or not. > If hwmon I/F is used, we can not take it away, nor change its name.
Even if the use doesn't depend on that name ?
> If thermal zone I/F is used, we can not change it's 'type' name to be > compatible with new hwmon API. >
You mean you can not fix the name to be compatible with libsensors.
Makes me wonder if there shouldn't be a rule that exploits must not be fixed if already exploited.
Guenter
>> Anyway, sounds good to me. No one will use something that isn't >> there, >> and no one will realize that it could have been there, so I don't >> expect >> anyone to complain. > > Yes, I agree. > > thanks, > rui >
| |