[PATCH v4 2/4] phy: ralink: Add PHY driver for MT7621 PCIe PHY
Vinod Koul
vkoul at kernel.org
Thu Nov 19 05:30:59 UTC 2020
On 31-10-20, 13:22, Sergio Paracuellos wrote:
> +#define RG_PE1_PIPE_REG 0x02c
> +#define RG_PE1_PIPE_RST BIT(12)
> +#define RG_PE1_PIPE_CMD_FRC BIT(4)
> +
> +#define RG_P0_TO_P1_WIDTH 0x100
> +#define RG_PE1_H_LCDDS_REG 0x49c
> +#define RG_PE1_H_LCDDS_PCW GENMASK(30, 0)
> +#define RG_PE1_H_LCDDS_PCW_VAL(x) ((0x7fffffff & (x)) << 0)
Pls use FIELD_{GET|PREP} instead of coding like this, you already
defined the mask, use it to set and get the reg field ;)
> +
> +#define RG_PE1_FRC_H_XTAL_REG 0x400
> +#define RG_PE1_FRC_H_XTAL_TYPE BIT(8)
> +#define RG_PE1_H_XTAL_TYPE GENMASK(10, 9)
> +#define RG_PE1_H_XTAL_TYPE_VAL(x) ((0x3 & (x)) << 9)
> +
> +#define RG_PE1_FRC_PHY_REG 0x000
> +#define RG_PE1_FRC_PHY_EN BIT(4)
> +#define RG_PE1_PHY_EN BIT(5)
> +
> +#define RG_PE1_H_PLL_REG 0x490
> +#define RG_PE1_H_PLL_BC GENMASK(23, 22)
> +#define RG_PE1_H_PLL_BC_VAL(x) ((0x3 & (x)) << 22)
> +#define RG_PE1_H_PLL_BP GENMASK(21, 18)
> +#define RG_PE1_H_PLL_BP_VAL(x) ((0xf & (x)) << 18)
> +#define RG_PE1_H_PLL_IR GENMASK(15, 12)
> +#define RG_PE1_H_PLL_IR_VAL(x) ((0xf & (x)) << 12)
> +#define RG_PE1_H_PLL_IC GENMASK(11, 8)
> +#define RG_PE1_H_PLL_IC_VAL(x) ((0xf & (x)) << 8)
> +#define RG_PE1_H_PLL_PREDIV GENMASK(7, 6)
> +#define RG_PE1_H_PLL_PREDIV_VAL(x) ((0x3 & (x)) << 6)
> +#define RG_PE1_PLL_DIVEN GENMASK(3, 1)
> +#define RG_PE1_PLL_DIVEN_VAL(x) ((0x7 & (x)) << 1)
> +
> +#define RG_PE1_H_PLL_FBKSEL_REG 0x4bc
> +#define RG_PE1_H_PLL_FBKSEL GENMASK(5, 4)
> +#define RG_PE1_H_PLL_FBKSEL_VAL(x) ((0x3 & (x)) << 4)
> +
> +#define RG_PE1_H_LCDDS_SSC_PRD_REG 0x4a4
> +#define RG_PE1_H_LCDDS_SSC_PRD GENMASK(15, 0)
> +#define RG_PE1_H_LCDDS_SSC_PRD_VAL(x) ((0xffff & (x)) << 0)
> +
> +#define RG_PE1_H_LCDDS_SSC_DELTA_REG 0x4a8
> +#define RG_PE1_H_LCDDS_SSC_DELTA GENMASK(11, 0)
> +#define RG_PE1_H_LCDDS_SSC_DELTA_VAL(x) ((0xfff & (x)) << 0)
> +#define RG_PE1_H_LCDDS_SSC_DELTA1 GENMASK(27, 16)
> +#define RG_PE1_H_LCDDS_SSC_DELTA1_VAL(x) ((0xff & (x)) << 16)
> +
> +#define RG_PE1_LCDDS_CLK_PH_INV_REG 0x4a0
> +#define RG_PE1_LCDDS_CLK_PH_INV BIT(5)
> +
> +#define RG_PE1_H_PLL_BR_REG 0x4ac
> +#define RG_PE1_H_PLL_BR GENMASK(18, 16)
> +#define RG_PE1_H_PLL_BR_VAL(x) ((0x7 & (x)) << 16)
> +
> +#define RG_PE1_MSTCKDIV_REG 0x414
> +#define RG_PE1_MSTCKDIV GENMASK(7, 6)
> +#define RG_PE1_MSTCKDIV_VAL(x) ((0x3 & (x)) << 6)
> +
> +#define RG_PE1_FRC_MSTCKDIV BIT(5)
> +
> +#define XTAL_MODE_SEL_SHIFT 6
Bonus you dont need to define shifts if you use stuff defined in
bitfield.h
> +struct mt7621_pci_phy {
> + struct device *dev;
> + struct regmap *regmap;
> + struct phy *phy;
> + void __iomem *port_base;
> + bool has_dual_port;
> + bool bypass_pipe_rst;
> +};
> +
> +static inline u32 phy_read(struct mt7621_pci_phy *phy, u32 reg)
> +{
> + u32 val;
> +
> + regmap_read(phy->regmap, reg, &val);
> +
> + return val;
> +}
> +
> +static inline void phy_write(struct mt7621_pci_phy *phy, u32 val, u32 reg)
> +{
> + regmap_write(phy->regmap, reg, val);
Why not use regmap_ calls directly and avoid the dummy wrappers..?
> +}
> +
> +static inline void mt7621_phy_rmw(struct mt7621_pci_phy *phy,
> + u32 reg, u32 clr, u32 set)
> +{
> + u32 val = phy_read(phy, reg);
> +
> + val &= ~clr;
> + val |= set;
> + phy_write(phy, val, reg);
why not use regmap_update_bits() instead
> +static void mt7621_set_phy_for_ssc(struct mt7621_pci_phy *phy)
> +{
> + struct device *dev = phy->dev;
> + u32 xtal_mode;
> +
> + xtal_mode = (rt_sysc_r32(SYSC_REG_SYSTEM_CONFIG0)
> + >> XTAL_MODE_SEL_SHIFT) & XTAL_MODE_SEL_MASK;
> +
> + /* Set PCIe Port PHY to disable SSC */
> + /* Debug Xtal Type */
> + mt7621_phy_rmw(phy, RG_PE1_FRC_H_XTAL_REG,
> + RG_PE1_FRC_H_XTAL_TYPE | RG_PE1_H_XTAL_TYPE,
> + RG_PE1_FRC_H_XTAL_TYPE | RG_PE1_H_XTAL_TYPE_VAL(0x00));
> +
> + /* disable port */
> + mt7621_phy_rmw(phy, RG_PE1_FRC_PHY_REG,
> + RG_PE1_PHY_EN, RG_PE1_FRC_PHY_EN);
> +
> + if (phy->has_dual_port) {
> + mt7621_phy_rmw(phy, RG_PE1_FRC_PHY_REG + RG_P0_TO_P1_WIDTH,
> + RG_PE1_PHY_EN, RG_PE1_FRC_PHY_EN);
> + }
> +
> + if (xtal_mode <= 5 && xtal_mode >= 3) { /* 40MHz Xtal */
> + /* Set Pre-divider ratio (for host mode) */
> + mt7621_phy_rmw(phy, RG_PE1_H_PLL_REG,
> + RG_PE1_H_PLL_PREDIV,
> + RG_PE1_H_PLL_PREDIV_VAL(0x01));
> + dev_info(dev, "Xtal is 40MHz\n");
> + } else if (xtal_mode >= 6) { /* 25MHz Xal */
> + mt7621_phy_rmw(phy, RG_PE1_H_PLL_REG,
> + RG_PE1_H_PLL_PREDIV,
> + RG_PE1_H_PLL_PREDIV_VAL(0x00));
> + /* Select feedback clock */
> + mt7621_phy_rmw(phy, RG_PE1_H_PLL_FBKSEL_REG,
> + RG_PE1_H_PLL_FBKSEL,
> + RG_PE1_H_PLL_FBKSEL_VAL(0x01));
> + /* DDS NCPO PCW (for host mode) */
> + mt7621_phy_rmw(phy, RG_PE1_H_LCDDS_SSC_PRD_REG,
> + RG_PE1_H_LCDDS_SSC_PRD,
> + RG_PE1_H_LCDDS_SSC_PRD_VAL(0x18000000));
> + /* DDS SSC dither period control */
> + mt7621_phy_rmw(phy, RG_PE1_H_LCDDS_SSC_PRD_REG,
> + RG_PE1_H_LCDDS_SSC_PRD,
> + RG_PE1_H_LCDDS_SSC_PRD_VAL(0x18d));
> + /* DDS SSC dither amplitude control */
> + mt7621_phy_rmw(phy, RG_PE1_H_LCDDS_SSC_DELTA_REG,
> + RG_PE1_H_LCDDS_SSC_DELTA |
> + RG_PE1_H_LCDDS_SSC_DELTA1,
> + RG_PE1_H_LCDDS_SSC_DELTA_VAL(0x4a) |
> + RG_PE1_H_LCDDS_SSC_DELTA1_VAL(0x4a));
> + dev_info(dev, "Xtal is 25MHz\n");
Debug please
> + } else { /* 20MHz Xtal */
> + mt7621_phy_rmw(phy, RG_PE1_H_PLL_REG,
> + RG_PE1_H_PLL_PREDIV,
> + RG_PE1_H_PLL_PREDIV_VAL(0x00));
> +
> + dev_info(dev, "Xtal is 20MHz\n");
ditto
--
~Vinod
More information about the devel
mailing list