Re: [PATCH v4 00/18] nvme: factor out cmb/pmr setup

Re: [PATCH v4 00/18] nvme: factor out cmb/pmr setup

[Klaus Jensen]

On Apr 22 13:01, Klaus Jensen wrote:
> From: Klaus Jensen <k.jensen@samsung.com>
> 
> Changes since v3
> ~~~~~~~~~~~~~~~~
> * Remove the addition of a new PROPERTIES macro in "nvme: move device
>   parameters to separate struct" (Philippe)
> 
> * Add NVME_PMR_BIR constant and use it in PMR setup.
> 
> * Split "nvme: factor out cmb/pmr setup" into
>   - "nvme: factor out cmb setup",
>   - "nvme: factor out pmr setup" and
>   - "nvme: do cmb/pmr init as part of pci init"
>   (Philippe)
> 
> 
> Klaus Jensen (18):
>   nvme: fix pci doorbell size calculation
>   nvme: rename trace events to pci_nvme
>   nvme: remove superfluous breaks
>   nvme: move device parameters to separate struct
>   nvme: use constants in identify
>   nvme: refactor nvme_addr_read
>   nvme: add max_ioqpairs device parameter
>   nvme: remove redundant cmbloc/cmbsz members
>   nvme: factor out property/constraint checks
>   nvme: factor out device state setup
>   nvme: factor out block backend setup
>   nvme: add namespace helpers
>   nvme: factor out namespace setup
>   nvme: factor out pci setup
>   nvme: factor out cmb setup
>   nvme: factor out pmr setup
>   nvme: do cmb/pmr init as part of pci init
>   nvme: factor out controller identify setup
> 
>  hw/block/nvme.c       | 543 ++++++++++++++++++++++++------------------
>  hw/block/nvme.h       |  31 ++-
>  hw/block/trace-events | 180 +++++++-------
>  include/block/nvme.h  |   8 +
>  4 files changed, 429 insertions(+), 333 deletions(-)
> 
> -- 
> 2.26.2
> 
> 

Gentle bump on this.

I apparently managed to screw up the git send-email this time, loosing a
bunch of CCs in the process. Sorry about that.

Re: [PATCH v4 00/18] nvme: factor out cmb/pmr setup

[Klaus Jensen]

On Apr 29 07:40, Klaus Jensen wrote:
> On Apr 22 13:01, Klaus Jensen wrote:
> > From: Klaus Jensen <k.jensen@samsung.com>
> > 
> > Changes since v3
> > ~~~~~~~~~~~~~~~~
> > * Remove the addition of a new PROPERTIES macro in "nvme: move device
> >   parameters to separate struct" (Philippe)
> > 
> > * Add NVME_PMR_BIR constant and use it in PMR setup.
> > 
> > * Split "nvme: factor out cmb/pmr setup" into
> >   - "nvme: factor out cmb setup",
> >   - "nvme: factor out pmr setup" and
> >   - "nvme: do cmb/pmr init as part of pci init"
> >   (Philippe)
> > 
> > 
> > Klaus Jensen (18):
> >   nvme: fix pci doorbell size calculation
> >   nvme: rename trace events to pci_nvme
> >   nvme: remove superfluous breaks
> >   nvme: move device parameters to separate struct
> >   nvme: use constants in identify
> >   nvme: refactor nvme_addr_read
> >   nvme: add max_ioqpairs device parameter
> >   nvme: remove redundant cmbloc/cmbsz members
> >   nvme: factor out property/constraint checks
> >   nvme: factor out device state setup
> >   nvme: factor out block backend setup
> >   nvme: add namespace helpers
> >   nvme: factor out namespace setup
> >   nvme: factor out pci setup
> >   nvme: factor out cmb setup
> >   nvme: factor out pmr setup
> >   nvme: do cmb/pmr init as part of pci init
> >   nvme: factor out controller identify setup
> > 
> >  hw/block/nvme.c       | 543 ++++++++++++++++++++++++------------------
> >  hw/block/nvme.h       |  31 ++-
> >  hw/block/trace-events | 180 +++++++-------
> >  include/block/nvme.h  |   8 +
> >  4 files changed, 429 insertions(+), 333 deletions(-)
> > 
> > -- 
> > 2.26.2
> > 
> > 
> 
> Gentle bump on this.
> 
> I apparently managed to screw up the git send-email this time, loosing a
> bunch of CCs in the process. Sorry about that.
> 

Bumping again. I have not received any new comments on this.

I'm missing a couple of Reviewed-by's (they all carry Maxim's) on

  nvme: move device parameters to separate struct
      I think this can also carry Philippe's Reviewed-by, since the only
      change is the removal of the PROPERTIES macro.

  nvme: factor out cmb setup
  nvme: factor out pmr setup
  nvme: do cmb/pmr init as part of pci init
      I think these could also carry Reviewed-by from Keith as well,
      since the only change is also factoring out the PMR setup (which
      was not there when Keith reviewed it) and the splitting into two
      trivial patches per request from Philippe.


Thanks,
Klaus

Re: [PATCH v4 00/18] nvme: factor out cmb/pmr setup

[Philippe Mathieu-Daudé]

Hi Klaus,

On 5/5/20 6:31 AM, Klaus Jensen wrote:
> On Apr 29 07:40, Klaus Jensen wrote:
>> On Apr 22 13:01, Klaus Jensen wrote:
>>> From: Klaus Jensen <k.jensen@samsung.com>
>>>
>>> Changes since v3
>>> ~~~~~~~~~~~~~~~~
>>> * Remove the addition of a new PROPERTIES macro in "nvme: move device
>>>    parameters to separate struct" (Philippe)
>>>
>>> * Add NVME_PMR_BIR constant and use it in PMR setup.
>>>
>>> * Split "nvme: factor out cmb/pmr setup" into
>>>    - "nvme: factor out cmb setup",
>>>    - "nvme: factor out pmr setup" and
>>>    - "nvme: do cmb/pmr init as part of pci init"
>>>    (Philippe)
>>>
>>>
>>> Klaus Jensen (18):
>>>    nvme: fix pci doorbell size calculation
>>>    nvme: rename trace events to pci_nvme
>>>    nvme: remove superfluous breaks
>>>    nvme: move device parameters to separate struct
>>>    nvme: use constants in identify
>>>    nvme: refactor nvme_addr_read
>>>    nvme: add max_ioqpairs device parameter
>>>    nvme: remove redundant cmbloc/cmbsz members
>>>    nvme: factor out property/constraint checks
>>>    nvme: factor out device state setup
>>>    nvme: factor out block backend setup
>>>    nvme: add namespace helpers
>>>    nvme: factor out namespace setup
>>>    nvme: factor out pci setup
>>>    nvme: factor out cmb setup
>>>    nvme: factor out pmr setup
>>>    nvme: do cmb/pmr init as part of pci init
>>>    nvme: factor out controller identify setup
>>>
>>>   hw/block/nvme.c       | 543 ++++++++++++++++++++++++------------------
>>>   hw/block/nvme.h       |  31 ++-
>>>   hw/block/trace-events | 180 +++++++-------
>>>   include/block/nvme.h  |   8 +
>>>   4 files changed, 429 insertions(+), 333 deletions(-)
>>>
>>> -- 
>>> 2.26.2
>>>
>>>
>>
>> Gentle bump on this.
>>
>> I apparently managed to screw up the git send-email this time, loosing a
>> bunch of CCs in the process. Sorry about that.
>>
> 
> Bumping again. I have not received any new comments on this.

My understanding is:
- this series goes via Kevin tree
- Kevin was waiting for Keith review (which occurred)
- Kevin tried to apply and asked for rebase
- Minor cosmetics changes on top (not logical)

> 
> I'm missing a couple of Reviewed-by's (they all carry Maxim's) on
> 
>    nvme: move device parameters to separate struct
>        I think this can also carry Philippe's Reviewed-by, since the only
>        change is the removal of the PROPERTIES macro.

I don't have this anymore in my mailbox, meaning I processed your 
series, likely giving a R-b.

> 
>    nvme: factor out cmb setup
>    nvme: factor out pmr setup
>    nvme: do cmb/pmr init as part of pci init
>        I think these could also carry Reviewed-by from Keith as well,
>        since the only change is also factoring out the PMR setup (which
>        was not there when Keith reviewed it) and the splitting into two
>        trivial patches per request from Philippe.

If respining a rebased v5 with all the previous tags added takes you 
<5min, I recommend you to do it, this will help Kevin. If you are 
comfortable with git-rebase and use git-publish, it can take you only 
2min :)

Looking forward for the next parts until the multiple namespace support!

Regards,

Phil.

> 
> 
> Thanks,
> Klaus
>