Hi Dan,
On Fri, Nov 03, 2023 at 10:51:09AM +0300, Dan Carpenter wrote:
> On Fri, Nov 03, 2023 at 07:32:41AM +0000, Sakari Ailus wrote:
> > > > > diff --git a/include/uapi/linux/v4l2-subdev.h b/include/uapi/linux/v4l2-subdev.h
> > > > > index 4a195b68f28f..21d149969119 100644
> > > > > --- a/include/uapi/linux/v4l2-subdev.h
> > > > > +++ b/include/uapi/linux/v4l2-subdev.h
> ^^^^
>
> > > > > @@ -239,7 +239,7 @@ struct v4l2_subdev_routing {
> > > > > * set (which is the default), the 'stream' fields will be forced to 0 by the
> > > > > * kernel.
> > > > > */
> > > > > - #define V4L2_SUBDEV_CLIENT_CAP_STREAMS (1U << 0)
> > > > > + #define V4L2_SUBDEV_CLIENT_CAP_STREAMS BIT_ULL(0)
> > > >
> > > > This is a UAPI header but BIT_ULL() is defined in kernel-only headers.
> > > >
> > > > So (1ULL << 0) ?
> > > >
> > > > uapi/linux/const.h also has _BITULL().
> > >
> > > Let's just do 1ULL < 0. I'll resend. Is there an automated way I could
> > > have caught this?
> >
> > I don't know. :-) Remember to use shift left for bit definitions in UAPI
> > headers?
>
> Yeah. I knew it was UAPI but I'm not used to thinking about UAPI rules.
> I only tried to build this on kernel .c files and didn't try to rebuild
> the usr/ dir.
>
> I bet someone would have complained eventually but who would have
> run into this first... I see there are existing BIT() users in the usr/
> dir, but everyone seems good about using __u32 instead of u32. Probably
> because declaring a variable as u32 causes an immediate compile error
> for everyone but bogus BIT() defines are not an issue until someone
> tries to use them.
>
> KTODO: write a script to check that UAPI doesn't use kernel types
>
> Maybe this could be a part of checkpatch.pl?
I think that would be useful.
I wonder if there are be better ways to check for non-existent macros
than e.g. singling out BIT() and others. I'd think just checking for BIT()
etc. should be enough.
>
> regards,
> dan carpenter
>
> $ grep BIT usr/ -Rw
> usr/include/misc/uacce/uacce.h:#define UACCE_DEV_SVA BIT(0)
> usr/include/linux/psci.h:#define PSCI_1_0_OS_INITIATED BIT(0)
> usr/include/linux/can/netlink.h: * For further information, please read chapter "8 BIT TIMING
> usr/include/linux/cxl_mem.h:#define CXL_MEM_COMMAND_FLAG_ENABLED BIT(0)
> usr/include/linux/cxl_mem.h:#define CXL_MEM_COMMAND_FLAG_EXCLUSIVE BIT(1)
> usr/include/linux/nl80211.h: * bitmask of BIT(NL80211_BAND_*) as described in %enum
> usr/include/linux/mdio.h:#define MDIO_AN_C73_0_PAUSE BIT(10)
> usr/include/linux/mdio.h:#define MDIO_AN_C73_0_ASM_DIR BIT(11)
> usr/include/linux/mdio.h:#define MDIO_AN_C73_0_C2 BIT(12)
> usr/include/linux/mdio.h:#define MDIO_AN_C73_0_RF BIT(13)
> usr/include/linux/mdio.h:#define MDIO_AN_C73_0_ACK BIT(14)
> usr/include/linux/mdio.h:#define MDIO_AN_C73_0_NP BIT(15)
> usr/include/linux/mdio.h:#define MDIO_AN_C73_1_1000BASE_KX BIT(5)
> usr/include/linux/mdio.h:#define MDIO_AN_C73_1_10GBASE_KX4 BIT(6)
> usr/include/linux/mdio.h:#define MDIO_AN_C73_1_10GBASE_KR BIT(7)
> usr/include/linux/mdio.h:#define MDIO_AN_C73_1_40GBASE_KR4 BIT(8)
> usr/include/linux/mdio.h:#define MDIO_AN_C73_1_40GBASE_CR4 BIT(9)
> usr/include/linux/mdio.h:#define MDIO_AN_C73_1_100GBASE_CR10 BIT(10)
> usr/include/linux/mdio.h:#define MDIO_AN_C73_1_100GBASE_KP4 BIT(11)
> usr/include/linux/mdio.h:#define MDIO_AN_C73_1_100GBASE_KR4 BIT(12)
> usr/include/linux/mdio.h:#define MDIO_AN_C73_1_100GBASE_CR4 BIT(13)
> usr/include/linux/mdio.h:#define MDIO_AN_C73_1_25GBASE_R_S BIT(14)
> usr/include/linux/mdio.h:#define MDIO_AN_C73_1_25GBASE_R BIT(15)
> usr/include/linux/mdio.h:#define MDIO_AN_C73_2_2500BASE_KX BIT(0)
> usr/include/linux/mdio.h:#define MDIO_AN_C73_2_5GBASE_KR BIT(1)
> usr/include/asm/kvm.h:#define KVM_PMU_EVENT_FLAG_MASKED_EVENTS BIT(0)
> usr/include/asm/kvm.h:#define KVM_EXIT_HYPERCALL_LONG_MODE BIT(0)
> usr/include/drm/radeon_drm.h: * THESE ARE NOT BIT FIELDS
> usr/include/drm/habanalabs_accel.h:#define HL_RAZWI_READ BIT(0)
> usr/include/drm/habanalabs_accel.h:#define HL_RAZWI_WRITE BIT(1)
> usr/include/drm/habanalabs_accel.h:#define HL_RAZWI_LBW BIT(2)
> usr/include/drm/habanalabs_accel.h:#define HL_RAZWI_HBW BIT(3)
> usr/include/drm/habanalabs_accel.h:#define HL_RAZWI_RR BIT(4)
> usr/include/drm/habanalabs_accel.h:#define HL_RAZWI_ADDR_DEC BIT(5)
Indeed.
--
Regards,
Sakari Ailus
