[U-Boot] [PATCH v1 1/4] tegra124: Add more registers to struct mc_ctlr

Ian Campbell
2015-01-16T09:32:55Z

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.