| Message ID | 20230923143200.268063-6-umang.jain@ideasonboard.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | staging: vc04_services: vchiq: Register devices with a custom bus_type | expand |
On Sat, Sep 23, 2023 at 08:01:59PM +0530, Umang Jain wrote: > Register the bcm2835-camera with the vchiq_bus_type instead of using > platform driver/device. > > Since we moved away bcm2835-camera from platform driver/device, > we have to set the DMA mask explicitly. Set the DMA mask at probe > time. > > Also the VCHIQ firmware doesn't support device enumeration, hence > one has to maintain a list of devices to be registered in the interface. > > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> > --- > .../bcm2835-camera/bcm2835-camera.c | 21 ++++++++++--------- > .../interface/vchiq_arm/vchiq_arm.c | 11 +++++++--- > 2 files changed, 19 insertions(+), 13 deletions(-) > > diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c > index fcad5118f3e8..c873eace1437 100644 > --- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c > +++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c > @@ -11,6 +11,7 @@ > * Luke Diamand @ Broadcom > */ > > +#include <linux/dma-mapping.h> > #include <linux/errno.h> > #include <linux/kernel.h> > #include <linux/module.h> > @@ -24,8 +25,8 @@ > #include <media/v4l2-event.h> > #include <media/v4l2-common.h> > #include <linux/delay.h> > -#include <linux/platform_device.h> > > +#include "../interface/vchiq_arm/vchiq_bus.h" > #include "../vchiq-mmal/mmal-common.h" > #include "../vchiq-mmal/mmal-encodings.h" > #include "../vchiq-mmal/mmal-vchiq.h" > @@ -1841,7 +1842,7 @@ static struct v4l2_format default_v4l2_format = { > .fmt.pix.sizeimage = 1024 * 768, > }; > > -static int bcm2835_mmal_probe(struct platform_device *pdev) > +static int bcm2835_mmal_probe(struct vchiq_device *device) > { > int ret; > struct bcm2835_mmal_dev *dev; > @@ -1852,9 +1853,9 @@ static int bcm2835_mmal_probe(struct platform_device *pdev) > unsigned int resolutions[MAX_BCM2835_CAMERAS][2]; > int i; > > - ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32)); > + ret = dma_set_mask_and_coherent(&device->dev, DMA_BIT_MASK(32)); > if (ret) { > - dev_err(&pdev->dev, "dma_set_mask_and_coherent failed: %d\n", ret); > + dev_err(&device->dev, "dma_set_mask_and_coherent failed: %d\n", ret); > return ret; > } > > @@ -1902,7 +1903,7 @@ static int bcm2835_mmal_probe(struct platform_device *pdev) > &camera_instance); > ret = v4l2_device_register(NULL, &dev->v4l2_dev); > if (ret) { > - dev_err(&pdev->dev, "%s: could not register V4L2 device: %d\n", > + dev_err(&device->dev, "%s: could not register V4L2 device: %d\n", > __func__, ret); > goto free_dev; > } > @@ -1982,7 +1983,7 @@ static int bcm2835_mmal_probe(struct platform_device *pdev) > return ret; > } > > -static void bcm2835_mmal_remove(struct platform_device *pdev) > +static void bcm2835_mmal_remove(struct vchiq_device *device) > { > int camera; > struct vchiq_mmal_instance *instance = gdev[0]->instance; > @@ -1994,17 +1995,17 @@ static void bcm2835_mmal_remove(struct platform_device *pdev) > vchiq_mmal_finalise(instance); > } > > -static struct platform_driver bcm2835_camera_driver = { > +static struct vchiq_driver bcm2835_camera_driver = { > .probe = bcm2835_mmal_probe, > - .remove_new = bcm2835_mmal_remove, > + .remove = bcm2835_mmal_remove, > .driver = { > .name = "bcm2835-camera", > }, > }; > > -module_platform_driver(bcm2835_camera_driver) > +module_vchiq_driver(bcm2835_camera_driver) > > MODULE_DESCRIPTION("Broadcom 2835 MMAL video capture"); > MODULE_AUTHOR("Vincent Sanders"); > MODULE_LICENSE("GPL"); > -MODULE_ALIAS("platform:bcm2835-camera"); > +MODULE_ALIAS("bcm2835-camera"); Now that you are on your own bus, why do you need the MODULE_ALIAS() line at all? thanks, greg k-h
Hi Greg, Sorry for late reply, as I was traveling and then was on vacation for rest of the week. On 10/5/23 1:34 PM, Greg Kroah-Hartman wrote: > On Sat, Sep 23, 2023 at 08:01:59PM +0530, Umang Jain wrote: >> Register the bcm2835-camera with the vchiq_bus_type instead of using >> platform driver/device. >> >> Since we moved away bcm2835-camera from platform driver/device, >> we have to set the DMA mask explicitly. Set the DMA mask at probe >> time. >> >> Also the VCHIQ firmware doesn't support device enumeration, hence >> one has to maintain a list of devices to be registered in the interface. >> >> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> >> --- >> .../bcm2835-camera/bcm2835-camera.c | 21 ++++++++++--------- >> .../interface/vchiq_arm/vchiq_arm.c | 11 +++++++--- >> 2 files changed, 19 insertions(+), 13 deletions(-) >> >> diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c >> index fcad5118f3e8..c873eace1437 100644 >> --- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c >> +++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c >> @@ -11,6 +11,7 @@ >> * Luke Diamand @ Broadcom >> */ >> >> +#include <linux/dma-mapping.h> >> #include <linux/errno.h> >> #include <linux/kernel.h> >> #include <linux/module.h> >> @@ -24,8 +25,8 @@ >> #include <media/v4l2-event.h> >> #include <media/v4l2-common.h> >> #include <linux/delay.h> >> -#include <linux/platform_device.h> >> >> +#include "../interface/vchiq_arm/vchiq_bus.h" >> #include "../vchiq-mmal/mmal-common.h" >> #include "../vchiq-mmal/mmal-encodings.h" >> #include "../vchiq-mmal/mmal-vchiq.h" >> @@ -1841,7 +1842,7 @@ static struct v4l2_format default_v4l2_format = { >> .fmt.pix.sizeimage = 1024 * 768, >> }; >> >> -static int bcm2835_mmal_probe(struct platform_device *pdev) >> +static int bcm2835_mmal_probe(struct vchiq_device *device) >> { >> int ret; >> struct bcm2835_mmal_dev *dev; >> @@ -1852,9 +1853,9 @@ static int bcm2835_mmal_probe(struct platform_device *pdev) >> unsigned int resolutions[MAX_BCM2835_CAMERAS][2]; >> int i; >> >> - ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32)); >> + ret = dma_set_mask_and_coherent(&device->dev, DMA_BIT_MASK(32)); >> if (ret) { >> - dev_err(&pdev->dev, "dma_set_mask_and_coherent failed: %d\n", ret); >> + dev_err(&device->dev, "dma_set_mask_and_coherent failed: %d\n", ret); >> return ret; >> } >> >> @@ -1902,7 +1903,7 @@ static int bcm2835_mmal_probe(struct platform_device *pdev) >> &camera_instance); >> ret = v4l2_device_register(NULL, &dev->v4l2_dev); >> if (ret) { >> - dev_err(&pdev->dev, "%s: could not register V4L2 device: %d\n", >> + dev_err(&device->dev, "%s: could not register V4L2 device: %d\n", >> __func__, ret); >> goto free_dev; >> } >> @@ -1982,7 +1983,7 @@ static int bcm2835_mmal_probe(struct platform_device *pdev) >> return ret; >> } >> >> -static void bcm2835_mmal_remove(struct platform_device *pdev) >> +static void bcm2835_mmal_remove(struct vchiq_device *device) >> { >> int camera; >> struct vchiq_mmal_instance *instance = gdev[0]->instance; >> @@ -1994,17 +1995,17 @@ static void bcm2835_mmal_remove(struct platform_device *pdev) >> vchiq_mmal_finalise(instance); >> } >> >> -static struct platform_driver bcm2835_camera_driver = { >> +static struct vchiq_driver bcm2835_camera_driver = { >> .probe = bcm2835_mmal_probe, >> - .remove_new = bcm2835_mmal_remove, >> + .remove = bcm2835_mmal_remove, >> .driver = { >> .name = "bcm2835-camera", >> }, >> }; >> >> -module_platform_driver(bcm2835_camera_driver) >> +module_vchiq_driver(bcm2835_camera_driver) >> >> MODULE_DESCRIPTION("Broadcom 2835 MMAL video capture"); >> MODULE_AUTHOR("Vincent Sanders"); >> MODULE_LICENSE("GPL"); >> -MODULE_ALIAS("platform:bcm2835-camera"); >> +MODULE_ALIAS("bcm2835-camera"); > Now that you are on your own bus, why do you need the MODULE_ALIAS() > line at all? Because it breaks the module auto-loading support... If you look at the vchiq_bus patch (3/6) in this series, there is vchiq_bus_uevent() which sends a event that includes the MODALIAS. > > thanks, > > greg k-h
On Mon, Oct 09, 2023 at 09:50:29AM +0530, Umang Jain wrote: > Hi Greg, > > Sorry for late reply, as I was traveling and then was on vacation for rest > of the week. > > On 10/5/23 1:34 PM, Greg Kroah-Hartman wrote: > > On Sat, Sep 23, 2023 at 08:01:59PM +0530, Umang Jain wrote: > > > Register the bcm2835-camera with the vchiq_bus_type instead of using > > > platform driver/device. > > > > > > Since we moved away bcm2835-camera from platform driver/device, > > > we have to set the DMA mask explicitly. Set the DMA mask at probe > > > time. > > > > > > Also the VCHIQ firmware doesn't support device enumeration, hence > > > one has to maintain a list of devices to be registered in the interface. > > > > > > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> > > > --- > > > .../bcm2835-camera/bcm2835-camera.c | 21 ++++++++++--------- > > > .../interface/vchiq_arm/vchiq_arm.c | 11 +++++++--- > > > 2 files changed, 19 insertions(+), 13 deletions(-) > > > > > > diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c > > > index fcad5118f3e8..c873eace1437 100644 > > > --- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c > > > +++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c > > > @@ -11,6 +11,7 @@ > > > * Luke Diamand @ Broadcom > > > */ > > > +#include <linux/dma-mapping.h> > > > #include <linux/errno.h> > > > #include <linux/kernel.h> > > > #include <linux/module.h> > > > @@ -24,8 +25,8 @@ > > > #include <media/v4l2-event.h> > > > #include <media/v4l2-common.h> > > > #include <linux/delay.h> > > > -#include <linux/platform_device.h> > > > +#include "../interface/vchiq_arm/vchiq_bus.h" > > > #include "../vchiq-mmal/mmal-common.h" > > > #include "../vchiq-mmal/mmal-encodings.h" > > > #include "../vchiq-mmal/mmal-vchiq.h" > > > @@ -1841,7 +1842,7 @@ static struct v4l2_format default_v4l2_format = { > > > .fmt.pix.sizeimage = 1024 * 768, > > > }; > > > -static int bcm2835_mmal_probe(struct platform_device *pdev) > > > +static int bcm2835_mmal_probe(struct vchiq_device *device) > > > { > > > int ret; > > > struct bcm2835_mmal_dev *dev; > > > @@ -1852,9 +1853,9 @@ static int bcm2835_mmal_probe(struct platform_device *pdev) > > > unsigned int resolutions[MAX_BCM2835_CAMERAS][2]; > > > int i; > > > - ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32)); > > > + ret = dma_set_mask_and_coherent(&device->dev, DMA_BIT_MASK(32)); > > > if (ret) { > > > - dev_err(&pdev->dev, "dma_set_mask_and_coherent failed: %d\n", ret); > > > + dev_err(&device->dev, "dma_set_mask_and_coherent failed: %d\n", ret); > > > return ret; > > > } > > > @@ -1902,7 +1903,7 @@ static int bcm2835_mmal_probe(struct platform_device *pdev) > > > &camera_instance); > > > ret = v4l2_device_register(NULL, &dev->v4l2_dev); > > > if (ret) { > > > - dev_err(&pdev->dev, "%s: could not register V4L2 device: %d\n", > > > + dev_err(&device->dev, "%s: could not register V4L2 device: %d\n", > > > __func__, ret); > > > goto free_dev; > > > } > > > @@ -1982,7 +1983,7 @@ static int bcm2835_mmal_probe(struct platform_device *pdev) > > > return ret; > > > } > > > -static void bcm2835_mmal_remove(struct platform_device *pdev) > > > +static void bcm2835_mmal_remove(struct vchiq_device *device) > > > { > > > int camera; > > > struct vchiq_mmal_instance *instance = gdev[0]->instance; > > > @@ -1994,17 +1995,17 @@ static void bcm2835_mmal_remove(struct platform_device *pdev) > > > vchiq_mmal_finalise(instance); > > > } > > > -static struct platform_driver bcm2835_camera_driver = { > > > +static struct vchiq_driver bcm2835_camera_driver = { > > > .probe = bcm2835_mmal_probe, > > > - .remove_new = bcm2835_mmal_remove, > > > + .remove = bcm2835_mmal_remove, > > > .driver = { > > > .name = "bcm2835-camera", > > > }, > > > }; > > > -module_platform_driver(bcm2835_camera_driver) > > > +module_vchiq_driver(bcm2835_camera_driver) > > > MODULE_DESCRIPTION("Broadcom 2835 MMAL video capture"); > > > MODULE_AUTHOR("Vincent Sanders"); > > > MODULE_LICENSE("GPL"); > > > -MODULE_ALIAS("platform:bcm2835-camera"); > > > +MODULE_ALIAS("bcm2835-camera"); > > Now that you are on your own bus, why do you need the MODULE_ALIAS() > > line at all? > > Because it breaks the module auto-loading support... But that's not how auto-loading should work once you have a real bus. > If you look at the vchiq_bus patch (3/6) in this series, there is > vchiq_bus_uevent() which sends a event that includes the MODALIAS. The module alias should be automatically picked out of the MODULE_DEVICE_TABLE() macro that you should be using for this driver, you should never have to manually create MODULE_ALIAS() lines. That's a bad hold-over because the platform driver subsystem can not handle module aliases on it's own, let's not copy that bad pattern here as well please. thanks, greg k-h
diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c index fcad5118f3e8..c873eace1437 100644 --- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c +++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c @@ -11,6 +11,7 @@ * Luke Diamand @ Broadcom */ +#include <linux/dma-mapping.h> #include <linux/errno.h> #include <linux/kernel.h> #include <linux/module.h> @@ -24,8 +25,8 @@ #include <media/v4l2-event.h> #include <media/v4l2-common.h> #include <linux/delay.h> -#include <linux/platform_device.h> +#include "../interface/vchiq_arm/vchiq_bus.h" #include "../vchiq-mmal/mmal-common.h" #include "../vchiq-mmal/mmal-encodings.h" #include "../vchiq-mmal/mmal-vchiq.h" @@ -1841,7 +1842,7 @@ static struct v4l2_format default_v4l2_format = { .fmt.pix.sizeimage = 1024 * 768, }; -static int bcm2835_mmal_probe(struct platform_device *pdev) +static int bcm2835_mmal_probe(struct vchiq_device *device) { int ret; struct bcm2835_mmal_dev *dev; @@ -1852,9 +1853,9 @@ static int bcm2835_mmal_probe(struct platform_device *pdev) unsigned int resolutions[MAX_BCM2835_CAMERAS][2]; int i; - ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32)); + ret = dma_set_mask_and_coherent(&device->dev, DMA_BIT_MASK(32)); if (ret) { - dev_err(&pdev->dev, "dma_set_mask_and_coherent failed: %d\n", ret); + dev_err(&device->dev, "dma_set_mask_and_coherent failed: %d\n", ret); return ret; } @@ -1902,7 +1903,7 @@ static int bcm2835_mmal_probe(struct platform_device *pdev) &camera_instance); ret = v4l2_device_register(NULL, &dev->v4l2_dev); if (ret) { - dev_err(&pdev->dev, "%s: could not register V4L2 device: %d\n", + dev_err(&device->dev, "%s: could not register V4L2 device: %d\n", __func__, ret); goto free_dev; } @@ -1982,7 +1983,7 @@ static int bcm2835_mmal_probe(struct platform_device *pdev) return ret; } -static void bcm2835_mmal_remove(struct platform_device *pdev) +static void bcm2835_mmal_remove(struct vchiq_device *device) { int camera; struct vchiq_mmal_instance *instance = gdev[0]->instance; @@ -1994,17 +1995,17 @@ static void bcm2835_mmal_remove(struct platform_device *pdev) vchiq_mmal_finalise(instance); } -static struct platform_driver bcm2835_camera_driver = { +static struct vchiq_driver bcm2835_camera_driver = { .probe = bcm2835_mmal_probe, - .remove_new = bcm2835_mmal_remove, + .remove = bcm2835_mmal_remove, .driver = { .name = "bcm2835-camera", }, }; -module_platform_driver(bcm2835_camera_driver) +module_vchiq_driver(bcm2835_camera_driver) MODULE_DESCRIPTION("Broadcom 2835 MMAL video capture"); MODULE_AUTHOR("Vincent Sanders"); MODULE_LICENSE("GPL"); -MODULE_ALIAS("platform:bcm2835-camera"); +MODULE_ALIAS("bcm2835-camera"); diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c index 9388859b9b56..886025f0a452 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c @@ -67,8 +67,13 @@ int vchiq_susp_log_level = VCHIQ_LOG_ERROR; DEFINE_SPINLOCK(msg_queue_spinlock); struct vchiq_state g_state; -static struct platform_device *bcm2835_camera; static struct platform_device *bcm2835_audio; +/* + * The devices implemented in the VCHIQ firmware are not discoverable, + * so we need to maintain a list of them in order to register them with + * the interface. + */ +static struct vchiq_device *bcm2835_camera; struct vchiq_drvdata { const unsigned int cache_line_size; @@ -1840,8 +1845,8 @@ static int vchiq_probe(struct platform_device *pdev) goto error_exit; } - bcm2835_camera = vchiq_register_child(pdev, "bcm2835-camera"); bcm2835_audio = vchiq_register_child(pdev, "bcm2835_audio"); + bcm2835_camera = vchiq_device_register(&pdev->dev, "bcm2835-camera"); return 0; @@ -1854,7 +1859,7 @@ static int vchiq_probe(struct platform_device *pdev) static void vchiq_remove(struct platform_device *pdev) { platform_device_unregister(bcm2835_audio); - platform_device_unregister(bcm2835_camera); + vchiq_device_unregister(bcm2835_camera); vchiq_debugfs_deinit(); vchiq_deregister_chrdev(); }
Register the bcm2835-camera with the vchiq_bus_type instead of using platform driver/device. Since we moved away bcm2835-camera from platform driver/device, we have to set the DMA mask explicitly. Set the DMA mask at probe time. Also the VCHIQ firmware doesn't support device enumeration, hence one has to maintain a list of devices to be registered in the interface. Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> --- .../bcm2835-camera/bcm2835-camera.c | 21 ++++++++++--------- .../interface/vchiq_arm/vchiq_arm.c | 11 +++++++--- 2 files changed, 19 insertions(+), 13 deletions(-)