linux-kernel - Re: [PATCH v3 3/5] phy: rockchip: naneng-combphy: Add SoC prefix to register definitions

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <aCv8vRu8gjrvK8wr@pie.lan>
Date: Tue, 20 May 2025 03:53:33 +0000
From: Yao Zi <ziyao@...root.org>
To: "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 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.

> > +
> > +/* 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