Re: [PATCH 2/5] PCI: Try to assign required+option size at first

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Jan 6, 2012 at 1:49 PM, Jesse Barnes <jbarnes@xxxxxxxxxxxxxxxx> wrote:
>
> Overall in looking at all this again I regret not asking for more
> cleanups before it went in:
>  1) resource_list_x?  really?
>  2) why aren't we using list_head?

Ugh. It's an abomination based on the 'struct resource_list' that
exists in <linux/ioport.h>. Which also has just a "next" pointer.

But that should actually be entirely private to
drivers/pci/setup-bus.c, except it is *also* used by
drivers/pci/setup-res.c: pdev_sort_resources().

I suspect that it would be a good idea to clean this up by

 (a) moving that pdev_sort_resources() to setup-bus.c (only user) and
making it static, and removing the declaration for <linux/pci.h>

 (b) moving 'struct resource_list' to also be local to setup-bus.c

 (c) just extending the resource_list with the few extra firlds that
'struct resource_list_x' has now, and getting rid of the stupid _x
version.

 (d) make it use 'struct list_head'. This patch adds those
'remove_from_list()' helpers exactly because the code isn't using the
list_head, so it does the stupid "walk the singly linked list to find
the entry" and as far as I can tell, it does it *wrong* (first entry
case?).

All of those cleanups are independent of this particular patch,
though, so should not be mixed up with this. They all look entirely
mechanical, though. And getting rid of that totally broken
"remove_from_list()" would be good (but requires that you do the
list_head cleanup first).

I dunno. It doesn't look all that horrible apart from the fairly
cosmetic issues, though.

                      Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux