[PATCH 0/4] clk: introduce TC9564 clock and reset

[PATCH 0/4] clk: introduce TC9564 clock and reset

[Alex Elder]

This series adds support for the clock and reset controller present
within the TC9564 SoC.

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 of these PCI functions provides access
to the registers that enable or disable a number of clock signals,
and assert or deassert a set of reset lines.

The TC9564 PCI functions provide PCI endpoint buses via devicetree.
This allows a PCI function to use devicetree to define subordinate
devices accessible using one or more of the function's BARs.  The
clock/reset driver binds with a device defined this way.  Only one
of the two PCI functions is permitted to define the clock/reset
device node.

					-Alex

Note: to work correctly, this series depends on this:
  https://lore.kernel.org/lkml/20260918152659.413268-1-elder@riscstar.com/


Alex Elder (4):
  dt-bindings: mfd: introduce the TC9564 config syscon
  dt-bindings: clock: introduce toshiba,tc9564-clock.yaml
  clk: toshiba: introduce a TC9564 SoC clock and reset driver
  arm64: dts: qcom: qcs6490-rb3gen2: add the clock controller

 .../bindings/clock/toshiba,tc9564-clock.yaml  |  64 +++
 .../bindings/mfd/toshiba,tc9564.yaml          |  56 +++
 MAINTAINERS                                   |   9 +
 arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts  |  22 +-
 drivers/clk/Kconfig                           |  11 +
 drivers/clk/Makefile                          |   1 +
 drivers/clk/clk-tc9564.c                      | 366 ++++++++++++++++++
 include/dt-bindings/clock/toshiba,tc9564.h    |  54 +++
 8 files changed, 582 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/clock/toshiba,tc9564-clock.yaml
 create mode 100644 Documentation/devicetree/bindings/mfd/toshiba,tc9564.yaml
 create mode 100644 drivers/clk/clk-tc9564.c
 create mode 100644 include/dt-bindings/clock/toshiba,tc9564.h


base-commit: 79d34e34e9ab629be19e588ec2eb7e1855d6dc46
-- 
2.53.0

[PATCH 1/4] dt-bindings: mfd: introduce the TC9564 config syscon

[Alex Elder]

Define the binding for a system controller used in the Toshiba
TC9564 SoC.

Co-developed-by: Daniel Thompson <daniel@riscstar.com>
Signed-off-by: Daniel Thompson <daniel@riscstar.com>
Signed-off-by: Alex Elder <elder@riscstar.com>
---
 .../bindings/mfd/toshiba,tc9564.yaml          | 56 +++++++++++++++++++
 MAINTAINERS                                   |  1 +
 2 files changed, 57 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/toshiba,tc9564.yaml

diff --git a/Documentation/devicetree/bindings/mfd/toshiba,tc9564.yaml b/Documentation/devicetree/bindings/mfd/toshiba,tc9564.yaml
new file mode 100644
index 0000000000000..32e73a727c82a
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/toshiba,tc9564.yaml
@@ -0,0 +1,56 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/toshiba,tc9564.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Toshiba TC9564 System Controller
+
+maintainers:
+  - Alex Elder <elder@riscstar.com>
+  - Daniel Thompson <daniel@riscstar.com>
+
+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 on one of its downstream ports that provides
+  access to various SoC peripherals (including a clock and reset controller)
+  via one of its BARs.  A system controller provides managed access to the
+  first two pages of this memory region to ensure accesses made by these
+  peripherals produce well-defined results.
+
+properties:
+  compatible:
+    items:
+      - const: toshiba,tc9564-config
+      - const: syscon
+      - const: simple-mfd
+
+  reg:
+    maxItems: 1
+
+  ranges: true
+
+  '#address-cells':
+    const: 1
+
+  '#size-cells':
+    const: 1
+
+required:
+  - compatible
+  - reg
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    syscon@0 {
+        compatible = "toshiba,tc9564-config",
+                     "syscon",
+                     "simple-mfd";
+        reg = <0x0 0x2000>;
+        #address-cells = <1>;
+        #size-cells = <1>;
+        ranges;
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index cf846853fdc79..b2d3257b8f892 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -27674,6 +27674,7 @@ TOSHIBA TC9564 PCI DRIVER
 M:	Alex Elder <elder@kernel.org>
 M:	Daniel Thompson <danielt@kernel.org>
 S:	Maintained
+F:	Documentation/devicetree/bindings/mfd/toshiba,tc9564.yaml
 F:	Documentation/devicetree/bindings/misc/pci1179,0220.yaml
 F:	drivers/misc/tc9564-pci.c
 
-- 
2.53.0

Re: [PATCH 1/4] dt-bindings: mfd: introduce the TC9564 config syscon

[Krzysztof Kozlowski]

On Fri, Sep 18, 2026 at 11:52:30AM -0500, Alex Elder wrote:
> Define the binding for a system controller used in the Toshiba
> TC9564 SoC.
> 
> Co-developed-by: Daniel Thompson <daniel@riscstar.com>
> Signed-off-by: Daniel Thompson <daniel@riscstar.com>
> Signed-off-by: Alex Elder <elder@riscstar.com>
> ---
>  .../bindings/mfd/toshiba,tc9564.yaml          | 56 +++++++++++++++++++
>  MAINTAINERS                                   |  1 +
>  2 files changed, 57 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/toshiba,tc9564.yaml
> 
> diff --git a/Documentation/devicetree/bindings/mfd/toshiba,tc9564.yaml b/Documentation/devicetree/bindings/mfd/toshiba,tc9564.yaml
> new file mode 100644
> index 0000000000000..32e73a727c82a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/toshiba,tc9564.yaml
> @@ -0,0 +1,56 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/toshiba,tc9564.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Toshiba TC9564 System Controller
> +
> +maintainers:
> +  - Alex Elder <elder@riscstar.com>
> +  - Daniel Thompson <daniel@riscstar.com>
> +
> +description: |

Do not need '|' unless you need to preserve formatting.

> +  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 on one of its downstream ports that provides
> +  access to various SoC peripherals (including a clock and reset controller)
> +  via one of its BARs.  A system controller provides managed access to the
> +  first two pages of this memory region to ensure accesses made by these
> +  peripherals produce well-defined results.
> +
> +properties:
> +  compatible:
> +    items:
> +      - const: toshiba,tc9564-config
> +      - const: syscon
> +      - const: simple-mfd
> +
> +  reg:
> +    maxItems: 1
> +
> +  ranges: true
> +
> +  '#address-cells':
> +    const: 1
> +
> +  '#size-cells':
> +    const: 1

Both properties and simple-mfd are redundant. You do not have children.

> +
> +required:
> +  - compatible
> +  - reg
> +
> +unevaluatedProperties: false

And this should be additionalProperties instead

> +
> +examples:
> +  - |
> +    syscon@0 {
> +        compatible = "toshiba,tc9564-config",
> +                     "syscon",
> +                     "simple-mfd";
> +        reg = <0x0 0x2000>;
> +        #address-cells = <1>;
> +        #size-cells = <1>;
> +        ranges;

As you can see here - no children.

I also have doubts that this is needed - I see no updates to the misc
binding, which would be referencing it. But then another point would be,
that you do not need separate child node, which has no properties.

Well, has one - address space.

Lack of full picture is not helping here.

Best regards,
Krzysztof

Re: [PATCH 1/4] dt-bindings: mfd: introduce the TC9564 config syscon

[Alex Elder]

On 9/20/26 1:18 PM, Krzysztof Kozlowski wrote:
> On Fri, Sep 18, 2026 at 11:52:30AM -0500, Alex Elder wrote:
>> Define the binding for a system controller used in the Toshiba
>> TC9564 SoC.
>>
>> Co-developed-by: Daniel Thompson <daniel@riscstar.com>
>> Signed-off-by: Daniel Thompson <daniel@riscstar.com>
>> Signed-off-by: Alex Elder <elder@riscstar.com>

Thank you for your feedback, Krzysztof.

>> ---
>>   .../bindings/mfd/toshiba,tc9564.yaml          | 56 +++++++++++++++++++
>>   MAINTAINERS                                   |  1 +
>>   2 files changed, 57 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/mfd/toshiba,tc9564.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/toshiba,tc9564.yaml b/Documentation/devicetree/bindings/mfd/toshiba,tc9564.yaml
>> new file mode 100644
>> index 0000000000000..32e73a727c82a
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mfd/toshiba,tc9564.yaml
>> @@ -0,0 +1,56 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/mfd/toshiba,tc9564.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Toshiba TC9564 System Controller
>> +
>> +maintainers:
>> +  - Alex Elder <elder@riscstar.com>
>> +  - Daniel Thompson <daniel@riscstar.com>
>> +
>> +description: |
> 
> Do not need '|' unless you need to preserve formatting.

OK.

>> +  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 on one of its downstream ports that provides
>> +  access to various SoC peripherals (including a clock and reset controller)
>> +  via one of its BARs.  A system controller provides managed access to the
>> +  first two pages of this memory region to ensure accesses made by these
>> +  peripherals produce well-defined results.
>> +
>> +properties:
>> +  compatible:
>> +    items:
>> +      - const: toshiba,tc9564-config
>> +      - const: syscon
>> +      - const: simple-mfd
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  ranges: true
>> +
>> +  '#address-cells':
>> +    const: 1
>> +
>> +  '#size-cells':
>> +    const: 1
> 
> Both properties and simple-mfd are redundant. You do not have children.

I think ranges, #address-cells, and #size-cells might have
been left here by mistake, and I hadn't noticed.  (They are
required to be included within a pci-ep-bus sub-node, but
even if that's why they're here, they're in the wrong place.)

I will remove these three properties, as well as the
"simple-mfd" compatible string in version 2.

>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +
>> +unevaluatedProperties: false
> 
> And this should be additionalProperties instead

OK.
>> +examples:
>> +  - |
>> +    syscon@0 {
>> +        compatible = "toshiba,tc9564-config",
>> +                     "syscon",
>> +                     "simple-mfd";
>> +        reg = <0x0 0x2000>;
>> +        #address-cells = <1>;
>> +        #size-cells = <1>;
>> +        ranges;
> 
> As you can see here - no children.

I have seen some examples of syscon nodes that incorporate
sub-devices, while others do not.

Our purpose for defining a syscon here is to coordinate
access for multiple devices to several registers located
within the same page of memory.

I think this device was originally defined in a sub-node
because the only memory accesses it requires are within
the range covered by the syscon.

Should we instead define the syscon to be a fairly trivial
standalone thing, and then refer to it in the clock/reset
device node by phandle?  (Or have the driver look it up
by compatible string?)

     tc9564_config_syscon0: syscon@0 {
         compatible = "toshiba,tc9564-config",
                      "syscon";
         reg = <0x0 0x2000>;
     };

     clock@1004 {
         compatible = "toshiba,tc9564-clock";
         toshiba,config-syscon = <&tc9564_config_syscon0 0x1004>;
         #clock-cells = <1>;
         #reset-cells = <1>;
     };
> I also have doubts that this is needed - I see no updates to the misc
> binding, which would be referencing it. But then another point would be,
> that you do not need separate child node, which has no properties.

I'm sorry if I'm missing something.  I saw other examples of
syscon nodes being defined, and tried to copy what I saw, but
obviously got it wrong.

> Well, has one - address space.
> 
> Lack of full picture is not helping here.

I will gladly provide a better picture, but I also want to
stay focused on what's necessary for the binding.

I have more in my next message.

					-Alex

> Best regards,
> Krzysztof

Re: [PATCH 1/4] dt-bindings: mfd: introduce the TC9564 config syscon

[Alex Elder]

On 9/21/26 4:46 PM, Alex Elder wrote:
>> I also have doubts that this is needed - I see no updates to the misc
>> binding, which would be referencing it. But then another point would be,
>> that you do not need separate child node, which has no properties.

Krzysztof, I've done some additional work on this based on
your feedback.

I think you are correct that there is no need to define
this as a special syscon with its own compatible string.

Instead, I can simply define a simple syscon with a label:

         tc9564_config_syscon0: syscon@0 {
                 compatible = "syscon";
                 reg = <0x0 0x2000>;
         };

And then have the devices that need to use it, refer to
its label using a phandle:

         tc9564_clock0: clock@1004 {
                 compatible = "toshiba,tc9564-clock";
                 toshiba,config-syscon = <&tc9564_config_syscon0>;
                 #clock-cells = <1>;
                 #reset-cells = <1>;
         };

         tc9564_emac0: ethernet@40000 {
                 compatible = "toshiba,tc9564-xgmac";
                 reg = <0x40000 0x8000>;
                 toshiba,config-syscon = <&tc9564_config_syscon0 0x1070>;
		. . .
	};

         tc9564_emac1: ethernet@48000 {
                 compatible = "toshiba,tc9564-xgmac";
                 reg = <0x48000 0x8000>;
                 toshiba,config-syscon = <&tc9564_config_syscon0 0x1074>;
		. . .
	};

That simplifies things a bit, and requires no devicetree binding.

If you think this is incorrect, please let me know, otherwise it
is the direction I plan to go with the next version of this
series.

Thank you again for your review.

					-Alex

Re: [PATCH 1/4] dt-bindings: mfd: introduce the TC9564 config syscon

[Krzysztof Kozlowski]

On 22/09/2026 23:02, Alex Elder wrote:
> On 9/21/26 4:46 PM, Alex Elder wrote:
>>> I also have doubts that this is needed - I see no updates to the misc
>>> binding, which would be referencing it. But then another point would be,
>>> that you do not need separate child node, which has no properties.
> 
> Krzysztof, I've done some additional work on this based on
> your feedback.
> 
> I think you are correct that there is no need to define
> this as a special syscon with its own compatible string.
> 
> Instead, I can simply define a simple syscon with a label:
> 
>          tc9564_config_syscon0: syscon@0 {
>                  compatible = "syscon";

I never proposed anything like that. I asked to drop simple-mfd.

>                  reg = <0x0 0x2000>;
>          };
> 
> And then have the devices that need to use it, refer to
> its label using a phandle:
> 
>          tc9564_clock0: clock@1004 {
>                  compatible = "toshiba,tc9564-clock";
>                  toshiba,config-syscon = <&tc9564_config_syscon0>;
>                  #clock-cells = <1>;
>                  #reset-cells = <1>;
>          };
> 
>          tc9564_emac0: ethernet@40000 {
>                  compatible = "toshiba,tc9564-xgmac";
>                  reg = <0x40000 0x8000>;
>                  toshiba,config-syscon = <&tc9564_config_syscon0 0x1070>;
> 		. . .
> 	};
> 
>          tc9564_emac1: ethernet@48000 {
>                  compatible = "toshiba,tc9564-xgmac";
>                  reg = <0x48000 0x8000>;
>                  toshiba,config-syscon = <&tc9564_config_syscon0 0x1074>;
> 		. . .
> 	};
> 
> That simplifies things a bit, and requires no devicetree binding.
> 
> If you think this is incorrect, please let me know, otherwise it
> is the direction I plan to go with the next version of this
> series.
> 

I think nothing behind PCI EP should be described in DT, because
everything is internal to the device and implied by compatible (or
vid/pid). We do describe internals of devices in DT usually for complex
devices (like some display blocks with multiple subblocks) or when they
are used outside of that device. That's why I asked how are all these
clocks and resets exposed to the host. If the syscon is used only by the
device itself, I don't quite get the benefits of describing it in DT.

Best regards,
Krzysztof

Re: [PATCH 1/4] dt-bindings: mfd: introduce the TC9564 config syscon

[Alex Elder]

On 9/23/26 2:25 AM, Krzysztof Kozlowski wrote:
> I think nothing behind PCI EP should be described in DT, because
> everything is internal to the device and implied by compatible (or
> vid/pid). We do describe internals of devices in DT usually for complex
> devices (like some display blocks with multiple subblocks) or when they
> are used outside of that device. That's why I asked how are all these
> clocks and resets exposed to the host. If the syscon is used only by the
> device itself, I don't quite get the benefits of describing it in DT.

You say you think "nothing behind PCI EP should be described in DT".
However the mere existence of the PCI endpoint bus as an option
contradicts that statement, by providing a means of doing *exactly*
that:  using devicetree to describe things behind a PCI endpoint.
This is the model we are using.

In June we switched over to using the pci-ep-bus model based on a
suggestion from Rob Herring (and Mani Sadhasivam).  Rob felt our
previous auxiliary device approach wasn't right for the peripherals
found within the SoC, given we were accessing them via PCI BARs.
   https://lore.kernel.org/lkml/20260605144032.GA3659201-robh@kernel.org/

We found that using pci-ep-bus provided a very natural and familiar
way of representing the organization of the TC9564 SoC, and used
it to represent all of the peripherals (including XGMAC).  It
nicely partitions the hardware design, as well as providing a
well-understood connection with the software.

The switch involved quite a bit of work, and the result is a set
of basically independent platform drivers that are modular and
easily reviewed.  I really would rather not go back to to what
we had before.

Do you have criteria for what in a chip/SoC is sufficiently
"complex" to warrant allowing devicetree to describe some of
its internal details?

I believe the TC9564 *is* such a "complex" SoC.  And in any case,
anywhere pci-ep-bus is used, it implies that some chip internals
would be described using devicetree.

Would incorporating the chip internals in a devicetree *overlay*
have any effect on your opinion on this?  I.e., the "main"
devicetree nodes would simply define the PCI device hierarchy,
but anything behind an endpoint would be described in an overlay.
This way, pci-ep-bus on a PCI endpoint just provides an external
interface onto which a very complex device could be "mounted"
(and described in an overlay).

I'd like to continue to move ahead with upstreaming support for
this SoC, but to me it seems you have basic doubts about
pci-ep-bus and I'd like to get past that.

Thanks.

					-Alex

[PATCH 2/4] dt-bindings: clock: introduce toshiba,tc9564-clock.yaml

[Alex Elder]

Define the binding for the clock controller functionality present in
the Toshiba TC9564 SoC.

Co-developed-by: Daniel Thompson <daniel@riscstar.com>
Signed-off-by: Daniel Thompson <daniel@riscstar.com>
Signed-off-by: Alex Elder <elder@riscstar.com>
---
 .../bindings/clock/toshiba,tc9564-clock.yaml  | 64 +++++++++++++++++++
 MAINTAINERS                                   |  7 ++
 include/dt-bindings/clock/toshiba,tc9564.h    | 54 ++++++++++++++++
 3 files changed, 125 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/toshiba,tc9564-clock.yaml
 create mode 100644 include/dt-bindings/clock/toshiba,tc9564.h

diff --git a/Documentation/devicetree/bindings/clock/toshiba,tc9564-clock.yaml b/Documentation/devicetree/bindings/clock/toshiba,tc9564-clock.yaml
new file mode 100644
index 0000000000000..cc7019bd240e4
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/toshiba,tc9564-clock.yaml
@@ -0,0 +1,64 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/toshiba,tc9564-clock.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Toshiba TC9564 Clock and Reset Controller
+
+maintainers:
+  - Alex Elder <elder@riscstar.com>
+  - Daniel Thompson <daniel@riscstar.com>
+
+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 clock/reset controller) via its BARs.
+
+  The SoC is represented using a devicetree PCI endpoint bus, so the
+  clock controller is bound to its driver using a platform driver
+  compatible string.  All of the clocks are simple gate clocks.
+
+  A total of 21 clocks are implemented, though two of these are not
+  controllable.  Access to the clock controller relies on PCIe being
+  functional, so the PCIe clock is assumed to be always on.  Similarly,
+  the PCIe controller relies on I2C, so the I2C clock is also assumed
+  to be always on.
+
+  In addition, 13 resets are implemented.  Similar to clocks, the PCIe
+  and I2C resets are assumed to be deasserted and cannot be controlled.
+
+  Clock and reset ids are defined in <dt-bindings/clock/toshiba,tc9564.h>.
+
+properties:
+  compatible:
+    const: toshiba,tc9564-clock
+
+  reg:
+    maxItems: 1
+
+  "#clock-cells":
+    const: 1
+
+  "#reset-cells":
+    const: 1
+
+required:
+  - compatible
+  - reg
+  - "#clock-cells"
+  - "#reset-cells"
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/toshiba,tc9564.h>
+
+    clock@1004 {
+        compatible = "toshiba,tc9564-clock";
+        reg = <0x1004 0x20>;
+        #clock-cells = <1>;
+        #reset-cells = <1>;
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index b2d3257b8f892..66d0e7e65adcb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -27670,6 +27670,13 @@ F:	Documentation/devicetree/bindings/media/i2c/toshiba,tc358743.txt
 F:	drivers/media/i2c/tc358743*
 F:	include/media/i2c/tc358743.h
 
+TOSHIBA TC9564 CLOCK DRIVER
+M:	Alex Elder <elder@kernel.org>
+M:	Daniel Thompson <danielt@kernel.org>
+S:	Maintained
+F:	Documentation/devicetree/bindings/clock/toshiba,tc9564-clock.yaml
+F:	include/dt-bindings/clock/toshiba,tc9564.h
+
 TOSHIBA TC9564 PCI DRIVER
 M:	Alex Elder <elder@kernel.org>
 M:	Daniel Thompson <danielt@kernel.org>
diff --git a/include/dt-bindings/clock/toshiba,tc9564.h b/include/dt-bindings/clock/toshiba,tc9564.h
new file mode 100644
index 0000000000000..75732fddd3025
--- /dev/null
+++ b/include/dt-bindings/clock/toshiba,tc9564.h
@@ -0,0 +1,54 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+
+/*
+ * Copyright (C) 2026 by RISCstar Solutions Corporation.  All rights reserved.
+ */
+
+#ifndef __ClOCK_TOSHIBA_TC9564_H__
+#define __ClOCK_TOSHIBA_TC9564_H__
+
+/* Clock IDs */
+
+#define CLOCK_MCU		0
+#define CLOCK_INTC		1
+/* #define CLOCK_PCIE		2 */
+/* #define CLOCK_I2C		3 */
+#define CLOCK_SRAM		4
+#define CLOCK_UART		5
+#define CLOCK_MSIGEN		6
+#define CLOCK_PLL		7
+#define CLOCK_SGMII		8
+#define CLOCK_REFCLKO		9
+
+#define CLOCK_MAC0_TX		10
+#define CLOCK_MAC0_RX		11
+#define CLOCK_MAC0_125M		12
+#define CLOCK_MAC0_312_5M	13
+#define CLOCK_MAC0_ALL		14
+
+#define CLOCK_MAC1_TX		15
+#define CLOCK_MAC1_RX		16
+#define CLOCK_MAC1_RMII		17
+#define CLOCK_MAC1_125M		18
+#define CLOCK_MAC1_312_5M	19
+#define CLOCK_MAC1_ALL		20
+
+/* Reset IDs */
+
+#define RESET_MCU		0
+#define RESET_MCU1		1
+#define RESET_MSIGEN		2
+#define RESET_INTC		3
+#define RESET_UART		4
+/* #define RESET_I2C		5 */
+/* #define RESET_PCIE		6 */
+
+#define RESET_MAC0_MAC		7
+#define RESET_MAC0_PMA		8
+#define RESET_MAC0_XPCS		9
+
+#define RESET_MAC1_MAC		10
+#define RESET_MAC1_PMA		11
+#define RESET_MAC1_XPCS		12
+
+#endif /* __ClOCK_TOSHIBA_TC9564_H__*/
-- 
2.53.0

Re: [PATCH 2/4] dt-bindings: clock: introduce toshiba,tc9564-clock.yaml

[Krzysztof Kozlowski]

On Fri, Sep 18, 2026 at 11:52:31AM -0500, Alex Elder wrote:
> +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 clock/reset controller) via its BARs.
> +
> +  The SoC is represented using a devicetree PCI endpoint bus, so the
> +  clock controller is bound to its driver using a platform driver

Do not mention drivers here. Just say that host can access clocks etc
via PCI.

But then more questions. This is PCI, yes? So how are the clocks from
PCI routed BACK to the main soc, which would justify having this in DTS
in the first place?


> +  compatible string.  All of the clocks are simple gate clocks.
> +
> +  A total of 21 clocks are implemented, though two of these are not
> +  controllable.  Access to the clock controller relies on PCIe being
> +  functional, so the PCIe clock is assumed to be always on.  Similarly,
> +  the PCIe controller relies on I2C, so the I2C clock is also assumed
> +  to be always on.
> +
> +  In addition, 13 resets are implemented.  Similar to clocks, the PCIe
> +  and I2C resets are assumed to be deasserted and cannot be controlled.
> +
> +  Clock and reset ids are defined in <dt-bindings/clock/toshiba,tc9564.h>.
> +
> +properties:
> +  compatible:
> +    const: toshiba,tc9564-clock
> +
> +  reg:
> +    maxItems: 1
> +
> +  "#clock-cells":
> +    const: 1
> +
> +  "#reset-cells":
> +    const: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - "#clock-cells"
> +  - "#reset-cells"
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/toshiba,tc9564.h>
> +
> +    clock@1004 {
> +        compatible = "toshiba,tc9564-clock";
> +        reg = <0x1004 0x20>;
> +        #clock-cells = <1>;
> +        #reset-cells = <1>;

No resources except small address space, so does not look like a
separate device and should be folded into the parent node.

> +    };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index b2d3257b8f892..66d0e7e65adcb 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -27670,6 +27670,13 @@ F:	Documentation/devicetree/bindings/media/i2c/toshiba,tc358743.txt
>  F:	drivers/media/i2c/tc358743*
>  F:	include/media/i2c/tc358743.h
>  
> +TOSHIBA TC9564 CLOCK DRIVER
> +M:	Alex Elder <elder@kernel.org>
> +M:	Daniel Thompson <danielt@kernel.org>
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/clock/toshiba,tc9564-clock.yaml
> +F:	include/dt-bindings/clock/toshiba,tc9564.h
> +
>  TOSHIBA TC9564 PCI DRIVER
>  M:	Alex Elder <elder@kernel.org>
>  M:	Daniel Thompson <danielt@kernel.org>
> diff --git a/include/dt-bindings/clock/toshiba,tc9564.h b/include/dt-bindings/clock/toshiba,tc9564.h
> new file mode 100644
> index 0000000000000..75732fddd3025
> --- /dev/null
> +++ b/include/dt-bindings/clock/toshiba,tc9564.h
> @@ -0,0 +1,54 @@
> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
> +
> +/*
> + * Copyright (C) 2026 by RISCstar Solutions Corporation.  All rights reserved.
> + */
> +
> +#ifndef __ClOCK_TOSHIBA_TC9564_H__
> +#define __ClOCK_TOSHIBA_TC9564_H__
> +
> +/* Clock IDs */
> +
> +#define CLOCK_MCU		0
> +#define CLOCK_INTC		1
> +/* #define CLOCK_PCIE		2 */
> +/* #define CLOCK_I2C		3 */

Drop both, and SRAM is number 2. Or enable and implement them.

Best regards,
Krzysztof

Re: [PATCH 2/4] dt-bindings: clock: introduce toshiba,tc9564-clock.yaml

[Alex Elder]

On 9/20/26 1:21 PM, Krzysztof Kozlowski wrote:
> On Fri, Sep 18, 2026 at 11:52:31AM -0500, Alex Elder wrote:
>> +description: |

I suppose I don't need the '|' here either.

>> +  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 clock/reset controller) via its BARs.
>> +
>> +  The SoC is represented using a devicetree PCI endpoint bus, so the
>> +  clock controller is bound to its driver using a platform driver
> 
> Do not mention drivers here. Just say that host can access clocks etc
> via PCI.

Daniel and I discussed this this morning, and in the next version
I'll explain things from a little different perspective, and I
think it will help keep this focused on the hardware.

This particular binding should not even mention anything about
PCI.  Using the PCI endpoint bus means that the devices can be
bound based on compatible string (i.e. with platform drivers),
with no knowledge of access being provided by PCI.

> But then more questions. This is PCI, yes? So how are the clocks from
> PCI routed BACK to the main soc, which would justify having this in DTS
> in the first place?

This clock+reset controller is one of several IP blocks
within an SoC (Toshiba TC9564).  Most of these clock and
reset signals control things internal to this SoC, but at
least one clock signal and a few reset lines are exposed
externally, allowing them to be used by the platform that
incorporates the SoC.

Ultimately the TC9564 SoC has a single 25 MHz input clock,
and internal to the SoC there is an internal PLL that then
drives other clocks at rates derived from that.  We can
only enable/disable the clocks (we have no rate control).


But I think you have more and deeper questions too.

We are using the *PCI endpoint bus* model to represent this
SoC.  To me it seems that there is not full agreement on
some aspects of this model.

It is a weird situation, where the use of this *software*
model provides a means of more completely describing (using
devicetree) details of the hardware "behind" a PCI endpoint.


Despite PCI doing its own enumeration, PCI_DYNAMIC_OF_NODES
allows the PCI hierarchy to *also* have devicetree nodes
representing bridges and endpoints.

Building on that, a PCI function with a devicetree node
can include a "pci-ep-bus" sub-node.  That sub-node
provides a place to describe IP blocks (devices) that
can be accessed via the endpoint bus.

In our case, in addition to this clock+reset device, the
TC9564 SoC implements a UART, a GPIO controller, a network
interface, and so on.  All of these are accessed via the
endpoint bus associated with the PCI function's BAR 4
address space.

So this model differs from "normal" PCI devices in that a
single PCI endpoint (function) can expose more than just
one capability (in contrast with, for example, a PCI
function that implements only an Ethernet interface).

>> +  compatible string.  All of the clocks are simple gate clocks.
>> +
>> +  A total of 21 clocks are implemented, though two of these are not
>> +  controllable.  Access to the clock controller relies on PCIe being
>> +  functional, so the PCIe clock is assumed to be always on.  Similarly,
>> +  the PCIe controller relies on I2C, so the I2C clock is also assumed
>> +  to be always on.
>> +
>> +  In addition, 13 resets are implemented.  Similar to clocks, the PCIe
>> +  and I2C resets are assumed to be deasserted and cannot be controlled.
>> +
>> +  Clock and reset ids are defined in <dt-bindings/clock/toshiba,tc9564.h>.
>> +
>> +properties:
>> +  compatible:
>> +    const: toshiba,tc9564-clock
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  "#clock-cells":
>> +    const: 1
>> +
>> +  "#reset-cells":
>> +    const: 1
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - "#clock-cells"
>> +  - "#reset-cells"
>> +
>> +unevaluatedProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/clock/toshiba,tc9564.h>
>> +
>> +    clock@1004 {
>> +        compatible = "toshiba,tc9564-clock";
>> +        reg = <0x1004 0x20>;
>> +        #clock-cells = <1>;
>> +        #reset-cells = <1>;
> 
> No resources except small address space, so does not look like a
> separate device and should be folded into the parent node.

Modeling the TC9564 as an SoC using pci-ep-bus, this
*is* sensibly a separate device.

We actually started with everything rolled into the
stmmac networking driver, but after some discussion
we concluded that the pci-ep-bus model would be simpler
(and other than some challenges related to it being a
relatively new thing, it *is* quite a bit simpler and
better partitioned).

					-Alex

> 
>> +    };
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index b2d3257b8f892..66d0e7e65adcb 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -27670,6 +27670,13 @@ F:	Documentation/devicetree/bindings/media/i2c/toshiba,tc358743.txt
>>   F:	drivers/media/i2c/tc358743*
>>   F:	include/media/i2c/tc358743.h
>>   
>> +TOSHIBA TC9564 CLOCK DRIVER
>> +M:	Alex Elder <elder@kernel.org>
>> +M:	Daniel Thompson <danielt@kernel.org>
>> +S:	Maintained
>> +F:	Documentation/devicetree/bindings/clock/toshiba,tc9564-clock.yaml
>> +F:	include/dt-bindings/clock/toshiba,tc9564.h
>> +
>>   TOSHIBA TC9564 PCI DRIVER
>>   M:	Alex Elder <elder@kernel.org>
>>   M:	Daniel Thompson <danielt@kernel.org>
>> diff --git a/include/dt-bindings/clock/toshiba,tc9564.h b/include/dt-bindings/clock/toshiba,tc9564.h
>> new file mode 100644
>> index 0000000000000..75732fddd3025
>> --- /dev/null
>> +++ b/include/dt-bindings/clock/toshiba,tc9564.h
>> @@ -0,0 +1,54 @@
>> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
>> +
>> +/*
>> + * Copyright (C) 2026 by RISCstar Solutions Corporation.  All rights reserved.
>> + */
>> +
>> +#ifndef __ClOCK_TOSHIBA_TC9564_H__
>> +#define __ClOCK_TOSHIBA_TC9564_H__
>> +
>> +/* Clock IDs */
>> +
>> +#define CLOCK_MCU		0
>> +#define CLOCK_INTC		1
>> +/* #define CLOCK_PCIE		2 */
>> +/* #define CLOCK_I2C		3 */
> 
> Drop both, and SRAM is number 2. Or enable and implement them.
> 
> Best regards,
> Krzysztof

Re: [PATCH 2/4] dt-bindings: clock: introduce toshiba,tc9564-clock.yaml

[Alex Elder]

On 9/20/26 1:21 PM, Krzysztof Kozlowski wrote:
>> diff --git a/include/dt-bindings/clock/toshiba,tc9564.h b/include/dt-bindings/clock/toshiba,tc9564.h
>> new file mode 100644
>> index 0000000000000..75732fddd3025
>> --- /dev/null
>> +++ b/include/dt-bindings/clock/toshiba,tc9564.h
>> @@ -0,0 +1,54 @@
>> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
>> +
>> +/*
>> + * Copyright (C) 2026 by RISCstar Solutions Corporation.  All rights reserved.
>> + */
>> +
>> +#ifndef __ClOCK_TOSHIBA_TC9564_H__
>> +#define __ClOCK_TOSHIBA_TC9564_H__
>> +
>> +/* Clock IDs */
>> +
>> +#define CLOCK_MCU		0
>> +#define CLOCK_INTC		1
>> +/* #define CLOCK_PCIE		2 */
>> +/* #define CLOCK_I2C		3 */
> Drop both, and SRAM is number 2. Or enable and implement them.

I forgot to respond to this.  I think you are confused.

SRAM is BAR number 2.  This is defining clock IDs.

And the reason these are commented out is that, while they
do exist, we explicitly do not want anyone to disable either
of these clocks because PCIe uses them.  This clock (and
reset) control is accessed via PCIe.

I do accept that someone might want me to simply omit
these commented-out definitions, and I'd be fine with
that.

But your suggestion wasn't sensible so I wanted to clear
that up before deleting these.

					-Alex

Re: [PATCH 2/4] dt-bindings: clock: introduce toshiba,tc9564-clock.yaml

[Krzysztof Kozlowski]

On 22/09/2026 15:13, Alex Elder wrote:
> On 9/20/26 1:21 PM, Krzysztof Kozlowski wrote:
>>> diff --git a/include/dt-bindings/clock/toshiba,tc9564.h b/include/dt-bindings/clock/toshiba,tc9564.h
>>> new file mode 100644
>>> index 0000000000000..75732fddd3025
>>> --- /dev/null
>>> +++ b/include/dt-bindings/clock/toshiba,tc9564.h
>>> @@ -0,0 +1,54 @@
>>> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
>>> +
>>> +/*
>>> + * Copyright (C) 2026 by RISCstar Solutions Corporation.  All rights reserved.
>>> + */
>>> +
>>> +#ifndef __ClOCK_TOSHIBA_TC9564_H__
>>> +#define __ClOCK_TOSHIBA_TC9564_H__
>>> +
>>> +/* Clock IDs */
>>> +
>>> +#define CLOCK_MCU		0
>>> +#define CLOCK_INTC		1
>>> +/* #define CLOCK_PCIE		2 */
>>> +/* #define CLOCK_I2C		3 */
>> Drop both, and SRAM is number 2. Or enable and implement them.
> 
> I forgot to respond to this.  I think you are confused.
> 
> SRAM is BAR number 2.  This is defining clock IDs.
> 
> And the reason these are commented out is that, while they
> do exist, we explicitly do not want anyone to disable either
> of these clocks because PCIe uses them.  This clock (and
> reset) control is accessed via PCIe.
> 
> I do accept that someone might want me to simply omit
> these commented-out definitions, and I'd be fine with
> that.
> 
> But your suggestion wasn't sensible so I wanted to clear
> that up before deleting these.
> 

Indices should be continuous, there is no point in having holes in
numbering.

Best regards,
Krzysztof

[PATCH 3/4] clk: toshiba: introduce a TC9564 SoC clock and reset driver

[Alex Elder]

Define a new platform driver that manages clock and reset signals
within the TC9564 SoC.  There are 21 clocks, which can only be
enabled and disabled, as well as 13 reset signals.

Two registers manage the state of the clocks and two others manage
the state of the resets.  The registers are accessed via a regmap
supplied by a system controller, which coordinates access to a
region of memory that will be shared with another driver.

Access to the memory region is provided via a BAR on a PCIe endpoint
function embedded in the TC9564 SoC.  For that reason, neither the
PCIe clock nor PCIe reset can be manipulated by this driver (they
are assumed always on and deasserted, respectively).

Similarly, control is not available for the I2C clock and reset,
because the PCIe subsystem on the TC9564 relies on I2C

Co-developed-by: Daniel Thompson <daniel@riscstar.com>
Signed-off-by: Daniel Thompson <daniel@riscstar.com>
Signed-off-by: Alex Elder <elder@riscstar.com>
---
 MAINTAINERS              |   1 +
 drivers/clk/Kconfig      |  11 ++
 drivers/clk/Makefile     |   1 +
 drivers/clk/clk-tc9564.c | 366 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 379 insertions(+)
 create mode 100644 drivers/clk/clk-tc9564.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 66d0e7e65adcb..39346a5cd9a7f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -27675,6 +27675,7 @@ M:	Alex Elder <elder@kernel.org>
 M:	Daniel Thompson <danielt@kernel.org>
 S:	Maintained
 F:	Documentation/devicetree/bindings/clock/toshiba,tc9564-clock.yaml
+F:	drivers/clk/clk-tc9564.c
 F:	include/dt-bindings/clock/toshiba,tc9564.h
 
 TOSHIBA TC9564 PCI DRIVER
diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index f9592fd9ec2bb..50efa10d48450 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -292,6 +292,17 @@ config COMMON_CLK_S2MPS11
 	  clock. These multi-function devices have two (S2MPS14) or three
 	  (S2MPS11, S5M8767) fixed-rate oscillators, clocked at 32KHz each.
 
+config COMMON_CLK_TC9564
+	tristate "Toshiba TC9564 clock support"
+	depends on TC9564_PCI
+	default m
+	help
+	  This enables support for the clock and reset controller embedded
+	  in the Toshiba TC9564 (and Qualcomm QPS615) SoC.  The state of
+	  clock and reset lines is controlled by MMIO to a region managed
+	  by a system controller; this ensures access to the region is
+	  coordinated between this and other drivers.
+
 config CLK_TWL
 	tristate "Clock driver for the TWL PMIC family"
 	depends on TWL4030_CORE
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index b18af485d7f03..a98ea8ff92066 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -110,6 +110,7 @@ obj-$(CONFIG_COMMON_CLK_SI570)		+= clk-si570.o
 obj-$(CONFIG_COMMON_CLK_SP7021)		+= clk-sp7021.o
 obj-$(CONFIG_COMMON_CLK_STM32F)		+= clk-stm32f4.o
 obj-$(CONFIG_COMMON_CLK_STM32H7)	+= clk-stm32h7.o
+obj-$(CONFIG_COMMON_CLK_TC9564)		+= clk-tc9564.o
 obj-$(CONFIG_COMMON_CLK_TPS68470)      += clk-tps68470.o
 obj-$(CONFIG_CLK_TWL6040)		+= clk-twl6040.o
 obj-$(CONFIG_CLK_TWL)			+= clk-twl.o
diff --git a/drivers/clk/clk-tc9564.c b/drivers/clk/clk-tc9564.c
new file mode 100644
index 0000000000000..4f900bc8994a7
--- /dev/null
+++ b/drivers/clk/clk-tc9564.c
@@ -0,0 +1,366 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+/*
+ * Copyright (C) 2026 by RISCstar Solutions Corporation.  All rights reserved.
+ */
+
+#include <linux/bits.h>
+#include <linux/clk-provider.h>
+#include <linux/mfd/syscon.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/reset-controller.h>
+
+#include <dt-bindings/clock/toshiba,tc9564.h>
+
+#define CLK_CTRL0_OFFSET	0x1004
+#define RST_CTRL0_OFFSET	0x1008
+#define CLK_CTRL1_OFFSET	0x100c
+#define RST_CTRL1_OFFSET	0x1010
+
+struct tc9564_clock_init {
+	const char *name;	/* NULL means unused entry */
+	u32 offset;
+	u32 mask;
+};
+
+struct tc9564_clock {
+	struct clk_hw hw;
+	u32 which;
+	u32 offset;		/* CLK_CTRL0_OFFSET or CLK_CTRL1_OFFSET */
+	u32 mask;		/* Zero means undefined clock */
+};
+
+struct tc9564_reset {
+	u32 offset;		/* RST_CTRL0_OFFSET or RST_CTRL1_OFFSET */
+	u32 mask;		/* Zero means undefined reset */
+};
+
+struct tc9564_clocks {
+	struct device *dev;
+	struct regmap *regmap;
+	struct reset_controller_dev rcdev;
+	size_t clock_count;
+	struct tc9564_clock clocks[] __counted_by(clock_count);
+};
+
+#define TC9564_CLOCK_INIT0(_name, _bit)	__TC9564_CLOCK_INIT(_name, 0, _bit)
+#define TC9564_CLOCK_INIT1(_name, _bit)	__TC9564_CLOCK_INIT(_name, 1, _bit)
+
+#define __TC9564_CLOCK_INIT(_name, _reg, _bit)		\
+	[CLOCK_##_name] = {				\
+		.name	= #_name,			\
+		.offset	= CLK_CTRL ## _reg ## _OFFSET,	\
+		.mask	= BIT(_bit),			\
+	}
+
+#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),			\
+	}
+
+static const struct tc9564_clock_init tc9564_clock_init[] = {
+	TC9564_CLOCK_INIT0(MCU, 0),
+	TC9564_CLOCK_INIT0(INTC, 4),
+	/* TC9564_CLOCK_INIT0(PCIE, 9), */
+	/* TC9564_CLOCK_INIT0(I2C, 12), */
+	TC9564_CLOCK_INIT0(SRAM, 13),
+	TC9564_CLOCK_INIT0(UART, 16),
+	TC9564_CLOCK_INIT0(MSIGEN, 18),
+	TC9564_CLOCK_INIT0(PLL, 24),
+	TC9564_CLOCK_INIT0(SGMII, 25),
+	TC9564_CLOCK_INIT0(REFCLKO, 26),
+
+	TC9564_CLOCK_INIT0(MAC0_TX, 7),
+	TC9564_CLOCK_INIT0(MAC0_RX, 14),
+	TC9564_CLOCK_INIT0(MAC0_125M, 29),
+	TC9564_CLOCK_INIT0(MAC0_312_5M, 30),
+	TC9564_CLOCK_INIT0(MAC0_ALL, 31),
+
+	TC9564_CLOCK_INIT1(MAC1_TX, 7),
+	TC9564_CLOCK_INIT1(MAC1_RX, 14),
+	TC9564_CLOCK_INIT1(MAC1_RMII, 15),
+	TC9564_CLOCK_INIT1(MAC1_125M, 29),
+	TC9564_CLOCK_INIT1(MAC1_312_5M, 30),
+	TC9564_CLOCK_INIT1(MAC1_ALL, 31),
+};
+#define TC9564_CLOCK_COUNT	ARRAY_SIZE(tc9564_clock_init)
+
+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_clock *hw_to_tc9564_clock(struct clk_hw *hw)
+{
+	return container_of_const(hw, struct tc9564_clock, hw);
+}
+
+static const struct tc9564_clocks *
+tc9564_clock_to_clocks(const struct tc9564_clock *clock)
+{
+	u32 which = clock->which;
+
+	if (which >= TC9564_CLOCK_COUNT)
+		return ERR_PTR(-ENXIO);
+
+	return container_of_const(clock, struct tc9564_clocks, clocks[which]);
+}
+
+static int tc9564_clk_manage(struct clk_hw *hw, bool enable)
+{
+	const struct tc9564_clock *clock = hw_to_tc9564_clock(hw);
+	const struct tc9564_clocks *clocks;
+	u32 offset = clock->offset;
+	u32 mask = clock->mask;
+
+	clocks = tc9564_clock_to_clocks(clock);
+	if (IS_ERR(clocks) || !mask) {
+		dev_err(clk_hw_get_dev(hw), "invalid clock (%s id %u)\n",
+			enable ? "enable" : "disable", clock->which);
+		return -ENXIO;
+	}
+
+	return regmap_update_bits(clocks->regmap, offset, mask,
+				  enable ? mask : 0);
+}
+
+static int tc9564_clk_enable(struct clk_hw *hw)
+{
+	return tc9564_clk_manage(hw, true);
+}
+
+static void tc9564_clk_disable(struct clk_hw *hw)
+{
+	(void)tc9564_clk_manage(hw, false);
+}
+
+static const struct clk_ops tc9564_clk_ops = {
+	.enable = tc9564_clk_enable,
+	.disable = tc9564_clk_disable,
+};
+
+static void tc9564_clock_disable_all(struct tc9564_clocks *clocks)
+{
+	for (int i = 0; i < clocks->clock_count; i++) {
+		const struct tc9564_clock *clock = &clocks->clocks[i];
+
+		if (clock->mask)
+			regmap_update_bits(clocks->regmap, clock->offset,
+					   clock->mask, 0);
+	}
+}
+
+static struct clk_hw *tc9564_clk_hw_get(struct of_phandle_args *clkspec,
+					void *data)
+{
+	struct tc9564_clocks *clocks = data;
+	unsigned int i = clkspec->args[0];
+
+	if (i < clocks->clock_count)
+		return &clocks->clocks[i].hw;
+
+	dev_err(clocks->dev, "invalid index %u\n", i);
+
+	return ERR_PTR(-EINVAL);
+}
+
+static struct tc9564_clocks *tc9564_clk_init(struct device *dev)
+{
+	struct tc9564_clocks *clocks;
+	size_t clocks_size;
+	int ret;
+
+	clocks_size = struct_size(clocks, clocks, TC9564_CLOCK_COUNT);
+	clocks = devm_kzalloc(dev, clocks_size, GFP_KERNEL);
+	if (!clocks)
+		return ERR_PTR(-ENOMEM);
+
+	clocks->dev = dev;
+	clocks->clock_count = TC9564_CLOCK_COUNT;
+
+	clocks->regmap = syscon_node_to_regmap(dev_of_node(dev->parent));
+	if (IS_ERR(clocks->regmap)) {
+		dev_err(dev, "failed to get config regmap\n");
+		return ERR_CAST(clocks->regmap);
+	}
+
+	for (u32 i = 0; i < TC9564_CLOCK_COUNT; i++) {
+		const struct tc9564_clock_init *clock_init;
+		struct clk_init_data init = { };
+		struct tc9564_clock *clock;
+
+		clock_init = &tc9564_clock_init[i];
+		if (!clock_init->name)
+			continue;
+
+		init.name = clock_init->name;
+		init.ops = &tc9564_clk_ops;
+
+		clock = &clocks->clocks[i];
+		clock->hw.init = &init;
+
+		ret = devm_clk_hw_register(dev, &clock->hw);
+		if (ret) {
+			dev_err(dev, "failed to register clock \"%s\"\n",
+				init.name);
+			return ERR_PTR(ret);
+		}
+
+		clock->which = i;
+		clock->offset = clock_init->offset;
+		clock->mask = clock_init->mask;
+	}
+
+	ret = devm_of_clk_add_hw_provider(dev, tc9564_clk_hw_get, clocks);
+	if (ret) {
+		dev_err(dev, "failed to add clock hardware provider\n");
+		return ERR_PTR(ret);
+	}
+
+	return clocks;
+}
+
+static const struct tc9564_clocks *
+rcdev_to_tc9564_clocks(struct reset_controller_dev *rcdev)
+{
+	return container_of_const(rcdev, struct tc9564_clocks, rcdev);
+}
+
+static int tc9564_reset_manage(struct reset_controller_dev *rcdev,
+			       unsigned long id, bool assert)
+{
+	const struct tc9564_clocks *clocks = rcdev_to_tc9564_clocks(rcdev);
+
+	if (id < rcdev->nr_resets) {
+		const struct tc9564_reset *reset = &tc9564_reset[id];
+		u32 mask = reset->mask;
+
+		if (mask)
+			return regmap_update_bits(clocks->regmap,
+						  reset->offset, mask,
+						  assert ? mask : 0);
+	}
+
+	dev_err(clocks->dev, "invalid reset (%sassert id %lu)\n",
+		assert ? "" : "de", id);
+
+	return -ENXIO;
+}
+
+static int tc9564_reset_assert(struct reset_controller_dev *rcdev,
+			       unsigned long id)
+{
+	return tc9564_reset_manage(rcdev, id, true);
+}
+
+static int tc9564_reset_deassert(struct reset_controller_dev *rcdev,
+				 unsigned long id)
+{
+	return tc9564_reset_manage(rcdev, id, false);
+}
+
+static const struct reset_control_ops tc9564_reset_control_ops = {
+	.assert		= tc9564_reset_assert,
+	.deassert	= tc9564_reset_deassert,
+};
+
+static void tc9564_reset_assert_all(struct tc9564_clocks *clocks)
+{
+	for (u32 id = 0; id < TC9564_RESET_COUNT; id++)
+		if (tc9564_reset[id].mask)
+			tc9564_reset_manage(&clocks->rcdev, id, true);
+}
+
+static int tc9564_reset_init(struct tc9564_clocks *clocks)
+{
+	struct reset_controller_dev *rcdev = &clocks->rcdev;
+
+	rcdev->ops = &tc9564_reset_control_ops;
+	rcdev->owner = THIS_MODULE;
+	rcdev->dev = clocks->dev;
+	rcdev->of_node = dev_of_node(clocks->dev);
+	rcdev->nr_resets = TC9564_RESET_COUNT;
+
+	return devm_reset_controller_register(clocks->dev, rcdev);
+}
+
+static int tc9564_clk_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct tc9564_clocks *clocks;
+	int ret;
+
+	if (!dev_of_node(dev))
+		return dev_err_probe(dev, -EINVAL, "no devicetree node\n");
+
+	clocks = tc9564_clk_init(dev);
+	if (IS_ERR(clocks))
+		return dev_err_probe(dev, PTR_ERR(clocks),
+				     "failed to initialize clocks\n");
+
+	ret = tc9564_reset_init(clocks);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to initialize resets\n");
+
+	/* Force all resets to be initially asserted */
+	tc9564_reset_assert_all(clocks);
+
+	/* Force all clocks to be initially disabled */
+	tc9564_clock_disable_all(clocks);
+
+	platform_set_drvdata(pdev, clocks);
+
+	return 0;
+}
+
+static void tc9564_clk_remove(struct platform_device *pdev)
+{
+	struct tc9564_clocks *clocks = platform_get_drvdata(pdev);
+
+	/* Leave all resets to be deasserted when done */
+	tc9564_reset_assert_all(clocks);
+
+	/* Leave all clocks disabled when done */
+	tc9564_clock_disable_all(clocks);
+}
+
+static const struct of_device_id tc9564_clk_ids[] = {
+	{ .compatible = "toshiba,tc9564-clock" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, tc9564_clk_ids);
+
+static struct platform_driver tc9564_clk_driver = {
+	.probe	= tc9564_clk_probe,
+	.remove	= tc9564_clk_remove,
+	.driver	= {
+		.name		= KBUILD_MODNAME,
+		.of_match_table = tc9564_clk_ids,
+		.probe_type	= PROBE_PREFER_ASYNCHRONOUS,
+	},
+};
+module_platform_driver(tc9564_clk_driver);
+
+MODULE_DESCRIPTION("Toshiba TC9564 Clock and Reset Driver");
+MODULE_LICENSE("GPL");
-- 
2.53.0

Re: [PATCH 3/4] clk: toshiba: introduce a TC9564 SoC clock and reset driver

[Uwe Kleine-König]

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

Hello Alex,

On Fri, Sep 18, 2026 at 11:52:32AM -0500, Alex Elder wrote:
> +#include <linux/bits.h>
> +#include <linux/clk-provider.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/reset-controller.h>

Please rely on linux/platform_device.h to provide of_device_id and drop
including <linux/mod_devicetable.h>. (If you want to go full iwyu,
include <linux/device-id/of.h> instead.)

> +static const struct of_device_id tc9564_clk_ids[] = {
> +	{ .compatible = "toshiba,tc9564-clock" },
> +	{ },
> +};

Drop the trailing comma after the list terminator please.

Best regards
Uwe

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

Re: [PATCH 3/4] clk: toshiba: introduce a TC9564 SoC clock and reset driver

[Alex Elder]

On 9/20/26 2:39 PM, Uwe Kleine-König wrote:
> Hello Alex,
> 
> On Fri, Sep 18, 2026 at 11:52:32AM -0500, Alex Elder wrote:
>> +#include <linux/bits.h>
>> +#include <linux/clk-provider.h>
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/mod_devicetable.h>
>> +#include <linux/module.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/regmap.h>
>> +#include <linux/reset-controller.h>
> 
> Please rely on linux/platform_device.h to provide of_device_id and drop
> including <linux/mod_devicetable.h>. (If you want to go full iwyu,
> include <linux/device-id/of.h> instead.)

In general I would like to "go full iwyu" but have never heard
it expressed that way.  Can you recommend a tool available that
will help me do that?

It looks like "mod_devicetable.h" doesn't help much unless I
need struct_cpu_feature (which I do not), so I'll include "of.h".

>> +static const struct of_device_id tc9564_clk_ids[] = {
>> +	{ .compatible = "toshiba,tc9564-clock" },
>> +	{ },
>> +};
> 
> Drop the trailing comma after the list terminator please.

OK.  Only for the list terminator, right, because nothing
will ever follow it?  Non-empty final initializer is OK
to have a comma as far as you're concerned?

Thanks for your review.

					-Alex

> Best regards
> Uwe

Re: [PATCH 3/4] clk: toshiba: introduce a TC9564 SoC clock and reset driver

[Uwe Kleine-König]

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

Hello Alex,

On Tue, Sep 22, 2026 at 07:37:53AM -0500, Alex Elder wrote:
> On 9/20/26 2:39 PM, Uwe Kleine-König wrote:
> > On Fri, Sep 18, 2026 at 11:52:32AM -0500, Alex Elder wrote:
> > > +#include <linux/bits.h>
> > > +#include <linux/clk-provider.h>
> > > +#include <linux/mfd/syscon.h>
> > > +#include <linux/mod_devicetable.h>
> > > +#include <linux/module.h>
> > > +#include <linux/platform_device.h>
> > > +#include <linux/regmap.h>
> > > +#include <linux/reset-controller.h>
> > 
> > Please rely on linux/platform_device.h to provide of_device_id and drop
> > including <linux/mod_devicetable.h>. (If you want to go full iwyu,
> > include <linux/device-id/of.h> instead.)
> 
> In general I would like to "go full iwyu" but have never heard
> it expressed that way.  Can you recommend a tool available that
> will help me do that?

There is `apt install iwyu`, but I only tried that shortly and didn't
manage to make it work nicely with-in the kernel.

> It looks like "mod_devicetable.h" doesn't help much unless I
> need struct_cpu_feature (which I do not), so I'll include "of.h".

Yeah, and for struct cpu_feature there is
https://lore.kernel.org/all/ef103a4b9e1cd7477fa52ee347c4ec301354fba9.1789988540.git.u.kleine-koenig@baylibre.com/
.
 
> > > +static const struct of_device_id tc9564_clk_ids[] = {
> > > +	{ .compatible = "toshiba,tc9564-clock" },
> > > +	{ },
> > > +};
> > 
> > Drop the trailing comma after the list terminator please.
> 
> OK.  Only for the list terminator, right, because nothing
> will ever follow it?  Non-empty final initializer is OK
> to have a comma as far as you're concerned?

correct. A non-empty last item even should have a comma.

Best regards
Uwe

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

Re: [PATCH 3/4] clk: toshiba: introduce a TC9564 SoC clock and reset driver

[Brian Masney]

Hi Alex,

On Fri, Sep 18, 2026 at 11:52:32AM -0500, Alex Elder wrote:
> Define a new platform driver that manages clock and reset signals
> within the TC9564 SoC.  There are 21 clocks, which can only be
> enabled and disabled, as well as 13 reset signals.
> 
> Two registers manage the state of the clocks and two others manage
> the state of the resets.  The registers are accessed via a regmap
> supplied by a system controller, which coordinates access to a
> region of memory that will be shared with another driver.
> 
> Access to the memory region is provided via a BAR on a PCIe endpoint
> function embedded in the TC9564 SoC.  For that reason, neither the
> PCIe clock nor PCIe reset can be manipulated by this driver (they
> are assumed always on and deasserted, respectively).
> 
> Similarly, control is not available for the I2C clock and reset,
> because the PCIe subsystem on the TC9564 relies on I2C
> 
> Co-developed-by: Daniel Thompson <daniel@riscstar.com>
> Signed-off-by: Daniel Thompson <daniel@riscstar.com>
> Signed-off-by: Alex Elder <elder@riscstar.com>
> ---
>  MAINTAINERS              |   1 +
>  drivers/clk/Kconfig      |  11 ++
>  drivers/clk/Makefile     |   1 +
>  drivers/clk/clk-tc9564.c | 366 +++++++++++++++++++++++++++++++++++++++
>  4 files changed, 379 insertions(+)
>  create mode 100644 drivers/clk/clk-tc9564.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 66d0e7e65adcb..39346a5cd9a7f 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -27675,6 +27675,7 @@ M:	Alex Elder <elder@kernel.org>
>  M:	Daniel Thompson <danielt@kernel.org>
>  S:	Maintained
>  F:	Documentation/devicetree/bindings/clock/toshiba,tc9564-clock.yaml
> +F:	drivers/clk/clk-tc9564.c
>  F:	include/dt-bindings/clock/toshiba,tc9564.h
>  
>  TOSHIBA TC9564 PCI DRIVER
> diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
> index f9592fd9ec2bb..50efa10d48450 100644
> --- a/drivers/clk/Kconfig
> +++ b/drivers/clk/Kconfig
> @@ -292,6 +292,17 @@ config COMMON_CLK_S2MPS11
>  	  clock. These multi-function devices have two (S2MPS14) or three
>  	  (S2MPS11, S5M8767) fixed-rate oscillators, clocked at 32KHz each.
>  
> +config COMMON_CLK_TC9564
> +	tristate "Toshiba TC9564 clock support"
> +	depends on TC9564_PCI

select RESET_CONTROLLER

> +	default m

default n

> +	help
> +	  This enables support for the clock and reset controller embedded
> +	  in the Toshiba TC9564 (and Qualcomm QPS615) SoC.  The state of
> +	  clock and reset lines is controlled by MMIO to a region managed
> +	  by a system controller; this ensures access to the region is
> +	  coordinated between this and other drivers.

Rather than 'other drivers', outline specifically which other drivers.

> +
>  config CLK_TWL
>  	tristate "Clock driver for the TWL PMIC family"
>  	depends on TWL4030_CORE
> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
> index b18af485d7f03..a98ea8ff92066 100644
> --- a/drivers/clk/Makefile
> +++ b/drivers/clk/Makefile
> @@ -110,6 +110,7 @@ obj-$(CONFIG_COMMON_CLK_SI570)		+= clk-si570.o
>  obj-$(CONFIG_COMMON_CLK_SP7021)		+= clk-sp7021.o
>  obj-$(CONFIG_COMMON_CLK_STM32F)		+= clk-stm32f4.o
>  obj-$(CONFIG_COMMON_CLK_STM32H7)	+= clk-stm32h7.o
> +obj-$(CONFIG_COMMON_CLK_TC9564)		+= clk-tc9564.o
>  obj-$(CONFIG_COMMON_CLK_TPS68470)      += clk-tps68470.o
>  obj-$(CONFIG_CLK_TWL6040)		+= clk-twl6040.o
>  obj-$(CONFIG_CLK_TWL)			+= clk-twl.o
> diff --git a/drivers/clk/clk-tc9564.c b/drivers/clk/clk-tc9564.c
> new file mode 100644
> index 0000000000000..4f900bc8994a7
> --- /dev/null
> +++ b/drivers/clk/clk-tc9564.c
> @@ -0,0 +1,366 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +/*
> + * Copyright (C) 2026 by RISCstar Solutions Corporation.  All rights reserved.
> + */
> +
> +#include <linux/bits.h>
> +#include <linux/clk-provider.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/mod_devicetable.h>

Uwe already pointed out removing this.

> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/reset-controller.h>
> +
> +#include <dt-bindings/clock/toshiba,tc9564.h>
> +
> +#define CLK_CTRL0_OFFSET	0x1004
> +#define RST_CTRL0_OFFSET	0x1008
> +#define CLK_CTRL1_OFFSET	0x100c
> +#define RST_CTRL1_OFFSET	0x1010
> +
> +struct tc9564_clock_init {
> +	const char *name;	/* NULL means unused entry */
> +	u32 offset;
> +	u32 mask;
> +};
> +
> +struct tc9564_clock {
> +	struct clk_hw hw;
> +	u32 which;
> +	u32 offset;		/* CLK_CTRL0_OFFSET or CLK_CTRL1_OFFSET */
> +	u32 mask;		/* Zero means undefined clock */
> +};
> +
> +struct tc9564_reset {
> +	u32 offset;		/* RST_CTRL0_OFFSET or RST_CTRL1_OFFSET */
> +	u32 mask;		/* Zero means undefined reset */
> +};
> +
> +struct tc9564_clocks {
> +	struct device *dev;
> +	struct regmap *regmap;
> +	struct reset_controller_dev rcdev;
> +	size_t clock_count;
> +	struct tc9564_clock clocks[] __counted_by(clock_count);
> +};
> +
> +#define TC9564_CLOCK_INIT0(_name, _bit)	__TC9564_CLOCK_INIT(_name, 0, _bit)
> +#define TC9564_CLOCK_INIT1(_name, _bit)	__TC9564_CLOCK_INIT(_name, 1, _bit)
> +
> +#define __TC9564_CLOCK_INIT(_name, _reg, _bit)		\
> +	[CLOCK_##_name] = {				\
> +		.name	= #_name,			\
> +		.offset	= CLK_CTRL ## _reg ## _OFFSET,	\
> +		.mask	= BIT(_bit),			\
> +	}
> +
> +#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),			\
> +	}
> +
> +static const struct tc9564_clock_init tc9564_clock_init[] = {
> +	TC9564_CLOCK_INIT0(MCU, 0),
> +	TC9564_CLOCK_INIT0(INTC, 4),
> +	/* TC9564_CLOCK_INIT0(PCIE, 9), */
> +	/* TC9564_CLOCK_INIT0(I2C, 12), */

A comment would be useful here to outline why these are commented out.
Is the PCIE one comment out because of what's outlined in the commit
message?

> +	TC9564_CLOCK_INIT0(SRAM, 13),
> +	TC9564_CLOCK_INIT0(UART, 16),
> +	TC9564_CLOCK_INIT0(MSIGEN, 18),
> +	TC9564_CLOCK_INIT0(PLL, 24),
> +	TC9564_CLOCK_INIT0(SGMII, 25),
> +	TC9564_CLOCK_INIT0(REFCLKO, 26),
> +
> +	TC9564_CLOCK_INIT0(MAC0_TX, 7),
> +	TC9564_CLOCK_INIT0(MAC0_RX, 14),
> +	TC9564_CLOCK_INIT0(MAC0_125M, 29),
> +	TC9564_CLOCK_INIT0(MAC0_312_5M, 30),
> +	TC9564_CLOCK_INIT0(MAC0_ALL, 31),
> +
> +	TC9564_CLOCK_INIT1(MAC1_TX, 7),
> +	TC9564_CLOCK_INIT1(MAC1_RX, 14),
> +	TC9564_CLOCK_INIT1(MAC1_RMII, 15),
> +	TC9564_CLOCK_INIT1(MAC1_125M, 29),
> +	TC9564_CLOCK_INIT1(MAC1_312_5M, 30),
> +	TC9564_CLOCK_INIT1(MAC1_ALL, 31),
> +};
> +#define TC9564_CLOCK_COUNT	ARRAY_SIZE(tc9564_clock_init)
> +
> +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_clock *hw_to_tc9564_clock(struct clk_hw *hw)
> +{
> +	return container_of_const(hw, struct tc9564_clock, hw);
> +}
> +
> +static const struct tc9564_clocks *
> +tc9564_clock_to_clocks(const struct tc9564_clock *clock)
> +{
> +	u32 which = clock->which;
> +
> +	if (which >= TC9564_CLOCK_COUNT)
> +		return ERR_PTR(-ENXIO);
> +
> +	return container_of_const(clock, struct tc9564_clocks, clocks[which]);
> +}
> +
> +static int tc9564_clk_manage(struct clk_hw *hw, bool enable)
> +{
> +	const struct tc9564_clock *clock = hw_to_tc9564_clock(hw);
> +	const struct tc9564_clocks *clocks;
> +	u32 offset = clock->offset;
> +	u32 mask = clock->mask;
> +
> +	clocks = tc9564_clock_to_clocks(clock);
> +	if (IS_ERR(clocks) || !mask) {
> +		dev_err(clk_hw_get_dev(hw), "invalid clock (%s id %u)\n",
> +			enable ? "enable" : "disable", clock->which);
> +		return -ENXIO;
> +	}
> +
> +	return regmap_update_bits(clocks->regmap, offset, mask,
> +				  enable ? mask : 0);
> +}
> +
> +static int tc9564_clk_enable(struct clk_hw *hw)
> +{
> +	return tc9564_clk_manage(hw, true);
> +}
> +
> +static void tc9564_clk_disable(struct clk_hw *hw)
> +{
> +	(void)tc9564_clk_manage(hw, false);
> +}
> +
> +static const struct clk_ops tc9564_clk_ops = {
> +	.enable = tc9564_clk_enable,
> +	.disable = tc9564_clk_disable,
> +};
> +
> +static void tc9564_clock_disable_all(struct tc9564_clocks *clocks)
> +{
> +	for (int i = 0; i < clocks->clock_count; i++) {
> +		const struct tc9564_clock *clock = &clocks->clocks[i];
> +
> +		if (clock->mask)
> +			regmap_update_bits(clocks->regmap, clock->offset,
> +					   clock->mask, 0);
> +	}
> +}
> +
> +static struct clk_hw *tc9564_clk_hw_get(struct of_phandle_args *clkspec,
> +					void *data)
> +{
> +	struct tc9564_clocks *clocks = data;
> +	unsigned int i = clkspec->args[0];
> +
> +	if (i < clocks->clock_count)
> +		return &clocks->clocks[i].hw;
> +
> +	dev_err(clocks->dev, "invalid index %u\n", i);
> +
> +	return ERR_PTR(-EINVAL);
> +}
> +
> +static struct tc9564_clocks *tc9564_clk_init(struct device *dev)
> +{
> +	struct tc9564_clocks *clocks;
> +	size_t clocks_size;
> +	int ret;
> +
> +	clocks_size = struct_size(clocks, clocks, TC9564_CLOCK_COUNT);
> +	clocks = devm_kzalloc(dev, clocks_size, GFP_KERNEL);
> +	if (!clocks)
> +		return ERR_PTR(-ENOMEM);
> +
> +	clocks->dev = dev;
> +	clocks->clock_count = TC9564_CLOCK_COUNT;
> +
> +	clocks->regmap = syscon_node_to_regmap(dev_of_node(dev->parent));
> +	if (IS_ERR(clocks->regmap)) {
> +		dev_err(dev, "failed to get config regmap\n");
> +		return ERR_CAST(clocks->regmap);
> +	}
> +
> +	for (u32 i = 0; i < TC9564_CLOCK_COUNT; i++) {
> +		const struct tc9564_clock_init *clock_init;
> +		struct clk_init_data init = { };
> +		struct tc9564_clock *clock;
> +
> +		clock_init = &tc9564_clock_init[i];
> +		if (!clock_init->name)
> +			continue;
> +
> +		init.name = clock_init->name;
> +		init.ops = &tc9564_clk_ops;
> +
> +		clock = &clocks->clocks[i];
> +		clock->hw.init = &init;
> +
> +		ret = devm_clk_hw_register(dev, &clock->hw);
> +		if (ret) {
> +			dev_err(dev, "failed to register clock \"%s\"\n",
> +				init.name);
> +			return ERR_PTR(ret);
> +		}
> +
> +		clock->which = i;
> +		clock->offset = clock_init->offset;
> +		clock->mask = clock_init->mask;
> +	}
> +
> +	ret = devm_of_clk_add_hw_provider(dev, tc9564_clk_hw_get, clocks);
> +	if (ret) {
> +		dev_err(dev, "failed to add clock hardware provider\n");
> +		return ERR_PTR(ret);
> +	}
> +
> +	return clocks;
> +}
> +
> +static const struct tc9564_clocks *
> +rcdev_to_tc9564_clocks(struct reset_controller_dev *rcdev)
> +{
> +	return container_of_const(rcdev, struct tc9564_clocks, rcdev);
> +}
> +
> +static int tc9564_reset_manage(struct reset_controller_dev *rcdev,
> +			       unsigned long id, bool assert)
> +{
> +	const struct tc9564_clocks *clocks = rcdev_to_tc9564_clocks(rcdev);
> +
> +	if (id < rcdev->nr_resets) {
> +		const struct tc9564_reset *reset = &tc9564_reset[id];
> +		u32 mask = reset->mask;
> +
> +		if (mask)
> +			return regmap_update_bits(clocks->regmap,
> +						  reset->offset, mask,
> +						  assert ? mask : 0);
> +	}
> +
> +	dev_err(clocks->dev, "invalid reset (%sassert id %lu)\n",
> +		assert ? "" : "de", id);
> +
> +	return -ENXIO;
> +}
> +
> +static int tc9564_reset_assert(struct reset_controller_dev *rcdev,
> +			       unsigned long id)
> +{
> +	return tc9564_reset_manage(rcdev, id, true);
> +}
> +
> +static int tc9564_reset_deassert(struct reset_controller_dev *rcdev,
> +				 unsigned long id)
> +{
> +	return tc9564_reset_manage(rcdev, id, false);
> +}
> +
> +static const struct reset_control_ops tc9564_reset_control_ops = {
> +	.assert		= tc9564_reset_assert,
> +	.deassert	= tc9564_reset_deassert,

I'd rather not have the extra spaces because sometimes these get out of
line with new members that need more space. Also tc9564_clk_ops above
doesn't have the extra spaces.

> +};
> +
> +static void tc9564_reset_assert_all(struct tc9564_clocks *clocks)
> +{
> +	for (u32 id = 0; id < TC9564_RESET_COUNT; id++)
> +		if (tc9564_reset[id].mask)
> +			tc9564_reset_manage(&clocks->rcdev, id, true);
> +}
> +
> +static int tc9564_reset_init(struct tc9564_clocks *clocks)
> +{
> +	struct reset_controller_dev *rcdev = &clocks->rcdev;
> +
> +	rcdev->ops = &tc9564_reset_control_ops;
> +	rcdev->owner = THIS_MODULE;
> +	rcdev->dev = clocks->dev;
> +	rcdev->of_node = dev_of_node(clocks->dev);
> +	rcdev->nr_resets = TC9564_RESET_COUNT;
> +
> +	return devm_reset_controller_register(clocks->dev, rcdev);
> +}
> +
> +static int tc9564_clk_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct tc9564_clocks *clocks;
> +	int ret;
> +
> +	if (!dev_of_node(dev))
> +		return dev_err_probe(dev, -EINVAL, "no devicetree node\n");
> +
> +	clocks = tc9564_clk_init(dev);
> +	if (IS_ERR(clocks))
> +		return dev_err_probe(dev, PTR_ERR(clocks),
> +				     "failed to initialize clocks\n");
> +
> +	ret = tc9564_reset_init(clocks);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to initialize resets\n");
> +
> +	/* Force all resets to be initially asserted */
> +	tc9564_reset_assert_all(clocks);
> +
> +	/* Force all clocks to be initially disabled */
> +	tc9564_clock_disable_all(clocks);
> +
> +	platform_set_drvdata(pdev, clocks);
> +
> +	return 0;
> +}
> +
> +static void tc9564_clk_remove(struct platform_device *pdev)
> +{
> +	struct tc9564_clocks *clocks = platform_get_drvdata(pdev);
> +
> +	/* Leave all resets to be deasserted when done */
> +	tc9564_reset_assert_all(clocks);

The comment says deasserted but the code calls assert_all.

Brian


> +
> +	/* Leave all clocks disabled when done */
> +	tc9564_clock_disable_all(clocks);
> +}
> +
> +static const struct of_device_id tc9564_clk_ids[] = {
> +	{ .compatible = "toshiba,tc9564-clock" },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, tc9564_clk_ids);
> +
> +static struct platform_driver tc9564_clk_driver = {
> +	.probe	= tc9564_clk_probe,
> +	.remove	= tc9564_clk_remove,
> +	.driver	= {
> +		.name		= KBUILD_MODNAME,
> +		.of_match_table = tc9564_clk_ids,
> +		.probe_type	= PROBE_PREFER_ASYNCHRONOUS,
> +	},
> +};
> +module_platform_driver(tc9564_clk_driver);
> +
> +MODULE_DESCRIPTION("Toshiba TC9564 Clock and Reset Driver");
> +MODULE_LICENSE("GPL");
> -- 
> 2.53.0
> 

Re: [PATCH 3/4] clk: toshiba: introduce a TC9564 SoC clock and reset driver

[Alex Elder]

On 9/21/26 5:59 PM, Brian Masney wrote:
> Hi Alex,
> 
> On Fri, Sep 18, 2026 at 11:52:32AM -0500, Alex Elder wrote:
>> Define a new platform driver that manages clock and reset signals
>> within the TC9564 SoC.  There are 21 clocks, which can only be
>> enabled and disabled, as well as 13 reset signals.
>>
>> Two registers manage the state of the clocks and two others manage
>> the state of the resets.  The registers are accessed via a regmap
>> supplied by a system controller, which coordinates access to a
>> region of memory that will be shared with another driver.
>>
>> Access to the memory region is provided via a BAR on a PCIe endpoint
>> function embedded in the TC9564 SoC.  For that reason, neither the
>> PCIe clock nor PCIe reset can be manipulated by this driver (they
>> are assumed always on and deasserted, respectively).
>>
>> Similarly, control is not available for the I2C clock and reset,
>> because the PCIe subsystem on the TC9564 relies on I2C
>>
>> Co-developed-by: Daniel Thompson <daniel@riscstar.com>
>> Signed-off-by: Daniel Thompson <daniel@riscstar.com>
>> Signed-off-by: Alex Elder <elder@riscstar.com>
>> ---
>>   MAINTAINERS              |   1 +
>>   drivers/clk/Kconfig      |  11 ++
>>   drivers/clk/Makefile     |   1 +
>>   drivers/clk/clk-tc9564.c | 366 +++++++++++++++++++++++++++++++++++++++
>>   4 files changed, 379 insertions(+)
>>   create mode 100644 drivers/clk/clk-tc9564.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 66d0e7e65adcb..39346a5cd9a7f 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -27675,6 +27675,7 @@ M:	Alex Elder <elder@kernel.org>
>>   M:	Daniel Thompson <danielt@kernel.org>
>>   S:	Maintained
>>   F:	Documentation/devicetree/bindings/clock/toshiba,tc9564-clock.yaml
>> +F:	drivers/clk/clk-tc9564.c
>>   F:	include/dt-bindings/clock/toshiba,tc9564.h
>>   
>>   TOSHIBA TC9564 PCI DRIVER
>> diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
>> index f9592fd9ec2bb..50efa10d48450 100644
>> --- a/drivers/clk/Kconfig
>> +++ b/drivers/clk/Kconfig
>> @@ -292,6 +292,17 @@ config COMMON_CLK_S2MPS11
>>   	  clock. These multi-function devices have two (S2MPS14) or three
>>   	  (S2MPS11, S5M8767) fixed-rate oscillators, clocked at 32KHz each.
>>   
>> +config COMMON_CLK_TC9564
>> +	tristate "Toshiba TC9564 clock support"
>> +	depends on TC9564_PCI
> 
> select RESET_CONTROLLER

Thank you.  The reset and clock drivers were previously separate
and the reset only became available if RESET_CONTROLLER was enabled.
Combining them means I need this.  I will add it.

I also need to select MFD_SYSCON to get syscon_node_to_regmap()
(or perhaps something similar, depending on the answers to my
questions about the proper way to define the syscon).
> 
>> +	default m
> 
> default n

This depends on TC9564_PCI, and if TC9564_PCI is enabled I would
like this to (automatically) be available (at least as a module).

Why do you recommend "n"?

>> +	help
>> +	  This enables support for the clock and reset controller embedded
>> +	  in the Toshiba TC9564 (and Qualcomm QPS615) SoC.  The state of
>> +	  clock and reset lines is controlled by MMIO to a region managed
>> +	  by a system controller; this ensures access to the region is
>> +	  coordinated between this and other drivers.
> 
> Rather than 'other drivers', outline specifically which other drivers.

I was intentionally vague at this point because the one other driver
has not yet gone out for upstream review for this iteration of the
code.  But I do agree with you, so I'll change this to say:

    ... between this and the XGMAC (stmmac) driver.

Then it will be correct without modification once that driver goes
out for review.  Is that better, or do you suggest something else?

>> +
>>   config CLK_TWL
>>   	tristate "Clock driver for the TWL PMIC family"
>>   	depends on TWL4030_CORE
>> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
>> index b18af485d7f03..a98ea8ff92066 100644
>> --- a/drivers/clk/Makefile
>> +++ b/drivers/clk/Makefile
>> @@ -110,6 +110,7 @@ obj-$(CONFIG_COMMON_CLK_SI570)		+= clk-si570.o
>>   obj-$(CONFIG_COMMON_CLK_SP7021)		+= clk-sp7021.o
>>   obj-$(CONFIG_COMMON_CLK_STM32F)		+= clk-stm32f4.o
>>   obj-$(CONFIG_COMMON_CLK_STM32H7)	+= clk-stm32h7.o
>> +obj-$(CONFIG_COMMON_CLK_TC9564)		+= clk-tc9564.o
>>   obj-$(CONFIG_COMMON_CLK_TPS68470)      += clk-tps68470.o
>>   obj-$(CONFIG_CLK_TWL6040)		+= clk-twl6040.o
>>   obj-$(CONFIG_CLK_TWL)			+= clk-twl.o
>> diff --git a/drivers/clk/clk-tc9564.c b/drivers/clk/clk-tc9564.c
>> new file mode 100644
>> index 0000000000000..4f900bc8994a7
>> --- /dev/null
>> +++ b/drivers/clk/clk-tc9564.c
>> @@ -0,0 +1,366 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +
>> +/*
>> + * Copyright (C) 2026 by RISCstar Solutions Corporation.  All rights reserved.
>> + */
>> +
>> +#include <linux/bits.h>
>> +#include <linux/clk-provider.h>
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/mod_devicetable.h>
> 
> Uwe already pointed out removing this.

Yes, done.

> 
>> +#include <linux/module.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/regmap.h>
>> +#include <linux/reset-controller.h>
>> +
>> +#include <dt-bindings/clock/toshiba,tc9564.h>
>> +
>> +#define CLK_CTRL0_OFFSET	0x1004
>> +#define RST_CTRL0_OFFSET	0x1008
>> +#define CLK_CTRL1_OFFSET	0x100c
>> +#define RST_CTRL1_OFFSET	0x1010
>> +
>> +struct tc9564_clock_init {
>> +	const char *name;	/* NULL means unused entry */
>> +	u32 offset;
>> +	u32 mask;
>> +};
>> +
>> +struct tc9564_clock {
>> +	struct clk_hw hw;
>> +	u32 which;
>> +	u32 offset;		/* CLK_CTRL0_OFFSET or CLK_CTRL1_OFFSET */
>> +	u32 mask;		/* Zero means undefined clock */
>> +};
>> +
>> +struct tc9564_reset {
>> +	u32 offset;		/* RST_CTRL0_OFFSET or RST_CTRL1_OFFSET */
>> +	u32 mask;		/* Zero means undefined reset */
>> +};
>> +
>> +struct tc9564_clocks {
>> +	struct device *dev;
>> +	struct regmap *regmap;
>> +	struct reset_controller_dev rcdev;
>> +	size_t clock_count;
>> +	struct tc9564_clock clocks[] __counted_by(clock_count);
>> +};
>> +
>> +#define TC9564_CLOCK_INIT0(_name, _bit)	__TC9564_CLOCK_INIT(_name, 0, _bit)
>> +#define TC9564_CLOCK_INIT1(_name, _bit)	__TC9564_CLOCK_INIT(_name, 1, _bit)
>> +
>> +#define __TC9564_CLOCK_INIT(_name, _reg, _bit)		\
>> +	[CLOCK_##_name] = {				\
>> +		.name	= #_name,			\
>> +		.offset	= CLK_CTRL ## _reg ## _OFFSET,	\
>> +		.mask	= BIT(_bit),			\
>> +	}
>> +
>> +#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),			\
>> +	}
>> +
>> +static const struct tc9564_clock_init tc9564_clock_init[] = {
>> +	TC9564_CLOCK_INIT0(MCU, 0),
>> +	TC9564_CLOCK_INIT0(INTC, 4),
>> +	/* TC9564_CLOCK_INIT0(PCIE, 9), */
>> +	/* TC9564_CLOCK_INIT0(I2C, 12), */
> 
> A comment would be useful here to outline why these are commented out.
> Is the PCIE one comment out because of what's outlined in the commit
> message?

Yes, that is why.  We are downstream of PCIe, and PCIe in this
case depends on I2C.  So we don't want to mess with these two
clocks or their reset counterparts.

I only provide this for the benefit of documentation.  If someone
happens to see bit 9 set in this register, it means the PCIe clock
is enabled.

Anyway, others have suggested simply removing these comments.
So I'll do what you suggest, but I'm interested to know whether
you would favor just deleting them instead.
> 
>> +	TC9564_CLOCK_INIT0(SRAM, 13),
>> +	TC9564_CLOCK_INIT0(UART, 16),
>> +	TC9564_CLOCK_INIT0(MSIGEN, 18),
>> +	TC9564_CLOCK_INIT0(PLL, 24),
>> +	TC9564_CLOCK_INIT0(SGMII, 25),
>> +	TC9564_CLOCK_INIT0(REFCLKO, 26),

. . .

>> +static const struct reset_control_ops tc9564_reset_control_ops = {
>> +	.assert		= tc9564_reset_assert,
>> +	.deassert	= tc9564_reset_deassert,
> 
> I'd rather not have the extra spaces because sometimes these get out of
> line with new members that need more space. Also tc9564_clk_ops above
> doesn't have the extra spaces.

That's OK with me.  I normally align assignments like this for
readability, but it's obviously an arbitrary stylistic choice.
I'll just use one space, no alignment (just as the clk_ops do
above).  I'll do the same in the platform driver structure
assignments.
> 
>> +};
>> +
>> +static void tc9564_reset_assert_all(struct tc9564_clocks *clocks)
>> +{
>> +	for (u32 id = 0; id < TC9564_RESET_COUNT; id++)
>> +		if (tc9564_reset[id].mask)
>> +			tc9564_reset_manage(&clocks->rcdev, id, true);
>> +}
>> +
>> +static int tc9564_reset_init(struct tc9564_clocks *clocks)
>> +{
>> +	struct reset_controller_dev *rcdev = &clocks->rcdev;
>> +
>> +	rcdev->ops = &tc9564_reset_control_ops;
>> +	rcdev->owner = THIS_MODULE;
>> +	rcdev->dev = clocks->dev;
>> +	rcdev->of_node = dev_of_node(clocks->dev);
>> +	rcdev->nr_resets = TC9564_RESET_COUNT;
>> +
>> +	return devm_reset_controller_register(clocks->dev, rcdev);
>> +}
>> +
>> +static int tc9564_clk_probe(struct platform_device *pdev)
>> +{
>> +	struct device *dev = &pdev->dev;
>> +	struct tc9564_clocks *clocks;
>> +	int ret;
>> +
>> +	if (!dev_of_node(dev))
>> +		return dev_err_probe(dev, -EINVAL, "no devicetree node\n");
>> +
>> +	clocks = tc9564_clk_init(dev);
>> +	if (IS_ERR(clocks))
>> +		return dev_err_probe(dev, PTR_ERR(clocks),
>> +				     "failed to initialize clocks\n");
>> +
>> +	ret = tc9564_reset_init(clocks);
>> +	if (ret)
>> +		return dev_err_probe(dev, ret, "failed to initialize resets\n");
>> +
>> +	/* Force all resets to be initially asserted */
>> +	tc9564_reset_assert_all(clocks);
>> +
>> +	/* Force all clocks to be initially disabled */
>> +	tc9564_clock_disable_all(clocks);
>> +
>> +	platform_set_drvdata(pdev, clocks);
>> +
>> +	return 0;
>> +}
>> +
>> +static void tc9564_clk_remove(struct platform_device *pdev)
>> +{
>> +	struct tc9564_clocks *clocks = platform_get_drvdata(pdev);
>> +
>> +	/* Leave all resets to be deasserted when done */
>> +	tc9564_reset_assert_all(clocks);
> 
> The comment says deasserted but the code calls assert_all.

Oops.  Will fix.  Thank you very much for your review Brian.

					-Alex

> 
> Brian
> 
> 
>> +
>> +	/* Leave all clocks disabled when done */
>> +	tc9564_clock_disable_all(clocks);
>> +}
>> +
>> +static const struct of_device_id tc9564_clk_ids[] = {
>> +	{ .compatible = "toshiba,tc9564-clock" },
>> +	{ },
>> +};
>> +MODULE_DEVICE_TABLE(of, tc9564_clk_ids);
>> +
>> +static struct platform_driver tc9564_clk_driver = {
>> +	.probe	= tc9564_clk_probe,
>> +	.remove	= tc9564_clk_remove,
>> +	.driver	= {
>> +		.name		= KBUILD_MODNAME,
>> +		.of_match_table = tc9564_clk_ids,
>> +		.probe_type	= PROBE_PREFER_ASYNCHRONOUS,
>> +	},
>> +};
>> +module_platform_driver(tc9564_clk_driver);
>> +
>> +MODULE_DESCRIPTION("Toshiba TC9564 Clock and Reset Driver");
>> +MODULE_LICENSE("GPL");
>> -- 
>> 2.53.0
>>
> 

Re: [PATCH 3/4] clk: toshiba: introduce a TC9564 SoC clock and reset driver

[Jerome Brunet]

On mar. 22 sept. 2026 at 08:33, Alex Elder <elder@riscstar.com> wrote:

> On 9/21/26 5:59 PM, Brian Masney wrote:
>> Hi Alex,
>> 
>> On Fri, Sep 18, 2026 at 11:52:32AM -0500, Alex Elder wrote:
>>> Define a new platform driver that manages clock and reset signals
>>> within the TC9564 SoC.  There are 21 clocks, which can only be
>>> enabled and disabled, as well as 13 reset signals.
>>>
>>> Two registers manage the state of the clocks and two others manage
>>> the state of the resets.  The registers are accessed via a regmap
>>> supplied by a system controller, which coordinates access to a
>>> region of memory that will be shared with another driver.
>>>
>>> Access to the memory region is provided via a BAR on a PCIe endpoint
>>> function embedded in the TC9564 SoC.  For that reason, neither the
>>> PCIe clock nor PCIe reset can be manipulated by this driver (they
>>> are assumed always on and deasserted, respectively).
>>>
>>> Similarly, control is not available for the I2C clock and reset,
>>> because the PCIe subsystem on the TC9564 relies on I2C
>>>
>>> Co-developed-by: Daniel Thompson <daniel@riscstar.com>
>>> Signed-off-by: Daniel Thompson <daniel@riscstar.com>
>>> Signed-off-by: Alex Elder <elder@riscstar.com>
>>> ---
>>>   MAINTAINERS              |   1 +
>>>   drivers/clk/Kconfig      |  11 ++
>>>   drivers/clk/Makefile     |   1 +
>>>   drivers/clk/clk-tc9564.c | 366 +++++++++++++++++++++++++++++++++++++++
>>>   4 files changed, 379 insertions(+)
>>>   create mode 100644 drivers/clk/clk-tc9564.c
>>>
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index 66d0e7e65adcb..39346a5cd9a7f 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -27675,6 +27675,7 @@ M:	Alex Elder <elder@kernel.org>
>>>   M:	Daniel Thompson <danielt@kernel.org>
>>>   S:	Maintained
>>>   F:	Documentation/devicetree/bindings/clock/toshiba,tc9564-clock.yaml
>>> +F:	drivers/clk/clk-tc9564.c
>>>   F:	include/dt-bindings/clock/toshiba,tc9564.h
>>>   
>>>   TOSHIBA TC9564 PCI DRIVER
>>> diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
>>> index f9592fd9ec2bb..50efa10d48450 100644
>>> --- a/drivers/clk/Kconfig
>>> +++ b/drivers/clk/Kconfig
>>> @@ -292,6 +292,17 @@ config COMMON_CLK_S2MPS11
>>>   	  clock. These multi-function devices have two (S2MPS14) or three
>>>   	  (S2MPS11, S5M8767) fixed-rate oscillators, clocked at 32KHz each.
>>>   
>>> +config COMMON_CLK_TC9564
>>> +	tristate "Toshiba TC9564 clock support"
>>> +	depends on TC9564_PCI
>> 
>> select RESET_CONTROLLER
>
> Thank you.  The reset and clock drivers were previously separate
> and the reset only became available if RESET_CONTROLLER was enabled.
> Combining them means I need this.  I will add it.

Why did you combine them ?

it would be a lot better if the reset were handled in
drivers/reset rather than in clock. There has already been some work to
move reset from clock back to reset. This often involve auxiliary drivers.

>
> I also need to select MFD_SYSCON to get syscon_node_to_regmap()
> (or perhaps something similar, depending on the answers to my
> questions about the proper way to define the syscon).
>> 
>>> +	default m
>> 
>> default n
>
> This depends on TC9564_PCI, and if TC9564_PCI is enabled I would
> like this to (automatically) be available (at least as a module).
>
> Why do you recommend "n"?
>
>>> +	help
>>> +	  This enables support for the clock and reset controller embedded
>>> +	  in the Toshiba TC9564 (and Qualcomm QPS615) SoC.  The state of
>>> +	  clock and reset lines is controlled by MMIO to a region managed
>>> +	  by a system controller; this ensures access to the region is
>>> +	  coordinated between this and other drivers.
>> 
>> Rather than 'other drivers', outline specifically which other drivers.
>
> I was intentionally vague at this point because the one other driver
> has not yet gone out for upstream review for this iteration of the
> code.  But I do agree with you, so I'll change this to say:
>
>     ... between this and the XGMAC (stmmac) driver.
>
> Then it will be correct without modification once that driver goes
> out for review.  Is that better, or do you suggest something else?
>
>>> +
>>>   config CLK_TWL
>>>   	tristate "Clock driver for the TWL PMIC family"
>>>   	depends on TWL4030_CORE
>>> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
>>> index b18af485d7f03..a98ea8ff92066 100644
>>> --- a/drivers/clk/Makefile
>>> +++ b/drivers/clk/Makefile
>>> @@ -110,6 +110,7 @@ obj-$(CONFIG_COMMON_CLK_SI570)		+= clk-si570.o
>>>   obj-$(CONFIG_COMMON_CLK_SP7021)		+= clk-sp7021.o
>>>   obj-$(CONFIG_COMMON_CLK_STM32F)		+= clk-stm32f4.o
>>>   obj-$(CONFIG_COMMON_CLK_STM32H7)	+= clk-stm32h7.o
>>> +obj-$(CONFIG_COMMON_CLK_TC9564)		+= clk-tc9564.o
>>>   obj-$(CONFIG_COMMON_CLK_TPS68470)      += clk-tps68470.o
>>>   obj-$(CONFIG_CLK_TWL6040)		+= clk-twl6040.o
>>>   obj-$(CONFIG_CLK_TWL)			+= clk-twl.o
>>> diff --git a/drivers/clk/clk-tc9564.c b/drivers/clk/clk-tc9564.c
>>> new file mode 100644
>>> index 0000000000000..4f900bc8994a7
>>> --- /dev/null
>>> +++ b/drivers/clk/clk-tc9564.c
>>> @@ -0,0 +1,366 @@
>>> +// SPDX-License-Identifier: GPL-2.0-only
>>> +
>>> +/*
>>> + * Copyright (C) 2026 by RISCstar Solutions Corporation.  All rights reserved.
>>> + */
>>> +
>>> +#include <linux/bits.h>
>>> +#include <linux/clk-provider.h>
>>> +#include <linux/mfd/syscon.h>
>>> +#include <linux/mod_devicetable.h>
>> 
>> Uwe already pointed out removing this.
>
> Yes, done.
>
>> 
>>> +#include <linux/module.h>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/regmap.h>
>>> +#include <linux/reset-controller.h>
>>> +
>>> +#include <dt-bindings/clock/toshiba,tc9564.h>
>>> +
>>> +#define CLK_CTRL0_OFFSET	0x1004
>>> +#define RST_CTRL0_OFFSET	0x1008
>>> +#define CLK_CTRL1_OFFSET	0x100c
>>> +#define RST_CTRL1_OFFSET	0x1010
>>> +
>>> +struct tc9564_clock_init {
>>> +	const char *name;	/* NULL means unused entry */
>>> +	u32 offset;
>>> +	u32 mask;
>>> +};
>>> +
>>> +struct tc9564_clock {
>>> +	struct clk_hw hw;
>>> +	u32 which;
>>> +	u32 offset;		/* CLK_CTRL0_OFFSET or CLK_CTRL1_OFFSET */
>>> +	u32 mask;		/* Zero means undefined clock */
>>> +};
>>> +
>>> +struct tc9564_reset {
>>> +	u32 offset;		/* RST_CTRL0_OFFSET or RST_CTRL1_OFFSET */
>>> +	u32 mask;		/* Zero means undefined reset */
>>> +};
>>> +
>>> +struct tc9564_clocks {
>>> +	struct device *dev;
>>> +	struct regmap *regmap;
>>> +	struct reset_controller_dev rcdev;
>>> +	size_t clock_count;
>>> +	struct tc9564_clock clocks[] __counted_by(clock_count);
>>> +};
>>> +
>>> +#define TC9564_CLOCK_INIT0(_name, _bit)	__TC9564_CLOCK_INIT(_name, 0, _bit)
>>> +#define TC9564_CLOCK_INIT1(_name, _bit)	__TC9564_CLOCK_INIT(_name, 1, _bit)
>>> +
>>> +#define __TC9564_CLOCK_INIT(_name, _reg, _bit)		\
>>> +	[CLOCK_##_name] = {				\
>>> +		.name	= #_name,			\
>>> +		.offset	= CLK_CTRL ## _reg ## _OFFSET,	\
>>> +		.mask	= BIT(_bit),			\
>>> +	}
>>> +
>>> +#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),			\
>>> +	}
>>> +
>>> +static const struct tc9564_clock_init tc9564_clock_init[] = {
>>> +	TC9564_CLOCK_INIT0(MCU, 0),
>>> +	TC9564_CLOCK_INIT0(INTC, 4),
>>> +	/* TC9564_CLOCK_INIT0(PCIE, 9), */
>>> +	/* TC9564_CLOCK_INIT0(I2C, 12), */
>> 
>> A comment would be useful here to outline why these are commented out.
>> Is the PCIE one comment out because of what's outlined in the commit
>> message?
>
> Yes, that is why.  We are downstream of PCIe, and PCIe in this
> case depends on I2C.  So we don't want to mess with these two
> clocks or their reset counterparts.
>
> I only provide this for the benefit of documentation.  If someone
> happens to see bit 9 set in this register, it means the PCIe clock
> is enabled.
>
> Anyway, others have suggested simply removing these comments.
> So I'll do what you suggest, but I'm interested to know whether
> you would favor just deleting them instead.
>> 
>>> +	TC9564_CLOCK_INIT0(SRAM, 13),
>>> +	TC9564_CLOCK_INIT0(UART, 16),
>>> +	TC9564_CLOCK_INIT0(MSIGEN, 18),
>>> +	TC9564_CLOCK_INIT0(PLL, 24),
>>> +	TC9564_CLOCK_INIT0(SGMII, 25),
>>> +	TC9564_CLOCK_INIT0(REFCLKO, 26),
>
> . . .
>
>>> +static const struct reset_control_ops tc9564_reset_control_ops = {
>>> +	.assert		= tc9564_reset_assert,
>>> +	.deassert	= tc9564_reset_deassert,
>> 
>> I'd rather not have the extra spaces because sometimes these get out of
>> line with new members that need more space. Also tc9564_clk_ops above
>> doesn't have the extra spaces.
>
> That's OK with me.  I normally align assignments like this for
> readability, but it's obviously an arbitrary stylistic choice.
> I'll just use one space, no alignment (just as the clk_ops do
> above).  I'll do the same in the platform driver structure
> assignments.
>> 
>>> +};
>>> +
>>> +static void tc9564_reset_assert_all(struct tc9564_clocks *clocks)
>>> +{
>>> +	for (u32 id = 0; id < TC9564_RESET_COUNT; id++)
>>> +		if (tc9564_reset[id].mask)
>>> +			tc9564_reset_manage(&clocks->rcdev, id, true);
>>> +}
>>> +
>>> +static int tc9564_reset_init(struct tc9564_clocks *clocks)
>>> +{
>>> +	struct reset_controller_dev *rcdev = &clocks->rcdev;
>>> +
>>> +	rcdev->ops = &tc9564_reset_control_ops;
>>> +	rcdev->owner = THIS_MODULE;
>>> +	rcdev->dev = clocks->dev;
>>> +	rcdev->of_node = dev_of_node(clocks->dev);
>>> +	rcdev->nr_resets = TC9564_RESET_COUNT;
>>> +
>>> +	return devm_reset_controller_register(clocks->dev, rcdev);
>>> +}
>>> +
>>> +static int tc9564_clk_probe(struct platform_device *pdev)
>>> +{
>>> +	struct device *dev = &pdev->dev;
>>> +	struct tc9564_clocks *clocks;
>>> +	int ret;
>>> +
>>> +	if (!dev_of_node(dev))
>>> +		return dev_err_probe(dev, -EINVAL, "no devicetree node\n");
>>> +
>>> +	clocks = tc9564_clk_init(dev);
>>> +	if (IS_ERR(clocks))
>>> +		return dev_err_probe(dev, PTR_ERR(clocks),
>>> +				     "failed to initialize clocks\n");
>>> +
>>> +	ret = tc9564_reset_init(clocks);
>>> +	if (ret)
>>> +		return dev_err_probe(dev, ret, "failed to initialize resets\n");
>>> +
>>> +	/* Force all resets to be initially asserted */
>>> +	tc9564_reset_assert_all(clocks);
>>> +
>>> +	/* Force all clocks to be initially disabled */
>>> +	tc9564_clock_disable_all(clocks);
>>> +
>>> +	platform_set_drvdata(pdev, clocks);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static void tc9564_clk_remove(struct platform_device *pdev)
>>> +{
>>> +	struct tc9564_clocks *clocks = platform_get_drvdata(pdev);
>>> +
>>> +	/* Leave all resets to be deasserted when done */
>>> +	tc9564_reset_assert_all(clocks);
>> 
>> The comment says deasserted but the code calls assert_all.
>
> Oops.  Will fix.  Thank you very much for your review Brian.
>
> 					-Alex
>
>> 
>> Brian
>> 
>> 
>>> +
>>> +	/* Leave all clocks disabled when done */
>>> +	tc9564_clock_disable_all(clocks);
>>> +}
>>> +
>>> +static const struct of_device_id tc9564_clk_ids[] = {
>>> +	{ .compatible = "toshiba,tc9564-clock" },
>>> +	{ },
>>> +};
>>> +MODULE_DEVICE_TABLE(of, tc9564_clk_ids);
>>> +
>>> +static struct platform_driver tc9564_clk_driver = {
>>> +	.probe	= tc9564_clk_probe,
>>> +	.remove	= tc9564_clk_remove,
>>> +	.driver	= {
>>> +		.name		= KBUILD_MODNAME,
>>> +		.of_match_table = tc9564_clk_ids,
>>> +		.probe_type	= PROBE_PREFER_ASYNCHRONOUS,
>>> +	},
>>> +};
>>> +module_platform_driver(tc9564_clk_driver);
>>> +
>>> +MODULE_DESCRIPTION("Toshiba TC9564 Clock and Reset Driver");
>>> +MODULE_LICENSE("GPL");
>>> -- 
>>> 2.53.0
>>>
>> 
>

-- 
Jerome

Re: [PATCH 3/4] clk: toshiba: introduce a TC9564 SoC clock and reset driver

[Alex Elder]

On 9/23/26 3:58 PM, Jerome Brunet wrote:
>>>> diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
>>>> index f9592fd9ec2bb..50efa10d48450 100644
>>>> --- a/drivers/clk/Kconfig
>>>> +++ b/drivers/clk/Kconfig
>>>> @@ -292,6 +292,17 @@ config COMMON_CLK_S2MPS11
>>>>    	  clock. These multi-function devices have two (S2MPS14) or three
>>>>    	  (S2MPS11, S5M8767) fixed-rate oscillators, clocked at 32KHz each.
>>>>    
>>>> +config COMMON_CLK_TC9564
>>>> +	tristate "Toshiba TC9564 clock support"
>>>> +	depends on TC9564_PCI
>>> select RESET_CONTROLLER
>> Thank you.  The reset and clock drivers were previously separate
>> and the reset only became available if RESET_CONTROLLER was enabled.
>> Combining them means I need this.  I will add it.
> Why did you combine them ?
> 
> it would be a lot better if the reset were handled in
> drivers/reset rather than in clock. There has already been some work to
> move reset from clock back to reset. This often involve auxiliary drivers.
To be honest, I got a suggestion from someone to combine them, but
I said "no, I got feedback last year that separating them was
better."  But then a second person told me that these are often
combined, so I took that as "two against one" and decided to
merge them.

The other reason is that four registers are used:
   #define CLK_CTRL0_OFFSET	0x1004
   #define RST_CTRL0_OFFSET	0x1008
   #define CLK_CTRL1_OFFSET	0x100c
   define RST_CTRL1_OFFSET	0x1010

They alternate between clock and reset.  The previous separated
drivers specified the two offsets for each driver in the reg
property.  But somehow these interleaved registers seemed a
little off, so I thought the combined driver cleaned that up
a little.

I don't have any strong preference one way or another.  If you
think they're better separate, I don't mind separating them
again.  The two drivers are extremely simple.

					-Alex

Re: [PATCH 3/4] clk: toshiba: introduce a TC9564 SoC clock and reset driver

[Jerome Brunet]

On mer. 23 sept. 2026 at 16:33, Alex Elder <elder@riscstar.com> wrote:

> On 9/23/26 3:58 PM, Jerome Brunet wrote:
>>>>> diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
>>>>> index f9592fd9ec2bb..50efa10d48450 100644
>>>>> --- a/drivers/clk/Kconfig
>>>>> +++ b/drivers/clk/Kconfig
>>>>> @@ -292,6 +292,17 @@ config COMMON_CLK_S2MPS11
>>>>>    	  clock. These multi-function devices have two (S2MPS14) or three
>>>>>    	  (S2MPS11, S5M8767) fixed-rate oscillators, clocked at 32KHz each.
>>>>>    
>>>>> +config COMMON_CLK_TC9564
>>>>> +	tristate "Toshiba TC9564 clock support"
>>>>> +	depends on TC9564_PCI
>>>> select RESET_CONTROLLER
>>> Thank you.  The reset and clock drivers were previously separate
>>> and the reset only became available if RESET_CONTROLLER was enabled.
>>> Combining them means I need this.  I will add it.
>> Why did you combine them ?
>> 
>> it would be a lot better if the reset were handled in
>> drivers/reset rather than in clock. There has already been some work to
>> move reset from clock back to reset. This often involve auxiliary drivers.
> To be honest, I got a suggestion from someone to combine them, but
> I said "no, I got feedback last year that separating them was
> better."  But then a second person told me that these are often
> combined, so I took that as "two against one" and decided to
> merge them.
>
> The other reason is that four registers are used:
>    #define CLK_CTRL0_OFFSET	0x1004
>    #define RST_CTRL0_OFFSET	0x1008
>    #define CLK_CTRL1_OFFSET	0x100c
>    define RST_CTRL1_OFFSET	0x1010
>
> They alternate between clock and reset.  The previous separated
> drivers specified the two offsets for each driver in the reg
> property.  But somehow these interleaved registers seemed a
> little off, so I thought the combined driver cleaned that up
> a little.
>
> I don't have any strong preference one way or another.  If you
> think they're better separate, I don't mind separating them
> again.  The two drivers are extremely simple.

Yes, SoC manufacturer tend to mix the 2, it is fairly usual and we used
to have many drivers in clock providing reset too. We have been slowly
cleaning that up over the past few years.

To be clear it is not strictly forbbiden to mix ressources but putting
things where they belong help review and maintainance.

There are several examples you can look at with auxiliary device
support, ./tenstorrent/atlantis-prcm.c is a good example.

>
> 					-Alex

-- 
Jerome

Re: [PATCH 3/4] clk: toshiba: introduce a TC9564 SoC clock and reset driver

[Brian Masney]

Hi Alex,

On Tue, Sep 22, 2026 at 08:33:37AM -0500, Alex Elder wrote:
> On 9/21/26 5:59 PM, Brian Masney wrote:
> > On Fri, Sep 18, 2026 at 11:52:32AM -0500, Alex Elder wrote:
> > > Define a new platform driver that manages clock and reset signals
> > > within the TC9564 SoC.  There are 21 clocks, which can only be
> > > enabled and disabled, as well as 13 reset signals.
> > > 
> > > Two registers manage the state of the clocks and two others manage
> > > the state of the resets.  The registers are accessed via a regmap
> > > supplied by a system controller, which coordinates access to a
> > > region of memory that will be shared with another driver.
> > > 
> > > Access to the memory region is provided via a BAR on a PCIe endpoint
> > > function embedded in the TC9564 SoC.  For that reason, neither the
> > > PCIe clock nor PCIe reset can be manipulated by this driver (they
> > > are assumed always on and deasserted, respectively).
> > > 
> > > Similarly, control is not available for the I2C clock and reset,
> > > because the PCIe subsystem on the TC9564 relies on I2C
> > > 
> > > Co-developed-by: Daniel Thompson <daniel@riscstar.com>
> > > Signed-off-by: Daniel Thompson <daniel@riscstar.com>
> > > Signed-off-by: Alex Elder <elder@riscstar.com>
> > > ---
> > >   MAINTAINERS              |   1 +
> > >   drivers/clk/Kconfig      |  11 ++
> > >   drivers/clk/Makefile     |   1 +
> > >   drivers/clk/clk-tc9564.c | 366 +++++++++++++++++++++++++++++++++++++++
> > >   4 files changed, 379 insertions(+)
> > >   create mode 100644 drivers/clk/clk-tc9564.c
> > > 
> > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > index 66d0e7e65adcb..39346a5cd9a7f 100644
> > > --- a/MAINTAINERS
> > > +++ b/MAINTAINERS
> > > @@ -27675,6 +27675,7 @@ M:	Alex Elder <elder@kernel.org>
> > >   M:	Daniel Thompson <danielt@kernel.org>
> > >   S:	Maintained
> > >   F:	Documentation/devicetree/bindings/clock/toshiba,tc9564-clock.yaml
> > > +F:	drivers/clk/clk-tc9564.c
> > >   F:	include/dt-bindings/clock/toshiba,tc9564.h
> > >   TOSHIBA TC9564 PCI DRIVER
> > > diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
> > > index f9592fd9ec2bb..50efa10d48450 100644
> > > --- a/drivers/clk/Kconfig
> > > +++ b/drivers/clk/Kconfig
> > > @@ -292,6 +292,17 @@ config COMMON_CLK_S2MPS11
> > >   	  clock. These multi-function devices have two (S2MPS14) or three
> > >   	  (S2MPS11, S5M8767) fixed-rate oscillators, clocked at 32KHz each.
> > > +config COMMON_CLK_TC9564
> > > +	tristate "Toshiba TC9564 clock support"
> > > +	depends on TC9564_PCI
> > 
> > select RESET_CONTROLLER
> 
> Thank you.  The reset and clock drivers were previously separate
> and the reset only became available if RESET_CONTROLLER was enabled.
> Combining them means I need this.  I will add it.
> 
> I also need to select MFD_SYSCON to get syscon_node_to_regmap()
> (or perhaps something similar, depending on the answers to my
> questions about the proper way to define the syscon).
> > 
> > > +	default m
> > 
> > default n
> 
> This depends on TC9564_PCI, and if TC9564_PCI is enabled I would
> like this to (automatically) be available (at least as a module).
> 
> Why do you recommend "n"?

There is no existing 'default m' or 'default y' in the toplevel clk Kconfig.
How about this instead?

    depends on TC9564_PCI || COMPILE_TEST
    default TC9564_PCI

> > > +	help
> > > +	  This enables support for the clock and reset controller embedded
> > > +	  in the Toshiba TC9564 (and Qualcomm QPS615) SoC.  The state of
> > > +	  clock and reset lines is controlled by MMIO to a region managed
> > > +	  by a system controller; this ensures access to the region is
> > > +	  coordinated between this and other drivers.
> > 
> > Rather than 'other drivers', outline specifically which other drivers.
> 
> I was intentionally vague at this point because the one other driver
> has not yet gone out for upstream review for this iteration of the
> code.  But I do agree with you, so I'll change this to say:
> 
>    ... between this and the XGMAC (stmmac) driver.
> 
> Then it will be correct without modification once that driver goes
> out for review.  Is that better, or do you suggest something else?

Yes that sounds good.

> > > +static const struct tc9564_clock_init tc9564_clock_init[] = {
> > > +	TC9564_CLOCK_INIT0(MCU, 0),
> > > +	TC9564_CLOCK_INIT0(INTC, 4),
> > > +	/* TC9564_CLOCK_INIT0(PCIE, 9), */
> > > +	/* TC9564_CLOCK_INIT0(I2C, 12), */
> > 
> > A comment would be useful here to outline why these are commented out.
> > Is the PCIE one comment out because of what's outlined in the commit
> > message?
> 
> Yes, that is why.  We are downstream of PCIe, and PCIe in this
> case depends on I2C.  So we don't want to mess with these two
> clocks or their reset counterparts.
> 
> I only provide this for the benefit of documentation.  If someone
> happens to see bit 9 set in this register, it means the PCIe clock
> is enabled.
> 
> Anyway, others have suggested simply removing these comments.
> So I'll do what you suggest, but I'm interested to know whether
> you would favor just deleting them instead.

I also think to delete these comments. Would it make sense to include
the comment in just include/dt-bindings/clock/toshiba,tc9564.h ?

Brian

[PATCH 4/4] arm64: dts: qcom: qcs6490-rb3gen2: add the clock controller

[Alex Elder]

The clock (and reset) controller within the TC9564 SoC is accessed
via a PCI endpoint bus through BAR 4 of the first embedded PCIe
function.  A syscon node is created to provide access to a range
of memory that is used by the clock/reset controller and shared
by other devices.

Co-developed-by: Daniel Thompson <daniel@riscstar.com>
Signed-off-by: Daniel Thompson <daniel@riscstar.com>
Signed-off-by: Alex Elder <elder@riscstar.com>
---
 arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 22 +++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
index 5b7f5292a8864..a99ee153e2473 100644
--- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
+++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
@@ -9,6 +9,7 @@
 #define PM7250B_SID 8
 #define PM7250B_SID1 9
 
+#include <dt-bindings/clock/toshiba,tc9564.h>
 #include <dt-bindings/iio/qcom,spmi-adc7-pmk8350.h>
 #include <dt-bindings/iio/qcom,spmi-adc7-pm7325.h>
 #include <dt-bindings/leds/common.h>
@@ -983,7 +984,7 @@ dev@0,1 {
 					 <0x2 0x0 0x0  0x83050100 0x0 0x4000  0x0 0x80000>,
 					 <0x4 0x0 0x0  0x83050100 0x0 0x84000  0x0 0x200000>;
 
-				pci-ep-bus@4 {
+				pcie_s1b5d0f0_bar4_bus: pci-ep-bus@4 {
 					compatible = "simple-bus";
 					#address-cells = <1>;
 					#size-cells = <1>;
@@ -995,6 +996,25 @@ pci-ep-bus@4 {
 	};
 };
 
+&pcie_s1b5d0f0_bar4_bus {
+	syscon@0 {
+		compatible = "toshiba,tc9564-config",
+			     "syscon",
+			     "simple-mfd";
+		reg = <0x0 0x2000>;
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges;
+
+		tc9564_clock0: clock@1004 {
+			compatible = "toshiba,tc9564-clock";
+			reg = <0x1004 0x10>;
+			#clock-cells = <1>;
+			#reset-cells = <1>;
+		};
+	};
+};
+
 &pm7325_gpios {
 	kypd_vol_up_n: kypd-vol-up-n-state {
 		pins = "gpio6";
-- 
2.53.0