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

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

[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 <paul.gortmaker@windriver.com>
CC: Mark Brown <broonie@opensource.wolfsonmicro.com>

diff --git a/arch/arm/mach-pxa/em-x270.c b/arch/arm/mach-pxa/em-x270.c
index d80c0ba..c8f3293 100644
--- a/arch/arm/mach-pxa/em-x270.c
+++ b/arch/arm/mach-pxa/em-x270.c
@@ -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 = {		\
-- 
1.7.9.1

Re: [PATCH] ARM: pxa: fix build failure for regulator consumer in em-x270.c

[Igor Grinberg]

Also Cc: Haojian Zhuang <haojian.zhuang@marvell.com>

On 03/06/12 01:12, Paul Gortmaker wrote:
> 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 <paul.gortmaker@windriver.com>
> CC: Mark Brown <broonie@opensource.wolfsonmicro.com>

You've missed the scissors line:
---

The patch itself:

Acked-by: Igor Grinberg <grinberg@compulab.co.il>

Thanks

> 
> diff --git a/arch/arm/mach-pxa/em-x270.c b/arch/arm/mach-pxa/em-x270.c
> index d80c0ba..c8f3293 100644
> --- a/arch/arm/mach-pxa/em-x270.c
> +++ b/arch/arm/mach-pxa/em-x270.c
> @@ -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 = {		\

-- 
Regards,
Igor.

Re: [PATCH] ARM: pxa: fix build failure for regulator consumer in em-x270.c

[Haojian Zhuang]

On Tue, Mar 6, 2012 at 4:46 PM, Igor Grinberg <grinberg@compulab.co.il> wrote:
> Also Cc: Haojian Zhuang <haojian.zhuang@marvell.com>
>
> On 03/06/12 01:12, Paul Gortmaker wrote:
>> 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 <paul.gortmaker@windriver.com>
>> CC: Mark Brown <broonie@opensource.wolfsonmicro.com>
>
> You've missed the scissors line:
> ---
>
> The patch itself:
>
> Acked-by: Igor Grinberg <grinberg@compulab.co.il>
>
> Thanks
>

Mark,

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

Best Regards
Haojian

Re: [PATCH] ARM: pxa: fix build failure for regulator consumer in em-x270.c

[Mark Brown]

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

On Tue, Mar 06, 2012 at 05:12:18PM +0800, Haojian Zhuang wrote:

> 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.

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

Re: [PATCH] ARM: pxa: fix build failure for regulator consumer in em-x270.c

[Haojian Zhuang]

On Tue, Mar 6, 2012 at 8:16 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Tue, Mar 06, 2012 at 05:12:18PM +0800, Haojian Zhuang wrote:
>
>> 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.

OK. Let me merge this patch.

Best Regards
Haojian

Re: [PATCH] ARM: pxa: fix build failure for regulator consumer in em-x270.c

[Paul Gortmaker]

On Mon, Mar 5, 2012 at 6:12 PM, Paul Gortmaker
<paul.gortmaker@windriver.com> wrote:

[...]

> -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");

Based on today's discussions, I'm no longer convinced the above
is right.  If I understand correctly, for the case of say the top one:

      em_x270_gps_userspace_consumer

I need to go find that struct, determine that it has a .name entry like:

        .name           = "reg-userspace-consumer",

and then use (for each case) something like:

      REGULATOR_SUPPLY("vcc gps", "reg-userspace-consumer")

The above wrapper for REGULATOR_CONSUMER would have
to be changed to either use or inline the REGULATOR_SUPPLY.

Let me know if I'm on the right track here.  This code is completely
foreign to me.   As such, the naming in the magician_defconfig
patch is also probably needing rework (i.e. it needs the corresponding
value for .name, and not the name of the parent struct itself.)

THanks,
Paul.
--

>
>  #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@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[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.  Update them to use the new dev_name
entries instead.

Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
CC: Mark Brown <broonie@opensource.wolfsonmicro.com>
---

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

diff --git a/arch/arm/mach-pxa/em-x270.c b/arch/arm/mach-pxa/em-x270.c
index c1b65da..0ffc100 100644
--- a/arch/arm/mach-pxa/em-x270.c
+++ b/arch/arm/mach-pxa/em-x270.c
@@ -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");
+REGULATOR_CONSUMER(ldo3, "reg-userspace-consumer", "vcc gps");
 REGULATOR_CONSUMER(ldo5, NULL, "vcc cam");
-REGULATOR_CONSUMER(ldo10, &pxa_device_mci.dev, "vcc sdio");
+REGULATOR_CONSUMER(ldo10, "pxa2xx-mci", "vcc sdio");
 REGULATOR_CONSUMER(ldo12, NULL, "vcc usb");
-REGULATOR_CONSUMER(ldo19, &em_x270_gprs_userspace_consumer.dev, "vcc gprs");
+REGULATOR_CONSUMER(ldo19, "reg-userspace-consumer", "vcc gprs");
 REGULATOR_CONSUMER(buck2, NULL, "vcc_core");
 
 #define REGULATOR_INIT(_ldo, _min_uV, _max_uV, _ops_mask)		\
-- 
1.7.9.1

Re: [PATCH] ARM: pxa: fix build failure for regulator consumer in em-x270.c

[Igor Grinberg]

Hi Paul,

I've finally found one of em-x270 to test on...

On 03/09/12 00:06, Paul Gortmaker wrote:
> 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 <paul.gortmaker@windriver.com>
> CC: Mark Brown <broonie@opensource.wolfsonmicro.com>
> ---
> 
> [v2: replace dev with dev_name instead of just deleting dev.
>  Note however there is dev_name overlap; not sure if that matters? ]
> 
> diff --git a/arch/arm/mach-pxa/em-x270.c b/arch/arm/mach-pxa/em-x270.c
> index c1b65da..0ffc100 100644
> --- a/arch/arm/mach-pxa/em-x270.c
> +++ b/arch/arm/mach-pxa/em-x270.c
> @@ -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");
> +REGULATOR_CONSUMER(ldo3, "reg-userspace-consumer", "vcc gps");

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

>  REGULATOR_CONSUMER(ldo5, NULL, "vcc cam");
> -REGULATOR_CONSUMER(ldo10, &pxa_device_mci.dev, "vcc sdio");
> +REGULATOR_CONSUMER(ldo10, "pxa2xx-mci", "vcc sdio");
>  REGULATOR_CONSUMER(ldo12, NULL, "vcc usb");
> -REGULATOR_CONSUMER(ldo19, &em_x270_gprs_userspace_consumer.dev, "vcc gprs");
> +REGULATOR_CONSUMER(ldo19, "reg-userspace-consumer", "vcc gprs");

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

the device even boots...

>  REGULATOR_CONSUMER(buck2, NULL, "vcc_core");
>  
>  #define REGULATOR_INIT(_ldo, _min_uV, _max_uV, _ops_mask)		\


Thanks for the patch, with the changes above:
Tested-by: Igor Grinberg <grinberg@compulab.co.il>

Haojian, can this patch be included in your pull request (or another one)?
It fixes the build for em_x270_defconfig and with the updates above
gets the board to boot.

Thanks.

-- 
Regards,
Igor.

Re: [PATCH] ARM: pxa: fix build failure for regulator consumer in em-x270.c

[Mark Brown]

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

On Wed, Mar 28, 2012 at 05:21:59PM +0200, Igor Grinberg wrote:
> On 03/09/12 00:06, Paul Gortmaker wrote:

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

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

> > +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?

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

Re: [PATCH] ARM: pxa: fix build failure for regulator consumer in em-x270.c

[Haojian Zhuang]

On Wed, Mar 28, 2012 at 11:27 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Wed, Mar 28, 2012 at 05:21:59PM +0200, Igor Grinberg wrote:
>> On 03/09/12 00:06, Paul Gortmaker wrote:
>
>> > +REGULATOR_CONSUMER(ldo3, "reg-userspace-consumer", "vcc gps");
>
>> If you make this:
>> REGULATOR_CONSUMER(ldo3, NULL, "vcc gps");
>
>> > +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?

Paul,

Could you do a quick update on dev_name() according to Mark's comment?

-REGULATOR_CONSUMER(ldo10, &pxa_device_mci.dev, "vcc sdio");
+REGULAOTR_CONSUMER(ldo10, dev_name(&pxa_device_mci.dev), "vcc sdio");

Best Regards
Haojian

Re: [PATCH] ARM: pxa: fix build failure for regulator consumer in em-x270.c

[Mark Brown]

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

On Wed, Mar 28, 2012 at 11:37:37PM +0800, Haojian Zhuang wrote:
> On Wed, Mar 28, 2012 at 11:27 PM, Mark Brown

> > 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?

> Could you do a quick update on dev_name() according to Mark's comment?

> -REGULATOR_CONSUMER(ldo10, &pxa_device_mci.dev, "vcc sdio");
> +REGULAOTR_CONSUMER(ldo10, dev_name(&pxa_device_mci.dev), "vcc sdio");

No, you're missing the point again.  As I said previously the whole
point of this interface is that you don't have to have access to the
struct device.  This should be the string *returned* by dev_name(), not
a direct call to dev_name().

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

Re: [PATCH] ARM: pxa: fix build failure for regulator consumer in em-x270.c

[Paul Gortmaker]

On 12-03-28 11:27 AM, Mark Brown wrote:
> On Wed, Mar 28, 2012 at 05:21:59PM +0200, Igor Grinberg wrote:
>> On 03/09/12 00:06, Paul Gortmaker wrote:
> 
>>> +REGULATOR_CONSUMER(ldo3, "reg-userspace-consumer", "vcc gps");
> 
>> If you make this:
>> REGULATOR_CONSUMER(ldo3, NULL, "vcc gps");
> 
>>> +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?

Here is what I meant when I said in my v2 comment that there was
naming overlap, and that I wasn't sure what impact that had.

The two consumers are:

static struct platform_device em_x270_gps_userspace_consumer = {
        .name           = "reg-userspace-consumer",
        .id             = 0,
        .dev            = {
                .platform_data = &em_x270_gps_consumer_data,
        },
};

and 

static struct platform_device em_x270_gprs_userspace_consumer = {
        .name           = "reg-userspace-consumer",
        .id             = 1,
        .dev            = {
                .platform_data = &em_x270_gprs_consumer_data,
        }
};

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

Paul.

Re: [PATCH] ARM: pxa: fix build failure for regulator consumer in em-x270.c

[Mark Brown]

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

On Wed, Mar 28, 2012 at 11:59:41AM -0400, Paul Gortmaker wrote:
> 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.

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.

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

Re: [PATCH] ARM: pxa: fix build failure for regulator consumer in em-x270.c

[Paul Gortmaker]

On 12-03-28 12:13 PM, Mark Brown wrote:
> On Wed, Mar 28, 2012 at 11:59:41AM -0400, Paul Gortmaker wrote:
>> 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.
> 
> 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:

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

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

Thanks,
Paul.

Re: [PATCH] ARM: pxa: fix build failure for regulator consumer in em-x270.c

[Igor Grinberg]

On 03/28/12 18:59, Paul Gortmaker wrote:
> On 12-03-28 12:13 PM, Mark Brown wrote:
>> On Wed, Mar 28, 2012 at 11:59:41AM -0400, Paul Gortmaker wrote:
>>> 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.
>>
>> 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:
> 
> -REGULATOR_CONSUMER(ldo3, "reg-userspace-consumer", "vcc gps");
> +REGULATOR_CONSUMER(ldo3, "reg-userspace-consumer.0", "vcc gps");
> 
> -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?

-- 
Regards,
Igor.

Re: [PATCH] ARM: pxa: fix build failure for regulator consumer in em-x270.c

[Mark Brown]

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

On Thu, Mar 29, 2012 at 04:28:48PM +0200, Igor Grinberg wrote:
> On 03/28/12 18:59, Paul Gortmaker 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,

> > -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.

> 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.

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

Re: [PATCH] ARM: pxa: fix build failure for regulator consumer in em-x270.c

[Paul Gortmaker]

On 12-03-29 10:28 AM, Igor Grinberg wrote:
> On 03/28/12 18:59, Paul Gortmaker wrote:
>> On 12-03-28 12:13 PM, Mark Brown wrote:
>>> On Wed, Mar 28, 2012 at 11:59:41AM -0400, Paul Gortmaker wrote:
>>>> 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.
>>>
>>> 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:
>>
>> -REGULATOR_CONSUMER(ldo3, "reg-userspace-consumer", "vcc gps");
>> +REGULATOR_CONSUMER(ldo3, "reg-userspace-consumer.0", "vcc gps");
>>
>> -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.

Paul.
--

> (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?
> 

Re: [PATCH] ARM: pxa: fix build failure for regulator consumer in em-x270.c

[Paul Gortmaker]

On 12-03-29 10:54 AM, Mark Brown wrote:
> On Thu, Mar 29, 2012 at 04:28:48PM +0200, Igor Grinberg wrote:
>> On 03/28/12 18:59, Paul Gortmaker 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,
> 
>>> -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.
> 
>> 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.

Mark,

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?

Paul.

Re: [PATCH] ARM: pxa: fix build failure for regulator consumer in em-x270.c

[Mark Brown]

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

On Thu, Mar 29, 2012 at 10:57:08AM -0400, Paul Gortmaker wrote:

> 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?

Please resend, makes life easier.  I'm not sure if it's me or Haojin
who will end up applying the change anyway.

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

Re: [PATCH] ARM: pxa: fix build failure for regulator consumer in em-x270.c

[Igor Grinberg]

On 03/29/12 16:55, Paul Gortmaker wrote:
> On 12-03-29 10:28 AM, Igor Grinberg wrote:
>> On 03/28/12 18:59, Paul Gortmaker wrote:
>>> On 12-03-28 12:13 PM, Mark Brown wrote:
>>>> On Wed, Mar 28, 2012 at 11:59:41AM -0400, Paul Gortmaker wrote:
>>>>> 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.
>>>>
>>>> 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:
>>>
>>> -REGULATOR_CONSUMER(ldo3, "reg-userspace-consumer", "vcc gps");
>>> +REGULATOR_CONSUMER(ldo3, "reg-userspace-consumer.0", "vcc gps");
>>>
>>> -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.

The problem is, that someone has to stop sending emails in a hurry...
Spitting all the mix in the head when in rush...
and that would be me...

Sorry, for the mess, I'll try to fix myself up...

-- 
Regards,
Igor.