[PATCH] Revert "MIPS: BCM47XX: Enable 74K Core ExternalSync for PCIe erratum"

[PATCH] Revert "MIPS: BCM47XX: Enable 74K Core ExternalSync for PCIe erratum"

[Rafał Miłecki]

From: Rafał Miłecki <rafal@milecki.pl>

This reverts commit 2a027b47dba6b77ab8c8e47b589ae9bbc5ac6175.

Enabling ExternalSync caused a regression for BCM4718A1 (used e.g. in
Netgear E3000 and ASUS RT-N16): it simply hangs during PCIe
initialization. It's likely that BCM4717A1 is also affected.

I didn't notice that earlier as the only BCM47XX devices with PCIe I
own are:
1) BCM4706 with 2 x 14e4:4331
2) BCM4706 with 14e4:4360 and 14e4:4331
it appears that BCM4706 is unaffected.

While BCM5300X-ES300-RDS.pdf seems to document that erratum and its
workarounds (according to quotes provided by Tokunori) it seems not even
Broadcom follows them.

According to the provided info Broadcom should define CONF7_ES in their
SDK's mipsinc.h and implement workaround in the si_mips_init(). Checking
both didn't reveal such code. It *could* mean Broadcom also had some
problems with the given workaround.

Reported-by: Michael Marley <michael@michaelmarley.com>
Cc: Tokunori Ikegami <ikegami@allied-telesis.co.jp>
Cc: Paul Burton <paul.burton@mips.com>
Cc: Hauke Mehrtens <hauke@hauke-m.de>
Cc: Chris Packham <chris.packham@alliedtelesis.co.nz>
Cc: stable@vger.kernel.org
Cc: James Hogan <jhogan@kernel.org>
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
This has been reported by Michael as OpenWrt bug at:
https://bugs.openwrt.org/index.php?do=details&task_id=1688
---
 arch/mips/bcm47xx/setup.c        | 6 ------
 arch/mips/include/asm/mipsregs.h | 3 ---
 2 files changed, 9 deletions(-)

diff --git a/arch/mips/bcm47xx/setup.c b/arch/mips/bcm47xx/setup.c
index 8c9cbf13d32a..6054d49e608e 100644
--- a/arch/mips/bcm47xx/setup.c
+++ b/arch/mips/bcm47xx/setup.c
@@ -212,12 +212,6 @@ static int __init bcm47xx_cpu_fixes(void)
 		 */
 		if (bcm47xx_bus.bcma.bus.chipinfo.id == BCMA_CHIP_ID_BCM4706)
 			cpu_wait = NULL;
-
-		/*
-		 * BCM47XX Erratum "R10: PCIe Transactions Periodically Fail"
-		 * Enable ExternalSync for sync instruction to take effect
-		 */
-		set_c0_config7(MIPS_CONF7_ES);
 		break;
 #endif
 	}
diff --git a/arch/mips/include/asm/mipsregs.h b/arch/mips/include/asm/mipsregs.h
index 0bc270806ec5..ae461d91cd1f 100644
--- a/arch/mips/include/asm/mipsregs.h
+++ b/arch/mips/include/asm/mipsregs.h
@@ -681,8 +681,6 @@
 #define MIPS_CONF7_WII		(_ULCAST_(1) << 31)
 
 #define MIPS_CONF7_RPS		(_ULCAST_(1) << 2)
-/* ExternalSync */
-#define MIPS_CONF7_ES		(_ULCAST_(1) << 8)
 
 #define MIPS_CONF7_IAR		(_ULCAST_(1) << 10)
 #define MIPS_CONF7_AR		(_ULCAST_(1) << 16)
@@ -2767,7 +2765,6 @@ __BUILD_SET_C0(status)
 __BUILD_SET_C0(cause)
 __BUILD_SET_C0(config)
 __BUILD_SET_C0(config5)
-__BUILD_SET_C0(config7)
 __BUILD_SET_C0(intcontrol)
 __BUILD_SET_C0(intctl)
 __BUILD_SET_C0(srsmap)
-- 
2.13.7

Re: [PATCH] Revert "MIPS: BCM47XX: Enable 74K Core ExternalSync for PCIe erratum"

[Paul Burton]

Hi Rafał,

On Fri, Jul 27, 2018 at 01:13:39PM +0200, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> This reverts commit 2a027b47dba6b77ab8c8e47b589ae9bbc5ac6175.
> 
> Enabling ExternalSync caused a regression for BCM4718A1 (used e.g. in
> Netgear E3000 and ASUS RT-N16): it simply hangs during PCIe
> initialization. It's likely that BCM4717A1 is also affected.
> 
> I didn't notice that earlier as the only BCM47XX devices with PCIe I
> own are:
> 1) BCM4706 with 2 x 14e4:4331
> 2) BCM4706 with 14e4:4360 and 14e4:4331
> it appears that BCM4706 is unaffected.
> 
> While BCM5300X-ES300-RDS.pdf seems to document that erratum and its
> workarounds (according to quotes provided by Tokunori) it seems not even
> Broadcom follows them.
> 
> According to the provided info Broadcom should define CONF7_ES in their
> SDK's mipsinc.h and implement workaround in the si_mips_init(). Checking
> both didn't reveal such code. It *could* mean Broadcom also had some
> problems with the given workaround.
> 
> Reported-by: Michael Marley <michael@michaelmarley.com>
> Cc: Tokunori Ikegami <ikegami@allied-telesis.co.jp>
> Cc: Paul Burton <paul.burton@mips.com>
> Cc: Hauke Mehrtens <hauke@hauke-m.de>
> Cc: Chris Packham <chris.packham@alliedtelesis.co.nz>
> Cc: stable@vger.kernel.org
> Cc: James Hogan <jhogan@kernel.org>
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
> This has been reported by Michael as OpenWrt bug at:
> https://bugs.openwrt.org/index.php?do=details&task_id=1688
> ---
>  arch/mips/bcm47xx/setup.c        | 6 ------
>  arch/mips/include/asm/mipsregs.h | 3 ---
>  2 files changed, 9 deletions(-)

Thanks - I've applied this to mips-fixes, and will send to Linus before
v4.18 final so this regression shouldn't appear in a stable kernel.

Tokunori - if this breaks your system then we'll need to look at
applying the workaround more selectively.

Paul

Re: [PATCH] Revert "MIPS: BCM47XX: Enable 74K Core ExternalSync for PCIe erratum"

[Chris Packham]

On 28/07/18 05:13, Paul Burton wrote:
> Hi Rafał,
> 
> On Fri, Jul 27, 2018 at 01:13:39PM +0200, Rafał Miłecki wrote:
>> From: Rafał Miłecki <rafal@milecki.pl>
>>
>> This reverts commit 2a027b47dba6b77ab8c8e47b589ae9bbc5ac6175.
>>
>> Enabling ExternalSync caused a regression for BCM4718A1 (used e.g. in
>> Netgear E3000 and ASUS RT-N16): it simply hangs during PCIe
>> initialization. It's likely that BCM4717A1 is also affected.
>>
>> I didn't notice that earlier as the only BCM47XX devices with PCIe I
>> own are:
>> 1) BCM4706 with 2 x 14e4:4331
>> 2) BCM4706 with 14e4:4360 and 14e4:4331
>> it appears that BCM4706 is unaffected.
>>
>> While BCM5300X-ES300-RDS.pdf seems to document that erratum and its
>> workarounds (according to quotes provided by Tokunori) it seems not even
>> Broadcom follows them.
>>
>> According to the provided info Broadcom should define CONF7_ES in their
>> SDK's mipsinc.h and implement workaround in the si_mips_init(). Checking
>> both didn't reveal such code. It *could* mean Broadcom also had some
>> problems with the given workaround.
>>
>> Reported-by: Michael Marley <michael@michaelmarley.com>
>> Cc: Tokunori Ikegami <ikegami@allied-telesis.co.jp>
>> Cc: Paul Burton <paul.burton@mips.com>
>> Cc: Hauke Mehrtens <hauke@hauke-m.de>
>> Cc: Chris Packham <chris.packham@alliedtelesis.co.nz>
>> Cc: stable@vger.kernel.org
>> Cc: James Hogan <jhogan@kernel.org>
>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>> ---
>> This has been reported by Michael as OpenWrt bug at:
>> https://bugs.openwrt.org/index.php?do=details&task_id=1688
>> ---
>>   arch/mips/bcm47xx/setup.c        | 6 ------
>>   arch/mips/include/asm/mipsregs.h | 3 ---
>>   2 files changed, 9 deletions(-)
> 
> Thanks - I've applied this to mips-fixes, and will send to Linus before
> v4.18 final so this regression shouldn't appear in a stable kernel.
> 
> Tokunori - if this breaks your system then we'll need to look at
> applying the workaround more selectively.

I wonder if the Erratum really is specific to the 5300X SoC (which isn't 
supported in upstream Linux). It's a little hard to get any useful 
information out of Broadcom, especially for an older chip.

RE: [PATCH] Revert "MIPS: BCM47XX: Enable 74K Core ExternalSync for PCIe erratum"

[IKEGAMI Tokunori]

Hi,

I am very sorry for the problem caused by my patch.
And very sorry for too late to reply to the mail since I have just noticed this revert today...
(The mail was automatically saved into a folder to filter mails unintentionally...)
Also Thank you so much for your fix by reverting the patch.

> Tokunori - if this breaks your system then we'll need to look at
> applying the workaround more selectively.

  Thank you so much.
  Noted it.
  At this moment it is not broken on our system but if needed I will do it.

Regards,
Ikegami

> -----Original Message-----
> From: Paul Burton [mailto:paul.burton@mips.com]
> Sent: Saturday, July 28, 2018 2:13 AM
> To: Rafał Miłecki; IKEGAMI Tokunori
> Cc: James Hogan; Ralf Baechle; Michael Marley; PACKHAM Chris; Hauke
> Mehrtens; linux-mips@linux-mips.org; Rafał Miłecki
> Subject: Re: [PATCH] Revert "MIPS: BCM47XX: Enable 74K Core ExternalSync
> for PCIe erratum"
> 
> Hi Rafał,
> 
> On Fri, Jul 27, 2018 at 01:13:39PM +0200, Rafał Miłecki wrote:
> > From: Rafał Miłecki <rafal@milecki.pl>
> >
> > This reverts commit 2a027b47dba6b77ab8c8e47b589ae9bbc5ac6175.
> >
> > Enabling ExternalSync caused a regression for BCM4718A1 (used e.g. in
> > Netgear E3000 and ASUS RT-N16): it simply hangs during PCIe
> > initialization. It's likely that BCM4717A1 is also affected.
> >
> > I didn't notice that earlier as the only BCM47XX devices with PCIe I
> > own are:
> > 1) BCM4706 with 2 x 14e4:4331
> > 2) BCM4706 with 14e4:4360 and 14e4:4331
> > it appears that BCM4706 is unaffected.
> >
> > While BCM5300X-ES300-RDS.pdf seems to document that erratum and its
> > workarounds (according to quotes provided by Tokunori) it seems not even
> > Broadcom follows them.
> >
> > According to the provided info Broadcom should define CONF7_ES in their
> > SDK's mipsinc.h and implement workaround in the si_mips_init(). Checking
> > both didn't reveal such code. It *could* mean Broadcom also had some
> > problems with the given workaround.
> >
> > Reported-by: Michael Marley <michael@michaelmarley.com>
> > Cc: Tokunori Ikegami <ikegami@allied-telesis.co.jp>
> > Cc: Paul Burton <paul.burton@mips.com>
> > Cc: Hauke Mehrtens <hauke@hauke-m.de>
> > Cc: Chris Packham <chris.packham@alliedtelesis.co.nz>
> > Cc: stable@vger.kernel.org
> > Cc: James Hogan <jhogan@kernel.org>
> > Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> > ---
> > This has been reported by Michael as OpenWrt bug at:
> > https://bugs.openwrt.org/index.php?do=details&task_id=1688
> > ---
> >  arch/mips/bcm47xx/setup.c        | 6 ------
> >  arch/mips/include/asm/mipsregs.h | 3 ---
> >  2 files changed, 9 deletions(-)
> 
> Thanks - I've applied this to mips-fixes, and will send to Linus before
> v4.18 final so this regression shouldn't appear in a stable kernel.
> 
> Tokunori - if this breaks your system then we'll need to look at
> applying the workaround more selectively.
> 
> Paul