* [PATCH] staging/vc04_services/bcm2835-camera: distinct numeration and names for devices @ 2019-12-04 11:48 Michael Kupfer 2019-12-04 19:37 ` Stefan Wahren 2019-12-04 19:40 ` Nicolas Saenz Julienne 0 siblings, 2 replies; 15+ messages in thread From: Michael Kupfer @ 2019-12-04 11:48 UTC (permalink / raw) To: eric, wahrenst, bcm-kernel-feedback-list, linux-rpi-kernel, linux-arm-kernel Cc: Kay Friedrich, Michael Kupfer, linux-kernel 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 <kay.friedrich@fau.de> Signed-off-by: Kay Friedrich <kay.friedrich@fau.de> Signed-off-by: Michael Kupfer <michael.kupfer@fau.de> --- .../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", -- 2.20.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH] staging/vc04_services/bcm2835-camera: distinct numeration and names for devices 2019-12-04 11:48 [PATCH] staging/vc04_services/bcm2835-camera: distinct numeration and names for devices Michael Kupfer @ 2019-12-04 19:37 ` Stefan Wahren 2019-12-04 19:40 ` Nicolas Saenz Julienne 1 sibling, 0 replies; 15+ messages in thread From: Stefan Wahren @ 2019-12-04 19:37 UTC (permalink / raw) To: Michael Kupfer, eric, bcm-kernel-feedback-list, linux-rpi-kernel, linux-arm-kernel Cc: Kay Friedrich, linux-kernel Hi Michael, Am 04.12.19 um 12:48 schrieb Michael Kupfer: > 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 <kay.friedrich@fau.de> > Signed-off-by: Kay Friedrich <kay.friedrich@fau.de> > Signed-off-by: Michael Kupfer <michael.kupfer@fau.de> this is a functional change. Please send this patch to Nicolas Saenz Julienne the new BCM2835 maintainer and the Video4Linux guys (Mauro and Hans) and the linux media mailing list. Regards Stefan _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] staging/vc04_services/bcm2835-camera: distinct numeration and names for devices 2019-12-04 11:48 [PATCH] staging/vc04_services/bcm2835-camera: distinct numeration and names for devices Michael Kupfer 2019-12-04 19:37 ` Stefan Wahren @ 2019-12-04 19:40 ` Nicolas Saenz Julienne 1 sibling, 0 replies; 15+ messages in thread From: Nicolas Saenz Julienne @ 2019-12-04 19:40 UTC (permalink / raw) To: Michael Kupfer, eric, wahrenst, bcm-kernel-feedback-list, linux-rpi-kernel, linux-arm-kernel Cc: Kay Friedrich, linux-kernel [-- Attachment #1.1: Type: text/plain, Size: 610 bytes --] Hi, On Wed, 2019-12-04 at 12:48 +0100, 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 <kay.friedrich@fau.de> > Signed-off-by: Kay Friedrich <kay.friedrich@fau.de> > Signed-off-by: Michael Kupfer <michael.kupfer@fau.de> > --- The patch itself seems fine, but could you resend it making sure you CC everyone listed by ./scripts/get_maintainer.sh. Regards, Nicolas [-- Attachment #1.2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 488 bytes --] [-- Attachment #2: Type: text/plain, Size: 176 bytes --] _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH] staging/vc04_services/bcm2835-camera: distinct numeration and names for devices @ 2019-12-06 8:54 ` Michael Kupfer 0 siblings, 0 replies; 15+ messages in thread From: Michael Kupfer @ 2019-12-06 8:54 UTC (permalink / raw) To: eric, wahrenst, bcm-kernel-feedback-list, linux-rpi-kernel, linux-arm-kernel, mchehab+samsung, hverkuil-cisco, linux-media Cc: gregkh, f.fainelli, rjui, sbranden, dave.stevenson, daniela.mormocea, devel, linux-kernel, linux-kernel, Michael Kupfer, Kay Friedrich 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 <kay.friedrich@fau.de> Signed-off-by: Kay Friedrich <kay.friedrich@fau.de> Signed-off-by: Michael Kupfer <michael.kupfer@fau.de> --- .../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", -- 2.20.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH] staging/vc04_services/bcm2835-camera: distinct numeration and names for devices @ 2019-12-06 8:54 ` Michael Kupfer 0 siblings, 0 replies; 15+ messages in thread From: Michael Kupfer @ 2019-12-06 8:54 UTC (permalink / raw) To: eric, wahrenst, bcm-kernel-feedback-list, linux-rpi-kernel, linux-arm-kernel, mchehab+samsung, hverkuil-cisco, linux-media Cc: devel, f.fainelli, Michael Kupfer, sbranden, linux-kernel, gregkh, linux-kernel, daniela.mormocea, dave.stevenson, rjui, Kay Friedrich 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 <kay.friedrich@fau.de> Signed-off-by: Kay Friedrich <kay.friedrich@fau.de> Signed-off-by: Michael Kupfer <michael.kupfer@fau.de> --- .../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", -- 2.20.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH] staging/vc04_services/bcm2835-camera: distinct numeration and names for devices @ 2019-12-06 8:54 ` Michael Kupfer 0 siblings, 0 replies; 15+ messages in thread From: Michael Kupfer @ 2019-12-06 8:54 UTC (permalink / raw) To: eric, wahrenst, bcm-kernel-feedback-list, linux-rpi-kernel, linux-arm-kernel, mchehab+samsung, hverkuil-cisco, linux-media Cc: devel, f.fainelli, Michael Kupfer, sbranden, linux-kernel, gregkh, linux-kernel, daniela.mormocea, dave.stevenson, rjui, Kay Friedrich 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 <kay.friedrich@fau.de> Signed-off-by: Kay Friedrich <kay.friedrich@fau.de> Signed-off-by: Michael Kupfer <michael.kupfer@fau.de> --- .../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", -- 2.20.1 _______________________________________________ devel mailing list devel@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH] staging/vc04_services/bcm2835-camera: distinct numeration and names for devices 2019-12-06 8:54 ` Michael Kupfer (?) @ 2020-01-10 13:25 ` Hans Verkuil -1 siblings, 0 replies; 15+ messages in thread From: Hans Verkuil @ 2020-01-10 13:25 UTC (permalink / raw) To: Michael Kupfer, eric, wahrenst, bcm-kernel-feedback-list, linux-rpi-kernel, linux-arm-kernel, mchehab+samsung, linux-media Cc: gregkh, f.fainelli, rjui, sbranden, dave.stevenson, daniela.mormocea, devel, linux-kernel, linux-kernel, Kay Friedrich 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 <kay.friedrich@fau.de> > Signed-off-by: Kay Friedrich <kay.friedrich@fau.de> > Signed-off-by: Michael Kupfer <michael.kupfer@fau.de> > --- > .../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. 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] staging/vc04_services/bcm2835-camera: distinct numeration and names for devices @ 2020-01-10 13:25 ` Hans Verkuil 0 siblings, 0 replies; 15+ messages in thread From: Hans Verkuil @ 2020-01-10 13:25 UTC (permalink / raw) To: Michael Kupfer, eric, wahrenst, bcm-kernel-feedback-list, linux-rpi-kernel, linux-arm-kernel, mchehab+samsung, linux-media Cc: devel, f.fainelli, Kay Friedrich, sbranden, linux-kernel, gregkh, linux-kernel, daniela.mormocea, dave.stevenson, rjui 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 <kay.friedrich@fau.de> > Signed-off-by: Kay Friedrich <kay.friedrich@fau.de> > Signed-off-by: Michael Kupfer <michael.kupfer@fau.de> > --- > .../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. 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] staging/vc04_services/bcm2835-camera: distinct numeration and names for devices @ 2020-01-10 13:25 ` Hans Verkuil 0 siblings, 0 replies; 15+ messages in thread From: Hans Verkuil @ 2020-01-10 13:25 UTC (permalink / raw) To: Michael Kupfer, eric, wahrenst, bcm-kernel-feedback-list, linux-rpi-kernel, linux-arm-kernel, mchehab+samsung, linux-media Cc: devel, f.fainelli, Kay Friedrich, sbranden, linux-kernel, gregkh, linux-kernel, daniela.mormocea, dave.stevenson, rjui 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 <kay.friedrich@fau.de> > Signed-off-by: Kay Friedrich <kay.friedrich@fau.de> > Signed-off-by: Michael Kupfer <michael.kupfer@fau.de> > --- > .../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. 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] staging/vc04_services/bcm2835-camera: distinct numeration and names for devices 2020-01-10 13:25 ` Hans Verkuil (?) @ 2020-01-10 14:35 ` Dave Stevenson -1 siblings, 0 replies; 15+ messages in thread From: Dave Stevenson @ 2020-01-10 14:35 UTC (permalink / raw) To: Hans Verkuil Cc: Michael Kupfer, eric, wahrenst, bcm-kernel-feedback-list, linux-rpi-kernel, linux-arm-kernel, mchehab+samsung, linux-media, gregkh, f.fainelli, rjui, sbranden, Dave Stevenson, daniela.mormocea, devel, linux-kernel, linux-kernel, Kay Friedrich Hi Hans On Fri, 10 Jan 2020 at 13:25, Hans Verkuil <hverkuil-cisco@xs4all.nl> 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 <kay.friedrich@fau.de> > > Signed-off-by: Kay Friedrich <kay.friedrich@fau.de> > > Signed-off-by: Michael Kupfer <michael.kupfer@fau.de> > > --- > > .../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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] staging/vc04_services/bcm2835-camera: distinct numeration and names for devices @ 2020-01-10 14:35 ` Dave Stevenson 0 siblings, 0 replies; 15+ messages in thread From: Dave Stevenson @ 2020-01-10 14:35 UTC (permalink / raw) To: Hans Verkuil Cc: devel, f.fainelli, sbranden, Michael Kupfer, linux-kernel, gregkh, linux-kernel, eric, daniela.mormocea, bcm-kernel-feedback-list, wahrenst, Dave Stevenson, rjui, mchehab+samsung, linux-media, Kay Friedrich, linux-arm-kernel, linux-rpi-kernel Hi Hans On Fri, 10 Jan 2020 at 13:25, Hans Verkuil <hverkuil-cisco@xs4all.nl> 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 <kay.friedrich@fau.de> > > Signed-off-by: Kay Friedrich <kay.friedrich@fau.de> > > Signed-off-by: Michael Kupfer <michael.kupfer@fau.de> > > --- > > .../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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] staging/vc04_services/bcm2835-camera: distinct numeration and names for devices @ 2020-01-10 14:35 ` Dave Stevenson 0 siblings, 0 replies; 15+ messages in thread From: Dave Stevenson @ 2020-01-10 14:35 UTC (permalink / raw) To: Hans Verkuil Cc: devel, f.fainelli, sbranden, Michael Kupfer, linux-kernel, gregkh, linux-kernel, eric, daniela.mormocea, bcm-kernel-feedback-list, wahrenst, Dave Stevenson, rjui, mchehab+samsung, linux-media, Kay Friedrich, linux-arm-kernel, linux-rpi-kernel Hi Hans On Fri, 10 Jan 2020 at 13:25, Hans Verkuil <hverkuil-cisco@xs4all.nl> 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 <kay.friedrich@fau.de> > > Signed-off-by: Kay Friedrich <kay.friedrich@fau.de> > > Signed-off-by: Michael Kupfer <michael.kupfer@fau.de> > > --- > > .../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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] staging/vc04_services/bcm2835-camera: distinct numeration and names for devices 2020-01-10 14:35 ` Dave Stevenson (?) @ 2020-01-10 14:49 ` Hans Verkuil -1 siblings, 0 replies; 15+ messages in thread From: Hans Verkuil @ 2020-01-10 14:49 UTC (permalink / raw) To: Dave Stevenson Cc: Michael Kupfer, eric, wahrenst, bcm-kernel-feedback-list, linux-rpi-kernel, linux-arm-kernel, mchehab+samsung, linux-media, gregkh, f.fainelli, rjui, sbranden, Dave Stevenson, daniela.mormocea, devel, linux-kernel, linux-kernel, Kay Friedrich On 1/10/20 3:35 PM, Dave Stevenson wrote: > Hi Hans > > On Fri, 10 Jan 2020 at 13:25, Hans Verkuil <hverkuil-cisco@xs4all.nl> 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 <kay.friedrich@fau.de> >>> Signed-off-by: Kay Friedrich <kay.friedrich@fau.de> >>> Signed-off-by: Michael Kupfer <michael.kupfer@fau.de> >>> --- >>> .../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? You can set ctrl_handlers at three levels: v4l2_device (common to all video devices), video_device (specific to a single video device), or for v4l2_fh (specific to that filehandle only). In this case you would assign it to the video_device: dev->vdev.ctrl_handler = &dev->ctrl_handler; > > 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. There probably is little to no gain. Hence my proposal to just stick the camera index after the module name. Regards, Hans > > 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] staging/vc04_services/bcm2835-camera: distinct numeration and names for devices @ 2020-01-10 14:49 ` Hans Verkuil 0 siblings, 0 replies; 15+ messages in thread From: Hans Verkuil @ 2020-01-10 14:49 UTC (permalink / raw) To: Dave Stevenson Cc: devel, f.fainelli, sbranden, Michael Kupfer, linux-kernel, gregkh, linux-kernel, eric, daniela.mormocea, bcm-kernel-feedback-list, wahrenst, Dave Stevenson, rjui, mchehab+samsung, linux-media, Kay Friedrich, linux-arm-kernel, linux-rpi-kernel On 1/10/20 3:35 PM, Dave Stevenson wrote: > Hi Hans > > On Fri, 10 Jan 2020 at 13:25, Hans Verkuil <hverkuil-cisco@xs4all.nl> 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 <kay.friedrich@fau.de> >>> Signed-off-by: Kay Friedrich <kay.friedrich@fau.de> >>> Signed-off-by: Michael Kupfer <michael.kupfer@fau.de> >>> --- >>> .../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? You can set ctrl_handlers at three levels: v4l2_device (common to all video devices), video_device (specific to a single video device), or for v4l2_fh (specific to that filehandle only). In this case you would assign it to the video_device: dev->vdev.ctrl_handler = &dev->ctrl_handler; > > 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. There probably is little to no gain. Hence my proposal to just stick the camera index after the module name. Regards, Hans > > 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] staging/vc04_services/bcm2835-camera: distinct numeration and names for devices @ 2020-01-10 14:49 ` Hans Verkuil 0 siblings, 0 replies; 15+ messages in thread From: Hans Verkuil @ 2020-01-10 14:49 UTC (permalink / raw) To: Dave Stevenson Cc: devel, f.fainelli, sbranden, Michael Kupfer, linux-kernel, gregkh, linux-kernel, eric, daniela.mormocea, bcm-kernel-feedback-list, wahrenst, Dave Stevenson, rjui, mchehab+samsung, linux-media, Kay Friedrich, linux-arm-kernel, linux-rpi-kernel On 1/10/20 3:35 PM, Dave Stevenson wrote: > Hi Hans > > On Fri, 10 Jan 2020 at 13:25, Hans Verkuil <hverkuil-cisco@xs4all.nl> 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 <kay.friedrich@fau.de> >>> Signed-off-by: Kay Friedrich <kay.friedrich@fau.de> >>> Signed-off-by: Michael Kupfer <michael.kupfer@fau.de> >>> --- >>> .../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? You can set ctrl_handlers at three levels: v4l2_device (common to all video devices), video_device (specific to a single video device), or for v4l2_fh (specific to that filehandle only). In this case you would assign it to the video_device: dev->vdev.ctrl_handler = &dev->ctrl_handler; > > 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. There probably is little to no gain. Hence my proposal to just stick the camera index after the module name. Regards, Hans > > 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2020-01-10 14:57 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-12-04 11:48 [PATCH] staging/vc04_services/bcm2835-camera: distinct numeration and names for devices Michael Kupfer 2019-12-04 19:37 ` Stefan Wahren 2019-12-04 19:40 ` Nicolas Saenz Julienne 2019-12-06 8:54 Michael Kupfer 2019-12-06 8:54 ` Michael Kupfer 2019-12-06 8:54 ` Michael Kupfer 2020-01-10 13:25 ` Hans Verkuil 2020-01-10 13:25 ` Hans Verkuil 2020-01-10 13:25 ` Hans Verkuil 2020-01-10 14:35 ` Dave Stevenson 2020-01-10 14:35 ` Dave Stevenson 2020-01-10 14:35 ` Dave Stevenson 2020-01-10 14:49 ` Hans Verkuil 2020-01-10 14:49 ` Hans Verkuil 2020-01-10 14:49 ` Hans Verkuil
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.