Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues: - [Medium] Unconditional `dev_err` logging in `mtk_dp_phy_get_one_cal_para()` causes log spam for expected fallback cases (missing eFuses) and probe deferrals. Pre-existing issues: - [High] Unconditional dereference of `dev->platform_data` in legacy probe before validating it is non-NULL. -- --- Patch [8]: [PATCH v3 08/12] phy: phy-mtk-dp: Add support for digital and analog calibration --- commit 637b7b00f751ea110c4b395e4e951eecc2ab535e Author: AngeloGioacchino Del Regno <[email protected]> phy: phy-mtk-dp: Add support for digital and analog calibration Add support for reading the calibration values from eFuse: if present, write those - otherwise, rely on the defaults from SoC-specific data. This also adds support for writing the calibration values for the analog part of the PHY. > diff --git a/drivers/phy/mediatek/phy-mtk-dp.c > b/drivers/phy/mediatek/phy-mtk-dp.c > index c87e9fa710a40..f59a415b401c5 100644 > --- a/drivers/phy/mediatek/phy-mtk-dp.c > +++ b/drivers/phy/mediatek/phy-mtk-dp.c [ ... ] > @@ -422,6 +541,111 @@ static const struct phy_ops mtk_dp_phy_dev_ops = { > .owner = THIS_MODULE, > }; > > +static void mtk_dp_phy_get_default_cal_data(struct mtk_dp_phy *dp_phy) [ ... ] > +static int mtk_dp_phy_get_one_cal_para(struct device *dev, const char *name, > u8 max_val) > +{ > + u8 buf_byte; > + u16 buf; > + int ret; > + > + /* > + * All of the calibrations are always max 8 bits long, but some may > + * be split between two different 8-bits cells: handle this corner > + * case by retrying reading as u16. > + */ > + ret = nvmem_cell_read_u8(dev, name, &buf_byte); > + if (ret) > + ret = nvmem_cell_read_u16(dev, name, &buf); > + else > + buf = buf_byte; > + > + if (ret) { > + dev_err(dev, "Cannot get calibration data for %s: %d\n", name, > ret); > + return ret; > + }; [Severity: Medium] Will this unconditionally log an error when calibration data is intentionally omitted or when the nvmem provider returns -EPROBE_DEFER? If the calibration data is missing, nvmem_cell_read_u8() returns -ENOENT, which is later handled gracefully by the caller by falling back to defaults. However, printing dev_err() here will cause log spam and false-positive errors in dmesg. Could this be silenced for -ENOENT and -EPROBE_DEFER cases? > + > + if (buf == 0) { [ ... ] > @@ -453,6 +677,13 @@ static int mtk_dp_phy_legacy_probe(struct > platform_device *pdev, struct mtk_dp_p > "Failed to create DP PHY\n"); > > phy_set_drvdata(phy, dp_phy); [Severity: High] This is a pre-existing issue, but earlier in mtk_dp_phy_legacy_probe(), does the code unconditionally dereference dev->platform_data before checking if it is NULL? drivers/phy/mediatek/phy-mtk-dp.c:mtk_dp_phy_legacy_probe() { ... dp_phy->regmap = *(struct regmap **)dev->platform_data; if (!dp_phy->regmap) return dev_err_probe(dev, -EINVAL, "No platform data available\n"); ... } If the platform device is instantiated without platform data, *(struct regmap **)NULL results in an immediate kernel panic, making the subsequent check dead code. > + > + /* > + * Set default calibration data before exposing the PHY. > + * For legacy probe, mtk_dp will set calibrations from eFuse, if found. > + */ > + mtk_dp_phy_get_default_cal_data(dp_phy); > + > ret = phy_create_lookup(phy, "dp", dev_name(dev)); > if (ret) > return ret; -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=8
