On 09/10/2026 15:44, Alex Elder wrote: > On 10/9/26 4:31 AM, Krzysztof Kozlowski wrote: >> On Mon, Oct 05, 2026 at 06:09:24PM -0500, Alex Elder wrote: >>> Define the binding for the clock controller functionality present in >>> the Toshiba TC9564 SoC. >>> >>> Co-developed-by: Daniel Thompson <[email protected]> >>> Signed-off-by: Daniel Thompson <[email protected]> >>> Signed-off-by: Alex Elder <[email protected]> > > I'm just about to send version 3 of this series (and > then a new reset series derived from the code that was > previously combined with the clock code). Some things > related to what you mention have changed. I show that > below, but also respond to your other comments, and try > to advocate for the approach used. >>> --- >>> v2: - Only define clock information, not reset information >>> - Reworded description to avoid talking about software >>> - Clock IDs are now consecutive (no more commented-out values) >>> >>> .../bindings/clock/toshiba,tc9564-clock.yaml | 54 +++++++++++++++++++ >>> MAINTAINERS | 7 +++ >>> include/dt-bindings/clock/toshiba,tc9564.h | 34 ++++++++++++ >>> 3 files changed, 95 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..329b8f002cf2d >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/clock/toshiba,tc9564-clock.yaml >>> @@ -0,0 +1,54 @@ >>> +# 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 Controller >>> + >>> +maintainers: >>> + - Alex Elder <[email protected]> >>> + - Daniel Thompson <[email protected]> >>> + >>> +description: >>> + The Toshiba TC9564 is an SoC accessed by a host system through the >>> + upstream PCIe port on the PCIe switch it implements. The switch >>> + includes an embedded PCIe endpoint that provides access to various >>> + SoC peripherals (including a clock controller) via its BARs. >>> + >>> + 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. >>> + >>> + Clock ids are defined in <dt-bindings/clock/toshiba,tc9564.h>. >>> + >>> +properties: >>> + compatible: >>> + const: toshiba,tc9564-clock >>> + >>> + toshiba,config-syscon: >>> + $ref: /schemas/types.yaml#/definitions/phandle >>> + description: >>> + Phandle for the configuration space system controller. >> >> I do not see my previous comment addressed - you have no resources here, >> so this belongs to the parent. You responded something about pci-ep, but >> the parent is not pci-ep. Open your code: >> https://lore.kernel.org/lkml/[email protected]/ >> >> I clearly see code like: >> syscon { >> clock@ { >> }; >> }; >> >> so I do not understand what pci-ep has anything to do here. > > What I have now (about to send) looks like this: > > syscon@0 { > compatible = "syscon", "simple-mfd"; > reg = <0x0 0x2000>; > > clock { > compatible = "toshiba,tc9564-clock"; > #clock-cells = <1>; > }; > }; > > A reset node will also go inside the syscon, so there is another > function for that MFD. > > The regmap belongs to the parent, and is looked up this way: > > regmap = syscon_node_to_regmap(dev_of_node(dev->parent));
That's driver code, so irrelevant. So how does this solve my comment from v1? > >> What's more, I still do not see any usage of these clocks outside. And I >> still did not receive actual answers (or I missed them) how these clocks >> are routed OUTSIDE of the connector. You said for example: >> "Ultimately the TC9564 SoC has a single 25 MHz input clock," >> >> but that is input. I did not ask how this device receives clocks. I >> asked how the host receives the clocks from this device. > > I was explaining that the clock input gets split into a number > of "output" clocks derived from that one input. However you're > right, almost all of these clocks are connected to blocks > internal to the SoC. There is only one 25 MHz clock that is > exposed externally (CLOCK_REFCLKO). > > I think your point (or one of them) is that, even if pci-ep-bus > is used, the only things that warrant being described with > devicetree are those that can affect things outside the chip. > Everything else is not really variable from the perspective of > the platform, and inner details can be determined (and controlled) > by software. > > > Our original version of this code incorporated clock and reset > control inside the networking driver. Rob commented that the > DWMAC driver should bind to the PCI functions and "everything > else...should be under the PCIe switch upstream node in a > pci-ep.bus". > > The only driver associated with the switch inside the TC9564 > is the special power control driver, accessed via I2C. The > PCI switch functionality is otherwise provided without any > special handling, or is handled by generic code. > > What *is* available is the PCI endpoint functions and their > BARs, so the endpoint buses use those. > > PCI endpoint bus makes available things that devicetree > does that are very useful (including a standard way of > describing connections between blocks in the SoC). > > > I know that doesn't address the "exposed clocks" issue, > but it provides some background on why things were done > the way they were. > > > The single exposed clock *might* justify presenting the > clock controller device in devicetree. There are also > resets exposed externally via GPIOs, and these control > external entities (PHYs). I cannot find any of these exposed. Please point me to DTS code showing this. Best regards, Krzysztof
