Re: [PATCH v3 1/6] dt-bindings: PCI: Add binding for qps615

[Krishna Chaitanya Chundru <quic_krichai@quicinc.com>]

On 11/20/2024 1:34 PM, Krzysztof Kozlowski wrote:
> On Tue, Nov 12, 2024 at 08:31:33PM +0530, Krishna chaitanya chundru wrote:
>> Add binding describing the Qualcomm PCIe switch, QPS615,
>> which provides Ethernet MAC integrated to the 3rd downstream port
>> and two downstream PCIe ports.
>>
>> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
>> ---
>>   .../devicetree/bindings/pci/qcom,qps615.yaml       | 205 +++++++++++++++++++++
>>   1 file changed, 205 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/pci/qcom,qps615.yaml b/Documentation/devicetree/bindings/pci/qcom,qps615.yaml
>> new file mode 100644
>> index 000000000000..e6a63a0bb0f3
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/pci/qcom,qps615.yaml
> 
> Isn't "qcom,qps615" a SoC name? This is supposed to be matching
> compatible, in your case probably qcom,qps615-whatever-this-is?
>
qcom,qcs615 is a soc name, qcom,qps615 is the pcie switch.

ack I will change it to qcom,qps615-pcie

> ...
> 
>> +  qps615,axi-clk-freq-hz:
> 
> That's a downstream code you send us.
> 
> Anyway, why assigned clock rates do not work for you? You are
> re-implementing legacy property now under different name :/
> 
> The assigned clock rates comes in to the picture when we are using clock
framework to control the clocks. For this switch there are no clocks 
needs to be control, the moment we power on the switch clocks are
enabled by default. This switch provides a mechanism to control the
frequency using i2c. And switch supports only two frequencies i.e
125MHz and 250MHZ by default it runs on 250MHz, we can do one i2c
write with which switch runs in 125MHz.
>> +    description:
>> +      AXI clock rate which is internal bus of the switch
>> +      The switch only runs in two frequencies i.e 250MHz and 125MHz.
>> +    enum: [125000000, 250000000]
>> +
>> +allOf:
>> +  - $ref: "#/$defs/qps615-node"
>> +
>> +patternProperties:
>> +  "@1?[0-9a-f](,[0-7])?$":
>> +    description: child nodes describing the internal downstream ports
>> +      the qps615 switch.
>> +    type: object
>> +    $ref: "#/$defs/qps615-node"
>> +    unevaluatedProperties: false
>> +
>> +$defs:
>> +  qps615-node:
>> +    type: object
>> +
>> +    properties:
>> +      qcom,l0s-entry-delay-ns:
>> +        description: Aspm l0s entry delay.
>> +
>> +      qcom,l1-entry-delay-ns:
>> +        description: Aspm l1 entry delay.
>> +
>> +      qcom,tx-amplitude-millivolt:
> 
> -microvolt does not work for you?
> 
let me check this if it is applicable we will change it to microvolt.
>> +        $ref: /schemas/types.yaml#/definitions/uint32
>> +        description: Change Tx Margin setting for low power consumption.
>> +
>> +      qcom,no-dfe-support:
>> +        type: boolean
>> +        description: Disable DFE (Decision Feedback Equalizer), which mitigates
>> +          intersymbol interference and some reflections caused by impedance mismatches.
>> +
>> +      qcom,nfts:
>> +        $ref: /schemas/types.yaml#/definitions/uint32
>> +        description:
>> +          Number of Fast Training Sequence (FTS) used during L0s to L0 exit
>> +          for bit and Symbol lock.
> 
> Use some of these properties in the example. I saw only one.
> 
In the next patch I will try to use all the properties as suggested.
>> +
>> +    allOf:
>> +      - $ref: /schemas/pci/pci-bus.yaml#
>> +
>> +unevaluatedProperties: false
>> +
>> +required:
>> +  - vdd18-supply
>> +  - vdd09-supply
>> +  - vddc-supply
>> +  - vddio1-supply
>> +  - vddio2-supply
>> +  - vddio18-supply
>> +  - i2c-parent
>> +  - reset-gpios
>> +
>> +examples:
>> +  - |
>> +
> 
> Drop blank line
> 
ack.

- Krishna Chaitanya.
>> +    #include <dt-bindings/gpio/gpio.h>
>> +
>> +    pcie {
>> +        #address-cells = <3>;
>> +        #size-cells = <2>;
>> +
>> +        pcie@0 {
>> +            device_type = "pci";
>> +            reg = <0x0 0x0 0x0 0x0 0x0>;
> 
> Best regards,
> Krzysztof
>