| 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
| ||
|
Message-ID: <a840aa80-75ef-4527-bc17-226ba5157a85@oss.qualcomm.com>
Date: Tue, 3 Jun 2025 15:41:12 +0800
From: Fenglin Wu <fenglin.wu@....qualcomm.com>
To: Krzysztof Kozlowski <krzk@...nel.org>, Sebastian Reichel
<sre@...nel.org>,
Bjorn Andersson <andersson@...nel.org>,
Konrad Dybcio <konradybcio@...nel.org>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley
<conor+dt@...nel.org>,
Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc: Subbaraman Narayanamurthy <subbaraman.narayanamurthy@....qualcomm.com>,
David Collins <david.collins@....qualcomm.com>,
linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-arm-msm@...r.kernel.org, kernel@....qualcomm.com,
devicetree@...r.kernel.org, linux-usb@...r.kernel.org
Subject: Re: [PATCH v2 6/8] dt-bindings: soc: qcom: pmic-glink: Move X1E80100
out of fallbacks
On 6/3/2025 3:06 PM, Krzysztof Kozlowski wrote:
> On 03/06/2025 08:59, Fenglin Wu wrote:
>> On 6/3/2025 2:47 PM, Krzysztof Kozlowski wrote:
>>> On 03/06/2025 08:42, Fenglin Wu wrote:
>>>> On 6/2/2025 3:40 PM, Krzysztof Kozlowski wrote:
>>>>> On 30/05/2025 09:35, Fenglin Wu via B4 Relay wrote:
>>>>>> From: Fenglin Wu <fenglin.wu@....qualcomm.com>
>>>>>>
>>>>>> Move X1E80100 out of the fallbacks of SM8550 in pmic-glink support.
>>>>> Why?
>>>>>
>>>>> Do not describe what you do here, it's obvious. We see it from the diff.
>>>>>
>>>>>
>>>>> Best regards,
>>>>> Krzysztof
>>>> Previously, in qcom_battmgr driver, x1e80100 was specified with a match
>>>> data the same as sc8280xp, also sm8550 was treated a fallback of sm8350
>>>> without the need of a match data.
>>>>
>>>> In ucsi_glink driver, sm8550 had a match data and x1e80100 was treated
>>>> as a fallback of sm8550. There was no issues to make x1e80100 as a
>>>> fallback of sm8550 from both qcom_battmgr and ucsi_glink driver perspective.
>>>>
>>>> In patch [5/8] in this series, in qcom_battmgr driver, it added charge
>>>> control functionality for sm8550 and x1e80100 differently hence
>>>> different match data was specified for them, and it makes x1e80100 ad
>>>> sm8550 incompatible and they need to be treated differently.
>>> So you break ABI and that's your problem to fix. You cannot make devices
>>> incompatible without good justification.
>> I would say x1e80100 and sm8550 are different and incompatible from a
>> battery management firmware support perspective. The x1e80100 follows
>> the sc8280xp as a compute platform, whereas the sm8550 follows the
>> sm8350 as a mobile platform.
> Not correct arguments for compatibility.
>
>> The difference between them was initially ignored because the sm8550
>> could use everything that the sm8350 has, and no match data needed to be
>> specified for it. However, now the sm8550 has new features that the
>> sm8350 doesn't have, requiring us to treat it differently, thus the
>> incompatibility was acknowledged.
> So they are perfectly compatible.
>
> I really do not understand what we are discussing here. Explain in
> simple terms of DT spec: what is incompatible that SW cannot use one
> interface to handle the other?
1. x1e80100 was a fallback of sc8280xp, it used "sc8280xp_bat_psy_desc"
when registering the power supply device.
2. sm8550 was a fallback of sm8350, and they all used
"sm8350_bat_psy_desc" when registering the power supply device.
3. x1e80100 and sm8550 they are incompatible as they are using different
data structure of "xxx_bat_psy_desc" and other “psy_desc" too, such as,
ac/usb/wls.
4. For charge control functionality, it's only supported in the battery
management firmware in x1e80100 and sm8550 platforms. And the change in
battmgr driver (patch [5/8]) adds the support by using 2 additional
power supply properties, which eventually need to be added in the
"properties" data member of "xxx_bat_psy_desc" when registering power
supply devices. Hence, "x1e80100_bat_psy_desc" and "sm8550_bat_psy_desc"
are created and used separately when registering power supply device
according to the "variant" value defined in the match data.
The main code change is in [5/8], I am pasting a snippet which might
help to explain this a little bit:
- if (battmgr->variant == QCOM_BATTMGR_SC8280XP) {
- battmgr->bat_psy = devm_power_supply_register(dev,
&sc8280xp_bat_psy_desc, &psy_cfg);
+ if (battmgr->variant == QCOM_BATTMGR_SC8280XP ||
battmgr->variant == QCOM_BATTMGR_X1E80100) {
+ if (battmgr->variant == QCOM_BATTMGR_X1E80100)
+ psy_desc = &x1e80100_bat_psy_desc;
+ else
+ psy_desc = &sc8280xp_bat_psy_desc;
+
+ battmgr->bat_psy = devm_power_supply_register(dev,
psy_desc, &psy_cfg);
if (IS_ERR(battmgr->bat_psy))
return dev_err_probe(dev,
PTR_ERR(battmgr->bat_psy),
"failed to register
battery power supply\n");
@@ -1394,7 +1628,12 @@ static int qcom_battmgr_probe(struct
auxiliary_device *adev,
return dev_err_probe(dev,
PTR_ERR(battmgr->wls_psy),
"failed to register
wireless charing power supply\n");
} else {
- battmgr->bat_psy = devm_power_supply_register(dev,
&sm8350_bat_psy_desc, &psy_cfg);
+ if (battmgr->variant == QCOM_BATTMGR_SM8550)
+ psy_desc = &sm8550_bat_psy_desc;
+ else
+ psy_desc = &sm8350_bat_psy_desc;
+
+ battmgr->bat_psy = devm_power_supply_register(dev,
psy_desc, &psy_cfg);
if (IS_ERR(battmgr->bat_psy))
return dev_err_probe(dev,
PTR_ERR(battmgr->bat_psy),
"failed to register
battery power supply\n");
>
> Best regards,
> Krzysztof
Powered by blists - more mailing lists