| Message ID | 20200319040648.10396-1-marmarek@invisiblethingslab.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xen-pciback: fix INTERRUPT_TYPE_* defines | expand |
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> > >
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> > >
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;
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(-)