Message ID | 20200324042213.GA10452@asgard.redhat.com (mailing list archive) |
---|---|
State | Mainlined |
Commit | 9b6eaaf3db5e5888df7bca7fed7752a90f7fd871 |
Headers | show |
Series | coresight: do not use the BIT() macro in the UAPI header | expand |
On Tue, Mar 24, 2020 at 05:22:13AM +0100, Eugene Syromiatnikov wrote: > The BIT() macro definition is not available for the UAPI headers > (moreover, it can be defined differently in the user space); replace > its usage with the _BITUL() macro that is defined in <linux/const.h>. Why is somehow _BITUL() ok to use here instead? Just open-code it, I didn't think we could use any BIT()-like macros in uapi .h files. thanks, greg k-h
On Tue, Mar 24, 2020 at 07:28:53AM +0100, Greg Kroah-Hartman wrote: > On Tue, Mar 24, 2020 at 05:22:13AM +0100, Eugene Syromiatnikov wrote: > > The BIT() macro definition is not available for the UAPI headers > > (moreover, it can be defined differently in the user space); replace > > its usage with the _BITUL() macro that is defined in <linux/const.h>. > > Why is somehow _BITUL() ok to use here instead? It is provided in an UAPI header (include/uapi/linux/const.h) and is appropriately prefixed with an underscore to avoid conflicts. > Just open-code it, I didn't think we could use any BIT()-like macros in > uapi .h files. > > thanks, > > greg k-h >
On Tue, Mar 24, 2020 at 10:53:04AM +0100, Eugene Syromiatnikov wrote: > On Tue, Mar 24, 2020 at 07:28:53AM +0100, Greg Kroah-Hartman wrote: > > On Tue, Mar 24, 2020 at 05:22:13AM +0100, Eugene Syromiatnikov wrote: > > > The BIT() macro definition is not available for the UAPI headers > > > (moreover, it can be defined differently in the user space); replace > > > its usage with the _BITUL() macro that is defined in <linux/const.h>. > > > > Why is somehow _BITUL() ok to use here instead? > > It is provided in an UAPI header (include/uapi/linux/const.h) > and is appropriately prefixed with an underscore to avoid conflicts. Because no one uses _ in their own macros? :) Anyway, this is fine, but if it's really the way forward, I think a lot of files will end up being changed... thanks, greg k-h
On Tue, Mar 24, 2020 at 11:19:38AM +0100, Greg Kroah-Hartman wrote: > On Tue, Mar 24, 2020 at 10:53:04AM +0100, Eugene Syromiatnikov wrote: > > On Tue, Mar 24, 2020 at 07:28:53AM +0100, Greg Kroah-Hartman wrote: > > > On Tue, Mar 24, 2020 at 05:22:13AM +0100, Eugene Syromiatnikov wrote: > > > > The BIT() macro definition is not available for the UAPI headers > > > > (moreover, it can be defined differently in the user space); replace > > > > its usage with the _BITUL() macro that is defined in <linux/const.h>. > > > > > > Why is somehow _BITUL() ok to use here instead? > > > > It is provided in an UAPI header (include/uapi/linux/const.h) > > and is appropriately prefixed with an underscore to avoid conflicts. > > Because no one uses _ in their own macros? :) Well, it is a reserved prefix (ANSI C99, 4.1.2 "Standard headers": "All other identifiers that begin with an underscore and either an upper-case letter or another underscore are reserved"), so valid C files shouldn't use them. > Anyway, this is fine, but if it's really the way forward, I think a lot > of files will end up being changed... There are 5 cases for using BIT() in UAPI headers so far (rtc.h[1], serio.h[2], psci.h[3], pkt_sched.h[4], coresight-stm.h), two of them were conversions from the open-coded variant; the usage of _BITUL in pkt_sched.h made me think that it is the better approach since people tend to use BIT-like macro anyway, so, by increasing a number of cases it may raise awareness of the UAPI specifics. [1] https://lore.kernel.org/lkml/20200324041209.GA30727@asgard.redhat.com/ [2] https://lore.kernel.org/lkml/20200324041341.GA32335@asgard.redhat.com/ [3] https://lore.kernel.org/lkml/20200324041526.GA1978@asgard.redhat.com/ [4] https://lore.kernel.org/lkml/20200324041920.GA7068@asgard.redhat.com/ > > thanks, > > greg k-h >
On Mon, 23 Mar 2020 at 22:22, Eugene Syromiatnikov <esyr@redhat.com> wrote: > > The BIT() macro definition is not available for the UAPI headers > (moreover, it can be defined differently in the user space); replace > its usage with the _BITUL() macro that is defined in <linux/const.h>. > > Fixes: 237483aa5cf4 ("coresight: stm: adding driver for CoreSight STM component") > Signed-off-by: Eugene Syromiatnikov <esyr@redhat.com> > --- > include/uapi/linux/coresight-stm.h | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/include/uapi/linux/coresight-stm.h b/include/uapi/linux/coresight-stm.h > index aac550a..8847dbf 100644 > --- a/include/uapi/linux/coresight-stm.h > +++ b/include/uapi/linux/coresight-stm.h > @@ -2,8 +2,10 @@ > #ifndef __UAPI_CORESIGHT_STM_H_ > #define __UAPI_CORESIGHT_STM_H_ > > -#define STM_FLAG_TIMESTAMPED BIT(3) > -#define STM_FLAG_GUARANTEED BIT(7) > +#include <linux/const.h> > + > +#define STM_FLAG_TIMESTAMPED _BITUL(3) > +#define STM_FLAG_GUARANTEED _BITUL(7) Greg, if you want to pick this up right away: Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org> Otherwise let me know and I'll add it to my next tree. Thanks, Mathieu > > /* > * The CoreSight STM supports guaranteed and invariant timing > -- > 2.1.4 >
On Tue, Mar 24, 2020 at 09:31:15AM -0600, Mathieu Poirier wrote: > On Mon, 23 Mar 2020 at 22:22, Eugene Syromiatnikov <esyr@redhat.com> wrote: > > > > The BIT() macro definition is not available for the UAPI headers > > (moreover, it can be defined differently in the user space); replace > > its usage with the _BITUL() macro that is defined in <linux/const.h>. > > > > Fixes: 237483aa5cf4 ("coresight: stm: adding driver for CoreSight STM component") > > Signed-off-by: Eugene Syromiatnikov <esyr@redhat.com> > > --- > > include/uapi/linux/coresight-stm.h | 6 ++++-- > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/include/uapi/linux/coresight-stm.h b/include/uapi/linux/coresight-stm.h > > index aac550a..8847dbf 100644 > > --- a/include/uapi/linux/coresight-stm.h > > +++ b/include/uapi/linux/coresight-stm.h > > @@ -2,8 +2,10 @@ > > #ifndef __UAPI_CORESIGHT_STM_H_ > > #define __UAPI_CORESIGHT_STM_H_ > > > > -#define STM_FLAG_TIMESTAMPED BIT(3) > > -#define STM_FLAG_GUARANTEED BIT(7) > > +#include <linux/const.h> > > + > > +#define STM_FLAG_TIMESTAMPED _BITUL(3) > > +#define STM_FLAG_GUARANTEED _BITUL(7) > > Greg, if you want to pick this up right away: > > Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org> > > Otherwise let me know and I'll add it to my next tree. I'll take it now, thanks. greg k-h
diff --git a/include/uapi/linux/coresight-stm.h b/include/uapi/linux/coresight-stm.h index aac550a..8847dbf 100644 --- a/include/uapi/linux/coresight-stm.h +++ b/include/uapi/linux/coresight-stm.h @@ -2,8 +2,10 @@ #ifndef __UAPI_CORESIGHT_STM_H_ #define __UAPI_CORESIGHT_STM_H_ -#define STM_FLAG_TIMESTAMPED BIT(3) -#define STM_FLAG_GUARANTEED BIT(7) +#include <linux/const.h> + +#define STM_FLAG_TIMESTAMPED _BITUL(3) +#define STM_FLAG_GUARANTEED _BITUL(7) /* * The CoreSight STM supports guaranteed and invariant timing
The BIT() macro definition is not available for the UAPI headers (moreover, it can be defined differently in the user space); replace its usage with the _BITUL() macro that is defined in <linux/const.h>. Fixes: 237483aa5cf4 ("coresight: stm: adding driver for CoreSight STM component") Signed-off-by: Eugene Syromiatnikov <esyr@redhat.com> --- include/uapi/linux/coresight-stm.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)