ARM: pxa: fix build failure for regulator consumer in em-x270.c

Commit 737f360d5bef5e01c6cfa755dca0b449a154c1e0 (linux-next)

"regulator: Remove support for supplies specified by struct device"

caused this file to break, since it was still relying on the device field to be present. Strip them out here too.

Signed-off-by Paul Gortmaker <[email protected]>

CC Mark Brown <[email protected]>

arch/arm/mach-pxa/em-x270.c +7 -8

index d80c0ba..c8f3293 100644
@@ -1083,20 +1083,19 @@ static void __init em_x270_userspace_consumers_init(void)
 }
 
 /* DA9030 related initializations */
-#define REGULATOR_CONSUMER(_name, _dev, _supply)			       \
+#define REGULATOR_CONSUMER(_name, _supply)			       \
 	static struct regulator_consumer_supply _name##_consumers[] = {	\
 		{							\
-			.dev = _dev,					\
 			.supply = _supply,				\
 		},							\
 	}
 
-REGULATOR_CONSUMER(ldo3, &em_x270_gps_userspace_consumer.dev, "vcc gps");
-REGULATOR_CONSUMER(ldo5, NULL, "vcc cam");
-REGULATOR_CONSUMER(ldo10, &pxa_device_mci.dev, "vcc sdio");
-REGULATOR_CONSUMER(ldo12, NULL, "vcc usb");
-REGULATOR_CONSUMER(ldo19, &em_x270_gprs_userspace_consumer.dev, "vcc gprs");
-REGULATOR_CONSUMER(buck2, NULL, "vcc_core");
+REGULATOR_CONSUMER(ldo3, "vcc gps");
+REGULATOR_CONSUMER(ldo5, "vcc cam");
+REGULATOR_CONSUMER(ldo10, "vcc sdio");
+REGULATOR_CONSUMER(ldo12, "vcc usb");
+REGULATOR_CONSUMER(ldo19, "vcc gprs");
+REGULATOR_CONSUMER(buck2, "vcc_core");
 
 #define REGULATOR_INIT(_ldo, _min_uV, _max_uV, _ops_mask)		\
 	static struct regulator_init_data _ldo##_data = {		\

Also Cc: Haojian Zhuang <[email protected]>

Quoted 9 lines · Paul Gortmaker

Commit 737f360d5bef5e01c6cfa755dca0b449a154c1e0 (linux-next)

? "regulator: Remove support for supplies specified by struct device"

caused this file to break, since it was still relying on the device field to be present. ?Strip them out here too.

Signed-off-by Paul Gortmaker <[email protected]>

CC Mark Brown <[email protected]>

You've missed the scissors line:
---

The patch itself:

Acked-by Igor Grinberg <[email protected]>

Thanks

Could you pick up this patch since it's depend on latest change on regulator?

Quoted 1 line · Haojian Zhuang

Could you pick up this patch since it's depend on latest change on regulator?

There's no direct dependency, there's been no need to use that struct member for years - should be safe to just apply via the PXA tree. ?If you do want it to go via regulator please resend, I didn't keep a copy of the patch.

?#define REGULATOR_INIT(_ldo, _min_uV, _max_uV, _ops_mask) ? ? ? ? ? ? ?\
? ? ? ?static struct regulator_init_data _ldo##_data = { ? ? ? ? ? ? ? \
--
1.7.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-next" in the body of a message to majordomo at vger.kernel.org
More majordomo info at ?http://vger.kernel.org/majordomo-info.html

Commit 737f360d5bef5e01c6cfa755dca0b449a154c1e0 (linux-next)

"regulator: Remove support for supplies specified by struct device"

caused this file to break, since it was still relying on the device field to be present. Update them to use the new dev_name entries instead.

Signed-off-by Paul Gortmaker <[email protected]>

CC Mark Brown <[email protected]>

---

[v2: replace dev with dev_name instead of just deleting dev.
Note however there is dev_name overlap; not sure if that matters? ]

arch/arm/mach-pxa/em-x270.c +1 -1

index c1b65da..0ffc100 100644
@@ -1083,19 +1083,19 @@ static void __init em_x270_userspace_consumers_init(void)
 }
 
 /* DA9030 related initializations */
-#define REGULATOR_CONSUMER(_name, _dev, _supply)			       \
+#define REGULATOR_CONSUMER(_name, _dev_name, _supply)		        \

Quoted 1 line · Paul Gortmaker

+REGULATOR_CONSUMER(ldo3, "reg-userspace-consumer", "vcc gps");

If you make this:
REGULATOR_CONSUMER(ldo3, NULL, "vcc gps");

Quoted 1 line

+REGULATOR_CONSUMER(ldo19, "reg-userspace-consumer", "vcc gprs");

and this:
REGULATOR_CONSUMER(ldo19, NULL, "vcc gprs");

Quoted 6 lines · Igor Grinberg

Quoted 1 line · Paul Gortmaker

+REGULATOR_CONSUMER(ldo3, "reg-userspace-consumer", "vcc gps");

If you make this:
REGULATOR_CONSUMER(ldo3, NULL, "vcc gps");

Quoted 4 lines

Quoted 1 line

+REGULATOR_CONSUMER(ldo19, "reg-userspace-consumer", "vcc gprs");

and this:
REGULATOR_CONSUMER(ldo19, NULL, "vcc gprs");

These don't look like good fixes, you should be specifying the dev_name() for the consumer device? ?Presumably it's two separate consumers and should be .0 and .1 or something?

Quoted 6 lines · Igor Grinberg

Quoted 1 line · Paul Gortmaker

+REGULATOR_CONSUMER(ldo3, "reg-userspace-consumer", "vcc gps");

If you make this:
REGULATOR_CONSUMER(ldo3, NULL, "vcc gps");

Quoted 4 lines

Quoted 1 line

+REGULATOR_CONSUMER(ldo19, "reg-userspace-consumer", "vcc gprs");

and this:
REGULATOR_CONSUMER(ldo19, NULL, "vcc gprs");

These don't look like good fixes, you should be specifying the dev_name() for the consumer device? Presumably it's two separate consumers and should be .0 and .1 or something?

On 12-03-28 11:27 AM, Mark Brown wrote:

static struct platform_device em_x270_gps_userspace_consumer = {
.name = "reg-userspace-consumer",
.id = 0,

static struct platform_device em_x270_gprs_userspace_consumer = {
.name = "reg-userspace-consumer",
.id = 1,

Note that the existing names currently don't incorporate the .id field as a suffix, and so never were unique.

Quoted 9 lines · Paul Gortmaker

On 12-03-28 11:27 AM, Mark Brown wrote:

static struct platform_device em_x270_gps_userspace_consumer = {
.name = "reg-userspace-consumer",
.id = 0,

static struct platform_device em_x270_gprs_userspace_consumer = {
.name = "reg-userspace-consumer",
.id = 1,

Quoted 2 lines

Note that the existing names currently don't incorporate the .id field as a suffix, and so never were unique.

No, this is just a basic part of how platform devices work - the device name is always the same and if you've got more than one of them they get different .ids. dev_name() returns name.id, or just name if id is set to -1 indicating that there's onyl one device of a given type.

Quoted 11 lines · Mark Brown

Quoted 9 lines · Paul Gortmaker

On 12-03-28 11:27 AM, Mark Brown wrote:

static struct platform_device em_x270_gps_userspace_consumer = {
.name = "reg-userspace-consumer",
.id = 0,

static struct platform_device em_x270_gprs_userspace_consumer = {
.name = "reg-userspace-consumer",
.id = 1,

Quoted 7 lines

Quoted 2 lines

Note that the existing names currently don't incorporate the .id field as a suffix, and so never were unique.

No, this is just a basic part of how platform devices work - the device name is always the same and if you've got more than one of them they get different .ids. dev_name() returns name.id, or just name if id is set to -1 indicating that there's onyl one device of a given type.

OK, so Igor - can you simply retest the v2 patch, but make the two trivial changes:

Quoted 7 lines · Paul Gortmaker

Quoted 7 lines

Quoted 7 lines

static struct platform_device em_x270_gps_userspace_consumer = {
.name = "reg-userspace-consumer",
.id = 0,

static struct platform_device em_x270_gprs_userspace_consumer = {
.name = "reg-userspace-consumer",
.id = 1,

Review

Quoted 2 lines

(quoted diff) +1 -1

-REGULATOR_CONSUMER(ldo19, "reg-userspace-consumer", "vcc gprs");
+REGULATOR_CONSUMER(ldo19, "reg-userspace-consumer.1", "vcc gprs");

Well, I thought of this solution, but I don't like it, as it makes the whole thing very fragile and sensitive to the reg-userspace-consumer platform device registration order and count, isn't it? (That's why I proposed to use NULL...).

So, Mark, how do you think the above issues can be handled without putting NULL into the dev_name?

Quoted 10 lines · Igor Grinberg

Quoted 7 lines · Paul Gortmaker

Quoted 7 lines

Quoted 7 lines

static struct platform_device em_x270_gps_userspace_consumer = {
.name = "reg-userspace-consumer",
.id = 0,

static struct platform_device em_x270_gprs_userspace_consumer = {
.name = "reg-userspace-consumer",
.id = 1,

Quoted 7 lines

Review

Quoted 2 lines

(quoted diff) +1 -1

-REGULATOR_CONSUMER(ldo19, "reg-userspace-consumer", "vcc gprs");
+REGULATOR_CONSUMER(ldo19, "reg-userspace-consumer.1", "vcc gprs");

Well, I thought of this solution, but I don't like it, as it makes the whole thing very fragile and sensitive to the reg-userspace-consumer platform device registration order and count, isn't it? (That's why I proposed to use NULL...).

No the platform device numbering should be totally stable for a given board unless someone deliberately sets out to renumber them - the .ids are explicitly assigned by the board when it registers the device.

Quoted 2 lines

So, Mark, how do you think the above issues can be handled without putting NULL into the dev_name?

It shouldn't be a problem I think.

Would you like me to send a v3 with the .0 and .1 added, or are you OK with making that small change to v2 yourself?

Commit 737f360d5bef5e01c6cfa755dca0b449a154c1e0

?"regulator: Remove support for supplies specified by struct device"

caused this file to break, since it was still relying on the device field to be present. ?Map it onto dev_name appropriately

Since there are two consumers with the name "reg-userspace-consumer", we have to supply the ID as a suffix in the REGULATOR_CONSUMER calls.

Signed-off-by Paul Gortmaker <[email protected]>

CC Mark Brown <[email protected]>

---

[v3: add .0/.1 ID suffix to "reg-userspace-consumer" instances ]

arch/arm/mach-pxa/em-x270.c +0 -0

index c1b65da..16ec557 100644
@@ -1083,19 +1083,19 @@ static void __init em_x270_userspace_consumers_init(void)
 ?}
 

?/* DA9030 related initializations */

arch/arm/mach-pxa/em-x270.c +1 -1

-#define REGULATOR_CONSUMER(_name, _dev, _supply) ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
+#define REGULATOR_CONSUMER(_name, _dev_name, _supply) ? ? ? ? ? ? ? ? ?\

? ? ? ?static struct regulator_consumer_supply _name##_consumers[] = { \
? ? ? ? ? ? ? ?{ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
- ? ? ? ? ? ? ? ? ? ? ? .dev = _dev, ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
+ ? ? ? ? ? ? ? ? ? ? ? .dev_name = _dev_name, ? ? ? ? ? ? ? ? ? ? ? ? ?\
? ? ? ? ? ? ? ? ? ? ? ?.supply = _supply, ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
? ? ? ? ? ? ? ?}, ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
? ? ? ?}

arch/arm/mach-pxa/em-x270.c +1 -1

-REGULATOR_CONSUMER(ldo3, &em_x270_gps_userspace_consumer.dev, "vcc gps");
+REGULATOR_CONSUMER(ldo3, "reg-userspace-consumer.0", "vcc gps");

?REGULATOR_CONSUMER(ldo5, NULL, "vcc cam");

arch/arm/mach-pxa/em-x270.c +1 -1

-REGULATOR_CONSUMER(ldo10, &pxa_device_mci.dev, "vcc sdio");
+REGULATOR_CONSUMER(ldo10, "pxa2xx-mci", "vcc sdio");

?REGULATOR_CONSUMER(ldo12, NULL, "vcc usb");

arch/arm/mach-pxa/em-x270.c +1 -1

-REGULATOR_CONSUMER(ldo19, &em_x270_gprs_userspace_consumer.dev, "vcc gprs");
+REGULATOR_CONSUMER(ldo19, "reg-userspace-consumer.1", "vcc gprs");

?REGULATOR_CONSUMER(buck2, NULL, "vcc_core");

?#define REGULATOR_INIT(_ldo, _min_uV, _max_uV, _ops_mask) ? ? ? ? ? ? ?\
--
1.7.9.1

Commit 737f360d5bef5e01c6cfa755dca0b449a154c1e0

"regulator: Remove support for supplies specified by struct device"

caused this file to break, since it was still relying on the device field to be present. Map it onto dev_name appropriately

Since there are two consumers with the name "reg-userspace-consumer", we have to supply the ID as a suffix in the REGULATOR_CONSUMER calls.

Signed-off-by Paul Gortmaker <[email protected]>

CC Mark Brown <[email protected]>

---

[v3: add .0/.1 ID suffix to "reg-userspace-consumer" instances ]

arch/arm/mach-pxa/em-x270.c +2 -3

index c1b65da..16ec557 100644
@@ -1083,19 +1083,19 @@ static void __init em_x270_userspace_consumers_init(void)
 }
 
 /* DA9030 related initializations */
-#define REGULATOR_CONSUMER(_name, _dev, _supply)			       \
+#define REGULATOR_CONSUMER(_name, _dev_name, _supply)		        \
 	static struct regulator_consumer_supply _name##_consumers[] = {	\
 		{							\
-			.dev = _dev,					\
+			.dev_name = _dev_name,				\
 			.supply = _supply,				\
 		},							\
 	}
 
-REGULATOR_CONSUMER(ldo3, &em_x270_gps_userspace_consumer.dev, "vcc gps");

Quoted 26 lines · Paul Gortmaker

Quoted 11 lines · Mark Brown

Quoted 9 lines · Paul Gortmaker

On 12-03-28 11:27 AM, Mark Brown wrote:

static struct platform_device em_x270_gps_userspace_consumer = {
.name = "reg-userspace-consumer",
.id = 0,

static struct platform_device em_x270_gprs_userspace_consumer = {
.name = "reg-userspace-consumer",
.id = 1,

Quoted 7 lines

Quoted 2 lines

Note that the existing names currently don't incorporate the .id field as a suffix, and so never were unique.

No, this is just a basic part of how platform devices work - the device name is always the same and if you've got more than one of them they get different .ids. dev_name() returns name.id, or just name if id is set to -1 indicating that there's onyl one device of a given type.

OK, so Igor - can you simply retest the v2 patch, but make the two trivial changes:

(quoted diff) +1 -1

-REGULATOR_CONSUMER(ldo3, "reg-userspace-consumer", "vcc gps");
+REGULATOR_CONSUMER(ldo3, "reg-userspace-consumer.0", "vcc gps");

Review

Quoted 2 lines

(quoted diff) +1 -1

-REGULATOR_CONSUMER(ldo19, "reg-userspace-consumer", "vcc gprs");
+REGULATOR_CONSUMER(ldo19, "reg-userspace-consumer.1", "vcc gprs");

Well, I thought of this solution, but I don't like it, as it makes the whole thing very fragile and sensitive to the reg-userspace-consumer platform device registration order and count, isn't it?

(That's why I proposed to use NULL...).

So, Mark, how do you think the above issues can be handled without putting NULL into the dev_name?

Quoted 27 lines · Igor Grinberg

Quoted 26 lines · Paul Gortmaker

Quoted 11 lines · Mark Brown

Quoted 9 lines · Paul Gortmaker

On 12-03-28 11:27 AM, Mark Brown wrote:

static struct platform_device em_x270_gps_userspace_consumer = {
.name = "reg-userspace-consumer",
.id = 0,

static struct platform_device em_x270_gprs_userspace_consumer = {
.name = "reg-userspace-consumer",
.id = 1,

Quoted 7 lines

Quoted 2 lines

Note that the existing names currently don't incorporate the .id field as a suffix, and so never were unique.

No, this is just a basic part of how platform devices work - the device name is always the same and if you've got more than one of them they get different .ids. dev_name() returns name.id, or just name if id is set to -1 indicating that there's onyl one device of a given type.

OK, so Igor - can you simply retest the v2 patch, but make the two trivial changes:

(quoted diff) +1 -1

-REGULATOR_CONSUMER(ldo3, "reg-userspace-consumer", "vcc gps");
+REGULATOR_CONSUMER(ldo3, "reg-userspace-consumer.0", "vcc gps");

Quoted 6 lines

Review

Quoted 2 lines

(quoted diff) +1 -1

-REGULATOR_CONSUMER(ldo19, "reg-userspace-consumer", "vcc gprs");
+REGULATOR_CONSUMER(ldo19, "reg-userspace-consumer.1", "vcc gprs");

Well, I thought of this solution, but I don't like it, as it makes the whole thing very fragile and sensitive to the reg-userspace-consumer platform device registration order and count, isn't it?

Why would it be order dependent? You can see in the above structs that they explicitly call out an ID value -- it is _not_ chosen at the time of registration. So it will always be:

reg-userspace-consumer.0  -->  gps consumer
reg-userspace-consumer.1  -->  gprs consumer

At least that is my (limited) understanding, based on what Mark was saying.