hw/block/nvme.c | 433 ++++++++++++++++++++++++------------------ hw/block/nvme.h | 36 +++- hw/block/trace-events | 172 ++++++++--------- include/block/nvme.h | 8 + 4 files changed, 372 insertions(+), 277 deletions(-)
From: Klaus Jensen <k.jensen@samsung.com>
Changes since v1
~~~~~~~~~~~~~~~~
* nvme: fix pci doorbell size calculation
- added some defines and a better comment (Philippe)
* nvme: rename trace events to pci_nvme
- changed the prefix from nvme_dev to pci_nvme (Philippe)
* nvme: add max_ioqpairs device parameter
- added a deprecation comment. I doubt this will go in until 5.1, so
changed it to "deprecated from 5.1" (Philippe)
* nvme: factor out property/constraint checks
* nvme: factor out block backend setup
- changed to return void and propagate errors in proper QEMU style
(Philippe)
* nvme: add namespace helpers
- use the helper immediately (Philippe)
* nvme: factor out pci setup
- removed setting of vendor and device id which is already inherited
from nvme_class_init() (Philippe)
* nvme: factor out cmb setup
- add lost comment (Philippe)
Klaus Jensen (16):
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 controller identify setup
hw/block/nvme.c | 433 ++++++++++++++++++++++++------------------
hw/block/nvme.h | 36 +++-
hw/block/trace-events | 172 ++++++++---------
include/block/nvme.h | 8 +
4 files changed, 372 insertions(+), 277 deletions(-)
--
2.26.0
Patchew URL: https://patchew.org/QEMU/20200415130159.611361-1-its@irrelevant.dk/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Subject: [PATCH v2 00/16] nvme: refactoring and cleanups
Message-id: 20200415130159.611361-1-its@irrelevant.dk
Type: series
=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
a0e808d nvme: factor out controller identify setup
ea2d295 nvme: factor out cmb setup
c3c1e51 nvme: factor out pci setup
b4b6c55 nvme: factor out namespace setup
79229ae nvme: add namespace helpers
c7d0f2b nvme: factor out block backend setup
0802218 nvme: factor out device state setup
b407ffa nvme: factor out property/constraint checks
a7b4ac0 nvme: remove redundant cmbloc/cmbsz members
bf8d4cc nvme: add max_ioqpairs device parameter
eef1607 nvme: refactor nvme_addr_read
c063b77 nvme: use constants in identify
9c1ad75 nvme: move device parameters to separate struct
72124cf nvme: remove superfluous breaks
8cbaf9e nvme: rename trace events to pci_nvme
826b0ca nvme: fix pci doorbell size calculation
=== OUTPUT BEGIN ===
1/16 Checking commit 826b0caf1bed (nvme: fix pci doorbell size calculation)
2/16 Checking commit 8cbaf9e98e00 (nvme: rename trace events to pci_nvme)
3/16 Checking commit 72124cfc8e35 (nvme: remove superfluous breaks)
4/16 Checking commit 9c1ad75817e7 (nvme: move device parameters to separate struct)
ERROR: Macros with complex values should be enclosed in parenthesis
#182: FILE: hw/block/nvme.h:6:
+#define DEFINE_NVME_PROPERTIES(_state, _props) \
+ DEFINE_PROP_STRING("serial", _state, _props.serial), \
+ DEFINE_PROP_UINT32("cmb_size_mb", _state, _props.cmb_size_mb, 0), \
+ DEFINE_PROP_UINT32("num_queues", _state, _props.num_queues, 64)
total: 1 errors, 0 warnings, 182 lines checked
Patch 4/16 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
5/16 Checking commit c063b77e218e (nvme: use constants in identify)
6/16 Checking commit eef16074638e (nvme: refactor nvme_addr_read)
7/16 Checking commit bf8d4cc67458 (nvme: add max_ioqpairs device parameter)
8/16 Checking commit a7b4ac0a9cbe (nvme: remove redundant cmbloc/cmbsz members)
9/16 Checking commit b407ffae89f8 (nvme: factor out property/constraint checks)
10/16 Checking commit 0802218ca18b (nvme: factor out device state setup)
11/16 Checking commit c7d0f2b17c08 (nvme: factor out block backend setup)
12/16 Checking commit 79229aef5988 (nvme: add namespace helpers)
13/16 Checking commit b4b6c55cd5af (nvme: factor out namespace setup)
14/16 Checking commit c3c1e5121db6 (nvme: factor out pci setup)
15/16 Checking commit ea2d29507c1e (nvme: factor out cmb setup)
16/16 Checking commit a0e808d2fca5 (nvme: factor out controller identify setup)
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20200415130159.611361-1-its@irrelevant.dk/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
On Wed, 2020-04-15 at 15:01 +0200, Klaus Jensen wrote: > From: Klaus Jensen <k.jensen@samsung.com> > > Changes since v1 > ~~~~~~~~~~~~~~~~ > * nvme: fix pci doorbell size calculation > - added some defines and a better comment (Philippe) > > * nvme: rename trace events to pci_nvme > - changed the prefix from nvme_dev to pci_nvme (Philippe) > > * nvme: add max_ioqpairs device parameter > - added a deprecation comment. I doubt this will go in until 5.1, so > changed it to "deprecated from 5.1" (Philippe) > > * nvme: factor out property/constraint checks > * nvme: factor out block backend setup > - changed to return void and propagate errors in proper QEMU style > (Philippe) > > * nvme: add namespace helpers > - use the helper immediately (Philippe) > > * nvme: factor out pci setup > - removed setting of vendor and device id which is already inherited > from nvme_class_init() (Philippe) > > * nvme: factor out cmb setup > - add lost comment (Philippe) > > > Klaus Jensen (16): > 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 controller identify setup > > hw/block/nvme.c | 433 ++++++++++++++++++++++++------------------ > hw/block/nvme.h | 36 +++- > hw/block/trace-events | 172 ++++++++--------- > include/block/nvme.h | 8 + > 4 files changed, 372 insertions(+), 277 deletions(-) > Should I also review the V7 series or I should wait for V8 which will not include these cleanups? Best regards, Maxim Levitsky
On Apr 21 19:24, Maxim Levitsky wrote: > Should I also review the V7 series or I should wait for V8 which will > not include these cleanups? Hi Maxim, Just wait for another series - I don't think I will post a v8, I will chop op the series into smaller ones instead. Most patches will hopefully not change too much, so should keep your Reviewed-by's ;) Thanks, Klaus
The series looks good to me. Reviewed-by: Keith Busch <kbusch@kernel.org>
On Apr 21 02:38, Keith Busch wrote: > The series looks good to me. > > Reviewed-by: Keith Busch <kbusch@kernel.org> Thanks for the review Keith! Kevin, should I rebase this on block-next? I think it might have some conflicts with the PMR patch that went in previously. Philippe, then I can also change the *err to *local_err ;)
Am 21.04.2020 um 08:38 hat Klaus Birkelund Jensen geschrieben: > On Apr 21 02:38, Keith Busch wrote: > > The series looks good to me. > > > > Reviewed-by: Keith Busch <kbusch@kernel.org> > > Thanks for the review Keith! > > Kevin, should I rebase this on block-next? I think it might have some > conflicts with the PMR patch that went in previously. The series doesn't apply at the moment anyway, I assume it's because of the PMR patch. So yes, I would appreciate a rebase. Kevin
On Apr 15 15:01, Klaus Jensen wrote: > From: Klaus Jensen <k.jensen@samsung.com> > > Changes since v1 > ~~~~~~~~~~~~~~~~ > * nvme: fix pci doorbell size calculation > - added some defines and a better comment (Philippe) > > * nvme: rename trace events to pci_nvme > - changed the prefix from nvme_dev to pci_nvme (Philippe) > > * nvme: add max_ioqpairs device parameter > - added a deprecation comment. I doubt this will go in until 5.1, so > changed it to "deprecated from 5.1" (Philippe) > > * nvme: factor out property/constraint checks > * nvme: factor out block backend setup > - changed to return void and propagate errors in proper QEMU style > (Philippe) > > * nvme: add namespace helpers > - use the helper immediately (Philippe) > > * nvme: factor out pci setup > - removed setting of vendor and device id which is already inherited > from nvme_class_init() (Philippe) > > * nvme: factor out cmb setup > - add lost comment (Philippe) > > > Klaus Jensen (16): > 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 controller identify setup > > hw/block/nvme.c | 433 ++++++++++++++++++++++++------------------ > hw/block/nvme.h | 36 +++- > hw/block/trace-events | 172 ++++++++--------- > include/block/nvme.h | 8 + > 4 files changed, 372 insertions(+), 277 deletions(-) > > -- > 2.26.0 > Hi Keith, You have acked most of this previously, but not in it's most recent state. Since a good bunch of the refactoring patches have been split up and changed, only a small subset of the patches still carry your Acked-by. The 'nvme: fix pci doorbell size calculation' and 'nvme: add max_ioqpairs device parameter' are new since your ack and given their nature a review from you would be nice :) Thanks, Klaus
© 2016 - 2024 Red Hat, Inc.