From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 06B95C52D6F for ; Thu, 8 Aug 2024 03:53:47 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:MIME-Version: Content-Transfer-Encoding:Content-Type:References:In-Reply-To:Date:Cc:To:From :Subject:Message-ID:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=PosnylbE4Y7ewXvd+zX7wq2nvzxbsSbmn+ew7CfQ/SM=; b=KD4ZB0jGcoavNpZF2fqfKe4Jmm +Cmh6dZEj5NC/Yb4LEQoerpYleSQ52CGgVGREddpPdY+5c2T9+j1d4DFM9focauV8RJLPzGJl0Rxt ltStSAqIX026kFd7C1yCngl+eqH9URZIJ9xF57s86rbZDsJ2ERxxkleEc7uiwj3Fspr45gsvlJYw9 ofsZK/98r5NX9YPLd1H8Qv9z81QsHqBeHSeFT9w0X+PW4DRoBk+FmjEIaD7Gpbm0K42aXEZyImiXs BxF2NaXgbaFJLKRCyBTdSFGe8t3LqRj0rto2ianQ1qiSWnRvAX/V2QruauaUvdCB4hPiM7awVdmBV i+uCLQHg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sbuDa-00000006xoG-1e87; Thu, 08 Aug 2024 03:53:38 +0000 Received: from pi.codeconstruct.com.au ([203.29.241.158] helo=codeconstruct.com.au) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1sbuD1-00000006xk0-0HZ2 for linux-arm-kernel@lists.infradead.org; Thu, 08 Aug 2024 03:53:04 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=codeconstruct.com.au; s=2022a; t=1723089176; bh=PosnylbE4Y7ewXvd+zX7wq2nvzxbsSbmn+ew7CfQ/SM=; h=Subject:From:To:Cc:Date:In-Reply-To:References; b=l2Ksm/2O4zEyMfZ/Bqws1Ow6EYrwShNr+Mci1PzlplSz5cAZXnwPOZCOW7ge7ROFT Yn54QhVOOfirUObYnPi1ClLRGBiDVpV38wh9AKXoqmNGhL+erEGZV8xi5IxRJUbYQz tOFP1OLk5rLj3uTV09U2fGE59CuTH0L82y1ox88+74/r0/RXhGVCbZ/R9Mdfk5D9wD 87XtbUIeP0LaGDqHbOG6qXD1R2c8KS3+hzxwU2HiCf4Y99t02xd9wv8IHVJKPsngFn SSL6McwgNZyoinMaYd/7mELVOzIPxWWFggu38V43zMr0laoex2QLyJKyRxZXYsuN48 /Q9RHkFq+zVwg== Received: from [192.168.68.112] (203-57-213-111.dyn.iinet.net.au [203.57.213.111]) by mail.codeconstruct.com.au (Postfix) with ESMTPSA id EA77A654E9; Thu, 8 Aug 2024 11:52:53 +0800 (AWST) Message-ID: <211294ed76c23c55518015f4beedeb6efa63d540.camel@codeconstruct.com.au> Subject: Re: [PATCH 2/2] dt-bindings: misc: aspeed,ast2400-cvic: Convert to DT schema From: Andrew Jeffery To: Krzysztof Kozlowski , Thomas Gleixner , Rob Herring , Krzysztof Kozlowski , Conor Dooley , Joel Stanley Cc: linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-aspeed@lists.ozlabs.org Date: Thu, 08 Aug 2024 13:22:53 +0930 In-Reply-To: <4d26bde0bda7cb1d44958d967c4b0c2da5b2abc4.camel@codeconstruct.com.au> References: <20240802-dt-warnings-irq-aspeed-dt-schema-v1-0-8cd4266d2094@codeconstruct.com.au> <20240802-dt-warnings-irq-aspeed-dt-schema-v1-2-8cd4266d2094@codeconstruct.com.au> <4d26bde0bda7cb1d44958d967c4b0c2da5b2abc4.camel@codeconstruct.com.au> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable User-Agent: Evolution 3.46.4-2 MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240807_205303_349959_63301814 X-CRM114-Status: GOOD ( 25.71 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Thu, 2024-08-08 at 11:36 +0930, Andrew Jeffery wrote: > On Tue, 2024-08-06 at 08:12 +0200, Krzysztof Kozlowski wrote: > > On 02/08/2024 07:36, Andrew Jeffery wrote: > > > Address warnings such as: > > >=20 > >=20 > >=20 > > > +description: > > > + The Aspeed AST2400 and AST2500 SoCs have a controller that provide= s interrupts > > > + to the ColdFire coprocessor. It's not a normal interrupt controlle= r and it > > > + would be rather inconvenient to create an interrupt tree for it, a= s it > > > + somewhat shares some of the same sources as the main ARM interrupt= controller > > > + but with different numbers. > > > + > > > + The AST2500 also supports a software generated interrupt. > > > + > > > +properties: > > > + compatible: > > > + items: > > > + - enum: > > > + - aspeed,ast2400-cvic > > > + - aspeed,ast2500-cvic > > > + - const: aspeed,cvic > > > + > > > + reg: > > > + maxItems: 1 > > > + > > > + valid-sources: > > > + $ref: /schemas/types.yaml#/definitions/uint32-array > > > + description: > > > + One cell, bitmap of support sources for the implementation. > >=20 > > maxItems: 1 > > (and drop "One cell" - no need to repeat constraints in free form text) >=20 > Ack to both. >=20 > >=20 > > BTW, for both bindings, I do not see any user in the kernel. Why is thi= s > > property needed in the DTS? >=20 > Good question. This is a hangover from when benh was involved in the > Aspeed kernel port. >=20 > Given it's specified in the prose binding and the devicetrees contain > the property I'll leaving it in for now, but I think it's something we > could consider removing down the track. >=20 > >=20 > > > + > > > + copro-sw-interrupts: > > > + $ref: /schemas/types.yaml#/definitions/uint32-array > >=20 > > uint32? I do not see anywhere usage as an array. The in-kernel driver > > explicitly reads just uint32. >=20 > You're right, and in the context of the hardware an array doesn't make > sense here. I'll switch it to a uint32. >=20 Actually, on further inspection, the description says the property should contain a list of interrupt _numbers_ (the hardware exposes 32 software drive-able interrupt bits). Given aspeed-g5.dtsi only lists the single value '1' the intent feels somewhat murky. When I wrote the reply above I had in my head that it was a bitmask like valid-sources but the description suggests that's not true. I'm not sure the described behaviour was chosen to be different to valid-sources, however, for the co-processor, index 1 is an interrupt from the main ARM core. Perhaps it felt less tedious just to write the index and not the mask. I'm going to backtrack on my reply above leave this as uint32-array with `maxItems: 32` to meet the intent of the description. If there are problems down the track we can consider the driver to have a bug with respect to the binding (I don't think there's much risk there though). Andrew