* Re: [PATCH v2 5/7] phy: ti: gmii-sel: add support for J722S SoC family
From: Vladimir Oltean @ 2026-03-30 22:37 UTC (permalink / raw)
To: Nora Schiffer
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Nishanth Menon, Vignesh Raghavendra, Tero Kristo,
Siddharth Vadapalli, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Vinod Koul, Neil Armstrong, netdev, devicetree,
linux-kernel, linux-phy, linux-arm-kernel, linux
In-Reply-To: <5d00697f133cd33a1df62ac7ebf73e507e49ed2f.1774354734.git.nora.schiffer@ew.tq-group.com>
Hi Nora,
On Tue, Mar 24, 2026 at 01:29:41PM +0100, Nora Schiffer wrote:
> The J722S gmii-sel is mostly identical to the AM64's, but additionally
> supports SGMII.
>
> Signed-off-by: Nora Schiffer <nora.schiffer@ew.tq-group.com>
> ---
> drivers/phy/ti/phy-gmii-sel.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/drivers/phy/ti/phy-gmii-sel.c b/drivers/phy/ti/phy-gmii-sel.c
> index 6213c2b6005a5..4e242b1892334 100644
> --- a/drivers/phy/ti/phy-gmii-sel.c
> +++ b/drivers/phy/ti/phy-gmii-sel.c
> @@ -251,6 +251,13 @@ struct phy_gmii_sel_soc_data phy_gmii_sel_soc_am654 = {
> .regfields = phy_gmii_sel_fields_am654,
> };
>
> +static const
> +struct phy_gmii_sel_soc_data phy_gmii_sel_soc_j722s = {
> + .use_of_data = true,
> + .regfields = phy_gmii_sel_fields_am654,
> + .extra_modes = BIT(PHY_INTERFACE_MODE_SGMII),
I'm not familiar with the hardware, but "mostly identical to AM64, but
additionally supports SGMII" does not explain why j722s does not inherit
the features that am654 has (PHY_GMII_SEL_RGMII_ID_MODE and
BIT(PHY_GMII_SEL_FIXED_TX_DELAY).
The phy-gmii-sel from j722s does support RGMII, right? Because in lack
of the PHY_GMII_SEL_RGMII_ID_MODE feature, phy_gmii_sel_mode() will just
silently skip the regmap_field_write(regfield, rgmii_id) call, and
return successfully despite an incomplete configuration.
We have the phy_validate() call and phy_ops::validate() through which
the PHY can report to the Ethernet controller which phy_interface_t it
supports and which it doesn't. If the j722s doesn't support RGMII, maybe
it should implement this method.
> +};
> +
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v2 6/6] phy: realtek: usb2: Make configs available for MACH_REALTEK_RTL
From: Vladimir Oltean @ 2026-03-30 21:52 UTC (permalink / raw)
To: Rustam Adilov
Cc: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Stanley Chang, linux-phy, devicetree, linux-kernel
In-Reply-To: <20260327160638.15134-7-adilov@disroot.org>
On Fri, Mar 27, 2026 at 09:06:38PM +0500, Rustam Adilov wrote:
> Add the MACH_REALTEK_RTL to the if statement to make the config
> options available for Realtek RTL SoCs as well.
>
> Signed-off-by: Rustam Adilov <adilov@disroot.org>
> ---
> drivers/phy/realtek/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/phy/realtek/Kconfig b/drivers/phy/realtek/Kconfig
> index 75ac7e7c31ae..f9eadffacd18 100644
> --- a/drivers/phy/realtek/Kconfig
> +++ b/drivers/phy/realtek/Kconfig
> @@ -3,7 +3,7 @@
> # Phy drivers for Realtek platforms
> #
>
> -if ARCH_REALTEK || COMPILE_TEST
> +if ARCH_REALTEK || MACH_REALTEK_RTL || COMPILE_TEST
>
> config PHY_RTK_RTD_USB2PHY
> tristate "Realtek RTD USB2 PHY Transceiver Driver"
> --
> 2.53.0
>
>
The file now reads:
if ARCH_REALTEK || MACH_REALTEK_RTL || COMPILE_TEST
...
endif # ARCH_REALTEK || COMPILE_TEST
Please update the end comment as well.
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v2 5/6] phy: realtek: usb2: add support for RTL9607C USB2 PHY
From: Vladimir Oltean @ 2026-03-30 21:50 UTC (permalink / raw)
To: Rustam Adilov
Cc: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Stanley Chang, linux-phy, devicetree, linux-kernel,
Michael Zavertkin
In-Reply-To: <20260327160638.15134-6-adilov@disroot.org>
On Fri, Mar 27, 2026 at 09:06:37PM +0500, Rustam Adilov wrote:
> Add support for the usb2 phy of RTL9607C series based SoCs.
> Add the macros and phy config struct for rtl9607.
>
> RTL9607C requires to clear a "force host disconnect" bit in the
> specific register (which is at an offset from reg_wrap_vstatus)
> before proceeding with phy parameter writes.
>
> Add the bool variable to the driver data struct and hide this whole
> procedure under the if statement that checks this new variable.
>
> Co-developed-by: Michael Zavertkin <misha.zavertkin@mail.ru>
> Signed-off-by: Michael Zavertkin <misha.zavertkin@mail.ru>
> Signed-off-by: Rustam Adilov <adilov@disroot.org>
> ---
> drivers/phy/realtek/phy-rtk-usb2.c | 57 ++++++++++++++++++++++++++++++
> 1 file changed, 57 insertions(+)
>
> diff --git a/drivers/phy/realtek/phy-rtk-usb2.c b/drivers/phy/realtek/phy-rtk-usb2.c
> index 070cba1e0e0a..bf22d12681dc 100644
> --- a/drivers/phy/realtek/phy-rtk-usb2.c
> +++ b/drivers/phy/realtek/phy-rtk-usb2.c
> @@ -26,6 +26,12 @@
> #define PHY_VCTRL_SHIFT 8
> #define PHY_REG_DATA_MASK 0xff
>
> +#define PHY_9607_VSTS_BUSY BIT(17)
> +#define PHY_9607_NEW_REG_REQ BIT(13)
> +
> +#define PHY_9607_FORCE_DISCONNECT_REG 0x10
> +#define PHY_9607_FORCE_DISCONNECT_BIT BIT(5)
> +
> #define GET_LOW_NIBBLE(addr) ((addr) & 0x0f)
> #define GET_HIGH_NIBBLE(addr) (((addr) & 0xf0) >> 4)
>
> @@ -109,6 +115,7 @@ struct phy_cfg {
>
> u32 (*read)(void __iomem *reg);
> void (*write)(u32 val, void __iomem *reg);
> + bool force_host_disconnect;
> };
>
> struct phy_parameter {
> @@ -614,6 +621,16 @@ static int do_rtk_phy_init(struct rtk_phy *rtk_phy, int index)
> goto do_toggle;
> }
>
> + if (phy_cfg->force_host_disconnect) {
> + /* disable force-host-disconnect */
> + u32 temp = readl(phy_reg->reg_wrap_vstatus + PHY_9607_FORCE_DISCONNECT_REG);
> +
> + temp &= ~PHY_9607_FORCE_DISCONNECT_BIT;
> + writel(temp, phy_reg->reg_wrap_vstatus + PHY_9607_FORCE_DISCONNECT_REG);
> +
> + mdelay(10);
LLM review:
Could we use msleep(10) or usleep_range(10000, 11000) here instead of
mdelay(10)?
Since do_rtk_phy_init() executes as part of the phy_ops->init callback
with a mutex held from a sleepable process context, spinning the CPU for
10ms wastes CPU resources and increases scheduling latency.
> + }
> +
> /* Set page 0 */
> phy_data_page = phy_cfg->page0;
> rtk_phy_set_page(phy_reg, 0);
> @@ -1141,6 +1158,7 @@ static const struct phy_cfg rtd1295_phy_cfg = {
> .new_reg_req = PHY_NEW_REG_REQ,
> .read = phy_read,
> .write = phy_write,
> + .force_host_disconnect = false,
You don't need to initialize rodata struct fields with false/0/NULL.
> };
>
> static const struct phy_cfg rtd1395_phy_cfg = {
> @@ -1170,6 +1188,7 @@ static const struct phy_cfg rtd1395_phy_cfg = {
> .new_reg_req = PHY_NEW_REG_REQ,
> .read = phy_read,
> .write = phy_write,
> + .force_host_disconnect = false,
> };
>
> static const struct phy_cfg rtd1395_phy_cfg_2port = {
> @@ -1199,6 +1218,7 @@ static const struct phy_cfg rtd1395_phy_cfg_2port = {
> .new_reg_req = PHY_NEW_REG_REQ,
> .read = phy_read,
> .write = phy_write,
> + .force_host_disconnect = false,
> };
>
> static const struct phy_cfg rtd1619_phy_cfg = {
> @@ -1226,6 +1246,7 @@ static const struct phy_cfg rtd1619_phy_cfg = {
> .new_reg_req = PHY_NEW_REG_REQ,
> .read = phy_read,
> .write = phy_write,
> + .force_host_disconnect = false,
> };
>
> static const struct phy_cfg rtd1319_phy_cfg = {
> @@ -1257,6 +1278,7 @@ static const struct phy_cfg rtd1319_phy_cfg = {
> .new_reg_req = PHY_NEW_REG_REQ,
> .read = phy_read,
> .write = phy_write,
> + .force_host_disconnect = false,
> };
>
> static const struct phy_cfg rtd1312c_phy_cfg = {
> @@ -1287,6 +1309,7 @@ static const struct phy_cfg rtd1312c_phy_cfg = {
> .new_reg_req = PHY_NEW_REG_REQ,
> .read = phy_read,
> .write = phy_write,
> + .force_host_disconnect = false,
> };
>
> static const struct phy_cfg rtd1619b_phy_cfg = {
> @@ -1317,6 +1340,7 @@ static const struct phy_cfg rtd1619b_phy_cfg = {
> .new_reg_req = PHY_NEW_REG_REQ,
> .read = phy_read,
> .write = phy_write,
> + .force_host_disconnect = false,
> };
>
> static const struct phy_cfg rtd1319d_phy_cfg = {
> @@ -1347,6 +1371,7 @@ static const struct phy_cfg rtd1319d_phy_cfg = {
> .new_reg_req = PHY_NEW_REG_REQ,
> .read = phy_read,
> .write = phy_write,
> + .force_host_disconnect = false,
> };
>
> static const struct phy_cfg rtd1315e_phy_cfg = {
> @@ -1378,6 +1403,37 @@ static const struct phy_cfg rtd1315e_phy_cfg = {
> .new_reg_req = PHY_NEW_REG_REQ,
> .read = phy_read,
> .write = phy_write,
> + .force_host_disconnect = false,
> +};
> +
> +static const struct phy_cfg rtl9607_phy_cfg = {
> + .page0_size = MAX_USB_PHY_PAGE0_DATA_SIZE,
> + .page0 = { [0] = {0xe0, 0x95},
> + [4] = {0xe4, 0x6a},
> + [12] = {0xf3, 0x31}, },
> + .page1_size = MAX_USB_PHY_PAGE1_DATA_SIZE,
> + .page1 = { [0] = {0xe0, 0x26}, },
> + .page2_size = MAX_USB_PHY_PAGE2_DATA_SIZE,
> + .page2 = { [7] = {0xe7, 0x33}, },
> + .num_phy = 1,
> + .check_efuse = false,
Similar for these (+do_toggle_driving, use_default_parameter).
> + .check_efuse_version = CHECK_EFUSE_V2,
> + .efuse_dc_driving_rate = EFUS_USB_DC_CAL_RATE,
> + .dc_driving_mask = 0x1f,
> + .efuse_dc_disconnect_rate = EFUS_USB_DC_DIS_RATE,
> + .dc_disconnect_mask = 0xf,
> + .usb_dc_disconnect_at_page0 = true,
> + .do_toggle = true,
> + .do_toggle_driving = false,
> + .driving_updated_for_dev_dis = 0x8,
> + .use_default_parameter = false,
> + .is_double_sensitivity_mode = true,
> + .vstatus_offset = 0xc,
> + .vstatus_busy = PHY_9607_VSTS_BUSY,
> + .new_reg_req = PHY_9607_NEW_REG_REQ,
> + .read = phy_read_le,
> + .write = phy_write_le,
> + .force_host_disconnect = true,
> };
>
> static const struct of_device_id usbphy_rtk_dt_match[] = {
> @@ -1390,6 +1446,7 @@ static const struct of_device_id usbphy_rtk_dt_match[] = {
> { .compatible = "realtek,rtd1395-usb2phy-2port", .data = &rtd1395_phy_cfg_2port },
> { .compatible = "realtek,rtd1619-usb2phy", .data = &rtd1619_phy_cfg },
> { .compatible = "realtek,rtd1619b-usb2phy", .data = &rtd1619b_phy_cfg },
> + { .compatible = "realtek,rtl9607-usb2phy", .data = &rtl9607_phy_cfg },
> {},
> };
> MODULE_DEVICE_TABLE(of, usbphy_rtk_dt_match);
> --
> 2.53.0
>
>
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v2 4/6] phy: realtek: usb2: introduce reset controller struct
From: Vladimir Oltean @ 2026-03-30 21:39 UTC (permalink / raw)
To: Rustam Adilov
Cc: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Stanley Chang, linux-phy, devicetree, linux-kernel,
Michael Zavertkin
In-Reply-To: <20260327160638.15134-5-adilov@disroot.org>
On Fri, Mar 27, 2026 at 09:06:36PM +0500, Rustam Adilov wrote:
> In RTL9607C, there is so called "IP Enable Controller" which resemble
> reset controller with reset lines and is used for various things like
> USB, PCIE, GMAC and such.
>
> Introduce the reset_control struct to this driver to handle deasserting
> usb2 phy reset line.
>
> Make use of the function devm_reset_control_array_get_optional_exclusive()
> function to get the reset controller and since existing RTD SoCs don't
> specify the resets we can have a cleaner code.
>
> Co-developed-by: Michael Zavertkin <misha.zavertkin@mail.ru>
> Signed-off-by: Michael Zavertkin <misha.zavertkin@mail.ru>
> Signed-off-by: Rustam Adilov <adilov@disroot.org>
> ---
> drivers/phy/realtek/phy-rtk-usb2.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/drivers/phy/realtek/phy-rtk-usb2.c b/drivers/phy/realtek/phy-rtk-usb2.c
> index e65b8525b88b..070cba1e0e0a 100644
> --- a/drivers/phy/realtek/phy-rtk-usb2.c
> +++ b/drivers/phy/realtek/phy-rtk-usb2.c
> @@ -17,6 +17,7 @@
> #include <linux/sys_soc.h>
> #include <linux/mfd/syscon.h>
> #include <linux/phy/phy.h>
> +#include <linux/reset.h>
> #include <linux/usb.h>
>
> /* GUSB2PHYACCn register */
> @@ -130,6 +131,7 @@ struct rtk_phy {
> struct phy_cfg *phy_cfg;
> int num_phy;
> struct phy_parameter *phy_parameter;
> + struct reset_control *phy_rst;
>
> struct dentry *debug_dir;
> };
> @@ -602,6 +604,10 @@ static int do_rtk_phy_init(struct rtk_phy *rtk_phy, int index)
> phy_parameter = &((struct phy_parameter *)rtk_phy->phy_parameter)[index];
> phy_reg = &phy_parameter->phy_reg;
>
> + reset_control_deassert(rtk_phy->phy_rst);
LLM review says:
(less important)
Can reset_control_deassert() fail here? If there is a hardware communication
error with the reset controller, should this check the return value and
propagate the error up instead of proceeding to configure the PHY?
Additionally, since the exclusive reset line is deasserted here, does this
code need a corresponding reset_control_assert() in the driver's teardown
or exit path? Leaving the IP block permanently enabled after shutdown could
lead to power leaks and prevent proper hardware re-initialization.
> +
> + mdelay(5);
(more important)
This code unnecessarily penalizes existing platforms. If rtk_phy->phy_rst
is NULL (as on older platforms where the optional reset is not defined), the
delay still executes.
Also, since PHY initialization callbacks run in a sleepable context, would it
be better to use a sleep-based delay like usleep_range(5000, 6000) to yield
the CPU instead of busy-waiting with mdelay(5)?
> +
> if (phy_cfg->use_default_parameter) {
> dev_dbg(rtk_phy->dev, "%s phy#%d use default parameter\n",
> __func__, index);
> @@ -1069,6 +1075,12 @@ static int rtk_usb2phy_probe(struct platform_device *pdev)
>
> rtk_phy->num_phy = phy_cfg->num_phy;
>
> + rtk_phy->phy_rst = devm_reset_control_array_get_optional_exclusive(dev);
> + if (IS_ERR(rtk_phy->phy_rst)) {
> + dev_err(dev, "usb2 phy resets are not working\n");
> + return PTR_ERR(rtk_phy->phy_rst);
> + }
> +
(still LLM review)
If the reset controller driver is not yet ready, this will return
-EPROBE_DEFER and print an error message to the kernel log.
Should this use dev_err_probe() to silently handle probe deferral while
correctly logging actual errors?
> ret = parse_phy_data(rtk_phy);
> if (ret)
> goto err;
> --
> 2.53.0
>
>
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v2 2/6] phy: realtek: usb2: introduce read and write functions to driver data
From: Vladimir Oltean @ 2026-03-30 21:32 UTC (permalink / raw)
To: Rustam Adilov
Cc: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Stanley Chang, linux-phy, devicetree, linux-kernel,
Michael Zavertkin
In-Reply-To: <20260330211918.y7su36j47e3uelcv@skbuf>
On Tue, Mar 31, 2026 at 12:19:18AM +0300, Vladimir Oltean wrote:
> On Fri, Mar 27, 2026 at 09:06:34PM +0500, Rustam Adilov wrote:
> > +static inline u32 phy_read(void __iomem *reg)
> > +{
> > + return readl(reg);
> > +}
> > +
> > +static inline u32 phy_read_le(void __iomem *reg)
> > +{
> > + return le32_to_cpu(readl(reg));
> > +}
> > +
> > +static inline void phy_write(u32 val, void __iomem *reg)
> > +{
> > + writel(val, reg);
> > +}
> > +
> > +static inline void phy_write_le(u32 val, void __iomem *reg)
> > +{
> > + writel(cpu_to_le32(val), reg);
> > +}
>
> Please don't name driver-level functions phy_read() and phy_write().
> That will collide with networking API functions of the same name and
> will make grep-based code searching more difficult.
>
> Also, have you looked at regmap? It has native support for endianness;
> it supports regmap_field_read()/regmap_field_write() for abstracting
> registers which may be found at different places for different HW;
> it offers regmap_read_poll_timeout() so you don't have to pass the
> function pointer to utmi_wait_register(). It seems the result would be a
> bit more elegant.
Even if you decide not to use regmap. I thought I should let you know
that LLM review says:
Are these double byte-swaps intentional?
Since readl() and writel() inherently perform little-endian memory accesses
and handle byte-swapping on big-endian architectures automatically, won't
wrapping them in le32_to_cpu() and cpu_to_le32() apply a second, redundant
byte-swap?
On big-endian systems, wouldn't these double swaps cancel each other out
and result in a native big-endian access instead of the intended
little-endian access? If the SoC bus bridge implicitly swaps and requires
a native access, should __raw_readl() and __raw_writel() (or ioread32be /
iowrite32be) be used instead to avoid obfuscating it with double-swaps?
Also, does passing the __le32 restricted type returned by cpu_to_le32()
into writel() (which expects a native u32) trigger Sparse static analysis
warnings for an incorrect type in argument?
For reference:
https://elixir.bootlin.com/linux/v6.19.10/source/include/asm-generic/io.h#L184
/*
* {read,write}{b,w,l,q}() access little endian memory and return result in
* native endianness.
*/
and yes, your patch does trigger sparse warnings:
../drivers/phy/realtek/phy-rtk-usb2.c:153:16: warning: cast to restricted __le32
../drivers/phy/realtek/phy-rtk-usb2.c:163:16: warning: incorrect type in argument 1 (different base types)
../drivers/phy/realtek/phy-rtk-usb2.c:163:16: expected unsigned int val
../drivers/phy/realtek/phy-rtk-usb2.c:163:16: got restricted __le32 [usertype]
Furthermore, please drop the 'inline' keyword from C files and let the
compiler decide. Your use of this keyword has no value - you declare
phy_read(), phy_read_le() etc as inline but then assign function
pointers to them. How can the compiler inline the indirect calls?
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v2 2/6] phy: realtek: usb2: introduce read and write functions to driver data
From: Vladimir Oltean @ 2026-03-30 21:19 UTC (permalink / raw)
To: Rustam Adilov
Cc: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Stanley Chang, linux-phy, devicetree, linux-kernel,
Michael Zavertkin
In-Reply-To: <20260327160638.15134-3-adilov@disroot.org>
On Fri, Mar 27, 2026 at 09:06:34PM +0500, Rustam Adilov wrote:
> +static inline u32 phy_read(void __iomem *reg)
> +{
> + return readl(reg);
> +}
> +
> +static inline u32 phy_read_le(void __iomem *reg)
> +{
> + return le32_to_cpu(readl(reg));
> +}
> +
> +static inline void phy_write(u32 val, void __iomem *reg)
> +{
> + writel(val, reg);
> +}
> +
> +static inline void phy_write_le(u32 val, void __iomem *reg)
> +{
> + writel(cpu_to_le32(val), reg);
> +}
Please don't name driver-level functions phy_read() and phy_write().
That will collide with networking API functions of the same name and
will make grep-based code searching more difficult.
Also, have you looked at regmap? It has native support for endianness;
it supports regmap_field_read()/regmap_field_write() for abstracting
registers which may be found at different places for different HW;
it offers regmap_read_poll_timeout() so you don't have to pass the
function pointer to utmi_wait_register(). It seems the result would be a
bit more elegant.
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v4 1/2] dt-bindings: phy: qcom-edp: Add reference clock for sa8775p eDP PHY
From: Dmitry Baryshkov @ 2026-03-30 18:42 UTC (permalink / raw)
To: Bjorn Andersson
Cc: Ritesh Kumar, robin.clark, lumag, abhinav.kumar, sean,
marijn.suijten, maarten.lankhorst, mripard, tzimmermann, airlied,
simona, robh, krzk+dt, conor+dt, quic_mahap, konradybcio, mani,
James.Bottomley, martin.petersen, vkoul, kishon,
cros-qcom-dts-watchers, linux-phy, linux-arm-msm, dri-devel,
freedreno, devicetree, linux-kernel, linux-scsi, quic_vproddut
In-Reply-To: <acqGFwaVFQ3ZNmlR@baldur>
On Mon, Mar 30, 2026 at 09:28:40AM -0500, Bjorn Andersson wrote:
> On Wed, Mar 11, 2026 at 06:37:31PM +0530, Ritesh Kumar wrote:
> >
> > On 3/5/2026 12:27 AM, Bjorn Andersson wrote:
> > > On Wed, Jan 28, 2026 at 05:18:49PM +0530, Ritesh Kumar wrote:
> > > > The initial sa8775p eDP PHY binding contribution missed adding support for
> > > > voting on the eDP reference clock. This went unnoticed because the UFS PHY
> > > > driver happened to enable the same clock.
> > > > > After commit 77d2fa54a945 ("scsi: ufs: qcom : Refactor
> > > phy_power_on/off
> > > > calls"), the eDP reference clock is no longer kept enabled, which results
> > > > in the following PHY power-on failure:
> > > > > phy phy-aec2a00.phy.10: phy poweron failed --> -110
> > > > > To fix this, explicit voting for the eDP reference clock is
> > > required.
> > > > This patch adds the eDP reference clock for sa8775p eDP PHY and updates
> > > > the corresponding example node.
> > > > > Signed-off-by: Ritesh Kumar <quic_riteshk@quicinc.com>
> > >
> > > Is there any reason why you didn't follow up on this patch Ritesh?
> > > Looks like it's ready to be merged.
> >
> > I was waiting for patch to merge as there is no pending comments.
> >
>
> It's been two months now, if you want your patches to be merged please
> show that - ask the maintainer for a status update, ask a colleague to
> send a reviewed-by...
>
> Perhaps the maintainer lost track of your change?
>
> Perhaps it's not clear that the change "need" an Ack from e.g. Dmitry
> and then it should be merged by Vinod? Because you're changing two
> different subsystems but leave it up to the maintainers to figure out
> how to deal with this...
>
>
> Either way, show that you want this to be merged, don't just wait until
> the situation resolves itself.
For merging through linux-phy:
Acked-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Vinod, please pick it up through your tree.
--
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: (subset) [PATCH v2 1/1] arm64: dts: qcom: hamoa: Fix incomplete Root Port property migration
From: Bjorn Andersson @ 2026-03-30 16:01 UTC (permalink / raw)
To: konradybcio, robh, krzk+dt, conor+dt, jingoohan1, mani,
lpieralisi, kwilczynski, bhelgaas, johan+linaro, vkoul, kishon,
neil.armstrong, Abel Vesa, Ziyue Zhang
Cc: linux-arm-msm, devicetree, linux-kernel, linux-pci, linux-phy,
qiang.yu, quic_krichai, quic_vbadigan
In-Reply-To: <20260330020934.3501247-1-ziyue.zhang@oss.qualcomm.com>
On Mon, 30 Mar 2026 10:09:34 +0800, Ziyue Zhang wrote:
> Historically, the Qualcomm PCIe controller node (Host bridge) described
> all Root Port properties, such as PHY, PERST#, and WAKE#. But to provide
> a more accurate hardware description and to support future multi-Root Port
> controllers, these properties were moved to the Root Port node in the
> devicetree bindings.
>
> Commit 960609b22be5 ("arm64: dts: qcom: hamoa: Move PHY, PERST, and Wake
> GPIOs to PCIe port nodes and add port Nodes for all PCIe ports")
> initiated this transition for the Hamoa platform by moving the PHY
> property to the Root Port node in hamoa.dtsi. However, it only updated
> some platform specific DTS files for PERST# and WAKE#, leaving others in
> a "mixed" binding state.
>
> [...]
Applied, thanks!
[1/1] arm64: dts: qcom: hamoa: Fix incomplete Root Port property migration
commit: 11b72b1ca9891c77bc876ef9fc39d6825847ffee
Best regards,
--
Bjorn Andersson <andersson@kernel.org>
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v4 1/2] dt-bindings: phy: qcom-edp: Add reference clock for sa8775p eDP PHY
From: Bjorn Andersson @ 2026-03-30 14:28 UTC (permalink / raw)
To: Ritesh Kumar
Cc: robin.clark, lumag, abhinav.kumar, sean, marijn.suijten,
maarten.lankhorst, mripard, tzimmermann, airlied, simona, robh,
krzk+dt, conor+dt, quic_mahap, konradybcio, mani, James.Bottomley,
martin.petersen, vkoul, kishon, cros-qcom-dts-watchers, linux-phy,
linux-arm-msm, dri-devel, freedreno, devicetree, linux-kernel,
linux-scsi, quic_vproddut
In-Reply-To: <c77ff64f-57d1-4495-bfbd-986644aad71d@quicinc.com>
On Wed, Mar 11, 2026 at 06:37:31PM +0530, Ritesh Kumar wrote:
>
> On 3/5/2026 12:27 AM, Bjorn Andersson wrote:
> > On Wed, Jan 28, 2026 at 05:18:49PM +0530, Ritesh Kumar wrote:
> > > The initial sa8775p eDP PHY binding contribution missed adding support for
> > > voting on the eDP reference clock. This went unnoticed because the UFS PHY
> > > driver happened to enable the same clock.
> > > > After commit 77d2fa54a945 ("scsi: ufs: qcom : Refactor
> > phy_power_on/off
> > > calls"), the eDP reference clock is no longer kept enabled, which results
> > > in the following PHY power-on failure:
> > > > phy phy-aec2a00.phy.10: phy poweron failed --> -110
> > > > To fix this, explicit voting for the eDP reference clock is
> > required.
> > > This patch adds the eDP reference clock for sa8775p eDP PHY and updates
> > > the corresponding example node.
> > > > Signed-off-by: Ritesh Kumar <quic_riteshk@quicinc.com>
> >
> > Is there any reason why you didn't follow up on this patch Ritesh?
> > Looks like it's ready to be merged.
>
> I was waiting for patch to merge as there is no pending comments.
>
It's been two months now, if you want your patches to be merged please
show that - ask the maintainer for a status update, ask a colleague to
send a reviewed-by...
Perhaps the maintainer lost track of your change?
Perhaps it's not clear that the change "need" an Ack from e.g. Dmitry
and then it should be merged by Vinod? Because you're changing two
different subsystems but leave it up to the maintainers to figure out
how to deal with this...
Either way, show that you want this to be merged, don't just wait until
the situation resolves itself.
Regards,
Bjorn
> > Reviewed-by: Bjorn Andersson <andersson@kernel.org>
> >
> > Regards,
> > Bjorn
> >
> > > ---
> > > .../devicetree/bindings/display/msm/qcom,sa8775p-mdss.yaml | 6 ++++--
> > > Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml | 1 +
> > > 2 files changed, 5 insertions(+), 2 deletions(-)
> > > > diff --git
> > a/Documentation/devicetree/bindings/display/msm/qcom,sa8775p-mdss.yaml
> > b/Documentation/devicetree/bindings/display/msm/qcom,sa8775p-mdss.yaml
> > > index e2730a2f25cf..6c827cf9692b 100644
> > > --- a/Documentation/devicetree/bindings/display/msm/qcom,sa8775p-mdss.yaml
> > > +++ b/Documentation/devicetree/bindings/display/msm/qcom,sa8775p-mdss.yaml
> > > @@ -200,9 +200,11 @@ examples:
> > > <0x0aec2000 0x1c8>;
> > > > clocks = <&dispcc0 MDSS_DISP_CC_MDSS_DPTX0_AUX_CLK>,
> > > - <&dispcc0 MDSS_DISP_CC_MDSS_AHB_CLK>;
> > > + <&dispcc0 MDSS_DISP_CC_MDSS_AHB_CLK>,
> > > + <&gcc GCC_EDP_REF_CLKREF_EN>;
> > > clock-names = "aux",
> > > - "cfg_ahb";
> > > + "cfg_ahb",
> > > + "ref";
> > > > #clock-cells = <1>;
> > > #phy-cells = <0>;
> > > diff --git a/Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml
> > > index 4a1daae3d8d4..0bf8bf4f66ac 100644
> > > --- a/Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml
> > > +++ b/Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml
> > > @@ -74,6 +74,7 @@ allOf:
> > > compatible:
> > > enum:
> > > - qcom,glymur-dp-phy
> > > + - qcom,sa8775p-edp-phy
> > > - qcom,x1e80100-dp-phy
> > > then:
> > > properties:
> > > -- > 2.34.1
> > >
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v6 phy-next 03/28] usb: add missing headers transitively included by <linux/phy/phy.h>
From: Greg Kroah-Hartman @ 2026-03-30 13:19 UTC (permalink / raw)
To: Vladimir Oltean
Cc: linux-phy, Vinod Koul, Neil Armstrong, dri-devel, freedreno,
linux-arm-kernel, linux-arm-msm, linux-can, linux-gpio, linux-ide,
linux-kernel, linux-media, linux-pci, linux-renesas-soc,
linux-riscv, linux-rockchip, linux-samsung-soc, linux-scsi,
linux-sunxi, linux-tegra, linux-usb, netdev, spacemit,
UNGLinuxDriver, Thinh Nguyen, Peter Chen, Frank Li, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam
In-Reply-To: <20260327184706.1600329-4-vladimir.oltean@nxp.com>
On Fri, Mar 27, 2026 at 08:46:41PM +0200, Vladimir Oltean wrote:
> The chipidea ci_hdrc_imx driver uses regulator consumer API like
> regulator_enable() but does not include <linux/regulator/consumer.h>.
>
> The core USB HCD driver calls invalidate_kernel_vmap_range() and
> flush_kernel_vmap_range(), but does not include <linux/highmem.h>.
>
> The DWC3 gadget driver calls:
> - device_property_present()
> - device_property_count_u8()
> - device_property_read_u8_array()
> but does not include <linux/property.h>
>
> The dwc3-generic-plat driver uses of_device_get_match_data() but does
> not include <linux/of.h>.
>
> In all these cases, the necessary includes were still provided somehow,
> directly or indirectly, through <linux/phy/phy.h>. The latter header
> wants to drop those includes, so fill in the required headers to avoid
> any breakage.
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> Acked-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com> # dwc3
> ---
> Cc: Peter Chen <peter.chen@kernel.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Frank Li <Frank.Li@nxp.com>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
>
> v2->v6: none
> v1->v2: collect tag
> ---
> drivers/usb/chipidea/ci_hdrc_imx.c | 1 +
> drivers/usb/core/hcd.c | 1 +
> drivers/usb/dwc3/dwc3-generic-plat.c | 1 +
> drivers/usb/dwc3/gadget.c | 1 +
> 4 files changed, 4 insertions(+)
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH] dt-bindings: phy: qcom,sc8280xp-qmp-usb43dp-phy: Add Eliza QMP PHY
From: Abel Vesa @ 2026-03-30 13:05 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: linux-arm-msm, linux-phy, devicetree, linux-kernel
In-Reply-To: <20260330-eliza-phy-usb-dp-combo-v1-1-2ec11e793a08@oss.qualcomm.com>
On 26-03-30 15:19:12, Abel Vesa wrote:
> The QMP combo PHY found on Eliza SoC is fully compatible with the one
> found on SM8650.
>
> So document its compatible string and use the SM8650 as fallback.
>
> Signed-off-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
Please ignore this one.
It has been sent already here:
https://lore.kernel.org/all/20260330-eliza-phy-usb-dp-combo-v1-1-2ec11e793a08@oss.qualcomm.com/
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* [PATCH] dt-bindings: phy: qcom,sc8280xp-qmp-usb43dp-phy: Add Eliza QMP PHY
From: Abel Vesa @ 2026-03-30 12:19 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: linux-arm-msm, linux-phy, devicetree, linux-kernel, Abel Vesa
The QMP combo PHY found on Eliza SoC is fully compatible with the one
found on SM8650.
So document its compatible string and use the SM8650 as fallback.
Signed-off-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
---
.../devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml | 4 ++++
1 file changed, 4 insertions(+)
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..4eff92343ce4 100644
--- a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml
@@ -16,6 +16,10 @@ description:
properties:
compatible:
oneOf:
+ - items:
+ - enum:
+ - qcom,eliza-qmp-usb3-dp-phy
+ - const: qcom,sm8650-qmp-usb3-dp-phy
- items:
- enum:
- qcom,kaanapali-qmp-usb3-dp-phy
---
base-commit: 3b058d1aeeeff27a7289529c4944291613b364e9
change-id: 20260327-eliza-phy-usb-dp-combo-5a117520ac5b
Best regards,
--
Abel Vesa <abel.vesa@oss.qualcomm.com>
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* Re: [PATCH v5 1/2] dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema
From: Bryan O'Donoghue @ 2026-03-30 12:03 UTC (permalink / raw)
To: Dmitry Baryshkov, Konrad Dybcio
Cc: Neil Armstrong, Bryan O'Donoghue, Vinod Koul,
Kishon Vijay Abraham I, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Vladimir Zapolskiy, linux-arm-msm, linux-phy,
linux-media, devicetree, linux-kernel
In-Reply-To: <gcu6qzsoff2fox3nbytegyikajn65ae4ekowcn7bs4x2k53vao@rykmxndzzhlu>
On 30/03/2026 12:49, Dmitry Baryshkov wrote:
>> // determined at runtime
>> csid_n: csid@1000000 {
>> phys = <&csiphy_0>,
>> [...]
>> <&csiphy_n-1>;
>> };
> I think the bigger problem is:
>
> &csid_L: {
> phys = <&csiphy_M>;
> };
>
> &csid_N: {
> phys = <&csiphy_M>;
> };
>
> aka split mode.
Depends on how you model it.
It feels like a philosophical as opposed to an engineering debate in a way.
The CSIPHY block is defined as one thing - is split mode one PHY or two ?
You could argue it either way BUT one strong argument for it being one
PHY is - voltage rails, input clocks etc power the block.
Sure there is an esoteric mode called split or combo mode but the
hardware block itself the thing we usually call the PHY lives as a block
diagram as a discreet entity.
So I think split-mode really is more like
&csid_L: {
phys = <&csiphy_0 DPHY>, <&csiphy_1 CPHY>;
};
&csid_M: {
phys = <&csiphy_0 DPHY>, <&csiphy_1 CPHY>;
};
Recall the debate about a mux is because CSID may connect to any CSIPHY.
csiphy_0: {
clocks = <&camcc CAM_CC_CSIPHY0_CLK>,
<&camcc CAM_CC_CSI0PHYTIMER_CLK>;
clock-names = "core",
"timer";
operating-points-v2 = <&csiphy_opp_table>;
interrupts = <GIC_SPI 1 IRQ_TYPE_EDGE_RISING>;
power-domains = <&rpmhpd RPMHPD_MX>,
<&rpmhpd RPMHPD_MMCX>;
power-domain-names = "mx",
"mmcx";
vdda-0p9-supply = <&vreg_xyz_0p8>;
vdda-1p2-supply = <&vreg_qrs_1p2>;
ports {
port@0{};
port@1{};
};
};
csiphy_N: {
clocks = <&camcc CAM_CC_CSIPHY1_CLK>,
<&camcc CAM_CC_CSI1PHYTIMER_CLK>;
clock-names = "core",
"timer";
operating-points-v2 = <&csiphy_opp_table>;
interrupts = <GIC_SPI 2 IRQ_TYPE_EDGE_RISING>;
power-domains = <&rpmhpd RPMHPD_MX>,
<&rpmhpd RPMHPD_MMCX>;
power-domain-names = "mx",
"mmcx";
vdda-0p9-supply = <&vreg_abc_0p8>;
vdda-1p2-supply = <&vreg_def_1p2>;
ports {
port@0{};
};
};
IMO split mode is a special mode of that hardware block, not two
individual PHYs.
---
bod
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH phy-next 0/3] Lynx 28G: better init(), exit(), power_on(), power_off()
From: Vladimir Oltean @ 2026-03-30 11:46 UTC (permalink / raw)
To: linux-phy
Cc: netdev, Ioana Ciornei, Vinod Koul, Neil Armstrong, Josua Mayer,
linux-kernel
In-Reply-To: <20260321011451.1557091-1-vladimir.oltean@nxp.com>
On Sat, Mar 21, 2026 at 03:14:48AM +0200, Vladimir Oltean wrote:
> This is a set of 3 improvements to the 28G Lynx SerDes driver as found
> on NXP Layerscape:
> - avoid kernel hangs if lane resets/halts fail due to other bugs
> - actually have phy_power_down() cut power from lanes, not just halt them
> - allow consumers to call phy_exit(), to balance the phy->init_count
>
> Especially change 3 will allow further development of the dpaa2-eth
> consumer. To permit phy_exit() and other patches in net-next to be
> submitted in this development cycle without functionally breaking
> networking, please apply this change on top of v7.0-rc1 and provide
> it as a stable tag to be pulled in netdev.
>
> Vladimir Oltean (3):
> phy: lynx-28g: use timeouts when waiting for lane halt and reset
> phy: lynx-28g: truly power the lanes up or down
> phy: lynx-28g: implement phy_exit() operation
>
> drivers/phy/freescale/phy-fsl-lynx-28g.c | 190 +++++++++++++++++++----
> 1 file changed, 157 insertions(+), 33 deletions(-)
>
> --
> 2.34.1
>
I've analyzed the AI review output from
https://sashiko.dev/#/patchset/20260321011451.1557091-1-vladimir.oltean%40nxp.com
and I don't consider anything from it to be actionable.
> > @@ -1065,7 +1097,7 @@ static void lynx_28g_cdr_lock_check(struct work_struct *work)
> > struct lynx_28g_priv *priv = work_to_lynx(work);
> > struct lynx_28g_lane *lane;
> > u32 rrstctl;
> > - int i;
> > + int err, i;
> >
> > for (i = 0; i < LYNX_28G_NUM_LANE; i++) {
> > lane = &priv->lane[i];
> Can this dereference a NULL pointer for unpopulated lanes?
> If the device tree contains available child nodes, lynx_28g_probe_lane()
> is only called for lanes explicitly defined by the "reg" property, leaving
> lane->phy as NULL for the rest.
> Since the loop unconditionally iterates over all LYNX_28G_NUM_LANE (8) lanes:
> for (i = 0; i < LYNX_28G_NUM_LANE; i++) {
> lane = &priv->lane[i];
> mutex_lock(&lane->phy->mutex);
> if (!lane->init || !lane->powered_up) {
> Would it crash on mutex_lock(&lane->phy->mutex) before checking
> if the lane is initialized?
This is fixed as of commit
https://git.kernel.org/pub/scm/linux/kernel/git/phy/linux-phy.git/commit/?id=a258d843a3e4cb687da19437f8f81fee55ad7d35
not present in linux-phy/next.
> > @@ -1081,9 +1113,17 @@ static void lynx_28g_cdr_lock_check(struct work_struct *work)
> > if (!(rrstctl & LNaRRSTCTL_CDR_LOCK)) {
> > lynx_28g_lane_rmw(lane, LNaRRSTCTL, LNaRRSTCTL_RST_REQ,
> > LNaRRSTCTL_RST_REQ);
> > - do {
> > - rrstctl = lynx_28g_lane_read(lane, LNaRRSTCTL);
> > - } while (!(rrstctl & LNaRRSTCTL_RST_DONE));
> > +
> > + err = read_poll_timeout(lynx_28g_lane_read, rrstctl,
> > + !!(rrstctl & LNaRRSTCTL_RST_DONE),
> > + LYNX_28G_LANE_RESET_SLEEP_US,
> > + LYNX_28G_LANE_RESET_TIMEOUT_US,
> > + false, lane, LNaRRSTCTL);
> > + if (err) {
> > + dev_warn_once(&lane->phy->dev,
> > + "Lane %c receiver reset failed: %pe\n",
> > + 'A' + lane->id, ERR_PTR(err));
> > + }
> Since dev_warn_once() uses a static boolean flag that is evaluated per call
> site rather than per lane, will this mask errors for subsequent lanes?
> If a receiver reset times out on any lane, the warning is printed and the
> static flag is set to true. If any other lane subsequently fails the reset,
> the warning will be silently suppressed.
> Would a per-lane state tracking mechanism or dev_warn_ratelimited() be more
> appropriate here to ensure multi-lane failures can be diagnosed?
Unjustified complexity for an error case that should never really trigger
in production environments.
> > diff --git a/drivers/phy/freescale/phy-fsl-lynx-28g.c b/drivers/phy/freescale/phy-fsl-lynx-28g.c
> > --- a/drivers/phy/freescale/phy-fsl-lynx-28g.c
> > +++ b/drivers/phy/freescale/phy-fsl-lynx-28g.c
> [ ... ]
> > +static int lynx_28g_power_on(struct phy *phy)
> > +{
> > + struct lynx_28g_lane *lane = phy_get_drvdata(phy);
> > + int err;
> > +
> > + if (lane->powered_up)
> > + return 0;
> > +
> > + /* Power up the RX and TX portions of the lane */
> > + lynx_28g_lane_rmw(lane, LNaRRSTCTL, 0, LNaRRSTCTL_DIS);
> > + lynx_28g_lane_rmw(lane, LNaTRSTCTL, 0, LNaTRSTCTL_DIS);
> > +
> > + err = lynx_28g_lane_reset(phy);
> > + if (err)
> > + return err;
> If lynx_28g_lane_reset() returns an error, the function returns immediately
> without setting lane->powered_up to true. Does this leave the hardware
> powered up while the software state considers it powered off?
> If a subsequent cleanup calls lynx_28g_power_off(), it will check
> if (!lane->powered_up) and return 0 immediately, leaving the DIS bits
> cleared permanently. Should the error path restore the DIS bits before
> returning?
Same. If lynx_28g_lane_reset() times out, there is really no way to keep
the software state in sync no matter whether lane->powered_up is true or
false. The lane is broken regardless.
> > @@ -1007,7 +1074,7 @@ static int lynx_28g_set_mode(struct phy *phy, enum phy_mode mode, int submode)
> >
> > out:
> > if (powered_up)
> > - lynx_28g_power_on(phy);
> > + lynx_28g_lane_reset(phy);
> >
> > return err;
> > }
> If lynx_28g_lane_reset() fails during the success path where err is 0,
> the return value of the reset call is discarded and the function still
> returns 0.
> Can this hide a hardware failure from the caller, misleading the MAC driver
> into thinking the link is successfully brought back up? Should err be
> updated with the return value of lynx_28g_lane_reset() here if err is
> currently 0?
As explained in the commit message, handling and propagating the
lynx_28g_lane_reset() error to the caller isn't really any better.
The caller would think that lynx_28g_set_mode() failed, thus that the
mode wasn't changed. But it was changed.
I do not plan to send a v2 to address any of the change suggestions above.
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v5 1/2] dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema
From: Dmitry Baryshkov @ 2026-03-30 11:49 UTC (permalink / raw)
To: Konrad Dybcio
Cc: Bryan O'Donoghue, Neil Armstrong, Bryan O'Donoghue,
Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Vladimir Zapolskiy,
linux-arm-msm, linux-phy, linux-media, devicetree, linux-kernel
In-Reply-To: <6d5ebab6-3c4e-4a1c-9578-6e926cbd96de@oss.qualcomm.com>
On Mon, Mar 30, 2026 at 01:34:57PM +0200, Konrad Dybcio wrote:
> On 3/30/26 11:25 AM, Bryan O'Donoghue wrote:
> > On 30/03/2026 10:17, Neil Armstrong wrote:
> >> On 3/30/26 11:02, Bryan O'Donoghue wrote:
> >>> On 30/03/2026 08:49, Neil Armstrong wrote:
> >>>> On 3/27/26 18:42, Bryan O'Donoghue wrote:
> >>>>> On 27/03/2026 15:28, Neil Armstrong wrote:
> >>>>>>> To be frankly honest you can make an argument for it either way. However my honestly held position is analysing other upstream implementations connecting to the PHY means we can't make the PHY device a drivers/phy device - it would have to be a V4L2 device and then for me the question is why is that even required ?
> >>>>>>
> >>>>>> This is plain wrong, DT definition is different from software implementation, you can do whatever you want if you describe HW accurately.
> >>>>>
> >>>>> I'm not sure what point it is you are trying to make here. Are you trying to say drivers/phy is OK with you but you want an endpoint ? If so, please just say so.
> >>>>
> >>>> I'm against using the "phys = <>" property in the CAMSS to reference the PHYs, a "PHY" in the classic terminology is tied to a single consumer, and if it can be shared to multiple consumer you must model a mux or whatever in the middle.
> >>>
> >>> The CSIPHY-to-CSID routing is runtime-configurable and is already managed by the media controller framework.
> >>
> >> This is not compatible with the PHY bindings if you don't have a defined MUX device in the middle, it's wrong. You're hiding the muxing details in the CAMSS blob node.
> >>
> >>>
> >>> DT describes static hardware connections. The dynamic mux is a software concern, not a hardware description concern.
> >>
> >> DT must describe the possible interconnections between the nodes, if a PHY can be used by multiple hardware components, it must be described.
> >
> > But right now the CAMSS block is described as a single block. There is no CSID device in the kernel _yet_.
> >
> > When we break CSID into its own block then fine, lets have a debate about a mux then but right now the "nodes" are CAMSS[MONOLITH] <=> CSIPHY there is no DT CSID device to model this to.
>
> Let's take a step back - since any CSIPHY can feed into any CSID (at runtime),
> the resulting nodes would either look like:
>
> // hardcoded, m may != n
> csid_n: csid@1000000 {
> phys = <&csiphy_m>;
> };
>
> or
>
> // determined at runtime
> csid_n: csid@1000000 {
> phys = <&csiphy_0>,
> [...]
> <&csiphy_n-1>;
> };
I think the bigger problem is:
&csid_L: {
phys = <&csiphy_M>;
};
&csid_N: {
phys = <&csiphy_M>;
};
aka split mode.
>
> or we could store them once, centrally, in the "CAMSS_TOP" node and
> pass handles around as necessary:
>
> // camss "catalog/manager" driver/library provides CSIDn with PHYm
> camss: camss@10000000 {
> phys = <&csiphy_0>,
> [...]
> <&csiphy_n-1>;
>
> csid_n: csid@1000 {
> // no PHY references
> };
> };
>
> Konrad
--
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 v5 1/2] dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema
From: Bryan O'Donoghue @ 2026-03-30 11:41 UTC (permalink / raw)
To: Konrad Dybcio, Neil Armstrong, Bryan O'Donoghue, Vinod Koul,
Kishon Vijay Abraham I, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: Vladimir Zapolskiy, linux-arm-msm, linux-phy, linux-media,
devicetree, linux-kernel
In-Reply-To: <6d5ebab6-3c4e-4a1c-9578-6e926cbd96de@oss.qualcomm.com>
On 30/03/2026 12:34, Konrad Dybcio wrote:
> Let's take a step back - since any CSIPHY can feed into any CSID (at runtime),
> the resulting nodes would either look like:
>
> // hardcoded, m may != n
> csid_n: csid@1000000 {
> phys = <&csiphy_m>;
> };
>
Well that would be wrong they can connect to any CSID. We'd be churning
the user-space ABI and imposing an artificial constraint on what the hw
can do.
>
> // determined at runtime
> csid_n: csid@1000000 {
> phys = <&csiphy_0>,
> [...]
> <&csiphy_n-1>;
> };
This I think works well and actually maps to what the hardware can do.
This would be where to talk more about Neil's mux.
>
> or we could store them once, centrally, in the "CAMSS_TOP" node and
> pass handles around as necessary:
>
> // camss "catalog/manager" driver/library provides CSIDn with PHYm
> camss: camss@10000000 {
> phys = <&csiphy_0>,
> [...]
> <&csiphy_n-1>;
>
> csid_n: csid@1000 {
> // no PHY references
> };
> };
That could work too.
Either way I think this is to be hashed out when doing CSID.
---
bod
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v5 1/2] dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema
From: Konrad Dybcio @ 2026-03-30 11:34 UTC (permalink / raw)
To: Bryan O'Donoghue, Neil Armstrong, Bryan O'Donoghue,
Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: Vladimir Zapolskiy, linux-arm-msm, linux-phy, linux-media,
devicetree, linux-kernel
In-Reply-To: <0101d8bc-1ae8-475e-bb9e-cc1e16db87ec@kernel.org>
On 3/30/26 11:25 AM, Bryan O'Donoghue wrote:
> On 30/03/2026 10:17, Neil Armstrong wrote:
>> On 3/30/26 11:02, Bryan O'Donoghue wrote:
>>> On 30/03/2026 08:49, Neil Armstrong wrote:
>>>> On 3/27/26 18:42, Bryan O'Donoghue wrote:
>>>>> On 27/03/2026 15:28, Neil Armstrong wrote:
>>>>>>> To be frankly honest you can make an argument for it either way. However my honestly held position is analysing other upstream implementations connecting to the PHY means we can't make the PHY device a drivers/phy device - it would have to be a V4L2 device and then for me the question is why is that even required ?
>>>>>>
>>>>>> This is plain wrong, DT definition is different from software implementation, you can do whatever you want if you describe HW accurately.
>>>>>
>>>>> I'm not sure what point it is you are trying to make here. Are you trying to say drivers/phy is OK with you but you want an endpoint ? If so, please just say so.
>>>>
>>>> I'm against using the "phys = <>" property in the CAMSS to reference the PHYs, a "PHY" in the classic terminology is tied to a single consumer, and if it can be shared to multiple consumer you must model a mux or whatever in the middle.
>>>
>>> The CSIPHY-to-CSID routing is runtime-configurable and is already managed by the media controller framework.
>>
>> This is not compatible with the PHY bindings if you don't have a defined MUX device in the middle, it's wrong. You're hiding the muxing details in the CAMSS blob node.
>>
>>>
>>> DT describes static hardware connections. The dynamic mux is a software concern, not a hardware description concern.
>>
>> DT must describe the possible interconnections between the nodes, if a PHY can be used by multiple hardware components, it must be described.
>
> But right now the CAMSS block is described as a single block. There is no CSID device in the kernel _yet_.
>
> When we break CSID into its own block then fine, lets have a debate about a mux then but right now the "nodes" are CAMSS[MONOLITH] <=> CSIPHY there is no DT CSID device to model this to.
Let's take a step back - since any CSIPHY can feed into any CSID (at runtime),
the resulting nodes would either look like:
// hardcoded, m may != n
csid_n: csid@1000000 {
phys = <&csiphy_m>;
};
or
// determined at runtime
csid_n: csid@1000000 {
phys = <&csiphy_0>,
[...]
<&csiphy_n-1>;
};
or we could store them once, centrally, in the "CAMSS_TOP" node and
pass handles around as necessary:
// camss "catalog/manager" driver/library provides CSIDn with PHYm
camss: camss@10000000 {
phys = <&csiphy_0>,
[...]
<&csiphy_n-1>;
csid_n: csid@1000 {
// no PHY references
};
};
Konrad
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v2 1/1] arm64: dts: qcom: hamoa: Fix incomplete Root Port property migration
From: Manivannan Sadhasivam @ 2026-03-30 11:26 UTC (permalink / raw)
To: andersson, Ziyue Zhang
Cc: konradybcio, robh, krzk+dt, conor+dt, jingoohan1, lpieralisi,
kwilczynski, bhelgaas, johan+linaro, vkoul, kishon,
neil.armstrong, abel.vesa, kw, linux-arm-msm, devicetree,
linux-kernel, linux-pci, linux-phy, qiang.yu, quic_krichai,
quic_vbadigan
In-Reply-To: <20260330020934.3501247-1-ziyue.zhang@oss.qualcomm.com>
On Mon, Mar 30, 2026 at 10:09:34AM +0800, Ziyue Zhang wrote:
> Historically, the Qualcomm PCIe controller node (Host bridge) described
> all Root Port properties, such as PHY, PERST#, and WAKE#. But to provide
> a more accurate hardware description and to support future multi-Root Port
> controllers, these properties were moved to the Root Port node in the
> devicetree bindings.
>
> Commit 960609b22be5 ("arm64: dts: qcom: hamoa: Move PHY, PERST, and Wake
> GPIOs to PCIe port nodes and add port Nodes for all PCIe ports")
> initiated this transition for the Hamoa platform by moving the PHY
> property to the Root Port node in hamoa.dtsi. However, it only updated
> some platform specific DTS files for PERST# and WAKE#, leaving others in
> a "mixed" binding state.
>
> While the PCIe controller driver supports both legacy and Root Port
> bindings, It cannot correctly handle a mix of both. In these cases, the
> driver parses the PHY from the Root Port node, but fails to find the
> PERST# property (which it then assumes is not present, as it is optional).
> Consequently, the controller probe succeeds, but PERST# remains
> uncontrolled, preventing PCIe endpoints from functioning.
>
> So, fix the incomplete migration by moving the PERST# and WAKE# properties
> from the controller node to the Root Port node in all remaining Hamoa
> platform DTS files.
>
> Fixes: 960609b22be5 ("arm64: dts: qcom: hamoa: Move PHY, PERST, and Wake GPIOs to PCIe port nodes and add port Nodes for all PCIe ports")
> Signed-off-by: Ziyue Zhang <ziyue.zhang@oss.qualcomm.com>
Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
FWIW: Bjorn, this is an -rc candidate.
- Mani
> ---
> .../boot/dts/qcom/x1-asus-zenbook-a14.dtsi | 16 ++++++++-----
> arch/arm64/boot/dts/qcom/x1-crd.dtsi | 24 ++++++++++++-------
> arch/arm64/boot/dts/qcom/x1-dell-thena.dtsi | 14 ++++++-----
> .../boot/dts/qcom/x1-hp-omnibook-x14.dtsi | 14 ++++++-----
> .../boot/dts/qcom/x1-microsoft-denali.dtsi | 8 ++++---
> .../dts/qcom/x1e80100-lenovo-yoga-slim7x.dts | 6 ++---
> .../qcom/x1e80100-medion-sprchrgd-14-s1.dts | 14 +++++------
> .../dts/qcom/x1p42100-lenovo-thinkbook-16.dts | 14 ++++++-----
> 8 files changed, 64 insertions(+), 46 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/x1-asus-zenbook-a14.dtsi b/arch/arm64/boot/dts/qcom/x1-asus-zenbook-a14.dtsi
> index cd062f844b2d..66d566808f58 100644
> --- a/arch/arm64/boot/dts/qcom/x1-asus-zenbook-a14.dtsi
> +++ b/arch/arm64/boot/dts/qcom/x1-asus-zenbook-a14.dtsi
> @@ -1079,9 +1079,6 @@ &mdss_dp3_phy {
> };
>
> &pcie4 {
> - perst-gpios = <&tlmm 146 GPIO_ACTIVE_LOW>;
> - wake-gpios = <&tlmm 148 GPIO_ACTIVE_LOW>;
> -
> pinctrl-0 = <&pcie4_default>;
> pinctrl-names = "default";
>
> @@ -1095,10 +1092,12 @@ &pcie4_phy {
> status = "okay";
> };
>
> -&pcie6a {
> - perst-gpios = <&tlmm 152 GPIO_ACTIVE_LOW>;
> - wake-gpios = <&tlmm 154 GPIO_ACTIVE_LOW>;
> +&pcie4_port0 {
> + reset-gpios = <&tlmm 146 GPIO_ACTIVE_LOW>;
> + wake-gpios = <&tlmm 148 GPIO_ACTIVE_LOW>;
> +};
>
> +&pcie6a {
> vddpe-3v3-supply = <&vreg_nvme>;
>
> pinctrl-0 = <&pcie6a_default>;
> @@ -1114,6 +1113,11 @@ &pcie6a_phy {
> status = "okay";
> };
>
> +&pcie6a_port0 {
> + reset-gpios = <&tlmm 152 GPIO_ACTIVE_LOW>;
> + wake-gpios = <&tlmm 154 GPIO_ACTIVE_LOW>;
> +};
> +
> &pm8550_gpios {
> rtmr0_default: rtmr0-reset-n-active-state {
> pins = "gpio10";
> diff --git a/arch/arm64/boot/dts/qcom/x1-crd.dtsi b/arch/arm64/boot/dts/qcom/x1-crd.dtsi
> index 485dcd946757..a9c5c523575e 100644
> --- a/arch/arm64/boot/dts/qcom/x1-crd.dtsi
> +++ b/arch/arm64/boot/dts/qcom/x1-crd.dtsi
> @@ -1248,15 +1248,17 @@ &mdss_dp3_phy {
> };
>
> &pcie4 {
> - perst-gpios = <&tlmm 146 GPIO_ACTIVE_LOW>;
> - wake-gpios = <&tlmm 148 GPIO_ACTIVE_LOW>;
> -
> pinctrl-0 = <&pcie4_default>;
> pinctrl-names = "default";
>
> status = "okay";
> };
>
> +&pcie4_port0 {
> + reset-gpios = <&tlmm 146 GPIO_ACTIVE_LOW>;
> + wake-gpios = <&tlmm 148 GPIO_ACTIVE_LOW>;
> +};
> +
> &pcie4_phy {
> vdda-phy-supply = <&vreg_l3i_0p8>;
> vdda-pll-supply = <&vreg_l3e_1p2>;
> @@ -1265,9 +1267,6 @@ &pcie4_phy {
> };
>
> &pcie5 {
> - perst-gpios = <&tlmm 149 GPIO_ACTIVE_LOW>;
> - wake-gpios = <&tlmm 151 GPIO_ACTIVE_LOW>;
> -
> vddpe-3v3-supply = <&vreg_wwan>;
>
> pinctrl-0 = <&pcie5_default>;
> @@ -1283,10 +1282,12 @@ &pcie5_phy {
> status = "okay";
> };
>
> -&pcie6a {
> - perst-gpios = <&tlmm 152 GPIO_ACTIVE_LOW>;
> - wake-gpios = <&tlmm 154 GPIO_ACTIVE_LOW>;
> +&pcie5_port0 {
> + reset-gpios = <&tlmm 149 GPIO_ACTIVE_LOW>;
> + wake-gpios = <&tlmm 151 GPIO_ACTIVE_LOW>;
> +};
>
> +&pcie6a {
> vddpe-3v3-supply = <&vreg_nvme>;
>
> pinctrl-names = "default";
> @@ -1302,6 +1303,11 @@ &pcie6a_phy {
> status = "okay";
> };
>
> +&pcie6a_port0 {
> + reset-gpios = <&tlmm 152 GPIO_ACTIVE_LOW>;
> + wake-gpios = <&tlmm 154 GPIO_ACTIVE_LOW>;
> +};
> +
> &pm8550_gpios {
> kypd_vol_up_n: kypd-vol-up-n-state {
> pins = "gpio6";
> diff --git a/arch/arm64/boot/dts/qcom/x1-dell-thena.dtsi b/arch/arm64/boot/dts/qcom/x1-dell-thena.dtsi
> index 343844cc62f2..0d9a324cc6cc 100644
> --- a/arch/arm64/boot/dts/qcom/x1-dell-thena.dtsi
> +++ b/arch/arm64/boot/dts/qcom/x1-dell-thena.dtsi
> @@ -1081,9 +1081,6 @@ &mdss_dp3_phy {
> };
>
> &pcie4 {
> - perst-gpios = <&tlmm 146 GPIO_ACTIVE_LOW>;
> - wake-gpios = <&tlmm 148 GPIO_ACTIVE_LOW>;
> -
> pinctrl-0 = <&pcie4_default>;
> pinctrl-names = "default";
>
> @@ -1098,6 +1095,9 @@ &pcie4_phy {
> };
>
> &pcie4_port0 {
> + reset-gpios = <&tlmm 146 GPIO_ACTIVE_LOW>;
> + wake-gpios = <&tlmm 148 GPIO_ACTIVE_LOW>;
> +
> wifi@0 {
> compatible = "pci17cb,1107";
> reg = <0x10000 0x0 0x0 0x0 0x0>;
> @@ -1115,9 +1115,6 @@ wifi@0 {
> };
>
> &pcie6a {
> - perst-gpios = <&tlmm 152 GPIO_ACTIVE_LOW>;
> - wake-gpios = <&tlmm 154 GPIO_ACTIVE_LOW>;
> -
> vddpe-3v3-supply = <&vreg_nvme>;
>
> pinctrl-0 = <&pcie6a_default>;
> @@ -1126,6 +1123,11 @@ &pcie6a {
> status = "okay";
> };
>
> +&pcie6a_port0 {
> + reset-gpios = <&tlmm 152 GPIO_ACTIVE_LOW>;
> + wake-gpios = <&tlmm 154 GPIO_ACTIVE_LOW>;
> +};
> +
> &pcie6a_phy {
> vdda-phy-supply = <&vreg_l1d_0p8>;
> vdda-pll-supply = <&vreg_l2j_1p2>;
> diff --git a/arch/arm64/boot/dts/qcom/x1-hp-omnibook-x14.dtsi b/arch/arm64/boot/dts/qcom/x1-hp-omnibook-x14.dtsi
> index 16437139d336..b773a4976d1b 100644
> --- a/arch/arm64/boot/dts/qcom/x1-hp-omnibook-x14.dtsi
> +++ b/arch/arm64/boot/dts/qcom/x1-hp-omnibook-x14.dtsi
> @@ -1065,9 +1065,6 @@ &mdss_dp3_phy {
> };
>
> &pcie4 {
> - perst-gpios = <&tlmm 146 GPIO_ACTIVE_LOW>;
> - wake-gpios = <&tlmm 148 GPIO_ACTIVE_LOW>;
> -
> pinctrl-0 = <&pcie4_default>;
> pinctrl-names = "default";
>
> @@ -1082,6 +1079,9 @@ &pcie4_phy {
> };
>
> &pcie4_port0 {
> + reset-gpios = <&tlmm 146 GPIO_ACTIVE_LOW>;
> + wake-gpios = <&tlmm 148 GPIO_ACTIVE_LOW>;
> +
> wifi@0 {
> compatible = "pci17cb,1107";
> reg = <0x10000 0x0 0x0 0x0 0x0>;
> @@ -1099,9 +1099,6 @@ wifi@0 {
> };
>
> &pcie6a {
> - perst-gpios = <&tlmm 152 GPIO_ACTIVE_LOW>;
> - wake-gpios = <&tlmm 154 GPIO_ACTIVE_LOW>;
> -
> vddpe-3v3-supply = <&vreg_nvme>;
>
> pinctrl-0 = <&pcie6a_default>;
> @@ -1110,6 +1107,11 @@ &pcie6a {
> status = "okay";
> };
>
> +&pcie6a_port0 {
> + reset-gpios = <&tlmm 152 GPIO_ACTIVE_LOW>;
> + wake-gpios = <&tlmm 154 GPIO_ACTIVE_LOW>;
> +};
> +
> &pcie6a_phy {
> vdda-phy-supply = <&vreg_l1d_0p8>;
> vdda-pll-supply = <&vreg_l2j_1p2>;
> diff --git a/arch/arm64/boot/dts/qcom/x1-microsoft-denali.dtsi b/arch/arm64/boot/dts/qcom/x1-microsoft-denali.dtsi
> index 6ab595b6ea30..dd2de1f723b0 100644
> --- a/arch/arm64/boot/dts/qcom/x1-microsoft-denali.dtsi
> +++ b/arch/arm64/boot/dts/qcom/x1-microsoft-denali.dtsi
> @@ -964,9 +964,6 @@ wifi@0 {
> };
>
> &pcie6a {
> - perst-gpios = <&tlmm 152 GPIO_ACTIVE_LOW>;
> - wake-gpios = <&tlmm 154 GPIO_ACTIVE_LOW>;
> -
> vddpe-3v3-supply = <&vreg_nvme>;
>
> pinctrl-0 = <&pcie6a_default>;
> @@ -982,6 +979,11 @@ &pcie6a_phy {
> status = "okay";
> };
>
> +&pcie6a_port0 {
> + reset-gpios = <&tlmm 152 GPIO_ACTIVE_LOW>;
> + wake-gpios = <&tlmm 154 GPIO_ACTIVE_LOW>;
> +};
> +
> &pm8550_gpios {
> rtmr0_default: rtmr0-reset-n-active-state {
> pins = "gpio10";
> diff --git a/arch/arm64/boot/dts/qcom/x1e80100-lenovo-yoga-slim7x.dts b/arch/arm64/boot/dts/qcom/x1e80100-lenovo-yoga-slim7x.dts
> index bd0e3009fb41..beb1475d7fa0 100644
> --- a/arch/arm64/boot/dts/qcom/x1e80100-lenovo-yoga-slim7x.dts
> +++ b/arch/arm64/boot/dts/qcom/x1e80100-lenovo-yoga-slim7x.dts
> @@ -1126,9 +1126,6 @@ &mdss_dp3_phy {
> };
>
> &pcie4 {
> - perst-gpios = <&tlmm 146 GPIO_ACTIVE_LOW>;
> - wake-gpios = <&tlmm 148 GPIO_ACTIVE_LOW>;
> -
> pinctrl-0 = <&pcie4_default>;
> pinctrl-names = "default";
>
> @@ -1143,6 +1140,9 @@ &pcie4_phy {
> };
>
> &pcie4_port0 {
> + reset-gpios = <&tlmm 146 GPIO_ACTIVE_LOW>;
> + wake-gpios = <&tlmm 148 GPIO_ACTIVE_LOW>;
> +
> wifi@0 {
> compatible = "pci17cb,1107";
> reg = <0x10000 0x0 0x0 0x0 0x0>;
> diff --git a/arch/arm64/boot/dts/qcom/x1e80100-medion-sprchrgd-14-s1.dts b/arch/arm64/boot/dts/qcom/x1e80100-medion-sprchrgd-14-s1.dts
> index 763efb9e070d..23a298248a29 100644
> --- a/arch/arm64/boot/dts/qcom/x1e80100-medion-sprchrgd-14-s1.dts
> +++ b/arch/arm64/boot/dts/qcom/x1e80100-medion-sprchrgd-14-s1.dts
> @@ -1033,9 +1033,6 @@ &mdss_dp3_phy {
> };
>
> &pcie4 {
> - perst-gpios = <&tlmm 146 GPIO_ACTIVE_LOW>;
> - wake-gpios = <&tlmm 148 GPIO_ACTIVE_LOW>;
> -
> pinctrl-0 = <&pcie4_default>;
> pinctrl-names = "default";
>
> @@ -1050,6 +1047,8 @@ &pcie4_phy {
> };
>
> &pcie4_port0 {
> + reset-gpios = <&tlmm 146 GPIO_ACTIVE_LOW>;
> + wake-gpios = <&tlmm 148 GPIO_ACTIVE_LOW>;
> wifi@0 {
> compatible = "pci17cb,1107";
> reg = <0x10000 0x0 0x0 0x0 0x0>;
> @@ -1067,10 +1066,6 @@ wifi@0 {
> };
>
> &pcie6a {
> - perst-gpios = <&tlmm 152 GPIO_ACTIVE_LOW>;
> -
> - wake-gpios = <&tlmm 154 GPIO_ACTIVE_LOW>;
> -
> vddpe-3v3-supply = <&vreg_nvme>;
>
> pinctrl-0 = <&pcie6a_default>;
> @@ -1086,6 +1081,11 @@ &pcie6a_phy {
> status = "okay";
> };
>
> +&pcie6a_port0 {
> + reset-gpios = <&tlmm 152 GPIO_ACTIVE_LOW>;
> + wake-gpios = <&tlmm 154 GPIO_ACTIVE_LOW>;
> +};
> +
> &pm8550_gpios {
> rtmr0_default: rtmr0-reset-n-active-state {
> pins = "gpio10";
> diff --git a/arch/arm64/boot/dts/qcom/x1p42100-lenovo-thinkbook-16.dts b/arch/arm64/boot/dts/qcom/x1p42100-lenovo-thinkbook-16.dts
> index ab309d547ed5..500809772097 100644
> --- a/arch/arm64/boot/dts/qcom/x1p42100-lenovo-thinkbook-16.dts
> +++ b/arch/arm64/boot/dts/qcom/x1p42100-lenovo-thinkbook-16.dts
> @@ -1131,9 +1131,6 @@ &mdss_dp3_phy {
> };
>
> &pcie4 {
> - perst-gpios = <&tlmm 146 GPIO_ACTIVE_LOW>;
> - wake-gpios = <&tlmm 148 GPIO_ACTIVE_LOW>;
> -
> pinctrl-0 = <&pcie4_default>;
> pinctrl-names = "default";
>
> @@ -1148,6 +1145,9 @@ &pcie4_phy {
> };
>
> &pcie4_port0 {
> + reset-gpios = <&tlmm 146 GPIO_ACTIVE_LOW>;
> + wake-gpios = <&tlmm 148 GPIO_ACTIVE_LOW>;
> +
> wifi@0 {
> compatible = "pci17cb,1107";
> reg = <0x10000 0x0 0x0 0x0 0x0>;
> @@ -1165,9 +1165,6 @@ wifi@0 {
> };
>
> &pcie6a {
> - perst-gpios = <&tlmm 152 GPIO_ACTIVE_LOW>;
> - wake-gpios = <&tlmm 154 GPIO_ACTIVE_LOW>;
> -
> vddpe-3v3-supply = <&vreg_nvme>;
>
> pinctrl-0 = <&pcie6a_default>;
> @@ -1183,6 +1180,11 @@ &pcie6a_phy {
> status = "okay";
> };
>
> +&pcie6a_port0 {
> + reset-gpios = <&tlmm 152 GPIO_ACTIVE_LOW>;
> + wake-gpios = <&tlmm 154 GPIO_ACTIVE_LOW>;
> +};
> +
> &pm8550_pwm {
> status = "okay";
> };
> --
> 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 v2 1/1] arm64: dts: qcom: hamoa: Fix incomplete Root Port property migration
From: Konrad Dybcio @ 2026-03-30 11:21 UTC (permalink / raw)
To: Ziyue Zhang, andersson, konradybcio, robh, krzk+dt, conor+dt,
jingoohan1, mani, lpieralisi, kwilczynski, bhelgaas, johan+linaro,
vkoul, kishon, neil.armstrong, abel.vesa, kw
Cc: linux-arm-msm, devicetree, linux-kernel, linux-pci, linux-phy,
qiang.yu, quic_krichai, quic_vbadigan
In-Reply-To: <20260330020934.3501247-1-ziyue.zhang@oss.qualcomm.com>
On 3/30/26 4:09 AM, Ziyue Zhang wrote:
> Historically, the Qualcomm PCIe controller node (Host bridge) described
> all Root Port properties, such as PHY, PERST#, and WAKE#. But to provide
> a more accurate hardware description and to support future multi-Root Port
> controllers, these properties were moved to the Root Port node in the
> devicetree bindings.
[...]
> --- a/arch/arm64/boot/dts/qcom/x1e80100-medion-sprchrgd-14-s1.dts
> +++ b/arch/arm64/boot/dts/qcom/x1e80100-medion-sprchrgd-14-s1.dts
> @@ -1033,9 +1033,6 @@ &mdss_dp3_phy {
> };
>
> &pcie4 {
> - perst-gpios = <&tlmm 146 GPIO_ACTIVE_LOW>;
> - wake-gpios = <&tlmm 148 GPIO_ACTIVE_LOW>;
> -
> pinctrl-0 = <&pcie4_default>;
> pinctrl-names = "default";
>
> @@ -1050,6 +1047,8 @@ &pcie4_phy {
> };
>
> &pcie4_port0 {
> + reset-gpios = <&tlmm 146 GPIO_ACTIVE_LOW>;
> + wake-gpios = <&tlmm 148 GPIO_ACTIVE_LOW>;
> wifi@0 {
> compatible = "pci17cb,1107";
nit: This single hunk misses a \n before the subnode, but maybe Bjorn
could fix that up while applying?
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH] dt-bindings: phy: qcom,snps-eusb2: Document the Eliza Synopsys eUSB2 PHY
From: Konrad Dybcio @ 2026-03-30 10:40 UTC (permalink / raw)
To: Abel Vesa, Vinod Koul, Neil Armstrong, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Abel Vesa
Cc: linux-arm-msm, linux-phy, devicetree, linux-kernel
In-Reply-To: <20260327-eliza-bindings-phy-eusb2-v1-1-1f8a9ad6a033@oss.qualcomm.com>
On 3/27/26 3:14 PM, Abel Vesa wrote:
> The Synopsys eUSB2 PHY found on the Eliza SoC is fully compatible with the
> one found the SM8550.
>
> So document it by adding the compatible to the list that has the SM8550
> one as fallback.
>
> Signed-off-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v5 1/2] dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema
From: Vladimir Zapolskiy @ 2026-03-30 10:39 UTC (permalink / raw)
To: Bryan O'Donoghue, Neil Armstrong, Konrad Dybcio, Vinod Koul,
Kishon Vijay Abraham I, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: Bryan O'Donoghue, linux-arm-msm, linux-phy, linux-media,
devicetree, linux-kernel
In-Reply-To: <456ded59-d13e-4b61-975b-97ca48b5e771@linaro.org>
On 3/30/26 12:02, Bryan O'Donoghue wrote:
> On 30/03/2026 08:49, Neil Armstrong wrote:
>> On 3/27/26 18:42, Bryan O'Donoghue wrote:
>>> On 27/03/2026 15:28, Neil Armstrong wrote:
>>>>> To be frankly honest you can make an argument for it either way.
>>>>> However my honestly held position is analysing other upstream
>>>>> implementations connecting to the PHY means we can't make the PHY
>>>>> device a drivers/phy device - it would have to be a V4L2 device and
>>>>> then for me the question is why is that even required ?
>>>>
>>>> This is plain wrong, DT definition is different from software
>>>> implementation, you can do whatever you want if you describe HW
>>>> accurately.
>>>
>>> I'm not sure what point it is you are trying to make here. Are you
>>> trying to say drivers/phy is OK with you but you want an endpoint ? If
>>> so, please just say so.
>>
>> I'm against using the "phys = <>" property in the CAMSS to reference the
>> PHYs, a "PHY" in the classic terminology is tied to a single consumer,
>> and if it can be shared to multiple consumer you must model a mux or
>> whatever in the middle.
>
> The CSIPHY-to-CSID routing is runtime-configurable and is already
> managed by the media controller framework.
>
> DT describes static hardware connections. The dynamic mux is a software
> concern, not a hardware description concern.
>
>
>> The PHY API as an internal software implementation is probably fine,
>> even if it makes implementation of split mode much much harder and
>> doesn't really solve anything, you can just call init()/poweron()/
>> poweroff()/exit() directly from the CSIPHY media callbacks.
>
> Great.
>
>>> I can see an argument for that hence my response to Konrad, I just
>>> don't see why its a Qualcomm specific argument and of course
>>> understood stuff bubbles up in review, we have a public debate and
>>> come to a consensus - that's a good thing.
>>>
>>> However, I'd want wider buy-in and understanding that endpoints in the
>>> PHYs is a more accurate description of the data-flow.
>>
>> It is, and it was designed for that, and extensively used in the media
>> DT representation, so I wonder here you would not use it...
>> In an ideal world, you would add nodes for each CAMSS hw elements and
>> adds port/endpoints links between all nodes to describe the data graph,
>> this would be used to construct the media controller graph, and make it
>> much easier supporting new hardware.
>
> Yes but be pragmatic Neil. The first step in making the monolith into
> sub-nodes is the CSIPHY.
>
> Once the CSIPHY is in, we can follow on with adding new nodes that way
> IPE, BPS, ICP, JPEG whatever and also work on implementing the old stuff
> in that new way.
>
>
>>
>>>
>>> We've been applying DT bindings aplenty without that so far. So we
>>> would establish new CSI2 PHY bindings should represent the sensor
>>> endpoints.
>>
>> We've been using a dummy representation of CAMM in a single node with
>> only endpoints connecting to the sensors and hiding all the hardware
>> layout in code, it doesn't scale and makes supporting new HW hard.
>> I mean this is common sense, why would we continue to stick to the
>> current CAMSS bindings ???
>
> We _won't_ I just don't support a big bang integration. Progressive
> changes over a longer timeline make the transition manageable, without
> accepting endless sub-standard stuff in the meantime or holding up all
> new SoC submission unless/until.
>
> I mean there is a CAMSS meeting which I've been running for nearly a
> year now that both you and Vlad are invited to where we have been
> discussing this for months...
The established process of Linux kernel development is based on email
discussions, the sent changes including CSIPHY dt bindings were plainly
ignored by the maintainer, because it's a NIH material or whatever.
There was a chance to discuss CSIPHY dt bindings changes one year ago,
now it might be not a coincidence that eventually after the series of
updates the new CSIPHY dt bindings will be very close to the once
displayed ones, it took a year, but still this is a good progress IMO.
--
Best wishes,
Vladimir
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v5 1/2] dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema
From: Konrad Dybcio @ 2026-03-30 9:46 UTC (permalink / raw)
To: Dmitry Baryshkov, Bryan O'Donoghue
Cc: Vladimir Zapolskiy, Bryan O'Donoghue, Vinod Koul,
Kishon Vijay Abraham I, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Neil Armstrong, linux-arm-msm, linux-phy,
linux-media, devicetree, linux-kernel
In-Reply-To: <r767islbwq2a3m6rf4wvl4hxzafdsw74jhev7jjz665kfymn56@vcn4p233n74f>
On 3/29/26 12:54 PM, Dmitry Baryshkov wrote:
> On Fri, Mar 27, 2026 at 11:40:51PM +0000, Bryan O'Donoghue wrote:
>> On 27/03/2026 23:23, Dmitry Baryshkov wrote:
>>> On Sat, Mar 28, 2026 at 01:12:22AM +0200, Vladimir Zapolskiy wrote:
>>>> On 3/28/26 00:29, Bryan O'Donoghue wrote:
>>>>> On 27/03/2026 20:51, Dmitry Baryshkov wrote:
>>>>>>> That's just not true. If you read the camx source code you can see
>>>>>>> split/combo mode 2+1 1+1 data/clock mode requires special programming of the
>>>>>>> PHY to support.
>>>>>> This needs to be identified from the data-lanes / clock-lanes topology.
>>>>>> And once you do that, there would be (probably) no difference in the
>>>>>> hardware definition.
>>>>>>
>>>>>>
>>>>>> In other words, I'd also ask to drop this mode from the DT. This
>>>>>> infromation can and should be deduced from other, already-defined
>>>>>> properties.
>>>>>
>>>>> It still needs to be communicated to the PHY from the controller,
>>>>> however that is not a problem I am trying to solve now.
>>>>>
>>>>> If I can't get consensus for PHY_QCOM_CSI2_MODE_SPLIT_DPHY then so be it.
>>>>>
>>>>> I'll aim for DPHY only and we can come back to this topic when someone
>>>>> actually tries to enable it.
>>>>>
>>>>
>>>> DPHY may be the only supported phy type in the driver, it does not matter
>>>> at this point, however it's totally essential to cover the called by you
>>>> 'split mode' right from the beginning in the renewed device tree binding
>>>> descriptions of CAMSS IPs to progress further.
>>>
>>> Okay. How would we describe that there are two sensors connected to the
>>> single PHY anyway? How would it be described with the current bindings?
>>>
>>> --
>>> With best wishes
>>> Dmitry
>>
>> Assuming you add endpoints to the PHY i.e. that is what Neil appears to be
>> asking for and I personally am _fine_ with that, then it should just be
>>
>> port@0
>> port@1
>>
>> if port@1 exists, you know you are in split-phy mode.
>>
>> Its actually straight forward enough, really. To be clear though I can write
>> that yaml - the _most_ support I'm willing to put into the PHY code is to
>> detect the port@1 and say "nope not supported yet", since like CPHY its not.
>
> SGTM. But let's define the schema for those usecases.
Let's perhaps also add a short example for both a single- and dual-sensor
cases in the YAML, even if there's no plans to support the latter
configuration now
Konrad
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v5 1/2] dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema
From: Bryan O'Donoghue @ 2026-03-30 9:25 UTC (permalink / raw)
To: Neil Armstrong, Bryan O'Donoghue, Konrad Dybcio, Vinod Koul,
Kishon Vijay Abraham I, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: Vladimir Zapolskiy, linux-arm-msm, linux-phy, linux-media,
devicetree, linux-kernel
In-Reply-To: <76ea03d0-d41b-4880-a48c-06570eb089ed@linaro.org>
On 30/03/2026 10:17, Neil Armstrong wrote:
> On 3/30/26 11:02, Bryan O'Donoghue wrote:
>> On 30/03/2026 08:49, Neil Armstrong wrote:
>>> On 3/27/26 18:42, Bryan O'Donoghue wrote:
>>>> On 27/03/2026 15:28, Neil Armstrong wrote:
>>>>>> To be frankly honest you can make an argument for it either way. However my honestly held position is analysing other upstream implementations connecting to the PHY means we can't make the PHY device a drivers/phy device - it would have to be a V4L2 device and then for me the question is why is that even required ?
>>>>>
>>>>> This is plain wrong, DT definition is different from software implementation, you can do whatever you want if you describe HW accurately.
>>>>
>>>> I'm not sure what point it is you are trying to make here. Are you trying to say drivers/phy is OK with you but you want an endpoint ? If so, please just say so.
>>>
>>> I'm against using the "phys = <>" property in the CAMSS to reference the PHYs, a "PHY" in the classic terminology is tied to a single consumer, and if it can be shared to multiple consumer you must model a mux or whatever in the middle.
>>
>> The CSIPHY-to-CSID routing is runtime-configurable and is already managed by the media controller framework.
>
> This is not compatible with the PHY bindings if you don't have a defined MUX device in the middle, it's wrong. You're hiding the muxing details in the CAMSS blob node.
>
>>
>> DT describes static hardware connections. The dynamic mux is a software concern, not a hardware description concern.
>
> DT must describe the possible interconnections between the nodes, if a PHY can be used by multiple hardware components, it must be described.
But right now the CAMSS block is described as a single block. There is
no CSID device in the kernel _yet_.
When we break CSID into its own block then fine, lets have a debate
about a mux then but right now the "nodes" are CAMSS[MONOLITH] <=>
CSIPHY there is no DT CSID device to model this to.
>
>>
>>
>>> The PHY API as an internal software implementation is probably fine, even if it makes implementation of split mode much much harder and doesn't really solve anything, you can just call init()/poweron()/ poweroff()/exit() directly from the CSIPHY media callbacks.
>>
>> Great.
>>
>>>> I can see an argument for that hence my response to Konrad, I just don't see why its a Qualcomm specific argument and of course understood stuff bubbles up in review, we have a public debate and come to a consensus - that's a good thing.
>>>>
>>>> However, I'd want wider buy-in and understanding that endpoints in the PHYs is a more accurate description of the data-flow.
>>>
>>> It is, and it was designed for that, and extensively used in the media DT representation, so I wonder here you would not use it...
>>> In an ideal world, you would add nodes for each CAMSS hw elements and adds port/endpoints links between all nodes to describe the data graph, this would be used to construct the media controller graph, and make it much easier supporting new hardware.
>>
>> Yes but be pragmatic Neil. The first step in making the monolith into sub-nodes is the CSIPHY.
>
> I am, and I agree it's fine to do it step by step.
Cool.
So let's talk about muxing to CSID devices, when we have CSID devices in
the DT.
>>
>> Once the CSIPHY is in, we can follow on with adding new nodes that way IPE, BPS, ICP, JPEG whatever and also work on implementing the old stuff in that new way.
>
> I agree on the approach, I never said otherwise, but you need to have the big picture in mind.
>
> When you'll have the CSID subnodes, where will you add the phys properties ? you'll keep them in the CAMSS top node ? add all CSIPHY to all CSID nodes ? this is wrong and this needs to be fixed now.
>
>>
>>
>>>
>>>>
>>>> We've been applying DT bindings aplenty without that so far. So we would establish new CSI2 PHY bindings should represent the sensor endpoints.
>>>
>>> We've been using a dummy representation of CAMM in a single node with only endpoints connecting to the sensors and hiding all the hardware layout in code, it doesn't scale and makes supporting new HW hard.
>>> I mean this is common sense, why would we continue to stick to the current CAMSS bindings ???
>>
>> We _won't_ I just don't support a big bang integration. Progressive changes over a longer timeline make the transition manageable, without accepting endless sub-standard stuff in the meantime or holding up all new SoC submission unless/until.
>>
>> I mean there is a CAMSS meeting which I've been running for nearly a year now that both you and Vlad are invited to where we have been discussing this for months...
>>
>> Anyway one conclusion of that is we want to transition to individual nodes for everything.
>>
>> PHY is the first step which I'm taking because its easier for me as CAMSS maintainer to convince the CAMSS maintainer to take the relevant patches.
>>
>> drivers/phy notwithstanding.
>
> As I said I agree on the progressive approach, not the PHY DT bindins approach.
>
>>
>>>>
>>>> Is that what you want ?
>>>>
>>>>> The CSIPHYs are not tied to a single "consumer" block, they can be connected to different consumers at runtime, which is not something classic PHY devices are designed for. So they are de facto a media element in the dynamic camera pipeline.
>>>>
>>>> The existing CAMSS binding and media graph are not changed by this series.
>>>
>>> This is not my point, I don't care about the software implementation at all, I care about accurate hardware representation. Using the "phys = <>" property does not describe hardware accurately.
>>>
>>> In other words: The CSIPHY are not connected to CAMSS. This is _not_ true, tying the CSIPHYs to the CAMSS block hides the real data muxing in software.
>>>
>>> Please remind DT is used by multiple operating systems, and properly describing hardware in DT will help have good software support over all OSes, not just Linux.
>>>
>>>>
>>>>> And actually Rob Herring asked use to define the complete data flow, it was a strong requirement. I don't see why we wouldn't here.
>>>>
>>>> I'm implementing feedback from Rob.
>>>>
>>>> https://lore.kernel.org/linux-media/20250710230846.GA44483- robh@kernel.org/
>>>
>>> Where did he ask using the PHY DT bindings ? Is he aware those CSIPHYs are muxed to multiple consumers which are burried in the CAMSS code ?
>>
>> I freely admit to taking the initiative of phys = <> but Neil there is _no_change_ to the media graph and the "mux" is a runtime configuration that is one register in the CSID.
>
> Honestly I don't care about the userspace media graph, this is a software problem and we can totally make the transition seamless if we want.
>
> Don't limit the DT hardware description because of a software userspace ABI breakage, this approach is not the right one.
>
>>
>> You seriously want a mux device in the kernel to model one bit in a register ?
>
> Why not ? We have drivers that even toggles even single bit to solve those kind of situations.
>
> Physically they're a mux to route the CSIPHY to the consumers, it's a fact... even if it's a register or a single bit.
That's fine. I can understanding making that case if/when CSID becomes
its own node but, I don't think it makes sense when connecting the PHY
back to the monolith.
---
bod
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v5 1/2] dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema
From: Neil Armstrong @ 2026-03-30 9:17 UTC (permalink / raw)
To: Bryan O'Donoghue, Konrad Dybcio, Vinod Koul,
Kishon Vijay Abraham I, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: Bryan O'Donoghue, Vladimir Zapolskiy, linux-arm-msm,
linux-phy, linux-media, devicetree, linux-kernel
In-Reply-To: <456ded59-d13e-4b61-975b-97ca48b5e771@linaro.org>
On 3/30/26 11:02, Bryan O'Donoghue wrote:
> On 30/03/2026 08:49, Neil Armstrong wrote:
>> On 3/27/26 18:42, Bryan O'Donoghue wrote:
>>> On 27/03/2026 15:28, Neil Armstrong wrote:
>>>>> To be frankly honest you can make an argument for it either way. However my honestly held position is analysing other upstream implementations connecting to the PHY means we can't make the PHY device a drivers/phy device - it would have to be a V4L2 device and then for me the question is why is that even required ?
>>>>
>>>> This is plain wrong, DT definition is different from software implementation, you can do whatever you want if you describe HW accurately.
>>>
>>> I'm not sure what point it is you are trying to make here. Are you trying to say drivers/phy is OK with you but you want an endpoint ? If so, please just say so.
>>
>> I'm against using the "phys = <>" property in the CAMSS to reference the PHYs, a "PHY" in the classic terminology is tied to a single consumer, and if it can be shared to multiple consumer you must model a mux or whatever in the middle.
>
> The CSIPHY-to-CSID routing is runtime-configurable and is already managed by the media controller framework.
This is not compatible with the PHY bindings if you don't have a defined MUX device in the middle, it's wrong. You're hiding the muxing details in the CAMSS blob node.
>
> DT describes static hardware connections. The dynamic mux is a software concern, not a hardware description concern.
DT must describe the possible interconnections between the nodes, if a PHY can be used by multiple hardware components, it must be described.
>
>
>> The PHY API as an internal software implementation is probably fine, even if it makes implementation of split mode much much harder and doesn't really solve anything, you can just call init()/poweron()/ poweroff()/exit() directly from the CSIPHY media callbacks.
>
> Great.
>
>>> I can see an argument for that hence my response to Konrad, I just don't see why its a Qualcomm specific argument and of course understood stuff bubbles up in review, we have a public debate and come to a consensus - that's a good thing.
>>>
>>> However, I'd want wider buy-in and understanding that endpoints in the PHYs is a more accurate description of the data-flow.
>>
>> It is, and it was designed for that, and extensively used in the media DT representation, so I wonder here you would not use it...
>> In an ideal world, you would add nodes for each CAMSS hw elements and adds port/endpoints links between all nodes to describe the data graph, this would be used to construct the media controller graph, and make it much easier supporting new hardware.
>
> Yes but be pragmatic Neil. The first step in making the monolith into sub-nodes is the CSIPHY.
I am, and I agree it's fine to do it step by step.
>
> Once the CSIPHY is in, we can follow on with adding new nodes that way IPE, BPS, ICP, JPEG whatever and also work on implementing the old stuff in that new way.
I agree on the approach, I never said otherwise, but you need to have the big picture in mind.
When you'll have the CSID subnodes, where will you add the phys properties ? you'll keep them in the CAMSS top node ? add all CSIPHY to all CSID nodes ? this is wrong and this needs to be fixed now.
>
>
>>
>>>
>>> We've been applying DT bindings aplenty without that so far. So we would establish new CSI2 PHY bindings should represent the sensor endpoints.
>>
>> We've been using a dummy representation of CAMM in a single node with only endpoints connecting to the sensors and hiding all the hardware layout in code, it doesn't scale and makes supporting new HW hard.
>> I mean this is common sense, why would we continue to stick to the current CAMSS bindings ???
>
> We _won't_ I just don't support a big bang integration. Progressive changes over a longer timeline make the transition manageable, without accepting endless sub-standard stuff in the meantime or holding up all new SoC submission unless/until.
>
> I mean there is a CAMSS meeting which I've been running for nearly a year now that both you and Vlad are invited to where we have been discussing this for months...
>
> Anyway one conclusion of that is we want to transition to individual nodes for everything.
>
> PHY is the first step which I'm taking because its easier for me as CAMSS maintainer to convince the CAMSS maintainer to take the relevant patches.
>
> drivers/phy notwithstanding.
As I said I agree on the progressive approach, not the PHY DT bindins approach.
>
>>>
>>> Is that what you want ?
>>>
>>>> The CSIPHYs are not tied to a single "consumer" block, they can be connected to different consumers at runtime, which is not something classic PHY devices are designed for. So they are de facto a media element in the dynamic camera pipeline.
>>>
>>> The existing CAMSS binding and media graph are not changed by this series.
>>
>> This is not my point, I don't care about the software implementation at all, I care about accurate hardware representation. Using the "phys = <>" property does not describe hardware accurately.
>>
>> In other words: The CSIPHY are not connected to CAMSS. This is _not_ true, tying the CSIPHYs to the CAMSS block hides the real data muxing in software.
>>
>> Please remind DT is used by multiple operating systems, and properly describing hardware in DT will help have good software support over all OSes, not just Linux.
>>
>>>
>>>> And actually Rob Herring asked use to define the complete data flow, it was a strong requirement. I don't see why we wouldn't here.
>>>
>>> I'm implementing feedback from Rob.
>>>
>>> https://lore.kernel.org/linux-media/20250710230846.GA44483- robh@kernel.org/
>>
>> Where did he ask using the PHY DT bindings ? Is he aware those CSIPHYs are muxed to multiple consumers which are burried in the CAMSS code ?
>
> I freely admit to taking the initiative of phys = <> but Neil there is _no_change_ to the media graph and the "mux" is a runtime configuration that is one register in the CSID.
Honestly I don't care about the userspace media graph, this is a software problem and we can totally make the transition seamless if we want.
Don't limit the DT hardware description because of a software userspace ABI breakage, this approach is not the right one.
>
> You seriously want a mux device in the kernel to model one bit in a register ?
Why not ? We have drivers that even toggles even single bit to solve those kind of situations.
Physically they're a mux to route the CSIPHY to the consumers, it's a fact... even if it's a register or a single bit.
>
> No.
>
>>>
>>> To me, here is where we stand:
>>>
>>> - Individual nodes - we all agree that
>>> - As sub-nodes - I think the majority agrees this Krzsztof, Dmitry
>>> I'm fine with it too.
>>> - drivers/phy - I think we are accepting this is also fine ?
>>
>> Like I said this adds a supplementary API layer for no reason and will make life harder, but I don't care personally.
>>
>>> - endpoints should flow into the PHY and then back to the controller
>>>
>>> I get that argument. In fact I _like_ that argument at least I like my conception of that argument.
>>>
>>> I'll stipulate to that argument meaning then that, new CSI2 PHYs shall include endpoints for this purpose globally.
>>>
>>> As I've said before, there's nothing Qualcomm specific about this discussion, really.
>>
>> There is, because the current Qualcomm CAMSS bindings are insufficient and should be entirely redesigned from the ground up to properly describe the HW.
>
> The long term plan is to do that. Discussed extensively with the Qcom teams delivering CAMSS, even invited community members along.
>
> Here are the meeting notes - its all in the public domain
>
> https://docs.google.com/document/d/1yUwWHaKLuovKVgGTzvKm68K1zS6xBONVzjOsRjDl03U/edit?tab=t.0#heading=h.mtbm7qfohwfs
>
> ---
> bod
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v5 1/2] dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema
From: Bryan O'Donoghue @ 2026-03-30 9:02 UTC (permalink / raw)
To: Neil Armstrong, Konrad Dybcio, Vinod Koul, Kishon Vijay Abraham I,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Bryan O'Donoghue, Vladimir Zapolskiy, linux-arm-msm,
linux-phy, linux-media, devicetree, linux-kernel
In-Reply-To: <016c03b8-27c3-41dc-a630-8e7095db1f88@linaro.org>
On 30/03/2026 08:49, Neil Armstrong wrote:
> On 3/27/26 18:42, Bryan O'Donoghue wrote:
>> On 27/03/2026 15:28, Neil Armstrong wrote:
>>>> To be frankly honest you can make an argument for it either way.
>>>> However my honestly held position is analysing other upstream
>>>> implementations connecting to the PHY means we can't make the PHY
>>>> device a drivers/phy device - it would have to be a V4L2 device and
>>>> then for me the question is why is that even required ?
>>>
>>> This is plain wrong, DT definition is different from software
>>> implementation, you can do whatever you want if you describe HW
>>> accurately.
>>
>> I'm not sure what point it is you are trying to make here. Are you
>> trying to say drivers/phy is OK with you but you want an endpoint ? If
>> so, please just say so.
>
> I'm against using the "phys = <>" property in the CAMSS to reference the
> PHYs, a "PHY" in the classic terminology is tied to a single consumer,
> and if it can be shared to multiple consumer you must model a mux or
> whatever in the middle.
The CSIPHY-to-CSID routing is runtime-configurable and is already
managed by the media controller framework.
DT describes static hardware connections. The dynamic mux is a software
concern, not a hardware description concern.
> The PHY API as an internal software implementation is probably fine,
> even if it makes implementation of split mode much much harder and
> doesn't really solve anything, you can just call init()/poweron()/
> poweroff()/exit() directly from the CSIPHY media callbacks.
Great.
>> I can see an argument for that hence my response to Konrad, I just
>> don't see why its a Qualcomm specific argument and of course
>> understood stuff bubbles up in review, we have a public debate and
>> come to a consensus - that's a good thing.
>>
>> However, I'd want wider buy-in and understanding that endpoints in the
>> PHYs is a more accurate description of the data-flow.
>
> It is, and it was designed for that, and extensively used in the media
> DT representation, so I wonder here you would not use it...
> In an ideal world, you would add nodes for each CAMSS hw elements and
> adds port/endpoints links between all nodes to describe the data graph,
> this would be used to construct the media controller graph, and make it
> much easier supporting new hardware.
Yes but be pragmatic Neil. The first step in making the monolith into
sub-nodes is the CSIPHY.
Once the CSIPHY is in, we can follow on with adding new nodes that way
IPE, BPS, ICP, JPEG whatever and also work on implementing the old stuff
in that new way.
>
>>
>> We've been applying DT bindings aplenty without that so far. So we
>> would establish new CSI2 PHY bindings should represent the sensor
>> endpoints.
>
> We've been using a dummy representation of CAMM in a single node with
> only endpoints connecting to the sensors and hiding all the hardware
> layout in code, it doesn't scale and makes supporting new HW hard.
> I mean this is common sense, why would we continue to stick to the
> current CAMSS bindings ???
We _won't_ I just don't support a big bang integration. Progressive
changes over a longer timeline make the transition manageable, without
accepting endless sub-standard stuff in the meantime or holding up all
new SoC submission unless/until.
I mean there is a CAMSS meeting which I've been running for nearly a
year now that both you and Vlad are invited to where we have been
discussing this for months...
Anyway one conclusion of that is we want to transition to individual
nodes for everything.
PHY is the first step which I'm taking because its easier for me as
CAMSS maintainer to convince the CAMSS maintainer to take the relevant
patches.
drivers/phy notwithstanding.
>>
>> Is that what you want ?
>>
>>> The CSIPHYs are not tied to a single "consumer" block, they can be
>>> connected to different consumers at runtime, which is not something
>>> classic PHY devices are designed for. So they are de facto a media
>>> element in the dynamic camera pipeline.
>>
>> The existing CAMSS binding and media graph are not changed by this
>> series.
>
> This is not my point, I don't care about the software implementation at
> all, I care about accurate hardware representation. Using the "phys =
> <>" property does not describe hardware accurately.
>
> In other words: The CSIPHY are not connected to CAMSS. This is _not_
> true, tying the CSIPHYs to the CAMSS block hides the real data muxing in
> software.
>
> Please remind DT is used by multiple operating systems, and properly
> describing hardware in DT will help have good software support over all
> OSes, not just Linux.
>
>>
>>> And actually Rob Herring asked use to define the complete data flow,
>>> it was a strong requirement. I don't see why we wouldn't here.
>>
>> I'm implementing feedback from Rob.
>>
>> https://lore.kernel.org/linux-media/20250710230846.GA44483-
>> robh@kernel.org/
>
> Where did he ask using the PHY DT bindings ? Is he aware those CSIPHYs
> are muxed to multiple consumers which are burried in the CAMSS code ?
I freely admit to taking the initiative of phys = <> but Neil there is
_no_change_ to the media graph and the "mux" is a runtime configuration
that is one register in the CSID.
You seriously want a mux device in the kernel to model one bit in a
register ?
No.
>>
>> To me, here is where we stand:
>>
>> - Individual nodes - we all agree that
>> - As sub-nodes - I think the majority agrees this Krzsztof, Dmitry
>> I'm fine with it too.
>> - drivers/phy - I think we are accepting this is also fine ?
>
> Like I said this adds a supplementary API layer for no reason and will
> make life harder, but I don't care personally.
>
>> - endpoints should flow into the PHY and then back to the controller
>>
>> I get that argument. In fact I _like_ that argument at least I like my
>> conception of that argument.
>>
>> I'll stipulate to that argument meaning then that, new CSI2 PHYs shall
>> include endpoints for this purpose globally.
>>
>> As I've said before, there's nothing Qualcomm specific about this
>> discussion, really.
>
> There is, because the current Qualcomm CAMSS bindings are insufficient
> and should be entirely redesigned from the ground up to properly
> describe the HW.
The long term plan is to do that. Discussed extensively with the Qcom
teams delivering CAMSS, even invited community members along.
Here are the meeting notes - its all in the public domain
https://docs.google.com/document/d/1yUwWHaKLuovKVgGTzvKm68K1zS6xBONVzjOsRjDl03U/edit?tab=t.0#heading=h.mtbm7qfohwfs
---
bod
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox