VLAN_CTRL should be treated as a 64-bit register. GENMASK and BIT
macros use unsigned long as the underlying type, which will result in a
build error on architectures where sizeof(long) == 32.
Replace them with unsigned long long variants.
Signed-off-by: David Yang <mmyangfl@gmail.com>
---
drivers/net/dsa/yt921x.h | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/net/dsa/yt921x.h b/drivers/net/dsa/yt921x.h
index 3e85d90826fb..85d995cdb7c5 100644
--- a/drivers/net/dsa/yt921x.h
+++ b/drivers/net/dsa/yt921x.h
@@ -328,23 +328,23 @@
#define YT921X_FDB_HW_FLUSH_ON_LINKDOWN BIT(0)
#define YT921X_VLANn_CTRL(vlan) (0x188000 + 8 * (vlan))
-#define YT921X_VLAN_CTRL_UNTAG_PORTS_M GENMASK(50, 40)
+#define YT921X_VLAN_CTRL_UNTAG_PORTS_M GENMASK_ULL(50, 40)
#define YT921X_VLAN_CTRL_UNTAG_PORTS(x) FIELD_PREP(YT921X_VLAN_CTRL_UNTAG_PORTS_M, (x))
-#define YT921X_VLAN_CTRL_UNTAG_PORTn(port) BIT((port) + 40)
-#define YT921X_VLAN_CTRL_STP_ID_M GENMASK(39, 36)
+#define YT921X_VLAN_CTRL_UNTAG_PORTn(port) BIT_ULL((port) + 40)
+#define YT921X_VLAN_CTRL_STP_ID_M GENMASK_ULL(39, 36)
#define YT921X_VLAN_CTRL_STP_ID(x) FIELD_PREP(YT921X_VLAN_CTRL_STP_ID_M, (x))
-#define YT921X_VLAN_CTRL_SVLAN_EN BIT(35)
-#define YT921X_VLAN_CTRL_FID_M GENMASK(34, 23)
+#define YT921X_VLAN_CTRL_SVLAN_EN BIT_ULL(35)
+#define YT921X_VLAN_CTRL_FID_M GENMASK_ULL(34, 23)
#define YT921X_VLAN_CTRL_FID(x) FIELD_PREP(YT921X_VLAN_CTRL_FID_M, (x))
-#define YT921X_VLAN_CTRL_LEARN_DIS BIT(22)
-#define YT921X_VLAN_CTRL_INT_PRI_EN BIT(21)
-#define YT921X_VLAN_CTRL_INT_PRI_M GENMASK(20, 18)
-#define YT921X_VLAN_CTRL_PORTS_M GENMASK(17, 7)
+#define YT921X_VLAN_CTRL_LEARN_DIS BIT_ULL(22)
+#define YT921X_VLAN_CTRL_INT_PRI_EN BIT_ULL(21)
+#define YT921X_VLAN_CTRL_INT_PRI_M GENMASK_ULL(20, 18)
+#define YT921X_VLAN_CTRL_PORTS_M GENMASK_ULL(17, 7)
#define YT921X_VLAN_CTRL_PORTS(x) FIELD_PREP(YT921X_VLAN_CTRL_PORTS_M, (x))
-#define YT921X_VLAN_CTRL_PORTn(port) BIT((port) + 7)
-#define YT921X_VLAN_CTRL_BYPASS_1X_AC BIT(6)
-#define YT921X_VLAN_CTRL_METER_EN BIT(5)
-#define YT921X_VLAN_CTRL_METER_ID_M GENMASK(4, 0)
+#define YT921X_VLAN_CTRL_PORTn(port) BIT_ULL((port) + 7)
+#define YT921X_VLAN_CTRL_BYPASS_1X_AC BIT_ULL(6)
+#define YT921X_VLAN_CTRL_METER_EN BIT_ULL(5)
+#define YT921X_VLAN_CTRL_METER_ID_M GENMASK_ULL(4, 0)
#define YT921X_TPID_IGRn(x) (0x210000 + 4 * (x)) /* [0, 3] */
#define YT921X_TPID_IGR_TPID_M GENMASK(15, 0)
--
2.51.0On Wed, 26 Nov 2025 17:32:34 +0800 David Yang <mmyangfl@gmail.com> wrote: > VLAN_CTRL should be treated as a 64-bit register. GENMASK and BIT > macros use unsigned long as the underlying type, which will result in a > build error on architectures where sizeof(long) == 4. I suspect GENMASK() should generate u32 or u64 depending on the value of a constant 'high bit'. I found code elsewhere that doesn't really want FIELD_PREP() to generate a 64bit value. There are actually a lot of dubious uses of 'long' throughout the kernel that break on 32bit. (Actually pretty much all of them!) David
On Fri, Nov 28, 2025 at 10:51:41AM +0000, david laight wrote:
> On Wed, 26 Nov 2025 17:32:34 +0800
> David Yang <mmyangfl@gmail.com> wrote:
>
> > VLAN_CTRL should be treated as a 64-bit register. GENMASK and BIT
> > macros use unsigned long as the underlying type, which will result in a
> > build error on architectures where sizeof(long) == 4.
>
> I suspect GENMASK() should generate u32 or u64 depending on the value
> of a constant 'high bit'.
I suggest checking before making such statements to save embarrasment.
The above is incorrect.
#define GENMASK_TYPE(t, h, l) \
((t)(GENMASK_INPUT_CHECK(h, l) + \
(type_max(t) << (l) & \
type_max(t) >> (BITS_PER_TYPE(t) - 1 - (h)))))
#define GENMASK(h, l) GENMASK_TYPE(unsigned long, h, l)
#define GENMASK_ULL(h, l) GENMASK_TYPE(unsigned long long, h, l)
#define GENMASK_U8(h, l) GENMASK_TYPE(u8, h, l)
#define GENMASK_U16(h, l) GENMASK_TYPE(u16, h, l)
#define GENMASK_U32(h, l) GENMASK_TYPE(u32, h, l)
#define GENMASK_U64(h, l) GENMASK_TYPE(u64, h, l)
#define GENMASK_U128(h, l) GENMASK_TYPE(u128, h, l)
Note that GENMASK(33, 15) will fail to build on 32-bit systems.
> I found code elsewhere that doesn't really want FIELD_PREP() to
> generate a 64bit value.
>
> There are actually a lot of dubious uses of 'long' throughout
> the kernel that break on 32bit.
> (Actually pretty much all of them!)
If you're referring to the use of GENMASK() with bitfields larger
than 32-bits, then as can be seen from the above, the code wouldn't
even compile and our CI systems would be screaming about it. They
aren't, so I think your statement here is also wrong.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
On Fri, 28 Nov 2025 11:43:17 +0000
"Russell King (Oracle)" <linux@armlinux.org.uk> wrote:
> On Fri, Nov 28, 2025 at 10:51:41AM +0000, david laight wrote:
> > On Wed, 26 Nov 2025 17:32:34 +0800
> > David Yang <mmyangfl@gmail.com> wrote:
> >
> > > VLAN_CTRL should be treated as a 64-bit register. GENMASK and BIT
> > > macros use unsigned long as the underlying type, which will result in a
> > > build error on architectures where sizeof(long) == 4.
> >
> > I suspect GENMASK() should generate u32 or u64 depending on the value
> > of a constant 'high bit'.
>
> I suggest checking before making such statements to save embarrasment.
> The above is incorrect.
Is was a suggestion about what it would be a good idea for it to do,
not a statement about what it actually does.
I know GENMASK() generates 'unsigned long'.
The problem is that (as here) if the value is larger than u32 you get
a build error on 32bit,
Then you get the opposite problem that any portable code has to handle
the fact that the result type is (effectively) u64 on 64bit so
you get unwanted (or just unnecessary) integer promotions where the
result is used.
Given that pretty much all GENMASK() are used for hardware bit-patterns
not having a fixed size result type seems wrong.
The newly added GENMASK_U8() and GENMASK_U16() are also pointless.
Integer promotion converts the value to 'signed int' before it
is used.
>...
> > I found code elsewhere that doesn't really want FIELD_PREP() to
> > generate a 64bit value.
> >
> > There are actually a lot of dubious uses of 'long' throughout
> > the kernel that break on 32bit.
> > (Actually pretty much all of them!)
>
> If you're referring to the use of GENMASK() with bitfields larger
> than 32-bits, then as can be seen from the above, the code wouldn't
> even compile and our CI systems would be screaming about it. They
> aren't, so I think your statement here is also wrong.
No, I'm talking about a 'normal' FIELD_PREP(GENMASK(7, 5), val) having
a 64bit type on 64bit.
It tripped up my min_t(int, ) 'cast truncation test' a few times :-)
The problem with with 'long' is more pervasive.
It gets used for 'quite big' values that are clearly independent of whether
a 32bit or 64bit kernel is being built.
You're going to want to see an example, I think this overflows badly on 32bit:
static long pll1443x_calc_kdiv(int mdiv, int pdiv, int sdiv,
unsigned long rate, unsigned long prate)
{
long kdiv;
/* calc kdiv = round(rate * pdiv * 65536 * 2^sdiv / prate) - (mdiv * 65536) */
kdiv = ((rate * ((pdiv * 65536) << sdiv) + prate / 2) / prate) - (mdiv * 65536);
return clamp_t(short, kdiv, KDIV_MIN, KDIV_MAX);
}
https://elixir.bootlin.com/linux/v6.18-rc6/source/drivers/clk/imx/clk-pll14xx.c#L120
(Spot the non-functional clamp_t() - which is why I found this code.)
I'm sure I've seen fs code comparing u64 and ulong that both hold block numbers.
Such code should really only use fixed size types.
Either a value fits in 32bits, so int or u32 is fine; or it doesn't and you
need a u64. 'long' doesn't enter into it.
David
On Wed, 26 Nov 2025 17:32:34 +0800 David Yang wrote: > VLAN_CTRL should be treated as a 64-bit register. GENMASK and BIT > macros use unsigned long as the underlying type, which will result in a > build error on architectures where sizeof(long) == 32. nit: sizeof(long) == 4 ?
© 2016 - 2025 Red Hat, Inc.