Re: [PATCH v1 01/14] xen/pci: Refactor MSI code that implements MSI functionality within XEN

From: Jan Beulich <jbeulich@suse.com>
To: Rahul Singh <Rahul.Singh@arm.com>
Cc: "Bertrand Marquis" <Bertrand.Marquis@arm.com>,
	"Andrew Cooper" <andrew.cooper3@citrix.com>,
	"Roger Pau Monné" <roger.pau@citrix.com>, "Wei Liu" <wl@xen.org>,
	"George Dunlap" <george.dunlap@citrix.com>,
	"Ian Jackson" <iwj@xenproject.org>,
	"Julien Grall" <julien@xen.org>,
	"Stefano Stabellini" <sstabellini@kernel.org>,
	"Paul Durrant" <paul@xen.org>,
	"Daniel De Graaf" <dgdegra@tycho.nsa.gov>,
	"Daniel P. Smith" <dpsmith@apertussolutions.com>,
	"xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>
Subject: Re: [PATCH v1 01/14] xen/pci: Refactor MSI code that implements MSI functionality within XEN
Date: Tue, 31 Aug 2021 15:00:13 +0200	[thread overview]
Message-ID: <fd8334af-afc1-9207-3b17-b0f271a6bb36@suse.com> (raw)
In-Reply-To: <8A59CEFF-6910-43F8-9F4B-EC923ADA80F9@arm.com>

On 31.08.2021 14:31, Rahul Singh wrote:
>> On 24 Aug 2021, at 4:53 pm, Jan Beulich <jbeulich@suse.com> wrote:
>> On 19.08.2021 14:02, Rahul Singh wrote:
>>> --- /dev/null
>>> +++ b/xen/drivers/passthrough/msi.c
>>> @@ -0,0 +1,96 @@
>>> +/*
>>> + * Copyright (C) 2008,  Netronome Systems, Inc.
>>
>> While generally copying copyright statements when splitting source
>> files is probably wanted (or even necessary) I doubt this is
>> suitable here: None of the MSI code that you move was contributed
>> by them afaict.
> 
> Let me remove the "Copyright Copyright (C) 2008,  Netronome Systems, Inc.” . 
> Can you please help me what copyright I will add for the next patch ?

None? If you look around, you will find that far from all source files
have such a line (or multiple of them).

>>> + * This program is free software; you can redistribute it and/or modify it
>>> + * under the terms and conditions of the GNU General Public License,
>>> + * version 2, as published by the Free Software Foundation.
>>> + *
>>> + * This program is distributed in the hope it will be useful, but WITHOUT
>>> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
>>> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
>>> + * more details.
>>> + *
>>> + * You should have received a copy of the GNU General Public License along with
>>> + * this program; If not, see <http://www.gnu.org/licenses/>.
>>> + */
>>> +
>>> +#include <xen/init.h>
>>> +#include <xen/pci.h>
>>> +#include <asm/msi.h>
>>
>> You surely mean xen/msi.h here: Headers like this one should always
>> be included by the producer, no matter that it builds fine without.
>> Else you risk declarations and definitions to go out of sync.
> Ok . Let me include here “xen/msi.h” and move other required includes to “xen/msi.h"

Why move stuff? xen/msi.h is fins to include asm/msi.h. It's just that
including asm/msi.h here is not enough.

Jan