* Re: [PATCH 1/2] dt-bindings: phy: qcom,ipq8074-qmp-pcie: Document the ipq5210 QMP PCIe PHY
From: sashiko-bot @ 2026-05-14 12:05 UTC (permalink / raw)
To: Varadarajan Narayanan
Cc: krzk+dt, linux-phy, conor+dt, robh, vkoul, neil.armstrong,
olteanv, devicetree
In-Reply-To: <20260514-pci-phy-v1-1-482429192746@oss.qualcomm.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The compatible string `qcom,ipq5210-qmp-gen3x1-pcie-phy` is added to both the standalone enum and the fallback items list, allowing it to bypass the intended fallback requirement.
--
commit 55b201f9c0cef59e94dd05d524ed71d29f4e86c9
Author: Varadarajan Narayanan <varadarajan.narayanan@oss.qualcomm.com>
dt-bindings: phy: qcom,ipq8074-qmp-pcie: Document the ipq5210 QMP PCIe PHY
This commit documents the ipq5210 QMP PCIe PHY. It aims to use the ipq9574
bindings as a fallback, as the hardware is similar.
> diff --git a/Documentation/devicetree/bindings/phy/qcom,ipq8074-qmp-pcie-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,ipq8074-qmp-pcie-phy.yaml
> index f60804687412a..1ac31439c20a0 100644
> --- a/Documentation/devicetree/bindings/phy/qcom,ipq8074-qmp-pcie-phy.yaml
> +++ b/Documentation/devicetree/bindings/phy/qcom,ipq8074-qmp-pcie-phy.yaml
> @@ -17,6 +17,7 @@ properties:
> compatible:
> oneOf:
> - enum:
> + - qcom,ipq5210-qmp-gen3x1-pcie-phy
The commit message states the intent to document the IPQ5210 PHY "using the
ipq9574 bindings as a fallback". Does adding the compatible string to this
standalone enum allow device trees to omit the fallback and still pass
validation?
Since this is inside a oneOf block, a device tree could specify just this
string without the ipq9574 fallback, defeating the intended requirement.
> - qcom,ipq6018-qmp-pcie-phy
> - qcom,ipq8074-qmp-gen3-pcie-phy
[ ... ]
> @@ -24,10 +25,12 @@ properties:
> - qcom,ipq9574-qmp-gen3x2-pcie-phy
> - items:
> - enum:
> + - qcom,ipq5210-qmp-gen3x1-pcie-phy
> - qcom,ipq5424-qmp-gen3x1-pcie-phy
> - const: qcom,ipq9574-qmp-gen3x1-pcie-phy
> - items:
> - enum:
> + - qcom,ipq5210-qmp-gen3x2-pcie-phy
> - qcom,ipq5424-qmp-gen3x2-pcie-phy
> - const: qcom,ipq9574-qmp-gen3x2-pcie-phy
The gen3x2 variant added here correctly enforces the fallback because it
was not added to the standalone enum above.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260514-pci-phy-v1-0-482429192746@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 v2] phy: ti: pipe3: Fix clock resource leak on probe errors
From: sashiko-bot @ 2026-05-14 12:02 UTC (permalink / raw)
To: Hongling Zeng; +Cc: vkoul, neil.armstrong, linux-phy, olteanv
In-Reply-To: <20260514023424.23815-1-zenghongling@kylinos.cn>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Unchecked clk_prepare_enable() return value leads to unbalanced clock disable.
- [Medium] Inverted resource teardown order in the error path.
--
commit 34a06b14102bc841b336d48ee9651b777cd39eb7
Author: Hongling Zeng <zenghongling@kylinos.cn>
phy: ti: pipe3: Fix clock resource leak on probe errors
When devm_phy_create() or devm_of_phy_provider_register() fails,
this commit adds an error handling path to disable the refclk that
was enabled earlier, preventing a resource leak.
> diff --git a/drivers/phy/ti/phy-ti-pipe3.c b/drivers/phy/ti/phy-ti-pipe3.c
> --- a/drivers/phy/ti/phy-ti-pipe3.c
> +++ b/drivers/phy/ti/phy-ti-pipe3.c
> @@ -837,15 +837,28 @@ static int ti_pipe3_probe(struct platform_device *pdev)
[ ... ]
> +err_clk_disable:
> + pm_runtime_disable(dev);
> + if (phy->sata_refclk_enabled)
> + clk_disable_unprepare(phy->refclk);
> + return ret;
> }
Is the teardown order in this new error path correct?
During probe, pm_runtime_enable() is called before clk_prepare_enable().
Should clk_disable_unprepare() be called before pm_runtime_disable() to
follow reverse-initialization order, similar to the existing logic in
ti_pipe3_remove()?
Additionally, looking at the clock initialization earlier in
ti_pipe3_probe():
if (phy->mode == PIPE3_MODE_SATA) {
if (!IS_ERR(phy->refclk)) {
clk_prepare_enable(phy->refclk);
phy->sata_refclk_enabled = true;
}
}
The return value of clk_prepare_enable() is not checked, and
phy->sata_refclk_enabled is set to true regardless of whether the clock
was successfully enabled.
If clk_prepare_enable() fails, and a subsequent step like
devm_phy_create() also fails, the code will jump out to err_clk_disable
and call clk_disable_unprepare() because phy->sata_refclk_enabled is
true.
Does this introduce a regression by causing an unbalanced clock disable?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260514023424.23815-1-zenghongling@kylinos.cn?part=1
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v9 5/5] phy: qualcomm: add MSM8974 HDMI PHY support
From: sashiko-bot @ 2026-05-14 11:14 UTC (permalink / raw)
To: Dmitry Baryshkov; +Cc: linux-phy, neil.armstrong, vkoul, olteanv
In-Reply-To: <20260513-fd-hdmi-phy-v9-5-ca98c72f1f9f@oss.qualcomm.com>
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Integer truncation when passing vco_freq to write16() writes garbage calibration data
- [High] Concurrent access to PLL registers between clock and PHY frameworks without a shared lock
- [High] User-controllable input triggers a WARN_ON(), enabling a kernel panic DoS
- [Medium] Error path in qcom_hdmi_msm8974_phy_pll_enable() leaks power by failing to restore PD_CTRL0
- [Medium] The read24() helper lacks bitmasks, leading to potential data corruption from upper register bits
--
commit d2fb8e86ab045b987f00fdf674f81b4bc2d4b950
Author: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
phy: qualcomm: add MSM8974 HDMI PHY support
Add support for HDMI PHY on Qualcomm MSM8974 / APQ8074 platforms.
> diff --git a/drivers/phy/qualcomm/phy-qcom-hdmi-28hpm.c b/drivers/phy/qualcomm/phy-qcom-hdmi-28hpm.c
> index 720757f8f3931..7d398060b3a3c 100644
> --- a/drivers/phy/qualcomm/phy-qcom-hdmi-28hpm.c
> +++ b/drivers/phy/qualcomm/phy-qcom-hdmi-28hpm.c
[ ... ]
> @@ -31,8 +34,282 @@
> #define REG_HDMI_8x74_BIST_PATN3 0x00000048
> #define REG_HDMI_8x74_STATUS 0x0000005c
[ ... ]
> +static inline u32 read24(void __iomem *reg)
> +{
> + u32 val = readl(reg);
> +
> + val |= readl(reg + 4) << 8;
> + val |= readl(reg + 8) << 16;
Can this result in data corruption if the hardware bus returns non-zero upper
bits for these registers?
Unlike write24() which correctly masks its values with & 0xff, the readl()
results are not masked before bit-shifting. If the hardware returns reserved
or uninitialized upper bits, they will bleed into and corrupt the assembled
24-bit value used in qcom_uniphy_recalc().
> +
> + return val;
> +}
> +
> +static void qcom_uniphy_setup(void __iomem *base, unsigned int ref_freq,
> + bool sdm_mode,
> + bool ref_freq_mult_2,
> + bool dither,
> + unsigned int refclk_div,
> + unsigned int vco_freq)
> +{
[ ... ]
> + write16(mult_frac(ref_freq, 5, 1000000), base + UNIPHY_PLL_CAL_CFG8);
> + write16(vco_freq / 16, base + UNIPHY_PLL_CAL_CFG10);
Will this cause integer truncation and misconfigure the PLL calibration?
The vco_freq parameter can be up to 1.8 GHz, meaning vco_freq / 16 can yield
a value up to ~112.5 million, which requires 27 bits. Since write16() takes a
u16 parameter, the upper 11 bits are silently discarded, and only the lower 16
bits are written to the CFG10 and CFG11 registers.
> +}
> +
> +static unsigned long qcom_uniphy_recalc(void __iomem *base, unsigned long parent_rate)
> +{
> + unsigned long rate;
> + u32 refclk_cfg;
> + u32 dc_offset;
> + u64 fraq_n;
> + u32 val;
> +
> + refclk_cfg = readl(base + UNIPHY_PLL_REFCLK_CFG);
[ ... ]
> + val = readl(base + UNIPHY_PLL_SDM_CFG0);
> + if (FIELD_GET(UNIPHY_PLL_SDM_BYP, val)) {
> + dc_offset = FIELD_GET(UNIPHY_PLL_SDM_BYP_DIV, val);
> + fraq_n = 0;
> + } else {
> + dc_offset = FIELD_GET(UNIPHY_PLL_SDM_DC_OFFSET,
> + readl(base + UNIPHY_PLL_SDM_CFG1));
> + fraq_n = read24(base + UNIPHY_PLL_SDM_CFG2);
> + }
Does this code race with qcom_hdmi_msm8974_phy_power_on()?
The PHY driver exposes a clock provider, and this recalc_rate callback reads
shared PLL hardware registers protected by the clock framework's prepare_lock.
However, qcom_hdmi_msm8974_phy_power_on() writes to these exact same PLL
registers while holding an unrelated PHY mutex.
Without a shared lock synchronizing these paths, reading from the clk_summary
debugfs file concurrently with a PHY power-on could cause torn reads,
especially since the 24-bit fractional divider is written across three
separate writel() calls.
[ ... ]
> +static int qcom_hdmi_msm8974_phy_pll_set_rate(struct qcom_hdmi_preqmp_phy *hdmi_phy)
> +{
> + unsigned long long pixclk = hdmi_phy->hdmi_opts.tmds_char_rate;
> + unsigned long vco_rate;
> + unsigned int div;
> + int div_idx = 0;
> +
> + div_idx = qcom_hdmi_msm8974_phy_find_div(pixclk);
> + if (WARN_ON(div_idx < 0))
> + return div_idx;
Is it safe to use a WARN_ON() for user-controllable input?
The requested pixel clock (hdmi_opts.tmds_char_rate) is influenced by DRM
modesetting or an external monitor's EDID. An unprivileged user could trigger
this path by requesting an unsupported display mode, intentionally hitting
this WARN_ON().
On systems configured with panic_on_warn=1, this would crash the kernel,
potentially leading to a local denial of service.
[ ... ]
> +static int qcom_hdmi_msm8974_phy_pll_enable(struct qcom_hdmi_preqmp_phy *hdmi_phy)
> +{
> + int ret;
> + unsigned long status;
> +
> + /* Global enable */
> + writel(0x81, hdmi_phy->phy_reg + REG_HDMI_8x74_GLB_CFG);
> +
> + /* Power up power gen */
> + writel(0x00, hdmi_phy->phy_reg + REG_HDMI_8x74_PD_CTRL0);
> + udelay(350);
[ ... ]
> + /* Poll for PHY ready status */
> + ret = readl_poll_timeout(hdmi_phy->phy_reg + REG_HDMI_8x74_STATUS,
> + status, status & BIT(0),
> + 100, 2000);
> + if (ret) {
> + dev_warn(hdmi_phy->dev, "HDMI PHY not ready\n");
> + goto err;
> + }
> +
> + return 0;
> +
> +err:
> + writel(0, hdmi_phy->pll_reg + UNIPHY_PLL_GLB_CFG);
> + udelay(5);
> + writel(0, hdmi_phy->phy_reg + REG_HDMI_8x74_GLB_CFG);
> +
> + return ret;
> +}
Does this error path leave the PHY partially powered up?
During the power-on sequence, the PHY is powered up by writing to
REG_HDMI_8x74_PD_CTRL0. If the subsequent polling fails, the code jumps to
the err label.
The error path clears the global configuration registers but fails to write
0x7f back to REG_HDMI_8x74_PD_CTRL0 to power down the generator. Because the
function returns an error, the PHY framework will not invoke the power_off
callback, leaking power indefinitely.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260513-fd-hdmi-phy-v9-0-ca98c72f1f9f@oss.qualcomm.com?part=5
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* [PATCH] phy: renesas: rcar-gen3-usb2: Avoid long delay in atomic context
From: Claudiu Beznea @ 2026-05-14 11:13 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.
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>
---
drivers/phy/renesas/phy-rcar-gen3-usb2.c | 163 ++++++++++++++++++-----
1 file changed, 132 insertions(+), 31 deletions(-)
diff --git a/drivers/phy/renesas/phy-rcar-gen3-usb2.c b/drivers/phy/renesas/phy-rcar-gen3-usb2.c
index 9a45d840efeb..bdb64726f4cf 100644
--- a/drivers/phy/renesas/phy-rcar-gen3-usb2.c
+++ b/drivers/phy/renesas/phy-rcar-gen3-usb2.c
@@ -15,6 +15,7 @@
#include <linux/extcon-provider.h>
#include <linux/interrupt.h>
#include <linux/io.h>
+#include <linux/lockdep.h>
#include <linux/module.h>
#include <linux/mutex.h>
#include <linux/mux/consumer.h>
@@ -138,12 +139,15 @@ 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;
+ int irq;
bool extcon_host;
bool is_otg_channel;
bool uses_otg_pins;
bool otg_internal_reg;
+ bool otg_initializing;
};
struct rcar_gen3_phy_drv_data {
@@ -386,32 +390,68 @@ static bool rcar_gen3_are_all_rphys_power_off(struct rcar_gen3_chan *ch)
return true;
}
+static int rcar_gen3_phy_wait_otg_init(struct rcar_gen3_chan *channel,
+ unsigned long *flags)
+{
+ unsigned long timeout = msecs_to_jiffies(25);
+ unsigned long ret = 1;
+
+ lockdep_assert_held(&channel->lock);
+
+ /*
+ * The OTG can be initialized only once and needs to release the lock
+ * and wait for 20 ms due to hardware constraints. Wait for the OTG PHY
+ * initialization to complete if another PHY executes configuration
+ * code while the OTG PHY is waiting. This avoids returning failures to
+ * PHY users.
+ */
+ if (READ_ONCE(channel->otg_initializing)) {
+ spin_unlock_irqrestore(&channel->lock, *flags);
+
+ ret = wait_for_completion_timeout(&channel->otg_init_done, timeout);
+
+ spin_lock_irqsave(&channel->lock, *flags);
+ }
+
+ return !ret ? -ETIMEDOUT : 0;
+}
+
static ssize_t role_store(struct device *dev, struct device_attribute *attr,
const char *buf, size_t count)
{
struct rcar_gen3_chan *ch = dev_get_drvdata(dev);
bool is_b_device;
enum phy_mode cur_mode, new_mode;
+ 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;
+ goto unlock;
+
+ ret = rcar_gen3_phy_wait_otg_init(ch, &flags);
+ if (ret)
+ 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;
+ }
if (new_mode == PHY_MODE_USB_HOST) { /* And is_host must be false */
if (!is_b_device) /* A-Peripheral */
@@ -425,7 +465,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);
+
+ return ret ?: count;
}
static ssize_t role_show(struct device *dev, struct device_attribute *attr,
@@ -441,14 +484,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 +511,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);
@@ -510,6 +554,11 @@ static irqreturn_t rcar_gen3_phy_usb2_irq(int irq, void *_ch)
goto rpm_put;
scoped_guard(spinlock, &ch->lock) {
+ if (READ_ONCE(ch->otg_initializing)) {
+ dev_warn(dev, "%s: Got IRQ while waiting for OTG init!\n", __func__);
+ return IRQ_NONE;
+ }
+
status = readl(usb2_base + USB2_OBINTSTA);
if (status & ch->phy_data->obint_enable_bits) {
dev_vdbg(dev, "%s: %08x\n", __func__, status);
@@ -533,9 +582,15 @@ 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;
+ unsigned long flags;
u32 val;
+ int ret;
- guard(spinlock_irqsave)(&channel->lock);
+ spin_lock_irqsave(&channel->lock, flags);
+
+ ret = rcar_gen3_phy_wait_otg_init(channel, &flags);
+ if (ret)
+ goto unlock;
/* Initialize USB2 part */
val = readl(usb2_base + USB2_INT_ENABLE);
@@ -548,8 +603,22 @@ 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)) {
+ rcar_gen3_init_otg_phase0(channel);
+ disable_irq_nosync(channel->irq);
+ reinit_completion(&channel->otg_init_done);
+ WRITE_ONCE(channel->otg_initializing, true);
+ spin_unlock_irqrestore(&channel->lock, flags);
+
+ fsleep(20000);
+
+ spin_lock_irqsave(&channel->lock, flags);
+ WRITE_ONCE(channel->otg_initializing, false);
+ complete_all(&channel->otg_init_done);
+ enable_irq(channel->irq);
+ rcar_gen3_init_otg_phase1(channel);
+ }
if (channel->phy_data->vblvl_ctrl) {
/* SIDDQ mode release */
@@ -568,7 +637,10 @@ static int rcar_gen3_phy_usb2_init(struct phy *p)
rphy->initialized = true;
- return 0;
+unlock:
+ spin_unlock_irqrestore(&channel->lock, flags);
+
+ return ret;
}
static int rcar_gen3_phy_usb2_exit(struct phy *p)
@@ -576,9 +648,15 @@ 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;
+ unsigned long flags;
u32 val;
+ int ret;
- guard(spinlock_irqsave)(&channel->lock);
+ spin_lock_irqsave(&channel->lock, flags);
+
+ ret = rcar_gen3_phy_wait_otg_init(channel, &flags);
+ if (ret)
+ goto unlock;
rphy->initialized = false;
@@ -588,7 +666,9 @@ static int rcar_gen3_phy_usb2_exit(struct phy *p)
val &= ~USB2_INT_ENABLE_UCOM_INTEN;
writel(val, usb2_base + USB2_INT_ENABLE);
- return 0;
+unlock:
+ spin_unlock_irqrestore(&channel->lock, flags);
+ return ret;
}
static int rcar_gen3_phy_usb2_power_on(struct phy *p)
@@ -596,6 +676,7 @@ 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;
+ unsigned long flags;
u32 val;
int ret = 0;
@@ -605,11 +686,15 @@ static int rcar_gen3_phy_usb2_power_on(struct phy *p)
return ret;
}
- guard(spinlock_irqsave)(&channel->lock);
+ spin_lock_irqsave(&channel->lock, flags);
if (!rcar_gen3_are_all_rphys_power_off(channel))
goto out;
+ ret = rcar_gen3_phy_wait_otg_init(channel, &flags);
+ if (ret)
+ goto unlock;
+
val = readl(usb2_base + USB2_USBCTR);
val |= USB2_USBCTR_PLL_RST;
writel(val, usb2_base + USB2_USBCTR);
@@ -620,27 +705,41 @@ 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;
+unlock:
+ spin_unlock_irqrestore(&channel->lock, flags);
+
+ 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 ret = 0;
+ unsigned long flags;
+ int ret;
- 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);
+ ret = rcar_gen3_phy_wait_otg_init(channel, &flags);
+ if (ret)
+ goto unlock;
- val |= USB2_USBCTR_PLL_RST;
- writel(val, channel->base + USB2_USBCTR);
- }
+ 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);
}
- if (channel->vbus && !channel->otg_internal_reg)
+unlock:
+ spin_unlock_irqrestore(&channel->lock, flags);
+
+ if (!ret && channel->vbus && !channel->otg_internal_reg)
ret = regulator_disable(channel->vbus);
return ret;
@@ -1007,6 +1106,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);
@@ -1048,6 +1148,7 @@ static int rcar_gen3_phy_usb2_probe(struct platform_device *pdev)
return dev_err_probe(dev, ret,
"Failed to request irq (%d)\n",
irq);
+ channel->irq = irq;
}
provider = devm_of_phy_provider_register(dev, rcar_gen3_phy_usb2_xlate);
--
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 phy-next 0/2] phy: ti: add driver for TI DS125DF111 Dual-Channel Retimer
From: Vladimir Oltean @ 2026-05-14 10:52 UTC (permalink / raw)
To: Ioana Ciornei
Cc: vkoul, neil.armstrong, robh, krzk+dt, conor+dt, johan, linux-phy,
devicetree, linux-kernel
In-Reply-To: <20260513185103.1371809-1-ioana.ciornei@nxp.com>
On Wed, May 13, 2026 at 09:51:01PM +0300, Ioana Ciornei wrote:
> This patch set adds a generic PHY driver and the corresponding DT
> binding for the TI DS125DF111 Dual-Channel retimer. The datasheet on
> which this driver was based on can be found at -
> https://www.ti.com/lit/gpn/DS125DF111.
>
> A separate generic PHY is registered for each of the two channels of the
> retimer, so consumers can drive each channel independently. This allows
> for independent control of the channels, which is especially important
> since each channel can be routed to different SerDes lanes and it is not
> guaranteed that the same retimer will do both directions of SerDes lane.
>
> This was tested on a LS1088ARDB board with the Lynx10G SerDes PHY driver
> yet to be submitted.
>
> Ioana Ciornei (2):
> dt-bindings: phy: add PHY bindings for the TI DS125DF111 Retimer PHY
> phy: ti: add PHY driver for TI DS125DF111 Dual-Channel Retimer
>
> .../bindings/phy/ti,ds125df111.yaml | 46 ++++
> drivers/phy/ti/Kconfig | 10 +
> drivers/phy/ti/Makefile | 1 +
> drivers/phy/ti/phy-ds125df111.c | 245 ++++++++++++++++++
> 4 files changed, 302 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/phy/ti,ds125df111.yaml
> create mode 100644 drivers/phy/ti/phy-ds125df111.c
>
> --
> 2.25.1
When you resend, please add a MAINTAINERS entry for this driver.
Thanks!
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH phy-next 2/2] phy: ti: add PHY driver for TI DS125DF111 Dual-Channel Retimer
From: Ioana Ciornei @ 2026-05-14 10:47 UTC (permalink / raw)
To: sashiko-reviews
Cc: robh, krzk+dt, conor+dt, neil.armstrong, olteanv, devicetree,
vkoul, linux-phy
In-Reply-To: <20260514103143.4B33BC2BCB8@smtp.kernel.org>
On Thu, May 14, 2026 at 10:31:42AM +0000, sashiko-bot@kernel.org wrote:
> [You don't often get email from sashiko-bot@kernel.org. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Medium] Uninitialized memory read in `ds125df111_xlate` due to missing `args_count` validation.
> - [Low] Missing explicit header inclusions for macros and memory allocation functions.
> --
>
> commit a5dcf247e2e2a6b752d667ac46227925aa476def
> Author: Ioana Ciornei <ioana.ciornei@nxp.com>
>
> phy: ti: add PHY driver for TI DS125DF111 Dual-Channel Retimer
>
> This commit adds a generic PHY driver for the TI DS125DF111 Multi-Protocol
> Dual-Channel Retimer over I2C. It currently supports 10G and 1G link speeds,
> and registers a separate PHY device for each of the two channels to allow
> independent configuration.
>
> > diff --git a/drivers/phy/ti/phy-ds125df111.c b/drivers/phy/ti/phy-ds125df111.c
> > new file mode 100644
> > index 0000000000000..8ac10c603bf70
> > --- /dev/null
> > +++ b/drivers/phy/ti/phy-ds125df111.c
> > @@ -0,0 +1,245 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/* Copyright 2026 NXP */
> > +
> > +#include <linux/i2c.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/phy.h>
> > +#include <linux/phy/phy.h>
>
> This isn't a bug, but should this include linux/bitfield.h, linux/bits.h,
> and linux/slab.h?
>
> The driver uses FIELD_PREP(), GENMASK(), and devm_kzalloc(), but relies on
> implicit inclusions for these headers. Explicitly including them can help
> prevent build breakages if the include chains are refactored in the future.
Ok, sure. Will fix in v2.
>
> [ ... ]
>
> > +static struct phy *ds125df111_xlate(struct device *dev,
> > + const struct of_phandle_args *args)
> > +{
> > + struct ds125df111_priv *priv = dev_get_drvdata(dev);
> > + u32 idx = args->args[0];
>
> Is it possible to read uninitialized memory here?
>
> If a consumer's Device Tree reference incorrectly configures #phy-cells = <0>,
> of_parse_phandle_with_args() sets args->args_count to 0 and leaves the
> args array unmodified.
>
> Since the of_phandle_args structure is often allocated on the stack, for
> example in _of_phy_get(), evaluating args->args[0] without verifying that
> args->args_count >= 1 could result in evaluating stack garbage against the
> maximum channel count.
Good point. Will verify the args_count.
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH 3/4] arm64: dts: qcom: Add Shikra CQM SoM platform
From: Kamal Wadhwa @ 2026-05-14 10:39 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Rakesh Kota, linux-arm-msm, sashiko-reviews, Komal Bajaj, robh,
linux-phy, neil.armstrong, vkoul, olteanv, krzk+dt, conor+dt,
devicetree
In-Reply-To: <0ac665bb-1a21-4077-aa9e-97162f7f4784@oss.qualcomm.com>
On Wed, May 13, 2026 at 06:14:20PM +0300, Dmitry Baryshkov wrote:
> On 13/05/2026 17:29, Rakesh Kota wrote:
> > On Wed, May 13, 2026 at 03:01:47PM +0300, Dmitry Baryshkov wrote:
> > > On Wed, May 13, 2026 at 04:28:35AM +0000, sashiko-bot@kernel.org wrote:
> > > > Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> > > > - [High] The PMIC regulator definitions omit their required input supply dependencies (e.g., `vdd_s2-supply`, `vdd_l3-supply`), breaking the power hierarchy.
> > > > - [Medium] The device tree inaccurately hardcodes the `compatible` string to a different PMIC model (`qcom,rpm-pm2250-regulators`) instead of explicitly identifying the actual hardware (PM4125).
> > > > --
> > > >
> > > > commit 457abf6e7ac410430c866842e08e602ec3daaa51
> > > > Author: Komal Bajaj <komal.bajaj@oss.qualcomm.com>
> > > >
> > > > arm64: dts: qcom: Add Shikra CQM SoM platform
> > > >
> > > > Add device tree include for the CQM variant of the Shikra System-on-Module.
> > > > This variant integrates the Shikra SoC and PM4125 PMIC. The DTSI includes
> > > > the common shikra.dtsi and adds PM4125 PMIC regulator definitions.
> > > >
> > > > > diff --git a/arch/arm64/boot/dts/qcom/shikra-cqm-som.dtsi b/arch/arm64/boot/dts/qcom/shikra-cqm-som.dtsi
> > > > > new file mode 100644
> > > > > index 0000000000000..401e71720519d
> > > > > --- /dev/null
> > > > > +++ b/arch/arm64/boot/dts/qcom/shikra-cqm-som.dtsi
> > > > [ ... ]
> > > > > +&rpm_requests {
> > > > > + regulators {
> > > > > + compatible = "qcom,rpm-pm2250-regulators";
> > > >
> > > > Since the commit message indicates this is a PM4125 PMIC, should the
> > > > compatible string include a specific identifier for the actual hardware?
> > > >
> > > > Using a specific identifier like "qcom,rpm-pm4125-regulators" alongside
> > > > the fallback allows the kernel to apply specific quirks if PM4125 errata
> > > > are discovered later.
> > >
> > > This can be ignored. The compat is a leftover from the historically
> > > incorrect naming.
> > >
> > > >
> > > > > +
> > > > > + pm4125_s2: s2 {
> > > > > + regulator-min-microvolt = <1000000>;
> > > > > + regulator-max-microvolt = <1200000>;
> > > > > + };
> > > >
> > > > Do these regulators need to explicitly define their input supply dependencies
> > > > such as vdd_s2-supply?
> > > >
> > > > Without these properties, the regulator framework might be unaware that the
> > > > PMIC regulators draw power from upstream supplies.
> > > >
> > > > If the kernel dynamically manages the upstream supply and its reference count
> > > > drops to zero, could it be disabled, causing an unexpected power loss for
> > > > downstream components?
> > >
> > > And this is a correct comment. Please provide missing supplies.
> > >
> > As per the Qualcomm system design, the parent-child supply relationship
> > is managed by the RPM firmware, not the Linux regulator framework. The
> > RPM ensures the parent supply is never disabled until all subsystem
> > votes are cleared.
>
> How is this different from other, previous platforms?
This is not different. In the previous platforms too this is taken care from the
RPM/RPMH firmware side, the only case where we may need explicit vote to parent
is for non-rpmh/rpm regulator rails (like i2c based regulator pm8008), which
may have a RPM/RPMH regulator as a parent.
Even on those previous targets the parent rail of all RPM/RPMH regulators are
internally voted by RPM/RPMH FW at proper voltage with required headroom
calculated based on the active child rails. This was done for all the
subsystems (including APPS) regulators.
So no explicit handling from the APPS is required for parent supply.
>
> --
> With best wishes
> Dmitry
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH phy-next 2/2] phy: ti: add PHY driver for TI DS125DF111 Dual-Channel Retimer
From: sashiko-bot @ 2026-05-14 10:31 UTC (permalink / raw)
To: Ioana Ciornei
Cc: robh, krzk+dt, conor+dt, neil.armstrong, olteanv, devicetree,
vkoul, linux-phy
In-Reply-To: <20260513185103.1371809-3-ioana.ciornei@nxp.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Uninitialized memory read in `ds125df111_xlate` due to missing `args_count` validation.
- [Low] Missing explicit header inclusions for macros and memory allocation functions.
--
commit a5dcf247e2e2a6b752d667ac46227925aa476def
Author: Ioana Ciornei <ioana.ciornei@nxp.com>
phy: ti: add PHY driver for TI DS125DF111 Dual-Channel Retimer
This commit adds a generic PHY driver for the TI DS125DF111 Multi-Protocol
Dual-Channel Retimer over I2C. It currently supports 10G and 1G link speeds,
and registers a separate PHY device for each of the two channels to allow
independent configuration.
> diff --git a/drivers/phy/ti/phy-ds125df111.c b/drivers/phy/ti/phy-ds125df111.c
> new file mode 100644
> index 0000000000000..8ac10c603bf70
> --- /dev/null
> +++ b/drivers/phy/ti/phy-ds125df111.c
> @@ -0,0 +1,245 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright 2026 NXP */
> +
> +#include <linux/i2c.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/phy.h>
> +#include <linux/phy/phy.h>
This isn't a bug, but should this include linux/bitfield.h, linux/bits.h,
and linux/slab.h?
The driver uses FIELD_PREP(), GENMASK(), and devm_kzalloc(), but relies on
implicit inclusions for these headers. Explicitly including them can help
prevent build breakages if the include chains are refactored in the future.
[ ... ]
> +static struct phy *ds125df111_xlate(struct device *dev,
> + const struct of_phandle_args *args)
> +{
> + struct ds125df111_priv *priv = dev_get_drvdata(dev);
> + u32 idx = args->args[0];
Is it possible to read uninitialized memory here?
If a consumer's Device Tree reference incorrectly configures #phy-cells = <0>,
of_parse_phandle_with_args() sets args->args_count to 0 and leaves the
args array unmodified.
Since the of_phandle_args structure is often allocated on the stack, for
example in _of_phy_get(), evaluating args->args[0] without verifying that
args->args_count >= 1 could result in evaluating stack garbage against the
maximum channel count.
> +
> + if (idx >= DS125DF111_NUM_CH) {
> + dev_err(dev, "Maximum number of channels is %d\n",
> + DS125DF111_NUM_CH);
> + return ERR_PTR(-EINVAL);
> + }
> +
> + return priv->ch[idx].phy;
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260513185103.1371809-1-ioana.ciornei@nxp.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 v9 3/5] phy: qcom: apq8064-sata: extract UNI PLL register defines
From: sashiko-bot @ 2026-05-14 10:23 UTC (permalink / raw)
To: Dmitry Baryshkov; +Cc: linux-phy, olteanv, neil.armstrong, vkoul
In-Reply-To: <20260513-fd-hdmi-phy-v9-3-ca98c72f1f9f@oss.qualcomm.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The newly added header file `phy-qcom-uniphy.h` uses the `BIT()` macro but does not include `<linux/bits.h>`.
--
commit a5da58070a4428e27ba521e2ed52d6c1d0de97b5
Author: Dmitry Baryshkov <lumag@kernel.org>
phy: qcom: apq8064-sata: extract UNI PLL register defines
The "uni" PLL is shared between several PHYS. Extract the common register
names to a separate header with the hope of later having the common code
to handle PLL in those PHYs.
> diff --git a/drivers/phy/qualcomm/phy-qcom-uniphy.h b/drivers/phy/qualcomm/phy-qcom-uniphy.h
> new file mode 100644
> index 0000000000000..5f18f9593cda7
> --- /dev/null
> +++ b/drivers/phy/qualcomm/phy-qcom-uniphy.h
> @@ -0,0 +1,33 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (c) 2014, The Linux Foundation. All rights reserved.
> + */
> +
> +#ifndef PHY_QCOM_UNIPHY_H
> +#define PHY_QCOM_UNIPHY_H
[ ... ]
> +#define UNIPHY_PLL_STATUS 0x0c0
> +#define UNIPHY_PLL_LOCK BIT(0)
Since this header file uses the BIT() macro, should it include
<linux/bits.h> to ensure it is self-contained?
Relying on implicit inclusions from the C files that include this header
might cause compilation errors in the future if those files do not already
have the macro defined in their include chain.
> +
> +#endif
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260513-fd-hdmi-phy-v9-0-ca98c72f1f9f@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 v9 2/5] phy: qualcomm: hdmi-28lpm: provide dynamic configuration
From: sashiko-bot @ 2026-05-14 10:19 UTC (permalink / raw)
To: Dmitry Baryshkov; +Cc: linux-phy, vkoul, neil.armstrong, olteanv
In-Reply-To: <20260513-fd-hdmi-phy-v9-2-ca98c72f1f9f@oss.qualcomm.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] VCO calibration frequency is calculated with the wrong unit, causing severe truncation and corrupting calibration registers.
- [High] Integer overflow in the `dc_offset` calculation for SDM bypass mode silently overlaps with control bits and severely misconfigures the PLL multiplier.
--
commit 26d0a45f7fc0a0f6484ab8dc0e93e6b81f47871e
Author: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
phy: qualcomm: hdmi-28lpm: provide dynamic configuration
This commit replaces the fixed value configuration tables for the HDMI PHY
with values calculated at runtime.
> diff --git a/drivers/phy/qualcomm/phy-qcom-hdmi-28lpm.c b/drivers/phy/qualcomm/phy-qcom-hdmi-28lpm.c
> index f1e7113e10bda..90d3331313c05 100644
> --- a/drivers/phy/qualcomm/phy-qcom-hdmi-28lpm.c
> +++ b/drivers/phy/qualcomm/phy-qcom-hdmi-28lpm.c
[ ... ]
> @@ -407,6 +202,70 @@ static unsigned long qcom_28lpm_recalc(struct qcom_hdmi_preqmp_phy *hdmi_phy,
> return rate;
> }
>
> +/* This function is close to UNIPHY, but it has slighly different fields */
> +static int qcom_28lpm_set_rate(struct qcom_hdmi_preqmp_phy *hdmi_phy, unsigned long parent_rate,
> + unsigned long vco_freq, u32 div_idx)
> +{
> + unsigned int pixclk = hdmi_phy->hdmi_opts.tmds_char_rate;
> + unsigned int int_ref_freq;
> + unsigned int dc_offset;
> + unsigned int sdm_freq_seed;
> + unsigned int val;
> + bool sdm_mode = false;
> + u32 refclk_cfg;
> + u32 lf_cfg0;
> + u32 lf_cfg1;
> + u64 tmp;
> +
> + dev_dbg(hdmi_phy->dev, "rate=%u, div = %d, vco = %lu", pixclk, div_idx, vco_freq);
> +
> + if (vco_freq % (parent_rate / 2) == 0) {
> + refclk_cfg = FIELD_PREP(HDMI_8960_PHY_CLK0_DIV, 1);
> + int_ref_freq = parent_rate / 2;
> + } else {
> + refclk_cfg = HDMI_8960_PHY_DBLR_EN;
> + int_ref_freq = parent_rate * 2;
> + sdm_mode = true;
> + }
> +
> + dc_offset = vco_freq / int_ref_freq - 1;
Could dc_offset overflow the 6-bit hardware field in SDM bypass mode?
If a high VCO frequency is requested (e.g., a 216 MHz pixel clock yielding a
1080 MHz VCO), int_ref_freq is 13.5 MHz. The calculation for dc_offset
becomes 1080 / 13.5 - 1 = 79 (0x4F).
[ ... ]
> + if (sdm_mode)
> + writel(dc_offset,
> + hdmi_phy->pll_reg + REG_HDMI_8960_PHY_PLL_SDM_CFG0);
> + else
> + writel(HDMI_8960_PHY_SDM_BYP | dc_offset,
> + hdmi_phy->pll_reg + REG_HDMI_8960_PHY_PLL_SDM_CFG0);
This value exceeds the 6-bit boundary. Bit 6 (0x40) overlaps directly with
the HDMI_8960_PHY_SDM_BYP control bit, so the hardware divider would only
receive the truncated lower 5 bits (0x0F = 15). The hardware would therefore
synthesize a VCO of 216 MHz instead of the expected 1080 MHz.
Does the bypass selection logic earlier need an additional constraint to fall
back to fractional SDM mode when the divider limit is exceeded?
> +
> + writel(HDMI_8960_PHY_DITHER | dc_offset,
> + hdmi_phy->pll_reg + REG_HDMI_8960_PHY_PLL_SDM_CFG1);
> +
> + write24(sdm_freq_seed, hdmi_phy->pll_reg + REG_HDMI_8960_PHY_PLL_SDM_CFG2);
> +
> + write16(vco_freq / 1000, hdmi_phy->pll_reg + REG_HDMI_8960_PHY_PLL_VCOCAL_CFG0);
Is dividing by 1000 yielding kHz instead of the expected MHz here?
Since vco_freq is in Hz, for a typical 1080p pixel clock (148.5 MHz),
vco_freq is 742,500,000 Hz. Dividing by 1000 results in 742,500 (0x0B5454).
Because write16 takes a u16 argument, this value would silently truncate to
0x5454, writing 0x54 to both VCOCAL_CFG0 and VCOCAL_CFG1.
The previous fixed configuration tables explicitly programmed 0x02E6 (742) for
this resolution, indicating the hardware expects the value in MHz.
Should this calculation be vco_freq / 1000000 instead?
> +
> + writel(0x3b, hdmi_phy->pll_reg + REG_HDMI_8960_PHY_PLL_VCOCAL_CFG2);
> +
> + return 0;
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260513-fd-hdmi-phy-v9-0-ca98c72f1f9f@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 v9 1/5] drm/msm/hdmi: switch to generic PHY subsystem
From: sashiko-bot @ 2026-05-14 9:45 UTC (permalink / raw)
To: Dmitry Baryshkov; +Cc: vkoul, neil.armstrong, linux-phy, olteanv
In-Reply-To: <20260513-fd-hdmi-phy-v9-1-ca98c72f1f9f@oss.qualcomm.com>
Thank you for your contribution! Sashiko AI review found 9 potential issue(s) to consider:
- [Critical] Resource Mapping Mismatch and DT ABI Breakage
- [High] Missing 'xo' Clock Initialization for MSM8998
- [High] Unbalanced Operations and Framework State Corruption on Early Return
- [High] Integer Overflow on 32-bit Architectures in PLL Calculation
- [High] Illegal Sleeping in Atomic Context
- [High] Missing Hardware Reset for MSM8x60
- [High] Unpowered Register Access in Clock Callbacks
- [Low] PM Callback Conditional Compilation Violation
- [Low] Missing GENERIC_PHY selection in Kconfig
--
(Note: Due to system recitation restrictions, this review uses a free-form
summary instead of inline patch quoting.)
commit 128cc7cf2eaed12c44a5fdd2ac7d194176f30165
Author: Dmitry Baryshkov <lumag@kernel.org>
Subject: drm/msm/hdmi: switch to generic PHY subsystem
This commit changes the MSM HDMI driver to use the generic PHY subsystem.
It moves the PHY drivers to the drivers/phy/qualcomm/ directory to allow
better code sharing.
In the new qmp_hdmi_probe function in phy-qcom-qmp-hdmi-base.c, memory
resources are fetched by index, using 0 for serdes and 5 for phy_reg. Since
existing device trees like qcom,hdmi-phy-8996 map hdmi_phy at index 0 and
hdmi_pll at index 1, does this swap the PHY and PLL register bases and
break the ABI?
Similarly, in phy-qcom-hdmi-preqmp.c, qcom_hdmi_preqmp_probe unconditionally
maps index 1 for pll_reg. Will this cause the probe to fail completely on
platforms like MSM8974 and MSM8x60 whose device trees only define a single
memory resource? Could devm_platform_ioremap_resource_byname be used here?
The new generic driver sets MAX_CLKS to 2 and only retrieves the iface and
ref clocks. The original hdmi_phy_8998.c explicitly retrieved and enabled a
third clock, xo. Without the board crystal oscillator enabled, won't this
lead to PLL lock timeouts and hardware hangs when powering on the MSM8998
PHY?
In msm_hdmi_bridge_atomic_pre_enable, if phy_power_on fails, the code
unlocks the mutex and returns early.
Since this skips clk_prepare_enable for extp_clk, but the disable path
unconditionally calls clk_disable_unprepare, could this cause an unbalanced
clock disable warning?
Additionally, the return value of phy_init is ignored. If phy_init fails,
the PHY framework leaves init_count at 0. Later, phy_exit in the disable
path will execute --phy->init_count, underflowing it to -1. Won't the next
modeset attempt see init_count equal to -1, skip the init callback, and then
cause a synchronous external abort by writing to unpowered registers in
phy_power_on?
In phy-qcom-qmp-hdmi-msm8998.c, qmp_hdmi_8998_pll_get_post_div calculates
rvar1 using HDMI_DEFAULT_REF_CLOCK * rng1 * HZ_PER_MHZ. On 32-bit
architectures, doesn't this overflow the 32-bit unsigned long arithmetic
since the product is roughly 307.2 trillion? The previous driver cast
HDMI_DEFAULT_REF_CLOCK to u64 to prevent this.
The patch replaces udelay loops with readl_poll_timeout in several PHY
power-on functions, and adds usleep_range in power-off functions.
Since these operations are called from the DRM bridge atomic pre-enable
and post-disable callbacks, which run in atomic context, will these sleeping
functions cause kernel warnings and system instability? Should we use
readl_poll_timeout_atomic instead?
In phy-qcom-hdmi-45nm.c, qcom_hdmi_msm8x60_phy_power_off omits the software
reset sequence (REG_HDMI_PHY_CTRL) that was present in the original driver.
Because this register resides in the HDMI controller space, the new PHY
driver does not access it. Does omitting this reset leave the PHY in a hung
or inconsistent state across power cycles?
In the clock framework callbacks like qmp_hdmi_899x_pll_recalc_rate, the
code directly reads PHY MMIO registers. Because the common clock framework
can invoke this callback at any time (for example, reading debugfs), can
this trigger an immediate bus timeout or external abort if the PHY is
runtime-suspended and unclocked?
In the platform driver definitions for the new PHY drivers, the pm field is
assigned directly to the pm_ops structures. This isn't a bug, but should
pm_ptr() be used here to avoid dead code retention when PM is disabled?
The new PHY_QCOM_QMP_HDMI configuration uses generic PHY framework APIs like
devm_phy_create, but its Kconfig definition does not select GENERIC_PHY.
If the framework is disabled, will these APIs be stubbed and cause the
driver to silently fail with -ENOSYS during probe?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260513-fd-hdmi-phy-v9-0-ca98c72f1f9f@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 v2 2/4] dt-bindings: phy: qcom,msm8998-qmp-usb3-phy: Add support for Shikra
From: Krishna Kurapati @ 2026-05-14 6:22 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Neil Armstrong, Vinod Koul, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, Konrad Dybcio, Xiangxu Yin,
Johan Hovold, Loic Poulain, Kathiravan Thirumoorthy,
Dmitry Baryshkov, linux-arm-msm, linux-phy, devicetree,
linux-kernel
In-Reply-To: <5baeeeed-7c18-46f7-82a3-5bb299ba05ef@kernel.org>
On 5/14/2026 12:26 AM, Krzysztof Kozlowski wrote:
> On 07/05/2026 13:37, Krishna Kurapati wrote:
>>
>>
>> On 5/5/2026 7:30 PM, Krzysztof Kozlowski wrote:
>>> On 05/05/2026 15:57, Krishna Kurapati wrote:
>>>>
>>>>
>>>> On 5/5/2026 6:59 PM, Krzysztof Kozlowski wrote:
>>>>> On 05/05/2026 15:27, Krishna Kurapati wrote:
>>>>>>
>>>>>>
>>>>>> On 5/5/2026 4:22 PM, Krzysztof Kozlowski wrote:
>>>>>>> On 05/05/2026 12:49, Krzysztof Kozlowski wrote:
>>>>>>>> On Mon, May 04, 2026 at 10:36:57PM +0530, Krishna Kurapati wrote:
>>>>>>>>> Declare the USB-C QMP PHY present on the Qualcomm Shikra platform.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
>>>>>>>>> ---
>>>>>>>>> .../devicetree/bindings/phy/qcom,msm8998-qmp-usb3-phy.yaml | 2 ++
>>>>>>>>> 1 file changed, 2 insertions(+)
>>>>>>>>
>>>>>>>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
>>>>>>>
>>>>>>> ... and then I looked at the driver. So un-reviewed. Devices are clearly
>>>>>>> compatible. If not, explain what is not compatible.
>>>>>>>
>>>>>> Talos uses GCC_USB3_PRIM_PHY_AUX_CLK.
>>>>>>
>>>>>> In Shikra, we are using GCC_USB3_PRIM_PHY_COM_AUX_CLK. We don't have
>>>>>> GCC_USB3_PRIM_PHY_AUX_CLK.
>>>>>>
>>>>>> Hence, I didn't use a fallback compatible.
>>>>>
>>>>> This still explains nothing. How different clock makes interface for SW
>>>>> incompatible exactly?
>>>>>
>>>> So I went by the naming. AUX vs COM_AUX.
>>>
>>> The naming does not matter. If the clock is called
>>> "no_one_expects_spanish_inquisition", does that make software
>>> incompatible? Why would the name itself matter?
>>>
>>>>
>>>> Can I use a fallback compatible and in DT vote for "COM_AUX" clock with
>>>> clock-names mentioning "aux" ?
>>>
>>> I don't know, I asked what is different in software interface.
>>>
>>
>> Hi Krzysztof,
>>
>> I checked with the hw team here and found out two things.
>>
>> 1. Shikra is a spinoff of Agatti and its sw interface (clocks used and
>> regulators used) is the same as agatti.
>>
>> 2. I thought we could use qcm2290 as a fallback since the phy register
>> init sequence is the same for Talos/Shikra/Agatti. The difference
>> between Talos and agatti when checked in the driver was the init load
>> settings. I checked with the hw team and they suggested using the init
>> load settings which talos was using.
>>
>> Hence both these compatibles (qcm2290 and qcs615) cannot be used as
>> fallback for Shikra.
>
> Then I do not understand why you are using qcs615_usb3phy_cfg for
> Shikra. You say that the initialization is different, but you use
> exactly the same initialization. So in a meaning of compatibility
> between hardware for Devicetree they are compatible.
>
Hi Krzysztof,
There are 3 things:
1. Clocks used:
-> Talos supports AUX Clock since it supports DP over USB.
-> Agatti and Shikra use COM_AUX clock since they dont support DP over USB.
2. Phy register Init sequence - same for all 3 targets
3. Regulator init load:
-> Different for both Talos and Agatti
-> Recommendation is to use Talos regulator load values.
SW interface wise, shikra is comaptible with agatti. If we use agatti as
fallback, we would end up using the platform data of Agatti where the
regulator init load is not suitable for Shikra. Hence not using Agatti
as fallback.
Coming to driver changes, I used qcs615_cfg because it has required phy
register sequence and regulator init load as needed by shikra.
Regards,
Krishna,
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v5 2/2] phy: add basic support for NXPs TJA1145 CAN transceiver
From: sashiko-bot @ 2026-05-14 5:44 UTC (permalink / raw)
To: Dimitri Fedrau via B4 Relay
Cc: olteanv, krzk+dt, vkoul, linux-phy, conor+dt, robh, devicetree,
neil.armstrong
In-Reply-To: <20260513-tja1145-support-v5-2-38720a7ee63e@liebherr.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The driver fails to enable the hardware power supplies (`vcc-supply`, `vio-supply`, `vbat-supply`) before communicating with the device.
- [High] The driver does not observe the required mode transition delay when powering on the PHY.
- [Medium] Missing rollback to standby mode in the error path of `tja1145_phy_power_on`.
--
commit 288134e179992cb2e690ff7a97502e72f04e9d5a
Author: Dimitri Fedrau <dimitri.fedrau@liebherr.com>
phy: add basic support for NXPs TJA1145 CAN transceiver
This commit adds basic driver support for the NXP TJA1145 CAN transceiver.
It implements the PHY operations to bring the hardware up and down by
switching between normal and standby modes using SPI commands.
> diff --git a/drivers/phy/phy-nxp-tja1145.c b/drivers/phy/phy-nxp-tja1145.c
> new file mode 100644
> index 0000000000000..1e8bd169743ab
> --- /dev/null
> +++ b/drivers/phy/phy-nxp-tja1145.c
[ ... ]
> +static int tja1145_phy_power_on(struct phy *phy)
> +{
> + struct regmap *map = phy_get_drvdata(phy);
> + int ret;
> +
> + /*
> + * Switch operating mode to normal which is the active operating mode.
> + * In this mode, the device is fully operational.
> + */
> + ret = regmap_update_bits(map, TJA1145_MODE_CTRL, TJA1145_MODE_CTRL_MC,
> + TJA1145_MODE_CTRL_NORMAL);
> + if (ret)
> + return ret;
According to the hardware datasheet, transitioning the transceiver to normal
mode can take up to 40 us. Does this sequence need a hardware transition
delay before sending the active mode command?
With a fast SPI bus, the next command might arrive before the hardware state
machine finishes transitioning, which could cause the active mode command to
be ignored.
> +
> + /*
> + * Switch to CAN operating mode active where the PHY can transmit and
> + * receive data.
> + */
> + return regmap_update_bits(map, TJA1145_CAN_CTRL, TJA1145_CAN_CTRL_CMC,
> + TJA1145_CAN_CTRL_ACTIVE);
If this second regmap_update_bits() fails, the function returns an error
without reverting the device to standby mode.
Since the PHY framework keeps the power_count at 0 on an error return,
phy_power_off() will not be called during shutdown or suspend. Could this
leave the hardware permanently stuck in normal mode, drawing power
unintentionally?
> +}
[ ... ]
> +static int tja1145_probe(struct spi_device *spi)
> +{
> + struct phy_provider *phy_provider;
> + struct device *dev = &spi->dev;
> + struct regmap *map;
> + struct phy *phy;
> + int ret;
> +
> + map = devm_regmap_init_spi(spi, &tja1145_regmap_config);
> + if (IS_ERR(map))
> + return dev_err_probe(dev, PTR_ERR(map), "failed to init regmap\n");
> +
> + ret = tja1145_check_ident(dev, map);
Will this fail to communicate with the hardware on platforms where power
supplies are dynamically controlled?
The device tree binding defines vcc-supply, vio-supply, and vbat-supply as
power sources, but they do not appear to be requested or enabled in this
function before tja1145_check_ident() is called.
This might leave the PHY unpowered, causing the SPI read to return garbage
data and fail the probe with -ENODEV.
> + if (ret)
> + return dev_err_probe(dev, ret, "failed to identify device\n");
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260513-tja1145-support-v5-0-38720a7ee63e@liebherr.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 v5 1/2] dt-bindings: phy: add support for NXPs TJA1145 CAN transceiver
From: sashiko-bot @ 2026-05-14 5:17 UTC (permalink / raw)
To: Dimitri Fedrau via B4 Relay
Cc: linux-phy, conor+dt, devicetree, vkoul, olteanv, robh,
neil.armstrong, krzk+dt
In-Reply-To: <20260513-tja1145-support-v5-1-38720a7ee63e@liebherr.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The dt-binding omits support for the `max-bitrate` property.
- [Medium] The dt-binding omits support for the `interrupts` property for wake-up routing.
--
commit 853ad7c93ebe1c2a625ddddde401c4aaad496a8e
Author: Dimitri Fedrau <dimitri.fedrau@liebherr.com>
dt-bindings: phy: add support for NXPs TJA1145 CAN transceiver
This commit adds documentation for the NXP TJA1145 CAN transceiver, residing
in the generic PHY subsystem bindings. It describes the required SPI
configurations and supply properties for the device.
> diff --git a/Documentation/devicetree/bindings/phy/nxp,tja1145.yaml b/Documentation/devicetree/bindings/phy/nxp,tja1145.yaml
> new file mode 100644
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/nxp,tja1145.yaml
[ ... ]
> + vbat-supply:
> + description:
> + Battery supply voltage
> +
> +required:
> + - compatible
> + - reg
> + - "#phy-cells"
> + - spi-cpha
> +
> +additionalProperties: false
Does this binding need to include the max-bitrate property?
This property is standard for CAN transceivers to describe board-level
capability constraints where the layout cannot support the chip's theoretical
maximum speed. Since additionalProperties is set to false, valid device trees
specifying a maximum bitrate will fail validation.
Also, does this device tree binding need support for the interrupts property?
The TJA1145 supports partial networking and wake-up functionality, which can
route a wake-up signal to an interrupt line on the host processor. With
additionalProperties set to false, it is impossible to describe this hardware
interrupt routing in the device tree.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260513-tja1145-support-v5-0-38720a7ee63e@liebherr.com?part=1
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* [PATCH 2/2] phy: qcom-qmp-pcie: Add support for ipq5210 PCIe phys
From: Varadarajan Narayanan @ 2026-05-14 3:58 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: linux-arm-msm, linux-phy, devicetree, linux-kernel,
Varadarajan Narayanan
In-Reply-To: <20260514-pci-phy-v1-0-482429192746@oss.qualcomm.com>
Add support for a PCIe phys found on Qualcomm ipq5210 platform.
Signed-off-by: Varadarajan Narayanan <varadarajan.narayanan@oss.qualcomm.com>
---
drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 152 +++++++++++++++++++++++++++++++
1 file changed, 152 insertions(+)
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
index fed2fc9bb311..5887306eb67b 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
@@ -514,6 +514,105 @@ static const struct qmp_phy_init_tbl ipq8074_pcie_gen3_pcs_misc_tbl[] = {
QMP_PHY_INIT_CFG(QPHY_V4_PCS_PCIE_ENDPOINT_REFCLK_DRIVE, 0xc1),
};
+static const struct qmp_phy_init_tbl ipq5210_gen3x1_pcie_ep_serdes_tbl[] = {
+ QMP_PHY_INIT_CFG(QSERDES_PLL_BG_TIMER, 0x02),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_BIAS_EN_CLKBUFLR_EN, 0x18),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_SYSCLK_BUF_ENABLE, 0x07),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_PLL_IVCO, 0x0f),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_LOCK_CMP1_MODE0, 0xff),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_LOCK_CMP2_MODE0, 0x04),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_LOCK_CMP1_MODE1, 0xff),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_LOCK_CMP2_MODE1, 0x09),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_BG_TRIM, 0x0f),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_CP_CTRL_MODE0, 0x23),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_CP_CTRL_MODE1, 0x23),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_PLL_RCTRL_MODE0, 0x10),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_PLL_RCTRL_MODE1, 0x10),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_PLL_CCTRL_MODE0, 0x01),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_PLL_CCTRL_MODE1, 0x01),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_BIAS_EN_CTRL_BY_PSM, 0x01),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_SYSCLK_EN_SEL, 0x00),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_RESETSM_CNTRL, 0x20),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_LOCK_CMP_EN, 0x42),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_DEC_START_MODE0, 0x19),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_DEC_START_MODE1, 0x14),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_INTEGLOOP_GAIN0_MODE0, 0xfe),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_INTEGLOOP_GAIN1_MODE0, 0x03),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_INTEGLOOP_GAIN0_MODE1, 0xfe),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_INTEGLOOP_GAIN1_MODE1, 0x03),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_VCO_TUNE1_MODE0, 0x24),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_VCO_TUNE2_MODE0, 0x02),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_VCO_TUNE1_MODE1, 0xb4),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_VCO_TUNE2_MODE1, 0x03),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_HSCLK_SEL, 0x01),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_CORE_CLK_EN, 0x00),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_CMN_CONFIG, 0x06),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_SVS_MODE_CLK_SEL, 0x05),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_CORECLK_DIV_MODE1, 0x08),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_SYS_CLK_CTRL, 0x07),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_SYSCLK_EN_SEL, 0x10),
+};
+
+static const struct qmp_phy_init_tbl ipq5210_gen3x1_pcie_ep_tx_tbl[] = {
+ QMP_PHY_INIT_CFG(QSERDES_V4_TX_RES_CODE_LANE_OFFSET_TX, 0x02),
+ QMP_PHY_INIT_CFG(QSERDES_V4_TX_LANE_MODE_1, 0x06),
+ QMP_PHY_INIT_CFG(QSERDES_V4_TX_RCV_DETECT_LVL_2, 0x12),
+};
+
+static const struct qmp_phy_init_tbl ipq5210_gen3x1_pcie_ep_rx_tbl[] = {
+ QMP_PHY_INIT_CFG(QSERDES_V4_RX_UCDR_FO_GAIN, 0x0c),
+ QMP_PHY_INIT_CFG(QSERDES_V4_RX_UCDR_SO_GAIN, 0x02),
+ QMP_PHY_INIT_CFG(QSERDES_V4_RX_UCDR_SO_SATURATION_AND_ENABLE, 0x7f),
+ QMP_PHY_INIT_CFG(QSERDES_V4_RX_UCDR_PI_CONTROLS, 0x70),
+ QMP_PHY_INIT_CFG(QSERDES_V4_RX_RX_EQU_ADAPTOR_CNTRL2, 0x61),
+ QMP_PHY_INIT_CFG(QSERDES_V4_RX_RX_EQU_ADAPTOR_CNTRL3, 0x04),
+ QMP_PHY_INIT_CFG(QSERDES_V4_RX_RX_EQU_ADAPTOR_CNTRL4, 0x1e),
+ QMP_PHY_INIT_CFG(QSERDES_V4_RX_RX_IDAC_TSETTLE_LOW, 0xc0),
+ QMP_PHY_INIT_CFG(QSERDES_V4_RX_RX_IDAC_TSETTLE_HIGH, 0x00),
+ QMP_PHY_INIT_CFG(QSERDES_V4_RX_RX_EQ_OFFSET_ADAPTOR_CNTRL1, 0x73),
+ QMP_PHY_INIT_CFG(QSERDES_V4_RX_RX_OFFSET_ADAPTOR_CNTRL2, 0x80),
+ QMP_PHY_INIT_CFG(QSERDES_V4_RX_SIGDET_ENABLES, 0x03),
+ QMP_PHY_INIT_CFG(QSERDES_V4_RX_SIGDET_DEGLITCH_CNTRL, 0x14),
+ QMP_PHY_INIT_CFG(QSERDES_V4_RX_RX_MODE_00_LOW, 0xf0),
+ QMP_PHY_INIT_CFG(QSERDES_V4_RX_RX_MODE_00_HIGH, 0x01),
+ QMP_PHY_INIT_CFG(QSERDES_V4_RX_RX_MODE_00_HIGH2, 0x2f),
+ QMP_PHY_INIT_CFG(QSERDES_V4_RX_RX_MODE_00_HIGH3, 0xd3),
+ QMP_PHY_INIT_CFG(QSERDES_V4_RX_RX_MODE_00_HIGH4, 0x40),
+ QMP_PHY_INIT_CFG(QSERDES_V4_RX_RX_MODE_01_LOW, 0x01),
+ QMP_PHY_INIT_CFG(QSERDES_V4_RX_RX_MODE_01_HIGH, 0x02),
+ QMP_PHY_INIT_CFG(QSERDES_V4_RX_RX_MODE_01_HIGH2, 0xc8),
+ QMP_PHY_INIT_CFG(QSERDES_V4_RX_RX_MODE_01_HIGH3, 0x09),
+ QMP_PHY_INIT_CFG(QSERDES_V4_RX_RX_MODE_01_HIGH4, 0xb1),
+ QMP_PHY_INIT_CFG(QSERDES_V4_RX_RX_MODE_10_LOW, 0x00),
+ QMP_PHY_INIT_CFG(QSERDES_V4_RX_RX_MODE_10_HIGH, 0x02),
+ QMP_PHY_INIT_CFG(QSERDES_V4_RX_RX_MODE_10_HIGH2, 0xc8),
+ QMP_PHY_INIT_CFG(QSERDES_V4_RX_RX_MODE_10_HIGH3, 0x09),
+ QMP_PHY_INIT_CFG(QSERDES_V4_RX_RX_MODE_10_HIGH4, 0xb1),
+ QMP_PHY_INIT_CFG(QSERDES_V4_RX_DFE_EN_TIMER, 0x04),
+};
+
+static const struct qmp_phy_init_tbl ipq5210_gen3x1_pcie_ep_pcs_tbl[] = {
+ QMP_PHY_INIT_CFG(QPHY_V4_PCS_FLL_CNTRL1, 0x01),
+ QMP_PHY_INIT_CFG(QPHY_V4_PCS_REFGEN_REQ_CONFIG1, 0x0d),
+ QMP_PHY_INIT_CFG(QPHY_V4_PCS_G12S1_TXDEEMPH_M3P5DB, 0x10),
+ QMP_PHY_INIT_CFG(QPHY_V4_PCS_RX_SIGDET_LVL, 0xaa),
+ QMP_PHY_INIT_CFG(QPHY_V4_PCS_P2U3_WAKEUP_DLY_TIME_AUXCLK_L, 0x01),
+ QMP_PHY_INIT_CFG(QPHY_V4_PCS_RX_DCC_CAL_CONFIG, 0x01),
+ QMP_PHY_INIT_CFG(QPHY_V4_PCS_EQ_CONFIG5, 0x01),
+};
+
+static const struct qmp_phy_init_tbl ipq5210_gen3x1_pcie_ep_pcs_misc_tbl[] = {
+ QMP_PHY_INIT_CFG(QPHY_V4_PCS_PCIE_POWER_STATE_CONFIG2, 0x04),
+ QMP_PHY_INIT_CFG(QPHY_V4_PCS_PCIE_POWER_STATE_CONFIG4, 0x07),
+ QMP_PHY_INIT_CFG(QPHY_V4_PCS_PCIE_L1P1_WAKEUP_DLY_TIME_AUXCLK_L, 0x01),
+ QMP_PHY_INIT_CFG(QPHY_V4_PCS_PCIE_L1P2_WAKEUP_DLY_TIME_AUXCLK_L, 0x01),
+ QMP_PHY_INIT_CFG(QPHY_V4_PCS_PCIE_EQ_CONFIG1, 0x11),
+ QMP_PHY_INIT_CFG(QPHY_V4_PCS_PCIE_PRESET_P10_PRE, 0x00),
+ QMP_PHY_INIT_CFG(QPHY_V4_PCS_PCIE_PRESET_P10_POST, 0x58),
+ QMP_PHY_INIT_CFG(QPHY_V4_PCS_PCIE_OSC_DTCT_CONFIG1, 0x02),
+ QMP_PHY_INIT_CFG(QPHY_V4_PCS_PCIE_OSC_DTCT_CONFIG4, 0xff),
+};
+
static const struct qmp_phy_init_tbl ipq9574_gen3x1_pcie_serdes_tbl[] = {
QMP_PHY_INIT_CFG(QSERDES_PLL_BIAS_EN_CLKBUFLR_EN, 0x18),
QMP_PHY_INIT_CFG(QSERDES_PLL_BIAS_EN_CTRL_BY_PSM, 0x01),
@@ -654,6 +753,13 @@ static const struct qmp_phy_init_tbl ipq9574_gen3x2_pcie_serdes_tbl[] = {
QMP_PHY_INIT_CFG(QSERDES_PLL_CLK_ENABLE1, 0x10),
};
+static const struct qmp_phy_init_tbl ipq9574_pcie_tx_tbl[] = {
+ QMP_PHY_INIT_CFG(QSERDES_V4_TX_RES_CODE_LANE_OFFSET_TX, 0x02),
+ QMP_PHY_INIT_CFG(QSERDES_V4_TX_RCV_DETECT_LVL_2, 0x12),
+ QMP_PHY_INIT_CFG(QSERDES_V4_TX_HIGHZ_DRVR_EN, 0x10),
+ QMP_PHY_INIT_CFG(QSERDES_V4_TX_LANE_MODE_1, 0x06),
+};
+
static const struct qmp_phy_init_tbl ipq9574_pcie_rx_tbl[] = {
QMP_PHY_INIT_CFG(QSERDES_V4_RX_SIGDET_CNTRL, 0x03),
QMP_PHY_INIT_CFG(QSERDES_V4_RX_SIGDET_ENABLES, 0x1c),
@@ -3613,6 +3719,49 @@ static const struct qmp_phy_cfg ipq6018_pciephy_cfg = {
.phy_status = PHYSTATUS,
};
+static const struct qmp_phy_cfg ipq5210_gen3x1_pciephy_cfg = {
+ .lanes = 1,
+
+ .offsets = &qmp_pcie_offsets_v4x1,
+
+ .tbls = {
+ .serdes = ipq9574_gen3x1_pcie_serdes_tbl,
+ .serdes_num = ARRAY_SIZE(ipq9574_gen3x1_pcie_serdes_tbl),
+ .tx = ipq9574_pcie_tx_tbl,
+ .tx_num = ARRAY_SIZE(ipq9574_pcie_tx_tbl),
+ .rx = ipq9574_pcie_rx_tbl,
+ .rx_num = ARRAY_SIZE(ipq9574_pcie_rx_tbl),
+ .pcs = ipq9574_gen3x1_pcie_pcs_tbl,
+ .pcs_num = ARRAY_SIZE(ipq9574_gen3x1_pcie_pcs_tbl),
+ .pcs_misc = ipq9574_gen3x1_pcie_pcs_misc_tbl,
+ .pcs_misc_num = ARRAY_SIZE(ipq9574_gen3x1_pcie_pcs_misc_tbl),
+ },
+
+ .tbls_ep = &(const struct qmp_phy_cfg_tbls) {
+ .serdes = ipq5210_gen3x1_pcie_ep_serdes_tbl,
+ .serdes_num = ARRAY_SIZE(ipq5210_gen3x1_pcie_ep_serdes_tbl),
+ .tx = ipq5210_gen3x1_pcie_ep_tx_tbl,
+ .tx_num = ARRAY_SIZE(ipq5210_gen3x1_pcie_ep_tx_tbl),
+ .rx = ipq5210_gen3x1_pcie_ep_rx_tbl,
+ .rx_num = ARRAY_SIZE(ipq5210_gen3x1_pcie_ep_rx_tbl),
+ .pcs = ipq5210_gen3x1_pcie_ep_pcs_tbl,
+ .pcs_num = ARRAY_SIZE(ipq5210_gen3x1_pcie_ep_pcs_tbl),
+ .pcs_misc = ipq5210_gen3x1_pcie_ep_pcs_misc_tbl,
+ .pcs_misc_num = ARRAY_SIZE(ipq5210_gen3x1_pcie_ep_pcs_misc_tbl),
+ },
+
+ .reset_list = ipq8074_pciephy_reset_l,
+ .num_resets = ARRAY_SIZE(ipq8074_pciephy_reset_l),
+ .vreg_list = NULL,
+ .num_vregs = 0,
+ .regs = pciephy_v4_regs_layout,
+
+ .pwrdn_ctrl = SW_PWRDN | REFCLK_DRV_DSBL,
+ .phy_status = PHYSTATUS,
+
+ .pipe_clock_rate = 250000000,
+};
+
static const struct qmp_phy_cfg ipq9574_gen3x1_pciephy_cfg = {
.lanes = 1,
@@ -5404,6 +5553,9 @@ static const struct of_device_id qmp_pcie_of_match_table[] = {
}, {
.compatible = "qcom,glymur-qmp-gen5x4-pcie-phy",
.data = &glymur_qmp_gen5x4_pciephy_cfg,
+ }, {
+ .compatible = "qcom,ipq5210-qmp-gen3x1-pcie-phy",
+ .data = &ipq5210_gen3x1_pciephy_cfg,
}, {
.compatible = "qcom,ipq6018-qmp-pcie-phy",
.data = &ipq6018_pciephy_cfg,
--
2.34.1
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* [PATCH 1/2] dt-bindings: phy: qcom,ipq8074-qmp-pcie: Document the ipq5210 QMP PCIe PHY
From: Varadarajan Narayanan @ 2026-05-14 3:58 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: linux-arm-msm, linux-phy, devicetree, linux-kernel,
Varadarajan Narayanan
In-Reply-To: <20260514-pci-phy-v1-0-482429192746@oss.qualcomm.com>
Document the PCIe phy on the ipq5210 platform using the ipq9574 bindings
as a fallback, since the PCIe phy on the ipq5210 is similar to ipq9574.
Signed-off-by: Varadarajan Narayanan <varadarajan.narayanan@oss.qualcomm.com>
---
Documentation/devicetree/bindings/phy/qcom,ipq8074-qmp-pcie-phy.yaml | 3 +++
1 file changed, 3 insertions(+)
diff --git a/Documentation/devicetree/bindings/phy/qcom,ipq8074-qmp-pcie-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,ipq8074-qmp-pcie-phy.yaml
index f60804687412..1ac31439c20a 100644
--- a/Documentation/devicetree/bindings/phy/qcom,ipq8074-qmp-pcie-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/qcom,ipq8074-qmp-pcie-phy.yaml
@@ -17,6 +17,7 @@ properties:
compatible:
oneOf:
- enum:
+ - qcom,ipq5210-qmp-gen3x1-pcie-phy
- qcom,ipq6018-qmp-pcie-phy
- qcom,ipq8074-qmp-gen3-pcie-phy
- qcom,ipq8074-qmp-pcie-phy
@@ -24,10 +25,12 @@ properties:
- qcom,ipq9574-qmp-gen3x2-pcie-phy
- items:
- enum:
+ - qcom,ipq5210-qmp-gen3x1-pcie-phy
- qcom,ipq5424-qmp-gen3x1-pcie-phy
- const: qcom,ipq9574-qmp-gen3x1-pcie-phy
- items:
- enum:
+ - qcom,ipq5210-qmp-gen3x2-pcie-phy
- qcom,ipq5424-qmp-gen3x2-pcie-phy
- const: qcom,ipq9574-qmp-gen3x2-pcie-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 0/2] Enable the QMP PCIe PHY present in Qualcomm ipq5210 SoC
From: Varadarajan Narayanan @ 2026-05-14 3:58 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: linux-arm-msm, linux-phy, devicetree, linux-kernel,
Varadarajan Narayanan
Document the bindings and update the driver to support
the PCIe phy present in Qualcomm ipq5210 SoC.
Signed-off-by: Varadarajan Narayanan <varadarajan.narayanan@oss.qualcomm.com>
---
Varadarajan Narayanan (2):
dt-bindings: phy: qcom,ipq8074-qmp-pcie: Document the ipq5210 QMP PCIe PHY
phy: qcom-qmp-pcie: Add support for ipq5210 PCIe phys
.../bindings/phy/qcom,ipq8074-qmp-pcie-phy.yaml | 3 +
drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 152 +++++++++++++++++++++
2 files changed, 155 insertions(+)
---
base-commit: e98d21c170b01ddef366f023bbfcf6b31509fa83
change-id: 20260514-pci-phy-38ec9b1c5a90
Best regards,
--
Varadarajan Narayanan <varadarajan.narayanan@oss.qualcomm.com>
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* [PATCH v2] phy: ti: pipe3: Fix clock resource leak on probe errors
From: Hongling Zeng @ 2026-05-14 2:34 UTC (permalink / raw)
To: vkoul, neil.armstrong, johan, kishon
Cc: linux-phy, linux-kernel, zhongling0719, Hongling Zeng
When devm_phy_create() or devm_of_phy_provider_register() fails,
the refclk that was enabled earlier is not disabled, causing a
resource leak.
Fix this by adding an error handling path to disable the clock
when these functions fail.
Fixes: 234738ea3390 ("phy: ti-pipe3: move clk initialization to a separate function")
Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
---
Change in v2:
-Add pm_runtime_disable() in error path (reported by Sashiko AI).
---
drivers/phy/ti/phy-ti-pipe3.c | 19 ++++++++++++++++---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/drivers/phy/ti/phy-ti-pipe3.c b/drivers/phy/ti/phy-ti-pipe3.c
index b5543b5c674c..5f4a81f6d2b9 100644
--- a/drivers/phy/ti/phy-ti-pipe3.c
+++ b/drivers/phy/ti/phy-ti-pipe3.c
@@ -837,15 +837,28 @@ static int ti_pipe3_probe(struct platform_device *pdev)
}
generic_phy = devm_phy_create(dev, NULL, &ops);
- if (IS_ERR(generic_phy))
- return PTR_ERR(generic_phy);
+ if (IS_ERR(generic_phy)) {
+ ret = PTR_ERR(generic_phy);
+ goto err_clk_disable;
+ }
phy_set_drvdata(generic_phy, phy);
ti_pipe3_power_off(generic_phy);
phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
- return PTR_ERR_OR_ZERO(phy_provider);
+ if (IS_ERR(phy_provider)) {
+ ret = PTR_ERR(phy_provider);
+ goto err_clk_disable;
+ }
+
+ return 0;
+
+err_clk_disable:
+ pm_runtime_disable(dev);
+ if (phy->sata_refclk_enabled)
+ clk_disable_unprepare(phy->refclk);
+ return ret;
}
static void ti_pipe3_remove(struct platform_device *pdev)
--
2.25.1
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* Re: [PATCH] phy: ti: pipe3: Fix clock resource leak on probe errors
From: sashiko-bot @ 2026-05-14 2:10 UTC (permalink / raw)
To: Hongling Zeng; +Cc: linux-phy, vkoul, neil.armstrong, olteanv
In-Reply-To: <20260513063902.113956-1-zenghongling@kylinos.cn>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing pm_runtime_disable() in the probe error path leaves PM runtime state unbalanced.
--
commit fd04e6bd036402bf170a7ff7978b95f1478bfd7b
Author: Hongling Zeng <zenghongling@kylinos.cn>
phy: ti: pipe3: Fix clock resource leak on probe errors
This commit addresses a clock resource leak in the probe error path of the
TI PIPE3 PHY driver. It adds an err_clk_disable label to ensure the reference
clock is disabled if PHY creation or provider registration fails.
> diff --git a/drivers/phy/ti/phy-ti-pipe3.c b/drivers/phy/ti/phy-ti-pipe3.c
> index b5543b5c674c8..29a4cbe737812 100644
> --- a/drivers/phy/ti/phy-ti-pipe3.c
> +++ b/drivers/phy/ti/phy-ti-pipe3.c
> @@ -837,15 +837,27 @@ static int ti_pipe3_probe(struct platform_device *pdev)
[ ... ]
> phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> - return PTR_ERR_OR_ZERO(phy_provider);
> + if (IS_ERR(phy_provider)) {
> + ret = PTR_ERR(phy_provider);
> + goto err_clk_disable;
> + }
> +
> + return 0;
> +
> +err_clk_disable:
> + if (phy->sata_refclk_enabled)
> + clk_disable_unprepare(phy->refclk);
> + return ret;
Does this new error path leave the power management runtime state unbalanced?
Earlier in ti_pipe3_probe(), pm_runtime_enable(dev) is called. Without a
corresponding pm_runtime_disable() call in this error path, it looks like
the device's power management runtime state is left unbalanced, which can
trigger a warning on subsequent probe retries.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260513063902.113956-1-zenghongling@kylinos.cn?part=1
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v1 2/6] usb: chipidea: tegra: Avoid controller/PHY init if bus is externally controlled
From: Peter Chen (CIX) @ 2026-05-14 1:42 UTC (permalink / raw)
To: Svyatoslav Ryhel
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Vinod Koul, Neil Armstrong, Thierry Reding, Jonathan Hunter,
Greg Kroah-Hartman, netdev, devicetree, linux-kernel, linux-phy,
linux-tegra, linux-usb
In-Reply-To: <CAPVz0n2yGALm1Z0j0j6_MaqAOZGL+a8EgCWYQpW0QN1YVf96EQ@mail.gmail.com>
On 26-05-13 20:30:10, Svyatoslav Ryhel wrote:
> вт, 12 трав. 2026 р. о 10:51 Peter Chen (CIX) <peter.chen@kernel.org> пише:
> >
> > On 26-05-12 09:13:40, Svyatoslav Ryhel wrote:
> > > вт, 12 трав. 2026 р. о 04:16 Peter Chen (CIX) <peter.chen@kernel.org> пише:
> > > >
> > > > On 26-05-11 16:56:57, Svyatoslav Ryhel wrote:
> > > > > If the USB controller and PHY are externally controlled, then the
> > > > > registration of the controller and the PHY initialization should be
> > > > > skipped, since these configurations must be done by the device that
> > > > > controls the bus to work correctly.
> > > > >
> > > >
> > > > I find you only control USB controller device add at PHY driver, most of USB drivers
> > > > has PHY control, for chipidea, it has PHY control at core.c, would please try to
> > > > adapt for it?
> > > >
> > >
> > > Usually yes, but this is not the case for Tegra unfortunately. As you
> > > can see Tegra specific section of Chipidea driver specifically
> > > describes why it has to set PHY manually.
> > >
> > > /*
> > > * USB controller registers shouldn't be touched before PHY is
> > > * initialized, otherwise CPU will hang because clocks are gated.
> > > * PHY driver controls gating of internal USB clocks on Tegra.
> > > */
> > >
> > > So in order to provide correct work of USB when set by an external
> > > device, both PHY and controller init/add must be skipped.
> >
> > You could call generic PHY APIs at ci_hdrc_tegra.c, after PHY init or power on,
> > call controller initialization.
> >
>
> I was tinkering with Chipidea driver for Tegra a bit. If you meant to
> use flag only to control usb controller device init and leave PHY to
> be handled by controller, then yes, that is possible and that is
> better approach.
>
Yes, you may summarize the initialization sequence, and see if put PHY
control at Tegra glue layer or at Chipidea core.
--
Best regards,
Peter
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* [PATCH v5 5/6] phy: phy-can-transceiver: Decouple assignment and definition in probe
From: Andy Shevchenko @ 2026-05-13 22:01 UTC (permalink / raw)
To: Peng Fan, linux-can, linux-phy, linux-kernel
Cc: Marc Kleine-Budde, Vincent Mailhol, Vinod Koul, Neil Armstrong,
Josua Mayer, Ulf Hansson, Andy Shevchenko
In-Reply-To: <20260513220336.369628-1-andriy.shevchenko@linux.intel.com>
The code like
int foo = X;
...
if (bar)
foo = Y;
is prone to subtle mistakes and hence harder to maintain as the foo value
may be changed inadvertently while code in '...' grown in lines. On top
it's harder to navigate to understand the possible values of foo when branch
is not taken (requires to look somewhere else in the code, far from the piece
at hand).
Besides that in case of taken branch the foo will be rewritten, which is
not a problem per se, just an unneeded operation.
Decouple assignment and definition to use if-else to address the inconveniences
described above.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/phy/phy-can-transceiver.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/phy/phy-can-transceiver.c b/drivers/phy/phy-can-transceiver.c
index 3cebaa54f7db..30330499585b 100644
--- a/drivers/phy/phy-can-transceiver.c
+++ b/drivers/phy/phy-can-transceiver.c
@@ -128,8 +128,8 @@ static int can_transceiver_phy_probe(struct platform_device *pdev)
struct gpio_desc *standby_gpio;
struct gpio_desc *enable_gpio;
struct mux_state *mux_state;
- int err, i, num_ch = 1;
const char *propname;
+ int err, i, num_ch;
u32 max_bitrate;
drvdata = device_get_match_data(dev);
@@ -138,6 +138,8 @@ static int can_transceiver_phy_probe(struct platform_device *pdev)
if (drvdata->flags & CAN_TRANSCEIVER_DUAL_CH)
num_ch = 2;
+ else
+ num_ch = 1;
priv = devm_kzalloc(dev, struct_size(priv, can_transceiver_phy, num_ch), GFP_KERNEL);
if (!priv)
--
2.50.1
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* [PATCH v5 6/6] phy: phy-can-transceiver: Drop unused include
From: Andy Shevchenko @ 2026-05-13 22:01 UTC (permalink / raw)
To: Peng Fan, linux-can, linux-phy, linux-kernel
Cc: Marc Kleine-Budde, Vincent Mailhol, Vinod Koul, Neil Armstrong,
Josua Mayer, Ulf Hansson, Andy Shevchenko
In-Reply-To: <20260513220336.369628-1-andriy.shevchenko@linux.intel.com>
This file does not use the symbols from the legacy
<linux/gpio.h> header, so let's drop it.
Reviewed-by: Josua Mayer <josua@solid-run.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/phy/phy-can-transceiver.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/phy/phy-can-transceiver.c b/drivers/phy/phy-can-transceiver.c
index 30330499585b..75dc49e75ca0 100644
--- a/drivers/phy/phy-can-transceiver.c
+++ b/drivers/phy/phy-can-transceiver.c
@@ -5,12 +5,11 @@
* Copyright (C) 2021 Texas Instruments Incorporated - https://www.ti.com
*
*/
+#include <linux/gpio/consumer.h>
#include <linux/phy/phy.h>
#include <linux/platform_device.h>
#include <linux/property.h>
#include <linux/module.h>
-#include <linux/gpio.h>
-#include <linux/gpio/consumer.h>
#include <linux/mux/consumer.h>
struct can_transceiver_data {
--
2.50.1
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* [PATCH v5 4/6] phy: phy-can-transceiver: Don't check for specific errors when parsing properties
From: Andy Shevchenko @ 2026-05-13 22:01 UTC (permalink / raw)
To: Peng Fan, linux-can, linux-phy, linux-kernel
Cc: Marc Kleine-Budde, Vincent Mailhol, Vinod Koul, Neil Armstrong,
Josua Mayer, Ulf Hansson, Andy Shevchenko
In-Reply-To: <20260513220336.369628-1-andriy.shevchenko@linux.intel.com>
Instead of checking for the specific error codes (that can be considered
a layering violation to some extent) check for the property existence first
and then either parse it, or apply a default value.
With that, return an error when parsing of the existing property fails.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/phy/phy-can-transceiver.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/drivers/phy/phy-can-transceiver.c b/drivers/phy/phy-can-transceiver.c
index 5c9698f77c7d..3cebaa54f7db 100644
--- a/drivers/phy/phy-can-transceiver.c
+++ b/drivers/phy/phy-can-transceiver.c
@@ -128,8 +128,9 @@ static int can_transceiver_phy_probe(struct platform_device *pdev)
struct gpio_desc *standby_gpio;
struct gpio_desc *enable_gpio;
struct mux_state *mux_state;
- u32 max_bitrate = 0;
int err, i, num_ch = 1;
+ const char *propname;
+ u32 max_bitrate;
drvdata = device_get_match_data(dev);
if (!drvdata)
@@ -151,9 +152,17 @@ static int can_transceiver_phy_probe(struct platform_device *pdev)
priv->mux_state = mux_state;
- err = device_property_read_u32(dev, "max-bitrate", &max_bitrate);
- if ((err != -EINVAL) && !max_bitrate)
- dev_warn(dev, "Invalid value for transceiver max bitrate. Ignoring bitrate limit\n");
+ propname = "max-bitrate";
+ if (device_property_present(dev, propname)) {
+ err = device_property_read_u32(dev, propname, &max_bitrate);
+ if (err)
+ return dev_err_probe(dev, err, "failed to parse %s\n", propname);
+
+ if (max_bitrate == 0)
+ dev_warn(dev, "Invalid value for transceiver max bitrate. Ignoring bitrate limit\n");
+ } else {
+ max_bitrate = 0;
+ }
for (i = 0; i < num_ch; i++) {
can_transceiver_phy = &priv->can_transceiver_phy[i];
--
2.50.1
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* [PATCH v5 2/6] phy: phy-can-transceiver: use device_get_match_data()
From: Andy Shevchenko @ 2026-05-13 22:01 UTC (permalink / raw)
To: Peng Fan, linux-can, linux-phy, linux-kernel
Cc: Marc Kleine-Budde, Vincent Mailhol, Vinod Koul, Neil Armstrong,
Josua Mayer, Ulf Hansson, Andy Shevchenko
In-Reply-To: <20260513220336.369628-1-andriy.shevchenko@linux.intel.com>
Use the generic firmware node interface for retrieving
device match data instead of the OF-specific one.
While at it, drop unneeded argument to devm_phy_create() which
extracts device node from the given device by default.
Reviewed-by: Josua Mayer <josua@solid-run.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/phy/phy-can-transceiver.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/phy/phy-can-transceiver.c b/drivers/phy/phy-can-transceiver.c
index 1808f903c057..37b706d841ff 100644
--- a/drivers/phy/phy-can-transceiver.c
+++ b/drivers/phy/phy-can-transceiver.c
@@ -5,9 +5,9 @@
* Copyright (C) 2021 Texas Instruments Incorporated - https://www.ti.com
*
*/
-#include <linux/of.h>
#include <linux/phy/phy.h>
#include <linux/platform_device.h>
+#include <linux/property.h>
#include <linux/module.h>
#include <linux/gpio.h>
#include <linux/gpio/consumer.h>
@@ -152,7 +152,6 @@ static int can_transceiver_phy_probe(struct platform_device *pdev)
struct can_transceiver_phy *can_transceiver_phy;
struct can_transceiver_priv *priv;
const struct can_transceiver_data *drvdata;
- const struct of_device_id *match;
struct phy *phy;
struct gpio_desc *silent_gpio;
struct gpio_desc *standby_gpio;
@@ -161,11 +160,10 @@ static int can_transceiver_phy_probe(struct platform_device *pdev)
u32 max_bitrate = 0;
int err, i, num_ch = 1;
- match = of_match_node(can_transceiver_phy_ids, pdev->dev.of_node);
- if (!match || !match->data)
+ drvdata = device_get_match_data(dev);
+ if (!drvdata)
return -ENODEV;
- drvdata = match->data;
if (drvdata->flags & CAN_TRANSCEIVER_DUAL_CH)
num_ch = 2;
@@ -190,7 +188,7 @@ static int can_transceiver_phy_probe(struct platform_device *pdev)
can_transceiver_phy = &priv->can_transceiver_phy[i];
can_transceiver_phy->priv = priv;
- phy = devm_phy_create(dev, dev->of_node, &can_transceiver_phy_ops);
+ phy = devm_phy_create(dev, NULL, &can_transceiver_phy_ops);
if (IS_ERR(phy)) {
dev_err(dev, "failed to create can transceiver phy\n");
return PTR_ERR(phy);
--
2.50.1
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* [PATCH v5 0/6] phy: phy-can-transceiver: Ad-hoc cleanups and refactoring
From: Andy Shevchenko @ 2026-05-13 22:01 UTC (permalink / raw)
To: Peng Fan, linux-can, linux-phy, linux-kernel
Cc: Marc Kleine-Budde, Vincent Mailhol, Vinod Koul, Neil Armstrong,
Josua Mayer, Ulf Hansson, Andy Shevchenko
The driver does two things that need to be addressed:
- includes subject to remove gpio.h
- checks for error code from device property APIs when it can be done in
a robust way
This series addresses the above and adds a couple of additional refactoring.
On top of that it fixes potential NULL pointer dereference when driver_override
is in use.
Changelog v5:
- fixed typo due to rebasing of patch 2 (Sashiko)
- added patch 5 to slightly improve maintainability
v4: 20260512130552.272476-1-andriy.shevchenko@linux.intel.com
Changelog v4:
- prepended the series with match and driver data check (Sashiko, Vinod)
- made the max_bitrate==0 warning only when property is present (Sashiko, Vinod)
- rebased the rest accordingly
v3: 20260504070054.29508-1-andriy.shevchenko@linux.intel.com
Changelog v3:
- fixed commit message in patch 1 (Josua)
- dropped stray change (Vinod)
- collected tags (Josua)
v2: 20260317203001.2108568-1-andriy.shevchenko@linux.intel.com
Changelog v2:
- rebased on top of the latest changes in the driver
- Cc'ed to Ulf and Josua due to above
- elaborated dropping of_node parameter (Vladimir)
v1: 20260219202910.2304440-1-andriy.shevchenko@linux.intel.com
*** BLURB HERE ***
Andy Shevchenko (6):
phy: phy-can-transceiver: Check driver match and driver data against
NULL
phy: phy-can-transceiver: use device_get_match_data()
phy: phy-can-transceiver: Move OF ID table closer to their user
phy: phy-can-transceiver: Don't check for specific errors when parsing
properties
phy: phy-can-transceiver: Decouple assignment and definition in probe
phy: phy-can-transceiver: Drop unused include
drivers/phy/phy-can-transceiver.c | 93 +++++++++++++++++--------------
1 file changed, 52 insertions(+), 41 deletions(-)
--
2.50.1
--
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