On 22-10-21, 07:26, Peter Geis wrote: > On Fri, Oct 22, 2021 at 6:51 AM Vinod Koul <vkoul@xxxxxxxxxx> wrote: > > On 13-10-21, 18:19, Yifeng Zhao wrote: > > > +#define RK3568_T22_PHYREG6 (0x6 << 2) > > > +#define T22_PHYREG6_TX_RTERM_MASK GENMASK(7, 4) > > > +#define T22_PHYREG6_TX_RTERM_SHIFT 4 > > > +#define T22_PHYREG6_TX_RTERM_50OHM 0x8 > > > +#define T22_PHYREG6_RX_RTERM_MASK GENMASK(3, 0) > > > +#define T22_PHYREG6_RX_RTERM_SHIFT 0 > > > +#define T22_PHYREG6_RX_RTERM_44OHM 0xF > > > + > > > +#define RK3568_T22_PHYREG7 (0x7 << 2) > > > > Pls use GENMASK for these? > > > > > +#define T22_PHYREG7_SSC_EN BIT(4) > > > + > > > +#define RK3568_T22_PHYREG10 (0xA << 2) > > > +#define T22_PHYREG10_SU_TRIM_0_7 0xF0 > > > + > > > +#define RK3568_T22_PHYREG11 (0xB << 2) > > > +#define T22_PHYREG11_PLL_LPF_ADJ 0x4 > > > + > > > +#define RK3568_T22_PHYREG12 (0xC << 2) > > > +#define T22_PHYREG12_RESISTER_MASK GENMASK(5, 4) > > > +#define T22_PHYREG12_RESISTER_SHIFT 0x4 > > > > bitfield.h has nice helpers which can extract/program values and avoid > > one to define these shifts > > They aren't values, they are registers. Yes! > This is a remnant from the downstream driver's attempt at obfuscating > the register it's touching. > Please define these correctly. The point of bitfield.h is one defines register bit fields using BIT/GENMASK, no need to define SHIFT etc and use the helpers to extract/program values and avoid defining these shifts etc! -- ~Vinod
