Skip to contentSkip to conversation

media.h: increase the spacing between function ranges

PATCH In review Hans Verkuil 3 participants 4 messages Feb 11, 2016 – Feb 29, 2016

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 media.h: increase the spacing between function ranges

Each function range is quite narrow and especially for connectors this will pose a problem. Increase the function ranges while we still can and move the connector range to the end so that range is practically limitless.

---
 include/uapi/linux/media.h | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)
include/uapi/linux/media.h +15 -15
index c9eb42a..a300a33 100644
@@ -72,21 +72,11 @@ struct media_device_info {
 #define MEDIA_ENT_F_DTV_NET_DECAP	(MEDIA_ENT_F_BASE + 4)
 
 /*
- * Connectors
- */
-/* It is a responsibility of the entity drivers to add connectors and links */
-#define MEDIA_ENT_F_CONN_RF		(MEDIA_ENT_F_BASE + 21)
-#define MEDIA_ENT_F_CONN_SVIDEO		(MEDIA_ENT_F_BASE + 22)
-#define MEDIA_ENT_F_CONN_COMPOSITE	(MEDIA_ENT_F_BASE + 23)
-/* For internal test signal generators and other debug connectors */
-#define MEDIA_ENT_F_CONN_TEST		(MEDIA_ENT_F_BASE + 24)
-
-/*
  * I/O entities
  */
-#define MEDIA_ENT_F_IO_DTV  		(MEDIA_ENT_F_BASE + 31)
-#define MEDIA_ENT_F_IO_VBI  		(MEDIA_ENT_F_BASE + 32)
-#define MEDIA_ENT_F_IO_SWRADIO		(MEDIA_ENT_F_BASE + 33)
+#define MEDIA_ENT_F_IO_DTV		(MEDIA_ENT_F_BASE + 1001)
+#define MEDIA_ENT_F_IO_VBI		(MEDIA_ENT_F_BASE + 1002)
+#define MEDIA_ENT_F_IO_SWRADIO		(MEDIA_ENT_F_BASE + 1003)
 
 /*
  * Analog TV IF-PLL decoders
@@ -94,8 +84,18 @@ struct media_device_info {
  * It is a responsibility of the master/bridge drivers to create links
  * for MEDIA_ENT_F_IF_VID_DECODER and MEDIA_ENT_F_IF_AUD_DECODER.
  */
-#define MEDIA_ENT_F_IF_VID_DECODER	(MEDIA_ENT_F_BASE + 41)
-#define MEDIA_ENT_F_IF_AUD_DECODER	(MEDIA_ENT_F_BASE + 42)
+#define MEDIA_ENT_F_IF_VID_DECODER	(MEDIA_ENT_F_BASE + 2001)
+#define MEDIA_ENT_F_IF_AUD_DECODER	(MEDIA_ENT_F_BASE + 2002)
+
+/*
+ * Connectors
+ */
+/* It is a responsibility of the entity drivers to add connectors and links */
+#define MEDIA_ENT_F_CONN_RF		(MEDIA_ENT_F_BASE + 10001)
+#define MEDIA_ENT_F_CONN_SVIDEO		(MEDIA_ENT_F_BASE + 10002)
+#define MEDIA_ENT_F_CONN_COMPOSITE	(MEDIA_ENT_F_BASE + 10003)
+/* For internal test signal generators and other debug connectors */
+#define MEDIA_ENT_F_CONN_TEST		(MEDIA_ENT_F_BASE + 10004)
 
 /*
  * Don't touch on those. The ranges MEDIA_ENT_F_OLD_BASE and
Signature · 1 line
2.7.0
#1 lore raw

Hi Hans,

Quoted 49 lines · Hans Verkuil

Each function range is quite narrow and especially for connectors this will pose a problem. Increase the function ranges while we still can and move the connector range to the end so that range is practically limitless.

---
 include/uapi/linux/media.h | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)
include/uapi/linux/media.h +8 -15
index c9eb42a..a300a33 100644
@@ -72,21 +72,11 @@ struct media_device_info {
 #define MEDIA_ENT_F_DTV_NET_DECAP	(MEDIA_ENT_F_BASE + 4)
 
 /*
- * Connectors
- */
-/* It is a responsibility of the entity drivers to add connectors and links */
-#define MEDIA_ENT_F_CONN_RF		(MEDIA_ENT_F_BASE + 21)
-#define MEDIA_ENT_F_CONN_SVIDEO		(MEDIA_ENT_F_BASE + 22)
-#define MEDIA_ENT_F_CONN_COMPOSITE	(MEDIA_ENT_F_BASE + 23)
-/* For internal test signal generators and other debug connectors */
-#define MEDIA_ENT_F_CONN_TEST		(MEDIA_ENT_F_BASE + 24)
-
-/*
  * I/O entities
  */
-#define MEDIA_ENT_F_IO_DTV  		(MEDIA_ENT_F_BASE + 31)
-#define MEDIA_ENT_F_IO_VBI  		(MEDIA_ENT_F_BASE + 32)
-#define MEDIA_ENT_F_IO_SWRADIO		(MEDIA_ENT_F_BASE + 33)
+#define MEDIA_ENT_F_IO_DTV		(MEDIA_ENT_F_BASE + 1001)
+#define MEDIA_ENT_F_IO_VBI		(MEDIA_ENT_F_BASE + 1002)
+#define MEDIA_ENT_F_IO_SWRADIO		(MEDIA_ENT_F_BASE + 1003)
 
 /*
  * Analog TV IF-PLL decoders
@@ -94,8 +84,18 @@ struct media_device_info {
  * It is a responsibility of the master/bridge drivers to create links
  * for MEDIA_ENT_F_IF_VID_DECODER and MEDIA_ENT_F_IF_AUD_DECODER.
  */
-#define MEDIA_ENT_F_IF_VID_DECODER	(MEDIA_ENT_F_BASE + 41)
-#define MEDIA_ENT_F_IF_AUD_DECODER	(MEDIA_ENT_F_BASE + 42)
+#define MEDIA_ENT_F_IF_VID_DECODER	(MEDIA_ENT_F_BASE + 2001)
+#define MEDIA_ENT_F_IF_AUD_DECODER	(MEDIA_ENT_F_BASE + 2002)
+
+/*
+ * Connectors

include/uapi/linux/media.h struct media_device_info { · line 101

Quoted 10 lines
include/uapi/linux/media.h +7 -0
+ */
+/* It is a responsibility of the entity drivers to add connectors and links */
+#define MEDIA_ENT_F_CONN_RF		(MEDIA_ENT_F_BASE + 10001)
+#define MEDIA_ENT_F_CONN_SVIDEO		(MEDIA_ENT_F_BASE + 10002)
+#define MEDIA_ENT_F_CONN_COMPOSITE	(MEDIA_ENT_F_BASE + 10003)
+/* For internal test signal generators and other debug connectors */
+#define MEDIA_ENT_F_CONN_TEST		(MEDIA_ENT_F_BASE + 10004)
 
 /*
  * Don't touch on those. The ranges MEDIA_ENT_F_OLD_BASE and

We have MEDIA_ENT_F_BASE used for all of these at the moment while we have plenty of room in the 32-bit space. How about adding a new base for each group, after MEDIA_ENT_F_OLD_SUBDEV_BASE? Each range would have 2^16 items.

What do you think?

Signature · 4 lines
Kind regards,

Sakari Ailus
e-mail: [email protected]	XMPP: [email protected]
maintainer
#2 lore raw
In reply to Hans Verkuil

Hi Hans,

Thank you for the patch.

Quoted 5 lines · On Thursday 11 February 2016 15:28:55 Hans Verkuil wrote:

Each function range is quite narrow and especially for connectors this will pose a problem. Increase the function ranges while we still can and move the connector range to the end so that range is practically limitless.

It looks like this got applied without taking Sakari's comments into account. Furthermore, I believe the IDs you use below will be confusing, as the base is a hex value and the offset a decimal value. It would making debugging much easier if both used hex values.

Quoted 54 lines
---
 include/uapi/linux/media.h | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)
include/uapi/linux/media.h +14 -14
index c9eb42a..a300a33 100644
@@ -72,21 +72,11 @@ struct media_device_info {
 #define MEDIA_ENT_F_DTV_NET_DECAP	(MEDIA_ENT_F_BASE + 4)
 
 /*
- * Connectors
- */
-/* It is a responsibility of the entity drivers to add connectors and links
 */ -#define MEDIA_ENT_F_CONN_RF		(MEDIA_ENT_F_BASE + 21)
-#define MEDIA_ENT_F_CONN_SVIDEO		(MEDIA_ENT_F_BASE + 22)
-#define MEDIA_ENT_F_CONN_COMPOSITE	(MEDIA_ENT_F_BASE + 23)
-/* For internal test signal generators and other debug connectors */
-#define MEDIA_ENT_F_CONN_TEST		(MEDIA_ENT_F_BASE + 24)
-
-/*
  * I/O entities
  */
-#define MEDIA_ENT_F_IO_DTV  		(MEDIA_ENT_F_BASE + 31)
-#define MEDIA_ENT_F_IO_VBI  		(MEDIA_ENT_F_BASE + 32)
-#define MEDIA_ENT_F_IO_SWRADIO		(MEDIA_ENT_F_BASE + 33)
+#define MEDIA_ENT_F_IO_DTV		(MEDIA_ENT_F_BASE + 1001)
+#define MEDIA_ENT_F_IO_VBI		(MEDIA_ENT_F_BASE + 1002)
+#define MEDIA_ENT_F_IO_SWRADIO		(MEDIA_ENT_F_BASE + 1003)
 
 /*
  * Analog TV IF-PLL decoders
@@ -94,8 +84,18 @@ struct media_device_info {
  * It is a responsibility of the master/bridge drivers to create links
  * for MEDIA_ENT_F_IF_VID_DECODER and MEDIA_ENT_F_IF_AUD_DECODER.
  */
-#define MEDIA_ENT_F_IF_VID_DECODER	(MEDIA_ENT_F_BASE + 41)
-#define MEDIA_ENT_F_IF_AUD_DECODER	(MEDIA_ENT_F_BASE + 42)
+#define MEDIA_ENT_F_IF_VID_DECODER	(MEDIA_ENT_F_BASE + 2001)
+#define MEDIA_ENT_F_IF_AUD_DECODER	(MEDIA_ENT_F_BASE + 2002)
+
+/*
+ * Connectors
+ */
+/* It is a responsibility of the entity drivers to add connectors and links
 */ +#define MEDIA_ENT_F_CONN_RF		(MEDIA_ENT_F_BASE + 10001)
+#define MEDIA_ENT_F_CONN_SVIDEO		(MEDIA_ENT_F_BASE + 10002)
+#define MEDIA_ENT_F_CONN_COMPOSITE	(MEDIA_ENT_F_BASE + 10003)
+/* For internal test signal generators and other debug connectors */
+#define MEDIA_ENT_F_CONN_TEST		(MEDIA_ENT_F_BASE + 10004)
 
 /*
  * Don't touch on those. The ranges MEDIA_ENT_F_OLD_BASE and
Signature · 3 lines
Regards,

Laurent Pinchart
#3 lore raw
Quoted 4 lines · Laurent Pinchart

Hi Hans,

Thank you for the patch.

Quoted 8 lines
Quoted 5 lines · On Thursday 11 February 2016 15:28:55 Hans Verkuil wrote:

Each function range is quite narrow and especially for connectors this will pose a problem. Increase the function ranges while we still can and move the connector range to the end so that range is practically limitless.

It looks like this got applied without taking Sakari's comments into account.

I saw that, but I think the ranges are large enough so I didn't change it.

Quoted 3 lines

Furthermore, I believe the IDs you use below will be confusing, as the base is a hex value and the offset a decimal value. It would making debugging much easier if both used hex values.

That's a good point. I'll make a patch to convert it to hex (add 0x).

Regards,

Hans

Quoted 56 lines
Quoted 55 lines
---
 include/uapi/linux/media.h | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)
include/uapi/linux/media.h +14 -14
index c9eb42a..a300a33 100644
@@ -72,21 +72,11 @@ struct media_device_info {
 #define MEDIA_ENT_F_DTV_NET_DECAP	(MEDIA_ENT_F_BASE + 4)
 
 /*
- * Connectors
- */
-/* It is a responsibility of the entity drivers to add connectors and links
 */ -#define MEDIA_ENT_F_CONN_RF		(MEDIA_ENT_F_BASE + 21)
-#define MEDIA_ENT_F_CONN_SVIDEO		(MEDIA_ENT_F_BASE + 22)
-#define MEDIA_ENT_F_CONN_COMPOSITE	(MEDIA_ENT_F_BASE + 23)
-/* For internal test signal generators and other debug connectors */
-#define MEDIA_ENT_F_CONN_TEST		(MEDIA_ENT_F_BASE + 24)
-
-/*
  * I/O entities
  */
-#define MEDIA_ENT_F_IO_DTV  		(MEDIA_ENT_F_BASE + 31)
-#define MEDIA_ENT_F_IO_VBI  		(MEDIA_ENT_F_BASE + 32)
-#define MEDIA_ENT_F_IO_SWRADIO		(MEDIA_ENT_F_BASE + 33)
+#define MEDIA_ENT_F_IO_DTV		(MEDIA_ENT_F_BASE + 1001)
+#define MEDIA_ENT_F_IO_VBI		(MEDIA_ENT_F_BASE + 1002)
+#define MEDIA_ENT_F_IO_SWRADIO		(MEDIA_ENT_F_BASE + 1003)
 
 /*
  * Analog TV IF-PLL decoders
@@ -94,8 +84,18 @@ struct media_device_info {
  * It is a responsibility of the master/bridge drivers to create links
  * for MEDIA_ENT_F_IF_VID_DECODER and MEDIA_ENT_F_IF_AUD_DECODER.
  */
-#define MEDIA_ENT_F_IF_VID_DECODER	(MEDIA_ENT_F_BASE + 41)
-#define MEDIA_ENT_F_IF_AUD_DECODER	(MEDIA_ENT_F_BASE + 42)
+#define MEDIA_ENT_F_IF_VID_DECODER	(MEDIA_ENT_F_BASE + 2001)
+#define MEDIA_ENT_F_IF_AUD_DECODER	(MEDIA_ENT_F_BASE + 2002)
+
+/*
+ * Connectors
+ */
+/* It is a responsibility of the entity drivers to add connectors and links
 */ +#define MEDIA_ENT_F_CONN_RF		(MEDIA_ENT_F_BASE + 10001)
+#define MEDIA_ENT_F_CONN_SVIDEO		(MEDIA_ENT_F_BASE + 10002)
+#define MEDIA_ENT_F_CONN_COMPOSITE	(MEDIA_ENT_F_BASE + 10003)
+/* For internal test signal generators and other debug connectors */
+#define MEDIA_ENT_F_CONN_TEST		(MEDIA_ENT_F_BASE + 10004)
 
 /*
  * Don't touch on those. The ranges MEDIA_ENT_F_OLD_BASE and