* [PATCH 0/3] Random cleanups
@ 2023-05-05 20:50 Sakari Ailus
2023-05-05 20:50 ` [PATCH 1/3] media: mc: Make media_get_pad_index() use pad type flag Sakari Ailus
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Sakari Ailus @ 2023-05-05 20:50 UTC (permalink / raw)
To: linux-media; +Cc: laurent.pinchart, tomi.valkeinen, bingbu.cao
Hi all,
These three patches are seemingly random cleanups but are a pre-requisite
for another set for supporting generic metadata formats. I'll post that
set separately. These three should be merged independently though..
Sakari Ailus (3):
media: mc: Make media_get_pad_index() use pad type flag
media: Documentation: Rename meta format files
media: uapi: Use unsigned int values for assigning bits in u32 fields
.../userspace-api/media/v4l/meta-formats.rst | 14 +++----
...{pixfmt-meta-d4xx.rst => metafmt-d4xx.rst} | 0
...-intel-ipu3.rst => metafmt-intel-ipu3.rst} | 0
...fmt-meta-rkisp1.rst => metafmt-rkisp1.rst} | 0
.../{pixfmt-meta-uvc.rst => metafmt-uvc.rst} | 0
...ixfmt-meta-vivid.rst => metafmt-vivid.rst} | 0
...meta-vsp1-hgo.rst => metafmt-vsp1-hgo.rst} | 0
...meta-vsp1-hgt.rst => metafmt-vsp1-hgt.rst} | 0
MAINTAINERS | 4 +-
drivers/media/dvb-core/dvbdev.c | 4 +-
drivers/media/mc/mc-entity.c | 16 +++-----
drivers/media/usb/au0828/au0828-core.c | 2 +-
drivers/media/v4l2-core/v4l2-mc.c | 38 ++++++++++++-------
include/media/media-entity.h | 4 +-
include/uapi/linux/media.h | 28 +++++++-------
15 files changed, 58 insertions(+), 52 deletions(-)
rename Documentation/userspace-api/media/v4l/{pixfmt-meta-d4xx.rst => metafmt-d4xx.rst} (100%)
rename Documentation/userspace-api/media/v4l/{pixfmt-meta-intel-ipu3.rst => metafmt-intel-ipu3.rst} (100%)
rename Documentation/userspace-api/media/v4l/{pixfmt-meta-rkisp1.rst => metafmt-rkisp1.rst} (100%)
rename Documentation/userspace-api/media/v4l/{pixfmt-meta-uvc.rst => metafmt-uvc.rst} (100%)
rename Documentation/userspace-api/media/v4l/{pixfmt-meta-vivid.rst => metafmt-vivid.rst} (100%)
rename Documentation/userspace-api/media/v4l/{pixfmt-meta-vsp1-hgo.rst => metafmt-vsp1-hgo.rst} (100%)
rename Documentation/userspace-api/media/v4l/{pixfmt-meta-vsp1-hgt.rst => metafmt-vsp1-hgt.rst} (100%)
--
2.30.2
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/3] media: mc: Make media_get_pad_index() use pad type flag
2023-05-05 20:50 [PATCH 0/3] Random cleanups Sakari Ailus
@ 2023-05-05 20:50 ` Sakari Ailus
2023-05-06 11:25 ` Laurent Pinchart
2023-05-05 20:51 ` [PATCH 2/3] media: Documentation: Rename meta format files Sakari Ailus
2023-05-05 20:51 ` [PATCH 3/3] media: uapi: Use unsigned int values for assigning bits in u32 fields Sakari Ailus
2 siblings, 1 reply; 12+ messages in thread
From: Sakari Ailus @ 2023-05-05 20:50 UTC (permalink / raw)
To: linux-media; +Cc: laurent.pinchart, tomi.valkeinen, bingbu.cao
Use the pad flag specifying the pad type instead of a boolean in
preparation for internal source pads.
Also make the loop variable unsigned.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
drivers/media/dvb-core/dvbdev.c | 4 +--
drivers/media/mc/mc-entity.c | 16 ++++-------
drivers/media/usb/au0828/au0828-core.c | 2 +-
drivers/media/v4l2-core/v4l2-mc.c | 38 +++++++++++++++++---------
include/media/media-entity.h | 4 +--
5 files changed, 35 insertions(+), 29 deletions(-)
diff --git a/drivers/media/dvb-core/dvbdev.c b/drivers/media/dvb-core/dvbdev.c
index 0ed087caf7f3..0091b5375e45 100644
--- a/drivers/media/dvb-core/dvbdev.c
+++ b/drivers/media/dvb-core/dvbdev.c
@@ -707,7 +707,7 @@ int dvb_create_media_graph(struct dvb_adapter *adap,
MEDIA_LNK_FL_ENABLED,
false);
} else {
- pad_sink = media_get_pad_index(tuner, true,
+ pad_sink = media_get_pad_index(tuner, MEDIA_PAD_FL_SINK,
PAD_SIGNAL_ANALOG);
if (pad_sink < 0)
return -EINVAL;
@@ -725,7 +725,7 @@ int dvb_create_media_graph(struct dvb_adapter *adap,
if (ntuner && ndemod) {
/* NOTE: first found tuner source pad presumed correct */
- pad_source = media_get_pad_index(tuner, false,
+ pad_source = media_get_pad_index(tuner, MEDIA_PAD_FL_SOURCE,
PAD_SIGNAL_ANALOG);
if (pad_source < 0)
return -EINVAL;
diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c
index e7216a985ba6..ef34ddd715c9 100644
--- a/drivers/media/mc/mc-entity.c
+++ b/drivers/media/mc/mc-entity.c
@@ -1052,25 +1052,19 @@ static void __media_entity_remove_link(struct media_entity *entity,
kfree(link);
}
-int media_get_pad_index(struct media_entity *entity, bool is_sink,
+int media_get_pad_index(struct media_entity *entity, u32 pad_type,
enum media_pad_signal_type sig_type)
{
- int i;
- bool pad_is_sink;
+ unsigned int i;
if (!entity)
return -EINVAL;
for (i = 0; i < entity->num_pads; i++) {
- if (entity->pads[i].flags & MEDIA_PAD_FL_SINK)
- pad_is_sink = true;
- else if (entity->pads[i].flags & MEDIA_PAD_FL_SOURCE)
- pad_is_sink = false;
- else
- continue; /* This is an error! */
-
- if (pad_is_sink != is_sink)
+ if ((entity->pads[i].flags &
+ (MEDIA_PAD_FL_SINK | MEDIA_PAD_FL_SOURCE)) != pad_type)
continue;
+
if (entity->pads[i].sig_type == sig_type)
return i;
}
diff --git a/drivers/media/usb/au0828/au0828-core.c b/drivers/media/usb/au0828/au0828-core.c
index b3a09d3ac7d2..1e246b47766d 100644
--- a/drivers/media/usb/au0828/au0828-core.c
+++ b/drivers/media/usb/au0828/au0828-core.c
@@ -250,7 +250,7 @@ static void au0828_media_graph_notify(struct media_entity *new,
create_link:
if (decoder && mixer) {
- ret = media_get_pad_index(decoder, false,
+ ret = media_get_pad_index(decoder, MEDIA_PAD_FL_SOURCE,
PAD_SIGNAL_AUDIO);
if (ret >= 0)
ret = media_create_pad_link(decoder, ret,
diff --git a/drivers/media/v4l2-core/v4l2-mc.c b/drivers/media/v4l2-core/v4l2-mc.c
index bf0c18100664..209a7efd08fe 100644
--- a/drivers/media/v4l2-core/v4l2-mc.c
+++ b/drivers/media/v4l2-core/v4l2-mc.c
@@ -105,9 +105,11 @@ int v4l2_mc_create_media_graph(struct media_device *mdev)
/* Link the tuner and IF video output pads */
if (tuner) {
if (if_vid) {
- pad_source = media_get_pad_index(tuner, false,
+ pad_source = media_get_pad_index(tuner,
+ MEDIA_PAD_FL_SOURCE,
PAD_SIGNAL_ANALOG);
- pad_sink = media_get_pad_index(if_vid, true,
+ pad_sink = media_get_pad_index(if_vid,
+ MEDIA_PAD_FL_SINK,
PAD_SIGNAL_ANALOG);
if (pad_source < 0 || pad_sink < 0) {
dev_warn(mdev->dev, "Couldn't get tuner and/or PLL pad(s): (%d, %d)\n",
@@ -122,9 +124,11 @@ int v4l2_mc_create_media_graph(struct media_device *mdev)
return ret;
}
- pad_source = media_get_pad_index(if_vid, false,
+ pad_source = media_get_pad_index(if_vid,
+ MEDIA_PAD_FL_SOURCE,
PAD_SIGNAL_ANALOG);
- pad_sink = media_get_pad_index(decoder, true,
+ pad_sink = media_get_pad_index(decoder,
+ MEDIA_PAD_FL_SINK,
PAD_SIGNAL_ANALOG);
if (pad_source < 0 || pad_sink < 0) {
dev_warn(mdev->dev, "get decoder and/or PLL pad(s): (%d, %d)\n",
@@ -139,9 +143,11 @@ int v4l2_mc_create_media_graph(struct media_device *mdev)
return ret;
}
} else {
- pad_source = media_get_pad_index(tuner, false,
+ pad_source = media_get_pad_index(tuner,
+ MEDIA_PAD_FL_SOURCE,
PAD_SIGNAL_ANALOG);
- pad_sink = media_get_pad_index(decoder, true,
+ pad_sink = media_get_pad_index(decoder,
+ MEDIA_PAD_FL_SINK,
PAD_SIGNAL_ANALOG);
if (pad_source < 0 || pad_sink < 0) {
dev_warn(mdev->dev, "couldn't get tuner and/or decoder pad(s): (%d, %d)\n",
@@ -156,9 +162,11 @@ int v4l2_mc_create_media_graph(struct media_device *mdev)
}
if (if_aud) {
- pad_source = media_get_pad_index(tuner, false,
+ pad_source = media_get_pad_index(tuner,
+ MEDIA_PAD_FL_SOURCE,
PAD_SIGNAL_AUDIO);
- pad_sink = media_get_pad_index(if_aud, true,
+ pad_sink = media_get_pad_index(if_aud,
+ MEDIA_PAD_FL_SINK,
PAD_SIGNAL_AUDIO);
if (pad_source < 0 || pad_sink < 0) {
dev_warn(mdev->dev, "couldn't get tuner and/or decoder pad(s) for audio: (%d, %d)\n",
@@ -180,7 +188,8 @@ int v4l2_mc_create_media_graph(struct media_device *mdev)
/* Create demod to V4L, VBI and SDR radio links */
if (io_v4l) {
- pad_source = media_get_pad_index(decoder, false, PAD_SIGNAL_DV);
+ pad_source = media_get_pad_index(decoder, MEDIA_PAD_FL_SOURCE,
+ PAD_SIGNAL_DV);
if (pad_source < 0) {
dev_warn(mdev->dev, "couldn't get decoder output pad for V4L I/O\n");
return -EINVAL;
@@ -195,7 +204,8 @@ int v4l2_mc_create_media_graph(struct media_device *mdev)
}
if (io_swradio) {
- pad_source = media_get_pad_index(decoder, false, PAD_SIGNAL_DV);
+ pad_source = media_get_pad_index(decoder, MEDIA_PAD_FL_SOURCE,
+ PAD_SIGNAL_DV);
if (pad_source < 0) {
dev_warn(mdev->dev, "couldn't get decoder output pad for SDR\n");
return -EINVAL;
@@ -210,7 +220,8 @@ int v4l2_mc_create_media_graph(struct media_device *mdev)
}
if (io_vbi) {
- pad_source = media_get_pad_index(decoder, false, PAD_SIGNAL_DV);
+ pad_source = media_get_pad_index(decoder, MEDIA_PAD_FL_SOURCE,
+ PAD_SIGNAL_DV);
if (pad_source < 0) {
dev_warn(mdev->dev, "couldn't get decoder output pad for VBI\n");
return -EINVAL;
@@ -231,7 +242,7 @@ int v4l2_mc_create_media_graph(struct media_device *mdev)
case MEDIA_ENT_F_CONN_RF:
if (!tuner)
continue;
- pad_sink = media_get_pad_index(tuner, true,
+ pad_sink = media_get_pad_index(tuner, MEDIA_PAD_FL_SINK,
PAD_SIGNAL_ANALOG);
if (pad_sink < 0) {
dev_warn(mdev->dev, "couldn't get tuner analog pad sink\n");
@@ -243,7 +254,8 @@ int v4l2_mc_create_media_graph(struct media_device *mdev)
break;
case MEDIA_ENT_F_CONN_SVIDEO:
case MEDIA_ENT_F_CONN_COMPOSITE:
- pad_sink = media_get_pad_index(decoder, true,
+ pad_sink = media_get_pad_index(decoder,
+ MEDIA_PAD_FL_SINK,
PAD_SIGNAL_ANALOG);
if (pad_sink < 0) {
dev_warn(mdev->dev, "couldn't get decoder analog pad sink\n");
diff --git a/include/media/media-entity.h b/include/media/media-entity.h
index 741f9c629c6f..e4f556911c3f 100644
--- a/include/media/media-entity.h
+++ b/include/media/media-entity.h
@@ -741,7 +741,7 @@ static inline void media_entity_cleanup(struct media_entity *entity) {}
* media_get_pad_index() - retrieves a pad index from an entity
*
* @entity: entity where the pads belong
- * @is_sink: true if the pad is a sink, false if it is a source
+ * @pad_type: the type of the pad, one of MEDIA_PAD_FL_* pad types
* @sig_type: type of signal of the pad to be search
*
* This helper function finds the first pad index inside an entity that
@@ -752,7 +752,7 @@ static inline void media_entity_cleanup(struct media_entity *entity) {}
* On success, return the pad number. If the pad was not found or the media
* entity is a NULL pointer, return -EINVAL.
*/
-int media_get_pad_index(struct media_entity *entity, bool is_sink,
+int media_get_pad_index(struct media_entity *entity, u32 pad_type,
enum media_pad_signal_type sig_type);
/**
--
2.30.2
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/3] media: Documentation: Rename meta format files
2023-05-05 20:50 [PATCH 0/3] Random cleanups Sakari Ailus
2023-05-05 20:50 ` [PATCH 1/3] media: mc: Make media_get_pad_index() use pad type flag Sakari Ailus
@ 2023-05-05 20:51 ` Sakari Ailus
2023-05-06 11:25 ` Laurent Pinchart
2023-05-05 20:51 ` [PATCH 3/3] media: uapi: Use unsigned int values for assigning bits in u32 fields Sakari Ailus
2 siblings, 1 reply; 12+ messages in thread
From: Sakari Ailus @ 2023-05-05 20:51 UTC (permalink / raw)
To: linux-media; +Cc: laurent.pinchart, tomi.valkeinen, bingbu.cao
Rename meta format files, using "metafmt" prefix instead of "pixfmt-meta".
These are metadata formats, not pixel formats.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: Bingbu Cao <bingbu.cao@intel.com>
Cc: Dafna Hirschfeld <dafna@fastmail.com>
---
.../userspace-api/media/v4l/meta-formats.rst | 14 +++++++-------
.../v4l/{pixfmt-meta-d4xx.rst => metafmt-d4xx.rst} | 0
...-meta-intel-ipu3.rst => metafmt-intel-ipu3.rst} | 0
.../{pixfmt-meta-rkisp1.rst => metafmt-rkisp1.rst} | 0
.../v4l/{pixfmt-meta-uvc.rst => metafmt-uvc.rst} | 0
.../{pixfmt-meta-vivid.rst => metafmt-vivid.rst} | 0
...xfmt-meta-vsp1-hgo.rst => metafmt-vsp1-hgo.rst} | 0
...xfmt-meta-vsp1-hgt.rst => metafmt-vsp1-hgt.rst} | 0
MAINTAINERS | 4 ++--
9 files changed, 9 insertions(+), 9 deletions(-)
rename Documentation/userspace-api/media/v4l/{pixfmt-meta-d4xx.rst => metafmt-d4xx.rst} (100%)
rename Documentation/userspace-api/media/v4l/{pixfmt-meta-intel-ipu3.rst => metafmt-intel-ipu3.rst} (100%)
rename Documentation/userspace-api/media/v4l/{pixfmt-meta-rkisp1.rst => metafmt-rkisp1.rst} (100%)
rename Documentation/userspace-api/media/v4l/{pixfmt-meta-uvc.rst => metafmt-uvc.rst} (100%)
rename Documentation/userspace-api/media/v4l/{pixfmt-meta-vivid.rst => metafmt-vivid.rst} (100%)
rename Documentation/userspace-api/media/v4l/{pixfmt-meta-vsp1-hgo.rst => metafmt-vsp1-hgo.rst} (100%)
rename Documentation/userspace-api/media/v4l/{pixfmt-meta-vsp1-hgt.rst => metafmt-vsp1-hgt.rst} (100%)
diff --git a/Documentation/userspace-api/media/v4l/meta-formats.rst b/Documentation/userspace-api/media/v4l/meta-formats.rst
index fff25357fe86..0bb61fc5bc00 100644
--- a/Documentation/userspace-api/media/v4l/meta-formats.rst
+++ b/Documentation/userspace-api/media/v4l/meta-formats.rst
@@ -12,10 +12,10 @@ These formats are used for the :ref:`metadata` interface only.
.. toctree::
:maxdepth: 1
- pixfmt-meta-d4xx
- pixfmt-meta-intel-ipu3
- pixfmt-meta-rkisp1
- pixfmt-meta-uvc
- pixfmt-meta-vsp1-hgo
- pixfmt-meta-vsp1-hgt
- pixfmt-meta-vivid
+ metafmt-d4xx
+ metafmt-intel-ipu3
+ metafmt-rkisp1
+ metafmt-uvc
+ metafmt-vsp1-hgo
+ metafmt-vsp1-hgt
+ metafmt-vivid
diff --git a/Documentation/userspace-api/media/v4l/pixfmt-meta-d4xx.rst b/Documentation/userspace-api/media/v4l/metafmt-d4xx.rst
similarity index 100%
rename from Documentation/userspace-api/media/v4l/pixfmt-meta-d4xx.rst
rename to Documentation/userspace-api/media/v4l/metafmt-d4xx.rst
diff --git a/Documentation/userspace-api/media/v4l/pixfmt-meta-intel-ipu3.rst b/Documentation/userspace-api/media/v4l/metafmt-intel-ipu3.rst
similarity index 100%
rename from Documentation/userspace-api/media/v4l/pixfmt-meta-intel-ipu3.rst
rename to Documentation/userspace-api/media/v4l/metafmt-intel-ipu3.rst
diff --git a/Documentation/userspace-api/media/v4l/pixfmt-meta-rkisp1.rst b/Documentation/userspace-api/media/v4l/metafmt-rkisp1.rst
similarity index 100%
rename from Documentation/userspace-api/media/v4l/pixfmt-meta-rkisp1.rst
rename to Documentation/userspace-api/media/v4l/metafmt-rkisp1.rst
diff --git a/Documentation/userspace-api/media/v4l/pixfmt-meta-uvc.rst b/Documentation/userspace-api/media/v4l/metafmt-uvc.rst
similarity index 100%
rename from Documentation/userspace-api/media/v4l/pixfmt-meta-uvc.rst
rename to Documentation/userspace-api/media/v4l/metafmt-uvc.rst
diff --git a/Documentation/userspace-api/media/v4l/pixfmt-meta-vivid.rst b/Documentation/userspace-api/media/v4l/metafmt-vivid.rst
similarity index 100%
rename from Documentation/userspace-api/media/v4l/pixfmt-meta-vivid.rst
rename to Documentation/userspace-api/media/v4l/metafmt-vivid.rst
diff --git a/Documentation/userspace-api/media/v4l/pixfmt-meta-vsp1-hgo.rst b/Documentation/userspace-api/media/v4l/metafmt-vsp1-hgo.rst
similarity index 100%
rename from Documentation/userspace-api/media/v4l/pixfmt-meta-vsp1-hgo.rst
rename to Documentation/userspace-api/media/v4l/metafmt-vsp1-hgo.rst
diff --git a/Documentation/userspace-api/media/v4l/pixfmt-meta-vsp1-hgt.rst b/Documentation/userspace-api/media/v4l/metafmt-vsp1-hgt.rst
similarity index 100%
rename from Documentation/userspace-api/media/v4l/pixfmt-meta-vsp1-hgt.rst
rename to Documentation/userspace-api/media/v4l/metafmt-vsp1-hgt.rst
diff --git a/MAINTAINERS b/MAINTAINERS
index e25ebb7c781b..546826109900 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10359,7 +10359,7 @@ L: linux-media@vger.kernel.org
S: Maintained
F: Documentation/admin-guide/media/ipu3.rst
F: Documentation/admin-guide/media/ipu3_rcb.svg
-F: Documentation/userspace-api/media/v4l/pixfmt-meta-intel-ipu3.rst
+F: Documentation/userspace-api/media/v4l/metafmt-intel-ipu3.rst
F: drivers/staging/media/ipu3/
INTEL IXP4XX CRYPTO SUPPORT
@@ -18026,7 +18026,7 @@ L: linux-rockchip@lists.infradead.org
S: Maintained
F: Documentation/admin-guide/media/rkisp1.rst
F: Documentation/devicetree/bindings/media/rockchip-isp1.yaml
-F: Documentation/userspace-api/media/v4l/pixfmt-meta-rkisp1.rst
+F: Documentation/userspace-api/media/v4l/metafmt-rkisp1.rst
F: drivers/media/platform/rockchip/rkisp1
F: include/uapi/linux/rkisp1-config.h
--
2.30.2
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 3/3] media: uapi: Use unsigned int values for assigning bits in u32 fields
2023-05-05 20:50 [PATCH 0/3] Random cleanups Sakari Ailus
2023-05-05 20:50 ` [PATCH 1/3] media: mc: Make media_get_pad_index() use pad type flag Sakari Ailus
2023-05-05 20:51 ` [PATCH 2/3] media: Documentation: Rename meta format files Sakari Ailus
@ 2023-05-05 20:51 ` Sakari Ailus
2023-05-06 11:32 ` Laurent Pinchart
2 siblings, 1 reply; 12+ messages in thread
From: Sakari Ailus @ 2023-05-05 20:51 UTC (permalink / raw)
To: linux-media; +Cc: laurent.pinchart, tomi.valkeinen, bingbu.cao
Use unsigned int values annoted by "U" for u32 fields. While this is a
good practice, there doesn't appear to be a bug that this patch would fix.
The patch has been generated using the following command:
perl -i -pe 's/\([0-9]+\K <</U <</g' -- include/uapi/linux/media.h
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
include/uapi/linux/media.h | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
index 3ddadaea849f..edb8dfef9eba 100644
--- a/include/uapi/linux/media.h
+++ b/include/uapi/linux/media.h
@@ -140,8 +140,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 (1U << 0)
+#define MEDIA_ENT_FL_CONNECTOR (1U << 1)
/* OR with the entity id value to find the next entity */
#define MEDIA_ENT_ID_FLAG_NEXT (1U << 31)
@@ -205,9 +205,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 (1U << 0)
+#define MEDIA_PAD_FL_SOURCE (1U << 1)
+#define MEDIA_PAD_FL_MUST_CONNECT (1U << 2)
struct media_pad_desc {
__u32 entity; /* entity ID */
@@ -216,14 +216,14 @@ 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 (1U << 0)
+#define MEDIA_LNK_FL_IMMUTABLE (1U << 1)
+#define MEDIA_LNK_FL_DYNAMIC (1U << 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_ANCILLARY_LINK (2 << 28)
+# define MEDIA_LNK_FL_DATA_LINK (0U << 28)
+# define MEDIA_LNK_FL_INTERFACE_LINK (1U << 28)
+# define MEDIA_LNK_FL_ANCILLARY_LINK (2U << 28)
struct media_link_desc {
struct media_pad_desc source;
@@ -293,7 +293,7 @@ struct media_links_enum {
* struct media_device_info.
*/
#define MEDIA_V2_ENTITY_HAS_FLAGS(media_version) \
- ((media_version) >= ((4 << 16) | (19 << 8) | 0))
+ ((media_version) >= ((4U << 16) | (19U << 8) | 0))
struct media_v2_entity {
__u32 id;
@@ -328,7 +328,7 @@ struct media_v2_interface {
* struct media_device_info.
*/
#define MEDIA_V2_PAD_HAS_INDEX(media_version) \
- ((media_version) >= ((4 << 16) | (19 << 8) | 0))
+ ((media_version) >= ((4U << 16) | (19U << 8) | 0))
struct media_v2_pad {
__u32 id;
@@ -432,7 +432,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 ((0U << 16) | (1U << 8) | 0)
#endif
--
2.30.2
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] media: mc: Make media_get_pad_index() use pad type flag
2023-05-05 20:50 ` [PATCH 1/3] media: mc: Make media_get_pad_index() use pad type flag Sakari Ailus
@ 2023-05-06 11:25 ` Laurent Pinchart
0 siblings, 0 replies; 12+ messages in thread
From: Laurent Pinchart @ 2023-05-06 11:25 UTC (permalink / raw)
To: Sakari Ailus; +Cc: linux-media, tomi.valkeinen, bingbu.cao
Hi Sakari,
Thank you for the patch.
On Fri, May 05, 2023 at 11:50:59PM +0300, Sakari Ailus wrote:
> Use the pad flag specifying the pad type instead of a boolean in
> preparation for internal source pads.
>
> Also make the loop variable unsigned.
>
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
> drivers/media/dvb-core/dvbdev.c | 4 +--
> drivers/media/mc/mc-entity.c | 16 ++++-------
> drivers/media/usb/au0828/au0828-core.c | 2 +-
> drivers/media/v4l2-core/v4l2-mc.c | 38 +++++++++++++++++---------
> include/media/media-entity.h | 4 +--
> 5 files changed, 35 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/media/dvb-core/dvbdev.c b/drivers/media/dvb-core/dvbdev.c
> index 0ed087caf7f3..0091b5375e45 100644
> --- a/drivers/media/dvb-core/dvbdev.c
> +++ b/drivers/media/dvb-core/dvbdev.c
> @@ -707,7 +707,7 @@ int dvb_create_media_graph(struct dvb_adapter *adap,
> MEDIA_LNK_FL_ENABLED,
> false);
> } else {
> - pad_sink = media_get_pad_index(tuner, true,
> + pad_sink = media_get_pad_index(tuner, MEDIA_PAD_FL_SINK,
> PAD_SIGNAL_ANALOG);
> if (pad_sink < 0)
> return -EINVAL;
> @@ -725,7 +725,7 @@ int dvb_create_media_graph(struct dvb_adapter *adap,
>
> if (ntuner && ndemod) {
> /* NOTE: first found tuner source pad presumed correct */
> - pad_source = media_get_pad_index(tuner, false,
> + pad_source = media_get_pad_index(tuner, MEDIA_PAD_FL_SOURCE,
> PAD_SIGNAL_ANALOG);
> if (pad_source < 0)
> return -EINVAL;
> diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c
> index e7216a985ba6..ef34ddd715c9 100644
> --- a/drivers/media/mc/mc-entity.c
> +++ b/drivers/media/mc/mc-entity.c
> @@ -1052,25 +1052,19 @@ static void __media_entity_remove_link(struct media_entity *entity,
> kfree(link);
> }
>
> -int media_get_pad_index(struct media_entity *entity, bool is_sink,
> +int media_get_pad_index(struct media_entity *entity, u32 pad_type,
> enum media_pad_signal_type sig_type)
> {
> - int i;
> - bool pad_is_sink;
> + unsigned int i;
>
> if (!entity)
> return -EINVAL;
>
> for (i = 0; i < entity->num_pads; i++) {
> - if (entity->pads[i].flags & MEDIA_PAD_FL_SINK)
> - pad_is_sink = true;
> - else if (entity->pads[i].flags & MEDIA_PAD_FL_SOURCE)
> - pad_is_sink = false;
> - else
> - continue; /* This is an error! */
> -
> - if (pad_is_sink != is_sink)
> + if ((entity->pads[i].flags &
> + (MEDIA_PAD_FL_SINK | MEDIA_PAD_FL_SOURCE)) != pad_type)
> continue;
> +
> if (entity->pads[i].sig_type == sig_type)
> return i;
> }
> diff --git a/drivers/media/usb/au0828/au0828-core.c b/drivers/media/usb/au0828/au0828-core.c
> index b3a09d3ac7d2..1e246b47766d 100644
> --- a/drivers/media/usb/au0828/au0828-core.c
> +++ b/drivers/media/usb/au0828/au0828-core.c
> @@ -250,7 +250,7 @@ static void au0828_media_graph_notify(struct media_entity *new,
>
> create_link:
> if (decoder && mixer) {
> - ret = media_get_pad_index(decoder, false,
> + ret = media_get_pad_index(decoder, MEDIA_PAD_FL_SOURCE,
> PAD_SIGNAL_AUDIO);
> if (ret >= 0)
> ret = media_create_pad_link(decoder, ret,
> diff --git a/drivers/media/v4l2-core/v4l2-mc.c b/drivers/media/v4l2-core/v4l2-mc.c
> index bf0c18100664..209a7efd08fe 100644
> --- a/drivers/media/v4l2-core/v4l2-mc.c
> +++ b/drivers/media/v4l2-core/v4l2-mc.c
> @@ -105,9 +105,11 @@ int v4l2_mc_create_media_graph(struct media_device *mdev)
> /* Link the tuner and IF video output pads */
> if (tuner) {
> if (if_vid) {
> - pad_source = media_get_pad_index(tuner, false,
> + pad_source = media_get_pad_index(tuner,
> + MEDIA_PAD_FL_SOURCE,
> PAD_SIGNAL_ANALOG);
> - pad_sink = media_get_pad_index(if_vid, true,
> + pad_sink = media_get_pad_index(if_vid,
> + MEDIA_PAD_FL_SINK,
> PAD_SIGNAL_ANALOG);
> if (pad_source < 0 || pad_sink < 0) {
> dev_warn(mdev->dev, "Couldn't get tuner and/or PLL pad(s): (%d, %d)\n",
> @@ -122,9 +124,11 @@ int v4l2_mc_create_media_graph(struct media_device *mdev)
> return ret;
> }
>
> - pad_source = media_get_pad_index(if_vid, false,
> + pad_source = media_get_pad_index(if_vid,
> + MEDIA_PAD_FL_SOURCE,
> PAD_SIGNAL_ANALOG);
> - pad_sink = media_get_pad_index(decoder, true,
> + pad_sink = media_get_pad_index(decoder,
> + MEDIA_PAD_FL_SINK,
> PAD_SIGNAL_ANALOG);
> if (pad_source < 0 || pad_sink < 0) {
> dev_warn(mdev->dev, "get decoder and/or PLL pad(s): (%d, %d)\n",
> @@ -139,9 +143,11 @@ int v4l2_mc_create_media_graph(struct media_device *mdev)
> return ret;
> }
> } else {
> - pad_source = media_get_pad_index(tuner, false,
> + pad_source = media_get_pad_index(tuner,
> + MEDIA_PAD_FL_SOURCE,
> PAD_SIGNAL_ANALOG);
> - pad_sink = media_get_pad_index(decoder, true,
> + pad_sink = media_get_pad_index(decoder,
> + MEDIA_PAD_FL_SINK,
> PAD_SIGNAL_ANALOG);
> if (pad_source < 0 || pad_sink < 0) {
> dev_warn(mdev->dev, "couldn't get tuner and/or decoder pad(s): (%d, %d)\n",
> @@ -156,9 +162,11 @@ int v4l2_mc_create_media_graph(struct media_device *mdev)
> }
>
> if (if_aud) {
> - pad_source = media_get_pad_index(tuner, false,
> + pad_source = media_get_pad_index(tuner,
> + MEDIA_PAD_FL_SOURCE,
> PAD_SIGNAL_AUDIO);
> - pad_sink = media_get_pad_index(if_aud, true,
> + pad_sink = media_get_pad_index(if_aud,
> + MEDIA_PAD_FL_SINK,
> PAD_SIGNAL_AUDIO);
> if (pad_source < 0 || pad_sink < 0) {
> dev_warn(mdev->dev, "couldn't get tuner and/or decoder pad(s) for audio: (%d, %d)\n",
> @@ -180,7 +188,8 @@ int v4l2_mc_create_media_graph(struct media_device *mdev)
>
> /* Create demod to V4L, VBI and SDR radio links */
> if (io_v4l) {
> - pad_source = media_get_pad_index(decoder, false, PAD_SIGNAL_DV);
> + pad_source = media_get_pad_index(decoder, MEDIA_PAD_FL_SOURCE,
> + PAD_SIGNAL_DV);
> if (pad_source < 0) {
> dev_warn(mdev->dev, "couldn't get decoder output pad for V4L I/O\n");
> return -EINVAL;
> @@ -195,7 +204,8 @@ int v4l2_mc_create_media_graph(struct media_device *mdev)
> }
>
> if (io_swradio) {
> - pad_source = media_get_pad_index(decoder, false, PAD_SIGNAL_DV);
> + pad_source = media_get_pad_index(decoder, MEDIA_PAD_FL_SOURCE,
> + PAD_SIGNAL_DV);
> if (pad_source < 0) {
> dev_warn(mdev->dev, "couldn't get decoder output pad for SDR\n");
> return -EINVAL;
> @@ -210,7 +220,8 @@ int v4l2_mc_create_media_graph(struct media_device *mdev)
> }
>
> if (io_vbi) {
> - pad_source = media_get_pad_index(decoder, false, PAD_SIGNAL_DV);
> + pad_source = media_get_pad_index(decoder, MEDIA_PAD_FL_SOURCE,
> + PAD_SIGNAL_DV);
> if (pad_source < 0) {
> dev_warn(mdev->dev, "couldn't get decoder output pad for VBI\n");
> return -EINVAL;
> @@ -231,7 +242,7 @@ int v4l2_mc_create_media_graph(struct media_device *mdev)
> case MEDIA_ENT_F_CONN_RF:
> if (!tuner)
> continue;
> - pad_sink = media_get_pad_index(tuner, true,
> + pad_sink = media_get_pad_index(tuner, MEDIA_PAD_FL_SINK,
> PAD_SIGNAL_ANALOG);
> if (pad_sink < 0) {
> dev_warn(mdev->dev, "couldn't get tuner analog pad sink\n");
> @@ -243,7 +254,8 @@ int v4l2_mc_create_media_graph(struct media_device *mdev)
> break;
> case MEDIA_ENT_F_CONN_SVIDEO:
> case MEDIA_ENT_F_CONN_COMPOSITE:
> - pad_sink = media_get_pad_index(decoder, true,
> + pad_sink = media_get_pad_index(decoder,
> + MEDIA_PAD_FL_SINK,
> PAD_SIGNAL_ANALOG);
> if (pad_sink < 0) {
> dev_warn(mdev->dev, "couldn't get decoder analog pad sink\n");
> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> index 741f9c629c6f..e4f556911c3f 100644
> --- a/include/media/media-entity.h
> +++ b/include/media/media-entity.h
> @@ -741,7 +741,7 @@ static inline void media_entity_cleanup(struct media_entity *entity) {}
> * media_get_pad_index() - retrieves a pad index from an entity
> *
> * @entity: entity where the pads belong
> - * @is_sink: true if the pad is a sink, false if it is a source
> + * @pad_type: the type of the pad, one of MEDIA_PAD_FL_* pad types
> * @sig_type: type of signal of the pad to be search
> *
> * This helper function finds the first pad index inside an entity that
> @@ -752,7 +752,7 @@ static inline void media_entity_cleanup(struct media_entity *entity) {}
> * On success, return the pad number. If the pad was not found or the media
> * entity is a NULL pointer, return -EINVAL.
> */
> -int media_get_pad_index(struct media_entity *entity, bool is_sink,
> +int media_get_pad_index(struct media_entity *entity, u32 pad_type,
> enum media_pad_signal_type sig_type);
>
> /**
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] media: Documentation: Rename meta format files
2023-05-05 20:51 ` [PATCH 2/3] media: Documentation: Rename meta format files Sakari Ailus
@ 2023-05-06 11:25 ` Laurent Pinchart
0 siblings, 0 replies; 12+ messages in thread
From: Laurent Pinchart @ 2023-05-06 11:25 UTC (permalink / raw)
To: Sakari Ailus; +Cc: linux-media, tomi.valkeinen, bingbu.cao
Hi Sakari,
Thank you for the patch.
On Fri, May 05, 2023 at 11:51:00PM +0300, Sakari Ailus wrote:
> Rename meta format files, using "metafmt" prefix instead of "pixfmt-meta".
> These are metadata formats, not pixel formats.
>
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> Cc: Bingbu Cao <bingbu.cao@intel.com>
> Cc: Dafna Hirschfeld <dafna@fastmail.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
> .../userspace-api/media/v4l/meta-formats.rst | 14 +++++++-------
> .../v4l/{pixfmt-meta-d4xx.rst => metafmt-d4xx.rst} | 0
> ...-meta-intel-ipu3.rst => metafmt-intel-ipu3.rst} | 0
> .../{pixfmt-meta-rkisp1.rst => metafmt-rkisp1.rst} | 0
> .../v4l/{pixfmt-meta-uvc.rst => metafmt-uvc.rst} | 0
> .../{pixfmt-meta-vivid.rst => metafmt-vivid.rst} | 0
> ...xfmt-meta-vsp1-hgo.rst => metafmt-vsp1-hgo.rst} | 0
> ...xfmt-meta-vsp1-hgt.rst => metafmt-vsp1-hgt.rst} | 0
> MAINTAINERS | 4 ++--
> 9 files changed, 9 insertions(+), 9 deletions(-)
> rename Documentation/userspace-api/media/v4l/{pixfmt-meta-d4xx.rst => metafmt-d4xx.rst} (100%)
> rename Documentation/userspace-api/media/v4l/{pixfmt-meta-intel-ipu3.rst => metafmt-intel-ipu3.rst} (100%)
> rename Documentation/userspace-api/media/v4l/{pixfmt-meta-rkisp1.rst => metafmt-rkisp1.rst} (100%)
> rename Documentation/userspace-api/media/v4l/{pixfmt-meta-uvc.rst => metafmt-uvc.rst} (100%)
> rename Documentation/userspace-api/media/v4l/{pixfmt-meta-vivid.rst => metafmt-vivid.rst} (100%)
> rename Documentation/userspace-api/media/v4l/{pixfmt-meta-vsp1-hgo.rst => metafmt-vsp1-hgo.rst} (100%)
> rename Documentation/userspace-api/media/v4l/{pixfmt-meta-vsp1-hgt.rst => metafmt-vsp1-hgt.rst} (100%)
>
> diff --git a/Documentation/userspace-api/media/v4l/meta-formats.rst b/Documentation/userspace-api/media/v4l/meta-formats.rst
> index fff25357fe86..0bb61fc5bc00 100644
> --- a/Documentation/userspace-api/media/v4l/meta-formats.rst
> +++ b/Documentation/userspace-api/media/v4l/meta-formats.rst
> @@ -12,10 +12,10 @@ These formats are used for the :ref:`metadata` interface only.
> .. toctree::
> :maxdepth: 1
>
> - pixfmt-meta-d4xx
> - pixfmt-meta-intel-ipu3
> - pixfmt-meta-rkisp1
> - pixfmt-meta-uvc
> - pixfmt-meta-vsp1-hgo
> - pixfmt-meta-vsp1-hgt
> - pixfmt-meta-vivid
> + metafmt-d4xx
> + metafmt-intel-ipu3
> + metafmt-rkisp1
> + metafmt-uvc
> + metafmt-vsp1-hgo
> + metafmt-vsp1-hgt
> + metafmt-vivid
> diff --git a/Documentation/userspace-api/media/v4l/pixfmt-meta-d4xx.rst b/Documentation/userspace-api/media/v4l/metafmt-d4xx.rst
> similarity index 100%
> rename from Documentation/userspace-api/media/v4l/pixfmt-meta-d4xx.rst
> rename to Documentation/userspace-api/media/v4l/metafmt-d4xx.rst
> diff --git a/Documentation/userspace-api/media/v4l/pixfmt-meta-intel-ipu3.rst b/Documentation/userspace-api/media/v4l/metafmt-intel-ipu3.rst
> similarity index 100%
> rename from Documentation/userspace-api/media/v4l/pixfmt-meta-intel-ipu3.rst
> rename to Documentation/userspace-api/media/v4l/metafmt-intel-ipu3.rst
> diff --git a/Documentation/userspace-api/media/v4l/pixfmt-meta-rkisp1.rst b/Documentation/userspace-api/media/v4l/metafmt-rkisp1.rst
> similarity index 100%
> rename from Documentation/userspace-api/media/v4l/pixfmt-meta-rkisp1.rst
> rename to Documentation/userspace-api/media/v4l/metafmt-rkisp1.rst
> diff --git a/Documentation/userspace-api/media/v4l/pixfmt-meta-uvc.rst b/Documentation/userspace-api/media/v4l/metafmt-uvc.rst
> similarity index 100%
> rename from Documentation/userspace-api/media/v4l/pixfmt-meta-uvc.rst
> rename to Documentation/userspace-api/media/v4l/metafmt-uvc.rst
> diff --git a/Documentation/userspace-api/media/v4l/pixfmt-meta-vivid.rst b/Documentation/userspace-api/media/v4l/metafmt-vivid.rst
> similarity index 100%
> rename from Documentation/userspace-api/media/v4l/pixfmt-meta-vivid.rst
> rename to Documentation/userspace-api/media/v4l/metafmt-vivid.rst
> diff --git a/Documentation/userspace-api/media/v4l/pixfmt-meta-vsp1-hgo.rst b/Documentation/userspace-api/media/v4l/metafmt-vsp1-hgo.rst
> similarity index 100%
> rename from Documentation/userspace-api/media/v4l/pixfmt-meta-vsp1-hgo.rst
> rename to Documentation/userspace-api/media/v4l/metafmt-vsp1-hgo.rst
> diff --git a/Documentation/userspace-api/media/v4l/pixfmt-meta-vsp1-hgt.rst b/Documentation/userspace-api/media/v4l/metafmt-vsp1-hgt.rst
> similarity index 100%
> rename from Documentation/userspace-api/media/v4l/pixfmt-meta-vsp1-hgt.rst
> rename to Documentation/userspace-api/media/v4l/metafmt-vsp1-hgt.rst
> diff --git a/MAINTAINERS b/MAINTAINERS
> index e25ebb7c781b..546826109900 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -10359,7 +10359,7 @@ L: linux-media@vger.kernel.org
> S: Maintained
> F: Documentation/admin-guide/media/ipu3.rst
> F: Documentation/admin-guide/media/ipu3_rcb.svg
> -F: Documentation/userspace-api/media/v4l/pixfmt-meta-intel-ipu3.rst
> +F: Documentation/userspace-api/media/v4l/metafmt-intel-ipu3.rst
> F: drivers/staging/media/ipu3/
>
> INTEL IXP4XX CRYPTO SUPPORT
> @@ -18026,7 +18026,7 @@ L: linux-rockchip@lists.infradead.org
> S: Maintained
> F: Documentation/admin-guide/media/rkisp1.rst
> F: Documentation/devicetree/bindings/media/rockchip-isp1.yaml
> -F: Documentation/userspace-api/media/v4l/pixfmt-meta-rkisp1.rst
> +F: Documentation/userspace-api/media/v4l/metafmt-rkisp1.rst
> F: drivers/media/platform/rockchip/rkisp1
> F: include/uapi/linux/rkisp1-config.h
>
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] media: uapi: Use unsigned int values for assigning bits in u32 fields
2023-05-05 20:51 ` [PATCH 3/3] media: uapi: Use unsigned int values for assigning bits in u32 fields Sakari Ailus
@ 2023-05-06 11:32 ` Laurent Pinchart
2023-05-08 6:19 ` Sakari Ailus
0 siblings, 1 reply; 12+ messages in thread
From: Laurent Pinchart @ 2023-05-06 11:32 UTC (permalink / raw)
To: Sakari Ailus; +Cc: linux-media, tomi.valkeinen, bingbu.cao
Hi Sakari,
Thank you for the patch.
On Fri, May 05, 2023 at 11:51:01PM +0300, Sakari Ailus wrote:
> Use unsigned int values annoted by "U" for u32 fields. While this is a
> good practice, there doesn't appear to be a bug that this patch would fix.
>
> The patch has been generated using the following command:
>
> perl -i -pe 's/\([0-9]+\K <</U <</g' -- include/uapi/linux/media.h
How about using the _BITUL() macro from include/uapi/linux/const.h ?
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
> include/uapi/linux/media.h | 28 ++++++++++++++--------------
> 1 file changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
> index 3ddadaea849f..edb8dfef9eba 100644
> --- a/include/uapi/linux/media.h
> +++ b/include/uapi/linux/media.h
> @@ -140,8 +140,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 (1U << 0)
> +#define MEDIA_ENT_FL_CONNECTOR (1U << 1)
>
> /* OR with the entity id value to find the next entity */
> #define MEDIA_ENT_ID_FLAG_NEXT (1U << 31)
> @@ -205,9 +205,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 (1U << 0)
> +#define MEDIA_PAD_FL_SOURCE (1U << 1)
> +#define MEDIA_PAD_FL_MUST_CONNECT (1U << 2)
>
> struct media_pad_desc {
> __u32 entity; /* entity ID */
> @@ -216,14 +216,14 @@ 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 (1U << 0)
> +#define MEDIA_LNK_FL_IMMUTABLE (1U << 1)
> +#define MEDIA_LNK_FL_DYNAMIC (1U << 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_ANCILLARY_LINK (2 << 28)
> +# define MEDIA_LNK_FL_DATA_LINK (0U << 28)
> +# define MEDIA_LNK_FL_INTERFACE_LINK (1U << 28)
> +# define MEDIA_LNK_FL_ANCILLARY_LINK (2U << 28)
>
> struct media_link_desc {
> struct media_pad_desc source;
> @@ -293,7 +293,7 @@ struct media_links_enum {
> * struct media_device_info.
> */
> #define MEDIA_V2_ENTITY_HAS_FLAGS(media_version) \
> - ((media_version) >= ((4 << 16) | (19 << 8) | 0))
> + ((media_version) >= ((4U << 16) | (19U << 8) | 0))
>
> struct media_v2_entity {
> __u32 id;
> @@ -328,7 +328,7 @@ struct media_v2_interface {
> * struct media_device_info.
> */
> #define MEDIA_V2_PAD_HAS_INDEX(media_version) \
> - ((media_version) >= ((4 << 16) | (19 << 8) | 0))
> + ((media_version) >= ((4U << 16) | (19U << 8) | 0))
>
> struct media_v2_pad {
> __u32 id;
> @@ -432,7 +432,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 ((0U << 16) | (1U << 8) | 0)
>
> #endif
>
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] media: uapi: Use unsigned int values for assigning bits in u32 fields
2023-05-06 11:32 ` Laurent Pinchart
@ 2023-05-08 6:19 ` Sakari Ailus
2023-05-08 6:30 ` Laurent Pinchart
0 siblings, 1 reply; 12+ messages in thread
From: Sakari Ailus @ 2023-05-08 6:19 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: linux-media, tomi.valkeinen, bingbu.cao
Hi Laurent,
On Sat, May 06, 2023 at 02:32:23PM +0300, Laurent Pinchart wrote:
> Hi Sakari,
>
> Thank you for the patch.
>
> On Fri, May 05, 2023 at 11:51:01PM +0300, Sakari Ailus wrote:
> > Use unsigned int values annoted by "U" for u32 fields. While this is a
> > good practice, there doesn't appear to be a bug that this patch would fix.
> >
> > The patch has been generated using the following command:
> >
> > perl -i -pe 's/\([0-9]+\K <</U <</g' -- include/uapi/linux/media.h
>
> How about using the _BITUL() macro from include/uapi/linux/const.h ?
These are u32 whereas _BITUL makes an unsigned long. Int (as in U) is 32
bits on all platforms where Linux is used AFAIK.
And thanks for the review!
--
Kind regards,
Sakari Ailus
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] media: uapi: Use unsigned int values for assigning bits in u32 fields
2023-05-08 6:19 ` Sakari Ailus
@ 2023-05-08 6:30 ` Laurent Pinchart
2023-05-08 6:58 ` Sakari Ailus
0 siblings, 1 reply; 12+ messages in thread
From: Laurent Pinchart @ 2023-05-08 6:30 UTC (permalink / raw)
To: Sakari Ailus; +Cc: linux-media, tomi.valkeinen, bingbu.cao
Hi Sakari,
On Mon, May 08, 2023 at 09:19:24AM +0300, Sakari Ailus wrote:
> On Sat, May 06, 2023 at 02:32:23PM +0300, Laurent Pinchart wrote:
> > On Fri, May 05, 2023 at 11:51:01PM +0300, Sakari Ailus wrote:
> > > Use unsigned int values annoted by "U" for u32 fields. While this is a
> > > good practice, there doesn't appear to be a bug that this patch would fix.
> > >
> > > The patch has been generated using the following command:
> > >
> > > perl -i -pe 's/\([0-9]+\K <</U <</g' -- include/uapi/linux/media.h
> >
> > How about using the _BITUL() macro from include/uapi/linux/const.h ?
>
> These are u32 whereas _BITUL makes an unsigned long. Int (as in U) is 32
> bits on all platforms where Linux is used AFAIK.
I know, but is it a problem ?
> And thanks for the review!
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] media: uapi: Use unsigned int values for assigning bits in u32 fields
2023-05-08 6:30 ` Laurent Pinchart
@ 2023-05-08 6:58 ` Sakari Ailus
2023-05-08 7:52 ` Laurent Pinchart
0 siblings, 1 reply; 12+ messages in thread
From: Sakari Ailus @ 2023-05-08 6:58 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: linux-media, tomi.valkeinen, bingbu.cao
Hi Laurent,
On Mon, May 08, 2023 at 09:30:23AM +0300, Laurent Pinchart wrote:
> Hi Sakari,
>
> On Mon, May 08, 2023 at 09:19:24AM +0300, Sakari Ailus wrote:
> > On Sat, May 06, 2023 at 02:32:23PM +0300, Laurent Pinchart wrote:
> > > On Fri, May 05, 2023 at 11:51:01PM +0300, Sakari Ailus wrote:
> > > > Use unsigned int values annoted by "U" for u32 fields. While this is a
> > > > good practice, there doesn't appear to be a bug that this patch would fix.
> > > >
> > > > The patch has been generated using the following command:
> > > >
> > > > perl -i -pe 's/\([0-9]+\K <</U <</g' -- include/uapi/linux/media.h
> > >
> > > How about using the _BITUL() macro from include/uapi/linux/const.h ?
> >
> > These are u32 whereas _BITUL makes an unsigned long. Int (as in U) is 32
> > bits on all platforms where Linux is used AFAIK.
>
> I know, but is it a problem ?
If we have a u32 field, unsigned int is the right type for that (from
non-fixed length C types), not unsigned long. In practice it would work, I
have no doubts about that. The compiler could still do different decisions
due to this, promoting values to a 64-bits for instance.
If we had _BITU(), I'd be happy to use that. :-)
How about this: let's merge this patch and then see how a _BITU() macro
would fare.
--
Kind regards,
Sakari Ailus
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] media: uapi: Use unsigned int values for assigning bits in u32 fields
2023-05-08 6:58 ` Sakari Ailus
@ 2023-05-08 7:52 ` Laurent Pinchart
2023-05-08 8:34 ` Sakari Ailus
0 siblings, 1 reply; 12+ messages in thread
From: Laurent Pinchart @ 2023-05-08 7:52 UTC (permalink / raw)
To: Sakari Ailus; +Cc: linux-media, tomi.valkeinen, bingbu.cao
Hi Sakari,
On Mon, May 08, 2023 at 09:58:48AM +0300, Sakari Ailus wrote:
> On Mon, May 08, 2023 at 09:30:23AM +0300, Laurent Pinchart wrote:
> > On Mon, May 08, 2023 at 09:19:24AM +0300, Sakari Ailus wrote:
> > > On Sat, May 06, 2023 at 02:32:23PM +0300, Laurent Pinchart wrote:
> > > > On Fri, May 05, 2023 at 11:51:01PM +0300, Sakari Ailus wrote:
> > > > > Use unsigned int values annoted by "U" for u32 fields. While this is a
> > > > > good practice, there doesn't appear to be a bug that this patch would fix.
> > > > >
> > > > > The patch has been generated using the following command:
> > > > >
> > > > > perl -i -pe 's/\([0-9]+\K <</U <</g' -- include/uapi/linux/media.h
> > > >
> > > > How about using the _BITUL() macro from include/uapi/linux/const.h ?
> > >
> > > These are u32 whereas _BITUL makes an unsigned long. Int (as in U) is 32
> > > bits on all platforms where Linux is used AFAIK.
> >
> > I know, but is it a problem ?
>
> If we have a u32 field, unsigned int is the right type for that (from
> non-fixed length C types), not unsigned long. In practice it would work, I
> have no doubts about that. The compiler could still do different decisions
> due to this, promoting values to a 64-bits for instance.
>
> If we had _BITU(), I'd be happy to use that. :-)
Note how BIT() is defined in include/vdso/bits.h:
#include <vdso/const.h>
#define BIT(nr) (UL(1) << (nr))
And in include/vdso/const.h:
#include <uapi/linux/const.h>
#define UL(x) (_UL(x))
BIT() is thus essentially identical to _BITUL(). As we use the former
everywhere without any trouble, I wouldn't expect issue with the latter.
> How about this: let's merge this patch and then see how a _BITU() macro
> would fare.
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] media: uapi: Use unsigned int values for assigning bits in u32 fields
2023-05-08 7:52 ` Laurent Pinchart
@ 2023-05-08 8:34 ` Sakari Ailus
0 siblings, 0 replies; 12+ messages in thread
From: Sakari Ailus @ 2023-05-08 8:34 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: linux-media, tomi.valkeinen, bingbu.cao
On Mon, May 08, 2023 at 10:52:09AM +0300, Laurent Pinchart wrote:
> Hi Sakari,
>
> On Mon, May 08, 2023 at 09:58:48AM +0300, Sakari Ailus wrote:
> > On Mon, May 08, 2023 at 09:30:23AM +0300, Laurent Pinchart wrote:
> > > On Mon, May 08, 2023 at 09:19:24AM +0300, Sakari Ailus wrote:
> > > > On Sat, May 06, 2023 at 02:32:23PM +0300, Laurent Pinchart wrote:
> > > > > On Fri, May 05, 2023 at 11:51:01PM +0300, Sakari Ailus wrote:
> > > > > > Use unsigned int values annoted by "U" for u32 fields. While this is a
> > > > > > good practice, there doesn't appear to be a bug that this patch would fix.
> > > > > >
> > > > > > The patch has been generated using the following command:
> > > > > >
> > > > > > perl -i -pe 's/\([0-9]+\K <</U <</g' -- include/uapi/linux/media.h
> > > > >
> > > > > How about using the _BITUL() macro from include/uapi/linux/const.h ?
> > > >
> > > > These are u32 whereas _BITUL makes an unsigned long. Int (as in U) is 32
> > > > bits on all platforms where Linux is used AFAIK.
> > >
> > > I know, but is it a problem ?
> >
> > If we have a u32 field, unsigned int is the right type for that (from
> > non-fixed length C types), not unsigned long. In practice it would work, I
> > have no doubts about that. The compiler could still do different decisions
> > due to this, promoting values to a 64-bits for instance.
> >
> > If we had _BITU(), I'd be happy to use that. :-)
>
> Note how BIT() is defined in include/vdso/bits.h:
>
> #include <vdso/const.h>
>
> #define BIT(nr) (UL(1) << (nr))
>
> And in include/vdso/const.h:
>
> #include <uapi/linux/const.h>
>
> #define UL(x) (_UL(x))
>
> BIT() is thus essentially identical to _BITUL(). As we use the former
> everywhere without any trouble, I wouldn't expect issue with the latter.
Fair enough. I'll switch to _BITUL() in v2.
--
Sakari Ailus
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2023-05-08 8:34 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-05-05 20:50 [PATCH 0/3] Random cleanups Sakari Ailus
2023-05-05 20:50 ` [PATCH 1/3] media: mc: Make media_get_pad_index() use pad type flag Sakari Ailus
2023-05-06 11:25 ` Laurent Pinchart
2023-05-05 20:51 ` [PATCH 2/3] media: Documentation: Rename meta format files Sakari Ailus
2023-05-06 11:25 ` Laurent Pinchart
2023-05-05 20:51 ` [PATCH 3/3] media: uapi: Use unsigned int values for assigning bits in u32 fields Sakari Ailus
2023-05-06 11:32 ` Laurent Pinchart
2023-05-08 6:19 ` Sakari Ailus
2023-05-08 6:30 ` Laurent Pinchart
2023-05-08 6:58 ` Sakari Ailus
2023-05-08 7:52 ` Laurent Pinchart
2023-05-08 8:34 ` Sakari Ailus
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox