* [PATCH 0/5] phy: rockchip: phy-rockchip-inno-csidphy: add support for rk3588 variant
@ 2025-06-17 8:54 Michael Riesch via B4 Relay
2025-06-17 8:54 ` [PATCH 1/5] dt-bindings: soc: rockchip: add rk3588 csidphy grf syscon Michael Riesch via B4 Relay
` (4 more replies)
0 siblings, 5 replies; 15+ messages in thread
From: Michael Riesch via B4 Relay @ 2025-06-17 8:54 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner,
Vinod Koul, Kishon Vijay Abraham I, Philipp Zabel, Jagan Teki,
Sebastian Reichel, Collabora Kernel Team
Cc: devicetree, linux-arm-kernel, linux-rockchip, linux-kernel,
linux-phy, Michael Riesch
Habidere,
The Rockchip RK3588 features two MIPI CSI-2 DPHYs (not to be confused with
the two combo MIPI DSI/CSI CPHY/DPHY blocks). The CSI-2 DPHYs can be
supported using the existing phy-rockchip-inno-csidphy driver, the notable
differences being
- the control bits in the GRF
- the additional reset line
This patch series adds support for this variant.
As you may have guessed, this is part of the efforts to bring the support
for the RK3588 Video Capture (VICAP) unit mainline.
Looking forward to your comments!
Signed-off-by: Michael Riesch <michael.riesch@collabora.com>
---
Michael Riesch (5):
dt-bindings: soc: rockchip: add rk3588 csidphy grf syscon
dt-bindings: phy: rockchip-inno-csi-dphy: add rk3588 variant
phy: rockchip: phy-rockchip-inno-csidphy: allow writes to grf register 0
phy: rockchip: phy-rockchip-inno-csidphy: add support for rk3588 variant
arm64: dts: rockchip: add mipi csi-2 dphy nodes to rk3588
.../bindings/phy/rockchip-inno-csi-dphy.yaml | 60 ++++++++++++++++++++--
.../devicetree/bindings/soc/rockchip/grf.yaml | 1 +
arch/arm64/boot/dts/rockchip/rk3588-base.dtsi | 31 +++++++++++
drivers/phy/rockchip/phy-rockchip-inno-csidphy.c | 28 ++++++++--
4 files changed, 112 insertions(+), 8 deletions(-)
---
base-commit: 19272b37aa4f83ca52bdf9c16d5d81bdd1354494
change-id: 20250616-rk3588-csi-dphy-c9ed2ad4cd9f
Best regards,
--
Michael Riesch <michael.riesch@collabora.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/5] dt-bindings: soc: rockchip: add rk3588 csidphy grf syscon
2025-06-17 8:54 [PATCH 0/5] phy: rockchip: phy-rockchip-inno-csidphy: add support for rk3588 variant Michael Riesch via B4 Relay
@ 2025-06-17 8:54 ` Michael Riesch via B4 Relay
2025-06-27 19:14 ` Rob Herring (Arm)
2025-06-17 8:54 ` [PATCH 2/5] dt-bindings: phy: rockchip-inno-csi-dphy: add rk3588 variant Michael Riesch via B4 Relay
` (3 subsequent siblings)
4 siblings, 1 reply; 15+ messages in thread
From: Michael Riesch via B4 Relay @ 2025-06-17 8:54 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner,
Vinod Koul, Kishon Vijay Abraham I, Philipp Zabel, Jagan Teki,
Sebastian Reichel, Collabora Kernel Team
Cc: devicetree, linux-arm-kernel, linux-rockchip, linux-kernel,
linux-phy, Michael Riesch
From: Michael Riesch <michael.riesch@collabora.com>
Add CSIDPHY GRF syscon compatible for the Rockchip RK3588.
Signed-off-by: Michael Riesch <michael.riesch@collabora.com>
---
Documentation/devicetree/bindings/soc/rockchip/grf.yaml | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/soc/rockchip/grf.yaml b/Documentation/devicetree/bindings/soc/rockchip/grf.yaml
index ccdcc889ba8e..3a1035fbd8dc 100644
--- a/Documentation/devicetree/bindings/soc/rockchip/grf.yaml
+++ b/Documentation/devicetree/bindings/soc/rockchip/grf.yaml
@@ -47,6 +47,7 @@ properties:
- rockchip,rk3576-vop-grf
- rockchip,rk3588-bigcore0-grf
- rockchip,rk3588-bigcore1-grf
+ - rockchip,rk3588-csidphy-grf
- rockchip,rk3588-dcphy-grf
- rockchip,rk3588-hdptxphy-grf
- rockchip,rk3588-ioc
--
2.39.5
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 2/5] dt-bindings: phy: rockchip-inno-csi-dphy: add rk3588 variant
2025-06-17 8:54 [PATCH 0/5] phy: rockchip: phy-rockchip-inno-csidphy: add support for rk3588 variant Michael Riesch via B4 Relay
2025-06-17 8:54 ` [PATCH 1/5] dt-bindings: soc: rockchip: add rk3588 csidphy grf syscon Michael Riesch via B4 Relay
@ 2025-06-17 8:54 ` Michael Riesch via B4 Relay
2025-06-17 9:31 ` neil.armstrong
2025-06-17 14:12 ` Diederik de Haas
2025-06-17 8:54 ` [PATCH 3/5] phy: rockchip: phy-rockchip-inno-csidphy: allow writes to grf register 0 Michael Riesch via B4 Relay
` (2 subsequent siblings)
4 siblings, 2 replies; 15+ messages in thread
From: Michael Riesch via B4 Relay @ 2025-06-17 8:54 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner,
Vinod Koul, Kishon Vijay Abraham I, Philipp Zabel, Jagan Teki,
Sebastian Reichel, Collabora Kernel Team
Cc: devicetree, linux-arm-kernel, linux-rockchip, linux-kernel,
linux-phy, Michael Riesch
From: Michael Riesch <michael.riesch@collabora.com>
The Rockchip RK3588 variant of the CSI-2 DPHY features two reset lines.
Add the variant and allow for the additional reset.
Signed-off-by: Michael Riesch <michael.riesch@collabora.com>
---
.../bindings/phy/rockchip-inno-csi-dphy.yaml | 60 ++++++++++++++++++++--
1 file changed, 55 insertions(+), 5 deletions(-)
diff --git a/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml b/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml
index 5ac994b3c0aa..6755738b13ee 100644
--- a/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml
+++ b/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml
@@ -21,6 +21,7 @@ properties:
- rockchip,rk3326-csi-dphy
- rockchip,rk3368-csi-dphy
- rockchip,rk3568-csi-dphy
+ - rockchip,rk3588-csi-dphy
reg:
maxItems: 1
@@ -39,18 +40,49 @@ properties:
maxItems: 1
resets:
- items:
- - description: exclusive PHY reset line
+ minItems: 1
+ maxItems: 2
reset-names:
- items:
- - const: apb
+ minItems: 1
+ maxItems: 2
rockchip,grf:
$ref: /schemas/types.yaml#/definitions/phandle
description:
Some additional phy settings are access through GRF regs.
+allOf:
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - rockchip,px30-csi-dphy
+ - rockchip,rk1808-csi-dphy
+ - rockchip,rk3326-csi-dphy
+ - rockchip,rk3368-csi-dphy
+ - rockchip,rk3568-csi-dphy
+ then:
+ properties:
+ resets:
+ items:
+ - description: exclusive PHY reset line
+
+ reset-names:
+ items:
+ - const: apb
+
+ required:
+ - reset-names
+ else:
+ properties:
+ resets:
+ minItems: 2
+
+ reset-names:
+ minItems: 2
+
required:
- compatible
- reg
@@ -59,7 +91,6 @@ required:
- '#phy-cells'
- power-domains
- resets
- - reset-names
- rockchip,grf
additionalProperties: false
@@ -78,3 +109,22 @@ examples:
reset-names = "apb";
rockchip,grf = <&grf>;
};
+ - |
+ #include <dt-bindings/clock/rockchip,rk3588-cru.h>
+ #include <dt-bindings/reset/rockchip,rk3588-cru.h>
+
+ soc {
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ csi_dphy0: phy@fedc0000 {
+ compatible = "rockchip,rk3588-csi-dphy";
+ reg = <0x0 0xfedc0000 0x0 0x8000>;
+ clocks = <&cru PCLK_CSIPHY0>;
+ clock-names = "pclk";
+ #phy-cells = <0>;
+ resets = <&cru SRST_CSIPHY0>, <&cru SRST_P_CSIPHY0>;
+ rockchip,grf = <&csidphy0_grf>;
+ status = "disabled";
+ };
+ };
--
2.39.5
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 3/5] phy: rockchip: phy-rockchip-inno-csidphy: allow writes to grf register 0
2025-06-17 8:54 [PATCH 0/5] phy: rockchip: phy-rockchip-inno-csidphy: add support for rk3588 variant Michael Riesch via B4 Relay
2025-06-17 8:54 ` [PATCH 1/5] dt-bindings: soc: rockchip: add rk3588 csidphy grf syscon Michael Riesch via B4 Relay
2025-06-17 8:54 ` [PATCH 2/5] dt-bindings: phy: rockchip-inno-csi-dphy: add rk3588 variant Michael Riesch via B4 Relay
@ 2025-06-17 8:54 ` Michael Riesch via B4 Relay
2025-06-17 9:30 ` neil.armstrong
2025-06-17 8:54 ` [PATCH 4/5] phy: rockchip: phy-rockchip-inno-csidphy: add support for rk3588 variant Michael Riesch via B4 Relay
2025-06-17 8:54 ` [PATCH 5/5] arm64: dts: rockchip: add mipi csi-2 dphy nodes to rk3588 Michael Riesch via B4 Relay
4 siblings, 1 reply; 15+ messages in thread
From: Michael Riesch via B4 Relay @ 2025-06-17 8:54 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner,
Vinod Koul, Kishon Vijay Abraham I, Philipp Zabel, Jagan Teki,
Sebastian Reichel, Collabora Kernel Team
Cc: devicetree, linux-arm-kernel, linux-rockchip, linux-kernel,
linux-phy, Michael Riesch
From: Michael Riesch <michael.riesch@collabora.com>
The driver for the Rockchip MIPI CSI-2 DPHY uses GRF register offset
value 0 to sort out undefined registers. However, the RK3588 CSIDPHY GRF
this offset is perfectly fine (in fact, register 0 is the only one in
this register y
file).
Introduce a boolean variable to indicate valid registers and allow writes
to register 0.
Signed-off-by: Michael Riesch <michael.riesch@collabora.com>
---
drivers/phy/rockchip/phy-rockchip-inno-csidphy.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-inno-csidphy.c b/drivers/phy/rockchip/phy-rockchip-inno-csidphy.c
index 2ab99e1d47eb..75533d071025 100644
--- a/drivers/phy/rockchip/phy-rockchip-inno-csidphy.c
+++ b/drivers/phy/rockchip/phy-rockchip-inno-csidphy.c
@@ -87,10 +87,11 @@ struct dphy_reg {
u32 offset;
u32 mask;
u32 shift;
+ u8 valid;
};
#define PHY_REG(_offset, _width, _shift) \
- { .offset = _offset, .mask = BIT(_width) - 1, .shift = _shift, }
+ { .offset = _offset, .mask = BIT(_width) - 1, .shift = _shift, .valid = 1, }
static const struct dphy_reg rk1808_grf_dphy_regs[] = {
[GRF_DPHY_CSIPHY_FORCERXMODE] = PHY_REG(RK1808_GRF_PD_VI_CON_OFFSET, 4, 0),
@@ -145,7 +146,7 @@ static inline void write_grf_reg(struct rockchip_inno_csidphy *priv,
const struct dphy_drv_data *drv_data = priv->drv_data;
const struct dphy_reg *reg = &drv_data->grf_regs[index];
- if (reg->offset)
+ if (reg->valid)
regmap_write(priv->grf, reg->offset,
HIWORD_UPDATE(value, reg->mask, reg->shift));
}
--
2.39.5
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 4/5] phy: rockchip: phy-rockchip-inno-csidphy: add support for rk3588 variant
2025-06-17 8:54 [PATCH 0/5] phy: rockchip: phy-rockchip-inno-csidphy: add support for rk3588 variant Michael Riesch via B4 Relay
` (2 preceding siblings ...)
2025-06-17 8:54 ` [PATCH 3/5] phy: rockchip: phy-rockchip-inno-csidphy: allow writes to grf register 0 Michael Riesch via B4 Relay
@ 2025-06-17 8:54 ` Michael Riesch via B4 Relay
2025-06-17 9:36 ` neil.armstrong
2025-06-17 8:54 ` [PATCH 5/5] arm64: dts: rockchip: add mipi csi-2 dphy nodes to rk3588 Michael Riesch via B4 Relay
4 siblings, 1 reply; 15+ messages in thread
From: Michael Riesch via B4 Relay @ 2025-06-17 8:54 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner,
Vinod Koul, Kishon Vijay Abraham I, Philipp Zabel, Jagan Teki,
Sebastian Reichel, Collabora Kernel Team
Cc: devicetree, linux-arm-kernel, linux-rockchip, linux-kernel,
linux-phy, Michael Riesch
From: Michael Riesch <michael.riesch@collabora.com>
The Rockchip RK3588 MIPI CSI-2 DPHY can be supported using the existing
phy-rockchip-inno-csidphy driver, the notable differences being
- the control bits in the GRF
- the additional reset line
Add support for this variant.
Signed-off-by: Michael Riesch <michael.riesch@collabora.com>
---
drivers/phy/rockchip/phy-rockchip-inno-csidphy.c | 23 ++++++++++++++++++++++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-inno-csidphy.c b/drivers/phy/rockchip/phy-rockchip-inno-csidphy.c
index 75533d071025..0840be668bfd 100644
--- a/drivers/phy/rockchip/phy-rockchip-inno-csidphy.c
+++ b/drivers/phy/rockchip/phy-rockchip-inno-csidphy.c
@@ -30,6 +30,8 @@
#define RK3568_GRF_VI_CON0 0x0340
#define RK3568_GRF_VI_CON1 0x0344
+#define RK3588_CSIDPHY_GRF_CON0 0x0000
+
/* PHY */
#define CSIDPHY_CTRL_LANE_ENABLE 0x00
#define CSIDPHY_CTRL_LANE_ENABLE_CK BIT(6)
@@ -115,6 +117,12 @@ static const struct dphy_reg rk3568_grf_dphy_regs[] = {
[GRF_DPHY_CSIPHY_CLKLANE_EN] = PHY_REG(RK3568_GRF_VI_CON0, 1, 8),
};
+static const struct dphy_reg rk3588_grf_dphy_regs[] = {
+ [GRF_DPHY_CSIPHY_FORCERXMODE] = PHY_REG(RK3588_CSIDPHY_GRF_CON0, 4, 0),
+ [GRF_DPHY_CSIPHY_DATALANE_EN] = PHY_REG(RK3588_CSIDPHY_GRF_CON0, 4, 4),
+ [GRF_DPHY_CSIPHY_CLKLANE_EN] = PHY_REG(RK3588_CSIDPHY_GRF_CON0, 1, 8),
+};
+
struct hsfreq_range {
u32 range_h;
u8 cfg_bit;
@@ -373,6 +381,15 @@ static const struct dphy_drv_data rk3568_mipidphy_drv_data = {
.grf_regs = rk3568_grf_dphy_regs,
};
+static const struct dphy_drv_data rk3588_mipidphy_drv_data = {
+ .pwrctl_offset = -1,
+ .ths_settle_offset = RK3568_CSIDPHY_CLK_WR_THS_SETTLE,
+ .calib_offset = RK3568_CSIDPHY_CLK_CALIB_EN,
+ .hsfreq_ranges = rk1808_mipidphy_hsfreq_ranges,
+ .num_hsfreq_ranges = ARRAY_SIZE(rk1808_mipidphy_hsfreq_ranges),
+ .grf_regs = rk3588_grf_dphy_regs,
+};
+
static const struct of_device_id rockchip_inno_csidphy_match_id[] = {
{
.compatible = "rockchip,px30-csi-dphy",
@@ -394,6 +411,10 @@ static const struct of_device_id rockchip_inno_csidphy_match_id[] = {
.compatible = "rockchip,rk3568-csi-dphy",
.data = &rk3568_mipidphy_drv_data,
},
+ {
+ .compatible = "rockchip,rk3588-csi-dphy",
+ .data = &rk3588_mipidphy_drv_data,
+ },
{}
};
MODULE_DEVICE_TABLE(of, rockchip_inno_csidphy_match_id);
@@ -435,7 +456,7 @@ static int rockchip_inno_csidphy_probe(struct platform_device *pdev)
return PTR_ERR(priv->pclk);
}
- priv->rst = devm_reset_control_get(dev, "apb");
+ priv->rst = devm_reset_control_array_get(dev, RESET_CONTROL_EXCLUSIVE);
if (IS_ERR(priv->rst)) {
dev_err(dev, "failed to get system reset control\n");
return PTR_ERR(priv->rst);
--
2.39.5
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 5/5] arm64: dts: rockchip: add mipi csi-2 dphy nodes to rk3588
2025-06-17 8:54 [PATCH 0/5] phy: rockchip: phy-rockchip-inno-csidphy: add support for rk3588 variant Michael Riesch via B4 Relay
` (3 preceding siblings ...)
2025-06-17 8:54 ` [PATCH 4/5] phy: rockchip: phy-rockchip-inno-csidphy: add support for rk3588 variant Michael Riesch via B4 Relay
@ 2025-06-17 8:54 ` Michael Riesch via B4 Relay
4 siblings, 0 replies; 15+ messages in thread
From: Michael Riesch via B4 Relay @ 2025-06-17 8:54 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner,
Vinod Koul, Kishon Vijay Abraham I, Philipp Zabel, Jagan Teki,
Sebastian Reichel, Collabora Kernel Team
Cc: devicetree, linux-arm-kernel, linux-rockchip, linux-kernel,
linux-phy, Michael Riesch
From: Michael Riesch <michael.riesch@collabora.com>
The Rockchip RK3588 features two MIPI CSI-2 DPHYs. Add the device
tree nodes for them.
Signed-off-by: Michael Riesch <michael.riesch@collabora.com>
---
arch/arm64/boot/dts/rockchip/rk3588-base.dtsi | 31 +++++++++++++++++++++++++++
1 file changed, 31 insertions(+)
diff --git a/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi b/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
index 70f03e68ba55..6a4ee0247fcf 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
@@ -621,6 +621,16 @@ php_grf: syscon@fd5b0000 {
reg = <0x0 0xfd5b0000 0x0 0x1000>;
};
+ csidphy0_grf: syscon@fd5b4000 {
+ compatible = "rockchip,rk3588-csidphy-grf", "syscon";
+ reg = <0x0 0xfd5b4000 0x0 0x1000>;
+ };
+
+ csidphy1_grf: syscon@fd5b5000 {
+ compatible = "rockchip,rk3588-csidphy-grf", "syscon";
+ reg = <0x0 0xfd5b5000 0x0 0x1000>;
+ };
+
pipe_phy0_grf: syscon@fd5bc000 {
compatible = "rockchip,rk3588-pipe-phy-grf", "syscon";
reg = <0x0 0xfd5bc000 0x0 0x100>;
@@ -3052,6 +3062,27 @@ mipidcphy1: phy@fedb0000 {
<&cru SRST_S_MIPI_DCPHY1>;
reset-names = "m_phy", "apb", "grf", "s_phy";
#phy-cells = <1>;
+ };
+
+ csi_dphy0: phy@fedc0000 {
+ compatible = "rockchip,rk3588-csi-dphy";
+ reg = <0x0 0xfedc0000 0x0 0x8000>;
+ clocks = <&cru PCLK_CSIPHY0>;
+ clock-names = "pclk";
+ #phy-cells = <0>;
+ resets = <&cru SRST_CSIPHY0>, <&cru SRST_P_CSIPHY0>;
+ rockchip,grf = <&csidphy0_grf>;
+ status = "disabled";
+ };
+
+ csi_dphy1: phy@fedc8000 {
+ compatible = "rockchip,rk3588-csi-dphy";
+ reg = <0x0 0xfedc8000 0x0 0x8000>;
+ clocks = <&cru PCLK_CSIPHY1>;
+ clock-names = "pclk";
+ #phy-cells = <0>;
+ resets = <&cru SRST_CSIPHY1>, <&cru SRST_P_CSIPHY1>;
+ rockchip,grf = <&csidphy1_grf>;
status = "disabled";
};
--
2.39.5
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 3/5] phy: rockchip: phy-rockchip-inno-csidphy: allow writes to grf register 0
2025-06-17 8:54 ` [PATCH 3/5] phy: rockchip: phy-rockchip-inno-csidphy: allow writes to grf register 0 Michael Riesch via B4 Relay
@ 2025-06-17 9:30 ` neil.armstrong
0 siblings, 0 replies; 15+ messages in thread
From: neil.armstrong @ 2025-06-17 9:30 UTC (permalink / raw)
To: michael.riesch, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Heiko Stuebner, Vinod Koul, Kishon Vijay Abraham I, Philipp Zabel,
Jagan Teki, Sebastian Reichel, Collabora Kernel Team
Cc: devicetree, linux-arm-kernel, linux-rockchip, linux-kernel,
linux-phy
On 17/06/2025 10:54, Michael Riesch via B4 Relay wrote:
> From: Michael Riesch <michael.riesch@collabora.com>
>
> The driver for the Rockchip MIPI CSI-2 DPHY uses GRF register offset
> value 0 to sort out undefined registers. However, the RK3588 CSIDPHY GRF
> this offset is perfectly fine (in fact, register 0 is the only one in
> this register y
> file).
> Introduce a boolean variable to indicate valid registers and allow writes
> to register 0.
>
> Signed-off-by: Michael Riesch <michael.riesch@collabora.com>
> ---
> drivers/phy/rockchip/phy-rockchip-inno-csidphy.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/phy/rockchip/phy-rockchip-inno-csidphy.c b/drivers/phy/rockchip/phy-rockchip-inno-csidphy.c
> index 2ab99e1d47eb..75533d071025 100644
> --- a/drivers/phy/rockchip/phy-rockchip-inno-csidphy.c
> +++ b/drivers/phy/rockchip/phy-rockchip-inno-csidphy.c
> @@ -87,10 +87,11 @@ struct dphy_reg {
> u32 offset;
> u32 mask;
> u32 shift;
> + u8 valid;
> };
>
> #define PHY_REG(_offset, _width, _shift) \
> - { .offset = _offset, .mask = BIT(_width) - 1, .shift = _shift, }
> + { .offset = _offset, .mask = BIT(_width) - 1, .shift = _shift, .valid = 1, }
>
> static const struct dphy_reg rk1808_grf_dphy_regs[] = {
> [GRF_DPHY_CSIPHY_FORCERXMODE] = PHY_REG(RK1808_GRF_PD_VI_CON_OFFSET, 4, 0),
> @@ -145,7 +146,7 @@ static inline void write_grf_reg(struct rockchip_inno_csidphy *priv,
> const struct dphy_drv_data *drv_data = priv->drv_data;
> const struct dphy_reg *reg = &drv_data->grf_regs[index];
>
> - if (reg->offset)
> + if (reg->valid)
> regmap_write(priv->grf, reg->offset,
> HIWORD_UPDATE(value, reg->mask, reg->shift));
> }
>
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/5] dt-bindings: phy: rockchip-inno-csi-dphy: add rk3588 variant
2025-06-17 8:54 ` [PATCH 2/5] dt-bindings: phy: rockchip-inno-csi-dphy: add rk3588 variant Michael Riesch via B4 Relay
@ 2025-06-17 9:31 ` neil.armstrong
2025-06-18 6:32 ` Michael Riesch
2025-06-17 14:12 ` Diederik de Haas
1 sibling, 1 reply; 15+ messages in thread
From: neil.armstrong @ 2025-06-17 9:31 UTC (permalink / raw)
To: michael.riesch, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Heiko Stuebner, Vinod Koul, Kishon Vijay Abraham I, Philipp Zabel,
Jagan Teki, Sebastian Reichel, Collabora Kernel Team
Cc: devicetree, linux-arm-kernel, linux-rockchip, linux-kernel,
linux-phy
On 17/06/2025 10:54, Michael Riesch via B4 Relay wrote:
> From: Michael Riesch <michael.riesch@collabora.com>
>
> The Rockchip RK3588 variant of the CSI-2 DPHY features two reset lines.
> Add the variant and allow for the additional reset.
No names for the new resets on the RK3588 ?
Neil
>
> Signed-off-by: Michael Riesch <michael.riesch@collabora.com>
> ---
> .../bindings/phy/rockchip-inno-csi-dphy.yaml | 60 ++++++++++++++++++++--
> 1 file changed, 55 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml b/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml
> index 5ac994b3c0aa..6755738b13ee 100644
> --- a/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml
> +++ b/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml
> @@ -21,6 +21,7 @@ properties:
> - rockchip,rk3326-csi-dphy
> - rockchip,rk3368-csi-dphy
> - rockchip,rk3568-csi-dphy
> + - rockchip,rk3588-csi-dphy
>
> reg:
> maxItems: 1
> @@ -39,18 +40,49 @@ properties:
> maxItems: 1
>
> resets:
> - items:
> - - description: exclusive PHY reset line
> + minItems: 1
> + maxItems: 2
>
> reset-names:
> - items:
> - - const: apb
> + minItems: 1
> + maxItems: 2
>
> rockchip,grf:
> $ref: /schemas/types.yaml#/definitions/phandle
> description:
> Some additional phy settings are access through GRF regs.
>
> +allOf:
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - rockchip,px30-csi-dphy
> + - rockchip,rk1808-csi-dphy
> + - rockchip,rk3326-csi-dphy
> + - rockchip,rk3368-csi-dphy
> + - rockchip,rk3568-csi-dphy
> + then:
> + properties:
> + resets:
> + items:
> + - description: exclusive PHY reset line
> +
> + reset-names:
> + items:
> + - const: apb
> +
> + required:
> + - reset-names
> + else:
> + properties:
> + resets:
> + minItems: 2
> +
> + reset-names:
> + minItems: 2
> +
> required:
> - compatible
> - reg
> @@ -59,7 +91,6 @@ required:
> - '#phy-cells'
> - power-domains
> - resets
> - - reset-names
> - rockchip,grf
>
> additionalProperties: false
> @@ -78,3 +109,22 @@ examples:
> reset-names = "apb";
> rockchip,grf = <&grf>;
> };
> + - |
> + #include <dt-bindings/clock/rockchip,rk3588-cru.h>
> + #include <dt-bindings/reset/rockchip,rk3588-cru.h>
> +
> + soc {
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + csi_dphy0: phy@fedc0000 {
> + compatible = "rockchip,rk3588-csi-dphy";
> + reg = <0x0 0xfedc0000 0x0 0x8000>;
> + clocks = <&cru PCLK_CSIPHY0>;
> + clock-names = "pclk";
> + #phy-cells = <0>;
> + resets = <&cru SRST_CSIPHY0>, <&cru SRST_P_CSIPHY0>;
> + rockchip,grf = <&csidphy0_grf>;
> + status = "disabled";
> + };
> + };
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 4/5] phy: rockchip: phy-rockchip-inno-csidphy: add support for rk3588 variant
2025-06-17 8:54 ` [PATCH 4/5] phy: rockchip: phy-rockchip-inno-csidphy: add support for rk3588 variant Michael Riesch via B4 Relay
@ 2025-06-17 9:36 ` neil.armstrong
0 siblings, 0 replies; 15+ messages in thread
From: neil.armstrong @ 2025-06-17 9:36 UTC (permalink / raw)
To: michael.riesch, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Heiko Stuebner, Vinod Koul, Kishon Vijay Abraham I, Philipp Zabel,
Jagan Teki, Sebastian Reichel, Collabora Kernel Team
Cc: devicetree, linux-arm-kernel, linux-rockchip, linux-kernel,
linux-phy
On 17/06/2025 10:54, Michael Riesch via B4 Relay wrote:
> From: Michael Riesch <michael.riesch@collabora.com>
>
> The Rockchip RK3588 MIPI CSI-2 DPHY can be supported using the existing
> phy-rockchip-inno-csidphy driver, the notable differences being
> - the control bits in the GRF
> - the additional reset line
> Add support for this variant.
>
> Signed-off-by: Michael Riesch <michael.riesch@collabora.com>
> ---
> drivers/phy/rockchip/phy-rockchip-inno-csidphy.c | 23 ++++++++++++++++++++++-
> 1 file changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/phy/rockchip/phy-rockchip-inno-csidphy.c b/drivers/phy/rockchip/phy-rockchip-inno-csidphy.c
> index 75533d071025..0840be668bfd 100644
> --- a/drivers/phy/rockchip/phy-rockchip-inno-csidphy.c
> +++ b/drivers/phy/rockchip/phy-rockchip-inno-csidphy.c
> @@ -30,6 +30,8 @@
> #define RK3568_GRF_VI_CON0 0x0340
> #define RK3568_GRF_VI_CON1 0x0344
>
> +#define RK3588_CSIDPHY_GRF_CON0 0x0000
> +
> /* PHY */
> #define CSIDPHY_CTRL_LANE_ENABLE 0x00
> #define CSIDPHY_CTRL_LANE_ENABLE_CK BIT(6)
> @@ -115,6 +117,12 @@ static const struct dphy_reg rk3568_grf_dphy_regs[] = {
> [GRF_DPHY_CSIPHY_CLKLANE_EN] = PHY_REG(RK3568_GRF_VI_CON0, 1, 8),
> };
>
> +static const struct dphy_reg rk3588_grf_dphy_regs[] = {
> + [GRF_DPHY_CSIPHY_FORCERXMODE] = PHY_REG(RK3588_CSIDPHY_GRF_CON0, 4, 0),
> + [GRF_DPHY_CSIPHY_DATALANE_EN] = PHY_REG(RK3588_CSIDPHY_GRF_CON0, 4, 4),
> + [GRF_DPHY_CSIPHY_CLKLANE_EN] = PHY_REG(RK3588_CSIDPHY_GRF_CON0, 1, 8),
> +};
> +
> struct hsfreq_range {
> u32 range_h;
> u8 cfg_bit;
> @@ -373,6 +381,15 @@ static const struct dphy_drv_data rk3568_mipidphy_drv_data = {
> .grf_regs = rk3568_grf_dphy_regs,
> };
>
> +static const struct dphy_drv_data rk3588_mipidphy_drv_data = {
> + .pwrctl_offset = -1,
> + .ths_settle_offset = RK3568_CSIDPHY_CLK_WR_THS_SETTLE,
> + .calib_offset = RK3568_CSIDPHY_CLK_CALIB_EN,
> + .hsfreq_ranges = rk1808_mipidphy_hsfreq_ranges,
> + .num_hsfreq_ranges = ARRAY_SIZE(rk1808_mipidphy_hsfreq_ranges),
> + .grf_regs = rk3588_grf_dphy_regs,
> +};
> +
> static const struct of_device_id rockchip_inno_csidphy_match_id[] = {
> {
> .compatible = "rockchip,px30-csi-dphy",
> @@ -394,6 +411,10 @@ static const struct of_device_id rockchip_inno_csidphy_match_id[] = {
> .compatible = "rockchip,rk3568-csi-dphy",
> .data = &rk3568_mipidphy_drv_data,
> },
> + {
> + .compatible = "rockchip,rk3588-csi-dphy",
> + .data = &rk3588_mipidphy_drv_data,
> + },
> {}
> };
> MODULE_DEVICE_TABLE(of, rockchip_inno_csidphy_match_id);
> @@ -435,7 +456,7 @@ static int rockchip_inno_csidphy_probe(struct platform_device *pdev)
> return PTR_ERR(priv->pclk);
> }
>
> - priv->rst = devm_reset_control_get(dev, "apb");
> + priv->rst = devm_reset_control_array_get(dev, RESET_CONTROL_EXCLUSIVE);
It would be preferable to have the names of the resets lines and use devm_reset_control_bulk_get_exclusive(),
and probably add the reset names to dphy_drv_data
Neil
> if (IS_ERR(priv->rst)) {
> dev_err(dev, "failed to get system reset control\n");
> return PTR_ERR(priv->rst);
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/5] dt-bindings: phy: rockchip-inno-csi-dphy: add rk3588 variant
2025-06-17 8:54 ` [PATCH 2/5] dt-bindings: phy: rockchip-inno-csi-dphy: add rk3588 variant Michael Riesch via B4 Relay
2025-06-17 9:31 ` neil.armstrong
@ 2025-06-17 14:12 ` Diederik de Haas
2025-06-18 7:45 ` Michael Riesch
1 sibling, 1 reply; 15+ messages in thread
From: Diederik de Haas @ 2025-06-17 14:12 UTC (permalink / raw)
To: michael.riesch, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Heiko Stuebner, Vinod Koul, Kishon Vijay Abraham I, Philipp Zabel,
Jagan Teki, Sebastian Reichel, Collabora Kernel Team
Cc: devicetree, linux-arm-kernel, linux-rockchip, linux-kernel,
linux-phy
[-- Attachment #1: Type: text/plain, Size: 3961 bytes --]
Hi,
I'm (unfortunately) not seeing any @rock-chips.com recipients ...
On Tue Jun 17, 2025 at 10:54 AM CEST, Michael Riesch via B4 Relay wrote:
> From: Michael Riesch <michael.riesch@collabora.com>
>
> The Rockchip RK3588 variant of the CSI-2 DPHY features two reset lines.
> Add the variant and allow for the additional reset.
>
> Signed-off-by: Michael Riesch <michael.riesch@collabora.com>
> ---
> .../bindings/phy/rockchip-inno-csi-dphy.yaml | 60 ++++++++++++++++++++--
> 1 file changed, 55 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml b/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml
> index 5ac994b3c0aa..6755738b13ee 100644
> --- a/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml
> +++ b/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml
> @@ -21,6 +21,7 @@ properties:
> - rockchip,rk3326-csi-dphy
> - rockchip,rk3368-csi-dphy
> - rockchip,rk3568-csi-dphy
> + - rockchip,rk3588-csi-dphy
>
> reg:
> maxItems: 1
> @@ -39,18 +40,49 @@ properties:
> maxItems: 1
>
> resets:
> - items:
> - - description: exclusive PHY reset line
> + minItems: 1
> + maxItems: 2
>
> reset-names:
> - items:
> - - const: apb
> + minItems: 1
> + maxItems: 2
>
> rockchip,grf:
> $ref: /schemas/types.yaml#/definitions/phandle
> description:
> Some additional phy settings are access through GRF regs.
>
> +allOf:
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - rockchip,px30-csi-dphy
> + - rockchip,rk1808-csi-dphy
> + - rockchip,rk3326-csi-dphy
> + - rockchip,rk3368-csi-dphy
> + - rockchip,rk3568-csi-dphy
> + then:
> + properties:
> + resets:
> + items:
> + - description: exclusive PHY reset line
> +
> + reset-names:
> + items:
> + - const: apb
> +
> + required:
> + - reset-names
> + else:
> + properties:
> + resets:
> + minItems: 2
> +
> + reset-names:
> + minItems: 2
> +
> required:
> - compatible
> - reg
> @@ -59,7 +91,6 @@ required:
> - '#phy-cells'
> - power-domains
> - resets
> - - reset-names
> - rockchip,grf
>
> additionalProperties: false
> @@ -78,3 +109,22 @@ examples:
> reset-names = "apb";
> rockchip,grf = <&grf>;
> };
> + - |
> + #include <dt-bindings/clock/rockchip,rk3588-cru.h>
> + #include <dt-bindings/reset/rockchip,rk3588-cru.h>
> +
> + soc {
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + csi_dphy0: phy@fedc0000 {
> + compatible = "rockchip,rk3588-csi-dphy";
> + reg = <0x0 0xfedc0000 0x0 0x8000>;
> + clocks = <&cru PCLK_CSIPHY0>;
> + clock-names = "pclk";
> + #phy-cells = <0>;
> + resets = <&cru SRST_CSIPHY0>, <&cru SRST_P_CSIPHY0>;
> + rockchip,grf = <&csidphy0_grf>;
> + status = "disabled";
> + };
> + };
... which could hopefully tell us what the value is/should be for the
*required* 'power-domains' property, which is missing in this example.
IOW: the binding example is invalid according to its own binding.
(btw: you can drop the 'csi_dphy0' label)
And hopefully also for rk3568 so we can add it to rk356x-base.dtsi and
you can add it in patch 5 where it's also missing.
Grepping for "csi-dphy" in arch/arm*/boot/dts/rockchip returns:
- px30.dtsi
- rk356x-base.dtsi
With this patch set applied, we'd have a 3rd result: rk3588-base.dtsi
For all the listed compatibles, it's only actually defined in px30.dtsi.
Cheers (and sorry),
Diederik
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/5] dt-bindings: phy: rockchip-inno-csi-dphy: add rk3588 variant
2025-06-17 9:31 ` neil.armstrong
@ 2025-06-18 6:32 ` Michael Riesch
2025-06-19 20:11 ` Heiko Stuebner
0 siblings, 1 reply; 15+ messages in thread
From: Michael Riesch @ 2025-06-18 6:32 UTC (permalink / raw)
To: Neil Armstrong, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Heiko Stuebner, Vinod Koul, Kishon Vijay Abraham I, Philipp Zabel,
Jagan Teki, Sebastian Reichel, Collabora Kernel Team
Cc: devicetree, linux-arm-kernel, linux-rockchip, linux-kernel,
linux-phy
Hi Neil,
Thanks for your comments!
On 6/17/25 11:31, neil.armstrong@linaro.org wrote:
> On 17/06/2025 10:54, Michael Riesch via B4 Relay wrote:
>> From: Michael Riesch <michael.riesch@collabora.com>
>>
>> The Rockchip RK3588 variant of the CSI-2 DPHY features two reset lines.
>> Add the variant and allow for the additional reset.
>
> No names for the new resets on the RK3588 ?
I left the names away because TBH I don't see the value in them (in that
case).
Downstream uses reset-names = "srst_csiphy0", "srst_p_csiphy0"; and
there is no better description. One could guess that the second reset
corresponds to "apb" but this is just guessing and we would still have
to guess/find a proper name for the first reset.
Amazingly the mainline driver does not seem to do anything with the
resets (unless I overlooked some implicit magic). Downstream does a
simple reset_control_{assert,deassert} before configuring the PHY. Now
if the different resets are handled in bulk mode, does it really make
sense to address each reset individually?
Best regards,
Michael
>
> Neil
>
>>
>> Signed-off-by: Michael Riesch <michael.riesch@collabora.com>
>> ---
>> .../bindings/phy/rockchip-inno-csi-dphy.yaml | 60 ++++++++++++
>> ++++++++--
>> 1 file changed, 55 insertions(+), 5 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/phy/rockchip-inno-csi-
>> dphy.yaml b/Documentation/devicetree/bindings/phy/rockchip-inno-csi-
>> dphy.yaml
>> index 5ac994b3c0aa..6755738b13ee 100644
>> --- a/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml
>> +++ b/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml
>> @@ -21,6 +21,7 @@ properties:
>> - rockchip,rk3326-csi-dphy
>> - rockchip,rk3368-csi-dphy
>> - rockchip,rk3568-csi-dphy
>> + - rockchip,rk3588-csi-dphy
>> reg:
>> maxItems: 1
>> @@ -39,18 +40,49 @@ properties:
>> maxItems: 1
>> resets:
>> - items:
>> - - description: exclusive PHY reset line
>> + minItems: 1
>> + maxItems: 2
>> reset-names:
>> - items:
>> - - const: apb
>> + minItems: 1
>> + maxItems: 2
>> rockchip,grf:
>> $ref: /schemas/types.yaml#/definitions/phandle
>> description:
>> Some additional phy settings are access through GRF regs.
>> +allOf:
>> + - if:
>> + properties:
>> + compatible:
>> + contains:
>> + enum:
>> + - rockchip,px30-csi-dphy
>> + - rockchip,rk1808-csi-dphy
>> + - rockchip,rk3326-csi-dphy
>> + - rockchip,rk3368-csi-dphy
>> + - rockchip,rk3568-csi-dphy
>> + then:
>> + properties:
>> + resets:
>> + items:
>> + - description: exclusive PHY reset line
>> +
>> + reset-names:
>> + items:
>> + - const: apb
>> +
>> + required:
>> + - reset-names
>> + else:
>> + properties:
>> + resets:
>> + minItems: 2
>> +
>> + reset-names:
>> + minItems: 2
>> +
>> required:
>> - compatible
>> - reg
>> @@ -59,7 +91,6 @@ required:
>> - '#phy-cells'
>> - power-domains
>> - resets
>> - - reset-names
>> - rockchip,grf
>> additionalProperties: false
>> @@ -78,3 +109,22 @@ examples:
>> reset-names = "apb";
>> rockchip,grf = <&grf>;
>> };
>> + - |
>> + #include <dt-bindings/clock/rockchip,rk3588-cru.h>
>> + #include <dt-bindings/reset/rockchip,rk3588-cru.h>
>> +
>> + soc {
>> + #address-cells = <2>;
>> + #size-cells = <2>;
>> +
>> + csi_dphy0: phy@fedc0000 {
>> + compatible = "rockchip,rk3588-csi-dphy";
>> + reg = <0x0 0xfedc0000 0x0 0x8000>;
>> + clocks = <&cru PCLK_CSIPHY0>;
>> + clock-names = "pclk";
>> + #phy-cells = <0>;
>> + resets = <&cru SRST_CSIPHY0>, <&cru SRST_P_CSIPHY0>;
>> + rockchip,grf = <&csidphy0_grf>;
>> + status = "disabled";
>> + };
>> + };
>>
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/5] dt-bindings: phy: rockchip-inno-csi-dphy: add rk3588 variant
2025-06-17 14:12 ` Diederik de Haas
@ 2025-06-18 7:45 ` Michael Riesch
2025-06-27 19:17 ` Rob Herring
0 siblings, 1 reply; 15+ messages in thread
From: Michael Riesch @ 2025-06-18 7:45 UTC (permalink / raw)
To: Diederik de Haas, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Heiko Stuebner, Vinod Koul, Kishon Vijay Abraham I, Philipp Zabel,
Jagan Teki, Sebastian Reichel, Collabora Kernel Team, Kever Yang
Cc: devicetree, linux-arm-kernel, linux-rockchip, linux-kernel,
linux-phy
Hi Diederik,
Thanks for your comments!
On 6/17/25 16:12, Diederik de Haas wrote:
> Hi,
>
> I'm (unfortunately) not seeing any @rock-chips.com recipients ...
Oops, I meant to include at least Kever, but forgot to do it. Will do in v2.
Cc: Kever
>
> On Tue Jun 17, 2025 at 10:54 AM CEST, Michael Riesch via B4 Relay wrote:
>> From: Michael Riesch <michael.riesch@collabora.com>
>>
>> The Rockchip RK3588 variant of the CSI-2 DPHY features two reset lines.
>> Add the variant and allow for the additional reset.
>>
>> Signed-off-by: Michael Riesch <michael.riesch@collabora.com>
>> ---
>> .../bindings/phy/rockchip-inno-csi-dphy.yaml | 60 ++++++++++++++++++++--
>> 1 file changed, 55 insertions(+), 5 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml b/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml
>> index 5ac994b3c0aa..6755738b13ee 100644
>> --- a/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml
>> +++ b/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml
>> @@ -21,6 +21,7 @@ properties:
>> - rockchip,rk3326-csi-dphy
>> - rockchip,rk3368-csi-dphy
>> - rockchip,rk3568-csi-dphy
>> + - rockchip,rk3588-csi-dphy
>>
>> reg:
>> maxItems: 1
>> @@ -39,18 +40,49 @@ properties:
>> maxItems: 1
>>
>> resets:
>> - items:
>> - - description: exclusive PHY reset line
>> + minItems: 1
>> + maxItems: 2
>>
>> reset-names:
>> - items:
>> - - const: apb
>> + minItems: 1
>> + maxItems: 2
>>
>> rockchip,grf:
>> $ref: /schemas/types.yaml#/definitions/phandle
>> description:
>> Some additional phy settings are access through GRF regs.
>>
>> +allOf:
>> + - if:
>> + properties:
>> + compatible:
>> + contains:
>> + enum:
>> + - rockchip,px30-csi-dphy
>> + - rockchip,rk1808-csi-dphy
>> + - rockchip,rk3326-csi-dphy
>> + - rockchip,rk3368-csi-dphy
>> + - rockchip,rk3568-csi-dphy
>> + then:
>> + properties:
>> + resets:
>> + items:
>> + - description: exclusive PHY reset line
>> +
>> + reset-names:
>> + items:
>> + - const: apb
>> +
>> + required:
>> + - reset-names
>> + else:
>> + properties:
>> + resets:
>> + minItems: 2
>> +
>> + reset-names:
>> + minItems: 2
>> +
>> required:
>> - compatible
>> - reg
>> @@ -59,7 +91,6 @@ required:
>> - '#phy-cells'
>> - power-domains
>> - resets
>> - - reset-names
>> - rockchip,grf
>>
>> additionalProperties: false
>> @@ -78,3 +109,22 @@ examples:
>> reset-names = "apb";
>> rockchip,grf = <&grf>;
>> };
>> + - |
>> + #include <dt-bindings/clock/rockchip,rk3588-cru.h>
>> + #include <dt-bindings/reset/rockchip,rk3588-cru.h>
>> +
>> + soc {
>> + #address-cells = <2>;
>> + #size-cells = <2>;
>> +
>> + csi_dphy0: phy@fedc0000 {
>> + compatible = "rockchip,rk3588-csi-dphy";
>> + reg = <0x0 0xfedc0000 0x0 0x8000>;
>> + clocks = <&cru PCLK_CSIPHY0>;
>> + clock-names = "pclk";
>> + #phy-cells = <0>;
>> + resets = <&cru SRST_CSIPHY0>, <&cru SRST_P_CSIPHY0>;
>> + rockchip,grf = <&csidphy0_grf>;
>> + status = "disabled";
>> + };
>> + };
>
> ... which could hopefully tell us what the value is/should be for the
> *required* 'power-domains' property, which is missing in this example.
> IOW: the binding example is invalid according to its own binding.
Huh, indeed. Hm. Why didn't make dt_binding_check warn me about that?!
TRM Part 1, p. 1097 states that HDMI_CSI_DPHY is in the ALIVE(PD_BUS)
power domain. With some creativity one can interpret that the CSI DPHY
is always on anyways. @Kever: Could you please elaborate on that?
> (btw: you can drop the 'csi_dphy0' label)
Will do.
>
> And hopefully also for rk3568 so we can add it to rk356x-base.dtsi and
> you can add it in patch 5 where it's also missing.
I recall a similar discussion [0]. In the RK3568 the CSIPHY is in the
ALIVE power domain.
Note that the PHY must not be confused with the HOST blocks, which are
the MIPI CSI-2 receivers.
I guess the correct solution is to make power-domains optional. Further
input welcome, though.
Best regards,
Michael
>
> Grepping for "csi-dphy" in arch/arm*/boot/dts/rockchip returns:
> - px30.dtsi
> - rk356x-base.dtsi
>
> With this patch set applied, we'd have a 3rd result: rk3588-base.dtsi
>
> For all the listed compatibles, it's only actually defined in px30.dtsi.
>
> Cheers (and sorry),
> Diederik
[0] https://lore.kernel.org/all/D4QNJ85V43NU.YD01E8AB4116@cknow.org/
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/5] dt-bindings: phy: rockchip-inno-csi-dphy: add rk3588 variant
2025-06-18 6:32 ` Michael Riesch
@ 2025-06-19 20:11 ` Heiko Stuebner
0 siblings, 0 replies; 15+ messages in thread
From: Heiko Stuebner @ 2025-06-19 20:11 UTC (permalink / raw)
To: Neil Armstrong, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Vinod Koul, Kishon Vijay Abraham I, Philipp Zabel, Jagan Teki,
Sebastian Reichel, Collabora Kernel Team, Michael Riesch
Cc: devicetree, linux-arm-kernel, linux-rockchip, linux-kernel,
linux-phy
Am Mittwoch, 18. Juni 2025, 08:32:25 Mitteleuropäische Sommerzeit schrieb Michael Riesch:
> Hi Neil,
>
> Thanks for your comments!
>
> On 6/17/25 11:31, neil.armstrong@linaro.org wrote:
> > On 17/06/2025 10:54, Michael Riesch via B4 Relay wrote:
> >> From: Michael Riesch <michael.riesch@collabora.com>
> >>
> >> The Rockchip RK3588 variant of the CSI-2 DPHY features two reset lines.
> >> Add the variant and allow for the additional reset.
> >
> > No names for the new resets on the RK3588 ?
>
> I left the names away because TBH I don't see the value in them (in that
> case).
>
> Downstream uses reset-names = "srst_csiphy0", "srst_p_csiphy0"; and
> there is no better description. One could guess that the second reset
> corresponds to "apb" but this is just guessing and we would still have
> to guess/find a proper name for the first reset.
>
> Amazingly the mainline driver does not seem to do anything with the
> resets (unless I overlooked some implicit magic). Downstream does a
> simple reset_control_{assert,deassert} before configuring the PHY. Now
> if the different resets are handled in bulk mode, does it really make
> sense to address each reset individually?
it might not make sense now, but possibly in the future?
A binding and the attached devicetrees are meant to be "forever", i.e. a
new kernel _should_ support all those old devicetrees you throw at it -
if they conform to (at some time) established bindings.
So while all drivers might not need the specific resets now, you don't
know what quirks you'll have discovered in two years ;-)
And instead of trying to update the binding and then carrying both the
new and the fallback code for the old binding around, why not do it now.
Then when you find a need for a specific reset, things magically just work.
Heiko
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/5] dt-bindings: soc: rockchip: add rk3588 csidphy grf syscon
2025-06-17 8:54 ` [PATCH 1/5] dt-bindings: soc: rockchip: add rk3588 csidphy grf syscon Michael Riesch via B4 Relay
@ 2025-06-27 19:14 ` Rob Herring (Arm)
0 siblings, 0 replies; 15+ messages in thread
From: Rob Herring (Arm) @ 2025-06-27 19:14 UTC (permalink / raw)
To: Michael Riesch
Cc: devicetree, Conor Dooley, Sebastian Reichel, linux-arm-kernel,
linux-rockchip, Jagan Teki, Krzysztof Kozlowski, linux-kernel,
linux-phy, Vinod Koul, Collabora Kernel Team, Philipp Zabel,
Kishon Vijay Abraham I, Heiko Stuebner
On Tue, 17 Jun 2025 10:54:42 +0200, Michael Riesch wrote:
> Add CSIDPHY GRF syscon compatible for the Rockchip RK3588.
>
> Signed-off-by: Michael Riesch <michael.riesch@collabora.com>
> ---
> Documentation/devicetree/bindings/soc/rockchip/grf.yaml | 1 +
> 1 file changed, 1 insertion(+)
>
Acked-by: Rob Herring (Arm) <robh@kernel.org>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/5] dt-bindings: phy: rockchip-inno-csi-dphy: add rk3588 variant
2025-06-18 7:45 ` Michael Riesch
@ 2025-06-27 19:17 ` Rob Herring
0 siblings, 0 replies; 15+ messages in thread
From: Rob Herring @ 2025-06-27 19:17 UTC (permalink / raw)
To: Michael Riesch
Cc: Diederik de Haas, Krzysztof Kozlowski, Conor Dooley,
Heiko Stuebner, Vinod Koul, Kishon Vijay Abraham I, Philipp Zabel,
Jagan Teki, Sebastian Reichel, Collabora Kernel Team, Kever Yang,
devicetree, linux-arm-kernel, linux-rockchip, linux-kernel,
linux-phy
On Wed, Jun 18, 2025 at 09:45:32AM +0200, Michael Riesch wrote:
> Hi Diederik,
>
> Thanks for your comments!
>
> On 6/17/25 16:12, Diederik de Haas wrote:
> > Hi,
> >
> > I'm (unfortunately) not seeing any @rock-chips.com recipients ...
>
> Oops, I meant to include at least Kever, but forgot to do it. Will do in v2.
>
> Cc: Kever
>
> >
> > On Tue Jun 17, 2025 at 10:54 AM CEST, Michael Riesch via B4 Relay wrote:
> >> From: Michael Riesch <michael.riesch@collabora.com>
> >>
> >> The Rockchip RK3588 variant of the CSI-2 DPHY features two reset lines.
> >> Add the variant and allow for the additional reset.
> >>
> >> Signed-off-by: Michael Riesch <michael.riesch@collabora.com>
> >> ---
> >> .../bindings/phy/rockchip-inno-csi-dphy.yaml | 60 ++++++++++++++++++++--
> >> 1 file changed, 55 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml b/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml
> >> index 5ac994b3c0aa..6755738b13ee 100644
> >> --- a/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml
> >> +++ b/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml
> >> @@ -21,6 +21,7 @@ properties:
> >> - rockchip,rk3326-csi-dphy
> >> - rockchip,rk3368-csi-dphy
> >> - rockchip,rk3568-csi-dphy
> >> + - rockchip,rk3588-csi-dphy
> >>
> >> reg:
> >> maxItems: 1
> >> @@ -39,18 +40,49 @@ properties:
> >> maxItems: 1
> >>
> >> resets:
> >> - items:
> >> - - description: exclusive PHY reset line
> >> + minItems: 1
> >> + maxItems: 2
> >>
> >> reset-names:
> >> - items:
> >> - - const: apb
> >> + minItems: 1
> >> + maxItems: 2
> >>
> >> rockchip,grf:
> >> $ref: /schemas/types.yaml#/definitions/phandle
> >> description:
> >> Some additional phy settings are access through GRF regs.
> >>
> >> +allOf:
> >> + - if:
> >> + properties:
> >> + compatible:
> >> + contains:
> >> + enum:
> >> + - rockchip,px30-csi-dphy
> >> + - rockchip,rk1808-csi-dphy
> >> + - rockchip,rk3326-csi-dphy
> >> + - rockchip,rk3368-csi-dphy
> >> + - rockchip,rk3568-csi-dphy
> >> + then:
> >> + properties:
> >> + resets:
> >> + items:
> >> + - description: exclusive PHY reset line
> >> +
> >> + reset-names:
> >> + items:
> >> + - const: apb
> >> +
> >> + required:
> >> + - reset-names
> >> + else:
> >> + properties:
> >> + resets:
> >> + minItems: 2
> >> +
> >> + reset-names:
> >> + minItems: 2
You have to define the names. Ideally, at the top level and then keep
this part like this.
> >> +
> >> required:
> >> - compatible
> >> - reg
> >> @@ -59,7 +91,6 @@ required:
> >> - '#phy-cells'
> >> - power-domains
> >> - resets
> >> - - reset-names
> >> - rockchip,grf
> >>
> >> additionalProperties: false
> >> @@ -78,3 +109,22 @@ examples:
> >> reset-names = "apb";
> >> rockchip,grf = <&grf>;
> >> };
> >> + - |
> >> + #include <dt-bindings/clock/rockchip,rk3588-cru.h>
> >> + #include <dt-bindings/reset/rockchip,rk3588-cru.h>
> >> +
> >> + soc {
> >> + #address-cells = <2>;
> >> + #size-cells = <2>;
> >> +
> >> + csi_dphy0: phy@fedc0000 {
> >> + compatible = "rockchip,rk3588-csi-dphy";
> >> + reg = <0x0 0xfedc0000 0x0 0x8000>;
> >> + clocks = <&cru PCLK_CSIPHY0>;
> >> + clock-names = "pclk";
> >> + #phy-cells = <0>;
> >> + resets = <&cru SRST_CSIPHY0>, <&cru SRST_P_CSIPHY0>;
> >> + rockchip,grf = <&csidphy0_grf>;
> >> + status = "disabled";
> >> + };
> >> + };
> >
> > ... which could hopefully tell us what the value is/should be for the
> > *required* 'power-domains' property, which is missing in this example.
> > IOW: the binding example is invalid according to its own binding.
>
> Huh, indeed. Hm. Why didn't make dt_binding_check warn me about that?!
You disabled the node, what do you want us to check?
Rob
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2025-06-27 19:17 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-17 8:54 [PATCH 0/5] phy: rockchip: phy-rockchip-inno-csidphy: add support for rk3588 variant Michael Riesch via B4 Relay
2025-06-17 8:54 ` [PATCH 1/5] dt-bindings: soc: rockchip: add rk3588 csidphy grf syscon Michael Riesch via B4 Relay
2025-06-27 19:14 ` Rob Herring (Arm)
2025-06-17 8:54 ` [PATCH 2/5] dt-bindings: phy: rockchip-inno-csi-dphy: add rk3588 variant Michael Riesch via B4 Relay
2025-06-17 9:31 ` neil.armstrong
2025-06-18 6:32 ` Michael Riesch
2025-06-19 20:11 ` Heiko Stuebner
2025-06-17 14:12 ` Diederik de Haas
2025-06-18 7:45 ` Michael Riesch
2025-06-27 19:17 ` Rob Herring
2025-06-17 8:54 ` [PATCH 3/5] phy: rockchip: phy-rockchip-inno-csidphy: allow writes to grf register 0 Michael Riesch via B4 Relay
2025-06-17 9:30 ` neil.armstrong
2025-06-17 8:54 ` [PATCH 4/5] phy: rockchip: phy-rockchip-inno-csidphy: add support for rk3588 variant Michael Riesch via B4 Relay
2025-06-17 9:36 ` neil.armstrong
2025-06-17 8:54 ` [PATCH 5/5] arm64: dts: rockchip: add mipi csi-2 dphy nodes to rk3588 Michael Riesch via B4 Relay
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).