Messages in this thread | | | From | Arnd Bergmann <> | Date | Fri, 26 Feb 2021 19:06:43 +0100 | Subject | Re: [RFC 13/13] scsi: megaraid: Make use of dev_64bit_mmio_supported() |
| |
On Fri, Feb 26, 2021 at 3:30 PM Arnd Bergmann <arnd@kernel.org> wrote: > > On Fri, Feb 26, 2021 at 3:03 PM Nicolas Saenz Julienne > <nsaenzjulienne@suse.de> wrote: > > > unsigned long flags; > > - spin_lock_irqsave(&instance->hba_lock, flags); > > - writel(le32_to_cpu(req_desc->u.low), > > - &instance->reg_set->inbound_low_queue_port); > > - writel(le32_to_cpu(req_desc->u.high), > > - &instance->reg_set->inbound_high_queue_port); > > - spin_unlock_irqrestore(&instance->hba_lock, flags); > > > + > > + if (dev_64bit_mmio_supported(&instance->pdev->dev)) { > > + writeq(req_data, &instance->reg_set->inbound_low_queue_port); > > + } else { > > + spin_lock_irqsave(&instance->hba_lock, flags); > > + lo_hi_writeq(req_data, &instance->reg_set->inbound_low_queue_port); > > + spin_unlock_irqrestore(&instance->hba_lock, flags); > > + } > > I see your patch changes the code to the lo_hi_writeq() accessor, > and it also fixes the endianness bug (double byteswap on big-endian), > but it does not fix the spinlock bug (writel on pci leaks out of the lock > unless it's followed by a read).
On second look, it seems your patch breaks the byteorder logic, rather than fixing it. It would seem better to leave it unchanged then, or to send a separate rework of the endianness conversion if you think it is wrong.
Arnd
| |