[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ab2692a5-e428-40bf-8071-cad6ae96dc3b@linaro.org>
Date: Tue, 20 May 2025 09:24:36 +0200
From: neil.armstrong@...aro.org
To: Yao Zi <ziyao@...root.org>, Diederik de Haas <didi.debian@...ow.org>,
Vinod Koul <vkoul@...nel.org>, Kishon Vijay Abraham I <kishon@...nel.org>,
Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, Heiko Stuebner <heiko@...ech.de>,
Frank Wang <frank.wang@...k-chips.com>, Andy Yan <andy.yan@...k-chips.com>,
Cristian Ciocaltea <cristian.ciocaltea@...labora.com>,
Detlev Casanova <detlev.casanova@...labora.com>,
Shresth Prasad <shresthprasad7@...il.com>, Chukun Pan <amadeus@....edu.cn>,
Jonas Karlman <jonas@...boo.se>
Cc: linux-phy@...ts.infradead.org, devicetree@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, linux-rockchip@...ts.infradead.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 3/5] phy: rockchip: naneng-combphy: Add SoC prefix to
register definitions
On 20/05/2025 05:53, Yao Zi wrote:
> On Mon, May 19, 2025 at 09:26:05PM +0200, Diederik de Haas wrote:
>> On Mon May 19, 2025 at 6:16 PM CEST, Yao Zi wrote:
>>> All supported variants of naneng-combphy follow a register layout
>>> similar to the RK3568 variant with some exceptions of SoC-specific
>>> registers.
>>>
>>> Add RK3568 prefix for the common set of registers and the corresponding
>>> SoC prefix for SoC-specific registers, making usage of definitions clear
>>> and preparing for future COMBPHY variants with a different register
>>> layout.
>>>
>>> Signed-off-by: Yao Zi <ziyao@...root.org>
>>> Reviewed-by: Heiko Stuebner <heiko@...ech.de>
>>> ---
>>> .../rockchip/phy-rockchip-naneng-combphy.c | 560 +++++++++---------
>>> 1 file changed, 288 insertions(+), 272 deletions(-)
>>>
>>> diff --git a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
>>> index ce91fb1d5167..1d1c7723584b 100644
>>> --- a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
>>> +++ b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
>>> @@ -21,78 +21,80 @@
>>> #define REF_CLOCK_100MHz (100 * HZ_PER_MHZ)
>>>
>>> /* COMBO PHY REG */
>>> <snip>
>>> -#define PHYREG33_PLL_KVCO_VALUE_RK3576 4
>>> +#define RK3568_PHYREG6 0x14
>>> +#define RK3568_PHYREG6_PLL_DIV_MASK GENMASK(7, 6)
>>> +#define RK3568_PHYREG6_PLL_DIV_SHIFT 6
>>> +#define RK3568_PHYREG6_PLL_DIV_2 1
>>> +
>>> +#define RK3568_PHYREG7 0x18
>>> +#define RK3568_PHYREG7_TX_RTERM_MASK GENMASK(7, 4)
>>> +#define RK3568_PHYREG7_TX_RTERM_SHIFT 4
>>> +#define RK3568_PHYREG7_TX_RTERM_50OHM 8
>>> +#define RK3568_PHYREG7_RX_RTERM_MASK GENMASK(3, 0)
>>> +#define RK3568_PHYREG7_RX_RTERM_SHIFT 0
>>> +#define RK3568_PHYREG7_RX_RTERM_44OHM 15
>>> +
>>> +#define RK3568_PHYREG8 0x1C
>>> +#define RK3568_PHYREG8_SSC_EN BIT(4)
>>> +
>>> +#define RK3568_PHYREG11 0x28
>>> +#define RK3568_PHYREG11_SU_TRIM_0_7 0xF0
>>> +
>>> +#define RK3568_PHYREG12 0x2C
>>> +#define RK3568_PHYREG12_PLL_LPF_ADJ_VALUE 4
>>> +
>>> +#define RK3568_PHYREG13 0x30
>>> +#define RK3568_PHYREG13_RESISTER_MASK GENMASK(5, 4)
>>> +#define RK3568_PHYREG13_RESISTER_SHIFT 0x4
>>> +#define RK3568_PHYREG13_RESISTER_HIGH_Z 3
>>> +#define RK3568_PHYREG13_CKRCV_AMP0 BIT(7)
>>> +
>>> +#define RK3568_PHYREG14 0x34
>>> +#define RK3568_PHYREG14_CKRCV_AMP1 BIT(0)
>>> +
>>> +#define RK3568_PHYREG15 0x38
>>> +#define RK3568_PHYREG15_CTLE_EN BIT(0)
>>> +#define RK3568_PHYREG15_SSC_CNT_MASK GENMASK(7, 6)
>>> +#define RK3568_PHYREG15_SSC_CNT_SHIFT 6
>>> +#define RK3568_PHYREG15_SSC_CNT_VALUE 1
>>> +
>>> +#define RK3568_PHYREG16 0x3C
>>> +#define RK3568_PHYREG16_SSC_CNT_VALUE 0x5f
>>> +
>>> +#define RK3568_PHYREG18 0x44
>>> +#define RK3568_PHYREG18_PLL_LOOP 0x32
>>> +
>>> +#define RK3568_PHYREG32 0x7C
>>> +#define RK3568_PHYREG32_SSC_MASK GENMASK(7, 4)
>>> +#define RK3568_PHYREG32_SSC_DIR_MASK GENMASK(5, 4)
>>> +#define RK3568_PHYREG32_SSC_DIR_SHIFT 4
>>> +#define RK3568_PHYREG32_SSC_UPWARD 0
>>> +#define RK3568_PHYREG32_SSC_DOWNWARD 1
>>> +#define RK3568_PHYREG32_SSC_OFFSET_MASK GENMASK(7, 6)
>>> +#define RK3568_PHYREG32_SSC_OFFSET_SHIFT 6
>>> +#define RK3568_PHYREG32_SSC_OFFSET_500PPM 1
>>> +
>>> +#define RK3568_PHYREG33 0x80
>>> +#define RK3568_PHYREG33_PLL_KVCO_MASK GENMASK(4, 2)
>>> +#define RK3568_PHYREG33_PLL_KVCO_SHIFT 2
>>> +#define RK3568_PHYREG33_PLL_KVCO_VALUE 2
>>> +#define RK3576_PHYREG33_PLL_KVCO_VALUE 4
>>> +
>>> +/* RK3588 COMBO PHY registers */
>>> +#define RK3588_PHYREG27 0x6C
>>> +#define RK3588_PHYREG27_RX_TRIM 0x4C
>>
>> Would it be better if RK3588_PHYREG* comes after RK3576_PHYREG*?
>>
>> Cheers,
>> Diederik
>
> It's intended to keep RK3576 definitions below RK3588 ones. The RK3576
> driver makes use of a register introduced for RK3588 variant
> (RK3588_PHYREG27). Since similar reusing doesn't happen reversely, I
> consider the design of RK3576 a superset of the RK3588 one, and put
> RK3576 definitions later in the file.
Sound logic, RK3576 was announced after RK3588, thus the order makes sense.
Add my:
Reviewed-by: Neil Armstrong <neil.armstrong@...aro.org>
>
>>> +
>>> +/* RK3576 COMBO PHY registers */
>>> +#define RK3576_PHYREG10 0x24
>>> +#define RK3576_PHYREG10_SSC_PCM_MASK GENMASK(3, 0)
>>> +#define RK3576_PHYREG10_SSC_PCM_3500PPM 7
>>> +
>>> +#define RK3576_PHYREG17 0x40
>>> +
>>> +#define RK3576_PHYREG21 0x50
>>> +#define RK3576_PHYREG21_RX_SQUELCH_VAL 0x0D
>>> +
>>> +#define RK3576_PHYREG30 0x74
>>>
>>> struct rockchip_combphy_priv;
>>> <snip>
>
>
> Thanks,
> Yao Zi
>
Powered by blists - more mailing lists