From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:38990)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <armbru@redhat.com>) id 1bpror-0005L2-9o
	for qemu-devel@nongnu.org; Fri, 30 Sep 2016 03:01:18 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <armbru@redhat.com>) id 1bprom-0004SN-3K
	for qemu-devel@nongnu.org; Fri, 30 Sep 2016 03:01:16 -0400
Received: from mx1.redhat.com ([209.132.183.28]:55528)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <armbru@redhat.com>) id 1bprol-0004SH-Tv
	for qemu-devel@nongnu.org; Fri, 30 Sep 2016 03:01:12 -0400
From: Markus Armbruster <armbru@redhat.com>
References: <1473839464-8670-1-git-send-email-caoj.fnst@cn.fujitsu.com>
	<1473839464-8670-7-git-send-email-caoj.fnst@cn.fujitsu.com>
	<87ponm7p0a.fsf@dusky.pond.sub.org> <57EE01A8.8010703@cn.fujitsu.com>
Date: Fri, 30 Sep 2016 09:01:08 +0200
In-Reply-To: <57EE01A8.8010703@cn.fujitsu.com> (Cao jin's message of "Fri, 30
	Sep 2016 14:09:44 +0800")
Message-ID: <87vaxdvpl7.fsf@dusky.pond.sub.org>
MIME-Version: 1.0
Content-Type: text/plain
Subject: Re: [Qemu-devel] [PATCH v3 6/8] megasas: remove unnecessary
 megasas_use_msix()
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel/>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Cao jin <caoj.fnst@cn.fujitsu.com>
Cc: Marcel Apfelbaum <marcel@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Hannes Reinecke <hare@suse.de>, qemu-devel@nongnu.org, "Michael S. Tsirkin" <mst@redhat.com>

Cao jin <caoj.fnst@cn.fujitsu.com> writes:

> On 09/29/2016 10:35 PM, Markus Armbruster wrote:
>> Cao jin <caoj.fnst@cn.fujitsu.com> writes:
>>
>
>>>
>>> @@ -2349,7 +2342,7 @@ static void megasas_scsi_realize(PCIDevice *dev, Error **errp)
>>>
>>>       memory_region_init_io(&s->mmio_io, OBJECT(s), &megasas_mmio_ops, s,
>>>                             "megasas-mmio", 0x4000);
>>> -    if (megasas_use_msix(s)) {
>>> +    if (s->msix != ON_OFF_AUTO_OFF) {
>>>           ret = msix_init(dev, 15, &s->mmio_io, b->mmio_bar, 0x2000,
>>>                           &s->mmio_io, b->mmio_bar, 0x3800, 0x68, &err);
>>>           /* Any error other than -ENOTSUP(board's MSI support is broken)
>>> @@ -2364,11 +2357,15 @@ static void megasas_scsi_realize(PCIDevice *dev, Error **errp)
>>>               error_propagate(errp, err);
>>>               return;
>>>           }
>>> -        assert(!err || d->msix == ON_OFF_AUTO_AUTO);
>>> +        assert(!err || s->msix == ON_OFF_AUTO_AUTO);
>>
>> You add this line in PATCH 4.  Could it use s->msix from the start?
>>
>
> It seems a copy&paste error...for the mistake.

Easy enough to fix :)

>>>           /* With msix=auto, we fall back to MSI off silently */
>>>           error_free(err);
>>>       }
>>>
>>> +    if (msix_enabled(dev)) {
>>> +        msix_vector_use(dev, 0);
>>> +    }
>>> +
>>>       memory_region_init_io(&s->port_io, OBJECT(s), &megasas_port_ops, s,
>>>                             "megasas-io", 256);
>>>       memory_region_init_io(&s->queue_io, OBJECT(s), &megasas_queue_ops, s,
>>> @@ -2384,10 +2381,6 @@ static void megasas_scsi_realize(PCIDevice *dev, Error **errp)
>>>       pci_register_bar(dev, b->mmio_bar, bar_type, &s->mmio_io);
>>>       pci_register_bar(dev, 3, bar_type, &s->queue_io);
>>>
>>> -    if (megasas_use_msix(s)) {
>>> -        msix_vector_use(dev, 0);
>>> -    }
>>> -
>>>       s->fw_state = MFI_FWSTATE_READY;
>>>       if (!s->sas_addr) {
>>>           s->sas_addr = ((NAA_LOCALLY_ASSIGNED_ID << 24) |
>>
>> Can you explain why you move the code?
>>
>
> Oh...just place the msix init related code together, for logical,
> nothing special.

Okay with me, but please mention it in the commit message.