[1/2] dt-bindings: spi: Add Baikal-T1 System Boot SPI Controller binding

Commit Message

Serge Semin May 8, 2020, 9:36 a.m. UTC

Baikal-T1 Boot SPI is a part of the SoC System Controller and is
responsible for the system bootup from an external SPI flash. It's a DW
APB SSI-based SPI-controller with no interrupts, no DMA, with just one
native chip-select available and a single reference clock. Since Baikal-T1
SoC is normally booted up from an external SPI flash this SPI controller
in most of the cases is supposed to be connected to a single SPI-nor
flash. Additionally in order to provide a transparent from CPU point of
view initial code execution procedure the system designers created an IP
block which physically maps the SPI flash found at CS0 to a memory region.

Co-developed-by: Ramil Zaripov <[e-mail]>
Signed-off-by: Ramil Zaripov <[e-mail]>
Signed-off-by: Serge Semin <[e-mail]>
Cc: Alexey Malahov <[e-mail]>
Cc: Thomas Bogendoerfer <[e-mail]>
Cc: Paul Burton <[e-mail]>
Cc: Ralf Baechle <[e-mail]>
Cc: John Garry <[e-mail]>
Cc: Chuanhong Guo <[e-mail]>
Cc: Tomer Maimon <[e-mail]>
Cc: Lee Jones <[e-mail]>
Cc: Miquel Raynal <[e-mail]>
Cc: Arnd Bergmann <[e-mail]>
Cc: [e-mail]
Cc: [e-mail]
---
 .../bindings/spi/baikal,bt1-sys-ssi.yaml      | 100 ++++++++++++++++++
 1 file changed, 100 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/spi/baikal,bt1-sys-ssi.yaml

Comments

Rob Herring May 18, 2020, 3:26 p.m. UTC | #1

On Fri, May 08, 2020 at 12:36:20PM +0300, Serge Semin wrote:
> Baikal-T1 Boot SPI is a part of the SoC System Controller and is
> responsible for the system bootup from an external SPI flash. It's a DW
> APB SSI-based SPI-controller with no interrupts, no DMA, with just one
> native chip-select available and a single reference clock. Since Baikal-T1
> SoC is normally booted up from an external SPI flash this SPI controller
> in most of the cases is supposed to be connected to a single SPI-nor
> flash. Additionally in order to provide a transparent from CPU point of
> view initial code execution procedure the system designers created an IP
> block which physically maps the SPI flash found at CS0 to a memory region.
> 
> Co-developed-by: Ramil Zaripov <[e-mail]>
> Signed-off-by: Ramil Zaripov <[e-mail]>
> Signed-off-by: Serge Semin <[e-mail]>
> Cc: Alexey Malahov <[e-mail]>
> Cc: Thomas Bogendoerfer <[e-mail]>
> Cc: Paul Burton <[e-mail]>
> Cc: Ralf Baechle <[e-mail]>
> Cc: John Garry <[e-mail]>
> Cc: Chuanhong Guo <[e-mail]>
> Cc: Tomer Maimon <[e-mail]>
> Cc: Lee Jones <[e-mail]>
> Cc: Miquel Raynal <[e-mail]>
> Cc: Arnd Bergmann <[e-mail]>
> Cc: [e-mail]
> Cc: [e-mail]
> ---
>  .../bindings/spi/baikal,bt1-sys-ssi.yaml      | 100 ++++++++++++++++++
>  1 file changed, 100 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/spi/baikal,bt1-sys-ssi.yaml
> 
> diff --git a/Documentation/devicetree/bindings/spi/baikal,bt1-sys-ssi.yaml b/Documentation/devicetree/bindings/spi/baikal,bt1-sys-ssi.yaml
> new file mode 100644
> index 000000000000..d9d3257d78f4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/spi/baikal,bt1-sys-ssi.yaml
> @@ -0,0 +1,100 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright (C) 2020 BAIKAL ELECTRONICS, JSC
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/spi/baikal,bt1-sys-ssi.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Baikal-T1 System Boot SSI Controller
> +
> +description: |
> +  Baikal-T1 System Controller includes a Boot SPI Controller, which is
> +  responsible for loading chip bootup code from an external SPI flash. In order
> +  to do this transparently from CPU point of view there is a dedicated IP block
> +  mapping the 16MB flash to a dedicated MMIO range. The controller is based on
> +  the DW APB SSI IP-core but equipped with very limited resources: no IRQ,
> +  no DMA, a single native CS being necessarily connected to a 16MB SPI flash
> +  (otherwise the system won't bootup from the flash), internal Tx/Rx FIFO of
> +  just 8 bytes depth. Access to DW APB SSI controller registers is mutually
> +  exclusive from normal MMIO interface and from physically mapped SPI Flash
> +  memory. So either one or another way of using the controller functionality
> +  can be enabled at a time.
> +
> +maintainers:
> +  - Serge Semin <[e-mail]>
> +
> +allOf:
> +  - $ref: spi-controller.yaml#
> +
> +properties:
> +  compatible:
> +    const: baikal,bt1-sys-ssi
> +
> +  reg:
> +    items:
> +      - description: Baikal-T1 Boot Controller configuration registers
> +      - description: Physically mapped SPI flash ROM found at CS0
> +
> +  reg-names:
> +    items:
> +      - const: config
> +      - const: map
> +
> +  clocks:
> +    description: SPI Controller reference clock source

Can drop this. 

> +    maxItems: 1
> +
> +  clock-names:
> +    items:
> +      - const: ssi_clk
> +
> +  num-cs:
> +    const: 1
> +
> +patternProperties:
> +  "^.*@[0-9a-f]+":
> +    type: object
> +    properties:
> +      reg:
> +        minimum: 0
> +        maximum: 0
> +
> +      spi-rx-bus-width:
> +        const: 1
> +
> +      spi-tx-bus-width:
> +        const: 1

What's the point of these 2 properties if they aren't required?

> +
> +unevaluatedProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +  - reg-names

> +  - "#address-cells"
> +  - "#size-cells"

These 2 are required by spi-controller.yaml, so you can drop here.

> +  - clocks
> +
> +examples:
> +  - |
> +    spi@1f040000 {
> +      compatible = "baikal,bt1-sys-ssi";
> +      reg = <0x1f040000 0x1000>,
> +            <0x1c000000 0x1000000>;
> +      reg-names = "config", "map";
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      clocks = <&ccu_sys>;
> +      clock-names = "ssi_clk";
> +
> +      boot_flash: flash@0 {
> +        compatible = "jedec,spi-nor";
> +        #address-cells = <1>;
> +        #size-cells = <1>;
> +        reg = <0>;
> +
> +        spi-max-frequency = <25000000>;
> +      };
> +    };
> +...
> -- 
> 2.25.1
>

Serge Semin May 18, 2020, 9:27 p.m. UTC | #2

On Mon, May 18, 2020 at 09:26:59AM -0600, Rob Herring wrote:
> On Fri, May 08, 2020 at 12:36:20PM +0300, Serge Semin wrote:
> > Baikal-T1 Boot SPI is a part of the SoC System Controller and is
> > responsible for the system bootup from an external SPI flash. It's a DW
> > APB SSI-based SPI-controller with no interrupts, no DMA, with just one
> > native chip-select available and a single reference clock. Since Baikal-T1
> > SoC is normally booted up from an external SPI flash this SPI controller
> > in most of the cases is supposed to be connected to a single SPI-nor
> > flash. Additionally in order to provide a transparent from CPU point of
> > view initial code execution procedure the system designers created an IP
> > block which physically maps the SPI flash found at CS0 to a memory region.
> > 
> > Co-developed-by: Ramil Zaripov <[e-mail]>
> > Signed-off-by: Ramil Zaripov <[e-mail]>
> > Signed-off-by: Serge Semin <[e-mail]>
> > Cc: Alexey Malahov <[e-mail]>
> > Cc: Thomas Bogendoerfer <[e-mail]>
> > Cc: Paul Burton <[e-mail]>
> > Cc: Ralf Baechle <[e-mail]>
> > Cc: John Garry <[e-mail]>
> > Cc: Chuanhong Guo <[e-mail]>
> > Cc: Tomer Maimon <[e-mail]>
> > Cc: Lee Jones <[e-mail]>
> > Cc: Miquel Raynal <[e-mail]>
> > Cc: Arnd Bergmann <[e-mail]>
> > Cc: [e-mail]
> > Cc: [e-mail]
> > ---
> >  .../bindings/spi/baikal,bt1-sys-ssi.yaml      | 100 ++++++++++++++++++
> >  1 file changed, 100 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/spi/baikal,bt1-sys-ssi.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/spi/baikal,bt1-sys-ssi.yaml b/Documentation/devicetree/bindings/spi/baikal,bt1-sys-ssi.yaml
> > new file mode 100644
> > index 000000000000..d9d3257d78f4
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/spi/baikal,bt1-sys-ssi.yaml
> > @@ -0,0 +1,100 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +# Copyright (C) 2020 BAIKAL ELECTRONICS, JSC
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/spi/baikal,bt1-sys-ssi.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Baikal-T1 System Boot SSI Controller
> > +
> > +description: |
> > +  Baikal-T1 System Controller includes a Boot SPI Controller, which is
> > +  responsible for loading chip bootup code from an external SPI flash. In order
> > +  to do this transparently from CPU point of view there is a dedicated IP block
> > +  mapping the 16MB flash to a dedicated MMIO range. The controller is based on
> > +  the DW APB SSI IP-core but equipped with very limited resources: no IRQ,
> > +  no DMA, a single native CS being necessarily connected to a 16MB SPI flash
> > +  (otherwise the system won't bootup from the flash), internal Tx/Rx FIFO of
> > +  just 8 bytes depth. Access to DW APB SSI controller registers is mutually
> > +  exclusive from normal MMIO interface and from physically mapped SPI Flash
> > +  memory. So either one or another way of using the controller functionality
> > +  can be enabled at a time.
> > +
> > +maintainers:
> > +  - Serge Semin <[e-mail]>
> > +
> > +allOf:
> > +  - $ref: spi-controller.yaml#
> > +
> > +properties:
> > +  compatible:
> > +    const: baikal,bt1-sys-ssi
> > +
> > +  reg:
> > +    items:
> > +      - description: Baikal-T1 Boot Controller configuration registers
> > +      - description: Physically mapped SPI flash ROM found at CS0
> > +
> > +  reg-names:
> > +    items:
> > +      - const: config
> > +      - const: map
> > +
> > +  clocks:
> > +    description: SPI Controller reference clock source
> 
> Can drop this.

Ok.
 
> 
> > +    maxItems: 1
> > +
> > +  clock-names:
> > +    items:
> > +      - const: ssi_clk
> > +
> > +  num-cs:
> > +    const: 1
> > +
> > +patternProperties:
> > +  "^.*@[0-9a-f]+":
> > +    type: object
> > +    properties:
> > +      reg:
> > +        minimum: 0
> > +        maximum: 0
> > +
> > +      spi-rx-bus-width:
> > +        const: 1
> > +
> > +      spi-tx-bus-width:
> > +        const: 1
> 
> What's the point of these 2 properties if they aren't required?

Yes, they are optional, but this is a constraint on the bus-width parameters.
DW APB SSI provides a single laned Tx and Rx.

> 
> > +
> > +unevaluatedProperties: false
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - reg-names
> 
> > +  - "#address-cells"
> > +  - "#size-cells"
> 
> These 2 are required by spi-controller.yaml, so you can drop here.

Yes, "#address-cells" is required, but "#size-cells" isn't. Is this supposed to
be like that?

-Sergey

> 
> > +  - clocks
> > +
> > +examples:
> > +  - |
> > +    spi@1f040000 {
> > +      compatible = "baikal,bt1-sys-ssi";
> > +      reg = <0x1f040000 0x1000>,
> > +            <0x1c000000 0x1000000>;
> > +      reg-names = "config", "map";
> > +      #address-cells = <1>;
> > +      #size-cells = <0>;
> > +
> > +      clocks = <&ccu_sys>;
> > +      clock-names = "ssi_clk";
> > +
> > +      boot_flash: flash@0 {
> > +        compatible = "jedec,spi-nor";
> > +        #address-cells = <1>;
> > +        #size-cells = <1>;
> > +        reg = <0>;
> > +
> > +        spi-max-frequency = <25000000>;
> > +      };
> > +    };
> > +...
> > -- 
> > 2.25.1
> >

Serge Semin May 18, 2020, 9:55 p.m. UTC | #3

On Mon, May 18, 2020 at 09:26:59AM -0600, Rob Herring wrote:
> On Fri, May 08, 2020 at 12:36:20PM +0300, Serge Semin wrote:
> > Baikal-T1 Boot SPI is a part of the SoC System Controller and is
> > responsible for the system bootup from an external SPI flash. It's a DW
> > APB SSI-based SPI-controller with no interrupts, no DMA, with just one
> > native chip-select available and a single reference clock. Since Baikal-T1
> > SoC is normally booted up from an external SPI flash this SPI controller
> > in most of the cases is supposed to be connected to a single SPI-nor
> > flash. Additionally in order to provide a transparent from CPU point of
> > view initial code execution procedure the system designers created an IP
> > block which physically maps the SPI flash found at CS0 to a memory region.

BTW Rob, could you also provide your review for the next patch:
https://lore.kernel.org/linux-mips/[e-mail]/
?

We agreed with Mark to merge this driver into the generic DW APB SSI code so to
be available for another platforms. Since our DW APB SSI IP's got too many
peculiarities I'll have to provide a new compatible string. It would be great to
add it into the DW schema binding instead of currently available text-based
legacy binding file.

[nip]

Rob Herring May 21, 2020, 2:57 p.m. UTC | #4

On Mon, May 18, 2020 at 3:27 PM Serge Semin
<[e-mail]> wrote:
>
> On Mon, May 18, 2020 at 09:26:59AM -0600, Rob Herring wrote:
> > On Fri, May 08, 2020 at 12:36:20PM +0300, Serge Semin wrote:
> > > Baikal-T1 Boot SPI is a part of the SoC System Controller and is
> > > responsible for the system bootup from an external SPI flash. It's a DW
> > > APB SSI-based SPI-controller with no interrupts, no DMA, with just one
> > > native chip-select available and a single reference clock. Since Baikal-T1
> > > SoC is normally booted up from an external SPI flash this SPI controller
> > > in most of the cases is supposed to be connected to a single SPI-nor
> > > flash. Additionally in order to provide a transparent from CPU point of
> > > view initial code execution procedure the system designers created an IP
> > > block which physically maps the SPI flash found at CS0 to a memory region.
> > >
> > > Co-developed-by: Ramil Zaripov <[e-mail]>
> > > Signed-off-by: Ramil Zaripov <[e-mail]>
> > > Signed-off-by: Serge Semin <[e-mail]>
> > > Cc: Alexey Malahov <[e-mail]>
> > > Cc: Thomas Bogendoerfer <[e-mail]>
> > > Cc: Paul Burton <[e-mail]>
> > > Cc: Ralf Baechle <[e-mail]>
> > > Cc: John Garry <[e-mail]>
> > > Cc: Chuanhong Guo <[e-mail]>
> > > Cc: Tomer Maimon <[e-mail]>
> > > Cc: Lee Jones <[e-mail]>
> > > Cc: Miquel Raynal <[e-mail]>
> > > Cc: Arnd Bergmann <[e-mail]>
> > > Cc: [e-mail]
> > > Cc: [e-mail]
> > > ---
> > >  .../bindings/spi/baikal,bt1-sys-ssi.yaml      | 100 ++++++++++++++++++
> > >  1 file changed, 100 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/spi/baikal,bt1-sys-ssi.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/spi/baikal,bt1-sys-ssi.yaml b/Documentation/devicetree/bindings/spi/baikal,bt1-sys-ssi.yaml
> > > new file mode 100644
> > > index 000000000000..d9d3257d78f4
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/spi/baikal,bt1-sys-ssi.yaml
> > > @@ -0,0 +1,100 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +# Copyright (C) 2020 BAIKAL ELECTRONICS, JSC
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/spi/baikal,bt1-sys-ssi.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Baikal-T1 System Boot SSI Controller
> > > +
> > > +description: |
> > > +  Baikal-T1 System Controller includes a Boot SPI Controller, which is
> > > +  responsible for loading chip bootup code from an external SPI flash. In order
> > > +  to do this transparently from CPU point of view there is a dedicated IP block
> > > +  mapping the 16MB flash to a dedicated MMIO range. The controller is based on
> > > +  the DW APB SSI IP-core but equipped with very limited resources: no IRQ,
> > > +  no DMA, a single native CS being necessarily connected to a 16MB SPI flash
> > > +  (otherwise the system won't bootup from the flash), internal Tx/Rx FIFO of
> > > +  just 8 bytes depth. Access to DW APB SSI controller registers is mutually
> > > +  exclusive from normal MMIO interface and from physically mapped SPI Flash
> > > +  memory. So either one or another way of using the controller functionality
> > > +  can be enabled at a time.
> > > +
> > > +maintainers:
> > > +  - Serge Semin <[e-mail]>
> > > +
> > > +allOf:
> > > +  - $ref: spi-controller.yaml#
> > > +
> > > +properties:
> > > +  compatible:
> > > +    const: baikal,bt1-sys-ssi
> > > +
> > > +  reg:
> > > +    items:
> > > +      - description: Baikal-T1 Boot Controller configuration registers
> > > +      - description: Physically mapped SPI flash ROM found at CS0
> > > +
> > > +  reg-names:
> > > +    items:
> > > +      - const: config
> > > +      - const: map
> > > +
> > > +  clocks:
> > > +    description: SPI Controller reference clock source
> >
> > Can drop this.
>
> Ok.
>
> >
> > > +    maxItems: 1
> > > +
> > > +  clock-names:
> > > +    items:
> > > +      - const: ssi_clk
> > > +
> > > +  num-cs:
> > > +    const: 1
> > > +
> > > +patternProperties:
> > > +  "^.*@[0-9a-f]+":
> > > +    type: object
> > > +    properties:
> > > +      reg:
> > > +        minimum: 0
> > > +        maximum: 0
> > > +
> > > +      spi-rx-bus-width:
> > > +        const: 1
> > > +
> > > +      spi-tx-bus-width:
> > > +        const: 1
> >
> > What's the point of these 2 properties if they aren't required?
>
> Yes, they are optional, but this is a constraint on the bus-width parameters.
> DW APB SSI provides a single laned Tx and Rx.

Are you just trying to keep someone from saying 'spi-tx-bus-width: 2'
for example?

You could also say 'spi-tx-bus-width: false' here to disallow the
property. I guess the above is fine.

Rob

Serge Semin May 21, 2020, 3:11 p.m. UTC | #5

On Thu, May 21, 2020 at 08:57:10AM -0600, Rob Herring wrote:
> On Mon, May 18, 2020 at 3:27 PM Serge Semin
> <[e-mail]> wrote:
> >
> > On Mon, May 18, 2020 at 09:26:59AM -0600, Rob Herring wrote:
> > > On Fri, May 08, 2020 at 12:36:20PM +0300, Serge Semin wrote:
> > > > Baikal-T1 Boot SPI is a part of the SoC System Controller and is
> > > > responsible for the system bootup from an external SPI flash. It's a DW
> > > > APB SSI-based SPI-controller with no interrupts, no DMA, with just one
> > > > native chip-select available and a single reference clock. Since Baikal-T1
> > > > SoC is normally booted up from an external SPI flash this SPI controller
> > > > in most of the cases is supposed to be connected to a single SPI-nor
> > > > flash. Additionally in order to provide a transparent from CPU point of
> > > > view initial code execution procedure the system designers created an IP
> > > > block which physically maps the SPI flash found at CS0 to a memory region.
> > > >
> > > > Co-developed-by: Ramil Zaripov <[e-mail]>
> > > > Signed-off-by: Ramil Zaripov <[e-mail]>
> > > > Signed-off-by: Serge Semin <[e-mail]>
> > > > Cc: Alexey Malahov <[e-mail]>
> > > > Cc: Thomas Bogendoerfer <[e-mail]>
> > > > Cc: Paul Burton <[e-mail]>
> > > > Cc: Ralf Baechle <[e-mail]>
> > > > Cc: John Garry <[e-mail]>
> > > > Cc: Chuanhong Guo <[e-mail]>
> > > > Cc: Tomer Maimon <[e-mail]>
> > > > Cc: Lee Jones <[e-mail]>
> > > > Cc: Miquel Raynal <[e-mail]>
> > > > Cc: Arnd Bergmann <[e-mail]>
> > > > Cc: [e-mail]
> > > > Cc: [e-mail]
> > > > ---
> > > >  .../bindings/spi/baikal,bt1-sys-ssi.yaml      | 100 ++++++++++++++++++
> > > >  1 file changed, 100 insertions(+)
> > > >  create mode 100644 Documentation/devicetree/bindings/spi/baikal,bt1-sys-ssi.yaml
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/spi/baikal,bt1-sys-ssi.yaml b/Documentation/devicetree/bindings/spi/baikal,bt1-sys-ssi.yaml
> > > > new file mode 100644
> > > > index 000000000000..d9d3257d78f4
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/spi/baikal,bt1-sys-ssi.yaml
> > > > @@ -0,0 +1,100 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > +# Copyright (C) 2020 BAIKAL ELECTRONICS, JSC
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/spi/baikal,bt1-sys-ssi.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: Baikal-T1 System Boot SSI Controller
> > > > +
> > > > +description: |
> > > > +  Baikal-T1 System Controller includes a Boot SPI Controller, which is
> > > > +  responsible for loading chip bootup code from an external SPI flash. In order
> > > > +  to do this transparently from CPU point of view there is a dedicated IP block
> > > > +  mapping the 16MB flash to a dedicated MMIO range. The controller is based on
> > > > +  the DW APB SSI IP-core but equipped with very limited resources: no IRQ,
> > > > +  no DMA, a single native CS being necessarily connected to a 16MB SPI flash
> > > > +  (otherwise the system won't bootup from the flash), internal Tx/Rx FIFO of
> > > > +  just 8 bytes depth. Access to DW APB SSI controller registers is mutually
> > > > +  exclusive from normal MMIO interface and from physically mapped SPI Flash
> > > > +  memory. So either one or another way of using the controller functionality
> > > > +  can be enabled at a time.
> > > > +
> > > > +maintainers:
> > > > +  - Serge Semin <[e-mail]>
> > > > +
> > > > +allOf:
> > > > +  - $ref: spi-controller.yaml#
> > > > +
> > > > +properties:
> > > > +  compatible:
> > > > +    const: baikal,bt1-sys-ssi
> > > > +
> > > > +  reg:
> > > > +    items:
> > > > +      - description: Baikal-T1 Boot Controller configuration registers
> > > > +      - description: Physically mapped SPI flash ROM found at CS0
> > > > +
> > > > +  reg-names:
> > > > +    items:
> > > > +      - const: config
> > > > +      - const: map
> > > > +
> > > > +  clocks:
> > > > +    description: SPI Controller reference clock source
> > >
> > > Can drop this.
> >
> > Ok.
> >
> > >
> > > > +    maxItems: 1
> > > > +
> > > > +  clock-names:
> > > > +    items:
> > > > +      - const: ssi_clk
> > > > +
> > > > +  num-cs:
> > > > +    const: 1
> > > > +
> > > > +patternProperties:
> > > > +  "^.*@[0-9a-f]+":
> > > > +    type: object
> > > > +    properties:
> > > > +      reg:
> > > > +        minimum: 0
> > > > +        maximum: 0
> > > > +
> > > > +      spi-rx-bus-width:
> > > > +        const: 1
> > > > +
> > > > +      spi-tx-bus-width:
> > > > +        const: 1
> > >
> > > What's the point of these 2 properties if they aren't required?
> >
> > Yes, they are optional, but this is a constraint on the bus-width parameters.
> > DW APB SSI provides a single laned Tx and Rx.
> 
> Are you just trying to keep someone from saying 'spi-tx-bus-width: 2'
> for example?

Right.

> 
> You could also say 'spi-tx-bus-width: false' here to disallow the
> property. I guess the above is fine.

Ok. If it's fine I'll leave them as is then. Right?

What about the next question you've asked:

> >
> > > +
> > > +unevaluatedProperties: false
> > > +
> > > +required:
> > > +  - compatible
> > > +  - reg
> > > +  - reg-names
> >
> > > +  - "#address-cells"
> > > +  - "#size-cells"
> >
> > These 2 are required by spi-controller.yaml, so you can drop here.
> 
> Yes, "#address-cells" is required, but "#size-cells" isn't. Is this supposed to
> be like that?

As far as I can see in spi-controller.yaml, "#address-cells" is required, but
"#size-cells" isn't. Is this intentional?

-Sergey

> 
> Rob

Rob Herring May 21, 2020, 9:35 p.m. UTC | #6

On Thu, May 21, 2020 at 9:12 AM Serge Semin
<[e-mail]> wrote:
>
> On Thu, May 21, 2020 at 08:57:10AM -0600, Rob Herring wrote:
> > On Mon, May 18, 2020 at 3:27 PM Serge Semin
> > <[e-mail]> wrote:
> > >
> > > On Mon, May 18, 2020 at 09:26:59AM -0600, Rob Herring wrote:
> > > > On Fri, May 08, 2020 at 12:36:20PM +0300, Serge Semin wrote:
> > > > > Baikal-T1 Boot SPI is a part of the SoC System Controller and is
> > > > > responsible for the system bootup from an external SPI flash. It's a DW
> > > > > APB SSI-based SPI-controller with no interrupts, no DMA, with just one
> > > > > native chip-select available and a single reference clock. Since Baikal-T1
> > > > > SoC is normally booted up from an external SPI flash this SPI controller
> > > > > in most of the cases is supposed to be connected to a single SPI-nor
> > > > > flash. Additionally in order to provide a transparent from CPU point of
> > > > > view initial code execution procedure the system designers created an IP
> > > > > block which physically maps the SPI flash found at CS0 to a memory region.
> > > > >
> > > > > Co-developed-by: Ramil Zaripov <[e-mail]>
> > > > > Signed-off-by: Ramil Zaripov <[e-mail]>
> > > > > Signed-off-by: Serge Semin <[e-mail]>
> > > > > Cc: Alexey Malahov <[e-mail]>
> > > > > Cc: Thomas Bogendoerfer <[e-mail]>
> > > > > Cc: Paul Burton <[e-mail]>
> > > > > Cc: Ralf Baechle <[e-mail]>
> > > > > Cc: John Garry <[e-mail]>
> > > > > Cc: Chuanhong Guo <[e-mail]>
> > > > > Cc: Tomer Maimon <[e-mail]>
> > > > > Cc: Lee Jones <[e-mail]>
> > > > > Cc: Miquel Raynal <[e-mail]>
> > > > > Cc: Arnd Bergmann <[e-mail]>
> > > > > Cc: [e-mail]
> > > > > Cc: [e-mail]
> > > > > ---
> > > > >  .../bindings/spi/baikal,bt1-sys-ssi.yaml      | 100 ++++++++++++++++++
> > > > >  1 file changed, 100 insertions(+)
> > > > >  create mode 100644 Documentation/devicetree/bindings/spi/baikal,bt1-sys-ssi.yaml
> > > > >
> > > > > diff --git a/Documentation/devicetree/bindings/spi/baikal,bt1-sys-ssi.yaml b/Documentation/devicetree/bindings/spi/baikal,bt1-sys-ssi.yaml
> > > > > new file mode 100644
> > > > > index 000000000000..d9d3257d78f4
> > > > > --- /dev/null
> > > > > +++ b/Documentation/devicetree/bindings/spi/baikal,bt1-sys-ssi.yaml
> > > > > @@ -0,0 +1,100 @@
> > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > > +# Copyright (C) 2020 BAIKAL ELECTRONICS, JSC
> > > > > +%YAML 1.2
> > > > > +---
> > > > > +$id: http://devicetree.org/schemas/spi/baikal,bt1-sys-ssi.yaml#
> > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > > +
> > > > > +title: Baikal-T1 System Boot SSI Controller
> > > > > +
> > > > > +description: |
> > > > > +  Baikal-T1 System Controller includes a Boot SPI Controller, which is
> > > > > +  responsible for loading chip bootup code from an external SPI flash. In order
> > > > > +  to do this transparently from CPU point of view there is a dedicated IP block
> > > > > +  mapping the 16MB flash to a dedicated MMIO range. The controller is based on
> > > > > +  the DW APB SSI IP-core but equipped with very limited resources: no IRQ,
> > > > > +  no DMA, a single native CS being necessarily connected to a 16MB SPI flash
> > > > > +  (otherwise the system won't bootup from the flash), internal Tx/Rx FIFO of
> > > > > +  just 8 bytes depth. Access to DW APB SSI controller registers is mutually
> > > > > +  exclusive from normal MMIO interface and from physically mapped SPI Flash
> > > > > +  memory. So either one or another way of using the controller functionality
> > > > > +  can be enabled at a time.
> > > > > +
> > > > > +maintainers:
> > > > > +  - Serge Semin <[e-mail]>
> > > > > +
> > > > > +allOf:
> > > > > +  - $ref: spi-controller.yaml#
> > > > > +
> > > > > +properties:
> > > > > +  compatible:
> > > > > +    const: baikal,bt1-sys-ssi
> > > > > +
> > > > > +  reg:
> > > > > +    items:
> > > > > +      - description: Baikal-T1 Boot Controller configuration registers
> > > > > +      - description: Physically mapped SPI flash ROM found at CS0
> > > > > +
> > > > > +  reg-names:
> > > > > +    items:
> > > > > +      - const: config
> > > > > +      - const: map
> > > > > +
> > > > > +  clocks:
> > > > > +    description: SPI Controller reference clock source
> > > >
> > > > Can drop this.
> > >
> > > Ok.
> > >
> > > >
> > > > > +    maxItems: 1
> > > > > +
> > > > > +  clock-names:
> > > > > +    items:
> > > > > +      - const: ssi_clk
> > > > > +
> > > > > +  num-cs:
> > > > > +    const: 1
> > > > > +
> > > > > +patternProperties:
> > > > > +  "^.*@[0-9a-f]+":
> > > > > +    type: object
> > > > > +    properties:
> > > > > +      reg:
> > > > > +        minimum: 0
> > > > > +        maximum: 0
> > > > > +
> > > > > +      spi-rx-bus-width:
> > > > > +        const: 1
> > > > > +
> > > > > +      spi-tx-bus-width:
> > > > > +        const: 1
> > > >
> > > > What's the point of these 2 properties if they aren't required?
> > >
> > > Yes, they are optional, but this is a constraint on the bus-width parameters.
> > > DW APB SSI provides a single laned Tx and Rx.
> >
> > Are you just trying to keep someone from saying 'spi-tx-bus-width: 2'
> > for example?
>
> Right.
>
> >
> > You could also say 'spi-tx-bus-width: false' here to disallow the
> > property. I guess the above is fine.
>
> Ok. If it's fine I'll leave them as is then. Right?

Yes.

> > > > +unevaluatedProperties: false
> > > > +
> > > > +required:
> > > > +  - compatible
> > > > +  - reg
> > > > +  - reg-names
> > >
> > > > +  - "#address-cells"
> > > > +  - "#size-cells"
> > >
> > > These 2 are required by spi-controller.yaml, so you can drop here.
> >
> > Yes, "#address-cells" is required, but "#size-cells" isn't. Is this supposed to
> > be like that?
>
> As far as I can see in spi-controller.yaml, "#address-cells" is required, but
> "#size-cells" isn't. Is this intentional?

Humm, I guess we didn't have either one because you can have no child
node defined, but then added #address-cells for the spi-slave case.

In any case, it's all pretty well covered already because the core
schema checks that both are present and dtc has some checking too.

Rob

Patch

diff --git a/Documentation/devicetree/bindings/spi/baikal,bt1-sys-ssi.yaml b/Documentation/devicetree/bindings/spi/baikal,bt1-sys-ssi.yaml
new file mode 100644
index 000000000000..d9d3257d78f4
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/baikal,bt1-sys-ssi.yaml
@@ -0,0 +1,100 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) 2020 BAIKAL ELECTRONICS, JSC
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/spi/baikal,bt1-sys-ssi.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Baikal-T1 System Boot SSI Controller
+
+description: |
+  Baikal-T1 System Controller includes a Boot SPI Controller, which is
+  responsible for loading chip bootup code from an external SPI flash. In order
+  to do this transparently from CPU point of view there is a dedicated IP block
+  mapping the 16MB flash to a dedicated MMIO range. The controller is based on
+  the DW APB SSI IP-core but equipped with very limited resources: no IRQ,
+  no DMA, a single native CS being necessarily connected to a 16MB SPI flash
+  (otherwise the system won't bootup from the flash), internal Tx/Rx FIFO of
+  just 8 bytes depth. Access to DW APB SSI controller registers is mutually
+  exclusive from normal MMIO interface and from physically mapped SPI Flash
+  memory. So either one or another way of using the controller functionality
+  can be enabled at a time.
+
+maintainers:
+  - Serge Semin <[e-mail]>
+
+allOf:
+  - $ref: spi-controller.yaml#
+
+properties:
+  compatible:
+    const: baikal,bt1-sys-ssi
+
+  reg:
+    items:
+      - description: Baikal-T1 Boot Controller configuration registers
+      - description: Physically mapped SPI flash ROM found at CS0
+
+  reg-names:
+    items:
+      - const: config
+      - const: map
+
+  clocks:
+    description: SPI Controller reference clock source
+    maxItems: 1
+
+  clock-names:
+    items:
+      - const: ssi_clk
+
+  num-cs:
+    const: 1
+
+patternProperties:
+  "^.*@[0-9a-f]+":
+    type: object
+    properties:
+      reg:
+        minimum: 0
+        maximum: 0
+
+      spi-rx-bus-width:
+        const: 1
+
+      spi-tx-bus-width:
+        const: 1
+
+unevaluatedProperties: false
+
+required:
+  - compatible
+  - reg
+  - reg-names
+  - "#address-cells"
+  - "#size-cells"
+  - clocks
+
+examples:
+  - |
+    spi@1f040000 {
+      compatible = "baikal,bt1-sys-ssi";
+      reg = <0x1f040000 0x1000>,
+            <0x1c000000 0x1000000>;
+      reg-names = "config", "map";
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      clocks = <&ccu_sys>;
+      clock-names = "ssi_clk";
+
+      boot_flash: flash@0 {
+        compatible = "jedec,spi-nor";
+        #address-cells = <1>;
+        #size-cells = <1>;
+        reg = <0>;
+
+        spi-max-frequency = <25000000>;
+      };
+    };
+...