linux-kernel - Re: [PATCH] Device tree binding for Avago APDS990X light sensor

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:   Wed, 27 Dec 2017 23:15:48 +0200
From:   Sakari Ailus <sakari.ailus@...ux.intel.com>
To:     Filip Matijević <filip.matijevic.pz@...il.com>
Cc:     Pavel Machek <pavel@....cz>, robh+dt@...nel.org,
        mark.rutland@....com, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, pali.rohar@...il.com, sre@...nel.org,
        linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
        linux-omap@...r.kernel.org, tony@...mide.com, khilman@...nel.org,
        aaro.koskinen@....fi, ivo.g.dimitrov.75@...il.com,
        patrikbachan@...il.com, serge@...lyn.com, abcloriens@...il.com,
        clayton@...ftyguy.net, martijn@...xit.nl
Subject: Re: [PATCH] Device tree binding for Avago APDS990X light sensor

On Wed, Dec 27, 2017 at 07:50:42PM +0100, Filip Matijević wrote:
> Hi Sakari,
> 
> and thank you for your input - I've added a few comments below.
> 
> On 12/27/2017 07:00 PM, Sakari Ailus wrote:
> > Hi Pavel,
> > 
> > Thanks for the patch. Please see my comments below.
> > 
> > On Wed, Dec 27, 2017 at 10:18:28AM +0100, Pavel Machek wrote:
> >> From: Filip Matijević <filip.matijevic.pz@...il.com>
> >>
> >> This prepares binding for light sensor used in Nokia N9. 
> >>
> >> Signed-off-by: Filip Matijević <filip.matijevic.pz@...il.com>
> >> Signed-off-by: Pavel machek <pavel@....cz>
> >>
> >> ---
> >>
> >> Patches to convert APDS990X driver to device tree and to switch to iio
> >> are available.
> >>
> >> diff --git a/Documentation/devicetree/bindings/misc/avago-apds990x.txt b/Documentation/devicetree/bindings/misc/avago-apds990x.txt
> >> new file mode 100644
> >> index 0000000..e038146
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/misc/avago-apds990x.txt
> >> @@ -0,0 +1,39 @@
> >> +Avago APDS990X driver
> >> +
> >> +Required properties:
> >> +- compatible: "avago,apds990x"
> >> +- reg: address on the I2C bus
> >> +- interrupts: external interrupt line number
> >> +- Vdd-supply: power supply for VDD
> >> +- Vled-supply: power supply for LEDA
> > 
> > AFAIK the custom is to use lower case letters for regulator supplies.
> 
> I've just used the same notation as in current driver. Datasheet calls
> those VDD (with DD being in subscript) and LEDA. I see no problem in
> changing those to vdd-supply and vled-supply if no one else objects.
> 
> > 
> >> +- ga: Glass attenuation
> >> +- cf1: Clear channel factor 1
> >> +- irf1: IR channel factor 1
> >> +- cf2: Clear channel factor 2
> >> +- irf2: IR channel factor 2
> >> +- df: Device factor
> >> +- pdrive: IR current, one of APDS_IRLED_CURR_XXXmA values
> >> +- ppcount: Proximity pulse count
> > 
> > Are these device specific? If so, please add the vendor prefix to them.
> 
> AFAIK yes - same as before if no one else objects, these will be changed.
> 
> > 
> > I might not use short abbreviations such as "df" either. I wonder what
> > others think.
> 
> These are also come from current driver - some of these comes from the
> datasheet itself, but not all. For example "ga" and "df" are in there
> (so I I would leave those alone), but "irf1" is called "B", "cf2" is
> called "C" and "irf2" is called "D" (I guess the missing "cf1" should be
> "A", but there is no such thing in the datasheet).
> IMHO using some other names might just add to the confusion.

Ack, datasheet names are fine.

You could also use a single property with all device specific coefficients
in a pre-defined order.

I don't have a strong opinion either way.

-- 
Regards,

Sakari Ailus
sakari.ailus@...ux.intel.com