Re: ACPI 'driver bug: Unable to set devclass'

On Wednesday, May 16, 2012 12:18:25 pm Andriy Gapon wrote:
> on 16/05/2012 17:50 John Baldwin said the following:
> > On Tuesday, May 15, 2012 12:35:12 pm Andriy Gapon wrote:
> >> Not sure what you disagree with...
> >> First, the wildcard device is added to the child list during the walk.
> >> Then, the unit 0 device is added to the list when acpi_timer identify is executed.
> >> Then, the wildcard device is probed and gets unit number of zero.
> >> Then, the fixed device is being probed and the unit number conflict arises.
> >>
> >> Am I misunderstanding something?
> > 
> > Yes.  The third step will see that unit 0 is already in use and shouldn't
> > reuse unit 0.
> > 
> 
> Looks like I missed the call to devclass_add_device() in make_device().
> 
> Your guess:
> > I wonder if this is related to the recent changes to set the unit number for CPUs?
> 
> seems to be true.
> 
> The device_t-s created for CPUs have NULL driver name / devclass, but a
> non-wildcard unit number.  So when such a device with unit number 0 is probed by
> acpi_timer we get a unit number conflict with acpi_timer0 pre-created via the
> identify.
> Similarly we get conflicts for acpi_sysresource driver, because we do an early
> probe-and-attach for this driver and the attached devices get some unit numbers
> (0, 1, etc).  So when during the normal probe pass the "CPU" devices with matching
> unit numbers are passed to the driver the conflict results.
> 
> I guess that it is an unorthodox use of newbus to specify a unit number without
> specifying a driver name...  It's like saying "this device must be unit N whatever
> driver claims it (be it kbdN or diskN) just because I say so".  Not sure if this
> ever makes sense and maybe we should prohibit such a combination (reject it earlier).
> I guess that in this particular case we already know that the devices are really
> CPU devices and are going to be claimed by acpi cpu driver.  So we should pass
> "cpu" as the name.

Oh, whoops.  Actually, the right way to do this I think is bus_hint_device_unit()
(and/or, not make the unit number in cpuX mean anything at all, but use a separate
ivar to track what PCPU_GET(cpuid) a given cpuX device corresponds to).  I think
the last approach is really the right way to fix this.

-- 
John Baldwin