Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] Inconsistent use of device pointers for error logging in `mtk_dp_phy_power_off` and `mtk_dp_phy_reset`. --
--- Patch [6]: [PATCH v3 06/12] phy: phy-mtk-dp: Support set_lanes in configure and properly cleanup --- commit cab309557ac4fcb462c5b8978a8cd660acab1ec1 Author: AngeloGioacchino Del Regno <[email protected]> phy: phy-mtk-dp: Support set_lanes in configure and properly cleanup This commit adds support for enabling a specifically requested number of lanes in the .configure() callback. It also updates the power off and reset callbacks to disable all lanes for proper hardware cleanup. > --- a/drivers/phy/mediatek/phy-mtk-dp.c > +++ b/drivers/phy/mediatek/phy-mtk-dp.c [ ... ] > @@ -263,31 +281,68 @@ static int mtk_dp_phy_power_on(struct phy *phy) [ ... ] > static int mtk_dp_phy_power_off(struct phy *phy) > { > struct mtk_dp_phy *dp_phy = phy_get_drvdata(phy); > const struct mtk_dp_phy_pdata *pdata = dp_phy->pdata; > const u8 *regs_dig = pdata->regs_dig_glb; > const u8 *regs_ana = pdata->regs_ana_glb; > - int ret_cktx, ret_aux; > + int ret_cktx, ret_aux, ret; > > ret_cktx = regmap_set_bits(dp_phy->regmap, > pdata->off_ana_glb + > regs_ana[DP_PHYA_GLB_FORCE_CTRL_1], > CKM_CKTX0_EN_FORCE_MODE); > + if (ret_cktx) > + dev_err(&phy->dev, "Could not disable CKTX0: %d\n", ret_cktx); [ ... ] > + ret = mtk_dp_phy_disable_all_lanes(dp_phy); > + if (ret) { > + dev_err(dp_phy->dev, "Could not disable lanes for poweroff!\n"); [Severity: Low] Should this use &phy->dev instead of dp_phy->dev for error logging? The adjacent error handlers for CKTX and AUX in this same function log errors using &phy->dev. Mixing device pointers might lead to inconsistent device names in the kernel log. > + return ret; > } [ ... ] > @@ -307,6 +363,11 @@ static int mtk_dp_phy_reset(struct phy *phy) [ ... ] > /* Setting bits means go out of reset */ > regmap_set_bits(dp_phy->regmap, pdata->off_dig_glb + reg_rst, > DP_GLB_SW_RST_PHYD); > > + /* Disable all lanes and continue reset even if this fails, but notify > */ > + ret = mtk_dp_phy_disable_all_lanes(dp_phy); > + if (ret) > + dev_err(dp_phy->dev, "Could not disable lanes during reset!\n"); [Severity: Low] Similarly here, would it be better to use &phy->dev for consistency with the rest of the API implementation? > + > return 0; > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=6
