Hi Garrit,
Thank you for the patch.
On Fri, Jul 24, 2020 at 09:01:18PM +0200, Garrit Franke wrote:
A commit message would be nice.
> Signed-off-by: Garrit Franke <garritfranke@xxxxxxxxx>
> ---
> include/uapi/linux/media.h | 20 ++++++++++----------
> 1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
> index 383ac7b7d8..5710ba0c83 100644
> --- a/include/uapi/linux/media.h
> +++ b/include/uapi/linux/media.h
> @@ -142,8 +142,8 @@ struct media_device_info {
> #define MEDIA_ENT_F_DV_ENCODER (MEDIA_ENT_F_BASE + 0x6002)
>
> /* Entity flags */
> -#define MEDIA_ENT_FL_DEFAULT (1 << 0)
> -#define MEDIA_ENT_FL_CONNECTOR (1 << 1)
> +#define MEDIA_ENT_FL_DEFAULT BIT(0)
> +#define MEDIA_ENT_FL_CONNECTOR BIT(1)
The BIT() macro isn't available in the uapi headers.
>
> /* OR with the entity id value to find the next entity */
> #define MEDIA_ENT_ID_FLAG_NEXT (1U << 31)
> @@ -207,9 +207,9 @@ struct media_entity_desc {
> };
> };
>
> -#define MEDIA_PAD_FL_SINK (1 << 0)
> -#define MEDIA_PAD_FL_SOURCE (1 << 1)
> -#define MEDIA_PAD_FL_MUST_CONNECT (1 << 2)
> +#define MEDIA_PAD_FL_SINK BIT(0)
> +#define MEDIA_PAD_FL_SOURCE BIT(1)
> +#define MEDIA_PAD_FL_MUST_CONNECT BIT(2)
>
> struct media_pad_desc {
> __u32 entity; /* entity ID */
> @@ -218,13 +218,13 @@ struct media_pad_desc {
> __u32 reserved[2];
> };
>
> -#define MEDIA_LNK_FL_ENABLED (1 << 0)
> -#define MEDIA_LNK_FL_IMMUTABLE (1 << 1)
> -#define MEDIA_LNK_FL_DYNAMIC (1 << 2)
> +#define MEDIA_LNK_FL_ENABLED BIT(0)
> +#define MEDIA_LNK_FL_IMMUTABLE BIT(1)
> +#define MEDIA_LNK_FL_DYNAMIC BIT(2)
>
> #define MEDIA_LNK_FL_LINK_TYPE (0xf << 28)
> # define MEDIA_LNK_FL_DATA_LINK (0 << 28)
> -# define MEDIA_LNK_FL_INTERFACE_LINK (1 << 28)
> +# define MEDIA_LNK_FL_INTERFACE_LINK BIT(28)
This is wrong, BIT() should only be used for fields that are 1-bit wide.
If you look at MEDIA_LNK_FL_LINK_TYPE you can see the field is 4-bits
wide, it stores a value between 0 and 15. MEDIA_LNK_FL_INTERFACE_LINK
happens to match the (1 << n) pattern, but it doesn't mean it's a single
bit field.
>
> struct media_link_desc {
> struct media_pad_desc source;
> @@ -433,7 +433,7 @@ struct media_v2_topology {
> #define MEDIA_INTF_T_ALSA_TIMER (MEDIA_INTF_T_ALSA_BASE + 7)
>
> /* Obsolete symbol for media_version, no longer used in the kernel */
> -#define MEDIA_API_VERSION ((0 << 16) | (1 << 8) | 0)
> +#define MEDIA_API_VERSION ((0 << 16) | BIT(8) | 0)
Even worse here, this is clearly not a bitfield.
>
> #endif
>
--
Regards,
Laurent Pinchart
