* Re: [PATCH v2 00/17] tee: Use bus callbacks instead of driver callbacks
From: Alexandre Belloni @ 2025-12-18 13:53 UTC (permalink / raw)
To: Jens Wiklander
Cc: Uwe Kleine-König, Jonathan Corbet, Sumit Garg,
Olivia Mackall, Herbert Xu, Clément Léger,
Ard Biesheuvel, Maxime Coquelin, Alexandre Torgue, Sumit Garg,
Ilias Apalodimas, Jan Kiszka, Sudeep Holla, Christophe JAILLET,
Rafał Miłecki, Michael Chan, Pavan Chebbi,
James Bottomley, Jarkko Sakkinen, Mimi Zohar, David Howells,
Paul Moore, James Morris, Serge E. Hallyn, Peter Huewe, op-tee,
linux-kernel, linux-doc, linux-crypto, linux-rtc, linux-efi,
linux-stm32, linux-arm-kernel, Cristian Marussi, arm-scmi,
linux-mips, netdev, linux-integrity, keyrings,
linux-security-module, Jason Gunthorpe
In-Reply-To: <CAHUa44FrDZbvRvfN8obf80_k=Eqxe9YxHpjaE5jU7nkxPUwfag@mail.gmail.com>
On 18/12/2025 08:21:27+0100, Jens Wiklander wrote:
> Hi,
>
> On Mon, Dec 15, 2025 at 3:17 PM Uwe Kleine-König
> <u.kleine-koenig@baylibre.com> wrote:
> >
> > Hello,
> >
> > the objective of this series is to make tee driver stop using callbacks
> > in struct device_driver. These were superseded by bus methods in 2006
> > (commit 594c8281f905 ("[PATCH] Add bus_type probe, remove, shutdown
> > methods.")) but nobody cared to convert all subsystems accordingly.
> >
> > Here the tee drivers are converted. The first commit is somewhat
> > unrelated, but simplifies the conversion (and the drivers). It
> > introduces driver registration helpers that care about setting the bus
> > and owner. (The latter is missing in all drivers, so by using these
> > helpers the drivers become more correct.)
> >
> > v1 of this series is available at
> > https://lore.kernel.org/all/cover.1765472125.git.u.kleine-koenig@baylibre.com
> >
> > Changes since v1:
> >
> > - rebase to v6.19-rc1 (no conflicts)
> > - add tags received so far
> > - fix whitespace issues pointed out by Sumit Garg
> > - fix shutdown callback to shutdown and not remove
> >
> > As already noted in v1's cover letter, this series should go in during a
> > single merge window as there are runtime warnings when the series is
> > only applied partially. Sumit Garg suggested to apply the whole series
> > via Jens Wiklander's tree.
> > If this is done the dependencies in this series are honored, in case the
> > plan changes: Patches #4 - #17 depend on the first two.
> >
> > Note this series is only build tested.
> >
> > Uwe Kleine-König (17):
> > tee: Add some helpers to reduce boilerplate for tee client drivers
> > tee: Add probe, remove and shutdown bus callbacks to tee_client_driver
> > tee: Adapt documentation to cover recent additions
> > hwrng: optee - Make use of module_tee_client_driver()
> > hwrng: optee - Make use of tee bus methods
> > rtc: optee: Migrate to use tee specific driver registration function
> > rtc: optee: Make use of tee bus methods
> > efi: stmm: Make use of module_tee_client_driver()
> > efi: stmm: Make use of tee bus methods
> > firmware: arm_scmi: optee: Make use of module_tee_client_driver()
> > firmware: arm_scmi: Make use of tee bus methods
> > firmware: tee_bnxt: Make use of module_tee_client_driver()
> > firmware: tee_bnxt: Make use of tee bus methods
> > KEYS: trusted: Migrate to use tee specific driver registration
> > function
> > KEYS: trusted: Make use of tee bus methods
> > tpm/tpm_ftpm_tee: Make use of tee specific driver registration
> > tpm/tpm_ftpm_tee: Make use of tee bus methods
> >
> > Documentation/driver-api/tee.rst | 18 +----
> > drivers/char/hw_random/optee-rng.c | 26 ++----
> > drivers/char/tpm/tpm_ftpm_tee.c | 31 +++++---
> > drivers/firmware/arm_scmi/transports/optee.c | 32 +++-----
> > drivers/firmware/broadcom/tee_bnxt_fw.c | 30 ++-----
> > drivers/firmware/efi/stmm/tee_stmm_efi.c | 25 ++----
> > drivers/rtc/rtc-optee.c | 27 ++-----
> > drivers/tee/tee_core.c | 84 ++++++++++++++++++++
> > include/linux/tee_drv.h | 12 +++
> > security/keys/trusted-keys/trusted_tee.c | 17 ++--
> > 10 files changed, 164 insertions(+), 138 deletions(-)
> >
> > base-commit: 8f0b4cce4481fb22653697cced8d0d04027cb1e8
> > --
> > 2.47.3
> >
>
> Thank you for the nice cleanup, Uwe.
>
> I've applied patch 1-3 to the branch tee_bus_callback_for_6.20 in my
> tree at https://git.kernel.org/pub/scm/linux/kernel/git/jenswi/linux-tee.git/
>
> The branch is based on v6.19-rc1, and I'll try to keep it stable for
> others to depend on, if needed. Let's see if we can agree on taking
> the remaining patches via that branch.
6 and 7 can go through your branch.
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH v2 00/17] tee: Use bus callbacks instead of driver callbacks
From: Jens Wiklander @ 2025-12-18 7:21 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Jonathan Corbet, Sumit Garg, Olivia Mackall, Herbert Xu,
Clément Léger, Alexandre Belloni, Ard Biesheuvel,
Maxime Coquelin, Alexandre Torgue, Sumit Garg, Ilias Apalodimas,
Jan Kiszka, Sudeep Holla, Christophe JAILLET,
Rafał Miłecki, Michael Chan, Pavan Chebbi,
James Bottomley, Jarkko Sakkinen, Mimi Zohar, David Howells,
Paul Moore, James Morris, Serge E. Hallyn, Peter Huewe, op-tee,
linux-kernel, linux-doc, linux-crypto, linux-rtc, linux-efi,
linux-stm32, linux-arm-kernel, Cristian Marussi, arm-scmi,
linux-mips, netdev, linux-integrity, keyrings,
linux-security-module, Jason Gunthorpe
In-Reply-To: <cover.1765791463.git.u.kleine-koenig@baylibre.com>
Hi,
On Mon, Dec 15, 2025 at 3:17 PM Uwe Kleine-König
<u.kleine-koenig@baylibre.com> wrote:
>
> Hello,
>
> the objective of this series is to make tee driver stop using callbacks
> in struct device_driver. These were superseded by bus methods in 2006
> (commit 594c8281f905 ("[PATCH] Add bus_type probe, remove, shutdown
> methods.")) but nobody cared to convert all subsystems accordingly.
>
> Here the tee drivers are converted. The first commit is somewhat
> unrelated, but simplifies the conversion (and the drivers). It
> introduces driver registration helpers that care about setting the bus
> and owner. (The latter is missing in all drivers, so by using these
> helpers the drivers become more correct.)
>
> v1 of this series is available at
> https://lore.kernel.org/all/cover.1765472125.git.u.kleine-koenig@baylibre.com
>
> Changes since v1:
>
> - rebase to v6.19-rc1 (no conflicts)
> - add tags received so far
> - fix whitespace issues pointed out by Sumit Garg
> - fix shutdown callback to shutdown and not remove
>
> As already noted in v1's cover letter, this series should go in during a
> single merge window as there are runtime warnings when the series is
> only applied partially. Sumit Garg suggested to apply the whole series
> via Jens Wiklander's tree.
> If this is done the dependencies in this series are honored, in case the
> plan changes: Patches #4 - #17 depend on the first two.
>
> Note this series is only build tested.
>
> Uwe Kleine-König (17):
> tee: Add some helpers to reduce boilerplate for tee client drivers
> tee: Add probe, remove and shutdown bus callbacks to tee_client_driver
> tee: Adapt documentation to cover recent additions
> hwrng: optee - Make use of module_tee_client_driver()
> hwrng: optee - Make use of tee bus methods
> rtc: optee: Migrate to use tee specific driver registration function
> rtc: optee: Make use of tee bus methods
> efi: stmm: Make use of module_tee_client_driver()
> efi: stmm: Make use of tee bus methods
> firmware: arm_scmi: optee: Make use of module_tee_client_driver()
> firmware: arm_scmi: Make use of tee bus methods
> firmware: tee_bnxt: Make use of module_tee_client_driver()
> firmware: tee_bnxt: Make use of tee bus methods
> KEYS: trusted: Migrate to use tee specific driver registration
> function
> KEYS: trusted: Make use of tee bus methods
> tpm/tpm_ftpm_tee: Make use of tee specific driver registration
> tpm/tpm_ftpm_tee: Make use of tee bus methods
>
> Documentation/driver-api/tee.rst | 18 +----
> drivers/char/hw_random/optee-rng.c | 26 ++----
> drivers/char/tpm/tpm_ftpm_tee.c | 31 +++++---
> drivers/firmware/arm_scmi/transports/optee.c | 32 +++-----
> drivers/firmware/broadcom/tee_bnxt_fw.c | 30 ++-----
> drivers/firmware/efi/stmm/tee_stmm_efi.c | 25 ++----
> drivers/rtc/rtc-optee.c | 27 ++-----
> drivers/tee/tee_core.c | 84 ++++++++++++++++++++
> include/linux/tee_drv.h | 12 +++
> security/keys/trusted-keys/trusted_tee.c | 17 ++--
> 10 files changed, 164 insertions(+), 138 deletions(-)
>
> base-commit: 8f0b4cce4481fb22653697cced8d0d04027cb1e8
> --
> 2.47.3
>
Thank you for the nice cleanup, Uwe.
I've applied patch 1-3 to the branch tee_bus_callback_for_6.20 in my
tree at https://git.kernel.org/pub/scm/linux/kernel/git/jenswi/linux-tee.git/
The branch is based on v6.19-rc1, and I'll try to keep it stable for
others to depend on, if needed. Let's see if we can agree on taking
the remaining patches via that branch.
Cheers,
Jens
^ permalink raw reply
* RE: [PATCH 4/4] rtc: zynqmp: use dynamic max and min offset ranges
From: T, Harini @ 2025-12-17 19:03 UTC (permalink / raw)
To: Tomas Melin, Alexandre Belloni, Simek, Michal
Cc: linux-rtc@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
In-Reply-To: <ea85c8d6-ead8-4fda-905d-909968de5056@vaisala.com>
[Public]
Hi,
> -----Original Message-----
> From: Tomas Melin <tomas.melin@vaisala.com>
> Sent: Wednesday, December 10, 2025 5:55 PM
> To: T, Harini <Harini.T@amd.com>; Alexandre Belloni
> <alexandre.belloni@bootlin.com>; Simek, Michal <michal.simek@amd.com>
> Cc: linux-rtc@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH 4/4] rtc: zynqmp: use dynamic max and min offset ranges
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> Hi,
>
> On 09/12/2025 21:28, T, Harini wrote:
> > [Public]
> >
> > Hi,
> >
> >> -----Original Message-----
> >> From: Tomas Melin <tomas.melin@vaisala.com>
> >> Sent: Monday, December 1, 2025 6:20 PM
> >> To: Alexandre Belloni <alexandre.belloni@bootlin.com>; Simek, Michal
> >> <michal.simek@amd.com>
> >> Cc: linux-rtc@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> >> linux- kernel@vger.kernel.org; Tomas Melin <tomas.melin@vaisala.com>
> >> Subject: [PATCH 4/4] rtc: zynqmp: use dynamic max and min offset
> >> ranges
> >>
> >> Caution: This message originated from an External Source. Use proper
> >> caution when opening attachments, clicking links, or responding.
> >>
> >>
> >> Maximum and minimum offsets in ppb that can be handled are dependent
> >> on the rtc clock frequency and what can fit in the 16-bit register field.
> >>
> >> Signed-off-by: Tomas Melin <tomas.melin@vaisala.com>
> >> ---
> >> drivers/rtc/rtc-zynqmp.c | 8 +++-----
> >> 1 file changed, 3 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/drivers/rtc/rtc-zynqmp.c b/drivers/rtc/rtc-zynqmp.c
> >> index
> >>
> 3bc8831ba2c4c4c701a49506b67ae6174f3ade3d..0cebc99b15a6de2440a6
> 0afc
> >> 2bd1769eccfa84b3 100644
> >> --- a/drivers/rtc/rtc-zynqmp.c
> >> +++ b/drivers/rtc/rtc-zynqmp.c
> >> @@ -44,8 +44,6 @@
> >> #define RTC_FR_MASK 0xF0000
> >> #define RTC_FR_MAX_TICKS 16
> >> #define RTC_PPB 1000000000LL
> >> -#define RTC_MIN_OFFSET -32768000
> >> -#define RTC_MAX_OFFSET 32767000
> >>
> >> struct xlnx_rtc_dev {
> >> struct rtc_device *rtc;
> >> @@ -215,12 +213,12 @@ static int xlnx_rtc_set_offset(struct device
> >> *dev, long offset)
> >>
> >> /* ticks to reach RTC_PPB */
> >> tick_mult = DIV_ROUND_CLOSEST(RTC_PPB, xrtcdev->freq);
> >> - if (offset < RTC_MIN_OFFSET || offset > RTC_MAX_OFFSET)
> >> - return -ERANGE;
> >> -
> >> /* Number ticks for given offset */
> >> max_tick = div_s64_rem(offset, tick_mult, &fract_offset);
> >>
> >> + if (freq + max_tick > RTC_TICK_MASK || (freq + max_tick < 1))
> > The check 'freq + max_tick < 1' should be '<2' to prevent writing 0 to the
> calibration register when fract_offset < 0 causes max_tick--.
> > Example: freq=32767, max_tick=-32766 passes (sum=1), but after
> decrement becomes calibval=0.
> calibval=0 is not documented as invalid calibration value. AFAIS it would mean
> a frequency of 1Hz. Can You provide more info on this?
>
You're right - calibval=0 is hardware-spec compliant and produces 1Hz
operation. The patch is correct as-is with '< 1'.
There's no issue with allowing calibval=0 from a hardware perspective.
> Thanks,
> Tomas
>
>
> >> + return -ERANGE;
> >> +
> >> /* Number fractional ticks for given offset */
> >> if (fract_offset) {
> >> /* round up here so we stay below a full tick */
> >>
> >> --
> >> 2.47.3
> >>
> >
> > Thanks,
> > Harini T
> >
Regards,
Harini T
^ permalink raw reply
* RE: [PATCH 3/4] rtc: zynqmp: rework set_offset
From: T, Harini @ 2025-12-17 18:33 UTC (permalink / raw)
To: Tomas Melin, Alexandre Belloni, Simek, Michal
Cc: linux-rtc@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
In-Reply-To: <d02fda5e-47d4-4613-9a21-ed81bc957146@vaisala.com>
[Public]
Hi,
> -----Original Message-----
> From: Tomas Melin <tomas.melin@vaisala.com>
> Sent: Wednesday, December 10, 2025 5:48 PM
> To: T, Harini <Harini.T@amd.com>; Alexandre Belloni
> <alexandre.belloni@bootlin.com>; Simek, Michal <michal.simek@amd.com>
> Cc: linux-rtc@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH 3/4] rtc: zynqmp: rework set_offset
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> Hi,
>
> On 09/12/2025 21:03, T, Harini wrote:
> > [Public]
> >
> > Hi,
> >
> >> -----Original Message-----
> >> From: Tomas Melin <tomas.melin@vaisala.com>
> >> Sent: Monday, December 1, 2025 6:20 PM
> >> To: Alexandre Belloni <alexandre.belloni@bootlin.com>; Simek, Michal
> >> <michal.simek@amd.com>
> >> Cc: linux-rtc@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> >> linux- kernel@vger.kernel.org; Tomas Melin <tomas.melin@vaisala.com>
> >> Subject: [PATCH 3/4] rtc: zynqmp: rework set_offset
> >>
> >> Caution: This message originated from an External Source. Use proper
> >> caution when opening attachments, clicking links, or responding.
> >>
> >>
> >> set_offset was using remainder of do_div as tick_mult which resulted
> >> in wrong offset. Calibration value also assumed builtin calibration default.
> >> Update fract_offset to correctly calculate the value for negative
> >> offset and replace the for loop with division.
> >>
> >> Signed-off-by: Tomas Melin <tomas.melin@vaisala.com>
> >> ---
> >> drivers/rtc/rtc-zynqmp.c | 29 +++++++++++------------------
> >> 1 file changed, 11 insertions(+), 18 deletions(-)
> >>
> >> diff --git a/drivers/rtc/rtc-zynqmp.c b/drivers/rtc/rtc-zynqmp.c
> >> index
> >>
> 7af5f6f99538f961a53ff56bfc656c907611b900..3bc8831ba2c4c4c701a495
> 06b6
> >> 7ae6174f3ade3d 100644
> >> --- a/drivers/rtc/rtc-zynqmp.c
> >> +++ b/drivers/rtc/rtc-zynqmp.c
> >> @@ -208,13 +208,13 @@ static int xlnx_rtc_read_offset(struct device
> >> *dev, long *offset) static int xlnx_rtc_set_offset(struct device *dev, long
> offset) {
> >> struct xlnx_rtc_dev *xrtcdev = dev_get_drvdata(dev);
> >> - unsigned long long rtc_ppb = RTC_PPB;
> >> - unsigned int tick_mult = do_div(rtc_ppb, xrtcdev->freq);
> >> + unsigned int calibval, tick_mult, fract_part;
> > tick_mult is mentioned as int in previous patch and unsigned here. Justify
> the type in commit description.
> >> unsigned char fract_tick = 0;
> >> - unsigned int calibval;
> >> - short int max_tick;
> >> - int fract_offset;
> >> + int freq = xrtcdev->freq;
> >> + int max_tick, fract_offset;
> > Please follow reverse xmas tree variable ordering.
> > Also keep the frac_* variables uniform in both set and read offset functions.
> Agreed, I will use same name of variables and types in next version.
>
> >>
> >> + /* ticks to reach RTC_PPB */
> > The comment is misleading. Its tick_mult is nanoseconds per tick, not a tick
> count.
> Answered in patch 2/4.
> >> + tick_mult = DIV_ROUND_CLOSEST(RTC_PPB, xrtcdev->freq);
> > We can first validate offset and then calculate tick_mult to reduce
> > CPU instructions incase of invalid inputs
> In this patch it would in theory apply, but when looking at patch 4/4 You will
> notice that we need to first calculate the helpers so offset is then performed as
> soon as possible.
>
> >> if (offset < RTC_MIN_OFFSET || offset > RTC_MAX_OFFSET)
> >> return -ERANGE;
> >>
> >> @@ -223,29 +223,22 @@ static int xlnx_rtc_set_offset(struct device
> >> *dev, long offset)
> >>
> >> /* Number fractional ticks for given offset */
> >> if (fract_offset) {
> >> + /* round up here so we stay below a full tick */
> >> + fract_part = DIV_ROUND_UP(tick_mult,
> >> + RTC_FR_MAX_TICKS);
> >> if (fract_offset < 0) {
> >> - fract_offset = fract_offset + tick_mult;
> >> + fract_offset += (fract_part *
> >> + RTC_FR_MAX_TICKS);
> > It would be better to add comment to explain on the negative offset
> > borrowing logic
> I will add comment about this.
>
>
> >> max_tick--;
> >> }
> >> - if (fract_offset > (tick_mult / RTC_FR_MAX_TICKS)) {
> >> - for (fract_tick = 1; fract_tick < 16; fract_tick++) {
> >> - if (fract_offset <=
> >> - (fract_tick *
> >> - (tick_mult / RTC_FR_MAX_TICKS)))
> >> - break;
> >> - }
> >> - }
> >> + fract_tick = fract_offset / fract_part;
> > Its better to use DIV_ROUND_UP()
> Please explain why, that would change the end result from what is is now.
The truncating division is acceptable as-is. Given the hardware's discrete
4-bit fractional field (16 levels), the quantization error from truncation
is negligible compared to the fundamental hardware precision limit. If you
want to optimize, DIV_ROUND_CLOSEST() would minimize average error, but it's
not required for correctness.
>
> Thanks,
> Tomas
>
> >> }
> >>
> >> /* Zynqmp RTC uses second and fractional tick
> >> * counters for compensation
> >> */
> >> - calibval = max_tick + RTC_CALIB_DEF;
> >> + calibval = max_tick + freq;
> >>
> >> if (fract_tick)
> >> - calibval |= RTC_FR_EN;
> >> -
> >> - calibval |= (fract_tick << RTC_FR_DATSHIFT);
> >> + calibval |= (RTC_FR_EN | (fract_tick <<
> >> + RTC_FR_DATSHIFT));
> >>
> >> writel(calibval, (xrtcdev->reg_base + RTC_CALIB_WR));
> >>
> >>
> >> --
> >> 2.47.3
> >>
> >
> > Regards,
> > Harini T
Regards,
Harini T
^ permalink raw reply
* Re: [PATCH 2/4] rtc: zynqmp: rework read_offset
From: Alexandre Belloni @ 2025-12-17 19:17 UTC (permalink / raw)
To: T, Harini
Cc: Tomas Melin, Simek, Michal, linux-rtc@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
In-Reply-To: <LV5PR12MB980448D3F4109DC0775A56AA92ABA@LV5PR12MB9804.namprd12.prod.outlook.com>
On 17/12/2025 18:14:30+0000, T, Harini wrote:
> [Public]
>
> Hi,
>
> > -----Original Message-----
> > From: Tomas Melin <tomas.melin@vaisala.com>
> > Sent: Wednesday, December 10, 2025 5:35 PM
> > To: T, Harini <Harini.T@amd.com>; Alexandre Belloni
> > <alexandre.belloni@bootlin.com>; Simek, Michal <michal.simek@amd.com>
> > Cc: linux-rtc@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> > kernel@vger.kernel.org
> > Subject: Re: [PATCH 2/4] rtc: zynqmp: rework read_offset
> >
> > Caution: This message originated from an External Source. Use proper caution
> > when opening attachments, clicking links, or responding.
> >
> >
> > Hi,
> >
> > On 09/12/2025 19:28, T, Harini wrote:
> > > [Public]
> > >
> > > Hi,
> > >
> > >> -----Original Message-----
> > >> From: Tomas Melin <tomas.melin@vaisala.com>
> > >> Sent: Monday, December 1, 2025 6:20 PM
> > >> To: Alexandre Belloni <alexandre.belloni@bootlin.com>; Simek, Michal
> > >> <michal.simek@amd.com>
> > >> Cc: linux-rtc@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> > >> linux- kernel@vger.kernel.org; Tomas Melin <tomas.melin@vaisala.com>
> > >> Subject: [PATCH 2/4] rtc: zynqmp: rework read_offset
> > >>
> > >> Caution: This message originated from an External Source. Use proper
> > >> caution when opening attachments, clicking links, or responding.
> > >>
> > >>
> > >> read_offset() was using static frequency for determining the tick
> > >> offset. It was also using remainder from do_div() operation as
> > >> tick_mult value which caused the offset to be incorrect.
> > >>
> > >> At the same time, rework function to improve readability.
> > >>
> > >> Signed-off-by: Tomas Melin <tomas.melin@vaisala.com>
> > >> ---
> > >> drivers/rtc/rtc-zynqmp.c | 25 ++++++++++++++++---------
> > >> 1 file changed, 16 insertions(+), 9 deletions(-)
> > >>
> > >> diff --git a/drivers/rtc/rtc-zynqmp.c b/drivers/rtc/rtc-zynqmp.c
> > >> index
> > >>
> > 856bc1678e7d31144f320ae9f75fc58c742a2a64..7af5f6f99538f961a53ff56
> > bfc6
> > >> 56c907611b900 100644
> > >> --- a/drivers/rtc/rtc-zynqmp.c
> > >> +++ b/drivers/rtc/rtc-zynqmp.c
> > >> @@ -178,21 +178,28 @@ static void xlnx_init_rtc(struct xlnx_rtc_dev
> > >> *xrtcdev) static int xlnx_rtc_read_offset(struct device *dev, long *offset) {
> > >> struct xlnx_rtc_dev *xrtcdev = dev_get_drvdata(dev);
> > >> - unsigned long long rtc_ppb = RTC_PPB;
> > >> - unsigned int tick_mult = do_div(rtc_ppb, xrtcdev->freq);
> > >> - unsigned int calibval;
> > >> + unsigned int calibval, fract_data, fract_part;
> > > Prefer one variable assignment per line for readability.
> > This is after all quite common practice, and in a function like this where several
> > variables are needed, I would argue that this is more readable than the
> > alternative. Is there some convention I'm not aware of?
> There is no such mandatory convention. It's up to the RTC maintainer.
I don't mind having multiple variable declarations on a single line.
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* RE: [PATCH 2/4] rtc: zynqmp: rework read_offset
From: T, Harini @ 2025-12-17 18:14 UTC (permalink / raw)
To: Tomas Melin, Alexandre Belloni, Simek, Michal
Cc: linux-rtc@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
In-Reply-To: <353422a2-ba6e-4600-9326-e0cee2098062@vaisala.com>
[Public]
Hi,
> -----Original Message-----
> From: Tomas Melin <tomas.melin@vaisala.com>
> Sent: Wednesday, December 10, 2025 5:35 PM
> To: T, Harini <Harini.T@amd.com>; Alexandre Belloni
> <alexandre.belloni@bootlin.com>; Simek, Michal <michal.simek@amd.com>
> Cc: linux-rtc@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH 2/4] rtc: zynqmp: rework read_offset
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> Hi,
>
> On 09/12/2025 19:28, T, Harini wrote:
> > [Public]
> >
> > Hi,
> >
> >> -----Original Message-----
> >> From: Tomas Melin <tomas.melin@vaisala.com>
> >> Sent: Monday, December 1, 2025 6:20 PM
> >> To: Alexandre Belloni <alexandre.belloni@bootlin.com>; Simek, Michal
> >> <michal.simek@amd.com>
> >> Cc: linux-rtc@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> >> linux- kernel@vger.kernel.org; Tomas Melin <tomas.melin@vaisala.com>
> >> Subject: [PATCH 2/4] rtc: zynqmp: rework read_offset
> >>
> >> Caution: This message originated from an External Source. Use proper
> >> caution when opening attachments, clicking links, or responding.
> >>
> >>
> >> read_offset() was using static frequency for determining the tick
> >> offset. It was also using remainder from do_div() operation as
> >> tick_mult value which caused the offset to be incorrect.
> >>
> >> At the same time, rework function to improve readability.
> >>
> >> Signed-off-by: Tomas Melin <tomas.melin@vaisala.com>
> >> ---
> >> drivers/rtc/rtc-zynqmp.c | 25 ++++++++++++++++---------
> >> 1 file changed, 16 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/drivers/rtc/rtc-zynqmp.c b/drivers/rtc/rtc-zynqmp.c
> >> index
> >>
> 856bc1678e7d31144f320ae9f75fc58c742a2a64..7af5f6f99538f961a53ff56
> bfc6
> >> 56c907611b900 100644
> >> --- a/drivers/rtc/rtc-zynqmp.c
> >> +++ b/drivers/rtc/rtc-zynqmp.c
> >> @@ -178,21 +178,28 @@ static void xlnx_init_rtc(struct xlnx_rtc_dev
> >> *xrtcdev) static int xlnx_rtc_read_offset(struct device *dev, long *offset) {
> >> struct xlnx_rtc_dev *xrtcdev = dev_get_drvdata(dev);
> >> - unsigned long long rtc_ppb = RTC_PPB;
> >> - unsigned int tick_mult = do_div(rtc_ppb, xrtcdev->freq);
> >> - unsigned int calibval;
> >> + unsigned int calibval, fract_data, fract_part;
> > Prefer one variable assignment per line for readability.
> This is after all quite common practice, and in a function like this where several
> variables are needed, I would argue that this is more readable than the
> alternative. Is there some convention I'm not aware of?
There is no such mandatory convention. It's up to the RTC maintainer.
>
> >> + int max_tick, tick_mult;
> > It would be better to explain why tick_mult is changed to int in the commit
> message.
> This is part of the refactoring, mixing signed and unsigned variables in
> operations is more risky than having same type.
I agree. But tick_mult is int in read_offset and unsigned in set_offset.
It would be better if both uses int to maintain consistency during the math operations.
>
> >> + int freq = xrtcdev->freq;
> > Please follow reverse xmas tree variable ordering.
> Ok fixing this and other occurances.
>
> >> long offset_val;
> >>
> >> + /* ticks to reach RTC_PPB */
> > The comment is misleading. Its tick_mult is nanoseconds per tick, not a tick
> count.
> Perhaps the comment was not well formulated. I suggest changing to
> /* Tick to offset multiplier */
> as that it what it is primarily used for. Would that be okay for You?
Yeah
>
> Thanks,
> Tomas
>
> >> + tick_mult = DIV_ROUND_CLOSEST(RTC_PPB, freq);
> >> +
> >> calibval = readl(xrtcdev->reg_base + RTC_CALIB_RD);
> >> /* Offset with seconds ticks */
> >> - offset_val = calibval & RTC_TICK_MASK;
> >> - offset_val = offset_val - RTC_CALIB_DEF;
> >> - offset_val = offset_val * tick_mult;
> >> + max_tick = calibval & RTC_TICK_MASK;
> >> + offset_val = max_tick - freq;
> >> + /* Convert to ppb */
> >> + offset_val *= tick_mult;
> >>
> >> /* Offset with fractional ticks */
> >> - if (calibval & RTC_FR_EN)
> >> - offset_val += ((calibval & RTC_FR_MASK) >> RTC_FR_DATSHIFT)
> >> - * (tick_mult / RTC_FR_MAX_TICKS);
> >> + if (calibval & RTC_FR_EN) {
> >> + fract_data = (calibval & RTC_FR_MASK) >> RTC_FR_DATSHIFT;
> >> + fract_part = DIV_ROUND_UP(tick_mult, RTC_FR_MAX_TICKS);
> >> + offset_val += (fract_part * fract_data);
> >> + }
> >> +
> >> *offset = offset_val;
> >>
> >> return 0;
> >>
> >> --
> >> 2.47.3
> >>
> > Regards,
> > Harini T
> >
>
Regards,
Harini T
^ permalink raw reply
* [PATCH RESEND v2 3/3] mfd: sec: drop now unused struct sec_pmic_dev::irq_data
From: André Draszik @ 2025-12-17 10:10 UTC (permalink / raw)
To: Krzysztof Kozlowski, Lee Jones, Alexandre Belloni
Cc: Peter Griffin, Tudor Ambarus, Will McVicker, Juan Yescas,
Douglas Anderson, kernel-team, Kaustabh Chakraborty, linux-kernel,
linux-samsung-soc, linux-rtc, André Draszik
In-Reply-To: <20251217-s5m-alarm-v2-0-b7bff003e94c@linaro.org>
This was used only to allow the s5m RTC driver to deal with the alarm
IRQ. That driver now uses a different approach to acquire that IRQ, and
::irq_data doesn't need to be kept around anymore.
Signed-off-by: André Draszik <andre.draszik@linaro.org>
---
drivers/mfd/sec-common.c | 9 +++---
drivers/mfd/sec-core.h | 2 +-
drivers/mfd/sec-irq.c | 63 ++++++++++++++++++----------------------
include/linux/mfd/samsung/core.h | 1 -
4 files changed, 35 insertions(+), 40 deletions(-)
diff --git a/drivers/mfd/sec-common.c b/drivers/mfd/sec-common.c
index 77370db52a7ba81234136b29f85892f4b197f429..0021f9ae8484fd0afc2e47c813a953c91fa38546 100644
--- a/drivers/mfd/sec-common.c
+++ b/drivers/mfd/sec-common.c
@@ -163,6 +163,7 @@ sec_pmic_parse_dt_pdata(struct device *dev)
int sec_pmic_probe(struct device *dev, int device_type, unsigned int irq,
struct regmap *regmap, struct i2c_client *client)
{
+ struct regmap_irq_chip_data *irq_data;
struct sec_platform_data *pdata;
const struct mfd_cell *sec_devs;
struct sec_pmic_dev *sec_pmic;
@@ -187,9 +188,9 @@ int sec_pmic_probe(struct device *dev, int device_type, unsigned int irq,
sec_pmic->pdata = pdata;
- ret = sec_irq_init(sec_pmic);
- if (ret)
- return ret;
+ irq_data = sec_irq_init(sec_pmic);
+ if (IS_ERR(irq_data))
+ return PTR_ERR(irq_data);
pm_runtime_set_active(sec_pmic->dev);
@@ -240,7 +241,7 @@ int sec_pmic_probe(struct device *dev, int device_type, unsigned int irq,
sec_pmic->device_type);
}
ret = devm_mfd_add_devices(sec_pmic->dev, -1, sec_devs, num_sec_devs,
- NULL, 0, regmap_irq_get_domain(sec_pmic->irq_data));
+ NULL, 0, regmap_irq_get_domain(irq_data));
if (ret)
return ret;
diff --git a/drivers/mfd/sec-core.h b/drivers/mfd/sec-core.h
index 92c7558ab8b0de44a52e028eeb7998e38358cb4c..8d85c70c232612d1f7e5fb61b2acd25bf03a62e0 100644
--- a/drivers/mfd/sec-core.h
+++ b/drivers/mfd/sec-core.h
@@ -18,6 +18,6 @@ int sec_pmic_probe(struct device *dev, int device_type, unsigned int irq,
struct regmap *regmap, struct i2c_client *client);
void sec_pmic_shutdown(struct device *dev);
-int sec_irq_init(struct sec_pmic_dev *sec_pmic);
+struct regmap_irq_chip_data *sec_irq_init(struct sec_pmic_dev *sec_pmic);
#endif /* __SEC_CORE_INT_H */
diff --git a/drivers/mfd/sec-irq.c b/drivers/mfd/sec-irq.c
index d992e41e716dcdc060421e1db8475523842a12be..96f53c3617da4cb54f650f9b98c0b934b823ceda 100644
--- a/drivers/mfd/sec-irq.c
+++ b/drivers/mfd/sec-irq.c
@@ -268,26 +268,28 @@ static const struct regmap_irq_chip s5m8767_irq_chip = {
.ack_base = S5M8767_REG_INT1,
};
-static int s2mpg1x_add_chained_irq_chip(struct device *dev, struct regmap *regmap, int pirq,
- struct regmap_irq_chip_data *parent,
- const struct regmap_irq_chip *chip,
- struct regmap_irq_chip_data **data)
+static struct regmap_irq_chip_data *
+s2mpg1x_add_chained_irq_chip(struct device *dev, struct regmap *regmap, int pirq,
+ struct regmap_irq_chip_data *parent,
+ const struct regmap_irq_chip *chip)
{
+ struct regmap_irq_chip_data *data;
int irq, ret;
irq = regmap_irq_get_virq(parent, pirq);
if (irq < 0)
- return dev_err_probe(dev, irq, "Failed to get parent vIRQ(%d) for chip %s\n", pirq,
- chip->name);
+ return dev_err_ptr_probe(dev, irq, "Failed to get parent vIRQ(%d) for chip %s\n",
+ pirq, chip->name);
- ret = devm_regmap_add_irq_chip(dev, regmap, irq, IRQF_ONESHOT | IRQF_SHARED, 0, chip, data);
+ ret = devm_regmap_add_irq_chip(dev, regmap, irq, IRQF_ONESHOT | IRQF_SHARED, 0, chip,
+ &data);
if (ret)
- return dev_err_probe(dev, ret, "Failed to add %s IRQ chip\n", chip->name);
+ return dev_err_ptr_probe(dev, ret, "Failed to add %s IRQ chip\n", chip->name);
- return 0;
+ return data;
}
-static int sec_irq_init_s2mpg1x(struct sec_pmic_dev *sec_pmic)
+static struct regmap_irq_chip_data *sec_irq_init_s2mpg1x(struct sec_pmic_dev *sec_pmic)
{
const struct regmap_irq_chip *irq_chip, *chained_irq_chip;
struct regmap_irq_chip_data *irq_data;
@@ -302,27 +304,28 @@ static int sec_irq_init_s2mpg1x(struct sec_pmic_dev *sec_pmic)
chained_pirq = S2MPG10_COMMON_IRQ_PMIC;
break;
default:
- return dev_err_probe(sec_pmic->dev, -EINVAL, "Unsupported device type %d\n",
- sec_pmic->device_type);
+ return dev_err_ptr_probe(sec_pmic->dev, -EINVAL, "Unsupported device type %d\n",
+ sec_pmic->device_type);
};
regmap_common = dev_get_regmap(sec_pmic->dev, "common");
if (!regmap_common)
- return dev_err_probe(sec_pmic->dev, -EINVAL, "No 'common' regmap %d\n",
- sec_pmic->device_type);
+ return dev_err_ptr_probe(sec_pmic->dev, -EINVAL, "No 'common' regmap %d\n",
+ sec_pmic->device_type);
ret = devm_regmap_add_irq_chip(sec_pmic->dev, regmap_common, sec_pmic->irq, IRQF_ONESHOT, 0,
irq_chip, &irq_data);
if (ret)
- return dev_err_probe(sec_pmic->dev, ret, "Failed to add %s IRQ chip\n",
- irq_chip->name);
+ return dev_err_ptr_probe(sec_pmic->dev, ret, "Failed to add %s IRQ chip\n",
+ irq_chip->name);
return s2mpg1x_add_chained_irq_chip(sec_pmic->dev, sec_pmic->regmap_pmic, chained_pirq,
- irq_data, chained_irq_chip, &sec_pmic->irq_data);
+ irq_data, chained_irq_chip);
}
-int sec_irq_init(struct sec_pmic_dev *sec_pmic)
+struct regmap_irq_chip_data *sec_irq_init(struct sec_pmic_dev *sec_pmic)
{
+ struct regmap_irq_chip_data *sec_irq_chip_data;
const struct regmap_irq_chip *sec_irq_chip;
int ret;
@@ -331,7 +334,7 @@ int sec_irq_init(struct sec_pmic_dev *sec_pmic)
sec_irq_chip = &s5m8767_irq_chip;
break;
case S2DOS05:
- return 0;
+ return NULL;
case S2MPA01:
sec_irq_chip = &s2mps14_irq_chip;
break;
@@ -356,30 +359,22 @@ int sec_irq_init(struct sec_pmic_dev *sec_pmic)
sec_irq_chip = &s2mpu05_irq_chip;
break;
default:
- return dev_err_probe(sec_pmic->dev, -EINVAL,
- "Unsupported device type %d\n",
- sec_pmic->device_type);
+ return dev_err_ptr_probe(sec_pmic->dev, -EINVAL, "Unsupported device type %d\n",
+ sec_pmic->device_type);
}
if (!sec_pmic->irq) {
dev_warn(sec_pmic->dev,
"No interrupt specified, no interrupts\n");
- return 0;
+ return NULL;
}
ret = devm_regmap_add_irq_chip(sec_pmic->dev, sec_pmic->regmap_pmic,
sec_pmic->irq, IRQF_ONESHOT,
- 0, sec_irq_chip, &sec_pmic->irq_data);
+ 0, sec_irq_chip, &sec_irq_chip_data);
if (ret)
- return dev_err_probe(sec_pmic->dev, ret,
- "Failed to add %s IRQ chip\n",
- sec_irq_chip->name);
+ return dev_err_ptr_probe(sec_pmic->dev, ret, "Failed to add %s IRQ chip\n",
+ sec_irq_chip->name);
- /*
- * The rtc-s5m driver requests S2MPS14_IRQ_RTCA0 also for S2MPS11
- * so the interrupt number must be consistent.
- */
- BUILD_BUG_ON(((enum s2mps14_irq)S2MPS11_IRQ_RTCA0) != S2MPS14_IRQ_RTCA0);
-
- return 0;
+ return sec_irq_chip_data;
}
diff --git a/include/linux/mfd/samsung/core.h b/include/linux/mfd/samsung/core.h
index d785e101fe795a5d8f9cccf4ccc4232437e89416..c7c3c8cd8d5f99ef0cc3188e1c3b49031f4750f2 100644
--- a/include/linux/mfd/samsung/core.h
+++ b/include/linux/mfd/samsung/core.h
@@ -69,7 +69,6 @@ struct sec_pmic_dev {
int device_type;
int irq;
- struct regmap_irq_chip_data *irq_data;
};
struct sec_platform_data {
--
2.52.0.351.gbe84eed79e-goog
^ permalink raw reply related
* [PATCH RESEND v2 2/3] rtc: s5m: query platform device IRQ resource for alarm IRQ
From: André Draszik @ 2025-12-17 10:10 UTC (permalink / raw)
To: Krzysztof Kozlowski, Lee Jones, Alexandre Belloni
Cc: Peter Griffin, Tudor Ambarus, Will McVicker, Juan Yescas,
Douglas Anderson, kernel-team, Kaustabh Chakraborty, linux-kernel,
linux-samsung-soc, linux-rtc, André Draszik
In-Reply-To: <20251217-s5m-alarm-v2-0-b7bff003e94c@linaro.org>
The core driver now exposes the alarm IRQ as a resource, so we can drop
the lookup from here to simplify the code and make adding support for
additional variants easier in this driver.
Acked-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Signed-off-by: André Draszik <andre.draszik@linaro.org>
---
drivers/rtc/rtc-s5m.c | 21 ++++++++-------------
1 file changed, 8 insertions(+), 13 deletions(-)
diff --git a/drivers/rtc/rtc-s5m.c b/drivers/rtc/rtc-s5m.c
index a7220b4d0e8dd35786b060e2a4106e2a39fe743f..c6ed5a4ca8a0e4554b1c88c879b01fc384735007 100644
--- a/drivers/rtc/rtc-s5m.c
+++ b/drivers/rtc/rtc-s5m.c
@@ -15,7 +15,6 @@
#include <linux/rtc.h>
#include <linux/platform_device.h>
#include <linux/mfd/samsung/core.h>
-#include <linux/mfd/samsung/irq.h>
#include <linux/mfd/samsung/rtc.h>
#include <linux/mfd/samsung/s2mps14.h>
@@ -683,22 +682,18 @@ static int s5m_rtc_probe(struct platform_device *pdev)
case S2MPS15X:
regmap_cfg = &s2mps14_rtc_regmap_config;
info->regs = &s2mps15_rtc_regs;
- alarm_irq = S2MPS14_IRQ_RTCA0;
break;
case S2MPS14X:
regmap_cfg = &s2mps14_rtc_regmap_config;
info->regs = &s2mps14_rtc_regs;
- alarm_irq = S2MPS14_IRQ_RTCA0;
break;
case S2MPS13X:
regmap_cfg = &s2mps14_rtc_regmap_config;
info->regs = &s2mps13_rtc_regs;
- alarm_irq = S2MPS14_IRQ_RTCA0;
break;
case S5M8767X:
regmap_cfg = &s5m_rtc_regmap_config;
info->regs = &s5m_rtc_regs;
- alarm_irq = S5M8767_IRQ_RTCA1;
break;
default:
return dev_err_probe(&pdev->dev, -ENODEV,
@@ -719,7 +714,6 @@ static int s5m_rtc_probe(struct platform_device *pdev)
"Failed to allocate regmap\n");
} else if (device_type == S2MPG10) {
info->regs = &s2mpg10_rtc_regs;
- alarm_irq = S2MPG10_IRQ_RTCA0;
} else {
return dev_err_probe(&pdev->dev, -ENODEV,
"Unsupported device type %d\n",
@@ -730,13 +724,14 @@ static int s5m_rtc_probe(struct platform_device *pdev)
info->s5m87xx = s5m87xx;
info->device_type = device_type;
- if (s5m87xx->irq_data) {
- info->irq = regmap_irq_get_virq(s5m87xx->irq_data, alarm_irq);
- if (info->irq <= 0)
- return dev_err_probe(&pdev->dev, -EINVAL,
- "Failed to get virtual IRQ %d\n",
- alarm_irq);
- }
+ alarm_irq = platform_get_irq_byname_optional(pdev, "alarm");
+ if (alarm_irq > 0)
+ info->irq = alarm_irq;
+ else if (alarm_irq == -ENXIO)
+ info->irq = 0;
+ else
+ return dev_err_probe(&pdev->dev, alarm_irq ? : -EINVAL,
+ "IRQ 'alarm' not found\n");
platform_set_drvdata(pdev, info);
--
2.52.0.351.gbe84eed79e-goog
^ permalink raw reply related
* [PATCH RESEND v2 1/3] mfd: sec: add rtc alarm IRQ as platform device resource
From: André Draszik @ 2025-12-17 10:10 UTC (permalink / raw)
To: Krzysztof Kozlowski, Lee Jones, Alexandre Belloni
Cc: Peter Griffin, Tudor Ambarus, Will McVicker, Juan Yescas,
Douglas Anderson, kernel-team, Kaustabh Chakraborty, linux-kernel,
linux-samsung-soc, linux-rtc, André Draszik
In-Reply-To: <20251217-s5m-alarm-v2-0-b7bff003e94c@linaro.org>
By adding the RTC alarm IRQ to the MFD cell as a resource, the child
driver (rtc) can simply query that IRQ, instead of having a lookup
table itself.
This change therefore allows the child driver to be simplified with
regards to determining the alarm IRQ.
Signed-off-by: André Draszik <andre.draszik@linaro.org>
---
drivers/mfd/sec-common.c | 38 +++++++++++++++++++++++++++++---------
1 file changed, 29 insertions(+), 9 deletions(-)
diff --git a/drivers/mfd/sec-common.c b/drivers/mfd/sec-common.c
index 42d55e70e34c8d7cd68cddaecc88017e259365b4..77370db52a7ba81234136b29f85892f4b197f429 100644
--- a/drivers/mfd/sec-common.c
+++ b/drivers/mfd/sec-common.c
@@ -23,9 +23,13 @@
#include <linux/regmap.h>
#include "sec-core.h"
+static const struct resource s5m8767_rtc_resources[] = {
+ DEFINE_RES_IRQ_NAMED(S5M8767_IRQ_RTCA1, "alarm"),
+};
+
static const struct mfd_cell s5m8767_devs[] = {
MFD_CELL_NAME("s5m8767-pmic"),
- MFD_CELL_NAME("s5m-rtc"),
+ MFD_CELL_RES("s5m-rtc", s5m8767_rtc_resources),
MFD_CELL_OF("s5m8767-clk", NULL, NULL, 0, 0, "samsung,s5m8767-clk"),
};
@@ -33,50 +37,66 @@ static const struct mfd_cell s2dos05_devs[] = {
MFD_CELL_NAME("s2dos05-regulator"),
};
+static const struct resource s2mpg10_rtc_resources[] = {
+ DEFINE_RES_IRQ_NAMED(S2MPG10_IRQ_RTCA0, "alarm"),
+};
+
static const struct mfd_cell s2mpg10_devs[] = {
MFD_CELL_NAME("s2mpg10-meter"),
MFD_CELL_NAME("s2mpg10-regulator"),
- MFD_CELL_NAME("s2mpg10-rtc"),
+ MFD_CELL_RES("s2mpg10-rtc", s2mpg10_rtc_resources),
MFD_CELL_OF("s2mpg10-clk", NULL, NULL, 0, 0, "samsung,s2mpg10-clk"),
MFD_CELL_OF("s2mpg10-gpio", NULL, NULL, 0, 0, "samsung,s2mpg10-gpio"),
};
+static const struct resource s2mps11_rtc_resources[] = {
+ DEFINE_RES_IRQ_NAMED(S2MPS11_IRQ_RTCA0, "alarm"),
+};
+
static const struct mfd_cell s2mps11_devs[] = {
MFD_CELL_NAME("s2mps11-regulator"),
- MFD_CELL_NAME("s2mps14-rtc"),
+ MFD_CELL_RES("s2mps14-rtc", s2mps11_rtc_resources),
MFD_CELL_OF("s2mps11-clk", NULL, NULL, 0, 0, "samsung,s2mps11-clk"),
};
+static const struct resource s2mps14_rtc_resources[] = {
+ DEFINE_RES_IRQ_NAMED(S2MPS14_IRQ_RTCA0, "alarm"),
+};
+
static const struct mfd_cell s2mps13_devs[] = {
MFD_CELL_NAME("s2mps13-regulator"),
- MFD_CELL_NAME("s2mps13-rtc"),
+ MFD_CELL_RES("s2mps13-rtc", s2mps14_rtc_resources),
MFD_CELL_OF("s2mps13-clk", NULL, NULL, 0, 0, "samsung,s2mps13-clk"),
};
static const struct mfd_cell s2mps14_devs[] = {
MFD_CELL_NAME("s2mps14-regulator"),
- MFD_CELL_NAME("s2mps14-rtc"),
+ MFD_CELL_RES("s2mps14-rtc", s2mps14_rtc_resources),
MFD_CELL_OF("s2mps14-clk", NULL, NULL, 0, 0, "samsung,s2mps14-clk"),
};
static const struct mfd_cell s2mps15_devs[] = {
MFD_CELL_NAME("s2mps15-regulator"),
- MFD_CELL_NAME("s2mps15-rtc"),
+ MFD_CELL_RES("s2mps15-rtc", s2mps14_rtc_resources),
MFD_CELL_OF("s2mps13-clk", NULL, NULL, 0, 0, "samsung,s2mps13-clk"),
};
static const struct mfd_cell s2mpa01_devs[] = {
MFD_CELL_NAME("s2mpa01-pmic"),
- MFD_CELL_NAME("s2mps14-rtc"),
+ MFD_CELL_RES("s2mps14-rtc", s2mps14_rtc_resources),
};
static const struct mfd_cell s2mpu02_devs[] = {
MFD_CELL_NAME("s2mpu02-regulator"),
};
+static const struct resource s2mpu05_rtc_resources[] = {
+ DEFINE_RES_IRQ_NAMED(S2MPU05_IRQ_RTCA0, "alarm"),
+};
+
static const struct mfd_cell s2mpu05_devs[] = {
MFD_CELL_NAME("s2mpu05-regulator"),
- MFD_CELL_NAME("s2mps15-rtc"),
+ MFD_CELL_RES("s2mps15-rtc", s2mpu05_rtc_resources),
};
static void sec_pmic_dump_rev(struct sec_pmic_dev *sec_pmic)
@@ -220,7 +240,7 @@ int sec_pmic_probe(struct device *dev, int device_type, unsigned int irq,
sec_pmic->device_type);
}
ret = devm_mfd_add_devices(sec_pmic->dev, -1, sec_devs, num_sec_devs,
- NULL, 0, NULL);
+ NULL, 0, regmap_irq_get_domain(sec_pmic->irq_data));
if (ret)
return ret;
--
2.52.0.351.gbe84eed79e-goog
^ permalink raw reply related
* [PATCH RESEND v2 0/3] Samsung mfd/rtc driver alarm IRQ simplification
From: André Draszik @ 2025-12-17 10:10 UTC (permalink / raw)
To: Krzysztof Kozlowski, Lee Jones, Alexandre Belloni
Cc: Peter Griffin, Tudor Ambarus, Will McVicker, Juan Yescas,
Douglas Anderson, kernel-team, Kaustabh Chakraborty, linux-kernel,
linux-samsung-soc, linux-rtc, André Draszik
Hi,
With the attached patches the Samsung s5m RTC driver is simplified a
little bit with regards to alarm IRQ acquisition.
The end result is that instead of having a list of IRQ numbers for each
variant (and a BUILD_BUG_ON() to ensure consistency), the RTC driver
queries the 'alarm' platform resource from the parent (mfd cell).
Additionally, we can drop a now-useless field from runtime data,
reducing memory consumption slightly.
The attached patches must be applied in-order as patch 2 without 1 will
fail at runtime, and patch 3 without 2 will fail at build time. I would
expect them all to go via the MFD tree. Alternatively, they could be
applied individually to the respective kernel trees during multiple
kernel release cycles, but that seems a needless complication and
delay.
Signed-off-by: André Draszik <andre.draszik@linaro.org>
---
Changes in v2:
- rebase on top of https://lore.kernel.org/r/20251114-s2mpg10-chained-irq-v1-1-34ddfa49c4cd@linaro.org
- return struct regmap_irq_chip_data * in sec_irq_init() (Lee)
- collect tags
- Link to v1: https://lore.kernel.org/r/20251114-s5m-alarm-v1-0-c9b3bebae65f@linaro.org
---
André Draszik (3):
mfd: sec: add rtc alarm IRQ as platform device resource
rtc: s5m: query platform device IRQ resource for alarm IRQ
mfd: sec: drop now unused struct sec_pmic_dev::irq_data
drivers/mfd/sec-common.c | 45 ++++++++++++++++++++--------
drivers/mfd/sec-core.h | 2 +-
drivers/mfd/sec-irq.c | 63 ++++++++++++++++++----------------------
drivers/rtc/rtc-s5m.c | 21 +++++---------
include/linux/mfd/samsung/core.h | 1 -
5 files changed, 71 insertions(+), 61 deletions(-)
---
base-commit: 9ad5de6d54f306b2bbf7ceb27e67a60c58a71224
change-id: 20251114-s5m-alarm-3de705ea53ce
Best regards,
--
André Draszik <andre.draszik@linaro.org>
^ permalink raw reply
* Re: [PATCH v1 00/17] tee: Use bus callbacks instead of driver callbacks
From: Sumit Garg @ 2025-12-17 9:02 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Sumit Garg, Jens Wiklander, Olivia Mackall, Herbert Xu,
Clément Léger, Alexandre Belloni, Ard Biesheuvel,
Maxime Coquelin, Alexandre Torgue, Ilias Apalodimas, Jan Kiszka,
Sudeep Holla, Christophe JAILLET, Michael Chan, Pavan Chebbi,
Rafał Miłecki, James Bottomley, Jarkko Sakkinen,
Mimi Zohar, David Howells, Paul Moore, James Morris,
Serge E. Hallyn, Peter Huewe, op-tee, linux-kernel, linux-crypto,
linux-rtc, linux-efi, linux-stm32, linux-arm-kernel,
Cristian Marussi, arm-scmi, netdev, linux-mips, linux-integrity,
keyrings, linux-security-module, Jason Gunthorpe
In-Reply-To: <max5wxkcjjvnftwfwgymybwbnvf5s3ytwpy4oo5i74kfvnav4m@m2wasqyxsf4h>
On Wed, Dec 17, 2025 at 09:21:41AM +0100, Uwe Kleine-König wrote:
> Hello Sumit,
>
> On Wed, Dec 17, 2025 at 01:25:39PM +0530, Sumit Garg wrote:
> > On Tue, Dec 16, 2025 at 12:08:38PM +0100, Uwe Kleine-König wrote:
> > > On Tue, Dec 16, 2025 at 01:08:38PM +0530, Sumit Garg wrote:
> > > > On Mon, Dec 15, 2025 at 3:02 PM Uwe Kleine-König
> > > > <u.kleine-koenig@baylibre.com> wrote:
> > > > > - Why does optee_probe() in drivers/tee/optee/smc_abi.c unregister all
> > > > > optee devices in its error path (optee_unregister_devices())?
> > > >
> > > > This is mostly to take care of if any device got registered before the
> > > > failure occured. Let me know if you have a better way to address that.
> > >
> > > Without understanding the tee stuff, I'd say: Don't bother and only undo
> > > the things that probe did before the failure.
> >
> > True, but this is special case where if there is any leftover device
> > registered from the TEE implementation then it is likely going to cause
> > the corresponding kernel client driver crash.
>
> You are aware that this is racy? So if a driver crashes e.g. after
> teedev_close_context() it might happen that it is registered just after
> optee_unregister_devices() returns.
>
I see your point about the unavoidable race. Maybe it's better to not
try anything and let the kernel client driver fail.
-Sumit
^ permalink raw reply
* Re: [PATCH v1 00/17] tee: Use bus callbacks instead of driver callbacks
From: Uwe Kleine-König @ 2025-12-17 8:21 UTC (permalink / raw)
To: Sumit Garg
Cc: Sumit Garg, Jens Wiklander, Olivia Mackall, Herbert Xu,
Clément Léger, Alexandre Belloni, Ard Biesheuvel,
Maxime Coquelin, Alexandre Torgue, Ilias Apalodimas, Jan Kiszka,
Sudeep Holla, Christophe JAILLET, Michael Chan, Pavan Chebbi,
Rafał Miłecki, James Bottomley, Jarkko Sakkinen,
Mimi Zohar, David Howells, Paul Moore, James Morris,
Serge E. Hallyn, Peter Huewe, op-tee, linux-kernel, linux-crypto,
linux-rtc, linux-efi, linux-stm32, linux-arm-kernel,
Cristian Marussi, arm-scmi, netdev, linux-mips, linux-integrity,
keyrings, linux-security-module, Jason Gunthorpe
In-Reply-To: <aUJh--HGVeJWIilS@sumit-xelite>
[-- Attachment #1: Type: text/plain, Size: 1171 bytes --]
Hello Sumit,
On Wed, Dec 17, 2025 at 01:25:39PM +0530, Sumit Garg wrote:
> On Tue, Dec 16, 2025 at 12:08:38PM +0100, Uwe Kleine-König wrote:
> > On Tue, Dec 16, 2025 at 01:08:38PM +0530, Sumit Garg wrote:
> > > On Mon, Dec 15, 2025 at 3:02 PM Uwe Kleine-König
> > > <u.kleine-koenig@baylibre.com> wrote:
> > > > - Why does optee_probe() in drivers/tee/optee/smc_abi.c unregister all
> > > > optee devices in its error path (optee_unregister_devices())?
> > >
> > > This is mostly to take care of if any device got registered before the
> > > failure occured. Let me know if you have a better way to address that.
> >
> > Without understanding the tee stuff, I'd say: Don't bother and only undo
> > the things that probe did before the failure.
>
> True, but this is special case where if there is any leftover device
> registered from the TEE implementation then it is likely going to cause
> the corresponding kernel client driver crash.
You are aware that this is racy? So if a driver crashes e.g. after
teedev_close_context() it might happen that it is registered just after
optee_unregister_devices() returns.
Best regards
Uwe
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH v1 00/17] tee: Use bus callbacks instead of driver callbacks
From: Sumit Garg @ 2025-12-17 7:55 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Sumit Garg, Jens Wiklander, Olivia Mackall, Herbert Xu,
Clément Léger, Alexandre Belloni, Ard Biesheuvel,
Maxime Coquelin, Alexandre Torgue, Ilias Apalodimas, Jan Kiszka,
Sudeep Holla, Christophe JAILLET, Michael Chan, Pavan Chebbi,
Rafał Miłecki, James Bottomley, Jarkko Sakkinen,
Mimi Zohar, David Howells, Paul Moore, James Morris,
Serge E. Hallyn, Peter Huewe, op-tee, linux-kernel, linux-crypto,
linux-rtc, linux-efi, linux-stm32, linux-arm-kernel,
Cristian Marussi, arm-scmi, netdev, linux-mips, linux-integrity,
keyrings, linux-security-module, Jason Gunthorpe
In-Reply-To: <ayebinxqpcnl7hpa35ytrudiy7j75u5bdk3enlirkp5pevppeg@6mx6a5fwymwf>
On Tue, Dec 16, 2025 at 12:08:38PM +0100, Uwe Kleine-König wrote:
> Hello,
>
> On Tue, Dec 16, 2025 at 01:08:38PM +0530, Sumit Garg wrote:
> > On Mon, Dec 15, 2025 at 3:02 PM Uwe Kleine-König
> > <u.kleine-koenig@baylibre.com> wrote:
> > > On Mon, Dec 15, 2025 at 04:54:11PM +0900, Sumit Garg wrote:
> > > > Feel free to make the tee_bus_type private as the last patch in the series
> > > > such that any followup driver follows this clean approach.
> > >
> > > There is a bit more to do for that than I'm willing to invest. With my
> > > patch series applied `tee_bus_type` is still used in
> > > drivers/tee/optee/device.c and drivers/tee/tee_core.c.
> >
> > Oh I see, I guess we need to come with some helpers around device
> > register/unregister from TEE subsystem as well. Let's plan that for a
> > followup patch-set, I don't want this patch-set to be bloated more.
>
> Don't consider me in for that. But it sounds like a nice addition.
>
No worries, the current cleanup is fine for now.
> > > Maybe it's
> > > sensible to merge these two files into a single one.
> >
> > It's not possible as the design for TEE bus is to have TEE
> > implementation drivers like OP-TEE, AMD-TEE, TS-TEE, QTEE and so on to
> > register devices on the bus.
>
> So only OP-TEE uses the bus for devices and the other *-TEE don't. Also
> sounds like something worth to be fixed.
The TEE bus is common for all the TEE implementation drivers which need
to support kernel TEE client drivers. I am aware there will be QTEE and
TS-TEE from past discussion that they will be exposing devices on the
TEE bus.
>
> > > The things I wonder about additionally are:
> > >
> > > - if CONFIG_OPTEE=n and CONFIG_TEE=y|m the tee bus is only used for
> > > drivers but not devices.
> >
> > Yeah since the devices are rather added by the TEE implementation driver.
> >
> > >
> > > - optee_register_device() calls device_create_file() on
> > > &optee_device->dev after device_register(&optee_device->dev).
> > > (Attention half-knowledge!) I think device_create_file() should not
> > > be called on an already registered device (or you have to send a
> > > uevent afterwards). This should probably use type attribute groups.
> > > (Or the need_supplicant attribute should be dropped as it isn't very
> > > useful. This would maybe be considered an ABI change however.)
> >
> > The reasoning for this attribute should be explained by commit:
> > 7269cba53d90 ("tee: optee: Fix supplicant based device enumeration").
> > In summary it's due to a weird dependency for devices we have with the
> > user-space daemon: tee-supplicant.
>
> From reading that once I don't understand it. (But no need to explain
> :-)
>
> Still the file should better be added before device_add() is called.
Noted, let me see if I can get to fix this until someone jumps in before
me.
>
> > > - Why does optee_probe() in drivers/tee/optee/smc_abi.c unregister all
> > > optee devices in its error path (optee_unregister_devices())?
> >
> > This is mostly to take care of if any device got registered before the
> > failure occured. Let me know if you have a better way to address that.
>
> Without understanding the tee stuff, I'd say: Don't bother and only undo
> the things that probe did before the failure.
>
True, but this is special case where if there is any leftover device
registered from the TEE implementation then it is likely going to cause
the corresponding kernel client driver crash.
-Sumit
^ permalink raw reply
* Re: [PATCH v2 0/3] Samsung mfd/rtc driver alarm IRQ simplification
From: Lee Jones @ 2025-12-16 16:20 UTC (permalink / raw)
To: André Draszik
Cc: Krzysztof Kozlowski, Alexandre Belloni, Peter Griffin,
Tudor Ambarus, Will McVicker, Juan Yescas, Douglas Anderson,
kernel-team, Kaustabh Chakraborty, linux-kernel,
linux-samsung-soc, linux-rtc
In-Reply-To: <20251126140409.GC3070764@google.com>
On Wed, 26 Nov 2025, Lee Jones wrote:
> On Wed, 26 Nov 2025, Lee Jones wrote:
>
> > On Thu, 20 Nov 2025, André Draszik wrote:
> >
> > > Hi,
> > >
> > > With the attached patches the Samsung s5m RTC driver is simplified a
> > > little bit with regards to alarm IRQ acquisition.
> > >
> > > The end result is that instead of having a list of IRQ numbers for each
> > > variant (and a BUILD_BUG_ON() to ensure consistency), the RTC driver
> > > queries the 'alarm' platform resource from the parent (mfd cell).
> > >
> > > Additionally, we can drop a now-useless field from runtime data,
> > > reducing memory consumption slightly.
> > >
> > > The attached patches must be applied in-order as patch 2 without 1 will
> > > fail at runtime, and patch 3 without 2 will fail at build time. I would
> > > expect them all to go via the MFD tree. Alternatively, they could be
> > > applied individually to the respective kernel trees during multiple
> > > kernel release cycles, but that seems a needless complication and
> > > delay.
> > >
> > > Signed-off-by: André Draszik <andre.draszik@linaro.org>
> > > ---
> > > Changes in v2:
> > > - rebase on top of https://lore.kernel.org/r/20251114-s2mpg10-chained-irq-v1-1-34ddfa49c4cd@linaro.org
> > > - return struct regmap_irq_chip_data * in sec_irq_init() (Lee)
> > > - collect tags
> > > - Link to v1: https://lore.kernel.org/r/20251114-s5m-alarm-v1-0-c9b3bebae65f@linaro.org
> > >
> > > ---
> > > André Draszik (3):
> > > mfd: sec: add rtc alarm IRQ as platform device resource
> > > rtc: s5m: query platform device IRQ resource for alarm IRQ
> > > mfd: sec: drop now unused struct sec_pmic_dev::irq_data
> > >
> > > drivers/mfd/sec-common.c | 45 ++++++++++++++++++++--------
> > > drivers/mfd/sec-core.h | 2 +-
> > > drivers/mfd/sec-irq.c | 63 ++++++++++++++++++----------------------
> > > drivers/rtc/rtc-s5m.c | 21 +++++---------
> > > include/linux/mfd/samsung/core.h | 1 -
> > > 5 files changed, 71 insertions(+), 61 deletions(-)
> >
> > The MFD parts look okay to me.
> >
> > Once we have the RTC Ack, I'll merge this and send out a PR.
>
> Ah, I see it. Apologies.
>
> It's too late in the cycle to take this now anyway.
>
> It's on my radar for when -rc1 is released.
This does not seem to apply well on v6.19-rc1.
Please rebase and send as a [RESEND].
-----
% cat drivers/mfd/sec-irq.c.rej
diff a/drivers/mfd/sec-irq.c b/drivers/mfd/sec-irq.c (rejected hunks)
@@ -302,27 +304,28 @@ static int sec_irq_init_s2mpg1x(struct sec_pmic_dev *sec_pmic)
chained_pirq = S2MPG10_COMMON_IRQ_PMIC;
break;
default:
- return dev_err_probe(sec_pmic->dev, -EINVAL, "Unsupported device type %d\n",
- sec_pmic->device_type);
+ return dev_err_ptr_probe(sec_pmic->dev, -EINVAL, "Unsupported device type %d\n",
+ sec_pmic->device_type);
};
regmap_common = dev_get_regmap(sec_pmic->dev, "common");
if (!regmap_common)
- return dev_err_probe(sec_pmic->dev, -EINVAL, "No 'common' regmap %d\n",
- sec_pmic->device_type);
+ return dev_err_ptr_probe(sec_pmic->dev, -EINVAL, "No 'common' regmap %d\n",
+ sec_pmic->device_type);
ret = devm_regmap_add_irq_chip(sec_pmic->dev, regmap_common, sec_pmic->irq, IRQF_ONESHOT, 0,
irq_chip, &irq_data);
if (ret)
- return dev_err_probe(sec_pmic->dev, ret, "Failed to add %s IRQ chip\n",
- irq_chip->name);
+ return dev_err_ptr_probe(sec_pmic->dev, ret, "Failed to add %s IRQ chip\n",
+ irq_chip->name);
return s2mpg1x_add_chained_irq_chip(sec_pmic->dev, sec_pmic->regmap_pmic, chained_pirq,
- irq_data, chained_irq_chip, &sec_pmic->irq_data);
+ irq_data, chained_irq_chip);
}
-int sec_irq_init(struct sec_pmic_dev *sec_pmic)
+struct regmap_irq_chip_data *sec_irq_init(struct sec_pmic_dev *sec_pmic)
{
+ struct regmap_irq_chip_data *sec_irq_chip_data;
const struct regmap_irq_chip *sec_irq_chip;
int ret;
--
Lee Jones [李琼斯]
^ permalink raw reply
* Re: [PATCH] rtc: interface: Alarm race handling should not discard preceding error
From: Nick Bowler @ 2025-12-16 15:09 UTC (permalink / raw)
To: Thorsten Leemhuis
Cc: Anthony Pighin (Nokia), linux-rtc@vger.kernel.org,
alexandre.belloni@bootlin.com, Esben Haabendal,
Linux kernel regressions list
In-Reply-To: <80e7450d-d842-49ca-8219-a995c8ce8bfe@leemhuis.info>
On Tue, Dec 16, 2025 at 11:51:30AM +0100, Thorsten Leemhuis wrote:
> Lo! FWIW, Nick Bowler (now CCed) reported that below patch fixes a
> regression for him caused by the commit from Esben (now also CCed) the
> Fixes: tag mentions. See "hwclock busted w/ M48T59 RTC (regression)" for
> details:
> https://lore.kernel.org/all/2t6bhs4udbu55ctbemkhlluchz2exrwown7kmu2gss6zukaxdm@ughygemahmem/
> and
>
> Nick, could you maybe provide a tested-by tag here? Maybe that would
> motivate someone to get this en route to mainline.
>
> Adding a "Cc: <stable@vger.kernel.org>" would be great, too, as Nick
> encountered this on earlier series, as it was backported all the way to
> 5.15.y
It was backported to 5.10.y and 5.4.y too, but only after I had reported
this regression back in October (and I guess 5.4.y is EOL now).
Tested-by: Nick Bowler <nbowler@draconx.ca>
Thanks,
Nick
^ permalink raw reply
* Re: (subset) [PATCH v6 0/7] mfd: macsmc: add rtc, hwmon and hid subdevices
From: Sven Peter @ 2025-12-16 12:37 UTC (permalink / raw)
To: Janne Grunau, Alyssa Rosenzweig, Neal Gompa, Lee Jones,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Alexandre Belloni,
Jean Delvare, Guenter Roeck, Dmitry Torokhov, Jonathan Corbet,
James Calligeros
Cc: Sven Peter, asahi, linux-arm-kernel, devicetree, linux-kernel,
linux-rtc, linux-hwmon, linux-input, linux-doc, Hector Martin
In-Reply-To: <20251215-macsmc-subdevs-v6-0-0518cb5f28ae@gmail.com>
On Mon, 15 Dec 2025 19:37:44 +1000, James Calligeros wrote:
> This series adds support for the remaining SMC subdevices. These are the
> RTC, hwmon, and HID devices. They are being submitted together as the RTC
> and hwmon drivers both require changes to the SMC DT schema.
>
> The RTC driver is responsible for getting and setting the system clock,
> and requires an NVMEM cell. This series replaces Sven's original RTC driver
> submission [1].
>
> [...]
Applied to local tree (apple-soc/dt-6.20), thanks!
[6/7] arm64: dts: apple: t8103,t60xx,t8112: Add SMC RTC node
https://github.com/AsahiLinux/linux/commit/faf317d4c705
Best regards,
--
Sven Peter <sven@kernel.org>
^ permalink raw reply
* Re: [PATCH v1 00/17] tee: Use bus callbacks instead of driver callbacks
From: Uwe Kleine-König @ 2025-12-16 11:08 UTC (permalink / raw)
To: Sumit Garg
Cc: Sumit Garg, Jens Wiklander, Olivia Mackall, Herbert Xu,
Clément Léger, Alexandre Belloni, Ard Biesheuvel,
Maxime Coquelin, Alexandre Torgue, Ilias Apalodimas, Jan Kiszka,
Sudeep Holla, Christophe JAILLET, Michael Chan, Pavan Chebbi,
Rafał Miłecki, James Bottomley, Jarkko Sakkinen,
Mimi Zohar, David Howells, Paul Moore, James Morris,
Serge E. Hallyn, Peter Huewe, op-tee, linux-kernel, linux-crypto,
linux-rtc, linux-efi, linux-stm32, linux-arm-kernel,
Cristian Marussi, arm-scmi, netdev, linux-mips, linux-integrity,
keyrings, linux-security-module, Jason Gunthorpe
In-Reply-To: <CAGptzHOOqLhBnAXDURAzkgckUvRr__UuF1S_7MLV0u-ZxYEdyA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2840 bytes --]
Hello,
On Tue, Dec 16, 2025 at 01:08:38PM +0530, Sumit Garg wrote:
> On Mon, Dec 15, 2025 at 3:02 PM Uwe Kleine-König
> <u.kleine-koenig@baylibre.com> wrote:
> > On Mon, Dec 15, 2025 at 04:54:11PM +0900, Sumit Garg wrote:
> > > Feel free to make the tee_bus_type private as the last patch in the series
> > > such that any followup driver follows this clean approach.
> >
> > There is a bit more to do for that than I'm willing to invest. With my
> > patch series applied `tee_bus_type` is still used in
> > drivers/tee/optee/device.c and drivers/tee/tee_core.c.
>
> Oh I see, I guess we need to come with some helpers around device
> register/unregister from TEE subsystem as well. Let's plan that for a
> followup patch-set, I don't want this patch-set to be bloated more.
Don't consider me in for that. But it sounds like a nice addition.
> > Maybe it's
> > sensible to merge these two files into a single one.
>
> It's not possible as the design for TEE bus is to have TEE
> implementation drivers like OP-TEE, AMD-TEE, TS-TEE, QTEE and so on to
> register devices on the bus.
So only OP-TEE uses the bus for devices and the other *-TEE don't. Also
sounds like something worth to be fixed.
> > The things I wonder about additionally are:
> >
> > - if CONFIG_OPTEE=n and CONFIG_TEE=y|m the tee bus is only used for
> > drivers but not devices.
>
> Yeah since the devices are rather added by the TEE implementation driver.
>
> >
> > - optee_register_device() calls device_create_file() on
> > &optee_device->dev after device_register(&optee_device->dev).
> > (Attention half-knowledge!) I think device_create_file() should not
> > be called on an already registered device (or you have to send a
> > uevent afterwards). This should probably use type attribute groups.
> > (Or the need_supplicant attribute should be dropped as it isn't very
> > useful. This would maybe be considered an ABI change however.)
>
> The reasoning for this attribute should be explained by commit:
> 7269cba53d90 ("tee: optee: Fix supplicant based device enumeration").
> In summary it's due to a weird dependency for devices we have with the
> user-space daemon: tee-supplicant.
From reading that once I don't understand it. (But no need to explain
:-)
Still the file should better be added before device_add() is called.
> > - Why does optee_probe() in drivers/tee/optee/smc_abi.c unregister all
> > optee devices in its error path (optee_unregister_devices())?
>
> This is mostly to take care of if any device got registered before the
> failure occured. Let me know if you have a better way to address that.
Without understanding the tee stuff, I'd say: Don't bother and only undo
the things that probe did before the failure.
Best regards
Uwe
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH] rtc: interface: Alarm race handling should not discard preceding error
From: Thorsten Leemhuis @ 2025-12-16 10:51 UTC (permalink / raw)
To: Anthony Pighin (Nokia), linux-rtc@vger.kernel.org
Cc: alexandre.belloni@bootlin.com, Nick Bowler, Esben Haabendal,
Linux kernel regressions list
In-Reply-To: <BN0PR08MB6951415A751F236375A2945683D1A@BN0PR08MB6951.namprd08.prod.outlook.com>
Lo! FWIW, Nick Bowler (now CCed) reported that below patch fixes a
regression for him caused by the commit from Esben (now also CCed) the
Fixes: tag mentions. See "hwclock busted w/ M48T59 RTC (regression)" for
details:
https://lore.kernel.org/all/2t6bhs4udbu55ctbemkhlluchz2exrwown7kmu2gss6zukaxdm@ughygemahmem/
and
Nick, could you maybe provide a tested-by tag here? Maybe that would
motivate someone to get this en route to mainline.
Adding a "Cc: <stable@vger.kernel.org>" would be great, too, as Nick
encountered this on earlier series, as it was backported all the way to
5.15.y
Ciao, Thorsten
On 11/25/25 18:35, Anthony Pighin (Nokia) wrote:
> Commit 795cda8338ea ("rtc: interface: Fix long-standing race when setting
> alarm") should not discard any errors from the preceding validations.
>
> Prior to that commit, if the alarm feature was disabled, or the
> set_alarm failed, a meaningful error code would be returned to the
> caller for further action.
>
> After, more often than not, the __rtc_read_time will cause a success
> return code instead, misleading the caller.
>
> An example of this is when timer_enqueue is called for a rtc-abx080x
> device. Since that driver does not clear the alarm feature bit, but
> instead relies on the set_alarm operation to return invalid, the discard
> of the return code causes very different behaviour; i.e.
> hwclock: select() to /dev/rtc0 to wait for clock tick timed out
>
> Fixes: 795cda8338ea ("rtc: interface: Fix long-standing race when setting alarm")
> Signed-off-by: Anthony Pighin <anthony.pighin@nokia.com>
> ---
> drivers/rtc/interface.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/rtc/interface.c b/drivers/rtc/interface.c
> index b8b298efd9a9..1906f4884a83 100644
> --- a/drivers/rtc/interface.c
> +++ b/drivers/rtc/interface.c
> @@ -457,7 +457,7 @@ static int __rtc_set_alarm(struct rtc_device *rtc, struct rtc_wkalrm *alarm)
> * are in, we can return -ETIME to signal that the timer has already
> * expired, which is true in both cases.
> */
> - if ((scheduled - now) <= 1) {
> + if (!err && (scheduled - now) <= 1) {
> err = __rtc_read_time(rtc, &tm);
> if (err)
> return err;
> --
> 2.43.0
^ permalink raw reply
* Re: [PATCH v1 00/17] tee: Use bus callbacks instead of driver callbacks
From: Sumit Garg @ 2025-12-16 7:38 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Sumit Garg, Jens Wiklander, Olivia Mackall, Herbert Xu,
Clément Léger, Alexandre Belloni, Ard Biesheuvel,
Maxime Coquelin, Alexandre Torgue, Ilias Apalodimas, Jan Kiszka,
Sudeep Holla, Christophe JAILLET, Michael Chan, Pavan Chebbi,
Rafał Miłecki, James Bottomley, Jarkko Sakkinen,
Mimi Zohar, David Howells, Paul Moore, James Morris,
Serge E. Hallyn, Peter Huewe, op-tee, linux-kernel, linux-crypto,
linux-rtc, linux-efi, linux-stm32, linux-arm-kernel,
Cristian Marussi, arm-scmi, netdev, linux-mips, linux-integrity,
keyrings, linux-security-module, Jason Gunthorpe
In-Reply-To: <dhunzydod4d7vj73llpuqemxb5er2ja4emxusr66irwf77jhhb@es4yd2axzl25>
Hi Uwe,
On Mon, Dec 15, 2025 at 3:02 PM Uwe Kleine-König
<u.kleine-koenig@baylibre.com> wrote:
>
> Hello Sumit,
>
> On Mon, Dec 15, 2025 at 04:54:11PM +0900, Sumit Garg wrote:
> > On Thu, Dec 11, 2025 at 06:14:54PM +0100, Uwe Kleine-König wrote:
> > > Hello,
> > >
> > > the objective of this series is to make tee driver stop using callbacks
> > > in struct device_driver. These were superseded by bus methods in 2006
> > > (commit 594c8281f905 ("[PATCH] Add bus_type probe, remove, shutdown
> > > methods.")) but nobody cared to convert all subsystems accordingly.
> > >
> > > Here the tee drivers are converted. The first commit is somewhat
> > > unrelated, but simplifies the conversion (and the drivers). It
> > > introduces driver registration helpers that care about setting the bus
> > > and owner. (The latter is missing in all drivers, so by using these
> > > helpers the drivers become more correct.)
> > >
> > > The patches #4 - #17 depend on the first two, so if they should be
> > > applied to their respective subsystem trees these must contain the first
> > > two patches first.
> >
> > Thanks Uwe for your efforts to clean up the boilerplate code for TEE bus
> > drivers.
>
> Thanks for your feedback. I will prepare a v2 and address your comments
> (whitespace issues and wrong callback in the shutdown method).
>
> > > Note that after patch #2 is applied, unconverted drivers provoke a
> > > warning in driver_register(), so it would be good for the user
> > > experience if the whole series goes in during a single merge window.
> >
> > +1
> >
> > I suggest the whole series goes via the Jens tree since there shouldn't
> > be any chances for conflict here.
> >
> > > So
> > > I guess an immutable branch containing the frist three patches that can
> > > be merged into the other subsystem trees would be sensible.
> > >
> > > After all patches are applied, tee_bus_type can be made private to
> > > drivers/tee as it's not used in other places any more.
> > >
> >
> > Feel free to make the tee_bus_type private as the last patch in the series
> > such that any followup driver follows this clean approach.
>
> There is a bit more to do for that than I'm willing to invest. With my
> patch series applied `tee_bus_type` is still used in
> drivers/tee/optee/device.c and drivers/tee/tee_core.c.
Oh I see, I guess we need to come with some helpers around device
register/unregister from TEE subsystem as well. Let's plan that for a
followup patch-set, I don't want this patch-set to be bloated more.
> Maybe it's
> sensible to merge these two files into a single one.
It's not possible as the design for TEE bus is to have TEE
implementation drivers like OP-TEE, AMD-TEE, TS-TEE, QTEE and so on to
register devices on the bus.
>
> The things I wonder about additionally are:
>
> - if CONFIG_OPTEE=n and CONFIG_TEE=y|m the tee bus is only used for
> drivers but not devices.
Yeah since the devices are rather added by the TEE implementation driver.
>
> - optee_register_device() calls device_create_file() on
> &optee_device->dev after device_register(&optee_device->dev).
> (Attention half-knowledge!) I think device_create_file() should not
> be called on an already registered device (or you have to send a
> uevent afterwards). This should probably use type attribute groups.
> (Or the need_supplicant attribute should be dropped as it isn't very
> useful. This would maybe be considered an ABI change however.)
The reasoning for this attribute should be explained by commit:
7269cba53d90 ("tee: optee: Fix supplicant based device enumeration").
In summary it's due to a weird dependency for devices we have with the
user-space daemon: tee-supplicant.
>
> - Why does optee_probe() in drivers/tee/optee/smc_abi.c unregister all
> optee devices in its error path (optee_unregister_devices())?
This is mostly to take care of if any device got registered before the
failure occured. Let me know if you have a better way to address that.
-Sumit
^ permalink raw reply
* [PATCH v2 07/17] rtc: optee: Make use of tee bus methods
From: Uwe Kleine-König @ 2025-12-15 14:16 UTC (permalink / raw)
To: Jens Wiklander, Clément Léger, Alexandre Belloni
Cc: Sumit Garg, op-tee, linux-rtc, linux-kernel, Sumit Garg
In-Reply-To: <cover.1765791463.git.u.kleine-koenig@baylibre.com>
The tee bus got dedicated callbacks for probe and remove. Make use of
these. This fixes a runtime warning about the driver needing to be
converted to the bus methods.
Reviewed-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
Acked-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
---
drivers/rtc/rtc-optee.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/rtc/rtc-optee.c b/drivers/rtc/rtc-optee.c
index f924a729ead0..eefde789d194 100644
--- a/drivers/rtc/rtc-optee.c
+++ b/drivers/rtc/rtc-optee.c
@@ -547,9 +547,9 @@ static int optee_ctx_match(struct tee_ioctl_version_data *ver, const void *data)
return 0;
}
-static int optee_rtc_probe(struct device *dev)
+static int optee_rtc_probe(struct tee_client_device *rtc_device)
{
- struct tee_client_device *rtc_device = to_tee_client_device(dev);
+ struct device *dev = &rtc_device->dev;
struct tee_ioctl_open_session_arg sess2_arg = {0};
struct tee_ioctl_open_session_arg sess_arg = {0};
struct optee_rtc *priv;
@@ -682,8 +682,9 @@ static int optee_rtc_probe(struct device *dev)
return err;
}
-static int optee_rtc_remove(struct device *dev)
+static void optee_rtc_remove(struct tee_client_device *rtc_device)
{
+ struct device *dev = &rtc_device->dev;
struct optee_rtc *priv = dev_get_drvdata(dev);
if (priv->features & TA_RTC_FEATURE_ALARM) {
@@ -696,8 +697,6 @@ static int optee_rtc_remove(struct device *dev)
tee_shm_free(priv->shm);
tee_client_close_session(priv->ctx, priv->session_id);
tee_client_close_context(priv->ctx);
-
- return 0;
}
static int optee_rtc_suspend(struct device *dev)
@@ -724,10 +723,10 @@ MODULE_DEVICE_TABLE(tee, optee_rtc_id_table);
static struct tee_client_driver optee_rtc_driver = {
.id_table = optee_rtc_id_table,
+ .probe = optee_rtc_probe,
+ .remove = optee_rtc_remove,
.driver = {
.name = "optee_rtc",
- .probe = optee_rtc_probe,
- .remove = optee_rtc_remove,
.pm = pm_sleep_ptr(&optee_rtc_pm_ops),
},
};
--
2.47.3
^ permalink raw reply related
* [PATCH v2 06/17] rtc: optee: Migrate to use tee specific driver registration function
From: Uwe Kleine-König @ 2025-12-15 14:16 UTC (permalink / raw)
To: Jens Wiklander, Clément Léger, Alexandre Belloni
Cc: Sumit Garg, op-tee, linux-rtc, linux-kernel, Sumit Garg
In-Reply-To: <cover.1765791463.git.u.kleine-koenig@baylibre.com>
The tee subsystem recently got a set of dedicated functions to register
(and unregister) a tee driver. Make use of them. These care for setting the
driver's bus (so the explicit assignment can be dropped) and the driver
owner (which is an improvement this driver benefits from).
Reviewed-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
Acked-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
---
drivers/rtc/rtc-optee.c | 14 +-------------
1 file changed, 1 insertion(+), 13 deletions(-)
diff --git a/drivers/rtc/rtc-optee.c b/drivers/rtc/rtc-optee.c
index 184c6d142801..f924a729ead0 100644
--- a/drivers/rtc/rtc-optee.c
+++ b/drivers/rtc/rtc-optee.c
@@ -726,25 +726,13 @@ static struct tee_client_driver optee_rtc_driver = {
.id_table = optee_rtc_id_table,
.driver = {
.name = "optee_rtc",
- .bus = &tee_bus_type,
.probe = optee_rtc_probe,
.remove = optee_rtc_remove,
.pm = pm_sleep_ptr(&optee_rtc_pm_ops),
},
};
-static int __init optee_rtc_mod_init(void)
-{
- return driver_register(&optee_rtc_driver.driver);
-}
-
-static void __exit optee_rtc_mod_exit(void)
-{
- driver_unregister(&optee_rtc_driver.driver);
-}
-
-module_init(optee_rtc_mod_init);
-module_exit(optee_rtc_mod_exit);
+module_tee_client_driver(optee_rtc_driver);
MODULE_LICENSE("GPL v2");
MODULE_AUTHOR("Clément Léger <clement.leger@bootlin.com>");
--
2.47.3
^ permalink raw reply related
* [PATCH v2 00/17] tee: Use bus callbacks instead of driver callbacks
From: Uwe Kleine-König @ 2025-12-15 14:16 UTC (permalink / raw)
To: Jens Wiklander, Jonathan Corbet, Sumit Garg, Olivia Mackall,
Herbert Xu, Clément Léger, Alexandre Belloni,
Ard Biesheuvel, Maxime Coquelin, Alexandre Torgue, Sumit Garg,
Ilias Apalodimas, Jan Kiszka, Uwe Kleine-König, Sudeep Holla,
Christophe JAILLET, Rafał Miłecki, Michael Chan,
Pavan Chebbi, James Bottomley, Jarkko Sakkinen, Mimi Zohar,
David Howells, Paul Moore, James Morris, Serge E. Hallyn,
Peter Huewe
Cc: op-tee, linux-kernel, linux-doc, linux-crypto, linux-rtc,
linux-efi, linux-stm32, linux-arm-kernel, Cristian Marussi,
arm-scmi, linux-mips, netdev, linux-integrity, keyrings,
linux-security-module, Jason Gunthorpe
Hello,
the objective of this series is to make tee driver stop using callbacks
in struct device_driver. These were superseded by bus methods in 2006
(commit 594c8281f905 ("[PATCH] Add bus_type probe, remove, shutdown
methods.")) but nobody cared to convert all subsystems accordingly.
Here the tee drivers are converted. The first commit is somewhat
unrelated, but simplifies the conversion (and the drivers). It
introduces driver registration helpers that care about setting the bus
and owner. (The latter is missing in all drivers, so by using these
helpers the drivers become more correct.)
v1 of this series is available at
https://lore.kernel.org/all/cover.1765472125.git.u.kleine-koenig@baylibre.com
Changes since v1:
- rebase to v6.19-rc1 (no conflicts)
- add tags received so far
- fix whitespace issues pointed out by Sumit Garg
- fix shutdown callback to shutdown and not remove
As already noted in v1's cover letter, this series should go in during a
single merge window as there are runtime warnings when the series is
only applied partially. Sumit Garg suggested to apply the whole series
via Jens Wiklander's tree.
If this is done the dependencies in this series are honored, in case the
plan changes: Patches #4 - #17 depend on the first two.
Note this series is only build tested.
Uwe Kleine-König (17):
tee: Add some helpers to reduce boilerplate for tee client drivers
tee: Add probe, remove and shutdown bus callbacks to tee_client_driver
tee: Adapt documentation to cover recent additions
hwrng: optee - Make use of module_tee_client_driver()
hwrng: optee - Make use of tee bus methods
rtc: optee: Migrate to use tee specific driver registration function
rtc: optee: Make use of tee bus methods
efi: stmm: Make use of module_tee_client_driver()
efi: stmm: Make use of tee bus methods
firmware: arm_scmi: optee: Make use of module_tee_client_driver()
firmware: arm_scmi: Make use of tee bus methods
firmware: tee_bnxt: Make use of module_tee_client_driver()
firmware: tee_bnxt: Make use of tee bus methods
KEYS: trusted: Migrate to use tee specific driver registration
function
KEYS: trusted: Make use of tee bus methods
tpm/tpm_ftpm_tee: Make use of tee specific driver registration
tpm/tpm_ftpm_tee: Make use of tee bus methods
Documentation/driver-api/tee.rst | 18 +----
drivers/char/hw_random/optee-rng.c | 26 ++----
drivers/char/tpm/tpm_ftpm_tee.c | 31 +++++---
drivers/firmware/arm_scmi/transports/optee.c | 32 +++-----
drivers/firmware/broadcom/tee_bnxt_fw.c | 30 ++-----
drivers/firmware/efi/stmm/tee_stmm_efi.c | 25 ++----
drivers/rtc/rtc-optee.c | 27 ++-----
drivers/tee/tee_core.c | 84 ++++++++++++++++++++
include/linux/tee_drv.h | 12 +++
security/keys/trusted-keys/trusted_tee.c | 17 ++--
10 files changed, 164 insertions(+), 138 deletions(-)
base-commit: 8f0b4cce4481fb22653697cced8d0d04027cb1e8
--
2.47.3
^ permalink raw reply
* Re: [PATCH RESEND v6 00/17] Support ROHM BD72720 PMIC
From: Matti Vaittinen @ 2025-12-15 13:27 UTC (permalink / raw)
To: Matti Vaittinen
Cc: Lee Jones, Pavel Machek, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Sebastian Reichel, Liam Girdwood, Mark Brown,
Michael Turquette, Stephen Boyd, Linus Walleij,
Bartosz Golaszewski, Alexandre Belloni, linux-leds, devicetree,
linux-kernel, linux-pm, linux-clk, linux-gpio, linux-rtc,
Andreas Kemnade
In-Reply-To: <cover.1765804226.git.mazziesaccount@gmail.com>
On 15/12/2025 15:16, Matti Vaittinen wrote:
> Resending the v6
// snip
> This series depends on
> 5bff79dad20a ("power: supply: Add bd718(15/28/78) charger driver")
> which is in power-supply tree, for-next. Thus, the series is based on
> it.
I forgot to update this. The 5bff79dad20a is now in v6.19-rc1, and as I
wrote above, this is now based on v6.19-rc1, not power-supply tree's
for-next.
Yours,
-- Matti
---
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland
~~ When things go utterly wrong vim users can always type :help! ~~
^ permalink raw reply
* [PATCH RESEND v6 17/17] MAINTAINERS: Add ROHM BD72720 PMIC
From: Matti Vaittinen @ 2025-12-15 13:21 UTC (permalink / raw)
To: Matti Vaittinen, Matti Vaittinen
Cc: Lee Jones, Pavel Machek, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Sebastian Reichel, Liam Girdwood, Mark Brown,
Michael Turquette, Stephen Boyd, Matti Vaittinen, Linus Walleij,
Bartosz Golaszewski, Alexandre Belloni, linux-leds, devicetree,
linux-kernel, linux-pm, linux-clk, linux-gpio, linux-rtc,
Andreas Kemnade
In-Reply-To: <cover.1765804226.git.mazziesaccount@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1022 bytes --]
From: Matti Vaittinen <mazziesaccount@gmail.com>
Add the ROHM BD72720 PMIC driver files to be maintained by undersigned.
Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
---
Revision history:
RFCv1 =>:
- No changes
---
MAINTAINERS | 2 ++
1 file changed, 2 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 5b11839cba9d..23bf05492d34 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -22745,6 +22745,7 @@ S: Supported
F: drivers/clk/clk-bd718x7.c
F: drivers/gpio/gpio-bd71815.c
F: drivers/gpio/gpio-bd71828.c
+F: drivers/gpio/gpio-bd72720.c
F: drivers/mfd/rohm-bd71828.c
F: drivers/mfd/rohm-bd718x7.c
F: drivers/mfd/rohm-bd9576.c
@@ -22761,6 +22762,7 @@ F: drivers/watchdog/bd96801_wdt.c
F: include/linux/mfd/rohm-bd71815.h
F: include/linux/mfd/rohm-bd71828.h
F: include/linux/mfd/rohm-bd718x7.h
+F: include/linux/mfd/rohm-bd72720.h
F: include/linux/mfd/rohm-bd957x.h
F: include/linux/mfd/rohm-bd96801.h
F: include/linux/mfd/rohm-bd96802.h
--
2.52.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply related
* [PATCH RESEND v6 16/17] power: supply: bd71828-power: Support ROHM BD72720
From: Matti Vaittinen @ 2025-12-15 13:21 UTC (permalink / raw)
To: Matti Vaittinen, Matti Vaittinen
Cc: Lee Jones, Pavel Machek, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Sebastian Reichel, Liam Girdwood, Mark Brown,
Michael Turquette, Stephen Boyd, Matti Vaittinen, Linus Walleij,
Bartosz Golaszewski, Alexandre Belloni, linux-leds, devicetree,
linux-kernel, linux-pm, linux-clk, linux-gpio, linux-rtc,
Andreas Kemnade
In-Reply-To: <cover.1765804226.git.mazziesaccount@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 11744 bytes --]
From: Matti Vaittinen <mazziesaccount@gmail.com>
The ROHM BD72720 is a power management IC with a charger and coulomb
counter block which is closely related to the charger / coulomb counter
found from the BD71815, BD71828, BD71879 which are all supported by the
bd71828-power driver. Due to the similarities it makes sense to support
also the BD72720 with the same driver.
Add basic support for the charger logic on ROHM BD72720.
Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
---
Revision history:
v2 => :
- No changes
RFCv1 => v2:
- Support using 9-bit register addresses (offset of 0x100) with the
BD72720
- Simplify probe and IC data as we don't need two regmaps
- Drop two BD72720 specific functions as we no longer need different
regmap for it.
Note: This patch depends on the series: "power: supply: add charger for
BD71828" by Andreas:
https://lore.kernel.org/all/20250918-bd71828-charger-v5-0-851164839c28@kemnade.info/
NOTE: Fuel-gauging is not supported. You can find an unmaintained
downstream reference-driver with a fuel-gauge example from:
https://github.com/RohmSemiconductor/Linux-Kernel-PMIC-Drivers/releases/tag/bd72720-reference-driver-v1
---
drivers/power/supply/bd71828-power.c | 134 +++++++++++++++++++++++----
1 file changed, 116 insertions(+), 18 deletions(-)
diff --git a/drivers/power/supply/bd71828-power.c b/drivers/power/supply/bd71828-power.c
index ce73c0f48397..438e220a9cb7 100644
--- a/drivers/power/supply/bd71828-power.c
+++ b/drivers/power/supply/bd71828-power.c
@@ -5,6 +5,7 @@
#include <linux/kernel.h>
#include <linux/mfd/rohm-bd71815.h>
#include <linux/mfd/rohm-bd71828.h>
+#include <linux/mfd/rohm-bd72720.h>
#include <linux/module.h>
#include <linux/mod_devicetable.h>
#include <linux/platform_device.h>
@@ -51,12 +52,14 @@ struct pwr_regs {
unsigned int chg_state;
unsigned int bat_temp;
unsigned int dcin_stat;
+ unsigned int dcin_online_mask;
unsigned int dcin_collapse_limit;
unsigned int chg_set1;
unsigned int chg_en;
unsigned int vbat_alm_limit_u;
unsigned int conf;
unsigned int vdcin;
+ unsigned int vdcin_himask;
};
static const struct pwr_regs pwr_regs_bd71828 = {
@@ -67,12 +70,14 @@ static const struct pwr_regs pwr_regs_bd71828 = {
.chg_state = BD71828_REG_CHG_STATE,
.bat_temp = BD71828_REG_BAT_TEMP,
.dcin_stat = BD71828_REG_DCIN_STAT,
+ .dcin_online_mask = BD7182x_MASK_DCIN_DET,
.dcin_collapse_limit = BD71828_REG_DCIN_CLPS,
.chg_set1 = BD71828_REG_CHG_SET1,
.chg_en = BD71828_REG_CHG_EN,
.vbat_alm_limit_u = BD71828_REG_ALM_VBAT_LIMIT_U,
.conf = BD71828_REG_CONF,
.vdcin = BD71828_REG_VDCIN_U,
+ .vdcin_himask = BD7182x_MASK_VDCIN_U,
};
static const struct pwr_regs pwr_regs_bd71815 = {
@@ -85,6 +90,7 @@ static const struct pwr_regs pwr_regs_bd71815 = {
.chg_state = BD71815_REG_CHG_STATE,
.bat_temp = BD71815_REG_BAT_TEMP,
.dcin_stat = BD71815_REG_DCIN_STAT,
+ .dcin_online_mask = BD7182x_MASK_DCIN_DET,
.dcin_collapse_limit = BD71815_REG_DCIN_CLPS,
.chg_set1 = BD71815_REG_CHG_SET1,
.chg_en = BD71815_REG_CHG_SET1,
@@ -92,6 +98,31 @@ static const struct pwr_regs pwr_regs_bd71815 = {
.conf = BD71815_REG_CONF,
.vdcin = BD71815_REG_VM_DCIN_U,
+ .vdcin_himask = BD7182x_MASK_VDCIN_U,
+};
+
+static struct pwr_regs pwr_regs_bd72720 = {
+ .vbat_avg = BD72720_REG_VM_SA_VBAT_U,
+ .ibat = BD72720_REG_CC_CURCD_U,
+ .ibat_avg = BD72720_REG_CC_SA_CURCD_U,
+ .btemp_vth = BD72720_REG_VM_BTMP_U,
+ /*
+ * Note, state 0x40 IMP_CHK. not documented
+ * on other variants but was still handled in
+ * existing code. No memory traces as to why.
+ */
+ .chg_state = BD72720_REG_CHG_STATE,
+ .bat_temp = BD72720_REG_CHG_BAT_TEMP_STAT,
+ .dcin_stat = BD72720_REG_INT_VBUS_SRC,
+ .dcin_online_mask = BD72720_MASK_DCIN_DET,
+ .dcin_collapse_limit = -1, /* Automatic. Setting not supported */
+ .chg_set1 = BD72720_REG_CHG_SET_1,
+ .chg_en = BD72720_REG_CHG_EN,
+ /* 15mV note in data-sheet */
+ .vbat_alm_limit_u = BD72720_REG_ALM_VBAT_TH_U,
+ .conf = BD72720_REG_CONF, /* o XSTB, only PON. Seprate slave addr */
+ .vdcin = BD72720_REG_VM_VBUS_U, /* 10 bits not 11 as with other ICs */
+ .vdcin_himask = BD72720_MASK_VDCIN_U,
};
struct bd71828_power {
@@ -298,7 +329,7 @@ static int get_chg_online(struct bd71828_power *pwr, int *chg_online)
dev_err(pwr->dev, "Failed to read DCIN status\n");
return ret;
}
- *chg_online = ((r & BD7182x_MASK_DCIN_DET) != 0);
+ *chg_online = ((r & pwr->regs->dcin_online_mask) != 0);
return 0;
}
@@ -329,8 +360,8 @@ static int bd71828_bat_inserted(struct bd71828_power *pwr)
ret = val & BD7182x_MASK_CONF_PON;
if (ret)
- regmap_update_bits(pwr->regmap, pwr->regs->conf,
- BD7182x_MASK_CONF_PON, 0);
+ if (regmap_update_bits(pwr->regmap, pwr->regs->conf, BD7182x_MASK_CONF_PON, 0))
+ dev_err(pwr->dev, "Failed to write CONF register\n");
return ret;
}
@@ -358,11 +389,13 @@ static int bd71828_init_hardware(struct bd71828_power *pwr)
int ret;
/* TODO: Collapse limit should come from device-tree ? */
- ret = regmap_write(pwr->regmap, pwr->regs->dcin_collapse_limit,
- BD7182x_DCIN_COLLAPSE_DEFAULT);
- if (ret) {
- dev_err(pwr->dev, "Failed to write DCIN collapse limit\n");
- return ret;
+ if (pwr->regs->dcin_collapse_limit != (unsigned int)-1) {
+ ret = regmap_write(pwr->regmap, pwr->regs->dcin_collapse_limit,
+ BD7182x_DCIN_COLLAPSE_DEFAULT);
+ if (ret) {
+ dev_err(pwr->dev, "Failed to write DCIN collapse limit\n");
+ return ret;
+ }
}
ret = pwr->bat_inserted(pwr);
@@ -419,7 +452,7 @@ static int bd71828_charger_get_property(struct power_supply *psy,
break;
case POWER_SUPPLY_PROP_VOLTAGE_NOW:
ret = bd7182x_read16_himask(pwr, pwr->regs->vdcin,
- BD7182x_MASK_VDCIN_U, &tmp);
+ pwr->regs->vdcin_himask, &tmp);
if (ret)
return ret;
@@ -630,6 +663,9 @@ BD_ISR_AC(dcin_ovp_det, "DCIN OVER VOLTAGE", true)
BD_ISR_DUMMY(dcin_mon_det, "DCIN voltage below threshold")
BD_ISR_DUMMY(dcin_mon_res, "DCIN voltage above threshold")
+BD_ISR_DUMMY(vbus_curr_limit, "VBUS current limited")
+BD_ISR_DUMMY(vsys_ov_res, "VSYS over-voltage cleared")
+BD_ISR_DUMMY(vsys_ov_det, "VSYS over-voltage")
BD_ISR_DUMMY(vsys_uv_res, "VSYS under-voltage cleared")
BD_ISR_DUMMY(vsys_uv_det, "VSYS under-voltage")
BD_ISR_DUMMY(vsys_low_res, "'VSYS low' cleared")
@@ -878,6 +914,51 @@ static int bd7182x_get_irqs(struct platform_device *pdev,
BDIRQ("bd71828-temp-125-over", bd71828_temp_vf125_det),
BDIRQ("bd71828-temp-125-under", bd71828_temp_vf125_res),
};
+ static const struct bd7182x_irq_res bd72720_irqs[] = {
+ BDIRQ("bd72720_int_vbus_rmv", BD_ISR_NAME(dcin_removed)),
+ BDIRQ("bd72720_int_vbus_det", bd7182x_dcin_detected),
+ BDIRQ("bd72720_int_vbus_mon_res", BD_ISR_NAME(dcin_mon_res)),
+ BDIRQ("bd72720_int_vbus_mon_det", BD_ISR_NAME(dcin_mon_det)),
+ BDIRQ("bd72720_int_vsys_mon_res", BD_ISR_NAME(vsys_mon_res)),
+ BDIRQ("bd72720_int_vsys_mon_det", BD_ISR_NAME(vsys_mon_det)),
+ BDIRQ("bd72720_int_vsys_uv_res", BD_ISR_NAME(vsys_uv_res)),
+ BDIRQ("bd72720_int_vsys_uv_det", BD_ISR_NAME(vsys_uv_det)),
+ BDIRQ("bd72720_int_vsys_lo_res", BD_ISR_NAME(vsys_low_res)),
+ BDIRQ("bd72720_int_vsys_lo_det", BD_ISR_NAME(vsys_low_det)),
+ BDIRQ("bd72720_int_vsys_ov_res", BD_ISR_NAME(vsys_ov_res)),
+ BDIRQ("bd72720_int_vsys_ov_det", BD_ISR_NAME(vsys_ov_det)),
+ BDIRQ("bd72720_int_bat_ilim", BD_ISR_NAME(vbus_curr_limit)),
+ BDIRQ("bd72720_int_chg_done", bd718x7_chg_done),
+ BDIRQ("bd72720_int_extemp_tout", BD_ISR_NAME(chg_wdg_temp)),
+ BDIRQ("bd72720_int_chg_wdt_exp", BD_ISR_NAME(chg_wdg)),
+ BDIRQ("bd72720_int_bat_mnt_out", BD_ISR_NAME(rechg_res)),
+ BDIRQ("bd72720_int_bat_mnt_in", BD_ISR_NAME(rechg_det)),
+ BDIRQ("bd72720_int_chg_trns", BD_ISR_NAME(chg_state_changed)),
+
+ BDIRQ("bd72720_int_vbat_mon_res", BD_ISR_NAME(bat_mon_res)),
+ BDIRQ("bd72720_int_vbat_mon_det", BD_ISR_NAME(bat_mon)),
+ BDIRQ("bd72720_int_vbat_sht_res", BD_ISR_NAME(bat_short_res)),
+ BDIRQ("bd72720_int_vbat_sht_det", BD_ISR_NAME(bat_short)),
+ BDIRQ("bd72720_int_vbat_lo_res", BD_ISR_NAME(bat_low_res)),
+ BDIRQ("bd72720_int_vbat_lo_det", BD_ISR_NAME(bat_low)),
+ BDIRQ("bd72720_int_vbat_ov_res", BD_ISR_NAME(bat_ov_res)),
+ BDIRQ("bd72720_int_vbat_ov_det", BD_ISR_NAME(bat_ov)),
+ BDIRQ("bd72720_int_bat_rmv", BD_ISR_NAME(bat_removed)),
+ BDIRQ("bd72720_int_bat_det", BD_ISR_NAME(bat_det)),
+ BDIRQ("bd72720_int_dbat_det", BD_ISR_NAME(bat_dead)),
+ BDIRQ("bd72720_int_bat_temp_trns", BD_ISR_NAME(temp_transit)),
+ BDIRQ("bd72720_int_lobtmp_res", BD_ISR_NAME(temp_bat_low_res)),
+ BDIRQ("bd72720_int_lobtmp_det", BD_ISR_NAME(temp_bat_low)),
+ BDIRQ("bd72720_int_ovbtmp_res", BD_ISR_NAME(temp_bat_hi_res)),
+ BDIRQ("bd72720_int_ovbtmp_det", BD_ISR_NAME(temp_bat_hi)),
+ BDIRQ("bd72720_int_ocur1_res", BD_ISR_NAME(bat_oc1_res)),
+ BDIRQ("bd72720_int_ocur1_det", BD_ISR_NAME(bat_oc1)),
+ BDIRQ("bd72720_int_ocur2_res", BD_ISR_NAME(bat_oc2_res)),
+ BDIRQ("bd72720_int_ocur2_det", BD_ISR_NAME(bat_oc2)),
+ BDIRQ("bd72720_int_ocur3_res", BD_ISR_NAME(bat_oc3_res)),
+ BDIRQ("bd72720_int_ocur3_det", BD_ISR_NAME(bat_oc3)),
+ BDIRQ("bd72720_int_cc_mon2_det", BD_ISR_NAME(bat_cc_mon)),
+ };
int num_irqs;
const struct bd7182x_irq_res *irqs;
@@ -890,6 +971,10 @@ static int bd7182x_get_irqs(struct platform_device *pdev,
irqs = &bd71815_irqs[0];
num_irqs = ARRAY_SIZE(bd71815_irqs);
break;
+ case ROHM_CHIP_TYPE_BD72720:
+ irqs = &bd72720_irqs[0];
+ num_irqs = ARRAY_SIZE(bd72720_irqs);
+ break;
default:
return -EINVAL;
}
@@ -958,21 +1043,27 @@ static int bd71828_power_probe(struct platform_device *pdev)
struct power_supply_config ac_cfg = {};
struct power_supply_config bat_cfg = {};
int ret;
- struct regmap *regmap;
-
- regmap = dev_get_regmap(pdev->dev.parent, NULL);
- if (!regmap) {
- dev_err(&pdev->dev, "No parent regmap\n");
- return -EINVAL;
- }
pwr = devm_kzalloc(&pdev->dev, sizeof(*pwr), GFP_KERNEL);
if (!pwr)
return -ENOMEM;
- pwr->regmap = regmap;
- pwr->dev = &pdev->dev;
+ /*
+ * The BD72720 MFD device registers two regmaps. Power-supply driver
+ * uses the "wrap-map", which provides access to both of the I2C slave
+ * addresses used by the BD72720
+ */
pwr->chip_type = platform_get_device_id(pdev)->driver_data;
+ if (pwr->chip_type != ROHM_CHIP_TYPE_BD72720)
+ pwr->regmap = dev_get_regmap(pdev->dev.parent, NULL);
+ else
+ pwr->regmap = dev_get_regmap(pdev->dev.parent, "wrap-map");
+ if (!pwr->regmap) {
+ dev_err(&pdev->dev, "No parent regmap\n");
+ return -EINVAL;
+ }
+
+ pwr->dev = &pdev->dev;
switch (pwr->chip_type) {
case ROHM_CHIP_TYPE_BD71828:
@@ -985,6 +1076,12 @@ static int bd71828_power_probe(struct platform_device *pdev)
pwr->get_temp = bd71815_get_temp;
pwr->regs = &pwr_regs_bd71815;
break;
+ case ROHM_CHIP_TYPE_BD72720:
+ pwr->bat_inserted = bd71828_bat_inserted;
+ pwr->regs = &pwr_regs_bd72720;
+ pwr->get_temp = bd71828_get_temp;
+ dev_dbg(pwr->dev, "Found ROHM BD72720\n");
+ break;
default:
dev_err(pwr->dev, "Unknown PMIC\n");
return -EINVAL;
@@ -1030,6 +1127,7 @@ static int bd71828_power_probe(struct platform_device *pdev)
static const struct platform_device_id bd71828_charger_id[] = {
{ "bd71815-power", ROHM_CHIP_TYPE_BD71815 },
{ "bd71828-power", ROHM_CHIP_TYPE_BD71828 },
+ { "bd72720-power", ROHM_CHIP_TYPE_BD72720 },
{ },
};
MODULE_DEVICE_TABLE(platform, bd71828_charger_id);
--
2.52.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox