Re: [PATCH v8 3/5] staging: bcm2835-camera: Register bcm2835-camera with vchiq_bus_type

Greg KH
2023-07-03T14:47:11Z

On Mon, Jul 03, 2023 at 04:44:39PM +0200, Umang Jain wrote:
> Hi Greg,
>
> On 7/3/23 3:29 PM, Greg KH wrote:
> > On Tue, Jun 27, 2023 at 10:16:26PM +0200, Umang Jain wrote:
> > > Register the bcm2835-camera with the vchiq_bus_type instead of using
> > > platform driver/device.
> > >
> > > 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 | 16 +++++++-------
> > > .../interface/vchiq_arm/vchiq_arm.c | 21 ++++++++++++++++---
> > > 2 files changed, 26 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
> > > index 346d00df815a..f37b2a881d92 100644
> > > --- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
> > > +++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
> > > @@ -24,8 +24,9 @@
> > > #include <media/v4l2-event.h>
> > > #include <media/v4l2-common.h>
> > > #include <linux/delay.h>
> > > -#include <linux/platform_device.h>
> > > +#include "../interface/vchiq_arm/vchiq_arm.h"
> > > +#include "../interface/vchiq_arm/vchiq_device.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;
> > > @@ -1896,7 +1897,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;
> > > }
> > > @@ -1976,7 +1977,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;
> > > @@ -1988,17 +1989,16 @@ 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,
> > No need to change this here, right? That's independant of this patch
> > series.
>
> Why not ?
>
> Should I have "remove_new()"  in the struct vchiq_driver {..} [Patch 1/5]
> instead of "remove()"  -  match up with platform_driver virtual interface ?

Ah, sorry, my fault, I thought this was just a platform driver change.
This is fine.

greg k-h