All of lore.kernel.org
 help / color / mirror / Atom feed
From: Wei Yang <weiyang@linux.vnet.ibm.com>
To: Bjorn Helgaas <bhelgaas@google.com>
Cc: Wei Yang <weiyang@linux.vnet.ibm.com>, linux-pci@vger.kernel.org
Subject: Re: [PATCH V2 0/4] PCI: code clean up on pci configuration space
Date: Wed, 15 Jul 2015 10:08:52 +0800	[thread overview]
Message-ID: <20150715020852.GD6525@richard> (raw)
In-Reply-To: <20150714213738.GJ24416@google.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


      reply	other threads:[~2015-07-15  2:09 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-30  1:16 [PATCH V2 0/4] PCI: code clean up on pci configuration space Wei Yang
2015-06-30  1:16 ` [PATCH V2 1/4] PCI: move PCI_FIND_CAP_TTL to pci.h and use it in quirks Wei Yang
2015-07-14 21:57   ` Bjorn Helgaas
2015-06-30  1:16 ` [PATCH V2 2/4] PCI: use u8 to represent pci configuration space pos and cap Wei Yang
2015-06-30  1:16 ` [PATCH V2 3/4] PCI: use u16 to represent pci express extended capabilities " Wei Yang
2015-06-30  1:16 ` [PATCH V2 4/4] PCI: consolidate return value check for pci_find_(ext_)capability Wei Yang
2015-07-14 22:00   ` Bjorn Helgaas
2015-07-15  2:02     ` Wei Yang
2015-07-15  2:11       ` Bjorn Helgaas
2015-07-15  5:46         ` Wei Yang
2015-07-14 21:37 ` [PATCH V2 0/4] PCI: code clean up on pci configuration space Bjorn Helgaas
2015-07-15  2:08   ` Wei Yang [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150715020852.GD6525@richard \
    --to=weiyang@linux.vnet.ibm.com \
    --cc=bhelgaas@google.com \
    --cc=linux-pci@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.