linux-kernel - Re: [PATCH 05/11] phy: rockchip: inno-usb2: Add support for RK3528

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <ff1765b0-ad78-4bdb-94fa-c58456358781@kwiboo.se>
Date: Sun, 19 Oct 2025 16:44:03 +0200
From: Jonas Karlman <jonas@...boo.se>
To: Vinod Koul <vkoul@...nel.org>
Cc: Heiko Stuebner <heiko@...ech.de>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.org>, Kishon Vijay Abraham I <kishon@...nel.org>,
 Yao Zi <ziyao@...root.org>, Chukun Pan <amadeus@....edu.cn>,
 linux-rockchip@...ts.infradead.org, devicetree@...r.kernel.org,
 linux-phy@...ts.infradead.org, linux-arm-kernel@...ts.infradead.org,
 linux-kernel@...r.kernel.org, Jianwei Zheng <jianwei.zheng@...k-chips.com>
Subject: Re: [PATCH 05/11] phy: rockchip: inno-usb2: Add support for RK3528

Hi Vinod,

On 8/12/2025 5:49 PM, Vinod Koul wrote:
> On 23-07-25, 12:23, Jonas Karlman wrote:
>> From: Jianwei Zheng <jianwei.zheng@...k-chips.com>
> 
> Please do not send encrypted emails over public mail lists
> 
>>
>> The RK3528 has a single USB2PHY with a otg and host port.
>>
>> Add support for the RK3528 variant of USB2PHY.
>>
>> PHY tuning for RK3528:
>>
>> - Turn off differential receiver in suspend mode to save power
>>   consumption.
>>
>> - Set HS eye-height to 400mV instead of default 450mV.
>>
>> - Choose the Tx fs/ls data as linestate from TX driver for otg port
>>   which uses dwc3 controller to improve fs/ls devices compatibility with
>>   long cables.
>>
>> This is based on vendor kernel linux-stan-6.1-rkr5 tag.
>>
>> Signed-off-by: Jianwei Zheng <jianwei.zheng@...k-chips.com>
>> Signed-off-by: Jonas Karlman <jonas@...boo.se>
>> ---
>>  drivers/phy/rockchip/phy-rockchip-inno-usb2.c | 74 +++++++++++++++++++
>>  1 file changed, 74 insertions(+)
>>
>> diff --git a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
>> index cd1a02b990ef..b8950d9f9e97 100644
>> --- a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
>> +++ b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
>> @@ -1507,6 +1507,28 @@ static int rk3128_usb2phy_tuning(struct rockchip_usb2phy *rphy)
>>  				BIT(2) << BIT_WRITEABLE_SHIFT | 0);
>>  }
>>  
>> +static int rk3528_usb2phy_tuning(struct rockchip_usb2phy *rphy)
>> +{
>> +	int ret = 0;
> 
> Superfluous init

I would think it make it much clearer to init to zero because we are
bitwise OR return values from the following calls.

> 
>> +
>> +	/* Turn off otg port differential receiver in suspend mode */
>> +	ret |= regmap_write(rphy->phy_base, 0x30, BIT(18) | 0x0000);
>> +
>> +	/* Turn off host port differential receiver in suspend mode */
>> +	ret |= regmap_write(rphy->phy_base, 0x430, BIT(18) | 0x0000);
>> +
>> +	/* Set otg port HS eye height to 400mv (default is 450mv) */
>> +	ret |= regmap_write(rphy->phy_base, 0x30, GENMASK(22, 20) | 0x0000);
>> +
>> +	/* Set host port HS eye height to 400mv (default is 450mv) */
>> +	ret |= regmap_write(rphy->phy_base, 0x430, GENMASK(22, 20) | 0x0000);
>> +
>> +	/* Choose the Tx fs/ls data as linestate from TX driver for otg port */
>> +	ret |= regmap_write(rphy->phy_base, 0x94, GENMASK(22, 19) | 0x0018);
> 
> No error checking?

Error checking is happening in the function calling this internal ops.

Similar to the other tuning ops in this driver it does not matter
exactly what fails, the use of |= is there to help simplify the function
and for the caller to just error out if any of the write fails.

I will shortly send a v2 without changing anything in this patch.

Regards,
Jonas