Re: [PATCH v3 12/12] phy: phy-mtk-dp: Add support for MT8196 eDP PHY

[AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>]

On 7/7/26 18:05, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Missing NULL check for the result of `device_get_match_data(dev)` before dereferencing it.
> --
> 
> --- Patch [12]: [PATCH v3 12/12] phy: phy-mtk-dp: Add support for MT8196 eDP PHY ---
> commit 171cadff965a41b7aba8d81fbb60622f2a8e1a97
> Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> 
> phy: phy-mtk-dp: Add support for MT8196 eDP PHY
> 
> The MT8196 SoC features an updated PHY IP compared to the older
> ones, and there is one that is specific to Embedded DisplayPort.
> This commit adds support for the eDP PHY found in the MediaTek MT8196 SoC
> and all of its variants.
> 
>> diff --git a/drivers/phy/mediatek/phy-mtk-dp.c b/drivers/phy/mediatek/phy-mtk-dp.c
>> index f5aadc5fdc42c..5decc29c77cb3 100644
>> --- a/drivers/phy/mediatek/phy-mtk-dp.c
>> +++ b/drivers/phy/mediatek/phy-mtk-dp.c
> 
> [ ... ]
> 
>> @@ -805,8 +838,39 @@ static const struct mtk_dp_phy_pdata mt8195_dp_phy_data = {
>>   	},
>>   };
>>   
>> +static const struct mtk_dp_phy_pdata mt8196_edp_phy_data = {
>> +	.off_ana_glb = 0x400,
> 
> [ ... ]
> 
>> +	.driving_params = (const u32[]) {
>> +		[0] = 0,
>> +		[1] = 0,
>> +		[2] = 0,
>> +		[3] = MT8196_DRIVING_PARAM_3_DEFAULT,
>> +		[4] = MT8196_DRIVING_PARAM_4_DEFAULT,
>> +		[5] = MT8196_DRIVING_PARAM_5_DEFAULT,
>> +		[6] = MT8196_DRIVING_PARAM_6_DEFAULT,
>> +		[7] = MT8196_DRIVING_PARAM_7_DEFAULT,
>> +		[8] = MT8196_DRIVING_PARAM_8_DEFAULT
>> +	},
>> +};
>> +
>>   static const struct of_device_id mtk_dp_phy_of_match[] = {
>>   	{ .compatible = "mediatek,mt8195-dp-phy", .data = &mt8195_dp_phy_data },
>> +	{ .compatible = "mediatek,mt8196-edp-phy", .data = &mt8196_edp_phy_data },
>>   	{ /* sentinel */ }
>>   };
> 
> [Severity: High]
> If the driver is forcefully bound to a device via the sysfs bind interface,
> and the device has an of_node but its compatible string does not match any
> entry in the driver's of_match_table, device_get_match_data(dev) will return
> NULL.
> 
> This NULL pointer is stored in dp_phy->pdata:
> 
> drivers/phy/mediatek/phy-mtk-dp.c:mtk_dp_phy_probe() {
>      ...
> 	dp_phy->pdata = device_get_match_data(dev);
> 
> 	if (IS_REACHABLE(CONFIG_NVMEM)) {
> 		ret = mtk_dp_phy_get_calibration_data(dp_phy);
>      ...
> }
> 
> And is unconditionally dereferenced here:
> 
> drivers/phy/mediatek/phy-mtk-dp.c:mtk_dp_phy_get_default_cal_data() {
>      ...
> 	const struct mtk_dp_phy_pdata *pdata = dp_phy->pdata;
> 	int i;
> 
> 	dp_phy->ana_bias_r = pdata->ana_bias_r;
>      ...
> }
> 
> Should device_get_match_data() be checked for NULL before continuing in
> mtk_dp_phy_probe() to prevent a potential kernel panic?
> 

Oh just shut up, Sashiko! :-)

Too many paranoid checks proposed, are we trying to slow down Linux by a factor
of 1000 with all the defensive coding?

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy