Re: [PATCH v2 2/2] phy: cadence-torrent: Add PCIe + XAUI multilink configuration for 100MHz refclk

[Siddharth Vadapalli]

On 05/01/26 3:27 AM, Andrew Lunn wrote:
> On Sun, Jan 04, 2026 at 05:14:18PM +0530, Siddharth Vadapalli wrote:
> > From: Swapnil Jakhade <sjakhade@xxxxxxxxxxx>
> >
> > Add register sequences for PCIe + XAUI multilink configuration for
> > 100MHz reference clock.
> >
> > Signed-off-by: Swapnil Jakhade <sjakhade@xxxxxxxxxxx>
> > Signed-off-by: Siddharth Vadapalli <s-vadapalli@xxxxxx>
> > ---
> >
> > v1 of this patch is at:
> > https://lore.kernel.org/r/20251224054905.763399-3-s-vadapalli@xxxxxx/
> > No changes since v1.
> >
> > Regards,
> > Siddharth.
> >
> >   drivers/phy/cadence/phy-cadence-torrent.c | 143 ++++++++++++++++++++--
> >   1 file changed, 136 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/phy/cadence/phy-cadence-torrent.c b/drivers/phy/cadence/phy-cadence-torrent.c
> > index 37fa4bad6bd7..f0d870886cca 100644
> > --- a/drivers/phy/cadence/phy-cadence-torrent.c
> > +++ b/drivers/phy/cadence/phy-cadence-torrent.c
> > @@ -300,6 +300,7 @@ enum cdns_torrent_phy_type {
> >   	TYPE_USB,
> >   	TYPE_USXGMII,
> >   	TYPE_PCIE_ML,
> > +	TYPE_XAUI,
> >   };
> >   
> >   enum cdns_torrent_ref_clk {
> > @@ -320,14 +321,14 @@ enum cdns_torrent_ssc_mode {
> >   /* Unique key id for vals table entry
> >    * REFCLK0_RATE | REFCLK1_RATE | LINK0_TYPE | LINK1_TYPE | SSC_TYPE
> >    */
> > -#define REFCLK0_SHIFT	12
> > -#define REFCLK0_MASK	GENMASK(14, 12)
> > -#define REFCLK1_SHIFT	9
> > -#define REFCLK1_MASK	GENMASK(11, 9)
> > -#define LINK0_SHIFT	6
> > -#define LINK0_MASK	GENMASK(8, 6)
> > +#define REFCLK0_SHIFT	15
> > +#define REFCLK0_MASK	GENMASK(18, 15)
> > +#define REFCLK1_SHIFT	11
> > +#define REFCLK1_MASK	GENMASK(14, 11)
> > +#define LINK0_SHIFT	7
> > +#define LINK0_MASK	GENMASK(10, 7)
>
> Why do these change? It would be good to add an explanation to the
> commit message about this, because it is not obvious why these need to
> change.

The 'key id' is supposed to be a unique value defined as the following 
Bitwise OR:
REFCLK0_RATE | REFCLK1_RATE | LINK0_TYPE | LINK1_TYPE | SSC_TYPE

The 'LINK_TYPE' parameter corresponds to the protocol (PHY Type). In the 
phy-cadence-torrent.c driver, the following enum describes the supported 
'LINK_TYPE's:
enum cdns_torrent_phy_type {
	TYPE_NONE,
	TYPE_DP,
	TYPE_PCIE,
	TYPE_SGMII,
	TYPE_QSGMII,
	TYPE_USB,
	TYPE_USXGMII,
	TYPE_PCIE_ML,
};
The last entry which is TYPE_PCIE_ML has the value of '7' (TYPE_NONE is 
'0'). Therefore, 'LINK_MASK' happened to be a 3-bit mask:
	#define LINK0_MASK	GENMASK(8, 6)
	#define LINK1_MASK	GENMASK(5, 3)
With the addition of TYPE_XAUI in the current patch, 'LINK_MASK' has to 
be extended to a 4-bit mask.

The 'REFCLK_RATE' parameter corresponds to the reference clock rates 
defined which are again described by the following enum in the driver:
enum cdns_torrent_ref_clk {
	CLK_19_2_MHZ,
	CLK_25_MHZ,
	CLK_100_MHZ,
	CLK_156_25_MHZ,
	CLK_ANY,
};
The last entry which is CLK_ANY has the value of '4' (CLK_19_2_MHZ is 
'0'). Although the existing macros defined as:
	#define REFCLK0_MASK	GENMASK(14, 12)
	#define REFCLK1_MASK	GENMASK(11, 9)
are 3-bit masks and can fit values until '7', for the sake of 
future-proofing the driver (the SERDES Hardware supports more reference 
clocks which are not yet enabled in the driver), 'REFCLK_MASK' has been 
extended to a 4-bit mask as well.

The shifts associated with the masks have been updated accordingly.

I shall include a summary of the above in the commit message of the v3 
series.

Thank you for reviewing the patch and providing feedback.

Regards,
Siddharth.