Re: [PATCH v3] ARM: OMAP2+: gpmc-smsc911x: add required smsc911x regulators

2012-03-08 Thread Tony Lindgren
* Kevin Hilman khil...@ti.com [120307 11:05]:
 
  I don't think the second smsc911x on the Overo, smsc911x.1, would
  find it due to the dev_id.
 
 It's not about finding the second regulator.  As stated in the
 changelog, it's about the duplicate attempt to register the exact same
 platform_device.
 
 Duplicate attempts to register the exact same platform_device cause
 kobject to panic and give up[1].  So, any platform that calls
 gpmc_smsc911x_init() twice (Overo and T35 in mainline) will panic on
 boot.
 
 This patch fixes those platforms so they can boot.

Yeah but I guess the second smsc911x instance still would not work,
or am I missing something?

Regards,

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


Re: [PATCH v3] ARM: OMAP2+: gpmc-smsc911x: add required smsc911x regulators

2012-03-08 Thread Kevin Hilman
Tony Lindgren t...@atomide.com writes:

 * Kevin Hilman khil...@ti.com [120307 11:05]:
 
  I don't think the second smsc911x on the Overo, smsc911x.1, would
  find it due to the dev_id.
 
 It's not about finding the second regulator.  As stated in the
 changelog, it's about the duplicate attempt to register the exact same
 platform_device.
 
 Duplicate attempts to register the exact same platform_device cause
 kobject to panic and give up[1].  So, any platform that calls
 gpmc_smsc911x_init() twice (Overo and T35 in mainline) will panic on
 boot.
 
 This patch fixes those platforms so they can boot.

 Yeah but I guess the second smsc911x instance still would not work,
 or am I missing something?

I don't know since my Overo expansion boards don't have a 2nd NIC, but I
suspect you're right.

However, my fix isn't addressing that.  I am fixing a problem where
mainline today will panic on some boards due to duplicate registration.

If the 2nd interface doesn't work, then the original patch that added
the regulators needs a rethink.  My patch to prevent the panic() is
needed for mainline.

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


Re: [PATCH v3] ARM: OMAP2+: gpmc-smsc911x: add required smsc911x regulators

2012-03-08 Thread Tony Lindgren
* Kevin Hilman khil...@ti.com [120308 12:54]:
 Tony Lindgren t...@atomide.com writes:
 
  * Kevin Hilman khil...@ti.com [120307 11:05]:
  
   I don't think the second smsc911x on the Overo, smsc911x.1, would
   find it due to the dev_id.
  
  It's not about finding the second regulator.  As stated in the
  changelog, it's about the duplicate attempt to register the exact same
  platform_device.
  
  Duplicate attempts to register the exact same platform_device cause
  kobject to panic and give up[1].  So, any platform that calls
  gpmc_smsc911x_init() twice (Overo and T35 in mainline) will panic on
  boot.
  
  This patch fixes those platforms so they can boot.
 
  Yeah but I guess the second smsc911x instance still would not work,
  or am I missing something?
 
 I don't know since my Overo expansion boards don't have a 2nd NIC, but I
 suspect you're right.
 
 However, my fix isn't addressing that.  I am fixing a problem where
 mainline today will panic on some boards due to duplicate registration.
 
 If the 2nd interface doesn't work, then the original patch that added
 the regulators needs a rethink.  My patch to prevent the panic() is
 needed for mainline.

With Kevin's second version of this patch applied we avoid the panic
on boards with more than one smsc911x.

After this patch, board-*.c files can add custom regulators for their
other smsc911x instances.

We should also add a regulator to the struct omap_smsc911x_platform_data,
and then only initialize the fixed regulator automatically
if (!board_data-regulator  !board_data-id).

So I've pushed Kevin's second version of the fix to fix-smsc911x-regulator
branch.

Regards,

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


Re: [PATCH v3] ARM: OMAP2+: gpmc-smsc911x: add required smsc911x regulators

2012-03-07 Thread Russ Dill
On Mon, Mar 5, 2012 at 12:18 PM, Kevin Hilman khil...@ti.com wrote:
 Felipe Balbi ba...@ti.com writes:

 Hi,

 On Thu, Mar 01, 2012 at 12:36:57PM -0800, Kevin Hilman wrote:
 Matt Porter mpor...@ti.com writes:

  This fixes smsc911x support on platforms using gpmc_smsc911x_init().
 
  Commit c7e963f616 (net/smsc911x: Add regulator support) added
  the requirement that platforms provide vdd33a and vddvario supplies.
 
  Signed-off-by: Matt Porter mpor...@ti.com

 [...]

   /*
    * Initialize smsc911x device connected to the GPMC. Note that we
    * assume that pin multiplexing is done in the board-*.c file,
  @@ -55,6 +101,12 @@ void __init gpmc_smsc911x_init(struct 
  omap_smsc911x_platform_data *board_data)
 
     gpmc_cfg = board_data;
 
  +  ret = platform_device_register(gpmc_smsc911x_regulator);
  +  if (ret  0) {
  +          pr_err(Unable to register smsc911x regulators: %d\n, ret);
  +          return;
  +  }
  +

 Boards that have more than one instance of the smsc911x (OMAP3/Overo)
 barf here because of trying to register the same device twice.

 We need something like the patch below to make Overo boot again.

 Kevin



 From 4114dea2fb897ab75d05eaa943d29454034fc025 Mon Sep 17 00:00:00 2001
 From: Kevin Hilman khil...@ti.com
 Date: Thu, 1 Mar 2012 12:30:42 -0800
 Subject: [PATCH] ARM: OMAP2+: gpmc-smsc911x: only register regulators once

 commit e4b0b2cbbb (ARM: OMAP2+: gpmc-smsc911x: add required smsc911x
 regulators) added regulators which are registered during
 gpmc_smsc911x_init().  However, some platforms (OMAP3/Overo) have more
 than one instance of the SMSC911x and result in attempting to register
 the same regulator more than once which causes a panic().  Fix this by
 tracking the regulator registration ensuring only a single device is
 registered.

 Cc: Matt Porter mpor...@ti.com
 Signed-off-by: Kevin Hilman khil...@ti.com
 ---
  arch/arm/mach-omap2/gpmc-smsc911x.c |   14 ++
  1 file changed, 10 insertions(+), 4 deletions(-)

 diff --git a/arch/arm/mach-omap2/gpmc-smsc911x.c 
 b/arch/arm/mach-omap2/gpmc-smsc911x.c
 index bbb870c..95e6c7d 100644
 --- a/arch/arm/mach-omap2/gpmc-smsc911x.c
 +++ b/arch/arm/mach-omap2/gpmc-smsc911x.c
 @@ -88,6 +88,8 @@ static struct platform_device gpmc_smsc911x_regulator = {
      },
  };

 +static bool regulator_registered;
 +
  /*
   * Initialize smsc911x device connected to the GPMC. Note that we
   * assume that pin multiplexing is done in the board-*.c file,
 @@ -101,10 +103,14 @@ void __init gpmc_smsc911x_init(struct 
 omap_smsc911x_platform_data *board_data)

      gpmc_cfg = board_data;

 -    ret = platform_device_register(gpmc_smsc911x_regulator);
 -    if (ret  0) {
 -            pr_err(Unable to register smsc911x regulators: %d\n, ret);
 -            return;
 +    if (!regulator_registered) {
 +            ret = platform_device_register(gpmc_smsc911x_regulator);
 +            if (ret  0) {
 +                    pr_err(Unable to register smsc911x regulators: %d\n,
 +                           ret);
 +                    return;
 +            }
 +            regulator_registered = true;

 Wow, this is quite a hack. Is the regulator part of the SMSC911x
 device or is it outside ? If it's outside the SMSC91xx (which I
 believe it is) there should be a regulator driver instead of this
 hack.  For boards which don't provide such a regulator (and tie the
 supply pin to some constant voltage source) there should be a constant
 regulator supplying the pin. But this patch is quite hacky.

 Are you referring to my patch or to the original $SUBJECT patch?  It's
 not terribly clear.

 My patch doesn't add any regulators, it just fixes a bug when this init
 function is called more than once.

 The addition of the regulators was done in $SUBJECT patch, not my fix.

 In either case $SUBJECT patch is already in Tony's fixes queue, so if it
 is going be merged, then my fix above is needed also to not break
 Overo and any other platform that has more than one smsc911x instance.

 Kevin


Have you tested this fix? The only regulator consumer supply would be:

static struct regulator_consumer_supply gpmc_smsc911x_supply[] = {
REGULATOR_SUPPLY(vddvario, smsc911x.0),
REGULATOR_SUPPLY(vdd33a, smsc911x.0),
};

I don't think the second smsc911x on the Overo, smsc911x.1, would
find it due to the dev_id.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3] ARM: OMAP2+: gpmc-smsc911x: add required smsc911x regulators

2012-03-07 Thread Tony Lindgren
* Russ Dill russ.d...@ti.com [120307 07:28]:
 On Mon, Mar 5, 2012 at 12:18 PM, Kevin Hilman khil...@ti.com wrote:
 
  In either case $SUBJECT patch is already in Tony's fixes queue, so if it
  is going be merged, then my fix above is needed also to not break
  Overo and any other platform that has more than one smsc911x instance.
 
  Kevin
 
 
 Have you tested this fix? The only regulator consumer supply would be:
 
 static struct regulator_consumer_supply gpmc_smsc911x_supply[] = {
 REGULATOR_SUPPLY(vddvario, smsc911x.0),
 REGULATOR_SUPPLY(vdd33a, smsc911x.0),
 };
 
 I don't think the second smsc911x on the Overo, smsc911x.1, would
 find it due to the dev_id.

Hmm yeah sounds like there's some further changes needed. Dropping
the second version from fixes until we have somebody reply with
tested-by using two smsc911x instances.

Regards,

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


Re: [PATCH v3] ARM: OMAP2+: gpmc-smsc911x: add required smsc911x regulators

2012-03-07 Thread Kevin Hilman
Hi Russ,

Russ Dill russ.d...@ti.com writes:

 On Mon, Mar 5, 2012 at 12:18 PM, Kevin Hilman khil...@ti.com wrote:
 Felipe Balbi ba...@ti.com writes:

 Hi,

 On Thu, Mar 01, 2012 at 12:36:57PM -0800, Kevin Hilman wrote:
 Matt Porter mpor...@ti.com writes:

  This fixes smsc911x support on platforms using gpmc_smsc911x_init().
 
  Commit c7e963f616 (net/smsc911x: Add regulator support) added
  the requirement that platforms provide vdd33a and vddvario supplies.
 
  Signed-off-by: Matt Porter mpor...@ti.com

 [...]

   /*
    * Initialize smsc911x device connected to the GPMC. Note that we
    * assume that pin multiplexing is done in the board-*.c file,
  @@ -55,6 +101,12 @@ void __init gpmc_smsc911x_init(struct 
  omap_smsc911x_platform_data *board_data)
 
     gpmc_cfg = board_data;
 
  +  ret = platform_device_register(gpmc_smsc911x_regulator);
  +  if (ret  0) {
  +          pr_err(Unable to register smsc911x regulators: %d\n, ret);
  +          return;
  +  }
  +

 Boards that have more than one instance of the smsc911x (OMAP3/Overo)
 barf here because of trying to register the same device twice.

 We need something like the patch below to make Overo boot again.

 Kevin



 From 4114dea2fb897ab75d05eaa943d29454034fc025 Mon Sep 17 00:00:00 2001
 From: Kevin Hilman khil...@ti.com
 Date: Thu, 1 Mar 2012 12:30:42 -0800
 Subject: [PATCH] ARM: OMAP2+: gpmc-smsc911x: only register regulators once

 commit e4b0b2cbbb (ARM: OMAP2+: gpmc-smsc911x: add required smsc911x
 regulators) added regulators which are registered during
 gpmc_smsc911x_init().  However, some platforms (OMAP3/Overo) have more
 than one instance of the SMSC911x and result in attempting to register
 the same regulator more than once which causes a panic().  Fix this by
 tracking the regulator registration ensuring only a single device is
 registered.

 Cc: Matt Porter mpor...@ti.com
 Signed-off-by: Kevin Hilman khil...@ti.com
 ---
  arch/arm/mach-omap2/gpmc-smsc911x.c |   14 ++
  1 file changed, 10 insertions(+), 4 deletions(-)

 diff --git a/arch/arm/mach-omap2/gpmc-smsc911x.c 
 b/arch/arm/mach-omap2/gpmc-smsc911x.c
 index bbb870c..95e6c7d 100644
 --- a/arch/arm/mach-omap2/gpmc-smsc911x.c
 +++ b/arch/arm/mach-omap2/gpmc-smsc911x.c
 @@ -88,6 +88,8 @@ static struct platform_device gpmc_smsc911x_regulator = {
      },
  };

 +static bool regulator_registered;
 +
  /*
   * Initialize smsc911x device connected to the GPMC. Note that we
   * assume that pin multiplexing is done in the board-*.c file,
 @@ -101,10 +103,14 @@ void __init gpmc_smsc911x_init(struct 
 omap_smsc911x_platform_data *board_data)

      gpmc_cfg = board_data;

 -    ret = platform_device_register(gpmc_smsc911x_regulator);
 -    if (ret  0) {
 -            pr_err(Unable to register smsc911x regulators: %d\n, ret);
 -            return;
 +    if (!regulator_registered) {
 +            ret = platform_device_register(gpmc_smsc911x_regulator);
 +            if (ret  0) {
 +                    pr_err(Unable to register smsc911x regulators: %d\n,
 +                           ret);
 +                    return;
 +            }
 +            regulator_registered = true;

 Wow, this is quite a hack. Is the regulator part of the SMSC911x
 device or is it outside ? If it's outside the SMSC91xx (which I
 believe it is) there should be a regulator driver instead of this
 hack.  For boards which don't provide such a regulator (and tie the
 supply pin to some constant voltage source) there should be a constant
 regulator supplying the pin. But this patch is quite hacky.

 Are you referring to my patch or to the original $SUBJECT patch?  It's
 not terribly clear.

 My patch doesn't add any regulators, it just fixes a bug when this init
 function is called more than once.

 The addition of the regulators was done in $SUBJECT patch, not my fix.

 In either case $SUBJECT patch is already in Tony's fixes queue, so if it
 is going be merged, then my fix above is needed also to not break
 Overo and any other platform that has more than one smsc911x instance.

 Kevin


 Have you tested this fix? 

Yes.  On 3530/Overo.

 The only regulator consumer supply would be:

 static struct regulator_consumer_supply gpmc_smsc911x_supply[] = {
 REGULATOR_SUPPLY(vddvario, smsc911x.0),
 REGULATOR_SUPPLY(vdd33a, smsc911x.0),
 };

 I don't think the second smsc911x on the Overo, smsc911x.1, would
 find it due to the dev_id.

It's not about finding the second regulator.  As stated in the
changelog, it's about the duplicate attempt to register the exact same
platform_device.

Duplicate attempts to register the exact same platform_device cause
kobject to panic and give up[1].  So, any platform that calls
gpmc_smsc911x_init() twice (Overo and T35 in mainline) will panic on
boot.

This patch fixes those platforms so they can boot.

Kevin



[1]
[0.337036] kobject (c06b1730): tried to init an initialized object, 
something is seriously wrong.
[0.346679] [c001421c] 

Re: [PATCH v3] ARM: OMAP2+: gpmc-smsc911x: add required smsc911x regulators

2012-03-05 Thread Kevin Hilman
Felipe Balbi ba...@ti.com writes:

 Hi,

 On Thu, Mar 01, 2012 at 12:36:57PM -0800, Kevin Hilman wrote:
 Matt Porter mpor...@ti.com writes:
 
  This fixes smsc911x support on platforms using gpmc_smsc911x_init().
 
  Commit c7e963f616 (net/smsc911x: Add regulator support) added
  the requirement that platforms provide vdd33a and vddvario supplies.
 
  Signed-off-by: Matt Porter mpor...@ti.com
 
 [...]
 
   /*
* Initialize smsc911x device connected to the GPMC. Note that we
* assume that pin multiplexing is done in the board-*.c file,
  @@ -55,6 +101,12 @@ void __init gpmc_smsc911x_init(struct 
  omap_smsc911x_platform_data *board_data)
   
 gpmc_cfg = board_data;
   
  +  ret = platform_device_register(gpmc_smsc911x_regulator);
  +  if (ret  0) {
  +  pr_err(Unable to register smsc911x regulators: %d\n, ret);
  +  return;
  +  }
  +
 
 Boards that have more than one instance of the smsc911x (OMAP3/Overo)
 barf here because of trying to register the same device twice.
 
 We need something like the patch below to make Overo boot again.
 
 Kevin
 
 
 
 From 4114dea2fb897ab75d05eaa943d29454034fc025 Mon Sep 17 00:00:00 2001
 From: Kevin Hilman khil...@ti.com
 Date: Thu, 1 Mar 2012 12:30:42 -0800
 Subject: [PATCH] ARM: OMAP2+: gpmc-smsc911x: only register regulators once
 
 commit e4b0b2cbbb (ARM: OMAP2+: gpmc-smsc911x: add required smsc911x
 regulators) added regulators which are registered during
 gpmc_smsc911x_init().  However, some platforms (OMAP3/Overo) have more
 than one instance of the SMSC911x and result in attempting to register
 the same regulator more than once which causes a panic().  Fix this by
 tracking the regulator registration ensuring only a single device is
 registered.
 
 Cc: Matt Porter mpor...@ti.com
 Signed-off-by: Kevin Hilman khil...@ti.com
 ---
  arch/arm/mach-omap2/gpmc-smsc911x.c |   14 ++
  1 file changed, 10 insertions(+), 4 deletions(-)
 
 diff --git a/arch/arm/mach-omap2/gpmc-smsc911x.c 
 b/arch/arm/mach-omap2/gpmc-smsc911x.c
 index bbb870c..95e6c7d 100644
 --- a/arch/arm/mach-omap2/gpmc-smsc911x.c
 +++ b/arch/arm/mach-omap2/gpmc-smsc911x.c
 @@ -88,6 +88,8 @@ static struct platform_device gpmc_smsc911x_regulator = {
  },
  };
  
 +static bool regulator_registered;
 +
  /*
   * Initialize smsc911x device connected to the GPMC. Note that we
   * assume that pin multiplexing is done in the board-*.c file,
 @@ -101,10 +103,14 @@ void __init gpmc_smsc911x_init(struct 
 omap_smsc911x_platform_data *board_data)
  
  gpmc_cfg = board_data;
  
 -ret = platform_device_register(gpmc_smsc911x_regulator);
 -if (ret  0) {
 -pr_err(Unable to register smsc911x regulators: %d\n, ret);
 -return;
 +if (!regulator_registered) {
 +ret = platform_device_register(gpmc_smsc911x_regulator);
 +if (ret  0) {
 +pr_err(Unable to register smsc911x regulators: %d\n,
 +   ret);
 +return;
 +}
 +regulator_registered = true;

 Wow, this is quite a hack. Is the regulator part of the SMSC911x
 device or is it outside ? If it's outside the SMSC91xx (which I
 believe it is) there should be a regulator driver instead of this
 hack.  For boards which don't provide such a regulator (and tie the
 supply pin to some constant voltage source) there should be a constant
 regulator supplying the pin. But this patch is quite hacky.

Are you referring to my patch or to the original $SUBJECT patch?  It's
not terribly clear.

My patch doesn't add any regulators, it just fixes a bug when this init
function is called more than once.

The addition of the regulators was done in $SUBJECT patch, not my fix.

In either case $SUBJECT patch is already in Tony's fixes queue, so if it
is going be merged, then my fix above is needed also to not break
Overo and any other platform that has more than one smsc911x instance.

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


Re: [PATCH v3] ARM: OMAP2+: gpmc-smsc911x: add required smsc911x regulators

2012-03-01 Thread Kevin Hilman
Matt Porter mpor...@ti.com writes:

 This fixes smsc911x support on platforms using gpmc_smsc911x_init().

 Commit c7e963f616 (net/smsc911x: Add regulator support) added
 the requirement that platforms provide vdd33a and vddvario supplies.

 Signed-off-by: Matt Porter mpor...@ti.com

[...]

  /*
   * Initialize smsc911x device connected to the GPMC. Note that we
   * assume that pin multiplexing is done in the board-*.c file,
 @@ -55,6 +101,12 @@ void __init gpmc_smsc911x_init(struct 
 omap_smsc911x_platform_data *board_data)
  
   gpmc_cfg = board_data;
  
 + ret = platform_device_register(gpmc_smsc911x_regulator);
 + if (ret  0) {
 + pr_err(Unable to register smsc911x regulators: %d\n, ret);
 + return;
 + }
 +

Boards that have more than one instance of the smsc911x (OMAP3/Overo)
barf here because of trying to register the same device twice.

We need something like the patch below to make Overo boot again.

Kevin



From 4114dea2fb897ab75d05eaa943d29454034fc025 Mon Sep 17 00:00:00 2001
From: Kevin Hilman khil...@ti.com
Date: Thu, 1 Mar 2012 12:30:42 -0800
Subject: [PATCH] ARM: OMAP2+: gpmc-smsc911x: only register regulators once

commit e4b0b2cbbb (ARM: OMAP2+: gpmc-smsc911x: add required smsc911x
regulators) added regulators which are registered during
gpmc_smsc911x_init().  However, some platforms (OMAP3/Overo) have more
than one instance of the SMSC911x and result in attempting to register
the same regulator more than once which causes a panic().  Fix this by
tracking the regulator registration ensuring only a single device is
registered.

Cc: Matt Porter mpor...@ti.com
Signed-off-by: Kevin Hilman khil...@ti.com
---
 arch/arm/mach-omap2/gpmc-smsc911x.c |   14 ++
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mach-omap2/gpmc-smsc911x.c 
b/arch/arm/mach-omap2/gpmc-smsc911x.c
index bbb870c..95e6c7d 100644
--- a/arch/arm/mach-omap2/gpmc-smsc911x.c
+++ b/arch/arm/mach-omap2/gpmc-smsc911x.c
@@ -88,6 +88,8 @@ static struct platform_device gpmc_smsc911x_regulator = {
},
 };
 
+static bool regulator_registered;
+
 /*
  * Initialize smsc911x device connected to the GPMC. Note that we
  * assume that pin multiplexing is done in the board-*.c file,
@@ -101,10 +103,14 @@ void __init gpmc_smsc911x_init(struct 
omap_smsc911x_platform_data *board_data)
 
gpmc_cfg = board_data;
 
-   ret = platform_device_register(gpmc_smsc911x_regulator);
-   if (ret  0) {
-   pr_err(Unable to register smsc911x regulators: %d\n, ret);
-   return;
+   if (!regulator_registered) {
+   ret = platform_device_register(gpmc_smsc911x_regulator);
+   if (ret  0) {
+   pr_err(Unable to register smsc911x regulators: %d\n,
+  ret);
+   return;
+   }
+   regulator_registered = true;
}
 
if (gpmc_cs_request(gpmc_cfg-cs, SZ_16M, cs_mem_base)  0) {
-- 
1.7.9.2

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


Re: [PATCH v3] ARM: OMAP2+: gpmc-smsc911x: add required smsc911x regulators

2012-03-01 Thread Felipe Balbi
Hi,

On Thu, Mar 01, 2012 at 12:36:57PM -0800, Kevin Hilman wrote:
 Matt Porter mpor...@ti.com writes:
 
  This fixes smsc911x support on platforms using gpmc_smsc911x_init().
 
  Commit c7e963f616 (net/smsc911x: Add regulator support) added
  the requirement that platforms provide vdd33a and vddvario supplies.
 
  Signed-off-by: Matt Porter mpor...@ti.com
 
 [...]
 
   /*
* Initialize smsc911x device connected to the GPMC. Note that we
* assume that pin multiplexing is done in the board-*.c file,
  @@ -55,6 +101,12 @@ void __init gpmc_smsc911x_init(struct 
  omap_smsc911x_platform_data *board_data)
   
  gpmc_cfg = board_data;
   
  +   ret = platform_device_register(gpmc_smsc911x_regulator);
  +   if (ret  0) {
  +   pr_err(Unable to register smsc911x regulators: %d\n, ret);
  +   return;
  +   }
  +
 
 Boards that have more than one instance of the smsc911x (OMAP3/Overo)
 barf here because of trying to register the same device twice.
 
 We need something like the patch below to make Overo boot again.
 
 Kevin
 
 
 
 From 4114dea2fb897ab75d05eaa943d29454034fc025 Mon Sep 17 00:00:00 2001
 From: Kevin Hilman khil...@ti.com
 Date: Thu, 1 Mar 2012 12:30:42 -0800
 Subject: [PATCH] ARM: OMAP2+: gpmc-smsc911x: only register regulators once
 
 commit e4b0b2cbbb (ARM: OMAP2+: gpmc-smsc911x: add required smsc911x
 regulators) added regulators which are registered during
 gpmc_smsc911x_init().  However, some platforms (OMAP3/Overo) have more
 than one instance of the SMSC911x and result in attempting to register
 the same regulator more than once which causes a panic().  Fix this by
 tracking the regulator registration ensuring only a single device is
 registered.
 
 Cc: Matt Porter mpor...@ti.com
 Signed-off-by: Kevin Hilman khil...@ti.com
 ---
  arch/arm/mach-omap2/gpmc-smsc911x.c |   14 ++
  1 file changed, 10 insertions(+), 4 deletions(-)
 
 diff --git a/arch/arm/mach-omap2/gpmc-smsc911x.c 
 b/arch/arm/mach-omap2/gpmc-smsc911x.c
 index bbb870c..95e6c7d 100644
 --- a/arch/arm/mach-omap2/gpmc-smsc911x.c
 +++ b/arch/arm/mach-omap2/gpmc-smsc911x.c
 @@ -88,6 +88,8 @@ static struct platform_device gpmc_smsc911x_regulator = {
   },
  };
  
 +static bool regulator_registered;
 +
  /*
   * Initialize smsc911x device connected to the GPMC. Note that we
   * assume that pin multiplexing is done in the board-*.c file,
 @@ -101,10 +103,14 @@ void __init gpmc_smsc911x_init(struct 
 omap_smsc911x_platform_data *board_data)
  
   gpmc_cfg = board_data;
  
 - ret = platform_device_register(gpmc_smsc911x_regulator);
 - if (ret  0) {
 - pr_err(Unable to register smsc911x regulators: %d\n, ret);
 - return;
 + if (!regulator_registered) {
 + ret = platform_device_register(gpmc_smsc911x_regulator);
 + if (ret  0) {
 + pr_err(Unable to register smsc911x regulators: %d\n,
 +ret);
 + return;
 + }
 + regulator_registered = true;

Wow, this is quite a hack. Is the regulator part of the SMSC911x device
or is it outside ? If it's outside the SMSC91xx (which I believe it is)
there should be a regulator driver instead of this hack. For boards
which don't provide such a regulator (and tie the supply pin to some
constant voltage source) there should be a constant regulator supplying
the pin. But this patch is quite hacky.

-- 
balbi


signature.asc
Description: Digital signature


[PATCH v3] ARM: OMAP2+: gpmc-smsc911x: add required smsc911x regulators

2012-02-23 Thread Matt Porter
This fixes smsc911x support on platforms using gpmc_smsc911x_init().

Commit c7e963f616 (net/smsc911x: Add regulator support) added
the requirement that platforms provide vdd33a and vddvario supplies.

Signed-off-by: Matt Porter mpor...@ti.com
---
Changes for v2:
   - temporary fix to avoid platform device id conflicts
   - add commit summary from the smsc911x regulator support commit
   - incorporate platform device registration error cause reporting
Changes for v3:
   - remove unneeded local variable err

 arch/arm/mach-omap2/gpmc-smsc911x.c |   52 +++
 1 files changed, 52 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/gpmc-smsc911x.c 
b/arch/arm/mach-omap2/gpmc-smsc911x.c
index 9970331..bbb870c 100644
--- a/arch/arm/mach-omap2/gpmc-smsc911x.c
+++ b/arch/arm/mach-omap2/gpmc-smsc911x.c
@@ -19,6 +19,8 @@
 #include linux/interrupt.h
 #include linux/io.h
 #include linux/smsc911x.h
+#include linux/regulator/fixed.h
+#include linux/regulator/machine.h
 
 #include plat/board.h
 #include plat/gpmc.h
@@ -42,6 +44,50 @@ static struct smsc911x_platform_config gpmc_smsc911x_config 
= {
.flags  = SMSC911X_USE_16BIT,
 };
 
+static struct regulator_consumer_supply gpmc_smsc911x_supply[] = {
+   REGULATOR_SUPPLY(vddvario, smsc911x.0),
+   REGULATOR_SUPPLY(vdd33a, smsc911x.0),
+};
+
+/* Generic regulator definition to satisfy smsc911x */
+static struct regulator_init_data gpmc_smsc911x_reg_init_data = {
+   .constraints = {
+   .min_uV = 330,
+   .max_uV = 330,
+   .valid_modes_mask   = REGULATOR_MODE_NORMAL
+   | REGULATOR_MODE_STANDBY,
+   .valid_ops_mask = REGULATOR_CHANGE_MODE
+   | REGULATOR_CHANGE_STATUS,
+   },
+   .num_consumer_supplies  = ARRAY_SIZE(gpmc_smsc911x_supply),
+   .consumer_supplies  = gpmc_smsc911x_supply,
+};
+
+static struct fixed_voltage_config gpmc_smsc911x_fixed_reg_data = {
+   .supply_name= gpmc_smsc911x,
+   .microvolts = 330,
+   .gpio   = -EINVAL,
+   .startup_delay  = 0,
+   .enable_high= 0,
+   .enabled_at_boot= 1,
+   .init_data  = gpmc_smsc911x_reg_init_data,
+};
+
+/*
+ * Platform device id of 42 is a temporary fix to avoid conflicts
+ * with other reg-fixed-voltage devices. The real fix should
+ * involve the driver core providing a way of dynamically
+ * assigning a unique id on registration for platform devices
+ * in the same name space.
+ */
+static struct platform_device gpmc_smsc911x_regulator = {
+   .name   = reg-fixed-voltage,
+   .id = 42,
+   .dev = {
+   .platform_data  = gpmc_smsc911x_fixed_reg_data,
+   },
+};
+
 /*
  * Initialize smsc911x device connected to the GPMC. Note that we
  * assume that pin multiplexing is done in the board-*.c file,
@@ -55,6 +101,12 @@ void __init gpmc_smsc911x_init(struct 
omap_smsc911x_platform_data *board_data)
 
gpmc_cfg = board_data;
 
+   ret = platform_device_register(gpmc_smsc911x_regulator);
+   if (ret  0) {
+   pr_err(Unable to register smsc911x regulators: %d\n, ret);
+   return;
+   }
+
if (gpmc_cs_request(gpmc_cfg-cs, SZ_16M, cs_mem_base)  0) {
pr_err(Failed to request GPMC mem region\n);
return;
-- 
1.7.5.4

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