linux-kernel - Re: [PATCH 1/2] media: i2c: Add driver for the Analog Devices ADDI9036 ToF front-end

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 11 Sep 2020 16:49:02 +0300
From:   Sakari Ailus <sakari.ailus@...ux.intel.com>
To:     "Togorean, Bogdan" <Bogdan.Togorean@...log.com>
Cc:     Laurent Pinchart <laurent.pinchart@...asonboard.com>,
        "linux-media@...r.kernel.org" <linux-media@...r.kernel.org>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Laurent Pinchart <laurent.pinchart+renesas@...asonboard.com>,
        Jacopo Mondi <jacopo+renesas@...ndi.org>,
        Kieran Bingham <kieran.bingham+renesas@...asonboard.com>,
        YueHaibing <yuehaibing@...wei.com>,
        Bingbu Cao <bingbu.cao@...el.com>,
        Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>,
        Shawn Tu <shawnx.tu@...el.com>,
        Dongchun Zhu <dongchun.zhu@...iatek.com>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "Cozma, Andrei" <Andrei.Cozma@...log.com>
Subject: Re: [PATCH 1/2] media: i2c: Add driver for the Analog Devices
 ADDI9036 ToF front-end

Hi Bogdan,

On Fri, Sep 11, 2020 at 01:32:39PM +0000, Togorean, Bogdan wrote:
> Hi Laurent,
> 
> > Hi Bogdan,
> > 
> > Thank you for the patch.
> 
> Thank you for review!
> 
> > 
> > On Thu, Sep 10, 2020 at 07:24:06PM +0300, Bogdan Togorean wrote:
> > > The ADDI9036 is a complete, 45 MHz, front-end solution for charge
> > > coupled device (CCD) time of flight (TOF) imaging applications.
> > >
> > > It has 2-lane MIPI CSI-2 RAW12 data output and i2c control interface.
> > >
> > > The programming of calibration and firmware is performed by userspace
> > > SDK through CID_USER class IOCTLs.
> > 
> > I'm afraid that's not acceptable. The firmware should be loaded using
> > the Linux kernel firmware API. As for calibration, is it static data ?
> > How is it generated ?
> 
> Calibration data is static data, generated at production and stored on and EEPROM
> memory on camera board. The camera board consists mainly of ADDI9036 processor
> + CCD image sensor + EEPROM.
> 
> The firmware is a set of (address, data) tuples that are written via I2C.
> We want this driver to be a generic one and support all implementations
> of ADDI9036 ToF processor. Now the problem is that some implementations 
> store firmware + calibration on external EEPROM and some store just calibration 
> data on EEPROM and firmware is loaded from a file stored in SDK. With the help 
> of the 2 custom controls we move the camera model detection, firmware
> and calibration fetching from driver to user space SDK.

Can you figure out by reading EEPROM which one of these is the case?
Alternatively you could add a custom DT property IMO.

The calibration data could be loaded using request_firmware(), too. I'd use
a specific directory for just this purpose.

> 
> Also an important point is that the ToF processor has 3 operating modes 
> (near, medium and far). At each mode change a new set of firmware + 
> calibration data is loaded into ADDI9036 chip. If we move the firmware loading
> in driver we would still need a custom enumeration control to select the working mode.

I don't see there being an issue having such a control.

-- 
Kind regards,

Sakari Ailus