* [PATCH v2 1/2] dt-bindings: phy: qcom,sc8280xp-qmp-usb3-uni: Add SA8255P USB PHY
From: Mattijs Korpershoek @ 2026-07-09 14:52 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: Maxime Ripard, linux-arm-msm, linux-phy, devicetree, linux-kernel,
Mattijs Korpershoek
In-Reply-To: <20260709-phy-qcom-qmp-usb-8255-v2-0-3b6c26a45788@kernel.org>
SA8255p platform abstracts resources such as clocks, interconnect and
GPIO pins configuration in Firmware. SCMI power protocol is used to
request resource configurations.
Two power domains can be requested: usb_transfer and usb_core.
usb_transfer controls clocks and interconnects and usb_core controls
the GDSC, resets and TLMM (GPIOs)
Add compatible string for Qualcomm QMP Super Speed (SS) UNI PHY found
in SA8255P.
Add conditionals to make some properties (clocks, resets, supplies)
optional for SA8255P while keeping them required for all other platforms.
Finally, also add a new example to illustrate a firmware-managed node.
Signed-off-by: Mattijs Korpershoek <mkorpershoek@kernel.org>
---
.../phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml | 92 +++++++++++++++++++---
1 file changed, 83 insertions(+), 9 deletions(-)
diff --git a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml
index 01342823e57f..80b04b7f7584 100644
--- a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml
@@ -26,6 +26,7 @@ properties:
- qcom,msm8996-qmp-usb3-phy
- qcom,qcs8300-qmp-usb3-uni-phy
- qcom,qdu1000-qmp-usb3-uni-phy
+ - qcom,sa8255p-qmp-usb3-uni-phy
- qcom,sa8775p-qmp-usb3-uni-phy
- qcom,sc8180x-qmp-usb3-uni-phy
- qcom,sc8280xp-qmp-usb3-uni-phy
@@ -54,7 +55,13 @@ properties:
maxItems: 5
power-domains:
- maxItems: 1
+ minItems: 1
+ maxItems: 2
+
+ power-domain-names:
+ items:
+ - const: usb_transfer
+ - const: usb_core
resets:
maxItems: 2
@@ -82,14 +89,6 @@ properties:
required:
- compatible
- reg
- - clocks
- - clock-names
- - resets
- - reset-names
- - vdda-phy-supply
- - vdda-pll-supply
- - "#clock-cells"
- - clock-output-names
- "#phy-cells"
allOf:
@@ -188,6 +187,66 @@ allOf:
properties:
refgen-supply: false
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - qcom,glymur-qmp-usb3-uni-phy
+ - qcom,ipq5424-qmp-usb3-phy
+ - qcom,ipq6018-qmp-usb3-phy
+ - qcom,ipq8074-qmp-usb3-phy
+ - qcom,ipq9574-qmp-usb3-phy
+ - qcom,msm8996-qmp-usb3-phy
+ - qcom,qcs8300-qmp-usb3-uni-phy
+ - qcom,qdu1000-qmp-usb3-uni-phy
+ - qcom,sa8775p-qmp-usb3-uni-phy
+ - qcom,sc8180x-qmp-usb3-uni-phy
+ - qcom,sc8280xp-qmp-usb3-uni-phy
+ - qcom,sdm845-qmp-usb3-uni-phy
+ - qcom,sdx55-qmp-usb3-uni-phy
+ - qcom,sdx65-qmp-usb3-uni-phy
+ - qcom,sdx75-qmp-usb3-uni-phy
+ - qcom,sm8150-qmp-usb3-uni-phy
+ - qcom,sm8250-qmp-usb3-uni-phy
+ - qcom,sm8350-qmp-usb3-uni-phy
+ - qcom,x1e80100-qmp-usb3-uni-phy
+ then:
+ properties:
+ power-domains:
+ maxItems: 1
+ required:
+ - clocks
+ - clock-names
+ - resets
+ - reset-names
+ - vdda-phy-supply
+ - vdda-pll-supply
+ - "#clock-cells"
+ - clock-output-names
+
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - qcom,sa8255p-qmp-usb3-uni-phy
+ then:
+ properties:
+ clocks: false
+ clock-names: false
+ resets: false
+ reset-names: false
+ vdda-phy-supply: false
+ vdda-pll-supply: false
+ "#clock-cells": false
+ clock-output-names: false
+ power-domains:
+ minItems: 2
+ required:
+ - power-domains
+ - power-domain-names
+
additionalProperties: false
examples:
@@ -219,3 +278,18 @@ examples:
#phy-cells = <0>;
};
+
+ - |
+ soc {
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ phy@88e8000 {
+ compatible = "qcom,sa8255p-qmp-usb3-uni-phy";
+ reg = <0x0 0x088e8000 0x0 0x2000>;
+ #phy-cells = <0>;
+
+ power-domains = <&scmi12_pd 2>, <&scmi12_pd 3>;
+ power-domain-names = "usb_transfer", "usb_core";
+ };
+ };
--
2.54.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* [PATCH v2 0/2] Add support for SA8255P in the qcom-qmp-usb phy driver
From: Mattijs Korpershoek @ 2026-07-09 14:52 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: Maxime Ripard, linux-arm-msm, linux-phy, devicetree, linux-kernel,
Mattijs Korpershoek, Sriram Dash, Shazad Hussain
The Qualcomm automotive SA8255p SoC relies on firmware to configure
platform resources, including clocks, interconnects and TLMM.
The driver requests resources operations over SCMI using power protocols.
The SCMI power protocol enables or disables resources like clocks,
interconnect paths, and TLMM (GPIOs) using runtime PM framework APIs,
such as resume/suspend, to control power on/off.
Power domain "core" controls the GDSC, resets, and TLMM (GPIOs)
whereas power domain "transfer" controls clocks and interconnects.
Patch 1 adds the binding change for sa8255p. I've considered to create
a totally separate binding like the I2C one [1].
Given that the existing binding already had multiple conditionals, I've
chosen to update qcom,sc8280xp-qmp-usb3-uni-phy.yaml by adding another
conditional for the power domains.
Patch 2 adds the driver change, which is based on a public vendor
patch from qualcomm [2].
I did the following changes on top of that:
* Use devm_add_action_or_reset() for dev_pm_domain_detach_list() instead
of implementing a driver .remove()
* Drop the .lanes struct member since it no longer exists.
* Re-order some functions
This has been tested on next-20260706 with a Ride4 SA8775P board that
has SCMI firmware (which is compatible).
[1] https://lore.kernel.org/all/20260617-enable-i2c-on-sa8255p-v7-1-ad736dbeab57@oss.qualcomm.com/
[2] https://git.codelinaro.org/clo/le/meta-qti-auto-kernel/-/blob/LY.AU.0.1.0.r1-16800-gen4meta.0/recipes-kernel/linux/files/usb/0011-PENDING-phy-qcom-qmp-usb-Add-support-for-SA8255P.patch
Signed-off-by: Mattijs Korpershoek <mkorpershoek@kernel.org>
---
Changes in v2:
* Driver changes:
* Fix comment typo tranfer -> transfer
* Fix print statement to add a missing '(' in the string
* Use dev_err_probe() in qmp_usb_domain_attach() to reduce log noise
* Fix pm_runtime_put_sync() order in qmp_usb_d0_to_d3 (Sashiko)
* Fix runtime PM reference leak on usb_transfer (Sashiko)
* Fix runtime PM reference leak on error in qmp_usb_init (Sashiko)
* Reorder qmp_usb_d0_to_d3() to mirror init ordering in qmp_usb_exit()
(Sashiko)
* Reorder qmp_usb_d0_to_d3() to mirror resume ordering in
qmp_usb_runtime_suspend() (Sashiko)
* Fix error handling in qmp_usb_runtime_resume() (Sashiko)
* Link to v1: https://patch.msgid.link/20260708-phy-qcom-qmp-usb-8255-v1-0-8d852a9535db@kernel.org
---
Mattijs Korpershoek (1):
dt-bindings: phy: qcom,sc8280xp-qmp-usb3-uni: Add SA8255P USB PHY
Sriram Dash (1):
phy: qcom: qmp-usb: Add support for SA8255P
.../phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml | 92 ++++++++++-
drivers/phy/qualcomm/phy-qcom-qmp-usb.c | 182 ++++++++++++++++++---
2 files changed, 243 insertions(+), 31 deletions(-)
---
base-commit: dc59e4fea9d83f03bad6bddf3fa2e52491777482
change-id: 20260708-phy-qcom-qmp-usb-8255-9986907cdbc9
Best regards,
--
Mattijs Korpershoek <mkorpershoek@kernel.org>
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v4] phy: renesas: rcar-gen3-usb2: Avoid long delay in atomic context
From: sashiko-bot @ 2026-07-09 14:18 UTC (permalink / raw)
To: Claudiu Beznea; +Cc: linux-phy, vkoul, neil.armstrong, olteanv, wsa+renesas
In-Reply-To: <20260709140257.843685-1-claudiu.beznea+renesas@tuxon.dev>
Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:
New issues:
- [High] A lingering `-ETIMEDOUT` from a previous loop iteration causes spurious failures and resource leaks in `rcar_gen3_phy_usb2_power_on()` and `rcar_gen3_phy_usb2_power_off()`.
- [High] `rcar_gen3_phy_usb2_irqs_mask_all()` erroneously clears `obint_enable_bits` from `USB2_INT_ENABLE` instead of `USB2_OBINTEN`, failing to mask OTG interrupts and corrupting general interrupt enables.
- [Medium] The OTG initialization retry loop short-circuits on the final retry, allowing concurrent hardware modification if a new initialization phase has started.
- [Medium] Calling `reinit_completion()` while there are active waiters causes a data race on `x->done`, violating the completion API.
Pre-existing issues:
- [High] The interrupt handler uses `pm_runtime_get_noresume()` and `pm_runtime_suspended()` to guard hardware access, which is racy and can lead to invalid register reads.
- [High] The sysfs attributes `role_show` and `role_store` access hardware registers without ensuring the device is actively powered.
- [High] Several paths call `pm_runtime_resume_and_get()` while holding the `ch->lock` spinlock, which can block and trigger a scheduling-while-atomic BUG.
--
--- Patch [1]: [PATCH v4] phy: renesas: rcar-gen3-usb2: Avoid long delay in atomic context ---
commit 9fdad1bb87b936de49c69272d521e46e26b7e16a
Author: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
phy: renesas: rcar-gen3-usb2: Avoid long delay in atomic context
The OTG PHY initialization sequence needs to wait for 20 ms at a specific
step, as described in commit 72c0339c115b ("phy: renesas:
rcar-gen3-usb2: follow the hardware manual procedure").
Fixes: 55a387ebb921 ("phy: renesas: rcar-gen3-usb2: Lock around hardware registers and driver data")
Cc: stable@vger.kernel.org
Reported-by: Pavel Machek <pavel@nabladev.com>
Closes: https://lore.kernel.org/all/afhkX2Ys2BG1gnqy@duo.ucw.cz
Reported-by: Nobuhiro Iwamatsu <iwamatsu@nigauri.org>
Closes: https://lore.kernel.org/all/afhkX2Ys2BG1gnqy@duo.ucw.cz
Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> diff --git a/drivers/phy/renesas/phy-rcar-gen3-usb2.c b/drivers/phy/renesas/phy-rcar-gen3-usb2.c
> index 9a45d840efeb2..e1873928cc847 100644
> --- a/drivers/phy/renesas/phy-rcar-gen3-usb2.c
> +++ b/drivers/phy/renesas/phy-rcar-gen3-usb2.c
[ ... ]
> @@ -392,26 +407,52 @@ static ssize_t role_store(struct device *dev, struct device_attribute *attr,
> struct rcar_gen3_chan *ch = dev_get_drvdata(dev);
> bool is_b_device;
> enum phy_mode cur_mode, new_mode;
> + int retries = NUM_OF_PHYS;
> + unsigned long flags;
> + int ret = -EIO;
>
> - guard(spinlock_irqsave)(&ch->lock);
> + spin_lock_irqsave(&ch->lock, flags);
[Severity: High]
This isn't a bug introduced by this patch, but does this call chain sleep while
holding the spinlock?
role_store() acquires ch->lock via spin_lock_irqsave(), then calls
rcar_gen3_init_from_a_peri_to_a_host(), which can cascade into
rcar_gen3_enable_vbus_ctrl().
For configurations using the internal regulator (where no_adp_ctrl is true),
this calls regulator_hardware_enable(), invoking the driver's .enable callback
(rcar_gen3_phy_usb2_regulator_enable).
That callback unconditionally calls pm_runtime_resume_and_get(), which can sleep
synchronously while waiting for the device to resume. Could this trigger a
scheduling-while-atomic panic?
>
> - if (!ch->is_otg_channel || !rcar_gen3_is_any_otg_rphy_initialized(ch))
> - return -EIO;
> + if (!ch->is_otg_channel)
> + goto unlock;
> +
> + while (retries-- && ch->otg_initializing) {
> + spin_unlock_irqrestore(&ch->lock, flags);
> +
> + ret = wait_for_completion_timeout(&ch->otg_init_done,
> + USB2_OTG_INIT_TIMEOUT);
> + ret = ret ? 0 : -ETIMEDOUT;
> + if (ret && !retries)
> + goto exit;
> +
> + spin_lock_irqsave(&ch->lock, flags);
> + }
> +
> + if (!rcar_gen3_is_any_otg_rphy_initialized(ch)) {
> + ret = -EIO;
> + goto unlock;
> + }
>
> - if (sysfs_streq(buf, "host"))
> + if (sysfs_streq(buf, "host")) {
> new_mode = PHY_MODE_USB_HOST;
> - else if (sysfs_streq(buf, "peripheral"))
> + } else if (sysfs_streq(buf, "peripheral")) {
> new_mode = PHY_MODE_USB_DEVICE;
> - else
> - return -EINVAL;
> + } else {
> + ret = -EINVAL;
> + goto unlock;
> + }
>
> /* is_b_device: true is B-Device. false is A-Device. */
> is_b_device = rcar_gen3_check_id(ch);
> cur_mode = rcar_gen3_get_phy_mode(ch);
[Severity: High]
This is a pre-existing issue, but are these hardware register accesses safe
without ensuring the device is actively powered?
role_store() calls rcar_gen3_check_id() and rcar_gen3_get_phy_mode(), which read
hardware registers like USB2_ADPCTRL and USB2_LINECTRL1.
Since these sysfs callbacks don't use pm_runtime_get_sync() before hardware
accesses, could reading these registers while the PHY device is runtime
suspended lead to invalid data reads or trigger asynchronous external aborts?
[ ... ]
> @@ -502,6 +547,7 @@ static irqreturn_t rcar_gen3_phy_usb2_irq(int irq, void *_ch)
> void __iomem *usb2_base = ch->base;
> struct device *dev = ch->dev;
> irqreturn_t ret = IRQ_NONE;
> + unsigned long flags;
> u32 status;
>
> pm_runtime_get_noresume(dev);
> @@ -509,33 +555,91 @@ static irqreturn_t rcar_gen3_phy_usb2_irq(int irq, void *_ch)
> if (pm_runtime_suspended(dev))
> goto rpm_put;
[Severity: High]
This is a pre-existing issue, but does this runtime PM check protect against
concurrent suspend transitions?
The handler uses pm_runtime_get_noresume() which increments the usage counter
but does not check the status atomically. If the device uses a shared IRQ line
and another device triggers an interrupt while this device is in the
RPM_SUSPENDING state, pm_runtime_suspended() will return false.
Could the handler then proceed to read hardware registers that are actively
being powered down, leading to invalid reads (e.g., 0xffffffff) and spurious
interrupt processing? Should this use pm_runtime_get_if_active() instead?
[ ... ]
> +static void rcar_gen3_phy_usb2_irqs_mask_all(struct rcar_gen3_chan *channel,
> + u32 *masked_irqs_bits)
> +{
> + u32 val, bitmask = USB2_INT_ENABLE_UCOM_INTEN;
> + void __iomem *usb2_base = channel->base;
> +
> + for (unsigned int i = 0; i < NUM_OF_PHYS; i++)
> + bitmask |= channel->rphys[i].int_enable_bits;
> +
> + val = readl(usb2_base + USB2_INT_ENABLE);
> + *masked_irqs_bits = val & bitmask;
> + /*
> + * Don't report channel->phy_data->obint_enable_bits IRQs. These are
> + * unmasked anyway in rcar_gen3_init_otg_phase1().
> + */
> + val &= ~(bitmask | channel->phy_data->obint_enable_bits);
> + writel(val, usb2_base + USB2_INT_ENABLE);
> +}
[Severity: High]
Does this mistakenly apply the obint_enable_bits to the wrong register?
This function reads the USB2_INT_ENABLE register, but obint_enable_bits belong
to the USB2_OBINTEN register.
When rcar_gen3_phy_usb2_irqs_unmask() is called later, it only restores bits
defined in bitmask, which doesn't include the obint bits. Will this leave the
corrupted bits in USB2_INT_ENABLE cleared permanently and fail to mask OTG
interrupts during the initialization window?
[ ... ]
> static int rcar_gen3_phy_usb2_init(struct phy *p)
> {
> struct rcar_gen3_phy *rphy = phy_get_drvdata(p);
> struct rcar_gen3_chan *channel = rphy->ch;
> void __iomem *usb2_base = channel->base;
> + int retries = NUM_OF_PHYS;
> + unsigned long flags;
> u32 val;
>
> - guard(spinlock_irqsave)(&channel->lock);
> + spin_lock_irqsave(&channel->lock, flags);
> +
> + while (retries-- && channel->otg_initializing) {
[Severity: Medium]
Is there a race condition when the retries counter hits zero?
On the final loop iteration (when retries evaluates to 0 inside the loop), if
wait_for_completion_timeout() succeeds without timing out, the loop condition
will be evaluated again.
Because retries is 0, the post-decrement operator returns 0, causing the &&
to short-circuit. The channel->otg_initializing variable is not checked, and
the loop exits.
If another thread had just started a new initialization phase and set
otg_initializing = true, could the exiting thread proceed to modify hardware
concurrently?
> + int ret;
> +
> + spin_unlock_irqrestore(&channel->lock, flags);
> +
> + ret = wait_for_completion_timeout(&channel->otg_init_done,
> + USB2_OTG_INIT_TIMEOUT);
> + ret = ret ? 0 : -ETIMEDOUT;
> + if (ret && !retries)
> + return ret;
> +
> + spin_lock_irqsave(&channel->lock, flags);
> + }
[ ... ]
> /* Initialize otg part (only if we initialize a PHY with IRQs). */
> - if (rphy->int_enable_bits)
> - rcar_gen3_init_otg(channel);
> + if (rphy->int_enable_bits && channel->is_otg_channel &&
> + !rcar_gen3_is_any_otg_rphy_initialized(channel)) {
> + u32 masked_irq_bits = 0;
> +
> + rcar_gen3_init_otg_phase0(channel);
> + rcar_gen3_phy_usb2_irqs_mask_all(channel, &masked_irq_bits);
> + reinit_completion(&channel->otg_init_done);
> + channel->otg_initializing = true;
[Severity: Medium]
Is it safe to reinitialize the completion here?
Waiters from a previous wait_for_completion_timeout() might still be inside
wait_for_common() processing their wakeup.
Since the completion API mandates that reinit_completion() must only be called
when there are no active waiters, could this cause a data race on x->done and
lead to unpredictable wake-up behavior for prior waiters?
[ ... ]
> static int rcar_gen3_phy_usb2_power_on(struct phy *p)
> {
> struct rcar_gen3_phy *rphy = phy_get_drvdata(p);
> struct rcar_gen3_chan *channel = rphy->ch;
> void __iomem *usb2_base = channel->base;
> + int retries = NUM_OF_PHYS;
> + unsigned long flags;
> u32 val;
> int ret = 0;
>
> @@ -605,7 +746,19 @@ static int rcar_gen3_phy_usb2_power_on(struct phy *p)
> return ret;
> }
>
> - guard(spinlock_irqsave)(&channel->lock);
> + spin_lock_irqsave(&channel->lock, flags);
> +
> + while (retries-- && channel->otg_initializing) {
> + spin_unlock_irqrestore(&channel->lock, flags);
> +
> + ret = wait_for_completion_timeout(&channel->otg_init_done,
> + USB2_OTG_INIT_TIMEOUT);
> + ret = ret ? 0 : -ETIMEDOUT;
> + if (ret && !retries)
> + goto disable_regulator;
> +
> + spin_lock_irqsave(&channel->lock, flags);
> + }
>
> if (!rcar_gen3_are_all_rphys_power_off(channel))
> goto out;
> @@ -620,27 +773,49 @@ static int rcar_gen3_phy_usb2_power_on(struct phy *p)
> /* The powered flag should be set for any other phys anyway */
> rphy->powered = true;
>
> - return 0;
> + spin_unlock_irqrestore(&channel->lock, flags);
> +
> +disable_regulator:
> + if (ret && channel->vbus && !channel->otg_internal_reg)
> + regulator_disable(channel->vbus);
> +
> + return ret;
> }
[Severity: High]
Could a lingering -ETIMEDOUT from the previous loop iteration cause a spurious
failure here?
If a thread enters the wait loop while another thread is executing the OTG
initialization, wait_for_completion_timeout() might time out at least once,
setting ret to -ETIMEDOUT.
If the OTG initialization completes before the next loop condition evaluation,
channel->otg_initializing becomes false and the loop exits normally.
However, ret retains the stale -ETIMEDOUT value. In power_on(), this causes the
disable_regulator cleanup path to execute despite success.
Should ret be reset to 0 after exiting the wait loop?
[ ... ]
> static int rcar_gen3_phy_usb2_power_off(struct phy *p)
> {
> struct rcar_gen3_phy *rphy = phy_get_drvdata(p);
> struct rcar_gen3_chan *channel = rphy->ch;
> + int retries = NUM_OF_PHYS;
> + unsigned long flags;
> int ret = 0;
>
> - scoped_guard(spinlock_irqsave, &channel->lock) {
> - rphy->powered = false;
> + spin_lock_irqsave(&channel->lock, flags);
>
> - if (rcar_gen3_are_all_rphys_power_off(channel)) {
> - u32 val = readl(channel->base + USB2_USBCTR);
> + while (retries-- && channel->otg_initializing) {
> + spin_unlock_irqrestore(&channel->lock, flags);
>
> - val |= USB2_USBCTR_PLL_RST;
> - writel(val, channel->base + USB2_USBCTR);
> - }
> + ret = wait_for_completion_timeout(&channel->otg_init_done,
> + USB2_OTG_INIT_TIMEOUT);
> + ret = ret ? 0 : -ETIMEDOUT;
> + if (ret && !retries)
> + return ret;
> +
> + spin_lock_irqsave(&channel->lock, flags);
> }
>
> - if (channel->vbus && !channel->otg_internal_reg)
> + rphy->powered = false;
> +
> + if (rcar_gen3_are_all_rphys_power_off(channel)) {
> + u32 val = readl(channel->base + USB2_USBCTR);
> +
> + val |= USB2_USBCTR_PLL_RST;
> + writel(val, channel->base + USB2_USBCTR);
> + }
> +
> + spin_unlock_irqrestore(&channel->lock, flags);
> +
> + if (!ret && channel->vbus && !channel->otg_internal_reg)
> ret = regulator_disable(channel->vbus);
>
> return ret;
> }
[Severity: High]
Similar to power_on(), could a stale -ETIMEDOUT in ret from the wait loop
prevent the regulator from being disabled here, leaking the resource?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709140257.843685-1-claudiu.beznea+renesas@tuxon.dev?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: renesas: rcar-gen3-usb2: Avoid long delay in atomic context
From: Claudiu Beznea @ 2026-07-09 14:02 UTC (permalink / raw)
To: yoshihiro.shimoda.uh, vkoul, neil.armstrong, geert+renesas,
magnus.damm, prabhakar.mahadev-lad.rj
Cc: claudiu.beznea, linux-renesas-soc, linux-phy, linux-kernel,
Claudiu Beznea, stable, Pavel Machek, Nobuhiro Iwamatsu
From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
The OTG PHY initialization sequence needs to wait for 20 ms at a specific
step, as described in commit 72c0339c115b ("phy: renesas:
rcar-gen3-usb2: follow the hardware manual procedure").
Commit 55a387ebb921 ("phy: renesas: rcar-gen3-usb2: Lock around hardware
registers and driver data") tried to address various problems in the
rcar-gen3-usb2 driver and converted the mutex protecting HW register
accesses to a spin lock, leaving, however, a long delay in the critical
section protected by the spin lock. This may become a problem,
especially on RT kernels.
To address this, release the spin lock before sleeping for 20 ms as
required by the HW manual and reacquire it afterwards. To avoid other
threads entering the critical section and configuring the HW while the
software is waiting for the OTG initialization to complete, introduce the
otg_initializing variable alongside the otg_init_done completion. Any
other thread trying to configure the HW while the OTG PHY initialization
is in progress waits for the completion instead of immediately returning
errors to PHY users. The IRQs were also disabled while waiting for the OTG
PHY initialization to complete, as the interrupt handler may also apply HW
settings.
The OTG can only be initialized once. It is initialized by the first PHY
that calls struct phy_ops::rcar_gen3_phy_usb2_init().
To avoid failures when multiple PHYs call struct
phy_ops::rcar_gen3_phy_usb2_init() simultaneously, and the PHY responsible
for initializing the OTG either fails or deinit quiqly and another PHY
takes over the PHY init role), the code waiting for the
channel->otg_init_done completion retries up to NUM_OF_PHYS times.
Fixes: 55a387ebb921 ("phy: renesas: rcar-gen3-usb2: Lock around hardware registers and driver data")
Cc: stable@vger.kernel.org
Reported-by: Pavel Machek <pavel@nabladev.com>
Closes: https://lore.kernel.org/all/afhkX2Ys2BG1gnqy@duo.ucw.cz
Reported-by: Nobuhiro Iwamatsu <iwamatsu@nigauri.org>
Closes: https://lore.kernel.org/all/afhkX2Ys2BG1gnqy@duo.ucw.cz
Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---
Changes in v4:
- increase the timeout to 120 ms and added a macro for it USB2_OTG_INIT_TIMEOUT
- add retry mechanism when waiting for otg_init_done completion
- read the status registers in rcar_gen3_phy_usb2_irq() before returning,
in case an unexpected IRQ is received while waiting for OTG init
- mask channel->phy_data->obint_enable_bits in rcar_gen3_phy_usb2_irqs_mask_all()
- updated the patch description
Changes in v3:
- initialize ret value in role_store()
- jump to exit label in role_store() to have the same exit path
- dropped scoped_guard() in rcar_gen3_phy_usb2_irq() to avoid mixing
goto statements with scoped_guard()
- take into account USB2_INT_ENABLE_UCOM_INTEN when masking/unmasking IRQs
- increase the completion timeout to 40ms
- in rcar_gen3_phy_usb2_power_on() disable regulator on failure path
Changes in v2:
- dropped the rcar_gen3_phy_wait_otg_init() and used inline the code; with it
the comment that was previously in rcar_gen3_phy_wait_otg_init() was added
in struct rcar_gen3_chan for above the org_initializing variable
- dropped READ_ONCE()/WRITE_ONCE() arround struct rcar_gen3_chan::otg_initializing
- checked for struct rcar_gen3_chan::otg_initializing before checking for
rcar_gen3_is_any_otg_rphy_initialized() in all code places
- added rcar_gen3_phy_usb2_irqs_mask_all() and rcar_gen3_phy_usb2_irqs_unmask()
to mask/unmask only the interrupts at the controller level since the line is
shared
- along with it dropped disable_irq_nosync()/enable_irq() from v1 along with
struct rcar_gen3_chan::irq
- increased the completion timeout to 30 ms
drivers/phy/renesas/phy-rcar-gen3-usb2.c | 258 +++++++++++++++++++----
1 file changed, 217 insertions(+), 41 deletions(-)
diff --git a/drivers/phy/renesas/phy-rcar-gen3-usb2.c b/drivers/phy/renesas/phy-rcar-gen3-usb2.c
index 9a45d840efeb..e1873928cc84 100644
--- a/drivers/phy/renesas/phy-rcar-gen3-usb2.c
+++ b/drivers/phy/renesas/phy-rcar-gen3-usb2.c
@@ -106,6 +106,13 @@
/* RZ/G2L specific */
#define USB2_LINECTRL1_USB2_IDMON BIT(0)
+/*
+ * The OTG initialization is expected to finish in 20ms. Choose a large enough
+ * timeout to avoid waiters exit prematurely the waiting section under heavy
+ * CPU load.
+ */
+#define USB2_OTG_INIT_TIMEOUT msecs_to_jiffies(120)
+
#define NUM_OF_PHYS 4
enum rcar_gen3_phy_index {
PHY_INDEX_BOTH_HC,
@@ -138,12 +145,20 @@ struct rcar_gen3_chan {
struct rcar_gen3_phy rphys[NUM_OF_PHYS];
struct regulator *vbus;
struct work_struct work;
+ struct completion otg_init_done;
spinlock_t lock; /* protects access to hardware and driver data structure. */
enum usb_dr_mode dr_mode;
bool extcon_host;
bool is_otg_channel;
bool uses_otg_pins;
bool otg_internal_reg;
+ /*
+ * The OTG can be initialized only once and needs to release the spinlock
+ * and wait for 20 ms due to hardware constraints. If a thread executes
+ * PHY configuration code while the OTG PHY is waiting for the 20 ms, the
+ * thread will have to wait for the OTG PHY initialization to complete.
+ */
+ bool otg_initializing;
};
struct rcar_gen3_phy_drv_data {
@@ -392,26 +407,52 @@ static ssize_t role_store(struct device *dev, struct device_attribute *attr,
struct rcar_gen3_chan *ch = dev_get_drvdata(dev);
bool is_b_device;
enum phy_mode cur_mode, new_mode;
+ int retries = NUM_OF_PHYS;
+ unsigned long flags;
+ int ret = -EIO;
- guard(spinlock_irqsave)(&ch->lock);
+ spin_lock_irqsave(&ch->lock, flags);
- if (!ch->is_otg_channel || !rcar_gen3_is_any_otg_rphy_initialized(ch))
- return -EIO;
+ if (!ch->is_otg_channel)
+ goto unlock;
+
+ while (retries-- && ch->otg_initializing) {
+ spin_unlock_irqrestore(&ch->lock, flags);
+
+ ret = wait_for_completion_timeout(&ch->otg_init_done,
+ USB2_OTG_INIT_TIMEOUT);
+ ret = ret ? 0 : -ETIMEDOUT;
+ if (ret && !retries)
+ goto exit;
+
+ spin_lock_irqsave(&ch->lock, flags);
+ }
+
+ if (!rcar_gen3_is_any_otg_rphy_initialized(ch)) {
+ ret = -EIO;
+ goto unlock;
+ }
- if (sysfs_streq(buf, "host"))
+ if (sysfs_streq(buf, "host")) {
new_mode = PHY_MODE_USB_HOST;
- else if (sysfs_streq(buf, "peripheral"))
+ } else if (sysfs_streq(buf, "peripheral")) {
new_mode = PHY_MODE_USB_DEVICE;
- else
- return -EINVAL;
+ } else {
+ ret = -EINVAL;
+ goto unlock;
+ }
/* is_b_device: true is B-Device. false is A-Device. */
is_b_device = rcar_gen3_check_id(ch);
cur_mode = rcar_gen3_get_phy_mode(ch);
/* If current and new mode is the same, this returns the error */
- if (cur_mode == new_mode)
- return -EINVAL;
+ if (cur_mode == new_mode) {
+ ret = -EINVAL;
+ goto unlock;
+ }
+
+ ret = 0;
if (new_mode == PHY_MODE_USB_HOST) { /* And is_host must be false */
if (!is_b_device) /* A-Peripheral */
@@ -425,7 +466,10 @@ static ssize_t role_store(struct device *dev, struct device_attribute *attr,
rcar_gen3_init_for_peri(ch);
}
- return count;
+unlock:
+ spin_unlock_irqrestore(&ch->lock, flags);
+exit:
+ return ret ?: count;
}
static ssize_t role_show(struct device *dev, struct device_attribute *attr,
@@ -441,14 +485,11 @@ static ssize_t role_show(struct device *dev, struct device_attribute *attr,
}
static DEVICE_ATTR_RW(role);
-static void rcar_gen3_init_otg(struct rcar_gen3_chan *ch)
+static void rcar_gen3_init_otg_phase0(struct rcar_gen3_chan *ch)
{
void __iomem *usb2_base = ch->base;
u32 val;
- if (!ch->is_otg_channel || rcar_gen3_is_any_otg_rphy_initialized(ch))
- return;
-
/* Should not use functions of read-modify-write a register */
val = readl(usb2_base + USB2_LINECTRL1);
val = (val & ~USB2_LINECTRL1_DP_RPD) | USB2_LINECTRL1_DPRPD_EN |
@@ -471,7 +512,11 @@ static void rcar_gen3_init_otg(struct rcar_gen3_chan *ch)
writel(val | USB2_ADPCTRL_IDPULLUP, usb2_base + USB2_ADPCTRL);
}
}
- mdelay(20);
+}
+
+static void rcar_gen3_init_otg_phase1(struct rcar_gen3_chan *ch)
+{
+ void __iomem *usb2_base = ch->base;
writel(0xffffffff, usb2_base + USB2_OBINTSTA);
writel(ch->phy_data->obint_enable_bits, usb2_base + USB2_OBINTEN);
@@ -502,6 +547,7 @@ static irqreturn_t rcar_gen3_phy_usb2_irq(int irq, void *_ch)
void __iomem *usb2_base = ch->base;
struct device *dev = ch->dev;
irqreturn_t ret = IRQ_NONE;
+ unsigned long flags;
u32 status;
pm_runtime_get_noresume(dev);
@@ -509,33 +555,91 @@ static irqreturn_t rcar_gen3_phy_usb2_irq(int irq, void *_ch)
if (pm_runtime_suspended(dev))
goto rpm_put;
- scoped_guard(spinlock, &ch->lock) {
- status = readl(usb2_base + USB2_OBINTSTA);
- if (status & ch->phy_data->obint_enable_bits) {
- dev_vdbg(dev, "%s: %08x\n", __func__, status);
- if (ch->phy_data->vblvl_ctrl)
- writel(USB2_OBINTSTA_CLEAR, usb2_base + USB2_OBINTSTA);
- else
- writel(ch->phy_data->obint_enable_bits, usb2_base + USB2_OBINTSTA);
- rcar_gen3_device_recognition(ch);
- rcar_gen3_configure_vblvl_ctrl(ch);
- ret = IRQ_HANDLED;
- }
+ spin_lock_irqsave(&ch->lock, flags);
+
+ status = readl(usb2_base + USB2_OBINTSTA);
+ if (status & ch->phy_data->obint_enable_bits) {
+ dev_vdbg(dev, "%s: %08x\n", __func__, status);
+ if (ch->phy_data->vblvl_ctrl)
+ writel(USB2_OBINTSTA_CLEAR, usb2_base + USB2_OBINTSTA);
+ else
+ writel(ch->phy_data->obint_enable_bits, usb2_base + USB2_OBINTSTA);
+
+ ret = IRQ_HANDLED;
+
+ /* This should not happen! */
+ if (ch->otg_initializing)
+ goto unlock;
+
+ rcar_gen3_device_recognition(ch);
+ rcar_gen3_configure_vblvl_ctrl(ch);
}
+unlock:
+ spin_unlock_irqrestore(&ch->lock, flags);
rpm_put:
pm_runtime_put_noidle(dev);
return ret;
}
+static void rcar_gen3_phy_usb2_irqs_mask_all(struct rcar_gen3_chan *channel,
+ u32 *masked_irqs_bits)
+{
+ u32 val, bitmask = USB2_INT_ENABLE_UCOM_INTEN;
+ void __iomem *usb2_base = channel->base;
+
+ for (unsigned int i = 0; i < NUM_OF_PHYS; i++)
+ bitmask |= channel->rphys[i].int_enable_bits;
+
+ val = readl(usb2_base + USB2_INT_ENABLE);
+ *masked_irqs_bits = val & bitmask;
+ /*
+ * Don't report channel->phy_data->obint_enable_bits IRQs. These are
+ * unmasked anyway in rcar_gen3_init_otg_phase1().
+ */
+ val &= ~(bitmask | channel->phy_data->obint_enable_bits);
+ writel(val, usb2_base + USB2_INT_ENABLE);
+}
+
+static void rcar_gen3_phy_usb2_irqs_unmask(struct rcar_gen3_chan *channel,
+ u32 irqs_bits)
+{
+ u32 val, bitmask = USB2_INT_ENABLE_UCOM_INTEN;
+ void __iomem *usb2_base = channel->base;
+
+ for (unsigned int i = 0; i < NUM_OF_PHYS; i++)
+ bitmask |= channel->rphys[i].int_enable_bits;
+
+ val = readl(usb2_base + USB2_INT_ENABLE);
+ val &= ~bitmask;
+ val |= irqs_bits;
+ writel(val, usb2_base + USB2_INT_ENABLE);
+}
+
static int rcar_gen3_phy_usb2_init(struct phy *p)
{
struct rcar_gen3_phy *rphy = phy_get_drvdata(p);
struct rcar_gen3_chan *channel = rphy->ch;
void __iomem *usb2_base = channel->base;
+ int retries = NUM_OF_PHYS;
+ unsigned long flags;
u32 val;
- guard(spinlock_irqsave)(&channel->lock);
+ spin_lock_irqsave(&channel->lock, flags);
+
+ while (retries-- && channel->otg_initializing) {
+ int ret;
+
+ spin_unlock_irqrestore(&channel->lock, flags);
+
+ ret = wait_for_completion_timeout(&channel->otg_init_done,
+ USB2_OTG_INIT_TIMEOUT);
+ ret = ret ? 0 : -ETIMEDOUT;
+ if (ret && !retries)
+ return ret;
+
+ spin_lock_irqsave(&channel->lock, flags);
+ }
/* Initialize USB2 part */
val = readl(usb2_base + USB2_INT_ENABLE);
@@ -548,8 +652,24 @@ static int rcar_gen3_phy_usb2_init(struct phy *p)
}
/* Initialize otg part (only if we initialize a PHY with IRQs). */
- if (rphy->int_enable_bits)
- rcar_gen3_init_otg(channel);
+ if (rphy->int_enable_bits && channel->is_otg_channel &&
+ !rcar_gen3_is_any_otg_rphy_initialized(channel)) {
+ u32 masked_irq_bits = 0;
+
+ rcar_gen3_init_otg_phase0(channel);
+ rcar_gen3_phy_usb2_irqs_mask_all(channel, &masked_irq_bits);
+ reinit_completion(&channel->otg_init_done);
+ channel->otg_initializing = true;
+ spin_unlock_irqrestore(&channel->lock, flags);
+
+ fsleep(20000);
+
+ spin_lock_irqsave(&channel->lock, flags);
+ channel->otg_initializing = false;
+ complete_all(&channel->otg_init_done);
+ rcar_gen3_phy_usb2_irqs_unmask(channel, masked_irq_bits);
+ rcar_gen3_init_otg_phase1(channel);
+ }
if (channel->phy_data->vblvl_ctrl) {
/* SIDDQ mode release */
@@ -568,6 +688,8 @@ static int rcar_gen3_phy_usb2_init(struct phy *p)
rphy->initialized = true;
+ spin_unlock_irqrestore(&channel->lock, flags);
+
return 0;
}
@@ -576,9 +698,25 @@ static int rcar_gen3_phy_usb2_exit(struct phy *p)
struct rcar_gen3_phy *rphy = phy_get_drvdata(p);
struct rcar_gen3_chan *channel = rphy->ch;
void __iomem *usb2_base = channel->base;
+ int retries = NUM_OF_PHYS;
+ unsigned long flags;
u32 val;
- guard(spinlock_irqsave)(&channel->lock);
+ spin_lock_irqsave(&channel->lock, flags);
+
+ while (retries-- && channel->otg_initializing) {
+ int ret;
+
+ spin_unlock_irqrestore(&channel->lock, flags);
+
+ ret = wait_for_completion_timeout(&channel->otg_init_done,
+ USB2_OTG_INIT_TIMEOUT);
+ ret = ret ? 0 : -ETIMEDOUT;
+ if (ret && !retries)
+ return ret;
+
+ spin_lock_irqsave(&channel->lock, flags);
+ }
rphy->initialized = false;
@@ -588,6 +726,7 @@ static int rcar_gen3_phy_usb2_exit(struct phy *p)
val &= ~USB2_INT_ENABLE_UCOM_INTEN;
writel(val, usb2_base + USB2_INT_ENABLE);
+ spin_unlock_irqrestore(&channel->lock, flags);
return 0;
}
@@ -596,6 +735,8 @@ static int rcar_gen3_phy_usb2_power_on(struct phy *p)
struct rcar_gen3_phy *rphy = phy_get_drvdata(p);
struct rcar_gen3_chan *channel = rphy->ch;
void __iomem *usb2_base = channel->base;
+ int retries = NUM_OF_PHYS;
+ unsigned long flags;
u32 val;
int ret = 0;
@@ -605,7 +746,19 @@ static int rcar_gen3_phy_usb2_power_on(struct phy *p)
return ret;
}
- guard(spinlock_irqsave)(&channel->lock);
+ spin_lock_irqsave(&channel->lock, flags);
+
+ while (retries-- && channel->otg_initializing) {
+ spin_unlock_irqrestore(&channel->lock, flags);
+
+ ret = wait_for_completion_timeout(&channel->otg_init_done,
+ USB2_OTG_INIT_TIMEOUT);
+ ret = ret ? 0 : -ETIMEDOUT;
+ if (ret && !retries)
+ goto disable_regulator;
+
+ spin_lock_irqsave(&channel->lock, flags);
+ }
if (!rcar_gen3_are_all_rphys_power_off(channel))
goto out;
@@ -620,27 +773,49 @@ static int rcar_gen3_phy_usb2_power_on(struct phy *p)
/* The powered flag should be set for any other phys anyway */
rphy->powered = true;
- return 0;
+ spin_unlock_irqrestore(&channel->lock, flags);
+
+disable_regulator:
+ if (ret && channel->vbus && !channel->otg_internal_reg)
+ regulator_disable(channel->vbus);
+
+ return ret;
}
static int rcar_gen3_phy_usb2_power_off(struct phy *p)
{
struct rcar_gen3_phy *rphy = phy_get_drvdata(p);
struct rcar_gen3_chan *channel = rphy->ch;
+ int retries = NUM_OF_PHYS;
+ unsigned long flags;
int ret = 0;
- scoped_guard(spinlock_irqsave, &channel->lock) {
- rphy->powered = false;
+ spin_lock_irqsave(&channel->lock, flags);
- if (rcar_gen3_are_all_rphys_power_off(channel)) {
- u32 val = readl(channel->base + USB2_USBCTR);
+ while (retries-- && channel->otg_initializing) {
+ spin_unlock_irqrestore(&channel->lock, flags);
- val |= USB2_USBCTR_PLL_RST;
- writel(val, channel->base + USB2_USBCTR);
- }
+ ret = wait_for_completion_timeout(&channel->otg_init_done,
+ USB2_OTG_INIT_TIMEOUT);
+ ret = ret ? 0 : -ETIMEDOUT;
+ if (ret && !retries)
+ return ret;
+
+ spin_lock_irqsave(&channel->lock, flags);
}
- if (channel->vbus && !channel->otg_internal_reg)
+ rphy->powered = false;
+
+ if (rcar_gen3_are_all_rphys_power_off(channel)) {
+ u32 val = readl(channel->base + USB2_USBCTR);
+
+ val |= USB2_USBCTR_PLL_RST;
+ writel(val, channel->base + USB2_USBCTR);
+ }
+
+ spin_unlock_irqrestore(&channel->lock, flags);
+
+ if (!ret && channel->vbus && !channel->otg_internal_reg)
ret = regulator_disable(channel->vbus);
return ret;
@@ -1007,6 +1182,7 @@ static int rcar_gen3_phy_usb2_probe(struct platform_device *pdev)
return ret;
spin_lock_init(&channel->lock);
+ init_completion(&channel->otg_init_done);
for (i = 0; i < NUM_OF_PHYS; i++) {
channel->rphys[i].phy = devm_phy_create(dev, NULL,
channel->phy_data->phy_usb2_ops);
--
2.43.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* Re: [PATCH 1/2] dt-bindings: clock: Drop incorrect usage of double '::'
From: Peter Griffin @ 2026-07-09 13:56 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Alim Akhtar, Michael Turquette, Stephen Boyd,
Brian Masney, Sylwester Nawrocki, Chanwoo Choi, Sam Protsenko,
Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
Sean Paul, Marijn Suijten, David Airlie, Simona Vetter,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, Inki Dae,
Seung-Woo Kim, Kyungmin Park, Andi Shyti, Georgi Djakov,
Lee Jones, Pavel Machek, Hans Verkuil, Mauro Carvalho Chehab,
Ulf Hansson, Peter Rosin, Vinod Koul, Neil Armstrong,
Linus Walleij, Geert Uytterhoeven, Magnus Damm, Sebastian Reichel,
Javier Martinez Canillas, Liam Girdwood, Mark Brown,
Greg Kroah-Hartman, Jiri Slaby, Srinivas Kandagatla,
Bartlomiej Zolnierkiewicz, Rafael J. Wysocki, Daniel Lezcano,
Zhang Rui, Lukasz Luba, Jonathan Marek, Taniya Das, Robert Marko,
Christian Marangi, Stephan Gerhold, Adam Skladowski,
Sireesh Kodali, Barnabas Czeman, Imran Shaik,
Sricharan Ramabadhran, Anusha Rao, Luo Jie, Tomasz Figa,
Chanho Park, Sunyeal Hong, Shin Son, Krishna Manikandan,
Jacek Anaszewski, Jaehoon Chung, Marek Szyprowski, Alina Yu,
Andy Gross, Niklas Söderlund, Wesley Cheng, linux-arm-msm,
devicetree, linux-kernel, linux-arm-kernel, linux-samsung-soc,
linux-clk, dri-devel, freedreno, linux-i2c, linux-pm, linux-leds,
linux-media, linux-mmc, linux-phy, linux-gpio, linux-renesas-soc,
linux-serial, linux-sound, linux-usb
In-Reply-To: <20260622101606.485961-3-krzysztof.kozlowski@oss.qualcomm.com>
On Mon, 22 Jun 2026 at 11:16, Krzysztof Kozlowski
<krzysztof.kozlowski@oss.qualcomm.com> wrote:
>
> There is no use of double colon '::' in YAML. OTOH, the literal style
> block, e.g. using '|' treats all characters as content [1] therefore
> single use of ':' in descriptions is perfectly fine, whenever '|' is
> used.
>
> Cleanup existing code, so the confusing style won't be re-used in new
> contributions.
>
> Link: https://yaml.org/spec/1.2.2/#literal-style [1]
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
>
> ---
>
> I split the patches to avoid bounces from mailing list due to email size.
>
> This can go via clock tree (no dependencies)... or both could go via
> Rob's tree.
> ---
Reviewed-by: Peter Griffin <peter.griffin@linaro.org> (for Samsung parts)
> .../devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml | 2 +-
> .../devicetree/bindings/clock/qcom,gcc-apq8064.yaml | 2 +-
> .../devicetree/bindings/clock/qcom,gcc-apq8084.yaml | 2 +-
> .../devicetree/bindings/clock/qcom,gcc-ipq6018.yaml | 2 +-
> .../devicetree/bindings/clock/qcom,gcc-ipq8064.yaml | 2 +-
> .../devicetree/bindings/clock/qcom,gcc-mdm9607.yaml | 2 +-
> .../devicetree/bindings/clock/qcom,gcc-mdm9615.yaml | 2 +-
> .../devicetree/bindings/clock/qcom,gcc-msm8660.yaml | 2 +-
> .../devicetree/bindings/clock/qcom,gcc-msm8909.yaml | 2 +-
> .../devicetree/bindings/clock/qcom,gcc-msm8916.yaml | 2 +-
> .../devicetree/bindings/clock/qcom,gcc-msm8953.yaml | 2 +-
> .../devicetree/bindings/clock/qcom,gcc-msm8974.yaml | 2 +-
> .../devicetree/bindings/clock/qcom,gcc-sdm660.yaml | 2 +-
> Documentation/devicetree/bindings/clock/qcom,gpucc.yaml | 2 +-
> .../devicetree/bindings/clock/qcom,ipq5018-gcc.yaml | 2 +-
> .../devicetree/bindings/clock/qcom,ipq9574-gcc.yaml | 2 +-
> .../devicetree/bindings/clock/qcom,qca8k-nsscc.yaml | 2 +-
> .../devicetree/bindings/clock/qcom,qcm2290-gpucc.yaml | 2 +-
> Documentation/devicetree/bindings/clock/qcom,rpmcc.yaml | 2 +-
> .../devicetree/bindings/clock/qcom,sc7280-lpasscorecc.yaml | 2 +-
> .../devicetree/bindings/clock/qcom,sc8280xp-lpasscc.yaml | 2 +-
> .../devicetree/bindings/clock/qcom,sm6115-lpasscc.yaml | 2 +-
> .../devicetree/bindings/clock/qcom,sm8350-videocc.yaml | 2 +-
> Documentation/devicetree/bindings/clock/qcom,videocc.yaml | 2 +-
> .../devicetree/bindings/clock/samsung,exynos5260-clock.yaml | 6 +++---
> .../devicetree/bindings/clock/samsung,exynos5410-clock.yaml | 2 +-
> .../devicetree/bindings/clock/samsung,exynos5433-clock.yaml | 2 +-
> .../devicetree/bindings/clock/samsung,exynos7-clock.yaml | 2 +-
> .../devicetree/bindings/clock/samsung,exynos850-clock.yaml | 2 +-
> .../bindings/clock/samsung,exynosautov9-clock.yaml | 2 +-
> .../bindings/clock/samsung,exynosautov920-clock.yaml | 2 +-
> .../devicetree/bindings/clock/samsung,s5pv210-clock.yaml | 2 +-
> 32 files changed, 34 insertions(+), 34 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml
> index 53a5ab319159..6863db9bd092 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml
> +++ b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml
> @@ -13,7 +13,7 @@ description: |
> Qualcomm display clock control module provides the clocks, resets and power
> domains on SM8150/SM8250/SM8350.
>
> - See also::
> + See also:
> include/dt-bindings/clock/qcom,dispcc-sm8150.h
> include/dt-bindings/clock/qcom,dispcc-sm8250.h
> include/dt-bindings/clock/qcom,dispcc-sm8350.h
> diff --git a/Documentation/devicetree/bindings/clock/qcom,gcc-apq8064.yaml b/Documentation/devicetree/bindings/clock/qcom,gcc-apq8064.yaml
> index 27df7e3e5bf3..68532244901e 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,gcc-apq8064.yaml
> +++ b/Documentation/devicetree/bindings/clock/qcom,gcc-apq8064.yaml
> @@ -14,7 +14,7 @@ description: |
> Qualcomm global clock control module provides the clocks, resets and power
> domains on APQ8064.
>
> - See also::
> + See also:
> include/dt-bindings/clock/qcom,gcc-msm8960.h
> include/dt-bindings/reset/qcom,gcc-msm8960.h
>
> diff --git a/Documentation/devicetree/bindings/clock/qcom,gcc-apq8084.yaml b/Documentation/devicetree/bindings/clock/qcom,gcc-apq8084.yaml
> index 0a0a26d9beab..1c022e75fd71 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,gcc-apq8084.yaml
> +++ b/Documentation/devicetree/bindings/clock/qcom,gcc-apq8084.yaml
> @@ -14,7 +14,7 @@ description: |
> Qualcomm global clock control module provides the clocks, resets and power
> domains on APQ8084.
>
> - See also::
> + See also:
> include/dt-bindings/clock/qcom,gcc-apq8084.h
> include/dt-bindings/reset/qcom,gcc-apq8084.h
>
> diff --git a/Documentation/devicetree/bindings/clock/qcom,gcc-ipq6018.yaml b/Documentation/devicetree/bindings/clock/qcom,gcc-ipq6018.yaml
> index 4d2614d4f368..c7fb84438db7 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,gcc-ipq6018.yaml
> +++ b/Documentation/devicetree/bindings/clock/qcom,gcc-ipq6018.yaml
> @@ -15,7 +15,7 @@ description: |
> Qualcomm global clock control module provides the clocks, resets and power
> domains on IPQ6018.
>
> - See also::
> + See also:
> include/dt-bindings/clock/qcom,gcc-ipq6018.h
> include/dt-bindings/reset/qcom,gcc-ipq6018.h
>
> diff --git a/Documentation/devicetree/bindings/clock/qcom,gcc-ipq8064.yaml b/Documentation/devicetree/bindings/clock/qcom,gcc-ipq8064.yaml
> index a71557395c01..b4d3175780bc 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,gcc-ipq8064.yaml
> +++ b/Documentation/devicetree/bindings/clock/qcom,gcc-ipq8064.yaml
> @@ -13,7 +13,7 @@ description: |
> Qualcomm global clock control module provides the clocks, resets and power
> domains on IPQ8064.
>
> - See also::
> + See also:
> include/dt-bindings/clock/qcom,gcc-ipq806x.h (qcom,gcc-ipq8064)
> include/dt-bindings/reset/qcom,gcc-ipq806x.h (qcom,gcc-ipq8064)
>
> diff --git a/Documentation/devicetree/bindings/clock/qcom,gcc-mdm9607.yaml b/Documentation/devicetree/bindings/clock/qcom,gcc-mdm9607.yaml
> index d7da30b0e7ee..0a7be7583bdd 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,gcc-mdm9607.yaml
> +++ b/Documentation/devicetree/bindings/clock/qcom,gcc-mdm9607.yaml
> @@ -14,7 +14,7 @@ description: |
> Qualcomm global clock control module provides the clocks, resets and power
> domains.
>
> - See also::
> + See also:
> include/dt-bindings/clock/qcom,gcc-mdm9607.h
>
> allOf:
> diff --git a/Documentation/devicetree/bindings/clock/qcom,gcc-mdm9615.yaml b/Documentation/devicetree/bindings/clock/qcom,gcc-mdm9615.yaml
> index 418dea31eb62..0656d5ee448d 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,gcc-mdm9615.yaml
> +++ b/Documentation/devicetree/bindings/clock/qcom,gcc-mdm9615.yaml
> @@ -14,7 +14,7 @@ description: |
> Qualcomm global clock control module provides the clocks, resets and power
> domains.
>
> - See also::
> + See also:
> include/dt-bindings/clock/qcom,gcc-mdm9615.h
>
> allOf:
> diff --git a/Documentation/devicetree/bindings/clock/qcom,gcc-msm8660.yaml b/Documentation/devicetree/bindings/clock/qcom,gcc-msm8660.yaml
> index e03b6d0acdb6..70c9da1f35c2 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,gcc-msm8660.yaml
> +++ b/Documentation/devicetree/bindings/clock/qcom,gcc-msm8660.yaml
> @@ -14,7 +14,7 @@ description: |
> Qualcomm global clock control module provides the clocks and resets on
> MSM8660
>
> - See also::
> + See also:
> include/dt-bindings/clock/qcom,gcc-msm8660.h
> include/dt-bindings/reset/qcom,gcc-msm8660.h
>
> diff --git a/Documentation/devicetree/bindings/clock/qcom,gcc-msm8909.yaml b/Documentation/devicetree/bindings/clock/qcom,gcc-msm8909.yaml
> index ce1f5a60bd8c..2edb6c251d99 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,gcc-msm8909.yaml
> +++ b/Documentation/devicetree/bindings/clock/qcom,gcc-msm8909.yaml
> @@ -13,7 +13,7 @@ description: |
> Qualcomm global clock control module provides the clocks, resets and power
> domains on MSM8909, MSM8917 or QM215.
>
> - See also::
> + See also:
> include/dt-bindings/clock/qcom,gcc-msm8909.h
> include/dt-bindings/clock/qcom,gcc-msm8917.h
>
> diff --git a/Documentation/devicetree/bindings/clock/qcom,gcc-msm8916.yaml b/Documentation/devicetree/bindings/clock/qcom,gcc-msm8916.yaml
> index 258b6b93deca..af4b639ea8c3 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,gcc-msm8916.yaml
> +++ b/Documentation/devicetree/bindings/clock/qcom,gcc-msm8916.yaml
> @@ -14,7 +14,7 @@ description: |
> Qualcomm global clock control module provides the clocks, resets and power
> domains on MSM8916 or MSM8939.
>
> - See also::
> + See also:
> include/dt-bindings/clock/qcom,gcc-msm8916.h
> include/dt-bindings/clock/qcom,gcc-msm8939.h
> include/dt-bindings/reset/qcom,gcc-msm8916.h
> diff --git a/Documentation/devicetree/bindings/clock/qcom,gcc-msm8953.yaml b/Documentation/devicetree/bindings/clock/qcom,gcc-msm8953.yaml
> index ced3118c8580..fc0360554f68 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,gcc-msm8953.yaml
> +++ b/Documentation/devicetree/bindings/clock/qcom,gcc-msm8953.yaml
> @@ -15,7 +15,7 @@ description: |
> Qualcomm global clock control module provides the clocks, resets and power
> domains on MSM8937, MSM8940, MSM8953 or SDM439.
>
> - See also::
> + See also:
> include/dt-bindings/clock/qcom,gcc-msm8917.h
> include/dt-bindings/clock/qcom,gcc-msm8953.h
>
> diff --git a/Documentation/devicetree/bindings/clock/qcom,gcc-msm8974.yaml b/Documentation/devicetree/bindings/clock/qcom,gcc-msm8974.yaml
> index 929fafc84c19..378dfe7854ac 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,gcc-msm8974.yaml
> +++ b/Documentation/devicetree/bindings/clock/qcom,gcc-msm8974.yaml
> @@ -15,7 +15,7 @@ description: |
> Qualcomm global clock control module provides the clocks, resets and power
> domains on MSM8974 (all variants) and MSM8226.
>
> - See also::
> + See also:
> include/dt-bindings/clock/qcom,gcc-msm8974.h (qcom,gcc-msm8226 and qcom,gcc-msm8974)
> include/dt-bindings/reset/qcom,gcc-msm8974.h (qcom,gcc-msm8226 and qcom,gcc-msm8974)
>
> diff --git a/Documentation/devicetree/bindings/clock/qcom,gcc-sdm660.yaml b/Documentation/devicetree/bindings/clock/qcom,gcc-sdm660.yaml
> index 724ce0491118..72aaf699cf70 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,gcc-sdm660.yaml
> +++ b/Documentation/devicetree/bindings/clock/qcom,gcc-sdm660.yaml
> @@ -14,7 +14,7 @@ description: |
> Qualcomm global clock control module provides the clocks, resets and power
> domains on SDM630, SDM636 and SDM660
>
> - See also::
> + See also:
> include/dt-bindings/clock/qcom,gcc-sdm660.h (qcom,gcc-sdm630 and qcom,gcc-sdm660)
>
> $ref: qcom,gcc.yaml#
> diff --git a/Documentation/devicetree/bindings/clock/qcom,gpucc.yaml b/Documentation/devicetree/bindings/clock/qcom,gpucc.yaml
> index 4cdff6161bf0..3ac4419009a9 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,gpucc.yaml
> +++ b/Documentation/devicetree/bindings/clock/qcom,gpucc.yaml
> @@ -14,7 +14,7 @@ description: |
> Qualcomm graphics clock control module provides the clocks, resets and power
> domains on Qualcomm SoCs.
>
> - See also::
> + See also:
> include/dt-bindings/clock/qcom,gpucc-sdm845.h
> include/dt-bindings/clock/qcom,gpucc-sa8775p.h
> include/dt-bindings/clock/qcom,gpucc-sc7180.h
> diff --git a/Documentation/devicetree/bindings/clock/qcom,ipq5018-gcc.yaml b/Documentation/devicetree/bindings/clock/qcom,ipq5018-gcc.yaml
> index 489d0fc5607c..9925b931ecad 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,ipq5018-gcc.yaml
> +++ b/Documentation/devicetree/bindings/clock/qcom,ipq5018-gcc.yaml
> @@ -13,7 +13,7 @@ description: |
> Qualcomm global clock control module provides the clocks, resets and power
> domains on IPQ5018
>
> - See also::
> + See also:
> include/dt-bindings/clock/qcom,ipq5018-gcc.h
> include/dt-bindings/reset/qcom,ipq5018-gcc.h
>
> diff --git a/Documentation/devicetree/bindings/clock/qcom,ipq9574-gcc.yaml b/Documentation/devicetree/bindings/clock/qcom,ipq9574-gcc.yaml
> index 27ae9938febc..5b128fa841aa 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,ipq9574-gcc.yaml
> +++ b/Documentation/devicetree/bindings/clock/qcom,ipq9574-gcc.yaml
> @@ -14,7 +14,7 @@ description: |
> Qualcomm global clock control module provides the clocks, resets and power
> domains on IPQ9574
>
> - See also::
> + See also:
> include/dt-bindings/clock/qcom,ipq9574-gcc.h
> include/dt-bindings/reset/qcom,ipq9574-gcc.h
>
> diff --git a/Documentation/devicetree/bindings/clock/qcom,qca8k-nsscc.yaml b/Documentation/devicetree/bindings/clock/qcom,qca8k-nsscc.yaml
> index 61473385da2d..3da10c364a85 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,qca8k-nsscc.yaml
> +++ b/Documentation/devicetree/bindings/clock/qcom,qca8k-nsscc.yaml
> @@ -14,7 +14,7 @@ description: |
> Qualcomm NSS clock control module provides the clocks and resets
> on QCA8386(switch mode)/QCA8084(PHY mode)
>
> - See also::
> + See also:
> include/dt-bindings/clock/qcom,qca8k-nsscc.h
> include/dt-bindings/reset/qcom,qca8k-nsscc.h
>
> diff --git a/Documentation/devicetree/bindings/clock/qcom,qcm2290-gpucc.yaml b/Documentation/devicetree/bindings/clock/qcom,qcm2290-gpucc.yaml
> index 734880805c1b..bedbdabef672 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,qcm2290-gpucc.yaml
> +++ b/Documentation/devicetree/bindings/clock/qcom,qcm2290-gpucc.yaml
> @@ -13,7 +13,7 @@ description: |
> Qualcomm graphics clock control module provides the clocks, resets and power
> domains on Qualcomm SoCs.
>
> - See also::
> + See also:
> include/dt-bindings/clock/qcom,qcm2290-gpucc.h
>
> properties:
> diff --git a/Documentation/devicetree/bindings/clock/qcom,rpmcc.yaml b/Documentation/devicetree/bindings/clock/qcom,rpmcc.yaml
> index ab97d4b7dba8..b6c835bfd0d9 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,rpmcc.yaml
> +++ b/Documentation/devicetree/bindings/clock/qcom,rpmcc.yaml
> @@ -12,7 +12,7 @@ maintainers:
>
> description: |
> The clock enumerators are defined in <dt-bindings/clock/qcom,rpmcc.h> and
> - come in pairs:: FOO_CLK followed by FOO_A_CLK. The latter clock is
> + come in pairs: FOO_CLK followed by FOO_A_CLK. The latter clock is
> an "active" clock, which means that the consumer only care that the clock is
> available when the apps CPU subsystem is active, i.e. not suspended or in
> deep idle. If it is important that the clock keeps running during system
> diff --git a/Documentation/devicetree/bindings/clock/qcom,sc7280-lpasscorecc.yaml b/Documentation/devicetree/bindings/clock/qcom,sc7280-lpasscorecc.yaml
> index 99ab9106009f..fd06ac9bceb9 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,sc7280-lpasscorecc.yaml
> +++ b/Documentation/devicetree/bindings/clock/qcom,sc7280-lpasscorecc.yaml
> @@ -13,7 +13,7 @@ description: |
> Qualcomm LPASS core and audio clock control module provides the clocks and
> power domains on SC7280.
>
> - See also::
> + See also:
> include/dt-bindings/clock/qcom,lpasscorecc-sc7280.h
> include/dt-bindings/clock/qcom,lpassaudiocc-sc7280.h
>
> diff --git a/Documentation/devicetree/bindings/clock/qcom,sc8280xp-lpasscc.yaml b/Documentation/devicetree/bindings/clock/qcom,sc8280xp-lpasscc.yaml
> index 273d66e245c5..f235b4e24cc7 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,sc8280xp-lpasscc.yaml
> +++ b/Documentation/devicetree/bindings/clock/qcom,sc8280xp-lpasscc.yaml
> @@ -13,7 +13,7 @@ description: |
> Qualcomm LPASS core and audio clock control module provides the clocks,
> and reset on SC8280XP.
>
> - See also::
> + See also:
> include/dt-bindings/clock/qcom,lpasscc-sc8280xp.h
>
> properties:
> diff --git a/Documentation/devicetree/bindings/clock/qcom,sm6115-lpasscc.yaml b/Documentation/devicetree/bindings/clock/qcom,sm6115-lpasscc.yaml
> index 8cbab3fbb660..d7e1938b5e1b 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,sm6115-lpasscc.yaml
> +++ b/Documentation/devicetree/bindings/clock/qcom,sm6115-lpasscc.yaml
> @@ -14,7 +14,7 @@ description: |
> Qualcomm LPASS core and audio clock controllers provide audio-related resets
> on SM6115 and its derivatives.
>
> - See also::
> + See also:
> include/dt-bindings/clock/qcom,sm6115-lpasscc.h
>
> properties:
> diff --git a/Documentation/devicetree/bindings/clock/qcom,sm8350-videocc.yaml b/Documentation/devicetree/bindings/clock/qcom,sm8350-videocc.yaml
> index 5c2ecec0624e..a986ab4ce7c7 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,sm8350-videocc.yaml
> +++ b/Documentation/devicetree/bindings/clock/qcom,sm8350-videocc.yaml
> @@ -13,7 +13,7 @@ description: |
> Qualcomm video clock control module provides the clocks, resets and power
> domains on Qualcomm SoCs.
>
> - See also::
> + See also:
> include/dt-bindings/clock/qcom,videocc-sm8350.h
> include/dt-bindings/reset/qcom,videocc-sm8350.h
>
> diff --git a/Documentation/devicetree/bindings/clock/qcom,videocc.yaml b/Documentation/devicetree/bindings/clock/qcom,videocc.yaml
> index f4ff9acef9d5..124d259fc85e 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,videocc.yaml
> +++ b/Documentation/devicetree/bindings/clock/qcom,videocc.yaml
> @@ -13,7 +13,7 @@ description: |
> Qualcomm video clock control module provides the clocks, resets and power
> domains on Qualcomm SoCs.
>
> - See also::
> + See also:
> include/dt-bindings/clock/qcom,sm6350-videocc.h
> include/dt-bindings/clock/qcom,videocc-sc7180.h
> include/dt-bindings/clock/qcom,videocc-sc7280.h
> diff --git a/Documentation/devicetree/bindings/clock/samsung,exynos5260-clock.yaml b/Documentation/devicetree/bindings/clock/samsung,exynos5260-clock.yaml
> index b05f83533e3d..56ab972c3da5 100644
> --- a/Documentation/devicetree/bindings/clock/samsung,exynos5260-clock.yaml
> +++ b/Documentation/devicetree/bindings/clock/samsung,exynos5260-clock.yaml
> @@ -14,17 +14,17 @@ maintainers:
>
> description: |
> Expected external clocks, defined in DTS as fixed-rate clocks with a matching
> - name::
> + name:
> - "fin_pll" - PLL input clock from XXTI
> - "xrtcxti" - input clock from XRTCXTI
> - "ioclk_pcm_extclk" - pcm external operation clock
> - "ioclk_spdif_extclk" - spdif external operation clock
> - "ioclk_i2s_cdclk" - i2s0 codec clock
>
> - Phy clocks::
> + Phy clocks:
> There are several clocks which are generated by specific PHYs. These clocks
> are fed into the clock controller and then routed to the hardware blocks.
> - These clocks are defined as fixed clocks in the driver with following names::
> + These clocks are defined as fixed clocks in the driver with following names:
> - "phyclk_dptx_phy_ch3_txd_clk" - dp phy clock for channel 3
> - "phyclk_dptx_phy_ch2_txd_clk" - dp phy clock for channel 2
> - "phyclk_dptx_phy_ch1_txd_clk" - dp phy clock for channel 1
> diff --git a/Documentation/devicetree/bindings/clock/samsung,exynos5410-clock.yaml b/Documentation/devicetree/bindings/clock/samsung,exynos5410-clock.yaml
> index b737c9d35a1c..1d907dd8fbf1 100644
> --- a/Documentation/devicetree/bindings/clock/samsung,exynos5410-clock.yaml
> +++ b/Documentation/devicetree/bindings/clock/samsung,exynos5410-clock.yaml
> @@ -14,7 +14,7 @@ maintainers:
>
> description: |
> Expected external clocks, defined in DTS as fixed-rate clocks with a matching
> - name::
> + name:
> - "fin_pll" - PLL input clock from XXTI
>
> All available clocks are defined as preprocessor macros in
> diff --git a/Documentation/devicetree/bindings/clock/samsung,exynos5433-clock.yaml b/Documentation/devicetree/bindings/clock/samsung,exynos5433-clock.yaml
> index 3f9326e09f79..8a289f1e2ace 100644
> --- a/Documentation/devicetree/bindings/clock/samsung,exynos5433-clock.yaml
> +++ b/Documentation/devicetree/bindings/clock/samsung,exynos5433-clock.yaml
> @@ -14,7 +14,7 @@ maintainers:
>
> description: |
> Expected external clocks, defined in DTS as fixed-rate clocks with a matching
> - name::
> + name:
> - "oscclk" - PLL input clock from XXTI
>
> All available clocks are defined as preprocessor macros in
> diff --git a/Documentation/devicetree/bindings/clock/samsung,exynos7-clock.yaml b/Documentation/devicetree/bindings/clock/samsung,exynos7-clock.yaml
> index c137c6744ef9..a51cd4fafb41 100644
> --- a/Documentation/devicetree/bindings/clock/samsung,exynos7-clock.yaml
> +++ b/Documentation/devicetree/bindings/clock/samsung,exynos7-clock.yaml
> @@ -14,7 +14,7 @@ maintainers:
>
> description: |
> Expected external clocks, defined in DTS as fixed-rate clocks with a matching
> - name::
> + name:
> - "fin_pll" - PLL input clock from XXTI
>
> All available clocks are defined as preprocessor macros in
> diff --git a/Documentation/devicetree/bindings/clock/samsung,exynos850-clock.yaml b/Documentation/devicetree/bindings/clock/samsung,exynos850-clock.yaml
> index cdc5ded59fe5..68c2fd318765 100644
> --- a/Documentation/devicetree/bindings/clock/samsung,exynos850-clock.yaml
> +++ b/Documentation/devicetree/bindings/clock/samsung,exynos850-clock.yaml
> @@ -17,7 +17,7 @@ description: |
> Exynos850 clock controller is comprised of several CMU units, generating
> clocks for different domains. Those CMU units are modeled as separate device
> tree nodes, and might depend on each other. Root clocks in that clock tree are
> - two external clocks:: OSCCLK (26 MHz) and RTCCLK (32768 Hz). Those external
> + two external clocks: OSCCLK (26 MHz) and RTCCLK (32768 Hz). Those external
> clocks must be defined as fixed-rate clocks in dts.
>
> CMU_TOP is a top-level CMU, where all base clocks are prepared using PLLs and
> diff --git a/Documentation/devicetree/bindings/clock/samsung,exynosautov9-clock.yaml b/Documentation/devicetree/bindings/clock/samsung,exynosautov9-clock.yaml
> index 32f39e543b36..e9d17d48b4f3 100644
> --- a/Documentation/devicetree/bindings/clock/samsung,exynosautov9-clock.yaml
> +++ b/Documentation/devicetree/bindings/clock/samsung,exynosautov9-clock.yaml
> @@ -17,7 +17,7 @@ description: |
> Exynos Auto v9 clock controller is comprised of several CMU units, generating
> clocks for different domains. Those CMU units are modeled as separate device
> tree nodes, and might depend on each other. Root clocks in that clock tree are
> - two external clocks:: OSCCLK/XTCXO (26 MHz) and RTCCLK/XrtcXTI (32768 Hz).
> + two external clocks: OSCCLK/XTCXO (26 MHz) and RTCCLK/XrtcXTI (32768 Hz).
> The external OSCCLK must be defined as fixed-rate clock in dts.
>
> CMU_TOP is a top-level CMU, where all base clocks are prepared using PLLs and
> diff --git a/Documentation/devicetree/bindings/clock/samsung,exynosautov920-clock.yaml b/Documentation/devicetree/bindings/clock/samsung,exynosautov920-clock.yaml
> index 6b1fc61a2ff9..475db824d4d3 100644
> --- a/Documentation/devicetree/bindings/clock/samsung,exynosautov920-clock.yaml
> +++ b/Documentation/devicetree/bindings/clock/samsung,exynosautov920-clock.yaml
> @@ -17,7 +17,7 @@ description: |
> ExynosAuto v920 clock controller is comprised of several CMU units, generating
> clocks for different domains. Those CMU units are modeled as separate device
> tree nodes, and might depend on each other. Root clocks in that clock tree are
> - two external clocks:: OSCCLK/XTCXO (38.4 MHz) and RTCCLK/XrtcXTI (32768 Hz).
> + two external clocks: OSCCLK/XTCXO (38.4 MHz) and RTCCLK/XrtcXTI (32768 Hz).
> The external OSCCLK must be defined as fixed-rate clock in dts.
>
> CMU_TOP is a top-level CMU, where all base clocks are prepared using PLLs and
> diff --git a/Documentation/devicetree/bindings/clock/samsung,s5pv210-clock.yaml b/Documentation/devicetree/bindings/clock/samsung,s5pv210-clock.yaml
> index 67a33665cf00..b1617d96d3fb 100644
> --- a/Documentation/devicetree/bindings/clock/samsung,s5pv210-clock.yaml
> +++ b/Documentation/devicetree/bindings/clock/samsung,s5pv210-clock.yaml
> @@ -14,7 +14,7 @@ maintainers:
>
> description: |
> Expected external clocks, defined in DTS as fixed-rate clocks with a matching
> - name::
> + name:
> - "xxti" - external crystal oscillator connected to XXTI and XXTO pins of
> the SoC,
> - "xusbxti" - external crystal oscillator connected to XUSBXTI and XUSBXTO
> --
> 2.53.0
>
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v3 4/4] PCI: qcom: enable Link retain logic for Hamoa
From: Konrad Dybcio @ 2026-07-09 13:19 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: <20260709-link_retain-v3-4-81a9d187bb61@oss.qualcomm.com>
On 7/9/26 8:35 AM, Krishna Chaitanya Chundru wrote:
> The Hamoa platform supports keeping the PCIe link active across
> bootloader and kernel handoff. To take advantage of this, introduce a
> specific configuration (cfg_x1e80100) with link_retain = true and
> update the device match table to use it.
>
> Tested-by: Qiang Yu <qiang.yu@oss.qualcomm.com>
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@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 2/2] phy: qcom: qmp-usb: Add support for SA8255P
From: Mattijs Korpershoek @ 2026-07-09 13:00 UTC (permalink / raw)
To: sashiko-reviews
Cc: olteanv, devicetree, robh, linux-phy, vkoul, conor+dt,
neil.armstrong
In-Reply-To: <20260708131123.3F29B1F00A3A@smtp.kernel.org>
On Wed, Jul 08, 2026 at 13:11, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] The driver leaks runtime PM usage counters for its manually managed power domains ('usb_transfer' and 'usb_core') on multiple error paths, leaving hardware permanently active and risking usage counter overflow.
> - [High] Teardown operations disable PM domains before disabling leaf consumer clocks and asserting resets, violating strict reverse-initialization ordering and risking hardware access faults.
> --
>
> --- Patch [2]: [PATCH 2/2] phy: qcom: qmp-usb: Add support for SA8255P ---
> commit 487667fe3c076bbe0851ee1739a5d0f2311634d9
> Author: Sriram Dash <quic_sriramd@quicinc.com>
> Subject: phy: qcom: qmp-usb: Add support for SA8255P
>
> This commit adds support for the SA8255P SoC to the QMP USB PHY driver.
> It introduces firmware-managed power domains that are controlled via
> runtime PM APIs during the driver's power state transitions.
>
>> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-usb.c b/drivers/phy/qualcomm/phy-qcom-qmp-usb.c
>> index c5507168e1354..2ac6a23868bf9 100644
>> --- a/drivers/phy/qualcomm/phy-qcom-qmp-usb.c
>> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-usb.c
> [ ... ]
>> @@ -1890,6 +1890,77 @@ static const struct qmp_phy_cfg glymur_usb3_uniphy_cfg = {
> [ ... ]
>> +/* d3_to_d0 transition by turning on all the suppliers */
>> +static int qmp_usb_d3_to_d0(struct qmp_usb *qmp)
>> +{
>> + int ret;
>> +
>> + if (!qmp->pd_list)
>> + return 0;
>> +
>> + ret = pm_runtime_resume_and_get(qmp->pd_list->pd_devs[0]);
>> + if (ret)
>> + return ret;
>> +
>> + ret = pm_runtime_resume_and_get(qmp->pd_list->pd_devs[1]);
>> +
>> + return ret;
>
> [Severity: High]
> If the resume of pd_devs[1] fails here, will this leak the runtime PM usage
> counter for pd_devs[0]? Should there be a pm_runtime_put_sync() for
> pd_devs[0] before returning the error?
Indeed. Will fix in v2.
>
>> +}
>> +
>> +/* d0_to_d3 transition by turning off all the suppliers */
>> +static void qmp_usb_d0_to_d3(struct qmp_usb *qmp)
>> +{
>> + if (!qmp->pd_list)
>> + return;
>> +
>> + pm_runtime_put_sync(qmp->pd_list->pd_devs[0]);
>> + pm_runtime_put_sync(qmp->pd_list->pd_devs[1]);
>
> [Severity: High]
> Since qmp_usb_d3_to_d0() enables these domains in the order 0 then 1,
> should this disable them in the reverse order (1 then 0)?
Indeed. Will fix in v2.
>
>> +}
> [ ... ]
>> @@ -1980,6 +1980,12 @@ static int qmp_usb_init(struct phy *phy)
>> void __iomem *pcs = qmp->pcs;
>> int ret;
>>
>> + ret = qmp_usb_d3_to_d0(qmp);
>> + if (ret < 0) {
>> + dev_err(qmp->dev, "Failed to transition to d0 state\n");
>> + return ret;
>> + }
>> +
>> ret = regulator_bulk_enable(cfg->num_vregs, qmp->vregs);
>> if (ret) {
>> dev_err(qmp->dev, "failed to enable regulators, err=%d\n", ret);
>> return ret;
>
> [Severity: High]
> If regulator_bulk_enable() fails, this returns directly. Does this leave
> the PM domains enabled, leaking their usage counters since qmp_usb_d0_to_d3()
> is not called to balance the earlier qmp_usb_d3_to_d0() call?
Indeed. Will fix in v2.
>
>> @@ -2025,6 +2025,8 @@ static int qmp_usb_exit(struct phy *phy)
>> struct qmp_usb *qmp = phy_get_drvdata(phy);
>> const struct qmp_phy_cfg *cfg = qmp->cfg;
>>
>> + qmp_usb_d0_to_d3(qmp);
>> +
>> reset_control_bulk_assert(qmp->num_resets, qmp->resets);
>>
>> clk_bulk_disable_unprepare(qmp->num_clks, qmp->clks);
>
> [Severity: High]
> This disables the power domains via qmp_usb_d0_to_d3() before disabling
> the leaf clocks and asserting the resets. Can this sequence cause
> unclocked or unpowered register accesses and trigger hardware faults?
> Should the domains be disabled at the end of the teardown instead?
No because resets and clocks are not used in the firmware-managed mode.
But for consistency, I will move qmp_usb_d0_to_d3() at the end of
qmp_usb_exit() in v2.
>
>> @@ -2212,6 +2212,8 @@ static int __maybe_unused qmp_usb_runtime_suspend(struct device *dev)
>>
>> qmp_usb_enable_autonomous_mode(qmp);
>>
>> + qmp_usb_d0_to_d1(qmp);
>> +
>> clk_disable_unprepare(qmp->pipe_clk);
>> clk_bulk_disable_unprepare(qmp->num_clks, qmp->clks);
>
> [Severity: High]
> Similar to the exit path, this powers off the transfer domain before
> the clocks are disabled. Could this cause bus errors when the clocks are
> manipulated?
Will move after clk_bulk_disable() in v2.
>
>> @@ -2232,6 +2232,12 @@ static int __maybe_unused qmp_usb_runtime_resume(struct device *dev)
>> return 0;
>> }
>>
>> + ret = qmp_usb_d1_to_d0(qmp);
>> + if (ret < 0) {
>> + dev_err(qmp->dev, "Failed to transition to d0 state\n");
>> + return ret;
>> + }
>> +
>> ret = clk_bulk_prepare_enable(qmp->num_clks, qmp->clks);
>> if (ret)
>> return ret;
>
> [Severity: High]
> If clk_bulk_prepare_enable() fails, the function returns ret without
> undoing qmp_usb_d1_to_d0(). Will this leak the transfer domain's PM usage
> counter?
Indeed, will fix in v2.
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260708-phy-qcom-qmp-usb-8255-v1-0-8d852a9535db@kernel.org?part=2
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v5 net-next 1/2] dt-bindings: phy: cadence-torrent: Update property values to support multilink SERDES configuration
From: Praveen, Gokul @ 2026-07-09 10:07 UTC (permalink / raw)
To: Conor Dooley
Cc: conor+dt, devicetree, krzk+dt, linux-arm-kernel, linux-kernel,
linux-phy, neil.armstrong, nm, robh, sjakhade, kristo, vigneshr,
vkoul, yamonkar, g-praveen
In-Reply-To: <20260708-purifier-subside-667f32244c09@spud>
Hi Conor,
On 08-07-2026 22:09, Conor Dooley wrote:
> On Wed, Jul 08, 2026 at 02:07:24PM +0530, Gokul Praveen wrote:
>> Update the maxItems value of clocks parameter as 3 clocks
>> (refclk,pll1_refclk,phy_en_refclk) are supported.
>>
>> Update the clock-names parameter to support mutilink SERDES configuration
>> as the existing enum configuration of the clock-names parameter does not
>> allow both pll1_refclk and phy_en_refclk to be used at the same time,
>> hence preventing the support for the configuration (refclk,pll1_refclk,
>> phy_en_refclk), which is neeed for multilink SERDES usecases.
>>
>> For multilink SERDES configurations where the links require different
>> clock speeds, all 3 clocks(refclk, pll1_refclk and phy_en_refclk)
>> are needed.
>>
>> For example,considering the USXGMII+SGMII multilink SERDES configuration
>> usecase, having only 1 reference clock(refclk) fails because USXGMII
>> requires a clock speed of 156.25 Mhz and SGMII protocol requires an
>> clock speed of 100 Mhz.
>>
>> Since one reference clock(refclk) alone cannot cater to the 2
>> different clock speed requirements of these protocols, the second
>> input reference clock(pll1_refclk) along with phy_en_refclk
>> is also needed.
> This binding supports 2 devices and the generic compatible. Do all these
> devices have the new refclk?
Not all of these devices have the new refclk(pll1_refclk), Conor, which
is is why the enum was kept as it is and in these devices multilink
serdes configuration will not be possible due to the limitation of not
having the new refclk(pll1_refclk).
However, The intent of this patch is to add multilink serdes support for
the devices which have the new refclk because the
earlier clock-names configuration could not support having all the 3
clocks(refclk, pll1_refclk, phy_en_refclk) in the clock-names, which is
needed for mutlilink serdes configuration configuration.
Also, Please feel free to ask if you have any other queries, Conor and
thank you for this query .
Thanks and Best Regards
Gokul Praveen
> Thanks,
> Conor.
>
>> Signed-off-by: Gokul Praveen <g-praveen@ti.com>
>> ---
>> Documentation/devicetree/bindings/phy/phy-cadence-torrent.yaml | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/phy/phy-cadence-torrent.yaml b/Documentation/devicetree/bindings/phy/phy-cadence-torrent.yaml
>> index 9af39b33646a..ac0f625cd76d 100644
>> --- a/Documentation/devicetree/bindings/phy/phy-cadence-torrent.yaml
>> +++ b/Documentation/devicetree/bindings/phy/phy-cadence-torrent.yaml
>> @@ -34,7 +34,7 @@ properties:
>>
>> clocks:
>> minItems: 1
>> - maxItems: 2
>> + maxItems: 3
>> description:
>> PHY input reference clocks - refclk (for PLL0) & pll1_refclk (for PLL1).
>> pll1_refclk is optional and used for multi-protocol configurations requiring
>> @@ -48,6 +48,7 @@ properties:
>> items:
>> - const: refclk
>> - enum: [ pll1_refclk, phy_en_refclk ]
>> + - const: phy_en_refclk
>>
>> reg:
>> minItems: 1
>> --
>> 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 v4 7/9] phy: renesas: phy-rcar-gen3-usb2: Fix devm action registration for disabled VBUS regulator
From: sashiko-bot @ 2026-07-09 8:54 UTC (permalink / raw)
To: Biju; +Cc: linux-phy, neil.armstrong, vkoul, olteanv, wsa+renesas
In-Reply-To: <20260709083108.108370-8-biju.das.jz@bp.renesas.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [High] The condition `if (regulator_is_enabled(channel->vbus))` incorrectly evaluates negative error codes as true, leading to unbalanced disables.
Pre-existing issues:
- [High] If the regulator is both pre-enabled by the bootloader and explicitly enabled by the driver, it leaks an enable_count, triggering a WARN_ON during teardown.
- [High] Missing cleanup for the implicit enable inherited from the bootloader if the explicit regulator_enable() fails.
--
--- Patch [7]: [PATCH v4 7/9] phy: renesas: phy-rcar-gen3-usb2: Fix devm action registration for disabled VBUS regulator ---
commit 3ef9475c853028e498f373a0d82121c2440a2aae
Author: Biju Das <biju.das.jz@bp.renesas.com>
phy: renesas: phy-rcar-gen3-usb2: Fix devm action registration for disabled VBUS regulator
This commit modifies the devm action registration for disabling the VBUS
regulator to rely on regulator_is_enabled() rather than checking if
enable was explicitly requested.
> diff --git a/drivers/phy/renesas/phy-rcar-gen3-usb2.c b/drivers/phy/renesas/phy-rcar-gen3-usb2.c
> index d06fb52ed5f1b..ef38c3b365d42 100644
> --- a/drivers/phy/renesas/phy-rcar-gen3-usb2.c
> +++ b/drivers/phy/renesas/phy-rcar-gen3-usb2.c
[ ... ]
> @@ -905,15 +905,17 @@ static int rcar_gen3_phy_usb2_vbus_regulator_get_exclusive_enable(struct rcar_ge
> if (IS_ERR(channel->vbus))
> return PTR_ERR(channel->vbus);
>
> - if (!enable)
> - return 0;
> + if (enable) {
> + ret = regulator_enable(channel->vbus);
> + if (ret)
> + return ret;
[Severity: High]
This is a pre-existing issue, but does this early return leak the implicit
enable count if the regulator was pre-enabled by the bootloader?
If the bootloader left the physical regulator enabled,
devm_regulator_get_exclusive() initializes the consumer with
enable_count = 1. If regulator_enable() fails here (e.g., due to an I2C
error), the function returns early without registering a devres disable
action.
When devres unwinds the exclusive get and calls regulator_put(), could this
trigger a WARN_ON(regulator->enable_count) since the implicitly acquired
enable_count is never decremented?
> + }
>
> - ret = regulator_enable(channel->vbus);
> - if (ret)
> - return ret;
> + if (regulator_is_enabled(channel->vbus))
[Severity: High]
Could this cause a regression since regulator_is_enabled() can return a
negative error code on failure? Will this condition incorrectly evaluate
negative error codes as true?
If the regulator wasn't pre-enabled (enable_count = 0) and enable is false,
but regulator_is_enabled() fails and returns an error (like -EIO), the
condition evaluates to true and registers the devm disable action.
Upon driver removal, the action would execute regulator_disable(), which
could underflow the count and trigger WARN(regulator->enable_count == 0)
in _regulator_disable(). Should this check explicitly for > 0?
[Severity: High]
This is a pre-existing issue, but if the regulator is both pre-enabled by
the bootloader and explicitly enabled by the driver, does this leak an
enable_count?
When the driver registers its internal regulator, it sets enable = true. If
the bootloader left the physical regulator enabled,
devm_regulator_get_exclusive() implicitly sets enable_count = 1.
The explicit enable path then calls regulator_enable(), incrementing
enable_count to 2.
Since only a single disable action is registered here, during driver
removal it reduces the count to 1. Does the subsequent devm cleanup call to
regulator_put() then trigger a WARN_ON(regulator->enable_count)?
> + return devm_add_action_or_reset(dev, rcar_gen3_phy_usb2_vbus_disable_action,
> + channel->vbus);
>
> - return devm_add_action_or_reset(dev, rcar_gen3_phy_usb2_vbus_disable_action,
> - channel->vbus);
> + return 0;
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709083108.108370-1-biju.das.jz@bp.renesas.com?part=7
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v4 5/9] dt-bindings: phy: renesas,usb2-phy: Document RZ/G3L PHY bindings
From: sashiko-bot @ 2026-07-09 8:50 UTC (permalink / raw)
To: Biju
Cc: vkoul, robh, wsa+renesas, conor+dt, olteanv, devicetree,
neil.armstrong, linux-phy
In-Reply-To: <20260709083108.108370-6-biju.das.jz@bp.renesas.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Missing related device tree binding update for the RZ/G3L USBHS (OTG) controller.
--
--- Patch [5]: [PATCH v4 5/9] dt-bindings: phy: renesas,usb2-phy: Document RZ/G3L PHY bindings ---
commit c4d05b9a2128264caed7e80893a3e4cb7297b32f
Author: Biju Das <biju.das.jz@bp.renesas.com>
dt-bindings: phy: renesas,usb2-phy: Document RZ/G3L PHY bindings
This patch adds device tree binding support for the RZ/G3L (r9a08g046) USB2
PHY. The commit message notes the RZ/G3L is almost identical to the RZ/G3S
USB PHY, but with 2 OTG blocks instead of 1.
> diff --git a/Documentation/devicetree/bindings/phy/renesas,usb2-phy.yaml b/Documentation/devicetree/bindings/phy/renesas,usb2-phy.yaml
> index 9740e5b335f93..d6b9d08ceec6d 100644
> --- a/Documentation/devicetree/bindings/phy/renesas,usb2-phy.yaml
> +++ b/Documentation/devicetree/bindings/phy/renesas,usb2-phy.yaml
> @@ -16,6 +16,7 @@ properties:
> - enum:
> - renesas,usb2-phy-r8a77470 # RZ/G1C
> - renesas,usb2-phy-r9a08g045 # RZ/G3S
> + - renesas,usb2-phy-r9a08g046 # RZ/G3L
[Severity: Low]
Does Documentation/devicetree/bindings/usb/renesas,usbhs.yaml also need to be
updated to include the "renesas,usbhs-r9a08g046" compatible string for the
RZ/G3L OTG blocks mentioned in the commit message?
Currently, renesas,usbhs.yaml enumerates related SoCs like RZ/G3S
(renesas,usbhs-r9a08g045) under the renesas,rzg2l-usbhs fallback, but is
missing RZ/G3L. This might cause device tree nodes for these OTG blocks to
fail schema validation.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709083108.108370-1-biju.das.jz@bp.renesas.com?part=5
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* [PATCH v4 7/9] phy: renesas: phy-rcar-gen3-usb2: Fix devm action registration for disabled VBUS regulator
From: Biju @ 2026-07-09 8:31 UTC (permalink / raw)
To: Yoshihiro Shimoda, Vinod Koul, Geert Uytterhoeven, Magnus Damm
Cc: Biju Das, Neil Armstrong, Philipp Zabel, linux-renesas-soc,
linux-phy, linux-kernel, Prabhakar Mahadev Lad, Biju Das
In-Reply-To: <20260709083108.108370-1-biju.das.jz@bp.renesas.com>
From: Biju Das <biju.das.jz@bp.renesas.com>
devm_regulator_get_exclusive() initialises the regulator with
enable_count = 1, requiring the consumer to disable it before release.
The devm disable action was previously only registered when the caller
explicitly requested enable, so when the regulator was left in its initial
enabled state without an explicit enable call, the cleanup path skipped
decrementing enable_count, triggering a WARN_ON during regulator
release on device removal.
Fix this by always registering the devm disable action based on the actual
enabled state via regulator_is_enabled(), regardless of whether the
caller requested an explicit enable. This covers both the explicitly
enabled case and the initial state set by devm_regulator_get_exclusive().
Fixes: 24843404efe4 ("phy: renesas: phy-rcar-gen3-usb2: Control VBUS for RZ/G2L SoCs")
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v3->v4:
* No change.
v2->v3:
* No change.
v1->v2:
* Updated commit description.
---
drivers/phy/renesas/phy-rcar-gen3-usb2.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/drivers/phy/renesas/phy-rcar-gen3-usb2.c b/drivers/phy/renesas/phy-rcar-gen3-usb2.c
index d06fb52ed5f1..ef38c3b365d4 100644
--- a/drivers/phy/renesas/phy-rcar-gen3-usb2.c
+++ b/drivers/phy/renesas/phy-rcar-gen3-usb2.c
@@ -905,15 +905,17 @@ static int rcar_gen3_phy_usb2_vbus_regulator_get_exclusive_enable(struct rcar_ge
if (IS_ERR(channel->vbus))
return PTR_ERR(channel->vbus);
- if (!enable)
- return 0;
+ if (enable) {
+ ret = regulator_enable(channel->vbus);
+ if (ret)
+ return ret;
+ }
- ret = regulator_enable(channel->vbus);
- if (ret)
- return ret;
+ if (regulator_is_enabled(channel->vbus))
+ return devm_add_action_or_reset(dev, rcar_gen3_phy_usb2_vbus_disable_action,
+ channel->vbus);
- return devm_add_action_or_reset(dev, rcar_gen3_phy_usb2_vbus_disable_action,
- channel->vbus);
+ return 0;
}
static int rcar_gen3_phy_usb2_vbus_regulator_register(struct rcar_gen3_chan *channel)
--
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 v4 5/9] dt-bindings: phy: renesas,usb2-phy: Document RZ/G3L PHY bindings
From: Biju @ 2026-07-09 8:30 UTC (permalink / raw)
To: Vinod Koul, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Geert Uytterhoeven, Magnus Damm
Cc: Biju Das, Neil Armstrong, Yoshihiro Shimoda, linux-phy,
devicetree, linux-kernel, linux-renesas-soc,
Prabhakar Mahadev Lad, Biju Das, Krzysztof Kozlowski
In-Reply-To: <20260709083108.108370-1-biju.das.jz@bp.renesas.com>
From: Biju Das <biju.das.jz@bp.renesas.com>
Add device tree binding support for the RZ/G3L (r9a08g046) USB2 PHY.
The RZ/G3L USB PHY is almost identical to the RZ/G3S USB PHY, the
difference being 2 OTG blocks on RZ/G3L compared to 1 on RZ/G3S.
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v3->v4:
* Moved the patch for easy merging.
v2->v3:
* No change
v1->v2:
* Collected tag.
---
Documentation/devicetree/bindings/phy/renesas,usb2-phy.yaml | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/phy/renesas,usb2-phy.yaml b/Documentation/devicetree/bindings/phy/renesas,usb2-phy.yaml
index 9740e5b335f9..d6b9d08ceec6 100644
--- a/Documentation/devicetree/bindings/phy/renesas,usb2-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/renesas,usb2-phy.yaml
@@ -16,6 +16,7 @@ properties:
- enum:
- renesas,usb2-phy-r8a77470 # RZ/G1C
- renesas,usb2-phy-r9a08g045 # RZ/G3S
+ - renesas,usb2-phy-r9a08g046 # RZ/G3L
- renesas,usb2-phy-r9a09g057 # RZ/V2H(P)
- items:
@@ -132,6 +133,7 @@ allOf:
enum:
- renesas,usb2-phy-r9a09g057
- renesas,usb2-phy-r9a08g045
+ - renesas,usb2-phy-r9a08g046
- renesas,rzg2l-usb2-phy
then:
properties:
--
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 v4 6/9] phy: renesas: phy-rcar-gen3-usb2: Add RZ/G3L support
From: Biju @ 2026-07-09 8:30 UTC (permalink / raw)
To: Yoshihiro Shimoda, Vinod Koul, Geert Uytterhoeven, Magnus Damm
Cc: Biju Das, Neil Armstrong, linux-renesas-soc, linux-phy,
linux-kernel, Prabhakar Mahadev Lad, Biju Das
In-Reply-To: <20260709083108.108370-1-biju.das.jz@bp.renesas.com>
From: Biju Das <biju.das.jz@bp.renesas.com>
Add renesas,usb2-phy-r9a08g046 to the OF match table, reusing
rz_g3s_phy_usb2_data as the PHY configuration is shared with RZ/G3S.
While the PHY data is shared, RZ/G3L differs from RZ/G3S in that it has
two OTG controllers, OTG interrupts on port 2, and a controllable
OTG_PERI bit in COMMCTRL for host/device switching on the port 2 USB
controller, which is fixed to host-only on RZ/G3S.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v3->v4:
* No change.
v2->v3:
* No change.
v1->v2:
* Updated commit description.
---
drivers/phy/renesas/phy-rcar-gen3-usb2.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/phy/renesas/phy-rcar-gen3-usb2.c b/drivers/phy/renesas/phy-rcar-gen3-usb2.c
index 9a45d840efeb..d06fb52ed5f1 100644
--- a/drivers/phy/renesas/phy-rcar-gen3-usb2.c
+++ b/drivers/phy/renesas/phy-rcar-gen3-usb2.c
@@ -721,6 +721,10 @@ static const struct of_device_id rcar_gen3_phy_usb2_match_table[] = {
.compatible = "renesas,usb2-phy-r9a08g045",
.data = &rz_g3s_phy_usb2_data,
},
+ {
+ .compatible = "renesas,usb2-phy-r9a08g046",
+ .data = &rz_g3s_phy_usb2_data,
+ },
{
.compatible = "renesas,usb2-phy-r9a09g057",
.data = &rz_v2h_phy_usb2_data,
--
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 v4 0/9] Add RZ/G3L USB2.0 host support
From: Biju @ 2026-07-09 8:30 UTC (permalink / raw)
To: Philipp Zabel, Vinod Koul, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Geert Uytterhoeven, Michael Turquette, Stephen Boyd,
Liam Girdwood, Mark Brown, Magnus Damm
Cc: Biju Das, Neil Armstrong, Yoshihiro Shimoda, linux-phy,
devicetree, linux-kernel, linux-clk, linux-renesas-soc,
Prabhakar Mahadev Lad, Biju Das
From: Biju Das <biju.das.jz@bp.renesas.com>
Hi All,
This patch series aims to add USB2.0 host support for the RZ/G3L SMARC
EVK. The RZ/G3L USB PHY block is similar to RZ/G3S, but each port
has an OTG controller, unlike RZ/G3S, which has an OTG controller only on
port 1. This series migrates regulator driver to use id-table so there
is hard dependency between reset driver and regulator driver.
Merge strategy:
An Ack/Rb tag from Regulator Maintainer for patch#4.
Patch #1,#2,#3,#4 - Reset subsystem
Patch #5,#6,#7 - PHY subsystem
Patch #8,#9 - Renesas SoC
v3->v4:
* Updated cover letter with merge strategy
* Reorder the phy binding patch for easy merging.
* Updated the commit description for patch #2,#3, #4.
* Moved the handling of regulator_driver_name to patch #3
in preparation for id_table match.
* Migrated to id_table match using driver_name and reduced the length
< 24.
* Dropped rzg3l specific platform driver.
* Dropped the tag for regulator driver,
* Added id_table handling by introducing a common probe().
v2->v3:
* Replaced items->enum in reset controller bindings.
* Collected the tag for reset binding patch.
* Dropped clock patch as it is accepted.
v1->v2:
* Dropped patch#6(Introduce helper for regulator registration)
* Passing pointer to an array of regulators to make it scalable.
* Dropped regulator1-vbus and added a regulators group node.
* Updated commit description for patch#1,#4,#6,#7,#8,#9 and #10.
* Added enum instead of const in the compatible section.
* Updated schema check.
* Collected tag for PHY binding patch.
in the regulator driver.
* Added regulators group node and its children in SoC dtsi.
Biju Das (9):
dt-bindings: reset: renesas,rzg2l-usbphy-ctrl: Document RZ/G3L support
reset: rzg2l-usbphy-ctrl: Introduce info struct for match data
reset: rzg2l-usbphy-ctrl: Add RZ/G3L support
regulator: renesas-usb-vbus-regulator: Add RZ/G3L VBUS regulator
support
dt-bindings: phy: renesas,usb2-phy: Document RZ/G3L PHY bindings
phy: renesas: phy-rcar-gen3-usb2: Add RZ/G3L support
phy: renesas: phy-rcar-gen3-usb2: Fix devm action registration for
disabled VBUS regulator
arm64: dts: renesas: r9a08g046: Add USB2.0 device nodes
arm64: dts: renesas: r9a08g046l48-smarc: Add USB2.0 support
.../bindings/phy/renesas,usb2-phy.yaml | 2 +
.../reset/renesas,rzg2l-usbphy-ctrl.yaml | 48 +++++++-
arch/arm64/boot/dts/renesas/r9a08g046.dtsi | 105 ++++++++++++++++++
.../boot/dts/renesas/r9a08g046l48-smarc.dts | 49 ++++++++
drivers/phy/renesas/phy-rcar-gen3-usb2.c | 20 ++--
.../regulator/renesas-usb-vbus-regulator.c | 74 +++++++++++-
drivers/reset/reset-rzg2l-usbphy-ctrl.c | 44 +++++---
7 files changed, 316 insertions(+), 26 deletions(-)
--
2.43.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v9 1/2] dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema
From: Vladimir Zapolskiy @ 2026-07-09 7:40 UTC (permalink / raw)
To: Bryan O'Donoghue, Bryan O'Donoghue, Vinod Koul,
Kishon Vijay Abraham I, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Neil Armstrong
Cc: linux-arm-msm, linux-phy, linux-media, devicetree, linux-kernel
In-Reply-To: <3ab676ac-919b-4a80-8dd6-71dd7f6fe06c@kernel.org>
On 7/9/26 00:55, Bryan O'Donoghue wrote:
> On 08/07/2026 08:44, Vladimir Zapolskiy wrote:
>> On 7/8/26 02:39, Bryan O'Donoghue wrote:
>>> Add a base schema initially compatible with x1e80100 to describe MIPI CSI2
>>> PHY devices.
>>>
>>> The hardware can support both CPHY, DPHY and a special split-mode DPHY.
>>>
>>> The schema here defines two ports with three endpoints:
>>>
>>> port@0: Sensor input.
>>> endpoint@0: primary sensor
>>> endpoint@1: optional second sensor, implies DPHY split-mode
>>>
>>> port@1: Controller output.
>>>
>>> The CSIPHY devices have their own pinouts on the SoC as well as their own
>>> individual voltage rails.
>>>
>>> The need to model voltage rails on a per-PHY basis leads us to define
>>> CSIPHY devices as individual nodes.
>>>
>>> Two nice outcomes in terms of schema and DT arise from this change.
>>>
>>> 1. The ability to define on a per-PHY basis voltage rails.
>>> 2. The ability to require those voltage.
>>>
>>> We have had a complete bodge upstream for this where a single set of
>>> voltage rail for all CSIPHYs has been buried inside of CAMSS.
>>>
>>> Much like the I2C bus which is dedicated to Camera sensors - the CCI bus in
>>> CAMSS parlance, the CSIPHY devices should be individually modelled.
>>>
>>> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>>> ---
>>> .../bindings/phy/qcom,x1e80100-csi2-phy.yaml | 202 +++++++++++++++++++++
>>> 1 file changed, 202 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml
>>> new file mode 100644
>>> index 0000000000000..a7fbf6804cd9e
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml
>>> @@ -0,0 +1,202 @@
>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/phy/qcom,x1e80100-csi2-phy.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Qualcomm SoC CSI2 PHY
>>> +
>>> +maintainers:
>>> + - Bryan O'Donoghue <bod@kernel.org>
>>> +
>>> +description:
>>> + Qualcomm MIPI CSI2 C-PHY/D-PHY combination PHY. Connects MIPI CSI2 sensors
>>> + to Qualcomm's Camera CSI Decoder. The PHY supports both C-PHY and D-PHY
>>> + modes.
>>> +
>>> +properties:
>>> + compatible:
>>> + const: qcom,x1e80100-csi2-phy
>>> +
>>> + reg:
>>> + maxItems: 1
>>> +
>>> + "#phy-cells":
>>> + const: 1
>>> + description:
>>> + The single cell specifies the PHY operating mode.
>>
>> Unfortunately my review comment given before was ignored before publishing
>> this version, thus I will repeat.
>>
>> There is a clash between the proposed phy cells value and 'bus-type' property
>> of the media endpoint, the proposed value of phy cells brings no information,
>> and therefore the whole proposed '#phy-cells' prorperty shall be removed.
>>
>> There shall be no third link introduced between CAMSS and CAMSS CSIPHYs.
>
> As explained in the cover letter: I'm following guidance from Rob on that.
Pehaps you've misread a given review comment, because Rob's guidance is
to remove 'phy-type' property, and so far there were no arguments given
against removing unnecessary '#phy-cells' propert, which is needed to
exclude the clash on peer's side between its value and 'bus-type' value.
>
> 20260708-x1e-csi2-phy-v9-0-0210b90c04cf@linaro.org
>
> Old guidance granted, so let me see if I can get his attention to this
> matter. Perhaps your suggestion is fine by him if so then fine, if not I
> will stick to his original nudge, either way its up to him as the
> original reviewer and senior schema maintainer to call this one.
Hopefully Rob will discuss my review comment, at the moment there is no
any technical arguments against it from anyone, so it's still open.
>>> +
>>> + clocks:
>>> + maxItems: 3
>>> +
>>> + clock-names:
>>> + items:
>>> + - const: core
>>> + - const: timer
>>> + - const: ahb
>>> +
>>> + interrupts:
>>> + maxItems: 1
>>> +
>>> + operating-points-v2: true
>>> +
>>> + power-domains:
>>> + items:
>>> + - description: Titan Top GDSC - Titan ISP Block, Global Distributed Switch Controller.
>>> + - description: MMCX voltage rail
>>> + - description: MXC or MXA voltage rail
>>> +
>>> + power-domain-names:
>>> + items:
>>> + - const: top
>>> + - const: mmcx
>>> + - const: mx
>>
>> None of the power domains finds its place in this device tree node, the
>> child device belongs to CAMSS, which already enables these power domains.
>
> Right, I think the whole subnode debate has run its course. Its a
> problem for JPEG and a bit of a fake debate, there is no real "bus" here
> so what is the point of mutating DT to look like a bus ? We are forever
> lecturing people about "making fake DT stuff for convenience".
Please remind me, what is the point of grouping CAMSS devices under one
device tree node? Is there an X1E CAMSS device?
>
> Peer nodes with TITAN_TOP_GDSC will perfectly adequately describe this
> hardware and the JPEG too.
It's not an argument against my proposal to get a simpler hardware
description in the form of device tree bindings. Please address
the review comments actually received.
>>
>>> +
>>> + vdda-0p8-supply:
>>> + description: Phandle to a 0.8V regulator supply to a PHY.
>>
>> The property name shall be vdda-0p9-supply, the description shall be
>> changed accordingly
>
> Hmm what ? I'll have to look back through my notes, I thought the opposite.
>
> Let me follow up in v10 after grepping for this again.>>
>>> +
>>> + vdda-1p2-supply:
>>> + description: Phandle to 1.2V regulator supply to a PHY.
>>> +
>>> + ports:
>>> + $ref: /schemas/graph.yaml#/properties/ports
>>> +
>>> + properties:
>>> + port@0:
>>> + $ref: /schemas/graph.yaml#/$defs/port-base
>>> + description:
>>> + Sensor input. Always present. A single sensor is described by a
>>> + single endpoint with one to four data lanes. DPHY split mode,
>>> + where two independent sensors share the same PHY, is described
>>> + by two endpoints; endpoint@0 with exactly two-data lanes and
>>> + endpoint@1 with exactly one data-lane.
>>> + unevaluatedProperties: false
>>> +
>>> + patternProperties:
>>> + "^endpoint(@[0-9a-f]+)?$":
>>
>> This is too wide regexp mask for one or two endpoints only.
>
> Yeah you're right, this is just fluff.
>
>>
>>> + $ref: /schemas/media/video-interfaces.yaml#
>>> + unevaluatedProperties: false
>>> + properties:
>>> + data-lanes:
>>> + minItems: 1
>>> + maxItems: 4
>>> + remote-endpoint: true
>>> + required:
>>> + - data-lanes
>>> + - remote-endpoint
>>> +
>>> + allOf:
>>> + - if:
>>> + required:
>>> + - endpoint@1
>>> + then:
>>> + properties:
>>> + endpoint@0:
>>> + properties:
>>> + data-lanes:
>>> + minItems: 2
>>> + maxItems: 2
>>> + endpoint@1:
>>> + properties:
>>> + data-lanes:
>>> + maxItems: 1
>>> + required:
>>> + - endpoint@0
>>> +
>>> + port@1:
>>> + $ref: /schemas/graph.yaml#/$defs/port-base
>>> + description: Output to the CAMSS CSID controller.
>>> + unevaluatedProperties: false
>>> +
>>> + patternProperties:
>>> + "^endpoint(@[0-9a-f]+)?$":
>>> + $ref: /schemas/graph.yaml#/$defs/endpoint-base
>>> + unevaluatedProperties: false
>>> + properties:
>>> + remote-endpoint: true
>>> + required:
>>> + - remote-endpoint
>>> +
>>> + required:
>>> + - port@0
>>> + - port@1
>>> +
>>> +required:
>>> + - compatible
>>> + - reg
>>> + - "#phy-cells"
>>> + - clocks
>>> + - clock-names
>>> + - interrupts
>>> + - operating-points-v2
>>> + - power-domains
>>> + - power-domain-names
>>> + - vdda-0p8-supply
>>> + - vdda-1p2-supply
>>> + - ports
>>> +
>>> +additionalProperties: false
>>> +
>>> +examples:
>>> + - |
>>> + #include <dt-bindings/interrupt-controller/arm-gic.h>
>>> + #include <dt-bindings/clock/qcom,x1e80100-camcc.h>
>>> + #include <dt-bindings/clock/qcom,x1e80100-gcc.h>
>>> + #include <dt-bindings/power/qcom,rpmhpd.h>
>>> +
>>> + phy@ace4000 {
>>> + compatible = "qcom,x1e80100-csi2-phy";
>>> + reg = <0x0ace4000 0x2000>;
>>> + #phy-cells = <1>;
>>> +
>>> + clocks = <&camcc CAM_CC_CSIPHY0_CLK>,
>>> + <&camcc CAM_CC_CSI0PHYTIMER_CLK>,
>>> + <&camcc CAM_CC_CORE_AHB_CLK>;
>>> + clock-names = "core",
>>> + "timer",
>>> + "ahb";
>>> +
>>> + interrupts = <GIC_SPI 477 IRQ_TYPE_EDGE_RISING>;
>>> +
>>> + operating-points-v2 = <&csiphy_opp_table>;
>>> +
>>> + power-domains = <&camcc CAM_CC_TITAN_TOP_GDSC>,
>>> + <&rpmhpd RPMHPD_MMCX>,
>>> + <&rpmhpd RPMHPD_MX>;
>>> + power-domain-names = "top",
>>> + "mmcx",
>>> + "mx";
>>> +
>>> + vdda-0p8-supply = <&vreg_l2c_0p8>;
>>> + vdda-1p2-supply = <&vreg_l1c_1p2>;
>>> +
>>> + ports {
>>> + #address-cells = <1>;
>>> + #size-cells = <0>;
>>> +
>>> + port@0 {
>>> + reg = <0>;
>>> + csiphy0_in: endpoint {
>>> + data-lanes = <0 1 2 3>;
>>
>> The previously given review comment about numeration of lanes was
>> not implemented.
>
> I understand your comment but as Nihal pointed out - CAMSS has an
> established pattern for this - and I think his argument is convincing -
> we should stick to that pattern.
I disagree with it, since it's a new dt binding description, and the
issue is known in advance, it should fixed and not ignored.
>>
>>> + remote-endpoint = <&sensor_out>;
>>> + };
>>> + };
>>> +
>>> + port@1 {
>>> + reg = <1>;
>>> + csiphy0_out: endpoint {
>>> + remote-endpoint = <&csid_in>;
>>> + };
>>> + };
>>> + };
>>> + };
>>> +
>>> + csiphy_opp_table: opp-table {
>>> + compatible = "operating-points-v2";
>>> +
>>> + opp-300000000 {
>>> + opp-hz = /bits/ 64 <300000000>;
>>> + required-opps = <&rpmhpd_opp_low_svs_d1>,
>>> + <&rpmhpd_opp_low_svs_d1>;
>>> + };
>>> + };
>>>
--
Best wishes,
Vladimir
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v3 4/4] PCI: qcom: enable Link retain logic for Hamoa
From: sashiko-bot @ 2026-07-09 7:02 UTC (permalink / raw)
To: Krishna Chaitanya Chundru
Cc: linux-pci, vkoul, linux-phy, olteanv, neil.armstrong
In-Reply-To: <20260709-link_retain-v3-4-81a9d187bb61@oss.qualcomm.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Probe deferral leaks PHY power-on, clock, and regulator enable counts when `link_retain` is active.
--
--- Patch [4]: [PATCH v3 4/4] PCI: qcom: enable Link retain logic for Hamoa ---
commit a80ea22accc5e92e309b80e7933e0f0bac88c9ec
Author: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
PCI: qcom: enable Link retain logic for Hamoa
The Hamoa platform supports keeping the PCIe link active across
bootloader and kernel handoff. To take advantage of this, introduce a
specific configuration (cfg_x1e80100) with link_retain = true and
update the device match table to use it.
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 77d2d989e9da5..0b345a16f4cf6 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -1671,6 +1671,12 @@ static const struct qcom_pcie_cfg cfg_sc8280xp = {
> .no_l0s = true,
> };
>
> +static const struct qcom_pcie_cfg cfg_x1e80100 = {
> + .ops = &ops_1_21_0,
> + .no_l0s = true,
> + .link_retain = true,
[Severity: High]
Does enabling link_retain here cause a resource leak during probe deferral?
In qcom_pcie_host_init(), if an initialization step returns -EPROBE_DEFER and
link_retain is true, the error path returns early:
drivers/pci/controller/dwc/pcie-qcom.c:qcom_pcie_host_init() {
...
err_pwrctrl_destroy:
if (pp->link_retain && ret == -EPROBE_DEFER)
return ret;
...
}
This bypasses err_disable_phy and err_deinit which would balance the
resource counts.
Since qcom_pcie_init_2_7_0() unconditionally enables the regulators and
clocks, and qcom_pcie_phy_power_on() enables the PHYs, won't these reference
counts be leaked on each deferral retry? When the driver is eventually
removed, deinit() will only decrement them once, leaving the resources
permanently enabled.
Is there a way to balance these reference counts during a probe deferral
when link_retain is active?
> +};
> +
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-link_retain-v3-0-81a9d187bb61@oss.qualcomm.com?part=4
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v3 3/4] PCI: qcom: Add link retention support
From: sashiko-bot @ 2026-07-09 6:53 UTC (permalink / raw)
To: Krishna Chaitanya Chundru
Cc: linux-pci, linux-phy, neil.armstrong, vkoul, olteanv
In-Reply-To: <20260709-link_retain-v3-3-81a9d187bb61@oss.qualcomm.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] Probe deferral causes reference count and resource leaks for clocks, regulators, and PHY components when link retention is active.
- [Medium] Bypassing `post_init` during link retention skips critical controller quirks and DMA coherency settings.
- [Medium] `qcom_pcie_check_link_retain()` uses an incorrect hardcoded mask (`0x1f`) to read the LTSSM state, risking state aliasing.
--
--- Patch [3]: [PATCH v3 3/4] PCI: qcom: Add link retention support ---
commit 48e4b763c5282c1440201e104587c3e6e2342ab6
Author: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
PCI: qcom: Add link retention support
This patch adds link retention support for Qualcomm PCIe controllers. It
skips controller reset and PERST# toggling if the bootloader has already
trained the link and left it in L0 or L1 idle. It checks device tree caps
and configures base addresses in the retention path.
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
[ ... ]
> @@ -996,6 +998,50 @@ 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.
> + *
> + * The max-link-speed and num-lanes specified in Device Tree are meant to cap
> + * the link's maximum capability. Retrain the link if the controller's max
> + * link speed/width exceeds the Device Tree cap, unless the currently
> + * negotiated speed/width is already within that cap, in which case the
> + * existing link can be retained as-is.
> + */
> +static bool qcom_pcie_check_link_retain(struct qcom_pcie *pcie)
> +{
> + u32 cap, speed, cur_speed, val, ltssm, width, cur_width;
> + struct dw_pcie *pci = pcie->pci;
> + u8 offset;
> +
> + val = readl(pcie->parf + PARF_LTSSM);
> + ltssm = val & 0x1f;
[Severity: Medium]
Does this hardcoded 0x1f mask truncate the LTSSM state?
The standard LTSSM state mask (PARF_LTSSM_STATE_MASK) is defined as
GENMASK(5, 0), which is 0x3f.
By discarding bit 5, higher-order states like 0x31 could erroneously alias
to DW_PCIE_LTSSM_L0 (0x11).
This might cause the driver to incorrectly assume the link is active and
retain it, skipping initialization and leaving the link broken.
[ ... ]
> @@ -1074,6 +1129,9 @@ static int qcom_pcie_post_init_2_7_0(struct qcom_pcie *pcie)
> {
> const struct qcom_pcie_cfg *pcie_cfg = pcie->cfg;
>
> + if (pcie->pci->pp.link_retain)
> + return 0;
> +
[Severity: Medium]
Does bypassing the rest of qcom_pcie_post_init_2_7_0() skip critical OS quirks?
By returning early, this bypasses the configuration of NO_SNOOP_OVERRIDE and
the NCCS hot-plug quirk.
Missing NO_SNOOP_OVERRIDE can cause DMA cache coherency issues and data
corruption on ARM.
Missing the NCCS quirk can cause the OS pciehp driver to wait for
non-existent command completions, leading to hot-plug timeouts.
> if (pcie_cfg->override_no_snoop)
> writel(WR_NO_SNOOP_OVERRIDE_EN | RD_NO_SNOOP_OVERRIDE_EN,
> pcie->parf + PARF_NO_SNOOP_OVERRIDE);
[ ... ]
> @@ -1414,6 +1474,8 @@ static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
> if (!pp->skip_pwrctrl_off)
> pci_pwrctrl_power_off_devices(pci->dev);
> err_pwrctrl_destroy:
> + if (pp->link_retain && ret == -EPROBE_DEFER)
> + return ret;
[Severity: Medium]
Could this early return leak hardware enable counts during probe deferral?
If a downstream function like pci_pwrctrl_power_on_devices() returns
-EPROBE_DEFER, this early return explicitly bypasses the err_disable_phy
and err_deinit cleanup paths.
The driver core's devres cleanup drops the resource references but does not
manually disable them.
On the next probe attempt, qcom_pcie_init_2_7_0() will re-enable the
regulators and clocks:
ret = regulator_bulk_enable(ARRAY_SIZE(res->supplies), res->supplies);
...
ret = clk_bulk_prepare_enable(res->num_clks, res->clks);
This compounds the leaked reference counts, preventing the resources from
ever being powered off upon driver unbind or system shutdown.
> if (ret != -EPROBE_DEFER && !pci->suspended)
> pci_pwrctrl_destroy_devices(pci->dev);
> err_disable_phy:
> qcom_pcie_phy_power_off(pcie);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-link_retain-v3-0-81a9d187bb61@oss.qualcomm.com?part=3
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v3 1/4] phy: qcom: qmp-pcie: Skip PHY reset if already up
From: sashiko-bot @ 2026-07-09 6:47 UTC (permalink / raw)
To: Krishna Chaitanya Chundru
Cc: linux-pci, vkoul, olteanv, linux-phy, neil.armstrong
In-Reply-To: <20260709-link_retain-v3-1-81a9d187bb61@oss.qualcomm.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The `skip_reset` condition is evaluated independently in both `qmp_pcie_init()` and `qmp_pcie_power_on()`, which can lead to unbalanced reset operations and leave the PHY permanently in reset.
--
--- Patch [1]: [PATCH v3 1/4] phy: qcom: qmp-pcie: Skip PHY reset if already up ---
commit 3002076fcac7c2d6108104d2911e0cbb6c6bf224
Author: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
phy: qcom: qmp-pcie: Skip PHY reset if already up
This patch extends the skip_init handling by introducing a skip_reset
condition for the qmp-pcie phy. It skips asserting and deasserting the
no-csr reset if the bootloader has already powered up the PCIe PHY and
it is operational. This reuses the bootloader-initialized PHY state to
save boot time and avoid redundant PCIe link retraining.
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
> index d3effad7a074b..49e10151cb1e4 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
> @@ -4862,6 +4862,7 @@ static int qmp_pcie_init(struct phy *phy)
> struct qmp_pcie *qmp = phy_get_drvdata(phy);
> const struct qmp_phy_cfg *cfg = qmp->cfg;
> void __iomem *pcs = qmp->pcs;
> + bool skip_reset;
> int ret;
>
> /*
> @@ -4877,6 +4878,9 @@ static int qmp_pcie_init(struct phy *phy)
> qphy_checkbits(pcs, cfg->regs[QPHY_START_CTRL], SERDES_START | PCS_START) &&
> qphy_checkbits(pcs, cfg->regs[QPHY_PCS_POWER_DOWN_CONTROL], cfg->pwrdn_ctrl);
>
> + skip_reset = qmp->skip_init && !qphy_checkbits(pcs, cfg->regs[QPHY_PCS_STATUS],
> + cfg->phy_status);
> +
> if (!qmp->skip_init && !cfg->tbls.serdes_num) {
> dev_err(qmp->dev, "Init sequence not available\n");
> return -ENODATA;
[ ... ]
> @@ -4900,13 +4904,15 @@ static int qmp_pcie_init(struct phy *phy)
> }
> }
>
> - ret = reset_control_assert(qmp->nocsr_reset);
> - if (ret) {
> - dev_err(qmp->dev, "no-csr reset assert failed\n");
> - goto err_assert_reset;
> - }
> + if (!skip_reset) {
> + ret = reset_control_assert(qmp->nocsr_reset);
> + if (ret) {
> + dev_err(qmp->dev, "no-csr reset assert failed\n");
> + goto err_assert_reset;
> + }
>
> - usleep_range(200, 300);
> + usleep_range(200, 300);
> + }
>
> if (!qmp->skip_init) {
> ret = reset_control_bulk_deassert(cfg->num_resets, qmp->resets);
[ ... ]
> @@ -4956,8 +4962,11 @@ static int qmp_pcie_power_on(struct phy *phy)
> void __iomem *pcs = qmp->pcs;
> void __iomem *status;
> unsigned int mask, val;
> + bool skip_reset;
> int ret;
>
> + skip_reset = qmp->skip_init && !qphy_checkbits(pcs, cfg->regs[QPHY_PCS_STATUS],
> + cfg->phy_status);
[Severity: High]
Will this lead to unbalanced reset operations if the hardware state changes
between qmp_pcie_init() and qmp_pcie_power_on()?
In qmp_pcie_init(), skip_reset is computed by reading the live PHY status
register. If it evaluates to false, nocsr_reset is asserted. However,
asserting the reset could alter the state of the PHY status register.
When qmp_pcie_power_on() subsequently executes, it reads the status register
again here. If the register now indicates the PHY is ready, skip_reset would
evaluate to true, and the driver would skip reset_control_deassert() below,
leaving the PHY permanently in reset.
Could we cache the skip_reset decision in struct qmp_pcie during init rather
than re-evaluating the live register here?
> /*
> * Write CSR register for PHY that doesn't support no_csr reset or has not
> * been initialized.
> @@ -4981,10 +4990,12 @@ static int qmp_pcie_power_on(struct phy *phy)
> if (ret)
> return ret;
>
> - ret = reset_control_deassert(qmp->nocsr_reset);
> - if (ret) {
> - dev_err(qmp->dev, "no-csr reset deassert failed\n");
> - goto err_disable_pipe_clk;
> + if (!skip_reset) {
> + ret = reset_control_deassert(qmp->nocsr_reset);
> + if (ret) {
> + dev_err(qmp->dev, "no-csr reset deassert failed\n");
> + goto err_disable_pipe_clk;
> + }
> }
>
> if (qmp->skip_init)
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-link_retain-v3-0-81a9d187bb61@oss.qualcomm.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 2/4] PCI: qcom: Keep PERST# GPIO state as-is during probe
From: sashiko-bot @ 2026-07-09 6:44 UTC (permalink / raw)
To: Krishna Chaitanya Chundru
Cc: linux-phy, neil.armstrong, vkoul, linux-pci, olteanv
In-Reply-To: <20260709-link_retain-v3-2-81a9d187bb61@oss.qualcomm.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Requesting the PERST# GPIO with GPIOD_ASIS leaves it as an input if not configured by the bootloader, causing subsequent reset toggles to silently fail.
--
--- Patch [2]: [PATCH v3 2/4] PCI: qcom: Keep PERST# GPIO state as-is during probe ---
commit 9e74f48a37d93ad22a1ec71470af58b05c07c912
Author: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
PCI: qcom: Keep PERST# GPIO state as-is during probe
This commit changes the PERST# GPIO request from GPIOD_OUT_HIGH to GPIOD_ASIS
in order to preserve the existing GPIO state configured by the bootloader or
firmware during probe.
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -1841,7 +1841,7 @@ static int qcom_pcie_parse_perst(struct qcom_pcie *pcie,
> goto parse_child_node;
>
> reset = devm_fwnode_gpiod_get(dev, of_fwnode_handle(np), "reset",
> - GPIOD_OUT_HIGH, "PERST#");
> + GPIOD_ASIS, "PERST#");
[Severity: High]
If the bootloader does not configure the PERST# GPIO as an output, does
requesting it with GPIOD_ASIS leave the pin configured as an input?
If so, later when __qcom_pcie_perst_assert() attempts to toggle the reset:
gpiod_set_value_cansleep(perst->desc, val);
Will it silently fail to drive the pin, preventing the PCIe link from being
established?
> if (IS_ERR(reset)) {
> /*
> * FIXME: GPIOLIB currently supports exclusive GPIO access only.
[ ... ]
> @@ -1918,7 +1918,7 @@ static int qcom_pcie_parse_ports(struct qcom_pcie *pcie)
>
> if (of_find_property(dev->of_node, "perst-gpios", NULL)) {
> pcie->reset = devm_gpiod_get_optional(dev, "perst",
> - GPIOD_OUT_HIGH);
> + GPIOD_ASIS);
[Severity: High]
This appears to have the same issue. If the GPIO defaults to an input and
is not configured by the bootloader, will gpiod_set_value_cansleep()
silently fail to drive the line later?
> if (IS_ERR(pcie->reset))
> return PTR_ERR(pcie->reset);
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-link_retain-v3-0-81a9d187bb61@oss.qualcomm.com?part=2
--
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: Krishna Chaitanya Chundru @ 2026-07-09 6:39 UTC (permalink / raw)
To: Konrad Dybcio, 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: <8bb365dc-25b4-4fe3-8ad9-1f110bad2417@oss.qualcomm.com>
On 6/16/2026 5:38 PM, Konrad Dybcio wrote:
> 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
I had updated the logic in v3 now,
"The max-link-speed and num-lanes properties in Device Tree cap the link's
maximum capability. Retrain the link if the controller's max link speed or
width exceeds the Device Tree cap, unless the currently negotiated speed
or width is already within that cap, in which case the existing link can
still be retained"
- Krishna Chaitanya.
>> + 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
* [PATCH v3 4/4] PCI: qcom: enable Link retain logic for Hamoa
From: Krishna Chaitanya Chundru @ 2026-07-09 6:35 UTC (permalink / raw)
To: 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,
Krishna Chaitanya Chundru, Qiang Yu
In-Reply-To: <20260709-link_retain-v3-0-81a9d187bb61@oss.qualcomm.com>
The Hamoa platform supports keeping the PCIe link active across
bootloader and kernel handoff. To take advantage of this, introduce a
specific configuration (cfg_x1e80100) with link_retain = true and
update the device match table to use it.
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-qcom.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 77d2d989e9da..0b345a16f4cf 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -1671,6 +1671,12 @@ static const struct qcom_pcie_cfg cfg_sc8280xp = {
.no_l0s = true,
};
+static const struct qcom_pcie_cfg cfg_x1e80100 = {
+ .ops = &ops_1_21_0,
+ .no_l0s = true,
+ .link_retain = true,
+};
+
static const struct qcom_pcie_cfg cfg_fw_managed = {
.firmware_managed = true,
};
@@ -2370,7 +2376,7 @@ static const struct of_device_id qcom_pcie_match[] = {
{ .compatible = "qcom,pcie-sm8450-pcie0", .data = &cfg_1_9_0 },
{ .compatible = "qcom,pcie-sm8450-pcie1", .data = &cfg_1_9_0 },
{ .compatible = "qcom,pcie-sm8550", .data = &cfg_1_9_0 },
- { .compatible = "qcom,pcie-x1e80100", .data = &cfg_sc8280xp },
+ { .compatible = "qcom,pcie-x1e80100", .data = &cfg_x1e80100 },
{ }
};
--
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 v3 3/4] PCI: qcom: Add link retention support
From: Krishna Chaitanya Chundru @ 2026-07-09 6:35 UTC (permalink / raw)
To: 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,
Krishna Chaitanya Chundru, Qiang Yu
In-Reply-To: <20260709-link_retain-v3-0-81a9d187bb61@oss.qualcomm.com>
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.
The max-link-speed and num-lanes properties in Device Tree cap the link's
maximum capability. Retrain the link if the controller's max link speed or
width exceeds the Device Tree cap, unless the currently negotiated speed
or width is already within that cap, in which case the existing link can
still be retained.
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.
Set l1ss_support in the retention path as well, since it is otherwise only
set later in the normal init flow which is skipped when the link is
retained.
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 | 68 ++++++++++++++++++++++++++--
2 files changed, 66 insertions(+), 3 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index de4b245b1758..0c98b914eb52 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -471,6 +471,7 @@ struct dw_pcie_rp {
bool native_ecam;
bool skip_l23_ready;
bool skip_pwrctrl_off;
+ 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 9ca620d4746a..77d2d989e9da 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -260,12 +260,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 {
@@ -996,6 +998,50 @@ 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.
+ *
+ * The max-link-speed and num-lanes specified in Device Tree are meant to cap
+ * the link's maximum capability. Retrain the link if the controller's max
+ * link speed/width exceeds the Device Tree cap, unless the currently
+ * negotiated speed/width is already within that cap, in which case the
+ * existing link can be retained as-is.
+ */
+static bool qcom_pcie_check_link_retain(struct qcom_pcie *pcie)
+{
+ u32 cap, speed, cur_speed, val, ltssm, width, cur_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);
+
+ val = dw_pcie_readw_dbi(pci, offset + PCI_EXP_LNKSTA);
+ cur_speed = FIELD_GET(PCI_EXP_LNKSTA_CLS, val);
+ cur_width = FIELD_GET(PCI_EXP_LNKSTA_NLW, val);
+
+ if (pci->max_link_speed > 0 && speed > pci->max_link_speed &&
+ cur_speed > pci->max_link_speed)
+ return false;
+
+ if (pci->num_lanes > 0 && width > pci->num_lanes &&
+ cur_width > pci->num_lanes)
+ return false;
+
+ return true;
+ }
+
+ return false;
+}
+
static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie)
{
struct qcom_pcie_resources_2_7_0 *res = &pcie->res.v2_7_0;
@@ -1014,6 +1060,15 @@ static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie)
if (ret < 0)
goto err_disable_regulators;
+ if (pcie->cfg->link_retain) {
+ pci->pp.link_retain = qcom_pcie_check_link_retain(pcie);
+ pci->l1ss_support = true;
+ if (pci->pp.link_retain) {
+ dev_info(dev, "Retaining PCIe link\n");
+ return 0;
+ }
+ }
+
ret = reset_control_assert(res->rst);
if (ret) {
dev_err(dev, "reset assert failed (%d)\n", ret);
@@ -1074,6 +1129,9 @@ static int qcom_pcie_post_init_2_7_0(struct qcom_pcie *pcie)
{
const struct qcom_pcie_cfg *pcie_cfg = pcie->cfg;
+ if (pcie->pci->pp.link_retain)
+ return 0;
+
if (pcie_cfg->override_no_snoop)
writel(WR_NO_SNOOP_OVERRIDE_EN | RD_NO_SNOOP_OVERRIDE_EN,
pcie->parf + PARF_NO_SNOOP_OVERRIDE);
@@ -1364,12 +1422,13 @@ static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
struct qcom_pcie *pcie = to_qcom_pcie(pci);
int ret;
- qcom_pcie_perst_assert(pcie);
-
ret = pcie->cfg->ops->init(pcie);
if (ret)
return ret;
+ if (!pp->link_retain)
+ qcom_pcie_perst_assert(pcie);
+
ret = qcom_pcie_phy_power_on(pcie);
if (ret)
goto err_deinit;
@@ -1398,7 +1457,8 @@ static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
qcom_pcie_configure_ports(pcie);
- qcom_pcie_perst_deassert(pcie);
+ if (!pp->link_retain)
+ qcom_pcie_perst_deassert(pcie);
if (pcie->cfg->ops->config_sid) {
ret = pcie->cfg->ops->config_sid(pcie);
@@ -1414,6 +1474,8 @@ static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
if (!pp->skip_pwrctrl_off)
pci_pwrctrl_power_off_devices(pci->dev);
err_pwrctrl_destroy:
+ if (pp->link_retain && ret == -EPROBE_DEFER)
+ return ret;
if (ret != -EPROBE_DEFER && !pci->suspended)
pci_pwrctrl_destroy_devices(pci->dev);
err_disable_phy:
--
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 v3 2/4] PCI: qcom: Keep PERST# GPIO state as-is during probe
From: Krishna Chaitanya Chundru @ 2026-07-09 6:35 UTC (permalink / raw)
To: 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,
Krishna Chaitanya Chundru, Qiang Yu
In-Reply-To: <20260709-link_retain-v3-0-81a9d187bb61@oss.qualcomm.com>
The PERST# signal is used to reset PCIe devices. Requesting the GPIO with
GPIOD_OUT_HIGH forces the line high during probe, which can unintentionally
assert reset on devices already out of reset and break proper link
sequencing.
Change the request to use GPIOD_ASIS so the driver preserves the existing
GPIO state configured by the bootloader or firmware. This allows platforms
that manage PERST# externally to maintain correct reset sequencing. PERST#
is asserted explicitly later during qcom_pcie_host_init(), so forcing it
high at probe time is unnecessary.
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-qcom.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index d8eb52857f69..9ca620d4746a 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -1841,7 +1841,7 @@ static int qcom_pcie_parse_perst(struct qcom_pcie *pcie,
goto parse_child_node;
reset = devm_fwnode_gpiod_get(dev, of_fwnode_handle(np), "reset",
- GPIOD_OUT_HIGH, "PERST#");
+ GPIOD_ASIS, "PERST#");
if (IS_ERR(reset)) {
/*
* FIXME: GPIOLIB currently supports exclusive GPIO access only.
@@ -1918,7 +1918,7 @@ static int qcom_pcie_parse_ports(struct qcom_pcie *pcie)
if (of_find_property(dev->of_node, "perst-gpios", NULL)) {
pcie->reset = devm_gpiod_get_optional(dev, "perst",
- GPIOD_OUT_HIGH);
+ GPIOD_ASIS);
if (IS_ERR(pcie->reset))
return PTR_ERR(pcie->reset);
}
--
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 v3 1/4] phy: qcom: qmp-pcie: Skip PHY reset if already up
From: Krishna Chaitanya Chundru @ 2026-07-09 6:35 UTC (permalink / raw)
To: 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,
Krishna Chaitanya Chundru, Abel Vesa, Konrad Dybcio, Qiang Yu
In-Reply-To: <20260709-link_retain-v3-0-81a9d187bb61@oss.qualcomm.com>
If the bootloader has already powered up the PCIe PHY, performing a
full reset and waiting for the PHY to come up again adds unnecessary
delay during boot.
Extend the existing skip_init handling by introducing a skip_reset
condition. When skip_init is active and the PHY status indicates that
the PHY is already operational, skip asserting and deasserting the
no-csr reset while still enabling the required resources during
power-on.
This allows reusing the bootloader-initialized PHY state and avoids
redundant PHY reinitialization and PCIe link retraining, which can
add hundred's of milliseconds of delay.
This relies on the assumption that when skip_init is enabled and the
PHY is reported as up, the bootloader has already configured the PHY
correctly and the link is in a usable state.
Reviewed-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Tested-by: Qiang Yu <qiang.yu@oss.qualcomm.com>
Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 31 +++++++++++++++++++++----------
1 file changed, 21 insertions(+), 10 deletions(-)
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
index d3effad7a074..49e10151cb1e 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
@@ -4862,6 +4862,7 @@ static int qmp_pcie_init(struct phy *phy)
struct qmp_pcie *qmp = phy_get_drvdata(phy);
const struct qmp_phy_cfg *cfg = qmp->cfg;
void __iomem *pcs = qmp->pcs;
+ bool skip_reset;
int ret;
/*
@@ -4877,6 +4878,9 @@ static int qmp_pcie_init(struct phy *phy)
qphy_checkbits(pcs, cfg->regs[QPHY_START_CTRL], SERDES_START | PCS_START) &&
qphy_checkbits(pcs, cfg->regs[QPHY_PCS_POWER_DOWN_CONTROL], cfg->pwrdn_ctrl);
+ skip_reset = qmp->skip_init && !qphy_checkbits(pcs, cfg->regs[QPHY_PCS_STATUS],
+ cfg->phy_status);
+
if (!qmp->skip_init && !cfg->tbls.serdes_num) {
dev_err(qmp->dev, "Init sequence not available\n");
return -ENODATA;
@@ -4900,13 +4904,15 @@ static int qmp_pcie_init(struct phy *phy)
}
}
- ret = reset_control_assert(qmp->nocsr_reset);
- if (ret) {
- dev_err(qmp->dev, "no-csr reset assert failed\n");
- goto err_assert_reset;
- }
+ if (!skip_reset) {
+ ret = reset_control_assert(qmp->nocsr_reset);
+ if (ret) {
+ dev_err(qmp->dev, "no-csr reset assert failed\n");
+ goto err_assert_reset;
+ }
- usleep_range(200, 300);
+ usleep_range(200, 300);
+ }
if (!qmp->skip_init) {
ret = reset_control_bulk_deassert(cfg->num_resets, qmp->resets);
@@ -4956,8 +4962,11 @@ static int qmp_pcie_power_on(struct phy *phy)
void __iomem *pcs = qmp->pcs;
void __iomem *status;
unsigned int mask, val;
+ bool skip_reset;
int ret;
+ skip_reset = qmp->skip_init && !qphy_checkbits(pcs, cfg->regs[QPHY_PCS_STATUS],
+ cfg->phy_status);
/*
* Write CSR register for PHY that doesn't support no_csr reset or has not
* been initialized.
@@ -4981,10 +4990,12 @@ static int qmp_pcie_power_on(struct phy *phy)
if (ret)
return ret;
- ret = reset_control_deassert(qmp->nocsr_reset);
- if (ret) {
- dev_err(qmp->dev, "no-csr reset deassert failed\n");
- goto err_disable_pipe_clk;
+ if (!skip_reset) {
+ ret = reset_control_deassert(qmp->nocsr_reset);
+ if (ret) {
+ dev_err(qmp->dev, "no-csr reset deassert failed\n");
+ goto err_disable_pipe_clk;
+ }
}
if (qmp->skip_init)
--
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 v3 0/4] PCI: qcom: Add link retention support
From: Krishna Chaitanya Chundru @ 2026-07-09 6:35 UTC (permalink / raw)
To: 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,
Krishna Chaitanya Chundru, Abel Vesa, Konrad Dybcio, Qiang Yu
This patch series introduces support for retaining the PCIe link across
bootloader and kernel handoff on Qualcomm platforms, specifically
X1E80100. The goal is to reduce boot time and avoid unnecessary link
reinitialization when the link is already up.
We are not enabling link retantion support for all the targets, as there
is no guarantee that the bootloader on all targets has initialized the
PCIe link in max supported speed. So we are enabling for hamoa & glymur
target only for now based on the config flag.
If the link is up and has link_retain is set to true in the
ithe driver config data then enable retain logic in the controller.
In phy as we already have skip init logic, the phy patch uses same
assumption that if there is phy no csr and bootloader has done the init
then driver can skip resetting the phy when phy status indicates it is
up.
Problem:-
1) As part of late init calls of clock the framework is disabling all the
unvoted resources by that time and also there is no sync state to keep
them enabled till the probe is completed. Due to dependencies with
regulators and phy, qcom pcie probe is happening after late init which is
causing the resources(clocks) to be off which causes the link to go down.
To avoid this we need to use this kernel command line argument
(clk_ignore_unused) to skip disabling clocks as part of late init for
initial version. Once it is resolved we can avoid those kernel command
line argument.
Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
Changes in v3:
- Updated the max link speed and current link speed logic for link
retain.
- Fixed l1ss_support & probe deffer fix as pointed by sashiko.
- Link to v2: https://patch.msgid.link/20260521-link_retain-v2-0-08ed448b081c@oss.qualcomm.com
Changes in v2:
- Rebased with latest changes
- GENPD/power domains are not longer getting turned off with the latest
kernel, only issue is with the clocks.
- Removed the patch [2/5] PCI: dwc: Add support for retaining link during host init
as we are not seeing much difference with this or without this (Bjorn).
- couple of nits in commit & prints (Mani).
- Remove skip_reset for the long term (Dmitry).
- Link to v1: https://lore.kernel.org/r/20260109-link_retain-v1-0-7e6782230f4b@oss.qualcomm.com
---
Krishna Chaitanya Chundru (4):
phy: qcom: qmp-pcie: Skip PHY reset if already up
PCI: qcom: Keep PERST# GPIO state as-is during probe
PCI: qcom: Add link retention support
PCI: qcom: enable Link retain logic for Hamoa
drivers/pci/controller/dwc/pcie-designware.h | 1 +
drivers/pci/controller/dwc/pcie-qcom.c | 80 +++++++++++++++++++++++++---
drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 31 +++++++----
3 files changed, 96 insertions(+), 16 deletions(-)
---
base-commit: ce1f20ceb245f81cc147bf4fd570c09f9f720ab2
change-id: 20251001-link_retain-f181307947e4
Best regards,
--
Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
--
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