Re: [PATCH v2 4/5] phy: rockchip: inno-usb2: Add clkout_ctl_phy support

[neil.armstrong@linaro.org]

On 5/5/26 19:04, Heiko Stuebner wrote:
> From: Jonas Karlman <jonas@kwiboo.se>
> 
> The 480m clk is controlled using regs in the PHY address space and not
> in the USB GRF address space on e.g. RK3528 and RK3506.
> 
> Add a clkout_ctl_phy usb2phy_reg to handle enable/disable of the 480m
> clk on these SoCs.
> 
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---
>   drivers/phy/rockchip/phy-rockchip-inno-usb2.c | 47 +++++++++++++++----
>   1 file changed, 38 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
> index 7cec45192393..d8879fcd4291 100644
> --- a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
> +++ b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
> @@ -179,6 +179,7 @@ struct rockchip_usb2phy_cfg {
>   	unsigned int	num_ports;
>   	int (*phy_tuning)(struct rockchip_usb2phy *rphy);
>   	struct usb2phy_reg	clkout_ctl;
> +	struct usb2phy_reg	clkout_ctl_phy;
>   	const struct rockchip_usb2phy_port_cfg	port_cfgs[USB2PHY_NUM_PORTS];
>   	const struct rockchip_chg_det_reg	chg_det;
>   };
> @@ -228,6 +229,7 @@ struct rockchip_usb2phy_port {
>    * struct rockchip_usb2phy - usb2.0 phy driver data.
>    * @dev: pointer to device.
>    * @grf: General Register Files regmap.
> + * @phy_base: USB PHY regmap.
>    * @clks: array of phy input clocks.
>    * @clk480m: clock struct of phy output clk.
>    * @clk480m_hw: clock struct of phy output clk management.
> @@ -245,6 +247,7 @@ struct rockchip_usb2phy_port {
>   struct rockchip_usb2phy {
>   	struct device	*dev;
>   	struct regmap	*grf;
> +	struct regmap	*phy_base;
>   	struct clk_bulk_data	*clks;
>   	struct clk	*clk480m;
>   	struct clk_hw	clk480m_hw;
> @@ -312,15 +315,33 @@ static void rockchip_usb2phy_clk_bulk_disable(void *data)
>   	clk_bulk_disable_unprepare(rphy->num_clks, rphy->clks);
>   }
>   
> -static int rockchip_usb2phy_clk480m_prepare(struct clk_hw *hw)
> +static void
> +rockchip_usb2phy_clk480m_clkout_ctl(struct clk_hw *hw, struct regmap **base,
> +				    const struct usb2phy_reg **clkout_ctl)
>   {
>   	struct rockchip_usb2phy *rphy =
>   		container_of(hw, struct rockchip_usb2phy, clk480m_hw);
> +
> +	if (rphy->phy_cfg->clkout_ctl_phy.enable) {
> +		*base = rphy->phy_base;
> +		*clkout_ctl = &rphy->phy_cfg->clkout_ctl_phy;
> +	} else {
> +		*base = rphy->grf;
> +		*clkout_ctl = &rphy->phy_cfg->clkout_ctl;
> +	}
> +}
> +
> +static int rockchip_usb2phy_clk480m_prepare(struct clk_hw *hw)
> +{
> +	const struct usb2phy_reg *clkout_ctl;
> +	struct regmap *base;
>   	int ret;
>   
> +	rockchip_usb2phy_clk480m_clkout_ctl(hw, &base, &clkout_ctl);
> +
>   	/* turn on 480m clk output if it is off */
> -	if (!property_enabled(rphy->grf, &rphy->phy_cfg->clkout_ctl)) {
> -		ret = property_enable(rphy->grf, &rphy->phy_cfg->clkout_ctl, true);
> +	if (!property_enabled(base, clkout_ctl)) {
> +		ret = property_enable(base, clkout_ctl, true);
>   		if (ret)
>   			return ret;
>   
> @@ -333,19 +354,23 @@ static int rockchip_usb2phy_clk480m_prepare(struct clk_hw *hw)
>   
>   static void rockchip_usb2phy_clk480m_unprepare(struct clk_hw *hw)
>   {
> -	struct rockchip_usb2phy *rphy =
> -		container_of(hw, struct rockchip_usb2phy, clk480m_hw);
> +	const struct usb2phy_reg *clkout_ctl;
> +	struct regmap *base;
> +
> +	rockchip_usb2phy_clk480m_clkout_ctl(hw, &base, &clkout_ctl);
>   
>   	/* turn off 480m clk output */
> -	property_enable(rphy->grf, &rphy->phy_cfg->clkout_ctl, false);
> +	property_enable(base, clkout_ctl, false);
>   }
>   
>   static int rockchip_usb2phy_clk480m_prepared(struct clk_hw *hw)
>   {
> -	struct rockchip_usb2phy *rphy =
> -		container_of(hw, struct rockchip_usb2phy, clk480m_hw);
> +	const struct usb2phy_reg *clkout_ctl;
> +	struct regmap *base;
> +
> +	rockchip_usb2phy_clk480m_clkout_ctl(hw, &base, &clkout_ctl);
>   
> -	return property_enabled(rphy->grf, &rphy->phy_cfg->clkout_ctl);
> +	return property_enabled(base, clkout_ctl);
>   }
>   
>   static unsigned long
> @@ -1336,9 +1361,13 @@ static int rockchip_usb2phy_probe(struct platform_device *pdev)
>   
>   	if (!dev->parent || !dev->parent->of_node ||
>   	    of_property_present(np, "rockchip,usbgrf")) {
> +		rphy->phy_base = device_node_to_regmap(np);
> +		if (IS_ERR(rphy->phy_base))
> +			return PTR_ERR(rphy->phy_base);
>   		rphy->grf = syscon_regmap_lookup_by_phandle(np, "rockchip,usbgrf");
>   	} else {
>   		rphy->grf = syscon_node_to_regmap(dev->parent->of_node);
> +		rphy->phy_base = rphy->grf;
>   	}
>   	if (IS_ERR(rphy->grf))
>   		return PTR_ERR(rphy->grf);

The logic is not easy to follow, but it looks right.

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>

Thanks,
Neil