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

[Krishna Chaitanya Chundru]

On 8/5/2024 10:58 AM, Krzysztof Kozlowski wrote:
> 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.
Hi Krzysztof,

QPS615 has a 3 downstream ports and 1 upstream port as described below
diagram.
For this entire switch there are some supplies which we described in the
dt-binding (vdd18-supply, vdd09-supply etc) and one GPIO which controls
reset of the switch (reset-gpio). The switch hardware can configure the
individual ports DSP0, DSP1, DSP2, upstream port and also one integrated
ethernet endpoint which is connected to DSP2(I didn't mentioned in the
diagram) through I2C.

The properties other than supplies,i2c client, reset gpio which
are added will be applicable for all the ports.
_______________________________________________________________
|   |i2c|                   QPS615       |Supplies||Resx gpio |
|   |___|              _________________ |________||__________|
|      ________________| Upstream port |_____________         |
|      |               |_______________|            |         |
|      |                       |                    |         |
|      |                       |                    |         |
|  ____|_____              ____|_____            ___|____     |
|  |DSP 0   |              | DSP 1  |            | DSP 2|     |
|  |________|              |________|            |______|     |
|_____________________________________________________________|

- Krishna Chaitanya.
> Best regards,
> Krzysztof