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 4B49CC27C4F for ; Sun, 23 Jun 2024 13:09:26 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=mkrgzJ7j5kelPRC1rhd0MZCM9ihiRsoeLjKj9esEsv4=; b=13gVC9k7lL7+X5 hQhUNeKj0dtsdaqBZjQGj0X4LOaUaS7ADL/ZuVz8dHYlHBNRsQEWkLI4PrHNY6wYaUMeITE6q70b0 nTKW8GhIfx2vCDIIgIgoOXKYkd3NvYthm91DSv1b/WjtgWAAAfUZiQmulTcVQQIFjkrJlohG+uA2c EVCnrgnovKnTr6dicNmIy1T2nIXiMoa+5bmpu4wEwgtR0siQL/BfWfzIPjCYEQScyJ/e4C8XB9gB4 JmsRGSOS/RMi8hx0WEMeH7Pa4EfUfApLB2zVPmN46pD5mXbDZj4c0TX7sqsse+fx/ipCaR6vzav9F UlYEPL2BzJIppk7z3ZwQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sLMy6-0000000E2HN-2FAk; Sun, 23 Jun 2024 13:09:18 +0000 Received: from pidgin.makrotopia.org ([2a07:2ec0:3002::65]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1sLMxy-0000000E2G8-1rxs; Sun, 23 Jun 2024 13:09:11 +0000 Received: from local by pidgin.makrotopia.org with esmtpsa (TLS1.3:TLS_AES_256_GCM_SHA384:256) (Exim 4.97.1) (envelope-from ) id 1sLMxY-0000000081f-4C5p; Sun, 23 Jun 2024 13:08:45 +0000 Date: Sun, 23 Jun 2024 14:08:35 +0100 From: Daniel Golle To: Krzysztof Kozlowski Cc: Aurelien Jarno , Olivia Mackall , Herbert Xu , Rob Herring , Krzysztof Kozlowski , Conor Dooley , Heiko Stuebner , Philipp Zabel , Uwe =?iso-8859-1?Q?Kleine-K=F6nig?= , Sebastian Reichel , Sascha Hauer , Dragan Simic , Martin Kaiser , Ard Biesheuvel , linux-crypto@vger.kernel.org, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-rockchip@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v4 1/3] dt-bindings: rng: Add Rockchip RNG bindings Message-ID: References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240623_060910_507194_71C77F26 X-CRM114-Status: GOOD ( 24.25 ) X-BeenThere: linux-rockchip@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Upstream kernel work for Rockchip platforms List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "Linux-rockchip" Errors-To: linux-rockchip-bounces+linux-rockchip=archiver.kernel.org@lists.infradead.org Hi Krzysztof, thank you for your patiente and repeated review of this series. On Sun, Jun 23, 2024 at 09:03:15AM +0200, Krzysztof Kozlowski wrote: > On 23/06/2024 05:32, Daniel Golle wrote: > > From: Aurelien Jarno > > > > Add the True Random Number Generator on the Rockchip RK3568 SoC. > > > > Signed-off-by: Aurelien Jarno > > Signed-off-by: Daniel Golle > > My comments from v2, which I reminded at v3, were not addressed. > > Respond to each of them and acknowledge that you are going to implement > the change. Your comments to v1which I'm aware of are: https://patchwork.kernel.org/comment/25087874/ > > +++ b/Documentation/devicetree/bindings/rng/rockchip-rng.yaml > Filename matching compatible, so "rockchip,rk3568-rng.yaml" I've changed the filename. > > +title: Rockchip TRNG bindings > Drop "bindings" I've changed the title accordingly (now: "Rockchip TRNG" in v4). > > +description: > > + This driver interface with the True Random Number Generator present in some > > Drop "This driver interface" and make it a proper sentence. Bindings are > not about drivers. This has been addressed by Aurelien and further improved by me in v3. > > + clocks: > > + minItems: 2 > Drop minItems. Aurelien did that in v2. > > + clock-names: > > + items: > > + - const: clk > > + - const: hclk > > You need to explain what are these in clocks. Also you need better > names. A clock name "clk" is useless. Clocks now have meaningful names and descriptions. > > + reset-names: > > + items: > > + - const: reset > > Drop reset-names entirely, not useful. Aurelien did so in v2. Your comments to v2 which I'm aware of are: https://patchwork.kernel.org/comment/25111597/ > > Add the RNG bindings for the RK3568 SoC from Rockchip > Use subject prefixes matching the subsystem (git log --oneline -- ...), > so it is rng, not RNG. Also, you are not adding all-Rockhip RNG but a > specific device. > > Subject: drop second, redundant "bindings". I've changed 'RNG' into 'rng' in the subject and spelled it out in the commit message. > > +description: True Random Number Generator for some Rockchip SoCs > > s/for some Rockchip SoCs/on Rokchip RK3568 SoC/ I've adopted your suggestion in v3 and then fixed the typo in v4. > > > + clock-names: > > + items: > > + - const: trng_clk > > + - const: trng_hclk > These are too vague names. Everything is a clk in clock-names, so no > need usually to add it as name suffix. Give them some descriptive names, > e.g. core and ahb. If changed the names to the suggested 'core' and 'ahb'. Before sending another round of patches, just to make sure we are on the same page, please confirm that what remains is Subject: dt-bindings: rng: Add Rockchip RNG bindings which not only should be 'rng' in small letters but also name the exact chip, eg.: Subject: dt-bindings: rng: add TRNG on the Rockchip RK3568 SoC If there are any other comments you made which I'm not aware of, please point me to them. Cheers Daniel _______________________________________________ Linux-rockchip mailing list Linux-rockchip@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-rockchip