Date: Mon, 19 May 1997 20:27:43 +0200 From: Stefan Esser <se@FreeBSD.ORG> To: Doug Rabson <dfr@nlsystems.com> Cc: Michael Smith <msmith@atrad.adelaide.edu.au>, current@FreeBSD.ORG Subject: Re: Backwards compatibiliy for isa_driver Message-ID: <19970519202743.46906@x14.mi.uni-koeln.de> In-Reply-To: <Pine.BSF.3.95q.970515090922.22139I-100000@herring.nlsystems.com>; from Doug Rabson on Thu, May 15, 1997 at 10:07:18AM %2B0100 References: <19970514224514.26045@x14.mi.uni-koeln.de> <Pine.BSF.3.95q.970515090922.22139I-100000@herring.nlsystems.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On May 15, Doug Rabson <dfr@nlsystems.com> wrote:
> How about this:
Seems we agree on all the stuff that I deleted ... :)
> #define ISA_RESOURCE_PORTS 0
> #define ISA_RESOURCE_MEMORY 1
> #define ISA_RESOURCE_IRQ 2
> #define ISA_RESOURCE_DRQ 3
>
> #define PCI_RESOURCE_MAP0 0
> #define PCI_RESOURCE_MAP1 1
> .....
>
>
> enum resource_type {
> RESOURCE_TYPE_PORT,
> RESOURCE_TYPE_MEMORY,
> RESOURCE_TYPE_IRQ,
> RESOURCE_TYPE_DRQ
> };
>
> struct resource {
> enum resource_type type;
> u_int start;
> u_int size;
> };
>
> struct bus_ops {
> ...
> struct resource *(*get_resource)(struct bus *bus, void *instance,
> int which);
> int *(*map_resource)(struct bus *bus, void *instance, int which);
> int *(*unmap_resource)(struct bus *bus, void *instance,
> int which);
> int *(*change_resource(struct bus *bus, void *instance, int which,
> struct resource *);
> }
>
> struct bus {
> ...
> struct bus_ops *ops;
> };
>
> int edprobe(struct bus *bus, void *instance)
> {
> struct resource *ports;
> struct resource *memory;
> struct resource *irq;
>
> ports = bus->ops->get_resource(bus, instance, ISA_RESOURCE_PORTS);
Hmmm, you are mixing a generic mechanism with a bus dependent
constant here ...
If you are using ISA_RESOURCE_PORTS, then you could as well
call just some global function get_resource() ...
> memory = bus->ops->get_resource(bus, instance,
> ISA_RESOURCE_MEMORY);
> irq = bus->ops->get_resource(bus, instance, ISA_RESOURCE_IRQ);
> ports->size = 0x10;
> bus->ops->change_resource(bus, instance, ISA_RESOURCE_PORTS);
>
> bus->ops->map_resource(bus, instance, ISA_RESOURCE_PORTS);
> bus->ops->map_resource(bus, instance, ISA_RESOURCE_MEMORY);
> bus->ops->map_resource(bus, instance, ISA_RESOURCE_IRQ);
>
> probe the device.
>
> if (probe failed) {
> bus->ops->unmap_resource(bus, instance,
> ISA_RESOURCE_PORTS);
> bus->ops->unmap_resource(bus, instance,
> ISA_RESOURCE_MEMORY);
> bus->ops->unmap_resource(bus, instance, ISA_RESOURCE_IRQ);
> }
> }
No, I don't like this, sorry..
We are forced to use some data representation for the resource
structure, that will be cast in stone, if we return pointers to
data structures as you suggest for get_resource().
You do not treat the "struct resource" as an opaque data type
(as obvious from the assignment to "ports->size"), and thus we
have an unlucky mix of complex data structures and lack of
flexibility ...
I'd rather have a function that retrieves config info (which is
not part of the resource management code) and then do conflicts
checks and finally try to probe/attach the device. After the
attach succeeded, the resources will be known because of the
device's "isa_device" structure (or rather some more generic
data structure as suggested in an earlier message) will be
known to the resource management code ...
I don't think this needs to be put into every driver, if we
find a good concept. And that's what I'm after :)
> Each bus and each bridge could provide its own ops vector. Each device
> attached to a bus would have a set of resources. The instance pointer
> above is a handle to the device resources.
Each device and device "location" should have an ops vector.
The location is not just the bus, it could be a PCI to PCI bridge
(which behaves slightly different from a CPU to PCI bridge, and
needs slightly different resource conflict checks for that reason).
> > But what I really want to create this way is a extensible persistent
> > store of config info, including local DEVFS permission and device
> > number choices.
>
> Sounds good. How far have you got, implementation wise?
I have written a sample implementation this afternoon. It compiles
to less than 1K of code, and I include it (GZIPPED and UUENCODED)
below.
This is not meant to be the final implementation, but should
provide all the required functionality. I guess there should be
one more function:
int cfg_getunits (char *what, char *name, int buffer[], int bufsize);
that returns a vector of unit numbers for the type/name tuple.
Beware: The cfg_getnames() and cfg_gettypes() functions return
pointers to IDs, which are delimited by white space. There is no
terminating NUL character. These pointers are acceptable as input
to the functions that take what/name arguments, since these
functions will know about the white space denoting a word end ...
> > For this to work, I need not only a function that retrieves the data
> > array for a given (type, selector, unit), but also a list of units
> > for a given type and selector, and a list of types (char *) for a
> > given selector, or a list of selectors for a given type. (E.g. list
> > all drivers that exist for type "isa" or "scbus", or list all types
> > that contain data for "ed0" which might be "isa" and "pci" ...)
> >
> > Think about:
> >
> > char:13:sd:0:0640:ROOT:OPERATOR:...
> > block:4:sd:0:0640:ROOT:OPERATOR:...
> >
> > (where sd identifies the driver, ROOT and OPERATOR are numeric uid and
> > gid values, 13 and 4 are the major numbers of /dev/rsdXX and /dev/sdXX
> > and the 0 after "sd" is the minor number, all values are actually stored
> > in decimal even if I wrote the permissions in octal above ...).
> >
>
> Shouldn't this be:
>
> sd:0:char:13:0:0640:ROOT:OPERATOR:...
> sd:0:block:4:0:...
>
> Translating to english:
>
> Data for sd, unit zero, type is character device
> (major=13,minor=0,...)
> Data for sd, unit zero, type is block device
> (major=4,minor=0,...)
No, it shouldn't :)
But I was also wrong with the excample I gave ...
It should be:
char:13:sd
sd:0:devfs:0640:ROOT:OPERATOR
sd:-1:devfs:0600:ROOT:OPERATOR
(This is my suggestion for a wild card unit number, but this would be
handled by a program asking for the config data in such a way, that
wherever this is appropriate, config data for unit -1 is retrieved,
if nothing could be found for the actual unit.)
> It seems to me that the type here is really 'char' or 'block', not 'sd'.
> This way, the knowledge of the data format (major, minor, mode etc) is
> encoded by a single piece of code which understands the 'char' and 'block'
> type.
>
> If the type is the driver name, then all drivers would need to understand
> this data format. In addition, wouldn't it prevent drivers from having
> their own private config data:
>
> psm:0:isa:0x60:0:12:...
> psm:0:char:21:0:0600:0:0
> psm:0:char:21:1:0600:0:0
> psm:0:psm:acceleration
Well, I didn't try to work out a complete tree of information, yet.
> I think I was drifting in the direction of the bus/instance/resourceid
> discussion above. The idea was that each resource is represented in the
> sysctl tree by a triple (type, start, size) and that the same resource
> triples could be used by both isa and pci devices. For probe-able busses,
> the resource triples could be generated by the bus probe, otherwise
> extracted from config data.
I have already implemented resource management functions like you
that, but I don't like the concept...
> > In fact, I'd loop over all isa_device structures, and just request
> > those values for each (i.e. with dev->id_drv->name instead of "ed"
> > and dev->unit instead of the 0 in the example above ...)
>
> That works for me. I think we should still have compiled in data for isa
> devices in ioconf.c for the case when the config file is missing or
> doesn't contain an item for the particular device instance required.
>
> if (values = config_query(...))) {
> dev->iobase = ...
> }
No, I don't think we want the compiled in defaults :)
Well, kind of: I'd have "config" create a textual representation similar
to that found in the config file. If a request for config data can't be
satisfied from the file's data, then the kernel's text array will be
scanned for it. Guess we should have three separate config "texts",
in fact:
1) compiled in
2) loaded into RAM by the boot loader
3) dynamically allocated by userconfig
and config data should be preferred from the higher number "texts".
Regards, STefan
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?19970519202743.46906>