* Re: [PATCH 2/3] dt-bindings: phy: rockchip-inno-csi-dphy: add rockchip,clk-lane-phase property
From: Gerald Loacker @ 2026-06-17 16:20 UTC (permalink / raw)
To: Conor Dooley
Cc: Vinod Koul, Neil Armstrong, Heiko Stuebner, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-phy, linux-arm-kernel,
linux-rockchip, linux-kernel, devicetree
In-Reply-To: <20260617-deviate-sulk-c57104ef939f@spud>
Hi Conor,
Am 17.06.2026 um 17:51 schrieb Conor Dooley:
> On Wed, Jun 17, 2026 at 02:23:14PM +0200, Gerald Loacker wrote:
>> Add support for the optional rockchip,clk-lane-phase device tree property
>> to allow board-specific tuning of the clock lane sampling phase for
>> improved signal integrity across supported data rates.
>>
>> Signed-off-by: Gerald Loacker <gerald.loacker@wolfvision.net>
>> ---
>> Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml | 7 +++++++
>> 1 file changed, 7 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml b/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml
>> index 03950b3cad08c..0d824d1511bc0 100644
>> --- a/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml
>> +++ b/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml
>> @@ -56,6 +56,13 @@ properties:
>> description:
>> Some additional phy settings are access through GRF regs.
>>
>> + rockchip,clk-lane-phase:
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + minimum: 0
>> + maximum: 7
>> + description:
>> + Clock lane sampling phase in 40 ps steps. The hardware default is 3.
>
> Can this instead become rockchip,clk-lane-phase-ps and be listed in the
> actual unit?
> With the -ps suffix, you can then drop the $ref.
> The default should be listed as "default: 3" (or default: 120)
>
> pw-bot: changes-requested
>
Thanks for the suggestion.
The phase setting is a hardware tap index (0–7) selecting a delay line
position. The datasheet mentions “about 40 ps” per step, but this is not
a calibrated or guaranteed value and may vary with PVT.
Because of that, I’d prefer to keep the property as an index and
document the approximate delay in the description:
Clock lane sampling phase selection (hardware tap index 0–7). Each step
corresponds to an approximately 40 ps delay as described in the hardware
specification.
This matches the hardware model more closely. Happy to adjust if needed.
>> +
>> required:
>> - compatible
>> - reg
>>
>> --
>> 2.34.1
>>
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH 2/3] dt-bindings: phy: rockchip-inno-csi-dphy: add rockchip,clk-lane-phase property
From: Conor Dooley @ 2026-06-17 15:51 UTC (permalink / raw)
To: Gerald Loacker
Cc: Vinod Koul, Neil Armstrong, Heiko Stuebner, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-phy, linux-arm-kernel,
linux-rockchip, linux-kernel, devicetree
In-Reply-To: <20260617-feature-mipi-csi-dphy-4k60-v1-2-4611ff00b0ff@wolfvision.net>
[-- Attachment #1.1: Type: text/plain, Size: 1474 bytes --]
On Wed, Jun 17, 2026 at 02:23:14PM +0200, Gerald Loacker wrote:
> Add support for the optional rockchip,clk-lane-phase device tree property
> to allow board-specific tuning of the clock lane sampling phase for
> improved signal integrity across supported data rates.
>
> Signed-off-by: Gerald Loacker <gerald.loacker@wolfvision.net>
> ---
> Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml b/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml
> index 03950b3cad08c..0d824d1511bc0 100644
> --- a/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml
> +++ b/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml
> @@ -56,6 +56,13 @@ properties:
> description:
> Some additional phy settings are access through GRF regs.
>
> + rockchip,clk-lane-phase:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + minimum: 0
> + maximum: 7
> + description:
> + Clock lane sampling phase in 40 ps steps. The hardware default is 3.
Can this instead become rockchip,clk-lane-phase-ps and be listed in the
actual unit?
With the -ps suffix, you can then drop the $ref.
The default should be listed as "default: 3" (or default: 120)
pw-bot: changes-requested
> +
> required:
> - compatible
> - reg
>
> --
> 2.34.1
>
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
[-- Attachment #2: Type: text/plain, Size: 112 bytes --]
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH 2/4] phy: qcom-qusb2: Fix SM6115 init sequence
From: Konrad Dybcio @ 2026-06-17 13:03 UTC (permalink / raw)
To: Iskren Chernev, Konrad Dybcio, Vinod Koul, Neil Armstrong,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Wesley Cheng,
Greg Kroah-Hartman, Bjorn Andersson
Cc: linux-arm-msm, linux-phy, devicetree, linux-kernel
In-Reply-To: <6fb6f805-aea1-47e7-bb7c-bc5ecb2201ae@iskren.info>
On 6/17/26 2:48 PM, Iskren Chernev wrote:
>
>
> On 6/15/26 1:44 PM, Konrad Dybcio wrote:
>> On 6/14/26 2:29 PM, Iskren Chernev wrote:
>>>
>>>
>>> On 6/10/26 3:04 PM, Konrad Dybcio wrote:
>>>> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>>>>
>>>> I don't know where the existing one came from, but it's apparently
>>>> wrong, according to both docs and a downstream DT [1]. Fix it up.
>>>
>>> They came from DTB extracted from a running billie2 (OnePlus Nord N100):
>>> [1] https://mainlining.dev/wp-content/uploads/2021/02/03_dtbdump_Qualcomm_Technologies_Inc._Bengal_SoC.dts
>>>
>>> The phone was bough early after launch, so it could have been wrong/updated later.
>>
>> Good to see you're still around!
>>
>> Looks like vendor tuning. I see that even the initial commit for
>> 6115 had the init sequence I posted. And the OnePlus sources have
>> what seems like a project-specific local copy of the DTSI:
>>
>> https://github.com/OnePlusOSS/android_kernel_oneplus_sm4250/blob/oneplus/SM4250_Q_10.0/arch/arm64/boot/dts/vendor/qcom/bengal-usb.dtsi#L145
>> https://github.com/OnePlusOSS/android_kernel_oneplus_sm4250/blob/oneplus/SM4250_Q_10.0/arch/arm64/boot/dts/vendor/20882/bengal-usb.dtsi#L148
>>
>> To support that, we should add a new property to override the TUNEx
>> registers - like e.g. qcom,hstx-trim-value that's already consumed
>
> My 2 cents - I never understood why init sequences are taboo in mainline
> and widely used in downstream. I guess if it doesn't change (but across
> what and who decides) it should be in code, but if it's "tuning"
> - whatever that means, possibly depends on other components around, it
> should be "configurable" via DT.
The PHY has some electrical characteristics of its own, and then atop
that are the characteristics of what's on the other end of it. Making
all parameters configurable (i.e. raw init sequence) leads to duplication
and pure blob seqeuences, whereas making everything constant leads to
polluting the driver (if every device-specific seq was to be in C files)
I think the current model of "override as necessary" is OK, especially
since we can use the upstream leverage to require describing what the
altered parameters actually change
>> Would you like to look into that, or should I take this?
>
> You can take it, the other option is to mark a TODO, and if somebody
> feels strongly about the binary value in a usb tune register s/he can
> take up the task.
Seems like OnePlus does.. actually, a number of vendors do. Sony
does/used to do some tuning there too.
> I just wanted to point out that the number didn't come from a random
> number generator (or AI).
I'm sorry if my language was too harsh. You used the best sources
you had and had no reason to believe they were not the expected values.
Konrad
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH 2/4] phy: qcom-qusb2: Fix SM6115 init sequence
From: Iskren Chernev @ 2026-06-17 12:48 UTC (permalink / raw)
To: Konrad Dybcio, Konrad Dybcio, Vinod Koul, Neil Armstrong,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Wesley Cheng,
Greg Kroah-Hartman, Bjorn Andersson
Cc: linux-arm-msm, linux-phy, devicetree, linux-kernel
In-Reply-To: <a51b6333-cd5a-4a38-b748-b6623c6a1078@oss.qualcomm.com>
On 6/15/26 1:44 PM, Konrad Dybcio wrote:
> On 6/14/26 2:29 PM, Iskren Chernev wrote:
>>
>>
>> On 6/10/26 3:04 PM, Konrad Dybcio wrote:
>>> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>>>
>>> I don't know where the existing one came from, but it's apparently
>>> wrong, according to both docs and a downstream DT [1]. Fix it up.
>>
>> They came from DTB extracted from a running billie2 (OnePlus Nord N100):
>> [1] https://mainlining.dev/wp-content/uploads/2021/02/03_dtbdump_Qualcomm_Technologies_Inc._Bengal_SoC.dts
>>
>> The phone was bough early after launch, so it could have been wrong/updated later.
>
> Good to see you're still around!
>
> Looks like vendor tuning. I see that even the initial commit for
> 6115 had the init sequence I posted. And the OnePlus sources have
> what seems like a project-specific local copy of the DTSI:
>
> https://github.com/OnePlusOSS/android_kernel_oneplus_sm4250/blob/oneplus/SM4250_Q_10.0/arch/arm64/boot/dts/vendor/qcom/bengal-usb.dtsi#L145
> https://github.com/OnePlusOSS/android_kernel_oneplus_sm4250/blob/oneplus/SM4250_Q_10.0/arch/arm64/boot/dts/vendor/20882/bengal-usb.dtsi#L148
>
> To support that, we should add a new property to override the TUNEx
> registers - like e.g. qcom,hstx-trim-value that's already consumed
My 2 cents - I never understood why init sequences are taboo in mainline
and widely used in downstream. I guess if it doesn't change (but across
what and who decides) it should be in code, but if it's "tuning"
- whatever that means, possibly depends on other components around, it
should be "configurable" via DT.
> Would you like to look into that, or should I take this?
You can take it, the other option is to mark a TODO, and if somebody
feels strongly about the binary value in a usb tune register s/he can
take up the task.
I just wanted to point out that the number didn't come from a random
number generator (or AI).
> Konrad
Iskren
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH 3/4] arm64: dts: qcom: Add Shikra CQM SoM platform
From: Dmitry Baryshkov @ 2026-06-17 12:48 UTC (permalink / raw)
To: Kamal Wadhwa
Cc: Rakesh Kota, linux-arm-msm, sashiko-reviews, Komal Bajaj, robh,
linux-phy, neil.armstrong, vkoul, olteanv, krzk+dt, conor+dt,
devicetree
In-Reply-To: <20260518114927.edxbzvklqatlntv2@hu-kamalw-hyd.qualcomm.com>
On Mon, 18 May 2026 at 14:49, Kamal Wadhwa
<kamal.wadhwa@oss.qualcomm.com> wrote:
>
> On Sun, May 17, 2026 at 08:18:15PM +0300, Dmitry Baryshkov wrote:
> > On Thu, May 14, 2026 at 04:09:18PM +0530, Kamal Wadhwa wrote:
> > > On Wed, May 13, 2026 at 06:14:20PM +0300, Dmitry Baryshkov wrote:
> > > > On 13/05/2026 17:29, Rakesh Kota wrote:
> > > > > On Wed, May 13, 2026 at 03:01:47PM +0300, Dmitry Baryshkov wrote:
> > > > > > On Wed, May 13, 2026 at 04:28:35AM +0000, sashiko-bot@kernel.org wrote:
> > > > > > > Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> > > > > > > - [High] The PMIC regulator definitions omit their required input supply dependencies (e.g., `vdd_s2-supply`, `vdd_l3-supply`), breaking the power hierarchy.
> > > > > > > - [Medium] The device tree inaccurately hardcodes the `compatible` string to a different PMIC model (`qcom,rpm-pm2250-regulators`) instead of explicitly identifying the actual hardware (PM4125).
> > > > > > > --
> > > > > > >
> > > > > > > commit 457abf6e7ac410430c866842e08e602ec3daaa51
> > > > > > > Author: Komal Bajaj <komal.bajaj@oss.qualcomm.com>
> > > > > > >
> > > > > > > arm64: dts: qcom: Add Shikra CQM SoM platform
> > > > > > >
> > > > > > > Add device tree include for the CQM variant of the Shikra System-on-Module.
> > > > > > > This variant integrates the Shikra SoC and PM4125 PMIC. The DTSI includes
> > > > > > > the common shikra.dtsi and adds PM4125 PMIC regulator definitions.
> > > > > > >
> > > > > > > > diff --git a/arch/arm64/boot/dts/qcom/shikra-cqm-som.dtsi b/arch/arm64/boot/dts/qcom/shikra-cqm-som.dtsi
> > > > > > > > new file mode 100644
> > > > > > > > index 0000000000000..401e71720519d
> > > > > > > > --- /dev/null
> > > > > > > > +++ b/arch/arm64/boot/dts/qcom/shikra-cqm-som.dtsi
> > > > > > > [ ... ]
> > > > > > > > +&rpm_requests {
> > > > > > > > + regulators {
> > > > > > > > + compatible = "qcom,rpm-pm2250-regulators";
> > > > > > >
> > > > > > > Since the commit message indicates this is a PM4125 PMIC, should the
> > > > > > > compatible string include a specific identifier for the actual hardware?
> > > > > > >
> > > > > > > Using a specific identifier like "qcom,rpm-pm4125-regulators" alongside
> > > > > > > the fallback allows the kernel to apply specific quirks if PM4125 errata
> > > > > > > are discovered later.
> > > > > >
> > > > > > This can be ignored. The compat is a leftover from the historically
> > > > > > incorrect naming.
> > > > > >
> > > > > > >
> > > > > > > > +
> > > > > > > > + pm4125_s2: s2 {
> > > > > > > > + regulator-min-microvolt = <1000000>;
> > > > > > > > + regulator-max-microvolt = <1200000>;
> > > > > > > > + };
> > > > > > >
> > > > > > > Do these regulators need to explicitly define their input supply dependencies
> > > > > > > such as vdd_s2-supply?
> > > > > > >
> > > > > > > Without these properties, the regulator framework might be unaware that the
> > > > > > > PMIC regulators draw power from upstream supplies.
> > > > > > >
> > > > > > > If the kernel dynamically manages the upstream supply and its reference count
> > > > > > > drops to zero, could it be disabled, causing an unexpected power loss for
> > > > > > > downstream components?
> > > > > >
> > > > > > And this is a correct comment. Please provide missing supplies.
> > > > > >
> > > > > As per the Qualcomm system design, the parent-child supply relationship
> > > > > is managed by the RPM firmware, not the Linux regulator framework. The
> > > > > RPM ensures the parent supply is never disabled until all subsystem
> > > > > votes are cleared.
> > > >
> > > > How is this different from other, previous platforms?
> > >
> > > This is not different. In the previous platforms too this is taken care from the
> > > RPM/RPMH firmware side, the only case where we may need explicit vote to parent
> > > is for non-rpmh/rpm regulator rails (like i2c based regulator pm8008), which
> > > may have a RPM/RPMH regulator as a parent.
> > >
> > > Even on those previous targets the parent rail of all RPM/RPMH regulators are
> > > internally voted by RPM/RPMH FW at proper voltage with required headroom
> > > calculated based on the active child rails. This was done for all the
> > > subsystems (including APPS) regulators.
> > >
> > > So no explicit handling from the APPS is required for parent supply.
> >
> > You are explaining the driver behaviour. But the question is about the
> > hardware description. If there is no difference, please add necessary
> > supplies back.
>
> I understand your concern about descibing the parent-child relation in the
> devicetree, and given that we have been almost always followed this for all
> the previous targets, it will expected of us to add them.
Yes.
>
> However, we want to avoid the unnecessary access to the parent from APPS.
Why? What is the reason? Do we want to do the same for all the
platforms? Only for Shikra? Something else?
> At the moment, I do not see a way to avoid that, if we add the parent
> regulators.
That depend on the answer to the previous question. In the end, we can
make the driver ignore the parents by removing them from the regulator
desc.
>
> @Bjorn, @Konrad - can you please also share your suggestion, how we can add
> parent-child desciption, but avoid accessing parent supply from APPS, as its
> Qualcomm's system design to handle this on RPM/RPMH firmware side (you may
> recall we had a verbal/offline discussion about same concern in context of
> RPMH regulators earlier).
That's why offline discussions are bad - you can't include other
participants in them.
--
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 3/4] arm64: dts: qcom: Add Shikra CQM SoM platform
From: Konrad Dybcio @ 2026-06-17 12:44 UTC (permalink / raw)
To: Kamal Wadhwa, Dmitry Baryshkov
Cc: Rakesh Kota, linux-arm-msm, sashiko-reviews, Komal Bajaj, robh,
linux-phy, neil.armstrong, vkoul, olteanv, krzk+dt, conor+dt,
devicetree
In-Reply-To: <20260518114927.edxbzvklqatlntv2@hu-kamalw-hyd.qualcomm.com>
On 5/18/26 1:49 PM, Kamal Wadhwa wrote:
> On Sun, May 17, 2026 at 08:18:15PM +0300, Dmitry Baryshkov wrote:
>> On Thu, May 14, 2026 at 04:09:18PM +0530, Kamal Wadhwa wrote:
>>> On Wed, May 13, 2026 at 06:14:20PM +0300, Dmitry Baryshkov wrote:
>>>> On 13/05/2026 17:29, Rakesh Kota wrote:
>>>>> On Wed, May 13, 2026 at 03:01:47PM +0300, Dmitry Baryshkov wrote:
>>>>>> On Wed, May 13, 2026 at 04:28:35AM +0000, sashiko-bot@kernel.org wrote:
>>>>>>> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>>>>>>> - [High] The PMIC regulator definitions omit their required input supply dependencies (e.g., `vdd_s2-supply`, `vdd_l3-supply`), breaking the power hierarchy.
>>>>>>> - [Medium] The device tree inaccurately hardcodes the `compatible` string to a different PMIC model (`qcom,rpm-pm2250-regulators`) instead of explicitly identifying the actual hardware (PM4125).
>>>>>>> --
>>>>>>>
>>>>>>> commit 457abf6e7ac410430c866842e08e602ec3daaa51
>>>>>>> Author: Komal Bajaj <komal.bajaj@oss.qualcomm.com>
>>>>>>>
>>>>>>> arm64: dts: qcom: Add Shikra CQM SoM platform
>>>>>>>
>>>>>>> Add device tree include for the CQM variant of the Shikra System-on-Module.
>>>>>>> This variant integrates the Shikra SoC and PM4125 PMIC. The DTSI includes
>>>>>>> the common shikra.dtsi and adds PM4125 PMIC regulator definitions.
>>>>>>>
>>>>>>>> diff --git a/arch/arm64/boot/dts/qcom/shikra-cqm-som.dtsi b/arch/arm64/boot/dts/qcom/shikra-cqm-som.dtsi
>>>>>>>> new file mode 100644
>>>>>>>> index 0000000000000..401e71720519d
>>>>>>>> --- /dev/null
>>>>>>>> +++ b/arch/arm64/boot/dts/qcom/shikra-cqm-som.dtsi
>>>>>>> [ ... ]
>>>>>>>> +&rpm_requests {
>>>>>>>> + regulators {
>>>>>>>> + compatible = "qcom,rpm-pm2250-regulators";
>>>>>>>
>>>>>>> Since the commit message indicates this is a PM4125 PMIC, should the
>>>>>>> compatible string include a specific identifier for the actual hardware?
>>>>>>>
>>>>>>> Using a specific identifier like "qcom,rpm-pm4125-regulators" alongside
>>>>>>> the fallback allows the kernel to apply specific quirks if PM4125 errata
>>>>>>> are discovered later.
>>>>>>
>>>>>> This can be ignored. The compat is a leftover from the historically
>>>>>> incorrect naming.
>>>>>>
>>>>>>>
>>>>>>>> +
>>>>>>>> + pm4125_s2: s2 {
>>>>>>>> + regulator-min-microvolt = <1000000>;
>>>>>>>> + regulator-max-microvolt = <1200000>;
>>>>>>>> + };
>>>>>>>
>>>>>>> Do these regulators need to explicitly define their input supply dependencies
>>>>>>> such as vdd_s2-supply?
>>>>>>>
>>>>>>> Without these properties, the regulator framework might be unaware that the
>>>>>>> PMIC regulators draw power from upstream supplies.
>>>>>>>
>>>>>>> If the kernel dynamically manages the upstream supply and its reference count
>>>>>>> drops to zero, could it be disabled, causing an unexpected power loss for
>>>>>>> downstream components?
>>>>>>
>>>>>> And this is a correct comment. Please provide missing supplies.
>>>>>>
>>>>> As per the Qualcomm system design, the parent-child supply relationship
>>>>> is managed by the RPM firmware, not the Linux regulator framework. The
>>>>> RPM ensures the parent supply is never disabled until all subsystem
>>>>> votes are cleared.
>>>>
>>>> How is this different from other, previous platforms?
>>>
>>> This is not different. In the previous platforms too this is taken care from the
>>> RPM/RPMH firmware side, the only case where we may need explicit vote to parent
>>> is for non-rpmh/rpm regulator rails (like i2c based regulator pm8008), which
>>> may have a RPM/RPMH regulator as a parent.
>>>
>>> Even on those previous targets the parent rail of all RPM/RPMH regulators are
>>> internally voted by RPM/RPMH FW at proper voltage with required headroom
>>> calculated based on the active child rails. This was done for all the
>>> subsystems (including APPS) regulators.
>>>
>>> So no explicit handling from the APPS is required for parent supply.
>>
>> You are explaining the driver behaviour. But the question is about the
>> hardware description. If there is no difference, please add necessary
>> supplies back.
>
> I understand your concern about descibing the parent-child relation in the
> devicetree, and given that we have been almost always followed this for all
> the previous targets, it will expected of us to add them.
>
> However, we want to avoid the unnecessary access to the parent from APPS.
> At the moment, I do not see a way to avoid that, if we add the parent
> regulators.
>
> @Bjorn, @Konrad - can you please also share your suggestion, how we can add
> parent-child desciption, but avoid accessing parent supply from APPS, as its
> Qualcomm's system design to handle this on RPM/RPMH firmware side (you may
> recall we had a verbal/offline discussion about same concern in context of
> RPMH regulators earlier).
Can we introduce some flag in idk.. struct regulation_constraints, make it
skip the parts we don't need and set it for all regulators registered in
qcom-rpm(h/smd).c?
Konrad
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH 3/3] phy: rockchip: phy-rockchip-inno-csidphy: add clock lane phase tuning
From: sashiko-bot @ 2026-06-17 12:32 UTC (permalink / raw)
To: Gerald Loacker
Cc: conor+dt, devicetree, neil.armstrong, olteanv, vkoul, linux-phy,
robh
In-Reply-To: <20260617-feature-mipi-csi-dphy-4k60-v1-3-4611ff00b0ff@wolfvision.net>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The macro `CSIDPHY_CLK_PHASE_DEFAULT` is defined but never used.
--
commit bfbb498bdd0c6aad9c80d6fe31b34f8a03261c76
Author: Gerald Loacker <gerald.loacker@wolfvision.net>
phy: rockchip: phy-rockchip-inno-csidphy: add clock lane phase tuning
This commit adds support for parsing and applying the optional
'rockchip,clk-lane-phase' device tree property. It allows for board-specific
tuning of the clock lane sampling phase. If the property is absent, the driver
relies on the hardware default without actively writing to the register.
> diff --git a/drivers/phy/rockchip/phy-rockchip-inno-csidphy.c b/drivers/phy/rockchip/phy-rockchip-inno-csidphy.c
> index 5281f8dea0ad3..3a15840e86cad 100644
> --- a/drivers/phy/rockchip/phy-rockchip-inno-csidphy.c
> +++ b/drivers/phy/rockchip/phy-rockchip-inno-csidphy.c
> @@ -69,6 +69,10 @@
> #define RK1808_CSIDPHY_CLK_CALIB_EN 0x168
> #define RK3568_CSIDPHY_CLK_CALIB_EN 0x168
>
> +#define CSIDPHY_LANE_CLK_3_PHASE 0x38
> +#define CSIDPHY_CLK_PHASE_MASK GENMASK(6, 4)
> +#define CSIDPHY_CLK_PHASE_DEFAULT 3
[Severity: Low]
This isn't a bug, but it looks like CSIDPHY_CLK_PHASE_DEFAULT is defined
here but never actually used in the driver code.
Since the fallback logic correctly relies on leaving the hardware at its
default state when the device tree property is missing (by initializing
clk_phase to -1), could this unused macro be removed to avoid keeping
dead code around?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260617-feature-mipi-csi-dphy-4k60-v1-0-4611ff00b0ff@wolfvision.net?part=3
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH 1/3] phy: rockchip: phy-rockchip-inno-csidphy: fix rk1808 hsfreq table
From: sashiko-bot @ 2026-06-17 12:31 UTC (permalink / raw)
To: Gerald Loacker
Cc: devicetree, neil.armstrong, conor+dt, vkoul, robh, olteanv,
linux-phy
In-Reply-To: <20260617-feature-mipi-csi-dphy-4k60-v1-1-4611ff00b0ff@wolfvision.net>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Valid low-frequency ranges mapping to configuration bit 0x00 are incorrectly rejected as errors.
--
commit 73e8683965255c344bec41b44d480c1cece33b57
Author: Gerald Loacker <gerald.loacker@wolfvision.net>
phy: rockchip: phy-rockchip-inno-csidphy: fix rk1808 hsfreq table
This commit extends the final entry in the rk1808 hsfreq table to exactly
2500 Mbps. This allows the RK3588, which reuses this array, to fully
support rates up to 2500 Mbps.
> diff --git a/drivers/phy/rockchip/phy-rockchip-inno-csidphy.c b/drivers/phy/rockchip/phy-rockchip-inno-csidphy.c
> index c79fb53d8ee5c..5281f8dea0ad3 100644
> --- a/drivers/phy/rockchip/phy-rockchip-inno-csidphy.c
> +++ b/drivers/phy/rockchip/phy-rockchip-inno-csidphy.c
> @@ -170,7 +170,7 @@ static const struct hsfreq_range rk1808_mipidphy_hsfreq_ranges[] = {
> { 299, 0x06}, { 399, 0x08}, { 499, 0x0b}, { 599, 0x0e},
> { 699, 0x10}, { 799, 0x12}, { 999, 0x16}, {1199, 0x1e},
> {1399, 0x23}, {1599, 0x2d}, {1799, 0x32}, {1999, 0x37},
> - {2199, 0x3c}, {2399, 0x41}, {2499, 0x46}
> + {2199, 0x3c}, {2399, 0x41}, {2500, 0x46}
> };
>
> static const struct hsfreq_range rk3326_mipidphy_hsfreq_ranges[] = {
[Severity: High]
This is a pre-existing issue, but while looking at these frequency range
tables it appears that valid low-frequency configurations are incorrectly
rejected as errors.
In rockchip_inno_csidphy_configure(), the code iterates through the
hsfreq_ranges to find the cfg_bit:
drivers/phy/rockchip/phy-rockchip-inno-csidphy.c:rockchip_inno_csidphy_configure() {
...
if (!hsfreq)
return -EINVAL;
...
}
The rk3326_mipidphy_hsfreq_ranges table defines an entry for frequencies up
to 109 Mbps with a valid cfg_bit of 0x00:
{ 109, 0x00}, { 149, 0x01}, { 199, 0x02}, { 249, 0x03},
If the data rate falls into this lowest range, hsfreq is correctly set
to 0x00, but the subsequent if (!hsfreq) check evaluates to true and
incorrectly returns -EINVAL instead of applying the configuration.
A similar issue appears in rk_dphy_configure() inside
drivers/phy/rockchip/phy-rockchip-dphy-rx0.c with tables like
rk3399_mipidphy_hsfreq_ranges which has { 89, 0x00 }.
Could this validation be modified to verify if a valid match was found
during iteration rather than checking if the resulting configuration bit
is non-zero?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260617-feature-mipi-csi-dphy-4k60-v1-0-4611ff00b0ff@wolfvision.net?part=1
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* [PATCH 3/3] phy: rockchip: phy-rockchip-inno-csidphy: add clock lane phase tuning
From: Gerald Loacker @ 2026-06-17 12:23 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Heiko Stuebner, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: linux-phy, linux-arm-kernel, linux-rockchip, linux-kernel,
devicetree, Gerald Loacker
In-Reply-To: <20260617-feature-mipi-csi-dphy-4k60-v1-0-4611ff00b0ff@wolfvision.net>
At high data rates like 4K60 (2500 Mbps), such as when using an
LT6911GXD bridge chip on an RK3588 board, fixed default timing parameters
can cause signal integrity issues and clock-data recovery failures.
The driver currently lacks a mechanism to adjust the clock lane sampling
phase to compensate for board-specific trace variations.
Resolve this by parsing and applying the optional 'rockchip,clk-lane-phase'
device tree property. This enables board-specific tuning of the clock
lane sampling phase in ~40 ps steps (range 0-7) to optimize link
stability. If the property is absent, the driver falls back to the
hardware default.
Signed-off-by: Gerald Loacker <gerald.loacker@wolfvision.net>
---
drivers/phy/rockchip/phy-rockchip-inno-csidphy.c | 25 ++++++++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/drivers/phy/rockchip/phy-rockchip-inno-csidphy.c b/drivers/phy/rockchip/phy-rockchip-inno-csidphy.c
index 5281f8dea0ad3..3a15840e86cad 100644
--- a/drivers/phy/rockchip/phy-rockchip-inno-csidphy.c
+++ b/drivers/phy/rockchip/phy-rockchip-inno-csidphy.c
@@ -69,6 +69,10 @@
#define RK1808_CSIDPHY_CLK_CALIB_EN 0x168
#define RK3568_CSIDPHY_CLK_CALIB_EN 0x168
+#define CSIDPHY_LANE_CLK_3_PHASE 0x38
+#define CSIDPHY_CLK_PHASE_MASK GENMASK(6, 4)
+#define CSIDPHY_CLK_PHASE_DEFAULT 3
+
#define RESETS_MAX 2
/*
@@ -151,6 +155,7 @@ struct rockchip_inno_csidphy {
const struct dphy_drv_data *drv_data;
struct phy_configure_opts_mipi_dphy config;
u8 hsfreq;
+ int clk_phase;
};
static inline void write_grf_reg(struct rockchip_inno_csidphy *priv,
@@ -304,6 +309,13 @@ static int rockchip_inno_csidphy_power_on(struct phy *phy)
rockchip_inno_csidphy_ths_settle(priv, priv->hsfreq,
CSIDPHY_LANE_THS_SETTLE(i));
+ if (priv->clk_phase >= 0) {
+ val = readl(priv->phy_base + CSIDPHY_LANE_CLK_3_PHASE);
+ val &= ~CSIDPHY_CLK_PHASE_MASK;
+ val |= FIELD_PREP(CSIDPHY_CLK_PHASE_MASK, priv->clk_phase);
+ writel(val, priv->phy_base + CSIDPHY_LANE_CLK_3_PHASE);
+ }
+
write_grf_reg(priv, GRF_DPHY_CSIPHY_CLKLANE_EN, 0x1);
write_grf_reg(priv, GRF_DPHY_CSIPHY_DATALANE_EN,
GENMASK(priv->config.lanes - 1, 0));
@@ -449,6 +461,7 @@ static int rockchip_inno_csidphy_probe(struct platform_device *pdev)
struct device *dev = &pdev->dev;
struct phy_provider *phy_provider;
struct phy *phy;
+ u32 phase;
int ret;
priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
@@ -464,6 +477,18 @@ static int rockchip_inno_csidphy_probe(struct platform_device *pdev)
return -ENODEV;
}
+ priv->clk_phase = -1;
+ if (device_property_read_u32(dev, "rockchip,clk-lane-phase",
+ &phase) == 0) {
+ if (phase >= BIT(3)) {
+ dev_err(dev,
+ "rockchip,clk-lane-phase %u out of range [0,7]\n",
+ phase);
+ return -EINVAL;
+ }
+ priv->clk_phase = phase;
+ }
+
priv->grf = syscon_regmap_lookup_by_phandle(dev->of_node,
"rockchip,grf");
if (IS_ERR(priv->grf)) {
--
2.34.1
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* [PATCH 2/3] dt-bindings: phy: rockchip-inno-csi-dphy: add rockchip,clk-lane-phase property
From: Gerald Loacker @ 2026-06-17 12:23 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Heiko Stuebner, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: linux-phy, linux-arm-kernel, linux-rockchip, linux-kernel,
devicetree, Gerald Loacker
In-Reply-To: <20260617-feature-mipi-csi-dphy-4k60-v1-0-4611ff00b0ff@wolfvision.net>
Add support for the optional rockchip,clk-lane-phase device tree property
to allow board-specific tuning of the clock lane sampling phase for
improved signal integrity across supported data rates.
Signed-off-by: Gerald Loacker <gerald.loacker@wolfvision.net>
---
Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml b/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml
index 03950b3cad08c..0d824d1511bc0 100644
--- a/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml
+++ b/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml
@@ -56,6 +56,13 @@ properties:
description:
Some additional phy settings are access through GRF regs.
+ rockchip,clk-lane-phase:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 0
+ maximum: 7
+ description:
+ Clock lane sampling phase in 40 ps steps. The hardware default is 3.
+
required:
- compatible
- reg
--
2.34.1
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* [PATCH 1/3] phy: rockchip: phy-rockchip-inno-csidphy: fix rk1808 hsfreq table
From: Gerald Loacker @ 2026-06-17 12:23 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Heiko Stuebner, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: linux-phy, linux-arm-kernel, linux-rockchip, linux-kernel,
devicetree, Gerald Loacker
In-Reply-To: <20260617-feature-mipi-csi-dphy-4k60-v1-0-4611ff00b0ff@wolfvision.net>
The rk1808 hsfreq table capped at 2499 Mbps, preventing a data rate of
exactly 2500 Mbps. Extend the final entry to 2500 Mbps to support this
rate.
This is essential for RK3588 reusing this array and fully supporting
rates up to 2500 Mbps.
Fixes: bd1f775d6027 ("phy/rockchip: add Innosilicon-based CSI dphy")
Signed-off-by: Gerald Loacker <gerald.loacker@wolfvision.net>
---
drivers/phy/rockchip/phy-rockchip-inno-csidphy.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-inno-csidphy.c b/drivers/phy/rockchip/phy-rockchip-inno-csidphy.c
index c79fb53d8ee5c..5281f8dea0ad3 100644
--- a/drivers/phy/rockchip/phy-rockchip-inno-csidphy.c
+++ b/drivers/phy/rockchip/phy-rockchip-inno-csidphy.c
@@ -170,7 +170,7 @@ static const struct hsfreq_range rk1808_mipidphy_hsfreq_ranges[] = {
{ 299, 0x06}, { 399, 0x08}, { 499, 0x0b}, { 599, 0x0e},
{ 699, 0x10}, { 799, 0x12}, { 999, 0x16}, {1199, 0x1e},
{1399, 0x23}, {1599, 0x2d}, {1799, 0x32}, {1999, 0x37},
- {2199, 0x3c}, {2399, 0x41}, {2499, 0x46}
+ {2199, 0x3c}, {2399, 0x41}, {2500, 0x46}
};
static const struct hsfreq_range rk3326_mipidphy_hsfreq_ranges[] = {
--
2.34.1
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* [PATCH 0/3] phy: rockchip: inno-csidphy: fix 2500 Mbps support and add clock lane phase tuning
From: Gerald Loacker @ 2026-06-17 12:23 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Heiko Stuebner, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: linux-phy, linux-arm-kernel, linux-rockchip, linux-kernel,
devicetree, Gerald Loacker
This series fixes and extends the Rockchip Innosilicon CSI D-PHY driver
to support data rates up to 2500 Mbps and adds optional board-specific
clock lane phase tuning for signal integrity.
Patch 1 fixes an off-by-one error in the rk1808 hsfreq range table:
the final entry was capped at 2499 Mbps, causing a rejection of the
maximum supported rate of 2500 Mbps.
Patches 2 and 3 add an optional rockchip,clk-lane-phase device tree
property that allows tuning the clock lane sampling phase in ~40 ps
steps to compensate for board-level signal integrity variations.
---
Gerald Loacker (3):
phy: rockchip: phy-rockchip-inno-csidphy: fix rk1808 hsfreq table
dt-bindings: phy: rockchip-inno-csi-dphy: add rockchip,clk-lane-phase property
phy: rockchip: phy-rockchip-inno-csidphy: add clock lane phase tuning
.../bindings/phy/rockchip-inno-csi-dphy.yaml | 7 ++++++
drivers/phy/rockchip/phy-rockchip-inno-csidphy.c | 27 +++++++++++++++++++++-
2 files changed, 33 insertions(+), 1 deletion(-)
---
base-commit: 8cd9520d35a6c38db6567e97dd93b1f11f185dc6
change-id: 20260617-feature-mipi-csi-dphy-4k60-9879c3d1fe4f
Best regards,
--
Gerald Loacker <gerald.loacker@wolfvision.net>
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH RFC v4 9/9] arm64: dts: qcom: glymur: Wire PCIe3a/3b to shared Gen5x8 PHY
From: Konrad Dybcio @ 2026-06-17 11:19 UTC (permalink / raw)
To: Qiang Yu, Vinod Koul, Neil Armstrong, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Philipp Zabel, Bjorn Andersson,
Konrad Dybcio
Cc: linux-arm-msm, linux-phy, devicetree, linux-kernel
In-Reply-To: <20260518-link_mode_0519-v4-9-269cd73cc5d1@oss.qualcomm.com>
On 5/19/26 7:47 AM, Qiang Yu wrote:
> Glymur PCIe3 uses a single shared Gen5x8 QMP PHY block. Model PCIe3a and
> PCIe3b as consumers of that shared PHY provider instead of separate PHY
> nodes.
>
> Update the DTS wiring to:
> - point GCC PCIe3A/3B pipe parents to the shared PHY clock outputs
> - add PCIe3a controller node and route PCIe3a/PCIe3b port phys to
> &pcie3_phy using two-cell PHY arguments
> - configure the shared PHY node with link-mode and dual pipe outputs
>
> Use QMP_PCIE_GLYMUR_MODE_* dt-binding macros for mode selection.
>
> Signed-off-by: Qiang Yu <qiang.yu@oss.qualcomm.com>
> ---
[...]
> + pcie3a: pci@1c10000 {
> + device_type = "pci";
> + compatible = "qcom,glymur-pcie", "qcom,pcie-x1e80100";
> + reg = <0x0 0x01c10000 0x0 0x3000>,
> + <0x0 0x70000000 0x0 0xf20>,
> + <0x0 0x70000f40 0x0 0xa8>,
> + <0x0 0x70001000 0x0 0x4000>,
> + <0x0 0x70100000 0x0 0x100000>,
> + <0x0 0x01c13000 0x0 0x1000>;
> + reg-names = "parf",
> + "dbi",
> + "elbi",
> + "atu",
> + "config",
> + "mhi";
> + #address-cells = <3>;
> + #size-cells = <2>;
> + ranges = <0x01000000 0x0 0x00000000 0x0 0x70200000 0x0 0x100000>,
> + <0x02000000 0x0 0x70000000 0x0 0x70300000 0x0 0x3d00000>,
> + <0x03000000 0x7 0x00000000 0x7 0x00000000 0x0 0x40000000>,
> + <0x43000000 0x70 0x00000000 0x70 0x00000000 0x10 0x00000000>;
> +
> + bus-range = <0 0xff>;
> +
> + dma-coherent;
> +
> + linux,pci-domain = <3>;
> + num-lanes = <8>;
Is it fine to keep num-lanes 8 here even for configurations with
bifurcated PHY?
I would assume so, given essentially this is a x8 host, whose 4
lanes may simply be effectively NC
Konrad
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v4 1/2] dt-bindings: phy: qcom,ipq8074-qmp-pcie: Document the ipq5210 QMP PCIe PHY
From: Krzysztof Kozlowski @ 2026-06-17 7:01 UTC (permalink / raw)
To: Varadarajan Narayanan
Cc: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, linux-arm-msm, linux-phy, devicetree, linux-kernel
In-Reply-To: <20260616-pcie-phy-v4-1-504677c3d727@oss.qualcomm.com>
On Tue, Jun 16, 2026 at 10:34:41AM +0530, Varadarajan Narayanan wrote:
> The ipq5210 has one dual lane and one single lane PCIe phy.
>
> The dual lane phy is similar to the dual lane phy present in ipq9574. Hence
> qcom,ipq5210-qmp-gen3x2-pcie-phy is documented with ipq9574's dual lane phy
> as fallback compatible.
>
> The single lane phy (qcom,ipq5210-qmp-gen3x1-pcie-phy) is documented as
> specific compatible as it uses a combination of its own initialization
> tables and some of the existing tables.
>
> Signed-off-by: Varadarajan Narayanan <varadarajan.narayanan@oss.qualcomm.com>
> ---
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
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 v4 2/2] phy: qcom-qmp-ufs: Add UFS PHY support on Hawi
From: Manivannan Sadhasivam @ 2026-06-17 6:00 UTC (permalink / raw)
To: palash.kambar
Cc: vkoul, neil.armstrong, robh, krzk+dt, conor+dt, alim.akhtar,
bvanassche, andersson, dmitry.baryshkov, abel.vesa, luca.weiss,
linux-arm-msm, linux-phy, devicetree, linux-kernel, linux-scsi,
nitin.rawat
In-Reply-To: <20260615091242.1617492-3-palash.kambar@oss.qualcomm.com>
On Mon, Jun 15, 2026 at 02:42:42PM +0530, palash.kambar@oss.qualcomm.com wrote:
> From: Palash Kambar <palash.kambar@oss.qualcomm.com>
>
> Add the init sequence tables and config for the UFS QMP phy found in
> the Hawi SoC.
>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> Signed-off-by: Palash Kambar <palash.kambar@oss.qualcomm.com>
Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
- Mani
> ---
> .../phy/qualcomm/phy-qcom-qmp-pcs-ufs-v7.h | 24 +++
> .../qualcomm/phy-qcom-qmp-qserdes-com-v8.h | 13 +-
> .../phy-qcom-qmp-qserdes-txrx-ufs-v8.h | 37 +++++
> drivers/phy/qualcomm/phy-qcom-qmp-ufs.c | 139 ++++++++++++++++++
> 4 files changed, 212 insertions(+), 1 deletion(-)
> create mode 100644 drivers/phy/qualcomm/phy-qcom-qmp-pcs-ufs-v7.h
> create mode 100644 drivers/phy/qualcomm/phy-qcom-qmp-qserdes-txrx-ufs-v8.h
>
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcs-ufs-v7.h b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-ufs-v7.h
> new file mode 100644
> index 000000000000..e80d3dd6a190
> --- /dev/null
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-ufs-v7.h
> @@ -0,0 +1,24 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2026, The Linux Foundation. All rights reserved.
> + */
> +
> +#ifndef QCOM_PHY_QMP_PCS_UFS_V7_H_
> +#define QCOM_PHY_QMP_PCS_UFS_V7_H_
> +
> +/* Only for QMP V7 PHY - UFS PCS registers */
> +#define QPHY_V7_PCS_UFS_PHY_START 0x000
> +#define QPHY_V7_PCS_UFS_POWER_DOWN_CONTROL 0x004
> +#define QPHY_V7_PCS_UFS_SW_RESET 0x008
> +#define QPHY_V7_PCS_UFS_PCS_CTRL1 0x01C
> +#define QPHY_V7_PCS_UFS_PLL_CNTL 0x028
> +#define QPHY_V7_PCS_UFS_TX_LARGE_AMP_DRV_LVL 0x02C
> +#define QPHY_V7_PCS_UFS_TX_HSGEAR_CAPABILITY 0x060
> +#define QPHY_V7_PCS_UFS_RX_HSGEAR_CAPABILITY 0x094
> +#define QPHY_V7_PCS_UFS_LINECFG_DISABLE 0x140
> +#define QPHY_V7_PCS_UFS_RX_SIGDET_CTRL2 0x150
> +#define QPHY_V7_PCS_UFS_READY_STATUS 0x16c
> +#define QPHY_V7_PCS_UFS_TX_MID_TERM_CTRL1 0x1b8
> +#define QPHY_V7_PCS_UFS_MULTI_LANE_CTRL1 0x1c0
> +
> +#endif
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-qserdes-com-v8.h b/drivers/phy/qualcomm/phy-qcom-qmp-qserdes-com-v8.h
> index d8ac4c4a2c31..d416113bcb3c 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-qserdes-com-v8.h
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-qserdes-com-v8.h
> @@ -1,6 +1,6 @@
> /* SPDX-License-Identifier: GPL-2.0 */
> /*
> - * Copyright (c) 2025 Qualcomm Innovation Center, Inc. All rights reserved.
> + * Copyright (c) 2026, The Linux Foundation. All rights reserved.
> */
>
> #ifndef QCOM_PHY_QMP_QSERDES_COM_V8_H_
> @@ -71,5 +71,16 @@
> #define QSERDES_V8_COM_ADDITIONAL_MISC 0x1b4
> #define QSERDES_V8_COM_CMN_STATUS 0x2c8
> #define QSERDES_V8_COM_C_READY_STATUS 0x2f0
> +#define QSERDES_V8_COM_PLL_IVCO_MODE1 0xf8
> +#define QSERDES_V8_COM_CMN_IETRIM 0xfc
> +#define QSERDES_V8_COM_CMN_IPTRIM 0x100
> +#define QSERDES_V8_COM_VCO_TUNE_CTRL 0x13c
> +#define QSERDES_V8_COM_ADAPTIVE_ANALOG_CONFIG 0x268
> +#define QSERDES_V8_COM_CP_CTRL_ADAPTIVE_MODE0 0x26c
> +#define QSERDES_V8_COM_PLL_RCCTRL_ADAPTIVE_MODE0 0x270
> +#define QSERDES_V8_COM_PLL_CCTRL_ADAPTIVE_MODE0 0x274
> +#define QSERDES_V8_COM_CP_CTRL_ADAPTIVE_MODE1 0x278
> +#define QSERDES_V8_COM_PLL_RCCTRL_ADAPTIVE_MODE1 0x27c
> +#define QSERDES_V8_COM_PLL_CCTRL_ADAPTIVE_MODE1 0x280
>
> #endif
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-qserdes-txrx-ufs-v8.h b/drivers/phy/qualcomm/phy-qcom-qmp-qserdes-txrx-ufs-v8.h
> new file mode 100644
> index 000000000000..5f923c3e64ec
> --- /dev/null
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-qserdes-txrx-ufs-v8.h
> @@ -0,0 +1,37 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2026, The Linux Foundation. All rights reserved.
> + */
> +
> +#ifndef QCOM_PHY_QMP_QSERDES_TXRX_UFS_V8_H_
> +#define QCOM_PHY_QMP_QSERDES_TXRX_UFS_V8_H_
> +
> +#define QSERDES_UFS_V8_TX_RES_CODE_LANE_OFFSET_TX (0x34)
> +#define QSERDES_UFS_V8_TX_RES_CODE_LANE_OFFSET_RX (0x38)
> +#define QSERDES_UFS_V8_TX_LANE_MODE_1 (0x80)
> +#define QSERDES_UFS_V8_RX_UCDR_FO_GAIN_RATE2 (0x1BC)
> +#define QSERDES_UFS_V8_RX_UCDR_FO_GAIN_RATE4 (0x1C4)
> +#define QSERDES_UFS_V8_RX_UCDR_SO_GAIN_RATE4 (0x1DC)
> +#define QSERDES_UFS_V8_RX_EQ_OFFSET_ADAPTOR_CNTRL1 (0x2C8)
> +#define QSERDES_UFS_V8_RX_UCDR_PI_CONTROLS (0x1E4)
> +#define QSERDES_UFS_V8_RX_OFFSET_ADAPTOR_CNTRL3 (0x2D0)
> +#define QSERDES_UFS_V8_RX_UCDR_FASTLOCK_COUNT_HIGH_RATE4 (0x120)
> +#define QSERDES_UFS_V8_RX_UCDR_FASTLOCK_FO_GAIN_RATE4 (0xD4)
> +#define QSERDES_UFS_V8_RX_UCDR_FASTLOCK_SO_GAIN_RATE4 (0xEC)
> +#define QSERDES_UFS_V8_RX_VGA_CAL_MAN_VAL (0x288)
> +#define QSERDES_UFS_V8_RX_EQU_ADAPTOR_CNTRL4 (0x2B0)
> +#define QSERDES_UFS_V8_RX_MODE_RATE_0_1_B4 (0x324)
> +#define QSERDES_UFS_V8_RX_MODE_RATE4_SA_B7 (0x3B4)
> +#define QSERDES_UFS_V8_RX_MODE_RATE4_SA_B9 (0x3BC)
> +#define QSERDES_UFS_V8_RX_MODE_RATE4_SB_B7 (0x3E0)
> +#define QSERDES_UFS_V8_RX_MODE_RATE4_SB_B9 (0x3E8)
> +#define QSERDES_UFS_V8_RX_MODE_RATE5_SA_B7 (0x40C)
> +#define QSERDES_UFS_V8_RX_MODE_RATE5_SA_B9 (0x414)
> +#define QSERDES_UFS_V8_RX_MODE_RATE5_SB_B7 (0x438)
> +#define QSERDES_UFS_V8_RX_MODE_RATE5_SB_B9 (0x440)
> +#define QSERDES_UFS_V8_RX_UCDR_SO_SATURATION (0xF4)
> +#define QSERDES_UFS_V8_RX_TERM_BW_CTRL0 (0x1AC)
> +#define QSERDES_UFS_V8_RX_DLL0_FTUNE_CTRL (0x498)
> +#define QSERDES_UFS_V8_RX_SIGDET_CAL_TRIM (0x4d0)
> +
> +#endif
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
> index 0f4ad24aa405..d4aca22c181e 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
> @@ -29,9 +29,11 @@
> #include "phy-qcom-qmp-pcs-ufs-v4.h"
> #include "phy-qcom-qmp-pcs-ufs-v5.h"
> #include "phy-qcom-qmp-pcs-ufs-v6.h"
> +#include "phy-qcom-qmp-pcs-ufs-v7.h"
>
> #include "phy-qcom-qmp-qserdes-txrx-ufs-v6.h"
> #include "phy-qcom-qmp-qserdes-txrx-ufs-v7.h"
> +#include "phy-qcom-qmp-qserdes-txrx-ufs-v8.h"
>
> /* QPHY_PCS_READY_STATUS bit */
> #define PCS_READY BIT(0)
> @@ -84,6 +86,13 @@ static const unsigned int ufsphy_v6_regs_layout[QPHY_LAYOUT_SIZE] = {
> [QPHY_PCS_POWER_DOWN_CONTROL] = QPHY_V6_PCS_UFS_POWER_DOWN_CONTROL,
> };
>
> +static const unsigned int ufsphy_v7_regs_layout[QPHY_LAYOUT_SIZE] = {
> + [QPHY_START_CTRL] = QPHY_V7_PCS_UFS_PHY_START,
> + [QPHY_PCS_READY_STATUS] = QPHY_V7_PCS_UFS_READY_STATUS,
> + [QPHY_SW_RESET] = QPHY_V7_PCS_UFS_SW_RESET,
> + [QPHY_PCS_POWER_DOWN_CONTROL] = QPHY_V7_PCS_UFS_POWER_DOWN_CONTROL,
> +};
> +
> static const struct qmp_phy_init_tbl milos_ufsphy_serdes[] = {
> QMP_PHY_INIT_CFG(QSERDES_V6_COM_SYSCLK_EN_SEL, 0xd9),
> QMP_PHY_INIT_CFG(QSERDES_V6_COM_CMN_CONFIG_1, 0x16),
> @@ -1307,6 +1316,11 @@ static const struct regulator_bulk_data sm8750_ufsphy_vreg_l[] = {
> { .supply = "vdda-pll", .init_load_uA = 18300 },
> };
>
> +static const struct regulator_bulk_data hawi_ufsphy_vreg_l[] = {
> + { .supply = "vdda-phy", .init_load_uA = 324000 },
> + { .supply = "vdda-pll", .init_load_uA = 27000 },
> +};
> +
> static const struct qmp_ufs_offsets qmp_ufs_offsets = {
> .serdes = 0,
> .pcs = 0xc00,
> @@ -1325,6 +1339,15 @@ static const struct qmp_ufs_offsets qmp_ufs_offsets_v6 = {
> .rx2 = 0x1a00,
> };
>
> +static const struct qmp_ufs_offsets qmp_ufs_offsets_v7 = {
> + .serdes = 0,
> + .pcs = 0x0400,
> + .tx = 0x2000,
> + .rx = 0x2000,
> + .tx2 = 0x3000,
> + .rx2 = 0x3000,
> +};
> +
> static const struct qmp_phy_cfg milos_ufsphy_cfg = {
> .lanes = 2,
>
> @@ -1845,6 +1868,119 @@ static const struct qmp_phy_cfg sm8750_ufsphy_cfg = {
>
> };
>
> +static const struct qmp_phy_init_tbl hawi_ufsphy_serdes[] = {
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_SYSCLK_EN_SEL, 0xd9),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_CMN_CONFIG_1, 0x16),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_HSCLK_SEL_1, 0x11),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_HSCLK_HS_SWITCH_SEL_1, 0x00),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_LOCK_CMP_EN, 0x01),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_LOCK_CMP_CFG, 0x60),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_PLL_IVCO, 0x1f),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_PLL_IVCO_MODE1, 0x1f),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_CMN_IETRIM, 0x07),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_CMN_IPTRIM, 0x20),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_VCO_TUNE_MAP, 0x04),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_VCO_TUNE_CTRL, 0x40),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_ADAPTIVE_ANALOG_CONFIG, 0x06),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_DEC_START_MODE0, 0x41),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_CP_CTRL_MODE0, 0x06),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_PLL_RCTRL_MODE0, 0x18),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_PLL_CCTRL_MODE0, 0x14),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_CP_CTRL_ADAPTIVE_MODE0, 0x06),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_PLL_RCCTRL_ADAPTIVE_MODE0, 0x18),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_PLL_CCTRL_ADAPTIVE_MODE0, 0x14),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_LOCK_CMP1_MODE0, 0x7f),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_LOCK_CMP2_MODE0, 0x06),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_BIN_VCOCAL_CMP_CODE1_MODE0, 0x92),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_BIN_VCOCAL_CMP_CODE2_MODE0, 0x1e),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_DEC_START_MODE1, 0x4c),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_CP_CTRL_MODE1, 0x06),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_PLL_RCTRL_MODE1, 0x18),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_PLL_CCTRL_MODE1, 0x14),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_CP_CTRL_ADAPTIVE_MODE1, 0x06),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_PLL_RCCTRL_ADAPTIVE_MODE1, 0x18),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_PLL_CCTRL_ADAPTIVE_MODE1, 0x14),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_LOCK_CMP1_MODE1, 0x99),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_LOCK_CMP2_MODE1, 0x07),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_BIN_VCOCAL_CMP_CODE1_MODE1, 0xbe),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_BIN_VCOCAL_CMP_CODE2_MODE1, 0x23),
> +};
> +
> +static const struct qmp_phy_init_tbl hawi_ufsphy_tx[] = {
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_TX_LANE_MODE_1, 0x0c),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_TX_RES_CODE_LANE_OFFSET_TX, 0x07),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_TX_RES_CODE_LANE_OFFSET_RX, 0x17),
> +};
> +
> +static const struct qmp_phy_init_tbl hawi_ufsphy_rx[] = {
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_UCDR_FO_GAIN_RATE2, 0x0c),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_UCDR_FO_GAIN_RATE4, 0x0c),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_UCDR_SO_GAIN_RATE4, 0x04),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_EQ_OFFSET_ADAPTOR_CNTRL1, 0x14),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_UCDR_PI_CONTROLS, 0x07),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_OFFSET_ADAPTOR_CNTRL3, 0x0e),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_UCDR_FASTLOCK_COUNT_HIGH_RATE4, 0x02),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_UCDR_FASTLOCK_FO_GAIN_RATE4, 0x1c),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_UCDR_FASTLOCK_SO_GAIN_RATE4, 0x06),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_VGA_CAL_MAN_VAL, 0x8e),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_EQU_ADAPTOR_CNTRL4, 0x0f),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_MODE_RATE_0_1_B4, 0xb8),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_MODE_RATE4_SA_B7, 0x66),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_MODE_RATE4_SA_B9, 0x1f),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_MODE_RATE4_SB_B7, 0x66),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_MODE_RATE4_SB_B9, 0x1f),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_MODE_RATE5_SA_B7, 0x66),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_MODE_RATE5_SA_B9, 0x1f),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_MODE_RATE5_SB_B7, 0x66),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_MODE_RATE5_SB_B9, 0x1f),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_UCDR_SO_SATURATION, 0x1f),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_TERM_BW_CTRL0, 0xfa),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_DLL0_FTUNE_CTRL, 0x30),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_SIGDET_CAL_TRIM, 0x77),
> +};
> +
> +static const struct qmp_phy_init_tbl hawi_ufsphy_pcs[] = {
> + QMP_PHY_INIT_CFG(QPHY_V7_PCS_UFS_TX_MID_TERM_CTRL1, 0x43),
> + QMP_PHY_INIT_CFG(QPHY_V7_PCS_UFS_PCS_CTRL1, 0x42),
> + QMP_PHY_INIT_CFG(QPHY_V7_PCS_UFS_TX_LARGE_AMP_DRV_LVL, 0x0f),
> + QMP_PHY_INIT_CFG(QPHY_V7_PCS_UFS_RX_SIGDET_CTRL2, 0x68),
> + QMP_PHY_INIT_CFG(QPHY_V7_PCS_UFS_MULTI_LANE_CTRL1, 0x02),
> +};
> +
> +static const struct qmp_phy_init_tbl hawi_ufsphy_g5_pcs[] = {
> + QMP_PHY_INIT_CFG(QPHY_V7_PCS_UFS_PLL_CNTL, 0x3b),
> + QMP_PHY_INIT_CFG(QPHY_V7_PCS_UFS_TX_HSGEAR_CAPABILITY, 0x05),
> + QMP_PHY_INIT_CFG(QPHY_V7_PCS_UFS_RX_HSGEAR_CAPABILITY, 0x05),
> +};
> +
> +static const struct qmp_phy_cfg hawi_ufsphy_cfg = {
> + .lanes = 2,
> +
> + .offsets = &qmp_ufs_offsets_v7,
> + .max_supported_gear = UFS_HS_G5,
> +
> + .tbls = {
> + .serdes = hawi_ufsphy_serdes,
> + .serdes_num = ARRAY_SIZE(hawi_ufsphy_serdes),
> + .tx = hawi_ufsphy_tx,
> + .tx_num = ARRAY_SIZE(hawi_ufsphy_tx),
> + .rx = hawi_ufsphy_rx,
> + .rx_num = ARRAY_SIZE(hawi_ufsphy_rx),
> + .pcs = hawi_ufsphy_pcs,
> + .pcs_num = ARRAY_SIZE(hawi_ufsphy_pcs),
> + },
> +
> + .tbls_hs_overlay[0] = {
> + .pcs = hawi_ufsphy_g5_pcs,
> + .pcs_num = ARRAY_SIZE(hawi_ufsphy_g5_pcs),
> + .max_gear = UFS_HS_G5,
> + },
> +
> + .vreg_list = hawi_ufsphy_vreg_l,
> + .num_vregs = ARRAY_SIZE(hawi_ufsphy_vreg_l),
> + .regs = ufsphy_v7_regs_layout,
> +};
> +
> static void qmp_ufs_serdes_init(struct qmp_ufs *qmp, const struct qmp_phy_cfg_tbls *tbls)
> {
> void __iomem *serdes = qmp->serdes;
> @@ -2259,6 +2395,9 @@ static int qmp_ufs_probe(struct platform_device *pdev)
>
> static const struct of_device_id qmp_ufs_of_match_table[] = {
> {
> + .compatible = "qcom,hawi-qmp-ufs-phy",
> + .data = &hawi_ufsphy_cfg,
> + }, {
> .compatible = "qcom,milos-qmp-ufs-phy",
> .data = &milos_ufsphy_cfg,
> }, {
> --
> 2.34.1
>
--
மணிவண்ணன் சதாசிவம்
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH] phy: sunplus: fix refcount leak in sp_uphy_init()
From: sashiko-bot @ 2026-06-16 14:33 UTC (permalink / raw)
To: Wentao Liang; +Cc: neil.armstrong, vkoul, olteanv, linux-phy
In-Reply-To: <20260616142234.1690457-1-vulab@iscas.ac.cn>
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
Pre-existing issues:
- [High] Kernel panic due to dereferencing an ERR_PTR if the optional nvmem cell fails to load.
- [High] Memory leak of the buffer returned by `nvmem_cell_read()`.
- [High] Unconditional out-of-bounds read on the nvmem buffer.
- [High] Imbalanced cleanup in error handling causing reference count underflows.
--
commit 98fd47236db60bb82bb9a431f603e28290ff2a8b
Author: Wentao Liang <vulab@iscas.ac.cn>
phy: sunplus: fix refcount leak in sp_uphy_init()
This commit changes the error path in sp_uphy_init to correctly jump to
err_reset instead of returning directly when update_disc_vol fails,
ensuring resources are cleaned up.
Cc: stable@vger.kernel.org
Fixes: 99d9ccd97385 ("phy: usb: Add USB2.0 phy driver for Sunplus SP7021")
> diff --git a/drivers/phy/sunplus/phy-sunplus-usb2.c b/drivers/phy/sunplus/phy-sunplus-usb2.c
> index 637a5fbae6d9a..c7bc3ac89ca9a 100644
> --- a/drivers/phy/sunplus/phy-sunplus-usb2.c
> +++ b/drivers/phy/sunplus/phy-sunplus-usb2.c
> @@ -129,7 +129,7 @@ static int sp_uphy_init(struct phy *phy)
> /* disconnect voltage */
> ret = update_disc_vol(usbphy);
[Severity: High]
This is a pre-existing issue, but could this call lead to a kernel panic
if the optional nvmem cell fails to load?
In drivers/phy/sunplus/phy-sunplus-usb2.c:update_disc_vol(), the return
value of nvmem_cell_get is checked, but if it is an error pointer other
than -EPROBE_DEFER, it is passed directly to nvmem_cell_read:
drivers/phy/sunplus/phy-sunplus-usb2.c:update_disc_vol() {
cell = nvmem_cell_get(usbphy->dev, disc_name);
if (IS_ERR_OR_NULL(cell)) {
if (PTR_ERR(cell) == -EPROBE_DEFER)
return -EPROBE_DEFER;
}
otp_v = nvmem_cell_read(cell, &otp_l);
...
}
Does this cause an invalid memory access when nvmem_cell_read attempts to
dereference the unhandled error pointer?
[Severity: High]
This is a pre-existing issue, but does update_disc_vol() leak the buffer
returned by nvmem_cell_read()?
otp_v = nvmem_cell_read(cell, &otp_l);
nvmem_cell_put(cell);
if (!IS_ERR(otp_v)) {
set = *(otp_v + 1);
set = (set << (sizeof(char) * 8)) | *otp_v;
...
Is the dynamically allocated memory for otp_v leaked when the function
returns?
[Severity: High]
This is a pre-existing issue, but is there a potential out-of-bounds read
on the nvmem buffer here?
if (!IS_ERR(otp_v)) {
set = *(otp_v + 1);
...
Are we unconditionally accessing the second byte of the buffer without
verifying that the returned length in otp_l is at least 2 bytes?
> if (ret < 0)
> - return ret;
> + goto err_reset;
[Severity: High]
This is a pre-existing issue, but do the error paths in sp_uphy_init()
incorrectly clean up resources that failed to initialize?
If clk_prepare_enable fails, the code jumps to err_clk and calls
clk_disable_unprepare on the same clock:
drivers/phy/sunplus/phy-sunplus-usb2.c:sp_uphy_init() {
ret = clk_prepare_enable(usbphy->phy_clk);
if (ret)
goto err_clk;
ret = reset_control_deassert(usbphy->rstc);
if (ret)
goto err_reset;
...
err_reset:
reset_control_assert(usbphy->rstc);
err_clk:
clk_disable_unprepare(usbphy->phy_clk);
return ret;
}
Would this decrement the clock framework's reference counts for a clock
that was never successfully enabled? Similarly, if reset_control_deassert
fails, it asserts the reset that failed to deassert.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260616142234.1690457-1-vulab@iscas.ac.cn?part=1
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* [PATCH] phy: sunplus: fix refcount leak in sp_uphy_init()
From: Wentao Liang @ 2026-06-16 14:22 UTC (permalink / raw)
To: vincent.sunplus, vkoul
Cc: neil.armstrong, linux-usb, linux-phy, linux-kernel, Wentao Liang,
stable
If update_disc_vol() fails after reset_control_deassert() succeeds,
the function returns without calling reset_control_assert() and
clk_disable_unprepare(). This leaks the reset control deassert count
and leaves the clock enabled.
Fix it by jumping to the err_reset label instead of returning
directly, so the cleanup path is executed.
Cc: stable@vger.kernel.org
Fixes: 99d9ccd97385 ("phy: usb: Add USB2.0 phy driver for Sunplus SP7021")
Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
---
drivers/phy/sunplus/phy-sunplus-usb2.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/phy/sunplus/phy-sunplus-usb2.c b/drivers/phy/sunplus/phy-sunplus-usb2.c
index 637a5fbae6d9..c7bc3ac89ca9 100644
--- a/drivers/phy/sunplus/phy-sunplus-usb2.c
+++ b/drivers/phy/sunplus/phy-sunplus-usb2.c
@@ -129,7 +129,7 @@ static int sp_uphy_init(struct phy *phy)
/* disconnect voltage */
ret = update_disc_vol(usbphy);
if (ret < 0)
- return ret;
+ goto err_reset;
/* board uphy 0 internal register modification for tid certification */
val = readl(usbphy->phy_regs + CONFIG9);
--
2.34.1
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* Re: [PATCH RFC v4 2/9] dt-bindings: phy: qcom-qmp: Add PHY selector and Glymur link-mode macros
From: Konrad Dybcio @ 2026-06-16 14:07 UTC (permalink / raw)
To: Qiang Yu, Vinod Koul, Neil Armstrong, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Philipp Zabel, Bjorn Andersson,
Konrad Dybcio
Cc: linux-arm-msm, linux-phy, devicetree, linux-kernel
In-Reply-To: <20260518-link_mode_0519-v4-2-269cd73cc5d1@oss.qualcomm.com>
On 5/19/26 7:47 AM, Qiang Yu wrote:
> Add two sets of constants to phy-qcom-qmp.h to support upcoming multiple
> link mode QMP PHY:
>
> - QMP_PHY_SELECTOR_0 / QMP_PHY_SELECTOR_1: generic logical PHY index
> values for QMP providers that expose multiple PHY instances under a
> single DT node (i.e. #phy-cells = <1>).
>
> - QMP_PCIE_GLYMUR_MODE_X8 / QMP_PCIE_GLYMUR_MODE_X4X4: link-mode
> values for the Glymur Gen5x8 PCIe PHY "qcom,link-mode" syscon property,
> selecting between the x8 single-PHY and x4+x4 dual-PHY topologies.
>
> Signed-off-by: Qiang Yu <qiang.yu@oss.qualcomm.com>
> ---
> include/dt-bindings/phy/phy-qcom-qmp.h | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/include/dt-bindings/phy/phy-qcom-qmp.h b/include/dt-bindings/phy/phy-qcom-qmp.h
> index 6b43ea9e0051..befa76f8392f 100644
> --- a/include/dt-bindings/phy/phy-qcom-qmp.h
> +++ b/include/dt-bindings/phy/phy-qcom-qmp.h
> @@ -21,4 +21,12 @@
> #define QMP_PCIE_PIPE_CLK 0
> #define QMP_PCIE_PHY_AUX_CLK 1
>
> +/* Generic QMP logical PHY selectors */
> +#define QMP_PHY_SELECTOR_0 0
> +#define QMP_PHY_SELECTOR_1 1
Is this for the second phy cell? FWIW I think it's fine to use raw
numbers as they're just indices (i.e. "nth bifurcated phy") anyway
Konrad
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH RFC v4 5/9] phy: qcom: qmp-pcie: Refactor pipe clk register and parse_dt helpers
From: Konrad Dybcio @ 2026-06-16 14:05 UTC (permalink / raw)
To: Qiang Yu, Dmitry Baryshkov
Cc: Manivannan Sadhasivam, Vinod Koul, Neil Armstrong, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Philipp Zabel, Bjorn Andersson,
Konrad Dybcio, linux-arm-msm, linux-phy, devicetree, linux-kernel
In-Reply-To: <ahk57lEoWQtkGsJt@hu-qianyu-lv.qualcomm.com>
On 5/29/26 9:02 AM, Qiang Yu wrote:
> On Thu, May 28, 2026 at 04:48:24PM +0300, Dmitry Baryshkov wrote:
>> On Fri, May 22, 2026 at 04:27:35PM +0530, Manivannan Sadhasivam wrote:
>>> On Wed, May 20, 2026 at 07:25:01PM +0300, Dmitry Baryshkov wrote:
>>>> On Mon, May 18, 2026 at 10:47:16PM -0700, Qiang Yu wrote:
>>>>> Some QMP PCIe PHY hardware blocks can be split into multiple sub-PHYs
>>>>> under a single DT node, each requiring its own pipe clock registration and
>>>>> DT resource mapping. The current helpers are tightly coupled to a single
>>>>> qmp_pcie instance, which prevents reuse across sub-PHY instances.
>>>>>
>>>>> Refactor __phy_pipe_clk_register() as a generic helper and reduce
>>>>> phy_pipe_clk_register() to a thin wrapper around it. Similarly, extract
>>>>> qmp_pcie_parse_dt_common() from qmp_pcie_parse_dt() to hold the register-
>>>>> mapping and pipe-clock setup that will be shared between sub-PHY instances,
>>>>> with pipe clock names parameterised per instance.
>>>>>
>>>>> This is a preparatory step before adding multi-PHY support. No functional
>>>>> change for existing platforms.
>>>>>
>>>>> Signed-off-by: Qiang Yu <qiang.yu@oss.qualcomm.com>
>>>>> ---
>>>>> drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 76 ++++++++++++++++++--------------
>>>>> 1 file changed, 44 insertions(+), 32 deletions(-)
>>>>
>>>> I'd suggest splitting the Glymur PHY to a separate driver. Otherwise we
>>>> end up having too many single-platform, single-device specifics which
>>>> don't apply to other platforms.
>>>>
>>>
>>> I don't think that's really needed. This shared PHY concept is going to be
>>> applicable to upcoming SoCs as well. And moreover, the split won't be clean
>>> either. We still need to reuse a lot of common logic in the 'phy-qcom-qmp-pcie'
>>> driver and may only end up keeping very minimal code in
>>> 'phy-qcom-qmp-pcie-glymur'.
>>
>> Then splitting makes even more sense. Let's not clutter the existing
>> driver with too many conditions and options.
>>
>>>
>>> If you are concerned about the file size of 'phy-qcom-qmp-pcie', then we should
>>> move the SoC specific 'cfg' structs into a separate file as that's what
>>> occupying majority of the space.
>>
>> No, it's really the 'shared' part.
>>
>
> To confirm, are you okay with some code duplication between the new
> Glymur-specific driver and phy-qcom-qmp-pcie driver.
That's a necessity, to some degree. See e.g. qmp-combo and qmp-usbc
Konrad
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH RFC v4 1/9] dt-bindings: phy: qcom,sc8280xp-qmp-pcie-phy: Add glymur-qmp-gen5x8-pcie-phy compatible
From: Konrad Dybcio @ 2026-06-16 14:03 UTC (permalink / raw)
To: Qiang Yu, Vinod Koul, Neil Armstrong, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Philipp Zabel, Bjorn Andersson,
Konrad Dybcio
Cc: linux-arm-msm, linux-phy, devicetree, linux-kernel
In-Reply-To: <20260518-link_mode_0519-v4-1-269cd73cc5d1@oss.qualcomm.com>
On 5/19/26 7:47 AM, Qiang Yu wrote:
> The Glymur SoC uses a single PCIe Gen5 PHY hardware block for the
> PCIe3a/PCIe3b controllers. This block supports two link modes:
>
> 1. x4+x4: two 4-lane PHY instances are exposed
> 2. x8: one 8-lane PHY instance is exposed
>
> Add qcom,glymur-qmp-gen5x8-pcie-phy as a multi-mode PHY compatible and
> document the new link-mode property, which selects the active link mode
> via a TCSR syscon register.
>
> Document the required clocks, resets, and power-domains for both PHY
> instances active in x8 mode. Use #phy-cells = <1> for this compatible,
> where the cell value is the PHY index within the active link mode.
>
> Signed-off-by: Qiang Yu <qiang.yu@oss.qualcomm.com>
> ---
[...]
> @@ -68,20 +69,29 @@ properties:
> - const: ref
> - enum: [rchng, refgen]
> - const: pipe
> - - const: pipediv2
> + - enum: [pipediv2, phy_b_aux]
I'm surprised to learn 3A doesnm'doesn't have a PIPE_DIV2 clk.. it does have
a non-div2 one though.
Seems like it's specifically not the case on Hamoa and Makena, so perhaps
it's better for maintainability if the Glymur list was separate
Konrad
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* [PATCH v3 1/2] dt-bindings: phy: qcom,usb-hs-phy: add qcom,hs-drv-slope
From: Herman van Hazendonk @ 2026-06-16 13:26 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, Philipp Zabel, Nathan Chancellor,
Nick Desaulniers, Bill Wendling, Justin Stitt
Cc: linux-arm-msm, linux-phy, devicetree, linux-kernel, llvm,
Herman van Hazendonk, konrad.dybcio, dmitry.baryshkov
In-Reply-To: <20260616-submit-phy-usb-hs-vendor-init-seq-v3-0-7d21fb1d1484@herrie.org>
The MSM8x60 / APQ8060 PHY needs three vendor ULPI register tweaks for
stable USB operation: pre-emphasis level, CDR auto-reset and SE1
gating in registers 0x32 and 0x36. A survey of MSM8x60-class
downstream board files (Qualcomm SURF/FFA/Fluid/Dragon, Samsung
Galaxy S2 family, Sony Xperia, HTC and HP TouchPad) shows that those
three values are identical across every reference board and can be
hardcoded in the driver behind the existing
qcom,usb-hs-phy-msm8660 compatible.
The only board-specific value is the 4-bit HS driver slope in bits
[3:0] of register 0x32:
HP TouchPad 5
HTC MSM8660 ports 1
Qualcomm / Samsung / Sony reference boards 0 (silicon default)
Add a qcom,hs-drv-slope property carrying that 4-bit value, valid
only on the qcom,usb-hs-phy-msm8660 variant. When the property is
absent the driver leaves the silicon default in place, matching the
behaviour of the Qualcomm reference platform.
No public Qualcomm documentation describes how the 4-bit value maps
to an actual slew rate, V/ns or %; the bits are an opaque hardware
control whose meaning only Qualcomm knows. The legal range (0..15)
comes from the field width in the downstream
arch/arm/mach-msm/include/mach/msm_hsusb_hw.h
(ULPI_HSDRVSLOPE_MASK == 0x0F). Boards must therefore copy the
value from their downstream/vendor kernel; this is a measured /
tuned-per-layout knob, not a derived one.
Assisted-by: Claude:claude-opus-4-7 dt_binding_check checkpatch
Assisted-by: Sashiko:claude-opus-4-7
Signed-off-by: Herman van Hazendonk <github.com@herrie.org>
---
.../devicetree/bindings/phy/qcom,usb-hs-phy.yaml | 89 +++++++++++++++-------
1 file changed, 63 insertions(+), 26 deletions(-)
diff --git a/Documentation/devicetree/bindings/phy/qcom,usb-hs-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,usb-hs-phy.yaml
index e03b516c698c..e605f5683f7d 100644
--- a/Documentation/devicetree/bindings/phy/qcom,usb-hs-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/qcom,usb-hs-phy.yaml
@@ -9,32 +9,43 @@ title: Qualcomm's USB HS PHY
maintainers:
- Bjorn Andersson <bjorn.andersson@linaro.org>
-if:
- properties:
- compatible:
- contains:
- enum:
- - qcom,usb-hs-phy-apq8064
- - qcom,usb-hs-phy-msm8660
- - qcom,usb-hs-phy-msm8960
-then:
- properties:
- resets:
- maxItems: 1
-
- reset-names:
- const: por
-
-else:
- properties:
- resets:
- minItems: 2
- maxItems: 2
-
- reset-names:
- items:
- - const: phy
- - const: por
+allOf:
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - qcom,usb-hs-phy-apq8064
+ - qcom,usb-hs-phy-msm8660
+ - qcom,usb-hs-phy-msm8960
+ then:
+ properties:
+ resets:
+ maxItems: 1
+
+ reset-names:
+ const: por
+
+ else:
+ properties:
+ resets:
+ minItems: 2
+ maxItems: 2
+
+ reset-names:
+ items:
+ - const: phy
+ - const: por
+
+ - if:
+ not:
+ properties:
+ compatible:
+ contains:
+ const: qcom,usb-hs-phy-msm8660
+ then:
+ properties:
+ qcom,hs-drv-slope: false
properties:
compatible:
@@ -85,6 +96,15 @@ properties:
the address is offset from the ULPI_EXT_VENDOR_SPECIFIC address
- description: value
+ qcom,hs-drv-slope:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description: >
+ 4-bit HS driver slope written to bits [3:0] of ULPI vendor
+ register 0x32. Board-specific tuning value; absent means
+ leave silicon default. Only valid on qcom,usb-hs-phy-msm8660.
+ minimum: 0
+ maximum: 15
+
required:
- clocks
- clock-names
@@ -114,3 +134,20 @@ examples:
};
};
};
+
+ - |
+ usb-controller {
+ #reset-cells = <1>;
+
+ ulpi {
+ phy {
+ compatible = "qcom,usb-hs-phy-msm8660", "qcom,usb-hs-phy";
+ #phy-cells = <0>;
+ clocks = <&clk 0>, <&clk 1>;
+ clock-names = "ref", "sleep";
+ resets = <&otg 0>;
+ reset-names = "por";
+ qcom,hs-drv-slope = <5>;
+ };
+ };
+ };
--
2.43.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* [PATCH v3 2/2] phy: qcom: usb-hs: program MSM8x60 vendor ULPI registers on power-on
From: Herman van Hazendonk @ 2026-06-16 13:26 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, Philipp Zabel, Nathan Chancellor,
Nick Desaulniers, Bill Wendling, Justin Stitt
Cc: linux-arm-msm, linux-phy, devicetree, linux-kernel, llvm,
Herman van Hazendonk, konrad.dybcio, dmitry.baryshkov
In-Reply-To: <20260616-submit-phy-usb-hs-vendor-init-seq-v3-0-7d21fb1d1484@herrie.org>
The MSM8x60-class PHY needs three vendor-register tweaks for stable
USB operation, which the legacy msm_otg driver used to drive from
board platform data. A survey of every MSM8x60-class downstream tree
(Qualcomm SURF/FFA/Fluid/Dragon, Samsung Galaxy S2 family, Sony
Xperia, HTC MSM8660 ports and HP TouchPad) shows that two of the
three settings are identical across every board:
- reg 0x32 [5:4] = 11b: pre-emphasis level set to 20%
- reg 0x36 bit 1 = 1, bit 2 = 1: CDR auto-reset and SE1 gating
disabled (the legacy driver inverts these bits, so setting them
disables the function)
Hardcode those two unconditionally behind the existing
qcom,usb-hs-phy-msm8660 compatible. The bit-level documentation
comes from the Code Aurora downstream header
arch/arm/mach-msm/include/mach/msm_hsusb_hw.h, which Samsung and HP
both shipped byte-for-byte identical.
The third setting -- reg 0x32 [3:0] HS driver slope -- is genuinely
board-specific (HP TouchPad uses 5, HTC MSM8660 ports use 1, every
Qualcomm/Samsung/Sony reference board leaves the silicon default of
0) and is consumed from the new qcom,hs-drv-slope DT property. When
the property is absent the silicon default is preserved.
No public Qualcomm documentation describes how the 4-bit slope value
maps to an actual slew rate, V/ns or %; the field is an opaque
hardware control whose semantics only Qualcomm knows. Boards must
copy the value from their vendor / downstream kernel -- this is a
measured / tuned-per-layout knob, not a derived one. We program the
4 bits verbatim and trust the silicon to do the right thing.
The writes live behind a runtime flag that only matches
"qcom,usb-hs-phy-msm8660" so the existing MSM8226/8916/8960/8974
consumers are untouched. They are issued *after*
reset_control_reset() so the values survive the register restore the
reset performs.
Note: HTC MSM8660 vendor kernels additionally write 0x0C to reg 0x31.
The HP TouchPad webOS kernel does not touch that register and USB is
stable without it, so those bits are omitted here until documentation
is available to explain what they control.
Assisted-by: Claude:claude-opus-4-7 sparse smatch clang-analyzer coccinelle checkpatch
Assisted-by: Sashiko:claude-opus-4-7
Signed-off-by: Herman van Hazendonk <github.com@herrie.org>
---
drivers/phy/qualcomm/phy-qcom-usb-hs.c | 68 ++++++++++++++++++++++++++++++++++
1 file changed, 68 insertions(+)
diff --git a/drivers/phy/qualcomm/phy-qcom-usb-hs.c b/drivers/phy/qualcomm/phy-qcom-usb-hs.c
index 98a18987f1be..a7649a09e82c 100644
--- a/drivers/phy/qualcomm/phy-qcom-usb-hs.c
+++ b/drivers/phy/qualcomm/phy-qcom-usb-hs.c
@@ -20,6 +20,14 @@
# define ULPI_MISC_A_VBUSVLDEXTSEL BIT(1)
# define ULPI_MISC_A_VBUSVLDEXT BIT(0)
+/* MSM8x60 vendor ULPI registers (raw addresses, not ULPI_EXT_VENDOR_SPECIFIC). */
+#define ULPI_MSM_CONFIG_REG3 0x32
+# define ULPI_MSM_HSDRVSLOPE_MASK GENMASK(3, 0)
+# define ULPI_MSM_PRE_EMPHASIS_MASK GENMASK(5, 4)
+# define ULPI_MSM_PRE_EMPHASIS_20PCT (3 << 4)
+#define ULPI_MSM_DIGOUT_CTRL 0x36
+# define ULPI_MSM_CDR_AUTORESET BIT(1)
+# define ULPI_MSM_SE1_GATE BIT(2)
struct ulpi_seq {
u8 addr;
@@ -37,6 +45,9 @@ struct qcom_usb_hs_phy {
struct ulpi_seq *init_seq;
struct extcon_dev *vbus_edev;
struct notifier_block vbus_notify;
+ bool msm8x60_init;
+ bool hs_drv_slope_present;
+ u8 hs_drv_slope;
};
static int qcom_usb_hs_phy_set_mode(struct phy *phy,
@@ -105,6 +116,41 @@ qcom_usb_hs_phy_vbus_notifier(struct notifier_block *nb, unsigned long event,
return ulpi_write(uphy->ulpi, addr, ULPI_MISC_A_VBUSVLDEXT);
}
+/*
+ * RMW the vendor registers to preserve silicon reserved bits.
+ * In reg 0x36 the legacy semantics are inverted: setting
+ * CDR_AUTORESET / SE1_GATE *disables* those functions.
+ */
+static int qcom_usb_hs_phy_msm8x60_init(struct qcom_usb_hs_phy *uphy)
+{
+ struct ulpi *ulpi = uphy->ulpi;
+ int reg32, reg36, ret;
+
+ reg32 = ulpi_read(ulpi, ULPI_MSM_CONFIG_REG3);
+ if (reg32 < 0)
+ return reg32;
+
+ reg32 &= ~ULPI_MSM_PRE_EMPHASIS_MASK;
+ reg32 |= ULPI_MSM_PRE_EMPHASIS_20PCT;
+
+ if (uphy->hs_drv_slope_present) {
+ reg32 &= ~ULPI_MSM_HSDRVSLOPE_MASK;
+ reg32 |= uphy->hs_drv_slope & ULPI_MSM_HSDRVSLOPE_MASK;
+ }
+
+ ret = ulpi_write(ulpi, ULPI_MSM_CONFIG_REG3, reg32);
+ if (ret)
+ return ret;
+
+ reg36 = ulpi_read(ulpi, ULPI_MSM_DIGOUT_CTRL);
+ if (reg36 < 0)
+ return reg36;
+
+ reg36 |= ULPI_MSM_CDR_AUTORESET | ULPI_MSM_SE1_GATE;
+
+ return ulpi_write(ulpi, ULPI_MSM_DIGOUT_CTRL, reg36);
+}
+
static int qcom_usb_hs_phy_power_on(struct phy *phy)
{
struct qcom_usb_hs_phy *uphy = phy_get_drvdata(phy);
@@ -154,6 +200,12 @@ static int qcom_usb_hs_phy_power_on(struct phy *phy)
goto err_ulpi;
}
+ if (uphy->msm8x60_init) {
+ ret = qcom_usb_hs_phy_msm8x60_init(uphy);
+ if (ret)
+ goto err_ulpi;
+ }
+
if (uphy->vbus_edev) {
state = extcon_get_state(uphy->vbus_edev, EXTCON_USB);
/* setup initial state */
@@ -214,6 +266,22 @@ static int qcom_usb_hs_phy_probe(struct ulpi *ulpi)
return -ENOMEM;
ulpi_set_drvdata(ulpi, uphy);
uphy->ulpi = ulpi;
+ uphy->msm8x60_init = of_device_is_compatible(ulpi->dev.of_node,
+ "qcom,usb-hs-phy-msm8660");
+
+ if (uphy->msm8x60_init) {
+ u32 slope;
+
+ if (!of_property_read_u32(ulpi->dev.of_node,
+ "qcom,hs-drv-slope", &slope)) {
+ if (slope > ULPI_MSM_HSDRVSLOPE_MASK)
+ return dev_err_probe(&ulpi->dev, -EINVAL,
+ "qcom,hs-drv-slope out of range (max %lu)\n",
+ ULPI_MSM_HSDRVSLOPE_MASK);
+ uphy->hs_drv_slope = slope;
+ uphy->hs_drv_slope_present = true;
+ }
+ }
size = of_property_count_u8_elems(ulpi->dev.of_node, "qcom,init-seq");
if (size < 0)
--
2.43.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* [PATCH v3 0/2] phy: qcom: usb-hs: MSM8x60 vendor ULPI init
From: Herman van Hazendonk @ 2026-06-16 13:26 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, Philipp Zabel, Nathan Chancellor,
Nick Desaulniers, Bill Wendling, Justin Stitt
Cc: linux-arm-msm, linux-phy, devicetree, linux-kernel, llvm,
Herman van Hazendonk, konrad.dybcio, dmitry.baryshkov
v3 (this round):
- Re-introduce a much smaller DT binding patch following Konrad's
"do we have values for MTP/QRD" question and Dmitry's
"qcom,hsdrvslope (or similarly named) property in DT" suggestion.
- Survey of every MSM8x60-class downstream tree I could reach --
Qualcomm reference (SURF/FFA/Fluid/Dragon/Fusion via
board-msm8x60.c on android.googlesource.com), Samsung Galaxy S2
family (Q1 / Celox / Dali / generic 8x60 MTP), Sony MSM8660
(sony-kernel-msm8660), HTC MSM8660 ports
(shooter / holiday / pyramid / doubleshot / shooter_u / ruby) and
HP TouchPad -- shows that pre-emphasis, CDR auto-reset and SE1
gating values are *identical* across every reference board.
Only the 4-bit HS driver slope in reg 0x32 [3:0] varies.
- Patch 1/2 adds a single qcom,hs-drv-slope DT property (u32,
range 0..15) gated to the qcom,usb-hs-phy-msm8660 compatible.
- Patch 2/2 hardcodes the three platform-wide writes in the driver
behind the same compatible match, consumes qcom,hs-drv-slope for
the board-specific bits, and leaves the silicon default in place
when the property is absent -- which matches Qualcomm's own MTP,
Samsung and Sony reference behaviour.
- The bit-level meaning we *do* have comes from Code Aurora's
downstream arch/arm/mach-msm/include/mach/msm_hsusb_hw.h, which
Samsung and HP both shipped byte-for-byte identical.
- Per Dmitry's request, both commit messages call out explicitly
that there is no public Qualcomm documentation describing how the
4-bit slope value maps to an actual slew rate / V/ns / %. The
field is an opaque hardware control; boards must copy the value
from their vendor / downstream kernel as a measured-per-layout
knob, not a derived one.
v2:
- Dropped the original qcom,vendor-init-seq DT property entirely
and folded all the vendor-register programming into the driver
behind the qcom,usb-hs-phy-msm8660 compatible.
- HS driver slope was hardcoded in v2. v3 promotes that one
varying value to a DT property as Dmitry requested.
Companion TouchPad DTS work (flipping the PHY compatible from
"qcom,usb-hs-phy-apq8064" to "qcom,usb-hs-phy-msm8660" and adding
qcom,hs-drv-slope = <5>) will be sent separately with the rest of
the apq8060-tenderloin DT series.
On-device validation (HP TouchPad / APQ8060):
- Booted with v3 + the upcoming DTS hookup. PHY driver bound,
msm_hsusb HS link came up at high-speed. No regression vs the v2
hardcoded build.
Build / schema verification:
- dt_binding_check DT_SCHEMA_FILES=.../qcom,usb-hs-phy.yaml: clean.
- dtbs_check on qcom-apq8060-dragonboard.dtb and
qcom-msm8960-cdp.dtb (the two existing in-tree usb-hs-phy
consumers): clean.
- drivers/phy/qualcomm/phy-qcom-usb-hs.o builds clean.
- checkpatch.pl --strict: no warnings on either patch.
Signed-off-by: Herman van Hazendonk <github.com@herrie.org>
---
Herman van Hazendonk (2):
dt-bindings: phy: qcom,usb-hs-phy: add qcom,hs-drv-slope
phy: qcom: usb-hs: program MSM8x60 vendor ULPI registers on power-on
.../devicetree/bindings/phy/qcom,usb-hs-phy.yaml | 89 +++++++++++++++-------
drivers/phy/qualcomm/phy-qcom-usb-hs.c | 68 +++++++++++++++++
2 files changed, 131 insertions(+), 26 deletions(-)
---
base-commit: 944125b4c454b58d2fe6e35f1087a932b2050dff
change-id: 20260616-submit-phy-usb-hs-vendor-init-seq-ad39d29ccaf5
Best regards,
--
Herman van Hazendonk <github.com@herrie.org>
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v2 3/4] arm64: dts: qcom: x1-dell-thena: mark l12b and l15b always-on
From: Konrad Dybcio @ 2026-06-16 12:22 UTC (permalink / raw)
To: Michael Scott, linux-arm-msm
Cc: vkoul, neil.armstrong, dmitry.baryshkov, wesley.cheng, abelvesa,
faisal.hassan, linux-phy, andersson, konradybcio, robh, krzk+dt,
conor+dt, devicetree, val, bryan.odonoghue, laurentiu.tudor1,
alex.vinarskis, linux-kernel, stable
In-Reply-To: <20260521010935.1333494-4-mike.scott@oss.qualcomm.com>
On 5/21/26 3:09 AM, Michael Scott wrote:
> The l12b and l15b supplies are used by components that are not (fully)
> described (and some never will be) and must never be disabled.
>
> Mark the regulators as always-on to prevent them from being disabled,
> for example, when consumers probe defer or suspend.
>
> Note that these supplies currently have no consumers described in
> mainline for dell-thena beyond the audio codec (vdd-buck/vdd-rxtx/
> vdd-io on wcd938x), which can release them when the codec goes idle.
> The board-level gpio-fixed regulators that feed the Type-C retimer's
> VDDIO and other rails are not described with a vin-supply link, so
> the kernel cannot keep their parent LDOs alive on its own.
>
> This mirrors the same change Johan Hovold applied to every other
> X1E80100 board in a March 2025 series; commit 63169c07d740
> ("arm64: dts: qcom: x1e80100-dell-xps13-9345: mark l12b and l15b always-on")
> is representative. The dell-thena board file was introduced four months
> later and did not inherit that change; this patch closes the gap.
>
> Fixes: e7733b42111c ("arm64: dts: qcom: Add support for Dell Inspiron 7441 / Latitude 7455")
> Cc: stable@vger.kernel.org
> Signed-off-by: Michael Scott <mike.scott@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 v2 3/4] PCI: qcom: Add link retention support
From: Konrad Dybcio @ 2026-06-16 12:08 UTC (permalink / raw)
To: Krishna Chaitanya Chundru, Vinod Koul, Neil Armstrong,
Philipp Zabel, Jingoo Han, Manivannan Sadhasivam,
Lorenzo Pieralisi, Krzysztof Wilczyński, Rob Herring,
Bjorn Helgaas
Cc: linux-arm-msm, linux-phy, linux-kernel, linux-pci, Qiang Yu
In-Reply-To: <20260521-link_retain-v2-3-08ed448b081c@oss.qualcomm.com>
On 5/21/26 2:56 PM, Krishna Chaitanya Chundru wrote:
> Some platforms keep the PCIe link active across bootloader and kernel
> handoff. Reinitializing the controller and toggling PERST# in such cases is
> unnecessary when the driver does not need to retrain the link.
>
> Introduce link_retain in both qcom_pcie_cfg and qcom_pcie to indicate when
> link retention is supported. During initialization, check the LTSSM state;
> if the link is already in L0 or L1 idle and LTSSM is enabled, set
> link_retain and skip controller reset, PERST# toggling, and other post-
> init steps.
>
> If the current link speed or lane width does not satisfy the constraints
> specified by max-link-speed or num-lanes in the device tree, fall back to
> normal initialization and retrain the link instead of retaining it.
>
> Configure the DBI and ATU base addresses in the retention path, since the
> bootloader may use different base addresses than those provided by the
> device tree.
>
> Also fix the -EPROBE_DEFER error handling path to return 0 instead of
> propagating the error, avoiding unnecessary cleanup when probe deferral is
> requested.
>
> Tested-by: Qiang Yu <qiang.yu@oss.qualcomm.com>
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> ---
> drivers/pci/controller/dwc/pcie-designware.h | 1 +
> drivers/pci/controller/dwc/pcie-qcom.c | 62 +++++++++++++++++++++++++---
> 2 files changed, 58 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index 3e69ef60165b..be6c4abf31e8 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -450,6 +450,7 @@ struct dw_pcie_rp {
> bool ecam_enabled;
> bool native_ecam;
> bool skip_l23_ready;
> + bool link_retain;
> };
>
> struct dw_pcie_ep_ops {
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index bfe873cbf44f..b061eaa227b3 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -253,12 +253,14 @@ struct qcom_pcie_ops {
> * @override_no_snoop: Override NO_SNOOP attribute in TLP to enable cache
> * snooping
> * @firmware_managed: Set if the Root Complex is firmware managed
> + * @link_retain: Set if controller supports retaining link from bootloader
> */
> struct qcom_pcie_cfg {
> const struct qcom_pcie_ops *ops;
> bool override_no_snoop;
> bool firmware_managed;
> bool no_l0s;
> + bool link_retain;
> };
>
> struct qcom_pcie_perst {
> @@ -960,6 +962,42 @@ static int qcom_pcie_get_resources_2_7_0(struct qcom_pcie *pcie)
> return 0;
> }
>
> +/*
> + * Determine whether the link established by the bootloader can be reused.
> + *
> + * Reuse the existing link only if its current speed and lane count match
> + * the max-link-speed and num-lanes specified in Device Tree; otherwise,
> + * retrain the link.
> + */
> +static bool qcom_pcie_check_link_retain(struct qcom_pcie *pcie)
> +{
> + u32 cap, speed, val, ltssm, width;
> + struct dw_pcie *pci = pcie->pci;
> + u8 offset;
> +
> + val = readl(pcie->parf + PARF_LTSSM);
> + ltssm = val & 0x1f;
> + if ((val & LTSSM_EN) &&
> + (ltssm == DW_PCIE_LTSSM_L0 || ltssm == DW_PCIE_LTSSM_L1_IDLE)) {
> + qcom_pcie_configure_dbi_atu_base(pcie);
> +
> + offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
> + cap = dw_pcie_readl_dbi(pci, offset + PCI_EXP_LNKCAP);
> + speed = FIELD_GET(PCI_EXP_LNKCAP_SLS, cap);
> + width = dw_pcie_link_get_max_link_width(pci);
> +
> + if (pci->max_link_speed > 0 && speed > pci->max_link_speed)
I think I raised this concern already, but this goes against what
max-link-speed is supposed to do, i.e. this will not retrain the link if
the bootloader had initialized the link to a speed faster than what the
DT requested
> + return false;
> +
> + if (pci->num_lanes > 0 && width > pci->num_lanes)
> + return false;
Similarly, this should be ==
Konrad
--
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