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;
Content-Type: text/plain;
        charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 12.4 \(3445.104.21\))
Subject: Re: [PATCH v2 1/2] dt-bindings: dwc2: Add bindings for new Ingenic
 SoCs.
From:   "H. Nikolaus Schaller" <hns@goldelico.com>
In-Reply-To: <c79a8ff7-7a3f-9627-f910-dbbf942e34cb@wanyeetech.com>
Date:   Wed, 13 Apr 2022 21:30:18 +0200
Cc:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        hminas@synopsys.com, Rob Herring <robh+dt@kernel.org>,
        linux-usb@vger.kernel.org, linux-mips <linux-mips@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS 
        <devicetree@vger.kernel.org>, dragancecavac@yahoo.com,
        dongsheng.qiu@ingenic.com, qipengzhen <aric.pzqi@ingenic.com>,
        rick.tyliu@ingenic.com, sernia.zhou@foxmail.com,
        zhenwenjin@gmail.com, reimu@sudomaker.com
Content-Transfer-Encoding: quoted-printable
Message-Id: <0AE74BF9-46F1-44EC-8E5F-40EA12851AD0@goldelico.com>
References: <1649788201-87620-1-git-send-email-zhouyanjie@wanyeetech.com>
 <1649788201-87620-2-git-send-email-zhouyanjie@wanyeetech.com>
 <6F03670F-9040-4560-AD78-CC7A03EC678F@goldelico.com>
 <c79a8ff7-7a3f-9627-f910-dbbf942e34cb@wanyeetech.com>
To:     Zhou Yanjie <zhouyanjie@wanyeetech.com>,
        Krzysztof Kozlowski <krzk+dt@kernel.org>
Precedence: bulk

Hi,


> Am 13.04.2022 um 20:55 schrieb Zhou Yanjie =
<zhouyanjie@wanyeetech.com>:
>=20
> Hi Nikolaus,
>=20
> On 2022/4/13 =E4=B8=8B=E5=8D=883:22, H. Nikolaus Schaller wrote:
>> Hi,
>>=20
>>=20
>>> Am 12.04.2022 um 20:30 schrieb =E5=91=A8=E7=90=B0=E6=9D=B0 (Zhou =
Yanjie) <zhouyanjie@wanyeetech.com>
>>> :
>>>=20
>>> Add the dwc2 bindings for the JZ4775 SoC, the JZ4780 SoC, the X1000
>>> SoC, the X1600 SoC, the X1700 SoC, the X1830 SoC, and the X2000 SoC
>>> from Ingenic.
>>>=20
>>> Signed-off-by: =E5=91=A8=E7=90=B0=E6=9D=B0 (Zhou Yanjie)=20
>>> <zhouyanjie@wanyeetech.com>
>>>=20
>>> Acked-by: Rob Herring=20
>>> <robh@kernel.org>
>>>=20
>>> ---
>>>=20
>>> Notes:
>>>    v1->v2:
>>>    Add Rob Herring's Acked-by.
>>>=20
>>> Documentation/devicetree/bindings/usb/dwc2.yaml | 7 +++++++
>>> 1 file changed, 7 insertions(+)
>>>=20
>>> diff --git a/Documentation/devicetree/bindings/usb/dwc2.yaml =
b/Documentation/devicetree/bindings/usb/dwc2.yaml
>>> index 4cebce6..c6e8c0b 100644
>>> --- a/Documentation/devicetree/bindings/usb/dwc2.yaml
>>> +++ b/Documentation/devicetree/bindings/usb/dwc2.yaml
>>> @@ -17,6 +17,13 @@ properties:
>>>     oneOf:
>>>       - const: brcm,bcm2835-usb
>>>       - const: hisilicon,hi6220-usb
>>> +      - const: ingenic,jz4775-otg
>>> +      - const: ingenic,jz4780-otg
>>> +      - const: ingenic,x1000-otg
>>> +      - const: ingenic,x1600-otg
>>> +      - const: ingenic,x1700-otg
>>> +      - const: ingenic,x1830-otg
>>> +      - const: ingenic,x2000-otg
>>>=20
>> I have merged it with my recently proposed removal of
>> ingenic,jz4780-otg in jz4780.dtsi but there was no dtbscheck
>> complaint about missing snps,dwc2.
>>=20
>> So I think should it be:
>>=20
>>       - items:
>>           - enum:
>>               - const: ingenic,jz4775-otg
>>               - const: ingenic,jz4780-otg
>>               - const: ingenic,x1000-otg
>>               - const: ingenic,x1600-otg
>>               - const: ingenic,x1700-otg
>>               - const: ingenic,x1830-otg
>>               - const: ingenic,x2000-otg
>>=20

PS: the const: above should be removed (I hadn't run it through the =
compiler).

>>           - const: snps,dwc2

here it is needed.

>>=20
>> similar to the entry for amlogic?
>>=20
>=20
>=20
> Or we can just remove the "snps,dwc2" from jz4780.dtsi?

Well, my recent proposal to fix dtbscheck was the other way round:
remove "ingenic,jz4780-otg" from jz4780.dtsi and leave it out here.

> I'm not too sure, but since we already have a dedicated "ingenic, =
jz4780-otg", it seems "snps,dwc2" is redundant.

As far as I see there is no driver specialization compatible to
"ingenic,jz4780-otg". `grep ingenic,jz4780-otg *` only shows the .dtsi =
(and the new .yaml).

So we need "snps,dwc2" to get any driver match and I thought the =
"ingenic,jz4780-otg" is redundant.

But maintainers convinced me to keep it as a dummy compatible in the =
.dtsi for potential future
specialization (which does not exist and seems not to be necessary). =
Unless I can convince them=20
that this is never ever needed. Which is beyond my knowledge and almost =
everyone.

So we can't remove the "snps,dwc2" here.

Well, we can with more work elsewhere.
You have to extend the dwc2_of_match_table to include all ingenic =
devices.

Therefore we now know 3 potential solutions:
a) remove "ingenic,jz4780-otg" from jz4780.dtsi (my proposal)
b) add "ingenic,jz4780-otg" to dwc2.yaml together with "snps,dwc2" (your =
proposal + my suggestion here)
c) add only "ingenic,jz4780-otg" to dwc2.yaml and extend the match table =
in drivers//usb/dwc2/params.c (new proposals)

=46rom consistency point of view I think variant b) is the right one. a) =
was rejected and c) only adds redundant code.

I am open to anything as long as the dtbscheck doesn't complain any =
more.

BR an thanks,
Nikolaus