[linux-sunxi] Re: [PATCH v2 14/21] phy: sun4i-usb: Rework "pmu_unk1" handling
在 2020-12-14星期一的 01:35 +，André Przywara写道：
> On 13/12/2020 18:24, Icenowy Zheng wrote:
> > 在 2020-12-11星期五的 01:19 +，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
> > > ---
> > > 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 =
[linux-sunxi] Re: [PATCH v2 14/21] phy: sun4i-usb: Rework "pmu_unk1" handling
On 13/12/2020 18:24, Icenowy Zheng wrote:
> 在 2020-12-11星期五的 01:19 +，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).
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.
Thanks!
Andre
>>
>> Generalise the existing code by allowing configs to specify a bitmask
>> of bits to clear.
>>
>> Signed-off-by: Andre Przywara
>> ---
>> 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,
>> -
[linux-sunxi] Re: [PATCH v2 14/21] phy: sun4i-usb: Rework "pmu_unk1" handling
在 2020-12-11星期五的 01:19 +，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
> ---
> 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
