On Tue, Mar 14, 2023 at 12:28:40PM +0100, Krzysztof Kozlowski wrote: > On 13/03/2023 21:12, Dmitry Rokosov wrote: [...] > > +#define CLKID_SPIFC 84 > > +#define CLKID_USB_BUS 85 > > +#define CLKID_SD_EMMC 86 > > +#define CLKID_PSRAM 87 > > +#define CLKID_DMC 88 > > And what is here? Between 88 and 121? > Explained below. > > +#define CLKID_GEN_SEL 121 > > + > > +#endif /* __A1_CLKC_H */ > > diff --git a/include/dt-bindings/clock/amlogic,a1-pll-clkc.h b/include/dt-bindings/clock/amlogic,a1-pll-clkc.h > > new file mode 100644 > > index 000000000000..8e97d3fb9d30 > > --- /dev/null > > +++ b/include/dt-bindings/clock/amlogic,a1-pll-clkc.h > > @@ -0,0 +1,20 @@ > > +/* SPDX-License-Identifier: GPL-2.0+ */ > > I found in changelog: > "fix license issue, it's GPL-2.0+ only in the current version" > and I do not understand. > > The license is wrong, so what did you fix? > Sorry don't get you. Why is it wrong? I've changed all new source files to GPL-2.0+ except yaml, because yaml dt bindings schemas require the following license: # SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause I've pointed it in the changelog. > > +/* > > + * 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. 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. -- Thank you, Dmitry