-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
drivers: tzc380: add tzc380 driver #1578
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some minor comments but looks ok to me.
core/include/drivers/tzc380.h
Outdated
#define TZC_REGION_SIZE_16E (0x3f) | ||
|
||
#define TZC_REGION_SIZE_SHIFT (0x1) | ||
#define TZC_REGION_SIZE_MASK (0x3f << 1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#define TZC_REGION_SIZE_MASK GENMASK_32(7, 1);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix in next version.
core/include/drivers/tzc380.h
Outdated
#define TZC_REGION_SIZE_MASK (0x3f << 1) | ||
#define TZC_ATTR_REGION_SIZE(s) ((s) << TZC_REGION_SIZE_SHIFT) | ||
|
||
#define TZC_ATTR() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
empty macro ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Drop this in next version. Thanks.
core/include/drivers/tzc380.h
Outdated
#define TZC_SP_NS_W (1 << 0) | ||
#define TZC_SP_NS_R (1 << 1) | ||
#define TZC_SP_S_W (1 << 2) | ||
#define TZC_SP_S_R (1 << 3) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#define TZC_SP_NS_W BIT(0)
and so on
core/include/drivers/tzc380.h
Outdated
|
||
/* Speculation is enabled by default. */ | ||
#define SPECULATION_CTRL_WRITE_DISABLE (1 << 1) | ||
#define SPECULATION_CTRL_READ_DISABLE (1 << 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
prefer use BIT(1)
and BIT(0)
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix to use BIT for all the bits things in next version
core/include/drivers/tzc380.h
Outdated
|
||
#include <stdint.h> | ||
#include <types_ext.h> | ||
#include <trace_levels.h> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shift type_ext.h below trace_levels.h
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix in next version.
core/drivers/tzc380.c
Outdated
DMSG("region_base: 0x%08x%08x", temp_32reg_h, temp_32reg); | ||
temp_32reg = tzc_read_region_attributes(tzc.base, n); | ||
DMSG("region sp: %x", temp_32reg >> TZC_ATTR_SP_SHIFT); | ||
DMSG("region size: %x\n", (temp_32reg & TZC_REGION_SIZE_MASK) >> TZC_REGION_SIZE_SHIFT); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: exeeds 80 chars
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix in next version.
* CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) | ||
* ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE | ||
* POSSIBILITY OF SUCH DAMAGE. | ||
*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tzc400.c also includes an ARM-TF notice. Should it be dumped here also ?
* Copyright (c) 2014, ARM Limited and Contributors. All rights reserved.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no tzc380 support in ARM-TF.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking of the tzc400.c from optee which is almost the same as this proposed tzc380.c.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I take tzc400.c as a reference to implement tzc380 support. Simliar functions are implemented in tzc380. But code is not copied from tzc400.c. I am not sure the copyright is needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok. discard my comments. thanks.
Updated with comments addressed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minor remaining comments. including several parenthesis that could be removed from core/include/drivers/tzc380.h.
Aside these: Acked-by: Etienne Carriere <etienne.carriere@linaro.org>
core/drivers/tzc380.c
Outdated
static uint32_t addr_high(vaddr_t addr __unused) | ||
{ | ||
#if (UINTPTR_MAX == UINT64_MAX) | ||
return (addr >> 32); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor style: no need parenthesis
core/drivers/tzc380.c
Outdated
return (uint32_t)addr; | ||
} | ||
|
||
static uint32_t addr_high(vaddr_t addr __unused) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor: prefer __maybe_unused
core/drivers/tzc380.c
Outdated
temp_32reg = tzc_read_region_attributes(tzc.base, n); | ||
DMSG("region sp: %x", temp_32reg >> TZC_ATTR_SP_SHIFT); | ||
DMSG("region size: %x\n", (temp_32reg & TZC_REGION_SIZE_MASK) | ||
>> TZC_REGION_SIZE_SHIFT); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor style: end line175 with the operator >>
.
Add tzc380 driver support. The usage: Use tzc_init(vaddr_t base) to get the tzc380 configuration. Use tzc_configure_region to configure the memory region, such as "tzc_configure_region(5, 0x4e000000, TZC_ATTR_REGION_SIZE(TZC_REGION_SIZE_32M) | TZC_ATTR_REGION_EN_MASK | TZC_ATTR_SP_S_RW);" Signed-off-by: Peng Fan <peng.fan@nxp.com> Acked-by: Etienne Carriere <etienne.carriere@linaro.org>
@etienne-lms Updated. |
|
@etienne-lms tags already applied, thanks:) |
The failed build status is due to a timeout (Travis seems to be slower than usual today), the final check was almost complete so I'm merging this regardless. |
Add tzc380 driver support.
The usage:
Use tzc_init(vaddr_t base) to get the tzc380 configuration.
Use tzc_configure_region to configure the memory region,
such as "tzc_configure_region(5, 0x4e000000,
TZC_ATTR_REGION_SIZE(TZC_REGION_SIZE_32M) | TZC_ATTR_REGION_EN_MASK |
TZC_ATTR_SP_S_RW);"
An example for 6ULEVK: