| Message ID | 20200610090421.3428945-3-daniel@0x0f.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Initial MStar/Sigmastar ARMv7 SoC support | expand |
On Wed, Jun 10, 2020 at 11:08 AM Daniel Palmer <daniel@0x0f.com> wrote: > +/* > + * This may need locking to deal with situations where an interrupt > + * happens while we are in here and mb() gets called by the interrupt handler. > + */ I would suspect that you don't need locking here, and locking would likely make it worse. From what I can tell, an interrupt happening anywhere inside of mstarv7_mb() would still result in the function completing (as miu_status still has the MSTARV7_L3BRIDGE_STATUS_DONE bit set) and nothing that could happen inside the irq would make the barrier weaker, only stronger. > +static void mstarv7_mb(void) > +{ > + /* toggle the flush miu pipe fire bit */ > + writel_relaxed(0, miu_flush); > + writel_relaxed(MSTARV7_L3BRIDGE_FLUSH_TRIGGER, miu_flush); > + while (!(readl_relaxed(miu_status) & MSTARV7_L3BRIDGE_STATUS_DONE)) { > + /* wait for flush to complete */ > + } > +} This is a heavy memory barrier indeed. The use of _relaxed() accessors is normally a bad idea and should be avoided, but this is one of the places where it is necessary because the normal writel()/readl() would recurse into arm_heavy_barrier(). Can you add a comment explaining this for the next reviewer? > +static void __init mstarv7_barriers_init(void) > +{ > + miu_flush = ioremap(MSTARV7_L3BRIDGE_FLUSH, sizeof(*miu_flush)); > + miu_status = ioremap(MSTARV7_L3BRIDGE_STATUS, sizeof(*miu_status)); > + soc_mb = mstarv7_mb; > +} Hardcoding physical addresses is generally considered bad style, even if you know the address in advance. Can you change this to get the address of the L3BRIDGE from DT instead and just hardcode the offsets? Note that they are in the same physical page, so you only need a single of_iomap(). > +static void __init mstarv7_init(void) > +{ > + mstarv7_barriers_init(); > +} I think you can fold this into a single function. Arnd
Am 10.06.20 um 11:04 schrieb Daniel Palmer: > Initial support for the MStar/Sigmastar infinity/mercury series of ARMv7 > based IP camera and dashcam SoCs. > > These chips are interesting in that they contain a Cortex A7, "Cortex-A7" > peripherals and system memory in a single tiny QFN package that > can be hand soldered allowing almost anyone to embed Linux "soldered, allowing"? > 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 | 26 +++++++++++++ > arch/arm/mach-mstar/Makefile | 1 + > arch/arm/mach-mstar/mstarv7.c | 72 +++++++++++++++++++++++++++++++++++ > 6 files changed, 103 insertions(+) > create mode 100644 arch/arm/mach-mstar/Kconfig > create mode 100644 arch/arm/mach-mstar/Makefile > create mode 100644 arch/arm/mach-mstar/mstarv7.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index 1ca77f97b8ee..754521938303 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -2114,6 +2114,7 @@ ARM/MStar/Sigmastar ARMv7 SoC support > M: Daniel Palmer <daniel@thingy.jp> > L: linux-arm-kernel@lists.infradead.org (moderated for non-subscribers) > S: Maintained > +F: arch/arm/mach-mstar/ > F: Documentation/devicetree/bindings/arm/mstar.yaml > > ARM/NEC MOBILEPRO 900/c MACHINE SUPPORT [snip] The sort order has recently been changed to case-sensitive, i.e., you should append arch after Documentation. Regards, Andreas
Am 10.06.20 um 11:04 schrieb Daniel Palmer: > diff --git a/arch/arm/Makefile b/arch/arm/Makefile > index 59fde2d598d8..e7f4ca060c0f 100644 > --- a/arch/arm/Makefile > +++ b/arch/arm/Makefile > @@ -197,6 +197,7 @@ machine-$(CONFIG_ARCH_MXC) += imx > machine-$(CONFIG_ARCH_MEDIATEK) += mediatek > machine-$(CONFIG_ARCH_MILBEAUT) += milbeaut > machine-$(CONFIG_ARCH_MXS) += mxs > +machine-$(CONFIG_ARCH_MSTARV7) += 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..6235d0a7860a > --- /dev/null > +++ b/arch/arm/mach-mstar/Kconfig > @@ -0,0 +1,26 @@ > +menuconfig ARCH_MSTARV7 You call the dir mach-mstar, but name the Kconfig ARCH_MSTARV7. I had previously been asked to just use the vendor name, so this should probably be just ARCH_MSTAR. Outside arch/arm/ you can then use ARM && ARCH_MSTAR condition to make things 32-bit only, allowing to reuse ARCH_MSTAR for arm64 or whatever. > + bool "MStar/Sigmastar ARMv7 SoC Support" > + depends on ARCH_MULTI_V7 > + select ARM_GIC > + select ARM_HEAVY_MB > + help > + Support for newer MStar/Sigmastar SoC families that are > + based on ARMv7 cores like the Cortex A7 and share the same > + basic hardware like the infinity and mercury series. > + > +if ARCH_MSTARV7 > + > +config MACH_INFINITY > + bool "MStar/Sigmastar infinity SoC support" > + default ARCH_MSTARV7 > + help > + Support for MStar/Sigmastar infinity IP camera SoCs. > + > +config MACH_MERCURY > + bool "MStar/Sigmastar mercury SoC support" > + default ARCH_MSTARV7 > + help > + Support for MStar/Sigmastar mercury dash camera SoCs. > + Note that older Mercury2 SoCs are ARM9 based and not supported. Is this comment really helpful? This menu item would only seem to come up after having selected multi_v7, which kind of rules out ARM9. Consider adding mercury in a second step? > + > +endif Regards, Andreas
Hi Andreas, On Thu, 11 Jun 2020 at 21:49, Andreas Färber <afaerber@suse.de> wrote: > > peripherals and system memory in a single tiny QFN package that > > can be hand soldered allowing almost anyone to embed Linux > > "soldered, allowing"? The original reads ok to me. Maybe I can just split that into two sentences? Like ".. QFN package that can be hand soldered. This allows almost anyone..". > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -2114,6 +2114,7 @@ ARM/MStar/Sigmastar ARMv7 SoC support > > M: Daniel Palmer <daniel@thingy.jp> > > L: linux-arm-kernel@lists.infradead.org (moderated for non-subscribers) > > S: Maintained > > +F: arch/arm/mach-mstar/ > > F: Documentation/devicetree/bindings/arm/mstar.yaml > > > > ARM/NEC MOBILEPRO 900/c MACHINE SUPPORT > [snip] > > The sort order has recently been changed to case-sensitive, i.e., you > should append arch after Documentation. Interesting. Checkpatch didn't complain about that although it complained about the original ordering I had. Thanks for the input. Daniel
Hi Andreas, On Thu, 11 Jun 2020 at 21:58, Andreas Färber <afaerber@suse.de> wrote: > You call the dir mach-mstar, but name the Kconfig ARCH_MSTARV7. I had > previously been asked to just use the vendor name, so this should > probably be just ARCH_MSTAR. Outside arch/arm/ you can then use ARM && > ARCH_MSTAR condition to make things 32-bit only, allowing to reuse > ARCH_MSTAR for arm64 or whatever. The ARM9 MStar chips before they switched to a common ARMv7 base aren't directly compatible so I thought there should be some distinction there. I doubt anyone will do it but I made the directory mach-mstar so potentially someone could add machine support for the older stuff to without having more directories. > > + bool "MStar/Sigmastar ARMv7 SoC Support" > > + depends on ARCH_MULTI_V7 > > + select ARM_GIC > > + select ARM_HEAVY_MB > > + help > > + Support for newer MStar/Sigmastar SoC families that are > > + based on ARMv7 cores like the Cortex A7 and share the same > > + basic hardware like the infinity and mercury series. > > + > > +if ARCH_MSTARV7 > > + > > +config MACH_INFINITY > > + bool "MStar/Sigmastar infinity SoC support" > > + default ARCH_MSTARV7 > > + help > > + Support for MStar/Sigmastar infinity IP camera SoCs. > > + > > +config MACH_MERCURY > > + bool "MStar/Sigmastar mercury SoC support" > > + default ARCH_MSTARV7 > > + help > > + Support for MStar/Sigmastar mercury dash camera SoCs. > > + Note that older Mercury2 SoCs are ARM9 based and not supported. > > Is this comment really helpful? This menu item would only seem to come > up after having selected multi_v7, which kind of rules out ARM9. The older mercury2 based chips seem to still be available and used in drive recorders that are on the market right now. The infinity series is all ARMv7 so can be supported but for the mercury series only the newer ones are ARMv7 so I thought it was worth mentioning that "mercury SoC support" doesn't mean all of them. I'll take it out if you think it's unnecessary. > Consider adding mercury in a second step? I'll think about that. I wanted to try to get a machine that isn't one I'm personally making into the series. Cheers, Daniel
Hi Daniel, Am 11.06.20 um 15:01 schrieb Daniel Palmer: > On Thu, 11 Jun 2020 at 21:49, Andreas Färber <afaerber@suse.de> wrote: >>> peripherals and system memory in a single tiny QFN package that >>> can be hand soldered allowing almost anyone to embed Linux >> >> "soldered, allowing"? > > The original reads ok to me. Maybe I can just split that into two sentences? > Like ".. QFN package that can be hand soldered. This allows almost anyone..". As non-native speaker I merely wondered whether a comma should better be inserted to separate the two parts of the sentence. Splitting it in two or leaving as is should be fine, too - I assume you're a native speaker. Most people will rather read the bindings document than old git history, so you might want to consider adding such a description below its title. >>> --- a/MAINTAINERS >>> +++ b/MAINTAINERS >>> @@ -2114,6 +2114,7 @@ ARM/MStar/Sigmastar ARMv7 SoC support >>> M: Daniel Palmer <daniel@thingy.jp> >>> L: linux-arm-kernel@lists.infradead.org (moderated for non-subscribers) >>> S: Maintained >>> +F: arch/arm/mach-mstar/ >>> F: Documentation/devicetree/bindings/arm/mstar.yaml >>> >>> ARM/NEC MOBILEPRO 900/c MACHINE SUPPORT >> [snip] >> >> The sort order has recently been changed to case-sensitive, i.e., you >> should append arch after Documentation. > > Interesting. Checkpatch didn't complain about that although it > complained about the > original ordering I had. I only noticed because someone refactored my Realtek section, causing a merge conflict. https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=3b50142d8528e1efc1c07f69c540f926c58ab3ad Which reminds me, in 1/5 you should probably add a W: line (after S: according to above sort commit) pointing to your http://linux-chenxing.org/ website. And for the community following your project, you may want to set up a linux-chenxing mailing list on vger.kernel.org or on infradead.org and add it as L:, to allow for error reports and patches to not just go to you and crowded LAKML. Cheers, Andreas
Hi Andreas, On Thu, 11 Jun 2020 at 23:27, Andreas Färber <afaerber@suse.de> wrote: > > Hi Daniel, > > Am 11.06.20 um 15:01 schrieb Daniel Palmer: > > On Thu, 11 Jun 2020 at 21:49, Andreas Färber <afaerber@suse.de> wrote: > >>> peripherals and system memory in a single tiny QFN package that > >>> can be hand soldered allowing almost anyone to embed Linux > >> > >> "soldered, allowing"? > > > > The original reads ok to me. Maybe I can just split that into two sentences? > > Like ".. QFN package that can be hand soldered. This allows almost anyone..". > > As non-native speaker I merely wondered whether a comma should better be > inserted to separate the two parts of the sentence. Splitting it in two > or leaving as is should be fine, too - I assume you're a native speaker. I'm a native speaker but it's not my daily driver anymore so I often mangle it. > Most people will rather read the bindings document than old git history, > so you might want to consider adding such a description below its title. I'll move the blurb and maybe reword it. > Which reminds me, in 1/5 you should probably add a W: line (after S: > according to above sort commit) pointing to your > http://linux-chenxing.org/ website. > > And for the community following your project, you may want to set up a > linux-chenxing mailing list on vger.kernel.org or on infradead.org and > add it as L:, to allow for error reports and patches to not just go to > you and crowded LAKML. Very good points. I was thinking I should probably get this into mainline before setting up lists etc. Thanks, Daniel
diff --git a/MAINTAINERS b/MAINTAINERS index 1ca77f97b8ee..754521938303 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2114,6 +2114,7 @@ ARM/MStar/Sigmastar ARMv7 SoC support M: Daniel Palmer <daniel@thingy.jp> L: linux-arm-kernel@lists.infradead.org (moderated for non-subscribers) S: Maintained +F: arch/arm/mach-mstar/ F: Documentation/devicetree/bindings/arm/mstar.yaml ARM/NEC MOBILEPRO 900/c MACHINE SUPPORT diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index fb6c85c5d344..e466694f8486 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -669,6 +669,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 59fde2d598d8..e7f4ca060c0f 100644 --- a/arch/arm/Makefile +++ b/arch/arm/Makefile @@ -197,6 +197,7 @@ machine-$(CONFIG_ARCH_MXC) += imx machine-$(CONFIG_ARCH_MEDIATEK) += mediatek machine-$(CONFIG_ARCH_MILBEAUT) += milbeaut machine-$(CONFIG_ARCH_MXS) += mxs +machine-$(CONFIG_ARCH_MSTARV7) += 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..6235d0a7860a --- /dev/null +++ b/arch/arm/mach-mstar/Kconfig @@ -0,0 +1,26 @@ +menuconfig ARCH_MSTARV7 + bool "MStar/Sigmastar ARMv7 SoC Support" + depends on ARCH_MULTI_V7 + select ARM_GIC + select ARM_HEAVY_MB + help + Support for newer MStar/Sigmastar SoC families that are + based on ARMv7 cores like the Cortex A7 and share the same + basic hardware like the infinity and mercury series. + +if ARCH_MSTARV7 + +config MACH_INFINITY + bool "MStar/Sigmastar infinity SoC support" + default ARCH_MSTARV7 + help + Support for MStar/Sigmastar infinity IP camera SoCs. + +config MACH_MERCURY + bool "MStar/Sigmastar mercury SoC support" + default ARCH_MSTARV7 + help + Support for MStar/Sigmastar mercury dash camera SoCs. + Note that older Mercury2 SoCs are ARM9 based and not supported. + +endif diff --git a/arch/arm/mach-mstar/Makefile b/arch/arm/mach-mstar/Makefile new file mode 100644 index 000000000000..93b0391ede7e --- /dev/null +++ b/arch/arm/mach-mstar/Makefile @@ -0,0 +1 @@ +obj-$(CONFIG_ARCH_MSTARV7) += mstarv7.o diff --git a/arch/arm/mach-mstar/mstarv7.c b/arch/arm/mach-mstar/mstarv7.c new file mode 100644 index 000000000000..ee96ce46cbbc --- /dev/null +++ b/arch/arm/mach-mstar/mstarv7.c @@ -0,0 +1,72 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Device Tree support for MStar/Sigmastar ARMv7 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> + +/* + * In the u-boot code the area these registers are in is + * called "L3 bridge" and there are register descriptions + * for something in the same area called "AXI". + * + * It's not exactly known what this 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 MSTARV7_L3BRIDGE_FLUSH 0x1f204414 +#define MSTARV7_L3BRIDGE_STATUS 0x1f204440 +#define MSTARV7_L3BRIDGE_FLUSH_TRIGGER BIT(0) +#define MSTARV7_L3BRIDGE_STATUS_DONE BIT(12) + +static u32 __iomem *miu_status; +static u32 __iomem *miu_flush; + +static const char * const mstarv7_board_dt_compat[] __initconst = { + "mstar,infinity", + "mstar,infinity3", + "mstar,mercury5", + NULL, +}; + +/* + * This may need locking to deal with situations where an interrupt + * happens while we are in here and mb() gets called by the interrupt handler. + */ +static void mstarv7_mb(void) +{ + /* toggle the flush miu pipe fire bit */ + writel_relaxed(0, miu_flush); + writel_relaxed(MSTARV7_L3BRIDGE_FLUSH_TRIGGER, miu_flush); + while (!(readl_relaxed(miu_status) & MSTARV7_L3BRIDGE_STATUS_DONE)) { + /* wait for flush to complete */ + } +} + +static void __init mstarv7_barriers_init(void) +{ + miu_flush = ioremap(MSTARV7_L3BRIDGE_FLUSH, sizeof(*miu_flush)); + miu_status = ioremap(MSTARV7_L3BRIDGE_STATUS, sizeof(*miu_status)); + soc_mb = mstarv7_mb; +} + +static void __init mstarv7_init(void) +{ + mstarv7_barriers_init(); +} + +DT_MACHINE_START(MSTARV7_DT, "MStar/Sigmastar ARMv7 (Device Tree)") + .dt_compat = mstarv7_board_dt_compat, + .init_machine = mstarv7_init, +MACHINE_END
Initial support for the MStar/Sigmastar infinity/mercury series of ARMv7 based IP camera and dashcam 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 | 26 +++++++++++++ arch/arm/mach-mstar/Makefile | 1 + arch/arm/mach-mstar/mstarv7.c | 72 +++++++++++++++++++++++++++++++++++ 6 files changed, 103 insertions(+) create mode 100644 arch/arm/mach-mstar/Kconfig create mode 100644 arch/arm/mach-mstar/Makefile create mode 100644 arch/arm/mach-mstar/mstarv7.c