Hi Tuukka,
On Wed, May 03, 2017 at 02:06:23PM +0300, Tuukka Toivonen wrote:
> Hi Sakari,
>
> On Wednesday, May 03, 2017 11:58:01 Sakari Ailus wrote:
> > Hi Yong,
> >
> > A few more minor comments below...
> >
> > On Sat, Apr 29, 2017 at 06:34:36PM -0500, Yong Zhi wrote:
> > ...
> > > +/******* V4L2 sub-device asynchronous registration callbacks***********/
> > > +
> > > +static struct cio2_queue *cio2_find_queue_by_sensor_node(struct cio2_queue *q,
> > > + struct fwnode_handle *fwnode)
> > > +{
> > > + int i;
> > > +
> > > + for (i = 0; i < CIO2_QUEUES; i++) {
> > > + if (q[i].sensor->fwnode == fwnode)
> > > + return &q[i];
> > > + }
> > > +
> > > + return NULL;
> > > +}
> > > +
> > > +/* The .bound() notifier callback when a match is found */
> > > +static int cio2_notifier_bound(struct v4l2_async_notifier *notifier,
> > > + struct v4l2_subdev *sd,
> > > + struct v4l2_async_subdev *asd)
> > > +{
> > > + struct cio2_device *cio2 = container_of(notifier,
> > > + struct cio2_device, notifier);
> > > + struct sensor_async_subdev *s_asd = container_of(asd,
> > > + struct sensor_async_subdev, asd);
> > > + struct cio2_queue *q;
> > > + struct device *dev;
> > > + int i;
> > > +
> > > + dev = &cio2->pci_dev->dev;
> > > +
> > > + /* Find first free slot for the subdev */
> > > + for (i = 0; i < CIO2_QUEUES; i++)
> > > + if (!cio2->queue[i].sensor)
> > > + break;
> > > +
> > > + if (i >= CIO2_QUEUES) {
> > > + dev_err(dev, "too many subdevs\n");
> > > + return -ENOSPC;
> > > + }
> > > + q = &cio2->queue[i];
> > > +
> > > + q->csi2.port = s_asd->vfwn_endpt.base.port;
> > > + q->csi2.num_of_lanes = s_asd->vfwn_endpt.bus.mipi_csi2.num_data_lanes;
> > > + q->sensor = sd;
> > > + q->csi_rx_base = cio2->base + CIO2_REG_PIPE_BASE(q->csi2.port);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +/* The .unbind callback */
> > > +static void cio2_notifier_unbind(struct v4l2_async_notifier *notifier,
> > > + struct v4l2_subdev *sd,
> > > + struct v4l2_async_subdev *asd)
> > > +{
> > > + struct cio2_device *cio2 = container_of(notifier,
> > > + struct cio2_device, notifier);
> > > + unsigned int i;
> > > +
> > > + /* Note: sd may here point to unallocated memory. Do not access. */
> >
> > When can this happen?
>
> If v4l2_device_register_subdev() fails, it may lead to
> calling subdevice's remove function and the devres system
> freeing the subdevice before unbind is called. This did
> happen during driver development.
Ouch. Indeed, we do have object lifetime issues. :-( They will obviously
need to be fixed.
--
Kind regards,
Sakari Ailus
e-mail: sakari.ailus@xxxxxx XMPP: sailus@xxxxxxxxxxxxxx
