Re: call for testing: completing the device/softc split

[Chuck Silvers <chuq%chuq.com@localhost>]

On Wed, Oct 10, 2012 at 03:57:30AM +0900, Izumi Tsutsui wrote:
> chs@ wrote:
> 
> > I put together a patch to complete the device/softc split
> > for the remaining drivers in the tree.  the patch is at:
> > http://ftp.netbsd.org/pub/NetBSD/misc/chs/diff.devsoftc.8
> 
> >> +++ sys/arch/amiga/amiga/autoconf.c     9 Oct 2012 02:53:15 -0000
> >> @@ -156,34 +156,52 @@ matchname(const char *fp, const char *sp
> >>   * always tell the difference betwean the real and console init
> >>   * by checking for NULL.
> >>   */
> >> +struct qq {
> >> +  int q;
> >> +  int c;
> >> +} qq;
> 
> Debug leftover?

oops, yes, thanks for catching that.


> >> +++ sys/dev/pci/if_devar.h      26 Sep 2012 23:44:22 -0000
> >> @@ -487,7 +487,7 @@ struct _tulip_softc_t {
> >>  #endif /* _BSDI_VERSION < 199401 */
> >>  #endif /* __bsdi__ */
> >>  #if defined(__NetBSD__)
> >> -    struct device tulip_dev;           /* base device */
> >> +    device_t tulip_dev;                /* base device */
> 
> Some more macro?
> >> #define tulip_unit                 tulip_dev.dv_unit
> 
> Or it's time to obsolete de driver?

last I heard, there were still some tulips that worked with de and not with tlp.
I'll update the de driver, though apparently no kernel config in the tree uses 
it...
I built every kernel config for every platform to make sure things compiled.

though I wonder how I missed that?  I grep'd for "struct device",
I must have been confused by the ifdef mess in there.
... and apparently my grep didn't handle tabs between the words,
I see I missed some other instances with varying whitespace.


> It would be nice to split the patch into two parts,
> cosmetic only changes (struct device * -> device_t,
> device_xname() macro etc) and actual split
> (CFATTACH_DECL -> CFATTACH_DECL_NEW with softc)
> that could have many pitfalls. (conversion between
> device_t and softc via (void *) casts/pointers)

yea, that would have been a better way to go about it,
but at this point the changes are pretty hopelessly intertwined.
separating them by hand now just isn't practical.
most of the device_xname() parts could probably be isolated with some
text processing of the diff, and perhaps quite a bit of the
"struct device *" -> "device_t" parts as well.
I can try doing that if more people actually want to look at
the code changes.

but I didn't expect that anyone would want to wade through even the
interesting changes, I was really only hoping for people to test.


> (though actually you've caught some botches in x68k ;-)

the adapt_dev thing?  yea, that was one of the useful things
that I found to grep for.

I also noticed that this patch fixes a problem on sparc,
booting a -current sparc GENERIC + QUEUEDEBUG hits an assertion
about the alldevs list being corrupted, whereas with this patch
it's fine.  it wasn't obvious exactly what I fixed though.

-Chuck