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

[Charles Perry]

----- On Jan 30, 2024, at 11:05 AM, Krzysztof Kozlowski krzysztof.kozlowski@xxxxxxxxxx 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