On 05/05/2012 09:50 AM, Nathan Hintz wrote: > On Sat, 2012-05-05 at 08:03 +0200, Arend van Spriel wrote: >> On 05/05/2012 06:56 AM, Nathan Hintz wrote: >>> Adds a missing read to flush the previous write (per the Broadcom SDK). >>> >>> Signed-off-by: Nathan Hintz <nlhintz@xxxxxxxxxxx> >>> --- >>> drivers/bcma/core.c | 1 + >>> 1 files changed, 1 insertions(+), 0 deletions(-) >>> >>> diff --git a/drivers/bcma/core.c b/drivers/bcma/core.c >>> index 893f6e0..c4e6deb 100644 >>> --- a/drivers/bcma/core.c >>> +++ b/drivers/bcma/core.c >>> @@ -30,6 +30,7 @@ void bcma_core_disable(struct bcma_device *core, u32 flags) >>> udelay(10); >>> >>> bcma_awrite32(core, BCMA_RESET_CTL, BCMA_RESET_CTL_RESET); >>> + bcma_aread32(core, BCMA_RESET_CTL); >>> udelay(1); >>> } >>> EXPORT_SYMBOL_GPL(bcma_core_disable); >> >> Hi Nathan, >> >> The read after write is only needed on (certain) SoCs. As bcma is not >> being used just for these SoCs I suggest to make the flush conditional. >> In brcmsmac we introduced "flushed write" function, which does the read >> only for those SoCs. >> >> Gr. AvS >> >> > There are several other occurrences in core.c that are the same > implementation as what was added in this patch. Is there a reason > why the existing code isn't the way you have suggested already? > Would it be better to submit your suggestion as a separate patch, or > just roll it all into this one? > > Nathan In some older versions of the Broadcom SDK (e.g. version with wl 5.10.147.0 and the version brcmsmac seams to be based on) this read is not included in ai_core_disable(), but in more recent versions (e.g. version including wl 5.100.138.9) this read is included unconditionally. @Arend For what SoCs should we read after this write? When you are speaking of (certain) SoCs are you talking about this code and comment from brcmsmac? #ifdef CONFIG_BCM47XX /* * bcm4716 (which includes 4717 & 4718), plus 4706 on PCIe can reorder * transactions. As a fix, a read after write is performed on certain places * in the code. Older chips and the newer 5357 family don't require this fix. */ #define bcma_wflush16(c, o, v) \ ({ bcma_write16(c, o, v); (void)bcma_read16(c, o); }) Hauke -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html