Date: Sun, 16 Dec 2001 02:33:21 +0100 From: Thomas Moestl <tmoestl@gmx.net> To: Mike Smith <msmith@freebsd.org> Cc: arch@FreeBSD.org Subject: Re: Please review: changes to MI bus code for sparc64 Message-ID: <20011216023321.C458@crow.dom2ip.de> In-Reply-To: <200112140028.fBE0Sol04630@mass.dis.org>; from msmith@freebsd.org on Thu, Dec 13, 2001 at 04:28:50PM -0800 References: <tmoestl@gmx.net> <200112140028.fBE0Sol04630@mass.dis.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, 2001/12/13 at 16:28:50 -0800, Mike Smith wrote:
> > > The PCI_BROKEN_INTPIN/PCI_INTLINE_0_BAD seem to be the same thing;
> > > they should be protected by a single name (probably PCI_BROKEN_INTPIN)
> > > in the #ifdef in pci.c; it should be "all or nothing" on a single
> > > value. As it is, you must define one if you define the other, but
> > > not vice versa, and the effect seems to be linked, anyway, so you
> > > might as well use a single protection mechanism.
> >
> > It is not uncommon that i386 BIOSes to set the intline register to 0
> > when it should really be 0xff (to indicate an unrouted interrupt). So,
> > I figured that it might be useful to make this an extra option.
>
> No. Fix the i386-specific config space accessor to convert 0 to 255.
>
> If you haven't seen the theme here yet; here it is. The MD layers should
> correct for platform-specific aberrations in the PCI implementation where
> possible.
>
> Adding compile-time options to MI code which indirectly relate exclusively
> to MD PCI issues is just the Wrong Thing to Do.
OK, I've attached a new diff. Changes are:
- remove PCI_INTLINE_0_BAD
- move the code for PCI_BROKEN_INTPIN to MD code, and use a quirk
table to identify the devices that need this fixup (I don't really
like this change, as I think it is not totally unlikely that the
device in question may also be used for other architectures; it is
not really specific to sparc64)
- revert the changes associated to pci_pci.c, at the cost of
duplicating some of the device hierarchy assumptions in the apb
driver code
- add a diff I had previously forgotten: move the PCI_ENABLE_IO_MODES
from conf/options.i386 to conf/options
This should remove all the changes you did not like, except the
addition to the resource manager. I do not see a good alternative to
this last change, and I think I did not fully understand what you did
not like about it (taking into consideration the task I am using it
for); I'm of course open for further discussion.
I would still like to commit this new diff around the 21st, if there
are no further objections and I can make it.
- thomas
--- freebsd/sys/isa/isa_common.c Sat Sep 8 19:46:52 2001
+++ sparc64/sys/isa/isa_common.c Wed Oct 10 00:59:24 2001
@@ -92,7 +92,7 @@
isa_probe(device_t dev)
{
device_set_desc(dev, "ISA bus");
- isa_init(); /* Allow machdep code to initialise */
+ isa_init(dev); /* Allow machdep code to initialise */
return 0;
}
@@ -634,37 +634,6 @@
}
static int
-isa_print_resources(struct resource_list *rl, const char *name, int type,
- int count, const char *format)
-{
- struct resource_list_entry *rle;
- int printed;
- int i, retval = 0;;
-
- printed = 0;
- for (i = 0; i < count; i++) {
- rle = resource_list_find(rl, type, i);
- if (rle) {
- if (printed == 0)
- retval += printf(" %s ", name);
- else if (printed > 0)
- retval += printf(",");
- printed++;
- retval += printf(format, rle->start);
- if (rle->count > 1) {
- retval += printf("-");
- retval += printf(format,
- rle->start + rle->count - 1);
- }
- } else if (i > 3) {
- /* check the first few regardless */
- break;
- }
- }
- return retval;
-}
-
-static int
isa_print_all_resources(device_t dev)
{
struct isa_device *idev = DEVTOISA(dev);
@@ -674,14 +643,10 @@
if (SLIST_FIRST(rl) || device_get_flags(dev))
retval += printf(" at");
- retval += isa_print_resources(rl, "port", SYS_RES_IOPORT,
- ISA_NPORT, "%#lx");
- retval += isa_print_resources(rl, "iomem", SYS_RES_MEMORY,
- ISA_NMEM, "%#lx");
- retval += isa_print_resources(rl, "irq", SYS_RES_IRQ,
- ISA_NIRQ, "%ld");
- retval += isa_print_resources(rl, "drq", SYS_RES_DRQ,
- ISA_NDRQ, "%ld");
+ retval += resource_list_print_type(rl, "port", SYS_RES_IOPORT, "%#lx");
+ retval += resource_list_print_type(rl, "iomem", SYS_RES_MEMORY, "%#lx");
+ retval += resource_list_print_type(rl, "irq", SYS_RES_IRQ, "%ld");
+ retval += resource_list_print_type(rl, "drq", SYS_RES_DRQ, "%ld");
if (device_get_flags(dev))
retval += printf(" flags %#x", device_get_flags(dev));
--- freebsd/sys/isa/isa_common.h Sat Sep 8 19:46:52 2001
+++ sparc64/sys/isa/isa_common.h Wed Oct 10 00:59:27 2001
@@ -65,7 +65,7 @@
/*
* These functions are architecture dependant.
*/
-extern void isa_init(void);
+extern void isa_init(device_t dev);
extern struct resource *isa_alloc_resource(device_t bus, device_t child,
int type, int *rid,
u_long start, u_long end,
--- freebsd/sys/dev/pci/pci.c Sun Nov 4 01:39:41 2001
+++ sparc64/sys/dev/pci/pci.c Sat Dec 15 18:14:45 2001
@@ -78,9 +78,6 @@
device_t dev);
static void pci_add_children(device_t dev, int busno);
static int pci_probe(device_t dev);
-static int pci_print_resources(struct resource_list *rl,
- const char *name, int type,
- const char *format);
static int pci_print_child(device_t dev, device_t child);
static void pci_probe_nomatch(device_t dev, device_t child);
static int pci_describe_parse_line(char **ptr, int *vendor,
@@ -831,34 +828,6 @@
}
static int
-pci_print_resources(struct resource_list *rl, const char *name, int type,
- const char *format)
-{
- struct resource_list_entry *rle;
- int printed, retval;
-
- printed = 0;
- retval = 0;
- /* Yes, this is kinda cheating */
- SLIST_FOREACH(rle, rl, link) {
- if (rle->type == type) {
- if (printed == 0)
- retval += printf(" %s ", name);
- else if (printed > 0)
- retval += printf(",");
- printed++;
- retval += printf(format, rle->start);
- if (rle->count > 1) {
- retval += printf("-");
- retval += printf(format, rle->start +
- rle->count - 1);
- }
- }
- }
- return retval;
-}
-
-static int
pci_print_child(device_t dev, device_t child)
{
struct pci_devinfo *dinfo;
@@ -872,9 +841,9 @@
retval += bus_print_child_header(dev, child);
- retval += pci_print_resources(rl, "port", SYS_RES_IOPORT, "%#lx");
- retval += pci_print_resources(rl, "mem", SYS_RES_MEMORY, "%#lx");
- retval += pci_print_resources(rl, "irq", SYS_RES_IRQ, "%ld");
+ retval += resource_list_print_type(rl, "port", SYS_RES_IOPORT, "%#lx");
+ retval += resource_list_print_type(rl, "mem", SYS_RES_MEMORY, "%#lx");
+ retval += resource_list_print_type(rl, "irq", SYS_RES_IRQ, "%ld");
if (device_get_flags(dev))
retval += printf(" flags %#x", device_get_flags(dev));
--- freebsd/sys/dev/pci/pcivar.h Sun Aug 5 19:55:43 2001
+++ sparc64/sys/dev/pci/pcivar.h Wed Oct 10 00:59:22 2001
@@ -182,20 +182,8 @@
/*
* Simplified accessors for pci devices
*/
-#define PCI_ACCESSOR(A, B, T) \
- \
-static __inline T pci_get_ ## A(device_t dev) \
-{ \
- uintptr_t v; \
- BUS_READ_IVAR(device_get_parent(dev), dev, PCI_IVAR_ ## B, &v); \
- return (T) v; \
-} \
- \
-static __inline void pci_set_ ## A(device_t dev, T t) \
-{ \
- uintptr_t v = (uintptr_t) t; \
- BUS_WRITE_IVAR(device_get_parent(dev), dev, PCI_IVAR_ ## B, v); \
-}
+#define PCI_ACCESSOR(var, ivar, type) \
+ __BUS_ACCESSOR(pci, var, PCI, ivar, type)
PCI_ACCESSOR(subvendor, SUBVENDOR, u_int16_t)
PCI_ACCESSOR(subdevice, SUBDEVICE, u_int16_t)
--- freebsd/sys/sys/bus.h Sun Nov 4 01:40:09 2001
+++ sparc64/sys/sys/bus.h Sun Nov 4 01:14:54 2001
@@ -180,6 +180,14 @@
int type, int rid, struct resource *res);
/*
+ * Print all resources of a specified type, for use in bus_print_child.
+ * The name is printed if at least one resource of the given type is available.
+ * The format ist used to print resource start and end.
+ */
+int resource_list_print_type(struct resource_list *rl,
+ const char *name, int type,
+ const char *format);
+/*
* The root bus, to which all top-level busses are attached.
*/
extern device_t root_bus;
@@ -410,6 +418,26 @@
}; \
DECLARE_MODULE(name##_##busname, name##_##busname##_mod, \
SI_SUB_DRIVERS, SI_ORDER_MIDDLE)
+
+/*
+ * Generic ivar accessor generation macros for bus drivers
+ */
+#define __BUS_ACCESSOR(varp, var, ivarp, ivar, type) \
+ \
+static __inline type varp ## _get_ ## var(device_t dev) \
+{ \
+ uintptr_t v; \
+ BUS_READ_IVAR(device_get_parent(dev), dev, \
+ ivarp ## _IVAR_ ## ivar, &v); \
+ return ((type) v); \
+} \
+ \
+static __inline void varp ## _set_ ## var(device_t dev, type t) \
+{ \
+ uintptr_t v = (uintptr_t) t; \
+ BUS_WRITE_IVAR(device_get_parent(dev), dev, \
+ ivarp ## _IVAR_ ## ivar, v); \
+}
#endif /* _KERNEL */
--- freebsd/sys/sys/rman.h Sat Sep 8 19:53:09 2001
+++ sparc64/sys/sys/rman.h Thu Nov 22 23:54:27 2001
@@ -126,6 +126,9 @@
struct resource *rman_reserve_resource(struct rman *rm, u_long start,
u_long end, u_long count,
u_int flags, struct device *dev);
+struct resource *rman_reserve_resource_bound(struct rman *rm, u_long start,
+ u_long end, u_long count, u_long bound,
+ u_int flags, struct device *dev);
uint32_t rman_make_alignment_flags(uint32_t size);
#define rman_get_start(r) ((r)->r_start)
--- freebsd/sys/kern/subr_bus.c Mon Dec 10 20:44:39 2001
+++ sparc64/sys/kern/subr_bus.c Thu Dec 13 04:06:25 2001
@@ -1207,8 +1207,8 @@
if (isdefault) {
start = rle->start;
- count = max(count, rle->count);
- end = max(rle->end, start + count - 1);
+ count = ulmax(count, rle->count);
+ end = ulmax(rle->end, start + count - 1);
}
rle->res = BUS_ALLOC_RESOURCE(device_get_parent(bus), child,
@@ -1253,6 +1253,34 @@
rle->res = NULL;
return (0);
+}
+
+int
+resource_list_print_type(struct resource_list *rl, const char *name, int type,
+ const char *format)
+{
+ struct resource_list_entry *rle;
+ int printed, retval;
+
+ printed = 0;
+ retval = 0;
+ /* Yes, this is kinda cheating */
+ SLIST_FOREACH(rle, rl, link) {
+ if (rle->type == type) {
+ if (printed == 0)
+ retval += printf(" %s ", name);
+ else
+ retval += printf(",");
+ printed++;
+ retval += printf(format, rle->start);
+ if (rle->count > 1) {
+ retval += printf("-");
+ retval += printf(format, rle->start +
+ rle->count - 1);
+ }
+ }
+ }
+ return retval;
}
/*
--- freebsd/sys/kern/subr_rman.c Sun Nov 25 14:51:37 2001
+++ sparc64/sys/kern/subr_rman.c Thu Nov 22 23:54:25 2001
@@ -175,12 +175,13 @@
}
struct resource *
-rman_reserve_resource(struct rman *rm, u_long start, u_long end, u_long count,
- u_int flags, struct device *dev)
+rman_reserve_resource_bound(struct rman *rm, u_long start, u_long end,
+ u_long count, u_long bound, u_int flags,
+ struct device *dev)
{
u_int want_activate;
struct resource *r, *s, *rv;
- u_long rstart, rend;
+ u_long rstart, rend, amask, bmask;
rv = 0;
@@ -202,6 +203,9 @@
goto out;
}
+ amask = (1ul << RF_ALIGNMENT(flags)) - 1;
+ /* If bound is 0, bmask will also be 0 */
+ bmask = ~(bound - 1);
/*
* First try to find an acceptable totally-unshared region.
*/
@@ -215,10 +219,19 @@
DPRINTF(("region is allocated\n"));
continue;
}
- rstart = max(s->r_start, start);
- rstart = (rstart + ((1ul << RF_ALIGNMENT(flags))) - 1) &
- ~((1ul << RF_ALIGNMENT(flags)) - 1);
- rend = min(s->r_end, max(rstart + count, end));
+ rstart = ulmax(s->r_start, start);
+ /*
+ * Try to find a region by adjusting to boundary and alignment
+ * until both conditions are satisfied. This is not an optimal
+ * algorithm, but in most cases it isn't really bad, either.
+ */
+ do {
+ rstart = (rstart + amask) & ~amask;
+ if (((rstart ^ (rstart + count)) & bmask) != 0)
+ rstart += bound - (rstart & ~bmask);
+ } while ((rstart & amask) != 0 && rstart < end &&
+ rstart < s->r_end);
+ rend = ulmin(s->r_end, ulmax(rstart + count, end));
DPRINTF(("truncated region: [%#lx, %#lx]; size %#lx (requested %#lx)\n",
rstart, rend, (rend - rstart + 1), count));
@@ -313,10 +326,12 @@
break;
if ((s->r_flags & flags) != flags)
continue;
- rstart = max(s->r_start, start);
- rend = min(s->r_end, max(start + count, end));
+ rstart = ulmax(s->r_start, start);
+ rend = ulmin(s->r_end, ulmax(start + count, end));
if (s->r_start >= start && s->r_end <= end
- && (s->r_end - s->r_start + 1) == count) {
+ && (s->r_end - s->r_start + 1) == count &&
+ (s->r_start & amask) == 0 &&
+ ((s->r_start ^ s->r_end) & bmask) == 0) {
rv = malloc(sizeof *rv, M_RMAN, M_NOWAIT | M_ZERO);
if (rv == 0)
goto out;
@@ -368,6 +383,15 @@
return (rv);
}
+struct resource *
+rman_reserve_resource(struct rman *rm, u_long start, u_long end, u_long count,
+ u_int flags, struct device *dev)
+{
+
+ return (rman_reserve_resource_bound(rm, start, end, count, 0, flags,
+ dev));
+}
+
static int
int_rman_activate_resource(struct rman *rm, struct resource *r,
struct resource **whohas)
@@ -575,7 +599,7 @@
* Find the hightest bit set, and add one if more than one bit
* set. We're effectively computing the ceil(log2(size)) here.
*/
- for (i = 32; i > 0; i--)
+ for (i = 31; i > 0; i--)
if ((1 << i) & size)
break;
if (~(1 << i) & size)
--- freebsd/sys/i386/isa/isa.c Fri Oct 12 16:18:20 2001
+++ sparc64/sys/i386/isa/isa.c Thu Dec 13 17:53:29 2001
@@ -68,7 +68,7 @@
#include <isa/isa_common.h>
void
-isa_init(void)
+isa_init(device_t dev)
{
}
--- freebsd/sys/ia64/isa/isa.c Sun Aug 5 20:05:14 2001
+++ sparc64/sys/ia64/isa/isa.c Thu Dec 13 17:53:29 2001
@@ -69,7 +69,7 @@
#include <isa/isa_common.h>
void
-isa_init(void)
+isa_init(device_t dev)
{
}
--- freebsd/sys/alpha/isa/isa.c Sun Aug 5 19:43:26 2001
+++ sparc64/sys/alpha/isa/isa.c Thu Dec 13 17:53:29 2001
@@ -94,7 +94,7 @@
}
void
-isa_init(void)
+isa_init(device_t dev)
{
isa_init_intr();
}
--- freebsd/sys/conf/options.i386 Fri Dec 14 23:26:25 2001
+++ sparc64/sys/conf/options.i386 Sat Dec 15 18:14:45 2001
@@ -107,8 +107,6 @@
PSM_RESETAFTERSUSPEND opt_psm.h
PSM_DEBUG opt_psm.h
-PCI_ENABLE_IO_MODES opt_pci.h
-
PCIC_RESUME_RESET opt_pcic.h
ATKBD_DFLT_KEYMAP opt_atkbd.h
--- freebsd/sys/conf/options Sun Nov 25 14:51:22 2001
+++ sparc64/sys/conf/options Sat Dec 15 18:14:45 2001
@@ -416,6 +419,7 @@
# PCI related options
PCI_QUIET opt_pci.h
+PCI_ENABLE_IO_MODES opt_pci.h
# NFS options
NFS_MINATTRTIMO opt_nfs.h
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-arch" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20011216023321.C458>