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

[Artyom Tarasenko]

On Mon, Jan 15, 2018 at 7:38 PM, Mark Cave-Ayland
<address@hidden> wrote:
> 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.
>

The whole series does look good to me. The new naming conventions are
really a lot cleaner than what we used to have. Well done!

Acked-by: Artyom Tarasenko <address@hidden>


-- 
Regards,
Artyom Tarasenko

SPARC and PPC PReP under qemu blog: http://tyom.blogspot.com/search/label/qemu