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

On Thu, Nov 19, 2015 at 11:10:42AM +1100, Alexey Kardashevskiy wrote:

On 11/17/2015 12:04 PM, Gavin Shan wrote:

quoted

On Mon, Nov 16, 2015 at 07:01:59PM +1100, Alexey Kardashevskiy wrote:

quoted

On 11/05/2015 12:12 AM, Gavin Shan wrote:

quoted

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