Messages in this thread | | | Subject | Re: [PATCH net-next v3 14/47] net: phy: aquantia: Add support for rate adaptation | From | Sean Anderson <> | Date | Sat, 16 Jul 2022 18:45:01 -0400 |
| |
On 7/16/22 2:38 PM, Andrew Lunn wrote: >> +#define VEND1_GLOBAL_CFG_10M 0x0310 >> +#define VEND1_GLOBAL_CFG_100M 0x031b >> +#define VEND1_GLOBAL_CFG_1G 0x031c >> +#define VEND1_GLOBAL_CFG_2_5G 0x031d >> +#define VEND1_GLOBAL_CFG_5G 0x031e >> +#define VEND1_GLOBAL_CFG_10G 0x031f > > I completely read this wrong the first time... The common meaning of > #defines line this is > > VEND1_GLOBAL_CFG_ is the register and what follows indicates some bits > in the register. > > However, this is not true here, these are all registers. Maybe add > _REG to the end? It makes them different to other defines for > registers, but if i parsed it wrong, probably other will as well?
How about a comment like
/* The following registers all have similar layouts; first the registers... */ #define VEND1_GLOBAL_CFG_10M 0x0310 ... /* ... and now the fields */ #define VEND1_GLOBAL_CFG_RATE_ADAPT GENMASK(8, 7)
>> static int aqr107_read_rate(struct phy_device *phydev) >> { >> int val; >> + u32 config_reg; > > Revere Christmass tree. config_reg should be first.
OK
--Sean
| |