Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
References: <20230321193014.26349-1-ddrokosov@sberdevices.ru>
 <20230321193014.26349-4-ddrokosov@sberdevices.ru>
 <1jmt3yo5r0.fsf@starbuckisacylon.baylibre.com>
 <20230327105115.ury3w4xpzhcpnqjg@CAB-WSD-L081021>
 <1jilemo1r9.fsf@starbuckisacylon.baylibre.com>
 <1f68ff4f-46ab-7d9e-fa57-bb94beb4c691@linaro.org>
User-agent: mu4e 1.8.13; emacs 28.2
From:   Jerome Brunet <jbrunet@baylibre.com>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
        Dmitry Rokosov <ddrokosov@sberdevices.ru>
Cc:     neil.armstrong@linaro.org, mturquette@baylibre.com,
        sboyd@kernel.org, robh+dt@kernel.org,
        krzysztof.kozlowski+dt@linaro.org, khilman@baylibre.com,
        martin.blumenstingl@googlemail.com, jian.hu@amlogic.com,
        kernel@sberdevices.ru, rockosov@gmail.com,
        linux-amlogic@lists.infradead.org, linux-clk@vger.kernel.org,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v11 3/5] dt-bindings: clock: meson: add A1 PLL and
 Peripherals clkcs bindings
Date:   Mon, 27 Mar 2023 15:59:02 +0200
In-reply-to: <1f68ff4f-46ab-7d9e-fa57-bb94beb4c691@linaro.org>
Message-ID: <1ja5zynv3v.fsf@starbuckisacylon.baylibre.com>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk


On Mon 27 Mar 2023 at 15:41, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:

> On 27/03/2023 13:39, Jerome Brunet wrote:
>> 
>> On Mon 27 Mar 2023 at 13:51, Dmitry Rokosov <ddrokosov@sberdevices.ru> wrote:
>> 
>>> On Mon, Mar 27, 2023 at 11:51:21AM +0200, Jerome Brunet wrote:
>>>>
>>>> On Tue 21 Mar 2023 at 22:30, Dmitry Rokosov <ddrokosov@sberdevices.ru> 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.
>>>>>
>>>>> 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   | 113 ++++++++++++++++++
>>>>>  .../dt-bindings/clock/amlogic,a1-pll-clkc.h   |  21 ++++
>>>>>  5 files changed, 267 insertions(+)
>>>>>  create mode 100644 Documentation/devicetree/bindings/clock/amlogic,a1-clkc.yaml
>>>>>  create mode 100644 Documentation/devicetree/bindings/clock/amlogic,a1-pll-clkc.yaml
>>>>
>>>> There is two drivers (and 2 independent patches). There should be 2
>>>> bindings patches as well.
>>>>
>>>
>>> Before, in previous versions I had two versions, but it wasn't bisectable
>>> approach.
>> 
>> You are confusing bisectable and Rob's robot. Splitting patches is more
>> that likely to help bisect (and patches backport) - not the other way around.
>
> No, he did not confuse. Splitting patches makes the series
> non-bisectable which was visible in the past.
>
> What's more, there is no reason to have bindings patches split just
> because you split drivers. Bindings are independent of drivers - we
> write them for hardware description.

Patches should do one thing, my comment is a simple application of that.

There no reason to have a single patch provide the bindings for 2
independent pieces of HW, which those components are. If a dependency
has been set, it is one that should not be there.

They do provide inputs to one another, yes but remain independent pieces of
HW. They have a different address space and as a consequences, different
drivers

If we were being strict, it should even be seperate series.

>
>> 
>>> a1-clkc schema depends on a1-pll-clkc headers and vice versa.
>>> It means dt schemas checkers will show us failure if we split them into two
>>> patchsets.
>> 
>> Only because you are patches are not upstream yet ...
>> 
>>> I know, that we can use raw digits instead of CLKID names, but IMO it doesn't
>>> look like production schema and it requires one more patchset above the
>>> series with proper CLKID definitons usage and proper header including.
>>>
>>> BTW, there is an example of Rob's test bot failure found in the previous
>>> v10 patch series due to chicken or the egg problem.
>>> https://lore.kernel.org/all/167769997208.7087.5344356236212731922.robh@kernel.org/
>>>
>>> Please advise what's the best practice to resolve that..
>> 
>> Don't use the header in your example would solve the problem and
>> still be correct DT wise.
>> 
>> The examples are just examples, they are not required to actually
>> matches a real HW, as far as I know.
>
> Yes, that would work... or just keep them here.
>
>
> Best regards,
> Krzysztof
>
>
> _______________________________________________
> linux-amlogic mailing list
> linux-amlogic@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-amlogic