From: Bertrand Marquis <Bertrand.Marquis@arm.com>
To: Jan Beulich <jbeulich@suse.com>
Cc: Rahul Singh <Rahul.Singh@arm.com>,
Andrew Cooper <andrew.cooper3@citrix.com>,
George Dunlap <george.dunlap@citrix.com>,
Ian Jackson <iwj@xenproject.org>, Julien Grall <julien@xen.org>,
Stefano Stabellini <sstabellini@kernel.org>, Wei Liu <wl@xen.org>,
"xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>
Subject: Re: [PATCH v4 3/3] ns16550: Gate all PCI code with CONFIG_X86
Date: Fri, 27 Nov 2020 14:16:07 +0000 [thread overview]
Message-ID: <F1A3739A-D07C-429F-AC7B-47F7E2710377@arm.com> (raw)
In-Reply-To: <bacfe1c3-d86d-95b2-c52a-4bb86f1338ea@suse.com>
Hi Jan,
> On 27 Nov 2020, at 13:58, Jan Beulich <jbeulich@suse.com> wrote:
>
> On 25.11.2020 19:16, Rahul Singh wrote:
>> --- a/xen/drivers/char/ns16550.c
>> +++ b/xen/drivers/char/ns16550.c
>> @@ -16,7 +16,7 @@
>> #include <xen/timer.h>
>> #include <xen/serial.h>
>> #include <xen/iocap.h>
>> -#ifdef CONFIG_HAS_PCI
>> +#if defined(CONFIG_X86) && defined(CONFIG_HAS_PCI)
>> #include <xen/pci.h>
>> #include <xen/pci_regs.h>
>> #include <xen/pci_ids.h>
>> @@ -51,7 +51,7 @@ static struct ns16550 {
>> unsigned int timeout_ms;
>> bool_t intr_works;
>> bool_t dw_usr_bsy;
>> -#ifdef CONFIG_HAS_PCI
>> +#if defined(CONFIG_X86) && defined(CONFIG_HAS_PCI)
>
> I'm sorry to be picky, but this being a hack wants, imo, also calling
> it so, by way of a code comment. Clearly this should go at one of the
> first instances, yet neither of the two above are really suitable imo.
> Hence I'm coming back to my prior suggestion of introducing a
> consolidated #define without this becoming a Kconfig setting:
>
> /*
> * The PCI part of the code in this file currently is only known to
> * work on x86. Undo this hack once the logic has been suitably
> * abstracted.
> */
> #if defined(CONFIG_HAS_PCI) && defined(CONFIG_X86)
> # define NS16550_PCI
> #endif
>
> And then use NS16550_PCI everywhere. I'd be fine making this
> adjustment while committing, if I knew that (a) you're okay with it
> and (b) the R-b and A-b you've already got can be kept.
>
Sounds ok to me so you can keep my R-b if you go this way.
Cheers
Bertrand
> Jan
>
next prev parent reply other threads:[~2020-11-27 14:16 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-25 18:16 [PATCH v4 0/3] xen/arm: Make PCI passthrough code non-x86 specific Rahul Singh
2020-11-25 18:16 ` [PATCH v4 1/3] xen/pci: Move x86 specific code to x86 directory Rahul Singh
2020-11-25 21:16 ` Stefano Stabellini
2020-11-26 9:04 ` Bertrand Marquis
2020-11-27 13:34 ` Jan Beulich
2020-11-25 18:16 ` [PATCH v4 2/3] xen/pci: solve compilation error on ARM with HAS_PCI enabled Rahul Singh
2020-11-25 21:22 ` Stefano Stabellini
2020-11-26 9:05 ` Bertrand Marquis
2020-11-27 13:47 ` Jan Beulich
2020-11-25 18:16 ` [PATCH v4 3/3] ns16550: Gate all PCI code with CONFIG_X86 Rahul Singh
2020-11-25 18:21 ` Rahul Singh
2020-11-26 9:08 ` Bertrand Marquis
2020-11-25 21:28 ` Stefano Stabellini
2020-11-27 13:58 ` Jan Beulich
2020-11-27 14:16 ` Bertrand Marquis [this message]
2020-11-30 21:24 ` Stefano Stabellini
2020-11-27 14:25 ` Rahul Singh
2020-11-27 20:04 ` [PATCH v4 0/3] xen/arm: Make PCI passthrough code non-x86 specific Andrew Cooper
2020-11-27 20:09 ` Andrew Cooper
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=F1A3739A-D07C-429F-AC7B-47F7E2710377@arm.com \
--to=bertrand.marquis@arm.com \
--cc=Rahul.Singh@arm.com \
--cc=andrew.cooper3@citrix.com \
--cc=george.dunlap@citrix.com \
--cc=iwj@xenproject.org \
--cc=jbeulich@suse.com \
--cc=julien@xen.org \
--cc=sstabellini@kernel.org \
--cc=wl@xen.org \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).