Re: [PATCH 3/8] dt-bindings: mfd: Add Google GS101 TMU Syscon

[Tudor Ambarus <tudor.ambarus@linaro.org>]

On 1/15/26 5:34 PM, Krzysztof Kozlowski wrote:
> On 15/01/2026 15:53, Tudor Ambarus wrote:
>>
>>
>> On 1/15/26 3:36 PM, Krzysztof Kozlowski wrote:
>>> On Wed, Jan 14, 2026 at 02:16:31PM +0000, Tudor Ambarus wrote:
>>>> Document the bindings for the Thermal Management Unit (TMU) System
>>>> Controller found on Google GS101 SoCs.
>>>>
>>>> This memory-mapped block exposes the registers required for reading
>>>> thermal interrupt status bits. It functions as a syscon provider,
>>>
>>> I don't think this is syscon, but the actual TMU. Syscon is various,
>>> unrelated system configuration registers.
>>>
>>>> allowing the main thermal driver to access these registers while
>>>> the firmware manages the core thermal logic.
>>>>
>>>> Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
>>>> ---
>>>>  .../bindings/mfd/google,gs101-tmu-syscon.yaml      | 37 ++++++++++++++++++++++
>>>>  1 file changed, 37 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/mfd/google,gs101-tmu-syscon.yaml b/Documentation/devicetree/bindings/mfd/google,gs101-tmu-syscon.yaml
>>>> new file mode 100644
>>>> index 0000000000000000000000000000000000000000..6a11e43abeaa23ee473be2153478436856277714
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/mfd/google,gs101-tmu-syscon.yaml
>>>
>>> Not MFD either, but soc.
>>
>> You are right, it's not a syscon, it's just a normal thermal IP block
>> from which I need to access the interrupt pending registers.
>>
>> Then I guess I shall describe the new binding in bindings/thermal/,
>> please correct me if I'm wrong.
>>
>>>
>>>> @@ -0,0 +1,37 @@
>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>>> +%YAML 1.2
>>>> +---
>>>> +$id: http://devicetree.org/schemas/mfd/google,gs101-tmu-syscon.yaml#
>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>> +
>>>> +title: Google GS101 TMU System Controller
>>>> +
>>>> +maintainers:
>>>> +  - Tudor Ambarus <tudor.ambarus@linaro.org>
>>>> +
>>>> +description: |
>>>
>>> Drop |
>>>
>>>> +  The TMU System Controller provides a memory-mapped interface for
>>>> +  accessing the interrupt status registers of the Thermal Management
>>>> +  Unit. It is used as a syscon provider for the main TMU driver.
>>>
>>> No, it is not a syscon provider. Entire last sentence is incorrect. You
>>> must describe here hardware and this hardware does not provide any sort
>>> of syscon to any sort of driver.
>>>
>>
>> Indeed.
>>
>> I'm going to link the ACPM TMU child node with the TMU node via a
>> "samsung,tmu-regs" property.
> 
> This could be fine, but I actually wonder what's there. What registers
> exactly. For example modern Exynos 88xx, already with APM block, still
> have exactly the same TMU unit at 0x1008{04}000 with all typical
> triminfo, current temperature and thresholds.
> 

It's the same for gs101, the TMU instances have all the typical registers,
it's just that everything is handled via ACPM but the intpend registers.

>>
>> Some concern that I have is that I describe the clocks and interrupts in
>> the ACPM TMU child node. Usually the clocks and interrupts belong to the
>> node that contains the reg property. But I guess that's alright because
>> the interrupts property is expected to be in the node that the driver
>> binds to. For the clocks, by placing it in the ACPM child node, I allow
>> runtime PM to manage it.
> 
> You have to first know whether these clocks and interrupts are going TO
> the ACPM node.

They aren't, the clocks and interrupts are going to the TMU node.

> 
> All this looks like designing for drivers, sorry.
> 

It's fine, I'm not looking to cut corners. Thanks for the feedback, it
helps us coming up to a better solution.

>>
>> Do you think the below description is accurate?
>>
>> soc: soc@0 {
>>     tmu_top: thermal-sensor@100a0000 {
>>         compatible = "google,gs101-tmu-top";
>>         reg = <0x100a0000 0x800>;
>>     };
>> };
>>
>> firmware {
>>     acpm_ipc: power-management {
>>         compatible = "google,gs101-acpm-ipc";
>>         /* ... */
>>
>>         thermal-sensor {
>>             compatible = "google,gs101-acpm-tmu-top";
>>             clocks = <&cmu_misc CLK_GOUT_MISC_TMU_TOP_PCLK>;
>>             interrupts = <GIC_SPI 769 IRQ_TYPE_LEVEL_HIGH 0>;
> 
> This I doubt, really. Why would ACPM child be hooked via CPU clock and
> interrupts?
> 

Yeah, that was my concern as well. Then the clocks and interrupts will
be described in the TMU node, not in the ACPM TMU child. The bindings
are clear to me now, thanks.

In what concerns the interrupts, I can obtain them via the phandle to the
TMU node.

The trickier part will be on how to handle runtime pm, I can no longer use
pm_runtime_get_sync(dev) in the ACPM child. But I think I can handle it with
device links. Get a pointer to the TMU device node and use device links:

device_link_add(acpm_dev, &tmu_pdev->dev, DL_FLAG_PM_RUNTIME | DL_FLAG_AUTOREMOVE_CONSUMER);

This tells the kernel that when the ACPM TMU driver is active, the TMU
hardware block must also be active.

Please let me know if this sounds reasonable. I'll start reworking the set.

Thanks for the help, I appreciate it!
ta