Messages in this thread | ![/](/images/icornerl.gif) | | Date | Tue, 14 Mar 2023 14:48:25 +0300 | From | Dmitry Rokosov <> | Subject | Re: [PATCH v10 3/5] dt-bindings: clock: meson: add A1 PLL and Peripherals clkcs bindings |
| |
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@amlogic.com> > > + * > > + * Copyright (c) 2023, SberDevices. All Rights Reserved. > > + * Author: Dmitry Rokosov <ddrokosov@sberdevices.ru> > > + */ > > + > > +#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
| ![\](/images/icornerr.gif) |