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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing error handling for of_iomap() leads to NULL pointer dereference
- [High] Incorrect pointer arithmetic on void __iomem * causes unaligned memory accesses
- [High] Resource leak of I/O memory mappings
--

commit 45e50610a9cf3e4d1ba9824c0450199938805d5b
Author: Rustam Adilov <adilov@disroot.org>

phy: realtek: usb2: add support for RTL9607C USB2 PHY

This commit adds support for the USB2 PHY of RTL9607C-based SoCs. It includes
macros, a PHY config structure, and logic to clear a "force host disconnect"
bit before writing parameters.

> diff --git a/drivers/phy/realtek/phy-rtk-usb2.c b/drivers/phy/realtek/phy-rtk-usb2.c
> index 16c5fc3191dec..69f0f5279b5ef 100644
> --- a/drivers/phy/realtek/phy-rtk-usb2.c
> +++ b/drivers/phy/realtek/phy-rtk-usb2.c
[ ... ]
> @@ -600,6 +617,19 @@ static int do_rtk_phy_init(struct rtk_phy *rtk_phy, int index)
>  		goto do_toggle;
>  	}
>  
> +	if (phy_cfg->force_host_disconnect) {
> +		/* disable force-host-disconnect */
> +		void __iomem *vstatus = phy_reg->reg_wrap_vstatus;
> +		u32 temp;
> +
> +		temp = phy_reg->read(vstatus + PHY_9607_FORCE_DISCONNECT_REG);

[Severity: High]
This isn't a bug introduced by this patch, but missing error handling for
of_iomap() in parse_phy_data() could lead to a NULL pointer dereference here.

drivers/phy/realtek/phy-rtk-usb2.c:parse_phy_data() {
    ...
    phy_parameter->phy_reg.reg_wrap_vstatus = of_iomap(np, 0);
    phy_parameter->phy_reg.reg_gusb2phyacc0 = of_iomap(np, 1) + index;
    ...
}

If the mapping fails, phy_reg->reg_wrap_vstatus will be NULL.

Does this new code risk dereferencing a NULL vstatus pointer if the device
tree is invalid or memory is exhausted?

[Severity: High]
This is a pre-existing issue, but does the driver leak the I/O memory mapped
by of_iomap()?

Since devm_of_iomap() is not used and there are no corresponding iounmap()
calls anywhere in the driver, could this exhaust I/O memory upon probe
failure or driver unbinding?

> +
> +		temp &= ~PHY_9607_FORCE_DISCONNECT_BIT;
> +		phy_reg->write(temp, vstatus + PHY_9607_FORCE_DISCONNECT_REG);
> +
> +		usleep_range(10000, 11000);
> +	}
> +
>  	/* Set page 0 */
>  	phy_data_page = phy_cfg->page0;
>  	rtk_phy_set_page(phy_reg, 0);
[ ... ]
> @@ -1374,6 +1404,33 @@ static const struct phy_cfg rtd1315e_phy_cfg = {
>  	.write = rtk_usb2phy_write,
>  };
>  
> +static const struct phy_cfg rtl9607_phy_cfg = {
> +	.page0_size = MAX_USB_PHY_PAGE0_DATA_SIZE,
> +	.page0 = { [0] = {0xe0, 0x95},
> +		   [4] = {0xe4, 0x6a},
> +		  [12] = {0xf3, 0x31}, },
> +	.page1_size = MAX_USB_PHY_PAGE1_DATA_SIZE,
> +	.page1 = { [0] = {0xe0, 0x26}, },
> +	.page2_size = MAX_USB_PHY_PAGE2_DATA_SIZE,
> +	.page2 = { [7] = {0xe7, 0x33}, },
> +	.num_phy = 1,

[Severity: High]
This isn't a bug introduced by this patch, but does the pointer arithmetic
incorrectly misalign the base pointer when num_phy > 1?

drivers/phy/realtek/phy-rtk-usb2.c:parse_phy_data() {
    ...
    for (index = 0; index < rtk_phy->num_phy; index++) {
        ...
        phy_parameter->phy_reg.reg_gusb2phyacc0 = of_iomap(np, 1) + index;
    ...
}

Since of_iomap() returns a void __iomem *, adding index increments the
pointer by exactly index bytes.

Could this cause an unaligned memory access exception (leading to a kernel
panic) or fail to map the correct register index from the device tree?

> +	.check_efuse_version = CHECK_EFUSE_V2,
> +	.efuse_dc_driving_rate = EFUS_USB_DC_CAL_RATE,
> +	.dc_driving_mask = 0x1f,
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260520175728.720877-1-adilov@disroot.org?part=5