[Xen-devel] [PATCH v2] xen/drivers/char: Don't require vpl011 for all non-x86 archs

Alistair Francis posted 1 patch 4 years, 10 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/xen tags/patchew/20190520171315.12026-1-alistair.francis@wdc.com
xen/drivers/char/console.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[Xen-devel] [PATCH v2] xen/drivers/char: Don't require vpl011 for all non-x86 archs
Posted by Alistair Francis 4 years, 10 months ago
Make the asm/vpl011.h dependent on the CONFIG_SBSA_VUART_CONSOLE define.

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 xen/drivers/char/console.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 9bbcb0f57a..24287e59cb 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -36,7 +36,7 @@
 #ifdef CONFIG_X86
 #include <xen/consoled.h>
 #include <asm/guest.h>
-#else
+#elif CONFIG_SBSA_VUART_CONSOLE
 #include <asm/vpl011.h>
 #endif
 
-- 
2.21.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] xen/drivers/char: Don't require vpl011 for all non-x86 archs
Posted by Julien Grall 4 years, 10 months ago
Hi Alistair,

On 20/05/2019 18:13, Alistair Francis wrote:
> Make the asm/vpl011.h dependent on the CONFIG_SBSA_VUART_CONSOLE define.

Your commit message wants a bit more explanation. In this case, you want to say 
the only user of this include is protected by CONFIG_SBSA_VUART_CONSOLE, hence 
it makes sense to protect it with the same define.

The title would need to be updated as well.

> 
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>   xen/drivers/char/console.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
> index 9bbcb0f57a..24287e59cb 100644
> --- a/xen/drivers/char/console.c
> +++ b/xen/drivers/char/console.c
> @@ -36,7 +36,7 @@
>   #ifdef CONFIG_X86
>   #include <xen/consoled.h>
>   #include <asm/guest.h>
> -#else
> +#elif CONFIG_SBSA_VUART_CONSOLE
>   #include <asm/vpl011.h>
>   #endif

This is a bit odds to require !CONFIG_X86 && CONFIG_SBSA_VUART_CONSOLE but the 
code is only protected with the second part.

How about:

#endif
#ifdef CONFIG_SBSA_VUART_CONSOLE
...
#endif

?

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] xen/drivers/char: Don't require vpl011 for all non-x86 archs
Posted by Jan Beulich 4 years, 10 months ago
>>> On 20.05.19 at 19:20, <julien.grall@arm.com> wrote:
>> --- a/xen/drivers/char/console.c
>> +++ b/xen/drivers/char/console.c
>> @@ -36,7 +36,7 @@
>>   #ifdef CONFIG_X86
>>   #include <xen/consoled.h>
>>   #include <asm/guest.h>
>> -#else
>> +#elif CONFIG_SBSA_VUART_CONSOLE
>>   #include <asm/vpl011.h>
>>   #endif
> 
> This is a bit odds to require !CONFIG_X86 && CONFIG_SBSA_VUART_CONSOLE but the 
> 
> code is only protected with the second part.
> 
> How about:
> 
> #endif
> #ifdef CONFIG_SBSA_VUART_CONSOLE
> ...
> #endif
> 
> ?

+1 - doing so will also save me from complaining about the missing
defined().

Jan



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