* [PATCH 1/3] devicetree: mfd: Add binding for the TI LM3533
@ 2015-10-25 18:09 Bjorn Andersson
2015-10-25 18:09 ` [PATCH 2/3] iio: light: lm3533-als: Print error message on invalid resistance Bjorn Andersson
` (3 more replies)
0 siblings, 4 replies; 20+ messages in thread
From: Bjorn Andersson @ 2015-10-25 18:09 UTC (permalink / raw)
To: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
Johan Hovold, Kumar Gala
Cc: devicetree, linux-iio, linux-kernel, linux-arm-kernel
Add the binding for the Texas Instruments LM3533 lighting power
solution.
Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
---
Documentation/devicetree/bindings/mfd/lm3533.txt | 183 +++++++++++++++++++++++
1 file changed, 183 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mfd/lm3533.txt
diff --git a/Documentation/devicetree/bindings/mfd/lm3533.txt b/Documentation/devicetree/bindings/mfd/lm3533.txt
new file mode 100644
index 000000000000..3cc41695a544
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/lm3533.txt
@@ -0,0 +1,183 @@
+Texas Instruments LM3533 binding
+
+This binding describes the Texas Instruments LM3533, a lighting power solution
+for smartphone handsets. The common properties are described directly in the
+node, while each individual component are described in an optional subnode.
+
+- compatible:
+ Usage: required
+ Value type: <stringlist>
+ Definition: must be:
+ "ti,lm3533"
+
+- reg:
+ Usage: required
+ Value type: <u32>
+ Definition: i2c address of the LM3533 chip
+
+- ti,hwen-gpios:
+ Usage: required
+ Value type: <prop-encoded-array>
+ Definition: reference to gpio pin connected to the HWEN input; as
+ specified in "gpio/gpio.txt"
+
+- ti,als-supply:
+ Usage: optional
+ Value type: <prop-encoded-array>
+ Definition: reference to regulator powering the V_als input; as
+ specified in "regulator/regulator.txt"
+
+- ti,boost-freq:
+ Usage: required
+ Value type: <u32>
+ Definition: switch-frequency of the boost converter, must be either:
+ 500000 or 1000000
+
+- ti,boost-ovp:
+ Usage: required
+ Value type: <u32>
+ Definition: over voltage protection limit, must be one of: 16, 24, 32
+ or 40
+
+- #address-cells:
+ Usage: required
+ Value type: <u32>
+ Definition: must be 1
+
+- #size-cells:
+ Usage: required
+ Value type: <u32>
+ Definition: must be 0
+
+= ALS SUBNODE
+The als subnode must be named "als", it carries the als related properties.
+
+- ti,als-resistance:
+ Usage: required (unless ti,pwm-mode is specified)
+ Value type: <u32>
+ Definition: specifies the resistor value (R_als), in Ohm. Valid values
+ ranges from 200kOhm to 1574Ohm.
+
+- ti,pwm-mode:
+ Usage: optional
+ Value type: <empty>
+ Definition: specifies, if present, that the als should operate in pwm
+ mode - rather than analog mode
+
+= BACKLIGHT NODES
+Backlight subnodes must be named "backlight", they carry the backlight related
+properties.
+
+- reg:
+ Usage: required
+ Value type: <u32>
+ Definition: specifies which of the two backlights this node corresponds
+ to
+
+- default-brightness:
+ Usage: optional
+ Value type: <32>
+ Definition: specifies the default brightness for the backlight, in
+ units of brightness [0-255]
+
+- label:
+ Usage: required
+ Value type: <string>
+ Definition: specifies a name of this backlight
+
+- led-max-microamp:
+ Usage: required
+ Value type: <u32>
+ Definition: specifies the max current for this backlight, in uA, as
+ described in "leds/common.txt"
+
+- ti,pwm-zones:
+ Usage: optional
+ Value type: <u32 list>
+ Definition: lists the ALS zones to be PWM controlled for this backlight,
+ the values in the list are in the range [0 - 4]
+
+= LED NODES
+LED subnodes must be named "led", they carry the LED related properties.
+
+- reg:
+ Usage: required
+ Value type: <u32>
+ Definition: specifies which of the four LEDs this node corresponds to
+
+- linux,default-trigger:
+ Usage: optional
+ Value type: <string>
+ Definition: specifies the default trigger for the LED, as described in
+ "leds/common.txt"
+
+- label:
+ Usage: required
+ Value type: <string>
+ Definition: specifies a name of this LED, as described in
+ "leds/common.txt"
+
+- led-max-microamp:
+ Usage: required
+ Value type: <u32>
+ Definition: specifies the max current for this LED, in uA, as described
+ in "leds/common.txt"
+
+- ti,pwm-zones:
+ Usage: optional
+ Value type: <u32 list>
+ Definition: lists the ALS zones to be PWM controlled for this LED, the
+ values in the list are in the range [0 - 4]
+
+= EXAMPLE
+
+i2c@12460000 {
+ compatible = "qcom,i2c-qup-v1.1.1";
+ ...
+
+ lm3533@36 {
+ compatible = "ti,lm3533";
+ reg = <0x36>;
+
+ ti,hwen-gpios = <&pm8921_gpio 26 GPIO_ACTIVE_HIGH>;
+ ti,als-supply = <&pm8921_l11>;
+
+ ti,boost-freq = <500000>;
+ ti,boost-ovp = <24>;
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ als {
+ ti,als-resistance = <200000>;
+ };
+
+ backlight@0 {
+ reg = <0>;
+ label = "backlight";
+
+ led-max-microamp = <20200>;
+ };
+
+ led@0 {
+ reg = <0>;
+ label = "red";
+
+ led-max-microamp = <5000>;
+ };
+
+ led@1 {
+ reg = <1>;
+ label = "green";
+
+ led-max-microamp = <5000>;
+ };
+
+ led@2 {
+ reg = <2>;
+ label = "blue";
+
+ led-max-microamp = <5000>;
+ };
+ };
+
--
2.4.2
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 2/3] iio: light: lm3533-als: Print error message on invalid resistance
2015-10-25 18:09 [PATCH 1/3] devicetree: mfd: Add binding for the TI LM3533 Bjorn Andersson
@ 2015-10-25 18:09 ` Bjorn Andersson
2015-10-27 19:18 ` Joe Perches
2015-10-25 18:09 ` [PATCH 3/3] mfd: lm3533: Support initialization from Device Tree Bjorn Andersson
` (2 subsequent siblings)
3 siblings, 1 reply; 20+ messages in thread
From: Bjorn Andersson @ 2015-10-25 18:09 UTC (permalink / raw)
To: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Johan Hovold, Peter Meerwald
Cc: linux-iio, linux-kernel, linux-arm-kernel
Print an error message to indicate that invalid configuration data was
provided in the platform_data, rather than just aborting initialization.
Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
---
This patch can be picked independently of the other 2, but included in the
series for "completeness".
drivers/iio/light/lm3533-als.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/iio/light/lm3533-als.c b/drivers/iio/light/lm3533-als.c
index 076bc46fad03..e56937c40a18 100644
--- a/drivers/iio/light/lm3533-als.c
+++ b/drivers/iio/light/lm3533-als.c
@@ -743,8 +743,10 @@ static int lm3533_als_set_resistor(struct lm3533_als *als, u8 val)
{
int ret;
- if (val < LM3533_ALS_RESISTOR_MIN || val > LM3533_ALS_RESISTOR_MAX)
+ if (val < LM3533_ALS_RESISTOR_MIN || val > LM3533_ALS_RESISTOR_MAX) {
+ dev_err(&als->pdev->dev, "invalid resistor value\n");
return -EINVAL;
+ };
ret = lm3533_write(als->lm3533, LM3533_REG_ALS_RESISTOR_SELECT, val);
if (ret) {
--
2.4.2
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 3/3] mfd: lm3533: Support initialization from Device Tree
2015-10-25 18:09 [PATCH 1/3] devicetree: mfd: Add binding for the TI LM3533 Bjorn Andersson
2015-10-25 18:09 ` [PATCH 2/3] iio: light: lm3533-als: Print error message on invalid resistance Bjorn Andersson
@ 2015-10-25 18:09 ` Bjorn Andersson
2015-10-27 7:57 ` [PATCH 1/3] devicetree: mfd: Add binding for the TI LM3533 Rob Herring
2015-10-27 16:29 ` [PATCH v2 " Bjorn Andersson
3 siblings, 0 replies; 20+ messages in thread
From: Bjorn Andersson @ 2015-10-25 18:09 UTC (permalink / raw)
To: Lee Jones, Johan Hovold; +Cc: linux-arm-kernel, linux-kernel
Implement Device Tree parsing functions to support initialization of the
lm3533 drivers.
Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
---
drivers/mfd/lm3533-core.c | 265 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 265 insertions(+)
diff --git a/drivers/mfd/lm3533-core.c b/drivers/mfd/lm3533-core.c
index 5abcbb2e8849..acdb3d580277 100644
--- a/drivers/mfd/lm3533-core.c
+++ b/drivers/mfd/lm3533-core.c
@@ -18,6 +18,7 @@
#include <linux/gpio.h>
#include <linux/i2c.h>
#include <linux/mfd/core.h>
+#include <linux/of_gpio.h>
#include <linux/regmap.h>
#include <linux/seq_file.h>
#include <linux/slab.h>
@@ -25,6 +26,8 @@
#include <linux/mfd/lm3533.h>
+#define LM3533_DEFAULT_BRIGHTNESS 200
#define LM3533_BOOST_OVP_MASK 0x06
#define LM3533_BOOST_OVP_SHIFT 1
@@ -588,10 +591,266 @@ static const struct regmap_config regmap_config = {
.precious_reg = lm3533_precious_register,
};
+static int lm3533_of_parse_enum(struct device_node *node,
+ const char *propname,
+ const unsigned *match, size_t num_matches)
+{
+ size_t i;
+ int ret;
+ u32 val;
+
+ ret = of_property_read_u32(node, propname, &val);
+ if (ret < 0)
+ return ret;
+
+ for (i = 0; i < num_matches; i++) {
+ if (val == match[i])
+ return i;
+ }
+
+ return -EINVAL;
+}
+
+static int lm3533_of_parse_pwm_zones(struct device_node *node)
+{
+ const char *propname = "ti,pwm-zones";
+ u32 zones[5];
+ int count;
+ int ret;
+ int i;
+
+ count = of_property_count_u32_elems(node, propname);
+ if (count == -EINVAL)
+ return 0;
+ if (count <= 0)
+ return count;
+ if (count >= ARRAY_SIZE(zones))
+ return -EINVAL;
+
+ ret = of_property_read_u32_array(node, propname, zones, count);
+ if (ret < 0)
+ return ret;
+
+ /* Enable pwm input, and enable the selected zones */
+ ret = BIT(0);
+ for (i = 0; i < count; i++)
+ ret |= BIT(zones[i] + 1);
+
+ return ret;
+}
+
+static int lm3533_of_parse_als_node(struct device *dev,
+ struct device_node *node,
+ struct lm3533_platform_data *pdata)
+{
+ struct lm3533_als_platform_data *als_pdata;
+ int ret;
+ u32 val;
+
+ als_pdata = devm_kzalloc(dev,
+ sizeof(struct lm3533_als_platform_data),
+ GFP_KERNEL);
+ if (!als_pdata)
+ return -ENOMEM;
+
+ als_pdata->pwm_mode = of_property_read_bool(node, "ti,pwm-mode");
+
+ ret = of_property_read_u32(node, "ti,als-resistance", &val);
+ if (ret < 0 && ret != -EINVAL) {
+ dev_err(dev, "unable to read ti,als-resistance");
+ return ret;
+ }
+
+ /* Leave at high-Z, if the property was omitted or specified as 0 */
+ if (!ret && val != 0)
+ als_pdata->r_select = 200000 / val;
+
+ pdata->als = als_pdata;
+
+ return 0;
+}
+
+static int lm3533_of_parse_bl_node(struct device *dev,
+ struct device_node *node,
+ struct lm3533_platform_data *pdata)
+{
+ struct lm3533_bl_platform_data *bl_pdata;
+ int ret;
+ u32 reg;
+ u32 val;
+
+ ret = of_property_read_u32(node, "reg", ®);
+ if (ret < 0 || reg >= ARRAY_SIZE(lm3533_bl_devs)) {
+ dev_err(dev, "invalid reg property\n");
+ return ret;
+ }
+
+ bl_pdata = &pdata->backlights[reg];
+
+ /* Increment num_backlights if we're filling a new space */
+ if (reg + 1 > pdata->num_backlights)
+ pdata->num_backlights = reg + 1;
+
+ ret = of_property_read_string(node, "label",
+ (const char **)&bl_pdata->name);
+ if (ret < 0) {
+ dev_err(dev, "unable to parse label\n");
+ return ret;
+ }
+
+ ret = of_property_read_u32(node, "led-max-microamp", &val);
+ if (ret < 0) {
+ dev_err(dev, "unable to parse led-max-microamp\n");
+ return ret;
+ }
+ bl_pdata->max_current = val;
+
+ val = LM3533_DEFAULT_BRIGHTNESS;
+ ret = of_property_read_u32(node, "default-brightness", &val);
+ if (ret < 0 && ret != -EINVAL) {
+ dev_err(dev, "unable to parse default-brightness\n");
+ return ret;
+ }
+ bl_pdata->default_brightness = val;
+
+ ret = lm3533_of_parse_pwm_zones(node);
+ if (ret < 0) {
+ dev_err(dev, "failed to parse ti,pwm-zones\n");
+ return ret;
+ }
+ bl_pdata->pwm = ret;
+
+ return 0;
+}
+
+static int lm3533_of_parse_led_node(struct device *dev,
+ struct device_node *node,
+ struct lm3533_platform_data *pdata)
+{
+ struct lm3533_led_platform_data *led_pdata;
+ int ret;
+ u32 reg;
+ u32 val;
+
+ ret = of_property_read_u32(node, "reg", ®);
+ if (ret < 0 || reg >= ARRAY_SIZE(lm3533_led_devs)) {
+ dev_err(dev, "invalid reg property\n");
+ return ret;
+ }
+
+ led_pdata = &pdata->leds[reg];
+
+ /* Increment num_leds if we're filling a new space */
+ if (reg + 1 > pdata->num_leds)
+ pdata->num_leds = reg + 1;
+
+ ret = of_property_read_string(node, "label",
+ (const char **)&led_pdata->name);
+ if (ret < 0) {
+ dev_err(dev, "unable to parse label\n");
+ return ret;
+ }
+
+ ret = of_property_read_u32(node, "led-max-microamp", &val);
+ if (ret < 0) {
+ dev_err(dev, "unable to parse led-max-microamp\n");
+ return ret;
+ }
+ led_pdata->max_current = val;
+
+ ret = of_property_read_string(node, "linux,default-trigger",
+ &led_pdata->default_trigger);
+ if (ret < 0 && ret != -EINVAL) {
+ dev_err(dev, "unable to parse default-trigger\n");
+ return ret;
+ }
+
+ ret = lm3533_of_parse_pwm_zones(node);
+ if (ret < 0) {
+ dev_err(dev, "failed to parse ti,pwm-zones\n");
+ return ret;
+ }
+ led_pdata->pwm = ret;
+
+ return 0;
+}
+
+static int lm3533_pdata_from_of_node(struct device *dev)
+{
+ struct lm3533_platform_data *pdata;
+ struct device_node *node;
+ int ret;
+ const unsigned freqs[] = {
+ [LM3533_BOOST_FREQ_500KHZ] = 500000,
+ [LM3533_BOOST_FREQ_1000KHZ] = 1000000,
+ };
+ const unsigned ovps[] = {
+ [LM3533_BOOST_OVP_16V] = 16,
+ [LM3533_BOOST_OVP_24V] = 24,
+ [LM3533_BOOST_OVP_32V] = 32,
+ [LM3533_BOOST_OVP_40V] = 40,
+ };
+
+ pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+ if (!pdata)
+ return -ENOMEM;
+
+ pdata->backlights = devm_kcalloc(dev,
+ ARRAY_SIZE(lm3533_bl_devs),
+ sizeof(struct lm3533_bl_platform_data),
+ GFP_KERNEL);
+ if (!pdata->backlights)
+ return -ENOMEM;
+
+ pdata->leds = devm_kcalloc(dev,
+ ARRAY_SIZE(lm3533_led_devs),
+ sizeof(struct lm3533_led_platform_data),
+ GFP_KERNEL);
+ if (!pdata->leds)
+ return -ENOMEM;
+
+ pdata->gpio_hwen = of_get_named_gpio(dev->of_node, "ti,hwen-gpios", 0);
+ if (pdata->gpio_hwen < 0)
+ return pdata->gpio_hwen;
+
+ ret = lm3533_of_parse_enum(dev->of_node, "ti,boost-freq", freqs, 2);
+ if (ret < 0) {
+ dev_err(dev, "failed to parse ti,boost-freq\n");
+ return ret;
+ }
+ pdata->boost_freq = ret;
+
+ ret = lm3533_of_parse_enum(dev->of_node, "ti,boost-ovp", ovps, 4);
+ if (ret < 0) {
+ dev_err(dev, "failed to parse ti,boost-ovp\n");
+ return ret;
+ }
+ pdata->boost_ovp = ret;
+
+ for_each_child_of_node(dev->of_node, node) {
+ if (!of_node_cmp(node->name, "als"))
+ ret = lm3533_of_parse_als_node(dev, node, pdata);
+ else if (!of_node_cmp(node->name, "backlight"))
+ ret = lm3533_of_parse_bl_node(dev, node, pdata);
+ else if (!of_node_cmp(node->name, "led"))
+ ret = lm3533_of_parse_led_node(dev, node, pdata);
+
+ if (ret < 0) {
+ of_node_put(node);
+ return ret;
+ }
+ }
+
+ dev->platform_data = pdata;
+
+ return 0;
+}
+
static int lm3533_i2c_probe(struct i2c_client *i2c,
const struct i2c_device_id *id)
{
struct lm3533 *lm3533;
+ int ret;
dev_dbg(&i2c->dev, "%s\n", __func__);
@@ -608,6 +867,12 @@ static int lm3533_i2c_probe(struct i2c_client *i2c,
lm3533->dev = &i2c->dev;
lm3533->irq = i2c->irq;
+ if (i2c->dev.of_node) {
+ ret = lm3533_pdata_from_of_node(lm3533->dev);
+ if (ret < 0)
+ return ret;
+ }
+
return lm3533_device_init(lm3533);
}
--
2.4.2
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 1/3] devicetree: mfd: Add binding for the TI LM3533
2015-10-25 18:09 [PATCH 1/3] devicetree: mfd: Add binding for the TI LM3533 Bjorn Andersson
2015-10-25 18:09 ` [PATCH 2/3] iio: light: lm3533-als: Print error message on invalid resistance Bjorn Andersson
2015-10-25 18:09 ` [PATCH 3/3] mfd: lm3533: Support initialization from Device Tree Bjorn Andersson
@ 2015-10-27 7:57 ` Rob Herring
2015-10-27 16:29 ` [PATCH v2 " Bjorn Andersson
3 siblings, 0 replies; 20+ messages in thread
From: Rob Herring @ 2015-10-27 7:57 UTC (permalink / raw)
To: Bjorn Andersson
Cc: Pawel Moll, Mark Rutland, Ian Campbell, Johan Hovold, Kumar Gala,
devicetree, linux-iio, linux-kernel, linux-arm-kernel
On Sun, Oct 25, 2015 at 1:09 PM, Bjorn Andersson
<bjorn.andersson@sonymobile.com> wrote:
> Add the binding for the Texas Instruments LM3533 lighting power
> solution.
>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
> ---
> Documentation/devicetree/bindings/mfd/lm3533.txt | 183 +++++++++++++++++++++++
> 1 file changed, 183 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mfd/lm3533.txt
>
> diff --git a/Documentation/devicetree/bindings/mfd/lm3533.txt b/Documentation/devicetree/bindings/mfd/lm3533.txt
> new file mode 100644
> index 000000000000..3cc41695a544
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/lm3533.txt
> @@ -0,0 +1,183 @@
> +Texas Instruments LM3533 binding
> +
> +This binding describes the Texas Instruments LM3533, a lighting power solution
> +for smartphone handsets. The common properties are described directly in the
> +node, while each individual component are described in an optional subnode.
> +
> +- compatible:
> + Usage: required
> + Value type: <stringlist>
> + Definition: must be:
> + "ti,lm3533"
> +
> +- reg:
> + Usage: required
> + Value type: <u32>
> + Definition: i2c address of the LM3533 chip
> +
> +- ti,hwen-gpios:
> + Usage: required
> + Value type: <prop-encoded-array>
> + Definition: reference to gpio pin connected to the HWEN input; as
> + specified in "gpio/gpio.txt"
> +
> +- ti,als-supply:
> + Usage: optional
> + Value type: <prop-encoded-array>
> + Definition: reference to regulator powering the V_als input; as
> + specified in "regulator/regulator.txt"
> +
> +- ti,boost-freq:
Append "-hz".
> + Usage: required
> + Value type: <u32>
> + Definition: switch-frequency of the boost converter, must be either:
> + 500000 or 1000000
> +
> +- ti,boost-ovp:
> + Usage: required
> + Value type: <u32>
> + Definition: over voltage protection limit, must be one of: 16, 24, 32
> + or 40
> +
> +- #address-cells:
> + Usage: required
> + Value type: <u32>
> + Definition: must be 1
> +
> +- #size-cells:
> + Usage: required
> + Value type: <u32>
> + Definition: must be 0
> +
> += ALS SUBNODE
> +The als subnode must be named "als", it carries the als related properties.
> +
> +- ti,als-resistance:
> + Usage: required (unless ti,pwm-mode is specified)
> + Value type: <u32>
> + Definition: specifies the resistor value (R_als), in Ohm. Valid values
> + ranges from 200kOhm to 1574Ohm.
So append "-kohm"?
> +
> +- ti,pwm-mode:
> + Usage: optional
> + Value type: <empty>
> + Definition: specifies, if present, that the als should operate in pwm
> + mode - rather than analog mode
> +
> += BACKLIGHT NODES
> +Backlight subnodes must be named "backlight", they carry the backlight related
> +properties.
> +
> +- reg:
> + Usage: required
> + Value type: <u32>
> + Definition: specifies which of the two backlights this node corresponds
> + to
> +
> +- default-brightness:
> + Usage: optional
> + Value type: <32>
> + Definition: specifies the default brightness for the backlight, in
> + units of brightness [0-255]
> +
> +- label:
> + Usage: required
> + Value type: <string>
> + Definition: specifies a name of this backlight
> +
> +- led-max-microamp:
> + Usage: required
> + Value type: <u32>
> + Definition: specifies the max current for this backlight, in uA, as
> + described in "leds/common.txt"
> +
> +- ti,pwm-zones:
> + Usage: optional
> + Value type: <u32 list>
> + Definition: lists the ALS zones to be PWM controlled for this backlight,
> + the values in the list are in the range [0 - 4]
> +
> += LED NODES
> +LED subnodes must be named "led", they carry the LED related properties.
> +
> +- reg:
> + Usage: required
> + Value type: <u32>
> + Definition: specifies which of the four LEDs this node corresponds to
> +
> +- linux,default-trigger:
> + Usage: optional
> + Value type: <string>
> + Definition: specifies the default trigger for the LED, as described in
> + "leds/common.txt"
> +
> +- label:
> + Usage: required
> + Value type: <string>
> + Definition: specifies a name of this LED, as described in
> + "leds/common.txt"
> +
> +- led-max-microamp:
> + Usage: required
> + Value type: <u32>
> + Definition: specifies the max current for this LED, in uA, as described
> + in "leds/common.txt"
> +
> +- ti,pwm-zones:
> + Usage: optional
> + Value type: <u32 list>
> + Definition: lists the ALS zones to be PWM controlled for this LED, the
> + values in the list are in the range [0 - 4]
> +
> += EXAMPLE
> +
> +i2c@12460000 {
> + compatible = "qcom,i2c-qup-v1.1.1";
> + ...
> +
> + lm3533@36 {
> + compatible = "ti,lm3533";
> + reg = <0x36>;
> +
> + ti,hwen-gpios = <&pm8921_gpio 26 GPIO_ACTIVE_HIGH>;
> + ti,als-supply = <&pm8921_l11>;
> +
> + ti,boost-freq = <500000>;
> + ti,boost-ovp = <24>;
> +
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + als {
> + ti,als-resistance = <200000>;
> + };
> +
> + backlight@0 {
> + reg = <0>;
> + label = "backlight";
> +
> + led-max-microamp = <20200>;
> + };
> +
> + led@0 {
> + reg = <0>;
> + label = "red";
> +
> + led-max-microamp = <5000>;
> + };
> +
> + led@1 {
> + reg = <1>;
> + label = "green";
> +
> + led-max-microamp = <5000>;
> + };
> +
> + led@2 {
> + reg = <2>;
> + label = "blue";
> +
> + led-max-microamp = <5000>;
> + };
> + };
> +
> --
> 2.4.2
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 1/3] devicetree: mfd: Add binding for the TI LM3533
2015-10-25 18:09 [PATCH 1/3] devicetree: mfd: Add binding for the TI LM3533 Bjorn Andersson
` (2 preceding siblings ...)
2015-10-27 7:57 ` [PATCH 1/3] devicetree: mfd: Add binding for the TI LM3533 Rob Herring
@ 2015-10-27 16:29 ` Bjorn Andersson
2015-10-27 16:29 ` [PATCH v2 2/3] iio: light: lm3533-als: Print error message on invalid resistance Bjorn Andersson
` (3 more replies)
3 siblings, 4 replies; 20+ messages in thread
From: Bjorn Andersson @ 2015-10-27 16:29 UTC (permalink / raw)
To: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
Johan Hovold, Kumar Gala
Cc: devicetree, linux-iio, linux-kernel, linux-arm-kernel
Add the binding for the Texas Instruments LM3533 lighting power
solution.
Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
---
Changes since v1:
- Added unit to boost-freq and als-resistance (as the frequency now comes with
a unit specifier I changed it to be expressed in kHz)
Documentation/devicetree/bindings/mfd/lm3533.txt | 183 +++++++++++++++++++++++
1 file changed, 183 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mfd/lm3533.txt
diff --git a/Documentation/devicetree/bindings/mfd/lm3533.txt b/Documentation/devicetree/bindings/mfd/lm3533.txt
new file mode 100644
index 000000000000..399df39ec7e7
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/lm3533.txt
@@ -0,0 +1,183 @@
+Texas Instruments LM3533 binding
+
+This binding describes the Texas Instruments LM3533, a lighting power solution
+for smartphone handsets. The common properties are described directly in the
+node, while each individual component are described in an optional subnode.
+
+- compatible:
+ Usage: required
+ Value type: <stringlist>
+ Definition: must be:
+ "ti,lm3533"
+
+- reg:
+ Usage: required
+ Value type: <u32>
+ Definition: i2c address of the LM3533 chip
+
+- ti,hwen-gpios:
+ Usage: required
+ Value type: <prop-encoded-array>
+ Definition: reference to gpio pin connected to the HWEN input; as
+ specified in "gpio/gpio.txt"
+
+- ti,als-supply:
+ Usage: optional
+ Value type: <prop-encoded-array>
+ Definition: reference to regulator powering the V_als input; as
+ specified in "regulator/regulator.txt"
+
+- ti,boost-freq-khz:
+ Usage: required
+ Value type: <u32>
+ Definition: switch-frequency of the boost converter, must be either:
+ 500 or 1000
+
+- ti,boost-ovp:
+ Usage: required
+ Value type: <u32>
+ Definition: over voltage protection limit, must be one of: 16, 24, 32
+ or 40
+
+- #address-cells:
+ Usage: required
+ Value type: <u32>
+ Definition: must be 1
+
+- #size-cells:
+ Usage: required
+ Value type: <u32>
+ Definition: must be 0
+
+= ALS SUBNODE
+The als subnode must be named "als", it carries the als related properties.
+
+- ti,als-resistance-ohm:
+ Usage: required (unless ti,pwm-mode is specified)
+ Value type: <u32>
+ Definition: specifies the resistor value (R_als), in Ohm. Valid values
+ ranges from 200kOhm to 1574Ohm.
+
+- ti,pwm-mode:
+ Usage: optional
+ Value type: <empty>
+ Definition: specifies, if present, that the als should operate in pwm
+ mode - rather than analog mode
+
+= BACKLIGHT NODES
+Backlight subnodes must be named "backlight", they carry the backlight related
+properties.
+
+- reg:
+ Usage: required
+ Value type: <u32>
+ Definition: specifies which of the two backlights this node corresponds
+ to
+
+- default-brightness:
+ Usage: optional
+ Value type: <32>
+ Definition: specifies the default brightness for the backlight, in
+ units of brightness [0-255]
+
+- label:
+ Usage: required
+ Value type: <string>
+ Definition: specifies a name of this backlight
+
+- led-max-microamp:
+ Usage: required
+ Value type: <u32>
+ Definition: specifies the max current for this backlight, in uA, as
+ described in "leds/common.txt"
+
+- ti,pwm-zones:
+ Usage: optional
+ Value type: <u32 list>
+ Definition: lists the ALS zones to be PWM controlled for this backlight,
+ the values in the list are in the range [0 - 4]
+
+= LED NODES
+LED subnodes must be named "led", they carry the LED related properties.
+
+- reg:
+ Usage: required
+ Value type: <u32>
+ Definition: specifies which of the four LEDs this node corresponds to
+
+- linux,default-trigger:
+ Usage: optional
+ Value type: <string>
+ Definition: specifies the default trigger for the LED, as described in
+ "leds/common.txt"
+
+- label:
+ Usage: required
+ Value type: <string>
+ Definition: specifies a name of this LED, as described in
+ "leds/common.txt"
+
+- led-max-microamp:
+ Usage: required
+ Value type: <u32>
+ Definition: specifies the max current for this LED, in uA, as described
+ in "leds/common.txt"
+
+- ti,pwm-zones:
+ Usage: optional
+ Value type: <u32 list>
+ Definition: lists the ALS zones to be PWM controlled for this LED, the
+ values in the list are in the range [0 - 4]
+
+= EXAMPLE
+
+i2c@12460000 {
+ compatible = "qcom,i2c-qup-v1.1.1";
+ ...
+
+ lm3533@36 {
+ compatible = "ti,lm3533";
+ reg = <0x36>;
+
+ ti,hwen-gpios = <&pm8921_gpio 26 GPIO_ACTIVE_HIGH>;
+ ti,als-supply = <&pm8921_l11>;
+
+ ti,boost-freq = <500000>;
+ ti,boost-ovp = <24>;
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ als {
+ ti,als-resistance = <200000>;
+ };
+
+ backlight@0 {
+ reg = <0>;
+ label = "backlight";
+
+ led-max-microamp = <20200>;
+ };
+
+ led@0 {
+ reg = <0>;
+ label = "red";
+
+ led-max-microamp = <5000>;
+ };
+
+ led@1 {
+ reg = <1>;
+ label = "green";
+
+ led-max-microamp = <5000>;
+ };
+
+ led@2 {
+ reg = <2>;
+ label = "blue";
+
+ led-max-microamp = <5000>;
+ };
+ };
+
--
2.4.2
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 2/3] iio: light: lm3533-als: Print error message on invalid resistance
2015-10-27 16:29 ` [PATCH v2 " Bjorn Andersson
@ 2015-10-27 16:29 ` Bjorn Andersson
2015-10-27 16:29 ` [PATCH v2 3/3] mfd: lm3533: Support initialization from Device Tree Bjorn Andersson
` (2 subsequent siblings)
3 siblings, 0 replies; 20+ messages in thread
From: Bjorn Andersson @ 2015-10-27 16:29 UTC (permalink / raw)
To: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Johan Hovold, Peter Meerwald
Cc: linux-iio, linux-kernel, linux-arm-kernel
Print an error message to indicate that invalid configuration data was
provided in the platform_data, rather than just aborting initialization.
Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
---
This patch can be picked independently of the other 2, but included in the
series for "completeness".
No changes since v1 in this patch.
drivers/iio/light/lm3533-als.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/iio/light/lm3533-als.c b/drivers/iio/light/lm3533-als.c
index 076bc46fad03..e56937c40a18 100644
--- a/drivers/iio/light/lm3533-als.c
+++ b/drivers/iio/light/lm3533-als.c
@@ -743,8 +743,10 @@ static int lm3533_als_set_resistor(struct lm3533_als *als, u8 val)
{
int ret;
- if (val < LM3533_ALS_RESISTOR_MIN || val > LM3533_ALS_RESISTOR_MAX)
+ if (val < LM3533_ALS_RESISTOR_MIN || val > LM3533_ALS_RESISTOR_MAX) {
+ dev_err(&als->pdev->dev, "invalid resistor value\n");
return -EINVAL;
+ };
ret = lm3533_write(als->lm3533, LM3533_REG_ALS_RESISTOR_SELECT, val);
if (ret) {
--
2.4.2
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 3/3] mfd: lm3533: Support initialization from Device Tree
2015-10-27 16:29 ` [PATCH v2 " Bjorn Andersson
2015-10-27 16:29 ` [PATCH v2 2/3] iio: light: lm3533-als: Print error message on invalid resistance Bjorn Andersson
@ 2015-10-27 16:29 ` Bjorn Andersson
2015-10-27 16:43 ` kbuild test robot
` (2 more replies)
2015-10-27 19:21 ` [PATCH v2 1/3] devicetree: mfd: Add binding for the TI LM3533 Rob Herring
2015-10-30 18:42 ` Lee Jones
3 siblings, 3 replies; 20+ messages in thread
From: Bjorn Andersson @ 2015-10-27 16:29 UTC (permalink / raw)
To: Lee Jones, Johan Hovold; +Cc: linux-arm-kernel, linux-kernel
Implement Device Tree parsing functions to support initialization of the
lm3533 drivers.
Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
---
Changes since v1:
- Added units to bost-freq and als resistance
drivers/mfd/lm3533-core.c | 264 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 264 insertions(+)
diff --git a/drivers/mfd/lm3533-core.c b/drivers/mfd/lm3533-core.c
index 5abcbb2e8849..9f0fb362ee76 100644
--- a/drivers/mfd/lm3533-core.c
+++ b/drivers/mfd/lm3533-core.c
@@ -18,6 +18,7 @@
#include <linux/gpio.h>
#include <linux/i2c.h>
#include <linux/mfd/core.h>
+#include <linux/of_gpio.h>
#include <linux/regmap.h>
#include <linux/seq_file.h>
#include <linux/slab.h>
@@ -25,6 +26,7 @@
#include <linux/mfd/lm3533.h>
+#define LM3533_DEFAULT_BRIGHTNESS 200
#define LM3533_BOOST_OVP_MASK 0x06
#define LM3533_BOOST_OVP_SHIFT 1
@@ -588,10 +590,266 @@ static const struct regmap_config regmap_config = {
.precious_reg = lm3533_precious_register,
};
+static int lm3533_of_parse_enum(struct device_node *node,
+ const char *propname,
+ const unsigned *match, size_t num_matches)
+{
+ size_t i;
+ int ret;
+ u32 val;
+
+ ret = of_property_read_u32(node, propname, &val);
+ if (ret < 0)
+ return ret;
+
+ for (i = 0; i < num_matches; i++) {
+ if (val == match[i])
+ return i;
+ }
+
+ return -EINVAL;
+}
+
+static int lm3533_of_parse_pwm_zones(struct device_node *node)
+{
+ const char *propname = "ti,pwm-zones";
+ u32 zones[5];
+ int count;
+ int ret;
+ int i;
+
+ count = of_property_count_u32_elems(node, propname);
+ if (count == -EINVAL)
+ return 0;
+ if (count <= 0)
+ return count;
+ if (count >= ARRAY_SIZE(zones))
+ return -EINVAL;
+
+ ret = of_property_read_u32_array(node, propname, zones, count);
+ if (ret < 0)
+ return ret;
+
+ /* Enable pwm input, and enable the selected zones */
+ ret = BIT(0);
+ for (i = 0; i < count; i++)
+ ret |= BIT(zones[i] + 1);
+
+ return ret;
+}
+
+static int lm3533_of_parse_als_node(struct device *dev,
+ struct device_node *node,
+ struct lm3533_platform_data *pdata)
+{
+ struct lm3533_als_platform_data *als_pdata;
+ int ret;
+ u32 val;
+
+ als_pdata = devm_kzalloc(dev,
+ sizeof(struct lm3533_als_platform_data),
+ GFP_KERNEL);
+ if (!als_pdata)
+ return -ENOMEM;
+
+ als_pdata->pwm_mode = of_property_read_bool(node, "ti,pwm-mode");
+
+ ret = of_property_read_u32(node, "ti,als-resistance-ohm", &val);
+ if (ret < 0 && ret != -EINVAL) {
+ dev_err(dev, "unable to read ti,als-resistance-ohm");
+ return ret;
+ }
+
+ /* Leave at high-Z, if the property was omitted or specified as 0 */
+ if (!ret && val != 0)
+ als_pdata->r_select = 200000 / val;
+
+ pdata->als = als_pdata;
+
+ return 0;
+}
+
+static int lm3533_of_parse_bl_node(struct device *dev,
+ struct device_node *node,
+ struct lm3533_platform_data *pdata)
+{
+ struct lm3533_bl_platform_data *bl_pdata;
+ int ret;
+ u32 reg;
+ u32 val;
+
+ ret = of_property_read_u32(node, "reg", ®);
+ if (ret < 0 || reg >= ARRAY_SIZE(lm3533_bl_devs)) {
+ dev_err(dev, "invalid reg property\n");
+ return ret;
+ }
+
+ bl_pdata = &pdata->backlights[reg];
+
+ /* Increment num_backlights if we're filling a new space */
+ if (reg + 1 > pdata->num_backlights)
+ pdata->num_backlights = reg + 1;
+
+ ret = of_property_read_string(node, "label",
+ (const char **)&bl_pdata->name);
+ if (ret < 0) {
+ dev_err(dev, "unable to parse label\n");
+ return ret;
+ }
+
+ ret = of_property_read_u32(node, "led-max-microamp", &val);
+ if (ret < 0) {
+ dev_err(dev, "unable to parse led-max-microamp\n");
+ return ret;
+ }
+ bl_pdata->max_current = val;
+
+ val = LM3533_DEFAULT_BRIGHTNESS;
+ ret = of_property_read_u32(node, "default-brightness", &val);
+ if (ret < 0 && ret != -EINVAL) {
+ dev_err(dev, "unable to parse default-brightness\n");
+ return ret;
+ }
+ bl_pdata->default_brightness = val;
+
+ ret = lm3533_of_parse_pwm_zones(node);
+ if (ret < 0) {
+ dev_err(dev, "failed to parse ti,pwm-zones\n");
+ return ret;
+ }
+ bl_pdata->pwm = ret;
+
+ return 0;
+}
+
+static int lm3533_of_parse_led_node(struct device *dev,
+ struct device_node *node,
+ struct lm3533_platform_data *pdata)
+{
+ struct lm3533_led_platform_data *led_pdata;
+ int ret;
+ u32 reg;
+ u32 val;
+
+ ret = of_property_read_u32(node, "reg", ®);
+ if (ret < 0 || reg >= ARRAY_SIZE(lm3533_led_devs)) {
+ dev_err(dev, "invalid reg property\n");
+ return ret;
+ }
+
+ led_pdata = &pdata->leds[reg];
+
+ /* Increment num_leds if we're filling a new space */
+ if (reg + 1 > pdata->num_leds)
+ pdata->num_leds = reg + 1;
+
+ ret = of_property_read_string(node, "label",
+ (const char **)&led_pdata->name);
+ if (ret < 0) {
+ dev_err(dev, "unable to parse label\n");
+ return ret;
+ }
+
+ ret = of_property_read_u32(node, "led-max-microamp", &val);
+ if (ret < 0) {
+ dev_err(dev, "unable to parse led-max-microamp\n");
+ return ret;
+ }
+ led_pdata->max_current = val;
+
+ ret = of_property_read_string(node, "linux,default-trigger",
+ &led_pdata->default_trigger);
+ if (ret < 0 && ret != -EINVAL) {
+ dev_err(dev, "unable to parse default-trigger\n");
+ return ret;
+ }
+
+ ret = lm3533_of_parse_pwm_zones(node);
+ if (ret < 0) {
+ dev_err(dev, "failed to parse ti,pwm-zones\n");
+ return ret;
+ }
+ led_pdata->pwm = ret;
+
+ return 0;
+}
+
+static int lm3533_pdata_from_of_node(struct device *dev)
+{
+ struct lm3533_platform_data *pdata;
+ struct device_node *node;
+ int ret;
+ const unsigned freqs[] = {
+ [LM3533_BOOST_FREQ_500KHZ] = 500,
+ [LM3533_BOOST_FREQ_1000KHZ] = 1000,
+ };
+ const unsigned ovps[] = {
+ [LM3533_BOOST_OVP_16V] = 16,
+ [LM3533_BOOST_OVP_24V] = 24,
+ [LM3533_BOOST_OVP_32V] = 32,
+ [LM3533_BOOST_OVP_40V] = 40,
+ };
+
+ pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+ if (!pdata)
+ return -ENOMEM;
+
+ pdata->backlights = devm_kcalloc(dev,
+ ARRAY_SIZE(lm3533_bl_devs),
+ sizeof(struct lm3533_bl_platform_data),
+ GFP_KERNEL);
+ if (!pdata->backlights)
+ return -ENOMEM;
+
+ pdata->leds = devm_kcalloc(dev,
+ ARRAY_SIZE(lm3533_led_devs),
+ sizeof(struct lm3533_led_platform_data),
+ GFP_KERNEL);
+ if (!pdata->leds)
+ return -ENOMEM;
+
+ pdata->gpio_hwen = of_get_named_gpio(dev->of_node, "ti,hwen-gpios", 0);
+ if (pdata->gpio_hwen < 0)
+ return pdata->gpio_hwen;
+
+ ret = lm3533_of_parse_enum(dev->of_node, "ti,boost-freq-khz", freqs, 2);
+ if (ret < 0) {
+ dev_err(dev, "failed to parse ti,boost-freq-khz\n");
+ return ret;
+ }
+ pdata->boost_freq = ret;
+
+ ret = lm3533_of_parse_enum(dev->of_node, "ti,boost-ovp", ovps, 4);
+ if (ret < 0) {
+ dev_err(dev, "failed to parse ti,boost-ovp\n");
+ return ret;
+ }
+ pdata->boost_ovp = ret;
+
+ for_each_child_of_node(dev->of_node, node) {
+ if (!of_node_cmp(node->name, "als"))
+ ret = lm3533_of_parse_als_node(dev, node, pdata);
+ else if (!of_node_cmp(node->name, "backlight"))
+ ret = lm3533_of_parse_bl_node(dev, node, pdata);
+ else if (!of_node_cmp(node->name, "led"))
+ ret = lm3533_of_parse_led_node(dev, node, pdata);
+
+ if (ret < 0) {
+ of_node_put(node);
+ return ret;
+ }
+ }
+
+ dev->platform_data = pdata;
+
+ return 0;
+}
+
static int lm3533_i2c_probe(struct i2c_client *i2c,
const struct i2c_device_id *id)
{
struct lm3533 *lm3533;
+ int ret;
dev_dbg(&i2c->dev, "%s\n", __func__);
@@ -608,6 +866,12 @@ static int lm3533_i2c_probe(struct i2c_client *i2c,
lm3533->dev = &i2c->dev;
lm3533->irq = i2c->irq;
+ if (i2c->dev.of_node) {
+ ret = lm3533_pdata_from_of_node(lm3533->dev);
+ if (ret < 0)
+ return ret;
+ }
+
return lm3533_device_init(lm3533);
}
--
2.4.2
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v2 3/3] mfd: lm3533: Support initialization from Device Tree
2015-10-27 16:29 ` [PATCH v2 3/3] mfd: lm3533: Support initialization from Device Tree Bjorn Andersson
@ 2015-10-27 16:43 ` kbuild test robot
2015-10-27 16:53 ` kbuild test robot
2015-10-28 11:40 ` Lee Jones
2 siblings, 0 replies; 20+ messages in thread
From: kbuild test robot @ 2015-10-27 16:43 UTC (permalink / raw)
To: Bjorn Andersson
Cc: kbuild-all, Lee Jones, Johan Hovold, linux-arm-kernel, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 2617 bytes --]
Hi Bjorn,
[auto build test WARNING on ljones-mfd/for-mfd-next -- if it's inappropriate base, please suggest rules for selecting the more suitable base]
url: https://github.com/0day-ci/linux/commits/Bjorn-Andersson/devicetree-mfd-Add-binding-for-the-TI-LM3533/20151028-003248
config: x86_64-randconfig-x012-201543 (attached as .config)
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All warnings (new ones prefixed by >>):
In file included from include/uapi/linux/stddef.h:1:0,
from include/linux/stddef.h:4,
from include/uapi/linux/posix_types.h:4,
from include/uapi/linux/types.h:13,
from include/linux/types.h:5,
from include/linux/list.h:4,
from include/linux/module.h:9,
from drivers/mfd/lm3533-core.c:14:
drivers/mfd/lm3533-core.c: In function 'lm3533_pdata_from_of_node':
drivers/mfd/lm3533-core.c:830:8: error: implicit declaration of function 'of_node_cmp' [-Werror=implicit-function-declaration]
if (!of_node_cmp(node->name, "als"))
^
include/linux/compiler.h:147:28: note: in definition of macro '__trace_if'
if (__builtin_constant_p((cond)) ? !!(cond) : \
^
>> drivers/mfd/lm3533-core.c:830:3: note: in expansion of macro 'if'
if (!of_node_cmp(node->name, "als"))
^
cc1: some warnings being treated as errors
vim +/if +830 drivers/mfd/lm3533-core.c
814
815 ret = lm3533_of_parse_enum(dev->of_node, "ti,boost-freq-khz", freqs, 2);
816 if (ret < 0) {
817 dev_err(dev, "failed to parse ti,boost-freq-khz\n");
818 return ret;
819 }
820 pdata->boost_freq = ret;
821
822 ret = lm3533_of_parse_enum(dev->of_node, "ti,boost-ovp", ovps, 4);
823 if (ret < 0) {
824 dev_err(dev, "failed to parse ti,boost-ovp\n");
825 return ret;
826 }
827 pdata->boost_ovp = ret;
828
829 for_each_child_of_node(dev->of_node, node) {
> 830 if (!of_node_cmp(node->name, "als"))
831 ret = lm3533_of_parse_als_node(dev, node, pdata);
832 else if (!of_node_cmp(node->name, "backlight"))
833 ret = lm3533_of_parse_bl_node(dev, node, pdata);
834 else if (!of_node_cmp(node->name, "led"))
835 ret = lm3533_of_parse_led_node(dev, node, pdata);
836
837 if (ret < 0) {
838 of_node_put(node);
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 19618 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 3/3] mfd: lm3533: Support initialization from Device Tree
2015-10-27 16:29 ` [PATCH v2 3/3] mfd: lm3533: Support initialization from Device Tree Bjorn Andersson
2015-10-27 16:43 ` kbuild test robot
@ 2015-10-27 16:53 ` kbuild test robot
2015-10-28 11:40 ` Lee Jones
2 siblings, 0 replies; 20+ messages in thread
From: kbuild test robot @ 2015-10-27 16:53 UTC (permalink / raw)
To: Bjorn Andersson
Cc: kbuild-all, Lee Jones, Johan Hovold, linux-arm-kernel, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1389 bytes --]
Hi Bjorn,
[auto build test ERROR on ljones-mfd/for-mfd-next -- if it's inappropriate base, please suggest rules for selecting the more suitable base]
url: https://github.com/0day-ci/linux/commits/Bjorn-Andersson/devicetree-mfd-Add-binding-for-the-TI-LM3533/20151028-003248
config: x86_64-randconfig-x019-201543 (attached as .config)
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
drivers/mfd/lm3533-core.c: In function 'lm3533_pdata_from_of_node':
>> drivers/mfd/lm3533-core.c:830:8: error: implicit declaration of function 'of_node_cmp' [-Werror=implicit-function-declaration]
if (!of_node_cmp(node->name, "als"))
^
cc1: some warnings being treated as errors
vim +/of_node_cmp +830 drivers/mfd/lm3533-core.c
824 dev_err(dev, "failed to parse ti,boost-ovp\n");
825 return ret;
826 }
827 pdata->boost_ovp = ret;
828
829 for_each_child_of_node(dev->of_node, node) {
> 830 if (!of_node_cmp(node->name, "als"))
831 ret = lm3533_of_parse_als_node(dev, node, pdata);
832 else if (!of_node_cmp(node->name, "backlight"))
833 ret = lm3533_of_parse_bl_node(dev, node, pdata);
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 25708 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/3] iio: light: lm3533-als: Print error message on invalid resistance
2015-10-25 18:09 ` [PATCH 2/3] iio: light: lm3533-als: Print error message on invalid resistance Bjorn Andersson
@ 2015-10-27 19:18 ` Joe Perches
2015-10-28 18:37 ` Bjorn Andersson
0 siblings, 1 reply; 20+ messages in thread
From: Joe Perches @ 2015-10-27 19:18 UTC (permalink / raw)
To: Bjorn Andersson
Cc: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Johan Hovold, Peter Meerwald, linux-iio, linux-kernel,
linux-arm-kernel
On Sun, 2015-10-25 at 11:09 -0700, Bjorn Andersson wrote:
> Print an error message to indicate that invalid configuration data was
> provided in the platform_data, rather than just aborting initialization.
Perhaps it'd be nicer to show the 3 values.
> diff --git a/drivers/iio/light/lm3533-als.c b/drivers/iio/light/lm3533-als.c
[]
> @@ -743,8 +743,10 @@ static int lm3533_als_set_resistor(struct lm3533_als *als, u8 val)
> {
> int ret;
>
> - if (val < LM3533_ALS_RESISTOR_MIN || val > LM3533_ALS_RESISTOR_MAX)
> + if (val < LM3533_ALS_RESISTOR_MIN || val > LM3533_ALS_RESISTOR_MAX) {
> + dev_err(&als->pdev->dev, "invalid resistor value\n");
> return -EINVAL;
> + };
>
> ret = lm3533_write(als->lm3533, LM3533_REG_ALS_RESISTOR_SELECT, val);
> if (ret) {
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 1/3] devicetree: mfd: Add binding for the TI LM3533
2015-10-27 16:29 ` [PATCH v2 " Bjorn Andersson
2015-10-27 16:29 ` [PATCH v2 2/3] iio: light: lm3533-als: Print error message on invalid resistance Bjorn Andersson
2015-10-27 16:29 ` [PATCH v2 3/3] mfd: lm3533: Support initialization from Device Tree Bjorn Andersson
@ 2015-10-27 19:21 ` Rob Herring
2015-10-30 18:42 ` Lee Jones
3 siblings, 0 replies; 20+ messages in thread
From: Rob Herring @ 2015-10-27 19:21 UTC (permalink / raw)
To: Bjorn Andersson
Cc: Pawel Moll, Mark Rutland, Ian Campbell, Johan Hovold, Kumar Gala,
devicetree, linux-iio, linux-kernel, linux-arm-kernel
On Tue, Oct 27, 2015 at 11:29 AM, Bjorn Andersson
<bjorn.andersson@sonymobile.com> wrote:
> Add the binding for the Texas Instruments LM3533 lighting power
> solution.
>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
Acked-by: Rob Herring <robh@kernel.org>
> ---
>
> Changes since v1:
> - Added unit to boost-freq and als-resistance (as the frequency now comes with
> a unit specifier I changed it to be expressed in kHz)
>
> Documentation/devicetree/bindings/mfd/lm3533.txt | 183 +++++++++++++++++++++++
> 1 file changed, 183 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mfd/lm3533.txt
>
> diff --git a/Documentation/devicetree/bindings/mfd/lm3533.txt b/Documentation/devicetree/bindings/mfd/lm3533.txt
> new file mode 100644
> index 000000000000..399df39ec7e7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/lm3533.txt
> @@ -0,0 +1,183 @@
> +Texas Instruments LM3533 binding
> +
> +This binding describes the Texas Instruments LM3533, a lighting power solution
> +for smartphone handsets. The common properties are described directly in the
> +node, while each individual component are described in an optional subnode.
> +
> +- compatible:
> + Usage: required
> + Value type: <stringlist>
> + Definition: must be:
> + "ti,lm3533"
> +
> +- reg:
> + Usage: required
> + Value type: <u32>
> + Definition: i2c address of the LM3533 chip
> +
> +- ti,hwen-gpios:
> + Usage: required
> + Value type: <prop-encoded-array>
> + Definition: reference to gpio pin connected to the HWEN input; as
> + specified in "gpio/gpio.txt"
> +
> +- ti,als-supply:
> + Usage: optional
> + Value type: <prop-encoded-array>
> + Definition: reference to regulator powering the V_als input; as
> + specified in "regulator/regulator.txt"
> +
> +- ti,boost-freq-khz:
> + Usage: required
> + Value type: <u32>
> + Definition: switch-frequency of the boost converter, must be either:
> + 500 or 1000
> +
> +- ti,boost-ovp:
> + Usage: required
> + Value type: <u32>
> + Definition: over voltage protection limit, must be one of: 16, 24, 32
> + or 40
> +
> +- #address-cells:
> + Usage: required
> + Value type: <u32>
> + Definition: must be 1
> +
> +- #size-cells:
> + Usage: required
> + Value type: <u32>
> + Definition: must be 0
> +
> += ALS SUBNODE
> +The als subnode must be named "als", it carries the als related properties.
> +
> +- ti,als-resistance-ohm:
> + Usage: required (unless ti,pwm-mode is specified)
> + Value type: <u32>
> + Definition: specifies the resistor value (R_als), in Ohm. Valid values
> + ranges from 200kOhm to 1574Ohm.
> +
> +- ti,pwm-mode:
> + Usage: optional
> + Value type: <empty>
> + Definition: specifies, if present, that the als should operate in pwm
> + mode - rather than analog mode
> +
> += BACKLIGHT NODES
> +Backlight subnodes must be named "backlight", they carry the backlight related
> +properties.
> +
> +- reg:
> + Usage: required
> + Value type: <u32>
> + Definition: specifies which of the two backlights this node corresponds
> + to
> +
> +- default-brightness:
> + Usage: optional
> + Value type: <32>
> + Definition: specifies the default brightness for the backlight, in
> + units of brightness [0-255]
> +
> +- label:
> + Usage: required
> + Value type: <string>
> + Definition: specifies a name of this backlight
> +
> +- led-max-microamp:
> + Usage: required
> + Value type: <u32>
> + Definition: specifies the max current for this backlight, in uA, as
> + described in "leds/common.txt"
> +
> +- ti,pwm-zones:
> + Usage: optional
> + Value type: <u32 list>
> + Definition: lists the ALS zones to be PWM controlled for this backlight,
> + the values in the list are in the range [0 - 4]
> +
> += LED NODES
> +LED subnodes must be named "led", they carry the LED related properties.
> +
> +- reg:
> + Usage: required
> + Value type: <u32>
> + Definition: specifies which of the four LEDs this node corresponds to
> +
> +- linux,default-trigger:
> + Usage: optional
> + Value type: <string>
> + Definition: specifies the default trigger for the LED, as described in
> + "leds/common.txt"
> +
> +- label:
> + Usage: required
> + Value type: <string>
> + Definition: specifies a name of this LED, as described in
> + "leds/common.txt"
> +
> +- led-max-microamp:
> + Usage: required
> + Value type: <u32>
> + Definition: specifies the max current for this LED, in uA, as described
> + in "leds/common.txt"
> +
> +- ti,pwm-zones:
> + Usage: optional
> + Value type: <u32 list>
> + Definition: lists the ALS zones to be PWM controlled for this LED, the
> + values in the list are in the range [0 - 4]
> +
> += EXAMPLE
> +
> +i2c@12460000 {
> + compatible = "qcom,i2c-qup-v1.1.1";
> + ...
> +
> + lm3533@36 {
> + compatible = "ti,lm3533";
> + reg = <0x36>;
> +
> + ti,hwen-gpios = <&pm8921_gpio 26 GPIO_ACTIVE_HIGH>;
> + ti,als-supply = <&pm8921_l11>;
> +
> + ti,boost-freq = <500000>;
> + ti,boost-ovp = <24>;
> +
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + als {
> + ti,als-resistance = <200000>;
> + };
> +
> + backlight@0 {
> + reg = <0>;
> + label = "backlight";
> +
> + led-max-microamp = <20200>;
> + };
> +
> + led@0 {
> + reg = <0>;
> + label = "red";
> +
> + led-max-microamp = <5000>;
> + };
> +
> + led@1 {
> + reg = <1>;
> + label = "green";
> +
> + led-max-microamp = <5000>;
> + };
> +
> + led@2 {
> + reg = <2>;
> + label = "blue";
> +
> + led-max-microamp = <5000>;
> + };
> + };
> +
> --
> 2.4.2
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 3/3] mfd: lm3533: Support initialization from Device Tree
2015-10-27 16:29 ` [PATCH v2 3/3] mfd: lm3533: Support initialization from Device Tree Bjorn Andersson
2015-10-27 16:43 ` kbuild test robot
2015-10-27 16:53 ` kbuild test robot
@ 2015-10-28 11:40 ` Lee Jones
2015-10-28 11:50 ` Joe Perches
2015-10-28 18:41 ` Bjorn Andersson
2 siblings, 2 replies; 20+ messages in thread
From: Lee Jones @ 2015-10-28 11:40 UTC (permalink / raw)
To: Bjorn Andersson; +Cc: Johan Hovold, linux-arm-kernel, linux-kernel
On Tue, 27 Oct 2015, Bjorn Andersson wrote:
> Implement Device Tree parsing functions to support initialization of the
> lm3533 drivers.
>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
> ---
>
> Changes since v1:
> - Added units to bost-freq and als resistance
>
> drivers/mfd/lm3533-core.c | 264 ++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 264 insertions(+)
I assume you plan on fixing the build robot's complains?
> diff --git a/drivers/mfd/lm3533-core.c b/drivers/mfd/lm3533-core.c
> index 5abcbb2e8849..9f0fb362ee76 100644
> --- a/drivers/mfd/lm3533-core.c
> +++ b/drivers/mfd/lm3533-core.c
> @@ -18,6 +18,7 @@
> #include <linux/gpio.h>
> #include <linux/i2c.h>
> #include <linux/mfd/core.h>
> +#include <linux/of_gpio.h>
> #include <linux/regmap.h>
> #include <linux/seq_file.h>
> #include <linux/slab.h>
> @@ -25,6 +26,7 @@
>
> #include <linux/mfd/lm3533.h>
>
> +#define LM3533_DEFAULT_BRIGHTNESS 200
>
> #define LM3533_BOOST_OVP_MASK 0x06
> #define LM3533_BOOST_OVP_SHIFT 1
> @@ -588,10 +590,266 @@ static const struct regmap_config regmap_config = {
> .precious_reg = lm3533_precious_register,
> };
>
> +static int lm3533_of_parse_enum(struct device_node *node,
> + const char *propname,
> + const unsigned *match, size_t num_matches)
> +{
> + size_t i;
> + int ret;
> + u32 val;
> +
> + ret = of_property_read_u32(node, propname, &val);
> + if (ret < 0)
> + return ret;
> +
> + for (i = 0; i < num_matches; i++) {
> + if (val == match[i])
> + return i;
> + }
> +
> + return -EINVAL;
> +}
> +
> +static int lm3533_of_parse_pwm_zones(struct device_node *node)
> +{
> + const char *propname = "ti,pwm-zones";
> + u32 zones[5];
> + int count;
> + int ret;
> + int i;
> +
> + count = of_property_count_u32_elems(node, propname);
> + if (count == -EINVAL)
> + return 0;
> + if (count <= 0)
> + return count;
> + if (count >= ARRAY_SIZE(zones))
> + return -EINVAL;
> +
> + ret = of_property_read_u32_array(node, propname, zones, count);
> + if (ret < 0)
> + return ret;
> +
> + /* Enable pwm input, and enable the selected zones */
> + ret = BIT(0);
> + for (i = 0; i < count; i++)
> + ret |= BIT(zones[i] + 1);
> +
> + return ret;
> +}
> +
> +static int lm3533_of_parse_als_node(struct device *dev,
> + struct device_node *node,
> + struct lm3533_platform_data *pdata)
> +{
> + struct lm3533_als_platform_data *als_pdata;
> + int ret;
> + u32 val;
> +
> + als_pdata = devm_kzalloc(dev,
> + sizeof(struct lm3533_als_platform_data),
> + GFP_KERNEL);
> + if (!als_pdata)
> + return -ENOMEM;
> +
> + als_pdata->pwm_mode = of_property_read_bool(node, "ti,pwm-mode");
> +
> + ret = of_property_read_u32(node, "ti,als-resistance-ohm", &val);
> + if (ret < 0 && ret != -EINVAL) {
> + dev_err(dev, "unable to read ti,als-resistance-ohm");
> + return ret;
> + }
> +
> + /* Leave at high-Z, if the property was omitted or specified as 0 */
> + if (!ret && val != 0)
> + als_pdata->r_select = 200000 / val;
> +
> + pdata->als = als_pdata;
> +
> + return 0;
> +}
> +
> +static int lm3533_of_parse_bl_node(struct device *dev,
> + struct device_node *node,
> + struct lm3533_platform_data *pdata)
> +{
> + struct lm3533_bl_platform_data *bl_pdata;
> + int ret;
> + u32 reg;
> + u32 val;
> +
> + ret = of_property_read_u32(node, "reg", ®);
> + if (ret < 0 || reg >= ARRAY_SIZE(lm3533_bl_devs)) {
> + dev_err(dev, "invalid reg property\n");
> + return ret;
> + }
> +
> + bl_pdata = &pdata->backlights[reg];
> +
> + /* Increment num_backlights if we're filling a new space */
> + if (reg + 1 > pdata->num_backlights)
> + pdata->num_backlights = reg + 1;
> +
> + ret = of_property_read_string(node, "label",
> + (const char **)&bl_pdata->name);
> + if (ret < 0) {
> + dev_err(dev, "unable to parse label\n");
> + return ret;
> + }
> +
> + ret = of_property_read_u32(node, "led-max-microamp", &val);
> + if (ret < 0) {
> + dev_err(dev, "unable to parse led-max-microamp\n");
> + return ret;
> + }
> + bl_pdata->max_current = val;
> +
> + val = LM3533_DEFAULT_BRIGHTNESS;
> + ret = of_property_read_u32(node, "default-brightness", &val);
> + if (ret < 0 && ret != -EINVAL) {
> + dev_err(dev, "unable to parse default-brightness\n");
> + return ret;
> + }
> + bl_pdata->default_brightness = val;
> +
> + ret = lm3533_of_parse_pwm_zones(node);
> + if (ret < 0) {
> + dev_err(dev, "failed to parse ti,pwm-zones\n");
> + return ret;
> + }
> + bl_pdata->pwm = ret;
> +
> + return 0;
> +}
> +
> +static int lm3533_of_parse_led_node(struct device *dev,
> + struct device_node *node,
> + struct lm3533_platform_data *pdata)
> +{
> + struct lm3533_led_platform_data *led_pdata;
> + int ret;
> + u32 reg;
> + u32 val;
> +
> + ret = of_property_read_u32(node, "reg", ®);
> + if (ret < 0 || reg >= ARRAY_SIZE(lm3533_led_devs)) {
> + dev_err(dev, "invalid reg property\n");
> + return ret;
> + }
> +
> + led_pdata = &pdata->leds[reg];
> +
> + /* Increment num_leds if we're filling a new space */
> + if (reg + 1 > pdata->num_leds)
> + pdata->num_leds = reg + 1;
> +
> + ret = of_property_read_string(node, "label",
> + (const char **)&led_pdata->name);
> + if (ret < 0) {
> + dev_err(dev, "unable to parse label\n");
> + return ret;
> + }
> +
> + ret = of_property_read_u32(node, "led-max-microamp", &val);
> + if (ret < 0) {
> + dev_err(dev, "unable to parse led-max-microamp\n");
> + return ret;
> + }
> + led_pdata->max_current = val;
> +
> + ret = of_property_read_string(node, "linux,default-trigger",
> + &led_pdata->default_trigger);
> + if (ret < 0 && ret != -EINVAL) {
> + dev_err(dev, "unable to parse default-trigger\n");
> + return ret;
> + }
> +
> + ret = lm3533_of_parse_pwm_zones(node);
> + if (ret < 0) {
> + dev_err(dev, "failed to parse ti,pwm-zones\n");
> + return ret;
> + }
> + led_pdata->pwm = ret;
> +
> + return 0;
> +}
> +
> +static int lm3533_pdata_from_of_node(struct device *dev)
> +{
> + struct lm3533_platform_data *pdata;
> + struct device_node *node;
> + int ret;
> + const unsigned freqs[] = {
> + [LM3533_BOOST_FREQ_500KHZ] = 500,
> + [LM3533_BOOST_FREQ_1000KHZ] = 1000,
> + };
> + const unsigned ovps[] = {
> + [LM3533_BOOST_OVP_16V] = 16,
> + [LM3533_BOOST_OVP_24V] = 24,
> + [LM3533_BOOST_OVP_32V] = 32,
> + [LM3533_BOOST_OVP_40V] = 40,
> + };
> +
> + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
> + if (!pdata)
> + return -ENOMEM;
> +
> + pdata->backlights = devm_kcalloc(dev,
> + ARRAY_SIZE(lm3533_bl_devs),
> + sizeof(struct lm3533_bl_platform_data),
> + GFP_KERNEL);
> + if (!pdata->backlights)
> + return -ENOMEM;
> +
> + pdata->leds = devm_kcalloc(dev,
> + ARRAY_SIZE(lm3533_led_devs),
> + sizeof(struct lm3533_led_platform_data),
> + GFP_KERNEL);
> + if (!pdata->leds)
> + return -ENOMEM;
> +
> + pdata->gpio_hwen = of_get_named_gpio(dev->of_node, "ti,hwen-gpios", 0);
> + if (pdata->gpio_hwen < 0)
> + return pdata->gpio_hwen;
> +
> + ret = lm3533_of_parse_enum(dev->of_node, "ti,boost-freq-khz", freqs, 2);
> + if (ret < 0) {
> + dev_err(dev, "failed to parse ti,boost-freq-khz\n");
> + return ret;
> + }
> + pdata->boost_freq = ret;
> +
> + ret = lm3533_of_parse_enum(dev->of_node, "ti,boost-ovp", ovps, 4);
> + if (ret < 0) {
> + dev_err(dev, "failed to parse ti,boost-ovp\n");
> + return ret;
> + }
> + pdata->boost_ovp = ret;
> +
> + for_each_child_of_node(dev->of_node, node) {
> + if (!of_node_cmp(node->name, "als"))
> + ret = lm3533_of_parse_als_node(dev, node, pdata);
> + else if (!of_node_cmp(node->name, "backlight"))
> + ret = lm3533_of_parse_bl_node(dev, node, pdata);
> + else if (!of_node_cmp(node->name, "led"))
> + ret = lm3533_of_parse_led_node(dev, node, pdata);
> +
> + if (ret < 0) {
> + of_node_put(node);
> + return ret;
> + }
> + }
> +
> + dev->platform_data = pdata;
> +
> + return 0;
> +}
> +
> static int lm3533_i2c_probe(struct i2c_client *i2c,
> const struct i2c_device_id *id)
> {
> struct lm3533 *lm3533;
> + int ret;
>
> dev_dbg(&i2c->dev, "%s\n", __func__);
>
> @@ -608,6 +866,12 @@ static int lm3533_i2c_probe(struct i2c_client *i2c,
> lm3533->dev = &i2c->dev;
> lm3533->irq = i2c->irq;
>
> + if (i2c->dev.of_node) {
> + ret = lm3533_pdata_from_of_node(lm3533->dev);
> + if (ret < 0)
> + return ret;
> + }
> +
> return lm3533_device_init(lm3533);
> }
>
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 3/3] mfd: lm3533: Support initialization from Device Tree
2015-10-28 11:40 ` Lee Jones
@ 2015-10-28 11:50 ` Joe Perches
2015-10-28 18:41 ` Bjorn Andersson
1 sibling, 0 replies; 20+ messages in thread
From: Joe Perches @ 2015-10-28 11:50 UTC (permalink / raw)
To: Lee Jones; +Cc: Bjorn Andersson, Johan Hovold, linux-arm-kernel, linux-kernel
On Wed, 2015-10-28 at 11:40 +0000, Lee Jones wrote:
> On Tue, 27 Oct 2015, Bjorn Andersson wrote:
> > Implement Device Tree parsing functions to support initialization of the
> > lm3533 drivers.
[]
> > Changes since v1:
> > - Added units to bost-freq and als resistance
[]
> I assume you plan on fixing the build robot's complains?
[ then +15 kb of useless quoted stuff ]
Please remember to trim your replies.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/3] iio: light: lm3533-als: Print error message on invalid resistance
2015-10-27 19:18 ` Joe Perches
@ 2015-10-28 18:37 ` Bjorn Andersson
2015-10-28 18:53 ` Joe Perches
0 siblings, 1 reply; 20+ messages in thread
From: Bjorn Andersson @ 2015-10-28 18:37 UTC (permalink / raw)
To: Joe Perches
Cc: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Johan Hovold, Peter Meerwald, linux-iio, linux-kernel,
linux-arm-kernel
On Tue 27 Oct 12:18 PDT 2015, Joe Perches wrote:
> On Sun, 2015-10-25 at 11:09 -0700, Bjorn Andersson wrote:
> > Print an error message to indicate that invalid configuration data was
> > provided in the platform_data, rather than just aborting initialization.
>
> Perhaps it'd be nicer to show the 3 values.
>
I do agree that it would be helpful to hint the developer about the
expected range. The problem I see here though is that, in the case of
devicetree we've now moved from resistance to a register value.
E.g. specifying 500kOhm in dt would make this piece complain about a
resistance of 0 being invalid and I would have to read the code and/or
datasheet anyways to find the valid range.
The reason I did not put this error print in the dt parser was that I
did not want to replicate the resistor limits and I figured it would be
useful for platform_data users as well. But if this is going to be
informative I think I need to move it.
So would you be okay with not improving the message? Or would you like
me to just move it into the code in patch 1 - and leave the
platform_data case as it was (with a silent failure)?
> > diff --git a/drivers/iio/light/lm3533-als.c b/drivers/iio/light/lm3533-als.c
> []
> > @@ -743,8 +743,10 @@ static int lm3533_als_set_resistor(struct lm3533_als *als, u8 val)
> > {
> > int ret;
> >
> > - if (val < LM3533_ALS_RESISTOR_MIN || val > LM3533_ALS_RESISTOR_MAX)
> > + if (val < LM3533_ALS_RESISTOR_MIN || val > LM3533_ALS_RESISTOR_MAX) {
> > + dev_err(&als->pdev->dev, "invalid resistor value\n");
> > return -EINVAL;
> > + };
> >
Regards,
Bjorn
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 3/3] mfd: lm3533: Support initialization from Device Tree
2015-10-28 11:40 ` Lee Jones
2015-10-28 11:50 ` Joe Perches
@ 2015-10-28 18:41 ` Bjorn Andersson
1 sibling, 0 replies; 20+ messages in thread
From: Bjorn Andersson @ 2015-10-28 18:41 UTC (permalink / raw)
To: Lee Jones; +Cc: Johan Hovold, linux-arm-kernel, linux-kernel
On Wed 28 Oct 04:40 PDT 2015, Lee Jones wrote:
> On Tue, 27 Oct 2015, Bjorn Andersson wrote:
>
> > Implement Device Tree parsing functions to support initialization of the
> > lm3533 drivers.
> >
> > Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
> > ---
> >
> > Changes since v1:
> > - Added units to bost-freq and als resistance
> >
> > drivers/mfd/lm3533-core.c | 264 ++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 264 insertions(+)
>
> I assume you plan on fixing the build robot's complains?
>
I do, unfortunately I didn't get those on v1 :/
Just trying to conclude if I should move the logic from patch 2 into
this patch before resending.
Regards,
Bjorn
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/3] iio: light: lm3533-als: Print error message on invalid resistance
2015-10-28 18:37 ` Bjorn Andersson
@ 2015-10-28 18:53 ` Joe Perches
0 siblings, 0 replies; 20+ messages in thread
From: Joe Perches @ 2015-10-28 18:53 UTC (permalink / raw)
To: Bjorn Andersson
Cc: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Johan Hovold, Peter Meerwald, linux-iio, linux-kernel,
linux-arm-kernel
On Wed, 2015-10-28 at 11:37 -0700, Bjorn Andersson wrote:
> On Tue 27 Oct 12:18 PDT 2015, Joe Perches wrote:
> > On Sun, 2015-10-25 at 11:09 -0700, Bjorn Andersson wrote:
> > > Print an error message to indicate that invalid configuration data was
> > > provided in the platform_data, rather than just aborting initialization.
> > Perhaps it'd be nicer to show the 3 values.
> I do agree that it would be helpful to hint the developer about the
> expected range. The problem I see here though is that, in the case of
> devicetree we've now moved from resistance to a register value.
[]
> So would you be okay with not improving the message? Or would you like
> me to just move it into the code in patch 1 - and leave the
> platform_data case as it was (with a silent failure)?
[]
> > > diff --git a/drivers/iio/light/lm3533-als.c b/drivers/iio/light/lm3533-als.c
> > []
> > > @@ -743,8 +743,10 @@ static int lm3533_als_set_resistor(struct lm3533_als *als, u8 val)
> > > {
> > > int ret;
> > >
> > > - if (val < LM3533_ALS_RESISTOR_MIN || val > LM3533_ALS_RESISTOR_MAX)
> > > + if (val < LM3533_ALS_RESISTOR_MIN || val > LM3533_ALS_RESISTOR_MAX) {
> > > + dev_err(&als->pdev->dev, "invalid resistor value\n");
> > > return -EINVAL;
> > > + };
> > >
What I suggested was just a trivial proposal.
Whatever you do is fine with me.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 1/3] devicetree: mfd: Add binding for the TI LM3533
2015-10-27 16:29 ` [PATCH v2 " Bjorn Andersson
` (2 preceding siblings ...)
2015-10-27 19:21 ` [PATCH v2 1/3] devicetree: mfd: Add binding for the TI LM3533 Rob Herring
@ 2015-10-30 18:42 ` Lee Jones
2015-10-30 19:41 ` Bjorn Andersson
3 siblings, 1 reply; 20+ messages in thread
From: Lee Jones @ 2015-10-30 18:42 UTC (permalink / raw)
To: Bjorn Andersson
Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
Johan Hovold, Kumar Gala, devicetree, linux-iio, linux-kernel,
linux-arm-kernel
On Tue, 27 Oct 2015, Bjorn Andersson wrote:
> Add the binding for the Texas Instruments LM3533 lighting power
> solution.
>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
> ---
>
> Changes since v1:
> - Added unit to boost-freq and als-resistance (as the frequency now comes with
> a unit specifier I changed it to be expressed in kHz)
>
> Documentation/devicetree/bindings/mfd/lm3533.txt | 183 +++++++++++++++++++++++
> 1 file changed, 183 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mfd/lm3533.txt
>
> diff --git a/Documentation/devicetree/bindings/mfd/lm3533.txt b/Documentation/devicetree/bindings/mfd/lm3533.txt
> new file mode 100644
> index 000000000000..399df39ec7e7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/lm3533.txt
> @@ -0,0 +1,183 @@
> +Texas Instruments LM3533 binding
> +
> +This binding describes the Texas Instruments LM3533, a lighting power solution
> +for smartphone handsets. The common properties are described directly in the
> +node, while each individual component are described in an optional subnode.
> +
> +- compatible:
> + Usage: required
> + Value type: <stringlist>
> + Definition: must be:
> + "ti,lm3533"
> +
> +- reg:
> + Usage: required
> + Value type: <u32>
> + Definition: i2c address of the LM3533 chip
> +
> +- ti,hwen-gpios:
> + Usage: required
> + Value type: <prop-encoded-array>
> + Definition: reference to gpio pin connected to the HWEN input; as
> + specified in "gpio/gpio.txt"
Why have you made this a vendor binding?
*-gpios is a generic property.
> +- ti,als-supply:
> + Usage: optional
> + Value type: <prop-encoded-array>
> + Definition: reference to regulator powering the V_als input; as
> + specified in "regulator/regulator.txt"
Same goes for *-supply.
> +- ti,boost-freq-khz:
> + Usage: required
> + Value type: <u32>
> + Definition: switch-frequency of the boost converter, must be either:
> + 500 or 1000
Quite a few vendors are using 'boost' now.
Perhaps we need to create a set of generic bindings.
Also, we usually measure DT bindings in HZ, not kHz.
> +- ti,boost-ovp:
> + Usage: required
> + Value type: <u32>
> + Definition: over voltage protection limit, must be one of: 16, 24, 32
> + or 40
Is this in volts? If so, it should be microvolts.
> +- #address-cells:
> + Usage: required
> + Value type: <u32>
> + Definition: must be 1
> +
> +- #size-cells:
> + Usage: required
> + Value type: <u32>
> + Definition: must be 0
> +
> += ALS SUBNODE
> +The als subnode must be named "als", it carries the als related properties.
Perfect time to tell us what ALS is/means.
> +- ti,als-resistance-ohm:
> + Usage: required (unless ti,pwm-mode is specified)
> + Value type: <u32>
> + Definition: specifies the resistor value (R_als), in Ohm. Valid values
> + ranges from 200kOhm to 1574Ohm.
Might be worth specifying the values which you are actually going to
use here i.e. "200kOhm" is not a valid u32.
> +- ti,pwm-mode:
> + Usage: optional
> + Value type: <empty>
> + Definition: specifies, if present, that the als should operate in pwm
Suggest s/pwm/PWM/
> + mode - rather than analog mode
> +
> += BACKLIGHT NODES
> +Backlight subnodes must be named "backlight", they carry the backlight related
> +properties.
> +
> +- reg:
> + Usage: required
> + Value type: <u32>
> + Definition: specifies which of the two backlights this node corresponds
> + to
> +
> +- default-brightness:
> + Usage: optional
> + Value type: <32>
> + Definition: specifies the default brightness for the backlight, in
> + units of brightness [0-255]
> +
> +- label:
> + Usage: required
> + Value type: <string>
> + Definition: specifies a name of this backlight
> +
> +- led-max-microamp:
> + Usage: required
> + Value type: <u32>
> + Definition: specifies the max current for this backlight, in uA, as
> + described in "leds/common.txt"
> +
> +- ti,pwm-zones:
> + Usage: optional
> + Value type: <u32 list>
> + Definition: lists the ALS zones to be PWM controlled for this backlight,
> + the values in the list are in the range [0 - 4]
> +
It's usually a good idea to point to where all of the aforementioned
generic properties are documented. I personally like the format
(See: ../<subsystem>/<binding>.txt)
> += LED NODES
> +LED subnodes must be named "led", they carry the LED related properties.
> +
> +- reg:
> + Usage: required
> + Value type: <u32>
> + Definition: specifies which of the four LEDs this node corresponds to
> +
> +- linux,default-trigger:
> + Usage: optional
> + Value type: <string>
> + Definition: specifies the default trigger for the LED, as described in
> + "leds/common.txt"
No such file. I think you mean "../leds/common/txt".
> +- label:
> + Usage: required
> + Value type: <string>
> + Definition: specifies a name of this LED, as described in
> + "leds/common.txt"
> +
> +- led-max-microamp:
> + Usage: required
> + Value type: <u32>
> + Definition: specifies the max current for this LED, in uA, as described
> + in "leds/common.txt"
> +
> +- ti,pwm-zones:
> + Usage: optional
> + Value type: <u32 list>
> + Definition: lists the ALS zones to be PWM controlled for this LED, the
> + values in the list are in the range [0 - 4]
> +
> += EXAMPLE
> +
> +i2c@12460000 {
> + compatible = "qcom,i2c-qup-v1.1.1";
> + ...
> +
> + lm3533@36 {
> + compatible = "ti,lm3533";
> + reg = <0x36>;
> +
> + ti,hwen-gpios = <&pm8921_gpio 26 GPIO_ACTIVE_HIGH>;
> + ti,als-supply = <&pm8921_l11>;
> +
> + ti,boost-freq = <500000>;
> + ti,boost-ovp = <24>;
> +
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + als {
> + ti,als-resistance = <200000>;
> + };
> +
> + backlight@0 {
> + reg = <0>;
> + label = "backlight";
> +
> + led-max-microamp = <20200>;
> + };
> +
> + led@0 {
> + reg = <0>;
> + label = "red";
> +
> + led-max-microamp = <5000>;
> + };
> +
> + led@1 {
> + reg = <1>;
> + label = "green";
> +
> + led-max-microamp = <5000>;
> + };
> +
> + led@2 {
> + reg = <2>;
> + label = "blue";
> +
> + led-max-microamp = <5000>;
> + };
> + };
> +
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 1/3] devicetree: mfd: Add binding for the TI LM3533
2015-10-30 18:42 ` Lee Jones
@ 2015-10-30 19:41 ` Bjorn Andersson
2015-10-30 20:18 ` Rob Herring
0 siblings, 1 reply; 20+ messages in thread
From: Bjorn Andersson @ 2015-10-30 19:41 UTC (permalink / raw)
To: Rob Herring, Lee Jones
Cc: Pawel Moll, Mark Rutland, Ian Campbell, Johan Hovold, Kumar Gala,
devicetree, linux-iio, linux-kernel, linux-arm-kernel
On Fri 30 Oct 11:42 PDT 2015, Lee Jones wrote:
Rob, please see the discussion regarding ti,boost-freq-khz below. Should
we both specify unit at the same time as we use standard units? (This is
not the first time I have to change this back and forth)
> On Tue, 27 Oct 2015, Bjorn Andersson wrote:
>
[..]
> > +- ti,hwen-gpios:
> > + Usage: required
> > + Value type: <prop-encoded-array>
> > + Definition: reference to gpio pin connected to the HWEN input; as
> > + specified in "gpio/gpio.txt"
>
> Why have you made this a vendor binding?
>
> *-gpios is a generic property.
>
Because the hwen gpio is a ti lm3533 specific thing, but I get what
you're saying. Will drop the prefix.
> > +- ti,als-supply:
> > + Usage: optional
> > + Value type: <prop-encoded-array>
> > + Definition: reference to regulator powering the V_als input; as
> > + specified in "regulator/regulator.txt"
>
> Same goes for *-supply.
>
Same here
> > +- ti,boost-freq-khz:
> > + Usage: required
> > + Value type: <u32>
> > + Definition: switch-frequency of the boost converter, must be either:
> > + 500 or 1000
>
> Quite a few vendors are using 'boost' now.
>
The ti,boost-low-freq from the bq25890 binding is the only other
property I can find that describes the same thing. So I'm not sure I
follow you here.
> Perhaps we need to create a set of generic bindings.
>
> Also, we usually measure DT bindings in HZ, not kHz.
>
I thought we had defined frequencies to be in HZ and HZ only, but then
Rob's comment that I need to actually specify the unit doesn't make any
sense.
Do we want these properties in a standard unit or do we want them
specifying the unit? Having both seems excessive.
> > +- ti,boost-ovp:
> > + Usage: required
> > + Value type: <u32>
> > + Definition: over voltage protection limit, must be one of: 16, 24, 32
> > + or 40
>
> Is this in volts? If so, it should be microvolts.
>
Okay, will update.
[..]
> > += ALS SUBNODE
> > +The als subnode must be named "als", it carries the als related properties.
>
> Perfect time to tell us what ALS is/means.
>
You're right, I'll expand this.
> > +- ti,als-resistance-ohm:
> > + Usage: required (unless ti,pwm-mode is specified)
> > + Value type: <u32>
> > + Definition: specifies the resistor value (R_als), in Ohm. Valid values
> > + ranges from 200kOhm to 1574Ohm.
>
> Might be worth specifying the values which you are actually going to
> use here i.e. "200kOhm" is not a valid u32.
>
I'll drop the units.
> > +- ti,pwm-mode:
> > + Usage: optional
> > + Value type: <empty>
> > + Definition: specifies, if present, that the als should operate in pwm
>
> Suggest s/pwm/PWM/
>
OK
[..]
> > +- ti,pwm-zones:
> > + Usage: optional
> > + Value type: <u32 list>
> > + Definition: lists the ALS zones to be PWM controlled for this backlight,
> > + the values in the list are in the range [0 - 4]
> > +
>
> It's usually a good idea to point to where all of the aforementioned
> generic properties are documented. I personally like the format
> (See: ../<subsystem>/<binding>.txt)
>
OK
> > += LED NODES
Regards,
Bjorn
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 1/3] devicetree: mfd: Add binding for the TI LM3533
2015-10-30 19:41 ` Bjorn Andersson
@ 2015-10-30 20:18 ` Rob Herring
2015-10-30 21:16 ` Bjorn Andersson
0 siblings, 1 reply; 20+ messages in thread
From: Rob Herring @ 2015-10-30 20:18 UTC (permalink / raw)
To: Bjorn Andersson
Cc: Lee Jones, Pawel Moll, Mark Rutland, Ian Campbell, Johan Hovold,
Kumar Gala, devicetree, linux-iio, linux-kernel,
linux-arm-kernel
On Fri, Oct 30, 2015 at 2:41 PM, Bjorn Andersson
<bjorn.andersson@sonymobile.com> wrote:
> On Fri 30 Oct 11:42 PDT 2015, Lee Jones wrote:
>
> Rob, please see the discussion regarding ti,boost-freq-khz below. Should
> we both specify unit at the same time as we use standard units? (This is
> not the first time I have to change this back and forth)
>
>> On Tue, 27 Oct 2015, Bjorn Andersson wrote:
>>
> [..]
>> > +- ti,hwen-gpios:
>> > + Usage: required
>> > + Value type: <prop-encoded-array>
>> > + Definition: reference to gpio pin connected to the HWEN input; as
>> > + specified in "gpio/gpio.txt"
>>
>> Why have you made this a vendor binding?
>>
>> *-gpios is a generic property.
>>
>
> Because the hwen gpio is a ti lm3533 specific thing, but I get what
> you're saying. Will drop the prefix.
Actually, that is fine. -gpios is common, but the rest is specific to
a binding. But if it is a common function, then a common name would be
fine. Enable gpios are common for example.
>> > +- ti,als-supply:
>> > + Usage: optional
>> > + Value type: <prop-encoded-array>
>> > + Definition: reference to regulator powering the V_als input; as
>> > + specified in "regulator/regulator.txt"
>>
>> Same goes for *-supply.
>>
>
> Same here
>
>> > +- ti,boost-freq-khz:
>> > + Usage: required
>> > + Value type: <u32>
>> > + Definition: switch-frequency of the boost converter, must be either:
>> > + 500 or 1000
>>
>> Quite a few vendors are using 'boost' now.
>>
>
> The ti,boost-low-freq from the bq25890 binding is the only other
> property I can find that describes the same thing. So I'm not sure I
> follow you here.
>
>> Perhaps we need to create a set of generic bindings.
>>
>> Also, we usually measure DT bindings in HZ, not kHz.
Surprisingly, there are not enough examples to draw much conclusion.
> I thought we had defined frequencies to be in HZ and HZ only, but then
> Rob's comment that I need to actually specify the unit doesn't make any
> sense.
I don't think we decided, but let's decide now. Go with Hz.
Really, I first prefer the property name has units and second having
standardized units. But if there is a common property without units, I
prefer that even more.
> Do we want these properties in a standard unit or do we want them
> specifying the unit? Having both seems excessive.
You mean "freq" would imply the units? No, we want the actual units in
the property.
Rob
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 1/3] devicetree: mfd: Add binding for the TI LM3533
2015-10-30 20:18 ` Rob Herring
@ 2015-10-30 21:16 ` Bjorn Andersson
0 siblings, 0 replies; 20+ messages in thread
From: Bjorn Andersson @ 2015-10-30 21:16 UTC (permalink / raw)
To: Rob Herring
Cc: Lee Jones, Pawel Moll, Mark Rutland, Ian Campbell, Johan Hovold,
Kumar Gala, devicetree, linux-iio, linux-kernel,
linux-arm-kernel
On Fri 30 Oct 13:18 PDT 2015, Rob Herring wrote:
> On Fri, Oct 30, 2015 at 2:41 PM, Bjorn Andersson
> <bjorn.andersson@sonymobile.com> wrote:
> > On Fri 30 Oct 11:42 PDT 2015, Lee Jones wrote:
> >
> > Rob, please see the discussion regarding ti,boost-freq-khz below. Should
> > we both specify unit at the same time as we use standard units? (This is
> > not the first time I have to change this back and forth)
> >
> >> On Tue, 27 Oct 2015, Bjorn Andersson wrote:
> >>
[..]
> > The ti,boost-low-freq from the bq25890 binding is the only other
> > property I can find that describes the same thing. So I'm not sure I
> > follow you here.
> >
> >> Perhaps we need to create a set of generic bindings.
> >>
> >> Also, we usually measure DT bindings in HZ, not kHz.
>
> Surprisingly, there are not enough examples to draw much conclusion.
>
> > I thought we had defined frequencies to be in HZ and HZ only, but then
> > Rob's comment that I need to actually specify the unit doesn't make any
> > sense.
>
> I don't think we decided, but let's decide now. Go with Hz.
>
+1
> Really, I first prefer the property name has units and second having
> standardized units. But if there is a common property without units, I
> prefer that even more.
>
You can find this property in a variety of hardware, but I don't think
it would make much sense to define this single property in a common
place today.
This seems to be the first case where we specify the unit on one of
these properties though.
> > Do we want these properties in a standard unit or do we want them
> > specifying the unit? Having both seems excessive.
>
> You mean "freq" would imply the units? No, we want the actual units in
> the property.
>
You're right, frequency can be measured in other units than Hz, I can't
think of one that would be applicable here though.
Either way, I'll slap on a unit to the property name - Hz that is...
Thanks for your input.
Regards,
Bjorn
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2015-10-30 21:17 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-25 18:09 [PATCH 1/3] devicetree: mfd: Add binding for the TI LM3533 Bjorn Andersson
2015-10-25 18:09 ` [PATCH 2/3] iio: light: lm3533-als: Print error message on invalid resistance Bjorn Andersson
2015-10-27 19:18 ` Joe Perches
2015-10-28 18:37 ` Bjorn Andersson
2015-10-28 18:53 ` Joe Perches
2015-10-25 18:09 ` [PATCH 3/3] mfd: lm3533: Support initialization from Device Tree Bjorn Andersson
2015-10-27 7:57 ` [PATCH 1/3] devicetree: mfd: Add binding for the TI LM3533 Rob Herring
2015-10-27 16:29 ` [PATCH v2 " Bjorn Andersson
2015-10-27 16:29 ` [PATCH v2 2/3] iio: light: lm3533-als: Print error message on invalid resistance Bjorn Andersson
2015-10-27 16:29 ` [PATCH v2 3/3] mfd: lm3533: Support initialization from Device Tree Bjorn Andersson
2015-10-27 16:43 ` kbuild test robot
2015-10-27 16:53 ` kbuild test robot
2015-10-28 11:40 ` Lee Jones
2015-10-28 11:50 ` Joe Perches
2015-10-28 18:41 ` Bjorn Andersson
2015-10-27 19:21 ` [PATCH v2 1/3] devicetree: mfd: Add binding for the TI LM3533 Rob Herring
2015-10-30 18:42 ` Lee Jones
2015-10-30 19:41 ` Bjorn Andersson
2015-10-30 20:18 ` Rob Herring
2015-10-30 21:16 ` Bjorn Andersson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).