Message ID | 20230601053546.9574-18-nikita.shubin@maquefel.me |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:994d:0:b0:3d9:f83d:47d9 with SMTP id k13csp71150vqr; Wed, 31 May 2023 22:47:19 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ6+qzyPTU8veEze4kX4HoFfclO2j2gTzPJQbJ6K6YWnSSfWlvy4IV2yQM4orQOYuuaEEZGG X-Received: by 2002:a05:620a:3e87:b0:75b:23a1:3659 with SMTP id tv7-20020a05620a3e8700b0075b23a13659mr7208948qkn.26.1685598438761; Wed, 31 May 2023 22:47:18 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1685598438; cv=none; d=google.com; s=arc-20160816; b=c7wQejRge0mJXL1TpR9VBZX+lFvcwH7w/RAIJI1VOv0ZEKZH6hKeMV1jwzdzhpU/Gn K9kTAyxZyyC67+493LSPg+EXleVrB4oMNBmmEN1we19UwiXbiMfAOgafymjnfbGMDmMO sVGgV7ubY0HSwkfTjgNLEOvZqVhnXp3hRQJ+LBeoTTQjoLaMMPDfxOm00YLO5VqEjnfg 7B5La8ZQqLYpDve65nXUinWv+nZxi2WLo+dHgT9QSPRN9sKMIUrWRQAZX0I69b9paMow TazyAfCTm1neckVu3PtBk9FToLBkexjNNkFNKHTq4UumC8wEyMZjdw/xid+YEgSqt4Tv U0Og== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=MVrfFhDDrzP+QZ7AKJkzp8cNrCZ+hvwz6QWWmxQNar0=; b=Nw+X66x6l6AqpyjGCq4aReBs431Kdg5MaEJU6qxUsSwy129haxYtX+HHH7g/wmK2xN S3lT6quiH3GrDJqbiJ0Z5LQrRQjr49p5mGumdWhi6bNce6AjAN1d0tBdhtNHb3rnjEe/ NV1OAWJuPce7J5SuqJRyhP/yahp0QzhDsshP73MJJPlT3piqXoWgWGst+1VaRoozoBDY MTxoDBMmX8vA3blMLdW/c4XQIE7CJD7PJUJtd4l7QngVvpulXazycin+LMotnRGR3uSU dGqGo4BZFLS2j+mIzsIU54nmywYsWchwS87EHUId7M4aPFGytcilcY1y+wXOSPjub7/w O4vQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@maquefel.me header.s=mail header.b="DJFo/I2f"; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id l2-20020a622502000000b006466813eba9si4906970pfl.47.2023.05.31.22.47.07; Wed, 31 May 2023 22:47:18 -0700 (PDT) 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; Authentication-Results: mx.google.com; dkim=pass header.i=@maquefel.me header.s=mail header.b="DJFo/I2f"; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231484AbjFAFjy (ORCPT <rfc822;limurcpp@gmail.com> + 99 others); Thu, 1 Jun 2023 01:39:54 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47626 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231493AbjFAFj0 (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Thu, 1 Jun 2023 01:39:26 -0400 Received: from forward103b.mail.yandex.net (forward103b.mail.yandex.net [178.154.239.150]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D13D71717; Wed, 31 May 2023 22:38:36 -0700 (PDT) Received: from mail-nwsmtp-smtp-production-main-45.sas.yp-c.yandex.net (mail-nwsmtp-smtp-production-main-45.sas.yp-c.yandex.net [IPv6:2a02:6b8:c14:c83:0:640:84f9:0]) by forward103b.mail.yandex.net (Yandex) with ESMTP id 86BCF6003B; Thu, 1 Jun 2023 08:37:17 +0300 (MSK) Received: by mail-nwsmtp-smtp-production-main-45.sas.yp-c.yandex.net (smtp/Yandex) with ESMTPSA id OaGNfZvWv8c0-HGdKrmol; Thu, 01 Jun 2023 08:37:17 +0300 X-Yandex-Fwd: 1 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=maquefel.me; s=mail; t=1685597837; bh=MVrfFhDDrzP+QZ7AKJkzp8cNrCZ+hvwz6QWWmxQNar0=; h=Message-Id:Date:In-Reply-To:Cc:Subject:References:To:From; b=DJFo/I2fNChDDfRzST21CjxcKfb4LS5XQ1cSS5uw6T32cKkAvmFsPl2ixOz8y5qvu /FAKYFKIbPTarwSqkk9Jy6PD219xLzGgNeS3iRCwRPOYbdv2FgnAOEiT9/iDRtSdR2 xQuZBBxsv6TpYBuH5lXtRWtaWrXSDJsWq0Ayep4E= Authentication-Results: mail-nwsmtp-smtp-production-main-45.sas.yp-c.yandex.net; dkim=pass header.i=@maquefel.me From: Nikita Shubin <nikita.shubin@maquefel.me> To: Alexander Sverdlin <alexander.sverdlin@gmail.com>, Arnd Bergmann <arnd@arndb.de>, Linus Walleij <linus.walleij@linaro.org>, Mark Brown <broonie@kernel.org>, Rob Herring <robh+dt@kernel.org>, Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>, Nikita Shubin <nikita.shubin@maquefel.me> Cc: Michael Peters <mpeters@embeddedTS.com>, Kris Bahnsen <kris@embeddedTS.com>, linux-spi@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH v1 17/43] dt-bindings: spi: Add Cirrus EP93xx Date: Thu, 1 Jun 2023 08:34:08 +0300 Message-Id: <20230601053546.9574-18-nikita.shubin@maquefel.me> X-Mailer: git-send-email 2.37.4 In-Reply-To: <20230424123522.18302-1-nikita.shubin@maquefel.me> References: <20230424123522.18302-1-nikita.shubin@maquefel.me> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,SPF_HELO_NONE,SPF_PASS, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1767478068574436744?= X-GMAIL-MSGID: =?utf-8?q?1767478068574436744?= |
Series |
None
|
|
Commit Message
Nikita Shubin
June 1, 2023, 5:34 a.m. UTC
Add YAML bindings for ep93xx SoC SPI.
Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
---
Notes:
v0 -> v1:
Krzysztof Kozlowski:
- replaced maintainers
- removed wildcards
- use fallback compatible and list all possible compatibles
- drop quotes in ref
- dropped "clock-names"
- dropped label
- fix ident
.../devicetree/bindings/spi/spi-ep9301.yaml | 69 +++++++++++++++++++
1 file changed, 69 insertions(+)
create mode 100644 Documentation/devicetree/bindings/spi/spi-ep9301.yaml
Comments
On 01/06/2023 07:34, Nikita Shubin wrote: > Add YAML bindings for ep93xx SoC SPI. > > Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me> > --- > > Notes: > v0 -> v1: > Krzysztof Kozlowski: > - replaced maintainers > - removed wildcards > - use fallback compatible and list all possible compatibles > - drop quotes in ref > - dropped "clock-names" > - dropped label > - fix ident > > .../devicetree/bindings/spi/spi-ep9301.yaml | 69 +++++++++++++++++++ > 1 file changed, 69 insertions(+) > create mode 100644 Documentation/devicetree/bindings/spi/spi-ep9301.yaml > > diff --git a/Documentation/devicetree/bindings/spi/spi-ep9301.yaml b/Documentation/devicetree/bindings/spi/spi-ep9301.yaml > new file mode 100644 > index 000000000000..c363b25a3074 > --- /dev/null > +++ b/Documentation/devicetree/bindings/spi/spi-ep9301.yaml > @@ -0,0 +1,69 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/spi/spi-ep9301.yaml# Filename based on compatible, so missing prefix, wrong order of name components. This applies everywhere, not to some files only. Applied to all your bindings. > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: EP93xx SoC SPI controller > + > +maintainers: > + - Alexander Sverdlin <alexander.sverdlin@gmail.com> > + - Nikita Shubin <nikita.shubin@maquefel.me> > + > +allOf: > + - $ref: spi-controller.yaml# > + > +properties: > + "#address-cells": true > + "#size-cells": true Drop these two. > + > + compatible: Anyway, compatible is always first. > + oneOf: > + - const: cirrus,ep9301-spi > + - items: > + - enum: > + - cirrus,ep9302-spi > + - cirrus,ep9307-spi > + - cirrus,ep9312-spi > + - cirrus,ep9315-spi > + - const: cirrus,ep9301-spi > + > + reg: > + items: > + - description: SPI registers region > + > + interrupts: > + maxItems: 1 > + > + clocks: > + items: > + - description: SPI Controller reference clock source > + > + cs-gpios: true Drop, not needed. > + > + cirrus,ep9301-use-dma: > + description: Flag indicating that the SPI should use dma > + type: boolean In such case where are dmas? Unless you meant some internal dma controller? In such case extend the description because now it just duplicates property name. > + > +required: > + - compatible > + - reg > + - interrupts > + - clocks > + > +unevaluatedProperties: false > + > +examples: > + - | > + #include <dt-bindings/clock/cirrus,ep93xx-clock.h> > + spi@808a0000 { > + compatible = "cirrus,ep9301-spi"; > + reg = <0x808a0000 0x18>; > + interrupt-parent = <&vic1>; > + interrupts = <21>; > + clocks = <&syscon EP93XX_CLK_SPI>; > + cs-gpios = <&gpio5 2 0>; Use proper gpio defines for flags. > + cirrus,ep9301-use-dma; > + }; > + > +... Best regards, Krzysztof
On Thu, Jun 01, 2023 at 08:34:08AM +0300, Nikita Shubin wrote: > + cirrus,ep9301-use-dma: > + description: Flag indicating that the SPI should use dma > + type: boolean My previous feedback on this property still applies. Please don't ignore review comments, people are generally making them for a reason and are likely to have the same concerns if issues remain unaddressed. Having to repeat the same comments can get repetitive and make people question the value of time spent reviewing. If you disagree with the review comments that's fine but you need to reply and discuss your concerns so that the reviewer can understand your decisions.
Hello Mark! On Thu, 1 Jun 2023 12:17:27 +0100 Mark Brown <broonie@kernel.org> wrote: > On Thu, Jun 01, 2023 at 08:34:08AM +0300, Nikita Shubin wrote: > > > + cirrus,ep9301-use-dma: > > + description: Flag indicating that the SPI should use dma > > + type: boolean > > My previous feedback on this property still applies. > > Please don't ignore review comments, people are generally making them > for a reason and are likely to have the same concerns if issues remain > unaddressed. Having to repeat the same comments can get repetitive > and make people question the value of time spent reviewing. If you > disagree with the review comments that's fine but you need to reply > and discuss your concerns so that the reviewer can understand your > decisions. Sorry - that was totally unintentional, i was tinkering with spi and got distracted on other part of this series (it's quite big for me, first time tinkering with a series more than 5-6 patches). > > + cirrus,ep9301-use-dma: The reason is that ep93xx DMA state is not quite device-tree ready at this moment, and clients use it with the help of: https://elixir.bootlin.com/linux/v6.4-rc4/source/include/linux/platform_data/dma-ep93xx.h I was hoping to slip by without changing much in ep93xx DMA driver, so i can deal with it later, especially seeing it's having some quirks like: https://elixir.bootlin.com/linux/v6.4-rc4/source/drivers/dma/ep93xx_dma.c#L471 And edb93xx and bk3 don't set use_dma with SPI for some reason. I can move "use-dma" to module parameters, if this is acceptable.
On Thu, Jun 01, 2023 at 03:41:54PM +0300, Nikita Shubin wrote: > Mark Brown <broonie@kernel.org> wrote: > > On Thu, Jun 01, 2023 at 08:34:08AM +0300, Nikita Shubin wrote: > > > + cirrus,ep9301-use-dma: > > > + description: Flag indicating that the SPI should use dma > > > + type: boolean > > My previous feedback on this property still applies. > > > + cirrus,ep9301-use-dma: > The reason is that ep93xx DMA state is not quite device-tree ready at > this moment, and clients use it with the help of: > https://elixir.bootlin.com/linux/v6.4-rc4/source/include/linux/platform_data/dma-ep93xx.h > I was hoping to slip by without changing much in ep93xx DMA driver, so You're definign new ABI here, that's not a good thing to do for a temporary workaround. > I can move "use-dma" to module parameters, if this is acceptable. That's less bad. I guess you could also define the bindings for the DMA controller so that the properties are there then instead of properly using the DMA API in the clients just check to see if the DMA properties are present and then proceed accordingly?
On Thu, 1 Jun 2023 13:55:03 +0100 Mark Brown <broonie@kernel.org> wrote: > On Thu, Jun 01, 2023 at 03:41:54PM +0300, Nikita Shubin wrote: > > Mark Brown <broonie@kernel.org> wrote: > > > On Thu, Jun 01, 2023 at 08:34:08AM +0300, Nikita Shubin wrote: > > > > > + cirrus,ep9301-use-dma: > > > > + description: Flag indicating that the SPI should use dma > > > > + type: boolean > > > > My previous feedback on this property still applies. > > > > > + cirrus,ep9301-use-dma: > > > The reason is that ep93xx DMA state is not quite device-tree ready > > at this moment, and clients use it with the help of: > > > https://elixir.bootlin.com/linux/v6.4-rc4/source/include/linux/platform_data/dma-ep93xx.h > > > > > I was hoping to slip by without changing much in ep93xx DMA driver, > > so > > You're definign new ABI here, that's not a good thing to do for a > temporary workaround. > > > I can move "use-dma" to module parameters, if this is acceptable. > > That's less bad. I guess you could also define the bindings for the > DMA controller so that the properties are there then instead of > properly using the DMA API in the clients just check to see if the > DMA properties are present and then proceed accordingly? This sounds like a way to go. Thank you, Mark!
diff --git a/Documentation/devicetree/bindings/spi/spi-ep9301.yaml b/Documentation/devicetree/bindings/spi/spi-ep9301.yaml new file mode 100644 index 000000000000..c363b25a3074 --- /dev/null +++ b/Documentation/devicetree/bindings/spi/spi-ep9301.yaml @@ -0,0 +1,69 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/spi/spi-ep9301.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: EP93xx SoC SPI controller + +maintainers: + - Alexander Sverdlin <alexander.sverdlin@gmail.com> + - Nikita Shubin <nikita.shubin@maquefel.me> + +allOf: + - $ref: spi-controller.yaml# + +properties: + "#address-cells": true + "#size-cells": true + + compatible: + oneOf: + - const: cirrus,ep9301-spi + - items: + - enum: + - cirrus,ep9302-spi + - cirrus,ep9307-spi + - cirrus,ep9312-spi + - cirrus,ep9315-spi + - const: cirrus,ep9301-spi + + reg: + items: + - description: SPI registers region + + interrupts: + maxItems: 1 + + clocks: + items: + - description: SPI Controller reference clock source + + cs-gpios: true + + cirrus,ep9301-use-dma: + description: Flag indicating that the SPI should use dma + type: boolean + +required: + - compatible + - reg + - interrupts + - clocks + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/clock/cirrus,ep93xx-clock.h> + spi@808a0000 { + compatible = "cirrus,ep9301-spi"; + reg = <0x808a0000 0x18>; + interrupt-parent = <&vic1>; + interrupts = <21>; + clocks = <&syscon EP93XX_CLK_SPI>; + cs-gpios = <&gpio5 2 0>; + cirrus,ep9301-use-dma; + }; + +...