This fixes smsc911x support on platforms using gpmc_smsc911x_init().
Commit c7e963f6888816f04d1f5da0e07bec4e0092f227 added the requirement
that platforms provide vdd33a and vddvario supplies.
Signed-off-by: Matt Porter <[email protected]>
---
arch/arm/mach-omap2/gpmc-smsc911x.c | 44 +++++++++++++++++++++++++++++++++++
1 files changed, 44 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-omap2/gpmc-smsc911x.c b/arch/arm/mach-omap2/gpmc-smsc911x.c
index 9970331..da8399e 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,43 @@ 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 = 3300000,
+ .max_uV = 3300000,
+ .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 = 3300000,
+ .gpio = -EINVAL,
+ .startup_delay = 0,
+ .enable_high = 0,
+ .enabled_at_boot = 1,
+ .init_data = &gpmc_smsc911x_reg_init_data,
+};
+
+static struct platform_device gpmc_smsc911x_regulator = {
+ .name = "reg-fixed-voltage",
+ .id = 0,
+ .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 +94,11 @@ void __init gpmc_smsc911x_init(struct omap_smsc911x_platform_data *board_data)
gpmc_cfg = board_data;
+ if (platform_device_register(&gpmc_smsc911x_regulator) < 0) {
+ pr_err("Unable to register smsc911x regulators\n");
+ 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
On Mon, Feb 13, 2012 at 11:43:42AM -0500, Matt Porter wrote:
> This fixes smsc911x support on platforms using gpmc_smsc911x_init().
> Commit c7e963f6888816f04d1f5da0e07bec4e0092f227 added the requirement
> that platforms provide vdd33a and vddvario supplies.
Thanks. Only one comment on this.
> +static struct platform_device gpmc_smsc911x_regulator = {
> + .name = "reg-fixed-voltage",
> + .id = 0,
$ grep -A1 reg-fixed-voltage arch/arm/*omap* -r
arch/arm/mach-omap2/board-omap3evm.c: .name = "reg-fixed-voltage",
arch/arm/mach-omap2/board-omap3evm.c- .id = 1,
--
arch/arm/mach-omap2/board-zoom-peripherals.c: .name = "reg-fixed-voltage",
arch/arm/mach-omap2/board-zoom-peripherals.c- .id = 1,
--
arch/arm/mach-omap2/board-overo.c: .name = "reg-fixed-voltage",
arch/arm/mach-omap2/board-overo.c- .id = 1,
--
arch/arm/mach-omap2/board-omap4panda.c: .name = "reg-fixed-voltage",
arch/arm/mach-omap2/board-omap4panda.c- .id = 1,
--
arch/arm/mach-omap2/board-rm680.c: .name = "reg-fixed-voltage",
arch/arm/mach-omap2/board-rm680.c- .dev = {
--
arch/arm/mach-omap2/board-igep0020.c: .name = "reg-fixed-voltage",
arch/arm/mach-omap2/board-igep0020.c- .id = 0,
--
arch/arm/mach-omap2/board-4430sdp.c: .name = "reg-fixed-voltage",
arch/arm/mach-omap2/board-4430sdp.c- .id = -1,
--
arch/arm/mach-omap2/board-4430sdp.c: .name = "reg-fixed-voltage",
arch/arm/mach-omap2/board-4430sdp.c- .id = 1,
--
arch/arm/mach-omap2/board-omap3pandora.c: .name = "reg-fixed-voltage",
arch/arm/mach-omap2/board-omap3pandora.c- .id = 1,
igep0020.c does this:
static struct omap_smsc911x_platform_data smsc911x_cfg = {
.cs = IGEP2_SMSC911X_CS,
.gpio_irq = IGEP2_SMSC911X_GPIO,
.gpio_reset = -EINVAL,
.flags = SMSC911X_USE_32BIT | SMSC911X_SAVE_MAC_ADDRESS,
};
static inline void __init igep2_init_smsc911x(void)
{
gpmc_smsc911x_init(&smsc911x_cfg);
}
so it seems to use the gpmc_smsc stuff as well. So we're ending up with
two "reg-fixed-voltage" devices both with ID 0. That's not going to work,
and the driver model/sysfs will give you a nice warning there about that.
I think OMAP (or even the reg-fixed-voltage folk) needs to create an enum
similar to what I did for 8250 to control the allocation of platform
device IDs for this, otherwise we're going to keep on running over this
problem.
/*
* Allocate 8250 platform device IDs. Nothing is implied by
* the numbering here, except for the legacy entry being -1.
*/
enum {
PLAT8250_DEV_LEGACY = -1,
PLAT8250_DEV_PLATFORM,
PLAT8250_DEV_PLATFORM1,
PLAT8250_DEV_PLATFORM2,
PLAT8250_DEV_FOURPORT,
PLAT8250_DEV_ACCENT,
PLAT8250_DEV_BOCA,
PLAT8250_DEV_EXAR_ST16C554,
PLAT8250_DEV_HUB6,
PLAT8250_DEV_MCA,
PLAT8250_DEV_AU1X00,
PLAT8250_DEV_SM501,
};
Added Mark for his comment.
On Mon, Feb 13, 2012 at 04:52:09PM +0000, Russell King - ARM Linux wrote:
> I think OMAP (or even the reg-fixed-voltage folk) needs to create an enum
> similar to what I did for 8250 to control the allocation of platform
> device IDs for this, otherwise we're going to keep on running over this
> problem.
> Added Mark for his comment.
Hrm, seems slightly icky but the enum will work if we can decide how to
add elements to the enum since it'll be easy to bloat a lot it if it's
central. It's tempting to suggest just using a random number to assign
the IDs randomly rather than have a central registry but obviously
there's no guarantees there.
* Mark Brown <[email protected]> [120213 08:25]:
> On Mon, Feb 13, 2012 at 04:52:09PM +0000, Russell King - ARM Linux wrote:
>
> > I think OMAP (or even the reg-fixed-voltage folk) needs to create an enum
> > similar to what I did for 8250 to control the allocation of platform
> > device IDs for this, otherwise we're going to keep on running over this
> > problem.
>
> > Added Mark for his comment.
>
> Hrm, seems slightly icky but the enum will work if we can decide how to
> add elements to the enum since it'll be easy to bloat a lot it if it's
> central. It's tempting to suggest just using a random number to assign
> the IDs randomly rather than have a central registry but obviously
> there's no guarantees there.
Can't we just leave out the .id and have it automatically assigned?
Regards,
Tony
On Mon, Feb 13, 2012 at 09:52:34AM -0800, Tony Lindgren wrote:
> * Mark Brown <[email protected]> [120213 08:25]:
> > On Mon, Feb 13, 2012 at 04:52:09PM +0000, Russell King - ARM Linux wrote:
> >
> > > I think OMAP (or even the reg-fixed-voltage folk) needs to create an enum
> > > similar to what I did for 8250 to control the allocation of platform
> > > device IDs for this, otherwise we're going to keep on running over this
> > > problem.
> >
> > > Added Mark for his comment.
> >
> > Hrm, seems slightly icky but the enum will work if we can decide how to
> > add elements to the enum since it'll be easy to bloat a lot it if it's
> > central. It's tempting to suggest just using a random number to assign
> > the IDs randomly rather than have a central registry but obviously
> > there's no guarantees there.
>
> Can't we just leave out the .id and have it automatically assigned?
Unfortunately, there's no such infrastructure in the driver model.
* Russell King - ARM Linux <[email protected]> [120213 09:31]:
> On Mon, Feb 13, 2012 at 09:52:34AM -0800, Tony Lindgren wrote:
> > * Mark Brown <[email protected]> [120213 08:25]:
> > > On Mon, Feb 13, 2012 at 04:52:09PM +0000, Russell King - ARM Linux wrote:
> > >
> > > > I think OMAP (or even the reg-fixed-voltage folk) needs to create an enum
> > > > similar to what I did for 8250 to control the allocation of platform
> > > > device IDs for this, otherwise we're going to keep on running over this
> > > > problem.
> > >
> > > > Added Mark for his comment.
> > >
> > > Hrm, seems slightly icky but the enum will work if we can decide how to
> > > add elements to the enum since it'll be easy to bloat a lot it if it's
> > > central. It's tempting to suggest just using a random number to assign
> > > the IDs randomly rather than have a central registry but obviously
> > > there's no guarantees there.
> >
> > Can't we just leave out the .id and have it automatically assigned?
>
> Unfortunately, there's no such infrastructure in the driver model.
Hmm OK, -1 seems to be just used for name in platform_device_add().
Tony
On Mon, Feb 13, 2012 at 08:56:18AM -0800, Mark Brown wrote:
> On Mon, Feb 13, 2012 at 04:52:09PM +0000, Russell King - ARM Linux wrote:
>
> > I think OMAP (or even the reg-fixed-voltage folk) needs to create an enum
> > similar to what I did for 8250 to control the allocation of platform
> > device IDs for this, otherwise we're going to keep on running over this
> > problem.
>
> > Added Mark for his comment.
>
> Hrm, seems slightly icky but the enum will work if we can decide how to
> add elements to the enum since it'll be easy to bloat a lot it if it's
> central. It's tempting to suggest just using a random number to assign
> the IDs randomly rather than have a central registry but obviously
> there's no guarantees there.
Another option might be to provide a generic platform device helper
that instantiates a per-named-platform-device id list that simply
provides the next available id sequentially as requests for platform
device registration arrive.
arch init:
platform_create_id_list_create("reg-fixed-voltage");
platform init:
/* I = 0..N, .id filled in by _autoid() registration */
static struct platform_device my_fixed_regulatorN = {
.name = "reg-fixed-voltage",
.dev = ...
}
platform_device_register_autoid(&my_fixed_regulator0);
platform_device_register_autoid(&my_fixed_regulator1);
platform_device_register_autoid(&my_fixed_regulator2);
...
platform_device_register_autoid(&my_fixed_regulatorN);
Something like this might be able to solve this once for the platform
device users don't need a fixed enum id. It would require changes to
platform_device_register() to update the list as static assignment
of ids occurs so those will not be used by the _autoid() method.
-Matt
On Mon, Feb 13, 2012 at 10:14:10AM -0800, Tony Lindgren wrote:
> * Russell King - ARM Linux <[email protected]> [120213 09:31]:
> > On Mon, Feb 13, 2012 at 09:52:34AM -0800, Tony Lindgren wrote:
> > > Can't we just leave out the .id and have it automatically assigned?
It'd be nice but...
> > Unfortunately, there's no such infrastructure in the driver model.
> Hmm OK, -1 seems to be just used for name in platform_device_add().
-1 is a valid ID also, it means "there's only one of these things so
don't display a number". Which is sad but there we are. I'm at ELC/ABS
so I might try and find Greg in person here to see if we can come up
with something better, it seems like this is something the core ought to
be able to help with in much the same way that the USB stuff can.
Two more points:
On Mon, Feb 13, 2012 at 11:43:42AM -0500, Matt Porter wrote:
> This fixes smsc911x support on platforms using gpmc_smsc911x_init().
> Commit c7e963f6888816f04d1f5da0e07bec4e0092f227 added the requirement
Always include the commit summary as well here, so:
Commit c7e963f6888816 (net/smsc911x: Add regulator support) added the ...
> @@ -55,6 +94,11 @@ void __init gpmc_smsc911x_init(struct omap_smsc911x_platform_data *board_data)
>
> gpmc_cfg = board_data;
>
> + if (platform_device_register(&gpmc_smsc911x_regulator) < 0) {
> + pr_err("Unable to register smsc911x regulators\n");
> + return;
> + }
It's always a good idea to indicate why something failed:
err = platform_device_register(&gpmc_smsc911x_regulator);
if (err < 0) {
pr_err("Unable to register smsc911x regulators: %d\n", err);
return;
}
so that people can see not only what failed, but also why it failed.
* Mark Brown <[email protected]> [120213 10:27]:
> On Mon, Feb 13, 2012 at 10:14:10AM -0800, Tony Lindgren wrote:
> > * Russell King - ARM Linux <[email protected]> [120213 09:31]:
> > > On Mon, Feb 13, 2012 at 09:52:34AM -0800, Tony Lindgren wrote:
>
> > > > Can't we just leave out the .id and have it automatically assigned?
>
> It'd be nice but...
>
> > > Unfortunately, there's no such infrastructure in the driver model.
>
> > Hmm OK, -1 seems to be just used for name in platform_device_add().
>
> -1 is a valid ID also, it means "there's only one of these things so
> don't display a number". Which is sad but there we are. I'm at ELC/ABS
> so I might try and find Greg in person here to see if we can come up
> with something better, it seems like this is something the core ought to
> be able to help with in much the same way that the USB stuff can.
Matt, care to refresh your original patch using some other number so
we can apply it as a regression fix for the -rc series while the
long term solution is being discussed?
Just use some random number for now with a comment, 42?
Regards,
Tony
On Tue, Feb 21, 2012 at 10:46:48AM -0800, Tony Lindgren wrote:
> * Mark Brown <[email protected]> [120213 10:27]:
> > On Mon, Feb 13, 2012 at 10:14:10AM -0800, Tony Lindgren wrote:
> > > * Russell King - ARM Linux <[email protected]> [120213 09:31]:
> > > > On Mon, Feb 13, 2012 at 09:52:34AM -0800, Tony Lindgren wrote:
> >
> > > > > Can't we just leave out the .id and have it automatically assigned?
> >
> > It'd be nice but...
> >
> > > > Unfortunately, there's no such infrastructure in the driver model.
> >
> > > Hmm OK, -1 seems to be just used for name in platform_device_add().
> >
> > -1 is a valid ID also, it means "there's only one of these things so
> > don't display a number". Which is sad but there we are. I'm at ELC/ABS
> > so I might try and find Greg in person here to see if we can come up
> > with something better, it seems like this is something the core ought to
> > be able to help with in much the same way that the USB stuff can.
>
> Matt, care to refresh your original patch using some other number so
> we can apply it as a regression fix for the -rc series while the
> long term solution is being discussed?
>
> Just use some random number for now with a comment, 42?
Will do, I'll post an update with this approach.
-Matt
On Tue, Feb 14, 2012 at 11:08:36AM +0000, Russell King - ARM Linux wrote:
> Two more points:
>
> On Mon, Feb 13, 2012 at 11:43:42AM -0500, Matt Porter wrote:
> > This fixes smsc911x support on platforms using gpmc_smsc911x_init().
> > Commit c7e963f6888816f04d1f5da0e07bec4e0092f227 added the requirement
>
> Always include the commit summary as well here, so:
>
> Commit c7e963f6888816 (net/smsc911x: Add regulator support) added the ...
Ok. Addressed in v2.
> > @@ -55,6 +94,11 @@ void __init gpmc_smsc911x_init(struct omap_smsc911x_platform_data *board_data)
> >
> > gpmc_cfg = board_data;
> >
> > + if (platform_device_register(&gpmc_smsc911x_regulator) < 0) {
> > + pr_err("Unable to register smsc911x regulators\n");
> > + return;
> > + }
>
> It's always a good idea to indicate why something failed:
>
> err = platform_device_register(&gpmc_smsc911x_regulator);
> if (err < 0) {
> pr_err("Unable to register smsc911x regulators: %d\n", err);
> return;
> }
>
> so that people can see not only what failed, but also why it failed.
Looks much better, also addressed in v2.
-Matt