| Message ID | 20240619124622.2798613-1-csokas.bence@prolan.hu (mailing list archive) |
|---|---|
| State | Rejected |
| Headers | show |
| Series | [v2,resub,1/2] net: include: mii: Refactor: Define LPA_* in terms of ADVERTISE_* | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
On Wed, Jun 19, 2024 at 02:46:22PM +0200, Csókás, Bence wrote: > Ethernet specification mandates that these bits will be equal. > To reduce the amount of magix hex'es in the code, just define > them in terms of each other. I have a quick email exchange with other PHY maintainers, and we agree. We will reject these changes, they are just churn and bring no real benefit. NACK Andrew
Hi On 6/20/24 21:07, Andrew Lunn wrote: > On Wed, Jun 19, 2024 at 02:46:22PM +0200, Csókás, Bence wrote: >> Ethernet specification mandates that these bits will be equal. >> To reduce the amount of magix hex'es in the code, just define >> them in terms of each other. > > I have a quick email exchange with other PHY maintainers, and we > agree. We will reject these changes, they are just churn and bring no > real benefit. > > NACK > > Andrew > The benefit is that I don't have to constantly convert between "n-th bit set" (which is how virtually all datasheets, specifications, documentation etc. represent MII bits) and these hex values. In most places in the kernel, register bits are already represented with BIT() et al., so why not here? Bence
On Fri, Jun 21, 2024 at 09:48:23AM +0200, Csókás Bence wrote: > Hi > > On 6/20/24 21:07, Andrew Lunn wrote: > > On Wed, Jun 19, 2024 at 02:46:22PM +0200, Csókás, Bence wrote: > > > Ethernet specification mandates that these bits will be equal. > > > To reduce the amount of magix hex'es in the code, just define > > > them in terms of each other. > > > > I have a quick email exchange with other PHY maintainers, and we > > agree. We will reject these changes, they are just churn and bring no > > real benefit. > > > > NACK > > > > Andrew > > > > The benefit is that I don't have to constantly convert between "n-th bit > set" (which is how virtually all datasheets, specifications, documentation > etc. represent MII bits) and these hex values. In most places in the kernel, > register bits are already represented with BIT() et al., so why not here? These are user API files, you can't use BIT() here (BIT() isn't defined for userspace header files.) Next, these are 'int's not 'longs' so using BIT() or _BITUL() could cause warnings - plus it changes the type of these definitions not only for kernel space but also user space. Thus, it's an API change. So no, we're not making changes to make this "more readable" at the expense of breaking the kernel's UAPI. Just get used to working with hex numbers like most of us had to do before BIT() was added to the kernel... it's not difficult, each hex digit is after all four binary bits. It's not like it's decimal.
diff --git a/include/uapi/linux/mii.h b/include/uapi/linux/mii.h index 39f7c44baf53..33e1b0c717e4 100644 --- a/include/uapi/linux/mii.h +++ b/include/uapi/linux/mii.h @@ -93,22 +93,22 @@ ADVERTISE_100HALF | ADVERTISE_100FULL) /* Link partner ability register. */ -#define LPA_SLCT 0x001f /* Same as advertise selector */ -#define LPA_10HALF 0x0020 /* Can do 10mbps half-duplex */ -#define LPA_1000XFULL 0x0020 /* Can do 1000BASE-X full-duplex */ -#define LPA_10FULL 0x0040 /* Can do 10mbps full-duplex */ -#define LPA_1000XHALF 0x0040 /* Can do 1000BASE-X half-duplex */ -#define LPA_100HALF 0x0080 /* Can do 100mbps half-duplex */ -#define LPA_1000XPAUSE 0x0080 /* Can do 1000BASE-X pause */ -#define LPA_100FULL 0x0100 /* Can do 100mbps full-duplex */ -#define LPA_1000XPAUSE_ASYM 0x0100 /* Can do 1000BASE-X pause asym*/ -#define LPA_100BASE4 0x0200 /* Can do 100mbps 4k packets */ -#define LPA_PAUSE_CAP 0x0400 /* Can pause */ -#define LPA_PAUSE_ASYM 0x0800 /* Can pause asymetrically */ -#define LPA_RESV 0x1000 /* Unused... */ -#define LPA_RFAULT 0x2000 /* Link partner faulted */ -#define LPA_LPACK 0x4000 /* Link partner acked us */ -#define LPA_NPAGE 0x8000 /* Next page bit */ +#define LPA_SLCT ADVERTISE_SLCT /* Same as advertise selector */ +#define LPA_10HALF ADVERTISE_10HALF +#define LPA_1000XFULL ADVERTISE_1000XFULL +#define LPA_10FULL ADVERTISE_10FULL +#define LPA_1000XHALF ADVERTISE_1000XHALF +#define LPA_100HALF ADVERTISE_100HALF +#define LPA_1000XPAUSE ADVERTISE_1000XPAUSE +#define LPA_100FULL ADVERTISE_100FULL +#define LPA_1000XPAUSE_ASYM ADVERTISE_1000XPSE_ASYM +#define LPA_100BASE4 ADVERTISE_100BASE4 +#define LPA_PAUSE_CAP ADVERTISE_PAUSE_CAP +#define LPA_PAUSE_ASYM ADVERTISE_PAUSE_ASYM +#define LPA_RESV ADVERTISE_RESV +#define LPA_RFAULT ADVERTISE_RFAULT /* Link partner faulted */ +#define LPA_LPACK ADVERTISE_LPACK /* Link partner acked us */ +#define LPA_NPAGE ADVERTISE_NPAGE #define LPA_DUPLEX (LPA_10FULL | LPA_100FULL) #define LPA_100 (LPA_100FULL | LPA_100HALF | LPA_100BASE4)
Ethernet specification mandates that these bits will be equal. To reduce the amount of magix hex'es in the code, just define them in terms of each other. Cc: trivial@kernel.org Signed-off-by: Csókás, Bence <csokas.bence@prolan.hu> --- include/uapi/linux/mii.h | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-)