All of lore.kernel.org
 help / color / mirror / Atom feed
From: Rob Herring <robh@kernel.org>
To: "Rafał Miłecki" <zajec5@gmail.com>
Cc: "Mark Brown" <broonie@kernel.org>,
	"Kamal Dasu" <kdasu.kdev@gmail.com>,
	linux-spi@vger.kernel.org, devicetree@vger.kernel.org,
	bcm-kernel-feedback-list@broadcom.com,
	"Rafał Miłecki" <rafal@milecki.pl>
Subject: Re: [PATCH V2] dt-bindings: spi: brcm,spi-bcm-qspi: convert to the json-schema
Date: Fri, 16 Apr 2021 14:27:45 -0500	[thread overview]
Message-ID: <20210416192745.GA3779932@robh.at.kernel.org> (raw)
In-Reply-To: <20210416062320.21414-1-zajec5@gmail.com>

On Fri, Apr 16, 2021 at 08:23:20AM +0200, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> This helps validating DTS files.
> 
> Changes that require mentioning:
> 1. reg-names
>    "mspi_regs" and "bspi_regs" were renamed to "mspi" and "bspi" as that
>    is what's used in DTS files and in Linux driver
> 2. interrupt-names
>    Names were reordered. "mspi_done" has to go first as it's always
>    required.
> 3. spi-rx-bus-width
>    Property description was dropped as it's part of the
>    spi-controller.yaml
> 4. Examples:
>    * flash0@0 was a duplicated node and got dropped
>    * regs and interrupts were formatted and reordered to match yaml
>    * <0x1c> was replaced with <&gic>
> 
> This rewritten binding validates cleanly using the  "dt_binding_check".
> Some Linux stored DTS files will require reordering regs and interrupts
> to make dtbs_check happy.
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
> V2: Add Kamal as maintainer
> ---
>  .../bindings/spi/brcm,spi-bcm-qspi.txt        | 245 -----------------
>  .../bindings/spi/brcm,spi-bcm-qspi.yaml       | 247 ++++++++++++++++++
>  MAINTAINERS                                   |   2 +-
>  3 files changed, 248 insertions(+), 246 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/spi/brcm,spi-bcm-qspi.txt
>  create mode 100644 Documentation/devicetree/bindings/spi/brcm,spi-bcm-qspi.yaml


> diff --git a/Documentation/devicetree/bindings/spi/brcm,spi-bcm-qspi.yaml b/Documentation/devicetree/bindings/spi/brcm,spi-bcm-qspi.yaml
> new file mode 100644
> index 000000000000..598a3984dada
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/spi/brcm,spi-bcm-qspi.yaml
> @@ -0,0 +1,247 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/spi/brcm,spi-bcm-qspi.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Broadcom SPI controller
> +
> +maintainers:
> +  - Kamal Dasu <kdasu.kdev@gmail.com>
> +  - Rafał Miłecki <rafal@milecki.pl>
> +
> +description: |
> +  The Broadcom SPI controller is a SPI master found on various SOCs, including
> +  BRCMSTB (BCM7XXX), Cygnus, NSP and NS2. The Broadcom Master SPI hw IP consits
> +  of:
> +    MSPI : SPI master controller can read and write to a SPI slave device
> +    BSPI : Broadcom SPI in combination with the MSPI hw IP provides acceleration
> +           for flash reads and be configured to do single, double, quad lane
> +           io with 3-byte and 4-byte addressing support.
> +
> +  Supported Broadcom SoCs have one instance of MSPI+BSPI controller IP.
> +  MSPI master can be used wihout BSPI. BRCMSTB SoCs have an additional instance
> +  of a MSPI master without the BSPI to use with non flash slave devices that
> +  use SPI protocol.
> +
> +allOf:
> +  - $ref: spi-controller.yaml#
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - description: Second Instance of MSPI BRCMSTB SoCs
> +        items:
> +          - enum:
> +              - brcm,spi-bcm7425-qspi
> +              - brcm,spi-bcm7429-qspi
> +              - brcm,spi-bcm7435-qspi
> +              - brcm,spi-bcm7445-qspi
> +              - brcm,spi-bcm7216-qspi
> +              - brcm,spi-bcm7278-qspi
> +          - const: brcm,spi-bcm-qspi
> +          - const: brcm,spi-brcmstb-mspi
> +      - description: Second Instance of MSPI BRCMSTB SoCs
> +        items:
> +          - enum:
> +              - brcm,spi-brcmstb-qspi
> +              - brcm,spi-brcmstb-mspi
> +              - brcm,spi-nsp-qspi
> +              - brcm,spi-ns2-qspi
> +          - const: brcm,spi-bcm-qspi
> +
> +  reg:
> +    minItems: 1
> +    maxItems: 5
> +
> +  reg-names:
> +    minItems: 1
> +    maxItems: 5
> +    items:
> +      - const: mspi
> +      - const: bspi
> +      - enum: [ intr_regs, intr_status_reg, cs_reg ]
> +      - enum: [ intr_regs, intr_status_reg, cs_reg ]
> +      - enum: [ intr_regs, intr_status_reg, cs_reg ]
> +
> +  interrupts:
> +    minItems: 1
> +    maxItems: 7
> +
> +  interrupt-names:
> +    oneOf:
> +      - minItems: 1
> +        maxItems: 7
> +        items:
> +          - const: mspi_done
> +          - const: mspi_halted
> +          - const: spi_lr_fullness_reached
> +          - const: spi_lr_session_aborted
> +          - const: spi_lr_impatient
> +          - const: spi_lr_session_done
> +          - const: spi_lr_overread
> +      - const: spi_l1_intr
> +
> +  clocks:
> +    maxItems: 1
> +    description: reference clock for this block
> +
> +  native-endian:
> +    $ref: /schemas/types.yaml#/definitions/flag
> +    description: Defined when using BE SoC and device uses BE register read/write
> +
> +unevaluatedProperties: false
> +
> +required:
> +  - reg
> +  - reg-names
> +  - interrupts
> +  - interrupt-names
> +
> +examples:
> +  - | # BRCMSTB SoC: SPI Master (MSPI+BSPI) for SPI-NOR access
> +    spi@f03e3400 {
> +            compatible = "brcm,spi-brcmstb-qspi", "brcm,spi-bcm-qspi";
> +            reg = <0xf03e3400 0x188>, <0xf03e3200 0x50>, <0xf03e0920 0x4>;
> +            reg-names = "mspi", "bspi", "cs_reg";
> +            interrupts = <0x5 0x6 0x1 0x2 0x3 0x4 0x0>;

Reformat with <> around each interrupt.

> +            interrupt-parent = <&gic>;
> +            interrupt-names = "mspi_done",
> +                              "mspi_halted",
> +                              "spi_lr_fullness_reached",
> +                              "spi_lr_session_aborted",
> +                              "spi_lr_impatient",
> +                              "spi_lr_session_done",
> +                              "spi_lr_overread";
> +            clocks = <&hif_spi>;
> +            clock-names = "sw_spi";

Looks like you have random clock-names...

> +            #address-cells = <0x1>;
> +            #size-cells = <0x0>;
> +
> +            m25p80@0 {

flash@0

> +                    #size-cells = <0x2>;
> +                    #address-cells = <0x2>;
> +                    compatible = "m25p80";
> +                    reg = <0x0>;
> +                    spi-max-frequency = <0x2625a00>;
> +                    spi-cpol;
> +                    spi-cpha;
> +                    m25p,fast-read;
> +
> +                    flash0.bolt@0 {
> +                        reg = <0x0 0x0 0x0 0x100000>;
> +                    };
> +
> +                    flash0.macadr@100000 {
> +                        reg = <0x0 0x100000 0x0 0x10000>;
> +                    };
> +
> +                    flash0.nvram@110000 {
> +                        reg = <0x0 0x110000 0x0 0x10000>;
> +                    };
> +
> +                    flash0.kernel@120000 {
> +                        reg = <0x0 0x120000 0x0 0x400000>;
> +                    };
> +
> +                    flash0.devtree@520000 {
> +                        reg = <0x0 0x520000 0x0 0x10000>;
> +                    };
> +
> +                    flash0.splash@530000 {
> +                        reg = <0x0 0x530000 0x0 0x80000>;
> +                    };
> +            };
> +    };
> +  - | # BRCMSTB SoC: MSPI master for any SPI device
> +    spi@f0416000 {
> +            clocks = <&upg_fixed>;
> +            compatible = "brcm,spi-brcmstb-mspi", "brcm,spi-bcm-qspi";
> +            reg = <0xf0416000 0x180>;
> +            reg-names = "mspi";
> +            interrupts = <0x14>;
> +            interrupt-parent = <&irq0_aon_intc>;
> +            interrupt-names = "mspi_done";
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +    };
> +  - | # iProc SoC
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +    spi@18027200 {
> +            compatible = "brcm,spi-nsp-qspi", "brcm,spi-bcm-qspi";
> +            reg = <0x18027200 0x184>,
> +                  <0x18027000 0x124>,
> +                  <0x1811c408 0x004>,
> +                  <0x180273a0 0x01c>;
> +            reg-names = "mspi", "bspi", "intr_regs", "intr_status_reg";
> +            interrupts = <GIC_SPI 77 IRQ_TYPE_LEVEL_HIGH>,
> +                         <GIC_SPI 78 IRQ_TYPE_LEVEL_HIGH>,
> +                         <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>,
> +                         <GIC_SPI 73 IRQ_TYPE_LEVEL_HIGH>,
> +                         <GIC_SPI 74 IRQ_TYPE_LEVEL_HIGH>,
> +                         <GIC_SPI 75 IRQ_TYPE_LEVEL_HIGH>,
> +                         <GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH>;
> +            interrupt-names = "mspi_done",
> +                              "mspi_halted",
> +                              "spi_lr_fullness_reached",
> +                              "spi_lr_session_aborted",
> +                              "spi_lr_impatient",
> +                              "spi_lr_session_done";
> +            clocks = <&iprocmed>;
> +            clock-names = "iprocmed";
> +            num-cs = <2>;
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +    };
> +  - | # NS2 SoC
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +    qspi: spi@66470200 {

Drop any unused labels.

> +            compatible = "brcm,spi-ns2-qspi", "brcm,spi-bcm-qspi";
> +            reg = <0x66470200 0x184>,
> +                  <0x66470000 0x124>,
> +                  <0x67017408 0x004>,
> +                  <0x664703a0 0x01c>;
> +            reg-names = "mspi", "bspi", "intr_regs", "intr_status_reg";
> +            interrupts = <GIC_SPI 419 IRQ_TYPE_LEVEL_HIGH>;
> +            interrupt-names = "spi_l1_intr";
> +            clocks = <&iprocmed>;
> +            clock-names = "iprocmed";
> +            num-cs = <2>;
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +
> +            flash: m25p80@0 {

flash@0

> +                    #address-cells = <1>;
> +                    #size-cells = <1>;
> +                    compatible = "m25p80";
> +                    reg = <0x0>;
> +                    spi-max-frequency = <12500000>;
> +                    m25p,fast-read;
> +                    spi-cpol;
> +                    spi-cpha;
> +
> +                    partition@0 {

Perhaps show the new, preferred form with a 'partitions' node. Or just 
drop as we have examples elsewhere.

> +                        label = "boot";
> +                        reg = <0x00000000 0x000a0000>;
> +                    };
> +
> +                    partition@a0000 {
> +                        label = "env";
> +                        reg = <0x000a0000 0x00060000>;
> +                    };
> +
> +                    partition@100000 {
> +                        label = "system";
> +                        reg = <0x00100000 0x00600000>;
> +                    };
> +
> +                    partition@700000 {
> +                        label = "rootfs";
> +                        reg = <0x00700000 0x01900000>;
> +                    };
> +            };
> +    };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a1c2b56f4f90..6fb31a735a0f 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -3696,7 +3696,7 @@ BROADCOM SPI DRIVER
>  M:	Kamal Dasu <kdasu.kdev@gmail.com>
>  M:	bcm-kernel-feedback-list@broadcom.com
>  S:	Maintained
> -F:	Documentation/devicetree/bindings/spi/brcm,spi-bcm-qspi.txt
> +F:	Documentation/devicetree/bindings/spi/brcm,spi-bcm-qspi.yaml
>  F:	drivers/spi/spi-bcm-qspi.*
>  F:	drivers/spi/spi-brcmstb-qspi.c
>  F:	drivers/spi/spi-iproc-qspi.c
> -- 
> 2.26.2
> 

  reply	other threads:[~2021-04-16 19:27 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-15 22:02 [PATCH] dt-bindings: spi: brcm,spi-bcm-qspi: convert to the json-schema Rafał Miłecki
2021-04-16  0:32 ` Florian Fainelli
2021-04-16  6:21   ` Rafał Miłecki
2021-04-16  6:23 ` [PATCH V2] " Rafał Miłecki
2021-04-16 19:27   ` Rob Herring [this message]
2021-04-16 19:47   ` [PATCH V3] " Rafał Miłecki
2021-04-20 16:21     ` Rob Herring
2021-04-20 18:35     ` Mark Brown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210416192745.GA3779932@robh.at.kernel.org \
    --to=robh@kernel.org \
    --cc=bcm-kernel-feedback-list@broadcom.com \
    --cc=broonie@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=kdasu.kdev@gmail.com \
    --cc=linux-spi@vger.kernel.org \
    --cc=rafal@milecki.pl \
    --cc=zajec5@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.