mbox series

[0/5] v4l2-fwnode: Remove v4l2_async_notifier_parse_fwnode_endpoints_by_port() helper

Message ID 20201125164450.2056963-1-niklas.soderlund+renesas@ragnatech.se (mailing list archive)
Headers show
Series v4l2-fwnode: Remove v4l2_async_notifier_parse_fwnode_endpoints_by_port() helper | expand

Message

Niklas Söderlund Nov. 25, 2020, 4:44 p.m. UTC
Hello,

This series aims to remove a V4L2 helper that provides a too simple 
implementation, v4l2_async_notifier_parse_fwnode_endpoints_by_port().  
Instead drivers shall implement what the helper does directly to get 
greater control of the process. There is only one user left in tree of 
this interface, R-Car VIN.

This series starts therefor to rework the R-Car VIN driver to not depend 
on the helper. And in the process a small fwnode imbalance is fixed.  
After the last user of the helper is reworked the series removes the 
helper to prevent usage of it to resurface.

This series is based on-top of the latest media tree and is tested on 
Renesas R-Car M3-N and Koelsch without any regressions or change in 
behavior detected.

Niklas Söderlund (5):
  rcar-vin: Only dynamically allocate v4l2_async_subdev
  rcar-vin: Rework parallel firmware parsing
  rcar-vin: Use v4l2_async_subdev instead of fwnode_handle to match
    subdevices
  rcar-vin: Rework CSI-2 firmware parsing
  v4l2-fwnode: Remove
    v4l2_async_notifier_parse_fwnode_endpoints_by_port()

 drivers/media/platform/rcar-vin/rcar-core.c | 156 ++++++++++++--------
 drivers/media/platform/rcar-vin/rcar-dma.c  |  16 +-
 drivers/media/platform/rcar-vin/rcar-v4l2.c |  12 +-
 drivers/media/platform/rcar-vin/rcar-vin.h  |   8 +-
 drivers/media/v4l2-core/v4l2-fwnode.c       |  14 --
 include/media/v4l2-fwnode.h                 |  53 -------
 6 files changed, 113 insertions(+), 146 deletions(-)

Comments

Jacopo Mondi Nov. 26, 2020, 10:12 a.m. UTC | #1
Hi Niklas, Sakari,

On Wed, Nov 25, 2020 at 05:44:45PM +0100, Niklas Söderlund wrote:
> Hello,
>
> This series aims to remove a V4L2 helper that provides a too simple
> implementation, v4l2_async_notifier_parse_fwnode_endpoints_by_port().
> Instead drivers shall implement what the helper does directly to get
> greater control of the process. There is only one user left in tree of
> this interface, R-Car VIN.

What is the plan going forward ?
removing v4l2_async_notifier_parse_fwnode_endpoints_by_port() here
then remove v4l2_async_notifier_parse_fwnode_endpoints() as it has a
single user in mainline too ?

Are we standardizing all platform drivers to use
v4l2_async_notifier_add_fwnode_subdev() and perform fwnode.match
initialization by themselves or is there a plan to replace
v4l2_async_notifier_parse_fwnode_endpoints*() with something else ?

Thanks
  j

>
> This series starts therefor to rework the R-Car VIN driver to not depend
> on the helper. And in the process a small fwnode imbalance is fixed.
> After the last user of the helper is reworked the series removes the
> helper to prevent usage of it to resurface.
>
> This series is based on-top of the latest media tree and is tested on
> Renesas R-Car M3-N and Koelsch without any regressions or change in
> behavior detected.
>
> Niklas Söderlund (5):
>   rcar-vin: Only dynamically allocate v4l2_async_subdev
>   rcar-vin: Rework parallel firmware parsing
>   rcar-vin: Use v4l2_async_subdev instead of fwnode_handle to match
>     subdevices
>   rcar-vin: Rework CSI-2 firmware parsing
>   v4l2-fwnode: Remove
>     v4l2_async_notifier_parse_fwnode_endpoints_by_port()
>
>  drivers/media/platform/rcar-vin/rcar-core.c | 156 ++++++++++++--------
>  drivers/media/platform/rcar-vin/rcar-dma.c  |  16 +-
>  drivers/media/platform/rcar-vin/rcar-v4l2.c |  12 +-
>  drivers/media/platform/rcar-vin/rcar-vin.h  |   8 +-
>  drivers/media/v4l2-core/v4l2-fwnode.c       |  14 --
>  include/media/v4l2-fwnode.h                 |  53 -------
>  6 files changed, 113 insertions(+), 146 deletions(-)
>
> --
> 2.29.2
>
Sakari Ailus Nov. 26, 2020, 10:22 a.m. UTC | #2
Hi Jacopo,

On Thu, Nov 26, 2020 at 11:12:51AM +0100, Jacopo Mondi wrote:
> Hi Niklas, Sakari,
> 
> On Wed, Nov 25, 2020 at 05:44:45PM +0100, Niklas Söderlund wrote:
> > Hello,
> >
> > This series aims to remove a V4L2 helper that provides a too simple
> > implementation, v4l2_async_notifier_parse_fwnode_endpoints_by_port().
> > Instead drivers shall implement what the helper does directly to get
> > greater control of the process. There is only one user left in tree of
> > this interface, R-Car VIN.
> 
> What is the plan going forward ?
> removing v4l2_async_notifier_parse_fwnode_endpoints_by_port() here
> then remove v4l2_async_notifier_parse_fwnode_endpoints() as it has a
> single user in mainline too ?
> 
> Are we standardizing all platform drivers to use
> v4l2_async_notifier_add_fwnode_subdev() and perform fwnode.match
> initialization by themselves or is there a plan to replace

Yes, please.

> v4l2_async_notifier_parse_fwnode_endpoints*() with something else ?

That's always been somewhat clunky and required special cases. The other
option, i.e. what this patchset does, is straightforward as well as allows
setting defaults in drivers, and admittedly, comes with a little bit of
extra code in drivers in areas that are driver specific. The old functions
such as v4l2_async_notifier_parse_fwnode_endpoints() just pretended they
were not...
Jacopo Mondi Nov. 27, 2020, 11:19 a.m. UTC | #3
Hi Sakari,

On Thu, Nov 26, 2020 at 12:22:05PM +0200, Sakari Ailus wrote:
> Hi Jacopo,
>
> On Thu, Nov 26, 2020 at 11:12:51AM +0100, Jacopo Mondi wrote:
> > Hi Niklas, Sakari,
> >
> > On Wed, Nov 25, 2020 at 05:44:45PM +0100, Niklas Söderlund wrote:
> > > Hello,
> > >
> > > This series aims to remove a V4L2 helper that provides a too simple
> > > implementation, v4l2_async_notifier_parse_fwnode_endpoints_by_port().
> > > Instead drivers shall implement what the helper does directly to get
> > > greater control of the process. There is only one user left in tree of
> > > this interface, R-Car VIN.
> >
> > What is the plan going forward ?
> > removing v4l2_async_notifier_parse_fwnode_endpoints_by_port() here
> > then remove v4l2_async_notifier_parse_fwnode_endpoints() as it has a
> > single user in mainline too ?
> >
> > Are we standardizing all platform drivers to use
> > v4l2_async_notifier_add_fwnode_subdev() and perform fwnode.match
> > initialization by themselves or is there a plan to replace
>
> Yes, please.
>
> > v4l2_async_notifier_parse_fwnode_endpoints*() with something else ?
>
> That's always been somewhat clunky and required special cases. The other
> option, i.e. what this patchset does, is straightforward as well as allows
> setting defaults in drivers, and admittedly, comes with a little bit of
> extra code in drivers in areas that are driver specific. The old functions
> such as v4l2_async_notifier_parse_fwnode_endpoints() just pretended they
> were not...

Agreed in full :)
(At the expense of a little extra code in drivers)

>
> --
> Regards,
>
> Sakari Ailus