Re: [PATCH v2 1/3] dt-bindings: clock: introduce toshiba,tc9564-clock.yaml

[Krzysztof Kozlowski]

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 <daniel@xxxxxxxxxxxx>
>>> Signed-off-by: Daniel Thompson <daniel@xxxxxxxxxxxx>
>>> Signed-off-by: Alex Elder <elder@xxxxxxxxxxxx>
> 
> 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 <elder@xxxxxxxxxxxx>
>>> +  - Daniel Thompson <daniel@xxxxxxxxxxxx>
>>> +
>>> +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/20260918165234.687224-5-elder@xxxxxxxxxxxx/
>>
>> 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