Re: [PATCH v8 1/7] dt-bindings: PCI: Add binding for Toshiba TC9563 PCIe switch

[Bjorn Helgaas <helgaas@kernel.org>]

On Fri, Oct 31, 2025 at 04:41:58PM +0530, Krishna Chaitanya Chundru wrote:
> Add a device tree binding for the Toshiba TC9563 PCIe switch, which
> provides an Ethernet MAC integrated to the 3rd downstream port and
> two downstream PCIe ports.

> +++ b/Documentation/devicetree/bindings/pci/toshiba,tc9563.yaml
> @@ -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,tc9563.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Toshiba TC9563 PCIe switch
> +
> +maintainers:
> +  - rishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>

s/rishna/Krishna/ ?

> +  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 tc9563 to change FTS, tx amplitude etc.

s/used to do/used to/

> +patternProperties:
> +  "^pcie@[1-3],0$":
> +    description:
> +      child nodes describing the internal downstream ports
> +      the tc9563 switch.

s/ports/ports of/ ?

> +      toshiba,no-dfe-support:
> +        type: boolean
> +        description:
> +          Disable DFE (Decision Feedback Equalizer), which mitigates
> +          intersymbol interference and some reflections caused by impedance mismatches.

Wrap to fit in 78 columns

> +    pcie {
> +        #address-cells = <3>;
> +        #size-cells = <2>;
> +
> +        pcie@0 {
> +            device_type = "pci";
> +            reg = <0x0 0x0 0x0 0x0 0x0>;
> +
> +            #address-cells = <3>;
> +            #size-cells = <2>;
> +            ranges;
> +            bus-range = <0x01 0xff>;
> +
> +            pcie@0,0 {
> +                compatible = "pci1179,0623";
> +
> +                reg = <0x10000 0x0 0x0 0x0 0x0>;
> +                device_type = "pci";
> +                #address-cells = <3>;
> +                #size-cells = <2>;
> +                ranges;
> +                bus-range = <0x02 0xff>;
> ...

> +                pcie@1,0 {
> +                    compatible = "pciclass,0604";
> +                    reg = <0x20800 0x0 0x0 0x0 0x0>;
> +                    #address-cells = <3>;
> +                    #size-cells = <2>;
> +                    device_type = "pci";
> +                    ranges;
> +                    bus-range = <0x03 0xff>;
> +
> +                    toshiba,no-dfe-support;

IIUC, there are two downstream ports available for external devices,
and pcie@1,0 is one of them.

  1) Putting "toshiba,no-dfe-support" in the pcie@1,0 stanza suggests
  that it only applies to that port.

  But from tc9563_pwrctrl_disable_dfe() in "[PATCH v8 6/7] PCI:
  pwrctrl: Add power control driver for tc9563", it looks like it's
  applied to the upstream port and both downstream ports.  So I guess
  my question is putting "toshiba,no-dfe-support" in just one
  downstream port is the right place for it.

  2) I see a lookup of "qcom,no-dfe-support" in [PATCH v8 6/7] PCI:
  pwrctrl: Add power control driver for tc9563; is that supposed to
  match this "toshiba,no-dfe-support"?

> +                };
> +
> +                pcie@2,0 {
> +                    compatible = "pciclass,0604";
> +                    reg = <0x21000 0x0 0x0 0x0 0x0>;
> +                    #address-cells = <3>;
> +                    #size-cells = <2>;
> +                    device_type = "pci";
> +                    ranges;
> +                    bus-range = <0x04 0xff>;
> +                };
> +
> +                pcie@3,0 {
> +                    compatible = "pciclass,0604";
> +                    reg = <0x21800 0x0 0x0 0x0 0x0>;
> +                    #address-cells = <3>;
> +                    #size-cells = <2>;
> +                    device_type = "pci";
> +                    ranges;
> +                    bus-range = <0x05 0xff>;
> +
> +                    toshiba,tx-amplitude-microvolt = <10>;
> +
> +                    ethernet@0,0 {
> +                        reg = <0x50000 0x0 0x0 0x0 0x0>;
> +                    };
> +
> +                    ethernet@0,1 {
> +                        reg = <0x50100 0x0 0x0 0x0 0x0>;
> +                    };
> +                };
> +            };
> +        };
> +    };
> 
> -- 
> 2.34.1
>