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

[Charles Perry <charles.perry@savoirfairelinux.com>]

----- 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