Anton Vorontsov wrote:
On Tue, Mar 18, 2008 at 02:55:14PM -0500, Scott Wood wrote:
Anton Vorontsov wrote:
+arch_initcall(qe_init_gtm);
If this happens before the gtm_init_gtm(),
"If" isn't possible, order is guaranteed.
You use arch_initcall for both, so you're relying on link order. I think this at least merits a comment.
then np->data will not be set.
It's a bug in the device tree or in the Linux code then.
Hmm? It's set by gtm_init_gtm(). If this code runs before gtm_init_gtm(), what are you expecting to initialize np->data?

What code exactly?

Sorry, "this code" == qe_init_gtm(). Obviously, if you assume that gtm_init_gtm() will always be linked earlier, then it's not an issue.

Also, what if some arch_initcall runs between gtm_init_gtm and qe_init_gtm, that registers itself as a client of the gtm driver, and uses the wrong clock value?

Again, what code exactly?   If it is a driver (for what this API is
created for), it hardly will run earlier than arch/ code.   If this is
platform code (arch/powerpc/platform/), then it is hardly will run
earlier than arch/sysdev/. Inside the arch/sysdev/ fsl_gtm.c is
guaranteed to run earlier than qe_lib/gtm.c. So, where is the problem?

That's a lot of implicit, undocumented dependency on link order... Things can be moved around, and driver-ish code can pop up in surprising places. All I meant was that having the gtm driver present itself as ready when it isn't, in a way which isn't readily apparent if it happens, is worrysome.

Since I'll implement clock-frequency inside the timer node, this
isn't relevant anymore...

OK, good.

Ah. You need specific timer. No problem. I don't like idea of new arguments
to the gtm_get_timer() function (complicates things), but we can just
implement another one. gtm_get_timer_<name>, choice the name please.
_specific, _2, _for, __gtm_get_timer, ...

How about:

struct gtm_timer *gtm_get_specific_timer(struct gtm *gtm, int timer,
                                         int width);

...with np->data used by the caller to figure out which gtm pointer to pass in.

-Scott
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Reply via email to