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 _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel