Re: [PATCH v2 1/8] dt-bindings: PCI: Add binding for qps615

Krzysztof Kozlowski
2024-08-05T05:29:04Z

On 05/08/2024 07:26, Krishna Chaitanya Chundru wrote:
>
>
> On 8/5/2024 10:44 AM, Krzysztof Kozlowski wrote:
>> On 05/08/2024 06:11, Krishna Chaitanya Chundru wrote:
>>
>>
>>>>> +
>>>>> + qcom,nfts:
>>>>> + $ref: /schemas/types.yaml#/definitions/uint8
>>>>> + description:
>>>>> + Fast Training Sequence (FTS) is the mechanism that
>>>>> + is used for bit and Symbol lock.
>>>>
>>>> What are the values? Why this is uint8?
>>>>
>>> These represents number of fast training sequence and doesn't have
>>> any units and the maximum value for this is 0xFF only so we used
>>> uint8.
>>>> You described the desired Linux feature or behavior, not the actual
>>>> hardware. The bindings are about the latter, so instead you need to
>>>> rephrase the property and its description to match actual hardware
>>>> capabilities/features/configuration etc.
>>> ack.
>>>>
>>>>> +
>>>>> +allOf:
>>>>> + - $ref: /schemas/pci/pci-bus-common.yaml#
>>>>> + - if:
>>>>> + properties:
>>>>> + compatible:
>>>>> + contains:
>>>>> + const: pci1179,0623
>>>>> + required:
>>>>> + - compatible
>>>>
>>>> Why do you have entire if? You do not have multiple variants, drop.
>>>>
>>> The child nodes also referencing the qcom,qps615.yaml# node, I tried
>>> to use this way to say "the below properties are for the required for
>>> parent and optional for child".
>>
>> I don't understand how child device can be exactly the same as parent
>> device. How does it look in terms of hardware? Pins and supplies?
>>
>>>>> + then:
>>>>> + required:
>>>>> + - vdd18-supply
>>>>> + - vdd09-supply
>>>>> + - vddc-supply
>>>>> + - vddio1-supply
>>>>> + - vddio2-supply
>>>>> + - vddio18-supply
>>>>> + - qcom,qps615-controller
>>>>> + - reset-gpios
>>>>> +
>>>>> +patternProperties:
>>>>> + "@1?[0-9a-f](,[0-7])?$":
>>>>> + type: object
>>>>> + $ref: qcom,qps615.yaml#
>>>>> + additionalProperties: true
>>>>
>>>> Nope, drop pattern Properties or explain what is this.
>>>>
>>> the child nodes represent the downstream ports of the PCIe
>>> switch which wants to use same properties that is why
>>> I tried to use this pattern properties.
>>
>> Downstream port is not the same as device. Why downstream port has the
>> same supplies? To which pins are they connected?
>>
>>
> Hi Krzysztof,
>
> Downstream ports dosen't have pins or supplies to power on.
>
> But there are properties like qcom,l0s-entry-delay-ns,
> qcom,l1-entry-delay-ns, qcom,tx-amplitude-millivolt etc which
> applicable for child nodes also. Instead of re-declaring the
> these properties again I tried to use pattern properties.

You could use $defs for them, but I don't understand how does these
properties apply for both main device and ports. It seems you are
writing binding to match some driver behavior. Let's start from basics -
describe the hardware.

Best regards,
Krzysztof