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

Pali Rohár
2021-12-23T13:21:10Z

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?