Looks OK to me from the InfiniPath side. Keeping the backported code as close as possible to the upstream code is a good thing in my view.
On Tue, 2007-09-11 at 09:28 +0300, Michael S. Tsirkin wrote: > Roland, Ralph, all, > I'd like to get your opinion on the following matter: > OFED is backporting upstream rdma code to older kernels. > While doing so, I really take pains to keep the ported > code as close as possible to upstream original, > mostly by using preprocessor to implement, as closely > as possible, the APIs from recent kernels on top of > older ones. > > As an example where this works well, see my backport of the > new workqueue API to 2.6.19: > http://www.openfabrics.org/git/?p=ofed_1_3/linux-2.6.git;a=blob;f=kernel_addons/backport/2.6.19/include/linux/workqueue.h;hb=HEAD > > However, sometimes I am forced to patch the upstream code. Here's an > example of the patch needed to make ipath build on > 2.6.22: > > > diff --git a/drivers/infiniband/hw/ipath/ipath_driver.c > b/drivers/infiniband/hw/ipath/ipath_driver.c > index 09c5fd8..94edb5d 100644 > --- a/drivers/infiniband/hw/ipath/ipath_driver.c > +++ b/drivers/infiniband/hw/ipath/ipath_driver.c > @@ -287,6 +287,7 @@ static int __devinit ipath_init_one(struct pci_dev *pdev, > struct ipath_devdata *dd; > unsigned long long addr; > u32 bar0 = 0, bar1 = 0; > + u8 rev; > > dd = ipath_alloc_devdata(pdev); > if (IS_ERR(dd)) { > @@ -448,7 +449,13 @@ static int __devinit ipath_init_one(struct pci_dev *pdev, > dd->ipath_deviceid = ent->device; /* save for later use */ > dd->ipath_vendorid = ent->vendor; > > - dd->ipath_pcirev = pdev->revision; > + ret = pci_read_config_byte(pdev, PCI_REVISION_ID, &rev); > + if (ret) { > + ipath_dev_err(dd, "Failed to read PCI revision ID unit " > + "%u: err %d\n", dd->ipath_unit, -ret); > + goto bail_regions; /* shouldn't ever happen */ > + } > + dd->ipath_pcirev = rev; > > #if defined(__powerpc__) > /* There isn't a generic way to specify writethrough mappings */ > > > As you can see, there's nothing I can do with macros outside the code > to make it work without code changes. > However, the patching mechanism is pretty fragile with respect > to code reorgs etc. > I wonder whether it's acceptable in cases such as this to add > a wrapper in upstream code. For example, upstream could have: > > #ifndef pci_get_revision > #define pci_get_revision(dev) ((dev)->revision) > #endif > > and then all a 2.6.22 backport needs to do is define it's own > pci_get_revision macro. > > Upstream maintainers, can you pls comment ASAP on whether such approach would > be > acceptable e.g. for 2.6.24? If I could get rid of backport > patches, it might make sense to start thinking about converting fixes > patches to git commits, post 1.3, as well. > > Thanks, > _______________________________________________ general mailing list [email protected] http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
