On 07/03/12 00:12, the mail apparently from Arnd Bergmann included:
On Monday 02 July 2012, Andy Green wrote:
From: Andy Green <andy@xxxxxxxxxxx>
This introduces a small helper in net/ethernet, which registers a
network notifier on init, and accepts registrations of expected asynchronously-
Yes, looks good to me in principle. It needs to get sent to the linux-kernel
and netdev mailing lists for review though. A few small comments.
I wanted to hear that it had actually converged with what was being
talked about first. I just sent out a v3 with the relevant patch also
cc-d to those lists but stg mail didn't seem to send it to them. I'll
wait for any further comments here and resend the whole thing including
those lists.
I find the name a bit non-obvious, not sure if we can come up with the
perfect one.
How about mac-platform?
Done.
endif # if NET
This one needs to be either a silent option (only selected by the
platforms that want it) or the header file has to provide a
static-inline fallback that does nothing, so we don't get
a build failure on platforms that need it if the option gets
disabled.
I'd prefer making it a silent option because then we don't bloat
the kernel if someone accidentally enables it on a platform that
does not use it.
Done. It's already added as a select on the Panda board config.
I also moved it out of the "if NET" clause and took care about the case
that CONFIG_NET is off but we still have mac-platform references.
+static struct mac_la_ap mac_la_ap_list;
The normal way to do this is to have just a list head that the
entries get added to:
static LIST_HEAD(mac_la_ap_list);
That also takes care of the initialization.
Thanks for normalizing it, done.
+DEFINE_MUTEX(mac_la_ap_mutex);
+bool mac_la_ap_started;
These should all be static.
Right, done.
I think it would be simpler to register the notifier from an
initcall and drop the mac_la_ap_started variable.
That was my first approach, to structure it as a real driver. I had
tried a few likely-looking initcall levels but the init of the helper
was coming after the init of the network code.
I found this time that core_initcall works, so I used that, dropped the
flag. But isn't it a bit tricky with these to know if the prerequisite
you're trying to ensure is already initialized might not actually be at
the same initcall level and you're working by accident?
+int mac_la_ap_register_device_macs(const struct mac_la_ap *macs)
+{
+}
+EXPORT_SYMBOL_GPL(mac_la_ap_register_device_macs);
I'm not sure if there is any point in exporting this function, my feeling
is that it would only ever be called from built-in code. If we can call it
from a module, we should probably also allow this file to be a loadable
module as well.
Being a modular API for platform code to call doesn't make sense, I got
rid of the export.
-Andy
--
Andy Green | TI Landing Team Leader
Linaro.org │ Open source software for ARM SoCs | Follow Linaro
http://facebook.com/pages/Linaro/155974581091106 -
http://twitter.com/#!/linaroorg - http://linaro.org/linaro-blog
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
