diff mbox series

[2/3] dt-bindings: fpga: xlnx,fpga-slave-selectmap: add DT schema

Message ID 20240131230542.3993409-3-charles.perry@savoirfairelinux.com (mailing list archive)
State New
Headers show
Series fpga: xilinx-selectmap: add new driver | expand

Commit Message

Charles Perry Jan. 31, 2024, 11:05 p.m. UTC 
Document the slave SelectMAP interface of Xilinx 7 series FPGA.

Signed-off-by: Charles Perry <charles.perry@savoirfairelinux.com>
---
 .../bindings/fpga/xlnx,fpga-selectmap.yaml    | 83 +++++++++++++++++++
 1 file changed, 83 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/fpga/xlnx,fpga-selectmap.yaml

Comments

Krzysztof Kozlowski Feb. 1, 2024, 8:07 a.m. UTC | #1 
On 01/02/2024 00:05, Charles Perry wrote:
> Document the slave SelectMAP interface of Xilinx 7 series FPGA.
> 
> Signed-off-by: Charles Perry <charles.perry@savoirfairelinux.com>
> ---
>  .../bindings/fpga/xlnx,fpga-selectmap.yaml    | 83 +++++++++++++++++++
>  1 file changed, 83 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..c9a446b43cdd9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/fpga/xlnx,fpga-selectmap.yaml
> @@ -0,0 +1,83 @@
> +# 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 <charles.perry@savoirfairelinux.com>
> +
> +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-selectmap

Your description mentions "7 Series" which is not present in compatible
and title. What is exactly the product here? Interface usually is not
the final binding, so is this specific to some particular FPGA or SoC?


Best regards,
Krzysztof
Charles Perry Feb. 1, 2024, 6:24 p.m. UTC | #2 
----- On Feb 1, 2024, at 3:07 AM, Krzysztof Kozlowski krzysztof.kozlowski@linaro.org wrote:

> On 01/02/2024 00:05, Charles Perry wrote:
>> Document the slave SelectMAP interface of Xilinx 7 series FPGA.
>> 
>> Signed-off-by: Charles Perry <charles.perry@savoirfairelinux.com>
>> ---
>>  .../bindings/fpga/xlnx,fpga-selectmap.yaml    | 83 +++++++++++++++++++
>>  1 file changed, 83 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..c9a446b43cdd9
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/fpga/xlnx,fpga-selectmap.yaml
>> @@ -0,0 +1,83 @@
>> +# 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 <charles.perry@savoirfairelinux.com>
>> +
>> +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-selectmap
> 
> Your description mentions "7 Series" which is not present in compatible
> and title. What is exactly the product here? Interface usually is not
> the final binding, so is this specific to some particular FPGA or SoC?
> 
> 
> Best regards,
> Krzysztof

This is specific to the FPGA, the 7 series encompass the following part
family:
 * Spartan-7 (XC7S6, XC7S15, ... XC7S100)
 * Artix-7 (XC7A12T, XC7A15T, ... XC7A200T)
 * Kintex-7 (XC7K70T, XC7K160T, ... XC7K480T)
 * Virtex-7 (XC7V585T, XC7V2000T, 
             XC7VX330T, XC7VX415T, ... XC7VX1140T,
             XC7VH580T, XC7VH870T)


The configuration guide of Xilinx [1] tells us that all those devices
share a common programming scheme.

I do agree that having a mention of "7 series" in the compatible name
would be beneficial as Xilinx has more FPGA than just the 7 series.
The name was inspired from "xlnx,fpga-slave-serial" which is the compatible
for the serial interface.

What about "xlnx,fpga-xc7-selectmap" ?

I'm also seeing that I missed some mention of the "slave" word in the
commit message, will fix.

Regards,
Charles

[1] https://www.xilinx.com/support/documentation/user_guides/ug470_7Series_Config.pdf
Krzysztof Kozlowski Feb. 2, 2024, 10:49 a.m. UTC | #3 
On 01/02/2024 19:24, Charles Perry wrote:
> 
> 
> ----- On Feb 1, 2024, at 3:07 AM, Krzysztof Kozlowski krzysztof.kozlowski@linaro.org wrote:
> 
>> On 01/02/2024 00:05, Charles Perry wrote:
>>> Document the slave SelectMAP interface of Xilinx 7 series FPGA.
>>>
>>> Signed-off-by: Charles Perry <charles.perry@savoirfairelinux.com>
>>> ---
>>>  .../bindings/fpga/xlnx,fpga-selectmap.yaml    | 83 +++++++++++++++++++
>>>  1 file changed, 83 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..c9a446b43cdd9
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/fpga/xlnx,fpga-selectmap.yaml
>>> @@ -0,0 +1,83 @@
>>> +# 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 <charles.perry@savoirfairelinux.com>
>>> +
>>> +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-selectmap
>>
>> Your description mentions "7 Series" which is not present in compatible
>> and title. What is exactly the product here? Interface usually is not
>> the final binding, so is this specific to some particular FPGA or SoC?
>>
>>
>> Best regards,
>> Krzysztof
> 
> This is specific to the FPGA, the 7 series encompass the following part
> family:
>  * Spartan-7 (XC7S6, XC7S15, ... XC7S100)
>  * Artix-7 (XC7A12T, XC7A15T, ... XC7A200T)
>  * Kintex-7 (XC7K70T, XC7K160T, ... XC7K480T)
>  * Virtex-7 (XC7V585T, XC7V2000T, 
>              XC7VX330T, XC7VX415T, ... XC7VX1140T,
>              XC7VH580T, XC7VH870T)
> 
> 
> The configuration guide of Xilinx [1] tells us that all those devices
> share a common programming scheme.
> 
> I do agree that having a mention of "7 series" in the compatible name
> would be beneficial as Xilinx has more FPGA than just the 7 series.
> The name was inspired from "xlnx,fpga-slave-serial" which is the compatible
> for the serial interface.
> 
> What about "xlnx,fpga-xc7-selectmap" ?
> 

I am not sure what xc7 is and how Xilinx numbers it products, but
compatibles are supposed to be device specific, not family. Common
programming model could be denoted with generic fallback, but then the
fallback could be device-specific as well, which usually we recommend.

Best regards,
Krzysztof
Charles Perry Feb. 2, 2024, 7:52 p.m. UTC | #4 
----- On Feb 2, 2024, at 5:49 AM, Krzysztof Kozlowski krzysztof.kozlowski@linaro.org wrote:

> On 01/02/2024 19:24, Charles Perry wrote:
>> 
>> 
>> ----- On Feb 1, 2024, at 3:07 AM, Krzysztof Kozlowski
>> krzysztof.kozlowski@linaro.org wrote:
>> 
>>> On 01/02/2024 00:05, Charles Perry wrote:
>>>> Document the slave SelectMAP interface of Xilinx 7 series FPGA.
>>>>
>>>> Signed-off-by: Charles Perry <charles.perry@savoirfairelinux.com>
>>>> ---
>>>>  .../bindings/fpga/xlnx,fpga-selectmap.yaml    | 83 +++++++++++++++++++
>>>>  1 file changed, 83 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..c9a446b43cdd9
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/fpga/xlnx,fpga-selectmap.yaml
>>>> @@ -0,0 +1,83 @@
>>>> +# 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 <charles.perry@savoirfairelinux.com>
>>>> +
>>>> +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-selectmap
>>>
>>> Your description mentions "7 Series" which is not present in compatible
>>> and title. What is exactly the product here? Interface usually is not
>>> the final binding, so is this specific to some particular FPGA or SoC?
>>>
>>>
>>> Best regards,
>>> Krzysztof
>> 
>> This is specific to the FPGA, the 7 series encompass the following part
>> family:
>>  * Spartan-7 (XC7S6, XC7S15, ... XC7S100)
>>  * Artix-7 (XC7A12T, XC7A15T, ... XC7A200T)
>>  * Kintex-7 (XC7K70T, XC7K160T, ... XC7K480T)
>>  * Virtex-7 (XC7V585T, XC7V2000T,
>>              XC7VX330T, XC7VX415T, ... XC7VX1140T,
>>              XC7VH580T, XC7VH870T)
>> 
>> 
>> The configuration guide of Xilinx [1] tells us that all those devices
>> share a common programming scheme.
>> 
>> I do agree that having a mention of "7 series" in the compatible name
>> would be beneficial as Xilinx has more FPGA than just the 7 series.
>> The name was inspired from "xlnx,fpga-slave-serial" which is the compatible
>> for the serial interface.
>> 
>> What about "xlnx,fpga-xc7-selectmap" ?
>> 
> 
> I am not sure what xc7 is and how Xilinx numbers it products, but
> compatibles are supposed to be device specific, not family. Common
> programming model could be denoted with generic fallback, but then the
> fallback could be device-specific as well, which usually we recommend.
> 
> Best regards,
> Krzysztof

XC7 is the common prefix for all 4 families of the 7 series.

Then we have XC7S, XC7A, XC7K and XC7V as the prefixes of those 4 family.

Following that is a number denoting the number of logic element, e.g.
XC7S6, XC7S15, XC7S100, ... I don't think that should be part of the
compatible.

Finally there will be another set of letters and numbers for the
temperature grade and physical package, but those are usually not part
of a compatible string.

I'll change it to four compatibles: "xlnx,fpga-xc7[sakv]-selectmap" so
that the driver can do device specific things if anomalies are found in
one of them.

Thanks again for the review!

Regards,
Charles
diff mbox series

Patch

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..c9a446b43cdd9
--- /dev/null
+++ b/Documentation/devicetree/bindings/fpga/xlnx,fpga-selectmap.yaml
@@ -0,0 +1,83 @@ 
+# 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 <charles.perry@savoirfairelinux.com>
+
+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-selectmap
+
+  reg:
+    description:
+      At least 1 byte of memory mapped IO
+    maxItems: 1
+
+  prog-gpios:
+    description:
+      config pin (referred to as PROGRAM_B in the manual)
+    maxItems: 1
+
+  done-gpios:
+    description:
+      config status pin (referred to as DONE in the manual)
+    maxItems: 1
+
+  init-gpios:
+    description:
+      initialization status and configuration error pin
+      (referred to as INIT_B in the manual)
+    maxItems: 1
+
+  csi-gpios:
+    description:
+      chip select pin (referred to as CSI_B in the manual)
+      Optional gpio for if the bus controller does not provide a chip select.
+    maxItems: 1
+
+  rdwr-gpios:
+    description:
+      read/write select pin (referred to as RDWR_B in the manual)
+      Optional gpio for if the bus controller does not provide this pin.
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - prog-gpios
+  - done-gpios
+  - init-gpios
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    fpga-mgr@8000000 {
+      compatible = "xlnx,fpga-selectmap";
+      reg = <0x8000000 0x4>;
+      prog-gpios = <&gpio5 5 GPIO_ACTIVE_LOW>;
+      init-gpios = <&gpio5 8 GPIO_ACTIVE_LOW>;
+      done-gpios = <&gpio2 30 GPIO_ACTIVE_HIGH>;
+      csi-gpios = <&gpio3 19 GPIO_ACTIVE_LOW>;
+      rdwr-gpios = <&gpio3 10 GPIO_ACTIVE_LOW>;
+    };
+...