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@xxxxxxxxxxxxxxx >> Signed-off-by: Richard Zhu <hongxing.zhu@xxxxxxx> >> --- > > 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@xxxxxxxxxxxxxxx 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