This series enables VM-BMS and LBC blocks in the pm8916 pmic.
The VM-BMS is a simple voltage monitoring block that allows the software
to estimate the battery capacity left.
The LBC is a linear battery charger for lipo batteries.
Add both devices to the pmic dtsi and enable them for Longcheer L8150
which makes use of them.
Signed-off-by: Nikita Travkin <[email protected]>
---
Changes in v2:
- No changes - resend with minor commit message edits.
- Link to v1: https://lore.kernel.org/r/[email protected]
---
Nikita Travkin (3):
dt-bindings: mfd: qcom,spmi-pmic: Add pm8916 vm-bms and lbc
arm64: dts: qcom: pm8916: Add BMS and charger
arm64: dts: qcom: msm8916-longcheer-l8150: Add battery and charger
.../devicetree/bindings/mfd/qcom,spmi-pmic.yaml | 6 +++
.../boot/dts/qcom/msm8916-longcheer-l8150.dts | 43 ++++++++++++++++---
arch/arm64/boot/dts/qcom/pm8916.dtsi | 48 ++++++++++++++++++++++
3 files changed, 91 insertions(+), 6 deletions(-)
---
base-commit: 2030579113a1b1b5bfd7ff24c0852847836d8fd1
change-id: 20230916-pm8916-dtsi-bms-lbc-2fb1b99d1eb2
Best regards,
--
Nikita Travkin <[email protected]>
Longcheer L8150 doesn't have any dedicated fuel-gauge or charger,
instead making use of the pmic hardware blocks for those purposes.
Add pm8916 bms and charger, as well as the battery cell description
that those blocks rely on.
Signed-off-by: Nikita Travkin <[email protected]>
---
.../boot/dts/qcom/msm8916-longcheer-l8150.dts | 43 +++++++++++++++++++---
1 file changed, 37 insertions(+), 6 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/msm8916-longcheer-l8150.dts b/arch/arm64/boot/dts/qcom/msm8916-longcheer-l8150.dts
index 47d1c5cb13f4..d35d34ef60d2 100644
--- a/arch/arm64/boot/dts/qcom/msm8916-longcheer-l8150.dts
+++ b/arch/arm64/boot/dts/qcom/msm8916-longcheer-l8150.dts
@@ -38,6 +38,25 @@ wcnss_mem: wcnss@8b600000 {
};
};
+ battery: battery {
+ compatible = "simple-battery";
+ voltage-min-design-microvolt = <3400000>;
+ voltage-max-design-microvolt = <4350000>;
+ energy-full-design-microwatt-hours = <9500000>;
+ charge-full-design-microamp-hours = <2500000>;
+
+ ocv-capacity-celsius = <25>;
+ ocv-capacity-table-0 = <4330000 100>, <4265000 95>,
+ <4208000 90>, <4153000 85>, <4100000 80>, <4049000 75>,
+ <4001000 70>, <3962000 65>, <3919000 60>, <3872000 55>,
+ <3839000 50>, <3817000 45>, <3798000 40>, <3783000 35>,
+ <3767000 30>, <3747000 25>, <3729000 20>, <3709000 16>,
+ <3688000 13>, <3681000 11>, <3680000 10>, <3679000 9>,
+ <3677000 8>, <3674000 7>, <3666000 6>, <3641000 5>,
+ <3597000 4>, <3537000 3>, <3457000 2>, <3336000 1>,
+ <3000000 0>;
+ };
+
gpio-keys {
compatible = "gpio-keys";
@@ -225,6 +244,22 @@ &blsp_uart2 {
status = "okay";
};
+&pm8916_bms {
+ status = "okay";
+
+ monitored-battery = <&battery>;
+ power-supplies = <&pm8916_charger>;
+};
+
+&pm8916_charger {
+ status = "okay";
+
+ monitored-battery = <&battery>;
+
+ qcom,fast-charge-safe-current = <900000>;
+ qcom,fast-charge-safe-voltage = <4300000>;
+};
+
&pm8916_resin {
status = "okay";
linux,code = <KEY_VOLUMEDOWN>;
@@ -237,10 +272,6 @@ pm8916_l17: l17 {
};
};
-&pm8916_usbin {
- status = "okay";
-};
-
&pm8916_vib {
status = "okay";
};
@@ -257,11 +288,11 @@ &sdhc_2 {
&usb {
status = "okay";
dr_mode = "peripheral";
- extcon = <&pm8916_usbin>;
+ extcon = <&pm8916_charger>;
};
&usb_hs_phy {
- extcon = <&pm8916_usbin>;
+ extcon = <&pm8916_charger>;
};
&venus {
--
2.41.0
PM8916 (and probably some other similar pmics) have hardware blocks for
battery monitoring and charging. Add patterns for respecive nodes so the
devicetree for those blocks can be validated properly.
Signed-off-by: Nikita Travkin <[email protected]>
---
Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml b/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml
index 9fa568603930..49103e07032a 100644
--- a/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml
+++ b/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml
@@ -134,9 +134,15 @@ patternProperties:
type: object
$ref: /schemas/sound/qcom,pm8916-wcd-analog-codec.yaml#
+ "^battery@[0-9a-f]+$":
+ type: object
+ oneOf:
+ - $ref: /schemas/power/supply/qcom,pm8916-bms-vm.yaml#
+
"^charger@[0-9a-f]+$":
type: object
oneOf:
+ - $ref: /schemas/power/supply/qcom,pm8916-lbc.yaml#
- $ref: /schemas/power/supply/qcom,pm8941-charger.yaml#
- $ref: /schemas/power/supply/qcom,pm8941-coincell.yaml#
- $ref: /schemas/power/supply/qcom,pmi8998-charger.yaml#
--
2.41.0
pm8916 contains some hardware blocks for battery powered devices:
- VM-BMS: Battery voltage monitoring block.
- LBC: Linear battery charger.
Add them to the pmic dtsi so the devices that make use of those blocks
can enable them.
Signed-off-by: Nikita Travkin <[email protected]>
---
arch/arm64/boot/dts/qcom/pm8916.dtsi | 48 ++++++++++++++++++++++++++++++++++++
1 file changed, 48 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/pm8916.dtsi b/arch/arm64/boot/dts/qcom/pm8916.dtsi
index f4de86787743..4b2e8fb47d2d 100644
--- a/arch/arm64/boot/dts/qcom/pm8916.dtsi
+++ b/arch/arm64/boot/dts/qcom/pm8916.dtsi
@@ -41,6 +41,35 @@ watchdog {
};
};
+ pm8916_charger: charger@1000 {
+ compatible = "qcom,pm8916-lbc";
+ reg = <0x1000>, <0x1200>, <0x1300>, <0x1600>;
+ reg-names = "chgr", "bat_if", "usb", "misc";
+
+ interrupts = <0x0 0x10 0 IRQ_TYPE_EDGE_BOTH>,
+ <0x0 0x10 5 IRQ_TYPE_EDGE_BOTH>,
+ <0x0 0x10 6 IRQ_TYPE_EDGE_BOTH>,
+ <0x0 0x10 7 IRQ_TYPE_EDGE_BOTH>,
+ <0x0 0x12 0 IRQ_TYPE_EDGE_BOTH>,
+ <0x0 0x12 1 IRQ_TYPE_EDGE_BOTH>,
+ <0x0 0x13 0 IRQ_TYPE_EDGE_BOTH>,
+ <0x0 0x13 1 IRQ_TYPE_EDGE_BOTH>,
+ <0x0 0x13 2 IRQ_TYPE_EDGE_BOTH>,
+ <0x0 0x13 4 IRQ_TYPE_EDGE_BOTH>;
+ interrupt-names = "vbat_det",
+ "fast_chg",
+ "chg_fail",
+ "chg_done",
+ "bat_pres",
+ "temp_ok",
+ "coarse_det",
+ "usb_vbus",
+ "chg_gone",
+ "overtemp";
+
+ status = "disabled";
+ };
+
pm8916_usbin: usb-detect@1300 {
compatible = "qcom,pm8941-misc";
reg = <0x1300>;
@@ -91,6 +120,25 @@ channel@f {
};
};
+ pm8916_bms: battery@4000 {
+ compatible = "qcom,pm8916-bms-vm";
+ reg = <0x4000>;
+ interrupts = <0x0 0x40 0 IRQ_TYPE_EDGE_RISING>,
+ <0x0 0x40 1 IRQ_TYPE_EDGE_RISING>,
+ <0x0 0x40 2 IRQ_TYPE_EDGE_RISING>,
+ <0x0 0x40 3 IRQ_TYPE_EDGE_RISING>,
+ <0x0 0x40 4 IRQ_TYPE_EDGE_RISING>,
+ <0x0 0x40 5 IRQ_TYPE_EDGE_RISING>;
+ interrupt-names = "cv_leave",
+ "cv_enter",
+ "ocv_good",
+ "ocv_thr",
+ "fifo",
+ "state_chg";
+
+ status = "disabled";
+ };
+
rtc@6000 {
compatible = "qcom,pm8941-rtc";
reg = <0x6000>, <0x6100>;
--
2.41.0
On Mon, 23 Oct 2023 11:20:32 +0500, Nikita Travkin wrote:
> PM8916 (and probably some other similar pmics) have hardware blocks for
> battery monitoring and charging. Add patterns for respecive nodes so the
> devicetree for those blocks can be validated properly.
>
> Signed-off-by: Nikita Travkin <[email protected]>
> ---
> Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml | 6 ++++++
> 1 file changed, 6 insertions(+)
>
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml:
Error in referenced schema matching $id: http://devicetree.org/schemas/power/supply/qcom,pm8916-bms-vm.yaml
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[email protected]
The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
Rob Herring писал(а) 23.10.2023 22:40:
> On Mon, 23 Oct 2023 11:20:32 +0500, Nikita Travkin wrote:
>> PM8916 (and probably some other similar pmics) have hardware blocks for
>> battery monitoring and charging. Add patterns for respecive nodes so the
>> devicetree for those blocks can be validated properly.
>>
>> Signed-off-by: Nikita Travkin <[email protected]>
>> ---
>> Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
>
> yamllint warnings/errors:
>
> dtschema/dtc warnings/errors:
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml:
> Error in referenced schema matching $id: http://devicetree.org/schemas/power/supply/qcom,pm8916-bms-vm.yaml
>
> doc reference errors (make refcheckdocs):
>
> See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[email protected]
>
> The base for the series is generally the latest rc1. A different dependency
> should be noted in *this* patch.
>
Somehow I missed the memo and thought it tracks -next...
This patch depends on 7f590e3831 and 5cee843d56 in linux-next.git
They were applied in [1].
I'm wondering if the bot just bails out when the "depend" is present
or there is some more sophisticated logic to suggest the base to it?
Sorry for the inconvenience
Nikita
[1] https://lore.kernel.org/r/[email protected]/
On 10/23/23 08:20, Nikita Travkin wrote:
> Longcheer L8150 doesn't have any dedicated fuel-gauge or charger,
> instead making use of the pmic hardware blocks for those purposes.
>
> Add pm8916 bms and charger, as well as the battery cell description
> that those blocks rely on.
>
> Signed-off-by: Nikita Travkin <[email protected]>
> ---
Doesn't apply on next, please rebase.
Konrad
On 10/23/23 08:20, Nikita Travkin wrote:
> pm8916 contains some hardware blocks for battery powered devices:
>
> - VM-BMS: Battery voltage monitoring block.
> - LBC: Linear battery charger.
>
> Add them to the pmic dtsi so the devices that make use of those blocks
> can enable them.
>
> Signed-off-by: Nikita Travkin <[email protected]>
> ---
> arch/arm64/boot/dts/qcom/pm8916.dtsi | 48 ++++++++++++++++++++++++++++++++++++
> 1 file changed, 48 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/pm8916.dtsi b/arch/arm64/boot/dts/qcom/pm8916.dtsi
> index f4de86787743..4b2e8fb47d2d 100644
> --- a/arch/arm64/boot/dts/qcom/pm8916.dtsi
> +++ b/arch/arm64/boot/dts/qcom/pm8916.dtsi
> @@ -41,6 +41,35 @@ watchdog {
> };
> };
>
> + pm8916_charger: charger@1000 {
> + compatible = "qcom,pm8916-lbc";
> + reg = <0x1000>, <0x1200>, <0x1300>, <0x1600>;
> + reg-names = "chgr", "bat_if", "usb", "misc";
> +
> + interrupts = <0x0 0x10 0 IRQ_TYPE_EDGE_BOTH>,
> + <0x0 0x10 5 IRQ_TYPE_EDGE_BOTH>,
> + <0x0 0x10 6 IRQ_TYPE_EDGE_BOTH>,
> + <0x0 0x10 7 IRQ_TYPE_EDGE_BOTH>,
> + <0x0 0x12 0 IRQ_TYPE_EDGE_BOTH>,
> + <0x0 0x12 1 IRQ_TYPE_EDGE_BOTH>,
> + <0x0 0x13 0 IRQ_TYPE_EDGE_BOTH>,
> + <0x0 0x13 1 IRQ_TYPE_EDGE_BOTH>,
> + <0x0 0x13 2 IRQ_TYPE_EDGE_BOTH>,
> + <0x0 0x13 4 IRQ_TYPE_EDGE_BOTH>;
> + interrupt-names = "vbat_det",
> + "fast_chg",
> + "chg_fail",
> + "chg_done",
> + "bat_pres",
> + "temp_ok",
> + "coarse_det",
> + "usb_vbus",
So, both the charger and the USBIN driver use the same irq? :/
Konrad
Konrad Dybcio писал(а) 24.10.2023 13:34:
> On 10/23/23 08:20, Nikita Travkin wrote:
>> pm8916 contains some hardware blocks for battery powered devices:
>>
>> - VM-BMS: Battery voltage monitoring block.
>> - LBC: Linear battery charger.
>>
>> Add them to the pmic dtsi so the devices that make use of those blocks
>> can enable them.
>>
>> Signed-off-by: Nikita Travkin <[email protected]>
>> ---
>> arch/arm64/boot/dts/qcom/pm8916.dtsi | 48 ++++++++++++++++++++++++++++++++++++
>> 1 file changed, 48 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/pm8916.dtsi b/arch/arm64/boot/dts/qcom/pm8916.dtsi
>> index f4de86787743..4b2e8fb47d2d 100644
>> --- a/arch/arm64/boot/dts/qcom/pm8916.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/pm8916.dtsi
>> @@ -41,6 +41,35 @@ watchdog {
>> };
>> };
>> + pm8916_charger: charger@1000 {
>> + compatible = "qcom,pm8916-lbc";
>> + reg = <0x1000>, <0x1200>, <0x1300>, <0x1600>;
>> + reg-names = "chgr", "bat_if", "usb", "misc";
>> +
>> + interrupts = <0x0 0x10 0 IRQ_TYPE_EDGE_BOTH>,
>> + <0x0 0x10 5 IRQ_TYPE_EDGE_BOTH>,
>> + <0x0 0x10 6 IRQ_TYPE_EDGE_BOTH>,
>> + <0x0 0x10 7 IRQ_TYPE_EDGE_BOTH>,
>> + <0x0 0x12 0 IRQ_TYPE_EDGE_BOTH>,
>> + <0x0 0x12 1 IRQ_TYPE_EDGE_BOTH>,
>> + <0x0 0x13 0 IRQ_TYPE_EDGE_BOTH>,
>> + <0x0 0x13 1 IRQ_TYPE_EDGE_BOTH>,
>> + <0x0 0x13 2 IRQ_TYPE_EDGE_BOTH>,
>> + <0x0 0x13 4 IRQ_TYPE_EDGE_BOTH>;
>> + interrupt-names = "vbat_det",
>> + "fast_chg",
>> + "chg_fail",
>> + "chg_done",
>> + "bat_pres",
>> + "temp_ok",
>> + "coarse_det",
>> + "usb_vbus",
> So, both the charger and the USBIN driver use the same irq? :/
>
AFAIU the usbin extcon driver pretty much just tracks the state
of the IRQ to report extcon. It happens to assume the same part
of the pmic though, yes, which also means there will be no user
that would enable both charger and vbus extcon, since charger
driver provides this functionality as well.
Nikita
> Konrad
Konrad Dybcio писал(а) 24.10.2023 13:33:
> On 10/23/23 08:20, Nikita Travkin wrote:
>> Longcheer L8150 doesn't have any dedicated fuel-gauge or charger,
>> instead making use of the pmic hardware blocks for those purposes.
>>
>> Add pm8916 bms and charger, as well as the battery cell description
>> that those blocks rely on.
>>
>> Signed-off-by: Nikita Travkin <[email protected]>
>> ---
> Doesn't apply on next, please rebase.
>
Ah, great, I guess I sent the series just a bit too early
and the -next didn't update yet...
I think at this point I might as well just wait for the -rc1,
will probably save everyone a bit of trouble...
Nikita
> Konrad
On Tue, 24 Oct 2023, Nikita Travkin wrote:
> Rob Herring писал(а) 23.10.2023 22:40:
> > On Mon, 23 Oct 2023 11:20:32 +0500, Nikita Travkin wrote:
> >> PM8916 (and probably some other similar pmics) have hardware blocks for
> >> battery monitoring and charging. Add patterns for respecive nodes so the
> >> devicetree for those blocks can be validated properly.
> >>
> >> Signed-off-by: Nikita Travkin <[email protected]>
> >> ---
> >> Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml | 6 ++++++
> >> 1 file changed, 6 insertions(+)
> >>
> >
> > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> > on your patch (DT_CHECKER_FLAGS is new in v5.13):
> >
> > yamllint warnings/errors:
> >
> > dtschema/dtc warnings/errors:
> > /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml:
> > Error in referenced schema matching $id: http://devicetree.org/schemas/power/supply/qcom,pm8916-bms-vm.yaml
> >
> > doc reference errors (make refcheckdocs):
> >
> > See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[email protected]
> >
> > The base for the series is generally the latest rc1. A different dependency
> > should be noted in *this* patch.
> >
>
> Somehow I missed the memo and thought it tracks -next...
>
> This patch depends on 7f590e3831 and 5cee843d56 in linux-next.git
> They were applied in [1].
>
> I'm wondering if the bot just bails out when the "depend" is present
> or there is some more sophisticated logic to suggest the base to it?
So is this good to go, or not?
--
Lee Jones [李琼斯]
Lee Jones писал(а) 25.10.2023 17:21:
> On Tue, 24 Oct 2023, Nikita Travkin wrote:
>
>> Rob Herring писал(а) 23.10.2023 22:40:
>> > On Mon, 23 Oct 2023 11:20:32 +0500, Nikita Travkin wrote:
>> >> PM8916 (and probably some other similar pmics) have hardware blocks for
>> >> battery monitoring and charging. Add patterns for respecive nodes so the
>> >> devicetree for those blocks can be validated properly.
>> >>
>> >> Signed-off-by: Nikita Travkin <[email protected]>
>> >> ---
>> >> Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml | 6 ++++++
>> >> 1 file changed, 6 insertions(+)
>> >>
>> >
>> > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
>> > on your patch (DT_CHECKER_FLAGS is new in v5.13):
>> >
>> > yamllint warnings/errors:
>> >
>> > dtschema/dtc warnings/errors:
>> > /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml:
>> > Error in referenced schema matching $id: http://devicetree.org/schemas/power/supply/qcom,pm8916-bms-vm.yaml
>> >
>> > doc reference errors (make refcheckdocs):
>> >
>> > See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[email protected]
>> >
>> > The base for the series is generally the latest rc1. A different dependency
>> > should be noted in *this* patch.
>> >
>>
>> Somehow I missed the memo and thought it tracks -next...
>>
>> This patch depends on 7f590e3831 and 5cee843d56 in linux-next.git
>> They were applied in [1].
>>
>> I'm wondering if the bot just bails out when the "depend" is present
>> or there is some more sophisticated logic to suggest the base to it?
>
> So is this good to go, or not?
IMO this patch should be good, it passes the check on today's linux-next
on my end.
The only concern might be that if someone runs dt_binding_check on
for-mfd-next, it would skip that file with an error since there is no
dependency yet.
If this is critical to you, I was going to respin this after the -rc1,
but if you can pick the schema now, I can respin the remainder earlier.
Nikita
On Mon, 23 Oct 2023 11:20:32 +0500, Nikita Travkin wrote:
> PM8916 (and probably some other similar pmics) have hardware blocks for
> battery monitoring and charging. Add patterns for respecive nodes so the
> devicetree for those blocks can be validated properly.
>
>
Applied, thanks!
[1/3] dt-bindings: mfd: qcom,spmi-pmic: Add pm8916 vm-bms and lbc
commit: e9aec86e211ee493081e8934b8c821d660b417ee
--
Lee Jones [李琼斯]
On 10/24/23 11:29, Nikita Travkin wrote:
> Konrad Dybcio писал(а) 24.10.2023 13:34:
>> On 10/23/23 08:20, Nikita Travkin wrote:
>>> pm8916 contains some hardware blocks for battery powered devices:
>>>
>>> - VM-BMS: Battery voltage monitoring block.
>>> - LBC: Linear battery charger.
>>>
>>> Add them to the pmic dtsi so the devices that make use of those blocks
>>> can enable them.
>>>
>>> Signed-off-by: Nikita Travkin <[email protected]>
>>> ---
>>> arch/arm64/boot/dts/qcom/pm8916.dtsi | 48 ++++++++++++++++++++++++++++++++++++
>>> 1 file changed, 48 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/qcom/pm8916.dtsi b/arch/arm64/boot/dts/qcom/pm8916.dtsi
>>> index f4de86787743..4b2e8fb47d2d 100644
>>> --- a/arch/arm64/boot/dts/qcom/pm8916.dtsi
>>> +++ b/arch/arm64/boot/dts/qcom/pm8916.dtsi
>>> @@ -41,6 +41,35 @@ watchdog {
>>> };
>>> };
>>> + pm8916_charger: charger@1000 {
>>> + compatible = "qcom,pm8916-lbc";
>>> + reg = <0x1000>, <0x1200>, <0x1300>, <0x1600>;
>>> + reg-names = "chgr", "bat_if", "usb", "misc";
>>> +
>>> + interrupts = <0x0 0x10 0 IRQ_TYPE_EDGE_BOTH>,
>>> + <0x0 0x10 5 IRQ_TYPE_EDGE_BOTH>,
>>> + <0x0 0x10 6 IRQ_TYPE_EDGE_BOTH>,
>>> + <0x0 0x10 7 IRQ_TYPE_EDGE_BOTH>,
>>> + <0x0 0x12 0 IRQ_TYPE_EDGE_BOTH>,
>>> + <0x0 0x12 1 IRQ_TYPE_EDGE_BOTH>,
>>> + <0x0 0x13 0 IRQ_TYPE_EDGE_BOTH>,
>>> + <0x0 0x13 1 IRQ_TYPE_EDGE_BOTH>,
>>> + <0x0 0x13 2 IRQ_TYPE_EDGE_BOTH>,
>>> + <0x0 0x13 4 IRQ_TYPE_EDGE_BOTH>;
>>> + interrupt-names = "vbat_det",
>>> + "fast_chg",
>>> + "chg_fail",
>>> + "chg_done",
>>> + "bat_pres",
>>> + "temp_ok",
>>> + "coarse_det",
>>> + "usb_vbus",
>> So, both the charger and the USBIN driver use the same irq? :/
>>
>
> AFAIU the usbin extcon driver pretty much just tracks the state
> of the IRQ to report extcon. It happens to assume the same part
> of the pmic though, yes, which also means there will be no user
> that would enable both charger and vbus extcon, since charger
> driver provides this functionality as well.
So, should USBIN be removed from PM8916 dt since it's essentially
a part of the charger block?
Konrad
On Thu, Oct 26, 2023 at 08:54:00PM +0200, Konrad Dybcio wrote:
> On 10/24/23 11:29, Nikita Travkin wrote:
> > Konrad Dybcio писал(а) 24.10.2023 13:34:
> > > On 10/23/23 08:20, Nikita Travkin wrote:
> > > > pm8916 contains some hardware blocks for battery powered devices:
> > > >
> > > > - VM-BMS: Battery voltage monitoring block.
> > > > - LBC: Linear battery charger.
> > > >
> > > > Add them to the pmic dtsi so the devices that make use of those blocks
> > > > can enable them.
> > > >
> > > > Signed-off-by: Nikita Travkin <[email protected]>
> > > > ---
> > > > arch/arm64/boot/dts/qcom/pm8916.dtsi | 48 ++++++++++++++++++++++++++++++++++++
> > > > 1 file changed, 48 insertions(+)
> > > >
> > > > diff --git a/arch/arm64/boot/dts/qcom/pm8916.dtsi b/arch/arm64/boot/dts/qcom/pm8916.dtsi
> > > > index f4de86787743..4b2e8fb47d2d 100644
> > > > --- a/arch/arm64/boot/dts/qcom/pm8916.dtsi
> > > > +++ b/arch/arm64/boot/dts/qcom/pm8916.dtsi
> > > > @@ -41,6 +41,35 @@ watchdog {
> > > > };
> > > > };
> > > > + pm8916_charger: charger@1000 {
> > > > + compatible = "qcom,pm8916-lbc";
> > > > + reg = <0x1000>, <0x1200>, <0x1300>, <0x1600>;
> > > > + reg-names = "chgr", "bat_if", "usb", "misc";
> > > > +
> > > > + interrupts = <0x0 0x10 0 IRQ_TYPE_EDGE_BOTH>,
> > > > + <0x0 0x10 5 IRQ_TYPE_EDGE_BOTH>,
> > > > + <0x0 0x10 6 IRQ_TYPE_EDGE_BOTH>,
> > > > + <0x0 0x10 7 IRQ_TYPE_EDGE_BOTH>,
> > > > + <0x0 0x12 0 IRQ_TYPE_EDGE_BOTH>,
> > > > + <0x0 0x12 1 IRQ_TYPE_EDGE_BOTH>,
> > > > + <0x0 0x13 0 IRQ_TYPE_EDGE_BOTH>,
> > > > + <0x0 0x13 1 IRQ_TYPE_EDGE_BOTH>,
> > > > + <0x0 0x13 2 IRQ_TYPE_EDGE_BOTH>,
> > > > + <0x0 0x13 4 IRQ_TYPE_EDGE_BOTH>;
> > > > + interrupt-names = "vbat_det",
> > > > + "fast_chg",
> > > > + "chg_fail",
> > > > + "chg_done",
> > > > + "bat_pres",
> > > > + "temp_ok",
> > > > + "coarse_det",
> > > > + "usb_vbus",
> > > So, both the charger and the USBIN driver use the same irq? :/
> > >
> >
> > AFAIU the usbin extcon driver pretty much just tracks the state
> > of the IRQ to report extcon. It happens to assume the same part
> > of the pmic though, yes, which also means there will be no user
> > that would enable both charger and vbus extcon, since charger
> > driver provides this functionality as well.
> So, should USBIN be removed from PM8916 dt since it's essentially
> a part of the charger block?
>
The "USB_IN" pad of the PM8916 seems to be connected on pretty much all
devices, even if they are using external chargers and the charging
functionality of PM8916 is completely disabled. For those devices, the
&pm8916_usbin device provides a convenient way to detect the USB state,
even without a working charger driver.
While we could modify the PM8916 charger driver and DT node to have some
special mode where charging and battery monitoring is completely
disabled and only the USBIN extcon is provided, I'm not sure if that
would provide a significant advantage compared to just keeping the
simple &pm8916_usbin node with the existing driver.
Thanks,
Stephan
On 10/26/23 21:17, Stephan Gerhold wrote:
> On Thu, Oct 26, 2023 at 08:54:00PM +0200, Konrad Dybcio wrote:
>> On 10/24/23 11:29, Nikita Travkin wrote:
>>> Konrad Dybcio писал(а) 24.10.2023 13:34:
>>>> On 10/23/23 08:20, Nikita Travkin wrote:
>>>>> pm8916 contains some hardware blocks for battery powered devices:
>>>>>
>>>>> - VM-BMS: Battery voltage monitoring block.
>>>>> - LBC: Linear battery charger.
>>>>>
>>>>> Add them to the pmic dtsi so the devices that make use of those blocks
>>>>> can enable them.
>>>>>
>>>>> Signed-off-by: Nikita Travkin <[email protected]>
>>>>> ---
>>>>> arch/arm64/boot/dts/qcom/pm8916.dtsi | 48 ++++++++++++++++++++++++++++++++++++
>>>>> 1 file changed, 48 insertions(+)
>>>>>
>>>>> diff --git a/arch/arm64/boot/dts/qcom/pm8916.dtsi b/arch/arm64/boot/dts/qcom/pm8916.dtsi
>>>>> index f4de86787743..4b2e8fb47d2d 100644
>>>>> --- a/arch/arm64/boot/dts/qcom/pm8916.dtsi
>>>>> +++ b/arch/arm64/boot/dts/qcom/pm8916.dtsi
>>>>> @@ -41,6 +41,35 @@ watchdog {
>>>>> };
>>>>> };
>>>>> + pm8916_charger: charger@1000 {
>>>>> + compatible = "qcom,pm8916-lbc";
>>>>> + reg = <0x1000>, <0x1200>, <0x1300>, <0x1600>;
>>>>> + reg-names = "chgr", "bat_if", "usb", "misc";
>>>>> +
>>>>> + interrupts = <0x0 0x10 0 IRQ_TYPE_EDGE_BOTH>,
>>>>> + <0x0 0x10 5 IRQ_TYPE_EDGE_BOTH>,
>>>>> + <0x0 0x10 6 IRQ_TYPE_EDGE_BOTH>,
>>>>> + <0x0 0x10 7 IRQ_TYPE_EDGE_BOTH>,
>>>>> + <0x0 0x12 0 IRQ_TYPE_EDGE_BOTH>,
>>>>> + <0x0 0x12 1 IRQ_TYPE_EDGE_BOTH>,
>>>>> + <0x0 0x13 0 IRQ_TYPE_EDGE_BOTH>,
>>>>> + <0x0 0x13 1 IRQ_TYPE_EDGE_BOTH>,
>>>>> + <0x0 0x13 2 IRQ_TYPE_EDGE_BOTH>,
>>>>> + <0x0 0x13 4 IRQ_TYPE_EDGE_BOTH>;
>>>>> + interrupt-names = "vbat_det",
>>>>> + "fast_chg",
>>>>> + "chg_fail",
>>>>> + "chg_done",
>>>>> + "bat_pres",
>>>>> + "temp_ok",
>>>>> + "coarse_det",
>>>>> + "usb_vbus",
>>>> So, both the charger and the USBIN driver use the same irq? :/
>>>>
>>>
>>> AFAIU the usbin extcon driver pretty much just tracks the state
>>> of the IRQ to report extcon. It happens to assume the same part
>>> of the pmic though, yes, which also means there will be no user
>>> that would enable both charger and vbus extcon, since charger
>>> driver provides this functionality as well.
>> So, should USBIN be removed from PM8916 dt since it's essentially
>> a part of the charger block?
>>
>
> The "USB_IN" pad of the PM8916 seems to be connected on pretty much all
> devices, even if they are using external chargers and the charging
> functionality of PM8916 is completely disabled. For those devices, the
> &pm8916_usbin device provides a convenient way to detect the USB state,
> even without a working charger driver.
>
> While we could modify the PM8916 charger driver and DT node to have some
> special mode where charging and battery monitoring is completely
> disabled and only the USBIN extcon is provided, I'm not sure if that
> would provide a significant advantage compared to just keeping the
> simple &pm8916_usbin node with the existing driver.
Hmm okay I see..
Generally it's rather "no bueno" to have two DT nodes consuming the
same register space.. What happens when you enable BMS on a device
with a non-PM8916 charger? Does it correctly recognize "no battery"
etc.?
Konrad
Konrad Dybcio писал(а) 27.10.2023 01:03:
> On 10/26/23 21:17, Stephan Gerhold wrote:
>> On Thu, Oct 26, 2023 at 08:54:00PM +0200, Konrad Dybcio wrote:
>>> On 10/24/23 11:29, Nikita Travkin wrote:
>>>> Konrad Dybcio писал(а) 24.10.2023 13:34:
>>>>> On 10/23/23 08:20, Nikita Travkin wrote:
>>>>>> pm8916 contains some hardware blocks for battery powered devices:
>>>>>>
>>>>>> - VM-BMS: Battery voltage monitoring block.
>>>>>> - LBC: Linear battery charger.
>>>>>>
>>>>>> Add them to the pmic dtsi so the devices that make use of those blocks
>>>>>> can enable them.
>>>>>>
>>>>>> Signed-off-by: Nikita Travkin <[email protected]>
>>>>>> ---
>>>>>> arch/arm64/boot/dts/qcom/pm8916.dtsi | 48 ++++++++++++++++++++++++++++++++++++
>>>>>> 1 file changed, 48 insertions(+)
>>>>>>
>>>>>> diff --git a/arch/arm64/boot/dts/qcom/pm8916.dtsi b/arch/arm64/boot/dts/qcom/pm8916.dtsi
>>>>>> index f4de86787743..4b2e8fb47d2d 100644
>>>>>> --- a/arch/arm64/boot/dts/qcom/pm8916.dtsi
>>>>>> +++ b/arch/arm64/boot/dts/qcom/pm8916.dtsi
>>>>>> @@ -41,6 +41,35 @@ watchdog {
>>>>>> };
>>>>>> };
>>>>>> + pm8916_charger: charger@1000 {
>>>>>> + compatible = "qcom,pm8916-lbc";
>>>>>> + reg = <0x1000>, <0x1200>, <0x1300>, <0x1600>;
>>>>>> + reg-names = "chgr", "bat_if", "usb", "misc";
>>>>>> +
>>>>>> + interrupts = <0x0 0x10 0 IRQ_TYPE_EDGE_BOTH>,
>>>>>> + <0x0 0x10 5 IRQ_TYPE_EDGE_BOTH>,
>>>>>> + <0x0 0x10 6 IRQ_TYPE_EDGE_BOTH>,
>>>>>> + <0x0 0x10 7 IRQ_TYPE_EDGE_BOTH>,
>>>>>> + <0x0 0x12 0 IRQ_TYPE_EDGE_BOTH>,
>>>>>> + <0x0 0x12 1 IRQ_TYPE_EDGE_BOTH>,
>>>>>> + <0x0 0x13 0 IRQ_TYPE_EDGE_BOTH>,
>>>>>> + <0x0 0x13 1 IRQ_TYPE_EDGE_BOTH>,
>>>>>> + <0x0 0x13 2 IRQ_TYPE_EDGE_BOTH>,
>>>>>> + <0x0 0x13 4 IRQ_TYPE_EDGE_BOTH>;
>>>>>> + interrupt-names = "vbat_det",
>>>>>> + "fast_chg",
>>>>>> + "chg_fail",
>>>>>> + "chg_done",
>>>>>> + "bat_pres",
>>>>>> + "temp_ok",
>>>>>> + "coarse_det",
>>>>>> + "usb_vbus",
>>>>> So, both the charger and the USBIN driver use the same irq? :/
>>>>>
>>>>
>>>> AFAIU the usbin extcon driver pretty much just tracks the state
>>>> of the IRQ to report extcon. It happens to assume the same part
>>>> of the pmic though, yes, which also means there will be no user
>>>> that would enable both charger and vbus extcon, since charger
>>>> driver provides this functionality as well.
>>> So, should USBIN be removed from PM8916 dt since it's essentially
>>> a part of the charger block?
>>>
>>
>> The "USB_IN" pad of the PM8916 seems to be connected on pretty much all
>> devices, even if they are using external chargers and the charging
>> functionality of PM8916 is completely disabled. For those devices, the
>> &pm8916_usbin device provides a convenient way to detect the USB state,
>> even without a working charger driver.
>>
>> While we could modify the PM8916 charger driver and DT node to have some
>> special mode where charging and battery monitoring is completely
>> disabled and only the USBIN extcon is provided, I'm not sure if that
>> would provide a significant advantage compared to just keeping the
>> simple &pm8916_usbin node with the existing driver.
> Hmm okay I see..
>
> Generally it's rather "no bueno" to have two DT nodes consuming the
> same register space.. What happens when you enable BMS on a device
> with a non-PM8916 charger? Does it correctly recognize "no battery"
> etc.?
>
The _charger and _bms are separate and communicate in a generic
manner via power-supplies and supply core (see 3/3) so giving
a different charger to _bms can work.
If an external charger is present in the device, qcom mandates
"external charger" optional line of the pmic to be tied, and
_charger is then disabled. The driver bails out in this case,
but _usbin could still be used.
> Konrad
On 25/10/2023 14:57, Nikita Travkin wrote:
> Lee Jones писал(а) 25.10.2023 17:21:
>> On Tue, 24 Oct 2023, Nikita Travkin wrote:
>>
>>> Rob Herring писал(а) 23.10.2023 22:40:
>>>> On Mon, 23 Oct 2023 11:20:32 +0500, Nikita Travkin wrote:
>>>>> PM8916 (and probably some other similar pmics) have hardware blocks for
>>>>> battery monitoring and charging. Add patterns for respecive nodes so the
>>>>> devicetree for those blocks can be validated properly.
>>>>>
>>>>> Signed-off-by: Nikita Travkin <[email protected]>
>>>>> ---
>>>>> Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml | 6 ++++++
>>>>> 1 file changed, 6 insertions(+)
>>>>>
>>>>
>>>> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
>>>> on your patch (DT_CHECKER_FLAGS is new in v5.13):
>>>>
>>>> yamllint warnings/errors:
>>>>
>>>> dtschema/dtc warnings/errors:
>>>> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml:
>>>> Error in referenced schema matching $id: http://devicetree.org/schemas/power/supply/qcom,pm8916-bms-vm.yaml
>>>>
>>>> doc reference errors (make refcheckdocs):
>>>>
>>>> See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[email protected]
>>>>
>>>> The base for the series is generally the latest rc1. A different dependency
>>>> should be noted in *this* patch.
>>>>
>>>
>>> Somehow I missed the memo and thought it tracks -next...
>>>
>>> This patch depends on 7f590e3831 and 5cee843d56 in linux-next.git
>>> They were applied in [1].
>>>
>>> I'm wondering if the bot just bails out when the "depend" is present
>>> or there is some more sophisticated logic to suggest the base to it?
>>
>> So is this good to go, or not?
>
> IMO this patch should be good, it passes the check on today's linux-next
> on my end.
It's not the next which matters, but maintainers tree.
>
> The only concern might be that if someone runs dt_binding_check on
> for-mfd-next, it would skip that file with an error since there is no
> dependency yet.
Eee, so this has dependency on some other tree? Then no, it is not good
to go.
Best regards,
Krzysztof
On 25/10/2023 17:44, Lee Jones wrote:
> On Mon, 23 Oct 2023 11:20:32 +0500, Nikita Travkin wrote:
>> PM8916 (and probably some other similar pmics) have hardware blocks for
>> battery monitoring and charging. Add patterns for respecive nodes so the
>> devicetree for those blocks can be validated properly.
>>
>>
>
> Applied, thanks!
>
> [1/3] dt-bindings: mfd: qcom,spmi-pmic: Add pm8916 vm-bms and lbc
> commit: e9aec86e211ee493081e8934b8c821d660b417ee
Hi Lee,
It seems this patch depends on something not in your tree. This should
have been clearly explained in cover letter or this patch changelog, but
wasn't.
Please drop the patch.
Best regards,
Krzysztof
On Fri, 27 Oct 2023, Krzysztof Kozlowski wrote:
> On 25/10/2023 17:44, Lee Jones wrote:
> > On Mon, 23 Oct 2023 11:20:32 +0500, Nikita Travkin wrote:
> >> PM8916 (and probably some other similar pmics) have hardware blocks for
> >> battery monitoring and charging. Add patterns for respecive nodes so the
> >> devicetree for those blocks can be validated properly.
> >>
> >>
> >
> > Applied, thanks!
> >
> > [1/3] dt-bindings: mfd: qcom,spmi-pmic: Add pm8916 vm-bms and lbc
> > commit: e9aec86e211ee493081e8934b8c821d660b417ee
>
> Hi Lee,
>
> It seems this patch depends on something not in your tree. This should
> have been clearly explained in cover letter or this patch changelog, but
> wasn't.
>
> Please drop the patch.
Done.
--
Lee Jones [李琼斯]
On 27.10.2023 07:44, Nikita Travkin wrote:
> Konrad Dybcio писал(а) 27.10.2023 01:03:
>> On 10/26/23 21:17, Stephan Gerhold wrote:
>>> On Thu, Oct 26, 2023 at 08:54:00PM +0200, Konrad Dybcio wrote:
>>>> On 10/24/23 11:29, Nikita Travkin wrote:
>>>>> Konrad Dybcio писал(а) 24.10.2023 13:34:
>>>>>> On 10/23/23 08:20, Nikita Travkin wrote:
>>>>>>> pm8916 contains some hardware blocks for battery powered devices:
>>>>>>>
>>>>>>> - VM-BMS: Battery voltage monitoring block.
>>>>>>> - LBC: Linear battery charger.
>>>>>>>
>>>>>>> Add them to the pmic dtsi so the devices that make use of those blocks
>>>>>>> can enable them.
>>>>>>>
>>>>>>> Signed-off-by: Nikita Travkin <[email protected]>
>>>>>>> ---
>>>>>>> arch/arm64/boot/dts/qcom/pm8916.dtsi | 48 ++++++++++++++++++++++++++++++++++++
>>>>>>> 1 file changed, 48 insertions(+)
>>>>>>>
>>>>>>> diff --git a/arch/arm64/boot/dts/qcom/pm8916.dtsi b/arch/arm64/boot/dts/qcom/pm8916.dtsi
>>>>>>> index f4de86787743..4b2e8fb47d2d 100644
>>>>>>> --- a/arch/arm64/boot/dts/qcom/pm8916.dtsi
>>>>>>> +++ b/arch/arm64/boot/dts/qcom/pm8916.dtsi
>>>>>>> @@ -41,6 +41,35 @@ watchdog {
>>>>>>> };
>>>>>>> };
>>>>>>> + pm8916_charger: charger@1000 {
>>>>>>> + compatible = "qcom,pm8916-lbc";
>>>>>>> + reg = <0x1000>, <0x1200>, <0x1300>, <0x1600>;
>>>>>>> + reg-names = "chgr", "bat_if", "usb", "misc";
>>>>>>> +
>>>>>>> + interrupts = <0x0 0x10 0 IRQ_TYPE_EDGE_BOTH>,
>>>>>>> + <0x0 0x10 5 IRQ_TYPE_EDGE_BOTH>,
>>>>>>> + <0x0 0x10 6 IRQ_TYPE_EDGE_BOTH>,
>>>>>>> + <0x0 0x10 7 IRQ_TYPE_EDGE_BOTH>,
>>>>>>> + <0x0 0x12 0 IRQ_TYPE_EDGE_BOTH>,
>>>>>>> + <0x0 0x12 1 IRQ_TYPE_EDGE_BOTH>,
>>>>>>> + <0x0 0x13 0 IRQ_TYPE_EDGE_BOTH>,
>>>>>>> + <0x0 0x13 1 IRQ_TYPE_EDGE_BOTH>,
>>>>>>> + <0x0 0x13 2 IRQ_TYPE_EDGE_BOTH>,
>>>>>>> + <0x0 0x13 4 IRQ_TYPE_EDGE_BOTH>;
>>>>>>> + interrupt-names = "vbat_det",
>>>>>>> + "fast_chg",
>>>>>>> + "chg_fail",
>>>>>>> + "chg_done",
>>>>>>> + "bat_pres",
>>>>>>> + "temp_ok",
>>>>>>> + "coarse_det",
>>>>>>> + "usb_vbus",
>>>>>> So, both the charger and the USBIN driver use the same irq? :/
>>>>>>
>>>>>
>>>>> AFAIU the usbin extcon driver pretty much just tracks the state
>>>>> of the IRQ to report extcon. It happens to assume the same part
>>>>> of the pmic though, yes, which also means there will be no user
>>>>> that would enable both charger and vbus extcon, since charger
>>>>> driver provides this functionality as well.
>>>> So, should USBIN be removed from PM8916 dt since it's essentially
>>>> a part of the charger block?
>>>>
>>>
>>> The "USB_IN" pad of the PM8916 seems to be connected on pretty much all
>>> devices, even if they are using external chargers and the charging
>>> functionality of PM8916 is completely disabled. For those devices, the
>>> &pm8916_usbin device provides a convenient way to detect the USB state,
>>> even without a working charger driver.
>>>
>>> While we could modify the PM8916 charger driver and DT node to have some
>>> special mode where charging and battery monitoring is completely
>>> disabled and only the USBIN extcon is provided, I'm not sure if that
>>> would provide a significant advantage compared to just keeping the
>>> simple &pm8916_usbin node with the existing driver.
>> Hmm okay I see..
>>
>> Generally it's rather "no bueno" to have two DT nodes consuming the
>> same register space.. What happens when you enable BMS on a device
>> with a non-PM8916 charger? Does it correctly recognize "no battery"
>> etc.?
>>
>
> The _charger and _bms are separate and communicate in a generic
> manner via power-supplies and supply core (see 3/3) so giving
> a different charger to _bms can work.
>
> If an external charger is present in the device, qcom mandates
> "external charger" optional line of the pmic to be tied, and
> _charger is then disabled. The driver bails out in this case,
> but _usbin could still be used.
Meh..
I guess I'll reluctantly let it slide, unless Bjorn has some objections
Konrad