* Re: [PATCH v3 4/4] phy: qualcomm: qmp-combo: Add support for Hawi SoC
From: Dmitry Baryshkov @ 2026-07-20 22:24 UTC (permalink / raw)
To: sashiko
Cc: Ronak Raheja, robh, krzk+dt, olteanv, devicetree, vkoul, conor+dt,
neil.armstrong, linux-phy, linux-arm-msm
In-Reply-To: <20260508232620.2FD04C2BCB0@smtp.kernel.org>
On Fri, May 08, 2026 at 11:26:19PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] Missing DP PHY function pointers in `hawi_usb3dpphy_cfg` lead to NULL pointer dereferences when the DP PHY is initialized.
> - [High] Valid register offset `0x00` causes the AON toggle hardware reset override to be silently skipped.
Please add DP programming. There should be no NULL dereferences.
> > @@ -3699,9 +3915,9 @@ static int qmp_combo_com_init(struct qmp_combo *qmp, bool force)
> > SW_USB3PHY_RESET_MUX | SW_USB3PHY_RESET);
> >
> > /* override hardware control for reset of qmp phy */
> > - if (pcs_aon && cfg->regs[QPHY_AON_TOGGLE_ENABLE]) {
> > - qphy_clrbits(pcs_aon, cfg->regs[QPHY_AON_TOGGLE_ENABLE], 0x1);
> > - qphy_clrbits(pcs_aon, cfg->regs[QPHY_DP_AON_TOGGLE_ENABLE], 0x1);
> > + if (aon_toggle && cfg->regs[QPHY_AON_TOGGLE_ENABLE]) {
> > + qphy_clrbits(aon_toggle, cfg->regs[QPHY_AON_TOGGLE_ENABLE], 0x1);
> > + qphy_clrbits(aon_toggle, cfg->regs[QPHY_DP_AON_TOGGLE_ENABLE], 0x1);
> > }
>
> Is it possible for the hardware reset override to be silently skipped here?
>
> The V10 register layout maps QPHY_AON_TOGGLE_ENABLE to
> QPHY_V10_COM_AON_USB3_AON_TOGGLE_ENABLE, which is defined as 0x00 in
> phy-qcom-qmp-com-aon-v10.h.
>
> Because 0x00 is a valid register offset but evaluates to false in C, the
> check for cfg->regs[QPHY_AON_TOGGLE_ENABLE] will fail. This would skip the
> qphy_clrbits() calls entirely, bypassing the necessary hardware reset override
> sequence and leaving the PHY in a broken state.
>
> Could we use a different way to check if this register is supported by the
> layout without relying on the truthiness of the offset value?
This deserves a comment.
--
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 v3 phy-next 8/8] phy: lynx-10g: use RCW override procedure for dynamic protocol change
From: Vladimir Oltean @ 2026-07-20 20:12 UTC (permalink / raw)
To: Vinod Koul
Cc: linux-phy, devicetree, linuxppc-dev, linux-arm-kernel,
Ioana Ciornei, Neil Armstrong, Tanjeff Moos,
Christophe Leroy (CS GROUP), Michael Walle, Shawn Guo, Frank Li,
linux-kernel
In-Reply-To: <al5ODzYhpE-wHSSN@vaman>
Hi Vinod,
On Mon, Jul 20, 2026 at 10:04:23PM +0530, Vinod Koul wrote:
> On 20-07-26, 16:36, Vladimir Oltean wrote:
> > Up until this patch, the only protocol change supported was between
> > 1000Base-X/SGMII and 2500Base-X. The others require an RCW override
> > procedure which was lacking.
> >
> > Since now the guts driver provides the means of applying this procedure,
> > make use of it and remove any comment which mentioned the limitation.
>
> lgtm, is there any dependency. If not I can pick it
>
> --
> ~Vinod
There is no other direct dependency, but I need to send a v4 tomorrow
due to some valid Sashiko comments.
There is also this patch which seems to have been forgotten:
https://patchwork.kernel.org/project/linux-phy/patch/20260612125731.133330-1-vladimir.oltean@nxp.com/
Thanks,
Vladimir
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v4 5/8] phy: rockchip: samsung-hdptx: Drop TMDS rate setup workaround
From: Cristian Ciocaltea @ 2026-07-20 19:24 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Vinod Koul, Neil Armstrong, Heiko Stuebner, Algea Cao,
Dmitry Baryshkov, kernel, linux-phy, linux-arm-kernel,
linux-rockchip, linux-kernel, Thomas Niederprüm,
Simon Wright
In-Reply-To: <al4ujA5d6B5s23C4@umbar.lan>
On 7/20/26 5:19 PM, Dmitry Baryshkov wrote:
> On Fri, Jun 12, 2026 at 02:45:24AM +0300, Cristian Ciocaltea wrote:
>> Since commit ba9c2fe18c17 ("drm/rockchip: dw_hdmi_qp: Switch to
>> phy_configure()") the TMDS rate setup doesn't rely anymore on the
>> unconventional usage of the bus width, instead it is managed exclusively
>> through the HDMI PHY configuration API.
>>
>> Drop the now obsolete workaround to retrieve the TMDS character rate via
>> phy_get_bus_width() during power_on().
>>
>> While at it, get rid of the extra call to rk_hdptx_phy_consumer_put() by
>> moving the statement at the end of the function.
>>
>> Tested-by: Thomas Niederprüm <dubito@online.de>
>> Tested-by: Simon Wright <simon@symple.nz>
>> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
>> ---
>> drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c | 27 +++++------------------
>> 1 file changed, 6 insertions(+), 21 deletions(-)
>>
>> diff --git a/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c b/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
>> index 25bd821cd039..35997087d61c 100644
>> --- a/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
>> +++ b/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
>> @@ -1660,22 +1660,6 @@ static int rk_hdptx_phy_power_on(struct phy *phy)
>> enum phy_mode mode = phy_get_mode(phy);
>> int ret, lane;
>>
>> - if (mode != PHY_MODE_DP) {
>> - if (!hdptx->hdmi_cfg.rate && hdptx->hdmi_cfg.mode != PHY_HDMI_MODE_FRL) {
>> - /*
>> - * FIXME: Temporary workaround to setup TMDS char rate
>> - * from the RK DW HDMI QP bridge driver.
>> - * Will be removed as soon the switch to the HDMI PHY
>> - * configuration API has been completed on both ends.
>> - */
>> - hdptx->hdmi_cfg.rate = phy_get_bus_width(hdptx->phy) & 0xfffffff;
>> - hdptx->hdmi_cfg.rate *= 100;
>> - }
>> -
>> - dev_dbg(hdptx->dev, "%s rate=%llu bpc=%u\n", __func__,
>> - hdptx->hdmi_cfg.rate, hdptx->hdmi_cfg.bpc);
>> - }
>> -
>> ret = rk_hdptx_phy_consumer_get(hdptx);
>> if (ret)
>> return ret;
>> @@ -1701,9 +1685,10 @@ static int rk_hdptx_phy_power_on(struct phy *phy)
>> rk_hdptx_dp_pll_init(hdptx);
>>
>> ret = rk_hdptx_dp_aux_init(hdptx);
>> - if (ret)
>> - rk_hdptx_phy_consumer_put(hdptx, true);
>> } else {
>> + dev_dbg(hdptx->dev, "%s rate=%llu bpc=%u\n", __func__,
>> + hdptx->hdmi_cfg.rate, hdptx->hdmi_cfg.bpc);
>> +
>> if (hdptx->pll_config_dirty)
>> ret = rk_hdptx_pll_cmn_config(hdptx);
>>
>> @@ -1716,11 +1701,11 @@ static int rk_hdptx_phy_power_on(struct phy *phy)
>> else
>> ret = rk_hdptx_tmds_ropll_mode_config(hdptx);
>> }
>> -
>> - if (ret)
>> - rk_hdptx_phy_consumer_put(hdptx, true);
>> }
>>
>> + if (ret)
>> + rk_hdptx_phy_consumer_put(hdptx, true);
>
> This looks likes an unrelated change. Could you please split it to a
> separate commit?
Ack.
Thanks,
Cristian
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v4 4/8] phy: rockchip: samsung-hdptx: Handle uncommitted PHY config changes
From: Cristian Ciocaltea @ 2026-07-20 19:22 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Vinod Koul, Neil Armstrong, Heiko Stuebner, Algea Cao,
Dmitry Baryshkov, kernel, linux-phy, linux-arm-kernel,
linux-rockchip, linux-kernel, Thomas Niederprüm,
Simon Wright
In-Reply-To: <al4t4aN01iiUKff_@umbar.lan>
On 7/20/26 5:17 PM, Dmitry Baryshkov wrote:
> On Fri, Jun 12, 2026 at 02:45:23AM +0300, Cristian Ciocaltea wrote:
>> Any changes to the PHY link rate and/or color depth done via the HDMI
>> PHY configuration API are not immediately programmed into the hardware,
>> but are delayed until the PHY usage count gets incremented from 0 to 1,
>> that is when it is powered on or when the PLL clock exposed through
>> the CCF API is prepared, whichever comes first.
>
> Why do you need either-of here? Can it be just one path?
The thing is that PLL clock usage is dynamic, depending on the rate, but it can
also be disabled permanently. When it's in use, it triggers before power on,
hence the hardware must be programmed at that time.
>>
>> Since the clock might remain in prepared state after subsequent PHY
>> config changes, the programming can also be triggered via
>> clk_ops.set_rate(). However, from the clock consumer perspective (i.e.
>> VOP2 display controller), the (pixel) clock rate doesn't vary with bpc,
>> as that is handled internally by the PHY and reflected in the TDMS
>> character rate only.
>>
>> As a consequence, changing the bpc while preserving the modeline may
>> lead to out-of-sync issues between CCF and HDMI PHY config state,
>> because the .set_rate() callback is not invoked when clock rate remains
>> constant. This may also happen when the PHY PLL has been pre-programmed
>> by an external entity, e.g. the bootloader, which is actually a
>> regression introduced by the recent FRL patches.
>>
>> Introduce a pll_config_dirty flag to keep track of uncommitted PHY
>> config changes and use it in clk_ops.determine_rate() to invalidate the
>> current clock rate (as known by CCF) and, consequently, ensure those
>> changes are programmed into hardware via clk_ops.set_rate().
>>
>> Moreover, proceed with a similar fix in phy_ops.power_on() callback, to
>> handle the scenario where the CCF API is not used due to operating in
>> FRL mode, while the clock is still in a prepared state and thus
>> preventing rk_hdptx_phy_consumer_get() to apply the updated PHY
>> configuration.
>>
>> Fixes: de5dba833118 ("phy: rockchip: samsung-hdptx: Add HDMI 2.1 FRL support")
>> Fixes: 9d0ec51d7c22 ("phy: rockchip: samsung-hdptx: Add high color depth management")
>> Tested-by: Thomas Niederprüm <dubito@online.de>
>> Tested-by: Simon Wright <simon@symple.nz>
>> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
>> ---
>> drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c | 84 +++++++++++++----------
>> 1 file changed, 48 insertions(+), 36 deletions(-)
>>
>> diff --git a/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c b/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
>> index b210c1a88b25..25bd821cd039 100644
>> --- a/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
>> +++ b/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
>> @@ -413,6 +413,7 @@ struct rk_hdptx_phy {
>>
>> /* clk provider */
>> struct clk_hw hw;
>> + bool pll_config_dirty;
>
> Which lock protects it?
This is not protected by a driver-local lock, as the impacted calls are
orchestrated by the DRM core, which should guaranty the expected sequence of
operations:
1. encoder-check -> phy_configure -> no HW programming, pll_config_dirty=true
2. VOP2 crtc-enable -> set_rate -> HW commit, pll_config_dirty=false
3. bridge-enable -> phy_power_on, no HW programming (pll_config_dirty already
false)
>
>> bool restrict_rate_change;
>>
>> atomic_t usage_count;
>
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v4 2/8] phy: rockchip: samsung-hdptx: Prevent divide-by-zero when computing clk rate
From: Cristian Ciocaltea @ 2026-07-20 18:36 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: sashiko-reviews, neil.armstrong, olteanv, linux-phy, vkoul
In-Reply-To: <al4sQ1kNriKH7hHL@umbar.lan>
Hi Dmitry,
Thanks a lot for taking the time to review the series!
On 7/20/26 5:10 PM, Dmitry Baryshkov wrote:
> On Fri, Jun 12, 2026 at 03:18:52AM +0300, Cristian Ciocaltea wrote:
>> On 6/12/26 2:57 AM, sashiko-bot@kernel.org wrote:
>>> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>>>
>>> Pre-existing issues:
>>> - [High] Potential integer underflow in `fout` calculation when hardware state is misconfigured, leading to a massive bogus clock rate that could hang downstream components.
>>> --
>>
>> If the hardware reached such a damaged state, the kernel is likely already encountering more serious issues.
>>
>> Regardless, I will address this in the "FRL TxFFE level control" series, which is currently blocked by this one, to avoid any further delays.
>>
>
> With the overflow handled:
>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Since a new revision is needed anyway, I'll add a new patch to handle this in
v5.
Regards,
Cristian
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v3 1/5] drm/bridge: Implement generic USB Type-C DP HPD bridge
From: Sebastian Reichel @ 2026-07-20 17:59 UTC (permalink / raw)
To: Chaoyi Chen
Cc: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Luca Ceresoli, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Sandy Huang, Heiko Stübner, Andy Yan, Vinod Koul,
Chaoyi Chen, Nicolas Frattaroli, linux-kernel, dri-devel,
linux-arm-kernel, linux-rockchip, linux-phy
In-Reply-To: <alpeAqKo72pwOEnX@venus>
[-- Attachment #1.1: Type: text/plain, Size: 3102 bytes --]
Hi,
On Fri, Jul 17, 2026 at 07:19:29PM +0200, Sebastian Reichel wrote:
> On Fri, Jul 17, 2026 at 03:23:19PM +0800, Chaoyi Chen wrote:
> > From: Chaoyi Chen <chaoyi.chen@rock-chips.com>
> >
> > The HPD function of Type-C DP is implemented through
> > drm_connector_oob_hotplug_event(). For embedded DP, it is required
> > that the DRM connector fwnode corresponds to the Type-C port fwnode.
> >
> > To describe the relationship between the DP controller and the Type-C
> > port device, we usually using drm_bridge to build a bridge chain.
> >
> > Now several USB-C controller drivers have already implemented the DP
> > HPD bridge function provided by aux-hpd-bridge.c, it will build a DP
> > HPD bridge on USB-C connector port device.
> >
> > But this requires the USB-C controller driver to manually register the
> > HPD bridge. If the driver does not implement this feature, the bridge
> > will not be create.
> >
> > So this patch implements a generic DP HPD bridge based on
> > aux-hpd-bridge.c. It will monitor Type-C bus events, and when a
> > Type-C port device containing the DP svid is registered, it will
> > create an HPD bridge for it without the need for the USB-C controller
> > driver to implement it.
> >
> > Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
> > Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > Reviewed-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> > ---
>
> Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> Tested-by: Sebastian Reichel <sebastian.reichel@collabora.com> # ArmSom Sige5
>
> I gave this a test together with the RK3588/RK3576 USB-C DP AltMode
> patches I'm working on. As the fusb302 does a manual registration
> for the DRM bridge in its probe function, the bridge is registered
> twice:
>
> root@sige5 # cat /sys/kernel/debug/dri/bridges
> ...
> bridge[1]: drm_aux_hpd_bridge_funcs
> refcount: 4
> type: [10] DP
> OF: /soc/i2c@2ac50000/typec-portc@22/connector:usb-c-connector
> ops: [0x4] hpd
> bridge[2]: drm_aux_hpd_bridge_funcs
> refcount: 2
> type: [10] DP
> OF: /soc/i2c@2ac50000/typec-portc@22/connector:usb-c-connector
> ops: [0x4] hpd
> ...
>
> Apparently the USB-C DP AltMode keeps working, so this just wastes
> a few CPU cycles and some memory. So this can land and then we can
> remove the manual code from the driver as a follow-up step. I also
> gave that a try and things keep working. I won't send the fusb302
> patch for now to ensure its not applied before this patch lands.
The above test was done with a kernel having all config options
built-in (i.e. no modules). Using arm64 defconfig one ends up with
CONFIG_DRM_AUX_HPD_TYPEC_BRIDGE=m
But the resulting 'aux-hpd-typec-dp-bridge' module is not loaded
automatically resulting in missing bridge registration. Running
'modprobe aux-hpd-typec-dp-bridge' manually in the booted system
does not work either as the TypeC controller has already been
registered and no new BUS_NOTIFY_ADD_DEVICE is generated.
Greetings,
-- Sebastian
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 112 bytes --]
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v3 phy-next 8/8] phy: lynx-10g: use RCW override procedure for dynamic protocol change
From: Vinod Koul @ 2026-07-20 16:34 UTC (permalink / raw)
To: Vladimir Oltean
Cc: linux-phy, devicetree, linuxppc-dev, linux-arm-kernel,
Ioana Ciornei, Neil Armstrong, Tanjeff Moos,
Christophe Leroy (CS GROUP), Michael Walle, Shawn Guo, Frank Li,
linux-kernel
In-Reply-To: <20260720133642.136324-9-vladimir.oltean@nxp.com>
On 20-07-26, 16:36, Vladimir Oltean wrote:
> Up until this patch, the only protocol change supported was between
> 1000Base-X/SGMII and 2500Base-X. The others require an RCW override
> procedure which was lacking.
>
> Since now the guts driver provides the means of applying this procedure,
> make use of it and remove any comment which mentioned the limitation.
lgtm, is there any dependency. If not I can pick it
--
~Vinod
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v9 03/23] dt-bindings: ufs: mediatek,ufs: Add mt8196 variant
From: Louis-Alexis Eyraud @ 2026-07-20 16:21 UTC (permalink / raw)
To: Rob Herring, Nicolas Frattaroli
Cc: Alim Akhtar, Avri Altman, Bart Van Assche, Krzysztof Kozlowski,
Conor Dooley, Matthias Brugger, AngeloGioacchino Del Regno,
Chunfeng Yun, Vinod Koul, Kishon Vijay Abraham I, Peter Wang,
Stanley Jhu, James E.J. Bottomley, Martin K. Petersen,
Philipp Zabel, Liam Girdwood, Mark Brown, Chaotian Jing,
Neil Armstrong, kernel, linux-scsi, devicetree, linux-kernel,
linux-arm-kernel, linux-mediatek, linux-phy, Conor Dooley
In-Reply-To: <CAL_Jsq+mF_Q7Ld8__ZTc5yDvWAT6uK5wxfBJM-YdFjOvdiDc-w@mail.gmail.com>
Hi Rob,
On Tue, 2026-03-10 at 13:10 -0500, Rob Herring wrote:
> On Fri, Mar 6, 2026 at 12:37 PM Nicolas Frattaroli
> <nicolas.frattaroli@collabora.com> wrote:
> >
> > On Friday, 6 March 2026 17:33:05 Central European Standard Time Rob
> > Herring wrote:
> > > On Fri, Mar 06, 2026 at 02:24:44PM +0100, Nicolas Frattaroli
> > > wrote:
> > > > The MediaTek MT8196 SoC's UFS controller uses three additional
> > > > clocks
> > > > compared to the MT8195, and a different set of supplies. It is
> > > > therefore
> > > > not compatible with the MT8195.
> > > >
> > > > While it does have a AVDD09_UFS_1 pin in addition to the
> > > > AVDD09_UFS pin,
> > > > it appears that these two pins are commoned together, as the
> > > > board
> > > > schematic I have access to uses the same supply for both, and
> > > > the
> > > > downstream driver does not distinguish between the two supplies
> > > > either.
> > > >
> > > > Add a compatible for it, and modify the binding
> > > > correspondingly.
> > > >
> > > > Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> > > > Acked-by: Vinod Koul <vkoul@kernel.org>
> > > > Acked-by: Conor Dooley <conor.dooley@microchip.com>
> > > > Reviewed-by: AngeloGioacchino Del Regno
> > > > <angelogioacchino.delregno@collabora.com>
> > > > Signed-off-by: Nicolas Frattaroli
> > > > <nicolas.frattaroli@collabora.com>
> > > > ---
> > > > .../devicetree/bindings/ufs/mediatek,ufs.yaml | 58
> > > > +++++++++++++++++++++-
> > > > 1 file changed, 57 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git
> > > > a/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml
> > > > b/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml
> > > > index e0aef3e5f56b..a82119ecbfe8 100644
> > > > --- a/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml
> > > > +++ b/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml
> > > > @@ -16,10 +16,11 @@ properties:
> > > > - mediatek,mt8183-ufshci
> > > > - mediatek,mt8192-ufshci
> > > > - mediatek,mt8195-ufshci
> > > > + - mediatek,mt8196-ufshci
> > > >
> > > > clocks:
> > > > minItems: 1
> > > > - maxItems: 13
> > > > + maxItems: 16
> > > >
> > > > clock-names:
> > > > minItems: 1
> > > > @@ -37,6 +38,9 @@ properties:
> > > > - const: crypt_perf
> > > > - const: ufs_rx_symbol0
> > > > - const: ufs_rx_symbol1
> > > > + - const: ufs_sel
> > >
> > > "ufs" is redundant as all the clocks are for UFS. Same comment on
> > > prior
> > > patch.
> >
> > Is this naming a big enough concern to block this series with two
> > explicit acks on this patch that fixes a wholly broken and useless
> > binding?
>
> Shrug... Is changing it really that hard?
>
Since I'm currently working on rebasing this series and fixing its
remaining open issues (compilation, dt-bindings warnings,...) to send a
new revision, I've looked at the questions you raised.
First, renaming those clocks and all the other starting with "ufs_"
prefix (including the one that is simply named ufs) is indeed easy and
needs just a little rework.
Mostly, a driver patch, as it is currently explicitly using the name of
the 3 ufs_sel clocks and devicetree patches to adapt to this change.
The next series revision will include the devicetree patches for MT8195
SoC and the two boards that integrate this SoC and an UFS storage
(Genio 1200 EVK UFS and Radxa NIO-12L), to fix the warnings that the
dt-binding changes, done by patch 1 (additional clocks, freq-table-hz
deprecation, additional power supplies) in the v9 revision, generate.
> > > > + - const: ufs_sel_min_src
> > > > + - const: ufs_sel_max_src
> > >
> > > "src" sounds like a parent clock? If so, probably shouldn't be in
> > > the
> > > clocks list. 'assigned-clocks' is for dealing with parent clocks.
> > >
> >
> > I don't know what it is, and I have no way to consult any
> > documentation
> > that would tell me what it is. I am trying to put out this dumpster
> > fire
> > of a downstream turd that made its way into mainline as the review
> > process
> > has been completely subverted, and is only getting worse with each
> > passing
> > month that MediaTek is allowed to block this series from
> > progressing while
> > sneaking further changes through.
>
> It's good Mediatek is active, then they can tell us what the clocks
> are for. I would think the driver would give some clue.
>
Second, when searching in the driver code and the git history, it shows
that ufs_sel is indeed a parent clock.
The ufs_sel/ufs_sel_min_src/ufs_sel_max_src clock use in the driver
code was introduced by the commit b7dbc686f60b ("scsi: ufs: ufs-
mediatek: Support clk-scaling to optimize power consumption") to
implement a dynamic clock scaling feature.
ufs_sel is supposed to be the parent clock of the main clock ("ufs" in
dt-bindings) and both ufs_sel_min_src/ufs_sel_max_src the parent of
ufs_sel.
The code switches conditionally the ufs_sel parent to modify the ufs
clock rate (ufs_sel_max_src for maximum performance, ufs_sel_min_src
otherwise).
I've also looked at different downstream kernel trees, the assigned
clocks values for those 3 clocks in the ufshci node in devicetree are
consistent with the clock hierarchy in the MT8196 clock controllers
drivers.
This feature also seems to be linked to another one that added the
clock scaling for the FDE clock (ufs_aes in dt-bindings).
The commit that introduced it is 5e5976f5242d ("scsi: ufs: host:
mediatek: Support FDE (AES) clock scaling") and it also added the 3
undocumented clocks: ufs_fde, ufs_fde_min_src, ufs_fde_max_src.
It is similar to the previous described one.
The current driver code seems to require having the "ufs_fde" clock
(supposed to be the ufs_aes parent clock) in the devicetree, so that
the clock scaling feature for the "ufs_sel" clock is performed and I
did not find why.
So, with this current series dt-bindings patches, ufs clock scaling
feature seems not completely described yet.
There is also the crypto boost feature that make use of the
crypt_mux/crypt_lp/crypt_perf clocks in a similar way.
They were missing from dt-bindings, before this series patches made by
Nicolas to documented them. Angelo also sent a patch two years ago in
that regard ([1]) but did not get picked.
The feature has been introduced by commit 590b0d2372fe ("scsi: ufs-
mediatek: Support performance mode for inline encryption engine")
The crypt_mux clock is also supposed to be the ufs_aes parent clock and
its own parent is switched on need between crypt_lp (low power) and
crypt_perf (performance).
This feature is also depending two undocumented property:
- dvfsrc-vcore-supply: Angelo sent [2] to add it and this current
series forgot to add it too (to be done for v10 ?)
- mediatek,ufs-boost-crypt: vendor specific property to enable this
feature. Angelo sent [3] to remove its need but got reject back then
Note that crypto boost and FDE clock scaling features seem not to be
supposed to be enabled at the same time.
Again, this crypto boost feature seems not completely described yet.
Sorry for the wall of text but I felt it was better to add extra
details regarding all those features and how they relate to each other
to have a more complete answer regarding ufs_sel.
[1]
https://lore.kernel.org/linux-mediatek/20240612074309.50278-8-angelogioacchino.delregno@collabora.com/
[2]
https://lore.kernel.org/linux-mediatek/20240612074309.50278-9-angelogioacchino.delregno@collabora.com/
[3]
https://lore.kernel.org/linux-mediatek/20240612074309.50278-4-angelogioacchino.delregno@collabora.com/
> I don't see how accepting sub-par bindings or not fixes the issues
> here.
So, since all these features rely heavily on optional parent clocks,
that were not documented before being used in driver code, what should
be done to make progress and fix these dt-bindings?
What would you recommend to do in order to resolve those topics?
I'm open for ideas, please.
Best regards,
Louis-Alexis
>
> Rob
--
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: zynqmp: fix runtime PM leak on probe allocation failure
From: sashiko-bot @ 2026-07-20 15:57 UTC (permalink / raw)
To: Radhey Shyam Pandey; +Cc: olteanv, vkoul, neil.armstrong, linux-phy
In-Reply-To: <20260720153832.1130006-3-radhey.shyam.pandey@amd.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Premature registration of the PHY provider exposes consumers to a Use-After-Free (UAF) if a subsequent probe initialization step fails.
--
--- Patch [2]: [PATCH 2/2] phy: zynqmp: fix runtime PM leak on probe allocation failure ---
commit e07d2a03d56ecad62f1f38421ad1054872a5ffb6
Author: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
phy: zynqmp: fix runtime PM leak on probe allocation failure
This commit moves the devm_kmalloc() allocation for saved_regs before calling
pm_runtime_resume_and_get(). This ensures that an allocation failure does not
leave an unreleased runtime PM usage counter.
> diff --git a/drivers/phy/xilinx/phy-zynqmp.c b/drivers/phy/xilinx/phy-zynqmp.c
> index c8230f2bda629..2138f5399821a 100644
> --- a/drivers/phy/xilinx/phy-zynqmp.c
> +++ b/drivers/phy/xilinx/phy-zynqmp.c
> @@ -1044,6 +1044,12 @@ static int xpsgtr_probe(struct platform_device *pdev)
> return PTR_ERR(provider);
> }
>
> + gtr_dev->saved_regs = devm_kmalloc(gtr_dev->dev,
> + sizeof(save_reg_address),
> + GFP_KERNEL);
> + if (!gtr_dev->saved_regs)
> + return -ENOMEM;
> +
[Severity: High]
This is a pre-existing issue, but does failing the probe here expose a
use-after-free race condition?
Just above this block in xpsgtr_probe(), the driver registers the PHY provider
using devm_of_phy_provider_register(). If this new devm_kmalloc() call (or the
subsequent pm_runtime_resume_and_get() call) fails, the probe function will
return an error. Devres will then automatically free the driver data, including
gtr_dev.
If a consumer device is probing concurrently and obtains a reference to the PHY
before this error path executes, could that consumer later attempt to use the
freed gtr_dev memory when calling phy_init()? Would it be safer to move the
devm_of_phy_provider_register() call to the very end of xpsgtr_probe() after
all initialization and allocations have succeeded?
> pm_runtime_set_active(gtr_dev->dev);
> pm_runtime_enable(gtr_dev->dev);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260720153832.1130006-1-radhey.shyam.pandey@amd.com?part=2
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* [PATCH 2/2] phy: zynqmp: fix runtime PM leak on probe allocation failure
From: Radhey Shyam Pandey @ 2026-07-20 15:38 UTC (permalink / raw)
To: tomi.valkeinen, vkoul, neil.armstrong, michal.simek
Cc: linux-kernel, linux-phy, linux-arm-kernel, git,
Radhey Shyam Pandey
In-Reply-To: <20260720153832.1130006-1-radhey.shyam.pandey@amd.com>
Allocate saved_regs before pm_runtime_resume_and_get() so a
devm_kmalloc() failure does not leave an unreleased runtime PM usage
counter.
Fixes: 5af9b304bc60 ("phy: xilinx: phy-zynqmp: Fix SGMII linkup failure on resume")
Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
---
drivers/phy/xilinx/phy-zynqmp.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/phy/xilinx/phy-zynqmp.c b/drivers/phy/xilinx/phy-zynqmp.c
index c8230f2bda62..2138f5399821 100644
--- a/drivers/phy/xilinx/phy-zynqmp.c
+++ b/drivers/phy/xilinx/phy-zynqmp.c
@@ -1044,6 +1044,12 @@ static int xpsgtr_probe(struct platform_device *pdev)
return PTR_ERR(provider);
}
+ gtr_dev->saved_regs = devm_kmalloc(gtr_dev->dev,
+ sizeof(save_reg_address),
+ GFP_KERNEL);
+ if (!gtr_dev->saved_regs)
+ return -ENOMEM;
+
pm_runtime_set_active(gtr_dev->dev);
pm_runtime_enable(gtr_dev->dev);
@@ -1053,12 +1059,6 @@ static int xpsgtr_probe(struct platform_device *pdev)
return ret;
}
- gtr_dev->saved_regs = devm_kmalloc(gtr_dev->dev,
- sizeof(save_reg_address),
- GFP_KERNEL);
- if (!gtr_dev->saved_regs)
- return -ENOMEM;
-
return 0;
}
--
2.43.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* [PATCH 1/2] phy: zynqmp: fix clock error handling in xpsgtr_phy_init()
From: Radhey Shyam Pandey @ 2026-07-20 15:38 UTC (permalink / raw)
To: tomi.valkeinen, vkoul, neil.armstrong, michal.simek
Cc: linux-kernel, linux-phy, linux-arm-kernel, git,
Radhey Shyam Pandey
In-Reply-To: <20260720153832.1130006-1-radhey.shyam.pandey@amd.com>
Propagate clk_prepare_enable() failures to the caller instead of
returning success, and disable the reference clock on initialization
error paths to avoid leaking clock references when phy_exit() is not
called.
Fixes: 25d700833513 ("phy: xilinx: phy-zynqmp: dynamic clock support for power-save")
Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
---
drivers/phy/xilinx/phy-zynqmp.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/phy/xilinx/phy-zynqmp.c b/drivers/phy/xilinx/phy-zynqmp.c
index fe6b4925d166..c8230f2bda62 100644
--- a/drivers/phy/xilinx/phy-zynqmp.c
+++ b/drivers/phy/xilinx/phy-zynqmp.c
@@ -658,12 +658,13 @@ static int xpsgtr_phy_init(struct phy *phy)
{
struct xpsgtr_phy *gtr_phy = phy_get_drvdata(phy);
struct xpsgtr_dev *gtr_dev = gtr_phy->dev;
- int ret = 0;
+ int ret;
mutex_lock(>r_dev->gtr_mutex);
/* Configure and enable the clock when peripheral phy_init call */
- if (clk_prepare_enable(gtr_dev->clk[gtr_phy->refclk]))
+ ret = clk_prepare_enable(gtr_dev->clk[gtr_phy->refclk]);
+ if (ret)
goto out;
/* Skip initialization if not required. */
@@ -673,7 +674,7 @@ static int xpsgtr_phy_init(struct phy *phy)
if (gtr_dev->tx_term_fix) {
ret = xpsgtr_phy_tx_term_fix(gtr_phy);
if (ret < 0)
- goto out;
+ goto out_disable_clk;
gtr_dev->tx_term_fix = false;
}
@@ -687,7 +688,7 @@ static int xpsgtr_phy_init(struct phy *phy)
*/
ret = xpsgtr_configure_pll(gtr_phy);
if (ret)
- goto out;
+ goto out_disable_clk;
xpsgtr_lane_set_protocol(gtr_phy);
@@ -705,6 +706,10 @@ static int xpsgtr_phy_init(struct phy *phy)
break;
}
+ goto out;
+
+out_disable_clk:
+ clk_disable_unprepare(gtr_dev->clk[gtr_phy->refclk]);
out:
mutex_unlock(>r_dev->gtr_mutex);
return ret;
--
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 0/2] phy: zynqmp: fix clock and runtime PM error paths in xpsgtr driver
From: Radhey Shyam Pandey @ 2026-07-20 15:38 UTC (permalink / raw)
To: tomi.valkeinen, vkoul, neil.armstrong, michal.simek
Cc: linux-kernel, linux-phy, linux-arm-kernel, git,
Radhey Shyam Pandey
This series fixes two pre-existing resource handling bugs in the Zynq
UltraScale+ MPSoC Gigabit Transceiver (XPSGTR) PHY driver. They were
reported during review of the SERDES scrambler series [1] and were
intentionally deferred to this follow-up series.
Patch 1:
Propagate clk_prepare_enable() failures from xpsgtr_phy_init() and
disable the reference clock on initialization error paths.
Patch 2:
Move saved_regs allocation before runtime PM resume to avoid
leaking the runtime PM usage count on probe failures.
[1]: https://lore.kernel.org/all/20260627155229.2791113-1-radhey.shyam.pandey@amd.com
Radhey Shyam Pandey (2):
phy: zynqmp: fix clock error handling in xpsgtr_phy_init()
phy: zynqmp: fix runtime PM leak on probe allocation failure
drivers/phy/xilinx/phy-zynqmp.c | 25 +++++++++++++++----------
1 file changed, 15 insertions(+), 10 deletions(-)
base-commit: 0718283ab28bc3907e10b61a6b4be6fefa1cbb2f
--
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 v3 2/2] phy: mediatek: Add support for PCI-Express Gen4 S-PHY
From: sashiko-bot @ 2026-07-20 15:37 UTC (permalink / raw)
To: AngeloGioacchino Del Regno
Cc: conor+dt, neil.armstrong, olteanv, linux-phy, vkoul, devicetree,
robh
In-Reply-To: <20260720152654.92216-3-angelogioacchino.delregno@collabora.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The driver emits a false-positive kernel error log when NVMEM is disabled or unsupported.
--
--- Patch [2]: [PATCH v3 2/2] phy: mediatek: Add support for PCI-Express Gen4 S-PHY ---
commit 93ce035a5e3a24b4c6f63697869bbcb5d8e50db4
Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Subject: phy: mediatek: Add support for PCI-Express Gen4 S-PHY
This commit adds support for the PCI-Express Gen4 S-PHY found in the latest
MediaTek SoCs, including MT8196, MT8894 and similar.
[ ... ]
> diff --git a/drivers/phy/mediatek/phy-mtk-pcie-sphy.c b/drivers/phy/mediatek/phy-mtk-pcie-sphy.c
> new file mode 100644
> index 0000000000000..853aafa182597
> --- /dev/null
> +++ b/drivers/phy/mediatek/phy-mtk-pcie-sphy.c
[ ... ]
> +static int mtk_pcie_sphy_get_one_cal_para(struct device *dev, const char *name, u8 max_val)
> +{
> + u16 buf;
> + u8 tmp;
> + int ret;
> +
> + /*
> + * All of the calibrations are always max 8 bits long, but some may
> + * be split between two different 8-bits cells: handle this corner
> + * case by retrying reading as u16.
> + */
> + ret = nvmem_cell_read_u8(dev, name, &tmp);
> + if (ret == 0)
> + buf = tmp;
> + else
> + ret = nvmem_cell_read_u16(dev, name, &buf);
> +
> + if (ret == -ENOENT) {
> + dev_info(dev, "No calibration for %s. Using defaults\n", name);
> + return -ENOENT;
> + } else if (ret)
> + return dev_err_probe(dev, ret,
> + "Cannot get calibration data for %s\n", name);
[Severity: Low]
Does this emit a false-positive kernel error log when NVMEM is disabled or
unsupported?
If CONFIG_NVMEM=n, the NVMEM APIs return -EOPNOTSUPP, which will cause this
path to print an error with dev_err_probe(). However, the caller in
mtk_pcie_sphy_get_calibration_data() gracefully handles -EOPNOTSUPP as a
non-error condition:
if (ret < 0) {
/*
* If any of the calibration values is missing, or if there is
* no calibration at all in the eFuses, this is not a problem,
* as the PHY doesn't require one to actually work.
*/
if (ret == -ENOENT || ret == -EOPNOTSUPP) {
cal->supported = false;
return 0;
}
Could -EOPNOTSUPP be handled silently here in mtk_pcie_sphy_get_one_cal_para()
to avoid spurious error messages during boot?
> +
> + if (buf > max_val)
> + return dev_err_probe(dev, -ERANGE,
> + "Bad value %u retrieved for %s.\n", buf, name);
> +
> + return buf;
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260720152654.92216-1-angelogioacchino.delregno@collabora.com?part=2
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* [PATCH v3 1/2] dt-bindings: phy: Document MT8196 MediaTek PCI-Express Gen4 S-PHY
From: AngeloGioacchino Del Regno @ 2026-07-20 15:26 UTC (permalink / raw)
To: chunfeng.yun
Cc: vkoul, neil.armstrong, robh, krzk+dt, conor+dt, matthias.bgg,
angelogioacchino.delregno, linux-arm-kernel, linux-mediatek,
linux-phy, devicetree, linux-kernel, kernel
In-Reply-To: <20260720152654.92216-1-angelogioacchino.delregno@collabora.com>
This adds bindings for the PCI-Express Gen4 S-PHY found in newer
MediaTek SoCs, such as MT8196 and its variants.
In the current "revision 3", depending on the specific port, this
S-PHY supports up to two lanes of PCI-Express Gen 4 and both EP
and RC modes.
It is not clear whether revisions/versions earlier than 3 have
ever been shipped in any other MediaTek SoC.
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
.../phy/mediatek,mt8196-pcie-sphy.yaml | 93 +++++++++++++++++++
1 file changed, 93 insertions(+)
create mode 100644 Documentation/devicetree/bindings/phy/mediatek,mt8196-pcie-sphy.yaml
diff --git a/Documentation/devicetree/bindings/phy/mediatek,mt8196-pcie-sphy.yaml b/Documentation/devicetree/bindings/phy/mediatek,mt8196-pcie-sphy.yaml
new file mode 100644
index 000000000000..3fd1bfbc3822
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/mediatek,mt8196-pcie-sphy.yaml
@@ -0,0 +1,93 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/mediatek,mt8196-pcie-sphy.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MediaTek PCI-Express Gen4 S-PHY
+
+maintainers:
+ - AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
+
+description:
+ The MediaTek PCI-Express Gen4 S-PHY is capable of data serialization
+ and de-serialization with 8, 10, 128 or 130 bits (16GT/s) encoding
+ and decoding, has analog buffers, elastic buffers and mechanisms for
+ receiver detection.
+ This PHY supports link rates of 2.5, 5.0, 8.0 and 16.0 GT/s per-lane
+ in both Root Complex (RC) and End Point (EP) modes, depending on the
+ chosen port.
+ The Digital PHY (PHYD) part adheres to the Intel PIPE (PHY Interface
+ for the PCIe) specification.
+
+properties:
+ compatible:
+ const: mediatek,mt8196-pcie-sphy
+
+ reg:
+ items:
+ - description: System Interface (SIF) registers
+ - description: Clock Management (CKM) registers
+
+ reg-names:
+ items:
+ - const: sif
+ - const: ckm
+
+ "#phy-cells":
+ const: 0
+
+ nvmem-cells:
+ items:
+ - description: Internal resistor selection of TX Bias Current
+ - description: XTP Bias V2V voltage calibration
+ - description: S-PLL CKTX Impedance Selection (P-MOSFET side)
+ - description: S-PLL CKTX Impedance Selection (N-MOSFET side)
+ - description: S-PLL CKTX Intermediate Transition Impedance
+ - description: RX Front-End Return Loss Continuous Time Linear EQ
+ - description: RX Front-End Return Loss Variable Gain Amplifier
+ - description: RX Front-End Return Loss Decision Feedback EQ
+ - description: RX Impedance Selection
+ - description: TX Impedance Selection (P-MOSFET side)
+ - description: TX Impedance Selection (N-MOSFET side)
+ - description: TX Lane 0 Switch Resistance (RSWn)
+ - description: TX Lane 1 Switch Resistance (RSWn)
+ - description: eFuse calibration table version
+
+ nvmem-cell-names:
+ items:
+ - const: int-r
+ - const: xtp-vtrim
+ - const: cktx-pmos
+ - const: cktx-nmos
+ - const: cktx-r-mid
+ - const: rxfe-lanes-rl-ctle
+ - const: rxfe-lanes-rl-vga
+ - const: rxfe-lanes-rl-dfe
+ - const: rx-lanes-imp
+ - const: tx-lanes-pmos
+ - const: tx-lanes-nmos
+ - const: tx-ln0-rswn
+ - const: tx-ln1-rswn
+ - const: cal-version
+
+ power-domains:
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+ - reg-names
+ - "#phy-cells"
+
+additionalProperties: false
+
+examples:
+ - |
+ phy@16900000 {
+ compatible = "mediatek,mt8196-pcie-sphy";
+ reg = <0x16900000 0x10000>, <0x16920000 0x10000>;
+ reg-names = "sif", "ckm";
+ power-domains = <&spm 10>;
+ #phy-cells = <0>;
+ };
--
2.55.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* [PATCH v3 0/2] PHY: Add MediaTek PCI-Express Gen4 S-PHY Driver
From: AngeloGioacchino Del Regno @ 2026-07-20 15:26 UTC (permalink / raw)
To: chunfeng.yun
Cc: vkoul, neil.armstrong, robh, krzk+dt, conor+dt, matthias.bgg,
angelogioacchino.delregno, linux-arm-kernel, linux-mediatek,
linux-phy, devicetree, linux-kernel, kernel
Changes in v3:
- Allowed value 0 in calibration fields (as it's a legal value)
- Fixed duplicated LN_TX_IMPSEL_{P,N}MOS writes
- Allowed -ENOENT for nvmem_cell_read_u{8,16}() calls, as the
calibration data is effectively optional
- Removed two development stage leftover unused definitions
- Moved num_lanes check in mtk_pcie_sphy_get_calibration_data()
to reduce indent in probe function.
Changes in v2:
- Added items/description to reg in binding
- Added missing pm_runtime.h inclusion
- Moved pm_runtime_enable call to before registering PHY
- Fixed pmos/nmos variable size for 5 bits calibration values
- Fixed calibration for single-lane PCIe, as in, the ln1 rswn
calibration nvmem value is ignored in code if it's single
lane and will not return an error; this is due to the fact
that, effectively, single-lane may have a zero calibration
in tx-ln1-rswn which is fine, as that'd be anyway unused
- Changed calibration data errors to dev_err_probe and changed
the "no calibration for ..." message to dev_info instead
This adds a driver for the PCI-Express Gen4 "S-PHY" found in the
Genio MT8894, Kompanio MT8196, Dimensity MT6991 SoCs (which are
all variants of the same chip).
This was successfully tested on MT8894 and MT8196.
AngeloGioacchino Del Regno (2):
dt-bindings: phy: Document MT8196 MediaTek PCI-Express Gen4 S-PHY
phy: mediatek: Add support for PCI-Express Gen4 S-PHY
.../phy/mediatek,mt8196-pcie-sphy.yaml | 93 +++
drivers/phy/mediatek/Kconfig | 9 +
drivers/phy/mediatek/Makefile | 1 +
drivers/phy/mediatek/phy-mtk-pcie-sphy.c | 556 ++++++++++++++++++
4 files changed, 659 insertions(+)
create mode 100644 Documentation/devicetree/bindings/phy/mediatek,mt8196-pcie-sphy.yaml
create mode 100644 drivers/phy/mediatek/phy-mtk-pcie-sphy.c
--
2.55.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* [PATCH v3 2/2] phy: mediatek: Add support for PCI-Express Gen4 S-PHY
From: AngeloGioacchino Del Regno @ 2026-07-20 15:26 UTC (permalink / raw)
To: chunfeng.yun
Cc: vkoul, neil.armstrong, robh, krzk+dt, conor+dt, matthias.bgg,
angelogioacchino.delregno, linux-arm-kernel, linux-mediatek,
linux-phy, devicetree, linux-kernel, kernel
In-Reply-To: <20260720152654.92216-1-angelogioacchino.delregno@collabora.com>
Add support for the PCI-Express Gen4 S-PHY found in the latest
MediaTek SoCs, including MT8196, MT8894 and similar.
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
drivers/phy/mediatek/Kconfig | 9 +
drivers/phy/mediatek/Makefile | 1 +
drivers/phy/mediatek/phy-mtk-pcie-sphy.c | 556 +++++++++++++++++++++++
3 files changed, 566 insertions(+)
create mode 100644 drivers/phy/mediatek/phy-mtk-pcie-sphy.c
diff --git a/drivers/phy/mediatek/Kconfig b/drivers/phy/mediatek/Kconfig
index ba6461350951..77236f3084c8 100644
--- a/drivers/phy/mediatek/Kconfig
+++ b/drivers/phy/mediatek/Kconfig
@@ -13,6 +13,15 @@ config PHY_MTK_PCIE
callback for PCIe GEN3 port, it supports software efuse
initialization.
+config PHY_MTK_PCIE_SPHY
+ tristate "MediaTek PCIe S-PHY Driver"
+ depends on ARCH_MEDIATEK || COMPILE_TEST
+ depends on OF
+ select GENERIC_PHY
+ help
+ Say 'Y' here to add support for MediaTek PCIe S-PHY driver for
+ PCI-Express Gen4 controllers as found in MT6991, MT8196 and others.
+
config PHY_MTK_XFI_TPHY
tristate "MediaTek 10GE SerDes XFI T-PHY driver"
depends on ARCH_MEDIATEK || COMPILE_TEST
diff --git a/drivers/phy/mediatek/Makefile b/drivers/phy/mediatek/Makefile
index ed0da708759b..7e984c7cfea3 100644
--- a/drivers/phy/mediatek/Makefile
+++ b/drivers/phy/mediatek/Makefile
@@ -5,6 +5,7 @@
obj-$(CONFIG_PHY_MTK_DP) += phy-mtk-dp.o
obj-$(CONFIG_PHY_MTK_PCIE) += phy-mtk-pcie.o
+obj-$(CONFIG_PHY_MTK_PCIE_SPHY) += phy-mtk-pcie-sphy.o
obj-$(CONFIG_PHY_MTK_TPHY) += phy-mtk-tphy.o
obj-$(CONFIG_PHY_MTK_UFS) += phy-mtk-ufs.o
obj-$(CONFIG_PHY_MTK_XSPHY) += phy-mtk-xsphy.o
diff --git a/drivers/phy/mediatek/phy-mtk-pcie-sphy.c b/drivers/phy/mediatek/phy-mtk-pcie-sphy.c
new file mode 100644
index 000000000000..853aafa18259
--- /dev/null
+++ b/drivers/phy/mediatek/phy-mtk-pcie-sphy.c
@@ -0,0 +1,556 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2022 MediaTek Inc.
+ * Copyright (c) 2026 Collabora Ltd.
+ * AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
+ */
+
+#include <linux/bitfield.h>
+#include <linux/module.h>
+#include <linux/nvmem-consumer.h>
+#include <linux/of.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/slab.h>
+
+#include "phy-mtk-io.h"
+
+/* PHY System Interface (SIF) registers */
+#define PEXTP_DIG_GLB_TOP 0x20
+# define RG_XTP_BYPASS_PIPE_RST_RC BIT(17)
+#define PEXTP_DIG_GLB_CKBG0 0x30
+# define RG_XTP_CKBG_XTAL_STABLE_TIME_SEL GENMASK(25, 16)
+#define PEXTP_DIG_GLB_TPLL_CTL0 0x38
+# define RG_XTP_TPLL_SET_STABLE_TIME_SEL GENMASK(7, 2)
+# define RG_XTP_TPLL_PWE_ON_STABLE_TIME_SEL GENMASK(9, 8)
+#define PEXTP_DIG_GLB_CLKREQ_CTL 0x50
+# define RG_XTP_CKM_EN_L1S0 BIT(13)
+# define RG_XTP_CKM_EN_L1S1 BIT(14)
+#define PEXTP_DIG_GLB_TPLL_CTL2 0xf4
+# define RG_XTP_TPLL_ISO_EN_STABLE_TIME_SEL GENMASK(13, 12)
+
+/* PHY System Interface Digital registers */
+#define PEXTP_DIG_LN_TRX_PIPE_IF_17 0x30e8
+# define RG_XTP_LN_RX_LF_CTLE_CSEL_GEN4 GENMASK(14, 12)
+#define PEXTP_DIG_LN_RX_F0 0x50f0
+# define RG_XTP_LN_RX_GEN1_CTLE1_CSEL GENMASK(3, 0)
+# define RG_XTP_LN_RX_GEN2_CTLE1_CSEL GENMASK(7, 4)
+# define RG_XTP_LN_RX_GEN3_CTLE1_CSEL GENMASK(11, 8)
+# define RG_XTP_LN_RX_GEN4_CTLE1_CSEL GENMASK(15, 12)
+#define PEXTP_DIG_LN_RX2_AEQ_EDGE_0 0x6004
+# define RG_XTP_LN_RX_AEQ_EGEQ_RATIO_GEN3 GENMASK(21, 16)
+# define RG_XTP_LN_RX_AEQ_EGEQ_RATIO_GEN4 GENMASK(29, 24)
+
+/* PHY System Interface Analog registers */
+#define PEXTP_ANA_GLB_TPLL1_RSVD 0x902c
+# define RG_XTP_GLB_TPLL1_P_PATH_GAIN GENMASK(2, 0)
+#define PEXTP_ANA_GLB_BIAS_0 0x9060
+# define RG_XTP_GLB_BIAS_INTR_CTRL GENMASK(5, 0)
+#define PEXTP_ANA_GLB_BIAS_1 0x90c0
+# define RG_XTP_GLB_BIAS_V2V_VTRIM GENMASK(9, 6)
+#define PEXTP_ANA_LN_TRX_0C 0xa00c
+# define RG_XTP_LN_TX_RSWN_IMPSEL GENMASK(20, 16)
+#define PEXTP_ANA_LN_TRX_34 0xa034
+# define RG_XTP_LN_RX_FE BIT(15)
+#define PEXTP_ANA_LN_TRX_6C 0xa06c
+# define RG_XTP_LN_RX_AEQ_CTLE_ERR_TYPE GENMASK(14, 13)
+# define AEQ_CTLE_SEARCH_ERR_TYPE_H1P5 0
+# define AEQ_CTLE_SEARCH_ERR_TYPE_H1P5_H2P5 1
+# define AEQ_CTLE_SEARCH_ERR_TYPE_P1P5_H2P5_H3P5 2
+#define PEXTP_ANA_LN_TRX_A0 0xa0a0
+# define RG_XTP_LN_TX_IMPSEL_PMOS GENMASK(4, 0)
+# define RG_XTP_LN_TX_IMPSEL_NMOS GENMASK(11, 7)
+# define RG_XTP_LN_RX_IMPSEL GENMASK(15, 12)
+#define PEXTP_ANA_LN_TRX_A8 0xa0a8
+# define RG_XTP_LN_RX_LEQ_RL_CTLE_CAL GENMASK(6, 2)
+# define RG_XTP_LN_RX_LEQ_RL_VGA_CAL GENMASK(11, 7)
+# define RG_XTP_LN_RX_LEQ_RL_DFE_CAL GENMASK(23, 19)
+#define PEXTP_DIG_LN_TX_LC_TABLE_RSWN_4 0xb004
+#define PEXTP_DIG_LN_TX_LC_TABLE_RSWN_8 0xb008
+#define PEXTP_DIG_LN_TX_LC_TABLE_RSWN_C 0xb00c
+#define PEXTP_DIG_LN_TX_LC_TABLE_RSWN_10 0xb010
+#define PEXTP_DIG_LN_TX_LC_TABLE_RSWN_14 0xb014
+#define PEXTP_DIG_LN_TX_LC_TABLE_RSWN_18 0xb018
+# define RG_XTP_LN_TX_LC_PRESET_MGx_Px_CM1 GENMASK(5, 0)
+# define RG_XTP_LN_TX_LC_PRESET_MGx_Px_C0 GENMASK(13, 8)
+# define RG_XTP_LN_TX_LC_PRESET_MGx_Px_CP1 GENMASK(21, 16)
+#define PEXTP_REG_LANE(x) ((x) * 0x100)
+
+/* PHY Clock Management (CKM) registers */
+#define XTP_CKM_FORCE_6 0x38
+# define RG_CKM_BIAS_WAIT_PRD_US GENMASK(21, 16)
+#define XTP_CKM_REG_SPLL_FBKDIV_5 0xd4
+# define RG_CKM_CKTX_IMPSEL_PMOS GENMASK(19, 16)
+# define RG_CKM_CKTX_IMPSEL_NMOS GENMASK(23, 20)
+# define RG_CKM_CKTX_IMPSEL_SW GENMASK(27, 24)
+
+/* Calibration data from eFuses */
+#define MTK_PCIE_SPHY_CALIBRATION_MAX_DATA_LANES 2
+#define MTK_PCIE_SPHY_CALIBRATION_LAST_QUIRK_VER 4
+
+/**
+ * struct mtk_pcie_sphy_imp_sel - Impedance Selection parameters
+ * @pmos: Impedance selection for P-Channel MOSFET
+ * @nmos: Impedance selection for N-Channel MOSFET
+ */
+struct mtk_pcie_sphy_imp_sel {
+ u8 pmos;
+ u8 nmos;
+};
+
+/**
+ * struct mtk_pcie_sphy_efuse - eFuse calibration data for S-PHY
+ * @int_r_ctrl: Internal resistor selection of TX Bias Current
+ * @xtp_vtrim: XTP Bias V2V voltage calibration
+ * @cktx_impsel: SPLL CKTX Impedance Selection (P and N MOSFET)
+ * @cktx_r_mid: SPLL CKTX Intermediate Transition Impedance (Rmid)
+ * @rx_leq_rl_ctle: RX Front-End Return Loss Continuous Time Linear Equalization value
+ * @rx_leq_rl_vga: RX Front-End Return Loss Variable Gain Amplifier value
+ * @rx_leq_rl_dfe: RX Front-End Return Loss Decision Feedback Equalization value
+ * @rx_impsel: RX Impedance Selection
+ * @tx_impsel: TX Impedance Selection (P and N MOSFET)
+ * @tx_rswn_impsel: TX RSWn (Switch Resistance) impedance selection
+ * @supported: eFuse calibration data is supported
+ */
+struct mtk_pcie_sphy_efuse {
+ u8 int_r_ctrl;
+ u8 xtp_vtrim;
+ struct mtk_pcie_sphy_imp_sel cktx_impsel;
+ u8 cktx_r_mid;
+ u8 rx_leq_rl_ctle;
+ u8 rx_leq_rl_vga;
+ u8 rx_leq_rl_dfe;
+ u8 rx_impsel;
+ struct mtk_pcie_sphy_imp_sel tx_impsel;
+ u8 tx_rswn_impsel[MTK_PCIE_SPHY_CALIBRATION_MAX_DATA_LANES];
+ bool supported;
+};
+
+/**
+ * struct mtk_pcie_sphy - PCI-Express S-PHY driver main structure
+ * @dev: Pointer to device structure
+ * @phy: Pointer to generic phy structure
+ * @sif_base: IO mapped register base address of system interface
+ * @ckm_base: IO mapped register base address of clock management interface
+ * @num_lanes: Number of lanes
+ * @calibration: eFuse calibration data for S-PHY
+ */
+struct mtk_pcie_sphy {
+ struct device *dev;
+ struct phy *phy;
+ void __iomem *sif_base;
+ void __iomem *ckm_base;
+ u8 num_lanes;
+ struct mtk_pcie_sphy_efuse calibration;
+};
+
+static void mtk_pcie_sphy_apply_calibration(struct mtk_pcie_sphy *pcie_sphy)
+{
+ struct mtk_pcie_sphy_efuse *cal = &pcie_sphy->calibration;
+ int i;
+
+ mtk_phy_update_field(pcie_sphy->sif_base + PEXTP_ANA_GLB_BIAS_0,
+ RG_XTP_GLB_BIAS_INTR_CTRL, cal->int_r_ctrl);
+
+ mtk_phy_update_field(pcie_sphy->sif_base + PEXTP_ANA_GLB_BIAS_1,
+ RG_XTP_GLB_BIAS_V2V_VTRIM, cal->xtp_vtrim);
+
+ mtk_phy_update_field(pcie_sphy->ckm_base + XTP_CKM_REG_SPLL_FBKDIV_5,
+ RG_CKM_CKTX_IMPSEL_PMOS, cal->cktx_impsel.pmos);
+
+ mtk_phy_update_field(pcie_sphy->ckm_base + XTP_CKM_REG_SPLL_FBKDIV_5,
+ RG_CKM_CKTX_IMPSEL_NMOS, cal->cktx_impsel.nmos);
+
+ mtk_phy_update_field(pcie_sphy->ckm_base + XTP_CKM_REG_SPLL_FBKDIV_5,
+ RG_CKM_CKTX_IMPSEL_SW, cal->cktx_r_mid);
+
+ for (i = 0; i < pcie_sphy->num_lanes; i++) {
+ void __iomem *sif_lane_base = pcie_sphy->sif_base + PEXTP_REG_LANE(i);
+
+ mtk_phy_update_field(sif_lane_base + PEXTP_ANA_LN_TRX_0C,
+ RG_XTP_LN_TX_RSWN_IMPSEL, cal->tx_rswn_impsel[i]);
+
+ mtk_phy_update_field(sif_lane_base + PEXTP_ANA_LN_TRX_A8,
+ RG_XTP_LN_RX_LEQ_RL_CTLE_CAL, cal->rx_leq_rl_ctle);
+
+ mtk_phy_update_field(sif_lane_base + PEXTP_ANA_LN_TRX_A8,
+ RG_XTP_LN_RX_LEQ_RL_VGA_CAL, cal->rx_leq_rl_vga);
+
+ mtk_phy_update_field(sif_lane_base + PEXTP_ANA_LN_TRX_A8,
+ RG_XTP_LN_RX_LEQ_RL_DFE_CAL, cal->rx_leq_rl_dfe);
+
+ mtk_phy_update_field(sif_lane_base + PEXTP_ANA_LN_TRX_A0,
+ RG_XTP_LN_RX_IMPSEL, cal->rx_impsel);
+ }
+}
+
+/**
+ * mtk_pcie_sphy_init() - Initialize the PCI-Express S-PHY
+ * @phy: the phy to be initialized
+ *
+ * The hardware settings will be reset during suspend, it should be
+ * reinitialized when the consumer calls phy_init() again on resume.
+ */
+static int mtk_pcie_sphy_init(struct phy *phy)
+{
+ struct mtk_pcie_sphy *pcie_sphy = phy_get_drvdata(phy);
+ struct mtk_pcie_sphy_imp_sel tx_impsel;
+ int i;
+
+ /* Set CKM Bias wait time to 4 microseconds */
+ mtk_phy_update_field(pcie_sphy->ckm_base + XTP_CKM_FORCE_6,
+ RG_CKM_BIAS_WAIT_PRD_US, 4);
+
+ /* TPLL needs 63 ref_ck ticks to stabilize when setting frequency */
+ mtk_phy_update_field(pcie_sphy->sif_base + PEXTP_DIG_GLB_TPLL_CTL0,
+ RG_XTP_TPLL_SET_STABLE_TIME_SEL, 63);
+
+ /* TPLL needs 3 ref_ck ticks to stabilize when powering on... */
+ mtk_phy_update_field(pcie_sphy->sif_base + PEXTP_DIG_GLB_TPLL_CTL0,
+ RG_XTP_TPLL_PWE_ON_STABLE_TIME_SEL, 3);
+
+ /* ...and the same goes for setting isolation */
+ mtk_phy_update_field(pcie_sphy->sif_base + PEXTP_DIG_GLB_TPLL_CTL2,
+ RG_XTP_TPLL_ISO_EN_STABLE_TIME_SEL, 3);
+
+ /* XTAL doesn't need any stabilization time */
+ mtk_phy_update_field(pcie_sphy->sif_base + PEXTP_DIG_GLB_CKBG0,
+ RG_XTP_CKBG_XTAL_STABLE_TIME_SEL, 0);
+
+ /* Keep pextp_ckm enabled when in L1SS_L1S1 state */
+ mtk_phy_clear_bits(pcie_sphy->sif_base + PEXTP_DIG_GLB_CLKREQ_CTL, RG_XTP_CKM_EN_L1S1);
+
+ /* Set PIPE to reset TPLL */
+ mtk_phy_clear_bits(pcie_sphy->sif_base + PEXTP_DIG_GLB_TOP, RG_XTP_BYPASS_PIPE_RST_RC);
+
+ /* Set TPLL P-Path gain compensation to 1 */
+ mtk_phy_update_field(pcie_sphy->sif_base + PEXTP_ANA_GLB_TPLL1_RSVD,
+ RG_XTP_GLB_TPLL1_P_PATH_GAIN, 1);
+
+ for (i = 0; i < pcie_sphy->num_lanes; i++) {
+ void __iomem *sif_lane_base = pcie_sphy->sif_base + PEXTP_REG_LANE(i);
+
+ /* Set RX Lane AEQ CTRL-E Search Error type to h1.5 + h2.5 */
+ mtk_phy_update_field(sif_lane_base + PEXTP_ANA_LN_TRX_6C,
+ RG_XTP_LN_RX_AEQ_CTLE_ERR_TYPE,
+ AEQ_CTLE_SEARCH_ERR_TYPE_H1P5_H2P5);
+
+ mtk_phy_set_bits(sif_lane_base + PEXTP_ANA_LN_TRX_34, RG_XTP_LN_RX_FE);
+
+ /* TRX: Select CTLE1 for RX Lane AutoEQ CTRL-E Setting on Gen4 */
+ mtk_phy_update_field(sif_lane_base + PEXTP_DIG_LN_TRX_PIPE_IF_17,
+ RG_XTP_LN_RX_LF_CTLE_CSEL_GEN4, 1);
+
+ /* Set RX Lane AutoEQ CTRL-E for PCI-Express Gen1 to Gen 4 */
+ mtk_phy_update_field(sif_lane_base + PEXTP_DIG_LN_RX_F0,
+ RG_XTP_LN_RX_GEN1_CTLE1_CSEL, 13);
+
+ mtk_phy_update_field(sif_lane_base + PEXTP_DIG_LN_RX_F0,
+ RG_XTP_LN_RX_GEN2_CTLE1_CSEL, 13);
+
+ mtk_phy_update_field(sif_lane_base + PEXTP_DIG_LN_RX_F0,
+ RG_XTP_LN_RX_GEN3_CTLE1_CSEL, 13);
+
+ mtk_phy_update_field(sif_lane_base + PEXTP_DIG_LN_RX_F0,
+ RG_XTP_LN_RX_GEN4_CTLE1_CSEL, 0);
+
+ /* Set RX Lane AutoEQ's Edge EQ Ratio to 22 * 0.0625 = 1.375 */
+ mtk_phy_update_field(sif_lane_base + PEXTP_DIG_LN_RX2_AEQ_EDGE_0,
+ RG_XTP_LN_RX_AEQ_EGEQ_RATIO_GEN3, 22);
+
+ mtk_phy_update_field(sif_lane_base + PEXTP_DIG_LN_RX2_AEQ_EDGE_0,
+ RG_XTP_LN_RX_AEQ_EGEQ_RATIO_GEN4, 22);
+
+ /* Setup Digital lane TX Link Characteristics Table */
+ mtk_phy_update_field(sif_lane_base + PEXTP_DIG_LN_TX_LC_TABLE_RSWN_4,
+ RG_XTP_LN_TX_LC_PRESET_MGx_Px_C0, 10);
+
+ mtk_phy_update_field(sif_lane_base + PEXTP_DIG_LN_TX_LC_TABLE_RSWN_4,
+ RG_XTP_LN_TX_LC_PRESET_MGx_Px_CP1, 2);
+
+ mtk_phy_update_field(sif_lane_base + PEXTP_DIG_LN_TX_LC_TABLE_RSWN_8,
+ RG_XTP_LN_TX_LC_PRESET_MGx_Px_C0, 11);
+
+ mtk_phy_update_field(sif_lane_base + PEXTP_DIG_LN_TX_LC_TABLE_RSWN_8,
+ RG_XTP_LN_TX_LC_PRESET_MGx_Px_CP1, 1);
+
+ mtk_phy_update_field(sif_lane_base + PEXTP_DIG_LN_TX_LC_TABLE_RSWN_C,
+ RG_XTP_LN_TX_LC_PRESET_MGx_Px_C0, 12);
+
+ mtk_phy_update_field(sif_lane_base + PEXTP_DIG_LN_TX_LC_TABLE_RSWN_10,
+ RG_XTP_LN_TX_LC_PRESET_MGx_Px_C0, 13);
+
+ mtk_phy_update_field(sif_lane_base + PEXTP_DIG_LN_TX_LC_TABLE_RSWN_10,
+ RG_XTP_LN_TX_LC_PRESET_MGx_Px_CM1, 1);
+
+ mtk_phy_update_field(sif_lane_base + PEXTP_DIG_LN_TX_LC_TABLE_RSWN_14,
+ RG_XTP_LN_TX_LC_PRESET_MGx_Px_C0, 11);
+
+ mtk_phy_update_field(sif_lane_base + PEXTP_DIG_LN_TX_LC_TABLE_RSWN_14,
+ RG_XTP_LN_TX_LC_PRESET_MGx_Px_CM1, 1);
+
+ mtk_phy_update_field(sif_lane_base + PEXTP_DIG_LN_TX_LC_TABLE_RSWN_18,
+ RG_XTP_LN_TX_LC_PRESET_MGx_Px_C0, 10);
+
+ mtk_phy_update_field(sif_lane_base + PEXTP_DIG_LN_TX_LC_TABLE_RSWN_18,
+ RG_XTP_LN_TX_LC_PRESET_MGx_Px_CM1, 2);
+ }
+
+ if (pcie_sphy->calibration.supported) {
+ mtk_pcie_sphy_apply_calibration(pcie_sphy);
+
+ tx_impsel.pmos = pcie_sphy->calibration.tx_impsel.pmos;
+ tx_impsel.nmos = pcie_sphy->calibration.tx_impsel.nmos;
+ } else {
+ /* Set P=10, N=9 to prevent EMI if no calibration present */
+ tx_impsel.pmos = 10;
+ tx_impsel.nmos = 9;
+ }
+
+ /* Select TX Impedance on N and P MOSFETs */
+ for (i = 0; i < pcie_sphy->num_lanes; i++) {
+ void __iomem *sif_lane_base = pcie_sphy->sif_base + PEXTP_REG_LANE(i);
+
+ mtk_phy_update_field(sif_lane_base + PEXTP_ANA_LN_TRX_A0,
+ RG_XTP_LN_TX_IMPSEL_PMOS, tx_impsel.pmos);
+
+ mtk_phy_update_field(sif_lane_base + PEXTP_ANA_LN_TRX_A0,
+ RG_XTP_LN_TX_IMPSEL_NMOS, tx_impsel.nmos);
+ }
+
+ return 0;
+}
+
+static const struct phy_ops mtk_pcie_sphy_ops = {
+ .init = mtk_pcie_sphy_init,
+ .owner = THIS_MODULE,
+};
+
+static int mtk_pcie_sphy_get_one_cal_para(struct device *dev, const char *name, u8 max_val)
+{
+ u16 buf;
+ u8 tmp;
+ int ret;
+
+ /*
+ * All of the calibrations are always max 8 bits long, but some may
+ * be split between two different 8-bits cells: handle this corner
+ * case by retrying reading as u16.
+ */
+ ret = nvmem_cell_read_u8(dev, name, &tmp);
+ if (ret == 0)
+ buf = tmp;
+ else
+ ret = nvmem_cell_read_u16(dev, name, &buf);
+
+ if (ret == -ENOENT) {
+ dev_info(dev, "No calibration for %s. Using defaults\n", name);
+ return -ENOENT;
+ } else if (ret)
+ return dev_err_probe(dev, ret,
+ "Cannot get calibration data for %s\n", name);
+
+ if (buf > max_val)
+ return dev_err_probe(dev, -ERANGE,
+ "Bad value %u retrieved for %s.\n", buf, name);
+
+ return buf;
+}
+
+static int mtk_pcie_sphy_get_calibration_data(struct mtk_pcie_sphy *pcie_sphy)
+{
+ struct mtk_pcie_sphy_efuse *cal = &pcie_sphy->calibration;
+ struct device *dev = pcie_sphy->dev;
+ u8 version;
+ int ret;
+
+ if (pcie_sphy->num_lanes > MTK_PCIE_SPHY_CALIBRATION_MAX_DATA_LANES) {
+ dev_info(dev, "Skipping PHY calibration for more than %u lanes.\n",
+ MTK_PCIE_SPHY_CALIBRATION_MAX_DATA_LANES);
+ ret = -EOPNOTSUPP;
+ goto end;
+ }
+
+ ret = mtk_pcie_sphy_get_one_cal_para(dev, "int-r",
+ FIELD_MAX(RG_XTP_GLB_BIAS_INTR_CTRL));
+ if (ret < 0)
+ goto end;
+ cal->int_r_ctrl = ret;
+
+ ret = mtk_pcie_sphy_get_one_cal_para(dev, "xtp-vtrim",
+ FIELD_MAX(RG_XTP_GLB_BIAS_V2V_VTRIM));
+ if (ret < 0)
+ goto end;
+ cal->xtp_vtrim = ret;
+
+ ret = mtk_pcie_sphy_get_one_cal_para(dev, "cktx-pmos",
+ FIELD_MAX(RG_CKM_CKTX_IMPSEL_PMOS));
+ if (ret < 0)
+ goto end;
+ cal->cktx_impsel.pmos = ret;
+
+ ret = mtk_pcie_sphy_get_one_cal_para(dev, "cktx-nmos",
+ FIELD_MAX(RG_CKM_CKTX_IMPSEL_NMOS));
+ if (ret < 0)
+ goto end;
+ cal->cktx_impsel.nmos = ret;
+
+ ret = mtk_pcie_sphy_get_one_cal_para(dev, "cktx-r-mid",
+ FIELD_MAX(RG_CKM_CKTX_IMPSEL_SW));
+ if (ret < 0)
+ goto end;
+ cal->cktx_r_mid = ret;
+
+ ret = mtk_pcie_sphy_get_one_cal_para(dev, "rxfe-lanes-rl-ctle",
+ FIELD_MAX(RG_XTP_LN_RX_LEQ_RL_CTLE_CAL));
+ if (ret < 0)
+ goto end;
+ cal->rx_leq_rl_ctle = ret;
+
+ ret = mtk_pcie_sphy_get_one_cal_para(dev, "rxfe-lanes-rl-vga",
+ FIELD_MAX(RG_XTP_LN_RX_LEQ_RL_VGA_CAL));
+ if (ret < 0)
+ goto end;
+ cal->rx_leq_rl_vga = ret;
+
+ ret = mtk_pcie_sphy_get_one_cal_para(dev, "rxfe-lanes-rl-dfe",
+ FIELD_MAX(RG_XTP_LN_RX_LEQ_RL_DFE_CAL));
+ if (ret < 0)
+ goto end;
+ cal->rx_leq_rl_dfe = ret;
+
+ ret = mtk_pcie_sphy_get_one_cal_para(dev, "rx-lanes-imp",
+ FIELD_MAX(RG_XTP_LN_RX_IMPSEL));
+ if (ret < 0)
+ goto end;
+ cal->rx_impsel = ret;
+
+ ret = mtk_pcie_sphy_get_one_cal_para(dev, "tx-lanes-pmos",
+ FIELD_MAX(RG_XTP_LN_TX_IMPSEL_PMOS));
+ if (ret < 0)
+ goto end;
+ cal->tx_impsel.pmos = ret;
+
+ ret = mtk_pcie_sphy_get_one_cal_para(dev, "tx-lanes-nmos",
+ FIELD_MAX(RG_XTP_LN_TX_IMPSEL_NMOS));
+ if (ret < 0)
+ goto end;
+ cal->tx_impsel.nmos = ret;
+
+ ret = mtk_pcie_sphy_get_one_cal_para(dev, "tx-ln0-rswn",
+ FIELD_MAX(RG_XTP_LN_TX_RSWN_IMPSEL));
+ if (ret < 0)
+ goto end;
+ cal->tx_rswn_impsel[0] = ret;
+
+ if (pcie_sphy->num_lanes == 2) {
+ ret = mtk_pcie_sphy_get_one_cal_para(dev, "tx-ln1-rswn",
+ FIELD_MAX(RG_XTP_LN_TX_RSWN_IMPSEL));
+ if (ret < 0)
+ goto end;
+ cal->tx_rswn_impsel[1] = ret;
+ }
+
+ ret = mtk_pcie_sphy_get_one_cal_para(dev, "cal-version", 15);
+ if (ret < 0)
+ goto end;
+ version = ret;
+
+ /* Quirk for eFuse calibration table versions 0 to 4 */
+ if ((version <= MTK_PCIE_SPHY_CALIBRATION_LAST_QUIRK_VER) &&
+ cal->rx_leq_rl_ctle == 10) {
+ cal->rx_leq_rl_vga = cal->rx_leq_rl_ctle;
+ cal->rx_leq_rl_dfe = cal->rx_leq_rl_ctle;
+ }
+
+end:
+ if (ret < 0) {
+ /*
+ * If any of the calibration values is missing, or if there is
+ * no calibration at all in the eFuses, this is not a problem,
+ * as the PHY doesn't require one to actually work.
+ */
+ if (ret == -ENOENT || ret == -EOPNOTSUPP) {
+ cal->supported = false;
+ return 0;
+ }
+ return ret;
+ };
+ cal->supported = true;
+
+ return 0;
+}
+
+static int mtk_pcie_sphy_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct phy_provider *provider;
+ struct mtk_pcie_sphy *pcie_sphy;
+ u32 num_lanes;
+ int ret;
+
+ pcie_sphy = devm_kzalloc(dev, sizeof(*pcie_sphy), GFP_KERNEL);
+ if (!pcie_sphy)
+ return -ENOMEM;
+
+ pcie_sphy->sif_base = devm_platform_ioremap_resource_byname(pdev, "sif");
+ if (IS_ERR(pcie_sphy->sif_base))
+ return dev_err_probe(dev, PTR_ERR(pcie_sphy->sif_base),
+ "Failed to map phy-sif base\n");
+
+ pcie_sphy->ckm_base = devm_platform_ioremap_resource_byname(pdev, "ckm");
+ if (IS_ERR(pcie_sphy->ckm_base))
+ return dev_err_probe(dev, PTR_ERR(pcie_sphy->ckm_base),
+ "Failed to map phy-ckm base\n");
+
+ pcie_sphy->phy = devm_phy_create(dev, dev->of_node, &mtk_pcie_sphy_ops);
+ if (IS_ERR(pcie_sphy->phy))
+ return dev_err_probe(dev, PTR_ERR(pcie_sphy->phy),
+ "Failed to create PCIe phy\n");
+
+ ret = of_property_read_u32(dev->of_node, "num-lanes", &num_lanes);
+ if (ret)
+ num_lanes = 1;
+ else if (num_lanes > 4)
+ return dev_err_probe(dev, -EINVAL, "Invalid number of lanes.\n");
+
+ pcie_sphy->num_lanes = num_lanes;
+ pcie_sphy->dev = dev;
+
+ ret = mtk_pcie_sphy_get_calibration_data(pcie_sphy);
+ if (ret)
+ return ret;
+
+ phy_set_drvdata(pcie_sphy->phy, pcie_sphy);
+
+ ret = devm_pm_runtime_enable(dev);
+ if (ret)
+ return ret;
+
+ provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
+ if (IS_ERR(provider))
+ return dev_err_probe(dev, PTR_ERR(provider),
+ "Could not register PCI-Express S-PHY\n");
+
+ return 0;
+}
+
+static const struct of_device_id mtk_pcie_sphy_of_match[] = {
+ { .compatible = "mediatek,mt8196-pcie-sphy" },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, mtk_pcie_sphy_of_match);
+
+static struct platform_driver mtk_pcie_sphy_driver = {
+ .probe = mtk_pcie_sphy_probe,
+ .driver = {
+ .name = "mtk-pcie-sphy",
+ .of_match_table = mtk_pcie_sphy_of_match,
+ },
+};
+module_platform_driver(mtk_pcie_sphy_driver);
+
+MODULE_DESCRIPTION("MediaTek PCIe SPHY driver");
+MODULE_AUTHOR("AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>");
+MODULE_LICENSE("GPL");
--
2.55.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* Re: [PATCH v2 2/2] phy: mediatek: Add support for PCI-Express Gen4 S-PHY
From: AngeloGioacchino Del Regno @ 2026-07-20 15:19 UTC (permalink / raw)
To: sashiko-reviews
Cc: neil.armstrong, olteanv, devicetree, vkoul, linux-phy, conor+dt,
robh
In-Reply-To: <20260720141301.482441F00A3A@smtp.kernel.org>
On 7/20/26 16:13, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
> - [High] Improper handling of missing or unsupported NVMEM calibration data causes spurious error logs and hard probe failures.
> - [Medium] A valid calibration value of 0 is incorrectly treated as missing data, discarding all calibrations.
> - [Low] The TX_IMPSEL registers are updated twice with identical values during initialization.
> - [Low] Macros for AEQ_EGEQ_RATIO are defined but unused in favor of literal magic numbers.
> --
>
> --- Patch [2]: [PATCH v2 2/2] phy: mediatek: Add support for PCI-Express Gen4 S-PHY ---
> commit d91c6c0718440c44adc7e6a83f5aeca706a6c3d5
> Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> Subject: phy: mediatek: Add support for PCI-Express Gen4 S-PHY
>
> This patch adds support for the PCI-Express Gen4 S-PHY found in the latest
> MediaTek SoCs, including MT8196 and MT8894.
>
>> --- a/drivers/phy/mediatek/phy-mtk-pcie-sphy.c
>> +++ b/drivers/phy/mediatek/phy-mtk-pcie-sphy.c
> [ ... ]
>> +#define PEXTP_DIG_LN_RX2_AEQ_EDGE_0 0x6004
>> +# define RG_XTP_LN_RX_AEQ_EGEQ_RATIO_GEN3 GENMASK(21, 16)
>> +# define RG_XTP_LN_RX_AEQ_EGEQ_RATIO_GEN4 GENMASK(29, 24)
>> +# define AEQ_EGEQ_RATIO_GEN3_TO_22 0x16
>> +# define AEQ_EGEQ_RATIO_GEN4_TO_22 0x16
> [ ... ]
>> + mtk_phy_update_field(sif_lane_base + PEXTP_DIG_LN_RX2_AEQ_EDGE_0,
>> + RG_XTP_LN_RX_AEQ_EGEQ_RATIO_GEN3, 22);
>> +
>> + mtk_phy_update_field(sif_lane_base + PEXTP_DIG_LN_RX2_AEQ_EDGE_0,
>> + RG_XTP_LN_RX_AEQ_EGEQ_RATIO_GEN4, 22);
>
> [Severity: Low]
> Is there a reason the literal 22 is used here instead of the
> AEQ_EGEQ_RATIO_GEN3_TO_22 and AEQ_EGEQ_RATIO_GEN4_TO_22 macros defined
> earlier?
>
I forgot to remove those definitions. Will do for a v3 tomorrow.
> [ ... ]
>> +static void mtk_pcie_sphy_apply_calibration(struct mtk_pcie_sphy *pcie_sphy)
>> +{
> [ ... ]
>> + mtk_phy_update_field(sif_lane_base + PEXTP_ANA_LN_TRX_A0,
>> + RG_XTP_LN_TX_IMPSEL_PMOS, cal->tx_impsel.pmos);
>> +
>> + mtk_phy_update_field(sif_lane_base + PEXTP_ANA_LN_TRX_A0,
>> + RG_XTP_LN_TX_IMPSEL_NMOS, cal->tx_impsel.nmos);
>> + }
>> +}
>> +
> [ ... ]
>> +static int mtk_pcie_sphy_init(struct phy *phy)
>> +{
> [ ... ]
>> + if (pcie_sphy->calibration.supported) {
>> + mtk_pcie_sphy_apply_calibration(pcie_sphy);
>> +
>> + tx_impsel.pmos = pcie_sphy->calibration.tx_impsel.pmos;
>> + tx_impsel.nmos = pcie_sphy->calibration.tx_impsel.nmos;
>> + } else {
>> + /* Set P=10, N=9 to prevent EMI if no calibration present */
>> + tx_impsel.pmos = 10;
>> + tx_impsel.nmos = 9;
>> + }
>> +
>> + /* Select TX Impedance on N and P MOSFETs */
>> + for (i = 0; i < pcie_sphy->num_lanes; i++) {
>> + void __iomem *sif_lane_base = pcie_sphy->sif_base + PEXTP_REG_LANE(i);
>> +
>> + mtk_phy_update_field(sif_lane_base + PEXTP_ANA_LN_TRX_A0,
>> + RG_XTP_LN_TX_IMPSEL_PMOS, tx_impsel.pmos);
>> +
>> + mtk_phy_update_field(sif_lane_base + PEXTP_ANA_LN_TRX_A0,
>> + RG_XTP_LN_TX_IMPSEL_NMOS, tx_impsel.nmos);
>> + }
>
> [Severity: Low]
> Are these register updates redundant?
>
> When calibration is supported, mtk_pcie_sphy_apply_calibration() already
> writes cal->tx_impsel.pmos and cal->tx_impsel.nmos to PEXTP_ANA_LN_TRX_A0.
>
> Then this loop immediately overwrites the same registers with the exact same
> values.
>
Eh yes, fixed for v3.
> [ ... ]
>> +static int mtk_pcie_sphy_get_one_cal_para(struct device *dev, const char *name, u8 max_val)
>> +{
> [ ... ]
>> + ret = nvmem_cell_read_u8(dev, name, &tmp);
>> + if (ret == 0)
>> + buf = tmp;
>> + else
>> + ret = nvmem_cell_read_u16(dev, name, &buf);
>> +
>> + if (ret)
>> + return dev_err_probe(dev, ret,
>> + "Cannot get calibration data for %s\n", name);
>
> [Severity: High]
> Will this cause spurious error logs or fail the probe when calibration data is
> missing or unsupported?
>
> If the system does not have CONFIG_NVMEM enabled, nvmem_cell_read_u8() returns
> -EOPNOTSUPP, which gets logged as an error here.
>
> This error is then passed back up to mtk_pcie_sphy_get_calibration_data(),
> where it is not caught. This will cause the entire driver probe to fail.
>
> Similarly, if the cell is simply missing from the Device Tree, it returns
> -ENOENT. This also triggers an error log via dev_err_probe(), even though
> missing calibration data is explicitly documented as non-fatal and optional.
>
>> +
>> + if (buf == 0) {
>> + dev_info(dev, "No calibration for %s. Using defaults\n", name);
>> + return -ENOENT;
>> + }
>
> [Severity: Medium]
> Is a value of 0 always indicative of missing calibration?
>
> If a valid tuned calibration value is 0, this check will return -ENOENT and
> cause mtk_pcie_sphy_get_calibration_data() to discard all calibrations entirely.
>
> This also seems to prevent the quirk for cal-version == 0 from ever being
> applied:
>
Fixed for v3.
> [ ... ]
>> +static int mtk_pcie_sphy_get_calibration_data(struct mtk_pcie_sphy *pcie_sphy)
>> +{
> [ ... ]
>> + ret = mtk_pcie_sphy_get_one_cal_para(dev, "cal-version", 15);
>> + if (ret < 0)
>> + goto end;
>> + version = ret;
>> +
>> + /* Quirk for eFuse calibration table versions 0 to 4 */
>> + if ((version <= MTK_PCIE_SPHY_CALIBRATION_LAST_QUIRK_VER) &&
>> + cal->rx_leq_rl_ctle == 10) {
>
> Because mtk_pcie_sphy_get_one_cal_para() returns -ENOENT when it reads a 0,
> the version == 0 condition will never be reached.
>
Fixed for v3.
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v4 3/4] phy: qcom: qmp-usb-legacy: Fix possible NULL-deref on early runtime suspend
From: sashiko-bot @ 2026-07-20 15:10 UTC (permalink / raw)
To: Loic Poulain; +Cc: neil.armstrong, olteanv, vkoul, linux-phy
In-Reply-To: <20260720-qcom-usb-phy-fix-null-v4-3-4d2a0f06d53a@oss.qualcomm.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [High] Asynchronous `pm_runtime_put()` combined with inverted devres cleanup order introduces a Use-After-Free of `qmp->phy` on probe error.
Pre-existing issues:
- [Low] The `dev_pm_ops` structure is assigned unconditionally to the driver's `.pm` field without the `pm_ptr()` wrapper, producing dead code when PM is disabled.
--
--- Patch [3]: [PATCH v4 3/4] phy: qcom: qmp-usb-legacy: Fix possible NULL-deref on early runtime suspend ---
commit 3b25be245d58901212b78dd8afabc38a9e9e8abe
Author: Loic Poulain <loic.poulain@oss.qualcomm.com>
phy: qcom: qmp-usb-legacy: Fix possible NULL-deref on early runtime suspend
This commit addresses a possible NULL pointer dereference in the runtime
suspend path. It takes a runtime PM usage reference with
pm_runtime_get_noresume() before enabling PM and releases it only after
the PHY is created, preventing the suspend callback from executing while
the pointer is uninitialized.
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-usb-legacy.c b/drivers/phy/qualcomm/phy-qcom-qmp-usb-legacy.c
> index 8bf951b0490cf..8b91eec0b5eee 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-usb-legacy.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-usb-legacy.c
> @@ -1277,10 +1277,16 @@ static int qmp_usb_legacy_probe(struct platform_device *pdev)
> if (ret)
> goto err_node_put;
>
> + /*
> + * Enable runtime PM before creating the PHY, phy_create() only enables
> + * it on the PHY device if already enabled on the parent. Hold a usage
> + * reference so callbacks cannot run before qmp->phy is assigned.
> + */
> + pm_runtime_get_noresume(dev);
> pm_runtime_set_active(dev);
> ret = devm_pm_runtime_enable(dev);
> if (ret)
> - goto err_node_put;
> + goto err_pm_put;
[ ... ]
> qmp->phy = devm_phy_create(dev, np, &qmp_usb_legacy_phy_ops);
> if (IS_ERR(qmp->phy)) {
> ret = PTR_ERR(qmp->phy);
> dev_err(dev, "failed to create PHY: %d\n", ret);
> - goto err_node_put;
> + goto err_pm_put;
> }
>
> phy_set_drvdata(qmp->phy, qmp);
>
> + pm_runtime_put(dev);
> +
[Severity: High]
Does this asynchronous pm_runtime_put() introduce a use-after-free on the
probe error path?
If a user enables runtime PM via sysfs (setting power/control to auto) before
or during probe, this call drops the usage counter to zero and queues an
asynchronous suspend.
If devm_of_phy_provider_register() subsequently fails, probe returns an error
and devres unwinds in reverse order:
1. devm_phy_create() unwinds, freeing qmp->phy.
2. devm_pm_runtime_enable() unwinds, calling pm_runtime_disable().
If the queued asynchronous suspend executes after qmp->phy is freed but before
pm_runtime_disable() completes, qmp_usb_legacy_runtime_suspend() will
dereference the freed pointer:
drivers/phy/qualcomm/phy-qcom-qmp-usb-legacy.c:qmp_usb_legacy_runtime_suspend()
if (!qmp->phy->init_count) {
Would using pm_runtime_put_sync() here ensure any pending suspend completes
before a potential devres cleanup?
> of_node_put(np);
>
> phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
>
> return PTR_ERR_OR_ZERO(phy_provider);
[ ... ]
[Severity: Low]
This is a pre-existing issue, but I noticed the dev_pm_ops structure is
assigned unconditionally to the driver:
drivers/phy/qualcomm/phy-qcom-qmp-usb-legacy.c:qmp_usb_legacy_driver
.pm = &qmp_usb_legacy_pm_ops,
If CONFIG_PM is disabled, does this leave qmp_usb_legacy_pm_ops and its
referenced callbacks as dead code?
Could pm_ptr() be used here to allow the compiler to optimize away the unused
structure when PM support is not built in?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260720-qcom-usb-phy-fix-null-v4-0-4d2a0f06d53a@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 phy-next 2/8] soc: fsl: guts: add a global structure to hold state
From: Vladimir Oltean @ 2026-07-20 14:57 UTC (permalink / raw)
To: Michael Walle
Cc: devicetree, robh, conor+dt, vkoul, neil.armstrong, olteanv,
linux-phy
In-Reply-To: <20260720140139.dmgysphpse2jf3fi@skbuf>
Hi Michael,
On Mon, Jul 20, 2026 at 05:01:39PM +0300, Vladimir Oltean wrote:
> On Mon, Jul 20, 2026 at 01:50:49PM +0000, sashiko-bot@kernel.org wrote:
> > [Severity: High]
> > Will jumping to err_nomem cause a NULL pointer dereference?
> >
> > If the allocation of soc_dev_attr fails, the code now jumps to the error
> > handling path which dereferences soc_dev_attr to free its members:
> >
> > > @@ -269,6 +271,8 @@ static int __init fsl_guts_init(void)
> > > kfree(soc_dev_attr->revision);
> > > kfree(soc_dev_attr->serial_number);
> >
> > Since soc_dev_attr is NULL when this allocation fails, won't evaluating
> > soc_dev_attr->revision and other members result in a kernel panic?
>
> yeah.
>
> I tried to be nice and play with the existing "err_nomem" and "err"
> error handling labels, but the truth is that a single error cleanup path
> is just an antipattern that doesn't scale as code becomes more complex.
>
> Instead of having a single error handling procedure which needs to
> carefully fend off from various invalid contexts from all the goto
> sites, the standard convention is to have an incremental set of labels
> which undo just the setup that was done up to the goto jump site.
>
> I'll refactor fsl_guts_init() to use that convention for v4.
While I was doing this refactoring, I noticed that this code path:
if (soc_data)
soc_uid = fsl_guts_get_soc_uid(soc_data->sfp_compat,
soc_data->uid_offset);
if (soc_uid) {
soc_dev_attr->serial_number = kasprintf(GFP_KERNEL, "%016llX",
soc_uid);
// no kasprintf() NULL return code check here, unlike
// the rest of the code
}
soc_dev = soc_device_register(soc_dev_attr);
if (IS_ERR(soc_dev)) {
ret = PTR_ERR(soc_dev);
goto err;
}
proceeds with soc_device_register() even if fsl_guts_get_soc_uid() was
able to get a soc unique id from the security fuse processor, but we
fail to print that string to the soc_dev_attr->serial_number variable.
I don't see anywhere mentioned in commit 786dde1e59d7 ("soc: fsl: guts:
add serial_number support") that this would be intentional, so it can
just as well be an omission.
Is it OK if I replace the silent failure with a loud failure of the
entire fsl_guts_init()? The code executes only if soc_uid is non-zero
anyway.
kasprintf() fails only for memory related reasons.
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* [PATCH v4 4/4] phy: qcom: snps-femto-v2: Fix possible NULL-deref on early runtime suspend
From: Loic Poulain @ 2026-07-20 14:53 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Dmitry Baryshkov, Wesley Cheng
Cc: linux-arm-msm, linux-phy, linux-kernel, Dmitry Baryshkov,
Konrad Dybcio, Abel Vesa, Loic Poulain
In-Reply-To: <20260720-qcom-usb-phy-fix-null-v4-0-4d2a0f06d53a@oss.qualcomm.com>
Runtime PM must be enabled before creating the PHY, since phy_create()
only enables runtime PM on the PHY device if it is already enabled on
this parent device. However, the runtime PM callbacks dereference the
hsphy instance, which is not yet ready, leaving a window where a suspend
callback may trigger a NULL pointer dereference.
Take a runtime PM usage reference with pm_runtime_get_noresume() before
enabling runtime PM and release it once the PHY has been created, so that
no runtime suspend can run before the PHY is ready. This also prevents a
short window where an unnecessary runtime suspend can occur.
Use the devres-managed version to ensure PM runtime is symmetrically
disabled during driver removal for proper cleanup.
Fixes: 0d75f508a9d5 ("phy: qcom-snps: Add runtime suspend and resume handlers")
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Reviewed-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
---
drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c b/drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c
index eb0b0f61d98e03963bf92e5c822334f52636abe7..b1a85518fed0a7a35516d16da3ebf69afe760e40 100644
--- a/drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c
+++ b/drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c
@@ -599,8 +599,18 @@ static int qcom_snps_hsphy_probe(struct platform_device *pdev)
return dev_err_probe(dev, ret,
"failed to get regulator supplies\n");
+ /*
+ * Enable runtime PM before creating the PHY, phy_create() only enables
+ * it on the PHY device if already enabled on the parent. Hold a usage
+ * reference so callbacks cannot run before the PHY is ready.
+ */
+ pm_runtime_get_noresume(dev);
pm_runtime_set_active(dev);
- pm_runtime_enable(dev);
+ ret = devm_pm_runtime_enable(dev);
+ if (ret) {
+ pm_runtime_put_noidle(dev);
+ return ret;
+ }
/*
* Prevent runtime pm from being ON by default. Users can enable
* it using power/control in sysfs.
@@ -611,6 +621,7 @@ static int qcom_snps_hsphy_probe(struct platform_device *pdev)
if (IS_ERR(generic_phy)) {
ret = PTR_ERR(generic_phy);
dev_err(dev, "failed to create phy, %d\n", ret);
+ pm_runtime_put_noidle(dev);
return ret;
}
hsphy->phy = generic_phy;
@@ -622,8 +633,8 @@ static int qcom_snps_hsphy_probe(struct platform_device *pdev)
phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
if (!IS_ERR(phy_provider))
dev_dbg(dev, "Registered Qcom-SNPS HS phy\n");
- else
- pm_runtime_disable(dev);
+
+ pm_runtime_put(dev);
return PTR_ERR_OR_ZERO(phy_provider);
}
--
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 v4 3/4] phy: qcom: qmp-usb-legacy: Fix possible NULL-deref on early runtime suspend
From: Loic Poulain @ 2026-07-20 14:53 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Dmitry Baryshkov, Wesley Cheng
Cc: linux-arm-msm, linux-phy, linux-kernel, Abel Vesa, Loic Poulain
In-Reply-To: <20260720-qcom-usb-phy-fix-null-v4-0-4d2a0f06d53a@oss.qualcomm.com>
Runtime PM has to be enabled before creating the PHY, since phy_create()
only enables runtime PM on the PHY device if it is already enabled on
this parent device. However, the runtime suspend/resume callbacks
dereference the qmp->phy pointer, which is only assigned by
devm_phy_create(), later in probe:
`if (!qmp->phy->init_count) {`
There is thus a small window where the callback may run after
pm_runtime_enable() but before qmp->phy is set, dereferencing a NULL
pointer. This can also happen if user re-enables runtime-pm via the
sysfs attribute before qmp phy is initialized.
Take a runtime PM usage reference with pm_runtime_get_noresume() before
enabling runtime PM and release it once qmp->phy has been created, so
that no runtime suspend can run while the phy pointer is still
uninitialized.
Fixes: e464a3180a43 ("phy: qcom-qmp-usb: split off the legacy USB+dp_com support")
Reviewed-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
---
drivers/phy/qualcomm/phy-qcom-qmp-usb-legacy.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-usb-legacy.c b/drivers/phy/qualcomm/phy-qcom-qmp-usb-legacy.c
index 8bf951b0490cfd811635df8940de1b789e21b46c..8b91eec0b5eee0c76b64e2c2b7a15f0169ce4b09 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-usb-legacy.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-usb-legacy.c
@@ -1277,10 +1277,16 @@ static int qmp_usb_legacy_probe(struct platform_device *pdev)
if (ret)
goto err_node_put;
+ /*
+ * Enable runtime PM before creating the PHY, phy_create() only enables
+ * it on the PHY device if already enabled on the parent. Hold a usage
+ * reference so callbacks cannot run before qmp->phy is assigned.
+ */
+ pm_runtime_get_noresume(dev);
pm_runtime_set_active(dev);
ret = devm_pm_runtime_enable(dev);
if (ret)
- goto err_node_put;
+ goto err_pm_put;
/*
* Prevent runtime pm from being ON by default. Users can enable
* it using power/control in sysfs.
@@ -1289,23 +1295,27 @@ static int qmp_usb_legacy_probe(struct platform_device *pdev)
ret = phy_pipe_clk_register(qmp, np);
if (ret)
- goto err_node_put;
+ goto err_pm_put;
qmp->phy = devm_phy_create(dev, np, &qmp_usb_legacy_phy_ops);
if (IS_ERR(qmp->phy)) {
ret = PTR_ERR(qmp->phy);
dev_err(dev, "failed to create PHY: %d\n", ret);
- goto err_node_put;
+ goto err_pm_put;
}
phy_set_drvdata(qmp->phy, qmp);
+ pm_runtime_put(dev);
+
of_node_put(np);
phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
return PTR_ERR_OR_ZERO(phy_provider);
+err_pm_put:
+ pm_runtime_put_noidle(dev);
err_node_put:
of_node_put(np);
return ret;
--
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 v4 2/4] phy: qcom: qmp-usbc: Prevent unnecessary PM runtime suspend at boot
From: Loic Poulain @ 2026-07-20 14:53 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Dmitry Baryshkov, Wesley Cheng
Cc: linux-arm-msm, linux-phy, linux-kernel, Loic Poulain,
Konrad Dybcio, Abel Vesa
In-Reply-To: <20260720-qcom-usb-phy-fix-null-v4-0-4d2a0f06d53a@oss.qualcomm.com>
Runtime PM has to be enabled before creating the PHYs, since phy_create()
only enables runtime PM on the PHY devices if it is already enabled on
this parent device. This opens a small window where the device can be
runtime suspended after pm_runtime_enable() and before the later
pm_runtime_forbid(), causing an unnecessary suspend/resume cycle while
the PHYs are not yet registered.
Take a runtime PM usage reference with pm_runtime_get_noresume() before
enabling runtime PM and release it once the PHYs have been created to
prevent the device from being runtime suspended during that window.
Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Reviewed-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
---
drivers/phy/qualcomm/phy-qcom-qmp-usbc.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c b/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c
index ab3055bb5b0c198832ae06dfcd04fd34395e271d..4317224070fd8dec98090e9200ac7d0e97265979 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c
@@ -1959,10 +1959,16 @@ static int qmp_usbc_probe(struct platform_device *pdev)
if (ret)
goto err_node_put;
+ /*
+ * Enable runtime PM before creating the PHYs, phy_create() only enables
+ * it on the PHY devices if already enabled on the parent. Hold a usage
+ * reference so callbacks cannot run until the PHY is ready.
+ */
+ pm_runtime_get_noresume(dev);
pm_runtime_set_active(dev);
ret = devm_pm_runtime_enable(dev);
if (ret)
- goto err_node_put;
+ goto err_pm_put;
/*
* Prevent runtime pm from being ON by default. Users can enable
* it using power/control in sysfs.
@@ -1971,13 +1977,13 @@ static int qmp_usbc_probe(struct platform_device *pdev)
ret = qmp_usbc_register_clocks(qmp, np);
if (ret)
- goto err_node_put;
+ goto err_pm_put;
qmp->usb_phy = devm_phy_create(dev, np, &qmp_usbc_usb_phy_ops);
if (IS_ERR(qmp->usb_phy)) {
ret = PTR_ERR(qmp->usb_phy);
dev_err(dev, "failed to create PHY: %d\n", ret);
- goto err_node_put;
+ goto err_pm_put;
}
phy_set_drvdata(qmp->usb_phy, qmp);
@@ -1987,17 +1993,21 @@ static int qmp_usbc_probe(struct platform_device *pdev)
if (IS_ERR(qmp->dp_phy)) {
ret = PTR_ERR(qmp->dp_phy);
dev_err(dev, "failed to create PHY: %d\n", ret);
- goto err_node_put;
+ goto err_pm_put;
}
phy_set_drvdata(qmp->dp_phy, qmp);
}
+ pm_runtime_put(dev);
+
of_node_put(np);
phy_provider = devm_of_phy_provider_register(dev, qmp_usbc_phy_xlate);
return PTR_ERR_OR_ZERO(phy_provider);
+err_pm_put:
+ pm_runtime_put_noidle(dev);
err_node_put:
of_node_put(np);
return ret;
--
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 v4 1/4] phy: qcom: qmp-combo: Prevent unnecessary PM runtime suspend at boot
From: Loic Poulain @ 2026-07-20 14:53 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Dmitry Baryshkov, Wesley Cheng
Cc: linux-arm-msm, linux-phy, linux-kernel, Dmitry Baryshkov,
Abel Vesa, Konrad Dybcio, Loic Poulain
In-Reply-To: <20260720-qcom-usb-phy-fix-null-v4-0-4d2a0f06d53a@oss.qualcomm.com>
Runtime PM has to be enabled before creating the PHYs, since phy_create()
only enables runtime PM on the PHY devices if it is already enabled on
this parent device. This opens a small window where the device can be
runtime suspended after pm_runtime_enable() and before the later
pm_runtime_forbid(), causing an unnecessary suspend/resume cycle while
the PHYs are not yet registered.
Take a runtime PM usage reference with pm_runtime_get_noresume() before
enabling runtime PM and release it once the PHYs have been created to
prevent the device from being runtime suspended during that window.
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Reviewed-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
---
drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 19 ++++++++++++++-----
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
index cdcfad2e86b1d37650c4d7b0433319837ee9c9d4..affc8e040bb968a5b9d0b6f6fc2694c471368729 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
@@ -4934,10 +4934,16 @@ static int qmp_combo_probe(struct platform_device *pdev)
if (ret)
goto err_node_put;
+ /*
+ * Enable runtime PM before creating the PHYs, phy_create() only enables
+ * it on the PHY devices if already enabled on the parent. Hold a usage
+ * reference so callbacks cannot run until the PHY is ready.
+ */
+ pm_runtime_get_noresume(dev);
pm_runtime_set_active(dev);
ret = devm_pm_runtime_enable(dev);
if (ret)
- goto err_node_put;
+ goto err_pm_put;
/*
* Prevent runtime pm from being ON by default. Users can enable
* it using power/control in sysfs.
@@ -4946,14 +4952,13 @@ static int qmp_combo_probe(struct platform_device *pdev)
ret = qmp_combo_register_clocks(qmp, usb_np, dp_np);
if (ret)
- goto err_node_put;
-
+ goto err_pm_put;
qmp->usb_phy = devm_phy_create(dev, usb_np, &qmp_combo_usb_phy_ops);
if (IS_ERR(qmp->usb_phy)) {
ret = PTR_ERR(qmp->usb_phy);
dev_err(dev, "failed to create USB PHY: %d\n", ret);
- goto err_node_put;
+ goto err_pm_put;
}
phy_set_drvdata(qmp->usb_phy, qmp);
@@ -4962,11 +4967,13 @@ static int qmp_combo_probe(struct platform_device *pdev)
if (IS_ERR(qmp->dp_phy)) {
ret = PTR_ERR(qmp->dp_phy);
dev_err(dev, "failed to create DP PHY: %d\n", ret);
- goto err_node_put;
+ goto err_pm_put;
}
phy_set_drvdata(qmp->dp_phy, qmp);
+ pm_runtime_put(dev);
+
if (usb_np == dev->of_node)
phy_provider = devm_of_phy_provider_register(dev, qmp_combo_phy_xlate);
else
@@ -4977,6 +4984,8 @@ static int qmp_combo_probe(struct platform_device *pdev)
return PTR_ERR_OR_ZERO(phy_provider);
+err_pm_put:
+ pm_runtime_put_noidle(dev);
err_node_put:
of_node_put(usb_np);
of_node_put(dp_np);
--
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 v4 0/4] phy: qcom: Fix possible NULL-deref and runtime PM race conditions
From: Loic Poulain @ 2026-07-20 14:53 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Dmitry Baryshkov, Wesley Cheng
Cc: linux-arm-msm, linux-phy, linux-kernel, Dmitry Baryshkov,
Abel Vesa, Konrad Dybcio, Loic Poulain
Address potential NULL pointer dereferences and race conditions related
to runtime PM in several Qualcomm PHY drivers. In all cases, enabling
runtime PM before the PHY instance is fully initialized can lead to
crashes during early runtime suspend callbacks.
- Attach driver data before enabling runtime PM.
- Introduce initialization flags where needed to avoid dereferencing
uninitialized pointers.
- Reorder pm_runtime_enable() and pm_runtime_forbid() calls to prevent
unnecessary suspend/resume cycles during driver probe.
- Use devres-managed PM runtime helpers for proper cleanup.
Why it happens?
The PHY is a supplier of the USB device. A USB device cannot be probed
until all its suppliers are ready. As long as the PHY is not ready, the
device core keeps retrying the probe, which fails with -EPROBE_DEFER.
At some point the PHY probe finally runs, but the device core may still be
attempting to probe the USB device concurrently.
Inside __driver_probe_device(), we have:
ret = really_probe(dev, drv);
pm_request_idle(dev);
if (dev->parent)
pm_runtime_put(dev->parent);
pm_runtime_put_suppliers(dev);
return ret;
This means that whenever a USB probe attempt completes, whether with an
error or not, its suppliers are released via pm_runtime_put_suppliers().
Releasing suppliers may in turn trigger a runtime suspend.
In our case, since the PHY is a supplier of the USB device, the USB core
keeps 'looping' in __driver_probe_device() returning -EPROBE_DEFER until
the PHY becomes ready. As a result, pm_runtime_put_suppliers() may run
concurrently with the PHY's probe function. If this happens after
runtime PM has been enabled for the PHY, but before the driver has
forbidden suspend or taken a PM reference, the PHY may end up being
runtime-suspended 'unexpectedly'
---
Changes in V4:
- Instead of moving pm/forbid, increment the pm usage counter before
enabling runtime pm and decrement it after the PHY has been created. (Johan)
- Drop now unnecessary dev_set_drvdata() move in snps-femto-v2
- Drop 4/5 (qmp-usb-legacy: Prevent unnecessary PM runtime suspend at boot)
which is not required anymore.
- Rebase
- Link to v3: https://lore.kernel.org/all/20260205160240.748371-1-loic.poulain@oss.qualcomm.com/
Changes in v3:
Rebase on next and remove 2/6 (obsolete)
Changes in v2:
Split patches 2/4 and 3/4 so that the null‑pointer dereference fix and
the runtime‑PM enable/forbid reordering are logically separated.
Loic Poulain (4):
phy: qcom: qmp-combo: Prevent unnecessary PM runtime suspend at boot
phy: qcom: qmp-usbc: Prevent unnecessary PM runtime suspend at boot
phy: qcom: qmp-usb-legacy: Fix possible NULL-deref on early runtime
suspend
phy: qcom: snps-femto-v2: Fix possible NULL-deref on early runtime
suspend
drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 10 ++++-----
.../phy/qualcomm/phy-qcom-qmp-usb-legacy.c | 21 ++++++++++++-------
drivers/phy/qualcomm/phy-qcom-qmp-usbc.c | 10 ++++-----
drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c | 15 ++++++-------
4 files changed, 32 insertions(+), 24 deletions(-)
--
2.34.1
To: Vinod Koul <vkoul@kernel.org>
To: Neil Armstrong <neil.armstrong@linaro.org>
To: Dmitry Baryshkov <lumag@kernel.org>
To: Wesley Cheng <quic_wcheng@quicinc.com>
Cc: linux-arm-msm@vger.kernel.org
Cc: linux-phy@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
---
Loic Poulain (4):
phy: qcom: qmp-combo: Prevent unnecessary PM runtime suspend at boot
phy: qcom: qmp-usbc: Prevent unnecessary PM runtime suspend at boot
phy: qcom: qmp-usb-legacy: Fix possible NULL-deref on early runtime suspend
phy: qcom: snps-femto-v2: Fix possible NULL-deref on early runtime suspend
drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 19 ++++++++++++++-----
drivers/phy/qualcomm/phy-qcom-qmp-usb-legacy.c | 16 +++++++++++++---
drivers/phy/qualcomm/phy-qcom-qmp-usbc.c | 18 ++++++++++++++----
drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c | 17 ++++++++++++++---
4 files changed, 55 insertions(+), 15 deletions(-)
---
base-commit: 910b828b22b7b91054b3bd4be676a017444b0e00
change-id: 20260720-qcom-usb-phy-fix-null-ff097f56d1e4
Best regards,
--
Loic Poulain <loic.poulain@oss.qualcomm.com>
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH 1/5] dt-bindings: phy: qcom,qmp-usb3-dp: Extend X1E description for USB4
From: Konrad Dybcio @ 2026-07-20 14:41 UTC (permalink / raw)
To: Konrad Dybcio, Vinod Koul, Neil Armstrong, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson
Cc: linux-kernel, linux-phy, linux-arm-msm, devicetree, usb4-upstream,
Raghavendra Thoorpu, Mika Westerberg, Sven Peter
In-Reply-To: <20260518-topic-usb4phy-v1-1-71d827c49dca@oss.qualcomm.com>
On 5/18/26 12:29 PM, Konrad Dybcio wrote:
> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>
> Some instances of the QMP combo PHY (called USB43DP) feature a third
> functional sub-block, responsible for USB4/Thunderbolt 3 communication.
>
> Compared to the today's state of the binding, one more clock (P2RR2P -
> PHY-to-Router, Router-to-PHY) needs to be enabled for the PHY to be
> able to switch to USB4 mode. Allow that for X1E.
>
> Also, add a bindings define to let consumers access it.
>
> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> ---
It's been a while and I see Vinod is picking up patches.
Krzysztof, could you please take a look at this binding change?
Konrad
> .../devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml | 3 ++-
> include/dt-bindings/phy/phy-qcom-qmp.h | 1 +
> 2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml
> index 3d537b7f9985..eba4bee474fb 100644
> --- a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml
> +++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml
> @@ -52,7 +52,7 @@ properties:
> - const: ref
> - const: com_aux
> - const: usb3_pipe
> - - const: cfg_ahb
> + - enum: [ p2rr2p_pipe, cfg_ahb ]
>
> power-domains:
> maxItems: 1
> @@ -186,6 +186,7 @@ allOf:
> enum:
> - qcom,sc7180-qmp-usb3-dp-phy
> - qcom,sdm845-qmp-usb3-dp-phy
> + - qcom,x1e80100-qmp-usb3-dp-phy
> then:
> properties:
> clocks:
> diff --git a/include/dt-bindings/phy/phy-qcom-qmp.h b/include/dt-bindings/phy/phy-qcom-qmp.h
> index 6b43ea9e0051..1c3ce0c02b0c 100644
> --- a/include/dt-bindings/phy/phy-qcom-qmp.h
> +++ b/include/dt-bindings/phy/phy-qcom-qmp.h
> @@ -16,6 +16,7 @@
> /* QMP USB4-USB3-DP PHYs */
> #define QMP_USB43DP_USB3_PHY 0
> #define QMP_USB43DP_DP_PHY 1
> +#define QMP_USB43DP_USB4_PHY 2
>
> /* QMP PCIE PHYs */
> #define QMP_PCIE_PIPE_CLK 0
>
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
page: next (older)
- 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