linux-kernel - Re: [PATCH] backlight: arcxcnn: devicetree bindings for ArticSand devices

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>] [day] [month] [year] [list]

Message-ID: <20160912140015.GF9789@dell>
Date:   Mon, 12 Sep 2016 15:00:15 +0100
From:   Lee Jones <lee.jones@...aro.org>
To:     Olimpiu Dejeu <olimpiu@...ticsand.com>
Cc:     Rob Herring <robh@...nel.org>, linux-kernel@...r.kernel.org,
        linux-fbdev@...r.kernel.org, devicetree@...r.kernel.org,
        jg1.han@...sung.com
Subject: Re: [PATCH] backlight: arcxcnn: devicetree bindings for ArticSand
 devices

On Fri, 09 Sep 2016, Olimpiu Dejeu wrote:

> On Tue, Aug 23, 2016 at 2:20 PM, Rob Herring <robh@...nel.org> wrote:
> 
> > On Mon, Aug 22, 2016 at 03:11:24PM -0400, Olimpiu Dejeu wrote:
> > > This is the device tree bindings documentation file
> > >
> > > Signed-off-by: Olimpiu Dejeu <olimpiu@...ticsand.com>
> > >
> > > ---
> > >  .../bindings/video/backlight/arcxcnn.txt           | 29
> > ++++++++++++++++++++++
> >
> >
> Check your directory location. Things have moved.
> >
> > Not sure what you mean. This seems to be the right place. Please advise.

What kernel version are you on?

Documentation/devicetree/bindings/leds/backlight

> > >  1 file changed, 29 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/video/backlight/
> > arcxcnn.txt
> > >
> > > diff --git a/Documentation/devicetree/bindings/video/backlight/arcxcnn.txt
> > b/Documentation/devicetree/bindings/video/backlight/arcxcnn.txt
> > > new file mode 100644
> > > index 0000000..9cd7315
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/video/backlight/arcxcnn.txt
> > > @@ -0,0 +1,29 @@
> > > +arcxcnn bindings
> > > +
> > > +Required properties:
> > > +  - compatible: "arc,arcxcnn", "arc,arc2c0608"
> >
> > One per line please.
> >
> > Is arcxcnn a specific chip? If not drop it. No wildcards in compatible
> > strings.
> >
> > Wild-cards can be dropped but there could be a large family of similar
> chips that would be supported with this one driver. How would one add
> support for a range of chips, some of which don't exist yet?

If they do not exist, then do not mention them in this document.

Every other chip needs to be named individually.

[...]

> > > +  - pwm-period: PWM period value. Set only PWM input mode used (u32)
> > > +  - prg-addr: Register address of ROM area to be updated (u32) low 8
> > bits used
> > > +  - prg-val: Register value to be updated (u32) low 8 bits used
> >
> > What is this for? This should be a specific property or properties to do
> > explicit things, not a generic fill registers/rom with magic values.
> >
> > We need to expose some registers for test without exposing underlying
> functionality. Do you suggest not documenting this in the bindings yet
> keeping the code in the driver? We followed the pattern from the lp855x_bl
> driver.

If you are supplying addresses, use the reg property.

> > > +
> > > +Example:
> > > +
> > > +     /* ARC2C0608 */
> > > +     backlight@30 {
> > > +             compatible = "arc,arc2c0608";
> > > +             reg = <0x30>;
> > > +
> > > +             init-brt = <123>;
> > > +
> > > +             /* LED0+1 string enabled */
> > > +             prg_06h {
> > > +                     prg-addr = <0x06>;
> > > +                     prg-val = <0x83>;
> > > +             };
> > > +
> > > +     };
> > >
> >

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog