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

  Document the slave SelectMAP interface of Xilinx 7 series FPGA.

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

On Mon, 29 Jan 2024 17:56:01 -0500, Charles Perry wrote:
> Document the slave SelectMAP interface of Xilinx 7 series FPGA.
> 
> Signed-off-by: Charles Perry <charles.perry@savoirfairelinux.com>
> ---
>  .../fpga/xlnx,fpga-slave-selectmap.yaml       | 85 +++++++++++++++++++
>  1 file changed, 85 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/fpga/xlnx,fpga-slave-selectmap.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/fpga/xlnx,fpga-slave-selectmap.yaml: 'maintainers' is a required property
	hint: Metaschema for devicetree binding documentation
	from schema $id: http://devicetree.org/meta-schemas/base.yaml#
Error: Documentation/devicetree/bindings/fpga/xlnx,fpga-slave-selectmap.example.dts:19.9-14 syntax error
FATAL ERROR: Unable to parse input tree
make[2]: *** [scripts/Makefile.lib:419: Documentation/devicetree/bindings/fpga/xlnx,fpga-slave-selectmap.example.dtb] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1428: dt_binding_check] Error 2
make: *** [Makefile:240: __sub-make] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240129225602.3832449-2-charles.perry@savoirfairelinux.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
  

On 29/01/2024 23:56, Charles Perry wrote:
> Document the slave SelectMAP interface of Xilinx 7 series FPGA.
> 
> Signed-off-by: Charles Perry <charles.perry@savoirfairelinux.com>
> ---
>  .../fpga/xlnx,fpga-slave-selectmap.yaml       | 85 +++++++++++++++++++
>  1 file changed, 85 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/fpga/xlnx,fpga-slave-selectmap.yaml
> 
> diff --git a/Documentation/devicetree/bindings/fpga/xlnx,fpga-slave-selectmap.yaml b/Documentation/devicetree/bindings/fpga/xlnx,fpga-slave-selectmap.yaml
> new file mode 100644
> index 0000000000000..20cea24e3e39a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/fpga/xlnx,fpga-slave-selectmap.yaml
> @@ -0,0 +1,85 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/fpga/xlnx,fpga-slave-selectmap.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Xilinx Slave SelectMAP FPGA
> +
> +description: |
> +  Xilinx 7 Series FPGAs support a method of loading the bitstream over a
> +  parallel port named the slave 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
> +
> +properties:
> +  compatible:
> +    enum:
> +      - xlnx,fpga-slave-selectmap

You did not test bindings, so only limited review.

> +
> +  reg:
> +    description:
> +      At least 1 byte of memory mapped IO
> +    maxItems: 1
> +
> +  prog_b-gpios:


No underscores in names.


> +    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-b-gpios:

Is there init-a? Open other bindings and look how these are called there.


> +    description:
> +      initialization status and configuration error pin
> +      (referred to as INIT_B in the manual)
> +    maxItems: 1
> +
> +  csi-b-gpios:

Where is csi-a?

> +    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-b-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_b-gpios
> +  - done-gpios
> +  - init-b-gpios
> +
> +additionalProperties: true

Nope, this cannot bue true.

> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +    &weim {

Drop or use some generic soc

> +      status = "okay";

Drop

> +      ranges = <0 0 0x08000000 0x04000000>;

Drop

> +
> +      fpga_mgr: fpga_programmer@0,0 {

No underscores in names, drop label.

Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation


> +        compatible = "xlnx,fpga-slave-selectmap";
> +        reg = <0 0 0x4000000>;
> +        fsl,weim-cs-timing = <0x00070031 0x00000142
> +              0x00020000 0x00000000
> +              0x0c000645 0x00000000>;

NAK.

Please run your patch through Xilinx folks before sending.

Best regards,
Krzysztof
  

On 30/01/2024 08:52, Krzysztof Kozlowski wrote:
> Please run your patch through Xilinx folks before sending.

Uh, you are not Xilinx folk, so disregard that. Yet still amount of
trivial issues here point that you need to start from scratch from
existing schema or example-schema.

Best regards,
Krzysztof
  

----- On Jan 30, 2024, at 2:52 AM, Krzysztof Kozlowski krzysztof.kozlowski@linaro.org wrote:

> On 29/01/2024 23:56, Charles Perry wrote:
>> Document the slave SelectMAP interface of Xilinx 7 series FPGA.
>> 
>> Signed-off-by: Charles Perry <charles.perry@savoirfairelinux.com>
>> ---
>>  .../fpga/xlnx,fpga-slave-selectmap.yaml       | 85 +++++++++++++++++++
>>  1 file changed, 85 insertions(+)
>>  create mode 100644
>>  Documentation/devicetree/bindings/fpga/xlnx,fpga-slave-selectmap.yaml
>> 
>> diff --git
>> a/Documentation/devicetree/bindings/fpga/xlnx,fpga-slave-selectmap.yaml
>> b/Documentation/devicetree/bindings/fpga/xlnx,fpga-slave-selectmap.yaml
>> new file mode 100644
>> index 0000000000000..20cea24e3e39a
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/fpga/xlnx,fpga-slave-selectmap.yaml
>> @@ -0,0 +1,85 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/fpga/xlnx,fpga-slave-selectmap.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Xilinx Slave SelectMAP FPGA
>> +
>> +description: |
>> +  Xilinx 7 Series FPGAs support a method of loading the bitstream over a
>> +  parallel port named the slave 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
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - xlnx,fpga-slave-selectmap
> 
> You did not test bindings, so only limited review.
> 

I had issues installing pylibfdt but that's fixed now, will do.

>> +
>> +  reg:
>> +    description:
>> +      At least 1 byte of memory mapped IO
>> +    maxItems: 1
>> +
>> +  prog_b-gpios:
> 
> 
> No underscores in names.
> 

This is heavily based on "xlnx,fpga-slave-serial.yaml" which uses an underscore.
I can use a dash instead but that would make things inconsistent across the two schemas. 

> 
>> +    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-b-gpios:
> 
> Is there init-a? Open other bindings and look how these are called there.
> 

No, the "-b" is there to denote that the signal is active low. I think its shorthand
for "bar" which is the overline (‾) that electronic engineer put on top of the name of the
signal on schematics. It comes from the datasheet.

> 
>> +    description:
>> +      initialization status and configuration error pin
>> +      (referred to as INIT_B in the manual)
>> +    maxItems: 1
>> +
>> +  csi-b-gpios:
> 
> Where is csi-a?
> 

No "csi-a", this is the CSI signal which is active low.

>> +    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-b-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_b-gpios
>> +  - done-gpios
>> +  - init-b-gpios
>> +
>> +additionalProperties: true
> 
> Nope, this cannot bue true.
> 

Ok, I'll put this to false but I'm not quite sure I understand the implications.

My reasoning behind assigning this to true was that the FPGA is an external
device on a bus that needs to be configured by a bus controller. The bus controller
would be the parent of the fpga DT node and the later would contain properties
parsed by the bus controller driver.

>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/gpio/gpio.h>
>> +    &weim {
> 
> Drop or use some generic soc
> 

Ok

>> +      status = "okay";
> 
> Drop
> 

Ok

>> +      ranges = <0 0 0x08000000 0x04000000>;
> 
> Drop
> 

Ok

>> +
>> +      fpga_mgr: fpga_programmer@0,0 {
> 
> No underscores in names, drop label.
> 
> Node names should be generic. See also an explanation and list of
> examples (not exhaustive) in DT specification:
> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
> 
> 

Ok, will use "fpga-mgr" as this seems to be the most common one for fpga managers.

>> +        compatible = "xlnx,fpga-slave-selectmap";
>> +        reg = <0 0 0x4000000>;
>> +        fsl,weim-cs-timing = <0x00070031 0x00000142
>> +              0x00020000 0x00000000
>> +              0x0c000645 0x00000000>;
> 
> NAK.
> 
> Please run your patch through Xilinx folks before sending.
> 
> Best regards,
> Krzysztof

Thank you,
Charles
  

On 30/01/2024 16:45, Charles Perry wrote:
> 
>>> +
>>> +  reg:
>>> +    description:
>>> +      At least 1 byte of memory mapped IO
>>> +    maxItems: 1
>>> +
>>> +  prog_b-gpios:
>>
>>
>> No underscores in names.
>>
> 
> This is heavily based on "xlnx,fpga-slave-serial.yaml" which uses an underscore.
> I can use a dash instead but that would make things inconsistent across the two schemas. 

Inconsistency is not a problem. Duplicating technical debt is.

> 
>>
>>> +    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-b-gpios:
>>
>> Is there init-a? Open other bindings and look how these are called there.
>>
> 
> No, the "-b" is there to denote that the signal is active low. I think its shorthand
> for "bar" which is the overline (‾) that electronic engineer put on top of the name of the
> signal on schematics. It comes from the datasheet.

Then just "init-gpios"

..

>>> +required:
>>> +  - compatible
>>> +  - reg
>>> +  - prog_b-gpios
>>> +  - done-gpios
>>> +  - init-b-gpios
>>> +
>>> +additionalProperties: true
>>
>> Nope, this cannot bue true.
>>
> 
> Ok, I'll put this to false but I'm not quite sure I understand the implications.
> 
> My reasoning behind assigning this to true was that the FPGA is an external
> device on a bus that needs to be configured by a bus controller. The bus controller
> would be the parent of the fpga DT node and the later would contain properties
> parsed by the bus controller driver.

Which bus controller? MMIO bus does not parse children properties.
Anyway, if that's the case you miss $ref to respective
peripheral-props.yaml matching your bus and then "unevaluatedProperties:
false".

Best regards,
Krzysztof
  

On 29/01/2024 23:56, Charles Perry wrote:
> Document the slave SelectMAP interface of Xilinx 7 series FPGA.
> 
> Signed-off-by: Charles Perry <charles.perry@savoirfairelinux.com>
> ---
>  .../fpga/xlnx,fpga-slave-selectmap.yaml       | 85 +++++++++++++++++++
>  1 file changed, 85 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/fpga/xlnx,fpga-slave-selectmap.yaml
> 
> diff --git a/Documentation/devicetree/bindings/fpga/xlnx,fpga-slave-selectmap.yaml b/Documentation/devicetree/bindings/fpga/xlnx,fpga-slave-selectmap.yaml
> new file mode 100644
> index 0000000000000..20cea24e3e39a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/fpga/xlnx,fpga-slave-selectmap.yaml
> @@ -0,0 +1,85 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/fpga/xlnx,fpga-slave-selectmap.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Xilinx Slave SelectMAP FPGA

https://elixir.bootlin.com/linux/v6.8-rc2/source/Documentation/process/coding-style.rst#L338

Everywhere: compatible, title, filename, descriptions.

> +
> +description: |
> +  Xilinx 7 Series FPGAs support a method of loading the bitstream over a
> +  parallel port named the slave 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

I am surprised that AMD/Xilinx still did not update the document to
modern naming (slave->secondary).

+Cc Michal,
Maybe that's something you could push it.

Best regards,
Krzysztof
  

----- On Jan 30, 2024, at 11:05 AM, Krzysztof Kozlowski krzysztof.kozlowski@linaro.org wrote:

> On 30/01/2024 16:45, Charles Perry wrote:
>> 
>>>> +
>>>> +  reg:
>>>> +    description:
>>>> +      At least 1 byte of memory mapped IO
>>>> +    maxItems: 1
>>>> +
>>>> +  prog_b-gpios:
>>>
>>>
>>> No underscores in names.
>>>
>> 
>> This is heavily based on "xlnx,fpga-slave-serial.yaml" which uses an underscore.
>> I can use a dash instead but that would make things inconsistent across the two
>> schemas.
> 
> Inconsistency is not a problem. Duplicating technical debt is.
> 
>> 
>>>
>>>> +    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-b-gpios:
>>>
>>> Is there init-a? Open other bindings and look how these are called there.
>>>
>> 
>> No, the "-b" is there to denote that the signal is active low. I think its
>> shorthand
>> for "bar" which is the overline (‾) that electronic engineer put on top of the
>> name of the
>> signal on schematics. It comes from the datasheet.
> 
> Then just "init-gpios"
> 
> ...
> 
>>>> +required:
>>>> +  - compatible
>>>> +  - reg
>>>> +  - prog_b-gpios
>>>> +  - done-gpios
>>>> +  - init-b-gpios
>>>> +
>>>> +additionalProperties: true
>>>
>>> Nope, this cannot bue true.
>>>
>> 
>> Ok, I'll put this to false but I'm not quite sure I understand the implications.
>> 
>> My reasoning behind assigning this to true was that the FPGA is an external
>> device on a bus that needs to be configured by a bus controller. The bus
>> controller
>> would be the parent of the fpga DT node and the later would contain properties
>> parsed by the bus controller driver.
> 
> Which bus controller? MMIO bus does not parse children properties.
> Anyway, if that's the case you miss $ref to respective
> peripheral-props.yaml matching your bus and then "unevaluatedProperties:
> false".

This one: https://elixir.bootlin.com/linux/v6.8-rc2/source/Documentation/devicetree/bindings/bus/imx-weim.txt#L56

> 
> Best regards,
> Krzysztof

Regards,
Charles
  

On 30/01/2024 18:05, Charles Perry wrote:
> 
> 
> ----- On Jan 30, 2024, at 11:05 AM, Krzysztof Kozlowski krzysztof.kozlowski@linaro.org wrote:
> 
>> On 30/01/2024 16:45, Charles Perry wrote:
>>>
>>>>> +
>>>>> +  reg:
>>>>> +    description:
>>>>> +      At least 1 byte of memory mapped IO
>>>>> +    maxItems: 1
>>>>> +
>>>>> +  prog_b-gpios:
>>>>
>>>>
>>>> No underscores in names.
>>>>
>>>
>>> This is heavily based on "xlnx,fpga-slave-serial.yaml" which uses an underscore.
>>> I can use a dash instead but that would make things inconsistent across the two
>>> schemas.
>>
>> Inconsistency is not a problem. Duplicating technical debt is.
>>
>>>
>>>>
>>>>> +    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-b-gpios:
>>>>
>>>> Is there init-a? Open other bindings and look how these are called there.
>>>>
>>>
>>> No, the "-b" is there to denote that the signal is active low. I think its
>>> shorthand
>>> for "bar" which is the overline (‾) that electronic engineer put on top of the
>>> name of the
>>> signal on schematics. It comes from the datasheet.
>>
>> Then just "init-gpios"
>>
>> ...
>>
>>>>> +required:
>>>>> +  - compatible
>>>>> +  - reg
>>>>> +  - prog_b-gpios
>>>>> +  - done-gpios
>>>>> +  - init-b-gpios
>>>>> +
>>>>> +additionalProperties: true
>>>>
>>>> Nope, this cannot bue true.
>>>>
>>>
>>> Ok, I'll put this to false but I'm not quite sure I understand the implications.
>>>
>>> My reasoning behind assigning this to true was that the FPGA is an external
>>> device on a bus that needs to be configured by a bus controller. The bus
>>> controller
>>> would be the parent of the fpga DT node and the later would contain properties
>>> parsed by the bus controller driver.
>>
>> Which bus controller? MMIO bus does not parse children properties.
>> Anyway, if that's the case you miss $ref to respective
>> peripheral-props.yaml matching your bus and then "unevaluatedProperties:
>> false".
> 
> This one: https://elixir.bootlin.com/linux/v6.8-rc2/source/Documentation/devicetree/bindings/bus/imx-weim.txt#L56

Eh, ok, so after fast check WEIM looks like some memory interface bus,
so the bus bindings should be moved to memory-controllers and converted
to YAML. Then you add child node properties to own schema and reference
in mc-peripheral-props, which is then referenced in your binding here,
as I mentioned.

Best regards,
Krzysztof
  

----- On Jan 30, 2024, at 12:58 PM, Krzysztof Kozlowski krzysztof.kozlowski@linaro.org wrote:

> On 30/01/2024 18:05, Charles Perry wrote:
>> 
>> 
>> ----- On Jan 30, 2024, at 11:05 AM, Krzysztof Kozlowski
>> krzysztof.kozlowski@linaro.org wrote:
>> 
>>> On 30/01/2024 16:45, Charles Perry wrote:
>>>>
>>>>>> +
>>>>>> +  reg:
>>>>>> +    description:
>>>>>> +      At least 1 byte of memory mapped IO
>>>>>> +    maxItems: 1
>>>>>> +
>>>>>> +  prog_b-gpios:
>>>>>
>>>>>
>>>>> No underscores in names.
>>>>>
>>>>
>>>> This is heavily based on "xlnx,fpga-slave-serial.yaml" which uses an underscore.
>>>> I can use a dash instead but that would make things inconsistent across the two
>>>> schemas.
>>>
>>> Inconsistency is not a problem. Duplicating technical debt is.
>>>
>>>>
>>>>>
>>>>>> +    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-b-gpios:
>>>>>
>>>>> Is there init-a? Open other bindings and look how these are called there.
>>>>>
>>>>
>>>> No, the "-b" is there to denote that the signal is active low. I think its
>>>> shorthand
>>>> for "bar" which is the overline (‾) that electronic engineer put on top of the
>>>> name of the
>>>> signal on schematics. It comes from the datasheet.
>>>
>>> Then just "init-gpios"
>>>
>>> ...
>>>
>>>>>> +required:
>>>>>> +  - compatible
>>>>>> +  - reg
>>>>>> +  - prog_b-gpios
>>>>>> +  - done-gpios
>>>>>> +  - init-b-gpios
>>>>>> +
>>>>>> +additionalProperties: true
>>>>>
>>>>> Nope, this cannot bue true.
>>>>>
>>>>
>>>> Ok, I'll put this to false but I'm not quite sure I understand the implications.
>>>>
>>>> My reasoning behind assigning this to true was that the FPGA is an external
>>>> device on a bus that needs to be configured by a bus controller. The bus
>>>> controller
>>>> would be the parent of the fpga DT node and the later would contain properties
>>>> parsed by the bus controller driver.
>>>
>>> Which bus controller? MMIO bus does not parse children properties.
>>> Anyway, if that's the case you miss $ref to respective
>>> peripheral-props.yaml matching your bus and then "unevaluatedProperties:
>>> false".
>> 
>> This one:
>> https://elixir.bootlin.com/linux/v6.8-rc2/source/Documentation/devicetree/bindings/bus/imx-weim.txt#L56
> 
> Eh, ok, so after fast check WEIM looks like some memory interface bus,
> so the bus bindings should be moved to memory-controllers and converted
> to YAML. Then you add child node properties to own schema and reference
> in mc-peripheral-props, which is then referenced in your binding here,
> as I mentioned.
> 
> Best regards,
> Krzysztof

Thank you for pointing that out, mc-peripheral-props.yaml seems to be
exactly what I was looking for.

Regards,
Charles
  

Hello Krzysztof,

On 30/01/2024 16:09, Krzysztof Kozlowski wrote:

>> +
>> +description: |
>> +  Xilinx 7 Series FPGAs support a method of loading the bitstream over a
>> +  parallel port named the slave 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
> 
> I am surprised that AMD/Xilinx still did not update the document to
> modern naming (slave->secondary).

Thank you for bringing this up.

We are moving away from using non-inclusive technical terminology and 
are removing non-inclusive language from our products and related 
collateral.  You will for some time find examples of non-inclusive 
language, especially in our older products as we work to make these 
changes and align with industry standards.  For new IP we're ensuring 
that we switch and stick to inclusive terminology, as you may have seen 
with my recent w1 driver submission.

SelectMAP is a decades-old interface and as such it is unlikely that we 
will update this in all documentation dating back this time.  I shall 
however look to understand what is planned here for active documentation 
and new driver submissions.

regards
Kris
  

On Wed, Jan 31, 2024 at 11:03:25AM +0000, Kris Chaplin wrote:
> Hello Krzysztof,
> 
> On 30/01/2024 16:09, Krzysztof Kozlowski wrote:
> 
> > > +
> > > +description: |
> > > +  Xilinx 7 Series FPGAs support a method of loading the bitstream over a
> > > +  parallel port named the slave 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
> > 
> > I am surprised that AMD/Xilinx still did not update the document to
> > modern naming (slave->secondary).
> 
> Thank you for bringing this up.
> 
> We are moving away from using non-inclusive technical terminology and are
> removing non-inclusive language from our products and related collateral.
> You will for some time find examples of non-inclusive language, especially
> in our older products as we work to make these changes and align with
> industry standards.  For new IP we're ensuring that we switch and stick to
> inclusive terminology, as you may have seen with my recent w1 driver
> submission.
> 
> SelectMAP is a decades-old interface and as such it is unlikely that we will
> update this in all documentation dating back this time.  I shall however
> look to understand what is planned here for active documentation and new
> driver submissions.

Yes, I need review from AMD/Xilinx side. Especially the HW parts, and
some namings of variables, e.g. if xilinx-core is proper for what products
it supports, and won't be an issue in future.

Thanks,
Yilun

> 
> regards
> Kris
>
  

On Feb 4, 2024, at 3:30 AM, Xu Yilun yilun.xu@linux.intel.com wrote:
> On Wed, Jan 31, 2024 at 11:03:25AM +0000, Kris Chaplin wrote:
>> Hello Krzysztof,
>> 
>> On 30/01/2024 16:09, Krzysztof Kozlowski wrote:
>> 
>> > > +
>> > > +description: |
>> > > +  Xilinx 7 Series FPGAs support a method of loading the bitstream over a
>> > > +  parallel port named the slave 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
>> > 
>> > I am surprised that AMD/Xilinx still did not update the document to
>> > modern naming (slave->secondary).
>> 
>> Thank you for bringing this up.
>> 
>> We are moving away from using non-inclusive technical terminology and are
>> removing non-inclusive language from our products and related collateral.
>> You will for some time find examples of non-inclusive language, especially
>> in our older products as we work to make these changes and align with
>> industry standards.  For new IP we're ensuring that we switch and stick to
>> inclusive terminology, as you may have seen with my recent w1 driver
>> submission.
>> 
>> SelectMAP is a decades-old interface and as such it is unlikely that we will
>> update this in all documentation dating back this time.  I shall however
>> look to understand what is planned here for active documentation and new
>> driver submissions.
> 
> Yes, I need review from AMD/Xilinx side. Especially the HW parts, and
> some namings of variables, e.g. if xilinx-core is proper for what products
> it supports, and won't be an issue in future.
> 
> Thanks,
> Yilun
> 
>> 
>> regards
>> Kris

Hello,

I chose the "-core" suffix as it seems widely used for core logic of device 
drivers for chips that comes in an i2c and spi flavour. It seems like "-common"
is also widespread, let me know if you prefer that suffix.

As for the HW parts, here's the compatibles used in the v3 patchset for 
convenience:
- xlnx,fpga-xc7s-selectmap
- xlnx,fpga-xc7a-selectmap
- xlnx,fpga-xc7k-selectmap
- xlnx,fpga-xc7v-selectmap

We're trying to be a bit more specific than the spi interface which uses
"xlnx,fpga-slave-serial"

Regards,
Charles