Date: Thu, 12 May 2011 21:40:55 -0400 From: John Baldwin <jhb@FreeBSD.org> To: Henrik Brix Andersen <brix@FreeBSD.org> Cc: freebsd-drivers@freebsd.org Subject: Re: Allocating resources to isab children Message-ID: <4DCC8C27.2010605@FreeBSD.org> In-Reply-To: <87F8ACF6-822B-41E3-B832-890F7F68ABA2@FreeBSD.org> References: <3550EA55-ADDE-40AC-9C22-1FAC441A0BC8@freebsd.org> <0A707516-C7D1-4441-B17B-1273B6C256B0@FreeBSD.org> <A5AEBC94-8E50-49E0-A72E-9B9E8A73E7F6@FreeBSD.org> <201103141028.57324.jhb@freebsd.org> <87F8ACF6-822B-41E3-B832-890F7F68ABA2@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 5/12/11 5:45 PM, Henrik Brix Andersen wrote: > Hi John, > > On Mar 14, 2011, at 15:28, John Baldwin wrote: >> On Sunday, March 13, 2011 12:12:59 pm Henrik Brix Andersen wrote: >>> On Mar 12, 2011, at 20:47, Henrik Brix Andersen wrote: >>> I have a patch ready for allowing the PCI-ISA bridge to proxy SYS_RES_MEMORY >> and SYS_RES_IOPORT resource allocations from direct children to the parent PCI >> bus. >>> The patch is available from here: http://people.freebsd.org/~brix/src- >> patches/isa_pci.c.diff >> >> Yes, I think this looks good. > > Having completed the driver for the I2C host bus controller, I have revised the patch a little. > The updated patch is available at http://people.freebsd.org/~brix/src-patches/isa_pci.c.diff A few cosmetic nits: - I would use 'isab_pci_*' rather than 'isa_pci_*' as it is an ISA bridge rather than an ISA bus device. - You can probably leave out the blank line in isa[b]_pci_attach() between bus_generic_probe() and isab_attach(). Other than that, you are fine to commit this with a reviewed by from me. > I would appreciate it if you could find the time to review this and - if possible - approve a commit of this patch. > > The patch for the glxiic(4) I2C host bus driver is available at http://people.freebsd.org/~brix/src-patches/glxiic.diff - if somebody could find the time to review this as well, I would be very grateful. Since I do not have a commit bit for src, I will eventually need an approval for this to enter the tree as well. This mostly looks good to me. I can't evaluate the actual driver logic, but the new-bus, etc. bits look fine. One thing is that since you use callout_init_mtx(), I don't think you need any of the callout stuff at the start of your timeout routine (callout_pending/active/deactivate). With callout_init_mtx() your callout will only be called if it is active. That is the only nit that I saw however. -- John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4DCC8C27.2010605>