All of lore.kernel.org
 help / color / mirror / Atom feed
From: Lee Jones <lee@kernel.org>
To: Chris Morgan <macroalpha82@gmail.com>
Cc: linux-rockchip@lists.infradead.org, linux-pm@vger.kernel.org,
	maccraft123mc@gmail.com, heiko@sntech.de, sre@kernel.org,
	Chris Morgan <macromorgan@hotmail.com>
Subject: Re: [PATCH V2 0/4] Fix Incorrect State of Charge for RK817
Date: Tue, 3 Nov 2026 09:24:17 +0000	[thread overview]
Message-ID: <20261103092417.GG13023@google.com> (raw)
In-Reply-To: <20261102143942.305293-1-macroalpha82@gmail.com>

On Mon, 02 Nov 2026, Chris Morgan wrote:

> From: Chris Morgan <macromorgan@hotmail.com>
> 
> Users of the popular firmware RockNIX had identified a few bugs with
> accurately displaying the correct state of charge for RK817 battery
> devices. With the help of one user analyzing the code with Claude
> they were able to identify a few potential bugs along with potential
> solutions which I was then able to verify. These bugs are as follows:
> 
> 1) Incorrectly clamping a value with a max of 10000 instead of 100000.
> This bug caused the state of charge to sometimes read as 10% instead
> of 100% as it should have.
> 
> 2) Using the boot_voltage value to calibrate the battery in scenarios
> where it could not be trusted. The boot_voltage register populates
> immediately at power-on, and the driver would check if this value
> was older than 30 minutes before using it. Unfortunately when combined
> with the default behavior of the bootloader to turn the device off
> when plugged in (requiring a user to explicitly power the device on)
> the boot_voltage and boot_timer values were wrong. The boot voltage
> would not account for any voltage added while the device was off,
> and the boot_counter that tracks the time the device was off would
> be reset. To fix this we changed the behavior to (outside of first-boot
> scenarios) use a value saved to NVRAM, the value from the coulomb
> counter directly, and an OCV lookup from the live battery voltage,
> and then using those values provide an initial "guess" of the state
> of charge and recalibrate with the OCV voltage once it is available.
> 
> Included in this series is another fix that would cause the battery
> to drain while the system was shut down. This patch replicates the
> Rockchip BSP driver to clear a shutdown register. Unlike the BSP there
> is no behavior to bypass this since no boards have yet been observed
> using the SYS_CAN_SD bypass on the BSP kernel.
> 
> Changes Since V1:
>  - Dropped the sleep fixes, as I would like to test them a bit more
>    extensively first.
>  - Dropped the devicetree parameter, as since no boards have *yet*
>    needed to alter the default behavior there's no need for it.
>  - Changed the behavior of how we select the initial SOC and made
>    it its own function to ease in code readibility. This new method
>    was tested with various boot scenarios. The average error was less
>    than 5%, and in a worst case scenario where the device was
>    discharged to 0%, then left to charge at the bootloader the device
>    over-reported its state of charge by about 10%. When this occurred
>    the device reported its SOC as 100% but that it was charging, which
>    was corrected by the new OCV logic that kicked in once charging
>    completed.
>  - Scrapped the more complicated sleep fix in favor of simply telling
>    the battery to recalibrate at full after suspend, in case of a 
>    counting anomaly. 
> 
> Chris Morgan (4):
>   power: supply: rk817-charger: Fix NVRAM SoC Value
>   power: supply: rk817-charger: Resolve boot state of charge from the
>     coulomb counter
>   power: supply: rk817-charger: Clear SYS_CAN_SD register by default
>   power: supply: rk817-charger: Clear full charge cal on resume
> 
>  drivers/power/supply/rk817_charger.c | 313 ++++++++++++++++++++-------

>  include/linux/mfd/rk808.h            |   3 +

Acked-by: Lee Jones <lee@kernel.org>

>  2 files changed, 239 insertions(+), 77 deletions(-)

-- 
Lee Jones

WARNING: multiple messages have this Message-ID (diff)
From: Lee Jones <lee@kernel.org>
To: Chris Morgan <macroalpha82@gmail.com>
Cc: linux-rockchip@lists.infradead.org, linux-pm@vger.kernel.org,
	maccraft123mc@gmail.com, heiko@sntech.de, sre@kernel.org,
	Chris Morgan <macromorgan@hotmail.com>
Subject: Re: [PATCH V2 0/4] Fix Incorrect State of Charge for RK817
Date: Tue, 3 Nov 2026 09:24:17 +0000	[thread overview]
Message-ID: <20261103092417.GG13023@google.com> (raw)
In-Reply-To: <20261102143942.305293-1-macroalpha82@gmail.com>

On Mon, 02 Nov 2026, Chris Morgan wrote:

> From: Chris Morgan <macromorgan@hotmail.com>
> 
> Users of the popular firmware RockNIX had identified a few bugs with
> accurately displaying the correct state of charge for RK817 battery
> devices. With the help of one user analyzing the code with Claude
> they were able to identify a few potential bugs along with potential
> solutions which I was then able to verify. These bugs are as follows:
> 
> 1) Incorrectly clamping a value with a max of 10000 instead of 100000.
> This bug caused the state of charge to sometimes read as 10% instead
> of 100% as it should have.
> 
> 2) Using the boot_voltage value to calibrate the battery in scenarios
> where it could not be trusted. The boot_voltage register populates
> immediately at power-on, and the driver would check if this value
> was older than 30 minutes before using it. Unfortunately when combined
> with the default behavior of the bootloader to turn the device off
> when plugged in (requiring a user to explicitly power the device on)
> the boot_voltage and boot_timer values were wrong. The boot voltage
> would not account for any voltage added while the device was off,
> and the boot_counter that tracks the time the device was off would
> be reset. To fix this we changed the behavior to (outside of first-boot
> scenarios) use a value saved to NVRAM, the value from the coulomb
> counter directly, and an OCV lookup from the live battery voltage,
> and then using those values provide an initial "guess" of the state
> of charge and recalibrate with the OCV voltage once it is available.
> 
> Included in this series is another fix that would cause the battery
> to drain while the system was shut down. This patch replicates the
> Rockchip BSP driver to clear a shutdown register. Unlike the BSP there
> is no behavior to bypass this since no boards have yet been observed
> using the SYS_CAN_SD bypass on the BSP kernel.
> 
> Changes Since V1:
>  - Dropped the sleep fixes, as I would like to test them a bit more
>    extensively first.
>  - Dropped the devicetree parameter, as since no boards have *yet*
>    needed to alter the default behavior there's no need for it.
>  - Changed the behavior of how we select the initial SOC and made
>    it its own function to ease in code readibility. This new method
>    was tested with various boot scenarios. The average error was less
>    than 5%, and in a worst case scenario where the device was
>    discharged to 0%, then left to charge at the bootloader the device
>    over-reported its state of charge by about 10%. When this occurred
>    the device reported its SOC as 100% but that it was charging, which
>    was corrected by the new OCV logic that kicked in once charging
>    completed.
>  - Scrapped the more complicated sleep fix in favor of simply telling
>    the battery to recalibrate at full after suspend, in case of a 
>    counting anomaly. 
> 
> Chris Morgan (4):
>   power: supply: rk817-charger: Fix NVRAM SoC Value
>   power: supply: rk817-charger: Resolve boot state of charge from the
>     coulomb counter
>   power: supply: rk817-charger: Clear SYS_CAN_SD register by default
>   power: supply: rk817-charger: Clear full charge cal on resume
> 
>  drivers/power/supply/rk817_charger.c | 313 ++++++++++++++++++++-------

>  include/linux/mfd/rk808.h            |   3 +

Acked-by: Lee Jones <lee@kernel.org>

>  2 files changed, 239 insertions(+), 77 deletions(-)

-- 
Lee Jones

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

  parent reply	other threads:[~2026-11-03  9:24 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-02 14:39 [PATCH V2 0/4] Fix Incorrect State of Charge for RK817 Chris Morgan
2026-11-02 14:39 ` Chris Morgan
2026-11-02 14:39 ` [PATCH V2 1/4] power: supply: rk817-charger: Fix NVRAM SoC Value Chris Morgan
2026-11-02 14:39   ` Chris Morgan
2026-11-02 14:39 ` [PATCH V2 2/4] power: supply: rk817-charger: Resolve boot state of charge from the coulomb counter Chris Morgan
2026-11-02 14:39   ` Chris Morgan
2026-11-02 14:39 ` [PATCH V2 3/4] power: supply: rk817-charger: Clear SYS_CAN_SD register by default Chris Morgan
2026-11-02 14:39   ` Chris Morgan
2026-11-02 14:39 ` [PATCH V2 4/4] power: supply: rk817-charger: Clear full charge cal on resume Chris Morgan
2026-11-02 14:39   ` Chris Morgan
2026-11-03  9:24 ` Lee Jones [this message]
2026-11-03  9:24   ` [PATCH V2 0/4] Fix Incorrect State of Charge for RK817 Lee Jones

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261103092417.GG13023@google.com \
    --to=lee@kernel.org \
    --cc=heiko@sntech.de \
    --cc=linux-pm@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=maccraft123mc@gmail.com \
    --cc=macroalpha82@gmail.com \
    --cc=macromorgan@hotmail.com \
    --cc=sre@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.