* Re: [PATCH v2 2/3] phy: qcom-qusb2: correst PHY description for IPQ6018
From: Konrad Dybcio @ 2026-07-07 7:39 UTC (permalink / raw)
To: Dmitry Baryshkov, Vinod Koul, Neil Armstrong, Kathiravan T,
Baruch Siach, Dmitry Baryshkov, Krishna Kurapati, Manu Gautam,
Kishon Vijay Abraham I
Cc: linux-arm-msm, linux-phy, linux-kernel
In-Reply-To: <20260706-fix-qusb2-v2-2-8d9cd73b1db7@oss.qualcomm.com>
On 7/6/26 3:53 PM, Dmitry Baryshkov wrote:
> Qualcomm IPQ6018 doesn't need to reach power collapse or retention of
> the USB voltage rails, so autoresume is not used on that platform.
> Instead of programming a fake register bit (BIT(0) of TEST1, while the
> QUSB2 platforms on that platform should use BIT(3) of TEST_CTRL),
> explicitly disable autoresume programming on these devices via the flag
> in the platform data.
>
> Fixes: 2cfbe6765b7a ("phy: qcom-qusb2: add QUSB2 support for IPQ6018")
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: pci: qcom,hawi-pcie: Add Maili PCIe compatible
From: Manivannan Sadhasivam @ 2026-07-07 7:19 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Vivek Pernamitta, Lorenzo Pieralisi, Krzysztof Wilczyński,
Rob Herring, Bjorn Helgaas, Krzysztof Kozlowski, Conor Dooley,
Bjorn Andersson, Vinod Koul, Neil Armstrong, linux-arm-msm,
linux-pci, devicetree, linux-kernel, linux-phy
In-Reply-To: <92b7efac-32c3-4a89-9b08-55da294a6bc0@kernel.org>
On Tue, Jul 07, 2026 at 07:55:45AM +0200, Krzysztof Kozlowski wrote:
> On 07/07/2026 06:53, Manivannan Sadhasivam wrote:
> > On Mon, Jul 06, 2026 at 07:35:52PM +0200, Krzysztof Kozlowski wrote:
> >> On 06/07/2026 18:36, Manivannan Sadhasivam wrote:
> >>> On Mon, Jul 06, 2026 at 08:46:41AM +0200, Krzysztof Kozlowski wrote:
> >>>> On Fri, Jul 03, 2026 at 05:38:40PM +0530, Vivek Pernamitta wrote:
> >>>>> Add qcom,maili-pcie as a compatible string that falls back to
> >>>>> qcom,hawi-pcie, as the Maili SoC reuses the Hawi PCIe controller IP.
> >>>>>
> >>>>> The Maili SoC is a derivative of Hawi and shares the same PCIe
> >>>>> controller architecture, allowing reuse of the existing Hawi PCIe
> >>>>> DT bindings.
> >>>>>
> >>>>> Signed-off-by: Vivek Pernamitta <vivek.pernamitta@oss.qualcomm.com>
> >>>>> ---
> >>>>> Dependencies:
> >>>>> - PCI: qcom: Add PCIe support for upcoming Hawi SoC
> >>>>> https://lore.kernel.org/all/20260625-hawi-pcie-v4-0-1a578603cd86@oss.qualcomm.com/
> >>>>
> >>>> Squash the patches then.
> >>>>
> >>>
> >>> But these are two independent SoC additions, isn't it?
> >>
> >> It's adding a single compatible, no? If a patch adding a single
> >> compatible cannot be done without multi-patchset dependencies making
> >> testing by tooling impossible, then probably that work should not be
> >> sent separately or even as separate patch. And I am not saying anything
> >> new because half a year ago (around Kaanapali and Glymur) I voiced
> >> strong opinion about that.
> >>
> >> But really, you do not need to add two compatibles in two separate patches.
> >>
> >
> > Ok then. I'll squash this patch with Hawi patch which already got applied.
>
> Applied? That's not what I had in mind. This mentions external
> dependency, not something which was applied. Should be squashed there.
>
I merged the dependency for 7.3 before looking at this series. So now, squashed
this patch with that.
- Mani
--
மணிவண்ணன் சதாசிவம்
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH 3/9] dt-bindings: PCI: Add bindings for endpoint gpios
From: Manivannan Sadhasivam @ 2026-07-07 6:48 UTC (permalink / raw)
To: Sushrut Shree Trivedi
Cc: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Helgaas, Lorenzo Pieralisi,
Krzysztof Wilczyński, Bjorn Andersson, Chaitanya Chundru,
Bartosz Golaszewski, Konrad Dybcio, linux-arm-msm, linux-phy,
devicetree, linux-kernel, linux-pci
In-Reply-To: <20260701-shikra-upstream-v1-3-e1a721eb8943@oss.qualcomm.com>
On Wed, Jul 01, 2026 at 12:32:45AM +0530, Sushrut Shree Trivedi wrote:
Use proper prefix for the subject.
> Add devicetree bindings for TC9563 GPIO's which are
> used to control endpoint power and reset.
>
> Signed-off-by: Sushrut Shree Trivedi <sushrut.trivedi@oss.qualcomm.com>
> ---
> .../devicetree/bindings/pci/toshiba,tc9563.yaml | 22 +++++++++++++++++++++-
> 1 file changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/pci/toshiba,tc9563.yaml b/Documentation/devicetree/bindings/pci/toshiba,tc9563.yaml
> index b3ad05d90201..f9f71f28aa92 100644
> --- a/Documentation/devicetree/bindings/pci/toshiba,tc9563.yaml
> +++ b/Documentation/devicetree/bindings/pci/toshiba,tc9563.yaml
> @@ -26,6 +26,11 @@ properties:
> reg:
> maxItems: 1
>
> + gpio-controller: true
> +
> + '#gpio-cells':
> + const: 2
Fix indent.
> +
> resx-gpios:
> maxItems: 1
> description:
> @@ -69,6 +74,17 @@ $defs:
> type: object
>
> properties:
> + reset-gpios:
> + description:
> + Specify the TC9563 GPIO used to reset the endpoint
> + connected to the particular TC9563 downstream port.
> +
This the PERST# signal, isn't it? If so, mention it as-is.
> + ep-pwr-en-gpios:
> + description:
> + Specify the TC9563 GPIO used for enabling power to
> + the endpoint connected to the particular TC9563
> + downstream port.
> +
So this GPIO controls 3.3v supply to the endpoint? If so, it should be described
as a GPIO controlled regulator:
vreg_pcie_3v3: regulator-pcie-3v3 {
compatible = "regulator-fixed";
...
gpio = <&tc9563 2 GPIO_ACTIVE_HIGH>;
...
};
And then you should let the pwrctrl driver control the upstream port. The upside
here is that the pwrctrl driver already controls both 3.3v and reset-gpios
(PERST#). So you do not need to have any change in the TC9563 driver.
- Mani
--
மணிவண்ணன் சதாசிவம்
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH 2/9] dt-bindings: PCI: qcom: Document the Shikra PCIe Controller
From: Manivannan Sadhasivam @ 2026-07-07 6:01 UTC (permalink / raw)
To: Sushrut Shree Trivedi
Cc: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Helgaas, Lorenzo Pieralisi,
Krzysztof Wilczyński, Bjorn Andersson, Chaitanya Chundru,
Bartosz Golaszewski, Konrad Dybcio, linux-arm-msm, linux-phy,
devicetree, linux-kernel, linux-pci
In-Reply-To: <20260701-shikra-upstream-v1-2-e1a721eb8943@oss.qualcomm.com>
On Wed, Jul 01, 2026 at 12:32:44AM +0530, Sushrut Shree Trivedi wrote:
> Add a dedicated schema for the PCIe controller found on the Shikra
> platform.
>
> Signed-off-by: Sushrut Shree Trivedi <sushrut.trivedi@oss.qualcomm.com>
> ---
> .../devicetree/bindings/pci/qcom,shikra-pcie.yaml | 211 +++++++++++++++++++++
> 1 file changed, 211 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/pci/qcom,shikra-pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,shikra-pcie.yaml
> new file mode 100644
> index 000000000000..f9d1dba9dd2e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/qcom,shikra-pcie.yaml
> @@ -0,0 +1,211 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pci/qcom,shikra-pcie.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm Shikra PCI Express Root Complex
> +
> +maintainers:
> + - Bjorn Andersson <andersson@kernel.org>
> + - Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> +
> +description:
> + Qualcomm Shikra SoC (and compatible) PCIe root complex controller is based on
s/PCIe root complex controller/PCIe Root Complex
> + the Synopsys DesignWare PCIe IP.
> +
> +properties:
> + compatible:
> + const: qcom,shikra-pcie
> +
> + reg:
> + minItems: 5
> + maxItems: 6
> +
> + reg-names:
> + minItems: 5
> + items:
> + - const: parf # Qualcomm specific registers
> + - const: dbi # DesignWare PCIe registers
> + - const: elbi # External local bus interface registers
> + - const: atu # ATU address space
> + - const: config # PCIe configuration space
> + - const: mhi # MHI registers
MHI is not optional.
> +
> + clocks:
> + minItems: 7
> + maxItems: 9
> +
> + clock-names:
> + minItems: 7
> + items:
> + - const: aux # Auxiliary clock
> + - const: cfg # Configuration clock
> + - const: bus_master # Master AXI clock
> + - const: bus_slave # Slave AXI clock
> + - const: slave_q2a # Slave Q2A clock
> + - const: ddrss_memnoc_pcie # PCIe SF MEMNOC clock
> + - const: tile # PCIe tile SYS NoC clock
> + - const: qmip_pcie_ahb # QMIP PCIe AHB clock
Why optional clocks?
> +
> + interrupts:
> + minItems: 8
> + maxItems: 9
> +
> + interrupt-names:
> + minItems: 8
> + items:
> + - const: msi0
> + - const: msi1
> + - const: msi2
> + - const: msi3
> + - const: msi4
> + - const: msi5
> + - const: msi6
> + - const: msi7
> + - const: global
Same here, why global interrupt is optional?
> +
> + resets:
> + minItems: 1
> + maxItems: 2
> +
> + reset-names:
> + minItems: 1
> + items:
> + - const: pci # PCIe core reset
> + - const: link_down # PCIe link down reset
Same here.
> +
> +required:
> + - power-domains
> + - resets
> + - reset-names
> +
> +allOf:
> + - $ref: qcom,pcie-common.yaml#
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/clock/qcom,dispcc-qcm2290.h>
> + #include <dt-bindings/clock/qcom,qcm2290-gpucc.h>
What are these includes for?
> + #include <dt-bindings/clock/qcom,rpmcc.h>
> +
> + soc {
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
Get rid of 'soc' node.
> + pcie@45e8000 {
> + device_type = "pci";
> + compatible = "qcom,shikra-pcie";
> + reg = <0x0 0x045e8000 0x0 0x3000>,
> + <0x0 0x60000000 0x0 0xf1d>,
> + <0x0 0x60000f20 0x0 0xa8>,
> + <0x0 0x60001000 0x0 0x1000>,
> + <0x0 0x60100000 0x0 0x100000>,
> + <0x0 0x045eb000 0x0 0x1000>;
> + reg-names = "parf",
> + "dbi",
> + "elbi",
> + "atu",
> + "config",
> + "mhi";
> + #address-cells = <3>;
> + #size-cells = <2>;
> + ranges = <0x01000000 0x0 0x00000000 0x0 0x60200000 0x0 0x100000>,
> + <0x02000000 0x0 0x60300000 0x0 0x60300000 0x0 0x3d00000>,
> + <0x03000000 0x4 0x00000000 0x4 0x00000000 0x3 0x0000000>;
> + bus-range = <0x00 0xff>;
> +
> + linux,pci-domain = <0>;
> + num-lanes = <1>;
> +
> + interrupts = <GIC_SPI 491 IRQ_TYPE_LEVEL_HIGH 0>,
> + <GIC_SPI 492 IRQ_TYPE_LEVEL_HIGH 0>,
> + <GIC_SPI 493 IRQ_TYPE_LEVEL_HIGH 0>,
> + <GIC_SPI 494 IRQ_TYPE_LEVEL_HIGH 0>,
> + <GIC_SPI 495 IRQ_TYPE_LEVEL_HIGH 0>,
> + <GIC_SPI 496 IRQ_TYPE_LEVEL_HIGH 0>,
> + <GIC_SPI 497 IRQ_TYPE_LEVEL_HIGH 0>,
> + <GIC_SPI 498 IRQ_TYPE_LEVEL_HIGH 0>,
> + <GIC_SPI 489 IRQ_TYPE_LEVEL_HIGH 0>;
> + interrupt-names = "msi0",
> + "msi1",
> + "msi2",
> + "msi3",
> + "msi4",
> + "msi5",
> + "msi6",
> + "msi7",
> + "global";
> +
> + interrupt-map = <0 0 0 1 &intc 0 0 0 499 IRQ_TYPE_LEVEL_HIGH>,
> + <0 0 0 2 &intc 0 0 0 500 IRQ_TYPE_LEVEL_HIGH>,
> + <0 0 0 3 &intc 0 0 0 501 IRQ_TYPE_LEVEL_HIGH>,
> + <0 0 0 4 &intc 0 0 0 502 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-map-mask = <0 0 0 0x7>;
> + #interrupt-cells = <1>;
> +
> + clocks = <&gcc GCC_PCIE_AUX_CLK>,
> + <&gcc GCC_PCIE_CFG_AHB_CLK>,
> + <&gcc GCC_PCIE_MSTR_AXI_CLK>,
> + <&gcc GCC_PCIE_SLV_AXI_CLK>,
> + <&gcc GCC_PCIE_SLV_Q2A_AXI_CLK>,
> + <&gcc GCC_DDRSS_MEMNOC_PCIE_SF_CLK>,
> + <&gcc GCC_PCIE_TILE_AXI_SYS_NOC_CLK>,
> + <&gcc GCC_QMIP_PCIE_CFG_AHB_CLK>;
> + clock-names = "aux",
> + "cfg",
> + "bus_master",
> + "bus_slave",
> + "slave_q2a",
> + "ddrss_memnoc_pcie",
> + "tile",
> + "qmip_pcie_ahb";
> +
> + assigned-clocks = <&gcc GCC_PCIE_AUX_CLK>;
> + assigned-clock-rates = <19200000>;
> +
> + interconnects = <&system_noc MASTER_PCIE2_0 RPM_ALWAYS_TAG
> + &mc_virt SLAVE_EBI_CH0 RPM_ALWAYS_TAG>,
> + <&mem_noc MASTER_AMPSS_M0 RPM_ACTIVE_TAG
> + &config_noc SLAVE_PCIE2_0 RPM_ACTIVE_TAG>;
> +
> + interconnect-names = "pcie-mem",
> + "cpu-pcie";
> +
> + iommu-map = <0x0 &apps_smmu 0x800 0x1>,
> + <0x100 &apps_smmu 0x801 0x1>;
> +
> + resets = <&gcc GCC_PCIE_BCR>;
> + reset-names = "pci";
> +
> + power-domains = <&gcc GCC_PCIE_GDSC>;
> +
> + max-link-speed = <2>;
You don't need 'max-link-speed' unless you want to limit the link speed.
> +
> + operating-points-v2 = <&pcie_opp_table>;
> +
> + status = "disabled";
> +
No, you should not disable the example.
> + pcie_opp_table: opp-table {
> + compatible = "operating-points-v2";
> +
> + /* GEN 1 x1 */
> + opp-2500000 {
> + opp-hz = /bits/ 64 <2500000>;
> + required-opps = <&rpmpd_opp_nom>;
Are you sure that the power domain need to be in NOM for 2.5 GT/s?
> + opp-peak-kBps = <250000 1>;
> + opp-level = <1>;
> + };
> +
> + /* GEN 2 x1 */
> + opp-5000000 {
> + opp-hz = /bits/ 64 <5000000>;
> + required-opps = <&rpmpd_opp_nom>;
Same here.
> + opp-peak-kBps = <500000 1>;
> + opp-level = <2>;
> + };
> + };
Odd indent.
- Mani
--
மணிவண்ணன் சதாசிவம்
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: pci: qcom,hawi-pcie: Add Maili PCIe compatible
From: Krzysztof Kozlowski @ 2026-07-07 5:55 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: Vivek Pernamitta, Lorenzo Pieralisi, Krzysztof Wilczyński,
Rob Herring, Bjorn Helgaas, Krzysztof Kozlowski, Conor Dooley,
Bjorn Andersson, Vinod Koul, Neil Armstrong, linux-arm-msm,
linux-pci, devicetree, linux-kernel, linux-phy
In-Reply-To: <43b5xhqvnapamfv4aomid647kaxrudgjcbnnca23t3vxitv67t@njdlthj2oisa>
On 07/07/2026 06:53, Manivannan Sadhasivam wrote:
> On Mon, Jul 06, 2026 at 07:35:52PM +0200, Krzysztof Kozlowski wrote:
>> On 06/07/2026 18:36, Manivannan Sadhasivam wrote:
>>> On Mon, Jul 06, 2026 at 08:46:41AM +0200, Krzysztof Kozlowski wrote:
>>>> On Fri, Jul 03, 2026 at 05:38:40PM +0530, Vivek Pernamitta wrote:
>>>>> Add qcom,maili-pcie as a compatible string that falls back to
>>>>> qcom,hawi-pcie, as the Maili SoC reuses the Hawi PCIe controller IP.
>>>>>
>>>>> The Maili SoC is a derivative of Hawi and shares the same PCIe
>>>>> controller architecture, allowing reuse of the existing Hawi PCIe
>>>>> DT bindings.
>>>>>
>>>>> Signed-off-by: Vivek Pernamitta <vivek.pernamitta@oss.qualcomm.com>
>>>>> ---
>>>>> Dependencies:
>>>>> - PCI: qcom: Add PCIe support for upcoming Hawi SoC
>>>>> https://lore.kernel.org/all/20260625-hawi-pcie-v4-0-1a578603cd86@oss.qualcomm.com/
>>>>
>>>> Squash the patches then.
>>>>
>>>
>>> But these are two independent SoC additions, isn't it?
>>
>> It's adding a single compatible, no? If a patch adding a single
>> compatible cannot be done without multi-patchset dependencies making
>> testing by tooling impossible, then probably that work should not be
>> sent separately or even as separate patch. And I am not saying anything
>> new because half a year ago (around Kaanapali and Glymur) I voiced
>> strong opinion about that.
>>
>> But really, you do not need to add two compatibles in two separate patches.
>>
>
> Ok then. I'll squash this patch with Hawi patch which already got applied.
Applied? That's not what I had in mind. This mentions external
dependency, not something which was applied. Should be squashed there.
Best regards,
Krzysztof
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH 3/9] dt-bindings: PCI: Add bindings for endpoint gpios
From: Sushrut Shree Trivedi @ 2026-07-07 5:42 UTC (permalink / raw)
To: Rob Herring (Arm)
Cc: Krzysztof Wilczyński, Neil Armstrong, Konrad Dybcio,
linux-phy, linux-kernel, Conor Dooley, Bjorn Andersson,
Bartosz Golaszewski, Vinod Koul, linux-arm-msm,
Manivannan Sadhasivam, Krzysztof Kozlowski, Lorenzo Pieralisi,
Chaitanya Chundru, linux-pci, devicetree, Bjorn Helgaas
In-Reply-To: <178285172702.288391.13610342196466613275.robh@kernel.org>
On 7/1/2026 2:05 AM, Rob Herring (Arm) wrote:
> On Wed, 01 Jul 2026 00:32:45 +0530, Sushrut Shree Trivedi wrote:
>> Add devicetree bindings for TC9563 GPIO's which are
>> used to control endpoint power and reset.
>>
>> Signed-off-by: Sushrut Shree Trivedi <sushrut.trivedi@oss.qualcomm.com>
>> ---
>> .../devicetree/bindings/pci/toshiba,tc9563.yaml | 22 +++++++++++++++++++++-
>> 1 file changed, 21 insertions(+), 1 deletion(-)
>>
> My bot found errors running 'make dt_binding_check' on your patch:
>
> yamllint warnings/errors:
> ./Documentation/devicetree/bindings/pci/toshiba,tc9563.yaml:32:9: [warning] wrong indentation: expected 4 but found 8 (indentation)
>
> dtschema/dtc warnings/errors:
>
> doc reference errors (make refcheckdocs):
>
> See https://patchwork.kernel.org/project/devicetree/patch/20260701-shikra-upstream-v1-3-e1a721eb8943@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.
ACK'd.
Sushrut
>
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH 2/9] dt-bindings: PCI: qcom: Document the Shikra PCIe Controller
From: Sushrut Shree Trivedi @ 2026-07-07 5:41 UTC (permalink / raw)
To: Rob Herring (Arm)
Cc: Lorenzo Pieralisi, Krzysztof Wilczyński, Konrad Dybcio,
Bjorn Andersson, Vinod Koul, Chaitanya Chundru, Bjorn Helgaas,
devicetree, Manivannan Sadhasivam, Krzysztof Kozlowski,
linux-kernel, linux-phy, linux-arm-msm, Conor Dooley,
Neil Armstrong, linux-pci, Bartosz Golaszewski
In-Reply-To: <178285172593.288348.3890129606237873483.robh@kernel.org>
On 7/1/2026 2:05 AM, Rob Herring (Arm) wrote:
> On Wed, 01 Jul 2026 00:32:44 +0530, Sushrut Shree Trivedi wrote:
>> Add a dedicated schema for the PCIe controller found on the Shikra
>> platform.
>>
>> Signed-off-by: Sushrut Shree Trivedi <sushrut.trivedi@oss.qualcomm.com>
>> ---
>> .../devicetree/bindings/pci/qcom,shikra-pcie.yaml | 211 +++++++++++++++++++++
>> 1 file changed, 211 insertions(+)
>>
> My bot found errors running 'make dt_binding_check' on your patch:
>
> yamllint warnings/errors:
>
> dtschema/dtc warnings/errors:
> Lexical error: Documentation/devicetree/bindings/pci/qcom,shikra-pcie.example.dts:57.29-36 Unexpected 'GIC_SPI'
> Lexical error: Documentation/devicetree/bindings/pci/qcom,shikra-pcie.example.dts:57.41-60 Unexpected 'IRQ_TYPE_LEVEL_HIGH'
> Lexical error: Documentation/devicetree/bindings/pci/qcom,shikra-pcie.example.dts:58.29-36 Unexpected 'GIC_SPI'
> Lexical error: Documentation/devicetree/bindings/pci/qcom,shikra-pcie.example.dts:58.41-60 Unexpected 'IRQ_TYPE_LEVEL_HIGH'
> Lexical error: Documentation/devicetree/bindings/pci/qcom,shikra-pcie.example.dts:59.29-36 Unexpected 'GIC_SPI'
> Lexical error: Documentation/devicetree/bindings/pci/qcom,shikra-pcie.example.dts:59.41-60 Unexpected 'IRQ_TYPE_LEVEL_HIGH'
> Lexical error: Documentation/devicetree/bindings/pci/qcom,shikra-pcie.example.dts:60.29-36 Unexpected 'GIC_SPI'
> Lexical error: Documentation/devicetree/bindings/pci/qcom,shikra-pcie.example.dts:60.41-60 Unexpected 'IRQ_TYPE_LEVEL_HIGH'
> Lexical error: Documentation/devicetree/bindings/pci/qcom,shikra-pcie.example.dts:61.29-36 Unexpected 'GIC_SPI'
> Lexical error: Documentation/devicetree/bindings/pci/qcom,shikra-pcie.example.dts:61.41-60 Unexpected 'IRQ_TYPE_LEVEL_HIGH'
> Lexical error: Documentation/devicetree/bindings/pci/qcom,shikra-pcie.example.dts:62.29-36 Unexpected 'GIC_SPI'
> Lexical error: Documentation/devicetree/bindings/pci/qcom,shikra-pcie.example.dts:62.41-60 Unexpected 'IRQ_TYPE_LEVEL_HIGH'
> Lexical error: Documentation/devicetree/bindings/pci/qcom,shikra-pcie.example.dts:63.29-36 Unexpected 'GIC_SPI'
> Lexical error: Documentation/devicetree/bindings/pci/qcom,shikra-pcie.example.dts:63.41-60 Unexpected 'IRQ_TYPE_LEVEL_HIGH'
> Lexical error: Documentation/devicetree/bindings/pci/qcom,shikra-pcie.example.dts:64.29-36 Unexpected 'GIC_SPI'
> Lexical error: Documentation/devicetree/bindings/pci/qcom,shikra-pcie.example.dts:64.41-60 Unexpected 'IRQ_TYPE_LEVEL_HIGH'
> Lexical error: Documentation/devicetree/bindings/pci/qcom,shikra-pcie.example.dts:65.29-36 Unexpected 'GIC_SPI'
> Lexical error: Documentation/devicetree/bindings/pci/qcom,shikra-pcie.example.dts:65.41-60 Unexpected 'IRQ_TYPE_LEVEL_HIGH'
> Lexical error: Documentation/devicetree/bindings/pci/qcom,shikra-pcie.example.dts:76.56-75 Unexpected 'IRQ_TYPE_LEVEL_HIGH'
> Lexical error: Documentation/devicetree/bindings/pci/qcom,shikra-pcie.example.dts:77.56-75 Unexpected 'IRQ_TYPE_LEVEL_HIGH'
> Lexical error: Documentation/devicetree/bindings/pci/qcom,shikra-pcie.example.dts:78.56-75 Unexpected 'IRQ_TYPE_LEVEL_HIGH'
> Lexical error: Documentation/devicetree/bindings/pci/qcom,shikra-pcie.example.dts:79.56-75 Unexpected 'IRQ_TYPE_LEVEL_HIGH'
> Lexical error: Documentation/devicetree/bindings/pci/qcom,shikra-pcie.example.dts:83.30-46 Unexpected 'GCC_PCIE_AUX_CLK'
> Lexical error: Documentation/devicetree/bindings/pci/qcom,shikra-pcie.example.dts:84.30-50 Unexpected 'GCC_PCIE_CFG_AHB_CLK'
> Lexical error: Documentation/devicetree/bindings/pci/qcom,shikra-pcie.example.dts:85.30-51 Unexpected 'GCC_PCIE_MSTR_AXI_CLK'
> Lexical error: Documentation/devicetree/bindings/pci/qcom,shikra-pcie.example.dts:86.30-50 Unexpected 'GCC_PCIE_SLV_AXI_CLK'
> Lexical error: Documentation/devicetree/bindings/pci/qcom,shikra-pcie.example.dts:87.30-54 Unexpected 'GCC_PCIE_SLV_Q2A_AXI_CLK'
> Lexical error: Documentation/devicetree/bindings/pci/qcom,shikra-pcie.example.dts:88.30-58 Unexpected 'GCC_DDRSS_MEMNOC_PCIE_SF_CLK'
> Lexical error: Documentation/devicetree/bindings/pci/qcom,shikra-pcie.example.dts:89.30-59 Unexpected 'GCC_PCIE_TILE_AXI_SYS_NOC_CLK'
> Lexical error: Documentation/devicetree/bindings/pci/qcom,shikra-pcie.example.dts:90.30-55 Unexpected 'GCC_QMIP_PCIE_CFG_AHB_CLK'
> Lexical error: Documentation/devicetree/bindings/pci/qcom,shikra-pcie.example.dts:100.39-55 Unexpected 'GCC_PCIE_AUX_CLK'
> Lexical error: Documentation/devicetree/bindings/pci/qcom,shikra-pcie.example.dts:103.44-58 Unexpected 'MASTER_PCIE2_0'
> Lexical error: Documentation/devicetree/bindings/pci/qcom,shikra-pcie.example.dts:103.59-73 Unexpected 'RPM_ALWAYS_TAG'
> Lexical error: Documentation/devicetree/bindings/pci/qcom,shikra-pcie.example.dts:104.40-53 Unexpected 'SLAVE_EBI_CH0'
> Lexical error: Documentation/devicetree/bindings/pci/qcom,shikra-pcie.example.dts:104.54-68 Unexpected 'RPM_ALWAYS_TAG'
> Lexical error: Documentation/devicetree/bindings/pci/qcom,shikra-pcie.example.dts:105.41-56 Unexpected 'MASTER_AMPSS_M0'
> Lexical error: Documentation/devicetree/bindings/pci/qcom,shikra-pcie.example.dts:105.57-71 Unexpected 'RPM_ACTIVE_TAG'
> Lexical error: Documentation/devicetree/bindings/pci/qcom,shikra-pcie.example.dts:106.43-56 Unexpected 'SLAVE_PCIE2_0'
> Lexical error: Documentation/devicetree/bindings/pci/qcom,shikra-pcie.example.dts:106.57-71 Unexpected 'RPM_ACTIVE_TAG'
> Lexical error: Documentation/devicetree/bindings/pci/qcom,shikra-pcie.example.dts:114.30-42 Unexpected 'GCC_PCIE_BCR'
> Lexical error: Documentation/devicetree/bindings/pci/qcom,shikra-pcie.example.dts:117.37-50 Unexpected 'GCC_PCIE_GDSC'
> FATAL ERROR: Syntax error parsing input tree
> make[2]: *** [scripts/Makefile.dtbs:140: Documentation/devicetree/bindings/pci/qcom,shikra-pcie.example.dtb] Error 1
> make[2]: *** Waiting for unfinished jobs....
> make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1669: dt_binding_check] Error 2
> make: *** [Makefile:248: __sub-make] Error 2
>
> doc reference errors (make refcheckdocs):
>
> See https://patchwork.kernel.org/project/devicetree/patch/20260701-shikra-upstream-v1-2-e1a721eb8943@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.
ACK'd.
Sushrut
>
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH 8/9] arm64: dts: qcom: shikra-evk: Add TC9563 PCIe switch node for PCIe
From: Sushrut Shree Trivedi @ 2026-07-07 5:41 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Helgaas, Lorenzo Pieralisi,
Krzysztof Wilczyński, Manivannan Sadhasivam, Bjorn Andersson,
Chaitanya Chundru, Bartosz Golaszewski, Konrad Dybcio,
linux-arm-msm, linux-phy, devicetree, linux-kernel, linux-pci
In-Reply-To: <20260630193059.GA225112@bhelgaas>
On 7/1/2026 1:00 AM, Bjorn Helgaas wrote:
> On Wed, Jul 01, 2026 at 12:32:50AM +0530, Sushrut Shree Trivedi wrote:
>> Add a node for the TC9563 PCIe switch connected to PCIe. The switch
>> has three downstream ports.Two embedded Ethernet devices are present
>> on one of the downstream ports. All the ports present in the
>> node represent the downstream ports and embedded endpoints.
>>
>> Power to the TC9563 is supplied through two LDO regulators, which
>> are on by default and are added as fixed regulators. TC9563 can be
>> configured through I2C.
> s/ports.Two/ports. Two/
>
> Possibly subject doesn't need two uses of "PCIe".
ACK'd.
Sushrut
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH 7/9] arm64: dts: qcom: shikra: Add PCIe PHY and controller nodes
From: Sushrut Shree Trivedi @ 2026-07-07 5:40 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Helgaas, Lorenzo Pieralisi,
Krzysztof Wilczyński, Manivannan Sadhasivam, Bjorn Andersson,
Chaitanya Chundru, Bartosz Golaszewski, Konrad Dybcio,
linux-arm-msm, linux-phy, devicetree, linux-kernel, linux-pci
In-Reply-To: <20260630192944.GA224708@bhelgaas>
On 7/1/2026 12:59 AM, Bjorn Helgaas wrote:
> On Wed, Jul 01, 2026 at 12:32:49AM +0530, Sushrut Shree Trivedi wrote:
>> Shikra supports single PCIe instance with 5GT/s x1 lane.
> s/ / /
> s/lane/link/
ACK'd
Sushrut
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH 6/9] PCI/pwrctrl: tc9563: Add API to control endpoint power and reset
From: Sushrut Shree Trivedi @ 2026-07-07 5:40 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Helgaas, Lorenzo Pieralisi,
Krzysztof Wilczyński, Manivannan Sadhasivam, Bjorn Andersson,
Chaitanya Chundru, Bartosz Golaszewski, Konrad Dybcio,
linux-arm-msm, linux-phy, devicetree, linux-kernel, linux-pci
In-Reply-To: <20260630192838.GA223662@bhelgaas>
On 7/1/2026 12:58 AM, Bjorn Helgaas wrote:
> On Wed, Jul 01, 2026 at 12:32:48AM +0530, Sushrut Shree Trivedi wrote:
>> Some platform utilise TC9563 GPIOs to enable power and
>> control reset of endpoints.
>>
>> This patch adds support to parse endpoint reset and power enable
>> gpios from each TC9563 port node in the devicetree. To configure
>> these GPIO's during the POWER ON sequence, two new API's are
>> introduced: tc9563_ep_pwr_en() and tc9563_ep_assert_deassert_reset().
> s/Some platform utilise/Some platforms utilise/
> s/This patch adds/Add/
> s/gpios/GPIOs/
>
> Add tc9563_ep_pwr_en() and tc9563_ep_assert_deassert_reset() to
> configure these GPIOs during the power-on sequence.
>
> Wrap to fill 75 columns.
ACK'd.
Sushrut
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH 3/9] dt-bindings: PCI: Add bindings for endpoint gpios
From: Sushrut Shree Trivedi @ 2026-07-07 5:39 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Helgaas, Lorenzo Pieralisi,
Krzysztof Wilczyński, Manivannan Sadhasivam, Bjorn Andersson,
Chaitanya Chundru, Bartosz Golaszewski, Konrad Dybcio,
linux-arm-msm, linux-phy, devicetree, linux-kernel, linux-pci
In-Reply-To: <20260630192236.GA222338@bhelgaas>
On 7/1/2026 12:52 AM, Bjorn Helgaas wrote:
> On Wed, Jul 01, 2026 at 12:32:45AM +0530, Sushrut Shree Trivedi wrote:
>> Add devicetree bindings for TC9563 GPIO's which are
>> used to control endpoint power and reset.
> Include context in subject line. Regrettably, previous commits to
> toshiba,tc9563.yaml don't include that either, but I think something
> like this would be good:
>
> dt-bindings: PCI: toshiba,tc9563: Add endpoint GPIO bindings
>
> s/GPIO's/GPIOs/
>
> Wrap to fill 75 columns.
ACK'd.
Sushrut
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH 2/9] dt-bindings: PCI: qcom: Document the Shikra PCIe Controller
From: Sushrut Shree Trivedi @ 2026-07-07 5:29 UTC (permalink / raw)
To: Krzysztof Kozlowski, Vinod Koul, Neil Armstrong, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bjorn Helgaas,
Lorenzo Pieralisi, Krzysztof Wilczyński,
Manivannan Sadhasivam, Bjorn Andersson, Chaitanya Chundru,
Bartosz Golaszewski, Konrad Dybcio
Cc: linux-arm-msm, linux-phy, devicetree, linux-kernel, linux-pci
In-Reply-To: <dd9e68f3-c33d-47d1-871f-f085550b88b1@kernel.org>
On 7/1/2026 11:56 AM, Krzysztof Kozlowski wrote:
> On 30/06/2026 21:02, Sushrut Shree Trivedi wrote:
>> Add a dedicated schema for the PCIe controller found on the Shikra
>> platform.
>>
>> Signed-off-by: Sushrut Shree Trivedi <sushrut.trivedi@oss.qualcomm.com>
>> ---
>> .../devicetree/bindings/pci/qcom,shikra-pcie.yaml | 211 +++++++++++++++++++++
>> 1 file changed, 211 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/pci/qcom,shikra-pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,shikra-pcie.yaml
>> new file mode 100644
>> index 000000000000..f9d1dba9dd2e
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/pci/qcom,shikra-pcie.yaml
>> @@ -0,0 +1,211 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/pci/qcom,shikra-pcie.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Qualcomm Shikra PCI Express Root Complex
>> +
>> +maintainers:
>> + - Bjorn Andersson <andersson@kernel.org>
>> + - Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
>> +
>> +description:
>> + Qualcomm Shikra SoC (and compatible) PCIe root complex controller is based on
>> + the Synopsys DesignWare PCIe IP.
>> +
>> +properties:
>> + compatible:
>> + const: qcom,shikra-pcie
>> +
>> + reg:
>> + minItems: 5
>> + maxItems: 6
> Same comments as other recent Qualcomm bindings. Don't invent stuff,
> take what was reviewed from the list so we won't have to repeat.
>
> ...
ACK'd. Will use latest binding patches as reference in v2.
>
>
>> + power-domains = <&gcc GCC_PCIE_GDSC>;
>> +
>> + max-link-speed = <2>;
>> +
>> + operating-points-v2 = <&pcie_opp_table>;
>> +
>> + status = "disabled";
> Drop, you never tested the binding in such case.
ACK'd.
>
>> +
>> + pcie_opp_table: opp-table {
>> + compatible = "operating-points-v2";
> Broken indent.
ACK'd.
>
>> +
>> + /* GEN 1 x1 */
>> + opp-2500000 {
>> + opp-hz = /bits/ 64 <2500000>;
>> + required-opps = <&rpmpd_opp_nom>;
>> + opp-peak-kBps = <250000 1>;
>> + opp-level = <1>;
>> + };
>> +
>> + /* GEN 2 x1 */
>> + opp-5000000 {
>> + opp-hz = /bits/ 64 <5000000>;
>> + required-opps = <&rpmpd_opp_nom>;
>> + opp-peak-kBps = <500000 1>;
>> + opp-level = <2>;
>> + };
>> + };
>> + };
>> + };
>>
>
> Best regards,
> Krzysztof
Sushrut
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH 3/9] dt-bindings: PCI: Add bindings for endpoint gpios
From: Sushrut Shree Trivedi @ 2026-07-07 5:26 UTC (permalink / raw)
To: Krzysztof Kozlowski, Vinod Koul, Neil Armstrong, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bjorn Helgaas,
Lorenzo Pieralisi, Krzysztof Wilczyński,
Manivannan Sadhasivam, Bjorn Andersson, Chaitanya Chundru,
Bartosz Golaszewski, Konrad Dybcio
Cc: linux-arm-msm, linux-phy, devicetree, linux-kernel, linux-pci
In-Reply-To: <59c96d6e-95f4-4b03-b1a7-21e7cf6ce43d@kernel.org>
On 7/1/2026 11:57 AM, Krzysztof Kozlowski wrote:
> On 30/06/2026 21:02, Sushrut Shree Trivedi wrote:
>> toshiba,tx-amplitude-microvolt:
>> description:
>> Change Tx Margin setting for low power consumption.
>> @@ -104,7 +120,7 @@ examples:
>> #address-cells = <3>;
>> #size-cells = <2>;
>>
>> - pcie@0 {
>> + tc9563: pcie@0 {
> And you change indentation because?
>
>
> Just like the other patch, this wasn't tested, right?
>
>
> Best regards,
> Krzysztof
That was a miss from my end, we only ran schema checks.
Will be fixed in v2.
Sushrut
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: pci: qcom,hawi-pcie: Add Maili PCIe compatible
From: Manivannan Sadhasivam @ 2026-07-07 5:25 UTC (permalink / raw)
To: Vivek Pernamitta
Cc: Lorenzo Pieralisi, Krzysztof Wilczyński, Rob Herring,
Bjorn Helgaas, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
Vinod Koul, Neil Armstrong, linux-arm-msm, linux-pci, devicetree,
linux-kernel, linux-phy
In-Reply-To: <20260703-vdev_maili_dt_bindings-v1-1-c9df0916cd5e@oss.qualcomm.com>
On Fri, Jul 03, 2026 at 05:38:40PM +0530, Vivek Pernamitta wrote:
> Add qcom,maili-pcie as a compatible string that falls back to
> qcom,hawi-pcie, as the Maili SoC reuses the Hawi PCIe controller IP.
>
> The Maili SoC is a derivative of Hawi and shares the same PCIe
> controller architecture, allowing reuse of the existing Hawi PCIe
> DT bindings.
>
> Signed-off-by: Vivek Pernamitta <vivek.pernamitta@oss.qualcomm.com>
> ---
> Dependencies:
> - PCI: qcom: Add PCIe support for upcoming Hawi SoC
> https://lore.kernel.org/all/20260625-hawi-pcie-v4-0-1a578603cd86@oss.qualcomm.com/
Squashed with the Hawi patch, thanks!
- Mani
--
மணிவண்ணன் சதாசிவம்
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH 7/9] arm64: dts: qcom: shikra: Add PCIe PHY and controller nodes
From: Sushrut Shree Trivedi @ 2026-07-07 5:23 UTC (permalink / raw)
To: Konrad Dybcio, Vinod Koul, Neil Armstrong, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bjorn Helgaas,
Lorenzo Pieralisi, Krzysztof Wilczyński,
Manivannan Sadhasivam, Bjorn Andersson, Chaitanya Chundru,
Bartosz Golaszewski, Konrad Dybcio
Cc: linux-arm-msm, linux-phy, devicetree, linux-kernel, linux-pci
In-Reply-To: <12e86cc4-d96e-44f5-8c87-87a6df321115@oss.qualcomm.com>
On 7/1/2026 4:04 PM, Konrad Dybcio wrote:
> On 6/30/26 9:02 PM, Sushrut Shree Trivedi wrote:
>> Shikra supports single PCIe instance with 5GT/s x1 lane.
>> Add PCIe controller and PHY node for this single instance.
>>
>> Signed-off-by: Sushrut Shree Trivedi <sushrut.trivedi@oss.qualcomm.com>
>> ---
> [...]
>
>
>> +
>> + max-link-speed = <2>;
> Please add a single-line comment right above explaining that the
> host supports higher speeds, but the attached PHY is only Gen2,
> so we need this manual limitation
>
> Konrad
ACK'd.
Sushrut
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH 8/9] arm64: dts: qcom: shikra-evk: Add TC9563 PCIe switch node for PCIe
From: Sushrut Shree Trivedi @ 2026-07-07 5:22 UTC (permalink / raw)
To: Konrad Dybcio, Vinod Koul, Neil Armstrong, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bjorn Helgaas,
Lorenzo Pieralisi, Krzysztof Wilczyński,
Manivannan Sadhasivam, Bjorn Andersson, Chaitanya Chundru,
Bartosz Golaszewski, Konrad Dybcio
Cc: linux-arm-msm, linux-phy, devicetree, linux-kernel, linux-pci
In-Reply-To: <2efcd4b6-0a99-4bb2-b428-4cb4805c20d1@oss.qualcomm.com>
On 7/1/2026 4:05 PM, Konrad Dybcio wrote:
> On 6/30/26 9:02 PM, Sushrut Shree Trivedi wrote:
>> Add a node for the TC9563 PCIe switch connected to PCIe. The switch
>> has three downstream ports.Two embedded Ethernet devices are present
>> on one of the downstream ports. All the ports present in the
>> node represent the downstream ports and embedded endpoints.
>>
>> Power to the TC9563 is supplied through two LDO regulators, which
>> are on by default and are added as fixed regulators. TC9563 can be
>> configured through I2C.
>>
>> Signed-off-by: Sushrut Shree Trivedi <sushrut.trivedi@oss.qualcomm.com>
>> ---
> [...]
>
>> +&pcie {
>> + wake-gpios = <&tlmm 119 GPIO_ACTIVE_LOW>;
> This property belongs to the port node
>
> Please also mention in the commit message the reason for the
> PERST# pin remaining undescribed
>
> Konrad
ACK'd.
Sushrut
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH 9/9] arm64: dts: qcom: shikra-(cqm/cqs/iqs)-evk: Enable PCIe PHY node
From: Sushrut Shree Trivedi @ 2026-07-07 5:21 UTC (permalink / raw)
To: Konrad Dybcio, Vinod Koul, Neil Armstrong, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bjorn Helgaas,
Lorenzo Pieralisi, Krzysztof Wilczyński,
Manivannan Sadhasivam, Bjorn Andersson, Chaitanya Chundru,
Bartosz Golaszewski, Konrad Dybcio
Cc: linux-arm-msm, linux-phy, devicetree, linux-kernel, linux-pci
In-Reply-To: <6e42bc67-9a2f-4e64-bb49-30fd845263a2@oss.qualcomm.com>
On 7/1/2026 4:06 PM, Konrad Dybcio wrote:
> On 6/30/26 9:02 PM, Sushrut Shree Trivedi wrote:
>> Enable the PCIe PHY for the single PCIe intance on the Shikra
>> CQS, CQM and the IQS platforms.
>>
>> IQS platform uses a different powergrid than CQS/CQM which explain
>> the different PHY supplies for IQS variant.
>>
>> Signed-off-by: Sushrut Shree Trivedi <sushrut.trivedi@oss.qualcomm.com>
>> ---
>> arch/arm64/boot/dts/qcom/shikra-cqm-evk.dts | 7 +++++++
>> arch/arm64/boot/dts/qcom/shikra-cqs-evk.dts | 7 +++++++
>> arch/arm64/boot/dts/qcom/shikra-iqs-evk.dts | 7 +++++++
>> 3 files changed, 21 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/shikra-cqm-evk.dts b/arch/arm64/boot/dts/qcom/shikra-cqm-evk.dts
>> index 683b5245923b..06ad32041546 100644
>> --- a/arch/arm64/boot/dts/qcom/shikra-cqm-evk.dts
>> +++ b/arch/arm64/boot/dts/qcom/shikra-cqm-evk.dts
>> @@ -60,6 +60,13 @@ vreg_pmu_ch1: ldo4 {
>> };
>> };
>>
>> +&pcie_phy {
>> + vdda-phy-supply = <&pm4125_l13>;
>> + vdda-pll-supply = <&pm4125_l9>;
>> +
>> + status = "okay";
>> +};
> I think it makes sense to push the status=okay to the evk file
> (because we already describe the PCIe switch there) and only keep
> the supplies here (because they differ)
>
> Konrad
ACK'd.
Sushrut
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: pci: qcom,hawi-pcie: Add Maili PCIe compatible
From: Manivannan Sadhasivam @ 2026-07-07 4:53 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Vivek Pernamitta, Lorenzo Pieralisi, Krzysztof Wilczyński,
Rob Herring, Bjorn Helgaas, Krzysztof Kozlowski, Conor Dooley,
Bjorn Andersson, Vinod Koul, Neil Armstrong, linux-arm-msm,
linux-pci, devicetree, linux-kernel, linux-phy
In-Reply-To: <9967f1a0-a9e3-4d62-b5df-7f625610a89d@kernel.org>
On Mon, Jul 06, 2026 at 07:35:52PM +0200, Krzysztof Kozlowski wrote:
> On 06/07/2026 18:36, Manivannan Sadhasivam wrote:
> > On Mon, Jul 06, 2026 at 08:46:41AM +0200, Krzysztof Kozlowski wrote:
> >> On Fri, Jul 03, 2026 at 05:38:40PM +0530, Vivek Pernamitta wrote:
> >>> Add qcom,maili-pcie as a compatible string that falls back to
> >>> qcom,hawi-pcie, as the Maili SoC reuses the Hawi PCIe controller IP.
> >>>
> >>> The Maili SoC is a derivative of Hawi and shares the same PCIe
> >>> controller architecture, allowing reuse of the existing Hawi PCIe
> >>> DT bindings.
> >>>
> >>> Signed-off-by: Vivek Pernamitta <vivek.pernamitta@oss.qualcomm.com>
> >>> ---
> >>> Dependencies:
> >>> - PCI: qcom: Add PCIe support for upcoming Hawi SoC
> >>> https://lore.kernel.org/all/20260625-hawi-pcie-v4-0-1a578603cd86@oss.qualcomm.com/
> >>
> >> Squash the patches then.
> >>
> >
> > But these are two independent SoC additions, isn't it?
>
> It's adding a single compatible, no? If a patch adding a single
> compatible cannot be done without multi-patchset dependencies making
> testing by tooling impossible, then probably that work should not be
> sent separately or even as separate patch. And I am not saying anything
> new because half a year ago (around Kaanapali and Glymur) I voiced
> strong opinion about that.
>
> But really, you do not need to add two compatibles in two separate patches.
>
Ok then. I'll squash this patch with Hawi patch which already got applied.
- Mani
--
மணிவண்ணன் சதாசிவம்
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH] phy: allwinner: sun4i-usb: disable the PHY2 PMU clock after SIDDQ setup
From: Xu Rao @ 2026-07-07 3:21 UTC (permalink / raw)
To: andre.przywara
Cc: jernej.skrabec, linux-arm-kernel, linux-kernel, linux-phy,
linux-sunxi, marco.crivellari, neil.armstrong, raoxu, samuel,
vkoul, wens
In-Reply-To: <d8042dd2-303e-4f24-b7bf-74ae080f4ae6@arm.com>
Hi Andre,
> > - clk_disable_unprepare(phy->clk2);
> > + clk_disable_unprepare(phy2->clk2);
>
> Interesting, this looks about right, and matches the comment above,
> noting that phy2->clk2 is just temporarily needed. I don't remember
> further details, only that this workaround was quite annoying and messy ;-)
>
> However I am wondering how this worked so far: This should sabotage the
> access to the local REG_HCI_PHY_CTL access in the next few lines ...
> Any idea why this worked nevertheless?
Yes, the current code is wrong, but the visible effect is limited.
In sun4i_usb_phy_init(), the auxiliary SIDDQ path explicitly enables
phy2->clk2 and then accesses phy2->pmu + REG_HCI_PHY_CTL. So the clock
that has to be disabled at the end of that block is phy2->clk2.
The current code disables phy->clk2 instead, which is a different object.
That does not undo the temporary phy2->clk2 enable.
The reason this has not shown up as an obvious functional failure is that
phy->clk2 is an optional PMU clock. On systems where the non-PHY2 PMU
clock is not provided, the wrong clk_disable_unprepare(phy->clk2) does
not actually disable a local PMU clock. In that case, the practical
visible issue is simply that the temporary phy2->clk2 enable is not
matched by a corresponding disable.
So the failure is not that the following local REG_HCI_PHY_CTL access is
known to break today. The concrete bug is that the auxiliary path enables
one clock and disables another one.
Thanks,
Xu Rao
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH 2/8] arm64: dts: qcom: sm8450: Remove unneeded reserved memory nodes
From: Esteban Urrutia @ 2026-07-07 3:19 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Konrad Dybcio, Bjorn Andersson, Michael Turquette, Stephen Boyd,
Brian Masney, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Rob Clark, Will Deacon, Robin Murphy,
Joerg Roedel (AMD), Vinod Koul, Neil Armstrong, linux-arm-msm,
linux-clk, linux-kernel, devicetree, iommu, linux-arm-kernel,
linux-phy
In-Reply-To: <4g6y4q2udludhwmoqc34afwj5svra4a4j5bmpqza7h3av2oov7@6k5r7a4meruk>
On 7/6/26 2:24 PM, Dmitry Baryshkov wrote:
> Please verify that you can actually access those areas (boot with
> memtest=1)
Thanks for the suggestion!
I actually did boot an SM8475 based device I'm mainlining (motorola-bronco)
with this parameter.
Deleted regions in DT are as follows:
/delete-node/ &mte_mem;
/delete-node/ &oem_vm_mem; (seems to not exist in SM8475?)
/delete-node/ &qheebsp_reserved_mem;
/delete-node/ &trust_ui_vm_mem;
/delete-node/ &trust_ui_vm_qrtr;
/delete-node/ &trust_ui_vm_swiotlb;
/delete-node/ &trust_ui_vm_vblk0_ring;
/delete-node/ &trusted_apps_ext_mem;
/delete-node/ &trusted_apps_mem;
Here are the results:
[ 0.000000] early_memtest: # of tests: 1
[ 0.000000] 0x00000000808f4000 - 0x0000000080900000 pattern 0000000000000000
[ 0.000000] 0x0000000085200000 - 0x0000000085700000 pattern 0000000000000000
[ 0.000000] 0x0000000087f00000 - 0x0000000088000000 pattern 0000000000000000
[ 0.000000] 0x000000008b91c000 - 0x000000008ba00000 pattern 0000000000000000
[ 0.000000] 0x000000009ff80000 - 0x00000000a6e00000 pattern 0000000000000000
[ 0.000000] 0x00000000a6e40000 - 0x00000000a6f00000 pattern 0000000000000000
[ 0.000000] 0x00000000a7000000 - 0x00000000a8000000 pattern 0000000000000000
[ 0.000000] 0x00000000ab350000 - 0x00000000b70cd000 pattern 0000000000000000
[ 0.000000] 0x00000000b70ebdd1 - 0x00000000b72ce000 pattern 0000000000000000
[ 0.000000] 0x00000000b7fff000 - 0x00000000b8000000 pattern 0000000000000000
[ 0.000000] 0x00000000b89e3400 - 0x00000000e0600000 pattern 0000000000000000
[ 0.000000] 0x00000000e0b00000 - 0x00000000e8800000 pattern 0000000000000000
[ 0.000000] 0x00000000f1400000 - 0x0000000100000000 pattern 0000000000000000
[ 0.000000] 0x0000000800000000 - 0x000000083af00000 pattern 0000000000000000
[ 0.000000] 0x000000083b500000 - 0x000000083d300000 pattern 0000000000000000
[ 0.000000] 0x0000000840000000 - 0x000000097f089d40 pattern 0000000000000000
[ 0.000000] 0x000000097f089d6f - 0x000000097f089d70 pattern 0000000000000000
[ 0.000000] 0x000000097f153ff8 - 0x000000097f154000 pattern 0000000000000000
The device didn't crash at boot, and everything works as expected.
For the record, the manufacturer has made changes in downstream device
trees that state TVM is not used, so it's expected that the device works
without QHEE and TVM regions.
Regards,
Esteban
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH 6/8] phy: qcom: qmp-combo: Drop qmp_v4_calibrate_dp_phy
From: Dmitry Baryshkov @ 2026-07-07 0:50 UTC (permalink / raw)
To: esteuwu
Cc: Bjorn Andersson, Michael Turquette, Stephen Boyd, Brian Masney,
Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Rob Clark, Will Deacon, Robin Murphy, Joerg Roedel (AMD),
Vinod Koul, Neil Armstrong, linux-arm-msm, linux-clk,
linux-kernel, devicetree, iommu, linux-arm-kernel, linux-phy
In-Reply-To: <20260622-sm8450-qol-v1-6-37e2ee8df9da@proton.me>
On Mon, Jun 22, 2026 at 08:54:27PM -0400, Esteban Urrutia via B4 Relay wrote:
> From: Esteban Urrutia <esteuwu@proton.me>
>
> There are no downstream device trees that specify five values in the
> qcom,aux-cfg1-settings array.
> Plus, after cross referencing both downstream device trees and entries
> which refer this function, only 0x13 is specified.
> Since 0x13 is written at initialization time, drop this function as a
> whole, and remove now unused variable assignations.
>
> Signed-off-by: Esteban Urrutia <esteuwu@proton.me>
> ---
> drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 31 -------------------------------
> 1 file changed, 31 deletions(-)
>
Indeed, 10nm and 14nm PHYs used several AUX CFG1 values for calibration.
PHYs starting from 7nm don't use this calibration.
Fixes: aff188feb5e1 ("phy: qcom-qmp: add support for sm8250-usb3-dp phy")
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
--
With best wishes
Dmitry
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v4] phy: Add USB3 PHY support to Google Tensor SoC USB PHY driver
From: sashiko-bot @ 2026-07-06 23:27 UTC (permalink / raw)
To: RD Babiera; +Cc: olteanv, linux-phy, vkoul, neil.armstrong
In-Reply-To: <20260706231633.700676-2-rdbabiera@google.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [High] Data race on `gphy->orientation` causes redundant hardware programming and a 2.5-second timeout.
Pre-existing issues:
- [High] TOCTOU race on `pm_runtime_suspended()` allows hardware access while suspended, causing a bus fault.
--
--- Patch [1]: [PATCH v4] phy: Add USB3 PHY support to Google Tensor SoC USB PHY driver ---
commit 1008e9b0470bdcef99e7408c0e43896673f90ba3
Author: RD Babiera <rdbabiera@google.com>
phy: Add USB3 PHY support to Google Tensor SoC USB PHY driver
This patch adds USB3 PHY support for the Google Tensor G5 USB PHY driver,
including the usb3_tca register, clock, and reset functionality. It also
refactors the probe sequence and implements USB3 phy_ops.
> diff --git a/drivers/phy/phy-google-usb.c b/drivers/phy/phy-google-usb.c
> index ab20bc20f19ea..19256588d50ec 100644
> --- a/drivers/phy/phy-google-usb.c
> +++ b/drivers/phy/phy-google-usb.c
[ ... ]
> @@ -92,6 +261,9 @@ static int google_usb_set_orientation(struct typec_switch_dev *sw,
>
> set_vbus_valid(gphy);
>
> + if (gphy->phy_state == COMBO_PHY_TCA_READY && orientation != TYPEC_ORIENTATION_NONE)
> + return program_tca_locked(gphy);
> +
[Severity: High]
This is a pre-existing issue, but does this function have a race condition
with power management that could cause a bus fault?
google_usb_set_orientation() checks pm_runtime_suspended(gphy->dev) without
holding a PM runtime reference. If the USB core releases its PM reference
immediately after this check, the device could suspend and power off the PHY.
The function would then acquire the lock and call set_vbus_valid() and
program_tca_locked() on unclocked and unpowered MMIO registers, resulting
in a synchronous external abort.
Would it be safer to use pm_runtime_get_sync() to ensure the hardware
remains active during these accesses?
> return 0;
> }
[ ... ]
> +static int google_usb3_phy_power_on(struct phy *_phy)
> +{
> + struct google_usb_phy_instance *inst = phy_get_drvdata(_phy);
> + struct google_usb_phy *gphy = inst->parent;
> + int ret;
> +
> + dev_dbg(gphy->dev, "power on usb3 phy\n");
> +
> + guard(mutex)(&gphy->phy_mutex);
> +
> + if (gphy->phy_state != COMBO_PHY_TCA_READY) {
> + /* Wait for PoR -> NC transitions*/
> + ret = wait_tca_xa_ack(gphy);
> + if (ret) {
> + dev_err(gphy->dev, "PoR->NC transition timeout");
> + return ret;
> + }
> + gphy->phy_state = COMBO_PHY_TCA_READY;
> + }
> +
> + if (gphy->orientation != TYPEC_ORIENTATION_NONE)
> + return program_tca_locked(gphy);
[Severity: High]
Can this lead to redundant TCA programming and a 2.5-second timeout?
Looking at google_usb_set_orientation(), gphy->orientation is updated before
the mutex is acquired:
google_usb_set_orientation() {
gphy->orientation = orientation;
if (pm_runtime_suspended(gphy->dev))
return 0;
guard(mutex)(&gphy->phy_mutex);
...
If a Type-C plug event (Thread A) sets gphy->orientation but is preempted
before taking the lock, and the USB core concurrently calls
google_usb3_phy_power_on() (Thread B), Thread B will see the new
orientation, program the TCA, and release the lock.
When Thread A resumes, it will acquire the mutex, see COMBO_PHY_TCA_READY,
and call program_tca_locked() a second time with the identical configuration.
Since writing the same configuration to the TCA hardware doesn't trigger a
new XA_ACT_EVT interrupt, wait_tca_xa_ack() will stall for its full timeout.
Should the orientation update be moved inside the lock?
> +
> + return 0;
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260706231633.700676-2-rdbabiera@google.com?part=1
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* [PATCH v4] phy: Add USB3 PHY support to Google Tensor SoC USB PHY driver
From: RD Babiera @ 2026-07-06 23:16 UTC (permalink / raw)
To: vkoul, peter.griffin, andre.draszik, tudor.ambarus, p.zabel,
neil.armstrong
Cc: badhri, linux-arm-kernel, linux-samsung-soc, linux-phy,
linux-kernel, RD Babiera
Add USB3 PHY support for the Google Tensor G5 USB PHY driver.
This patch adds functionality for the usb3_tca register, usb3 clock,
and usb3 reset as defined in google,lga-usb-phy.yaml. Kconfig now lists
USB SuperSpeed support.
Refactor the probe sequence to initialize the USB2 and USB3 PHYs, and then
initialize clocks and resets for both PHYs afterwards.
Refactor set_vbus_valid to reduce duplicated code.
Implement USB3 phy_ops for phy_init, phy_exit, and phy_power_on.
combo_phy_state enum is added to track PHY bringup state across
PHY API calls.
Signed-off-by: RD Babiera <rdbabiera@google.com>
---
Changes since v1:
* Removed mix of goto-based and scope-based cleanup from usb3 phy_init
* Removed unused usb3_core resource from probe
* Added combo_phy_state enum to interally track ComboPHY bringup state
to allow google_usb_set_orientation() to change TCA orientation.
* Modify Kconfig documentation to reflect SuperSpeed support
Changes since v2:
* google_usb3_phy_init now sets USBDP_TOP_CFG_REG_PMGT_REF_CLK_REQ_N
to false if phy_init fails elsewhere.
* google_usb3_phy_init errors are now handled via DEFINE_FREE structures.
This affects set_pmgt_ref_clk_req_n, clk_bulk_prepare_enable, and
reset_control_bulk_deassert.
* google_usb2_phy_init also handles undoing clk_bulk_prepare_enable via
DEFINE_FREE structure.
* google_usb3_phy_power_on allows program_tca_locked in the
COMBO_PHY_TCA_READY state. Waiting for PoR=>NC is only performed once.
* Note: there are checkpatch errors for the DEFINE_FREE macros resulting
in "ERROR: trailing statements should be on next line". Other cases of
DEFINE_FREE where the line limit would otherwise exceed 100 columns
have the indentation done the same way.
Changes since v3:
* set_pmgt_ref_clk_req_n(false) in google_usb3_phy_exit() comes after
reset assertion and clock disable to match phy_init() sequence.
* program_tca_locked in usb3 power_on() now requires a valid orientation
to match google_usb_set_orientation requirements.
---
drivers/phy/Kconfig | 2 +-
drivers/phy/phy-google-usb.c | 407 +++++++++++++++++++++++++++++++----
2 files changed, 371 insertions(+), 38 deletions(-)
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 19f3b7d12b7d..d2d401129af7 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -100,7 +100,7 @@ config PHY_GOOGLE_USB
the G5 generation (Laguna). This driver provides the PHY interfaces
to interact with the SNPS eUSB2 and USB 3.2/DisplayPort Combo PHY,
both of which are integrated with the DWC3 USB DRD controller.
- This driver currently supports USB high-speed.
+ This driver currently supports USB high-speed and SuperSpeed.
config USB_LGM_PHY
tristate "INTEL Lightning Mountain USB PHY Driver"
diff --git a/drivers/phy/phy-google-usb.c b/drivers/phy/phy-google-usb.c
index ab20bc20f19e..19256588d50e 100644
--- a/drivers/phy/phy-google-usb.c
+++ b/drivers/phy/phy-google-usb.c
@@ -20,6 +20,7 @@
#include <linux/reset.h>
#include <linux/usb/typec_mux.h>
+/* USB_CFG_CSR */
#define USBCS_USB2PHY_CFG19_OFFSET 0x0
#define USBCS_USB2PHY_CFG19_PHY_CFG_PLL_FB_DIV GENMASK(19, 8)
@@ -28,11 +29,41 @@
#define USBCS_USB2PHY_CFG21_REF_FREQ_SEL GENMASK(15, 13)
#define USBCS_USB2PHY_CFG21_PHY_TX_DIG_BYPASS_SEL BIT(19)
+/* USBDP_TOP */
#define USBCS_PHY_CFG1_OFFSET 0x28
+#define USBCS_PHY_CFG1_PHY0_MPLLA_SSC_EN BIT(1)
+#define USBCS_PHY_CFG1_PHY0_SRAM_BYPASS_MODE GENMASK(11, 10)
+#define SRAM_BYPASS_MODE_BYPASS_FIRMWARE BIT(0)
+#define SRAM_BYPASS_MODE_BYPASS_CONTEXT BIT(1)
#define USBCS_PHY_CFG1_SYS_VBUSVALID BIT(17)
+#define USBDP_TOP_CFG_REG_OFFSET 0x44
+#define USBDP_TOP_CFG_REG_PMGT_REF_CLK_REQ_N BIT(0)
+
+#define PHY_POWER_CONFIG_REG1_OFFSET 0x48
+#define PHY_POWER_CONFIG_REG1_PG_MODE_EN BIT(1)
+#define PHY_POWER_CONFIG_REG1_UPCS_PIPE_CONFIG GENMASK(31, 14)
+#define UPCS_PIPE_CONFIG_ISO_CPM BIT(5)
+#define UPCS_PIPE_CONFIG_PG_MODE_STATIC BIT(6)
+#define UPCS_PIPE_CONFIG_LANE_RESET_NO_PG_EXIT BIT(9)
+
+/* USB3_TCA */
+#define TCA_INTR_STS_OFFSET 0x8
+#define TCA_INTR_STS_XA_ACT_EVT BIT(0)
+#define TCA_TCPC_OFFSET 0x14
+#define TCA_TCPC_MUX_CONTROL GENMASK(2, 0)
+#define TCA_TCPC_MUX_CONTROL_USB_ONLY 0x1
+#define TCA_TCPC_CONNECTOR_ORIENTATION BIT(3)
+#define TCA_TCPC_VALID BIT(4)
+#define TCA_PSTATE_0_OFFSET 0x50
+#define TCA_PSTATE_0_UPCS_LANE0_PHYSTATUS BIT(8)
+
+#define GPHY_TCA_DELAY_US 10
+#define GPHY_TCA_TIMEOUT_US 2500000
+
enum google_usb_phy_id {
GOOGLE_USB2_PHY,
+ GOOGLE_USB3_PHY,
GOOGLE_USB_PHY_NUM,
};
@@ -46,34 +77,172 @@ struct google_usb_phy_instance {
struct reset_control_bulk_data *rsts;
};
+struct google_usb_phy_config {
+ const char * const *clk_names;
+ unsigned int num_clks;
+ const char * const *rst_names;
+ unsigned int num_rsts;
+};
+
+static const char * const u2phy_clk_names[] = {
+ "usb2",
+ "usb2_apb",
+};
+static const char * const u3phy_clk_names[] = {
+ "usb3"
+};
+static const char * const u2phy_rst_names[] = {
+ "usb2",
+ "usb2_apb",
+};
+static const char * const u3phy_rst_names[] = {
+ "usb3"
+};
+
+static const struct google_usb_phy_config phy_configs[GOOGLE_USB_PHY_NUM] = {
+ [GOOGLE_USB2_PHY] = {
+ .clk_names = u2phy_clk_names,
+ .num_clks = ARRAY_SIZE(u2phy_clk_names),
+ .rst_names = u2phy_rst_names,
+ .num_rsts = ARRAY_SIZE(u2phy_rst_names),
+ },
+ [GOOGLE_USB3_PHY] = {
+ .clk_names = u3phy_clk_names,
+ .num_clks = ARRAY_SIZE(u3phy_clk_names),
+ .rst_names = u3phy_rst_names,
+ .num_rsts = ARRAY_SIZE(u3phy_rst_names),
+ },
+};
+
+static inline void google_usb_phy_clk_disable(struct google_usb_phy_instance *inst)
+{
+ clk_bulk_disable_unprepare(inst->num_clks, inst->clks);
+}
+DEFINE_FREE(inst_clk_disable, struct google_usb_phy_instance *,
+ if (_T) google_usb_phy_clk_disable(_T))
+
+static inline void google_usb_phy_rst_disable(struct google_usb_phy_instance *inst)
+{
+ reset_control_bulk_assert(inst->num_rsts, inst->rsts);
+}
+DEFINE_FREE(inst_rst_disable, struct google_usb_phy_instance *,
+ if (_T) google_usb_phy_rst_disable(_T))
+
+/*
+ * combo_phy_state
+ * COMBO_PHY_IDLE: The ComboPHY has been torn down and USB3 has not completed
+ * bringup
+ * COMBO_PHY_INIT_DONE: The ComboPHY bringup sequence is complete.
+ * COMBO_PHY_TCA_READY: The PoR => NC transition is complete, and the TCA can be
+ * moved into USB.
+ */
+enum combo_phy_state {
+ COMBO_PHY_IDLE,
+ COMBO_PHY_INIT_DONE,
+ COMBO_PHY_TCA_READY,
+};
+
struct google_usb_phy {
struct device *dev;
struct regmap *usb_cfg_regmap;
unsigned int usb2_cfg_offset;
void __iomem *usbdp_top_base;
+ void __iomem *usb3_tca_base;
struct google_usb_phy_instance *insts;
/*
* Protect phy registers from concurrent access, specifically via
- * google_usb_set_orientation callback.
+ * google_usb_set_orientation callback. phy_mutex also protects
+ * concurrent access to phy_state.
*/
struct mutex phy_mutex;
struct typec_switch_dev *sw;
enum typec_orientation orientation;
+ enum combo_phy_state phy_state;
};
static void set_vbus_valid(struct google_usb_phy *gphy)
{
u32 reg;
- if (gphy->orientation == TYPEC_ORIENTATION_NONE) {
- reg = readl(gphy->usbdp_top_base + USBCS_PHY_CFG1_OFFSET);
+ reg = readl(gphy->usbdp_top_base + USBCS_PHY_CFG1_OFFSET);
+ if (gphy->orientation == TYPEC_ORIENTATION_NONE)
reg &= ~USBCS_PHY_CFG1_SYS_VBUSVALID;
- writel(reg, gphy->usbdp_top_base + USBCS_PHY_CFG1_OFFSET);
- } else {
- reg = readl(gphy->usbdp_top_base + USBCS_PHY_CFG1_OFFSET);
+ else
reg |= USBCS_PHY_CFG1_SYS_VBUSVALID;
- writel(reg, gphy->usbdp_top_base + USBCS_PHY_CFG1_OFFSET);
- }
+ writel(reg, gphy->usbdp_top_base + USBCS_PHY_CFG1_OFFSET);
+}
+
+static void set_sram_bypass(struct google_usb_phy *gphy, u32 bypass)
+{
+ u32 reg;
+
+ reg = readl(gphy->usbdp_top_base + USBCS_PHY_CFG1_OFFSET);
+ reg &= ~USBCS_PHY_CFG1_PHY0_SRAM_BYPASS_MODE;
+ reg |= FIELD_PREP(USBCS_PHY_CFG1_PHY0_SRAM_BYPASS_MODE, bypass);
+ writel(reg, gphy->usbdp_top_base + USBCS_PHY_CFG1_OFFSET);
+}
+
+static void set_pmgt_ref_clk_req_n(struct google_usb_phy *gphy, bool resume)
+{
+ u32 reg;
+
+ reg = readl(gphy->usbdp_top_base + USBDP_TOP_CFG_REG_OFFSET);
+ if (resume)
+ reg |= USBDP_TOP_CFG_REG_PMGT_REF_CLK_REQ_N;
+ else
+ reg &= ~USBDP_TOP_CFG_REG_PMGT_REF_CLK_REQ_N;
+ writel(reg, gphy->usbdp_top_base + USBDP_TOP_CFG_REG_OFFSET);
+}
+
+static inline void disable_pmgt_ref_clk_req_n(struct google_usb_phy *gphy)
+{
+ set_pmgt_ref_clk_req_n(gphy, false);
+}
+DEFINE_FREE(pmgt_ref_clk_req_n, struct google_usb_phy *, if (_T) disable_pmgt_ref_clk_req_n(_T))
+
+static int wait_tca_xa_ack(struct google_usb_phy *gphy)
+{
+ int ret;
+ u32 reg;
+
+ ret = readl_poll_timeout(gphy->usb3_tca_base + TCA_INTR_STS_OFFSET,
+ reg, !!(reg & TCA_INTR_STS_XA_ACT_EVT),
+ GPHY_TCA_DELAY_US, GPHY_TCA_TIMEOUT_US);
+ if (ret)
+ dev_err(gphy->dev, "tca xa_ack timeout, ret=%d", ret);
+
+ return ret;
+}
+
+static int program_tca_locked(struct google_usb_phy *gphy)
+ __must_hold(&gphy->phy_mutex)
+{
+ int ret;
+ u32 reg;
+
+ reg = readl(gphy->usb3_tca_base + TCA_INTR_STS_OFFSET);
+ writel(reg, gphy->usb3_tca_base + TCA_INTR_STS_OFFSET);
+
+ reg = readl(gphy->usb3_tca_base + TCA_TCPC_OFFSET);
+ reg &= ~TCA_TCPC_MUX_CONTROL;
+ reg |= FIELD_PREP(TCA_TCPC_MUX_CONTROL, TCA_TCPC_MUX_CONTROL_USB_ONLY);
+ if (gphy->orientation == TYPEC_ORIENTATION_REVERSE)
+ reg |= TCA_TCPC_CONNECTOR_ORIENTATION;
+ else
+ reg &= ~TCA_TCPC_CONNECTOR_ORIENTATION;
+ reg |= TCA_TCPC_VALID;
+ writel(reg, gphy->usb3_tca_base + TCA_TCPC_OFFSET);
+
+ ret = wait_tca_xa_ack(gphy);
+ dev_dbg(gphy->dev, "TCA switch %s, mux %lu, orientation %s",
+ ret ? "failed" : "success",
+ FIELD_GET(TCA_TCPC_MUX_CONTROL, reg),
+ FIELD_GET(TCA_TCPC_CONNECTOR_ORIENTATION, reg) ? "reverse" : "normal");
+
+ reg = readl(gphy->usb3_tca_base + TCA_INTR_STS_OFFSET);
+ writel(reg, gphy->usb3_tca_base + TCA_INTR_STS_OFFSET);
+
+ return ret;
}
static int google_usb_set_orientation(struct typec_switch_dev *sw,
@@ -92,6 +261,9 @@ static int google_usb_set_orientation(struct typec_switch_dev *sw,
set_vbus_valid(gphy);
+ if (gphy->phy_state == COMBO_PHY_TCA_READY && orientation != TYPEC_ORIENTATION_NONE)
+ return program_tca_locked(gphy);
+
return 0;
}
@@ -122,17 +294,18 @@ static int google_usb2_phy_init(struct phy *_phy)
ret = clk_bulk_prepare_enable(inst->num_clks, inst->clks);
if (ret)
return ret;
+ struct google_usb_phy_instance *clk_dev __free(inst_clk_disable) = inst;
ret = reset_control_bulk_deassert(inst->num_rsts, inst->rsts);
- if (ret) {
- clk_bulk_disable_unprepare(inst->num_clks, inst->clks);
+ if (ret)
return ret;
- }
regmap_read(gphy->usb_cfg_regmap, gphy->usb2_cfg_offset + USBCS_USB2PHY_CFG21_OFFSET, ®);
reg |= USBCS_USB2PHY_CFG21_PHY_ENABLE;
regmap_write(gphy->usb_cfg_regmap, gphy->usb2_cfg_offset + USBCS_USB2PHY_CFG21_OFFSET, reg);
+ retain_and_null_ptr(clk_dev);
+
return 0;
}
@@ -161,6 +334,119 @@ static const struct phy_ops google_usb2_phy_ops = {
.exit = google_usb2_phy_exit,
};
+static int google_usb3_phy_init(struct phy *_phy)
+{
+ struct google_usb_phy_instance *inst = phy_get_drvdata(_phy);
+ struct google_usb_phy *gphy = inst->parent;
+ int ret = 0;
+ u32 reg;
+
+ dev_dbg(gphy->dev, "initializing usb3 phy\n");
+
+ guard(mutex)(&gphy->phy_mutex);
+
+ if (gphy->phy_state != COMBO_PHY_IDLE) {
+ dev_warn(gphy->dev, "usb3 phy init called when combo phy state is not idle");
+ return 0;
+ }
+
+ reg = readl(gphy->usbdp_top_base + PHY_POWER_CONFIG_REG1_OFFSET);
+ reg |= PHY_POWER_CONFIG_REG1_PG_MODE_EN;
+ reg &= ~PHY_POWER_CONFIG_REG1_UPCS_PIPE_CONFIG;
+ reg |= FIELD_PREP(PHY_POWER_CONFIG_REG1_UPCS_PIPE_CONFIG,
+ (UPCS_PIPE_CONFIG_ISO_CPM |
+ UPCS_PIPE_CONFIG_PG_MODE_STATIC |
+ UPCS_PIPE_CONFIG_LANE_RESET_NO_PG_EXIT));
+ writel(reg, gphy->usbdp_top_base + PHY_POWER_CONFIG_REG1_OFFSET);
+
+ set_vbus_valid(gphy);
+
+ reg = readl(gphy->usbdp_top_base + USBCS_PHY_CFG1_OFFSET);
+ reg |= USBCS_PHY_CFG1_PHY0_MPLLA_SSC_EN;
+ writel(reg, gphy->usbdp_top_base + USBCS_PHY_CFG1_OFFSET);
+
+ set_sram_bypass(gphy, SRAM_BYPASS_MODE_BYPASS_FIRMWARE |
+ SRAM_BYPASS_MODE_BYPASS_CONTEXT);
+ set_pmgt_ref_clk_req_n(gphy, true);
+ struct google_usb_phy *pmgt_ref_clk_req_dev __free(pmgt_ref_clk_req_n) = gphy;
+
+ ret = clk_bulk_prepare_enable(inst->num_clks, inst->clks);
+ if (ret)
+ return ret;
+ struct google_usb_phy_instance *clk_dev __free(inst_clk_disable) = inst;
+
+ ret = reset_control_bulk_deassert(inst->num_rsts, inst->rsts);
+ if (ret)
+ return ret;
+ struct google_usb_phy_instance *rst_dev __free(inst_rst_disable) = inst;
+
+ ret = readl_poll_timeout(gphy->usb3_tca_base + TCA_PSTATE_0_OFFSET,
+ reg, !(reg & TCA_PSTATE_0_UPCS_LANE0_PHYSTATUS),
+ GPHY_TCA_DELAY_US, GPHY_TCA_TIMEOUT_US);
+ if (ret) {
+ dev_err(gphy->dev, "wait for lane0 phystatus timed out");
+ return ret;
+ }
+
+ gphy->phy_state = COMBO_PHY_INIT_DONE;
+
+ retain_and_null_ptr(rst_dev);
+ retain_and_null_ptr(clk_dev);
+ retain_and_null_ptr(pmgt_ref_clk_req_dev);
+
+ return 0;
+}
+
+static int google_usb3_phy_exit(struct phy *_phy)
+{
+ struct google_usb_phy_instance *inst = phy_get_drvdata(_phy);
+ struct google_usb_phy *gphy = inst->parent;
+
+ dev_dbg(gphy->dev, "exiting usb3 phy\n");
+
+ guard(mutex)(&gphy->phy_mutex);
+
+ reset_control_bulk_assert(inst->num_rsts, inst->rsts);
+ clk_bulk_disable_unprepare(inst->num_clks, inst->clks);
+ set_pmgt_ref_clk_req_n(gphy, false);
+
+ gphy->phy_state = COMBO_PHY_IDLE;
+
+ return 0;
+}
+
+static int google_usb3_phy_power_on(struct phy *_phy)
+{
+ struct google_usb_phy_instance *inst = phy_get_drvdata(_phy);
+ struct google_usb_phy *gphy = inst->parent;
+ int ret;
+
+ dev_dbg(gphy->dev, "power on usb3 phy\n");
+
+ guard(mutex)(&gphy->phy_mutex);
+
+ if (gphy->phy_state != COMBO_PHY_TCA_READY) {
+ /* Wait for PoR -> NC transitions*/
+ ret = wait_tca_xa_ack(gphy);
+ if (ret) {
+ dev_err(gphy->dev, "PoR->NC transition timeout");
+ return ret;
+ }
+ gphy->phy_state = COMBO_PHY_TCA_READY;
+ }
+
+ if (gphy->orientation != TYPEC_ORIENTATION_NONE)
+ return program_tca_locked(gphy);
+
+ return 0;
+}
+
+static const struct phy_ops google_usb3_phy_ops = {
+ .init = google_usb3_phy_init,
+ .exit = google_usb3_phy_exit,
+ .power_on = google_usb3_phy_power_on,
+};
+
static struct phy *google_usb_phy_xlate(struct device *dev,
const struct of_phandle_args *args)
{
@@ -173,14 +459,61 @@ static struct phy *google_usb_phy_xlate(struct device *dev,
return gphy->insts[args->args[0]].phy;
}
+static int google_usb_phy_parse_clocks(struct google_usb_phy *gphy)
+{
+ struct device *dev = gphy->dev;
+ int id, i, ret;
+
+ for (id = 0; id < GOOGLE_USB_PHY_NUM; id++) {
+ const struct google_usb_phy_config *cfg = &phy_configs[id];
+ struct google_usb_phy_instance *inst = &gphy->insts[id];
+
+ inst->num_clks = cfg->num_clks;
+ inst->clks = devm_kcalloc(dev, inst->num_clks, sizeof(*inst->clks), GFP_KERNEL);
+ if (!inst->clks)
+ return -ENOMEM;
+
+ for (i = 0; i < inst->num_clks; i++)
+ inst->clks[i].id = cfg->clk_names[i];
+
+ ret = devm_clk_bulk_get(dev, inst->num_clks, inst->clks);
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to get phy%d clks\n", id);
+ }
+
+ return 0;
+}
+
+static int google_usb_phy_parse_resets(struct google_usb_phy *gphy)
+{
+ struct device *dev = gphy->dev;
+ int id, i, ret;
+
+ for (id = 0; id < GOOGLE_USB_PHY_NUM; id++) {
+ const struct google_usb_phy_config *cfg = &phy_configs[id];
+ struct google_usb_phy_instance *inst = &gphy->insts[id];
+
+ inst->num_rsts = cfg->num_rsts;
+ inst->rsts = devm_kcalloc(dev, inst->num_rsts, sizeof(*inst->rsts), GFP_KERNEL);
+ if (!inst->rsts)
+ return -ENOMEM;
+
+ for (i = 0; i < inst->num_rsts; i++)
+ inst->rsts[i].id = cfg->rst_names[i];
+ ret = devm_reset_control_bulk_get_exclusive(dev, inst->num_rsts, inst->rsts);
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to get phy%d resets\n", id);
+ }
+
+ return 0;
+}
+
static int google_usb_phy_probe(struct platform_device *pdev)
{
struct typec_switch_desc sw_desc = { };
- struct google_usb_phy_instance *inst;
struct phy_provider *phy_provider;
struct device *dev = &pdev->dev;
struct google_usb_phy *gphy;
- struct phy *phy;
u32 args[1];
int ret;
@@ -212,39 +545,39 @@ static int google_usb_phy_probe(struct platform_device *pdev)
return dev_err_probe(dev, PTR_ERR(gphy->usbdp_top_base),
"invalid usbdp top\n");
+ gphy->usb3_tca_base = devm_platform_ioremap_resource_byname(pdev,
+ "usb3_tca");
+ if (IS_ERR(gphy->usb3_tca_base))
+ return dev_err_probe(dev, PTR_ERR(gphy->usb3_tca_base),
+ "invalid usb3 tca\n");
+
gphy->insts = devm_kcalloc(dev, GOOGLE_USB_PHY_NUM, sizeof(*gphy->insts), GFP_KERNEL);
if (!gphy->insts)
return -ENOMEM;
- inst = &gphy->insts[GOOGLE_USB2_PHY];
- inst->parent = gphy;
- inst->index = GOOGLE_USB2_PHY;
- phy = devm_phy_create(dev, NULL, &google_usb2_phy_ops);
- if (IS_ERR(phy))
- return dev_err_probe(dev, PTR_ERR(phy),
+ gphy->insts[GOOGLE_USB2_PHY].phy = devm_phy_create(dev, NULL, &google_usb2_phy_ops);
+ gphy->insts[GOOGLE_USB2_PHY].index = GOOGLE_USB2_PHY;
+ gphy->insts[GOOGLE_USB2_PHY].parent = gphy;
+ if (IS_ERR(gphy->insts[GOOGLE_USB2_PHY].phy))
+ return dev_err_probe(dev, PTR_ERR(gphy->insts[GOOGLE_USB2_PHY].phy),
"failed to create usb2 phy instance\n");
- inst->phy = phy;
- phy_set_drvdata(phy, inst);
+ phy_set_drvdata(gphy->insts[GOOGLE_USB2_PHY].phy, &gphy->insts[GOOGLE_USB2_PHY]);
- inst->num_clks = 2;
- inst->clks = devm_kcalloc(dev, inst->num_clks, sizeof(*inst->clks), GFP_KERNEL);
- if (!inst->clks)
- return -ENOMEM;
- inst->clks[0].id = "usb2";
- inst->clks[1].id = "usb2_apb";
- ret = devm_clk_bulk_get(dev, inst->num_clks, inst->clks);
+ gphy->insts[GOOGLE_USB3_PHY].phy = devm_phy_create(dev, NULL, &google_usb3_phy_ops);
+ gphy->insts[GOOGLE_USB3_PHY].index = GOOGLE_USB3_PHY;
+ gphy->insts[GOOGLE_USB3_PHY].parent = gphy;
+ if (IS_ERR(gphy->insts[GOOGLE_USB3_PHY].phy))
+ return dev_err_probe(dev, PTR_ERR(gphy->insts[GOOGLE_USB3_PHY].phy),
+ "failed to create usb3 phy instance\n");
+ phy_set_drvdata(gphy->insts[GOOGLE_USB3_PHY].phy, &gphy->insts[GOOGLE_USB3_PHY]);
+
+ ret = google_usb_phy_parse_clocks(gphy);
if (ret)
- return dev_err_probe(dev, ret, "failed to get u2 phy clks\n");
+ return ret;
- inst->num_rsts = 2;
- inst->rsts = devm_kcalloc(dev, inst->num_rsts, sizeof(*inst->rsts), GFP_KERNEL);
- if (!inst->rsts)
- return -ENOMEM;
- inst->rsts[0].id = "usb2";
- inst->rsts[1].id = "usb2_apb";
- ret = devm_reset_control_bulk_get_exclusive(dev, inst->num_rsts, inst->rsts);
+ ret = google_usb_phy_parse_resets(gphy);
if (ret)
- return dev_err_probe(dev, ret, "failed to get u2 phy resets\n");
+ return ret;
phy_provider = devm_of_phy_provider_register(dev, google_usb_phy_xlate);
if (IS_ERR(phy_provider))
base-commit: 2ace2e949979b82f82f12dd76d7c5a6145246ca3
--
2.55.0.rc2.803.g1fd1e6609c-goog
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* Re: [RFC PATCH v1 1/2] phy: rockchip: inno-usb2: keep peripheral-only OTG port active
From: sashiko-bot @ 2026-07-06 22:55 UTC (permalink / raw)
To: Andreas Zdziarstek
Cc: olteanv, vkoul, linux-phy, conor+dt, devicetree, robh,
neil.armstrong
In-Reply-To: <20260706223627.113814-2-andreas.zdziarstek@gmail.com>
Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
New issues:
- [Medium] The patch introduces a permanent power state leak for peripheral-only ports by skipping autonomous PHY power-offs but failing to skip autonomous power-ons.
Pre-existing issues:
- [Critical] The charger detection logic accesses uninitialized `chg_det` registers on SoCs like RK3036, inadvertently corrupting GRF register offset 0x0.
- [High] Delayed works (`otg_sm_work`, `chg_work`, `sm_work`) are not properly canceled on driver unbind, and mutual re-arming prevents clean teardown, leading to use-after-free.
- [High] The `cable` variable in `rockchip_usb2phy_otg_sm_work` is declared as `static`, which causes state corruption if multiple OTG ports exist.
- [High] Unprotected concurrent modifications to OTG and charger state variables (`rport->state`, `rphy->chg_state`, `rphy->chg_type`).
- [Medium] Concurrent lockless access to `rport->suspended` and hardware clock/register state between workqueues and the PHY core leads to a data race and clock leaks.
--
--- Patch [1]: [RFC PATCH v1 1/2] phy: rockchip: inno-usb2: keep peripheral-only OTG port active ---
commit dc6ab852562662efe7db14ed699e4f8c82b9f053
Author: Andreas Zdziarstek <andreas.zdziarstek@gmail.com>
phy: rockchip: inno-usb2: keep peripheral-only OTG port active
This commit updates the OTG state machine to skip autonomous power-offs
when the port is peripheral-only. This prevents the state machine from
suspending the port and killing the USB gadget when booting without an
attached cable.
> diff --git a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
> index 7d8a533f24aea..9b138b7aaeb89 100644
> --- a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
> +++ b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
> @@ -670,6 +670,23 @@ static const struct phy_ops rockchip_usb2phy_ops = {
[ ... ]
> +static void rockchip_usb2phy_sm_power_off(struct rockchip_usb2phy_port *rport)
> +{
> + if (rport->mode == USB_DR_MODE_PERIPHERAL)
> + return;
> +
> + rockchip_usb2phy_power_off(rport->phy);
> +}
[Severity: Medium]
The patch updates this function to skip powering off the PHY when the mode is
USB_DR_MODE_PERIPHERAL during a cable detach. However, further down in
rockchip_usb2phy_otg_sm_work(), the state machine still autonomously powers
ON the PHY when a cable is attached.
Does this asymmetrical behavior leave the PHY powered on indefinitely after
the first cable insertion, preventing system suspend?
> +
> static void rockchip_usb2phy_otg_sm_work(struct work_struct *work)
> {
> struct rockchip_usb2phy_port *rport =
> @@ -693,7 +710,7 @@ static void rockchip_usb2phy_otg_sm_work(struct work_struct *work)
> case OTG_STATE_UNDEFINED:
> rport->state = OTG_STATE_B_IDLE;
[Severity: High]
This is a pre-existing issue, but is it safe to locklessly modify shared state
variables like rport->state here?
This state machine can run in the delayed workqueue, and is concurrently
invoked synchronously by the threaded IRQ handler
rockchip_usb2phy_bvalid_irq() as well as the rockchip_chg_detect_work()
workqueue.
Could this lack of mutual exclusion allow interleaved state transitions on
multiple CPUs and corrupt the state machine?
> if (!vbus_attach)
> - rockchip_usb2phy_power_off(rport->phy);
> + rockchip_usb2phy_sm_power_off(rport);
> fallthrough;
> case OTG_STATE_B_IDLE:
> if (extcon_get_state(rphy->edev, EXTCON_USB_HOST) > 0) {
> @@ -719,7 +736,7 @@ static void rockchip_usb2phy_otg_sm_work(struct work_struct *work)
> break;
> case POWER_SUPPLY_TYPE_USB_DCP:
> dev_dbg(&rport->phy->dev, "dcp cable is connected\n");
> - rockchip_usb2phy_power_off(rport->phy);
> + rockchip_usb2phy_sm_power_off(rport);
> notify_charger = true;
> sch_work = true;
> cable = EXTCON_CHG_USB_DCP;
[Severity: High]
This is a pre-existing issue, but does the static declaration of this cable
variable cause state corruption if multiple OTG ports exist?
SoCs like RK3399 define multiple USB2 PHY instances with OTG ports. If VBUS
events overlap or alternate between ports, could the shared cable variable be
overwritten by the latest event, causing disconnected ports to report the
charger type of a different port to extcon?
> @@ -765,7 +782,7 @@ static void rockchip_usb2phy_otg_sm_work(struct work_struct *work)
[ ... ]
> if (extcon_get_state(rphy->edev, EXTCON_USB_HOST) == 0) {
> dev_dbg(&rport->phy->dev, "usb otg host disconnect\n");
> rport->state = OTG_STATE_B_IDLE;
> - rockchip_usb2phy_power_off(rport->phy);
> + rockchip_usb2phy_sm_power_off(rport);
> }
> break;
> default:
[Severity: Medium]
This isn't a bug introduced by this patch, but does invoking the internal
power management functions directly from the workqueue bypass the generic PHY
framework mutex?
The generic PHY subsystem calls phy_power_on() (protected by phy->mutex),
which invokes rockchip_usb2phy_power_on(). Concurrently, the driver's
workqueues invoke rockchip_usb2phy_power_on() directly without acquiring
any locks.
Since rockchip_usb2phy_power_on() performs a check-then-act on
rport->suspended:
rockchip_usb2phy_power_on() {
if (!rport->suspended)
return 0;
ret = clk_prepare_enable(rphy->clk480m);
...
}
Could both threads evaluate rport->suspended as true simultaneously, causing
double clock enables but only a single disable later?
[Severity: High]
This is a pre-existing issue, but how are these delayed works safely torn
down?
The works otg_sm_work and chg_work re-arm each other. In
rockchip_usb2phy_exit(), they are cancelled sequentially:
rockchip_usb2phy_exit() {
cancel_delayed_work_sync(&rport->otg_sm_work);
cancel_delayed_work_sync(&rport->chg_work);
...
}
If chg_work executes while otg_sm_work is being cancelled, can it re-arm
otg_sm_work after the cancellation finishes?
Additionally, since these works are initialized on devm-managed memory but
there is no driver .remove callback to cancel them, could pending works
execute on devm-freed memory upon driver unbind?
> @@ -838,7 +855,7 @@ static void rockchip_chg_detect_work(struct work_struct *work)
> switch (rphy->chg_state) {
> case USB_CHG_STATE_UNDEFINED:
> if (!rport->suspended && !vbus_attach)
> - rockchip_usb2phy_power_off(rport->phy);
> + rockchip_usb2phy_sm_power_off(rport);
> /* put the controller in non-driving mode */
> if (!vbus_attach)
> property_enable(rphy->grf, &rphy->phy_cfg->chg_det.opmode, false);
[Severity: Critical]
This is a pre-existing issue, but does this logic safely handle SoCs where
chg_det is not defined?
For SoCs like RK3036, the rk3036_phy_cfgs does not define chg_det. However,
rockchip_usb2phy_otg_port_init() unconditionally initializes charger detection.
When a cable is attached, rockchip_chg_detect_work() invokes
property_enable() on the zeroed chg_det fields:
property_enable() {
...
return regmap_write(base, reg->offset, val);
}
Because it lacks validation, would this silently write to reg->offset 0x0
with a write mask, corrupting the General Register Files (GRF) at offset 0x0?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260706223627.113814-1-andreas.zdziarstek@gmail.com?part=1
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v3] phy: Add USB3 PHY support to Google Tensor SoC USB PHY driver
From: RD Babiera @ 2026-07-06 22:43 UTC (permalink / raw)
To: sashiko-reviews, Badhri Jagan Sridharan
Cc: vkoul, neil.armstrong, olteanv, linux-phy
In-Reply-To: <20260701024002.DB4E71F000E9@smtp.kernel.org>
On Tue, Jun 30, 2026 at 7:40 PM <sashiko-bot@kernel.org> wrote:
> > +static int google_usb3_phy_exit(struct phy *_phy)
> > +{
> > + struct google_usb_phy_instance *inst = phy_get_drvdata(_phy);
> > + struct google_usb_phy *gphy = inst->parent;
> > +
> > + dev_dbg(gphy->dev, "exiting usb3 phy\n");
> > +
> > + guard(mutex)(&gphy->phy_mutex);
> > +
> > + set_pmgt_ref_clk_req_n(gphy, false);
> > + reset_control_bulk_assert(inst->num_rsts, inst->rsts);
> > + clk_bulk_disable_unprepare(inst->num_clks, inst->clks);
>
> [Severity: High]
> Is the manual teardown sequence here in the correct order?
>
> In google_usb3_phy_init(), resources are acquired in this order:
> 1) set_pmgt_ref_clk_req_n(true)
> 2) clock enable
> 3) reset deassert
>
> The __free blocks in the init function correctly establish the error cleanup
> sequence in reverse order (LIFO). However, here the upstream reference clock
> request is disabled before placing the hardware block into reset and
> disabling clocks.
>
> Could disabling the clock request before asserting reset freeze the IP in an
> invalid state and prevent subsequent wakeups?
I've verified both orderings of set_pmgt_ref_clk_req_n in phy_exit, will
change to be consistent with phy_init ordering though.
> > +
> > + return program_tca_locked(gphy);
>
> [Severity: High]
> Should this check if the Type-C port is actually connected before calling
> program_tca_locked()?
>
> If the port is disconnected (gphy->orientation == TYPEC_ORIENTATION_NONE),
> program_tca_locked() falls through its else branch, configuring the registers
> for TYPEC_ORIENTATION_NORMAL and setting TCA_TCPC_VALID.
>
> Since the Type-C switch callback google_usb_set_orientation() correctly skips
> TCA programming if the orientation is NONE, should a similar check be present
> here to prevent enabling the SuperSpeed data path for a phantom connection?
Will guard behind gphy->orientation in next version. If the port is
disconnected the
PHY will be treated as being in normal orientation, but given USB mode is being
programmed here and not NC, it's more correct to only program USB when
a port partner is actually connected.
Best,
RD
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox