Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 07 Jul 2004 18:58:14 -0600 (MDT)
From:      "M. Warner Losh" <imp@bsdimp.com>
To:        nate@root.org
Cc:        cvs-all@FreeBSD.org
Subject:   Re: [src] cvs commit: src/sys/dev/fdc fdc.c fdc_isa.c fdc_pccard.c fdcvar.h src/sys/modules/fdc Makefile
Message-ID:  <20040707.185814.113735786.imp@bsdimp.com>
In-Reply-To: <40EC9698.4050201@root.org>
References:  <40EC7A5A.3010303@root.org> <20040707.183145.79073073.imp@bsdimp.com> <40EC9698.4050201@root.org>

next in thread | previous in thread | raw e-mail | index | archive | help
In message: <40EC9698.4050201@root.org>
            Nate Lawson <nate@root.org> writes:
: > : Also, you went the path of completely exposing the softc (and the 
: > : requisite enum fields).  In response to Bruce's comments, I had made 
: > : major efforts to hide it and have accessor functions for the probe 
: > : routines.  This is why I didn't commit the acpi attachment a month ago 
: > : (and still haven't committed it).  I'm a little frustrated that it 
: > : appears I could have gone with the exposed softc approach and saved 
: > : quite a few hours of work.
: > 
: > I went the route of 'exposing' the softc, because that's how newbus
: > works: it manages the softc, and in order to manage the softc you have
: > to expose its size.  I treid to express that in the reviews, but I
: > guess that got lost in the shuffle.  Bruce doesn't like it, but we do
: > it all over the place and the world hasn't come to the end.  Anyway,
: > after the set of mail that was sent out, I thought that I was supposed
: > to commit the simple split, then you were going to specialize things
: > for acpi.  That's why I went ahead and committed this.  phk's recent
: > changes to fdc reminded me to merge this stuff....
: 
: What about instead exposing the size through a extern const int and used 
: that to set the softc size in the device initialization?  The internals 
: of the softc aren't really needed.

No other driver does that.  Nearly all the other drivers in the tree
just expose softc in an include file that nobody else includes.
There's no benefit from doing this.

: My code uses a softc size of zero and has a function in fdc.c that is 
: called by bus attachments that allocates it and does a 
: device_set_softc().  This mostly works.

That's a perversion of how newbus works.  Rather than fighting the
tools, we should use them correctly.  We shouldn't use special use
tools to do an end run around newbus when the normal newbus tools
work.

: The problem I've run into involves the question that when multiple 
: drivers can claim a device, can you use anything in the device to pass 
: info to the attach routine?  I want to do a device_set_flags() to pass 
: the PNP flag to isa_attach() iff the isa PNP routine wins the probe.  Is 
: this ok since device_probe will be called a second time on the winning 
: driver?  As long as I destructively set the flags, it seems this is ok.

Generally this is considered bad form.  The only reason we call probe
the second time is to get the device name correct.  We may optimize
this away at some point, or we may call probe dozens of time in the
future.  Generally, you put code into attach routine to cope.  The
attach routine can call the get pnp id call in isa to find out if it
is plug and play and set the flags there.

This was just a first cut at getting the attachment separated out.
More work is needed to get the code better separated ot make the
pccard case work (the FDC_ISPNP and FDC_ISPCMCIA flags I think aren't
right).

Normally drivers have a common alloc_resources, but the floppy disk
can have so many different allocations from the upper layers that I
think that we'll have problems having one for all the attachments.

I think once I have pccard attachment is working, we should revisit
things.

Warner



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20040707.185814.113735786.imp>