From: sashiko-bot@kernel.org
To: "Fredrik M Olsson" <fredrik.m.olsson@axis.com>
Cc: conor+dt@kernel.org, devicetree@vger.kernel.org, robh@kernel.org
Subject: Re: [PATCH v2 3/3] rtc: ds1307: Add driver for Epson RX8901CE
Date: Wed, 20 May 2026 17:03:13 +0000 [thread overview]
Message-ID: <20260520170313.C15271F00894@smtp.kernel.org> (raw)
In-Reply-To: <20260520-ds1307-rx8901-add-v2-3-e069ea32e1db@axis.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Trickle charging is not explicitly disabled when requested, posing a battery safety hazard.
--
commit 1b11fec487efda4e55a4152eb696205b964ae2cd
Author: Fredrik M Olsson <fredrik.m.olsson@axis.com>
rtc: ds1307: Add driver for Epson RX8901CE
This commit adds I2C RTC driver support for the Epson RX8901CE. It includes
support for time reading/writing, Backup Switch Mode (BSM) configuration,
trickle charging, and battery voltage low status detection.
> diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
> index bf42c250ea7d1..23dd104aa2be7 100644
> --- a/drivers/rtc/rtc-ds1307.c
> +++ b/drivers/rtc/rtc-ds1307.c
[ ... ]
> @@ -568,6 +717,26 @@ static u8 do_trickle_setup_rx8130(struct ds1307 *ds1307, u32 ohms, bool diode)
> return setup;
> }
>
> +static int do_trickle_setup_rx8901(struct ds1307 *ds1307, u32 ohms __always_unused, bool diode)
> +{
> + int ret;
> + unsigned int setup;
> +
> + ret = regmap_read(ds1307->regmap, RX8901_REG_PWSW_CFG, &setup);
> + if (ret) {
> + dev_err(ds1307->dev, "Failed to read PWSW_CFG register\n");
> + return ret;
> + }
> +
> + /* Enable low battery voltage detection */
> + setup |= RX8901_REG_PWSW_CFG_VBATLDETEN;
> +
> + if (diode)
> + setup |= RX8901_REG_PWSW_CFG_CHGEN;
[Severity: High]
If diode is false, does this code need to explicitly clear the
RX8901_REG_PWSW_CFG_CHGEN bit?
Because RX8901_REG_PWSW_CFG is a battery-backed register, a previously
set charge enable bit (for example, from a bootloader or a prior warm
reboot) might remain set even if trickle charging is explicitly disabled
by the system configuration.
Could this lead to a physical safety hazard by unintentionally charging a
primary (non-rechargeable) battery if the bit is not explicitly cleared
using something like setup &= ~RX8901_REG_PWSW_CFG_CHGEN?
> +
> + return setup;
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260520-ds1307-rx8901-add-v2-0-e069ea32e1db@axis.com?part=3
next prev parent reply other threads:[~2026-05-20 17:03 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-20 14:48 [PATCH v2 0/3] rtc: ds1307: Add support for Epson RX8901CE Fredrik M Olsson
2026-05-20 14:48 ` [PATCH v2 1/3] dt-bindings: rtc: ds1307: Add epson,rx8901 Fredrik M Olsson
2026-05-20 14:48 ` [PATCH v2 2/3] rtc: ds1307: Fix off-by-one issue with wday for rx8130 Fredrik M Olsson
2026-05-20 15:45 ` sashiko-bot
2026-05-20 14:48 ` [PATCH v2 3/3] rtc: ds1307: Add driver for Epson RX8901CE Fredrik M Olsson
2026-05-20 17:03 ` sashiko-bot [this message]
2026-06-24 21:38 ` Alexandre Belloni
2026-06-24 21:42 ` (subset) [PATCH v2 0/3] rtc: ds1307: Add support " Alexandre Belloni
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=20260520170313.C15271F00894@smtp.kernel.org \
--to=sashiko-bot@kernel.org \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=fredrik.m.olsson@axis.com \
--cc=robh@kernel.org \
--cc=sashiko-reviews@lists.linux.dev \
/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.