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
