INTERRUPT_TYPE_NONE should be 0, as it is assumed in
xen_pcibk_get_interrupt_type(). Fix the definition, and also shift other
values to not leave holes.
But also use INTERRUPT_TYPE_NONE in xen_pcibk_get_interrupt_type() to
avoid similar confusions in the future.

Fixes: 476878e4b2be ("xen-pciback: optionally allow interrupt enable flag writes")
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
 drivers/xen/xen-pciback/conf_space.c | 2 +-
 drivers/xen/xen-pciback/conf_space.h | 8 ++++----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/xen/xen-pciback/conf_space.c b/drivers/xen/xen-pciback/conf_space.c
index b20e43e148ce..b4e4ec9cd496 100644
--- a/drivers/xen/xen-pciback/conf_space.c
+++ b/drivers/xen/xen-pciback/conf_space.c
@@ -290,7 +290,7 @@ int xen_pcibk_get_interrupt_type(struct pci_dev *dev)
 {
 	int err;
 	u16 val;
-	int ret = 0;
+	int ret = INTERRUPT_TYPE_NONE;
 
 	err = pci_read_config_word(dev, PCI_COMMAND, &val);
 	if (err)
diff --git a/drivers/xen/xen-pciback/conf_space.h b/drivers/xen/xen-pciback/conf_space.h
index 28c45180a12e..5fe431c79f25 100644
--- a/drivers/xen/xen-pciback/conf_space.h
+++ b/drivers/xen/xen-pciback/conf_space.h
@@ -65,10 +65,10 @@ struct config_field_entry {
 	void *data;
 };
 
-#define INTERRUPT_TYPE_NONE (1<<0)
-#define INTERRUPT_TYPE_INTX (1<<1)
-#define INTERRUPT_TYPE_MSI  (1<<2)
-#define INTERRUPT_TYPE_MSIX (1<<3)
+#define INTERRUPT_TYPE_NONE (0)
+#define INTERRUPT_TYPE_INTX (1<<0)
+#define INTERRUPT_TYPE_MSI  (1<<1)
+#define INTERRUPT_TYPE_MSIX (1<<2)
 
 extern bool xen_pcibk_permissive;
 
-- 
2.21.0


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

On 3/19/20 12:06 AM, Marek Marczykowski-Górecki wrote:
> INTERRUPT_TYPE_NONE should be 0,


Would

   return ret ?: INTERRUPT_TYPE_NONE

in xen_pcibk_get_interrupt_type() work?


I think it's better not to tie macro name to a particular value.


-boris


>   as it is assumed in
> xen_pcibk_get_interrupt_type(). Fix the definition, and also shift other
> values to not leave holes.
> But also use INTERRUPT_TYPE_NONE in xen_pcibk_get_interrupt_type() to
> avoid similar confusions in the future.
>
> Fixes: 476878e4b2be ("xen-pciback: optionally allow interrupt enable flag writes")
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
>
>   

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

On Thu, Mar 19, 2020 at 11:07:13AM -0400, Boris Ostrovsky wrote:
> 
> On 3/19/20 12:06 AM, Marek Marczykowski-Górecki wrote:
> > INTERRUPT_TYPE_NONE should be 0,
> 
> 
> Would
> 
>   return ret ?: INTERRUPT_TYPE_NONE
> 
> in xen_pcibk_get_interrupt_type() work?
> 
> 
> I think it's better not to tie macro name to a particular value.

I can do that too. But I'd change INTERRUPT_TYPE_NONE to 0 anyway, as
more logical value (as the value is a bitmask).

> -boris
> 
> 
> >   as it is assumed in
> > xen_pcibk_get_interrupt_type(). Fix the definition, and also shift other
> > values to not leave holes.
> > But also use INTERRUPT_TYPE_NONE in xen_pcibk_get_interrupt_type() to
> > avoid similar confusions in the future.
> > 
> > Fixes: 476878e4b2be ("xen-pciback: optionally allow interrupt enable flag writes")
> > Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> > 

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel