Re: [PATCH 4/8] dt-bindings: Add Tegra264 clock and reset definitions

[Krzysztof Kozlowski <krzk@kernel.org>]

On 08/05/2025 09:53, Thierry Reding wrote:
> On Thu, May 08, 2025 at 09:40:02AM +0200, Krzysztof Kozlowski wrote:
>> On 07/05/2025 16:37, Thierry Reding wrote:
>>> @@ -0,0 +1,9 @@
>>> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
>>> +/* Copyright (c) 2022-2023, NVIDIA CORPORATION. All rights reserved. */
>>> +
>>> +#ifndef DT_BINDINGS_CLOCK_TEGRA264_CLOCK_H
>>> +#define DT_BINDINGS_CLOCK_TEGRA264_CLOCK_H
>>> +
>>> +#define TEGRA264_CLK_CLK_S			2U
>>
>> Abstract IDs start from 0 or 1, not 2. Also drop "U".
> 
> These are not abstract IDs, they are defined by the BPMP ABI. We cannot
> change them, otherwise it'll completely break.


You mean from the firmware? Sure. You have entire commit msg to explain
all unusual things here...

> 
> For similar reasons I'd like to keep the "U". These definitions are for
> the most part directly imported from the BPMP ABI headers, though we do
> try to be selective about what we add, to avoid adding hundreds of new
> lines in one go, and several safety-checking tools run on these headers
> that happen to require the "U" suffix to make sure these have a defined
> type.
> 
>>> +
>>> +#endif /* DT_BINDINGS_CLOCK_TEGRA264_CLOCK_H */
>>> diff --git a/include/dt-bindings/reset/tegra264-reset.h b/include/dt-bindings/reset/tegra264-reset.h
>>> new file mode 100644
>>> index 000000000000..31d89dcf62fa
>>> --- /dev/null
>>> +++ b/include/dt-bindings/reset/tegra264-reset.h
>>> @@ -0,0 +1,7 @@
>>> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
>>> +/* Copyright (c) 2022-2023, NVIDIA CORPORATION. All rights reserved. */
>>> +
>>> +#ifndef DT_BINDINGS_RESET_TEGRA264_RESET_H
>>> +#define DT_BINDINGS_RESET_TEGRA264_RESET_H
>>> +
>> This is empty, drop.
> 
> We have three people currently working on additional drivers for this
> SoC and they all need to add to these two files. Adding the empty file
> here makes it a bit easier to coordinate things, making the resulting
> conflicts trivial to resolve.


Bindings are supposed to be complete (see writing bindings doc), this
means also bindings headers. If the constants come from firmware, they
are defined so I really do not understands why they cannot be published now.

Unless you mean that this is a new SoC and the firmware is not yet
fixed/finished, but all this must be explained in the commit msg.

Best regards,
Krzysztof