pmbus: ltc2978: add regulator support

From PMBus_Specification_Part_II_Rev_1-3_20140318.pdf:

From: Alan Tull <[email protected]>

Add device tree bindings documentation for ltc2978.

Signed-off-by Alan Tull <[email protected]>

Cc Guenter Roeck <[email protected]>

Cc Mark Rutland <[email protected]>

Cc Mark Brown <[email protected]>

---
v2: clean whitespace

v3: list compatible strings in single column
add vendor as lltc
mention regulators.txt instead of documenting regulator-name s/vout_en/vout/
remove size_cells, address-cells
use node name to identify regulators

---
 .../devicetree/bindings/hwmon/ltc2978.txt          |   39 ++++++++++++++++++++
 1 file changed, 39 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/ltc2978.txt

Documentation/devicetree/bindings/hwmon/ltc2978.txt new file +19 -0

new file mode 100644
index 0000000..16c1227
@@ -0,0 +1,39 @@
+ltc2978
+
+Required properties:
+- compatible: should contain one of:
+  * "lltc,ltc2974"
+  * "lltc,ltc2977"
+  * "lltc,ltc2978"
+  * "lltc,ltc3880"
+  * "lltc,ltc3883"
+  * "lltc,ltm4676"
+- reg: I2C slave address
+
+Optional properties:
+- regulators: A node that houses a sub-node for each regulator controlled by
+  the device. Each sub-node is identified using the node's name, with valid
+  values listed below. The content of each sub-node is defined by the
+  standard binding for regulators; see regulator.txt.
+
+Valid names of regulators depend on number of supplies supported per device:

Documentation/devicetree/bindings/hwmon/ltc2978.txt new file +10 -0

+	reg = <0x5e>;
+	regulators {
+		vout0 {
+			regulator-name = "FPGA-2.5V";
+		};
+		vout2 {
+			regulator-name = "FPGA-1.5V";
+		};
+	};
+};

Signature · 1 line

1.7.9.5

From: Alan Tull <[email protected]>

Add support for simple on/off control of each channel.

To add regulator support, the pmbus part driver needs to add regulator_desc information and number of regulators to its pmbus_driver_info struct.

regulator_desc can be declared using default macro for a regulator (PMBUS_REGULATOR) that is in pmbus.h

The regulator_init_data can be initialized from either platform data or the device tree.

Signed-off-by Alan Tull <[email protected]>

Reviewed-by Mark Brown <[email protected]>

Cc Guenter Roeck <[email protected]>

Quoted 11 lines · On Wed, Oct 15, 2014 at 01:55:09PM -0500…

From: Alan Tull <[email protected]>

Add support for simple on/off control of each channel.

To add regulator support, the pmbus part driver needs to add regulator_desc information and number of regulators to its pmbus_driver_info struct.

regulator_desc can be declared using default macro for a regulator (PMBUS_REGULATOR) that is in pmbus.h

Quoted 6 lines

The regulator_init_data can be initialized from either platform data or the device tree.

Signed-off-by Alan Tull <[email protected]>

Reviewed-by Mark Brown <[email protected]>

Cc Guenter Roeck <[email protected]>

Hi Alan,

I am still seeing lots of the following:

vout0: Failed to create debugfs directory
vout1: Failed to create debugfs directory
vout2: Failed to create debugfs directory
vout3: Failed to create debugfs directory
vout4: Failed to create debugfs directory
vout5: Failed to create debugfs directory
vout6: Failed to create debugfs directory
vout7: Failed to create debugfs directory

I thought there was a problem in the regulator core, but after looking into it concluded that the regulator core _should_ prepend the names with the device name when creating the debugfs entries, unless no device name is specified. So something must be missing. We'll need to sort this out before I can accept the code.

Thanks,
Guenter

Quoted 38 lines · Guenter Roeck

Quoted 11 lines · On Wed, Oct 15, 2014 at 01:55:09PM -0500, [email protected] wrote:

From: Alan Tull <[email protected]>

Add support for simple on/off control of each channel.

To add regulator support, the pmbus part driver needs to add regulator_desc information and number of regulators to its pmbus_driver_info struct.

regulator_desc can be declared using default macro for a regulator (PMBUS_REGULATOR) that is in pmbus.h

Quoted 6 lines

The regulator_init_data can be initialized from either platform data or the device tree.

Signed-off-by Alan Tull <[email protected]>

Reviewed-by Mark Brown <[email protected]>

Cc Guenter Roeck <[email protected]>

Hi Alan,

I am still seeing lots of the following:

vout0: Failed to create debugfs directory
vout1: Failed to create debugfs directory
vout2: Failed to create debugfs directory
vout3: Failed to create debugfs directory
vout4: Failed to create debugfs directory
vout5: Failed to create debugfs directory
vout6: Failed to create debugfs directory
vout7: Failed to create debugfs directory

I thought there was a problem in the regulator core, but after looking into it concluded that the regulator core _should_ prepend the names with the device name when creating the debugfs entries, unless no device name is specified. So something must be missing. We'll need to sort this out before I can accept the code.

Quoted 3 lines

Thanks,
Guenter

Hi Guenter,

OK, I will look into it.

Quoted 38 lines · Guenter Roeck

Quoted 11 lines · On Wed, Oct 15, 2014 at 01:55:09PM -0500, [email protected] wrote:

From: Alan Tull <[email protected]>

Add support for simple on/off control of each channel.

To add regulator support, the pmbus part driver needs to add regulator_desc information and number of regulators to its pmbus_driver_info struct.

regulator_desc can be declared using default macro for a regulator (PMBUS_REGULATOR) that is in pmbus.h

Quoted 6 lines

The regulator_init_data can be initialized from either platform data or the device tree.

Signed-off-by Alan Tull <[email protected]>

Reviewed-by Mark Brown <[email protected]>

Cc Guenter Roeck <[email protected]>

Hi Alan,

I am still seeing lots of the following:

vout0: Failed to create debugfs directory
vout1: Failed to create debugfs directory
vout2: Failed to create debugfs directory
vout3: Failed to create debugfs directory
vout4: Failed to create debugfs directory
vout5: Failed to create debugfs directory
vout6: Failed to create debugfs directory
vout7: Failed to create debugfs directory

I thought there was a problem in the regulator core, but after looking into it concluded that the regulator core _should_ prepend the names with the device name when creating the debugfs entries, unless no device name is specified. So something must be missing. We'll need to sort this out before I can accept the code.

Quoted 3 lines

Thanks,
Guenter

Hi Guenter,

OK, I will look into it.

Quoted 40 lines · atull

Quoted 38 lines · Guenter Roeck

Quoted 11 lines · On Wed, Oct 15, 2014 at 01:55:09PM -0500, [email protected] wrote:

From: Alan Tull <[email protected]>

Add support for simple on/off control of each channel.

To add regulator support, the pmbus part driver needs to add regulator_desc information and number of regulators to its pmbus_driver_info struct.

regulator_desc can be declared using default macro for a regulator (PMBUS_REGULATOR) that is in pmbus.h

Quoted 6 lines

The regulator_init_data can be initialized from either platform data or the device tree.

Signed-off-by Alan Tull <[email protected]>

Reviewed-by Mark Brown <[email protected]>

Cc Guenter Roeck <[email protected]>

Hi Alan,

I am still seeing lots of the following:

vout0: Failed to create debugfs directory
vout1: Failed to create debugfs directory
vout2: Failed to create debugfs directory
vout3: Failed to create debugfs directory
vout4: Failed to create debugfs directory
vout5: Failed to create debugfs directory
vout6: Failed to create debugfs directory
vout7: Failed to create debugfs directory

I thought there was a problem in the regulator core, but after looking into it concluded that the regulator core _should_ prepend the names with the device name when creating the debugfs entries, unless no device name is specified. So something must be missing. We'll need to sort this out before I can accept the code.

Quoted 8 lines

Quoted 3 lines

Thanks,
Guenter

Hi Guenter,

OK, I will look into it.

Can you try the following patch ? It may not be perfect, but it does the job for me.

Mark, would that patch be acceptable ?

Thanks,
Guenter
---
From: Guenter Roeck <[email protected]>
Date: Thu, 16 Oct 2014 13:08:35 -0700
Subject: [PATCH] regulator: Ensure unique regulator debugfs directory names

If multiple regulator devices of the same type exist in a system, the regulator driver assigns generic names for the regulators it provides, and debugfs is enabled, the regulator subsystem attempts to create multiple entries with the same name in the regulator debugfs directory. This fails for all but the first regulator, resulting in multiple "Failed to create debugfs directory" log entries.

To avoid the problem, prepend the debugfs directory name for a regulator with its parent device name if available, but only if no explicit regulator name was provided.

Cc Alan Tull <[email protected]>

Signed-off-by Guenter Roeck <[email protected]>

---
 drivers/regulator/core.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

drivers/regulator/core.c +12 -1

index cd87c0c..92f7a53 100644
@@ -3538,7 +3538,18 @@ static int add_regulator_attributes(struct regulator_dev *rdev)
 
 static void rdev_init_debugfs(struct regulator_dev *rdev)
 {
-	rdev->debugfs = debugfs_create_dir(rdev_get_name(rdev), debugfs_root);
+	struct device *parent = rdev->dev.parent;
+	const char *rname = rdev_get_name(rdev);
+	char name[NAME_MAX];
+
+	/* Avoid duplicate debugfs directory names */
+	if (parent && rname == rdev->desc->name) {
+		snprintf(name, sizeof(name), "%s-%s", dev_name(parent),
+			 rname);
+		rname = name;
+	}
+
+	rdev->debugfs = debugfs_create_dir(rname, debugfs_root);
 	if (!rdev->debugfs) {
 		rdev_warn(rdev, "Failed to create debugfs directory\n");
 		return;

Signature · 1 line

1.9.1

Quoted 104 lines · Guenter Roeck

Quoted 40 lines · atull

Quoted 38 lines · Guenter Roeck

Quoted 11 lines · On Wed, Oct 15, 2014 at 01:55:09PM -0500, [email protected] wrote:

From: Alan Tull <[email protected]>

Add support for simple on/off control of each channel.

To add regulator support, the pmbus part driver needs to add regulator_desc information and number of regulators to its pmbus_driver_info struct.

regulator_desc can be declared using default macro for a regulator (PMBUS_REGULATOR) that is in pmbus.h

Quoted 6 lines

The regulator_init_data can be initialized from either platform data or the device tree.

Signed-off-by Alan Tull <[email protected]>

Reviewed-by Mark Brown <[email protected]>

Cc Guenter Roeck <[email protected]>

Hi Alan,

I am still seeing lots of the following:

vout0: Failed to create debugfs directory
vout1: Failed to create debugfs directory
vout2: Failed to create debugfs directory
vout3: Failed to create debugfs directory
vout4: Failed to create debugfs directory
vout5: Failed to create debugfs directory
vout6: Failed to create debugfs directory
vout7: Failed to create debugfs directory

I thought there was a problem in the regulator core, but after looking into it concluded that the regulator core _should_ prepend the names with the device name when creating the debugfs entries, unless no device name is specified. So something must be missing. We'll need to sort this out before I can accept the code.

Quoted 8 lines

Quoted 3 lines

Thanks,
Guenter

Hi Guenter,

OK, I will look into it.

Can you try the following patch ? It may not be perfect, but it does the job for me.

Mark, would that patch be acceptable ?

Thanks,
Guenter
---
From: Guenter Roeck <[email protected]>
Date: Thu, 16 Oct 2014 13:08:35 -0700
Subject: [PATCH] regulator: Ensure unique regulator debugfs directory names

If multiple regulator devices of the same type exist in a system, the regulator driver assigns generic names for the regulators it provides, and debugfs is enabled, the regulator subsystem attempts to create multiple entries with the same name in the regulator debugfs directory. This fails for all but the first regulator, resulting in multiple "Failed to create debugfs directory" log entries.

To avoid the problem, prepend the debugfs directory name for a regulator with its parent device name if available, but only if no explicit regulator name was provided.

Cc Alan Tull <[email protected]>

Signed-off-by Guenter Roeck <[email protected]>

---
  drivers/regulator/core.c | 13 ++++++++++++-
  1 file changed, 12 insertions(+), 1 deletion(-)

drivers/regulator/core.c +12 -2

index cd87c0c..92f7a53 100644
@@ -3538,7 +3538,18 @@ static int add_regulator_attributes(struct regulator_dev *rdev)
 
  static void rdev_init_debugfs(struct regulator_dev *rdev)
  {
-	rdev->debugfs = debugfs_create_dir(rdev_get_name(rdev), debugfs_root);
+	struct device *parent = rdev->dev.parent;
+	const char *rname = rdev_get_name(rdev);
+	char name[NAME_MAX];
+
+	/* Avoid duplicate debugfs directory names */
+	if (parent && rname == rdev->desc->name) {
+		snprintf(name, sizeof(name), "%s-%s", dev_name(parent),
+			 rname);
+		rname = name;
+	}
+
+	rdev->debugfs = debugfs_create_dir(rname, debugfs_root);
  	if (!rdev->debugfs) {
  		rdev_warn(rdev, "Failed to create debugfs directory\n");
  		return;
--

Quoted 2 lines

1.9.1

Hi Guenter,

I tried it out on a board with two ltc2978's. It looked really good to me. It solves the debugfs issue without affecting the DT.

root@socfpga_cyclone5:~# ls /sys/kernel/debug/regulator/
0-005c-vout0               0-005c-vout6               0-005e-vout7
0-005c-vout1               0-005c-vout7               FPGA-1.1V
0-005c-vout2               0-005e-vout1               FPGA-1.5V
0-005c-vout3               0-005e-vout3               FPGA-2.5V
0-005c-vout4               0-005e-vout5
reg-dummy-regulator-dummy
0-005c-vout5               0-005e-vout6               supply_map

Alan

From: Alan Tull <[email protected]>

Each output has individual on/off control.

New in v6:
* Much cleanup of the bindings document
* s/vout_en/vout/g

Hopefully this is getting closer. We will still have the potential problem of repeated node names on boards on boards that have many regulators of the same kind. If this can be handled in the regulator layer, that might be great. If we handle it here, whatever we come up with will have to keep the same name over reboots and be predictable enough to serve as regulator node names in the DT. I'm open for suggestions here.

This patchset now uses "regulator: of: Provide simplified DT parsing method" which are in the next-20141014 tag of linux-next.

Quoted 15 lines · On Wed, Oct 15, 2014 at 01:55:06PM -0500, [email protected] wrote:

From: Alan Tull <[email protected]>

Each output has individual on/off control.

New in v6:
* Much cleanup of the bindings document
* s/vout_en/vout/g

Hopefully this is getting closer. We will still have the potential problem of repeated node names on boards on boards that have many regulators of the same kind. If this can be handled in the regulator layer, that might be great. If we handle it here, whatever we come up with will have to keep the same name over reboots and be predictable enough to serve as regulator node names in the DT. I'm open for suggestions here.

Quoted 3 lines

This patchset now uses "regulator: of: Provide simplified DT parsing method" which are in the next-20141014 tag of linux-next.

Hi Alan,

Since the problems we have seen are in the regulator core code and not in your patches, I added the series to -next. I fixed up the 'compatible' example in the devicetree description, so there is no need to resubmit the patches.

Thanks,
Guenter