Re: [PATCH v10 3/5] dt-bindings: clock: meson: add A1 PLL and Peripherals clkcs bindings

From: Krzysztof Kozlowski
Date: Tue Mar 14 2023 - 07:28:54 EST


On 13/03/2023 21:12, Dmitry Rokosov wrote:
> Add the documentation for Amlogic A1 PLL and Amlogic A1 Peripherals
> clock drivers.
> Introduce Amlogic A1 PLL and Amlogic A1 Peripherals device tree
> bindings and include them to MAINTAINERS.
>


>
> diff --git a/include/dt-bindings/clock/amlogic,a1-clkc.h b/include/dt-bindings/clock/amlogic,a1-clkc.h
> new file mode 100644
> index 000000000000..2b16b1f1a5bf
> --- /dev/null
> +++ b/include/dt-bindings/clock/amlogic,a1-clkc.h
> @@ -0,0 +1,100 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */

???

> +#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?

> +#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?

> +/*
> + * 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.

Best regards,
Krzysztof