| Message ID | 20201022133753.310506-7-ribalda@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Show privacy_gpio as a v4l2_ctrl | expand |
Hi On Thu, Oct 22, 2020 at 3:38 PM Ricardo Ribalda <ribalda@chromium.org> wrote: > > If the privacy pin produces an IRQ, read the gpio and notify userspace > via an event. > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> > --- > drivers/media/usb/uvc/uvc_ctrl.c | 3 +++ > drivers/media/usb/uvc/uvc_driver.c | 30 ++++++++++++++++++++++++++++++ > 2 files changed, 33 insertions(+) > > diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c > index 3a49a1326a90..00c41cba0f68 100644 > --- a/drivers/media/usb/uvc/uvc_ctrl.c > +++ b/drivers/media/usb/uvc/uvc_ctrl.c > @@ -1346,6 +1346,9 @@ static void uvc_ctrl_status_event_work(struct work_struct *work) > > mutex_unlock(&chain->ctrl_mutex); > > + if (!w->urb) > + return; > + > /* Resubmit the URB. */ > w->urb->interval = dev->int_ep->desc.bInterval; > ret = usb_submit_urb(w->urb, GFP_KERNEL); > diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c > index 180e503e900f..d1260d131bd8 100644 > --- a/drivers/media/usb/uvc/uvc_driver.c > +++ b/drivers/media/usb/uvc/uvc_driver.c > @@ -1460,6 +1460,25 @@ static int uvc_gpio_get_info(struct uvc_entity *entity, u8 cs, u8 *caps) > return 0; > } > > +static irqreturn_t uvc_privacy_gpio_irq(int irq, void *data) > +{ > + struct uvc_device *dev = data; > + struct uvc_video_chain *chain; > + struct uvc_entity *term; > + u8 value; > + > + list_for_each_entry(chain, &dev->chains, list) { > + list_for_each_entry(term, &dev->entities, list) { > + if (UVC_ENTITY_TYPE(term) == UVC_GPIO_UNIT) { > + value = gpiod_get_value(term->gpio.gpio_privacy); > + uvc_ctrl_status_event(NULL, chain, term->controls, &value); > + } > + } > + } > + > + return IRQ_HANDLED; > +} > + > static int uvc_parse_gpio(struct uvc_device *dev) > { > struct uvc_entity *unit; > @@ -1490,6 +1509,17 @@ static int uvc_parse_gpio(struct uvc_device *dev) > > list_add_tail(&unit->list, &dev->entities); > > + irq = gpiod_to_irq(gpio_privacy); > + > + if (irq == -EPROBE_DEFER) > + return -EPROBE_DEFER; > + > + if (irq < 0) > + return 0; > + > + ret = devm_request_irq(&dev->udev->dev, irq, uvc_privacy_gpio_irq, IRQF_SHARED, > + "uvc_privacy_gpio", dev); Here it should say IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING instead of IRQF_SHARED. I will fix that on v2, > + > return 0; > } > > -- > 2.29.0.rc1.297.gfa9743e501-goog >
Hi Ricardo, Thank you for the patch. On Thu, Oct 22, 2020 at 03:37:53PM +0200, Ricardo Ribalda wrote: > If the privacy pin produces an IRQ, read the gpio and notify userspace > via an event. > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> > --- > drivers/media/usb/uvc/uvc_ctrl.c | 3 +++ > drivers/media/usb/uvc/uvc_driver.c | 30 ++++++++++++++++++++++++++++++ > 2 files changed, 33 insertions(+) > > diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c > index 3a49a1326a90..00c41cba0f68 100644 > --- a/drivers/media/usb/uvc/uvc_ctrl.c > +++ b/drivers/media/usb/uvc/uvc_ctrl.c > @@ -1346,6 +1346,9 @@ static void uvc_ctrl_status_event_work(struct work_struct *work) > > mutex_unlock(&chain->ctrl_mutex); > > + if (!w->urb) > + return; > + > /* Resubmit the URB. */ > w->urb->interval = dev->int_ep->desc.bInterval; > ret = usb_submit_urb(w->urb, GFP_KERNEL); > diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c > index 180e503e900f..d1260d131bd8 100644 > --- a/drivers/media/usb/uvc/uvc_driver.c > +++ b/drivers/media/usb/uvc/uvc_driver.c > @@ -1460,6 +1460,25 @@ static int uvc_gpio_get_info(struct uvc_entity *entity, u8 cs, u8 *caps) > return 0; > } > > +static irqreturn_t uvc_privacy_gpio_irq(int irq, void *data) > +{ > + struct uvc_device *dev = data; > + struct uvc_video_chain *chain; > + struct uvc_entity *term; The entity isn't a terminal, so I'd call the variable unit. I think there was another occurrence of this issue in another patch in the series. > + u8 value; > + > + list_for_each_entry(chain, &dev->chains, list) { This will effectively call uvc_ctrl_status_event() once per chain, is that really what you were intending ? > + list_for_each_entry(term, &dev->entities, list) { > + if (UVC_ENTITY_TYPE(term) == UVC_GPIO_UNIT) { > + value = gpiod_get_value(term->gpio.gpio_privacy); > + uvc_ctrl_status_event(NULL, chain, term->controls, &value); 80 columns. > + } > + } > + } > + > + return IRQ_HANDLED; > +} > + > static int uvc_parse_gpio(struct uvc_device *dev) > { > struct uvc_entity *unit; > @@ -1490,6 +1509,17 @@ static int uvc_parse_gpio(struct uvc_device *dev) > > list_add_tail(&unit->list, &dev->entities); > > + irq = gpiod_to_irq(gpio_privacy); > + No need for a blank line. And it looks like the local irq variable should be introduced in this patch, not in the previous one. I think I'd squash this with 5/6. > + if (irq == -EPROBE_DEFER) > + return -EPROBE_DEFER; > + > + if (irq < 0) > + return 0; > + > + ret = devm_request_irq(&dev->udev->dev, irq, uvc_privacy_gpio_irq, IRQF_SHARED, 80 columns. > + "uvc_privacy_gpio", dev); Do we need to handle failures ? > + > return 0; > } >
diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c index 3a49a1326a90..00c41cba0f68 100644 --- a/drivers/media/usb/uvc/uvc_ctrl.c +++ b/drivers/media/usb/uvc/uvc_ctrl.c @@ -1346,6 +1346,9 @@ static void uvc_ctrl_status_event_work(struct work_struct *work) mutex_unlock(&chain->ctrl_mutex); + if (!w->urb) + return; + /* Resubmit the URB. */ w->urb->interval = dev->int_ep->desc.bInterval; ret = usb_submit_urb(w->urb, GFP_KERNEL); diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c index 180e503e900f..d1260d131bd8 100644 --- a/drivers/media/usb/uvc/uvc_driver.c +++ b/drivers/media/usb/uvc/uvc_driver.c @@ -1460,6 +1460,25 @@ static int uvc_gpio_get_info(struct uvc_entity *entity, u8 cs, u8 *caps) return 0; } +static irqreturn_t uvc_privacy_gpio_irq(int irq, void *data) +{ + struct uvc_device *dev = data; + struct uvc_video_chain *chain; + struct uvc_entity *term; + u8 value; + + list_for_each_entry(chain, &dev->chains, list) { + list_for_each_entry(term, &dev->entities, list) { + if (UVC_ENTITY_TYPE(term) == UVC_GPIO_UNIT) { + value = gpiod_get_value(term->gpio.gpio_privacy); + uvc_ctrl_status_event(NULL, chain, term->controls, &value); + } + } + } + + return IRQ_HANDLED; +} + static int uvc_parse_gpio(struct uvc_device *dev) { struct uvc_entity *unit; @@ -1490,6 +1509,17 @@ static int uvc_parse_gpio(struct uvc_device *dev) list_add_tail(&unit->list, &dev->entities); + irq = gpiod_to_irq(gpio_privacy); + + if (irq == -EPROBE_DEFER) + return -EPROBE_DEFER; + + if (irq < 0) + return 0; + + ret = devm_request_irq(&dev->udev->dev, irq, uvc_privacy_gpio_irq, IRQF_SHARED, + "uvc_privacy_gpio", dev); + return 0; }
If the privacy pin produces an IRQ, read the gpio and notify userspace via an event. Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> --- drivers/media/usb/uvc/uvc_ctrl.c | 3 +++ drivers/media/usb/uvc/uvc_driver.c | 30 ++++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+)