linux-kernel - Re: [PATCH/RFC v12 10/19] DT: Add documentation for the mfd Maxim max77693

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: <20150309123716.GE11954@valkosipuli.retiisi.org.uk>
Date:	Mon, 9 Mar 2015 14:37:17 +0200
From:	Sakari Ailus <sakari.ailus@....fi>
To:	Jacek Anaszewski <j.anaszewski@...sung.com>
Cc:	linux-leds@...r.kernel.org, linux-media@...r.kernel.org,
	linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
	kyungmin.park@...sung.com, pavel@....cz, cooloney@...il.com,
	rpurdie@...ys.net, s.nawrocki@...sung.com,
	Andrzej Hajda <a.hajda@...sung.com>,
	Lee Jones <lee.jones@...aro.org>,
	Chanwoo Choi <cw00.choi@...sung.com>,
	laurent.pinchart@...asonboard.com, hverkuil@...all.nl
Subject: Re: [PATCH/RFC v12 10/19] DT: Add documentation for the mfd Maxim
 max77693

Hi Jacek,

On Mon, Mar 09, 2015 at 01:19:32PM +0100, Jacek Anaszewski wrote:
> Hi Sakari,
> 
> Thanks for the review.
> 
> On 03/09/2015 11:54 AM, Sakari Ailus wrote:
> >Hi Jacek,
> >
> >On Wed, Mar 04, 2015 at 05:14:31PM +0100, Jacek Anaszewski wrote:
> >>This patch adds device tree binding documentation for
> >>the flash cell of the Maxim max77693 multifunctional device.
> >>
> >>Signed-off-by: Jacek Anaszewski <j.anaszewski@...sung.com>
> >>Signed-off-by: Andrzej Hajda <a.hajda@...sung.com>
> >>Acked-by: Kyungmin Park <kyungmin.park@...sung.com>
> >>Cc: Lee Jones <lee.jones@...aro.org>
> >>Cc: Chanwoo Choi <cw00.choi@...sung.com>
> >>Cc: Bryan Wu <cooloney@...il.com>
> >>Cc: Richard Purdie <rpurdie@...ys.net>
> >>---
> >>  Documentation/devicetree/bindings/mfd/max77693.txt |   61 ++++++++++++++++++++
> >>  1 file changed, 61 insertions(+)
> >>
> >>diff --git a/Documentation/devicetree/bindings/mfd/max77693.txt b/Documentation/devicetree/bindings/mfd/max77693.txt
> >>index 38e6440..ab8fbd5 100644
> >>--- a/Documentation/devicetree/bindings/mfd/max77693.txt
> >>+++ b/Documentation/devicetree/bindings/mfd/max77693.txt
> >>@@ -76,7 +76,53 @@ Optional properties:
> >>      Valid values: 4300000, 4700000, 4800000, 4900000
> >>      Default: 4300000
> >>
> >>+- led : the LED submodule device node
> >>+
> >>+There are two LED outputs available - FLED1 and FLED2. Each of them can
> >>+control a separate LED or they can be connected together to double
> >>+the maximum current for a single connected LED. One LED is represented
> >>+by one child node.
> >>+
> >>+Required properties:
> >>+- compatible : Must be "maxim,max77693-led".
> >>+
> >>+Optional properties:
> >>+- maxim,trigger-type : Flash trigger type.
> >>+	Possible trigger types:
> >>+		LEDS_TRIG_TYPE_EDGE (0) - Rising edge of the signal triggers
> >>+			the flash,
> >>+		LEDS_TRIG_TYPE_LEVEL (1) - Strobe pulse length controls duration
> >>+			of the flash.
> >>+- maxim,boost-mode :
> >>+	In boost mode the device can produce up to 1.2A of total current
> >>+	on both outputs. The maximum current on each output is reduced
> >>+	to 625mA then. If not enabled explicitly, boost setting defaults to
> >>+	LEDS_BOOST_FIXED in case both current sources are used.
> >>+	Possible values:
> >>+		LEDS_BOOST_OFF (0) - no boost,
> >>+		LEDS_BOOST_ADAPTIVE (1) - adaptive mode,
> >>+		LEDS_BOOST_FIXED (2) - fixed mode.
> >>+- maxim,boost-mvout : Output voltage of the boost module in millivolts.
> >>+- maxim,mvsys-min : Low input voltage level in millivolts. Flash is not fired
> >>+	if chip estimates that system voltage could drop below this level due
> >>+	to flash power consumption.
> >>+
> >>+Required properties of the LED child node:
> >>+- label : see Documentation/devicetree/bindings/leds/common.txt
> >
> >According to ePAPR, label is "a human readable string describing a device".
> >There's no requirement that this would be unique, for instance. If you have
> >a camera flash LED, there's necessarily no meaningful label for it, as it
> >doesn't really tell the user anything (vs. HDD activity LED, for instance).
> >
> >I think I'd make this optional.
> 
> OK.
> 
> >What comes to entity naming in Media controller, the label isn't enough. As
> >we haven't yet fully agreed on how to name the entities in the future, I'd
> >propose sticking to current practices: chip name (and optional numerical LED
> >ID) followed by the I2C address. The name should be specified by the driver.
> >
> >Do you have other than I2C busses required by the current drivers?
> 
> I have AAT1290 device driven through GPIOs. There was also other driver,
> for a similar device, submitted few days ago to linux-leds list.

The problem indeed is defining a stable and unique identifier for a device
in a system. In context of your patchset, I think this mostly matters in the
V4L2 flash API wrapper patch.

GPIO controlled devices are little bit more troublesome, as GPIO numbers
alone aren't necessarily stable, but depend on the probing order. Well, i2c
controllers could also be registered dynamically. The same goes for PCI
devices, too, for instance.

Most i2c adapters have a static id, and PCI devices have a stable bus
address (unless system configuration is modified by e.g. adding or removing
OTHER devices).

I wonder if this could be resolved on OF-based systems by adding a string
property, say, device name, whenever where are more than one device of a
kind in the system. The string could just contain a numeric value, say 0 or
1.

Cc Hans and Laurent.

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus@....fi	XMPP: sailus@...iisi.org.uk
--
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/