linux-kernel - Re: [PATCH v3 3/4] ata: ahci_imx: Enlarge RX water mark for i.MX8QM SATA

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <daf86dec-9c35-49a5-ab81-ee667074f503@kernel.org>
Date: Thu, 18 Jul 2024 08:43:16 +0900
From: Damien Le Moal <dlemoal@...nel.org>
To: Niklas Cassel <cassel@...nel.org>, Richard Zhu <hongxing.zhu@....com>
Cc: tj@...nel.org, robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org,
 shawnguo@...nel.org, s.hauer@...gutronix.de, festevam@...il.com,
 linux-ide@...r.kernel.org, stable@...r.kernel.org,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org, imx@...ts.linux.dev,
 kernel@...gutronix.de
Subject: Re: [PATCH v3 3/4] ata: ahci_imx: Enlarge RX water mark for i.MX8QM
 SATA

On 7/18/24 3:17 AM, Niklas Cassel wrote:
> Hello Richard,
> 
> On Tue, Jul 16, 2024 at 11:18:14AM +0800, Richard Zhu wrote:
>> The RXWM(RxWaterMark) sets the minimum number of free location within
>> the RX FIFO before the watermark is exceeded which in turn will cause
>> the Transport Layer to instruct the Link Layer to transmit HOLDS to
>> the transmitting end.
>>
>> Based on the default RXWM value 0x20, RX FIFO overflow might be
>> observed on i.MX8QM MEK board, when some Gen3 SATA disks are used.
>>
>> The FIFO overflow will result in CRC error, internal error and protocol
>> error, then the SATA link is not stable anymore.
>>
>> To fix this issue, enlarge RX water mark setting from 0x20 to 0x29.
>>
>> Fixes: 027fa4dee935 ("ahci: imx: add the imx8qm ahci sata support")
>> Cc: stable@...r.kernel.org
>> Signed-off-by: Richard Zhu <hongxing.zhu@....com>
>> ---
> 
> Looking at the title of this patch:
> "ahci_imx: Enlarge RX water mark for i.MX8QM SATA"
> 
> This suggests that this fix is only needed for i.MX8QM.
> 
> Support for i.MX8QM was added to the device tree binding in patch 1/4 in
> this series.
> 
> Doing a git grep in linux-next gives the following result:
> 
> $ git grep fsl,imx8qm-ahci linux-next/master
> linux-next/master:drivers/ata/ahci_imx.c:       { .compatible = "fsl,imx8qm-ahci", .data = (void *)AHCI_IMX8QM },
> 
> 
> This is interesting for two reasons:
> 1) drivers/ata/ahci_imx.c already has support for this compatible string,
> even though this compatible string does not exist in any DT binding
> (in linux-next).
> 
> 2) There is not a single in-tree device tree (DTS) that uses this compatible
> string ....and we do not care about out of tree device trees.
> 
> 
> Considering 2) I do NOT think that we should have
> Cc: stable@...r.kernel.org on this... we shouldn't just backport random driver
> fixes is there are no in-tree users of this compatible string.
> 
> So I suggest that:
> -Drop the CC: stable.
> -I actually think that it is better that you drop the Fixes tag too, because if
> you keep it, the stable bots will automatically select this for backporting,
> and then we will need to reply and say that this should not be backported, so
> better to avoid adding the Fixes tag in the first place.
> (Since there are no users of this compatible string, there is nothing that is
> broken, so there is nothing to fix.)
> 
> 
> Damien, when applying this patch, I suggest that we apply it to for-6.12
> together with the rest of the series (instead of applying it to
> for-6.11-fixes).

It was me who asked for the Fixes and Cc-stable tags, but I had not checked
that the compatible is not being used in any DT. So good catch.
I will apply everything to for-6.12.

> 
> 
> Kind regards,
> Niklas

-- 
Damien Le Moal
Western Digital Research