* [PATCH 1/4] dt-bindings: phy: rockchip,rk3588-mipi-dcphy: support per-direction phys
From: Jason Yang via B4 Relay @ 2026-07-21 10:35 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Heiko Stuebner, Guochun Huang, Philipp Zabel
Cc: linux-phy, devicetree, linux-arm-kernel, linux-rockchip,
linux-kernel, Jason Yang
In-Reply-To: <20260721-dcphy-rx-v1-v1-0-4fc83c0ccac9@gmail.com>
From: Jason Yang <jason98166@gmail.com>
The Samsung MIPI D-/C-PHY block on RK3588 drives a MIPI DSI transmitter
and, on the same PHY, can receive a MIPI CSI camera. Describing both as
independent consumers of one PHY requires the direction to be expressed
in the devicetree.
Allow "#phy-cells" to be 1 or 2. The existing single-cell form still
selects the PHY type and refers to the transmitter, so shipped DSI
device trees stay valid. A second cell adds the direction, letting a
DSI (TX) and a CSI (RX) reference the same PHY node as separate
consumers.
Add include/dt-bindings/phy/phy-rockchip-samsung-dcphy.h for the
RK_DCPHY_DIR_TX and RK_DCPHY_DIR_RX direction constants.
Signed-off-by: Jason Yang <jason98166@gmail.com>
Assisted-by: Claude:claude-opus-4-8
---
.../bindings/phy/rockchip,rk3588-mipi-dcphy.yaml | 22 ++++++++++++++++------
.../dt-bindings/phy/phy-rockchip-samsung-dcphy.h | 13 +++++++++++++
2 files changed, 29 insertions(+), 6 deletions(-)
diff --git a/Documentation/devicetree/bindings/phy/rockchip,rk3588-mipi-dcphy.yaml b/Documentation/devicetree/bindings/phy/rockchip,rk3588-mipi-dcphy.yaml
index c8ff5ba22a86..53d695137566 100644
--- a/Documentation/devicetree/bindings/phy/rockchip,rk3588-mipi-dcphy.yaml
+++ b/Documentation/devicetree/bindings/phy/rockchip,rk3588-mipi-dcphy.yaml
@@ -20,12 +20,22 @@ properties:
maxItems: 1
"#phy-cells":
- const: 1
+ enum: [1, 2]
description: |
- Argument is mode to operate in. Supported modes are:
- - PHY_TYPE_DPHY
- - PHY_TYPE_CPHY
- See include/dt-bindings/phy/phy.h for constants.
+ A single cell selects the PHY type and always refers to the
+ transmitter (DSI) direction. It is kept for backwards compatibility
+ with existing device trees:
+ - <PHY_TYPE_DPHY>
+ - <PHY_TYPE_CPHY>
+
+ Two cells select the PHY type and the direction, so that the
+ transmitter (DSI) and the receiver (CSI) can be described as
+ independent consumers of the same PHY:
+ - <PHY_TYPE_DPHY RK_DCPHY_DIR_TX>
+ - <PHY_TYPE_DPHY RK_DCPHY_DIR_RX>
+
+ See include/dt-bindings/phy/phy.h and
+ include/dt-bindings/phy/phy-rockchip-samsung-dcphy.h for constants.
clocks:
maxItems: 2
@@ -82,6 +92,6 @@ examples:
<&cru SRST_S_MIPI_DCPHY0>;
reset-names = "m_phy", "apb", "grf", "s_phy";
rockchip,grf = <&mipidcphy0_grf>;
- #phy-cells = <1>;
+ #phy-cells = <2>;
};
};
diff --git a/include/dt-bindings/phy/phy-rockchip-samsung-dcphy.h b/include/dt-bindings/phy/phy-rockchip-samsung-dcphy.h
new file mode 100644
index 000000000000..60f6630f58b3
--- /dev/null
+++ b/include/dt-bindings/phy/phy-rockchip-samsung-dcphy.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+/*
+ * Copyright (c) 2026 Jason Yang <jason98166@gmail.com>
+ */
+
+#ifndef _DT_BINDINGS_PHY_ROCKCHIP_SAMSUNG_DCPHY_H
+#define _DT_BINDINGS_PHY_ROCKCHIP_SAMSUNG_DCPHY_H
+
+/* Direction of a Rockchip Samsung MIPI D-/C-PHY combo block */
+#define RK_DCPHY_DIR_TX 0
+#define RK_DCPHY_DIR_RX 1
+
+#endif /* _DT_BINDINGS_PHY_ROCKCHIP_SAMSUNG_DCPHY_H */
--
2.43.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* [PATCH 3/4] phy: rockchip-samsung-dcphy: model TX and RX as separate PHYs
From: Jason Yang via B4 Relay @ 2026-07-21 10:35 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Heiko Stuebner, Guochun Huang, Philipp Zabel
Cc: linux-phy, devicetree, linux-arm-kernel, linux-rockchip,
linux-kernel, Jason Yang
In-Reply-To: <20260721-dcphy-rx-v1-v1-0-4fc83c0ccac9@gmail.com>
From: Jason Yang <jason98166@gmail.com>
The DC-PHY drives a MIPI DSI transmitter and a MIPI CSI receiver, and on
RK3588 both can be wired to the same PHY as independent consumers. The
PHY core reference-counts power_on() per struct phy, so a single struct
phy cannot bring the two directions up independently.
Register one struct phy per direction and move the per-direction state
(direction and PHY type) into its own driver data. of_xlate() accepts
the legacy single cell as the transmitter and an optional second cell
that selects the direction, so existing single-cell DSI device trees
keep resolving to the transmitter.
The shared BIAS block must be programmed only once for the whole block.
Whichever direction powers on first enables it, checking the peer phy's
power_count under a per-provider mutex so that powering one direction on
does not disturb an already running one. The APB reset is issued on the
transmitter bring-up path.
The receiver bring-up itself is added in a later change.
Signed-off-by: Jason Yang <jason98166@gmail.com>
Assisted-by: Claude:claude-opus-4-8
---
drivers/phy/rockchip/phy-rockchip-samsung-dcphy.c | 133 +++++++++++++++-------
1 file changed, 95 insertions(+), 38 deletions(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-samsung-dcphy.c b/drivers/phy/rockchip/phy-rockchip-samsung-dcphy.c
index d0d77421bd4b..0248424929bb 100644
--- a/drivers/phy/rockchip/phy-rockchip-samsung-dcphy.c
+++ b/drivers/phy/rockchip/phy-rockchip-samsung-dcphy.c
@@ -5,6 +5,7 @@
* Guochun Huang <hero.huang@rock-chips.com>
*/
+#include <dt-bindings/phy/phy-rockchip-samsung-dcphy.h>
#include <dt-bindings/phy/phy.h>
#include <linux/bitfield.h>
#include <linux/clk.h>
@@ -13,6 +14,7 @@
#include <linux/kernel.h>
#include <linux/mfd/syscon.h>
#include <linux/module.h>
+#include <linux/mutex.h>
#include <linux/of.h>
#include <linux/phy/phy.h>
#include <linux/platform_device.h>
@@ -280,6 +282,16 @@ struct samsung_mipi_dcphy_plat_data {
u32 dphy_tx_max_lane_kbps;
};
+struct samsung_mipi_dcphy;
+
+/* One PHY per direction: transmitter (DSI) and receiver (CSI). */
+struct samsung_mipi_dcphy_dir {
+ struct phy *phy;
+ struct samsung_mipi_dcphy *parent;
+ u8 dir;
+ u8 type;
+};
+
struct samsung_mipi_dcphy {
struct device *dev;
struct clk *ref_clk;
@@ -291,8 +303,9 @@ struct samsung_mipi_dcphy {
struct reset_control *apb_rst;
struct reset_control *grf_apb_rst;
unsigned int lanes;
- struct phy *phy;
- u8 type;
+ struct samsung_mipi_dcphy_dir phys[2];
+ /* Serialises the two directions' access to the shared PHY state. */
+ struct mutex lock;
const struct samsung_mipi_dcphy_plat_data *pdata;
struct {
@@ -1332,13 +1345,26 @@ samsung_mipi_dphy_data_lane_timing_init(struct samsung_mipi_dcphy *samsung)
regmap_write(samsung->regmap, DPHY_MD3_TIME_CON4, 0x1f4);
}
-static int samsung_mipi_dphy_tx_power_on(struct samsung_mipi_dcphy *samsung)
+static int samsung_mipi_dphy_tx_power_on(struct samsung_mipi_dcphy *samsung,
+ struct phy *peer)
{
+ bool first = peer->power_count == 0;
int ret;
+ /*
+ * The shared BIAS block is brought up by whichever direction powers
+ * on first, leaving an already active peer undisturbed.
+ */
+ if (first) {
+ reset_control_assert(samsung->apb_rst);
+ udelay(1);
+ reset_control_deassert(samsung->apb_rst);
+ }
+
reset_control_assert(samsung->m_phy_rst);
- samsung_mipi_dcphy_bias_block_enable(samsung);
+ if (first)
+ samsung_mipi_dcphy_bias_block_enable(samsung);
samsung_mipi_dcphy_pll_configure(samsung);
samsung_mipi_dphy_clk_lane_timing_init(samsung);
samsung_mipi_dphy_data_lane_timing_init(samsung);
@@ -1368,34 +1394,41 @@ static int samsung_mipi_dphy_tx_power_off(struct samsung_mipi_dcphy *samsung)
static int samsung_mipi_dcphy_power_on(struct phy *phy)
{
- struct samsung_mipi_dcphy *samsung = phy_get_drvdata(phy);
-
- reset_control_assert(samsung->apb_rst);
- udelay(1);
- reset_control_deassert(samsung->apb_rst);
+ struct samsung_mipi_dcphy_dir *pd = phy_get_drvdata(phy);
+ struct samsung_mipi_dcphy *samsung = pd->parent;
+ struct phy *peer = samsung->phys[!pd->dir].phy;
+ int ret;
- switch (samsung->type) {
- case PHY_TYPE_DPHY:
- return samsung_mipi_dphy_tx_power_on(samsung);
- default:
- /* CPHY part to be implemented later */
+ if (pd->type != PHY_TYPE_DPHY)
return -EOPNOTSUPP;
- }
- return 0;
+ mutex_lock(&samsung->lock);
+ if (pd->dir == RK_DCPHY_DIR_RX)
+ ret = -EOPNOTSUPP;
+ else
+ ret = samsung_mipi_dphy_tx_power_on(samsung, peer);
+ mutex_unlock(&samsung->lock);
+
+ return ret;
}
static int samsung_mipi_dcphy_power_off(struct phy *phy)
{
- struct samsung_mipi_dcphy *samsung = phy_get_drvdata(phy);
+ struct samsung_mipi_dcphy_dir *pd = phy_get_drvdata(phy);
+ struct samsung_mipi_dcphy *samsung = pd->parent;
+ int ret;
- switch (samsung->type) {
- case PHY_TYPE_DPHY:
- return samsung_mipi_dphy_tx_power_off(samsung);
- default:
- /* CPHY part to be implemented later */
+ if (pd->type != PHY_TYPE_DPHY)
return -EOPNOTSUPP;
- }
+
+ if (pd->dir == RK_DCPHY_DIR_RX)
+ return -EOPNOTSUPP;
+
+ mutex_lock(&samsung->lock);
+ ret = samsung_mipi_dphy_tx_power_off(samsung);
+ mutex_unlock(&samsung->lock);
+
+ return ret;
}
static int
@@ -1488,11 +1521,15 @@ samsung_mipi_dcphy_pll_calc_rate(struct samsung_mipi_dcphy *samsung,
static int samsung_mipi_dcphy_configure(struct phy *phy,
union phy_configure_opts *opts)
{
- struct samsung_mipi_dcphy *samsung = phy_get_drvdata(phy);
+ struct samsung_mipi_dcphy_dir *pd = phy_get_drvdata(phy);
+ struct samsung_mipi_dcphy *samsung = pd->parent;
unsigned long long target_rate = opts->mipi_dphy.hs_clk_rate;
samsung->lanes = opts->mipi_dphy.lanes > 4 ? 4 : opts->mipi_dphy.lanes;
+ if (pd->dir == RK_DCPHY_DIR_RX)
+ return -EOPNOTSUPP;
+
samsung_mipi_dcphy_pll_calc_rate(samsung, target_rate);
opts->mipi_dphy.hs_clk_rate = samsung->pll.rate;
@@ -1501,16 +1538,16 @@ static int samsung_mipi_dcphy_configure(struct phy *phy,
static int samsung_mipi_dcphy_init(struct phy *phy)
{
- struct samsung_mipi_dcphy *samsung = phy_get_drvdata(phy);
+ struct samsung_mipi_dcphy_dir *pd = phy_get_drvdata(phy);
- return pm_runtime_resume_and_get(samsung->dev);
+ return pm_runtime_resume_and_get(pd->parent->dev);
}
static int samsung_mipi_dcphy_exit(struct phy *phy)
{
- struct samsung_mipi_dcphy *samsung = phy_get_drvdata(phy);
+ struct samsung_mipi_dcphy_dir *pd = phy_get_drvdata(phy);
- pm_runtime_put(samsung->dev);
+ pm_runtime_put(pd->parent->dev);
return 0;
}
@@ -1536,19 +1573,29 @@ static struct phy *samsung_mipi_dcphy_xlate(struct device *dev,
const struct of_phandle_args *args)
{
struct samsung_mipi_dcphy *samsung = dev_get_drvdata(dev);
+ struct samsung_mipi_dcphy_dir *pd;
+ u8 dir = RK_DCPHY_DIR_TX;
- if (args->args_count != 1) {
+ if (args->args_count < 1 || args->args_count > 2) {
dev_err(dev, "invalid number of arguments\n");
return ERR_PTR(-EINVAL);
}
- if (samsung->type != PHY_NONE && samsung->type != args->args[0])
- dev_warn(dev, "phy type select %d overwriting type %d\n",
- args->args[0], samsung->type);
+ if (args->args_count == 2) {
+ if (args->args[1] > RK_DCPHY_DIR_RX) {
+ dev_err(dev, "invalid direction %u\n", args->args[1]);
+ return ERR_PTR(-EINVAL);
+ }
+ dir = args->args[1];
+ }
- samsung->type = args->args[0];
+ pd = &samsung->phys[dir];
+ if (pd->type != PHY_NONE && pd->type != args->args[0])
+ dev_warn(dev, "phy type select %u overwriting type %u\n",
+ args->args[0], pd->type);
+ pd->type = args->args[0];
- return samsung->phy;
+ return pd->phy;
}
static int samsung_mipi_dcphy_probe(struct platform_device *pdev)
@@ -1559,6 +1606,7 @@ static int samsung_mipi_dcphy_probe(struct platform_device *pdev)
struct phy_provider *phy_provider;
struct resource *res;
void __iomem *regs;
+ unsigned int i;
int ret;
samsung = devm_kzalloc(dev, sizeof(*samsung), GFP_KERNEL);
@@ -1568,6 +1616,7 @@ static int samsung_mipi_dcphy_probe(struct platform_device *pdev)
samsung->dev = dev;
samsung->pdata = device_get_match_data(dev);
platform_set_drvdata(pdev, samsung);
+ mutex_init(&samsung->lock);
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
regs = devm_ioremap_resource(dev, res);
@@ -1613,11 +1662,19 @@ static int samsung_mipi_dcphy_probe(struct platform_device *pdev)
return dev_err_probe(dev, PTR_ERR(samsung->grf_apb_rst),
"Failed to get system grf_apb_rst control\n");
- samsung->phy = devm_phy_create(dev, NULL, &samsung_mipi_dcphy_ops);
- if (IS_ERR(samsung->phy))
- return dev_err_probe(dev, PTR_ERR(samsung->phy), "Failed to create MIPI DC-PHY\n");
+ for (i = 0; i < ARRAY_SIZE(samsung->phys); i++) {
+ struct phy *phy = devm_phy_create(dev, NULL,
+ &samsung_mipi_dcphy_ops);
- phy_set_drvdata(samsung->phy, samsung);
+ if (IS_ERR(phy))
+ return dev_err_probe(dev, PTR_ERR(phy),
+ "Failed to create MIPI DC-PHY\n");
+
+ samsung->phys[i].phy = phy;
+ samsung->phys[i].parent = samsung;
+ samsung->phys[i].dir = i;
+ phy_set_drvdata(phy, &samsung->phys[i]);
+ }
ret = devm_pm_runtime_enable(dev);
if (ret)
--
2.43.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* [PATCH 2/4] phy: rockchip-samsung-dcphy: factor MIPI D-PHY power on/off into helpers
From: Jason Yang via B4 Relay @ 2026-07-21 10:35 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Heiko Stuebner, Guochun Huang, Philipp Zabel
Cc: linux-phy, devicetree, linux-arm-kernel, linux-rockchip,
linux-kernel, Jason Yang
In-Reply-To: <20260721-dcphy-rx-v1-v1-0-4fc83c0ccac9@gmail.com>
From: Jason Yang <jason98166@gmail.com>
To prepare for adding a receiver (CSI) direction next to the existing
transmitter (DSI) one, rename the transmitter power-on helper to
samsung_mipi_dphy_tx_power_on() and gather its teardown into a new
samsung_mipi_dphy_tx_power_off(), so both directions can later be
powered on and off through symmetric helpers.
No functional change intended.
Signed-off-by: Jason Yang <jason98166@gmail.com>
Assisted-by: Claude:claude-opus-4-8
---
drivers/phy/rockchip/phy-rockchip-samsung-dcphy.c | 19 +++++++++++--------
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-samsung-dcphy.c b/drivers/phy/rockchip/phy-rockchip-samsung-dcphy.c
index cbd780556da8..d0d77421bd4b 100644
--- a/drivers/phy/rockchip/phy-rockchip-samsung-dcphy.c
+++ b/drivers/phy/rockchip/phy-rockchip-samsung-dcphy.c
@@ -1332,7 +1332,7 @@ samsung_mipi_dphy_data_lane_timing_init(struct samsung_mipi_dcphy *samsung)
regmap_write(samsung->regmap, DPHY_MD3_TIME_CON4, 0x1f4);
}
-static int samsung_mipi_dphy_power_on(struct samsung_mipi_dcphy *samsung)
+static int samsung_mipi_dphy_tx_power_on(struct samsung_mipi_dcphy *samsung)
{
int ret;
@@ -1358,6 +1358,14 @@ static int samsung_mipi_dphy_power_on(struct samsung_mipi_dcphy *samsung)
return 0;
}
+static int samsung_mipi_dphy_tx_power_off(struct samsung_mipi_dcphy *samsung)
+{
+ samsung_mipi_dphy_lane_disable(samsung);
+ samsung_mipi_dcphy_pll_disable(samsung);
+
+ return 0;
+}
+
static int samsung_mipi_dcphy_power_on(struct phy *phy)
{
struct samsung_mipi_dcphy *samsung = phy_get_drvdata(phy);
@@ -1368,7 +1376,7 @@ static int samsung_mipi_dcphy_power_on(struct phy *phy)
switch (samsung->type) {
case PHY_TYPE_DPHY:
- return samsung_mipi_dphy_power_on(samsung);
+ return samsung_mipi_dphy_tx_power_on(samsung);
default:
/* CPHY part to be implemented later */
return -EOPNOTSUPP;
@@ -1383,16 +1391,11 @@ static int samsung_mipi_dcphy_power_off(struct phy *phy)
switch (samsung->type) {
case PHY_TYPE_DPHY:
- samsung_mipi_dphy_lane_disable(samsung);
- break;
+ return samsung_mipi_dphy_tx_power_off(samsung);
default:
/* CPHY part to be implemented later */
return -EOPNOTSUPP;
}
-
- samsung_mipi_dcphy_pll_disable(samsung);
-
- return 0;
}
static int
--
2.43.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* [PATCH 0/4] phy: rockchip-samsung-dcphy: support the D-PHY receiver direction
From: Jason Yang via B4 Relay @ 2026-07-21 10:35 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Heiko Stuebner, Guochun Huang, Philipp Zabel
Cc: linux-phy, devicetree, linux-arm-kernel, linux-rockchip,
linux-kernel, Jason Yang
The Samsung DC-PHY on RK3588 contains both a MIPI transmitter (DSI)
and a MIPI receiver (CSI) with separate register banks and resets.
Mainline currently models only the transmitter. This series adds the
receiver so a CSI camera can use the PHY, including running both
directions concurrently (a DSI panel and a CSI camera on the same PHY
instance).
Design notes, to answer the obvious questions up front:
- Direction as a phy-cell rather than a phy_set_mode() submode:
The PHY core reference-counts power_on() per struct phy. Bringing TX
and RX up and down independently requires one struct phy per
direction; a submode on a single phy cannot express two concurrent
owners. The extra cell selects which of the two phys a consumer
gets, following how phy-exynos-mipi-video exposes multiple phys
from one register block. The existing single-cell binding still
resolves to the transmitter, ensuring shipped device trees remain
unaffected.
- BIAS block coordination:
The BIAS block is part of the PHY's common block and exists once for
both directions (RK3588 TRM section 22.2), so it must be programmed
only once. Whichever direction powers on first enables it; the
first-on/last-off coordination follows phy-exynos-mipi-video.
- RX tuning:
RX tuning uses fixed RK3588 defaults. Boards that need finer
receiver tuning should extend phy_configure_opts_mipi_dphy rather
than add private device-tree properties.
Patch summary:
patch 1: dt-bindings: optional second #phy-cells cell for direction
patch 2: prep: symmetric TX power on/off helpers, no functional change
patch 3: register one struct phy per direction
patch 4: implement the receiver
Testing:
The series was tested on an IEI WAFER-RK3588 board across the following
configurations:
- Concurrent TX/RX: IMX219 CSI camera capture (640x480, 1640x1232,
1920x1080, and 3280x2464 in both RAW10 and RAW8) and OV5640 capture
(1920x1080) through the DC-PHY RX, running concurrently with a MIPI
DSI panel (ILI9881C, 720x1280) driven through the TX half of the same
PHY instance.
- Standalone modes: DSI-only and CSI-only configurations.
- Backward compatibility: A shipped single-cell DSI device tree
resolving unchanged to the transmitter.
HS-video fix that is not part of this submission; however, the receiver
support added here is exercised and verified independently by the
CSI-only capture tests.
Note: The DSI half of the concurrent test required a local dw-mipi-dsi2
---
Jason Yang (4):
dt-bindings: phy: rockchip,rk3588-mipi-dcphy: support per-direction phys
phy: rockchip-samsung-dcphy: factor MIPI D-PHY power on/off into helpers
phy: rockchip-samsung-dcphy: model TX and RX as separate PHYs
phy: rockchip-samsung-dcphy: add MIPI D-PHY receiver support
.../bindings/phy/rockchip,rk3588-mipi-dcphy.yaml | 22 +-
drivers/phy/rockchip/phy-rockchip-samsung-dcphy.c | 456 +++++++++++++++++++--
.../dt-bindings/phy/phy-rockchip-samsung-dcphy.h | 13 +
3 files changed, 442 insertions(+), 49 deletions(-)
---
base-commit: 1590cf0329716306e948a8fc29f1d3ee87d3989f
change-id: 20260721-dcphy-rx-v1-9af57f3edfd9
Best regards,
--
Jason Yang <jason98166@gmail.com>
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* [PATCH 4/4] phy: rockchip-samsung-dcphy: add MIPI D-PHY receiver support
From: Jason Yang via B4 Relay @ 2026-07-21 10:35 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Heiko Stuebner, Guochun Huang, Philipp Zabel
Cc: linux-phy, devicetree, linux-arm-kernel, linux-rockchip,
linux-kernel, Jason Yang
In-Reply-To: <20260721-dcphy-rx-v1-v1-0-4fc83c0ccac9@gmail.com>
From: Jason Yang <jason98166@gmail.com>
Implement the receiver (CSI) direction of the D-PHY. Add the RX
register layout and the HS-frequency settle table, and hook the
receiver bring-up and teardown into the per-direction power_on(),
power_off() and configure() paths, all serialized on the provider
mutex alongside the transmitter.
Only the D-PHY receiver is implemented, with fixed RK3588 tuning
defaults. The RX register layout and field definitions follow the
RK3588 TRM (section 22.4); what the TRM does not publish are the
per-data-rate HS settle values (it points to a separate timing-table
supplement and gives only a single example point) and the analog
tuning values, both of which are taken from the Rockchip vendor
kernel. The PLL is left untouched on the receive path: the sensor
supplies the link clock, and the TRM start-up sequence (figure 22-8)
allows the PLL sequence to be skipped when only the receiver (slave)
lanes are used. configure() only records the incoming rate so the
settle table can be indexed.
Signed-off-by: Jason Yang <jason98166@gmail.com>
Assisted-by: Claude:claude-opus-4-8
---
drivers/phy/rockchip/phy-rockchip-samsung-dcphy.c | 330 +++++++++++++++++++++-
1 file changed, 320 insertions(+), 10 deletions(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-samsung-dcphy.c b/drivers/phy/rockchip/phy-rockchip-samsung-dcphy.c
index 0248424929bb..49243ba93bd0 100644
--- a/drivers/phy/rockchip/phy-rockchip-samsung-dcphy.c
+++ b/drivers/phy/rockchip/phy-rockchip-samsung-dcphy.c
@@ -237,6 +237,63 @@
#define DPHY_MD3_TIME_CON4 0x0740
#define DPHY_MD3_DATA_CON0 0x0744
+/* D-PHY receiver direction registers (clock lane + four data lanes) */
+#define DPHY_SC_GNR_CON0 0x0b00
+#define DPHY_SC_GNR_CON1 0x0b04
+#define DPHY_SC_ANA_CON1 0x0b0c
+#define HS_RX_BIAS_CON(x) FIELD_PREP(GENMASK(15, 11), x)
+#define DPHY_SC_ANA_CON2 0x0b10
+#define HS_TERM_SW(x) FIELD_PREP(GENMASK(2, 0), x)
+#define DPHY_SC_ANA_CON3 0x0b14
+#define ULPS_HYS_SW_DPHY(x) FIELD_PREP(GENMASK(10, 8), x)
+#define DPHY_SC_TIME_CON0 0x0b30
+#define T_CLK_SETTLE(x) FIELD_PREP(GENMASK(7, 0), x)
+#define T_CLK_MISS(x) FIELD_PREP(GENMASK(11, 8), x)
+#define COMBO_SD0_GNR_CON0 0x0c00
+#define COMBO_SD0_GNR_CON1 0x0c04
+#define COMBO_SD0_ANA_CON1 0x0c0c
+#define COMBO_SD0_ANA_CON2 0x0c10
+#define SKEW_DLYSEL(x) FIELD_PREP(GENMASK(9, 8), x)
+#define RX_TERM_SW(x) FIELD_PREP(GENMASK(2, 0), x)
+#define COMBO_SD0_ANA_CON3 0x0c14
+#define SEL_ESCPOL BIT(11)
+#define LP_HYS_SW(x) FIELD_PREP(GENMASK(5, 4), x)
+#define COMBO_SD0_ANA_CON7 0x0c24
+#define CLK_DBL_CTRL(x) FIELD_PREP(GENMASK(7, 6), x)
+#define COMBO_SD0_TIME_CON0 0x0c30
+#define T_HS_SETTLE(x) FIELD_PREP(GENMASK(7, 0), x)
+#define SETTLE_CLK_SEL BIT(8)
+#define COMBO_SD0_TIME_CON1 0x0c34
+#define T_ERR_SOT_SYNC(x) FIELD_PREP(GENMASK(7, 0), x)
+#define COMBO_SD0_DESKEW_CON2 0x0c48
+#define SKEW_CAL_CLK_COARSE_SET(x) FIELD_PREP(GENMASK(4, 0), x)
+#define COMBO_SD1_GNR_CON0 0x0d00
+#define COMBO_SD1_GNR_CON1 0x0d04
+#define COMBO_SD1_ANA_CON1 0x0d0c
+#define COMBO_SD1_ANA_CON2 0x0d10
+#define COMBO_SD1_ANA_CON3 0x0d14
+#define COMBO_SD1_ANA_CON7 0x0d24
+#define COMBO_SD1_TIME_CON0 0x0d30
+#define COMBO_SD1_TIME_CON1 0x0d34
+#define COMBO_SD1_DESKEW_CON2 0x0d48
+#define COMBO_SD2_GNR_CON0 0x0e00
+#define COMBO_SD2_GNR_CON1 0x0e04
+#define COMBO_SD2_ANA_CON1 0x0e0c
+#define COMBO_SD2_ANA_CON2 0x0e10
+#define COMBO_SD2_ANA_CON3 0x0e14
+#define COMBO_SD2_ANA_CON7 0x0e24
+#define COMBO_SD2_TIME_CON0 0x0e30
+#define COMBO_SD2_TIME_CON1 0x0e34
+#define COMBO_SD2_DESKEW_CON2 0x0e48
+#define DPHY_SD3_GNR_CON0 0x0f00
+#define DPHY_SD3_GNR_CON1 0x0f04
+#define DPHY_SD3_ANA_CON1 0x0f0c
+#define DPHY_SD3_ANA_CON2 0x0f10
+#define DPHY_SD3_ANA_CON3 0x0f14
+#define DPHY_SD3_TIME_CON0 0x0f30
+#define DPHY_SD3_TIME_CON1 0x0f34
+#define DPHY_SD3_DESKEW_CON2 0x0f48
+
#define T_LP_EXIT_SKEW(x) FIELD_PREP(GENMASK(3, 2), x)
#define T_LP_ENTRY_SKEW(x) FIELD_PREP(GENMASK(1, 0), x)
#define T_HS_ZERO(x) FIELD_PREP(GENMASK(15, 8), x)
@@ -303,6 +360,7 @@ struct samsung_mipi_dcphy {
struct reset_control *apb_rst;
struct reset_control *grf_apb_rst;
unsigned int lanes;
+ unsigned long long hs_clk_rate;
struct samsung_mipi_dcphy_dir phys[2];
/* Serialises the two directions' access to the shared PHY state. */
struct mutex lock;
@@ -986,6 +1044,210 @@ struct samsung_mipi_dphy_timing samsung_mipi_dphy_timing_table[] = {
{ 80, 2, 0, 0, 28, 5, 0, 22, 2, 0, 5},
};
+/* D-PHY receiver HS-RX configuration lookup */
+struct samsung_mipi_dphy_rx_hsfreq_range {
+ u32 range_h_mbps;
+ u16 cfg_bit;
+};
+
+/*
+ * HS RX settle values taken from the rk3588 vendor kernel. The TRM
+ * defines these fields but does not publish the per-data-rate table
+ * (it defers to a separate timing-table supplement). Each cfg_bit is a
+ * pre-combined SETTLE_CLK_SEL | T_HS_SETTLE value written to the
+ * data-lane TIME_CON0. Sorted by .range_h_mbps ascending.
+ */
+static const struct samsung_mipi_dphy_rx_hsfreq_range samsung_mipi_dphy_rx_hsfreq_ranges[] = {
+ { 80, 0x105 }, { 100, 0x106 }, { 120, 0x107 }, { 140, 0x108 },
+ { 160, 0x109 }, { 180, 0x10a }, { 200, 0x10b }, { 220, 0x10c },
+ { 240, 0x10d }, { 270, 0x10e }, { 290, 0x10f }, { 310, 0x110 },
+ { 330, 0x111 }, { 350, 0x112 }, { 370, 0x113 }, { 390, 0x114 },
+ { 410, 0x115 }, { 430, 0x116 }, { 450, 0x117 }, { 470, 0x118 },
+ { 490, 0x119 }, { 510, 0x11a }, { 540, 0x11b }, { 560, 0x11c },
+ { 580, 0x11d }, { 600, 0x11e }, { 620, 0x11f }, { 640, 0x120 },
+ { 660, 0x121 }, { 680, 0x122 }, { 700, 0x123 }, { 720, 0x124 },
+ { 740, 0x125 }, { 760, 0x126 }, { 790, 0x127 }, { 810, 0x128 },
+ { 830, 0x129 }, { 850, 0x12a }, { 870, 0x12b }, { 890, 0x12c },
+ { 910, 0x12d }, { 930, 0x12e }, { 950, 0x12f }, { 970, 0x130 },
+ { 990, 0x131 }, { 1010, 0x132 }, { 1030, 0x133 }, { 1060, 0x134 },
+ { 1080, 0x135 }, { 1100, 0x136 }, { 1120, 0x137 }, { 1140, 0x138 },
+ { 1160, 0x139 }, { 1180, 0x13a }, { 1200, 0x13b }, { 1220, 0x13c },
+ { 1240, 0x13d }, { 1260, 0x13e }, { 1280, 0x13f }, { 1310, 0x140 },
+ { 1330, 0x141 }, { 1350, 0x142 }, { 1370, 0x143 }, { 1390, 0x144 },
+ { 1410, 0x145 }, { 1430, 0x146 }, { 1450, 0x147 }, { 1470, 0x148 },
+ { 1490, 0x149 }, { 1580, 0x007 }, { 1740, 0x008 }, { 1910, 0x009 },
+ { 2070, 0x00a }, { 2240, 0x00b }, { 2410, 0x00c }, { 2570, 0x00d },
+ { 2740, 0x00e }, { 2910, 0x00f }, { 3070, 0x010 }, { 3240, 0x011 },
+ { 3410, 0x012 }, { 3570, 0x013 }, { 3740, 0x014 }, { 3890, 0x015 },
+ { 4070, 0x016 }, { 4240, 0x017 }, { 4400, 0x018 }, { 4500, 0x019 },
+};
+
+static int samsung_mipi_dphy_rx_lookup_hsfreq(u32 lane_mbps, u16 *cfg_bit)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(samsung_mipi_dphy_rx_hsfreq_ranges); i++) {
+ if (lane_mbps <= samsung_mipi_dphy_rx_hsfreq_ranges[i].range_h_mbps) {
+ *cfg_bit = samsung_mipi_dphy_rx_hsfreq_ranges[i].cfg_bit;
+ return 0;
+ }
+ }
+
+ return -ERANGE;
+}
+
+/* SKEW_DLYSEL per data rate, from the RK3588 TRM ANA_CON2 description */
+static u32 samsung_mipi_dphy_rx_data_lane_dlysel(u32 lane_mbps)
+{
+ if (lane_mbps < 1500)
+ return 0;
+ if (lane_mbps < 2000)
+ return SKEW_DLYSEL(3);
+ if (lane_mbps < 3000)
+ return SKEW_DLYSEL(2);
+ if (lane_mbps < 4000)
+ return SKEW_DLYSEL(1);
+ return 0;
+}
+
+/*
+ * Per-data-lane register bases, indexed by lane number. The clock lane is
+ * programmed separately; its enable/settle registers do not follow this
+ * regular spacing.
+ */
+static const u32 rx_data_lane_gnr_con0[] = {
+ COMBO_SD0_GNR_CON0, COMBO_SD1_GNR_CON0,
+ COMBO_SD2_GNR_CON0, DPHY_SD3_GNR_CON0,
+};
+
+static const u32 rx_lane_time_con0[] = {
+ COMBO_SD0_TIME_CON0, COMBO_SD1_TIME_CON0,
+ COMBO_SD2_TIME_CON0, DPHY_SD3_TIME_CON0,
+};
+
+static const u32 rx_lane_time_con1[] = {
+ COMBO_SD0_TIME_CON1, COMBO_SD1_TIME_CON1,
+ COMBO_SD2_TIME_CON1, DPHY_SD3_TIME_CON1,
+};
+
+/*
+ * These RX analog tuning values come from the vendor BSP; the
+ * termination (RX_TERM_SW/HS_TERM_SW = 2, 96.6 ohm) additionally
+ * matches the TRM ANA_CON2 default.
+ */
+static void samsung_mipi_dphy_rx_config_common(struct samsung_mipi_dcphy *samsung)
+{
+ u32 dlysel = samsung_mipi_dphy_rx_data_lane_dlysel(div_u64(samsung->hs_clk_rate,
+ 1000000));
+ u32 ana_con2_common = dlysel | RX_TERM_SW(2);
+
+ /* Clock lane */
+ regmap_write(samsung->regmap, DPHY_SC_GNR_CON1, T_PHY_READY(0x1450));
+ regmap_write(samsung->regmap, DPHY_SC_ANA_CON1, HS_RX_BIAS_CON(0x10));
+ regmap_write(samsung->regmap, DPHY_SC_ANA_CON2, HS_TERM_SW(2));
+ regmap_write(samsung->regmap, DPHY_SC_ANA_CON3, ULPS_HYS_SW_DPHY(6));
+
+ /* Data lane 0 */
+ if (samsung->lanes > 0) {
+ regmap_write(samsung->regmap, COMBO_SD0_GNR_CON1, T_PHY_READY(0x1450));
+ regmap_write(samsung->regmap, COMBO_SD0_ANA_CON1, HS_RX_BIAS_CON(0x10));
+ regmap_write(samsung->regmap, COMBO_SD0_ANA_CON2, ana_con2_common);
+ regmap_write(samsung->regmap, COMBO_SD0_ANA_CON3,
+ ULPS_HYS_SW_DPHY(6) | LP_HYS_SW(3) | SEL_ESCPOL);
+ regmap_write(samsung->regmap, COMBO_SD0_ANA_CON7, CLK_DBL_CTRL(1));
+ regmap_write(samsung->regmap, COMBO_SD0_DESKEW_CON2, SKEW_CAL_CLK_COARSE_SET(0));
+ }
+
+ /* Data lane 1 */
+ if (samsung->lanes > 1) {
+ regmap_write(samsung->regmap, COMBO_SD1_GNR_CON1, T_PHY_READY(0x1450));
+ regmap_write(samsung->regmap, COMBO_SD1_ANA_CON1, HS_RX_BIAS_CON(0x10));
+ regmap_write(samsung->regmap, COMBO_SD1_ANA_CON2, ana_con2_common);
+ regmap_write(samsung->regmap, COMBO_SD1_ANA_CON3, ULPS_HYS_SW_DPHY(6));
+ regmap_write(samsung->regmap, COMBO_SD1_ANA_CON7, CLK_DBL_CTRL(1));
+ regmap_write(samsung->regmap, COMBO_SD1_DESKEW_CON2, SKEW_CAL_CLK_COARSE_SET(3));
+ }
+
+ /* Data lane 2 */
+ if (samsung->lanes > 2) {
+ regmap_write(samsung->regmap, COMBO_SD2_GNR_CON1, T_PHY_READY(0x1450));
+ regmap_write(samsung->regmap, COMBO_SD2_ANA_CON1, HS_RX_BIAS_CON(0x10));
+ regmap_write(samsung->regmap, COMBO_SD2_ANA_CON2, ana_con2_common);
+ regmap_write(samsung->regmap, COMBO_SD2_ANA_CON3, ULPS_HYS_SW_DPHY(6));
+ regmap_write(samsung->regmap, COMBO_SD2_ANA_CON7, CLK_DBL_CTRL(1));
+ regmap_write(samsung->regmap, COMBO_SD2_DESKEW_CON2, SKEW_CAL_CLK_COARSE_SET(3));
+ }
+
+ /* Data lane 3 */
+ if (samsung->lanes > 3) {
+ regmap_write(samsung->regmap, DPHY_SD3_GNR_CON1, T_PHY_READY(0x1450));
+ regmap_write(samsung->regmap, DPHY_SD3_ANA_CON1, HS_RX_BIAS_CON(0x10));
+ regmap_write(samsung->regmap, DPHY_SD3_ANA_CON2, ana_con2_common);
+ regmap_write(samsung->regmap, DPHY_SD3_ANA_CON3, ULPS_HYS_SW_DPHY(6));
+ regmap_write(samsung->regmap, DPHY_SD3_DESKEW_CON2, SKEW_CAL_CLK_COARSE_SET(3));
+ }
+}
+
+static int samsung_mipi_dphy_rx_config_settle(struct samsung_mipi_dcphy *samsung)
+{
+ u32 lane_mbps = div_u64(samsung->hs_clk_rate, 1000000);
+ u16 cfg_bit;
+ int ret, i;
+
+ ret = samsung_mipi_dphy_rx_lookup_hsfreq(lane_mbps, &cfg_bit);
+ if (ret) {
+ dev_err(samsung->dev,
+ "no RX hsfreq cfg for %u Mbps (limit ~4500 Mbps)\n",
+ lane_mbps);
+ return ret;
+ }
+
+ /*
+ * Clock-lane settle uses the fixed value from the TRM RX bring-up
+ * example, unlike the per-rate data-lane settle below.
+ */
+ regmap_write(samsung->regmap, DPHY_SC_TIME_CON0,
+ T_CLK_SETTLE(0x01) | T_CLK_MISS(0x03));
+
+ for (i = 0; i < samsung->lanes; i++) {
+ regmap_update_bits(samsung->regmap, rx_lane_time_con0[i],
+ T_HS_SETTLE(0xff) | SETTLE_CLK_SEL, cfg_bit);
+ regmap_update_bits(samsung->regmap, rx_lane_time_con1[i],
+ T_ERR_SOT_SYNC(0xff), T_ERR_SOT_SYNC(0x03));
+ }
+
+ return 0;
+}
+
+static int samsung_mipi_dphy_rx_lane_enable(struct samsung_mipi_dcphy *samsung)
+{
+ u32 sts;
+ int ret, i;
+
+ regmap_update_bits(samsung->regmap, DPHY_SC_GNR_CON0, PHY_ENABLE, PHY_ENABLE);
+ for (i = 0; i < samsung->lanes; i++)
+ regmap_update_bits(samsung->regmap, rx_data_lane_gnr_con0[i],
+ PHY_ENABLE, PHY_ENABLE);
+
+ ret = regmap_read_poll_timeout(samsung->regmap, DPHY_SC_GNR_CON0,
+ sts, sts & PHY_READY, 200, 4000);
+ if (ret) {
+ dev_err(samsung->dev, "RX clock lane not ready\n");
+ return ret;
+ }
+
+ for (i = 0; i < samsung->lanes; i++) {
+ ret = regmap_read_poll_timeout(samsung->regmap, rx_data_lane_gnr_con0[i],
+ sts, sts & PHY_READY, 200, 2000);
+ if (ret) {
+ dev_err(samsung->dev, "RX data lane %d not ready\n", i);
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
static void samsung_mipi_dcphy_bias_block_enable(struct samsung_mipi_dcphy *samsung)
{
regmap_write(samsung->regmap, BIAS_CON0, I_DEV_DIV_6 | I_RES_100_2UA);
@@ -1002,6 +1264,50 @@ static void samsung_mipi_dcphy_bias_block_enable(struct samsung_mipi_dcphy *sams
I_MUX_SEL_MASK, I_MUX_400MV);
}
+static int samsung_mipi_dphy_rx_power_on(struct samsung_mipi_dcphy *samsung,
+ struct phy *peer)
+{
+ bool first = peer->power_count == 0;
+ int ret;
+
+ if (!samsung->hs_clk_rate)
+ return -EINVAL;
+
+ reset_control_assert(samsung->s_phy_rst);
+
+ if (first)
+ samsung_mipi_dcphy_bias_block_enable(samsung);
+ samsung_mipi_dphy_rx_config_common(samsung);
+
+ ret = samsung_mipi_dphy_rx_config_settle(samsung);
+ if (ret)
+ goto out_deassert;
+
+ ret = samsung_mipi_dphy_rx_lane_enable(samsung);
+
+out_deassert:
+ reset_control_deassert(samsung->s_phy_rst);
+
+ return ret;
+}
+
+static int samsung_mipi_dphy_rx_power_off(struct samsung_mipi_dcphy *samsung)
+{
+ int i;
+
+ reset_control_assert(samsung->s_phy_rst);
+
+ regmap_update_bits(samsung->regmap, DPHY_SC_GNR_CON0, PHY_ENABLE, 0);
+ for (i = 0; i < samsung->lanes; i++)
+ regmap_update_bits(samsung->regmap, rx_data_lane_gnr_con0[i],
+ PHY_ENABLE, 0);
+
+ reset_control_deassert(samsung->s_phy_rst);
+ usleep_range(500, 1000);
+
+ return 0;
+}
+
static void samsung_mipi_dphy_lane_enable(struct samsung_mipi_dcphy *samsung)
{
regmap_write(samsung->regmap, DPHY_MC_GNR_CON1, T_PHY_READY(0x2000));
@@ -1404,7 +1710,7 @@ static int samsung_mipi_dcphy_power_on(struct phy *phy)
mutex_lock(&samsung->lock);
if (pd->dir == RK_DCPHY_DIR_RX)
- ret = -EOPNOTSUPP;
+ ret = samsung_mipi_dphy_rx_power_on(samsung, peer);
else
ret = samsung_mipi_dphy_tx_power_on(samsung, peer);
mutex_unlock(&samsung->lock);
@@ -1421,11 +1727,11 @@ static int samsung_mipi_dcphy_power_off(struct phy *phy)
if (pd->type != PHY_TYPE_DPHY)
return -EOPNOTSUPP;
- if (pd->dir == RK_DCPHY_DIR_RX)
- return -EOPNOTSUPP;
-
mutex_lock(&samsung->lock);
- ret = samsung_mipi_dphy_tx_power_off(samsung);
+ if (pd->dir == RK_DCPHY_DIR_RX)
+ ret = samsung_mipi_dphy_rx_power_off(samsung);
+ else
+ ret = samsung_mipi_dphy_tx_power_off(samsung);
mutex_unlock(&samsung->lock);
return ret;
@@ -1525,13 +1831,17 @@ static int samsung_mipi_dcphy_configure(struct phy *phy,
struct samsung_mipi_dcphy *samsung = pd->parent;
unsigned long long target_rate = opts->mipi_dphy.hs_clk_rate;
+ mutex_lock(&samsung->lock);
samsung->lanes = opts->mipi_dphy.lanes > 4 ? 4 : opts->mipi_dphy.lanes;
- if (pd->dir == RK_DCPHY_DIR_RX)
- return -EOPNOTSUPP;
-
- samsung_mipi_dcphy_pll_calc_rate(samsung, target_rate);
- opts->mipi_dphy.hs_clk_rate = samsung->pll.rate;
+ if (pd->dir == RK_DCPHY_DIR_RX) {
+ /* The sensor supplies the link clock; the PLL stays off. */
+ samsung->hs_clk_rate = target_rate;
+ } else {
+ samsung_mipi_dcphy_pll_calc_rate(samsung, target_rate);
+ opts->mipi_dphy.hs_clk_rate = samsung->pll.rate;
+ }
+ mutex_unlock(&samsung->lock);
return 0;
}
--
2.43.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* Re: [PATCH v3 phy-next 2/8] soc: fsl: guts: add a global structure to hold state
From: Vladimir Oltean @ 2026-07-21 10:43 UTC (permalink / raw)
To: Michael Walle
Cc: devicetree, robh, conor+dt, vkoul, neil.armstrong, olteanv,
linux-phy
In-Reply-To: <DK44809N7Y8I.J2Z3U4N32H0Q@kernel.org>
Hi Michael,
On Tue, Jul 21, 2026 at 10:44:30AM +0200, Michael Walle wrote:
> Hi Vladimir,
>
> On Mon Jul 20, 2026 at 4:57 PM CEST, Vladimir Oltean wrote:
> > On Mon, Jul 20, 2026 at 05:01:39PM +0300, Vladimir Oltean wrote:
> >> On Mon, Jul 20, 2026 at 01:50:49PM +0000, sashiko-bot@kernel.org wrote:
> >> > [Severity: High]
> >> > Will jumping to err_nomem cause a NULL pointer dereference?
> >> >
> >> > If the allocation of soc_dev_attr fails, the code now jumps to the error
> >> > handling path which dereferences soc_dev_attr to free its members:
> >> >
> >> > > @@ -269,6 +271,8 @@ static int __init fsl_guts_init(void)
> >> > > kfree(soc_dev_attr->revision);
> >> > > kfree(soc_dev_attr->serial_number);
> >> >
> >> > Since soc_dev_attr is NULL when this allocation fails, won't evaluating
> >> > soc_dev_attr->revision and other members result in a kernel panic?
> >>
> >> yeah.
> >>
> >> I tried to be nice and play with the existing "err_nomem" and "err"
> >> error handling labels, but the truth is that a single error cleanup path
> >> is just an antipattern that doesn't scale as code becomes more complex.
> >>
> >> Instead of having a single error handling procedure which needs to
> >> carefully fend off from various invalid contexts from all the goto
> >> sites, the standard convention is to have an incremental set of labels
> >> which undo just the setup that was done up to the goto jump site.
> >>
> >> I'll refactor fsl_guts_init() to use that convention for v4.
> >
> > While I was doing this refactoring, I noticed that this code path:
> >
> > if (soc_data)
> > soc_uid = fsl_guts_get_soc_uid(soc_data->sfp_compat,
> > soc_data->uid_offset);
> > if (soc_uid) {
> > soc_dev_attr->serial_number = kasprintf(GFP_KERNEL, "%016llX",
> > soc_uid);
> > // no kasprintf() NULL return code check here, unlike
> > // the rest of the code
>
> It's a long time ago, but it seems to be designed be optional. In
> that case a NULL pointer check was omitted, because nothing bad can
> happen anyway. Either you'll free the string if != NULL or you don't
> free anything.
>
> > }
> >
> > soc_dev = soc_device_register(soc_dev_attr);
> > if (IS_ERR(soc_dev)) {
> > ret = PTR_ERR(soc_dev);
> > goto err;
> > }
> >
> > proceeds with soc_device_register() even if fsl_guts_get_soc_uid() was
> > able to get a soc unique id from the security fuse processor, but we
> > fail to print that string to the soc_dev_attr->serial_number variable.
> >
> > I don't see anywhere mentioned in commit 786dde1e59d7 ("soc: fsl: guts:
> > add serial_number support") that this would be intentional, so it can
> > just as well be an omission.
> >
> > Is it OK if I replace the silent failure with a loud failure of the
> > entire fsl_guts_init()? The code executes only if soc_uid is non-zero
> > anyway.
> >
> > kasprintf() fails only for memory related reasons.
>
> I don't have a strong opinion. If you ever get that error, something
> is seriously bad anyway.
Thanks for responding.
I think the optionality is already taken care of by the "if (soc_uid)" check.
My intuition is that if the guts driver decides to try and populate the
soc_dev_attr->serial_number attribute, it should try and run this to
completion.
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH 1/4] dt-bindings: phy: rockchip,rk3588-mipi-dcphy: support per-direction phys
From: sashiko-bot @ 2026-07-21 10:44 UTC (permalink / raw)
To: Jason Yang
Cc: devicetree, linux-phy, neil.armstrong, vkoul, robh, conor+dt,
olteanv
In-Reply-To: <20260721-dcphy-rx-v1-v1-1-4fc83c0ccac9@gmail.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The `#phy-cells` property uses `enum: [1, 2]` instead of a `const` constraint, which violates subsystem guidelines and creates a fundamentally broken device tree parsing design.
- [High] The commit introduces a 2-cell format for `#phy-cells` to support independent TX/RX consumers, but the PHY driver actively rejects anything other than 1 cell, resulting in probe failures.
--
--- Patch [1]: [PATCH 1/4] dt-bindings: phy: rockchip,rk3588-mipi-dcphy: support per-direction phys ---
commit 0077b5ec50ff5c6cc10afd337f193a9f090d6946
Author: Jason Yang <jason98166@gmail.com>
dt-bindings: phy: rockchip,rk3588-mipi-dcphy: support per-direction phys
The Samsung MIPI D-/C-PHY block on RK3588 drives a MIPI DSI transmitter
and, on the same PHY, can receive a MIPI CSI camera. Describing both as
independent consumers of one PHY requires the direction to be expressed
in the devicetree.
> diff --git a/Documentation/devicetree/bindings/phy/rockchip,rk3588-mipi-dcphy.yaml b/Documentation/devicetree/bindings/phy/rockchip,rk3588-mipi-dcphy.yaml
> index c8ff5ba22a86b..53d6951375667 100644
> --- a/Documentation/devicetree/bindings/phy/rockchip,rk3588-mipi-dcphy.yaml
> +++ b/Documentation/devicetree/bindings/phy/rockchip,rk3588-mipi-dcphy.yaml
> @@ -20,12 +20,22 @@ properties:
> maxItems: 1
>
> "#phy-cells":
> - const: 1
> + enum: [1, 2]
[Severity: High]
Does this violate the subsystem requirement that cell-count properties must
use a const constraint?
If a board developer updates the PHY provider node to #phy-cells = <2> to use
the new CSI consumer, will any existing DSI consumer relying on 1-cell parsing
fail to probe?
The function of_parse_phandle_with_args() strictly enforces that the consumer's
argument count exactly matches the provider's #phy-cells count.
Additionally, it looks like the driver hasn't been updated to support this yet.
In drivers/phy/rockchip/phy-rockchip-samsung-dcphy.c, the translation callback
samsung_mipi_dcphy_xlate() still explicitly checks for exactly 1 cell:
if (args->args_count != 1) {
dev_err(dev, "invalid number of arguments\n");
return ERR_PTR(-EINVAL);
}
Will this cause any device tree attempting to use the newly documented 2-cell
format to fail to initialize at runtime?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260721-dcphy-rx-v1-v1-0-4fc83c0ccac9@gmail.com?part=1
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH 3/4] phy: rockchip-samsung-dcphy: model TX and RX as separate PHYs
From: sashiko-bot @ 2026-07-21 10:44 UTC (permalink / raw)
To: Jason Yang
Cc: conor+dt, neil.armstrong, olteanv, vkoul, devicetree, robh,
linux-phy
In-Reply-To: <20260721-dcphy-rx-v1-v1-3-4fc83c0ccac9@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Concurrent or unsupported PHY configuration clobbers the shared `samsung->lanes` driver state before verifying the direction is supported.
--
--- Patch [3]: [PATCH 3/4] phy: rockchip-samsung-dcphy: model TX and RX as separate PHYs ---
commit 242b9435d09e0133de4658156f3cc224aa2f84c9
Author: Jason Yang <jason98166@gmail.com>
phy: rockchip-samsung-dcphy: model TX and RX as separate PHYs
This commit models the MIPI DSI transmitter and MIPI CSI receiver as separate
PHYs that share the same hardware block. It registers one struct phy per
direction and adds a per-provider mutex to serialize access to the shared
state.
> diff --git a/drivers/phy/rockchip/phy-rockchip-samsung-dcphy.c b/drivers/phy/rockchip/phy-rockchip-samsung-dcphy.c
> --- a/drivers/phy/rockchip/phy-rockchip-samsung-dcphy.c
> +++ b/drivers/phy/rockchip/phy-rockchip-samsung-dcphy.c
[ ... ]
> @@ -1488,11 +1521,15 @@ samsung_mipi_dcphy_pll_calc_rate(struct samsung_mipi_dcphy *samsung,
> static int samsung_mipi_dcphy_configure(struct phy *phy,
> union phy_configure_opts *opts)
> {
> - struct samsung_mipi_dcphy *samsung = phy_get_drvdata(phy);
> + struct samsung_mipi_dcphy_dir *pd = phy_get_drvdata(phy);
> + struct samsung_mipi_dcphy *samsung = pd->parent;
> unsigned long long target_rate = opts->mipi_dphy.hs_clk_rate;
>
> samsung->lanes = opts->mipi_dphy.lanes > 4 ? 4 : opts->mipi_dphy.lanes;
>
> + if (pd->dir == RK_DCPHY_DIR_RX)
> + return -EOPNOTSUPP;
> +
[Severity: High]
Does this code modify the shared samsung->lanes state before checking if
the direction is supported?
Since this is done without holding samsung->lock, can a consumer
configuring the RX PHY clobber the active TX PHY's lane count?
If the lane count is corrupted, it looks like it could lead to incorrect lane
power-down sequences when the TX PHY is disabled later, potentially leaving
MIPI lanes actively driven.
Should the pd->dir check and the lock acquisition happen before updating
samsung->lanes?
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260721-dcphy-rx-v1-v1-0-4fc83c0ccac9@gmail.com?part=3
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v13 1/2] dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema
From: Bryan O'Donoghue @ 2026-07-21 10:47 UTC (permalink / raw)
To: Vladimir Zapolskiy, Bryan O'Donoghue, Vinod Koul,
Kishon Vijay Abraham I, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Neil Armstrong
Cc: linux-arm-msm, linux-phy, linux-media, devicetree, linux-kernel,
Krzysztof Kozlowski
In-Reply-To: <345ca98e-09f6-4965-87bb-e64dac096498@linaro.org>
On 21/07/2026 10:56, Vladimir Zapolskiy wrote:
> On 7/21/26 12:39, Bryan O'Donoghue wrote:
>> On 21/07/2026 09:17, Vladimir Zapolskiy wrote:
>>>> + "#phy-cells":
>>>> + const: 1
>>>> + description:
>>>> + The single cell specifies the PHY operating mode.
>>> This has been reported before, #phy-cells shall be 0, the operation
>>> mode is selected by bus-type of endpoints as it is described in
>>> video-interfaces.yaml
>>>
>>> Two competing values are redundant and it opens a door to a wrong
>>> hardware description, the technical discussion has not been started yet.
>> So as I said before, I'm following Rob Herring's input on this and
>> Krzysztof has given RB for this binding. At this stage, I don't see
>> scope to change it.
>>
>> Adding endpoints was already a compromise to address your feedback,
>> which I hoped would bring you on board with the design.
> That's my comment dated March 27, 2026, and it says moving phy type
> from phy cell to the endpoint property excludes the need to set any
> phy cells:
>
> https://lore.kernel.org/linux-arm-msm/e37ce438-12c7-462d-
> b2bc-1351be62b806@linaro.org/
>
> The technical comment is left unresponded so far since then.
>
> Since there is no expected phy cells to get on consumer's side, and
> since there is another link between CSIPHY and CSID described in
> video-interfaces.yaml, any kind of dependency of this IP on phy can
> be just removed, which serves the purpose of hardware description
> simplification.
Rob's v1 feedback was consumer-decides-mode via the cell; Krzysztof has
R-b'd the binding with cells=1.
I'm happy to lock down this binding as-is on that basis.
Are you arguing about phy-cells or are you actually arguing the entire
instantiation of Qualcomm MIPI CSI2 as a standalone generic driver in
drivers/phy ? Its not the same debate at all.
As the CAMSS maintainer, I'm fully against adding more inline monolithic
CSIPHY init sequences. That could be represented as a separate
linux-media driver but, then that implies that _all_ upstream CSIPHY
drivers in drivers/phy are wrong, and I just don't accept that.
The bindings have DT maintainer RB, and the driver has Qcom engineer
tested-by, review-by.
Vinod, Kishon - Again, I'd request this merges as-is.
---
bod
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH 4/4] phy: rockchip-samsung-dcphy: add MIPI D-PHY receiver support
From: sashiko-bot @ 2026-07-21 10:57 UTC (permalink / raw)
To: Jason Yang
Cc: linux-phy, conor+dt, neil.armstrong, vkoul, robh, olteanv,
devicetree
In-Reply-To: <20260721-dcphy-rx-v1-v1-4-4fc83c0ccac9@gmail.com>
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
New issues:
- [High] The RX power-on sequence fails to toggle the `apb_rst` reset line, potentially leaving the shared APB bus interface uninitialized if the RX direction is powered on before TX.
- [High] If `samsung_mipi_dphy_rx_lane_enable()` fails (e.g., due to a PHY_READY timeout), it returns an error but leaves the `PHY_ENABLE` bits set in the PHY registers and unconditionally deasserts the `s_phy_rst` core reset.
- [High] The `lanes` configuration field is shared between TX and RX, leading to state corruption and incorrect lane cleanup if both directions are configured independently.
Pre-existing issues:
- [High] Concurrent power_on calls for TX and RX can race due to a time-of-check to time-of-use (TOCTOU) vulnerability on the unprotected `peer->power_count`.
--
--- Patch [4]: [PATCH 4/4] phy: rockchip-samsung-dcphy: add MIPI D-PHY receiver support ---
commit 1a7ff981a5098deb181df94fdb6e07f8dc4040a9
Author: Jason Yang <jason98166@gmail.com>
phy: rockchip-samsung-dcphy: add MIPI D-PHY receiver support
This commit implements the receiver (CSI) direction of the D-PHY. It adds the
RX register layout, the HS-frequency settle table, and hooks the receiver
bring-up and teardown into the per-direction power and configuration paths.
> diff --git a/drivers/phy/rockchip/phy-rockchip-samsung-dcphy.c b/drivers/phy/rockchip/phy-rockchip-samsung-dcphy.c
> --- a/drivers/phy/rockchip/phy-rockchip-samsung-dcphy.c
> +++ b/drivers/phy/rockchip/phy-rockchip-samsung-dcphy.c
[ ... ]
> +static int samsung_mipi_dphy_rx_lane_enable(struct samsung_mipi_dcphy *samsung)
> +{
> + u32 sts;
> + int ret, i;
> +
> + regmap_update_bits(samsung->regmap, DPHY_SC_GNR_CON0, PHY_ENABLE, PHY_ENABLE);
> + for (i = 0; i < samsung->lanes; i++)
> + regmap_update_bits(samsung->regmap, rx_data_lane_gnr_con0[i],
> + PHY_ENABLE, PHY_ENABLE);
> +
> + ret = regmap_read_poll_timeout(samsung->regmap, DPHY_SC_GNR_CON0,
> + sts, sts & PHY_READY, 200, 4000);
> + if (ret) {
> + dev_err(samsung->dev, "RX clock lane not ready\n");
> + return ret;
> + }
[Severity: High]
If regmap_read_poll_timeout() fails here, do we leak the previously set
PHY_ENABLE bits?
Returning early leaves the PHY_ENABLE bits set in the PHY registers.
The caller catches the error and propagates it, meaning the framework
will not call power_off() to clean up. On subsequent retries, writing
PHY_ENABLE to a register where it is already 1 will fail to produce the
0 -> 1 edge transition required by the PHY state machine to start, breaking
future attempts to initialize the RX lanes.
[ ... ]
> +static int samsung_mipi_dphy_rx_power_on(struct samsung_mipi_dcphy *samsung,
> + struct phy *peer)
> +{
> + bool first = peer->power_count == 0;
[Severity: High]
This is a pre-existing issue, but is it safe to rely on peer->power_count
here to determine if shared initialization is needed?
The PHY core framework increments phy->power_count outside the driver's lock,
only after the power_on() callback returns. If Thread A (RX) and Thread B
(TX) call phy_power_on() concurrently:
Thread A configures RX, drops samsung->lock, but is preempted before
rx_phy->power_count is incremented.
Thread B acquires samsung->lock, sees rx_phy->power_count is still 0, and
executes its 'first' initialization sequence.
For TX, this involves explicitly asserting the shared APB bus reset
(apb_rst), which would wipe out the entire RX configuration Thread A just
applied. Should the driver track the initialization state internally instead
of relying on power_count?
> + int ret;
> +
> + if (!samsung->hs_clk_rate)
> + return -EINVAL;
> +
> + reset_control_assert(samsung->s_phy_rst);
> +
> + if (first)
> + samsung_mipi_dcphy_bias_block_enable(samsung);
[Severity: High]
If the PHY is used in a receive-first or receive-only role, does this sequence
omit the apb_rst hardware initialization for the shared APB bus?
Looking at the TX power-on sequence, it toggles apb_rst when first is true to
ensure the shared APB bus interface is properly initialized. If RX powers on
first, the apb_rst toggle is skipped here, and a subsequent TX power-on will
see first == false and also skip it. Can this leave the shared APB bus
interface uninitialized and potentially cause system hangs during regmap
accesses?
> + samsung_mipi_dphy_rx_config_common(samsung);
> +
> + ret = samsung_mipi_dphy_rx_config_settle(samsung);
> + if (ret)
> + goto out_deassert;
> +
> + ret = samsung_mipi_dphy_rx_lane_enable(samsung);
> +
> +out_deassert:
> + reset_control_deassert(samsung->s_phy_rst);
> +
> + return ret;
> +}
[Severity: High]
Does this unconditionally commit the leaked PHY_ENABLE state to the hardware?
If samsung_mipi_dphy_rx_lane_enable() fails due to the timeout mentioned
earlier, it drops through to here and deasserts s_phy_rst. Combined with the
missing PHY_ENABLE cleanup, this commits the broken hardware state.
[ ... ]
> @@ -1525,13 +1831,17 @@ static int samsung_mipi_dcphy_configure(struct phy *phy,
> struct samsung_mipi_dcphy *samsung = pd->parent;
> unsigned long long target_rate = opts->mipi_dphy.hs_clk_rate;
>
> + mutex_lock(&samsung->lock);
> samsung->lanes = opts->mipi_dphy.lanes > 4 ? 4 : opts->mipi_dphy.lanes;
[Severity: High]
Does sharing the samsung->lanes variable between TX and RX lead to state
corruption?
Since the driver models TX and RX as two independent PHY instances, either
instance will blindly overwrite samsung->lanes here. If a consumer configures
TX with 4 lanes and a second consumer configures RX with 2 lanes,
samsung->lanes becomes 2. A subsequent TX power_off() will read this state
and only disable 2 lanes, leaking the other 2 active TX lanes. Should lanes
be tracked per-direction?
> - if (pd->dir == RK_DCPHY_DIR_RX)
> - return -EOPNOTSUPP;
> -
> - samsung_mipi_dcphy_pll_calc_rate(samsung, target_rate);
> - opts->mipi_dphy.hs_clk_rate = samsung->pll.rate;
> + if (pd->dir == RK_DCPHY_DIR_RX) {
> + /* The sensor supplies the link clock; the PLL stays off. */
> + samsung->hs_clk_rate = target_rate;
> + } else {
> + samsung_mipi_dcphy_pll_calc_rate(samsung, target_rate);
> + opts->mipi_dphy.hs_clk_rate = samsung->pll.rate;
> + }
> + mutex_unlock(&samsung->lock);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260721-dcphy-rx-v1-v1-0-4fc83c0ccac9@gmail.com?part=4
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v13 1/2] dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema
From: Vladimir Zapolskiy @ 2026-07-21 11:05 UTC (permalink / raw)
To: Bryan O'Donoghue, Bryan O'Donoghue, Vinod Koul,
Kishon Vijay Abraham I, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Neil Armstrong
Cc: linux-arm-msm, linux-phy, linux-media, devicetree, linux-kernel,
Krzysztof Kozlowski
In-Reply-To: <c236cf94-a775-4158-88db-9a337c7fa862@kernel.org>
On 7/21/26 13:47, Bryan O'Donoghue wrote:
> On 21/07/2026 10:56, Vladimir Zapolskiy wrote:
>> On 7/21/26 12:39, Bryan O'Donoghue wrote:
>>> On 21/07/2026 09:17, Vladimir Zapolskiy wrote:
>>>>> + "#phy-cells":
>>>>> + const: 1
>>>>> + description:
>>>>> + The single cell specifies the PHY operating mode.
>>>> This has been reported before, #phy-cells shall be 0, the operation
>>>> mode is selected by bus-type of endpoints as it is described in
>>>> video-interfaces.yaml
>>>>
>>>> Two competing values are redundant and it opens a door to a wrong
>>>> hardware description, the technical discussion has not been started yet.
>>> So as I said before, I'm following Rob Herring's input on this and
>>> Krzysztof has given RB for this binding. At this stage, I don't see
>>> scope to change it.
>>>
>>> Adding endpoints was already a compromise to address your feedback,
>>> which I hoped would bring you on board with the design.
>> That's my comment dated March 27, 2026, and it says moving phy type
>> from phy cell to the endpoint property excludes the need to set any
>> phy cells:
>>
>> https://lore.kernel.org/linux-arm-msm/e37ce438-12c7-462d-
>> b2bc-1351be62b806@linaro.org/
>>
>> The technical comment is left unresponded so far since then.
>>
>> Since there is no expected phy cells to get on consumer's side, and
>> since there is another link between CSIPHY and CSID described in
>> video-interfaces.yaml, any kind of dependency of this IP on phy can
>> be just removed, which serves the purpose of hardware description
>> simplification.
>
> Rob's v1 feedback was consumer-decides-mode via the cell; Krzysztof has
> R-b'd the binding with cells=1.
Unfortunately this is not a technical discussion to move forward
a better hardware description.
> I'm happy to lock down this binding as-is on that basis.
>
> Are you arguing about phy-cells or are you actually arguing the entire
> instantiation of Qualcomm MIPI CSI2 as a standalone generic driver in
> drivers/phy ? Its not the same debate at all.
So far I don't review the driver, only CSIPHY IP hardware description.
In the current version of the hardware description one may find that
the exploited 'video-intefaces' interface completely covers the 'phys'
interface between CSIPHY and CSID, quite probably both are not needed
at the same time, and the simpler 'phys' can be removed without any
losses. But if it is kept, then its cell value is anyway redundant.
> As the CAMSS maintainer, I'm fully against adding more inline monolithic
> CSIPHY init sequences. That could be represented as a separate
> linux-media driver but, then that implies that _all_ upstream CSIPHY
> drivers in drivers/phy are wrong, and I just don't accept that.
Other two drivers are different and should be excluded as good
references, both of them links exactly one sensor to exactly one
CSI-2 decoder, it's not the case here. Also one of these two drivers
has no upstream users.
> The bindings have DT maintainer RB, and the driver has Qcom engineer
> tested-by, review-by.
>
> Vinod, Kishon - Again, I'd request this merges as-is.
>
--
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 v13 1/2] dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema
From: Bryan O'Donoghue @ 2026-07-21 11:24 UTC (permalink / raw)
To: Vladimir Zapolskiy, Bryan O'Donoghue, Vinod Koul,
Kishon Vijay Abraham I, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Neil Armstrong
Cc: linux-arm-msm, linux-phy, linux-media, devicetree, linux-kernel,
Krzysztof Kozlowski
In-Reply-To: <e8bc8ade-d699-497e-8c4a-fbdc03fcc549@linaro.org>
On 21/07/2026 12:05, Vladimir Zapolskiy wrote:
> On 7/21/26 13:47, Bryan O'Donoghue wrote:
>> On 21/07/2026 10:56, Vladimir Zapolskiy wrote:
>>> On 7/21/26 12:39, Bryan O'Donoghue wrote:
>>>> On 21/07/2026 09:17, Vladimir Zapolskiy wrote:
>>>>>> + "#phy-cells":
>>>>>> + const: 1
>>>>>> + description:
>>>>>> + The single cell specifies the PHY operating mode.
>>>>> This has been reported before, #phy-cells shall be 0, the operation
>>>>> mode is selected by bus-type of endpoints as it is described in
>>>>> video-interfaces.yaml
>>>>>
>>>>> Two competing values are redundant and it opens a door to a wrong
>>>>> hardware description, the technical discussion has not been started
>>>>> yet.
>>>> So as I said before, I'm following Rob Herring's input on this and
>>>> Krzysztof has given RB for this binding. At this stage, I don't see
>>>> scope to change it.
>>>>
>>>> Adding endpoints was already a compromise to address your feedback,
>>>> which I hoped would bring you on board with the design.
>>> That's my comment dated March 27, 2026, and it says moving phy type
>>> from phy cell to the endpoint property excludes the need to set any
>>> phy cells:
>>>
>>> https://lore.kernel.org/linux-arm-msm/e37ce438-12c7-462d-
>>> b2bc-1351be62b806@linaro.org/
>>>
>>> The technical comment is left unresponded so far since then.
>>>
>>> Since there is no expected phy cells to get on consumer's side, and
>>> since there is another link between CSIPHY and CSID described in
>>> video-interfaces.yaml, any kind of dependency of this IP on phy can
>>> be just removed, which serves the purpose of hardware description
>>> simplification.
>>
>> Rob's v1 feedback was consumer-decides-mode via the cell; Krzysztof has
>> R-b'd the binding with cells=1.
>
> Unfortunately this is not a technical discussion to move forward
> a better hardware description.
>
>> I'm happy to lock down this binding as-is on that basis.
>>
>> Are you arguing about phy-cells or are you actually arguing the entire
>> instantiation of Qualcomm MIPI CSI2 as a standalone generic driver in
>> drivers/phy ? Its not the same debate at all.
>
> So far I don't review the driver, only CSIPHY IP hardware description.
>
> In the current version of the hardware description one may find that
> the exploited 'video-intefaces' interface completely covers the 'phys'
> interface between CSIPHY and CSID, quite probably both are not needed
> at the same time, and the simpler 'phys' can be removed without any
> losses. But if it is kept, then its cell value is anyway redundant.
>
>> As the CAMSS maintainer, I'm fully against adding more inline monolithic
>> CSIPHY init sequences. That could be represented as a separate
>> linux-media driver but, then that implies that _all_ upstream CSIPHY
>> drivers in drivers/phy are wrong, and I just don't accept that.
>
> Other two drivers are different and should be excluded as good
> references, both of them links exactly one sensor to exactly one
> CSI-2 decoder, it's not the case here. Also one of these two drivers
> has no upstream users.
>
>> The bindings have DT maintainer RB, and the driver has Qcom engineer
>> tested-by, review-by.
>>
>> Vinod, Kishon - Again, I'd request this merges as-is.
>>
>
The graph and phys are different layers, endpoints describe the
media topology, phys is how CSID controls the PHY through the PHY
framework (phy_get/configure/power_on).
Removing phys doesn't simplify the description, it removes the control
interface which is the monolith again.
We have multiple examples of this model upstream, cdns-dphy-rx +
ti-csi2rx in-tree, with users.
I'm still kicking this up to Vinod, Kishon, request stands.
Multiple DT and driver users are RB/Tested-by on the schema and driver,
lots of other work is gated.
It is time to merge.
Please adjudicate.
---
bod
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v13 2/2] phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver
From: Vladimir Zapolskiy @ 2026-07-21 11:30 UTC (permalink / raw)
To: Bryan O'Donoghue, Vinod Koul, Kishon Vijay Abraham I,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Neil Armstrong
Cc: Bryan O'Donoghue, linux-arm-msm, linux-phy, linux-media,
devicetree, linux-kernel
In-Reply-To: <20260720-x1e-csi2-phy-v13-2-160c31958863@linaro.org>
On 7/20/26 04:11, Bryan O'Donoghue wrote:
> Add a new MIPI CSI2 driver in DPHY mode initially. The entire set of
> existing CAMSS CSI PHY init sequences are imported in order to save time
> and effort in later patches.
>
> The following devices are supported in this drop:
> "qcom,x1e80100-csi2-phy"
>
> In-line with other PHY drivers the process node is included in the name.
> Data-lane and clock lane positioning and polarity selection via newly
> amended struct phy_configure_opts_mipi_dphy{} is supported.
>
> The Qualcomm 3PH class of PHYs can do both DPHY and CPHY mode. For now only
> DPHY is supported.
>
> In porting some of the logic over from camss-csiphy*.c to here its also
> possible to rationalise some of the code.
>
> In particular use of regulator_bulk and clk_bulk as well as dropping the
> seemingly useless and unused interrupt handler.
>
> The PHY sequences and a lot of the logic that goes with them are well
> proven in CAMSS and mature so the main thing to watch out for here is how
> to get the right sequencing of regulators, clocks and register-writes.
>
> The register init sequence table is imported verbatim from the existing
> CAMSS csiphy driver. A follow-up series will rework the table to extract
> the repetitive per-lane pattern into a loop.
>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
<snip>
> diff --git a/drivers/phy/qualcomm/phy-qcom-mipi-csi2.h b/drivers/phy/qualcomm/phy-qcom-mipi-csi2.h
> new file mode 100644
> index 0000000000000..7e55ae0073704
> --- /dev/null
> +++ b/drivers/phy/qualcomm/phy-qcom-mipi-csi2.h
> @@ -0,0 +1,97 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + *
> + * Qualcomm MIPI CSI2 CPHY/DPHY driver
> + *
> + * Copyright (C) 2025 Linaro Ltd.
> + */
> +#ifndef __PHY_QCOM_MIPI_CSI2_H__
> +#define __PHY_QCOM_MIPI_CSI2_H__
> +
> +#include <linux/phy/phy.h>
> +
> +#define CSI2_MAX_DATA_LANES 4
> +#define CSI2_DEFAULT_CLK_LANE 7
There is no 7 lanes on CSIPHY, this value is bogus, also it was reported
previously.
The 'data-lanes' handling is broken in this version of the driver, since
it does not follow the description from included video-interfaces.yaml.
> +
> +struct mipi_csi2phy_lane {
> + u8 pos;
> + u8 pol;
> +};
> +
> +struct mipi_csi2phy_lanes_cfg {
> + struct mipi_csi2phy_lane data[CSI2_MAX_DATA_LANES];
> + struct mipi_csi2phy_lane clk;
> +};
> +
> +struct mipi_csi2phy_stream_cfg {
> + s64 link_freq;
> + u8 num_data_lanes;
> + struct mipi_csi2phy_lanes_cfg lane_cfg;
> +};
> +
> +struct mipi_csi2phy_device;
> +
> +struct mipi_csi2phy_hw_ops {
> + void (*hw_version_read)(struct mipi_csi2phy_device *csi2phy_dev);
> + void (*reset)(struct mipi_csi2phy_device *csi2phy_dev);
> + int (*lanes_enable)(struct mipi_csi2phy_device *csi2phy_dev,
> + struct mipi_csi2phy_stream_cfg *cfg);
> + void (*lanes_disable)(struct mipi_csi2phy_device *csi2phy_dev,
> + struct mipi_csi2phy_stream_cfg *cfg);
> +};
> +
> +struct mipi_csi2phy_lane_regs {
> + const s32 reg_addr;
> + const s32 reg_data;
> + const u32 delay_us;
> + const u32 param_type;
> +};
> +
> +struct mipi_csi2phy_device_regs {
> + const struct mipi_csi2phy_lane_regs *init_seq;
> + const int lane_array_size;
> + const u32 common_regs_offset;
> +};
> +
> +struct mipi_csi2_genpd {
> + const char *name;
> + bool scaled;
> +};
> +
> +struct mipi_csi2phy_soc_cfg {
> + const struct mipi_csi2phy_hw_ops *ops;
> + const struct mipi_csi2phy_device_regs reg_info;
> +
> + const char ** const supply_names;
> + const unsigned int num_supplies;
> +
> + const char ** const clk_names;
> + const unsigned int num_clk;
> +
> + const struct mipi_csi2_genpd *genpds;
> + const unsigned int num_genpds;
> +};
> +
> +struct mipi_csi2phy_device {
> + struct device *dev;
> + u8 phy_mode;
> +
> + struct phy *phy;
> + void __iomem *base;
> +
> + struct clk_bulk_data *clks;
> + struct clk *timer_clk;
> + u32 timer_clk_rate;
> +
> + struct regulator_bulk_data *supplies;
> + struct dev_pm_domain_list *pd_list;
> +
> + const struct mipi_csi2phy_soc_cfg *soc_cfg;
> + struct mipi_csi2phy_stream_cfg stream_cfg;
> +
> + u32 hw_version;
> +};
> +
> +extern const struct mipi_csi2phy_soc_cfg mipi_csi2_dphy_4nm_x1e;
> +
> +#endif /* __PHY_QCOM_MIPI_CSI2_H__ */
>
--
Best wishes,
Vladimir
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* [PATCH v6 1/7] dt-bindings: phy: qcom,x1e80100-csi2-phy: Add SM8750 CSI2 PHY
From: Hangxiang Ma @ 2026-07-21 12:46 UTC (permalink / raw)
To: Robert Foss, Todor Tomov, Bryan O'Donoghue,
Vladimir Zapolskiy, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bryan O'Donoghue,
Loic Poulain, Vinod Koul, Neil Armstrong
Cc: linux-media, linux-arm-msm, devicetree, linux-kernel, linux-phy,
jeyaprakash.soundrapandian, Vijay Kumar Tumati, Hangxiang Ma
In-Reply-To: <20260721-add-support-for-camss-on-sm8750-v6-0-a5f841061d18@oss.qualcomm.com>
The SM8750 camera subsystem exposes six CSI2 D-PHY blocks that follow the
same programming model as Kaanapali. Describe the SM8750 CSI2 PHY using a
"qcom,sm8750-csi2-phy" compatible with the "qcom,kaanapali-csi2-phy"
fallback, so the CAMSS binding can instantiate the CSIPHY nodes as
separate phy devices.
Signed-off-by: Hangxiang Ma <hangxiang.ma@oss.qualcomm.com>
---
.../devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml
index 7943049834f9..95375efb2597 100644
--- a/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml
@@ -16,9 +16,13 @@ description:
properties:
compatible:
- enum:
- - qcom,kaanapali-csi2-phy
- - qcom,x1e80100-csi2-phy
+ oneOf:
+ - enum:
+ - qcom,kaanapali-csi2-phy
+ - qcom,x1e80100-csi2-phy
+ - items:
+ - const: qcom,sm8750-csi2-phy
+ - const: qcom,kaanapali-csi2-phy
reg:
maxItems: 1
--
2.34.1
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* [PATCH v6 0/7] media: qcom: camss: Add SM8750 support
From: Hangxiang Ma @ 2026-07-21 12:45 UTC (permalink / raw)
To: Robert Foss, Todor Tomov, Bryan O'Donoghue,
Vladimir Zapolskiy, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bryan O'Donoghue,
Loic Poulain, Vinod Koul, Neil Armstrong
Cc: linux-media, linux-arm-msm, devicetree, linux-kernel, linux-phy,
jeyaprakash.soundrapandian, Vijay Kumar Tumati, Hangxiang Ma,
Krzysztof Kozlowski, Atiya Kailany
Add support for the RDI only CAMSS camera driver on SM8750. Enabling
RDI path involves adding the support for a set of CSIPHY, CSID and TFE
modules, with each TFE having multiple RDI ports. This hardware
architecture requires 'qdss_debug_xo' clock for CAMNOC to be functional.
SM8750 camera subsystem provides:
- 6 x CSIPHY (CSI Physical Layer)
- 3 x TPG (Test Pattern Generator)
- 3 x CSID (CSI Decoder)
- 2 x CSID Lite
- 3 x VFE (Video Front End), 5 RDI per VFE
- 2 x VFE Lite, 4 RDI per VFE Lite
This series has been tested using the following commands with S5KJN5 sensor.
- media-ctl --reset
- media-ctl -V '"msm_csiphy2":0[fmt:SGBRG10/4096x3072]'
- media-ctl -V '"msm_csid0":0[fmt:SGBRG10/4096x3072]'
- media-ctl -V '"msm_vfe0_rdi0":0[fmt:SGBRG10/4096x3072]'
- media-ctl -l '"msm_csiphy2":1->"msm_csid0":0[1]'
- media-ctl -l '"msm_csid0":1->"msm_vfe0_rdi0":0[1]'
- yavta --capture=20 -I -n 5 -f SGBRG10P -s 4096x3072 -F /dev/video0
Dependencies:
- https://lore.kernel.org/all/20260720-x1e-csi2-phy-v13-0-160c31958863@linaro.org/
- https://lore.kernel.org/all/20260708-b4-linux-next-25-03-13-dtsi-x1e80100-camss-v12-0-f8588da41f16@linaro.org/
- https://lore.kernel.org/all/20260720-kaanapali-camss-v15-0-c0b1c1167c5d@oss.qualcomm.com/
Signed-off-by: Hangxiang Ma <hangxiang.ma@oss.qualcomm.com>
---
Changes in v6:
- Drive the CSIPHY through the standalone phy-qcom-mipi-csi2 DPHY driver
based on new split-csiphy arch, reusing the shared kaanapali resources.
- Add a dt-bindings patch describing the SM8750 CSI2 PHY with the
"qcom,sm8750-csi2-phy", "qcom,kaanapali-csi2-phy" compatible
- Add TPG v2.3.0 support, reusing the Kaanapali TPG resources
- Link to v5: https://lore.kernel.org/r/20260601-add-support-for-camss-on-sm8750-v5-0-dac36a190de8@oss.qualcomm.com
Changes in v5:
- Add missing CSI2_RX_CFG0_PHY_SEL_BASE_IDX macro due to kaanapali
series has merged it into gen4 driver
- Link to v4: https://lore.kernel.org/r/20260601-add-support-for-camss-on-sm8750-v4-0-1eb6f432cfd1@oss.qualcomm.com
Changes in v4:
- Update dependency to kaanapali camss v14 and rebase to resolve conflict
- Link to v3: https://lore.kernel.org/r/20260508-add-support-for-camss-on-sm8750-v3-0-fc6861a65c67@oss.qualcomm.com
Changes in v3:
- Revert binding from full hardware description to CAMSS-only scope for
modular and incremental development - bod
- Rename icc path names and vfe clock names to drop redundancies - Krzysztof
- Separate port index from VC value in csid_configure_stream(). Previously
vc was used as both the loop iterator and the hardware VC, causing
misconfiguration on RDI path starting from 1 - bod
- Link to v2: https://lore.kernel.org/all/20260119-add-support-for-camss-on-sm8750-v2-0-3e06583a0a81@oss.qualcomm.com
Changes in v2:
- Rebase this series due to conflict - bod
- Add module descriptions in binding commit message and cover letter
- Update property names to align with previous generations - Krzysztof
- Update the vdd supply names with 0p88 to 0p9 in binding to keep such name
style consistency - Krzysztof/Vladimir
- Add missing Kaanapali dependency - Krzysztof
- Add regulator current in csiphy resource due to interface changed - bod
- Make csid board level code style consistent and add comments to explain
the differences between csid full and lite configurations - bod
- Remove redundant initialization for empty set in csid and vfe - bod
- Remove DTS patch due to conflict with camcc dependency. Will post it
as an individual series.
- Update vfe commit message as renaming work done in Kaanapali series
- Revert change-id to v1 to avoid increasing reviewers' workload
- Link to v1: https://lore.kernel.org/r/20251126-add-support-for-camss-on-sm8750-v1-0-646fee2eb720@oss.qualcomm.com
---
Hangxiang Ma (7):
dt-bindings: phy: qcom,x1e80100-csi2-phy: Add SM8750 CSI2 PHY
media: dt-bindings: Add CAMSS device for SM8750
media: qcom: camss: Add SM8750 compatible camss driver
media: qcom: camss: csiphy: Add support for v2.3.0 two-phase CSIPHY
media: qcom: camss: csid: Add support for CSID 980
media: qcom: camss: vfe: Add support for VFE 980
media: qcom: camss: tpg: Add support for v2.3.0 TPG
.../bindings/media/qcom,sm8750-camss.yaml | 427 +++++++++++++++++++
.../bindings/phy/qcom,x1e80100-csi2-phy.yaml | 10 +-
drivers/media/platform/qcom/camss/Makefile | 1 +
drivers/media/platform/qcom/camss/camss-csid-980.c | 454 +++++++++++++++++++++
drivers/media/platform/qcom/camss/camss-csid.h | 1 +
drivers/media/platform/qcom/camss/camss-tpg-gen1.c | 9 +-
drivers/media/platform/qcom/camss/camss-vfe-gen4.c | 10 +-
drivers/media/platform/qcom/camss/camss-vfe.c | 2 +
drivers/media/platform/qcom/camss/camss.c | 103 +++++
drivers/media/platform/qcom/camss/camss.h | 1 +
10 files changed, 1008 insertions(+), 10 deletions(-)
---
base-commit: 03cfefef0119410fc464d0d56e83626cb570b7a8
change-id: 20251126-add-support-for-camss-on-sm8750-506c4de36d88
prerequisite-change-id: 20250710-x1e-csi2-phy-f6434b651d3a:v13
prerequisite-patch-id: 2f70527d6a880f047a4858f75813420c81b4667a
prerequisite-patch-id: b797e1ad3e25f79b0bac8f885b2866206a6e0260
prerequisite-change-id: 20250313-b4-linux-next-25-03-13-dtsi-x1e80100-camss-1506f74bbd3a:v12
prerequisite-patch-id: 6e8e67cd3ab96a602971bbeeb7dfdeaf3f1426a2
prerequisite-patch-id: a7fbea14628b62a8de096dea420473b283010aba
prerequisite-patch-id: b6b6c4e7a5818e1b93fe2758902bd32d2be48509
prerequisite-patch-id: 6fb94a3fa54b9810ff6603960770011db1f0b2d6
prerequisite-patch-id: e31f60c0c41e5b6baa464339263a33e5b2d69fac
prerequisite-patch-id: 83a3c9f2c2478a4a3a9441f58c58d04b04cec44d
prerequisite-change-id: 20260112-kaanapali-camss-73772d44eff7:v15
prerequisite-patch-id: 322972f71789ddcc32cae5e94d71bdac75b0d571
prerequisite-patch-id: 92fb4faf61647d08ec71d038c1dcdc92f72c5f0a
prerequisite-patch-id: bd1bcd82db3b28174e1598ae285430568cf94134
prerequisite-patch-id: b347ad60c475f3035b04ec76741e0f1b5f095580
prerequisite-patch-id: 1d8b84bb0f1f86699b1f7699f3125ef5e4e2eb22
prerequisite-patch-id: ec8bcc12a96780388b1633deab8a9816d8e02617
prerequisite-patch-id: d6a128a29162994f73c84e0935607c1a02d8e988
prerequisite-patch-id: b5740c83f750d116dfe565b8ab70542e27c09e52
Best regards,
--
Hangxiang Ma <hangxiang.ma@oss.qualcomm.com>
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* [PATCH v6 3/7] media: qcom: camss: Add SM8750 compatible camss driver
From: Hangxiang Ma @ 2026-07-21 12:46 UTC (permalink / raw)
To: Robert Foss, Todor Tomov, Bryan O'Donoghue,
Vladimir Zapolskiy, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bryan O'Donoghue,
Loic Poulain, Vinod Koul, Neil Armstrong
Cc: linux-media, linux-arm-msm, devicetree, linux-kernel, linux-phy,
jeyaprakash.soundrapandian, Vijay Kumar Tumati, Hangxiang Ma
In-Reply-To: <20260721-add-support-for-camss-on-sm8750-v6-0-a5f841061d18@oss.qualcomm.com>
Add support for SM8750 in the camss driver. Add high level resource
information along with the bus bandwidth votes. Module level detailed
resource information will be enumerated in the following patches of the
series.
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Hangxiang Ma <hangxiang.ma@oss.qualcomm.com>
---
drivers/media/platform/qcom/camss/camss.c | 22 ++++++++++++++++++++++
drivers/media/platform/qcom/camss/camss.h | 1 +
2 files changed, 23 insertions(+)
diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
index 252dc2fe4207..9982174ed1ad 100644
--- a/drivers/media/platform/qcom/camss/camss.c
+++ b/drivers/media/platform/qcom/camss/camss.c
@@ -4477,6 +4477,20 @@ static const struct resources_icc icc_res_sa8775p[] = {
},
};
+static const struct resources_icc icc_res_sm8750[] = {
+ {
+ .name = "ahb",
+ .icc_bw_tbl.avg = 150000,
+ .icc_bw_tbl.peak = 300000,
+ },
+ /* Based on 4096 x 3072 30 FPS 2496 Mbps mode */
+ {
+ .name = "hf_mnoc",
+ .icc_bw_tbl.avg = 471860,
+ .icc_bw_tbl.peak = 925857,
+ },
+};
+
static const struct camss_subdev_resources csiphy_res_x1e80100[] = {
/* CSIPHY0 */
{
@@ -6068,6 +6082,13 @@ static const struct camss_resources sm8650_resources = {
.vfe_num = ARRAY_SIZE(vfe_res_sm8650),
};
+static const struct camss_resources sm8750_resources = {
+ .version = CAMSS_8750,
+ .pd_name = "top",
+ .icc_res = icc_res_sm8750,
+ .icc_path_num = ARRAY_SIZE(icc_res_sm8750),
+};
+
static const struct camss_resources x1e80100_resources = {
.version = CAMSS_X1E80100,
.pd_name = "top",
@@ -6103,6 +6124,7 @@ static const struct of_device_id camss_dt_match[] = {
{ .compatible = "qcom,sm8250-camss", .data = &sm8250_resources },
{ .compatible = "qcom,sm8550-camss", .data = &sm8550_resources },
{ .compatible = "qcom,sm8650-camss", .data = &sm8650_resources },
+ { .compatible = "qcom,sm8750-camss", .data = &sm8750_resources },
{ .compatible = "qcom,x1e80100-camss", .data = &x1e80100_resources },
{ }
};
diff --git a/drivers/media/platform/qcom/camss/camss.h b/drivers/media/platform/qcom/camss/camss.h
index cea213dc36eb..7f525ac37cbb 100644
--- a/drivers/media/platform/qcom/camss/camss.h
+++ b/drivers/media/platform/qcom/camss/camss.h
@@ -95,6 +95,7 @@ enum camss_version {
CAMSS_845,
CAMSS_8550,
CAMSS_8650,
+ CAMSS_8750,
CAMSS_8775P,
CAMSS_KAANAPALI,
CAMSS_X1E80100,
--
2.34.1
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* [PATCH v6 2/7] media: dt-bindings: Add CAMSS device for SM8750
From: Hangxiang Ma @ 2026-07-21 12:46 UTC (permalink / raw)
To: Robert Foss, Todor Tomov, Bryan O'Donoghue,
Vladimir Zapolskiy, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bryan O'Donoghue,
Loic Poulain, Vinod Koul, Neil Armstrong
Cc: linux-media, linux-arm-msm, devicetree, linux-kernel, linux-phy,
jeyaprakash.soundrapandian, Vijay Kumar Tumati, Hangxiang Ma,
Krzysztof Kozlowski
In-Reply-To: <20260721-add-support-for-camss-on-sm8750-v6-0-a5f841061d18@oss.qualcomm.com>
Add bindings for Camera Subsystem (CAMSS) on the Qualcomm SM8750 platform.
The SM8750 platform provides:
- 6 x CSIPHY (CSI Physical Layer)
- 3 x TPG (Test Pattern Generator)
- 3 x CSID (CSI Decoder)
- 2 x CSID Lite
- 3 x VFE (Video Front End), 5 RDI per VFE
- 2 x VFE Lite, 4 RDI per VFE Lite
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Signed-off-by: Hangxiang Ma <hangxiang.ma@oss.qualcomm.com>
---
.../bindings/media/qcom,sm8750-camss.yaml | 427 +++++++++++++++++++++
1 file changed, 427 insertions(+)
diff --git a/Documentation/devicetree/bindings/media/qcom,sm8750-camss.yaml b/Documentation/devicetree/bindings/media/qcom,sm8750-camss.yaml
new file mode 100644
index 000000000000..fca288d2b82a
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/qcom,sm8750-camss.yaml
@@ -0,0 +1,427 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/qcom,sm8750-camss.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm SM8750 Camera Subsystem (CAMSS)
+
+maintainers:
+ - Hangxiang Ma <hangxiang.ma@oss.qualcomm.com>
+
+description:
+ The CAMSS IP is a CSI decoder and ISP present on Qualcomm platforms.
+
+properties:
+ compatible:
+ const: qcom,sm8750-camss
+
+ "#address-cells":
+ const: 2
+
+ "#size-cells":
+ const: 2
+
+ ranges: true
+
+ reg:
+ maxItems: 13
+
+ reg-names:
+ items:
+ - const: csid0
+ - const: csid1
+ - const: csid2
+ - const: csid_lite0
+ - const: csid_lite1
+ - const: csitpg0
+ - const: csitpg1
+ - const: csitpg2
+ - const: vfe0
+ - const: vfe1
+ - const: vfe2
+ - const: vfe_lite0
+ - const: vfe_lite1
+
+ clocks:
+ maxItems: 23
+
+ clock-names:
+ items:
+ - const: camnoc_nrt_axi
+ - const: camnoc_rt_axi
+ - const: cpas_ahb
+ - const: cpas_fast_ahb
+ - const: cpas_vfe0
+ - const: cpas_vfe1
+ - const: cpas_vfe2
+ - const: cpas_vfe_lite
+ - const: csid
+ - const: csid_csiphy_rx
+ - const: gcc_axi_hf
+ - const: gcc_axi_sf
+ - const: vfe0
+ - const: vfe0_fast_ahb
+ - const: vfe1
+ - const: vfe1_fast_ahb
+ - const: vfe2
+ - const: vfe2_fast_ahb
+ - const: vfe_lite
+ - const: vfe_lite_ahb
+ - const: vfe_lite_cphy_rx
+ - const: vfe_lite_csid
+ - const: qdss_debug_xo
+
+ interrupts:
+ maxItems: 10
+
+ interrupt-names:
+ items:
+ - const: csid0
+ - const: csid1
+ - const: csid2
+ - const: csid_lite0
+ - const: csid_lite1
+ - const: vfe0
+ - const: vfe1
+ - const: vfe2
+ - const: vfe_lite0
+ - const: vfe_lite1
+
+ phys:
+ maxItems: 6
+ description: CSIPHY 0-5 PHYs used by the CSID controllers.
+
+ phy-names:
+ items:
+ - const: csiphy0
+ - const: csiphy1
+ - const: csiphy2
+ - const: csiphy3
+ - const: csiphy4
+ - const: csiphy5
+
+ interconnects:
+ maxItems: 4
+
+ interconnect-names:
+ items:
+ - const: ahb
+ - const: hf_mnoc
+ - const: sf_mnoc
+ - const: sf_icp_mnoc
+
+ iommus:
+ maxItems: 1
+
+ power-domains:
+ items:
+ - description:
+ IFE0 GDSC - Global Distributed Switch Controller for IFE0.
+ - description:
+ IFE1 GDSC - Global Distributed Switch Controller for IFE1.
+ - description:
+ IFE2 GDSC - Global Distributed Switch Controller for IFE2.
+ - description:
+ Titan GDSC - Global Distributed Switch Controller for the entire camss.
+
+ power-domain-names:
+ items:
+ - const: ife0
+ - const: ife1
+ - const: ife2
+ - const: top
+
+ ports:
+ $ref: /schemas/graph.yaml#/properties/ports
+
+ description:
+ CSI input ports, one per CSID. Each port receives the CSI data
+ decoded by the matching CSIPHY.
+
+ patternProperties:
+ "^port@[0-5]$":
+ $ref: /schemas/graph.yaml#/$defs/port-base
+ unevaluatedProperties: false
+ description:
+ Input port for receiving CSI data from CSIPHY 0-5.
+
+ properties:
+ endpoint:
+ $ref: video-interfaces.yaml#
+ unevaluatedProperties: false
+
+ properties:
+ data-lanes:
+ minItems: 1
+ maxItems: 4
+
+ bus-type:
+ enum:
+ - 1 # MEDIA_BUS_TYPE_CSI2_CPHY
+ - 4 # MEDIA_BUS_TYPE_CSI2_DPHY
+
+ required:
+ - data-lanes
+
+patternProperties:
+ "^phy@[0-9a-f]+$":
+ $ref: /schemas/phy/qcom,x1e80100-csi2-phy.yaml#
+ unevaluatedProperties: false
+ description:
+ CSIPHY blocks decoding the incoming MIPI CSI2 sensor streams.
+
+ "^opp-table(-.*)?$":
+ type: object
+
+required:
+ - compatible
+ - reg
+ - reg-names
+ - clocks
+ - clock-names
+ - interrupts
+ - interrupt-names
+ - interconnects
+ - interconnect-names
+ - iommus
+ - power-domains
+ - power-domain-names
+ - phys
+ - phy-names
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/qcom,sm8750-gcc.h>
+ #include <dt-bindings/clock/qcom,sm8750-camcc.h>
+ #include <dt-bindings/interconnect/qcom,icc.h>
+ #include <dt-bindings/interconnect/qcom,sm8750-rpmh.h>
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/phy/phy.h>
+ #include <dt-bindings/power/qcom,rpmhpd.h>
+
+ soc {
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ isp@ad27000 {
+ compatible = "qcom,sm8750-camss";
+
+ #address-cells = <2>;
+ #size-cells = <2>;
+ ranges;
+
+ reg = <0x0 0x0ad27000 0x0 0x2b00>,
+ <0x0 0x0ad2a000 0x0 0x2b00>,
+ <0x0 0x0ad2d000 0x0 0x2b00>,
+ <0x0 0x0ad6d000 0x0 0xa00>,
+ <0x0 0x0ad72000 0x0 0xa00>,
+ <0x0 0x0ad8b000 0x0 0x400>,
+ <0x0 0x0ad8c000 0x0 0x400>,
+ <0x0 0x0ad8d000 0x0 0x400>,
+ <0x0 0x0ac86000 0x0 0x10000>,
+ <0x0 0x0ac96000 0x0 0x10000>,
+ <0x0 0x0aca6000 0x0 0x10000>,
+ <0x0 0x0ad6e000 0x0 0x3000>,
+ <0x0 0x0ad73000 0x0 0x3000>;
+ reg-names = "csid0",
+ "csid1",
+ "csid2",
+ "csid_lite0",
+ "csid_lite1",
+ "csitpg0",
+ "csitpg1",
+ "csitpg2",
+ "vfe0",
+ "vfe1",
+ "vfe2",
+ "vfe_lite0",
+ "vfe_lite1";
+
+ clocks = <&camcc CAM_CC_CAMNOC_NRT_AXI_CLK>,
+ <&camcc CAM_CC_CAMNOC_RT_AXI_CLK>,
+ <&camcc CAM_CC_CAM_TOP_AHB_CLK>,
+ <&camcc CAM_CC_CAM_TOP_FAST_AHB_CLK>,
+ <&camcc CAM_CC_CAMNOC_RT_TFE_0_MAIN_CLK>,
+ <&camcc CAM_CC_CAMNOC_RT_TFE_1_MAIN_CLK>,
+ <&camcc CAM_CC_CAMNOC_RT_TFE_2_MAIN_CLK>,
+ <&camcc CAM_CC_CAMNOC_RT_IFE_LITE_CLK>,
+ <&camcc CAM_CC_CSID_CLK>,
+ <&camcc CAM_CC_CSID_CSIPHY_RX_CLK>,
+ <&gcc GCC_CAMERA_HF_AXI_CLK>,
+ <&gcc GCC_CAMERA_SF_AXI_CLK>,
+ <&camcc CAM_CC_TFE_0_MAIN_CLK>,
+ <&camcc CAM_CC_TFE_0_MAIN_FAST_AHB_CLK>,
+ <&camcc CAM_CC_TFE_1_MAIN_CLK>,
+ <&camcc CAM_CC_TFE_1_MAIN_FAST_AHB_CLK>,
+ <&camcc CAM_CC_TFE_2_MAIN_CLK>,
+ <&camcc CAM_CC_TFE_2_MAIN_FAST_AHB_CLK>,
+ <&camcc CAM_CC_IFE_LITE_CLK>,
+ <&camcc CAM_CC_IFE_LITE_AHB_CLK>,
+ <&camcc CAM_CC_IFE_LITE_CPHY_RX_CLK>,
+ <&camcc CAM_CC_IFE_LITE_CSID_CLK>,
+ <&camcc CAM_CC_QDSS_DEBUG_XO_CLK>;
+ clock-names = "camnoc_nrt_axi",
+ "camnoc_rt_axi",
+ "cpas_ahb",
+ "cpas_fast_ahb",
+ "cpas_vfe0",
+ "cpas_vfe1",
+ "cpas_vfe2",
+ "cpas_vfe_lite",
+ "csid",
+ "csid_csiphy_rx",
+ "gcc_axi_hf",
+ "gcc_axi_sf",
+ "vfe0",
+ "vfe0_fast_ahb",
+ "vfe1",
+ "vfe1_fast_ahb",
+ "vfe2",
+ "vfe2_fast_ahb",
+ "vfe_lite",
+ "vfe_lite_ahb",
+ "vfe_lite_cphy_rx",
+ "vfe_lite_csid",
+ "qdss_debug_xo";
+
+ interrupts = <GIC_SPI 601 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 603 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 431 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 605 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 376 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 433 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 436 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 457 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 606 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 377 IRQ_TYPE_EDGE_RISING>;
+ interrupt-names = "csid0",
+ "csid1",
+ "csid2",
+ "csid_lite0",
+ "csid_lite1",
+ "vfe0",
+ "vfe1",
+ "vfe2",
+ "vfe_lite0",
+ "vfe_lite1";
+
+ interconnects = <&gem_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ACTIVE_ONLY
+ &config_noc SLAVE_CAMERA_CFG QCOM_ICC_TAG_ACTIVE_ONLY>,
+ <&mmss_noc MASTER_CAMNOC_HF QCOM_ICC_TAG_ALWAYS
+ &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
+ <&mmss_noc MASTER_CAMNOC_SF QCOM_ICC_TAG_ALWAYS
+ &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
+ <&mmss_noc MASTER_CAMNOC_NRT_ICP_SF QCOM_ICC_TAG_ALWAYS
+ &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>;
+ interconnect-names = "ahb",
+ "hf_mnoc",
+ "sf_mnoc",
+ "sf_icp_mnoc";
+
+ iommus = <&apps_smmu 0x1c00 0x00>;
+
+ power-domains = <&camcc CAM_CC_TFE_0_GDSC>,
+ <&camcc CAM_CC_TFE_1_GDSC>,
+ <&camcc CAM_CC_TFE_2_GDSC>,
+ <&camcc CAM_CC_TITAN_TOP_GDSC>;
+ power-domain-names = "ife0",
+ "ife1",
+ "ife2",
+ "top";
+
+ phys = <&csiphy0 PHY_TYPE_DPHY>,
+ <&csiphy1 PHY_TYPE_DPHY>,
+ <&csiphy2 PHY_TYPE_DPHY>,
+ <&csiphy3 PHY_TYPE_DPHY>,
+ <&csiphy4 PHY_TYPE_DPHY>,
+ <&csiphy5 PHY_TYPE_DPHY>;
+ phy-names = "csiphy0",
+ "csiphy1",
+ "csiphy2",
+ "csiphy3",
+ "csiphy4",
+ "csiphy5";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ camss_csiphy0_inep0: endpoint@0 {
+ reg = <0>;
+ data-lanes = <0 1 2 3>;
+ remote-endpoint = <&csiphy0_out>;
+ };
+ };
+ };
+
+ csiphy0: phy@ada9000 {
+ compatible = "qcom,sm8750-csi2-phy",
+ "qcom,kaanapali-csi2-phy";
+ reg = <0x0 0x0ada9000 0x0 0x2000>;
+
+ clocks = <&camcc CAM_CC_CSIPHY0_CLK>,
+ <&camcc CAM_CC_CSI0PHYTIMER_CLK>,
+ <&camcc CAM_CC_CORE_AHB_CLK>;
+ clock-names = "core",
+ "timer",
+ "ahb";
+
+ interrupts = <GIC_SPI 477 IRQ_TYPE_EDGE_RISING>;
+
+ operating-points-v2 = <&csiphy_mxc_opp_table>;
+
+ power-domains = <&camcc CAM_CC_TITAN_TOP_GDSC>,
+ <&rpmhpd RPMHPD_MMCX>,
+ <&rpmhpd RPMHPD_MXC>;
+ power-domain-names = "top",
+ "mmcx",
+ "mx";
+
+ vdda-0p9-supply = <&vreg_0p9_supply>;
+ vdda-1p2-supply = <&vreg_1p2_supply>;
+
+ #phy-cells = <1>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+
+ csiphy0_in: endpoint {
+ data-lanes = <0 1 2 3>;
+ clock-lanes = <7>;
+ remote-endpoint = <&sensor_out>;
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+
+ csiphy0_out: endpoint {
+ remote-endpoint = <&camss_csiphy0_inep0>;
+ };
+ };
+ };
+
+ csiphy_mxc_opp_table: opp-table {
+ compatible = "operating-points-v2";
+
+ opp-300000000 {
+ opp-hz = /bits/ 64 <300000000>;
+ required-opps = <&rpmhpd_opp_low_svs_d1>,
+ <&rpmhpd_opp_low_svs_d1>;
+ };
+ };
+ };
+ };
+ };
--
2.34.1
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* [PATCH v6 4/7] media: qcom: camss: csiphy: Add support for v2.3.0 two-phase CSIPHY
From: Hangxiang Ma @ 2026-07-21 12:46 UTC (permalink / raw)
To: Robert Foss, Todor Tomov, Bryan O'Donoghue,
Vladimir Zapolskiy, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bryan O'Donoghue,
Loic Poulain, Vinod Koul, Neil Armstrong
Cc: linux-media, linux-arm-msm, devicetree, linux-kernel, linux-phy,
jeyaprakash.soundrapandian, Vijay Kumar Tumati, Hangxiang Ma
In-Reply-To: <20260721-add-support-for-camss-on-sm8750-v6-0-a5f841061d18@oss.qualcomm.com>
Add more detailed resource information for CSIPHY devices in the camss
driver along with the support for v2.3.0 in the 2 phase CSIPHY driver
that is responsible for the PHY lane register configuration, module
reset and interrupt handling.
Reviewed-by: Bryan O'Donoghue <bod@kernel.org>
Signed-off-by: Hangxiang Ma <hangxiang.ma@oss.qualcomm.com>
---
drivers/media/platform/qcom/camss/camss.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
index 9982174ed1ad..1ab1a0cd82d2 100644
--- a/drivers/media/platform/qcom/camss/camss.c
+++ b/drivers/media/platform/qcom/camss/camss.c
@@ -6085,7 +6085,9 @@ static const struct camss_resources sm8650_resources = {
static const struct camss_resources sm8750_resources = {
.version = CAMSS_8750,
.pd_name = "top",
+ .csiphy_res = csiphy_res_kaanapali,
.icc_res = icc_res_sm8750,
+ .csiphy_num = ARRAY_SIZE(csiphy_res_kaanapali),
.icc_path_num = ARRAY_SIZE(icc_res_sm8750),
};
--
2.34.1
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* [PATCH v6 5/7] media: qcom: camss: csid: Add support for CSID 980
From: Hangxiang Ma @ 2026-07-21 12:46 UTC (permalink / raw)
To: Robert Foss, Todor Tomov, Bryan O'Donoghue,
Vladimir Zapolskiy, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bryan O'Donoghue,
Loic Poulain, Vinod Koul, Neil Armstrong
Cc: linux-media, linux-arm-msm, devicetree, linux-kernel, linux-phy,
jeyaprakash.soundrapandian, Vijay Kumar Tumati, Hangxiang Ma,
Atiya Kailany
In-Reply-To: <20260721-add-support-for-camss-on-sm8750-v6-0-a5f841061d18@oss.qualcomm.com>
Add more detailed resource information for CSID devices along with the
driver for CSID 980 that is responsible for CSID register
configuration, module reset and IRQ handling for BUF_DONE events.
In SM8750, RUP and AUP updates for the CSID Full modules are split into
two registers along with a SET register. However, CSID Lite modules
still use a single register to update RUP and AUP without the additional
SET register. Handled such differences in the driver.
Co-developed-by: Atiya Kailany <atiya.kailany@oss.qualcomm.com>
Signed-off-by: Atiya Kailany <atiya.kailany@oss.qualcomm.com>
Signed-off-by: Hangxiang Ma <hangxiang.ma@oss.qualcomm.com>
---
drivers/media/platform/qcom/camss/Makefile | 1 +
drivers/media/platform/qcom/camss/camss-csid-980.c | 443 +++++++++++++++++++++
drivers/media/platform/qcom/camss/camss-csid.h | 1 +
drivers/media/platform/qcom/camss/camss.c | 75 ++++
4 files changed, 520 insertions(+)
diff --git a/drivers/media/platform/qcom/camss/Makefile b/drivers/media/platform/qcom/camss/Makefile
index b114ca37e36e..c19d3183882a 100644
--- a/drivers/media/platform/qcom/camss/Makefile
+++ b/drivers/media/platform/qcom/camss/Makefile
@@ -8,6 +8,7 @@ qcom-camss-objs += \
camss-csid-4-7.o \
camss-csid-340.o \
camss-csid-680.o \
+ camss-csid-980.o \
camss-csid-gen2.o \
camss-csid-gen3.o \
camss-csid-gen4.o \
diff --git a/drivers/media/platform/qcom/camss/camss-csid-980.c b/drivers/media/platform/qcom/camss/camss-csid-980.c
new file mode 100644
index 000000000000..6714b8aa8c2a
--- /dev/null
+++ b/drivers/media/platform/qcom/camss/camss-csid-980.c
@@ -0,0 +1,443 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * camss-csid-980.c
+ *
+ * Qualcomm MSM Camera Subsystem - CSID (CSI Decoder) Module
+ *
+ * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
+ */
+#include <linux/completion.h>
+#include <linux/delay.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/of.h>
+#include "camss.h"
+#include "camss-csid.h"
+#include "camss-csid-gen3.h"
+
+/* Reset and Command Registers */
+#define CSID_RST_CFG 0xC
+#define RST_MODE BIT(0)
+#define RST_LOCATION BIT(4)
+
+/* Reset and Command Registers */
+#define CSID_RST_CMD 0x10
+#define SELECT_HW_RST BIT(0)
+#define SELECT_IRQ_RST BIT(2)
+#define CSID_IRQ_CMD 0x14
+#define IRQ_CMD_CLEAR BIT(0)
+
+/* Register Update Commands, RUP/AUP */
+#define CSID_RUP_CMD 0x18
+#define CSID_AUP_CMD 0x1C
+#define CSID_RUP_AUP_RDI(rdi) (BIT(8) << (rdi))
+#define CSID_RUP_AUP_CMD 0x20
+#define RUP_SET BIT(0)
+#define MUP BIT(4)
+
+#define CSID_LITE_RUP_AUP_CMD 0x18
+#define CSID_LITE_RUP_RDI(rdi) (BIT(4) << (rdi))
+#define CSID_LITE_AUP_RDI(rdi) (BIT(20) << (rdi))
+
+/* Top level interrupt registers */
+#define CSID_TOP_IRQ_STATUS (csid_is_lite(csid) ? 0x7C : 0x84)
+#define CSID_TOP_IRQ_MASK (csid_is_lite(csid) ? 0x80 : 0x88)
+#define CSID_TOP_IRQ_CLEAR (csid_is_lite(csid) ? 0x84 : 0x8C)
+#define CSID_TOP_IRQ_SET (csid_is_lite(csid) ? 0x88 : 0x90)
+#define INFO_RST_DONE BIT(0)
+#define CSI2_RX_IRQ_STATUS BIT(2)
+#define BUF_DONE_IRQ_STATUS BIT(csid_is_lite(csid) ? 13 : 3)
+
+/* Buffer done interrupt registers */
+#define CSID_BUF_DONE_IRQ_STATUS (csid_is_lite(csid) ? 0x8C : 0xA4)
+#define BUF_DONE_IRQ_STATUS_RDI_OFFSET (csid_is_lite(csid) ? 1 : 16)
+#define CSID_BUF_DONE_IRQ_MASK (csid_is_lite(csid) ? 0x90 : 0xA8)
+#define CSID_BUF_DONE_IRQ_CLEAR (csid_is_lite(csid) ? 0x94 : 0xAC)
+#define CSID_BUF_DONE_IRQ_SET (csid_is_lite(csid) ? 0x98 : 0xB0)
+
+/* CSI2 RX interrupt registers */
+#define CSID_CSI2_RX_IRQ_STATUS (csid_is_lite(csid) ? 0x9C : 0xB4)
+#define CSID_CSI2_RX_IRQ_MASK (csid_is_lite(csid) ? 0xA0 : 0xB8)
+#define CSID_CSI2_RX_IRQ_CLEAR (csid_is_lite(csid) ? 0xA4 : 0xBC)
+#define CSID_CSI2_RX_IRQ_SET (csid_is_lite(csid) ? 0xA8 : 0xC0)
+
+/* CSI2 RX Configuration */
+#define CSID_CSI2_RX_CFG0 (csid_is_lite(csid) ? 0x200 : 0x400)
+#define CSI2_RX_CFG0_NUM_ACTIVE_LANES 0
+#define CSI2_RX_CFG0_PHY_SEL_BASE_IDX 1
+#define CSI2_RX_CFG0_DL0_INPUT_SEL 4
+#define CSI2_RX_CFG0_PHY_NUM_SEL 20
+#define CSID_CSI2_RX_CFG1 (csid_is_lite(csid) ? 0x204 : 0x404)
+#define CSI2_RX_CFG1_ECC_CORRECTION_EN BIT(0)
+#define CSI2_RX_CFG1_VC_MODE BIT(2)
+
+#define MSM_CSID_MAX_SRC_STREAMS_980 (csid_is_lite(csid) ? 4 : 5)
+
+#define CSID_RDI_CFG0(rdi) \
+ ({ \
+ __typeof__(rdi) _rdi = (rdi); \
+ csid_is_lite(csid) ? 0x500 + 0x100 * _rdi : \
+ 0xE00 + 0x200 * _rdi; \
+ })
+#define RDI_CFG0_RETIME_BS BIT(5)
+#define RDI_CFG0_TIMESTAMP_EN BIT(6)
+#define RDI_CFG0_TIMESTAMP_STB_SEL BIT(8)
+#define RDI_CFG0_DECODE_FORMAT 12
+#define RDI_CFG0_DT 16
+#define RDI_CFG0_VC 22
+#define RDI_CFG0_DT_ID 27
+#define RDI_CFG0_EN BIT(31)
+
+/* RDI Control and Configuration */
+#define CSID_RDI_CTRL(rdi) \
+ ({ \
+ __typeof__(rdi) _rdi = (rdi); \
+ csid_is_lite(csid) ? 0x504 + 0x100 * _rdi : \
+ 0xE04 + 0x200 * _rdi; \
+ })
+#define RDI_CTRL_START_CMD BIT(0)
+
+#define CSID_RDI_CFG1(rdi) \
+ ({ \
+ __typeof__(rdi) _rdi = (rdi); \
+ csid_is_lite(csid) ? 0x510 + 0x100 * _rdi : \
+ 0xE10 + 0x200 * _rdi; \
+ })
+#define RDI_CFG1_DROP_H_EN BIT(5)
+#define RDI_CFG1_DROP_V_EN BIT(6)
+#define RDI_CFG1_CROP_H_EN BIT(7)
+#define RDI_CFG1_CROP_V_EN BIT(8)
+#define RDI_CFG1_PACKING_FORMAT_MIPI BIT(15)
+
+/* RDI Pixel Store Configuration */
+#define CSID_RDI_PIX_STORE_CFG0(rdi) (0xE14 + 0x200 * (rdi))
+#define RDI_PIX_STORE_CFG0_EN BIT(0)
+#define RDI_PIX_STORE_CFG0_MIN_HBI 1
+
+/* RDI IRQ Status in wrapper */
+#define CSID_CSI2_RDIN_IRQ_STATUS(rdi) \
+ (csid_is_lite(csid) ? 0xEC : 0x114 + 0x10 * (rdi))
+#define CSID_CSI2_RDIN_IRQ_CLEAR(rdi) \
+ (csid_is_lite(csid) ? 0xF4 : 0x11C + 0x10 * (rdi))
+#define INFO_RUP_DONE BIT(23)
+
+static void __csid_full_aup_rup_trigger(struct csid_device *csid)
+{
+ /* trigger SET in combined register */
+ writel(RUP_SET, csid->base + CSID_RUP_AUP_CMD);
+}
+
+static void __csid_aup_update(struct csid_device *csid, int port_id)
+{
+ if (csid_is_lite(csid)) {
+ /* CSID Lites in v980 follow the legacy way of a combined RUP
+ * and AUP commands without an explicit SET register.
+ */
+ csid->reg_update |= CSID_LITE_AUP_RDI(port_id);
+ writel(csid->reg_update, csid->base + CSID_LITE_RUP_AUP_CMD);
+ } else {
+ csid->aup_update |= CSID_RUP_AUP_RDI(port_id);
+ writel(csid->aup_update, csid->base + CSID_AUP_CMD);
+
+ /* CSID Fulls in v980 split AUP and RUP commands, which requires
+ * additional SET operation to make registers modification take
+ * effect.
+ */
+ __csid_full_aup_rup_trigger(csid);
+ }
+}
+
+static void __csid_rup_update(struct csid_device *csid, int port_id)
+{
+ if (csid_is_lite(csid)) {
+ /* CSID Lites in v980 follow the legacy way of a combined RUP
+ * and AUP commands without an explicit SET register.
+ */
+ csid->reg_update |= CSID_LITE_RUP_RDI(port_id);
+ writel(csid->reg_update, csid->base + CSID_LITE_RUP_AUP_CMD);
+ } else {
+ csid->rup_update |= CSID_RUP_AUP_RDI(port_id);
+ writel(csid->rup_update, csid->base + CSID_RUP_CMD);
+
+ /* CSID Fulls in v980 split AUP and RUP commands, which requires
+ * additional SET operation to make registers modification take
+ * effect.
+ */
+ __csid_full_aup_rup_trigger(csid);
+ }
+}
+
+static void __csid_aup_rup_clear(struct csid_device *csid, int port_id)
+{
+ /* Hardware clears the registers upon consuming the settings */
+ if (csid_is_lite(csid)) {
+ csid->reg_update &= ~CSID_LITE_RUP_RDI(port_id);
+ csid->reg_update &= ~CSID_LITE_AUP_RDI(port_id);
+ } else {
+ csid->aup_update &= ~CSID_RUP_AUP_RDI(port_id);
+ csid->rup_update &= ~CSID_RUP_AUP_RDI(port_id);
+ }
+}
+
+static void __csid_configure_rx(struct csid_device *csid,
+ struct csid_phy_config *phy)
+{
+ int val;
+
+ val = (phy->lane_cnt - 1) << CSI2_RX_CFG0_NUM_ACTIVE_LANES;
+ val |= phy->lane_assign << CSI2_RX_CFG0_DL0_INPUT_SEL;
+ val |= (phy->csiphy_id + CSI2_RX_CFG0_PHY_SEL_BASE_IDX)
+ << CSI2_RX_CFG0_PHY_NUM_SEL;
+ writel(val, csid->base + CSID_CSI2_RX_CFG0);
+
+ val = CSI2_RX_CFG1_ECC_CORRECTION_EN;
+ writel(val, csid->base + CSID_CSI2_RX_CFG1);
+}
+
+static void __csid_configure_rx_vc(struct csid_device *csid, int vc)
+{
+ int val;
+
+ if (vc > 3) {
+ val = readl(csid->base + CSID_CSI2_RX_CFG1);
+ val |= CSI2_RX_CFG1_VC_MODE;
+ writel(val, csid->base + CSID_CSI2_RX_CFG1);
+ }
+}
+
+static void __csid_ctrl_rdi(struct csid_device *csid, int enable, u8 rdi)
+{
+ u32 rdi_ctrl_offset = CSID_RDI_CTRL(rdi);
+ int val = 0;
+
+ if (enable)
+ val = RDI_CTRL_START_CMD;
+
+ writel(val, csid->base + rdi_ctrl_offset);
+}
+
+static void __csid_configure_rdi_pix_store(struct csid_device *csid, u8 rdi)
+{
+ u32 val;
+
+ /*
+ * Configure pixel store to allow absorption of hblanking or idle time.
+ * This helps with horizontal crop and prevents line buffer conflicts.
+ * Reset state is 0x8 which has MIN_HBI=4, we keep the default MIN_HBI
+ * and just enable the pixel store functionality.
+ */
+ val = (4 << RDI_PIX_STORE_CFG0_MIN_HBI) | RDI_PIX_STORE_CFG0_EN;
+ writel(val, csid->base + CSID_RDI_PIX_STORE_CFG0(rdi));
+}
+
+static void __csid_configure_rdi_stream(struct csid_device *csid, u8 enable, u8 port, u8 vc)
+{
+ u8 lane_cnt = csid->phy.lane_cnt;
+ u32 val;
+
+ /* Source pads matching RDI channels on hardware.
+ * E.g. Pad 1 -> RDI0, Pad 2 -> RDI1, etc.
+ */
+ struct v4l2_mbus_framefmt *input_format = &csid->fmt[MSM_CSID_PAD_FIRST_SRC + port];
+ const struct csid_format_info *format = csid_get_fmt_entry(csid->res->formats->formats,
+ csid->res->formats->nformats,
+ input_format->code);
+
+ if (!lane_cnt)
+ lane_cnt = 4;
+
+ /*
+ * DT_ID is a two bit bitfield that is concatenated with
+ * the four least significant bits of the five bit VC
+ * bitfield to generate an internal CID value.
+ *
+ * CSID_RDI_CFG0(vc)
+ * DT_ID : 28:27
+ * VC : 26:22
+ * DT : 21:16
+ *
+ * CID : VC 3:0 << 2 | DT_ID 1:0
+ */
+ u8 dt_id = vc & 0x03;
+ u32 rdi_cfg0_offset = CSID_RDI_CFG0(port);
+ u32 rdi_cfg1_offset = CSID_RDI_CFG1(port);
+ u32 rdi_ctrl_offset = CSID_RDI_CTRL(port);
+
+ val = RDI_CFG0_TIMESTAMP_EN;
+ val |= RDI_CFG0_TIMESTAMP_STB_SEL;
+ val |= RDI_CFG0_RETIME_BS;
+
+ /* note: for non-RDI path, this should be format->decode_format */
+ val |= DECODE_FORMAT_PAYLOAD_ONLY << RDI_CFG0_DECODE_FORMAT;
+ val |= vc << RDI_CFG0_VC;
+ val |= format->data_type << RDI_CFG0_DT;
+ val |= dt_id << RDI_CFG0_DT_ID;
+ writel(val, csid->base + rdi_cfg0_offset);
+
+ val = RDI_CFG1_PACKING_FORMAT_MIPI;
+ writel(val, csid->base + rdi_cfg1_offset);
+
+ /* Configure pixel store using dedicated register in 980 */
+ if (!csid_is_lite(csid))
+ __csid_configure_rdi_pix_store(csid, port);
+
+ val = 0;
+ writel(val, csid->base + rdi_ctrl_offset);
+
+ val = readl(csid->base + rdi_cfg0_offset);
+
+ if (enable)
+ val |= RDI_CFG0_EN;
+
+ writel(val, csid->base + rdi_cfg0_offset);
+}
+
+static void csid_configure_stream(struct csid_device *csid, u8 enable)
+{
+ u8 i, k;
+
+ __csid_configure_rx(csid, &csid->phy);
+
+ for (i = 0; i < MSM_CSID_MAX_SRC_STREAMS_980; i++) {
+ if (csid->phy.en_vc & BIT(i)) {
+ __csid_configure_rdi_stream(csid, enable, i, 0);
+ __csid_configure_rx_vc(csid, 0);
+
+ for (k = 0; k < CAMSS_INIT_BUF_COUNT; k++) {
+ __csid_aup_update(csid, i);
+ __csid_rup_update(csid, i);
+ }
+
+ __csid_ctrl_rdi(csid, enable, i);
+ }
+ }
+}
+
+static int csid_configure_testgen_pattern(struct csid_device *csid, s32 val)
+{
+ return 0;
+}
+
+static void csid_subdev_reg_update(struct csid_device *csid, int port_id,
+ bool clear)
+{
+ if (clear)
+ __csid_aup_rup_clear(csid, port_id);
+ else
+ __csid_aup_update(csid, port_id);
+}
+
+/**
+ * csid_isr - CSID module interrupt service routine
+ * @irq: Interrupt line
+ * @dev: CSID device
+ *
+ * Return IRQ_HANDLED on success
+ */
+static irqreturn_t csid_isr(int irq, void *dev)
+{
+ struct csid_device *csid = dev;
+ u32 val, buf_done_val;
+ u8 reset_done;
+ int i;
+
+ val = readl(csid->base + CSID_TOP_IRQ_STATUS);
+ writel(val, csid->base + CSID_TOP_IRQ_CLEAR);
+
+ reset_done = val & INFO_RST_DONE;
+
+ buf_done_val = readl(csid->base + CSID_BUF_DONE_IRQ_STATUS);
+ writel(buf_done_val, csid->base + CSID_BUF_DONE_IRQ_CLEAR);
+
+ for (i = 0; i < MSM_CSID_MAX_SRC_STREAMS_980; i++) {
+ if (csid->phy.en_vc & BIT(i)) {
+ val = readl(csid->base + CSID_CSI2_RDIN_IRQ_STATUS(i));
+ writel(val, csid->base + CSID_CSI2_RDIN_IRQ_CLEAR(i));
+
+ if (val & INFO_RUP_DONE)
+ csid_subdev_reg_update(csid, i, true);
+
+ if (buf_done_val & BIT(BUF_DONE_IRQ_STATUS_RDI_OFFSET + i))
+ camss_buf_done(csid->camss, csid->id, i);
+ }
+ }
+
+ val = IRQ_CMD_CLEAR;
+ writel(val, csid->base + CSID_IRQ_CMD);
+
+ if (reset_done)
+ complete(&csid->reset_complete);
+
+ return IRQ_HANDLED;
+}
+
+/**
+ * csid_reset - Trigger reset on CSID module and wait to complete
+ * @csid: CSID device
+ *
+ * Return 0 on success or a negative error code otherwise
+ */
+static int csid_reset(struct csid_device *csid)
+{
+ unsigned long time;
+ u32 val;
+ int i;
+
+ reinit_completion(&csid->reset_complete);
+
+ val = INFO_RST_DONE | BUF_DONE_IRQ_STATUS;
+ writel(val, csid->base + CSID_TOP_IRQ_CLEAR);
+ writel(val, csid->base + CSID_TOP_IRQ_MASK);
+
+ val = 0;
+ for (i = 0; i < MSM_CSID_MAX_SRC_STREAMS_980; i++) {
+ if (csid->phy.en_vc & BIT(i)) {
+ /*
+ * Only need to clear buf done IRQ status here,
+ * RUP done IRQ status will be cleared once isr
+ * strobe generated by CSID_RST_CMD
+ */
+ val |= BIT(BUF_DONE_IRQ_STATUS_RDI_OFFSET + i);
+ }
+ }
+ writel(val, csid->base + CSID_BUF_DONE_IRQ_CLEAR);
+ writel(val, csid->base + CSID_BUF_DONE_IRQ_MASK);
+
+ /* Clear all IRQ status with CLEAR bits set */
+ val = IRQ_CMD_CLEAR;
+ writel(val, csid->base + CSID_IRQ_CMD);
+
+ val = RST_LOCATION | RST_MODE;
+ writel(val, csid->base + CSID_RST_CFG);
+
+ val = SELECT_HW_RST | SELECT_IRQ_RST;
+ writel(val, csid->base + CSID_RST_CMD);
+
+ time = wait_for_completion_timeout(&csid->reset_complete,
+ msecs_to_jiffies(CSID_RESET_TIMEOUT_MS));
+
+ if (!time) {
+ dev_err(csid->camss->dev, "CSID reset timeout\n");
+ return -ETIMEDOUT;
+ }
+
+ return 0;
+}
+
+static void csid_subdev_init(struct csid_device *csid)
+{
+ csid->testgen.nmodes = CSID_PAYLOAD_MODE_DISABLED;
+}
+
+const struct csid_hw_ops csid_ops_980 = {
+ .configure_stream = csid_configure_stream,
+ .configure_testgen_pattern = csid_configure_testgen_pattern,
+ .hw_version = csid_hw_version,
+ .isr = csid_isr,
+ .reset = csid_reset,
+ .src_pad_code = csid_src_pad_code,
+ .subdev_init = csid_subdev_init,
+ .reg_update = csid_subdev_reg_update,
+};
+
diff --git a/drivers/media/platform/qcom/camss/camss-csid.h b/drivers/media/platform/qcom/camss/camss-csid.h
index 4f31ad303c4e..8edf3548d692 100644
--- a/drivers/media/platform/qcom/camss/camss-csid.h
+++ b/drivers/media/platform/qcom/camss/camss-csid.h
@@ -222,6 +222,7 @@ extern const struct csid_hw_ops csid_ops_4_1;
extern const struct csid_hw_ops csid_ops_4_7;
extern const struct csid_hw_ops csid_ops_340;
extern const struct csid_hw_ops csid_ops_680;
+extern const struct csid_hw_ops csid_ops_980;
extern const struct csid_hw_ops csid_ops_gen2;
extern const struct csid_hw_ops csid_ops_gen3;
extern const struct csid_hw_ops csid_ops_gen4;
diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
index 1ab1a0cd82d2..e0b181ff33aa 100644
--- a/drivers/media/platform/qcom/camss/camss.c
+++ b/drivers/media/platform/qcom/camss/camss.c
@@ -4477,6 +4477,79 @@ static const struct resources_icc icc_res_sa8775p[] = {
},
};
+static const struct camss_subdev_resources csid_res_sm8750[] = {
+ /* CSID0 */
+ {
+ .clock = { "csid", "csid_csiphy_rx" },
+ .clock_rate = { { 400000000, 480000000 },
+ { 400000000, 480000000 } },
+ .reg = { "csid0" },
+ .interrupt = { "csid0" },
+ .csid = {
+ .is_lite = false,
+ .parent_dev_ops = &vfe_parent_dev_ops,
+ .hw_ops = &csid_ops_980,
+ .formats = &csid_formats_gen2
+ }
+ },
+ /* CSID1 */
+ {
+ .clock = { "csid", "csid_csiphy_rx" },
+ .clock_rate = { { 400000000, 480000000 },
+ { 400000000, 480000000 } },
+ .reg = { "csid1" },
+ .interrupt = { "csid1" },
+ .csid = {
+ .is_lite = false,
+ .parent_dev_ops = &vfe_parent_dev_ops,
+ .hw_ops = &csid_ops_980,
+ .formats = &csid_formats_gen2
+ }
+ },
+ /* CSID2 */
+ {
+ .clock = { "csid", "csid_csiphy_rx" },
+ .clock_rate = { { 400000000, 480000000 },
+ { 400000000, 480000000 } },
+ .reg = { "csid2" },
+ .interrupt = { "csid2" },
+ .csid = {
+ .is_lite = false,
+ .parent_dev_ops = &vfe_parent_dev_ops,
+ .hw_ops = &csid_ops_980,
+ .formats = &csid_formats_gen2
+ }
+ },
+ /* CSID_LITE0 */
+ {
+ .clock = { "vfe_lite_csid", "vfe_lite_cphy_rx" },
+ .clock_rate = { { 400000000, 480000000 },
+ { 400000000, 480000000 } },
+ .reg = { "csid_lite0" },
+ .interrupt = { "csid_lite0" },
+ .csid = {
+ .is_lite = true,
+ .parent_dev_ops = &vfe_parent_dev_ops,
+ .hw_ops = &csid_ops_980,
+ .formats = &csid_formats_gen2
+ }
+ },
+ /* CSID_LITE1 */
+ {
+ .clock = { "vfe_lite_csid", "vfe_lite_cphy_rx" },
+ .clock_rate = { { 400000000, 480000000 },
+ { 400000000, 480000000 } },
+ .reg = { "csid_lite1" },
+ .interrupt = { "csid_lite1" },
+ .csid = {
+ .is_lite = true,
+ .parent_dev_ops = &vfe_parent_dev_ops,
+ .hw_ops = &csid_ops_980,
+ .formats = &csid_formats_gen2
+ }
+ }
+};
+
static const struct resources_icc icc_res_sm8750[] = {
{
.name = "ahb",
@@ -6086,8 +6159,10 @@ static const struct camss_resources sm8750_resources = {
.version = CAMSS_8750,
.pd_name = "top",
.csiphy_res = csiphy_res_kaanapali,
+ .csid_res = csid_res_sm8750,
.icc_res = icc_res_sm8750,
.csiphy_num = ARRAY_SIZE(csiphy_res_kaanapali),
+ .csid_num = ARRAY_SIZE(csid_res_sm8750),
.icc_path_num = ARRAY_SIZE(icc_res_sm8750),
};
--
2.34.1
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* [PATCH v6 6/7] media: qcom: camss: vfe: Add support for VFE 980
From: Hangxiang Ma @ 2026-07-21 12:46 UTC (permalink / raw)
To: Robert Foss, Todor Tomov, Bryan O'Donoghue,
Vladimir Zapolskiy, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bryan O'Donoghue,
Loic Poulain, Vinod Koul, Neil Armstrong
Cc: linux-media, linux-arm-msm, devicetree, linux-kernel, linux-phy,
jeyaprakash.soundrapandian, Vijay Kumar Tumati, Hangxiang Ma,
Atiya Kailany
In-Reply-To: <20260721-add-support-for-camss-on-sm8750-v6-0-a5f841061d18@oss.qualcomm.com>
Add support for Video Front End (VFE) that is on the SM8750 SoCs. VFE
gen4 has support for VFE 980. This change limits SM8750 VFE output lines
to 3 for now as constrained by the CAMSS driver framework.
The cpas_ahb and cpas_fast_ahb clocks are enabled here rather than in the
CSIPHY resources since they belong to the CPAS/VFE power path, matching
the Kaanapali layout.
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Co-developed-by: Atiya Kailany <atiya.kailany@oss.qualcomm.com>
Signed-off-by: Atiya Kailany <atiya.kailany@oss.qualcomm.com>
Signed-off-by: Hangxiang Ma <hangxiang.ma@oss.qualcomm.com>
---
drivers/media/platform/qcom/camss/camss-vfe-gen4.c | 10 +++++++---
drivers/media/platform/qcom/camss/camss-vfe.c | 2 ++
drivers/media/platform/qcom/camss/camss.c | 2 ++
3 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/media/platform/qcom/camss/camss-vfe-gen4.c b/drivers/media/platform/qcom/camss/camss-vfe-gen4.c
index d73d70898710..46d8e61b9bac 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe-gen4.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe-gen4.c
@@ -13,8 +13,12 @@
#include "camss.h"
#include "camss-vfe.h"
-/* VFE-gen4 Bus Register Base Addresses */
-#define BUS_REG_BASE (vfe_is_lite(vfe) ? 0x800 : 0x1000)
+#define IS_VFE_980(vfe) ((vfe)->camss->res->version == CAMSS_8750)
+
+#define BUS_REG_BASE_980 (vfe_is_lite(vfe) ? 0x200 : 0x800)
+#define BUS_REG_BASE_1080 (vfe_is_lite(vfe) ? 0x800 : 0x1000)
+#define BUS_REG_BASE \
+ (IS_VFE_980(vfe) ? BUS_REG_BASE_980 : BUS_REG_BASE_1080)
#define VFE_BUS_WM_CGC_OVERRIDE (BUS_REG_BASE + 0x08)
#define WM_CGC_OVERRIDE_ALL (0x7FFFFFF)
@@ -55,7 +59,7 @@
* DISPLAY_DS2_C 6
* FD_Y 7
* FD_C 8
- * PIXEL_RAW 9
+ * RAW_OUT(1080)/IR_OUT(980) 9
* STATS_AEC_BG 10
* STATS_AEC_BHIST 11
* STATS_TINTLESS_BG 12
diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c b/drivers/media/platform/qcom/camss/camss-vfe.c
index 826ab85a44a1..ffa61ce021b4 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe.c
@@ -352,6 +352,7 @@ static u32 vfe_src_pad_code(struct vfe_line *line, u32 sink_code,
case CAMSS_845:
case CAMSS_8550:
case CAMSS_8650:
+ case CAMSS_8750:
case CAMSS_8775P:
case CAMSS_KAANAPALI:
case CAMSS_X1E80100:
@@ -2016,6 +2017,7 @@ static int vfe_bpl_align_rdi(struct vfe_device *vfe)
case CAMSS_845:
case CAMSS_8550:
case CAMSS_8650:
+ case CAMSS_8750:
case CAMSS_8775P:
case CAMSS_KAANAPALI:
case CAMSS_X1E80100:
diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
index e0b181ff33aa..011ece0a7692 100644
--- a/drivers/media/platform/qcom/camss/camss.c
+++ b/drivers/media/platform/qcom/camss/camss.c
@@ -6160,9 +6160,11 @@ static const struct camss_resources sm8750_resources = {
.pd_name = "top",
.csiphy_res = csiphy_res_kaanapali,
.csid_res = csid_res_sm8750,
+ .vfe_res = vfe_res_kaanapali,
.icc_res = icc_res_sm8750,
.csiphy_num = ARRAY_SIZE(csiphy_res_kaanapali),
.csid_num = ARRAY_SIZE(csid_res_sm8750),
+ .vfe_num = ARRAY_SIZE(vfe_res_kaanapali),
.icc_path_num = ARRAY_SIZE(icc_res_sm8750),
};
--
2.34.1
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* [PATCH v6 7/7] media: qcom: camss: tpg: Add support for v2.3.0 TPG
From: Hangxiang Ma @ 2026-07-21 12:46 UTC (permalink / raw)
To: Robert Foss, Todor Tomov, Bryan O'Donoghue,
Vladimir Zapolskiy, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bryan O'Donoghue,
Loic Poulain, Vinod Koul, Neil Armstrong
Cc: linux-media, linux-arm-msm, devicetree, linux-kernel, linux-phy,
jeyaprakash.soundrapandian, Vijay Kumar Tumati, Hangxiang Ma
In-Reply-To: <20260721-add-support-for-camss-on-sm8750-v6-0-a5f841061d18@oss.qualcomm.com>
Add support for the TPG found on SM8750. This TPG uses hardware version
2.3.0, which drives test-enable and reset through a separate
TPG_CTRL_CMD register instead of TPG_CTRL, and routes its output into
the CSID 980 RX via the CSI2 TPG mux.
The 2.3.0 and 2.4.0 (Kaanapali) TPGs share the same programming model,
so reuse the Kaanapali TPG resources for SM8750 and lower the hardware
version gate in the TPG gen1 driver from 2.4.0 to 2.3.0.
Signed-off-by: Hangxiang Ma <hangxiang.ma@oss.qualcomm.com>
---
drivers/media/platform/qcom/camss/camss-csid-980.c | 15 +++++++++++++--
drivers/media/platform/qcom/camss/camss-tpg-gen1.c | 9 +++++----
drivers/media/platform/qcom/camss/camss.c | 2 ++
3 files changed, 20 insertions(+), 6 deletions(-)
diff --git a/drivers/media/platform/qcom/camss/camss-csid-980.c b/drivers/media/platform/qcom/camss/camss-csid-980.c
index 6714b8aa8c2a..16184fe69398 100644
--- a/drivers/media/platform/qcom/camss/camss-csid-980.c
+++ b/drivers/media/platform/qcom/camss/camss-csid-980.c
@@ -68,6 +68,8 @@
#define CSI2_RX_CFG0_PHY_SEL_BASE_IDX 1
#define CSI2_RX_CFG0_DL0_INPUT_SEL 4
#define CSI2_RX_CFG0_PHY_NUM_SEL 20
+#define CSI2_RX_CFG0_TPG_MUX_EN BIT(27)
+#define CSI2_RX_CFG0_TPG_MUX_SEL GENMASK(29, 28)
#define CSID_CSI2_RX_CFG1 (csid_is_lite(csid) ? 0x204 : 0x404)
#define CSI2_RX_CFG1_ECC_CORRECTION_EN BIT(0)
#define CSI2_RX_CFG1_VC_MODE BIT(2)
@@ -183,12 +185,21 @@ static void __csid_aup_rup_clear(struct csid_device *csid, int port_id)
static void __csid_configure_rx(struct csid_device *csid,
struct csid_phy_config *phy)
{
+ struct camss *camss = csid->camss;
int val;
val = (phy->lane_cnt - 1) << CSI2_RX_CFG0_NUM_ACTIVE_LANES;
val |= phy->lane_assign << CSI2_RX_CFG0_DL0_INPUT_SEL;
- val |= (phy->csiphy_id + CSI2_RX_CFG0_PHY_SEL_BASE_IDX)
- << CSI2_RX_CFG0_PHY_NUM_SEL;
+
+ if (camss->tpg && csid->tpg_linked &&
+ camss->tpg[phy->csiphy_id].testgen.mode != TPG_PAYLOAD_MODE_DISABLED) {
+ val |= FIELD_PREP(CSI2_RX_CFG0_TPG_MUX_SEL, phy->csiphy_id + 1);
+ val |= CSI2_RX_CFG0_TPG_MUX_EN;
+ } else {
+ val |= (phy->csiphy_id + CSI2_RX_CFG0_PHY_SEL_BASE_IDX)
+ << CSI2_RX_CFG0_PHY_NUM_SEL;
+ }
+
writel(val, csid->base + CSID_CSI2_RX_CFG0);
val = CSI2_RX_CFG1_ECC_CORRECTION_EN;
diff --git a/drivers/media/platform/qcom/camss/camss-tpg-gen1.c b/drivers/media/platform/qcom/camss/camss-tpg-gen1.c
index 770a9e5d5ba5..5c452f7a412d 100644
--- a/drivers/media/platform/qcom/camss/camss-tpg-gen1.c
+++ b/drivers/media/platform/qcom/camss/camss-tpg-gen1.c
@@ -22,6 +22,7 @@
#define TPG_HW_VER_2_0_0 TPG_HW_VER(2, 0, 0)
#define TPG_HW_VER_2_1_0 TPG_HW_VER(2, 1, 0)
+#define TPG_HW_VER_2_3_0 TPG_HW_VER(2, 3, 0)
#define TPG_HW_VER_2_4_0 TPG_HW_VER(2, 4, 0)
#define TPG_HW_STATUS 0x4
@@ -170,7 +171,7 @@ static int tpg_stream_on(struct tpg_device *tpg)
val = FIELD_PREP(TPG_CTRL_NUM_ACTIVE_LANES, lane_cnt - 1) |
FIELD_PREP(TPG_CTRL_NUM_ACTIVE_VC, last_vc);
- if (tpg->hw_version >= TPG_HW_VER_2_4_0) {
+ if (tpg->hw_version >= TPG_HW_VER_2_3_0) {
writel(val, tpg->base + TPG_CTRL);
writel(TPG_CTRL_CMD_TEST_EN, tpg->base + TPG_CTRL_CMD);
} else {
@@ -184,10 +185,10 @@ static int tpg_stream_on(struct tpg_device *tpg)
static int tpg_reset(struct tpg_device *tpg)
{
/*
- * On TPG older than v2.4.0 test-enable lives in TPG_CTRL, so clear it
- * first; v2.4.0+ drives both test-enable and reset through TPG_CTRL_CMD.
+ * On TPG older than v2.3.0 test-enable lives in TPG_CTRL, so clear it
+ * first; v2.3.0+ drives both test-enable and reset through TPG_CTRL_CMD.
*/
- if (tpg->hw_version < TPG_HW_VER_2_4_0)
+ if (tpg->hw_version < TPG_HW_VER_2_3_0)
writel(0, tpg->base + TPG_CTRL);
writel(TPG_CTRL_CMD_HW_RESET, tpg->base + TPG_CTRL_CMD);
diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
index 011ece0a7692..418deb74cd98 100644
--- a/drivers/media/platform/qcom/camss/camss.c
+++ b/drivers/media/platform/qcom/camss/camss.c
@@ -6159,10 +6159,12 @@ static const struct camss_resources sm8750_resources = {
.version = CAMSS_8750,
.pd_name = "top",
.csiphy_res = csiphy_res_kaanapali,
+ .tpg_res = tpg_res_x1e80100,
.csid_res = csid_res_sm8750,
.vfe_res = vfe_res_kaanapali,
.icc_res = icc_res_sm8750,
.csiphy_num = ARRAY_SIZE(csiphy_res_kaanapali),
+ .tpg_num = ARRAY_SIZE(tpg_res_x1e80100),
.csid_num = ARRAY_SIZE(csid_res_sm8750),
.vfe_num = ARRAY_SIZE(vfe_res_kaanapali),
.icc_path_num = ARRAY_SIZE(icc_res_sm8750),
--
2.34.1
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* [PATCH v5 0/5] phy: qcom: Fix possible NULL-deref and runtime PM race conditions
From: Loic Poulain @ 2026-07-21 13:05 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Dmitry Baryshkov, Wesley Cheng
Cc: linux-arm-msm, linux-phy, linux-kernel, Dmitry Baryshkov,
Abel Vesa, Konrad Dybcio, Loic Poulain
Address potential NULL pointer dereferences and race conditions related
to runtime PM in several Qualcomm PHY drivers. In all cases, enabling
runtime PM before the PHY instance is fully initialized can lead to
crashes during early runtime suspend callbacks.
- Attach driver data before enabling runtime PM.
- Introduce initialization flags where needed to avoid dereferencing
uninitialized pointers.
- Reorder pm_runtime_enable() and pm_runtime_forbid() calls to prevent
unnecessary suspend/resume cycles during driver probe.
- Use devres-managed PM runtime helpers for proper cleanup.
Why it happens?
The PHY is a supplier of the USB device. A USB device cannot be probed
until all its suppliers are ready. As long as the PHY is not ready, the
device core keeps retrying the probe, which fails with -EPROBE_DEFER.
At some point the PHY probe finally runs, but the device core may still be
attempting to probe the USB device concurrently.
Inside __driver_probe_device(), we have:
ret = really_probe(dev, drv);
pm_request_idle(dev);
if (dev->parent)
pm_runtime_put(dev->parent);
pm_runtime_put_suppliers(dev);
return ret;
This means that whenever a USB probe attempt completes, whether with an
error or not, its suppliers are released via pm_runtime_put_suppliers().
Releasing suppliers may in turn trigger a runtime suspend.
In our case, since the PHY is a supplier of the USB device, the USB core
keeps 'looping' in __driver_probe_device() returning -EPROBE_DEFER until
the PHY becomes ready. As a result, pm_runtime_put_suppliers() may run
concurrently with the PHY's probe function. If this happens after
runtime PM has been enabled for the PHY, but before the driver has
forbidden suspend or taken a PM reference, the PHY may end up being
runtime-suspended 'unexpectedly'
---
Changes in v5:
- Re-introduce phy: qcom: qmp-usb-legacy: Fix possible NULL-deref (sashiko)
- Link to v4: https://lore.kernel.org/r/20260720-qcom-usb-phy-fix-null-v4-0-4d2a0f06d53a@oss.qualcomm.com
Changes in V4:
- Instead of moving pm/forbid, increment the pm usage counter before
enabling runtime pm and decrement it after the PHY has been created. (Johan)
- Drop now unnecessary dev_set_drvdata() move in snps-femto-v2
- Drop 4/5 (qmp-usb-legacy: Prevent unnecessary PM runtime suspend at boot)
which is not required anymore.
- Rebase
- Link to v3: https://lore.kernel.org/all/20260205160240.748371-1-loic.poulain@oss.qualcomm.com/
Changes in v3:
Rebase on next and remove 2/6 (obsolete)
Changes in v2:
Split patches 2/4 and 3/4 so that the null‑pointer dereference fix and
the runtime‑PM enable/forbid reordering are logically separated.
Loic Poulain (4):
phy: qcom: qmp-combo: Prevent unnecessary PM runtime suspend at boot
phy: qcom: qmp-usbc: Prevent unnecessary PM runtime suspend at boot
phy: qcom: qmp-usb-legacy: Fix possible NULL-deref on early runtime
suspend
phy: qcom: snps-femto-v2: Fix possible NULL-deref on early runtime
suspend
drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 10 ++++-----
.../phy/qualcomm/phy-qcom-qmp-usb-legacy.c | 21 ++++++++++++-------
drivers/phy/qualcomm/phy-qcom-qmp-usbc.c | 10 ++++-----
drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c | 15 ++++++-------
4 files changed, 32 insertions(+), 24 deletions(-)
--
2.34.1
To: Vinod Koul <vkoul@kernel.org>
To: Neil Armstrong <neil.armstrong@linaro.org>
To: Dmitry Baryshkov <lumag@kernel.org>
To: Wesley Cheng <quic_wcheng@quicinc.com>
Cc: linux-arm-msm@vger.kernel.org
Cc: linux-phy@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
---
Loic Poulain (5):
phy: qcom: qmp-combo: Prevent unnecessary PM runtime suspend at boot
phy: qcom: qmp-usbc: Prevent unnecessary PM runtime suspend at boot
phy: qcom: qmp-usb-legacy: Fix possible NULL-deref on early runtime suspend
phy: qcom: qmp-usb-legacy: Prevent unnecessary PM runtime suspend at boot
phy: qcom: snps-femto-v2: Fix possible NULL-deref on early runtime suspend
drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 19 ++++++++++++-----
drivers/phy/qualcomm/phy-qcom-qmp-usb-legacy.c | 28 +++++++++++++++++++++-----
drivers/phy/qualcomm/phy-qcom-qmp-usbc.c | 18 +++++++++++++----
drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c | 17 +++++++++++++---
4 files changed, 65 insertions(+), 17 deletions(-)
---
base-commit: 910b828b22b7b91054b3bd4be676a017444b0e00
change-id: 20260720-qcom-usb-phy-fix-null-ff097f56d1e4
Best regards,
--
Loic Poulain <loic.poulain@oss.qualcomm.com>
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* [PATCH v5 1/5] phy: qcom: qmp-combo: Prevent unnecessary PM runtime suspend at boot
From: Loic Poulain @ 2026-07-21 13:05 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Dmitry Baryshkov, Wesley Cheng
Cc: linux-arm-msm, linux-phy, linux-kernel, Dmitry Baryshkov,
Abel Vesa, Konrad Dybcio, Loic Poulain
In-Reply-To: <20260721-qcom-usb-phy-fix-null-v5-0-a181e2adbd2d@oss.qualcomm.com>
Runtime PM has to be enabled before creating the PHYs, since phy_create()
only enables runtime PM on the PHY devices if it is already enabled on
this parent device. This opens a small window where the device can be
runtime suspended after pm_runtime_enable() and before the later
pm_runtime_forbid(), causing an unnecessary suspend/resume cycle while
the PHYs are not yet registered.
Take a runtime PM usage reference with pm_runtime_get_noresume() before
enabling runtime PM and release it once the PHYs have been created to
prevent the device from being runtime suspended during that window.
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Reviewed-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
---
drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 19 ++++++++++++++-----
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
index cdcfad2e86b1d37650c4d7b0433319837ee9c9d4..affc8e040bb968a5b9d0b6f6fc2694c471368729 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
@@ -4934,10 +4934,16 @@ static int qmp_combo_probe(struct platform_device *pdev)
if (ret)
goto err_node_put;
+ /*
+ * Enable runtime PM before creating the PHYs, phy_create() only enables
+ * it on the PHY devices if already enabled on the parent. Hold a usage
+ * reference so callbacks cannot run until the PHY is ready.
+ */
+ pm_runtime_get_noresume(dev);
pm_runtime_set_active(dev);
ret = devm_pm_runtime_enable(dev);
if (ret)
- goto err_node_put;
+ goto err_pm_put;
/*
* Prevent runtime pm from being ON by default. Users can enable
* it using power/control in sysfs.
@@ -4946,14 +4952,13 @@ static int qmp_combo_probe(struct platform_device *pdev)
ret = qmp_combo_register_clocks(qmp, usb_np, dp_np);
if (ret)
- goto err_node_put;
-
+ goto err_pm_put;
qmp->usb_phy = devm_phy_create(dev, usb_np, &qmp_combo_usb_phy_ops);
if (IS_ERR(qmp->usb_phy)) {
ret = PTR_ERR(qmp->usb_phy);
dev_err(dev, "failed to create USB PHY: %d\n", ret);
- goto err_node_put;
+ goto err_pm_put;
}
phy_set_drvdata(qmp->usb_phy, qmp);
@@ -4962,11 +4967,13 @@ static int qmp_combo_probe(struct platform_device *pdev)
if (IS_ERR(qmp->dp_phy)) {
ret = PTR_ERR(qmp->dp_phy);
dev_err(dev, "failed to create DP PHY: %d\n", ret);
- goto err_node_put;
+ goto err_pm_put;
}
phy_set_drvdata(qmp->dp_phy, qmp);
+ pm_runtime_put(dev);
+
if (usb_np == dev->of_node)
phy_provider = devm_of_phy_provider_register(dev, qmp_combo_phy_xlate);
else
@@ -4977,6 +4984,8 @@ static int qmp_combo_probe(struct platform_device *pdev)
return PTR_ERR_OR_ZERO(phy_provider);
+err_pm_put:
+ pm_runtime_put_noidle(dev);
err_node_put:
of_node_put(usb_np);
of_node_put(dp_np);
--
2.34.1
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* [PATCH v5 2/5] phy: qcom: qmp-usbc: Prevent unnecessary PM runtime suspend at boot
From: Loic Poulain @ 2026-07-21 13:05 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Dmitry Baryshkov, Wesley Cheng
Cc: linux-arm-msm, linux-phy, linux-kernel, Konrad Dybcio, Abel Vesa,
Dmitry Baryshkov, Loic Poulain
In-Reply-To: <20260721-qcom-usb-phy-fix-null-v5-0-a181e2adbd2d@oss.qualcomm.com>
Runtime PM has to be enabled before creating the PHYs, since phy_create()
only enables runtime PM on the PHY devices if it is already enabled on
this parent device. This opens a small window where the device can be
runtime suspended after pm_runtime_enable() and before the later
pm_runtime_forbid(), causing an unnecessary suspend/resume cycle while
the PHYs are not yet registered.
Take a runtime PM usage reference with pm_runtime_get_noresume() before
enabling runtime PM and release it once the PHYs have been created to
prevent the device from being runtime suspended during that window.
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Reviewed-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
---
drivers/phy/qualcomm/phy-qcom-qmp-usbc.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c b/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c
index ab3055bb5b0c198832ae06dfcd04fd34395e271d..4317224070fd8dec98090e9200ac7d0e97265979 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c
@@ -1959,10 +1959,16 @@ static int qmp_usbc_probe(struct platform_device *pdev)
if (ret)
goto err_node_put;
+ /*
+ * Enable runtime PM before creating the PHYs, phy_create() only enables
+ * it on the PHY devices if already enabled on the parent. Hold a usage
+ * reference so callbacks cannot run until the PHY is ready.
+ */
+ pm_runtime_get_noresume(dev);
pm_runtime_set_active(dev);
ret = devm_pm_runtime_enable(dev);
if (ret)
- goto err_node_put;
+ goto err_pm_put;
/*
* Prevent runtime pm from being ON by default. Users can enable
* it using power/control in sysfs.
@@ -1971,13 +1977,13 @@ static int qmp_usbc_probe(struct platform_device *pdev)
ret = qmp_usbc_register_clocks(qmp, np);
if (ret)
- goto err_node_put;
+ goto err_pm_put;
qmp->usb_phy = devm_phy_create(dev, np, &qmp_usbc_usb_phy_ops);
if (IS_ERR(qmp->usb_phy)) {
ret = PTR_ERR(qmp->usb_phy);
dev_err(dev, "failed to create PHY: %d\n", ret);
- goto err_node_put;
+ goto err_pm_put;
}
phy_set_drvdata(qmp->usb_phy, qmp);
@@ -1987,17 +1993,21 @@ static int qmp_usbc_probe(struct platform_device *pdev)
if (IS_ERR(qmp->dp_phy)) {
ret = PTR_ERR(qmp->dp_phy);
dev_err(dev, "failed to create PHY: %d\n", ret);
- goto err_node_put;
+ goto err_pm_put;
}
phy_set_drvdata(qmp->dp_phy, qmp);
}
+ pm_runtime_put(dev);
+
of_node_put(np);
phy_provider = devm_of_phy_provider_register(dev, qmp_usbc_phy_xlate);
return PTR_ERR_OR_ZERO(phy_provider);
+err_pm_put:
+ pm_runtime_put_noidle(dev);
err_node_put:
of_node_put(np);
return ret;
--
2.34.1
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* [PATCH v5 4/5] phy: qcom: qmp-usb-legacy: Prevent unnecessary PM runtime suspend at boot
From: Loic Poulain @ 2026-07-21 13:05 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Dmitry Baryshkov, Wesley Cheng
Cc: linux-arm-msm, linux-phy, linux-kernel, Abel Vesa,
Dmitry Baryshkov, Loic Poulain
In-Reply-To: <20260721-qcom-usb-phy-fix-null-v5-0-a181e2adbd2d@oss.qualcomm.com>
Runtime PM has to be enabled before creating the PHY, since phy_create()
only enables runtime PM on the PHY device if it is already enabled on
this parent device. This opens a small window where the device can be
runtime suspended after pm_runtime_enable() and before the later
pm_runtime_forbid(), causing an unnecessary suspend/resume cycle while
the PHY is not yet registered.
Take a runtime PM usage reference with pm_runtime_get_noresume() before
enabling runtime PM and release it once the PHY has been created to
prevent the device from being runtime suspended during that window.
Reviewed-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
---
drivers/phy/qualcomm/phy-qcom-qmp-usb-legacy.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-usb-legacy.c b/drivers/phy/qualcomm/phy-qcom-qmp-usb-legacy.c
index fc490589c8e4888770807a60170905a1c7610421..eb1826fb26f0868f7758245b0cbe1a0590fea9a3 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-usb-legacy.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-usb-legacy.c
@@ -1285,10 +1285,16 @@ static int qmp_usb_legacy_probe(struct platform_device *pdev)
if (ret)
goto err_node_put;
+ /*
+ * Enable runtime PM before creating the PHY, phy_create() only enables
+ * it on the PHY device if already enabled on the parent. Hold a usage
+ * reference so callbacks cannot run before qmp->phy is assigned.
+ */
+ pm_runtime_get_noresume(dev);
pm_runtime_set_active(dev);
ret = devm_pm_runtime_enable(dev);
if (ret)
- goto err_node_put;
+ goto err_pm_put;
/*
* Prevent runtime pm from being ON by default. Users can enable
* it using power/control in sysfs.
@@ -1297,23 +1303,27 @@ static int qmp_usb_legacy_probe(struct platform_device *pdev)
ret = phy_pipe_clk_register(qmp, np);
if (ret)
- goto err_node_put;
+ goto err_pm_put;
qmp->phy = devm_phy_create(dev, np, &qmp_usb_legacy_phy_ops);
if (IS_ERR(qmp->phy)) {
ret = PTR_ERR(qmp->phy);
dev_err(dev, "failed to create PHY: %d\n", ret);
- goto err_node_put;
+ goto err_pm_put;
}
phy_set_drvdata(qmp->phy, qmp);
+ pm_runtime_put(dev);
+
of_node_put(np);
phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
return PTR_ERR_OR_ZERO(phy_provider);
+err_pm_put:
+ pm_runtime_put_noidle(dev);
err_node_put:
of_node_put(np);
return ret;
--
2.34.1
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
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