[Skiboot] [PATCH 2/3] core/pci: Change capability data type to uint64_t

[Gavin Shan]

On Thu, Jun 15, 2017 at 03:43:18PM +1000, Benjamin Herrenschmidt wrote:
>On Fri, 2017-05-26 at 11:53 +1000, Gavin Shan wrote:
>> This changes the capability data type from "void *" to "uint64_t"
>> so that it can hold pointer and data at the same time. No logicial
>> changes.
>
>I don't like that capability data stuff. It's only used by SR-IOV
>isn't it ?
>
>Why not just have a struct iov pointer in the pci_dev that you keep
>NULL when not used ? At least you get type checking...
>
>So I'd rather you remove the capability data completely.
>
>Also, we should just scan all caps at probe time and fill up the
>cache once and for all, so we don't have to use pci_set_cap() and
>worry as to whether a capability offset is cached or not.
>

Ok. All suggestions will be included in v2.

Cheers,
Gavin

>> Signed-off-by: Gavin Shan <gwshan at linux.vnet.ibm.com>
>> ---
>>  core/pci-iov.c | 2 +-
>>  core/pci.c     | 4 ++--
>>  include/pci.h  | 8 ++++----
>>  3 files changed, 7 insertions(+), 7 deletions(-)
>> 
>> diff --git a/core/pci-iov.c b/core/pci-iov.c
>> index 14c810b..7b1d6dd 100644
>> --- a/core/pci-iov.c
>> +++ b/core/pci-iov.c
>> @@ -253,5 +253,5 @@ void pci_init_iov_cap(struct phb *phb, struct pci_device *pd)
>>  	iov->pos = pos;
>>  	iov->enabled = false;
>>  	pci_iov_update_parameters(iov);
>> -	pci_set_cap(pd, PCIECAP_ID_SRIOV, pos, iov, true);
>> +	pci_set_cap(pd, PCIECAP_ID_SRIOV, pos, (uint64_t)iov, true);
>>  }
>> diff --git a/core/pci.c b/core/pci.c
>> index 7ec8409..29c3df6 100644
>> --- a/core/pci.c
>> +++ b/core/pci.c
>> @@ -162,7 +162,7 @@ static void pci_init_pcie_cap(struct phb *phb, struct pci_device *pd)
>>  		return;
>>  	}
>>  
>> -	pci_set_cap(pd, PCI_CFG_CAP_ID_EXP, ecap, NULL, false);
>> +	pci_set_cap(pd, PCI_CFG_CAP_ID_EXP, ecap, 0ul, false);
>>  
>>  	/*
>>  	 * XXX We observe a problem on some PLX switches where one
>> @@ -198,7 +198,7 @@ static void pci_init_aer_cap(struct phb *phb, struct pci_device *pd)
>>  
>>  	pos = pci_find_ecap(phb, pd->bdfn, PCIECAP_ID_AER, NULL);
>>  	if (pos > 0)
>> -		pci_set_cap(pd, PCIECAP_ID_AER, pos, NULL, true);
>> +		pci_set_cap(pd, PCIECAP_ID_AER, pos, 0ul, true);
>>  }
>>  
>>  void pci_init_capabilities(struct phb *phb, struct pci_device *pd)
>> diff --git a/include/pci.h b/include/pci.h
>> index dc418a9..aaf11a6 100644
>> --- a/include/pci.h
>> +++ b/include/pci.h
>> @@ -76,7 +76,7 @@ struct pci_device {
>>  	uint64_t		cap_list;
>>  	struct {
>>  		uint32_t	pos;
>> -		void		*data;
>> +		uint64_t	data;
>>  	} cap[64];
>>  	uint32_t		mps;		/* Max payload size capability */
>>  
>> @@ -91,8 +91,8 @@ struct pci_device {
>>  	struct list_node	link;
>>  };
>>  
>> -static inline void pci_set_cap(struct pci_device *pd, int id,
>> -			       int pos, void *data, bool ext)
>> +static inline void pci_set_cap(struct pci_device *pd, uint32_t id,
>> +			       uint32_t pos, uint64_t data, bool ext)
>>  {
>>  	if (!ext) {
>>  		pd->cap_list |= (0x1ul << id);
>> @@ -123,7 +123,7 @@ static inline int pci_cap(struct pci_device *pd,
>>  		return pd->cap[id + 32].pos;
>>  }
>>  
>> -static inline void *pci_cap_data(struct pci_device *pd, int id, bool ext)
>> +static inline uint64_t pci_cap_data(struct pci_device *pd, int id, bool ext)
>>  {
>>  	if (!ext)
>>  		return pd->cap[id].data;
>