* [PATCH 2/6] dt-bindings: leds: sc2731: Add compatible for SC2730
From: Otto Pflüger @ 2026-02-22 13:16 UTC (permalink / raw)
To: Alexandre Belloni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Orson Zhai, Baolin Wang, Chunyan Zhang, Lee Jones, Pavel Machek,
Liam Girdwood, Mark Brown, Sebastian Reichel
Cc: linux-rtc, devicetree, linux-kernel, linux-leds, linux-pm,
Otto Pflüger
In-Reply-To: <20260222-sc27xx-mfd-cells-v1-0-69526fe74c77@abscue.de>
The LED controller found in the SC2730 PMIC is compatible with the one
found in the SC2731 PMIC.
Signed-off-by: Otto Pflüger <otto.pflueger@abscue.de>
---
Documentation/devicetree/bindings/leds/sprd,sc2731-bltc.yaml | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/leds/sprd,sc2731-bltc.yaml b/Documentation/devicetree/bindings/leds/sprd,sc2731-bltc.yaml
index 97535d6dc47a..2ae5cc31e623 100644
--- a/Documentation/devicetree/bindings/leds/sprd,sc2731-bltc.yaml
+++ b/Documentation/devicetree/bindings/leds/sprd,sc2731-bltc.yaml
@@ -18,7 +18,12 @@ description: |
properties:
compatible:
- const: sprd,sc2731-bltc
+ oneOf:
+ - items:
+ - enum:
+ - sprd,sc2730-bltc
+ - const: sprd,sc2731-bltc
+ - const: sprd,sc2731-bltc
reg:
maxItems: 1
--
2.51.0
^ permalink raw reply related
* [PATCH 1/6] dt-bindings: rtc: sc2731: Add compatible for SC2730
From: Otto Pflüger @ 2026-02-22 13:16 UTC (permalink / raw)
To: Alexandre Belloni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Orson Zhai, Baolin Wang, Chunyan Zhang, Lee Jones, Pavel Machek,
Liam Girdwood, Mark Brown, Sebastian Reichel
Cc: linux-rtc, devicetree, linux-kernel, linux-leds, linux-pm,
Otto Pflüger
In-Reply-To: <20260222-sc27xx-mfd-cells-v1-0-69526fe74c77@abscue.de>
The RTC block found in the SC2730 PMIC is compatible with the one found
in the SC2731 PMIC.
Signed-off-by: Otto Pflüger <otto.pflueger@abscue.de>
---
Documentation/devicetree/bindings/rtc/sprd,sc2731-rtc.yaml | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/rtc/sprd,sc2731-rtc.yaml b/Documentation/devicetree/bindings/rtc/sprd,sc2731-rtc.yaml
index 5756f617df36..1deae2f4f09d 100644
--- a/Documentation/devicetree/bindings/rtc/sprd,sc2731-rtc.yaml
+++ b/Documentation/devicetree/bindings/rtc/sprd,sc2731-rtc.yaml
@@ -13,7 +13,12 @@ maintainers:
properties:
compatible:
- const: sprd,sc2731-rtc
+ oneOf:
+ - items:
+ - enum:
+ - sprd,sc2730-rtc
+ - const: sprd,sc2731-rtc
+ - const: sprd,sc2731-rtc
reg:
maxItems: 1
--
2.51.0
^ permalink raw reply related
* [PATCH 3/6] regulator: dt-bindings: sc2731: Deprecate compatible property
From: Otto Pflüger @ 2026-02-22 13:16 UTC (permalink / raw)
To: Alexandre Belloni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Orson Zhai, Baolin Wang, Chunyan Zhang, Lee Jones, Pavel Machek,
Liam Girdwood, Mark Brown, Sebastian Reichel
Cc: linux-rtc, devicetree, linux-kernel, linux-leds, linux-pm,
Otto Pflüger
In-Reply-To: <20260222-sc27xx-mfd-cells-v1-0-69526fe74c77@abscue.de>
The node containing the regulators is always a child of the main PMIC
node, which already has a compatible property identifying the type of
PMIC. This makes the compatible in the child node redundant. Mark it
as deprecated and remove it from the required property list and the
examples.
Signed-off-by: Otto Pflüger <otto.pflueger@abscue.de>
---
Documentation/devicetree/bindings/mfd/sprd,sc2731.yaml | 2 --
.../devicetree/bindings/regulator/sprd,sc2731-regulator.yaml | 4 +---
2 files changed, 1 insertion(+), 5 deletions(-)
diff --git a/Documentation/devicetree/bindings/mfd/sprd,sc2731.yaml b/Documentation/devicetree/bindings/mfd/sprd,sc2731.yaml
index b023e1ef8d3c..12b3258daef5 100644
--- a/Documentation/devicetree/bindings/mfd/sprd,sc2731.yaml
+++ b/Documentation/devicetree/bindings/mfd/sprd,sc2731.yaml
@@ -222,8 +222,6 @@ examples:
};
regulators {
- compatible = "sprd,sc2731-regulator";
-
BUCK_CPU0 {
regulator-name = "vddarm0";
regulator-min-microvolt = <400000>;
diff --git a/Documentation/devicetree/bindings/regulator/sprd,sc2731-regulator.yaml b/Documentation/devicetree/bindings/regulator/sprd,sc2731-regulator.yaml
index 9bd752bab68e..7af20a4781b7 100644
--- a/Documentation/devicetree/bindings/regulator/sprd,sc2731-regulator.yaml
+++ b/Documentation/devicetree/bindings/regulator/sprd,sc2731-regulator.yaml
@@ -26,6 +26,7 @@ description: |
properties:
compatible:
+ deprecated: true
const: sprd,sc2731-regulator
patternProperties:
@@ -39,8 +40,5 @@ patternProperties:
$ref: regulator.yaml#
unevaluatedProperties: false
-required:
- - compatible
-
additionalProperties: false
...
--
2.51.0
^ permalink raw reply related
* [PATCH 4/6] mfd: sprd-sc27xx: Switch to devm_mfd_add_devices()
From: Otto Pflüger @ 2026-02-22 13:16 UTC (permalink / raw)
To: Alexandre Belloni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Orson Zhai, Baolin Wang, Chunyan Zhang, Lee Jones, Pavel Machek,
Liam Girdwood, Mark Brown, Sebastian Reichel
Cc: linux-rtc, devicetree, linux-kernel, linux-leds, linux-pm,
Otto Pflüger
In-Reply-To: <20260222-sc27xx-mfd-cells-v1-0-69526fe74c77@abscue.de>
To allow instantiating subdevices such as the regulator and poweroff
devices that do not have corresponding device tree nodes with a
"compatible" property, use devm_mfd_add_devices() with MFD cells instead
of devm_of_platform_populate(). Since different PMICs in the SC27xx
series contain different components, use separate MFD cell tables for
each PMIC model. Define cells for all components that have upstream
drivers at this point.
Signed-off-by: Otto Pflüger <otto.pflueger@abscue.de>
---
drivers/mfd/sprd-sc27xx-spi.c | 33 ++++++++++++++++++++++++++++++++-
1 file changed, 32 insertions(+), 1 deletion(-)
diff --git a/drivers/mfd/sprd-sc27xx-spi.c b/drivers/mfd/sprd-sc27xx-spi.c
index d6b4350779e6..57941bec4c2e 100644
--- a/drivers/mfd/sprd-sc27xx-spi.c
+++ b/drivers/mfd/sprd-sc27xx-spi.c
@@ -48,6 +48,31 @@ struct sprd_pmic_data {
u32 irq_base;
u32 num_irqs;
u32 charger_det;
+ const struct mfd_cell *cells;
+ int num_cells;
+};
+
+static const struct mfd_cell sc2730_devices[] = {
+ MFD_CELL_OF("sc2730-adc", NULL, NULL, 0, 0, "sprd,sc2730-adc"),
+ MFD_CELL_OF("sc2730-bltc", NULL, NULL, 0, 0, "sprd,sc2730-bltc"),
+ MFD_CELL_OF("sc2730-efuse", NULL, NULL, 0, 0, "sprd,sc2730-efuse"),
+ MFD_CELL_OF("sc2730-eic", NULL, NULL, 0, 0, "sprd,sc2730-eic"),
+ MFD_CELL_OF("sc2730-fgu", NULL, NULL, 0, 0, "sprd,sc2730-fgu"),
+ MFD_CELL_OF("sc2730-rtc", NULL, NULL, 0, 0, "sprd,sc2730-rtc"),
+ MFD_CELL_OF("sc2730-vibrator", NULL, NULL, 0, 0, "sprd,sc2730-vibrator"),
+};
+
+static const struct mfd_cell sc2731_devices[] = {
+ MFD_CELL_OF("sc2731-adc", NULL, NULL, 0, 0, "sprd,sc2731-adc"),
+ MFD_CELL_OF("sc2731-bltc", NULL, NULL, 0, 0, "sprd,sc2731-bltc"),
+ MFD_CELL_OF("sc2731-charger", NULL, NULL, 0, 0, "sprd,sc2731-charger"),
+ MFD_CELL_OF("sc2731-efuse", NULL, NULL, 0, 0, "sprd,sc2731-efuse"),
+ MFD_CELL_OF("sc2731-eic", NULL, NULL, 0, 0, "sprd,sc2731-eic"),
+ MFD_CELL_OF("sc2731-fgu", NULL, NULL, 0, 0, "sprd,sc2731-fgu"),
+ MFD_CELL_NAME("sc2731-poweroff"),
+ MFD_CELL_NAME("sc2731-regulator"),
+ MFD_CELL_OF("sc2731-rtc", NULL, NULL, 0, 0, "sprd,sc2731-rtc"),
+ MFD_CELL_OF("sc2731-vibrator", NULL, NULL, 0, 0, "sprd,sc2731-vibrator"),
};
/*
@@ -59,12 +84,16 @@ static const struct sprd_pmic_data sc2730_data = {
.irq_base = SPRD_SC2730_IRQ_BASE,
.num_irqs = SPRD_SC2730_IRQ_NUMS,
.charger_det = SPRD_SC2730_CHG_DET,
+ .cells = sc2730_devices,
+ .num_cells = ARRAY_SIZE(sc2730_devices),
};
static const struct sprd_pmic_data sc2731_data = {
.irq_base = SPRD_SC2731_IRQ_BASE,
.num_irqs = SPRD_SC2731_IRQ_NUMS,
.charger_det = SPRD_SC2731_CHG_DET,
+ .cells = sc2731_devices,
+ .num_cells = ARRAY_SIZE(sc2731_devices),
};
enum usb_charger_type sprd_pmic_detect_charger_type(struct device *dev)
@@ -204,7 +233,9 @@ static int sprd_pmic_probe(struct spi_device *spi)
return ret;
}
- ret = devm_of_platform_populate(&spi->dev);
+ ret = devm_mfd_add_devices(&spi->dev, PLATFORM_DEVID_AUTO,
+ pdata->cells, pdata->num_cells, NULL, 0,
+ regmap_irq_get_domain(ddata->irq_data));
if (ret) {
dev_err(&spi->dev, "Failed to populate sub-devices %d\n", ret);
return ret;
--
2.51.0
^ permalink raw reply related
* [PATCH 5/6] power: reset: sc27xx: Add platform_device_id table
From: Otto Pflüger @ 2026-02-22 13:16 UTC (permalink / raw)
To: Alexandre Belloni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Orson Zhai, Baolin Wang, Chunyan Zhang, Lee Jones, Pavel Machek,
Liam Girdwood, Mark Brown, Sebastian Reichel
Cc: linux-rtc, devicetree, linux-kernel, linux-leds, linux-pm,
Otto Pflüger
In-Reply-To: <20260222-sc27xx-mfd-cells-v1-0-69526fe74c77@abscue.de>
Make the poweroff driver for SC27xx-series PMICs probe automatically.
Since the device representing the poweroff functionality of the SC27xx
PMIC is not supposed to have a dedicated device tree node without any
corresponding DT resources [1], an of_device_id table cannot be used
here. Instead, use a platform_device_id table to match the poweroff
sub-device instantiated by the parent MFD driver.
Signed-off-by: Otto Pflüger <otto.pflueger@abscue.de>
[1]: https://lore.kernel.org/all/20251002025344.GA2958334-robh@kernel.org/
---
drivers/power/reset/sc27xx-poweroff.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/power/reset/sc27xx-poweroff.c b/drivers/power/reset/sc27xx-poweroff.c
index 393bd1c33b73..6376706bf561 100644
--- a/drivers/power/reset/sc27xx-poweroff.c
+++ b/drivers/power/reset/sc27xx-poweroff.c
@@ -6,6 +6,7 @@
#include <linux/cpu.h>
#include <linux/kernel.h>
+#include <linux/mod_devicetable.h>
#include <linux/module.h>
#include <linux/platform_device.h>
#include <linux/pm.h>
@@ -70,11 +71,18 @@ static int sc27xx_poweroff_probe(struct platform_device *pdev)
return 0;
}
+static const struct platform_device_id sc27xx_poweroff_id_table[] = {
+ { "sc2731-poweroff" },
+ { }
+};
+MODULE_DEVICE_TABLE(platform, sc27xx_poweroff_id_table);
+
static struct platform_driver sc27xx_poweroff_driver = {
.probe = sc27xx_poweroff_probe,
.driver = {
.name = "sc27xx-poweroff",
},
+ .id_table = sc27xx_poweroff_id_table,
};
module_platform_driver(sc27xx_poweroff_driver);
--
2.51.0
^ permalink raw reply related
* [PATCH 6/6] regulator: sc2731: Add platform_device_id table
From: Otto Pflüger @ 2026-02-22 13:16 UTC (permalink / raw)
To: Alexandre Belloni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Orson Zhai, Baolin Wang, Chunyan Zhang, Lee Jones, Pavel Machek,
Liam Girdwood, Mark Brown, Sebastian Reichel
Cc: linux-rtc, devicetree, linux-kernel, linux-leds, linux-pm,
Otto Pflüger
In-Reply-To: <20260222-sc27xx-mfd-cells-v1-0-69526fe74c77@abscue.de>
Make the regulator driver for the SC2731 PMIC probe automatically. Using
a platform_device_id table instead of DT compatible matching avoids the
need for a separate compatible property in the "regulators" node, which
simplifies the DT bindings and makes the parent MFD device responsible
for selecting the correct regulator driver for the PMIC.
However, this means that the regulator device is not automatically
associated with the "regulators" node. Tell the regulator core to
perform device tree lookups using the parent MFD device instead of
the regulator sub-device and set the .regulators_node member in all
regulator definitions so that the "regulators" sub-node is used.
Signed-off-by: Otto Pflüger <otto.pflueger@abscue.de>
---
drivers/regulator/sc2731-regulator.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/regulator/sc2731-regulator.c b/drivers/regulator/sc2731-regulator.c
index 5447e1a47d15..93c8156c5110 100644
--- a/drivers/regulator/sc2731-regulator.c
+++ b/drivers/regulator/sc2731-regulator.c
@@ -131,6 +131,7 @@ static const struct regulator_ops sc2731_regu_linear_ops = {
vstep, vmin, vmax) { \
.name = #_id, \
.of_match = of_match_ptr(#_id), \
+ .regulators_node = of_match_ptr("regulators"), \
.ops = &sc2731_regu_linear_ops, \
.type = REGULATOR_VOLTAGE, \
.id = SC2731_##_id, \
@@ -226,7 +227,7 @@ static int sc2731_regulator_probe(struct platform_device *pdev)
return ret;
}
- config.dev = &pdev->dev;
+ config.dev = pdev->dev.parent;
config.regmap = regmap;
for (i = 0; i < ARRAY_SIZE(regulators); i++) {
@@ -242,12 +243,19 @@ static int sc2731_regulator_probe(struct platform_device *pdev)
return 0;
}
+static const struct platform_device_id sc2731_regulator_id_table[] = {
+ { "sc2731-regulator" },
+ { }
+};
+MODULE_DEVICE_TABLE(platform, sc2731_regulator_id_table);
+
static struct platform_driver sc2731_regulator_driver = {
.driver = {
.name = "sc27xx-regulator",
.probe_type = PROBE_PREFER_ASYNCHRONOUS,
},
.probe = sc2731_regulator_probe,
+ .id_table = sc2731_regulator_id_table,
};
module_platform_driver(sc2731_regulator_driver);
--
2.51.0
^ permalink raw reply related
* Re: [RFC PATCH v3 1/5] rtc: add device selector for rtc_class_ops callbacks
From: Rafael J. Wysocki @ 2026-02-22 14:01 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Alexandre Belloni, Rafael J. Wysocki, Alvin Sun, Miguel Ojeda,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, linux-rtc,
rust-for-linux, Greg Kroah-Hartman
In-Reply-To: <DGLI4H9M0T6D.25RTLDVU5JRBE@kernel.org>
On Sun, Feb 22, 2026 at 1:49 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> On Sun Feb 22, 2026 at 1:05 AM CET, Alexandre Belloni wrote:
> > On 21/02/2026 15:33:48+0100, Danilo Krummrich wrote:
> >> (2) Bus device private data vs. class device private data.
> >>
> >> The change to pass a struct rtc_device in class device callbacks of RTC,
> >> rather than the base struct device of the corresponding bus device (e.g. AMBA,
> >> platform, etc.) should not aim at storing all data in rtc->dev.private_data
> >> that was previously stored in rtc->dev.parent->private_data.
> >>
> >
> > But what you explain here is that the drive is forbidden to use
> > rtc->dev.parent->private_data at all because the rust core is already
> > using it. What I'm saying is that it won't work because more than half
> > of the drivers currently need it.
>
> I think I was explaining the exact opposite, i.e. the driver's bus device
> private data is stored in the exact same way as in C, but it has a defined
> lifetime (from probe() until remove() and all devres callbacks have been
> completed) and is managed by the driver-core.
>
> Look at this example:
>
> // The bus device private data.
> struct SampleDriver {
> foo: u32,
> bar: u32,
> }
>
> impl pci::Driver for SampleDriver {
> fn probe(pdev: &pci::Device<Core>, info: &Self::IdInfo) -> impl PinInit<Self, Error> {
> // Return the driver's bus device private data. The driver-core
> // will do all the work for us and eventually call
> // `dev_set_drvdata()` on the base `struct device` of `pdev`.
So that's how I thought it would work.
> Ok(Foo {
> foo: 42,
> bar: 24,
> })
> }
>
> fn unbind(pdev: &pci::Device<Core>, self: Pin<&Self>) {
> // Use the driver's device private data from the method's `self` argument.
> dev_info!(pdev, "foo: {}, bar: {}\n", self.foo, self.bar);
>
> // There's also the `Device<Bound>::drvdata()` accessor, let's
> // use that as well.
>
> // Get a generic `device::Device` from the `pci::Device`.
> let dev = pdev.as_ref();
>
> // Get the `drvdata`; we have to assert the type and let the driver-core
> // validate that the asserted type is correct.
> let drvdata = dev.drvdata::<SampleDriver>()?;
>
> dev_info!(pdev, "foo: {}, bar: {}\n", drvdata.foo, drvdata.bar);
> }
> }
This helps, thanks!
> >> This thread seems to contain quite a bit of confusion and misunderstandings --
> >> let me try to clarify.
> >>
> >> (1) How Rust handles bus device private data.
> >>
> >> In Rust the probe() function of a bus implementation (platform, PCI, etc.)
> >> returns an initializer (impl PinInit<T, Error>) for the driver's device
> >> private data.
> >>
> >> The bus implementation takes this initializer and passes it (together with the
> >> underlying struct device) to the driver-core. The driver-core allocates the
> >> required memory, initializes the memory with the given initializer and stores
> >> a pointer to the corresponding object with dev_set_drvdata().
> >>
> >> So, technically, in Rust all platform drivers call platform_set_drvdata().
> >>
> >> (Note that this is also true when the driver's device private data type is
> >> empty (i.e. it has no fields). In this case it could still have a destructor
> >> that must be called when the device private data structure is destroyed. Of
> >> course there is no real memory allocation when the struct's size is zero.)
> >>
> >> The driver's device private data can only be accessed when the bus device is
> >> bound to the driver, i.e. the driver can only access it with a &Device<Bound>;
> >> it (the driver's device private data) is automatically freed by the
> >> driver-core when remove() and all devres callbacks have been completed.
> >>
> >> I.e. the rules are - of course - the same as on the C side, but they are
> >> enforced by the type system and the driver-core code.
> >>
> >
> > This still doesn't explain how you get the class private data that you
> > need when you are in a driver callback that is called from the bus (e.g.
> > suspend/resume) from what you explain, the driver doesn't have any
> > chance to pass it. The whole goal of a device driver is to be the glue
> > between a class device and a bus device as essentially this is the exact
> > physical device, just represented differently.
>
> It is not always as simple as "one bus device corresponds to one class device".
>
> Sometimes drivers have to deal with multiple class devices for a single bus
> device, sometimes they are separated through the auxiliary bus or MFD.
>
> For instance, take struct net_device. In the context of cfg80211 wireless
> drivers may create arbitrary struct net_device instances, depending on how often
> the add_virtual_intf() callback is called (through netlink); example in [1].
>
> Now, regarding your question "How to access class device private data from bus
> device callbacks?". Nothing prevents a driver from embedding the class device in
> its bus device private data in one or the other way.
>
> In the net device example above, the driver would probably keep a list (or
> xarray, etc.) of net devices in its bus device private data, as they can be
> created and removed at any point of time.
>
> (Note that this is also a good example for when the class device private data
> lives shorter than the bus device private data.)
>
> Here is an example of this looks like in code using a DRM device as class
> device.
>
> struct SampleIrqData;
>
> // The class device private data.
> struct SampleDrm {
> sched: drm::GpuScheduler,
> }
>
> // The bus device private data.
> #[pin_data]
> struct SampleDriver {
> #[pin]
> irq: irq::Registration<SampleIrqData>,
> drm: ARef<drm::Device<SampleDrm>, // Refcount of a `drm::Device`.
> }
>
> impl pci::Driver for SampleDriver {
> fn probe(pdev: &pci::Device<Core>, info: &Self::IdInfo) -> impl PinInit<Self, Error> {
> let dev = pdev.as_ref();
>
> let drm = drm::Device::new(dev, SampleDrm::new()?)?;
> drm.register();
>
> Ok(impl_pin_init!(Self {
> irq <- irq::Registration::new(...);
> drm,
> })
> }
>
> fn unbind(pdev: &pci::Device<Core>, self: Pin<&Self>) {
> self.irq.synchronize();
> self.drm.flush_scheduler();
> }
> }
>
> Note that the irq::Registration is dropped when the bus device is unbound,
> whereas the GpuScheduler is dropped when the DRM device is dropped.
>
> Of course, depending on the actual class device this difference may be more or
> less important, i.e. absolutely crucial in the net device example, less
> important for RTC apparently. But it is always good to be precise about
> ownership and lifetime of data.
>
> As I already mentioned, I don't think it is good to fold ownership and lifetime
> for bus and class devices together in cases where it happens that they are
> mostly identical, i.e.:
>
> >> I think it would be good to keep driver authors finding a common pattern,
> >> where class device callbacks carry the corresponding class device struct
> >> (instead of the parent base struct device).
> >>
> >> Especially on the Rust side we now have the chance to make the experience
> >> of writing drivers as consistent as possible, which should help (new)
> >> driver authors a lot in terms of learning the driver lifetime patterns.
>
> [1] https://elixir.bootlin.com/linux/v6.19.2/source/drivers/net/wireless/microchip/wilc1000/netdev.c#L948
General considerations aside, regarding the $subject patch, I don't
think that it is an improvement because it would confuse the existing
RTC class device interface quite a bit.
Regarding whether or not switching over the RTC class device interface
to passing RTC device pointers to its class callbacks is a good idea,
I think that the difference really boils down to what data needs to be
accessed by the driver.
As it stands today, in an RTC class callback, the driver gets directly
to the parent of the given RTC device and if it does not need to get
to the RTC device at all, that's convenient. However, if the driver
needs to get to the RTC device, it will most likely need to use
driver_data in the parent device for this purpose (and store the RTC
device pointer).
If the interface were changed to pass RTC device pointers to class
callbacks, the driver would get to the RTC device and from there it
would need to get to the parent device in the majority of cases. Even
though getting from an RTC device to its parent device is
straightforward (and does not require using driver_data at all), in
some drivers that would be an extra step that would make the picture
somewhat less clear than it is today (and arguably less clear than it
really needs to be). Still, the change of the interface might allow
some other drivers that do access the RTC device in class callback
functions to be simplified.
Granted, the above options had already been there when the RTC class
interface was designed and the choice was made at that time. Revising
it now would require clear technical reasons IMV.
How hard would it be for Rust to cope with the existing calling
convention in the RTC class?
^ permalink raw reply
* Re: [RFC PATCH v3 1/5] rtc: add device selector for rtc_class_ops callbacks
From: Rafael J. Wysocki @ 2026-02-22 14:24 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Danilo Krummrich, Alexandre Belloni, Alvin Sun, Miguel Ojeda,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, linux-rtc,
rust-for-linux, Greg Kroah-Hartman
In-Reply-To: <CAJZ5v0gtrpFKPV0LPzfz4JHkEqwK1XRoqO8peWYKw_4j5ti1MA@mail.gmail.com>
On Sun, Feb 22, 2026 at 1:25 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> On Sat, Feb 21, 2026 at 3:33 PM Danilo Krummrich <dakr@kernel.org> wrote:
> >
> > On Sat Feb 21, 2026 at 12:19 PM CET, Rafael J. Wysocki wrote:
> > > On Sat, Feb 21, 2026 at 12:16 PM Alexandre Belloni
> > > <alexandre.belloni@bootlin.com> wrote:
> > >> > > Out of 29 drivers, 18 are doing so.
> > >
> > > The vast majority of around 50 platform drivers I've inspected
> > > recently use platform_set_drvdata() or equivalent in probe.
> >
> > This thread seems to contain quite a bit of confusion and misunderstandings --
> > let me try to clarify.
> >
> > (1) How Rust handles bus device private data.
> >
> > In Rust the probe() function of a bus implementation (platform, PCI, etc.)
> > returns an initializer (impl PinInit<T, Error>) for the driver's device
> > private data.
> >
> > The bus implementation takes this initializer and passes it (together with the
> > underlying struct device) to the driver-core. The driver-core allocates the
> > required memory, initializes the memory with the given initializer and stores
> > a pointer to the corresponding object with dev_set_drvdata().
> >
> > So, technically, in Rust all platform drivers call platform_set_drvdata().
>
> So do I understand correctly that the driver is required to tell the
> core what type its driver_data will be and then the core will allocate
> memory for it and clean it up on remove?
>
> > (Note that this is also true when the driver's device private data type is
> > empty (i.e. it has no fields). In this case it could still have a destructor
> > that must be called when the device private data structure is destroyed. Of
> > course there is no real memory allocation when the struct's size is zero.)
>
> So in the simplest case when the driver doesn't need driver_data at
> all, it will just use a struct with no fields as that driver_data
> type, IIUC.
>
> > The driver's device private data can only be accessed when the bus device is
> > bound to the driver, i.e. the driver can only access it with a &Device<Bound>;
> > it (the driver's device private data) is automatically freed by the
> > driver-core when remove() and all devres callbacks have been completed.
>
> Well, that's what happens on the C side of things too most of the time
> because driver_data is allocated at probe time, very often using
> devm_kzalloc() or equivalent.
>
> > I.e. the rules are - of course - the same as on the C side, but they are
> > enforced by the type system and the driver-core code.
>
> OK
>
> > (2) Bus device private data vs. class device private data.
> >
> > The change to pass a struct rtc_device in class device callbacks of RTC,
> > rather than the base struct device of the corresponding bus device (e.g. AMBA,
> > platform, etc.) should not aim at storing all data in rtc->dev.private_data
> > that was previously stored in rtc->dev.parent->private_data.
> >
> > Instead, it gives drivers the option to differentiate in terms of ownership
> > and lifetime.
> >
> > While the bus device private data has a very defined lifetime from probe()
> > until the device is unbound from the driver, class device private data might
> > live shorter than this, or might even out-live driver unbind in some cases. It
> > really depends on the lifetime of the class device itself, which is not
> > generally defined.
> >
> > Now, from a C side point of view this may not look like a big deal, as it
> > (unfortunately) is not that uncommon that struct fields are just initialized
> > and destroyed whenever needed and the code just takes it into account.
> >
> > But at the same time, this is what leads to a lot of lifetime problems and
> > memory bugs and it is one of those things that Rust aims at avoiding by being
> > very strict about initialization, ownership and lifetimes.
>
> As a general rule, I agree, but I would advise against applying
> general rules automatically everywhere.
>
> > However, I do also recognize that drivers creating an RTC device are typically
> > very simple and in practice I would not be surprised if it turns out that it
> > happens that drivers keep the struct rtc_device alive from probe() until the
> > bus device is unbound from the driver, i.e. lifetimes just end up being almost
> > the same. But I don't know if that's always the case.
> >
> > Regardless of that, I think it would be good to keep driver authors finding a
> > common pattern, where class device callbacks carry the corresponding class
> > device struct (instead of the parent base struct device).
>
> TBH I'm not really convinced about this particular thing and I think I
> can provide an illustrative example.
>
> Namely, quite incidentally, I've recently set out to add an RTC class
> device to an existing driver, which is the ACPI time and alarm device
> (TAD) one. The TAD functionality is based on ACPI control methods
> provided by the platform firmware and it may (or may not) include
> RTC-equivalent functions. So far, the driver has been providing a
> completely custom sysfs interface to user space, but since more and
> more platforms contain an ACPI TAD and some of them may not contain a
> "traditional" RTC, having an RTC class device interface in that driver
> may be quite useful.
>
> I have a prototype of the requisite change (I'll post it shortly for
> reference) and it turns out that because the RTC class callbacks take
> the parent device pointer as an argument, wrapping them around the
> existing driver routines backing the existing sysfs interface is
> super-straightforward. Had the RTC class passed an RTC device pointer
> to those callbacks, the driver would have had to do something to get
> back from it to the parent device (which is what the driver really
> works with). If there are more similar drivers, that would have led
> to some code duplication that is in fact unnecessary.
>
> Moreover, the RTC device pointer doesn't even need to be stored
> anywhere in that driver because the driver need not use it directly at
> all and the RTC device object memory is freed by the core when the
> driver unbinds.
The above is referring specifically to this patch:
https://lore.kernel.org/linux-acpi/10819001.nUPlyArG6x@rafael.j.wysocki/
^ permalink raw reply
* Re: [RFC PATCH v3 1/5] rtc: add device selector for rtc_class_ops callbacks
From: Danilo Krummrich @ 2026-02-22 15:29 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Alexandre Belloni, Alvin Sun, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, linux-rtc, rust-for-linux, Greg Kroah-Hartman
In-Reply-To: <CAJZ5v0gtrpFKPV0LPzfz4JHkEqwK1XRoqO8peWYKw_4j5ti1MA@mail.gmail.com>
On Sun Feb 22, 2026 at 1:25 PM CET, Rafael J. Wysocki wrote:
> On Sat, Feb 21, 2026 at 3:33 PM Danilo Krummrich <dakr@kernel.org> wrote:
>>
>> On Sat Feb 21, 2026 at 12:19 PM CET, Rafael J. Wysocki wrote:
>> > On Sat, Feb 21, 2026 at 12:16 PM Alexandre Belloni
>> > <alexandre.belloni@bootlin.com> wrote:
>> >> > > Out of 29 drivers, 18 are doing so.
>> >
>> > The vast majority of around 50 platform drivers I've inspected
>> > recently use platform_set_drvdata() or equivalent in probe.
>>
>> This thread seems to contain quite a bit of confusion and misunderstandings --
>> let me try to clarify.
>>
>> (1) How Rust handles bus device private data.
>>
>> In Rust the probe() function of a bus implementation (platform, PCI, etc.)
>> returns an initializer (impl PinInit<T, Error>) for the driver's device
>> private data.
>>
>> The bus implementation takes this initializer and passes it (together with the
>> underlying struct device) to the driver-core. The driver-core allocates the
>> required memory, initializes the memory with the given initializer and stores
>> a pointer to the corresponding object with dev_set_drvdata().
>>
>> So, technically, in Rust all platform drivers call platform_set_drvdata().
>
> So do I understand correctly that the driver is required to tell the
> core what type its driver_data will be and then the core will allocate
> memory for it and clean it up on remove?
Yes, but it's not really that the driver actively has to tell the driver-core,
etc.
probe() functions return an initializer for the driver's device private data, so
the type is known anyways.
fn probe(
pdev: &pci::Device<Core>,
info: &Self::IdInfo,
) -> impl PinInit<T, Error> {
...
}
So, the return type is a fallible initializer for T, where T is the type of the
driver's device private data.
I assume this may sound a bit odd with little or no Rust experience. Hence, for
people without much Rust experience reading along a quick explanation:
On the more general side of things, Rust has a very powerful type system, which
includes generics, hence modeling such kind of things with generics is pretty
straight forward and preferred over passing void pointers.
But there is also a much more specific reason; In C dev_get_drvdata() has two
pitfalls:
(1) The pointer returned by dev_get_drvdata() is only valid as long as the
bus device is bound to the driver.
(2) The driver has to cast the pointer returned by dev_get_drvdata() to the
correct type.
Since Rust is a memory safe language, we can't allow UB for safe APIs. Hence,
the Rust Device::drvdata() [1] method has to consider both (1) and (2).
(1) is rather simple as we have a type state found bound devices, i.e.
Device<Bound>, so drvdata() is simply only implemented for Device<Bound>.
(2) is much more tricky as we can't statically type the device over its private
data, as a single device instance can be bound to multiple drivers at runtime.
Hence, we need a runtime check, which the driver-core does for us. When a driver
calls the drvdata() method it looks like this:
fn foo(dev: &Device<Bound>) -> Result {
let data = dev.drvdata::<MyDataType>()?;
data.bar();
}
The driver-core takes care of checking that the private data associated with
`dev` actually is of type MyDataType. If this is not the case, the call simply
fails.
The alternative would be an infallible unsafe API, such as:
fn foo(dev: &Device<Bound>) -> Result {
// SAFETY:
// - `dev` is guaranteed to be bound, because ...
// - The private data type of `dev` is guaranteed to be
// `MyDataType`, since ...
let data = unsafe { dev.drvdata::<MyDataType>() };
data.bar();
}
[1] https://rust.docs.kernel.org/kernel/device/struct.Device.html#method.drvdata
>> (Note that this is also true when the driver's device private data type is
>> empty (i.e. it has no fields). In this case it could still have a destructor
>> that must be called when the device private data structure is destroyed. Of
>> course there is no real memory allocation when the struct's size is zero.)
>
> So in the simplest case when the driver doesn't need driver_data at
> all, it will just use a struct with no fields as that driver_data
> type, IIUC.
Yes, it would look like this:
struct SampleDriver;
impl pci::Driver for SampleDriver {
fn probe(pdev: &pci::Device<Core>, info: &Self::IdInfo) -> impl PinInit<Self, Error> {
if !validate_something() {
return Err(EINVAL);
}
Ok(Self)
}
}
>> But at the same time, this is what leads to a lot of lifetime problems and
>> memory bugs and it is one of those things that Rust aims at avoiding by being
>> very strict about initialization, ownership and lifetimes.
>
> As a general rule, I agree, but I would advise against applying
> general rules automatically everywhere.
>
>> However, I do also recognize that drivers creating an RTC device are typically
>> very simple and in practice I would not be surprised if it turns out that it
>> happens that drivers keep the struct rtc_device alive from probe() until the
>> bus device is unbound from the driver, i.e. lifetimes just end up being almost
>> the same. But I don't know if that's always the case.
>>
>> Regardless of that, I think it would be good to keep driver authors finding a
>> common pattern, where class device callbacks carry the corresponding class
>> device struct (instead of the parent base struct device).
>
> TBH I'm not really convinced about this particular thing and I think I
> can provide an illustrative example.
>
> Namely, quite incidentally, I've recently set out to add an RTC class
> device to an existing driver, which is the ACPI time and alarm device
> (TAD) one. The TAD functionality is based on ACPI control methods
> provided by the platform firmware and it may (or may not) include
> RTC-equivalent functions. So far, the driver has been providing a
> completely custom sysfs interface to user space, but since more and
> more platforms contain an ACPI TAD and some of them may not contain a
> "traditional" RTC, having an RTC class device interface in that driver
> may be quite useful.
>
> I have a prototype of the requisite change (I'll post it shortly for
> reference) and it turns out that because the RTC class callbacks take
> the parent device pointer as an argument, wrapping them around the
> existing driver routines backing the existing sysfs interface is
> super-straightforward. Had the RTC class passed an RTC device pointer
> to those callbacks, the driver would have had to do something to get
> back from it to the parent device (which is what the driver really
> works with). If there are more similar drivers, that would have led
> to some code duplication that is in fact unnecessary.
The type system in Rust is powerfuly enough, so drivers can even get the exact
type of the parent device the RTC device has as an additional argument to the
callback. The infrastructure for this is in place and it is used by subsystems.
I.e. in RTC we can do something like this:
impl rtc::Ops for MyRtcOps {
type BusDeviceType = platform::Device<Bound>;
fn read_time(
rtc: &rtc::Device<MyRtcData>,
parent: &platform::Device<Bound>,
time: rtc::Time,
) -> Result {
...
}
}
where the corresponding rtc::Device::register() method would ensure that the
associated BusDeviceType matches the parent device type that is passed to
rtc::Device::register().
A real example of this can be found in the LED class device abstractions [2].
Note that in contrast to a bus device, class devices can be statically typed
over their private data: rtc::Device<MyRtcData>.
We usually also implment the Deref trait, such that rtc::Device<MyRtcData>
automatically dereferences to MyRtcData.
Having that said, if RTC drivers *never* have any private data that should be
associated with the RTC device exclusively (i.e. data that is only relevant in
the context of class device callbacks and class device infrastructure in
general, or logically belongs to the class device), then the `rdev` argument
would indeed be always unused and hence pointless.
I usually would assume that there are such cases, but if that's really never the
case for any RTC drivers, then I agree we should change the above code to:
impl rtc::Ops for MyRtcOps {
type BusDeviceType = platform::Device<Bound>;
fn read_time(
parent: &platform::Device<Bound>,
time: rtc::Time,
) -> Result {
...
}
}
This way it is at least clear what kind of device is passed through the class
device callbacks.
[2] https://lore.kernel.org/all/20260207-rust_leds-v12-1-fdb518417b75@posteo.de/
> Moreover, the RTC device pointer doesn't even need to be stored
> anywhere in that driver because the driver need not use it directly at
> all and the RTC device object memory is freed by the core when the
> driver unbinds.
I don't think that is true, I think there are a few drivers accessing the RTC
device from IRQs or workqueues.
Besides that, quite a lot of RTC drivers actually seem to save a pointer to the
struct rtc_device within their bus device private data, e.g. [3] and [4].
[3] https://elixir.bootlin.com/linux/v6.19.2/source/drivers/rtc/rtc-ac100.c#L91
[4] https://elixir.bootlin.com/linux/v6.19.2/source/drivers/rtc/rtc-cros-ec.c#L30
>> Especially on the Rust side we now have the chance to make the experience of
>> writing drivers as consistent as possible, which should help (new) driver
>> authors a lot in terms of learning the driver lifetime patterns.
>
> Well, I'm not sure if "the experience of writing drivers as consistent
> as possible" is more important than less code duplication and simpler
> code.
Yeah, it always depends, and if there *really* is no point in having any class
device private data in RTC, then that's of course fine too.
I just want to make sure we do not encourage to just through all the private
data a driver may potentially have into the bus device private data struct.
I saw this a lot in C drivers and it actually causes ownership and lifetime
problems.
^ permalink raw reply
* Re: [RFC PATCH v3 1/5] rtc: add device selector for rtc_class_ops callbacks
From: Rafael J. Wysocki @ 2026-02-22 15:43 UTC (permalink / raw)
To: Danilo Krummrich, Alvin Sun
Cc: Alexandre Belloni, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, linux-rtc, rust-for-linux, Greg Kroah-Hartman
In-Reply-To: <DGLLJ541SEJW.160MET6OCQHKS@kernel.org>
On Sun, Feb 22, 2026 at 4:29 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> On Sun Feb 22, 2026 at 1:25 PM CET, Rafael J. Wysocki wrote:
> > On Sat, Feb 21, 2026 at 3:33 PM Danilo Krummrich <dakr@kernel.org> wrote:
> >>
> >> On Sat Feb 21, 2026 at 12:19 PM CET, Rafael J. Wysocki wrote:
> >> > On Sat, Feb 21, 2026 at 12:16 PM Alexandre Belloni
> >> > <alexandre.belloni@bootlin.com> wrote:
> >> >> > > Out of 29 drivers, 18 are doing so.
> >> >
> >> > The vast majority of around 50 platform drivers I've inspected
> >> > recently use platform_set_drvdata() or equivalent in probe.
> >>
> >> This thread seems to contain quite a bit of confusion and misunderstandings --
> >> let me try to clarify.
> >>
> >> (1) How Rust handles bus device private data.
> >>
> >> In Rust the probe() function of a bus implementation (platform, PCI, etc.)
> >> returns an initializer (impl PinInit<T, Error>) for the driver's device
> >> private data.
> >>
> >> The bus implementation takes this initializer and passes it (together with the
> >> underlying struct device) to the driver-core. The driver-core allocates the
> >> required memory, initializes the memory with the given initializer and stores
> >> a pointer to the corresponding object with dev_set_drvdata().
> >>
> >> So, technically, in Rust all platform drivers call platform_set_drvdata().
> >
> > So do I understand correctly that the driver is required to tell the
> > core what type its driver_data will be and then the core will allocate
> > memory for it and clean it up on remove?
>
> Yes, but it's not really that the driver actively has to tell the driver-core,
> etc.
>
> probe() functions return an initializer for the driver's device private data, so
> the type is known anyways.
>
> fn probe(
> pdev: &pci::Device<Core>,
> info: &Self::IdInfo,
> ) -> impl PinInit<T, Error> {
> ...
> }
>
> So, the return type is a fallible initializer for T, where T is the type of the
> driver's device private data.
>
> I assume this may sound a bit odd with little or no Rust experience. Hence, for
> people without much Rust experience reading along a quick explanation:
>
> On the more general side of things, Rust has a very powerful type system, which
> includes generics, hence modeling such kind of things with generics is pretty
> straight forward and preferred over passing void pointers.
>
> But there is also a much more specific reason; In C dev_get_drvdata() has two
> pitfalls:
>
> (1) The pointer returned by dev_get_drvdata() is only valid as long as the
> bus device is bound to the driver.
>
> (2) The driver has to cast the pointer returned by dev_get_drvdata() to the
> correct type.
>
> Since Rust is a memory safe language, we can't allow UB for safe APIs. Hence,
> the Rust Device::drvdata() [1] method has to consider both (1) and (2).
>
> (1) is rather simple as we have a type state found bound devices, i.e.
> Device<Bound>, so drvdata() is simply only implemented for Device<Bound>.
>
> (2) is much more tricky as we can't statically type the device over its private
> data, as a single device instance can be bound to multiple drivers at runtime.
>
> Hence, we need a runtime check, which the driver-core does for us. When a driver
> calls the drvdata() method it looks like this:
>
> fn foo(dev: &Device<Bound>) -> Result {
> let data = dev.drvdata::<MyDataType>()?;
>
> data.bar();
> }
>
> The driver-core takes care of checking that the private data associated with
> `dev` actually is of type MyDataType. If this is not the case, the call simply
> fails.
>
> The alternative would be an infallible unsafe API, such as:
>
> fn foo(dev: &Device<Bound>) -> Result {
> // SAFETY:
> // - `dev` is guaranteed to be bound, because ...
> // - The private data type of `dev` is guaranteed to be
> // `MyDataType`, since ...
> let data = unsafe { dev.drvdata::<MyDataType>() };
>
> data.bar();
> }
>
> [1] https://rust.docs.kernel.org/kernel/device/struct.Device.html#method.drvdata
>
> >> (Note that this is also true when the driver's device private data type is
> >> empty (i.e. it has no fields). In this case it could still have a destructor
> >> that must be called when the device private data structure is destroyed. Of
> >> course there is no real memory allocation when the struct's size is zero.)
> >
> > So in the simplest case when the driver doesn't need driver_data at
> > all, it will just use a struct with no fields as that driver_data
> > type, IIUC.
>
> Yes, it would look like this:
>
> struct SampleDriver;
>
> impl pci::Driver for SampleDriver {
> fn probe(pdev: &pci::Device<Core>, info: &Self::IdInfo) -> impl PinInit<Self, Error> {
> if !validate_something() {
> return Err(EINVAL);
> }
>
> Ok(Self)
> }
> }
>
> >> But at the same time, this is what leads to a lot of lifetime problems and
> >> memory bugs and it is one of those things that Rust aims at avoiding by being
> >> very strict about initialization, ownership and lifetimes.
> >
> > As a general rule, I agree, but I would advise against applying
> > general rules automatically everywhere.
> >
> >> However, I do also recognize that drivers creating an RTC device are typically
> >> very simple and in practice I would not be surprised if it turns out that it
> >> happens that drivers keep the struct rtc_device alive from probe() until the
> >> bus device is unbound from the driver, i.e. lifetimes just end up being almost
> >> the same. But I don't know if that's always the case.
> >>
> >> Regardless of that, I think it would be good to keep driver authors finding a
> >> common pattern, where class device callbacks carry the corresponding class
> >> device struct (instead of the parent base struct device).
> >
> > TBH I'm not really convinced about this particular thing and I think I
> > can provide an illustrative example.
> >
> > Namely, quite incidentally, I've recently set out to add an RTC class
> > device to an existing driver, which is the ACPI time and alarm device
> > (TAD) one. The TAD functionality is based on ACPI control methods
> > provided by the platform firmware and it may (or may not) include
> > RTC-equivalent functions. So far, the driver has been providing a
> > completely custom sysfs interface to user space, but since more and
> > more platforms contain an ACPI TAD and some of them may not contain a
> > "traditional" RTC, having an RTC class device interface in that driver
> > may be quite useful.
> >
> > I have a prototype of the requisite change (I'll post it shortly for
> > reference) and it turns out that because the RTC class callbacks take
> > the parent device pointer as an argument, wrapping them around the
> > existing driver routines backing the existing sysfs interface is
> > super-straightforward. Had the RTC class passed an RTC device pointer
> > to those callbacks, the driver would have had to do something to get
> > back from it to the parent device (which is what the driver really
> > works with). If there are more similar drivers, that would have led
> > to some code duplication that is in fact unnecessary.
>
> The type system in Rust is powerfuly enough, so drivers can even get the exact
> type of the parent device the RTC device has as an additional argument to the
> callback. The infrastructure for this is in place and it is used by subsystems.
> I.e. in RTC we can do something like this:
>
> impl rtc::Ops for MyRtcOps {
> type BusDeviceType = platform::Device<Bound>;
>
> fn read_time(
> rtc: &rtc::Device<MyRtcData>,
> parent: &platform::Device<Bound>,
> time: rtc::Time,
> ) -> Result {
> ...
> }
> }
>
> where the corresponding rtc::Device::register() method would ensure that the
> associated BusDeviceType matches the parent device type that is passed to
> rtc::Device::register().
>
> A real example of this can be found in the LED class device abstractions [2].
>
> Note that in contrast to a bus device, class devices can be statically typed
> over their private data: rtc::Device<MyRtcData>.
>
> We usually also implment the Deref trait, such that rtc::Device<MyRtcData>
> automatically dereferences to MyRtcData.
>
> Having that said, if RTC drivers *never* have any private data that should be
> associated with the RTC device exclusively (i.e. data that is only relevant in
> the context of class device callbacks and class device infrastructure in
> general, or logically belongs to the class device), then the `rdev` argument
> would indeed be always unused and hence pointless.
>
> I usually would assume that there are such cases, but if that's really never the
> case for any RTC drivers, then I agree we should change the above code to:
>
> impl rtc::Ops for MyRtcOps {
> type BusDeviceType = platform::Device<Bound>;
>
> fn read_time(
> parent: &platform::Device<Bound>,
> time: rtc::Time,
> ) -> Result {
> ...
> }
> }
>
> This way it is at least clear what kind of device is passed through the class
> device callbacks.
>
> [2] https://lore.kernel.org/all/20260207-rust_leds-v12-1-fdb518417b75@posteo.de/
>
> > Moreover, the RTC device pointer doesn't even need to be stored
> > anywhere in that driver because the driver need not use it directly at
> > all and the RTC device object memory is freed by the core when the
> > driver unbinds.
>
> I don't think that is true, I think there are a few drivers accessing the RTC
> device from IRQs or workqueues.
I was referring to the specific case I had in mind, I didn't mean that
this was always the case. Sorry for being unclear.
> Besides that, quite a lot of RTC drivers actually seem to save a pointer to the
> struct rtc_device within their bus device private data, e.g. [3] and [4].
>
> [3] https://elixir.bootlin.com/linux/v6.19.2/source/drivers/rtc/rtc-ac100.c#L91
> [4] https://elixir.bootlin.com/linux/v6.19.2/source/drivers/rtc/rtc-cros-ec.c#L30
Right.
> >> Especially on the Rust side we now have the chance to make the experience of
> >> writing drivers as consistent as possible, which should help (new) driver
> >> authors a lot in terms of learning the driver lifetime patterns.
> >
> > Well, I'm not sure if "the experience of writing drivers as consistent
> > as possible" is more important than less code duplication and simpler
> > code.
>
> Yeah, it always depends, and if there *really* is no point in having any class
> device private data in RTC, then that's of course fine too.
>
> I just want to make sure we do not encourage to just through all the private
> data a driver may potentially have into the bus device private data struct.
>
> I saw this a lot in C drivers and it actually causes ownership and lifetime
> problems.
Sure.
Alexandre doesn't seem to be convinced that it is necessary to change
the RTC class device interface, so if there are any existing RTC
drivers that would benefit from doing so, it would be good to refer to
them specifically as examples. Alvin, can you find any?
^ permalink raw reply
* Re: [RFC PATCH v3 1/5] rtc: add device selector for rtc_class_ops callbacks
From: Danilo Krummrich @ 2026-02-22 16:13 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Alexandre Belloni, Alvin Sun, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, linux-rtc, rust-for-linux, Greg Kroah-Hartman
In-Reply-To: <CAJZ5v0gtiQxBCknkaOzLKrDqUQfhKh_UjQkvgxJBL4UthbCOkg@mail.gmail.com>
On Sun Feb 22, 2026 at 3:01 PM CET, Rafael J. Wysocki wrote:
> General considerations aside, regarding the $subject patch, I don't
> think that it is an improvement because it would confuse the existing
> RTC class device interface quite a bit.
>
> Regarding whether or not switching over the RTC class device interface
> to passing RTC device pointers to its class callbacks is a good idea,
> I think that the difference really boils down to what data needs to be
> accessed by the driver.
>
> As it stands today, in an RTC class callback, the driver gets directly
> to the parent of the given RTC device and if it does not need to get
> to the RTC device at all, that's convenient. However, if the driver
> needs to get to the RTC device, it will most likely need to use
> driver_data in the parent device for this purpose (and store the RTC
> device pointer).
>
> If the interface were changed to pass RTC device pointers to class
> callbacks, the driver would get to the RTC device and from there it
> would need to get to the parent device in the majority of cases. Even
> though getting from an RTC device to its parent device is
> straightforward (and does not require using driver_data at all), in
> some drivers that would be an extra step that would make the picture
> somewhat less clear than it is today (and arguably less clear than it
> really needs to be). Still, the change of the interface might allow
> some other drivers that do access the RTC device in class callback
> functions to be simplified.
>
> Granted, the above options had already been there when the RTC class
> interface was designed and the choice was made at that time. Revising
> it now would require clear technical reasons IMV.
>
> How hard would it be for Rust to cope with the existing calling
> convention in the RTC class?
There is no limitation from the Rust side of things, as mentioned in [1], this
should work perfectly fine, but it might be slightly less convinient:
Example 1:
impl rtc::Ops for MyRtcOps {
type BusDeviceType = platform::Device<Bound>;
fn read_time(
parent: &platform::Device<Bound>,
time: &mut rtc::Time,
) -> Result {
let drvdata = pdev.as_ref().drvdata::<MyDriver>()?;
...
}
}
We can improve this by avoiding that the driver has to extract its bus device
private data all the time by doing this instead:
Example 2:
impl rtc::Ops for MyRtcOps {
type BusDeviceType = platform::Device<Bound>;
type BusDeviceData = MyDriver;
fn read_time(
parent: &platform::Device<Bound>,
drvdata: &MyDriver,
time: &mut rtc::Time,
) -> Result {
...
}
}
However, this would require to type the rtc::Device over the bus device private
data type, which works perfectly fine, but arguably is a bit odd.
The alternative to make it convinient for drivers and avoid this oddity is to
just let drivers store the data that needs to be accessed from class device
callbacks in the class device private data and data that is only needed in bus
device callbacks in the bus device private data. Additionally, drivers can of
course save a refcount of the class device in their bus device private data as
well.
This is mostly what patch 5 of this series currently does (with a few
modifications) and I personally think it's the most clean approach in terms of
how it turns out for drivers:
Example 3:
// The bus device private data.
#[pin_data]
struct MyDriver {
#[pin]
irq: irq::Registration<MyIrqHandler>,
}
// The class device private data.
struct MyRtcData {
io: Devres<IoMem<PL031_REG_SIZE>>,
hw_variant: VendorVariant,
}
impl rtc::Ops for MyRtcOps {
type BusDeviceType = platform::Device<Bound>;
fn read_time(
rtc: &rtc::Device<MyRtcData>
parent: &platform::Device<Bound>,
time: &mut rtc::Time,
) -> Result {
let io = rtc.io.access(parent)?;
match rtc.hw_variant {
VendorVariant::Arm | VendorVariant::StV1 => {
let my_time = io.read(...);
my_time.write_into(time);
},
VendorVariant::StV2 => { ... },
}
}
}
(Note how we can directly access the `io` and `hw_variant` fields from `rtc`.)
If the data from struct MyRtcData is also needed from other bus callbacks, we
can simply extend struct MyDriver:
#[pin_data]
struct MyDriver {
#[pin]
irq: irq::Registration<MyIrqHandler>,
rtc: ARef<rtc::Device<MyRtcData>>,
}
But again, I just want to make sure we do not encourage to just throw all the
private data a driver may potentially have into the bus device private data
struct.
Besides that, it is of course a subsystem decision and I don't want to be
presumptuous or anything.
My recommendation is to go with something like in (3), otherwise I'd choose (1)
and (2) is my least favorite as I think it is a bit odd to type a class device
over bus device private data.
[1] https://lore.kernel.org/all/DGLLJ541SEJW.160MET6OCQHKS@kernel.org/
^ permalink raw reply
* Re: [GIT PULL] RTC for 7.0
From: pr-tracker-bot @ 2026-02-22 20:18 UTC (permalink / raw)
To: Alexandre Belloni; +Cc: Linus Torvalds, linux-rtc, linux-kernel
In-Reply-To: <202602220133264688bffa@mail.local>
The pull request you sent on Sun, 22 Feb 2026 02:33:26 +0100:
> git://git.kernel.org/pub/scm/linux/kernel/git/abelloni/linux.git tags/rtc-7.0
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/5f2eac7767493bf23e6552db82ab25de0dccd54f
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html
^ permalink raw reply
* [PATCH] rtc: pic32: allow driver to be compiled with COMPILE_TEST
From: Brian Masney @ 2026-02-22 23:30 UTC (permalink / raw)
To: Alexandre Belloni; +Cc: linux-rtc, linux-kernel, Brian Masney
This driver currently only supports builds against a PIC32 target. Now
that commit ed65ae9f6c6b ("rtc: pic32: update include to use pic32.h
from platform_data") is merged, it's possible to compile this driver on
other architectures.
To avoid future breakage of this driver in the future, let's update the
Kconfig so that it can be built with COMPILE_TEST enabled on all
architectures.
Signed-off-by: Brian Masney <bmasney@redhat.com>
---
drivers/rtc/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index b46ac73a2124e62f9232e9667da91fcb7ce36a62..364afc73f8abd2ade7e7570ba59c0f1aa0bbe78e 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -1986,7 +1986,7 @@ config RTC_DRV_XGENE
config RTC_DRV_PIC32
tristate "Microchip PIC32 RTC"
- depends on MACH_PIC32
+ depends on MACH_PIC32 || COMPILE_TEST
default y
help
If you say yes here you get support for the PIC32 RTC module.
---
base-commit: 6de23f81a5e08be8fbf5e8d7e9febc72a5b5f27f
change-id: 20260222-rtc-pic32-fce9ce197535
Best regards,
--
Brian Masney <bmasney@redhat.com>
^ permalink raw reply related
* [PATCH v2 0/2] rtc: max77686: convert to i2c_new_ancillary_device
From: Svyatoslav Ryhel @ 2026-02-23 6:43 UTC (permalink / raw)
To: Chanwoo Choi, Krzysztof Kozlowski, Lee Jones, Rob Herring,
Conor Dooley, Alexandre Belloni, Svyatoslav Ryhel
Cc: linux-kernel, devicetree, linux-rtc
Convert RTC I2C device creation from devm_i2c_new_dummy_device() to
i2c_new_ancillary_device() to enable the use of a device tree-specified
RTC address instead of a hardcoded value. If the device tree does not
provide an address, use hardcoded values as a fallback.
This addresses an issue with the MAX77663 PMIC, which can have the RTC at
different I2C positions (either 0x48, like the MAX77714, or 0x68, like
the MAX77620). The MAX77620 value is used as the default. The I2C position
of the MAX77663 is factory-set and cannot be detected from the chip
itself.
I have tested this patch on LG Optimus Vu P895 with max77663 PMIC and
non-default RTC position. RTC is registered correctly.
---
Changes in v2
- dropped patch that changes max77686 and adjusted max77620 where max77663
is described.
---
Svyatoslav Ryhel (2):
dt-bindings: mfd: max77620: document optional RTC address for max77663
rtc: max77686: convert to i2c_new_ancillary_device
Documentation/devicetree/bindings/mfd/max77620.txt | 4 +++-
drivers/rtc/rtc-max77686.c | 14 ++++++++++++--
2 files changed, 15 insertions(+), 3 deletions(-)
--
2.51.0
^ permalink raw reply
* [PATCH v2 1/2] dt-bindings: mfd: max77620: document optional RTC address for max77663
From: Svyatoslav Ryhel @ 2026-02-23 6:43 UTC (permalink / raw)
To: Chanwoo Choi, Krzysztof Kozlowski, Lee Jones, Rob Herring,
Conor Dooley, Alexandre Belloni, Svyatoslav Ryhel
Cc: linux-kernel, devicetree, linux-rtc
In-Reply-To: <20260223064343.12516-1-clamor95@gmail.com>
Document an optional second I2C address for the MAX77663 PMIC's RTC
device, to be used if the MAX77663 RTC is located at a non-default I2C
address.
Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
Documentation/devicetree/bindings/mfd/max77620.txt | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/mfd/max77620.txt b/Documentation/devicetree/bindings/mfd/max77620.txt
index 5a642a51d58e..0bbb42e5dcfe 100644
--- a/Documentation/devicetree/bindings/mfd/max77620.txt
+++ b/Documentation/devicetree/bindings/mfd/max77620.txt
@@ -6,10 +6,12 @@ Required properties:
"maxim,max77620"
"maxim,max20024"
"maxim,max77663"
-- reg: I2C device address.
+- reg: I2C device address. In case of MAX77663 there can be 2 addresses,
+ second one specifies position of RTC.
Optional properties:
-------------------
+- reg-names: If 2 addresses are specified then, must be "pmic", "rtc"
- interrupts: The interrupt on the parent the controller is
connected to.
- interrupt-controller: Marks the device node as an interrupt controller.
--
2.51.0
^ permalink raw reply related
* [PATCH v2 2/2] rtc: max77686: convert to i2c_new_ancillary_device
From: Svyatoslav Ryhel @ 2026-02-23 6:43 UTC (permalink / raw)
To: Chanwoo Choi, Krzysztof Kozlowski, Lee Jones, Rob Herring,
Conor Dooley, Alexandre Belloni, Svyatoslav Ryhel
Cc: linux-kernel, devicetree, linux-rtc
In-Reply-To: <20260223064343.12516-1-clamor95@gmail.com>
Convert RTC I2C device creation from devm_i2c_new_dummy_device() to
i2c_new_ancillary_device() to enable the use of a device tree-specified
RTC address instead of a hardcoded value. If the device tree does not
provide an address, use hardcoded values as a fallback.
This addresses an issue with the MAX77663 PMIC, which can have the RTC at
different I2C positions (either 0x48, like the MAX77714, or 0x68, like
the MAX77620). The MAX77620 value is used as the default. The I2C position
of the MAX77663 is factory-set and cannot be detected from the chip
itself.
Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
drivers/rtc/rtc-max77686.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/rtc/rtc-max77686.c b/drivers/rtc/rtc-max77686.c
index 69ea3ce75b5a..3cdfd78a07cc 100644
--- a/drivers/rtc/rtc-max77686.c
+++ b/drivers/rtc/rtc-max77686.c
@@ -686,6 +686,11 @@ static int max77686_rtc_init_reg(struct max77686_rtc_info *info)
return ret;
}
+static void max77686_rtc_release_dev(void *client)
+{
+ i2c_unregister_device(client);
+}
+
static int max77686_init_rtc_regmap(struct max77686_rtc_info *info)
{
struct device *parent = info->dev->parent;
@@ -713,12 +718,17 @@ static int max77686_init_rtc_regmap(struct max77686_rtc_info *info)
goto add_rtc_irq;
}
- client = devm_i2c_new_dummy_device(info->dev, parent_i2c->adapter,
- info->drv_data->rtc_i2c_addr);
+ client = i2c_new_ancillary_device(parent_i2c, "rtc",
+ info->drv_data->rtc_i2c_addr);
if (IS_ERR(client))
return dev_err_probe(info->dev, PTR_ERR(client),
"Failed to allocate I2C device for RTC\n");
+ ret = devm_add_action_or_reset(info->dev, max77686_rtc_release_dev,
+ client);
+ if (ret)
+ return ret;
+
info->rtc_regmap = devm_regmap_init_i2c(client,
info->drv_data->regmap_config);
if (IS_ERR(info->rtc_regmap))
--
2.51.0
^ permalink raw reply related
* Re: [PATCH v2 06/12] mfd: sec: add support for S2MU005 PMIC
From: Kaustabh Chakraborty @ 2026-02-23 13:56 UTC (permalink / raw)
To: Sander Vanheule, Kaustabh Chakraborty, André Draszik,
Lee Jones, Pavel Machek, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, MyungJoo Ham, Chanwoo Choi, Sebastian Reichel,
Krzysztof Kozlowski, Alexandre Belloni, Jonathan Corbet,
Shuah Khan
Cc: linux-leds, devicetree, linux-kernel, linux-pm, linux-samsung-soc,
linux-rtc, linux-doc
In-Reply-To: <4cf24e281fac45637fedf40944d8b5230c0e22d9.camel@svanheule.net>
On 2026-02-20 17:56 +01:00, Sander Vanheule wrote:
> Hi,
>
> On Thu, 2026-02-05 at 21:02 +0530, Kaustabh Chakraborty wrote:
>> On 2026-02-04 15:23 +00:00, André Draszik wrote:
>> > On Mon, 2026-01-26 at 00:37 +0530, Kaustabh Chakraborty wrote:
>> > > +static const struct regmap_config s2mu005_regmap_config = {
>> > > + .reg_bits = 8,
>> > > + .val_bits = 8,
>> > > +};
>> >
>> > No cache? And what is the .max_register value?
>> >
>>
>> This was in the previous revision, but I ended up removing it because
>> (at least I thought at that time) interfered with interrupts firing in
>> some way. The actual issue was unrelated, so I will add it back.
>>
>> However, there is also another thing I see in logs:
>>
>> sec-pmic-i2c 2-003d: using zero-initialized flat cache, this may cause
>> unexpected behavior
>>
>> This is due to REGCACHE_FLAT, I am not sure if I should just ignore
>> this.
>
> Sorry to be late to the party, but I'm somewhat responsible for that warning, so
> allow me to chime in :-)
>
> What you are might have been seeing is REGCACHE_FLAT giving you "cached" values
> of 0x0, while the hardware actually has something else. This can cause omitted
> writes, existing (bootloader) config to overwritten, etc.
>
> As André suggested, using .num_reg_defaults_raw is a possibility, but then you
> have to remember that the register defaults are taken to be what the hardware
> state is at that moment, including pre-probe changes. These defaults are used to
> seed the cache (so far, so good), but this may break the contract of
> regmap_sync() if you ever want to use that after actually resetting the PMIC.
>
> If you want to use the flat cache, I would suggest you use REGCACHE_FLAT_S,
> which will track what has already been read from/written to hardware. You will
> also need to specifiy .max_register.
I had figured that out by going through the logs, thanks. :)
>
> I see the other regmap_config-s in this driver also use REGCACHE_FLAT, so you
> may want to consider switching those over as well if these are also showing the
> new warning.
>
>
> Best,
> Sander
^ permalink raw reply
* [PATCH v1 8/8] ACPI: TAD/x86: cmos_rtc: Consolidate address space handler setup
From: Rafael J. Wysocki @ 2026-02-23 15:33 UTC (permalink / raw)
To: Linux ACPI; +Cc: LKML, x86 Maintainers, linux-rtc, Alexandre Belloni
In-Reply-To: <5983325.DvuYhMxLoT@rafael.j.wysocki>
From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
On x86, as a rule the CMOS RTC address space handler is set up by the
CMOS RTC ACPI scan handler attach callback, acpi_cmos_rtc_attach(),
but if the ACPI namespace does not contain a CMOS RTC device object,
the CMOS RTC address space handler installation is taken care of the
ACPI TAD (Timer and Alarm Device) driver.
This is not particularly straightforward and can be avoided by adding
the ACPI TAD device ID to the CMOS RTC ACPI scan handler which will
cause it to create a platform device for ACPI TAD after installing
the CMOS RTC address space handler. One related detail that needs to
be taken care of, though, is that the creation of an ACPI TAD platform
device should not cause cmos_rtc_platform_device_present to be set,
since this may cause add_rtc_cmos() to suppress the creation of a
fallback CMOS RTC platform device which may not be the right thing
to do (for instance, due to the fact that the ACPI TAD driver is
missing an RTC class device interface).
After doing the above, the CMOS RTC address space handler installation
and removal can be dropped from the ACPI TAD driver (which allows it to
be simplified quite a bit), acpi_remove_cmos_rtc_space_handler() can
be dropped and acpi_install_cmos_rtc_space_handler() can be made static.
Update the code as per the above.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/acpi/acpi_tad.c | 27 +++++----------------------
drivers/acpi/x86/cmos_rtc.c | 26 +++++---------------------
include/acpi/acpi_bus.h | 9 ---------
3 files changed, 10 insertions(+), 52 deletions(-)
diff --git a/drivers/acpi/acpi_tad.c b/drivers/acpi/acpi_tad.c
index 6d870d97ada6..4f5089fc023d 100644
--- a/drivers/acpi/acpi_tad.c
+++ b/drivers/acpi/acpi_tad.c
@@ -563,7 +563,6 @@ static int acpi_tad_disable_timer(struct device *dev, u32 timer_id)
static void acpi_tad_remove(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
- acpi_handle handle = ACPI_HANDLE(dev);
struct acpi_tad_driver_data *dd = dev_get_drvdata(dev);
device_init_wakeup(dev, false);
@@ -587,7 +586,6 @@ static void acpi_tad_remove(struct platform_device *pdev)
pm_runtime_suspend(dev);
pm_runtime_disable(dev);
- acpi_remove_cmos_rtc_space_handler(handle);
}
static int acpi_tad_probe(struct platform_device *pdev)
@@ -599,11 +597,6 @@ static int acpi_tad_probe(struct platform_device *pdev)
unsigned long long caps;
int ret;
- ret = acpi_install_cmos_rtc_space_handler(handle);
- if (ret < 0) {
- dev_info(dev, "Unable to install space handler\n");
- return -ENODEV;
- }
/*
* Initialization failure messages are mostly about firmware issues, so
* print them at the "info" level.
@@ -611,27 +604,22 @@ static int acpi_tad_probe(struct platform_device *pdev)
status = acpi_evaluate_integer(handle, "_GCP", NULL, &caps);
if (ACPI_FAILURE(status)) {
dev_info(dev, "Unable to get capabilities\n");
- ret = -ENODEV;
- goto remove_handler;
+ return -ENODEV;
}
if (!(caps & ACPI_TAD_AC_WAKE)) {
dev_info(dev, "Unsupported capabilities\n");
- ret = -ENODEV;
- goto remove_handler;
+ return -ENODEV;
}
if (!acpi_has_method(handle, "_PRW")) {
dev_info(dev, "Missing _PRW\n");
- ret = -ENODEV;
- goto remove_handler;
+ return -ENODEV;
}
dd = devm_kzalloc(dev, sizeof(*dd), GFP_KERNEL);
- if (!dd) {
- ret = -ENOMEM;
- goto remove_handler;
- }
+ if (!dd)
+ return -ENOMEM;
dd->capabilities = caps;
dev_set_drvdata(dev, dd);
@@ -673,11 +661,6 @@ static int acpi_tad_probe(struct platform_device *pdev)
fail:
acpi_tad_remove(pdev);
- /* Don't fallthrough because cmos rtc space handler is removed in acpi_tad_remove() */
- return ret;
-
-remove_handler:
- acpi_remove_cmos_rtc_space_handler(handle);
return ret;
}
diff --git a/drivers/acpi/x86/cmos_rtc.c b/drivers/acpi/x86/cmos_rtc.c
index a6df5b991c96..ced334e19896 100644
--- a/drivers/acpi/x86/cmos_rtc.c
+++ b/drivers/acpi/x86/cmos_rtc.c
@@ -18,13 +18,12 @@
#include "../internal.h"
static const struct acpi_device_id acpi_cmos_rtc_ids[] = {
+ { "ACPI000E", 1 }, /* ACPI Time and Alarm Device (TAD) */
ACPI_CMOS_RTC_IDS
};
bool cmos_rtc_platform_device_present;
-static bool cmos_rtc_space_handler_present __read_mostly;
-
static acpi_status acpi_cmos_rtc_space_handler(u32 function,
acpi_physical_address address,
u32 bits, u64 *value64,
@@ -56,8 +55,9 @@ static acpi_status acpi_cmos_rtc_space_handler(u32 function,
return AE_BAD_PARAMETER;
}
-int acpi_install_cmos_rtc_space_handler(acpi_handle handle)
+static int acpi_install_cmos_rtc_space_handler(acpi_handle handle)
{
+ static bool cmos_rtc_space_handler_present __read_mostly;
acpi_status status;
if (cmos_rtc_space_handler_present)
@@ -76,22 +76,6 @@ int acpi_install_cmos_rtc_space_handler(acpi_handle handle)
return 1;
}
-EXPORT_SYMBOL_GPL(acpi_install_cmos_rtc_space_handler);
-
-void acpi_remove_cmos_rtc_space_handler(acpi_handle handle)
-{
- acpi_status status;
-
- if (cmos_rtc_space_handler_present)
- return;
-
- status = acpi_remove_address_space_handler(handle,
- ACPI_ADR_SPACE_CMOS,
- acpi_cmos_rtc_space_handler);
- if (ACPI_FAILURE(status))
- pr_err("Failed to remove CMOS-RTC address space handler\n");
-}
-EXPORT_SYMBOL_GPL(acpi_remove_cmos_rtc_space_handler);
static int acpi_cmos_rtc_attach(struct acpi_device *adev,
const struct acpi_device_id *id)
@@ -103,9 +87,9 @@ static int acpi_cmos_rtc_attach(struct acpi_device *adev,
return ret;
if (IS_ERR_OR_NULL(acpi_create_platform_device(adev, NULL))) {
- pr_err("Failed to create CMOS-RTC platform device\n");
+ pr_err("Failed to create a platform device for %s\n", (char *)id->id);
return 0;
- } else {
+ } else if (!id->driver_data) {
cmos_rtc_platform_device_present = true;
}
return 1;
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index aad1a95e6863..be6d9032a161 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -760,8 +760,6 @@ int acpi_disable_wakeup_device_power(struct acpi_device *dev);
#ifdef CONFIG_X86
bool acpi_device_override_status(struct acpi_device *adev, unsigned long long *status);
bool acpi_quirk_skip_acpi_ac_and_battery(void);
-int acpi_install_cmos_rtc_space_handler(acpi_handle handle);
-void acpi_remove_cmos_rtc_space_handler(acpi_handle handle);
int acpi_quirk_skip_serdev_enumeration(struct device *controller_parent, bool *skip);
#else
static inline bool acpi_device_override_status(struct acpi_device *adev,
@@ -773,13 +771,6 @@ static inline bool acpi_quirk_skip_acpi_ac_and_battery(void)
{
return false;
}
-static inline int acpi_install_cmos_rtc_space_handler(acpi_handle handle)
-{
- return 1;
-}
-static inline void acpi_remove_cmos_rtc_space_handler(acpi_handle handle)
-{
-}
static inline int
acpi_quirk_skip_serdev_enumeration(struct device *controller_parent, bool *skip)
{
--
2.51.0
^ permalink raw reply related
* [PATCH v1 7/8] rtc: cmos: Drop PNP device support
From: Rafael J. Wysocki @ 2026-02-23 15:32 UTC (permalink / raw)
To: Linux ACPI; +Cc: LKML, x86 Maintainers, linux-rtc, Alexandre Belloni
In-Reply-To: <5983325.DvuYhMxLoT@rafael.j.wysocki>
From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
Previous changes effectively prevented PNP devices from being created
for the CMOS RTC on x86 with ACPI.
Although in principle a CMOS RTC PNP device may exist on an x86 system
without ACPI (that is, an x86 system where there is no ACPI at all, not
one booted with ACPI disabled), such systems were there in the field ~30
years ago and most likely they would not be able to run a contemporary
Linux kernel.
For the above reasons, drop the PNP device support from the rtc-cmos
driver.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/rtc/rtc-cmos.c | 113 +++--------------------------------------
1 file changed, 8 insertions(+), 105 deletions(-)
diff --git a/drivers/rtc/rtc-cmos.c b/drivers/rtc/rtc-cmos.c
index 7457f42fd6f0..9ac5bab846c1 100644
--- a/drivers/rtc/rtc-cmos.c
+++ b/drivers/rtc/rtc-cmos.c
@@ -1370,85 +1370,6 @@ static int __maybe_unused cmos_resume(struct device *dev)
static SIMPLE_DEV_PM_OPS(cmos_pm_ops, cmos_suspend, cmos_resume);
-/*----------------------------------------------------------------*/
-
-/* On non-x86 systems, a "CMOS" RTC lives most naturally on platform_bus.
- * ACPI systems always list these as PNPACPI devices, and pre-ACPI PCs
- * probably list them in similar PNPBIOS tables; so PNP is more common.
- *
- * We don't use legacy "poke at the hardware" probing. Ancient PCs that
- * predate even PNPBIOS should set up platform_bus devices.
- */
-
-#ifdef CONFIG_PNP
-
-#include <linux/pnp.h>
-
-static int cmos_pnp_probe(struct pnp_dev *pnp, const struct pnp_device_id *id)
-{
- int irq;
-
- if (pnp_port_start(pnp, 0) == 0x70 && !pnp_irq_valid(pnp, 0)) {
- irq = 0;
-#ifdef CONFIG_X86
- /* Some machines contain a PNP entry for the RTC, but
- * don't define the IRQ. It should always be safe to
- * hardcode it on systems with a legacy PIC.
- */
- if (nr_legacy_irqs())
- irq = RTC_IRQ;
-#endif
- } else {
- irq = pnp_irq(pnp, 0);
- }
-
- return cmos_do_probe(&pnp->dev, pnp_get_resource(pnp, IORESOURCE_IO, 0), irq);
-}
-
-static void cmos_pnp_remove(struct pnp_dev *pnp)
-{
- cmos_do_remove(&pnp->dev);
-}
-
-static void cmos_pnp_shutdown(struct pnp_dev *pnp)
-{
- struct device *dev = &pnp->dev;
- struct cmos_rtc *cmos = dev_get_drvdata(dev);
-
- if (system_state == SYSTEM_POWER_OFF) {
- int retval = cmos_poweroff(dev);
-
- if (cmos_aie_poweroff(dev) < 0 && !retval)
- return;
- }
-
- cmos_do_shutdown(cmos->irq);
-}
-
-static const struct pnp_device_id rtc_ids[] = {
- { .id = "PNP0b00", },
- { .id = "PNP0b01", },
- { .id = "PNP0b02", },
- { },
-};
-MODULE_DEVICE_TABLE(pnp, rtc_ids);
-
-static struct pnp_driver cmos_pnp_driver = {
- .name = driver_name,
- .id_table = rtc_ids,
- .probe = cmos_pnp_probe,
- .remove = cmos_pnp_remove,
- .shutdown = cmos_pnp_shutdown,
-
- /* flag ensures resume() gets called, and stops syslog spam */
- .flags = PNP_DRIVER_RES_DO_NOT_CHANGE,
- .driver = {
- .pm = &cmos_pm_ops,
- },
-};
-
-#endif /* CONFIG_PNP */
-
#ifdef CONFIG_OF
static const struct of_device_id of_cmos_match[] = {
{
@@ -1543,45 +1464,27 @@ static struct platform_driver cmos_platform_driver = {
}
};
-#ifdef CONFIG_PNP
-static bool pnp_driver_registered;
-#endif
static bool platform_driver_registered;
static int __init cmos_init(void)
{
- int retval = 0;
+ int retval;
-#ifdef CONFIG_PNP
- retval = pnp_register_driver(&cmos_pnp_driver);
- if (retval == 0)
- pnp_driver_registered = true;
-#endif
+ if (cmos_rtc.dev)
+ return 0;
- if (!cmos_rtc.dev) {
- retval = platform_driver_probe(&cmos_platform_driver,
- cmos_platform_probe);
- if (retval == 0)
- platform_driver_registered = true;
- }
+ retval = platform_driver_probe(&cmos_platform_driver, cmos_platform_probe);
+ if (retval)
+ return retval;
- if (retval == 0)
- return 0;
+ platform_driver_registered = true;
-#ifdef CONFIG_PNP
- if (pnp_driver_registered)
- pnp_unregister_driver(&cmos_pnp_driver);
-#endif
- return retval;
+ return 0;
}
module_init(cmos_init);
static void __exit cmos_exit(void)
{
-#ifdef CONFIG_PNP
- if (pnp_driver_registered)
- pnp_unregister_driver(&cmos_pnp_driver);
-#endif
if (platform_driver_registered)
platform_driver_unregister(&cmos_platform_driver);
}
--
2.51.0
^ permalink raw reply related
* [PATCH v1 6/8] x86: rtc: Drop PNP device check
From: Rafael J. Wysocki @ 2026-02-23 15:31 UTC (permalink / raw)
To: Linux ACPI; +Cc: LKML, x86 Maintainers, linux-rtc, Alexandre Belloni
In-Reply-To: <5983325.DvuYhMxLoT@rafael.j.wysocki>
From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
Previous changes effectively prevented PNP devices from being created
for the CMOS RTC on x86 with ACPI.
Although in principle a CMOS RTC PNP device may exist on an x86 system
without ACPI (that is, an x86 system where there is no ACPI at all, not
one booted with ACPI disabled), such systems were there in the field ~30
years ago and most likely they would not be able to run a contemporary
Linux kernel.
For the above reasons, drop the PNP device check from add_rtc_cmos().
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
arch/x86/kernel/rtc.c | 17 +----------------
1 file changed, 1 insertion(+), 16 deletions(-)
diff --git a/arch/x86/kernel/rtc.c b/arch/x86/kernel/rtc.c
index b112178e8185..314b062a15de 100644
--- a/arch/x86/kernel/rtc.c
+++ b/arch/x86/kernel/rtc.c
@@ -6,7 +6,6 @@
#include <linux/platform_device.h>
#include <linux/mc146818rtc.h>
#include <linux/export.h>
-#include <linux/pnp.h>
#include <asm/vsyscall.h>
#include <asm/x86_init.h>
@@ -134,19 +133,6 @@ static struct platform_device rtc_device = {
static __init int add_rtc_cmos(void)
{
-#ifdef CONFIG_PNP
- static const char * const ids[] __initconst =
- { "PNP0b00", "PNP0b01", "PNP0b02", };
- struct pnp_dev *dev;
- int i;
-
- pnp_for_each_dev(dev) {
- for (i = 0; i < ARRAY_SIZE(ids); i++) {
- if (compare_pnp_id(dev->id, ids[i]) != 0)
- return 0;
- }
- }
-#endif
if (cmos_rtc_platform_device_present)
return 0;
@@ -154,8 +140,7 @@ static __init int add_rtc_cmos(void)
return -ENODEV;
platform_device_register(&rtc_device);
- dev_info(&rtc_device.dev,
- "registered platform RTC device (no PNP device found)\n");
+ dev_info(&rtc_device.dev, "registered fallback platform RTC device\n");
return 0;
}
--
2.51.0
^ permalink raw reply related
* [PATCH v1 5/8] ACPI: PNP: Drop CMOS RTC PNP device support
From: Rafael J. Wysocki @ 2026-02-23 15:31 UTC (permalink / raw)
To: Linux ACPI; +Cc: LKML, x86 Maintainers, linux-rtc, Alexandre Belloni
In-Reply-To: <5983325.DvuYhMxLoT@rafael.j.wysocki>
From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
A previous change updated is_cmos_rtc_device() to effectively never
allow PNP devices to be created for the CMOS RTC on x86 with ACPI
and the PNP bus type is only used on x86, so the CMOS RTC device IDs
can be dropped from acpi_pnp_device_ids[] and is_cmos_rtc_device() can
go away completely.
Update the code accordingly.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/acpi/acpi_pnp.c | 22 +---------------------
1 file changed, 1 insertion(+), 21 deletions(-)
diff --git a/drivers/acpi/acpi_pnp.c b/drivers/acpi/acpi_pnp.c
index 4ad8f56d1a5d..da886923b008 100644
--- a/drivers/acpi/acpi_pnp.c
+++ b/drivers/acpi/acpi_pnp.c
@@ -125,10 +125,6 @@ static const struct acpi_device_id acpi_pnp_device_ids[] = {
{"PNP0401"}, /* ECP Printer Port */
/* apple-gmux */
{"APP000B"},
- /* rtc_cmos */
- {"PNP0b00"},
- {"PNP0b01"},
- {"PNP0b02"},
/* c6xdigio */
{"PNP0400"}, /* Standard LPT Printer Port */
{"PNP0401"}, /* ECP Printer Port */
@@ -355,25 +351,9 @@ static struct acpi_scan_handler acpi_pnp_handler = {
.attach = acpi_pnp_attach,
};
-/*
- * For CMOS RTC devices, the PNP ACPI scan handler does not work, because
- * there is a CMOS RTC ACPI scan handler installed already, so we need to
- * check those devices and enumerate them to the PNP bus directly.
- */
-static int is_cmos_rtc_device(struct acpi_device *adev)
-{
- static const struct acpi_device_id ids[] = {
- { "PNP0B00" },
- { "PNP0B01" },
- { "PNP0B02" },
- {""},
- };
- return !cmos_rtc_platform_device_present && !acpi_match_device_ids(adev, ids);
-}
-
bool acpi_is_pnp_device(struct acpi_device *adev)
{
- return adev->handler == &acpi_pnp_handler || is_cmos_rtc_device(adev);
+ return adev->handler == &acpi_pnp_handler;
}
EXPORT_SYMBOL_GPL(acpi_is_pnp_device);
--
2.51.0
^ permalink raw reply related
* [PATCH v1 4/8] ACPI: x86/rtc-cmos: Use platform device for driver binding
From: Rafael J. Wysocki @ 2026-02-23 15:30 UTC (permalink / raw)
To: Linux ACPI; +Cc: LKML, x86 Maintainers, linux-rtc, Alexandre Belloni
In-Reply-To: <5983325.DvuYhMxLoT@rafael.j.wysocki>
From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
Modify the rtc-cmos driver to bind to a platform device on systems with
ACPI via acpi_match_table and advertise the CMOST RTC ACPI device IDs
for driver auto-loading. Note that adding the requisite device IDs to
it and exposing them via MODULE_DEVICE_TABLE() is sufficient for this
purpose.
Since the ACPI device IDs in question are the same as for the CMOS RTC
ACPI scan handler, put them into a common header file and use the
definition from there in both places.
Additionally, to prevent a PNP device from being created for the CMOS
RTC if a platform one is present already, make is_cmos_rtc_device()
check cmos_rtc_platform_device_present introduced previously.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/acpi/acpi_pnp.c | 2 +-
drivers/acpi/x86/cmos_rtc.c | 5 +----
drivers/rtc/rtc-cmos.c | 10 ++++++++++
include/linux/acpi.h | 6 ++++++
4 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/drivers/acpi/acpi_pnp.c b/drivers/acpi/acpi_pnp.c
index 85d9f78619a2..4ad8f56d1a5d 100644
--- a/drivers/acpi/acpi_pnp.c
+++ b/drivers/acpi/acpi_pnp.c
@@ -368,7 +368,7 @@ static int is_cmos_rtc_device(struct acpi_device *adev)
{ "PNP0B02" },
{""},
};
- return !acpi_match_device_ids(adev, ids);
+ return !cmos_rtc_platform_device_present && !acpi_match_device_ids(adev, ids);
}
bool acpi_is_pnp_device(struct acpi_device *adev)
diff --git a/drivers/acpi/x86/cmos_rtc.c b/drivers/acpi/x86/cmos_rtc.c
index bdd66dfd4a44..a6df5b991c96 100644
--- a/drivers/acpi/x86/cmos_rtc.c
+++ b/drivers/acpi/x86/cmos_rtc.c
@@ -18,10 +18,7 @@
#include "../internal.h"
static const struct acpi_device_id acpi_cmos_rtc_ids[] = {
- { "PNP0B00" },
- { "PNP0B01" },
- { "PNP0B02" },
- {}
+ ACPI_CMOS_RTC_IDS
};
bool cmos_rtc_platform_device_present;
diff --git a/drivers/rtc/rtc-cmos.c b/drivers/rtc/rtc-cmos.c
index 0743c6acd6e2..7457f42fd6f0 100644
--- a/drivers/rtc/rtc-cmos.c
+++ b/drivers/rtc/rtc-cmos.c
@@ -27,6 +27,7 @@
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+#include <linux/acpi.h>
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/init.h>
@@ -1476,6 +1477,14 @@ static __init void cmos_of_init(struct platform_device *pdev)
#else
static inline void cmos_of_init(struct platform_device *pdev) {}
#endif
+
+#ifdef CONFIG_ACPI
+static const struct acpi_device_id acpi_cmos_rtc_ids[] = {
+ ACPI_CMOS_RTC_IDS
+};
+MODULE_DEVICE_TABLE(acpi, acpi_cmos_rtc_ids);
+#endif
+
/*----------------------------------------------------------------*/
/* Platform setup should have set up an RTC device, when PNP is
@@ -1530,6 +1539,7 @@ static struct platform_driver cmos_platform_driver = {
.name = driver_name,
.pm = &cmos_pm_ops,
.of_match_table = of_match_ptr(of_cmos_match),
+ .acpi_match_table = ACPI_PTR(acpi_cmos_rtc_ids),
}
};
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 2bdb801cee01..5ecdcdaf31aa 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -791,6 +791,12 @@ const char *acpi_get_subsystem_id(acpi_handle handle);
int acpi_mrrm_max_mem_region(void);
#endif
+#define ACPI_CMOS_RTC_IDS \
+ { "PNP0B00", }, \
+ { "PNP0B01", }, \
+ { "PNP0B02", }, \
+ { "", }
+
extern bool cmos_rtc_platform_device_present;
#else /* !CONFIG_ACPI */
--
2.51.0
^ permalink raw reply related
* [PATCH v1 3/8] ACPI: x86: cmos_rtc: Create a CMOS RTC platform device
From: Rafael J. Wysocki @ 2026-02-23 15:29 UTC (permalink / raw)
To: Linux ACPI; +Cc: LKML, x86 Maintainers, linux-rtc, Alexandre Belloni
In-Reply-To: <5983325.DvuYhMxLoT@rafael.j.wysocki>
From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
Make the CMOS RTC ACPI scan handler create a platform device that will
be used subsequently by rtc-cmos for driver binding on x86 systems with
ACPI and update add_rtc_cmos() to skip registering a fallback platform
device for the CMOS RTC when the above one has been registered.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
arch/x86/kernel/rtc.c | 4 ++++
drivers/acpi/x86/cmos_rtc.c | 8 ++++++++
include/linux/acpi.h | 4 ++++
3 files changed, 16 insertions(+)
diff --git a/arch/x86/kernel/rtc.c b/arch/x86/kernel/rtc.c
index 51a849a79c98..b112178e8185 100644
--- a/arch/x86/kernel/rtc.c
+++ b/arch/x86/kernel/rtc.c
@@ -2,6 +2,7 @@
/*
* RTC related functions
*/
+#include <linux/acpi.h>
#include <linux/platform_device.h>
#include <linux/mc146818rtc.h>
#include <linux/export.h>
@@ -146,6 +147,9 @@ static __init int add_rtc_cmos(void)
}
}
#endif
+ if (cmos_rtc_platform_device_present)
+ return 0;
+
if (!x86_platform.legacy.rtc)
return -ENODEV;
diff --git a/drivers/acpi/x86/cmos_rtc.c b/drivers/acpi/x86/cmos_rtc.c
index 45db7e51cbe6..bdd66dfd4a44 100644
--- a/drivers/acpi/x86/cmos_rtc.c
+++ b/drivers/acpi/x86/cmos_rtc.c
@@ -24,6 +24,8 @@ static const struct acpi_device_id acpi_cmos_rtc_ids[] = {
{}
};
+bool cmos_rtc_platform_device_present;
+
static bool cmos_rtc_space_handler_present __read_mostly;
static acpi_status acpi_cmos_rtc_space_handler(u32 function,
@@ -103,6 +105,12 @@ static int acpi_cmos_rtc_attach(struct acpi_device *adev,
if (ret < 0)
return ret;
+ if (IS_ERR_OR_NULL(acpi_create_platform_device(adev, NULL))) {
+ pr_err("Failed to create CMOS-RTC platform device\n");
+ return 0;
+ } else {
+ cmos_rtc_platform_device_present = true;
+ }
return 1;
}
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 4d2f0bed7a06..2bdb801cee01 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -791,6 +791,8 @@ const char *acpi_get_subsystem_id(acpi_handle handle);
int acpi_mrrm_max_mem_region(void);
#endif
+extern bool cmos_rtc_platform_device_present;
+
#else /* !CONFIG_ACPI */
#define acpi_disabled 1
@@ -1116,6 +1118,8 @@ static inline int acpi_mrrm_max_mem_region(void)
return 1;
}
+#define cmos_rtc_platform_device_present false
+
#endif /* !CONFIG_ACPI */
#ifdef CONFIG_ACPI_HMAT
--
2.51.0
^ permalink raw reply related
* [PATCH v1 2/8] ACPI: x86: cmos_rtc: Improve coordination with ACPI TAD driver
From: Rafael J. Wysocki @ 2026-02-23 15:28 UTC (permalink / raw)
To: Linux ACPI; +Cc: LKML, x86 Maintainers, linux-rtc, Alexandre Belloni
In-Reply-To: <5983325.DvuYhMxLoT@rafael.j.wysocki>
From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
If a CMOS RTC (PNP0B00/PNP0B01/PNP0B02) device coexists with an ACPI
TAD (timer and event alarm device, ACPI000E), the ACPI TAD driver will
attempt to install the CMOS RTC address space hanlder that has been
installed already and the TAD probing will fail.
Avoid that by changing acpi_install_cmos_rtc_space_handler() to return
zero and acpi_remove_cmos_rtc_space_handler() to do nothing if the CMOS
RTC address space handler has been installed already.
Fixes: 596ca52a56da ("ACPI: TAD: Install SystemCMOS address space handler for ACPI000E")
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/acpi/x86/cmos_rtc.c | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/drivers/acpi/x86/cmos_rtc.c b/drivers/acpi/x86/cmos_rtc.c
index 977234da9fc1..45db7e51cbe6 100644
--- a/drivers/acpi/x86/cmos_rtc.c
+++ b/drivers/acpi/x86/cmos_rtc.c
@@ -24,6 +24,8 @@ static const struct acpi_device_id acpi_cmos_rtc_ids[] = {
{}
};
+static bool cmos_rtc_space_handler_present __read_mostly;
+
static acpi_status acpi_cmos_rtc_space_handler(u32 function,
acpi_physical_address address,
u32 bits, u64 *value64,
@@ -59,6 +61,9 @@ int acpi_install_cmos_rtc_space_handler(acpi_handle handle)
{
acpi_status status;
+ if (cmos_rtc_space_handler_present)
+ return 0;
+
status = acpi_install_address_space_handler(handle,
ACPI_ADR_SPACE_CMOS,
acpi_cmos_rtc_space_handler,
@@ -68,6 +73,8 @@ int acpi_install_cmos_rtc_space_handler(acpi_handle handle)
return -ENODEV;
}
+ cmos_rtc_space_handler_present = true;
+
return 1;
}
EXPORT_SYMBOL_GPL(acpi_install_cmos_rtc_space_handler);
@@ -76,6 +83,9 @@ void acpi_remove_cmos_rtc_space_handler(acpi_handle handle)
{
acpi_status status;
+ if (cmos_rtc_space_handler_present)
+ return;
+
status = acpi_remove_address_space_handler(handle,
ACPI_ADR_SPACE_CMOS,
acpi_cmos_rtc_space_handler);
@@ -87,7 +97,13 @@ EXPORT_SYMBOL_GPL(acpi_remove_cmos_rtc_space_handler);
static int acpi_cmos_rtc_attach(struct acpi_device *adev,
const struct acpi_device_id *id)
{
- return acpi_install_cmos_rtc_space_handler(adev->handle);
+ int ret;
+
+ ret = acpi_install_cmos_rtc_space_handler(adev->handle);
+ if (ret < 0)
+ return ret;
+
+ return 1;
}
static struct acpi_scan_handler cmos_rtc_handler = {
--
2.51.0
^ permalink raw reply related
* [PATCH v1 1/8] ACPI: x86: cmos_rtc: Clean up address space handler driver
From: Rafael J. Wysocki @ 2026-02-23 15:28 UTC (permalink / raw)
To: Linux ACPI; +Cc: LKML, x86 Maintainers, linux-rtc, Alexandre Belloni
In-Reply-To: <5983325.DvuYhMxLoT@rafael.j.wysocki>
From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
Make multiple changes that do not alter functionality to the CMOS RTC
ACPI address space handler driver, including the following:
- Drop the unused .detach() callback from cmos_rtc_handler.
- Rename acpi_cmos_rtc_attach_handler() to acpi_cmos_rtc_attach().
- Rearrange acpi_cmos_rtc_space_handler() to reduce the number of
redundant checks and make white space follow the coding style.
- Adjust an error message in acpi_install_cmos_rtc_space_handler()
and make the white space follow the coding style.
- Rearrange acpi_remove_cmos_rtc_space_handler() and adjust an error
message in it.
No intentional functional impact.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/acpi/x86/cmos_rtc.c | 61 +++++++++++++++++++------------------
1 file changed, 32 insertions(+), 29 deletions(-)
diff --git a/drivers/acpi/x86/cmos_rtc.c b/drivers/acpi/x86/cmos_rtc.c
index 51643ff6fe5f..977234da9fc1 100644
--- a/drivers/acpi/x86/cmos_rtc.c
+++ b/drivers/acpi/x86/cmos_rtc.c
@@ -24,31 +24,35 @@ static const struct acpi_device_id acpi_cmos_rtc_ids[] = {
{}
};
-static acpi_status
-acpi_cmos_rtc_space_handler(u32 function, acpi_physical_address address,
- u32 bits, u64 *value64,
- void *handler_context, void *region_context)
+static acpi_status acpi_cmos_rtc_space_handler(u32 function,
+ acpi_physical_address address,
+ u32 bits, u64 *value64,
+ void *handler_context,
+ void *region_context)
{
- int i;
+ unsigned int i, bytes = DIV_ROUND_UP(bits, 8);
u8 *value = (u8 *)value64;
if (address > 0xff || !value64)
return AE_BAD_PARAMETER;
- if (function != ACPI_WRITE && function != ACPI_READ)
- return AE_BAD_PARAMETER;
+ guard(spinlock_irq)(&rtc_lock);
+
+ if (function == ACPI_WRITE) {
+ for (i = 0; i < bytes; i++, address++, value++)
+ CMOS_WRITE(*value, address);
- spin_lock_irq(&rtc_lock);
+ return AE_OK;
+ }
- for (i = 0; i < DIV_ROUND_UP(bits, 8); ++i, ++address, ++value)
- if (function == ACPI_READ)
+ if (function == ACPI_READ) {
+ for (i = 0; i < bytes; i++, address++, value++)
*value = CMOS_READ(address);
- else
- CMOS_WRITE(*value, address);
- spin_unlock_irq(&rtc_lock);
+ return AE_OK;
+ }
- return AE_OK;
+ return AE_BAD_PARAMETER;
}
int acpi_install_cmos_rtc_space_handler(acpi_handle handle)
@@ -56,11 +60,11 @@ int acpi_install_cmos_rtc_space_handler(acpi_handle handle)
acpi_status status;
status = acpi_install_address_space_handler(handle,
- ACPI_ADR_SPACE_CMOS,
- &acpi_cmos_rtc_space_handler,
- NULL, NULL);
+ ACPI_ADR_SPACE_CMOS,
+ acpi_cmos_rtc_space_handler,
+ NULL, NULL);
if (ACPI_FAILURE(status)) {
- pr_err("Error installing CMOS-RTC region handler\n");
+ pr_err("Failed to install CMOS-RTC address space handler\n");
return -ENODEV;
}
@@ -70,26 +74,25 @@ EXPORT_SYMBOL_GPL(acpi_install_cmos_rtc_space_handler);
void acpi_remove_cmos_rtc_space_handler(acpi_handle handle)
{
- if (ACPI_FAILURE(acpi_remove_address_space_handler(handle,
- ACPI_ADR_SPACE_CMOS, &acpi_cmos_rtc_space_handler)))
- pr_err("Error removing CMOS-RTC region handler\n");
+ acpi_status status;
+
+ status = acpi_remove_address_space_handler(handle,
+ ACPI_ADR_SPACE_CMOS,
+ acpi_cmos_rtc_space_handler);
+ if (ACPI_FAILURE(status))
+ pr_err("Failed to remove CMOS-RTC address space handler\n");
}
EXPORT_SYMBOL_GPL(acpi_remove_cmos_rtc_space_handler);
-static int acpi_cmos_rtc_attach_handler(struct acpi_device *adev, const struct acpi_device_id *id)
+static int acpi_cmos_rtc_attach(struct acpi_device *adev,
+ const struct acpi_device_id *id)
{
return acpi_install_cmos_rtc_space_handler(adev->handle);
}
-static void acpi_cmos_rtc_detach_handler(struct acpi_device *adev)
-{
- acpi_remove_cmos_rtc_space_handler(adev->handle);
-}
-
static struct acpi_scan_handler cmos_rtc_handler = {
.ids = acpi_cmos_rtc_ids,
- .attach = acpi_cmos_rtc_attach_handler,
- .detach = acpi_cmos_rtc_detach_handler,
+ .attach = acpi_cmos_rtc_attach,
};
void __init acpi_cmos_rtc_init(void)
--
2.51.0
^ 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