From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 18C70425CC6 for ; Tue, 7 Jul 2026 14:31:41 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783434703; cv=none; b=N16KHnaM7v81aqCSFPKsjmGxEqIoz6rpOT2gEAJvO6yHDYwv9d3q09P2yOUS71ELlCMhGhV5yI9RN82s+s3B0uzQL0E50sMTmUWtGF5LMPHsWDBYhKetSVtudXNwHZ67weHZoipMsHyiryNKXNthV519Jf7yJ0Z9UabECCSGf+s= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783434703; c=relaxed/simple; bh=Y4vQaZRdXzI/UDHQiKLzKNCubRb8wrWT4BH1FFCDyD0=; h=From:Subject:To:Cc:In-Reply-To:References:Content-Type:Date: Message-Id; b=aGsTqeqqR9SJ9/FaUfKlyBTg50af6YXKqyr6B6tIMW7NYETRACfcKyQDrw5SZz6Ej1ljhyewuzyqoJM72tfOzJVscXtQAXRHK2Che8DDyvwUvSs1R5sIPoLUTwEGrllfxerfBAeQ80b9II9TUB2r3CkqaPhPSAashtxVlsbwrb0= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=DqCm9TTS; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="DqCm9TTS" Received: by smtp.kernel.org (Postfix) with ESMTPSA id BED6D1F000E9; Tue, 7 Jul 2026 14:31:40 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1783434701; bh=sXLtoFh+CUx/tyfknuCX6sDSL7lW5AVQhCz46fBe5Z8=; h=From:Subject:Reply-To:To:Cc:In-Reply-To:References:Date; b=DqCm9TTSRnjxgTDqvZLTzlbf+jBcUzL1dbxvpHlLC55R01QgjDSm6re/6WyQYSbY8 7sPl7sjWOOCqHzNdvZJPl3QKzuwSu0G6dz4O3HFu9U4q0U8H2A8SLra8KN2ETroNVE sGKhiGuDnb9yW3K5QoDoiykk7uS2lQi5XPIMAqHzCBhPh5gHkbfZst050xznKivXIe /fOZSsJGtEmw/7AJpyESlVekLUhO2L9UeBdzHZgPY2jPUAd2HUj5DbdT+mgfw5gdlG CiICXWYkdyeW8KznU8TW4P0HI0sLp0Xh0LFrn0TwDgpKg1I1srqpOpAzRhDSMzVEZa rwC7hqPxbTMnQ== From: sashiko-bot@kernel.org Subject: Re: [PATCH v2 08/12] phy: phy-mtk-dp: Add support for digital and analog calibration Reply-To: sashiko-reviews@lists.linux.dev To: "AngeloGioacchino Del Regno" Cc: dri-devel@lists.freedesktop.org, linux-phy@lists.infradead.org, vkoul@kernel.org, robh@kernel.org, conor+dt@kernel.org, neil.armstrong@linaro.org, olteanv@gmail.com, devicetree@vger.kernel.org In-Reply-To: <20260707141931.191172-9-angelogioacchino.delregno@collabora.com> References: <20260707141931.191172-1-angelogioacchino.delregno@collabora.com> <20260707141931.191172-9-angelogioacchino.delregno@collabora.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Date: Tue, 07 Jul 2026 14:31:40 +0000 Message-Id: <20260707143140.BED6D1F000E9@smtp.kernel.org> Precedence: bulk X-Mailing-List: devicetree@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Thank you for your contribution! Sashiko AI review found 3 potential issue(= s) to consider: New issues: - [High] The DP PHY is published and exposed to concurrent consumers before= its calibration data is initialized in the legacy probe path. - [Medium] Unconditional `dev_err` logs an error for missing optional NVMEM= cells and probe deferrals in `mtk_dp_phy_get_one_cal_para`. Pre-existing issues: - [High] Dereferencing `dev->platform_data` without checking for NULL in `m= tk_dp_phy_legacy_probe`. -- --- Patch [8]: [PATCH v2 08/12] phy: phy-mtk-dp: Add support for digital an= d analog calibration --- commit 0db0b57c522a5f7eb7d179aeee49708015901046 Author: AngeloGioacchino Del Regno phy: phy-mtk-dp: Add support for digital and analog calibration This commit adds support for reading DP PHY calibration values from eFuse via nvmem. If the values are present, they are applied; otherwise, the driver relies on default calibration data. Analog calibration writing support is also introduced. > diff --git a/drivers/phy/mediatek/phy-mtk-dp.c b/drivers/phy/mediatek/phy= -mtk-dp.c > --- a/drivers/phy/mediatek/phy-mtk-dp.c > +++ b/drivers/phy/mediatek/phy-mtk-dp.c [ ... ] > +static int mtk_dp_phy_get_one_cal_para(struct device *dev, const char *n= ame, 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 =3D nvmem_cell_read_u8(dev, name, &buf_byte); > + if (ret) > + ret =3D nvmem_cell_read_u16(dev, name, &buf); > + else > + buf =3D 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 a hard error if the NVMEM cells are absent or if the provider returns -EPROBE_DEFER? Since nvmem_cell_read_u8() returns -ENOENT when optional cells are missing in the device tree, could this cause spurious error messages in the kernel log on normal boots? It might also spam the logs during standard probe deferrals. [ ... ] > @@ -444,6 +671,8 @@ static int mtk_dp_phy_legacy_probe(struct platform_de= vice *pdev, struct mtk_dp_p > if (ret) > return ret; > =20 > + mtk_dp_phy_get_default_cal_data(dp_phy); > + > return 0; > } [Severity: High] Is it safe to publish the PHY to consumers before the calibration defaults are initialized?=20 Earlier in mtk_dp_phy_legacy_probe(), phy_create_lookup() exposes the PHY. If a consumer driver probes concurrently and acquires the PHY right after that lookup registration, could it call phy_init() while the analog parameter registers are still zero-initialized by devm_kzalloc(), rather than having these defaults? [Severity: High] This is a pre-existing issue, but can dev->platform_data be NULL earlier in mtk_dp_phy_legacy_probe()? Looking at the start of the function: dp_phy->regmap =3D *(struct regmap **)dev->platform_data; if (!dp_phy->regmap) return dev_err_probe(dev, -EINVAL, "No platform data available\n"); If the driver is bound to a device without platform data (e.g. manually via sysfs bind), it looks like dereferencing dev->platform_data to read the regmap will cause a kernel panic before it reaches the NULL check. --=20 Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260707141931.1911= 72-1-angelogioacchino.delregno@collabora.com?part=3D8