Devicetree
 help / color / mirror / Atom feed
From: Krzysztof Kozlowski <krzk@kernel.org>
To: Alex Elder <elder@riscstar.com>
Cc: sboyd@kernel.org, bmasney+clk@redhat.com,
	jbrunet+clk@baylibre.com, robh@kernel.org, krzk+dt@kernel.org,
	conor+dt@kernel.org, andersson@kernel.org,
	konradybcio@kernel.org, abelvesa@kernel.org, kees@kernel.org,
	gustavoars@kernel.org, mohd.anwar@oss.qualcomm.com,
	lorenzo.bianconi@oss.qualcomm.com, danielt@kernel.org,
	linux-clk@vger.kernel.org, devicetree@vger.kernel.org,
	linux-arm-msm@vger.kernel.org, linux-hardening@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	Daniel Thompson <daniel@riscstar.com>
Subject: Re: [PATCH v2 1/3] dt-bindings: clock: introduce toshiba,tc9564-clock.yaml
Date: Fri, 9 Oct 2026 16:46:02 +0200	[thread overview]
Message-ID: <ad511ae7-755d-42bc-a55d-12b93d49ddaf@kernel.org> (raw)
In-Reply-To: <77dacc46-7294-47c8-899a-fd56b6a2af55@riscstar.com>

On 09/10/2026 16:23, Alex Elder 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 <daniel@riscstar.com>
>>>>> Signed-off-by: Daniel Thompson <daniel@riscstar.com>
>>>>> Signed-off-by: Alex Elder <elder@riscstar.com>
> 
> . . .
> 
>>>>> +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@riscstar.com/
>>>>
>>>> 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.

In v1 I asked to fold the node into the parent. v2 did not have it. v3,
which you are preparing, still has no node folded, because they are
separate.

> 
> 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.

Nodes should be squashed.

> 
>>>> 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.

Yeah, but you do not describe the SoC but PCI EP device, thus my claim
is that clocks cannot be used by the host. The SoC itself for different
hardware uses would have different binding, so that's not an argument to
have anything here, unless these different uses are also documented here.

Best regards,
Krzysztof

  parent reply	other threads:[~2026-10-09 14:46 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-05 23:09 [PATCH v2 0/3] clk: introduce TC9564 clock support Alex Elder
2026-10-05 23:09 ` [PATCH v2 1/3] dt-bindings: clock: introduce toshiba,tc9564-clock.yaml Alex Elder
2026-10-09  9:31   ` Krzysztof Kozlowski
2026-10-09 13:44     ` Alex Elder
2026-10-09 13:56       ` Krzysztof Kozlowski
2026-10-09 14:23         ` Alex Elder
2026-10-09 14:38           ` Jerome Brunet
2026-10-09 15:27             ` Alex Elder
2026-10-09 14:46           ` Krzysztof Kozlowski [this message]
2026-10-09 15:09             ` Alex Elder
2026-10-05 23:09 ` [PATCH v2 2/3] clk: toshiba: introduce a TC9564 SoC clock driver Alex Elder
2026-10-09 17:42   ` Brian Masney
2026-10-05 23:09 ` [PATCH v2 3/3] arm64: dts: qcom: qcs6490-rb3gen2: add the clock controller Alex Elder
2026-10-06  0:30   ` Alex Elder

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ad511ae7-755d-42bc-a55d-12b93d49ddaf@kernel.org \
    --to=krzk@kernel.org \
    --cc=abelvesa@kernel.org \
    --cc=andersson@kernel.org \
    --cc=bmasney+clk@redhat.com \
    --cc=conor+dt@kernel.org \
    --cc=daniel@riscstar.com \
    --cc=danielt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=elder@riscstar.com \
    --cc=gustavoars@kernel.org \
    --cc=jbrunet+clk@baylibre.com \
    --cc=kees@kernel.org \
    --cc=konradybcio@kernel.org \
    --cc=krzk+dt@kernel.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lorenzo.bianconi@oss.qualcomm.com \
    --cc=mohd.anwar@oss.qualcomm.com \
    --cc=robh@kernel.org \
    --cc=sboyd@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox