Re: newbus' ivar's limitation..

Hi,

On Mon, Jul 30, 2012 at 5:06 PM, John Baldwin <jhb_at_freebsd.org> wrote:
> On Tuesday, July 17, 2012 2:03:14 am Arnaud Lacombe wrote:
>> Hi,
>>
>> On Fri, Jul 13, 2012 at 1:56 PM, Arnaud Lacombe <lacombar_at_gmail.com> wrote:
>> > Hi,
>> >
>> > On Thu, Jul 12, 2012 at 1:20 AM, Warner Losh <imp_at_bsdimp.com> wrote:
>> >> [..]
>> >> Honestly, though, I think you'll be more pissed when you find out that
> the N:1 interface that you want is being done in the wrong domain.  But I've
> been wrong before and look forward to seeing your replacement.
>> >>
>> > I will just pass function pointers for now, if things should be done
>> > dirty, let's be explicit about it.
>> >
>> > Now, the hinted device attachment did work quite smoothly, however, I
>> > would have a few suggestion:
>> >  1) add a call to bus_enumerate_hinted_children() before the call
>> > DEVICE_IDENTIFY() call in bus_generic_driver_added()
>> >
>> > this is required to be able to support dynamic loading and attachment
>> > of hinted children.
>
> I'm not sure this is a feature we want to support (to date hinted children
> have only been created at boot time).
>
>> >  2) have a generic bus_hinted_child method which would just add a new
>> > child to the bus.
>
> Possibly, but hinted children are intentionally opt-in and not enabled
> by default.
>
>> >  3) have bus_enumerate_hinted_children() and bus_generic_attach()
>> > always ran on device attachment.
>> >
>> > There is current +100 explicit call to bus_generic_attach() in the
>> > sys/dev/ tree. This should be done always and implicitly.
>
> No.  One of the problems is that different busses want to do it at
> different times.  It is usually done last, but some buses may want to
> do additional work after the bus_generic_attach().
>
>> >  4) have bus_generic_detach() always ran prior to device detachment
>
> Similar.
>
>> >  5) have the bus_generic_* method be the default of their respective
> method
>
> No.  However, what would be a good idea (and one thing I've had on my
> list), is to create a "bus_generic" driver that uses the bus_generic_*
> methods for all it's methods and let bus drivers inherit from that to
> get the generic methods if they are appropriate.  If you do this, I
> would also add a second "bus_generic_rl" that inherits from "bus_generic"
> but uses the generic resource list methods for resources.
>
>> >  6) have device_delete_child() called upon device detachment.
>
> No.  This is for a bus to decide.  This would be horrifically wrong
> for things like kldunloading a PCI device driver.  Just because you
> unload a driver (so that it detaches from devices) does not mean those
> physical devices have gone away.
>
>> > As a rule of thumb, when a kld is unloaded there should not be any
>> > remains of anything built previously. Without device_delete_child() or
>> > proper singleton implementation, multiple load/unload sequence of bus
>> > will attempt to attach multiple version of a child, even if the single
>> > child was added prior to the bus_generic_attach() call.
>
> Hinted devices should perhaps be removed, yes.  However, what other drivers
> often do is use a singleton approach instead (despite your claim that they
> don't).
>
FreeBSD's newbus device framework already sucks (as in "too
static"/"inflexible"), making it sucks even more (as in "more
static"/"more inflexible") might not be the wisest approach... This is
no longer the 90'. The good old enumerating-buses, tree-based, model
is to be relegated to a corner-case of the computer world with
profusion of embedded, non-enumerating, highly integrated, highly
interconnected, highly custom SoCs.

I am yet to see a robust approach to the various problem I submitted.

> For example:
> static void
> ipmi_isa_identify(driver_t *driver, device_t parent)
> {
>         struct ipmi_get_info info;
>         uint32_t devid;
>
>         if (ipmi_smbios_identify(&info) && info.iface_type != SSIF_MODE &&
>             device_find_child(parent, "ipmi", -1) == NULL) {
>                 ...
>                 BUS_ADD_CHILD(parent, 0, "ipmi", -1);
>         }
> }
>
duplicated code doing the exact same abstract, hardcoded, function,
all over the tree, absolutely unacceptable, if not a blatant proof of
failure of what should be made generic, if not fully dynamic.

 - Arnaud