Re: [Qemu-devel] [PATCH 00/11] sun4u: APB tidy-up/rename and tracepoint conversions

[Mark Cave-Ayland]

On 14/01/18 13:25, Philippe Mathieu-Daudé wrote:

> On 01/14/2018 08:21 AM, Mark Cave-Ayland wrote:
> > On 14/01/18 11:15, address@hidden wrote:
> > > Hi,
> > >
> > > This series seems to have some coding style problems. See output below
> > > for
> > > more information:
> > >
> > > Type: series
> > > Message-id: address@hidden
> > > Subject: [Qemu-devel] [PATCH 00/11] sun4u: APB tidy-up/rename and
> > > tracepoint conversions
> >
> > (lots cut)
> >
> > > === OUTPUT BEGIN ===
> > > Checking PATCH 1/11: apb: split simba PCI bridge into
> > > hw/pci-bridge/simba.c...
> > > Checking PATCH 2/11: simba: rename PBMPCIBridge and QOM types to
> > > reflect simba naming...
> > > Checking PATCH 3/11: apb: rename APB functions to use sabre prefix...
> > > Checking PATCH 4/11: apb: change pbm_pci_host prefix functions to use
> > > sabre_pci prefix...
> > > Checking PATCH 5/11: apb: QOMify sabre PCI host bridge...
> > > Checking PATCH 6/11: apb: rename QOM type from TYPE_APB to TYPE_SABRE...
> > > Checking PATCH 7/11: sun4u: rename apb variables and constants...
> > > Checking PATCH 8/11: apb: rename apb.c to sabre.c...
> > > ERROR: do not use C99 // comments
> > > #86: FILE: hw/pci-host/sabre.c:41:
> > > +//#define DEBUG_SABRE
> > >
> > > total: 1 errors, 0 warnings, 188 lines checked
> > >
> > > Your patch has style problems, please review.  If any of these errors
> > > are false positives report them to the maintainer, see
> > > CHECKPATCH in MAINTAINERS.
> > >
> > > Checking PATCH 9/11: pci: add trace-events support for hw/pci-host...
> > > Checking PATCH 10/11: sabre: convert from SABRE_DPRINTF macro to
> > > trace-events...
> > > Checking PATCH 11/11: sparc64: convert hw/sparc64/sparc64.c from
> > > DPRINTF macros to trace events...
> > > === OUTPUT END ===
> > >
> > > Test command exited with code: 1
> >
> > This is fine - it's just a side-effect of renaming DEBUG_APB to
> > DEBUG_SABRE as part of the APB to sabre rename, and in fact this code is
> > completely removed in patch 10 with the conversion to tracepoints.
>
> This can be avoided moving patch #8 after #10, although not worthy IMHO.

Thanks once again for the review. It was quite a tricky patchset to 
order in the end because of the rename, and there wasn't really one 
ordering which I found completely satisfactory.

Given that you're generally happy with the patches, I'm inclined to keep 
the patchset in its current form, pending any further comments from Artyom.


ATB,

Mark.