Message ID | 20231018103337.34622-1-victorshihgli@gmail.com |
---|---|
State | New |
Headers | show |
Series | [V2] mmc: sdhci-pci-gli: GL975[05]: Mask the replay timer timeout of AER | expand |
Hi, Ulf Please help review this patch and give me some advice. I look forward to your reply. Thanks, Victor Shih On Wed, Oct 18, 2023 at 6:33 PM Victor Shih <victorshihgli@gmail.com> wrote: > > From: Victor Shih <victor.shih@genesyslogic.com.tw> > > Due to a flaw in the hardware design, the GL975x replay timer frequently > times out when ASPM is enabled. As a result, the warning messages that will > often appear in the system log when the system accesses the GL975x > PCI config. Therefore, the replay timer timeout must be masked. > > Signed-off-by: Victor Shih <victor.shih@genesyslogic.com.tw> > --- > > Updates in V2: > - Modify the commit message. > > --- > > drivers/mmc/host/sdhci-pci-gli.c | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/drivers/mmc/host/sdhci-pci-gli.c b/drivers/mmc/host/sdhci-pci-gli.c > index d83261e857a5..d8a991b349a8 100644 > --- a/drivers/mmc/host/sdhci-pci-gli.c > +++ b/drivers/mmc/host/sdhci-pci-gli.c > @@ -28,6 +28,9 @@ > #define PCI_GLI_9750_PM_CTRL 0xFC > #define PCI_GLI_9750_PM_STATE GENMASK(1, 0) > > +#define PCI_GLI_9750_CORRERR_MASK 0x214 > +#define PCI_GLI_9750_CORRERR_MASK_REPLAY_TIMER_TIMEOUT BIT(12) > + > #define SDHCI_GLI_9750_CFG2 0x848 > #define SDHCI_GLI_9750_CFG2_L1DLY GENMASK(28, 24) > #define GLI_9750_CFG2_L1DLY_VALUE 0x1F > @@ -152,6 +155,9 @@ > #define PCI_GLI_9755_PM_CTRL 0xFC > #define PCI_GLI_9755_PM_STATE GENMASK(1, 0) > > +#define PCI_GLI_9755_CORRERR_MASK 0x214 > +#define PCI_GLI_9755_CORRERR_MASK_REPLAY_TIMER_TIMEOUT BIT(12) > + > #define SDHCI_GLI_9767_GM_BURST_SIZE 0x510 > #define SDHCI_GLI_9767_GM_BURST_SIZE_AXI_ALWAYS_SET BIT(8) > > @@ -561,6 +567,11 @@ static void gl9750_hw_setting(struct sdhci_host *host) > value &= ~PCI_GLI_9750_PM_STATE; > pci_write_config_dword(pdev, PCI_GLI_9750_PM_CTRL, value); > > + /* mask the replay timer timeout of AER */ > + pci_read_config_dword(pdev, PCI_GLI_9750_CORRERR_MASK, &value); > + value |= PCI_GLI_9750_CORRERR_MASK_REPLAY_TIMER_TIMEOUT; > + pci_write_config_dword(pdev, PCI_GLI_9750_CORRERR_MASK, value); > + > gl9750_wt_off(host); > } > > @@ -770,6 +781,11 @@ static void gl9755_hw_setting(struct sdhci_pci_slot *slot) > value &= ~PCI_GLI_9755_PM_STATE; > pci_write_config_dword(pdev, PCI_GLI_9755_PM_CTRL, value); > > + /* mask the replay timer timeout of AER */ > + pci_read_config_dword(pdev, PCI_GLI_9755_CORRERR_MASK, &value); > + value |= PCI_GLI_9755_CORRERR_MASK_REPLAY_TIMER_TIMEOUT; > + pci_write_config_dword(pdev, PCI_GLI_9755_CORRERR_MASK, value); > + > gl9755_wt_off(pdev); > } > > -- > 2.25.1 >
On Fri, 3 Nov 2023 at 11:13, Victor Shih <victorshihgli@gmail.com> wrote: > > Hi, Ulf > > Please help review this patch and give me some advice. > I look forward to your reply. > > Thanks, Victor Shih I don't have in-depth knowledge of how to properly solve this for pci, so I am relying on Kai and others to help review. > > On Wed, Oct 18, 2023 at 6:33 PM Victor Shih <victorshihgli@gmail.com> wrote: > > > > From: Victor Shih <victor.shih@genesyslogic.com.tw> > > > > Due to a flaw in the hardware design, the GL975x replay timer frequently > > times out when ASPM is enabled. As a result, the warning messages that will > > often appear in the system log when the system accesses the GL975x > > PCI config. Therefore, the replay timer timeout must be masked. > > > > Signed-off-by: Victor Shih <victor.shih@genesyslogic.com.tw> The code looks good to me, but let's allow Kai to get some time to come back to this before I apply it. Kind regards Uffe > > --- > > > > Updates in V2: > > - Modify the commit message. > > > > --- > > > > drivers/mmc/host/sdhci-pci-gli.c | 16 ++++++++++++++++ > > 1 file changed, 16 insertions(+) > > > > diff --git a/drivers/mmc/host/sdhci-pci-gli.c b/drivers/mmc/host/sdhci-pci-gli.c > > index d83261e857a5..d8a991b349a8 100644 > > --- a/drivers/mmc/host/sdhci-pci-gli.c > > +++ b/drivers/mmc/host/sdhci-pci-gli.c > > @@ -28,6 +28,9 @@ > > #define PCI_GLI_9750_PM_CTRL 0xFC > > #define PCI_GLI_9750_PM_STATE GENMASK(1, 0) > > > > +#define PCI_GLI_9750_CORRERR_MASK 0x214 > > +#define PCI_GLI_9750_CORRERR_MASK_REPLAY_TIMER_TIMEOUT BIT(12) > > + > > #define SDHCI_GLI_9750_CFG2 0x848 > > #define SDHCI_GLI_9750_CFG2_L1DLY GENMASK(28, 24) > > #define GLI_9750_CFG2_L1DLY_VALUE 0x1F > > @@ -152,6 +155,9 @@ > > #define PCI_GLI_9755_PM_CTRL 0xFC > > #define PCI_GLI_9755_PM_STATE GENMASK(1, 0) > > > > +#define PCI_GLI_9755_CORRERR_MASK 0x214 > > +#define PCI_GLI_9755_CORRERR_MASK_REPLAY_TIMER_TIMEOUT BIT(12) > > + > > #define SDHCI_GLI_9767_GM_BURST_SIZE 0x510 > > #define SDHCI_GLI_9767_GM_BURST_SIZE_AXI_ALWAYS_SET BIT(8) > > > > @@ -561,6 +567,11 @@ static void gl9750_hw_setting(struct sdhci_host *host) > > value &= ~PCI_GLI_9750_PM_STATE; > > pci_write_config_dword(pdev, PCI_GLI_9750_PM_CTRL, value); > > > > + /* mask the replay timer timeout of AER */ > > + pci_read_config_dword(pdev, PCI_GLI_9750_CORRERR_MASK, &value); > > + value |= PCI_GLI_9750_CORRERR_MASK_REPLAY_TIMER_TIMEOUT; > > + pci_write_config_dword(pdev, PCI_GLI_9750_CORRERR_MASK, value); > > + > > gl9750_wt_off(host); > > } > > > > @@ -770,6 +781,11 @@ static void gl9755_hw_setting(struct sdhci_pci_slot *slot) > > value &= ~PCI_GLI_9755_PM_STATE; > > pci_write_config_dword(pdev, PCI_GLI_9755_PM_CTRL, value); > > > > + /* mask the replay timer timeout of AER */ > > + pci_read_config_dword(pdev, PCI_GLI_9755_CORRERR_MASK, &value); > > + value |= PCI_GLI_9755_CORRERR_MASK_REPLAY_TIMER_TIMEOUT; > > + pci_write_config_dword(pdev, PCI_GLI_9755_CORRERR_MASK, value); > > + > > gl9755_wt_off(pdev); > > } > > > > -- > > 2.25.1 > >
On Fri, Nov 3, 2023 at 6:28 PM Ulf Hansson <ulf.hansson@linaro.org> wrote: > > On Fri, 3 Nov 2023 at 11:13, Victor Shih <victorshihgli@gmail.com> wrote: > > > > Hi, Ulf > > > > Please help review this patch and give me some advice. > > I look forward to your reply. > > > > Thanks, Victor Shih > > I don't have in-depth knowledge of how to properly solve this for pci, > so I am relying on Kai and others to help review. > Hi, Ulf This is a standalone patch, this patch only solves the warning messages that will often appear in the system log when the system accesses the GL975x PCI config. As I discussed with Kai-Heng at the end of [V1] mmc: sdhci-pci-gli: GL975[05]: Mask the replay timer timeout of AER, Kai-Heng will be submitting a patch for the PCIe layer to solve the situation he encountered. I will also help Kai-Heng test when he submits the patch. Thanks, Victor Shih > > > > On Wed, Oct 18, 2023 at 6:33 PM Victor Shih <victorshihgli@gmail.com> wrote: > > > > > > From: Victor Shih <victor.shih@genesyslogic.com.tw> > > > > > > Due to a flaw in the hardware design, the GL975x replay timer frequently > > > times out when ASPM is enabled. As a result, the warning messages that will > > > often appear in the system log when the system accesses the GL975x > > > PCI config. Therefore, the replay timer timeout must be masked. > > > > > > Signed-off-by: Victor Shih <victor.shih@genesyslogic.com.tw> > > The code looks good to me, but let's allow Kai to get some time to > come back to this before I apply it. > > Kind regards > Uffe > > > > --- > > > > > > Updates in V2: > > > - Modify the commit message. > > > > > > --- > > > > > > drivers/mmc/host/sdhci-pci-gli.c | 16 ++++++++++++++++ > > > 1 file changed, 16 insertions(+) > > > > > > diff --git a/drivers/mmc/host/sdhci-pci-gli.c b/drivers/mmc/host/sdhci-pci-gli.c > > > index d83261e857a5..d8a991b349a8 100644 > > > --- a/drivers/mmc/host/sdhci-pci-gli.c > > > +++ b/drivers/mmc/host/sdhci-pci-gli.c > > > @@ -28,6 +28,9 @@ > > > #define PCI_GLI_9750_PM_CTRL 0xFC > > > #define PCI_GLI_9750_PM_STATE GENMASK(1, 0) > > > > > > +#define PCI_GLI_9750_CORRERR_MASK 0x214 > > > +#define PCI_GLI_9750_CORRERR_MASK_REPLAY_TIMER_TIMEOUT BIT(12) > > > + > > > #define SDHCI_GLI_9750_CFG2 0x848 > > > #define SDHCI_GLI_9750_CFG2_L1DLY GENMASK(28, 24) > > > #define GLI_9750_CFG2_L1DLY_VALUE 0x1F > > > @@ -152,6 +155,9 @@ > > > #define PCI_GLI_9755_PM_CTRL 0xFC > > > #define PCI_GLI_9755_PM_STATE GENMASK(1, 0) > > > > > > +#define PCI_GLI_9755_CORRERR_MASK 0x214 > > > +#define PCI_GLI_9755_CORRERR_MASK_REPLAY_TIMER_TIMEOUT BIT(12) > > > + > > > #define SDHCI_GLI_9767_GM_BURST_SIZE 0x510 > > > #define SDHCI_GLI_9767_GM_BURST_SIZE_AXI_ALWAYS_SET BIT(8) > > > > > > @@ -561,6 +567,11 @@ static void gl9750_hw_setting(struct sdhci_host *host) > > > value &= ~PCI_GLI_9750_PM_STATE; > > > pci_write_config_dword(pdev, PCI_GLI_9750_PM_CTRL, value); > > > > > > + /* mask the replay timer timeout of AER */ > > > + pci_read_config_dword(pdev, PCI_GLI_9750_CORRERR_MASK, &value); > > > + value |= PCI_GLI_9750_CORRERR_MASK_REPLAY_TIMER_TIMEOUT; > > > + pci_write_config_dword(pdev, PCI_GLI_9750_CORRERR_MASK, value); > > > + > > > gl9750_wt_off(host); > > > } > > > > > > @@ -770,6 +781,11 @@ static void gl9755_hw_setting(struct sdhci_pci_slot *slot) > > > value &= ~PCI_GLI_9755_PM_STATE; > > > pci_write_config_dword(pdev, PCI_GLI_9755_PM_CTRL, value); > > > > > > + /* mask the replay timer timeout of AER */ > > > + pci_read_config_dword(pdev, PCI_GLI_9755_CORRERR_MASK, &value); > > > + value |= PCI_GLI_9755_CORRERR_MASK_REPLAY_TIMER_TIMEOUT; > > > + pci_write_config_dword(pdev, PCI_GLI_9755_CORRERR_MASK, value); > > > + > > > gl9755_wt_off(pdev); > > > } > > > > > > -- > > > 2.25.1 > > >
On 18/10/23 13:33, Victor Shih wrote: > From: Victor Shih <victor.shih@genesyslogic.com.tw> > > Due to a flaw in the hardware design, the GL975x replay timer frequently > times out when ASPM is enabled. As a result, the warning messages that will > often appear in the system log when the system accesses the GL975x > PCI config. Therefore, the replay timer timeout must be masked. > > Signed-off-by: Victor Shih <victor.shih@genesyslogic.com.tw> Acked-by: Adrian Hunter <adrian.hunter@intel.com> > --- > > Updates in V2: > - Modify the commit message. > > --- > > drivers/mmc/host/sdhci-pci-gli.c | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/drivers/mmc/host/sdhci-pci-gli.c b/drivers/mmc/host/sdhci-pci-gli.c > index d83261e857a5..d8a991b349a8 100644 > --- a/drivers/mmc/host/sdhci-pci-gli.c > +++ b/drivers/mmc/host/sdhci-pci-gli.c > @@ -28,6 +28,9 @@ > #define PCI_GLI_9750_PM_CTRL 0xFC > #define PCI_GLI_9750_PM_STATE GENMASK(1, 0) > > +#define PCI_GLI_9750_CORRERR_MASK 0x214 > +#define PCI_GLI_9750_CORRERR_MASK_REPLAY_TIMER_TIMEOUT BIT(12) > + > #define SDHCI_GLI_9750_CFG2 0x848 > #define SDHCI_GLI_9750_CFG2_L1DLY GENMASK(28, 24) > #define GLI_9750_CFG2_L1DLY_VALUE 0x1F > @@ -152,6 +155,9 @@ > #define PCI_GLI_9755_PM_CTRL 0xFC > #define PCI_GLI_9755_PM_STATE GENMASK(1, 0) > > +#define PCI_GLI_9755_CORRERR_MASK 0x214 > +#define PCI_GLI_9755_CORRERR_MASK_REPLAY_TIMER_TIMEOUT BIT(12) > + > #define SDHCI_GLI_9767_GM_BURST_SIZE 0x510 > #define SDHCI_GLI_9767_GM_BURST_SIZE_AXI_ALWAYS_SET BIT(8) > > @@ -561,6 +567,11 @@ static void gl9750_hw_setting(struct sdhci_host *host) > value &= ~PCI_GLI_9750_PM_STATE; > pci_write_config_dword(pdev, PCI_GLI_9750_PM_CTRL, value); > > + /* mask the replay timer timeout of AER */ > + pci_read_config_dword(pdev, PCI_GLI_9750_CORRERR_MASK, &value); > + value |= PCI_GLI_9750_CORRERR_MASK_REPLAY_TIMER_TIMEOUT; > + pci_write_config_dword(pdev, PCI_GLI_9750_CORRERR_MASK, value); > + > gl9750_wt_off(host); > } > > @@ -770,6 +781,11 @@ static void gl9755_hw_setting(struct sdhci_pci_slot *slot) > value &= ~PCI_GLI_9755_PM_STATE; > pci_write_config_dword(pdev, PCI_GLI_9755_PM_CTRL, value); > > + /* mask the replay timer timeout of AER */ > + pci_read_config_dword(pdev, PCI_GLI_9755_CORRERR_MASK, &value); > + value |= PCI_GLI_9755_CORRERR_MASK_REPLAY_TIMER_TIMEOUT; > + pci_write_config_dword(pdev, PCI_GLI_9755_CORRERR_MASK, value); > + > gl9755_wt_off(pdev); > } >
On Mon, Nov 6, 2023 at 11:29 AM Adrian Hunter <adrian.hunter@intel.com> wrote: > > On 18/10/23 13:33, Victor Shih wrote: > > From: Victor Shih <victor.shih@genesyslogic.com.tw> > > > > Due to a flaw in the hardware design, the GL975x replay timer frequently > > times out when ASPM is enabled. As a result, the warning messages that will > > often appear in the system log when the system accesses the GL975x > > PCI config. Therefore, the replay timer timeout must be masked. > > > > Signed-off-by: Victor Shih <victor.shih@genesyslogic.com.tw> > > Acked-by: Adrian Hunter <adrian.hunter@intel.com> Acked-by: Kai-Heng Feng <kai.heng.feng@canonical.com> I'll send another patch to address the issue I am seeing. Kai-Heng > > > --- > > > > Updates in V2: > > - Modify the commit message. > > > > --- > > > > drivers/mmc/host/sdhci-pci-gli.c | 16 ++++++++++++++++ > > 1 file changed, 16 insertions(+) > > > > diff --git a/drivers/mmc/host/sdhci-pci-gli.c b/drivers/mmc/host/sdhci-pci-gli.c > > index d83261e857a5..d8a991b349a8 100644 > > --- a/drivers/mmc/host/sdhci-pci-gli.c > > +++ b/drivers/mmc/host/sdhci-pci-gli.c > > @@ -28,6 +28,9 @@ > > #define PCI_GLI_9750_PM_CTRL 0xFC > > #define PCI_GLI_9750_PM_STATE GENMASK(1, 0) > > > > +#define PCI_GLI_9750_CORRERR_MASK 0x214 > > +#define PCI_GLI_9750_CORRERR_MASK_REPLAY_TIMER_TIMEOUT BIT(12) > > + > > #define SDHCI_GLI_9750_CFG2 0x848 > > #define SDHCI_GLI_9750_CFG2_L1DLY GENMASK(28, 24) > > #define GLI_9750_CFG2_L1DLY_VALUE 0x1F > > @@ -152,6 +155,9 @@ > > #define PCI_GLI_9755_PM_CTRL 0xFC > > #define PCI_GLI_9755_PM_STATE GENMASK(1, 0) > > > > +#define PCI_GLI_9755_CORRERR_MASK 0x214 > > +#define PCI_GLI_9755_CORRERR_MASK_REPLAY_TIMER_TIMEOUT BIT(12) > > + > > #define SDHCI_GLI_9767_GM_BURST_SIZE 0x510 > > #define SDHCI_GLI_9767_GM_BURST_SIZE_AXI_ALWAYS_SET BIT(8) > > > > @@ -561,6 +567,11 @@ static void gl9750_hw_setting(struct sdhci_host *host) > > value &= ~PCI_GLI_9750_PM_STATE; > > pci_write_config_dword(pdev, PCI_GLI_9750_PM_CTRL, value); > > > > + /* mask the replay timer timeout of AER */ > > + pci_read_config_dword(pdev, PCI_GLI_9750_CORRERR_MASK, &value); > > + value |= PCI_GLI_9750_CORRERR_MASK_REPLAY_TIMER_TIMEOUT; > > + pci_write_config_dword(pdev, PCI_GLI_9750_CORRERR_MASK, value); > > + > > gl9750_wt_off(host); > > } > > > > @@ -770,6 +781,11 @@ static void gl9755_hw_setting(struct sdhci_pci_slot *slot) > > value &= ~PCI_GLI_9755_PM_STATE; > > pci_write_config_dword(pdev, PCI_GLI_9755_PM_CTRL, value); > > > > + /* mask the replay timer timeout of AER */ > > + pci_read_config_dword(pdev, PCI_GLI_9755_CORRERR_MASK, &value); > > + value |= PCI_GLI_9755_CORRERR_MASK_REPLAY_TIMER_TIMEOUT; > > + pci_write_config_dword(pdev, PCI_GLI_9755_CORRERR_MASK, value); > > + > > gl9755_wt_off(pdev); > > } > > >
On Mon, 6 Nov 2023 at 09:56, Victor Shih <victorshihgli@gmail.com> wrote: > > On Fri, Nov 3, 2023 at 6:28 PM Ulf Hansson <ulf.hansson@linaro.org> wrote: > > > > On Fri, 3 Nov 2023 at 11:13, Victor Shih <victorshihgli@gmail.com> wrote: > > > > > > Hi, Ulf > > > > > > Please help review this patch and give me some advice. > > > I look forward to your reply. > > > > > > Thanks, Victor Shih > > > > I don't have in-depth knowledge of how to properly solve this for pci, > > so I am relying on Kai and others to help review. > > > > Hi, Ulf > > This is a standalone patch, this patch only solves the warning > messages that will > often appear in the system log when the system accesses the GL975x PCI config. > > As I discussed with Kai-Heng at the end of > [V1] mmc: sdhci-pci-gli: GL975[05]: Mask the replay timer timeout of AER, > Kai-Heng will be submitting a patch for the PCIe layer to solve the > situation he encountered. > I will also help Kai-Heng test when he submits the patch. > > Thanks, Victor Shih Hi Victor, I was just about to apply the patch for my fixes branch, when I realized that it would be better to split it up in two patches. One patch for GL9755 and another for GL9750. In this way we can easier manage the back-porting to older stable kernels. The patch for GL9755 should have the below fixes tag: Fixes: 36ed2fd32b2c ("mmc: sdhci-pci-gli: A workaround to allow GL9755 to enter ASPM L1.2") The patch for GL9750 should have the below fixes tag: Fixes: d7133797e9e1 ("mmc: sdhci-pci-gli: A workaround to allow GL9750 to enter ASPM L1.2") I think you should feel free to keep Adrian's and Kai's acks for both of the two new patches, as the end result should end up being the same. [...] Kind regards Uffe
diff --git a/drivers/mmc/host/sdhci-pci-gli.c b/drivers/mmc/host/sdhci-pci-gli.c index d83261e857a5..d8a991b349a8 100644 --- a/drivers/mmc/host/sdhci-pci-gli.c +++ b/drivers/mmc/host/sdhci-pci-gli.c @@ -28,6 +28,9 @@ #define PCI_GLI_9750_PM_CTRL 0xFC #define PCI_GLI_9750_PM_STATE GENMASK(1, 0) +#define PCI_GLI_9750_CORRERR_MASK 0x214 +#define PCI_GLI_9750_CORRERR_MASK_REPLAY_TIMER_TIMEOUT BIT(12) + #define SDHCI_GLI_9750_CFG2 0x848 #define SDHCI_GLI_9750_CFG2_L1DLY GENMASK(28, 24) #define GLI_9750_CFG2_L1DLY_VALUE 0x1F @@ -152,6 +155,9 @@ #define PCI_GLI_9755_PM_CTRL 0xFC #define PCI_GLI_9755_PM_STATE GENMASK(1, 0) +#define PCI_GLI_9755_CORRERR_MASK 0x214 +#define PCI_GLI_9755_CORRERR_MASK_REPLAY_TIMER_TIMEOUT BIT(12) + #define SDHCI_GLI_9767_GM_BURST_SIZE 0x510 #define SDHCI_GLI_9767_GM_BURST_SIZE_AXI_ALWAYS_SET BIT(8) @@ -561,6 +567,11 @@ static void gl9750_hw_setting(struct sdhci_host *host) value &= ~PCI_GLI_9750_PM_STATE; pci_write_config_dword(pdev, PCI_GLI_9750_PM_CTRL, value); + /* mask the replay timer timeout of AER */ + pci_read_config_dword(pdev, PCI_GLI_9750_CORRERR_MASK, &value); + value |= PCI_GLI_9750_CORRERR_MASK_REPLAY_TIMER_TIMEOUT; + pci_write_config_dword(pdev, PCI_GLI_9750_CORRERR_MASK, value); + gl9750_wt_off(host); } @@ -770,6 +781,11 @@ static void gl9755_hw_setting(struct sdhci_pci_slot *slot) value &= ~PCI_GLI_9755_PM_STATE; pci_write_config_dword(pdev, PCI_GLI_9755_PM_CTRL, value); + /* mask the replay timer timeout of AER */ + pci_read_config_dword(pdev, PCI_GLI_9755_CORRERR_MASK, &value); + value |= PCI_GLI_9755_CORRERR_MASK_REPLAY_TIMER_TIMEOUT; + pci_write_config_dword(pdev, PCI_GLI_9755_CORRERR_MASK, value); + gl9755_wt_off(pdev); }