2022-08-08 08:22:40

by Yassine Oudjana

[permalink] [raw]
Subject: [PATCH 0/8] power: supply: Add driver for Qualcomm SMBCHG

From: Yassine Oudjana <[email protected]>

This series adds a driver for the switch-mode battery charger found on PMICs
such as PMI8994, and referred to in the vendor kernel[1] as smbcharger or
SMBCHG. More details on this block can be found in the last patch message.

This driver currently supports the charger blocks of PMI8994 and PMI8996.
PMI8950 was also to be supported, but it was dropped due to some last minute
issues, to be brought back at a later time once ready.

The OTG regulator remains unused on devices where the charger is enabled in
this series due to lack of a consumer. Applying a patch[2] adding vbus-supply
to DWC3 allows it to enable the OTG regulator making USB host without
external power possible.

[1] https://github.com/android-linux-stable/msm-3.18/blob/kernel.lnx.3.18.r34-rel/drivers/power/qpnp-smbcharger.c
[2] https://lore.kernel.org/linux-usb/[email protected]/

Yassine Oudjana (8):
dt-bindings: power: supply: Add DT schema for Qualcomm SMBCHG
arm64: dts: qcom: pmi8994: Add SMBCHG
arm64: dts: qcom: pmi8996: Add SMBCHG
arm64: dts: qcom: msm8996-xiaomi-*: Add battery data
arm64: dts: qcom: msm8996-xiaomi-*: Enable SMBCHG
soc: qcom: Add PMIC secure register write helpers
util_macros.h: Add macro to find closest smaller value in array
power: supply: Add driver for Qualcomm SMBCHG

.../bindings/power/supply/qcom,smbchg.yaml | 205 ++
MAINTAINERS | 16 +
.../boot/dts/qcom/msm8996-xiaomi-common.dtsi | 15 +
.../boot/dts/qcom/msm8996-xiaomi-gemini.dts | 5 +
.../boot/dts/qcom/msm8996-xiaomi-natrium.dts | 5 +
.../boot/dts/qcom/msm8996-xiaomi-scorpio.dts | 5 +
arch/arm64/boot/dts/qcom/pmi8994.dtsi | 72 +
arch/arm64/boot/dts/qcom/pmi8996.dtsi | 4 +
drivers/power/supply/Kconfig | 11 +
drivers/power/supply/Makefile | 1 +
drivers/power/supply/qcom-smbchg.c | 1664 +++++++++++++++++
drivers/power/supply/qcom-smbchg.h | 428 +++++
drivers/soc/qcom/Kconfig | 4 +
drivers/soc/qcom/Makefile | 1 +
drivers/soc/qcom/pmic-sec-write.c | 82 +
include/linux/util_macros.h | 22 +
include/soc/qcom/pmic-sec-write.h | 9 +
17 files changed, 2549 insertions(+)
create mode 100644 Documentation/devicetree/bindings/power/supply/qcom,smbchg.yaml
create mode 100644 drivers/power/supply/qcom-smbchg.c
create mode 100644 drivers/power/supply/qcom-smbchg.h
create mode 100644 drivers/soc/qcom/pmic-sec-write.c
create mode 100644 include/soc/qcom/pmic-sec-write.h

--
2.37.1


2022-08-08 09:22:57

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 0/8] power: supply: Add driver for Qualcomm SMBCHG

On 08/08/2022 10:34, Yassine Oudjana wrote:
> From: Yassine Oudjana <[email protected]>
>
> This series adds a driver for the switch-mode battery charger found on PMICs
> such as PMI8994, and referred to in the vendor kernel[1] as smbcharger or
> SMBCHG. More details on this block can be found in the last patch message.
>
> This driver currently supports the charger blocks of PMI8994 and PMI8996.
> PMI8950 was also to be supported, but it was dropped due to some last minute
> issues, to be brought back at a later time once ready.
>
> The OTG regulator remains unused on devices where the charger is enabled in
> this series due to lack of a consumer. Applying a patch[2] adding vbus-supply
> to DWC3 allows it to enable the OTG regulator making USB host without
> external power possible.
>
> [1] https://github.com/android-linux-stable/msm-3.18/blob/kernel.lnx.3.18.r34-rel/drivers/power/qpnp-smbcharger.c
> [2] https://lore.kernel.org/linux-usb/[email protected]/

How is it different from PMI8998? I expect not that much, so this should
be based on existing work:
https://lore.kernel.org/linux-arm-msm/[email protected]/

Unless they are different, but then please create common parts and
explain the differences.

Best regards,
Krzysztof

2022-08-08 10:07:26

by Yassine Oudjana

[permalink] [raw]
Subject: Re: [PATCH 0/8] power: supply: Add driver for Qualcomm SMBCHG


On Mon, Aug 8 2022 at 11:41:26 +03:00:00, Krzysztof Kozlowski
<[email protected]> wrote:
> On 08/08/2022 10:34, Yassine Oudjana wrote:
>> From: Yassine Oudjana <[email protected]>
>>
>> This series adds a driver for the switch-mode battery charger found
>> on PMICs
>> such as PMI8994, and referred to in the vendor kernel[1] as
>> smbcharger or
>> SMBCHG. More details on this block can be found in the last patch
>> message.
>>
>> This driver currently supports the charger blocks of PMI8994 and
>> PMI8996.
>> PMI8950 was also to be supported, but it was dropped due to some
>> last minute
>> issues, to be brought back at a later time once ready.
>>
>> The OTG regulator remains unused on devices where the charger is
>> enabled in
>> this series due to lack of a consumer. Applying a patch[2] adding
>> vbus-supply
>> to DWC3 allows it to enable the OTG regulator making USB host
>> without
>> external power possible.
>>
>> [1]
>> https://github.com/android-linux-stable/msm-3.18/blob/kernel.lnx.3.18.r34-rel/drivers/power/qpnp-smbcharger.c
>> [2]
>> https://lore.kernel.org/linux-usb/[email protected]/
>
> How is it different from PMI8998? I expect not that much, so this
> should
> be based on existing work:
> https://lore.kernel.org/linux-arm-msm/[email protected]/
>
> Unless they are different, but then please create common parts and
> explain the differences.
>
> Best regards,
> Krzysztof

This driver has been in slow developement for a long time before that
one existed, which was why no initial attempt at a common driver was
made. With that said however, I've been watching its development even
before it was sent for review, and It seems that the hardware is
actually quite different. For example, the original charger entirely
lacks the type-c functionality that exists on the second gen one. There
are a couple of similar registers like CMD_APSD (same address and
function) CHGR_CFG2 (same/similar function, different address), but
other than that there don't seem to be any major similarities. While I
guess it would technically be possible to force them into one driver
with multiple register tables and separate functions for most tasks, I
think it would just unnecessarily complicate things. One thing that is
common however is the secure register unlock sequence, which I have
separated in patch 6 to allow for its use in other drivers (the fuel
gauge block has secure registers too so it will also be used in an
upcoming fuel gauge driver).


2022-08-08 13:33:16

by Caleb Connolly

[permalink] [raw]
Subject: Re: [PATCH 0/8] power: supply: Add driver for Qualcomm SMBCHG



On 08/08/2022 10:39, Yassine Oudjana wrote:
>
> On Mon, Aug 8 2022 at 11:41:26 +03:00:00, Krzysztof Kozlowski
> <[email protected]> wrote:
>> On 08/08/2022 10:34, Yassine Oudjana wrote:
>>>  From: Yassine Oudjana <[email protected]>
>>>
>>>  This series adds a driver for the switch-mode battery charger found on PMICs
>>>  such as PMI8994, and referred to in the vendor kernel[1] as smbcharger or
>>>  SMBCHG. More details on this block can be found in the last patch message.
>>>
>>>  This driver currently supports the charger blocks of PMI8994 and PMI8996.
>>>  PMI8950 was also to be supported, but it was dropped due to some last minute
>>>  issues, to be brought back at a later time once ready.
>>>
>>>  The OTG regulator remains unused on devices where the charger is enabled in
>>>  this series due to lack of a consumer. Applying a patch[2] adding vbus-supply
>>>  to DWC3 allows it to enable the OTG regulator making USB host without
>>>  external power possible.
>>>
>>>  [1]
>>> https://github.com/android-linux-stable/msm-3.18/blob/kernel.lnx.3.18.r34-rel/drivers/power/qpnp-smbcharger.c
>>>
>>>  [2]
>>> https://lore.kernel.org/linux-usb/[email protected]/
>>>
>>
>> How is it different from PMI8998? I expect not that much, so this should
>> be based on existing work:
>> https://lore.kernel.org/linux-arm-msm/[email protected]/
>>
>>
>> Unless they are different, but then please create common parts and
>> explain the differences.
>>
>> Best regards,
>> Krzysztof
>
> This driver has been in slow developement for a long time before that one
> existed, which was why no initial attempt at a common driver was made. With that
> said however, I've been watching its development even before it was sent for
> review, and It seems that the hardware is actually quite different. For example,
> the original charger entirely lacks the type-c functionality that exists on the
> second gen one. There are a couple of similar registers like CMD_APSD (same
> address and function) CHGR_CFG2 (same/similar function, different address), but
> other than that there don't seem to be any major similarities. While I guess it
> would technically be possible to force them into one driver with multiple
> register tables and separate functions for most tasks, I think it would just
> unnecessarily complicate things. One thing that is common however is the secure
> register unlock sequence, which I have separated in patch 6 to allow for its use
> in other drivers (the fuel gauge block has secure registers too so it will also
> be used in an upcoming fuel gauge driver).

Yes, we took the shared approach for the still work in progress fuel gauge
driver, and whilst there are more similarities in that block for basic
functionality at least, more complicated components differ quite a lot as far as
I'm aware.

Even for the fuel gauge, separate handlers are needed for a lot of things still:
https://gitlab.com/sdm845-mainline/linux/-/blob/sdm845/5.19-release/drivers/power/supply/qcom_fg.c#L792
So I don't think trying to create a common driver here is the right approach.

Perhaps some abstraction is possible for the overall similarities like handling
the APSD, dealing with current limiting, cable detection etc, perhaps some of
this common code could be pulled out into a shared "helper"?

Maybe this is something worth reconsidering as and when we look at adding
support for some of the more complicated features this hardware supports.
>
>

--
Kind Regards,
Caleb (they/he)