Re: [PATCH v2 2/6] phy: realtek: usb2: introduce read and write functions to driver data
From: Rustam Adilov
Date: Tue Mar 31 2026 - 12:12:00 EST
Hello,
On 2026-03-30 21:19, Vladimir Oltean wrote:
> On Fri, Mar 27, 2026 at 09:06:34PM +0500, Rustam Adilov wrote:
>> +static inline u32 phy_read(void __iomem *reg)
>> +{
>> + return readl(reg);
>> +}
>> +
>> +static inline u32 phy_read_le(void __iomem *reg)
>> +{
>> + return le32_to_cpu(readl(reg));
>> +}
>> +
>> +static inline void phy_write(u32 val, void __iomem *reg)
>> +{
>> + writel(val, reg);
>> +}
>> +
>> +static inline void phy_write_le(u32 val, void __iomem *reg)
>> +{
>> + writel(cpu_to_le32(val), reg);
>> +}
>
> Please don't name driver-level functions phy_read() and phy_write().
> That will collide with networking API functions of the same name and
> will make grep-based code searching more difficult.
I can change it to something like "rtk_phy_read" or "usb2phy_read" then.
> Also, have you looked at regmap? It has native support for endianness;
> it supports regmap_field_read()/regmap_field_write() for abstracting
> registers which may be found at different places for different HW;
> it offers regmap_read_poll_timeout() so you don't have to pass the
> function pointer to utmi_wait_register(). It seems the result would be a
> bit more elegant.
In fact, I did not because it would involve in way more refactoring for patch
series that is supposed to simply add RTL9607C support. And unfortunately,
the regmap is not going to the solve the issue, which i will explain in the
later email to your LLM review on readl/writel.