Skip to contentSkip to conversation

ux500: add a few AB8500 regulator consumers

PATCH New Linus Walleij 4 participants 7 messages Jan 17, 2011 – Jan 20, 2011

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 ux500: add a few AB8500 regulator consumers

Try to make the regulators a little bit more useful by adding some of the most basic consumers we're going to have in the end.

---
 arch/arm/mach-ux500/board-mop500-regulators.c |   93 +++++++++++++++++++++++++
 1 files changed, 93 insertions(+), 0 deletions(-)
arch/arm/mach-ux500/board-mop500-regulators.c +93 -0
index 533967c..b6b9399 100644
@@ -11,6 +11,87 @@
 #include <linux/kernel.h>
 #include <linux/regulator/machine.h>
 #include <linux/regulator/ab8500.h>
+#include "board-mop500-regulators.h"
+
+static struct regulator_consumer_supply ab8500_vaux1_consumers[] = {
+	{
+		.dev = NULL,
+		.supply = "v-display",
+	},
+	{
+		.dev_name = "bh1780",
+		.supply = "v-als",
+	},
+	{
+		.dev_name = "lsm303dlh.0",
+		.supply = "v-accel",
+	},
+	{
+		.dev_name = "lsm303dlh.1",
+		.supply = "v-mag",
+	},
+	{
+		.dev = NULL,
+		.supply = "v-mmio-camera",
+	},
+	{
+		.dev_name = "bu21013_ts.0",
+		.supply = "v-touch",
+	},
+	{
+		.dev_name = "bu21013_ts.1",
+		.supply = "v-touch",
+	},
+	{
+		.dev_name = "synaptics_rmi4_i2c0",
+		.supply = "v-touch",
+	},
+};
+
+static struct regulator_consumer_supply ab8500_vaux2_consumers[] = {
+	{
+		.dev_name = "sdi4",
+		.supply = "vmmc",
+	},
+	{
+		.dev_name = "ab8500-codec.0",
+		.supply = "vcc-N2158",
+	},
+};
+
+static struct regulator_consumer_supply ab8500_vaux3_consumers[] = {
+	{
+		.dev_name = "sdi0",
+		.supply = "vmmc"
+	},
+};
+
+static struct regulator_consumer_supply ab8500_vtvout_consumers[] = {
+	{
+		.supply = "v-tvout",
+	},
+	{
+		.supply = "ab8500-gpadc",
+	},
+};
+
+static struct regulator_consumer_supply ab8500_vintcore_consumers[] = {
+	{
+		.supply = "v-intcore",
+	},
+	{
+		.dev_name = "ab8500-usb.0",
+		.supply = "v-intcore",
+	},
+
+};
+
+static struct regulator_consumer_supply ab8500_vana_consumers[] = {
+	{
+		.dev_name = "mcde",
+		.supply = "v-ana",
+	},
+};
 
 /* AB8500 regulators */
 struct regulator_init_data ab8500_regulators[AB8500_NUM_REGULATORS] = {
@@ -23,6 +104,8 @@ struct regulator_init_data ab8500_regulators[AB8500_NUM_REGULATORS] = {
 			.valid_ops_mask = REGULATOR_CHANGE_VOLTAGE |
 					  REGULATOR_CHANGE_STATUS,
 		},
+		.num_consumer_supplies = ARRAY_SIZE(ab8500_vaux1_consumers),
+		.consumer_supplies = ab8500_vaux1_consumers,
 	},
 	/* supplies to the on-board eMMC */
 	[AB8500_LDO_AUX2] = {
@@ -33,6 +116,8 @@ struct regulator_init_data ab8500_regulators[AB8500_NUM_REGULATORS] = {
 			.valid_ops_mask = REGULATOR_CHANGE_VOLTAGE |
 					  REGULATOR_CHANGE_STATUS,
 		},
+		.num_consumer_supplies = ARRAY_SIZE(ab8500_vaux2_consumers),
+		.consumer_supplies = ab8500_vaux2_consumers,
 	},
 	/* supply for VAUX3, supplies to SDcard slots */
 	[AB8500_LDO_AUX3] = {
@@ -43,6 +128,8 @@ struct regulator_init_data ab8500_regulators[AB8500_NUM_REGULATORS] = {
 			.valid_ops_mask = REGULATOR_CHANGE_VOLTAGE |
 					  REGULATOR_CHANGE_STATUS,
 		},
+		.num_consumer_supplies = ARRAY_SIZE(ab8500_vaux3_consumers),
+		.consumer_supplies = ab8500_vaux3_consumers,
 	},
 	/* supply for tvout, gpadc, TVOUT LDO */
 	[AB8500_LDO_TVOUT] = {
@@ -50,6 +137,8 @@ struct regulator_init_data ab8500_regulators[AB8500_NUM_REGULATORS] = {
 			.name = "V-TVOUT",
 			.valid_ops_mask = REGULATOR_CHANGE_STATUS,
 		},
+		.num_consumer_supplies = ARRAY_SIZE(ab8500_vtvout_consumers),
+		.consumer_supplies = ab8500_vtvout_consumers,
 	},
 	/* supply for ab8500-vaudio, VAUDIO LDO */
 	[AB8500_LDO_AUDIO] = {
@@ -85,6 +174,8 @@ struct regulator_init_data ab8500_regulators[AB8500_NUM_REGULATORS] = {
 			.name = "V-INTCORE",
 			.valid_ops_mask = REGULATOR_CHANGE_STATUS,
 		},
+		.num_consumer_supplies = ARRAY_SIZE(ab8500_vintcore_consumers),
+		.consumer_supplies = ab8500_vintcore_consumers,
 	},
 	/* supply for U8500 CSI/DSI, VANA LDO */
 	[AB8500_LDO_ANA] = {
@@ -92,5 +183,7 @@ struct regulator_init_data ab8500_regulators[AB8500_NUM_REGULATORS] = {
 			.name = "V-CSI/DSI",
 			.valid_ops_mask = REGULATOR_CHANGE_STATUS,
 		},
+		.num_consumer_supplies = ARRAY_SIZE(ab8500_vana_consumers),
+		.consumer_supplies = ab8500_vana_consumers,
 	},
 };
Signature · 1 line
1.7.3.2
#1 lore raw

PATCH 1/1 ux500: add a few AB8500 regulator consumers

arch/arm/mach-ux500/board-mop500-regulators.c line 20

Quoted 5 lines · Linus Walleij
arch/arm/mach-ux500/board-mop500-regulators.c +5 -0
+static struct regulator_consumer_supply ab8500_vaux1_consumers[] = {
+	{
+		.dev = NULL,
+		.supply = "v-display",
+	},

Your use of NULL where there's no device in this patch is somewhat erratic.

arch/arm/mach-ux500/board-mop500-regulators.c line 24

Quoted 4 lines
arch/arm/mach-ux500/board-mop500-regulators.c +4 -0
+	{
+		.dev_name = "bh1780",
+		.supply = "v-als",
+	},

This supply name (like many of the others) looks a bit odd, it's an unusual idiom. Indeed, looking at (first hit in Google):

www.chipcatalog.com/ROHM/BH1780GLI.htm

it appears that the chip has one supply called VCC (subscripted CC) - I'd therefore expect the driver to be asking for the supply using that name. It looks like most of the supplies here have been given the names for the rail on the board rather than the name the chip expects for them.

arch/arm/mach-ux500/board-mop500-regulators.c line 75

Quoted 7 lines
arch/arm/mach-ux500/board-mop500-regulators.c +7 -0
+static struct regulator_consumer_supply ab8500_vtvout_consumers[] = {
+	{
+		.supply = "v-tvout",
+	},
+	{
+		.supply = "ab8500-gpadc",
+	},

I suspect there's a device for the ab8500?

#2 lore raw

PATCH 1/1 ux500: add a few AB8500 regulator consumers

2011/1/17 Mark Brown <[email protected]>:

Quoted 10 lines · Linus Walleij
Quoted 6 lines

+static struct regulator_consumer_supply ab8500_vaux1_consumers[] = {
+ ? ? {
+ ? ? ? ? ? ? .dev = NULL,
+ ? ? ? ? ? ? .supply = "v-display",
+ ? ? },

Your use of NULL where there's no device in this patch is somewhat erratic.

Yes, you're right, hm, I'll assign this to the MCDE (Multi-Channel Display Engine) instead.

Right, Robert/Jimmy?

Quoted 7 lines
Quoted 4 lines

+ ? ? {
+ ? ? ? ? ? ? .dev_name = "bh1780",
+ ? ? ? ? ? ? .supply = "v-als",
+ ? ? },

This supply name (like many of the others) looks a bit odd, it's an unusual idiom. ?Indeed, looking at (first hit in Google):

I'll change it for "vcc", the driver in drivers/misc/bh1780gli.c does not yet utilize regulators so now the authors of that driver need to look into requesting "vcc".

Lee, can you look into adding optional regulator support for BH1780GLI, and then use "vcc" for power supply...

Quoted 9 lines
Quoted 7 lines

+static struct regulator_consumer_supply ab8500_vtvout_consumers[] = {
+ ? ? {
+ ? ? ? ? ? ? .supply = "v-tvout",
+ ? ? },
+ ? ? {
+ ? ? ? ? ? ? .supply = "ab8500-gpadc",
+ ? ? },

I suspect there's a device for the ab8500?

This is not the AB8500 actually, but a subdevice (MFD cell) for it that creates its own platform device and is separately powered. (Though using in-chip wiring I presume.)

Ther driver is public but not mainlined:
http://git.linaro.org/gitweb?p=bsp/st-ericsson/linux-2.6.35-ux500.git;a=blob;f=drivers/misc/ab8500_gpadc.c;h=d27ccce90466f21e961975bf3868f9e0614545cf;hb=HEAD

I think the proper thing to add is .dev_name = "ab8500-gpadc.0". The name of the supply is "vddadc" on the datasheet so I'll change to this.

http://git.linaro.org/gitweb?p=bsp/st-ericsson/linux-2.6.35-ux500.git;a=blob;f=drivers/misc/ab8500_gpadc.c;h=d27ccce90466f21e961975bf3868f9e0614545cf;hb=HEAD

Yours,
Linus Walleij

#3 lore raw

PATCH 1/1 ux500: add a few AB8500 regulator consumers

Quoted 11 lines · Linus Walleij

2011/1/17 Mark Brown <[email protected]>:

Quoted 5 lines
Quoted 3 lines

+ ? ? {
+ ? ? ? ? ? ? .supply = "ab8500-gpadc",
+ ? ? },

I suspect there's a device for the ab8500?

This is not the AB8500 actually, but a subdevice (MFD cell) for it that creates its own platform device and is separately powered. (Though using in-chip wiring I presume.)

It may be more sane to just have the subdevices use the core struct device for requests for legibility.

#4 lore raw

PATCH 1/1 ux500: add a few AB8500 regulator consumers

2011/1/17 Mark Brown <[email protected]>:

Quoted 7 lines · Linus Walleij
Quoted 5 lines

2011/1/17 Mark Brown <[email protected]>:

Quoted 3 lines
Quoted 3 lines

+ ? ? {
+ ? ? ? ? ? ? .supply = "ab8500-gpadc",
+ ? ? },

Quoted 8 lines
Quoted 5 lines
Quoted 1 line

I suspect there's a device for the ab8500?

This is not the AB8500 actually, but a subdevice (MFD cell) for it that creates its own platform device and is separately powered. (Though using in-chip wiring I presume.)

It may be more sane to just have the subdevices use the core struct device for requests for legibility.

I'm not sure I'm following, so help me out...

I sort of like the detail I get in sysfs for the regulators, for example I can see in

/sys/devices/platform/ab8500-i2c.0/ab8500-regulator.0/regulator/regulator.3 sdi0-vmmc -> ../../../../../sdi0

Aha so sdi0 is using this regulator.

If all AB8500 subdevices were to ask for regulators using the core device there would be just one big entry pointing to the ab8500-i2c.0 entry in this case, and a huge usecount, which is not helpful IMO. I like the detail.

We have scripts extracting that info as well.

Therefore I prefer to register each consumer to the subdevice...

BTW I also checked that this one line is not internal to the AB8500, it's an external pin, which is then connected back to one of the regulators provided by the same chip, but could as well be some other regulator.

Yours,
Linus Walleij

#5 lore raw

PATCH 1/1 ux500: add a few AB8500 regulator consumers

Quoted 4 lines · Linus Walleij

If all AB8500 subdevices were to ask for regulators using the core device there would be just one big entry pointing to the ab8500-i2c.0 entry in this case, and a huge usecount, which is not helpful IMO. I like the detail.

It can make life easier from the point of view of setting stuff up since you're not reliant on the internals of the MFD - for example, if you fix the MFD to not create .0 devices you'll end up needing to update all the users.

The supply names are generally relatively obvious.

#6 lore raw

PATCH 1/1 ux500: add a few AB8500 regulator consumers

In reply to Linus Walleij
Quoted 12 lines · Linus Walleij

+ ? ? ? {
+ ? ? ? ? ? ? ? .dev_name = "bh1780",
+ ? ? ? ? ? ? ? .supply = "v-als",
+ ? ? ? },
+ ? ? ? {
+ ? ? ? ? ? ? ? .dev_name = "lsm303dlh.0",
+ ? ? ? ? ? ? ? .supply = "v-accel",
+ ? ? ? },
+ ? ? ? {
+ ? ? ? ? ? ? ? .dev_name = "lsm303dlh.1",
+ ? ? ? ? ? ? ? .supply = "v-mag",
+ ? ? ? },

Quoted 8 lines

+ ? ? ? {
+ ? ? ? ? ? ? ? .dev_name = "bu21013_ts.0",
+ ? ? ? ? ? ? ? .supply = "v-touch",
+ ? ? ? },
+ ? ? ? {
+ ? ? ? ? ? ? ? .dev_name = "bu21013_ts.1",
+ ? ? ? ? ? ? ? .supply = "v-touch",
+ ? ? ? },

These device names are all wrong; they need to use the i2c client dev names. And the recent patches for regulator support in the bu driver use a different supply name.

Also, there's a macro to make this list more compact:
REGULATOR_SUPPLY().