Re: [PATCH] leds: lm3697: Fix out-of-bound access

[Pavel Machek <pavel@ucw.cz>]

[-- Attachment #1: Type: text/plain, Size: 2256 bytes --]

Hi!

> > >> By the way I just realized that the DT binding in this driver seems
> > >> incorrect to me.
> > >>
> > >> The controller logically supports 3 LED strings, each having
> > >> configurable control bank.  
> > 
> > There are two control banks. You can connect the HVLED outputs to either 
> > control bank A or B there is no individual control of the LED strings.
> > 
> > 
> > >> But the DT binding supports 2 DT nodes, one for each control bank
> > >> (identified by the `reg` property) and then `led-sources` says which
> > >> string should be controlled by given bank.
> > >>
> > >> But taking in mind that DT should describe how devices are connected to
> > >> each other, I think the child nodes in the binding should instead
> > >> describe the 3 supported LED strings...  
> > 
> > The outputs in this case are virtual outputs which are the banks (A and B).
> > 
> > Since the device is bank controlled the actual current sinks are not 
> > defined thus making the the banks the actual outputs.
> > 
> > This is why the 'reg' property defines the control bank either A or B 
> > and the led-sources indicates the strings associated with the control bank.

> Dan, I looked at the datasheet, I understand this.
> 
> Nonetheless, device tree should describe how devices are connected to
> each other. The chip has 3 pins for 3 LED strings.

Well, device tree is not a device schematics...

> If this controller should be able to support 3 LED strings via 3
> outputs, the device tree binding nodes should, in my opinion, describe
> each pin connected string. The nodes should maybe even be called
> 'led-string@N' where N is from [0, 1, 2].
> 
> The fact that the device is bank controlled and there are only two
> banks (and it is configurable by which bank each LED string is
> controlled) is more relevant to the driver, not as much to device tree
> binding.

Seems to me like two independend LEDs, and I'd describe it as
such. The fact that it goes over 3 wires is just a implementation
detail. Lets keep it simple...

Best regards,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]