On 22/07/17 15:24, Mauro Carvalho Chehab wrote:
> Em Sat, 22 Jul 2017 13:30:57 +0200
> Hans Verkuil <hverkuil@xxxxxxxxx> escreveu:
>
>> From: Hans Verkuil <hans.verkuil@xxxxxxxxx>
>>
>> Set media_version to LINUX_VERSION_CODE, just as we did for
>> driver_version.
>>
>> Nobody ever rememebers to update the version number, but
>> LINUX_VERSION_CODE will always be updated.
>>
>> Move the MEDIA_API_VERSION define to the ifndef __KERNEL__ section of the
>> media.h header. That way kernelspace can't accidentally start to use
>> it again.
>>
>> Signed-off-by: Hans Verkuil <hans.verkuil@xxxxxxxxx>
>> ---
>> drivers/media/media-device.c | 3 +--
>> include/uapi/linux/media.h | 5 +++--
>> 2 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
>> index 979e4307d248..3c99294e3ebf 100644
>> --- a/drivers/media/media-device.c
>> +++ b/drivers/media/media-device.c
>> @@ -69,9 +69,8 @@ static int media_device_get_info(struct media_device *dev,
>> strlcpy(info->serial, dev->serial, sizeof(info->serial));
>> strlcpy(info->bus_info, dev->bus_info, sizeof(info->bus_info));
>>
>> - info->media_version = MEDIA_API_VERSION;
>> + info->media_version = info->driver_version = LINUX_VERSION_CODE;
>> info->hw_revision = dev->hw_revision;
>> - info->driver_version = LINUX_VERSION_CODE;
>>
>> return 0;
>> }
>> diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
>> index fac96c64fe51..4865f1e71339 100644
>> --- a/include/uapi/linux/media.h
>> +++ b/include/uapi/linux/media.h
>> @@ -30,8 +30,6 @@
>> #include <linux/types.h>
>> #include <linux/version.h>
>>
>> -#define MEDIA_API_VERSION KERNEL_VERSION(0, 1, 0)
>> -
>> struct media_device_info {
>> char driver[16];
>> char model[32];
>> @@ -187,6 +185,9 @@ struct media_device_info {
>> #define MEDIA_ENT_T_V4L2_SUBDEV_LENS MEDIA_ENT_F_LENS
>> #define MEDIA_ENT_T_V4L2_SUBDEV_DECODER MEDIA_ENT_F_ATV_DECODER
>> #define MEDIA_ENT_T_V4L2_SUBDEV_TUNER MEDIA_ENT_F_TUNER
>> +
>> +/* Obsolete symbol for media_version, no longer used in the kernel */
>> +#define MEDIA_API_VERSION KERNEL_VERSION(0, 1, 0)
>
> IMHO, it should, instead be identical to LINUX_VERSION_CODE, as
> applications might be relying on it in order to check what
> media API version they receive from the MC queries.
That's useless. The only reason you want to use media_version in an application
is to check whether a feature is present provided you know for which kernel
version it appeared. So you will explicitly compare it against e.g. 4.x.0.
Never against LINUX_VERSION_CODE.
In fact, any application using this today will do something like:
media_version >= MEDIA_API_VERSION
and that should keep working in the future. Since any kernel release is >= 0.1.0
this will always work.
media_version >= LINUX_VERSION_CODE is dangerous since this define comes from
a userspace header (/usr/include/linux/version.h) which may be newer/older than the
actual running kernel. So the result of "media_version >= LINUX_VERSION_CODE" is
effectively undefined.
Regards,
Hans
>
> The problem is that this macro is defined only internally inside
> the Kernel tree.
>
>> #endif
>>
>> /* Entity flags */
>
>
>
> Thanks,
> Mauro
>
