[PATCH 0/4] Add tooling to disable debugfs on OMAP based systems

[PATCH 0/4] Add tooling to disable debugfs on OMAP based systems

[Richard Weinberger]

It came to my attention that commands such as `grep -r / -e ...` can cause
crashes on an AM572x based system.
An investigation found that reading from various files in /sys/kernel/debug/regmap
causes imprecise async data aborts.

One of these register maps is the CTRL_MODULE_CORE register map at 0x4A002000.
It contains various registers marked as reserved, but the manual indicates
that read access is still allowed.
On said system, reading from most registers seems to work, but for some
an async data abort happens. So it's not entirely clear what registers are safe
and which are not.

So, add tooling to allow disabling debugfs access to such dangerous registers.
Splitting the register map definitions in the device tree seemed less practical to
me since it would unnecessarily make the device trees more complicated.

Richard Weinberger (4):
  dt-bindings: Document new common property: has-inaccessible-regs
  regmap: Allow disabling debugfs via regmap_config
  syscon: Wire up has-inaccessible-regs
  arm: dts: omap: Mark various register maps as dangerous

 .../devicetree/bindings/common-properties.txt        | 12 ++++++++++++
 arch/arm/boot/dts/ti/omap/dra7-l4.dtsi               |  4 ++++
 arch/arm/boot/dts/ti/omap/dra7.dtsi                  |  1 +
 arch/arm/boot/dts/ti/omap/dra74x.dtsi                |  1 +
 drivers/base/regmap/regmap.c                         |  2 ++
 drivers/mfd/syscon.c                                 | 10 ++++++++++
 include/linux/regmap.h                               |  3 +++
 7 files changed, 33 insertions(+)

-- 
2.51.0

[PATCH 1/4] dt-bindings: Document new common property: has-inaccessible-regs

[Richard Weinberger]

This property is used to denote that a certain register map contains
registers that are inaccessible under conditions only a device driver
can know.
The purpose of this property is to disable register access through debug
facilities outside of the device driver.

Signed-off-by: Richard Weinberger <richard@nod.at>
---
 .../devicetree/bindings/common-properties.txt        | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/Documentation/devicetree/bindings/common-properties.txt b/Documentation/devicetree/bindings/common-properties.txt
index 98a28130e100f..edf6d0b8cf1b1 100644
--- a/Documentation/devicetree/bindings/common-properties.txt
+++ b/Documentation/devicetree/bindings/common-properties.txt
@@ -83,3 +83,15 @@ gpio@0 {
 	      #gpio-cells = <2>;
 	      #daisy-chained-devices = <3>;
 };
+
+Inaccessible registers
+----------------------
+
+If a register map as described by the 'reg' property contains registers
+that cannot be accessed for various reasons and splitting the register
+definition is not possible, use this property to denote that uncontrolled
+access outside of a device driver should be disabled.
+On Linux, for example, this disables regmap debugfs access
+
+Optional properties:
+- has-inaccessible-regs: Boolean
-- 
2.51.0

[PATCH 2/4] regmap: Allow disabling debugfs via regmap_config

[Richard Weinberger]

Regmap already disables register access via debugfs as soon as a register
map is used without taking locks.
Go a step further and allow disabling debugfs via regmap_config such that
device drivers can decide on their own whether uncontrolled register access
via debugfs is harmful.

Signed-off-by: Richard Weinberger <richard@nod.at>
---
 drivers/base/regmap/regmap.c | 2 ++
 include/linux/regmap.h       | 3 +++
 2 files changed, 5 insertions(+)

diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index ce9be3989a218..38d7a03aa7637 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -697,6 +697,8 @@ struct regmap *__regmap_init(struct device *dev,
 	if (ret)
 		goto err_map;
 
+	map->debugfs_disable = config->debugfs_disable;
+
 	ret = -EINVAL; /* Later error paths rely on this */
 
 	if (config->disable_locking) {
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index 55343795644b9..646501a055eba 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -395,6 +395,7 @@ typedef void (*regmap_unlock)(void *);
  *
  * @ranges: Array of configuration entries for virtual address ranges.
  * @num_ranges: Number of range configuration entries.
+ * @debugfs_disable: Disable debugfs access to this register.
  */
 struct regmap_config {
 	const char *name;
@@ -467,6 +468,8 @@ struct regmap_config {
 
 	const struct regmap_range_cfg *ranges;
 	unsigned int num_ranges;
+
+	bool debugfs_disable;
 };
 
 /**
-- 
2.51.0

[PATCH 3/4] syscon: Wire up has-inaccessible-regs

[Richard Weinberger]

Evaluate the has-inaccessible-regs device tree property to disable
debugfs access if a register map contains dangerous/harmful registers.

Signed-off-by: Richard Weinberger <richard@nod.at>
---
 drivers/mfd/syscon.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/mfd/syscon.c b/drivers/mfd/syscon.c
index ae71a2710bed8..73fff0df3f42f 100644
--- a/drivers/mfd/syscon.c
+++ b/drivers/mfd/syscon.c
@@ -70,6 +70,16 @@ static struct syscon *of_syscon_register(struct device_node *np, bool check_res)
 	else if (of_property_read_bool(np, "native-endian"))
 		syscon_config.val_format_endian = REGMAP_ENDIAN_NATIVE;
 
+
+	/*
+	 * Disable debugfs access if a register map has various inaccessible
+	 * registers.
+	 * In such a case the device driver has to know exactly how and when
+	 * access is allowed but general access via userspace can cause harm.
+	 */
+	if (of_property_read_bool(np, "has-inaccessible-regs"))
+		syscon_config.debugfs_disable = true;
+
 	/*
 	 * search for reg-io-width property in DT. If it is not provided,
 	 * default to 4 bytes. regmap_init_mmio will return an error if values
-- 
2.51.0

[PATCH 4/4] arm: dts: omap: Mark various register maps as dangerous

[Richard Weinberger]

These register maps contain registers where reads can cause an
imprecise async data abort.
Mark these register maps with has-inaccessible-regs to make sure
they cannot be accessed via debugfs and only by device drivers that
hopefully know exactly how and when access is allowed.

Signed-off-by: Richard Weinberger <richard@nod.at>
---
 arch/arm/boot/dts/ti/omap/dra7-l4.dtsi | 4 ++++
 arch/arm/boot/dts/ti/omap/dra7.dtsi    | 1 +
 arch/arm/boot/dts/ti/omap/dra74x.dtsi  | 1 +
 3 files changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/ti/omap/dra7-l4.dtsi b/arch/arm/boot/dts/ti/omap/dra7-l4.dtsi
index c9282f57ffa5e..bd037e199946e 100644
--- a/arch/arm/boot/dts/ti/omap/dra7-l4.dtsi
+++ b/arch/arm/boot/dts/ti/omap/dra7-l4.dtsi
@@ -69,6 +69,7 @@ scm_conf: scm_conf@0 {
 					#address-cells = <1>;
 					#size-cells = <1>;
 					ranges = <0 0x0 0x1400>;
+					has-inaccessible-regs;
 
 					pbias_regulator: pbias_regulator@e00 {
 						compatible = "ti,pbias-dra7", "ti,pbias-omap";
@@ -110,11 +111,13 @@ scm_conf1: scm_conf@1c04 {
 					compatible = "syscon";
 					reg = <0x1c04 0x0020>;
 					#syscon-cells = <2>;
+					has-inaccessible-regs;
 				};
 
 				scm_conf_pcie: scm_conf@1c24 {
 					compatible = "syscon";
 					reg = <0x1c24 0x0024>;
+					has-inaccessible-regs;
 				};
 
 				sdma_xbar: dma-router@b78 {
@@ -4306,6 +4309,7 @@ target-module@c000 {			/* 0x4ae0c000, ap 17 50.0 */
 			scm_wkup: scm_conf@0 {
 				compatible = "syscon";
 				reg = <0 0x1000>;
+				has-inaccessible-regs;
 			};
 		};
 	};
diff --git a/arch/arm/boot/dts/ti/omap/dra7.dtsi b/arch/arm/boot/dts/ti/omap/dra7.dtsi
index 711ce4c31bb1f..1b1f31608d37e 100644
--- a/arch/arm/boot/dts/ti/omap/dra7.dtsi
+++ b/arch/arm/boot/dts/ti/omap/dra7.dtsi
@@ -359,6 +359,7 @@ bandgap: bandgap@4a0021e0 {
 		dsp1_system: dsp_system@40d00000 {
 			compatible = "syscon";
 			reg = <0x40d00000 0x100>;
+			has-inaccessible-regs;
 		};
 
 		dra7_iodelay_core: padconf@4844a000 {
diff --git a/arch/arm/boot/dts/ti/omap/dra74x.dtsi b/arch/arm/boot/dts/ti/omap/dra74x.dtsi
index cfb39dde49300..d814c31576797 100644
--- a/arch/arm/boot/dts/ti/omap/dra74x.dtsi
+++ b/arch/arm/boot/dts/ti/omap/dra74x.dtsi
@@ -47,6 +47,7 @@ ocp {
 		dsp2_system: dsp_system@41500000 {
 			compatible = "syscon";
 			reg = <0x41500000 0x100>;
+			has-inaccessible-regs;
 		};
 
 
-- 
2.51.0

Re: [PATCH 0/4] Add tooling to disable debugfs on OMAP based systems

[Andreas Kemnade]

On Sat, 29 Nov 2025 15:20:38 +0100
Richard Weinberger <richard@nod.at> wrote:

> It came to my attention that commands such as `grep -r / -e ...` can cause
> crashes on an AM572x based system.
> An investigation found that reading from various files in /sys/kernel/debug/regmap
> causes imprecise async data aborts.
> 
> One of these register maps is the CTRL_MODULE_CORE register map at 0x4A002000.
> It contains various registers marked as reserved, but the manual indicates
> that read access is still allowed.
> On said system, reading from most registers seems to work, but for some
> an async data abort happens. So it's not entirely clear what registers are safe
> and which are not.
> 
it is usually not about individual registers, but about accessing 
unpowered devices/modules,

so it is probably more the logic like:

if (pm_runtime_is_suspended(regmap->device))
	-EACCESS;

Try to play around with on >power/control in sysfs.

> So, add tooling to allow disabling debugfs access to such dangerous registers.
> Splitting the register map definitions in the device tree seemed less practical to
> me since it would unnecessarily make the device trees more complicated.
> 
So is it really a description of the hardware? Maybe there are some special
cases, too.

Regards,
Andreas

Re: [PATCH 0/4] Add tooling to disable debugfs on OMAP based systems

[Richard Weinberger]

----- Ursprüngliche Mail -----
> Von: "Andreas Kemnade" <andreas@kemnade.info>
> it is usually not about individual registers, but about accessing
> unpowered devices/modules,
> 
> so it is probably more the logic like:
> 
> if (pm_runtime_is_suspended(regmap->device))
>	-EACCESS;
> 
> Try to play around with on >power/control in sysfs.

Well the that regmap is owned by the syscon mfd.
And in case of CTRL_MODULE_CORE only accessing some reserved registers causes
an abort.

For registers like dsp_system@40d00000 it's maybe indeed due to an unpowered module.
I'll double check that.
 
>> So, add tooling to allow disabling debugfs access to such dangerous registers.
>> Splitting the register map definitions in the device tree seemed less practical
>> to
>> me since it would unnecessarily make the device trees more complicated.
>> 
> So is it really a description of the hardware? Maybe there are some special
> cases, too.

IMHO, "There are problematic registers, don't blindly mess with them" is a description of the hardware.

Thanks,
//richard

Re: [PATCH 1/4] dt-bindings: Document new common property: has-inaccessible-regs

[Krzysztof Kozlowski]

On 29/11/2025 15:20, Richard Weinberger wrote:
> This property is used to denote that a certain register map contains
> registers that are inaccessible under conditions only a device driver
> can know.

So device driver controls fully their exposure via sysfs.

Binding cannot help here at all.

> The purpose of this property is to disable register access through debug
> facilities outside of the device driver.

You described OS policy which is not suitable for bindings at all. Plus
commit msg really mixes up two separate points - buggy driver which
fails to properly set regmap (or other facility) with some DT-based
restrictions.



Best regards,
Krzysztof

Re: [PATCH 3/4] syscon: Wire up has-inaccessible-regs

[Krzysztof Kozlowski]

On 29/11/2025 15:20, Richard Weinberger wrote:
> Evaluate the has-inaccessible-regs device tree property to disable
> debugfs access if a register map contains dangerous/harmful registers.
> 
> Signed-off-by: Richard Weinberger <richard@nod.at>
> ---
>  drivers/mfd/syscon.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/mfd/syscon.c b/drivers/mfd/syscon.c
> index ae71a2710bed8..73fff0df3f42f 100644
> --- a/drivers/mfd/syscon.c
> +++ b/drivers/mfd/syscon.c
> @@ -70,6 +70,16 @@ static struct syscon *of_syscon_register(struct device_node *np, bool check_res)
>  	else if (of_property_read_bool(np, "native-endian"))
>  		syscon_config.val_format_endian = REGMAP_ENDIAN_NATIVE;
>  
> +
> +	/*
> +	 * Disable debugfs access if a register map has various inaccessible
> +	 * registers.
> +	 * In such a case the device driver has to know exactly how and when
> +	 * access is allowed but general access via userspace can cause harm.
> +	 */
> +	if (of_property_read_bool(np, "has-inaccessible-regs"))
> +		syscon_config.debugfs_disable = true;

So you mark ENTIRE device like that? Then you don't need the property.
Your compatible already tells you that (IOW, this is fully deducible
from the compatible).

Best regards,
Krzysztof

Re: [PATCH 4/4] arm: dts: omap: Mark various register maps as dangerous

[Krzysztof Kozlowski]

On 29/11/2025 15:20, Richard Weinberger wrote:
> index 711ce4c31bb1f..1b1f31608d37e 100644
> --- a/arch/arm/boot/dts/ti/omap/dra7.dtsi
> +++ b/arch/arm/boot/dts/ti/omap/dra7.dtsi
> @@ -359,6 +359,7 @@ bandgap: bandgap@4a0021e0 {
>  		dsp1_system: dsp_system@40d00000 {
>  			compatible = "syscon";

Oh, no no, sorry, but buggy/incomplete/legacy DT is not an excuse for
new properties. You cannot have such compatible alone in the first place.

>  			reg = <0x40d00000 0x100>;
> +			has-inaccessible-regs;
>  		};




Best regards,
Krzysztof

Re: [PATCH 1/4] dt-bindings: Document new common property: has-inaccessible-regs

[Richard Weinberger]

----- Ursprüngliche Mail -----
> Von: "Krzysztof Kozlowski" <krzk@kernel.org>
> On 29/11/2025 15:20, Richard Weinberger wrote:
>> This property is used to denote that a certain register map contains
>> registers that are inaccessible under conditions only a device driver
>> can know.
> 
> So device driver controls fully their exposure via sysfs.
> 
> Binding cannot help here at all.

The driver does not expose them via sysfs, it's the regmap framework via debugfs.
 
>> The purpose of this property is to disable register access through debug
>> facilities outside of the device driver.
> 
> You described OS policy which is not suitable for bindings at all. Plus
> commit msg really mixes up two separate points - buggy driver which
> fails to properly set regmap (or other facility) with some DT-based
> restrictions.

I kind of expected this answer. ;-)

Currently arch/arm/boot/dts/ti/omap/dra7-l4.dtsi binds CTRL_MODULE_CORE to the syscon mfd driver
and various child nodes bind to subranges.
e.g.
scm_conf: scm_conf@0 {
        compatible = "syscon", "simple-bus";
        reg = <0x0 0x1400>;
        #address-cells = <1>;
        #size-cells = <1>;
        ranges = <0 0x0 0x1400>;
        has-inaccessible-regs;

        pbias_regulator: pbias_regulator@e00 {
                compatible = "ti,pbias-dra7", "ti,pbias-omap";
                reg = <0xe00 0x4>;
                syscon = <&scm_conf>;
                pbias_mmc_reg: pbias_mmc_omap5 {
                        regulator-name = "pbias_mmc_omap5";
                        regulator-min-microvolt = <1800000>;
                        regulator-max-microvolt = <3300000>;
                };      
        };      

        phy_gmii_sel: phy-gmii-sel@554 {
                compatible = "ti,dra7xx-phy-gmii-sel";
                reg = <0x554 0x4>;
                #phy-cells = <1>;
        };      

        scm_conf_clocks: clocks {
                #address-cells = <1>;
                #size-cells = <0>;
        };      
};

So, drivers like ti,pbias-dra7 or ti,dra7xx-phy-gmii-sel touch only registers
they know about and this works well.
But syscon manages the whole register map via regmap, and regmap exposes it all
via debugfs.

What solution do you propose?
Splitting reg = <0x0 0x1400> into many tiny fractions and not using an mfd anymore?

Thanks,
//richard

Re: [PATCH 4/4] arm: dts: omap: Mark various register maps as dangerous

[Richard Weinberger]

----- Ursprüngliche Mail -----
> Von: "Krzysztof Kozlowski" <krzk@kernel.org>
> An: "richard" <richard@nod.at>, "linux-kernel" <linux-kernel@vger.kernel.org>
> CC: "linux-omap" <linux-omap@vger.kernel.org>, "devicetree" <devicetree@vger.kernel.org>, "Arnd Bergmann"
> <arnd@arndb.de>, "Lee Jones" <lee@kernel.org>, "dakr" <dakr@kernel.org>, "Rafael J. Wysocki" <rafael@kernel.org>, "Greg
> Kroah-Hartman" <gregkh@linuxfoundation.org>, "Mark Brown" <broonie@kernel.org>, "tony" <tony@atomide.com>, "rogerq"
> <rogerq@kernel.org>, "khilman" <khilman@baylibre.com>, "Andreas Kemnade" <andreas@kemnade.info>, "aaro koskinen"
> <aaro.koskinen@iki.fi>, "Conor Dooley" <conor+dt@kernel.org>, "Krzysztof Kozlowski" <krzk+dt@kernel.org>, "robh"
> <robh@kernel.org>
> Gesendet: Samstag, 29. November 2025 16:26:19
> Betreff: Re: [PATCH 4/4] arm: dts: omap: Mark various register maps as dangerous

> On 29/11/2025 15:20, Richard Weinberger wrote:
>> index 711ce4c31bb1f..1b1f31608d37e 100644
>> --- a/arch/arm/boot/dts/ti/omap/dra7.dtsi
>> +++ b/arch/arm/boot/dts/ti/omap/dra7.dtsi
>> @@ -359,6 +359,7 @@ bandgap: bandgap@4a0021e0 {
>>  		dsp1_system: dsp_system@40d00000 {
>>  			compatible = "syscon";
> 
> Oh, no no, sorry, but buggy/incomplete/legacy DT is not an excuse for
> new properties. You cannot have such compatible alone in the first place.

Okay, I didn't know that the OMAP DT is in that bad shape.
Is somebody working on a solution?

Thanks,
//richard

Re: [PATCH 1/4] dt-bindings: Document new common property: has-inaccessible-regs

[Krzysztof Kozlowski]

On 29/11/2025 16:33, Richard Weinberger wrote:
> ----- Ursprüngliche Mail -----
>> Von: "Krzysztof Kozlowski" <krzk@kernel.org>
>> On 29/11/2025 15:20, Richard Weinberger wrote:
>>> This property is used to denote that a certain register map contains
>>> registers that are inaccessible under conditions only a device driver
>>> can know.
>>
>> So device driver controls fully their exposure via sysfs.
>>
>> Binding cannot help here at all.
> 
> The driver does not expose them via sysfs, it's the regmap framework via debugfs.

Driver always tells the regmap which registers are valid. This is not a
new problem, we had it in several devices and fixed drivers.

>  
>>> The purpose of this property is to disable register access through debug
>>> facilities outside of the device driver.
>>
>> You described OS policy which is not suitable for bindings at all. Plus
>> commit msg really mixes up two separate points - buggy driver which
>> fails to properly set regmap (or other facility) with some DT-based
>> restrictions.
> 
> I kind of expected this answer. ;-)
> 
> Currently arch/arm/boot/dts/ti/omap/dra7-l4.dtsi binds CTRL_MODULE_CORE to the syscon mfd driver
> and various child nodes bind to subranges.
> e.g.
> scm_conf: scm_conf@0 {
>         compatible = "syscon", "simple-bus";
>         reg = <0x0 0x1400>;
>         #address-cells = <1>;
>         #size-cells = <1>;
>         ranges = <0 0x0 0x1400>;
>         has-inaccessible-regs;
> 
>         pbias_regulator: pbias_regulator@e00 {
>                 compatible = "ti,pbias-dra7", "ti,pbias-omap";
>                 reg = <0xe00 0x4>;
>                 syscon = <&scm_conf>;
>                 pbias_mmc_reg: pbias_mmc_omap5 {
>                         regulator-name = "pbias_mmc_omap5";
>                         regulator-min-microvolt = <1800000>;
>                         regulator-max-microvolt = <3300000>;
>                 };      
>         };      
> 
>         phy_gmii_sel: phy-gmii-sel@554 {
>                 compatible = "ti,dra7xx-phy-gmii-sel";
>                 reg = <0x554 0x4>;
>                 #phy-cells = <1>;
>         };      
> 
>         scm_conf_clocks: clocks {
>                 #address-cells = <1>;
>                 #size-cells = <0>;
>         };      
> };
> 
> So, drivers like ti,pbias-dra7 or ti,dra7xx-phy-gmii-sel touch only registers
> they know about and this works well.
> But syscon manages the whole register map via regmap, and regmap exposes it all
> via debugfs.
> 
> What solution do you propose?
> Splitting reg = <0x0 0x1400> into many tiny fractions and not using an mfd anymore?

Fix the driver. In your case, the syscon driver.


Best regards,
Krzysztof

Re: [PATCH 1/4] dt-bindings: Document new common property: has-inaccessible-regs

[Krzysztof Kozlowski]

On 29/11/2025 16:44, Krzysztof Kozlowski wrote:
>>         scm_conf_clocks: clocks {
>>                 #address-cells = <1>;
>>                 #size-cells = <0>;
>>         };      
>> };
>>
>> So, drivers like ti,pbias-dra7 or ti,dra7xx-phy-gmii-sel touch only registers
>> they know about and this works well.
>> But syscon manages the whole register map via regmap, and regmap exposes it all
>> via debugfs.
>>
>> What solution do you propose?
>> Splitting reg = <0x0 0x1400> into many tiny fractions and not using an mfd anymore?
> 
> Fix the driver. In your case, the syscon driver.

BTW, the state of existing TI DRA code is so poor that you don't have
many choices... or rather every choice has drawbacks. If this was proper
DTS, then I would say - define register map, used by regmap, for your
compatible either in syscon driver or dedicated driver (thus new driver
will be the syscon provider for you, just like Google GS101 syscon is
special).

Or maybe this is not syscon at all!

Remember that syscon is a collection of miscellaneous system controller
registers. You should not use syscon for other things, like devices with
incomplete hardware description.

Best regards,
Krzysztof

Re: [PATCH 4/4] arm: dts: omap: Mark various register maps as dangerous

[Krzysztof Kozlowski]

On 29/11/2025 16:35, Richard Weinberger wrote:
> ----- Ursprüngliche Mail -----
>> Von: "Krzysztof Kozlowski" <krzk@kernel.org>
>> An: "richard" <richard@nod.at>, "linux-kernel" <linux-kernel@vger.kernel.org>
>> CC: "linux-omap" <linux-omap@vger.kernel.org>, "devicetree" <devicetree@vger.kernel.org>, "Arnd Bergmann"
>> <arnd@arndb.de>, "Lee Jones" <lee@kernel.org>, "dakr" <dakr@kernel.org>, "Rafael J. Wysocki" <rafael@kernel.org>, "Greg
>> Kroah-Hartman" <gregkh@linuxfoundation.org>, "Mark Brown" <broonie@kernel.org>, "tony" <tony@atomide.com>, "rogerq"
>> <rogerq@kernel.org>, "khilman" <khilman@baylibre.com>, "Andreas Kemnade" <andreas@kemnade.info>, "aaro koskinen"
>> <aaro.koskinen@iki.fi>, "Conor Dooley" <conor+dt@kernel.org>, "Krzysztof Kozlowski" <krzk+dt@kernel.org>, "robh"
>> <robh@kernel.org>
>> Gesendet: Samstag, 29. November 2025 16:26:19
>> Betreff: Re: [PATCH 4/4] arm: dts: omap: Mark various register maps as dangerous
> 
>> On 29/11/2025 15:20, Richard Weinberger wrote:
>>> index 711ce4c31bb1f..1b1f31608d37e 100644
>>> --- a/arch/arm/boot/dts/ti/omap/dra7.dtsi
>>> +++ b/arch/arm/boot/dts/ti/omap/dra7.dtsi
>>> @@ -359,6 +359,7 @@ bandgap: bandgap@4a0021e0 {
>>>  		dsp1_system: dsp_system@40d00000 {
>>>  			compatible = "syscon";
>>
>> Oh, no no, sorry, but buggy/incomplete/legacy DT is not an excuse for
>> new properties. You cannot have such compatible alone in the first place.
> 
> Okay, I didn't know that the OMAP DT is in that bad shape.

Heh, let me give you a piece of a spoiler of my OSS Japan talk in a
week: TI (entire TI, not individual sub-platforms) for arm32 is 0%
compliant, meaning EVERY board has issues, and TI has THE HIGHEST total
number of warnings of all active mainline arm32 platforms.

TI ARM32 status is absolutely terrible... TI ARM64 is on quite different
side. :)

More of hall-of-shame on 9th of December...

Best regards,
Krzysztof

Re: [PATCH 1/4] dt-bindings: Document new common property: has-inaccessible-regs

[Richard Weinberger]

----- Ursprüngliche Mail -----
> Von: "Krzysztof Kozlowski" <krzk@kernel.org>
>> So, drivers like ti,pbias-dra7 or ti,dra7xx-phy-gmii-sel touch only registers
>> they know about and this works well.
>> But syscon manages the whole register map via regmap, and regmap exposes it all
>> via debugfs.
>> 
>> What solution do you propose?
>> Splitting reg = <0x0 0x1400> into many tiny fractions and not using an mfd
>> anymore?
> 
> Fix the driver. In your case, the syscon driver.

Please help me to understand what the desired behavior of the driver is.

Currently syscon creates one regmap for everything and passes this regmap
to the individual syscon users.
These users have to know what offset within the regmap is their playground.
If I understand correctly, it would be better if every syscon user would register their own regmap?

Lee, Arnd, what do you think?

Thanks,
//richard

Re: [PATCH 4/4] arm: dts: omap: Mark various register maps as dangerous

[Richard Weinberger]

----- Ursprüngliche Mail -----
> Von: "Krzysztof Kozlowski" <krzk@kernel.org>
>> Okay, I didn't know that the OMAP DT is in that bad shape.
> 
> Heh, let me give you a piece of a spoiler of my OSS Japan talk in a
> week: TI (entire TI, not individual sub-platforms) for arm32 is 0%
> compliant, meaning EVERY board has issues, and TI has THE HIGHEST total
> number of warnings of all active mainline arm32 platforms.

Oh dear. :-S
 
> TI ARM32 status is absolutely terrible... TI ARM64 is on quite different
> side. :)
> 
> More of hall-of-shame on 9th of December...

Let's hope this helps to motivate some TI guys.
I can help with TI platforms I have access to but I don't consider
myself an TI SoC expert to do massive refactoring of their device trees.

Thanks,
//richard

Re: [PATCH 4/4] arm: dts: omap: Mark various register maps as dangerous

[Andreas Kemnade]

On Sat, 29 Nov 2025 16:35:31 +0100 (CET)
Richard Weinberger <richard@nod.at> wrote:

> ----- Ursprüngliche Mail -----
> > Von: "Krzysztof Kozlowski" <krzk@kernel.org>
> > An: "richard" <richard@nod.at>, "linux-kernel" <linux-kernel@vger.kernel.org>
> > CC: "linux-omap" <linux-omap@vger.kernel.org>, "devicetree" <devicetree@vger.kernel.org>, "Arnd Bergmann"
> > <arnd@arndb.de>, "Lee Jones" <lee@kernel.org>, "dakr" <dakr@kernel.org>, "Rafael J. Wysocki" <rafael@kernel.org>, "Greg
> > Kroah-Hartman" <gregkh@linuxfoundation.org>, "Mark Brown" <broonie@kernel.org>, "tony" <tony@atomide.com>, "rogerq"
> > <rogerq@kernel.org>, "khilman" <khilman@baylibre.com>, "Andreas Kemnade" <andreas@kemnade.info>, "aaro koskinen"
> > <aaro.koskinen@iki.fi>, "Conor Dooley" <conor+dt@kernel.org>, "Krzysztof Kozlowski" <krzk+dt@kernel.org>, "robh"
> > <robh@kernel.org>
> > Gesendet: Samstag, 29. November 2025 16:26:19
> > Betreff: Re: [PATCH 4/4] arm: dts: omap: Mark various register maps as dangerous  
> 
> > On 29/11/2025 15:20, Richard Weinberger wrote:  
> >> index 711ce4c31bb1f..1b1f31608d37e 100644
> >> --- a/arch/arm/boot/dts/ti/omap/dra7.dtsi
> >> +++ b/arch/arm/boot/dts/ti/omap/dra7.dtsi
> >> @@ -359,6 +359,7 @@ bandgap: bandgap@4a0021e0 {
> >>  		dsp1_system: dsp_system@40d00000 {
> >>  			compatible = "syscon";  
> > 
> > Oh, no no, sorry, but buggy/incomplete/legacy DT is not an excuse for
> > new properties. You cannot have such compatible alone in the first place.  
> 
> Okay, I didn't know that the OMAP DT is in that bad shape.
> Is somebody working on a solution?
> 
well, it is an arduous work, piece by piece work. But things are getting better
slowly. I convert bindings from time to time, mostly the ones
needed by omap3-5 parts, not the active am3/5 parts.

compare
https://gitlab.com/robherring/linux-dt/-/jobs/5620809910#L5618

to the newer
https://gitlab.com/robherring/linux-dt/-/jobs/12112550529#L5156


Regards,
Andreas

Re: [PATCH 1/4] dt-bindings: Document new common property: has-inaccessible-regs

[Andreas Kemnade]

On Sat, 29 Nov 2025 16:49:11 +0100
Krzysztof Kozlowski <krzk@kernel.org> wrote:

> On 29/11/2025 16:44, Krzysztof Kozlowski wrote:
> >>         scm_conf_clocks: clocks {
> >>                 #address-cells = <1>;
> >>                 #size-cells = <0>;
> >>         };      
> >> };
> >>
> >> So, drivers like ti,pbias-dra7 or ti,dra7xx-phy-gmii-sel touch only registers
> >> they know about and this works well.
> >> But syscon manages the whole register map via regmap, and regmap exposes it all
> >> via debugfs.
> >>
> >> What solution do you propose?
> >> Splitting reg = <0x0 0x1400> into many tiny fractions and not using an mfd anymore?  
> > 
> > Fix the driver. In your case, the syscon driver.  
> 
> BTW, the state of existing TI DRA code is so poor that you don't have
> many choices... or rather every choice has drawbacks. If this was proper
> DTS, then I would say - define register map, used by regmap, for your
> compatible either in syscon driver or dedicated driver (thus new driver
> will be the syscon provider for you, just like Google GS101 syscon is
> special).
> 
> Or maybe this is not syscon at all!
> 
> Remember that syscon is a collection of miscellaneous system controller
> registers. You should not use syscon for other things, like devices with
> incomplete hardware description.
> 
It is referenced in mmu0_disp1, it looks like some syscon.
mmu0_dsp1:
...
   ti,syscon-mmuconfig = <&dsp1_system 0x0>;

So it looks valid. In code omap-iommu.c, aparrently only one register is
written.

But again, DRA7 is not my area, OMAP3-5: yes.

Regards,
Andreas

Re: [PATCH 1/4] dt-bindings: Document new common property: has-inaccessible-regs

[Krzysztof Kozlowski]

On 29/11/2025 16:56, Richard Weinberger wrote:
> ----- Ursprüngliche Mail -----
>> Von: "Krzysztof Kozlowski" <krzk@kernel.org>
>>> So, drivers like ti,pbias-dra7 or ti,dra7xx-phy-gmii-sel touch only registers
>>> they know about and this works well.
>>> But syscon manages the whole register map via regmap, and regmap exposes it all
>>> via debugfs.
>>>
>>> What solution do you propose?
>>> Splitting reg = <0x0 0x1400> into many tiny fractions and not using an mfd
>>> anymore?
>>
>> Fix the driver. In your case, the syscon driver.
> 
> Please help me to understand what the desired behavior of the driver is.
> 
> Currently syscon creates one regmap for everything and passes this regmap
> to the individual syscon users.
> These users have to know what offset within the regmap is their playground.
> If I understand correctly, it would be better if every syscon user would register their own regmap?

I don't think so. This device driver, so the syscon, creates the regmap
and knows EXACTLY which registers are valid or not. It is not
responsibility of the consumer to tell the syscon what this syscon is.
Syscon knows that...

Best regards,
Krzysztof

Re: [PATCH 2/4] regmap: Allow disabling debugfs via regmap_config

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 478 bytes --]

On Sat, Nov 29, 2025 at 03:20:40PM +0100, Richard Weinberger wrote:
> Regmap already disables register access via debugfs as soon as a register
> map is used without taking locks.
> Go a step further and allow disabling debugfs via regmap_config such that
> device drivers can decide on their own whether uncontrolled register access
> via debugfs is harmful.

What's the use case for this?  Drivers can already mark registers as
unreadable and precious to stop spurious reads.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 1/4] dt-bindings: Document new common property: has-inaccessible-regs

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 369 bytes --]

On Sat, Nov 29, 2025 at 04:33:38PM +0100, Richard Weinberger wrote:
> ----- Ursprüngliche Mail -----

> > Binding cannot help here at all.

> The driver does not expose them via sysfs, it's the regmap framework via debugfs.

regmap exposes the registers that the driver told it about, it won't
expose anything without the driver telling it about the register.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 0/4] Add tooling to disable debugfs on OMAP based systems

[Rob Herring]

On Sat, 29 Nov 2025 15:20:38 +0100, Richard Weinberger wrote:
> It came to my attention that commands such as `grep -r / -e ...` can cause
> crashes on an AM572x based system.
> An investigation found that reading from various files in /sys/kernel/debug/regmap
> causes imprecise async data aborts.
> 
> One of these register maps is the CTRL_MODULE_CORE register map at 0x4A002000.
> It contains various registers marked as reserved, but the manual indicates
> that read access is still allowed.
> On said system, reading from most registers seems to work, but for some
> an async data abort happens. So it's not entirely clear what registers are safe
> and which are not.
> 
> So, add tooling to allow disabling debugfs access to such dangerous registers.
> Splitting the register map definitions in the device tree seemed less practical to
> me since it would unnecessarily make the device trees more complicated.
> 
> Richard Weinberger (4):
>   dt-bindings: Document new common property: has-inaccessible-regs
>   regmap: Allow disabling debugfs via regmap_config
>   syscon: Wire up has-inaccessible-regs
>   arm: dts: omap: Mark various register maps as dangerous
> 
>  .../devicetree/bindings/common-properties.txt        | 12 ++++++++++++
>  arch/arm/boot/dts/ti/omap/dra7-l4.dtsi               |  4 ++++
>  arch/arm/boot/dts/ti/omap/dra7.dtsi                  |  1 +
>  arch/arm/boot/dts/ti/omap/dra74x.dtsi                |  1 +
>  drivers/base/regmap/regmap.c                         |  2 ++
>  drivers/mfd/syscon.c                                 | 10 ++++++++++
>  include/linux/regmap.h                               |  3 +++
>  7 files changed, 33 insertions(+)
> 
> --
> 2.51.0
> 
> 
> 


My bot found new DTB warnings on the .dts files added or changed in this
series.

Some warnings may be from an existing SoC .dtsi. Or perhaps the warnings
are fixed by another series. Ultimately, it is up to the platform
maintainer whether these warnings are acceptable or not. No need to reply
unless the platform maintainer has comments.

If you already ran DT checks and didn't see these error(s), then
make sure dt-schema is up to date:

  pip3 install dtschema --upgrade


This patch series was applied (using b4) to base:
 Base: attempting to guess base-commit...
 Base: tags/v6.18-rc7-1539-gff736a286116 (exact match)
 Base: tags/v6.18-rc7-1539-gff736a286116 (use --merge-base to override)

If this is not the correct base, please add 'base-commit' tag
(or use b4 which does this automatically)

New warnings running 'make CHECK_DTBS=y for arch/arm/boot/dts/ti/' for 20251129142042.344359-1-richard@nod.at:

arch/arm/boot/dts/ti/omap/am571x-idk.dtb: clock@400 (ti,omap4-cm): '#clock-cells' is a dependency of 'clock-output-names'
	from schema $id: http://devicetree.org/schemas/clock/clock.yaml
arch/arm/boot/dts/ti/omap/am57xx-beagle-x15.dtb: clock@c00 (ti,omap4-cm): '#clock-cells' is a dependency of 'clock-output-names'
	from schema $id: http://devicetree.org/schemas/clock/clock.yaml
arch/arm/boot/dts/ti/omap/am57xx-beagle-x15-revc.dtb: clock@500 (ti,omap4-cm): '#clock-cells' is a dependency of 'clock-output-names'
	from schema $id: http://devicetree.org/schemas/clock/clock.yaml

Re: [PATCH 0/4] Add tooling to disable debugfs on OMAP based systems

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 262 bytes --]

On Sat, Nov 29, 2025 at 03:36:44PM +0100, Andreas Kemnade wrote:

> it is usually not about individual registers, but about accessing 
> unpowered devices/modules,

If that's the case the regmap should be in cache only mode while the
device is unpowered.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 1/4] dt-bindings: Document new common property: has-inaccessible-regs

[Rob Herring]

On Sat, Nov 29, 2025 at 8:22 AM Richard Weinberger <richard@nod.at> wrote:
>
> This property is used to denote that a certain register map contains
> registers that are inaccessible under conditions only a device driver
> can know.
> The purpose of this property is to disable register access through debug
> facilities outside of the device driver.
>
> Signed-off-by: Richard Weinberger <richard@nod.at>
> ---
>  .../devicetree/bindings/common-properties.txt        | 12 ++++++++++++

This seems to be dead, but just so you are aware, new properties must
be in a schema. Something like this would probably go in dtschema, not
the kernel. (And this file needs to be removed)

Rob

Re: [PATCH 1/4] dt-bindings: Document new common property: has-inaccessible-regs

[Richard Weinberger]

----- Ursprüngliche Mail -----
> Von: "Krzysztof Kozlowski" <krzk@kernel.org>
>>>> What solution do you propose?
>>>> Splitting reg = <0x0 0x1400> into many tiny fractions and not using an mfd
>>>> anymore?
>>>
>>> Fix the driver. In your case, the syscon driver.
>> 
>> Please help me to understand what the desired behavior of the driver is.
>> 
>> Currently syscon creates one regmap for everything and passes this regmap
>> to the individual syscon users.
>> These users have to know what offset within the regmap is their playground.
>> If I understand correctly, it would be better if every syscon user would
>> register their own regmap?
> 
> I don't think so. This device driver, so the syscon, creates the regmap
> and knows EXACTLY which registers are valid or not. It is not
> responsibility of the consumer to tell the syscon what this syscon is.
> Syscon knows that...

How to configure this in syscon?
AFAIK it takes only a single reg property.
Are you suggesting to add many more syscon nodes to the DT to skip the holes?

Currently the scm_conf@0 DT node defines the first 0x1400 bytes
of the CTRL_MODULE_CORE register[0].

Reading from register 0x180 triggers an async data abort here.
The manual describes it as "RESERVED" of type "R".
Lots of other offsets in CTRL_MODULE_CORE are reserved, but reading works.

Long story short, please tell me how to model it in DT and I'll do so.

Thanks,
//richard

[0] https://www.ti.com/lit/ug/spruhz6l/spruhz6l.pdf 18.5.2.1 CTRL_MODULE_CORE Register Summary

Re: [PATCH 1/4] dt-bindings: Document new common property: has-inaccessible-regs

[Krzysztof Kozlowski]

On 01/12/2025 22:34, Richard Weinberger wrote:
> ----- Ursprüngliche Mail -----
>> Von: "Krzysztof Kozlowski" <krzk@kernel.org>
>>>>> What solution do you propose?
>>>>> Splitting reg = <0x0 0x1400> into many tiny fractions and not using an mfd
>>>>> anymore?
>>>>
>>>> Fix the driver. In your case, the syscon driver.
>>>
>>> Please help me to understand what the desired behavior of the driver is.
>>>
>>> Currently syscon creates one regmap for everything and passes this regmap
>>> to the individual syscon users.
>>> These users have to know what offset within the regmap is their playground.
>>> If I understand correctly, it would be better if every syscon user would
>>> register their own regmap?
>>
>> I don't think so. This device driver, so the syscon, creates the regmap
>> and knows EXACTLY which registers are valid or not. It is not
>> responsibility of the consumer to tell the syscon what this syscon is.
>> Syscon knows that...
> 
> How to configure this in syscon?
> AFAIK it takes only a single reg property.
> Are you suggesting to add many more syscon nodes to the DT to skip the holes?
> 
> Currently the scm_conf@0 DT node defines the first 0x1400 bytes
> of the CTRL_MODULE_CORE register[0].
> 
> Reading from register 0x180 triggers an async data abort here.
> The manual describes it as "RESERVED" of type "R".
> Lots of other offsets in CTRL_MODULE_CORE are reserved, but reading works.
> 
> Long story short, please tell me how to model it in DT and I'll do so.

I already told you:

"...we had it in several devices and fixed drivers."

"Fix the driver. In your case, the syscon driver."

and finally:

"BTW, the state of existing TI DRA code is so poor that you don't have
many choices... or rather every choice has drawbacks. If this was proper
DTS, then I would say - define register map, used by regmap, for your
compatible either in syscon driver or dedicated driver (thus new driver
will be the syscon provider for you, just like Google GS101 syscon is
special)."

What to say more? This is the instruction/proposal.

Best regards,
Krzysztof

Re: [PATCH 1/4] dt-bindings: Document new common property: has-inaccessible-regs

[Richard Weinberger]

----- Ursprüngliche Mail -----
> Von: "Krzysztof Kozlowski" <krzk@kernel.org>
>> Long story short, please tell me how to model it in DT and I'll do so.
> 
> I already told you:
> 
> "...we had it in several devices and fixed drivers."
> 
> "Fix the driver. In your case, the syscon driver."
> 
> and finally:
> 
> "BTW, the state of existing TI DRA code is so poor that you don't have
> many choices... or rather every choice has drawbacks. If this was proper
> DTS, then I would say - define register map, used by regmap, for your
> compatible either in syscon driver or dedicated driver (thus new driver
> will be the syscon provider for you, just like Google GS101 syscon is
> special)."
> 
> What to say more? This is the instruction/proposal.

I'm still chewing.
Let me do more research before I come back with more questions.

Thanks,
//richard