17.12.2020 16:33, Thierry Reding пишет:
>> + /* PHY won't resume if reset is asserted */
>> + if (phy->wakeup_enabled)
>> + goto chrg_cfg0;
>>
>> val = readl_relaxed(base + USB_SUSP_CTRL);
>> val |= UTMIP_RESET;
>> writel_relaxed(val, base + USB_SUSP_CTRL);
>>
>> +chrg_cfg0:
> I found this diffcult to read until I realized that it was basically
> just the equivalent of this:
>
> if (!phy->wakeup_enabled) {
> val = readl_relaxed(base + USB_SUSP_CTRL);
> val |= UTMIP_RESET;
> writel_relaxed(val, base + USB_SUSP_CTRL);
> }
>
>> val = readl_relaxed(base + UTMIP_BAT_CHRG_CFG0);
>> val |= UTMIP_PD_CHRG;
>> writel_relaxed(val, base + UTMIP_BAT_CHRG_CFG0);
>>
>> + if (phy->wakeup_enabled)
>> + goto xcvr_cfg1;
>> +
>> val = readl_relaxed(base + UTMIP_XCVR_CFG0);
>> val |= UTMIP_FORCE_PD_POWERDOWN | UTMIP_FORCE_PD2_POWERDOWN |
>> UTMIP_FORCE_PDZI_POWERDOWN;
>> writel_relaxed(val, base + UTMIP_XCVR_CFG0);
>>
>> +xcvr_cfg1:
> Similarly, I think this is more readable as:
>
> if (!phy->wakeup_enabled) {
> val = readl_relaxed(base + UTMIP_XCVR_CFG0);
> val |= UTMIP_FORCE_PD_POWERDOWN | UTMIP_FORCE_PD2_POWERDOWN |
> UTMIP_FORCE_PDZI_POWERDOWN;
> writel_relaxed(val, base + UTMIP_XCVR_CFG0);
> }
>
>> val = readl_relaxed(base + UTMIP_XCVR_CFG1);
>> val |= UTMIP_FORCE_PDDISC_POWERDOWN | UTMIP_FORCE_PDCHRP_POWERDOWN |
>> UTMIP_FORCE_PDDR_POWERDOWN;
>> writel_relaxed(val, base + UTMIP_XCVR_CFG1);
>>
>> + if (phy->wakeup_enabled) {
> Which then also matches the style of this conditional here.
I'll change it in v3, thanks.
