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 36C33C48BF6 for ; Tue, 5 Mar 2024 02:27:52 +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:MIME-Version:Subject:References: In-Reply-To:Message-ID: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=eV7ydeF0k8B75ZCThewQGAJgcrjNFMnv1VZFvY1tUWM=; b=i9iBVmYilRELC7h81MX2ytroZG 7KKQ5/hF4+DsEx6japhM79nFsh/YyAW74rsjKzR8X47S3vruMO94NdTl+HDSVXIGhP70JpMHZvoCl ZR6pYiVPYpOgh99+RuXHnFrzd3SbSNPbgDAnnEUz1P8b+rgBZqZol2MhCkEAyAAQbtd0WrE2DOYin 3sFXvUvzxKPASKYpGMAjrTUt63qN0pCMz3GwH03vuV/2OGvV7lPIKLL2D5cAYRyHATTO+sWFj0Xp8 ccIvc2V2K9d3GinXYGdTM01/y17rAOKxPQ0w89EimIjJu0+0VDd9vi9vbeli5Mv2jDeyrPJiARUBV 6aMToGvQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1rhKWa-0000000BYU5-3aI9; Tue, 05 Mar 2024 02:27:24 +0000 Received: from mail.savoirfairelinux.com ([208.88.110.44]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1rhKWU-0000000BYRo-2gCT for linux-arm-kernel@lists.infradead.org; Tue, 05 Mar 2024 02:27:22 +0000 Received: from localhost (localhost [127.0.0.1]) by mail.savoirfairelinux.com (Postfix) with ESMTP id 76D0A9C422B; Mon, 4 Mar 2024 21:27:06 -0500 (EST) Received: from mail.savoirfairelinux.com ([127.0.0.1]) by localhost (mail.savoirfairelinux.com [127.0.0.1]) (amavis, port 10032) with ESMTP id LGf-g4yRHTxB; Mon, 4 Mar 2024 21:27:05 -0500 (EST) Received: from localhost (localhost [127.0.0.1]) by mail.savoirfairelinux.com (Postfix) with ESMTP id 0BB709C49D5; Mon, 4 Mar 2024 21:27:05 -0500 (EST) DKIM-Filter: OpenDKIM Filter v2.10.3 mail.savoirfairelinux.com 0BB709C49D5 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=savoirfairelinux.com; s=DFC430D2-D198-11EC-948E-34200CB392D2; t=1709605625; bh=RkpXqE9XRyAqX18Vp/lUDrOt5pgpgCQl2D2nYPx3pu4=; h=Date:From:To:Message-ID:MIME-Version; b=GUErl1RZTHwnh1cCXYyrTLnOG7pBh5dYSOokQmPBXjcA/NVfrnrbE+5ZSB7nuK8xH 14D8RdtDRK6SXg1S2fx6LN8DwDJq4+9MlDH12kt2TMMR+wsIuEirC78hHW7CjQZF08 mF7B6CyC8F6pLng4IplU5fdHqn9tBO8/ZM1J403rrFm0eAjWG08NZOgPVsQD+VAJUs 7yRXIIDQLPk1qNWuGmWezon2kJKuEGuTWiNqyGkiquzuxuGUXS3mmMfziC/kSpV1lD VV5QVAbP0zw80BMaJ/XGsOC4R9mGVtwcj/G78b9IRUPFN8KaE89WBmXsOEu3jWohfV /5SqLMWVCv7lA== X-Virus-Scanned: amavis at mail.savoirfairelinux.com Received: from mail.savoirfairelinux.com ([127.0.0.1]) by localhost (mail.savoirfairelinux.com [127.0.0.1]) (amavis, port 10026) with ESMTP id LOzQ1csyfsyJ; Mon, 4 Mar 2024 21:27:04 -0500 (EST) Received: from mail.savoirfairelinux.com (mail.savoirfairelinux.com [192.168.48.237]) by mail.savoirfairelinux.com (Postfix) with ESMTP id C232B9C422B; Mon, 4 Mar 2024 21:27:04 -0500 (EST) Date: Mon, 4 Mar 2024 21:27:04 -0500 (EST) From: Charles Perry To: Krzysztof Kozlowski , yilun xu Cc: Rob Herring , mdf , Allen VANDIVER , Brian CODY , hao wu , Tom Rix , krzysztof kozlowski+dt , Conor Dooley , Michal Simek , linux-fpga , devicetree , linux-kernel , linux-arm-kernel Message-ID: <23887452.1534761.1709605624728.JavaMail.zimbra@savoirfairelinux.com> In-Reply-To: References: <20240221195058.1281973-1-charles.perry@savoirfairelinux.com> <20240221195058.1281973-3-charles.perry@savoirfairelinux.com> <4a9f0eef-590b-45df-92bc-b63ad9282e18@linaro.org> <1012793477.1508198.1709486517581.JavaMail.zimbra@savoirfairelinux.com> Subject: Re: [PATCH v4 2/3] dt-bindings: fpga: xlnx,fpga-selectmap: add DT schema MIME-Version: 1.0 X-Mailer: Zimbra 8.8.15_GA_4581 (ZimbraWebClient - FF123 (Linux)/8.8.15_GA_4581) Thread-Topic: dt-bindings: fpga: xlnx,fpga-selectmap: add DT schema Thread-Index: rJyWbF4Dp/Vu1Z8peWGRMOSlsVPpnQ== X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240304_182718_923179_4D185AED X-CRM114-Status: GOOD ( 24.99 ) 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: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Mar 4, 2024, at 12:31 AM, Krzysztof Kozlowski krzysztof.kozlowski@linaro.org wrote: > On 04/03/2024 08:30, Krzysztof Kozlowski wrote: >> On 03/03/2024 18:21, Charles Perry wrote: >>> On Feb 27, 2024, at 3:10 AM, Krzysztof Kozlowski krzysztof.kozlowski@linaro.org >>> wrote: >>> >>>> On 21/02/2024 20:50, Charles Perry wrote: >>>>> Document the SelectMAP interface of Xilinx 7 series FPGA. >>>>> >>>>> Signed-off-by: Charles Perry >>>>> --- >>>>> .../bindings/fpga/xlnx,fpga-selectmap.yaml | 86 +++++++++++++++++++ >>>>> 1 file changed, 86 insertions(+) >>>>> create mode 100644 >>>>> Documentation/devicetree/bindings/fpga/xlnx,fpga-selectmap.yaml >>>>> >>>>> diff --git a/Documentation/devicetree/bindings/fpga/xlnx,fpga-selectmap.yaml >>>>> b/Documentation/devicetree/bindings/fpga/xlnx,fpga-selectmap.yaml >>>>> new file mode 100644 >>>>> index 0000000000000..08a5e92781657 >>>>> --- /dev/null >>>>> +++ b/Documentation/devicetree/bindings/fpga/xlnx,fpga-selectmap.yaml >>>>> @@ -0,0 +1,86 @@ >>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>>>> +%YAML 1.2 >>>>> +--- >>>>> +$id: http://devicetree.org/schemas/fpga/xlnx,fpga-selectmap.yaml# >>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>>>> + >>>>> +title: Xilinx SelectMAP FPGA interface >>>>> + >>>>> +maintainers: >>>>> + - Charles Perry >>>>> + >>>>> +description: | >>>>> + Xilinx 7 Series FPGAs support a method of loading the bitstream over a >>>>> + parallel port named the SelectMAP interface in the documentation. Only >>>>> + the x8 mode is supported where data is loaded at one byte per rising edge of >>>>> + the clock, with the MSB of each byte presented to the D0 pin. >>>>> + >>>>> + Datasheets: >>>>> + >>>>> https://www.xilinx.com/support/documentation/user_guides/ug470_7Series_Config.pdf >>>>> + >>>>> +allOf: >>>>> + - $ref: /schemas/memory-controllers/mc-peripheral-props.yaml# >>>>> + >>>>> +properties: >>>>> + compatible: >>>>> + enum: >>>>> + - xlnx,fpga-xc7s-selectmap >>>>> + - xlnx,fpga-xc7a-selectmap >>>>> + - xlnx,fpga-xc7k-selectmap >>>>> + - xlnx,fpga-xc7v-selectmap >>>>> + >>>>> + reg: >>>>> + description: >>>>> + At least 1 byte of memory mapped IO >>>>> + maxItems: 1 >>>>> + >>>>> + prog_b-gpios: >>>> >>>> I commented on this and still see underscore. Nothing in commit msg >>>> explains why this should have underscore. Changelog is also vague - >>>> describes that you brought back underscores, instead of explaining why >>>> you did it. >>>> >>>> So the same comments as usual: >>>> >>>> No underscores in names. >>>> >>>> Best regards, >>>> Krzysztof >>> >>> Hello Krzysztof, >>> >>> Yes, I've gone full circle on that issue. Here's what I tried so far: >> >> And what part of the commit description allows me to understand this? >> I have a changelog in the cover letter: https://lore.kernel.org/all/20240221195058.1281973-1-charles.perry@savoirfairelinux.com/ >>> >>> 1) Reuse the same gpio names: Duplicates errors of the past, Krzysztof >>> doesn't like it. >>> 2) Different gpio names for new driver only: Makes the driver code >>> overly complicated, Yilun doesn't like it. >> >> That's a new driver, right? So what is complicated here? You have new >> code and you take prog-b or prog_b? >> >>> 3) Change gpio names for both drivers, deprecate the old names: Makes >>> the DT binding and the driver code overly complicated, Rob doesn't >>> like it. >> >> I don't think I proposed changing existing bindings. >> >>> >>> I think that while the driver code shouldn't be the driving force for >>> the DT spec, it can be a good indication that the spec is unpractical to >>> implement. >> >> What is impractical in implementing this? You just pass either A or B to >> function requesting GPIO. Just choose proper name. >> It's not complicated but it requires more code than if "prog_b" had been used. >>> >>> In this case, there are two interfaces on a chip that uses the same GPIO >>> protocol, it would only make sense that they use the same names, this >>> discards solution #2. >> >> I don't understand this. You have devm_gpiod_get() in your new code. Why >> is it difficult to use different name? Yilun asked to avoid changing the names between the two drivers. First comment in this mail: https://lore.kernel.org/all/Zb9GkY6cMtR+4xOX@yilunxu-OptiPlex-7050/ Yilun, let me know if this is something you'd accept as this is a concern for the device tree maintainers. > > And I forgot to emphasize: none of these is mentioned in commit msg, so > for v5 you will get exactly the same complains. And for every other > patch which repeats the same and does not clarify caveats or exceptions. > > Best regards, > Krzysztof Should I keep my changelog in the individual commits? I thought the norm was to put this the cover letter. Regards, Charles _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel