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

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

New issues:
- [Medium] The rtl9607_phy_cfg structure omits the .check_efuse flag, causing eFuse parameters to be ignored.
- [Medium] The page0 register configuration stores address 0xf3 at the wrong index, corrupting debugfs output.

Pre-existing issues:
- [High] Unchecked return value from of_iomap() leads to a NULL pointer dereference.
- [High] Pointer arithmetic on void __iomem * causes unaligned device register accesses.
- [High] Signed char integer promotion corrupts register writes on architectures with signed chars (e.g., MIPS).
- [High] Silent failure when do_rtk_phy_init() fails for multi-port SoCs.
--

--- Patch [5]: [PATCH v7 5/6] phy: realtek: usb2: add support for RTL9607C USB2 PHY ---

commit 6a5b4e2235314d4730ccd147be12148b19afc984
Author: Rustam Adilov [off-list ref]

    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.

[ ... ]

[Severity: High]
This is a pre-existing issue, but does writing 0x95 here corrupt the register
on architectures like MIPS?

rtk_phy_write() takes data as a signed char:

drivers/phy/realtek/phy-rtk-usb2.c:rtk_phy_write() {
...
	reg_desc->write((u32)data << shift_bits,
...
}

On architectures where char is signed by default, a value of 0x95 is
interpreted as a negative number (-107). When cast to u32, it undergoes sign
extension to 0xFFFFFF95, and after shifting, it can clobber the upper bits
of the shared reg_wrap_vstatus register.

+		   [4] = {0xe4, 0x6a},
+		  [12] = {0xf3, 0x31}, },

[Severity: Medium]
Will this map to the correct debugfs output?

Address 0xf3 mathematically maps to array index 11 via the strict mapping
formula in page_addr_to_array_index(). Because it is placed at index 12
here, when rtk_usb2_parameter_show() computes the target read address
dynamically, it will read from 0xf4 and display it alongside the expected
0xf3 data.

[Severity: Medium]
Is the .check_efuse flag missing here?

Without .check_efuse = true, the eFuse parameters like check_efuse_version
and efuse_dc_driving_rate specified in this structure appear to be ignored
by get_phy_data_by_efuse(), leaving the hardware uncalibrated.

[ ... ]

[Severity: High]
This isn't a bug introduced by this patch, but looking at the existing
parse_phy_data() function:

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

Does this code dereference a NULL pointer during device probe if the device
tree lacks the required reg properties?

The return values from of_iomap() are assigned and subsequently dereferenced
without any validation.

[Severity: High]
This is a pre-existing issue, but does adding index to the of_iomap() result
cause unaligned device register accesses?

Looking at the existing parse_phy_data() function:

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

Since of_iomap() returns void __iomem *, GCC treats the pointer arithmetic
as 1-byte elements. For multi-port PHYs where index > 0, this adds exactly
1 byte to the base address, which will trigger alignment exceptions during
subsequent 32-bit writel() calls on architectures like ARM64.

[Severity: High]
This is a pre-existing issue, but does the initialization loop mask errors
for multi-port SoCs?

Looking at the existing rtk_phy_init() function:

drivers/phy/realtek/phy-rtk-usb2.c:rtk_phy_init() {
...
	for (i = 0; i < rtk_phy->num_phy; i++)
		ret = do_rtk_phy_init(rtk_phy, i);
...
}

If do_rtk_phy_init() fails for any port except the last one, ret is
overwritten by the result of the subsequent PHY's initialization, silently
ignoring earlier failures while incorrectly reporting probe success.

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

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