[1/2] dt-bindings: spi: Add Socionext F_OSPI controller bindings

Message ID 20221118005904.23557-2-hayashi.kunihiko@socionext.com
State New
Headers
Series Introduce Socionext F_OSPI SPI flash controller |

Commit Message

Kunihiko Hayashi Nov. 18, 2022, 12:59 a.m. UTC
  Add devicetree binding documentation for Socionext F_OSPI SPI flash
controller.

Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
---
 .../bindings/spi/socionext,f-ospi.yaml        | 99 +++++++++++++++++++
 1 file changed, 99 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/spi/socionext,f-ospi.yaml
  

Comments

Mark Brown Nov. 18, 2022, 11:39 a.m. UTC | #1
On Fri, Nov 18, 2022 at 09:59:03AM +0900, Kunihiko Hayashi wrote:

> +  socionext,cs-start-cycle:
> +  socionext,cs-end-cycle:
> +  socionext,cs-deassert-clk-cycle:

These are all generic SPI properties so we should add them
generically, on the device rather than the controller since this
is something that might vary per client device.  There was also a
core function spi_set_cs_timing() which was in earlier versions
and is about to get reintroduced.

> +  socionext,alternative-byte:
> +    description:
> +      Specify the extra bytes to transfer after address transfer.

This just doesn't seem like something the controller should be
worrying about, the device should just send whatever the device
needs sending.  

> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    items:
> +      - description: the number of bytes to transfer
> +        maximum: 4
> +      - description: value to transfer
> +        default: 0
> +      - description: bit-width to transfer
> +        enum: [0, 1, 2, 4, 8]

This is also something SPI device should set up, as far as I can
tell this should be set vis spi_mem_op.dummy.nbytes.

> +  socionext,data-swap-2byte:
> +    description:
> +      Indicates swap byte order per 2-bytes.
> +    type: boolean

> +  socionext,data-swap-4byte:
> +    description:
> +      Indicates swap byte order per 4-bytes.
> +    type: boolean

Again these should be set by the device.  I think these should be
set based on a combination of bits per word and if the host is in
big endian or little endian mode.
  
Kunihiko Hayashi Nov. 18, 2022, 2:16 p.m. UTC | #2
Hi Mark,

Thank you for your comment.

On 2022/11/18 20:39, Mark Brown wrote:
> On Fri, Nov 18, 2022 at 09:59:03AM +0900, Kunihiko Hayashi wrote:
> 
>> +  socionext,cs-start-cycle:
>> +  socionext,cs-end-cycle:
>> +  socionext,cs-deassert-clk-cycle:
> 
> These are all generic SPI properties so we should add them
> generically, on the device rather than the controller since this
> is something that might vary per client device.  There was also a
> core function spi_set_cs_timing() which was in earlier versions
> and is about to get reintroduced.

So I understand you mean that these properties should be defined like
spi-peripheral-props.yaml for the devices.

If yes, I'll drop these properties once and define our vendor-specific
"peripheral-props" in the next series.

>> +  socionext,alternative-byte:
>> +    description:
>> +      Specify the extra bytes to transfer after address transfer.
> 
> This just doesn't seem like something the controller should be
> worrying about, the device should just send whatever the device
> needs sending.

This seems to be a feature added for the special devices,
so this should be device matter.

>> +    $ref: /schemas/types.yaml#/definitions/uint32-array
>> +    items:
>> +      - description: the number of bytes to transfer
>> +        maximum: 4
>> +      - description: value to transfer
>> +        default: 0
>> +      - description: bit-width to transfer
>> +        enum: [0, 1, 2, 4, 8]
> 
> This is also something SPI device should set up, as far as I can
> tell this should be set vis spi_mem_op.dummy.nbytes.

Yes, but the controller also supports dummy cycles, and can send
extra bytes before the dummy cycles.

>> +  socionext,data-swap-2byte:
>> +    description:
>> +      Indicates swap byte order per 2-bytes.
>> +    type: boolean
> 
>> +  socionext,data-swap-4byte:
>> +    description:
>> +      Indicates swap byte order per 4-bytes.
>> +    type: boolean
> 
> Again these should be set by the device.  I think these should be
> set based on a combination of bits per word and if the host is in
> big endian or little endian mode.

I see. This feature is complicated to use, so I'll not add it here.

Thank you,

---
Best Regards
Kunihiko Hayashi
  
Mark Brown Nov. 18, 2022, 3:36 p.m. UTC | #3
On Fri, Nov 18, 2022 at 11:16:22PM +0900, Kunihiko Hayashi wrote:
> On 2022/11/18 20:39, Mark Brown wrote:
> > On Fri, Nov 18, 2022 at 09:59:03AM +0900, Kunihiko Hayashi wrote:

> > > +  socionext,cs-start-cycle:
> > > +  socionext,cs-end-cycle:
> > > +  socionext,cs-deassert-clk-cycle:

> > These are all generic SPI properties so we should add them
> > generically, on the device rather than the controller since this
> > is something that might vary per client device.  There was also a
> > core function spi_set_cs_timing() which was in earlier versions
> > and is about to get reintroduced.

> So I understand you mean that these properties should be defined like
> spi-peripheral-props.yaml for the devices.

> If yes, I'll drop these properties once and define our vendor-specific
> "peripheral-props" in the next series.

Yes, sounds good.

> > > +    $ref: /schemas/types.yaml#/definitions/uint32-array
> > > +    items:
> > > +      - description: the number of bytes to transfer
> > > +        maximum: 4
> > > +      - description: value to transfer
> > > +        default: 0
> > > +      - description: bit-width to transfer
> > > +        enum: [0, 1, 2, 4, 8]

> > This is also something SPI device should set up, as far as I can
> > tell this should be set vis spi_mem_op.dummy.nbytes.

> Yes, but the controller also supports dummy cycles, and can send
> extra bytes before the dummy cycles.

Ah, so this is some additional thing on top of dummy cycles?  I'd
not realised that.  It probably wants to be added into spi-mem I
guess.

> > > +  socionext,data-swap-2byte:
> > > +    description:
> > > +      Indicates swap byte order per 2-bytes.
> > > +    type: boolean
> > 
> > > +  socionext,data-swap-4byte:
> > > +    description:
> > > +      Indicates swap byte order per 4-bytes.
> > > +    type: boolean

> > Again these should be set by the device.  I think these should be
> > set based on a combination of bits per word and if the host is in
> > big endian or little endian mode.

> I see. This feature is complicated to use, so I'll not add it here.

That also works, someone can always add additional support later
when they have a concrete use case.
  
Kunihiko Hayashi Nov. 18, 2022, 4:41 p.m. UTC | #4
On 2022/11/19 0:36, Mark Brown wrote:
> On Fri, Nov 18, 2022 at 11:16:22PM +0900, Kunihiko Hayashi wrote:
>> On 2022/11/18 20:39, Mark Brown wrote:
>>> On Fri, Nov 18, 2022 at 09:59:03AM +0900, Kunihiko Hayashi wrote:
> 
>>>> +  socionext,cs-start-cycle:
>>>> +  socionext,cs-end-cycle:
>>>> +  socionext,cs-deassert-clk-cycle:
> 
>>> These are all generic SPI properties so we should add them
>>> generically, on the device rather than the controller since this
>>> is something that might vary per client device.  There was also a
>>> core function spi_set_cs_timing() which was in earlier versions
>>> and is about to get reintroduced.
> 
>> So I understand you mean that these properties should be defined like
>> spi-peripheral-props.yaml for the devices.
> 
>> If yes, I'll drop these properties once and define our vendor-specific
>> "peripheral-props" in the next series.
> 
> Yes, sounds good.

Okay, I'll send simple one in v2.

>>>> +    $ref: /schemas/types.yaml#/definitions/uint32-array
>>>> +    items:
>>>> +      - description: the number of bytes to transfer
>>>> +        maximum: 4
>>>> +      - description: value to transfer
>>>> +        default: 0
>>>> +      - description: bit-width to transfer
>>>> +        enum: [0, 1, 2, 4, 8]
> 
>>> This is also something SPI device should set up, as far as I can
>>> tell this should be set vis spi_mem_op.dummy.nbytes.
> 
>> Yes, but the controller also supports dummy cycles, and can send
>> extra bytes before the dummy cycles.
> 
> Ah, so this is some additional thing on top of dummy cycles?  I'd
> not realised that.  It probably wants to be added into spi-mem I
> guess.

Yes, however, it's tough to handle with spi-mem and I don't have
any use case, so drop it.

>>>> +  socionext,data-swap-2byte:
>>>> +    description:
>>>> +      Indicates swap byte order per 2-bytes.
>>>> +    type: boolean
>>>
>>>> +  socionext,data-swap-4byte:
>>>> +    description:
>>>> +      Indicates swap byte order per 4-bytes.
>>>> +    type: boolean
> 
>>> Again these should be set by the device.  I think these should be
>>> set based on a combination of bits per word and if the host is in
>>> big endian or little endian mode.
> 
>> I see. This feature is complicated to use, so I'll not add it here.
> 
> That also works, someone can always add additional support later
> when they have a concrete use case.

I understand.

Thank you,

---
Best Regards
Kunihiko Hayashi
  

Patch

diff --git a/Documentation/devicetree/bindings/spi/socionext,f-ospi.yaml b/Documentation/devicetree/bindings/spi/socionext,f-ospi.yaml
new file mode 100644
index 000000000000..f52cac9ee9f8
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/socionext,f-ospi.yaml
@@ -0,0 +1,99 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/spi/socionext,f-ospi.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Socionext F_OSPI controller
+
+description: |
+  The Socionext F_OSPI is a controller used to interface with flash
+  memories using the SPI communication interface.
+
+maintainers:
+  - Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
+
+allOf:
+  - $ref: "spi-controller.yaml#"
+
+properties:
+  compatible:
+    const: socionext,f-ospi
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  num-cs:
+    minimum: 1
+    maximum: 4
+
+  socionext,cs-start-cycle:
+    description:
+      Specifies the number of SPI_CLK cycles between CS assertion and
+      command transfer.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    maximum: 15
+
+  socionext,cs-end-cycle:
+    description:
+      Specify the number of SPI_CLK cycles between data transfer and
+      CS deassertion.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    maximum: 15
+
+  socionext,cs-deassert-clk-cycle:
+    description:
+      Specify the number of SPI_CLK cycles to output while CS is deasserted.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    maximum: 15
+
+  socionext,alternative-byte:
+    description:
+      Specify the extra bytes to transfer after address transfer.
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    items:
+      - description: the number of bytes to transfer
+        maximum: 4
+      - description: value to transfer
+        default: 0
+      - description: bit-width to transfer
+        enum: [0, 1, 2, 4, 8]
+
+  socionext,data-swap-2byte:
+    description:
+      Indicates swap byte order per 2-bytes.
+    type: boolean
+
+  socionext,data-swap-4byte:
+    description:
+      Indicates swap byte order per 4-bytes.
+    type: boolean
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - "#address-cells"
+  - "#size-cells"
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    ospi0: spi@80000000 {
+        compatible = "socionext,f-ospi";
+        reg = <0x80000000 0x1000>;
+        clocks = <&clks 0>;
+        num-cs = <1>;
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        flash@0 {
+            compatible = "spansion,s25fl128s", "jedec,spi-nor";
+            reg = <0>;
+            spi-max-frequency = <50000000>;
+        };
+    };