Re: [PATCH v2 14/21] phy: sun4i-usb: Rework "pmu_unk1" handling

From: Icenowy Zheng <icenowy@aosc.io>
To: Andre Przywara <andre.przywara@arm.com>,
	Maxime Ripard <mripard@kernel.org>, Chen-Yu Tsai <wens@csie.org>,
	Jernej Skrabec <jernej.skrabec@siol.net>
Cc: "Icenowy Zheng" <icenowy@aosc.xyz>,
	"Linus Walleij" <linus.walleij@linaro.org>,
	"Rob Herring" <robh@kernel.org>,
	"Clément Péron" <peron.clem@gmail.com>,
	"Shuosheng Huang" <huangshuosheng@allwinnertech.com>,
	"Yangtao Li" <tiny.windzz@gmail.com>,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, linux-sunxi@googlegroups.com,
	"Kishon Vijay Abraham I" <kishon@ti.com>,
	"Vinod Koul" <vkoul@kernel.org>
Subject: Re: [PATCH v2 14/21] phy: sun4i-usb: Rework "pmu_unk1" handling
Date: Mon, 14 Dec 2020 02:24:58 +0800	[thread overview]
Message-ID: <02cbecddf07dc46f8fb7e0d93b7532e3c239084e.camel@aosc.io> (raw)
In-Reply-To: <20201211011934.6171-15-andre.przywara@arm.com>

在 2020-12-11星期五的 01:19 +0000，Andre Przywara写道：
> Newer SoCs (A100, H616) need to clear a different bit in our
> "unknown"
> PMU PHY register.

It looks like that the unknown PHY register is PHYCTL register for each
individual PHY, and the bit that is cleared is
called SUNXI_HCI_PHY_CTRL_SIDDQ in the BSP (similar to
the USBC_PHY_CTL_SIDDQ we cleared for main PHYCTL).

> 
> Generalise the existing code by allowing configs to specify a bitmask
> of bits to clear.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  drivers/phy/allwinner/phy-sun4i-usb.c | 28 +++++++++++------------
> ----
>  1 file changed, 11 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/phy/allwinner/phy-sun4i-usb.c
> b/drivers/phy/allwinner/phy-sun4i-usb.c
> index 651d5e2a25ce..4ba0699e0bb4 100644
> --- a/drivers/phy/allwinner/phy-sun4i-usb.c
> +++ b/drivers/phy/allwinner/phy-sun4i-usb.c
> @@ -115,9 +115,9 @@ struct sun4i_usb_phy_cfg {
>  	int hsic_index;
>  	enum sun4i_usb_phy_type type;
>  	u32 disc_thresh;
> +	u32 pmu_unk1_clrbits;
>  	u8 phyctl_offset;
>  	bool dedicated_clocks;
> -	bool enable_pmu_unk1;
>  	bool phy0_dual_route;
>  	int missing_phys;
>  };
> @@ -288,6 +288,12 @@ static int sun4i_usb_phy_init(struct phy *_phy)
>  		return ret;
>  	}
>  
> +	if (phy->pmu && data->cfg->pmu_unk1_clrbits) {
> +		val = readl(phy->pmu + REG_PMU_UNK1);
> +		val &= ~data->cfg->pmu_unk1_clrbits;
> +		writel(val, phy->pmu + REG_PMU_UNK1);
> +	}
> +
>  	if (data->cfg->type == sun8i_a83t_phy ||
>  	    data->cfg->type == sun50i_h6_phy) {
>  		if (phy->index == 0) {
> @@ -297,11 +303,6 @@ static int sun4i_usb_phy_init(struct phy *_phy)
>  			writel(val, data->base + data->cfg-
> >phyctl_offset);
>  		}
>  	} else {
> -		if (phy->pmu && data->cfg->enable_pmu_unk1) {
> -			val = readl(phy->pmu + REG_PMU_UNK1);
> -			writel(val & ~2, phy->pmu + REG_PMU_UNK1);
> -		}
> -
>  		/* Enable USB 45 Ohm resistor calibration */
>  		if (phy->index == 0)
>  			sun4i_usb_phy_write(phy, PHY_RES45_CAL_EN,
> 0x01, 1);
> @@ -867,7 +868,6 @@ static const struct sun4i_usb_phy_cfg
> sun4i_a10_cfg = {
>  	.disc_thresh = 3,
>  	.phyctl_offset = REG_PHYCTL_A10,
>  	.dedicated_clocks = false,
> -	.enable_pmu_unk1 = false,
>  };
>  
>  static const struct sun4i_usb_phy_cfg sun5i_a13_cfg = {
> @@ -876,7 +876,6 @@ static const struct sun4i_usb_phy_cfg
> sun5i_a13_cfg = {
>  	.disc_thresh = 2,
>  	.phyctl_offset = REG_PHYCTL_A10,
>  	.dedicated_clocks = false,
> -	.enable_pmu_unk1 = false,
>  };
>  
>  static const struct sun4i_usb_phy_cfg sun6i_a31_cfg = {
> @@ -885,7 +884,6 @@ static const struct sun4i_usb_phy_cfg
> sun6i_a31_cfg = {
>  	.disc_thresh = 3,
>  	.phyctl_offset = REG_PHYCTL_A10,
>  	.dedicated_clocks = true,
> -	.enable_pmu_unk1 = false,
>  };
>  
>  static const struct sun4i_usb_phy_cfg sun7i_a20_cfg = {
> @@ -894,7 +892,6 @@ static const struct sun4i_usb_phy_cfg
> sun7i_a20_cfg = {
>  	.disc_thresh = 2,
>  	.phyctl_offset = REG_PHYCTL_A10,
>  	.dedicated_clocks = false,
> -	.enable_pmu_unk1 = false,
>  };
>  
>  static const struct sun4i_usb_phy_cfg sun8i_a23_cfg = {
> @@ -903,7 +900,6 @@ static const struct sun4i_usb_phy_cfg
> sun8i_a23_cfg = {
>  	.disc_thresh = 3,
>  	.phyctl_offset = REG_PHYCTL_A10,
>  	.dedicated_clocks = true,
> -	.enable_pmu_unk1 = false,
>  };
>  
>  static const struct sun4i_usb_phy_cfg sun8i_a33_cfg = {
> @@ -912,7 +908,6 @@ static const struct sun4i_usb_phy_cfg
> sun8i_a33_cfg = {
>  	.disc_thresh = 3,
>  	.phyctl_offset = REG_PHYCTL_A33,
>  	.dedicated_clocks = true,
> -	.enable_pmu_unk1 = false,
>  };
>  
>  static const struct sun4i_usb_phy_cfg sun8i_a83t_cfg = {
> @@ -929,7 +924,7 @@ static const struct sun4i_usb_phy_cfg
> sun8i_h3_cfg = {
>  	.disc_thresh = 3,
>  	.phyctl_offset = REG_PHYCTL_A33,
>  	.dedicated_clocks = true,
> -	.enable_pmu_unk1 = true,
> +	.pmu_unk1_clrbits = BIT(1),
>  	.phy0_dual_route = true,
>  };
>  
> @@ -939,7 +934,7 @@ static const struct sun4i_usb_phy_cfg
> sun8i_r40_cfg = {
>  	.disc_thresh = 3,
>  	.phyctl_offset = REG_PHYCTL_A33,
>  	.dedicated_clocks = true,
> -	.enable_pmu_unk1 = true,
> +	.pmu_unk1_clrbits = BIT(1),
>  	.phy0_dual_route = true,
>  };
>  
> @@ -949,7 +944,7 @@ static const struct sun4i_usb_phy_cfg
> sun8i_v3s_cfg = {
>  	.disc_thresh = 3,
>  	.phyctl_offset = REG_PHYCTL_A33,
>  	.dedicated_clocks = true,
> -	.enable_pmu_unk1 = true,
> +	.pmu_unk1_clrbits = BIT(1),
>  	.phy0_dual_route = true,
>  };
>  
> @@ -959,7 +954,7 @@ static const struct sun4i_usb_phy_cfg
> sun50i_a64_cfg = {
>  	.disc_thresh = 3,
>  	.phyctl_offset = REG_PHYCTL_A33,
>  	.dedicated_clocks = true,
> -	.enable_pmu_unk1 = true,
> +	.pmu_unk1_clrbits = BIT(1),
>  	.phy0_dual_route = true,
>  };
>  
> @@ -969,7 +964,6 @@ static const struct sun4i_usb_phy_cfg
> sun50i_h6_cfg = {
>  	.disc_thresh = 3,
>  	.phyctl_offset = REG_PHYCTL_A33,
>  	.dedicated_clocks = true,
> -	.enable_pmu_unk1 = true,
>  	.phy0_dual_route = true,
>  	.missing_phys = BIT(1) | BIT(2),
>  };