On Thu, 2015-01-15 at 16:37 -0700, Stephen Warren wrote:
On 01/13/2015 12:45 PM, Ian Campbell wrote:
I will need mc_security_cfg0/1 in a future patch and I added the rest while debugging, so thought I might as well commit them.
diff --git a/arch/arm/include/asm/arch-tegra124/mc.h b/arch/arm/include/asm/arch-tegra124/mc.h
@@ -35,9 +35,40 @@ struct mc_ctlr { u32 mc_emem_adr_cfg; /* offset 0x54 */ u32 mc_emem_adr_cfg_dev0; /* offset 0x58 */ u32 mc_emem_adr_cfg_dev1; /* offset 0x5C */
- u32 reserved3[12]; /* offset 0x60 - 0x8C */
- u32 reserved3[4]; /* offset 0x60 - 0x6C */
- u32 mc_security_cfg0; /* offset 0x70 */
I didn't check the fields you've added, but this patch sounds fine.
One thing I did wonder though: rather than naming the reserved registers 1, 2, 3, 4, ... (which must be renumbered any time we add new registers in the middle of a previously reserved section), perhaps we should name the reserved registers after the base offset they cover,
That's a good idea, I'll do that for the header I'm touching at least.
e.g.:
- u32 reserved1[3]; /* offset 0x24 - 0x2C */
- u32 reserved24[3]; /* offset 0x24 - 0x2C */ u32 mc_smmu_tlb_flush; /* offset 0x30 */ u32 mc_smmu_ptc_flush; /* offset 0x34 */
- u32 reserved2[6]; /* offset 0x38 - 0x4C */
- u32 reserved38[6]; /* offset 0x38 - 0x4C */
I presume this isn't necessary for the T124 header since I presume you've filled out everything from the docs,
Up to the register I was interested in yes (at least I think so), but there are more past where I stopped, I think your suggestion makes good sense nonetheless.
The is another MC_SECURITY register further up which covers the base address bits 32 onwards, it resets to zero so since the secure region is <4G I don't strictly need to configure it, but I've been considering setting it any way for belt and braces reasons.
but perhaps it'd make it easier to extend this header to future chips assuming the MC register layout is similar but modified there?
Sounds plausible to me.
Ian.