Currently the media subsystem has a very creative abuse of the
KERNEL_VERSION macro to encode an arbitrary version triplet for media
drivers and device hardware revisions.
This series introduces a new macro called MEDIA_REVISION which encodes
a version triplet like KERNEL_VERSION does, but clearly has media
centric semantics and doesn't fool someone into thinking specific
parts are defined for a specific kernel version only like in out of
tree drivers.
Johannes Thumshirn (7):
[media] media: introduce MEDIA_REVISION macro
video: fbdev: don't use KERNEL_VERSION macro for MEDIA_REVISION
[media] media: document the use of MEDIA_REVISION instead of
KERNEL_VERSION
[media] cx25821: use MEDIA_REVISION instead of KERNEL_VERSION
[media] media: s3c-camif: Use MEDIA_REVISON instead of KERNEL_VERSION
[media] media: bcm2048: use MEDIA_REVISION isntead of KERNEL_VERSION
staging/atomisp: use MEDIA_VERSION instead of KERNEL_VERSION
Documentation/media/uapi/cec/cec-ioc-adap-g-caps.rst | 2 +-
Documentation/media/uapi/mediactl/media-ioc-device-info.rst | 4 ++--
Documentation/media/uapi/v4l/vidioc-querycap.rst | 6 +++---
drivers/media/pci/cx25821/cx25821.h | 2 +-
drivers/media/platform/s3c-camif/camif-core.c | 2 +-
drivers/staging/media/atomisp/include/linux/atomisp.h | 6 +++---
drivers/staging/media/bcm2048/radio-bcm2048.c | 2 +-
drivers/video/fbdev/matrox/matroxfb_base.c | 3 ++-
include/media/media-device.h | 5 ++---
include/uapi/linux/media.h | 4 +++-
10 files changed, 19 insertions(+), 17 deletions(-)
--
2.12.3
Use MEDIA_REVISION instead of KERNEL_VERSION to encode the
CX25821_VERSION_CODE.
Signed-off-by: Johannes Thumshirn <[email protected]>
---
drivers/media/pci/cx25821/cx25821.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/pci/cx25821/cx25821.h b/drivers/media/pci/cx25821/cx25821.h
index 0f20e89b0cde..7fea6b07cf19 100644
--- a/drivers/media/pci/cx25821/cx25821.h
+++ b/drivers/media/pci/cx25821/cx25821.h
@@ -41,7 +41,7 @@
#include <linux/version.h>
#include <linux/mutex.h>
-#define CX25821_VERSION_CODE KERNEL_VERSION(0, 0, 106)
+#define CX25821_VERSION_CODE MEDIA_REVISION(0, 0, 106)
#define UNSET (-1U)
#define NO_SYNC_LINE (-1U)
--
2.12.3
Currently the media code abuses the KERNEL_VERSION macro to encode a
version triplet.
Introduce a MEDIA_REVISION macro to get rid of the confusing and
creative KERNEL_VERSION usage in the media subsystem.
Signed-off-by: Johannes Thumshirn <[email protected]>
---
include/uapi/linux/media.h | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
index 4890787731b8..25e2ae4432bd 100644
--- a/include/uapi/linux/media.h
+++ b/include/uapi/linux/media.h
@@ -30,7 +30,9 @@
#include <linux/types.h>
#include <linux/version.h>
-#define MEDIA_API_VERSION KERNEL_VERSION(0, 1, 0)
+#define MEDIA_REVISION(a,b,c) (((a) << 16) + ((b) << 8) + (c))
+
+#define MEDIA_API_VERSION MEDIA_REVISION(0, 1, 0)
struct media_device_info {
char driver[16];
--
2.12.3
Use MEDIA_REVISION isntead of KERNEL_VERSION to encode the bcm2048
driver version.
Signed-off-by: Johannes Thumshirn <[email protected]>
---
drivers/staging/media/bcm2048/radio-bcm2048.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/media/bcm2048/radio-bcm2048.c b/drivers/staging/media/bcm2048/radio-bcm2048.c
index 38f72d069e27..ffe9b63cbf59 100644
--- a/drivers/staging/media/bcm2048/radio-bcm2048.c
+++ b/drivers/staging/media/bcm2048/radio-bcm2048.c
@@ -48,7 +48,7 @@
/* driver definitions */
#define BCM2048_DRIVER_AUTHOR "Eero Nurkkala <[email protected]>"
#define BCM2048_DRIVER_NAME BCM2048_NAME
-#define BCM2048_DRIVER_VERSION KERNEL_VERSION(0, 0, 1)
+#define BCM2048_DRIVER_VERSION MEDIA_REVISION(0, 0, 1)
#define BCM2048_DRIVER_CARD "Broadcom bcm2048 FM Radio Receiver"
#define BCM2048_DRIVER_DESC "I2C driver for BCM2048 FM Radio Receiver"
--
2.12.3
Use MEDIA_REVISON instead of KERNEL_VERSION to encode the driver
version.
Signed-off-by: Johannes Thumshirn <[email protected]>
---
drivers/media/platform/s3c-camif/camif-core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/platform/s3c-camif/camif-core.c b/drivers/media/platform/s3c-camif/camif-core.c
index ec4001970313..98bd5719fdf5 100644
--- a/drivers/media/platform/s3c-camif/camif-core.c
+++ b/drivers/media/platform/s3c-camif/camif-core.c
@@ -317,7 +317,7 @@ static int camif_media_dev_init(struct camif_dev *camif)
ip_rev == S3C6410_CAMIF_IP_REV ? "6410" : "244X");
strlcpy(md->bus_info, "platform", sizeof(md->bus_info));
md->hw_revision = ip_rev;
- md->driver_version = KERNEL_VERSION(1, 0, 0);
+ md->driver_version = MEDIA_REVISION(1, 0, 0);
md->dev = camif->dev;
--
2.12.3
Use MEDIA_VERSION instead of KERNEL_VERSION to encode the driver
version of the Atom ISP driver.
Signed-off-by: Johannes Thumshirn <[email protected]>
---
drivers/staging/media/atomisp/include/linux/atomisp.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/media/atomisp/include/linux/atomisp.h b/drivers/staging/media/atomisp/include/linux/atomisp.h
index 35865462ccf9..0b664ee6f825 100644
--- a/drivers/staging/media/atomisp/include/linux/atomisp.h
+++ b/drivers/staging/media/atomisp/include/linux/atomisp.h
@@ -30,9 +30,9 @@
/* struct media_device_info.driver_version */
#define ATOMISP_CSS_VERSION_MASK 0x00ffffff
-#define ATOMISP_CSS_VERSION_15 KERNEL_VERSION(1, 5, 0)
-#define ATOMISP_CSS_VERSION_20 KERNEL_VERSION(2, 0, 0)
-#define ATOMISP_CSS_VERSION_21 KERNEL_VERSION(2, 1, 0)
+#define ATOMISP_CSS_VERSION_15 MEDIA_REVISION(1, 5, 0)
+#define ATOMISP_CSS_VERSION_20 MEDIA_REVISION(2, 0, 0)
+#define ATOMISP_CSS_VERSION_21 MEDIA_REVISION(2, 1, 0)
/* struct media_device_info.hw_revision */
#define ATOMISP_HW_REVISION_MASK 0x0000ff00
--
2.12.3
Don't use the KERNEL_VERSION() macro for the v4l2 capabilities, use
MEDIA_REVISION instead.
Signed-off-by: Johannes Thumshirn <[email protected]>
---
drivers/video/fbdev/matrox/matroxfb_base.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/matrox/matroxfb_base.c b/drivers/video/fbdev/matrox/matroxfb_base.c
index 11eb094396ae..eb92a325033c 100644
--- a/drivers/video/fbdev/matrox/matroxfb_base.c
+++ b/drivers/video/fbdev/matrox/matroxfb_base.c
@@ -113,6 +113,7 @@
#include <linux/interrupt.h>
#include <linux/slab.h>
#include <linux/uaccess.h>
+#include <linux/media.h>
#ifdef CONFIG_PPC_PMAC
#include <asm/machdep.h>
@@ -1091,7 +1092,7 @@ static int matroxfb_ioctl(struct fb_info *info,
strcpy(r.driver, "matroxfb");
strcpy(r.card, "Matrox");
sprintf(r.bus_info, "PCI:%s", pci_name(minfo->pcidev));
- r.version = KERNEL_VERSION(1,0,0);
+ r.version = MEDIA_REVISION(1, 0, 0);
r.capabilities = V4L2_CAP_VIDEO_OUTPUT;
if (copy_to_user(argp, &r, sizeof(r)))
return -EFAULT;
--
2.12.3
Update the documentation to introduce the use of MEDIA_REVISON instead
of KERNEL_VERSION for the verison triplets of a media drivers hardware
revision or driver version.
Signed-off-by: Johannes Thumshirn <[email protected]>
---
Documentation/media/uapi/cec/cec-ioc-adap-g-caps.rst | 2 +-
Documentation/media/uapi/mediactl/media-ioc-device-info.rst | 4 ++--
Documentation/media/uapi/v4l/vidioc-querycap.rst | 6 +++---
include/media/media-device.h | 5 ++---
4 files changed, 8 insertions(+), 9 deletions(-)
diff --git a/Documentation/media/uapi/cec/cec-ioc-adap-g-caps.rst b/Documentation/media/uapi/cec/cec-ioc-adap-g-caps.rst
index a0e961f11017..749054f11c77 100644
--- a/Documentation/media/uapi/cec/cec-ioc-adap-g-caps.rst
+++ b/Documentation/media/uapi/cec/cec-ioc-adap-g-caps.rst
@@ -56,7 +56,7 @@ returns the information to the application. The ioctl never fails.
:ref:`cec-capabilities`.
* - __u32
- ``version``
- - CEC Framework API version, formatted with the ``KERNEL_VERSION()``
+ - CEC Framework API version, formatted with the ``MEDIA_REVISION()``
macro.
diff --git a/Documentation/media/uapi/mediactl/media-ioc-device-info.rst b/Documentation/media/uapi/mediactl/media-ioc-device-info.rst
index f690f9afc470..7a18cb6dbe84 100644
--- a/Documentation/media/uapi/mediactl/media-ioc-device-info.rst
+++ b/Documentation/media/uapi/mediactl/media-ioc-device-info.rst
@@ -96,7 +96,7 @@ ioctl never fails.
- ``media_version``
- - Media API version, formatted with the ``KERNEL_VERSION()`` macro.
+ - Media API version, formatted with the ``MEDIA_REVISION()`` macro.
- .. row 6
@@ -113,7 +113,7 @@ ioctl never fails.
- ``driver_version``
- Media device driver version, formatted with the
- ``KERNEL_VERSION()`` macro. Together with the ``driver`` field
+ ``MEDIA_REVISION()`` macro. Together with the ``driver`` field
this identifies a particular driver.
- .. row 8
diff --git a/Documentation/media/uapi/v4l/vidioc-querycap.rst b/Documentation/media/uapi/v4l/vidioc-querycap.rst
index 12e0d9a63cd8..b66d9caa7211 100644
--- a/Documentation/media/uapi/v4l/vidioc-querycap.rst
+++ b/Documentation/media/uapi/v4l/vidioc-querycap.rst
@@ -90,13 +90,13 @@ specification the ioctl returns an ``EINVAL`` error code.
example, a stable or distribution-modified kernel uses the V4L2
stack from a newer kernel.
- The version number is formatted using the ``KERNEL_VERSION()``
+ The version number is formatted using the ``MEDIA_REVISION()``
macro:
* - :cspan:`2`
- ``#define KERNEL_VERSION(a,b,c) (((a) << 16) + ((b) << 8) + (c))``
+ ``#define MEDIA_REVISION(a,b,c) (((a) << 16) + ((b) << 8) + (c))``
- ``__u32 version = KERNEL_VERSION(0, 8, 1);``
+ ``__u32 version = MEDIA_REVISION(0, 8, 1);``
``printf ("Version: %u.%u.%u\\n",``
diff --git a/include/media/media-device.h b/include/media/media-device.h
index 6896266031b9..6b3057266ad1 100644
--- a/include/media/media-device.h
+++ b/include/media/media-device.h
@@ -247,9 +247,9 @@ void media_device_cleanup(struct media_device *mdev);
*
* - &media_entity.hw_revision is the hardware device revision in a
* driver-specific format. When possible the revision should be formatted
- * with the KERNEL_VERSION() macro.
+ * with the MEDIA_REVISION() macro.
*
- * - &media_entity.driver_version is formatted with the KERNEL_VERSION()
+ * - &media_entity.driver_version is formatted with the MEDIA_REVISION()
* macro. The version minor must be incremented when new features are added
* to the userspace API without breaking binary compatibility. The version
* major must be incremented when binary compatibility is broken.
@@ -265,7 +265,6 @@ void media_device_cleanup(struct media_device *mdev);
int __must_check __media_device_register(struct media_device *mdev,
struct module *owner);
-
/**
* media_device_register() - Registers a media device element
*
--
2.12.3
Em Wed, 21 Jun 2017 10:08:05 +0200
Johannes Thumshirn <[email protected]> escreveu:
> Currently the media subsystem has a very creative abuse of the
> KERNEL_VERSION macro to encode an arbitrary version triplet for media
> drivers and device hardware revisions.
>
> This series introduces a new macro called MEDIA_REVISION which encodes
> a version triplet like KERNEL_VERSION does, but clearly has media
> centric semantics and doesn't fool someone into thinking specific
> parts are defined for a specific kernel version only like in out of
> tree drivers.
Sorry, but I can't see any advantage on it. On the downside, it
includes the media controller header file (media.h) where it
is not needed.
>
> Johannes Thumshirn (7):
> [media] media: introduce MEDIA_REVISION macro
> video: fbdev: don't use KERNEL_VERSION macro for MEDIA_REVISION
> [media] media: document the use of MEDIA_REVISION instead of
> KERNEL_VERSION
> [media] cx25821: use MEDIA_REVISION instead of KERNEL_VERSION
> [media] media: s3c-camif: Use MEDIA_REVISON instead of KERNEL_VERSION
> [media] media: bcm2048: use MEDIA_REVISION isntead of KERNEL_VERSION
> staging/atomisp: use MEDIA_VERSION instead of KERNEL_VERSION
That's said, some of the above shouldn't be using KERNEL_VERSION
at all. The V4L2 core sets the version already. So, drivers like
cx25821, s3c-camif, bcm2048 and atomisp are likely doing the wrong
thing.
Thanks,
Mauro
On Sat, Jun 24, 2017 at 05:15:07PM -0300, Mauro Carvalho Chehab wrote:
> Sorry, but I can't see any advantage on it. On the downside, it
> includes the media controller header file (media.h) where it
> is not needed.
My reasoning was the differences in semantics. KERNEL_VERSION() is for
encoding the kernel's version triplet not a API or Hardware or whatever
version. Other subsystems do this as well, for instance in NVMe we have the
NVME_VS() macro which is used to encode the NVMe Spec compliance from a human
readable form to the hardware's u32. Also KERNEL_VERISON() shouldn't have
in-tree users IMHO. Yes there is _one_ other user of it in-tree which is EXT4
and I already talked to Jan Kara about it and we decided to leave it in until
4.20.
Byte,
Johannes
--
Johannes Thumshirn Storage
[email protected] +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
On Thu, 29 Jun 2017 11:42:59 +0200
Johannes Thumshirn <[email protected]> wrote:
> On Sat, Jun 24, 2017 at 05:15:07PM -0300, Mauro Carvalho Chehab wrote:
> > Sorry, but I can't see any advantage on it. On the downside, it
> > includes the media controller header file (media.h) where it
> > is not needed.
>
> My reasoning was the differences in semantics. KERNEL_VERSION() is for
> encoding the kernel's version triplet not a API or Hardware or whatever
> version. Other subsystems do this as well, for instance in NVMe we have the
> NVME_VS() macro which is used to encode the NVMe Spec compliance from a human
> readable form to the hardware's u32. Also KERNEL_VERISON() shouldn't have
> in-tree users IMHO. Yes there is _one_ other user of it in-tree which is EXT4
> and I already talked to Jan Kara about it and we decided to leave it in until
> 4.20.
>
> Byte,
> Johannes
If you read Linus's comments on version.
Driver version is meaningless and there is a desire to rip it out of all
drivers. The reason is that drivers must always behave the same, i.e you
can't use version to change API/ABI behavior.
Any upstream driver should never use KERNEL_VERSION().
On Thu, Jun 29, 2017 at 10:01:05AM -0700, Stephen Hemminger wrote:
> If you read Linus's comments on version.
> Driver version is meaningless and there is a desire to rip it out of all
> drivers. The reason is that drivers must always behave the same, i.e you
> can't use version to change API/ABI behavior.
Indeed this causes more harm than good. We had support calls regarding the
mlx4 driver because of not incremented MODLE_VERSION()s. If we follow your
and Linus' path we shouldn't just get rid of the KERNEL_VERSION() usage
in media and replace it with a new version, but kill all the versioning
stuff out of media (and others) except for maybe the HW version.
> Any upstream driver should never use KERNEL_VERSION().
Exactly my reasoning.
--
Johannes Thumshirn Storage
[email protected] +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850