linux-kernel - Re: [PATCH v3 2/2] media: i2c: adv7343: add OF support

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]

Message-ID: <CA+V-a8tStFRbELAmZL=418VpR9SJgp8uo_4hrtny2UEMEoXakg@mail.gmail.com>
Date:	Mon, 26 Aug 2013 08:11:45 +0530
From:	Prabhakar Lad <prabhakar.csengg@...il.com>
To:	Sylwester Nawrocki <s.nawrocki@...sung.com>
Cc:	LMML <linux-media@...r.kernel.org>,
	DLOS <davinci-linux-open-source@...ux.davincidsp.com>,
	LKML <linux-kernel@...r.kernel.org>,
	device-tree <devicetree-discuss@...ts.ozlabs.org>,
	LDOC <linux-doc@...r.kernel.org>,
	Stephen Warren <swarren@...dotorg.org>,
	Mark Rutland <Mark.Rutland@....com>,
	Pawel Moll <Pawel.Moll@....com>,
	Kumar Gala <galak@...eaurora.org>,
	Rob Herring <rob.herring@...xeda.com>
Subject: Re: [PATCH v3 2/2] media: i2c: adv7343: add OF support

Hi Sylwester,

On Fri, Aug 23, 2013 at 11:33 PM, Sylwester Nawrocki
<s.nawrocki@...sung.com> wrote:
> Cc: DT binding maintainers
>
> On 07/20/2013 08:21 AM, Lad, Prabhakar wrote:
>> From: "Lad, Prabhakar" <prabhakar.csengg@...il.com>
>>
>> add OF support for the adv7343 driver.
>>
>> Signed-off-by: Lad, Prabhakar <prabhakar.csengg@...il.com>
>> ---
> [...]
>>  .../devicetree/bindings/media/i2c/adv7343.txt      |   48 ++++++++++++++++++++
>>  drivers/media/i2c/adv7343.c                        |   46 ++++++++++++++++++-
>>  2 files changed, 93 insertions(+), 1 deletion(-)
>>  create mode 100644 Documentation/devicetree/bindings/media/i2c/adv7343.txt
>>
>> diff --git a/Documentation/devicetree/bindings/media/i2c/adv7343.txt b/Documentation/devicetree/bindings/media/i2c/adv7343.txt
>> new file mode 100644
>> index 0000000..5653bc2
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/i2c/adv7343.txt
>> @@ -0,0 +1,48 @@
>> +* Analog Devices adv7343 video encoder
>> +
>> +The ADV7343 are high speed, digital-to-analog video encoders in a 64-lead LQFP
>> +package. Six high speed, 3.3 V, 11-bit video DACs provide support for composite
>> +(CVBS), S-Video (Y-C), and component (YPrPb/RGB) analog outputs in standard
>> +definition (SD), enhanced definition (ED), or high definition (HD) video
>> +formats.
>> +
>> +Required Properties :
>> +- compatible: Must be "adi,adv7343"
>> +
>> +Optional Properties :
>> +- adi,power-mode-sleep-mode: on enable the current consumption is reduced to
>> +                           micro ampere level. All DACs and the internal PLL
>> +                           circuit are disabled.
>
> Sorry for getting back so late to this. I realize this is already queued in
> the media tree. But this binding doesn't look good enough to me. I think it
> will need to be corrected during upcoming -rc period.
>
Thanks for the catch :-)

> It might be hard to figure out only from the chip's datasheet what
> adi,power-mode-sleep-mode really refers to. AFAICS it is for assigning some
> value to a specific register. If we really need to specify register values
> in the device tree then it would probably make sense to describe to which
> register this apply. Now the name looks like derived from some structure
> member name in the Linux driver of the device.
>
the property is derived from the datasheet itself for example the
'adi,power-mode-sleep-mode' --> Register 0x0 power mode bit 0
'adi,power-mode-pll-ctrl' ---> Register 0x0 power mode bit 1
'adi,dac-enable' ----> Register 0x0 power mode bit 2-7
'adi,sd-dac-enable' ---> Register 0x82 SD mode register bit 1-2

[1] http://www.analog.com/static/imported-files/data_sheets/ADV7342_7343.pdf

>> +- adi,power-mode-pll-ctrl: PLL and oversampling control. This control allows
>> +                        internal PLL 1 circuit to be powered down and the
>> +                        oversampling to be switched off.
>
> Similar comments applies to this property.
>
>> +- ad,adv7343-power-mode-dac: array configuring the power on/off DAC's 1..6,
>> +                           0 = OFF and 1 = ON, Default value when this
>> +                           property is not specified is <0 0 0 0 0 0>.
>
> Name of the property is incorrect here. It has changed to "adi,dac-enable".
>
OK

>> +- ad,adv7343-sd-config-dac-out: array configure SD DAC Output's 1 and 2, 0 = OFF
>> +                              and 1 = ON, Default value when this property is
>> +                              not specified is <0 0>.
>
> Similarly, "adi,sd-dac-enable.
>
OK

Regards,
--Prabhakar Lad
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/