Re: RFC: Arducam 64 MP (Hawkeye)

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hello,

On Fri, Sep 23, 2022 at 10:03:19AM +0000, Sakari Ailus wrote:
> On Fri, Sep 23, 2022 at 10:49:29AM +0100, Kieran Bingham wrote:
> > Hi Sakari, Laurent,
> > 
> > Last night I hooked up the Arducam 64 MP Hawkeye camera [0], to an RPi4,
> > and added the camera helper for the RaspberryPi IPA to libcamera, and
> > was able to use the camera directly with dtoverlay=arducam_64mp in the
> > Raspberry Pi /boot/config.txt using the latest release from Raspberry
> > Pi.
> > 
> > Raspberry Pi have already added the driver for this camera [1] to their
> > tree, and I have been given the tuning files from Arducam.
> > 
> > This has allowed me to capture an image with pretty good response to
> > changing lighting conditions, and colours. [2] ... The AF isn't yet
> > enabled, so that shot is out of focus a little. (That's for later).
> > 
> > Arducam do not wish to name the sensor used in the module, and have
> > called it the 'arducam_64mp'. But attempting to upstreaming this with
> > that name worries me.
> > 
> > Furthermore, we would like to maintain libcamera as supporting cameras
> > that have 'upstream' drivers (or drivers that are on their way
> > upstream), so I'm keen to identify how we can upstream the drivers to
> > create a better experience for users who are currently finding that they
> > need to run a fork of libcamera to operate the module.
> > 
> > So ultimately - my open question is ... Is it acceptable to have camera
> > drivers that are named by their 'module/integration' rather than their
> > sensor?
> 
> A lot of users (especially the existing ones) depend on the entity name
> currently. I don't think we could change it. For new ones I guess that
> would be possible.
> 
> Alternatively this could be a string control I think. That could be added
> to existing drivers as well.

The question here, as far as I understand, isn't about the entity name
exposed to userspace, but about the driver name. The entity name
certainly matters too.

Camera sensor drivers we have in the mainline kernel are named after the
camera sensor model, and I think we should continue that. If this were a
custom silicon made by Arducam they could name it any way they want, but
if it's a sensor from a known sensor manufacturer, I don't think the
name should be hidden. I wouldn't be surprised if it would be possible
to identify the camera sensor relatively easily from the register set
anyway, which would render this whole game pointless.

One of the major advantages of upstreaming driver is the community
maintenance that you get from free, including improvements to the driver
from other developers who use the same camera sensor in a different
product. We don't want to have multiple drivers for the same hardware in
the mainline kernel, there are precedents for that due to historical
reasons (mostly people not realizing that the same IP core was used in
different SoCs), and there are efforts to fix that. I don't see a reason
to go the opposite way.

TL;DR: Unless there's a very compeling reason not to follow the usual
practice, I don't see why we should make an exception in this case.

-- 
Regards,

Laurent Pinchart