Re: [PATCH v4 01/10] dt-bindings: clock: Add Realtek RTD1625 Clock & Reset Controller
From: Krzysztof Kozlowski
Date: Tue Mar 17 2026 - 03:15:22 EST
On 17/03/2026 02:57, Yu-Chun Lin [林祐君] wrote:
>> On Fri, Mar 13, 2026 at 04:10:51PM +0800, Yu-Chun Lin wrote:
>>> +#endif /* __DT_BINDINGS_RTK_CLOCK_RTD1625_H */
>>> diff --git a/include/dt-bindings/reset/realtek,rtd1625.h
>>> b/include/dt-bindings/reset/realtek,rtd1625.h
>>> new file mode 100644
>>> index 000000000000..e69b5ee14cc6
>>> --- /dev/null
>>> +++ b/include/dt-bindings/reset/realtek,rtd1625.h
>>> @@ -0,0 +1,195 @@
>>> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
>>> +/*
>>> + * Copyright (C) 2025 Realtek Semiconductor Corp.
>>> + */
>>> +#ifndef __DT_BINDINGS_RTK_RESET_RTD1625_H #define
>>> +__DT_BINDINGS_RTK_RESET_RTD1625_H
>>> +
>>> +#define RTD1625_RSTN_REG_ID_SOFT_RESET1 0
>>> +#define RTD1625_RSTN_REG_ID_SOFT_RESET2 1
>>> +#define RTD1625_RSTN_REG_ID_SOFT_RESET3 2
>>> +#define RTD1625_RSTN_REG_ID_SOFT_RESET4 3
>>> +#define RTD1625_RSTN_REG_ID_SOFT_RESET7 4
>>> +#define RTD1625_RSTN_REG_ID_SOFT_RESET9 5
>>> +#define RTD1625_RSTN_REG_ID_SOFT_RESET_BIST 6
>>> +#define RTD1625_RSTN_REG_ID_DUMMY0 7
>>> +#define RTD1625_RSTN_REG_ID_DUMMY1 8
>>> +#define RTD1625_RSTN_REG_ID_DUMMY4 9
>>> +#define RTD1625_RSTN_REG_ID_SOFT_RESET11 10
>>> +#define RTD1625_RSTN_REG_ID_SOFT_RESET12 11
>>> +
>>> +#define _REG_ID(n) ((RTD1625_RSTN_REG_ID_ ## n) << 5)
>>> +
>>
>> Not a binding.
>>
>>> +#define RTD1625_CRT_RSTN_MISC (_REG_ID(SOFT_RESET1) | 0)
>>> +#define RTD1625_CRT_RSTN_DIP (_REG_ID(SOFT_RESET1) | 2)
>>> +#define RTD1625_CRT_RSTN_GSPI (_REG_ID(SOFT_RESET1) | 4)
>>
>> None of these are really bindings. Register offsets is hardware data, not a
>> binding.
>>
>> Drop all of this above and further - actually nothing in this header look like a
>> binding, since you called these "REG".
>>
>> Best regards,
>> Krzysztof
>
> The defines in this header are NOT register offsets. They encode "Bank ID+ Bit
> Position" for use in Device Tree.
Bit position as in register bit position? That's still not a binding.
>
> Agreed, The naming with "REG_ID" is misleading. I will improve naming all reset
> IDs in v5.
No, all are to be dropped or renamed to standard IDs (incremented by one
from 0 or 1).
Best regards,
Krzysztof