| Message ID | 1440470959-29744-1-git-send-email-wens@csie.org |
|---|---|
| State | Accepted |
| Delegated to: | Hans de Goede |
| Headers | show |
On Tue, 25 Aug 2015 10:49:19 +0800 Chen-Yu Tsai <wens@csie.org> wrote: Hi, Thanks for putting this patch together. A few comments below: > On the A31s the RTC is by default secured. Thus when u-boot > loads the kernel in non-secure world, the RTC is unavailable. The > SoC has a TrustZone Protection Controller, which can be used to > enable non-secure access to the RTC. > > On the A31 the TZPC doesn't seem to do anything, i.e. changes to > its register contents do not affect access to the RTC. Does it mean that the RTC is still inaccessible? Or that it has always been accessible? > > Signed-off-by: Chen-Yu Tsai <wens@csie.org> > --- > arch/arm/cpu/armv7/sunxi/Makefile | 1 + > arch/arm/cpu/armv7/sunxi/board.c | 5 +++++ > arch/arm/cpu/armv7/sunxi/tzpc.c | 18 ++++++++++++++++++ > arch/arm/include/asm/arch-sunxi/tzpc.h | 23 +++++++++++++++++++++++ > 4 files changed, 47 insertions(+) > create mode 100644 arch/arm/cpu/armv7/sunxi/tzpc.c > create mode 100644 arch/arm/include/asm/arch-sunxi/tzpc.h > > diff --git a/arch/arm/cpu/armv7/sunxi/Makefile b/arch/arm/cpu/armv7/sunxi/Makefile > index 76c7e55..459d5d8 100644 > --- a/arch/arm/cpu/armv7/sunxi/Makefile > +++ b/arch/arm/cpu/armv7/sunxi/Makefile > @@ -28,6 +28,7 @@ obj-$(CONFIG_MACH_SUN6I) += clock_sun6i.o > obj-$(CONFIG_MACH_SUN7I) += clock_sun4i.o > obj-$(CONFIG_MACH_SUN8I) += clock_sun6i.o > obj-$(CONFIG_MACH_SUN9I) += clock_sun9i.o > +obj-$(CONFIG_MACH_SUN6I) += tzpc.o > > obj-$(CONFIG_AXP152_POWER) += pmic_bus.o > obj-$(CONFIG_AXP209_POWER) += pmic_bus.o > diff --git a/arch/arm/cpu/armv7/sunxi/board.c b/arch/arm/cpu/armv7/sunxi/board.c > index f01846e..b40198b 100644 > --- a/arch/arm/cpu/armv7/sunxi/board.c > +++ b/arch/arm/cpu/armv7/sunxi/board.c > @@ -23,6 +23,7 @@ > #include <asm/arch/gpio.h> > #include <asm/arch/sys_proto.h> > #include <asm/arch/timer.h> > +#include <asm/arch/tzpc.h> > #include <asm/arch/mmc.h> > > #include <linux/compiler.h> > @@ -115,6 +116,10 @@ void s_init(void) > "orr r0, r0, #1 << 6\n" > "mcr p15, 0, r0, c1, c0, 1\n"); > #endif > +#if defined CONFIG_MACH_SUN6I > + /* Enable non-secure access to the RTC */ > + tzpc_init(); > +#endif > > clock_init(); > timer_init(); > diff --git a/arch/arm/cpu/armv7/sunxi/tzpc.c b/arch/arm/cpu/armv7/sunxi/tzpc.c > new file mode 100644 > index 0000000..5c9c69b > --- /dev/null > +++ b/arch/arm/cpu/armv7/sunxi/tzpc.c > @@ -0,0 +1,18 @@ > +/* > + * (C) Copyright 2015 Chen-Yu Tsai <wens@csie.org> > + * > + * SPDX-License-Identifier: GPL-2.0+ > + */ > + > +#include <asm/io.h> > +#include <asm/arch/cpu.h> > +#include <asm/arch/tzpc.h> > + > +/* Configure Trust Zone Protection Controller */ > +void tzpc_init(void) > +{ > + struct sunxi_tzpc *tzpc = (struct sunxi_tzpc *)SUNXI_TZPC_BASE; > + > + /* Enable non-secure access to the RTC */ > + writel(SUNXI_TZPC_DECPORT0_RTC, &tzpc->decport0_set); Irk. Seem blow. > +} > diff --git a/arch/arm/include/asm/arch-sunxi/tzpc.h b/arch/arm/include/asm/arch-sunxi/tzpc.h > new file mode 100644 > index 0000000..ba4d43b > --- /dev/null > +++ b/arch/arm/include/asm/arch-sunxi/tzpc.h > @@ -0,0 +1,23 @@ > +/* > + * (C) Copyright 2015 Chen-Yu Tsai <wens@csie.org> > + * > + * SPDX-License-Identifier: GPL-2.0+ > + */ > + > +#ifndef _SUNXI_TZPC_H > +#define _SUNXI_TZPC_H Why is this sunxi specific? This seems to describe a standard ARM BP147 (http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dto0015a/index.html), so I'd expect the various offsets and the file location to reflect this. Another platform (FSL 2085) seems to use the same IP, so it would make sense to clean that part of the code too. > + > +#ifndef __ASSEMBLY__ > +struct sunxi_tzpc { > + u32 r0size; /* 0x00 Size of secure RAM region */ > + u32 decport0_status; /* 0x04 Status of decode protection port 0 */ > + u32 decport0_set; /* 0x08 Set decode protection port 0 */ > + u32 decport0_clear; /* 0x0c Clear decode protection port 0 */ > +}; I'm not overly fond of this way to describe a set of contiguous register. It tends to be fairly fragile (the compiler is free to insert some padding) and prevents the use of offsets from assembly code. > +#endif > + > +#define SUNXI_TZPC_DECPORT0_RTC (1 << 1) > + > +void tzpc_init(void); And this won't help if you have assembly code either. > + > +#endif /* _SUNXI_TZPC_H */ If I can suggest something, it would be to make this something generic, with a standard config option describing the base, a set of offsets from the base (as #defines), and then a set of accessors accessible from C code. LS2085 could also be cleaned up to reuse the same offsets (instead of hardcoding the register addresses). Thanks, M.
On Tue, Aug 25, 2015 at 3:34 PM, Marc Zyngier <marc.zyngier@arm.com> wrote: > On Tue, 25 Aug 2015 10:49:19 +0800 > Chen-Yu Tsai <wens@csie.org> wrote: > > Hi, > > Thanks for putting this patch together. A few comments below: > >> On the A31s the RTC is by default secured. Thus when u-boot >> loads the kernel in non-secure world, the RTC is unavailable. The >> SoC has a TrustZone Protection Controller, which can be used to >> enable non-secure access to the RTC. >> >> On the A31 the TZPC doesn't seem to do anything, i.e. changes to >> its register contents do not affect access to the RTC. > > Does it mean that the RTC is still inaccessible? Or that it has always > been accessible? Means its always accessible. >> >> Signed-off-by: Chen-Yu Tsai <wens@csie.org> >> --- >> arch/arm/cpu/armv7/sunxi/Makefile | 1 + >> arch/arm/cpu/armv7/sunxi/board.c | 5 +++++ >> arch/arm/cpu/armv7/sunxi/tzpc.c | 18 ++++++++++++++++++ >> arch/arm/include/asm/arch-sunxi/tzpc.h | 23 +++++++++++++++++++++++ >> 4 files changed, 47 insertions(+) >> create mode 100644 arch/arm/cpu/armv7/sunxi/tzpc.c >> create mode 100644 arch/arm/include/asm/arch-sunxi/tzpc.h >> >> diff --git a/arch/arm/cpu/armv7/sunxi/Makefile b/arch/arm/cpu/armv7/sunxi/Makefile >> index 76c7e55..459d5d8 100644 >> --- a/arch/arm/cpu/armv7/sunxi/Makefile >> +++ b/arch/arm/cpu/armv7/sunxi/Makefile >> @@ -28,6 +28,7 @@ obj-$(CONFIG_MACH_SUN6I) += clock_sun6i.o >> obj-$(CONFIG_MACH_SUN7I) += clock_sun4i.o >> obj-$(CONFIG_MACH_SUN8I) += clock_sun6i.o >> obj-$(CONFIG_MACH_SUN9I) += clock_sun9i.o >> +obj-$(CONFIG_MACH_SUN6I) += tzpc.o >> >> obj-$(CONFIG_AXP152_POWER) += pmic_bus.o >> obj-$(CONFIG_AXP209_POWER) += pmic_bus.o >> diff --git a/arch/arm/cpu/armv7/sunxi/board.c b/arch/arm/cpu/armv7/sunxi/board.c >> index f01846e..b40198b 100644 >> --- a/arch/arm/cpu/armv7/sunxi/board.c >> +++ b/arch/arm/cpu/armv7/sunxi/board.c >> @@ -23,6 +23,7 @@ >> #include <asm/arch/gpio.h> >> #include <asm/arch/sys_proto.h> >> #include <asm/arch/timer.h> >> +#include <asm/arch/tzpc.h> >> #include <asm/arch/mmc.h> >> >> #include <linux/compiler.h> >> @@ -115,6 +116,10 @@ void s_init(void) >> "orr r0, r0, #1 << 6\n" >> "mcr p15, 0, r0, c1, c0, 1\n"); >> #endif >> +#if defined CONFIG_MACH_SUN6I >> + /* Enable non-secure access to the RTC */ >> + tzpc_init(); >> +#endif >> >> clock_init(); >> timer_init(); >> diff --git a/arch/arm/cpu/armv7/sunxi/tzpc.c b/arch/arm/cpu/armv7/sunxi/tzpc.c >> new file mode 100644 >> index 0000000..5c9c69b >> --- /dev/null >> +++ b/arch/arm/cpu/armv7/sunxi/tzpc.c >> @@ -0,0 +1,18 @@ >> +/* >> + * (C) Copyright 2015 Chen-Yu Tsai <wens@csie.org> >> + * >> + * SPDX-License-Identifier: GPL-2.0+ >> + */ >> + >> +#include <asm/io.h> >> +#include <asm/arch/cpu.h> >> +#include <asm/arch/tzpc.h> >> + >> +/* Configure Trust Zone Protection Controller */ >> +void tzpc_init(void) >> +{ >> + struct sunxi_tzpc *tzpc = (struct sunxi_tzpc *)SUNXI_TZPC_BASE; >> + >> + /* Enable non-secure access to the RTC */ >> + writel(SUNXI_TZPC_DECPORT0_RTC, &tzpc->decport0_set); > > Irk. Seem blow. > >> +} >> diff --git a/arch/arm/include/asm/arch-sunxi/tzpc.h b/arch/arm/include/asm/arch-sunxi/tzpc.h >> new file mode 100644 >> index 0000000..ba4d43b >> --- /dev/null >> +++ b/arch/arm/include/asm/arch-sunxi/tzpc.h >> @@ -0,0 +1,23 @@ >> +/* >> + * (C) Copyright 2015 Chen-Yu Tsai <wens@csie.org> >> + * >> + * SPDX-License-Identifier: GPL-2.0+ >> + */ >> + >> +#ifndef _SUNXI_TZPC_H >> +#define _SUNXI_TZPC_H > > Why is this sunxi specific? This seems to describe a standard ARM BP147 > (http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dto0015a/index.html), > so I'd expect the various offsets and the file location to reflect this. > > Another platform (FSL 2085) seems to use the same IP, so it would make > sense to clean that part of the code too. I guess we could make this generic. Seems samsung uses this as well. >> + >> +#ifndef __ASSEMBLY__ >> +struct sunxi_tzpc { >> + u32 r0size; /* 0x00 Size of secure RAM region */ >> + u32 decport0_status; /* 0x04 Status of decode protection port 0 */ >> + u32 decport0_set; /* 0x08 Set decode protection port 0 */ >> + u32 decport0_clear; /* 0x0c Clear decode protection port 0 */ >> +}; > > I'm not overly fond of this way to describe a set of contiguous > register. It tends to be fairly fragile (the compiler is free to > insert some padding) and prevents the use of offsets from assembly code. Seems this is how we are doing things in U-boot... I'm OK either way. >> +#endif >> + >> +#define SUNXI_TZPC_DECPORT0_RTC (1 << 1) >> + >> +void tzpc_init(void); > > And this won't help if you have assembly code either. > >> + >> +#endif /* _SUNXI_TZPC_H */ > > If I can suggest something, it would be to make this something generic, > with a standard config option describing the base, a set of offsets > from the base (as #defines), and then a set of accessors accessible from > C code. > > LS2085 could also be cleaned up to reuse the same offsets (instead of > hardcoding the register addresses). OK. Will look into it. ChenYu
Hi, On 25-08-15 09:40, Chen-Yu Tsai wrote: > On Tue, Aug 25, 2015 at 3:34 PM, Marc Zyngier <marc.zyngier@arm.com> wrote: >> On Tue, 25 Aug 2015 10:49:19 +0800 >> Chen-Yu Tsai <wens@csie.org> wrote: >> >> Hi, >> >> Thanks for putting this patch together. A few comments below: >> >>> On the A31s the RTC is by default secured. Thus when u-boot >>> loads the kernel in non-secure world, the RTC is unavailable. The >>> SoC has a TrustZone Protection Controller, which can be used to >>> enable non-secure access to the RTC. >>> >>> On the A31 the TZPC doesn't seem to do anything, i.e. changes to >>> its register contents do not affect access to the RTC. >> >> Does it mean that the RTC is still inaccessible? Or that it has always >> been accessible? > > Means its always accessible. Ah, so what you're saying is that on A31 the RTC is always accessible and on A31s it depends on the TZPC settings ? That does not make sense since both are the same die used in a different package, so chances are this does not depend on A31 vs A31s but on a difference in revision of the die. It does not matter, we can just flip the bit in the TZPC everywhere to make sure the kernel has access to the RTC. Regards, Hans > >>> >>> Signed-off-by: Chen-Yu Tsai <wens@csie.org> >>> --- >>> arch/arm/cpu/armv7/sunxi/Makefile | 1 + >>> arch/arm/cpu/armv7/sunxi/board.c | 5 +++++ >>> arch/arm/cpu/armv7/sunxi/tzpc.c | 18 ++++++++++++++++++ >>> arch/arm/include/asm/arch-sunxi/tzpc.h | 23 +++++++++++++++++++++++ >>> 4 files changed, 47 insertions(+) >>> create mode 100644 arch/arm/cpu/armv7/sunxi/tzpc.c >>> create mode 100644 arch/arm/include/asm/arch-sunxi/tzpc.h >>> >>> diff --git a/arch/arm/cpu/armv7/sunxi/Makefile b/arch/arm/cpu/armv7/sunxi/Makefile >>> index 76c7e55..459d5d8 100644 >>> --- a/arch/arm/cpu/armv7/sunxi/Makefile >>> +++ b/arch/arm/cpu/armv7/sunxi/Makefile >>> @@ -28,6 +28,7 @@ obj-$(CONFIG_MACH_SUN6I) += clock_sun6i.o >>> obj-$(CONFIG_MACH_SUN7I) += clock_sun4i.o >>> obj-$(CONFIG_MACH_SUN8I) += clock_sun6i.o >>> obj-$(CONFIG_MACH_SUN9I) += clock_sun9i.o >>> +obj-$(CONFIG_MACH_SUN6I) += tzpc.o >>> >>> obj-$(CONFIG_AXP152_POWER) += pmic_bus.o >>> obj-$(CONFIG_AXP209_POWER) += pmic_bus.o >>> diff --git a/arch/arm/cpu/armv7/sunxi/board.c b/arch/arm/cpu/armv7/sunxi/board.c >>> index f01846e..b40198b 100644 >>> --- a/arch/arm/cpu/armv7/sunxi/board.c >>> +++ b/arch/arm/cpu/armv7/sunxi/board.c >>> @@ -23,6 +23,7 @@ >>> #include <asm/arch/gpio.h> >>> #include <asm/arch/sys_proto.h> >>> #include <asm/arch/timer.h> >>> +#include <asm/arch/tzpc.h> >>> #include <asm/arch/mmc.h> >>> >>> #include <linux/compiler.h> >>> @@ -115,6 +116,10 @@ void s_init(void) >>> "orr r0, r0, #1 << 6\n" >>> "mcr p15, 0, r0, c1, c0, 1\n"); >>> #endif >>> +#if defined CONFIG_MACH_SUN6I >>> + /* Enable non-secure access to the RTC */ >>> + tzpc_init(); >>> +#endif >>> >>> clock_init(); >>> timer_init(); >>> diff --git a/arch/arm/cpu/armv7/sunxi/tzpc.c b/arch/arm/cpu/armv7/sunxi/tzpc.c >>> new file mode 100644 >>> index 0000000..5c9c69b >>> --- /dev/null >>> +++ b/arch/arm/cpu/armv7/sunxi/tzpc.c >>> @@ -0,0 +1,18 @@ >>> +/* >>> + * (C) Copyright 2015 Chen-Yu Tsai <wens@csie.org> >>> + * >>> + * SPDX-License-Identifier: GPL-2.0+ >>> + */ >>> + >>> +#include <asm/io.h> >>> +#include <asm/arch/cpu.h> >>> +#include <asm/arch/tzpc.h> >>> + >>> +/* Configure Trust Zone Protection Controller */ >>> +void tzpc_init(void) >>> +{ >>> + struct sunxi_tzpc *tzpc = (struct sunxi_tzpc *)SUNXI_TZPC_BASE; >>> + >>> + /* Enable non-secure access to the RTC */ >>> + writel(SUNXI_TZPC_DECPORT0_RTC, &tzpc->decport0_set); >> >> Irk. Seem blow. >> >>> +} >>> diff --git a/arch/arm/include/asm/arch-sunxi/tzpc.h b/arch/arm/include/asm/arch-sunxi/tzpc.h >>> new file mode 100644 >>> index 0000000..ba4d43b >>> --- /dev/null >>> +++ b/arch/arm/include/asm/arch-sunxi/tzpc.h >>> @@ -0,0 +1,23 @@ >>> +/* >>> + * (C) Copyright 2015 Chen-Yu Tsai <wens@csie.org> >>> + * >>> + * SPDX-License-Identifier: GPL-2.0+ >>> + */ >>> + >>> +#ifndef _SUNXI_TZPC_H >>> +#define _SUNXI_TZPC_H >> >> Why is this sunxi specific? This seems to describe a standard ARM BP147 >> (http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dto0015a/index.html), >> so I'd expect the various offsets and the file location to reflect this. >> >> Another platform (FSL 2085) seems to use the same IP, so it would make >> sense to clean that part of the code too. > > I guess we could make this generic. Seems samsung uses this as well. > >>> + >>> +#ifndef __ASSEMBLY__ >>> +struct sunxi_tzpc { >>> + u32 r0size; /* 0x00 Size of secure RAM region */ >>> + u32 decport0_status; /* 0x04 Status of decode protection port 0 */ >>> + u32 decport0_set; /* 0x08 Set decode protection port 0 */ >>> + u32 decport0_clear; /* 0x0c Clear decode protection port 0 */ >>> +}; >> >> I'm not overly fond of this way to describe a set of contiguous >> register. It tends to be fairly fragile (the compiler is free to >> insert some padding) and prevents the use of offsets from assembly code. > > Seems this is how we are doing things in U-boot... > I'm OK either way. > >>> +#endif >>> + >>> +#define SUNXI_TZPC_DECPORT0_RTC (1 << 1) >>> + >>> +void tzpc_init(void); >> >> And this won't help if you have assembly code either. >> >>> + >>> +#endif /* _SUNXI_TZPC_H */ >> >> If I can suggest something, it would be to make this something generic, >> with a standard config option describing the base, a set of offsets >> from the base (as #defines), and then a set of accessors accessible from >> C code. >> >> LS2085 could also be cleaned up to reuse the same offsets (instead of >> hardcoding the register addresses). > > OK. Will look into it. > > ChenYu >
On 25/08/15 08:40, Chen-Yu Tsai wrote: > On Tue, Aug 25, 2015 at 3:34 PM, Marc Zyngier <marc.zyngier@arm.com> wrote: >> On Tue, 25 Aug 2015 10:49:19 +0800 >> Chen-Yu Tsai <wens@csie.org> wrote: >> >> Hi, >> >> Thanks for putting this patch together. A few comments below: >> >>> On the A31s the RTC is by default secured. Thus when u-boot >>> loads the kernel in non-secure world, the RTC is unavailable. The >>> SoC has a TrustZone Protection Controller, which can be used to >>> enable non-secure access to the RTC. >>> >>> On the A31 the TZPC doesn't seem to do anything, i.e. changes to >>> its register contents do not affect access to the RTC. >> >> Does it mean that the RTC is still inaccessible? Or that it has always >> been accessible? > > Means its always accessible. > >>> >>> Signed-off-by: Chen-Yu Tsai <wens@csie.org> >>> --- >>> arch/arm/cpu/armv7/sunxi/Makefile | 1 + >>> arch/arm/cpu/armv7/sunxi/board.c | 5 +++++ >>> arch/arm/cpu/armv7/sunxi/tzpc.c | 18 ++++++++++++++++++ >>> arch/arm/include/asm/arch-sunxi/tzpc.h | 23 +++++++++++++++++++++++ >>> 4 files changed, 47 insertions(+) >>> create mode 100644 arch/arm/cpu/armv7/sunxi/tzpc.c >>> create mode 100644 arch/arm/include/asm/arch-sunxi/tzpc.h >>> >>> diff --git a/arch/arm/cpu/armv7/sunxi/Makefile b/arch/arm/cpu/armv7/sunxi/Makefile >>> index 76c7e55..459d5d8 100644 >>> --- a/arch/arm/cpu/armv7/sunxi/Makefile >>> +++ b/arch/arm/cpu/armv7/sunxi/Makefile >>> @@ -28,6 +28,7 @@ obj-$(CONFIG_MACH_SUN6I) += clock_sun6i.o >>> obj-$(CONFIG_MACH_SUN7I) += clock_sun4i.o >>> obj-$(CONFIG_MACH_SUN8I) += clock_sun6i.o >>> obj-$(CONFIG_MACH_SUN9I) += clock_sun9i.o >>> +obj-$(CONFIG_MACH_SUN6I) += tzpc.o >>> >>> obj-$(CONFIG_AXP152_POWER) += pmic_bus.o >>> obj-$(CONFIG_AXP209_POWER) += pmic_bus.o >>> diff --git a/arch/arm/cpu/armv7/sunxi/board.c b/arch/arm/cpu/armv7/sunxi/board.c >>> index f01846e..b40198b 100644 >>> --- a/arch/arm/cpu/armv7/sunxi/board.c >>> +++ b/arch/arm/cpu/armv7/sunxi/board.c >>> @@ -23,6 +23,7 @@ >>> #include <asm/arch/gpio.h> >>> #include <asm/arch/sys_proto.h> >>> #include <asm/arch/timer.h> >>> +#include <asm/arch/tzpc.h> >>> #include <asm/arch/mmc.h> >>> >>> #include <linux/compiler.h> >>> @@ -115,6 +116,10 @@ void s_init(void) >>> "orr r0, r0, #1 << 6\n" >>> "mcr p15, 0, r0, c1, c0, 1\n"); >>> #endif >>> +#if defined CONFIG_MACH_SUN6I >>> + /* Enable non-secure access to the RTC */ >>> + tzpc_init(); >>> +#endif >>> >>> clock_init(); >>> timer_init(); >>> diff --git a/arch/arm/cpu/armv7/sunxi/tzpc.c b/arch/arm/cpu/armv7/sunxi/tzpc.c >>> new file mode 100644 >>> index 0000000..5c9c69b >>> --- /dev/null >>> +++ b/arch/arm/cpu/armv7/sunxi/tzpc.c >>> @@ -0,0 +1,18 @@ >>> +/* >>> + * (C) Copyright 2015 Chen-Yu Tsai <wens@csie.org> >>> + * >>> + * SPDX-License-Identifier: GPL-2.0+ >>> + */ >>> + >>> +#include <asm/io.h> >>> +#include <asm/arch/cpu.h> >>> +#include <asm/arch/tzpc.h> >>> + >>> +/* Configure Trust Zone Protection Controller */ >>> +void tzpc_init(void) >>> +{ >>> + struct sunxi_tzpc *tzpc = (struct sunxi_tzpc *)SUNXI_TZPC_BASE; >>> + >>> + /* Enable non-secure access to the RTC */ >>> + writel(SUNXI_TZPC_DECPORT0_RTC, &tzpc->decport0_set); >> >> Irk. Seem blow. >> >>> +} >>> diff --git a/arch/arm/include/asm/arch-sunxi/tzpc.h b/arch/arm/include/asm/arch-sunxi/tzpc.h >>> new file mode 100644 >>> index 0000000..ba4d43b >>> --- /dev/null >>> +++ b/arch/arm/include/asm/arch-sunxi/tzpc.h >>> @@ -0,0 +1,23 @@ >>> +/* >>> + * (C) Copyright 2015 Chen-Yu Tsai <wens@csie.org> >>> + * >>> + * SPDX-License-Identifier: GPL-2.0+ >>> + */ >>> + >>> +#ifndef _SUNXI_TZPC_H >>> +#define _SUNXI_TZPC_H >> >> Why is this sunxi specific? This seems to describe a standard ARM BP147 >> (http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dto0015a/index.html), >> so I'd expect the various offsets and the file location to reflect this. >> >> Another platform (FSL 2085) seems to use the same IP, so it would make >> sense to clean that part of the code too. > > I guess we could make this generic. Seems samsung uses this as well. > >>> + >>> +#ifndef __ASSEMBLY__ >>> +struct sunxi_tzpc { >>> + u32 r0size; /* 0x00 Size of secure RAM region */ >>> + u32 decport0_status; /* 0x04 Status of decode protection port 0 */ >>> + u32 decport0_set; /* 0x08 Set decode protection port 0 */ >>> + u32 decport0_clear; /* 0x0c Clear decode protection port 0 */ >>> +}; >> >> I'm not overly fond of this way to describe a set of contiguous >> register. It tends to be fairly fragile (the compiler is free to >> insert some padding) and prevents the use of offsets from assembly code. > > Seems this is how we are doing things in U-boot... > I'm OK either way. > >>> +#endif >>> + >>> +#define SUNXI_TZPC_DECPORT0_RTC (1 << 1) >>> + >>> +void tzpc_init(void); >> >> And this won't help if you have assembly code either. >> >>> + >>> +#endif /* _SUNXI_TZPC_H */ >> >> If I can suggest something, it would be to make this something generic, >> with a standard config option describing the base, a set of offsets >> from the base (as #defines), and then a set of accessors accessible from >> C code. >> >> LS2085 could also be cleaned up to reuse the same offsets (instead of >> hardcoding the register addresses). > > OK. Will look into it. As I was pointed out over IRC, the memory map is subtly different from the original BP147 (hey, being awkward is be a good thing, isn't it?), which makes it difficult to have common code for this. So let's not bother having a generic BP147 setup for now - at least not for sunxi. It could still be done for FSL and Samsung, but that'd be unfair to ask you to do this. Thanks, M.
Hi, On 25-08-15 04:49, Chen-Yu Tsai wrote: > On the A31s the RTC is by default secured. Thus when u-boot > loads the kernel in non-secure world, the RTC is unavailable. The > SoC has a TrustZone Protection Controller, which can be used to > enable non-secure access to the RTC. > > On the A31 the TZPC doesn't seem to do anything, i.e. changes to > its register contents do not affect access to the RTC. > > Signed-off-by: Chen-Yu Tsai <wens@csie.org> Thanks I've queued this up in my tree, and it will be part of my next pull-req for u-boot/master. Regards, Hans > --- > arch/arm/cpu/armv7/sunxi/Makefile | 1 + > arch/arm/cpu/armv7/sunxi/board.c | 5 +++++ > arch/arm/cpu/armv7/sunxi/tzpc.c | 18 ++++++++++++++++++ > arch/arm/include/asm/arch-sunxi/tzpc.h | 23 +++++++++++++++++++++++ > 4 files changed, 47 insertions(+) > create mode 100644 arch/arm/cpu/armv7/sunxi/tzpc.c > create mode 100644 arch/arm/include/asm/arch-sunxi/tzpc.h > > diff --git a/arch/arm/cpu/armv7/sunxi/Makefile b/arch/arm/cpu/armv7/sunxi/Makefile > index 76c7e55..459d5d8 100644 > --- a/arch/arm/cpu/armv7/sunxi/Makefile > +++ b/arch/arm/cpu/armv7/sunxi/Makefile > @@ -28,6 +28,7 @@ obj-$(CONFIG_MACH_SUN6I) += clock_sun6i.o > obj-$(CONFIG_MACH_SUN7I) += clock_sun4i.o > obj-$(CONFIG_MACH_SUN8I) += clock_sun6i.o > obj-$(CONFIG_MACH_SUN9I) += clock_sun9i.o > +obj-$(CONFIG_MACH_SUN6I) += tzpc.o > > obj-$(CONFIG_AXP152_POWER) += pmic_bus.o > obj-$(CONFIG_AXP209_POWER) += pmic_bus.o > diff --git a/arch/arm/cpu/armv7/sunxi/board.c b/arch/arm/cpu/armv7/sunxi/board.c > index f01846e..b40198b 100644 > --- a/arch/arm/cpu/armv7/sunxi/board.c > +++ b/arch/arm/cpu/armv7/sunxi/board.c > @@ -23,6 +23,7 @@ > #include <asm/arch/gpio.h> > #include <asm/arch/sys_proto.h> > #include <asm/arch/timer.h> > +#include <asm/arch/tzpc.h> > #include <asm/arch/mmc.h> > > #include <linux/compiler.h> > @@ -115,6 +116,10 @@ void s_init(void) > "orr r0, r0, #1 << 6\n" > "mcr p15, 0, r0, c1, c0, 1\n"); > #endif > +#if defined CONFIG_MACH_SUN6I > + /* Enable non-secure access to the RTC */ > + tzpc_init(); > +#endif > > clock_init(); > timer_init(); > diff --git a/arch/arm/cpu/armv7/sunxi/tzpc.c b/arch/arm/cpu/armv7/sunxi/tzpc.c > new file mode 100644 > index 0000000..5c9c69b > --- /dev/null > +++ b/arch/arm/cpu/armv7/sunxi/tzpc.c > @@ -0,0 +1,18 @@ > +/* > + * (C) Copyright 2015 Chen-Yu Tsai <wens@csie.org> > + * > + * SPDX-License-Identifier: GPL-2.0+ > + */ > + > +#include <asm/io.h> > +#include <asm/arch/cpu.h> > +#include <asm/arch/tzpc.h> > + > +/* Configure Trust Zone Protection Controller */ > +void tzpc_init(void) > +{ > + struct sunxi_tzpc *tzpc = (struct sunxi_tzpc *)SUNXI_TZPC_BASE; > + > + /* Enable non-secure access to the RTC */ > + writel(SUNXI_TZPC_DECPORT0_RTC, &tzpc->decport0_set); > +} > diff --git a/arch/arm/include/asm/arch-sunxi/tzpc.h b/arch/arm/include/asm/arch-sunxi/tzpc.h > new file mode 100644 > index 0000000..ba4d43b > --- /dev/null > +++ b/arch/arm/include/asm/arch-sunxi/tzpc.h > @@ -0,0 +1,23 @@ > +/* > + * (C) Copyright 2015 Chen-Yu Tsai <wens@csie.org> > + * > + * SPDX-License-Identifier: GPL-2.0+ > + */ > + > +#ifndef _SUNXI_TZPC_H > +#define _SUNXI_TZPC_H > + > +#ifndef __ASSEMBLY__ > +struct sunxi_tzpc { > + u32 r0size; /* 0x00 Size of secure RAM region */ > + u32 decport0_status; /* 0x04 Status of decode protection port 0 */ > + u32 decport0_set; /* 0x08 Set decode protection port 0 */ > + u32 decport0_clear; /* 0x0c Clear decode protection port 0 */ > +}; > +#endif > + > +#define SUNXI_TZPC_DECPORT0_RTC (1 << 1) > + > +void tzpc_init(void); > + > +#endif /* _SUNXI_TZPC_H */ >
diff --git a/arch/arm/cpu/armv7/sunxi/Makefile b/arch/arm/cpu/armv7/sunxi/Makefile index 76c7e55..459d5d8 100644 --- a/arch/arm/cpu/armv7/sunxi/Makefile +++ b/arch/arm/cpu/armv7/sunxi/Makefile @@ -28,6 +28,7 @@ obj-$(CONFIG_MACH_SUN6I) += clock_sun6i.o obj-$(CONFIG_MACH_SUN7I) += clock_sun4i.o obj-$(CONFIG_MACH_SUN8I) += clock_sun6i.o obj-$(CONFIG_MACH_SUN9I) += clock_sun9i.o +obj-$(CONFIG_MACH_SUN6I) += tzpc.o obj-$(CONFIG_AXP152_POWER) += pmic_bus.o obj-$(CONFIG_AXP209_POWER) += pmic_bus.o diff --git a/arch/arm/cpu/armv7/sunxi/board.c b/arch/arm/cpu/armv7/sunxi/board.c index f01846e..b40198b 100644 --- a/arch/arm/cpu/armv7/sunxi/board.c +++ b/arch/arm/cpu/armv7/sunxi/board.c @@ -23,6 +23,7 @@ #include <asm/arch/gpio.h> #include <asm/arch/sys_proto.h> #include <asm/arch/timer.h> +#include <asm/arch/tzpc.h> #include <asm/arch/mmc.h> #include <linux/compiler.h> @@ -115,6 +116,10 @@ void s_init(void) "orr r0, r0, #1 << 6\n" "mcr p15, 0, r0, c1, c0, 1\n"); #endif +#if defined CONFIG_MACH_SUN6I + /* Enable non-secure access to the RTC */ + tzpc_init(); +#endif clock_init(); timer_init(); diff --git a/arch/arm/cpu/armv7/sunxi/tzpc.c b/arch/arm/cpu/armv7/sunxi/tzpc.c new file mode 100644 index 0000000..5c9c69b --- /dev/null +++ b/arch/arm/cpu/armv7/sunxi/tzpc.c @@ -0,0 +1,18 @@ +/* + * (C) Copyright 2015 Chen-Yu Tsai <wens@csie.org> + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <asm/io.h> +#include <asm/arch/cpu.h> +#include <asm/arch/tzpc.h> + +/* Configure Trust Zone Protection Controller */ +void tzpc_init(void) +{ + struct sunxi_tzpc *tzpc = (struct sunxi_tzpc *)SUNXI_TZPC_BASE; + + /* Enable non-secure access to the RTC */ + writel(SUNXI_TZPC_DECPORT0_RTC, &tzpc->decport0_set); +} diff --git a/arch/arm/include/asm/arch-sunxi/tzpc.h b/arch/arm/include/asm/arch-sunxi/tzpc.h new file mode 100644 index 0000000..ba4d43b --- /dev/null +++ b/arch/arm/include/asm/arch-sunxi/tzpc.h @@ -0,0 +1,23 @@ +/* + * (C) Copyright 2015 Chen-Yu Tsai <wens@csie.org> + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#ifndef _SUNXI_TZPC_H +#define _SUNXI_TZPC_H + +#ifndef __ASSEMBLY__ +struct sunxi_tzpc { + u32 r0size; /* 0x00 Size of secure RAM region */ + u32 decport0_status; /* 0x04 Status of decode protection port 0 */ + u32 decport0_set; /* 0x08 Set decode protection port 0 */ + u32 decport0_clear; /* 0x0c Clear decode protection port 0 */ +}; +#endif + +#define SUNXI_TZPC_DECPORT0_RTC (1 << 1) + +void tzpc_init(void); + +#endif /* _SUNXI_TZPC_H */
On the A31s the RTC is by default secured. Thus when u-boot loads the kernel in non-secure world, the RTC is unavailable. The SoC has a TrustZone Protection Controller, which can be used to enable non-secure access to the RTC. On the A31 the TZPC doesn't seem to do anything, i.e. changes to its register contents do not affect access to the RTC. Signed-off-by: Chen-Yu Tsai <wens@csie.org> --- arch/arm/cpu/armv7/sunxi/Makefile | 1 + arch/arm/cpu/armv7/sunxi/board.c | 5 +++++ arch/arm/cpu/armv7/sunxi/tzpc.c | 18 ++++++++++++++++++ arch/arm/include/asm/arch-sunxi/tzpc.h | 23 +++++++++++++++++++++++ 4 files changed, 47 insertions(+) create mode 100644 arch/arm/cpu/armv7/sunxi/tzpc.c create mode 100644 arch/arm/include/asm/arch-sunxi/tzpc.h