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 ;-) ).

> 
> 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

Reply via email to