| Message ID | 1722581213-15221-5-git-send-email-hongxing.zhu@nxp.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Refine i.MX8QM SATA based on generic PHY callbacks | expand |
On Fri, Aug 02, 2024 at 02:46:52PM +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. > > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com> This is bug fix. need fix tag and cc stable Frank > --- > drivers/ata/ahci_imx.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/drivers/ata/ahci_imx.c b/drivers/ata/ahci_imx.c > index 4dd98368f8562..627b36cc4b5c1 100644 > --- a/drivers/ata/ahci_imx.c > +++ b/drivers/ata/ahci_imx.c > @@ -45,6 +45,10 @@ enum { > /* Clock Reset Register */ > IMX_CLOCK_RESET = 0x7f3f, > IMX_CLOCK_RESET_RESET = 1 << 0, > + /* IMX8QM SATA specific control registers */ > + IMX8QM_SATA_AHCI_PTC = 0xc8, > + IMX8QM_SATA_AHCI_PTC_RXWM_MASK = GENMASK(6, 0), > + IMX8QM_SATA_AHCI_PTC_RXWM = 0x29, > }; > > enum ahci_imx_type { > @@ -466,6 +470,12 @@ static int imx8_sata_enable(struct ahci_host_priv *hpriv) > phy_power_off(imxpriv->cali_phy0); > phy_exit(imxpriv->cali_phy0); > > + /* RxWaterMark setting */ > + val = readl(hpriv->mmio + IMX8QM_SATA_AHCI_PTC); > + val &= ~IMX8QM_SATA_AHCI_PTC_RXWM_MASK; > + val |= IMX8QM_SATA_AHCI_PTC_RXWM; > + writel(val, hpriv->mmio + IMX8QM_SATA_AHCI_PTC); > + > return 0; > > err_sata_phy_exit: > -- > 2.37.1 >
> -----Original Message----- > From: Frank Li <frank.li@nxp.com> > Sent: 2024年8月3日 4:46 > To: Hongxing Zhu <hongxing.zhu@nxp.com> > Cc: robh@kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org; > shawnguo@kernel.org; l.stach@pengutronix.de; devicetree@vger.kernel.org; > linux-pci@vger.kernel.org; linux-arm-kernel@lists.infradead.org; > linux-kernel@vger.kernel.org; kernel@pengutronix.de; imx@lists.linux.dev > Subject: Re: [PATCH v5 4/5] ata: ahci_imx: Enlarge RX water mark for i.MX8QM > SATA > Importance: High > > On Fri, Aug 02, 2024 at 02:46:52PM +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. > > > > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com> > > This is bug fix. need fix tag and cc stable Hi Frank: Thanks for your comments. Niklas suggested not add cc stable and so on. Here is the discussion. https://patchwork.kernel.org/project/linux-arm-kernel/patch/1721099895-26098-4-git-send-email-hongxing.zhu@nxp.com/ So, I removed them from v4 series. Best Regards Richard Zhu > > Frank > > --- > > drivers/ata/ahci_imx.c | 10 ++++++++++ > > 1 file changed, 10 insertions(+) > > > > diff --git a/drivers/ata/ahci_imx.c b/drivers/ata/ahci_imx.c index > > 4dd98368f8562..627b36cc4b5c1 100644 > > --- a/drivers/ata/ahci_imx.c > > +++ b/drivers/ata/ahci_imx.c > > @@ -45,6 +45,10 @@ enum { > > /* Clock Reset Register */ > > IMX_CLOCK_RESET = 0x7f3f, > > IMX_CLOCK_RESET_RESET = 1 << 0, > > + /* IMX8QM SATA specific control registers */ > > + IMX8QM_SATA_AHCI_PTC = 0xc8, > > + IMX8QM_SATA_AHCI_PTC_RXWM_MASK = GENMASK(6, 0), > > + IMX8QM_SATA_AHCI_PTC_RXWM = 0x29, > > }; > > > > enum ahci_imx_type { > > @@ -466,6 +470,12 @@ static int imx8_sata_enable(struct ahci_host_priv > *hpriv) > > phy_power_off(imxpriv->cali_phy0); > > phy_exit(imxpriv->cali_phy0); > > > > + /* RxWaterMark setting */ > > + val = readl(hpriv->mmio + IMX8QM_SATA_AHCI_PTC); > > + val &= ~IMX8QM_SATA_AHCI_PTC_RXWM_MASK; > > + val |= IMX8QM_SATA_AHCI_PTC_RXWM; > > + writel(val, hpriv->mmio + IMX8QM_SATA_AHCI_PTC); > > + > > return 0; > > > > err_sata_phy_exit: > > -- > > 2.37.1 > >
diff --git a/drivers/ata/ahci_imx.c b/drivers/ata/ahci_imx.c index 4dd98368f8562..627b36cc4b5c1 100644 --- a/drivers/ata/ahci_imx.c +++ b/drivers/ata/ahci_imx.c @@ -45,6 +45,10 @@ enum { /* Clock Reset Register */ IMX_CLOCK_RESET = 0x7f3f, IMX_CLOCK_RESET_RESET = 1 << 0, + /* IMX8QM SATA specific control registers */ + IMX8QM_SATA_AHCI_PTC = 0xc8, + IMX8QM_SATA_AHCI_PTC_RXWM_MASK = GENMASK(6, 0), + IMX8QM_SATA_AHCI_PTC_RXWM = 0x29, }; enum ahci_imx_type { @@ -466,6 +470,12 @@ static int imx8_sata_enable(struct ahci_host_priv *hpriv) phy_power_off(imxpriv->cali_phy0); phy_exit(imxpriv->cali_phy0); + /* RxWaterMark setting */ + val = readl(hpriv->mmio + IMX8QM_SATA_AHCI_PTC); + val &= ~IMX8QM_SATA_AHCI_PTC_RXWM_MASK; + val |= IMX8QM_SATA_AHCI_PTC_RXWM; + writel(val, hpriv->mmio + IMX8QM_SATA_AHCI_PTC); + return 0; err_sata_phy_exit:
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. Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com> --- drivers/ata/ahci_imx.c | 10 ++++++++++ 1 file changed, 10 insertions(+)