Re: [PATCH 09/18] media: v4l: async: Differentiate connecting and creating sub-devices

[Sakari Ailus <sakari.ailus@linux.intel.com>]

Hi Laurent,

On Tue, Apr 25, 2023 at 05:14:42AM +0300, Laurent Pinchart wrote:
> Hi Sakari,
> 
> Thank you for the patch.
> 
> On Fri, Apr 14, 2023 at 04:35:00PM +0300, Sakari Ailus wrote:
> > On Fri, Apr 14, 2023 at 10:52:25AM +0200, Jacopo Mondi wrote:
> > > On Thu, Mar 30, 2023 at 02:58:44PM +0300, Sakari Ailus wrote:
> > > > When the v4l2-async framework was introduced, the use case for it was to
> > > > connect a camera sensor with a parallel receiver. Both tended to be rather
> > > > simple devices with a single connection between them.
> > > >
> > > > The framework has been since improved in multiple ways but there are
> > > > limitations that have remained, for instance the assumption an async
> > > > sub-device is connected towards a single notifier and via a single link
> > > > only.
> > > >
> > > > This patch adds an object that represents the device while an earlier
> > > > patch in the series re-purposed the old struct v4l2_async_subdev as the
> > > > connection.
> > > >
> > > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > > > ---
> > > >  .../platform/rockchip/rkisp1/rkisp1-common.h  |   2 +-
> > > >  .../platform/rockchip/rkisp1/rkisp1-dev.c     |   8 +-
> > > >  drivers/media/platform/ti/omap3isp/isp.h      |   2 +-
> > > >  drivers/media/v4l2-core/v4l2-async.c          | 163 ++++++++++++++++--
> > > >  include/media/v4l2-async.h                    |  32 +++-
> > > >  include/media/v4l2-subdev.h                   |   2 +-
> 
> We the introduction of such a new core concept, Documentation/ should
> also be updated.
> 
> > > >  6 files changed, 179 insertions(+), 30 deletions(-)
> > > >
> > > > diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
> > > > index d30f0ecb1bfd..a1293c45aae1 100644
> > > > --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
> > > > +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
> > > > @@ -148,7 +148,7 @@ struct rkisp1_info {
> > > >   * @port:		port number (0: MIPI, 1: Parallel)
> > > >   */
> > > >  struct rkisp1_sensor_async {
> > > > -	struct v4l2_async_connection asd;
> > > > +	struct v4l2_async_subdev asd;
> 
> Ah, we're back to "asd" naming... There's no point in going back and
> forth in drivers, so you can ignore my comment about renaming the
> variables in drivers. Please however record the reason why variables are
> not renamed in the commit message of patch 08/18.
> 
> On second thought, why does patch 08/18 switch to v4l2_async_connection
> in a very large number of drivers, and this patch only moves back to
> v4l2_async_subdev in rkisp1 and omap3isp ? What's special about these
> two drivers ?

These changes actually appear unintended currently. The relation between
the driver struct remains related to the async connection. I'll drop them.

-- 
Sakari Ailus