Re: [PATCH v8 5/6] phy: realtek: usb2: add support for RTL9607C USB2 PHY

[Rustam Adilov <adilov@disroot.org>]

On 2026-10-12 18:17, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] phy: missing NULL check for of_iomap leads to dereference
> --
> 
> commit ab09dd4def3f2215950726cb780ea645be439c3a
> Author: Rustam Adilov <adilov@disroot.org>
> 
> phy: realtek: usb2: add support for RTL9607C USB2 PHY
> 
> Add support for the usb2 phy of RTL9607C series based SoCs. Add the macros
> and phy config struct for rtl9607. RTL9607C requires to clear a force host
> disconnect bit in the specific register before proceeding with phy
> parameter writes.
> 
>> diff --git a/drivers/phy/realtek/phy-rtk-usb2.c b/drivers/phy/realtek/phy-rtk-usb2.c
>> index 2ddad7be73534..64076938c821d 100644
>> --- a/drivers/phy/realtek/phy-rtk-usb2.c
>> +++ b/drivers/phy/realtek/phy-rtk-usb2.c
> 
> [ ... ]
> 
>> @@ -600,6 +617,23 @@ static int do_rtk_phy_init(struct rtk_phy *rtk_phy, int index)
>>  		goto do_toggle;
>>  	}
>>  
>> +	if (phy_cfg->force_host_disconnect) {
>> +		const struct phy_reg_desc *reg_desc = phy_reg->desc;
>> +		void __iomem *vstatus = phy_reg->reg_wrap_vstatus;
>> +		u32 temp;
>> +
>> +		temp = reg_desc->read(vstatus + PHY_9607_FORCE_DISCONNECT_REG);
> 
> [Severity: High]
> Is it possible for vstatus to be NULL here?
> 
> In parse_phy_data(), the driver calls of_iomap() and assigns the result without
> checking for a NULL return value:
> 
> parse_phy_data() {
> 	...
> 	phy_parameter->phy_reg.reg_wrap_vstatus = of_iomap(np, 0);
> 	...
> }
> 
> If the device tree node lacks the memory region or mapping fails, vstatus
> will be NULL. Unconditionally dereferencing it here via reg_desc->read()
> could cause a null pointer dereference.
> 
>> +
>> +		/* disable force-host-disconnect */
>> +		temp &= ~PHY_9607_FORCE_DISCONNECT_BIT;
>> +
>> +		reg_desc->write(temp, vstatus + PHY_9607_FORCE_DISCONNECT_REG);
>> +		temp = reg_desc->read(vstatus + PHY_9607_FORCE_DISCONNECT_REG);
>> +
>> +		/* allow IP to startup */
>> +		usleep_range(10000, 11000);
>> +	}
>> +
>>  	/* Set page 0 */
>>  	phy_data_page = phy_cfg->page0;
>>  	rtk_phy_set_page(phy_reg, 0);

Why is this not tagged with pre-exesiting issue just like with the previous
review on patch 1..

Not related to this patch series at all.

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