Received-SPF: pass (google.com: domain of linux-kernel+bounces-45356-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <c2adae73-9f1b-467e-880d-3e87eaeb10ea@gmail.com>
Date: Tue, 30 Jan 2024 22:57:47 +0100
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 2/3] dt-bindings: arm: mediatek: convert PCIESYS to the
 json-schema
Content-Language: en-US
To: Rob Herring <robh@kernel.org>,
 AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
 Conor Dooley <conor+dt@kernel.org>, Matthias Brugger
 <matthias.bgg@gmail.com>, devicetree@vger.kernel.org,
 linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
 linux-mediatek@lists.infradead.org, linux-kernel@vger.kernel.org,
 =?UTF-8?B?UmFmYcWCIE1pxYJlY2tp?= <rafal@milecki.pl>,
 Russell King <linux@armlinux.org.uk>,
 Daniel Lezcano <daniel.lezcano@linaro.org>,
 Thomas Gleixner <tglx@linutronix.de>
References: <20240123082100.7334-1-zajec5@gmail.com>
 <20240123082100.7334-3-zajec5@gmail.com>
 <e17b85b1-7f1f-4b60-89b7-43f560466cc2@collabora.com>
 <20240130203413.GA2290196-robh@kernel.org>
From: =?UTF-8?B?UmFmYcWCIE1pxYJlY2tp?= <zajec5@gmail.com>
In-Reply-To: <20240130203413.GA2290196-robh@kernel.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit

On 30.01.2024 21:34, Rob Herring wrote:
> On Tue, Jan 23, 2024 at 12:20:29PM +0100, AngeloGioacchino Del Regno wrote:
>> Il 23/01/24 09:20, Rafał Miłecki ha scritto:
>>> From: Rafał Miłecki <rafal@milecki.pl>
>>>
>>> This helps validating DTS files. Introduced changes:
>>> 1. Documented "reg" property
>>> 2. Adjusted "reg" in example
>>>
>>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>>> ---
>>>    .../arm/mediatek/mediatek,mt7622-pciesys.yaml | 47 +++++++++++++++++++
>>>    .../arm/mediatek/mediatek,pciesys.txt         | 25 ----------
>>>    2 files changed, 47 insertions(+), 25 deletions(-)
>>>    create mode 100644 Documentation/devicetree/bindings/arm/mediatek/mediatek,mt7622-pciesys.yaml
>>>    delete mode 100644 Documentation/devicetree/bindings/arm/mediatek/mediatek,pciesys.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt7622-pciesys.yaml b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt7622-pciesys.yaml
>>> new file mode 100644
>>> index 000000000000..7340a2512402
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt7622-pciesys.yaml
>>
>> I think that we should really move all those clock controller yaml files to their
>> proper directory, which would be
>>
>> Documentation/devicetree/bindings/clock/
>>
>> ...because those are clock controllers anyway and the fact that they do also
>> provide a reset controller doesn't really justify having them in arm/mediatek.
>>
>> Besides, I would appreciate if you could also move mt8186/92/95 and eventual
>> others that are there to clock/.
> 
> Yes, please move it.
> 
>>
>>> @@ -0,0 +1,47 @@
>>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/arm/mediatek/mediatek,mt7622-pciesys.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: MediaTek PCIESYS controller
>>> +
>>> +description:
>>> +  The MediaTek PCIESYS controller provides various clocks to the system.
>>> +
>>> +maintainers:
>>> +  - Matthias Brugger <matthias.bgg@gmail.com>
>>> +
>>> +properties:
>>> +  compatible:
>>> +    items:
>>> +      - enum:
>>> +          - mediatek,mt7622-pciesys
>>> +          - mediatek,mt7629-pciesys
>>> +      - const: syscon
>>
>> I know that there's syscon all over the place and, even if I admit I didn't check,
>> I am fairly sure that there's absolutely no reason to have syscon there, and that
>> the syscon compatible never did anything for (most of, or all of) those clock
>> controllers, at all.
>>
>> I'm not sure - though - if removing syscon during the txt->yaml conversion is
>> acceptable (yeah we'd be cheating a bit), but something makes me say it is, because
>> the bindings couldn't validate before that one as well.
> 
> As long as you state why you are removing it in the commit msg.
> 
>>
>> Of course you'd have to remove the syscon compatible from the affected device trees
>> as well as omitting it here.
> 
> You could also do 'minItems: 1' and 'deprecated' in the 2nd item.

I expected this to be quite straightforward but I can't get it to work.

Can you tell me if I'm making some mistake or is that some meta schema
limitation?

properties:
   compatible:
     oneOf:
       - minItems: 1
         items:
           - enum:
               - mediatek,mt2701-hifsys
               - mediatek,mt7622-hifsys
           - const: syscon
             deprecated: true
       - minItems: 2
         items:
           - enum:
               - mediatek,mt7623-hifsys
           - const: mediatek,mt2701-hifsys
           - const: syscon
             deprecated: true

Gives me:

Documentation/devicetree/bindings/clock/mediatek,mt2701-hifsys.yaml: properties:compatible:oneOf:0:items: 'oneOf' conditional failed, one must be fixed:
         [{'enum': ['mediatek,mt2701-hifsys', 'mediatek,mt7622-hifsys']}, {'const': 'syscon', 'deprecated': True}] is not of type 'object'
         Additional properties are not allowed ('deprecated' was unexpected)
         from schema $id: http://devicetree.org/meta-schemas/string-array.yaml#
Documentation/devicetree/bindings/clock/mediatek,mt2701-hifsys.yaml: properties:compatible:oneOf:1:items: 'oneOf' conditional failed, one must be fixed:
         [{'enum': ['mediatek,mt7623-hifsys']}, {'const': 'mediatek,mt2701-hifsys'}, {'const': 'syscon', 'deprecated': True}] is not of type 'object'
         Additional properties are not allowed ('deprecated' was unexpected)
         from schema $id: http://devicetree.org/meta-schemas/string-array.yaml#