Re: [PATCH v7 12/50] powerpc/powernv: Track M64 segment consumption

Gavin Shan
2015-11-23T22:43:32Z

On Thu, Nov 19, 2015 at 11:10:42AM +1100, Alexey Kardashevskiy wrote:
>On 11/17/2015 12:04 PM, Gavin Shan wrote:
>>On Mon, Nov 16, 2015 at 07:01:59PM +1100, Alexey Kardashevskiy wrote:
>>>On 11/05/2015 12:12 AM, Gavin Shan wrote:
>>>>As we track M32 segment consumption, this introduces an array to
>>>>the PHB to track the mapping between M64 segment and PE number.
>>>>The information is going to be used to find M64 segment from the
>>>>PE number during PCI unplugging time in subsequent patches.
>>>
>>>
>>>It would not hurt to put a few words about how we managed to live without
>>>such a mapping for M64 before but we needed mapping for M32.
>>>
>>
>>The M32 mapping (phb->ioda.m32_segmap[]) isn't used for anything before
>>this patcheset. There're no need for M64 mapping before this patchset
>>similarly, no need to add the words.
>
>After years I learned that reviewers ask less questions about new but yet
>unused code when I put few words in the commit log confirming that it is not
>used now but it will be used for <here I put what it is for> later.
>
>And it is not obvious that m32_segment is not used. And m64_segmap is started
>being used only 13 patches later in:
>
>[PATCH v7 27/50] powerpc/powernv: Dynamically release PEs
>
>which is quite far, complicates reviewing. 12/50 is better be moved there (to
>make it 26/50) or just merged into 27/50.
>

It doesn't make sense to me. As said in PATCH[00/50], the patchset consists of
3 separate parts: PowerNV PCI rework; Using PCI slot; Hotplug standalone driver;
For the first part ("PowerNV PCI rework"), the patches are organized in order:
refactor/cleanup, IO/M32/M64, DMA, PE allocation/deallocation. So I don't think
I need move the patch around if you don't have a stronger reason.

Thanks,
Gavin