From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 83461C282DD for ; Fri, 10 Jan 2020 14:36:00 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 56D8C2082E for ; Fri, 10 Jan 2020 14:36:00 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=raspberrypi.com header.i=@raspberrypi.com header.b="oMKyFTUn" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728201AbgAJOf7 (ORCPT ); Fri, 10 Jan 2020 09:35:59 -0500 Received: from mail-wr1-f67.google.com ([209.85.221.67]:39194 "EHLO mail-wr1-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727581AbgAJOf6 (ORCPT ); Fri, 10 Jan 2020 09:35:58 -0500 Received: by mail-wr1-f67.google.com with SMTP id y11so2026771wrt.6 for ; Fri, 10 Jan 2020 06:35:57 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=raspberrypi.com; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=fR6lP1zgvDKu/Vk7f95tLyKjHNECmLy4ZODwU6XXvRk=; b=oMKyFTUnPn8RNJpLhn1q02KOjxZJlZZ9VPykA25mq7Qu2JP+ihwHk0gSWGLtpxMDz2 /AQfwwbC0TD/nXTTKi57chkFmPcoxO7vB8UkXx8YLSu1qkSDKVXKAIQHE4L4mtNkUK6B EEQjSgImvLDw8OhHqbgHe4K+SgwLMVLzgxRjolX/wSuQyynOf587dS5kGTkkrmHUIHlM /B/MDdLk0zENjZMUh1aBEDzP1Bbb2VgndUwbPqka0QYTdhNNZNEaKohUSiUYK8Bwh1uh hZTG7FonKTzkOi85mDmfoak7tBoRe7uxQ/W+MYnSKt+ABcPjCvy048Uy1gZbt/SUVS7z OCZg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=fR6lP1zgvDKu/Vk7f95tLyKjHNECmLy4ZODwU6XXvRk=; b=XmSzAGdB/s9T/wyOxrsgnTjZJhr0PoUD9AKNRRz4BuvjAgmwQqDEI2YIexb80Ibbgm Wr+RqDOI9+VVvTkSFzcPL1sArclEotekKjxIqunCZado8PDFYfOl8ymINcfBSMcqbl7x DrWANHKEcumBL2Mp26YXoqlceWg5vnnOtiqYcjz/Na6Ns7YRiFQuykYdlSqxRlTvkxtN HP24hJ8Ngva12rom4i2YcwRIdio8Oqr5EFQpDzoXVALwnHT3nhqEQ4xXO+R5mXs3B4ee vjxQEfVvf0aJFJ38HkyUzqtumaL7URckg62I5tBsen6olT6meCpB9KadSKfsFh4KVa+e vn3w== X-Gm-Message-State: APjAAAUQtJ6KX0I5qO6tstOyluDwgGPeIUa6cYrrzbZjcOSy8fStIipN H6KvKBDRYjSUVVkui9FiDw4L2JUT3wdUmMcXMCYi3LFJKLSAtg== X-Google-Smtp-Source: APXvYqxEFT4t6ci312y+/g7e9vAaR90TUgHTNowFXe9gfh6DUIG6dMP98qv1vvSieZtNtI/KYxW53dyyPKyUy7ah+Nk= X-Received: by 2002:adf:fc4b:: with SMTP id e11mr3904527wrs.326.1578666956616; Fri, 10 Jan 2020 06:35:56 -0800 (PST) MIME-Version: 1.0 References: <20191206085432.19962-1-michael.kupfer@fau.de> <3db2350b-0a6d-0693-258c-9d47f71c0627@xs4all.nl> In-Reply-To: <3db2350b-0a6d-0693-258c-9d47f71c0627@xs4all.nl> From: Dave Stevenson Date: Fri, 10 Jan 2020 14:35:38 +0000 Message-ID: Subject: Re: [PATCH] staging/vc04_services/bcm2835-camera: distinct numeration and names for devices To: Hans Verkuil Cc: Michael Kupfer , eric@anholt.net, wahrenst@gmx.net, bcm-kernel-feedback-list@broadcom.com, linux-rpi-kernel@lists.infradead.org, linux-arm-kernel@lists.infradead.org, mchehab+samsung@kernel.org, linux-media@vger.kernel.org, gregkh@linuxfoundation.org, f.fainelli@gmail.com, rjui@broadcom.com, sbranden@broadcom.com, Dave Stevenson , daniela.mormocea@gmail.com, devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org, linux-kernel@i4.cs.fau.de, Kay Friedrich Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Hans On Fri, 10 Jan 2020 at 13:25, Hans Verkuil wrote: > > Hi Michael, Kay, > > On 12/6/19 9:54 AM, Michael Kupfer wrote: > > Create a static atomic counter for numerating cameras. > > Use the Media Subsystem Kernel Internal API to create distinct > > device-names, so that the camera-number (given by the counter) > > matches the camera-name. > > > > Co-developed-by: Kay Friedrich > > Signed-off-by: Kay Friedrich > > Signed-off-by: Michael Kupfer > > --- > > .../vc04_services/bcm2835-camera/bcm2835-camera.c | 9 ++++++--- > > 1 file changed, 6 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c > > index beb6a0063bb8..be5f90a8b49d 100644 > > --- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c > > +++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c > > @@ -60,6 +60,9 @@ MODULE_PARM_DESC(max_video_width, "Threshold for video mode"); > > module_param(max_video_height, int, 0644); > > MODULE_PARM_DESC(max_video_height, "Threshold for video mode"); > > > > +/* camera instance counter */ > > +static atomic_t camera_instance = ATOMIC_INIT(0); > > + > > /* global device data array */ > > static struct bm2835_mmal_dev *gdev[MAX_BCM2835_CAMERAS]; > > > > @@ -1870,7 +1873,6 @@ static int bcm2835_mmal_probe(struct platform_device *pdev) > > > > /* v4l2 core mutex used to protect all fops and v4l2 ioctls. */ > > mutex_init(&dev->mutex); > > - dev->camera_num = camera; > > dev->max_width = resolutions[camera][0]; > > dev->max_height = resolutions[camera][1]; > > > > @@ -1886,8 +1888,9 @@ static int bcm2835_mmal_probe(struct platform_device *pdev) > > dev->capture.fmt = &formats[3]; /* JPEG */ > > > > /* v4l device registration */ > > - snprintf(dev->v4l2_dev.name, sizeof(dev->v4l2_dev.name), > > - "%s", BM2835_MMAL_MODULE_NAME); > > + dev->camera_num = v4l2_device_set_name(&dev->v4l2_dev, > > + BM2835_MMAL_MODULE_NAME, > > + &camera_instance); > > ret = v4l2_device_register(NULL, &dev->v4l2_dev); > > if (ret) { > > dev_err(&pdev->dev, "%s: could not register V4L2 device: %d\n", > > > > Actually, in this specific case I would not use v4l2_device_set_name(). > > Instead just use: > > snprintf(dev->v4l2_dev.name, sizeof(dev->v4l2_dev.name), > "%s-%u", BM2835_MMAL_MODULE_NAME, camera); > > It would be even better if there would be just one top-level v4l2_device used > for all the camera instances. After all, there really is just one platform > device for all of the cameras, and I would expect to see just a single > v4l2_device as well. > > It doesn't hurt to have multiple v4l2_device structs, but it introduces a > slight memory overhead since one would have been sufficient. Doesn't that make all controls for all cameras common? The struct v4l2_ctrl_handler is part of struct v4l2_device. Or do we: - ditch the use of ctrl_handler in struct v4l2_device - create and initialise a ctrl_handler per camera on an internal structure so we retain the control state - assign ctrl_handler in struct v4l2_fh to it every time a file handle on the device is opened? And if we only have one struct v4l2_device then is there the possibility of the unique names that Michael and Kay are trying to introduce? I'm a little confused as to whether there really is a gain in having a single v4l2_device. In this case the two cameras are independent devices, even if they are loaded by a single platform driver. Dave > v4l2_device_set_name() is meant for pci-like devices. And it really > is a bit overkill to have it as a helper function. > > Regards, > > Hans From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.5 required=3.0 tests=DKIM_INVALID,DKIM_SIGNED, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id EB817C282DD for ; Fri, 10 Jan 2020 14:57:26 +0000 (UTC) Received: from silver.osuosl.org (smtp3.osuosl.org [140.211.166.136]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id AA30920721 for ; Fri, 10 Jan 2020 14:57:26 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=raspberrypi.com header.i=@raspberrypi.com header.b="oMKyFTUn" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org AA30920721 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=raspberrypi.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=driverdev-devel-bounces@linuxdriverproject.org Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id 72D86221CC; Fri, 10 Jan 2020 14:57:26 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from silver.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id szPkRSECGw5a; Fri, 10 Jan 2020 14:57:25 +0000 (UTC) Received: from ash.osuosl.org (ash.osuosl.org [140.211.166.34]) by silver.osuosl.org (Postfix) with ESMTP id 09BF32214F; Fri, 10 Jan 2020 14:57:25 +0000 (UTC) Received: from whitealder.osuosl.org (smtp1.osuosl.org [140.211.166.138]) by ash.osuosl.org (Postfix) with ESMTP id 5AC421BF863 for ; Fri, 10 Jan 2020 14:57:23 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id 4EBFE875E0 for ; Fri, 10 Jan 2020 14:57:23 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from whitealder.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id OFoOHHx5m8iz for ; Fri, 10 Jan 2020 14:57:21 +0000 (UTC) X-Greylist: delayed 00:15:38 by SQLgrey-1.7.6 Received: from mail-lj1-f193.google.com (mail-lj1-f193.google.com [209.85.208.193]) by whitealder.osuosl.org (Postfix) with ESMTPS id 9DA0687582 for ; Fri, 10 Jan 2020 14:57:21 +0000 (UTC) Received: by mail-lj1-f193.google.com with SMTP id y4so2425765ljj.9 for ; Fri, 10 Jan 2020 06:57:21 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=raspberrypi.com; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=fR6lP1zgvDKu/Vk7f95tLyKjHNECmLy4ZODwU6XXvRk=; b=oMKyFTUnPn8RNJpLhn1q02KOjxZJlZZ9VPykA25mq7Qu2JP+ihwHk0gSWGLtpxMDz2 /AQfwwbC0TD/nXTTKi57chkFmPcoxO7vB8UkXx8YLSu1qkSDKVXKAIQHE4L4mtNkUK6B EEQjSgImvLDw8OhHqbgHe4K+SgwLMVLzgxRjolX/wSuQyynOf587dS5kGTkkrmHUIHlM /B/MDdLk0zENjZMUh1aBEDzP1Bbb2VgndUwbPqka0QYTdhNNZNEaKohUSiUYK8Bwh1uh hZTG7FonKTzkOi85mDmfoak7tBoRe7uxQ/W+MYnSKt+ABcPjCvy048Uy1gZbt/SUVS7z OCZg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=fR6lP1zgvDKu/Vk7f95tLyKjHNECmLy4ZODwU6XXvRk=; b=uTqOF/EBK+4iw+UM2HnLKtssdot+KvwSWIqAYbvOclR3+8r2mMm1oZ/DxJAA/QQca6 ES71AVBi0gLqSbZRmIJU1L7OBk/OtWqo0N4M6XYORTLsT/Rv72NJJfSeE97ekwIIEKIi 4mYMnLkM7pKK4M7AZU3FtEAQFBItZPpFrEO/tEkrCP1TyJ63EX0u4r4puFw1TMNtjLM5 bFy1DANKV4owC+BCDuq21GqNTQT3LM0Rf1WELSCfaJUqVy1NQHk7d5rlr7dtlnbHer8x E49dOnJ8aj/R5UjsYxOJz83Muqb8hs3mycjjBM5ZrnHI110wl+Zo+zELKb6xOCy2Ln6n XBYw== X-Gm-Message-State: APjAAAVm2jDJUplFTdtju1i2EibWkcT8woFUAErlg4v9OOh4mCSLpmQR 26CqlpW2Aa8ICgCBMB8MDFXGLdSp4LXrIC+6S/pOFHx0H2Rhgg== X-Google-Smtp-Source: APXvYqxEFT4t6ci312y+/g7e9vAaR90TUgHTNowFXe9gfh6DUIG6dMP98qv1vvSieZtNtI/KYxW53dyyPKyUy7ah+Nk= X-Received: by 2002:adf:fc4b:: with SMTP id e11mr3904527wrs.326.1578666956616; Fri, 10 Jan 2020 06:35:56 -0800 (PST) MIME-Version: 1.0 References: <20191206085432.19962-1-michael.kupfer@fau.de> <3db2350b-0a6d-0693-258c-9d47f71c0627@xs4all.nl> In-Reply-To: <3db2350b-0a6d-0693-258c-9d47f71c0627@xs4all.nl> From: Dave Stevenson Date: Fri, 10 Jan 2020 14:35:38 +0000 Message-ID: Subject: Re: [PATCH] staging/vc04_services/bcm2835-camera: distinct numeration and names for devices To: Hans Verkuil X-BeenThere: driverdev-devel@linuxdriverproject.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Linux Driver Project Developer List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: devel@driverdev.osuosl.org, f.fainelli@gmail.com, sbranden@broadcom.com, Michael Kupfer , linux-kernel@i4.cs.fau.de, gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org, eric@anholt.net, daniela.mormocea@gmail.com, bcm-kernel-feedback-list@broadcom.com, wahrenst@gmx.net, Dave Stevenson , rjui@broadcom.com, mchehab+samsung@kernel.org, linux-media@vger.kernel.org, Kay Friedrich , linux-arm-kernel@lists.infradead.org, linux-rpi-kernel@lists.infradead.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: driverdev-devel-bounces@linuxdriverproject.org Sender: "devel" Hi Hans On Fri, 10 Jan 2020 at 13:25, Hans Verkuil wrote: > > Hi Michael, Kay, > > On 12/6/19 9:54 AM, Michael Kupfer wrote: > > Create a static atomic counter for numerating cameras. > > Use the Media Subsystem Kernel Internal API to create distinct > > device-names, so that the camera-number (given by the counter) > > matches the camera-name. > > > > Co-developed-by: Kay Friedrich > > Signed-off-by: Kay Friedrich > > Signed-off-by: Michael Kupfer > > --- > > .../vc04_services/bcm2835-camera/bcm2835-camera.c | 9 ++++++--- > > 1 file changed, 6 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c > > index beb6a0063bb8..be5f90a8b49d 100644 > > --- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c > > +++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c > > @@ -60,6 +60,9 @@ MODULE_PARM_DESC(max_video_width, "Threshold for video mode"); > > module_param(max_video_height, int, 0644); > > MODULE_PARM_DESC(max_video_height, "Threshold for video mode"); > > > > +/* camera instance counter */ > > +static atomic_t camera_instance = ATOMIC_INIT(0); > > + > > /* global device data array */ > > static struct bm2835_mmal_dev *gdev[MAX_BCM2835_CAMERAS]; > > > > @@ -1870,7 +1873,6 @@ static int bcm2835_mmal_probe(struct platform_device *pdev) > > > > /* v4l2 core mutex used to protect all fops and v4l2 ioctls. */ > > mutex_init(&dev->mutex); > > - dev->camera_num = camera; > > dev->max_width = resolutions[camera][0]; > > dev->max_height = resolutions[camera][1]; > > > > @@ -1886,8 +1888,9 @@ static int bcm2835_mmal_probe(struct platform_device *pdev) > > dev->capture.fmt = &formats[3]; /* JPEG */ > > > > /* v4l device registration */ > > - snprintf(dev->v4l2_dev.name, sizeof(dev->v4l2_dev.name), > > - "%s", BM2835_MMAL_MODULE_NAME); > > + dev->camera_num = v4l2_device_set_name(&dev->v4l2_dev, > > + BM2835_MMAL_MODULE_NAME, > > + &camera_instance); > > ret = v4l2_device_register(NULL, &dev->v4l2_dev); > > if (ret) { > > dev_err(&pdev->dev, "%s: could not register V4L2 device: %d\n", > > > > Actually, in this specific case I would not use v4l2_device_set_name(). > > Instead just use: > > snprintf(dev->v4l2_dev.name, sizeof(dev->v4l2_dev.name), > "%s-%u", BM2835_MMAL_MODULE_NAME, camera); > > It would be even better if there would be just one top-level v4l2_device used > for all the camera instances. After all, there really is just one platform > device for all of the cameras, and I would expect to see just a single > v4l2_device as well. > > It doesn't hurt to have multiple v4l2_device structs, but it introduces a > slight memory overhead since one would have been sufficient. Doesn't that make all controls for all cameras common? The struct v4l2_ctrl_handler is part of struct v4l2_device. Or do we: - ditch the use of ctrl_handler in struct v4l2_device - create and initialise a ctrl_handler per camera on an internal structure so we retain the control state - assign ctrl_handler in struct v4l2_fh to it every time a file handle on the device is opened? And if we only have one struct v4l2_device then is there the possibility of the unique names that Michael and Kay are trying to introduce? I'm a little confused as to whether there really is a gain in having a single v4l2_device. In this case the two cameras are independent devices, even if they are loaded by a single platform driver. Dave > v4l2_device_set_name() is meant for pci-like devices. And it really > is a bit overkill to have it as a helper function. > > Regards, > > Hans _______________________________________________ devel mailing list devel@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9FBC8C282DD for ; Fri, 10 Jan 2020 14:36:07 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 72BBF2077C for ; Fri, 10 Jan 2020 14:36:07 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="ZCc5hrd8"; dkim=fail reason="signature verification failed" (2048-bit key) header.d=raspberrypi.com header.i=@raspberrypi.com header.b="oMKyFTUn" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 72BBF2077C Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=raspberrypi.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:To:Subject:Message-ID:Date:From: In-Reply-To:References:MIME-Version:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=sGd71FcKpHUWC2u9Qo/HKDtTrefxvkYJ+gJTxv0IVVA=; b=ZCc5hrd8Rw1Byf mDaggidcSKUDduoSRr3Enr3v9noOpg13XxtowedBBNhnjr6jKjYQ7RhwiNQXdk21PJXC5J96Uy/on gkKV3ssSnDDl1ouWCfRZVTB9b/23SWgXLRwtkdJe7U+4gk6QdkIq7GortF08uNHwTCDxX6Vzkxj1d AyoMkkekbawsGwt36Poc7WjefJfWozOu0gm7r303SymBAd3wHHital6oNX/iJi+w8fBLSa5UmtY7y KkIW4Sfn69k7iIfCO8cObPnUNQZmJBiT5TtHv29ZYmc352oqoduyZx7K8W2HNHV0OcsA9/7I8uOyS RpD3tiJno64JzmNNIIlA==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1ipvON-0003s9-W0; Fri, 10 Jan 2020 14:36:03 +0000 Received: from mail-wr1-x443.google.com ([2a00:1450:4864:20::443]) by bombadil.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1ipvOL-0003r5-9S for linux-arm-kernel@lists.infradead.org; Fri, 10 Jan 2020 14:36:02 +0000 Received: by mail-wr1-x443.google.com with SMTP id t2so2050645wrr.1 for ; Fri, 10 Jan 2020 06:35:57 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=raspberrypi.com; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=fR6lP1zgvDKu/Vk7f95tLyKjHNECmLy4ZODwU6XXvRk=; b=oMKyFTUnPn8RNJpLhn1q02KOjxZJlZZ9VPykA25mq7Qu2JP+ihwHk0gSWGLtpxMDz2 /AQfwwbC0TD/nXTTKi57chkFmPcoxO7vB8UkXx8YLSu1qkSDKVXKAIQHE4L4mtNkUK6B EEQjSgImvLDw8OhHqbgHe4K+SgwLMVLzgxRjolX/wSuQyynOf587dS5kGTkkrmHUIHlM /B/MDdLk0zENjZMUh1aBEDzP1Bbb2VgndUwbPqka0QYTdhNNZNEaKohUSiUYK8Bwh1uh hZTG7FonKTzkOi85mDmfoak7tBoRe7uxQ/W+MYnSKt+ABcPjCvy048Uy1gZbt/SUVS7z OCZg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=fR6lP1zgvDKu/Vk7f95tLyKjHNECmLy4ZODwU6XXvRk=; b=YDmeAkckTOGL17tMoBW9hr+1Kmhy7S86W8rup8IJpDYZ/BQw+L/hyAIdQw1cCGBTMs Doa9nK+cUC7VjPthovgtPLyTpTK9b95fqzPxqreT6qder5TR5D3/pQDNY7ZMeys5deOI HxJSiBu8TWpT/wIIje93bxvoyyH0tWE8EZYIhCHmSNFwB4CMTvZyUtaPMScrX1+BrhcW hyTb9TAiMlDDCEg2/6uytxiLW+jANQE2d/9X5bUwiSiIROTvGg+7iuOIQMZRA1Dgr3J/ HYUhql6pJBRtobymCLBX4Cggibq6akLrq6AvZRb3VjZyQmrR4Ys/h/v5qSCN7Wd+aM1d S/Mg== X-Gm-Message-State: APjAAAXikNk8DCIYQ+/P6FfR19gEbwq+6amrBTUUdhnM/1Eip50PO3fH N17Gqiq1JDYWBoGrQ9WfHo1S8SGo9lBpKZWdoXQbrQ== X-Google-Smtp-Source: APXvYqxEFT4t6ci312y+/g7e9vAaR90TUgHTNowFXe9gfh6DUIG6dMP98qv1vvSieZtNtI/KYxW53dyyPKyUy7ah+Nk= X-Received: by 2002:adf:fc4b:: with SMTP id e11mr3904527wrs.326.1578666956616; Fri, 10 Jan 2020 06:35:56 -0800 (PST) MIME-Version: 1.0 References: <20191206085432.19962-1-michael.kupfer@fau.de> <3db2350b-0a6d-0693-258c-9d47f71c0627@xs4all.nl> In-Reply-To: <3db2350b-0a6d-0693-258c-9d47f71c0627@xs4all.nl> From: Dave Stevenson Date: Fri, 10 Jan 2020 14:35:38 +0000 Message-ID: Subject: Re: [PATCH] staging/vc04_services/bcm2835-camera: distinct numeration and names for devices To: Hans Verkuil X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200110_063601_370679_9183B9B6 X-CRM114-Status: GOOD ( 23.22 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: devel@driverdev.osuosl.org, f.fainelli@gmail.com, sbranden@broadcom.com, Michael Kupfer , linux-kernel@i4.cs.fau.de, gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org, eric@anholt.net, daniela.mormocea@gmail.com, bcm-kernel-feedback-list@broadcom.com, wahrenst@gmx.net, Dave Stevenson , rjui@broadcom.com, mchehab+samsung@kernel.org, linux-media@vger.kernel.org, Kay Friedrich , linux-arm-kernel@lists.infradead.org, linux-rpi-kernel@lists.infradead.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Hans On Fri, 10 Jan 2020 at 13:25, Hans Verkuil wrote: > > Hi Michael, Kay, > > On 12/6/19 9:54 AM, Michael Kupfer wrote: > > Create a static atomic counter for numerating cameras. > > Use the Media Subsystem Kernel Internal API to create distinct > > device-names, so that the camera-number (given by the counter) > > matches the camera-name. > > > > Co-developed-by: Kay Friedrich > > Signed-off-by: Kay Friedrich > > Signed-off-by: Michael Kupfer > > --- > > .../vc04_services/bcm2835-camera/bcm2835-camera.c | 9 ++++++--- > > 1 file changed, 6 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c > > index beb6a0063bb8..be5f90a8b49d 100644 > > --- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c > > +++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c > > @@ -60,6 +60,9 @@ MODULE_PARM_DESC(max_video_width, "Threshold for video mode"); > > module_param(max_video_height, int, 0644); > > MODULE_PARM_DESC(max_video_height, "Threshold for video mode"); > > > > +/* camera instance counter */ > > +static atomic_t camera_instance = ATOMIC_INIT(0); > > + > > /* global device data array */ > > static struct bm2835_mmal_dev *gdev[MAX_BCM2835_CAMERAS]; > > > > @@ -1870,7 +1873,6 @@ static int bcm2835_mmal_probe(struct platform_device *pdev) > > > > /* v4l2 core mutex used to protect all fops and v4l2 ioctls. */ > > mutex_init(&dev->mutex); > > - dev->camera_num = camera; > > dev->max_width = resolutions[camera][0]; > > dev->max_height = resolutions[camera][1]; > > > > @@ -1886,8 +1888,9 @@ static int bcm2835_mmal_probe(struct platform_device *pdev) > > dev->capture.fmt = &formats[3]; /* JPEG */ > > > > /* v4l device registration */ > > - snprintf(dev->v4l2_dev.name, sizeof(dev->v4l2_dev.name), > > - "%s", BM2835_MMAL_MODULE_NAME); > > + dev->camera_num = v4l2_device_set_name(&dev->v4l2_dev, > > + BM2835_MMAL_MODULE_NAME, > > + &camera_instance); > > ret = v4l2_device_register(NULL, &dev->v4l2_dev); > > if (ret) { > > dev_err(&pdev->dev, "%s: could not register V4L2 device: %d\n", > > > > Actually, in this specific case I would not use v4l2_device_set_name(). > > Instead just use: > > snprintf(dev->v4l2_dev.name, sizeof(dev->v4l2_dev.name), > "%s-%u", BM2835_MMAL_MODULE_NAME, camera); > > It would be even better if there would be just one top-level v4l2_device used > for all the camera instances. After all, there really is just one platform > device for all of the cameras, and I would expect to see just a single > v4l2_device as well. > > It doesn't hurt to have multiple v4l2_device structs, but it introduces a > slight memory overhead since one would have been sufficient. Doesn't that make all controls for all cameras common? The struct v4l2_ctrl_handler is part of struct v4l2_device. Or do we: - ditch the use of ctrl_handler in struct v4l2_device - create and initialise a ctrl_handler per camera on an internal structure so we retain the control state - assign ctrl_handler in struct v4l2_fh to it every time a file handle on the device is opened? And if we only have one struct v4l2_device then is there the possibility of the unique names that Michael and Kay are trying to introduce? I'm a little confused as to whether there really is a gain in having a single v4l2_device. In this case the two cameras are independent devices, even if they are loaded by a single platform driver. Dave > v4l2_device_set_name() is meant for pci-like devices. And it really > is a bit overkill to have it as a helper function. > > Regards, > > Hans _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel