On 02/02/2017 05:31 PM, Petr Cvek wrote:
Dne 2.2.2017 v 15:23 Andrew F. Davis napsal(a):
On 02/01/2017 07:05 PM, Petr Cvek wrote:
Patch
w1: remove need for ida and use PLATFORM_DEVID_AUTO
098f9fb0c962eb2fdba5f9d34f4cf7a938237184
causes a regression in the w1_ds2760 driver. Initialization creates a name
"ds2760-battery.0.auto". It seems that name of that size will cause checking
code in __thermal_cooling_device_register() from thermal_core.c to fail as it checks for
names shorter than 20 chars:
if (type && strlen(type) >= THERMAL_NAME_LENGTH)
return ERR_PTR(-EINVAL);
A second problem seems to be with magician_supplicants list as
"ds2760-battery.0" does not match in a test power_supply_am_i_supplied() in
ds2760_battery_update_status() from drivers/power/supply/ds2760_battery.c . This causes
status flag to be forever in POWER_SUPPLY_STATUS_DISCHARGING state.
Functionality returned after I hotfix changed (drivers/w1/slaves/w1_ds2760.c)
pdev = platform_device_alloc("ds2760-battery", PLATFORM_DEVID_AUTO);
to
pdev = platform_device_alloc("ds2760-battery", 0);
Which solution do you advise. Shortening the name let's say just to "ds2760"
(with changes to arch files, w1 files and supply files), increasing THERMAL_NAME_LENGTH
to let's say 32 (I don't know if this doesn't break something other) or just reverting to
the old behavior?
If 20 is no longer the max thermal name then THERMAL_NAME_LENGTH should
be changed.
Yes it is that ".auto" addition to "ds2760-battery.0" makes 21 chars long string. With
higher number (".10" onwards) it is 22 chars. Load of the module then fails at test in
thermal_zone_device_register():
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/thermal/thermal_core.c?id=refs/tags/v4.10-rc6#n1158
if (type && strlen(type) >= THERMAL_NAME_LENGTH)
return ERR_PTR(-EINVAL);
I searched in code further and with THERMAL_NAME_LENGTH = 30 (enough space) the
module load fails at another place in __hwmon_device_register():
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/hwmon/hwmon.c?id=refs/tags/v4.10-rc6#n548
if (name && (!strlen(name) || strpbrk(name, "-* \t\n")))
return ERR_PTR(-EINVAL);
That's because there is "-" in "ds2760-battery.0.auto" (and that's why shortening to
"ds2760.0" works). Is there some reason for disallowing "-" in a name? If not then I'm for removal
(in other case we need to change ds2760-battery and others names). Increasing THERMAL_NAME_LENGTH to 22 should be just
OK for ds2760 (but maybe not enough for others) I would increased it to 32 (nice number ;-) ).
'-' is not permitted in hwmon name attributes, primarily because libsensors
interprets it as a delimiter in its configuration file.
Anyway, the related change in thermal has been reverted, so that should
no longer be an issue. Also, we'll change the hwmon code in 4.11 to no longer
return an error but to (only) issue a warning if an invalid name is provided.
Guenter
As for the couple boards hard-coding "ds2760-battery.0", this is a
problem that should be fix regardless of any fix we chose here as IDA is
not guaranteed to return 0, so the battery could just as easily be
called ds2760-battery.7. (I've had this discussion before as the N900
user-space also make this mistake).
I'm not familiar with power supply subsystem, so just a naive idea. Should suffice to change
drivers/power/supply/power_supply_core.c __power_supply_is_supplied_by() for only first part of the
string? (like "ds2760-battery" subset of "ds2760-battery.7").
Do you have a link to discussion about N900?
Petr
--
To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
