reset: introduce TC9564 reset support
Subsystems (2)
cover reset: introduce TC9564 reset support
This series adds support for the reset controller present within the Toshiba TC9564 SoC. It is being considered the first version of the series despite being previously posted as part of a combined clock/reset driver:
https://lore.kernel.org/lkml/[email protected]/
This SoC attaches to a host system via PCIe. It includes a PCIe switch with three downstream ports. To one of these ports an embedded PCIe endpoint is attached, and it implements two PCI functions. BAR 4 for both PCI functions provides access to the registers that enable or disable a set of resets.
Both TC9564 PCI functions can provide PCI endpoint buses via devicetree. Each PCI endpoint bus provides access to one of the function's BARs. Subordinate devices (like the reset controller) can then be defined as nodes accessible on the endpoint bus.
Only one of the two PCI functions is permitted to define the reset device node.
Note:
Other TC9564 drivers have been posted together with DTS file updates for the Qualcomm RB3gen2. To allow this (and a few other) drivers to be considered in isolation, I'm omiting that here. (I can add it to future versions if requested.)
-Alex
Alex Elder (2):
dt-bindings: reset: introduce toshiba,tc9564-reset.yaml
reset: toshiba: introduce a TC9564 SoC reset driver
.../bindings/reset/toshiba,tc9564-reset.yaml | 56 +++++ MAINTAINERS | 8 + drivers/reset/Kconfig | 12 ++ drivers/reset/Makefile | 1 + drivers/reset/reset-tc9564.c | 192 ++++++++++++++++++ include/dt-bindings/reset/toshiba,tc9564.h | 26 +++ 6 files changed, 295 insertions(+) create mode 100644 Documentation/devicetree/bindings/reset/toshiba,tc9564-reset.yaml create mode 100644 drivers/reset/reset-tc9564.c create mode 100644 include/dt-bindings/reset/toshiba,tc9564.h
base-commit: becb95e45ef0dda9ffb35156d909d3da7a2789c3
Signature · 1 line
2.53.0
PATCH 1/2 dt-bindings: reset: introduce toshiba,tc9564-reset.yaml
Define the binding for the reset controller functionality present in the Toshiba TC9564 SoC.
--- .../bindings/reset/toshiba,tc9564-reset.yaml | 56 +++++++++++++++++++ MAINTAINERS | 7 +++ include/dt-bindings/reset/toshiba,tc9564.h | 26 +++++++++ 3 files changed, 89 insertions(+) create mode 100644 Documentation/devicetree/bindings/reset/toshiba,tc9564-reset.yaml create mode 100644 include/dt-bindings/reset/toshiba,tc9564.h
Documentation/devicetree/bindings/reset/toshiba,tc9564-reset.yaml new file +56 -0
@@ -0,0 +1,56 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/reset/toshiba,tc9564-reset.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Toshiba TC9564 Reset Controller + +maintainers: + - Alex Elder <[email protected]> + - Daniel Thompson <[email protected]> + +description: + The Toshiba TC9564 is an SoC accessed by a host system through the + upstream PCIe port on the PCIe switch it implements. The switch + includes an embedded PCIe endpoint that provides access to various + SoC peripherals (including a reset controller) via its BARs. + + A total of 13 resets are implemented, though two of these are not + controllable. Access to the reset controller relies on PCIe being + functional, so the PCIe reset is assumed to be deasserted. Similarly, + the PCIe controller relies on I2C, so the I2C reset is also assumed + to be deasserted. + + Reset ids are defined in <dt-bindings/reset/toshiba,tc9564.h>. + + The reset controller uses registers within the same page as a + clock controller, so both devices are defined as children of a + "config" syscon. + +properties: + compatible: + const: toshiba,tc9564-reset + + "#reset-cells": + const: 1 + +required: + - compatible + - "#reset-cells" + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/reset/toshiba,tc9564.h> + + syscon@0 { + compatible = "syscon", "simple-mfd"; + reg = <0x0 0x2000>; + + reset { + compatible = "toshiba,tc9564-reset"; + #reset-cells = <1>; + }; + };
MAINTAINERS +7 -0
@@ -28180,6 +28180,13 @@ S: Maintained F: Documentation/devicetree/bindings/misc/pci1179,0220.yaml F: drivers/misc/tc9564-pci.c +TOSHIBA TC9564 RESET DRIVER +M: Alex Elder <[email protected]> +M: Daniel Thompson <[email protected]> +S: Maintained +F: Documentation/devicetree/bindings/reset/toshiba,tc9564-reset.yaml +F: include/dt-bindings/reset/toshiba,tc9564.h + TOSHIBA WMI HOTKEYS DRIVER M: Azael Avalos <[email protected]> L: [email protected]
include/dt-bindings/reset/toshiba,tc9564.h new file +26 -0
@@ -0,0 +1,26 @@ +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */ + +/* +* Copyright (C) 2026 by RISCstar Solutions Corporation. All rights reserved. +*/ + +#ifndef __RESET_TOSHIBA_TC9564_H__ +#define __RESET_TOSHIBA_TC9564_H__ + +/* Reset IDs */ + +#define RESET_MCU 0 +#define RESET_MCU1 1 +#define RESET_MSIGEN 2 +#define RESET_INTC 3 +#define RESET_UART 4 + +#define RESET_MAC0_MAC 5 +#define RESET_MAC0_PMA 6 +#define RESET_MAC0_XPCS 7 + +#define RESET_MAC1_MAC 8 +#define RESET_MAC1_PMA 9 +#define RESET_MAC1_XPCS 10 + +#endif /* __RESET_TOSHIBA_TC9564_H__*/
Signature · 1 line
2.53.0
[email protected]
AI reviewRe: [PATCH 1/2] dt-bindings: reset: introduce toshiba,tc9564-reset.yaml
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Low] dt-bindings: syscon missing hardware-specific compatible in example - [Low] dt-bindings: incorrect…
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] dt-bindings: syscon missing hardware-specific compatible in example
- [Low] dt-bindings: incorrect node name 'reset' instead of 'reset-controller' --
commit 86eb967be387fadac7b22aba279d8ed01983af26
Author: Alex Elder <[email protected]>
dt-bindings: reset: introduce toshiba,tc9564-reset.yaml
This commit defines the binding for the reset controller functionality present in the Toshiba TC9564 SoC. It includes a new YAML schema, a header file for the reset IDs, and updates the MAINTAINERS file.
Quoted 4 lines
Documentation/devicetree/bindings/reset/toshiba,tc9564-reset.yaml new file +0
-0
[ ... ]
Documentation/devicetree/bindings/reset/toshiba,tc9564-reset.yaml line 49
Quoted 6 lines
Documentation/devicetree/bindings/reset/toshiba,tc9564-reset.yaml new file +6
-0+examples: + - | + #include <dt-bindings/reset/toshiba,tc9564.h> + + syscon@0 { + compatible = "syscon", "simple-mfd";
[Severity: Low]
Does this example fail dt_binding_check validation? The core syscon.yaml schema typically requires a hardware-specific compatible string to precede generic fallbacks like "syscon" and "simple-mfd".
Documentation/devicetree/bindings/reset/toshiba,tc9564-reset.yaml line 52
Quoted 3 lines
Documentation/devicetree/bindings/reset/toshiba,tc9564-reset.yaml new file +3
-0+ reg = <0x0 0x2000>; + + reset {
[Severity: Low]
Should this node be named "reset-controller" instead of "reset"? The core reset.yaml schema enforces the ^reset-controller(@.*)?$ naming convention for nodes providing resets (identified by the #reset-cells property).
Quoted 4 lines
Documentation/devicetree/bindings/reset/toshiba,tc9564-reset.yaml new file +4
-0+ compatible = "toshiba,tc9564-reset"; + #reset-cells = <1>; + }; + };
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
Re: [PATCH 1/2] dt-bindings: reset: introduce toshiba,tc9564-reset.yaml
Quoted 14 lines · On 10/12/26 11:48 AM, [email protected] wrote:
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] dt-bindings: syscon missing hardware-specific compatible in example
- [Low] dt-bindings: incorrect node name 'reset' instead of 'reset-controller' --commit
86eb967be387fadac7b22aba279d8ed01983af26
Author: Alex Elder <[email protected]>dt-bindings: reset: introduce toshiba,tc9564-reset.yaml
This commit defines the binding for the reset controller functionality present in the Toshiba TC9564 SoC. It includes a new YAML schema, a header file for the reset IDs, and updates the
MAINTAINERSfile.
Quoted 10 lines
Quoted 4 lines
Documentation/devicetree/bindings/reset/toshiba,tc9564-reset.yaml new file +0
-0[ ... ]
Quoted 3 lines
Documentation/devicetree/bindings/reset/toshiba,tc9564-reset.yaml new file +3
-0+examples: + - | + #include <dt-bindings/reset/toshiba,tc9564.h>
Unfortunately I hadn't noticed this before I sent it. Krzysztof pointed out in the clock series, including this here is unnecessary, because its contents are not actually used in the example. I will remove this for version 2.
Quoted 8 lines
Documentation/devicetree/bindings/reset/toshiba,tc9564-reset.yaml line 49
Quoted 3 lines
Documentation/devicetree/bindings/reset/toshiba,tc9564-reset.yaml new file +3
-0+ + syscon@0 { + compatible = "syscon", "simple-mfd";[Severity: Low]
Does this example faildt_binding_checkvalidation? The coresyscon.yamlschema typically requires a hardware-specific compatible string to precede generic fallbacks like "syscon" and "simple-mfd".
It did not fail dt_binding_check validation for me, however I am rarely surprised to discover there are errors in my bindings...
Quoted 8 lines
Documentation/devicetree/bindings/reset/toshiba,tc9564-reset.yaml line 52
Quoted 3 lines
Documentation/devicetree/bindings/reset/toshiba,tc9564-reset.yaml new file +3
-0+ reg = <0x0 0x2000>; + + reset {[Severity: Low]
Should this node be named "reset-controller" instead of "reset"? The corereset.yamlschema enforces the ^reset-controller(@.*)?$ naming convention for nodes providing resets (identified by the #reset-cells property).
Yes it should. I corrected this in my private DTS file but neglected to correct it in the binding example. I will fix this when I send version 2.
-Alex
Quoted 5 lines
Quoted 4 lines
Documentation/devicetree/bindings/reset/toshiba,tc9564-reset.yaml new file +4
-0+ compatible = "toshiba,tc9564-reset"; + #reset-cells = <1>; + }; + };
Re: [PATCH 1/2] dt-bindings: reset: introduce toshiba,tc9564-reset.yaml
In reply to Alex ElderQuoted 43 lines · Alex Elder
Define the binding for the reset controller functionality present in the Toshiba TC9564 SoC.
Co-developed-by Daniel Thompson <[email protected]>Signed-off-by Daniel Thompson <[email protected]>Signed-off-by Alex Elder <[email protected]>--- .../bindings/reset/toshiba,tc9564-reset.yaml | 56 +++++++++++++++++++ MAINTAINERS | 7 +++ include/dt-bindings/reset/toshiba,tc9564.h | 26 +++++++++ 3 files changed, 89 insertions(+) create mode 100644 Documentation/devicetree/bindings/reset/toshiba,tc9564-reset.yaml create mode 100644 include/dt-bindings/reset/toshiba,tc9564.hDocumentation/devicetree/bindings/reset/toshiba,tc9564-reset.yaml new file +23
-0@@ -0,0 +1,56 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/reset/toshiba,tc9564-reset.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Toshiba TC9564 Reset Controller + +maintainers: + - Alex Elder <[email protected]> + - Daniel Thompson <[email protected]> + +description: + The Toshiba TC9564 is an SoC accessed by a host system through the + upstream PCIe port on the PCIe switch it implements. The switch + includes an embedded PCIe endpoint that provides access to various + SoC peripherals (including a reset controller) via its BARs. + + A total of 13 resets are implemented, though two of these are not + controllable. Access to the reset controller relies on PCIe being + functional, so the PCIe reset is assumed to be deasserted. Similarly, + the PCIe controller relies on I2C, so the I2C reset is also assumed + to be deasserted.
Documentation/devicetree/bindings/reset/toshiba,tc9564-reset.yaml line 33
Quoted 10 lines
Documentation/devicetree/bindings/reset/toshiba,tc9564-reset.yaml new file +10
-0+ + Reset ids are defined in <dt-bindings/reset/toshiba,tc9564.h>. + + The reset controller uses registers within the same page as a + clock controller, so both devices are defined as children of a + "config" syscon. + +properties: + compatible: + const: toshiba,tc9564-reset
Why does this need to exist as a standalone device, rather than be rolled up into the parent syscon node? It doesn't have any resources, so it should be doable.
This kind of thing for a syscon is always difficult to review when only one element is present - where is the rest of the syscon's functionality?
Thanks,
Conor.
Quoted 76 lines
Documentation/devicetree/bindings/reset/toshiba,tc9564-reset.yaml new file +23
-0+ + "#reset-cells": + const: 1 + +required: + - compatible + - "#reset-cells" + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/reset/toshiba,tc9564.h> + + syscon@0 { + compatible = "syscon", "simple-mfd"; + reg = <0x0 0x2000>; + + reset { + compatible = "toshiba,tc9564-reset"; + #reset-cells = <1>; + }; + };MAINTAINERS +7
-0@@ -28180,6 +28180,13 @@ S: Maintained F: Documentation/devicetree/bindings/misc/pci1179,0220.yaml F: drivers/misc/tc9564-pci.c +TOSHIBA TC9564 RESET DRIVER +M: Alex Elder <[email protected]> +M: Daniel Thompson <[email protected]> +S: Maintained +F: Documentation/devicetree/bindings/reset/toshiba,tc9564-reset.yaml +F: include/dt-bindings/reset/toshiba,tc9564.h + TOSHIBA WMI HOTKEYS DRIVER M: Azael Avalos <[email protected]> L: [email protected]include/dt-bindings/reset/toshiba,tc9564.h new file +26
-0@@ -0,0 +1,26 @@ +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */ + +/* +* Copyright (C) 2026 by RISCstar Solutions Corporation. All rights reserved. +*/ + +#ifndef __RESET_TOSHIBA_TC9564_H__ +#define __RESET_TOSHIBA_TC9564_H__ + +/* Reset IDs */ + +#define RESET_MCU 0 +#define RESET_MCU1 1 +#define RESET_MSIGEN 2 +#define RESET_INTC 3 +#define RESET_UART 4 + +#define RESET_MAC0_MAC 5 +#define RESET_MAC0_PMA 6 +#define RESET_MAC0_XPCS 7 + +#define RESET_MAC1_MAC 8 +#define RESET_MAC1_PMA 9 +#define RESET_MAC1_XPCS 10 + +#endif /* __RESET_TOSHIBA_TC9564_H__*/ Signature · 1 line
2.53.0
Re: [PATCH 1/2] dt-bindings: reset: introduce toshiba,tc9564-reset.yaml
Quoted 50 lines · Conor Dooley
Quoted 49 lines · Alex Elder
Define the binding for the reset controller functionality present in the Toshiba TC9564 SoC.
Co-developed-by Daniel Thompson <[email protected]>Signed-off-by Daniel Thompson <[email protected]>Signed-off-by Alex Elder <[email protected]>--- .../bindings/reset/toshiba,tc9564-reset.yaml | 56 +++++++++++++++++++ MAINTAINERS | 7 +++ include/dt-bindings/reset/toshiba,tc9564.h | 26 +++++++++ 3 files changed, 89 insertions(+)create mode 100644
Documentation/devicetree/bindings/reset/toshiba,tc9564-reset.yamlcreate mode 100644include/dt-bindings/reset/toshiba,tc9564.hDocumentation/devicetree/bindings/reset/toshiba,tc9564-reset.yaml new file +29
-0@@ -0,0 +1,56 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/reset/toshiba,tc9564-reset.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Toshiba TC9564 Reset Controller + +maintainers: + - Alex Elder <[email protected]> + - Daniel Thompson <[email protected]> + +description: + The Toshiba TC9564 is an SoC accessed by a host system through the + upstream PCIe port on the PCIe switch it implements. The switch + includes an embedded PCIe endpoint that provides access to various + SoC peripherals (including a reset controller) via its BARs. + + A total of 13 resets are implemented, though two of these are not + controllable. Access to the reset controller relies on PCIe being + functional, so the PCIe reset is assumed to be deasserted. Similarly, + the PCIe controller relies on I2C, so the I2C reset is also assumed + to be deasserted. + + Reset ids are defined in <dt-bindings/reset/toshiba,tc9564.h>. + + The reset controller uses registers within the same page as a + clock controller, so both devices are defined as children of a + "config" syscon.
Quoted 8 lines
Documentation/devicetree/bindings/reset/toshiba,tc9564-reset.yaml line 33
Quoted 4 lines
Documentation/devicetree/bindings/reset/toshiba,tc9564-reset.yaml new file +4
-0+ +properties: + compatible: + const: toshiba,tc9564-resetWhy does this need to exist as a standalone device, rather than be rolled up into the parent syscon node? It doesn't have any resources, so it should be doable.
This is along the lines of what Krzysztof has mentioned, and it seems clear I must be missing something.
Quoted 3 lines
This kind of thing for a syscon is always difficult to review when only one element is present - where is the rest of the syscon's functionality?
The reason a syscon was used was simply for the benefit of having a shared regmap that the reset, clock, and DWMAC drivers can use. All three of those drivers use registers that reside within a single page of addressible MMIO space. And I don't know that I can guarantee their probe order.
There is otherwise no "system controller" that is treated as a logical entity within the SoC.
What is the proper way for me to represent this? It's fairly simple; and I mainly want to ensure all three of those drivers can safely access their addresses.
Thanks.
-Alex
Quoted 79 lines
Thanks,
Conor.Quoted 76 lines
Documentation/devicetree/bindings/reset/toshiba,tc9564-reset.yaml new file +23
-0+ + "#reset-cells": + const: 1 + +required: + - compatible + - "#reset-cells" + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/reset/toshiba,tc9564.h> + + syscon@0 { + compatible = "syscon", "simple-mfd"; + reg = <0x0 0x2000>; + + reset { + compatible = "toshiba,tc9564-reset"; + #reset-cells = <1>; + }; + };MAINTAINERS +7
-0@@ -28180,6 +28180,13 @@ S: Maintained F: Documentation/devicetree/bindings/misc/pci1179,0220.yaml F: drivers/misc/tc9564-pci.c +TOSHIBA TC9564 RESET DRIVER +M: Alex Elder <[email protected]> +M: Daniel Thompson <[email protected]> +S: Maintained +F: Documentation/devicetree/bindings/reset/toshiba,tc9564-reset.yaml +F: include/dt-bindings/reset/toshiba,tc9564.h + TOSHIBA WMI HOTKEYS DRIVER M: Azael Avalos <[email protected]> L: [email protected]include/dt-bindings/reset/toshiba,tc9564.h new file +26
-0@@ -0,0 +1,26 @@ +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */ + +/* +* Copyright (C) 2026 by RISCstar Solutions Corporation. All rights reserved. +*/ + +#ifndef __RESET_TOSHIBA_TC9564_H__ +#define __RESET_TOSHIBA_TC9564_H__ + +/* Reset IDs */ + +#define RESET_MCU 0 +#define RESET_MCU1 1 +#define RESET_MSIGEN 2 +#define RESET_INTC 3 +#define RESET_UART 4 + +#define RESET_MAC0_MAC 5 +#define RESET_MAC0_PMA 6 +#define RESET_MAC0_XPCS 7 + +#define RESET_MAC1_MAC 8 +#define RESET_MAC1_PMA 9 +#define RESET_MAC1_XPCS 10 + +#endif /* __RESET_TOSHIBA_TC9564_H__*/ Signature · 1 line
2.53.0
Re: [PATCH 1/2] dt-bindings: reset: introduce toshiba,tc9564-reset.yaml
Quoted 63 lines · Alex Elder
Quoted 50 lines · Conor Dooley
Quoted 49 lines · Alex Elder
Define the binding for the reset controller functionality present in the Toshiba TC9564 SoC.
Co-developed-by Daniel Thompson <[email protected]>Signed-off-by Daniel Thompson <[email protected]>Signed-off-by Alex Elder <[email protected]>--- .../bindings/reset/toshiba,tc9564-reset.yaml | 56 +++++++++++++++++++ MAINTAINERS | 7 +++ include/dt-bindings/reset/toshiba,tc9564.h | 26 +++++++++ 3 files changed, 89 insertions(+)create mode 100644
Documentation/devicetree/bindings/reset/toshiba,tc9564-reset.yamlcreate mode 100644include/dt-bindings/reset/toshiba,tc9564.hDocumentation/devicetree/bindings/reset/toshiba,tc9564-reset.yaml new file +29
-0@@ -0,0 +1,56 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/reset/toshiba,tc9564-reset.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Toshiba TC9564 Reset Controller + +maintainers: + - Alex Elder <[email protected]> + - Daniel Thompson <[email protected]> + +description: + The Toshiba TC9564 is an SoC accessed by a host system through the + upstream PCIe port on the PCIe switch it implements. The switch + includes an embedded PCIe endpoint that provides access to various + SoC peripherals (including a reset controller) via its BARs. + + A total of 13 resets are implemented, though two of these are not + controllable. Access to the reset controller relies on PCIe being + functional, so the PCIe reset is assumed to be deasserted. Similarly, + the PCIe controller relies on I2C, so the I2C reset is also assumed + to be deasserted. + + Reset ids are defined in <dt-bindings/reset/toshiba,tc9564.h>. + + The reset controller uses registers within the same page as a + clock controller, so both devices are defined as children of a + "config" syscon.Quoted 8 lines
Documentation/devicetree/bindings/reset/toshiba,tc9564-reset.yaml line 33
Quoted 4 lines
Documentation/devicetree/bindings/reset/toshiba,tc9564-reset.yaml new file +4
-0+ +properties: + compatible: + const: toshiba,tc9564-resetWhy does this need to exist as a standalone device, rather than be rolled up into the parent syscon node? It doesn't have any resources, so it should be doable.
This is along the lines of what Krzysztof has mentioned, and it seems clear I must be missing something.
Quoted 8 lines
Quoted 3 lines
This kind of thing for a syscon is always difficult to review when only one element is present - where is the rest of the syscon's functionality?
The reason a syscon was used was simply for the benefit of having a shared regmap that the reset, clock, and DWMAC drivers can use. All three of those drivers use registers that reside within a single page of addressible MMIO space.
What does the dwmac use the region for? I assume it's for some misc register and you don't mean that the dwmac lies within the region?
There's nothing inherently wrong with the act of using of a syscon here, what I am actually complaining about is how you are using it. In cases where a syscon region contains clock and reset, the typical way we want people to do this is via something like
syscon@f00 {
compatible = "toshiba,tc9564-bla-syscon", "syscon", "simple-mfd";
reg = <0x0 0x2000>;
#reset-cells = <1>;
#clock-cells = <1>;
};because the child node exists for no other reason than to access linux driver probing in the most trivial way. If there's some features that actually need a child node, that's fine, but those will end up being things with significantly more properties or things with competing property use (e.g. if there were devices with buses that need address/size cells).
Quoted 1 line
And I don't know that I can guarantee their probe order.
I am assuming this is moot, since I am not objecting to using a syscon?
Quoted 6 lines
There is otherwise no "system controller" that is treated as a logical entity within the SoC.
What is the proper way for me to represent this? It's fairly simple; and I mainly want to ensure all three of those drivers can safely access their addresses.
What else does this region actually contain?
Cheers,
Conor.
Quoted 76 lines
Quoted 75 lines
Quoted 75 lines
Documentation/devicetree/bindings/reset/toshiba,tc9564-reset.yaml new file +23
-0+ + "#reset-cells": + const: 1 + +required: + - compatible + - "#reset-cells" + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/reset/toshiba,tc9564.h> + + syscon@0 { + compatible = "syscon", "simple-mfd"; + reg = <0x0 0x2000>; + + reset { + compatible = "toshiba,tc9564-reset"; + #reset-cells = <1>; + }; + };MAINTAINERS +7
-0@@ -28180,6 +28180,13 @@ S: Maintained F: Documentation/devicetree/bindings/misc/pci1179,0220.yaml F: drivers/misc/tc9564-pci.c +TOSHIBA TC9564 RESET DRIVER +M: Alex Elder <[email protected]> +M: Daniel Thompson <[email protected]> +S: Maintained +F: Documentation/devicetree/bindings/reset/toshiba,tc9564-reset.yaml +F: include/dt-bindings/reset/toshiba,tc9564.h + TOSHIBA WMI HOTKEYS DRIVER M: Azael Avalos <[email protected]> L: [email protected]include/dt-bindings/reset/toshiba,tc9564.h new file +26
-0@@ -0,0 +1,26 @@ +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */ + +/* +* Copyright (C) 2026 by RISCstar Solutions Corporation. All rights reserved. +*/ + +#ifndef __RESET_TOSHIBA_TC9564_H__ +#define __RESET_TOSHIBA_TC9564_H__ + +/* Reset IDs */ + +#define RESET_MCU 0 +#define RESET_MCU1 1 +#define RESET_MSIGEN 2 +#define RESET_INTC 3 +#define RESET_UART 4 + +#define RESET_MAC0_MAC 5 +#define RESET_MAC0_PMA 6 +#define RESET_MAC0_XPCS 7 + +#define RESET_MAC1_MAC 8 +#define RESET_MAC1_PMA 9 +#define RESET_MAC1_XPCS 10 + +#endif /* __RESET_TOSHIBA_TC9564_H__*/ Signature · 1 line
2.53.0
Re: [PATCH 1/2] dt-bindings: reset: introduce toshiba,tc9564-reset.yaml
Quoted 68 lines · Conor Dooley
Quoted 67 lines · Alex Elder
Quoted 50 lines · Conor Dooley
Quoted 49 lines · Alex Elder
Define the binding for the reset controller functionality present in the Toshiba TC9564 SoC.
Co-developed-by Daniel Thompson <[email protected]>Signed-off-by Daniel Thompson <[email protected]>Signed-off-by Alex Elder <[email protected]>--- .../bindings/reset/toshiba,tc9564-reset.yaml | 56 +++++++++++++++++++ MAINTAINERS | 7 +++ include/dt-bindings/reset/toshiba,tc9564.h | 26 +++++++++ 3 files changed, 89 insertions(+)create mode 100644
Documentation/devicetree/bindings/reset/toshiba,tc9564-reset.yamlcreate mode 100644include/dt-bindings/reset/toshiba,tc9564.hDocumentation/devicetree/bindings/reset/toshiba,tc9564-reset.yaml new file +29
-0@@ -0,0 +1,56 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/reset/toshiba,tc9564-reset.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Toshiba TC9564 Reset Controller + +maintainers: + - Alex Elder <[email protected]> + - Daniel Thompson <[email protected]> + +description: + The Toshiba TC9564 is an SoC accessed by a host system through the + upstream PCIe port on the PCIe switch it implements. The switch + includes an embedded PCIe endpoint that provides access to various + SoC peripherals (including a reset controller) via its BARs. + + A total of 13 resets are implemented, though two of these are not + controllable. Access to the reset controller relies on PCIe being + functional, so the PCIe reset is assumed to be deasserted. Similarly, + the PCIe controller relies on I2C, so the I2C reset is also assumed + to be deasserted. + + Reset ids are defined in <dt-bindings/reset/toshiba,tc9564.h>. + + The reset controller uses registers within the same page as a + clock controller, so both devices are defined as children of a + "config" syscon.Quoted 8 lines
Documentation/devicetree/bindings/reset/toshiba,tc9564-reset.yaml line 33
Quoted 4 lines
Documentation/devicetree/bindings/reset/toshiba,tc9564-reset.yaml new file +4
-0+ +properties: + compatible: + const: toshiba,tc9564-resetWhy does this need to exist as a standalone device, rather than be rolled up into the parent syscon node? It doesn't have any resources, so it should be doable.
This is along the lines of what Krzysztof has mentioned, and it seems clear I must be missing something.
Quoted 3 lines
This kind of thing for a syscon is always difficult to review when only one element is present - where is the rest of the syscon's functionality?
Quoted 7 lines
Quoted 4 lines
The reason a syscon was used was simply for the benefit of having a shared regmap that the reset, clock, and DWMAC drivers can use. All three of those drivers use registers that reside within a single page of addressible MMIO space.
What does the dwmac use the region for? I assume it's for some misc register and you don't mean that the dwmac lies within the region?
That's right. There is one register (per DWMAC--there are two)
that contains one bit (bit 21) that is used to signal that initialization of the PMA is done. We assert reset, set up clocks, then deassert, and finally wait for this bit to be set.
The DWMAC code uses a phandle property to refer to the syscon,
like:
toshiba,config-syscon = <&tc9564_config_syscon0 0x1070>
So again, the purpose of the syscon was to get the automatic regmap for managing the region, which contains registers used for distinct purposes.
After making the syscon be the parent of both the reset and clock nodes, I *removed* the "reg" properties and just defined those offsets in the driver. Maybe that was a mistake.
Quoted 11 lines
There's nothing inherently wrong with the act of using of a syscon here, what I am actually complaining about is how you are using it. In cases where a syscon region contains clock and reset, the typical way we want people to do this is via something like
syscon@f00 { compatible = "toshiba,tc9564-bla-syscon", "syscon", "simple-mfd"; reg = <0x0 0x2000>; #reset-cells = <1>; #clock-cells = <1>; };
In this case it seems I must create a syscon driver that provides combined reset and clock functionality. Is that right? (This series is the result of separating the reset code from the clock code in a combined driver.)
To be honest I don't have a lot of experience with proper use of syscons. I have no problem writing or restructuring the code, it just seems I need help knowing what the final structure should look like.
Quoted 6 lines
because the child node exists for no other reason than to access linux driver probing in the most trivial way. If there's some features that actually need a child node, that's fine, but those will end up being things with significantly more properties or things with competing property use (e.g. if there were devices with buses that need address/size cells).
I'm not sure I understand this statement (and that might be part of the problem).
There are other devices (currently just two DWMACs, plus an interrupt controller that generates MSIs for each PCI function) that use the reset and clock signals. I haven't yet posted the reworked DWMAC code but I just sent the first version of the interrupt driver yesterday.
https://lore.kernel.org/lkml/[email protected]/
Quoted 4 lines
Quoted 1 line
And I don't know that I can guarantee their probe order.
I am assuming this is moot, since I am not objecting to using a syscon?
Quoted 8 lines
Quoted 6 lines
There is otherwise no "system controller" that is treated as a logical entity within the SoC.
What is the proper way for me to represent this? It's fairly simple; and I mainly want to ensure all three of those drivers can safely access their addresses.
What else does this region actually contain?
At this point I've described everything we now use. Two registers control a set of resets; two registers enable and disable a set of clocks; and two registers each contain a bit to signal readiness of a DWMAC device.
There are other registers in this region, but at this time we don't use them.
-Alex
Quoted 80 lines
Cheers,
Conor.Quoted 76 lines
Quoted 75 lines
Quoted 75 lines
Documentation/devicetree/bindings/reset/toshiba,tc9564-reset.yaml new file +23
-0+ + "#reset-cells": + const: 1 + +required: + - compatible + - "#reset-cells" + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/reset/toshiba,tc9564.h> + + syscon@0 { + compatible = "syscon", "simple-mfd"; + reg = <0x0 0x2000>; + + reset { + compatible = "toshiba,tc9564-reset"; + #reset-cells = <1>; + }; + };MAINTAINERS +7
-0@@ -28180,6 +28180,13 @@ S: Maintained F: Documentation/devicetree/bindings/misc/pci1179,0220.yaml F: drivers/misc/tc9564-pci.c +TOSHIBA TC9564 RESET DRIVER +M: Alex Elder <[email protected]> +M: Daniel Thompson <[email protected]> +S: Maintained +F: Documentation/devicetree/bindings/reset/toshiba,tc9564-reset.yaml +F: include/dt-bindings/reset/toshiba,tc9564.h + TOSHIBA WMI HOTKEYS DRIVER M: Azael Avalos <[email protected]> L: [email protected]include/dt-bindings/reset/toshiba,tc9564.h new file +26
-0@@ -0,0 +1,26 @@ +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */ + +/* +* Copyright (C) 2026 by RISCstar Solutions Corporation. All rights reserved. +*/ + +#ifndef __RESET_TOSHIBA_TC9564_H__ +#define __RESET_TOSHIBA_TC9564_H__ + +/* Reset IDs */ + +#define RESET_MCU 0 +#define RESET_MCU1 1 +#define RESET_MSIGEN 2 +#define RESET_INTC 3 +#define RESET_UART 4 + +#define RESET_MAC0_MAC 5 +#define RESET_MAC0_PMA 6 +#define RESET_MAC0_XPCS 7 + +#define RESET_MAC1_MAC 8 +#define RESET_MAC1_PMA 9 +#define RESET_MAC1_XPCS 10 + +#endif /* __RESET_TOSHIBA_TC9564_H__*/ Signature · 1 line
2.53.0
Re: [PATCH 1/2] dt-bindings: reset: introduce toshiba,tc9564-reset.yaml
Quoted 85 lines · Alex Elder
Quoted 68 lines · Conor Dooley
Quoted 67 lines · Alex Elder
Quoted 50 lines · Conor Dooley
Quoted 49 lines · Alex Elder
Define the binding for the reset controller functionality present in the Toshiba TC9564 SoC.
Co-developed-by Daniel Thompson <[email protected]>Signed-off-by Daniel Thompson <[email protected]>Signed-off-by Alex Elder <[email protected]>--- .../bindings/reset/toshiba,tc9564-reset.yaml | 56 +++++++++++++++++++ MAINTAINERS | 7 +++ include/dt-bindings/reset/toshiba,tc9564.h | 26 +++++++++ 3 files changed, 89 insertions(+)create mode 100644
Documentation/devicetree/bindings/reset/toshiba,tc9564-reset.yamlcreate mode 100644include/dt-bindings/reset/toshiba,tc9564.hDocumentation/devicetree/bindings/reset/toshiba,tc9564-reset.yaml new file +29
-0@@ -0,0 +1,56 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/reset/toshiba,tc9564-reset.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Toshiba TC9564 Reset Controller + +maintainers: + - Alex Elder <[email protected]> + - Daniel Thompson <[email protected]> + +description: + The Toshiba TC9564 is an SoC accessed by a host system through the + upstream PCIe port on the PCIe switch it implements. The switch + includes an embedded PCIe endpoint that provides access to various + SoC peripherals (including a reset controller) via its BARs. + + A total of 13 resets are implemented, though two of these are not + controllable. Access to the reset controller relies on PCIe being + functional, so the PCIe reset is assumed to be deasserted. Similarly, + the PCIe controller relies on I2C, so the I2C reset is also assumed + to be deasserted. + + Reset ids are defined in <dt-bindings/reset/toshiba,tc9564.h>. + + The reset controller uses registers within the same page as a + clock controller, so both devices are defined as children of a + "config" syscon.Quoted 8 lines
Documentation/devicetree/bindings/reset/toshiba,tc9564-reset.yaml line 33
Quoted 4 lines
Documentation/devicetree/bindings/reset/toshiba,tc9564-reset.yaml new file +4
-0+ +properties: + compatible: + const: toshiba,tc9564-resetWhy does this need to exist as a standalone device, rather than be rolled up into the parent syscon node? It doesn't have any resources, so it should be doable.
This is along the lines of what Krzysztof has mentioned, and it seems clear I must be missing something.
Quoted 3 lines
This kind of thing for a syscon is always difficult to review when only one element is present - where is the rest of the syscon's functionality?
Quoted 7 lines
Quoted 4 lines
The reason a syscon was used was simply for the benefit of having a shared regmap that the reset, clock, and DWMAC drivers can use. All three of those drivers use registers that reside within a single page of addressible MMIO space.
What does the dwmac use the region for? I assume it's for some misc register and you don't mean that the dwmac lies within the region?
That's right. There is one register (per DWMAC--there are two) that contains one bit (bit 21) that is used to signal that initialization of the PMA is done. We assert reset, set up
clocks, then deassert, and finally wait for this bit to be set. The DWMAC code uses a phandle property to refer to the syscon, like:toshiba,config-syscon = <&tc9564_config_syscon0 0x1070>
Quoted 7 lines
So again, the purpose of the syscon was to get the automatic regmap for managing the region, which contains registers used for distinct purposes.
After making the syscon be the parent of both the reset and clock nodes, I *removed* the "reg" properties and just defined those offsets in the driver. Maybe that was a mistake.
No, that's typically what happens in the setup I describe below, so that's not abnormal.
Quoted 12 lines
Quoted 11 lines
There's nothing inherently wrong with the act of using of a syscon here, what I am actually complaining about is how you are using it. In cases where a syscon region contains clock and reset, the typical way we want people to do this is via something like
syscon@f00 { compatible = "toshiba,tc9564-bla-syscon", "syscon", "simple-mfd"; reg = <0x0 0x2000>; #reset-cells = <1>; #clock-cells = <1>; };
Quoted 4 lines
In this case it seems I must create a syscon driver that provides combined reset and clock functionality. Is that right? (This series is the result of separating the reset code from the clock code in a combined driver.)
There's two ways to do it, depending on what sort of device you have. If you have a clock controller that also does reset, the preferred thing to do is something like
clock-controller@f00 {
compatible = "toshiba,tc9564-clk";
reg = <0x0 0x2000>;
#reset-cells = <1>;
#clock-cells = <1>;
};and in the clock controller driver probe the reset controller driver as a auxiliary bus device.
You have something else though, because there's assorted other functions in here, including the dwmac and things you don't use yet. In that case, I would make a node like
syscon@f00 {
compatible = "toshiba,tc9564-bla-syscon", "syscon";
reg = <0x0 0x2000>;
#reset-cells = <1>;
#clock-cells = <1>;
};and in the syscon driver use MFD_CELL_NAME() to probe both clock and reset drivers, and can use device_node_to_regmap(). Your dwmac stuff (and any other future stuff that needs a syscon reference) can use the automagic syscon regmap stuff that we've already talked about.
Note, there's no "simple-mfd" here, because you have no child nodes. The point of "simple-mfd" is to probe those kinds of devices.
Quoted 15 lines
To be honest I don't have a lot of experience with proper use of syscons. I have no problem writing or restructuring the code, it just seems I need help knowing what the final structure should look like.
Quoted 6 lines
because the child node exists for no other reason than to access linux driver probing in the most trivial way. If there's some features that actually need a child node, that's fine, but those will end up being things with significantly more properties or things with competing property use (e.g. if there were devices with buses that need address/size cells).
I'm not sure I understand this statement (and that might be part of the problem).
Quoted 6 lines
There are other devices (currently just two DWMACs, plus an interrupt controller that generates MSIs for each PCI function) that use the reset and clock signals. I haven't yet posted the reworked DWMAC code but I just sent the first version of the interrupt driver yesterday.
https://lore.kernel.org/lkml/[email protected]/
Here I am talking about why child nodes aren't correct in your case. Neither clock nor reset needs anything other than the relevant #cells property, so the only benefit of a child node over having these in the syscon node itself are to use the compatible to probe the driver. Child nodes should only be used when the functions provided by the syscon have conflicting properties or are very complicated (I would probably always use a child node for pinctrl for example). It'd also be reasonable if the functions are reused across devices, but that's rarely the case with clock or reset controllers (or really anything that appears in these kinds of "random register regions" in SoCs).
Does that make more sense?
Quoted 105 lines
Quoted 5 lines
Quoted 2 lines
And I don't know that I can guarantee their probe order.
I am assuming this is moot, since I am not objecting to using a syscon?
Quoted 8 lines
Quoted 6 lines
There is otherwise no "system controller" that is treated as a logical entity within the SoC.
What is the proper way for me to represent this? It's fairly simple; and I mainly want to ensure all three of those drivers can safely access their addresses.
What else does this region actually contain?
At this point I've described everything we now use. Two registers control a set of resets; two registers enable and disable a set of clocks; and two registers each contain a bit to signal readiness of a DWMAC device.
There are other registers in this region, but at this time we don't use them.
-Alex
Quoted 80 lines
Cheers,
Conor.Quoted 76 lines
Quoted 75 lines
Quoted 75 lines
Documentation/devicetree/bindings/reset/toshiba,tc9564-reset.yaml new file +23
-0+ + "#reset-cells": + const: 1 + +required: + - compatible + - "#reset-cells" + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/reset/toshiba,tc9564.h> + + syscon@0 { + compatible = "syscon", "simple-mfd"; + reg = <0x0 0x2000>; + + reset { + compatible = "toshiba,tc9564-reset"; + #reset-cells = <1>; + }; + };MAINTAINERS +7
-0@@ -28180,6 +28180,13 @@ S: Maintained F: Documentation/devicetree/bindings/misc/pci1179,0220.yaml F: drivers/misc/tc9564-pci.c +TOSHIBA TC9564 RESET DRIVER +M: Alex Elder <[email protected]> +M: Daniel Thompson <[email protected]> +S: Maintained +F: Documentation/devicetree/bindings/reset/toshiba,tc9564-reset.yaml +F: include/dt-bindings/reset/toshiba,tc9564.h + TOSHIBA WMI HOTKEYS DRIVER M: Azael Avalos <[email protected]> L: [email protected]include/dt-bindings/reset/toshiba,tc9564.h new file +26
-0@@ -0,0 +1,26 @@ +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */ + +/* +* Copyright (C) 2026 by RISCstar Solutions Corporation. All rights reserved. +*/ + +#ifndef __RESET_TOSHIBA_TC9564_H__ +#define __RESET_TOSHIBA_TC9564_H__ + +/* Reset IDs */ + +#define RESET_MCU 0 +#define RESET_MCU1 1 +#define RESET_MSIGEN 2 +#define RESET_INTC 3 +#define RESET_UART 4 + +#define RESET_MAC0_MAC 5 +#define RESET_MAC0_PMA 6 +#define RESET_MAC0_XPCS 7 + +#define RESET_MAC1_MAC 8 +#define RESET_MAC1_PMA 9 +#define RESET_MAC1_XPCS 10 + +#endif /* __RESET_TOSHIBA_TC9564_H__*/ Signature · 1 line
2.53.0
Re: [PATCH 1/2] dt-bindings: reset: introduce toshiba,tc9564-reset.yaml
Quoted 4 lines · Conor Dooley
Quoted 3 lines · Alex Elder
Quoted 2 lines · Conor Dooley
Quoted 1 line · Alex Elder
On 10/12/26 1:51 PM, Conor Dooley wrote:
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines · Alex Elder
Quoted 7 lines
Define the binding for the reset controller functionality present in the Toshiba TC9564 SoC.
Co-developed-by Daniel Thompson <[email protected]>Signed-off-by Daniel Thompson <[email protected]>Signed-off-by Alex Elder <[email protected]>---
Finally back to looking at this again. Sorry for the delay.
Quoted 78 lines
Quoted 78 lines
Quoted 58 lines
Quoted 58 lines
Quoted 42 lines
Quoted 42 lines
.../bindings/reset/toshiba,tc9564-reset.yaml | 56 +++++++++++++++++++ MAINTAINERS | 7 +++ include/dt-bindings/reset/toshiba,tc9564.h | 26 +++++++++ 3 files changed, 89 insertions(+)create mode 100644
Documentation/devicetree/bindings/reset/toshiba,tc9564-reset.yamlcreate mode 100644include/dt-bindings/reset/toshiba,tc9564.hDocumentation/devicetree/bindings/reset/toshiba,tc9564-reset.yaml new file +29
-0@@ -0,0 +1,56 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/reset/toshiba,tc9564-reset.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Toshiba TC9564 Reset Controller + +maintainers: + - Alex Elder <[email protected]> + - Daniel Thompson <[email protected]> + +description: + The Toshiba TC9564 is an SoC accessed by a host system through the + upstream PCIe port on the PCIe switch it implements. The switch + includes an embedded PCIe endpoint that provides access to various + SoC peripherals (including a reset controller) via its BARs. + + A total of 13 resets are implemented, though two of these are not + controllable. Access to the reset controller relies on PCIe being + functional, so the PCIe reset is assumed to be deasserted. Similarly, + the PCIe controller relies on I2C, so the I2C reset is also assumed + to be deasserted. + + Reset ids are defined in <dt-bindings/reset/toshiba,tc9564.h>. + + The reset controller uses registers within the same page as a + clock controller, so both devices are defined as children of a + "config" syscon.Quoted 8 lines
Documentation/devicetree/bindings/reset/toshiba,tc9564-reset.yaml line 33
Quoted 4 lines
Documentation/devicetree/bindings/reset/toshiba,tc9564-reset.yaml new file +4
-0+ +properties: + compatible: + const: toshiba,tc9564-resetWhy does this need to exist as a standalone device, rather than be rolled up into the parent syscon node? It doesn't have any resources, so it should be doable.
This is along the lines of what Krzysztof has mentioned, and it seems clear I must be missing something.
Quoted 3 lines
This kind of thing for a syscon is always difficult to review when only one element is present - where is the rest of the syscon's functionality?
Quoted 7 lines
Quoted 4 lines
The reason a syscon was used was simply for the benefit of having a shared regmap that the reset, clock, and DWMAC drivers can use. All three of those drivers use registers that reside within a single page of addressible MMIO space.
What does the dwmac use the region for? I assume it's for some misc register and you don't mean that the dwmac lies within the region?
That's right. There is one register (per DWMAC--there are two) that contains one bit (bit 21) that is used to signal that initialization of the PMA is done. We assert reset, set up
clocks, then deassert, and finally wait for this bit to be set. The DWMAC code uses a phandle property to refer to the syscon, like:toshiba,config-syscon = <&tc9564_config_syscon0 0x1070>So again, the purpose of the syscon was to get the automatic regmap for managing the region, which contains registers used for distinct purposes.
Quoted 6 lines
Quoted 3 lines
After making the syscon be the parent of both the reset and clock nodes, I *removed* the "reg" properties and just defined those offsets in the driver. Maybe that was a mistake.
No, that's typically what happens in the setup I describe below, so that's not abnormal.
Thank you for confirming this.
Quoted 28 lines
Quoted 12 lines
Quoted 11 lines
There's nothing inherently wrong with the act of using of a syscon here, what I am actually complaining about is how you are using it. In cases where a syscon region contains clock and reset, the typical way we want people to do this is via something like
syscon@f00 { compatible = "toshiba,tc9564-bla-syscon", "syscon", "simple-mfd"; reg = <0x0 0x2000>; #reset-cells = <1>; #clock-cells = <1>; };Quoted 4 lines
In this case it seems I must create a syscon driver that provides combined reset and clock functionality. Is that right? (This series is the result of separating the reset code from the clock code in a combined driver.)
There's two ways to do it, depending on what sort of device you have. If you have a clock controller that also does reset, the preferred thing to do is something like
clock-controller@f00 { compatible = "toshiba,tc9564-clk"; reg = <0x0 0x2000>; #reset-cells = <1>; #clock-cells = <1>; };
Quoted 2 lines
and in the clock controller driver probe the reset controller driver as a auxiliary bus device.
My last version combined these two but did not make the reset an auxiliary device.
Quoted 11 lines
You have something else though, because there's assorted other functions in here, including the dwmac and things you don't use yet. In that case, I would make a node like
syscon@f00 { compatible = "toshiba,tc9564-bla-syscon", "syscon"; reg = <0x0 0x2000>; #reset-cells = <1>; #clock-cells = <1>; };
Quoted 4 lines
and in the syscon driver use
MFD_CELL_NAME()to probe both clock and reset drivers, and can usedevice_node_to_regmap(). Your dwmac stuff (and any other future stuff that needs a syscon reference) can use the automagic syscon regmap stuff that we've already talked about.
This sounds like the way to go.
Quoted 2 lines
Note, there's no "simple-mfd" here, because you have no child nodes. The point of "simple-mfd" is to probe those kinds of devices.
OK. Let's see if I understand what you're saying.
This approach uses:
- A single top-level syscon driver (under drivers/mfd?) which probes based on a devicetree node compatible string ("toshiba,...")
- That compatible string will require a (fairly simple) devicetree binding
- That driver initializes a regmap which will cover the space defined in its devicetree node (<0x0 0x2000>)
- That driver has an array that uses MFD_CELL_NAME() to define cells for clock and reset
- That driver will call some form of mfd_add_devices() using the MFD cell array
- The stmmac driver can refer to this device node with a phandle property to specify the syscon that provides the regmap it needs
And then, for the clock and reset devices:
- A clock platform driver (under drivers/clk) will match based on the driver name (matching what's supplied by MFD_CELL_NAME())
- A reset driver will be similar (under drivers/reset)
- As you mentioned, these drivers get at the regmap they need using device_node_to_regmap() (or dev_get_regmap() or something)
- And despite these being separate drivers (from the syscon driver) they don't have devicetree nodes, and the syscon node itself defines the clock-controller and reset-controller properties.
Does that sound right? Am I missing anything?
Quoted 22 lines
Quoted 15 lines
To be honest I don't have a lot of experience with proper use of syscons. I have no problem writing or restructuring the code, it just seems I need help knowing what the final structure should look like.
Quoted 6 lines
because the child node exists for no other reason than to access linux driver probing in the most trivial way. If there's some features that actually need a child node, that's fine, but those will end up being things with significantly more properties or things with competing property use (e.g. if there were devices with buses that need address/size cells).
I'm not sure I understand this statement (and that might be part of the problem).
Quoted 6 lines
There are other devices (currently just two DWMACs, plus an interrupt controller that generates MSIs for each PCI function) that use the reset and clock signals. I haven't yet posted the reworked DWMAC code but I just sent the first version of the interrupt driver yesterday.
https://lore.kernel.org/lkml/[email protected]/
Quoted 4 lines
Here I am talking about why child nodes aren't correct in your case. Neither clock nor reset needs anything other than the relevant #cells property, so the only benefit of a child node over having these in the syscon node itself are to use the compatible to probe the driver.
Yes I now see what you mean. By using a MFD model the child device is just a function provided by the top-level syscon device. The driver code still benefits from the device model, but the subordinate devices match based on the device name and not a devicetree compatible string. And no nodes exposed in devicetree are needed for the clock and reset.
Quoted 8 lines
Child nodes should only be used when the functions provided by the syscon have conflicting properties or are very complicated (I would probably always use a child node for pinctrl for example). It'd also be reasonable if the functions are reused across devices, but that's rarely the case with clock or reset controllers (or really anything that appears in these kinds of "random register regions" in SoCs).
Does that make more sense?
Yes, it definitely helps a lot. I realize I might not have every detail right, but I feel much better about how to organize this (and presumably this also addresses what Krzysztof was asking for).
Thank you Conor.
-Alex
Quoted 106 lines
Quoted 106 lines
Quoted 6 lines
Quoted 3 lines
And I don't know that I can guarantee their probe order.
I am assuming this is moot, since I am not objecting to using a syscon?
Quoted 8 lines
Quoted 6 lines
There is otherwise no "system controller" that is treated as a logical entity within the SoC.
What is the proper way for me to represent this? It's fairly simple; and I mainly want to ensure all three of those drivers can safely access their addresses.
What else does this region actually contain?
At this point I've described everything we now use. Two registers control a set of resets; two registers enable and disable a set of clocks; and two registers each contain a bit to signal readiness of a DWMAC device.
There are other registers in this region, but at this time we don't use them.
-Alex
Quoted 80 lines
Cheers,
Conor.Quoted 76 lines
Quoted 75 lines
Quoted 75 lines
Documentation/devicetree/bindings/reset/toshiba,tc9564-reset.yaml new file +23
-0+ + "#reset-cells": + const: 1 + +required: + - compatible + - "#reset-cells" + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/reset/toshiba,tc9564.h> + + syscon@0 { + compatible = "syscon", "simple-mfd"; + reg = <0x0 0x2000>; + + reset { + compatible = "toshiba,tc9564-reset"; + #reset-cells = <1>; + }; + };MAINTAINERS +7
-0@@ -28180,6 +28180,13 @@ S: Maintained F: Documentation/devicetree/bindings/misc/pci1179,0220.yaml F: drivers/misc/tc9564-pci.c +TOSHIBA TC9564 RESET DRIVER +M: Alex Elder <[email protected]> +M: Daniel Thompson <[email protected]> +S: Maintained +F: Documentation/devicetree/bindings/reset/toshiba,tc9564-reset.yaml +F: include/dt-bindings/reset/toshiba,tc9564.h + TOSHIBA WMI HOTKEYS DRIVER M: Azael Avalos <[email protected]> L: [email protected]include/dt-bindings/reset/toshiba,tc9564.h new file +26
-0@@ -0,0 +1,26 @@ +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */ + +/* +* Copyright (C) 2026 by RISCstar Solutions Corporation. All rights reserved. +*/ + +#ifndef __RESET_TOSHIBA_TC9564_H__ +#define __RESET_TOSHIBA_TC9564_H__ + +/* Reset IDs */ + +#define RESET_MCU 0 +#define RESET_MCU1 1 +#define RESET_MSIGEN 2 +#define RESET_INTC 3 +#define RESET_UART 4 + +#define RESET_MAC0_MAC 5 +#define RESET_MAC0_PMA 6 +#define RESET_MAC0_XPCS 7 + +#define RESET_MAC1_MAC 8 +#define RESET_MAC1_PMA 9 +#define RESET_MAC1_XPCS 10 + +#endif /* __RESET_TOSHIBA_TC9564_H__*/ Signature · 1 line
2.53.0
Re: [PATCH 1/2] dt-bindings: reset: introduce toshiba,tc9564-reset.yaml
Quoted 155 lines · Alex Elder
Quoted 4 lines · Conor Dooley
Quoted 3 lines · Alex Elder
Quoted 2 lines · Conor Dooley
Quoted 1 line · Alex Elder
On 10/12/26 1:51 PM, Conor Dooley wrote:
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines · Alex Elder
Quoted 7 lines
Define the binding for the reset controller functionality present in the Toshiba TC9564 SoC.
Co-developed-by Daniel Thompson <[email protected]>Signed-off-by Daniel Thompson <[email protected]>Signed-off-by Alex Elder <[email protected]>---
Finally back to looking at this again. Sorry for the delay.
Quoted 78 lines
Quoted 78 lines
Quoted 58 lines
Quoted 58 lines
Quoted 42 lines
Quoted 42 lines
.../bindings/reset/toshiba,tc9564-reset.yaml | 56 +++++++++++++++++++ MAINTAINERS | 7 +++ include/dt-bindings/reset/toshiba,tc9564.h | 26 +++++++++ 3 files changed, 89 insertions(+)create mode 100644
Documentation/devicetree/bindings/reset/toshiba,tc9564-reset.yamlcreate mode 100644include/dt-bindings/reset/toshiba,tc9564.hDocumentation/devicetree/bindings/reset/toshiba,tc9564-reset.yaml new file +29
-0@@ -0,0 +1,56 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/reset/toshiba,tc9564-reset.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Toshiba TC9564 Reset Controller + +maintainers: + - Alex Elder <[email protected]> + - Daniel Thompson <[email protected]> + +description: + The Toshiba TC9564 is an SoC accessed by a host system through the + upstream PCIe port on the PCIe switch it implements. The switch + includes an embedded PCIe endpoint that provides access to various + SoC peripherals (including a reset controller) via its BARs. + + A total of 13 resets are implemented, though two of these are not + controllable. Access to the reset controller relies on PCIe being + functional, so the PCIe reset is assumed to be deasserted. Similarly, + the PCIe controller relies on I2C, so the I2C reset is also assumed + to be deasserted. + + Reset ids are defined in <dt-bindings/reset/toshiba,tc9564.h>. + + The reset controller uses registers within the same page as a + clock controller, so both devices are defined as children of a + "config" syscon.Quoted 8 lines
Documentation/devicetree/bindings/reset/toshiba,tc9564-reset.yaml line 33
Quoted 4 lines
Documentation/devicetree/bindings/reset/toshiba,tc9564-reset.yaml new file +4
-0+ +properties: + compatible: + const: toshiba,tc9564-resetWhy does this need to exist as a standalone device, rather than be rolled up into the parent syscon node? It doesn't have any resources, so it should be doable.
This is along the lines of what Krzysztof has mentioned, and it seems clear I must be missing something.
Quoted 3 lines
This kind of thing for a syscon is always difficult to review when only one element is present - where is the rest of the syscon's functionality?
Quoted 7 lines
Quoted 4 lines
The reason a syscon was used was simply for the benefit of having a shared regmap that the reset, clock, and DWMAC drivers can use. All three of those drivers use registers that reside within a single page of addressible MMIO space.
What does the dwmac use the region for? I assume it's for some misc register and you don't mean that the dwmac lies within the region?
That's right. There is one register (per DWMAC--there are two) that contains one bit (bit 21) that is used to signal that initialization of the PMA is done. We assert reset, set up
clocks, then deassert, and finally wait for this bit to be set. The DWMAC code uses a phandle property to refer to the syscon, like:toshiba,config-syscon = <&tc9564_config_syscon0 0x1070>So again, the purpose of the syscon was to get the automatic regmap for managing the region, which contains registers used for distinct purposes.
Quoted 6 lines
Quoted 3 lines
After making the syscon be the parent of both the reset and clock nodes, I *removed* the "reg" properties and just defined those offsets in the driver. Maybe that was a mistake.
No, that's typically what happens in the setup I describe below, so that's not abnormal.
Thank you for confirming this.
Quoted 28 lines
Quoted 12 lines
Quoted 11 lines
There's nothing inherently wrong with the act of using of a syscon here, what I am actually complaining about is how you are using it. In cases where a syscon region contains clock and reset, the typical way we want people to do this is via something like
syscon@f00 { compatible = "toshiba,tc9564-bla-syscon", "syscon", "simple-mfd"; reg = <0x0 0x2000>; #reset-cells = <1>; #clock-cells = <1>; };Quoted 4 lines
In this case it seems I must create a syscon driver that provides combined reset and clock functionality. Is that right? (This series is the result of separating the reset code from the clock code in a combined driver.)
There's two ways to do it, depending on what sort of device you have. If you have a clock controller that also does reset, the preferred thing to do is something like
clock-controller@f00 { compatible = "toshiba,tc9564-clk"; reg = <0x0 0x2000>; #reset-cells = <1>; #clock-cells = <1>; };Quoted 2 lines
and in the clock controller driver probe the reset controller driver as a auxiliary bus device.
My last version combined these two but did not make the reset an auxiliary device.
Quoted 11 lines
You have something else though, because there's assorted other functions in here, including the dwmac and things you don't use yet. In that case, I would make a node like
syscon@f00 { compatible = "toshiba,tc9564-bla-syscon", "syscon"; reg = <0x0 0x2000>; #reset-cells = <1>; #clock-cells = <1>; };Quoted 4 lines
and in the syscon driver use
MFD_CELL_NAME()to probe both clock and reset drivers, and can usedevice_node_to_regmap(). Your dwmac stuff (and any other future stuff that needs a syscon reference) can use the automagic syscon regmap stuff that we've already talked about.This sounds like the way to go.
Quoted 8 lines
Quoted 2 lines
Note, there's no "simple-mfd" here, because you have no child nodes. The point of "simple-mfd" is to probe those kinds of devices.
OK. Let's see if I understand what you're saying.
This approach uses:
- A single top-level syscon driver (underdrivers/mfd?) which probes based on a devicetree node compatible string ("toshiba,...")
I'm not sure if it needs to be under mfd (there's some in soc/foo too) but I guess so.
Quoted 4 lines
- That compatible string will require a (fairly simple) devicetree binding
- That driver initializes a regmap which will cover the space defined in its devicetree node (<0x00x2000>)
I don't think the driver has to even do this at all, e.g. sg2044-topsys.c doesn't. Whenever someone requests the regmap it will get created automatically since you're a syscon.
Quoted 7 lines
- That driver has an array that uses
MFD_CELL_NAME()to define cells for clock and reset
- That driver will call some form ofmfd_add_devices()using the MFD cell array
- The stmmac driver can refer to this device node with a phandle property to specify the syscon that provides the regmap it needs
Quoted 6 lines
And then, for the clock and reset devices:
- A clock platform driver (underdrivers/clk) will match based on the driver name (matching what's supplied byMFD_CELL_NAME())
- A reset driver will be similar (underdrivers/reset)
- As you mentioned, these drivers get at the regmap they need usingdevice_node_to_regmap()(ordev_get_regmap()or something)
I think "regmap = device_node_to_regmap(pdev->dev.parent->of_node);" is the correct way to do it.
Quoted 5 lines
- And despite these being separate drivers (from the syscon driver) they don't have devicetree nodes, and the syscon node itself defines the clock-controller and reset-controller properties.
Does that sound right? Am I missing anything?
Sounds about right.
Quoted 43 lines
Quoted 24 lines
Quoted 17 lines
To be honest I don't have a lot of experience with proper use of syscons. I have no problem writing or restructuring the code, it just seems I need help knowing what the final structure should look like.
Quoted 6 lines
because the child node exists for no other reason than to access linux driver probing in the most trivial way. If there's some features that actually need a child node, that's fine, but those will end up being things with significantly more properties or things with competing property use (e.g. if there were devices with buses that need address/size cells).
I'm not sure I understand this statement (and that might be part of the problem).
Quoted 6 lines
There are other devices (currently just two DWMACs, plus an interrupt controller that generates MSIs for each PCI function) that use the reset and clock signals. I haven't yet posted the reworked DWMAC code but I just sent the first version of the interrupt driver yesterday.
https://lore.kernel.org/lkml/[email protected]/Quoted 4 lines
Here I am talking about why child nodes aren't correct in your case. Neither clock nor reset needs anything other than the relevant #cells property, so the only benefit of a child node over having these in the syscon node itself are to use the compatible to probe the driver.
Yes I now see what you mean. By using a MFD model the child device is just a function provided by the top-level syscon device. The driver code still benefits from the device model, but the subordinate devices match based on the device name and not a devicetree compatible string. And no nodes exposed in devicetree are needed for the clock and reset.
Quoted 7 lines
Child nodes should only be used when the functions provided by the syscon have conflicting properties or are very complicated (I would probably always use a child node for pinctrl for example). It'd also be reasonable if the functions are reused across devices, but that's rarely the case with clock or reset controllers (or really anything that appears in these kinds of "random register regions" in SoCs).
Quoted 5 lines
Quoted 1 line
Does that make more sense?
Yes, it definitely helps a lot. I realize I might not have every detail right, but I feel much better about how to organize this (and presumably this also addresses what Krzysztof was asking for).
Yeah presumably!
Cheers,
Conor.
Re: [PATCH 1/2] dt-bindings: reset: introduce toshiba,tc9564-reset.yaml
Quoted 1 line · Conor Dooley
On Wed, Oct 21, 2026 at 02:50:41PM -0500, Alex Elder wrote:
. . .
Quoted 8 lines
Quoted 5 lines
OK. Let's see if I understand what you're saying.
This approach uses:
- A single top-level syscon driver (underdrivers/mfd?) which probes based on a devicetree node compatible string ("toshiba,...")I'm not sure if it needs to be under mfd (there's some in soc/foo too) but I guess so.
I'll look at both places and will choose the one that seems most similar to what I end up with.
Quoted 8 lines
Quoted 4 lines
- That compatible string will require a (fairly simple) devicetree binding
- That driver initializes a regmap which will cover the space defined in its devicetree node (<0x00x2000>)I don't think the driver has to even do this at all, e.g.
sg2044-topsys.cdoesn't. Whenever someone requests the regmap it will get created automatically since you're a syscon.
You're right.
Quoted 14 lines
Quoted 7 lines
- That driver has an array that uses
MFD_CELL_NAME()to define cells for clock and reset
- That driver will call some form ofmfd_add_devices()using the MFD cell array
- The stmmac driver can refer to this device node with a phandle property to specify the syscon that provides the regmap it needsQuoted 6 lines
And then, for the clock and reset devices:
- A clock platform driver (underdrivers/clk) will match based on the driver name (matching what's supplied byMFD_CELL_NAME())
- A reset driver will be similar (underdrivers/reset)
- As you mentioned, these drivers get at the regmap they need usingdevice_node_to_regmap()(ordev_get_regmap()or something)
Quoted 2 lines
I think "regmap =
device_node_to_regmap(pdev->dev.parent->of_node);" is the correct way to do it.
regmap = device_node_to_regmap(dev_of_node(dev->parent));
Quoted 7 lines
Quoted 5 lines
- And despite these being separate drivers (from the syscon driver) they don't have devicetree nodes, and the syscon node itself defines the clock-controller and reset-controller properties.
Does that sound right? Am I missing anything?
Sounds about right.
This is awesome. I went ahead with this plan and it's working fine now.
This syscon driver and "sg2044-topsys.c" are awfully trivial, and they seem like they could be wrapped in something along the lines of "simple-mfd-i2c.c".
Thank you very much for your help on this Conor.
-Alex
. . .
Re: [PATCH 1/2] dt-bindings: reset: introduce toshiba,tc9564-reset.yaml
Quoted 6 lines · Alex Elder
Quoted 1 line · Conor Dooley
On Wed, Oct 21, 2026 at 02:50:41PM -0500, Alex Elder wrote:
This syscon driver and "
sg2044-topsys.c" are awfully trivial, and they seem like they could be wrapped in something along the lines of "simple-mfd-i2c.c".
Yea, I guess there could be some sort of trivial driver that covers these syscons with match data that contains the mfd cells and is effectively just a probe function that looks like:
static int _probe(struct platform_device *pdev)
{
ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_NONE, match_data.devs, ARRAY_SIZE(match_data.devs), NULL, 0, NULL); if (ret) return ret; return devm_of_platform_populate(dev); }
The devm_of_platform_populate() you don't need, but there's other syscons that could use this that contain subnodes too so might as well accommodate them.
Re: [PATCH 1/2] dt-bindings: reset: introduce toshiba,tc9564-reset.yaml
Quoted 24 lines · Conor Dooley
Quoted 6 lines · Alex Elder
Quoted 1 line · Conor Dooley
On Wed, Oct 21, 2026 at 02:50:41PM -0500, Alex Elder wrote:
This syscon driver and "
sg2044-topsys.c" are awfully trivial, and they seem like they could be wrapped in something along the lines of "simple-mfd-i2c.c".Yea, I guess there could be some sort of trivial driver that covers these syscons with match data that contains the mfd cells and is effectively just a probe function that looks like:
static int_probe(struct platform_device *pdev)
{ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_NONE, match_data.devs, ARRAY_SIZE(match_data.devs), NULL, 0, NULL); if (ret) return ret; return devm_of_platform_populate(dev); }
Quoted 3 lines
The
devm_of_platform_populate()you don't need, but there's other syscons that could use this that contain subnodes too so might as well accommodate them.
I'll add this to an eventual TODO list for myself. I won't get to it right away but I'd like to see this improved. We're left with a little duplication of some extremely simple boilerplate code.
-Alex
Re: [PATCH 1/2] dt-bindings: reset: introduce toshiba,tc9564-reset.yaml
In reply to Conor DooleyQuoted 50 lines · Conor Dooley
Quoted 49 lines · Alex Elder
Define the binding for the reset controller functionality present in the Toshiba TC9564 SoC.
Co-developed-by Daniel Thompson <[email protected]>Signed-off-by Daniel Thompson <[email protected]>Signed-off-by Alex Elder <[email protected]>--- .../bindings/reset/toshiba,tc9564-reset.yaml | 56 +++++++++++++++++++ MAINTAINERS | 7 +++ include/dt-bindings/reset/toshiba,tc9564.h | 26 +++++++++ 3 files changed, 89 insertions(+) create mode 100644 Documentation/devicetree/bindings/reset/toshiba,tc9564-reset.yaml create mode 100644 include/dt-bindings/reset/toshiba,tc9564.hDocumentation/devicetree/bindings/reset/toshiba,tc9564-reset.yaml new file +29
-0@@ -0,0 +1,56 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/reset/toshiba,tc9564-reset.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Toshiba TC9564 Reset Controller + +maintainers: + - Alex Elder <[email protected]> + - Daniel Thompson <[email protected]> + +description: + The Toshiba TC9564 is an SoC accessed by a host system through the + upstream PCIe port on the PCIe switch it implements. The switch + includes an embedded PCIe endpoint that provides access to various + SoC peripherals (including a reset controller) via its BARs. + + A total of 13 resets are implemented, though two of these are not + controllable. Access to the reset controller relies on PCIe being + functional, so the PCIe reset is assumed to be deasserted. Similarly, + the PCIe controller relies on I2C, so the I2C reset is also assumed + to be deasserted. + + Reset ids are defined in <dt-bindings/reset/toshiba,tc9564.h>. + + The reset controller uses registers within the same page as a + clock controller, so both devices are defined as children of a + "config" syscon.
Quoted 8 lines
Documentation/devicetree/bindings/reset/toshiba,tc9564-reset.yaml line 33
Quoted 4 lines
Documentation/devicetree/bindings/reset/toshiba,tc9564-reset.yaml new file +4
-0+ +properties: + compatible: + const: toshiba,tc9564-resetWhy does this need to exist as a standalone device, rather than be rolled up into the parent syscon node? It doesn't have any resources, so it should be doable.
I am asking to fold into the parent for multiple revisions now.
Quoted 8 lines
This kind of thing for a syscon is always difficult to review when only one element is present - where is the rest of the syscon's functionality?
Thanks,
Conor.
Quoted 16 lines
Quoted 6 lines
Documentation/devicetree/bindings/reset/toshiba,tc9564-reset.yaml new file +6
-0+ + "#reset-cells": + const: 1 + +required: + - compatibleDocumentation/devicetree/bindings/reset/toshiba,tc9564-reset.yaml line 49
Quoted 10 lines
Documentation/devicetree/bindings/reset/toshiba,tc9564-reset.yaml new file +10
-0+ - "#reset-cells" + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/reset/toshiba,tc9564.h> + + syscon@0 { + compatible = "syscon", "simple-mfd";
Such combination is not allowed. Bindings produce a warning, probably. Anyway, node is pointless here.
Best regards,
Krzysztof
PATCH 2/2 reset: toshiba: introduce a TC9564 SoC reset driver
In reply to Alex ElderDefine a new platform driver that manages reset signals implemented by the Toshiba TC9564 SoC. There are 13 reset signals available, although only 11 can be controlled.
Two registers manage the state of the resets. The registers are accessed via a regmap supplied by a system controller, because the region of memory accessed is shared with other drivers.
Access to the memory region is provided via a BAR on a PCIe endpoint function embedded in the TC9564 SoC. For that reason the PCIe reset cannot be manipulated by this driver (it is assumed to be deasserted). Similarly, control is not available for the I2C reset, because the PCIe subsystem on the TC9564 relies on I2C.
--- MAINTAINERS | 1 + drivers/reset/Kconfig | 12 +++ drivers/reset/Makefile | 1 + drivers/reset/reset-tc9564.c | 192 +++++++++++++++++++++++++++++++++++ 4 files changed, 206 insertions(+) create mode 100644 drivers/reset/reset-tc9564.c
MAINTAINERS +1 -0
@@ -28185,6 +28185,7 @@ M: Alex Elder <[email protected]> M: Daniel Thompson <[email protected]> S: Maintained F: Documentation/devicetree/bindings/reset/toshiba,tc9564-reset.yaml +F: drivers/reset/reset-tc9564.c F: include/dt-bindings/reset/toshiba,tc9564.h TOSHIBA WMI HOTKEYS DRIVER
drivers/reset/Kconfig +12 -0
@@ -358,6 +358,18 @@ config RESET_SUNXI help This enables the reset driver for Allwinner SoCs. +config RESET_TC9564 + tristate "Toshiba TC9564 reset support" + depends on TC9564_PCI + select MFD_SYSCON + default TC9564_PCI + help + This enables support for the reset controller embedded in the + Toshiba TC9564 SoC. The state of reset lines is controlled + via MMIO to a region managed by a system controller; this + ensures access to the region is coordinated between this and + the clock and XGMAC (stmmac) drivers. + config RESET_TENSTORRENT_ATLANTIS tristate "Tenstorrent atlantis reset driver" depends on ARCH_TENSTORRENT || COMPILE_TEST
drivers/reset/Makefile +1 -0
@@ -45,6 +45,7 @@ obj-$(CONFIG_RESET_SKY1_AUDSS) += reset-sky1-audss.o obj-$(CONFIG_RESET_SOCFPGA) += reset-socfpga.o obj-$(CONFIG_RESET_SUNPLUS) += reset-sunplus.o obj-$(CONFIG_RESET_SUNXI) += reset-sunxi.o +obj-$(CONFIG_RESET_TC9564) += reset-tc9564.o obj-$(CONFIG_RESET_TENSTORRENT_ATLANTIS) += reset-tenstorrent-atlantis.o obj-$(CONFIG_RESET_TH1520) += reset-th1520.o obj-$(CONFIG_RESET_TI_SCI) += reset-ti-sci.o
drivers/reset/reset-tc9564.c new file +192 -0
192 lines not on this page · raw message
Signature · 1 line
2.53.0
[email protected]
AI reviewRe: [PATCH 2/2] reset: toshiba: introduce a TC9564 SoC reset driver
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] reset: tc9564: hardware state initialized after reset controller registration - [High] reset…
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] reset: tc9564: hardware state initialized after reset controller registration
- [High] reset: tc9564: active consumers forcibly reset during driver removal --
commit 504236856b96e78b9064b4d58c3ef7ba166341e3
Author: Alex Elder <[email protected]>
reset: toshiba: introduce a TC9564 SoC reset driver
Defines a new platform driver that manages reset signals implemented by the Toshiba TC9564 SoC. The state of the resets is controlled via MMIO to a region managed by a system controller.
Quoted 5 lines
drivers/reset/reset-tc9564.c new file +0
-0
[ ... ]
Quoted 5 lines
drivers/reset/reset-tc9564.c new file +5
-0+static int tc9564_reset_probe (struct platform_device *pdev) +{ +struct device *dev = &pdev->dev; +struct tc9564_resets *resets; +
drivers/reset/reset-tc9564.c line 159
Quoted 10 lines
drivers/reset/reset-tc9564.c new file +10
-0+if (!dev_of_node (dev)) +return dev_err_probe (dev, -EINVAL ,"no devicetree node\n" ); + + resets =tc9564_reset_init (dev); +if (IS_ERR (resets)) +return dev_err_probe (dev,PTR_ERR (resets), +"failed to initialize resets\n" ); + +/* Force all resets to be initially asserted */ +tc9564_reset_assert_all (resets);
[Severity: High]
Does this cause a race condition with consumer drivers?
tc9564_reset_init() globally registers the reset controller via devm_reset_controller_register(). Once registered, consumers can immediately probe and deassert their reset lines.
If a consumer driver deasserts its reset right after registration, will this subsequent call to tc9564_reset_assert_all() overwrite the consumer's hardware state and unexpectedly yank running hardware back into reset?
drivers/reset/reset-tc9564.c line 171
Quoted 12 lines
drivers/reset/reset-tc9564.c new file +12
-0+ +platform_set_drvdata (pdev, resets); + +return 0 ; +} + +static void tc9564_reset_remove (struct platform_device *pdev) +{ +struct tc9564_resets *resets =platform_get_drvdata (pdev); + +/* Leave all resets asserted when done */ +tc9564_reset_assert_all (resets);
[Severity: High]
Does this unexpectedly assert resets for active consumers?
Since the reset controller is registered using devm_reset_controller_register(), the devres subsystem will unregister the controller only after this remove callback completes.
This means the controller is still active and consumers might be actively using their deasserted reset lines when tc9564_reset_assert_all() is called here. Should the driver coordinate with consumers or unregister itself first before forcing all lines into reset?
[ ... ]
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
Re: [PATCH 2/2] reset: toshiba: introduce a TC9564 SoC reset driver
Quoted 32 lines · On 10/12/26 11:54 AM, [email protected] wrote:
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] reset: tc9564: hardware state initialized after reset controller registration
- [High] reset: tc9564: active consumers forcibly reset during driver removal --commit
504236856b96e78b9064b4d58c3ef7ba166341e3
Author: Alex Elder <[email protected]>reset: toshiba: introduce a TC9564 SoC reset driver
Defines a new platform driver that manages reset signals implemented by the Toshiba TC9564 SoC. The state of the resets is controlled via MMIO to a region managed by a system controller.
Quoted 5 lines
drivers/reset/reset-tc9564.c new file +0
-0[ ... ]
Quoted 10 lines
drivers/reset/reset-tc9564.c new file +10
-0+static int tc9564_reset_probe (struct platform_device *pdev) +{ +struct device *dev = &pdev->dev; +struct tc9564_resets *resets; + +if (!dev_of_node (dev)) +return dev_err_probe (dev, -EINVAL ,"no devicetree node\n" ); + + resets =tc9564_reset_init (dev); +if (IS_ERR (resets))
Quoted 8 lines
drivers/reset/reset-tc9564.c line 159
Quoted 5 lines
drivers/reset/reset-tc9564.c new file +5
-0+return dev_err_probe (dev,PTR_ERR (resets), +"failed to initialize resets\n" ); + +/* Force all resets to be initially asserted */ +tc9564_reset_assert_all (resets);[Severity: High]
Does this cause a race condition with consumer drivers?
I hadn't thought of that, but yes, this is a good point. We would like to ensure this is the initial state, but I will rework this so that all defined resets will be asserted initially by "directly" writing registers through the regmap, rather than doing so using the reset API.
Quoted 10 lines
tc9564_reset_init()globally registers the reset controller viadevm_reset_controller_register(). Once registered, consumers can immediately probe and deassert their reset lines.If a consumer driver deasserts its reset right after registration, will this subsequent call to
tc9564_reset_assert_all()overwrite the consumer's hardware state and unexpectedly yank running hardware back into reset?
Quoted 15 lines
drivers/reset/reset-tc9564.c line 171
Quoted 12 lines
drivers/reset/reset-tc9564.c new file +12
-0+ +platform_set_drvdata (pdev, resets); + +return 0 ; +} + +static void tc9564_reset_remove (struct platform_device *pdev) +{ +struct tc9564_resets *resets =platform_get_drvdata (pdev); + +/* Leave all resets asserted when done */ +tc9564_reset_assert_all (resets);[Severity: High]
Does this unexpectedly assert resets for active consumers?
I suppose that's possible, and it's easy enough to simply omit this remove callback (and the assert-all call).
I will adjust this for version 2.
-Alex
Quoted 12 lines
Since the reset controller is registered using
devm_reset_controller_register(), the devres subsystem will unregister the controller only after this remove callback completes.This means the controller is still active and consumers might be actively using their deasserted reset lines when
tc9564_reset_assert_all()is called here. Should the driver coordinate with consumers or unregister itself first before forcing all lines into reset?[ ... ]
Philipp Zabel
maintainerRe: [PATCH 2/2] reset: toshiba: introduce a TC9564 SoC reset driver
In reply to Alex ElderQuoted 141 lines · Alex Elder
Define a new platform driver that manages reset signals implemented by the Toshiba TC9564 SoC. There are 13 reset signals available, although only 11 can be controlled.
Two registers manage the state of the resets. The registers are accessed via a regmap supplied by a system controller, because the region of memory accessed is shared with other drivers.
Access to the memory region is provided via a BAR on a PCIe endpoint function embedded in the TC9564 SoC. For that reason the PCIe reset cannot be manipulated by this driver (it is assumed to be deasserted). Similarly, control is not available for the I2C reset, because the PCIe subsystem on the TC9564 relies on I2C.
Co-developed-by Daniel Thompson <[email protected]>Signed-off-by Daniel Thompson <[email protected]>Signed-off-by Alex Elder <[email protected]>--- MAINTAINERS | 1 + drivers/reset/Kconfig | 12 +++ drivers/reset/Makefile | 1 + drivers/reset/reset-tc9564.c | 192 +++++++++++++++++++++++++++++++++++ 4 files changed, 206 insertions(+) create mode 100644 drivers/reset/reset-tc9564.cMAINTAINERS +1
-0@@ -28185,6 +28185,7 @@ M: Alex Elder <[email protected]> M: Daniel Thompson <[email protected]> S: Maintained F: Documentation/devicetree/bindings/reset/toshiba,tc9564-reset.yaml +F: drivers/reset/reset-tc9564.c F: include/dt-bindings/reset/toshiba,tc9564.h TOSHIBA WMI HOTKEYS DRIVERdrivers/reset/Kconfig +12
-0@@ -358,6 +358,18 @@ config RESET_SUNXI help This enables the reset driver for Allwinner SoCs. +config RESET_TC9564 + tristate "Toshiba TC9564 reset support" + depends on TC9564_PCI + select MFD_SYSCON + default TC9564_PCI + help + This enables support for the reset controller embedded in the + Toshiba TC9564 SoC. The state of reset lines is controlled + via MMIO to a region managed by a system controller; this + ensures access to the region is coordinated between this and + the clock and XGMAC (stmmac) drivers. + config RESET_TENSTORRENT_ATLANTIS tristate "Tenstorrent atlantis reset driver" depends on ARCH_TENSTORRENT || COMPILE_TESTdrivers/reset/Makefile +1
-0@@ -45,6 +45,7 @@ obj-$(CONFIG_RESET_SKY1_AUDSS) += reset-sky1-audss.o obj-$(CONFIG_RESET_SOCFPGA) += reset-socfpga.o obj-$(CONFIG_RESET_SUNPLUS) += reset-sunplus.o obj-$(CONFIG_RESET_SUNXI) += reset-sunxi.o +obj-$(CONFIG_RESET_TC9564) += reset-tc9564.o obj-$(CONFIG_RESET_TENSTORRENT_ATLANTIS) += reset-tenstorrent-atlantis.o obj-$(CONFIG_RESET_TH1520) += reset-th1520.o obj-$(CONFIG_RESET_TI_SCI) += reset-ti-sci.odrivers/reset/reset-tc9564.c new file +63
-0@@ -0,0 +1,192 @@ +// SPDX-License-Identifier: GPL-2.0-only + +/* +* Copyright (C) 2026 by RISCstar Solutions Corporation. All rights reserved. +*/ + +#include <linux/bits.h> +#include <linux/device-id/of.h> +#include <linux/mfd/syscon.h> +#include <linux/module.h> +#include <linux/platform_device.h> +#include <linux/regmap.h> +#include <linux/reset-controller.h> + +#include <dt-bindings/reset/toshiba,tc9564.h> + +#define RST_CTRL0_OFFSET 0x1008 +#define RST_CTRL1_OFFSET 0x1010 + +struct tc9564_reset { +u32 offset;/* RST_CTRL0_OFFSET or RST_CTRL1_OFFSET */ +u32 mask;/* Zero means undefined reset */ +}; + +struct tc9564_resets { +struct regmap *regmap; +struct reset_controller_dev rcdev; +}; + +#define TC9564_RESET_INIT0 (_name, _bit) __TC9564_RESET_INIT(_name, 0, _bit) +#define TC9564_RESET_INIT1 (_name, _bit) __TC9564_RESET_INIT(_name, 1, _bit) + +#define __TC9564_RESET_INIT (_name, _reg, _bit) \ + [RESET_##_name] = { \ + .offset = RST_CTRL ## _reg ## _OFFSET, \ + .mask = BIT(_bit), \ + } + +/* +* The PCIe and I2C resets are controllable, but we depend on PCIe +* and that relies on I2C, so we don't allow them to be asserted. +*/ +static const struct tc9564_reset tc9564_reset[] = { +TC9564_RESET_INIT0 (MCU ,0 ), +TC9564_RESET_INIT0 (MCU1 ,1 ), +TC9564_RESET_INIT0 (INTC ,4 ), +/* TC9564_RESET_INIT0(PCIE, 9), */ +/* TC9564_RESET_INIT0(I2C, 12), */ +TC9564_RESET_INIT0 (UART ,16 ), +TC9564_RESET_INIT0 (MSIGEN ,18 ), + +TC9564_RESET_INIT0 (MAC0_MAC ,7 ), +TC9564_RESET_INIT0 (MAC0_PMA ,30 ), +TC9564_RESET_INIT0 (MAC0_XPCS ,31 ), + +TC9564_RESET_INIT1 (MAC1_MAC ,7 ), +TC9564_RESET_INIT1 (MAC1_PMA ,30 ), +TC9564_RESET_INIT1 (MAC1_XPCS ,31 ), +}; +#define TC9564_RESET_COUNT ARRAY_SIZE(tc9564_reset) + +static const struct tc9564_resets * +rcdev_to_tc9564_resets (struct reset_controller_dev *rcdev)
drivers/reset/reset-tc9564.c line 73
Quoted 10 lines
drivers/reset/reset-tc9564.c new file +10
-0+{ + return container_of_const(rcdev, struct tc9564_resets, rcdev); +} + +static int tc9564_reset_manage (struct reset_controller_dev *rcdev, +unsigned long id,bool assert) +{ +const struct tc9564_resets *resets =rcdev_to_tc9564_resets (rcdev); + +if (id < rcdev->nr_resets) {
This check is superfluous.
fwnode_reset_simple_xlate() already checked this before a reset control was even created.
drivers/reset/reset-tc9564.c line 77
Quoted 4 lines
drivers/reset/reset-tc9564.c new file +4
-0+const struct tc9564_reset *reset = &tc9564_reset[id]; +u32 mask = reset->mask; + +if (mask)
This check is superfluous, as every element in the tc9564_reset array has mask set.
drivers/reset/reset-tc9564.c line 86
Quoted 9 lines
drivers/reset/reset-tc9564.c new file +9
-0+return regmap_update_bits (resets->regmap, + reset->offset, mask, + assert ? mask :0 ); + } + +dev_err (rcdev->dev,"invalid reset (%s id %lu)\n" , + assert ?"assert" :"deassert" , id); + +return -ENXIO ;
This branch is never taken.
Quoted 2 lines
drivers/reset/reset-tc9564.c new file +2
-0+} +
[...]
Quoted 49 lines
drivers/reset/reset-tc9564.c new file +49
-0+static struct tc9564_resets *tc9564_reset_init (struct device *dev) +{ +struct reset_controller_dev *rcdev; +struct tc9564_resets *resets; +struct regmap *regmap; +int ret; + + regmap =syscon_node_to_regmap (dev_of_node (dev->parent)); +if (IS_ERR (regmap)) { +dev_err (dev,"failed to get config regmap\n" ); +return ERR_CAST (regmap); + } + + resets =devm_kzalloc (dev,sizeof (*resets),GFP_KERNEL ); +if (!resets) +return ERR_PTR (-ENOMEM ); + resets->regmap = regmap; + + rcdev = &resets->rcdev; + rcdev->ops = &tc9564_reset_control_ops; + rcdev->owner =THIS_MODULE ; + rcdev->dev = dev; + rcdev->of_node =dev_of_node (dev); + rcdev->nr_resets =TC9564_RESET_COUNT ; + + ret =devm_reset_controller_register (dev, rcdev); +if (ret) +return ERR_PTR (ret); + +return resets; +} + +static int tc9564_reset_probe (struct platform_device *pdev) +{ +struct device *dev = &pdev->dev; +struct tc9564_resets *resets; + +if (!dev_of_node (dev)) +return dev_err_probe (dev, -EINVAL ,"no devicetree node\n" ); + + resets =tc9564_reset_init (dev); +if (IS_ERR (resets)) +return dev_err_probe (dev,PTR_ERR (resets), +"failed to initialize resets\n" ); + +/* Force all resets to be initially asserted */ +tc9564_reset_assert_all (resets); + +platform_set_drvdata (pdev, resets);
drivers/reset/reset-tc9564.c line 171
Quoted 10 lines
drivers/reset/reset-tc9564.c new file +10
-0+ +return 0 ; +} + +static void tc9564_reset_remove (struct platform_device *pdev) +{ +struct tc9564_resets *resets =platform_get_drvdata (pdev); + +/* Leave all resets asserted when done */ +tc9564_reset_assert_all (resets);
Should this be done by the individual drivers?
What happens if the reset controller driver is unbound while e.g. one of the MACs is active?
regards
Philipp