On 2023/6/21 20:30, Vinod Koul wrote:
> On 29-05-23, 05:15, Changhuang Liang wrote:
>> +static int stf_dphy_configure(struct phy *phy, union phy_configure_opts *opts)
>> +{
>> + struct stf_dphy *dphy = phy_get_drvdata(phy);
>> + const struct stf_dphy_info *info = dphy->info;
>> + int i;
>> +
>> + for (i = 0; i < ARRAY_SIZE(stf_dphy_init_list); i++)
>> + writel(stf_dphy_init_list[i].val,
>> + dphy->regs + stf_dphy_init_list[i].addr);
>> +
>> + writel(FIELD_PREP(STF_DPHY_DA_CDPHY_R100_CTRL0_2D1C_EFUSE_EN, 1) |
>> + FIELD_PREP(STF_DPHY_DA_CDPHY_R100_CTRL0_2D1C_EFUSE_IN, 0x1b) |
>> + FIELD_PREP(STF_DPHY_DA_CDPHY_R100_CTRL1_2D1C_EFUSE_EN, 1) |
>> + FIELD_PREP(STF_DPHY_DA_CDPHY_R100_CTRL1_2D1C_EFUSE_IN, 0x1b),
>> + dphy->regs + STF_DPHY_APBCFGSAIF_SYSCFG(0));
>> +
>> + writel(FIELD_PREP(STF_DPHY_DATA_BUS16_8, 0) |
>> + FIELD_PREP(STF_DPHY_DEBUG_MODE_SEL, 0x5a),
>> + dphy->regs + STF_DPHY_APBCFGSAIF_SYSCFG(184));
>
> bunch of magic numbers here and previous one..?
>
magic numbers is "1" "0x1b" "0" "0x5a"?
If so, I will move this two registers into stf_dphy_init_list[].
And maybe also have some magic numbers below, I will replace them will macro.
>> +
>> + writel(FIELD_PREP(STF_DPHY_ENABLE_CLK, 1) |
>> + FIELD_PREP(STF_DPHY_ENABLE_CLK1, 1) |
>> + FIELD_PREP(STF_DPHY_ENABLE_LAN0, 1) |
>> + FIELD_PREP(STF_DPHY_ENABLE_LAN1, 1) |
>> + FIELD_PREP(STF_DPHY_ENABLE_LAN2, 1) |
>> + FIELD_PREP(STF_DPHY_ENABLE_LAN3, 1) |
>> + FIELD_PREP(STF_DPHY_GPI_EN, 0) |
>> + FIELD_PREP(STF_DPHY_HS_FREQ_CHANGE_CLK, 0) |
>> + FIELD_PREP(STF_DPHY_HS_FREQ_CHANGE_CLK1, 0) |
>> + FIELD_PREP(STF_DPHY_LANE_SWAP_CLK, info->maps[0]) |
>> + FIELD_PREP(STF_DPHY_LANE_SWAP_CLK1, info->maps[5]) |
>> + FIELD_PREP(STF_DPHY_LANE_SWAP_LAN0, info->maps[1]) |
>> + FIELD_PREP(STF_DPHY_LANE_SWAP_LAN1, info->maps[2]),
>> + dphy->regs + STF_DPHY_APBCFGSAIF_SYSCFG(188));
[...]
>> +
>> + writel(FIELD_PREP(STF_DPHY_PRECOUNTER_IN_LAN3, 7) |
>> + FIELD_PREP(STF_DPHY_RX_1C2C_SEL, 0),
>> + dphy->regs + STF_DPHY_APBCFGSAIF_SYSCFG(200));
>> +
>> + return 0;
>> +}
>> +
>> +static int stf_dphy_power_on(struct phy *phy)
>> +{
>> + struct stf_dphy *dphy = phy_get_drvdata(phy);
>> + int ret;
>> +
>> + pm_runtime_get_sync(dphy->dev);
>
> no error check?
>
I will replace pm_runtime_get_sync with pm_runtime_resume_and_get to handle error:
ret = pm_runtime_resume_and_get(dphy->dev);
if (ret < 0)
return ret;
>> +
>> + ret = regulator_enable(dphy->mipi_0p9);
>> + if (ret)
>> + return ret;
>
> should you not drop the pm reference on error here?
>
I will add pm_runtime_put in this error handle.
Thanks for your comments.
