| Message ID | 20191014061617.10296-2-daniel@0x0f.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/4] dt-bindings: arm: Initial MStar vendor prefixes and compatible strings | expand |
On Mon, Oct 14, 2019 at 8:21 AM Daniel Palmer <daniel@0x0f.com> wrote: > > Initial support for the MStar infinity/infinity3 series of Cortex A7 > based IP camera SoCs. > > These chips are interesting in that they contain a Cortex A7, > peripherals and system memory in a single tiny QFN package that > can be hand soldered allowing almost anyone to embed Linux > in their projects. > > Signed-off-by: Daniel Palmer <daniel@0x0f.com> > + > +static void __init infinity_map_io(void) > +{ > + iotable_init(infinity_io_desc, ARRAY_SIZE(infinity_io_desc)); > + miu_flush = (void __iomem *)(infinity_io_desc[0].virtual > + + INFINITY_L3BRIDGE_FLUSH); > + miu_status = (void __iomem *)(infinity_io_desc[0].virtual > + + INFINITY_L3BRIDGE_STATUS); > +} If you do this a little later in .init_machine, you can use a simple ioremap() rather than picking a hardcoded physical address. It looks like nothing uses the mapping before you set soc_mb anyway. > +static DEFINE_SPINLOCK(infinity_mb_lock); > + > +static void infinity_mb(void) > +{ > + unsigned long flags; > + > + spin_lock_irqsave(&infinity_mb_lock, flags); > + /* toggle the flush miu pipe fire bit */ > + writel_relaxed(0, miu_flush); > + writel_relaxed(INFINITY_L3BRIDGE_FLUSH_TRIGGER, miu_flush); > + while (!(readl_relaxed(miu_status) & INFINITY_L3BRIDGE_STATUS_DONE)) { > + /* wait for flush to complete */ > + } > + spin_unlock_irqrestore(&infinity_mb_lock, flags); > +} Wow, this is a heavy barrier. From your description it doesn't sound like there is anything to be done about it unfortunately. Two possible issues I see here: * It looks like it relies on CONFIG_ARM_HEAVY_BARRIER, but your Kconfig entry does not select that. In many configurations, CACHE_L2X0 would be set, but there is no need for yours on the Cortex-A7. * You might get into a deadlock if you get an FIQ (NMI) interrupt while holding the infinity_mb_lock, and then issue another memory barrier from that handler, so you might need to use local_irq_disable()+local_fiq_disable()+raw_spin_lock() here, making it even more expensive. Not sure if it matters in practice, as almost nothing uses fiq any more. OTOH, maybe the lock is not needed at all? AFAICT if the sequence gets interrupted by a handler that also calls mb(), you would still continue in the original thread while observing a full l3 barrier. ;-) Arnd
> > + > > +static void __init infinity_map_io(void) > > +{ > > + iotable_init(infinity_io_desc, ARRAY_SIZE(infinity_io_desc)); > > + miu_flush = (void __iomem *)(infinity_io_desc[0].virtual > > + + INFINITY_L3BRIDGE_FLUSH); > > + miu_status = (void __iomem *)(infinity_io_desc[0].virtual > > + + INFINITY_L3BRIDGE_STATUS); > > +} > > If you do this a little later in .init_machine, you can use a simple ioremap() > rather than picking a hardcoded physical address. It looks like nothing > uses the mapping before you set soc_mb anyway. I've moved this into infinity_barriers_init() using ioremap() as suggested. I'd like to keep the fixed remap address for now as there are some drivers in the vendor code that might be useful until rewrites are done but are littered with hard coded addresses. > > +static DEFINE_SPINLOCK(infinity_mb_lock); > > + > > +static void infinity_mb(void) > > +{ > > + unsigned long flags; > > + > > + spin_lock_irqsave(&infinity_mb_lock, flags); > > + /* toggle the flush miu pipe fire bit */ > > + writel_relaxed(0, miu_flush); > > + writel_relaxed(INFINITY_L3BRIDGE_FLUSH_TRIGGER, miu_flush); > > + while (!(readl_relaxed(miu_status) & INFINITY_L3BRIDGE_STATUS_DONE)) { > > + /* wait for flush to complete */ > > + } > > + spin_unlock_irqrestore(&infinity_mb_lock, flags); > > +} > > Wow, this is a heavy barrier. From your description it doesn't sound like > there is anything to be done about it unfortunately. It's possible there is a better way once I can find out what the L3 bridge actually is. There is a small amount of documentation for the miu (DDR controller) that says it has an 8 or 4 operation configurable pipeline but this flushing bit is in a totally different area that's only documented by the comment about it in u-boot. > Two possible issues I see here: > > * It looks like it relies on CONFIG_ARM_HEAVY_BARRIER, but your Kconfig > entry does not select that. In many configurations, CACHE_L2X0 would > be set, but there is no need for yours on the Cortex-A7. Fixed. > Not sure if it matters in practice, as almost nothing uses fiq any more. > OTOH, maybe the lock is not needed at all? AFAICT if the sequence > gets interrupted by a handler that also calls mb(), you would still > continue in the original thread while observing a full l3 barrier. ;-) I've taken the lock out and tested that the ethernet isn't sending garbage and everything looks good. I'm still hoping for some feedback on the other parts of the series. I'll post a v2 series in a few days. Thanks! Daniel
On Wed, Oct 16, 2019 at 10:32 PM Daniel Palmer <daniel@0x0f.com> wrote: > > > > + > > > +static void __init infinity_map_io(void) > > > +{ > > > + iotable_init(infinity_io_desc, ARRAY_SIZE(infinity_io_desc)); > > > + miu_flush = (void __iomem *)(infinity_io_desc[0].virtual > > > + + INFINITY_L3BRIDGE_FLUSH); > > > + miu_status = (void __iomem *)(infinity_io_desc[0].virtual > > > + + INFINITY_L3BRIDGE_STATUS); > > > +} > > > > If you do this a little later in .init_machine, you can use a simple ioremap() > > rather than picking a hardcoded physical address. It looks like nothing > > uses the mapping before you set soc_mb anyway. > > I've moved this into infinity_barriers_init() using ioremap() as suggested. > I'd like to keep the fixed remap address for now as there are some > drivers in the vendor code that might be useful until rewrites are done but > are littered with hard coded addresses. Maybe keep the infinity_io_desc as an out-of-tree patch then? You can simply do both, and ioremap() will return the hardcoded address. > > Not sure if it matters in practice, as almost nothing uses fiq any more. > > OTOH, maybe the lock is not needed at all? AFAICT if the sequence > > gets interrupted by a handler that also calls mb(), you would still > > continue in the original thread while observing a full l3 barrier. ;-) > > I've taken the lock out and tested that the ethernet isn't sending garbage > and everything looks good. I would not expect a missing spinlock to have an observable effect, the question is more whether it's correct in all rare corner cases where the barrier is interrupted and the interrupt handler uses another barrier. I think it is, but I would recommend adding a comment to explain this if you drop the spinlock. (or a comment about why this works with fiq if you keep the lock). Arnd
On Thu, Oct 17, 2019 at 03:02:22PM +0200, Arnd Bergmann wrote: > > I've moved this into infinity_barriers_init() using ioremap() as suggested. > > I'd like to keep the fixed remap address for now as there are some > > drivers in the vendor code that might be useful until rewrites are done but > > are littered with hard coded addresses. > > Maybe keep the infinity_io_desc as an out-of-tree patch then? You can > simply do both, and ioremap() will return the hardcoded address. That makes sense. > > I've taken the lock out and tested that the ethernet isn't sending garbage > > and everything looks good. > > I would not expect a missing spinlock to have an observable effect, the > question is more whether it's correct in all rare corner cases where > the barrier is interrupted and the interrupt handler uses another barrier. > > I think it is, but I would recommend adding a comment to explain this if > you drop the spinlock. (or a comment about why this works with fiq if you > keep the lock). I think I'll drop the lock for now and add it back if it becomes apparent it's needed. I suspect it was added in the vendor code out of habit instead of need. Thanks for the input. Daniel
diff --git a/MAINTAINERS b/MAINTAINERS index 8b7913c13f9a..e35c3eb2b680 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1985,6 +1985,7 @@ ARM/MStar SoC support M: Daniel Palmer <daniel@thingy.jp> L: linux-arm-kernel@lists.infradead.org (moderated for non-subscribers) F: Documentation/devicetree/bindings/arm/mstar.yaml +F: arch/arm/mach-mstar/ S: Maintained ARM/NEC MOBILEPRO 900/c MACHINE SUPPORT diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 8a50efb559f3..b8450ed8d946 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -667,6 +667,8 @@ source "arch/arm/mach-mmp/Kconfig" source "arch/arm/mach-moxart/Kconfig" +source "arch/arm/mach-mstar/Kconfig" + source "arch/arm/mach-mv78xx0/Kconfig" source "arch/arm/mach-mvebu/Kconfig" diff --git a/arch/arm/Makefile b/arch/arm/Makefile index db857d07114f..2a3c127cd243 100644 --- a/arch/arm/Makefile +++ b/arch/arm/Makefile @@ -196,6 +196,7 @@ machine-$(CONFIG_ARCH_MXC) += imx machine-$(CONFIG_ARCH_MEDIATEK) += mediatek machine-$(CONFIG_ARCH_MILBEAUT) += milbeaut machine-$(CONFIG_ARCH_MXS) += mxs +machine-$(CONFIG_ARCH_MSTAR) += mstar machine-$(CONFIG_ARCH_NOMADIK) += nomadik machine-$(CONFIG_ARCH_NPCM) += npcm machine-$(CONFIG_ARCH_NSPIRE) += nspire diff --git a/arch/arm/mach-mstar/Kconfig b/arch/arm/mach-mstar/Kconfig new file mode 100644 index 000000000000..7bc79c296ebb --- /dev/null +++ b/arch/arm/mach-mstar/Kconfig @@ -0,0 +1,15 @@ +menuconfig ARCH_MSTAR + bool "MStar SoC Support" + depends on ARCH_MULTI_V7 + select ARM_GIC + help + Support for MStar ARMv7 SoCs + +if ARCH_MSTAR + +config MACH_INFINITY + bool "MStar infinity SoC support" + default ARCH_INFINITY + help + Support for MStar infinity(1/3) IP camera SoCs +endif diff --git a/arch/arm/mach-mstar/Makefile b/arch/arm/mach-mstar/Makefile new file mode 100644 index 000000000000..144b58b189e3 --- /dev/null +++ b/arch/arm/mach-mstar/Makefile @@ -0,0 +1 @@ +obj-$(CONFIG_MACH_INFINITY) += infinity.o diff --git a/arch/arm/mach-mstar/infinity.c b/arch/arm/mach-mstar/infinity.c new file mode 100644 index 000000000000..520581660bef --- /dev/null +++ b/arch/arm/mach-mstar/infinity.c @@ -0,0 +1,96 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Device Tree support for MStar Infinity SoCs + * + * Copyright (c) 2019 thingy.jp + * Author: Daniel Palmer <daniel@thingy.jp> + */ + +#include <linux/init.h> +#include <asm/mach/arch.h> +#include <asm/mach/map.h> +#include <linux/of.h> +#include <linux/io.h> + +/* + * The IO space is remapped to the same place + * the vendor kernel does so that the hardcoded + * addresses all over the vendor drivers line up. + */ + +#define INFINITY_IO_PHYS 0x1f000000 +#define INFINITY_IO_OFFSET 0xde000000 +#define INFINITY_IO_VIRT (INFINITY_IO_PHYS + INFINITY_IO_OFFSET) +#define INFINITY_IO_SIZE 0x00400000 + +/* + * In the u-boot code the area these registers are in is + * called "L3 bridge". + * + * It's not exactly known what is the L3 bridge is but + * the vendor code for both u-boot and linux share calls + * to "flush the miu pipe". This seems to be to force pending + * CPU writes to memory so that the state is right before + * DMA capable devices try to read descriptors and data + * the CPU has prepared. Without doing this ethernet doesn't + * work reliably for example. + */ + +#define INFINITY_L3BRIDGE_FLUSH 0x204414 +#define INFINITY_L3BRIDGE_STATUS 0x204440 +#define INFINITY_L3BRIDGE_FLUSH_TRIGGER BIT(0) +#define INFINITY_L3BRIDGE_STATUS_DONE BIT(12) + +static void __iomem *miu_status; +static void __iomem *miu_flush; + +static struct map_desc infinity_io_desc[] __initdata = { + {INFINITY_IO_VIRT, __phys_to_pfn(INFINITY_IO_PHYS), + INFINITY_IO_SIZE, MT_DEVICE}, +}; + +static void __init infinity_map_io(void) +{ + iotable_init(infinity_io_desc, ARRAY_SIZE(infinity_io_desc)); + miu_flush = (void __iomem *)(infinity_io_desc[0].virtual + + INFINITY_L3BRIDGE_FLUSH); + miu_status = (void __iomem *)(infinity_io_desc[0].virtual + + INFINITY_L3BRIDGE_STATUS); +} + +static const char * const infinity_board_dt_compat[] = { + "mstar,infinity", + NULL, +}; + +static DEFINE_SPINLOCK(infinity_mb_lock); + +static void infinity_mb(void) +{ + unsigned long flags; + + spin_lock_irqsave(&infinity_mb_lock, flags); + /* toggle the flush miu pipe fire bit */ + writel_relaxed(0, miu_flush); + writel_relaxed(INFINITY_L3BRIDGE_FLUSH_TRIGGER, miu_flush); + while (!(readl_relaxed(miu_status) & INFINITY_L3BRIDGE_STATUS_DONE)) { + /* wait for flush to complete */ + } + spin_unlock_irqrestore(&infinity_mb_lock, flags); +} + +static void __init infinity_barriers_init(void) +{ + soc_mb = infinity_mb; +} + +static void __init infinity_init(void) +{ + infinity_barriers_init(); +} + +DT_MACHINE_START(INFINITY_DT, "MStar Infinity (Device Tree)") + .dt_compat = infinity_board_dt_compat, + .init_machine = infinity_init, + .map_io = infinity_map_io, +MACHINE_END
Initial support for the MStar infinity/infinity3 series of Cortex A7 based IP camera SoCs. These chips are interesting in that they contain a Cortex A7, peripherals and system memory in a single tiny QFN package that can be hand soldered allowing almost anyone to embed Linux in their projects. Signed-off-by: Daniel Palmer <daniel@0x0f.com> --- MAINTAINERS | 1 + arch/arm/Kconfig | 2 + arch/arm/Makefile | 1 + arch/arm/mach-mstar/Kconfig | 15 ++++++ arch/arm/mach-mstar/Makefile | 1 + arch/arm/mach-mstar/infinity.c | 96 ++++++++++++++++++++++++++++++++++ 6 files changed, 116 insertions(+) create mode 100644 arch/arm/mach-mstar/Kconfig create mode 100644 arch/arm/mach-mstar/Makefile create mode 100644 arch/arm/mach-mstar/infinity.c