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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

I have just sent the v2 version patch with cover letter so please review again.
The patch itself is not changed from previous version that fixed from original version.
But it is not in patchwork so let me send this as the v2 version.
Please review the patch again and if any concern or problem please let me know.
We would like to resolve the issue caused on our products by this patch.

Regards,
Ikegami

> -----Original Message-----
> From: IKEGAMI Tokunori
> Sent: Wednesday, April 25, 2018 10:29 AM
> To: 'Hauke Mehrtens'
> Cc: James Hogan; PACKHAM Chris; Rafał Miłecki; linux-mips@xxxxxxxxxxxxxx
> Subject: RE: [PATCH] MIPS: BCM47XX: Enable MIPS32 74K Core ExternalSync
> for BCM47XX PCIe erratum
> 
> 
> 
> > -----Original Message-----
> > From: Hauke Mehrtens [mailto:hauke@xxxxxxxxxx]
> > Sent: Wednesday, April 25, 2018 6:42 AM
> > To: IKEGAMI Tokunori
> > Cc: James Hogan; PACKHAM Chris; Rafał Miłecki; linux-mips@xxxxxxxxxxxxxx
> > Subject: Re: [PATCH] MIPS: BCM47XX: Enable MIPS32 74K Core ExternalSync
> > for BCM47XX PCIe erratum
> >
> > On 04/24/2018 09:19 PM, smtpuser wrote:
> > > From: Tokunori Ikegami <ikegami@xxxxxxxxxxxxxxxxxxxx>
> > >
> > > The erratum and workaround are described by BCM5300X-ES300-RDS.pdf as
> > below.
> > >
> > >   R10: PCIe Transactions Periodically Fail
> > >
> > >     Description: The BCM5300X PCIe does not maintain transaction
> > ordering.
> > >                  This may cause PCIe transaction failure.
> > >     Fix Comment: Add a dummy PCIe configuration read after a PCIe
> > >                  configuration write to ensure PCIe configuration
> > access
> > >                  ordering. Set ES bit of CP0 configu7 register to enable
> > >                  sync function so that the sync instruction is
> > functional.
> > >     Resolution:  hndpci.c: extpci_write_config()
> > >                  hndmips.c: si_mips_init()
> > >                  mipsinc.h CONF7_ES
> > >
> > > This is fixed by the CFE MIPS bcmsi chipset driver also for BCM47XX.
> > > Also the dummy PCIe configuration read is already implemented in the
> Linux
> > > BCMA driver.
> > > Enable ExternalSync in Config7 when CONFIG_BCMA_DRIVER_PCI_HOSTMODE=y
> > > too so that the sync instruction is externalised.
> > >
> > > Signed-off-by: Tokunori Ikegami <ikegami@xxxxxxxxxxxxxxxxxxxx>
> > > Cc: Chris Packham <chris.packham@xxxxxxxxxxxxxxxxxxx>
> > > Cc: Hauke Mehrtens <hauke@xxxxxxxxxx>
> > > Cc: Rafał Miłecki <zajec5@xxxxxxxxx>
> > > Cc: linux-mips@xxxxxxxxxxxxxx
> > > ---
> > > I have just fixed your comments.
> > > This patch will be sent by git-send-email.
> > > Also for our company mail system the sender mail address is needed to
> > be set as smtpuser <smtpuser@xxxxxxxxxxxxxxxxxxxx>.
> > > But do not reply to the email address smtpuser
> > <smtpuser@xxxxxxxxxxxxxxxxxxxx>.
> > > Please reply to my email address if any comemnt or problem.
> > > Sorry for inconvinient about this.
> >
> > Does the CFE boot loader normally apply this workaround? All devices I
> > have with this SoC use CFE to boot Linux and I am wondering why "only"
> > the workaround in the driver helped to workaround this problem.
> >
> > Not all Broadcom MIPS SoCs from the BRCM47xx line with MIPS 74K CPUs are
> > affected.
> >
> > See here, we only apply this for the BCM4716 SoCs:
> > https://git.kernel.org/linus/25c15566635fef86e87f762f73a19f24598e45fa
> >
> > Here the brcmsmac driver provided by Broadcom, it says that only BCM4716
> > and BCM4706 are affected:
> >
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tre
> > e/drivers/net/wireless/broadcom/brcm80211/brcmsmac/types.h#n255
> >
> > I am not sure if the version of the bcm4706 which is used on most
> > devices is affected as this workaround is not applied for this SoC in
> > b43, could be that Broadcom fixed that in a later revision and the
> > broken revision never made it into mass production.
> 
> Thank you so much for your comments.
> The CFE is implemented to enable the ExternalSync on the bcmsi chip set
> those are defined as mips74k CPU also.
> Also for other boards for example bcm94704cpci that I think that this is
> for BCM4704 the CPU is defined as bcmcore, sb1250 or mpc8245 but not defined
> as mips74k.
> I have just confirmed that BCM5357x family uses ARM CPU so it seems no
> problem.
> I am not sure if there is any other MIPS 74K CPU enabled
> CONFIG_BCMA_DRIVER_PCI_HOSTMODE option.
> But if needed to make sure I can change to use CONFIG_BCM47XX for the change
> instead of CONFIG_BCMA_DRIVER_PCI_HOSTMODE.
> So please let me know if needed to change.
>   Note: For our BCM53003 CPU if possible we would like to use
> CONFIG_BCMA_DRIVER_PCI_HOSTMODE as current patch.
> 
> > Do you ses the problem on the BCM53001 I think this is the same silicon
> > as the bcm4706?
> 
> Yes we are using BCM53003 and the BCM53003 chip ID is same with BCM4706.
> So it looks that the BCM53001 is also same.
> At this moment I could not confirm the actual erratum error behavior.
> 
> > >  arch/mips/include/asm/mipsregs.h |  3 +++
> > >  arch/mips/kernel/cpu-probe.c     | 12 +++++++++++-
> > >  2 files changed, 14 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/arch/mips/include/asm/mipsregs.h
> > b/arch/mips/include/asm/mipsregs.h
> > > index 858752dac337..0f94acf60144 100644
> > > --- a/arch/mips/include/asm/mipsregs.h
> > > +++ b/arch/mips/include/asm/mipsregs.h
> > > @@ -680,6 +680,8 @@
> > >  #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)
> > > @@ -2759,6 +2761,7 @@ __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)
> > > diff --git a/arch/mips/kernel/cpu-probe.c
> > b/arch/mips/kernel/cpu-probe.c
> > > index cf3fd549e16d..75039e89694f 100644
> > > --- a/arch/mips/kernel/cpu-probe.c
> > > +++ b/arch/mips/kernel/cpu-probe.c
> > > @@ -427,8 +427,18 @@ static inline void check_errata(void)
> > >  		 * making use of VPE1 will be responsable for that VPE.
> > >  		 */
> > >  		if ((c->processor_id & PRID_REV_MASK) <=
> > PRID_REV_34K_V1_0_2)
> > > -			write_c0_config7(read_c0_config7() |
> > MIPS_CONF7_RPS);
> > > +			set_c0_config7(MIPS_CONF7_RPS);
> > >  		break;
> > > +#ifdef CONFIG_BCMA_DRIVER_PCI_HOSTMODE
> > > +	case CPU_74K:
> > > +		/*
> > > +		 * BCM47XX Erratum "R10: PCIe Transactions Periodically
> > Fail"
> > > +		 * Enable ExternalSync for sync instruction to take effect
> > > +		 */
> > > +		pr_info("ExternalSync has been enabled\n");
> > > +		set_c0_config7(MIPS_CONF7_ES);
> > > +		break;
> > > +#endif
> > >  	default:
> > >  		break;
> > >  	}
> > >





[Index of Archives]     [Linux MIPS Home]     [LKML Archive]     [Linux ARM Kernel]     [Linux ARM]     [Linux]     [Git]     [Yosemite News]     [Linux SCSI]     [Linux Hams]

  Powered by Linux