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

[Niklas Cassel <cassel@kernel.org>]

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@vger.kernel.org
> Signed-off-by: Richard Zhu <hongxing.zhu@nxp.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@vger.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).


Kind regards,
Niklas