lkml.org 
[lkml]   [2012]   [Mar]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [PATCH v3] ARM: OMAP2+: gpmc-smsc911x: add required smsc911x regulators
    Date
    Hi Russ,

    Russ Dill <Russ.Dill@ti.com> writes:

    > On Mon, Mar 5, 2012 at 12:18 PM, Kevin Hilman <khilman@ti.com> wrote:
    >> Felipe Balbi <balbi@ti.com> writes:
    >>
    >>> Hi,
    >>>
    >>> On Thu, Mar 01, 2012 at 12:36:57PM -0800, Kevin Hilman wrote:
    >>>> Matt Porter <mporter@ti.com> writes:
    >>>>
    >>>> > This fixes smsc911x support on platforms using gpmc_smsc911x_init().
    >>>> >
    >>>> > Commit c7e963f6888816 (net/smsc911x: Add regulator support) added
    >>>> > the requirement that platforms provide vdd33a and vddvario supplies.
    >>>> >
    >>>> > Signed-off-by: Matt Porter <mporter@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 <khilman@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 <mporter@ti.com>
    >>>> Signed-off-by: Kevin Hilman <khilman@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>] (unwind_backtrace+0x0/0xf0) from [<c022b944>] (kobject_init+0x74/0x94)
    [ 0.355804] [<c022b944>] (kobject_init+0x74/0x94) from [<c0284fa8>] (device_initialize+0x20/0x90)
    [ 0.365112] [<c0284fa8>] (device_initialize+0x20/0x90) from [<c02894fc>] (platform_device_register+0x10/0x24)
    [ 0.375488] [<c02894fc>] (platform_device_register+0x10/0x24) from [<c0619d7c>] (gpmc_smsc911x_init+0x1c/0x200)
    [ 0.386077] [<c0619d7c>] (gpmc_smsc911x_init+0x1c/0x200) from [<c06165e0>] (overo_init+0xdc/0x27c)
    [ 0.395446] [<c06165e0>] (overo_init+0xdc/0x27c) from [<c0609380>] (customize_machine+0x1c/0x28)
    [ 0.404663] [<c0609380>] (customize_machine+0x1c/0x28) from [<c0008800>] (do_one_initcall+0x34/0x178)
    [ 0.414306] [<c0008800>] (do_one_initcall+0x34/0x178) from [<c06068ac>] (kernel_init+0x8c/0x130)
    [ 0.423553] [<c06068ac>] (kernel_init+0x8c/0x130) from [<c000e850>] (kernel_thread_exit+0x0/0x8)
    [ 0.432800] ------------[ cut here ]------------
    [ 0.437652] WARNING: at /work/kernel/omap/pm/fs/sysfs/dir.c:481 sysfs_add_one+0x88/0xb0()
    [ 0.446228] sysfs: cannot create duplicate filename '/devices/platform/reg-fixed-voltage.42'
    [ 0.455047] Modules linked in:
    [ 0.458312] [<c001421c>] (unwind_backtrace+0x0/0xf0) from [<c00391e4>] (warn_slowpath_common+0x4c/0x64)
    [ 0.468170] [<c00391e4>] (warn_slowpath_common+0x4c/0x64) from [<c0039290>] (warn_slowpath_fmt+0x30/0x40)
    [ 0.478179] [<c0039290>] (warn_slowpath_fmt+0x30/0x40) from [<c014eed8>] (sysfs_add_one+0x88/0xb0)
    [ 0.487548] [<c014eed8>] (sysfs_add_one+0x88/0xb0) from [<c014ef60>] (create_dir+0x60/0xc4)
    [ 0.496307] [<c014ef60>] (create_dir+0x60/0xc4) from [<c014f048>] (sysfs_create_dir+0x58/0x8c)
    [ 0.505340] [<c014f048>] (sysfs_create_dir+0x58/0x8c) from [<c022bbec>] (kobject_add_internal+0x9c/0x1b8)
    [ 0.515350] [<c022bbec>] (kobject_add_internal+0x9c/0x1b8) from [<c022bfd8>] (kobject_add+0x50/0x98)
    [ 0.524932] [<c022bfd8>] (kobject_add+0x50/0x98) from [<c0285a14>] (device_add+0xb8/0x358)
    [ 0.533599] [<c0285a14>] (device_add+0xb8/0x358) from [<c0289234>] (platform_device_add+0xf8/0x1a4)
    [ 0.543090] [<c0289234>] (platform_device_add+0xf8/0x1a4) from [<c0619d7c>] (gpmc_smsc911x_init+0x1c/0x200)
    [ 0.553283] [<c0619d7c>] (gpmc_smsc911x_init+0x1c/0x200) from [<c06165e0>] (overo_init+0xdc/0x27c)
    [ 0.562652] [<c06165e0>] (overo_init+0xdc/0x27c) from [<c0609380>] (customize_machine+0x1c/0x28)
    [ 0.571868] [<c0609380>] (customize_machine+0x1c/0x28) from [<c0008800>] (do_one_initcall+0x34/0x178)
    [ 0.581512] [<c0008800>] (do_one_initcall+0x34/0x178) from [<c06068ac>] (kernel_init+0x8c/0x130)
    [ 0.590728] [<c06068ac>] (kernel_init+0x8c/0x130) from [<c000e850>] (kernel_thread_exit+0x0/0x8)
    [ 0.600372] ---[ end trace 1b75b31a2719ed1c ]---
    [ 0.605285] kobject_add_internal failed for reg-fixed-voltage.42 with -EEXIST, don't try to register things with the same name in the same directory.
    [ 0.619262] [<c001421c>] (unwind_backtrace+0x0/0xf0) from [<c022bcf0>] (kobject_add_internal+0x1a0/0x1b8)
    [ 0.629272] [<c022bcf0>] (kobject_add_internal+0x1a0/0x1b8) from [<c022bfd8>] (kobject_add+0x50/0x98)
    [ 0.638946] [<c022bfd8>] (kobject_add+0x50/0x98) from [<c0285a14>] (device_add+0xb8/0x358)
    [ 0.647613] [<c0285a14>] (device_add+0xb8/0x358) from [<c0289234>] (platform_device_add+0xf8/0x1a4)
    [ 0.657073] [<c0289234>] (platform_device_add+0xf8/0x1a4) from [<c0619d7c>] (gpmc_smsc911x_init+0x1c/0x200)
    [ 0.667266] [<c0619d7c>] (gpmc_smsc911x_init+0x1c/0x200) from [<c06165e0>] (overo_init+0xdc/0x27c)
    [ 0.676666] [<c06165e0>] (overo_init+0xdc/0x27c) from [<c0609380>] (customize_machine+0x1c/0x28)
    [ 0.685852] [<c0609380>] (customize_machine+0x1c/0x28) from [<c0008800>] (do_one_initcall+0x34/0x178)
    [ 0.695526] [<c0008800>] (do_one_initcall+0x34/0x178) from [<c06068ac>] (kernel_init+0x8c/0x130)
    [ 0.704711] [<c06068ac>] (kernel_init+0x8c/0x130) from [<c000e850>] (kernel_thread_exit+0x0/0x8)
    [ 0.713897] gpmc_smsc911x_init: Unable to register smsc911x regulators: -17

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2012-03-07 20:39    [W:0.042 / U:149.816 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site