Re: [RFC][PATCH] split PCI probing code [1/9]

2005-07-14 Thread Francois Romieu
Greg KH <[EMAIL PROTECTED]> :
> On Thu, Jul 14, 2005 at 07:10:14PM +0200, Francois Romieu wrote:
> > Adam Belay <[EMAIL PROTECTED]> :
> > [...]
> > 
> > Some nits + a suspect error branch. It seems nice otherwise.
> 
> If I'm correct, this patch only moves the code into different files, it
> doesn't change any of it, so your comments apply to the current code
> today, not Adam's changes :)

The summary of the serie advertised cleaner code (TM). I wish I could clean
the clothes simply by moving them around.

[...]
> > mm/slab.c provides kcalloc.
> 
> Ick, I hate that function call, this is nicer :)

No problem. I just want to be sure that janitors have noticed it. O:o)

--
Ueimor
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC][PATCH] split PCI probing code [1/9]

2005-07-14 Thread Adam Belay
On Thu, 2005-07-14 at 12:30 -0700, Greg KH wrote:
> On Thu, Jul 14, 2005 at 07:10:14PM +0200, Francois Romieu wrote:
> > Adam Belay <[EMAIL PROTECTED]> :
> > [...]
> > 
> > Some nits + a suspect error branch. It seems nice otherwise.
> 
> If I'm correct, this patch only moves the code into different files, it
> doesn't change any of it, so your comments apply to the current code
> today, not Adam's changes :)

Correct.  I've been trying to make my changes incremental.  Nonetheless,
I do appreciate the comments.  I'll try to apply these fixes to my
current tree.

Thanks,
Adam


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC][PATCH] split PCI probing code [1/9]

2005-07-14 Thread Greg KH
On Thu, Jul 14, 2005 at 07:10:14PM +0200, Francois Romieu wrote:
> Adam Belay <[EMAIL PROTECTED]> :
> [...]
> 
> Some nits + a suspect error branch. It seems nice otherwise.

If I'm correct, this patch only moves the code into different files, it
doesn't change any of it, so your comments apply to the current code
today, not Adam's changes :)

> > --- a/drivers/pci/bus/bus.c 1969-12-31 19:00:00.0 -0500
> > +++ b/drivers/pci/bus/bus.c 2005-07-10 22:32:53.0 -0400
> [...]
> > +struct pci_bus * pci_alloc_bus(void)
> > +{
> > +   struct pci_bus *b;
> > +
> > +   b = kmalloc(sizeof(*b), GFP_KERNEL);
> > +   if (b) {
> > +   memset(b, 0, sizeof(*b));
> 
> mm/slab.c provides kcalloc.

Ick, I hate that function call, this is nicer :)

thanks,

greg k-h
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC][PATCH] split PCI probing code [1/9]

2005-07-14 Thread John Rose
I like it.  I'm a little hung up on the fact that actual device probing
happens in config.c rather than probe.c (if I'm correct).  Regardless,
this patch set cleans things up nicely.

John

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC][PATCH] split PCI probing code [1/9]

2005-07-14 Thread Francois Romieu
Adam Belay <[EMAIL PROTECTED]> :
[...]

Some nits + a suspect error branch. It seems nice otherwise.

> --- a/drivers/pci/bus/bus.c   1969-12-31 19:00:00.0 -0500
> +++ b/drivers/pci/bus/bus.c   2005-07-10 22:32:53.0 -0400
[...]
> +struct pci_bus * pci_alloc_bus(void)
> +{
> + struct pci_bus *b;
> +
> + b = kmalloc(sizeof(*b), GFP_KERNEL);
> + if (b) {
> + memset(b, 0, sizeof(*b));

mm/slab.c provides kcalloc.

[...]
> --- a/drivers/pci/bus/config.c1969-12-31 19:00:00.0 -0500
> +++ b/drivers/pci/bus/config.c2005-07-12 00:52:35.147664368 -0400
[...]
> +static void pci_read_bases(struct pci_dev *dev, unsigned int howmany, int 
> rom)
> +{
> + unsigned int pos, reg, next;
> + u32 l, sz;
> + struct resource *res;
> +
> + for(pos=0; pos +static struct pci_dev * __devinit
> +pci_scan_device(struct pci_bus *bus, int devfn)
> +{
[...]
> + dev = kmalloc(sizeof(struct pci_dev), GFP_KERNEL);
> + if (!dev)
> + return NULL;
> +
> + memset(dev, 0, sizeof(struct pci_dev));

kcalloc

[...]
> + /* Assume 32-bit PCI; let 64-bit PCI cards (which are far rarer)
> +set this higher, assuming the system even supports it.  */
> + dev->dma_mask = 0x;

DMA_32BIT_MASK

> + if (pci_setup_device(dev) < 0) {
> + kfree(dev);
> + return NULL;
> + }
> + device_initialize(>dev);
> + dev->dev.release = pci_release_dev;
> + pci_dev_get(dev);
> +
> + pci_name_device(dev);
> +
> + dev->dev.dma_mask = >dma_mask;
> + dev->dev.coherent_dma_mask = 0xull;

DMA_32BIT_MASK

[...]
> +struct pci_dev * __devinit
> +pci_scan_single_device(struct pci_bus *bus, int devfn)
> +{
> + struct pci_dev *dev;
> +
> + dev = pci_scan_device(bus, devfn);
> + pci_scan_msi_device(dev);
> +
> + if (!dev)
> + return NULL;

Why not do the test immediately ?

[...]
> --- a/drivers/pci/bus/probe.c 1969-12-31 19:00:00.0 -0500
> +++ b/drivers/pci/bus/probe.c 2005-07-12 00:55:50.580953992 -0400
[...]
> +int __devinit pci_scan_bridge(struct pci_bus *bus, struct pci_dev * dev, int 
> max, int pass)
[...]
> +
> + /* Prevent assigning a bus number that already exists.
> +  * This can happen when a bridge is hot-plugged */
> + if (pci_find_bus(pci_domain_nr(bus), max+1))

if (pci_find_bus(pci_domain_nr(bus), max + 1))

[...]
> + /*
> +  * For CardBus bridges, we leave 4 bus numbers
> +  * as cards with a PCI-to-PCI bridge can be
> +  * inserted later.
> +  */
> + for (i=0; i +int __devinit pci_scan_slot(struct pci_bus *bus, int devfn)
> +{
> + int func, nr = 0;
> + int scan_all_fns;
> +
> + scan_all_fns = pcibios_scan_all_fns(bus, devfn);
> +
> + for (func = 0; func < 8; func++, devfn++) {
> + struct pci_dev *dev;
> +
> + dev = pci_scan_single_device(bus, devfn);
> + if (dev) {
> + nr++;
> +
> + /*
> +  * If this is a single function device,
> +  * don't scan past the first function.
> +  */
> + if (!dev->multifunction) {
> + if (func > 0) {
> + dev->multifunction = 1;
> + } else {
> + break;
> + }

if (func == 0)
break;
dev->multifunction = 1;


[...]
> +unsigned int __devinit pci_scan_child_bus(struct pci_bus *bus)
> +{
[...]
> + pcibios_fixup_bus(bus);
> + for (pass=0; pass < 2; pass++)

for (pass = 0; pass < 2; pass++)

[...]
> +struct pci_bus * __devinit pci_scan_bus_parented(struct device *parent, int 
> bus, struct pci_ops *ops, void *sysdata)
> +{
> + int error;
> + struct pci_bus *b;
> + struct device *dev;
> +
> + b = pci_alloc_bus();
> + if (!b)
> + return NULL;
> +
> + dev = kmalloc(sizeof(*dev), GFP_KERNEL);
> + if (!dev){
> + kfree(b);
> + return NULL;
> + }

The code below uses goto. Why not here ?

> +
> + b->sysdata = sysdata;
> + b->ops = ops;
> +
> + if (pci_find_bus(pci_domain_nr(b), bus)) {
> + /* If we already got to this bus through a different bridge, 
> ignore it */
> + pr_debug("PCI: Bus %04x:%02x already known\n", 
> pci_domain_nr(b), bus);
> + goto err_out;
> + }
> + spin_lock(_bus_lock);
> + list_add_tail(>node, _root_buses);
> + spin_unlock(_bus_lock);
> +
> + memset(dev, 0, sizeof(*dev));

kcalloc

> + dev->parent = parent;
> + dev->release = pci_release_bus_bridge_dev;
> + 

[RFC][PATCH] split PCI probing code [1/9]

2005-07-14 Thread Adam Belay
This patch divides the PCI probing code into three smaller files:

config.c - PCI configuration space parsing
probe.c - PCI bus detection routines
bus.c - the PCI bus class driver core

These files are placed in the new directory "drivers/pci/bus".  It will
be used for functions related to the PCI bus class driver and PCI device
detection in general.

Signed-off-by: Adam Belay <[EMAIL PROTECTED]>


--- a/drivers/pci/Makefile  2005-07-08 17:06:19.0 -0400
+++ b/drivers/pci/Makefile  2005-07-10 22:32:53.0 -0400
@@ -2,9 +2,9 @@
 # Makefile for the PCI bus specific drivers.
 #
 
-obj-y  += access.o bus.o probe.o remove.o pci.o quirks.o \
-   names.o pci-driver.o search.o pci-sysfs.o \
-   rom.o
+obj-y  += access.o bus.o remove.o pci.o quirks.o names.o \
+  pci-driver.o search.o pci-sysfs.o rom.o bus/
+
 obj-$(CONFIG_PROC_FS) += proc.o
 
 ifndef CONFIG_SPARC64
--- a/drivers/pci/bus/Makefile  1969-12-31 19:00:00.0 -0500
+++ b/drivers/pci/bus/Makefile  2005-07-10 22:32:53.0 -0400
@@ -0,0 +1,5 @@
+#
+# Makefile for the PCI device detection
+#
+
+obj-y :=  bus.o config.o probe.o
--- a/drivers/pci/bus/bus.c 1969-12-31 19:00:00.0 -0500
+++ b/drivers/pci/bus/bus.c 2005-07-10 22:32:53.0 -0400
@@ -0,0 +1,69 @@
+/*
+ * bus.c - the PCI bus class driver
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+#include "bus.h"
+
+#undef DEBUG
+
+#ifdef DEBUG
+#define DBG(x...) printk(x)
+#else
+#define DBG(x...)
+#endif
+
+
+/*
+ * PCI Bus Class
+ */
+ 
+static void pci_release_bus_classdev(struct class_device *class_dev)
+{
+   struct pci_bus *pci_bus = to_pci_bus(class_dev);
+
+   if (pci_bus->bridge)
+   put_device(pci_bus->bridge);
+   kfree(pci_bus);
+}
+
+struct class pcibus_class = {
+   .name   = "pci_bus",
+   .release= _release_bus_classdev,
+};
+
+static int __init pcibus_class_init(void)
+{
+   return class_register(_class);
+}
+
+postcore_initcall(pcibus_class_init);
+
+
+/*
+ * Registration
+ */
+
+/**
+ * pci_alloc_bus - allocates a "pci_bus" structure
+ */
+struct pci_bus * pci_alloc_bus(void)
+{
+   struct pci_bus *b;
+
+   b = kmalloc(sizeof(*b), GFP_KERNEL);
+   if (b) {
+   memset(b, 0, sizeof(*b));
+   INIT_LIST_HEAD(>node);
+   INIT_LIST_HEAD(>children);
+   INIT_LIST_HEAD(>devices);
+   }
+   return b;
+}
+
+EXPORT_SYMBOL(pci_alloc_bus);
--- a/drivers/pci/bus/bus.h 1969-12-31 19:00:00.0 -0500
+++ b/drivers/pci/bus/bus.h 2005-07-10 22:32:53.0 -0400
@@ -0,0 +1,5 @@
+/*
+ * bus.h - functions internal to PCI device detection
+ */
+ 
+extern struct class pcibus_class; 
--- a/drivers/pci/bus/config.c  1969-12-31 19:00:00.0 -0500
+++ b/drivers/pci/bus/config.c  2005-07-12 00:52:35.147664368 -0400
@@ -0,0 +1,466 @@
+/*
+ * config.c - PCI configuration space parsing code
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "../pci.h"
+
+#define PCI_CFG_SPACE_SIZE 256
+#define PCI_CFG_SPACE_EXP_SIZE 4096
+
+LIST_HEAD(pci_devices);
+
+/**
+ * pci_release_dev - free a pci device structure when all users of it are 
finished.
+ * @dev: device that's been disconnected
+ *
+ * Will be called only by the device core when all users of this pci device are
+ * done.
+ */
+static void pci_release_dev(struct device *dev)
+{
+   struct pci_dev *pci_dev;
+
+   pci_dev = to_pci_dev(dev);
+   kfree(pci_dev);
+}
+
+/*
+ * Translate the low bits of the PCI base
+ * to the resource type
+ */
+static inline unsigned int pci_calc_resource_flags(unsigned int flags)
+{
+   if (flags & PCI_BASE_ADDRESS_SPACE_IO)
+   return IORESOURCE_IO;
+
+   if (flags & PCI_BASE_ADDRESS_MEM_PREFETCH)
+   return IORESOURCE_MEM | IORESOURCE_PREFETCH;
+
+   return IORESOURCE_MEM;
+}
+
+/*
+ * Find the extent of a PCI decode..
+ */
+static u32 pci_size(u32 base, u32 maxbase, unsigned long mask)
+{
+   u32 size = mask & maxbase;  /* Find the significant bits */
+   if (!size)
+   return 0;
+
+   /* Get the lowest of them to find the decode size, and
+  from that the extent.  */
+   size = (size & ~(size-1)) - 1;
+
+   /* base == maxbase can be valid only if the BAR has
+  already been programmed with all 1s.  */
+   if (base == maxbase && ((base | size) & mask) != mask)
+   return 0;
+
+   return size;
+}
+
+static void pci_read_bases(struct pci_dev *dev, unsigned int howmany, int rom)
+{
+   unsigned int pos, reg, next;
+   u32 l, sz;
+   struct resource *res;
+
+   for(pos=0; posresource[pos];
+   res->name = pci_name(dev);
+   reg = PCI_BASE_ADDRESS_0 + (pos << 2);
+   pci_read_config_dword(dev, reg, );
+   pci_write_config_dword(dev, reg, ~0);
+  

[RFC][PATCH] split PCI probing code [1/9]

2005-07-14 Thread Adam Belay
This patch divides the PCI probing code into three smaller files:

config.c - PCI configuration space parsing
probe.c - PCI bus detection routines
bus.c - the PCI bus class driver core

These files are placed in the new directory drivers/pci/bus.  It will
be used for functions related to the PCI bus class driver and PCI device
detection in general.

Signed-off-by: Adam Belay [EMAIL PROTECTED]


--- a/drivers/pci/Makefile  2005-07-08 17:06:19.0 -0400
+++ b/drivers/pci/Makefile  2005-07-10 22:32:53.0 -0400
@@ -2,9 +2,9 @@
 # Makefile for the PCI bus specific drivers.
 #
 
-obj-y  += access.o bus.o probe.o remove.o pci.o quirks.o \
-   names.o pci-driver.o search.o pci-sysfs.o \
-   rom.o
+obj-y  += access.o bus.o remove.o pci.o quirks.o names.o \
+  pci-driver.o search.o pci-sysfs.o rom.o bus/
+
 obj-$(CONFIG_PROC_FS) += proc.o
 
 ifndef CONFIG_SPARC64
--- a/drivers/pci/bus/Makefile  1969-12-31 19:00:00.0 -0500
+++ b/drivers/pci/bus/Makefile  2005-07-10 22:32:53.0 -0400
@@ -0,0 +1,5 @@
+#
+# Makefile for the PCI device detection
+#
+
+obj-y :=  bus.o config.o probe.o
--- a/drivers/pci/bus/bus.c 1969-12-31 19:00:00.0 -0500
+++ b/drivers/pci/bus/bus.c 2005-07-10 22:32:53.0 -0400
@@ -0,0 +1,69 @@
+/*
+ * bus.c - the PCI bus class driver
+ *
+ */
+
+#include linux/init.h
+#include linux/pci.h
+#include linux/slab.h
+#include linux/module.h
+
+#include bus.h
+
+#undef DEBUG
+
+#ifdef DEBUG
+#define DBG(x...) printk(x)
+#else
+#define DBG(x...)
+#endif
+
+
+/*
+ * PCI Bus Class
+ */
+ 
+static void pci_release_bus_classdev(struct class_device *class_dev)
+{
+   struct pci_bus *pci_bus = to_pci_bus(class_dev);
+
+   if (pci_bus-bridge)
+   put_device(pci_bus-bridge);
+   kfree(pci_bus);
+}
+
+struct class pcibus_class = {
+   .name   = pci_bus,
+   .release= pci_release_bus_classdev,
+};
+
+static int __init pcibus_class_init(void)
+{
+   return class_register(pcibus_class);
+}
+
+postcore_initcall(pcibus_class_init);
+
+
+/*
+ * Registration
+ */
+
+/**
+ * pci_alloc_bus - allocates a pci_bus structure
+ */
+struct pci_bus * pci_alloc_bus(void)
+{
+   struct pci_bus *b;
+
+   b = kmalloc(sizeof(*b), GFP_KERNEL);
+   if (b) {
+   memset(b, 0, sizeof(*b));
+   INIT_LIST_HEAD(b-node);
+   INIT_LIST_HEAD(b-children);
+   INIT_LIST_HEAD(b-devices);
+   }
+   return b;
+}
+
+EXPORT_SYMBOL(pci_alloc_bus);
--- a/drivers/pci/bus/bus.h 1969-12-31 19:00:00.0 -0500
+++ b/drivers/pci/bus/bus.h 2005-07-10 22:32:53.0 -0400
@@ -0,0 +1,5 @@
+/*
+ * bus.h - functions internal to PCI device detection
+ */
+ 
+extern struct class pcibus_class; 
--- a/drivers/pci/bus/config.c  1969-12-31 19:00:00.0 -0500
+++ b/drivers/pci/bus/config.c  2005-07-12 00:52:35.147664368 -0400
@@ -0,0 +1,466 @@
+/*
+ * config.c - PCI configuration space parsing code
+ */
+
+#include linux/delay.h
+#include linux/pci.h
+#include linux/slab.h
+#include linux/module.h
+#include linux/cpumask.h
+
+#include ../pci.h
+
+#define PCI_CFG_SPACE_SIZE 256
+#define PCI_CFG_SPACE_EXP_SIZE 4096
+
+LIST_HEAD(pci_devices);
+
+/**
+ * pci_release_dev - free a pci device structure when all users of it are 
finished.
+ * @dev: device that's been disconnected
+ *
+ * Will be called only by the device core when all users of this pci device are
+ * done.
+ */
+static void pci_release_dev(struct device *dev)
+{
+   struct pci_dev *pci_dev;
+
+   pci_dev = to_pci_dev(dev);
+   kfree(pci_dev);
+}
+
+/*
+ * Translate the low bits of the PCI base
+ * to the resource type
+ */
+static inline unsigned int pci_calc_resource_flags(unsigned int flags)
+{
+   if (flags  PCI_BASE_ADDRESS_SPACE_IO)
+   return IORESOURCE_IO;
+
+   if (flags  PCI_BASE_ADDRESS_MEM_PREFETCH)
+   return IORESOURCE_MEM | IORESOURCE_PREFETCH;
+
+   return IORESOURCE_MEM;
+}
+
+/*
+ * Find the extent of a PCI decode..
+ */
+static u32 pci_size(u32 base, u32 maxbase, unsigned long mask)
+{
+   u32 size = mask  maxbase;  /* Find the significant bits */
+   if (!size)
+   return 0;
+
+   /* Get the lowest of them to find the decode size, and
+  from that the extent.  */
+   size = (size  ~(size-1)) - 1;
+
+   /* base == maxbase can be valid only if the BAR has
+  already been programmed with all 1s.  */
+   if (base == maxbase  ((base | size)  mask) != mask)
+   return 0;
+
+   return size;
+}
+
+static void pci_read_bases(struct pci_dev *dev, unsigned int howmany, int rom)
+{
+   unsigned int pos, reg, next;
+   u32 l, sz;
+   struct resource *res;
+
+   for(pos=0; poshowmany; pos = next) {
+   next = pos+1;
+   res = dev-resource[pos];
+   res-name = 

Re: [RFC][PATCH] split PCI probing code [1/9]

2005-07-14 Thread Francois Romieu
Adam Belay [EMAIL PROTECTED] :
[...]

Some nits + a suspect error branch. It seems nice otherwise.

 --- a/drivers/pci/bus/bus.c   1969-12-31 19:00:00.0 -0500
 +++ b/drivers/pci/bus/bus.c   2005-07-10 22:32:53.0 -0400
[...]
 +struct pci_bus * pci_alloc_bus(void)
 +{
 + struct pci_bus *b;
 +
 + b = kmalloc(sizeof(*b), GFP_KERNEL);
 + if (b) {
 + memset(b, 0, sizeof(*b));

mm/slab.c provides kcalloc.

[...]
 --- a/drivers/pci/bus/config.c1969-12-31 19:00:00.0 -0500
 +++ b/drivers/pci/bus/config.c2005-07-12 00:52:35.147664368 -0400
[...]
 +static void pci_read_bases(struct pci_dev *dev, unsigned int howmany, int 
 rom)
 +{
 + unsigned int pos, reg, next;
 + u32 l, sz;
 + struct resource *res;
 +
 + for(pos=0; poshowmany; pos = next) {

for (pos = 0; pos  howmany; pos = next) {

[...]
 +static struct pci_dev * __devinit
 +pci_scan_device(struct pci_bus *bus, int devfn)
 +{
[...]
 + dev = kmalloc(sizeof(struct pci_dev), GFP_KERNEL);
 + if (!dev)
 + return NULL;
 +
 + memset(dev, 0, sizeof(struct pci_dev));

kcalloc

[...]
 + /* Assume 32-bit PCI; let 64-bit PCI cards (which are far rarer)
 +set this higher, assuming the system even supports it.  */
 + dev-dma_mask = 0x;

DMA_32BIT_MASK

 + if (pci_setup_device(dev)  0) {
 + kfree(dev);
 + return NULL;
 + }
 + device_initialize(dev-dev);
 + dev-dev.release = pci_release_dev;
 + pci_dev_get(dev);
 +
 + pci_name_device(dev);
 +
 + dev-dev.dma_mask = dev-dma_mask;
 + dev-dev.coherent_dma_mask = 0xull;

DMA_32BIT_MASK

[...]
 +struct pci_dev * __devinit
 +pci_scan_single_device(struct pci_bus *bus, int devfn)
 +{
 + struct pci_dev *dev;
 +
 + dev = pci_scan_device(bus, devfn);
 + pci_scan_msi_device(dev);
 +
 + if (!dev)
 + return NULL;

Why not do the test immediately ?

[...]
 --- a/drivers/pci/bus/probe.c 1969-12-31 19:00:00.0 -0500
 +++ b/drivers/pci/bus/probe.c 2005-07-12 00:55:50.580953992 -0400
[...]
 +int __devinit pci_scan_bridge(struct pci_bus *bus, struct pci_dev * dev, int 
 max, int pass)
[...]
 +
 + /* Prevent assigning a bus number that already exists.
 +  * This can happen when a bridge is hot-plugged */
 + if (pci_find_bus(pci_domain_nr(bus), max+1))

if (pci_find_bus(pci_domain_nr(bus), max + 1))

[...]
 + /*
 +  * For CardBus bridges, we leave 4 bus numbers
 +  * as cards with a PCI-to-PCI bridge can be
 +  * inserted later.
 +  */
 + for (i=0; iCARDBUS_RESERVE_BUSNR; i++)

for (i = 0; i  CARDBUS_RESERVE_BUSNR; i++)


[...]
 +int __devinit pci_scan_slot(struct pci_bus *bus, int devfn)
 +{
 + int func, nr = 0;
 + int scan_all_fns;
 +
 + scan_all_fns = pcibios_scan_all_fns(bus, devfn);
 +
 + for (func = 0; func  8; func++, devfn++) {
 + struct pci_dev *dev;
 +
 + dev = pci_scan_single_device(bus, devfn);
 + if (dev) {
 + nr++;
 +
 + /*
 +  * If this is a single function device,
 +  * don't scan past the first function.
 +  */
 + if (!dev-multifunction) {
 + if (func  0) {
 + dev-multifunction = 1;
 + } else {
 + break;
 + }

if (func == 0)
break;
dev-multifunction = 1;


[...]
 +unsigned int __devinit pci_scan_child_bus(struct pci_bus *bus)
 +{
[...]
 + pcibios_fixup_bus(bus);
 + for (pass=0; pass  2; pass++)

for (pass = 0; pass  2; pass++)

[...]
 +struct pci_bus * __devinit pci_scan_bus_parented(struct device *parent, int 
 bus, struct pci_ops *ops, void *sysdata)
 +{
 + int error;
 + struct pci_bus *b;
 + struct device *dev;
 +
 + b = pci_alloc_bus();
 + if (!b)
 + return NULL;
 +
 + dev = kmalloc(sizeof(*dev), GFP_KERNEL);
 + if (!dev){
 + kfree(b);
 + return NULL;
 + }

The code below uses goto. Why not here ?

 +
 + b-sysdata = sysdata;
 + b-ops = ops;
 +
 + if (pci_find_bus(pci_domain_nr(b), bus)) {
 + /* If we already got to this bus through a different bridge, 
 ignore it */
 + pr_debug(PCI: Bus %04x:%02x already known\n, 
 pci_domain_nr(b), bus);
 + goto err_out;
 + }
 + spin_lock(pci_bus_lock);
 + list_add_tail(b-node, pci_root_buses);
 + spin_unlock(pci_bus_lock);
 +
 + memset(dev, 0, sizeof(*dev));

kcalloc

 + dev-parent = parent;
 + 

Re: [RFC][PATCH] split PCI probing code [1/9]

2005-07-14 Thread Greg KH
On Thu, Jul 14, 2005 at 07:10:14PM +0200, Francois Romieu wrote:
 Adam Belay [EMAIL PROTECTED] :
 [...]
 
 Some nits + a suspect error branch. It seems nice otherwise.

If I'm correct, this patch only moves the code into different files, it
doesn't change any of it, so your comments apply to the current code
today, not Adam's changes :)

  --- a/drivers/pci/bus/bus.c 1969-12-31 19:00:00.0 -0500
  +++ b/drivers/pci/bus/bus.c 2005-07-10 22:32:53.0 -0400
 [...]
  +struct pci_bus * pci_alloc_bus(void)
  +{
  +   struct pci_bus *b;
  +
  +   b = kmalloc(sizeof(*b), GFP_KERNEL);
  +   if (b) {
  +   memset(b, 0, sizeof(*b));
 
 mm/slab.c provides kcalloc.

Ick, I hate that function call, this is nicer :)

thanks,

greg k-h
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC][PATCH] split PCI probing code [1/9]

2005-07-14 Thread Adam Belay
On Thu, 2005-07-14 at 12:30 -0700, Greg KH wrote:
 On Thu, Jul 14, 2005 at 07:10:14PM +0200, Francois Romieu wrote:
  Adam Belay [EMAIL PROTECTED] :
  [...]
  
  Some nits + a suspect error branch. It seems nice otherwise.
 
 If I'm correct, this patch only moves the code into different files, it
 doesn't change any of it, so your comments apply to the current code
 today, not Adam's changes :)

Correct.  I've been trying to make my changes incremental.  Nonetheless,
I do appreciate the comments.  I'll try to apply these fixes to my
current tree.

Thanks,
Adam


-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC][PATCH] split PCI probing code [1/9]

2005-07-14 Thread Francois Romieu
Greg KH [EMAIL PROTECTED] :
 On Thu, Jul 14, 2005 at 07:10:14PM +0200, Francois Romieu wrote:
  Adam Belay [EMAIL PROTECTED] :
  [...]
  
  Some nits + a suspect error branch. It seems nice otherwise.
 
 If I'm correct, this patch only moves the code into different files, it
 doesn't change any of it, so your comments apply to the current code
 today, not Adam's changes :)

The summary of the serie advertised cleaner code (TM). I wish I could clean
the clothes simply by moving them around.

[...]
  mm/slab.c provides kcalloc.
 
 Ick, I hate that function call, this is nicer :)

No problem. I just want to be sure that janitors have noticed it. O:o)

--
Ueimor
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/