linux-kernel - Re: v4.10-rc4 to v4.10-rc5: battery regression on Nokia N900

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <88c94ea6-abe2-0f20-337e-e9ee00c883d8@roeck-us.net>
Date:   Tue, 24 Jan 2017 06:18:26 -0800
From:   Guenter Roeck <linux@...ck-us.net>
To:     Pavel Machek <pavel@....cz>, Zhang Rui <rui.zhang@...el.com>
Cc:     pali.rohar@...il.com, sre@...nel.org,
        kernel list <linux-kernel@...r.kernel.org>,
        linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
        linux-omap@...r.kernel.org, tony@...mide.com, khilman@...nel.org,
        aaro.koskinen@....fi, ivo.g.dimitrov.75@...il.com,
        patrikbachan@...il.com, serge@...lyn.com, abcloriens@...il.com,
        fabio.estevam@....com
Subject: Re: v4.10-rc4 to v4.10-rc5: battery regression on Nokia N900

On 01/23/2017 11:37 PM, Pavel Machek wrote:
> On Tue 2017-01-24 15:06:39, Zhang Rui wrote:
>> On Mon, Jan 23, 2017 at 03:49:12PM -0800, Guenter Roeck wrote:
>>> On Tue, Jan 24, 2017 at 12:26:54AM +0100, Pavel Machek wrote:
>>>> Hi!
>>>>
>>>>>>> I'll try to revert it on the top of v4.10-rc5 now... and yes, it fixes
>>>>>>> the issue.
>>>>>>>
>>>>>>> Any idea what went wrong and how to fix that?
>>>>>>>
>>>>>>> Anyway as we are at -rc5 and this is warning fix that caused a
>>>>>>> regression on different hardware... it should be reverted.
>>>>>>>
>>>>>> Agreed.
>>>>>>
>>>>>> What exactly does "stopped working" mean ? That might help understanding
>>>>>> what went wrong.
>>>>>
>>>>> /sys files related to battery no longer appear. I beieve this has
>>>>> something to do with it:
>>>>>
>>>>> [    2.374877] of_get_named_gpiod_flags: parsed 'reset-gpios' property
>>>>> of node '/ocp@...00000/spi@...98000/tsc2005@0[0]' - status (0)
>>>>> [    2.375946] input: TSC2005 touchscreen as
>>>>> /devices/platform/68000000.ocp/48098000.spi/spi_master/spi1/spi1.0/input/input5
>>>>> [    2.392120] rx51-battery: probe of n900-battery failed with error
>>>>> -22
>>>>
>>>> Mystery solved:
>>>>
>>>> diff --git a/drivers/hwmon/hwmon.c b/drivers/hwmon/hwmon.c
>>>> index 3932f92..fe5ec82 100644
>>>> --- a/drivers/hwmon/hwmon.c
>>>> +++ b/drivers/hwmon/hwmon.c
>>>> @@ -545,8 +545,10 @@ __hwmon_device_register(struct device *dev, const char *name, void *drvdata,
>>>>  	int i, j, err, id;
>>>>
>>>>  	/* Do not accept invalid characters in hwmon name attribute */
>>>> -	if (name && (!strlen(name) || strpbrk(name, "-* \t\n")))
>>>> +	if (name && (!strlen(name) || strpbrk(name, "-* \t\n"))) {
>>>> +		printk("hwmon: Invalid character detected: %s\n", name);
>>>>  		return ERR_PTR(-EINVAL);
>>>> +	}
>>>>
>>>>  	id = ida_simple_get(&hwmon_ida, 0, 0, GFP_KERNEL);
>>>>  	if (id < 0)
>>>>
>>>>
>>>> pavel@...0:~$ dmesg | grep -5 Invalid
>>>> [    0.829650] of_get_named_gpiod_flags: parsed 'gpio-reset' property
>>>> of node '/ocp@...00000/i2c@...72000/tlv320aic3x@19[0]' - status (0)
>>>> [    0.833831] tsl2563 2-0029: model 7, rev. 0
>>>> [    0.837768] of_get_named_gpiod_flags: parsed 'enable-gpio' property
>>>> of node '/ocp@...00000/i2c@...72000/lp5523@32[0]' - status (0)
>>>> [    1.921417] omap_i2c 48072000.i2c: controller timed out
>>>> [    2.056823] lp5523x 2-0032: lp5523 Programmable led chip found
>>>> [    2.064147] hwmon: Invalid character detected: bq27200-0
>>>
>>> So the problem is really that the thermal driver needs to create a valid name.
>>>
>> 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.

Guenter