[PATCH v3 0/3] DT support for AS3711 MFD, regulator and backlight drivers

[PATCH v3 0/3] DT support for AS3711 MFD, regulator and backlight drivers

[Guennadi Liakhovetski]

This is just a split of the previous version of this patch(-set), no code 
changes.

Guennadi Liakhovetski (3):
  mfd: as3711: add OF support
  regulator: as3711: add OF support
  backlight: as3711: add OF support

 Documentation/devicetree/bindings/mfd/as3711.txt |   73 +++++++++++++
 drivers/mfd/as3711.c                             |   27 ++++-
 drivers/regulator/as3711-regulator.c             |   74 +++++++++++++-
 drivers/video/backlight/as3711_bl.c              |  118 +++++++++++++++++++++-
 4 files changed, 284 insertions(+), 8 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mfd/as3711.txt

-- 
1.7.2.5

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

[PATCH v3 1/3] mfd: as3711: add OF support

[Guennadi Liakhovetski]

Add Flat Device Tree support to the AS3711 MFD driver. This patch just
allows to bind the driver to I2C devices, instantiated from the DT.
DT support for AS3711 cell drivers will be added in separate drivers.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
Reviwed-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 Documentation/devicetree/bindings/mfd/as3711.txt |   73 ++++++++++++++++++++++
 drivers/mfd/as3711.c                             |   27 +++++++-
 2 files changed, 96 insertions(+), 4 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mfd/as3711.txt

diff --git a/Documentation/devicetree/bindings/mfd/as3711.txt b/Documentation/devicetree/bindings/mfd/as3711.txt
new file mode 100644
index 0000000..d98cf18
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/as3711.txt
@@ -0,0 +1,73 @@
+AS3711 is an I2C PMIC from Austria MicroSystems with multiple DCDC and LDO power
+supplies, a battery charger and an RTC. So far only bindings for the two stepup
+DCDC converters are defined. Other DCDC and LDO supplies are configured, using
+standard regulator properties, they must belong to a sub-node, called
+"regulators" and be called "sd1" to "sd4" and "ldo1" to "ldo8." Stepup converter
+configuration should be placed in a subnode, called "backlight."
+
+Compulsory properties:
+- compatible		: must be "ams,as3711"
+- reg			: specifies the I2C address
+
+To use the SU1 converter as a backlight source the following two properties must
+be provided:
+- su1-dev		: framebuffer phandle
+- su1-max-uA		: maximum current
+
+To use the SU2 converter as a backlight source the following two properties must
+be provided:
+- su2-dev		: framebuffer phandle
+- su1-max-uA		: maximum current
+
+Additionally one of these properties must be provided to select the type of
+feedback used:
+- su2-feedback-voltage	: voltage feedback is used
+- su2-feedback-curr1	: CURR1 input used for current feedback
+- su2-feedback-curr2	: CURR2 input used for current feedback
+- su2-feedback-curr3	: CURR3 input used for current feedback
+- su2-feedback-curr-auto: automatic current feedback selection
+
+and one of these to select the over-voltage protection pin
+- su2-fbprot-lx-sd4	: LX_SD4 is used for over-voltage protection
+- su2-fbprot-gpio2	: GPIO2 is used for over-voltage protection
+- su2-fbprot-gpio3	: GPIO3 is used for over-voltage protection
+- su2-fbprot-gpio4	: GPIO4 is used for over-voltage protection
+
+If "su2-feedback-curr-auto" is selected, one or more of the following properties
+have to be specified:
+- su2-auto-curr1	: use CURR1 input for current feedback
+- su2-auto-curr2	: use CURR2 input for current feedback
+- su2-auto-curr3	: use CURR3 input for current feedback
+
+Example:
+
+as3711@40 {
+	compatible = "ams,as3711";
+	reg = <0x40>;
+
+	regulators {
+		sd4 {
+			regulator-name = "1.215V";
+			regulator-min-microvolt = <1215000>;
+			regulator-max-microvolt = <1235000>;
+		};
+		ldo2 {
+			regulator-name = "2.8V CPU";
+			regulator-min-microvolt = <2800000>;
+			regulator-max-microvolt = <2800000>;
+			regulator-always-on;
+			regulator-boot-on;
+		};
+	};
+
+	backlight {
+		compatible = "ams,as3711-bl";
+		su2-dev = <&lcdc>;
+		su2-max-uA = <36000>;
+		su2-feedback-curr-auto;
+		su2-fbprot-gpio4;
+		su2-auto-curr1;
+		su2-auto-curr2;
+		su2-auto-curr3;
+	};
+};
diff --git a/drivers/mfd/as3711.c b/drivers/mfd/as3711.c
index e994c96..01e4141 100644
--- a/drivers/mfd/as3711.c
+++ b/drivers/mfd/as3711.c
@@ -112,16 +112,34 @@ static const struct regmap_config as3711_regmap_config = {
 	.cache_type = REGCACHE_RBTREE,
 };
 
+#ifdef CONFIG_OF
+static struct of_device_id as3711_of_match[] = {
+	{.compatible = "ams,as3711",},
+	{}
+};
+MODULE_DEVICE_TABLE(of, as3711_of_match);
+#endif
+
 static int as3711_i2c_probe(struct i2c_client *client,
 			    const struct i2c_device_id *id)
 {
 	struct as3711 *as3711;
-	struct as3711_platform_data *pdata = client->dev.platform_data;
+	struct as3711_platform_data *pdata;
 	unsigned int id1, id2;
 	int ret;
 
-	if (!pdata)
-		dev_dbg(&client->dev, "Platform data not found\n");
+	if (!client->dev.of_node) {
+		pdata = client->dev.platform_data;
+		if (!pdata)
+			dev_dbg(&client->dev, "Platform data not found\n");
+	} else {
+		pdata = devm_kzalloc(&client->dev,
+				     sizeof(*pdata), GFP_KERNEL);
+		if (!pdata) {
+			dev_err(&client->dev, "Failed to allocate pdata\n");
+			return -ENOMEM;
+		}
+	}
 
 	as3711 = devm_kzalloc(&client->dev, sizeof(struct as3711), GFP_KERNEL);
 	if (!as3711) {
@@ -193,7 +211,8 @@ static struct i2c_driver as3711_i2c_driver = {
 	.driver = {
 		   .name = "as3711",
 		   .owner = THIS_MODULE,
-		   },
+		   .of_match_table = of_match_ptr(as3711_of_match),
+	},
 	.probe = as3711_i2c_probe,
 	.remove = as3711_i2c_remove,
 	.id_table = as3711_i2c_id,
-- 
1.7.2.5

Re: [PATCH v3 1/3] mfd: as3711: add OF support

[Samuel Ortiz]

Hi Guennadi,

On Fri, Mar 22, 2013 at 05:15:47PM +0100, Guennadi Liakhovetski wrote:
> Add Flat Device Tree support to the AS3711 MFD driver. This patch just
> allows to bind the driver to I2C devices, instantiated from the DT.
> DT support for AS3711 cell drivers will be added in separate drivers.
> 
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
> Reviwed-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> ---
>  Documentation/devicetree/bindings/mfd/as3711.txt |   73 ++++++++++++++++++++++
>  drivers/mfd/as3711.c                             |   27 +++++++-
>  2 files changed, 96 insertions(+), 4 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/mfd/as3711.txt
Applied, thanks.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

[PATCH v3 2/3] regulator: as3711: add OF support

[Guennadi Liakhovetski]

AS3711 regulator OF support only evaluates standard regulator DT
properties.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
Reviwed-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 drivers/regulator/as3711-regulator.c |   74 ++++++++++++++++++++++++++++++++-
 1 files changed, 71 insertions(+), 3 deletions(-)

diff --git a/drivers/regulator/as3711-regulator.c b/drivers/regulator/as3711-regulator.c
index f0ba8c4..0539b3e 100644
--- a/drivers/regulator/as3711-regulator.c
+++ b/drivers/regulator/as3711-regulator.c
@@ -13,9 +13,11 @@
 #include <linux/init.h>
 #include <linux/mfd/as3711.h>
 #include <linux/module.h>
+#include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
 #include <linux/regulator/driver.h>
+#include <linux/regulator/of_regulator.h>
 #include <linux/slab.h>
 
 struct as3711_regulator_info {
@@ -276,6 +278,60 @@ static struct as3711_regulator_info as3711_reg_info[] = {
 
 #define AS3711_REGULATOR_NUM ARRAY_SIZE(as3711_reg_info)
 
+static const char *as3711_regulator_of_names[AS3711_REGULATOR_NUM] = {
+	[AS3711_REGULATOR_SD_1] = "sd1",
+	[AS3711_REGULATOR_SD_2] = "sd2",
+	[AS3711_REGULATOR_SD_3] = "sd3",
+	[AS3711_REGULATOR_SD_4] = "sd4",
+	[AS3711_REGULATOR_LDO_1] = "ldo1",
+	[AS3711_REGULATOR_LDO_2] = "ldo2",
+	[AS3711_REGULATOR_LDO_3] = "ldo3",
+	[AS3711_REGULATOR_LDO_4] = "ldo4",
+	[AS3711_REGULATOR_LDO_5] = "ldo5",
+	[AS3711_REGULATOR_LDO_6] = "ldo6",
+	[AS3711_REGULATOR_LDO_7] = "ldo7",
+	[AS3711_REGULATOR_LDO_8] = "ldo8",
+};
+
+static int as3711_regulator_parse_dt(struct device *dev,
+				struct device_node **of_node, const int count)
+{
+	struct as3711_regulator_pdata *pdata = dev_get_platdata(dev);
+	struct device_node *regulators =
+		of_find_node_by_name(dev->parent->of_node, "regulators");
+	struct of_regulator_match *matches, *match;
+	int ret, i;
+
+	if (!regulators) {
+		dev_err(dev, "regulator node not found\n");
+		return -ENODEV;
+	}
+
+	matches = devm_kzalloc(dev, sizeof(*matches) * count, GFP_KERNEL);
+	if (!matches)
+		return -ENOMEM;
+
+	for (i = 0, match = matches; i < count; i++, match++) {
+		match->name = as3711_regulator_of_names[i];
+		match->driver_data = as3711_reg_info + i;
+	}
+
+	ret = of_regulator_match(dev->parent, regulators, matches, count);
+	of_node_put(regulators);
+	if (ret < 0) {
+		dev_err(dev, "Error parsing regulator init data: %d\n", ret);
+		return ret;
+	}
+
+	for (i = 0, match = matches; i < count; i++, match++)
+		if (match->of_node) {
+			pdata->init_data[i] = match->init_data;
+			of_node[i] = match->of_node;
+		}
+
+	return 0;
+}
+
 static int as3711_regulator_probe(struct platform_device *pdev)
 {
 	struct as3711_regulator_pdata *pdata = dev_get_platdata(&pdev->dev);
@@ -284,13 +340,24 @@ static int as3711_regulator_probe(struct platform_device *pdev)
 	struct regulator_config config = {.dev = &pdev->dev,};
 	struct as3711_regulator *reg = NULL;
 	struct as3711_regulator *regs;
+	struct device_node *of_node[AS3711_REGULATOR_NUM] = {};
 	struct regulator_dev *rdev;
 	struct as3711_regulator_info *ri;
 	int ret;
 	int id;
 
-	if (!pdata)
-		dev_dbg(&pdev->dev, "No platform data...\n");
+	if (!pdata) {
+		dev_err(&pdev->dev, "No platform data...\n");
+		return -ENODEV;
+	}
+
+	if (pdev->dev.parent->of_node) {
+		ret = as3711_regulator_parse_dt(&pdev->dev, of_node, AS3711_REGULATOR_NUM);
+		if (ret < 0) {
+			dev_err(&pdev->dev, "DT parsing failed: %d\n", ret);
+			return ret;
+		}
+	}
 
 	regs = devm_kzalloc(&pdev->dev, AS3711_REGULATOR_NUM *
 			sizeof(struct as3711_regulator), GFP_KERNEL);
@@ -300,7 +367,7 @@ static int as3711_regulator_probe(struct platform_device *pdev)
 	}
 
 	for (id = 0, ri = as3711_reg_info; id < AS3711_REGULATOR_NUM; ++id, ri++) {
-		reg_data = pdata ? pdata->init_data[id] : NULL;
+		reg_data = pdata->init_data[id];
 
 		/* No need to register if there is no regulator data */
 		if (!reg_data)
@@ -312,6 +379,7 @@ static int as3711_regulator_probe(struct platform_device *pdev)
 		config.init_data = reg_data;
 		config.driver_data = reg;
 		config.regmap = as3711->regmap;
+		config.of_node = of_node[id];
 
 		rdev = regulator_register(&ri->desc, &config);
 		if (IS_ERR(rdev)) {
-- 
1.7.2.5

Re: [PATCH v3 2/3] regulator: as3711: add OF support

[Mark Brown]

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

On Fri, Mar 22, 2013 at 05:15:48PM +0100, Guennadi Liakhovetski wrote:
> AS3711 regulator OF support only evaluates standard regulator DT
> properties.

It looks like this has no dependencies on the MFD patch, is that
correct?

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v3 2/3] regulator: as3711: add OF support

[Guennadi Liakhovetski]

On Fri, 22 Mar 2013, Mark Brown wrote:

> On Fri, Mar 22, 2013 at 05:15:48PM +0100, Guennadi Liakhovetski wrote:
> > AS3711 regulator OF support only evaluates standard regulator DT
> > properties.
> 
> It looks like this has no dependencies on the MFD patch, is that
> correct?

Yes, that's correct. That was the purpose of this split - to pull the 
patches via respective trees.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH v3 2/3] regulator: as3711: add OF support

[Mark Brown]

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

On Fri, Mar 22, 2013 at 05:15:48PM +0100, Guennadi Liakhovetski wrote:
> AS3711 regulator OF support only evaluates standard regulator DT
> properties.

Applied, thanks.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH v3 3/3] backlight: as3711: add OF support

[Guennadi Liakhovetski]

Add support for configuring AS3711 backlight driver from DT.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
Reviwed-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 drivers/video/backlight/as3711_bl.c |  118 ++++++++++++++++++++++++++++++++++-
 1 files changed, 117 insertions(+), 1 deletions(-)

diff --git a/drivers/video/backlight/as3711_bl.c b/drivers/video/backlight/as3711_bl.c
index 41d52fe..123887c 100644
--- a/drivers/video/backlight/as3711_bl.c
+++ b/drivers/video/backlight/as3711_bl.c
@@ -258,6 +258,109 @@ static int as3711_bl_register(struct platform_device *pdev,
 	return 0;
 }
 
+static int as3711_backlight_parse_dt(struct device *dev)
+{
+	struct as3711_bl_pdata *pdata = dev_get_platdata(dev);
+	struct device_node *bl =
+		of_find_node_by_name(dev->parent->of_node, "backlight"), *fb;
+	int ret;
+
+	if (!bl) {
+		dev_dbg(dev, "backlight node not found\n");
+		return -ENODEV;
+	}
+
+	fb = of_parse_phandle(bl, "su1-dev", 0);
+	if (fb) {
+		pdata->su1_fb = fb->full_name;
+
+		ret = of_property_read_u32(bl, "su1-max-uA", &pdata->su1_max_uA);
+		if (pdata->su1_max_uA <= 0)
+			ret = -EINVAL;
+		if (ret < 0)
+			return ret;
+	}
+
+	fb = of_parse_phandle(bl, "su2-dev", 0);
+	if (fb) {
+		int count = 0;
+
+		pdata->su2_fb = fb->full_name;
+
+		ret = of_property_read_u32(bl, "su2-max-uA", &pdata->su2_max_uA);
+		if (pdata->su2_max_uA <= 0)
+			ret = -EINVAL;
+		if (ret < 0)
+			return ret;
+
+		if (of_find_property(bl, "su2-feedback-voltage", NULL)) {
+			pdata->su2_feedback = AS3711_SU2_VOLTAGE;
+			count++;
+		}
+		if (of_find_property(bl, "su2-feedback-curr1", NULL)) {
+			pdata->su2_feedback = AS3711_SU2_CURR1;
+			count++;
+		}
+		if (of_find_property(bl, "su2-feedback-curr2", NULL)) {
+			pdata->su2_feedback = AS3711_SU2_CURR2;
+			count++;
+		}
+		if (of_find_property(bl, "su2-feedback-curr3", NULL)) {
+			pdata->su2_feedback = AS3711_SU2_CURR3;
+			count++;
+		}
+		if (of_find_property(bl, "su2-feedback-curr-auto", NULL)) {
+			pdata->su2_feedback = AS3711_SU2_CURR_AUTO;
+			count++;
+		}
+		if (count != 1)
+			return -EINVAL;
+
+		count = 0;
+		if (of_find_property(bl, "su2-fbprot-lx-sd4", NULL)) {
+			pdata->su2_fbprot = AS3711_SU2_LX_SD4;
+			count++;
+		}
+		if (of_find_property(bl, "su2-fbprot-gpio2", NULL)) {
+			pdata->su2_fbprot = AS3711_SU2_GPIO2;
+			count++;
+		}
+		if (of_find_property(bl, "su2-fbprot-gpio3", NULL)) {
+			pdata->su2_fbprot = AS3711_SU2_GPIO3;
+			count++;
+		}
+		if (of_find_property(bl, "su2-fbprot-gpio4", NULL)) {
+			pdata->su2_fbprot = AS3711_SU2_GPIO4;
+			count++;
+		}
+		if (count != 1)
+			return -EINVAL;
+
+		count = 0;
+		if (of_find_property(bl, "su2-auto-curr1", NULL)) {
+			pdata->su2_auto_curr1 = true;
+			count++;
+		}
+		if (of_find_property(bl, "su2-auto-curr2", NULL)) {
+			pdata->su2_auto_curr2 = true;
+			count++;
+		}
+		if (of_find_property(bl, "su2-auto-curr3", NULL)) {
+			pdata->su2_auto_curr3 = true;
+			count++;
+		}
+
+		/*
+		 * At least one su2-auto-curr* must be specified iff
+		 * AS3711_SU2_CURR_AUTO is used
+		 */
+		if (!count ^ (pdata->su2_feedback != AS3711_SU2_CURR_AUTO))
+			return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int as3711_backlight_probe(struct platform_device *pdev)
 {
 	struct as3711_bl_pdata *pdata = dev_get_platdata(&pdev->dev);
@@ -267,11 +370,24 @@ static int as3711_backlight_probe(struct platform_device *pdev)
 	unsigned int max_brightness;
 	int ret;
 
-	if (!pdata || (!pdata->su1_fb && !pdata->su2_fb)) {
+	if (!pdata) {
 		dev_err(&pdev->dev, "No platform data, exiting...\n");
 		return -ENODEV;
 	}
 
+	if (pdev->dev.parent->of_node) {
+		ret = as3711_backlight_parse_dt(&pdev->dev);
+		if (ret < 0) {
+			dev_err(&pdev->dev, "DT parsing failed: %d\n", ret);
+			return ret;
+		}
+	}
+
+	if (!pdata->su1_fb && !pdata->su2_fb) {
+		dev_err(&pdev->dev, "No framebuffer specified\n");
+		return -EINVAL;
+	}
+
 	/*
 	 * Due to possible hardware damage I chose to block all modes,
 	 * unsupported on my hardware. Anyone, wishing to use any of those modes
-- 
1.7.2.5

Re: [PATCH v3 3/3] backlight: as3711: add OF support

[Jingoo Han]

On Saturday, March 23, 2013 1:16 AM, Guennadi Liakhovetski wrote:
> 
> Add support for configuring AS3711 backlight driver from DT.
> 
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
> Reviwed-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

Acked-by: Jingoo Han <jg1.han@samsung.com>


But, there is a typo in comment.
> +		 * At least one su2-auto-curr* must be specified iff
s/iff/if

Best regards.
Jingoo Han

> ---
>  drivers/video/backlight/as3711_bl.c |  118 ++++++++++++++++++++++++++++++++++-
>  1 files changed, 117 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/video/backlight/as3711_bl.c b/drivers/video/backlight/as3711_bl.c
> index 41d52fe..123887c 100644
> --- a/drivers/video/backlight/as3711_bl.c
> +++ b/drivers/video/backlight/as3711_bl.c
> @@ -258,6 +258,109 @@ static int as3711_bl_register(struct platform_device *pdev,
>  	return 0;
>  }
> 
> +static int as3711_backlight_parse_dt(struct device *dev)
> +{
> +	struct as3711_bl_pdata *pdata = dev_get_platdata(dev);
> +	struct device_node *bl =
> +		of_find_node_by_name(dev->parent->of_node, "backlight"), *fb;
> +	int ret;
> +
> +	if (!bl) {
> +		dev_dbg(dev, "backlight node not found\n");
> +		return -ENODEV;
> +	}
> +
> +	fb = of_parse_phandle(bl, "su1-dev", 0);
> +	if (fb) {
> +		pdata->su1_fb = fb->full_name;
> +
> +		ret = of_property_read_u32(bl, "su1-max-uA", &pdata->su1_max_uA);
> +		if (pdata->su1_max_uA <= 0)
> +			ret = -EINVAL;
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	fb = of_parse_phandle(bl, "su2-dev", 0);
> +	if (fb) {
> +		int count = 0;
> +
> +		pdata->su2_fb = fb->full_name;
> +
> +		ret = of_property_read_u32(bl, "su2-max-uA", &pdata->su2_max_uA);
> +		if (pdata->su2_max_uA <= 0)
> +			ret = -EINVAL;
> +		if (ret < 0)
> +			return ret;
> +
> +		if (of_find_property(bl, "su2-feedback-voltage", NULL)) {
> +			pdata->su2_feedback = AS3711_SU2_VOLTAGE;
> +			count++;
> +		}
> +		if (of_find_property(bl, "su2-feedback-curr1", NULL)) {
> +			pdata->su2_feedback = AS3711_SU2_CURR1;
> +			count++;
> +		}
> +		if (of_find_property(bl, "su2-feedback-curr2", NULL)) {
> +			pdata->su2_feedback = AS3711_SU2_CURR2;
> +			count++;
> +		}
> +		if (of_find_property(bl, "su2-feedback-curr3", NULL)) {
> +			pdata->su2_feedback = AS3711_SU2_CURR3;
> +			count++;
> +		}
> +		if (of_find_property(bl, "su2-feedback-curr-auto", NULL)) {
> +			pdata->su2_feedback = AS3711_SU2_CURR_AUTO;
> +			count++;
> +		}
> +		if (count != 1)
> +			return -EINVAL;
> +
> +		count = 0;
> +		if (of_find_property(bl, "su2-fbprot-lx-sd4", NULL)) {
> +			pdata->su2_fbprot = AS3711_SU2_LX_SD4;
> +			count++;
> +		}
> +		if (of_find_property(bl, "su2-fbprot-gpio2", NULL)) {
> +			pdata->su2_fbprot = AS3711_SU2_GPIO2;
> +			count++;
> +		}
> +		if (of_find_property(bl, "su2-fbprot-gpio3", NULL)) {
> +			pdata->su2_fbprot = AS3711_SU2_GPIO3;
> +			count++;
> +		}
> +		if (of_find_property(bl, "su2-fbprot-gpio4", NULL)) {
> +			pdata->su2_fbprot = AS3711_SU2_GPIO4;
> +			count++;
> +		}
> +		if (count != 1)
> +			return -EINVAL;
> +
> +		count = 0;
> +		if (of_find_property(bl, "su2-auto-curr1", NULL)) {
> +			pdata->su2_auto_curr1 = true;
> +			count++;
> +		}
> +		if (of_find_property(bl, "su2-auto-curr2", NULL)) {
> +			pdata->su2_auto_curr2 = true;
> +			count++;
> +		}
> +		if (of_find_property(bl, "su2-auto-curr3", NULL)) {
> +			pdata->su2_auto_curr3 = true;
> +			count++;
> +		}
> +
> +		/*
> +		 * At least one su2-auto-curr* must be specified iff
> +		 * AS3711_SU2_CURR_AUTO is used
> +		 */
> +		if (!count ^ (pdata->su2_feedback != AS3711_SU2_CURR_AUTO))
> +			return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
>  static int as3711_backlight_probe(struct platform_device *pdev)
>  {
>  	struct as3711_bl_pdata *pdata = dev_get_platdata(&pdev->dev);
> @@ -267,11 +370,24 @@ static int as3711_backlight_probe(struct platform_device *pdev)
>  	unsigned int max_brightness;
>  	int ret;
> 
> -	if (!pdata || (!pdata->su1_fb && !pdata->su2_fb)) {
> +	if (!pdata) {
>  		dev_err(&pdev->dev, "No platform data, exiting...\n");
>  		return -ENODEV;
>  	}
> 
> +	if (pdev->dev.parent->of_node) {
> +		ret = as3711_backlight_parse_dt(&pdev->dev);
> +		if (ret < 0) {
> +			dev_err(&pdev->dev, "DT parsing failed: %d\n", ret);
> +			return ret;
> +		}
> +	}
> +
> +	if (!pdata->su1_fb && !pdata->su2_fb) {
> +		dev_err(&pdev->dev, "No framebuffer specified\n");
> +		return -EINVAL;
> +	}
> +
>  	/*
>  	 * Due to possible hardware damage I chose to block all modes,
>  	 * unsupported on my hardware. Anyone, wishing to use any of those modes
> --
> 1.7.2.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 3/3] backlight: as3711: add OF support

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 381 bytes --]

On Mon, Mar 25, 2013 at 02:12:21PM +0900, Jingoo Han wrote:
> On Saturday, March 23, 2013 1:16 AM, Guennadi Liakhovetski wrote:

> But, there is a typo in comment.
> > +		 * At least one su2-auto-curr* must be specified iff
> s/iff/if

Are you sure that's a typo?  "Iff" is a bit of mathematical jargon
which means "if and only if" that's fairly often used in a computing
context.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH v3 3/3] backlight: as3711: add OF support

[Guennadi Liakhovetski]

On Mon, 25 Mar 2013, Mark Brown wrote:

> On Mon, Mar 25, 2013 at 02:12:21PM +0900, Jingoo Han wrote:
> > On Saturday, March 23, 2013 1:16 AM, Guennadi Liakhovetski wrote:
> 
> > But, there is a typo in comment.
> > > +		 * At least one su2-auto-curr* must be specified iff
> > s/iff/if
> 
> Are you sure that's a typo?  "Iff" is a bit of mathematical jargon
> which means "if and only if" that's fairly often used in a computing
> context.

Thanks, Mark, that's exactly what I was trying to say there :)

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH v3 3/3] backlight: as3711: add OF support

[Jingoo Han]

On Monday, March 25, 2013 7:15 PM, Mark Brown wrote:
> 
> On Mon, Mar 25, 2013 at 02:12:21PM +0900, Jingoo Han wrote:
> > On Saturday, March 23, 2013 1:16 AM, Guennadi Liakhovetski wrote:
> 
> > But, there is a typo in comment.
> > > +		 * At least one su2-auto-curr* must be specified iff
> > s/iff/if
> 
> Are you sure that's a typo?  "Iff" is a bit of mathematical jargon
> which means "if and only if" that's fairly often used in a computing
> context.

Oh, it's my mistake.

'xor' is used; thus, it is not a typo.
 Thank you for your comment.

Best regards,
Jingoo Han

Re: [PATCH v3 3/3] backlight: as3711: add OF support

[Andrew Morton]

On Fri, 22 Mar 2013 17:15:49 +0100 Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:

> Add support for configuring AS3711 backlight driver from DT.
> 
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
> Reviwed-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> ---
>  drivers/video/backlight/as3711_bl.c |  118 ++++++++++++++++++++++++++++++++++-
>  1 files changed, 117 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/video/backlight/as3711_bl.c b/drivers/video/backlight/as3711_bl.c
> index 41d52fe..123887c 100644
> --- a/drivers/video/backlight/as3711_bl.c
> +++ b/drivers/video/backlight/as3711_bl.c
> @@ -258,6 +258,109 @@ static int as3711_bl_register(struct platform_device *pdev,
>  	return 0;
>  }
>  
> +static int as3711_backlight_parse_dt(struct device *dev)
> +{
> +	struct as3711_bl_pdata *pdata = dev_get_platdata(dev);
> +	struct device_node *bl =
> +		of_find_node_by_name(dev->parent->of_node, "backlight"), *fb;
> +	int ret;

It's tidier to do

	struct device_node *bl;
	
	bl = of_find_node_by_name(dev->parent->of_node, "backlight"), *fb;

and avoid the 80-col trickery.

> +	if (!bl) {
> +		dev_dbg(dev, "backlight node not found\n");
> +		return -ENODEV;
> +	}
> +
> +	fb = of_parse_phandle(bl, "su1-dev", 0);
> +	if (fb) {
> +		pdata->su1_fb = fb->full_name;
> +
> +		ret = of_property_read_u32(bl, "su1-max-uA", &pdata->su1_max_uA);
> +		if (pdata->su1_max_uA <= 0)
> +			ret = -EINVAL;
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	fb = of_parse_phandle(bl, "su2-dev", 0);
> +	if (fb) {
> +		int count = 0;
> +
> +		pdata->su2_fb = fb->full_name;
> +
> +		ret = of_property_read_u32(bl, "su2-max-uA", &pdata->su2_max_uA);
> +		if (pdata->su2_max_uA <= 0)
> +			ret = -EINVAL;
> +		if (ret < 0)
> +			return ret;
> +
> +		if (of_find_property(bl, "su2-feedback-voltage", NULL)) {
> +			pdata->su2_feedback = AS3711_SU2_VOLTAGE;
> +			count++;
> +		}
> +		if (of_find_property(bl, "su2-feedback-curr1", NULL)) {
> +			pdata->su2_feedback = AS3711_SU2_CURR1;
> +			count++;
> +		}
> +		if (of_find_property(bl, "su2-feedback-curr2", NULL)) {
> +			pdata->su2_feedback = AS3711_SU2_CURR2;
> +			count++;
> +		}
> +		if (of_find_property(bl, "su2-feedback-curr3", NULL)) {
> +			pdata->su2_feedback = AS3711_SU2_CURR3;
> +			count++;
> +		}
> +		if (of_find_property(bl, "su2-feedback-curr-auto", NULL)) {
> +			pdata->su2_feedback = AS3711_SU2_CURR_AUTO;
> +			count++;
> +		}
> +		if (count != 1)
> +			return -EINVAL;

This looks odd.  If the firmware provides both su2-feedback-voltage and
su2-feedback-curr1, we fail?  Firmware developers are notoriously
flakey - can the code be more defensive here?

> +		count = 0;
> +		if (of_find_property(bl, "su2-fbprot-lx-sd4", NULL)) {
> +			pdata->su2_fbprot = AS3711_SU2_LX_SD4;
> +			count++;
> +		}
> +		if (of_find_property(bl, "su2-fbprot-gpio2", NULL)) {
> +			pdata->su2_fbprot = AS3711_SU2_GPIO2;
> +			count++;
> +		}
> +		if (of_find_property(bl, "su2-fbprot-gpio3", NULL)) {
> +			pdata->su2_fbprot = AS3711_SU2_GPIO3;
> +			count++;
> +		}
> +		if (of_find_property(bl, "su2-fbprot-gpio4", NULL)) {
> +			pdata->su2_fbprot = AS3711_SU2_GPIO4;
> +			count++;
> +		}
> +		if (count != 1)
> +			return -EINVAL;
> +
> +		count = 0;
> +		if (of_find_property(bl, "su2-auto-curr1", NULL)) {
> +			pdata->su2_auto_curr1 = true;
> +			count++;
> +		}
> +		if (of_find_property(bl, "su2-auto-curr2", NULL)) {
> +			pdata->su2_auto_curr2 = true;
> +			count++;
> +		}
> +		if (of_find_property(bl, "su2-auto-curr3", NULL)) {
> +			pdata->su2_auto_curr3 = true;
> +			count++;
> +		}
> +
> +		/*
> +		 * At least one su2-auto-curr* must be specified iff
> +		 * AS3711_SU2_CURR_AUTO is used
> +		 */
> +		if (!count ^ (pdata->su2_feedback != AS3711_SU2_CURR_AUTO))
> +			return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
>
> ...
>

Re: [PATCH v3 3/3] backlight: as3711: add OF support

[Ryan Mallon]

On 26/03/13 09:40, Andrew Morton wrote:
> On Fri, 22 Mar 2013 17:15:49 +0100 Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:
> 
>> Add support for configuring AS3711 backlight driver from DT.
>>
>> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
>> Reviwed-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
>> ---
>>  drivers/video/backlight/as3711_bl.c |  118 ++++++++++++++++++++++++++++++++++-
>>  1 files changed, 117 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/video/backlight/as3711_bl.c b/drivers/video/backlight/as3711_bl.c
>> index 41d52fe..123887c 100644
>> --- a/drivers/video/backlight/as3711_bl.c
>> +++ b/drivers/video/backlight/as3711_bl.c
>> @@ -258,6 +258,109 @@ static int as3711_bl_register(struct platform_device *pdev,
>>  	return 0;
>>  }
>>  
>> +static int as3711_backlight_parse_dt(struct device *dev)
>> +{
>> +	struct as3711_bl_pdata *pdata = dev_get_platdata(dev);
>> +	struct device_node *bl =
>> +		of_find_node_by_name(dev->parent->of_node, "backlight"), *fb;
>> +	int ret;
> 
> It's tidier to do
> 
> 	struct device_node *bl;
> 	
> 	bl = of_find_node_by_name(dev->parent->of_node, "backlight"), *fb;
> 
> and avoid the 80-col trickery.

The other reason being that it now becomes much more apparent that *fb
is not an argument to of_find_node_by_name(), but a second variable of
type struct device_node :-).

~Ryan