Charger smb1360: Increase timeout and current

[Jakko3]

There has been an issue report pmaports #1949 that the charging of device bq-paella with charger smb1360 stops after 3 hours.

This PR disables the charging timeout in the devicetree of the devices using this charger. And additionally increases the charging current of the charger (hard-coded charging current as a temporary solution). More details can be found in the commit messages.

WIP: The changes are currently tested.

[Jakko3]

Changed comment and implementation of "fastcharge current", as discussed further above.

[Jakko3]

Rebased to msm8916/6.4-rc4.

I'm not very familiar with GitHub, it looks a bit strange with that list of all those commits up above. I hope I've done it correctly.

[Jakko3]

Hm, nope, doesn't look like I've done the rebasing correctly. I'll try to fix this.

[Jakko3]

Ah, fixed that by changing the base in the GitHub user interface.

[Jakko3]

Build failed because "mixing declarations and code". Therefore I moved the declarations to the beginning of function smb1360_hw_init().

[Jakko3]

I removed the "draft" status and resolved the discussions so far.

We could wait a bit longer, maybe Random Modder can add some testing experience.

However, I think the current setting with "input current limit" at 800 mA and default AICL setting is the best compromise in the actual situation.

[stephan-gh]

I guess the charging timeout might also exist to continue charging a faulty battery forever. Should we really remove it completely? With the higher input current limit, how likely is it that you'd still discharge the device under heavy usage?

[Jakko3]

My impression was that the charging timeout is an alternative to the recharge cycle, which can be disabled. However, I can't think of a use case where this would be a good choice.

With a charging cable connected, stopping charging and switch to discharging is always a surprise to a user, I'd say.

As switching to (complete) discharging doesn't happen when reaching the recharge cycle, it would only happen if the device is floating between empty and full until the timeout kicks in. Setting the timeout to 13 hours, this is quite unlikely to happen. Hypothetically at weak power supply after e.g. 10 hours of notable computing load and thereafter 3 more hours of charging might not reach the recharge cycle. Although I can't think of a realistic scenario where this would happen. Watching movies at a long flight – but the passenger needs to leave the airplane at some point anyway. Using the phone as a navigation device in the car – but that shouldn't cause a computing load that draws as much power as even a weak power supply would provide. Yet possibly there are use scenarios that are beyond my imagination, I didn't want to risk a user running into an unexpecedly discharged battery in such extreme cases.

Another reason why I tended to disable the charging timeout was because there were some uncertainties about the behavior. One report was about switching to discharing after reaching 100 %. Another one was switching to discharging after several hours of recharging cycle. Both issues were neither understood nor could be reproduced. I wanted to avoid such things being caused by the charging timeout. However, the first issue should become unlikely when setting the timeout to 13 hours. The second one actually shouldn't happen, there is no obvious relation to the charging timeout.

• https://gitlab.com/postmarketOS/pmaports/-/issues/1586
• https://gitlab.com/postmarketOS/pmaports/-/issues/1949#note_1284020196

In the downstream devicetrees, some timeouts were set to 0 and some to 768 minutes. That doesn't help for the decision neither.

Another guideline could be the default setting of the chip. In the second code review discussion above I did a fastboot oem smb1360-reload and the dmesg dump thereafter shows the timeout setting before and after the driver applies its settings. The timeout is set in register 0x0a CFG_SFY_TIMER_CTRL_REG. The default value of that register seems to be 0x0a (00001010). After the driver probed, it was set to 0x2a (00101010). The SAFETY_TIME_DISABLE_BIT is BIT(5), this disable bit is not set by default (timeout enabled) and later got set by the testing driver as currently intended by this pull request (timeout disabled). The time is defined in bits[3:2], which are "10" in both cases. The possible times are 192, 384, 768, and 1536 minutes, so these bits at "10" are 768 minutes (approx. 13 hours). Therefore the default setting of the chip is to enable the timeout at 768 minutes.

I now also realize that the name of this feature in the register is actually "safety time" and not "charging timeout".

Summing up, it looks reasonable to set the "safety time" to 768 minutes. I'll change the pull request within the next days.

[Jakko3]

As discussed, I enabled the charging timeout, set it to 768 minutes (approx. 13 hours) and adapted the commit message.

[Jakko3]

From my point of view smb1360_hw_init() is the right place to do the initial setup. It's the function where the basic settings are processed. Also, &smb->delayed_init_work gets scheduled at the end of smb1360_probe() [1], after smb1360_hw_init() has been performed [2] and the power supply registred [3].

[1] https://github.com/msm8916-mainline/linux/blob/msm8916/6.4-rc4/drivers/power/supply/smb1360.c#L1729-L1730
[2] https://github.com/msm8916-mainline/linux/blob/msm8916/6.4-rc4/drivers/power/supply/smb1360.c#L1680-L1684
[3] https://github.com/msm8916-mainline/linux/blob/msm8916/6.4-rc4/drivers/power/supply/smb1360.c#L1708-L1714

On the USB_500 quirk: I couldn't confirm the issue on my device. I'm on smb1360 chip rev. 3, device bq-paella. I also tried fastboot oem smb1360-reload and USB_100 before going to USB_AC, haven't noticed anything strange. Therefore I would skip the quirk for now, not adding a quirk without knowing why. If it later turns out that under certain circumstances or on other chip revisions the input current limit gets stuck at a low level, we still can add the quirk later on.

The other suggestions I have implemented:

• Moved setting the fastcharge current into a separate function.
• Corrected multi-line comment style.
• Applied same order as downstream: input current limit, fastcharge current, USB speed bit.

I changed and extended the comments on the fastcharge currrent and hope it's ok that way. Otherwise let me know if you have suggestion for improvement.

Additionally, in the comments I changed wording "extcon cable type" to a more general "connector type detection", as you explained further above in the second collapsed review the diffiulties on detecting the extcon cable type and the need of implemeting it in the USB PHY layer. And in the commit message I added a more specific hint to USB PHY layer incl. the downstream example (for bq-paella).

[stephan-gh]

Integrated this into wip/msm8916/6.5-rc4, thanks again for making the changes :)