Re: [RFC] ahci(4) patch

On 17.11.2011 00:44, Maksim Yevmenkin wrote:
> On Wed, Nov 16, 2011 at 2:35 PM, Alexander Motin<mav_at_freebsd.org>  wrote:
>> On 16.11.2011 23:59, Maksim Yevmenkin wrote:
>>>
>>> would anyone object to the following ahci(4) patch?
>>>
>>> ==
>>>
>>> --- ahci.c.orig 2011-11-16 21:35:26.000000000 +0000
>>> +++ ahci.c      2011-11-16 21:35:41.000000000 +0000
>>> _at__at_ -500,7 +500,7 _at__at_
>>>         for (unit = 0; unit<    ctlr->channels; unit++) {
>>>                 if ((ctlr->ichannels&    (1<<    unit)) == 0)
>>>                         continue;
>>> -               child = device_add_child(dev, "ahcich", -1);
>>> +               child = device_add_child(dev, "ahcich", unit);
>>>                 if (child == NULL)
>>>                         device_printf(dev, "failed to add channel
>>> device\n");
>>>                 else
>>>
>>> ==
>>>
>>> the idea is to have "static" numbering for ada(4) disks.
>>
>> I do. The only way I see this useful is if you have BIOS configured for
>> non-hot-swappable disks, in which case you have some AHCI channels disabled,
>> but want to keep numbers of the rest. While I don't like this mode in
>> general, especially when it can't be disabled, that patch could be useful in
>> these cases. But in other cases, when you have several AHCI controllers, it
>> just wont not work. You will receive error on attempt to create second
>> ahcich0.
>
> shouldn't achcichX be destroyed when disk is detached/removed/etc.?

List of implemented AHCI channels to expose as ahcichX set by BIOS in 
vendor-specific way, but only during boot and not by many BIOSes. 
Destroying them on disk detach theoretically possible, but IMHO not 
right, as bus doesn't disappear on disk disconnect.

> the particular problem i'm trying to address is disk re-numbering when
> one of the disks fails/removed/etc. i'm trying to use hints to wire
> disks to controllers/busses. it works perfectly fine with da(4) disks
> (even hot swappable ones) , but i can not make it to work with ada(4)
> disks. i'm perfectly fine to hid this under some sort of option
> (something similar to ATA_STATIC_ID, AHCI_STATIC_ID for example)

Wiring works for adaX also, unless your BIOS is so "intelligent" to 
report unconnected ports and not impliemented.. You should just wire CAM 
buses to ahcichX, not to ahciX. It could be done in other way changing 
just by one line around xpt_bus_register(), but now it is done so.

-- 
Alexander Motin