On ven. 09 oct. 2026 at 09:23, Alex Elder <[email protected]> wrote:
> On 10/9/26 8:56 AM, Krzysztof Kozlowski wrote: >> 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]> > > . . . > >>>>> +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? > > I'm trying Krzysztof. > > The clock controller uses two registers, 0x1004 and 0x100c, > to manage whether a set of clock signals are enabled or not. > (The reset controller uses two adjacent registers, 0x1008 > and 0x1010, to manage whether a set of reset signals are > asserted or not.) > > You said "no resources except a small address space" and I > guess it's not clear to me what size is "big enough" to > warrant representing something as a separate device. > > *One* of the managed clocks is a 25 MHz clock, exposed > through a pin on the SoC. That one clock signal is > therefore usable by the platform (although on the RB3gen2 > it's not used). > > Rather than expose the register addresses in the clock > node, a syscon is defined, covering 8 KB, and the actual > offsets used are just defined in the clock and reset > driver source code. > > If that's not the right thing to do, please say that. > >>>> 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," > . . . > >>> 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. > > It is not used by this platform, but is available for other > platforms to use. Its name is "REFCLKO" and is exposed on > ball C17 of the SoC, if a platform designer decided to use it. > > I only mention its existence as a reason to justify defining > the clock as a separate device, but I realize you are arguing > that I should do it somehow differently. > > -Alex While on the topic of description, I'm little bit concerned that this controller does not any input ? Does it have an on-board oscillator somehow ? None of the clocks described in your driver take a parent from what I can see. It is as if the clocks of this device are generated out of thin air. Is it really how this works ? > >> >> Best regards, >> Krzysztof > -- Jerome
