Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH v3 0/1] libata: Add ATA_HORKAGE_NONCQ_ON_AMD for Samsung
 860 and 870 SSD.
To:     Damien Le Moal <Damien.LeMoal@wdc.com>,
        Kate Hsuan <hpa@redhat.com>, Jens Axboe <axboe@kernel.dk>
Cc:     "linux-ide@vger.kernel.org" <linux-ide@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
References: <20210830144253.289542-1-hpa@redhat.com>
 <DM6PR04MB708147357F91D5AEB6128A2AE7CB9@DM6PR04MB7081.namprd04.prod.outlook.com>
From:   Hans de Goede <hdegoede@redhat.com>
Message-ID: <a76b2acd-edfa-55b2-907c-f1faa8ce4914@redhat.com>
Date:   Tue, 31 Aug 2021 09:38:04 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.11.0
MIME-Version: 1.0
In-Reply-To: <DM6PR04MB708147357F91D5AEB6128A2AE7CB9@DM6PR04MB7081.namprd04.prod.outlook.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

Hi,

On 8/30/21 11:45 PM, Damien Le Moal wrote:
> On 2021/08/30 23:43, Kate Hsuan wrote:
>> Many users reported the issue when running the system with Samsung 860,
>> 870 SSD, and AMD chipset. Therefore, completely disabling the NCQ can
>> avoid this issue.
>>
>> Entire disabling NCQ for Samsung 860/870 SSD will cause I/O performance
>> drop. In this case, a flag ATA_HORKAGE_NONCQ_ON_AMD is introduced to
>> used to perform an additional check for these SSDs. If it finds its parent
>> ATA controller is AMD, the NCQ will be disabled. Otherwise, the NCQ is kept
>> to enable.
> 
> For a single patch, generally, a cover letter is not needed. Especially so in
> this case since your cover letter message is the same as the patch commit message.
> 
>>
>> Changes since v3
>> * Modified the flag from ATA_HORKAGE_NONCQ_ON_ASMEDIA_AMD_MARVELL to
>>   ATA_HORKAGE_NONCQ_ON_AMD.
>> * Modified and fixed the code to completely disable NCQ on AMD controller.
> 
> Technically, this is a v2 right ? Also, by "completely", did you mean "always" ?
> (see patch comments).

Right, as mentioned in my reply to the v1 which was accidentally labelled v2
I suggested to just make this v3 to avoid confusion, otherwise we would have
2 v2-s.

Regards,

Hans