* Re: [PATCH 3/6] regulator: dt-bindings: sc2731: Deprecate compatible property
From: Mark Brown @ 2026-03-02 16:28 UTC (permalink / raw)
To: Otto Pflüger
Cc: Alexandre Belloni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Orson Zhai, Baolin Wang, Chunyan Zhang, Lee Jones, Pavel Machek,
Liam Girdwood, Sebastian Reichel, linux-rtc, devicetree,
linux-kernel, linux-leds, linux-pm
In-Reply-To: <20260222-sc27xx-mfd-cells-v1-3-69526fe74c77@abscue.de>
[-- Attachment #1: Type: text/plain, Size: 410 bytes --]
On Sun, Feb 22, 2026 at 02:16:47PM +0100, Otto Pflüger wrote:
> 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.
Acked-by: Mark Brown <broonie@kernel.org>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH 6/6] regulator: sc2731: Add platform_device_id table
From: Mark Brown @ 2026-03-02 16:06 UTC (permalink / raw)
To: Otto Pflüger
Cc: Alexandre Belloni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Orson Zhai, Baolin Wang, Chunyan Zhang, Lee Jones, Pavel Machek,
Liam Girdwood, Sebastian Reichel, linux-rtc, devicetree,
linux-kernel, linux-leds, linux-pm
In-Reply-To: <20260222-sc27xx-mfd-cells-v1-6-69526fe74c77@abscue.de>
[-- Attachment #1: Type: text/plain, Size: 468 bytes --]
On Sun, Feb 22, 2026 at 02:16:50PM +0100, Otto Pflüger wrote:
> 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.
Acked-by: Mark Brown <broonie@kernel.org>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* [PATCH] rtc: spear: check return value of clk_enable in resume
From: Zhaoyang Yu @ 2026-03-01 16:19 UTC (permalink / raw)
To: alexandre.belloni; +Cc: linux-rtc, linux-kernel, Zhaoyang Yu
In spear_rtc_resume(), the return value of clk_enable() is currently
ignored. If clk_enable() fails, the driver proceeds to call
spear_rtc_enable_interrupt().
The spear_rtc_enable_interrupt() function performs a readl() on the
RTC control register (CTRL_REG) as its first operation. Accessing an
MMIO register of a peripheral without an enabled functional clock is
unsafe on SPEAr architectures and can lead to a system hang or data
abort.
Fix this by checking the return value of clk_enable(). If it fails,
print an error message and return the error code to avoid the unsafe
register access.
Signed-off-by: Zhaoyang Yu <2426767509@qq.com>
---
drivers/rtc/rtc-spear.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/rtc/rtc-spear.c b/drivers/rtc/rtc-spear.c
index 959acff8faff..9bf7cf264715 100644
--- a/drivers/rtc/rtc-spear.c
+++ b/drivers/rtc/rtc-spear.c
@@ -437,7 +437,7 @@ static int spear_rtc_resume(struct device *dev)
{
struct platform_device *pdev = to_platform_device(dev);
struct spear_rtc_config *config = platform_get_drvdata(pdev);
- int irq;
+ int irq, ret;
irq = platform_get_irq(pdev, 0);
@@ -447,7 +447,11 @@ static int spear_rtc_resume(struct device *dev)
config->irq_wake = 0;
}
} else {
- clk_enable(config->clk);
+ ret = clk_enable(config->clk);
+ if (ret) {
+ dev_err(dev, "Unable to enable clock on resume: %d\n", ret);
+ return ret;
+ }
spear_rtc_enable_interrupt(config);
}
--
2.34.1
^ permalink raw reply related
* Re: [PATCH v4] dt-bindings: rtc: isl12026: convert to YAML schema
From: Krzysztof Kozlowski @ 2026-02-28 14:15 UTC (permalink / raw)
To: Piyush Patle, Alexandre Belloni
Cc: robh, krzk+dt, conor+dt, linux-rtc, devicetree, linux-kernel
In-Reply-To: <20260228140825.108205-1-piyushpatle228@gmail.com>
On 28/02/2026 15:08, Piyush Patle wrote:
> Convert the ISL12026 RTC binding from text format to YAML schema.
> Remove the legacy text binding.
>
> The new schema enables dtbs_check validation.
>
> ---
>
> Changes in v3:
> - Removed unsupported select section
>
> Changes in v2:
> - Fixed dt_binding_check warnings
> - Improved example formatting
>
> Changes in v4:
> - Fixed Warnings and resent in-thread as requested
What warnings?
I did not request it. And again you ignored the review. You kept sending
me some messages in private, but did you read the response where I asked
you read submitting patches?
I am dropping this as well. Alexandre can just pick up v3.
Best regards,
Krzysztof
^ permalink raw reply
* [PATCH v4] dt-bindings: rtc: isl12026: convert to YAML schema
From: Piyush Patle @ 2026-02-28 14:08 UTC (permalink / raw)
To: Alexandre Belloni
Cc: robh, krzk+dt, conor+dt, linux-rtc, devicetree, linux-kernel
In-Reply-To: <1c104639-4286-464c-aaf5-82f80b903bbb@kernel.org>
Convert the ISL12026 RTC binding from text format to YAML schema.
Remove the legacy text binding.
The new schema enables dtbs_check validation.
---
Changes in v3:
- Removed unsupported select section
Changes in v2:
- Fixed dt_binding_check warnings
- Improved example formatting
Changes in v4:
- Fixed Warnings and resent in-thread as requested
Signed-off-by: Piyush Patle <piyushpatle228@gmail.com>
---
.../devicetree/bindings/rtc/isil,isl12026.txt | 28 ---------
.../bindings/rtc/isil,isl12026.yaml | 59 +++++++++++++++++++
2 files changed, 59 insertions(+), 28 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/rtc/isil,isl12026.txt
create mode 100644 Documentation/devicetree/bindings/rtc/isil,isl12026.yaml
diff --git a/Documentation/devicetree/bindings/rtc/isil,isl12026.txt b/Documentation/devicetree/bindings/rtc/isil,isl12026.txt
deleted file mode 100644
index 2e0be45193bb..000000000000
--- a/Documentation/devicetree/bindings/rtc/isil,isl12026.txt
+++ /dev/null
@@ -1,28 +0,0 @@
-ISL12026 I2C RTC/EEPROM
-
-ISL12026 is an I2C RTC/EEPROM combination device. The RTC and control
-registers respond at bus address 0x6f, and the EEPROM array responds
-at bus address 0x57. The canonical "reg" value will be for the RTC portion.
-
-Required properties supported by the device:
-
- - "compatible": must be "isil,isl12026"
- - "reg": I2C bus address of the device (always 0x6f)
-
-Optional properties:
-
- - "isil,pwr-bsw": If present PWR.BSW bit must be set to the specified
- value for proper operation.
-
- - "isil,pwr-sbib": If present PWR.SBIB bit must be set to the specified
- value for proper operation.
-
-
-Example:
-
- rtc@6f {
- compatible = "isil,isl12026";
- reg = <0x6f>;
- isil,pwr-bsw = <0>;
- isil,pwr-sbib = <1>;
- }
diff --git a/Documentation/devicetree/bindings/rtc/isil,isl12026.yaml b/Documentation/devicetree/bindings/rtc/isil,isl12026.yaml
new file mode 100644
index 000000000000..152edce2ab41
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/isil,isl12026.yaml
@@ -0,0 +1,59 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/rtc/isil,isl12026.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Intersil ISL12026 I2C RTC/EEPROM
+
+maintainers:
+ - Piyush Patle <piyushpatle228@gmail.com>
+
+description:
+ The ISL12026 is a combination RTC and EEPROM device connected via I2C.
+ The RTC and control registers respond at address 0x6f, while the EEPROM
+ array responds at address 0x57. The "reg" property refers to the RTC
+ portion of the device.
+
+allOf:
+ - $ref: rtc.yaml#
+
+properties:
+ compatible:
+ const: isil,isl12026
+
+ reg:
+ maxItems: 1
+ description: I2C address of the RTC portion (must be 0x6f)
+
+ isil,pwr-bsw:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ enum: [ 0, 1 ]
+ description:
+ Value written to the PWR.BSW bit for proper device operation.
+
+ isil,pwr-sbib:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ enum: [ 0, 1 ]
+ description:
+ Value written to the PWR.SBIB bit for proper device operation.
+
+required:
+ - compatible
+ - reg
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ rtc@6f {
+ compatible = "isil,isl12026";
+ reg = <0x6f>;
+ isil,pwr-bsw = <0>;
+ isil,pwr-sbib = <1>;
+ };
+ };
--
2.34.1
^ permalink raw reply related
* Re: [PATCH v3] dt-bindings: rtc: isl12026: convert to YAML schema
From: Krzysztof Kozlowski @ 2026-02-28 13:26 UTC (permalink / raw)
To: Piyush Patle, Alexandre Belloni
Cc: robh, krzk+dt, conor+dt, linux-rtc, devicetree, linux-kernel
In-Reply-To: <20260228131311.37169-1-piyushpatle228@gmail.com>
On 28/02/2026 14:13, Piyush Patle wrote:
> Convert the ISL12026 RTC binding from text format to YAML schema.
> Remove the legacy text binding.
>
> The new schema enables dtbs_check validation.
>
> Signed-off-by: Piyush Patle <piyushpatle228@gmail.com>
>
> ---
>
> Changes in v3:
You already sent it and you got review, so now that duplicate posting
AFTER my review basically removes it.
I will not be doing work twice.
NAK for this patch
Best regards,
Krzysztof
^ permalink raw reply
* [PATCH v3] dt-bindings: rtc: isl12026: convert to YAML schema
From: Piyush Patle @ 2026-02-28 13:13 UTC (permalink / raw)
To: Alexandre Belloni
Cc: robh, krzk+dt, conor+dt, linux-rtc, devicetree, linux-kernel
Convert the ISL12026 RTC binding from text format to YAML schema.
Remove the legacy text binding.
The new schema enables dtbs_check validation.
Signed-off-by: Piyush Patle <piyushpatle228@gmail.com>
---
Changes in v3:
- Removed unsupported select section
Changes in v2:
- Fixed dt_binding_check warnings
- Improved example formatting
---
.../devicetree/bindings/rtc/isil,isl12026.txt | 28 ---------
.../bindings/rtc/isil,isl12026.yaml | 59 +++++++++++++++++++
2 files changed, 59 insertions(+), 28 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/rtc/isil,isl12026.txt
create mode 100644 Documentation/devicetree/bindings/rtc/isil,isl12026.yaml
diff --git a/Documentation/devicetree/bindings/rtc/isil,isl12026.txt b/Documentation/devicetree/bindings/rtc/isil,isl12026.txt
deleted file mode 100644
index 2e0be45193bb..000000000000
--- a/Documentation/devicetree/bindings/rtc/isil,isl12026.txt
+++ /dev/null
@@ -1,28 +0,0 @@
-ISL12026 I2C RTC/EEPROM
-
-ISL12026 is an I2C RTC/EEPROM combination device. The RTC and control
-registers respond at bus address 0x6f, and the EEPROM array responds
-at bus address 0x57. The canonical "reg" value will be for the RTC portion.
-
-Required properties supported by the device:
-
- - "compatible": must be "isil,isl12026"
- - "reg": I2C bus address of the device (always 0x6f)
-
-Optional properties:
-
- - "isil,pwr-bsw": If present PWR.BSW bit must be set to the specified
- value for proper operation.
-
- - "isil,pwr-sbib": If present PWR.SBIB bit must be set to the specified
- value for proper operation.
-
-
-Example:
-
- rtc@6f {
- compatible = "isil,isl12026";
- reg = <0x6f>;
- isil,pwr-bsw = <0>;
- isil,pwr-sbib = <1>;
- }
diff --git a/Documentation/devicetree/bindings/rtc/isil,isl12026.yaml b/Documentation/devicetree/bindings/rtc/isil,isl12026.yaml
new file mode 100644
index 000000000000..152edce2ab41
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/isil,isl12026.yaml
@@ -0,0 +1,59 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/rtc/isil,isl12026.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Intersil ISL12026 I2C RTC/EEPROM
+
+maintainers:
+ - Piyush Patle <piyushpatle228@gmail.com>
+
+description:
+ The ISL12026 is a combination RTC and EEPROM device connected via I2C.
+ The RTC and control registers respond at address 0x6f, while the EEPROM
+ array responds at address 0x57. The "reg" property refers to the RTC
+ portion of the device.
+
+allOf:
+ - $ref: rtc.yaml#
+
+properties:
+ compatible:
+ const: isil,isl12026
+
+ reg:
+ maxItems: 1
+ description: I2C address of the RTC portion (must be 0x6f)
+
+ isil,pwr-bsw:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ enum: [ 0, 1 ]
+ description:
+ Value written to the PWR.BSW bit for proper device operation.
+
+ isil,pwr-sbib:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ enum: [ 0, 1 ]
+ description:
+ Value written to the PWR.SBIB bit for proper device operation.
+
+required:
+ - compatible
+ - reg
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ rtc@6f {
+ compatible = "isil,isl12026";
+ reg = <0x6f>;
+ isil,pwr-bsw = <0>;
+ isil,pwr-sbib = <1>;
+ };
+ };
--
2.34.1
^ permalink raw reply related
* Re: [PATCH v3] dt-bindings: rtc: isl12026: convert to YAML schema
From: Krzysztof Kozlowski @ 2026-02-28 10:40 UTC (permalink / raw)
To: Piyush Patle
Cc: Alexandre Belloni, robh, krzk+dt, conor+dt, linux-rtc, devicetree,
linux-kernel
In-Reply-To: <20260227185115.174997-1-piyushpatle228@gmail.com>
On Sat, Feb 28, 2026 at 12:21:15AM +0530, Piyush Patle wrote:
> Convert the ISL12026 RTC binding from text format to YAML schema.
> Remove the legacy text binding.
>
> The new schema enables dtbs_check validation.
>
> Signed-off-by: Piyush Patle <piyushpatle228@gmail.com>
> ---
>
> Changes in v3:
> - Removed unsupported select section
>
> Changes in v2:
> - Fixed dt_binding_check warnings
> - Improved example formatting
>
> Signed-off-by: Piyush Patle <piyushpatle228@gmail.com>
Duplicate signature. Please run scripts/checkpatch.pl on the patches and
fix reported warnings. After that, run also 'scripts/checkpatch.pl
--strict' on the patches and (probably) fix more warnings. Some warnings
can be ignored, especially from --strict run, but the code here looks
like it needs a fix. Feel free to get in touch if the warning is not
clear.
Rest is fine and duplicated SoB might be just ignored due to cut
separator.
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Best regards,
Krzysztof
^ permalink raw reply
* [PATCH v3] dt-bindings: rtc: isl12026: convert to YAML schema
From: Piyush Patle @ 2026-02-27 18:51 UTC (permalink / raw)
To: Alexandre Belloni
Cc: robh, krzk+dt, conor+dt, linux-rtc, devicetree, linux-kernel
Convert the ISL12026 RTC binding from text format to YAML schema.
Remove the legacy text binding.
The new schema enables dtbs_check validation.
Signed-off-by: Piyush Patle <piyushpatle228@gmail.com>
---
Changes in v3:
- Removed unsupported select section
Changes in v2:
- Fixed dt_binding_check warnings
- Improved example formatting
Signed-off-by: Piyush Patle <piyushpatle228@gmail.com>
---
.../devicetree/bindings/rtc/isil,isl12026.txt | 28 ---------
.../bindings/rtc/isil,isl12026.yaml | 59 +++++++++++++++++++
2 files changed, 59 insertions(+), 28 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/rtc/isil,isl12026.txt
create mode 100644 Documentation/devicetree/bindings/rtc/isil,isl12026.yaml
diff --git a/Documentation/devicetree/bindings/rtc/isil,isl12026.txt b/Documentation/devicetree/bindings/rtc/isil,isl12026.txt
deleted file mode 100644
index 2e0be45193bb..000000000000
--- a/Documentation/devicetree/bindings/rtc/isil,isl12026.txt
+++ /dev/null
@@ -1,28 +0,0 @@
-ISL12026 I2C RTC/EEPROM
-
-ISL12026 is an I2C RTC/EEPROM combination device. The RTC and control
-registers respond at bus address 0x6f, and the EEPROM array responds
-at bus address 0x57. The canonical "reg" value will be for the RTC portion.
-
-Required properties supported by the device:
-
- - "compatible": must be "isil,isl12026"
- - "reg": I2C bus address of the device (always 0x6f)
-
-Optional properties:
-
- - "isil,pwr-bsw": If present PWR.BSW bit must be set to the specified
- value for proper operation.
-
- - "isil,pwr-sbib": If present PWR.SBIB bit must be set to the specified
- value for proper operation.
-
-
-Example:
-
- rtc@6f {
- compatible = "isil,isl12026";
- reg = <0x6f>;
- isil,pwr-bsw = <0>;
- isil,pwr-sbib = <1>;
- }
diff --git a/Documentation/devicetree/bindings/rtc/isil,isl12026.yaml b/Documentation/devicetree/bindings/rtc/isil,isl12026.yaml
new file mode 100644
index 000000000000..152edce2ab41
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/isil,isl12026.yaml
@@ -0,0 +1,59 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/rtc/isil,isl12026.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Intersil ISL12026 I2C RTC/EEPROM
+
+maintainers:
+ - Piyush Patle <piyushpatle228@gmail.com>
+
+description:
+ The ISL12026 is a combination RTC and EEPROM device connected via I2C.
+ The RTC and control registers respond at address 0x6f, while the EEPROM
+ array responds at address 0x57. The "reg" property refers to the RTC
+ portion of the device.
+
+allOf:
+ - $ref: rtc.yaml#
+
+properties:
+ compatible:
+ const: isil,isl12026
+
+ reg:
+ maxItems: 1
+ description: I2C address of the RTC portion (must be 0x6f)
+
+ isil,pwr-bsw:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ enum: [ 0, 1 ]
+ description:
+ Value written to the PWR.BSW bit for proper device operation.
+
+ isil,pwr-sbib:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ enum: [ 0, 1 ]
+ description:
+ Value written to the PWR.SBIB bit for proper device operation.
+
+required:
+ - compatible
+ - reg
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ rtc@6f {
+ compatible = "isil,isl12026";
+ reg = <0x6f>;
+ isil,pwr-bsw = <0>;
+ isil,pwr-sbib = <1>;
+ };
+ };
--
2.34.1
^ permalink raw reply related
* Re: [PATCH v3 03/13] dt-bindings: extcon: document Samsung S2M series PMIC extcon device
From: Kaustabh Chakraborty @ 2026-02-27 15:11 UTC (permalink / raw)
To: Kaustabh Chakraborty, Lee Jones, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, MyungJoo Ham, Chanwoo Choi,
Sebastian Reichel, Krzysztof Kozlowski, André Draszik,
Alexandre Belloni, Jonathan Corbet, Shuah Khan, Nam Tran
Cc: linux-leds, devicetree, linux-kernel, linux-pm, linux-samsung-soc,
linux-rtc, linux-doc
In-Reply-To: <20260225-s2mu005-pmic-v3-3-b4afee947603@disroot.org>
On 2026-02-25 00:45 +05:30, Kaustabh Chakraborty wrote:
> Certain Samsung S2M series PMICs have a MUIC device which reports
> various cable states by measuring the ID-GND resistance with an internal
> ADC. Document the devicetree schema for this device.
>
> Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
> ---
> .../bindings/extcon/samsung,s2mu005-muic.yaml | 39 ++++++++++++++++++++++
> 1 file changed, 39 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/extcon/samsung,s2mu005-muic.yaml b/Documentation/devicetree/bindings/extcon/samsung,s2mu005-muic.yaml
> new file mode 100644
> index 0000000000000..e047e8cbc264e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/extcon/samsung,s2mu005-muic.yaml
> @@ -0,0 +1,39 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/extcon/samsung,s2mu005-muic.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MUIC Device for Samsung S2M series PMICs
> +
> +maintainers:
> + - Kaustabh Chakraborty <kauschluss@disroot.org>
> +
> +description: |
> + The Samsung S2M series PMIC MUIC device is a USB port accessory
> + detector. It reports multiple states depending on the ID-GND
> + resistance measured by an internal ADC.
> +
> + This is a part of device tree bindings for S2M and S5M family of Power
> + Management IC (PMIC).
> +
> + See also Documentation/devicetree/bindings/mfd/samsung,s2mps11.yaml for
> + additional information and example.
> +
> +properties:
> + compatible:
> + enum:
> + - samsung,s2mu005-muic
> +
> + connector:
> + $ref: /schemas/connector/usb-connector.yaml#
> +
> + port:
> + $ref: /schemas/graph.yaml#/properties/port
A v1 review from Conor says:
Why does this need a dedicated child node for just a port property?
In v3, connector is added. This now has the same properties as
maxim,max14526. If this still applies, it would be nice to have more
insight...
> +
> +required:
> + - compatible
> + - connector
> + - port
> +
> +additionalProperties: false
^ permalink raw reply
* Re: [RFC PATCH v3 1/5] rtc: add device selector for rtc_class_ops callbacks
From: Benno Lossin @ 2026-02-27 15:09 UTC (permalink / raw)
To: Danilo Krummrich, Rafael J. Wysocki
Cc: Alexandre Belloni, Alvin Sun, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
linux-rtc, rust-for-linux, Greg Kroah-Hartman
In-Reply-To: <DGO6MEKIIHGH.3L06QJ47CP3CU@kernel.org>
On Wed Feb 25, 2026 at 5:26 PM CET, Danilo Krummrich wrote:
> For the future we will also be able to support references within initializers to
> other pinned fields, which make things a bit more convinient, so you could do
> things like this:
>
> let irq_data = impl_pin_init!(SampleIrqData {
> io <- pdev.iomap_region_sized::<BAR0_SIZE>(0, c"my_rtc/bar0")?,
> hw_variant: VendorVariant::StV1,
> });
>
> let rtc_data = impl_pin_init!(SampleRtcData {
> irq <- irq::Registration::new(..., irq_data),
> io: &irq.io,
> ...,
> });
>
> let rtc = rtc::Device::new(dev, rtc_data)?;
>
> Note the additional `io: &irq.io,` in the rtc_data initializer. This would be
> legal as we know that `irq` is pinned within `rtc_data`, hence it is valid to
> hold a reference to one of its pinned fields.
>
> I am not sure how far we are from having this supported, I assume Benno and Gary
> can say more about this.
We added that support in 42415d163e5d ("rust: pin-init: add references
to previously initialized fields"). Note that any references created
during the initializer are only valid inside of that initializer. You
can convert into a raw pointer though and if the struct is `!Unpin` it
is sound to use that pointer at the same time as references to the
pinned value.
Cheers,
Benno
^ permalink raw reply
* Re: [PATCH v3 04/13] dt-bindings: power: supply: document Samsung S2M series PMIC charger device
From: Kaustabh Chakraborty @ 2026-02-27 14:26 UTC (permalink / raw)
To: Krzysztof Kozlowski, Kaustabh Chakraborty
Cc: Lee Jones, Pavel Machek, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, MyungJoo Ham, Chanwoo Choi, Sebastian Reichel,
André Draszik, Alexandre Belloni, Jonathan Corbet,
Shuah Khan, Nam Tran, linux-leds, devicetree, linux-kernel,
linux-pm, linux-samsung-soc, linux-rtc, linux-doc
In-Reply-To: <20260225-secret-amusing-cuttlefish-d3bee5@quoll>
On 2026-02-25 11:44 +01:00, Krzysztof Kozlowski wrote:
> On Wed, Feb 25, 2026 at 12:45:06AM +0530, Kaustabh Chakraborty wrote:
>> +
>> + This is a part of device tree bindings for S2M and S5M family of Power
>> + Management IC (PMIC).
>> +
>> + See also Documentation/devicetree/bindings/mfd/samsung,s2mps11.yaml for
>> + additional information and example.
>> +
>> +allOf:
>> + - $ref: power-supply.yaml#
>> +
>> +properties:
>> + compatible:
>> + enum:
>> + - samsung,s2mu005-charger
>
> Review from v1 still applies. I think you ignored several reviews, so I
> will mark entire patchset as changes requested.
Somehow I missed this one... anyways I address them here:
Why do you need a dedicated child node for this? It's got one property,
other than the compatible, that you're using. It could easily just go
in the parent without a dedicated node etc.
The dt node also references a simple-battery node, that's why it's
required.
>
> Best regards,
> Krzysztof
^ permalink raw reply
* Re: [PATCH v1 7/8] rtc: cmos: Drop PNP device support
From: Alexandre Belloni @ 2026-02-26 13:02 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: Linux ACPI, LKML, x86 Maintainers, linux-rtc
In-Reply-To: <2355012.iZASKD2KPV@rafael.j.wysocki>
On 23/02/2026 16:32:29+0100, Rafael J. Wysocki wrote:
> 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>
Acked-by: Alexandre Belloni <alexandre.belloni@bootlin.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
>
>
>
>
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH v1 4/8] ACPI: x86/rtc-cmos: Use platform device for driver binding
From: Alexandre Belloni @ 2026-02-26 13:01 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: Linux ACPI, LKML, x86 Maintainers, linux-rtc
In-Reply-To: <13969123.uLZWGnKmhe@rafael.j.wysocki>
On 23/02/2026 16:30:21+0100, Rafael J. Wysocki wrote:
> 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>
Acked-by: Alexandre Belloni <alexandre.belloni@bootlin.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
>
>
>
>
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [RFC PATCH v3 1/5] rtc: add device selector for rtc_class_ops callbacks
From: Rafael J. Wysocki @ 2026-02-26 12:28 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Rafael J. Wysocki, 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: <DGO6MEKIIHGH.3L06QJ47CP3CU@kernel.org>
On Wed, Feb 25, 2026 at 5:26 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> On Wed Feb 25, 2026 at 2:33 PM CET, Rafael J. Wysocki wrote:
> > On Tue, Feb 24, 2026 at 11:23 PM Danilo Krummrich <dakr@kernel.org> wrote:
> >> Here's also some sketched up code for what I wrote above:
> >>
> >> fn probe(pdev: &pci::Device<Core>, info: &Self::IdInfo) -> impl PinInit<Self, Error> {
> >> let dev = pdev.as_ref();
> >>
> >> let rtc_data = impl_pin_init!(SampleRtcData {
> >> io: pdev.iomap_region_sized::<BAR0_SIZE>(0, c"my_rtc/bar0")?,
> >> hw_variant: VendorVariant::StV1,
> >> irq <- irq::Registration::new(...),
> >> });
> >>
> >> let rtc = rtc::Device::new(dev, rtc_data)?;
> >>
> >> rtc::Registration::register(rtc)?;
> >>
> >> Ok(Self { rtc })
> >> }
> >>
> >> Note that if any of the RTC callbacks would ever need to call irq.synchronize(),
> >> irq.disable(), etc. the compiler would enforce correct ordering, as there would
> >> not be any other possibility to put the irq::Registration other than into the
> >> rtc_data that goes into rtc::Device::new().
> >
> > IIUC, the interrupt handler can only access the rtc_data because the
> > parent's driver_data may not exist yet when it runs. Or am I missing
> > something?
>
> In the code above the IRQ handler can also not access rtc_data, as struct
> SampleRtcData might not be fully initialized when it runs, i.e.
>
> let rtc_data = impl_pin_init!(SampleRtcData {
> io: pdev.iomap_region_sized::<BAR0_SIZE>(0, c"my_rtc/bar0")?,
> hw_variant: VendorVariant::StV1,
> irq <- irq::Registration::new(..., rtc_data),
> });
>
> would not compile in the first place.
>
> irq::Registration, for this purpose, has its own private data on the handler
> itself, see also [1]. In fact, the C code has the same concept with the dev_id
> argument in request_threaded_irq() [2].
>
> The difference is that the C compiler does not ensure that the IRQ handler
> actually owns the data behind the dev_id pointer. I.e. the driver has to somehow
> ensure that whatever is behind the dev_id pointer remains valid for the duration
> the IRQ handler is registered.
>
> In the Rust implementation the compiler does ensure that what is behind the
> dev_id pointer remains valid for the duration of the lifetime of the
> irq::Registration.
>
> Having that said, I assume you wonder what we would pass into the
> irq::Registration instead, if it is not rtc_data.
>
> The answer is it depends; it depends on what's actually needed, what other
> entities interact with the IRQ (e.g. some scheduled work, etc.) and maybe even
> preference to some extend.
>
> Here is one example:
>
> let irq_data = impl_pin_init!(SampleIrqData {
> io <- pdev.iomap_region_sized::<BAR0_SIZE>(0, c"my_rtc/bar0")?,
> hw_variant: VendorVariant::StV1,
> });
>
> let rtc_data = impl_pin_init!(SampleRtcData {
> irq <- irq::Registration::new(..., irq_data),
> ...,
> });
>
> let rtc = rtc::Device::new(dev, rtc_data)?;
>
> This would compile as it ensures that irq_data (struct SampleIrqData) is fully
> initialized before irq::Registration::new() is called.
>
> At a first glance this might look like we need an additional allocation, one for
> irq_data and one for rtc_data, but that is not the case. irq_data is an
> initializer that is passed to another initializer, i.e. rtc_data is still an
> initializer.
>
> The actual (single) allocation happens in rtc::Device::new().
I think that the key observation here is that C and Rust are
substantially different with respect to how things get initialized.
In C, we first allocate memory, then initialize it, and then start
services that will refer to it. All of these steps need to be taken
explicitly and separately in the right order and the compiler simply
processes the high-level language into CPU instructions. On the way
out, all of that needs to be cleaned up directly, most of the time in
reverse order. If anything is missed or forgotten, or the ordering is
messed up, troubles ensue.
In Rust, IIUC, the compiler is essentially told about what data will
be there in the memory, how to initialize it and what services to
start and all of that happens in one go when memory gets allocated
(so, apparently, a good part of the code doesn't even produce CPU
instructions at all, as it is all about feeding the information to the
compiler). So long as the compiler has complete information, it can
figure out the right ordering automatically and it will complain if
something is not right. The difficulty here is to find a way to
provide the compiler with complete information.
> In terms of accessing it through the the rtc::Device in an RTC device callback,
> we would likely use accessor methods to make it a bit more convinient, i.e.
>
> fn read_time(
> rtc: &rtc::Device<SampleRtcData>
> 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 => { ... },
> }
> }
>
> As mentioned above there are a few other options to implement this, depending on
> what's required, etc.
>
> For instance, if the I/O bar is actually shared between multiple entities we
> might want to initialize it within an Arc [3] (reference count it) for shared
> ownership.
>
> For the future we will also be able to support references within initializers to
> other pinned fields, which make things a bit more convinient, so you could do
> things like this:
>
> let irq_data = impl_pin_init!(SampleIrqData {
> io <- pdev.iomap_region_sized::<BAR0_SIZE>(0, c"my_rtc/bar0")?,
> hw_variant: VendorVariant::StV1,
> });
>
> let rtc_data = impl_pin_init!(SampleRtcData {
> irq <- irq::Registration::new(..., irq_data),
> io: &irq.io,
> ...,
> });
>
> let rtc = rtc::Device::new(dev, rtc_data)?;
>
> Note the additional `io: &irq.io,` in the rtc_data initializer. This would be
> legal as we know that `irq` is pinned within `rtc_data`, hence it is valid to
> hold a reference to one of its pinned fields.
>
> I am not sure how far we are from having this supported, I assume Benno and Gary
> can say more about this.
>
> I hope this helps, and thanks for asking those questions!
>
> [1] https://rust.docs.kernel.org/kernel/irq/struct.Registration.html
> [2] https://elixir.bootlin.com/linux/v6.19.3/source/kernel/irq/manage.c#L2090
> [3] https://rust.docs.kernel.org/kernel/sync/struct.Arc.html
^ permalink raw reply
* Re: [RFC PATCH v3 1/5] rtc: add device selector for rtc_class_ops callbacks
From: Rafael J. Wysocki @ 2026-02-25 21:15 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Rafael J. Wysocki, 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: <DGO6MEKIIHGH.3L06QJ47CP3CU@kernel.org>
On Wed, Feb 25, 2026 at 5:26 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> On Wed Feb 25, 2026 at 2:33 PM CET, Rafael J. Wysocki wrote:
> > On Tue, Feb 24, 2026 at 11:23 PM Danilo Krummrich <dakr@kernel.org> wrote:
> >> Here's also some sketched up code for what I wrote above:
> >>
> >> fn probe(pdev: &pci::Device<Core>, info: &Self::IdInfo) -> impl PinInit<Self, Error> {
> >> let dev = pdev.as_ref();
> >>
> >> let rtc_data = impl_pin_init!(SampleRtcData {
> >> io: pdev.iomap_region_sized::<BAR0_SIZE>(0, c"my_rtc/bar0")?,
> >> hw_variant: VendorVariant::StV1,
> >> irq <- irq::Registration::new(...),
> >> });
> >>
> >> let rtc = rtc::Device::new(dev, rtc_data)?;
> >>
> >> rtc::Registration::register(rtc)?;
> >>
> >> Ok(Self { rtc })
> >> }
> >>
> >> Note that if any of the RTC callbacks would ever need to call irq.synchronize(),
> >> irq.disable(), etc. the compiler would enforce correct ordering, as there would
> >> not be any other possibility to put the irq::Registration other than into the
> >> rtc_data that goes into rtc::Device::new().
> >
> > IIUC, the interrupt handler can only access the rtc_data because the
> > parent's driver_data may not exist yet when it runs. Or am I missing
> > something?
>
> In the code above the IRQ handler can also not access rtc_data, as struct
> SampleRtcData might not be fully initialized when it runs, i.e.
>
> let rtc_data = impl_pin_init!(SampleRtcData {
> io: pdev.iomap_region_sized::<BAR0_SIZE>(0, c"my_rtc/bar0")?,
> hw_variant: VendorVariant::StV1,
> irq <- irq::Registration::new(..., rtc_data),
> });
>
> would not compile in the first place.
>
> irq::Registration, for this purpose, has its own private data on the handler
> itself, see also [1]. In fact, the C code has the same concept with the dev_id
> argument in request_threaded_irq() [2].
>
> The difference is that the C compiler does not ensure that the IRQ handler
> actually owns the data behind the dev_id pointer. I.e. the driver has to somehow
> ensure that whatever is behind the dev_id pointer remains valid for the duration
> the IRQ handler is registered.
>
> In the Rust implementation the compiler does ensure that what is behind the
> dev_id pointer remains valid for the duration of the lifetime of the
> irq::Registration.
>
> Having that said, I assume you wonder what we would pass into the
> irq::Registration instead, if it is not rtc_data.
>
> The answer is it depends; it depends on what's actually needed, what other
> entities interact with the IRQ (e.g. some scheduled work, etc.) and maybe even
> preference to some extend.
>
> Here is one example:
>
> let irq_data = impl_pin_init!(SampleIrqData {
> io <- pdev.iomap_region_sized::<BAR0_SIZE>(0, c"my_rtc/bar0")?,
> hw_variant: VendorVariant::StV1,
> });
>
> let rtc_data = impl_pin_init!(SampleRtcData {
> irq <- irq::Registration::new(..., irq_data),
> ...,
> });
>
> let rtc = rtc::Device::new(dev, rtc_data)?;
>
> This would compile as it ensures that irq_data (struct SampleIrqData) is fully
> initialized before irq::Registration::new() is called.
>
> At a first glance this might look like we need an additional allocation, one for
> irq_data and one for rtc_data, but that is not the case. irq_data is an
> initializer that is passed to another initializer, i.e. rtc_data is still an
> initializer.
>
> The actual (single) allocation happens in rtc::Device::new().
>
> In terms of accessing it through the the rtc::Device in an RTC device callback,
> we would likely use accessor methods to make it a bit more convinient, i.e.
>
> fn read_time(
> rtc: &rtc::Device<SampleRtcData>
> 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 => { ... },
> }
> }
>
> As mentioned above there are a few other options to implement this, depending on
> what's required, etc.
>
> For instance, if the I/O bar is actually shared between multiple entities we
> might want to initialize it within an Arc [3] (reference count it) for shared
> ownership.
>
> For the future we will also be able to support references within initializers to
> other pinned fields, which make things a bit more convinient, so you could do
> things like this:
>
> let irq_data = impl_pin_init!(SampleIrqData {
> io <- pdev.iomap_region_sized::<BAR0_SIZE>(0, c"my_rtc/bar0")?,
> hw_variant: VendorVariant::StV1,
> });
>
> let rtc_data = impl_pin_init!(SampleRtcData {
> irq <- irq::Registration::new(..., irq_data),
> io: &irq.io,
> ...,
> });
>
> let rtc = rtc::Device::new(dev, rtc_data)?;
>
> Note the additional `io: &irq.io,` in the rtc_data initializer. This would be
> legal as we know that `irq` is pinned within `rtc_data`, hence it is valid to
> hold a reference to one of its pinned fields.
>
> I am not sure how far we are from having this supported, I assume Benno and Gary
> can say more about this.
>
> I hope this helps, and thanks for asking those questions!
Well, I'm using an opportunity to learn something, thank you!
> [1] https://rust.docs.kernel.org/kernel/irq/struct.Registration.html
> [2] https://elixir.bootlin.com/linux/v6.19.3/source/kernel/irq/manage.c#L2090
> [3] https://rust.docs.kernel.org/kernel/sync/struct.Arc.html
^ permalink raw reply
* Re: [PATCH v1 6/8] x86: rtc: Drop PNP device check
From: Rafael J. Wysocki @ 2026-02-25 18:10 UTC (permalink / raw)
To: Dave Hansen
Cc: Rafael J. Wysocki, Linux ACPI, LKML, x86 Maintainers, linux-rtc,
Alexandre Belloni
In-Reply-To: <6b751b23-ded2-4343-bf29-103f4a7ab6ba@intel.com>
On Wed, Feb 25, 2026 at 7:01 PM Dave Hansen <dave.hansen@intel.com> wrote:
>
> On 2/23/26 07:31, Rafael J. Wysocki wrote:
> > 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().
>
> If someone had one of these devices, what would they see after applying
> this? Would they just not be able to detect the RTC any longer?
No, add_rtc_cmos() would just create a fallback platform device for
the CMOS RTC AFAICS and the driver would pick up that one.
> In any case, this does seem obscure enough to not worry about any more:
>
> Acked-by: Dave Hansen <dave.hansen@linux.intel.com> # x86
Thanks!
^ permalink raw reply
* Re: [PATCH v1 3/8] ACPI: x86: cmos_rtc: Create a CMOS RTC platform device
From: Dave Hansen @ 2026-02-25 18:01 UTC (permalink / raw)
To: Rafael J. Wysocki, Linux ACPI
Cc: LKML, x86 Maintainers, linux-rtc, Alexandre Belloni
In-Reply-To: <1962427.tdWV9SEqCh@rafael.j.wysocki>
On 2/23/26 07:29, Rafael J. Wysocki wrote:
> 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.
Acked-by: Dave Hansen <dave.hansen@linux.intel.com> # x86
^ permalink raw reply
* Re: [PATCH v1 6/8] x86: rtc: Drop PNP device check
From: Dave Hansen @ 2026-02-25 18:01 UTC (permalink / raw)
To: Rafael J. Wysocki, Linux ACPI
Cc: LKML, x86 Maintainers, linux-rtc, Alexandre Belloni
In-Reply-To: <8660687.T7Z3S40VBb@rafael.j.wysocki>
On 2/23/26 07:31, Rafael J. Wysocki wrote:
> 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().
If someone had one of these devices, what would they see after applying
this? Would they just not be able to detect the RTC any longer?
In any case, this does seem obscure enough to not worry about any more:
Acked-by: Dave Hansen <dave.hansen@linux.intel.com> # x86
^ permalink raw reply
* Re: [RFC PATCH v3 1/5] rtc: add device selector for rtc_class_ops callbacks
From: Danilo Krummrich @ 2026-02-25 16:26 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: <CAJZ5v0iA88G0ZRVB347dXEu2y8mT=d+aWd42cB2tpO5pLVpKuQ@mail.gmail.com>
On Wed Feb 25, 2026 at 2:33 PM CET, Rafael J. Wysocki wrote:
> On Tue, Feb 24, 2026 at 11:23 PM Danilo Krummrich <dakr@kernel.org> wrote:
>> Here's also some sketched up code for what I wrote above:
>>
>> fn probe(pdev: &pci::Device<Core>, info: &Self::IdInfo) -> impl PinInit<Self, Error> {
>> let dev = pdev.as_ref();
>>
>> let rtc_data = impl_pin_init!(SampleRtcData {
>> io: pdev.iomap_region_sized::<BAR0_SIZE>(0, c"my_rtc/bar0")?,
>> hw_variant: VendorVariant::StV1,
>> irq <- irq::Registration::new(...),
>> });
>>
>> let rtc = rtc::Device::new(dev, rtc_data)?;
>>
>> rtc::Registration::register(rtc)?;
>>
>> Ok(Self { rtc })
>> }
>>
>> Note that if any of the RTC callbacks would ever need to call irq.synchronize(),
>> irq.disable(), etc. the compiler would enforce correct ordering, as there would
>> not be any other possibility to put the irq::Registration other than into the
>> rtc_data that goes into rtc::Device::new().
>
> IIUC, the interrupt handler can only access the rtc_data because the
> parent's driver_data may not exist yet when it runs. Or am I missing
> something?
In the code above the IRQ handler can also not access rtc_data, as struct
SampleRtcData might not be fully initialized when it runs, i.e.
let rtc_data = impl_pin_init!(SampleRtcData {
io: pdev.iomap_region_sized::<BAR0_SIZE>(0, c"my_rtc/bar0")?,
hw_variant: VendorVariant::StV1,
irq <- irq::Registration::new(..., rtc_data),
});
would not compile in the first place.
irq::Registration, for this purpose, has its own private data on the handler
itself, see also [1]. In fact, the C code has the same concept with the dev_id
argument in request_threaded_irq() [2].
The difference is that the C compiler does not ensure that the IRQ handler
actually owns the data behind the dev_id pointer. I.e. the driver has to somehow
ensure that whatever is behind the dev_id pointer remains valid for the duration
the IRQ handler is registered.
In the Rust implementation the compiler does ensure that what is behind the
dev_id pointer remains valid for the duration of the lifetime of the
irq::Registration.
Having that said, I assume you wonder what we would pass into the
irq::Registration instead, if it is not rtc_data.
The answer is it depends; it depends on what's actually needed, what other
entities interact with the IRQ (e.g. some scheduled work, etc.) and maybe even
preference to some extend.
Here is one example:
let irq_data = impl_pin_init!(SampleIrqData {
io <- pdev.iomap_region_sized::<BAR0_SIZE>(0, c"my_rtc/bar0")?,
hw_variant: VendorVariant::StV1,
});
let rtc_data = impl_pin_init!(SampleRtcData {
irq <- irq::Registration::new(..., irq_data),
...,
});
let rtc = rtc::Device::new(dev, rtc_data)?;
This would compile as it ensures that irq_data (struct SampleIrqData) is fully
initialized before irq::Registration::new() is called.
At a first glance this might look like we need an additional allocation, one for
irq_data and one for rtc_data, but that is not the case. irq_data is an
initializer that is passed to another initializer, i.e. rtc_data is still an
initializer.
The actual (single) allocation happens in rtc::Device::new().
In terms of accessing it through the the rtc::Device in an RTC device callback,
we would likely use accessor methods to make it a bit more convinient, i.e.
fn read_time(
rtc: &rtc::Device<SampleRtcData>
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 => { ... },
}
}
As mentioned above there are a few other options to implement this, depending on
what's required, etc.
For instance, if the I/O bar is actually shared between multiple entities we
might want to initialize it within an Arc [3] (reference count it) for shared
ownership.
For the future we will also be able to support references within initializers to
other pinned fields, which make things a bit more convinient, so you could do
things like this:
let irq_data = impl_pin_init!(SampleIrqData {
io <- pdev.iomap_region_sized::<BAR0_SIZE>(0, c"my_rtc/bar0")?,
hw_variant: VendorVariant::StV1,
});
let rtc_data = impl_pin_init!(SampleRtcData {
irq <- irq::Registration::new(..., irq_data),
io: &irq.io,
...,
});
let rtc = rtc::Device::new(dev, rtc_data)?;
Note the additional `io: &irq.io,` in the rtc_data initializer. This would be
legal as we know that `irq` is pinned within `rtc_data`, hence it is valid to
hold a reference to one of its pinned fields.
I am not sure how far we are from having this supported, I assume Benno and Gary
can say more about this.
I hope this helps, and thanks for asking those questions!
[1] https://rust.docs.kernel.org/kernel/irq/struct.Registration.html
[2] https://elixir.bootlin.com/linux/v6.19.3/source/kernel/irq/manage.c#L2090
[3] https://rust.docs.kernel.org/kernel/sync/struct.Arc.html
^ permalink raw reply
* Re: [RFC PATCH v3 1/5] rtc: add device selector for rtc_class_ops callbacks
From: Rafael J. Wysocki @ 2026-02-25 13:33 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: <DGNJKZA00MNT.2C7NAQYG597MO@kernel.org>
On Tue, Feb 24, 2026 at 11:23 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> On Tue Feb 24, 2026 at 6:28 PM CET, Alexandre Belloni wrote:
> > On 24/02/2026 17:35:23+0100, Danilo Krummrich wrote:
> >> (I did not have any specific hardware in mind when sketching this up (e.g. an
> >> IRQ could also only be needed in bus device callbacks, e.g. for loading firmware
> >> etc.). But for RTC it obviously is common that it is relevant to the class
> >> device too.)
> >>
> >> So, I assume you mean because there could already be an ioctl before the IRQ has
> >> been successfully registered, and this ioctl may wait for an IRQ?
> >>
> >> In this case the irq::Registration should go into rtc_data instead to account
> >> for this dependency. Unfortunately, this is a semantic dependency that we can't
> >> always catch at compile time.
> >>
> >> The reason we sometimes can is because, if you would need access to the
> >> irq::Registration from ioctls (e.g. for calling synchronize(), enable(),
> >> disable() etc.) it would be caught, because you couldn't access it without it
> >> being in rtc_data in the first place, and being forced to have it in rtc_data
> >> guarantees that the ordering can't be wrong.
> >
> > No, once you register the rtc, the character device will appear in
> > userspace and may be opened, at this point, probe is not allowed to fail
> > anymore which you are allowing by trying to register the IRQ so late.
>
> This does not seem to correspond to my previous reply -- may I kindly ask you to
> read it again?
>
> Here's also some sketched up code for what I wrote above:
>
> fn probe(pdev: &pci::Device<Core>, info: &Self::IdInfo) -> impl PinInit<Self, Error> {
> let dev = pdev.as_ref();
>
> let rtc_data = impl_pin_init!(SampleRtcData {
> io: pdev.iomap_region_sized::<BAR0_SIZE>(0, c"my_rtc/bar0")?,
> hw_variant: VendorVariant::StV1,
> irq <- irq::Registration::new(...),
> });
>
> let rtc = rtc::Device::new(dev, rtc_data)?;
>
> rtc::Registration::register(rtc)?;
>
> Ok(Self { rtc })
> }
>
> Note that if any of the RTC callbacks would ever need to call irq.synchronize(),
> irq.disable(), etc. the compiler would enforce correct ordering, as there would
> not be any other possibility to put the irq::Registration other than into the
> rtc_data that goes into rtc::Device::new().
IIUC, the interrupt handler can only access the rtc_data because the
parent's driver_data may not exist yet when it runs. Or am I missing
something?
^ permalink raw reply
* Re: [PATCH 0/7] rtc: sun6i: Add support for Allwinner A733 SoC
From: Junhui Liu @ 2026-02-25 12:02 UTC (permalink / raw)
To: Jernej Škrabec, Michael Turquette, Stephen Boyd,
Chen-Yu Tsai, Samuel Holland, Alexandre Belloni, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Maxime Ripard, Junhui Liu
Cc: linux-clk, linux-arm-kernel, linux-sunxi, linux-kernel, linux-rtc,
devicetree
In-Reply-To: <5061953.GXAFRqVoOG@jernej-laptop>
Hi Jernej,
Thanks for your review.
On Sun Feb 22, 2026 at 6:41 PM CST, Jernej Škrabec wrote:
> Hi!
>
> Dne sreda, 21. januar 2026 ob 11:59:06 Srednjeevropski standardni čas je Junhui Liu napisal(a):
>> Add support for the Allwinner A733 RTC and its internal Clock Control
>> Unit (CCU). Reuse the rtc-sun6i rtc driver while introducing a new
>> SoC-specific RTC CCU driver to handle the hardware's evolved clock
>> structure.
>>
>> To facilitate this addition and improve driver modularity, transition
>> the binding between the RTC and its internal CCU from direct
>> cross-subsystem function calls to the auxiliary bus. Also extract shared
>> IOSC and 32kHz clock logic into a standalone ccu_rtc module for reuse
>> across newer SoC generations.
>>
>> The A733 implementation supports hardware detection of three external
>> crystal frequencies (19.2MHz, 24MHz and 26MHz), which is represented in
>> the driver via read-only mux operations. Implement logic to derive a
>> normalized 32kHz reference from these DCXO sources using fixed
>> pre-dividers. Additionally, provide several new DCXO gate clocks for
>> peripherals, including SerDes, HDMI, and UFS.
>
> This work looks nice, but I have some questions/comments:
> - you're missing RTC SPI clock, which is needed for RTC, at least according
> to vendor 5.15 DT. Could it be that this bit set by vendor U-Boot so you
> missed it during testing? Manual says that it's disabled by default.
You're right! I tried disabling the RTC SPI clock in U-Boot and found
that the output of UART became garbled during booting kernel. I will add
it in the next version.
> - Vendor DT has strange RTC CCU phandles for UFS and HDMI. In first case
> uses RTC wakeup and in second DCXO, which doesn't make any sense. Did you
> do any experimentation with these clocks? It wouldn't be the first time
> that either code or manual contained some kind of error.
Regarding UFS, I am still working on getting UFS functional on the
mainline kernel. I will investigate the actual relationship between the
RTC wakeup clock and UFS during this process.
As for HDMI, I believe it actually requires the hosc_hdmi_clk
(DCXO_HDMI_GATING in the manual) provided by the RTC module.
>
> Btw, switch last two patches. With current order during bisection you would
> get a complaint that A733 RTC CCU driver is not present.
Okay, I will do it.
>
> Best regards,
> Jernej
--
Best regards,
Junhui Liu
^ permalink raw reply
* Re: [PATCH v3 04/13] dt-bindings: power: supply: document Samsung S2M series PMIC charger device
From: Krzysztof Kozlowski @ 2026-02-25 10:44 UTC (permalink / raw)
To: Kaustabh Chakraborty
Cc: Lee Jones, Pavel Machek, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, MyungJoo Ham, Chanwoo Choi, Sebastian Reichel,
André Draszik, Alexandre Belloni, Jonathan Corbet,
Shuah Khan, Nam Tran, linux-leds, devicetree, linux-kernel,
linux-pm, linux-samsung-soc, linux-rtc, linux-doc
In-Reply-To: <20260225-s2mu005-pmic-v3-4-b4afee947603@disroot.org>
On Wed, Feb 25, 2026 at 12:45:06AM +0530, Kaustabh Chakraborty wrote:
> +
> + This is a part of device tree bindings for S2M and S5M family of Power
> + Management IC (PMIC).
> +
> + See also Documentation/devicetree/bindings/mfd/samsung,s2mps11.yaml for
> + additional information and example.
> +
> +allOf:
> + - $ref: power-supply.yaml#
> +
> +properties:
> + compatible:
> + enum:
> + - samsung,s2mu005-charger
Review from v1 still applies. I think you ignored several reviews, so I
will mark entire patchset as changes requested.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [RFC PATCH v3 1/5] rtc: add device selector for rtc_class_ops callbacks
From: Gary Guo @ 2026-02-25 3:19 UTC (permalink / raw)
To: Alexandre Belloni
Cc: Danilo Krummrich, Rafael J. Wysocki, Alvin Sun, Miguel Ojeda,
Boqun Feng, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Alice Ryhl, Trevor Gross, linux-rtc, rust-for-linux,
Greg Kroah-Hartman
In-Reply-To: <20260224224449f141e366@mail.local>
On 2026-02-24 22:44, Alexandre Belloni wrote:
> On 24/02/2026 23:23:29+0100, Danilo Krummrich wrote:
>> On Tue Feb 24, 2026 at 6:28 PM CET, Alexandre Belloni wrote:
>> > On 24/02/2026 17:35:23+0100, Danilo Krummrich wrote:
>> >> (I did not have any specific hardware in mind when sketching this up (e.g. an
>> >> IRQ could also only be needed in bus device callbacks, e.g. for loading firmware
>> >> etc.). But for RTC it obviously is common that it is relevant to the class
>> >> device too.)
>> >>
>> >> So, I assume you mean because there could already be an ioctl before the IRQ has
>> >> been successfully registered, and this ioctl may wait for an IRQ?
>> >>
>> >> In this case the irq::Registration should go into rtc_data instead to account
>> >> for this dependency. Unfortunately, this is a semantic dependency that we can't
>> >> always catch at compile time.
>> >>
>> >> The reason we sometimes can is because, if you would need access to the
>> >> irq::Registration from ioctls (e.g. for calling synchronize(), enable(),
>> >> disable() etc.) it would be caught, because you couldn't access it without it
>> >> being in rtc_data in the first place, and being forced to have it in rtc_data
>> >> guarantees that the ordering can't be wrong.
>> >
>> > No, once you register the rtc, the character device will appear in
>> > userspace and may be opened, at this point, probe is not allowed to fail
>> > anymore which you are allowing by trying to register the IRQ so late.
>>
>> This does not seem to correspond to my previous reply -- may I kindly ask you to
>> read it again?
>>
>> Here's also some sketched up code for what I wrote above:
>>
>> fn probe(pdev: &pci::Device<Core>, info: &Self::IdInfo) -> impl PinInit<Self, Error> {
>> let dev = pdev.as_ref();
>>
>> let rtc_data = impl_pin_init!(SampleRtcData {
>> io: pdev.iomap_region_sized::<BAR0_SIZE>(0, c"my_rtc/bar0")?,
>> hw_variant: VendorVariant::StV1,
>> irq <- irq::Registration::new(...),
>> });
>>
>> let rtc = rtc::Device::new(dev, rtc_data)?;
>>
>> rtc::Registration::register(rtc)?;
>>
>> Ok(Self { rtc })
>> }
>>
>> Note that if any of the RTC callbacks would ever need to call irq.synchronize(),
>> irq.disable(), etc. the compiler would enforce correct ordering, as there would
>> not be any other possibility to put the irq::Registration other than into the
>> rtc_data that goes into rtc::Device::new().
>
> Right but again, the issue is not about the irq or resource allocation
> ordering, it is about probe failing after the character device creation.
>
>>
>> Besides that, you above mentioned "probe is not allowed to fail anymore" after
>> the RTC device is registered and the corresponding character device becomes
>> visible to userspace.
>>
>> While there most likely isn't any good reason for probe() to fail afterwards for
>> RTC devices, it is not the case that this isn't allowed. We generally can unwind
>> from a class device registration. In fact, this is not different to remove()
>> being called (immediately).
>
> It is actually different, this was the race back then:
>
> CPU0: CPU1:
> sys_load_module()
> do_init_module()
> do_one_initcall()
> cmos_do_probe()
> rtc_device_register()
> __register_chrdev()
> cdev->owner = struct module*
> open("/dev/rtc0")
> rtc_device_unregister()
> module_put()
> free_module()
> module_free(mod->module_core)
> /* struct module *module is now
> freed */
> chrdev_open()
> spin_lock(cdev_lock)
> cdev_get()
> try_module_get()
> module_is_live()
> /* dereferences already
> freed struct module* */
>
>
> I don't think it has been solved since then.
I think it is not realistic to require module init to always complete once a char
dev is registered. What if you're trying to register multiple char devs?
I think this requires that either `rtc_device_unregister` to wait until all
opened fds on the char dev to be closed before returning, or a reference
count to the module is kept. (Or having the file ops being swapped out so
further operation on the fd doesn't hit the module anymore).
Anyhow, this looks exactly what all the driver-core revocable discussion is about.
Best,
Gary
^ permalink raw reply
* Re: [RFC PATCH v3 1/5] rtc: add device selector for rtc_class_ops callbacks
From: Alexandre Belloni @ 2026-02-24 22:44 UTC (permalink / raw)
To: Danilo Krummrich
Cc: 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: <DGNJKZA00MNT.2C7NAQYG597MO@kernel.org>
On 24/02/2026 23:23:29+0100, Danilo Krummrich wrote:
> On Tue Feb 24, 2026 at 6:28 PM CET, Alexandre Belloni wrote:
> > On 24/02/2026 17:35:23+0100, Danilo Krummrich wrote:
> >> (I did not have any specific hardware in mind when sketching this up (e.g. an
> >> IRQ could also only be needed in bus device callbacks, e.g. for loading firmware
> >> etc.). But for RTC it obviously is common that it is relevant to the class
> >> device too.)
> >>
> >> So, I assume you mean because there could already be an ioctl before the IRQ has
> >> been successfully registered, and this ioctl may wait for an IRQ?
> >>
> >> In this case the irq::Registration should go into rtc_data instead to account
> >> for this dependency. Unfortunately, this is a semantic dependency that we can't
> >> always catch at compile time.
> >>
> >> The reason we sometimes can is because, if you would need access to the
> >> irq::Registration from ioctls (e.g. for calling synchronize(), enable(),
> >> disable() etc.) it would be caught, because you couldn't access it without it
> >> being in rtc_data in the first place, and being forced to have it in rtc_data
> >> guarantees that the ordering can't be wrong.
> >
> > No, once you register the rtc, the character device will appear in
> > userspace and may be opened, at this point, probe is not allowed to fail
> > anymore which you are allowing by trying to register the IRQ so late.
>
> This does not seem to correspond to my previous reply -- may I kindly ask you to
> read it again?
>
> Here's also some sketched up code for what I wrote above:
>
> fn probe(pdev: &pci::Device<Core>, info: &Self::IdInfo) -> impl PinInit<Self, Error> {
> let dev = pdev.as_ref();
>
> let rtc_data = impl_pin_init!(SampleRtcData {
> io: pdev.iomap_region_sized::<BAR0_SIZE>(0, c"my_rtc/bar0")?,
> hw_variant: VendorVariant::StV1,
> irq <- irq::Registration::new(...),
> });
>
> let rtc = rtc::Device::new(dev, rtc_data)?;
>
> rtc::Registration::register(rtc)?;
>
> Ok(Self { rtc })
> }
>
> Note that if any of the RTC callbacks would ever need to call irq.synchronize(),
> irq.disable(), etc. the compiler would enforce correct ordering, as there would
> not be any other possibility to put the irq::Registration other than into the
> rtc_data that goes into rtc::Device::new().
Right but again, the issue is not about the irq or resource allocation
ordering, it is about probe failing after the character device creation.
>
> Besides that, you above mentioned "probe is not allowed to fail anymore" after
> the RTC device is registered and the corresponding character device becomes
> visible to userspace.
>
> While there most likely isn't any good reason for probe() to fail afterwards for
> RTC devices, it is not the case that this isn't allowed. We generally can unwind
> from a class device registration. In fact, this is not different to remove()
> being called (immediately).
It is actually different, this was the race back then:
CPU0: CPU1:
sys_load_module()
do_init_module()
do_one_initcall()
cmos_do_probe()
rtc_device_register()
__register_chrdev()
cdev->owner = struct module*
open("/dev/rtc0")
rtc_device_unregister()
module_put()
free_module()
module_free(mod->module_core)
/* struct module *module is now
freed */
chrdev_open()
spin_lock(cdev_lock)
cdev_get()
try_module_get()
module_is_live()
/* dereferences already
freed struct module* */
I don't think it has been solved since then.
>
> Imagine a case where a driver registers multiple class devices, or a class
> device and an auxiliary device, etc.
>
> (But I assume your point was more that for an RTC device specifically this would
> be odd or uncommon.)
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
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