[PATCH v4 00/10] PCI: Enable Power and configure the TC956x PCIe switch

[PATCH v4 00/10] PCI: Enable Power and configure the TC956x PCIe switch

[Krishna Chaitanya Chundru]

TC956x is the PCIe switch which has one upstream and three downstream
ports. To one of the downstream ports ethernet MAC is connected as endpoint
device. Other two downstream ports are supposed to connect to external
device. One Host can connect to TC956x by upstream port.

TC956x switch power is controlled by the GPIO's. After powering on
the switch will immediately participate in the link training. if the
host is also ready by that time PCIe link will established. 

The TC956x needs to configured certain parameters like de-emphasis,
disable unused port etc before link is established.

As the controller starts link training before the probe of pwrctl driver,
the PCIe link may come up as soon as we power on the switch. Due to this
configuring the switch itself through i2c will not have any effect as
this configuration needs to done before link training. To avoid this
introduce two functions in pci_ops to start_link() & stop_link() which
will disable the link training if the PCIe link is not up yet.

Enable global IRQ for PCIe controller so that recan can happen when
link was up through global IRQ.
 
This series depends on the https://lore.kernel.org/all/20250124101038.3871768-3-krishna.chundru@oss.qualcomm.com/

Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
changes in v3:
- The QPS615 PCIe switch is rebranded version of Toshiba switch TC956x.
  There is no difference between both the switches, both
  has two open downstream ports and one embedded downstream port
  to which Ethernet MAC is connected.
  As QPS615 is the rebranded version of Toshiba switch rename qps615
  with tc956x so that this driver can be leveraged by all who are using Toshiba switch.
- move common properties like l0s-delay, l1-delay and nfts to pci-host-common.yaml (bjorn H)
- remove axi-clk-frequency property (Krzysztof)
- Update the pattern properties (rob)
- use pci-pci-bridge as the reference (rob)
- change tx-amplitude-millivolt to tx-amplitude-microvolt  (Krzysztof)
- rename qps615_pwrctl_power_on to qps615_pwrctl_bring_up (Bart)
- move the checks for l0s_delay, l1_delay etc to helper functon to
  reduce a level of indentation (bjorn H)
- move platform_set_drvdata to end after there is no error return (bjorn H)
- Replace GPIOD_ASIS to GPIOD_OUT_HIGH (mani)
- Create a common api to check if link is up or not and use that to call
  stop_link() and start_link().
- couple of nits in comments, names etc from everyone
Link to v3: https://lore.kernel.org/all/20241112-qps615_pwr-v3-3-29a1e98aa2b0@quicinc.com/T/
Changes in v2:
- As per offline discussions with rob i2c-parent is best suitable to
  use i2c client device. So use i2c-parent as suggested and remove i2c
  client node reference from the dt-bindings & devicetree.
- Remove "PCI: Change the parent to correctly represent pcie hierarchy"
  as this requires seperate discussions.
- Remove bdf logic to identify the dsp's and usp's to make it generic
  by using the logic that downstream devices will always child of
  upstream node and dsp1, dsp2 will always in same order (dmitry)
- Remove recursive function for parsing devicetree instead parse
  only for required devicetree nodes (dmitry)
- Fix the issue in be & le conversion (dmitry).
- Call put_device for i2c device once done with the usage (dmitry)
- Use $defs to describe common properties between upstream port and
  downstream properties. and remove unneccessary if then. (Krzysztof)
- Place the qcom,qps615 compatibility in dt-binding document in alphabatic order (Krzysztof)
- Rename qcom,no-dfe to describe it as hardware capability and change
  qcom,nfts description to reflect hardware details (Krzysztof)
- Fix the indentation in the example in dt binding (dmitry)
- Add more description to qcom,nfts (dmitry)
- Remove nanosec from the property description (dmitry)
- Link to v2: https://lore.kernel.org/r/linux-arm-msm/20240803-qps615-v2-0-9560b7c71369@quicinc.com/T/
Changes in v1:
- Instead of referencing whole i2c-bus add i2c-client node and reference it (Dmitry)
- Change the regulator's as per the schematics as per offline review
(bjorn Andresson)
- Remove additional host check in bus.c (Bart)
- For stop_link op change return type from int to void (Bart)
- Remove firmware based approach for configuring sequence as suggested
by multiple reviewers.
- Introduce new dt-properties for the switch to configure the switch
as we are replacing the firmware based approach.
- The downstream ports add properties in the child nodes which will
represented in PCIe hierarchy format.
- Removed D3cold D0 sequence in suspend resume for now as it needs
separate discussion.
- Link to v1: https://lore.kernel.org/linux-pci/20240626-qps615-v1-4-2ade7bd91e02@quicinc.com/T/

---
Krishna Chaitanya Chundru (9):
      arm64: dts: qcom: qcs6490-rb3gen2: Add TC956x PCIe switch node
      PCI: Add new start_link() & stop_link function ops
      PCI: dwc: Add host_start_link() & host_start_link() hooks for dwc glue drivers
      PCI: dwc: Implement .start_link(), .stop_link() hooks
      PCI: qcom: Add support for host_stop_link() & host_start_link()
      PCI: PCI: Add pcie_is_link_active() to determine if the PCIe link is active
      PCI: pwrctrl: Add power control driver for tc956x
      dt-bindings: PCI: qcom,pcie-sc7280: Add 'global' interrupt
      arm64: dts: qcom: sc7280: Add 'global' interrupt to the PCIe RC nodes

Krishna chaitanya chundru (1):
      dt-bindings: PCI: Add binding for Toshiba TC956x PCIe switch

 .../devicetree/bindings/pci/qcom,pcie-sc7280.yaml  |   8 +-
 .../devicetree/bindings/pci/toshiba,tc956x.yaml    | 178 ++++++
 arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts       | 116 ++++
 arch/arm64/boot/dts/qcom/sc7280.dtsi               |   7 +-
 drivers/pci/controller/dwc/pcie-designware-host.c  |  18 +
 drivers/pci/controller/dwc/pcie-designware.h       |  16 +
 drivers/pci/controller/dwc/pcie-qcom.c             |  35 ++
 drivers/pci/hotplug/pciehp_hpc.c                   |  13 +-
 drivers/pci/pci.c                                  |  26 +-
 drivers/pci/pwrctrl/Kconfig                        |   6 +
 drivers/pci/pwrctrl/Makefile                       |   1 +
 drivers/pci/pwrctrl/pci-pwrctrl-tc956x.c           | 625 +++++++++++++++++++++
 include/linux/pci.h                                |   7 +
 13 files changed, 1035 insertions(+), 21 deletions(-)
---
base-commit: 09fbf3d502050282bf47ab3babe1d4ed54dd1fd8
change-id: 20250212-qps615_v4_1-f8e62fa11786

Best regards,
-- 
Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>

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

[Krishna Chaitanya Chundru]

From: Krishna chaitanya chundru <quic_krichai@quicinc.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@quicinc.com>
Reviewed-by: Bjorn Andersson <andersson@kernel.org>
---
 .../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
@@ -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@quicinc.com>
+
+description: |
+  Toshiba TC956x PCIe switch has one upstream and three downstream
+  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"
+      - 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:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        description:
+          Change Tx Margin setting for low power consumption.
+
+      tc956x,no-dfe-support:
+        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
+
+required:
+  - vdd18-supply
+  - vdd09-supply
+  - vddc-supply
+  - vddio1-supply
+  - vddio2-supply
+  - vddio18-supply
+  - i2c-parent
+  - reset-gpios
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    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", "pciclass,0604";
+
+                reg = <0x10000 0x0 0x0 0x0 0x0>;
+                device_type = "pci";
+                #address-cells = <3>;
+                #size-cells = <2>;
+                ranges;
+                bus-range = <0x02 0xff>;
+
+                i2c-parent = <&qup_i2c 0x77>;
+
+                vdd18-supply = <&vdd>;
+                vdd09-supply = <&vdd>;
+                vddc-supply = <&vdd>;
+                vddio1-supply = <&vdd>;
+                vddio2-supply = <&vdd>;
+                vddio18-supply = <&vdd>;
+
+                reset-gpios = <&gpio 1 GPIO_ACTIVE_LOW>;
+
+                pcie@1,0 {
+                    reg = <0x20800 0x0 0x0 0x0 0x0>;
+                    #address-cells = <3>;
+                    #size-cells = <2>;
+                    device_type = "pci";
+                    ranges;
+                    bus-range = <0x03 0xff>;
+
+                    tc956x,no-dfe-support;
+                };
+
+                pcie@2,0 {
+                    reg = <0x21000 0x0 0x0 0x0 0x0>;
+                    #address-cells = <3>;
+                    #size-cells = <2>;
+                    device_type = "pci";
+                    ranges;
+                    bus-range = <0x04 0xff>;
+                };
+
+                pcie@3,0 {
+                    reg = <0x21800 0x0 0x0 0x0 0x0>;
+                    #address-cells = <3>;
+                    #size-cells = <2>;
+                    device_type = "pci";
+                    ranges;
+                    bus-range = <0x05 0xff>;
+
+                    tc956x,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

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

[Rob Herring (Arm)]

On Tue, 25 Feb 2025 15:03:58 +0530, Krishna Chaitanya Chundru wrote:
> From: Krishna chaitanya chundru <quic_krichai@quicinc.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@quicinc.com>
> Reviewed-by: Bjorn Andersson <andersson@kernel.org>
> ---
>  .../devicetree/bindings/pci/toshiba,tc956x.yaml    | 178 +++++++++++++++++++++
>  1 file changed, 178 insertions(+)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Warning: Duplicate compatible "pciclass,0604" found in schemas matching "$id":
	http://devicetree.org/schemas/pci/toshiba,tc956x.yaml#
	http://devicetree.org/schemas/pci/pci-pci-bridge.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pci/toshiba,tc956x.example.dtb: pcie@1,0: 'tc956x,no-dfe-support' does not match any of the regexes: '^#.*', '^(at25|bm|devbus|dmacap|dsa|exynos|fsi[ab]|gpio-fan|gpio-key|gpio|gpmc|hdmi|i2c-gpio),.*', '^(keypad|m25p|max8952|max8997|max8998|mpmc),.*', '^(pinctrl-single|#pinctrl-single|PowerPC),.*', '^(pl022|pxa-mmc|rcar_sound|rotary-encoder|s5m8767|sdhci),.*', '^(simple-audio-card|st-plgpio|st-spics|ts),.*', '^100ask,.*', '^70mai,.*', '^8dev,.*', '^GEFanuc,.*', '^IBM,.*', '^ORCL,.*', '^SUNW,.*', '^[a-zA-Z0-9#_][a-zA-Z0-9+\\-._@]{0,63}$', '^[a-zA-Z0-9+\\-._]*@[0-9a-zA-Z,]*$', '^abb,.*', '^abilis,.*', '^abracon,.*', '^abt,.*', '^acbel,.*', '^acelink,.*', '^acer,.*', '^acme,.*', '^actions,.*', '^active-semi,.*', '^ad,.*', '^adafruit,.*', '^adapteva,.*', '^adaptrum,.*', '^adh,.*', '^adi,.*', '^adieng,.*', '^admatec,.*', '^advantech,.*', '^aeroflexgaisler,.*', '^aesop,.*', '^airoha,.*', '^al,.*', '^alcatel,.*', '^aldec,.*', '^alfa-network,.*', '^allegro,.*', '^allegromicro,.*', '^alliedvision,.*', '^allo,.*', '^allwinner,.*', '^alphascale,.*', '^alps,.*', '^alt,.*', '^altr,.*', '^amarula,.*', '^amazon,.*', '^amcc,.*', '^amd,.*', '^amediatech,.*', '^amlogic,.*', '^ampere,.*', '^amphenol,.*', '^ampire,.*', '^ams,.*', '^amstaos,.*', '^analogix,.*', '^anbernic,.*', '^andestech,.*', '^anvo,.*', '^aosong,.*', '^apm,.*', '^apple,.*', '^aptina,.*', '^arasan,.*', '^archermind,.*', '^arcom,.*', '^arctic,.*', '^arcx,.*', '^aries,.*', '^arm,.*', '^armadeus,.*', '^armsom,.*', '^arrow,.*', '^artesyn,.*', '^asahi-kasei,.*', '^asc,.*', '^asix,.*', '^aspeed,.*', '^asrock,.*', '^asteralabs,.*', '^asus,.*', '^atheros,.*', '^atlas,.*', '^atmel,.*', '^auo,.*', '^auvidea,.*', '^avago,.*', '^avia,.*', '^avic,.*', '^avnet,.*', '^awinic,.*', '^axentia,.*', '^axis,.*', '^azoteq,.*', '^azw,.*', '^baikal,.*', '^bananapi,.*', '^beacon,.*', '^beagle,.*', '^belling,.*', '^bhf,.*', '^bigtreetech,.*', '^bitmain,.*', '^blaize,.*', '^blutek,.*', '^boe,.*', '^bosch,.*', '^boundary,.*', '^brcm,.*', '^broadmobi,.*', '^bsh,.*', '^bticino,.*', '^buffalo,.*', '^bur,.*', '^bytedance,.*', '^calamp,.*', '^calao,.*', '^calaosystems,.*', '^calxeda,.*', '^cameo,.*', '^canaan,.*', '^caninos,.*', '^capella,.*', '^cascoda,.*', '^catalyst,.*', '^cavium,.*', '^cct,.*', '^cdns,.*', '^cdtech,.*', '^cellwise,.*', '^ceva,.*', '^chargebyte,.*', '^checkpoint,.*', '^chefree,.*', '^chipidea,.*', '^chipone,.*', '^chipspark,.*', '^chongzhou,.*', '^chrontel,.*', '^chrp,.*', '^chunghwa,.*', '^chuwi,.*', '^ciaa,.*', '^cirrus,.*', '^cisco,.*', '^clockwork,.*', '^cloos,.*', '^cloudengines,.*', '^cnm,.*', '^cnxt,.*', '^colorfly,.*', '^compulab,.*', '^comvetia,.*', '^congatec,.*', '^coolpi,.*', '^coreriver,.*', '^corpro,.*', '^cortina,.*', '^cosmic,.*', '^crane,.*', '^creative,.*', '^crystalfontz,.*', '^csky,.*', '^csq,.*', '^ctera,.*', '^ctu,.*', '^cubietech,.*', '^cudy,.*', '^cui,.*', '^cypress,.*', '^cyx,.*', '^cznic,.*', '^dallas,.*', '^dataimage,.*', '^davicom,.*', '^deepcomputing,.*', '^dell,.*', '^delta,.*', '^densitron,.*', '^denx,.*', '^devantech,.*', '^dfi,.*', '^dfrobot,.*', '^dh,.*', '^difrnce,.*', '^digi,.*', '^digilent,.*', '^dimonoff,.*', '^diodes,.*', '^dioo,.*', '^dlc,.*', '^dlg,.*', '^dlink,.*', '^dmo,.*', '^domintech,.*', '^dongwoon,.*', '^dptechnics,.*', '^dragino,.*', '^dream,.*', '^ds,.*', '^dserve,.*', '^dynaimage,.*', '^ea,.*', '^ebang,.*', '^ebbg,.*', '^ebs-systart,.*', '^ebv,.*', '^eckelmann,.*', '^edgeble,.*', '^edimax,.*', '^edt,.*', '^ees,.*', '^eeti,.*', '^einfochips,.*', '^eink,.*', '^elan,.*', '^element14,.*', '^elgin,.*', '^elida,.*', '^elimo,.*', '^elpida,.*', '^embedfire,.*', '^embest,.*', '^emcraft,.*', '^emlid,.*', '^emmicro,.*', '^empire-electronix,.*', '^emtrion,.*', '^enclustra,.*', '^endless,.*', '^ene,.*', '^energymicro,.*', '^engicam,.*', '^engleder,.*', '^epcos,.*', '^epfl,.*', '^epson,.*', '^esp,.*', '^est,.*', '^ettus,.*', '^eukrea,.*', '^everest,.*', '^everspin,.*', '^evervision,.*', '^exar,.*', '^excito,.*', '^exegin,.*', '^ezchip,.*', '^facebook,.*', '^fairchild,.*', '^fairphone,.*', '^faraday,.*', '^fascontek,.*', '^fastrax,.*', '^fcs,.*', '^feixin,.*', '^feiyang,.*', '^fii,.*', '^firefly,.*', '^focaltech,.*', '^forlinx,.*', '^freebox,.*', '^freecom,.*', '^frida,.*', '^friendlyarm,.*', '^fsl,.*', '^fujitsu,.*', '^fxtec,.*', '^galaxycore,.*', '^gameforce,.*', '^gardena,.*', '^gateway,.*', '^gateworks,.*', '^gcw,.*', '^ge,.*', '^geekbuying,.*', '^gef,.*', '^gehc,.*', '^gemei,.*', '^gemtek,.*', '^genesys,.*', '^genexis,.*', '^geniatech,.*', '^giantec,.*', '^giantplus,.*', '^glinet,.*', '^globalscale,.*', '^globaltop,.*', '^gmt,.*', '^goldelico,.*', '^goodix,.*', '^google,.*', '^goramo,.*', '^gplus,.*', '^grinn,.*', '^grmn,.*', '^gumstix,.*', '^gw,.*', '^hannstar,.*', '^haochuangyi,.*', '^haoyu,.*', '^hardkernel,.*', '^hechuang,.*', '^hideep,.*', '^himax,.*', '^hirschmann,.*', '^hisi,.*', '^hisilicon,.*', '^hit,.*', '^hitex,.*', '^holt,.*', '^holtek,.*', '^honestar,.*', '^honeywell,.*', '^hoperf,.*', '^hoperun,.*', '^hp,.*', '^hpe,.*', '^hsg,.*', '^htc,.*', '^huawei,.*', '^hugsun,.*', '^hwacom,.*', '^hxt,.*', '^hycon,.*', '^hydis,.*', '^hynitron,.*', '^hynix,.*', '^hyundai,.*', '^i2se,.*', '^ibm,.*', '^icplus,.*', '^idt,.*', '^iei,.*', '^ifi,.*', '^ilitek,.*', '^imagis,.*', '^img,.*', '^imi,.*', '^inanbo,.*', '^incircuit,.*', '^indiedroid,.*', '^inet-tek,.*', '^infineon,.*', '^inforce,.*', '^ingenic,.*', '^ingrasys,.*', '^injoinic,.*', '^innocomm,.*', '^innolux,.*', '^inside-secure,.*', '^insignal,.*', '^inspur,.*', '^intel,.*', '^intercontrol,.*', '^invensense,.*', '^inventec,.*', '^inversepath,.*', '^iom,.*', '^irondevice,.*', '^isee,.*', '^isil,.*', '^issi,.*', '^ite,.*', '^itead,.*', '^itian,.*', '^ivo,.*', '^iwave,.*', '^jadard,.*', '^jasonic,.*', '^jdi,.*', '^jedec,.*', '^jenson,.*', '^jesurun,.*', '^jethome,.*', '^jianda,.*', '^jide,.*', '^joz,.*', '^kam,.*', '^karo,.*', '^keithkoep,.*', '^keymile,.*', '^khadas,.*', '^kiebackpeter,.*', '^kinetic,.*', '^kingdisplay,.*', '^kingnovel,.*', '^kionix,.*', '^kobo,.*', '^kobol,.*', '^koe,.*', '^kontron,.*', '^kosagi,.*', '^kvg,.*', '^kyo,.*', '^lacie,.*', '^laird,.*', '^lamobo,.*', '^lantiq,.*', '^lattice,.*', '^lckfb,.*', '^lctech,.*', '^leadtek,.*', '^leez,.*', '^lego,.*', '^lemaker,.*', '^lenovo,.*', '^lg,.*', '^lgphilips,.*', '^libretech,.*', '^licheepi,.*', '^linaro,.*', '^lincolntech,.*', '^lineartechnology,.*', '^linksprite,.*', '^linksys,.*', '^linutronix,.*', '^linux,.*', '^linx,.*', '^liteon,.*', '^litex,.*', '^lltc,.*', '^logicpd,.*', '^logictechno,.*', '^longcheer,.*', '^lontium,.*', '^loongmasses,.*', '^loongson,.*', '^lsi,.*', '^lunzn,.*', '^luxul,.*', '^lwn,.*', '^lxa,.*', '^m5stack,.*', '^macnica,.*', '^mantix,.*', '^mapleboard,.*', '^marantec,.*', '^marvell,.*', '^maxbotix,.*', '^maxim,.*', '^maxlinear,.*', '^mbvl,.*', '^mcube,.*', '^meas,.*', '^mecer,.*', '^mediatek,.*', '^megachips,.*', '^mele,.*', '^melexis,.*', '^melfas,.*', '^mellanox,.*', '^memsensing,.*', '^memsic,.*', '^menlo,.*', '^mentor,.*', '^meraki,.*', '^merrii,.*', '^methode,.*', '^micrel,.*', '^microchip,.*', '^microcrystal,.*', '^micron,.*', '^microsoft,.*', '^microsys,.*', '^microtips,.*', '^mikroe,.*', '^mikrotik,.*', '^milkv,.*', '^miniand,.*', '^minix,.*', '^mips,.*', '^miramems,.*', '^mitsubishi,.*', '^mitsumi,.*', '^mixel,.*', '^miyoo,.*', '^mntre,.*', '^mobileye,.*', '^modtronix,.*', '^moortec,.*', '^mosaixtech,.*', '^motorcomm,.*', '^motorola,.*', '^moxa,.*', '^mpl,.*', '^mps,.*', '^mqmaker,.*', '^mrvl,.*', '^mscc,.*', '^msi,.*', '^mstar,.*', '^mti,.*', '^multi-inno,.*', '^mundoreader,.*', '^murata,.*', '^mxic,.*', '^mxicy,.*', '^myir,.*', '^national,.*', '^neardi,.*', '^nec,.*', '^neofidelity,.*', '^neonode,.*', '^netgear,.*', '^netlogic,.*', '^netron-dy,.*', '^netronix,.*', '^netxeon,.*', '^neweast,.*', '^newhaven,.*', '^newvision,.*', '^nexbox,.*', '^nextthing,.*', '^ni,.*', '^nintendo,.*', '^nlt,.*', '^nokia,.*', '^nordic,.*', '^nothing,.*', '^novatek,.*', '^novtech,.*', '^numonyx,.*', '^nutsboard,.*', '^nuvoton,.*', '^nvd,.*', '^nvidia,.*', '^nxp,.*', '^oceanic,.*', '^ocs,.*', '^oct,.*', '^okaya,.*', '^oki,.*', '^olimex,.*', '^olpc,.*', '^oneplus,.*', '^onie,.*', '^onion,.*', '^onnn,.*', '^ontat,.*', '^opalkelly,.*', '^openailab,.*', '^opencores,.*', '^openembed,.*', '^openpandora,.*', '^openrisc,.*', '^openwrt,.*', '^option,.*', '^oranth,.*', '^orisetech,.*', '^ortustech,.*', '^osddisplays,.*', '^osmc,.*', '^ouya,.*', '^overkiz,.*', '^ovti,.*', '^oxsemi,.*', '^ozzmaker,.*', '^panasonic,.*', '^parade,.*', '^parallax,.*', '^pda,.*', '^pericom,.*', '^pervasive,.*', '^phicomm,.*', '^phytec,.*', '^picochip,.*', '^pine64,.*', '^pineriver,.*', '^pixcir,.*', '^plantower,.*', '^plathome,.*', '^plda,.*', '^plx,.*', '^ply,.*', '^pni,.*', '^pocketbook,.*', '^polaroid,.*', '^polyhex,.*', '^portwell,.*', '^poslab,.*', '^pov,.*', '^powertip,.*', '^powervr,.*', '^powkiddy,.*', '^primeview,.*', '^primux,.*', '^probox2,.*', '^prt,.*', '^pulsedlight,.*', '^purism,.*', '^puya,.*', '^qca,.*', '^qcom,.*', '^qemu,.*', '^qi,.*', '^qiaodian,.*', '^qihua,.*', '^qishenglong,.*', '^qnap,.*', '^quanta,.*', '^radxa,.*', '^raidsonic,.*', '^ralink,.*', '^ramtron,.*', '^raspberrypi,.*', '^raydium,.*', '^rda,.*', '^realtek,.*', '^relfor,.*', '^remarkable,.*', '^renesas,.*', '^rervision,.*', '^revotics,.*', '^rex,.*', '^richtek,.*', '^ricoh,.*', '^rikomagic,.*', '^riot,.*', '^riscv,.*', '^rockchip,.*', '^rocktech,.*', '^rohm,.*', '^ronbo,.*', '^roofull,.*', '^roseapplepi,.*', '^rve,.*', '^saef,.*', '^samsung,.*', '^samtec,.*', '^sancloud,.*', '^sandisk,.*', '^satoz,.*', '^sbs,.*', '^schindler,.*', '^schneider,.*', '^sciosense,.*', '^seagate,.*', '^seeed,.*', '^seirobotics,.*', '^semtech,.*', '^senseair,.*', '^sensirion,.*', '^sensortek,.*', '^sercomm,.*', '^sff,.*', '^sgd,.*', '^sgmicro,.*', '^sgx,.*', '^sharp,.*', '^shift,.*', '^shimafuji,.*', '^shineworld,.*', '^shiratech,.*', '^si-en,.*', '^si-linux,.*', '^siemens,.*', '^sifive,.*', '^siflower,.*', '^sigma,.*', '^sii,.*', '^sil,.*', '^silabs,.*', '^silan,.*', '^silead,.*', '^silergy,.*', '^silex-insight,.*', '^siliconfile,.*', '^siliconmitus,.*', '^silvaco,.*', '^simtek,.*', '^sinlinx,.*', '^sinovoip,.*', '^sinowealth,.*', '^sipeed,.*', '^sirf,.*', '^sis,.*', '^sitronix,.*', '^skov,.*', '^skyworks,.*', '^smartlabs,.*', '^smartrg,.*', '^smi,.*', '^smsc,.*', '^snps,.*', '^sochip,.*', '^socionext,.*', '^solidrun,.*', '^solomon,.*', '^sony,.*', '^sophgo,.*', '^sourceparts,.*', '^spacemit,.*', '^spansion,.*', '^sparkfun,.*', '^spinalhdl,.*', '^sprd,.*', '^square,.*', '^ssi,.*', '^sst,.*', '^sstar,.*', '^st,.*', '^st-ericsson,.*', '^starfive,.*', '^starry,.*', '^startek,.*', '^starterkit,.*', '^ste,.*', '^stericsson,.*', '^storlink,.*', '^storm,.*', '^storopack,.*', '^summit,.*', '^sunchip,.*', '^sundance,.*', '^sunplus,.*', '^supermicro,.*', '^swir,.*', '^syna,.*', '^synology,.*', '^synopsys,.*', '^tbs,.*', '^tbs-biometrics,.*', '^tcg,.*', '^tcl,.*', '^tcs,.*', '^tdo,.*', '^team-source-display,.*', '^technexion,.*', '^technologic,.*', '^techstar,.*', '^techwell,.*', '^teejet,.*', '^teltonika,.*', '^tempo,.*', '^terasic,.*', '^tesla,.*', '^test,.*', '^tfc,.*', '^thead,.*', '^thine,.*', '^thingyjp,.*', '^thundercomm,.*', '^thwc,.*', '^ti,.*', '^tianma,.*', '^tlm,.*', '^tmt,.*', '^topeet,.*', '^topic,.*', '^topland,.*', '^toppoly,.*', '^topwise,.*', '^toradex,.*', '^toshiba,.*', '^toumaz,.*', '^tpk,.*', '^tplink,.*', '^tpo,.*', '^tq,.*', '^transpeed,.*', '^traverse,.*', '^tronfy,.*', '^tronsmart,.*', '^truly,.*', '^tsd,.*', '^turing,.*', '^tyan,.*', '^tyhx,.*', '^u-blox,.*', '^u-boot,.*', '^ubnt,.*', '^ucrobotics,.*', '^udoo,.*', '^ufispace,.*', '^ugoos,.*', '^uni-t,.*', '^uniwest,.*', '^upisemi,.*', '^urt,.*', '^usi,.*', '^usr,.*', '^utoo,.*', '^v3,.*', '^vaisala,.*', '^vamrs,.*', '^variscite,.*', '^vdl,.*', '^vertexcom,.*', '^via,.*', '^vialab,.*', '^vicor,.*', '^videostrong,.*', '^virtio,.*', '^virtual,.*', '^vishay,.*', '^visionox,.*', '^vitesse,.*', '^vivante,.*', '^vivax,.*', '^vocore,.*', '^voipac,.*', '^voltafield,.*', '^vot,.*', '^vscom,.*', '^vxt,.*', '^wacom,.*', '^wanchanglong,.*', '^wand,.*', '^waveshare,.*', '^wd,.*', '^we,.*', '^welltech,.*', '^wetek,.*', '^wexler,.*', '^whwave,.*', '^wi2wi,.*', '^widora,.*', '^wiligear,.*', '^willsemi,.*', '^winbond,.*', '^wingtech,.*', '^winlink,.*', '^winstar,.*', '^wirelesstag,.*', '^wits,.*', '^wlf,.*', '^wm,.*', '^wobo,.*', '^wolfvision,.*', '^x-powers,.*', '^xen,.*', '^xes,.*', '^xiaomi,.*', '^xillybus,.*', '^xingbangda,.*', '^xinpeng,.*', '^xiphera,.*', '^xlnx,.*', '^xnano,.*', '^xunlong,.*', '^xylon,.*', '^yadro,.*', '^yamaha,.*', '^yes-optoelectronics,.*', '^yic,.*', '^yiming,.*', '^ylm,.*', '^yna,.*', '^yones-toptech,.*', '^ys,.*', '^ysoft,.*', '^zarlink,.*', '^zealz,.*', '^zeitec,.*', '^zidoo,.*', '^zii,.*', '^zinitix,.*', '^zkmagic,.*', '^zte,.*', '^zyxel,.*', 'pinctrl-[0-9]+'
	from schema $id: http://devicetree.org/schemas/vendor-prefixes.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pci/toshiba,tc956x.example.dtb: pcie@3,0: 'tc956x,tx-amplitude-microvolt' does not match any of the regexes: '^#.*', '^(at25|bm|devbus|dmacap|dsa|exynos|fsi[ab]|gpio-fan|gpio-key|gpio|gpmc|hdmi|i2c-gpio),.*', '^(keypad|m25p|max8952|max8997|max8998|mpmc),.*', '^(pinctrl-single|#pinctrl-single|PowerPC),.*', '^(pl022|pxa-mmc|rcar_sound|rotary-encoder|s5m8767|sdhci),.*', '^(simple-audio-card|st-plgpio|st-spics|ts),.*', '^100ask,.*', '^70mai,.*', '^8dev,.*', '^GEFanuc,.*', '^IBM,.*', '^ORCL,.*', '^SUNW,.*', '^[a-zA-Z0-9#_][a-zA-Z0-9+\\-._@]{0,63}$', '^[a-zA-Z0-9+\\-._]*@[0-9a-zA-Z,]*$', '^abb,.*', '^abilis,.*', '^abracon,.*', '^abt,.*', '^acbel,.*', '^acelink,.*', '^acer,.*', '^acme,.*', '^actions,.*', '^active-semi,.*', '^ad,.*', '^adafruit,.*', '^adapteva,.*', '^adaptrum,.*', '^adh,.*', '^adi,.*', '^adieng,.*', '^admatec,.*', '^advantech,.*', '^aeroflexgaisler,.*', '^aesop,.*', '^airoha,.*', '^al,.*', '^alcatel,.*', '^aldec,.*', '^alfa-network,.*', '^allegro,.*', '^allegromicro,.*', '^alliedvision,.*', '^allo,.*', '^allwinner,.*', '^alphascale,.*', '^alps,.*', '^alt,.*', '^altr,.*', '^amarula,.*', '^amazon,.*', '^amcc,.*', '^amd,.*', '^amediatech,.*', '^amlogic,.*', '^ampere,.*', '^amphenol,.*', '^ampire,.*', '^ams,.*', '^amstaos,.*', '^analogix,.*', '^anbernic,.*', '^andestech,.*', '^anvo,.*', '^aosong,.*', '^apm,.*', '^apple,.*', '^aptina,.*', '^arasan,.*', '^archermind,.*', '^arcom,.*', '^arctic,.*', '^arcx,.*', '^aries,.*', '^arm,.*', '^armadeus,.*', '^armsom,.*', '^arrow,.*', '^artesyn,.*', '^asahi-kasei,.*', '^asc,.*', '^asix,.*', '^aspeed,.*', '^asrock,.*', '^asteralabs,.*', '^asus,.*', '^atheros,.*', '^atlas,.*', '^atmel,.*', '^auo,.*', '^auvidea,.*', '^avago,.*', '^avia,.*', '^avic,.*', '^avnet,.*', '^awinic,.*', '^axentia,.*', '^axis,.*', '^azoteq,.*', '^azw,.*', '^baikal,.*', '^bananapi,.*', '^beacon,.*', '^beagle,.*', '^belling,.*', '^bhf,.*', '^bigtreetech,.*', '^bitmain,.*', '^blaize,.*', '^blutek,.*', '^boe,.*', '^bosch,.*', '^boundary,.*', '^brcm,.*', '^broadmobi,.*', '^bsh,.*', '^bticino,.*', '^buffalo,.*', '^bur,.*', '^bytedance,.*', '^calamp,.*', '^calao,.*', '^calaosystems,.*', '^calxeda,.*', '^cameo,.*', '^canaan,.*', '^caninos,.*', '^capella,.*', '^cascoda,.*', '^catalyst,.*', '^cavium,.*', '^cct,.*', '^cdns,.*', '^cdtech,.*', '^cellwise,.*', '^ceva,.*', '^chargebyte,.*', '^checkpoint,.*', '^chefree,.*', '^chipidea,.*', '^chipone,.*', '^chipspark,.*', '^chongzhou,.*', '^chrontel,.*', '^chrp,.*', '^chunghwa,.*', '^chuwi,.*', '^ciaa,.*', '^cirrus,.*', '^cisco,.*', '^clockwork,.*', '^cloos,.*', '^cloudengines,.*', '^cnm,.*', '^cnxt,.*', '^colorfly,.*', '^compulab,.*', '^comvetia,.*', '^congatec,.*', '^coolpi,.*', '^coreriver,.*', '^corpro,.*', '^cortina,.*', '^cosmic,.*', '^crane,.*', '^creative,.*', '^crystalfontz,.*', '^csky,.*', '^csq,.*', '^ctera,.*', '^ctu,.*', '^cubietech,.*', '^cudy,.*', '^cui,.*', '^cypress,.*', '^cyx,.*', '^cznic,.*', '^dallas,.*', '^dataimage,.*', '^davicom,.*', '^deepcomputing,.*', '^dell,.*', '^delta,.*', '^densitron,.*', '^denx,.*', '^devantech,.*', '^dfi,.*', '^dfrobot,.*', '^dh,.*', '^difrnce,.*', '^digi,.*', '^digilent,.*', '^dimonoff,.*', '^diodes,.*', '^dioo,.*', '^dlc,.*', '^dlg,.*', '^dlink,.*', '^dmo,.*', '^domintech,.*', '^dongwoon,.*', '^dptechnics,.*', '^dragino,.*', '^dream,.*', '^ds,.*', '^dserve,.*', '^dynaimage,.*', '^ea,.*', '^ebang,.*', '^ebbg,.*', '^ebs-systart,.*', '^ebv,.*', '^eckelmann,.*', '^edgeble,.*', '^edimax,.*', '^edt,.*', '^ees,.*', '^eeti,.*', '^einfochips,.*', '^eink,.*', '^elan,.*', '^element14,.*', '^elgin,.*', '^elida,.*', '^elimo,.*', '^elpida,.*', '^embedfire,.*', '^embest,.*', '^emcraft,.*', '^emlid,.*', '^emmicro,.*', '^empire-electronix,.*', '^emtrion,.*', '^enclustra,.*', '^endless,.*', '^ene,.*', '^energymicro,.*', '^engicam,.*', '^engleder,.*', '^epcos,.*', '^epfl,.*', '^epson,.*', '^esp,.*', '^est,.*', '^ettus,.*', '^eukrea,.*', '^everest,.*', '^everspin,.*', '^evervision,.*', '^exar,.*', '^excito,.*', '^exegin,.*', '^ezchip,.*', '^facebook,.*', '^fairchild,.*', '^fairphone,.*', '^faraday,.*', '^fascontek,.*', '^fastrax,.*', '^fcs,.*', '^feixin,.*', '^feiyang,.*', '^fii,.*', '^firefly,.*', '^focaltech,.*', '^forlinx,.*', '^freebox,.*', '^freecom,.*', '^frida,.*', '^friendlyarm,.*', '^fsl,.*', '^fujitsu,.*', '^fxtec,.*', '^galaxycore,.*', '^gameforce,.*', '^gardena,.*', '^gateway,.*', '^gateworks,.*', '^gcw,.*', '^ge,.*', '^geekbuying,.*', '^gef,.*', '^gehc,.*', '^gemei,.*', '^gemtek,.*', '^genesys,.*', '^genexis,.*', '^geniatech,.*', '^giantec,.*', '^giantplus,.*', '^glinet,.*', '^globalscale,.*', '^globaltop,.*', '^gmt,.*', '^goldelico,.*', '^goodix,.*', '^google,.*', '^goramo,.*', '^gplus,.*', '^grinn,.*', '^grmn,.*', '^gumstix,.*', '^gw,.*', '^hannstar,.*', '^haochuangyi,.*', '^haoyu,.*', '^hardkernel,.*', '^hechuang,.*', '^hideep,.*', '^himax,.*', '^hirschmann,.*', '^hisi,.*', '^hisilicon,.*', '^hit,.*', '^hitex,.*', '^holt,.*', '^holtek,.*', '^honestar,.*', '^honeywell,.*', '^hoperf,.*', '^hoperun,.*', '^hp,.*', '^hpe,.*', '^hsg,.*', '^htc,.*', '^huawei,.*', '^hugsun,.*', '^hwacom,.*', '^hxt,.*', '^hycon,.*', '^hydis,.*', '^hynitron,.*', '^hynix,.*', '^hyundai,.*', '^i2se,.*', '^ibm,.*', '^icplus,.*', '^idt,.*', '^iei,.*', '^ifi,.*', '^ilitek,.*', '^imagis,.*', '^img,.*', '^imi,.*', '^inanbo,.*', '^incircuit,.*', '^indiedroid,.*', '^inet-tek,.*', '^infineon,.*', '^inforce,.*', '^ingenic,.*', '^ingrasys,.*', '^injoinic,.*', '^innocomm,.*', '^innolux,.*', '^inside-secure,.*', '^insignal,.*', '^inspur,.*', '^intel,.*', '^intercontrol,.*', '^invensense,.*', '^inventec,.*', '^inversepath,.*', '^iom,.*', '^irondevice,.*', '^isee,.*', '^isil,.*', '^issi,.*', '^ite,.*', '^itead,.*', '^itian,.*', '^ivo,.*', '^iwave,.*', '^jadard,.*', '^jasonic,.*', '^jdi,.*', '^jedec,.*', '^jenson,.*', '^jesurun,.*', '^jethome,.*', '^jianda,.*', '^jide,.*', '^joz,.*', '^kam,.*', '^karo,.*', '^keithkoep,.*', '^keymile,.*', '^khadas,.*', '^kiebackpeter,.*', '^kinetic,.*', '^kingdisplay,.*', '^kingnovel,.*', '^kionix,.*', '^kobo,.*', '^kobol,.*', '^koe,.*', '^kontron,.*', '^kosagi,.*', '^kvg,.*', '^kyo,.*', '^lacie,.*', '^laird,.*', '^lamobo,.*', '^lantiq,.*', '^lattice,.*', '^lckfb,.*', '^lctech,.*', '^leadtek,.*', '^leez,.*', '^lego,.*', '^lemaker,.*', '^lenovo,.*', '^lg,.*', '^lgphilips,.*', '^libretech,.*', '^licheepi,.*', '^linaro,.*', '^lincolntech,.*', '^lineartechnology,.*', '^linksprite,.*', '^linksys,.*', '^linutronix,.*', '^linux,.*', '^linx,.*', '^liteon,.*', '^litex,.*', '^lltc,.*', '^logicpd,.*', '^logictechno,.*', '^longcheer,.*', '^lontium,.*', '^loongmasses,.*', '^loongson,.*', '^lsi,.*', '^lunzn,.*', '^luxul,.*', '^lwn,.*', '^lxa,.*', '^m5stack,.*', '^macnica,.*', '^mantix,.*', '^mapleboard,.*', '^marantec,.*', '^marvell,.*', '^maxbotix,.*', '^maxim,.*', '^maxlinear,.*', '^mbvl,.*', '^mcube,.*', '^meas,.*', '^mecer,.*', '^mediatek,.*', '^megachips,.*', '^mele,.*', '^melexis,.*', '^melfas,.*', '^mellanox,.*', '^memsensing,.*', '^memsic,.*', '^menlo,.*', '^mentor,.*', '^meraki,.*', '^merrii,.*', '^methode,.*', '^micrel,.*', '^microchip,.*', '^microcrystal,.*', '^micron,.*', '^microsoft,.*', '^microsys,.*', '^microtips,.*', '^mikroe,.*', '^mikrotik,.*', '^milkv,.*', '^miniand,.*', '^minix,.*', '^mips,.*', '^miramems,.*', '^mitsubishi,.*', '^mitsumi,.*', '^mixel,.*', '^miyoo,.*', '^mntre,.*', '^mobileye,.*', '^modtronix,.*', '^moortec,.*', '^mosaixtech,.*', '^motorcomm,.*', '^motorola,.*', '^moxa,.*', '^mpl,.*', '^mps,.*', '^mqmaker,.*', '^mrvl,.*', '^mscc,.*', '^msi,.*', '^mstar,.*', '^mti,.*', '^multi-inno,.*', '^mundoreader,.*', '^murata,.*', '^mxic,.*', '^mxicy,.*', '^myir,.*', '^national,.*', '^neardi,.*', '^nec,.*', '^neofidelity,.*', '^neonode,.*', '^netgear,.*', '^netlogic,.*', '^netron-dy,.*', '^netronix,.*', '^netxeon,.*', '^neweast,.*', '^newhaven,.*', '^newvision,.*', '^nexbox,.*', '^nextthing,.*', '^ni,.*', '^nintendo,.*', '^nlt,.*', '^nokia,.*', '^nordic,.*', '^nothing,.*', '^novatek,.*', '^novtech,.*', '^numonyx,.*', '^nutsboard,.*', '^nuvoton,.*', '^nvd,.*', '^nvidia,.*', '^nxp,.*', '^oceanic,.*', '^ocs,.*', '^oct,.*', '^okaya,.*', '^oki,.*', '^olimex,.*', '^olpc,.*', '^oneplus,.*', '^onie,.*', '^onion,.*', '^onnn,.*', '^ontat,.*', '^opalkelly,.*', '^openailab,.*', '^opencores,.*', '^openembed,.*', '^openpandora,.*', '^openrisc,.*', '^openwrt,.*', '^option,.*', '^oranth,.*', '^orisetech,.*', '^ortustech,.*', '^osddisplays,.*', '^osmc,.*', '^ouya,.*', '^overkiz,.*', '^ovti,.*', '^oxsemi,.*', '^ozzmaker,.*', '^panasonic,.*', '^parade,.*', '^parallax,.*', '^pda,.*', '^pericom,.*', '^pervasive,.*', '^phicomm,.*', '^phytec,.*', '^picochip,.*', '^pine64,.*', '^pineriver,.*', '^pixcir,.*', '^plantower,.*', '^plathome,.*', '^plda,.*', '^plx,.*', '^ply,.*', '^pni,.*', '^pocketbook,.*', '^polaroid,.*', '^polyhex,.*', '^portwell,.*', '^poslab,.*', '^pov,.*', '^powertip,.*', '^powervr,.*', '^powkiddy,.*', '^primeview,.*', '^primux,.*', '^probox2,.*', '^prt,.*', '^pulsedlight,.*', '^purism,.*', '^puya,.*', '^qca,.*', '^qcom,.*', '^qemu,.*', '^qi,.*', '^qiaodian,.*', '^qihua,.*', '^qishenglong,.*', '^qnap,.*', '^quanta,.*', '^radxa,.*', '^raidsonic,.*', '^ralink,.*', '^ramtron,.*', '^raspberrypi,.*', '^raydium,.*', '^rda,.*', '^realtek,.*', '^relfor,.*', '^remarkable,.*', '^renesas,.*', '^rervision,.*', '^revotics,.*', '^rex,.*', '^richtek,.*', '^ricoh,.*', '^rikomagic,.*', '^riot,.*', '^riscv,.*', '^rockchip,.*', '^rocktech,.*', '^rohm,.*', '^ronbo,.*', '^roofull,.*', '^roseapplepi,.*', '^rve,.*', '^saef,.*', '^samsung,.*', '^samtec,.*', '^sancloud,.*', '^sandisk,.*', '^satoz,.*', '^sbs,.*', '^schindler,.*', '^schneider,.*', '^sciosense,.*', '^seagate,.*', '^seeed,.*', '^seirobotics,.*', '^semtech,.*', '^senseair,.*', '^sensirion,.*', '^sensortek,.*', '^sercomm,.*', '^sff,.*', '^sgd,.*', '^sgmicro,.*', '^sgx,.*', '^sharp,.*', '^shift,.*', '^shimafuji,.*', '^shineworld,.*', '^shiratech,.*', '^si-en,.*', '^si-linux,.*', '^siemens,.*', '^sifive,.*', '^siflower,.*', '^sigma,.*', '^sii,.*', '^sil,.*', '^silabs,.*', '^silan,.*', '^silead,.*', '^silergy,.*', '^silex-insight,.*', '^siliconfile,.*', '^siliconmitus,.*', '^silvaco,.*', '^simtek,.*', '^sinlinx,.*', '^sinovoip,.*', '^sinowealth,.*', '^sipeed,.*', '^sirf,.*', '^sis,.*', '^sitronix,.*', '^skov,.*', '^skyworks,.*', '^smartlabs,.*', '^smartrg,.*', '^smi,.*', '^smsc,.*', '^snps,.*', '^sochip,.*', '^socionext,.*', '^solidrun,.*', '^solomon,.*', '^sony,.*', '^sophgo,.*', '^sourceparts,.*', '^spacemit,.*', '^spansion,.*', '^sparkfun,.*', '^spinalhdl,.*', '^sprd,.*', '^square,.*', '^ssi,.*', '^sst,.*', '^sstar,.*', '^st,.*', '^st-ericsson,.*', '^starfive,.*', '^starry,.*', '^startek,.*', '^starterkit,.*', '^ste,.*', '^stericsson,.*', '^storlink,.*', '^storm,.*', '^storopack,.*', '^summit,.*', '^sunchip,.*', '^sundance,.*', '^sunplus,.*', '^supermicro,.*', '^swir,.*', '^syna,.*', '^synology,.*', '^synopsys,.*', '^tbs,.*', '^tbs-biometrics,.*', '^tcg,.*', '^tcl,.*', '^tcs,.*', '^tdo,.*', '^team-source-display,.*', '^technexion,.*', '^technologic,.*', '^techstar,.*', '^techwell,.*', '^teejet,.*', '^teltonika,.*', '^tempo,.*', '^terasic,.*', '^tesla,.*', '^test,.*', '^tfc,.*', '^thead,.*', '^thine,.*', '^thingyjp,.*', '^thundercomm,.*', '^thwc,.*', '^ti,.*', '^tianma,.*', '^tlm,.*', '^tmt,.*', '^topeet,.*', '^topic,.*', '^topland,.*', '^toppoly,.*', '^topwise,.*', '^toradex,.*', '^toshiba,.*', '^toumaz,.*', '^tpk,.*', '^tplink,.*', '^tpo,.*', '^tq,.*', '^transpeed,.*', '^traverse,.*', '^tronfy,.*', '^tronsmart,.*', '^truly,.*', '^tsd,.*', '^turing,.*', '^tyan,.*', '^tyhx,.*', '^u-blox,.*', '^u-boot,.*', '^ubnt,.*', '^ucrobotics,.*', '^udoo,.*', '^ufispace,.*', '^ugoos,.*', '^uni-t,.*', '^uniwest,.*', '^upisemi,.*', '^urt,.*', '^usi,.*', '^usr,.*', '^utoo,.*', '^v3,.*', '^vaisala,.*', '^vamrs,.*', '^variscite,.*', '^vdl,.*', '^vertexcom,.*', '^via,.*', '^vialab,.*', '^vicor,.*', '^videostrong,.*', '^virtio,.*', '^virtual,.*', '^vishay,.*', '^visionox,.*', '^vitesse,.*', '^vivante,.*', '^vivax,.*', '^vocore,.*', '^voipac,.*', '^voltafield,.*', '^vot,.*', '^vscom,.*', '^vxt,.*', '^wacom,.*', '^wanchanglong,.*', '^wand,.*', '^waveshare,.*', '^wd,.*', '^we,.*', '^welltech,.*', '^wetek,.*', '^wexler,.*', '^whwave,.*', '^wi2wi,.*', '^widora,.*', '^wiligear,.*', '^willsemi,.*', '^winbond,.*', '^wingtech,.*', '^winlink,.*', '^winstar,.*', '^wirelesstag,.*', '^wits,.*', '^wlf,.*', '^wm,.*', '^wobo,.*', '^wolfvision,.*', '^x-powers,.*', '^xen,.*', '^xes,.*', '^xiaomi,.*', '^xillybus,.*', '^xingbangda,.*', '^xinpeng,.*', '^xiphera,.*', '^xlnx,.*', '^xnano,.*', '^xunlong,.*', '^xylon,.*', '^yadro,.*', '^yamaha,.*', '^yes-optoelectronics,.*', '^yic,.*', '^yiming,.*', '^ylm,.*', '^yna,.*', '^yones-toptech,.*', '^ys,.*', '^ysoft,.*', '^zarlink,.*', '^zealz,.*', '^zeitec,.*', '^zidoo,.*', '^zii,.*', '^zinitix,.*', '^zkmagic,.*', '^zte,.*', '^zyxel,.*', 'pinctrl-[0-9]+'
	from schema $id: http://devicetree.org/schemas/vendor-prefixes.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pci/toshiba,tc956x.example.dtb: pcie@3,0: tc956x,tx-amplitude-microvolt: 10 is not of type 'array'
	from schema $id: http://devicetree.org/schemas/property-units.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250225-qps615_v4_1-v4-1-e08633a7bdf8@oss.qualcomm.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

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

[Krishna Chaitanya Chundru]

On 2/25/2025 6:53 PM, Rob Herring (Arm) wrote:
> 
> On Tue, 25 Feb 2025 15:03:58 +0530, Krishna Chaitanya Chundru wrote:
>> From: Krishna chaitanya chundru <quic_krichai@quicinc.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@quicinc.com>
>> Reviewed-by: Bjorn Andersson <andersson@kernel.org>
>> ---
>>   .../devicetree/bindings/pci/toshiba,tc956x.yaml    | 178 +++++++++++++++++++++
>>   1 file changed, 178 insertions(+)
>>
> 
> My bot found errors running 'make dt_binding_check' on your patch:
> 
> yamllint warnings/errors:
> 
> dtschema/dtc warnings/errors:
> Warning: Duplicate compatible "pciclass,0604" found in schemas matching "$id":
> 	http://devicetree.org/schemas/pci/toshiba,tc956x.yaml#
> 	http://devicetree.org/schemas/pci/pci-pci-bridge.yaml#
Hi Rob,

As we want to refernce pci-pci-bridge.yaml which is expecting compatible
as "pciclass,0604", we modified the compatible as "pci1179,0623",
"pciclass,0604". Now adding pciclass0604 is giving this warning. can you
suggest correct way to represent this.

The warning is coming from this part
https://github.com/devicetree-org/dt-schema/blob/main/dtschema/validator.py#L397-L404

Thanks & Regards,
Krishna Chaitanya.
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pci/toshiba,tc956x.example.dtb: pcie@1,0: 'tc956x,no-dfe-support' does not match any of the regexes: '^#.*', '^(at25|bm|devbus|dmacap|dsa|exynos|fsi[ab]|gpio-fan|gpio-key|gpio|gpmc|hdmi|i2c-gpio),.*', '^(keypad|m25p|max8952|max8997|max8998|mpmc),.*', '^(pinctrl-single|#pinctrl-single|PowerPC),.*', '^(pl022|pxa-mmc|rcar_sound|rotary-encoder|s5m8767|sdhci),.*', '^(simple-audio-card|st-plgpio|st-spics|ts),.*', '^100ask,.*', '^70mai,.*', '^8dev,.*', '^GEFanuc,.*', '^IBM,.*', '^ORCL,.*', '^SUNW,.*', '^[a-zA-Z0-9#_][a-zA-Z0-9+\\-._@]{0,63}$', '^[a-zA-Z0-9+\\-._]*@[0-9a-zA-Z,]*$', '^abb,.*', '^abilis,.*', '^abracon,.*', '^abt,.*', '^acbel,.*', '^acelink,.*', '^acer,.*', '^acme,.*', '^actions,.*', '^active-semi,.*', '^ad,.*', '^adafruit,.*', '^adapteva,.*', '^adaptrum,.*', '^adh,.*', '^adi,.*', '^adieng,.*', '^admatec,.*', '^advantech,.*', '^aeroflexgaisler,.*', '^aesop,.*', '^airoha,.*', '^al,.*', '^alcatel,.*', '^aldec
>   ,.*', '^alfa-network,.*', '^allegro,.*', '^allegromicro,.*', '^alliedvision,.*', '^allo,.*', '^allwinner,.*', '^alphascale,.*', '^alps,.*', '^alt,.*', '^altr,.*', '^amarula,.*', '^amazon,.*', '^amcc,.*', '^amd,.*', '^amediatech,.*', '^amlogic,.*', '^ampere,.*', '^amphenol,.*', '^ampire,.*', '^ams,.*', '^amstaos,.*', '^analogix,.*', '^anbernic,.*', '^andestech,.*', '^anvo,.*', '^aosong,.*', '^apm,.*', '^apple,.*', '^aptina,.*', '^arasan,.*', '^archermind,.*', '^arcom,.*', '^arctic,.*', '^arcx,.*', '^aries,.*', '^arm,.*', '^armadeus,.*', '^armsom,.*', '^arrow,.*', '^artesyn,.*', '^asahi-kasei,.*', '^asc,.*', '^asix,.*', '^aspeed,.*', '^asrock,.*', '^asteralabs,.*', '^asus,.*', '^atheros,.*', '^atlas,.*', '^atmel,.*', '^auo,.*', '^auvidea,.*', '^avago,.*', '^avia,.*', '^avic,.*', '^avnet,.*', '^awinic,.*', '^axentia,.*', '^axis,.*', '^azoteq,.*', '^azw,.*', '^baikal,.*', '^bananapi,.*', '^beacon,.*', '^beagle,.*', '^belling,.*', '^bhf,.*', '^bigtreetech,.*', '^bitmain,.*', '^blaize,.*'
>   , '^blutek,.*', '^boe,.*', '^bosch,.*', '^boundary,.*', '^brcm,.*', '^broadmobi,.*', '^bsh,.*', '^bticino,.*', '^buffalo,.*', '^bur,.*', '^bytedance,.*', '^calamp,.*', '^calao,.*', '^calaosystems,.*', '^calxeda,.*', '^cameo,.*', '^canaan,.*', '^caninos,.*', '^capella,.*', '^cascoda,.*', '^catalyst,.*', '^cavium,.*', '^cct,.*', '^cdns,.*', '^cdtech,.*', '^cellwise,.*', '^ceva,.*', '^chargebyte,.*', '^checkpoint,.*', '^chefree,.*', '^chipidea,.*', '^chipone,.*', '^chipspark,.*', '^chongzhou,.*', '^chrontel,.*', '^chrp,.*', '^chunghwa,.*', '^chuwi,.*', '^ciaa,.*', '^cirrus,.*', '^cisco,.*', '^clockwork,.*', '^cloos,.*', '^cloudengines,.*', '^cnm,.*', '^cnxt,.*', '^colorfly,.*', '^compulab,.*', '^comvetia,.*', '^congatec,.*', '^coolpi,.*', '^coreriver,.*', '^corpro,.*', '^cortina,.*', '^cosmic,.*', '^crane,.*', '^creative,.*', '^crystalfontz,.*', '^csky,.*', '^csq,.*', '^ctera,.*', '^ctu,.*', '^cubietech,.*', '^cudy,.*', '^cui,.*', '^cypress,.*', '^cyx,.*', '^cznic,.*', '^dallas,.*', '^
>   dataimage,.*', '^davicom,.*', '^deepcomputing,.*', '^dell,.*', '^delta,.*', '^densitron,.*', '^denx,.*', '^devantech,.*', '^dfi,.*', '^dfrobot,.*', '^dh,.*', '^difrnce,.*', '^digi,.*', '^digilent,.*', '^dimonoff,.*', '^diodes,.*', '^dioo,.*', '^dlc,.*', '^dlg,.*', '^dlink,.*', '^dmo,.*', '^domintech,.*', '^dongwoon,.*', '^dptechnics,.*', '^dragino,.*', '^dream,.*', '^ds,.*', '^dserve,.*', '^dynaimage,.*', '^ea,.*', '^ebang,.*', '^ebbg,.*', '^ebs-systart,.*', '^ebv,.*', '^eckelmann,.*', '^edgeble,.*', '^edimax,.*', '^edt,.*', '^ees,.*', '^eeti,.*', '^einfochips,.*', '^eink,.*', '^elan,.*', '^element14,.*', '^elgin,.*', '^elida,.*', '^elimo,.*', '^elpida,.*', '^embedfire,.*', '^embest,.*', '^emcraft,.*', '^emlid,.*', '^emmicro,.*', '^empire-electronix,.*', '^emtrion,.*', '^enclustra,.*', '^endless,.*', '^ene,.*', '^energymicro,.*', '^engicam,.*', '^engleder,.*', '^epcos,.*', '^epfl,.*', '^epson,.*', '^esp,.*', '^est,.*', '^ettus,.*', '^eukrea,.*', '^everest,.*', '^everspin,.*', '^ever
>   vision,.*', '^exar,.*', '^excito,.*', '^exegin,.*', '^ezchip,.*', '^facebook,.*', '^fairchild,.*', '^fairphone,.*', '^faraday,.*', '^fascontek,.*', '^fastrax,.*', '^fcs,.*', '^feixin,.*', '^feiyang,.*', '^fii,.*', '^firefly,.*', '^focaltech,.*', '^forlinx,.*', '^freebox,.*', '^freecom,.*', '^frida,.*', '^friendlyarm,.*', '^fsl,.*', '^fujitsu,.*', '^fxtec,.*', '^galaxycore,.*', '^gameforce,.*', '^gardena,.*', '^gateway,.*', '^gateworks,.*', '^gcw,.*', '^ge,.*', '^geekbuying,.*', '^gef,.*', '^gehc,.*', '^gemei,.*', '^gemtek,.*', '^genesys,.*', '^genexis,.*', '^geniatech,.*', '^giantec,.*', '^giantplus,.*', '^glinet,.*', '^globalscale,.*', '^globaltop,.*', '^gmt,.*', '^goldelico,.*', '^goodix,.*', '^google,.*', '^goramo,.*', '^gplus,.*', '^grinn,.*', '^grmn,.*', '^gumstix,.*', '^gw,.*', '^hannstar,.*', '^haochuangyi,.*', '^haoyu,.*', '^hardkernel,.*', '^hechuang,.*', '^hideep,.*', '^himax,.*', '^hirschmann,.*', '^hisi,.*', '^hisilicon,.*', '^hit,.*', '^hitex,.*', '^holt,.*', '^holtek,.
>   *', '^honestar,.*', '^honeywell,.*', '^hoperf,.*', '^hoperun,.*', '^hp,.*', '^hpe,.*', '^hsg,.*', '^htc,.*', '^huawei,.*', '^hugsun,.*', '^hwacom,.*', '^hxt,.*', '^hycon,.*', '^hydis,.*', '^hynitron,.*', '^hynix,.*', '^hyundai,.*', '^i2se,.*', '^ibm,.*', '^icplus,.*', '^idt,.*', '^iei,.*', '^ifi,.*', '^ilitek,.*', '^imagis,.*', '^img,.*', '^imi,.*', '^inanbo,.*', '^incircuit,.*', '^indiedroid,.*', '^inet-tek,.*', '^infineon,.*', '^inforce,.*', '^ingenic,.*', '^ingrasys,.*', '^injoinic,.*', '^innocomm,.*', '^innolux,.*', '^inside-secure,.*', '^insignal,.*', '^inspur,.*', '^intel,.*', '^intercontrol,.*', '^invensense,.*', '^inventec,.*', '^inversepath,.*', '^iom,.*', '^irondevice,.*', '^isee,.*', '^isil,.*', '^issi,.*', '^ite,.*', '^itead,.*', '^itian,.*', '^ivo,.*', '^iwave,.*', '^jadard,.*', '^jasonic,.*', '^jdi,.*', '^jedec,.*', '^jenson,.*', '^jesurun,.*', '^jethome,.*', '^jianda,.*', '^jide,.*', '^joz,.*', '^kam,.*', '^karo,.*', '^keithkoep,.*', '^keymile,.*', '^khadas,.*', '^kie
>   backpeter,.*', '^kinetic,.*', '^kingdisplay,.*', '^kingnovel,.*', '^kionix,.*', '^kobo,.*', '^kobol,.*', '^koe,.*', '^kontron,.*', '^kosagi,.*', '^kvg,.*', '^kyo,.*', '^lacie,.*', '^laird,.*', '^lamobo,.*', '^lantiq,.*', '^lattice,.*', '^lckfb,.*', '^lctech,.*', '^leadtek,.*', '^leez,.*', '^lego,.*', '^lemaker,.*', '^lenovo,.*', '^lg,.*', '^lgphilips,.*', '^libretech,.*', '^licheepi,.*', '^linaro,.*', '^lincolntech,.*', '^lineartechnology,.*', '^linksprite,.*', '^linksys,.*', '^linutronix,.*', '^linux,.*', '^linx,.*', '^liteon,.*', '^litex,.*', '^lltc,.*', '^logicpd,.*', '^logictechno,.*', '^longcheer,.*', '^lontium,.*', '^loongmasses,.*', '^loongson,.*', '^lsi,.*', '^lunzn,.*', '^luxul,.*', '^lwn,.*', '^lxa,.*', '^m5stack,.*', '^macnica,.*', '^mantix,.*', '^mapleboard,.*', '^marantec,.*', '^marvell,.*', '^maxbotix,.*', '^maxim,.*', '^maxlinear,.*', '^mbvl,.*', '^mcube,.*', '^meas,.*', '^mecer,.*', '^mediatek,.*', '^megachips,.*', '^mele,.*', '^melexis,.*', '^melfas,.*', '^mellanox,
>   .*', '^memsensing,.*', '^memsic,.*', '^menlo,.*', '^mentor,.*', '^meraki,.*', '^merrii,.*', '^methode,.*', '^micrel,.*', '^microchip,.*', '^microcrystal,.*', '^micron,.*', '^microsoft,.*', '^microsys,.*', '^microtips,.*', '^mikroe,.*', '^mikrotik,.*', '^milkv,.*', '^miniand,.*', '^minix,.*', '^mips,.*', '^miramems,.*', '^mitsubishi,.*', '^mitsumi,.*', '^mixel,.*', '^miyoo,.*', '^mntre,.*', '^mobileye,.*', '^modtronix,.*', '^moortec,.*', '^mosaixtech,.*', '^motorcomm,.*', '^motorola,.*', '^moxa,.*', '^mpl,.*', '^mps,.*', '^mqmaker,.*', '^mrvl,.*', '^mscc,.*', '^msi,.*', '^mstar,.*', '^mti,.*', '^multi-inno,.*', '^mundoreader,.*', '^murata,.*', '^mxic,.*', '^mxicy,.*', '^myir,.*', '^national,.*', '^neardi,.*', '^nec,.*', '^neofidelity,.*', '^neonode,.*', '^netgear,.*', '^netlogic,.*', '^netron-dy,.*', '^netronix,.*', '^netxeon,.*', '^neweast,.*', '^newhaven,.*', '^newvision,.*', '^nexbox,.*', '^nextthing,.*', '^ni,.*', '^nintendo,.*', '^nlt,.*', '^nokia,.*', '^nordic,.*', '^nothing,.*
>   ', '^novatek,.*', '^novtech,.*', '^numonyx,.*', '^nutsboard,.*', '^nuvoton,.*', '^nvd,.*', '^nvidia,.*', '^nxp,.*', '^oceanic,.*', '^ocs,.*', '^oct,.*', '^okaya,.*', '^oki,.*', '^olimex,.*', '^olpc,.*', '^oneplus,.*', '^onie,.*', '^onion,.*', '^onnn,.*', '^ontat,.*', '^opalkelly,.*', '^openailab,.*', '^opencores,.*', '^openembed,.*', '^openpandora,.*', '^openrisc,.*', '^openwrt,.*', '^option,.*', '^oranth,.*', '^orisetech,.*', '^ortustech,.*', '^osddisplays,.*', '^osmc,.*', '^ouya,.*', '^overkiz,.*', '^ovti,.*', '^oxsemi,.*', '^ozzmaker,.*', '^panasonic,.*', '^parade,.*', '^parallax,.*', '^pda,.*', '^pericom,.*', '^pervasive,.*', '^phicomm,.*', '^phytec,.*', '^picochip,.*', '^pine64,.*', '^pineriver,.*', '^pixcir,.*', '^plantower,.*', '^plathome,.*', '^plda,.*', '^plx,.*', '^ply,.*', '^pni,.*', '^pocketbook,.*', '^polaroid,.*', '^polyhex,.*', '^portwell,.*', '^poslab,.*', '^pov,.*', '^powertip,.*', '^powervr,.*', '^powkiddy,.*', '^primeview,.*', '^primux,.*', '^probox2,.*', '^prt,.*
>   ', '^pulsedlight,.*', '^purism,.*', '^puya,.*', '^qca,.*', '^qcom,.*', '^qemu,.*', '^qi,.*', '^qiaodian,.*', '^qihua,.*', '^qishenglong,.*', '^qnap,.*', '^quanta,.*', '^radxa,.*', '^raidsonic,.*', '^ralink,.*', '^ramtron,.*', '^raspberrypi,.*', '^raydium,.*', '^rda,.*', '^realtek,.*', '^relfor,.*', '^remarkable,.*', '^renesas,.*', '^rervision,.*', '^revotics,.*', '^rex,.*', '^richtek,.*', '^ricoh,.*', '^rikomagic,.*', '^riot,.*', '^riscv,.*', '^rockchip,.*', '^rocktech,.*', '^rohm,.*', '^ronbo,.*', '^roofull,.*', '^roseapplepi,.*', '^rve,.*', '^saef,.*', '^samsung,.*', '^samtec,.*', '^sancloud,.*', '^sandisk,.*', '^satoz,.*', '^sbs,.*', '^schindler,.*', '^schneider,.*', '^sciosense,.*', '^seagate,.*', '^seeed,.*', '^seirobotics,.*', '^semtech,.*', '^senseair,.*', '^sensirion,.*', '^sensortek,.*', '^sercomm,.*', '^sff,.*', '^sgd,.*', '^sgmicro,.*', '^sgx,.*', '^sharp,.*', '^shift,.*', '^shimafuji,.*', '^shineworld,.*', '^shiratech,.*', '^si-en,.*', '^si-linux,.*', '^siemens,.*', '^si
>   five,.*', '^siflower,.*', '^sigma,.*', '^sii,.*', '^sil,.*', '^silabs,.*', '^silan,.*', '^silead,.*', '^silergy,.*', '^silex-insight,.*', '^siliconfile,.*', '^siliconmitus,.*', '^silvaco,.*', '^simtek,.*', '^sinlinx,.*', '^sinovoip,.*', '^sinowealth,.*', '^sipeed,.*', '^sirf,.*', '^sis,.*', '^sitronix,.*', '^skov,.*', '^skyworks,.*', '^smartlabs,.*', '^smartrg,.*', '^smi,.*', '^smsc,.*', '^snps,.*', '^sochip,.*', '^socionext,.*', '^solidrun,.*', '^solomon,.*', '^sony,.*', '^sophgo,.*', '^sourceparts,.*', '^spacemit,.*', '^spansion,.*', '^sparkfun,.*', '^spinalhdl,.*', '^sprd,.*', '^square,.*', '^ssi,.*', '^sst,.*', '^sstar,.*', '^st,.*', '^st-ericsson,.*', '^starfive,.*', '^starry,.*', '^startek,.*', '^starterkit,.*', '^ste,.*', '^stericsson,.*', '^storlink,.*', '^storm,.*', '^storopack,.*', '^summit,.*', '^sunchip,.*', '^sundance,.*', '^sunplus,.*', '^supermicro,.*', '^swir,.*', '^syna,.*', '^synology,.*', '^synopsys,.*', '^tbs,.*', '^tbs-biometrics,.*', '^tcg,.*', '^tcl,.*', '^tcs
>   ,.*', '^tdo,.*', '^team-source-display,.*', '^technexion,.*', '^technologic,.*', '^techstar,.*', '^techwell,.*', '^teejet,.*', '^teltonika,.*', '^tempo,.*', '^terasic,.*', '^tesla,.*', '^test,.*', '^tfc,.*', '^thead,.*', '^thine,.*', '^thingyjp,.*', '^thundercomm,.*', '^thwc,.*', '^ti,.*', '^tianma,.*', '^tlm,.*', '^tmt,.*', '^topeet,.*', '^topic,.*', '^topland,.*', '^toppoly,.*', '^topwise,.*', '^toradex,.*', '^toshiba,.*', '^toumaz,.*', '^tpk,.*', '^tplink,.*', '^tpo,.*', '^tq,.*', '^transpeed,.*', '^traverse,.*', '^tronfy,.*', '^tronsmart,.*', '^truly,.*', '^tsd,.*', '^turing,.*', '^tyan,.*', '^tyhx,.*', '^u-blox,.*', '^u-boot,.*', '^ubnt,.*', '^ucrobotics,.*', '^udoo,.*', '^ufispace,.*', '^ugoos,.*', '^uni-t,.*', '^uniwest,.*', '^upisemi,.*', '^urt,.*', '^usi,.*', '^usr,.*', '^utoo,.*', '^v3,.*', '^vaisala,.*', '^vamrs,.*', '^variscite,.*', '^vdl,.*', '^vertexcom,.*', '^via,.*', '^vialab,.*', '^vicor,.*', '^videostrong,.*', '^virtio,.*', '^virtual,.*', '^vishay,.*', '^visionox,.
>   *', '^vitesse,.*', '^vivante,.*', '^vivax,.*', '^vocore,.*', '^voipac,.*', '^voltafield,.*', '^vot,.*', '^vscom,.*', '^vxt,.*', '^wacom,.*', '^wanchanglong,.*', '^wand,.*', '^waveshare,.*', '^wd,.*', '^we,.*', '^welltech,.*', '^wetek,.*', '^wexler,.*', '^whwave,.*', '^wi2wi,.*', '^widora,.*', '^wiligear,.*', '^willsemi,.*', '^winbond,.*', '^wingtech,.*', '^winlink,.*', '^winstar,.*', '^wirelesstag,.*', '^wits,.*', '^wlf,.*', '^wm,.*', '^wobo,.*', '^wolfvision,.*', '^x-powers,.*', '^xen,.*', '^xes,.*', '^xiaomi,.*', '^xillybus,.*', '^xingbangda,.*', '^xinpeng,.*', '^xiphera,.*', '^xlnx,.*', '^xnano,.*', '^xunlong,.*', '^xylon,.*', '^yadro,.*', '^yamaha,.*', '^yes-optoelectronics,.*', '^yic,.*', '^yiming,.*', '^ylm,.*', '^yna,.*', '^yones-toptech,.*', '^ys,.*', '^ysoft,.*', '^zarlink,.*', '^zealz,.*', '^zeitec,.*', '^zidoo,.*', '^zii,.*', '^zinitix,.*', '^zkmagic,.*', '^zte,.*', '^zyxel,.*', 'pinctrl-[0-9]+'
> 	from schema $id: http://devicetree.org/schemas/vendor-prefixes.yaml#
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pci/toshiba,tc956x.example.dtb: pcie@3,0: 'tc956x,tx-amplitude-microvolt' does not match any of the regexes: '^#.*', '^(at25|bm|devbus|dmacap|dsa|exynos|fsi[ab]|gpio-fan|gpio-key|gpio|gpmc|hdmi|i2c-gpio),.*', '^(keypad|m25p|max8952|max8997|max8998|mpmc),.*', '^(pinctrl-single|#pinctrl-single|PowerPC),.*', '^(pl022|pxa-mmc|rcar_sound|rotary-encoder|s5m8767|sdhci),.*', '^(simple-audio-card|st-plgpio|st-spics|ts),.*', '^100ask,.*', '^70mai,.*', '^8dev,.*', '^GEFanuc,.*', '^IBM,.*', '^ORCL,.*', '^SUNW,.*', '^[a-zA-Z0-9#_][a-zA-Z0-9+\\-._@]{0,63}$', '^[a-zA-Z0-9+\\-._]*@[0-9a-zA-Z,]*$', '^abb,.*', '^abilis,.*', '^abracon,.*', '^abt,.*', '^acbel,.*', '^acelink,.*', '^acer,.*', '^acme,.*', '^actions,.*', '^active-semi,.*', '^ad,.*', '^adafruit,.*', '^adapteva,.*', '^adaptrum,.*', '^adh,.*', '^adi,.*', '^adieng,.*', '^admatec,.*', '^advantech,.*', '^aeroflexgaisler,.*', '^aesop,.*', '^airoha,.*', '^al,.*', '^alcatel,.*',
>    '^aldec,.*', '^alfa-network,.*', '^allegro,.*', '^allegromicro,.*', '^alliedvision,.*', '^allo,.*', '^allwinner,.*', '^alphascale,.*', '^alps,.*', '^alt,.*', '^altr,.*', '^amarula,.*', '^amazon,.*', '^amcc,.*', '^amd,.*', '^amediatech,.*', '^amlogic,.*', '^ampere,.*', '^amphenol,.*', '^ampire,.*', '^ams,.*', '^amstaos,.*', '^analogix,.*', '^anbernic,.*', '^andestech,.*', '^anvo,.*', '^aosong,.*', '^apm,.*', '^apple,.*', '^aptina,.*', '^arasan,.*', '^archermind,.*', '^arcom,.*', '^arctic,.*', '^arcx,.*', '^aries,.*', '^arm,.*', '^armadeus,.*', '^armsom,.*', '^arrow,.*', '^artesyn,.*', '^asahi-kasei,.*', '^asc,.*', '^asix,.*', '^aspeed,.*', '^asrock,.*', '^asteralabs,.*', '^asus,.*', '^atheros,.*', '^atlas,.*', '^atmel,.*', '^auo,.*', '^auvidea,.*', '^avago,.*', '^avia,.*', '^avic,.*', '^avnet,.*', '^awinic,.*', '^axentia,.*', '^axis,.*', '^azoteq,.*', '^azw,.*', '^baikal,.*', '^bananapi,.*', '^beacon,.*', '^beagle,.*', '^belling,.*', '^bhf,.*', '^bigtreetech,.*', '^bitmain,.*', '^bl
>   aize,.*', '^blutek,.*', '^boe,.*', '^bosch,.*', '^boundary,.*', '^brcm,.*', '^broadmobi,.*', '^bsh,.*', '^bticino,.*', '^buffalo,.*', '^bur,.*', '^bytedance,.*', '^calamp,.*', '^calao,.*', '^calaosystems,.*', '^calxeda,.*', '^cameo,.*', '^canaan,.*', '^caninos,.*', '^capella,.*', '^cascoda,.*', '^catalyst,.*', '^cavium,.*', '^cct,.*', '^cdns,.*', '^cdtech,.*', '^cellwise,.*', '^ceva,.*', '^chargebyte,.*', '^checkpoint,.*', '^chefree,.*', '^chipidea,.*', '^chipone,.*', '^chipspark,.*', '^chongzhou,.*', '^chrontel,.*', '^chrp,.*', '^chunghwa,.*', '^chuwi,.*', '^ciaa,.*', '^cirrus,.*', '^cisco,.*', '^clockwork,.*', '^cloos,.*', '^cloudengines,.*', '^cnm,.*', '^cnxt,.*', '^colorfly,.*', '^compulab,.*', '^comvetia,.*', '^congatec,.*', '^coolpi,.*', '^coreriver,.*', '^corpro,.*', '^cortina,.*', '^cosmic,.*', '^crane,.*', '^creative,.*', '^crystalfontz,.*', '^csky,.*', '^csq,.*', '^ctera,.*', '^ctu,.*', '^cubietech,.*', '^cudy,.*', '^cui,.*', '^cypress,.*', '^cyx,.*', '^cznic,.*', '^dallas
>   ,.*', '^dataimage,.*', '^davicom,.*', '^deepcomputing,.*', '^dell,.*', '^delta,.*', '^densitron,.*', '^denx,.*', '^devantech,.*', '^dfi,.*', '^dfrobot,.*', '^dh,.*', '^difrnce,.*', '^digi,.*', '^digilent,.*', '^dimonoff,.*', '^diodes,.*', '^dioo,.*', '^dlc,.*', '^dlg,.*', '^dlink,.*', '^dmo,.*', '^domintech,.*', '^dongwoon,.*', '^dptechnics,.*', '^dragino,.*', '^dream,.*', '^ds,.*', '^dserve,.*', '^dynaimage,.*', '^ea,.*', '^ebang,.*', '^ebbg,.*', '^ebs-systart,.*', '^ebv,.*', '^eckelmann,.*', '^edgeble,.*', '^edimax,.*', '^edt,.*', '^ees,.*', '^eeti,.*', '^einfochips,.*', '^eink,.*', '^elan,.*', '^element14,.*', '^elgin,.*', '^elida,.*', '^elimo,.*', '^elpida,.*', '^embedfire,.*', '^embest,.*', '^emcraft,.*', '^emlid,.*', '^emmicro,.*', '^empire-electronix,.*', '^emtrion,.*', '^enclustra,.*', '^endless,.*', '^ene,.*', '^energymicro,.*', '^engicam,.*', '^engleder,.*', '^epcos,.*', '^epfl,.*', '^epson,.*', '^esp,.*', '^est,.*', '^ettus,.*', '^eukrea,.*', '^everest,.*', '^everspin,.*'
>   , '^evervision,.*', '^exar,.*', '^excito,.*', '^exegin,.*', '^ezchip,.*', '^facebook,.*', '^fairchild,.*', '^fairphone,.*', '^faraday,.*', '^fascontek,.*', '^fastrax,.*', '^fcs,.*', '^feixin,.*', '^feiyang,.*', '^fii,.*', '^firefly,.*', '^focaltech,.*', '^forlinx,.*', '^freebox,.*', '^freecom,.*', '^frida,.*', '^friendlyarm,.*', '^fsl,.*', '^fujitsu,.*', '^fxtec,.*', '^galaxycore,.*', '^gameforce,.*', '^gardena,.*', '^gateway,.*', '^gateworks,.*', '^gcw,.*', '^ge,.*', '^geekbuying,.*', '^gef,.*', '^gehc,.*', '^gemei,.*', '^gemtek,.*', '^genesys,.*', '^genexis,.*', '^geniatech,.*', '^giantec,.*', '^giantplus,.*', '^glinet,.*', '^globalscale,.*', '^globaltop,.*', '^gmt,.*', '^goldelico,.*', '^goodix,.*', '^google,.*', '^goramo,.*', '^gplus,.*', '^grinn,.*', '^grmn,.*', '^gumstix,.*', '^gw,.*', '^hannstar,.*', '^haochuangyi,.*', '^haoyu,.*', '^hardkernel,.*', '^hechuang,.*', '^hideep,.*', '^himax,.*', '^hirschmann,.*', '^hisi,.*', '^hisilicon,.*', '^hit,.*', '^hitex,.*', '^holt,.*', '^
>   holtek,.*', '^honestar,.*', '^honeywell,.*', '^hoperf,.*', '^hoperun,.*', '^hp,.*', '^hpe,.*', '^hsg,.*', '^htc,.*', '^huawei,.*', '^hugsun,.*', '^hwacom,.*', '^hxt,.*', '^hycon,.*', '^hydis,.*', '^hynitron,.*', '^hynix,.*', '^hyundai,.*', '^i2se,.*', '^ibm,.*', '^icplus,.*', '^idt,.*', '^iei,.*', '^ifi,.*', '^ilitek,.*', '^imagis,.*', '^img,.*', '^imi,.*', '^inanbo,.*', '^incircuit,.*', '^indiedroid,.*', '^inet-tek,.*', '^infineon,.*', '^inforce,.*', '^ingenic,.*', '^ingrasys,.*', '^injoinic,.*', '^innocomm,.*', '^innolux,.*', '^inside-secure,.*', '^insignal,.*', '^inspur,.*', '^intel,.*', '^intercontrol,.*', '^invensense,.*', '^inventec,.*', '^inversepath,.*', '^iom,.*', '^irondevice,.*', '^isee,.*', '^isil,.*', '^issi,.*', '^ite,.*', '^itead,.*', '^itian,.*', '^ivo,.*', '^iwave,.*', '^jadard,.*', '^jasonic,.*', '^jdi,.*', '^jedec,.*', '^jenson,.*', '^jesurun,.*', '^jethome,.*', '^jianda,.*', '^jide,.*', '^joz,.*', '^kam,.*', '^karo,.*', '^keithkoep,.*', '^keymile,.*', '^khadas,.*
>   ', '^kiebackpeter,.*', '^kinetic,.*', '^kingdisplay,.*', '^kingnovel,.*', '^kionix,.*', '^kobo,.*', '^kobol,.*', '^koe,.*', '^kontron,.*', '^kosagi,.*', '^kvg,.*', '^kyo,.*', '^lacie,.*', '^laird,.*', '^lamobo,.*', '^lantiq,.*', '^lattice,.*', '^lckfb,.*', '^lctech,.*', '^leadtek,.*', '^leez,.*', '^lego,.*', '^lemaker,.*', '^lenovo,.*', '^lg,.*', '^lgphilips,.*', '^libretech,.*', '^licheepi,.*', '^linaro,.*', '^lincolntech,.*', '^lineartechnology,.*', '^linksprite,.*', '^linksys,.*', '^linutronix,.*', '^linux,.*', '^linx,.*', '^liteon,.*', '^litex,.*', '^lltc,.*', '^logicpd,.*', '^logictechno,.*', '^longcheer,.*', '^lontium,.*', '^loongmasses,.*', '^loongson,.*', '^lsi,.*', '^lunzn,.*', '^luxul,.*', '^lwn,.*', '^lxa,.*', '^m5stack,.*', '^macnica,.*', '^mantix,.*', '^mapleboard,.*', '^marantec,.*', '^marvell,.*', '^maxbotix,.*', '^maxim,.*', '^maxlinear,.*', '^mbvl,.*', '^mcube,.*', '^meas,.*', '^mecer,.*', '^mediatek,.*', '^megachips,.*', '^mele,.*', '^melexis,.*', '^melfas,.*', '^m
>   ellanox,.*', '^memsensing,.*', '^memsic,.*', '^menlo,.*', '^mentor,.*', '^meraki,.*', '^merrii,.*', '^methode,.*', '^micrel,.*', '^microchip,.*', '^microcrystal,.*', '^micron,.*', '^microsoft,.*', '^microsys,.*', '^microtips,.*', '^mikroe,.*', '^mikrotik,.*', '^milkv,.*', '^miniand,.*', '^minix,.*', '^mips,.*', '^miramems,.*', '^mitsubishi,.*', '^mitsumi,.*', '^mixel,.*', '^miyoo,.*', '^mntre,.*', '^mobileye,.*', '^modtronix,.*', '^moortec,.*', '^mosaixtech,.*', '^motorcomm,.*', '^motorola,.*', '^moxa,.*', '^mpl,.*', '^mps,.*', '^mqmaker,.*', '^mrvl,.*', '^mscc,.*', '^msi,.*', '^mstar,.*', '^mti,.*', '^multi-inno,.*', '^mundoreader,.*', '^murata,.*', '^mxic,.*', '^mxicy,.*', '^myir,.*', '^national,.*', '^neardi,.*', '^nec,.*', '^neofidelity,.*', '^neonode,.*', '^netgear,.*', '^netlogic,.*', '^netron-dy,.*', '^netronix,.*', '^netxeon,.*', '^neweast,.*', '^newhaven,.*', '^newvision,.*', '^nexbox,.*', '^nextthing,.*', '^ni,.*', '^nintendo,.*', '^nlt,.*', '^nokia,.*', '^nordic,.*', '^no
>   thing,.*', '^novatek,.*', '^novtech,.*', '^numonyx,.*', '^nutsboard,.*', '^nuvoton,.*', '^nvd,.*', '^nvidia,.*', '^nxp,.*', '^oceanic,.*', '^ocs,.*', '^oct,.*', '^okaya,.*', '^oki,.*', '^olimex,.*', '^olpc,.*', '^oneplus,.*', '^onie,.*', '^onion,.*', '^onnn,.*', '^ontat,.*', '^opalkelly,.*', '^openailab,.*', '^opencores,.*', '^openembed,.*', '^openpandora,.*', '^openrisc,.*', '^openwrt,.*', '^option,.*', '^oranth,.*', '^orisetech,.*', '^ortustech,.*', '^osddisplays,.*', '^osmc,.*', '^ouya,.*', '^overkiz,.*', '^ovti,.*', '^oxsemi,.*', '^ozzmaker,.*', '^panasonic,.*', '^parade,.*', '^parallax,.*', '^pda,.*', '^pericom,.*', '^pervasive,.*', '^phicomm,.*', '^phytec,.*', '^picochip,.*', '^pine64,.*', '^pineriver,.*', '^pixcir,.*', '^plantower,.*', '^plathome,.*', '^plda,.*', '^plx,.*', '^ply,.*', '^pni,.*', '^pocketbook,.*', '^polaroid,.*', '^polyhex,.*', '^portwell,.*', '^poslab,.*', '^pov,.*', '^powertip,.*', '^powervr,.*', '^powkiddy,.*', '^primeview,.*', '^primux,.*', '^probox2,.*',
>   '^prt,.*', '^pulsedlight,.*', '^purism,.*', '^puya,.*', '^qca,.*', '^qcom,.*', '^qemu,.*', '^qi,.*', '^qiaodian,.*', '^qihua,.*', '^qishenglong,.*', '^qnap,.*', '^quanta,.*', '^radxa,.*', '^raidsonic,.*', '^ralink,.*', '^ramtron,.*', '^raspberrypi,.*', '^raydium,.*', '^rda,.*', '^realtek,.*', '^relfor,.*', '^remarkable,.*', '^renesas,.*', '^rervision,.*', '^revotics,.*', '^rex,.*', '^richtek,.*', '^ricoh,.*', '^rikomagic,.*', '^riot,.*', '^riscv,.*', '^rockchip,.*', '^rocktech,.*', '^rohm,.*', '^ronbo,.*', '^roofull,.*', '^roseapplepi,.*', '^rve,.*', '^saef,.*', '^samsung,.*', '^samtec,.*', '^sancloud,.*', '^sandisk,.*', '^satoz,.*', '^sbs,.*', '^schindler,.*', '^schneider,.*', '^sciosense,.*', '^seagate,.*', '^seeed,.*', '^seirobotics,.*', '^semtech,.*', '^senseair,.*', '^sensirion,.*', '^sensortek,.*', '^sercomm,.*', '^sff,.*', '^sgd,.*', '^sgmicro,.*', '^sgx,.*', '^sharp,.*', '^shift,.*', '^shimafuji,.*', '^shineworld,.*', '^shiratech,.*', '^si-en,.*', '^si-linux,.*', '^siemens,.
>   *', '^sifive,.*', '^siflower,.*', '^sigma,.*', '^sii,.*', '^sil,.*', '^silabs,.*', '^silan,.*', '^silead,.*', '^silergy,.*', '^silex-insight,.*', '^siliconfile,.*', '^siliconmitus,.*', '^silvaco,.*', '^simtek,.*', '^sinlinx,.*', '^sinovoip,.*', '^sinowealth,.*', '^sipeed,.*', '^sirf,.*', '^sis,.*', '^sitronix,.*', '^skov,.*', '^skyworks,.*', '^smartlabs,.*', '^smartrg,.*', '^smi,.*', '^smsc,.*', '^snps,.*', '^sochip,.*', '^socionext,.*', '^solidrun,.*', '^solomon,.*', '^sony,.*', '^sophgo,.*', '^sourceparts,.*', '^spacemit,.*', '^spansion,.*', '^sparkfun,.*', '^spinalhdl,.*', '^sprd,.*', '^square,.*', '^ssi,.*', '^sst,.*', '^sstar,.*', '^st,.*', '^st-ericsson,.*', '^starfive,.*', '^starry,.*', '^startek,.*', '^starterkit,.*', '^ste,.*', '^stericsson,.*', '^storlink,.*', '^storm,.*', '^storopack,.*', '^summit,.*', '^sunchip,.*', '^sundance,.*', '^sunplus,.*', '^supermicro,.*', '^swir,.*', '^syna,.*', '^synology,.*', '^synopsys,.*', '^tbs,.*', '^tbs-biometrics,.*', '^tcg,.*', '^tcl,.*
>   ', '^tcs,.*', '^tdo,.*', '^team-source-display,.*', '^technexion,.*', '^technologic,.*', '^techstar,.*', '^techwell,.*', '^teejet,.*', '^teltonika,.*', '^tempo,.*', '^terasic,.*', '^tesla,.*', '^test,.*', '^tfc,.*', '^thead,.*', '^thine,.*', '^thingyjp,.*', '^thundercomm,.*', '^thwc,.*', '^ti,.*', '^tianma,.*', '^tlm,.*', '^tmt,.*', '^topeet,.*', '^topic,.*', '^topland,.*', '^toppoly,.*', '^topwise,.*', '^toradex,.*', '^toshiba,.*', '^toumaz,.*', '^tpk,.*', '^tplink,.*', '^tpo,.*', '^tq,.*', '^transpeed,.*', '^traverse,.*', '^tronfy,.*', '^tronsmart,.*', '^truly,.*', '^tsd,.*', '^turing,.*', '^tyan,.*', '^tyhx,.*', '^u-blox,.*', '^u-boot,.*', '^ubnt,.*', '^ucrobotics,.*', '^udoo,.*', '^ufispace,.*', '^ugoos,.*', '^uni-t,.*', '^uniwest,.*', '^upisemi,.*', '^urt,.*', '^usi,.*', '^usr,.*', '^utoo,.*', '^v3,.*', '^vaisala,.*', '^vamrs,.*', '^variscite,.*', '^vdl,.*', '^vertexcom,.*', '^via,.*', '^vialab,.*', '^vicor,.*', '^videostrong,.*', '^virtio,.*', '^virtual,.*', '^vishay,.*', '^vi
>   sionox,.*', '^vitesse,.*', '^vivante,.*', '^vivax,.*', '^vocore,.*', '^voipac,.*', '^voltafield,.*', '^vot,.*', '^vscom,.*', '^vxt,.*', '^wacom,.*', '^wanchanglong,.*', '^wand,.*', '^waveshare,.*', '^wd,.*', '^we,.*', '^welltech,.*', '^wetek,.*', '^wexler,.*', '^whwave,.*', '^wi2wi,.*', '^widora,.*', '^wiligear,.*', '^willsemi,.*', '^winbond,.*', '^wingtech,.*', '^winlink,.*', '^winstar,.*', '^wirelesstag,.*', '^wits,.*', '^wlf,.*', '^wm,.*', '^wobo,.*', '^wolfvision,.*', '^x-powers,.*', '^xen,.*', '^xes,.*', '^xiaomi,.*', '^xillybus,.*', '^xingbangda,.*', '^xinpeng,.*', '^xiphera,.*', '^xlnx,.*', '^xnano,.*', '^xunlong,.*', '^xylon,.*', '^yadro,.*', '^yamaha,.*', '^yes-optoelectronics,.*', '^yic,.*', '^yiming,.*', '^ylm,.*', '^yna,.*', '^yones-toptech,.*', '^ys,.*', '^ysoft,.*', '^zarlink,.*', '^zealz,.*', '^zeitec,.*', '^zidoo,.*', '^zii,.*', '^zinitix,.*', '^zkmagic,.*', '^zte,.*', '^zyxel,.*', 'pinctrl-[0-9]+'
> 	from schema $id: http://devicetree.org/schemas/vendor-prefixes.yaml#
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pci/toshiba,tc956x.example.dtb: pcie@3,0: tc956x,tx-amplitude-microvolt: 10 is not of type 'array'
> 	from schema $id: http://devicetree.org/schemas/property-units.yaml#
> 
> doc reference errors (make refcheckdocs):
> 
> See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250225-qps615_v4_1-v4-1-e08633a7bdf8@oss.qualcomm.com
> 
> The base for the series is generally the latest rc1. A different dependency
> should be noted in *this* patch.
> 
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
> 
> pip3 install dtschema --upgrade
> 
> Please check and re-submit after running the above command yourself. Note
> that DT_SCHEMA_FILES can be set to your schema file to speed up checking
> your schema. However, it must be unset to test all examples with your schema.
> 

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

[Manivannan Sadhasivam]

On Fri, Feb 28, 2025 at 04:26:23AM +0530, Krishna Chaitanya Chundru wrote:
> 
> 
> On 2/25/2025 6:53 PM, Rob Herring (Arm) wrote:
> > 
> > On Tue, 25 Feb 2025 15:03:58 +0530, Krishna Chaitanya Chundru wrote:
> > > From: Krishna chaitanya chundru <quic_krichai@quicinc.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@quicinc.com>
> > > Reviewed-by: Bjorn Andersson <andersson@kernel.org>
> > > ---
> > >   .../devicetree/bindings/pci/toshiba,tc956x.yaml    | 178 +++++++++++++++++++++
> > >   1 file changed, 178 insertions(+)
> > > 
> > 
> > My bot found errors running 'make dt_binding_check' on your patch:
> > 
> > yamllint warnings/errors:
> > 
> > dtschema/dtc warnings/errors:
> > Warning: Duplicate compatible "pciclass,0604" found in schemas matching "$id":
> > 	http://devicetree.org/schemas/pci/toshiba,tc956x.yaml#
> > 	http://devicetree.org/schemas/pci/pci-pci-bridge.yaml#
> Hi Rob,
> 
> As we want to refernce pci-pci-bridge.yaml which is expecting compatible
> as "pciclass,0604", we modified the compatible as "pci1179,0623",
> "pciclass,0604". Now adding pciclass0604 is giving this warning. can you
> suggest correct way to represent this.
> 

I think the PCI-PCI bridge compatible should be part of the actual bridge nodes
inside the switch. I still stand by my view that the bridge compatible doesn't
make sense for the top level switch node as this switch is a sort of MFD.

So you should do:

	pcie@0,0 {
		compatible = "pci1179,0623";
		reg = <0x10000 0x0 0x0 0x0 0x0>;
		...

		pcie@1,0 {
			compatible = "pciclass,0604";
			reg = <0x20800 0x0 0x0 0x0 0x0>;
			...
		};
	};


- Mani

-- 
மணிவண்ணன் சதாசிவம்

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

[Krishna Chaitanya Chundru]

On 3/13/2025 11:26 AM, Manivannan Sadhasivam wrote:
> On Fri, Feb 28, 2025 at 04:26:23AM +0530, Krishna Chaitanya Chundru wrote:
>>
>>
>> On 2/25/2025 6:53 PM, Rob Herring (Arm) wrote:
>>>
>>> On Tue, 25 Feb 2025 15:03:58 +0530, Krishna Chaitanya Chundru wrote:
>>>> From: Krishna chaitanya chundru <quic_krichai@quicinc.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@quicinc.com>
>>>> Reviewed-by: Bjorn Andersson <andersson@kernel.org>
>>>> ---
>>>>    .../devicetree/bindings/pci/toshiba,tc956x.yaml    | 178 +++++++++++++++++++++
>>>>    1 file changed, 178 insertions(+)
>>>>
>>>
>>> My bot found errors running 'make dt_binding_check' on your patch:
>>>
>>> yamllint warnings/errors:
>>>
>>> dtschema/dtc warnings/errors:
>>> Warning: Duplicate compatible "pciclass,0604" found in schemas matching "$id":
>>> 	http://devicetree.org/schemas/pci/toshiba,tc956x.yaml#
>>> 	http://devicetree.org/schemas/pci/pci-pci-bridge.yaml#
>> Hi Rob,
>>
>> As we want to refernce pci-pci-bridge.yaml which is expecting compatible
>> as "pciclass,0604", we modified the compatible as "pci1179,0623",
>> "pciclass,0604". Now adding pciclass0604 is giving this warning. can you
>> suggest correct way to represent this.
>>
> 
> I think the PCI-PCI bridge compatible should be part of the actual bridge nodes
> inside the switch. I still stand by my view that the bridge compatible doesn't
> make sense for the top level switch node as this switch is a sort of MFD.
> 
> So you should do:
> 
> 	pcie@0,0 {
> 		compatible = "pci1179,0623";
> 		reg = <0x10000 0x0 0x0 0x0 0x0>;
> 		...
> 
> 		pcie@1,0 {
> 			compatible = "pciclass,0604";
> 			reg = <0x20800 0x0 0x0 0x0 0x0>;
> 			...
> 		};
> 	};
> 
Thanks mani for the inputs on this.
For child nodes I will make pci-pci-bridge as a reference and for the
main node( pcie@0,0) I will pci-bus-common.yaml as reference.

- Krishna Chaitanya.
> 
> - Mani
> 

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

[Krzysztof Kozlowski]

On Tue, Feb 25, 2025 at 03:03:58PM +0530, Krishna Chaitanya Chundru wrote:
> From: Krishna chaitanya chundru <quic_krichai@quicinc.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@quicinc.com>
> Reviewed-by: Bjorn Andersson <andersson@kernel.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@oss.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@quicinc.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

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

[Krishna Chaitanya Chundru]

On 2/26/2025 1:00 PM, Krzysztof Kozlowski wrote:
> On Tue, Feb 25, 2025 at 03:03:58PM +0530, Krishna Chaitanya Chundru wrote:
>> From: Krishna chaitanya chundru <quic_krichai@quicinc.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@quicinc.com>
>> Reviewed-by: Bjorn Andersson <andersson@kernel.org>
> 
> Drop, file was named entirely different. I see other changes, altough
> comparing with b4 is impossible.
> 
ack
> Why b4 does not work for this patch?
> 
>    b4 diff '20250225-qps615_v4_1-v4-1-e08633a7bdf8@oss.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?
> 
There is workspace issue at my end instead of fixing them I tried to
create new branch. I will try to use same changeset from next series
onwards.
> 
>> ---
>>   .../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@quicinc.com>
>> +
>> +description: |
>> +  Toshiba TC956x PCIe switch has one upstream and three downstream
> 
> TC9560? Which one are you using here?
> 
'x' is the series of the switch and we use tc9563
>> +  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?
> 
I will remove the, in the next patch.
>> +      - 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.
> 
In V3 I got a comment saying "-microvolt does not work for you?"
so based on this we changed to microvolt.
>> +        $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?
>
It is my mistake I taught the comment was applicable to removed property
"qps615,axi-clk-freq-hz". It is applicable to all the properties I will
correct it in the next patch.

For some reasons there are no errors shown in my workspace when I run
make dt_binding_check DT_SCHEMA_FILES=toshiba,tc956x.yaml
& make CHECK_DTBS=y qcom/qcs6490-rb3gen2.dtb

I will check my tools versions etc and will fix them.

>> +        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.
> 
ack.

- Krishna Chaitanya.
> Best regards,
> Krzysztof
> 

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

[Krzysztof Kozlowski]

On 27/02/2025 04:53, Krishna Chaitanya Chundru wrote:
>>> +
>>> +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.
>>
> In V3 I got a comment saying "-microvolt does not work for you?"
> so based on this we changed to microvolt.


And then what pops up when you test? Drop ref which is pointed out by
testing, example-schema and all other bindings.

Best regards,
Krzysztof

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

[Konrad Dybcio]

On 2/26/25 8:30 AM, Krzysztof Kozlowski wrote:
> On Tue, Feb 25, 2025 at 03:03:58PM +0530, Krishna Chaitanya Chundru wrote:
>> From: Krishna chaitanya chundru <quic_krichai@quicinc.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@quicinc.com>
>> Reviewed-by: Bjorn Andersson <andersson@kernel.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@oss.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?

Yes, an overly broad one. Let's use the actual name going forward.

Konrad

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

[Krishna Chaitanya Chundru]

On 3/25/2025 7:26 PM, Konrad Dybcio wrote:
> On 2/26/25 8:30 AM, Krzysztof Kozlowski wrote:
>> On Tue, Feb 25, 2025 at 03:03:58PM +0530, Krishna Chaitanya Chundru wrote:
>>> From: Krishna chaitanya chundru <quic_krichai@quicinc.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@quicinc.com>
>>> Reviewed-by: Bjorn Andersson <andersson@kernel.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@oss.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?
> 
> Yes, an overly broad one. Let's use the actual name going forward.
> 
ok I will update the next series the name from tc956x to tc9563 as
suggested.

- Krishna Chaitanya.
> Konrad

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

[Konrad Dybcio]

On 4/1/25 7:52 AM, Krishna Chaitanya Chundru wrote:
> 
> 
> On 3/25/2025 7:26 PM, Konrad Dybcio wrote:
>> On 2/26/25 8:30 AM, Krzysztof Kozlowski wrote:
>>> On Tue, Feb 25, 2025 at 03:03:58PM +0530, Krishna Chaitanya Chundru wrote:
>>>> From: Krishna chaitanya chundru <quic_krichai@quicinc.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@quicinc.com>
>>>> Reviewed-by: Bjorn Andersson <andersson@kernel.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@oss.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?
>>
>> Yes, an overly broad one. Let's use the actual name going forward.
>>
> ok I will update the next series the name from tc956x to tc9563 as
> suggested.

As per internal discussions, 956*4* would be the correct name for this one

Konrad

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

[Konrad Dybcio]

On 4/9/25 3:22 PM, Konrad Dybcio wrote:
> On 4/1/25 7:52 AM, Krishna Chaitanya Chundru wrote:
>>
>>
>> On 3/25/2025 7:26 PM, Konrad Dybcio wrote:
>>> On 2/26/25 8:30 AM, Krzysztof Kozlowski wrote:
>>>> On Tue, Feb 25, 2025 at 03:03:58PM +0530, Krishna Chaitanya Chundru wrote:
>>>>> From: Krishna chaitanya chundru <quic_krichai@quicinc.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@quicinc.com>
>>>>> Reviewed-by: Bjorn Andersson <andersson@kernel.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@oss.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?
>>>
>>> Yes, an overly broad one. Let's use the actual name going forward.
>>>
>> ok I will update the next series the name from tc956x to tc9563 as
>> suggested.
> 
> As per internal discussions, 956*4* would be the correct name for this one

I misread. It's supposed to be 3. Sorry for the confusion.

Konrad

[PATCH v4 02/10] arm64: dts: qcom: qcs6490-rb3gen2: Add TC956x PCIe switch node

[Krishna Chaitanya Chundru]

Add a node for the TC956x PCIe switch, which has three downstream ports.
Two embedded Ethernet devices are present on one of the downstream ports.

Power to the TC956x is supplied through two LDO regulators, controlled by
two GPIOs, which are added as fixed regulators. Configure the TC956x
through I2C.

Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
Reviewed-by: Bjorn Andersson <andersson@kernel.org>
Acked-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 116 +++++++++++++++++++++++++++
 arch/arm64/boot/dts/qcom/sc7280.dtsi         |   2 +-
 2 files changed, 117 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
index 7a36c90ad4ec..13dbb24a3179 100644
--- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
+++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
@@ -218,6 +218,31 @@ vph_pwr: vph-pwr-regulator {
 		regulator-min-microvolt = <3700000>;
 		regulator-max-microvolt = <3700000>;
 	};
+
+	vdd_ntn_0p9: regulator-vdd-ntn-0p9 {
+		compatible = "regulator-fixed";
+		regulator-name = "VDD_NTN_0P9";
+		gpio = <&pm8350c_gpios 2 GPIO_ACTIVE_HIGH>;
+		regulator-min-microvolt = <899400>;
+		regulator-max-microvolt = <899400>;
+		enable-active-high;
+		pinctrl-0 = <&ntn_0p9_en>;
+		pinctrl-names = "default";
+		regulator-enable-ramp-delay = <4300>;
+	};
+
+	vdd_ntn_1p8: regulator-vdd-ntn-1p8 {
+		compatible = "regulator-fixed";
+		regulator-name = "VDD_NTN_1P8";
+		gpio = <&pm8350c_gpios 3 GPIO_ACTIVE_HIGH>;
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <1800000>;
+		enable-active-high;
+		pinctrl-0 = <&ntn_1p8_en>;
+		pinctrl-names = "default";
+		regulator-enable-ramp-delay = <10000>;
+	};
+
 };
 
 &apps_rsc {
@@ -735,6 +760,75 @@ &pcie1_phy {
 	status = "okay";
 };
 
+&pcie1_port {
+	pcie@0,0 {
+		compatible = "pci1179,0623", "pciclass,0604";
+		reg = <0x10000 0x0 0x0 0x0 0x0>;
+		#address-cells = <3>;
+		#size-cells = <2>;
+
+		device_type = "pci";
+		ranges;
+		bus-range = <0x2 0xff>;
+
+		vddc-supply = <&vdd_ntn_0p9>;
+		vdd18-supply = <&vdd_ntn_1p8>;
+		vdd09-supply = <&vdd_ntn_0p9>;
+		vddio1-supply = <&vdd_ntn_1p8>;
+		vddio2-supply = <&vdd_ntn_1p8>;
+		vddio18-supply = <&vdd_ntn_1p8>;
+
+		i2c-parent = <&i2c0 0x77>;
+
+		reset-gpios = <&pm8350c_gpios 1 GPIO_ACTIVE_LOW>;
+
+		pcie@1,0 {
+			reg = <0x20800 0x0 0x0 0x0 0x0>;
+			#address-cells = <3>;
+			#size-cells = <2>;
+
+			device_type = "pci";
+			ranges;
+			bus-range = <0x3 0xff>;
+		};
+
+		pcie@2,0 {
+			reg = <0x21000 0x0 0x0 0x0 0x0>;
+			#address-cells = <3>;
+			#size-cells = <2>;
+
+			device_type = "pci";
+			ranges;
+			bus-range = <0x4 0xff>;
+		};
+
+		pcie@3,0 {
+			reg = <0x21800 0x0 0x0 0x0 0x0>;
+			#address-cells = <3>;
+			#size-cells = <2>;
+			device_type = "pci";
+			ranges;
+			bus-range = <0x5 0xff>;
+
+			pcie@0,0 {
+				reg = <0x50000 0x0 0x0 0x0 0x0>;
+				#address-cells = <3>;
+				#size-cells = <2>;
+				device_type = "pci";
+				ranges;
+			};
+
+			pcie@0,1 {
+				reg = <0x50100 0x0 0x0 0x0 0x0>;
+				#address-cells = <3>;
+				#size-cells = <2>;
+				device_type = "pci";
+				ranges;
+			};
+		};
+	};
+};
+
 &pm7325_gpios {
 	kypd_vol_up_n: kypd-vol-up-n-state {
 		pins = "gpio6";
@@ -839,6 +933,28 @@ &sdhc_2 {
 	status = "okay";
 };
 
+&pm8350c_gpios {
+	ntn_0p9_en: ntn-0p9-en-state {
+		pins = "gpio2";
+		function = "normal";
+
+		bias-disable;
+		input-disable;
+		output-enable;
+		power-source = <0>;
+	};
+
+	ntn_1p8_en: ntn-1p8-en-state {
+		pins = "gpio3";
+		function = "normal";
+
+		bias-disable;
+		input-disable;
+		output-enable;
+		power-source = <0>;
+	};
+};
+
 &tlmm {
 	gpio-reserved-ranges = <32 2>, /* ADSP */
 			       <48 4>; /* NFC */
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index 0f2caf36910b..b2e2b1f26731 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -2284,7 +2284,7 @@ pcie1: pcie@1c08000 {
 
 			status = "disabled";
 
-			pcie@0 {
+			pcie1_port: pcie@0 {
 				device_type = "pci";
 				reg = <0x0 0x0 0x0 0x0 0x0>;
 				bus-range = <0x01 0xff>;

-- 
2.34.1

Re: [PATCH v4 02/10] arm64: dts: qcom: qcs6490-rb3gen2: Add TC956x PCIe switch node

[Dmitry Baryshkov]

On Tue, Feb 25, 2025 at 03:03:59PM +0530, Krishna Chaitanya Chundru wrote:
> Add a node for the TC956x PCIe switch, which has three downstream ports.
> Two embedded Ethernet devices are present on one of the downstream ports.
> 
> Power to the TC956x is supplied through two LDO regulators, controlled by
> two GPIOs, which are added as fixed regulators. Configure the TC956x
> through I2C.
> 
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> Reviewed-by: Bjorn Andersson <andersson@kernel.org>
> Acked-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
>  arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 116 +++++++++++++++++++++++++++
>  arch/arm64/boot/dts/qcom/sc7280.dtsi         |   2 +-
>  2 files changed, 117 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> index 7a36c90ad4ec..13dbb24a3179 100644
> --- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> +++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> @@ -218,6 +218,31 @@ vph_pwr: vph-pwr-regulator {
>  		regulator-min-microvolt = <3700000>;
>  		regulator-max-microvolt = <3700000>;
>  	};
> +
> +	vdd_ntn_0p9: regulator-vdd-ntn-0p9 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "VDD_NTN_0P9";
> +		gpio = <&pm8350c_gpios 2 GPIO_ACTIVE_HIGH>;
> +		regulator-min-microvolt = <899400>;
> +		regulator-max-microvolt = <899400>;
> +		enable-active-high;
> +		pinctrl-0 = <&ntn_0p9_en>;
> +		pinctrl-names = "default";
> +		regulator-enable-ramp-delay = <4300>;
> +	};
> +
> +	vdd_ntn_1p8: regulator-vdd-ntn-1p8 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "VDD_NTN_1P8";
> +		gpio = <&pm8350c_gpios 3 GPIO_ACTIVE_HIGH>;
> +		regulator-min-microvolt = <1800000>;
> +		regulator-max-microvolt = <1800000>;
> +		enable-active-high;
> +		pinctrl-0 = <&ntn_1p8_en>;
> +		pinctrl-names = "default";
> +		regulator-enable-ramp-delay = <10000>;
> +	};
> +
>  };
>  
>  &apps_rsc {
> @@ -735,6 +760,75 @@ &pcie1_phy {
>  	status = "okay";
>  };
>  
> +&pcie1_port {
> +	pcie@0,0 {
> +		compatible = "pci1179,0623", "pciclass,0604";
> +		reg = <0x10000 0x0 0x0 0x0 0x0>;
> +		#address-cells = <3>;
> +		#size-cells = <2>;
> +
> +		device_type = "pci";
> +		ranges;
> +		bus-range = <0x2 0xff>;
> +
> +		vddc-supply = <&vdd_ntn_0p9>;
> +		vdd18-supply = <&vdd_ntn_1p8>;
> +		vdd09-supply = <&vdd_ntn_0p9>;
> +		vddio1-supply = <&vdd_ntn_1p8>;
> +		vddio2-supply = <&vdd_ntn_1p8>;
> +		vddio18-supply = <&vdd_ntn_1p8>;
> +
> +		i2c-parent = <&i2c0 0x77>;
> +
> +		reset-gpios = <&pm8350c_gpios 1 GPIO_ACTIVE_LOW>;
> +
> +		pcie@1,0 {

PCIe bus can be autodetected. Is there a reason for describing all the
ports and a full topology? If so, it should be stated in the commit
message.

> +			reg = <0x20800 0x0 0x0 0x0 0x0>;
> +			#address-cells = <3>;
> +			#size-cells = <2>;
> +
> +			device_type = "pci";
> +			ranges;
> +			bus-range = <0x3 0xff>;
> +		};
> +
> +		pcie@2,0 {
> +			reg = <0x21000 0x0 0x0 0x0 0x0>;
> +			#address-cells = <3>;
> +			#size-cells = <2>;
> +
> +			device_type = "pci";
> +			ranges;
> +			bus-range = <0x4 0xff>;
> +		};
> +
> +		pcie@3,0 {

-- 
With best wishes
Dmitry

Re: [PATCH v4 02/10] arm64: dts: qcom: qcs6490-rb3gen2: Add TC956x PCIe switch node

[Krishna Chaitanya Chundru]

On 2/25/2025 5:19 PM, Dmitry Baryshkov wrote:
> On Tue, Feb 25, 2025 at 03:03:59PM +0530, Krishna Chaitanya Chundru wrote:
>> Add a node for the TC956x PCIe switch, which has three downstream ports.
>> Two embedded Ethernet devices are present on one of the downstream ports.
>>
>> Power to the TC956x is supplied through two LDO regulators, controlled by
>> two GPIOs, which are added as fixed regulators. Configure the TC956x
>> through I2C.
>>
>> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
>> Reviewed-by: Bjorn Andersson <andersson@kernel.org>
>> Acked-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
>> ---
>>   arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 116 +++++++++++++++++++++++++++
>>   arch/arm64/boot/dts/qcom/sc7280.dtsi         |   2 +-
>>   2 files changed, 117 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
>> index 7a36c90ad4ec..13dbb24a3179 100644
>> --- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
>> +++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
>> @@ -218,6 +218,31 @@ vph_pwr: vph-pwr-regulator {
>>   		regulator-min-microvolt = <3700000>;
>>   		regulator-max-microvolt = <3700000>;
>>   	};
>> +
>> +	vdd_ntn_0p9: regulator-vdd-ntn-0p9 {
>> +		compatible = "regulator-fixed";
>> +		regulator-name = "VDD_NTN_0P9";
>> +		gpio = <&pm8350c_gpios 2 GPIO_ACTIVE_HIGH>;
>> +		regulator-min-microvolt = <899400>;
>> +		regulator-max-microvolt = <899400>;
>> +		enable-active-high;
>> +		pinctrl-0 = <&ntn_0p9_en>;
>> +		pinctrl-names = "default";
>> +		regulator-enable-ramp-delay = <4300>;
>> +	};
>> +
>> +	vdd_ntn_1p8: regulator-vdd-ntn-1p8 {
>> +		compatible = "regulator-fixed";
>> +		regulator-name = "VDD_NTN_1P8";
>> +		gpio = <&pm8350c_gpios 3 GPIO_ACTIVE_HIGH>;
>> +		regulator-min-microvolt = <1800000>;
>> +		regulator-max-microvolt = <1800000>;
>> +		enable-active-high;
>> +		pinctrl-0 = <&ntn_1p8_en>;
>> +		pinctrl-names = "default";
>> +		regulator-enable-ramp-delay = <10000>;
>> +	};
>> +
>>   };
>>   
>>   &apps_rsc {
>> @@ -735,6 +760,75 @@ &pcie1_phy {
>>   	status = "okay";
>>   };
>>   
>> +&pcie1_port {
>> +	pcie@0,0 {
>> +		compatible = "pci1179,0623", "pciclass,0604";
>> +		reg = <0x10000 0x0 0x0 0x0 0x0>;
>> +		#address-cells = <3>;
>> +		#size-cells = <2>;
>> +
>> +		device_type = "pci";
>> +		ranges;
>> +		bus-range = <0x2 0xff>;
>> +
>> +		vddc-supply = <&vdd_ntn_0p9>;
>> +		vdd18-supply = <&vdd_ntn_1p8>;
>> +		vdd09-supply = <&vdd_ntn_0p9>;
>> +		vddio1-supply = <&vdd_ntn_1p8>;
>> +		vddio2-supply = <&vdd_ntn_1p8>;
>> +		vddio18-supply = <&vdd_ntn_1p8>;
>> +
>> +		i2c-parent = <&i2c0 0x77>;
>> +
>> +		reset-gpios = <&pm8350c_gpios 1 GPIO_ACTIVE_LOW>;
>> +
>> +		pcie@1,0 {
> 
> PCIe bus can be autodetected. Is there a reason for describing all the
> ports and a full topology? If so, it should be stated in the commit
> message.
> 
As these ports are fixed we are defining them here, I will mention this
in the commit message. It is similar to how we added pcieport for all
the platforms, I tried to add full topology here. And if we want to
configure any ports like l0s entry delay, l1 entry delay etc in future
we need these full topology to be present.

- Krishna Chaitanya.
>> +			reg = <0x20800 0x0 0x0 0x0 0x0>;
>> +			#address-cells = <3>;
>> +			#size-cells = <2>;
>> +
>> +			device_type = "pci";
>> +			ranges;
>> +			bus-range = <0x3 0xff>;
>> +		};
>> +
>> +		pcie@2,0 {
>> +			reg = <0x21000 0x0 0x0 0x0 0x0>;
>> +			#address-cells = <3>;
>> +			#size-cells = <2>;
>> +
>> +			device_type = "pci";
>> +			ranges;
>> +			bus-range = <0x4 0xff>;
>> +		};
>> +
>> +		pcie@3,0 {
> 

Re: [PATCH v4 02/10] arm64: dts: qcom: qcs6490-rb3gen2: Add TC956x PCIe switch node

[Dmitry Baryshkov]

On Mon, Mar 17, 2025 at 03:05:17PM +0530, Krishna Chaitanya Chundru wrote:
> 
> 
> On 2/25/2025 5:19 PM, Dmitry Baryshkov wrote:
> > On Tue, Feb 25, 2025 at 03:03:59PM +0530, Krishna Chaitanya Chundru wrote:
> > > Add a node for the TC956x PCIe switch, which has three downstream ports.
> > > Two embedded Ethernet devices are present on one of the downstream ports.
> > > 
> > > Power to the TC956x is supplied through two LDO regulators, controlled by
> > > two GPIOs, which are added as fixed regulators. Configure the TC956x
> > > through I2C.
> > > 
> > > Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> > > Reviewed-by: Bjorn Andersson <andersson@kernel.org>
> > > Acked-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > > ---
> > >   arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 116 +++++++++++++++++++++++++++
> > >   arch/arm64/boot/dts/qcom/sc7280.dtsi         |   2 +-
> > >   2 files changed, 117 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> > > index 7a36c90ad4ec..13dbb24a3179 100644
> > > --- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> > > +++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> > > @@ -218,6 +218,31 @@ vph_pwr: vph-pwr-regulator {
> > >   		regulator-min-microvolt = <3700000>;
> > >   		regulator-max-microvolt = <3700000>;
> > >   	};
> > > +
> > > +	vdd_ntn_0p9: regulator-vdd-ntn-0p9 {
> > > +		compatible = "regulator-fixed";
> > > +		regulator-name = "VDD_NTN_0P9";
> > > +		gpio = <&pm8350c_gpios 2 GPIO_ACTIVE_HIGH>;
> > > +		regulator-min-microvolt = <899400>;
> > > +		regulator-max-microvolt = <899400>;
> > > +		enable-active-high;
> > > +		pinctrl-0 = <&ntn_0p9_en>;
> > > +		pinctrl-names = "default";
> > > +		regulator-enable-ramp-delay = <4300>;
> > > +	};
> > > +
> > > +	vdd_ntn_1p8: regulator-vdd-ntn-1p8 {
> > > +		compatible = "regulator-fixed";
> > > +		regulator-name = "VDD_NTN_1P8";
> > > +		gpio = <&pm8350c_gpios 3 GPIO_ACTIVE_HIGH>;
> > > +		regulator-min-microvolt = <1800000>;
> > > +		regulator-max-microvolt = <1800000>;
> > > +		enable-active-high;
> > > +		pinctrl-0 = <&ntn_1p8_en>;
> > > +		pinctrl-names = "default";
> > > +		regulator-enable-ramp-delay = <10000>;
> > > +	};
> > > +
> > >   };
> > >   &apps_rsc {
> > > @@ -735,6 +760,75 @@ &pcie1_phy {
> > >   	status = "okay";
> > >   };
> > > +&pcie1_port {
> > > +	pcie@0,0 {
> > > +		compatible = "pci1179,0623", "pciclass,0604";
> > > +		reg = <0x10000 0x0 0x0 0x0 0x0>;
> > > +		#address-cells = <3>;
> > > +		#size-cells = <2>;
> > > +
> > > +		device_type = "pci";
> > > +		ranges;
> > > +		bus-range = <0x2 0xff>;
> > > +
> > > +		vddc-supply = <&vdd_ntn_0p9>;
> > > +		vdd18-supply = <&vdd_ntn_1p8>;
> > > +		vdd09-supply = <&vdd_ntn_0p9>;
> > > +		vddio1-supply = <&vdd_ntn_1p8>;
> > > +		vddio2-supply = <&vdd_ntn_1p8>;
> > > +		vddio18-supply = <&vdd_ntn_1p8>;
> > > +
> > > +		i2c-parent = <&i2c0 0x77>;
> > > +
> > > +		reset-gpios = <&pm8350c_gpios 1 GPIO_ACTIVE_LOW>;
> > > +
> > > +		pcie@1,0 {
> > 
> > PCIe bus can be autodetected. Is there a reason for describing all the
> > ports and a full topology? If so, it should be stated in the commit
> > message.
> > 
> As these ports are fixed we are defining them here, I will mention this
> in the commit message. It is similar to how we added pcieport for all
> the platforms, I tried to add full topology here. And if we want to
> configure any ports like l0s entry delay, l1 entry delay etc in future
> we need these full topology to be present.

Ack

> 
> - Krishna Chaitanya.
> > > +			reg = <0x20800 0x0 0x0 0x0 0x0>;
> > > +			#address-cells = <3>;
> > > +			#size-cells = <2>;
> > > +
> > > +			device_type = "pci";
> > > +			ranges;
> > > +			bus-range = <0x3 0xff>;
> > > +		};
> > > +
> > > +		pcie@2,0 {
> > > +			reg = <0x21000 0x0 0x0 0x0 0x0>;
> > > +			#address-cells = <3>;
> > > +			#size-cells = <2>;
> > > +
> > > +			device_type = "pci";
> > > +			ranges;
> > > +			bus-range = <0x4 0xff>;
> > > +		};
> > > +
> > > +		pcie@3,0 {
> > 

-- 
With best wishes
Dmitry

Re: [PATCH v4 02/10] arm64: dts: qcom: qcs6490-rb3gen2: Add TC956x PCIe switch node

[Dmitry Baryshkov]

On Tue, Feb 25, 2025 at 03:03:59PM +0530, Krishna Chaitanya Chundru wrote:
> Add a node for the TC956x PCIe switch, which has three downstream ports.
> Two embedded Ethernet devices are present on one of the downstream ports.
> 
> Power to the TC956x is supplied through two LDO regulators, controlled by
> two GPIOs, which are added as fixed regulators. Configure the TC956x
> through I2C.
> 
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> Reviewed-by: Bjorn Andersson <andersson@kernel.org>
> Acked-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
>  arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 116 +++++++++++++++++++++++++++
>  arch/arm64/boot/dts/qcom/sc7280.dtsi         |   2 +-
>  2 files changed, 117 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> index 7a36c90ad4ec..13dbb24a3179 100644
> --- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> +++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> @@ -218,6 +218,31 @@ vph_pwr: vph-pwr-regulator {
>  		regulator-min-microvolt = <3700000>;
>  		regulator-max-microvolt = <3700000>;
>  	};
> +
> +	vdd_ntn_0p9: regulator-vdd-ntn-0p9 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "VDD_NTN_0P9";
> +		gpio = <&pm8350c_gpios 2 GPIO_ACTIVE_HIGH>;
> +		regulator-min-microvolt = <899400>;
> +		regulator-max-microvolt = <899400>;
> +		enable-active-high;
> +		pinctrl-0 = <&ntn_0p9_en>;
> +		pinctrl-names = "default";
> +		regulator-enable-ramp-delay = <4300>;
> +	};
> +
> +	vdd_ntn_1p8: regulator-vdd-ntn-1p8 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "VDD_NTN_1P8";
> +		gpio = <&pm8350c_gpios 3 GPIO_ACTIVE_HIGH>;
> +		regulator-min-microvolt = <1800000>;
> +		regulator-max-microvolt = <1800000>;
> +		enable-active-high;
> +		pinctrl-0 = <&ntn_1p8_en>;
> +		pinctrl-names = "default";
> +		regulator-enable-ramp-delay = <10000>;
> +	};
> +
>  };
>  
>  &apps_rsc {
> @@ -735,6 +760,75 @@ &pcie1_phy {
>  	status = "okay";
>  };
>  
> +&pcie1_port {
> +	pcie@0,0 {
> +		compatible = "pci1179,0623", "pciclass,0604";
> +		reg = <0x10000 0x0 0x0 0x0 0x0>;
> +		#address-cells = <3>;
> +		#size-cells = <2>;
> +
> +		device_type = "pci";
> +		ranges;
> +		bus-range = <0x2 0xff>;
> +
> +		vddc-supply = <&vdd_ntn_0p9>;
> +		vdd18-supply = <&vdd_ntn_1p8>;
> +		vdd09-supply = <&vdd_ntn_0p9>;
> +		vddio1-supply = <&vdd_ntn_1p8>;
> +		vddio2-supply = <&vdd_ntn_1p8>;
> +		vddio18-supply = <&vdd_ntn_1p8>;
> +
> +		i2c-parent = <&i2c0 0x77>;
> +
> +		reset-gpios = <&pm8350c_gpios 1 GPIO_ACTIVE_LOW>;

Please add pinctrl blck for this reset line.

-- 
With best wishes
Dmitry

Re: [PATCH v4 02/10] arm64: dts: qcom: qcs6490-rb3gen2: Add TC956x PCIe switch node

[Dmitry Baryshkov]

On Tue, Feb 25, 2025 at 03:03:59PM +0530, Krishna Chaitanya Chundru wrote:
> Add a node for the TC956x PCIe switch, which has three downstream ports.
> Two embedded Ethernet devices are present on one of the downstream ports.
> 
> Power to the TC956x is supplied through two LDO regulators, controlled by
> two GPIOs, which are added as fixed regulators. Configure the TC956x
> through I2C.
> 
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> Reviewed-by: Bjorn Andersson <andersson@kernel.org>
> Acked-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
>  arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 116 +++++++++++++++++++++++++++
>  arch/arm64/boot/dts/qcom/sc7280.dtsi         |   2 +-
>  2 files changed, 117 insertions(+), 1 deletion(-)
> 
> @@ -735,6 +760,75 @@ &pcie1_phy {
>  	status = "okay";
>  };
>  
> +&pcie1_port {
> +	pcie@0,0 {
> +		compatible = "pci1179,0623", "pciclass,0604";
> +		reg = <0x10000 0x0 0x0 0x0 0x0>;
> +		#address-cells = <3>;
> +		#size-cells = <2>;
> +
> +		device_type = "pci";
> +		ranges;
> +		bus-range = <0x2 0xff>;
> +
> +		vddc-supply = <&vdd_ntn_0p9>;
> +		vdd18-supply = <&vdd_ntn_1p8>;
> +		vdd09-supply = <&vdd_ntn_0p9>;
> +		vddio1-supply = <&vdd_ntn_1p8>;
> +		vddio2-supply = <&vdd_ntn_1p8>;
> +		vddio18-supply = <&vdd_ntn_1p8>;
> +
> +		i2c-parent = <&i2c0 0x77>;
> +
> +		reset-gpios = <&pm8350c_gpios 1 GPIO_ACTIVE_LOW>;
> +

I think I've responded here, but I'm not sure where the message went:
please add pinctrl entry for this pin.

-- 
With best wishes
Dmitry

Re: [PATCH v4 02/10] arm64: dts: qcom: qcs6490-rb3gen2: Add TC956x PCIe switch node

[Krishna Chaitanya Chundru]

On 3/17/2025 4:57 PM, Dmitry Baryshkov wrote:
> On Tue, Feb 25, 2025 at 03:03:59PM +0530, Krishna Chaitanya Chundru wrote:
>> Add a node for the TC956x PCIe switch, which has three downstream ports.
>> Two embedded Ethernet devices are present on one of the downstream ports.
>>
>> Power to the TC956x is supplied through two LDO regulators, controlled by
>> two GPIOs, which are added as fixed regulators. Configure the TC956x
>> through I2C.
>>
>> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
>> Reviewed-by: Bjorn Andersson <andersson@kernel.org>
>> Acked-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
>> ---
>>   arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 116 +++++++++++++++++++++++++++
>>   arch/arm64/boot/dts/qcom/sc7280.dtsi         |   2 +-
>>   2 files changed, 117 insertions(+), 1 deletion(-)
>>
>> @@ -735,6 +760,75 @@ &pcie1_phy {
>>   	status = "okay";
>>   };
>>   
>> +&pcie1_port {
>> +	pcie@0,0 {
>> +		compatible = "pci1179,0623", "pciclass,0604";
>> +		reg = <0x10000 0x0 0x0 0x0 0x0>;
>> +		#address-cells = <3>;
>> +		#size-cells = <2>;
>> +
>> +		device_type = "pci";
>> +		ranges;
>> +		bus-range = <0x2 0xff>;
>> +
>> +		vddc-supply = <&vdd_ntn_0p9>;
>> +		vdd18-supply = <&vdd_ntn_1p8>;
>> +		vdd09-supply = <&vdd_ntn_0p9>;
>> +		vddio1-supply = <&vdd_ntn_1p8>;
>> +		vddio2-supply = <&vdd_ntn_1p8>;
>> +		vddio18-supply = <&vdd_ntn_1p8>;
>> +
>> +		i2c-parent = <&i2c0 0x77>;
>> +
>> +		reset-gpios = <&pm8350c_gpios 1 GPIO_ACTIVE_LOW>;
>> +
> 
> I think I've responded here, but I'm not sure where the message went:
> please add pinctrl entry for this pin.
>
Do we need to also add pinctrl property for this node and refer the
pinctrl entry for this pin?

- Krishna Chaitanya.

Re: [PATCH v4 02/10] arm64: dts: qcom: qcs6490-rb3gen2: Add TC956x PCIe switch node

[Dmitry Baryshkov]

On Tue, 18 Mar 2025 at 18:11, Krishna Chaitanya Chundru
<krishna.chundru@oss.qualcomm.com> wrote:
>
>
>
> On 3/17/2025 4:57 PM, Dmitry Baryshkov wrote:
> > On Tue, Feb 25, 2025 at 03:03:59PM +0530, Krishna Chaitanya Chundru wrote:
> >> Add a node for the TC956x PCIe switch, which has three downstream ports.
> >> Two embedded Ethernet devices are present on one of the downstream ports.
> >>
> >> Power to the TC956x is supplied through two LDO regulators, controlled by
> >> two GPIOs, which are added as fixed regulators. Configure the TC956x
> >> through I2C.
> >>
> >> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> >> Reviewed-by: Bjorn Andersson <andersson@kernel.org>
> >> Acked-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> >> ---
> >>   arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 116 +++++++++++++++++++++++++++
> >>   arch/arm64/boot/dts/qcom/sc7280.dtsi         |   2 +-
> >>   2 files changed, 117 insertions(+), 1 deletion(-)
> >>
> >> @@ -735,6 +760,75 @@ &pcie1_phy {
> >>      status = "okay";
> >>   };
> >>
> >> +&pcie1_port {
> >> +    pcie@0,0 {
> >> +            compatible = "pci1179,0623", "pciclass,0604";
> >> +            reg = <0x10000 0x0 0x0 0x0 0x0>;
> >> +            #address-cells = <3>;
> >> +            #size-cells = <2>;
> >> +
> >> +            device_type = "pci";
> >> +            ranges;
> >> +            bus-range = <0x2 0xff>;
> >> +
> >> +            vddc-supply = <&vdd_ntn_0p9>;
> >> +            vdd18-supply = <&vdd_ntn_1p8>;
> >> +            vdd09-supply = <&vdd_ntn_0p9>;
> >> +            vddio1-supply = <&vdd_ntn_1p8>;
> >> +            vddio2-supply = <&vdd_ntn_1p8>;
> >> +            vddio18-supply = <&vdd_ntn_1p8>;
> >> +
> >> +            i2c-parent = <&i2c0 0x77>;
> >> +
> >> +            reset-gpios = <&pm8350c_gpios 1 GPIO_ACTIVE_LOW>;
> >> +
> >
> > I think I've responded here, but I'm not sure where the message went:
> > please add pinctrl entry for this pin.
> >
> Do we need to also add pinctrl property for this node and refer the
> pinctrl entry for this pin?

I think that is what I've asked for, was that not?

-- 
With best wishes
Dmitry

Re: [PATCH v4 02/10] arm64: dts: qcom: qcs6490-rb3gen2: Add TC956x PCIe switch node

[Krishna Chaitanya Chundru]

On 3/18/2025 10:30 PM, Dmitry Baryshkov wrote:
> On Tue, 18 Mar 2025 at 18:11, Krishna Chaitanya Chundru
> <krishna.chundru@oss.qualcomm.com> wrote:
>>
>>
>>
>> On 3/17/2025 4:57 PM, Dmitry Baryshkov wrote:
>>> On Tue, Feb 25, 2025 at 03:03:59PM +0530, Krishna Chaitanya Chundru wrote:
>>>> Add a node for the TC956x PCIe switch, which has three downstream ports.
>>>> Two embedded Ethernet devices are present on one of the downstream ports.
>>>>
>>>> Power to the TC956x is supplied through two LDO regulators, controlled by
>>>> two GPIOs, which are added as fixed regulators. Configure the TC956x
>>>> through I2C.
>>>>
>>>> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
>>>> Reviewed-by: Bjorn Andersson <andersson@kernel.org>
>>>> Acked-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
>>>> ---
>>>>    arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 116 +++++++++++++++++++++++++++
>>>>    arch/arm64/boot/dts/qcom/sc7280.dtsi         |   2 +-
>>>>    2 files changed, 117 insertions(+), 1 deletion(-)
>>>>
>>>> @@ -735,6 +760,75 @@ &pcie1_phy {
>>>>       status = "okay";
>>>>    };
>>>>
>>>> +&pcie1_port {
>>>> +    pcie@0,0 {
>>>> +            compatible = "pci1179,0623", "pciclass,0604";
>>>> +            reg = <0x10000 0x0 0x0 0x0 0x0>;
>>>> +            #address-cells = <3>;
>>>> +            #size-cells = <2>;
>>>> +
>>>> +            device_type = "pci";
>>>> +            ranges;
>>>> +            bus-range = <0x2 0xff>;
>>>> +
>>>> +            vddc-supply = <&vdd_ntn_0p9>;
>>>> +            vdd18-supply = <&vdd_ntn_1p8>;
>>>> +            vdd09-supply = <&vdd_ntn_0p9>;
>>>> +            vddio1-supply = <&vdd_ntn_1p8>;
>>>> +            vddio2-supply = <&vdd_ntn_1p8>;
>>>> +            vddio18-supply = <&vdd_ntn_1p8>;
>>>> +
>>>> +            i2c-parent = <&i2c0 0x77>;
>>>> +
>>>> +            reset-gpios = <&pm8350c_gpios 1 GPIO_ACTIVE_LOW>;
>>>> +
>>>
>>> I think I've responded here, but I'm not sure where the message went:
>>> please add pinctrl entry for this pin.
>>>
>> Do we need to also add pinctrl property for this node and refer the
>> pinctrl entry for this pin?
> 
> I think that is what I've asked for, was that not?
Currently there is no pincntrl property defined for this.

- Krishna Chaitanya.
> 

Re: [PATCH v4 02/10] arm64: dts: qcom: qcs6490-rb3gen2: Add TC956x PCIe switch node

[Dmitry Baryshkov]

On Wed, Mar 19, 2025 at 09:14:22AM +0530, Krishna Chaitanya Chundru wrote:
> 
> 
> On 3/18/2025 10:30 PM, Dmitry Baryshkov wrote:
> > On Tue, 18 Mar 2025 at 18:11, Krishna Chaitanya Chundru
> > <krishna.chundru@oss.qualcomm.com> wrote:
> > > 
> > > 
> > > 
> > > On 3/17/2025 4:57 PM, Dmitry Baryshkov wrote:
> > > > On Tue, Feb 25, 2025 at 03:03:59PM +0530, Krishna Chaitanya Chundru wrote:
> > > > > Add a node for the TC956x PCIe switch, which has three downstream ports.
> > > > > Two embedded Ethernet devices are present on one of the downstream ports.
> > > > > 
> > > > > Power to the TC956x is supplied through two LDO regulators, controlled by
> > > > > two GPIOs, which are added as fixed regulators. Configure the TC956x
> > > > > through I2C.
> > > > > 
> > > > > Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> > > > > Reviewed-by: Bjorn Andersson <andersson@kernel.org>
> > > > > Acked-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > > > > ---
> > > > >    arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 116 +++++++++++++++++++++++++++
> > > > >    arch/arm64/boot/dts/qcom/sc7280.dtsi         |   2 +-
> > > > >    2 files changed, 117 insertions(+), 1 deletion(-)
> > > > > 
> > > > > @@ -735,6 +760,75 @@ &pcie1_phy {
> > > > >       status = "okay";
> > > > >    };
> > > > > 
> > > > > +&pcie1_port {
> > > > > +    pcie@0,0 {
> > > > > +            compatible = "pci1179,0623", "pciclass,0604";
> > > > > +            reg = <0x10000 0x0 0x0 0x0 0x0>;
> > > > > +            #address-cells = <3>;
> > > > > +            #size-cells = <2>;
> > > > > +
> > > > > +            device_type = "pci";
> > > > > +            ranges;
> > > > > +            bus-range = <0x2 0xff>;
> > > > > +
> > > > > +            vddc-supply = <&vdd_ntn_0p9>;
> > > > > +            vdd18-supply = <&vdd_ntn_1p8>;
> > > > > +            vdd09-supply = <&vdd_ntn_0p9>;
> > > > > +            vddio1-supply = <&vdd_ntn_1p8>;
> > > > > +            vddio2-supply = <&vdd_ntn_1p8>;
> > > > > +            vddio18-supply = <&vdd_ntn_1p8>;
> > > > > +
> > > > > +            i2c-parent = <&i2c0 0x77>;
> > > > > +
> > > > > +            reset-gpios = <&pm8350c_gpios 1 GPIO_ACTIVE_LOW>;
> > > > > +
> > > > 
> > > > I think I've responded here, but I'm not sure where the message went:
> > > > please add pinctrl entry for this pin.
> > > > 
> > > Do we need to also add pinctrl property for this node and refer the
> > > pinctrl entry for this pin?
> > 
> > I think that is what I've asked for, was that not?
> Currently there is no pincntrl property defined for this.

Does it need to be defined separately / specially?

-- 
With best wishes
Dmitry

Re: [PATCH v4 02/10] arm64: dts: qcom: qcs6490-rb3gen2: Add TC956x PCIe switch node

[Krishna Chaitanya Chundru]

On 3/19/2025 3:43 PM, Dmitry Baryshkov wrote:
> On Wed, Mar 19, 2025 at 09:14:22AM +0530, Krishna Chaitanya Chundru wrote:
>>
>>
>> On 3/18/2025 10:30 PM, Dmitry Baryshkov wrote:
>>> On Tue, 18 Mar 2025 at 18:11, Krishna Chaitanya Chundru
>>> <krishna.chundru@oss.qualcomm.com> wrote:
>>>>
>>>>
>>>>
>>>> On 3/17/2025 4:57 PM, Dmitry Baryshkov wrote:
>>>>> On Tue, Feb 25, 2025 at 03:03:59PM +0530, Krishna Chaitanya Chundru wrote:
>>>>>> Add a node for the TC956x PCIe switch, which has three downstream ports.
>>>>>> Two embedded Ethernet devices are present on one of the downstream ports.
>>>>>>
>>>>>> Power to the TC956x is supplied through two LDO regulators, controlled by
>>>>>> two GPIOs, which are added as fixed regulators. Configure the TC956x
>>>>>> through I2C.
>>>>>>
>>>>>> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
>>>>>> Reviewed-by: Bjorn Andersson <andersson@kernel.org>
>>>>>> Acked-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
>>>>>> ---
>>>>>>     arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 116 +++++++++++++++++++++++++++
>>>>>>     arch/arm64/boot/dts/qcom/sc7280.dtsi         |   2 +-
>>>>>>     2 files changed, 117 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> @@ -735,6 +760,75 @@ &pcie1_phy {
>>>>>>        status = "okay";
>>>>>>     };
>>>>>>
>>>>>> +&pcie1_port {
>>>>>> +    pcie@0,0 {
>>>>>> +            compatible = "pci1179,0623", "pciclass,0604";
>>>>>> +            reg = <0x10000 0x0 0x0 0x0 0x0>;
>>>>>> +            #address-cells = <3>;
>>>>>> +            #size-cells = <2>;
>>>>>> +
>>>>>> +            device_type = "pci";
>>>>>> +            ranges;
>>>>>> +            bus-range = <0x2 0xff>;
>>>>>> +
>>>>>> +            vddc-supply = <&vdd_ntn_0p9>;
>>>>>> +            vdd18-supply = <&vdd_ntn_1p8>;
>>>>>> +            vdd09-supply = <&vdd_ntn_0p9>;
>>>>>> +            vddio1-supply = <&vdd_ntn_1p8>;
>>>>>> +            vddio2-supply = <&vdd_ntn_1p8>;
>>>>>> +            vddio18-supply = <&vdd_ntn_1p8>;
>>>>>> +
>>>>>> +            i2c-parent = <&i2c0 0x77>;
>>>>>> +
>>>>>> +            reset-gpios = <&pm8350c_gpios 1 GPIO_ACTIVE_LOW>;
>>>>>> +
>>>>>
>>>>> I think I've responded here, but I'm not sure where the message went:
>>>>> please add pinctrl entry for this pin.
>>>>>
>>>> Do we need to also add pinctrl property for this node and refer the
>>>> pinctrl entry for this pin?
>>>
>>> I think that is what I've asked for, was that not?
>> Currently there is no pincntrl property defined for this.
> 
> Does it need to be defined separately / specially?
> 
yes we need to define this property now.

- Krishna Chaitanya.

Re: [PATCH v4 02/10] arm64: dts: qcom: qcs6490-rb3gen2: Add TC956x PCIe switch node

[Dmitry Baryshkov]

On Wed, Mar 19, 2025 at 03:46:00PM +0530, Krishna Chaitanya Chundru wrote:
> 
> 
> On 3/19/2025 3:43 PM, Dmitry Baryshkov wrote:
> > On Wed, Mar 19, 2025 at 09:14:22AM +0530, Krishna Chaitanya Chundru wrote:
> > > 
> > > 
> > > On 3/18/2025 10:30 PM, Dmitry Baryshkov wrote:
> > > > On Tue, 18 Mar 2025 at 18:11, Krishna Chaitanya Chundru
> > > > <krishna.chundru@oss.qualcomm.com> wrote:
> > > > > 
> > > > > 
> > > > > 
> > > > > On 3/17/2025 4:57 PM, Dmitry Baryshkov wrote:
> > > > > > On Tue, Feb 25, 2025 at 03:03:59PM +0530, Krishna Chaitanya Chundru wrote:
> > > > > > > Add a node for the TC956x PCIe switch, which has three downstream ports.
> > > > > > > Two embedded Ethernet devices are present on one of the downstream ports.
> > > > > > > 
> > > > > > > Power to the TC956x is supplied through two LDO regulators, controlled by
> > > > > > > two GPIOs, which are added as fixed regulators. Configure the TC956x
> > > > > > > through I2C.
> > > > > > > 
> > > > > > > Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> > > > > > > Reviewed-by: Bjorn Andersson <andersson@kernel.org>
> > > > > > > Acked-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > > > > > > ---
> > > > > > >     arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 116 +++++++++++++++++++++++++++
> > > > > > >     arch/arm64/boot/dts/qcom/sc7280.dtsi         |   2 +-
> > > > > > >     2 files changed, 117 insertions(+), 1 deletion(-)
> > > > > > > 
> > > > > > > @@ -735,6 +760,75 @@ &pcie1_phy {
> > > > > > >        status = "okay";
> > > > > > >     };
> > > > > > > 
> > > > > > > +&pcie1_port {
> > > > > > > +    pcie@0,0 {
> > > > > > > +            compatible = "pci1179,0623", "pciclass,0604";
> > > > > > > +            reg = <0x10000 0x0 0x0 0x0 0x0>;
> > > > > > > +            #address-cells = <3>;
> > > > > > > +            #size-cells = <2>;
> > > > > > > +
> > > > > > > +            device_type = "pci";
> > > > > > > +            ranges;
> > > > > > > +            bus-range = <0x2 0xff>;
> > > > > > > +
> > > > > > > +            vddc-supply = <&vdd_ntn_0p9>;
> > > > > > > +            vdd18-supply = <&vdd_ntn_1p8>;
> > > > > > > +            vdd09-supply = <&vdd_ntn_0p9>;
> > > > > > > +            vddio1-supply = <&vdd_ntn_1p8>;
> > > > > > > +            vddio2-supply = <&vdd_ntn_1p8>;
> > > > > > > +            vddio18-supply = <&vdd_ntn_1p8>;
> > > > > > > +
> > > > > > > +            i2c-parent = <&i2c0 0x77>;
> > > > > > > +
> > > > > > > +            reset-gpios = <&pm8350c_gpios 1 GPIO_ACTIVE_LOW>;
> > > > > > > +
> > > > > > 
> > > > > > I think I've responded here, but I'm not sure where the message went:
> > > > > > please add pinctrl entry for this pin.
> > > > > > 
> > > > > Do we need to also add pinctrl property for this node and refer the
> > > > > pinctrl entry for this pin?
> > > > 
> > > > I think that is what I've asked for, was that not?
> > > Currently there is no pincntrl property defined for this.
> > 
> > Does it need to be defined separately / specially?
> > 
> yes we need to define this property now.

Could you please point out existing schema files defining those
properties?

-- 
With best wishes
Dmitry

Re: [PATCH v4 02/10] arm64: dts: qcom: qcs6490-rb3gen2: Add TC956x PCIe switch node

[Krishna Chaitanya Chundru]

On 3/19/2025 3:51 PM, Dmitry Baryshkov wrote:
> On Wed, Mar 19, 2025 at 03:46:00PM +0530, Krishna Chaitanya Chundru wrote:
>>
>>
>> On 3/19/2025 3:43 PM, Dmitry Baryshkov wrote:
>>> On Wed, Mar 19, 2025 at 09:14:22AM +0530, Krishna Chaitanya Chundru wrote:
>>>>
>>>>
>>>> On 3/18/2025 10:30 PM, Dmitry Baryshkov wrote:
>>>>> On Tue, 18 Mar 2025 at 18:11, Krishna Chaitanya Chundru
>>>>> <krishna.chundru@oss.qualcomm.com> wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 3/17/2025 4:57 PM, Dmitry Baryshkov wrote:
>>>>>>> On Tue, Feb 25, 2025 at 03:03:59PM +0530, Krishna Chaitanya Chundru wrote:
>>>>>>>> Add a node for the TC956x PCIe switch, which has three downstream ports.
>>>>>>>> Two embedded Ethernet devices are present on one of the downstream ports.
>>>>>>>>
>>>>>>>> Power to the TC956x is supplied through two LDO regulators, controlled by
>>>>>>>> two GPIOs, which are added as fixed regulators. Configure the TC956x
>>>>>>>> through I2C.
>>>>>>>>
>>>>>>>> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
>>>>>>>> Reviewed-by: Bjorn Andersson <andersson@kernel.org>
>>>>>>>> Acked-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
>>>>>>>> ---
>>>>>>>>      arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 116 +++++++++++++++++++++++++++
>>>>>>>>      arch/arm64/boot/dts/qcom/sc7280.dtsi         |   2 +-
>>>>>>>>      2 files changed, 117 insertions(+), 1 deletion(-)
>>>>>>>>
>>>>>>>> @@ -735,6 +760,75 @@ &pcie1_phy {
>>>>>>>>         status = "okay";
>>>>>>>>      };
>>>>>>>>
>>>>>>>> +&pcie1_port {
>>>>>>>> +    pcie@0,0 {
>>>>>>>> +            compatible = "pci1179,0623", "pciclass,0604";
>>>>>>>> +            reg = <0x10000 0x0 0x0 0x0 0x0>;
>>>>>>>> +            #address-cells = <3>;
>>>>>>>> +            #size-cells = <2>;
>>>>>>>> +
>>>>>>>> +            device_type = "pci";
>>>>>>>> +            ranges;
>>>>>>>> +            bus-range = <0x2 0xff>;
>>>>>>>> +
>>>>>>>> +            vddc-supply = <&vdd_ntn_0p9>;
>>>>>>>> +            vdd18-supply = <&vdd_ntn_1p8>;
>>>>>>>> +            vdd09-supply = <&vdd_ntn_0p9>;
>>>>>>>> +            vddio1-supply = <&vdd_ntn_1p8>;
>>>>>>>> +            vddio2-supply = <&vdd_ntn_1p8>;
>>>>>>>> +            vddio18-supply = <&vdd_ntn_1p8>;
>>>>>>>> +
>>>>>>>> +            i2c-parent = <&i2c0 0x77>;
>>>>>>>> +
>>>>>>>> +            reset-gpios = <&pm8350c_gpios 1 GPIO_ACTIVE_LOW>;
>>>>>>>> +
>>>>>>>
>>>>>>> I think I've responded here, but I'm not sure where the message went:
>>>>>>> please add pinctrl entry for this pin.
>>>>>>>
>>>>>> Do we need to also add pinctrl property for this node and refer the
>>>>>> pinctrl entry for this pin?
>>>>>
>>>>> I think that is what I've asked for, was that not?
>>>> Currently there is no pincntrl property defined for this.
>>>
>>> Does it need to be defined separately / specially?
>>>
>> yes we need to define this property now.
> 
> Could you please point out existing schema files defining those
> properties?
sorry I was not able to get which schema file you are requesting for,
if it is tc956x it is in this series only.

What I understood from these conversation is we need to define pinctrl
property and refer the reset gpio pin in next series. If it was wrong
please correct me.

- Krishna Chaitanya.
> 

Re: [PATCH v4 02/10] arm64: dts: qcom: qcs6490-rb3gen2: Add TC956x PCIe switch node

[Dmitry Baryshkov]

On Wed, Mar 19, 2025 at 04:16:33PM +0530, Krishna Chaitanya Chundru wrote:
> 
> 
> On 3/19/2025 3:51 PM, Dmitry Baryshkov wrote:
> > On Wed, Mar 19, 2025 at 03:46:00PM +0530, Krishna Chaitanya Chundru wrote:
> > > 
> > > 
> > > On 3/19/2025 3:43 PM, Dmitry Baryshkov wrote:
> > > > On Wed, Mar 19, 2025 at 09:14:22AM +0530, Krishna Chaitanya Chundru wrote:
> > > > > 
> > > > > 
> > > > > On 3/18/2025 10:30 PM, Dmitry Baryshkov wrote:
> > > > > > On Tue, 18 Mar 2025 at 18:11, Krishna Chaitanya Chundru
> > > > > > <krishna.chundru@oss.qualcomm.com> wrote:
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > On 3/17/2025 4:57 PM, Dmitry Baryshkov wrote:
> > > > > > > > On Tue, Feb 25, 2025 at 03:03:59PM +0530, Krishna Chaitanya Chundru wrote:
> > > > > > > > > Add a node for the TC956x PCIe switch, which has three downstream ports.
> > > > > > > > > Two embedded Ethernet devices are present on one of the downstream ports.
> > > > > > > > > 
> > > > > > > > > Power to the TC956x is supplied through two LDO regulators, controlled by
> > > > > > > > > two GPIOs, which are added as fixed regulators. Configure the TC956x
> > > > > > > > > through I2C.
> > > > > > > > > 
> > > > > > > > > Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> > > > > > > > > Reviewed-by: Bjorn Andersson <andersson@kernel.org>
> > > > > > > > > Acked-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > > > > > > > > ---
> > > > > > > > >      arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 116 +++++++++++++++++++++++++++
> > > > > > > > >      arch/arm64/boot/dts/qcom/sc7280.dtsi         |   2 +-
> > > > > > > > >      2 files changed, 117 insertions(+), 1 deletion(-)
> > > > > > > > > 
> > > > > > > > > @@ -735,6 +760,75 @@ &pcie1_phy {
> > > > > > > > >         status = "okay";
> > > > > > > > >      };
> > > > > > > > > 
> > > > > > > > > +&pcie1_port {
> > > > > > > > > +    pcie@0,0 {
> > > > > > > > > +            compatible = "pci1179,0623", "pciclass,0604";
> > > > > > > > > +            reg = <0x10000 0x0 0x0 0x0 0x0>;
> > > > > > > > > +            #address-cells = <3>;
> > > > > > > > > +            #size-cells = <2>;
> > > > > > > > > +
> > > > > > > > > +            device_type = "pci";
> > > > > > > > > +            ranges;
> > > > > > > > > +            bus-range = <0x2 0xff>;
> > > > > > > > > +
> > > > > > > > > +            vddc-supply = <&vdd_ntn_0p9>;
> > > > > > > > > +            vdd18-supply = <&vdd_ntn_1p8>;
> > > > > > > > > +            vdd09-supply = <&vdd_ntn_0p9>;
> > > > > > > > > +            vddio1-supply = <&vdd_ntn_1p8>;
> > > > > > > > > +            vddio2-supply = <&vdd_ntn_1p8>;
> > > > > > > > > +            vddio18-supply = <&vdd_ntn_1p8>;
> > > > > > > > > +
> > > > > > > > > +            i2c-parent = <&i2c0 0x77>;
> > > > > > > > > +
> > > > > > > > > +            reset-gpios = <&pm8350c_gpios 1 GPIO_ACTIVE_LOW>;
> > > > > > > > > +
> > > > > > > > 
> > > > > > > > I think I've responded here, but I'm not sure where the message went:
> > > > > > > > please add pinctrl entry for this pin.
> > > > > > > > 
> > > > > > > Do we need to also add pinctrl property for this node and refer the
> > > > > > > pinctrl entry for this pin?
> > > > > > 
> > > > > > I think that is what I've asked for, was that not?
> > > > > Currently there is no pincntrl property defined for this.
> > > > 
> > > > Does it need to be defined separately / specially?
> > > > 
> > > yes we need to define this property now.
> > 
> > Could you please point out existing schema files defining those
> > properties?
> sorry I was not able to get which schema file you are requesting for,
> if it is tc956x it is in this series only.
> 
> What I understood from these conversation is we need to define pinctrl
> property and refer the reset gpio pin in next series. If it was wrong
> please correct me.

You claimed that pinctrl properties (there are several of those) are to
be defined in the schema for TC956x. I asked you to point out other
schema files which define those properties for the devices that use
GPIO pins.

-- 
With best wishes
Dmitry

Re: [PATCH v4 02/10] arm64: dts: qcom: qcs6490-rb3gen2: Add TC956x PCIe switch node

[Konrad Dybcio]

On 3/19/25 12:06 PM, Dmitry Baryshkov wrote:
> On Wed, Mar 19, 2025 at 04:16:33PM +0530, Krishna Chaitanya Chundru wrote:
>>
>>
>> On 3/19/2025 3:51 PM, Dmitry Baryshkov wrote:
>>> On Wed, Mar 19, 2025 at 03:46:00PM +0530, Krishna Chaitanya Chundru wrote:
>>>>
>>>>
>>>> On 3/19/2025 3:43 PM, Dmitry Baryshkov wrote:
>>>>> On Wed, Mar 19, 2025 at 09:14:22AM +0530, Krishna Chaitanya Chundru wrote:
>>>>>>
>>>>>>
>>>>>> On 3/18/2025 10:30 PM, Dmitry Baryshkov wrote:
>>>>>>> On Tue, 18 Mar 2025 at 18:11, Krishna Chaitanya Chundru
>>>>>>> <krishna.chundru@oss.qualcomm.com> wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On 3/17/2025 4:57 PM, Dmitry Baryshkov wrote:
>>>>>>>>> On Tue, Feb 25, 2025 at 03:03:59PM +0530, Krishna Chaitanya Chundru wrote:
>>>>>>>>>> Add a node for the TC956x PCIe switch, which has three downstream ports.
>>>>>>>>>> Two embedded Ethernet devices are present on one of the downstream ports.
>>>>>>>>>>
>>>>>>>>>> Power to the TC956x is supplied through two LDO regulators, controlled by
>>>>>>>>>> two GPIOs, which are added as fixed regulators. Configure the TC956x
>>>>>>>>>> through I2C.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
>>>>>>>>>> Reviewed-by: Bjorn Andersson <andersson@kernel.org>
>>>>>>>>>> Acked-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
>>>>>>>>>> ---
>>>>>>>>>>      arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 116 +++++++++++++++++++++++++++
>>>>>>>>>>      arch/arm64/boot/dts/qcom/sc7280.dtsi         |   2 +-
>>>>>>>>>>      2 files changed, 117 insertions(+), 1 deletion(-)
>>>>>>>>>>
>>>>>>>>>> @@ -735,6 +760,75 @@ &pcie1_phy {
>>>>>>>>>>         status = "okay";
>>>>>>>>>>      };
>>>>>>>>>>
>>>>>>>>>> +&pcie1_port {
>>>>>>>>>> +    pcie@0,0 {
>>>>>>>>>> +            compatible = "pci1179,0623", "pciclass,0604";
>>>>>>>>>> +            reg = <0x10000 0x0 0x0 0x0 0x0>;
>>>>>>>>>> +            #address-cells = <3>;
>>>>>>>>>> +            #size-cells = <2>;
>>>>>>>>>> +
>>>>>>>>>> +            device_type = "pci";
>>>>>>>>>> +            ranges;
>>>>>>>>>> +            bus-range = <0x2 0xff>;
>>>>>>>>>> +
>>>>>>>>>> +            vddc-supply = <&vdd_ntn_0p9>;
>>>>>>>>>> +            vdd18-supply = <&vdd_ntn_1p8>;
>>>>>>>>>> +            vdd09-supply = <&vdd_ntn_0p9>;
>>>>>>>>>> +            vddio1-supply = <&vdd_ntn_1p8>;
>>>>>>>>>> +            vddio2-supply = <&vdd_ntn_1p8>;
>>>>>>>>>> +            vddio18-supply = <&vdd_ntn_1p8>;
>>>>>>>>>> +
>>>>>>>>>> +            i2c-parent = <&i2c0 0x77>;
>>>>>>>>>> +
>>>>>>>>>> +            reset-gpios = <&pm8350c_gpios 1 GPIO_ACTIVE_LOW>;
>>>>>>>>>> +
>>>>>>>>>
>>>>>>>>> I think I've responded here, but I'm not sure where the message went:
>>>>>>>>> please add pinctrl entry for this pin.
>>>>>>>>>
>>>>>>>> Do we need to also add pinctrl property for this node and refer the
>>>>>>>> pinctrl entry for this pin?
>>>>>>>
>>>>>>> I think that is what I've asked for, was that not?
>>>>>> Currently there is no pincntrl property defined for this.
>>>>>
>>>>> Does it need to be defined separately / specially?
>>>>>
>>>> yes we need to define this property now.
>>>
>>> Could you please point out existing schema files defining those
>>> properties?
>> sorry I was not able to get which schema file you are requesting for,
>> if it is tc956x it is in this series only.
>>
>> What I understood from these conversation is we need to define pinctrl
>> property and refer the reset gpio pin in next series. If it was wrong
>> please correct me.
> 
> You claimed that pinctrl properties (there are several of those) are to
> be defined in the schema for TC956x. I asked you to point out other
> schema files which define those properties for the devices that use
> GPIO pins.

pinctrl-x is part of common schema (see gh/devicetree-org/dt-schema/)

Konrad

Re: [PATCH v4 02/10] arm64: dts: qcom: qcs6490-rb3gen2: Add TC956x PCIe switch node

[Dmitry Baryshkov]

On Wed, Mar 19, 2025 at 03:12:50PM +0100, Konrad Dybcio wrote:
> On 3/19/25 12:06 PM, Dmitry Baryshkov wrote:
> > On Wed, Mar 19, 2025 at 04:16:33PM +0530, Krishna Chaitanya Chundru wrote:
> >>
> >>
> >> On 3/19/2025 3:51 PM, Dmitry Baryshkov wrote:
> >>> On Wed, Mar 19, 2025 at 03:46:00PM +0530, Krishna Chaitanya Chundru wrote:
> >>>>
> >>>>
> >>>> On 3/19/2025 3:43 PM, Dmitry Baryshkov wrote:
> >>>>> On Wed, Mar 19, 2025 at 09:14:22AM +0530, Krishna Chaitanya Chundru wrote:
> >>>>>>
> >>>>>>
> >>>>>> On 3/18/2025 10:30 PM, Dmitry Baryshkov wrote:
> >>>>>>> On Tue, 18 Mar 2025 at 18:11, Krishna Chaitanya Chundru
> >>>>>>> <krishna.chundru@oss.qualcomm.com> wrote:
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> On 3/17/2025 4:57 PM, Dmitry Baryshkov wrote:
> >>>>>>>>> On Tue, Feb 25, 2025 at 03:03:59PM +0530, Krishna Chaitanya Chundru wrote:
> >>>>>>>>>> Add a node for the TC956x PCIe switch, which has three downstream ports.
> >>>>>>>>>> Two embedded Ethernet devices are present on one of the downstream ports.
> >>>>>>>>>>
> >>>>>>>>>> Power to the TC956x is supplied through two LDO regulators, controlled by
> >>>>>>>>>> two GPIOs, which are added as fixed regulators. Configure the TC956x
> >>>>>>>>>> through I2C.
> >>>>>>>>>>
> >>>>>>>>>> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> >>>>>>>>>> Reviewed-by: Bjorn Andersson <andersson@kernel.org>
> >>>>>>>>>> Acked-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> >>>>>>>>>> ---
> >>>>>>>>>>      arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 116 +++++++++++++++++++++++++++
> >>>>>>>>>>      arch/arm64/boot/dts/qcom/sc7280.dtsi         |   2 +-
> >>>>>>>>>>      2 files changed, 117 insertions(+), 1 deletion(-)
> >>>>>>>>>>
> >>>>>>>>>> @@ -735,6 +760,75 @@ &pcie1_phy {
> >>>>>>>>>>         status = "okay";
> >>>>>>>>>>      };
> >>>>>>>>>>
> >>>>>>>>>> +&pcie1_port {
> >>>>>>>>>> +    pcie@0,0 {
> >>>>>>>>>> +            compatible = "pci1179,0623", "pciclass,0604";
> >>>>>>>>>> +            reg = <0x10000 0x0 0x0 0x0 0x0>;
> >>>>>>>>>> +            #address-cells = <3>;
> >>>>>>>>>> +            #size-cells = <2>;
> >>>>>>>>>> +
> >>>>>>>>>> +            device_type = "pci";
> >>>>>>>>>> +            ranges;
> >>>>>>>>>> +            bus-range = <0x2 0xff>;
> >>>>>>>>>> +
> >>>>>>>>>> +            vddc-supply = <&vdd_ntn_0p9>;
> >>>>>>>>>> +            vdd18-supply = <&vdd_ntn_1p8>;
> >>>>>>>>>> +            vdd09-supply = <&vdd_ntn_0p9>;
> >>>>>>>>>> +            vddio1-supply = <&vdd_ntn_1p8>;
> >>>>>>>>>> +            vddio2-supply = <&vdd_ntn_1p8>;
> >>>>>>>>>> +            vddio18-supply = <&vdd_ntn_1p8>;
> >>>>>>>>>> +
> >>>>>>>>>> +            i2c-parent = <&i2c0 0x77>;
> >>>>>>>>>> +
> >>>>>>>>>> +            reset-gpios = <&pm8350c_gpios 1 GPIO_ACTIVE_LOW>;
> >>>>>>>>>> +
> >>>>>>>>>
> >>>>>>>>> I think I've responded here, but I'm not sure where the message went:
> >>>>>>>>> please add pinctrl entry for this pin.
> >>>>>>>>>
> >>>>>>>> Do we need to also add pinctrl property for this node and refer the
> >>>>>>>> pinctrl entry for this pin?
> >>>>>>>
> >>>>>>> I think that is what I've asked for, was that not?
> >>>>>> Currently there is no pincntrl property defined for this.
> >>>>>
> >>>>> Does it need to be defined separately / specially?
> >>>>>
> >>>> yes we need to define this property now.
> >>>
> >>> Could you please point out existing schema files defining those
> >>> properties?
> >> sorry I was not able to get which schema file you are requesting for,
> >> if it is tc956x it is in this series only.
> >>
> >> What I understood from these conversation is we need to define pinctrl
> >> property and refer the reset gpio pin in next series. If it was wrong
> >> please correct me.
> > 
> > You claimed that pinctrl properties (there are several of those) are to
> > be defined in the schema for TC956x. I asked you to point out other
> > schema files which define those properties for the devices that use
> > GPIO pins.
> 
> pinctrl-x is part of common schema (see gh/devicetree-org/dt-schema/)

Right :-)

So there is no need to define anything, they just need to be added to
the DTS file.

-- 
With best wishes
Dmitry

[PATCH v4 03/10] PCI: Add new start_link() & stop_link function ops

[Krishna Chaitanya Chundru]

First controller driver probes, enables link training and scans the
bus. When the PCI bridge is found, its child DT nodes will be scanned
and pwrctrl devices will be created if needed. By the time pwrctrl
driver probe gets called link training is already enabled by controller
driver.

Certain devices like TC956x which uses PCI pwrctl framework needs to
configure the device before PCI link is up.

As the controller driver already enables link training as part of
its probe, the moment device is powered on, controller and device
participates in the link training and link can come up immediately
and maynot have time to configure the device.

So we need to stop the link training by using stop_link() and enable
them back after device is configured by using start_link().

Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
 include/linux/pci.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/linux/pci.h b/include/linux/pci.h
index 47b31ad724fa..bbec32be668b 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -804,6 +804,8 @@ struct pci_ops {
 	void __iomem *(*map_bus)(struct pci_bus *bus, unsigned int devfn, int where);
 	int (*read)(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 *val);
 	int (*write)(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 val);
+	int (*start_link)(struct pci_bus *bus);
+	void (*stop_link)(struct pci_bus *bus);
 };
 
 /*

-- 
2.34.1

[PATCH v4 04/10] PCI: dwc: Add host_start_link() & host_start_link() hooks for dwc glue drivers

[Krishna Chaitanya Chundru]

Add host_start_link() and host_stop_link() functions to dwc glue drivers to
register with start_link() and stop_link() of pci ops, allowing for better
control over the link initialization and shutdown process.

Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
 drivers/pci/controller/dwc/pcie-designware.h | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 501d9ddfea16..47f34dd12c39 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -435,6 +435,8 @@ struct dw_pcie_ops {
 	enum dw_pcie_ltssm (*get_ltssm)(struct dw_pcie *pcie);
 	int	(*start_link)(struct dw_pcie *pcie);
 	void	(*stop_link)(struct dw_pcie *pcie);
+	int	(*host_start_link)(struct dw_pcie *pcie);
+	void	(*host_stop_link)(struct dw_pcie *pcie);
 };
 
 struct dw_pcie {
@@ -664,6 +666,20 @@ static inline void dw_pcie_stop_link(struct dw_pcie *pci)
 		pci->ops->stop_link(pci);
 }
 
+static inline int dw_pcie_host_start_link(struct dw_pcie *pci)
+{
+	if (pci->ops && pci->ops->host_start_link)
+		return pci->ops->host_start_link(pci);
+
+	return 0;
+}
+
+static inline void dw_pcie_host_stop_link(struct dw_pcie *pci)
+{
+	if (pci->ops && pci->ops->host_stop_link)
+		pci->ops->host_stop_link(pci);
+}
+
 static inline enum dw_pcie_ltssm dw_pcie_get_ltssm(struct dw_pcie *pci)
 {
 	u32 val;

-- 
2.34.1

[PATCH v4 05/10] PCI: dwc: Implement .start_link(), .stop_link() hooks

[Krishna Chaitanya Chundru]

Implement stop_link() and  start_link() function op for dwc drivers.

Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
 drivers/pci/controller/dwc/pcie-designware-host.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index ffaded8f2df7..2d3ec61e8dfa 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -697,10 +697,28 @@ void __iomem *dw_pcie_own_conf_map_bus(struct pci_bus *bus, unsigned int devfn,
 }
 EXPORT_SYMBOL_GPL(dw_pcie_own_conf_map_bus);
 
+static int dw_pcie_op_start_link(struct pci_bus *bus)
+{
+	struct dw_pcie_rp *pp = bus->sysdata;
+	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+
+	return dw_pcie_host_start_link(pci);
+}
+
+static void dw_pcie_op_stop_link(struct pci_bus *bus)
+{
+	struct dw_pcie_rp *pp = bus->sysdata;
+	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+
+	dw_pcie_host_stop_link(pci);
+}
+
 static struct pci_ops dw_pcie_ops = {
 	.map_bus = dw_pcie_own_conf_map_bus,
 	.read = pci_generic_config_read,
 	.write = pci_generic_config_write,
+	.start_link = dw_pcie_op_start_link,
+	.stop_link = dw_pcie_op_stop_link,
 };
 
 static int dw_pcie_iatu_setup(struct dw_pcie_rp *pp)

-- 
2.34.1

[PATCH v4 06/10] PCI: qcom: Add support for host_stop_link() & host_start_link()

[Krishna Chaitanya Chundru]

Add support for host_stop_link() and host_start_link() for switches like
TC956x, which require configuration before the PCIe link is established.

Assert PERST# and disable LTSSM bit to prevent the PCIe controller from
participating in link training during host_stop_link(). De-assert PERST#
and enable LTSSM bit during host_start_link().

Introduce ltssm_disable function op to stop link training.
For the switches like TC956x, which needs to configure it before
the PCIe link is established.

Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 35 ++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index e4d3366ead1f..8c9c89417440 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -247,6 +247,7 @@ struct qcom_pcie_ops {
 	void (*host_post_init)(struct qcom_pcie *pcie);
 	void (*deinit)(struct qcom_pcie *pcie);
 	void (*ltssm_enable)(struct qcom_pcie *pcie);
+	void (*ltssm_disable)(struct qcom_pcie *pcie);
 	int (*config_sid)(struct qcom_pcie *pcie);
 };
 
@@ -618,6 +619,37 @@ static int qcom_pcie_post_init_1_0_0(struct qcom_pcie *pcie)
 	return 0;
 }
 
+static int qcom_pcie_host_start_link(struct dw_pcie *pci)
+{
+	struct qcom_pcie *pcie = to_qcom_pcie(pci);
+
+	qcom_ep_reset_deassert(pcie);
+
+	if (pcie->cfg->ops->ltssm_enable)
+		pcie->cfg->ops->ltssm_enable(pcie);
+
+	return 0;
+}
+
+static void qcom_pcie_host_stop_link(struct dw_pcie *pci)
+{
+	struct qcom_pcie *pcie = to_qcom_pcie(pci);
+
+	qcom_ep_reset_assert(pcie);
+
+	if (pcie->cfg->ops->ltssm_disable)
+		pcie->cfg->ops->ltssm_disable(pcie);
+}
+
+static void qcom_pcie_2_3_2_ltssm_disable(struct qcom_pcie *pcie)
+{
+	u32 val;
+
+	val = readl(pcie->parf + PARF_LTSSM);
+	val &= ~LTSSM_EN;
+	writel(val, pcie->parf + PARF_LTSSM);
+}
+
 static void qcom_pcie_2_3_2_ltssm_enable(struct qcom_pcie *pcie)
 {
 	u32 val;
@@ -1362,6 +1394,7 @@ static const struct qcom_pcie_ops ops_1_9_0 = {
 	.host_post_init = qcom_pcie_host_post_init_2_7_0,
 	.deinit = qcom_pcie_deinit_2_7_0,
 	.ltssm_enable = qcom_pcie_2_3_2_ltssm_enable,
+	.ltssm_disable = qcom_pcie_2_3_2_ltssm_disable,
 	.config_sid = qcom_pcie_config_sid_1_9_0,
 };
 
@@ -1429,6 +1462,8 @@ static const struct qcom_pcie_cfg cfg_sc8280xp = {
 static const struct dw_pcie_ops dw_pcie_ops = {
 	.link_up = qcom_pcie_link_up,
 	.start_link = qcom_pcie_start_link,
+	.host_start_link = qcom_pcie_host_start_link,
+	.host_stop_link = qcom_pcie_host_stop_link,
 };
 
 static int qcom_pcie_icc_init(struct qcom_pcie *pcie)

-- 
2.34.1

[PATCH v4 07/10] PCI: PCI: Add pcie_is_link_active() to determine if the PCIe link is active

[Krishna Chaitanya Chundru]

Introduce a common API to check if the PCIe link is active, replacing
duplicate code in multiple locations.

Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
 drivers/pci/hotplug/pciehp_hpc.c | 13 +------------
 drivers/pci/pci.c                | 26 +++++++++++++++++++++++---
 include/linux/pci.h              |  5 +++++
 3 files changed, 29 insertions(+), 15 deletions(-)

diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
index bb5a8d9f03ad..d0a2efebb519 100644
--- a/drivers/pci/hotplug/pciehp_hpc.c
+++ b/drivers/pci/hotplug/pciehp_hpc.c
@@ -234,18 +234,7 @@ static void pcie_write_cmd_nowait(struct controller *ctrl, u16 cmd, u16 mask)
  */
 int pciehp_check_link_active(struct controller *ctrl)
 {
-	struct pci_dev *pdev = ctrl_dev(ctrl);
-	u16 lnk_status;
-	int ret;
-
-	ret = pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &lnk_status);
-	if (ret == PCIBIOS_DEVICE_NOT_FOUND || PCI_POSSIBLE_ERROR(lnk_status))
-		return -ENODEV;
-
-	ret = !!(lnk_status & PCI_EXP_LNKSTA_DLLLA);
-	ctrl_dbg(ctrl, "%s: lnk_status = %x\n", __func__, lnk_status);
-
-	return ret;
+	return pcie_is_link_active(ctrl_dev(ctrl));
 }
 
 static bool pci_bus_check_dev(struct pci_bus *bus, int devfn)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 869d204a70a3..3d4fe6fefa13 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -4907,7 +4907,6 @@ int pci_bridge_wait_for_secondary_bus(struct pci_dev *dev, char *reset_type)
 		return 0;
 
 	if (pcie_get_speed_cap(dev) <= PCIE_SPEED_5_0GT) {
-		u16 status;
 
 		pci_dbg(dev, "waiting %d ms for downstream link\n", delay);
 		msleep(delay);
@@ -4923,8 +4922,7 @@ int pci_bridge_wait_for_secondary_bus(struct pci_dev *dev, char *reset_type)
 		if (!dev->link_active_reporting)
 			return -ENOTTY;
 
-		pcie_capability_read_word(dev, PCI_EXP_LNKSTA, &status);
-		if (!(status & PCI_EXP_LNKSTA_DLLLA))
+		if (pcie_is_link_active(dev))
 			return -ENOTTY;
 
 		return pci_dev_wait(child, reset_type,
@@ -6219,6 +6217,28 @@ void pcie_print_link_status(struct pci_dev *dev)
 }
 EXPORT_SYMBOL(pcie_print_link_status);
 
+/**
+ * pcie_is_link_active() - Checks if the link is active or not
+ * @pdev: PCI device to query
+ *
+ * Check whether the link is active or not.
+ *
+ * If the config read returns error then return -ENODEV.
+ */
+int pcie_is_link_active(struct pci_dev *pdev)
+{
+	u16 lnk_status;
+	int ret;
+
+	ret = pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &lnk_status);
+	if (ret == PCIBIOS_DEVICE_NOT_FOUND || PCI_POSSIBLE_ERROR(lnk_status))
+		return -ENODEV;
+
+	pci_dbg(pdev, "lnk_status = %x\n", lnk_status);
+	return !!(lnk_status & PCI_EXP_LNKSTA_DLLLA);
+}
+EXPORT_SYMBOL(pcie_is_link_active);
+
 /**
  * pci_select_bars - Make BAR mask from the type of resource
  * @dev: the PCI device for which BAR mask is made
diff --git a/include/linux/pci.h b/include/linux/pci.h
index bbec32be668b..84bb98e61e8a 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1946,6 +1946,7 @@ pci_release_mem_regions(struct pci_dev *pdev)
 			    pci_select_bars(pdev, IORESOURCE_MEM));
 }
 
+int pcie_is_link_active(struct pci_dev *dev);
 #else /* CONFIG_PCI is not enabled */
 
 static inline void pci_set_flags(int flags) { }
@@ -2094,6 +2095,10 @@ pci_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs,
 {
 	return -ENOSPC;
 }
+
+static inline int pcie_is_link_active(struct pci_dev *dev)
+{ return -ENODEV; }
+
 #endif /* CONFIG_PCI */
 
 /* Include architecture-dependent settings and functions */

-- 
2.34.1

Re: [PATCH v4 07/10] PCI: PCI: Add pcie_is_link_active() to determine if the PCIe link is active

[Lukas Wunner]

On Tue, Feb 25, 2025 at 03:04:04PM +0530, Krishna Chaitanya Chundru wrote:
> Introduce a common API to check if the PCIe link is active, replacing
> duplicate code in multiple locations.
[...]
> --- a/drivers/pci/hotplug/pciehp_hpc.c
> +++ b/drivers/pci/hotplug/pciehp_hpc.c
> @@ -234,18 +234,7 @@ static void pcie_write_cmd_nowait(struct controller *ctrl, u16 cmd, u16 mask)
>   */
>  int pciehp_check_link_active(struct controller *ctrl)
>  {
> -	struct pci_dev *pdev = ctrl_dev(ctrl);
> -	u16 lnk_status;
> -	int ret;
> -
> -	ret = pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &lnk_status);
> -	if (ret == PCIBIOS_DEVICE_NOT_FOUND || PCI_POSSIBLE_ERROR(lnk_status))
> -		return -ENODEV;
> -
> -	ret = !!(lnk_status & PCI_EXP_LNKSTA_DLLLA);
> -	ctrl_dbg(ctrl, "%s: lnk_status = %x\n", __func__, lnk_status);
> -
> -	return ret;
> +	return pcie_is_link_active(ctrl_dev(ctrl));
>  }

Please replace all call sites of pciehp_check_link_active() with a call
to the new function.


> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -4923,8 +4922,7 @@ int pci_bridge_wait_for_secondary_bus(struct pci_dev *dev, char *reset_type)
>  		if (!dev->link_active_reporting)
>  			return -ENOTTY;
>  
> -		pcie_capability_read_word(dev, PCI_EXP_LNKSTA, &status);
> -		if (!(status & PCI_EXP_LNKSTA_DLLLA))
> +		if (pcie_is_link_active(dev))
>  			return -ENOTTY;

Missing negation.


> +/**
> + * pcie_is_link_active() - Checks if the link is active or not
> + * @pdev: PCI device to query
> + *
> + * Check whether the link is active or not.
> + *
> + * If the config read returns error then return -ENODEV.
> + */
> +int pcie_is_link_active(struct pci_dev *pdev)

Why not return bool?

I don't quite like the function name because in English the correct word
order is subject - predicate - object, i.e. pcie_link_is_active() or
even shorter, pcie_link_active().


> @@ -2094,6 +2095,10 @@ pci_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs,
>  {
>  	return -ENOSPC;
>  }
> +
> +static inline int pcie_is_link_active(struct pci_dev *dev)
> +{ return -ENODEV; }
> +
>  #endif /* CONFIG_PCI */

Is the empty inline really necessary?  What breaks if you leave it out?

Thanks,

Lukas

Re: [PATCH v4 07/10] PCI: PCI: Add pcie_is_link_active() to determine if the PCIe link is active

[Krishna Chaitanya Chundru]

On 2/25/2025 3:24 PM, Lukas Wunner wrote:
> On Tue, Feb 25, 2025 at 03:04:04PM +0530, Krishna Chaitanya Chundru wrote:
>> Introduce a common API to check if the PCIe link is active, replacing
>> duplicate code in multiple locations.
> [...]
>> --- a/drivers/pci/hotplug/pciehp_hpc.c
>> +++ b/drivers/pci/hotplug/pciehp_hpc.c
>> @@ -234,18 +234,7 @@ static void pcie_write_cmd_nowait(struct controller *ctrl, u16 cmd, u16 mask)
>>    */
>>   int pciehp_check_link_active(struct controller *ctrl)
>>   {
>> -	struct pci_dev *pdev = ctrl_dev(ctrl);
>> -	u16 lnk_status;
>> -	int ret;
>> -
>> -	ret = pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &lnk_status);
>> -	if (ret == PCIBIOS_DEVICE_NOT_FOUND || PCI_POSSIBLE_ERROR(lnk_status))
>> -		return -ENODEV;
>> -
>> -	ret = !!(lnk_status & PCI_EXP_LNKSTA_DLLLA);
>> -	ctrl_dbg(ctrl, "%s: lnk_status = %x\n", __func__, lnk_status);
>> -
>> -	return ret;
>> +	return pcie_is_link_active(ctrl_dev(ctrl));
>>   }
> 
> Please replace all call sites of pciehp_check_link_active() with a call
> to the new function.
> 
> 
ack
>> --- a/drivers/pci/pci.c
>> +++ b/drivers/pci/pci.c
>> @@ -4923,8 +4922,7 @@ int pci_bridge_wait_for_secondary_bus(struct pci_dev *dev, char *reset_type)
>>   		if (!dev->link_active_reporting)
>>   			return -ENOTTY;
>>   
>> -		pcie_capability_read_word(dev, PCI_EXP_LNKSTA, &status);
>> -		if (!(status & PCI_EXP_LNKSTA_DLLLA))
>> +		if (pcie_is_link_active(dev))
>>   			return -ENOTTY;
> 
> Missing negation.
> 
> 
ack
>> +/**
>> + * pcie_is_link_active() - Checks if the link is active or not
>> + * @pdev: PCI device to query
>> + *
>> + * Check whether the link is active or not.
>> + *
>> + * If the config read returns error then return -ENODEV.
>> + */
>> +int pcie_is_link_active(struct pci_dev *pdev)
> 
> Why not return bool?
> 
pciehp_check_link_active is expecting int to make sure this new function
not disturbing the hotplug driver I added return type as int, I can 
change it to bool if it fine with hotplug drivers.
> I don't quite like the function name because in English the correct word
> order is subject - predicate - object, i.e. pcie_link_is_active() or
> even shorter, pcie_link_active().
> 
ack
> 
>> @@ -2094,6 +2095,10 @@ pci_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs,
>>   {
>>   	return -ENOSPC;
>>   }
>> +
>> +static inline int pcie_is_link_active(struct pci_dev *dev)
>> +{ return -ENODEV; }
>> +
>>   #endif /* CONFIG_PCI */
> 
> Is the empty inline really necessary?  What breaks if you leave it out?
> 
ack I will remove it.

- Krishna Chaitanya.
> Thanks,
> 
> Lukas

[PATCH v4 08/10] PCI: pwrctrl: Add power control driver for tc956x

[Krishna Chaitanya Chundru]

TC956x is a PCIe switch which has one upstream and three downstream
ports. To one of the downstream ports ethernet MAC is connected as endpoint
device. Other two downstream ports are supposed to connect to external
device. One Host can connect to TC956x by upstream port. TC956x switch
needs to be configured after powering on and before PCIe link was up.

The PCIe controller driver already enables link training at the host side
even before this driver probe happens, due to this when driver enables
power to the switch it participates in the link training and PCIe link
may come up before configuring the switch through i2c. Once the link is
up the configuration done through i2c will not have any affect.To prevent
the host from participating in link training, disable link training on the
host side to ensure the link does not come up before the switch is
configured via I2C.

Based up on dt property and type of the port, tc956x is configured
through i2c.

Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
Reviewed-by: Bjorn Andersson <andersson@kernel.org>
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 drivers/pci/pwrctrl/Kconfig              |   6 +
 drivers/pci/pwrctrl/Makefile             |   1 +
 drivers/pci/pwrctrl/pci-pwrctrl-tc956x.c | 625 +++++++++++++++++++++++++++++++
 3 files changed, 632 insertions(+)

diff --git a/drivers/pci/pwrctrl/Kconfig b/drivers/pci/pwrctrl/Kconfig
index 54589bb2403b..ae8a0a39f586 100644
--- a/drivers/pci/pwrctrl/Kconfig
+++ b/drivers/pci/pwrctrl/Kconfig
@@ -10,3 +10,9 @@ config PCI_PWRCTL_PWRSEQ
 	tristate
 	select POWER_SEQUENCING
 	select PCI_PWRCTL
+
+config PCI_PWRCTRL_TC956X
+	tristate "PCI Power Control driver for TC956x PCIe switch"
+	select PCI_PWRCTL
+	help
+	  Say Y here to enable the pwrctrl driver for TC956x PCIe switch.
diff --git a/drivers/pci/pwrctrl/Makefile b/drivers/pci/pwrctrl/Makefile
index 75c7ce531c7e..93f32871260b 100644
--- a/drivers/pci/pwrctrl/Makefile
+++ b/drivers/pci/pwrctrl/Makefile
@@ -4,3 +4,4 @@ obj-$(CONFIG_PCI_PWRCTL)		+= pci-pwrctrl-core.o
 pci-pwrctrl-core-y			:= core.o
 
 obj-$(CONFIG_PCI_PWRCTL_PWRSEQ)		+= pci-pwrctrl-pwrseq.o
+obj-$(CONFIG_PCI_PWRCTRL_TC956X)	+= pci-pwrctrl-tc956x.o
diff --git a/drivers/pci/pwrctrl/pci-pwrctrl-tc956x.c b/drivers/pci/pwrctrl/pci-pwrctrl-tc956x.c
new file mode 100644
index 000000000000..bf72bbcae536
--- /dev/null
+++ b/drivers/pci/pwrctrl/pci-pwrctrl-tc956x.c
@@ -0,0 +1,625 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
+ */
+
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/i2c.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/pci.h>
+#include <linux/pci-pwrctrl.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/consumer.h>
+#include <linux/string.h>
+#include <linux/types.h>
+#include <linux/unaligned.h>
+
+#include "../pci.h"
+
+#define TC956X_GPIO_CONFIG		0x801208
+#define TC956X_RESET_GPIO		0x801210
+
+#define TC956X_BUS_CONTROL		0x801014
+
+#define TC956X_PORT_L0S_DELAY		0x82496c
+#define TC956X_PORT_L1_DELAY		0x824970
+
+#define TC956X_EMBEDDED_ETH_DELAY	0x8200d8
+#define TC956X_ETH_L1_DELAY_MASK	GENMASK(27, 18)
+#define TC956X_ETH_L1_DELAY_VALUE(x)	FIELD_PREP(TC956X_ETH_L1_DELAY_MASK, x)
+#define TC956X_ETH_L0S_DELAY_MASK	GENMASK(17, 13)
+#define TC956X_ETH_L0S_DELAY_VALUE(x)	FIELD_PREP(TC956X_ETH_L0S_DELAY_MASK, x)
+
+#define TC956X_NFTS_2_5_GT		0x824978
+#define TC956X_NFTS_5_GT		0x82497c
+
+#define TC956X_PORT_LANE_ACCESS_ENABLE	0x828000
+
+#define TC956X_PHY_RATE_CHANGE_OVERRIDE	0x828040
+#define TC956X_PHY_RATE_CHANGE		0x828050
+
+#define TC956X_TX_MARGIN		0x828234
+
+#define TC956X_DFE_ENABLE		0x828a04
+#define TC956X_DFE_EQ0_MODE		0x828a08
+#define TC956X_DFE_EQ1_MODE		0x828a0c
+#define TC956X_DFE_EQ2_MODE		0x828a14
+#define TC956X_DFE_PD_MASK		0x828254
+
+#define TC956X_PORT_SELECT		0x82c02c
+#define TC956X_PORT_ACCESS_ENABLE	0x82c030
+
+#define TC956X_POWER_CONTROL		0x82b09c
+#define TC956X_POWER_CONTROL_OVREN	0x82b2c8
+
+#define TC956X_GPIO_MASK		0xfffffff3
+
+#define TC956X_TX_MARGIN_MIN_VAL	400000
+
+struct tc956x_pwrctrl_reg_setting {
+	unsigned int offset;
+	unsigned int val;
+};
+
+enum tc956x_pwrctrl_ports {
+	TC956X_USP,
+	TC956X_DSP1,
+	TC956X_DSP2,
+	TC956X_DSP3,
+	TC956X_ETHERNET,
+	TC956X_MAX
+};
+
+struct tc956x_pwrctrl_cfg {
+	u32 l0s_delay;
+	u32 l1_delay;
+	u32 tx_amp;
+	u8 nfts[2]; /* GEN1 & GEN2*/
+	bool disable_dfe;
+	bool disable_port;
+};
+
+#define TC956X_PWRCTL_MAX_SUPPLY	6
+
+struct tc956x_pwrctrl_ctx {
+	struct regulator_bulk_data supplies[TC956X_PWRCTL_MAX_SUPPLY];
+	struct tc956x_pwrctrl_cfg cfg[TC956X_MAX];
+	struct gpio_desc *reset_gpio;
+	struct i2c_adapter *adapter;
+	struct i2c_client *client;
+	struct pci_pwrctrl pwrctrl;
+};
+
+/*
+ * downstream port power off sequence, hardcoding the address
+ * as we don't know register names for these register offsets.
+ */
+static const struct tc956x_pwrctrl_reg_setting common_pwroff_seq[] = {
+	{0x82900c, 0x1},
+	{0x829010, 0x1},
+	{0x829018, 0x0},
+	{0x829020, 0x1},
+	{0x82902c, 0x1},
+	{0x829030, 0x1},
+	{0x82903c, 0x1},
+	{0x829058, 0x0},
+	{0x82905c, 0x1},
+	{0x829060, 0x1},
+	{0x8290cc, 0x1},
+	{0x8290d0, 0x1},
+	{0x8290d8, 0x1},
+	{0x8290e0, 0x1},
+	{0x8290e8, 0x1},
+	{0x8290ec, 0x1},
+	{0x8290f4, 0x1},
+	{0x82910c, 0x1},
+	{0x829110, 0x1},
+	{0x829114, 0x1},
+};
+
+static const struct tc956x_pwrctrl_reg_setting dsp1_pwroff_seq[] = {
+	{TC956X_PORT_ACCESS_ENABLE, 0x2},
+	{TC956X_PORT_LANE_ACCESS_ENABLE, 0x3},
+	{TC956X_POWER_CONTROL, 0x014f4804},
+	{TC956X_POWER_CONTROL_OVREN, 0x1},
+	{TC956X_PORT_ACCESS_ENABLE, 0x4},
+};
+
+static const struct tc956x_pwrctrl_reg_setting dsp2_pwroff_seq[] = {
+	{TC956X_PORT_ACCESS_ENABLE, 0x8},
+	{TC956X_PORT_LANE_ACCESS_ENABLE, 0x1},
+	{TC956X_POWER_CONTROL, 0x014f4804},
+	{TC956X_POWER_CONTROL_OVREN, 0x1},
+	{TC956X_PORT_ACCESS_ENABLE, 0x8},
+};
+
+/*
+ * Since all transfers are initiated by the probe, no locks are necessary,
+ * as there are no concurrent calls.
+ */
+static int tc956x_pwrctrl_i2c_write(struct i2c_client *client,
+				    u32 reg_addr, u32 reg_val)
+{
+	struct i2c_msg msg;
+	u8 msg_buf[7];
+	int ret;
+
+	msg.addr = client->addr;
+	msg.len = 7;
+	msg.flags = 0;
+
+	/* Big Endian for reg addr */
+	put_unaligned_be24(reg_addr, &msg_buf[0]);
+
+	/* Little Endian for reg val */
+	put_unaligned_le32(reg_val, &msg_buf[3]);
+
+	msg.buf = msg_buf;
+	ret = i2c_transfer(client->adapter, &msg, 1);
+	return ret == 1 ? 0 : ret;
+}
+
+static int tc956x_pwrctrl_i2c_read(struct i2c_client *client,
+				   u32 reg_addr, u32 *reg_val)
+{
+	struct i2c_msg msg[2];
+	u8 wr_data[3];
+	u32 rd_data;
+	int ret;
+
+	msg[0].addr = client->addr;
+	msg[0].len = 3;
+	msg[0].flags = 0;
+
+	/* Big Endian for reg addr */
+	put_unaligned_be24(reg_addr, &wr_data[0]);
+
+	msg[0].buf = wr_data;
+
+	msg[1].addr = client->addr;
+	msg[1].len = 4;
+	msg[1].flags = I2C_M_RD;
+
+	msg[1].buf = (u8 *)&rd_data;
+
+	ret = i2c_transfer(client->adapter, &msg[0], 2);
+	if (ret == 2) {
+		*reg_val = get_unaligned_le32(&rd_data);
+		return 0;
+	}
+
+	/* If only one message successfully completed, return -EIO */
+	return ret == 1 ? -EIO : ret;
+}
+
+static int tc956x_pwrctrl_i2c_bulk_write(struct i2c_client *client,
+					 const struct tc956x_pwrctrl_reg_setting *seq, int len)
+{
+	int ret, i;
+
+	for (i = 0; i < len; i++) {
+		ret = tc956x_pwrctrl_i2c_write(client, seq[i].offset, seq[i].val);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static int tc956x_pwrctrl_disable_port(struct tc956x_pwrctrl_ctx *ctx,
+				       enum tc956x_pwrctrl_ports port)
+{
+	struct tc956x_pwrctrl_cfg *cfg  = &ctx->cfg[port];
+	const struct tc956x_pwrctrl_reg_setting *seq;
+	int ret, len;
+
+	if (!cfg->disable_port)
+		return 0;
+
+	if (port == TC956X_DSP1) {
+		seq = dsp1_pwroff_seq;
+		len = ARRAY_SIZE(dsp1_pwroff_seq);
+	} else {
+		seq = dsp2_pwroff_seq;
+		len = ARRAY_SIZE(dsp2_pwroff_seq);
+	}
+
+	ret = tc956x_pwrctrl_i2c_bulk_write(ctx->client, seq, len);
+	if (ret)
+		return ret;
+
+	return tc956x_pwrctrl_i2c_bulk_write(ctx->client,
+					    common_pwroff_seq, ARRAY_SIZE(common_pwroff_seq));
+}
+
+static int tc956x_pwrctrl_set_l0s_l1_entry_delay(struct tc956x_pwrctrl_ctx *ctx,
+						 enum tc956x_pwrctrl_ports port, bool is_l1, u32 ns)
+{
+	u32 rd_val, units;
+	int ret;
+
+	if (!ns)
+		return 0;
+
+	/* convert to units of 256ns */
+	units = ns / 256;
+
+	if (port == TC956X_ETHERNET) {
+		ret = tc956x_pwrctrl_i2c_read(ctx->client, TC956X_EMBEDDED_ETH_DELAY, &rd_val);
+		if (ret)
+			return ret;
+
+		if (is_l1)
+			rd_val = u32_replace_bits(rd_val, units, TC956X_ETH_L1_DELAY_MASK);
+		else
+			rd_val = u32_replace_bits(rd_val, units, TC956X_ETH_L0S_DELAY_MASK);
+
+		return tc956x_pwrctrl_i2c_write(ctx->client, TC956X_EMBEDDED_ETH_DELAY, rd_val);
+	}
+
+	ret = tc956x_pwrctrl_i2c_write(ctx->client, TC956X_PORT_SELECT, BIT(port));
+	if (ret)
+		return ret;
+
+	return tc956x_pwrctrl_i2c_write(ctx->client,
+				       is_l1 ? TC956X_PORT_L1_DELAY : TC956X_PORT_L0S_DELAY, units);
+}
+
+static int tc956x_pwrctrl_set_tx_amplitude(struct tc956x_pwrctrl_ctx *ctx,
+					   enum tc956x_pwrctrl_ports port, u32 amp)
+{
+	int port_access;
+
+	if (amp < TC956X_TX_MARGIN_MIN_VAL)
+		return 0;
+
+	/*  txmargin = (Amp(uV) - 400000) / 3125 */
+	amp = (amp - TC956X_TX_MARGIN_MIN_VAL) / 3125;
+
+	switch (port) {
+	case TC956X_USP:
+		port_access = 0x1;
+		break;
+	case TC956X_DSP1:
+		port_access = 0x2;
+		break;
+	case TC956X_DSP2:
+		port_access = 0x8;
+		break;
+	default:
+		return -EINVAL;
+	};
+
+	struct tc956x_pwrctrl_reg_setting tx_amp_seq[] = {
+		{TC956X_PORT_ACCESS_ENABLE, port_access},
+		{TC956X_PORT_LANE_ACCESS_ENABLE, 0x3},
+		{TC956X_TX_MARGIN, amp},
+	};
+
+	return tc956x_pwrctrl_i2c_bulk_write(ctx->client, tx_amp_seq, ARRAY_SIZE(tx_amp_seq));
+}
+
+static int tc956x_pwrctrl_disable_dfe(struct tc956x_pwrctrl_ctx *ctx,
+				      enum tc956x_pwrctrl_ports port)
+{
+	struct tc956x_pwrctrl_cfg *cfg  = &ctx->cfg[port];
+	int port_access, lane_access = 0x3;
+	u32 phy_rate = 0x21;
+
+	if (!cfg->disable_dfe)
+		return 0;
+
+	switch (port) {
+	case TC956X_USP:
+		phy_rate = 0x1;
+		port_access = 0x1;
+		break;
+	case TC956X_DSP1:
+		port_access = 0x2;
+		break;
+	case TC956X_DSP2:
+		port_access = 0x8;
+		lane_access = 0x1;
+		break;
+	default:
+		return -EINVAL;
+	};
+
+	struct tc956x_pwrctrl_reg_setting disable_dfe_seq[] = {
+		{TC956X_PORT_ACCESS_ENABLE, port_access},
+		{TC956X_PORT_LANE_ACCESS_ENABLE, lane_access},
+		{TC956X_DFE_ENABLE, 0x0},
+		{TC956X_DFE_EQ0_MODE, 0x411},
+		{TC956X_DFE_EQ1_MODE, 0x11},
+		{TC956X_DFE_EQ2_MODE, 0x11},
+		{TC956X_DFE_PD_MASK, 0x7},
+		{TC956X_PHY_RATE_CHANGE_OVERRIDE, 0x10},
+		{TC956X_PHY_RATE_CHANGE, phy_rate},
+		{TC956X_PHY_RATE_CHANGE, 0x0},
+		{TC956X_PHY_RATE_CHANGE_OVERRIDE, 0x0},
+	};
+
+	return tc956x_pwrctrl_i2c_bulk_write(ctx->client,
+					    disable_dfe_seq, ARRAY_SIZE(disable_dfe_seq));
+}
+
+static int tc956x_pwrctrl_set_nfts(struct tc956x_pwrctrl_ctx *ctx,
+				   enum tc956x_pwrctrl_ports port, u8 *nfts)
+{
+	struct tc956x_pwrctrl_reg_setting nfts_seq[] = {
+		{TC956X_NFTS_2_5_GT, nfts[0]},
+		{TC956X_NFTS_5_GT, nfts[1]},
+	};
+	int ret;
+
+	if (!nfts[0])
+		return 0;
+
+	ret =  tc956x_pwrctrl_i2c_write(ctx->client, TC956X_PORT_SELECT, BIT(port));
+	if (ret)
+		return ret;
+
+	return tc956x_pwrctrl_i2c_bulk_write(ctx->client, nfts_seq, ARRAY_SIZE(nfts_seq));
+}
+
+static int tc956x_pwrctrl_assert_deassert_reset(struct tc956x_pwrctrl_ctx *ctx, bool deassert)
+{
+	int ret, val;
+
+	ret = tc956x_pwrctrl_i2c_write(ctx->client, TC956X_GPIO_CONFIG, TC956X_GPIO_MASK);
+	if (ret)
+		return ret;
+
+	val = deassert ? 0xc : 0;
+
+	return tc956x_pwrctrl_i2c_write(ctx->client, TC956X_RESET_GPIO, val);
+}
+
+static int tc956x_pwrctrl_parse_device_dt(struct tc956x_pwrctrl_ctx *ctx, struct device_node *node,
+					  enum tc956x_pwrctrl_ports port)
+{
+	struct tc956x_pwrctrl_cfg *cfg;
+	int ret;
+
+	cfg = &ctx->cfg[port];
+
+	/* Disable port if the status of the port is disabled. */
+	if (!of_device_is_available(node)) {
+		cfg->disable_port = true;
+		return 0;
+	};
+
+	ret = of_property_read_u32(node, "aspm-l0s-entry-delay-ns", &cfg->l0s_delay);
+	if (ret && ret != -EINVAL)
+		return ret;
+
+	ret = of_property_read_u32(node, "aspm-l1-entry-delay-ns", &cfg->l1_delay);
+	if (ret && ret != -EINVAL)
+		return ret;
+
+	ret = of_property_read_u32(node, "qcom,tx-amplitude-microvolt", &cfg->tx_amp);
+	if (ret && ret != -EINVAL)
+		return ret;
+
+	ret = of_property_read_u8_array(node, "nfts", cfg->nfts, 2);
+	if (ret && ret != -EINVAL)
+		return ret;
+
+	cfg->disable_dfe = of_property_read_bool(node, "qcom,no-dfe-support");
+
+	return 0;
+}
+
+static void tc956x_pwrctrl_power_off(struct tc956x_pwrctrl_ctx *ctx)
+{
+	gpiod_set_value(ctx->reset_gpio, 1);
+
+	regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
+}
+
+static int tc956x_pwrctrl_bring_up(struct tc956x_pwrctrl_ctx *ctx)
+{
+	struct tc956x_pwrctrl_cfg *cfg;
+	int ret, i;
+
+	ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
+	if (ret < 0)
+		return dev_err_probe(ctx->pwrctrl.dev, ret, "cannot enable regulators\n");
+
+	gpiod_set_value(ctx->reset_gpio, 0);
+
+	 /* wait for the internal osc frequency to stablise */
+	usleep_range(10000, 10500);
+
+	ret = tc956x_pwrctrl_assert_deassert_reset(ctx, false);
+	if (ret)
+		goto power_off;
+
+	for (i = 0; i < TC956X_MAX; i++) {
+		cfg = &ctx->cfg[i];
+		ret = tc956x_pwrctrl_disable_port(ctx, i);
+		if (ret) {
+			dev_err(ctx->pwrctrl.dev, "Disabling port failed\n");
+			goto power_off;
+		}
+
+		ret = tc956x_pwrctrl_set_l0s_l1_entry_delay(ctx, i, false, cfg->l0s_delay);
+		if (ret) {
+			dev_err(ctx->pwrctrl.dev, "Setting L0s entry delay failed\n");
+			goto power_off;
+		}
+
+		ret = tc956x_pwrctrl_set_l0s_l1_entry_delay(ctx, i, true, cfg->l1_delay);
+		if (ret) {
+			dev_err(ctx->pwrctrl.dev, "Setting L1 entry delay failed\n");
+			goto power_off;
+		}
+
+		ret = tc956x_pwrctrl_set_tx_amplitude(ctx, i, cfg->tx_amp);
+		if (ret) {
+			dev_err(ctx->pwrctrl.dev, "Setting Tx amplitube failed\n");
+			goto power_off;
+		}
+
+		ret = tc956x_pwrctrl_set_nfts(ctx, i, cfg->nfts);
+		if (ret) {
+			dev_err(ctx->pwrctrl.dev, "Setting nfts failed\n");
+			goto power_off;
+		}
+
+		ret = tc956x_pwrctrl_disable_dfe(ctx, i);
+		if (ret) {
+			dev_err(ctx->pwrctrl.dev, "Disabling DFE failed\n");
+			goto power_off;
+		}
+	}
+
+	ret = tc956x_pwrctrl_assert_deassert_reset(ctx, true);
+	if (!ret)
+		return 0;
+
+power_off:
+	tc956x_pwrctrl_power_off(ctx);
+	return ret;
+}
+
+static int tc956x_pwrctrl_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct pci_dev *pci_dev = to_pci_dev(dev->parent);
+	struct pci_host_bridge *bridge = pci_find_host_bridge(pci_dev->bus);
+	enum tc956x_pwrctrl_ports port;
+	struct tc956x_pwrctrl_ctx *ctx;
+	int ret, addr;
+
+	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
+	if (!ctx)
+		return -ENOMEM;
+
+	ret = of_property_read_u32_index(pdev->dev.of_node, "i2c-parent", 1, &addr);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to read i2c-parent property\n");
+
+	ctx->adapter = of_find_i2c_adapter_by_node(of_parse_phandle(dev->of_node, "i2c-parent", 0));
+	of_node_put(dev->of_node);
+	if (!ctx->adapter)
+		return dev_err_probe(dev, -EPROBE_DEFER, "Failed to find I2C adapter\n");
+
+	ctx->client = i2c_new_dummy_device(ctx->adapter, addr);
+	if (IS_ERR(ctx->client)) {
+		dev_err(dev, "Failed to create I2C client\n");
+		i2c_put_adapter(ctx->adapter);
+		return PTR_ERR(ctx->client);
+	}
+
+	ctx->supplies[0].supply = "vddc";
+	ctx->supplies[1].supply = "vdd18";
+	ctx->supplies[2].supply = "vdd09";
+	ctx->supplies[3].supply = "vddio1";
+	ctx->supplies[4].supply = "vddio2";
+	ctx->supplies[5].supply = "vddio18";
+	ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(ctx->supplies), ctx->supplies);
+	if (ret) {
+		dev_err_probe(dev, ret,
+			      "failed to get supply regulator\n");
+		goto remove_i2c;
+	}
+
+	ctx->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(ctx->reset_gpio)) {
+		ret = dev_err_probe(dev, PTR_ERR(ctx->reset_gpio), "failed to get reset GPIO\n");
+		goto remove_i2c;
+	}
+
+	pci_pwrctrl_init(&ctx->pwrctrl, dev);
+
+	port = TC956X_USP;
+	ret = tc956x_pwrctrl_parse_device_dt(ctx, pdev->dev.of_node, port);
+	if (ret) {
+		dev_err(dev, "failed to parse device tree properties: %d\n", ret);
+		goto remove_i2c;
+	}
+
+	/*
+	 * Downstream ports are always children of the upstream port.
+	 * The first node represents DSP1, the second node represents DSP2, and so on.
+	 */
+	for_each_child_of_node_scoped(pdev->dev.of_node, child) {
+		ret = tc956x_pwrctrl_parse_device_dt(ctx, child, port++);
+		if (ret)
+			break;
+		/* Embedded ethernet device are under DSP3 */
+		if (port == TC956X_DSP3)
+			for_each_child_of_node_scoped(child, child1) {
+				ret = tc956x_pwrctrl_parse_device_dt(ctx, child1, port++);
+				if (ret)
+					break;
+			}
+	}
+	if (ret) {
+		dev_err(dev, "failed to parse device tree properties: %d\n", ret);
+		goto remove_i2c;
+	}
+
+	if (!pcie_is_link_active(pci_dev) && bridge->ops->stop_link)
+		bridge->ops->stop_link(pci_dev->bus);
+
+	ret = tc956x_pwrctrl_bring_up(ctx);
+	if (ret)
+		goto remove_i2c;
+
+	if (!pcie_is_link_active(pci_dev) && bridge->ops->start_link) {
+		ret = bridge->ops->start_link(pci_dev->bus);
+		if (ret)
+			goto power_off;
+	}
+
+	ret = devm_pci_pwrctrl_device_set_ready(dev, &ctx->pwrctrl);
+	if (ret)
+		goto power_off;
+
+	platform_set_drvdata(pdev, ctx);
+
+	return 0;
+
+power_off:
+	tc956x_pwrctrl_power_off(ctx);
+remove_i2c:
+	i2c_unregister_device(ctx->client);
+	i2c_put_adapter(ctx->adapter);
+	return ret;
+}
+
+static void tc956x_pwrctrl_remove(struct platform_device *pdev)
+{
+	struct tc956x_pwrctrl_ctx *ctx = platform_get_drvdata(pdev);
+
+	tc956x_pwrctrl_power_off(ctx);
+	i2c_unregister_device(ctx->client);
+	i2c_put_adapter(ctx->adapter);
+}
+
+static const struct of_device_id tc956x_pwrctrl_of_match[] = {
+	{ .compatible = "pci1179,0623"},
+	{ }
+};
+MODULE_DEVICE_TABLE(of, tc956x_pwrctrl_of_match);
+
+static struct platform_driver tc956x_pwrctrl_driver = {
+	.driver = {
+		.name = "pwrctrl-tc956x",
+		.of_match_table = tc956x_pwrctrl_of_match,
+		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
+	},
+	.probe = tc956x_pwrctrl_probe,
+	.remove = tc956x_pwrctrl_remove,
+};
+module_platform_driver(tc956x_pwrctrl_driver);
+
+MODULE_AUTHOR("Krishna chaitanya chundru <quic_krichai@quicinc.com>");
+MODULE_DESCRIPTION("TC956x power control driver");
+MODULE_LICENSE("GPL");

-- 
2.34.1

Re: [PATCH v4 08/10] PCI: pwrctrl: Add power control driver for tc956x

[Dmitry Baryshkov]

On Tue, Feb 25, 2025 at 03:04:05PM +0530, Krishna Chaitanya Chundru wrote:
> TC956x is a PCIe switch which has one upstream and three downstream
> ports. To one of the downstream ports ethernet MAC is connected as endpoint
> device. Other two downstream ports are supposed to connect to external
> device. One Host can connect to TC956x by upstream port. TC956x switch
> needs to be configured after powering on and before PCIe link was up.
> 
> The PCIe controller driver already enables link training at the host side
> even before this driver probe happens, due to this when driver enables
> power to the switch it participates in the link training and PCIe link
> may come up before configuring the switch through i2c. Once the link is
> up the configuration done through i2c will not have any affect.To prevent
> the host from participating in link training, disable link training on the
> host side to ensure the link does not come up before the switch is
> configured via I2C.
> 
> Based up on dt property and type of the port, tc956x is configured
> through i2c.
> 
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> Reviewed-by: Bjorn Andersson <andersson@kernel.org>
> Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> ---
>  drivers/pci/pwrctrl/Kconfig              |   6 +
>  drivers/pci/pwrctrl/Makefile             |   1 +
>  drivers/pci/pwrctrl/pci-pwrctrl-tc956x.c | 625 +++++++++++++++++++++++++++++++
>  3 files changed, 632 insertions(+)
> 
> diff --git a/drivers/pci/pwrctrl/Kconfig b/drivers/pci/pwrctrl/Kconfig
> index 54589bb2403b..ae8a0a39f586 100644
> --- a/drivers/pci/pwrctrl/Kconfig
> +++ b/drivers/pci/pwrctrl/Kconfig
> @@ -10,3 +10,9 @@ config PCI_PWRCTL_PWRSEQ
>  	tristate
>  	select POWER_SEQUENCING
>  	select PCI_PWRCTL
> +
> +config PCI_PWRCTRL_TC956X
> +	tristate "PCI Power Control driver for TC956x PCIe switch"
> +	select PCI_PWRCTL
> +	help
> +	  Say Y here to enable the pwrctrl driver for TC956x PCIe switch.

If you were to resend the series for any reason, please include the
defconfig changes to enable the driver. If not, please submit it
separately.


-- 
With best wishes
Dmitry

Re: [PATCH v4 08/10] PCI: pwrctrl: Add power control driver for tc956x

[Konrad Dybcio]

On 25.02.2025 10:34 AM, Krishna Chaitanya Chundru wrote:
> TC956x is a PCIe switch which has one upstream and three downstream
> ports. To one of the downstream ports ethernet MAC is connected as endpoint
> device. Other two downstream ports are supposed to connect to external
> device. One Host can connect to TC956x by upstream port. TC956x switch
> needs to be configured after powering on and before PCIe link was up.
> 
> The PCIe controller driver already enables link training at the host side
> even before this driver probe happens, due to this when driver enables
> power to the switch it participates in the link training and PCIe link
> may come up before configuring the switch through i2c. Once the link is
> up the configuration done through i2c will not have any affect.To prevent
> the host from participating in link training, disable link training on the
> host side to ensure the link does not come up before the switch is
> configured via I2C.
> 
> Based up on dt property and type of the port, tc956x is configured
> through i2c.
> 
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> Reviewed-by: Bjorn Andersson <andersson@kernel.org>
> Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> ---


> +struct tc956x_pwrctrl_cfg {
> +	u32 l0s_delay;
> +	u32 l1_delay;
> +	u32 tx_amp;
> +	u8 nfts[2]; /* GEN1 & GEN2*/

GEN2 */

[...]

> +static int tc956x_pwrctrl_set_l0s_l1_entry_delay(struct tc956x_pwrctrl_ctx *ctx,
> +						 enum tc956x_pwrctrl_ports port, bool is_l1, u32 ns)
> +{
> +	u32 rd_val, units;
> +	int ret;
> +
> +	if (!ns)
> +		return 0;
> +
> +	/* convert to units of 256ns */
> +	units = ns / 256;

Should we round up here, so that values in 1 <= x < 256 give a delay
value of 1 unit? Or maybe such values are never expected?

[...]

> +static int tc956x_pwrctrl_set_tx_amplitude(struct tc956x_pwrctrl_ctx *ctx,
> +					   enum tc956x_pwrctrl_ports port, u32 amp)
> +{
> +	int port_access;
> +
> +	if (amp < TC956X_TX_MARGIN_MIN_VAL)
> +		return 0;
> +
> +	/*  txmargin = (Amp(uV) - 400000) / 3125 */

double space

> +	amp = (amp - TC956X_TX_MARGIN_MIN_VAL) / 3125;

similarly here, is 0 an expected value for 1 <= x < 3125?

Konrad

Re: [PATCH v4 08/10] PCI: pwrctrl: Add power control driver for tc956x

[Krishna Chaitanya Chundru]

On 2/25/2025 5:39 PM, Konrad Dybcio wrote:
> On 25.02.2025 10:34 AM, Krishna Chaitanya Chundru wrote:
>> TC956x is a PCIe switch which has one upstream and three downstream
>> ports. To one of the downstream ports ethernet MAC is connected as endpoint
>> device. Other two downstream ports are supposed to connect to external
>> device. One Host can connect to TC956x by upstream port. TC956x switch
>> needs to be configured after powering on and before PCIe link was up.
>>
>> The PCIe controller driver already enables link training at the host side
>> even before this driver probe happens, due to this when driver enables
>> power to the switch it participates in the link training and PCIe link
>> may come up before configuring the switch through i2c. Once the link is
>> up the configuration done through i2c will not have any affect.To prevent
>> the host from participating in link training, disable link training on the
>> host side to ensure the link does not come up before the switch is
>> configured via I2C.
>>
>> Based up on dt property and type of the port, tc956x is configured
>> through i2c.
>>
>> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
>> Reviewed-by: Bjorn Andersson <andersson@kernel.org>
>> Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>> ---
> 
> 
>> +struct tc956x_pwrctrl_cfg {
>> +	u32 l0s_delay;
>> +	u32 l1_delay;
>> +	u32 tx_amp;
>> +	u8 nfts[2]; /* GEN1 & GEN2*/
> 
> GEN2 */
> 
> [...]
> 
>> +static int tc956x_pwrctrl_set_l0s_l1_entry_delay(struct tc956x_pwrctrl_ctx *ctx,
>> +						 enum tc956x_pwrctrl_ports port, bool is_l1, u32 ns)
>> +{
>> +	u32 rd_val, units;
>> +	int ret;
>> +
>> +	if (!ns)
>> +		return 0;
>> +
>> +	/* convert to units of 256ns */
>> +	units = ns / 256;
> 
> Should we round up here, so that values in 1 <= x < 256 give a delay
> value of 1 unit? Or maybe such values are never expected?
> 
I will add a check above to return if ns < 256 as 0 is not expected
value.
> [...]
> 
>> +static int tc956x_pwrctrl_set_tx_amplitude(struct tc956x_pwrctrl_ctx *ctx,
>> +					   enum tc956x_pwrctrl_ports port, u32 amp)
>> +{
>> +	int port_access;
>> +
>> +	if (amp < TC956X_TX_MARGIN_MIN_VAL)
>> +		return 0;
>> +
>> +	/*  txmargin = (Amp(uV) - 400000) / 3125 */
> 
> double space
> 
>> +	amp = (amp - TC956X_TX_MARGIN_MIN_VAL) / 3125;
> 
> similarly here, is 0 an expected value for 1 <= x < 3125?
> 
Here 0 is expected value in this case.

- Krishna Chaitanya.
> Konrad

[PATCH v4 09/10] dt-bindings: PCI: qcom,pcie-sc7280: Add 'global' interrupt

[Krishna Chaitanya Chundru]

Qcom PCIe RC controllers are capable of generating 'global' SPI interrupt
to the host CPU. This interrupt can be used by the device driver to handle
PCIe link specific events such as Link up and Link down, which give the
driver a chance to start bus enumeration on its own when link is up and
initiate link training if link goes to a bad state. The PCIe driver can
still work without this interrupt but it will provide a nice user
experience when device gets plugged and removed.

Hence, document it in the binding along with the existing MSI interrupts.
Global interrupt is parsed as optional in driver, so adding it in bindings
will not break the ABI.

Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
 Documentation/devicetree/bindings/pci/qcom,pcie-sc7280.yaml | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie-sc7280.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie-sc7280.yaml
index 76cb9fbfd476..7ae09ba8da60 100644
--- a/Documentation/devicetree/bindings/pci/qcom,pcie-sc7280.yaml
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie-sc7280.yaml
@@ -54,7 +54,7 @@ properties:
 
   interrupts:
     minItems: 8
-    maxItems: 8
+    maxItems: 9
 
   interrupt-names:
     items:
@@ -66,6 +66,7 @@ properties:
       - const: msi5
       - const: msi6
       - const: msi7
+      - const: global
 
   resets:
     maxItems: 1
@@ -149,9 +150,10 @@ examples:
                          <GIC_SPI 313 IRQ_TYPE_LEVEL_HIGH>,
                          <GIC_SPI 314 IRQ_TYPE_LEVEL_HIGH>,
                          <GIC_SPI 374 IRQ_TYPE_LEVEL_HIGH>,
-                         <GIC_SPI 375 IRQ_TYPE_LEVEL_HIGH>;
+                         <GIC_SPI 375 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 306 IRQ_TYPE_LEVEL_HIGH>;
             interrupt-names = "msi0", "msi1", "msi2", "msi3",
-                              "msi4", "msi5", "msi6", "msi7";
+                              "msi4", "msi5", "msi6", "msi7", "global";
             #interrupt-cells = <1>;
             interrupt-map-mask = <0 0 0 0x7>;
             interrupt-map = <0 0 0 1 &intc 0 0 0 434 IRQ_TYPE_LEVEL_HIGH>,

-- 
2.34.1

Re: [PATCH v4 09/10] dt-bindings: PCI: qcom,pcie-sc7280: Add 'global' interrupt

[Krzysztof Kozlowski]

On Tue, Feb 25, 2025 at 03:04:06PM +0530, Krishna Chaitanya Chundru wrote:
> Qcom PCIe RC controllers are capable of generating 'global' SPI interrupt
> to the host CPU. This interrupt can be used by the device driver to handle
> PCIe link specific events such as Link up and Link down, which give the
> driver a chance to start bus enumeration on its own when link is up and
> initiate link training if link goes to a bad state. The PCIe driver can
> still work without this interrupt but it will provide a nice user
> experience when device gets plugged and removed.
> 
> Hence, document it in the binding along with the existing MSI interrupts.
> Global interrupt is parsed as optional in driver, so adding it in bindings
> will not break the ABI.
> 
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> ---
>  Documentation/devicetree/bindings/pci/qcom,pcie-sc7280.yaml | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie-sc7280.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie-sc7280.yaml
> index 76cb9fbfd476..7ae09ba8da60 100644
> --- a/Documentation/devicetree/bindings/pci/qcom,pcie-sc7280.yaml
> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie-sc7280.yaml
> @@ -54,7 +54,7 @@ properties:
>  
>    interrupts:
>      minItems: 8
> -    maxItems: 8
> +    maxItems: 9
>  
>    interrupt-names:
>      items:
> @@ -66,6 +66,7 @@ properties:
>        - const: msi5
>        - const: msi6
>        - const: msi7
> +      - const: global

Either context is missing or these are not synced with interrupts.

Best regards,
Krzysztof

Re: [PATCH v4 09/10] dt-bindings: PCI: qcom,pcie-sc7280: Add 'global' interrupt

[Bjorn Andersson]

On Wed, Feb 26, 2025 at 08:32:42AM +0100, Krzysztof Kozlowski wrote:
> On Tue, Feb 25, 2025 at 03:04:06PM +0530, Krishna Chaitanya Chundru wrote:
> > Qcom PCIe RC controllers are capable of generating 'global' SPI interrupt
> > to the host CPU. This interrupt can be used by the device driver to handle
> > PCIe link specific events such as Link up and Link down, which give the
> > driver a chance to start bus enumeration on its own when link is up and
> > initiate link training if link goes to a bad state. The PCIe driver can
> > still work without this interrupt but it will provide a nice user
> > experience when device gets plugged and removed.
> > 
> > Hence, document it in the binding along with the existing MSI interrupts.
> > Global interrupt is parsed as optional in driver, so adding it in bindings
> > will not break the ABI.
> > 
> > Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> > ---
> >  Documentation/devicetree/bindings/pci/qcom,pcie-sc7280.yaml | 8 +++++---
> >  1 file changed, 5 insertions(+), 3 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie-sc7280.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie-sc7280.yaml
> > index 76cb9fbfd476..7ae09ba8da60 100644
> > --- a/Documentation/devicetree/bindings/pci/qcom,pcie-sc7280.yaml
> > +++ b/Documentation/devicetree/bindings/pci/qcom,pcie-sc7280.yaml
> > @@ -54,7 +54,7 @@ properties:
> >  
> >    interrupts:
> >      minItems: 8
> > -    maxItems: 8
> > +    maxItems: 9
> >  
> >    interrupt-names:
> >      items:
> > @@ -66,6 +66,7 @@ properties:
> >        - const: msi5
> >        - const: msi6
> >        - const: msi7
> > +      - const: global
> 
> Either context is missing or these are not synced with interrupts.
> 

I think the patch context ("properties") is confusing here, but it looks
to me that these are in sync: interrupts is defined to have 8 items, and
interrupt-names is a list of msi0 through msi7.

@Krishna, these two last patches (adding the global interrupt) doesn't
seem strongly connected to the switch patches. So, if Krzysztof agrees
with above assessment, please submit them separately (i.e. a new series,
2 patches, v5).

Regards,
Bjorn

> Best regards,
> Krzysztof
> 

Re: [PATCH v4 09/10] dt-bindings: PCI: qcom,pcie-sc7280: Add 'global' interrupt

[Krzysztof Kozlowski]

On 26/02/2025 17:29, Bjorn Andersson wrote:
>>> @@ -54,7 +54,7 @@ properties:
>>>  
>>>    interrupts:
>>>      minItems: 8
>>> -    maxItems: 8
>>> +    maxItems: 9
>>>  
>>>    interrupt-names:
>>>      items:
>>> @@ -66,6 +66,7 @@ properties:
>>>        - const: msi5
>>>        - const: msi6
>>>        - const: msi7
>>> +      - const: global
>>
>> Either context is missing or these are not synced with interrupts.
>>
> 
> I think the patch context ("properties") is confusing here, but it looks
> to me that these are in sync: interrupts is defined to have 8 items, and
> interrupt-names is a list of msi0 through msi7.

interrupt-names has minItems 9 in this case, so they are not synced.
That's my concern


Best regards,
Krzysztof

Re: [PATCH v4 09/10] dt-bindings: PCI: qcom,pcie-sc7280: Add 'global' interrupt

[Krishna Chaitanya Chundru]

On 2/27/2025 3:03 AM, Krzysztof Kozlowski wrote:
> On 26/02/2025 17:29, Bjorn Andersson wrote:
>>>> @@ -54,7 +54,7 @@ properties:
>>>>   
>>>>     interrupts:
>>>>       minItems: 8
>>>> -    maxItems: 8
>>>> +    maxItems: 9
>>>>   
>>>>     interrupt-names:
>>>>       items:
>>>> @@ -66,6 +66,7 @@ properties:
>>>>         - const: msi5
>>>>         - const: msi6
>>>>         - const: msi7
>>>> +      - const: global
>>>
>>> Either context is missing or these are not synced with interrupts.
>>>
>>
>> I think the patch context ("properties") is confusing here, but it looks
>> to me that these are in sync: interrupts is defined to have 8 items, and
>> interrupt-names is a list of msi0 through msi7.
> 
> interrupt-names has minItems 9 in this case, so they are not synced.
> That's my concern
> 
Ok I will update the minItems to 9 as suggested.

- Krishna Chaitanya.
> 
> Best regards,
> Krzysztof

Re: [PATCH v4 09/10] dt-bindings: PCI: qcom,pcie-sc7280: Add 'global' interrupt

[Manivannan Sadhasivam]

On Thu, Feb 27, 2025 at 09:09:38AM +0530, Krishna Chaitanya Chundru wrote:
> 
> 
> On 2/27/2025 3:03 AM, Krzysztof Kozlowski wrote:
> > On 26/02/2025 17:29, Bjorn Andersson wrote:
> > > > > @@ -54,7 +54,7 @@ properties:
> > > > >     interrupts:
> > > > >       minItems: 8
> > > > > -    maxItems: 8
> > > > > +    maxItems: 9
> > > > >     interrupt-names:
> > > > >       items:
> > > > > @@ -66,6 +66,7 @@ properties:
> > > > >         - const: msi5
> > > > >         - const: msi6
> > > > >         - const: msi7
> > > > > +      - const: global
> > > > 
> > > > Either context is missing or these are not synced with interrupts.
> > > > 
> > > 
> > > I think the patch context ("properties") is confusing here, but it looks
> > > to me that these are in sync: interrupts is defined to have 8 items, and
> > > interrupt-names is a list of msi0 through msi7.
> > 
> > interrupt-names has minItems 9 in this case, so they are not synced.
> > That's my concern
> > 
> Ok I will update the minItems to 9 as suggested.
> 

You got it backwards. interrupt-names should have minItems as 8. Otherwise, old
DTS will break.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v4 09/10] dt-bindings: PCI: qcom,pcie-sc7280: Add 'global' interrupt

[Jorge Ramirez]

On 26/02/25 10:29:43, Bjorn Andersson wrote:
> On Wed, Feb 26, 2025 at 08:32:42AM +0100, Krzysztof Kozlowski wrote:
> > On Tue, Feb 25, 2025 at 03:04:06PM +0530, Krishna Chaitanya Chundru wrote:
> > > Qcom PCIe RC controllers are capable of generating 'global' SPI interrupt
> > > to the host CPU. This interrupt can be used by the device driver to handle
> > > PCIe link specific events such as Link up and Link down, which give the
> > > driver a chance to start bus enumeration on its own when link is up and
> > > initiate link training if link goes to a bad state. The PCIe driver can
> > > still work without this interrupt but it will provide a nice user
> > > experience when device gets plugged and removed.
> > > 
> > > Hence, document it in the binding along with the existing MSI interrupts.
> > > Global interrupt is parsed as optional in driver, so adding it in bindings
> > > will not break the ABI.
> > > 
> > > Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> > > ---
> > >  Documentation/devicetree/bindings/pci/qcom,pcie-sc7280.yaml | 8 +++++---
> > >  1 file changed, 5 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie-sc7280.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie-sc7280.yaml
> > > index 76cb9fbfd476..7ae09ba8da60 100644
> > > --- a/Documentation/devicetree/bindings/pci/qcom,pcie-sc7280.yaml
> > > +++ b/Documentation/devicetree/bindings/pci/qcom,pcie-sc7280.yaml
> > > @@ -54,7 +54,7 @@ properties:
> > >  
> > >    interrupts:
> > >      minItems: 8
> > > -    maxItems: 8
> > > +    maxItems: 9
> > >  
> > >    interrupt-names:
> > >      items:
> > > @@ -66,6 +66,7 @@ properties:
> > >        - const: msi5
> > >        - const: msi6
> > >        - const: msi7
> > > +      - const: global
> > 
> > Either context is missing or these are not synced with interrupts.
> > 
> 
> I think the patch context ("properties") is confusing here, but it looks
> to me that these are in sync: interrupts is defined to have 8 items, and
> interrupt-names is a list of msi0 through msi7.
> 
> @Krishna, these two last patches (adding the global interrupt) doesn't
> seem strongly connected to the switch patches. So, if Krzysztof agrees
> with above assessment, please submit them separately (i.e. a new series,
> 2 patches, v5).

um, but without these two patches, the functionality is broken requiring
users to manually rescan the pci bus (ie, via sysfs) to see what is
behind the bridge.

shouldnt the set include all the necessary patches? 


> 
> Regards,
> Bjorn
> 
> > Best regards,
> > Krzysztof
> > 

Re: [PATCH v4 09/10] dt-bindings: PCI: qcom,pcie-sc7280: Add 'global' interrupt

[Manivannan Sadhasivam]

On Wed, Mar 05, 2025 at 08:36:26AM +0100, Jorge Ramirez wrote:
> On 26/02/25 10:29:43, Bjorn Andersson wrote:
> > On Wed, Feb 26, 2025 at 08:32:42AM +0100, Krzysztof Kozlowski wrote:
> > > On Tue, Feb 25, 2025 at 03:04:06PM +0530, Krishna Chaitanya Chundru wrote:
> > > > Qcom PCIe RC controllers are capable of generating 'global' SPI interrupt
> > > > to the host CPU. This interrupt can be used by the device driver to handle
> > > > PCIe link specific events such as Link up and Link down, which give the
> > > > driver a chance to start bus enumeration on its own when link is up and
> > > > initiate link training if link goes to a bad state. The PCIe driver can
> > > > still work without this interrupt but it will provide a nice user
> > > > experience when device gets plugged and removed.
> > > > 
> > > > Hence, document it in the binding along with the existing MSI interrupts.
> > > > Global interrupt is parsed as optional in driver, so adding it in bindings
> > > > will not break the ABI.
> > > > 
> > > > Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> > > > ---
> > > >  Documentation/devicetree/bindings/pci/qcom,pcie-sc7280.yaml | 8 +++++---
> > > >  1 file changed, 5 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie-sc7280.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie-sc7280.yaml
> > > > index 76cb9fbfd476..7ae09ba8da60 100644
> > > > --- a/Documentation/devicetree/bindings/pci/qcom,pcie-sc7280.yaml
> > > > +++ b/Documentation/devicetree/bindings/pci/qcom,pcie-sc7280.yaml
> > > > @@ -54,7 +54,7 @@ properties:
> > > >  
> > > >    interrupts:
> > > >      minItems: 8
> > > > -    maxItems: 8
> > > > +    maxItems: 9
> > > >  
> > > >    interrupt-names:
> > > >      items:
> > > > @@ -66,6 +66,7 @@ properties:
> > > >        - const: msi5
> > > >        - const: msi6
> > > >        - const: msi7
> > > > +      - const: global
> > > 
> > > Either context is missing or these are not synced with interrupts.
> > > 
> > 
> > I think the patch context ("properties") is confusing here, but it looks
> > to me that these are in sync: interrupts is defined to have 8 items, and
> > interrupt-names is a list of msi0 through msi7.
> > 
> > @Krishna, these two last patches (adding the global interrupt) doesn't
> > seem strongly connected to the switch patches. So, if Krzysztof agrees
> > with above assessment, please submit them separately (i.e. a new series,
> > 2 patches, v5).
> 
> um, but without these two patches, the functionality is broken requiring
> users to manually rescan the pci bus (ie, via sysfs) to see what is
> behind the bridge.
> 

It is not *broken* actually. The series is for enabling the PCIe switch and the
'global' IRQ is a host behavior. So technically both are not dependent on each
other.

> shouldnt the set include all the necessary patches? 
> 

FWIW, I have submitted a series that adds the IRQ for most of the arm64
platforms:
https://lore.kernel.org/linux-arm-msm/20250227-pcie-global-irq-v1-0-2b70a7819d1e@linaro.org/

There is a possibility that the above series could get merged before this one.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v4 09/10] dt-bindings: PCI: qcom,pcie-sc7280: Add 'global' interrupt

[Krzysztof Kozlowski]

On 05/03/2025 08:36, Jorge Ramirez wrote:
>> I think the patch context ("properties") is confusing here, but it looks
>> to me that these are in sync: interrupts is defined to have 8 items, and
>> interrupt-names is a list of msi0 through msi7.
>>
>> @Krishna, these two last patches (adding the global interrupt) doesn't
>> seem strongly connected to the switch patches. So, if Krzysztof agrees
>> with above assessment, please submit them separately (i.e. a new series,
>> 2 patches, v5).
> 
> um, but without these two patches, the functionality is broken requiring
> users to manually rescan the pci bus (ie, via sysfs) to see what is
> behind the bridge.
> 
> shouldnt the set include all the necessary patches? 

Broken? Then the patchset should be NAKed because nothing can depend on
the DTS and the DTS should not depend on the drivers without some sort
of explanation, either.

Also nothing in the commit msg explains this dependency, so I don't get
the problem here.

Best regards,
Krzysztof

[PATCH v4 10/10] arm64: dts: qcom: sc7280: Add 'global' interrupt to the PCIe RC nodes

[Krishna Chaitanya Chundru]

Qcom PCIe RC controllers are capable of generating 'global' SPI interrupt
to the host CPUs. This interrupt can be used by the device driver to
identify events such as PCIe link specific events, safety events, etc...

Hence, add it to the PCIe RC node along with the existing MSI interrupts.

Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
 arch/arm64/boot/dts/qcom/sc7280.dtsi | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index b2e2b1f26731..6d71353592c9 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -2225,9 +2225,10 @@ pcie1: pcie@1c08000 {
 				     <GIC_SPI 313 IRQ_TYPE_LEVEL_HIGH>,
 				     <GIC_SPI 314 IRQ_TYPE_LEVEL_HIGH>,
 				     <GIC_SPI 374 IRQ_TYPE_LEVEL_HIGH>,
-				     <GIC_SPI 375 IRQ_TYPE_LEVEL_HIGH>;
+				     <GIC_SPI 375 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 306 IRQ_TYPE_LEVEL_HIGH>;
 			interrupt-names = "msi0", "msi1", "msi2", "msi3",
-					  "msi4", "msi5", "msi6", "msi7";
+					  "msi4", "msi5", "msi6", "msi7", "global";
 			#interrupt-cells = <1>;
 			interrupt-map-mask = <0 0 0 0x7>;
 			interrupt-map = <0 0 0 1 &intc 0 0 0 434 IRQ_TYPE_LEVEL_HIGH>,

-- 
2.34.1

Re: [PATCH v4 10/10] arm64: dts: qcom: sc7280: Add 'global' interrupt to the PCIe RC nodes

[Dmitry Baryshkov]

On Tue, Feb 25, 2025 at 03:04:07PM +0530, Krishna Chaitanya Chundru wrote:
> Qcom PCIe RC controllers are capable of generating 'global' SPI interrupt
> to the host CPUs. This interrupt can be used by the device driver to
> identify events such as PCIe link specific events, safety events, etc...
> 
> Hence, add it to the PCIe RC node along with the existing MSI interrupts.
> 
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> ---
>  arch/arm64/boot/dts/qcom/sc7280.dtsi | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

-- 
With best wishes
Dmitry

Re: [PATCH v4 10/10] arm64: dts: qcom: sc7280: Add 'global' interrupt to the PCIe RC nodes

[Konrad Dybcio]

On 25.02.2025 10:34 AM, Krishna Chaitanya Chundru wrote:
> Qcom PCIe RC controllers are capable of generating 'global' SPI interrupt
> to the host CPUs. This interrupt can be used by the device driver to
> identify events such as PCIe link specific events, safety events, etc...
> 
> Hence, add it to the PCIe RC node along with the existing MSI interrupts.
> 
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> ---

The computer tells me this one is wakeup-capable - is this something we
are interested in describing for link up?

Konrad

Re: [PATCH v4 00/10] PCI: Enable Power and configure the TC956x PCIe switch

[Manivannan Sadhasivam]

On Tue, Feb 25, 2025 at 03:03:57PM +0530, Krishna Chaitanya Chundru wrote:
> TC956x is the PCIe switch which has one upstream and three downstream
> ports. To one of the downstream ports ethernet MAC is connected as endpoint
> device. Other two downstream ports are supposed to connect to external
> device. One Host can connect to TC956x by upstream port.
> 
> TC956x switch power is controlled by the GPIO's. After powering on
> the switch will immediately participate in the link training. if the
> host is also ready by that time PCIe link will established. 
> 
> The TC956x needs to configured certain parameters like de-emphasis,
> disable unused port etc before link is established.
> 
> As the controller starts link training before the probe of pwrctl driver,
> the PCIe link may come up as soon as we power on the switch. Due to this
> configuring the switch itself through i2c will not have any effect as
> this configuration needs to done before link training. To avoid this
> introduce two functions in pci_ops to start_link() & stop_link() which
> will disable the link training if the PCIe link is not up yet.
> 
> Enable global IRQ for PCIe controller so that recan can happen when
> link was up through global IRQ.
>  

Move these patches to a separate series.

> This series depends on the https://lore.kernel.org/all/20250124101038.3871768-3-krishna.chundru@oss.qualcomm.com/
> 
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> ---
> changes in v3:
> - The QPS615 PCIe switch is rebranded version of Toshiba switch TC956x.
>   There is no difference between both the switches, both
>   has two open downstream ports and one embedded downstream port
>   to which Ethernet MAC is connected.
>   As QPS615 is the rebranded version of Toshiba switch rename qps615
>   with tc956x so that this driver can be leveraged by all who are using Toshiba switch.

This should be moved to the description as a NOTE.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v4 00/10] PCI: Enable Power and configure the TC956x PCIe switch

[Manivannan Sadhasivam]

On Thu, Feb 27, 2025 at 09:27:47AM +0530, Manivannan Sadhasivam wrote:
> On Tue, Feb 25, 2025 at 03:03:57PM +0530, Krishna Chaitanya Chundru wrote:
> > TC956x is the PCIe switch which has one upstream and three downstream
> > ports. To one of the downstream ports ethernet MAC is connected as endpoint
> > device. Other two downstream ports are supposed to connect to external
> > device. One Host can connect to TC956x by upstream port.
> > 
> > TC956x switch power is controlled by the GPIO's. After powering on
> > the switch will immediately participate in the link training. if the
> > host is also ready by that time PCIe link will established. 
> > 
> > The TC956x needs to configured certain parameters like de-emphasis,
> > disable unused port etc before link is established.
> > 
> > As the controller starts link training before the probe of pwrctl driver,
> > the PCIe link may come up as soon as we power on the switch. Due to this
> > configuring the switch itself through i2c will not have any effect as
> > this configuration needs to done before link training. To avoid this
> > introduce two functions in pci_ops to start_link() & stop_link() which
> > will disable the link training if the PCIe link is not up yet.
> > 
> > Enable global IRQ for PCIe controller so that recan can happen when
> > link was up through global IRQ.
> >  
> 
> Move these patches to a separate series.
> 

Or you can just drop them. I have a series that adds global IRQ to most of the
SoCs and sc7280 is one of them.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v4 00/10] PCI: Enable Power and configure the TC956x PCIe switch

[Krishna Chaitanya Chundru]

On 2/27/2025 9:29 AM, Manivannan Sadhasivam wrote:
> On Thu, Feb 27, 2025 at 09:27:47AM +0530, Manivannan Sadhasivam wrote:
>> On Tue, Feb 25, 2025 at 03:03:57PM +0530, Krishna Chaitanya Chundru wrote:
>>> TC956x is the PCIe switch which has one upstream and three downstream
>>> ports. To one of the downstream ports ethernet MAC is connected as endpoint
>>> device. Other two downstream ports are supposed to connect to external
>>> device. One Host can connect to TC956x by upstream port.
>>>
>>> TC956x switch power is controlled by the GPIO's. After powering on
>>> the switch will immediately participate in the link training. if the
>>> host is also ready by that time PCIe link will established.
>>>
>>> The TC956x needs to configured certain parameters like de-emphasis,
>>> disable unused port etc before link is established.
>>>
>>> As the controller starts link training before the probe of pwrctl driver,
>>> the PCIe link may come up as soon as we power on the switch. Due to this
>>> configuring the switch itself through i2c will not have any effect as
>>> this configuration needs to done before link training. To avoid this
>>> introduce two functions in pci_ops to start_link() & stop_link() which
>>> will disable the link training if the PCIe link is not up yet.
>>>
>>> Enable global IRQ for PCIe controller so that recan can happen when
>>> link was up through global IRQ.
>>>   
>>
>> Move these patches to a separate series.
>>
> 
> Or you can just drop them. I have a series that adds global IRQ to most of the
> SoCs and sc7280 is one of them.
> 
> - Mani
fine for me, I will drop.

- Krishna Chaitanya.
>