[src] cvs commit: src/sys/dev/fdc fdc.c fdc_isa.c fdc_pccard.c fdcvar.h src/sys/modules/fdc Makefile

[Nate Lawson]

M. Warner Losh wrote:

> In message: <40EC7A5A.3010303 at root.org>
>             Nate Lawson <nate at root.org> writes:
> : Warner Losh wrote:
> : > imp         2004-07-07 22:35:27 UTC
> : > 
> : >   FreeBSD src repository
> : > 
> : >   Modified files:
> : >     sys/dev/fdc          fdc.c 
> : >     sys/modules/fdc      Makefile 
> : >   Added files:
> : >     sys/dev/fdc          fdc_isa.c fdc_pccard.c fdcvar.h 
> : >   Log:
> : >   Break out the isa and pccard front ends from fdc.  This is the first
> : >   step in making this driver more attachment neutral.  Others plan on
> : >   adding acpi front ends.
> : >   
> : >   Still need to cleanup the MI part of the driver because it isn't as
> : >   bus independent as it could be.
> : >   
> : >   Revision  Changes    Path
> : >   1.276     +12 -304   src/sys/dev/fdc/fdc.c
> : >   1.1       +145 -0    src/sys/dev/fdc/fdc_isa.c (new)
> : >   1.1       +114 -0    src/sys/dev/fdc/fdc_pccard.c (new)
> : >   1.1       +134 -0    src/sys/dev/fdc/fdcvar.h (new)
> : >   1.9       +2 -14     src/sys/modules/fdc/Makefile
> : 
> : I don't think you should put your copyright on code that is 100% from 
> : fdc.c.  Bruce complained about this for my version of fdcvar.h.
> 
> Also, I do have a copyright on it.  That's 100% legit.  I did write
> parts of this file.  The thing that isn't legit is failing to include
> the relevant parts of fdc.c's copyright.  I noticed literally at the
> last minute before committing that there was no copyright on the files
> so I added mine with a note that said I should audit things.  For
> example, none of this code was in the original 1990 version of fdc.c
> (except maybe the softc that was in fdcvar.h).

Ok, I'll let you sort that out.

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

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.

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.

-- 
-Nate