Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
AI-Spam-Status: Not processed
Message-ID: <639cbac6bf755dc28f35557dc74c0ed5e9162707.camel@aosc.io>
Subject: Re: [PATCH v2 14/21] phy: sun4i-usb: Rework "pmu_unk1" handling
From:   Icenowy Zheng <icenowy@aosc.io>
To:     =?ISO-8859-1?Q?Andr=E9?= 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>,
        =?ISO-8859-1?Q?Cl=E9ment_P=E9ron?= <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>
Date:   Mon, 14 Dec 2020 12:45:45 +0800
In-Reply-To: <86fd61f1-9bdd-5004-758b-8e21134fadd2@arm.com>
References: <20201211011934.6171-1-andre.przywara@arm.com>
         <20201211011934.6171-15-andre.przywara@arm.com>
         <02cbecddf07dc46f8fb7e0d93b7532e3c239084e.camel@aosc.io>
         <86fd61f1-9bdd-5004-758b-8e21134fadd2@arm.com>
Content-Type: text/plain; charset="UTF-8"
User-Agent: Evolution 3.36.5 
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

在 2020-12-14星期一的 01:35 +0000，André Przywara写道：
> On 13/12/2020 18:24, Icenowy Zheng wrote:
> > 在 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).

In addition, both the BIT(1) in old PHYs and the BIT(3) in new PHYs are
SIDDQ. (In BSP it's a #define inside #if)

Should we add some flag for 28nm PHY?

> 
> Oh, right, somehow I failed to find this in the BSP, I guess I got
> confused over that similar symbol. I will put proper names to it.

In addition, the region at 0x800 is not PMU at all, but some [OE]HCI-
related control registers. The name "PMU" can only cover the first
register here.

We may need to address this.

> 
> Thanks!
> Andre
> 
> > > 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),
> > >  };