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

  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.

Signed-off-by: Jian Hu <jian.hu@amlogic.com>
Signed-off-by: Dmitry Rokosov <ddrokosov@sberdevices.ru>
---
 .../bindings/clock/amlogic,a1-clkc.yaml       |  73 +++++++++++++
 .../bindings/clock/amlogic,a1-pll-clkc.yaml   |  59 +++++++++++
 MAINTAINERS                                   |   1 +
 include/dt-bindings/clock/amlogic,a1-clkc.h   | 100 ++++++++++++++++++
 .../dt-bindings/clock/amlogic,a1-pll-clkc.h   |  20 ++++
 5 files changed, 253 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/amlogic,a1-clkc.yaml
 create mode 100644 Documentation/devicetree/bindings/clock/amlogic,a1-pll-clkc.yaml
 create mode 100644 include/dt-bindings/clock/amlogic,a1-clkc.h
 create mode 100644 include/dt-bindings/clock/amlogic,a1-pll-clkc.h
  

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@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.

Best regards,
Krzysztof
  

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.
  

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:
> 
> [...]
> 
>>> +#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?

Run checkpatch - it will tell you why wrong. The license is not correct.
This is part of binding and should be the same as binding.

> 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.

The only thing I found was:
"fix license issue, it's GPL-2.0+ only in the current version"

so what exactly you pointed out in changelog? What was to fix? What was
fixed? Correct license into incorrect? But why?

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

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).

Best regards,
Krzysztof
  

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:
> > 
> > [...]
> > 
> >>> +#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?
> 
> Run checkpatch - it will tell you why wrong. The license is not correct.
> This is part of binding and should be the same as binding.
> 

I always run checkpatch before sending the next patch series. Checkpatch
doesn't highlight this problem:

--------------
$ rg SPDX a1_clkc_v10/v10-0003-dt-bindings-clock-meson-add-A1-PLL-and-Periphera.patch
32:+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
111:+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
188:+/* SPDX-License-Identifier: GPL-2.0+ */
294:+/* SPDX-License-Identifier: GPL-2.0+ */

$ ./scripts/checkpatch.pl --strict a1_clkc_v10/v10-0003-dt-bindings-clock-meson-add-A1-PLL-and-Periphera.patch
total: 0 errors, 0 warnings, 0 checks, 259 lines checked

a1_clkc_v10/v10-0003-dt-bindings-clock-meson-add-A1-PLL-and-Periphera.patch has no obvious style problems and is ready for submission.
--------------

I've got your point, will fix in the next version.

> > 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.
> 
> The only thing I found was:
> "fix license issue, it's GPL-2.0+ only in the current version"
> 
> so what exactly you pointed out in changelog? What was to fix? What was
> fixed? Correct license into incorrect? But why?
> 

By 'license issue' I meant your comment for the previous version at:
https://lore.kernel.org/all/6a950a51-fe90-9163-b73d-0a396d7187ee@linaro.org/

I thought you mentioned the problem is in two license usage in the one
line (GPL + MIT), I've fixed it to GPL2 only, and mentioned it in the
changelog.

I didn't know about the special requirement for a dt-bindings license, I've
just checked other clock dt-bindings and found that license is different
in the many places:

$ grep -r "SPDX" include/dt-bindings/clock | grep -v -e "GPL-2.0.*BSD-2-Clause" | wc -l
291

And Tegra Car 124 as an example for different license between yaml
schema and binding header:
$ grep "SPDX" include/dt-bindings/clock/tegra124-car.h
/* SPDX-License-Identifier: GPL-2.0 */
$ grep "SPDX" Documentation/devicetree/bindings/clock/nvidia,tegra124-car.yaml
# SPDX-License-Identifier: (GPL-2.0+ OR BSD-2-Clause)

Anyway, it's not a problem to fix the license to the same value between
header and yaml schema, I'll fix it in the next version.
But based on the above experiments, other clock bindings should be fixed
as well, checkpatch behavior should be extended for dt bindings headers
licence checking.

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

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:
>>>
>>> [...]
>>>
>>>>> +#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?
>>
>> Run checkpatch - it will tell you why wrong. The license is not correct.
>> This is part of binding and should be the same as binding.
>>
> 
> I always run checkpatch before sending the next patch series. Checkpatch
> doesn't highlight this problem:
> 
> --------------
> $ rg SPDX a1_clkc_v10/v10-0003-dt-bindings-clock-meson-add-A1-PLL-and-Periphera.patch
> 32:+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> 111:+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> 188:+/* SPDX-License-Identifier: GPL-2.0+ */
> 294:+/* SPDX-License-Identifier: GPL-2.0+ */
> 
> $ ./scripts/checkpatch.pl --strict a1_clkc_v10/v10-0003-dt-bindings-clock-meson-add-A1-PLL-and-Periphera.patch
> total: 0 errors, 0 warnings, 0 checks, 259 lines checked

Hmm, my bad, that's something to fix/improve in checkpatch.

> 
> a1_clkc_v10/v10-0003-dt-bindings-clock-meson-add-A1-PLL-and-Periphera.patch has no obvious style problems and is ready for submission.
> --------------
> 
> I've got your point, will fix in the next version.
> 
>>> 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.
>>
>> The only thing I found was:
>> "fix license issue, it's GPL-2.0+ only in the current version"
>>
>> so what exactly you pointed out in changelog? What was to fix? What was
>> fixed? Correct license into incorrect? But why?
>>
> 
> By 'license issue' I meant your comment for the previous version at:
> https://lore.kernel.org/all/6a950a51-fe90-9163-b73d-0a396d7187ee@linaro.org/
> 
> I thought you mentioned the problem is in two license usage in the one
> line (GPL + MIT), I've fixed it to GPL2 only, and mentioned it in the
> changelog.

The comment was for a reason why the license here is different than in
the binding. Should be the same. Binding had license:
GPL-2.0-only OR BSD-2-Clause

> 
> I didn't know about the special requirement for a dt-bindings license, I've
> just checked other clock dt-bindings and found that license is different
> in the many places:
> 
> $ grep -r "SPDX" include/dt-bindings/clock | grep -v -e "GPL-2.0.*BSD-2-Clause" | wc -l
> 291
> 
> And Tegra Car 124 as an example for different license between yaml
> schema and binding header:
> $ grep "SPDX" include/dt-bindings/clock/tegra124-car.h
> /* SPDX-License-Identifier: GPL-2.0 */
> $ grep "SPDX" Documentation/devicetree/bindings/clock/nvidia,tegra124-car.yaml
> # SPDX-License-Identifier: (GPL-2.0+ OR BSD-2-Clause)

checkpatch has the correct license. Many files were licensed differently
*on purpose* so I asked about purpose here.

> 
> Anyway, it's not a problem to fix the license to the same value between
> header and yaml schema, I'll fix it in the next version.
> But based on the above experiments, other clock bindings should be fixed

Your binding has a correct license. What should be fixed?

> as well, checkpatch behavior should be extended for dt bindings headers
> licence checking.

Yes.

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

Best regards,
Krzysztof
  

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@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.
>>>
>>> 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@linaro.org/

I don't see what's different with this one.

Neil

> 
> Best regards,
> Krzysztof
>
  

On 14/03/2023 16:33, neil.armstrong@linaro.org wrote:
>> 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@linaro.org/
> 
> I don't see what's different with this one.

So you are aware that all undocumented clock IDs are still allowed to
use in DTS and they are ABI? Changing them will be an ABI break.

Best regards,
Krzysztof
  

On 14/03/2023 16:37, Krzysztof Kozlowski wrote:
> On 14/03/2023 16:33, neil.armstrong@linaro.org wrote:
>>> 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@linaro.org/
>>
>> I don't see what's different with this one.
> 
> So you are aware that all undocumented clock IDs are still allowed to
> use in DTS and they are ABI? Changing them will be an ABI break.

Yes of course.

Neil

> 
> Best regards,
> Krzysztof
>
  

On Tue, Mar 14, 2023 at 04:40:19PM +0100, neil.armstrong@linaro.org wrote:
> On 14/03/2023 16:37, Krzysztof Kozlowski wrote:
> > On 14/03/2023 16:33, neil.armstrong@linaro.org wrote:
> > > > 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@linaro.org/
> > > 
> > > I don't see what's different with this one.
> > 
> > So you are aware that all undocumented clock IDs are still allowed to
> > use in DTS and they are ABI? Changing them will be an ABI break.
> 
> Yes of course.
> 
> Neil
> 
> > 
> > Best regards,
> > Krzysztof
> > 
> 

Sorry, guys, I'm little bit confused.
In the discussion pointed by Neil not-by-one-increment ID with public and
private parts are acked by Krzysztof due to explicit explanation in the
gxbb header. Have I to comment out my situation and stay it as is?

BTW, I think changing IDs value would not affect logic, because
it's not connected to driver logic 'by values', but 'by constants
names'. We can expose/hide anything from device tree bindings, it will
not change the clk driver logic.
  

On Tue, Mar 14, 2023 at 04:19:22PM +0100, 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:
> >>>
> >>> [...]
> >>>
> >>>>> +#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?
> >>
> >> Run checkpatch - it will tell you why wrong. The license is not correct.
> >> This is part of binding and should be the same as binding.
> >>
> > 
> > I always run checkpatch before sending the next patch series. Checkpatch
> > doesn't highlight this problem:
> > 
> > --------------
> > $ rg SPDX a1_clkc_v10/v10-0003-dt-bindings-clock-meson-add-A1-PLL-and-Periphera.patch
> > 32:+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > 111:+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > 188:+/* SPDX-License-Identifier: GPL-2.0+ */
> > 294:+/* SPDX-License-Identifier: GPL-2.0+ */
> > 
> > $ ./scripts/checkpatch.pl --strict a1_clkc_v10/v10-0003-dt-bindings-clock-meson-add-A1-PLL-and-Periphera.patch
> > total: 0 errors, 0 warnings, 0 checks, 259 lines checked
> 
> Hmm, my bad, that's something to fix/improve in checkpatch.
> 
> > 
> > a1_clkc_v10/v10-0003-dt-bindings-clock-meson-add-A1-PLL-and-Periphera.patch has no obvious style problems and is ready for submission.
> > --------------
> > 
> > I've got your point, will fix in the next version.
> > 
> >>> 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.
> >>
> >> The only thing I found was:
> >> "fix license issue, it's GPL-2.0+ only in the current version"
> >>
> >> so what exactly you pointed out in changelog? What was to fix? What was
> >> fixed? Correct license into incorrect? But why?
> >>
> > 
> > By 'license issue' I meant your comment for the previous version at:
> > https://lore.kernel.org/all/6a950a51-fe90-9163-b73d-0a396d7187ee@linaro.org/
> > 
> > I thought you mentioned the problem is in two license usage in the one
> > line (GPL + MIT), I've fixed it to GPL2 only, and mentioned it in the
> > changelog.
> 
> The comment was for a reason why the license here is different than in
> the binding. Should be the same. Binding had license:
> GPL-2.0-only OR BSD-2-Clause
> 
> > 
> > I didn't know about the special requirement for a dt-bindings license, I've
> > just checked other clock dt-bindings and found that license is different
> > in the many places:
> > 
> > $ grep -r "SPDX" include/dt-bindings/clock | grep -v -e "GPL-2.0.*BSD-2-Clause" | wc -l
> > 291
> > 
> > And Tegra Car 124 as an example for different license between yaml
> > schema and binding header:
> > $ grep "SPDX" include/dt-bindings/clock/tegra124-car.h
> > /* SPDX-License-Identifier: GPL-2.0 */
> > $ grep "SPDX" Documentation/devicetree/bindings/clock/nvidia,tegra124-car.yaml
> > # SPDX-License-Identifier: (GPL-2.0+ OR BSD-2-Clause)
> 
> checkpatch has the correct license. Many files were licensed differently
> *on purpose* so I asked about purpose here.
> 

Okay, got it. So no reason to fix this difference.

> > 
> > Anyway, it's not a problem to fix the license to the same value between
> > header and yaml schema, I'll fix it in the next version.
> > But based on the above experiments, other clock bindings should be fixed
> 
> Your binding has a correct license. What should be fixed?
> 

Other clocks bindings for another drivers. But you've explained the
reason above, so skip it.

> > as well, checkpatch behavior should be extended for dt bindings headers
> > licence checking.
> 
> Yes.
> 
> > 
> >>>
> >>>>> +/*
> >>>>> + * 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.
> >>
> >> 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).
> 

Hmmm, I like this way. But first, I think we need to make meson clock driver
overall situation clear.

[...]
  

On 14/03/2023 16:56, Dmitry Rokosov wrote:
> On Tue, Mar 14, 2023 at 04:40:19PM +0100, neil.armstrong@linaro.org wrote:
>> On 14/03/2023 16:37, Krzysztof Kozlowski wrote:
>>> On 14/03/2023 16:33, neil.armstrong@linaro.org wrote:
>>>>> 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@linaro.org/
>>>>
>>>> I don't see what's different with this one.
>>>
>>> So you are aware that all undocumented clock IDs are still allowed to
>>> use in DTS and they are ABI? Changing them will be an ABI break.
>>
>> Yes of course.
>>
>> Neil
>>
>>>
>>> Best regards,
>>> Krzysztof
>>>
>>
> 
> Sorry, guys, I'm little bit confused.
> In the discussion pointed by Neil not-by-one-increment ID with public and
> private parts are acked by Krzysztof due to explicit explanation in the
> gxbb header. Have I to comment out my situation and stay it as is?

I did not NAK your solution here. I just pointed my usual remarks that
it has certain outcome and minuses (undocumented ABI). But it is OK.

> 
> BTW, I think changing IDs value would not affect logic, because
> it's not connected to driver logic 'by values', but 'by constants

You cannot change the IDs, neither their values nor the names (with
exceptions). IDs - so the numbers - are ABI.

"Constant names" - I assume you mean the names of defines - do not exist
after preprocessing, so also not really relevant here...

> names'. We can expose/hide anything from device tree bindings, it will
> not change the clk driver logic.


Best regards,
Krzysztof
  

On Tue, Mar 14, 2023 at 05:37:04PM +0100, Krzysztof Kozlowski wrote:
> On 14/03/2023 16:56, Dmitry Rokosov wrote:
> > On Tue, Mar 14, 2023 at 04:40:19PM +0100, neil.armstrong@linaro.org wrote:
> >> On 14/03/2023 16:37, Krzysztof Kozlowski wrote:
> >>> On 14/03/2023 16:33, neil.armstrong@linaro.org wrote:
> >>>>> 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@linaro.org/
> >>>>
> >>>> I don't see what's different with this one.
> >>>
> >>> So you are aware that all undocumented clock IDs are still allowed to
> >>> use in DTS and they are ABI? Changing them will be an ABI break.
> >>
> >> Yes of course.
> >>
> >> Neil
> >>
> >>>
> >>> Best regards,
> >>> Krzysztof
> >>>
> >>
> > 
> > Sorry, guys, I'm little bit confused.
> > In the discussion pointed by Neil not-by-one-increment ID with public and
> > private parts are acked by Krzysztof due to explicit explanation in the
> > gxbb header. Have I to comment out my situation and stay it as is?
> 
> I did not NAK your solution here. I just pointed my usual remarks that
> it has certain outcome and minuses (undocumented ABI). But it is OK.
> 

Got it, thank you.

> > 
> > BTW, I think changing IDs value would not affect logic, because
> > it's not connected to driver logic 'by values', but 'by constants
> 
> You cannot change the IDs, neither their values nor the names (with
> exceptions). IDs - so the numbers - are ABI.
> 
> "Constant names" - I assume you mean the names of defines - do not exist
> after preprocessing, so also not really relevant here...
> 

Ah, you mean the situation when dtb blob is old and module or kernel
image is new, so ABI is broken. Yep, agree with you.

[...]
  

On Tue, Mar 14, 2023 at 02:48:25PM +0300, 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:
> 
> [...]
> 
> > > +#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:

Why 2.0+? The kernel's default license is 2.0-only. Are you (and 
your lawyer) okay with GPL v4?

But this is still part of the DT binding and has the same license 
preference:
 
>     # SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause

However, the header licenses are complicated due to .dts licenses which 
are all over the place. The requirement is dual licensed and matching 
what includes it.

Rob
  

On Fri, Mar 17, 2023 at 01:53:17PM -0500, Rob Herring wrote:
> On Tue, Mar 14, 2023 at 02:48:25PM +0300, 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:
> > 
> > [...]
> > 
> > > > +#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:
> 
> Why 2.0+? The kernel's default license is 2.0-only. Are you (and 
> your lawyer) okay with GPL v4?
> 
> But this is still part of the DT binding and has the same license 
> preference:
>  
> >     # SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> 
> However, the header licenses are complicated due to .dts licenses which 
> are all over the place. The requirement is dual licensed and matching 
> what includes it.

Agree with you. As we discussed with Krzysztof, checkpatch must verify
such wrong license tags. I've introduced the patchset for that, please
take a look:

https://lore.kernel.org/all/20230317201621.15518-1-ddrokosov@sberdevices.ru/