Re: [PATCH phy v2 2/6] phy: marvell: phy-mvebu-a3700-comphy: Add native kernel implementation

From: "Pali Rohár" <pali@kernel.org>
To: Vinod Koul <vkoul@kernel.org>
Cc: "Marek Behún" <kabel@kernel.org>,
	linux-phy@lists.infradead.org,
	"Kishon Vijay Abraham I" <kishon@ti.com>,
	"Miquel Raynal" <miquel.raynal@bootlin.com>,
	"Gregory CLEMENT" <gregory.clement@bootlin.com>
Subject: Re: [PATCH phy v2 2/6] phy: marvell: phy-mvebu-a3700-comphy: Add native kernel implementation
Date: Thu, 23 Dec 2021 14:21:02 +0100	[thread overview]
Message-ID: <20211223132102.lgne4vsdjeqqudwc@pali> (raw)
In-Reply-To: <YcRuVCdXztTTrJs7@matsya>

On Thursday 23 December 2021 18:10:52 Vinod Koul wrote:
> On 08-12-21, 03:40, Marek Behún wrote:
> > From: Pali Rohár <pali@kernel.org>
> 
> > +/* COMPHY registers */
> > +#define COMPHY_POWER_PLL_CTRL		0x01
> > +#define PU_IVREF_BIT			BIT(15)
> > +#define PU_PLL_BIT			BIT(14)
> > +#define PU_RX_BIT			BIT(13)
> > +#define PU_TX_BIT			BIT(12)
> > +#define PU_TX_INTP_BIT			BIT(11)
> > +#define PU_DFE_BIT			BIT(10)
> > +#define RESET_DTL_RX_BIT		BIT(9)
> > +#define PLL_LOCK_BIT			BIT(8)
> > +#define REF_FREF_SEL_MASK		GENMASK(4, 0)
> > +#define REF_FREF_SEL_SERDES_25MHZ	(0x1 << 0)
> > +#define REF_FREF_SEL_SERDES_40MHZ	(0x3 << 0)
> > +#define REF_FREF_SEL_SERDES_50MHZ	(0x4 << 0)
> > +#define REF_FREF_SEL_PCIE_USB3_25MHZ	(0x2 << 0)
> > +#define REF_FREF_SEL_PCIE_USB3_40MHZ	(0x3 << 0)
> > +#define COMPHY_MODE_MASK		GENMASK(7, 5)
> > +#define COMPHY_MODE_SATA		(0x0 << 5)
> > +#define COMPHY_MODE_PCIE		(0x3 << 5)
> > +#define COMPHY_MODE_SERDES		(0x4 << 5)
> > +#define COMPHY_MODE_USB3		(0x5 << 5)
> 
> Any reason why these are not using GENMASK. I guess documentation would
> define these as BIT x-y right?

Hello Vinod! I'm not sure to which macro refers your question as all
*_MASK defines are now using GENMASK() macros.

Definitions which do not have mask _MASK are not masks, but rather
particular values for corresponding _MASK constant.

So, for example: REF_FREF_SEL_MASK is 5 bit mask for fref_sel and
fref_sel can contain exactly one of the following values:
REF_FREF_SEL_SERDES_25MHZ, REF_FREF_SEL_SERDES_40MHZ,
REF_FREF_SEL_SERDES_50MHZ, REF_FREF_SEL_PCIE_USB3_25MHZ,
REF_FREF_SEL_PCIE_USB3_40MHZ. So you want to set serdes ref freq to
50 MHz you need to clear bits [4:0] (macro REF_FREF_SEL_MASK) and then
set to value 0x4 (macro REF_FREF_SEL_SERDES_50MHZ) to bits [4:0].

It is clear now? Or did you mean something different?

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy