Re: [PATCH 1/2 v2] xen: Add support for initializing 16550 UART using ACPI

From: "Jan Beulich" <JBeulich@suse.com>
To: "Roger Pau Monné" <roger.pau@citrix.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>,
	Wei Liu <wei.liu2@citrix.com>,
	KonradRzeszutekWilk <konrad.wilk@oracle.com>,
	George Dunlap <George.Dunlap@eu.citrix.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Ian Jackson <ian.jackson@eu.citrix.com>, Tim Deegan <tim@xen.org>,
	Julien Grall <julien.grall@arm.com>,
	Bhupinder Thakur <bhupinder.thakur@linaro.org>,
	xen-devel@lists.xenproject.org
Subject: Re: [PATCH 1/2 v2] xen: Add support for initializing 16550 UART using ACPI
Date: Thu, 09 Nov 2017 08:26:19 -0700	[thread overview]
Message-ID: <5A0481AB020000780018DA51@prv-mh.provo.novell.com> (raw)
In-Reply-To: <20171109150715.64pr7avwb63cwctc@dhcp-3-128.uk.xensource.com>

>>> On 09.11.17 at 16:07, <roger.pau@citrix.com> wrote:
> On Thu, Nov 09, 2017 at 06:18:21AM -0700, Jan Beulich wrote:
>> >>> On 09.11.17 at 12:31, <roger.pau@citrix.com> wrote:
>> > On Thu, Nov 09, 2017 at 03:49:23PM +0530, Bhupinder Thakur wrote:
>> >> +static int ns16550_init_dt(struct ns16550 *uart,
>> >> +                           const struct dt_device_node *dev)
>> >> +{
>> >> +    return -EINVAL;
>> >> +}
>> >> +#endif
>> >> +
>> >> +#ifdef CONFIG_ACPI
>> >> +#include <xen/acpi.h>
>> > 
>> > Please place the include at the top of the file, together with the
>> > other ones.
>> 
>> I disagree here, at least as long as that header isn't itself expanding
>> to nothing (or only stubs) when !CONFIG_ACPI.
> 
> The header already has a CONFIG_ACPI guard inside, but it doesn't
> cover the full content, so my suggestion was to move the include to
> the top of the file, but keeping the CONFIG_ACPI guards around it.

Ah, I see. I'd then still prefer one less #ifdef by keeping it where it
is, but that's a matter of taste I admit.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel