[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ec255edc-adcd-4c18-8f9c-209298f2bbff@kernel.org>
Date: Mon, 13 Jan 2025 13:17:41 +0100
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Sergio Paracuellos <sergio.paracuellos@...il.com>
Cc: linux-clk@...r.kernel.org, sboyd@...nel.org, mturquette@...libre.com,
robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org,
yangshiji66@...look.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] dt-bindings: clock: add clock definitions for Ralink SoCs
On 13/01/2025 12:32, Sergio Paracuellos wrote:
> Hi Krzysztof,
>
> On Mon, Jan 13, 2025 at 12:18 PM Krzysztof Kozlowski <krzk@...nel.org> wrote:
>>
>> On 13/01/2025 09:28, Sergio Paracuellos wrote:
>>> Add clock missing definitions for RT2880, RT305X, RT3352, RT3383, RT5350,
>>> MT-7620 and MT-76X8 Ralink SoCs.
>>>
>>> Signed-off-by: Sergio Paracuellos <sergio.paracuellos@...il.com>
>>
>> 1. Please use get_maintainers.
>
> I did but from my tree which is tag: v6.12.5, linux-stable/linux-6.12.y. Sorry.
That's separate problem, but not one I was thinking about. Even on 6.12
stable you would get dt list, which is missing here.
This means no testing, although does not matter here, and it does not
appear in Patchwork which means does not reach us, except my filter for
nagging.
>
>> 2. What is the point of this? We do not add constants when there are no
>> users. Commit msg explains here nothing.
>
> All of the old ralink SoCs' dts files which are in the tree are not
> properly updated. I expect to have them updated somewhere in time
> merging real base stuff from openwrt dts [0] files. Not having this
> header with definitions makes very hard to update dts and then
> checking the driver code becomes a need to see the indexes for the
> clocks to properly setup a consumer node. Because of this, this file
> is added here.
Still there is no point without the users. I do not see any reason why
this cannot be combined with fixing driver to use the header. Not
combining is an indication this is not a binding in the first place.
In any case there should be explanation in commit msg about such choice.
>
>>
>>> ---
>>> include/dt-bindings/clock/mtmips-clk.h | 130 +++++++++++++++++++++++++
>>
>> Filename matching compatible.
>
> There are multiple compatibles regarding this, since the driver covers
Driver almost does not matter.
> a lot of SoCs. So I preferred to put them all in a single header file.
> See [1]. Should I add a different file for any single compatible
> instead?
Well, are all in the same bindings file? If so, can stay but then:
filename matching binding, because binding has proper naming scheme.
Best regards,
Krzysztof
Powered by blists - more mailing lists