Re: [RESEND PATCH v3 14/32] media: v4l: async: Rename v4l2_async_subdev as v4l2_async_connection

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Laurent,

Thank you for the review, again.

On Tue, May 30, 2023 at 08:50:03AM +0300, Laurent Pinchart wrote:
> Hi Sakari,
> 
> Thank you for the patch.
> 
> On Thu, May 25, 2023 at 12:15:57PM +0300, Sakari Ailus wrote:
> > Rename v4l2_async_subdev as v4l2_async_connection, in order to
> > differentiate between the sub-devices and their connections: one
> > sub-device can have many connections but the V4L2 async framework has so
> > far allowed just a single one. Connections in this context will later
> > translate into either MC ancillary or data links.
> > 
> > This patch prepares changing that relation by changing existing users of
> > v4l2_async_subdev to switch to v4l2_async_connection. Async sub-devices
> > themselves will not be needed anymore
> > 
> > Additionally, __v4l2_async_nf_add_subdev() has been renamed as
> 
> s/renamed as/renamed to/
> 
> > __v4l2_async_nf_add_connection().

Actually I think there should be no preposition at all here.

> 
> I still don't like the name "connection" at all :-( It may seem fine as
> you've been working on this extensively, but for people less familiar
> with v4l2-async (myself included), I fear it will make the framework
> more difficult to understand.
> 
> At the very least I'd like a detailed and clear glossary, to explain
> what a connection *is*. The v4l2-async documentation is fairly bad (and
> what is currently documented in v4l2-subdev.rst should likely be moved
> to v4l2-async.rst).

That's a fair point. I'll add documentation on this.

What I do like with this is that there's now a clear separation between the
placeholder registered with the notifier and the sub-device itself. That
used to be sometimes a bit confusing.

> 
> > Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
> > ---
> >  .../driver-api/media/v4l2-subdev.rst          |  12 +-
> >  drivers/media/i2c/max9286.c                   |   9 +-
> >  drivers/media/i2c/st-mipid02.c                |   8 +-
> >  drivers/media/i2c/tc358746.c                  |   6 +-
> >  drivers/media/pci/intel/ipu3/ipu3-cio2-main.c |  10 +-
> >  drivers/media/platform/atmel/atmel-isi.c      |   8 +-
> >  drivers/media/platform/atmel/atmel-isi.h      |   2 +-
> >  drivers/media/platform/cadence/cdns-csi2rx.c  |   6 +-
> >  drivers/media/platform/intel/pxa_camera.c     |  12 +-
> >  drivers/media/platform/marvell/cafe-driver.c  |   5 +-
> >  drivers/media/platform/marvell/mcam-core.c    |   4 +-
> >  drivers/media/platform/marvell/mmp-driver.c   |   4 +-
> >  .../platform/microchip/microchip-csi2dc.c     |   6 +-
> >  .../platform/microchip/microchip-isc-base.c   |   4 +-
> >  .../media/platform/microchip/microchip-isc.h  |   2 +-
> >  .../microchip/microchip-sama5d2-isc.c         |   4 +-
> >  .../microchip/microchip-sama7g5-isc.c         |   4 +-
> >  drivers/media/platform/nxp/imx-mipi-csis.c    |   6 +-
> >  drivers/media/platform/nxp/imx7-media-csi.c   |   6 +-
> >  .../platform/nxp/imx8-isi/imx8-isi-core.c     |   8 +-
> >  drivers/media/platform/qcom/camss/camss.c     |   2 +-
> >  drivers/media/platform/qcom/camss/camss.h     |   2 +-
> >  drivers/media/platform/renesas/rcar-isp.c     |   8 +-
> >  .../platform/renesas/rcar-vin/rcar-core.c     |  44 ++---
> >  .../platform/renesas/rcar-vin/rcar-csi2.c     |  16 +-
> >  .../platform/renesas/rcar-vin/rcar-vin.h      |   8 +-
> >  drivers/media/platform/renesas/rcar_drif.c    |   8 +-
> >  drivers/media/platform/renesas/renesas-ceu.c  |   6 +-
> >  .../platform/renesas/rzg2l-cru/rzg2l-core.c   |  10 +-
> >  .../platform/renesas/rzg2l-cru/rzg2l-cru.h    |   2 +-
> >  .../platform/renesas/rzg2l-cru/rzg2l-csi2.c   |   8 +-
> >  .../platform/rockchip/rkisp1/rkisp1-common.h  |   2 +-
> >  .../platform/rockchip/rkisp1/rkisp1-dev.c     |   8 +-
> >  .../platform/samsung/exynos4-is/media-dev.c   |   6 +-
> >  .../platform/samsung/exynos4-is/media-dev.h   |   2 +-
> >  drivers/media/platform/st/stm32/stm32-dcmi.c  |   8 +-
> >  .../platform/sunxi/sun4i-csi/sun4i_csi.c      |   6 +-
> >  .../sunxi/sun6i-csi/sun6i_csi_bridge.c        |   2 +-
> >  .../sunxi/sun6i-csi/sun6i_csi_bridge.h        |   2 +-
> >  .../sunxi/sun6i-mipi-csi2/sun6i_mipi_csi2.c   |   6 +-
> >  .../sun8i_a83t_mipi_csi2.c                    |   6 +-
> >  .../media/platform/ti/am437x/am437x-vpfe.c    |   5 +-
> >  .../media/platform/ti/am437x/am437x-vpfe.h    |   2 +-
> >  drivers/media/platform/ti/cal/cal.c           |   6 +-
> >  .../media/platform/ti/davinci/vpif_capture.c  |   7 +-
> >  drivers/media/platform/ti/omap3isp/isp.h      |   2 +-
> >  drivers/media/platform/video-mux.c            |   6 +-
> >  drivers/media/platform/xilinx/xilinx-vipp.c   |  22 +--
> >  drivers/media/v4l2-core/v4l2-async.c          | 159 +++++++++---------
> >  drivers/media/v4l2-core/v4l2-fwnode.c         |   8 +-
> >  .../media/deprecated/atmel/atmel-isc-base.c   |   4 +-
> >  .../media/deprecated/atmel/atmel-isc.h        |   2 +-
> >  .../deprecated/atmel/atmel-sama5d2-isc.c      |   4 +-
> >  .../deprecated/atmel/atmel-sama7g5-isc.c      |   4 +-
> >  drivers/staging/media/imx/imx-media-csi.c     |   6 +-
> >  .../staging/media/imx/imx-media-dev-common.c  |   2 +-
> >  drivers/staging/media/imx/imx-media-dev.c     |   2 +-
> >  drivers/staging/media/imx/imx-media-of.c      |   4 +-
> >  drivers/staging/media/imx/imx6-mipi-csi2.c    |   8 +-
> >  drivers/staging/media/imx/imx8mq-mipi-csi2.c  |   6 +-
> >  .../media/sunxi/sun6i-isp/sun6i_isp_proc.c    |   2 +-
> >  .../media/sunxi/sun6i-isp/sun6i_isp_proc.h    |   2 +-
> >  drivers/staging/media/tegra-video/vi.c        |  14 +-
> >  drivers/staging/media/tegra-video/vi.h        |   2 +-
> >  include/media/davinci/vpif_types.h            |   2 +-
> >  include/media/v4l2-async.h                    |  66 ++++----
> >  include/media/v4l2-fwnode.h                   |   2 +-
> >  include/media/v4l2-subdev.h                   |   5 +-
> >  68 files changed, 320 insertions(+), 322 deletions(-)
> > 
> > diff --git a/Documentation/driver-api/media/v4l2-subdev.rst b/Documentation/driver-api/media/v4l2-subdev.rst
> > index ce8e9d0a332bc..83d3d29608136 100644
> > --- a/Documentation/driver-api/media/v4l2-subdev.rst
> > +++ b/Documentation/driver-api/media/v4l2-subdev.rst
> > @@ -214,14 +214,14 @@ notifier and further registers async sub-devices for lens and flash devices
> >  found in firmware. The notifier for the sub-device is unregistered with the
> >  async sub-device.
> >  
> > -These functions allocate an async sub-device descriptor which is of type struct
> > -:c:type:`v4l2_async_subdev` embedded in a driver-specific struct. The &struct
> > -:c:type:`v4l2_async_subdev` shall be the first member of this struct:
> > +These functions allocate an async connection descriptor which is of type struct
> > +:c:type:`v4l2_async_connection` embedded in a driver-specific struct. The &struct
> > +:c:type:`v4l2_async_connection` shall be the first member of this struct:
> >  
> >  .. code-block:: c
> >  
> >  	struct my_async_subdev {
> > -		struct v4l2_async_subdev asd;
> > +		struct v4l2_async_connection asd;
> 
> s/asd/asc/

Yes.

There are other minor issues in the example, I'll address them in a
separate patch.

> 
> >  		...
> >  	};
> >  
> > @@ -244,10 +244,10 @@ notifier callback is called. After all subdevices have been located the
> >  system the .unbind() method is called. All three callbacks are optional.
> >  
> >  Drivers can store any type of custom data in their driver-specific
> > -:c:type:`v4l2_async_subdev` wrapper. If any of that data requires special
> > +:c:type:`v4l2_async_connection` wrapper. If any of that data requires special
> >  handling when the structure is freed, drivers must implement the ``.destroy()``
> >  notifier callback. The framework will call it right before freeing the
> > -:c:type:`v4l2_async_subdev`.
> > +:c:type:`v4l2_async_connection`.
> >  
> >  Calling subdev operations
> >  ~~~~~~~~~~~~~~~~~~~~~~~~~
> > diff --git a/drivers/media/i2c/max9286.c b/drivers/media/i2c/max9286.c
> > index 64f5c49cae776..44def5e3ba5d1 100644
> > --- a/drivers/media/i2c/max9286.c
> > +++ b/drivers/media/i2c/max9286.c
> > @@ -161,11 +161,12 @@ struct max9286_source {
> >  };
> >  
> >  struct max9286_asd {
> 
> This should be renamed to max9286_asc. I said in the review of a
> previous version that I was fine keeping the name as-is, as the
> v4l2_async_subdev structure was reintroduced later in the series, but
> that's not the case anymore.

This should be less confusing now than with v2 where the internal async
sub-device struct was not the same object that was used by drivers
previously.

> 
> > -	struct v4l2_async_subdev base;
> > +	struct v4l2_async_connection base;
> >  	struct max9286_source *source;
> >  };
> >  
> > -static inline struct max9286_asd *to_max9286_asd(struct v4l2_async_subdev *asd)
> > +static inline struct max9286_asd *
> > +to_max9286_asd(struct v4l2_async_connection *asd)
> 
> s/asd/asc/g
> 
> There's more below, in most drivers.

Yes, but is it worth renaming them? In all existing drivers there's no
functional change here. If you'd like them to be renamed, I think it should
be done after this set, that has already more than 32 patches and more than
2000 lines of changes.

> 
> >  {
> >  	return container_of(asd, struct max9286_asd, base);
> >  }
> > @@ -659,7 +660,7 @@ static int max9286_set_pixelrate(struct max9286_priv *priv)
> >  
> >  static int max9286_notify_bound(struct v4l2_async_notifier *notifier,
> >  				struct v4l2_subdev *subdev,
> > -				struct v4l2_async_subdev *asd)
> > +				struct v4l2_async_connection *asd)
> >  {
> >  	struct max9286_priv *priv = sd_to_max9286(notifier->sd);
> >  	struct max9286_source *source = to_max9286_asd(asd)->source;
> > @@ -721,7 +722,7 @@ static int max9286_notify_bound(struct v4l2_async_notifier *notifier,
> >  
> >  static void max9286_notify_unbind(struct v4l2_async_notifier *notifier,
> >  				  struct v4l2_subdev *subdev,
> > -				  struct v4l2_async_subdev *asd)
> > +				  struct v4l2_async_connection *asd)
> >  {
> >  	struct max9286_priv *priv = sd_to_max9286(notifier->sd);
> >  	struct max9286_source *source = to_max9286_asd(asd)->source;
> 
> [snip]
> 
> > diff --git a/drivers/media/platform/atmel/atmel-isi.h b/drivers/media/platform/atmel/atmel-isi.h
> > index 7ad3895a2c87e..58ce900ca4c90 100644
> > --- a/drivers/media/platform/atmel/atmel-isi.h
> > +++ b/drivers/media/platform/atmel/atmel-isi.h
> > @@ -121,7 +121,7 @@
> >  #define ISI_DATAWIDTH_8				0x01
> >  #define ISI_DATAWIDTH_10			0x02
> >  
> > -struct v4l2_async_subdev;
> > +struct v4l2_async_connection;
> 
> You can actually drop this, it's not used in this file.

Ack. I think I'll add a new patch for this as these don't really belong
here.

> 
> >  
> >  struct isi_platform_data {
> >  	u8 has_emb_sync;
> 
> [snip]
> 
> > diff --git a/drivers/media/platform/renesas/rcar-vin/rcar-vin.h b/drivers/media/platform/renesas/rcar-vin/rcar-vin.h
> > index cb206d3976ddf..c99d64e1cb01f 100644
> > --- a/drivers/media/platform/renesas/rcar-vin/rcar-vin.h
> > +++ b/drivers/media/platform/renesas/rcar-vin/rcar-vin.h
> > @@ -106,7 +106,7 @@ struct rvin_video_format {
> >  
> >  /**
> >   * struct rvin_parallel_entity - Parallel video input endpoint descriptor
> > - * @asd:	sub-device descriptor for async framework
> > + * @asc:	sub-device descriptor for async framework
> 
> The description of the field should also be updated.

I'll address that in v4.

> 
> >   * @subdev:	subdevice matched using async framework
> >   * @mbus_type:	media bus type
> >   * @bus:	media bus parallel configuration
> > @@ -115,7 +115,7 @@ struct rvin_video_format {
> >   *
> >   */
> >  struct rvin_parallel_entity {
> > -	struct v4l2_async_subdev *asd;
> > +	struct v4l2_async_connection *asc;
> >  	struct v4l2_subdev *subdev;
> >  
> >  	enum v4l2_mbus_type mbus_type;
> > @@ -275,7 +275,7 @@ struct rvin_dev {
> >   * @notifier:		group notifier for CSI-2 async subdevices
> >   * @vin:		VIN instances which are part of the group
> >   * @link_setup:		Callback to create all links for the media graph
> > - * @remotes:		array of pairs of fwnode and subdev pointers
> > + * @remotes:		array of pairs of async connection and subdev pointers
> >   *			to all remote subdevices.
> >   */
> >  struct rvin_group {
> > @@ -291,7 +291,7 @@ struct rvin_group {
> >  	int (*link_setup)(struct rvin_dev *vin);
> >  
> >  	struct {
> > -		struct v4l2_async_subdev *asd;
> > +		struct v4l2_async_connection *asc;
> >  		struct v4l2_subdev *subdev;
> >  	} remotes[RVIN_REMOTES_MAX];
> >  };
> 
> [snip]
> 
> > diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h
> > index 54f9f45ed3d8e..38d9d097fdb52 100644
> > --- a/include/media/v4l2-async.h
> > +++ b/include/media/v4l2-async.h
> > @@ -34,7 +34,7 @@ enum v4l2_async_match_type {
> >  };
> >  
> >  /**
> > - * struct v4l2_async_match_desc - async sub-device match information
> > + * struct v4l2_async_match_desc - async connection match information
> >   *
> >   * @type:	type of match that will be used
> >   * @fwnode:	pointer to &struct fwnode_handle to be matched.
> > @@ -62,21 +62,21 @@ struct v4l2_async_match_desc {
> >  };
> >  
> >  /**
> > - * struct v4l2_async_subdev - sub-device descriptor, as known to a bridge
> > + * struct v4l2_async_connection - connection descriptor, as known to a bridge
> >   *
> >   * @match:	struct of match type and per-bus type matching data sets
> > - * @asd_entry:	used to add struct v4l2_async_subdev objects to the
> > - *		master notifier @asd_entry
> > - * @waiting_entry: used to link struct v4l2_async_subdev objects, waiting to be
> > - *		probed, to a notifier->waiting_list list
> > + * @asc_entry:	used to add struct v4l2_async_connection objects to the
> > + *		master notifier @asc_list
> > + * @waiting_entry: used to link struct v4l2_async_connection objects, waiting to
> > + *		be probed, to a notifier->waiting_list list
> >   *
> >   * When this struct is used as a member in a driver specific struct,
> >   * the driver specific struct shall contain the &struct
> > - * v4l2_async_subdev as its first member.
> > + * v4l2_async_connection as its first member.
> >   */
> > -struct v4l2_async_subdev {
> > +struct v4l2_async_connection {
> >  	struct v4l2_async_match_desc match;
> > -	struct list_head asd_entry;
> > +	struct list_head asc_entry;
> >  	struct list_head waiting_entry;
> >  };
> >  
> > @@ -86,17 +86,17 @@ struct v4l2_async_subdev {
> >   * @complete:	All subdevices have been probed successfully. The complete
> >   *		callback is only executed for the root notifier.
> >   * @unbind:	a subdevice is leaving
> > - * @destroy:	the asd is about to be freed
> > + * @destroy:	the asc is about to be freed
> >   */
> >  struct v4l2_async_notifier_operations {
> >  	int (*bound)(struct v4l2_async_notifier *notifier,
> >  		     struct v4l2_subdev *subdev,
> > -		     struct v4l2_async_subdev *asd);
> > +		     struct v4l2_async_connection *asc);
> >  	int (*complete)(struct v4l2_async_notifier *notifier);
> >  	void (*unbind)(struct v4l2_async_notifier *notifier,
> >  		       struct v4l2_subdev *subdev,
> > -		       struct v4l2_async_subdev *asd);
> > -	void (*destroy)(struct v4l2_async_subdev *asd);
> > +		       struct v4l2_async_connection *asc);
> > +	void (*destroy)(struct v4l2_async_connection *asc);
> >  };
> >  
> >  /**
> > @@ -106,7 +106,7 @@ struct v4l2_async_notifier_operations {
> >   * @v4l2_dev:	v4l2_device of the root notifier, NULL otherwise
> >   * @sd:		sub-device that registered the notifier, NULL otherwise
> >   * @parent:	parent notifier
> > - * @asd_list:	master list of struct v4l2_async_subdev
> > + * @asc_list:	master list of struct v4l2_async_subdev
> 
> v4l2_async_subdev is no more.
> 
> >   * @waiting_list: list of struct v4l2_async_subdev, waiting for their drivers
> 
> Same here.

I'll check these for v4.

> 
> >   * @done_list:	list of struct v4l2_subdev, already probed
> >   * @notifier_entry: member in a global list of notifiers
> > @@ -116,7 +116,7 @@ struct v4l2_async_notifier {
> >  	struct v4l2_device *v4l2_dev;
> >  	struct v4l2_subdev *sd;
> >  	struct v4l2_async_notifier *parent;
> > -	struct list_head asd_list;
> > +	struct list_head asc_list;
> >  	struct list_head waiting_list;
> >  	struct list_head done_list;
> >  	struct list_head notifier_entry;
> > @@ -134,53 +134,53 @@ void v4l2_async_debug_init(struct dentry *debugfs_dir);
> >   *
> >   * @notifier: pointer to &struct v4l2_async_notifier
> >   *
> > - * This function initializes the notifier @asd_entry. It must be called
> > + * This function initializes the notifier @asc_entry. It must be called
> 
> This sounds like it should be asc_list. The issues was likely introduced
> in an earlier patch in the series.

I believe so, yes. For v4.

> 
> >   * before adding a subdevice to a notifier, using one of:
> >   * v4l2_async_nf_add_fwnode_remote(), v4l2_async_nf_add_fwnode() or
> >   * v4l2_async_nf_add_i2c().
> >   */
> >  void v4l2_async_nf_init(struct v4l2_async_notifier *notifier);
> >  
> > -struct v4l2_async_subdev *
> > +struct v4l2_async_connection *
> >  __v4l2_async_nf_add_fwnode(struct v4l2_async_notifier *notifier,
> >  			   struct fwnode_handle *fwnode,
> > -			   unsigned int asd_struct_size);
> > +			   unsigned int asc_struct_size);
> >  /**
> >   * v4l2_async_nf_add_fwnode - Allocate and add a fwnode async
> > - *				subdev to the notifier's master asd_entry.
> > + *				subdev to the notifier's master asc_entry.
> 
> Same here.
> 
> >   *
> >   * @notifier: pointer to &struct v4l2_async_notifier
> >   * @fwnode: fwnode handle of the sub-device to be matched, pointer to
> >   *	    &struct fwnode_handle
> > - * @type: Type of the driver's async sub-device struct. The &struct
> > - *	  v4l2_async_subdev shall be the first member of the driver's async
> > - *	  sub-device struct, i.e. both begin at the same memory address.
> > + * @type: Type of the driver's async sub-device or connection struct. The
> > + *	  &struct v4l2_async_connection shall be the first member of the
> > + *	  driver's async struct, i.e. both begin at the same memory address.
> >   *
> > - * Allocate a fwnode-matched asd of size asd_struct_size, and add it to the
> > - * notifiers @asd_entry. The function also gets a reference of the fwnode which
> > + * Allocate a fwnode-matched asc of size asc_struct_size, and add it to the
> > + * notifiers @asc_entry. The function also gets a reference of the fwnode which
> 
> Here too.
> 
> >   * is released later at notifier cleanup time.
> >   */
> >  #define v4l2_async_nf_add_fwnode(notifier, fwnode, type)		\
> >  	((type *)__v4l2_async_nf_add_fwnode(notifier, fwnode, sizeof(type)))
> >  
> > -struct v4l2_async_subdev *
> > +struct v4l2_async_connection *
> >  __v4l2_async_nf_add_fwnode_remote(struct v4l2_async_notifier *notif,
> >  				  struct fwnode_handle *endpoint,
> > -				  unsigned int asd_struct_size);
> > +				  unsigned int asc_struct_size);
> >  /**
> >   * v4l2_async_nf_add_fwnode_remote - Allocate and add a fwnode
> >   *						  remote async subdev to the
> > - *						  notifier's master asd_entry.
> > + *						  notifier's master asc_entry.
> 
> And here.
> 
> >   *
> >   * @notifier: pointer to &struct v4l2_async_notifier
> >   * @ep: local endpoint pointing to the remote sub-device to be matched,
> >   *	pointer to &struct fwnode_handle
> >   * @type: Type of the driver's async sub-device struct. The &struct
> > - *	  v4l2_async_subdev shall be the first member of the driver's async
> > + *	  v4l2_async_connection shall be the first member of the driver's async
> >   *	  sub-device struct, i.e. both begin at the same memory address.
> >   *
> >   * Gets the remote endpoint of a given local endpoint, set it up for fwnode
> > - * matching and adds the async sub-device to the notifier's @asd_entry. The
> > + * matching and adds the async connection to the notifier's @asc_entry. The
> 
> Here too again.
> 
> >   * function also gets a reference of the fwnode which is released later at
> >   * notifier cleanup time.
> >   *
> > @@ -190,19 +190,19 @@ __v4l2_async_nf_add_fwnode_remote(struct v4l2_async_notifier *notif,
> >  #define v4l2_async_nf_add_fwnode_remote(notifier, ep, type) \
> >  	((type *)__v4l2_async_nf_add_fwnode_remote(notifier, ep, sizeof(type)))
> >  
> > -struct v4l2_async_subdev *
> > +struct v4l2_async_connection *
> >  __v4l2_async_nf_add_i2c(struct v4l2_async_notifier *notifier,
> >  			int adapter_id, unsigned short address,
> > -			unsigned int asd_struct_size);
> > +			unsigned int asc_struct_size);
> >  /**
> >   * v4l2_async_nf_add_i2c - Allocate and add an i2c async
> > - *				subdev to the notifier's master asd_entry.
> > + *				subdev to the notifier's master asc_entry.
> 
> And finally here.
> 
> >   *
> >   * @notifier: pointer to &struct v4l2_async_notifier
> >   * @adapter: I2C adapter ID to be matched
> >   * @address: I2C address of sub-device to be matched
> >   * @type: Type of the driver's async sub-device struct. The &struct
> > - *	  v4l2_async_subdev shall be the first member of the driver's async
> > + *	  v4l2_async_connection shall be the first member of the driver's async
> >   *	  sub-device struct, i.e. both begin at the same memory address.
> 
> s/sub-device/connection/ ? Please double-check if other instances of
> subdev(ice) have been forgotten.

Sure.

> 
> >   *
> >   * Same as v4l2_async_nf_add_fwnode() but for I2C matched
> > diff --git a/include/media/v4l2-fwnode.h b/include/media/v4l2-fwnode.h
> > index 855dae84b751d..4e4a6cf83097a 100644
> > --- a/include/media/v4l2-fwnode.h
> > +++ b/include/media/v4l2-fwnode.h
> > @@ -23,7 +23,7 @@
> >  
> >  struct fwnode_handle;
> >  struct v4l2_async_notifier;
> > -struct v4l2_async_subdev;
> > +struct v4l2_async_connection;
> 
> This is a sign the forward-declaration isn't needed.

Actually I think they all can be now removed, probably due to Jacopo's
patch. I'll address this for v4.

> 
> >  
> >  /**
> >   * struct v4l2_fwnode_endpoint - the endpoint data structure
> > diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> > index 82e4cf3dd2e05..215fc8af87614 100644
> > --- a/include/media/v4l2-subdev.h
> > +++ b/include/media/v4l2-subdev.h
> > @@ -1022,8 +1022,7 @@ struct v4l2_subdev_platform_data {
> >   *	    either dev->of_node->fwnode or dev->fwnode (whichever is non-NULL).
> >   * @async_list: Links this subdev to a global subdev_entry or @notifier->done
> >   *	list.
> > - * @asd: Pointer to respective &struct v4l2_async_subdev.
> > - * @notifier: Pointer to the managing notifier.
> 
> Did you drop this field by mistake ?

Oops. This indeed belongs to a later patch, not here.

> 
> > + * @asd: Pointer to respective &struct v4l2_async_connection.
> >   * @subdev_notifier: A sub-device notifier implicitly registered for the sub-
> >   *		     device using v4l2_async_register_subdev_sensor().
> >   * @pdata: common part of subdevice platform data
> > @@ -1065,7 +1064,7 @@ struct v4l2_subdev {
> >  	struct device *dev;
> >  	struct fwnode_handle *fwnode;
> >  	struct list_head async_list;
> > -	struct v4l2_async_subdev *asd;
> > +	struct v4l2_async_connection *asd;
> >  	struct v4l2_async_notifier *notifier;
> >  	struct v4l2_async_notifier *subdev_notifier;
> >  	struct v4l2_subdev_platform_data *pdata;
> 

-- 
Kind regards,

Sakari Ailus



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux