* [PATCH 00/12] treewide: Fix GENMASK misuses @ 2019-07-10 5:04 ` Joe Perches 2019-07-10 5:04 ` [PATCH 03/12] drm: aspeed_gfx: Fix misuse of GENMASK macro Joe Perches ` (3 more replies) 0 siblings, 4 replies; 13+ messages in thread From: Joe Perches @ 2019-07-10 5:04 UTC (permalink / raw) To: linux-aspeed These GENMASK uses are inverted argument order and the actual masks produced are incorrect. Fix them. Add checkpatch tests to help avoid more misuses too. Joe Perches (12): checkpatch: Add GENMASK tests clocksource/drivers/npcm: Fix misuse of GENMASK macro drm: aspeed_gfx: Fix misuse of GENMASK macro iio: adc: max9611: Fix misuse of GENMASK macro irqchip/gic-v3-its: Fix misuse of GENMASK macro mmc: meson-mx-sdio: Fix misuse of GENMASK macro net: ethernet: mediatek: Fix misuses of GENMASK macro net: stmmac: Fix misuses of GENMASK macro rtw88: Fix misuse of GENMASK macro phy: amlogic: G12A: Fix misuse of GENMASK macro staging: media: cedrus: Fix misuse of GENMASK macro ASoC: wcd9335: Fix misuse of GENMASK macro drivers/clocksource/timer-npcm7xx.c | 2 +- drivers/gpu/drm/aspeed/aspeed_gfx.h | 2 +- drivers/iio/adc/max9611.c | 2 +- drivers/irqchip/irq-gic-v3-its.c | 2 +- drivers/mmc/host/meson-mx-sdio.c | 2 +- drivers/net/ethernet/mediatek/mtk_eth_soc.h | 2 +- drivers/net/ethernet/mediatek/mtk_sgmii.c | 2 +- drivers/net/ethernet/stmicro/stmmac/descs.h | 2 +- drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 4 ++-- drivers/net/wireless/realtek/rtw88/rtw8822b.c | 2 +- drivers/phy/amlogic/phy-meson-g12a-usb2.c | 2 +- drivers/staging/media/sunxi/cedrus/cedrus_regs.h | 2 +- scripts/checkpatch.pl | 15 +++++++++++++++ sound/soc/codecs/wcd-clsh-v2.c | 2 +- 14 files changed, 29 insertions(+), 14 deletions(-) -- 2.15.0 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 03/12] drm: aspeed_gfx: Fix misuse of GENMASK macro 2019-07-10 5:04 ` [PATCH 00/12] treewide: Fix GENMASK misuses Joe Perches @ 2019-07-10 5:04 ` Joe Perches 2019-07-24 17:16 ` Joe Perches 2019-07-10 9:17 ` [PATCH 00/12] treewide: Fix GENMASK misuses Johannes Berg ` (2 subsequent siblings) 3 siblings, 1 reply; 13+ messages in thread From: Joe Perches @ 2019-07-10 5:04 UTC (permalink / raw) To: linux-aspeed Arguments are supposed to be ordered high then low. Signed-off-by: Joe Perches <joe@perches.com> --- drivers/gpu/drm/aspeed/aspeed_gfx.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx.h b/drivers/gpu/drm/aspeed/aspeed_gfx.h index a10358bb61ec..095ea03e5833 100644 --- a/drivers/gpu/drm/aspeed/aspeed_gfx.h +++ b/drivers/gpu/drm/aspeed/aspeed_gfx.h @@ -74,7 +74,7 @@ int aspeed_gfx_create_output(struct drm_device *drm); /* CTRL2 */ #define CRT_CTRL_DAC_EN BIT(0) #define CRT_CTRL_VBLANK_LINE(x) (((x) << 20) & CRT_CTRL_VBLANK_LINE_MASK) -#define CRT_CTRL_VBLANK_LINE_MASK GENMASK(20, 31) +#define CRT_CTRL_VBLANK_LINE_MASK GENMASK(31, 20) /* CRT_HORIZ0 */ #define CRT_H_TOTAL(x) (x) -- 2.15.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 03/12] drm: aspeed_gfx: Fix misuse of GENMASK macro 2019-07-10 5:04 ` [PATCH 03/12] drm: aspeed_gfx: Fix misuse of GENMASK macro Joe Perches @ 2019-07-24 17:16 ` Joe Perches 2019-07-25 1:10 ` Andrew Jeffery 0 siblings, 1 reply; 13+ messages in thread From: Joe Perches @ 2019-07-24 17:16 UTC (permalink / raw) To: linux-aspeed On Tue, 2019-07-09 at 22:04 -0700, Joe Perches wrote: > Arguments are supposed to be ordered high then low. > > Signed-off-by: Joe Perches <joe@perches.com> > --- > drivers/gpu/drm/aspeed/aspeed_gfx.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx.h b/drivers/gpu/drm/aspeed/aspeed_gfx.h > index a10358bb61ec..095ea03e5833 100644 > --- a/drivers/gpu/drm/aspeed/aspeed_gfx.h > +++ b/drivers/gpu/drm/aspeed/aspeed_gfx.h > @@ -74,7 +74,7 @@ int aspeed_gfx_create_output(struct drm_device *drm); > /* CTRL2 */ > #define CRT_CTRL_DAC_EN BIT(0) > #define CRT_CTRL_VBLANK_LINE(x) (((x) << 20) & CRT_CTRL_VBLANK_LINE_MASK) > -#define CRT_CTRL_VBLANK_LINE_MASK GENMASK(20, 31) > +#define CRT_CTRL_VBLANK_LINE_MASK GENMASK(31, 20) ping? ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 03/12] drm: aspeed_gfx: Fix misuse of GENMASK macro 2019-07-24 17:16 ` Joe Perches @ 2019-07-25 1:10 ` Andrew Jeffery 2019-07-25 1:18 ` Joe Perches 0 siblings, 1 reply; 13+ messages in thread From: Andrew Jeffery @ 2019-07-25 1:10 UTC (permalink / raw) To: linux-aspeed On Thu, 25 Jul 2019, at 02:46, Joe Perches wrote: > On Tue, 2019-07-09 at 22:04 -0700, Joe Perches wrote: > > Arguments are supposed to be ordered high then low. > > > > Signed-off-by: Joe Perches <joe@perches.com> > > --- > > drivers/gpu/drm/aspeed/aspeed_gfx.h | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx.h b/drivers/gpu/drm/aspeed/aspeed_gfx.h > > index a10358bb61ec..095ea03e5833 100644 > > --- a/drivers/gpu/drm/aspeed/aspeed_gfx.h > > +++ b/drivers/gpu/drm/aspeed/aspeed_gfx.h > > @@ -74,7 +74,7 @@ int aspeed_gfx_create_output(struct drm_device *drm); > > /* CTRL2 */ > > #define CRT_CTRL_DAC_EN BIT(0) > > #define CRT_CTRL_VBLANK_LINE(x) (((x) << 20) & CRT_CTRL_VBLANK_LINE_MASK) > > -#define CRT_CTRL_VBLANK_LINE_MASK GENMASK(20, 31) > > +#define CRT_CTRL_VBLANK_LINE_MASK GENMASK(31, 20) Reviewed-by: Andrew Jeffery <andrew@aj.id.au> > > ping? > > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 03/12] drm: aspeed_gfx: Fix misuse of GENMASK macro 2019-07-25 1:10 ` Andrew Jeffery @ 2019-07-25 1:18 ` Joe Perches 2019-07-25 2:52 ` Joel Stanley 0 siblings, 1 reply; 13+ messages in thread From: Joe Perches @ 2019-07-25 1:18 UTC (permalink / raw) To: linux-aspeed On Thu, 2019-07-25 at 10:40 +0930, Andrew Jeffery wrote: > > On Thu, 25 Jul 2019, at 02:46, Joe Perches wrote: > > On Tue, 2019-07-09 at 22:04 -0700, Joe Perches wrote: > > > Arguments are supposed to be ordered high then low. > > > > > > Signed-off-by: Joe Perches <joe@perches.com> > > > --- > > > drivers/gpu/drm/aspeed/aspeed_gfx.h | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx.h b/drivers/gpu/drm/aspeed/aspeed_gfx.h > > > index a10358bb61ec..095ea03e5833 100644 > > > --- a/drivers/gpu/drm/aspeed/aspeed_gfx.h > > > +++ b/drivers/gpu/drm/aspeed/aspeed_gfx.h > > > @@ -74,7 +74,7 @@ int aspeed_gfx_create_output(struct drm_device *drm); > > > /* CTRL2 */ > > > #define CRT_CTRL_DAC_EN BIT(0) > > > #define CRT_CTRL_VBLANK_LINE(x) (((x) << 20) & CRT_CTRL_VBLANK_LINE_MASK) > > > -#define CRT_CTRL_VBLANK_LINE_MASK GENMASK(20, 31) > > > +#define CRT_CTRL_VBLANK_LINE_MASK GENMASK(31, 20) > > Reviewed-by: Andrew Jeffery <andrew@aj.id.au> This hardly needs a review, it needs to be applied. There's a nominal git tree for aspeed here: T: git git://git.kernel.org/pub/scm/linux/kernel/git/joel/aspeed.git But who's going to do apply this? ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 03/12] drm: aspeed_gfx: Fix misuse of GENMASK macro 2019-07-25 1:18 ` Joe Perches @ 2019-07-25 2:52 ` Joel Stanley 2019-07-25 14:37 ` Joe Perches 0 siblings, 1 reply; 13+ messages in thread From: Joel Stanley @ 2019-07-25 2:52 UTC (permalink / raw) To: linux-aspeed On Thu, 25 Jul 2019 at 01:18, Joe Perches <joe@perches.com> wrote: > > On Thu, 2019-07-25 at 10:40 +0930, Andrew Jeffery wrote: > > > > On Thu, 25 Jul 2019, at 02:46, Joe Perches wrote: > > > On Tue, 2019-07-09 at 22:04 -0700, Joe Perches wrote: > > > > Arguments are supposed to be ordered high then low. > > > > > > > > Signed-off-by: Joe Perches <joe@perches.com> > > > > --- > > > > drivers/gpu/drm/aspeed/aspeed_gfx.h | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx.h b/drivers/gpu/drm/aspeed/aspeed_gfx.h > > > > index a10358bb61ec..095ea03e5833 100644 > > > > --- a/drivers/gpu/drm/aspeed/aspeed_gfx.h > > > > +++ b/drivers/gpu/drm/aspeed/aspeed_gfx.h > > > > @@ -74,7 +74,7 @@ int aspeed_gfx_create_output(struct drm_device *drm); > > > > /* CTRL2 */ > > > > #define CRT_CTRL_DAC_EN BIT(0) > > > > #define CRT_CTRL_VBLANK_LINE(x) (((x) << 20) & CRT_CTRL_VBLANK_LINE_MASK) > > > > -#define CRT_CTRL_VBLANK_LINE_MASK GENMASK(20, 31) > > > > +#define CRT_CTRL_VBLANK_LINE_MASK GENMASK(31, 20) > > > > Reviewed-by: Andrew Jeffery <andrew@aj.id.au> > > This hardly needs a review, it needs to be applied. > There's a nominal git tree for aspeed here: > > T: git git://git.kernel.org/pub/scm/linux/kernel/git/joel/aspeed.git > > But who's going to do apply this? This is a DRM patch, so it goes through the DRM tree. I am a co-maintainer there and can apply it once I remember how to drive the tools. (FYI, this macro is not used by the current driver). Cheers, Joel ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 03/12] drm: aspeed_gfx: Fix misuse of GENMASK macro 2019-07-25 2:52 ` Joel Stanley @ 2019-07-25 14:37 ` Joe Perches 0 siblings, 0 replies; 13+ messages in thread From: Joe Perches @ 2019-07-25 14:37 UTC (permalink / raw) To: linux-aspeed On Thu, 2019-07-25 at 02:52 +0000, Joel Stanley wrote: > On Thu, 25 Jul 2019 at 01:18, Joe Perches <joe@perches.com> wrote: > > On Thu, 2019-07-25 at 10:40 +0930, Andrew Jeffery wrote: > > > On Thu, 25 Jul 2019, at 02:46, Joe Perches wrote: > > > > On Tue, 2019-07-09 at 22:04 -0700, Joe Perches wrote: > > > > > Arguments are supposed to be ordered high then low. > > > > > > > > > > Signed-off-by: Joe Perches <joe@perches.com> > > > > > --- > > > > > drivers/gpu/drm/aspeed/aspeed_gfx.h | 2 +- > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx.h b/drivers/gpu/drm/aspeed/aspeed_gfx.h > > > > > index a10358bb61ec..095ea03e5833 100644 > > > > > --- a/drivers/gpu/drm/aspeed/aspeed_gfx.h > > > > > +++ b/drivers/gpu/drm/aspeed/aspeed_gfx.h > > > > > @@ -74,7 +74,7 @@ int aspeed_gfx_create_output(struct drm_device *drm); > > > > > /* CTRL2 */ > > > > > #define CRT_CTRL_DAC_EN BIT(0) > > > > > #define CRT_CTRL_VBLANK_LINE(x) (((x) << 20) & CRT_CTRL_VBLANK_LINE_MASK) > > > > > -#define CRT_CTRL_VBLANK_LINE_MASK GENMASK(20, 31) > > > > > +#define CRT_CTRL_VBLANK_LINE_MASK GENMASK(31, 20) > > > > > > Reviewed-by: Andrew Jeffery <andrew@aj.id.au> > > > > This hardly needs a review, it needs to be applied. > > There's a nominal git tree for aspeed here: > > > > T: git git://git.kernel.org/pub/scm/linux/kernel/git/joel/aspeed.git > > > > But who's going to do apply this? > > This is a DRM patch, so it goes through the DRM tree. I am a > co-maintainer there and can apply it once I remember how to drive the > tools. > > (FYI, this macro is not used by the current driver). Then perhaps CRT_CTRL_VBLANK and _MASK defines should be removed instead. cheers, Joe ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 00/12] treewide: Fix GENMASK misuses 2019-07-10 5:04 ` [PATCH 00/12] treewide: Fix GENMASK misuses Joe Perches 2019-07-10 5:04 ` [PATCH 03/12] drm: aspeed_gfx: Fix misuse of GENMASK macro Joe Perches @ 2019-07-10 9:17 ` Johannes Berg 2019-07-10 9:43 ` Russell King - ARM Linux admin 2019-07-11 21:30 ` David Miller 2019-07-12 12:54 ` Andrzej Hajda 3 siblings, 1 reply; 13+ messages in thread From: Johannes Berg @ 2019-07-10 9:17 UTC (permalink / raw) To: linux-aspeed On Tue, 2019-07-09 at 22:04 -0700, Joe Perches wrote: > These GENMASK uses are inverted argument order and the > actual masks produced are incorrect. Fix them. > > Add checkpatch tests to help avoid more misuses too. > > Joe Perches (12): > checkpatch: Add GENMASK tests IMHO this doesn't make a lot of sense as a checkpatch test - just throw in a BUILD_BUG_ON()? johannes ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 00/12] treewide: Fix GENMASK misuses 2019-07-10 9:17 ` [PATCH 00/12] treewide: Fix GENMASK misuses Johannes Berg @ 2019-07-10 9:43 ` Russell King - ARM Linux admin 2019-07-10 15:45 ` Joe Perches 0 siblings, 1 reply; 13+ messages in thread From: Russell King - ARM Linux admin @ 2019-07-10 9:43 UTC (permalink / raw) To: linux-aspeed On Wed, Jul 10, 2019 at 11:17:31AM +0200, Johannes Berg wrote: > On Tue, 2019-07-09 at 22:04 -0700, Joe Perches wrote: > > These GENMASK uses are inverted argument order and the > > actual masks produced are incorrect. Fix them. > > > > Add checkpatch tests to help avoid more misuses too. > > > > Joe Perches (12): > > checkpatch: Add GENMASK tests > > IMHO this doesn't make a lot of sense as a checkpatch test - just throw > in a BUILD_BUG_ON()? My personal take on this is that GENMASK() is really not useful, it's just pure obfuscation and leads to exactly these kinds of mistakes. Yes, I fully understand the argument that you can just specify the start and end bits, and it _in theory_ makes the code more readable. However, the problem is when writing code. GENMASK(a, b). Is a the starting bit or ending bit? Is b the number of bits? It's confusing and causes mistakes resulting in incorrect code. A BUILD_BUG_ON() can catch some of the cases, but not all of them. For example: GENMASK(6, 2) would satisify the requirement that a > b, so a BUILD_BUG_ON() will not trigger, but was the author meaning 0x3c or 0xc0? Personally, I've decided I am _not_ going to use GENMASK() in my code because I struggle to get the macro arguments correct - I'm _much_ happier, and it is way more reliable for me to write the mask in hex notation. I think this is where use of a ternary operator would come in use. The normal way of writing a number of bits tends to be "a:b", so if GENMASK took something like GENMASK(6:2), then I'd have less issue with it, because it's argument is then in a familiar notation. Yes, I'm sure that someone will point out that the GENMASK arguments are just in the same order, but that doesn't prevent _me_ frequently getting it wrong - and that's the point. The macro seems to me to cause more problems than it solves. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 00/12] treewide: Fix GENMASK misuses 2019-07-10 9:43 ` Russell King - ARM Linux admin @ 2019-07-10 15:45 ` Joe Perches 2019-07-10 16:01 ` Joe Perches 0 siblings, 1 reply; 13+ messages in thread From: Joe Perches @ 2019-07-10 15:45 UTC (permalink / raw) To: linux-aspeed On Wed, 2019-07-10 at 10:43 +0100, Russell King - ARM Linux admin wrote: > On Wed, Jul 10, 2019 at 11:17:31AM +0200, Johannes Berg wrote: > > On Tue, 2019-07-09 at 22:04 -0700, Joe Perches wrote: > > > These GENMASK uses are inverted argument order and the > > > actual masks produced are incorrect. Fix them. > > > > > > Add checkpatch tests to help avoid more misuses too. > > > > > > Joe Perches (12): > > > checkpatch: Add GENMASK tests > > > > IMHO this doesn't make a lot of sense as a checkpatch test - just throw > > in a BUILD_BUG_ON()? I tried that. It'd can't be done as it's used in declarations and included in asm files and it uses the UL() macro. I also tried just making it do the right thing whatever the argument order. Oh well. > My personal take on this is that GENMASK() is really not useful, it's > just pure obfuscation and leads to exactly these kinds of mistakes. > > Yes, I fully understand the argument that you can just specify the > start and end bits, and it _in theory_ makes the code more readable. > > However, the problem is when writing code. GENMASK(a, b). Is a the > starting bit or ending bit? Is b the number of bits? It's confusing > and causes mistakes resulting in incorrect code. A BUILD_BUG_ON() > can catch some of the cases, but not all of them. It's a horrid little macro and I agree with Russell. I also think if it existed at all it should have been GENMASK(low, high) not GENMASK(high, low). I ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 00/12] treewide: Fix GENMASK misuses 2019-07-10 15:45 ` Joe Perches @ 2019-07-10 16:01 ` Joe Perches 0 siblings, 0 replies; 13+ messages in thread From: Joe Perches @ 2019-07-10 16:01 UTC (permalink / raw) To: linux-aspeed On Wed, 2019-07-10 at 08:45 -0700, Joe Perches wrote: > On Wed, 2019-07-10 at 10:43 +0100, Russell King - ARM Linux admin wrote: > > On Wed, Jul 10, 2019 at 11:17:31AM +0200, Johannes Berg wrote: > > > On Tue, 2019-07-09 at 22:04 -0700, Joe Perches wrote: > > > > These GENMASK uses are inverted argument order and the > > > > actual masks produced are incorrect. Fix them. > > > > > > > > Add checkpatch tests to help avoid more misuses too. > > > > > > > > Joe Perches (12): > > > > checkpatch: Add GENMASK tests > > > > > > IMHO this doesn't make a lot of sense as a checkpatch test - just throw > > > in a BUILD_BUG_ON()? > > I tried that. > > It'd can't be done as it's used in declarations > and included in asm files and it uses the UL() > macro. > > I also tried just making it do the right thing > whatever the argument order. I forgot. I also made all those arguments when it was introduced in 2013. https://lore.kernel.org/patchwork/patch/414248/ > Oh well. yeah. ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 00/12] treewide: Fix GENMASK misuses 2019-07-10 5:04 ` [PATCH 00/12] treewide: Fix GENMASK misuses Joe Perches 2019-07-10 5:04 ` [PATCH 03/12] drm: aspeed_gfx: Fix misuse of GENMASK macro Joe Perches 2019-07-10 9:17 ` [PATCH 00/12] treewide: Fix GENMASK misuses Johannes Berg @ 2019-07-11 21:30 ` David Miller 2019-07-12 12:54 ` Andrzej Hajda 3 siblings, 0 replies; 13+ messages in thread From: David Miller @ 2019-07-11 21:30 UTC (permalink / raw) To: linux-aspeed From: Joe Perches <joe@perches.com> Date: Tue, 9 Jul 2019 22:04:13 -0700 > These GENMASK uses are inverted argument order and the > actual masks produced are incorrect. Fix them. > > Add checkpatch tests to help avoid more misuses too. Patches #7 and #8 applied to 'net', with appropriate Fixes tags added to #8. ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 00/12] treewide: Fix GENMASK misuses 2019-07-10 5:04 ` [PATCH 00/12] treewide: Fix GENMASK misuses Joe Perches ` (2 preceding siblings ...) 2019-07-11 21:30 ` David Miller @ 2019-07-12 12:54 ` Andrzej Hajda 3 siblings, 0 replies; 13+ messages in thread From: Andrzej Hajda @ 2019-07-12 12:54 UTC (permalink / raw) To: linux-aspeed Hi Joe, On 10.07.2019 07:04, Joe Perches wrote: > These GENMASK uses are inverted argument order and the > actual masks produced are incorrect. Fix them. > > Add checkpatch tests to help avoid more misuses too. > > Joe Perches (12): > checkpatch: Add GENMASK tests > clocksource/drivers/npcm: Fix misuse of GENMASK macro > drm: aspeed_gfx: Fix misuse of GENMASK macro > iio: adc: max9611: Fix misuse of GENMASK macro > irqchip/gic-v3-its: Fix misuse of GENMASK macro > mmc: meson-mx-sdio: Fix misuse of GENMASK macro > net: ethernet: mediatek: Fix misuses of GENMASK macro > net: stmmac: Fix misuses of GENMASK macro > rtw88: Fix misuse of GENMASK macro > phy: amlogic: G12A: Fix misuse of GENMASK macro > staging: media: cedrus: Fix misuse of GENMASK macro > ASoC: wcd9335: Fix misuse of GENMASK macro > > drivers/clocksource/timer-npcm7xx.c | 2 +- > drivers/gpu/drm/aspeed/aspeed_gfx.h | 2 +- > drivers/iio/adc/max9611.c | 2 +- > drivers/irqchip/irq-gic-v3-its.c | 2 +- > drivers/mmc/host/meson-mx-sdio.c | 2 +- > drivers/net/ethernet/mediatek/mtk_eth_soc.h | 2 +- > drivers/net/ethernet/mediatek/mtk_sgmii.c | 2 +- > drivers/net/ethernet/stmicro/stmmac/descs.h | 2 +- > drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 4 ++-- > drivers/net/wireless/realtek/rtw88/rtw8822b.c | 2 +- > drivers/phy/amlogic/phy-meson-g12a-usb2.c | 2 +- > drivers/staging/media/sunxi/cedrus/cedrus_regs.h | 2 +- > scripts/checkpatch.pl | 15 +++++++++++++++ > sound/soc/codecs/wcd-clsh-v2.c | 2 +- > 14 files changed, 29 insertions(+), 14 deletions(-) > After adding following compile time check: ------ diff --git a/Makefile b/Makefile index 5102b2bbd224..ac4ea5f443a9 100644 --- a/Makefile +++ b/Makefile @@ -457,7 +457,7 @@ KBUILD_AFLAGS?? := -D__ASSEMBLY__ -fno-PIE ?KBUILD_CFLAGS?? := -Wall -Wundef -Werror=strict-prototypes -Wno-trigraphs \ ?????????????????? -fno-strict-aliasing -fno-common -fshort-wchar -fno-PIE \ ?????????????????? -Werror=implicit-function-declaration -Werror=implicit-int \ -????????????????? -Wno-format-security \ +????????????????? -Wno-format-security -Werror=div-by-zero \ ?????????????????? -std=gnu89 ?KBUILD_CPPFLAGS := -D__KERNEL__ ?KBUILD_AFLAGS_KERNEL := diff --git a/include/linux/bits.h b/include/linux/bits.h index 669d69441a62..61d74b103055 100644 --- a/include/linux/bits.h +++ b/include/linux/bits.h @@ -19,11 +19,11 @@ ? * GENMASK_ULL(39, 21) gives us the 64bit vector 0x000000ffffe00000. ? */ ?#define GENMASK(h, l) \ -?????? (((~UL(0)) - (UL(1) << (l)) + 1) & \ +?????? (((~UL(0)) - (UL(1) << (l)) + 1 + 0/((h) >= (l))) & \ ???????? (~UL(0) >> (BITS_PER_LONG - 1 - (h)))) ? ?#define GENMASK_ULL(h, l) \ -?????? (((~ULL(0)) - (ULL(1) << (l)) + 1) & \ +?????? (((~ULL(0)) - (ULL(1) << (l)) + 1 + 0/((h) >= (l))) & \ ???????? (~ULL(0) >> (BITS_PER_LONG_LONG - 1 - (h)))) ? ?#endif /* __LINUX_BITS_H */ ------- I was able to detect one more GENMASK misue (AARCH64, allyesconfig): ? CC????? drivers/phy/rockchip/phy-rockchip-inno-hdmi.o In file included from ../include/linux/bitops.h:5:0, ???????????????? from ../include/linux/kernel.h:12, ???????????????? from ../include/linux/clk.h:13, ???????????????? from ../drivers/phy/rockchip/phy-rockchip-inno-hdmi.c:9: ../drivers/phy/rockchip/phy-rockchip-inno-hdmi.c: In function ?inno_hdmi_phy_rk3328_power_on?: ../include/linux/bits.h:22:37: error: division by zero [-Werror=div-by-zero] ? (((~UL(0)) - (UL(1) << (l)) + 1 + 0/((h) >= (l))) & \ ???????????????????????????????????? ^ ../drivers/phy/rockchip/phy-rockchip-inno-hdmi.c:24:42: note: in expansion of macro ?GENMASK? ?#define UPDATE(x, h, l)? (((x) << (l)) & GENMASK((h), (l))) ????????????????????????????????????????? ^~~~~~~ ../drivers/phy/rockchip/phy-rockchip-inno-hdmi.c:201:50: note: in expansion of macro ?UPDATE? ?#define RK3328_TERM_RESISTOR_CALIB_SPEED_7_0(x)? UPDATE(x, 7, 9) ????????????????????????????????????????????????? ^~~~~~ ../drivers/phy/rockchip/phy-rockchip-inno-hdmi.c:1046:26: note: in expansion of macro ?RK3328_TERM_RESISTOR_CALIB_SPEED_7_0? ?? inno_write(inno, 0xc6, RK3328_TERM_RESISTOR_CALIB_SPEED_7_0(v)); Of course I do not advise to add the check as is to Kernel - it is undefined behavior area AFAIK. Regards Andrzej ^ permalink raw reply related [flat|nested] 13+ messages in thread
end of thread, other threads:[~2019-07-25 14:37 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- [not found] <CGME20190710050444epcas1p250f7aa0f8798a7757df51d66f5970c2a@epcas1p2.samsung.com> 2019-07-10 5:04 ` [PATCH 00/12] treewide: Fix GENMASK misuses Joe Perches 2019-07-10 5:04 ` [PATCH 03/12] drm: aspeed_gfx: Fix misuse of GENMASK macro Joe Perches 2019-07-24 17:16 ` Joe Perches 2019-07-25 1:10 ` Andrew Jeffery 2019-07-25 1:18 ` Joe Perches 2019-07-25 2:52 ` Joel Stanley 2019-07-25 14:37 ` Joe Perches 2019-07-10 9:17 ` [PATCH 00/12] treewide: Fix GENMASK misuses Johannes Berg 2019-07-10 9:43 ` Russell King - ARM Linux admin 2019-07-10 15:45 ` Joe Perches 2019-07-10 16:01 ` Joe Perches 2019-07-11 21:30 ` David Miller 2019-07-12 12:54 ` Andrzej Hajda
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).