From 53aa6b38f10cc42136551acf115a8b04e013ec89 Mon Sep 17 00:00:00 2001 From: Sakari Ailus Date: Thu, 25 May 2023 14:17:58 +0300 Subject: media: v4l2-mc: Add debug prints for v4l2_fwnode_create_links_for_pad() Add relevant debug prints for v4l2_fwnode_create_links_for_pad(). This should help debugging when things go wrong. Signed-off-by: Sakari Ailus Reviewed-by: Laurent Pinchart Signed-off-by: Hans Verkuil --- drivers/media/v4l2-core/v4l2-mc.c | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) (limited to 'drivers/media/v4l2-core') diff --git a/drivers/media/v4l2-core/v4l2-mc.c b/drivers/media/v4l2-core/v4l2-mc.c index 52d349e72b8c..4bb91359e3a9 100644 --- a/drivers/media/v4l2-core/v4l2-mc.c +++ b/drivers/media/v4l2-core/v4l2-mc.c @@ -337,12 +337,18 @@ int v4l2_create_fwnode_links_to_pad(struct v4l2_subdev *src_sd, src_idx = media_entity_get_fwnode_pad(&src_sd->entity, endpoint, MEDIA_PAD_FL_SOURCE); - if (src_idx < 0) + if (src_idx < 0) { + dev_dbg(src_sd->dev, "no source pad found for %pfw\n", + endpoint); continue; + } remote_ep = fwnode_graph_get_remote_endpoint(endpoint); - if (!remote_ep) + if (!remote_ep) { + dev_dbg(src_sd->dev, "no remote ep found for %pfw\n", + endpoint); continue; + } /* * ask the sink to verify it owns the remote endpoint, @@ -353,8 +359,12 @@ int v4l2_create_fwnode_links_to_pad(struct v4l2_subdev *src_sd, MEDIA_PAD_FL_SINK); fwnode_handle_put(remote_ep); - if (sink_idx < 0 || sink_idx != sink->index) + if (sink_idx < 0 || sink_idx != sink->index) { + dev_dbg(src_sd->dev, + "sink pad index mismatch or error (is %d, expected %u)\n", + sink_idx, sink->index); continue; + } /* * the source endpoint corresponds to one of its source pads, @@ -367,8 +377,13 @@ int v4l2_create_fwnode_links_to_pad(struct v4l2_subdev *src_sd, src = &src_sd->entity.pads[src_idx]; /* skip if link already exists */ - if (media_entity_find_link(src, sink)) + if (media_entity_find_link(src, sink)) { + dev_dbg(src_sd->dev, + "link %s:%d -> %s:%d already exists\n", + src_sd->entity.name, src_idx, + sink->entity->name, sink_idx); continue; + } dev_dbg(src_sd->dev, "creating link %s:%d -> %s:%d\n", src_sd->entity.name, src_idx, -- cgit From 58ab1f9e140006e9e5686640f1773260038fe889 Mon Sep 17 00:00:00 2001 From: Julien Massot Date: Thu, 11 Jan 2024 14:20:03 +0100 Subject: media: v4l2: cci: print leading 0 on error In some error cases leading '0' for register address were missing. Fixes: 613cbb91e9ce ("media: Add MIPI CCI register access helper functions") Signed-off-by: Julien Massot Reviewed-by: Hans de Goede Signed-off-by: Sakari Ailus Signed-off-by: Mauro Carvalho Chehab --- drivers/media/v4l2-core/v4l2-cci.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/media/v4l2-core') diff --git a/drivers/media/v4l2-core/v4l2-cci.c b/drivers/media/v4l2-core/v4l2-cci.c index 10005c80f43b..ee3475bed37f 100644 --- a/drivers/media/v4l2-core/v4l2-cci.c +++ b/drivers/media/v4l2-core/v4l2-cci.c @@ -32,7 +32,7 @@ int cci_read(struct regmap *map, u32 reg, u64 *val, int *err) ret = regmap_bulk_read(map, reg, buf, len); if (ret) { - dev_err(regmap_get_device(map), "Error reading reg 0x%4x: %d\n", + dev_err(regmap_get_device(map), "Error reading reg 0x%04x: %d\n", reg, ret); goto out; } @@ -131,7 +131,7 @@ int cci_write(struct regmap *map, u32 reg, u64 val, int *err) ret = regmap_bulk_write(map, reg, buf, len); if (ret) - dev_err(regmap_get_device(map), "Error writing reg 0x%4x: %d\n", + dev_err(regmap_get_device(map), "Error writing reg 0x%04x: %d\n", reg, ret); out: -- cgit From a68e88e2cf9e4f3bec1783d686beb2439f4a26e0 Mon Sep 17 00:00:00 2001 From: Sakari Ailus Date: Mon, 8 Jan 2024 16:18:02 +0100 Subject: media: v4l: Add a helper for setting up link-frequencies control Add a helper for obtaining supported link frequencies in form most drivers need them. The result is a bitmap of supported controls. Signed-off-by: Sakari Ailus Signed-off-by: Mauro Carvalho Chehab --- drivers/media/v4l2-core/v4l2-common.c | 47 +++++++++++++++++++++++++++++++++++ include/media/v4l2-common.h | 25 +++++++++++++++++++ 2 files changed, 72 insertions(+) (limited to 'drivers/media/v4l2-core') diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c index 273d83de2a87..d34d210908d9 100644 --- a/drivers/media/v4l2-core/v4l2-common.c +++ b/drivers/media/v4l2-core/v4l2-common.c @@ -585,3 +585,50 @@ u32 v4l2_fraction_to_interval(u32 numerator, u32 denominator) return denominator ? numerator * multiplier / denominator : 0; } EXPORT_SYMBOL_GPL(v4l2_fraction_to_interval); + +int v4l2_link_freq_to_bitmap(struct device *dev, const u64 *fw_link_freqs, + unsigned int num_of_fw_link_freqs, + const s64 *driver_link_freqs, + unsigned int num_of_driver_link_freqs, + unsigned long *bitmap) +{ + unsigned int i; + + *bitmap = 0; + + if (!num_of_fw_link_freqs) { + dev_err(dev, "no link frequencies in firmware\n"); + return -ENODATA; + } + + for (i = 0; i < num_of_fw_link_freqs; i++) { + unsigned int j; + + for (j = 0; j < num_of_driver_link_freqs; j++) { + if (fw_link_freqs[i] != driver_link_freqs[j]) + continue; + + dev_dbg(dev, "enabling link frequency %lld Hz\n", + driver_link_freqs[j]); + *bitmap |= BIT(j); + break; + } + } + + if (!*bitmap) { + dev_err(dev, "no matching link frequencies found\n"); + + dev_dbg(dev, "specified in firmware:\n"); + for (i = 0; i < num_of_fw_link_freqs; i++) + dev_dbg(dev, "\t%llu Hz\n", fw_link_freqs[i]); + + dev_dbg(dev, "driver supported:\n"); + for (i = 0; i < num_of_driver_link_freqs; i++) + dev_dbg(dev, "\t%lld Hz\n", driver_link_freqs[i]); + + return -ENOENT; + } + + return 0; +} +EXPORT_SYMBOL_GPL(v4l2_link_freq_to_bitmap); diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h index acf5be24a5ca..cd2163f24f8a 100644 --- a/include/media/v4l2-common.h +++ b/include/media/v4l2-common.h @@ -547,6 +547,31 @@ void v4l2_simplify_fraction(u32 *numerator, u32 *denominator, unsigned int n_terms, unsigned int threshold); u32 v4l2_fraction_to_interval(u32 numerator, u32 denominator); +/** + * v4l2_link_freq_to_bitmap - Figure out platform-supported link frequencies + * @dev: The struct device + * @fw_link_freqs: Array of link frequencies from firmware + * @num_of_fw_link_freqs: Number of entries in @fw_link_freqs + * @driver_link_freqs: Array of link frequencies supported by the driver + * @num_of_driver_link_freqs: Number of entries in @driver_link_freqs + * @bitmap: Bitmap of driver-supported link frequencies found in @fw_link_freqs + * + * This function checks which driver-supported link frequencies are enabled in + * system firmware and sets the corresponding bits in @bitmap (after first + * zeroing it). + * + * Return values: + * 0: Success + * -ENOENT: No match found between driver-supported link frequencies and + * those available in firmware. + * -ENODATA: No link frequencies were specified in firmware. + */ +int v4l2_link_freq_to_bitmap(struct device *dev, const u64 *fw_link_freqs, + unsigned int num_of_fw_link_freqs, + const s64 *driver_link_freqs, + unsigned int num_of_driver_link_freqs, + unsigned long *bitmap); + static inline u64 v4l2_buffer_get_timestamp(const struct v4l2_buffer *buf) { /* -- cgit From 9801b5b28c6929139d6fceeee8d739cc67bb2739 Mon Sep 17 00:00:00 2001 From: Hans Verkuil Date: Wed, 17 Jan 2024 15:52:04 +0100 Subject: media: v4l2-ctrls: show all owned controls in log_status VIDIOC_LOG_STATUS will log the controls owned by the driver. But the code didn't take into account the case where a single driver creates multiple control handlers. A good example is the vivid driver, but others use it as well. Modify v4l2_ctrl_handler_log_status() so that it really shows all controls owned by this driver. Signed-off-by: Hans Verkuil --- drivers/media/v4l2-core/v4l2-ctrls-core.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) (limited to 'drivers/media/v4l2-core') diff --git a/drivers/media/v4l2-core/v4l2-ctrls-core.c b/drivers/media/v4l2-core/v4l2-ctrls-core.c index a662fb60f73f..0accb96001db 100644 --- a/drivers/media/v4l2-core/v4l2-ctrls-core.c +++ b/drivers/media/v4l2-core/v4l2-ctrls-core.c @@ -2503,7 +2503,8 @@ int v4l2_ctrl_handler_setup(struct v4l2_ctrl_handler *hdl) EXPORT_SYMBOL(v4l2_ctrl_handler_setup); /* Log the control name and value */ -static void log_ctrl(const struct v4l2_ctrl *ctrl, +static void log_ctrl(const struct v4l2_ctrl_handler *hdl, + struct v4l2_ctrl *ctrl, const char *prefix, const char *colon) { if (ctrl->flags & (V4L2_CTRL_FLAG_DISABLED | V4L2_CTRL_FLAG_WRITE_ONLY)) @@ -2513,7 +2514,11 @@ static void log_ctrl(const struct v4l2_ctrl *ctrl, pr_info("%s%s%s: ", prefix, colon, ctrl->name); + if (ctrl->handler != hdl) + v4l2_ctrl_lock(ctrl); ctrl->type_ops->log(ctrl); + if (ctrl->handler != hdl) + v4l2_ctrl_unlock(ctrl); if (ctrl->flags & (V4L2_CTRL_FLAG_INACTIVE | V4L2_CTRL_FLAG_GRABBED | @@ -2532,7 +2537,7 @@ static void log_ctrl(const struct v4l2_ctrl *ctrl, void v4l2_ctrl_handler_log_status(struct v4l2_ctrl_handler *hdl, const char *prefix) { - struct v4l2_ctrl *ctrl; + struct v4l2_ctrl_ref *ref; const char *colon = ""; int len; @@ -2544,9 +2549,12 @@ void v4l2_ctrl_handler_log_status(struct v4l2_ctrl_handler *hdl, if (len && prefix[len - 1] != ' ') colon = ": "; mutex_lock(hdl->lock); - list_for_each_entry(ctrl, &hdl->ctrls, node) - if (!(ctrl->flags & V4L2_CTRL_FLAG_DISABLED)) - log_ctrl(ctrl, prefix, colon); + list_for_each_entry(ref, &hdl->ctrl_refs, node) { + if (ref->from_other_dev || + (ref->ctrl->flags & V4L2_CTRL_FLAG_DISABLED)) + continue; + log_ctrl(hdl, ref->ctrl, prefix, colon); + } mutex_unlock(hdl->lock); } EXPORT_SYMBOL(v4l2_ctrl_handler_log_status); -- cgit From 8f94b49a5b5d386c038e355bef6347298aabd211 Mon Sep 17 00:00:00 2001 From: Zhipeng Lu Date: Thu, 1 Feb 2024 20:48:44 +0800 Subject: media: v4l2-mem2mem: fix a memleak in v4l2_m2m_register_entity The entity->name (i.e. name) is allocated in v4l2_m2m_register_entity but isn't freed in its following error-handling paths. This patch adds such deallocation to prevent memleak of entity->name. Fixes: be2fff656322 ("media: add helpers for memory-to-memory media controller") Signed-off-by: Zhipeng Lu Signed-off-by: Hans Verkuil --- drivers/media/v4l2-core/v4l2-mem2mem.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) (limited to 'drivers/media/v4l2-core') diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c b/drivers/media/v4l2-core/v4l2-mem2mem.c index 9e983176542b..75517134a5e9 100644 --- a/drivers/media/v4l2-core/v4l2-mem2mem.c +++ b/drivers/media/v4l2-core/v4l2-mem2mem.c @@ -1087,11 +1087,17 @@ static int v4l2_m2m_register_entity(struct media_device *mdev, entity->function = function; ret = media_entity_pads_init(entity, num_pads, pads); - if (ret) + if (ret) { + kfree(entity->name); + entity->name = NULL; return ret; + } ret = media_device_register_entity(mdev, entity); - if (ret) + if (ret) { + kfree(entity->name); + entity->name = NULL; return ret; + } return 0; } -- cgit From ee0f8674654023320ab61373e2da83e3e0044331 Mon Sep 17 00:00:00 2001 From: Hans Verkuil Date: Mon, 5 Feb 2024 12:10:31 +0100 Subject: media: v4l2-ctrls-core.c: check min/max for menu, controls Menu controls that use the menu_skip_mask require that the min-max range is inside 0-63. Negative values obviously make no sense for menu controls, and the maximum value is currently limited by the number of bits of the menu_skip_mask value. However, if menu_skip_mask == 0, then larger menus are fine. If we ever need to add support for larger menus that support the skip mask, then more work is needed. In the places where the menu_skip_mask is checked, use BIT_ULL to get the bit to check and check if the bit number is < BITS_PER_LONG_LONG to avoid shifting out of range. With the new check in check_range this should never happen, but it is better to be safe and avoid static analyzer warnings. Signed-off-by: Hans Verkuil --- drivers/media/v4l2-core/v4l2-ctrls-api.c | 2 +- drivers/media/v4l2-core/v4l2-ctrls-core.c | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) (limited to 'drivers/media/v4l2-core') diff --git a/drivers/media/v4l2-core/v4l2-ctrls-api.c b/drivers/media/v4l2-core/v4l2-ctrls-api.c index 002ea6588edf..d9a422017bd9 100644 --- a/drivers/media/v4l2-core/v4l2-ctrls-api.c +++ b/drivers/media/v4l2-core/v4l2-ctrls-api.c @@ -1179,7 +1179,7 @@ int v4l2_querymenu(struct v4l2_ctrl_handler *hdl, struct v4l2_querymenu *qm) return -EINVAL; /* Use mask to see if this menu item should be skipped */ - if (ctrl->menu_skip_mask & (1ULL << i)) + if (i < BITS_PER_LONG_LONG && (ctrl->menu_skip_mask & BIT_ULL(i))) return -EINVAL; /* Empty menu items should also be skipped */ if (ctrl->type == V4L2_CTRL_TYPE_MENU) { diff --git a/drivers/media/v4l2-core/v4l2-ctrls-core.c b/drivers/media/v4l2-core/v4l2-ctrls-core.c index 0accb96001db..c4d995f32191 100644 --- a/drivers/media/v4l2-core/v4l2-ctrls-core.c +++ b/drivers/media/v4l2-core/v4l2-ctrls-core.c @@ -1504,11 +1504,12 @@ int check_range(enum v4l2_ctrl_type type, return 0; case V4L2_CTRL_TYPE_MENU: case V4L2_CTRL_TYPE_INTEGER_MENU: - if (min > max || def < min || def > max) + if (min > max || def < min || def > max || + min < 0 || (step && max >= BITS_PER_LONG_LONG)) return -ERANGE; /* Note: step == menu_skip_mask for menu controls. So here we check if the default value is masked out. */ - if (step && ((1 << def) & step)) + if (def < BITS_PER_LONG_LONG && (step & BIT_ULL(def))) return -EINVAL; return 0; case V4L2_CTRL_TYPE_STRING: -- cgit From c464c2e3bbab3f341ddbd2f32a1c680b29f52f48 Mon Sep 17 00:00:00 2001 From: Hans Verkuil Date: Wed, 7 Feb 2024 10:03:30 +0100 Subject: media: core: v4l2-ioctl.c: use is_valid_ioctl() In most cases the is_valid_ioctl() macro is used to check if an ioctl is valid, except in one place. Use it there as well as it makes the code easier to read. Signed-off-by: Hans Verkuil --- drivers/media/v4l2-core/v4l2-ioctl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/media/v4l2-core') diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c index 33076af4dfdb..6e7b8b682d13 100644 --- a/drivers/media/v4l2-core/v4l2-ioctl.c +++ b/drivers/media/v4l2-core/v4l2-ioctl.c @@ -3028,7 +3028,7 @@ static long __video_do_ioctl(struct file *file, if (v4l2_is_known_ioctl(cmd)) { info = &v4l2_ioctls[_IOC_NR(cmd)]; - if (!test_bit(_IOC_NR(cmd), vfd->valid_ioctls) && + if (!is_valid_ioctl(vfd, cmd) && !((info->flags & INFO_FL_CTRL) && vfh && vfh->ctrl_handler)) goto done; -- cgit