SoC child devices strike back, was Re: H2200 - Red charging LED is not working

Hello Gerhard,

Monday, July 2, 2007, 9:03:51 PM, you wrote:

[]
>> > Switching on Bluetooth gives me the lines:
>> > hamcop_set_led: LED number 2, 1, 31
>> > hamcop_set_led: LED number 2, 0, 0
>> > The blue LED is flashing!
>>
>> Bluetooth LED is working just because it access hamcop in old adhoc
>> way, that needs to be fixed once leds-hamcop is debugged.
>>
>> > Putting h2200 into craddle or give power to the plug I see that the
>> > battery gets loaded but nothing happens. No LED is flashing, no dmesg
>> > output is created in leds-hamcop.c.
>> >
>> > leds-hamcop.c seems not to be the problem.
> I see that there are a lot of pr_debug statements in leds-hamcop.c but as I
> told you yesterday, the above two lines are the only thing I got by switching
> Bluetooth on and off. No additional lines could be found in dmesg file.

  Woops. I only finally "noticed":

EXPORT_SYMBOL_GPL(hamcop_leds_register);
EXPORT_SYMBOL_GPL(hamcop_leds_unregister);

  Anton, please remind me, why do we have such hack in all our leds
drivers instead of

module_init(hamcop_leds_register);
module_exit(hamcop_leds_unregister);

? It's all that child devices problem, right? And it's time to solve
it via existing child devices patterns? And note that we have two
different methods to deal with child devices after all:

1. Making SoC driver register its subblocks per its own internal list.
2. Making SoC driver register externally passed list of
platform_devices.

  And let's consider when we use one or another, and if we need two of
them. So, first take advantage that we don't need to repeat subblock
definition for each client machine. We also handle bus_shift this way,
but well, there's apparently nothing impossible in making it work for
externally passed platform_device list.

  Second method is used when we have separate device, not really a
part of SoC, but which still depends on some SoC-provided resource.
As we must ensure that this resource is available before this separate
device is used, we must postpone its registration until we sure that
the SoC is up. And of course, in most, if not only, cases this
resource is SoC-provided EGPIO. That's the sole purpose of this second
method.

  And here's the crucial question - should we consider SoCs
in question a normal device or after all an unalienable part of platform,
always available. If make the second choice, no such parent-child
hierarchies are required, but at the same time, modular SoC drivers
are not allowed, and when we come to building cross-device modular
kernels (hopefully soon), all SoC drivers must be compiled in.

  Making first choice, we need to add child devices support
consistently for all SoC drivers, and accept the fact that the same
device will be initialized at different times on different machines.
And some hacks will still be required, for example for devices which
are builtin to CPU (and thus registered in adhoc ways), but which
depend on SoC EGPIO (example: UDC on h4000).

       So, both approaches have drawbacks. Besides mentioned, child
devices approach also apparently introduce new concept to mainline,
while static SoC is not very well bound - for example, what if UDC
pull-up pins are on some I2C GPIO extender? Patch it to allow adhoc
initialization and make whole I2C subsystem builtin? (OTOH, child
devices support for it would need patching either).

        So, please let's decide on this matter this time, as it keeps
popping up again and again.

        Well, and how this all relates to leds-hamcop problem? If we
chose child devices approach for it, we'd need to add child devices
support for hamcop_base. Otherwise, need to add .leds_params to
struct hamcop_platform_data (and make corresponding change to all
other SoC drivers too!)

> I meanwhile realize that the two lines

> hamcop_set_led: LED number 2, 1, 31
> hamcop_set_led: LED number 2, 0, 0

> are printed also during reboot as the blue LED is flashing once during boot.

>> Based on your report, and answering your question in another mail,
>> leds-hamcop exactly appears to be a culprit. It's what registers entries
>> in /sys/class/leds/. So, please debug it - there're lots of printk's
>> already.

> Sorry to couldn't give you more info from DEBUG output.
> Gerhard

-- 
Best regards,
 Paul                            mailto:pmiscml_at_gmail.com