[PATCH v2 1/2] dt-bindings: phy: rockchip,inno-usb2phy: add rk3576

[PATCH v2 1/2] dt-bindings: phy: rockchip,inno-usb2phy: add rk3576

[Frank Wang]

From: Frank Wang <frank.wang@rock-chips.com>

Add compatible for the USB2 phy in the Rockchip RK3576 SoC.

Signed-off-by: Frank Wang <frank.wang@rock-chips.com>
---
Changelog:
v2:
 - Categorize clock names by oneOf keyword.

v1:
 - https://patchwork.kernel.org/project/linux-phy/patch/20240923025326.10467-1-frank.wang@rock-chips.com/

 .../bindings/phy/rockchip,inno-usb2phy.yaml      | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml b/Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml
index 5254413137c64..8af4e0f8637fc 100644
--- a/Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml
+++ b/Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml
@@ -20,6 +20,7 @@ properties:
       - rockchip,rk3366-usb2phy
       - rockchip,rk3399-usb2phy
       - rockchip,rk3568-usb2phy
+      - rockchip,rk3576-usb2phy
       - rockchip,rk3588-usb2phy
       - rockchip,rv1108-usb2phy
 
@@ -34,10 +35,20 @@ properties:
     const: 0
 
   clocks:
-    maxItems: 1
+    minItems: 1
+    maxItems: 3
 
   clock-names:
-    const: phyclk
+    minItems: 1
+    maxItems: 3
+    items:
+      oneOf:
+        - description: aclk for USB MMU.
+          const: aclk
+        - description: aclk_slv for USB MMU.
+          const: aclk_slv
+        - description: PHY input reference clocks.
+          const: phyclk
 
   assigned-clocks:
     description:
@@ -143,6 +154,7 @@ allOf:
           contains:
             enum:
               - rockchip,rk3568-usb2phy
+              - rockchip,rk3576-usb2phy
               - rockchip,rk3588-usb2phy
 
     then:
-- 
2.45.2


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

[PATCH v2 2/2] phy: rockchip: inno-usb2: Add usb2 phys support for rk3576

[Frank Wang]

From: William Wu <william.wu@rock-chips.com>

The RK3576 SoC has two independent USB2.0 PHYs, and
each PHY has one port.

Signed-off-by: William Wu <william.wu@rock-chips.com>
Signed-off-by: Frank Wang <frank.wang@rock-chips.com>
---
Changelog:
v2:
 - no changes.

v1:
 - https://patchwork.kernel.org/project/linux-phy/patch/20240923025326.10467-2-frank.wang@rock-chips.com/

 drivers/phy/rockchip/phy-rockchip-inno-usb2.c | 131 ++++++++++++++++--
 1 file changed, 123 insertions(+), 8 deletions(-)

diff --git a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
index 4f71373ae6e1a..51ca16cbb86cb 100644
--- a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
+++ b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
@@ -229,9 +229,10 @@ struct rockchip_usb2phy_port {
  * @dev: pointer to device.
  * @grf: General Register Files regmap.
  * @usbgrf: USB General Register Files regmap.
- * @clk: clock struct of phy input clk.
+ * @clks: array of phy input clocks.
  * @clk480m: clock struct of phy output clk.
  * @clk480m_hw: clock struct of phy output clk management.
+ * @num_clks: number of phy input clocks.
  * @phy_reset: phy reset control.
  * @chg_state: states involved in USB charger detection.
  * @chg_type: USB charger types.
@@ -246,9 +247,10 @@ struct rockchip_usb2phy {
 	struct device	*dev;
 	struct regmap	*grf;
 	struct regmap	*usbgrf;
-	struct clk	*clk;
+	struct clk_bulk_data	*clks;
 	struct clk	*clk480m;
 	struct clk_hw	clk480m_hw;
+	int			num_clks;
 	struct reset_control	*phy_reset;
 	enum usb_chg_state	chg_state;
 	enum power_supply_type	chg_type;
@@ -376,6 +378,7 @@ rockchip_usb2phy_clk480m_register(struct rockchip_usb2phy *rphy)
 {
 	struct device_node *node = rphy->dev->of_node;
 	struct clk_init_data init;
+	struct clk *refclk = of_clk_get_by_name(node, "phyclk");
 	const char *clk_name;
 	int ret = 0;
 
@@ -386,8 +389,8 @@ rockchip_usb2phy_clk480m_register(struct rockchip_usb2phy *rphy)
 	/* optional override of the clockname */
 	of_property_read_string(node, "clock-output-names", &init.name);
 
-	if (rphy->clk) {
-		clk_name = __clk_get_name(rphy->clk);
+	if (!IS_ERR(refclk)) {
+		clk_name = __clk_get_name(refclk);
 		init.parent_names = &clk_name;
 		init.num_parents = 1;
 	} else {
@@ -1406,22 +1409,29 @@ static int rockchip_usb2phy_probe(struct platform_device *pdev)
 	if (IS_ERR(rphy->phy_reset))
 		return PTR_ERR(rphy->phy_reset);
 
-	rphy->clk = devm_clk_get_optional_enabled(dev, "phyclk");
-	if (IS_ERR(rphy->clk)) {
-		return dev_err_probe(&pdev->dev, PTR_ERR(rphy->clk),
+	ret = devm_clk_bulk_get_all(dev, &rphy->clks);
+	if (ret == -EPROBE_DEFER) {
+		return dev_err_probe(&pdev->dev, -EPROBE_DEFER,
 				     "failed to get phyclk\n");
 	}
 
+	/* Clocks are optional */
+	rphy->num_clks = ret < 0 ? 0 : ret;
+
 	ret = rockchip_usb2phy_clk480m_register(rphy);
 	if (ret) {
 		dev_err(dev, "failed to register 480m output clock\n");
 		return ret;
 	}
 
+	ret = clk_bulk_prepare_enable(rphy->num_clks, rphy->clks);
+	if (ret)
+		return ret;
+
 	if (rphy->phy_cfg->phy_tuning) {
 		ret = rphy->phy_cfg->phy_tuning(rphy);
 		if (ret)
-			return ret;
+			goto disable_clks;
 	}
 
 	index = 0;
@@ -1484,6 +1494,8 @@ static int rockchip_usb2phy_probe(struct platform_device *pdev)
 
 put_child:
 	of_node_put(child_np);
+disable_clks:
+	clk_bulk_disable_unprepare(rphy->num_clks, rphy->clks);
 	return ret;
 }
 
@@ -1495,6 +1507,30 @@ static int rk3128_usb2phy_tuning(struct rockchip_usb2phy *rphy)
 				BIT(2) << BIT_WRITEABLE_SHIFT | 0);
 }
 
+static int rk3576_usb2phy_tuning(struct rockchip_usb2phy *rphy)
+{
+	int ret;
+	u32 reg = rphy->phy_cfg->reg;
+
+	/* Deassert SIDDQ to power on analog block */
+	ret = regmap_write(rphy->grf, reg + 0x0010, GENMASK(29, 29) | 0x0000);
+	if (ret)
+		return ret;
+
+	/* Do reset after exit IDDQ mode */
+	ret = rockchip_usb2phy_reset(rphy);
+	if (ret)
+		return ret;
+
+	/* HS DC Voltage Level Adjustment 4'b1001 : +5.89% */
+	ret |= regmap_write(rphy->grf, reg + 0x000c, GENMASK(27, 24) | 0x0900);
+
+	/* HS Transmitter Pre-Emphasis Current Control 2'b10 : 2x */
+	ret |= regmap_write(rphy->grf, reg + 0x0010, GENMASK(20, 19) | 0x0010);
+
+	return ret;
+}
+
 static int rk3588_usb2phy_tuning(struct rockchip_usb2phy *rphy)
 {
 	int ret;
@@ -1923,6 +1959,84 @@ static const struct rockchip_usb2phy_cfg rk3568_phy_cfgs[] = {
 	{ /* sentinel */ }
 };
 
+static const struct rockchip_usb2phy_cfg rk3576_phy_cfgs[] = {
+	{
+		.reg = 0x0,
+		.num_ports	= 1,
+		.phy_tuning	= rk3576_usb2phy_tuning,
+		.clkout_ctl	= { 0x0008, 0, 0, 1, 0 },
+		.port_cfgs	= {
+			[USB2PHY_PORT_OTG] = {
+				.phy_sus	= { 0x0000, 8, 0, 0, 0x1d1 },
+				.bvalid_det_en	= { 0x00c0, 1, 1, 0, 1 },
+				.bvalid_det_st	= { 0x00c4, 1, 1, 0, 1 },
+				.bvalid_det_clr = { 0x00c8, 1, 1, 0, 1 },
+				.ls_det_en	= { 0x00c0, 0, 0, 0, 1 },
+				.ls_det_st	= { 0x00c4, 0, 0, 0, 1 },
+				.ls_det_clr	= { 0x00c8, 0, 0, 0, 1 },
+				.disfall_en	= { 0x00c0, 6, 6, 0, 1 },
+				.disfall_st	= { 0x00c4, 6, 6, 0, 1 },
+				.disfall_clr	= { 0x00c8, 6, 6, 0, 1 },
+				.disrise_en	= { 0x00c0, 5, 5, 0, 1 },
+				.disrise_st	= { 0x00c4, 5, 5, 0, 1 },
+				.disrise_clr	= { 0x00c8, 5, 5, 0, 1 },
+				.utmi_avalid	= { 0x0080, 1, 1, 0, 1 },
+				.utmi_bvalid	= { 0x0080, 0, 0, 0, 1 },
+				.utmi_ls	= { 0x0080, 5, 4, 0, 1 },
+			}
+		},
+		.chg_det = {
+			.cp_det		= { 0x0080, 8, 8, 0, 1 },
+			.dcp_det	= { 0x0080, 8, 8, 0, 1 },
+			.dp_det		= { 0x0080, 9, 9, 1, 0 },
+			.idm_sink_en	= { 0x0010, 5, 5, 1, 0 },
+			.idp_sink_en	= { 0x0010, 5, 5, 0, 1 },
+			.idp_src_en	= { 0x0010, 14, 14, 0, 1 },
+			.rdm_pdwn_en	= { 0x0010, 14, 14, 0, 1 },
+			.vdm_src_en	= { 0x0010, 7, 6, 0, 3 },
+			.vdp_src_en	= { 0x0010, 7, 6, 0, 3 },
+		},
+	},
+	{
+		.reg = 0x2000,
+		.num_ports	= 1,
+		.phy_tuning	= rk3576_usb2phy_tuning,
+		.clkout_ctl	= { 0x2008, 0, 0, 1, 0 },
+		.port_cfgs	= {
+			[USB2PHY_PORT_OTG] = {
+				.phy_sus	= { 0x2000, 8, 0, 0, 0x1d1 },
+				.bvalid_det_en	= { 0x20c0, 1, 1, 0, 1 },
+				.bvalid_det_st	= { 0x20c4, 1, 1, 0, 1 },
+				.bvalid_det_clr = { 0x20c8, 1, 1, 0, 1 },
+				.ls_det_en	= { 0x20c0, 0, 0, 0, 1 },
+				.ls_det_st	= { 0x20c4, 0, 0, 0, 1 },
+				.ls_det_clr	= { 0x20c8, 0, 0, 0, 1 },
+				.disfall_en	= { 0x20c0, 6, 6, 0, 1 },
+				.disfall_st	= { 0x20c4, 6, 6, 0, 1 },
+				.disfall_clr	= { 0x20c8, 6, 6, 0, 1 },
+				.disrise_en	= { 0x20c0, 5, 5, 0, 1 },
+				.disrise_st	= { 0x20c4, 5, 5, 0, 1 },
+				.disrise_clr	= { 0x20c8, 5, 5, 0, 1 },
+				.utmi_avalid	= { 0x2080, 1, 1, 0, 1 },
+				.utmi_bvalid	= { 0x2080, 0, 0, 0, 1 },
+				.utmi_ls	= { 0x2080, 5, 4, 0, 1 },
+			}
+		},
+		.chg_det = {
+			.cp_det		= { 0x2080, 8, 8, 0, 1 },
+			.dcp_det	= { 0x2080, 8, 8, 0, 1 },
+			.dp_det		= { 0x2080, 9, 9, 1, 0 },
+			.idm_sink_en	= { 0x2010, 5, 5, 1, 0 },
+			.idp_sink_en	= { 0x2010, 5, 5, 0, 1 },
+			.idp_src_en	= { 0x2010, 14, 14, 0, 1 },
+			.rdm_pdwn_en	= { 0x2010, 14, 14, 0, 1 },
+			.vdm_src_en	= { 0x2010, 7, 6, 0, 3 },
+			.vdp_src_en	= { 0x2010, 7, 6, 0, 3 },
+		},
+	},
+	{ /* sentinel */ }
+};
+
 static const struct rockchip_usb2phy_cfg rk3588_phy_cfgs[] = {
 	{
 		.reg = 0x0000,
@@ -2094,6 +2208,7 @@ static const struct of_device_id rockchip_usb2phy_dt_match[] = {
 	{ .compatible = "rockchip,rk3366-usb2phy", .data = &rk3366_phy_cfgs },
 	{ .compatible = "rockchip,rk3399-usb2phy", .data = &rk3399_phy_cfgs },
 	{ .compatible = "rockchip,rk3568-usb2phy", .data = &rk3568_phy_cfgs },
+	{ .compatible = "rockchip,rk3576-usb2phy", .data = &rk3576_phy_cfgs },
 	{ .compatible = "rockchip,rk3588-usb2phy", .data = &rk3588_phy_cfgs },
 	{ .compatible = "rockchip,rv1108-usb2phy", .data = &rv1108_phy_cfgs },
 	{}
-- 
2.45.2


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v2 2/2] phy: rockchip: inno-usb2: Add usb2 phys support for rk3576

[Heiko Stuebner]

Am Dienstag, 24. September 2024, 10:55:10 CEST schrieb Frank Wang:
> From: William Wu <william.wu@rock-chips.com>
> 
> The RK3576 SoC has two independent USB2.0 PHYs, and
> each PHY has one port.

Can you please split the content into "converting to clk_bulk" (see
additional comment below) and "add rk3576" please?

That would make the patch a lot cleaner.


> @@ -376,6 +378,7 @@ rockchip_usb2phy_clk480m_register(struct rockchip_usb2phy *rphy)
>  {
>  	struct device_node *node = rphy->dev->of_node;
>  	struct clk_init_data init;
> +	struct clk *refclk = of_clk_get_by_name(node, "phyclk");

Doesn't this create an imbalance - with the missing put?
I think ideally just define clk_bulk_data structs for the
1-clock and 3-clock variant, attach that to the device-data
and then use the regular devm_clk_bulk_get ?

That way you can then retrieve the clock from that struct?


Thanks
Heiko



-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v2 2/2] phy: rockchip: inno-usb2: Add usb2 phys support for rk3576

[frawang]

Hi Heiko,

On 2024/9/24 18:01, Heiko Stuebner wrote:
> Am Dienstag, 24. September 2024, 10:55:10 CEST schrieb Frank Wang:
>> From: William Wu <william.wu@rock-chips.com>
>>
>> The RK3576 SoC has two independent USB2.0 PHYs, and
>> each PHY has one port.
> Can you please split the content into "converting to clk_bulk" (see
> additional comment below) and "add rk3576" please?
>
> That would make the patch a lot cleaner.

OK, I shall amend in the next patch.

>
>> @@ -376,6 +378,7 @@ rockchip_usb2phy_clk480m_register(struct rockchip_usb2phy *rphy)
>>   {
>>   	struct device_node *node = rphy->dev->of_node;
>>   	struct clk_init_data init;
>> +	struct clk *refclk = of_clk_get_by_name(node, "phyclk");
> Doesn't this create an imbalance - with the missing put?
> I think ideally just define clk_bulk_data structs for the
> 1-clock and 3-clock variant, attach that to the device-data
> and then use the regular devm_clk_bulk_get ?
>
> That way you can then retrieve the clock from that struct?

How about keep the clk_bulk_data and num_clks member in rockchip_usb2phy 
structs, and retrieve the clock by "clks.id" here?
Just like the following codes.

@@ -378,8 +378,9 @@ rockchip_usb2phy_clk480m_register(struct 
rockchip_usb2phy *rphy)
  {
         struct device_node *node = rphy->dev->of_node;
         struct clk_init_data init;
-       struct clk *refclk = of_clk_get_by_name(node, "phyclk");
+       struct clk *refclk = NULL;
         const char *clk_name;
+       int i;
         int ret = 0;

         init.flags = 0;
@@ -389,6 +390,13 @@ rockchip_usb2phy_clk480m_register(struct 
rockchip_usb2phy *rphy)
         /* optional override of the clockname */
         of_property_read_string(node, "clock-output-names", &init.name);

+       for (i = 0; i < rphy->num_clks; i++) {
+               if (!strncmp(rphy->clks[i].id, "phyclk", 6)) {
+                       refclk = rphy->clks[i].clk;
+                       break;
+               }
+       }
+

BR.
Frank

>
> Thanks
> Heiko
>

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v2 2/2] phy: rockchip: inno-usb2: Add usb2 phys support for rk3576

[Heiko Stuebner]

Hi Frank,

Am Mittwoch, 25. September 2024, 03:42:35 CEST schrieb frawang:
> >> @@ -376,6 +378,7 @@ rockchip_usb2phy_clk480m_register(struct rockchip_usb2phy *rphy)
> >>   {
> >>   	struct device_node *node = rphy->dev->of_node;
> >>   	struct clk_init_data init;
> >> +	struct clk *refclk = of_clk_get_by_name(node, "phyclk");
> > Doesn't this create an imbalance - with the missing put?
> > I think ideally just define clk_bulk_data structs for the
> > 1-clock and 3-clock variant, attach that to the device-data
> > and then use the regular devm_clk_bulk_get ?
> >
> > That way you can then retrieve the clock from that struct?
> 
> How about keep the clk_bulk_data and num_clks member in rockchip_usb2phy 
> structs, and retrieve the clock by "clks.id" here?
> Just like the following codes.
> 
> @@ -378,8 +378,9 @@ rockchip_usb2phy_clk480m_register(struct 
> rockchip_usb2phy *rphy)
>   {
>          struct device_node *node = rphy->dev->of_node;
>          struct clk_init_data init;
> -       struct clk *refclk = of_clk_get_by_name(node, "phyclk");
> +       struct clk *refclk = NULL;
>          const char *clk_name;
> +       int i;
>          int ret = 0;
> 
>          init.flags = 0;
> @@ -389,6 +390,13 @@ rockchip_usb2phy_clk480m_register(struct 
> rockchip_usb2phy *rphy)
>          /* optional override of the clockname */
>          of_property_read_string(node, "clock-output-names", &init.name);
> 
> +       for (i = 0; i < rphy->num_clks; i++) {
> +               if (!strncmp(rphy->clks[i].id, "phyclk", 6)) {
> +                       refclk = rphy->clks[i].clk;
> +                       break;
> +               }
> +       }
> +

this is exactly what I had in mind :-)

Thanks
Heiko



-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v2 1/2] dt-bindings: phy: rockchip,inno-usb2phy: add rk3576

[Heiko Stuebner]

Hi Frank,

Am Dienstag, 24. September 2024, 10:55:09 CEST schrieb Frank Wang:
> From: Frank Wang <frank.wang@rock-chips.com>
> 
> Add compatible for the USB2 phy in the Rockchip RK3576 SoC.

can you please add some details to the commit message, about those
new clocks. I.e. what they do.

Thanks
Heiko

> Signed-off-by: Frank Wang <frank.wang@rock-chips.com>
> ---
> Changelog:
> v2:
>  - Categorize clock names by oneOf keyword.
> 
> v1:
>  - https://patchwork.kernel.org/project/linux-phy/patch/20240923025326.10467-1-frank.wang@rock-chips.com/
> 
>  .../bindings/phy/rockchip,inno-usb2phy.yaml      | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml b/Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml
> index 5254413137c64..8af4e0f8637fc 100644
> --- a/Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml
> +++ b/Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml
> @@ -20,6 +20,7 @@ properties:
>        - rockchip,rk3366-usb2phy
>        - rockchip,rk3399-usb2phy
>        - rockchip,rk3568-usb2phy
> +      - rockchip,rk3576-usb2phy
>        - rockchip,rk3588-usb2phy
>        - rockchip,rv1108-usb2phy
>  
> @@ -34,10 +35,20 @@ properties:
>      const: 0
>  
>    clocks:
> -    maxItems: 1
> +    minItems: 1
> +    maxItems: 3
>  
>    clock-names:
> -    const: phyclk
> +    minItems: 1
> +    maxItems: 3
> +    items:
> +      oneOf:
> +        - description: aclk for USB MMU.
> +          const: aclk
> +        - description: aclk_slv for USB MMU.
> +          const: aclk_slv
> +        - description: PHY input reference clocks.
> +          const: phyclk
>  
>    assigned-clocks:
>      description:
> @@ -143,6 +154,7 @@ allOf:
>            contains:
>              enum:
>                - rockchip,rk3568-usb2phy
> +              - rockchip,rk3576-usb2phy
>                - rockchip,rk3588-usb2phy
>  
>      then:
> 





-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v2 1/2] dt-bindings: phy: rockchip,inno-usb2phy: add rk3576

[Frank Wang]

Hi Heiko,

On 2024/9/24 18:01, Heiko Stuebner wrote:
> Hi Frank,
>
> Am Dienstag, 24. September 2024, 10:55:09 CEST schrieb Frank Wang:
>> From: Frank Wang <frank.wang@rock-chips.com>
>>
>> Add compatible for the USB2 phy in the Rockchip RK3576 SoC.
> can you please add some details to the commit message, about those
> new clocks. I.e. what they do.

OK, I shall add in the next version.

BR.
Frank

> Thanks
> Heiko
>
>> Signed-off-by: Frank Wang <frank.wang@rock-chips.com>
>> ---
>> Changelog:
>> v2:
>>   - Categorize clock names by oneOf keyword.
>>
>> v1:
>>   - https://patchwork.kernel.org/project/linux-phy/patch/20240923025326.10467-1-frank.wang@rock-chips.com/
>>
>>   .../bindings/phy/rockchip,inno-usb2phy.yaml      | 16 ++++++++++++++--
>>   1 file changed, 14 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml b/Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml
>> index 5254413137c64..8af4e0f8637fc 100644
>> --- a/Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml
>> +++ b/Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml
>> @@ -20,6 +20,7 @@ properties:
>>         - rockchip,rk3366-usb2phy
>>         - rockchip,rk3399-usb2phy
>>         - rockchip,rk3568-usb2phy
>> +      - rockchip,rk3576-usb2phy
>>         - rockchip,rk3588-usb2phy
>>         - rockchip,rv1108-usb2phy
>>   
>> @@ -34,10 +35,20 @@ properties:
>>       const: 0
>>   
>>     clocks:
>> -    maxItems: 1
>> +    minItems: 1
>> +    maxItems: 3
>>   
>>     clock-names:
>> -    const: phyclk
>> +    minItems: 1
>> +    maxItems: 3
>> +    items:
>> +      oneOf:
>> +        - description: aclk for USB MMU.
>> +          const: aclk
>> +        - description: aclk_slv for USB MMU.
>> +          const: aclk_slv
>> +        - description: PHY input reference clocks.
>> +          const: phyclk
>>   
>>     assigned-clocks:
>>       description:
>> @@ -143,6 +154,7 @@ allOf:
>>             contains:
>>               enum:
>>                 - rockchip,rk3568-usb2phy
>> +              - rockchip,rk3576-usb2phy
>>                 - rockchip,rk3588-usb2phy
>>   
>>       then:
>>


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v2 1/2] dt-bindings: phy: rockchip,inno-usb2phy: add rk3576

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 2116 bytes --]

On Tue, Sep 24, 2024 at 04:55:09PM +0800, Frank Wang wrote:
> From: Frank Wang <frank.wang@rock-chips.com>
> 
> Add compatible for the USB2 phy in the Rockchip RK3576 SoC.
> 
> Signed-off-by: Frank Wang <frank.wang@rock-chips.com>
> ---
> Changelog:
> v2:
>  - Categorize clock names by oneOf keyword.
> 
> v1:
>  - https://patchwork.kernel.org/project/linux-phy/patch/20240923025326.10467-1-frank.wang@rock-chips.com/
> 
>  .../bindings/phy/rockchip,inno-usb2phy.yaml      | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml b/Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml
> index 5254413137c64..8af4e0f8637fc 100644
> --- a/Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml
> +++ b/Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml
> @@ -20,6 +20,7 @@ properties:
>        - rockchip,rk3366-usb2phy
>        - rockchip,rk3399-usb2phy
>        - rockchip,rk3568-usb2phy
> +      - rockchip,rk3576-usb2phy
>        - rockchip,rk3588-usb2phy
>        - rockchip,rv1108-usb2phy
>  
> @@ -34,10 +35,20 @@ properties:
>      const: 0
>  
>    clocks:
> -    maxItems: 1
> +    minItems: 1
> +    maxItems: 3
>  
>    clock-names:
> -    const: phyclk
> +    minItems: 1
> +    maxItems: 3

clock-names isn't a required property, you can't allow jumbling the order
like this does without breaking the ABI. Why can't the new device have
phyclk in position 1?

> +    items:
> +      oneOf:
> +        - description: aclk for USB MMU.
> +          const: aclk
> +        - description: aclk_slv for USB MMU.
> +          const: aclk_slv
> +        - description: PHY input reference clocks.
> +          const: phyclk
>  
>    assigned-clocks:
>      description:
> @@ -143,6 +154,7 @@ allOf:
>            contains:
>              enum:
>                - rockchip,rk3568-usb2phy
> +              - rockchip,rk3576-usb2phy
>                - rockchip,rk3588-usb2phy
>  
>      then:
> -- 
> 2.45.2
> 

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 112 bytes --]

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v2 1/2] dt-bindings: phy: rockchip,inno-usb2phy: add rk3576

[Frank Wang]

Hi Conor,

On 2024/9/25 0:11, Conor Dooley wrote:
> On Tue, Sep 24, 2024 at 04:55:09PM +0800, Frank Wang wrote:
>> From: Frank Wang <frank.wang@rock-chips.com>
>>
>> Add compatible for the USB2 phy in the Rockchip RK3576 SoC.
>>
>> Signed-off-by: Frank Wang <frank.wang@rock-chips.com>
>> ---
>> Changelog:
>> v2:
>>   - Categorize clock names by oneOf keyword.
>>
>> v1:
>>   - https://patchwork.kernel.org/project/linux-phy/patch/20240923025326.10467-1-frank.wang@rock-chips.com/
>>
>>   .../bindings/phy/rockchip,inno-usb2phy.yaml      | 16 ++++++++++++++--
>>   1 file changed, 14 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml b/Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml
>> index 5254413137c64..8af4e0f8637fc 100644
>> --- a/Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml
>> +++ b/Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml
>> @@ -20,6 +20,7 @@ properties:
>>         - rockchip,rk3366-usb2phy
>>         - rockchip,rk3399-usb2phy
>>         - rockchip,rk3568-usb2phy
>> +      - rockchip,rk3576-usb2phy
>>         - rockchip,rk3588-usb2phy
>>         - rockchip,rv1108-usb2phy
>>   
>> @@ -34,10 +35,20 @@ properties:
>>       const: 0
>>   
>>     clocks:
>> -    maxItems: 1
>> +    minItems: 1
>> +    maxItems: 3
>>   
>>     clock-names:
>> -    const: phyclk
>> +    minItems: 1
>> +    maxItems: 3
> clock-names isn't a required property, you can't allow jumbling the order
> like this does without breaking the ABI. Why can't the new device have
> phyclk in position 1?

I sent a draft changes in patch v1 comments which put the "phyclk" in 
position 1, Krzysztof said I have messed the order, so I reorder them in v2.
Did I misunderstand? anyway, should the changes like the below?

@@ -34,10 +35,20 @@ properties:
       const: 0

     clocks:
-    maxItems: 1
+    minItems: 1
+    maxItems: 3

     clock-names:
-    const: phyclk
+    minItems: 1
+    maxItems: 3
+    items:
+      oneOf:
+        - description: PHY input reference clocks.
+          const: phyclk
+        - description: aclk for USB MMU.
+          const: aclk
+        - description: aclk_slv for USB MMU.
+          const: aclk_slv


BR.
Frank

>> +    items:
>> +      oneOf:
>> +        - description: aclk for USB MMU.
>> +          const: aclk
>> +        - description: aclk_slv for USB MMU.
>> +          const: aclk_slv
>> +        - description: PHY input reference clocks.
>> +          const: phyclk
>>   
>>     assigned-clocks:
>>       description:
>> @@ -143,6 +154,7 @@ allOf:
>>             contains:
>>               enum:
>>                 - rockchip,rk3568-usb2phy
>> +              - rockchip,rk3576-usb2phy
>>                 - rockchip,rk3588-usb2phy
>>   
>>       then:
>> -- 
>> 2.45.2
>>


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v2 1/2] dt-bindings: phy: rockchip,inno-usb2phy: add rk3576

[Krzysztof Kozlowski]

On 25/09/2024 04:09, Frank Wang wrote:
> Hi Conor,
> 
> On 2024/9/25 0:11, Conor Dooley wrote:
>> On Tue, Sep 24, 2024 at 04:55:09PM +0800, Frank Wang wrote:
>>> From: Frank Wang <frank.wang@rock-chips.com>
>>>
>>> Add compatible for the USB2 phy in the Rockchip RK3576 SoC.
>>>
>>> Signed-off-by: Frank Wang <frank.wang@rock-chips.com>
>>> ---
>>> Changelog:
>>> v2:
>>>   - Categorize clock names by oneOf keyword.
>>>
>>> v1:
>>>   - https://patchwork.kernel.org/project/linux-phy/patch/20240923025326.10467-1-frank.wang@rock-chips.com/
>>>
>>>   .../bindings/phy/rockchip,inno-usb2phy.yaml      | 16 ++++++++++++++--
>>>   1 file changed, 14 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml b/Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml
>>> index 5254413137c64..8af4e0f8637fc 100644
>>> --- a/Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml
>>> +++ b/Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml
>>> @@ -20,6 +20,7 @@ properties:
>>>         - rockchip,rk3366-usb2phy
>>>         - rockchip,rk3399-usb2phy
>>>         - rockchip,rk3568-usb2phy
>>> +      - rockchip,rk3576-usb2phy
>>>         - rockchip,rk3588-usb2phy
>>>         - rockchip,rv1108-usb2phy
>>>   
>>> @@ -34,10 +35,20 @@ properties:
>>>       const: 0
>>>   
>>>     clocks:
>>> -    maxItems: 1
>>> +    minItems: 1
>>> +    maxItems: 3
>>>   
>>>     clock-names:
>>> -    const: phyclk
>>> +    minItems: 1
>>> +    maxItems: 3
>> clock-names isn't a required property, you can't allow jumbling the order
>> like this does without breaking the ABI. Why can't the new device have
>> phyclk in position 1?
> 
> I sent a draft changes in patch v1 comments which put the "phyclk" in 

No, you did not. You sent buggy code which was never tested.

> position 1, Krzysztof said I have messed the order, so I reorder them in v2.

No, I did not. I said your current code (from your reply or patch v2)
messes the order. Even though I sent you reply that this code is wrong,
you still decided to ignore my feedback and send it.

To be clear:
NAK

> Did I misunderstand? anyway, should the changes like the below?

Read all the answers again instead of putting wrong words to wrong patches.


Best regards,
Krzysztof


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v2 1/2] dt-bindings: phy: rockchip,inno-usb2phy: add rk3576

[Frank Wang]

Hi Krzysztof,

On 2024/9/25 15:16, Krzysztof Kozlowski wrote:
> On 25/09/2024 04:09, Frank Wang wrote:
>> Hi Conor,
>>
>> On 2024/9/25 0:11, Conor Dooley wrote:
>>> On Tue, Sep 24, 2024 at 04:55:09PM +0800, Frank Wang wrote:
>>>> From: Frank Wang <frank.wang@rock-chips.com>
>>>>
>>>> Add compatible for the USB2 phy in the Rockchip RK3576 SoC.
>>>>
>>>> Signed-off-by: Frank Wang <frank.wang@rock-chips.com>
>>>> ---
>>>> Changelog:
>>>> v2:
>>>>    - Categorize clock names by oneOf keyword.
>>>>
>>>> v1:
>>>>    - https://patchwork.kernel.org/project/linux-phy/patch/20240923025326.10467-1-frank.wang@rock-chips.com/
>>>>
>>>>    .../bindings/phy/rockchip,inno-usb2phy.yaml      | 16 ++++++++++++++--
>>>>    1 file changed, 14 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml b/Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml
>>>> index 5254413137c64..8af4e0f8637fc 100644
>>>> --- a/Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml
>>>> +++ b/Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml
>>>> @@ -20,6 +20,7 @@ properties:
>>>>          - rockchip,rk3366-usb2phy
>>>>          - rockchip,rk3399-usb2phy
>>>>          - rockchip,rk3568-usb2phy
>>>> +      - rockchip,rk3576-usb2phy
>>>>          - rockchip,rk3588-usb2phy
>>>>          - rockchip,rv1108-usb2phy
>>>>    
>>>> @@ -34,10 +35,20 @@ properties:
>>>>        const: 0
>>>>    
>>>>      clocks:
>>>> -    maxItems: 1
>>>> +    minItems: 1
>>>> +    maxItems: 3
>>>>    
>>>>      clock-names:
>>>> -    const: phyclk
>>>> +    minItems: 1
>>>> +    maxItems: 3
>>> clock-names isn't a required property, you can't allow jumbling the order
>>> like this does without breaking the ABI. Why can't the new device have
>>> phyclk in position 1?
>> I sent a draft changes in patch v1 comments which put the "phyclk" in
> No, you did not. You sent buggy code which was never tested.
>
>> position 1, Krzysztof said I have messed the order, so I reorder them in v2.
> No, I did not. I said your current code (from your reply or patch v2)
> messes the order. Even though I sent you reply that this code is wrong,
> you still decided to ignore my feedback and send it.
Sorry I misunderstood the 'oneOf' and "order" you said.

I shall amend the patch and send v3 later.

BR.
Frank

> To be clear:
> NAK
>
>> Did I misunderstand? anyway, should the changes like the below?
> Read all the answers again instead of putting wrong words to wrong patches.
>
>
> Best regards,
> Krzysztof
>


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy