linux-kernel - Re: [PATCH v5 1/2] Add OV5647 device tree documentation

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:   Mon, 12 Dec 2016 12:15:04 +0000
From:   Ramiro Oliveira <Ramiro.Oliveira@...opsys.com>
To:     Sakari Ailus <sakari.ailus@....fi>,
        Ramiro Oliveira <Ramiro.Oliveira@...opsys.com>
CC:     <mchehab@...nel.org>, <linux-kernel@...r.kernel.org>,
        <linux-media@...r.kernel.org>, <robh+dt@...nel.org>,
        <devicetree@...r.kernel.org>, <davem@...emloft.net>,
        <gregkh@...uxfoundation.org>, <geert+renesas@...der.be>,
        <akpm@...ux-foundation.org>, <linux@...ck-us.net>,
        <hverkuil@...all.nl>, <dheitmueller@...nellabs.com>,
        <slongerbeam@...il.com>, <lars@...afoo.de>,
        <robert.jarzmik@...e.fr>, <pavel@....cz>, <pali.rohar@...il.com>,
        <sakari.ailus@...ux.intel.com>, <mark.rutland@....com>,
        <CARLOS.PALMINHA@...opsys.com>
Subject: Re: [PATCH v5 1/2] Add OV5647 device tree documentation

Hi Sakari

On 12/12/2016 11:49 AM, Sakari Ailus wrote:
> Hi Ramiro,
> 
> On Mon, Dec 12, 2016 at 11:39:31AM +0000, Ramiro Oliveira wrote:
>> Hi Sakari,
>>
>> Thank you for the feedback.
>>
>> On 12/7/2016 10:33 PM, Sakari Ailus wrote:
>>> Hi Ramiro,
>>>
>>> Thank you for the patch.
>>>
>>> On Mon, Dec 05, 2016 at 05:36:33PM +0000, Ramiro Oliveira wrote:
>>>> Add device tree documentation.
>>>>
>>>> Signed-off-by: Ramiro Oliveira <roliveir@...opsys.com>
>>>> ---
>>>>  .../devicetree/bindings/media/i2c/ov5647.txt          | 19 +++++++++++++++++++
>>>>  1 file changed, 19 insertions(+)
>>>>  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov5647.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/media/i2c/ov5647.txt b/Documentation/devicetree/bindings/media/i2c/ov5647.txt
>>>> new file mode 100644
>>>> index 0000000..4c91b3b
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/media/i2c/ov5647.txt
>>>> @@ -0,0 +1,19 @@
>>>> +Omnivision OV5647 raw image sensor
>>>> +---------------------------------
>>>> +
>>>> +OV5647 is a raw image sensor with MIPI CSI-2 and CCP2 image data interfaces
>>>> +and CCI (I2C compatible) control bus.
>>>> +
>>>> +Required properties:
>>>> +
>>>> +- compatible	: "ovti,ov5647";
>>>> +- reg		: I2C slave address of the sensor;
>>>> +
>>>> +The common video interfaces bindings (see video-interfaces.txt) should be
>>>> +used to specify link to the image data receiver. The OV5647 device
>>>> +node should contain one 'port' child node with an 'endpoint' subnode.
>>>> +
>>>> +Following properties are valid for the endpoint node:
>>>> +
>>>> +- data-lanes : (optional) specifies MIPI CSI-2 data lanes as covered in
>>>> +  video-interfaces.txt.  The sensor supports only two data lanes.
>>>
>>> Doesn't this sensor require a external clock, a reset GPIO and / or a
>>> regulator or a few? Do you need data-lanes, unless you can change the order
>>> or the number?
>>
>> In the setup I'm using, I'm not aware of any reset GPIO or regulator. I do use a
>> external clock but it's fixed and not controlled by SW. Should I add a property
>> for this?
> 
> The sensor datasheet defines a power-up and power-down sequence for the
> device. If you don't implement these sequences in the driver on a DT based
> system, nothing suggests that they're implemented correctly. Could it be
> that the boot loader simply enables the regulators or another device
> requires them to be enabled?
> 
> I presume at least the reset GPIO should be controlled explicitly in order
> to ensure correct function. Although hardware can be surprising: I have one
> production system that has no reset GPIO for the sensor albeit the sensor
> datasheet says that's part of the power up sequence.
> 

Sorry for the misunderstanding. I wanted to say that, there is no SW controlled
reset. In the board we're using to connect the sensor to our D-PHY we have a
GPIO controller that when it receives power, it removes the sensor from reset,
so I have no control over that.

Regarding the clock, should I create a new property?

And also, regarding the data-lanes, AFAIK it isn't possible to change the order
of the data and clock lanes so should I remove that property?

>>
>>>
>>> An example DT snippet wouldn't hurt.
>>
>> Sure, I can add a example snippet.
>>
>>>
>>
>