ns16550: Properly gate Exar PCIe UART cards support

[PATCH] ns16550: Properly gate Exar PCIe UART cards support

Posted by Oleksandr Andrushchenko 4 years, 5 months ago

From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>

This fixes Arm build which doesn't have ns16550 PCI support.

ns16550.c:313:5: error: implicit declaration of function 'enable_exar_enhanced_bits' [-Werror=implicit-function-declaration]
  313 |     enable_exar_enhanced_bits(uart);
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~

Fixes: 5ffd37db2ff6 ("ns16550: add Exar PCIe UART cards support")

Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
---
 xen/drivers/char/ns16550.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
index b777c8711ee0..e2c24082c368 100644
--- a/xen/drivers/char/ns16550.c
+++ b/xen/drivers/char/ns16550.c
@@ -308,7 +308,7 @@ static void ns16550_setup_preirq(struct ns16550 *uart)
     /* Handle the DesignWare 8250 'busy-detect' quirk. */
     handle_dw_usr_busy_quirk(uart);
 
-#ifdef CONFIG_HAS_PCI
+#ifdef NS16550_PCI
     /* Enable Exar "Enhanced function bits" */
     enable_exar_enhanced_bits(uart);
 #endif
-- 
2.25.1

Re: [PATCH] ns16550: Properly gate Exar PCIe UART cards support

Posted by Jan Beulich 4 years, 5 months ago

On 20.08.2021 13:54, Oleksandr Andrushchenko wrote:
> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
> 
> This fixes Arm build which doesn't have ns16550 PCI support.
> 
> ns16550.c:313:5: error: implicit declaration of function 'enable_exar_enhanced_bits' [-Werror=implicit-function-declaration]
>   313 |     enable_exar_enhanced_bits(uart);
>       |     ^~~~~~~~~~~~~~~~~~~~~~~~~

This can't be the full story - both Arm32 and Arm64 build fine for me.
In fact I can't find any "select HAS_PCI" outside of x86'es subtree.

> Fixes: 5ffd37db2ff6 ("ns16550: add Exar PCIe UART cards support")

IOW this tag is wrong, no matter that I agree that ...

> --- a/xen/drivers/char/ns16550.c
> +++ b/xen/drivers/char/ns16550.c
> @@ -308,7 +308,7 @@ static void ns16550_setup_preirq(struct ns16550 *uart)
>      /* Handle the DesignWare 8250 'busy-detect' quirk. */
>      handle_dw_usr_busy_quirk(uart);
>  
> -#ifdef CONFIG_HAS_PCI
> +#ifdef NS16550_PCI
>      /* Enable Exar "Enhanced function bits" */
>      enable_exar_enhanced_bits(uart);
>  #endif

... this change is wanted, but just for consistency for now. If you
can supply an improved / accurate description, I'll be happy to commit
this with
Reviewed-by: Jan Beulich <jbeulich@suse.com>

As an aside - please follow patch submission guidelines: Patches go
To the list, with maintainers (and perhaps other relevant folks) on Cc.

Jan

Re: [PATCH] ns16550: Properly gate Exar PCIe UART cards support

Posted by Oleksandr Andrushchenko 4 years, 5 months ago

On 20.08.21 15:07, Jan Beulich wrote:
> On 20.08.2021 13:54, Oleksandr Andrushchenko wrote:
>> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
>>
>> This fixes Arm build which doesn't have ns16550 PCI support.
>>
>> ns16550.c:313:5: error: implicit declaration of function 'enable_exar_enhanced_bits' [-Werror=implicit-function-declaration]
>>    313 |     enable_exar_enhanced_bits(uart);
>>        |     ^~~~~~~~~~~~~~~~~~~~~~~~~
> This can't be the full story - both Arm32 and Arm64 build fine for me.
> In fact I can't find any "select HAS_PCI" outside of x86'es subtree.
>
>> Fixes: 5ffd37db2ff6 ("ns16550: add Exar PCIe UART cards support")
> IOW this tag is wrong, no matter that I agree that ...

Ok, the full story is that I am building this with PCI passthrough support on Arm,

so yes, you are obviously correct here and "Fixes" tag does not apply.

I will remove it.

>
>> --- a/xen/drivers/char/ns16550.c
>> +++ b/xen/drivers/char/ns16550.c
>> @@ -308,7 +308,7 @@ static void ns16550_setup_preirq(struct ns16550 *uart)
>>       /* Handle the DesignWare 8250 'busy-detect' quirk. */
>>       handle_dw_usr_busy_quirk(uart);
>>   
>> -#ifdef CONFIG_HAS_PCI
>> +#ifdef NS16550_PCI
>>       /* Enable Exar "Enhanced function bits" */
>>       enable_exar_enhanced_bits(uart);
>>   #endif
> ... this change is wanted, but just for consistency for now. If you
> can supply an improved / accurate description, I'll be happy to commit
> this with
> Reviewed-by: Jan Beulich <jbeulich@suse.com>

I can put the following description:

     ns16550: Properly gate Exar PCIe UART cards support

     Arm is about to get PCI passthrough support which means CONFIG_HAS_PCI
     will be enabled, so this code will fail as Arm doesn't have ns16550
     PCI support:

     ns16550.c:313:5: error: implicit declaration of function 'enable_exar_enhanced_bits' [-Werror=implicit-function-declaration]
       313 |     enable_exar_enhanced_bits(uart);
           |     ^~~~~~~~~~~~~~~~~~~~~~~~~

     Fix this by gating Exar PCIe UART cards support with the above in mind.

Will this be ok?

Can I keep your rb tag with this description?

>
> As an aside - please follow patch submission guidelines: Patches go
> To the list, with maintainers (and perhaps other relevant folks) on Cc.
Sure, sorry about that
>
> Jan
>
Thank you,

Oleksandr

Re: [PATCH] ns16550: Properly gate Exar PCIe UART cards support

Posted by Jan Beulich 4 years, 5 months ago

On 20.08.2021 14:18, Oleksandr Andrushchenko wrote:
> 
> On 20.08.21 15:07, Jan Beulich wrote:
>> On 20.08.2021 13:54, Oleksandr Andrushchenko wrote:
>>> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
>>>
>>> This fixes Arm build which doesn't have ns16550 PCI support.
>>>
>>> ns16550.c:313:5: error: implicit declaration of function 'enable_exar_enhanced_bits' [-Werror=implicit-function-declaration]
>>>    313 |     enable_exar_enhanced_bits(uart);
>>>        |     ^~~~~~~~~~~~~~~~~~~~~~~~~
>> This can't be the full story - both Arm32 and Arm64 build fine for me.
>> In fact I can't find any "select HAS_PCI" outside of x86'es subtree.
>>
>>> Fixes: 5ffd37db2ff6 ("ns16550: add Exar PCIe UART cards support")
>> IOW this tag is wrong, no matter that I agree that ...
> 
> Ok, the full story is that I am building this with PCI passthrough support on Arm,
> 
> so yes, you are obviously correct here and "Fixes" tag does not apply.
> 
> I will remove it.
> 
>>
>>> --- a/xen/drivers/char/ns16550.c
>>> +++ b/xen/drivers/char/ns16550.c
>>> @@ -308,7 +308,7 @@ static void ns16550_setup_preirq(struct ns16550 *uart)
>>>       /* Handle the DesignWare 8250 'busy-detect' quirk. */
>>>       handle_dw_usr_busy_quirk(uart);
>>>   
>>> -#ifdef CONFIG_HAS_PCI
>>> +#ifdef NS16550_PCI
>>>       /* Enable Exar "Enhanced function bits" */
>>>       enable_exar_enhanced_bits(uart);
>>>   #endif
>> ... this change is wanted, but just for consistency for now. If you
>> can supply an improved / accurate description, I'll be happy to commit
>> this with
>> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> 
> I can put the following description:
> 
>      ns16550: Properly gate Exar PCIe UART cards support
> 
>      Arm is about to get PCI passthrough support which means CONFIG_HAS_PCI
>      will be enabled, so this code will fail as Arm doesn't have ns16550
>      PCI support:
> 
>      ns16550.c:313:5: error: implicit declaration of function 'enable_exar_enhanced_bits' [-Werror=implicit-function-declaration]
>        313 |     enable_exar_enhanced_bits(uart);
>            |     ^~~~~~~~~~~~~~~~~~~~~~~~~
> 
>      Fix this by gating Exar PCIe UART cards support with the above in mind.
> 
> Will this be ok?
> 
> Can I keep your rb tag with this description?

No need to resubmit - I've replaced the original description and
committed the result.

Jan