public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/7] media: uvcvideo: Implement the Privacy GPIO as a subdevice
@ 2024-10-31 13:43 Ricardo Ribalda
  2024-10-31 13:43 ` [PATCH 1/7] media: uvcvideo: Factor out gpio functions to its own file Ricardo Ribalda
                   ` (6 more replies)
  0 siblings, 7 replies; 9+ messages in thread
From: Ricardo Ribalda @ 2024-10-31 13:43 UTC (permalink / raw)
  To: Laurent Pinchart, Mauro Carvalho Chehab, Sakari Ailus
  Cc: linux-kernel, linux-media, Yunke Cao, Hans Verkuil,
	Ricardo Ribalda

Some notebooks have a button to disable the camera (not to be mistaken
with the mechanical cover). This is a standard GPIO linked to the
camera via the ACPI table.

4 years ago we added support for this button in UVC via the Privacy control.
This has two issues:
- If the camera has its own privacy control, it will be masked
- We need to power-up the camera to read the privacy control gpio.

We tried to fix the power-up issues implementing "granular power
saving" but it has been more complicated than anticipated....

Last year, we proposed a patchset to implement the privacy gpio as a
subdevice https://lore.kernel.org/linux-media/20230111-uvc_privacy_subdev-v1-0-f859ac9a01e3@chromium.org/

I think it is a pretty clean solution and makes sense to use a
subdevice for something that is a sub device of the camera :).

This is an attempt to continue with that approach.

Tested on gimble:
gimble-rev3 ~ # v4l2-ctl --all -d /dev/v4l-subdev0
Driver Info:
        Driver version   : 6.6.56
        Capabilities     : 0x00000000
Media Driver Info:
        Driver name      : uvcvideo
        Model            : HP 5M Camera: HP 5M Camera
        Serial           : 0001
        Bus info         : usb-0000:00:14.0-6
        Media version    : 6.6.56
        Hardware revision: 0x00009601 (38401)
        Driver version   : 6.6.56
Interface Info:
        ID               : 0x0300001d
        Type             : V4L Sub-Device
Entity Info:
        ID               : 0x00000013 (19)
        Name             : GPIO
        Function         : Analog Video Decoder

Camera Controls

                        privacy 0x009a0910 (bool)   : default=0 value=0 flags=read-only, volatile

gimble-rev3 ~ # media-ctl  -p
Media controller API version 6.6.56

Media device information
------------------------
driver          uvcvideo
model           HP 5M Camera: HP 5M Camera
serial          0001
bus info        usb-0000:00:14.0-6
hw revision     0x9601
driver version  6.6.56

Device topology
- entity 1: HP 5M Camera: HP 5M Camera (1 pad, 1 link)
            type Node subtype V4L flags 1
            device node name /dev/video0
        pad0: Sink
                <- "Extension 8":1 [ENABLED,IMMUTABLE]

- entity 4: HP 5M Camera: HP 5M Camera (0 pad, 0 link)
            type Node subtype V4L flags 0
            device node name /dev/video1

- entity 8: Extension 8 (2 pads, 2 links, 0 routes)
            type V4L2 subdev subtype Unknown flags 0
        pad0: Sink
                <- "Extension 4":1 [ENABLED,IMMUTABLE]
        pad1: Source
                -> "HP 5M Camera: HP 5M Camera":0 [ENABLED,IMMUTABLE]

- entity 11: Extension 4 (2 pads, 2 links, 0 routes)
             type V4L2 subdev subtype Unknown flags 0
        pad0: Sink
                <- "Processing 2":1 [ENABLED,IMMUTABLE]
        pad1: Source
                -> "Extension 8":0 [ENABLED,IMMUTABLE]

- entity 14: Processing 2 (2 pads, 2 links, 0 routes)
             type V4L2 subdev subtype Unknown flags 0
        pad0: Sink
                <- "Camera 1":0 [ENABLED,IMMUTABLE]
        pad1: Source
                -> "Extension 4":0 [ENABLED,IMMUTABLE]

- entity 17: Camera 1 (1 pad, 1 link, 0 routes)
             type V4L2 subdev subtype Sensor flags 0
        pad0: Source
                -> "Processing 2":0 [ENABLED,IMMUTABLE]

- entity 19: GPIO (0 pad, 0 link, 0 routes)
             type V4L2 subdev subtype Decoder flags 0
             device node name /dev/v4l-subdev0

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
Ricardo Ribalda (6):
      media: uvcvideo: Factor out gpio functions to its own file
      Revert "media: uvcvideo: Allow entity-defined get_info and get_cur"
      media: uvcvideo: Create ancillary link for GPIO subdevice
      media: v4l2-core: Add new MEDIA_ENT_F_GPIO
      media: uvcvideo: Use MEDIA_ENT_F_GPIO for the GPIO entity
      media: uvcvideo: Introduce UVC_QUIRK_PRIVACY_DURING_STREAM

Yunke Cao (1):
      media: uvcvideo: reimplement privacy GPIO as a separate subdevice

 .../userspace-api/media/mediactl/media-types.rst   |   4 +
 drivers/media/usb/uvc/Makefile                     |   3 +-
 drivers/media/usb/uvc/uvc_ctrl.c                   |  40 +----
 drivers/media/usb/uvc/uvc_driver.c                 | 112 +------------
 drivers/media/usb/uvc/uvc_entity.c                 |  20 ++-
 drivers/media/usb/uvc/uvc_gpio.c                   | 178 +++++++++++++++++++++
 drivers/media/usb/uvc/uvc_video.c                  |   4 +
 drivers/media/usb/uvc/uvcvideo.h                   |  31 ++--
 drivers/media/v4l2-core/v4l2-async.c               |   3 +-
 include/uapi/linux/media.h                         |   1 +
 10 files changed, 242 insertions(+), 154 deletions(-)
---
base-commit: c7ccf3683ac9746b263b0502255f5ce47f64fe0a
change-id: 20241030-uvc-subdev-89f4467a00b5

Best regards,
-- 
Ricardo Ribalda <ribalda@chromium.org>


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/7] media: uvcvideo: Factor out gpio functions to its own file
  2024-10-31 13:43 [PATCH 0/7] media: uvcvideo: Implement the Privacy GPIO as a subdevice Ricardo Ribalda
@ 2024-10-31 13:43 ` Ricardo Ribalda
  2024-10-31 13:43 ` [PATCH 2/7] media: uvcvideo: reimplement privacy GPIO as a separate subdevice Ricardo Ribalda
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Ricardo Ribalda @ 2024-10-31 13:43 UTC (permalink / raw)
  To: Laurent Pinchart, Mauro Carvalho Chehab, Sakari Ailus
  Cc: linux-kernel, linux-media, Yunke Cao, Hans Verkuil,
	Ricardo Ribalda

This is just a refactor patch, no new functionality is added.

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 drivers/media/usb/uvc/Makefile     |   3 +-
 drivers/media/usb/uvc/uvc_driver.c | 107 ++-----------------------------------
 drivers/media/usb/uvc/uvc_gpio.c   | 107 +++++++++++++++++++++++++++++++++++++
 drivers/media/usb/uvc/uvcvideo.h   |   7 +++
 4 files changed, 119 insertions(+), 105 deletions(-)

diff --git a/drivers/media/usb/uvc/Makefile b/drivers/media/usb/uvc/Makefile
index 4f9eee4f81ab..85514b6e538f 100644
--- a/drivers/media/usb/uvc/Makefile
+++ b/drivers/media/usb/uvc/Makefile
@@ -1,6 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 uvcvideo-objs  := uvc_driver.o uvc_queue.o uvc_v4l2.o uvc_video.o uvc_ctrl.o \
-		  uvc_status.o uvc_isight.o uvc_debugfs.o uvc_metadata.o
+		  uvc_status.o uvc_isight.o uvc_debugfs.o uvc_metadata.o \
+		  uvc_gpio.o
 ifeq ($(CONFIG_MEDIA_CONTROLLER),y)
 uvcvideo-objs  += uvc_entity.o
 endif
diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
index a96f6ca0889f..94fb8e50a50c 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -8,7 +8,6 @@
 
 #include <linux/atomic.h>
 #include <linux/bits.h>
-#include <linux/gpio/consumer.h>
 #include <linux/kernel.h>
 #include <linux/list.h>
 #include <linux/module.h>
@@ -775,9 +774,9 @@ static const u8 uvc_media_transport_input_guid[16] =
 	UVC_GUID_UVC_MEDIA_TRANSPORT_INPUT;
 static const u8 uvc_processing_guid[16] = UVC_GUID_UVC_PROCESSING;
 
-static struct uvc_entity *uvc_alloc_new_entity(struct uvc_device *dev, u16 type,
-					       u16 id, unsigned int num_pads,
-					       unsigned int extra_size)
+struct uvc_entity *uvc_alloc_new_entity(struct uvc_device *dev, u16 type,
+					u16 id, unsigned int num_pads,
+					unsigned int extra_size)
 {
 	struct uvc_entity *entity;
 	unsigned int num_inputs;
@@ -1240,106 +1239,6 @@ static int uvc_parse_control(struct uvc_device *dev)
 	return 0;
 }
 
-/* -----------------------------------------------------------------------------
- * Privacy GPIO
- */
-
-static void uvc_gpio_event(struct uvc_device *dev)
-{
-	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);
-}
-
-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;
-
-	*(u8 *)data = gpiod_get_value_cansleep(entity->gpio.gpio_privacy);
-
-	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 irqreturn_t uvc_gpio_irq(int irq, void *data)
-{
-	struct uvc_device *dev = data;
-
-	uvc_gpio_event(dev);
-	return IRQ_HANDLED;
-}
-
-static int uvc_gpio_parse(struct uvc_device *dev)
-{
-	struct uvc_entity *unit;
-	struct gpio_desc *gpio_privacy;
-	int irq;
-
-	gpio_privacy = devm_gpiod_get_optional(&dev->udev->dev, "privacy",
-					       GPIOD_IN);
-	if (IS_ERR_OR_NULL(gpio_privacy))
-		return PTR_ERR_OR_ZERO(gpio_privacy);
-
-	irq = gpiod_to_irq(gpio_privacy);
-	if (irq < 0)
-		return dev_err_probe(&dev->udev->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);
-	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;
-
-	return 0;
-}
-
-static int uvc_gpio_init_irq(struct uvc_device *dev)
-{
-	struct uvc_entity *unit = dev->gpio_unit;
-
-	if (!unit || unit->gpio.irq < 0)
-		return 0;
-
-	return devm_request_threaded_irq(&dev->udev->dev, unit->gpio.irq, NULL,
-					 uvc_gpio_irq,
-					 IRQF_ONESHOT | IRQF_TRIGGER_FALLING |
-					 IRQF_TRIGGER_RISING,
-					 "uvc_privacy_gpio", dev);
-}
-
 /* ------------------------------------------------------------------------
  * UVC device scan
  */
diff --git a/drivers/media/usb/uvc/uvc_gpio.c b/drivers/media/usb/uvc/uvc_gpio.c
new file mode 100644
index 000000000000..5b74131795c5
--- /dev/null
+++ b/drivers/media/usb/uvc/uvc_gpio.c
@@ -0,0 +1,107 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ *      uvc_gpio.c  --  USB Video Class driver
+ *
+ *      Copyright 2024 Google LLC
+ */
+
+#include <linux/kernel.h>
+#include <linux/gpio/consumer.h>
+#include "uvcvideo.h"
+
+static void uvc_gpio_event(struct uvc_device *dev)
+{
+	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);
+}
+
+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;
+
+	*(u8 *)data = gpiod_get_value_cansleep(entity->gpio.gpio_privacy);
+
+	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 irqreturn_t uvc_gpio_irq(int irq, void *data)
+{
+	struct uvc_device *dev = data;
+
+	uvc_gpio_event(dev);
+	return IRQ_HANDLED;
+}
+
+int uvc_gpio_parse(struct uvc_device *dev)
+{
+	struct uvc_entity *unit;
+	struct gpio_desc *gpio_privacy;
+	int irq;
+
+	gpio_privacy = devm_gpiod_get_optional(&dev->udev->dev, "privacy",
+					       GPIOD_IN);
+	if (IS_ERR_OR_NULL(gpio_privacy))
+		return PTR_ERR_OR_ZERO(gpio_privacy);
+
+	irq = gpiod_to_irq(gpio_privacy);
+	if (irq < 0)
+		return dev_err_probe(&dev->udev->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);
+	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;
+
+	return 0;
+}
+
+int uvc_gpio_init_irq(struct uvc_device *dev)
+{
+	struct uvc_entity *unit = dev->gpio_unit;
+
+	if (!unit || unit->gpio.irq < 0)
+		return 0;
+
+	return devm_request_threaded_irq(&dev->udev->dev, unit->gpio.irq, NULL,
+					 uvc_gpio_irq,
+					 IRQF_ONESHOT | IRQF_TRIGGER_FALLING |
+					 IRQF_TRIGGER_RISING,
+					 "uvc_privacy_gpio", dev);
+}
+
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index 07f9921d83f2..c7519d59d611 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -672,6 +672,9 @@ do {									\
 extern struct uvc_driver uvc_driver;
 
 struct uvc_entity *uvc_entity_by_id(struct uvc_device *dev, int id);
+struct uvc_entity *uvc_alloc_new_entity(struct uvc_device *dev, u16 type,
+					u16 id, unsigned int num_pads,
+					unsigned int extra_size);
 
 /* Video buffers queue management. */
 int uvc_queue_init(struct uvc_video_queue *queue, enum v4l2_buf_type type,
@@ -816,4 +819,8 @@ void uvc_debugfs_cleanup_stream(struct uvc_streaming *stream);
 size_t uvc_video_stats_dump(struct uvc_streaming *stream, char *buf,
 			    size_t size);
 
+/* gpio */
+int uvc_gpio_init_irq(struct uvc_device *dev);
+int uvc_gpio_parse(struct uvc_device *dev);
+
 #endif

-- 
2.47.0.163.g1226f6d8fa-goog


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/7] media: uvcvideo: reimplement privacy GPIO as a separate subdevice
  2024-10-31 13:43 [PATCH 0/7] media: uvcvideo: Implement the Privacy GPIO as a subdevice Ricardo Ribalda
  2024-10-31 13:43 ` [PATCH 1/7] media: uvcvideo: Factor out gpio functions to its own file Ricardo Ribalda
@ 2024-10-31 13:43 ` Ricardo Ribalda
  2024-10-31 13:43 ` [PATCH 3/7] Revert "media: uvcvideo: Allow entity-defined get_info and get_cur" Ricardo Ribalda
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Ricardo Ribalda @ 2024-10-31 13:43 UTC (permalink / raw)
  To: Laurent Pinchart, Mauro Carvalho Chehab, Sakari Ailus
  Cc: linux-kernel, linux-media, Yunke Cao, Hans Verkuil,
	Ricardo Ribalda

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 |   5 +-
 drivers/media/usb/uvc/uvc_entity.c |   6 ++
 drivers/media/usb/uvc/uvc_gpio.c   | 118 ++++++++++++++++++++++---------------
 drivers/media/usb/uvc/uvcvideo.h   |  18 +++---
 5 files changed, 92 insertions(+), 75 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 94fb8e50a50c..8dacb0783eb1 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -2169,6 +2169,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);
 
@@ -2180,7 +2181,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);
@@ -2207,6 +2208,8 @@ static void uvc_disconnect(struct usb_interface *intf)
 {
 	struct uvc_device *dev = usb_get_intfdata(intf);
 
+	uvc_gpio_cleanup(dev->gpio_unit);
+
 	/*
 	 * Set the USB interface data to NULL. This can be done outside the
 	 * lock, as there's no other reader.
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 5b74131795c5..e02d46ef9566 100644
--- a/drivers/media/usb/uvc/uvc_gpio.c
+++ b/drivers/media/usb/uvc/uvc_gpio.c
@@ -7,83 +7,66 @@
 
 #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;
+	int ret;
+	struct uvc_gpio *gpio =
+		container_of(ctrl->handler, struct uvc_gpio, hdl);
 
-	new_val = gpiod_get_value_cansleep(unit->gpio.gpio_privacy);
+	ret = gpiod_get_value_cansleep(gpio->gpio_privacy);
+	if (ret < 0)
+		return ret;
 
-	/* 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);
-}
-
-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;
-
-	*(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->udev->dev, "privacy",
+	gpio_privacy = devm_gpiod_get_optional(&dev->intf->dev, "privacy",
 					       GPIOD_IN);
 	if (IS_ERR_OR_NULL(gpio_privacy))
 		return PTR_ERR_OR_ZERO(gpio_privacy);
 
 	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,17 +74,58 @@ 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;
 
-	return devm_request_threaded_irq(&dev->udev->dev, unit->gpio.irq, NULL,
-					 uvc_gpio_irq,
-					 IRQF_ONESHOT | IRQF_TRIGGER_FALLING |
-					 IRQF_TRIGGER_RISING,
-					 "uvc_privacy_gpio", dev);
+	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 = devm_request_threaded_irq(&dev->intf->dev, unit->gpio.irq, NULL,
+					uvc_gpio_irq,
+					IRQF_ONESHOT | IRQF_TRIGGER_FALLING |
+					IRQF_TRIGGER_RISING,
+					"uvc_privacy_gpio", &unit->gpio);
+	if (ret)
+		goto cleanup;
+	return 0;
+
+cleanup:
+	v4l2_ctrl_handler_free(&unit->gpio.hdl);
+	return ret;
 }
 
+void uvc_gpio_cleanup(struct uvc_entity *entity)
+{
+	if (!entity)
+		return;
+
+	v4l2_ctrl_handler_free(&entity->gpio.hdl);
+}
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index c7519d59d611..1827f4048f5a 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,13 @@ struct uvc_control {
 
 #define UVC_ENTITY_FLAG_DEFAULT		(1 << 0)
 
+struct uvc_gpio {
+	struct gpio_desc *gpio_privacy;
+	int irq;
+	struct v4l2_ctrl_handler hdl;
+	struct v4l2_ctrl *privacy_ctrl;
+};
+
 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,12 +237,7 @@ struct uvc_entity {
 			u8  *bmControlsType;
 		} extension;
 
-		struct {
-			u8  bControlSize;
-			u8  *bmControls;
-			struct gpio_desc *gpio_privacy;
-			int irq;
-		} gpio;
+		struct uvc_gpio gpio;
 	};
 
 	u8 bNrInPins;
@@ -820,7 +823,8 @@ size_t uvc_video_stats_dump(struct uvc_streaming *stream, char *buf,
 			    size_t size);
 
 /* gpio */
-int uvc_gpio_init_irq(struct uvc_device *dev);
+int uvc_gpio_init(struct uvc_device *dev);
 int uvc_gpio_parse(struct uvc_device *dev);
+void uvc_gpio_cleanup(struct uvc_entity *entity);
 
 #endif

-- 
2.47.0.163.g1226f6d8fa-goog


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/7] Revert "media: uvcvideo: Allow entity-defined get_info and get_cur"
  2024-10-31 13:43 [PATCH 0/7] media: uvcvideo: Implement the Privacy GPIO as a subdevice Ricardo Ribalda
  2024-10-31 13:43 ` [PATCH 1/7] media: uvcvideo: Factor out gpio functions to its own file Ricardo Ribalda
  2024-10-31 13:43 ` [PATCH 2/7] media: uvcvideo: reimplement privacy GPIO as a separate subdevice Ricardo Ribalda
@ 2024-10-31 13:43 ` Ricardo Ribalda
  2024-10-31 13:43 ` [PATCH 4/7] media: uvcvideo: Create ancillary link for GPIO subdevice Ricardo Ribalda
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Ricardo Ribalda @ 2024-10-31 13:43 UTC (permalink / raw)
  To: Laurent Pinchart, Mauro Carvalho Chehab, Sakari Ailus
  Cc: linux-kernel, linux-media, Yunke Cao, Hans Verkuil,
	Ricardo Ribalda

With the privacy gpio now handled as a subdevice, there is no more need
for this.

This reverts commit 65900c581d014499f0f8ceabfc02c652e9a88771.

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 drivers/media/usb/uvc/uvc_ctrl.c | 20 +++++---------------
 drivers/media/usb/uvc/uvcvideo.h |  5 -----
 2 files changed, 5 insertions(+), 20 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
index b98f4778d8aa..90bc2132d8d5 100644
--- a/drivers/media/usb/uvc/uvc_ctrl.c
+++ b/drivers/media/usb/uvc/uvc_ctrl.c
@@ -1087,15 +1087,9 @@ static int __uvc_ctrl_load_cur(struct uvc_video_chain *chain,
 		return 0;
 	}
 
-	if (ctrl->entity->get_cur)
-		ret = ctrl->entity->get_cur(chain->dev, ctrl->entity,
-					    ctrl->info.selector, data,
-					    ctrl->info.size);
-	else
-		ret = uvc_query_ctrl(chain->dev, UVC_GET_CUR,
-				     ctrl->entity->id, chain->dev->intfnum,
-				     ctrl->info.selector, data,
-				     ctrl->info.size);
+	ret = uvc_query_ctrl(chain->dev, UVC_GET_CUR, ctrl->entity->id,
+			     chain->dev->intfnum, ctrl->info.selector, data,
+			     ctrl->info.size);
 
 	if (ret < 0)
 		return ret;
@@ -2055,12 +2049,8 @@ static int uvc_ctrl_get_flags(struct uvc_device *dev,
 	if (data == NULL)
 		return -ENOMEM;
 
-	if (ctrl->entity->get_info)
-		ret = ctrl->entity->get_info(dev, ctrl->entity,
-					     ctrl->info.selector, data);
-	else
-		ret = uvc_query_ctrl(dev, UVC_GET_INFO, ctrl->entity->id,
-				     dev->intfnum, info->selector, data, 1);
+	ret = uvc_query_ctrl(dev, UVC_GET_INFO, ctrl->entity->id, dev->intfnum,
+			     info->selector, data, 1);
 
 	if (!ret) {
 		info->flags &= ~(UVC_CTRL_FLAG_GET_CUR |
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index 1827f4048f5a..37991d35088c 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -243,11 +243,6 @@ struct uvc_entity {
 	u8 bNrInPins;
 	u8 *baSourceID;
 
-	int (*get_info)(struct uvc_device *dev, struct uvc_entity *entity,
-			u8 cs, u8 *caps);
-	int (*get_cur)(struct uvc_device *dev, struct uvc_entity *entity,
-		       u8 cs, void *data, u16 size);
-
 	unsigned int ncontrols;
 	struct uvc_control *controls;
 };

-- 
2.47.0.163.g1226f6d8fa-goog


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 4/7] media: uvcvideo: Create ancillary link for GPIO subdevice
  2024-10-31 13:43 [PATCH 0/7] media: uvcvideo: Implement the Privacy GPIO as a subdevice Ricardo Ribalda
                   ` (2 preceding siblings ...)
  2024-10-31 13:43 ` [PATCH 3/7] Revert "media: uvcvideo: Allow entity-defined get_info and get_cur" Ricardo Ribalda
@ 2024-10-31 13:43 ` Ricardo Ribalda
  2024-10-31 13:43 ` [PATCH 5/7] media: v4l2-core: Add new MEDIA_ENT_F_GPIO Ricardo Ribalda
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Ricardo Ribalda @ 2024-10-31 13:43 UTC (permalink / raw)
  To: Laurent Pinchart, Mauro Carvalho Chehab, Sakari Ailus
  Cc: linux-kernel, linux-media, Yunke Cao, Hans Verkuil,
	Ricardo Ribalda

Make an ancillary device between the streaming subdevice and the GPIO
subdevice.

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 drivers/media/usb/uvc/uvc_entity.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/media/usb/uvc/uvc_entity.c b/drivers/media/usb/uvc/uvc_entity.c
index c1b69f9eaa56..dad77b96fe16 100644
--- a/drivers/media/usb/uvc/uvc_entity.c
+++ b/drivers/media/usb/uvc/uvc_entity.c
@@ -53,6 +53,16 @@ static int uvc_mc_create_links(struct uvc_video_chain *chain,
 			return ret;
 	}
 
+	/* Create ancillary link for the GPIO. */
+	if (chain->dev->gpio_unit && UVC_ENTITY_TYPE(entity) == UVC_ITT_CAMERA) {
+		struct media_link *link;
+
+		link = media_create_ancillary_link(sink,
+					&chain->dev->gpio_unit->subdev.entity);
+		if (IS_ERR(link))
+			return PTR_ERR(link);
+	}
+
 	return 0;
 }
 

-- 
2.47.0.163.g1226f6d8fa-goog


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 5/7] media: v4l2-core: Add new MEDIA_ENT_F_GPIO
  2024-10-31 13:43 [PATCH 0/7] media: uvcvideo: Implement the Privacy GPIO as a subdevice Ricardo Ribalda
                   ` (3 preceding siblings ...)
  2024-10-31 13:43 ` [PATCH 4/7] media: uvcvideo: Create ancillary link for GPIO subdevice Ricardo Ribalda
@ 2024-10-31 13:43 ` Ricardo Ribalda
  2024-10-31 13:43 ` [PATCH 6/7] media: uvcvideo: Use MEDIA_ENT_F_GPIO for the GPIO entity Ricardo Ribalda
  2024-10-31 13:43 ` [PATCH 7/7] media: uvcvideo: Introduce UVC_QUIRK_PRIVACY_DURING_STREAM Ricardo Ribalda
  6 siblings, 0 replies; 9+ messages in thread
From: Ricardo Ribalda @ 2024-10-31 13:43 UTC (permalink / raw)
  To: Laurent Pinchart, Mauro Carvalho Chehab, Sakari Ailus
  Cc: linux-kernel, linux-media, Yunke Cao, Hans Verkuil,
	Ricardo Ribalda

Add a new media entity type to define a GPIO entity. This can be used to
represent the privacy switch GPIO associated to a sensor.

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 Documentation/userspace-api/media/mediactl/media-types.rst | 4 ++++
 drivers/media/v4l2-core/v4l2-async.c                       | 3 ++-
 include/uapi/linux/media.h                                 | 1 +
 3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/Documentation/userspace-api/media/mediactl/media-types.rst b/Documentation/userspace-api/media/mediactl/media-types.rst
index 6332e8395263..7ede3954a96a 100644
--- a/Documentation/userspace-api/media/mediactl/media-types.rst
+++ b/Documentation/userspace-api/media/mediactl/media-types.rst
@@ -24,6 +24,7 @@ Types and flags used to represent the media graph elements
 .. _MEDIA-ENT-F-CAM-SENSOR:
 .. _MEDIA-ENT-F-FLASH:
 .. _MEDIA-ENT-F-LENS:
+.. _MEDIA-ENT-F-GPIO:
 .. _MEDIA-ENT-F-ATV-DECODER:
 .. _MEDIA-ENT-F-TUNER:
 .. _MEDIA-ENT-F-IF-VID-DECODER:
@@ -100,6 +101,9 @@ Types and flags used to represent the media graph elements
     *  -  ``MEDIA_ENT_F_LENS``
        -  Lens controller entity.
 
+    *  -  ``MEDIA_ENT_F_GPIO``
+       -  GPIO controller entity.
+
     *  -  ``MEDIA_ENT_F_ATV_DECODER``
        -  Analog video decoder, the basic function of the video decoder is
 	  to accept analogue video from a wide variety of sources such as
diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
index ee884a8221fb..8a902fc897d1 100644
--- a/drivers/media/v4l2-core/v4l2-async.c
+++ b/drivers/media/v4l2-core/v4l2-async.c
@@ -320,7 +320,8 @@ static int v4l2_async_create_ancillary_links(struct v4l2_async_notifier *n,
 	struct media_link *link;
 
 	if (sd->entity.function != MEDIA_ENT_F_LENS &&
-	    sd->entity.function != MEDIA_ENT_F_FLASH)
+	    sd->entity.function != MEDIA_ENT_F_FLASH &&
+	    sd->entity.function != MEDIA_ENT_F_GPIO)
 		return 0;
 
 	if (!n->sd) {
diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
index 1c80b1d6bbaf..62fc4691923b 100644
--- a/include/uapi/linux/media.h
+++ b/include/uapi/linux/media.h
@@ -84,6 +84,7 @@ struct media_device_info {
 #define MEDIA_ENT_F_CAM_SENSOR			(MEDIA_ENT_F_OLD_SUBDEV_BASE + 1)
 #define MEDIA_ENT_F_FLASH			(MEDIA_ENT_F_OLD_SUBDEV_BASE + 2)
 #define MEDIA_ENT_F_LENS			(MEDIA_ENT_F_OLD_SUBDEV_BASE + 3)
+#define MEDIA_ENT_F_GPIO			(MEDIA_ENT_F_OLD_SUBDEV_BASE + 4)
 
 /*
  * Digital TV, analog TV, radio and/or software defined radio tuner functions.

-- 
2.47.0.163.g1226f6d8fa-goog


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 6/7] media: uvcvideo: Use MEDIA_ENT_F_GPIO for the GPIO entity
  2024-10-31 13:43 [PATCH 0/7] media: uvcvideo: Implement the Privacy GPIO as a subdevice Ricardo Ribalda
                   ` (4 preceding siblings ...)
  2024-10-31 13:43 ` [PATCH 5/7] media: v4l2-core: Add new MEDIA_ENT_F_GPIO Ricardo Ribalda
@ 2024-10-31 13:43 ` Ricardo Ribalda
  2024-11-04  9:37   ` Sergey Senozhatsky
  2024-10-31 13:43 ` [PATCH 7/7] media: uvcvideo: Introduce UVC_QUIRK_PRIVACY_DURING_STREAM Ricardo Ribalda
  6 siblings, 1 reply; 9+ messages in thread
From: Ricardo Ribalda @ 2024-10-31 13:43 UTC (permalink / raw)
  To: Laurent Pinchart, Mauro Carvalho Chehab, Sakari Ailus
  Cc: linux-kernel, linux-media, Yunke Cao, Hans Verkuil,
	Ricardo Ribalda

Right now we are setting the entity type to unknown for the privacy GPIO
entity. Which results in the following error in dmesg.
uvcvideo 3-6:1.0: Entity type for entity GPIO was not initialized!

Use the newly created type to fix this.

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 drivers/media/usb/uvc/uvc_entity.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/media/usb/uvc/uvc_entity.c b/drivers/media/usb/uvc/uvc_entity.c
index dad77b96fe16..3cb95df1f670 100644
--- a/drivers/media/usb/uvc/uvc_entity.c
+++ b/drivers/media/usb/uvc/uvc_entity.c
@@ -114,6 +114,9 @@ static int uvc_mc_init_entity(struct uvc_video_chain *chain,
 		case UVC_ITT_CAMERA:
 			function = MEDIA_ENT_F_CAM_SENSOR;
 			break;
+		case UVC_EXT_GPIO_UNIT:
+			function = MEDIA_ENT_F_GPIO;
+			break;
 		case UVC_TT_VENDOR_SPECIFIC:
 		case UVC_ITT_VENDOR_SPECIFIC:
 		case UVC_ITT_MEDIA_TRANSPORT_INPUT:
@@ -121,7 +124,6 @@ static int uvc_mc_init_entity(struct uvc_video_chain *chain,
 		case UVC_OTT_DISPLAY:
 		case UVC_OTT_MEDIA_TRANSPORT_OUTPUT:
 		case UVC_EXTERNAL_VENDOR_SPECIFIC:
-		case UVC_EXT_GPIO_UNIT:
 		default:
 			function = MEDIA_ENT_F_V4L2_SUBDEV_UNKNOWN;
 			break;

-- 
2.47.0.163.g1226f6d8fa-goog


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 7/7] media: uvcvideo: Introduce UVC_QUIRK_PRIVACY_DURING_STREAM
  2024-10-31 13:43 [PATCH 0/7] media: uvcvideo: Implement the Privacy GPIO as a subdevice Ricardo Ribalda
                   ` (5 preceding siblings ...)
  2024-10-31 13:43 ` [PATCH 6/7] media: uvcvideo: Use MEDIA_ENT_F_GPIO for the GPIO entity Ricardo Ribalda
@ 2024-10-31 13:43 ` Ricardo Ribalda
  6 siblings, 0 replies; 9+ messages in thread
From: Ricardo Ribalda @ 2024-10-31 13:43 UTC (permalink / raw)
  To: Laurent Pinchart, Mauro Carvalho Chehab, Sakari Ailus
  Cc: linux-kernel, linux-media, Yunke Cao, Hans Verkuil,
	Ricardo Ribalda

Some devices power the GPIO pull-up with the same power-supply as the
camera. Avoid reading the GPIO if the device is not streaming.

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 drivers/media/usb/uvc/uvc_gpio.c  | 47 +++++++++++++++++++++++++++++++++++++++
 drivers/media/usb/uvc/uvc_video.c |  4 ++++
 drivers/media/usb/uvc/uvcvideo.h  |  7 ++++--
 3 files changed, 56 insertions(+), 2 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_gpio.c b/drivers/media/usb/uvc/uvc_gpio.c
index e02d46ef9566..b49a7fbd5adf 100644
--- a/drivers/media/usb/uvc/uvc_gpio.c
+++ b/drivers/media/usb/uvc/uvc_gpio.c
@@ -5,6 +5,7 @@
  *      Copyright 2024 Google LLC
  */
 
+#include <linux/dmi.h>
 #include <linux/kernel.h>
 #include <linux/gpio/consumer.h>
 #include <media/v4l2-ctrls.h>
@@ -16,6 +17,9 @@ static int uvc_gpio_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
 	struct uvc_gpio *gpio =
 		container_of(ctrl->handler, struct uvc_gpio, hdl);
 
+	if (!gpio->gpio_ready)
+		return -EBUSY;
+
 	ret = gpiod_get_value_cansleep(gpio->gpio_privacy);
 	if (ret < 0)
 		return ret;
@@ -43,6 +47,24 @@ static irqreturn_t uvc_gpio_irq(int irq, void *data)
 	return IRQ_HANDLED;
 }
 
+static const struct dmi_system_id privacy_valid_during_streamon[] = {
+	{
+		.ident = "HP Elite c1030 Chromebook",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "HP"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "Jinlon"),
+		},
+	},
+	{
+		.ident = "HP Pro c640 Chromebook",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "HP"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "Dratini"),
+		},
+	},
+	{ } /* terminate list */
+};
+
 int uvc_gpio_parse(struct uvc_device *dev)
 {
 	struct gpio_desc *gpio_privacy;
@@ -64,6 +86,15 @@ int uvc_gpio_parse(struct uvc_device *dev)
 	if (IS_ERR(unit))
 		return PTR_ERR(unit);
 
+	/*
+	 * Note: This quirk will not match external UVC cameras,
+	 * as they will not have the corresponding ACPI GPIO entity.
+	 */
+	if (dmi_check_system(privacy_valid_during_streamon))
+		dev->quirks |= UVC_QUIRK_PRIVACY_DURING_STREAM;
+	else
+		unit->gpio.gpio_ready = true;
+
 	unit->gpio.gpio_privacy = gpio_privacy;
 	unit->gpio.irq = irq;
 	strscpy(unit->name, "GPIO", sizeof(unit->name));
@@ -74,6 +105,20 @@ int uvc_gpio_parse(struct uvc_device *dev)
 	return 0;
 }
 
+void uvc_gpio_quirk(struct uvc_device *dev, bool stream_on)
+{
+	if (!dev->gpio_unit || !(dev->quirks & UVC_QUIRK_PRIVACY_DURING_STREAM))
+		return;
+
+	dev->gpio_unit->gpio.gpio_ready = stream_on;
+	if (stream_on) {
+		enable_irq(dev->gpio_unit->gpio.irq);
+		uvc_gpio_irq(0, &dev->gpio_unit->gpio);
+	} else {
+		disable_irq(dev->gpio_unit->gpio.irq);
+	}
+}
+
 int uvc_gpio_init(struct uvc_device *dev)
 {
 	struct uvc_entity *unit = dev->gpio_unit;
@@ -97,6 +142,8 @@ int uvc_gpio_init(struct uvc_device *dev)
 		goto cleanup;
 	}
 
+	uvc_gpio_quirk(dev, false);
+
 	unit->gpio.privacy_ctrl->flags |= V4L2_CTRL_FLAG_VOLATILE |
 					  V4L2_CTRL_FLAG_READ_ONLY;
 
diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
index cd9c29532fb0..0d542176ccde 100644
--- a/drivers/media/usb/uvc/uvc_video.c
+++ b/drivers/media/usb/uvc/uvc_video.c
@@ -2296,6 +2296,8 @@ int uvc_video_start_streaming(struct uvc_streaming *stream)
 	if (ret < 0)
 		goto error_video;
 
+	uvc_gpio_quirk(stream->dev, true);
+
 	return 0;
 
 error_video:
@@ -2308,6 +2310,8 @@ int uvc_video_start_streaming(struct uvc_streaming *stream)
 
 void uvc_video_stop_streaming(struct uvc_streaming *stream)
 {
+	uvc_gpio_quirk(stream->dev, false);
+
 	uvc_video_stop_transfer(stream, 1);
 
 	if (stream->intf->num_altsetting > 1) {
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index 37991d35088c..f154cb2932a0 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -77,6 +77,7 @@
 #define UVC_QUIRK_NO_RESET_RESUME	0x00004000
 #define UVC_QUIRK_DISABLE_AUTOSUSPEND	0x00008000
 #define UVC_QUIRK_INVALID_DEVICE_SOF	0x00010000
+#define UVC_QUIRK_PRIVACY_DURING_STREAM	0x00020000
 
 /* Format flags */
 #define UVC_FMT_FLAG_COMPRESSED		0x00000001
@@ -173,10 +174,11 @@ struct uvc_control {
 #define UVC_ENTITY_FLAG_DEFAULT		(1 << 0)
 
 struct uvc_gpio {
-	struct gpio_desc *gpio_privacy;
+	bool gpio_ready;
 	int irq;
-	struct v4l2_ctrl_handler hdl;
 	struct v4l2_ctrl *privacy_ctrl;
+	struct v4l2_ctrl_handler hdl;
+	struct gpio_desc *gpio_privacy;
 };
 
 struct uvc_entity {
@@ -821,5 +823,6 @@ size_t uvc_video_stats_dump(struct uvc_streaming *stream, char *buf,
 int uvc_gpio_init(struct uvc_device *dev);
 int uvc_gpio_parse(struct uvc_device *dev);
 void uvc_gpio_cleanup(struct uvc_entity *entity);
+void uvc_gpio_quirk(struct uvc_device *dev, bool stream_on);
 
 #endif

-- 
2.47.0.163.g1226f6d8fa-goog


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 6/7] media: uvcvideo: Use MEDIA_ENT_F_GPIO for the GPIO entity
  2024-10-31 13:43 ` [PATCH 6/7] media: uvcvideo: Use MEDIA_ENT_F_GPIO for the GPIO entity Ricardo Ribalda
@ 2024-11-04  9:37   ` Sergey Senozhatsky
  0 siblings, 0 replies; 9+ messages in thread
From: Sergey Senozhatsky @ 2024-11-04  9:37 UTC (permalink / raw)
  To: Ricardo Ribalda
  Cc: Laurent Pinchart, Mauro Carvalho Chehab, Sakari Ailus,
	linux-kernel, linux-media, Yunke Cao, Hans Verkuil

On (24/10/31 13:43), Ricardo Ribalda wrote:
> Right now we are setting the entity type to unknown for the privacy GPIO
> entity. Which results in the following error in dmesg.
> uvcvideo 3-6:1.0: Entity type for entity GPIO was not initialized!

Should this be squashed with the previous patch?

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2024-11-04  9:37 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-31 13:43 [PATCH 0/7] media: uvcvideo: Implement the Privacy GPIO as a subdevice Ricardo Ribalda
2024-10-31 13:43 ` [PATCH 1/7] media: uvcvideo: Factor out gpio functions to its own file Ricardo Ribalda
2024-10-31 13:43 ` [PATCH 2/7] media: uvcvideo: reimplement privacy GPIO as a separate subdevice Ricardo Ribalda
2024-10-31 13:43 ` [PATCH 3/7] Revert "media: uvcvideo: Allow entity-defined get_info and get_cur" Ricardo Ribalda
2024-10-31 13:43 ` [PATCH 4/7] media: uvcvideo: Create ancillary link for GPIO subdevice Ricardo Ribalda
2024-10-31 13:43 ` [PATCH 5/7] media: v4l2-core: Add new MEDIA_ENT_F_GPIO Ricardo Ribalda
2024-10-31 13:43 ` [PATCH 6/7] media: uvcvideo: Use MEDIA_ENT_F_GPIO for the GPIO entity Ricardo Ribalda
2024-11-04  9:37   ` Sergey Senozhatsky
2024-10-31 13:43 ` [PATCH 7/7] media: uvcvideo: Introduce UVC_QUIRK_PRIVACY_DURING_STREAM Ricardo Ribalda

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox