Re: [PATCH V2 0/4] PCI: code clean up on pci configuration space

[Wei Yang <weiyang@linux.vnet.ibm.com>]

On Tue, Jul 14, 2015 at 04:37:38PM -0500, Bjorn Helgaas wrote:
>On Tue, Jun 30, 2015 at 09:16:40AM +0800, Wei Yang wrote:
>> This series is a clean up in the pci subsystem when accessing the pci
>> configuration space.
>> 
>> The first one is to re-use the PCI_FIND_CAP_TTL to limit the times iterating
>> in pci configuration space.
>> 
>> The next three are to use the exact type to access the pci cap and pcie ext
>> cap.
>> 
>> Tested on x86 and powerpc on top of v4.1.
>> 
>> The original thread could be referenced in below link:
>> http://comments.gmane.org/gmane.linux.kernel.pci/35931
>> 
>> ---
>> v1->v2:
>>    * define PCI_FIND_CAP_TTL in drivers/pci/pci.h instead of
>>      include/linux/pci.h
>>    * split the change for return position check from second one, and make the
>>      forth patch
>> 
>> Wei Yang (4):
>>   PCI: move PCI_FIND_CAP_TTL to pci.h and use it in quirks
>>   PCI: use u8 to represent pci configuration space pos and cap
>>   PCI: use u16 to represent pci express extended capabilities pos and
>>     cap
>>   PCI: consolidate return value check for pci_find_(ext_)capability
>
>The first and last seem fine to me.  As far as I can tell, the last does
>not actually depend on the u8 and u16 changes.
>

With patch 2/3, changing the return value, it makes sure pos will not be
negative. So that patch 4 could make sure there is not cap when pos is 0. And
no possibility to check the negative value.

>The u8 and u16 patches change the signatures of several exported functions.
>Do they fix some problem?  Unless there's something broken, these seem like
>pretty minor changes, and I don't think they're worth it.
>

You are right, no functional change and currently no bug related.

While without 2/3, it would not be that clear the pos return value must not
be negative. Then then the change in patch 4 would not be that obvious.

>Bjorn

-- 
Richard Yang
Help you, Help me