linux-kernel - Re: [PATCH v4 01/10] dt-bindings: PCI: Add binding for Toshiba TC956x PCIe switch

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250226-eager-urchin-of-performance-b71ae4@krzk-bin>
Date: Wed, 26 Feb 2025 08:30:17 +0100
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Krishna Chaitanya Chundru <krishna.chundru@....qualcomm.com>
Cc: Bjorn Helgaas <bhelgaas@...gle.com>, 
	Lorenzo Pieralisi <lpieralisi@...nel.org>, Krzysztof Wilczyński <kw@...ux.com>, 
	Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>, Rob Herring <robh@...nel.org>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, 
	chaitanya chundru <quic_krichai@...cinc.com>, Bjorn Andersson <andersson@...nel.org>, 
	Konrad Dybcio <konradybcio@...nel.org>, cros-qcom-dts-watchers@...omium.org, 
	Jingoo Han <jingoohan1@...il.com>, Bartosz Golaszewski <brgl@...ev.pl>, quic_vbadigan@...cnic.com, 
	amitk@...nel.org, dmitry.baryshkov@...aro.org, linux-pci@...r.kernel.org, 
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org, 
	jorge.ramirez@....qualcomm.com
Subject: Re: [PATCH v4 01/10] dt-bindings: PCI: Add binding for Toshiba
 TC956x PCIe switch

On Tue, Feb 25, 2025 at 03:03:58PM +0530, Krishna Chaitanya Chundru wrote:
> From: Krishna chaitanya chundru <quic_krichai@...cinc.com>
> 
> Add a device tree binding for the Toshiba TC956x PCIe switch, which
> provides an Ethernet MAC integrated to the 3rd downstream port and two
> downstream PCIe ports.
> 
> Signed-off-by: Krishna chaitanya chundru <quic_krichai@...cinc.com>
> Reviewed-by: Bjorn Andersson <andersson@...nel.org>

Drop, file was named entirely different. I see other changes, altough
comparing with b4 is impossible.

Why b4 does not work for this patch?

  b4 diff '20250225-qps615_v4_1-v4-1-e08633a7bdf8@....qualcomm.com'
  Checking for older revisions
  Grabbing search results from lore.kernel.org
  Nothing matching that query.

Looks like you use b4 but decide to not use b4 changesets/versions. Why
making it difficult for reviewers and for yourself?


> ---
>  .../devicetree/bindings/pci/toshiba,tc956x.yaml    | 178 +++++++++++++++++++++
>  1 file changed, 178 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pci/toshiba,tc956x.yaml b/Documentation/devicetree/bindings/pci/toshiba,tc956x.yaml
> new file mode 100644
> index 000000000000..ffed23004f0d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/toshiba,tc956x.yaml

What is "x" here? Wildcard?

> @@ -0,0 +1,178 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pci/toshiba,tc956x.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Toshiba TC956x PCIe switch
> +
> +maintainers:
> +  - Krishna chaitanya chundru <quic_krichai@...cinc.com>
> +
> +description: |
> +  Toshiba TC956x PCIe switch has one upstream and three downstream

TC9560? Which one are you using here?

> +  ports. The 3rd downstream port has integrated endpoint device of
> +  Ethernet MAC. Other two downstream ports are supposed to connect
> +  to external device.
> +
> +  The TC956x PCIe switch can be configured through I2C interface before
> +  PCIe link is established to change FTS, ASPM related entry delays,
> +  tx amplitude etc for better power efficiency and functionality.
> +
> +properties:
> +  compatible:
> +    items:
> +      - enum:
> +          - "pci1179,0623"

Why quotes?

> +      - const: pciclass,0604
> +
> +  reg:
> +    maxItems: 1
> +
> +  i2c-parent:
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    description:
> +      A phandle to the parent I2C node and the slave address of the device
> +      used to do configure tc956x to change FTS, tx amplitude etc.
> +    items:
> +      - description: Phandle to the I2C controller node
> +      - description: I2C slave address
> +
> +  vdd18-supply: true
> +
> +  vdd09-supply: true
> +
> +  vddc-supply: true
> +
> +  vddio1-supply: true
> +
> +  vddio2-supply: true
> +
> +  vddio18-supply: true
> +
> +  reset-gpios:
> +    maxItems: 1
> +    description:
> +      GPIO controlling the RESX# pin.
> +
> +allOf:
> +  - $ref: "#/$defs/tc956x-node"
> +
> +patternProperties:
> +  "^pcie@[1-3],0$":
> +    description:
> +      child nodes describing the internal downstream ports
> +      the tc956x switch.
> +    type: object
> +    $ref: "#/$defs/tc956x-node"
> +    unevaluatedProperties: false
> +
> +$defs:
> +  tc956x-node:
> +    type: object
> +
> +    properties:
> +      tc956x,tx-amplitude-microvolt:

You already got comments on this.

> +        $ref: /schemas/types.yaml#/definitions/uint32

Never tested.


> +        description:
> +          Change Tx Margin setting for low power consumption.
> +
> +      tc956x,no-dfe-support:

There is no such vendor prefix and you already got exactly the same
comment at v3. How did you resolve that comment?

> +        type: boolean
> +        description:
> +          Disable DFE (Decision Feedback Equalizer), which mitigates
> +          intersymbol interference and some reflections caused by impedance mismatches.
> +
> +    allOf:
> +      - $ref: /schemas/pci/pci-pci-bridge.yaml#
> +
> +unevaluatedProperties: false

Keep order as in example-schema.

Best regards,
Krzysztof