Date: Sat, 29 Dec 2007 11:29:35 -0800 From: Marcel Moolenaar <marcelm@juniper.net> To: "M. Warner Losh" <imp@bsdimp.com> Cc: embedded@freebsd.org Subject: Re: ocpbus(4) Message-ID: <D1988B62-7E9F-48D6-806E-C8625F498B17@juniper.net> In-Reply-To: <20071228.192115.783710089.imp@bsdimp.com> References: <1B0F37B8-D492-4E15-82A8-704243E67E90@juniper.net> <20071228.181939.-957829045.imp@bsdimp.com> <6712844B-1863-4316-9BEE-E43E9BE232A8@juniper.net> <20071228.192115.783710089.imp@bsdimp.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Dec 28, 2007, at 6:21 PM, M. Warner Losh wrote:
> : Thus, if uart(4) has an ocpbus bus attachment, it will
> : expect a couple of IVARs. Any SoC that has a ns8250 can
> : implementation those IVARs and have uart(4) attach with
> : the ocpbus attachment. No need to also create a new
> : bus attachment. Look at puc(4) and scc(4). Both expose
> : an abstract bus and both have separate bus attachments
> : for uart(4). It would be nice if we can stop the growth
> : of bus attachments (especially for uart(4)).
>
> I'm not sure how much you can do about that. You'll run into problems
> doing that. If you have a 'generic' attachment for uart, then you'll
> have code that looks like:
>
> switch (class) {
> case NS8250: sc->sc_class = &uart_ns8250_class; break;
> case AT91: sc->sc_class = &at91_usart_class; break;
> case CIRRUS: sc->sc_class = &uart_cirrus_class; break;
> case ARMIP: sc->sc_class = &uart_armip_class; break;
> ...
> }
>
> which drags in a bunch of unwanted code. The ARMIP only exists on one
> of our mips ports, while the cirrus and at91 ports typically don't
> have ns8250.
Ah, but that is easy to handle with weak references. If the
symbol you reference (like above) is not compiled-in, you
get a NULL value. So, you check the IVAR to get the class,
use a weak reference to get the address of the uart class
structure and if that's NULL, you know you can't attach.
The only additional code is the case statement in the switch
(like above), but nothing else. That can't be too bad?
No ifdefs in sight either...
> : > : > I agree that's a noble goal, and maybe we can avoid it in many
> : > cases,
> : > : > but not in all cases. I just don't think having a large
> : > enumeration
> : > : > table to make this work is worth the effort of maintaining it.
> : > When
> : > : > you go to add a new ocpbus driver, you need to touch
> additional
> : > places
> : > : > than the direct method.
> : > :
> : > : How so?
> : >
> : > If you had a string that's the device to attach in the table,
> then you
> : > wouldn't have to edit ocpbusvar.h, or whatever file defined the
> : > enumeration, every time you added new devices that could attach to
> : > ocpbus.
> :
> : True. There will be a single header that defines the magic
> : constants and that header needs updating whenever we need
> : to add a magic constant.
>
> Right. For me, this is a needless file that will be a cause of
> integration problems. This is my biggest complaint: we're
> reintroducing a file that lists all possible devices.
Granted. Since the big win is in re-using common code, any
driver that's specific to a platform is not going to benefit
from this. So, what if we put only the MI drivers in there
(i.e. uart and maybe a handful of others) and simply reserve
a range for MD drivers or platform specific drivers.
We don't list all possible drivers; only those that share
the bus attachment across platforms and CPU models.
Would that still be useful, or do we limit the scope too
much then?
--
Marcel Moolenaar
marcelm@juniper.net
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?D1988B62-7E9F-48D6-806E-C8625F498B17>