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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20230314114825.yiv4vcszr6b7m45w@CAB-WSD-L081021>
Date:   Tue, 14 Mar 2023 14:48:25 +0300
From:   Dmitry Rokosov <ddrokosov@...rdevices.ru>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
CC:     <neil.armstrong@...aro.org>, <jbrunet@...libre.com>,
        <mturquette@...libre.com>, <sboyd@...nel.org>,
        <robh+dt@...nel.org>, <krzysztof.kozlowski+dt@...aro.org>,
        <khilman@...libre.com>, <martin.blumenstingl@...glemail.com>,
        <jian.hu@...ogic.com>, <kernel@...rdevices.ru>,
        <rockosov@...il.com>, <linux-amlogic@...ts.infradead.org>,
        <linux-clk@...r.kernel.org>, <devicetree@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>
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@...ogic.com>
> > + *
> > + * Copyright (c) 2023, SberDevices. All Rights Reserved.
> > + * Author: Dmitry Rokosov <ddrokosov@...rdevices.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