public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Ricardo Ribalda <ribalda@chromium.org>
To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	 Mauro Carvalho Chehab <mchehab@kernel.org>,
	 Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: linux-kernel@vger.kernel.org, linux-media@vger.kernel.org,
	 Yunke Cao <yunkec@chromium.org>,
	Hans Verkuil <hverkuil@xs4all.nl>,
	 Hans de Goede <hdegoede@redhat.com>,
	Ricardo Ribalda <ribalda@chromium.org>
Subject: [PATCH v2 2/6] media: uvcvideo: Re-implement privacy GPIO as a separate subdevice
Date: Fri, 08 Nov 2024 20:25:46 +0000	[thread overview]
Message-ID: <20241108-uvc-subdev-v2-2-85d8a051a3d3@chromium.org> (raw)
In-Reply-To: <20241108-uvc-subdev-v2-0-85d8a051a3d3@chromium.org>

From: Yunke Cao <yunkec@chromium.org>

Reimplement privacy GPIO as a v4l2 subdev with a volatile privacy control.
A v4l2 control event is sent in irq when privacy control value changes.

The behavior matches the original implementation, except that the
control is of a separate subdevice.

V4L2 control kAPI is used for simplicity.

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
Signed-off-by: Yunke Cao <yunkec@chromium.org>
---
 drivers/media/usb/uvc/uvc_ctrl.c   |  20 -------
 drivers/media/usb/uvc/uvc_driver.c |   4 +-
 drivers/media/usb/uvc/uvc_entity.c |   6 ++
 drivers/media/usb/uvc/uvc_gpio.c   | 111 ++++++++++++++++++++++---------------
 drivers/media/usb/uvc/uvcvideo.h   |  20 ++++---
 5 files changed, 88 insertions(+), 73 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
index 4fe26e82e3d1..b98f4778d8aa 100644
--- a/drivers/media/usb/uvc/uvc_ctrl.c
+++ b/drivers/media/usb/uvc/uvc_ctrl.c
@@ -350,14 +350,6 @@ static const struct uvc_control_info uvc_ctrls[] = {
 				| UVC_CTRL_FLAG_RESTORE
 				| UVC_CTRL_FLAG_AUTO_UPDATE,
 	},
-	{
-		.entity		= UVC_GUID_EXT_GPIO_CONTROLLER,
-		.selector	= UVC_CT_PRIVACY_CONTROL,
-		.index		= 0,
-		.size		= 1,
-		.flags		= UVC_CTRL_FLAG_GET_CUR
-				| UVC_CTRL_FLAG_AUTO_UPDATE,
-	},
 };
 
 static const u32 uvc_control_classes[] = {
@@ -827,15 +819,6 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
 		.v4l2_type	= V4L2_CTRL_TYPE_BOOLEAN,
 		.data_type	= UVC_CTRL_DATA_TYPE_BOOLEAN,
 	},
-	{
-		.id		= V4L2_CID_PRIVACY,
-		.entity		= UVC_GUID_EXT_GPIO_CONTROLLER,
-		.selector	= UVC_CT_PRIVACY_CONTROL,
-		.size		= 1,
-		.offset		= 0,
-		.v4l2_type	= V4L2_CTRL_TYPE_BOOLEAN,
-		.data_type	= UVC_CTRL_DATA_TYPE_BOOLEAN,
-	},
 	{
 		.entity		= UVC_GUID_UVC_PROCESSING,
 		.selector	= UVC_PU_POWER_LINE_FREQUENCY_CONTROL,
@@ -2718,9 +2701,6 @@ static int uvc_ctrl_init_chain(struct uvc_video_chain *chain)
 		} else if (UVC_ENTITY_TYPE(entity) == UVC_ITT_CAMERA) {
 			bmControls = entity->camera.bmControls;
 			bControlSize = entity->camera.bControlSize;
-		} else if (UVC_ENTITY_TYPE(entity) == UVC_EXT_GPIO_UNIT) {
-			bmControls = entity->gpio.bmControls;
-			bControlSize = entity->gpio.bControlSize;
 		}
 
 		/* Remove bogus/blacklisted controls */
diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
index 5b48768a4f7f..fe5a53fd0e14 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -1779,6 +1779,7 @@ static void uvc_delete(struct kref *kref)
 	struct uvc_device *dev = container_of(kref, struct uvc_device, ref);
 	struct list_head *p, *n;
 
+	uvc_gpio_cleanup(dev);
 	uvc_status_cleanup(dev);
 	uvc_ctrl_cleanup_device(dev);
 
@@ -2171,6 +2172,7 @@ static int uvc_probe(struct usb_interface *intf,
 	if (media_device_register(&dev->mdev) < 0)
 		goto error;
 #endif
+
 	/* Save our data pointer in the interface data. */
 	usb_set_intfdata(intf, dev);
 
@@ -2182,7 +2184,7 @@ static int uvc_probe(struct usb_interface *intf,
 			 ret);
 	}
 
-	ret = uvc_gpio_init_irq(dev);
+	ret = uvc_gpio_init(dev);
 	if (ret < 0) {
 		dev_err(&dev->udev->dev,
 			"Unable to request privacy GPIO IRQ (%d)\n", ret);
diff --git a/drivers/media/usb/uvc/uvc_entity.c b/drivers/media/usb/uvc/uvc_entity.c
index cc68dd24eb42..c1b69f9eaa56 100644
--- a/drivers/media/usb/uvc/uvc_entity.c
+++ b/drivers/media/usb/uvc/uvc_entity.c
@@ -56,7 +56,13 @@ static int uvc_mc_create_links(struct uvc_video_chain *chain,
 	return 0;
 }
 
+static const struct v4l2_subdev_core_ops uvc_subdev_core_ops = {
+	.subscribe_event = v4l2_ctrl_subdev_subscribe_event,
+	.unsubscribe_event = v4l2_event_subdev_unsubscribe,
+};
+
 static const struct v4l2_subdev_ops uvc_subdev_ops = {
+	.core = &uvc_subdev_core_ops,
 };
 
 void uvc_mc_cleanup_entity(struct uvc_entity *entity)
diff --git a/drivers/media/usb/uvc/uvc_gpio.c b/drivers/media/usb/uvc/uvc_gpio.c
index 453739acbe8f..af72df96bc57 100644
--- a/drivers/media/usb/uvc/uvc_gpio.c
+++ b/drivers/media/usb/uvc/uvc_gpio.c
@@ -7,57 +7,46 @@
 
 #include <linux/kernel.h>
 #include <linux/gpio/consumer.h>
+#include <media/v4l2-ctrls.h>
 #include "uvcvideo.h"
 
-static void uvc_gpio_event(struct uvc_device *dev)
+static int uvc_gpio_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
 {
-	struct uvc_entity *unit = dev->gpio_unit;
-	struct uvc_video_chain *chain;
-	u8 new_val;
-
-	if (!unit)
-		return;
-
-	new_val = gpiod_get_value_cansleep(unit->gpio.gpio_privacy);
-
-	/* GPIO entities are always on the first chain. */
-	chain = list_first_entry(&dev->chains, struct uvc_video_chain, list);
-	uvc_ctrl_status_event(chain, unit->controls, &new_val);
-}
+	struct uvc_gpio *gpio =
+		container_of(ctrl->handler, struct uvc_gpio, hdl);
+	int ret;
 
-static int uvc_gpio_get_cur(struct uvc_device *dev, struct uvc_entity *entity,
-			    u8 cs, void *data, u16 size)
-{
-	if (cs != UVC_CT_PRIVACY_CONTROL || size < 1)
-		return -EINVAL;
+	ret = gpiod_get_value_cansleep(gpio->gpio_privacy);
+	if (ret < 0)
+		return ret;
 
-	*(u8 *)data = gpiod_get_value_cansleep(entity->gpio.gpio_privacy);
+	ctrl->cur.val = ret;
 
 	return 0;
 }
 
-static int uvc_gpio_get_info(struct uvc_device *dev, struct uvc_entity *entity,
-			     u8 cs, u8 *caps)
-{
-	if (cs != UVC_CT_PRIVACY_CONTROL)
-		return -EINVAL;
-
-	*caps = UVC_CONTROL_CAP_GET | UVC_CONTROL_CAP_AUTOUPDATE;
-	return 0;
-}
+static const struct v4l2_ctrl_ops uvc_gpio_ctrl_ops = {
+	.g_volatile_ctrl = uvc_gpio_g_volatile_ctrl,
+};
 
 static irqreturn_t uvc_gpio_irq(int irq, void *data)
 {
-	struct uvc_device *dev = data;
+	struct uvc_gpio *uvc_gpio = data;
+	int new_val;
+
+	new_val = gpiod_get_value_cansleep(uvc_gpio->gpio_privacy);
+	if (new_val < 0)
+		return IRQ_HANDLED;
+
+	v4l2_ctrl_s_ctrl(uvc_gpio->privacy_ctrl, new_val);
 
-	uvc_gpio_event(dev);
 	return IRQ_HANDLED;
 }
 
 int uvc_gpio_parse(struct uvc_device *dev)
 {
-	struct uvc_entity *unit;
 	struct gpio_desc *gpio_privacy;
+	struct uvc_entity *unit;
 	int irq;
 
 	gpio_privacy = devm_gpiod_get_optional(&dev->intf->dev, "privacy",
@@ -67,23 +56,17 @@ int uvc_gpio_parse(struct uvc_device *dev)
 
 	irq = gpiod_to_irq(gpio_privacy);
 	if (irq < 0)
-		return dev_err_probe(&dev->udev->dev, irq,
+		return dev_err_probe(&dev->intf->dev, irq,
 				     "No IRQ for privacy GPIO\n");
 
 	unit = uvc_alloc_new_entity(dev, UVC_EXT_GPIO_UNIT,
-				    UVC_EXT_GPIO_UNIT_ID, 0, 1);
+				    UVC_EXT_GPIO_UNIT_ID, 0, 0);
 	if (IS_ERR(unit))
 		return PTR_ERR(unit);
 
 	unit->gpio.gpio_privacy = gpio_privacy;
 	unit->gpio.irq = irq;
-	unit->gpio.bControlSize = 1;
-	unit->gpio.bmControls = (u8 *)unit + sizeof(*unit);
-	unit->gpio.bmControls[0] = 1;
-	unit->get_cur = uvc_gpio_get_cur;
-	unit->get_info = uvc_gpio_get_info;
 	strscpy(unit->name, "GPIO", sizeof(unit->name));
-
 	list_add_tail(&unit->list, &dev->entities);
 
 	dev->gpio_unit = unit;
@@ -91,21 +74,53 @@ int uvc_gpio_parse(struct uvc_device *dev)
 	return 0;
 }
 
-int uvc_gpio_init_irq(struct uvc_device *dev)
+int uvc_gpio_init(struct uvc_device *dev)
 {
 	struct uvc_entity *unit = dev->gpio_unit;
+	int init_val;
 	int ret;
 
 	if (!unit || unit->gpio.irq < 0)
 		return 0;
 
+	init_val = gpiod_get_value_cansleep(unit->gpio.gpio_privacy);
+	if (init_val < 0)
+		return init_val;
+
+	v4l2_ctrl_handler_init(&unit->gpio.hdl, 1);
+	unit->gpio.privacy_ctrl = v4l2_ctrl_new_std(&unit->gpio.hdl,
+						    &uvc_gpio_ctrl_ops,
+						    V4L2_CID_PRIVACY,
+						    0, 1, 1, init_val);
+	if (!unit->gpio.privacy_ctrl) {
+		ret = unit->gpio.hdl.error;
+		goto cleanup;
+	}
+
+	unit->gpio.privacy_ctrl->flags |= V4L2_CTRL_FLAG_VOLATILE |
+					  V4L2_CTRL_FLAG_READ_ONLY;
+
+	unit->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE |
+			      V4L2_SUBDEV_FL_HAS_EVENTS;
+	unit->subdev.ctrl_handler = &unit->gpio.hdl;
+
+	ret = v4l2_device_register_subdev_nodes(&dev->vdev);
+	if (ret)
+		goto cleanup;
+
 	ret = request_threaded_irq(unit->gpio.irq, NULL, uvc_gpio_irq,
 				   IRQF_ONESHOT | IRQF_TRIGGER_FALLING |
 				   IRQF_TRIGGER_RISING,
-				   "uvc_privacy_gpio", dev);
+				   "uvc_privacy_gpio", &unit->gpio);
+	if (ret)
+		goto cleanup;
 
-	unit->gpio.initialized = !ret;
+	unit->gpio.initialized = true;
 
+	return 0;
+
+cleanup:
+	v4l2_ctrl_handler_free(&unit->gpio.hdl);
 	return ret;
 }
 
@@ -114,5 +129,13 @@ void uvc_gpio_deinit(struct uvc_device *dev)
 	if (!dev->gpio_unit || !dev->gpio_unit->gpio.initialized)
 		return;
 
-	free_irq(dev->gpio_unit->gpio.irq, dev);
+	free_irq(dev->gpio_unit->gpio.irq, &dev->gpio_unit->gpio);
+}
+
+void uvc_gpio_cleanup(struct uvc_device *dev)
+{
+	if (!dev->gpio_unit || !dev->gpio_unit->gpio.initialized)
+		return;
+
+	v4l2_ctrl_handler_free(&dev->gpio_unit->gpio.hdl);
 }
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index 91ed59b54d9a..4a06db329aa9 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -15,6 +15,7 @@
 #include <linux/videodev2.h>
 #include <linux/workqueue.h>
 #include <media/media-device.h>
+#include <media/v4l2-ctrls.h>
 #include <media/v4l2-device.h>
 #include <media/v4l2-event.h>
 #include <media/v4l2-fh.h>
@@ -171,6 +172,14 @@ struct uvc_control {
 
 #define UVC_ENTITY_FLAG_DEFAULT		(1 << 0)
 
+struct uvc_gpio {
+	int irq;
+	bool initialized;
+	struct v4l2_ctrl_handler hdl;
+	struct v4l2_ctrl *privacy_ctrl;
+	struct gpio_desc *gpio_privacy;
+};
+
 struct uvc_entity {
 	struct list_head list;		/* Entity as part of a UVC device. */
 	struct list_head chain;		/* Entity as part of a video device chain. */
@@ -229,13 +238,7 @@ struct uvc_entity {
 			u8  *bmControlsType;
 		} extension;
 
-		struct {
-			u8  bControlSize;
-			u8  *bmControls;
-			struct gpio_desc *gpio_privacy;
-			int irq;
-			bool initialized;
-		} gpio;
+		struct uvc_gpio gpio;
 	};
 
 	u8 bNrInPins;
@@ -822,7 +825,8 @@ size_t uvc_video_stats_dump(struct uvc_streaming *stream, char *buf,
 
 /* gpio */
 int uvc_gpio_parse(struct uvc_device *dev);
-int uvc_gpio_init_irq(struct uvc_device *dev);
+int uvc_gpio_init(struct uvc_device *dev);
 void uvc_gpio_deinit(struct uvc_device *dev);
+void uvc_gpio_cleanup(struct uvc_device *dev);
 
 #endif

-- 
2.47.0.277.g8800431eea-goog


  parent reply	other threads:[~2024-11-08 20:25 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-11-08 20:25 [PATCH v2 0/6] media: uvcvideo: Implement the Privacy GPIO as a subdevice Ricardo Ribalda
2024-11-08 20:25 ` [PATCH v2 1/6] media: uvcvideo: Factor out gpio functions to its own file Ricardo Ribalda
2024-11-08 20:25 ` Ricardo Ribalda [this message]
2024-11-08 20:25 ` [PATCH v2 3/6] Revert "media: uvcvideo: Allow entity-defined get_info and get_cur" Ricardo Ribalda
2024-11-08 20:25 ` [PATCH v2 4/6] media: uvcvideo: Create ancillary link for GPIO subdevice Ricardo Ribalda
2024-11-10 10:05   ` Sakari Ailus
2024-11-08 20:25 ` [PATCH v2 5/6] media: v4l2-core: Add new MEDIA_ENT_F_GPIO Ricardo Ribalda
2024-11-08 20:25 ` [PATCH v2 6/6] media: uvcvideo: Use MEDIA_ENT_F_GPIO for the GPIO entity Ricardo Ribalda
2024-11-09 14:04 ` [PATCH v2 0/6] media: uvcvideo: Implement the Privacy GPIO as a subdevice Mauro Carvalho Chehab
2024-11-09 14:57   ` Ricardo Ribalda
2024-11-09 15:37 ` Hans de Goede
2024-11-09 16:29   ` Ricardo Ribalda
2024-11-10 10:02     ` Mauro Carvalho Chehab
2024-11-10 10:29       ` Hans Verkuil
2024-11-10 10:37         ` Ricardo Ribalda
2024-11-10 10:47           ` Hans Verkuil
2024-11-10 12:48           ` Mauro Carvalho Chehab
2024-11-10 10:32       ` Ricardo Ribalda
2024-11-10 10:59         ` Ricardo Ribalda
2024-11-10 12:46         ` Mauro Carvalho Chehab
2024-11-10 16:01           ` Ricardo Ribalda
2024-11-10 15:14     ` Laurent Pinchart
2024-11-10 16:04       ` Ricardo Ribalda
2024-11-25 12:31         ` Hans de Goede
2024-11-25 12:56           ` Laurent Pinchart
2024-11-25 13:35             ` Hans de Goede
2024-11-10 16:07       ` Ricardo Ribalda
2024-11-25 12:39         ` Hans de Goede
2024-11-25 12:58           ` Laurent Pinchart
2024-11-25 13:44             ` Hans de Goede
2024-11-25 13:50               ` Ricardo Ribalda
2024-11-11 12:03     ` Hans de Goede
2024-11-25 12:25     ` Hans de Goede
2024-11-25 12:49       ` Laurent Pinchart
2024-11-25 13:29         ` Hans de Goede
2024-11-26 17:22           ` Laurent Pinchart
2024-11-25 13:39       ` Ricardo Ribalda
2024-11-25 14:02         ` Hans de Goede
2024-11-26 16:22           ` Ricardo Ribalda
2024-11-26 17:18             ` Laurent Pinchart
2024-11-26 17:29               ` Ricardo Ribalda
2024-11-10 15:08   ` Laurent Pinchart
2024-11-11 12:59 ` Hans de Goede
2024-11-11 14:35   ` Hans de Goede
2024-11-12 17:31   ` Ricardo Ribalda
2024-11-13 15:19     ` Hans de Goede

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20241108-uvc-subdev-v2-2-85d8a051a3d3@chromium.org \
    --to=ribalda@chromium.org \
    --cc=hdegoede@redhat.com \
    --cc=hverkuil@xs4all.nl \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=sakari.ailus@linux.intel.com \
    --cc=yunkec@chromium.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox