Hi,
On 14/03/2023 16:19, Krzysztof Kozlowski wrote:
On 14/03/2023 16:01, Dmitry Rokosov wrote:
On Tue, Mar 14, 2023 at 03:05:48PM +0100, Krzysztof Kozlowski wrote:
On 14/03/2023 12:48, Dmitry Rokosov wrote:
On Tue, Mar 14, 2023 at 12:28:40PM +0100, Krzysztof Kozlowski wrote:
On 13/03/2023 21:12, Dmitry Rokosov wrote:
<snip>
+/*
+ * Copyright (c) 2019 Amlogic, Inc. All rights reserved.
+ * Author: Jian Hu <jian.hu@xxxxxxxxxxx>
+ *
+ * Copyright (c) 2023, SberDevices. All Rights Reserved.
+ * Author: Dmitry Rokosov <ddrokosov@xxxxxxxxxxxxxx>
+ */
+
+#ifndef __A1_PLL_CLKC_H
+#define __A1_PLL_CLKC_H
+
+#define CLKID_FIXED_PLL 1
+#define CLKID_FCLK_DIV2 6
+#define CLKID_FCLK_DIV3 7
+#define CLKID_FCLK_DIV5 8
+#define CLKID_FCLK_DIV7 9
+#define CLKID_HIFI_PLL 10
Probably I asked about this... why indices are not continuous? You know
that consumers are allowed to use number 2 and it will be your ABI, even
though you did not write it in the binding? That's a tricky and
confusing pattern for no real gains.
Actually, indices are continuou but splitted into two parts: public and
private. The public part is located in the dt bindings and can be included
from device tree sources. The private part is in the drivers/clk/meson
folder, and only clk drivers can use it.
I know, there is some trick when the user just inserts a digit value and
doesn't use constants.
This is not a trick. This is how DTS works. You have only indices/numbers.
But I'm starting from the assumption that such
dts changes will not be approved by maintainers. In other words, the user
*must* apply defined ABI constants from dt bindings; it's a strong
restriction.
But it is not correct assumption. Defines are very important, but they
are just helpers. Otherwise without defines you could not use any clock?
We pretty often use IDs - for DTS to allow merging via different trees,
for DT binding examples to not rely on headers.
Your driver implements the ABI and the driver exposes for example clock
ID=2, even if it is not in the header.
These IDs are unfortunately undocumented ABI and you if you change them,
users are allowed to complain.
Solution: don't do this. Have all exposed clock IDs and clocks in sync
(and continuous).
I see. But I don't understand how I can restrict access to private
clock objects. I don't want to open ability to change system clocks
parents, for example. Or it's under device tree developer responsibility?
I would appreciate any assistance in determining the best path.
There are many ways - depend on your driver. For example like this:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clk/samsung/clk-exynos5420.c#n975
The first argument is the clock ID (or ignore).
BTW, quite likely the problem is generic to all Meson clock drivers.
This issue about "public" non-continuous defined was already discussed at https://lore.kernel.org/all/c088e01c-0714-82be-8347-6140daf56640@xxxxxxxxxx/
I don't see what's different with this one.
Neil
Best regards,
Krzysztof
