[PATCH v7 10/50] powerpc/powernv: Simplify pnv_ioda_setup_pe_seg()

[Alexey Kardashevskiy]

On 11/06/2015 10:52 AM, Gavin Shan wrote:
> On Fri, Nov 06, 2015 at 09:56:06AM +1100, Daniel Axtens wrote:
>> Gavin Shan <gwshan at linux.vnet.ibm.com> writes:
>>
>>> The original implementation of pnv_ioda_setup_pe_seg() configures
>>> IO and M32 segments by separate logics, which can be merged by
>>> by caching @segmap, @seg_size, @win in advance. This shouldn't
>>> cause any behavioural changes.
>>>
>>> Signed-off-by: Gavin Shan <gwshan at linux.vnet.ibm.com>
>>> ---
>>>   arch/powerpc/platforms/powernv/pci-ioda.c | 62 ++++++++++++++-----------------
>>>   1 file changed, 28 insertions(+), 34 deletions(-)
>>>
>>> diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
>>> index 7ee7cfe..553d3f3 100644
>>> --- a/arch/powerpc/platforms/powernv/pci-ioda.c
>>> +++ b/arch/powerpc/platforms/powernv/pci-ioda.c
>>> @@ -2752,8 +2752,10 @@ static void pnv_ioda_setup_pe_seg(struct pci_controller *hose,
>>>   	struct pnv_phb *phb = hose->private_data;
>>>   	struct pci_bus_region region;
>>>   	struct resource *res;
>>> -	int i, index;
>>> -	int rc;
>>> +	unsigned int segsize;
>>> +	int *segmap, index, i;
>>> +	uint16_t win;
>>> +	int64_t rc;
>>
>> Good catch! Opal return codes are 64 bit and that should be explicit
>> in the type. However, I seem to remember that we preferred a different
>> type for 64 bit ints in the kernel. I think it's s64, and there are some
>> other uses of that in pci_ioda.c for return codes.
>>
>
> Both int64_t and s64 are fine. I used s64 for the OPAL return value, but
> Alexey likes "int64_t", which is ok to me as well. I won't change it back
> to s64 :-)
>
>> (I'm actually surprised that's not picked up as a compiler
>> warning. Maybe that's something to look at in future.)
>>
>
> Indeed, I didn't see a warning from gcc.
>
>> The rest of the patch looks good on casual inspection - to be sure I'll
>> test the entire series on a machine. (hopefully, time permitting!)
>>
>
> I run scripts/checkpatch.pl on the patchset. Only one warning came from
> [PATCH 44/50], but I won't bother to change that as the warning was
> brought by original code.

None of these patches failed checkpatch.pl check, what was the error in 44/50?




-- 
Alexey