Re: request for review: bus_alloc_resource(9)
In message [EMAIL PROTECTED] Sergey Babkin writes: : Warner Losh wrote: : : In message [EMAIL PROTECTED] Sergey Babkin writes: : : The code seems to guarantee that if the probe routine returns 0 : : then the attach routine will be called right away. So if the probe : : routine returns 0 they don't have to be freed. Actually, the : : comments seem to say explicitly that the resources should be : : freed only if the probe routine returns a negative value but not 0. : : Or am I missing something ? : : The code doesn't write the guarantee. In general probe routines are : supposed to be idempotent. You are supposed to be able to have them : be called multiple times, at least in theory. The probe routine : should not hold resources past the end of its execution, positive or : negative. : : I'm not sure where you found the comments that say that the probe : routine can hold resources after it is called. I couldn't find any. : : Sorry that I did not quote it in the first e-mail. In kern/device_if.m: A pointer would have been sufficient :-) : # Probe to see if the device is present. Return 0 if the device exists, : # ENXIO if it cannot be found. If some other error happens during the : # probe (such as a memory allocation failure), an appropriate error code : # should be returned. For cases where more than one driver matches a : # device, a priority value can be returned. In this case, success codes : # are values less than or equal to zero with the highest value representing : # the best match. Failure codes are represented by positive values and : # the regular unix error codes should be used for the purpose. : : # If a driver returns a success code which is less than zero, it must : # not assume that it will be the same driver which is attached to the : # device. In particular, it must not assume that any values stored in : # the softc structure will be available for its attach method and any : # resources allocated during probe must be released and re-allocated : # if the attach method is called. If a success code of zero is : # returned, the driver can assume that it will be the one attached. Notice it doesn't say that one can hold RESOURCES past the end of probe. Only that values set in the softc (which one should try to avoid) aren't guaranteed to be there if the return value is 0. It can only assume that it will be the driver that is attached. This comment is poorly worded anyway and should be changed to explicitly state that one shouldn't hold resources at the end of the probe routine. : That comment does not guarantee that attach will be called right : away but I suppose it guarantees that if probe() returns 0 it : may expect that attach() will eventually be called and keep the : resources and contents of its structs softc between probe() and : attach(). Yes and no. It has long been stated that one cannot hold resources past the end of a probe. It is attaches job to allocate them permanantly, code and docs notwithstanding. I recall this coming up from time to time on the newbus list a long time ago, but haven't checked the archives. Going back in time to prenewbus, this certainly was the case. And it is the case in other Unixes that I've worked on in the past. : Yes, that's what I said in the first quoted paraghaph: if probe : returns 0 (priority arbitration between multiple drivers in a class) : or 0 (error) then it must free the resources before exit and don't : assume that the contents of struct softc will be presevred between : probe and attach. But if probe returns 0 it may keep the resources. I'm still not sure that I agree. Unless I hear from the new-bus meister, I won't feel obligated to necessarily keep this condition in the NEWCARD code. I don't think I'll need to violate it, but I will if I have to because of how things are allocated in most device drivers right now. : Also some resources have problems with freeing them: for example, : if a piece of memory was allocated using bus_dmamem_alloc() with : lowaddr of BUS_SPACE_MAXADDR_24BIT then bus_dmamem_free() won't : really free it but just throw it away (I suppose contigfree() did not : exist when it was written). So it's better to keep these resources : between probe and attach if probe returns 0. No. Absolutely not. One should allocate them once only in the attach code. The probe code almost never needs to do a DMA to probe the device. Probes are supposed to be non-destructive (eg read only on the registers) if at all possible. One shouldn't try to allocate it there anyway for the same reason you say we should. If you are using it to see if the device exists, and it doesn't, then you can't free it. Sounds like a bad way to probe the hardware. The probe routine is supposed to do the absolute minimum to the hardware to see if it is really there. It shouldn't be bringing in all kinds of extra resources that it doesn't need to determine if the device is there or not. Probe's role
Re: request for review: bus_alloc_resource(9)
In message [EMAIL PROTECTED] Sergey Babkin writes: : Warner Losh wrote: : : In message [EMAIL PROTECTED] Sergey Babkin writes: : : : # If a driver returns a success code which is less than zero, it must : : # not assume that it will be the same driver which is attached to the : : # device. In particular, it must not assume that any values stored in : : # the softc structure will be available for its attach method and any : : # resources allocated during probe must be released and re-allocated : : : # if the attach method is called. If a success code of zero is : : # returned, the driver can assume that it will be the one attached. : : Notice it doesn't say that one can hold RESOURCES past the end of : probe. Only that values set in the softc (which one should try to : : It says, about the case of negative return value. So I guess that : implies that in case if 0 is returned keeping resources is permitted. I guess I'm on the other side of the fence. It doesn't specifically allow it, so it must be forbidden. I've seen too much hardware docs that say one thing for condition A and one might assume that for A' that wouldn't be the case, but in fact the same condition applies for A' as well. : avoid) aren't guaranteed to be there if the return value is 0. It : can only assume that it will be the driver that is attached. This : comment is poorly worded anyway and should be changed to explicitly : state that one shouldn't hold resources at the end of the probe : routine. : : I suppose that "driver can assume that it will be the one attached" : means that the probe routine will not be called again for this : device, thet the resources won't change and that the attach routine : will be called eventually. Provided with all this keeping the resources : seems to be safe. But of course that's only my interpretation. Yes. I don't think it is safe, and I don't that we should document it as safe. : As far as I remember in prenewbus there were no "official resources". : Yes, I agree that keeping anything allocated when returning : from probe is a bad idea. In prenewbus, one couldn't malloc memory in probe that one didn't return right away. : : Yes, that's what I said in the first quoted paraghaph: if probe : : returns 0 (priority arbitration between multiple drivers in a class) : : or 0 (error) then it must free the resources before exit and don't : : assume that the contents of struct softc will be presevred between : : probe and attach. But if probe returns 0 it may keep the resources. : : I'm still not sure that I agree. Unless I hear from the new-bus : meister, I won't feel obligated to necessarily keep this condition in : the NEWCARD code. I don't think I'll need to violate it, but I will : if I have to because of how things are allocated in most device : drivers right now. : : It's not like I'm saying that it MUST keep the resources from probe but : I suppose it's allowed to if it really wants. There might be some cases where it makes sense to do it, but those cases are going to be few and far between, imho. : : Also some resources have problems with freeing them: for example, : : if a piece of memory was allocated using bus_dmamem_alloc() with : : lowaddr of BUS_SPACE_MAXADDR_24BIT then bus_dmamem_free() won't : : really free it but just throw it away (I suppose contigfree() did not : : exist when it was written). So it's better to keep these resources : : between probe and attach if probe returns 0. : : No. Absolutely not. One should allocate them once only in the attach : code. The probe code almost never needs to do a DMA to probe the : device. Probes are supposed to be non-destructive (eg read only on : the registers) if at all possible. One shouldn't try to allocate it : there anyway for the same reason you say we should. If you are using : it to see if the device exists, and it doesn't, then you can't free : it. Sounds like a bad way to probe the hardware. The probe routine : : As far as I remember in prenewbus there were no "official resources". : Yes, I agree that keeping anything allocated when returning : from probe is a bad idea. In prenewbus, one couldn't malloc memory in probe that one didn't return right away. : : Yes, that's what I said in the first quoted paraghaph: if probe : : returns 0 (priority arbitration between multiple drivers in a class) : : or 0 (error) then it must free the resources before exit and don't : : assume that the contents of struct softc will be presevred between : : probe and attach. But if probe returns 0 it may keep the resources. : : I'm still not sure that I agree. Unless I hear from the new-bus : meister, I won't feel obligated to necessarily keep this condition in : the NEWCARD code. I don't think I'll need to violate it, but I will : if I have to because of how things are allocated in most device : drivers right now. : : It's not like I'm saying
Re: request for review: bus_alloc_resource(9)
Thus spake Matthew N. Dodd ([EMAIL PROTECTED]): is just a range; start and length, and a type. The 'rid' has nothing to do with offsets into a memory/port resource. Hmm. When I wrote Doug Rabson about newbus months ago, he gave me that part of code: rid = 0x10; /* offset of pci mapping register - check your docs */ res = bus_alloc_resource(dev, SYS_RES_IOPORTS, rid, 0, ~0, 1, RF_ACTIVE); st = rman_get_bustag(res); sh = rman_get_bushandle(res); This "offset of the pci mapping register" is quite confusing for me then. Alex -- I need a new ~/.sig. To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-hackers" in the body of the message
Re: request for review: bus_alloc_resource(9)
Thus spake Matthew N. Dodd ([EMAIL PROTECTED]): Think of an 'rid' as in index into an array of like resources. A resource is just a range; start and length, and a type. The 'rid' has nothing to do with offsets into a memory/port resource. Ah, yes. That is probably why resource_list_alloc(rl, dev, child, type, rid, ... is called for the pci-version of bus_alloc_resource :-) (sys/pci/pci.c) Alex -- I need a new ~/.sig. To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-hackers" in the body of the message
Re: request for review: bus_alloc_resource(9)
Thus spake Matthew N. Dodd ([EMAIL PROTECTED]): is just a range; start and length, and a type. The 'rid' has nothing to do with offsets into a memory/port resource. Hmm. When I wrote Doug Rabson about newbus months ago, he gave me that part of code: rid = 0x10; /* offset of pci mapping register - check your docs */ res = bus_alloc_resource(dev, SYS_RES_IOPORTS, rid, 0, ~0, 1, RF_ACTIVE); st = rman_get_bustag(res); sh = rman_get_bushandle(res); This "offset of the pci mapping register" is quite confusing for me then. Not at all; in the PCI context, that's what the rid is. As has been said several times now, the meaning of the rid is _bus_specific_. -- \\ Give a man a fish, and you feed him for a day. \\ Mike Smith \\ Tell him he should learn how to fish himself, \\ [EMAIL PROTECTED] \\ and he'll hate you for a lifetime. \\ [EMAIL PROTECTED] To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-hackers" in the body of the message
Re: request for review: bus_alloc_resource(9)
Thus spake Mike Smith ([EMAIL PROTECTED]): Not at all; in the PCI context, that's what the rid is. As has been said several times now, the meaning of the rid is _bus_specific_. Ah, I wrote that before reading the next mails, stupid me. Well, thanks for all your comments. I'll add/merge the information you gave me in the evening. Thanks! Alex -- I need a new ~/.sig. To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-hackers" in the body of the message
Re: request for review: bus_alloc_resource(9)
Second, reworked version now available. http://big.endian.de/FreeBSD/bus_alloc_resource.9 In my eyes, it's quite correct now and is worth a PR/commit. I'll send a PR if I get your ok. Alex -- I need a new ~/.sig. To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-hackers" in the body of the message
Re: request for review: bus_alloc_resource(9)
Warner Losh wrote:
In message [EMAIL PROTECTED] Sergey Babkin writes:
: The code seems to guarantee that if the probe routine returns 0
: then the attach routine will be called right away. So if the probe
: routine returns 0 they don't have to be freed. Actually, the
: comments seem to say explicitly that the resources should be
: freed only if the probe routine returns a negative value but not 0.
: Or am I missing something ?
The code doesn't write the guarantee. In general probe routines are
supposed to be idempotent. You are supposed to be able to have them
be called multiple times, at least in theory. The probe routine
should not hold resources past the end of its execution, positive or
negative.
I'm not sure where you found the comments that say that the probe
routine can hold resources after it is called. I couldn't find any.
Sorry that I did not quote it in the first e-mail. In kern/device_if.m:
#
# Probe to see if the device is present. Return 0 if the device exists,
# ENXIO if it cannot be found. If some other error happens during the
# probe (such as a memory allocation failure), an appropriate error code
# should be returned. For cases where more than one driver matches a
# device, a priority value can be returned. In this case, success codes
# are values less than or equal to zero with the highest value representing
# the best match. Failure codes are represented by positive values and
# the regular unix error codes should be used for the purpose.
# If a driver returns a success code which is less than zero, it must
# not assume that it will be the same driver which is attached to the
# device. In particular, it must not assume that any values stored in
# the softc structure will be available for its attach method and any
# resources allocated during probe must be released and re-allocated
# if the attach method is called. If a success code of zero is
# returned, the driver can assume that it will be the one attached.
#
# Devices which implement busses should use this method to probe for
# the existence of devices attached to the bus and add them as
# children. If this is combined with the use of bus_generic_attach,
# the child devices will be automatically probed and attached.
#
METHOD int probe {
device_t dev;
};
It is also legal for buses to probe all their devices before attaching
any of them (the pci bus does this, iirc, so that generic drivers can
handle some hardware and more specific drivers can handle other).
There's nothing that states probe_and_attach is the only way to get
things done.
That comment does not guarantee that attach will be called right
away but I suppose it guarantees that if probe() returns 0 it
may expect that attach() will eventually be called and keep the
resources and contents of its structs softc between probe() and
attach().
Finally, there's a comment in subr_bus:
device_set_driver(child, best-driver);
if (pri 0) {
/*
* A bit bogus. Call the probe method again to make sure
* that we have the right description.
*/
DEVICE_PROBE(child);
}
which indicates to me that the probe routines will be called multiple
times if they return 0 (pri is the priority they returned, 0 meaning
it is mine and nobody else's).
Yes, that's what I said in the first quoted paraghaph: if probe
returns 0 (priority arbitration between multiple drivers in a class)
or 0 (error) then it must free the resources before exit and don't
assume that the contents of struct softc will be presevred between
probe and attach. But if probe returns 0 it may keep the resources.
Also some resources have problems with freeing them: for example,
if a piece of memory was allocated using bus_dmamem_alloc() with
lowaddr of BUS_SPACE_MAXADDR_24BIT then bus_dmamem_free() won't
really free it but just throw it away (I suppose contigfree() did not
exist when it was written). So it's better to keep these resources
between probe and attach if probe returns 0.
-SB
To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-hackers" in the body of the message
request for review: bus_alloc_resource(9)
Hello! I've written bus_alloc_resource(9). I need one with _experience_ on newbus to review it. http://big.endian.de/FreeBSD/bus_alloc_resource.9 Thanks! Alex -- I need a new ~/.sig. To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-hackers" in the body of the message
Re: request for review: bus_alloc_resource(9)
On Thu, 18 May 2000, Alexander Langer wrote: I've written bus_alloc_resource(9). I need one with _experience_ on newbus to review it. http://big.endian.de/FreeBSD/bus_alloc_resource.9 You still don't understand what the 'rid' parameter is. Think of an 'rid' as in index into an array of like resources. A resource is just a range; start and length, and a type. The 'rid' has nothing to do with offsets into a memory/port resource. -- | Matthew N. Dodd | '78 Datsun 280Z | '75 Volvo 164E | FreeBSD/NetBSD | | [EMAIL PROTECTED] | 2 x '84 Volvo 245DL| ix86,sparc,pmax | | http://www.jurai.net/~winter | This Space For Rent | ISO8802.5 4ever | To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-hackers" in the body of the message
Re: request for review: bus_alloc_resource(9)
On Thu, 18 May 2000, Alexander Langer wrote: I've written bus_alloc_resource(9). I need one with _experience_ on newbus to review it. http://big.endian.de/FreeBSD/bus_alloc_resource.9 You still don't understand what the 'rid' parameter is. Think of an 'rid' as in index into an array of like resources. A resource is just a range; start and length, and a type. The 'rid' has nothing to do with offsets into a memory/port resource. More to the point, the rid is a bus-specific uniqifier - it's not necessarily even a linear index (consider eg. PCI). -- \\ Give a man a fish, and you feed him for a day. \\ Mike Smith \\ Tell him he should learn how to fish himself, \\ [EMAIL PROTECTED] \\ and he'll hate you for a lifetime. \\ [EMAIL PROTECTED] To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-hackers" in the body of the message
Re: request for review: bus_alloc_resource(9)
Some problems. Here's my comments. I'm posting them in case my understanding is inaccurate. If it is, I don't want you to codify it. And I want to know so I don't keep misunderstanding :-) First, rid. "rid" is a bus specific handle that identifies the resource being allocated. For ISA this is an index into an array of resources that have been setup for this device by either the PnP mechanism, or via the hints mechanism. For PCCARD, similar things are used, but that may change in the future with newcard. For PCI it just happens to be the offset into pci config space which has a word that describes the resource. Who knows what it will be for other things. Second, RF_SHAREABLE should generally never be used. There's one exception. That's for interrupts that can be shared in the hardware. PCI bus is the only bus that does this reliably. Since the PCCARD bus can be bridged to the ISA bus or to PCI bus, you can't reliably share interrupts there. ISA bus interrupts cannot be shared (well, unless you have custom hardware or hack stock hardware, but if you are doing that, then you know what you are doing). Memory resources, ioport resources, etc are not usually shared between devices. There are lots of bus issues with doing that when you have a driver for multiple hungs of hardware. There are exceptions ot that, but not enough to be worth messing with. The dev parameter is the device requesting ownership, and optional activation of this resource. The parent, or its parent, owns the resource until it is delegated to this device. [[ as an aside, this is generally only done in the nexus, but may be done in pccard in the future ]]. When default values are used for start and end, then the count parameter may be ignored and the amount of resource specified by the bus, or the hints mechanism, is used instead of the vaule passed in. This is why you often see '1'. RF_ACTIVE doesn't need to be set. Once can reserve the resource before activating it. RF_ACTIVATE doesn't mean that the device has been activated, but rather that the bus_activate_resource() method should be called to cause the resource to become active. This isn't important on the ISA bus since resources are always active (even in the PnP model). For other buses, this may cause the bus bridges to make the resources actually available. The bus methods are free to change the RIDs that are passed to it. That's why it is a pointer. Many devices in the tree blindly assume that htis isn't the case and do not store the rids for later freeing. Bus interfaces may change in the future which might cause changes to happen where they don't happen today. Think of rids a a well defined cookie. One cookie may be a meta-cookie that causes the real cookie to be returned. Generally, one should mirror one's allocate and release calls. A technique that works well is to set the rid/res in a softc structure and then have the detach function automatically free these resources. This allows one to call, manually, the detach function when the attach fails. One should generally only call these functions in attach. If you must call them in probe, one must release the resource before returning from the probe. However, since they can affect bridge settings, it may be unavoidable to call them from the probe routine. There's no published interface that allows a probe/identify routine to ask if a range of addresses are in use. This generally only bites those busses which aren't self identifying. Currently the ISA bus is the only bus we support that isn't self identifying. PnP helps, but when you are trying to deal with legacy hardware that doesn't support PnP it can get gross. Warner To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-hackers" in the body of the message
Re: request for review: bus_alloc_resource(9)
Second, RF_SHAREABLE should generally never be used. There's one exception. That's for interrupts that can be shared in the hardware. Actually, this is almost exactly the wrong way up; RF_SHAREABLE should always be used unless you know that the interrupt _can't_ be shared. In many cases, the driver can't tell whether it's allocating an interrupt which is or isn't shareable, and the only code that _can_ make this determination is in the parent layers. ie. as this request propagates upwards, the parent bus(ses) code should remove the flag in cases where it's know the interrupt can't be shared. PCI bus is the only bus that does this reliably. Since the PCCARD bus can be bridged to the ISA bus or to PCI bus, you can't reliably share interrupts there. ... and this is why. The pccard/cardbus bridge code, or the parent bus code, needs to decide whether the interrupt is or is not shareable - not the driver. -- \\ Give a man a fish, and you feed him for a day. \\ Mike Smith \\ Tell him he should learn how to fish himself, \\ [EMAIL PROTECTED] \\ and he'll hate you for a lifetime. \\ [EMAIL PROTECTED] To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-hackers" in the body of the message
Re: request for review: bus_alloc_resource(9)
In message [EMAIL PROTECTED] Mike Smith writes: : ... and this is why. The pccard/cardbus bridge code, or the parent bus : code, needs to decide whether the interrupt is or is not shareable - not : the driver. Ah. That's a good point. The ISA and PCCARD bridges should turn off interrupt sharing and the PCI and Cardbus bridges should pass it through. One other time when you can't share interrupts: Fast interrupts aren't shareable. Warner To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-hackers" in the body of the message
Re: request for review: bus_alloc_resource(9)
Warner Losh wrote: One should generally only call these functions in attach. If you must call them in probe, one must release the resource before returning from the probe. However, since they can affect bridge settings, it may be unavoidable to call them from the probe routine. The code seems to guarantee that if the probe routine returns 0 then the attach routine will be called right away. So if the probe routine returns 0 they don't have to be freed. Actually, the comments seem to say explicitly that the resources should be freed only if the probe routine returns a negative value but not 0. Or am I missing something ? -SB To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-hackers" in the body of the message
Re: request for review: bus_alloc_resource(9)
In message [EMAIL PROTECTED] Sergey Babkin writes:
: The code seems to guarantee that if the probe routine returns 0
: then the attach routine will be called right away. So if the probe
: routine returns 0 they don't have to be freed. Actually, the
: comments seem to say explicitly that the resources should be
: freed only if the probe routine returns a negative value but not 0.
: Or am I missing something ?
The code doesn't write the guarantee. In general probe routines are
supposed to be idempotent. You are supposed to be able to have them
be called multiple times, at least in theory. The probe routine
should not hold resources past the end of its execution, positive or
negative.
I'm not sure where you found the comments that say that the probe
routine can hold resources after it is called. I couldn't find any.
It is also legal for buses to probe all their devices before attaching
any of them (the pci bus does this, iirc, so that generic drivers can
handle some hardware and more specific drivers can handle other).
There's nothing that states probe_and_attach is the only way to get
things done.
Finally, there's a comment in subr_bus:
device_set_driver(child, best-driver);
if (pri 0) {
/*
* A bit bogus. Call the probe method again to make sure
* that we have the right description.
*/
DEVICE_PROBE(child);
}
which indicates to me that the probe routines will be called multiple
times if they return 0 (pri is the priority they returned, 0 meaning
it is mine and nobody else's).
Warner
To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-hackers" in the body of the message
