Re: newbus' ivar's limitation..

On Jul 30, 2012, at 8:30 PM, Arnaud Lacombe wrote:

> 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).

Yes.  FDT should replace hinted things as much as possible.  However, FDT is an in for a penny, in for a pound technology like acpi: you more or less have to use it for everything.

>>>> 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.

Yes.  I made it that way on purpose because most buses are enumerated, and things are moving that way even in the embedded world, or at least seemed that way when I was doing it.  Either the buses are enumerated, like PCI or some of the silicon frameworks, or you used FDT, which is also fully enumerated.

>>>> 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().

Yes.  This was specifically due to how different buses enumerate.

>>>> 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.

It has been a mistake to not more aggressively investigate this line of coding.

>>>> 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.

It is almost always horrifically wrong.

>>>> 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.

FDT handles the enumeration problem.  FreeBSD's lack of a decent gpio, pinctl, pinmux and other infrastructure are much bigger issues.  At least those are the real issues that I'm running into working on the Atmel SoCs and bringing FDT to them.  Hinted buses really have no place in an FDT world.  None.  They are an ugly hack that were intended to be a stop-gap until something better than hints came along.  If you are trying to use them in an FDT world, you are likely doing something horribly wrong.

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

That's because you're asking us to pound screws with a hammer.  For the things you've complained about, we really need to have better GPIO infrastructure, a better pin control and pin mux infrastructure, etc.  We lack that right now, which is why you're trying to shoe-horn the FDT connections into a newbus world and complaining that everything sucks because it is a poor fit.  I'd suggest that different mechanisms are necessary.

>> 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.

Perhaps, but design patterns can be just as useful for extremely short code snippets.

Warner